Enforce the background-wait discipline mechanically, and fix what that exposed - #1637
Open
michael-wojcik wants to merge 109 commits into
Open
Enforce the background-wait discipline mechanically, and fix what that exposed#1637michael-wojcik wants to merge 109 commits into
michael-wojcik wants to merge 109 commits into
Conversation
A teammate that backgrounds work and ends its turn without flagging the wait is never woken, because teammate wake is wake-on-read. Three layers now catch it: record the launch, advise the teammate after three consecutive unflagged idles, and surface it to the lead once the wait goes stale. Every layer folds into a hook already registered on the event it needs, so hooks.json is unchanged: zero new registrations and zero new subprocesses in consumer sessions. track_files gains 18 lines and one guarded call; all logic lives in shared/background_work.py. Identity is resolved by validating the frame's agent_type against the team config's members[] names -- a lookup in state the platform wrote at spawn, not a type-strip. The raw value is matched and never stripped first: a type stripping down to a member name is ordinary usage, since member names are conventionally the stripped forms. A deny set derived from agents/*.md at runtime backs it up. Residual, stated rather than claimed away: an unknown future platform type colliding with a member name fails toward mis-bind rather than silence. A record is retired when a covering wait is observed, gated on since >= registered_at. The signal the advisory needs is acknowledgment, not completion -- the flag is the teammate saying it knows. Residual: discharge requires an idle between flagging and clearing. The two read paths carry different gates because the predicates differ in lifetime. Task-status is expiry, a property of the record, so it filters the read and every consumer inherits it; the raw read is private and the discharge path must name itself to get it. Flagged-wait is suppression, a property of the moment, so it filters the surface -- in the read it would hide the records the discharge exists to retire. resolve_agent_name's team-config lookup is structurally dead: it matches member["id"] while every config key is agentId. Recorded, not changed; a rename adds no route. background_work imports session_registry as a labeling surface, not an authority one. Forging an entry mis-attributes an advisory and crosses no privilege boundary, since writing that registry already requires the local access that could edit these hooks directly.
The detection layers are a recovery net: they fire after an agent has already gone idle repeatedly. The reported failure happens earlier, when an agent runs a background command and ends its turn -- so this warns there, on the PreToolUse Bash surface. The advisory reads run_in_background and nothing else. No identity, no team config, no task store, so it works on frames where identity cannot resolve at all and it reaches agents holding no task, which the detection layers structurally cannot. Folded into wait_filler_gate rather than registered separately. That PreToolUse Bash block already runs three hooks, so a fourth command entry would have cost a fourth subprocess on every Bash call in every consumer session -- the permanent tax this design exists to avoid. It rides the allow branch only, and the deny output is byte-identical whether or not the advisory condition holds: two concerns sharing a process, never a term in the verdict. Bound worth knowing before this is priced as the fix: whether PreToolUse additionalContext reliably reaches the model is an open question in this repo, documented against task_claim_gate, and this shares that channel. A prompt-time nudge, not a mechanical guarantee.
The teammate row asserted agent_type carries the specialist value, with no mode split. Measured against a live in-process Agent-Teams teammate PostToolUse Bash frame: it carries the teammate's own NAME, not the pact-prefixed type the team config records. Four consequences, spelled out because other hooks key on this file. Role classification is unaffected -- neither a name nor a type appears in LEAD_AGENT_TYPES, so a teammate classifies correctly either way. Any code treating the field as a type is on a false premise, including resolve_agent_name's type-strip, which is right in-process only by coincidence of the value. An argument excluding a type-strip by pointing at distinct names across same-agentType members is invalid, because the strip operates on the frame's field, which already differs per member. A caller needing identity must validate against members[]. The tmux row is marked untested for the Bash PostToolUse path: it rests on a single PreToolUse capture, and the machine this was measured on had no tmux team. That bounds the verification, not the behaviour -- this plugin ships to consumers who may run them.
Reported instances split two ways: agents that never opened the file holding the rule, and agents that opened it and failed to connect it at the decision point. The pointer goes in pact-teachback, which teammates demonstrably load, directly under the paragraph permitting concurrent unrelated tasks -- the case where a wait is most easily forgotten. The clause naming that no tool will wake you is deliberate rather than explanatory: one reported instance was an agent that knew the rule and framed the moment as one the tool would handle.
Patch tier: additive enforcement behind existing hook registrations, no new command or agent, no user-facing surface added or removed.
The captured-frame fixture recorded tool_response and effort as the string "<synthetic>" where both are dicts -- shape lost to a placeholder in the one file whose purpose is to carry a frame shape correctly. Both now hold their real key sets with synthetic values. tool_response's key set VARIES between frames: backgroundTaskId on a run_in_background launch, bashEditDiff instead on a foreground one, and timedOutAfterMs and returnCodeInterpretation on some and not others. The entry says so, because a fixture showing one fixed key set invites a reader to treat it as a schema. Readers must use .get() with a default. The entry's provenance is split and labelled: the top-level key set is one agent's capture, while tool_response's shape comes from another agent's instrument -- the first recorded only a presence boolean for that field, which could not answer the question the field later decided. effort's dict shape is inherited from an older capture of a different role on a different harness version and is marked the least certain claim in the entry rather than quietly asserted. agent_type is polymorphic by role, not randomly unreliable: consistently the member name on teammate frames, consistently the type spelling on lead frames. Measured on three teammates by two instruments and two operators. The earlier wording invited the reading that it varies per frame, which would misstate when the membership match can be trusted -- it is deterministic per role, which is also why the bind correctly refuses the lead. The agent_name branch is inert: the key is absent from all fifteen on teammate frames, and the stdin SSOT records it absent under tmux too, so it may be dead on every topology -- nobody has established that. It costs one dict lookup on a fail-open ordering, so it stays as a cheap option against a future harness, with a note not to delete it as unreachable without measuring the topology being deleted for. Docstrings only: stripping every docstring from both versions of background_work.py yields identical syntax trees.
An adversarial pass drove the production entry points rather than the helpers beneath them, on the rule that testing a gate does not test that the caller uses it. Two production paths had no arms at all. Fixed, both found by the new arms: - The durable-command filter missed the FLAG spelling. `-` sat in the lookbehind exclusion class, so `--watch` never matched and `vite --watch` was recorded as a wait — a process meant to run forever, drawing an advisory and a lead-side surface. Dropping `-` from the lookbehind closes it. Newly filtered: `--watch`, `vite --watch`, `-w --serve`, `foo --dev bar`. Newly recorded: nothing; the change is one-directional, because the lookahead still blocks `test-start-helper` and only the lookbehind moved. - A non-numeric `count` in the idle counter raised ValueError from inside the atomic update. An unreadable count now restarts the threshold rather than crashing the tick: the advisory fires later than it might have, which is the safe direction for an alarm. The filter's remaining over-firing is documented, not fixed, and recorded as an open finding: three of its four tokens are ordinary vocabulary, so `pytest -k start` and `grep -rn watch hooks/` are silently not recorded. The one-character change that closes it opens a new false negative, so the predicate needs redesigning rather than patching. One concurrency arm was deleted rather than shipped. It asserted that two unlocked processes lose appends; measured 1, 9 and 15 of 80 on an idle machine and all 50 surviving under load. A clock-barrier defect was real and fixing it with a true rendezvous was still not enough — the race is probabilistic. Its procedure and all three measurements live in the docstring. Safe because with the lock no-loss is guaranteed rather than likely, so the surviving arm cannot redden spuriously. Gate: 16053 passed, 85 skipped, 716.25s, unpiped exit 0, errors token 0, collection parity 16138 at both scopes.
The mechanism observes launches from a PostToolUse Bash frame, so the only background work it can hold is a shell command. The platform tracks ten types; a monitor, an Agent-tool subagent, an MCP task, a workflow and a scheduled wakeup raise no Bash event and are recorded by nothing. The docstrings claimed the rule generally. Enumerated from the tree rather than from memory: sixteen mentions across five directories, ten correctly out of scope. Four narrowed, three left intact. The remembered list held six and missed the journal event-schema comment — in a file edited hours earlier the same evening, which is the argument for enumerating rather than recalling. The advisories are NOT narrowed. They fire because a shell launch was recorded, so "you have outstanding background work" is true of the agent reading one; qualifying an instruction at the point of decision trades clarity for an accuracy the reader cannot act on. The lead-facing surface needed a different fix: its defect is not falsehood but a completeness implication, a reader taking a short list as an all-clear. It now states its own coverage and names what will not appear. Three watcher-discipline gaps, all measured this session: - A watcher that terminates on a proxy rather than the awaited event discharges early and leaves the wait unattended. Observed: one keyed on a file's mtime, discharged by an unrelated status write while the gate ran on. - A watcher that fires is spent. Observed: a 23.6-minute silence with nothing armed, after an earlier watcher had fired correctly. - Retiring a watcher deliberately is a third route to unarmed and the only one chosen. Observed: a correct decision to kill a redundant gate, followed by an expectation that a replacement existed. Announcing that one is needed is not arming one, and a check that nothing is running cannot separate "nobody is coming" from "the replacement is mid-setup". All three are sharpenings of the paragraphs that were already defective, not new rules; a second overlapping instruction is worse than none.
Folding the two stale-wait finders into one file moved the parked scope all-clear from architecture to layout. It used to hold because they were separate hooks, which structurally cannot couple; they are now separate functions sharing a file, and both are individually correct today, so nothing in the suite reddens if a tidy-up couples them — and the tidy-up will look like an improvement. The guard bars two specific edits rather than asserting a property: hoisting the reason filter to module scope, and routing both finders through one selector. A descriptive framing is untestable; these are assertable. AST-only, no import of the hook, no subprocess. Verified by four constructed couplings plus an unmutated control — two idioms per barred edit, the second written because the first was the one already in mind. All four killed, control green, mutations run in a detached worktree with sha256-verified byte restore. The control earned its place before any mutant ran: the first assertion named a lane of one function and fired on the unmutated tree, because a second function legitimately reads the same constant to build its journal event. Without the control that red would have been narrowed away, and the narrowing would have looked like debugging. A companion arm asserts the lane has not been widened, because the cheapest way to silence the first is to append a name to it. The docstring carries the guard's own expiry: if the parked disagreement is resolved and the finders are deliberately unified, delete this rather than amend it. A structural guard kept past its premise blocks the change it was written to make safe. Gate: 16061 passed, 85 skipped, unpiped exit 0, errors token 0, no FAILED lines, 1124s.
…sed one Landing as one commit rather than four. The fixes interleave in two files — background_work.py carries the anchor split, the consultant widening and the deletion; missed_wake_scan.py carries two of the new surfaces — so splitting by claim needs hunk surgery on the exact tree the gate certified, and each intermediate is a state that was never run. Two of them also cannot be separated in principle: the mutual-wait detector measures age from the anchor the first fix introduces, so shipping it alone gives a detector that cannot fire. THE ANCHOR SPLIT. `since` carried two jobs: a re-stampable freshness clock and the anchor that scopes which records a wait covers. The framework INSTRUCTS re-stamping, so a re-stamp moved `since` forward and silently widened the wait to cover launches made after it was raised — a rolling amnesty annulling the exact comparison built to prevent one. `covers_since` now carries the scope; `since` stays the clock. Written at the RE-STAMP rather than at first SET, which leaves every existing SET example unchanged: one instruction site instead of seventeen, identical behaviour. It is not immutable and the code does not claim it is. Agents write it, in the same call that re-stamps the clock, so its stability rests on the same discipline whose failure created the defect. Strictly better than scoping on a re-stamped clock; not robust. Absent is a third state, not a silent pass or fail: coverage falls back to `since` and the absence is surfaced. MUTUAL WAITS. Two agents can each make the other the resolver of their own wait, both flagged honestly, both holding what would clear the other. The surface says POSSIBLE, because `expected_resolver` records the KIND of resolver and never which one — two agents waiting on a third is byte- identical to two waiting on each other. Blind arm stated rather than implied: a wait on `user` or `external` cannot appear, so an empty result never rules a deadlock out. CONSULTANTS. The zero-match no-write was justified by "a teammate with no in_progress task is not inside a dispatch", which is false: consultants own zero in_progress tasks by definition, do real work, and were invisible to every layer. The resolver that gets this right already existed in the idle hook; it is now lifted and used. The false rationale is replaced with why it was false. Their expiry is weaker and the code says so. For a teammate, task completion IS the expiry signal. A consultant's anchor is already completed when the record is written, so that signal is spent before it can fire: coverage yes, expiry parity no, TTL only. THE DELETED PREDICATE. `is_durable_command` suppressed recording for commands matching dev|start|serve|watch. Three of those four are ordinary vocabulary, so `pytest -k start`, `grep -rn watch hooks/` and `build.py --env dev` were silently never recorded and every layer was off for them. It is not answerable from the command text. `npm run dev` and `pytest -k start` are identical in every property a matcher can see, and the over-fire population is DRAWN FROM the target population — a systematic exclusion of the core use case, not a tunable miss rate. Three candidate inputs were measured and all fail for one reason: they describe the command or the process, and the question is about the agent. "Is anyone blocked on this finishing" is intent, it arrives later, and `intentional_wait` already carries it. The suppressor asked the right question at the wrong time. Its measured behaviour is banked before deletion, because all three carriers died in this commit: the docstring, the arms that the docstring delegated to, and the comment on the regex. The former call site names the rule and points at the record. Removing the regex orphaned `import re`, found by AST rather than grep — `re\.` matches prose like "here." and "fire." That would have been a CI-only lint failure, since the local gate runs no linter. Gate: 16030 passed, 85 skipped, unpiped exit 0, errors token 0, no FAILED or ERROR lines, 940.91s. Collection parity 16115 at both scopes. Predicted before the run and matched exactly. Bracketed over 580 files across four markdown directories, staleness query passing its own control. A green gate certifies no regression. It does not certify the new surfaces fire: every non-vacuity arm enters at the finder, so all of them are silent on whether the caller reaches it. Entry-point and structural arms are tracked separately.
The surface asserted a single cause -- a forgotten paired wake -- for a condition that has several, and told the reader to act on that one. The hook cannot know which cause holds: SendMessage fires no hookable event and the inbox is written async-on-delivery, so the alarm sees a stale well-formed wait and nothing about why it is stale. Regroup by RESPONSE rather than by cause. Three classes, each naming what to do: send a wake, do nothing because the wait is legitimate, or the teammate must clear its own flag. Six causes sit under them, and the "nothing" class is stated as an outcome rather than as a prompt to investigate -- a deliberate hold is the common case and must not read as a failure. The cause list is grouped, not enumerated to completion. Two independent enumerations of this condition, written without sight of each other, overlapped in two of four; a flat list of any length would still assert a closure the hook cannot support. Header stays a single line with exactly one ACTION: token -- the render is pinned against forged line-leading directives from teammate-authored owner and subject fields. Full suite from pact-plugin: 16030 passed, 85 skipped, 0 errors, rc 0.
…sites The alarm and its two docstring justifications named a forgotten paired wake after a completion as the cause of a stale awaiting_lead_completion flag. find_stale_missed_wakes selects on status == "in_progress", so a task the lead completed leaves the population at the moment that cause becomes true. It is not merely unknowable from here -- it is unselectable. The cause that IS selectable was missing: a rejection written without its paired wake keeps the task in_progress with the wait still live. Swap it in, and keep the sent-but-undelivered case alongside it. The lead-side justification now rests on the true reason -- every response this condition needs is a lead action -- and carries a do-not-restate clause naming the in_progress gate, so the next reader does not re-derive the claim from the same plausible intuition that produced it. Surface compressed 1018 -> 847 characters. Header stays one line with one ACTION: token -- the render is pinned against forged line-leading directives from teammate-authored owner and subject fields, and that pin is what makes the header's shape load-bearing rather than stylistic. Targeted: 447 passed across 7 suites, rc 0, plus 55 on the prose gates.
Four arms asserted len(rendered) == 2. That 2 is the one-task instance of HEADER_LINES + len(stale), and the arms are injection guards rather than shape or size checks: owner and subject are teammate-authored, so the line count is how a forged line-leading directive is detected, and the ACTION: count is the same guard against a forged second directive. Written as a bare 2, a deliberate header change reddens all four and the natural repair is renumbering to 4 -- which restores the guard by coincidence and leaves the next change to break it silently. Assert the relation instead, against a hand-written HEADER_LINES rather than one derived from the render: an output-derived constant rises with a forged line and the arm becomes an identity that cannot fail, and even a non-identity derivation makes the guard follow the product instead of forcing a human to decide the shape changed. The relation and the per-row count cover opposite halves. Counting task rows catches a forged row; the total catches a forged line that does not start with the row prefix, which is the shape a forged directive has. These arms are a backstop for the sanitizer, not a first-line check -- the sanitizer strips the separator set before interpolation, so an injected newline never reaches the render by the normal path. The control therefore disables the sanitizer to exercise the backstop; removing that substitution kills the control and nothing else, which is what shows its redness comes from the disabled primary rather than from the fixture. The character budget bounds the HEADER only. The header is the fixed cost injected on every lead turn and is what a prose edit moves; rows scale with how many teammates are genuinely stranded, which is the condition being reported rather than a cost to cap. The fixture is named in the assertion because this one string has been measured at six different lengths, each correct for its own fixture. A companion arm reddens if the ceiling is raised far enough to stop binding. Targeted: pytest tests/test_missed_wake_scan.py -q -> 58 passed, rc 0. Mutation-verified on the pre-ceiling-change tree, and the counts do not depend on the ceiling: HEADER_LINES 1->2 kills 15 arms, the directive count 1->2 kills 2, and removing the sanitizer substitution kills only the control.
The startup notice warned that unattended runs may stall and told the user to relaunch with tmux. That remedy operates only on the teammate-message channel. An unattended run also stalls when a background job finishes, and that path never touches the message channel -- measured: message deliveries produce no queue records, background completions produce them by the dozen. A user who followed the notice would have switched modes and still stalled on the failure it appeared to describe. The document the notice cites was not a dangling pointer. It carried the same recommendation as its own summary, stated more confidently. Its teammateMode settings guidance was correct and is preserved verbatim; a correct section lost while rewriting a wrong one is a new defect rather than a cleanup. Correcting the wording alone would have moved the falsehood rather than removed it -- true about the channel, false about us -- because nothing instructed the lead to tell the human anything. The wait-discipline section covered arming a watcher, spending one, and retiring one, and was silent on a watcher that fires while nobody is there to read it. It now says to send a push in that turn, and to let the tool's own refusal decide whether the human is present rather than inferring it. The deadline instruction already present is the floor and is not restated: a reader who satisfies one copy believes the subject covered. The mobile surface is not claimed. The call reports what it requested, and from here an unconnected remote is indistinguishable from a silent failure, so no wording invites testing it. Targeted: 338 passed, 61 passed, 152 passed, rc 0 on each.
The summary table pairs each stall with the remedy for its own channel, and stops there. Neither row covers the reader who is simply not at the desk -- which is the condition under which either stall matters at all. The fallback that covers both already existed further down, because it depends on neither channel working. A reader who stops at the summary, which is what a summary is for, would not reach it. Prose pointer rather than an intra-document anchor: the sentence works without one, and an anchor is a new construct for the cross-reference gate to validate. Targeted: 410 passed, rc 0.
The push instruction tells the lead to notify the human when a watcher delivers or dies. Nothing covered the case where the lead itself is never told -- idle, with the work long finished and no signal reaching anyone. A one-shot scheduled job fires into an idle session on the scheduler's own clock, so it is not subject to whatever holds a completion notification. It reports the watcher's silence rather than the work. Key it on the deliverable, never on whether a message arrived: any message satisfies "has anyone reported since T", so such a job fires, finds its condition met, and is spent while the wait is still open. That is the existing spent-watcher failure reached by a new route, which is why the paragraph names a fired job as a spent watcher and sends the reader to the rule two paragraphs above rather than restating it. One-shot rather than recurring. A recurring job firing through a long wait is polling, and delegating the trigger to a scheduler does not change what the action is. The gap that recurring would close -- forgetting to re-arm -- is already closed by the spent-watcher test, once a job is subject to it. Scope stated in the prose: a lost notification, not a dead session, since these jobs live only as long as the session. Where the tools are absent the paragraph defers to the deadline statement already present, which is not restated here. Targeted: 167 passed, rc 0, across the agent-surface convention suites.
The previous paragraph specified a one-shot re-armed on each firing. That polls exactly as a recurring job does -- same firings, same turns -- while moving the remembering from one act to one per interval. The two differ only in what forgetting costs: a missed delete is spurious wakes, a missed re-arm is coverage that stops with nothing reporting it. The bound that made the one-shot look safer does not survive either. These jobs live only as long as the session, so a forgotten recurring one cannot run past it, which is the same limit a forgotten one-shot's silence has. Four clauses depended on the one-shot shape, not the one that named it: the job type, the self-deletion, the spent-watcher linkage, and the word describing a mis-keyed job. A mis-keyed recurring job is not spent -- it fires again and reports nothing each time -- so that word reverts. The linkage is cut rather than reworked. A recurring job is the one watcher in this section that does not go unarmed by firing, so pointing at that hazard would misdescribe it. The hazard that does apply, deleting the job while the wait is open, is retiring a watcher in the ordinary sense and the paragraph above already covers it. New: the interval is sized to the expected duration. A short interval through a long wait is the polling the previous framing objected to, and that objection was the part of it that held. Targeted: 167 passed, rc 0.
…king them The suite could not tell the fixed implementation from the one it replaced. Breaking a function outright reddened nine existing arms; reverting it to its previous behaviour -- never reading the anchor, always using the re-stampable timestamp -- reddened none of a hundred and twenty-four. So the surfaces read as covered while the change to them was unguarded, which is worse than a gap, because a gap sends a reader to write an arm. Every case here is proved against a revert to the prior behaviour rather than against a destroyed function. A destroyed function is not the edit anyone makes; restoring the simpler earlier shape because it reads cleaner is. Three such reverts on the consultant path -- ranking completed anchors by string order, expiring those rows like ordinary ones, restoring the status filter -- each killed by these arms and by nothing that existed. Two behaviours needed pinning in both directions on identical inputs, because each reads as an oversight alone: an absent anchor still covers deliberately, and consultant rows deliberately do not expire on theirs. Stated as an asymmetry someone must keep rather than two rules a reader reconciles by deleting one. The alarm's second response class must keep prescribing nothing. A literal on its text would have passed the revert criterion vacuously, since the older wording shares no words with the current one -- so the arms are built against a future editor making that class actionable, which arrives either as a replaced directive or as an action added to an intact one. Two shapes, two arms, each the sole detector for its own, with the bound recorded where the next reader will be standing. Entry points are paired: a structural arm alone passes against code that raises, so each is matched with one that drives the behaviour. Collection 16119 -> 16170, equal at both scopes. 109 passed across the four files, rc 0.
The gate regenerates this map, and the three test files added with the wait-enforcement coverage are new keys in it. Thirty-three insertions, no deletions, no changed entries, and the drift field byte-identical -- checked by set-differencing the parsed blobs rather than by reading hunks, and the three new entries independently regenerated to confirm they match. A first regeneration reported all three differing, every origin null against a resolved path in the file. That was the probe: the resolver uses find_spec, which cannot resolve a module the probing process never imported, while a real run has the path setup and the modules loaded. The tell was that the disagreement was uniform -- a real identity change is lumpy, a broken probe nulls everything at once.
A declared project scope was treated as a hint. When the directory it names holds no CLAUDE.md the resolver did not stop -- it continued through the git anchors and wrote into whichever project it reached next. Measured on this checkout: a declared directory with no CLAUDE.md resolved to the maintainer's real file; the same declaration with a file was honoured. The suite's own fixtures had been landing there twice per run against a three-deep window, so real working memory was evicted rather than merely joined. The discriminator is escape, not absence. Absence is legitimate: a worktree carries no CLAUDE.md of its own and depends on reaching the main checkout's. So the question is not whether resolution fell through but whether it left the project, and a predicate answering that already existed in the pin archival path. It moves to hooks/shared, composing the git wrapper rather than widening a module scoped to subprocess boilerplate, and the original site keeps a re-export -- verified by object identity, so a later divergence cannot hide behind a passing test. That predicate is not symmetric. It asks whether one directory is the main repo of the checkout at another, so the worktree case and a subdirectory case call it in opposite directions and get opposite answers. Its four directions are now stated at its home, because the reversed call is the one a reader reaches for. Both declarations are bounded -- the environment and the session record -- and the derivations below them still chain, which is what keeps the worktree path open. The refusal is loud rather than silent, matching the two guards already in that file: the save completes and returns its id, and only the projection is refused, so a misconfigured scope costs a display line and reports why. Measured: the incident refuses, the worktree fall-through is allowed, git and non-git layouts behave identically, and a non-git project with no CLAUDE.md never reaches the guard because resolution returns nothing first. One accepted under-block is now closed and its tripwire inverted rather than deleted: an escape is refused whether the store was redirected or default, because the incident had a redirected store and escaped anyway. Store origin never tracked the property; repository identity does. The cardinal case this design protects had no coverage. The arm claiming to hold it used a fixture with no git repository, and the nearest substitute used a subdirectory -- which reports a relative common dir where a worktree reports an absolute one, the opposite side of the predicate's own branch. A real worktree arm now covers it: deleting the exemption or swapping the predicate's arguments reddens it, and reddens nothing among 226 other arms.
Four arms, one per claim the refusal makes, each proved to die only to the specific wrong idea it exists to catch. Measured against three mutations in a detached scratch tree with a byte-restore check: re-adopt store origin as the discriminator -> 4 arms red swap the predicate for a containment check -> 2 arms red refuse on absence instead of on escape -> 1 arm red The containment result is the one worth recording, and it strengthens the rejection of a path-prefix proxy beyond the argument that was actually made. The argument was that containment does not track repository identity. True, and the weaker half. The stronger half is measured: a worktree is a SIBLING of the main checkout rather than a descendant, so a relative_to() check refuses the very fall-through the design exists to protect. A prefix proxy would have re-created the cardinal over-block while closing the narrow one. The narrow refusal gets a pin for the same reason the tripwire did: it is an accepted boundary a future reader will want to relax for a plausible reason, and the arm carries that reason so the relaxation has to be deliberate. The unresolved arm pins the exact resolver value rather than merely not-refused, because a not-equal assertion also passes on a failed resolve.
The registry's expiry prune runs on the read that happens inside the write, so a caller could pin the clock for every read and still not pin the write it was pinning it for. Two entry points carried that: append_record, which prunes via _atomic_update_records, and record_background_launch, which prunes there AND stamps registered_at through a second, independent clock. iso_now falls through to canonical_since when called bare, and canonical_since reads datetime.now(timezone.utc) directly rather than through this module's utc_now. Threading the prune alone would have pruned at the injected time while stamping at the real one — manufacturing a divergence the system clock cannot produce, which is precisely the artifact that made four unrelated arms read as failures earlier. canonical_since sits on the `now is None` branch, so a supplied clock bypasses it rather than diverging from it, and its contract — shared with the intentional-wait surface — is untouched. FIVE OTHER WRITERS DELIBERATELY DO NOT GET A now, AND THAT IS THE POINT. save_records replaces the file whole and never enters the read, so no prune runs inside it; the three idle-count writers and _write_records have no expiry semantics at any depth. A now= on a function with no clock is not a harmless courtesy: the parameter is a PROMISE of determinism, so a test written against it passes while determining nothing. That is the same defect as a partially threaded clock, taken to its limit. The distinction the census has to make is lacks-a-clock-it-needs versus has-no-clock, and only the first is a defect. Every parameter is keyword-optional with the default path byte-unchanged, so no existing caller's behaviour moves and no test needed a signature change. The closure behind the five omissions is a full transitive walk of the call graph, not a name search. The first attempt filtered callees against a hand-written list that misnamed one of them, and returned "no callees" for a function whose callee had never been opened — a hand-curated filter is the same failure as a hand-curated census, one level further down.
The new parameter was left unannotated on the theory that a type would drag a datetime import into a hook that runs a subprocess in every consumer session. That theory was false: this file carries `from __future__ import annotations`, so annotations are strings and the import never executes. The only real cost was a linter complaint about an undefined name, which a TYPE_CHECKING block answers for nothing — verified by confirming the name is absent from the module namespace at runtime. The block keeps a comment where the parameter does not need one. Nothing about the annotation requires explaining any more, but the construct does: a reader tidying this into a plain import would silently reintroduce the runtime cost it exists to avoid. That is the durable kind of comment — it explains a deviation the reader would otherwise undo, rather than justifying an absence. First use of the pattern under hooks/; it appears elsewhere only in the import auditing tooling and its own tests, which is why those suites were run against this change specifically rather than only the hook's own.
Ten arms went red between two gate runs this morning with no source change between them. The fixtures pinned an absolute date while the entry points they called resolved the real clock, and records expire at twenty-four hours, so the fixture crossed that threshold mid-morning. The arms were green for exactly one day after they were written and would have been red on every run thereafter, including CI, with nothing having changed. Repaired by giving each arm control of the clock it is measured against rather than by moving the fixture forward. A fixture rewritten as now-minus-an-hour greens the suite today and keeps the bomb, relocating the failure into whatever window the new offset happens to sit inside; an arm that injects the clock has no bomb at any interval. Five arms inject at the call, four pin the clock at the module seam because their entry point takes no clock, and one stamps at write time through the same call production stamps with — that one had no gap to begin with, since it writes and reads inside the same milliseconds. The repair is proved not to have blinded them: under the mutation these arms exist to catch, the same five die BY NAME before and after, so what changed is their greenness and not their sight. The concurrency arm now dies to a neutered exclusive lock; the first attempt at that mutation disabled the lock by a route that SKIPS the arm, and a skip exits zero exactly like a pass. Two arms pinning exact thresholds are untouched and were never at risk — they pass an explicit clock into pure predicates, so no live clock is involved. Verified across the whole tree rather than the touched files: the full suite at the real clock and at the real clock plus four hundred days produces identical results, so no arm anywhere can be failed by moving the clock forward. That holds outside a stated blind spot — a hundred and sixty-two arms spawn children with an environment that drops the shim, and are unreachable by this instrument. The literal census that mapped the search is not the measurement; an absolute date is only a bomb when something ages it against a live clock, and only the clock-move run can tell those apart.
Two entries only, for the module added with the project-escape refusal. The file regenerates during a run and also wanted to rewrite a path array with an absolute path naming this worktree; that hunk is deliberately excluded. It duplicated an entry already present, so it carried no information, and the array's standing habit of embedding a developer's home directory into a tracked file is a real defect but not this change's — closing it means deciding whether the array should be repo-relative or dropped at all, which is a design call rather than a cleanup. Nothing in the repository reads this file. It is written only when an environment variable names an output path, so no gate consults it and no test depends on it. The two entries land anyway because leaving them out makes the next reader's diff attribute the import edge to whoever happens to regenerate it next.
…he promise The detector read one field — the harness background flag — and never the command text, so work backgrounded with a trailing `&` inside an ordinary foreground call was invisible to every layer. That is not a corner: it is the shape a teammate used for the incident this branch was built around, so the mechanism could not have caught the case that motivated it. A live probe against a patched install found it; no fixture could, because the failure is a hook declining to consider a launch a launch. Two changes, and they are one claim. The predicate gains a separate, separately testable companion matching a command that ENDS in a bare `&`, composed as an OR at the single call site. And the orchestrator's watcher line, which promised that backgrounded work would re-invoke the reader on exit, now names the mechanism that actually does that — the promise was true of the flag and false of a trailing `&`, which returns immediately and re-invokes nobody. WHY A TEXT PREDICATE IS ADMISSIBLE HERE WHEN ONE WAS DELETED FROM THIS FILE. The deleted matcher asked whether a program would run long, which its text cannot answer, and it was a SUPPRESSOR: an over-fire removed a record and produced silence, which is the exact failure a detector exists to prevent. This one asks a question the shell's own grammar answers, and it is a WIDENER: an over-fire produces one extra row that is visible and dischargeable. The direction of failure is inverted, not merely the decidability, and the rule the call-site comment now carries is the usable form of that — nothing here may REMOVE a launch from the registry. The widening is under-inclusive and the docstring says so rather than implying coverage: a command that backgrounds mid-line, in a subshell, or with the job disowned is still missed. The population is now "the flag, or a command ending in a bare `&`" — narrower than the phrase this branch has been using, which is corrected wherever it appeared, including the module's own scope paragraph and the lead-facing surface string that both described the old, wider claim.
…empts it The call-site comment carries the rule for the filter someone might add there. This carries it for the predicate itself, because the mistake it guards against is made by a different reader — the one extending this function to catch another launch shape, who needs to know which extensions are in bounds. Two reasons are given and the second is load-bearing. That `&` is shell grammar with one meaning, answerable from the string, where "will this run a long time" is a fact about program behaviour that is not, explains why this check is decidable. It does not explain why it is SAFE. The safety comes from direction: this predicate only ever adds to the recorded population, so an over-fire costs one visible, dischargeable row, while the durability filter this module used to carry paid for an over-fire in silence — the exact miss the mechanism exists to prevent. Stated as the rule a future editor can apply rather than as history: a new check may widen what is recorded and may never narrow it. "No text matching" would have been the wrong rule, since following it here would have meant leaving the population wrong.
The composition arm is the one that carries the weight, and it was built and proved before anything else. A frame with no background flag and a command ending in a bare `&` must produce a registry row through the real seam. Reverting the gate to its flag-only form reddens exactly that arm, by name, and nothing else. Why it had to come first: the other arms test the new predicate in isolation, and every one of them passes whether or not the recorder ever calls it. Twenty- two green predicate cases can coexist with a widening that is not wired in at all. That is not a weakness of predicate tests, it is what they are — and the class docstring says so before anything else, so nobody counts the greens and concludes the widening is verified. The property underneath is general: only an assertion of a POSITIVE record through the gate can detect a wiring break, because an unwired gate fails closed and a silent refusal is indistinguishable from a correct one. That is also why no arm was added for the flag branch. The obvious worry — an OR has two branches, and only one was being exercised — was checked by dropping that branch instead of assumed, and two existing arms already driving the seam with a flag-present frame caught it. A third would have been redundant. The documented-limits arm asserts a limitation, not a requirement: a subshell, a mid-line background and a disowned job are still missed. If the predicate is ever legitimately widened, the right response to that arm reddening is to delete the now-caught case and move the docstring's list with it, and its failure message says so, because an arm that reads as a requirement invites someone to restore the old behaviour to make it pass. The structural guard asserts that a clock parameter is never decorative. Its invariant was derived by measuring and rejecting two stronger forms first — "must read a clock" and "must forward the clock" both fire on a function that reads no clock and is correct. What survives is the weakest form that still catches the defect: a `now` parameter must be referenced at all. The guarded set was enumerated from source rather than carried from an earlier list, and that list turned out to be wrong in both directions — it named a pure reader, and missed two writers. The guard carries its own control, checked against two synthetic functions whose answers are known, because if the reference detector ever stops discriminating then both guard arms go quietly green together and the control is the only thing that says so.
3 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…mmate's team The background-launch advisory told Agent-tool subagents that nothing would wake them, which is false: their parent waits for them. And a teammate running as its own process resolved no team at all, because the session context file is written for lead frames only, so its launches were never recorded. background_work gains frame_team_and_name(input_data): the session context when set, otherwise a member-validated session-registry entry, otherwise an agent_id of the form name@team validated against team membership. A single teammate-launch predicate built on it drives both the advisory in wait_filler_gate and the launch record. It returns true for a member name, for name@team, and for a registry-resolved separate-process teammate; false for a lead frame and for a hex agent_id that no member owns. wait_filler_gate imports shared only for a teammate-shaped background launch, so ordinary Bash calls stay stdlib-only. It is now a seam-dependent hook with an integration test and a pinned import closure.
test_merge_guard_1178_f2_cert compared the live classifier against an old version of merge_guard_common.py read with `git show`. That commit is reachable from no ref in any clone, so the file's differential rows skipped everywhere, not only in CI: the certification had never run. The version (8906175) is now stored under tests/fixtures, registered in the vendored loader by git blob id, and loaded from there. The history loader, its skip marker and the unused import are removed, so the file's 71 tests run everywhere with no skips. This completes the five merge-guard certification files: together they run 698 tests with none skipped, where CI previously skipped 434 of them.
Several test docstrings, comments and names stated how many members a set had instead of naming the set: "both guards", "the two ambient refusals", "all 13 agent files", test_all_13_agent_files_present. Each count goes stale the next time a member is added or removed, and nothing checks it. Each site now names what it counts, for example "the stderr guard and the store scope", "every agent body under agents/" and "every canonical template across SPAWN_PROMPT_SURFACES". Two names change with their count: the class TestTheStderrGuardAndStoreScopeUnwindWithoutLeaking and the test test_every_expected_agent_file_is_present. Nothing else referenced either old name. Prose and names only; each file's AST is otherwise unchanged. Three of these lines sat in files this branch already changes, but inside docstrings it never touched. The earlier sweep scanned only the string and comment units that overlapped added lines, so it could not reach them.
The turn-end check compares running background tasks against launch records to tell a teammate's jobs from the lead's. The running-task list carries each job's harness id, but the record never stored it, so there was nothing to join on. record_background_launch now stores tool_response.backgroundTaskId as harness_task_id, and the record sanitiser keeps it only when it is a non-empty string. A shell `&` launch has no harness id and records none. A direct-call test now covers the teammate predicate's registry-team check: the same frame is a teammate for the team its registry entry names and is not one for any other team. Two documented ablation counts in the integration tests are corrected to their measured values.
… reach Date bombs are found by running the suite with the clock shifted a day either way. The shim that did the shifting lived outside the repository, was activated by hand, and silently missed children that start a bare python3 or a `bash -c` template, plus any code calling time.time or the no-argument time.localtime/gmtime/ctime/strftime. tests/clock_shift now holds the shim, a one-line sitecustomize that imports it, and a census. The shim does nothing unless PACT_TEST_CLOCK_SHIFT_SECONDS is set, and activation is through the environment (PYTHONPATH), so every child inherits it. While a shift is set, the census watches each Python or shell child the suite launches and fails the run if a child's environment drops the shift or the shim path, if another sitecustomize shadows the shim without chaining to it, or if a child that should write this run's ledger ran and the ledger stayed empty. The census installs from conftest only while a shift is set, and a launch is exempt only when its own environment carries PACT_TEST_CLOCK_SHIFT_CENSUS_EXEMPT. test_cli_output_purity writes its own sitecustomize ahead of the shim, so under a shift its children never loaded the shim. Its generated blocker now chains to the shim, and the census catches any sitecustomize that doesn't.
…ater A launch record lists the tasks the teammate held when it started the job. A wait flagged later on a newly claimed task matched no record, so it neither quieted the reminder nor cleared the record, and the teammate kept drawing an advisory for a job it had acknowledged. When a teammate claims a task, task_lifecycle_gate now adds that task to every live record the teammate launched, in one registry update. The owner is read from the task on disk and the team is resolved through frame_team_and_name, so separate-process teammates are covered too. The record's registered_at is untouched, so a wait still clears only jobs launched before its anchor. No registry file is created when none exists, and an update that doesn't claim a task adds nothing.
A lead or a separate-process teammate could end its turn with a background job still running and no wait flagged. Nothing noticed: PACT registered no Stop hook, and the teammate-side layers only run inside the lead's process. A new Stop hook, stop_background_gate.py, reads the running jobs Claude Code passes at turn end. With none running it returns before importing anything. Otherwise turn_end_gate resolves who is stopping, positively or not at all: the lead by its lead spelling or by the team's lead session id with no agent id, a separate-process teammate by a member-validated identity. Anything unresolved is allowed, so a plain non-PACT session is never blocked. For the lead, jobs a teammate's launch record claims by harness task id are excluded, and a scheduled wake counts as a flagged wait. For a teammate, jobs covered by a valid wait on its own tasks are excluded. A remaining job blocks the turn end once, naming the job and the fix: flag the wait or collect the result. Each job id is reported at most once through a told-once file, and nothing blocks while stop_hook_active is set. Every evaluation past the fast path writes a background_stop_gate journal event, so the hook's firing is observable. The livelock invariants that pinned "no Stop hook" now require that Stop binds only this hook, and the livelock scan treats a printed block decision as a sink that needs an attestation.
…rktree is gone Once a linked worktree was removed with `git worktree remove` from outside its repository, nothing left on disk tied the folder to its project, so the memory write guard refused every later write that still named it. Sessions started in a subdirectory of a worktree, and teammates running as their own process, had no route at all. At session start, session_init now writes worktree-identity.json into the session's own folder, through state_file and failing open, for every role and before the lead check, whenever the project directory sits inside a linked worktree. It records the exact declared directory, the worktree root and the common git directory. pact_session reads that record back only when its session id matches CLAUDE_CODE_SESSION_ID and every path is absolute. When the declared directory no longer exists, a record naming exactly that directory decides the verdict: the write is admitted only at a checkout root of the recorded repository and refused otherwise, including when git gives no answer. A record for any other directory, or none, leaves the existing checks unchanged. Sessions started before this change have no record and keep the refusal. session_init now reaches state_file, so its pinned import closure gains it.
…d work A teammate running inside the lead's process ends its turns through SubagentStop, where PACT's only hook is validate_handoff. So the new turn-end check could not see those teammates, which is PACT's default setup. validate_handoff now also runs the shared turn_end_gate evaluation, and only when a job is running, so an ordinary SubagentStop still never imports it. A teammate is identified from the platform's own subagent metadata or from team membership. Both reasons join into one decision, so a turn end gets at most one block from PACT, and nothing is marked as told while stop_hook_active is set. validate_handoff also stops applying the prose HANDOFF check to teammates on purpose: a teammate's HANDOFF lives in task metadata and its ordinary turn ends are idles, so that check would refuse them. Its docstring now states the population it validates. stop_background_gate joins the seam-dependent hooks with a pinned closure, integration coverage and a live-probe entry. validate_handoff moves to the live-probe set, because its output now depends on a seam.
…dy reads archive_pin resolves CLAUDE.md with the same project-scope predicate as the working-memory write guard, but never passed it the session's worktree record. So once a linked worktree was removed, the two gave different verdicts in four layouts: archival still refused a sibling worktree removed from outside and a worktree-subdirectory session, and still admitted a removed worktree whose record names another repository, and a nested worktree subdirectory resolving into the enclosing repository. The record reader and its session-folder glob move unchanged into shared/project_scope. pact_session re-exports the reader rather than wrapping it and delegates its context-file lookup to the shared glob, and archive_pin imports the reader through the shared package and passes it to the predicate. The guard and archival now hold one reader and agree in all four layouts. No hook's import closure changes.
stop_background_gate.py was the one hook without `from __future__ import annotations` after its docstring, so the py39 annotation-compatibility rule failed on it in every run. The fast-path docstring said nothing but json and sys was imported before a running job is found; the future import makes that literally false, so it now states what the test pins: no plugin module is imported until a running job is found.
…shim The shim shifted time.time() and datetime but left file timestamps on the real clock, so under a shift a file written a moment ago read as a day old (or, backwards, as future-dated). Every expiry check that compares the clock with a file's mtime then fired or failed to fire for the instrument's reasons, not the code's: 346 of the 348 reds in a -1/+1 day sweep flipped exactly across the 900 s, 3600 s and 10800 s windows in the code. os.stat, os.lstat, os.fstat and os.scandir's DirEntry.stat now return atime, mtime, ctime and birthtime shifted, with their nanosecond and integer forms, and os.utime subtracts the shift from explicit times so they read back as set. The posix module stays untouched, so bytecode validation runs on the real clock, and a zero or unset shift installs nothing.
The census printed a launch's raw argv, so a multi-line `-c` script split one problem across several lines and broke per-file grouping. The excerpt is now whitespace-flattened. The docstring also states that a line names its test from PYTEST_CURRENT_TEST at launch, so a test that deletes that variable prints as outside a test and is placed by launch order.
…onment Tests that build a child's environment by hand dropped the clock-shift variables, so those children ran on the real clock. Under a shift, two consultant-seam tests failed when a real-clock child's record looked past its 24-hour lifetime, and 76 test nodes in 12 files went unmeasured by the sweep. carry_clock_shift(env) returns a copy that adds only the shift seconds, the ledger path, and the shim directory ahead of the env's own PYTHONPATH, and returns the env unchanged when the run is unshifted. It copies nothing else, because several of these tests exclude PYTEST_CURRENT_TEST or the real HOME on purpose. Every explicit-env launch in the 12 files now goes through it; launches that already spread os.environ inherit the shift and are left as they were.
…replace The shim installed plain Python functions as os.stat, os.lstat, os.fstat, os.scandir and os.utime. A plain function binds as a method when a class stores it as an attribute, and the builtins do not, so Python 3.9's pathlib accessor passed itself as the path and a shifted run failed where an unshifted one passed. The wrappers were also missing from os.supports_*, so shutil fixed its rmtree path differently under a shift. Each wrapper is now a callable instance, which a class calls unbound, and joins every os.supports_* set its real function belongs to at install, before shutil can be imported.
…modules The baseline still recorded test_background_work importing discharge_acknowledged, which this branch removed, and had no entry for the 14 test modules the branch adds. Regenerated from pact-plugin with PACT_IDENTITY_MAP_OUT. Against the old map, no import name that appears in both resolves to a different file, and the drift list is unchanged.
…oved test_merge_guard_1136_canonical_join_ssot said its loader mirrored a 1118 recert helper that vendoring removed, and that other certification files skip on commits no origin ref reaches, which stopped being true once their bases were stored in the repository. It now describes its own git-history loader and says vendored certification files read no history. test_lazy_load_cross_references described xfail-strict markers the file no longer has, and carried rollout and planning labels. It now describes what its tests check. Both edits are prose only: each file's code is unchanged once docstrings are set aside.
…eammates Four agent-facing passages said a teammate's background-task notification never wakes it. That holds for a teammate running inside the lead's process, and was observed false for a teammate running as its own tmux process, which its own completion notice woke. Each passage keeps its instruction, which stays universal because a teammate cannot know its own mode, and now scopes the factual claim to running in-process: the pact-agent-teams wait discipline, the pact-teachback intentional_wait rule, the orchestrator's watcher paragraph, and the auditor's wait note. The layer pin follows the new sentence, and a new structural test asserts the three unscoped phrasings are gone from every agent-loaded surface.
… crons only on Stop A live session showed that background_tasks lists every live teammate and subagent as a running entry. The turn-end gate counted every running entry, so a PACT lead with live teammates was refused once per teammate and told they were background work. A turn end with only two teammates running was refused over exactly those two. An in-process teammate's SubagentStop frame also carries the lead process's crons, so any lead cron silenced every in-process teammate's check. One import-free module, shared/turn_end_jobs.py, now holds per-role allowlists in the frame's own type labels. The lead counts shell jobs; a teammate in its own process counts its shell, subagent, monitor, workflow and MCP jobs; an in-process teammate counts its recorded shell jobs. A type the platform adds later is not counted. The Stop hook loads that module by file path, so its fast path still imports no plugin package, and a lead with only teammates running now takes it. session_crons exempts a stop only on Stop, where the list belongs to the ending agent's own process. A turn end with nothing counted traces allow_no_job.
The launch advisory, the idle-count advisory and the lead's unflagged-work surface exist because a teammate is not woken when its own background job finishes. That holds for a teammate running inside the lead's process. A teammate running as its own tmux process was observed being woken by its own completion notice, so for it those three layers are false and alarm about work it has already collected. background_work.teammate_is_separate_process is true only when the member's team-config backendType is exactly "tmux"; anything else, including a missing or unreadable config, keeps in-process behaviour. The three layers skip a separate-process teammate, and launch recording is unchanged. teammate_launch_name is now the one copy of the launch identity steps. The lead's surface says the teammates it lists are not woken by their own job finishing.
The lead's turn-end refusal said a background job's completion notice "does not start" a turn. For the lead that is false: its completion notices were measured starting lead turns every time. The text now says the notice usually starts the next turn but can go undelivered, which is the case the refusal exists for. The teammate text is unchanged.
The launch advisory told every teammate that nothing would wake it when its background job finished; it now scopes that to an in-process teammate. The idle advisory and the lead's unflagged-work surface called a recorded launch outstanding, but the record outlives the job, so the job may already have finished by the time either fires. Both now say the teammate launched background work with no flagged wait, and that the job may have finished.
A teammate running as its own tmux process was refused at turn end with text saying its completion notice would not wake it. That was observed false: its own completion notice started its next turn. The Stop hook now gives a separate-process teammate its own text, saying the notice usually starts the next turn but can go undelivered. A teammate running in the lead's process keeps the not-woken text on SubagentStop, and the lead keeps its own. What each role counts and when it is refused is unchanged.
…lling A live session showed an Agent-tool subagent spawned as PACT:pact-preparer reporting that literal agent_type on SubagentStop. is_pact_agent matched only the bare pact- spellings, so the prose handoff check was silently skipped for the namespaced form the model is offered. pact_context now defines PACT_NAMESPACE and strip_pact_namespace, which removes exactly one leading, case-sensitive "PACT:". is_pact_agent applies it to string identifiers, and session_init's inline strip uses the same function, so the namespace is spelled in one place and pinned to the plugin name.
…are spelling The dispatch gate treated any subagent_type not starting with "pact-" as outside its concern, so a spawn written as PACT:pact-preparer skipped every check: name, registry, team and task. A live session's journal showed one such spawn allowed with no rule. The gate now strips one leading "PACT:" from a string subagent_type before its first rule, using the same normalizer as the handoff check, so the namespaced spelling gets exactly the bare spelling's verdict. The journal still records the spelling the caller used, and a non-string subagent_type falls through as before.
…ad instead of pointing it at bootstrap A separate-process teammate never has a PACT session context file, so the dispatch gate's missing-team refusal fired in that process too. There it told the teammate to re-run bootstrap, and appended a stale-session warning, because the teammate's own session id never matches the one the project records. Both pieces of advice are wrong in that process. A frame classified as a teammate now gets a refusal that names the team-lead as the one who spawns specialists, with neither the context-file diagnosis nor the stale-session hint. Lead frames, frames with no agent type, and callers that pass no frame keep today's text. The verdict and rule id are unchanged.
…o the idle checks run there A separate-process teammate's TeammateIdle hook runs in the teammate's own process, which never has a PACT session context file. teammate_idle read its team only from that context, so it exited before any check ran: no stall advisory, and no idle timestamp for the lead-side scan. frame_team_and_name gains a route between the context route and the registry route. It trusts the frame's team_name when that name is path-safe and the frame's teammate_name is a member of that team. teammate_idle now takes its team and its tasks through that helper. In the teammate's own process, meaning no context team and a frame session different from the team's lead session, the hook runs only the teammate-facing background check. It skips the lead-facing idle cleanup, whose shutdown text would otherwise reach the teammate. A lead whose context file is missing still runs the cleanup. Tests that controlled main through the calls it no longer makes now patch the helper and the task iterator instead.
… their peer list file_tracker and peer_inject read their team only from the PACT session context, which a separate-process teammate's own process never has. So a tmux teammate's edits were never recorded or checked for conflicts, although the tracker's docstring claimed that coverage, and its PACT subagents were given an empty peer list. Both hooks now take their team through frame_team_and_name. When the context has no session id, file_tracker keys the editor on the frame's own session id, so two separate-process teammates with the same name still see each other's conflicts. The classifier's import closures for both hooks gain the helper's modules.
…tate-file tests file_tracker and teammate_idle now take their team from frame_team_and_name and teammate_idle reads its tasks through iter_team_task_jsons, so the two tests that patched the removed get_team_name and get_task_list names failed with AttributeError. They now patch the helper and the task iterator. Both still reach the containment refusal: with the shared state-file directory check disabled, each test fails and the write lands outside the teams directory.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0e2eae0. Configure here.
| if not session_team: | ||
| if (isinstance(input_data, dict) | ||
| and pact_context.classify_session_role(input_data) == "teammate"): | ||
| return ("DENY", _TEAMMATE_SPAWN_REFUSAL, "team_name_unavailable") |
There was a problem hiding this comment.
Teammate spawn deny matches too broadly
Medium Severity
Rule ⑥’s new teammate refusal keys on classify_session_role returning teammate, which is any non-lead agent_type. That includes a solo specialist session and an Agent-tool subagent, not only a teammate in its own process. When session context is missing, those frames are told to ask the team-lead instead of restoring context with bootstrap.
Reviewed by Cursor Bugbot for commit 0e2eae0. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enforce the background-wait discipline mechanically, and fix what that exposed
An agent that starts a long job and ends its turn looks exactly like an agent
still working. Nobody learns anyone is blocked until someone thinks to check.
This makes the wait visible — recorded when it starts, surfaced when it goes
stale — and fixes the defects that building it uncovered.
docs/is gitignored and stays untracked.Why a fresh branch
The prior attempt was sound in content and unusable as history: every commit
authored by a non-human identity with attribution trailers, 480 lines of
force-added
docs/against.gitignore, a stale version, and fivesys.pathinserts that merge cleanly and then fail an import pin landed since. Content
carried forward; history re-landed.
What the mechanism does
Five layers. Four ride hooks PACT already had. The fifth, a
Stophook, is theone new registration. Every hook in
hooks.jsonruns a subprocess in everyconsumer session, so its cost is measured below rather than asserted.
PreToolUseBash hook tells a teammate thatlaunches background work that nothing will wake it, and to flag the wait. A
lead, a plain session and an Agent-tool subagent get nothing: team
membership decides, not the shape of the frame.
PostToolUseBash hook writes a teammate'sbackgrounded command, and the harness task id it was given, into a
team-scoped registry.
launch outstanding, a one-shot advisory fires.
threshold at the start of a lead turn opened by a user prompt, a scheduled
wake or a background-task notification, and at session start. It does not run
on a turn opened by a teammate message (measured: 0 of 355).
the jobs still running. The new
Stophook reads that list for the lead andfor a teammate in its own process. The existing
SubagentStophook reads itfor a teammate running inside the lead's process. If a job is still running,
no wake is scheduled, and no wait covers it, the turn end is refused with an
instruction: the lead is told to wait, schedule a wake or stop the job, and a
teammate to wait or flag the wait. Each job is refused at most once, so an
agent that cannot comply is never trapped.
Cost of layer 5, per turn end, median of 50 interleaved runs on an Apple M4
(10 CPUs, Python 3.14.6) at load average 18 falling to 16.5, busy rather than
idle: +9.5 ms CPU, +9.9 ms wall when nothing is running, over a bare
python3 -c passat 24.2 ms wall. A lead turn end that is refused takes 96 mswall (p90 119). An earlier run at load 53 measured +9.2 ms CPU, so the figure
does not depend on load. Nothing from the plugin is imported until a running
job is found.
Scope. This paragraph has now been wrong twice, so it states the population
as the code implements it rather than as prose about the code.
A launch is recorded when a Bash frame carries the harness background flag,
or its command ends in a bare
&. That is the whole population. Two limits,an outer and an inner:
nine background-task types the platform tracks raise nothing there. Layer 5
narrows this at the turn end: the lead and a teammate in its own process are
refused over any running job in their process, whatever its type. A teammate
inside the lead's process is refused only over jobs layers 1 and 2 recorded
for it, because the list it receives is the lead process's, with no owner. A
running job that no record claims is put to the lead, whose refusal says to
end the turn again if the job is a teammate's.
(
./gate.sh & echo started), in a subshell (( ./gate.sh & )), or with thejob disowned is not recorded. Measured, not estimated.
The earlier wording said "shell-shaped work", and that was false in the
direction that overclaims. Until this branch the detector read the flag
alone, so the incident this PR was built around — a teammate backgrounding a
test sweep with
nohup … &, then thirty minutes with nobody knowing — satoutside the population. The mechanism would not have caught the case that
motivated it. Three people believed otherwise, each having read this paragraph
and taken it for a description of the predicate. It was prose we wrote about
our own code, and we read it back as evidence; the code's own docstring was
accurate throughout.
The instruction to agents stays unconditional — flag every self-started wait.
What is detected is a subset, and conflating the two is what makes an
absent advisory read as an all-clear.
The defects, and one that fired during review
pytest -k start,grep -rn watch,build.py --env dev— silently disabling all three layersWhy 3 was deleted rather than tuned
npm run dev(suppress) andpytest -k start(record) are identical in everyproperty a matcher can see. The over-fire population is drawn from the target
population, so this was a systematic exclusion of the core use case rather than
a tunable miss rate. The error directions are also asymmetric: this is a
detector, not a guard. A guard optimises against false positives; a detector
optimises against misses, because a silent non-detection is the failure it
exists to prevent.
Defect 7 has survived three fixes that each believed themselves complete
The escape — a declared project scope abandoned when its probe misses, so
resolution continues into a different project's file — has been approached
three times:
The first two landed a day apart, so they read as one push closing two routes
rather than as independent attempts. The third, six weeks later, called itself
the root fix.
Each closed a real route. None closed this one. #1617 in particular built a
refusal positioned directly in front of it —
_refuse_ambient_sync_on_project_dir_disagreement— which fires when the twodeclarations are present and differ, and returns "no disagreement" when
either is absent. A disagreement detector doing a containment check's job.
The same PR added the test arm that drives the escape on every suite run. So
the observable damage is three days old even though the route is older: two
records per run into the maintainer's live
CLAUDE.md, against a rollingwindow of three, which means real working memory is evicted rather than
merely joined.
That history is the argument for where this fix sits. Three caller-side
containments did not hold; this one is a predicate at the resolution point,
reused from the one place that already had it right.
Defect 5 fired during this PR's own review, in its shipped form
The lead left a teammate idling for seven hours. The installed version of
the alarm surfaced it — correctly — and said:
In that instance the guess was right. The alarm had no way to know that. The
scan selects only
in_progresstasks, so a task whose completion metadata waswritten is filtered out before the surface is built — the named cause is not
merely unknowable from there, it is unselectable. After this PR the same
event offers three responses and prescribes nothing for the case where the wait
is legitimate.
Nobody constructed that scenario. It is better evidence than a built one.
Two wake mechanisms, both measured here
A watcher that fires while nobody is present tells nobody. Two mechanisms
cover the two halves, and each was measured in a live session rather than read
from documentation:
PushNotificationCronCreateMonitor was measured and excluded. It reaches an idle session but queues
— an event emitted at 08:16:08Z was not delivered until ~08:23Z, on a wake
caused by something else. Same subject, same idle state, five minutes after the
cron arm. The discriminator is wake capability, not availability or reach.
Instructions in the orchestrator surface accordingly: send the push when a
watcher delivers or dies, arm a recurring cron sized to the expected duration,
key it on the deliverable rather than on whether anyone reported, and delete
it once when the wait resolves.
Bounds: one Monitor shape was tested; persistent and unbounded monitors are
unmeasured. The cron result is n=1 per arm.
PushNotification's mobile leg isuntestable from here — an unconnected remote is indistinguishable from a silent
failure — so no wording invites checking it.
The coverage story, which is the substantive finding
The suite could not tell the fixed implementation from the one it replaced.
Breaking a function is not the edit anyone makes. Restoring the simpler earlier
shape because it reads cleaner is. Every case added here is proved against a
revert to prior behaviour rather than against a destroyed function, which is
now the acceptance criterion rather than a courtesy.
A symbol census cannot see this. Five surfaces showed zero test references; one
of them turned out to have nine arms reaching it transitively, and none of the
nine could see the fix being undone. A count above zero is worse than a zero
— a zero sends you to write an arm, a two sends you past the surface.
Collection 16119 → 16208, equal at both scopes. The case count is deliberately
not quoted as a total here: it moved four times during this branch's life, and
a number carried forward from an earlier paragraph is the same defect as a
carried-forward population claim — true when written, asserted afterwards.
What cannot be tested here
Pending on this head: the live probe of layers 3 and 5 below runs next, including tmux teammates (now installed) and the interactive hook payload. This section is updated from its results.
measures the fixture's idea of a frame, not a frame.
against a real registry, but the hook's own firing was not observed: it runs
on
UserPromptSubmit, and a synthetic invocation cannot supply the task listthe selector reads. Unmeasured, not falsified, and stated that way on purpose.
additionalContextdelivery. It is measured, notopen: an allow-path
additionalContextreaches the model together with thetool result, after the call has run.
genuinely probabilistic; procedure and three measurements live in the
docstring, including the one that refuted the author's own fix.
The mechanism was run for real, and that is how the scope error was found
The mechanism was tested in a live session by patching this branch's code
into the installed plugin for eighteen minutes, with the user's authorisation,
then reverting and verifying the restore against two independent sources.
One fix was confirmed that nobody designed the probe to test: the row anchored
to a completed task, because the teammate held no in-progress one. That is
the defect-4 fix working in the field, and it had no field evidence before.
The probe's real product was a refutation. Two arms were run across the
population boundary — a launch carrying the harness flag, and the same work
backgrounded with
nohup … &inside a foreground call. Only the first wasrecorded. The scope paragraph above is written from that measurement rather
than from the predicate's prose, because reading our own prose as evidence is
exactly what produced the earlier error.
Then it was run again, against the committed fix. Same patched-install
procedure, three arms, one on each side of each boundary:
nohup sleep 600 > /dev/null 2>&1 &— foreground, no flag( sleep 600 & )— a documented missArm A is the confirmation: the shape the detector could not see is the shape it
now sees, measured against a baseline taken the same day rather than inferred
from a green suite. Arm C is its matched null, and shows the stated limit is a
measurement rather than a guess.
And a third time, at
3bd3f52c, after the first two review cycles.Same procedure. The teammate was the subject and was barred from reading the
registry or the hooks:
python3 -c "import time; time.sleep(20)" &— foreground, no flagprintArm E uses
python3because Claude Code itself refuses a baresleep 30 &before any hook runs, so that spelling can never reach the detector.
Arm E's record then cleared the way the design says it should. It was still
present with no flag set; the teammate set a wait covering the launch, and after
its next idle the record and the unflagged-idle count were both gone. The
plugin was then restored and verified byte-for-byte against
mainand thechecksummed backups.
A census with a stated boundary
The instrument matched self-declared limits, so it converges on gaps the
authors wrote down and structurally cannot find gaps nobody documented. Two of
the last three items were found by instruments other than the one being run.
This instrument is complete over self-declared limits. Anything beyond needs a
different instrument, scoped as its own work rather than as "finishing the
sweep."
Verification
Full suite from
pact-plugin, no path argument, at the final head8d7063b8:16519 passed, 15 skipped, 0 failed, 0 errors, reconciling exactly against
16534 collected. That count was read three times, all agreeing: before the
run, after it, and from the repository root. The head before the fourth
remediation cycle,
26c141ce, certified at 16263 passed, 85 skipped of 16348,and the earlier heads certified the same way. No path argument, because a path
argument silently drops test files that sit beside the artifact they pin.
This cycle's run was split into six chunks, each run in the foreground.
Claude Code stopped long background jobs under memory pressure, and a stopped
run leaves no failure, only missing output. The chunks' test ids were checked
as sets against the full collection: their union equals it, with no overlap,
nothing missing and nothing extra. Every chunk exited 0.
Certified under an edit freeze, with three checks, because each catches a
failure the others cannot and a green summary line is byte-identical whether
or not any of them failed:
pact-plugin/and the repository root outsidedocs/, caches excluded —nothing the suite reads was rewritten under the run. A changed assertion
leaves the count untouched, so the first two cannot see it. The sweep has to
be that wide: pin tests read agent bodies, protocols and root manifests, so a
sweep of Python files alone would come back empty while a pinned markdown
file changed mid-run.
An earlier run on this branch printed a clean green while twenty-seven tests
went unexecuted, because test files were edited after collection. That run was
struck rather than quoted.
Duration is load-dependent and does not generalise: 490s on a quiet machine,
1048s with three agents working concurrently.
The reason this branch previously gave for launching the gate detached was
false, and it is corrected here because that sentence is the origin of the
idiom the diff removes. The claim was that the run exceeds the 600s Bash
timeout, so it must be launched with
nohup. Measured: a job started withrun_in_background: trueran 750 seconds and exited 0, 150 past the cap.Both shapes escape it, by different routes — the foreground call because it
returns immediately, the flag because the harness does not apply the timeout to
it. So the difference was never duration. It is visibility: the flag
carries a task id, an output file, a completion notice, and the field this
mechanism reads;
nohup … &carries none of them. The gate was launched withthe flag until this cycle, when stopped background jobs forced the foreground
chunks described above.
One thing the flag does not buy for a teammate, measured in the same
run: the completion notice arrives but does not wake an idle teammate — it
rendered only on the next tool call, after an unrelated wake. A lead is
different: its background completion does re-invoke it. That matches the Monitor result above,
so it is two independent mechanisms with the same behaviour rather than a
one-off, and it is why the cron instruction is keyed on the deliverable.
Nor is it a new finding, and it should not be read as one. The same
behaviour was first recorded in #976 — a concurrent auditor armed a
background monitor, idled, and was not woken when it exited — and fixed
there by making the wake a relay. These measurements corroborate it on two
further mechanisms, months apart; they do not discover it.
Review and remediation
Four reviewers, three of them the same agent types that built the branch and so
not independent; the security reviewer had no exposure to this session and is
the independent check. Plus Cursor Bugbot. 5 blocking, 24 minor, 8 future.
Greedy-fix was on, so every minor and future finding was fixed except three
excluded as out of scope or expansive, and the fourth cycle below fixed those
three as well.
The blocking five, each fixed and verified:
launch, and a cleared wait was never discharged. Fixed and pinned through the
real hooks.
submodule, a
--separate-git-dirrepository, a removed worktree, a declaredmain checkout whose CLAUDE.md sits in a worktree). Replaced with one shared
predicate for all three call sites; an inherited
GIT_DIRescape closed alongthe way.
Now teammate frames only.
through
teammate_idle.main().covers_sinceinstruction was the only thing making a fix work, and ittold agents to write a first SET the detector then reported as a defect. The
field is now written on every SET, in every copyable template, and pinned.
Verified by re-running the review's own instruments, not by reading diffs:
each reviewer re-measured its findings at the fixed head; the coverage reviewer
re-ran its 18 surviving mutants and all 18 now fail a test; the security reviewer
built its own 252-cell harness for the guard. Remediation found three new small
issues — a mirrored constant with no parity pin, a docstring that overclaimed a
refusal, a bare assert — fixed in a second cycle and verified the same way.
A third cycle, from Bugbot. Bugbot's review of the certified head flagged a
test docstring that said "These six" writers beside a tuple that now lists four.
Prose that repeats the size of a list next to it goes stale the next time the
list changes. A sweep of every line this PR changes found 38 such counts in 12
files, several already wrong: "both arms" over a class that now has three,
"hosts TWO independent alarms" for a surface that hosts four, and "the two jobs
above" where only one job sits above. Each now names the set instead. The edits
are prose-only, checked by comparing each file's parsed code before and after,
plus one renamed test and one assertion that now derives its count from the set
it checks.
Owner rulings. A removed independent repository nested inside another is admitted
into the enclosing project: it needs deleting the folder the session was
launched from, and nothing the check reads separates it from a removed
subdirectory.
A fourth cycle: every open item fixed, none recorded. The owner ruled that a
recorded gap is still a deferral. So every gap and exclusion the earlier cycles
listed was built in this PR, along with the layer 5 hook above:
lock, through a synced temporary file and a rename, and the five older writers
that wrote in place now use it. The Windows branch was removed, not half kept.
path and must stay inside the root. The earlier
O_NOFOLLOWguarded only thelast path component.
GIT_DIRand its siblings, soa stray variable cannot misroute a resolution into a refusal.
name come from the session context, then a member-checked session registry,
then a
name@teamid; anything unresolved gets no warning.task.
did.
task extends the launch record to that task.
refused outright. Session start writes the worktree's identity into the
session folder. When the declared directory is gone, a record for exactly that
directory decides: a checkout root of the recorded repository is admitted, and
anything else is refused, including when git gives no answer. The memory write
guard and pin archiving share the one reader and agree in all four layouts
measured: two removed-worktree layouts move from refused to admitted, and two
where the record names a different repository move from admitted to refused.
Sessions started before this change have no record and keep the refusal.
sets.
against commits a CI clone does not have, so 434 tests skipped silently there.
The 15 file versions they read are now stored in the repository, each checked
against its git blob id, and all 698 tests run with 0 skipped.
timestamps with the clock and carries the shift into child processes that
tests start with a hand-built environment, and its wrappers now behave like the builtins they replace (they do not bind as methods, and they keep their os.supports_* membership). The full suite was then swept a day back and a day forward, 16,534 tests each
way. Every test passed, no production code depended on the date, and no child
process was left on the real clock.
Found during the cycle, and fixed:
Stophook shipped without thefrom __future__ import annotationslineevery hook carries. Only the whole-suite run checks that, and the targeted
runs did not include it.
mismatch and the script carried on. The commit was reset and remade by path,
and every check now stops the script on failure.
A second silent way to lose a background job, measured this cycle. Claude
Code stopped three background jobs under memory pressure, one within a minute of
launch, while the system still showed about half its memory free. A stopped job
leaves no failure behind, only missing output, so the checks that depended on
those jobs were rerun as foreground calls. A teammate also idled with background
jobs running and no wait flagged, the exact failure this PR addresses, and was
corrected through the wait discipline the PR adds.
Still open, decided by the live probe before merge:
Stopframe carriesagent_type(if not, the lead is identified by session id alone);SubagentStopfires at an in-process teammate's ordinary turn end (if not, that half of layer 5 is removed before merge);PACT:-prefixed agent type reachesSubagentStop(if so, the handoff check strips the prefix);