feat(bin): pre-flight the gate fork-push target at session start#633
Open
zachlandes wants to merge 5 commits into
Open
feat(bin): pre-flight the gate fork-push target at session start#633zachlandes wants to merge 5 commits into
zachlandes wants to merge 5 commits into
Conversation
* A gate-initialized repo whose upstream the current gh user cannot push to must route the push step to a writable fork via repos.fork_url in the local gate DB. That routing is a one-time hand-applied init step, absent from a fresh DB or new home and checked by nothing, so a missing fork target surfaced only as a 403 at push (step 7 of 9) after the whole review+test+doc+lint pipeline had already run - 5 of 20 historical runs died exactly this way. * Add bin/fm-gate-fork-assert.sh: for each repo whose upstream is unwritable, assert a push-writable fork_url and emit a one-line GATE_FORK remediation otherwise. Read-only, best-effort, silent when it cannot decide (no DB, no sqlite3, gh absent or unauthenticated). * Wire it into fm-bootstrap.sh as a detect line beside the TANGLE guard, add GATE_FORK handling to bootstrap-diagnostics, and reconcile AGENTS.md section 6 with CONTRIBUTING.md's --fork-url init requirement.
* bin/fm-bootstrap.sh now runs bin/fm-gate-fork-assert.sh, which reads the machine-global no-mistakes gate DB and probes each repo's push access over the real gh network. Bootstrap-invoking tests that do not stub gh (e.g. the X-mode activation cases) would otherwise reach the network and the developer's real ~/.no-mistakes DB, adding ~3.5s per bootstrap and making those unit tests network-dependent. * Default FM_GATE_STATE_DB to a guaranteed-absent path in tests/lib.sh, so the check is an instant silent no-op in every test unless one sets its own synthetic DB - mirroring the existing FM_GATE_REFUSE_BYPASS exemption.
…en test hermeticity
…w, diagnostics router
zachlandes
force-pushed
the
fm/fork-push-preflight
branch
from
July 16, 2026 07:41
ccc1dab to
626e7c3
Compare
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
Implement Fix 1 from the fm-pr-stuck-w4 scout report: pre-flight the no-mistakes fork-push target so a firstmate-repo ship never runs the whole review+test+doc+lint pipeline and only then 403s at the push step (step 7 of 9) because the gate has no writable fork_url. Historically 5 of 20 runs died this way after the full pipeline had already run.
New bin/fm-gate-fork-assert.sh reads the gate DB (FM_GATE_STATE_DB overridable) and, for each gate-initialized repo whose upstream the current gh user cannot push to, asserts a push-writable fork_url; it emits one GATE_FORK: line per offending repo with a derived 'no-mistakes init --fork-url git@github.com:/.git' remediation and exits non-zero. It is deliberately read-only, best-effort, and a SILENT exit-0 no-op when it cannot decide (no DB, no sqlite3, gh absent or unauthenticated - NEEDS_GH_AUTH already owns that; a non-github upstream it cannot verify). Push-access verdicts are memoized so repos sharing an upstream cost one gh call; the code is intentionally bash 3.2 safe (no associative arrays, process-substitution loop, no heredoc-in-command-substitution).
Wired into bin/fm-bootstrap.sh as a detect line beside the existing TANGLE guard (best-effort, never fatal), surfaced through the session-start digest as a GATE_FORK: line; bootstrap header documents it. Added GATE_FORK handling to the bootstrap-diagnostics skill (a captain-owned local fix; relay the command, rerun session start to confirm it clears; never blocks a scout or already-routed repo). Reconciled AGENTS.md section 6's init line with CONTRIBUTING.md's --fork-url requirement under the one-owner rule (CONTRIBUTING owns onboarding detail, the script header owns the mechanism, bootstrap-diagnostics owns the response, AGENTS.md states it concisely and cross-references).
Colocated test tests/fm-gate-fork-assert.test.sh (6 cases) via a gh PATH-shim and synthetic gate DBs covers: fires on unset fork_url + unwritable upstream and on set-but-unwritable fork; stays quiet on writable fork, writable upstream, and non-github upstream; silent no-op on missing DB / unauthenticated gh.
Deliberate hermeticity decision: because bootstrap now does real gh network I/O via this check, tests/lib.sh defaults FM_GATE_STATE_DB to a guaranteed-absent path so every bootstrap-invoking test stays a hermetic instant no-op (mirroring the existing FM_GATE_REFUSE_BYPASS exemption); only the dedicated test sets its own synthetic DB.
This is a firstmate-repo change shipping through firstmate's own no-mistakes gate to a cross-repo fork PR for the captain to merge. The fork routing is healthy right now, so the ship itself will not 403.
What Changed
bin/fm-gate-fork-assert.sh: it reads the local no-mistakes gate DB (FM_GATE_STATE_DBoverridable) and, for each gate-initialized repo whose upstream the currentghuser cannot push to, asserts a push-writablefork_url— printing oneGATE_FORK:line per offending repo with a derivedno-mistakes init --fork-url ...remediation, so a ship can no longer run the full review+test+doc+lint pipeline and only then 403 at the push step. The check is read-only, memoizes push verdicts so repos sharing an upstream cost oneghcall, and stays silent with exit 0 whenever it cannot decide (no DB, nosqlite3,ghabsent or unauthenticated, a non-github upstream, or a failed API probe). It's wired intobin/fm-bootstrap.shas a best-effort, never-fatal detect line beside the TANGLE guard; thebootstrap-diagnosticsskill,AGENTS.mdsection 6, anddocs/scripts.mddocument the line and its captain-owned fix.tests/fm-gate-fork-assert.test.sh— 7 cases over aghPATH-shim and synthetic gate DBs, covering both firing cases (unsetfork_url, set-but-unwritable fork), the quiet cases (writable fork, writable upstream, non-github upstream), and the silent no-op paths. Because bootstrap now performs realghnetwork I/O,tests/lib.shunconditionally exportsFM_GATE_STATE_DBto a guaranteed-absent path so every bootstrap-invoking test stays a hermetic instant no-op, mirroring the existingFM_GATE_REFUSE_BYPASSexemption.^[❯›]to the alternation^(❯|›): under a byte-oriented locale, grep walks a bracket class byte-wise, so any row whose first byte is0xE2— including the composer's box-drawing borders╭ ╰ │— matched as a bare prompt.docs/configuration.mdnow records that overrides must be spelled as an alternation.Risk Assessment
✅ Low: The two prior info-level findings were fixed correctly and completely — undecidable gh probes now record an explicit
unknownverdict and stay silent, and the test hermeticity export is now unconditional — with all six original test cases preserved, a new regression test added, and the bootstrap wiring/docs untouched, introducing no new issues.Testing
Ran the full configured suite (1351 ok, 0 failures, exit 0) plus product-level verification of the fork-push pre-flight against the real gate DB and real authenticated gh: it stays silent/exit-0 on today's healthy routing, and on a DB carrying the same real repo rows with the --fork-url routing missing it emits the GATE_FORK line whose derived remediation matches the fork_url the real DB actually carries, corroborated by GitHub reporting push=false on kunchenguid/firstmate and push=true on zachlandes/firstmate. Drove the real session-start digest in a hermetic fake home to capture the captain-facing before/after transcript (line appears, then clears once the fix is applied), and confirmed read-only behavior, non-fatal bootstrap wiring, every silent cannot-decide path, bash 3.2 safety, memoization, and the tests/lib.sh hermeticity decision (0 gh probes vs 6 real 4-second round-trips without it). Round 1's failure was investigated rather than merely re-run: it is not a flake but a deterministic LC_CTYPE=C + BSD-grep bug that commit 2a74d40 fixes — reproduced exactly by forcing the pre-fix regex back through its env override, green with the committed default, and stable across 5 consecutive runs of both composer test files. No visual artifact applies: the change's only end-user surface is a CLI line in the session-start digest, so terminal transcripts are the faithful medium. Worktree left clean; no code changes were needed.
Evidence: Session-start digest (the end-user surface): GATE_FORK line appears, then clears after the printed fix
=== BEFORE: gate DB has no fork_url for /Users/zacharylandes/code/firstmate (a fresh gate DB, a reinstall, or a new firstmate home) === $ bin/fm-session-start.sh BOOTSTRAP -------------------------------------------------------------------------------- MISSING: tasks-axi (install: npm install -g tasks-axi) MISSING: quota-axi (install: npm install -g quota-axi) GATE_FORK: /Users/zacharylandes/code/firstmate: no push access to kunchenguid/firstmate and no fork target; the next ship 403s at push after the full pipeline. Fix: (cd /Users/zacharylandes/code/firstmate && no-mistakes init --fork-url git@github.com:zachlandes/firstmate.git) === The captain runs the printed fix: (cd /Users/zacharylandes/code/firstmate && no-mistakes init --fork-url git@github.com:zachlandes/firstmate.git) ...which writes fork_url into the gate DB. Rerun session start to confirm: === $ bin/fm-session-start.sh BOOTSTRAP -------------------------------------------------------------------------------- MISSING: tasks-axi (install: npm install -g tasks-axi) MISSING: quota-axi (install: npm install -g quota-axi)Evidence: Pre-flight against the REAL gate DB and REAL gh: healthy today (silent, exit 0) vs broken routing (GATE_FORK, exit 1), with the memoized probe log
gh user: zachlandes $ sqlite3 ~/.no-mistakes/state.sqlite 'SELECT working_path, upstream_url, fork_url FROM repos' /Users/zacharylandes/code/firstmate/projects/help-connect-platform | https://github.com/Acelero-Inc/help-connect-platform.git | (NO FORK URL) /Users/zacharylandes/code/help-connect-platform | https://github.com/Acelero-Inc/help-connect-platform.git | (NO FORK URL) /Users/zacharylandes/code/firstmate/projects/lavish-axi | https://github.com/kunchenguid/lavish-axi | https://github.com/zachlandes/lavish-axi.git /Users/zacharylandes/code/firstmate | https://github.com/kunchenguid/firstmate | https://github.com/zachlandes/firstmate.git /Users/zacharylandes/code/firstmate-herdr/projects/herdr-stash | https://github.com/zachlandes/herdr-stash.git | (NO FORK URL) /Users/zacharylandes/code/firstmate-herdr | https://github.com/kunchenguid/firstmate | https://github.com/zachlandes/firstmate.git -------------------------------------------------------------------------------- 1. HEALTHY (today's real routing) -------------------------------------------------------------------------------- $ bin/fm-gate-fork-assert.sh (no output) exit=0 real gh probes made for those 6 repos (memoized - repos sharing an upstream cost ONE call: help-connect-platform x2 -> 1, kunchenguid/firstmate x2 -> 1): gh auth status gh api user --jq .login gh api repos/Acelero-Inc/help-connect-platform --jq .permissions.push // false gh api repos/kunchenguid/lavish-axi --jq .permissions.push // false gh api repos/zachlandes/lavish-axi --jq .permissions.push // false gh api repos/kunchenguid/firstmate --jq .permissions.push // false gh api repos/zachlandes/firstmate --jq .permissions.push // false gh api repos/zachlandes/herdr-stash --jq .permissions.push // false -------------------------------------------------------------------------------- 2. BROKEN (a fresh gate DB / reinstall / new firstmate home): the same real repos, with the one-time --fork-url routing not yet applied. -------------------------------------------------------------------------------- $ FM_GATE_STATE_DB=<that DB> bin/fm-gate-fork-assert.sh GATE_FORK: /Users/zacharylandes/code/firstmate: no push access to kunchenguid/firstmate and no fork target; the next ship 403s at push after the full pipeline. Fix: (cd /Users/zacharylandes/code/firstmate && no-mistakes init --fork-url git@github.com:zachlandes/firstmate.git) GATE_FORK: /Users/zacharylandes/code/firstmate-herdr: no push access to kunchenguid/firstmate and no fork target; the next ship 403s at push after the full pipeline. Fix: (cd /Users/zacharylandes/code/firstmate-herdr && no-mistakes init --fork-url git@github.com:zachlandes/firstmate.git) exit=1 The derived fix matches the routing the real DB actually carries today (https://github.com/zachlandes/firstmate.git), so following the printed line restores exactly the healthy state above. lavish-axi, whose fork IS writable, stays quiet in the same run.Evidence: Round 1's "flake" was a deterministic C-locale bug that commit 2a74d40 fixes (reproduced via the regex's env override, no code edited)
Round 1 reported the herdr/cmux composer bare-prompt case as a FLAKE. It is not flaky - it is deterministic under this environment's locale. $ locale | head -2 -> LANG="" LC_COLLATE="C" (LC_CTYPE="C") tests resolve grep via BASE_PATH=/usr/bin:/bin -> /usr/bin/grep (BSD) === mechanism: a bracket class is walked BYTE-wise under LC_CTYPE=C === the composer's TOP BORDER row: ╭────────────────────────╮ (╭ = bytes e2 95 ad) OLD '^[❯›]' : MATCHES the border row -> a border is misread as a bare ❯ prompt (the bug) NEW '^(❯|›)': correctly does NOT match the border row NEW '^(❯|›)': still matches a real ❯ prompt glyph === end-to-end, against the real test case (tests/fm-backend-herdr.test.sh) === --- pre-fix regex forced back in via its env override (no code edited) --- not ok - a bare prompt glyph should read as empty, got 'pending' --- this commit's default regex --- ok - fm_backend_herdr_composer_state: a bare '❯' composer row reads emptyEvidence: Hermeticity of the tests/lib.sh FM_GATE_STATE_DB export: 0 gh probes vs 6 real network round-trips without it
Hermeticity of bootstrap-invoking tests (tests/lib.sh export), on a machine that HAS a real gate DB at ~/.no-mistakes/state.sqlite with 6 real repos: --- WITH tests/lib.sh sourced (every test's env) --- FM_GATE_STATE_DB=<worktree>/tests/.no-such-dir/gate-absent.sqlite path is guaranteed-absent -> guard exits before any gh call bootstrap wall time: 841ms gh api push probes made: 0 --- COUNTERFACTUAL: same guard with the export removed (reads the real machine DB) --- guard wall time against the real DB: 4003ms gh api push probes made: 6 (those are real network round-trips every bootstrap-invoking test would have paid)Evidence: Round 1's assertion re-checked: 5 consecutive runs of both composer-state test files, all green
Round 1's assertion, 5 consecutive runs of both composer-state test files: run 1: herdr exit=0 (101 ok, 0 not ok) | cmux exit=0 (55 ok, 0 not ok) | bare-prompt case: 2 ok run 2: herdr exit=0 (101 ok, 0 not ok) | cmux exit=0 (55 ok, 0 not ok) | bare-prompt case: 2 ok run 3: herdr exit=0 (101 ok, 0 not ok) | cmux exit=0 (55 ok, 0 not ok) | bare-prompt case: 2 ok run 4: herdr exit=0 (101 ok, 0 not ok) | cmux exit=0 (55 ok, 0 not ok) | bare-prompt case: 2 ok run 5: herdr exit=0 (101 ok, 0 not ok) | cmux exit=0 (55 ok, 0 not ok) | bare-prompt case: 2 okEvidence: GitHub's real push permissions, corroborating the guard's verdicts
=== corroboration: the guard's verdicts match GitHub's real answer === kunchenguid/firstmate permissions: push=false admin=false zachlandes/firstmate permissions: push=true admin=true -> the gate cannot push branches to kunchenguid/firstmate (push=false): that is the 403 at step 7 of 9 the pre-flight now predicts at session start instead.Evidence: Reproduction driver for the session-start digest evidence (hermetic fake FM_HOME/FM_ROOT, stubbed gh)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed ✅
bin/fm-gate-fork-assert.sh:81- gate_can_push cannot distinguish a failed/rate-limitedgh api repos/<slug>call from a genuinepush:falseresponse:2>/dev/null || truecollapses both to emptyperm, which line 82 records as verdictnopush. A transient gh failure at session start (rate limit, brief offline, VPN handoff) therefore flips a healthy repo to 'no push access'. The sharpest case: a direct-push repo the captain owns with a NULL fork_url is flagged 'no push access to <upstream> and no fork target; the next ship 403s ... Fix: no-mistakes init --fork-url ...', i.e. it tells the captain to add a fork their healthy repo does not need. This is largely mitigated because bootstrap-diagnostics instructs 'rerun session start to confirm the line clears before dispatching', so a transient false positive self-clears on rerun; surfacing for author awareness since a failed API call could instead be treated as a 'cannot decide' silent skip rather than a confident nopush verdict.bin/fm-gate-fork-assert.sh:93- The guard runs on every session start (including FM_BOOTSTRAP_DETECT_ONLY read-only sessions, since line 635 is above the detect-only gate) and makes serialized, un-timedgh apicalls: onegh api userplus onegh api repos/<slug>per distinct upstream/fork slug. Memoization only dedups shared upstreams, so a firstmate home with N distinct gate-repo upstreams pays ~N serial network round-trips added to the bootstrap hot path even when every repo is healthy, with no cross-session cache and no timeout (macOS has notimeoutbuiltin), so a slow/rate-limited GitHub API stalls session start. The intent explicitly accepts that bootstrap now does real gh network I/O; noting the per-repo scaling and lack of any latency safeguard as an accepted tradeoff worth confirming.tests/lib.sh:49- FM_GATE_STATE_DB is defaulted with${FM_GATE_STATE_DB:-...}, so a value already exported in the environment is preserved rather than overridden. The comment says this mirrors the FM_GATE_REFUSE_BYPASS exemption, but that one (line 35) is an unconditionalexport FM_GATE_REFUSE_BYPASS=1. If a developer (or a runner) has FM_GATE_STATE_DB exported, every bootstrap-invoking test would hit the real gate DB and network - exactly what the hermeticity is meant to prevent. The dedicated test sets FM_GATE_STATE_DB per-command (env prefix), not via pre-export, so making the default unconditional like FM_GATE_REFUSE_BYPASS would strictly harden hermeticity with no downside.🔧 Fix: treat undecidable gh push probes as silent, harden test hermeticity
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed ✅
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: fix(herdr): byte-safe bare-prompt regex for C locale composer scan
✅ Re-checked - no issues remain.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"for t in tests/*.test.sh; do bash "$t"; done— full configured suite: 1351 ok, 0 failures, exit 0; round 1's failure did not recurbash tests/fm-gate-fork-assert.test.shand/bin/bash tests/fm-gate-fork-assert.test.sh— all 7 cases pass, including under bash 3.2.57 (the intent's stated bash-3.2-safety constraint)bin/fm-gate-fork-assert.shagainst the REAL ~/.no-mistakes/state.sqlite with the REAL authenticated gh — silent, exit 0 (fork routing healthy), 6 memoized probes for 6 reposFM_GATE_STATE_DB=<broken-routing DB built from the real firstmate/firstmate-herdr/lavish-axi rows> bin/fm-gate-fork-assert.shwith real gh — 2 GATE_FORK lines, exit 1, derived fix matches the real DB's actual fork_url; the writable-fork repo stays quiet in the same runManual: drove the realbin/fm-session-start.shin a hermetic fake FM_HOME/FM_ROOT (mirroring tests/fm-session-start.test.sh's world builder) before and after applying the printed --fork-url fix — GATE_FORK appears in the digest's BOOTSTRAP section, then clearsgh api repos/kunchenguid/firstmate --jq .permissions/repos/zachlandes/firstmate— push=false vs push=true, corroborating the guard's verdicts against GitHub's real answerManual: reproduced round 1's exact failure withFM_BACKEND_HERDR_BARE_PROMPT_RE='^[❯›]' bash tests/fm-backend-herdr.test.sh("not ok - a bare prompt glyph should read as empty, got 'pending'") vs ok with the committed default — no code editedManual: 5 consecutive runs each ofbash tests/fm-backend-herdr.test.sh(101 ok) andbash tests/fm-backend-cmux.test.sh(55 ok) — stable, 0 failuresManual: read-only proof — sha256 of both the real and synthetic gate DBs unchanged across firing and healthy runs; no fm-gate-fork.* temp cache left behindManual: best-effort wiring —bin/fm-bootstrap.shexits 0 while the guard exits 1 and still prints both GATE_FORK linesManual: cannot-decide no-op paths — sqlite3 absent from PATH, gh absent from PATH, missing DB, unauthenticated gh, failed gh probe, non-github upstream all exit 0 silentlyManual: hermeticity — with tests/lib.sh sourced, FM_GATE_STATE_DB points at a guaranteed-absent path and bootstrap makes 0 gh probes; without the export the guard makes 6 real network round-trips (4.0s) against the real machine DBgit status --porcelain— worktree clean, no test artifacts or code edits left behinddocs/scripts.md:8- Out-of-scope follow-up, not caused by this change: the bin/ toolbelt table has pre-existing omissions beyond the row I added.fm-lint.sh,fm-cd-pretool-check.sh,fm-cd-command-policy.mjs,fm-install-shellcheck.sh, andfm-transition-lib.shhave no row, even thoughfm-cd-pretool-check.sh/fm-cd-command-policy.mjsare exact analogues of the listedfm-arm-pretool-check.sh/fm-arm-command-policy.mjspair. All butfm-install-shellcheck.share described in another doc (CONTRIBUTING.md, docs/cd-guard.md, docs/herdr-backend.md), so nothing is undocumented — the inventory is just incomplete. Worth one sweep to either backfill the rows or state the table's inclusion rule; I left it alone here to avoid an unrelated edit.AGENTS.md:762- Judgment call worth surfacing. Thebootstrap-diagnosticsrouter entry duplicated the skill description's literal list of bootstrap lines, and this change updated the skill copy but not this one — the drift that omitted GATE_FORK. I removed the enumeration and pointed at the owners (the skill's trigger list; bin/fm-bootstrap.sh's header for which lines exist) instead of adding GATE_FORK to a second copy, per the one-owner rule. The trigger is unaffected — 'any diagnostic or capability line at all' plus 'silence needs no load' is already binary, the enumeration added no operative force, and .claude/skills symlinks to .agents/skills so the skill's own description is surfaced in the harness listing anyway. This also matches the section's style: entries 763-769 describe trigger situations without enumerating literal tokens. If the enumeration was intentional redundancy for an always-loaded surface, revert to simply addingGATE_FORK:to the list.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.