fix(bin): stop firstmate-repo crews from adopting the firstmate role - #2880
fix(bin): stop firstmate-repo crews from adopting the firstmate role#2880cedmos wants to merge 5 commits into
Conversation
Harnesses auto-load AGENTS.md and CLAUDE.md from the worktree as project instructions that outrank the launch brief. When the project is firstmate itself, those files are the firstmate job description, so a crew or scout runs session start, scaffolds a brief, and tries to spawn a worker for its own task. After refreshing a ship or scout worktree, spawn now overlays those auto-loaded files with crewmate instructions and hides the overlay from git status. The committed job stays in HEAD for editing, and the overlay keeps firstmate-coding-guidelines reachable. Session start refuses in the same linked firstmate worktree so a worker that still reaches for it cannot build a ghost home. The regression asserts the auto-load corpus no longer contains firstmate identity, not merely that a warning string is present.
Confidence Score: 5/5The PR appears safe to merge because no unacknowledged blocking failure remains. No blocking failure remains; the two previously reported silent-loss behaviors are explicitly deferred to the separate hiding-mechanism redesign described by the author. Reviews (2): Last reviewed commit: "no-mistakes(document): fix stale brief-s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if ! rm -f "$wt/$FM_CREW_AGENTS_WIP" "$wt/$FM_CREW_CLAUDE_WIP"; then | ||
| echo "error: could not remove the saved instruction sidecars in $wt" >&2 | ||
| failed=1 | ||
| fi |
There was a problem hiding this comment.
When a task is interrupted after modified AGENTS.md or CLAUDE.md content is saved to an excluded sidecar, the next pooled-slot refresh removes that sidecar before checking cleanliness, permanently deleting the only durable copy of the worker's uncommitted edits.
Context Used: If there is a VISION.md file at the root of the re... (source)
| head_hash=$(git -C "$wt" rev-parse --verify --quiet "HEAD:$rel" 2>/dev/null) || return 1 | ||
| disk_hash=$(git -C "$wt" hash-object --no-filters -- "$wt/$rel" 2>/dev/null) || return 1 | ||
| [ -n "$head_hash" ] && [ -n "$disk_hash" ] || return 1 | ||
| [ "$head_hash" != "$disk_hash" ] | ||
| } | ||
|
|
||
| fm_crew_save_instruction_wip() { # <worktree> <rel> <dest> | ||
| local wt=$1 rel=$2 dest=$3 | ||
| [ -f "$wt/$rel" ] || return 0 | ||
| fm_crew_file_is_installed_overlay "$wt" "$rel" && return 0 | ||
| fm_crew_file_differs_from_head "$wt" "$rel" || return 0 |
There was a problem hiding this comment.
When a relaunch worktree contains an untracked instruction file with no corresponding HEAD blob, fm_crew_file_differs_from_head returns false and the save helper reports success without creating a sidecar; overlay installation then replaces the file and silently destroys its in-progress content.
…heckout doc claims
cedmos
changed the title
1 participant
What Changed
bin/fm-crew-worktree-instructions-lib.shoverlays crewmate project instructions onto the auto-loadedAGENTS.mdandCLAUDE.mdof a firstmate-repo ship or scout worktree (CLAUDE.mdkeeps the canonical@AGENTS.mdpointer), hides them with skip-worktree, installs a worktree-localcore.hooksPathpre-commit guard so an overlaid file cannot be committed with overlay content or silently dropped from a commit, and refuses the spawn when the overlay cannot be hidden or still presents firstmate identity; secondmate homes, non-firstmate projects, and primary checkouts are left alone.bin/fm-spawn.shinstalls the overlay after the pooled-worktree refresh and removes any stale overlay (plus its saved.fm-agents-md-edit/.fm-claude-md-editsidecars) before that refresh, so a returned slot self-heals instead of wedginggit reset --hardon an invisible skip-worktree entry;bin/fm-session-start.shnow exits 2 with no digest in a linked firstmate task worktree.bin/fm-brief.shscaffolds a crew role fence into ship and scout briefs ("You have no fleet", no fleet-management commands, no interactive dialog, conditionalfirstmate-coding-guidelinesload) but not into secondmate charters;tests/fm-crew-worktree-instructions.test.shcovers the library andtests/fm-brief.test.shasserts the fence, withAGENTS.md,CONTRIBUTING.md,docs/architecture.md,docs/scripts.md, the shard-timing table, and the guidelines skill updated to match.Risk Assessment
Testing
Ran the new crew-worktree-instructions suite plus fm-brief, and the three touched consumers (fm-session-start, fm-spawn-pool-base-freshen, fm-ensure-agents-md) — all green. For product-level proof I drove the real fm-brief/fm-spawn/fm-session-start scripts against a real clone of this repo in a throwaway sandbox fleet and captured two CLI transcripts: at the base commit a spawned firstmate-repo crew worktree still auto-loads "You are the first mate." and session start runs there, while at the target commit the same spawn leaves the crewmate role file as the auto-load corpus with the committed job untouched and git clean, session start refuses with exit 2, a reused pooled slot self-heals and refreshes onto a moved default branch, and the pre-commit guard blocks a commit that would silently drop an AGENTS.md edit until the documented restore is done. I used the repo's own documented FM_GATE_REFUSE_BYPASS test-harness hatch (the same one tests/lib.sh sets) so spawn would drive the temp sandbox fleet, never a real one. No screenshot artifact applies: this change has no rendered UI surface — the end-user surface is the agent-facing instruction files and CLI refusals, which the transcripts show verbatim. Worktree left clean and the per-task /tmp/fm-* roots the demo created were removed.
Evidence: End-to-end transcript at target commit: spawn → crew overlay, session-start refusal, commit guard, pooled-slot self-heal
Source: End-to-end transcript at target commit: spawn → crew overlay, session-start refusal, commit guard, pooled-slot self-heal
Evidence: Baseline transcript at base commit f170ced: the role inversion this change removes
Source: Baseline transcript at base commit f170ced: the role inversion this change removes
Evidence: Demo driver used for the target-commit transcript (reproducible)
Source: Demo driver used for the target-commit transcript (reproducible)
Evidence: Demo driver used for the base-commit baseline (reproducible)
Source: Demo driver used for the base-commit baseline (reproducible)
Evidence: What a crewmate now reads before vs after (excerpt from the transcripts)
Evidence: CONTRIBUTING claim check: guidelines load line is scaffolded into every crewmate brief
Pipeline
Updates from git push no-mistakes
⏭️ **intent** - skipped
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-crew-worktree-instructions-lib.sh:202- The overlay makes crewmate work on the instruction files invisible to fm-teardown.sh's uncommitted-work guard, so it can be discarded without a refusal. Concrete path: a crew in a firstmate-repo worktree edits the overlaid AGENTS.md in place (the case the pre-commit guard exists precisely because it is expected). skip-worktree (fm_crew_skip_worktree, line 212) makesgit status --porcelainreport nothing. fm-teardown.sh:1145 computesdirtyfrom exactly that command and its filter at :1153 only whitelists.claude/and.fm-(grok|kimi)-turnend, sodirtyis empty and validate_worktree_teardown_safety returns 0 instead of REFUSED at :1183. The edit is destroyed. The same hole covers the saved sidecar: fm_crew_save_instruction_wip appends.fm-agents-md-editto info/exclude (line 202), so a sidecar holding a relaunch-sidelined AGENTS.md edit is not even an untracked??entry that would have blocked teardown. The change closes this silent-loss class at the commit boundary but not at the teardown boundary. Earliest supported shared boundary: export a predicate from this library (e.g. fm_crew_worktree_has_unsaved_instruction_work, reusing fm_crew_file_is_installed_overlay plus a sidecar existence check) and have validate_worktree_teardown_safety consult it alongsidegit status, rather than adding a third symptom-specific guard.bin/fm-crew-worktree-instructions-lib.sh:212- The skip-worktree bits wedge the branch-freshening step the crew brief mandates, with no in-task recovery documented. bin/fm-brief.sh:381 instructs every crewmate: "ifmainhas advanced, rebase onto it so the eventual merge stays a fast-forward", and bin/fm-merge-local.sh:61 makes firstmate issue the same instruction. Concrete path: a firstmate-repo crew is spawned, the overlay sets skip-worktree on AGENTS.md and CLAUDE.md,mainthen advances with a commit touching AGENTS.md (routine in this repo - this very branch changes it), and the crew runsgit rebase origin/main. git's unpack-trees refuses with "Your local changes to the following files would be overwritten by checkout: AGENTS.md". This branch's own test test_removal_unwedges_a_pooled_reset (tests/fm-crew-worktree-instructions.test.sh:442) asserts that exact refusal forgit reset --hard origin/main, proving the git behavior; rebase/merge/checkout go through the same machinery. fm_remove_crew_worktree_instructions only runs at the NEXT spawn (bin/fm-spawn.sh:1745), so the running crew has no supported exit: the overlay body (lines 83-86) documents restoring AGENTS.md only for the case where the task edits it, never mentions CLAUDE.md's restore, and never connects a failing rebase to the overlay. Options: have the overlay body name this failure and its recipe for both files, or clear the bits around a base refresh instead of leaving them set for the task's whole lifetime.bin/fm-brief.sh:307- The new absolute claim in the scout brief contradicts a capability the same generated brief grants 40 lines later. Line 307 emits "the only channel that reaches anyone is the status file named below", but line 347 of the same heredoc tells the scout "If your deliverable is a visual artifact the captain will review and iterate on, you may host the Lavish review loop yourself (poll, revise, re-serve, staying alive)", and line 349 routes it through the captain-hold-lifecycle completion gate. A scout that honors line 307 will not host the Lavish loop, silently dropping a supported scout deliverable path; a scout that honors line 347 has to decide line 307 is wrong. The intended fence appears to be the harness question dialog specifically, not every channel. Narrowing the clause (e.g. "the only channel that reaches firstmate is the status file named below") would fence the dialog without contradicting the Lavish loop.bin/fm-crew-worktree-instructions-lib.sh:117- fm_crew_write_overlay_body has no callers anywhere in the repository - fm_crew_install_overlay_file writes through fm_crew_overlay_content instead, which is the variant that handles the CLAUDE.md pointer case. Leaving it in invites a future caller to use it for CLAUDE.md and write the crew body over the canonical @AGENTS.md pointer, which would make the brief-mandated fm-ensure-agents-md.sh report a conflict. Delete it.✅ **Test** - passed
✅ No issues found.
bin/fm-test-run.sh tests/fm-crew-worktree-instructions.test.sh— all 22 cases pass (overlay, refusals, commit guard, sidecars, pooled reuse, real spawn)bin/fm-test-run.sh tests/fm-brief.test.sh— includes the new crew-role-fence assertions for ship, scout, and the secondmate negative casebin/fm-test-run.sh tests/fm-session-start.test.sh tests/fm-spawn-pool-base-freshen.test.sh tests/fm-ensure-agents-md.test.sh— regression check on the three consumers this change touchesManual end-to-end at target commit:bash crew-role-demo.sh <worktree> 8e3a68e— realfm-brief.sh+fm-spawn.sh --scoutinto a pooled worktree of a real firstmate clone, thenfm-session-start.shin that worktree,git commitover the overlay, the documentedgit update-index --no-skip-worktree -- AGENTS.md && git checkout HEAD -- AGENTS.mdrestore, and a second spawn into the same slot after origin/main movedManual baseline at base commit:bash crew-role-baseline.sh <worktree> f170ced— same spawn shows the pre-fix role inversion (auto-loaded AGENTS.md still says "You are the first mate.", session start prints the digest)Doc-claim check: generated briefs withbin/fm-brief.sh <id> firstmate --mode {no-mistakes,direct-PR,local-only}and--scout, confirming each scaffolds thefirstmate-coding-guidelinesload line CONTRIBUTING.md now claimsdocs/fm-test-portable-shards.md:72- docs/fm-test-portable-shards.md's own rule is to refresh the per-script weight hints in bin/fm-test-run.sh whenever the serial lane gains scripts. The new tests/fm-crew-worktree-instructions.test.sh has no measured hint and falls back to PORTABLE_SERIAL_DEFAULT_WEIGHT_MS (20000), and the recomputed table shows the lane was already carrying one other unhinted script before this change. I updated the derived table to current truth, but refreshing the hints themselves needs per-shard timing artifacts from a green CI run and a code edit to bin/fm-test-run.sh, both outside this documentation phase.docs/architecture.md:170- docs/architecture.md:170 asserted that firstmate's operating checkout (FM_ROOT) is itself a linked git worktree. That contradicts bin/fm-tangle-lib.sh's header ("the PRIMARY checkout ... is a normal checkout on a real branch"), bin/fm-primary-scope-lib.sh ("only a plain checkout is primary, never a linked task worktree"), docs/turnend-guard.md's guard-predicate section, and this change's new linked-worktree session-start refusal, which would refuse in a linked FM_ROOT. I resolved the conflict in favor of the three code owners and corrected the prose. Flagging it because it is a pre-existing architectural premise, not a fact this change introduced.✅ **Push** - passed
✅ No issues found.