Skip to content

feat(bin): prepare firstmate for no-mistakes 1.62+ and gh 2.99+ PR screenshots - #33

Merged
SchokoShake merged 2 commits into
mainfrom
fm/fm-tooling-screenshots
Sep 22, 2026
Merged

SchokoShake merged 2 commits into
mainfrom
fm/fm-tooling-screenshots

Conversation

@SchokoShake

Copy link
Copy Markdown
Owner

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:

  1. bin/fm-crew-state.sh outcome mapping: add the newer no-mistakes result words - passed-with-override and passed-with-skips mapped like passed; ci-monitor-interrupted mapped to a state that sends firstmate to check the still-open PR (chosen: blocked, since the daemon restart left the PR open but no longer rebased/fixed by the pipeline) - with colocated tests, so a newer no-mistakes never reads as unknown. This must land before the upgrade.
  2. bin/fm-brief.sh: one line in the generated PR-based ship sections (no-mistakes, direct-PR, and the direct-PR native stack variant; not local-only, which opens no PR) telling workers to post screenshots they capture themselves as one PR comment right after the PR opens with 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.
  3. bin/fm-bootstrap.sh: raise NO_MISTAKES_MIN from 1.31.2 to 1.62.0 and add a gh floor GH_MIN=2.99.0, with tests. A home still on the old versions must get a clear actionable diagnostic (the existing MISSING: line; bootstrap still exits 0 and the session is not blocked), not a hard failure. For an installed-but-outdated no-mistakes the install command is 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.
  4. Any shared doc or skill that tells firstmate to put screenshot local paths in PR bodies: replace with a pointer to item 2's line. None exists in shared tracked material (the offending wording was only in private task briefs), so this is a no-op.
    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.sh now maps the newer no-mistakes run outcomes. passed-with-override and passed-with-skips read as done, like passed, and the detail line names the outcome. ci-monitor-interrupted reads as blocked, with a detail saying the PR is still open and its CI and merge state need checking. Before this, all three came out as unknown. New tests in tests/fm-crew-state.test.sh cover them. AGENTS.md now points to the script header for the outcome-to-state mapping instead of keeping its own copy.
  • bin/fm-bootstrap.sh raises NO_MISTAKES_MIN from 1.31.2 to 1.62.0 and adds a new GH_MIN=2.99.0 floor. An older no-mistakes or gh now prints the usual MISSING: diagnostic, and bootstrap still exits 0. If no-mistakes is installed but too old, the suggested command is no-mistakes update instead 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.md and the bootstrap-diagnostics skill now list the new floors.
  • bin/fm-brief.sh defines one SCREENSHOTS line 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, using gh 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 in tests/fm-brief.test.sh checks 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)

### [base] no-mistakes axi status -> status: completed / outcome: passed
$ fm-crew-state.sh feat-demo
  state: done · source: run-step · run passed: PR merged/closed

### [target] no-mistakes axi status -> status: completed / outcome: passed
$ fm-crew-state.sh feat-demo
  state: done · source: run-step · run passed: PR merged/closed

### [base] no-mistakes axi status -> status: completed / outcome: passed-with-override
$ fm-crew-state.sh feat-demo
  state: unknown · source: run-step · outcome: passed-with-override

### [target] no-mistakes axi status -> status: completed / outcome: passed-with-override
$ fm-crew-state.sh feat-demo
  state: done · source: run-step · run passed-with-override: PR merged/closed

### [base] no-mistakes axi status -> status: completed / outcome: passed-with-skips
$ fm-crew-state.sh feat-demo
  state: unknown · source: run-step · outcome: passed-with-skips

### [target] no-mistakes axi status -> status: completed / outcome: passed-with-skips
$ fm-crew-state.sh feat-demo
  state: done · source: run-step · run passed-with-skips: PR merged/closed

### [base] no-mistakes axi status -> status: ci_monitor_interrupted / outcome: ci-monitor-interrupted
$ fm-crew-state.sh feat-demo
  state: unknown · source: run-step · outcome: ci-monitor-interrupted

### [target] no-mistakes axi status -> status: ci_monitor_interrupted / outcome: ci-monitor-interrupted
$ fm-crew-state.sh feat-demo
  state: blocked · source: run-step · CI monitoring interrupted: PR still open, check its CI and merge state

### [base] no-mistakes axi status -> status: running / outcome: checks-passed
$ fm-crew-state.sh feat-demo
  state: done · source: run-step · checks green: PR ready for review

### [target] no-mistakes axi status -> status: running / outcome: checks-passed
$ fm-crew-state.sh feat-demo
  state: done · source: run-step · checks green: PR ready for review

### [base] no-mistakes axi status -> status: failed / outcome: failed
$ fm-crew-state.sh feat-demo
  state: failed · source: run-step · run failed

### [target] no-mistakes axi status -> status: failed / outcome: failed
$ fm-crew-state.sh feat-demo
  state: failed · source: run-step · run failed
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 update install path

Host toolchain (real binaries; only --version is ever invoked on them here):
  gh version 2.95.0 (2026-06-17)
  https://github.com/cli/cli/releases/tag/v2.95.0
  no-mistakes version v1.48.0 (2ac3769) 2026-08-08T06:39:01Z

### BEFORE (base fdafbfb): home on the host's real gh 2.95.0 + no-mistakes v1.48.0
$ fm-bootstrap.sh 
  (silent stdout: nothing to report)
  [exit 0]

