feat(certification): add durable Firstmate final-certification coordinator#983
feat(certification): add durable Firstmate final-certification coordinator#983ChrisGutierrezNet wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1983adc0b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| qroot=$(shell_quote "$fm_root/bin/fm-certification.sh") | ||
| qtask=$(shell_quote "$task") | ||
| qtoken=$(shell_quote "$token") | ||
| message="Certification slot admitted for $task. Run exactly: FM_HOME=$qhome $qroot start $qtask $qtoken" |
There was a problem hiding this comment.
Preserve coordinator settings in admitted start commands
Captain, when the shared coordinator is initialized with non-default settings (for example FM_CERTIFICATION_ROOT/NO_MISTAKES_HOME or FM_CERTIFICATION_CAPACITY>1), this generated “run exactly” command only exports FM_HOME. The worker's start path recomputes the ledger root and calls capacity_ensure, so a worker that doesn't already have those env vars reads the wrong queue or refuses the durable capacity before invoking No Mistakes, leaving the admitted item occupying capacity until manual recovery. Include the coordinator root/capacity in the token-bound command or make start read the durable values without requiring matching ambient env.
AGENTS.md reference: AGENTS.md:L284-L286
Useful? React with 👍 / 👎.
…scalation, retention
The rebase onto main picked up bin/fm-doc-audience-check.sh, which requires every tracked prose surface to be classified. Register the coordinator design doc and its skill so the audience inventory check passes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e1983ad to
b2bbe3d
Compare
Queue status - 2026-07-24 UTCThis PR is open, non-draft, and green; the durable record says final validation passed at the published head. Next decision: maintainer review and merge approval. |
|
Automated reminder: thanks for the PR! This branch currently has a merge conflict with the base branch. When you get a chance, please rebase onto (or merge) the latest base branch, resolve the conflict, and push. After that, checks will re-run and the PR will get looked at again. Noted for firstmate#983 at |
Intent
Implement and publish a safe, durable Firstmate final-certification coordinator that serializes heavy No Mistakes validations, performs strict identity and delivery-mode preflight checks, survives restarts, automatically advances queued work, preserves worker ownership of No Mistakes gates, handles obsolete requests, crashed launches, malformed or version-skewed records, and bounded retention safely, remains backend- and adapter-neutral, and includes tracked design, documentation, skills, AGENTS updates, lint, and focused regression coverage. Push the feature branch to the authorized ChrisGutierrezNet fork and open an upstream PR against kunchenguid/firstmate main; do not merge.
What Changed
bin/fm-certification.sh, a restart-durable coordinator that serializes heavy No Mistakes validations behind a machine-global lock, enforces strict identity and delivery-mode preflight checks (detached HEAD, wrong branch, changed HEAD, dirty work, non-no-mistakes delivery refusal), and auto-advances queued work in FIFO order while preserving worker ownership of the No Mistakes gates.certification-coordinatorskill andharness-adaptersnote,AGENTS.md/CONTRIBUTING.md/config/scripts docs, a design doc (docs/certification-coordinator.md), touch-ups tofm-brief.sh,fm-session-start.sh,fm-test-run.sh, andfm-watch.sh, plus 18 focused regression tests intests/fm-certification.test.sh(all passing).Risk Assessment
✅ Low: The change is a well-bounded, single-owner shell coordinator that matches its documented invariants and ships comprehensive regression coverage for every state path; the only findings are minor, non-blocking hardening/simplification notes.
Testing
Baseline: the focused suite
tests/fm-certification.test.shpasses all 18 cases covering the coordinator's safety invariants. To show the intent working the way an operator experiences it, I built and ran a CLI transcript that drives the real script against real git worktrees through the complete lifecycle — capacity-1 serialized admission, a single token-bound worker steer, worker-ownedstart, authoritative terminal reconciliation, and automatic FIFO advancement — and demonstrates the safety boundaries (direct-PR delivery refused, dirty unpublished work preserved with HEAD unchanged, and a call log proving the coordinator issued only read-only no-mistakes inspections and never a respond/abort/rerun/sync/recover/daemon/run command). Adjacent suites touched by the diff (fm-watch-triage, fm-watch-checkpoint, fm-brief, fm-test-run) pass. One fm-session-start assertion fails, but it reproduces identically on the base commit (a tmux window-relaunch liveness check in this sandbox) and the test file is unchanged by the diff, so it is pre-existing and unrelated. The only unmet part of the intent is the publish step: no upstream PR exists on kunchenguid/firstmate and origin is the upstream rather than the ChrisGutierrezNet fork — surfaced as an ask-user finding since performing the push/PR is outside testing scope. No visual/UI surface is involved (this is a shell CLI), so the reviewer-visible evidence is the captured CLI transcript rather than a screenshot. The working tree was left clean.Evidence: Operator CLI transcript: full certification lifecycle + safety invariants
Evidence: Reproducible demo harness driving the real coordinator
Evidence: fm-certification.test.sh — 18/18 focused regression tests pass
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-certification.sh:297- no-mistakes CLI reads are deliberately time-bounded via run_bounded()/timeout because they can hang, but the surrounding git commands in preflight and inspect_record_run are not — notablygit status --porcelain=v1 --untracked-files=all(line 297) and the rev-parse/merge-base calls (lines 289-296, 360-362). All of these execute while holding the single machine-global coordinator lock ($CERT_ROOT/.lock) that serializes every FM_HOME's certification work. A slow or wedged git in one worktree (huge untracked tree, stuck index/fs) therefore stalls the coordinator for all homes, undercutting the same availability property the timeout wrapping was added to protect. Consider running the git identity/cleanliness probes through the same bounded helper.bin/fm-certification.sh:236- record_field() (line 129) and meta_value() (line 236) are byte-for-byte identical grep|tail|cut helpers. They can be collapsed to a single helper (or one aliased to the other) to remove the duplication; the two names can be kept as thin wrappers if the call-site semantics are worth preserving.docs/certification-coordinator.md:1- The intent's publish requirement — push the feature branch to the authorized ChrisGutierrezNet fork and open an upstream PR against kunchenguid/firstmate main — is not verifiably satisfied from the test environment.gh-axi search prsfor a certification-coordinator PR and for author:ChrisGutierrezNet on kunchenguid/firstmate both return 0, and the worktree'soriginpoints at kunchenguid/firstmate (the upstream), not a ChrisGutierrezNet fork. The target commit is not on any remote branch. Opening/pushing a PR is an outward-facing, hard-to-reverse action outside testing scope, so I did not perform it. The user needs to confirm the branch was pushed to the fork and the upstream PR opened (and left unmerged).tests/fm-session-start.test.sh:1- tests/fm-session-start.test.sh has one failing assertion ('the later fleet read did not confirm the relaunched window' — a tmux window-relaunch liveness check). It reproduces identically on the base commit 10ee779 (the test file is unchanged by this diff), so it is a pre-existing environmental/tmux-sandbox failure, not a regression from the certification change. No action needed for this change.bash tests/fm-certification.test.sh— all 18 focused regression tests pass (concurrent admission, restart/FIFO advance, detached/wrong-branch/changed-head preflight, obsolete custody, dirty-work preservation, uncoordinated-run block, non-no-mistakes delivery refusal, malformed/version-skew quarantine, launch-grace stall/withdraw/retry, bounded terminal/quarantine/withdrawn retention)Ran a custom operator CLI transcript (/tmp/no-mistakes-evidence/01KYA7YMYR2CYGGRT4C4MGRK6Y/demo-lifecycle.sh) driving the realbin/fm-certification.shthrough enqueue → status → notify → start → reconcile terminal → FIFO advance, plus direct-PR refusal and dirty-work preservation, verifying only read-only no-mistakes inspections were issuedbash tests/fm-watch-triage.test.sh,bash tests/fm-watch-checkpoint.test.sh,bash tests/fm-brief.test.sh,bash tests/fm-test-run.test.sh— all pass (affected/adjacent scripts touched by the diff)bash tests/fm-session-start.test.sh— 1 pre-existing tmux-liveness failure; confirmed identical failure on base commit 10ee779 viagit checkout <base> -- bin/fm-session-start.sh tests/fm-session-start.test.sh(then restored), establishing it is environmental, not a regressiongh-axi search prs "certification coordinator repo:kunchenguid/firstmate"andauthor:ChrisGutierrezNet— 0 results;git remote -vandgit branch -r --contains <target>confirm no upstream PR/push yet✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.