docs: add safe Lavish repair playbook - #1202
Open
ruby-dlee wants to merge 222 commits into
Open
Conversation
… worktree pool_preflight iterates every Treehouse worktree in every pool and set failed=1 whenever any one could not be fully inspected (identity, git root, repo identity, or cleanliness) - even worktrees in unrelated pools, and even before confirming the worktree belongs to the target pool. One busy, foreign, or half-torn-down worktree anywhere then failed every spawn, imposing a false capacity cap. Treat an uninspectable worktree like the dirty case already does: skip it and continue. Pools grow and acquire creates a fresh clean slot, so a bad slot never blocks acquisition. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_011Ait6JzBNi3HXxyGheRADM
The spawn path (container_ensure -> server_ensure) still refused an adapter-owned, running, but closed-shell-drifted herdr server when it was occupied - which is always true when FirstMate runs inside the session it manages. Crews could not be launched at all once the server certificate drifted mid-session (an fm-update or landed source change altering the managed-shell digest). Accept the occupied case: each new crew pane is individually env-scrubbed by fm-herdr-worker-shell at creation, so a drifted SERVER cert does not weaken a NEW pane. The certified restart still runs when the session is empty, preserving the boundary whenever it can be enforced. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_011Ait6JzBNi3HXxyGheRADM
Spawn a herdr crew with `herdr agent start <label> --tab <id> --cwd <wt> --env KEY=VALUE ... -- <argv>` instead of creating a bare tab and typing the launch command into its pane composer, and stop requiring a firstmate-issued closed-shell certificate on the herdr server. The certificate existed to prove a crew pane's environment had been scrubbed by firstmate's own managed worker shell. A natively started agent receives its environment from herdr directly, so there is nothing left for a server-level certificate to attest - and requiring one deadlocked every spawn whenever firstmate itself ran inside the herdr session it needed to spawn into: that server was launched by the interactive launcher, could never be certified, and could not be restarted because it was occupied. - fm_backend_herdr_create_task takes an optional agent argv. When present it starts the agent natively into the tab it just created, then closes the tab's original shell pane (only after the agent pane exists - closing a tab's last pane deletes the tab), leaving the same one-pane-per-task shape every identity, capture, and husk-classification path already assumes. - fm_backend_herdr_cli_argv pins --session before the trailing `--`. The plain helper appends it last, which for `agent start` lands inside the launched program's argv; herdr then never sees the session at all. - The agent argv is a login shell evaluating the very same LAUNCH string every other backend types into its pane, so per-harness launch semantics and the Agent Fleet enforced-mode command stay byte-identical across backends. - Account routing is delivered natively: CODEX_HOME / CLAUDE_CONFIG_DIR and GOTMPDIR ride --env rather than a command-scoped shell prefix. Verified first that no login profile on the host sets either, so the login shell cannot overwrite them. - fm-spawn's launch preparation and launch-command construction are wrapped in two functions with unchanged bodies so herdr can run them before endpoint creation, which is when it now needs the final argv. Every other backend still calls them exactly where they ran before; orca cannot be hoisted because it only learns its worktree path during endpoint creation. - Steering prefers `agent send`, which addresses the registered agent and so cannot type into a pane whose agent died and left a bare shell. `pane send-text` remains the fallback for husk and recovery paths. - fm_backend_herdr_agent_session_id reads the harness's own session id from `agent get`, herdr's own observation of the live agent. Verified end to end against real herdr 0.7.3 with one real codex crew: native start, `agent get` reporting agent=codex/status=working, native steer, native read, and the reported agent_session id landing in the sessions directory of exactly the Agent Fleet account directory fm-spawn selected. The now-dead certificate, closed-server, and managed-shell machinery is removed in a follow-up, which the plan gates on this being proven first.
The destructive Treehouse-return boundary walk refused the whole operation if ANY symlink existed anywhere in the tree. That made every repository whose own committed layout uses symlinks permanently un-reapable: relvino carries 177 in every worktree (its CLAUDE.md -> AGENTS.md convention plus symlinked skills), so no crew working there could ever be torn down, and finished crews sat holding their worktree leases indefinitely. The walk exists to prove the return cannot ESCAPE the tree it is pointed at. A symlink ENTRY cannot cause that here: it is inspected with follow_symlinks=False, it is not a directory so it is never queued for descent, and every descent opens with O_NOFOLLOW and re-proves dev/ino identity, single-device, and non-mount. The redirection vector is a symlinked ANCESTOR, and the path-component loop still refuses that. Skip in-tree symlinks instead of refusing them, which is exactly how fm-teardown.sh's sibling validator (removal_tree_operation) has always treated them. New tests extract the embedded boundary program from the library rather than reimplementing it, so they cannot drift from the code they describe: in-tree file, nested, and directory symlinks are accepted; an escaping symlink is accepted as an entry but proven never to be followed; and a symlinked ancestor, a non-directory target, and a project/cwd mismatch are all still refused. The first case fails against the pre-fix library, so it pins the regression.
pool_preflight ran before every spawn and took 343 seconds for a repository
whose worktree is 12 MB. The cost had nothing to do with the repository being
spawned into: it grew with the total number of Treehouse worktrees on the
machine. Batch spawns routinely exceeded ten-minute timeouts because of it, and
it would have been crippling at fleet width.
Two independent causes, both measured rather than assumed:
- The loop visited every worktree in every pool, paying full inspection before
the `common = expected_common` test discarded everything belonging to another
repository. Ask the cheap identity question first and skip foreign pools
outright. The skip is fail-safe by construction: it triggers only when the
probe SUCCEEDS and positively proves the worktree belongs to a different
repository, and any probe failure falls through to the original path unchanged,
so a worktree that is ours is still inspected and reported exactly as before.
- backing_checkout's alias-detection loop called exact_git_root on every
registered worktree, three or four subprocesses each, once per worktree in the
pool - quadratic, roughly 266 full resolutions per preflight here. That loop
only needs to prove exactly one registration resolves to this worktree;
authority for the paths that matter was already established above it.
Canonicalization answers that question on the same comparison basis, since
exact_git_root returns its canonicalized path. A path that cannot be
canonicalized is skipped rather than fatal: it cannot alias a worktree that
canonicalized successfully, so it cannot change the count.
Measured under identical conditions: 343s before, 62s after, stable across runs.
Dirty worktrees in the target pool are still detected and reported.
Not fixed here: tests/fm-checkout-refresh.test.sh has one failing case
("nested non-repository active project was not surfaced") which fails
identically at the base commit and is unrelated to this change.
…atically Follow-up to 52ffbf8, which cut preflight from 343s to 62s by skipping foreign pools and dropping needless resolution in alias detection. The remaining cost had the wrong SHAPE for a growing fleet: every worktree in a pool recomputed facts that are identical for all of them. Every worktree in a pool belongs to the same repository, so `git worktree list`, the primary checkout, its common dir, and the canonical form of every registration are the same for all of them. Recomputing per worktree made a whole-pool walk scale as pools x worktrees x registrations - it got worse as the fleet grew, which is exactly backwards. Compute those once per pool behind an opt-in cache, and reduce alias detection to string comparison over the already-canonicalized registrations, so its per-worktree cost is zero regardless of pool size. The cache is off by default. Only the preflight sweep enables it, scoped to its own loop and cleared immediately after, so the other two callers - both making one-off decisions - recompute exactly as before. It is keyed on the pool and only consulted for that same pool, so one repository's answers can never be used for another. Measured, same conditions as before: 343s originally, 62s after 52ffbf8, 44-46s now, stable across runs. The relvino pool - sixteen worktrees at 6.6 GB each, previously the worst case - preflights in 32s. What matters more than the constant is the shape: per-worktree cost is now independent of how many registrations the pool holds. Dirty worktrees in the target pool are still detected and reported. The one failing case in tests/fm-checkout-refresh.test.sh is the same pre-existing failure present at the base commit; no new failures.
A harness runs its tool commands through a non-interactive shell. On zsh that reads only ~/.zshenv - not ~/.zprofile or ~/.zshrc, where Homebrew installs itself - so a crew can fall back to zsh's compiled-in default of /bin:/usr/bin:/usr/ucb:/usr/local/bin and lose sight of gh, node, and the axi tooling even though all of them are installed and its worktree is fine. Crews burned entire CI-repair rounds reporting "gh is absent". Compute the PATH at spawn and inject it: via --env for a herdr-native agent start, and via the pane export line every other backend already uses for GOTMPDIR. Firstmate's own PATH is the seed, so the host's existing resolution order is preserved exactly and this can only add reach, never repoint a command. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VtDQaE3RdeuYd3aEaUvLkk
… typo SC1007 flags 'out=' in a local declaration list as a probable missing value. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VtDQaE3RdeuYd3aEaUvLkk
Bulk campaigns shipped green while campaign creation was completely broken. Nothing in the chain had ever created a campaign: the pipeline ran unit tests, fixture tests and a mocked harness, and rendered a template screenshot. Every one of those passes while the product is unusable, so "green" proved code executed, not that a merchant can use it. Every ship brief now states the bar directly - exercise the real path against a real stack before reporting done, confirm the outcome exists, and say so explicitly if you could not rather than substituting harness coverage. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VtDQaE3RdeuYd3aEaUvLkk
The contract named the six required sections but never said they must be level-two headings, while publication requires exactly `## `. Crews reasonably wrote `# Summary`, teardown refused, and the report had to be hand-patched - four times in one day, each blocking a finished crew from being reaped. Spell the headings out verbatim, say that another level fails, and cover the two adjacent traps: sub-headings must be `###` so they nest instead of ending the section, and an inapplicable section says so under its heading rather than being omitted. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VtDQaE3RdeuYd3aEaUvLkk
The suite aborted at its eighth test and had been red on main for 16h, which blocked every firstmate PR behind a failure none of them caused. Three fixtures had drifted from the direct-account spawn path they model: - The fake treehouse never answered `get --lease`, so fm-spawn correctly refused to launch without a worktree. It now reports one on stdout, as treehouse does. - The fixture worktree was attached to a branch. A leased Treehouse worktree is handed over at detached HEAD, and fm-spawn refuses an attached one, so the fixture could never reach the behavior it was asserting. - The identity-drift fake detached an already-detached worktree, which is a no-op, so the post-preparation recheck had nothing to catch. It now moves the worktree onto a branch, which is a real drift. One assertion also pinned the order of two correct guards rather than the refusal itself; it now accepts either and additionally requires the refusal to name the offending path. Twelve tests go from unreachable to passing. The one remaining failure is the failed-spawn worktree leak itself, which is under repair separately. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VtDQaE3RdeuYd3aEaUvLkk
…isting them The completion-report contract was reworded to spell out the level-two heading requirement crews kept getting wrong. This assertion matched the old comma-list sentence verbatim, so it broke even though the contract still names all six sections - it was pinned to phrasing rather than to the guarantee. Assert each required heading in the exact form publication demands. That is what the brief actually has to carry, and it survives the next rewording. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01VtDQaE3RdeuYd3aEaUvLkk
* no-mistakes(test): Stabilize worktree safety and watcher cleanup tests * no-mistakes(test): Correct unknown-endpoint teardown test simulation * no-mistakes(document): Document failed-spawn worktree retention safety * no-mistakes(document): Isolate failed-spawn Treehouse test fixtures * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes * no-mistakes: apply CI fixes
* fix: verify passed-run PR state * no-mistakes(review): Clamp GitHub state query timeout * no-mistakes(review): Clamp all invalid GitHub timeout forms * no-mistakes(document): Document verified passed-run PR states
* fix(herdr): align behavior gate with native routing * no-mistakes(review): Isolate Herdr lifecycle validation from default fleet * no-mistakes(review): Forbid default targeting in Herdr lifecycle fake * no-mistakes(review): Guard positional default Herdr lifecycle targets * no-mistakes(test): Captain: distinguish selection contention from provider maintenance * no-mistakes(test): Synchronize watcher takeover test with observation loop * no-mistakes(test): Captain: isolate retained endpoint fixture identity * no-mistakes(test): Captain: isolate current secondmate fixture contracts * no-mistakes(test): Synchronize account-routing lifecycle race fixtures * fix(account-routing): assert timeout behavior directly * test(account-routing): complete restored predecessor fixture * no-mistakes(document): Clarify Herdr production lifecycle ownership
3 tasks
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.
Summary
lavish-repairplaybook that proves the HTTP, live-channel, poll-listener, and browser layers in cheapest-first order.pkill -fsafety gate with the 2026-07-28 incident, explicit-PID repair, stable-session guidance, and crew-kill recovery boundaries.Verification
bash tests/lavish-repair.test.shbash tests/operating-fundamentals.test.shbin/fm-lint.shno-mistakesreview and documentation checks passed.