Repository navigation
fix(spawn): require the resolved task worktree to belong to the project - #3
Open
jonathan-edgar wants to merge 8 commits into
Open
jonathan-edgar wants to merge 8 commits into
jonathan-edgar wants to merge 8 commits into
Conversation
jonathan-edgar
force-pushed
the
main
branch
from
September 22, 2026 14:37
0b9406d to
6f0f139
Compare
jonathan-edgar
pushed a commit
that referenced
this pull request
Sep 22, 2026
…kunchenguid#3578) * fix(bin): let verified harness ancestry outrank retained markers (#3) * fix(bin): let a structural harness ancestor outrank a retained marker bin/fm-harness.sh treated a verified environment marker as unconditionally authoritative, so a Codex session started from an environment that had retained CLAUDECODE=1 detected as claude. Session start then emitted Claude's Stop-owned supervision protocol to a Codex primary, and every turn end was blocked for missing Claude recovery. The defect is the precedence boundary, not any one harness. codex, opencode, kimi, and muse publish no identity marker at all, so with markers winning outright any retained CLAUDECODE renamed them; the Cursor-before-Claude ordering was a point patch on the same class of problem, and the launch-time marker clearing only ever covered sessions fm-spawn started. Markers and ancestry are now separate evidence layers that detect_own arbitrates: - no ancestry match, or no marker: the single available layer answers, unchanged; - same harness family: the marker's finer verdict stands, so a launch-selected pi-signed is not flattened to pi by an ancestry walk that can only see the shared launcher name; - different harness with a structural (command-name) ancestor: ancestry wins, because only ancestry proves who owns the process tree; - different harness with only a bare-interpreter script-path match: the marker wins, since a harness-shaped path in some node process's arguments is weaker evidence than a harness publishing its own identity. The correction is symmetric: a retained CURSOR_AGENT no longer renames a claude worker nested under cursor either. Adds fm-harness.sh ancestry [<pid>], ancestry evidence with no marker layer, so a real harness process can be asked what the walk makes of it. tests/fm-harness-precedence.test.sh is the portable regression, built from real renamed processes with no harness installed. Every case drives the two layers apart and asserts each alone as well as the combination, so no case can pass vacuously; it also pins Codex's real two-process install topology, since the fix depends on the native binary being what a tool subprocess meets first. The opt-in drift guard gains the matching live half: each installed harness's real running process must still be identified by the ancestry walk, and it fails naming the harness and version when a release changes that name. Documentation follows the corrected contract in the script header, the harness-adapters detection section, the codex, opencode, kimi, and cursor references, and a dated verification record. * fix(tests): drop the unused argument pass-through in the shim-topology helper bin/fm-lint.sh refused the branch: run_shim declared a `[ancestry]` argument and forwarded "$@", but every call site that varies the environment or passes the ancestry subcommand invokes the shim entry point directly, so the helper is only ever called with no arguments (ShellCheck SC2120/SC2119). Behavior is unchanged: with no arguments "$@" expanded to nothing. * fix(bin): examine the top of the process chain instead of assuming init harness_ancestry stopped as soon as the next pid was 1, on the assumption that pid 1 is always init and can never be a harness. Inside a PID namespace that assumption inverts: the harness itself is pid 1, so the walk never examined the one process that proves who owns the tree, reported no ancestry at all, and handed the verdict straight back to a retained marker. A real Codex session under `codex sandbox`, holding CLAUDECODE=1 and CLAUDE_CODE_ENTRYPOINT=cli, is exactly that shape: it resolved claude and rendered Claude's Stop-owned supervision protocol even with the marker-vs-ancestry precedence boundary in place. The same probe now resolves codex and renders the Codex foreground checkpoint. A host's real pid 1 (init, systemd, launchd) matches no harness name, so examining it costs one ps call and can introduce no false positive; the walk still stops once that top process has been read, and a non-numeric or zero ppid still ends it. tests/fm-harness-precedence.test.sh pins the namespace shape with a fake ps that reports every process as bash with ppid 1 and pid 1 as the harness. The case asserts the marker still answers alone when pid 1 is host-shaped, so it cannot pass vacuously, and it fails against the previous stop condition. * docs(verification): record the real-Codex retained-marker evidence The existing record proved the precedence boundary with the portable regression and recorded each installed harness's process name behind the ancestry walk, but it had no evidence from a real Codex process actually holding a retained Claude marker, which is the failure the boundary exists for. Adds the dated before/after result from codex-cli 0.152.0 under `codex sandbox`, with the exact command and the decisive verdict and rendered protocol on each side, and records the second boundary that shape exposed: the walk must examine the top of the process chain, because inside a PID namespace the harness is pid 1. Refreshes the portable regression's observed output for the case it gained. * no-mistakes(review): blind ancestry in marker-pinned harness tests * no-mistakes(review): blind ancestry in the Pi guard-routing test * no-mistakes(review): classify precedence suite, dedupe ps stub, soften claims * no-mistakes(review): model the spawn-and-wait Codex shim topology * no-mistakes(document): correct stale muse marker-clearing detection claims * no-mistakes: apply CI fixes * fix(bin): examine the top of the chain in the lock and nudge walks too The pid-1 defect corrected in bin/fm-harness.sh survived unchanged in the two other harness-ancestry walks, on the exact topology the branch verified against a real Codex process. bin/fm-session-lock-lib.sh's fm_harness_ancestry_pids stopped as soon as the next pid was 1, so a firstmate whose harness is pid 1 of its own PID namespace could not find that harness at all and did not recognize its own session lock. bin/fm-sessionstart-nudge.sh carried the same stop plus a blanket rejection of a lock pid of 1, so the same session was told to run session start again on every turn. Both walks now compare the top process before stopping, matching the shape used in bin/fm-harness.sh. For the lock walk this is safe because fm_harness_process_matches rejects a host's real pid 1. For the nudge, `kill -0` still gates the lock pid, and on a host an unprivileged `kill -0 1` fails, so a lock file that wrongly names pid 1 leaves the hook silent rather than acting on init. Each walk gains one regression case. The lock case drives a deterministic process table whose pid 1 is the harness and asserts a host-shaped pid 1 still finds nothing, so it cannot pass vacuously. The nudge case needs a real PID namespace, because the builtin `kill -0` gate cannot be reached through a fake ps, and it first proves the same fixture nudges with no lock present; it skips explicitly where unprivileged namespaces are unavailable. * no-mistakes(review): assert comm-strength detection from subprocess vantage in drift guard * fix(bin): verify the live harness guard at the strength the guarantee needs The marker-versus-ancestry boundary this branch ships is a strength claim: detect_own hands an args-strength verdict straight back to a retained foreign marker, so a harness is only protected where the ancestry walk reaches it at comm strength. The installed-harness drift guard probed the pane process alone. Under an interpreter shim the pane process IS the shim, whose own script path is args strength, while the native binary that carries comm strength is its child. The guard therefore observed args for Codex, passed, and would have kept passing if a release stopped spawning that native child at all, while real sessions silently regressed to the original bug. fm-harness.sh gains `ancestry-subtree`, which asks the walk from the pane process and every descendant of it, the vantage a tool subprocess actually occupies. The guard now requires comm strength somewhere in that set and requires every vantage to name the same harness. This supersedes the preceding commit's in-guard leaf walk, which reached the same vantage but left the logic inside the test file, where CI could not pin it and nothing else could reuse it. A harness-dependent check needs both halves: `tests/fm-harness-precedence.test.sh` now carries a portable case proving the subtree probe reaches a strength the top-of-session probe cannot, mutation checked twice, once against the pre-change script and once by disabling descendant enumeration. The subtree walk also avoids depending on tty and process-group semantics that differ between Linux and macOS. Verified live: codex-cli 0.152.0 reports [args codex;comm codex] and Claude Code 2.1.257 reports [comm claude]. * no-mistakes(review): narrow drift guard to the upward vantage path * no-mistakes(review): judge only comm-strength vantages in drift guard * no-mistakes(document): drop duplicated rationale in detection precedence evidence * no-mistakes(review): fix pid-1 nudge case vacuity and descent no-arg expansion * no-mistakes(document): drop branch-relative phrasing in detection precedence evidence * no-mistakes(review): guard remaining empty positional expansions in fm-harness * no-mistakes(document): scope cursor marker-ordering claim to the marker layer * no-mistakes(review): Prefer comm-strength leaves in equal-depth descent ties * no-mistakes(document): Document comm-strength descent tie-break --------- * no-mistakes(review): Blind ancestry in stale gemini/rovo marker-precedence tests * no-mistakes(document): Add missing equal-depth-tie test line to precedence evidence transcript * no-mistakes(review): Fix stale/vacuous agy precedence test, add agy to precedence suite and docs * no-mistakes(document): Fix stale kimi.md marker doc missed by ancestry-precedence fix --------- Co-authored-by: NewAiCoder <170579485+NewAiCoder@users.noreply.github.com>
Sums per-message token usage from claude's own session transcripts (the main agent jsonl plus each subagents/*.jsonl) so a finished crewmate or scout's exact token cost can be reported, instead of eyeballing pane counters that undercount. - bin/fm-token-usage.sh: task-id or --cwd mode, session scoping (newest / --session / --since / --all), --json output, claude-harness only. - tests/fm-token-usage.test.sh: hermetic coverage incl. a regression for the jq accumulator reset on interleaved non-usage lines. - docs/token-usage.md: data-source layout and verification evidence. - AGENTS.md: one-line pointer at the completion-reporting step.
…down On a ship teardown, copy every untracked, non-ignored file the crew left in its worktree into the project's primary checkout at the same relative path (never overwriting an existing file), then remove it from the worktree. treehouse return hard-resets the worktree, so this preserves generated notes/docs/scratch that would otherwise be lost, and leaves the tree clean so leftover untracked files no longer refuse teardown. Purely additive, no git-state change; committed-but- unlanded work is untouched and still refuses. Scout (scratch worktree) and secondmate teardowns are exempt. - bin/fm-teardown.sh: harvest_untracked_into_project() + call before the safety check; header updated. - tests/fm-teardown.test.sh: copy-into-project, no-clobber, scout-exemption cases. - AGENTS.md: seventh sanctioned write exception (section 1) + teardown note (section 7).
The post-`treehouse get` worktree poll accepted the first pane cwd that differed from the project directory, and `validate_spawn_worktree` only asked whether that path was a git repo whose root is itself and is not the primary checkout. Both tests are too weak. oh-my-zsh.sh runs `builtin cd -q "$ZSH"` on every shell startup to stamp the zcompdump revision, so a freshly spawned pane transiently reports ~/.oh-my-zsh as its foreground cwd. The poll latched onto it and the guard passed it, because ~/.oh-my-zsh is a git repo and is not the primary. Five agents launched into the user's shell framework directory with --dangerously-skip-permissions before it was diagnosed. The property that actually identifies a task worktree is a shared --git-common-dir: every linked worktree of the project reports the project's, an unrelated repo reports its own. Enforce it in the poll, so a transient impostor cwd is waited out rather than accepted, and in validate_spawn_worktree as a backstop for the Orca path, which sets the worktree without polling. The predicate fails open when the project's common dir cannot be read, so a git failure cannot make a spawn unlaunchable. Also bound the poll with FM_SPAWN_WORKTREE_TIMEOUT (default 60, unchanged behaviour) and name the last cwd seen in the give-up error, so the next impostor is visible in one look instead of five launches.
…rd Orca assumption
…-timeout doc claims
jonathan-edgar
force-pushed
the
fm/spawnfix-w2
branch
from
September 22, 2026 15:07
6aa2bcd to
536dd39
Compare
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
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.
What this fixes
fm-spawn.shdiscovers a task's worktree by sendingtreehouse getto the new pane, then polling the pane's foreground cwd. It accepted the first path that merely differed from the project directory.oh-my-zsh.shrunsbuiltin cd -q "$ZSH"on every shell startup to stamp the zcompdump revision. It is unconditional. So a freshly spawned pane transiently reports~/.oh-my-zsh, the poll accepted it, and the agent launched in the user's shell framework directory with--dangerously-skip-permissions. This happened five times before it was diagnosed.validate_spawn_worktreedid not catch it either, because it asked the wrong question.What the guard used to test, and what it now tests
--git-common-dir?~/.oh-my-zshsatisfies the old rule completely: it is a git repo, its root is itself, and it is not the primary checkout. The old rule tested isolation from the primary when what it needed to test was identity with the project.Every linked worktree of the project reports the project's
--git-common-dir; an unrelated repository reports its own. That is the property that actually identifies a task worktree.Enforced in two places:
validate_spawn_worktree- a backstop that refuses the launch. This is the only enforcement on the Orca path, which has no poll.The predicate fails open when the project's common dir cannot be read, so a git failure cannot make a spawn unlaunchable. That case now warns on stderr naming the cause rather than degrading silently.
Also
FM_SPAWN_WORKTREE_TIMEOUT(default 60, unchanged behaviour) bounds the poll; the give-up error names the last cwd seen, which would have shown~/.oh-my-zshon the first bad launch rather than the fifth.docs/orca-backend.mdrecords as a known unknown that real Orca's worktree shape is not smoke-proven, with the exact probe to settle it.Tests
Extended
tests/fm-tangle-guard.test.shandtests/fm-backend-orca.test.shrather than adding a runner. Covered: a genuine worktree accepted; a different repository rejected; a non-git path rejected; the primary checkout still rejected by the isolation rule; the fail-open branch; and the live shape where the pane sits in the foreign repo before landing in the real worktree.Each new case was proven to fail against the unfixed script - unfixed, the foreign-repo spawn exits 0 and launches there.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-spawn.sh:681- The common-dir guard fails open silently. Whengit rev-parse --path-format=absolute --git-common-diron the project returns nothing (git older than 2.31, where--path-formatis unsupported, or any other rev-parse failure),PROJ_GIT_COMMONis empty andis_project_worktreereturns success for every candidate at bin/fm-spawn.sh:681, so both the discovery poll and thevalidate_spawn_worktreebackstop revert to the exact pre-fix rule that launched five agents into ~/.oh-my-zsh with --dangerously-skip-permissions. The fail-open itself is the author's deliberate, stated choice and should stay, but nothing anywhere - not stderr, not the spawn summary line, not the recorded meta - reveals that the guard is inert, and the crewmate brief's own isolation assertion (bin/fm-brief.sh:237) does not catch a foreign repo either, since the agent's toplevel does equal the directory it was launched in. Emit one stderr notice whenPROJ_GIT_COMMONis empty (still launching), so an operator on an old git sees the guard is off instead of rediscovering it through another bad launch.bin/fm-spawn.sh:861-FM_SPAWN_WORKTREE_TIMEOUTis documented only in the script header, but docs/configuration.md keeps an otherwise exhaustive tunables reference where every comparable knob is listed with a default and a one-line purpose (FM_CHECK_TIMEOUT,FM_CREW_STATE_NM_TIMEOUT,FM_ARM_CONFIRM_TIMEOUT,FM_FLEET_SYNC_BOOTSTRAP_TIMEOUTat docs/configuration.md:242-263). Since the intent describes this as an operator-facing knob for shortening the wait while debugging a spawn, an operator scanning that list will not find it. One line in that block would close the gap; this is additive to the existing list, not a new doc or a second owner of the contract.🔧 Fix: report disabled worktree-identity check; document spawn timeout
3 infos still open:
bin/fm-spawn.sh:668- The new spawn-time warning names only one cause.PROJ_GIT_COMMONcomes back empty for ANY failure ofgit -C "$PROJ_ABS" rev-parse --path-format=absolute --git-common-dir, not just an old git: I verified a non-git project directory produces the same empty result (fatal: not a git repository), and git'ssafe.directorydubious-ownership refusal does too. In those cases the operator is told "needs git 2.31+ for rev-parse --path-format", which points at the wrong thing while the identity guard is silently off. Since the message exists purely for diagnosis - the incident's whole cost was five bad launches before anyone knew where to look - broaden it to state the observed fact and list the plausible causes, e.g. "...could not determine the git common dir of <path> (the directory may not be a git repo, git may have refused it, or git may predate 2.31's rev-parse --path-format)". No behavior change; the fail-open stays exactly as the intent requires.bin/fm-spawn.sh:667- The fail-open branch is the one branch of the new guard with no test. The change covers a genuine worktree (accepted), a different repository (rejected), a non-git path (rejected), the primary-checkout subdir (rejected by the isolation rule), and the transient-then-real sequence - but not the case wherePROJ_GIT_COMMONis empty, which is precisely the state in which the original oh-my-zsh incident becomes reachable again. It is hermetically testable with the fixtures already in tests/fm-tangle-guard.test.sh: shadowgiton the fakebin PATH with a stub that failsrev-parse --path-format(or handrun_spawna project directory that is not a git repo), point FM_FAKE_PANE_PATH at the foreign repo, and assert both halves of the deliberate contract - the warning text reaches stderr AND the spawn still proceeds (exit 0, meta recorded) rather than aborting. That pins the fail-open as intended behavior so a later hardening pass cannot silently flip it, and pins the warning so it cannot be dropped.bin/fm-spawn.sh:723- The new backstop adds a precondition to the Orca path that is not covered by any recorded Orca smoke evidence.validate_spawn_worktreenow requires the Orca-provided path to share the project's--git-common-dir, and forbackend=orcathis predicate is the only enforcement (there is no poll). That holds iforca worktree create --repo id:<repo>produces a linked git worktree of the registered repo; if Orca instead provisions from its own clone or mirror, every Orca spawn would now refuse with "NOT a worktree of". docs/orca-backend.md's verified-facts section records the smoke-proven shapes for repo registration, worktree creation, and terminal handles, but says nothing about the worktree's git identity relative to the registered repo, and Orca is not installed here so I could not check it. The test that covers this asserts the refusal, not the acceptance, so it would pass either way. Worth onegit -C <orca worktree> rev-parse --path-format=absolute --git-common-diragainst a real Orca worktree, with the result added to that doc's verified list - Orca is experimental, so this is not merge-blocking.🔧 Fix: broaden common-dir warning, test fail-open, record Orca assumption
1 info still open:
docs/orca-backend.md:112- The new known-unknown record ends with a claim its own file contradicts three lines later. Line 112 states "The existing fake-Orca test asserts the refusal rather than the acceptance, so it passes either way", buttest_spawn_writes_orca_metadata_and_launches_harness(tests/fm-backend-orca.test.sh:531) builds a genuine linked worktree viafm_git_worktree "$proj" "$wt"- which shares the project's--git-common-dir- feeds that path as Orca'sresult.worktree.path, and asserts exit 0 plusworktree=$wtin meta. That is the acceptance path of exactly this check, and the doc's own "Fake-Orca tests cover" list at line 118 names it ("fm-spawn.sh --backend orcametadata creation and harness launch"). The genuine gap is different and narrower: every fake-Orca worktree is hand-built withgit worktree add, so the suite proves the predicate accepts a linked worktree but cannot prove real Orca provisions one. Since this paragraph exists specifically to be accurate about what is and is not proven, the closing sentence should say that instead - e.g. "The fake-Orca tests feed a hand-built linked worktree, so they pin the predicate's accept and refuse behaviour but cannot confirm the shape real Orca provisions." This sentence was prescribed verbatim in the round-2 instructions (carried over from my own round-2 wording, which was itself inaccurate), so it is flagged rather than corrected silently. No code change; the recorded probe, the fallback plan, and the guard itself are all correct.🔧 Fix: correct Orca doc claim about fake-Orca test coverage
1 info still open:
bin/fm-spawn.sh:726- The identity probegit -C <dir> rev-parse --path-format=absolute --git-common-diris now written out three times in this file (line 660 for the project, line 694 insideis_project_worktree, and line 726 inside the refusal message), and the third is a second, independent read of the same candidate rather than a reuse of what the predicate just resolved. Nothing is functionally wrong today - both reads are the same command against the same path - but the refusal message prints a value it re-derived instead of the value the decision was actually made on, and three literal copies of a fiddly flag string can drift apart silently (a future edit to the flags in one place would leave the guard checking one thing and reporting another). Simplest fix: haveis_project_worktreestash the candidate's resolved common dir in a script-level variable (e.g.CANDIDATE_GIT_COMMON) and print that in the error, collapsing three invocations to two. Non-functional, no behaviour change, and it leaves the fail-open and the warning exactly as the intent requires.command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: no code change; tmux precondition met, targeted tests green
1 warning still open:
docs/orca-backend.md:107- The new identity check fails closed on the Orca backend path, but there is no evidence that real Orca'sorca worktree create --repo id:<repo>yields a linked worktree sharing the project's --git-common-dir. The fake-Orca test pins the predicate's accept/refuse behaviour but cannot confirm the shape real Orca provisions, and no Orca runtime is available here to probe it. If Orca provisions from its own clone or mirror, every real Orca spawn would now be refused at validate_spawn_worktree. This is already recorded in docs/orca-backend.md as a known unknown; it is surfaced here only because it is the one acceptance criterion I could not produce evidence for. Settling it needs one probe on a real Orca worktree:git -C <orca worktree> rev-parse --path-format=absolute --git-common-dircompared against the same command in the project directory.command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-tangle-guard.test.sh- 7 cases pass, including the newtest_spawn_rejects_foreign_repo_cwdandtest_spawn_fail_open_when_common_dir_unknownbash tests/fm-backend-orca.test.sh- 51 cases pass, including the newtest_spawn_refuses_orca_foreign_repo_worktreeFail-before-fix: copiedbin/+tests/to a scratch tree, replacedbin/fm-spawn.shwithgit show 0b9406d:bin/fm-spawn.sh, and ran both suites there - the foreign-repo spawn exits 0 (launches into the impostor) and the Orca backstop case gets past validation into the foreign repoManual end-to-end reprorepro-ohmyzsh-spawn.sh: fake tmux replays pane cwd~/.oh-my-zsh,~/.oh-my-zsh,<real worktree>; compared recordedworktree=instate/<id>.metabefore vs after the fixManual end-to-end reprorepro-ohmyzsh-stuck.sh: pane never leaves the impostor; asserted exit 1, no meta recorded, and the give-up error naming the last pane cwdManual reprorepro-failopen.sh: shimmedgitto rejectrev-parse --path-format(pre-2.31 behaviour) and confirmed the spawn still launches while warning that the worktree-identity check is disabledFlakiness check:bash tests/fm-tangle-guard.test.shrun 3x consecutively, all exit 0git status --porcelain- worktree left clean, all evidence written outside it🔧 **Document** - 1 issue found → auto-fixed ✅
docs/zellij-backend.md:209- docs/zellij-backend.md:209 is a past-tense E2E verification record stating that a pane which never left the project directory refused to launch with "did not yield an isolated worktree" via "the 60-second poll's own comparison". After this change that same shape no longer reaches validate_spawn_worktree: the discovery poll rejects the unmoved cwd and gives up with "did not enter a worktree of <project> within Ns" instead. I deliberately left it unchanged because it is a dated record of what actually happened during that run, not a statement of current behaviour, and rewriting verification history is worse than the residual hazard. The risk if left: a reader debugging a similar symptom looks for the isolation error and does not find it. If you would rather it not mislead, the minimal fix is a trailing clause noting which guard reports this case today.🔧 Fix: note current guard for superseded zellij E2E record
✅ Re-checked - no issues remain.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.