From bc54150fc97ba7699df276dd26c2ac0504966d95 Mon Sep 17 00:00:00 2001 From: ohdearquant <122793010+ohdearquant@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:21:57 -0400 Subject: [PATCH 1/4] instrument: the lock-holder and scheduled-job instruments, a notice told once its send lands, and a positive check-command test --- apps/cli/lion_cli/agent.py | 4 +- .../014-instrument/ADR-0014-the-instrument.md | 47 +++-- docs/adr/015-chores/ADR-0015-chores.md | 4 +- docs/adr/INDEX.md | 2 +- docs/adr/PLAN.md | 6 +- hub/agent/instruments.py | 177 +++++++++++++++++- tests/test_agent.py | 95 ++++++++++ tests/test_chores.py | 33 ++++ tests/test_instruments.py | 163 ++++++++++++++++ 9 files changed, 509 insertions(+), 22 deletions(-) diff --git a/apps/cli/lion_cli/agent.py b/apps/cli/lion_cli/agent.py index 0a25eb3..143f5ad 100644 --- a/apps/cli/lion_cli/agent.py +++ b/apps/cli/lion_cli/agent.py @@ -219,11 +219,13 @@ async def arm(chores: Chores, agent: Agent) -> tuple[int, list[str]]: def render(name: str, inst: Any, out: Outcome) -> str: - """One instrument's outcome as the landing file and the console show it.""" + """One instrument's outcome as the landing file and the console show it, unvetted, and saying so.""" control = "ok" if out.control else "FAILED" lines = [ f"{name}: control {control}, {len(out.findings)} finding(s), escalate={out.escalate}, " f"count={out.count}, direction {inst.direction}", + "unvetted: run outside the chore path, so no refusal ran over it (a blank population, predicate, " + "count or known-positive passes), nothing was sent and no ledger row was written", f"answers: {inst.answers}", f"known-positive: {out.positive or '(none)'}", f"population: {out.population}\npredicate: {out.predicate}", diff --git a/docs/adr/014-instrument/ADR-0014-the-instrument.md b/docs/adr/014-instrument/ADR-0014-the-instrument.md index 04ac055..ddcf14a 100644 --- a/docs/adr/014-instrument/ADR-0014-the-instrument.md +++ b/docs/adr/014-instrument/ADR-0014-the-instrument.md @@ -1,7 +1,7 @@ --- adr: ADR-0014 status: draft -liveness: operating (eleven instruments build from one config and run under tests against scripted commands and stores; the command-line check prints a measurement unvetted and has no test; a lock-holder and a scheduled-job instrument are owed) +liveness: operating (thirteen instruments build from one config and run under tests against scripted commands and stores, a lock holder and a scheduled job among them; the command-line check prints its measurement unvetted, says so, and is under test) date: "2026-09-25" area: instrument kind: new @@ -149,8 +149,8 @@ one default floor of 300 GiB; an empty list declines every floor. ### C7: An instrument reads and acts on nothing _(enforced: process)_ ^c7 -An instrument reads: `gh` reads, `git` reads, `ps`, `launchctl print`, `df`, `find` and `du`, and -the store through the agent's bounded client, which admits only the verbs on its list +An instrument reads: `gh` reads, `git` reads, `ps`, `lsof`, `launchctl print`, `df`, `find` and +`du`, and the store through the agent's bounded client, which admits only the verbs on its list ([[ADR-0015-chores|ADR-0015]], chores). It writes only its snapshot or cursor to the agent's note store ([[ADR-0007-the-record#^d3|ADR-0007/D3]]) and sends nothing; the report is the handler's. A remedy, a restart or a removal, is the owner's. @@ -176,13 +176,14 @@ direction, cadence and question. ### D1: One builder names the instruments from the config ^d1 Serves C3 and C8. `instruments(cfg, khive, state)` builds only what the `[chores.]` tables -name, eleven names in all, with their arguments and a cadence from `every` (none: on ask only). +name, thirteen names in all, with their arguments and a cadence from `every` (none: on ask only). Constructors refuse a malformed declaration at build; `Chores` refuses an error direction outside the two and a blank `answers`. `every` is a store repeat such as `every:30m` or `daily`; `gh` and `git` resolve once to an absolute path where one is found. - **Landing evidence**: - `test_instruments_wires_the_desk_chores_and_signs_as_the_owners_desk_by_default` in + `test_instruments_wires_the_desk_chores_and_signs_as_the_owners_desk_by_default` and + `test_instruments_builds_the_lock_holder_on_the_agents_own_pid_file_and_the_scheduled_jobs` in `tests/test_instruments.py`; `test_a_chore_without_an_error_direction_or_a_question_is_refused_at_build` in `tests/test_chores.py`. @@ -238,9 +239,26 @@ sizes each once with `/usr/bin/du`, reporting those over `target_cap_gib`, 20 by Serves C8, and stands outside C1 to C3. The command builds the agent from its directory, takes the identity's lock (refused while the agent serves there), calls `run({})`, prints `render()` and writes the text to a dated file in the agent directory, the day's latest run of that name kept. -Nothing is sent, no ledger row is written, and `_vet` is not called. +Nothing is sent, no ledger row is written, and `_vet` is not called; the text's second line says so. -- **Landing evidence**: none; no test drives `--check`, `render` or `land` (S3). +- **Landing evidence**: in `tests/test_agent.py`, + `test_check_runs_one_instrument_prints_and_lands_it_and_sends_nothing`, + `test_render_prints_the_measurement_unvetted_and_says_so`, + `test_land_writes_the_days_file_of_that_name_and_the_latest_run_wins` (S3). + +### D7: The `lock-holder` and `scheduled-job` instruments ^d7 + +Serves C1 and C7. `LockHolder.run` lists the named lock files and the agent's own pid file in one +`lsof` call and resolves every pid to its full command line with one `ps` read; its control is this +process holding its own pid file. `ScheduledJob.run` reads each artifact's age against its bound, +then `launchctl print` for a named label's last exit. + +- **Landing evidence**: in `tests/test_instruments.py`, + `test_lock_holder_resolves_each_holders_pid_to_its_full_command_line_beside_its_own_held_lock`, + `test_lock_holder_fails_its_control_when_its_own_held_lock_does_not_read_as_its_own`, + `test_lock_holder_reads_a_lock_a_live_process_holds_through_the_real_lsof_and_ps`, + `test_scheduled_job_reads_the_artifact_first_then_the_supervisors_last_exit`, + `test_scheduled_job_fails_its_control_on_an_absent_directory_or_a_row_without_its_exit` (S9). ## Alternatives @@ -265,8 +283,9 @@ Nothing is sent, no ledger row is written, and `_vet` is not called. transition is not reported to the owner again when its report was not delivered, or when a run off the chore path measured it: `lion agent --check`, or a desk answer (one narrowed to named pull requests spares only the `required-contexts` and `ci-red` snapshots). -- **S3**: `lion agent --check` prints a measurement unvetted and has no test; a missing - known-positive prints as "(none)" and nothing refuses it. +- **S3**: `lion agent --check` prints a measurement unvetted and its text says so: no refusal ran, + nothing was sent, no ledger row was written. A missing known-positive prints as "(none)" and + nothing refuses it; the tests in D6 drive the check, `render` and `land`. - **S4**: A full page of 100 open pull requests costs one `gh pr view` per pull request the last snapshot held and the page lacks, on every `pr-state` run. `verdict-freshness` has no full-page check and calls a pull request beyond the page no longer open. @@ -281,11 +300,11 @@ Nothing is sent, no ledger row is written, and `_vet` is not called. - **S8**: The boundary: this record is product-side and names no kernel concern. An instrument reads with the credentials and namespace its process was started with; a read they deny is a failed control, or an empty answer the control must catch (A1). -- **S9**: Two instrument shapes are decided and owed, and no code builds them. A lock holder is read - as a pid resolved to its full command line beside a lock known to be held, never by matching text - or by taking the lock. A scheduled job is read by its artifact's freshness first and its - supervisor's exit second, as `served` reads a served agent: its last-wake note before its pid and - its launchd row. +- **S9**: Two instrument shapes are built (D7). `lock-holder` reads a pid resolved to its full + command line beside a lock known to be held, the agent's own pid file, never by matching text or + by taking the lock. `scheduled-job` reads the artifact's freshness first and the supervisor's exit + second, as `served` reads a last-wake note before a pid. `lsof` names the processes with the file + open, so a waiter reads as a holder; the only supervisor read is a launchd row. - **S10**: `comm.probe` returns a page of at most 100 rows and caps its stale count at 1000, where 1000 means at least that many. `inbox-sla` prints the count as it came, so a capped count reads as exact; its population is the rows on the probe pages. diff --git a/docs/adr/015-chores/ADR-0015-chores.md b/docs/adr/015-chores/ADR-0015-chores.md index 9a8c08b..597cd3c 100644 --- a/docs/adr/015-chores/ADR-0015-chores.md +++ b/docs/adr/015-chores/ADR-0015-chores.md @@ -79,7 +79,7 @@ No findings is quiet: a row, and no mail beyond the answer a question is owed. F to the owner. An escalation, marked so, goes to the owner and the steward; the agent never performs the remedy. A chore runs as one command inside a wake ([[ADR-0013-the-agent|ADR-0013]]). -### C2: The chores are the ones `chores.toml` names, from a fixed set of eleven _(enforced: mechanical)_ ^c2 +### C2: The chores are the ones `chores.toml` names, from a fixed set of thirteen _(enforced: mechanical)_ ^c2 - **Subject**: every agent built from its directory. - **Violated when**: a chore runs that the file does not name, or one is built without its error @@ -97,6 +97,8 @@ and what it keeps between runs in the agent's notes: | `verdict-freshness` | the newest review verdict file per open pull request against its head; the pull requests from `gh` or a configured enumerator | one snapshot | over-report | | `ci-red` | the failing lines at each red head, from `gh run view --log-failed` or a local CI transcript | a snapshot per repository | under-report | | `served` | each served agent's last-wake note, pid file and launchd row | nothing | under-report | +| `lock-holder` | the pids holding each named lock file open (`lsof`), each with its full command line (`ps`) | nothing | under-report | +| `scheduled-job` | each job's artifact age against its bound, then its launchd row's last exit | nothing | under-report | | `gtd-inbox` | the assignee's inbox tasks older than the triage age, through the store | nothing | under-report | | `inbox-sla` | unread mail older than the age per watched actor, by `comm.probe`, no bodies | nothing | under-report | | `disk` | free space per volume against each named floor (`df`), and build targets over a size cap (`find`, `du`) | nothing | over-report | diff --git a/docs/adr/INDEX.md b/docs/adr/INDEX.md index 2659b64..61c6700 100644 --- a/docs/adr/INDEX.md +++ b/docs/adr/INDEX.md @@ -15,7 +15,7 @@ | [ADR-0011](011-box/ADR-0011-the-box.md) | 011-box | draft | operating (three boxes under tests and live runs, the coding tools under tests and one live console run, the watch under tests and on the bench; the real-box tests are opt-in) | ADR-0002, ADR-0003, ADR-0005, ADR-0006, ADR-0007 | | [ADR-0012](012-bench/ADR-0012-the-bench.md) | 012-bench | draft | operating (the set runner, the in-box grader, the closure, the run identity and both arms are under tests; that compared bench runs share instances, head and budgets is the reader's check; no CLI-arm bench run on the hard set has the package indexes closed) | ADR-0002, ADR-0003, ADR-0006, ADR-0009 | | [ADR-0013](013-agent/ADR-0013-the-agent.md) | 013-agent | draft | operating (the wake, the gate, the caps, the hop count, the cursor, the lock and the continuous run with its checkpoint are pinned by tests; supervision and the lease are not in the process) | ADR-0001, ADR-0002, ADR-0003, ADR-0007, ADR-0010 | -| [ADR-0014](014-instrument/ADR-0014-the-instrument.md) | 014-instrument | draft | operating (eleven instruments build from one config and run under tests against scripted commands and stores; the command-line check prints a measurement unvetted and has no test; a lock-holder and a scheduled-job instrument are owed) | ADR-0005, ADR-0007 | +| [ADR-0014](014-instrument/ADR-0014-the-instrument.md) | 014-instrument | draft | operating (thirteen instruments build from one config and run under tests against scripted commands and stores, a lock holder and a scheduled job among them; the command-line check prints its measurement unvetted, says so, and is under test) | ADR-0005, ADR-0007 | | [ADR-0015](015-chores/ADR-0015-chores.md) | 015-chores | draft | operating (the handler, the gate, the store boundary, the ticks and the catch-up are pinned by tests with scripted backends and a fake store; no test drives a positive hand-run check; the catch-up's store-side filters are owed) | ADR-0001, ADR-0005, ADR-0007, ADR-0010 | | [ADR-0016](016-command/ADR-0016-the-lion-command.md) | 016-command | draft | partial (the command tree, the shared lock and the spend row are under tests; the text `--stats` prints has no test; a mark for an unknown or partial cost is owed) | ADR-0007, ADR-0009, ADR-0010 | | [ADR-0017](017-desk/ADR-0017-the-desk.md) | 017-desk | draft | operating (every claim is pinned by tests with scripted backends and a stubbed store; no bench exercises the desk; the gate for a clarify's answer mailed to the desk is owed, S8) | ADR-0001, ADR-0002, ADR-0003, ADR-0010 | diff --git a/docs/adr/PLAN.md b/docs/adr/PLAN.md index b80df8b..4493a4c 100644 --- a/docs/adr/PLAN.md +++ b/docs/adr/PLAN.md @@ -36,8 +36,8 @@ sandboxed executor are the published side of that boundary; the rest is named by | ADR-0011 | The box: one shape for three boxes, the coding tools over a tree, the watch and its gate, the patch | draft (2026-09-25); the real-box tests opt-in; resolving a boxed path in the box owed | the sandboxed executor: content-addressed trees, declared write paths, receipts | | ADR-0013 | The long-running agent: the wake, the send gate and the hop count, the caps, the cursor and posture, the lock, the continuous run and its checkpoint | draft (2026-09-25) | the process: one identity per process, lease by turn activity, one supervisor | | ADR-0012 | The bench as the instrument: the grade in the box, the row, the run id, unaided by default, what compares with what, the cited figures | draft (2026-09-25); one carried figure not reproduced | the instrument contract | -| ADR-0014 | The instrument contract: the measurement and its proof, the refusals, the dead instrument, first sight then transitions, the floors | draft (2026-09-25); the command-line check unvetted and untested; a lock-holder and a scheduled-job instrument, a dead mark on delivery owed | none | -| ADR-0015 | Chores: three endings chosen by code, eleven chores from one file, ticks as store reminders, the bounded client, print and save | draft (2026-09-25); no test drives a positive hand-run check; the catch-up's store-side filters owed | the scheduled run of a process | +| ADR-0014 | The instrument contract: the measurement and its proof, the refusals, the dead instrument, first sight then transitions, the floors | draft (2026-09-25); the command-line check unvetted and saying so; a dead mark on delivery owed | none | +| ADR-0015 | Chores: three endings chosen by code, thirteen chores from one file, ticks as store reminders, the bounded client, print and save | draft (2026-09-25); no test drives a positive hand-run check; the catch-up's store-side filters owed | the scheduled run of a process | | ADR-0016 | The lion command and the spend row: one entry, the shared lock, the row from the envelopes, the readers | draft (2026-09-25); the unknown-cost mark owed | none | | ADR-0017 | The desk: the read-only reader, the cursor that never marks, the gate, code escalations, every model-facing command with its gate, pending until its row, keyed replay, record answers, the brief, areas and chairs | draft (2026-09-25); no bench exercises the desk | mail settlement by keyed replay | | ADR-0018 | Chat in a box: one run that waits for the person, the flags, the person's tree or a copy, the patch home, the resume | draft (2026-09-25); no test interrupts a boxed chat | none | @@ -166,7 +166,7 @@ hard-set pair as code-host-closed, not unaided (0012 C8). says what the instance cost and an unmeasured spend is unknown, never zero; a run id is one bench run and a disagreeing launch is refused; the bench runs unaided unless a launch says otherwise, and unaided is the number; cited figures belong to named bench runs (0012 C1 to C5, C9). -- A chore ends quiet, reported or escalated and code picks the ending; the chores are the eleven the +- A chore ends quiet, reported or escalated and code picks the ending; the chores are the thirteen the file names; a chore runs on its tick, a trusted question or a missed tick, inside a wake, as the agent's actor behind the gate, reaching the store only through the bounded client; a report names its prior and the steward hears only what the owner left (0015 C1 to C5, C7). diff --git a/hub/agent/instruments.py b/hub/agent/instruments.py index 9ebc360..0a16ea3 100644 --- a/hub/agent/instruments.py +++ b/hub/agent/instruments.py @@ -1,6 +1,7 @@ """The instruments: the read-only measurements an actor's long-running agent runs as chores -(hub.agent.chores). gh for PR state, ps and launchctl for served things, khive for a gtd inbox and -mail, df and du for disk, git for worktrees, the bench actor's notes for its jobs. +(hub.agent.chores). gh for PR state, ps and launchctl for served things and scheduled jobs, lsof and +ps for lock holders, khive for a gtd inbox and mail, df and du for disk, git for worktrees, the bench +actor's notes for its jobs. Every instrument runs its known-positive in the invocation that produces its answer and hands what it read to the handler, which refuses an outcome without one, so an empty answer from a broken instrument escalates instead of reading clean. `instruments(cfg, ...)` builds the ones a @@ -37,9 +38,12 @@ "GH", "GIT", "LAUNCHCTL", + "LSOF", "sh", "PrState", "Served", + "LockHolder", + "ScheduledJob", "GtdInbox", "InboxSla", "Disk", @@ -78,6 +82,7 @@ def _bin(name: str, *fallbacks: str) -> str: GH = _bin("gh", "/opt/homebrew/bin/gh", "/usr/local/bin/gh") GIT = _bin("git", "/usr/bin/git") LAUNCHCTL = "/bin/launchctl" +LSOF = _bin("lsof", "/usr/sbin/lsof", "/usr/bin/lsof") async def sh(argv: list[str], *, timeout: float = 60, cwd: str | None = None) -> tuple[int, str, str]: @@ -512,6 +517,163 @@ async def run(self, args: dict[str, Any]) -> Outcome: ) +class LockHolder: + """Who holds each named lock: the processes lsof finds with the lock's file open, each pid resolved + to its full command line by `ps`; never by matching command text and never by taking the lock. The + control is a lock known to be held, the agent's own pid file, read in the same lsof call: this + process must come back as its holder, and its own pid through the same `ps` read.""" + + name = "lock-holder" + about = "each named lock file: the pids holding it open, with their full command lines; reports holders" + answers = "which processes hold each named lock file open right now, by pid and full command line" + direction = "under-report" + mechanism = "lsof lists only the processes this user may inspect; a holder beyond them reads as no holder" + + def __init__( + self, + locks: list[str], + held: str, + *, + every: str | None, + run: Runner = sh, + empty_expected: bool = False, + ): + self.locks, self.held, self.every, self.sh = list(locks), held, every, run + self.empty_expected = empty_expected + + async def run(self, args: dict[str, Any]) -> Outcome: + paths = [self.held, *self.locks] + _, out, err = await self.sh([LSOF, "-w", "-F", "pn", "--", *paths]) # rc 1: some file open by none + opened: dict[str, list[int]] = {} + pid = None + for line in out.splitlines(): + if line[:1] == "p" and line[1:].isdigit(): + pid = int(line[1:]) + elif line[:1] == "n" and pid is not None and pid not in opened.setdefault(line[1:], []): + opened[line[1:]].append(pid) + # lsof names a file by its resolved path (/tmp reads /private/tmp on macOS) + holders = {p: opened.get(os.path.realpath(p), []) for p in paths} + me = os.getpid() + pids = sorted({me, *(p for ps in holders.values() for p in ps)}) + _, out, _ = await self.sh(["/bin/ps", "-ww", "-o", "pid=,command=", "-p", ",".join(map(str, pids))]) + commands = {} + for line in out.splitlines(): + head, _, command = line.strip().partition(" ") + if head.isdigit(): + commands[int(head)] = command.strip() + own = commands.get(me, "") + control = me in holders[self.held] and bool(own) + positive = f"own lock {self.held} held by own pid {me}: {own[:120]}" if control else "" + lines = [ + "control: " + + ( + positive + or f"own lock {self.held} open by {holders[self.held] or 'no pid'}, own pid {me}, its " + f"command line {'read' if own else 'unread'} (NOT held by this process: read broken)" + ) + ] + findings, matched = [], 0 + for lock in self.locks: + if not holders[lock]: + lines.append(f"{lock}: no holder") + continue + matched += 1 + for p in holders[lock]: + command = commands.get(p) or "(no command line: the pid exited between the reads)" + lines.append(f"{lock}: pid {p}: {command}") + findings.append(f"{lock}: held by pid {p}: {command[:200]}") + if err.strip(): + lines.append(f"lsof: {err.strip()[:300]}") + return Outcome( + findings=tuple(findings), + evidence="\n".join(lines), + control=control, + population=f"lock files named: {self.locks}", + predicate=f"open by a process ({matched} match)", + count=len(self.locks), + positive=positive, + ) + + +class ScheduledJob: + """Scheduled jobs, artifact first: the file each job writes is younger than its bound, then its + supervisor's last exit, read from its launchd row when it names one, in that order of weight (a + clean exit beside a stale artifact is a finding), as `served` reads a served agent. The finding + names the bound and the age stays in the evidence, so a stale artifact is told once. The control is + the directory each artifact is written into: an artifact absent from an absent directory is a + wrong path or an unmounted volume, which the artifact read alone cannot tell from a job that never + ran; a loaded row without its last exit is an unread supervisor.""" + + name = "scheduled-job" + about = "scheduled jobs: artifact age first, the supervisor's last exit second; reports stale and failed" + answers = ( + "has each scheduled job written its artifact within its bound, and did its supervisor's last " + "run exit 0, read in that order of weight" + ) + direction = "under-report" + mechanism = "an artifact another writer touched reads fresh whether or not the job ran" + + def __init__( + self, jobs: list[dict], *, every: str | None, run: Runner = sh, empty_expected: bool = False + ): + self.jobs, self.every, self.sh = [dict(j) for j in jobs], every, run + self.empty_expected = empty_expected + for j in self.jobs: + if "artifact" not in j or "max_age_h" not in j: + raise ValueError(f"a scheduled job names its artifact and its max_age_h: {j}") + + async def run(self, args: dict[str, Any]) -> Outcome: + findings, lines, positives, control, matched = [], [], [], True, 0 + for j in self.jobs: + art, bound = Path(j["artifact"]), float(j["max_age_h"]) + line, bad = f"{art}:", False + if not art.parent.is_dir(): + control = False + line += " its directory is absent (a wrong path reads like a job that never ran)" + elif not art.exists(): + positives.append(f"{art.parent} present") + bad = True + line += " absent" + findings.append(f"{art}: absent, bound {bound:g}h") + else: + positives.append(f"{art.parent} present") + age_h = (time.time() - art.stat().st_mtime) / 3600 + line += f" written {age_h:.1f}h ago, bound {bound:g}h" + if age_h > bound: + bad = True + findings.append(f"{art}: older than its {bound:g}h bound") + if j.get("label"): + rc, out, _ = await self.sh([LAUNCHCTL, "print", f"gui/{os.getuid()}/{j['label']}"]) + m = re.search(r"^\s*last exit code = (\(never exited\)|-?\d+)", out, re.M) + if rc != 0: + bad = True + line += f" | launchd {j['label']}: not loaded" + findings.append(f"{art}: its supervisor {j['label']} is not loaded") + elif m is None: + control = False + line += f" | launchd {j['label']}: loaded, no last exit code in its row (unread)" + else: + positives.append(f"launchd {j['label']} row read") + line += f" | launchd {j['label']}: last exit {m.group(1)}" + if m.group(1) not in ("0", "(never exited)"): + bad = True + findings.append(f"{art}: its supervisor {j['label']} last exited {m.group(1)}") + matched += bad + lines.append(line) + return Outcome( + findings=tuple(findings), + evidence="\n".join(lines), + control=control, + population=f"scheduled jobs configured: {[j['artifact'] for j in self.jobs]}", + predicate=( + "artifact absent or past its bound, or supervisor not loaded or its last exit nonzero " + f"({matched} match)" + ), + count=len(self.jobs), + positive="; ".join(positives) if control else "", + ) + + class GtdInbox: """The assignee's gtd inbox rows past the triage SLA. Reads through khive as the agent (the assignee's namespace is visible to it); the control, in the same call, is that the assignee has @@ -1897,6 +2059,17 @@ def empty(a: dict, default: bool) -> bool: if "served" in c: a = c["served"] out["served"] = Served(a["agents"], every=a.get("every"), empty_expected=empty(a, False)) + if "lock-holder" in c: # the lock known to be held: this agent's own pid file, which its process holds + a = c["lock-holder"] + out["lock-holder"] = LockHolder( + a["locks"], + str(Path(khive.cwd) / "agent.pid"), + every=a.get("every"), + empty_expected=empty(a, False), + ) + if "scheduled-job" in c: + a = c["scheduled-job"] + out["scheduled-job"] = ScheduledJob(a["jobs"], every=a.get("every"), empty_expected=empty(a, False)) if "gtd-inbox" in c: a = c["gtd-inbox"] out["gtd-inbox"] = GtdInbox( diff --git a/tests/test_agent.py b/tests/test_agent.py index 5e145aa..c4ee206 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -733,6 +733,101 @@ async def fake_exec(self, op): ) +class Measured: + """An instrument whose one outcome the test chooses.""" + + direction, answers, every, about, mechanism, empty_expected = ( + "under-report", + "what the test pointed it at", + None, + "a scripted instrument", + "the test says so", + False, + ) + + def __init__(self, out): + self.out, self.args = out, [] + + async def run(self, args): + self.args.append(args) + return self.out + + +def test_render_prints_the_measurement_unvetted_and_says_so(): + from lion_cli.agent import render + + from hub.agent.chores import Outcome + + blank = Outcome(findings=("a finding",), evidence="the instrument's own line") + assert render("probe", Measured(blank), blank).splitlines() == [ + "probe: control ok, 1 finding(s), escalate=False, count=None, direction under-report", + "unvetted: run outside the chore path, so no refusal ran over it (a blank population, predicate, " + "count or known-positive passes), nothing was sent and no ledger row was written", + "answers: what the test pointed it at", + "known-positive: (none)", + "population: ", + "predicate: ", + "- a finding", + "the instrument's own line", + ] + failed = Outcome(control=False, escalate=True, population="rows", predicate="p", count=0, positive="x") + text = render("probe", Measured(failed), failed) + assert text.startswith("probe: control FAILED, 0 finding(s), escalate=True, count=0,") + assert "known-positive: x\npopulation: rows\npredicate: p" in text + + +def test_land_writes_the_days_file_of_that_name_and_the_latest_run_wins(tmp_path): + from lion_cli.agent import land + + path = land(tmp_path, "check-probe", "first") + assert path.parent == tmp_path / "landing" and re.fullmatch(r"\d{8}-check-probe\.txt", path.name) + assert land(tmp_path, "check-probe", "second") == path and path.read_text() == "second" + assert sorted(p.name for p in (tmp_path / "landing").iterdir()) == [path.name] + + +def test_check_runs_one_instrument_prints_and_lands_it_and_sends_nothing( + tmp_path, kkernel_stub, monkeypatch, capsys +): + """`--check `: the instrument runs once with no args under the identity's lock, its unvetted + measurement is printed and landed, and no mail, ledger row or store call follows.""" + from hub.agent.chores import Outcome + + d = tmp_path / "agent" + (d / ".khive").mkdir(parents=True) + (d / ".khive" / "config.toml").write_text('[actor]\nid = "lambda:x:agent"\n') + (d / "chores.toml").write_text('actor = "lambda:x:agent"\nowner = "lambda:x"\nsteward = "lambda:owner"\n') + ops: list[str] = [] + + async def fake_exec(self, op): + ops.append(op) + return [{}] + + monkeypatch.setattr(Khive, "exec", fake_exec) + held: list[bool] = [] + probe = Measured(Outcome(population="rows", predicate="probed", count=0, evidence="read 0 rows")) + orig = probe.run + + async def run(args): # the identity is held while the instrument runs + held.append((d / "agent.pid").read_text().strip() == str(os.getpid())) + return await orig(args) + + probe.run = run + monkeypatch.setattr("lion_cli.agent.instruments", lambda cfg, khive, state: {"probe": probe}) + base = dict(dir=str(d), stats=False, model=None, ticks=False, arm=False, once=False, rerun=None, wait=0) + asyncio.run(agent_main(argparse.Namespace(**base, check="probe"))) + printed = capsys.readouterr().out + landed = Path(printed.splitlines()[-1].removeprefix("landed: ")) + assert landed.parent == d / "landing" and re.fullmatch(r"\d{8}-check-probe\.txt", landed.name) + assert printed == landed.read_text() + f"\nlanded: {landed}\n" + assert printed.startswith( + "probe: control ok, 0 finding(s), escalate=False, count=0, direction under-report\n" + ) + # unvetted: an empty population nobody declared expected and a blank known-positive pass unrefused + assert "unvetted: run outside the chore path" in printed and "known-positive: (none)" in printed + assert probe.args == [{}] and held == [True] and ops == [] + assert not (d / "ledger.jsonl").exists() and not (d / "agent.pid").exists() + + def test_serve_keeps_the_lock_its_process_already_claimed(tmp_path): """The CLI claims before its first act; serve takes that lock over instead of claiming it again, and lets it go when it stops.""" diff --git a/tests/test_chores.py b/tests/test_chores.py index cfa8976..900c16e 100644 --- a/tests/test_chores.py +++ b/tests/test_chores.py @@ -452,6 +452,39 @@ def notices() -> list[str]: assert notices() == [OWNER, STEWARD] # every copy landed: told once +def test_a_dead_instrument_notice_that_reached_nobody_leaves_the_crossing_untold_until_a_send_lands( + tmp_path, monkeypatch +): + ticks, start = itertools.count(), datetime(2026, 9, 22) + monkeypatch.setattr( + "hub.agent.chores._now", + lambda: (start + timedelta(seconds=next(ticks))).strftime("%Y-%m-%d %H:%M:%S"), + ) + khive = FakeKhive(*(mail(SELF, "tick: disk", mid=f"m{i}000000") for i in range(6))) + khive.refuse = {OWNER, STEWARD} # neither copy is delivered + dead = Outcome(findings=("NOT-MEASURED",), evidence="rc 1", control=False) + chores, agent = build(tmp_path, khive, [CHECK, DONE] * 6, disk=Probe("disk", dead)) + + def notices() -> list[str]: + return [s["to"] for s in khive.sent if s["subject"] == "ESCALATE disk: instrument dead"] + + for _ in range(3): + run = asyncio.run(agent.wake()) + assert notices() == [] and agent.state.get("dead_told").value == [] # the crossing, told to nobody + assert any( + f"told ['{OWNER} undelivered: hop limit" in r and f"'{STEWARD} undelivered: hop limit" in r + for r in results(run) + ) + run = asyncio.run(agent.wake()) # still undelivered: still untold, so the notice is owed again + assert any("instrument dead after 4 unanswered run(s)" in r for r in results(run)) + assert notices() == [] and agent.state.get("dead_told").value == [] + khive.refuse = set() + asyncio.run(agent.wake()) # delivered: now the chore is marked told + assert notices() == [OWNER, STEWARD] and agent.state.get("dead_told").value == ["disk"] + asyncio.run(agent.wake()) + assert notices() == [OWNER, STEWARD] # told once + + def test_an_ask_is_answered_on_its_thread_whatever_the_outcome_and_the_owner_is_not_told_twice(tmp_path): khive = FakeKhive( mail(OWNER, "check disk please"), mail(STEWARD, "disk?", mid="b2c3d4e5", thread="t-steward") diff --git a/tests/test_instruments.py b/tests/test_instruments.py index 345534a..b01d91a 100644 --- a/tests/test_instruments.py +++ b/tests/test_instruments.py @@ -24,8 +24,10 @@ Disk, GtdInbox, InboxSla, + LockHolder, PrState, RequiredContexts, + ScheduledJob, Served, VerdictFreshness, Worktrees, @@ -710,6 +712,167 @@ def test_bench_jobs_reports_an_unreported_job_whose_process_is_gone(tmp_path): assert not out.control +def test_lock_holder_resolves_each_holders_pid_to_its_full_command_line_beside_its_own_held_lock(tmp_path): + own, lock, free = tmp_path / "agent.pid", tmp_path / "build.lock", tmp_path / "free.lock" + me = os.getpid() + listed = f"p{me}\nf3\nn{os.path.realpath(own)}\np4242\nf5\nn{os.path.realpath(lock)}\n" + sh = FakeSh( + **{ + "-F pn": (1, listed, ""), # rc 1: a named file nobody holds open + "-o pid=,command= -p": ( + f"{me} /usr/bin/python3 -m lion agent --dir /a\n 4242 cargo build --release\n" + ), + } + ) + inst = LockHolder([str(lock), str(free)], str(own), every="every:15m", run=sh) + out = asyncio.run(inst.run({})) + assert out.control and out.findings == (f"{lock}: held by pid 4242: cargo build --release",) + assert out.positive == f"own lock {own} held by own pid {me}: /usr/bin/python3 -m lion agent --dir /a" + assert out.evidence.startswith(f"control: {out.positive}\n") and f"{free}: no holder" in out.evidence + assert (out.count, out.predicate) == (2, "open by a process (1 match)") + # read by the lock's file and the pid, never by matching command text and never by taking the lock + assert sh.calls == [ + [instr.LSOF, "-w", "-F", "pn", "--", str(own), str(lock), str(free)], + ["/bin/ps", "-ww", "-o", "pid=,command=", "-p", ",".join(map(str, sorted({me, 4242})))], + ] + sh.set("-o pid=,command= -p", f"{me} /usr/bin/python3 -m lion agent --dir /a\n") + out = asyncio.run(inst.run({})) # the holder exited between the two reads + assert out.control and out.findings == ( + f"{lock}: held by pid 4242: (no command line: the pid exited between the reads)", + ) + + +@pytest.mark.parametrize("broken", ["lsof", "ps"]) +def test_lock_holder_fails_its_control_when_its_own_held_lock_does_not_read_as_its_own(tmp_path, broken): + own, lock = tmp_path / "agent.pid", tmp_path / "build.lock" + me = os.getpid() + sh = FakeSh( + **{ + "-F pn": f"p{me}\nf3\nn{os.path.realpath(own)}\n", + "-o pid=,command= -p": f"{me} /usr/bin/python3 -m lion agent\n", + } + ) + # a dead lsof answers the way a lock nobody holds does: empty, rc 1 + sh.set("-F pn" if broken == "lsof" else "-o pid=,command= -p", (1, "", "no such program")) + out = asyncio.run(LockHolder([str(lock)], str(own), every=None, run=sh).run({})) + assert not out.control and out.findings == () and out.positive == "" + assert "read broken" in out.evidence.splitlines()[0] + + +@pytest.mark.skipif(instr.LSOF == "lsof", reason="no lsof on this machine") +def test_lock_holder_reads_a_lock_a_live_process_holds_through_the_real_lsof_and_ps(tmp_path): + import fcntl + import subprocess + import sys + + own, lock, free = tmp_path / "agent.pid", tmp_path / "held.lock", tmp_path / "free.lock" + fd = os.open(own, os.O_RDWR | os.O_CREAT) + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + hold = ( + "import fcntl, os, sys; fd = os.open(sys.argv[1], os.O_RDWR | os.O_CREAT); " + "fcntl.flock(fd, fcntl.LOCK_EX); print('locked', flush=True); sys.stdin.read()" + ) + holder = subprocess.Popen( + [sys.executable, "-c", hold, str(lock)], stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True + ) + try: + assert holder.stdout.readline().strip() == "locked" + out = asyncio.run(LockHolder([str(lock), str(free)], str(own), every=None).run({})) + finally: + holder.stdin.close() + holder.wait(timeout=5) + holder.stdout.close() + os.close(fd) + assert out.control, out.evidence + assert len(out.findings) == 1 and out.findings[0].startswith(f"{lock}: held by pid {holder.pid}: ") + # the evidence carries the full command line, its argument included; the finding caps it + assert any( + ln.startswith(f"{lock}: pid {holder.pid}: ") and ln.endswith(f" {lock}") + for ln in out.evidence.splitlines() + ) + assert f"{free}: no holder" in out.evidence + + +def test_scheduled_job_reads_the_artifact_first_then_the_supervisors_last_exit(tmp_path): + d = tmp_path / "out" + d.mkdir() + (d / "fresh.json").write_text("{}") + (d / "stale.json").write_text("{}") + old = time.time() - 30 * 3600 + os.utime(d / "stale.json", (old, old)) + sh = FakeSh( + **{ + "/com.example.fresh": "\tstate = not running\n\tlast exit code = 0\n", + "/com.example.stale": "\tstate = not running\n\tlast exit code = 0\n", + "/com.example.failed": "\tstate = not running\n\tlast exit code = 1: Operation not permitted\n", + "/com.example.new": "\tstate = waiting\n\tlast exit code = (never exited)\n", + "/com.example.gone": (113, "", 'Could not find service "com.example.gone" in domain'), + } + ) + jobs = [ + {"artifact": str(d / "fresh.json"), "max_age_h": 24, "label": "com.example.fresh"}, + {"artifact": str(d / "stale.json"), "max_age_h": 24, "label": "com.example.stale"}, + {"artifact": str(d / "none.json"), "max_age_h": 24, "label": "com.example.failed"}, + {"artifact": str(d / "fresh.json"), "max_age_h": 1, "label": "com.example.new"}, + {"artifact": str(d / "fresh.json"), "max_age_h": 24, "label": "com.example.gone"}, + {"artifact": str(d / "fresh.json"), "max_age_h": 24}, + ] + inst = ScheduledJob(jobs, every="every:1h", run=sh) + out = asyncio.run(inst.run({})) + assert out.control and out.findings == ( + f"{d}/stale.json: older than its 24h bound", # a clean exit beside a stale artifact is a finding + f"{d}/none.json: absent, bound 24h", + f"{d}/none.json: its supervisor com.example.failed last exited 1", + f"{d}/fresh.json: its supervisor com.example.gone is not loaded", + ) + lines = out.evidence.splitlines() + assert lines[1].startswith(f"{d}/stale.json: written 30.0h ago, bound 24h | launchd com.example.stale") + assert lines[3].endswith("| launchd com.example.new: last exit (never exited)") + assert (out.count, out.predicate.endswith("(3 match)")) == (6, True) + assert out.positive.startswith(f"{d} present; launchd com.example.fresh row read; {d} present") + assert [c[:2] for c in sh.calls] == [[instr.LAUNCHCTL, "print"]] * 5 + assert asyncio.run(inst.run({})).findings == out.findings # the age stays in the evidence: told once + + +@pytest.mark.parametrize("broken", ["directory", "row"]) +def test_scheduled_job_fails_its_control_on_an_absent_directory_or_a_row_without_its_exit(tmp_path, broken): + (tmp_path / "out").mkdir() + (tmp_path / "out" / "a.json").write_text("{}") + artifact = tmp_path / ("gone" if broken == "directory" else "out") / "a.json" + row = "\tstate = running\n" if broken == "row" else "\tlast exit code = 0\n" + inst = ScheduledJob( + [{"artifact": str(artifact), "max_age_h": 24, "label": "com.example.job"}], + every=None, + run=FakeSh(**{"/com.example.job": row}), + ) + out = asyncio.run(inst.run({})) + assert not out.control and out.findings == () and out.positive == "" + reason = "its directory is absent" if broken == "directory" else "no last exit code in its row (unread)" + assert reason in out.evidence + + +def test_instruments_builds_the_lock_holder_on_the_agents_own_pid_file_and_the_scheduled_jobs(tmp_path): + from types import SimpleNamespace + + cfg = { + "owner": "lambda:x", + "chores": { + "lock-holder": {"every": "every:15m", "locks": ["/tmp/a.lock"]}, + "scheduled-job": {"every": "every:1h", "jobs": [{"artifact": "/x/out.json", "max_age_h": 26}]}, + }, + } + built = instr.instruments(cfg, SimpleNamespace(cwd=tmp_path), MemoryNoteStore()) + assert sorted(built) == ["lock-holder", "scheduled-job"] + lock_holder = built["lock-holder"] + assert (lock_holder.locks, lock_holder.held) == (["/tmp/a.lock"], str(tmp_path / "agent.pid")) + assert built["scheduled-job"].jobs == [{"artifact": "/x/out.json", "max_age_h": 26}] + assert {i.every for i in built.values()} == {"every:15m", "every:1h"} + assert not any(i.empty_expected for i in built.values()) + cfg["chores"]["scheduled-job"]["jobs"] = [{"artifact": "/x/out.json"}] + with pytest.raises(ValueError, match="a scheduled job names its artifact and its max_age_h"): + instr.instruments(cfg, SimpleNamespace(cwd=tmp_path), MemoryNoteStore()) + + def test_build_reads_the_actors_config_bounds_the_client_and_gates_the_mail(tmp_path, kkernel_stub): (tmp_path / ".khive").mkdir() (tmp_path / ".khive" / "config.toml").write_text('[actor]\nid = "lambda:x:agent"\n') From 8792b7f7c97d8f1e2cdd2d81472150109d5019dd Mon Sep 17 00:00:00 2001 From: ohdearquant <122793010+ohdearquant@users.noreply.github.com> Date: Mon, 28 Sep 2026 14:42:37 -0400 Subject: [PATCH 2/4] instrument: a lock lsof cannot stat is reported unreadable, not unheld, and S11 says a notice marks its recipient told once the send lands --- .../014-instrument/ADR-0014-the-instrument.md | 13 ++++++------ hub/agent/instruments.py | 20 ++++++++++++++++--- tests/test_instruments.py | 20 +++++++++++++++++++ 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/docs/adr/014-instrument/ADR-0014-the-instrument.md b/docs/adr/014-instrument/ADR-0014-the-instrument.md index ddcf14a..d98425d 100644 --- a/docs/adr/014-instrument/ADR-0014-the-instrument.md +++ b/docs/adr/014-instrument/ADR-0014-the-instrument.md @@ -302,12 +302,13 @@ then `launchctl print` for a named label's last exit. control, or an empty answer the control must catch (A1). - **S9**: Two instrument shapes are built (D7). `lock-holder` reads a pid resolved to its full command line beside a lock known to be held, the agent's own pid file, never by matching text or - by taking the lock. `scheduled-job` reads the artifact's freshness first and the supervisor's exit - second, as `served` reads a last-wake note before a pid. `lsof` names the processes with the file - open, so a waiter reads as a holder; the only supervisor read is a launchd row. + by taking the lock. A waiter reads as a holder, a lock file `lsof` cannot stat reads unreadable, + and a lock with no file is held by nobody. `scheduled-job` reads the artifact's freshness first + and the supervisor's exit second, as `served` reads a last-wake note before a pid. - **S10**: `comm.probe` returns a page of at most 100 rows and caps its stale count at 1000, where 1000 means at least that many. `inbox-sla` prints the count as it came, so a capped count reads as exact; its population is the rows on the probe pages. -- **S11**: A dead-instrument notice that reached neither the owner nor the steward still marks the - chore told, so that crossing is never told until an answered run clears the mark. Marking the - chore only once a send lands is owed; no test sends the notice undelivered. +- **S11**: A dead-instrument notice marks its recipient told only once the send lands: a copy the + crossing refused is owed at the next run, and the chore's own mark waits for every copy. A notice + that reached nobody leaves the chore untold: + `test_a_dead_instrument_notice_that_reached_nobody_leaves_the_crossing_untold_until_a_send_lands`. diff --git a/hub/agent/instruments.py b/hub/agent/instruments.py index 0a16ea3..757a1e8 100644 --- a/hub/agent/instruments.py +++ b/hub/agent/instruments.py @@ -527,7 +527,10 @@ class LockHolder: about = "each named lock file: the pids holding it open, with their full command lines; reports holders" answers = "which processes hold each named lock file open right now, by pid and full command line" direction = "under-report" - mechanism = "lsof lists only the processes this user may inspect; a holder beyond them reads as no holder" + mechanism = ( + "lsof lists only the processes this user may inspect; a holder beyond them reads as no holder. A " + "lock file lsof cannot stat is reported unreadable, not unheld" + ) def __init__( self, @@ -551,8 +554,14 @@ async def run(self, args: dict[str, Any]) -> Outcome: pid = int(line[1:]) elif line[:1] == "n" and pid is not None and pid not in opened.setdefault(line[1:], []): opened[line[1:]].append(pid) - # lsof names a file by its resolved path (/tmp reads /private/tmp on macOS) + # lsof names a file by its resolved path (/tmp reads /private/tmp on macOS); a path it could not + # stat is named the same way on stderr, one `status error` line each holders = {p: opened.get(os.path.realpath(p), []) for p in paths} + unread = {} + for line in err.splitlines(): + head, _, why = line.rpartition(": ") + if head.startswith("lsof: status error on "): + unread[head.removeprefix("lsof: status error on ")] = why.strip() me = os.getpid() pids = sorted({me, *(p for ps in holders.values() for p in ps)}) _, out, _ = await self.sh(["/bin/ps", "-ww", "-o", "pid=,command=", "-p", ",".join(map(str, pids))]) @@ -574,8 +583,13 @@ async def run(self, args: dict[str, Any]) -> Outcome: ] findings, matched = [], 0 for lock in self.locks: + why = unread.get(os.path.realpath(lock)) + if why and not why.startswith("No such file"): # a lock lsof could not read is not unheld + lines.append(f"{lock}: unreadable: {why}") + findings.append(f"{lock}: unreadable by lsof: {why}") + continue if not holders[lock]: - lines.append(f"{lock}: no holder") + lines.append(f"{lock}: no holder" + (" (no such file)" if why else "")) continue matched += 1 for p in holders[lock]: diff --git a/tests/test_instruments.py b/tests/test_instruments.py index b01d91a..bc57e32 100644 --- a/tests/test_instruments.py +++ b/tests/test_instruments.py @@ -759,6 +759,26 @@ def test_lock_holder_fails_its_control_when_its_own_held_lock_does_not_read_as_i assert "read broken" in out.evidence.splitlines()[0] +def test_lock_holder_reports_a_lock_lsof_could_not_read_as_unreadable_not_unheld(tmp_path): + own, lock, gone = tmp_path / "agent.pid", tmp_path / "shut" / "build.lock", tmp_path / "free.lock" + me = os.getpid() + # lsof names each file it could not stat on stderr, by its resolved path, and still lists the rest + err = ( + f"lsof: status error on {os.path.realpath(lock)}: Permission denied\n" + f"lsof: status error on {os.path.realpath(gone)}: No such file or directory\n" + ) + sh = FakeSh( + **{ + "-F pn": (1, f"p{me}\nf3\nn{os.path.realpath(own)}\n", err), + "-o pid=,command= -p": f"{me} /usr/bin/python3 -m lion agent\n", + } + ) + out = asyncio.run(LockHolder([str(lock), str(gone)], str(own), every=None, run=sh).run({})) + assert out.control and out.findings == (f"{lock}: unreadable by lsof: Permission denied",) + assert f"{lock}: unreadable: Permission denied" in out.evidence + assert f"{gone}: no holder (no such file)" in out.evidence # a lock with no file is held by nobody + + @pytest.mark.skipif(instr.LSOF == "lsof", reason="no lsof on this machine") def test_lock_holder_reads_a_lock_a_live_process_holds_through_the_real_lsof_and_ps(tmp_path): import fcntl From 4059d746a36c9d8cf4043a97e298468a7a39c243 Mon Sep 17 00:00:00 2001 From: ohdearquant <122793010+ohdearquant@users.noreply.github.com> Date: Mon, 28 Sep 2026 17:28:27 -0400 Subject: [PATCH 3/4] instrument: a name lsof escapes fails the lock read, a scheduled job's bound is a number and its artifact a file, a supervisor read that did not finish is unread lsof prints an escaped form of a name holding a tab, a newline, a backslash or a byte outside the locale, on stdout and on its status error lines; the lock-holder looked those names up by the raw path and read a held or unreadable lock as no holder. A name no configured path resolves to now fails the control and is named. A scheduled job refuses a max_age_h that is a bool, not a number or NaN at build; its artifact is read with one stat, and anything but a regular file is a finding; a launchctl read that failed or did not finish is unread, not loaded is only what launchd says. ADR-0014 C4 now agrees with S11; ADR-0015 cites the hand-run check's test and lists lsof. --- .../014-instrument/ADR-0014-the-instrument.md | 30 ++++--- docs/adr/015-chores/ADR-0015-chores.md | 22 ++--- docs/adr/INDEX.md | 2 +- docs/adr/PLAN.md | 4 +- hub/agent/instruments.py | 43 +++++++-- tests/test_instruments.py | 88 +++++++++++++++++-- 6 files changed, 152 insertions(+), 37 deletions(-) diff --git a/docs/adr/014-instrument/ADR-0014-the-instrument.md b/docs/adr/014-instrument/ADR-0014-the-instrument.md index d98425d..a300462 100644 --- a/docs/adr/014-instrument/ADR-0014-the-instrument.md +++ b/docs/adr/014-instrument/ADR-0014-the-instrument.md @@ -111,10 +111,10 @@ expect empty by default, the rest do not, and `expect_empty` in the config overr again on every run after it, or an answered run leaves the mark. `Chores._dead` counts the chore's unanswered rows back from the newest. At `dead_after`, three by -default, it tells the owner and the steward once that the instrument is dead, and marks the chore -told whether or not either send landed (S11). An answered run clears the mark, so the next crossing -is told again. The desk writes no chore ledger row, so a measurement it received does not count -here. +default, it tells the owner and the steward once that the instrument is dead. Each recipient is +marked told once its send lands, and the chore once every copy has (S11). An answered run clears the +mark, so the next crossing is told again. The desk writes no chore ledger row, so a measurement it +received does not count here. ### C5: First sight is the whole row, then transitions, and a crossing is told once _(enforced: mechanical)_ ^c5 @@ -182,8 +182,9 @@ the two and a blank `answers`. `every` is a store repeat such as `every:30m` or `git` resolve once to an absolute path where one is found. - **Landing evidence**: - `test_instruments_wires_the_desk_chores_and_signs_as_the_owners_desk_by_default` and - `test_instruments_builds_the_lock_holder_on_the_agents_own_pid_file_and_the_scheduled_jobs` in + `test_instruments_wires_the_desk_chores_and_signs_as_the_owners_desk_by_default`, + `test_instruments_builds_the_lock_holder_on_the_agents_own_pid_file_and_the_scheduled_jobs` and + `test_scheduled_job_a_max_age_h_that_is_not_a_number_is_refused_at_build` in `tests/test_instruments.py`; `test_a_chore_without_an_error_direction_or_a_question_is_refused_at_build` in `tests/test_chores.py`. @@ -250,15 +251,19 @@ Nothing is sent, no ledger row is written, and `_vet` is not called; the text's Serves C1 and C7. `LockHolder.run` lists the named lock files and the agent's own pid file in one `lsof` call and resolves every pid to its full command line with one `ps` read; its control is this -process holding its own pid file. `ScheduledJob.run` reads each artifact's age against its bound, -then `launchctl print` for a named label's last exit. +process holding its own pid file. A name `lsof` prints that no configured path resolves to, one it +escaped, fails the control. `ScheduledJob.run` reads each artifact's age against its bound, then +`launchctl print` for a named label's last exit. A bound that is not a number is refused at build. - **Landing evidence**: in `tests/test_instruments.py`, `test_lock_holder_resolves_each_holders_pid_to_its_full_command_line_beside_its_own_held_lock`, `test_lock_holder_fails_its_control_when_its_own_held_lock_does_not_read_as_its_own`, + `test_lock_holder_a_name_lsof_escaped_fails_the_control_and_is_named_as_unmapped`, `test_lock_holder_reads_a_lock_a_live_process_holds_through_the_real_lsof_and_ps`, `test_scheduled_job_reads_the_artifact_first_then_the_supervisors_last_exit`, - `test_scheduled_job_fails_its_control_on_an_absent_directory_or_a_row_without_its_exit` (S9). + `test_scheduled_job_fails_its_control_on_an_absent_directory_or_a_row_without_its_exit`, + `test_scheduled_job_an_artifact_that_is_not_a_regular_file_is_a_finding_never_fresh`, + `test_scheduled_job_a_supervisor_read_that_did_not_finish_is_unread_not_unloaded` (S9). ## Alternatives @@ -302,9 +307,10 @@ then `launchctl print` for a named label's last exit. control, or an empty answer the control must catch (A1). - **S9**: Two instrument shapes are built (D7). `lock-holder` reads a pid resolved to its full command line beside a lock known to be held, the agent's own pid file, never by matching text or - by taking the lock. A waiter reads as a holder, a lock file `lsof` cannot stat reads unreadable, - and a lock with no file is held by nobody. `scheduled-job` reads the artifact's freshness first - and the supervisor's exit second, as `served` reads a last-wake note before a pid. + by taking the lock. A waiter reads as a holder, a lock file `lsof` cannot stat reads unreadable, a + name `lsof` escaped voids the read, and a lock with no file is held by nobody. `scheduled-job` + reads the artifact's freshness first and the supervisor's exit second, as `served` reads a + last-wake note before a pid. - **S10**: `comm.probe` returns a page of at most 100 rows and caps its stale count at 1000, where 1000 means at least that many. `inbox-sla` prints the count as it came, so a capped count reads as exact; its population is the rows on the probe pages. diff --git a/docs/adr/015-chores/ADR-0015-chores.md b/docs/adr/015-chores/ADR-0015-chores.md index 597cd3c..d065d8f 100644 --- a/docs/adr/015-chores/ADR-0015-chores.md +++ b/docs/adr/015-chores/ADR-0015-chores.md @@ -1,7 +1,7 @@ --- adr: ADR-0015 status: draft -liveness: operating (the handler, the gate, the store boundary, the ticks and the catch-up are pinned by tests with scripted backends and a fake store; no test drives a positive hand-run check; the catch-up's store-side filters are owed) +liveness: operating (the handler, the gate, the store boundary, the ticks and the catch-up are pinned by tests with scripted backends and a fake store; a positive hand-run check is under test; the catch-up's store-side filters are owed) date: "2026-09-25" area: chores kind: new @@ -154,10 +154,10 @@ answers ([[ADR-0017-the-desk#^c12|ADR-0017/C12]]). ### C6: A chore changes no tree, so its result lands by print and save _(enforced: process)_ ^c6 The instruments' commands read: `gh` lists, views and GETs, `git` worktree lists, status, log and -remote lookups, `ps`, `launchctl print`, `df`, `find` and `du`. The chores profile offers `check`, -`digest`, `aged` and the handlers a front desk adds; none is a file tool or a note command, so a -blank `note.find` ([[ADR-0007-the-record#^c5|ADR-0007/C5]]) never reaches a chore. The handler reads -and writes the agent's notes by key. A chore has no diff to land. +remote lookups, `ps`, `lsof`, `launchctl print`, `df`, `find` and `du`. The chores profile offers +`check`, `digest`, `aged` and the handlers a front desk adds; none is a file tool or a note command, +so a blank `note.find` ([[ADR-0007-the-record#^c5|ADR-0007/C5]]) never reaches a chore. The handler +reads and writes the agent's notes by key. A chore has no diff to land. Its hand-run form is print and save: `lion agent --check ` runs the instrument alone, prints it and saves the same text in the agent directory's `landing` folder, one file per chore and day; it @@ -237,8 +237,9 @@ Serves C6. `agent_main` claims the identity, runs the named instrument with no a `render()` of it and writes the same text with `land()`, overwriting the day's file of that name. It bypasses the handler: no refusals, no row, no send. -- **Landing evidence**: `test_once_refuses_served_identity` in `tests/test_agent.py` pins its - refusal while a live process holds the identity; no test drives a positive run. +- **Landing evidence**: in `tests/test_agent.py`, `test_once_refuses_served_identity` pins its + refusal while a live process holds the identity, and + `test_check_runs_one_instrument_prints_and_lands_it_and_sends_nothing` drives a positive run. ## Alternatives @@ -268,9 +269,10 @@ bypasses the handler: no refusals, no row, no send. - **S4**: A standing state is told once until it changes, a standing failure included: a chore that recovers quietly and fails again the same way is not resent. The digest carries the counts, and an owner who wants a state again asks. -- **S5**: The hand-run check skips the handler's refusals, so it shows the raw measurement, and no - test drives it past the identity claim. The `verdict-freshness` enumerator runs as the config - gives it, outside anything that checks it only reads. +- **S5**: The hand-run check skips the handler's refusals, so it shows the raw measurement; + `test_check_runs_one_instrument_prints_and_lands_it_and_sends_nothing` drives it past the identity + claim. The `verdict-freshness` enumerator runs as the config gives it, outside anything that + checks it only reads. - **S6**: The chore ledger is read whole on every check and never rotated, so a check's cost grows with the agent's age. Its `at` stamps are local time without a zone; a row also carries `ts`, the instant in epoch seconds, which the aged pass reads, and a row without one is read by its `at`. diff --git a/docs/adr/INDEX.md b/docs/adr/INDEX.md index 61c6700..3419929 100644 --- a/docs/adr/INDEX.md +++ b/docs/adr/INDEX.md @@ -16,7 +16,7 @@ | [ADR-0012](012-bench/ADR-0012-the-bench.md) | 012-bench | draft | operating (the set runner, the in-box grader, the closure, the run identity and both arms are under tests; that compared bench runs share instances, head and budgets is the reader's check; no CLI-arm bench run on the hard set has the package indexes closed) | ADR-0002, ADR-0003, ADR-0006, ADR-0009 | | [ADR-0013](013-agent/ADR-0013-the-agent.md) | 013-agent | draft | operating (the wake, the gate, the caps, the hop count, the cursor, the lock and the continuous run with its checkpoint are pinned by tests; supervision and the lease are not in the process) | ADR-0001, ADR-0002, ADR-0003, ADR-0007, ADR-0010 | | [ADR-0014](014-instrument/ADR-0014-the-instrument.md) | 014-instrument | draft | operating (thirteen instruments build from one config and run under tests against scripted commands and stores, a lock holder and a scheduled job among them; the command-line check prints its measurement unvetted, says so, and is under test) | ADR-0005, ADR-0007 | -| [ADR-0015](015-chores/ADR-0015-chores.md) | 015-chores | draft | operating (the handler, the gate, the store boundary, the ticks and the catch-up are pinned by tests with scripted backends and a fake store; no test drives a positive hand-run check; the catch-up's store-side filters are owed) | ADR-0001, ADR-0005, ADR-0007, ADR-0010 | +| [ADR-0015](015-chores/ADR-0015-chores.md) | 015-chores | draft | operating (the handler, the gate, the store boundary, the ticks and the catch-up are pinned by tests with scripted backends and a fake store; a positive hand-run check is under test; the catch-up's store-side filters are owed) | ADR-0001, ADR-0005, ADR-0007, ADR-0010 | | [ADR-0016](016-command/ADR-0016-the-lion-command.md) | 016-command | draft | partial (the command tree, the shared lock and the spend row are under tests; the text `--stats` prints has no test; a mark for an unknown or partial cost is owed) | ADR-0007, ADR-0009, ADR-0010 | | [ADR-0017](017-desk/ADR-0017-the-desk.md) | 017-desk | draft | operating (every claim is pinned by tests with scripted backends and a stubbed store; no bench exercises the desk; the gate for a clarify's answer mailed to the desk is owed, S8) | ADR-0001, ADR-0002, ADR-0003, ADR-0010 | | [ADR-0018](018-chat/ADR-0018-chat-in-a-box.md) | 018-chat | draft | operating (the chat, its resume and the boxed chat are pinned by tests against scripted backends and a stubbed box; the one real-box test runs only when enabled; no test interrupts a boxed chat) | ADR-0002, ADR-0003, ADR-0005, ADR-0007 | diff --git a/docs/adr/PLAN.md b/docs/adr/PLAN.md index 4493a4c..4092685 100644 --- a/docs/adr/PLAN.md +++ b/docs/adr/PLAN.md @@ -36,8 +36,8 @@ sandboxed executor are the published side of that boundary; the rest is named by | ADR-0011 | The box: one shape for three boxes, the coding tools over a tree, the watch and its gate, the patch | draft (2026-09-25); the real-box tests opt-in; resolving a boxed path in the box owed | the sandboxed executor: content-addressed trees, declared write paths, receipts | | ADR-0013 | The long-running agent: the wake, the send gate and the hop count, the caps, the cursor and posture, the lock, the continuous run and its checkpoint | draft (2026-09-25) | the process: one identity per process, lease by turn activity, one supervisor | | ADR-0012 | The bench as the instrument: the grade in the box, the row, the run id, unaided by default, what compares with what, the cited figures | draft (2026-09-25); one carried figure not reproduced | the instrument contract | -| ADR-0014 | The instrument contract: the measurement and its proof, the refusals, the dead instrument, first sight then transitions, the floors | draft (2026-09-25); the command-line check unvetted and saying so; a dead mark on delivery owed | none | -| ADR-0015 | Chores: three endings chosen by code, thirteen chores from one file, ticks as store reminders, the bounded client, print and save | draft (2026-09-25); no test drives a positive hand-run check; the catch-up's store-side filters owed | the scheduled run of a process | +| ADR-0014 | The instrument contract: the measurement and its proof, the refusals, the dead instrument, first sight then transitions, the floors | draft (2026-09-25); the command-line check unvetted and saying so | none | +| ADR-0015 | Chores: three endings chosen by code, thirteen chores from one file, ticks as store reminders, the bounded client, print and save | draft (2026-09-25); the catch-up's store-side filters owed | the scheduled run of a process | | ADR-0016 | The lion command and the spend row: one entry, the shared lock, the row from the envelopes, the readers | draft (2026-09-25); the unknown-cost mark owed | none | | ADR-0017 | The desk: the read-only reader, the cursor that never marks, the gate, code escalations, every model-facing command with its gate, pending until its row, keyed replay, record answers, the brief, areas and chairs | draft (2026-09-25); no bench exercises the desk | mail settlement by keyed replay | | ADR-0018 | Chat in a box: one run that waits for the person, the flags, the person's tree or a copy, the patch home, the resume | draft (2026-09-25); no test interrupts a boxed chat | none | diff --git a/hub/agent/instruments.py b/hub/agent/instruments.py index 757a1e8..c94d222 100644 --- a/hub/agent/instruments.py +++ b/hub/agent/instruments.py @@ -14,6 +14,7 @@ import contextlib import hashlib import json +import math import os import re import shutil @@ -21,6 +22,7 @@ from collections.abc import Awaitable, Callable from datetime import UTC, datetime from pathlib import Path +from stat import S_ISREG from typing import Any from urllib.parse import quote @@ -562,6 +564,9 @@ async def run(self, args: dict[str, Any]) -> Outcome: head, _, why = line.rpartition(": ") if head.startswith("lsof: status error on "): unread[head.removeprefix("lsof: status error on ")] = why.strip() + # lsof escapes some names (a tab reads `\t`, a newline `\n`, a byte outside the locale `\xNN`); a + # name no configured path resolves to cannot be mapped back, so the read is void, never unheld + unmapped = sorted({*opened, *unread} - {os.path.realpath(p) for p in paths}) me = os.getpid() pids = sorted({me, *(p for ps in holders.values() for p in ps)}) _, out, _ = await self.sh(["/bin/ps", "-ww", "-o", "pid=,command=", "-p", ",".join(map(str, pids))]) @@ -571,16 +576,23 @@ async def run(self, args: dict[str, Any]) -> Outcome: if head.isdigit(): commands[int(head)] = command.strip() own = commands.get(me, "") - control = me in holders[self.held] and bool(own) + held = me in holders[self.held] and bool(own) + control = held and not unmapped positive = f"own lock {self.held} held by own pid {me}: {own[:120]}" if control else "" lines = [ "control: " + ( positive - or f"own lock {self.held} open by {holders[self.held] or 'no pid'}, own pid {me}, its " - f"command line {'read' if own else 'unread'} (NOT held by this process: read broken)" + or ( + f"lsof named {len(unmapped)} file(s) no configured path resolves to, so a lock " + "among them cannot be read (read broken)" + if held + else f"own lock {self.held} open by {holders[self.held] or 'no pid'}, own pid {me}, " + f"its command line {'read' if own else 'unread'} (NOT held by this process: read broken)" + ) ) ] + lines += [f"unmapped: {name}" for name in unmapped] findings, matched = [], 0 for lock in self.locks: why = unread.get(os.path.realpath(lock)) @@ -635,34 +647,51 @@ def __init__( for j in self.jobs: if "artifact" not in j or "max_age_h" not in j: raise ValueError(f"a scheduled job names its artifact and its max_age_h: {j}") + b = j["max_age_h"] + # a NaN bound compares false against every age, so it would read every artifact fresh + if isinstance(b, bool) or not isinstance(b, int | float) or math.isnan(b): + raise ValueError(f"a scheduled job's max_age_h is a number of hours: {j}") + j["max_age_h"] = float(b) async def run(self, args: dict[str, Any]) -> Outcome: findings, lines, positives, control, matched = [], [], [], True, 0 for j in self.jobs: art, bound = Path(j["artifact"]), float(j["max_age_h"]) line, bad = f"{art}:", False + try: # one stat for the type and the age, so the two cannot disagree + st = art.stat() + except (FileNotFoundError, NotADirectoryError): + st = None if not art.parent.is_dir(): control = False line += " its directory is absent (a wrong path reads like a job that never ran)" - elif not art.exists(): + elif st is None: positives.append(f"{art.parent} present") bad = True line += " absent" findings.append(f"{art}: absent, bound {bound:g}h") + elif not S_ISREG(st.st_mode): # a directory where the job writes its file is not fresh + positives.append(f"{art.parent} present") + bad = True + line += " not a regular file" + findings.append(f"{art}: not a regular file, bound {bound:g}h") else: positives.append(f"{art.parent} present") - age_h = (time.time() - art.stat().st_mtime) / 3600 + age_h = (time.time() - st.st_mtime) / 3600 line += f" written {age_h:.1f}h ago, bound {bound:g}h" if age_h > bound: bad = True findings.append(f"{art}: older than its {bound:g}h bound") if j.get("label"): - rc, out, _ = await self.sh([LAUNCHCTL, "print", f"gui/{os.getuid()}/{j['label']}"]) + rc, out, err = await self.sh([LAUNCHCTL, "print", f"gui/{os.getuid()}/{j['label']}"]) m = re.search(r"^\s*last exit code = (\(never exited\)|-?\d+)", out, re.M) - if rc != 0: + if rc != 0 and "Could not find service" in out + err: bad = True line += f" | launchd {j['label']}: not loaded" findings.append(f"{art}: its supervisor {j['label']} is not loaded") + elif rc != 0: # a read that failed or did not finish says nothing about the row + control = False + line += f" | launchd {j['label']}: unread, rc {rc}: {err.strip()[:200]}" elif m is None: control = False line += f" | launchd {j['label']}: loaded, no last exit code in its row (unread)" diff --git a/tests/test_instruments.py b/tests/test_instruments.py index bc57e32..685fb53 100644 --- a/tests/test_instruments.py +++ b/tests/test_instruments.py @@ -779,6 +779,25 @@ def test_lock_holder_reports_a_lock_lsof_could_not_read_as_unreadable_not_unheld assert f"{gone}: no holder (no such file)" in out.evidence # a lock with no file is held by nobody +@pytest.mark.parametrize("stream", ["held", "unreadable"]) +@pytest.mark.parametrize("name", ["build\nline.lock", "build\ttab.lock", "build\\back.lock"]) +def test_lock_holder_a_name_lsof_escaped_fails_the_control_and_is_named_as_unmapped(tmp_path, stream, name): + own, lock = tmp_path / "agent.pid", tmp_path / name + me = os.getpid() + # lsof prints a name it escapes as the escape (`\n`, `\t`, `\\`), on stdout and on stderr alike + shown = os.path.realpath(lock).replace("\\", "\\\\").replace("\n", "\\n").replace("\t", "\\t") + listed = f"p{me}\nf3\nn{os.path.realpath(own)}\n" + answer = ( + (0, listed + f"p4242\nf5\nn{shown}\n", "") + if stream == "held" + else (1, listed, f"lsof: status error on {shown}: Permission denied\n") + ) + sh = FakeSh(**{"-F pn": answer, "-o pid=,command= -p": f"{me} python -m lion agent\n 4242 cargo build\n"}) + out = asyncio.run(LockHolder([str(lock)], str(own), every=None, run=sh).run({})) + assert not out.control and out.positive == "" + assert "read broken" in out.evidence.splitlines()[0] and f"unmapped: {shown}" in out.evidence + + @pytest.mark.skipif(instr.LSOF == "lsof", reason="no lsof on this machine") def test_lock_holder_reads_a_lock_a_live_process_holds_through_the_real_lsof_and_ps(tmp_path): import fcntl @@ -786,18 +805,24 @@ def test_lock_holder_reads_a_lock_a_live_process_holds_through_the_real_lsof_and import sys own, lock, free = tmp_path / "agent.pid", tmp_path / "held.lock", tmp_path / "free.lock" + tabbed = tmp_path / "held\ttab.lock" fd = os.open(own, os.O_RDWR | os.O_CREAT) fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) hold = ( - "import fcntl, os, sys; fd = os.open(sys.argv[1], os.O_RDWR | os.O_CREAT); " - "fcntl.flock(fd, fcntl.LOCK_EX); print('locked', flush=True); sys.stdin.read()" + "import fcntl, os, sys; fds = [os.open(p, os.O_RDWR | os.O_CREAT) for p in sys.argv[1:]]; " + "fcntl.flock(fds[0], fcntl.LOCK_EX); print('locked', flush=True); sys.stdin.read()" ) holder = subprocess.Popen( - [sys.executable, "-c", hold, str(lock)], stdin=subprocess.PIPE, stdout=subprocess.PIPE, text=True + [sys.executable, "-c", hold, str(lock), str(tabbed)], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + text=True, ) try: assert holder.stdout.readline().strip() == "locked" out = asyncio.run(LockHolder([str(lock), str(free)], str(own), every=None).run({})) + # lsof prints the tab as `\t`: a held lock under a name it escaped voids the read, never unheld + escaped = asyncio.run(LockHolder([str(tabbed)], str(own), every=None).run({})) finally: holder.stdin.close() holder.wait(timeout=5) @@ -807,10 +832,11 @@ def test_lock_holder_reads_a_lock_a_live_process_holds_through_the_real_lsof_and assert len(out.findings) == 1 and out.findings[0].startswith(f"{lock}: held by pid {holder.pid}: ") # the evidence carries the full command line, its argument included; the finding caps it assert any( - ln.startswith(f"{lock}: pid {holder.pid}: ") and ln.endswith(f" {lock}") - for ln in out.evidence.splitlines() + ln.startswith(f"{lock}: pid {holder.pid}: ") and f" {lock} " in ln for ln in out.evidence.splitlines() ) assert f"{free}: no holder" in out.evidence + assert not escaped.control and escaped.positive == "", escaped.evidence + assert f"unmapped: {os.path.realpath(tabbed)}".replace("\t", "\\t") in escaped.evidence def test_scheduled_job_reads_the_artifact_first_then_the_supervisors_last_exit(tmp_path): @@ -871,6 +897,58 @@ def test_scheduled_job_fails_its_control_on_an_absent_directory_or_a_row_without assert reason in out.evidence +@pytest.mark.parametrize("kind", ["dir", "symlink-to-dir", "file", "symlink-to-file"]) +def test_scheduled_job_an_artifact_that_is_not_a_regular_file_is_a_finding_never_fresh(tmp_path, kind): + (tmp_path / "out").mkdir() + artifact, real = tmp_path / "out" / "result.json", tmp_path / "real" + made = artifact if kind in ("dir", "file") else real + if kind.endswith("dir"): + made.mkdir() + else: + made.write_text("{}") + if made is real: + artifact.symlink_to(real) + inst = ScheduledJob([{"artifact": str(artifact), "max_age_h": 24}], every=None, run=FakeSh()) + out = asyncio.run(inst.run({})) + assert out.control + if kind.endswith("dir"): + assert out.findings == (f"{artifact}: not a regular file, bound 24h",) + assert out.evidence == f"{artifact}: not a regular file" + else: + assert out.findings == () and out.evidence == f"{artifact}: written 0.0h ago, bound 24h" + + +@pytest.mark.parametrize("stale", [False, True], ids=["fresh", "stale"]) +def test_scheduled_job_a_supervisor_read_that_did_not_finish_is_unread_not_unloaded(tmp_path, stale): + (tmp_path / "out").mkdir() + artifact = tmp_path / "out" / "a.json" + artifact.write_text("{}") + if stale: + old = time.time() - 30 * 3600 + os.utime(artifact, (old, old)) + sh = FakeSh(**{"/com.example.job": (124, "", "/bin/launchctl did not finish within 60s")}) + job = {"artifact": str(artifact), "max_age_h": 24, "label": "com.example.job"} + out = asyncio.run(ScheduledJob([job], every=None, run=sh).run({})) + assert not out.control and out.positive == "" + assert out.findings == ((f"{artifact}: older than its 24h bound",) if stale else ()) + assert "launchd com.example.job: unread, rc 124: /bin/launchctl did not finish within 60s" in out.evidence + assert "not loaded" not in out.evidence + sh.set("/com.example.job", (113, "", 'Could not find service "com.example.job" in domain')) + out = asyncio.run(ScheduledJob([job], every=None, run=sh).run({})) + assert out.control and out.findings[-1] == f"{artifact}: its supervisor com.example.job is not loaded" + + +@pytest.mark.parametrize("bound", ["nan", '"soon"', "true"]) +def test_scheduled_job_a_max_age_h_that_is_not_a_number_is_refused_at_build(tmp_path, bound): + import tomllib + from types import SimpleNamespace + + job = f'{{artifact = "/x/out.json", max_age_h = {bound}}}' + cfg = tomllib.loads(f'owner = "lambda:x"\n[chores.scheduled-job]\njobs = [{job}]\n') + with pytest.raises(ValueError, match="max_age_h is a number of hours"): + instr.instruments(cfg, SimpleNamespace(cwd=tmp_path), MemoryNoteStore()) + + def test_instruments_builds_the_lock_holder_on_the_agents_own_pid_file_and_the_scheduled_jobs(tmp_path): from types import SimpleNamespace From 098beef6efa7e1d4b4f65994e2d00f8d51352410 Mon Sep 17 00:00:00 2001 From: ohdearquant <122793010+ohdearquant@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:26:33 -0400 Subject: [PATCH 4/4] instrument: a scheduled job's max_age_h is a positive finite number An infinite bound read every artifact fresh, the same effect a NaN bound was refused for; zero or a negative bound read every artifact stale. All three are refused at build now, beside NaN. --- docs/adr/014-instrument/ADR-0014-the-instrument.md | 5 +++-- hub/agent/instruments.py | 7 ++++--- tests/test_instruments.py | 6 +++--- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/docs/adr/014-instrument/ADR-0014-the-instrument.md b/docs/adr/014-instrument/ADR-0014-the-instrument.md index a300462..4496bc6 100644 --- a/docs/adr/014-instrument/ADR-0014-the-instrument.md +++ b/docs/adr/014-instrument/ADR-0014-the-instrument.md @@ -184,7 +184,7 @@ the two and a blank `answers`. `every` is a store repeat such as `every:30m` or - **Landing evidence**: `test_instruments_wires_the_desk_chores_and_signs_as_the_owners_desk_by_default`, `test_instruments_builds_the_lock_holder_on_the_agents_own_pid_file_and_the_scheduled_jobs` and - `test_scheduled_job_a_max_age_h_that_is_not_a_number_is_refused_at_build` in + `test_scheduled_job_a_max_age_h_that_is_not_a_positive_number_is_refused_at_build` in `tests/test_instruments.py`; `test_a_chore_without_an_error_direction_or_a_question_is_refused_at_build` in `tests/test_chores.py`. @@ -253,7 +253,8 @@ Serves C1 and C7. `LockHolder.run` lists the named lock files and the agent's ow `lsof` call and resolves every pid to its full command line with one `ps` read; its control is this process holding its own pid file. A name `lsof` prints that no configured path resolves to, one it escaped, fails the control. `ScheduledJob.run` reads each artifact's age against its bound, then -`launchctl print` for a named label's last exit. A bound that is not a number is refused at build. +`launchctl print` for a named label's last exit. A bound that is not a positive finite number is +refused at build. - **Landing evidence**: in `tests/test_instruments.py`, `test_lock_holder_resolves_each_holders_pid_to_its_full_command_line_beside_its_own_held_lock`, diff --git a/hub/agent/instruments.py b/hub/agent/instruments.py index c94d222..1fb5dde 100644 --- a/hub/agent/instruments.py +++ b/hub/agent/instruments.py @@ -648,9 +648,10 @@ def __init__( if "artifact" not in j or "max_age_h" not in j: raise ValueError(f"a scheduled job names its artifact and its max_age_h: {j}") b = j["max_age_h"] - # a NaN bound compares false against every age, so it would read every artifact fresh - if isinstance(b, bool) or not isinstance(b, int | float) or math.isnan(b): - raise ValueError(f"a scheduled job's max_age_h is a number of hours: {j}") + # a NaN bound compares false against every age and an infinite one above every age, so + # either would read every artifact fresh; a bound of zero or less reads every one stale + if isinstance(b, bool) or not isinstance(b, int | float) or not math.isfinite(b) or b <= 0: + raise ValueError(f"a scheduled job's max_age_h is a positive number of hours: {j}") j["max_age_h"] = float(b) async def run(self, args: dict[str, Any]) -> Outcome: diff --git a/tests/test_instruments.py b/tests/test_instruments.py index 685fb53..0b9718f 100644 --- a/tests/test_instruments.py +++ b/tests/test_instruments.py @@ -938,14 +938,14 @@ def test_scheduled_job_a_supervisor_read_that_did_not_finish_is_unread_not_unloa assert out.control and out.findings[-1] == f"{artifact}: its supervisor com.example.job is not loaded" -@pytest.mark.parametrize("bound", ["nan", '"soon"', "true"]) -def test_scheduled_job_a_max_age_h_that_is_not_a_number_is_refused_at_build(tmp_path, bound): +@pytest.mark.parametrize("bound", ["nan", '"soon"', "true", "inf", "-1.0", "0"]) +def test_scheduled_job_a_max_age_h_that_is_not_a_positive_number_is_refused_at_build(tmp_path, bound): import tomllib from types import SimpleNamespace job = f'{{artifact = "/x/out.json", max_age_h = {bound}}}' cfg = tomllib.loads(f'owner = "lambda:x"\n[chores.scheduled-job]\njobs = [{job}]\n') - with pytest.raises(ValueError, match="max_age_h is a number of hours"): + with pytest.raises(ValueError, match="max_age_h is a positive number of hours"): instr.instruments(cfg, SimpleNamespace(cwd=tmp_path), MemoryNoteStore())