fix: unify firstmate repository predicate and check landing remote drift - #11
Open
BohnBawerick wants to merge 4 commits into
Open
fix: unify firstmate repository predicate and check landing remote drift#11BohnBawerick wants to merge 4 commits into
BohnBawerick wants to merge 4 commits into
Conversation
… drift - Add bin/fm-self-repo-lib.sh as the one owner of "is this project firstmate's own repository?", compared by physical path, plus its path canonicalizer. - Replace the four open-coded copies in fm-merge-local.sh, fm-pr-merge.sh, fm-fleet-sync.sh, and fm-spawn.sh's freshen_spawn_worktree_base, so the merge, the sync skip, and the spawn base cannot drift apart. - Drop fm-spawn.sh's duplicate proj_real aliases and its private real_path_or_raw copy of the same canonicalization. - Give fm-landing-remote.sh verify a no---ours drift mode asserting the shape apply leaves behind, and run it from fm-bootstrap.sh against the primary at every session start as a LANDING_REMOTE line. Both stay offline. - Fix a stale test assertion left by cd3bcb9: the suite still required the removed --yes flag on no-mistakes init.
…ble apply command
…e and respect read-only bootstrap
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
Unify the firstmate-on-itself predicate and wire landing-remote verification, as two follow-ups to the landed fm-origin-points-upstream work.
Constraints: changes must be clean, self-contained and tested; follow the firstmate-coding-guidelines skill for firstmate's own tracked material (one sentence per line in tracked Markdown, one owner per contract, knowledge routed to its correct owner rather than duplicated); never use em dashes, use plain hyphens; never add an agent co-author line. Our local main is authoritative and origin is https://github.com/BohnBawerick/firstmate.git.
What Changed
bin/fm-self-repo-lib.shto unify the repository detection predicate and path canonicalization acrossfm-merge-local.sh,fm-pr-merge.sh,fm-fleet-sync.sh, andfm-spawn.sh.bin/fm-landing-remote.sh verifyto support drift verification without explicit URLs and emit runnable repair commands on invalid remote configurations.bin/fm-bootstrap.shto surface actionableLANDING_REMOTEdiagnostics during session startup.Risk Assessment
✅ Low: The firstmate-on-itself predicate is unified into a single shared helper, landing-remote drift checking is integrated into bootstrap with read-only mode handling, and all remediation paths are tested and runnable.
Testing
Exercised the unified fm_is_firstmate_repo predicate across all four consuming workflows and validated landing-remote verification and bootstrap drift detection end-to-end with all targeted automated test suites passing.
Evidence: Landing remote drift verification and self-repo predicate test evidence
Source: Landing remote drift verification and self-repo predicate test evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (2) ✅
bin/fm-landing-remote.sh:440- Every drift refusal ends with "run fm-landing-remote.sh apply on the primary checkout", but cmd_apply refuses immediately with "apply requires --ours <url>" / "apply requires --upstream <url>" (bin/fm-landing-remote.sh:488-489). bin/fm-bootstrap.sh:1192 relays that exact sentence as the LANDING_REMOTE remediation, and .agents/skills/bootstrap-diagnostics/SKILL.md tells the agent "The line names the one broken invariant and the command that repairs it". Following the printed command verbatim always fails. The script has already read both URLs it needs (the fork case has fork=ours and origin=parent; the config-drift cases have origin=ours and upstream=parent), so it can name them. Separately, the "origin remote is absent" refusal at line 438 carries no remediation clause at all, so that LANDING_REMOTE line does not match the documented "LANDING_REMOTE: <drift>; <remediation>" shape.bin/fm-landing-remote.sh:433- verify_remapped_shape returns 0 silently whenever neitherupstreamnorforkexists. cmd_apply accepts a pre-apply checkout with origin == the parent and no upstream and no fork remote (bin/fm-landing-remote.sh:509-511 only refuse when origin is neither --ours nor --upstream), so a primary that is a plain clone of the third-party parent is exactly "a primary checkout where apply was not run" and the new bootstrap check stays silent on it. Closing this needs the landing URL passed as --ours, which firstmate does not store anywhere today, so it would mean new configuration beyond the stated scope. The header comment at bin/fm-landing-remote.sh:60-63 states the tradeoff explicitly. Noting it so the coverage limit is a known one, not an assumed-complete check.🔧 Fix: make landing-remote drift refusals print a runnable apply command
3 issues (2 warnings, 1 info) still open:
bin/fm-landing-remote.sh:451- The origin-absent refusal now carries remediation text, but the command it names still cannot run. It prints "run fm-landing-remote.sh apply --ours <fork-url> --upstream <upstream-url> on the primary checkout", and cmd_apply refuses that invocation at bin/fm-landing-remote.sh:511 withorigin=$(remote_url origin); [ -n "$origin" ] || fail "origin remote is absent"before it reaches any remap shape. Concrete reachable sequence: the script's own header (bin/fm-landing-remote.sh:35-37) states that an uncatchable SIGKILL can leave the mutating window half written; a SIGKILL betweengit remote rename origin upstreamandgit remote rename fork origin(bin/fm-landing-remote.sh:565-571) leaves upstream=parent, fork=ours, and no origin. verify_remapped_shape then takes the line-450 branch, bin/fm-bootstrap.sh:1195 relays it verbatim as the LANDING_REMOTE remediation, and .agents/skills/bootstrap-diagnostics/SKILL.md documents that line as naming "the command that repairs it". The new test test_drift_refusal_prints_a_repair_that_actually_repairs (tests/fm-landing-remote.test.sh:894) runs the printed repair for the fork branch and the config-drift branch only, so this branch is asserted by no test and the invariant the fix round was given ("every refusal must name a runnable repair") is still violated on it. Earliest supported boundary: in the origin-absent branch name the single git command that reconstructs origin (git -C <repo> remote rename fork originwhen fork exists, otherwisegit -C <repo> remote rename upstream origin) and then apply, or let cmd_apply accept an absent origin when --ours and --upstream fully determine the shape.bin/fm-bootstrap.sh:1195- The LANDING_REMOTE relay always prints its repair command, including in read-only session start. bin/fm-bootstrap.sh:89-98 documents FM_BOOTSTRAP_DETECT_ONLY=1 as the mode fm-session-start.sh uses when another live session holds the fleet lock, "so a second concurrent session never race-mutates PR-check artifacts, secondmate homes, pending handoff outboxes, X-mode artifacts, project clones, or repair instructions", and the TANGLE check immediately above (bin/fm-bootstrap.sh:1176-1181) implements exactly that by switching to advisory-only wording when FM_BOOTSTRAP_DETECT_ONLY=1 and FM_BOOTSTRAP_LOCKED != 1. The landing-remote repair is a heavier mutation than the tangle one: fm-landing-remote.sh apply rewrites the primary's remotes, refetches origin, repoints branch tracking, writes git config, sets the gh default, and re-inits no-mistakes. tests/fm-session-start.test.sh:879 confirms a read-only bootstrap does surface these diagnostics, so a non-lock-holding session is now told to run that remap while the lock holder may be doing the same. Mirror the TANGLE detect-only branch: keep the drift sentence, drop the apply command, in read-only mode.bin/fm-landing-remote.sh:87- bin/fm-landing-remote.sh keeps its own canonical_dir() (used once at line 663) with the same body as the new fm_canonical_dir() in bin/fm-self-repo-lib.sh. Not a defect and outside the four call sites the intent named, but it is the fifth copy of the same physical-path canonicalizer, so noting it as an optional consolidation rather than a required one.🔧 Fix: make origin-absent landing-remote repair runnable and respect read-only bootstrap
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
tests/fm-landing-remote.test.shtests/fm-bootstrap.test.shtests/fm-merge-local.test.shtests/fm-fleet-sync.test.shtests/fm-pr-merge.test.shtests/fm-spawn-pool-base-freshen.test.shtests/fm-spawn-batch.test.shtests/fm-spawn-dispatch-profile.test.shtests/fm-spawn-worktree-settle.test.shEnd-to-end verification ofbin/fm-self-repo-lib.sh(fm_is_firstmate_repo) physical-path resolution across direct, symlink, and relative path inputsEnd-to-end verification ofbin/fm-landing-remote.sh verifydrift detection andbin/fm-bootstrap.shruntimeLANDING_REMOTEwarning and remediation output✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.