fix(spawn): validate each sampled pane cwd in the worktree-discovery poll#653
Open
karotkriss wants to merge 2 commits into
Open
fix(spawn): validate each sampled pane cwd in the worktree-discovery poll#653karotkriss wants to merge 2 commits into
karotkriss wants to merge 2 commits into
Conversation
…poll The worktree-discovery poll after treehouse get accepted the first sampled cwd that differed from the project checkout. treehouse setup hops through intermediate directories (the user's home, the firstmate home) before settling, so a mid-hop sample recorded a wrong worktree= in task meta and installed the crew's turn-end hook into the wrong checkout - when that checkout is the firstmate home/root, the primary session fires the crew's turn-ended marker on every turn, producing phantom watcher wakes that survive teardown. Accept a candidate only once the shared fm_isolated_worktree_root predicate (bin/fm-tangle-lib.sh) proves it is physically at a git worktree top, distinct from both the project checkout and the firstmate home/root; a mis-sample can itself be another valid repo root, so not-the-project is not enough. validate_spawn_worktree now delegates to the same predicate so one definition of isolated-worktree exists. The poll timeout is test-overridable via FM_SPAWN_WT_TIMEOUT and the refusal names the last sampled path. Regression tests replay the observed hop sequence through the poll's accept logic and pin the predicate against every observed mis-sample shape.
… docs/architecture.md, docs/configuration.md, docs/scripts.md) were verified accurate; fixed one remaining stale restatement in AGENTS.md section 8 that still said the spawn guard only protects against the primary checkout.
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.
Intent
Fix a race in
bin/fm-spawn.sh's worktree discovery aftertreehouse get.The discovery poll accepted the first sampled pane cwd that differed from the project checkout, but treehouse's setup hops through intermediate directories (the user's home directory, the firstmate operational home) before settling in the pooled worktree.
A sample landing mid-hop records a wrong
worktree=in the task metadata and installs the crew's turn-end (Stop) hook into the wrong checkout.When that checkout is the primary firstmate home/root, the primary session fires the crew's turn-ended marker on every turn, producing phantom watcher wakes that survive teardown (same failure family as #608).
The fix accepts a candidate only once the shared
fm_isolated_worktree_rootpredicate (bin/fm-tangle-lib.sh) proves it is physically AT a git worktree top and distinct from BOTH the project's primary checkout AND the firstmate home/root.A mid-hop sample can itself be another valid repo root, so not-the-project alone is not enough.
validate_spawn_worktreenow delegates to the same predicate so there is exactly one definition of a genuine isolated worktree; its orca-path call keeps the existing project-only exclusion (behavior-preserving, since orca worktrees come from orca's own API rather than a cwd poll).The poll bound is test-overridable via
FM_SPAWN_WT_TIMEOUT(production keeps 60s) and the timeout refusal names the last sampled path for diagnosis.Relation to open PR #648: it introduces the same worktree-top validation for the discovery poll, but its predicate excludes only the project checkout, so the firstmate home/root - itself a valid repo root that is its own toplevel - still passes it.
This change additionally rejects the firstmate home/root, which is the observed production failure, and carries regression tests for the hop sequence.
Built against
mainas it stands; sequencing and preference relative to #648 are left to the maintainer.What Changed
fm_isolated_worktree_roottobin/fm-tangle-lib.sh: a shared predicate that accepts a sampled path only when its physical (pwd -P) form resolves back to itself viagit rev-parse --show-topleveland differs from every forbidden physical path passed to it (the project checkout, the firstmate home/root).bin/fm-spawn.sh's post-treehouse getwait loop to validate every sample with that predicate instead of accepting the first cwd that differs from the project, remove the now-redundant post-loop treehouse validation call, make the wait boundFM_SPAWN_WT_TIMEOUT-overridable, and name the last sample in the timeout refusal;validate_spawn_worktree(orca path) delegates to the same predicate.tests/fm-spawn-worktree-wait.test.sh: predicate acceptance/rejection cases (non-repo dir, nonexistent path, project clone, symlink to the project, firstmate home as a valid-but-wrong repo root, worktree subdir, genuine worktree top), a replay of the observed hop sequence (project -> user home -> firstmate home -> worktree) that must settle only on the last, and structural pins that the poll is wired to the predicate.tests/fm-tangle-guard.test.shfor the new per-sample refusal message and the test-scoped wait bound, and sync docs (AGENTS.mdisolation wording,docs/architecture.md, theFM_SPAWN_WT_TIMEOUTentry indocs/configuration.md, thefm-tangle-lib.shrow indocs/scripts.md).Risk Assessment
✅ Low: the poll's accept condition is strictly narrower than before (a genuine isolated worktree top instead of any non-project cwd), the timeout and refusal paths are preserved, and the behavior is pinned by new regression tests replaying the observed failure sequence.
Testing
bash tests/fm-spawn-worktree-wait.test.sh- green (predicate cases, hop-sequence replay, structural pins).bash tests/fm-tangle-guard.test.sh- green (realbin/fm-spawn.shagainst a fake tmux/treehouse: aborts on non-worktree and primary-checkout panes, succeeds on a genuine worktree).bin/fm-lint.sh- green under the pinned ShellCheck 0.11.0.tests/*.test.shsweep run locally on this branch and on a clean checkout ofmain: the only failures are identical on both (local-environment gaps in herdr lab e2e preparation, an orca mkdir message-wording mismatch, and a quota-axi dispatch probe), none related to this change.Pipeline
Updates from git push no-mistakes
This branch was pushed through
git push no-mistakesand validated by the pipeline (run outcome: passed): intent, rebase, review, document, lint, and push completed; the review round surfaced no findings on the final head; the local test step was skipped in favor of the direct suite runs above because the only local failures are the environmental ones that reproduce identically on cleanmain.The pipeline's PR-creation step was skipped by a transient local
ghauth check failure, so this PR was opened directly from the pipeline-validated head (90300f9, the exact commit the pipeline pushed to the fork); CI on this PR runs the portable suite as usual.