fix(spawn): record task worktrees from allocator leases - #5
Merged
Merged
Conversation
… a pane probe fm-spawn.sh inferred the worktree by typing `treehouse get` into the fresh pane and polling the backend's current-path probe for the first path that left the project. That inference races treehouse's own allocation: `treehouse get` walks candidate pool slots in order, running git probes with their working directory inside slots it has not chosen, and a backend probe can surface those child locations - herdr's foreground_cwd reports the first foreground-job member whose cwd differs from the shell's while the treehouse process itself still sits in the project directory. A sample taken during that window records a slot treehouse never allocated: a real, distinct worktree that also passes validate_spawn_worktree, which only checked generic properties of the very value the poll had just chosen. The wrong record then receives the harness hooks and vault guard, and teardown either aborts (recorded path not pool-managed) or, worse, runs its unlanded-work refusal against a clean wrong worktree and passes vacuously while the agent's real commits sit elsewhere. Observed live seven times across two repos, both claude and codex harnesses, and ship and scout tasks. In the two 2026-08-06 occurrences both spawns recorded the pool's lowest available slot - permanently wedged by a stale index.lock, so treehouse probes it first and silently skips it on every acquire - while the agents landed in later slots, and the wedged slot physically held the wrong task's hook files. Two prior fixes (physical-path canonicalization, stable window id plus two consecutive agreeing reads) hardened the inference without removing it; a stable foreign value defeats any settle heuristic. Take the worktree from the allocator instead: - `treehouse get --lease --lease-holder fm-<id>` reserves the slot and prints its path as the sole stdout line, mirroring the pattern fm-home-seed.sh already uses for secondmate homes; bootstrap already blocks dispatch on a treehouse without lease support. The durable lease also closes a reuse hole: the old subshell-scoped reservation died with its process, leaving a crashed task's slot reclaimable by a later get while unlanded work sat inside it. - The pane is moved with a plain quoted cd; every backend's probe follows a top-level shell cd (zellij's and cmux's probes never saw the old subshell at all). - The probe is now a confirmation witness, not the source of the record: launch proceeds only once the backend observes the pane at the leased path, one equal read being arrival, so the record and an independent observation must agree; a pane that never arrives fails the spawn loudly and the pre-launch abort path releases the lease. Teardown's existing `treehouse return --force` releases the lease at end of life. The Orca branch already takes its worktree from Orca's own create call and does not share the defect; secondmate spawns never entered this block and are unchanged; the flow is harness-independent. tests/fm-spawn-worktree-settle.test.sh now drives a probe that reports a stale foreign path for enough consecutive reads to defeat any settle heuristic - red before this change (meta recorded the stale path), green now (meta records the leased path) - and proves a persistently wrong probe fails the spawn and releases the lease. tests/lib.sh gains a shared lease-capable fake treehouse, defaulting its lease path to the settled path spawn tests already export.
This was referenced Aug 7, 2026
elixlabssolutions
added a commit
that referenced
this pull request
Aug 7, 2026
…fake treehouse (#7) The crew-unaffected block spawns an ordinary ship task through fm-spawn.sh, which since the allocator-lease change (#5) takes the task worktree from 'treehouse get --lease' before confirming it against the pane probe. That block's fakebin carried no treehouse at all and the test's BASE_PATH is the bare system directories, so the spawn died on a missing allocator before writing its meta record and the crew-unaffected assertion failed deterministically, in CI and locally. The lease change migrated every other spawn-driving test to tests/lib.sh's shared fake treehouse but missed this file. Install the shared fake in make_launch_capturing_tmux, whose stub set exists exactly to serve the crew/scout spawn path. The fake leases FM_FAKE_PANE_PATH, so the recorded worktree and the pane witness agree and the confirmation step is exercised rather than bypassed. Secondmate spawns never consult the allocator and are unaffected.
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.
Intent
Root-cause and fix the long-standing defect where bin/fm-spawn.sh records a worktree= value that is not where the agent actually is (7 live occurrences across two repos, claude and codex harnesses, ship and scout tasks). Root cause, proven by source reading of treehouse v2.0.0 and herdr 0.8.0 plus live pool forensics and a sandbox reproduction: the spawn inferred the worktree by typing 'treehouse get' into the pane and polling the backend's current-path probe for the first path that left the project; treehouse's acquire walks candidate pool slots lowest-first running git probes with cwd inside not-yet-chosen slots, and herdr's foreground_cwd reports the first foreground-job member whose cwd differs from the shell's while the leader (treehouse) still sits in the project - so the poll records a slot treehouse never allocated, typically the pool's lowest available slot (two live slots are permanently wedged by stale index.lock files, so they are probed first and skipped on every acquire). validate_spawn_worktree only checked generic properties of the recorded value, so it could never catch the divergence; hooks and the vault guard were installed into the wrong worktree and teardown's unlanded-work refusal inspected the wrong path. Deliberate decisions in this change: (1) take the worktree from the allocator - 'treehouse get --lease --lease-holder fm-' prints the allocated path as the sole stdout line and holds a durable lease until 'treehouse return' (mirrors fm-home-seed.sh's existing secondmate-home pattern; bootstrap already blocks dispatch on a treehouse without lease support; the durable lease also closes a reuse hole where a crashed task's slot was reclaimable while unlanded work sat inside it); (2) move the pane with a plain quoted cd instead of the treehouse subshell - every backend's probe follows a top-level shell cd, which zellij's and cmux's probes could never do for the subshell; (3) demote the probe to an independent confirmation witness: launch proceeds only once the pane is observed at the leased path, one equal read being arrival, so the record and an independent observation must agree and a pane that never arrives fails the spawn loudly while the pre-launch abort path releases the lease (teardown's existing 'treehouse return --force' releases it at end of life); (4) the old two-consecutive-agreeing-reads settle heuristic is deliberately removed - equality with the known allocation supersedes it and cannot be satisfied by a stale foreign path; (5) new env knobs FM_SPAWN_WORKTREE_CONFIRM_POLLS/INTERVAL exist only so tests can bound the confirmation wait, defaulting to the old 60x1s budget; (6) tests/lib.sh gains a shared lease-capable fake treehouse whose lease path deliberately defaults to FM_FAKE_PANE_PATH so existing spawn tests need only a one-line construction swap, and fm-backend.test.sh gets a bespoke baked-path fake matching its bespoke tmux stubs; (7) tests/fm-spawn-worktree-settle.test.sh's new cases were verified red against the old code (meta recorded the probe's stale path) and green after; (8) tests/fm-tangle-guard.test.sh's window-construction assertions were updated from 'treehouse get typed to the stable window id' to 'the worktree cd and confirmation poll target the stable window id' - same kunchenguid#134 invariant, new flow; (9) backend adapter and doc comments were reworded from worktree-discovery to worktree-confirmation language, keeping each adapter's verified probe facts, and docs/verification/runtime-backends.md annotates that the nested-subshell discovery leg of the old zellij/cmux smoke evidence is superseded by the lease+cd flow rather than pretending those dated smokes were re-run; (10) the Orca branch is untouched because it already takes its worktree from Orca's own create call, and secondmate spawns never entered this block. Explicitly out of scope per the captain: no sweep of historical stale worktree= metadata (recommended separately), and the sibling defects spawn-stale-turnend-hook and turnend-guard-race-s1 are not folded in.
What Changed
Risk Assessment
✅ Low: Captain: The change is tightly bounded and correctly makes the allocator authoritative while requiring backend confirmation before launch.
Testing
Dedicated regression, direct CLI evidence, stable-window targeting, dispatch, trace, busy-adapter, and worktree hook coverage passed. This is a shell lifecycle change with no UI surface; the CLI transcript is the appropriate end-user evidence. Two unrelated constraints remain: a historical send-parity fixture failure before spawn coverage and missing Python tomllib for Kimi tests.
Evidence: Lease-to-metadata end-to-end transcript
Allocator lease, pane confirmation, persisted metadata, and failed-confirmation lease release are shown in one end-to-end CLI transcript.Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
tests/fm-backend.test.sh:704- Historical fm-send parity fixture fails before its spawn section: unchanged base fm-send exits 1 while the current copy exits 0 under its synthetic old-adapter setup. This is deterministic but outside this commit's changed spawn path.bin/fm-kimi-turnend-hook.sh:36- Kimi harness coverage cannot run here because the available Python is 3.9.6 and lacks tomllib. No compatible interpreter is installed, and installation is outside the workspace boundary.bin/fm-test-run.sh tests/fm-spawn-worktree-settle.test.sh tests/fm-backend.test.sh tests/fm-tangle-guard.test.shbin/fm-test-run.sh tests/fm-backend.test.sh/var/folders/_4/q0rlygb56rs6y6xp9sbql5240000gn/T/no-mistakes-evidence/01KZC3K7BCHJSH17XZ7Q6BETKZ/fm-spawn-worktree-lease-e2e.shbin/fm-test-run.sh tests/fm-busy-adapter-wiring.test.sh tests/fm-decision-hold-lifecycle.test.sh tests/fm-gate-refuse.test.sh tests/fm-grok-harness.test.sh tests/fm-kimi-harness.test.sh tests/fm-public-followup.test.sh tests/fm-shared-captain-inheritance.test.sh tests/fm-spawn-dispatch-profile.test.sh tests/fm-trace-context-spawn.test.sh tests/fm-vault-guard.test.shbin/fm-test-run.sh tests/fm-decision-hold-lifecycle.test.shbin/fm-test-run.sh tests/fm-vault-guard.test.shpython3 -c 'import tomllib'✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Findings beyond this change (from the investigation)
worktree=records match pool truth (two were hand-corrected during the incident). But: firstmate pool slot 6 is in-use by codex processes that no live task record claims - consistent with past misrecord damage where teardown returned the wrong slot and stranded the real one; firstmate pool slots 1 and 3 are permanently wedged by staleindex.lockfiles (Jul 16 / Jul 20) in their private git dirs, so every acquire probes and skips them - clearing the locks unwedges them; xlabs-os pool slot 1 is dirty and may hold unlanded work - inspect before any cleanup.spawn-stale-turnend-hook: hook files survive slot reuse because fm-spawn lists them in.git/info/exclude, which also shields them from treehouse'sgit clean -fdat return time. A claude-task-then-codex-task reuse therefore leaves the prior claude Stop hook firing for a retired task id - matching that item's symptom exactly. This fix ensures hooks land in the correct worktree, but does not remove them at end of life.index.locksilently and permanently wedges a pool slot -statusreports it "available", everygetprobes and skips it, and nothing ever surfaces the wedge. Worth an upstream issue (repro: touchindex.lockin a slot's git dir).main, and PR merge: bring the fork up to date with upstream (150 commits), preserving both fork commits #4 having triggered the same workflows 13h earlier; no branch protection requires them. Compensating local evidence on the exact PR head:bin/fm-lint.sh(the pinned-ShellCheck CI lint) passes,bin/fm-test-run.sh --check-coverage(CI's coverage guard) passes, and the no-mistakes test step ran the spawn-path regression suites green.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.