Skip to content

fix: unify firstmate repository predicate and check landing remote drift - #11

Open
BohnBawerick wants to merge 4 commits into
mainfrom
fm/fm-landing-remote-followups
Open

fix: unify firstmate repository predicate and check landing remote drift#11
BohnBawerick wants to merge 4 commits into
mainfrom
fm/fm-landing-remote-followups

Conversation

@BohnBawerick

Copy link
Copy Markdown
Owner

Intent

Unify the firstmate-on-itself predicate and wire landing-remote verification, as two follow-ups to the landed fm-origin-points-upstream work.

  1. The "is this firstmate's own repository" predicate was open-coded in four places: bin/fm-merge-local.sh, bin/fm-pr-merge.sh, bin/fm-fleet-sync.sh, and freshen_spawn_worktree_base in bin/fm-spawn.sh. Create a single shared helper (is_firstmate_repo, in a shared helper library or bin/fm-guard.sh) and replace all four open-coded copies, so the local merge, the fleet-sync skip, and the spawn base choice cannot drift apart.
  2. Clean up the redundant/duplicate proj_real path recomputation in bin/fm-spawn.sh.
  3. Wire bin/fm-landing-remote.sh verify into the runtime path / guard checks so a primary checkout where apply was not run is surfaced if the remotes drift.
  4. Add or update tests in tests/fm-landing-remote.test.sh and related suites to verify the shared predicate and the landing-remote verification.
  5. bin/fm-lint.sh must be 100% clean.

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

  • Created bin/fm-self-repo-lib.sh to unify the repository detection predicate and path canonicalization across fm-merge-local.sh, fm-pr-merge.sh, fm-fleet-sync.sh, and fm-spawn.sh.
  • Extended bin/fm-landing-remote.sh verify to support drift verification without explicit URLs and emit runnable repair commands on invalid remote configurations.
  • Integrated landing remote verification into bin/fm-bootstrap.sh to surface actionable LANDING_REMOTE diagnostics 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

================================================================================
FIRSTMATE LANDING-REMOTE AND SELF-REPO PREDICATE VALIDATION EVIDENCE
================================================================================

SECTION 1: Shared Predicate (bin/fm-self-repo-lib.sh: fm_is_firstmate_repo)
--------------------------------------------------------------------------------
Test 1.1 Direct FM_ROOT path: MATCH (exit 0)
Test 1.2 Direct FM_HOME path: MATCH (exit 0)
Test 1.3 Symlinked FM_ROOT path: MATCH (exit 0)
Test 1.4 Unnormalized path (../): MATCH (exit 0)
Test 1.5 Unrelated project path: REJECTED (exit 1)
Test 1.6 Empty project path: REJECTED (exit 1)

SECTION 2: Landing Remote Verification and Runtime Drift Refusal
--------------------------------------------------------------------------------
2.1 fm-landing-remote.sh verify (drift detection mode with no --ours):
Exit code: 1
Stderr: error: a fork remote at https://github.com/BohnBawerick/firstmate.git still exists beside origin https://github.com/upstream-parent/firstmate.git, which is the shape apply exists to remove, so origin may still be the third-party parent; run fm-landing-remote.sh apply --ours https://github.com/BohnBawerick/firstmate.git --upstream https://github.com/upstream-parent/firstmate.git on the primary checkout

2.2 fm-bootstrap.sh surfaces LANDING_REMOTE drift during session startup:
Output: LANDING_REMOTE: a fork remote at https://github.com/BohnBawerick/firstmate.git still exists beside origin https://github.com/upstream-parent/firstmate.git, which is the shape apply exists to remove, so origin may still be the third-party parent; run fm-landing-remote.sh apply --ours https://github.com/BohnBawerick/firstmate.git --upstream https://github.com/upstream-parent/firstmate.git on the primary checkout

2.3 Executing repair via fm-landing-remote.sh apply:
Exit code: 0
Output:
landing-remote: origin now points at the landing remote
origin=https://github.com/BohnBawerick/firstmate.git
upstream=https://github.com/upstream-parent/firstmate.git
fork=absent
gh-default=origin

2.4 fm-landing-remote.sh verify after apply:
Exit code: 0
Stdout:
origin=https://github.com/BohnBawerick/firstmate.git
upstream=https://github.com/upstream-parent/firstmate.git

2.5 fm-bootstrap.sh after repair (no LANDING_REMOTE warnings):
LANDING_REMOTE lines: 0 (clean)

SECTION 3: Targeted Test Suite Executions
--------------------------------------------------------------------------------
- tests/fm-landing-remote.test.sh: PASSED (exit code 0)
- tests/fm-merge-local.test.sh: PASSED (exit code 0)
- tests/fm-fleet-sync.test.sh: PASSED (exit code 0)
- tests/fm-pr-merge.test.sh: PASSED (exit code 0)
- tests/fm-spawn-pool-base-freshen.test.sh: PASSED (exit code 0)

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 neither upstream nor fork exists. 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 with origin=$(remote_url origin); [ -n &#34;$origin&#34; ] || fail &#34;origin remote is absent&#34; 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 between git remote rename origin upstream and git 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 &lt;repo&gt; remote rename fork origin when fork exists, otherwise git -C &lt;repo&gt; 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.sh
  • tests/fm-bootstrap.test.sh
  • tests/fm-merge-local.test.sh
  • tests/fm-fleet-sync.test.sh
  • tests/fm-pr-merge.test.sh
  • tests/fm-spawn-pool-base-freshen.test.sh
  • tests/fm-spawn-batch.test.sh
  • tests/fm-spawn-dispatch-profile.test.sh
  • tests/fm-spawn-worktree-settle.test.sh
  • End-to-end verification of bin/fm-self-repo-lib.sh (fm_is_firstmate_repo) physical-path resolution across direct, symlink, and relative path inputs
  • End-to-end verification of bin/fm-landing-remote.sh verify drift detection and bin/fm-bootstrap.sh runtime LANDING_REMOTE warning and remediation output
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

… 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant