Skip to content

pool: provision worktrees by APFS clone, and stop the scratch reaper deleting live sessions - #44

Merged
ignovak merged 4 commits into
mainfrom
fm/treehouse-apfs-clone-k7
Jul 30, 2026
Merged

ignovak merged 4 commits into
mainfrom
fm/treehouse-apfs-clone-k7

Conversation

@ignovak

@ignovak ignovak commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

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.sh decided whether a harness session was still alive with:

find "$d" -type f -newermt "@$cutoff" -print -quit 2>/dev/null

Both of those primaries are GNU-only. BSD find - what macOS ships, and what the script actually gets - rejects the @epoch form outright:

$ /usr/bin/find "$d" -type f -newermt "@1785062658" -print -quit
find: Can't parse date/time: @1785062658
$ echo $?
1

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:396 calls 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, so bin/fm-test.sh did 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:

$ bin/fm-scratch-reap.sh --dry-run --verbose
SCRATCH_REAP: would reap /tmp/claude-501/.../4034fb45-...  (~8K, untouched >48h)   # live, written 1s earlier
SCRATCH_REAP: would reap /tmp/claude-501/.../937617bc-...  (~4K, untouched >48h)   # the supervising session's own

Why it hid for so long

The find in 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, where find resolves 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:

  • The probe moves into 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.
  • -mmin replaces -newermt "@epoch" - BSD and GNU both accept it, and it needs no reference file. GNU-only -quit is dropped for the same reason.
  • A spared-on-unknown is counted and reported, because a probe that cannot run means the reaper has silently stopped doing its job. On stdout, not stderr, since fm-bootstrap.sh discards 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]} after out=$(… | 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 find that reproduces BSD's rejection exactly - a diagnostic on stderr, non-zero exit, nothing on stdout:

  • (g) a live session survives a find that rejects the GNU-only date form - and the dead one is still reaped, so the probe is proven to work, not merely to spare everything.
  • (h) a probe that cannot run spares the dir and says so.

Both were verified to fail against the pre-fix script and pass against this one, individually, on this box's real BSD find:

test_live_session_survives_bsd_find      -> not ok - live session reaped under a find that rejects -newermt @<epoch>
test_fails_closed_when_probe_cannot_run  -> not ok - reaper deleted a session dir when its liveness probe could not run

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 portable age_days (date -v-Nd else date -d 'N days ago', then POSIX touch -t).


2. Provision pooled worktrees by APFS clone

Every pooled worktree used to build its own full, independent node_modules. On this box ~/.treehouse had 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 df deltas:

wall time real disk (df delta)
cold install from scratch 56 s 2,832,680 KB = 2.70 GB
clone from cache + reconcile 53 s 98,660 KB = 96 MB
harvest a slot into the cache 45 s 83,284 KB = 81 MB

96.5% less real disk per additional slot, and essentially no time saved - 56 s versus 53 s. npm install dominates and the npm cache is already shared, so anyone deciding whether this is worth it should know it buys disk, not speed.

du cannot see any of this: it counts every block a file references, shared or not, so a cloned tree looks full-size to du. Only free-space accounting shows the truth, which is why every number above is a df delta.

Correctness is proven by building in the provisioned slot, not by listing directories - which a broken clone would pass trivially. npm run build at the repo root and in src/portal-ui, both exit 0. End-to-end with the real treehouse binary and real npm, a leased slot with node_modules removed provisioned as clone=yes and then worked: node -e "require('ms')(90000)" -> 2m, npm ls ms -> └── ms@2.1.3.

It degrades rather than lies

can_clone probes with a real one-byte cp -c before 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-doubling cp -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 toward projects/ - 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/optiroq was performed (three roots, 53 s), and both find <firstmate-home>/projects -newer <marker> and find ~/.config -newer <marker> came back empty. Nothing in this design writes under projects/ or to the user-global config, so no AGENTS.md rule-1 exception is needed.

bin/fm-pool-warm.sh's header was false

It promised that a treehouse post_create hook pre-installed dependencies, and cited "137 s cold versus 2 s warm". Neither was true. Repo-level treehouse.toml hooks 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.md with 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 get that 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_bounded in bin/fm-pool-lib.sh is now the single owner of how a warm step is time-bounded. Stock macOS ships no timeout, so the bound silently did not hold there - survivable when the only bounded step was a treehouse get, but not now that a long npm install runs 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 -v assignment (newline in string ... at source line 1, rc=2), and bin/fm-mission.sh:191 replace_section passes a whole multi-line section body straight into -v body=. Worth its own task; deliberately not fixed here. (bin/fm-direction.sh is not affected - it collapses newlines with tr '\n' ' ' before its awk call.)
  • tests/fm-watch-checkpoint.test.sh - an artifact of the test harness, not a repo defect. bin/fm-test.sh hard-requires coreutils timeout, which stock macOS lacks, so the run needed a stand-in on PATH; bin/fm-watch-checkpoint.sh:77 then prefers that timeout over 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 coreutils timeout, so it does not apply there. bin/fm-test.sh itself 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.sh clean.

Reviewer notes

  • /code-review and /verify are 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.
  • There is no direction on file for firstmate, so the usual direction check has nothing to judge against.
  • Noted but deliberately out of scope: docs/scripts.md's toolbelt table has no row for fm-scratch-reap.sh, and bin/fm-bootstrap.sh:396 calls the reaper without --self, so the running session relies on the liveness probe alone rather than the by-name rail the header advertises.

ignovak added 4 commits July 30, 2026 13:27
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.
@ignovak
ignovak merged commit cd9c6d7 into main Jul 30, 2026
3 checks passed
@ignovak
ignovak deleted the fm/treehouse-apfs-clone-k7 branch July 30, 2026 13:23
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.
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