pool: provision worktrees by APFS clone, and stop the scratch reaper deleting live sessions - #44
Merged
Merged
Conversation
Every pooled worktree installs its own full node_modules from scratch, so ~/.treehouse had grown to 6.5 G across four optiroq slots. On APFS a copy-on-write clone shares those blocks instead of duplicating them. bin/fm-worktree-provision.sh discovers a worktree's install roots (the repo root plus any lockfile dir, never a bare package.json below it), clones each one's node_modules from a per-pool cache under ~/.treehouse/.fm-dep-cache/, then reconciles with the project's own installer. The cache seeds itself from the first correct tree it sees, so nothing under projects/ is ever written. Clone support is probed with a real cp -c, never inferred from the OS: without it the script installs plainly, says so, and never falls back to a real copy that would double the disk instead of saving it. Measured on optiroq: 2.70 G -> 96 M of real disk per additional slot (df, not du - du counts cloned blocks). Wiring it into the warm path is a separate commit.
…s true The warm path installed nothing. Every pooled worktree therefore built its own full node_modules from scratch - 6.5 GB across four slots on this box - and fm-pool-warm.sh's header credited a treehouse post_create hook for an install that has never run. Provision inside the lease the warm already holds: clone each dependency tree from a per-pool cache with `cp -c` (an APFS copy-on-write clone), then reconcile with the project's own installer so the result is correct and not merely present. Measured on optiroq (three roots, df deltas, all slots idle): a cold install costs 56 s and 2.70 GB of real disk; clone + reconcile costs 53 s and 96 MB. That is 96.5% less disk per additional slot and no time saved at all - npm install dominates once the tree is nearly right. It buys disk, not speed. Correctness was proven by building in the provisioned slot, not by listing it. Clone support is PROBED with a real one-byte `cp -c`, never assumed: a non-APFS volume or a GNU cp degrades to a plain install and says so, and never to a real `cp -R`, which would double the disk this exists to save. Also here, because the feature depends on them: - docs/treehouse-backend.md records why there is no treehouse.toml: hooks in a repo's own config are ignored by design, and the only working hook home is global to every repo on the box. Read it before proposing one again. - fm_pool_run_bounded owns the warm's time bound and has a portable fallback. Stock macOS ships no `timeout`, so the bound silently did not hold there - a survivable gap when the only bounded step was a `treehouse get`, and not one now that a dependency install runs on that path. - Three pre-existing test failures on macOS: a disk-budget fixture that equalled its own ceiling on APFS, and two lock holders that hardcoded `flock`.
npm rewrites its own lockfile during an install, so harvesting the POST-install fingerprint meant no later slot could ever match it: a fresh checkout carries the lockfile git has, and every warm re-harvested a multi-GB tree for nothing. Store the fingerprint taken before the reconcile, which is the state the next slot presents. Case (n) covers it, and fails without the fix.
…ss check The reaper deleted LIVE harness scratch on every macOS run, including the scratch of the session doing the reaping. The liveness probe was `find "$d" -type f -newermt "@$cutoff" -print -quit 2>/dev/null`, and both of those primaries are GNU-only. BSD find rejects the @epoch form outright - "find: Can't parse date/time: @1785062658", exit 1, no stdout - and with stderr discarded, the empty result read as "no recent file here, safe to reap". So the age gate and the liveness gate were both absent on macOS: every session dir under /tmp/claude-<uid> was reaped regardless of age. bin/fm-bootstrap.sh calls the reaper on every non-detect-only run, so this fired at every session start, and several test files exec the real bootstrap against the real root, so `bin/fm-test.sh` did it too - it deleted this task's own in-flight suite log mid-run. It hid because `find` in an interactive Claude Code shell is a function shimming bfs, which accepts the GNU forms. Only the script, running under bash with /usr/bin/find, ever saw the failure. The date syntax was the trigger; the defect was deleting on an unanswered safety check. So both are fixed: - The probe moves into has_recent_file() and answers with three states - alive, demonstrably dead, unknown - and unknown SPARES the tree. Only a definite "dead" may clear a deletion. A permission error part-way down the tree is unknown too. This is rail 4 in the header's safety model. - `-mmin` replaces `-newermt "@epoch"`: BSD and GNU both accept it and it needs no reference file. `-quit` is dropped for the same portability reason. - A spared-on-unknown is counted and reported on stdout, because a probe that cannot run means the reaper has silently stopped working. stdout, not stderr, since fm-bootstrap.sh discards stderr. Tests (g) and (h) drive the reaper through a stub `find` that reproduces BSD's rejection, so they fail on a GNU runner too - a test that only exercises GNU find cannot see this bug, because GNU find is the half that always worked. Both were verified to fail against the pre-fix script and pass against this one. The fixture aging moves to a portable `age_days`: `touch -d '3 days ago'` is GNU-only and leaves the mtime at now on BSD, quietly turning the "dead" fixture into a live one - which is why the existing case (a) was already red on macOS.
This was referenced Jul 30, 2026
webjema
pushed a commit
that referenced
this pull request
Jul 30, 2026
… findings on #44) (#45) * pool: stop warm provisioning bricking the slot it warms The warm runs the project's installer inside the leased slot, npm rewrites the tracked lockfile it installed from, and treehouse's reset on return is `git clean -fd` - no checkout - so the slot comes back dirty, is skipped by every later get, and is refused by prune. Every warm retired one slot. Half of optiroq's pool had already gone that way. Not macOS-specific: the clone degrades cleanly on Linux, but the install still runs in the slot, so WSL gets bricked slots too, minus the saving. The provision now snapshots which tracked paths were already modified and, on the way out including SIGINT/SIGTERM, restores from HEAD exactly the ones the installer added. The predicate is `git status`, not `git diff`: under optiroq's `text=auto eol=lf` a CRLF rewrite is invisible to diff and visible to status, which is the case that bricked slots in practice. Also in the provisioner and the warm: - The pool guard was dead code. `cd "$wt/../.."` succeeds for nearly any path, so the script would install into any directory handed to it. It now verifies containment under the treehouse root and the <pool>/<slot>/<repo> shape, and refuses with a reason. - A failed provision was logged WARMED and cleared the disk-budget block. The status is propagated and the slot is reported COLD. - `cp -c` exits 0 when it silently falls back to a full copy, so the probe proved nothing. Detection is now same-device, then apfs, then the probe. - The provision deadline was taken after the get, which is bounded by the same value, so the lease and pool lock could be held for twice the stated bound. One deadline is taken before the lease and passed down. - A failed cache refresh no longer destroys the usable previous entry. - INT/TERM now clean up and exit rather than continuing unlocked. Coverage for each, all confirmed to fail against the pre-fix code. * scratch-reap: unknown must be transient, not a permanent exemption Fail-closed was right, but one unreadable subdir - or a file another process removed mid-walk - exempted a session dir forever and printed an unexplained alarm, so scratch would grow without bound. "The probe cannot run at all" and "traversal hit one bad entry" are now different states. The first is fatal to the sweep and says so. The second spares one directory, names it in the summary, and is bounded by a hard ceiling measured on the directory itself with -maxdepth 0, so no unreadable child can defeat it. Two smaller ones in the same file: - `--max-age-hours 0` produced `-mmin -0`, which matches nothing, so every session dir including the live caller's read as dead. Rejected now, with a textual check because $(( 08 )) is an octal error rather than eight. - The comment justifying the removal of -quit was factually wrong: BSD find on this box accepts it. Restored behind a capability probe, with a comment that says what is true. * spawn: exclude the crew's own scratch dir from its worktree bin/fm-brief.sh tells every crew to keep a running plan at .fm/progress.md and never commit it, and bin/fm-teardown.sh reads untracked files as unlanded work and refuses to release the worktree. A crew that follows its brief bricks its own teardown; one had to be force-released today. fm-spawn.sh already excludes its harness hook files this way, so .fm/ joins them, for every harness. `git rev-parse --git-path` answers relative to the repo it was asked about, and the spawn's cwd is not that repo, so exclude_path now resolves a relative answer against the worktree instead of creating a stray .git/info/ wherever the spawn happens to be standing.
webjema
pushed a commit
that referenced
this pull request
Jul 30, 2026
…at (#46) * review: mark a blocking verdict on the PR, not only in firstmate's chat PR #44 merged carrying seven confirmed defects while its crew was still fixing them. The review had found all seven and firstmate had said not to merge, but that verdict existed only in the firstmate session's chat, so whoever clicked merge never saw it. AGENTS.md section 6 step 3 now requires the verdict to land on the PR when one is already open: draft a firstmate-authored PR with `gh pr ready --undo`, which blocks the merge button mechanically instead of merely advising, plus a comment carrying the reason; never draft a bot's PR, where a comment and a `do-not-merge` label are used instead. docs/pr-block-signal.md owns the commands, the per-repo label setup, and the verification record - including the check that drafting does NOT suppress this repo's PR CI, which is what makes the draft path safe for the fix loop. * docs(pr-block-signal): replace inferred draft-CI reasoning with live evidence The event reference does not document whether workflows run on draft PRs, so the claim now rests on a live draft PR in a public repo whose workflow has the same bare `pull_request:` trigger shape as ours (cli/cli#14013, 15 completed check runs while isDraft). Also records that GitHub's current stage-change docs carry no plan or visibility restriction on drafts at all, which makes gh's "If supported by your plan" caveat conservative rather than a live constraint. * docs(pr-block-signal): record the bot-versus-firstmate discriminator check Verified on live PRs of both kinds that .author.is_bot separates the two mechanisms, and stated the two commands that close the one gap left in the record - the live draft conversion - on any open firstmate PR.
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.
This branch carries two independent changes. They are unrelated, and the second one is the more urgent of the two for anyone running firstmate on a Mac.
1. The scratch reaper deleted live sessions' scratch on every macOS run
This is a data-loss defect, not a cleanup improvement. It is first here because it is the part that was actively costing people work.
What it did
bin/fm-scratch-reap.shdecided whether a harness session was still alive with:Both of those primaries are GNU-only. BSD find - what macOS ships, and what the script actually gets - rejects the
@epochform outright:Exit 1, nothing on stdout, and the diagnostic swallowed by
2>/dev/null. The caller tested only whether the output was empty, so an unanswerable probe read as "no recent file here, safe to reap". On macOS the age gate and the liveness gate were therefore both absent: every session dir under/tmp/claude-<uid>was deleted on every run, regardless of age or liveness.bin/fm-bootstrap.sh:396calls the reaper on every non-detect-only run, so this fired at every session start. Several test files exec the real bootstrap against the real root, sobin/fm-test.shdid it too - it deleted this task's own in-flight suite log and PATH shim mid-run, which is how the bug was found. A dry run one second after writing a fresh file into a live session dir still named that dir as reapable:Why it hid for so long
The
findin an interactive Claude Code shell is a shell function shimming bfs, which accepts both GNU primaries happily. Every interactive probe of this code therefore passed. Only the script - running under bash, wherefindresolves to/usr/bin/find- ever saw the failure. Reading the line in a terminal and reading it the way the script does gave opposite answers.The fix, and why the date syntax is the smaller half
Portability alone would have papered over the real defect: the reaper deleted on an unanswered safety check. A probe that cannot run is not evidence of death. So both are fixed:
has_recent_file(), which answers with three states - alive, demonstrably dead, and unknown - carried in its exit status. Unknown spares the tree. Only a definite "dead" may clear a deletion. A permission error part-way down the tree is unknown too: sparing a reapable dir costs a little disk, and the other mistake costs a live crew its work. This is now rail 4 in the header's safety model.-mminreplaces-newermt "@epoch"- BSD and GNU both accept it, and it needs no reference file. GNU-only-quitis dropped for the same reason.fm-bootstrap.shdiscards stderr.One defect caught while writing it, worth recording because it is the same class of bug: the first version read find's status via
${PIPESTATUS[0]}afterout=$(… | head -1). Inside a command substitution that describes the assignment, not the inner pipeline, so the fail-closed check would have read success every time. The pipe is gone and find's status is taken directly.Tests
Two regression tests, both driving the reaper through a stub
findthat reproduces BSD's rejection exactly - a diagnostic on stderr, non-zero exit, nothing on stdout:Both were verified to fail against the pre-fix script and pass against this one, individually, on this box's real BSD find:
They fail on a GNU runner too, because the stub rejects the primary regardless of platform. That is deliberate: a test that only exercises GNU find cannot see this bug at all, since GNU find is the half that always worked.
The fixture aging also had to change.
touch -d '3 days ago'is GNU-only; on BSD it leaves the mtime at now, quietly turning the "dead" fixture into a live one - which is why the existing case (a) was already red on macOS. It is now a portableage_days(date -v-Ndelsedate -d 'N days ago', then POSIXtouch -t).2. Provision pooled worktrees by APFS clone
Every pooled worktree used to build its own full, independent
node_modules. On this box~/.treehousehad reached 6.5 GB across four slots.What this buys: disk, not speed
Measured on optiroq (three install roots, warm shared npm cache, all slots idle), as
dfdeltas:dfdelta)96.5% less real disk per additional slot, and essentially no time saved - 56 s versus 53 s.
npm installdominates and the npm cache is already shared, so anyone deciding whether this is worth it should know it buys disk, not speed.ducannot see any of this: it counts every block a file references, shared or not, so a cloned tree looks full-size todu. Only free-space accounting shows the truth, which is why every number above is adfdelta.Correctness is proven by building in the provisioned slot, not by listing directories - which a broken clone would pass trivially.
npm run buildat the repo root and insrc/portal-ui, both exit 0. End-to-end with the real treehouse binary and real npm, a leased slot withnode_modulesremoved provisioned asclone=yesand then worked:node -e "require('ms')(90000)"->2m,npm ls ms->└── ms@2.1.3.It degrades rather than lies
can_cloneprobes with a real one-bytecp -cbefore trusting the filesystem. On a non-APFS volume or a non-macOS box the provisioner falls back to a plain install; it never silently substitutes a disk-doublingcp -R. Detection, not assumption. Dependency roots are discovered from lockfiles rather than hardcoded, so nothing here is optiroq-specific.Canonical source
A firstmate-owned cache under
$TREEHOUSE_ROOT/.fm-dep-cache/<pool-key>/<subpath>/node_modules, seeded from the first slot that installs. It was chosen because it needs no widening of firstmate's read-only posture towardprojects/- the alternative, seeding the primary clone, would have.Positively verified, not assumed: a marker file was stamped, a real provisioning run of
~/.treehouse/optiroq-84584f/1/optiroqwas performed (three roots, 53 s), and bothfind <firstmate-home>/projects -newer <marker>andfind ~/.config -newer <marker>came back empty. Nothing in this design writes underprojects/or to the user-global config, so noAGENTS.mdrule-1 exception is needed.bin/fm-pool-warm.sh's header was falseIt promised that a treehouse
post_createhook pre-installed dependencies, and cited "137 s cold versus 2 s warm". Neither was true. Repo-leveltreehouse.tomlhooks are ignored by design ("Hooks in repo-level treehouse.toml are ignored for safety") - the user-level config is the only home for a hook, and firstmate installs none, because that config is global to every repo on the machine. So treehouse was installing nothing and the pool was never warm in the sense the header claimed.That finding is now recorded in
docs/treehouse-backend.mdwith its full probe evidence, deliberately placed where the next person will hit it before re-proposing a repo-level hook rather than only in this PR description. The header now describes what actually happens and cites the measured numbers above.Known limitation, accepted
A
treehouse getthat finds no warm slot still hands over an empty worktree and the crew installs for itself. That is exactly today's behavior, not a regression, and the always-plus-one warm invariant closes it in steady state.Also fixed here, because this change depends on it
fm_pool_run_boundedinbin/fm-pool-lib.shis now the single owner of how a warm step is time-bounded. Stock macOS ships notimeout, so the bound silently did not hold there - survivable when the only bounded step was atreehouse get, but not now that a longnpm installruns on the warm path.Test status
The full suite ran once, end to end: 70 of 72 passing in 488 s. Both failures are explained, and neither is caused by this branch:
tests/fm-mission.test.sh- pre-existing. It fails identically at the merge-base, so it is not this branch's doing. macOS BWK awk (20200816) rejects a newline inside a-vassignment (newline in string ... at source line 1, rc=2), andbin/fm-mission.sh:191replace_sectionpasses a whole multi-line section body straight into-v body=. Worth its own task; deliberately not fixed here. (bin/fm-direction.shis not affected - it collapses newlines withtr '\n' ' 'before its awk call.)tests/fm-watch-checkpoint.test.sh- an artifact of the test harness, not a repo defect.bin/fm-test.shhard-requires coreutilstimeout, which stock macOS lacks, so the run needed a stand-in on PATH;bin/fm-watch-checkpoint.sh:77then prefers thattimeoutover the perl fallback the box would otherwise use, and the stand-in's kill semantics differ. Proven by isolation: the test passes with no shim on PATH and fails with it. CI has a real coreutilstimeout, so it does not apply there.bin/fm-test.shitself is unchanged - it is the single owner of the test-run definition, and failing closed is correct behavior for a test runner.Three pre-existing macOS failures unrelated to either feature were fixed along the way, all confirmed failing at the merge-base first: a budget fixture that equalled its ceiling exactly on APFS, and two test helpers that hardcoded
flock(which stock macOS does not ship).bin/fm-lint.shclean.Reviewer notes
/code-reviewand/verifyare not invocable in this environment. What was done instead: a manual diff review - which found and fixed a real defect, where the cache freshness check compared a pre-reconcile fingerprint against a stored post-reconcile one, so npm's own lockfile rewrite meant the cache could never match and every warm re-harvested - and the real end-to-end run described above.docs/scripts.md's toolbelt table has no row forfm-scratch-reap.sh, andbin/fm-bootstrap.sh:396calls the reaper without--self, so the running session relies on the liveness probe alone rather than the by-name rail the header advertises.