fix(bin): require project clone roots during fleet sync - #2849
Conversation
Git repository discovery walks upward, so `git -C projects/<dir>` on a plain directory nested under projects/ resolves to the enclosing repository - in a firstmate home, the firstmate checkout itself. fm-fleet-sync.sh guarded its candidates with `rev-parse --is-inside-work-tree`, which such a directory passes, so every later git call read, pruned and fast-forwarded firstmate's own default branch and reported it under the project directory's label. A running session's AGENTS.md changed underneath it, and the report named a project that had nothing to do with the change. Require each candidate to be the root of its own work tree before any other git command: compare `rev-parse --show-toplevel` against the directory's own physical path. Both sides are physical, so a symlinked clone still compares equal. Anything else is skipped by name, naming the repository that would have been touched, and bootstrap relays that as a FLEET_SYNC line. Regression coverage reproduces the wrong-repo fast-forward against a home nested inside another repository, in both the whole-fleet and single-project forms, and pins that a symlinked clone dir still syncs.
Confidence Score: 5/5The PR appears safe to merge, with the clone-root guard preventing the unintended enclosing-repository update while preserving supported synchronization paths. The changed script rejects candidates whose physical directory is not their resolved Git work-tree root before running subsequent Git operations, and the added tests cover both vulnerable invocation forms plus the symlink compatibility case. Reviews (1): Last reviewed commit: "no-mistakes(review): Keep enclosing fixt..." | Re-trigger Greptile |
|
Speaking as Kun's firstmate: Scheduled 3:10am PT 8/23 pass. Main reconfirmed VISION (inspected Class: corrective. Replaces Security: none. Path comparison plus a louder skip. No Overlap / HOLD: not spawn/teardown, does not edit CI / NM: HEAD Workflows: already approved (CI completed SUCCESS on this HEAD). Run IDs: 32629198233 (CI), 32629198212 (Require no-mistakes), 32630050933 (Require no-mistakes). No pending first-time-fork approval. Land-eligible rec: YES (corrective, green CI+NM matching this HEAD, VISION aligns, not default-behavior, not a spawn/teardown/herdr hold). Captain-flag NOW: no. |
|
Speaking as Kun's firstmate: this is merged. Thank you @karotkriss — really appreciate you taking the time on this. |
Fixes #2691
Intent
Fix a defect in bin/fm-fleet-sync.sh, reported as issue #2691: the project-clone refresh treats every directory under projects/ as a clone. Git repository discovery walks upward, so for a directory that is not itself a repository (for example a worktree container left under projects/, where the actual repository is one level below), every 'git -C
' resolves to the ENCLOSING repository. In a firstmate home that enclosing repository is the firstmate checkout itself, so a routine fleet refresh silently fetched, pruned and fast-forwarded firstmate's own default branch and reported the result under the unrelated project directory's label - an unrequested self-update outside the guarded update path, with a report that named the wrong repository.The goal is the minimal, targeted guard the issue asks for: before any other git command runs against a candidate under projects/, require that the candidate is the root of its OWN work tree, comparing 'git rev-parse --show-toplevel' against the directory's own physical path. Anything else is skipped by name, loudly enough to be noticed, and the skip message names the repository that would otherwise have been touched so an operator can see what discovery resolved to. The existing 'skipped: not a git repo' wording and behavior for a candidate that resolves to no repository at all is deliberately left unchanged, so only the new enclosing-repository case gets new wording.
Deliberate decisions a reviewer reading only the diff would not know:
What Changed
Risk Assessment
✅ Low: Captain, the targeted root guard closes the reported upward discovery path, preserves intended skip behavior, and the corrected regression fixture now genuinely exercises the pre-fix fast-forward risk.
Testing
The focused regression suite passed, and an end-to-end CLI transcript proves the fixed behavior, preserved cases, and historical failure reproduction. The first transcript command was rejected before execution due to its cleanup syntax, then rerun successfully without worktree changes.
Evidence: Issue #2691 end-to-end CLI transcript
Source: Issue #2691 end-to-end CLI transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (2) ✅
tests/fm-fleet-sync.test.sh:116- The regression fixture does not reproduce the reported fast-forward. Its initial commit contains only AGENTS.md, so creating projects/not-a-clone makes the enclosing checkout dirty. Against the pre-fix script, fetch succeeds but status --porcelain sees that untracked directory and returns STUCK instead of fast-forwarding. Track a firstmate-like .gitignore containing /projects/ in the fixture's initial commit so the enclosing checkout remains clean and the before/after SHA assertion proves the dangerous mutation.🔧 Fix: Confirm clone-root regression fixture remains valid
1 warning still open:
tests/fm-fleet-sync.test.sh:116- The fixture does not reproduce the reported fast-forward. Its initial commit tracks only AGENTS.md, so creating projects/not-a-clone makes the enclosing checkout dirty. Against the pre-fix script, status --porcelain therefore reports STUCK instead of advancing HEAD. Track a .gitignore containing /projects/ in the initial fixture commit so the checkout remains clean and the SHA assertion proves the dangerous mutation.🔧 Fix: Keep enclosing fixture clean during clone-root regression
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-fleet-sync.test.sh(run twice; both completed successfully)Invokedbin/fm-fleet-sync.shin whole-fleet and single-project forms against a nested candidate whose enclosing repository had a pending fast-forward; verified the skip output named the enclosing repository and HEAD remained unchangedInvokedbin/fm-fleet-sync.sh symlinked-clone; verified physical and Git top-level paths matched and the clone fast-forwardedInvokedbin/fm-fleet-sync.sh plain-diroutside any repository; verifiedskipped: not a git reporemained unchangedExecuted base revision8714c9aagainst the same enclosing-repository fixture; reproduced the mislabeled sync and unintended fast-forward✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.