Skip to content

fix(spawn): refuse a spawn the worktree pool cannot serve, naming the cause - #4512

Open
KakkoiDev wants to merge 4 commits into
kunchenguid:mainfrom
KakkoiDev:fm/spawn-never-blocks
Open

KakkoiDev wants to merge 4 commits into
kunchenguid:mainfrom
KakkoiDev:fm/spawn-never-blocks

Conversation

@KakkoiDev

Copy link
Copy Markdown

Summary

A crewmate spawn used to type treehouse get into the task pane and then watch the pane's working directory for sixty seconds. When the treehouse pool had nothing to hand out, the only thing the captain saw was a timeout naming the directory the shell was still sitting in. The real answer, which treehouse itself prints in one line ("all 16 worktrees are in use or dirty"), never reached bin/fm-spawn.sh. One such outage cost six failed launches and several wrong repairs, including a duplicate max_trees key that made treehouse refuse its own config.

This branch adds bin/fm-treehouse-pool-lib.sh, which reads treehouse status --json before any endpoint, worktree, or record is created for the task, and refuses the spawn on the two causes it can prove: a pool at its max_trees cap with every slot in use or dirty, and a treehouse.toml that treehouse will not load. The refusal carries the slot breakdown, and for each blocked slot either the count of unlanded commits (left alone, no clearing command offered) or the exact commands that first preserve and then clear its leftovers, plus the exact line to edit to raise the cap. Nothing is cleared automatically. Inside the pane, the treehouse get command now prints an exit-status marker on failure, so a get that fails outright is reported immediately with the pool diagnosis or treehouse's own words, instead of being waited out. An inspection that cannot settle the question launches exactly as before, with the timeout as backstop.

