Skip to content

fix(spawn): validate each sampled pane cwd in the worktree-discovery poll#653

Open
karotkriss wants to merge 2 commits into
kunchenguid:mainfrom
karotkriss:fm/fm-spawn-fix-upstream-u7
Open

fix(spawn): validate each sampled pane cwd in the worktree-discovery poll#653
karotkriss wants to merge 2 commits into
kunchenguid:mainfrom
karotkriss:fm/fm-spawn-fix-upstream-u7

Conversation

@karotkriss

Copy link
Copy Markdown

Intent

Fix a race in bin/fm-spawn.sh's worktree discovery after treehouse 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_root predicate (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_worktree now 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 main as it stands; sequencing and preference relative to #648 are left to the maintainer.

What Changed

  • Add fm_isolated_worktree_root to bin/fm-tangle-lib.sh: a shared predicate that accepts a sampled path only when its physical (pwd -P) form resolves back to itself via git rev-parse --show-toplevel and differs from every forbidden physical path passed to it (the project checkout, the firstmate home/root).
  • Rework bin/fm-spawn.sh's post-treehouse get wait 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 bound FM_SPAWN_WT_TIMEOUT-overridable, and name the last sample in the timeout refusal; validate_spawn_worktree (orca path) delegates to the same predicate.
  • Add 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.
  • Update tests/fm-tangle-guard.test.sh for the new per-sample refusal message and the test-scoped wait bound, and sync docs (AGENTS.md isolation wording, docs/architecture.md, the FM_SPAWN_WT_TIMEOUT entry in docs/configuration.md, the fm-tangle-lib.sh row in docs/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 (real bin/fm-spawn.sh against 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.
  • Full tests/*.test.sh sweep run locally on this branch and on a clean checkout of main: 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-mistakes and 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 clean main.
The pipeline's PR-creation step was skipped by a transient local gh auth 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.

karotkriss and others added 2 commits July 16, 2026 16:51
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant