Repository navigation
feat(bin): prepare firstmate for no-mistakes 1.62+ and gh 2.99+ PR screenshots - #33
Merged
Merged
Conversation
…reenshots - fm-crew-state: map passed-with-override and passed-with-skips like passed, and ci-monitor-interrupted to blocked so firstmate checks the still-open PR, instead of reading newer no-mistakes outcomes as unknown. - fm-brief: every PR-based ship brief tells the worker to post screenshots it captures itself as one PR comment with `gh pr comment --attach`, never committed and never cited by local path. - fm-bootstrap: raise NO_MISTAKES_MIN to 1.62.0 and add a gh floor of 2.99.0; an outdated tool is a MISSING diagnostic (bootstrap still exits 0), and an installed-but-old no-mistakes names its guarded `no-mistakes update`. - Point the toolchain docs and bootstrap-diagnostics at the new floors, and bump test stand-ins to current versions.
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
Make the small firstmate change that upgrading no-mistakes past v1.48 needs, so screenshots become visible on PRs. This is firstmate's shared tracked material (firstmate-coding-guidelines applies). The captain (2026-09-22): "lets generally evaluate updating ourt tooling and in low risk cases do it", after learning that no-mistakes v1.62+ uploads PR screenshots as GitHub attachments and gh 2.99+ has
gh pr comment --attach. The plan is a scout report's section 4c, items 1-4:gh pr comment <url> --body-file <md> --attach <file>...(gh >= 2.99; plain gh, not gh-axi, because gh-axi has no --attach), and never to commit them or cite local paths. Defined once and interpolated; tests updated.no-mistakes update(guarded in-place upgrade that refuses while a validation run is active) rather than a fresh curl install. Test stand-in fakes in other suites that pretended to be no-mistakes v1.31.2 or a version-less gh were bumped so their silent-bootstrap assertions still hold. Toolchain docs (docs/configuration.md, CONTRIBUTING.md) and the bootstrap-diagnostics skill were updated to the new floors.Constraints: do NOT upgrade no-mistakes or gh and do not restart the no-mistakes daemon - firstmate does the upgrades separately. Keep the change minimal and shellcheck-clean. Known unrelated pre-existing test failures: fm-session-start "successful Herdr husk recovery should stay non-actionable" fails on main too; fm-teardown fork-allow fails only in a non-detached worktree on this host because of a host tangle/secondmate environment condition.
What Changed
bin/fm-crew-state.shnow maps the newer no-mistakes run outcomes.passed-with-overrideandpassed-with-skipsread asdone, likepassed, and the detail line names the outcome.ci-monitor-interruptedreads asblocked, with a detail saying the PR is still open and its CI and merge state need checking. Before this, all three came out asunknown. New tests intests/fm-crew-state.test.shcover them.AGENTS.mdnow points to the script header for the outcome-to-state mapping instead of keeping its own copy.bin/fm-bootstrap.shraisesNO_MISTAKES_MINfrom 1.31.2 to 1.62.0 and adds a newGH_MIN=2.99.0floor. An older no-mistakes or gh now prints the usualMISSING:diagnostic, and bootstrap still exits 0. If no-mistakes is installed but too old, the suggested command isno-mistakes updateinstead of the curl install script. New tests cover the gh floor. Test fakes in other suites that reported no-mistakes 1.31.2 or a gh with no version now report newer versions, so their silent-bootstrap checks still pass.docs/configuration.md,CONTRIBUTING.mdand the bootstrap-diagnostics skill now list the new floors.bin/fm-brief.shdefines oneSCREENSHOTSline and inserts it into the briefs for the no-mistakes, direct-PR and direct-PR native-stack ship modes. It tells workers to post screenshots they captured as a single comment right after the PR opens, usinggh pr comment <PR url> --body-file <md> --attach <file>...(gh >= 2.99, plain gh rather than gh-axi). It also tells them never to commit the images or cite local paths. Local-only briefs don't get the line. A test intests/fm-brief.test.shchecks that the line appears exactly once in each PR-based brief and not at all in local-only.🤖 Generated with Claude Code
Risk Assessment
✅ Low: The change is small and meets all four intent items. New outcome words map as specified with tests that run the script. The screenshot line is defined once and appears only in the PR-based ship sections. The new version floors produce a MISSING line and exit 0, and nothing in bin/ acts on MISSING lines automatically. The two findings are informational: a teardown race that fails safe, and one branch without a test.
Testing
I ran the three changed suites (crew-state, brief, bootstrap), and all pass; their new tests fail against the base code. The seven other suites whose fake tools were bumped, plus fm-pr-check-security, also pass. fm-session-start fails only on the known Herdr husk test, which fails the same way on the untouched base, and its other 51 tests pass. Manual before/after runs of the real scripts produced three pieces of evidence: a crew-state transcript, a bootstrap transcript using this host's real outdated gh and no-mistakes, and a diff of the generated briefs. Together they show every item in the intent working, and neither tool was upgraded. The change has no UI, so the evidence is CLI transcripts and generated brief text, not screenshots.
Evidence: fm-crew-state.sh outcome mapping, base vs target (new no-mistakes words: unknown -> done/blocked)
Source: fm-crew-state.sh outcome mapping, base vs target (new no-mistakes words: unknown -> done/blocked)
Evidence: fm-bootstrap.sh with the host's real gh 2.95.0 / no-mistakes v1.48.0: MISSING diagnostics, exit 0, `no-mistakes update` install path
Source: fm-bootstrap.sh with the host's real gh 2.95.0 / no-mistakes v1.48.0: MISSING diagnostics, exit 0,
no-mistakes updateinstall pathEvidence: Generated worker briefs base vs target: screenshot line in no-mistakes/direct-PR/stack, local-only unchanged
Source: Generated worker briefs base vs target: screenshot line in no-mistakes/direct-PR/stack, local-only unchanged
Evidence: Bootstrap on this host after the change
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-teardown.sh:1436-task_status_is_terminal_runin fm-teardown.sh still accepts onlycancelled|failed|passed|checks-passed. It is the only other place in bin/ that lists no-mistakes outcome words, and it was not taughtpassed-with-override,passed-with-skipsorci-monitor-interrupted. It only runs right afterconclude_task_no_mistakes_runhas confirmed the run was parked with no outcome and then issuedaxi abort. A newer outcome can show up only if the run finishes in that roughly 10-second window. If that happens, teardown fails safe with a misleading "still parked after axi abort" REFUSED instead of acting wrongly. It is not practically reachable, and the intent asks for a minimal change, so this is noted rather than required. Adding the three words would make it consistent with fm-crew-state.sh.bin/fm-bootstrap.sh:775-install_cmd no-mistakesnow branches oncommand -v no-mistakes, but only the installed-and-outdated branch is tested (test_no_mistakes_min_version). No test covers a home with no no-mistakes at all, which must still print the curl installer. Theinstallsubcommand runs this string througheval, so if the condition were inverted it would runno-mistakes updateon a machine without the binary, and the suite would not catch it. Suggested fix: add a case to tests/fm-bootstrap.test.sh that removes$fakebin/no-mistakesand checks that the output is exactly theMISSING: no-mistakes (install: curl -fsSL .../docs/install.sh | sh)line.✅ **Test** - passed
✅ No issues found.
bash tests/fm-crew-state.test.sh(all pass, including the new tests for the newer outcome words and for ci-monitor-interrupted)bash tests/fm-brief.test.sh(all pass, including the new test that PR-based ships post worker screenshots as one attachment comment)bash tests/fm-bootstrap.test.sh(all pass, including the gh and no-mistakes minimum-version tests)Regression check: the new fm-crew-state, fm-brief and bootstrap version-floor tests, run against the base fdafbfbbin/in a scratch copy, all fail as expectedSuites whose stand-in tools were bumped:tests/fm-secondmate-harness.test.sh,fm-secondmate-liveness,fm-secondmate-sync,fm-shared-captain-inheritance,fm-startup-memory-budget,fm-x-mode, plusfm-pr-check-security(it fakes gh and bootstrap now callsgh --version): all passbash tests/fm-session-start.test.sh: stops at the known 'successful Herdr husk recovery should stay non-actionable' failure. The untouched base fdafbfb tree fails the same way, and the other 51 tests pass on the target with only that call removed (scratch copy)Manual: ran the realbin/fm-crew-state.sh feat-demoat base and at target, against fakeno-mistakes axi statusoutput for passed, passed-with-override, passed-with-skips, ci-monitor-interrupted (with status ci_monitor_interrupted), checks-passed and failedManual: ran the realbin/fm-bootstrap.shat base and at target with the host's real gh 2.95.0 and no-mistakes v1.48.0 (only--versionwas forwarded to them), with already-upgraded tools, and with no no-mistakes installed; also ranfm-bootstrap.sh install no-mistakesthrough a wrapper that only logs the callManual: generated briefs withbin/fm-brief.sh <id> demo-proj --mode no-mistakes|direct-PR|direct-PR --stack|local-onlyat base and at target and diffed themRead-only upstream checks viagh api/gh search code: no-mistakesoutcomeFor/outcomeForRun, the 1.62.0 changelog entry 'attach media evidence to GitHub pull requests', the v1.48.0internal/updateactive-run refusal, and gh v2.99.0'spr comment --attachgit grepof the shared tracked material for screenshot/recording guidance (item 4 no-op)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.