✅ What this PR does

  • Preflights the treehouse pool under the allocation lock and refuses a spawn whose pool is full or whose treehouse.toml will not load, before anything for the task exists (bin/fm-spawn.sh#L2389-L2404).
  • Names the remedy per blocked slot: unlanded commits are reported and left untouched; leftover-only slots get a preserve-then-treehouse return --force command pair (bin/fm-treehouse-pool-lib.sh).
  • Tells the captain how to raise the cap safely, pointing at the existing max_trees line and warning that a second key breaks the file, which is the failure mode hit during the incident.
  • Reports a failing treehouse get at once with its exit status, by having the pane print a marker the spawn polls for during the wait, and falls back to the pane's last lines when the pool inspection is inconclusive.
  • Appends the pool diagnosis to the old timeout message so even the backstop path now says why.
  • Bounds the pool read with FM_TREEHOUSE_POOL_TIMEOUT (default 45s) so a slow or wedged repository cannot turn the preflight into a new hang; a timed-out read is treated as unknown and the spawn proceeds.
  • Makes the pane wait configurable via FM_SPAWN_WORKTREE_WAIT (default 60s, positive integer enforced).
  • Reads max_trees through a symlinked treehouse.toml, so an alternate config location does not make the preflight invent or miss a full-pool refusal.
  • Documents the new library in docs/scripts.md and points spawn regressions at the new test in docs/architecture.md.

⚠️ Not in this PR, needs attention later

  • Blocked slots are still cleared by hand. The report prints the commands but never runs them, so a fully blocked pool still stops every spawn for that project until the captain acts. The intent asked to never be blocked again; this PR makes the block instant and explained, not impossible.
  • TREEHOUSE_DIR still does not move the pool root, and treehouse.toml is still read only from the repository root. Both are treehouse behaviours the report now states, but they remain traps for anyone repairing by hand.
  • Worst-case spawn latency on a healthy but slow repository grows by up to the pool read bound (45s) before the ordinary 60s wait, because an inconclusive inspection is not a refusal.
  • max_trees detection depends on treehouse's default of 16 being unchanged upstream; a drift there can delay a refusal, never create one.

🧪 Regression evidence

tests/fm-spawn-pool-preflight.test.sh (permalink) runs against a fake treehouse and a real two-slot pool of worktrees. All eight cases pass on this branch:

Case Pins
full pool refuses before any task state or metadata exists, names counts, cap, and per-slot commands
unlanded slot reported as work, no clearing command offered
unloadable treehouse.toml refuses naming file, line, and treehouse's own diagnosis
failing treehouse get in pane reported at once with exit status, in treehouse's words
hanging pane still fails at the wait deadline (backstop preserved)
pool read never returns bounded, spawn falls back to the ordinary wait
symlinked lower cap full-pool refusal still fires
symlinked higher cap no invented refusal

The tests were run in this worktree only. No live spawn against the original 16-slot pool was reproduced here.

🤖 Generated with Claude Code

Risk Assessment

✅ Low: The fix round is a one-token predicate change that makes the cap read follow symlinks exactly as treehouse does, and both added regression tests exercise the behaviour through a real spawn in both directions and would fail if the -L test were restored.

Testing

Ran the targeted preflight test file (8 cases, ~30s) which drives the real fm-spawn.sh with a fake tmux and a fake treehouse status --json; all pass. Confirmed fail-on-revert twice: against the base commit's spawn script the test reproduces the incident's blind 60s timeout verbatim, and against the pre-symlink-fix pool lib the symlinked-lower-cap case fails. Captured a CLI transcript of the end-user refusal for both incident failure modes (full pool with leftover-only slots, duplicate max_trees key). No visual surface involved, so no screenshot.

Evidence: End-user refusal transcript: full pool and unloadable config

Source: End-user refusal transcript: full pool and unloadable config

$ bin/fm-spawn.sh meetsone-repro <project> --mode no-mistakes --yolo off
error: task meetsone-repro cannot be given an isolated copy of <tmp>/project, so it was not launched:
all 2 of 2 worktrees in the pool are in use or dirty (max_trees = 2)
  pool: <tmp>/pool
  0 held by a running worker, 0 held by a durable lease, 2 blocked by leftovers
  slot 1  <tmp>/pool/1/repo
    leftovers only: 0 modified tracked file(s), 1 untracked path(s), no commits off origin/main
      preserve first: (cd '<tmp>/pool/1/repo' && git add -A && git commit -q -m 'wip: firstmate reclaim of pool slot 1' && git branch "fm-reclaim/1-$(date +%s)")
      then clear:     treehouse return --force '<tmp>/pool/1/repo'
  slot 2  <tmp>/pool/2/repo
    leftovers only: 0 modified tracked file(s), 1 untracked path(s), no commits off origin/main
      preserve first: (cd '<tmp>/pool/2/repo' && git add -A && git commit -q -m 'wip: firstmate reclaim of pool slot 2' && git branch "fm-reclaim/2-$(date +%s)")
      then clear:     treehouse return --force '<tmp>/pool/2/repo'
  to raise the cap instead: edit line 1 of <tmp>/project/treehouse.toml, which reads "max_trees = 2", to a larger number - a second max_trees key makes treehouse refuse the file entirely
exit status: 1

# Same pool, but treehouse.toml has a duplicate max_trees key (the third failure mode from the incident):
$ bin/fm-spawn.sh meetsone-repro-cfg <project> --mode no-mistakes --yolo off
error: task meetsone-repro-cfg cannot be given an isolated copy of <tmp>/project, so it was not launched:
treehouse cannot load <tmp>/project/treehouse.toml, so no worktree can be handed out:
  failed to load config: toml: line 3 (last key "max_trees"): Key 'max_trees' has already been defined.
Fix that file before any spawn; treehouse reads it only at the repository root.
exit status: 1
Evidence: Base commit reproduces the incident's 60s timeout (test fails on revert)

Source: Base commit reproduces the incident's 60s timeout (test fails on revert)

not ok - the spawn waited for a pane instead of refusing before it launched anything (unexpected: 'did not enter an isolated worktree')
--- output ---
warning: /private/var/folders/yk/3x0wgs_s12v47knmxd574hm40000gp/T/fm-spawn-pool-preflight.wzfC95/pool-full/home/data/pool-full-p1/launch-brief.md records no delivery contract line (scaffolded before ship briefs recorded one); launching on the explicit --mode no-mistakes - confirm its definition of done matches
error: treehouse get did not enter an isolated worktree within 60s (last seen '/private/var/folders/yk/3x0wgs_s12v47knmxd574hm40000gp/T/fm-spawn-pool-preflight.wzfC95/pool-full/project': it is the spawning project itself; spawning project '/private/var/folders/yk/3x0wgs_s12v47knmxd574hm40000gp/T/fm-spawn-pool-preflight.wzfC95/pool-full/project'); inspect window firstmate:fm-pool-full-p1
Evidence: Symlink regression test fails with a6bb6ff reverted

Source: Symlink regression test fails with a6bb6ff reverted

ok - a pool with no obtainable copy refuses before any record exists, naming cause and remedy
ok - a slot holding unlanded commits is reported as work, with no command that would discard it
ok - an unloadable treehouse.toml refuses naming the file, the line, and the problem
ok - a treehouse get that fails in the pane is reported at once, in its own words
ok - a pane that hangs with nothing to report still fails at the wait's deadline
ok - a pool read that never returns is bounded, and the spawn falls back to its ordinary wait
not ok - a symlinked cap was ignored and the spawn waited for a pane instead of refusing (unexpected: 'did not enter an isolated worktree')

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 3 issues found → auto-fixed ✅
  • ⚠️ bin/fm-spawn.sh:2397 - The intent requires "find a way to never have issues with booting crewmates anymore ever", and the change converts the blind 60s timeout into a fast refusal that names the cause and the remedy. It does not touch why the pool filled: 11 of 16 slots were left dirty by tasks that had already finished, meaning the teardown or return path released those tasks without treehouse return --force landing, or panes were closed outside teardown. That failure remains fully reachable: the next time slots accumulate leftovers, every spawn for that project refuses again until the captain runs the printed commands by hand. Refusing is the correct call given AGENTS.md hard rule 3 (uncommitted changes are never landed), so auto-clearing is not the fix. The durable boundary is the one that finishes a task: bin/fm-teardown.sh already owns treehouse return --force; the question is why those 11 slots escaped it. Remedy extends scope beyond this change (teardown investigation, possibly a slot-reclaim step gated on a captain decision), so this needs the author's decision, not a silent fix.
  • ⚠️ bin/fm-treehouse-pool-lib.sh:67 - fm_treehouse_pool_read_cap returns the default cap of 16 when treehouse.toml is a symlink ([ ! -L &#34;$toml&#34; ]). treehouse itself reads the file through the symlink (Go os.ReadFile follows links). Concrete wrong refusal: a repo whose treehouse.toml is a dotfiles symlink setting max_trees = 32, pool holding 20 worktrees all in-use or dirty, zero available. treehouse get would create slot 21; the preflight computes total=20 >= cap=16 and free=0, sets verdict full, and refuses the spawn with a report claiming "max_trees = 16" and telling the captain to add a max_trees line to a file that already has one. The header comment claims a drift in the default can "delay a refusal, never invent one"; this path invents one. The inverse (symlinked cap of 2, two dirty slots) silently downgrades to the old timeout. Fix: drop the -L test and read the file as treehouse does.
  • ℹ️ bin/fm-treehouse-pool-lib.sh:54 - fm_treehouse_pool_json_objects splits the array on },{, which also splits inside each slot's processes array. On this host's real output an in-use slot with 7 processes yields 8 fragments; it works only because name, path, and status precede processes and the process fragments carry no path key. The greedy .*&#34;name&#34; regex also returns the last process name (e.g. 2.1.272) as the slot name for in-use slots; harmless today because the name is only printed for dirty slots, which have no processes. A treehouse update that reorders keys or adds a path to process objects degrades to verdict unknown, which is fail-open, so the risk is losing the preflight silently, not a wrong refusal. treehouse v2.3.0 is already advertised on this host. Noting the tradeoff; a jq-free parser was clearly the intent.

🔧 Fix: read max_trees through symlinked treehouse.toml, add regression tests
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-spawn-pool-preflight.test.sh on target commit: all 8 cases pass (full pool, unlanded slot, unloadable config, failing get reported verbatim, hanging pane backstop, bounded pool read, symlinked lower cap, symlinked higher cap)
  • Same test file with bin/fm-spawn.sh swapped to base commit 616049a: first case fails with the incident's original 60s timeout message, proving the test fails on revert
  • Same test file with bin/fm-treehouse-pool-lib.sh swapped to ce3486c (pre-symlink-fix): test_symlinked_lower_cap_is_read fails, the other 6 prior cases pass
  • Manual CLI transcript: spawn against a 2-of-2 dirty pool (one slot holding a .gitignore line, one an untracked scratch dir) and against a treehouse.toml with a duplicate max_trees key, captured to the evidence dir
  • git status --short after each swap: worktree restored clean
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Cyril and others added 4 commits September 15, 2026 14:12
The pool preflight reads every worktree in the pool, which is a git status
per slot, and it now runs ahead of every spawn. Left unbounded it would be a
new way for a launch to hang with nothing to say, which is the failure this
work exists to remove. Bound it with fm_run_timed; hitting the bound is an
unsettled question, not a refusal, so the spawn proceeds and the ordinary
wait stays the backstop.

Read the pane's failure marker with grep -o rather than a line anchor, so a
prompt prefix or a carriage return in the captured pane cannot hide a get
that already failed.

Regression: an unreadable pool bounds its read instead of hanging the spawn.
It fails on revert, taking 242s instead of the bounded path.
treehouse reads its config through a symlink, so the preflight must too:
a linked lower cap is now still refused at that cap, and a linked higher
cap no longer invents a full-pool refusal at treehouse's default.
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