### AFTER (b63d696): home on the host's real gh 2.95.0 + no-mistakes v1.48.0
$ fm-bootstrap.sh 
  MISSING: no-mistakes (install: no-mistakes update  # refuses while a validation run is active; retry when the fleet is quiet)
  MISSING: gh (install: brew install gh  # or the platform's package manager)
  [exit 0]

### AFTER: home already upgraded (gh 2.99.0, no-mistakes v1.62.0)
$ fm-bootstrap.sh 
  (silent stdout: nothing to report)
  [exit 0]

### AFTER: fresh home with no no-mistakes installed at all
$ fm-bootstrap.sh 
  MISSING: no-mistakes (install: curl -fsSL https://raw.githubusercontent.com/kunchenguid/no-mistakes/main/docs/install.sh | sh)
  [exit 0]

### AFTER: captain approves -> install no-mistakes (installed v1.48.0; the real binary is NOT called for anything but --version)
$ fm-bootstrap.sh install no-mistakes
  installing no-mistakes: no-mistakes update
  [exit 0]

  no-mistakes invocations recorded by the stand-in wrapper:
    no-mistakes update
Evidence: 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

### fm-brief.sh shots-nm demo-proj --mode no-mistakes   (base fdafbfb -> target b63d696)
--- base
+++ target
@@ -84,4 +84,6 @@
   When the decision comes back, feed it to the gate with `no-mistakes axi respond` and let the pipeline apply it - do not route the question to "the user" or implement the fix yourself.
 - Avoid `--yes`: it would silently bypass firstmate's authority check and any required captain escalation.
 
+Screenshots or recordings you capture yourself go on the PR as one comment right after it opens: `gh pr comment <PR url> --body-file <md> --attach <file>...` (gh >= 2.99; plain gh, not gh-axi). Never commit them into the branch and never cite their local paths.
+
 After /no-mistakes reports CI green (the CI-ready return point - do not wait for it to keep monitoring in the background until merge), append `done: PR {url} checks green` and stop. You are finished.

### fm-brief.sh shots-direct demo-proj --mode direct-PR   (base fdafbfb -> target b63d696)
--- base
+++ target
@@ -70,5 +70,6 @@
 Delivery contract: mode=direct-PR
 This task ships **direct-PR**: you raise the PR yourself, without the no-mistakes pipeline.
 The task is complete only when committed on your branch.
-When it is implemented and committed, push your branch and open a PR with `gh-axi`, then append `done: PR {url}` to the status file and stop.
+Screenshots or recordings you capture yourself go on the PR as one comment right after it opens: `gh pr comment <PR url> --body-file <md> --attach <file>...` (gh >= 2.99; plain gh, not gh-axi). Never commit them into the branch and never cite their local paths.
+When it is implemented and committed, push your branch and open a PR with `gh-axi`, post any such screenshots, then append `done: PR {url}` to the status file and stop.
 Do NOT run /no-mistakes. The configured merge authority decides whether to merge the PR; firstmate relays the outcome.

### fm-brief.sh shots-stack demo-proj --mode direct-PR --stack   (base fdafbfb -> target b63d696)
--- base
+++ target
@@ -80,6 +80,7 @@
 
 Run the project's local checks at every layer's own head, and put the exact commands and their results in that layer's PR body, so every layer carries its own evidence whether or not CI runs for it.
 When every layer is committed, pushed, and in the stack, prove it: `<FM_ROOT>/bin/fm-stack-check.sh <bottom-pr-url> ... <top-pr-url>` must exit 0.
+Screenshots or recordings you capture yourself go on the PR as one comment right after it opens: `gh pr comment <PR url> --body-file <md> --attach <file>...` (gh >= 2.99; plain gh, not gh-axi). Never commit them into the branch and never cite their local paths.
 Then append `done: {the one line fm-stack-check.sh printed}` to the status file and stop.
 Never report done without that passing output.
 The captain merges the stack; firstmate never merges a stacked PR and relays the outcome.

### fm-brief.sh shots-local demo-proj --mode local-only   (base fdafbfb -> target b63d696)
(identical: local-only opens no PR, so no screenshot line)
Evidence: Bootstrap on this host after the change
$ fm-bootstrap.sh # real gh 2.95.0 + no-mistakes v1.48.0
MISSING: no-mistakes (install: no-mistakes update # refuses while a validation run is active; retry when the fleet is quiet)
MISSING: gh (install: brew install gh # or the platform's package manager)
[exit 0]

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 2 infos
  • ℹ️ bin/fm-teardown.sh:1436 - task_status_is_terminal_run in fm-teardown.sh still accepts only cancelled|failed|passed|checks-passed. It is the only other place in bin/ that lists no-mistakes outcome words, and it was not taught passed-with-override, passed-with-skips or ci-monitor-interrupted. It only runs right after conclude_task_no_mistakes_run has confirmed the run was parked with no outcome and then issued axi 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-mistakes now branches on command -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. The install subcommand runs this string through eval, so if the condition were inverted it would run no-mistakes update on 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-mistakes and checks that the output is exactly the MISSING: 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 fdafbfb bin/ in a scratch copy, all fail as expected
  • Suites 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, plus fm-pr-check-security (it fakes gh and bootstrap now calls gh --version): all pass
  • bash 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 real bin/fm-crew-state.sh feat-demo at base and at target, against fake no-mistakes axi status output for passed, passed-with-override, passed-with-skips, ci-monitor-interrupted (with status ci_monitor_interrupted), checks-passed and failed
  • Manual: ran the real bin/fm-bootstrap.sh at base and at target with the host's real gh 2.95.0 and no-mistakes v1.48.0 (only --version was forwarded to them), with already-upgraded tools, and with no no-mistakes installed; also ran fm-bootstrap.sh install no-mistakes through a wrapper that only logs the call
  • Manual: generated briefs with bin/fm-brief.sh &lt;id&gt; demo-proj --mode no-mistakes|direct-PR|direct-PR --stack|local-only at base and at target and diffed them
  • Read-only upstream checks via gh api / gh search code: no-mistakes outcomeFor/outcomeForRun, the 1.62.0 changelog entry 'attach media evidence to GitHub pull requests', the v1.48.0 internal/update active-run refusal, and gh v2.99.0's pr comment --attach
  • git grep of 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.

…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.
@SchokoShake
SchokoShake merged commit 10fa2e5 into main Sep 22, 2026
13 checks passed
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