feat(bin): support per-project worktree acquisition commands - #2411
Open
Parsunic wants to merge 7 commits into
Open
feat(bin): support per-project worktree acquisition commands#2411Parsunic wants to merge 7 commits into
Parsunic wants to merge 7 commits into
Conversation
…settle test paths
…h native Git cleanup
…ider metadata key
…preflight child provider
… provider wording
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.
Intent
Teach Firstmate to support an optional local, per-project worktree acquisition command so Mathpath can use its own prepared-worktree script instead of Treehouse. The local configuration must follow existing conventions and be documented with a task-slug placeholder; the accepted implementation uses gitignored config/worktree-acquire/, containing exactly one non-empty shell command line with literal . Mathpath's effective command must be representable exactly as scripts/worktree-new.sh && cd ../mathpath-worktrees/, because its script creates but does not enter the directory. Treat this command as trusted local operator configuration, but substitute every occurrence of only the already-validated task slug using shell-safe quoting so task input cannot become shell syntax. Do not put the captain's private effective configuration or private absolute paths into tracked examples; report after merge that config/worktree-acquire/mathpath should contain scripts/worktree-new.sh && cd ../mathpath-worktrees/.
When this config is absent, preserve the existing Treehouse acquisition path with byte-for-byte-equivalent outcomes. Preserve the isolation assertion, two consecutive identical working-directory reads, 60 one-second polls, base freshness, failure preservation, and cleanup safety; do not weaken or simplify them. A configured command that fails, including a retry where Mathpath's target directory already exists, must surface its actionable status promptly rather than hanging until the poll timeout, publish no successful task record, preserve every existing or partially created target and all unlanded work, and never fall back to Treehouse. A successful custom acquisition must retain provider identity so guarded cleanup performs all ordinary dirty, landed-work, process, endpoint, and path-identity checks, verifies that the exact recorded worktree remains registered to the exact recorded project, then uses native Git worktree removal rather than asking Treehouse to manage a worktree it did not create.
Freshen every fresh ship/scout worktree by fetching origin and resetting content to the fetched remote default tip while preserving the acquired task branch's exact attached/detached identity: it must not check out the default branch, detach an attached task branch, or erase task-branch identity. Control-plane relaunch must reuse the recorded worktree and provider, must not rerun fresh acquisition, and must preserve the worktree on failure. Apply the shared acquisition path to every currently participating session-provider runtime (tmux, Herdr, Zellij, and cmux), while keeping Orca excluded because it owns acquisition and cleanup and keeping secondmate launch excluded because it uses its seeded home; do not redesign Treehouse globally.
Add executable-interface regression coverage using existing test patterns for safe placeholder substitution, successful creator-plus-cd acquisition, attached task-branch preservation, two-read settling, prompt existing-target refusal and preservation, malformed configuration refusal, relaunch reuse without reacquisition, and guarded provider-aware cleanup. Update docs/configuration.md as schema owner, owning script headers and help as exact mechanics owners, architecture/backend/operator docs, README, and relevant recovery/runtime guidance without bloating AGENTS.md or creating static source-grep proxy tests. Run the relevant tests, bin/fm-doc-audience-check.sh, bin/fm-lint.sh with pinned ShellCheck 0.11.0, repository-required checks, and fm-ensure-agents-md.sh.
Prove behavior end to end with one real throwaway Mathpath task using /home/parsu/firstmate/projects/mathpath/scripts/worktree-new.sh from a disposable test home. Confirm the resulting /home/parsu/firstmate/projects/mathpath-worktrees/ location, attached task branch at the refreshed origin/main tip, .env.local presence without reading or printing its contents, and node_modules as the expected link. Clean up only the artifacts created by this proof through guarded paths; leave Mathpath's primary copy clean on main. Do not edit Mathpath. Mathpath's current full test command has an independently recorded baseline failure involving environment loading and one curriculum audit record; do not attribute it to this Firstmate change.
Three unrelated Firstmate broad-suite failures were also observed and must not be attributed to or changed by this task. A clean detached origin/main control at commit 6789876 reproduced all three with exit 1 and the same exact assertions: tests/fm-calm-pi-extension.test.sh reported “Pi follow-up absent case rendered a duplicate captain answer”; tests/fm-backend-tmux-smoke.test.sh reported “the tmux task shell did not become ready”; tests/fm-tmux-agent-liveness.test.sh reported “a running harness-named foreground process must classify alive”. The branch broad run showed those same baseline failures; do not change unrelated Calm or tmux behavior to address them. Exact clean-control logs are in /tmp/fm-fm-mathpath-worktree-command/control-logs for this validation session.
Later accepted corrections remain part of this task. Forced secondmate child cleanup must read worktree_provider from each child record and route project-command children through the existing Git-aware registered-worktree removal path, while preserving the Treehouse and Orca paths, adding focused executable regression coverage, and not broadening the feature. For project-command acquisition on Herdr, use a positive marker-based completion and path probe that does not depend on foreground_cwd after a top-level shell cd; preserve Treehouse, Zellij, cmux, Orca, secondmate exclusions, the 60-second timeout, two stable path reads, isolation proof, and existing failure cleanup, with focused executable regression tests. Do not drive live Herdr lifecycle from this task because its brief was not scaffolded as a guarded Herdr lab.
What Changed
bin/fm-spawn.shreads an optional localconfig/worktree-acquire/<project-name>file (exactly one non-empty command line containing a literal<slug>) and runs it in the task pane instead oftreehouse getfor fresh ship/scout spawns on tmux, herdr, zellij, and cmux; every<slug>is replaced with a shell-quoted form of the validated task id, the command's exit status is published atomically so a failure (including an already-existing target) is reported without waiting out the 60s poll, and the existing isolation assertion, two-identical-cwd-read settling, and failure preservation are unchanged. Relaunch, secondmate, and Orca spawns keep their existing paths, and base freshening now asserts the worktree's attached/detached branch identity is unchanged by thereset --hardto the fetched remote default tip.worktree_provider=project-commandin task metadata, whichbin/fm-teardown.shuses to run the full dirty/landed/process/endpoint checks, verify the recorded worktree is still registered to the recorded project, and then remove it viagit worktree remove --forceinstead oftreehouse return; forced secondmate child cleanup reads the same field per child, preflights it non-destructively, and refuses unknown or misplaced provider values while records stay intact.bin/backends/herdr.shgains a marker-based activepwdprobe for this provider, sinceforeground_cwdis stale once the command'scdruns in the pane's own top-level shell.tests/fm-spawn-worktree-settle.test.sh,tests/fm-teardown.test.sh,tests/fm-backend-herdr.test.sh,tests/fm-control-relaunch.test.sh, andtests/fm-spawn-pool-base-freshen.test.sh; documents the new config file indocs/configuration.md(schema owner),AGENTS.md,README.md, and the architecture/backend/skill docs.Risk Assessment
Testing
Ran the five suites the change touches — all pass, including every newly added executable-interface case (safe slug substitution, creator-plus-cd acquisition, two-read settling, no cwd probe before the acquisition publishes completion, malformed-config and existing-target refusals, relaunch reuse without reacquisition, Herdr marker-probe reads, and provider-aware guarded teardown incl. the forced secondmate child path and unknown-provider preflight refusal). Beyond unit coverage I drove the real product surface: real fm-spawn.sh and fm-teardown.sh against a real tmux server with a disposable FM_HOME and Mathpath's own unmodified worktree-new.sh, capturing the actual task-window transcript. The surface here is a terminal, so the reviewer-visible artifact is a real tmux pane capture rather than a screenshot — no GUI is involved. It shows the shell-quoted slug at each placeholder, Mathpath's script creating and entering ../mathpath-worktrees/<slug>, the attached task branch sitting at the freshened origin/main tip, .env.local present without its contents being read, node_modules symlinked to the primary, a prompt 1-second actionable refusal (vs the 60s timeout) on an already-existing target that preserved unlanded work and published nothing, and guarded native-Git teardown with Treehouse never invoked. Lint, doc-audience, and AGENTS.md checks were left to their owning phases per this phase's rules, and the three recorded baseline suite failures were neither rerun nor touched. The /tmp sandbox and tmux server were deleted afterwards; the repo worktree is clean and Mathpath's real checkout was never written to.Evidence: End-to-end proof: config, task-window transcript, resulting state, prompt refusal, guarded cleanup
Source: End-to-end proof: config, task-window transcript, resulting state, prompt refusal, guarded cleanup
Evidence: Raw tmux capture-pane transcript of the crewmate task window during acquisition
Source: Raw tmux capture-pane transcript of the crewmate task window during acquisition
Evidence: Task window: the effective Mathpath command as it actually ran
Evidence: Resulting task record and worktree identity
Evidence: Retry onto an existing target: prompt actionable refusal, nothing destroyed
Evidence: Guarded provider-aware teardown
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-spawn.sh:2371- The worktree-discovery poll reads the pane path on every iteration even when the project command has not reported completion, then throws the value away (line 2372 resetscandidateunconditionally whileacquire_complete != 1). For herdr, zellij, and cmux that read is an ACTIVE probe:fm_backend_herdr_probe_current_path/fm_backend_zellij_current_pathcallsend_text_line, which for herdr ispane run(types the text AND submits it). So while the operator's trusted acquisition command is still running in the pane's foreground, up to ~60 lines ofprintf '%s\n' '__FM_..._CWD_BEGIN__'; pwd; printf ...are typed into that running process's stdin, plus 60 extra backend CLI round-trips and 0.3s sleeps, purely to compute a discarded value. Unliketreehouse get(firstmate-owned, non-interactive), a project-local creator script is arbitrary operator code that may read stdin. Fix: whenWORKTREE_PROVIDER = project-commandandacquire_complete -ne 1, resetcandidateandsleep 1; continuebefore callingspawn_current_path, so no probe is issued until the status file proves the command finished.bin/fm-teardown.sh:2233- The newworktree_providervalidation for secondmate children (unsupported value, orproject-commandon an orca/secondmate child) runs only insidecleanup_firstmate_home_children, which is the destructive loop. The file's established invariant is that every refusal is discovered before anything is destroyed:validate_firstmate_home_children_removal(fm-teardown.sh:2054, called at 2355/2357) andpreflight_firstmate_home_herdr_children(2361) both enumerate the same children for exactly that reason, and the preflight'svalidate_child_worktree_for_removalalready covers the missing-project and unregistered-worktree cases the new code re-checks at 2298/2302. The provider-value check is the one that is not mirrored, so a home whose second child records an unknown provider (a forward-version or hand-edited record) has its first child's pane killed, worktree returned, and records removed before teardown refuses and stops. Fix: readworktree_providerinvalidate_firstmate_home_children_removaland apply the samecaserefusal there.bin/fm-spawn.sh:2390- On the 60s acquisition timeout, fm-spawn exits andspawn_abort_cleanup(813-816) deletes the status file, but the project command is still running in the pane and will later recreatestate/.worktree-acquire-<id>.<pid>.statusitself. Nothing ever removes that late-written file; it is a dotfile so it does not match thestate/*.metaglobs other tooling uses, making this harmless clutter rather than a correctness problem. If you want it bounded, sweep"$STATE"/.worktree-acquire-$ID.*.status*at line 2320 alongside the existingrm -fof the current pair (safe: concurrent same-id spawns are already excluded by SPAWN_TASK_LOCK).🔧 Fix: gate acquisition cwd probe on published status; preflight child provider
2 infos still open:
bin/fm-teardown.sh:2319- Forced secondmate cleanup of a project-command child callsteardown_project_worktree_removewithout first detaching and deleting the child's task branch, unlike the top-level path (fm-teardown.sh:2513-2518) which doesgit checkout --detach+git branch -Dunder the stated goal "drop the local task branch so the shared repo does not accumulate refs". Native Git removal deletes the child worktree outright, so the<slug>ref survives pointing at a worktree that no longer exists. This is not a regression - the sibling Treehouse child branch at 2325 never dropped the ref either, andtreehouse return's "clean, reset" leaves the pooled worktree in place - so project-command children inherit the child path's pre-existing shape rather than introducing new leakage. Noting it because the top-level and child paths now state opposite policies for the same provider.bin/fm-teardown.sh:2071- Theworktree_providerschemacasenow exists three times with divergent wording: the top-level task at 457 ("refusing cleanup"), the non-destructive preflight at 2071 ("forced teardown changed nothing"), and the destructive child loop at 2251 ("retaining that child's durable identity records"). The duplication at 2071/2251 is deliberate defence-in-depth per the accepted correction, and the messages are intentionally distinct, so this is not a demand for consolidation - only a note that adding a future provider value requires editing all three sites, and missing one makes preflight and cleanup disagree about what is acceptable. A shared helper taking a message prefix would preserve both the defensive re-check and the distinct wording.mathpathdriven by Mathpath's own unmodified scripts/worktree-new.sh + scripts/sync-main.sh (copied read-only), not against /home/parsu/firstmate/projects/mathpath itself. This test phase is bound to a worktree-only write boundary, and a live Mathpath run would create /home/parsu/firstmate/projects/mathpath-worktrees/<slug> and fast-forward Mathpath's primary main — writes outside the assigned worktree. Everything the intent asks to confirm (target path shape ../mathpath-worktrees/<slug>, attached task branch at the refreshed origin/main tip, .env.local present without reading it, node_modules symlink to the primary) was confirmed on the sandbox with the real scripts, real tmux, real fm-spawn.sh and real fm-teardown.sh. The captain should decide whether they additionally want the literal throwaway task run inside their real Mathpath checkout.bin/fm-test-run.sh tests/fm-spawn-worktree-settle.test.sh(6 cases, incl. safe <slug> substitution, creator+cd acquisition, two-read settling, no cwd read before completion is published, malformed-config refusal, existing-target refusal)bin/fm-test-run.sh tests/fm-spawn-pool-base-freshen.test.sh tests/fm-control-relaunch.test.sh(attached-task-branch freshening; relaunch reuses the recorded worktree/provider without rerunning acquisition)bin/fm-test-run.sh tests/fm-teardown.test.sh tests/fm-backend-herdr.test.sh(project-command guarded native-Git cleanup, forced secondmate project-command child routing, unknown-child-provider preflight refusal, Herdr marked-probe cwd read and unrendered-probe empty read)Manual E2E: realbin/fm-spawn.sh <id> <project> --mode local-only --yolo offon a real tmux server (private socket), disposable FM_HOME withconfig/worktree-acquire/mathpath=scripts/worktree-new.sh <slug> && cd ../mathpath-worktrees/<slug>, running Mathpath's unmodifiedscripts/worktree-new.shagainst a disposable clone with origin/main advanced after the primary cloneManual E2E evidence capture:tmux capture-paneof the task window,state/<id>.meta,git worktree list,git symbolic-ref --short HEADvsgit rev-parse origin/main,test -f .env.local(contents never read),readlink node_modules, treehouse tripwire logManual E2E failure path: second spawn onto a pre-created target holding unlanded work — timed refusal (1s vs the 60s poll timeout), no meta published, target and work intact, zero Treehouse callsManual E2E cleanup: realbin/fm-teardown.sh <id>— worktree removed and unregistered via native Git, task record retired, Treehouse never invoked, primary checkout left clean on maindocs/configuration.md:108- Operator action that deliberately cannot live in tracked documentation: per the change's own intent, the captain's effective local config fileconfig/worktree-acquire/mathpathmust be created after merge containingscripts/worktree-new.sh <slug> && cd ../mathpath-worktrees/<slug>. Tracked docs intentionally carry only the genericpath/to/create-worktree <slug> && cd path/to/worktrees/<slug>placeholder shape, so this remains an out-of-repo step.✅ **Push** - passed
✅ No issues found.