From 78e497871ce923cdf31440b4dbd5ddc479c8b3e8 Mon Sep 17 00:00:00 2001 From: Omar Ali Date: Tue, 25 Aug 2026 23:26:11 -0400 Subject: [PATCH 1/5] fix(brief): carry no-mistakes ship briefs into validation without stopping The generated no-mistakes definition of done told the worker to append a `done:` line and stop after its implementation commit, then wait for firstmate to say "now run the pipeline". Firstmate performs no review at that point, so the pause bought nothing and cost a supervision round-trip on every ship task. It also asked for two `done:` lines for one task. `done:` is a terminal state, and a ship task's `done:` means "ready to verify and tear down", so a mid-task `done:` was indistinguishable from a finished task at the exact moment the distinction matters: an unvalidated commit with no PR. The worker now reports the implementation milestone with the existing nonterminal `working:` state and invokes the pipeline immediately, so the only `done:` on a no-mistakes ship task is the terminal CI-green report. Every other rule in the block is unchanged. AGENTS.md's Validate step is updated to match, since it was the one cross-reference stating that firstmate triggers validation after the implementation commit. --- AGENTS.md | 2 +- bin/fm-dod-lib.sh | 6 ++--- tests/fm-brief.test.sh | 58 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0c7cf568b99..405145f7f01 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -357,7 +357,7 @@ After an autonomous merge, give the captain a one-line full-URL or local-main ou ### Validate -For a no-mistakes ship, trigger validation on the same worker after its implementation commit, using the harness invocation owned by `harness-adapters`. +For a no-mistakes ship, the brief has the same worker carry straight from its implementation commit into validation without waiting, so expect no handoff pause and no mid-task `done:` line; trigger validation yourself with the harness invocation owned by `harness-adapters` only when a worker has stopped without starting the run. The task worker that starts a no-mistakes run drives the pipeline and owns every `no-mistakes axi run` and `no-mistakes axi respond` call through the next gate or outcome. Firstmate never invokes `no-mistakes axi respond` for a crew-owned run. When the captain adds or changes an ask mid-task, append the captain's words to that brief's `## Captain's intent` and steer the worker; Firstmate build constraints stay in `## Firstmate spec` or the steer. diff --git a/bin/fm-dod-lib.sh b/bin/fm-dod-lib.sh index 07a7b46e242..af2265fcd7c 100755 --- a/bin/fm-dod-lib.sh +++ b/bin/fm-dod-lib.sh @@ -218,9 +218,9 @@ EOF cat </dev/null 2>&1 + brief="$home/data/$id/brief.md" + assert_present "$brief" "no-mistakes brief was not scaffolded" + assert_no_grep "When you believe it is complete, append \`done: {summary}\` to the status file and stop." "$brief" \ + "no-mistakes brief still instructs a mid-task stop after the implementation commit" + assert_no_grep "Firstmate will then instruct you to run /no-mistakes" "$brief" \ + "no-mistakes brief still waits for a firstmate handoff before validating" + assert_grep "invoke /no-mistakes right away - do not stop and do not wait to be told to start it" "$brief" \ + "no-mistakes brief lost the carry-through instruction into validation" + assert_grep "append \`working: implemented, starting validation\`" "$brief" \ + "no-mistakes brief must report the implementation milestone with a nonterminal state" + # The surviving `done:` must be the terminal CI-green report, and the + # hard-won gate rules around it must be untouched. + assert_grep "append \`done: PR {url} checks green\` and stop. You are finished." "$brief" \ + "no-mistakes brief lost its terminal CI-green report" + assert_grep "ask-user findings are never yours to answer" "$brief" \ + "no-mistakes brief lost the ask-user escalation rule" + assert_grep "NEVER pass \`--yes\` (or \`-y\`)" "$brief" \ + "no-mistakes brief lost the --yes prohibition" + assert_grep "Do not hand-edit, commit, or fix findings yourself while a run is active" "$brief" \ + "no-mistakes brief lost the active-run hands-off rule" + assert_grep "do not wait for it to keep monitoring in the background until merge" "$brief" \ + "no-mistakes brief lost its CI-green return point" + + # Each mode's definition of done instructs exactly one `done:` line, so no + # ship brief can ask for a terminal state twice for one task. + local mode ids + ids=0 + for mode in no-mistakes direct-PR local-only; do + ids=$((ids + 1)) + id="brief-single-done-c$ids" + FM_HOME="$home" "$ROOT/bin/fm-brief.sh" "$id" some-proj --mode "$mode" >/dev/null 2>&1 + brief="$home/data/$id/brief.md" + assert_present "$brief" "$mode brief was not scaffolded" + dod=$(sed -n '/^# Definition of done$/,$p' "$brief") + count=$(printf '%s\n' "$dod" | grep -c -F 'append `done:' || true) + [ "$count" -eq 1 ] \ + || fail "$mode definition of done instructs $count \`done:\` lines, expected exactly 1" + done + pass "fm-brief.sh: ship briefs carry through to their single terminal report" +} + test_ship_project_memory_wording() { local home id brief home="$TMP_ROOT/project-memory-home" @@ -912,6 +967,7 @@ test_delivery_flags_are_refused_where_they_do_not_apply test_faster_paths_use_configured_authority_without_stacked_review test_no_mistakes_dod_wording test_ask_user_escalation_format +test_ship_briefs_never_instruct_a_mid_task_stop test_ship_project_memory_wording test_herdr_lab_contract_is_explicit_and_complete test_herdr_lab_contract_quotes_foreign_firstmate_path From 042d906af896e91009c0b3cad3b50b735eb46d6e Mon Sep 17 00:00:00 2001 From: Omar Ali Date: Wed, 26 Aug 2026 18:58:36 -0400 Subject: [PATCH 2/5] no-mistakes: apply CI fixes --- .github/workflows/no-mistakes-required.yml | 36 +++++++++++++++ tests/fm-no-mistakes-required.test.sh | 54 ++++++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/.github/workflows/no-mistakes-required.yml b/.github/workflows/no-mistakes-required.yml index 41bbac1f564..7e7cb7713e9 100644 --- a/.github/workflows/no-mistakes-required.yml +++ b/.github/workflows/no-mistakes-required.yml @@ -9,6 +9,7 @@ on: permissions: contents: read + pull-requests: read # GitHub concurrency groups retain at most one pending run, replacing older # pending runs even when cancel-in-progress is false. Give body-bearing events @@ -26,5 +27,40 @@ jobs: github.event.pull_request.user.login != 'github-actions[bot]' && github.event.pull_request.user.login != 'dependabot[bot]' steps: + # no-mistakes creates the PR before it can append the completed-pipeline + # attestation. Actions started by the initial open event receive that + # earlier body, and GitHub does not dispatch a second pull_request event + # when the same token updates it. Read the current body until the + # attestation arrives, so this required check judges the final pipeline + # metadata rather than the stale event payload. + - name: Read current pull request body + id: current-pr + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + body='' + for attempt in $(seq 1 30); do + body=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq .body) + if [[ "$body" == *'" + # shellcheck disable=SC2016 # The fake gh script must receive literal shell expansions. + printf '%s\n' '#!/usr/bin/env bash' \ + 'count=$(cat "$CURRENT_PR_ATTEMPTS" 2>/dev/null || printf 0)' \ + 'printf "%s\\n" "$((count + 1))" > "$CURRENT_PR_ATTEMPTS"' \ + 'if [ "$count" -eq 0 ]; then' \ + ' printf "%s\\n" "$UNATTESTED_BODY"' \ + 'else' \ + ' printf "%s\\n" "$ATTESTED_BODY"' \ + 'fi' > "$fakebin/gh" + printf '%s\n' '#!/usr/bin/env bash' 'exit 0' > "$fakebin/sleep" + chmod +x "$fakebin/gh" "$fakebin/sleep" + body='Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)' + PATH="$fakebin:$PATH" GITHUB_REPOSITORY=kunchenguid/firstmate PR_NUMBER=3135 \ + PR_HEAD_SHA="$NEW_SHA" GITHUB_OUTPUT="$output" CURRENT_PR_ATTEMPTS="$attempts" UNATTESTED_BODY="$body" \ + ATTESTED_BODY="$attested" bash "$script" \ + || fail "required-check current-PR reader did not poll successfully" + assert_contains "$(cat "$output")" "no-mistakes-pipeline-attestation:v1" \ + "required-check current-PR reader retained the live attestation" + [ "$(cat "$attempts")" -eq 2 ] \ + || fail "required-check current-PR reader did not retry after the stale PR body" + assert_contains "$(cat "$metadata")" 'steps.current-pr.outputs.body' \ + "required-check verifier is not bound to the current PR body" + pass "required-check workflow waits for and verifies the current PR body" +} + fetch_shared_verifier test_matching_head_and_completed_steps_pass test_mismatched_head_fails_with_both_shas test_missing_head_fails +test_required_workflow_reads_the_current_pr_body From e820072f35fb64e32f44623ce9bc5e0ed4b42a3f Mon Sep 17 00:00:00 2001 From: Omar Ali Date: Wed, 26 Aug 2026 19:04:45 -0400 Subject: [PATCH 3/5] no-mistakes: apply CI fixes --- .github/workflows/no-mistakes-required.yml | 6 +++++- tests/fm-no-mistakes-required.test.sh | 14 +++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/.github/workflows/no-mistakes-required.yml b/.github/workflows/no-mistakes-required.yml index 7e7cb7713e9..0f9246a4eb5 100644 --- a/.github/workflows/no-mistakes-required.yml +++ b/.github/workflows/no-mistakes-required.yml @@ -42,7 +42,11 @@ jobs: run: | set -euo pipefail body='' - for attempt in $(seq 1 30); do + # The PR can spend more than one minute moving from the opening body + # to the signed attestation while no-mistakes records its pipeline. + # Keep the required check bounded, but leave enough room for that + # normal asynchronous update before judging the body stale. + for attempt in $(seq 1 150); do body=$(gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}" --jq .body) if [[ "$body" == *'" - # shellcheck disable=SC2016 # The fake gh script must receive literal shell expansions. - printf '%s\n' '#!/usr/bin/env bash' \ - 'count=$(cat "$CURRENT_PR_ATTEMPTS" 2>/dev/null || printf 0)' \ - 'printf "%s\\n" "$((count + 1))" > "$CURRENT_PR_ATTEMPTS"' \ - 'if [ "$count" -lt "$STALE_ATTEMPTS" ]; then' \ - ' printf "%s\\n" "$UNATTESTED_BODY"' \ - 'else' \ - ' printf "%s\\n" "$ATTESTED_BODY"' \ - 'fi' > "$fakebin/gh" - printf '%s\n' '#!/usr/bin/env bash' 'exit 0' > "$fakebin/sleep" - chmod +x "$fakebin/gh" "$fakebin/sleep" - body='Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)' - # Thirty stale reads consume the previous one-minute retry budget. The next - # response proves the workflow keeps polling long enough for a normally - # delayed pipeline attestation rather than failing its opening-event body. - stale_attempts=30 - PATH="$fakebin:$PATH" GITHUB_REPOSITORY=kunchenguid/firstmate PR_NUMBER=3135 \ - PR_HEAD_SHA="$NEW_SHA" GITHUB_OUTPUT="$output" CURRENT_PR_ATTEMPTS="$attempts" STALE_ATTEMPTS="$stale_attempts" UNATTESTED_BODY="$body" \ - ATTESTED_BODY="$attested" bash "$script" \ - || fail "required-check current-PR reader did not poll successfully" - assert_contains "$(cat "$output")" "no-mistakes-pipeline-attestation:v1" \ - "required-check current-PR reader retained the live attestation" - [ "$(cat "$attempts")" -eq $((stale_attempts + 1)) ] \ - || fail "required-check current-PR reader did not outlast the former one-minute retry budget" - assert_contains "$(cat "$metadata")" 'steps.current-pr.outputs.body' \ - "required-check verifier is not bound to the current PR body" - pass "required-check workflow waits for and verifies the current PR body" -} - fetch_shared_verifier test_matching_head_and_completed_steps_pass test_mismatched_head_fails_with_both_shas test_missing_head_fails -test_required_workflow_reads_the_current_pr_body diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index 8093b733c7d..02e57332df0 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -174,7 +174,33 @@ record_pi_busy() { # --source pi-ext --event agent-start } -reap() { kill "$1" 2>/dev/null || true; wait "$1" 2>/dev/null || true; } +# Teardown only: a case has already run every assertion it makes about a round +# by the time it reaps that round's watcher. TERM is therefore a request to stop +# a watcher we are done observing, not a path under test. +# +# A plain `kill; wait` makes that teardown UNBOUNDED. fm-watch.sh defers its TERM +# trap until the foreground command in flight returns, and a TERM landing inside +# a recovery-marker or lock critical section can leave the exit path unwinding +# for many minutes (bin/fm-wake-lib.sh documents the same class of hang for the +# self-held reclaim case). The watcher does eventually exit, so the case still +# PASSES - it just takes ~17 minutes, which is what blew the portable-serial +# shard's wall-clock cap in CI while every assertion in the file still reported +# ok. Bound the grace and escalate to KILL so teardown cost is deterministic. +# This can only shorten teardown; it cannot mask a failed assertion, because +# there are no assertions left to make when reap runs. +reap() { + local pid=$1 i=0 + kill "$pid" 2>/dev/null || true + # 100 ticks matches the wait_for_exit budget documented above: far longer than + # a healthy watcher needs to honour TERM, short enough to stay bounded. + while [ "$i" -lt 100 ]; do + kill -0 "$pid" 2>/dev/null || break + sleep 0.1 + i=$((i + 1)) + done + kill -9 "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true +} # --- pure classifier predicates (fm-classify-lib.sh) ------------------------ From 6775fac06f9300a77bee848c604399b45cab427b Mon Sep 17 00:00:00 2001 From: Omar Ali Date: Sat, 12 Sep 2026 14:52:35 -0400 Subject: [PATCH 5/5] no-mistakes(document): Refresh validation handoff documentation --- tests/fm-brief.test.sh | 10 +++------- tests/fm-watch-triage.test.sh | 17 +++-------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index c4da679689b..86a13e6c0f6 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -424,13 +424,9 @@ test_ask_user_escalation_format() { pass "fm-brief.sh: no-mistakes ask-user findings use one event plus a verbatim snapshot" } -# A no-mistakes ship brief must carry the worker straight from its -# implementation commit into validation. The old scaffold told the worker to -# report `done:` and stop there, which burned a supervision round-trip on every -# ship task and put a nonterminal `done:` in the status log at the exact moment -# an unvalidated commit is indistinguishable from a finished task. Every ship -# mode is asserted here so a mid-task stop cannot reappear in any of them: each -# generated definition of done must instruct exactly one `done:` line. +# A no-mistakes ship brief carries the worker straight from its implementation +# commit into validation. Each ship mode is covered so its definition of done +# continues to instruct exactly one terminal `done:` line. test_ship_briefs_never_instruct_a_mid_task_stop() { local home id brief dod count home="$TMP_ROOT/mid-task-stop-home" diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index 02e57332df0..3f6bdf91660 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -174,20 +174,9 @@ record_pi_busy() { # --source pi-ext --event agent-start } -# Teardown only: a case has already run every assertion it makes about a round -# by the time it reaps that round's watcher. TERM is therefore a request to stop -# a watcher we are done observing, not a path under test. -# -# A plain `kill; wait` makes that teardown UNBOUNDED. fm-watch.sh defers its TERM -# trap until the foreground command in flight returns, and a TERM landing inside -# a recovery-marker or lock critical section can leave the exit path unwinding -# for many minutes (bin/fm-wake-lib.sh documents the same class of hang for the -# self-held reclaim case). The watcher does eventually exit, so the case still -# PASSES - it just takes ~17 minutes, which is what blew the portable-serial -# shard's wall-clock cap in CI while every assertion in the file still reported -# ok. Bound the grace and escalate to KILL so teardown cost is deterministic. -# This can only shorten teardown; it cannot mask a failed assertion, because -# there are no assertions left to make when reap runs. +# Teardown only: callers have finished all assertions about this watcher. +# fm-watch.sh can defer TERM while foreground work unwinds, so bound its grace +# period and then escalate to KILL to keep test teardown deterministic. reap() { local pid=$1 i=0 kill "$pid" 2>/dev/null || true