fix(bin): forbid crewmates from administering shared worktrees - #2868
fix(bin): forbid crewmates from administering shared worktrees#2868karotkriss wants to merge 2 commits into
Conversation
…riefs A crewmate ran a `git worktree remove` loop over the treehouse pool its own worktree came from, destroying five worktrees - four belonging to tasks that were running mid-pipeline. The generated brief's rule 2, "stay inside this worktree; modify nothing outside it", is a rule about files: removing a worktree is administration of shared state, not an edit outside a directory, so the sentence never reached the act. The worker satisfied its brief completely. Rule 7 already named one piece of shared infrastructure - the no-mistakes daemon, one instance serving every lane - with the reason stated plainly. The worktree pool is the same class of thing and was unnamed. Fold the pool into that existing rule rather than adding a second warning: state the constraint around the act (create, remove, return, prune, move, reassign a worktree or pool slot; write into a sibling slot), keep concrete commands as examples rather than as the definition so no single provider is pinned, and give the prohibition a real exit through `blocked:`. The rule is emitted from one shared string interpolated into both crewmate scaffolds, so the ship and scout copies cannot drift apart. The secondmate charter deliberately omits it: that home runs its own fleet and legitimately allocates and returns slots for its own crewmates. Contract text only; no runtime enforcement layer.
Confidence Score: 5/5The PR appears safe to merge with no actionable defects identified. The generated rule is consistently included in every intended crewmate scaffold, retains the existing daemon guidance, remains absent from secondmate charters, and follows the repository’s supported shell pattern and delegation model. Reviews (1): Last reviewed commit: "no-mistakes(document): Distill pool-safe..." | Re-trigger Greptile |
|
Speaking as Kun's firstmate: VISION (current main Class: corrective (honest brief; no pool-admin grant). Security: none. Ahead of main; MERGEABLE / UNSTABLE after #2838 landed (brief hunks did not conflict). Matching attestation for HEAD Waiting on green CI, not the captain. Preferred later if fully green. |
|
Speaking as Kun's firstmate: Scheduled 7:10am PT 8/23 pass. VISION.md was read in full from then-main The 14:23Z stamp on this thread was waiting-CI and said there was no spawn/pool hold on these files. CI is now green. Re-inspect of standing spawn-freshen #2622 shows it does edit VISION (inspected Class: corrective. Generated ship/scout briefs gain missing safety-contract text; nothing that already worked changes runtime behavior. Security: none. Brief-generator text and colocated tests only. No Overlap / HOLD: file overlap with standing spawn-freshen #2622 ( CI / NM: HEAD Workflows: already approved (CI completed SUCCESS on this HEAD). Run IDs: 32644739616 (CI), 32644739629 (Require no-mistakes). No pending first-time-fork approval. Land-eligible rec: NO (spawn-freshen hold #2622 shares |
Fixes #2340
Intent
Fixes #2340: generated crewmate briefs never forbid administering the shared worktree pool, and the existing rule 2 ('Stay inside this worktree; modify nothing outside it') does not reach it.
Reported incident: a crewmate ran a
git worktree removeloop over the treehouse pool its own worktree came from and removed five worktrees; four belonged to other tasks running mid-pipeline, which lost their working directories underneath them. No commits were lost only because pooled worktrees share one repository. The worker was not ignoring its brief - rule 2 is a rule about FILES, and removing a worktree is a change to shared administration rather than an edit outside a directory, so the sentence never reached the act.Accepted fix shape, binding and taken from the issue's own 'shape of a fix' section plus the dispatching triage report:
treehousewould leave the same hole open on every other worktree provider and runtime backend, so the text says 'the equivalent operations on any other worktree provider or runtime backend'. Concrete forbidden verbs named: treehouse get/return/remove/prune,git worktree add|remove|move|prune, sibling-slot writes.no-mistakes-daemon rule (rule 7) rather than adding a second warning. Both are one shared instance serving every lane and both have the same exit. The daemon half of rule 7 is preserved verbatim in substance, including the 'On ANY no-mistakes daemon error, append blocked: {the daemon error} and stop' instruction.blocked: {what you need}and stop; firstmate arranges it.'Deliberate implementation decisions a reviewer reading only the diff would not know:
SHARED_INFRA_RULEvariable and interpolated into both the ship and scout heredocs, replacing what were two byte-identical copies of rule 7. This is deliberate anti-drift: a later edit must not be able to fix one scaffold and miss the other. The colocated test asserts the emitted rule blocks are byte-identical across ship and scout, not merely that both contain some matching phrase.IFS= read -r -d '' VAR <<'EOF' || truefollowed by stripping the single trailing newline, matching the existing HERDR_SECTION/DOD pattern in this file. That pattern exists becauseVAR=$(cat <<EOF ...)breaks Bash 3.2 parsing (issues firstmate's fm-brief.sh scaffold script has a bug #166/fm-brief.sh: scaffold fails to parse on macOS system bash 3.2 (line 314: unexpected EOF while looking for matching `)') #958/fm-brief.sh: unescaped apostrophe inside a heredoc breaks bash parsing #1069) and this file has a structural guard test against it; the new code must not reintroduce that class.Scope boundaries: bin/fm-brief.sh and tests/fm-brief.test.sh only. Do not add runtime enforcement, do not extend the prohibition to the secondmate charter, and do not remove or weaken the existing no-mistakes daemon rule.
What Changed
Risk Assessment
✅ Low: The narrowly scoped contract change satisfies the stated requirements across ship and scout briefs while preserving secondmate behavior and the existing daemon rule.
Testing
The pre-fix baseline reproduced the missing pool rule, the focused suite passed on the target, and generated end-user briefs confirmed identical ship/scout protection across every ship mode, preserved daemon instructions, and intentional secondmate exclusion.
Evidence: Generated brief contract comparison
Source: Generated brief contract comparison
Evidence: Generated no-mistakes ship brief
Source: Generated no-mistakes ship brief
Evidence: Generated direct-PR ship brief
Source: Generated direct-PR ship brief
Evidence: Generated local-only ship brief
Source: Generated local-only ship brief
Evidence: Generated scout brief
Source: Generated scout brief
Evidence: Generated secondmate charter
Source: Generated secondmate charter
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-brief.test.shRan the target regression suite against base commit505c8195122b6d3e3a04fa48c13cd184df0321bain an ephemeral fixture; it failed because the no-mistakes ship brief lacked the shared worktree-pool rule, reproducing the original omissionGenerated no-mistakes, direct-PR, and local-only ship briefs plus scout and secondmate outputs withbin/fm-brief.shCompared generated ship and scout rule blocks usingdiff -u; they were byte-identicalVerified rule 7 appears in every generated ship mode and the crewmate prohibition is absent from the generated secondmate charter✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.