From 0860965aa16456f8f4e7c75dd034e176efd740fb Mon Sep 17 00:00:00 2001 From: Bchue Date: Fri, 7 Aug 2026 10:17:29 -0400 Subject: [PATCH 1/3] fix(fleet): cap published doing/status text to the measured card-fit budget Firstmate published a worker's current-state detail (the "doing" text surfaced by fm-fleet-snapshot.sh and fm-bearings-snapshot.sh) with no length discipline at the source and only ad hoc, ungrounded jq trunc() ceilings downstream, leaving a rendering surface to elide text that was simply written too long. Add FM_DOING_CHAR_CAP (59 chars) to bin/fm-classify-lib.sh, sourced from data/herdr-card-iteration-2/report.md's measured fit ladder: at 11.5px Medium 500, all ten sampled real doing strings fit their card column with zero elision, and the longest is 59 characters. Enforce it at the true publish point (fm-fleet-snapshot.sh's crew_state_json) with a word-boundary- aware bash truncation (fm_doing_truncate), and apply the same budget via a matching jq helper (doing_trunc) at every downstream doing field in fm-fleet-snapshot.sh and fm-bearings-snapshot.sh so nothing re-lengthens it. --- bin/fm-bearings-snapshot.sh | 27 ++++- bin/fm-classify-lib.sh | 37 +++++++ bin/fm-fleet-snapshot.sh | 20 +++- tests/fm-doing-cap.test.sh | 192 ++++++++++++++++++++++++++++++++++++ 4 files changed, 270 insertions(+), 6 deletions(-) create mode 100755 tests/fm-doing-cap.test.sh diff --git a/bin/fm-bearings-snapshot.sh b/bin/fm-bearings-snapshot.sh index 7564f9ffac..ea532b65a1 100755 --- a/bin/fm-bearings-snapshot.sh +++ b/bin/fm-bearings-snapshot.sh @@ -60,6 +60,11 @@ set -u SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" FLEET="$SCRIPT_DIR/fm-fleet-snapshot.sh" +# Only needed for the shared FM_DOING_CHAR_CAP constant (see that file); this +# wrapper does JSON projection in jq, not the classifier logic the lib mostly +# holds. +# shellcheck source=bin/fm-classify-lib.sh +. "$SCRIPT_DIR/fm-classify-lib.sh" # Bounds (overridable for tests / large fleets). FM_BEARINGS_LANDED=${FM_BEARINGS_LANDED:-6} @@ -301,9 +306,23 @@ MODEL=$(printf '%s' "$SNAP" | jq \ --argjson pr_repos_shown "$PR_REPOS_SHOWN" \ --argjson pr_rows_capped "$PR_ROWS_CAPPED" \ --argjson pr_rows_min_total "$PR_ROWS_MIN_TOTAL" \ - --argjson candidate_prs "$CANDIDATE_PRS" ' + --argjson candidate_prs "$CANDIDATE_PRS" \ + --argjson doing_cap "$FM_DOING_CHAR_CAP" ' def trunc($n): if . == null then null else (tostring | gsub("\\s+"; " ") | if (length > $n) then (.[:$n] + "…") else . end) end; + # Word-boundary-aware cousin of trunc(), for "doing" fields only: a + # published status text should already fit a rendering surface without a + # mid-word cut. See FM_DOING_CHAR_CAP in bin/fm-classify-lib.sh + # for the sourced cap and the bash twin (fm_doing_truncate) this mirrors. + def doing_trunc($n): + tostring | gsub("\\s+"; " ") as $s + | if ($s | length) <= $n then $s + else ($s[0:$n]) as $cut + | ($cut | split(" ")) as $words + | (if ($words | length) > 1 then ($words[0:-1] | join(" ")) else "" end) as $boundary + | (if ($boundary | length) >= (($n * 3) / 5 | floor) and ($boundary | length) > 0 + then $boundary else $cut end) + "…" + end; def round_robin_landed($n): . as $groups | [range(0; (($groups | map(length) | max) // 0)) as $i @@ -365,7 +384,7 @@ MODEL=$(printf '%s' "$SNAP" | jq \ elif .bearings_state == "externally_held" then ([.bearings_holds[] | .id + ": " + (.reason // "held")] | join("; ")) elif .bearings_state == "no_active_work" then "No active child work" - else (.current.reason // "Current home state unavailable") end) | trunc(120)), + else (.current.reason // "Current home state unavailable") end) | doing_trunc($doing_cap)), provenance:.provenance.selected,freshness:.freshness.status, age_seconds:.freshness.age_seconds,contradiction:(.contradiction // false), reason:(.current.reason // "-")} ]) as $secondmates_all @@ -376,12 +395,12 @@ MODEL=$(printf '%s' "$SNAP" | jq \ | {id, kind, state: .current_state.state, doing: ((.current_state.detail // "") as $d - | (if $d != "" then $d else (.hints.last_event_text // "") end) | trunc(90)) + | (if $d != "" then $d else (.hints.last_event_text // "") end) | doing_trunc($doing_cap)) } ] + [ $secondmate_views[] | select(.bearings_state == "active_child_work") | {id,kind:"secondmate",state:.bearings_state, - doing:([.active_children[] | .id + ": " + (.doing // .state)] | join("; ") | trunc(90))} ]) as $in_flight_all + doing:([.active_children[] | .id + ": " + (.doing // .state)] | join("; ") | doing_trunc($doing_cap))} ]) as $in_flight_all | ([ .backlog.records[] | select(.structured and .captain_actionable == true) | {id,key:.id,verb:"captain-hold", diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index d80840f6a1..0cc7cd1e1d 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -412,3 +412,40 @@ scan_captain_relevant_statuses() { # done return 0 } + +# The character budget a published "doing"/status-detail string must fit +# without needing a renderer to elide it. Sourced from +# data/herdr-card-iteration-2/report.md's measured fit ladder: at 11.5px +# Medium 500 (the report's recommended card type), all ten real `doing` +# strings sampled from a live fleet fit their card column with zero elision, +# and the longest of those ten is 59 characters (the binding case: a 59-char +# title uses 299px of a 315px depth-2 column). Below 11.5px, or at a lighter +# weight, some of those same strings elide. Overridable for tests or a +# different rendering surface; any override should stay grounded in a +# measured fit, not a guess. +FM_DOING_CHAR_CAP=${FM_DOING_CHAR_CAP:-59} + +# fm_doing_truncate [cap]: collapse whitespace and, only if +# exceeds the char cap, shorten it to fit - preferring a cut at the last word +# boundary within the cap over a mid-word hard cut, but falling back to a hard +# cut when the word-boundary cut would throw away more than 40% of the +# budget (e.g. one long token with no early space). Mirrors the jq +# `doing_trunc($n)` helper duplicated in bin/fm-fleet-snapshot.sh and +# bin/fm-bearings-snapshot.sh for the same fields in JSON output; keep the +# three in sync. +fm_doing_truncate() { # [cap] + local text=$1 cap=${2:-$FM_DOING_CHAR_CAP} collapsed cut boundary floor + collapsed=$(printf '%s' "$text" | tr -s '[:space:]' ' ') + [ "${#collapsed}" -gt "$cap" ] || { printf '%s' "$collapsed"; return 0; } + cut=${collapsed:0:cap} + case "$cut" in + *' '*) boundary=${cut% *} ;; + *) boundary=$cut ;; + esac + floor=$((cap * 3 / 5)) + if [ "${#boundary}" -ge "$floor" ] && [ "${#boundary}" -gt 0 ]; then + printf '%s…' "$boundary" + else + printf '%s…' "$cut" + fi +} diff --git a/bin/fm-fleet-snapshot.sh b/bin/fm-fleet-snapshot.sh index 4836f36395..7c6a9ee6db 100755 --- a/bin/fm-fleet-snapshot.sh +++ b/bin/fm-fleet-snapshot.sh @@ -225,6 +225,7 @@ crew_state_json() { # esac ;; esac + detail=$(fm_doing_truncate "$detail") jq -n --arg raw "$raw" --arg state "$state" --arg source "$source" --arg detail "$detail" \ '{state:$state,source:$source,detail:$detail,raw:$raw}' } @@ -604,10 +605,25 @@ secondmate_home_summary_json() { # --argjson decisions_n "$FM_SNAPSHOT_SECONDMATE_DECISIONS" \ --argjson landed_n "$FM_SNAPSHOT_SECONDMATE_LANDED_PER_HOME" \ --argjson backlog "$1" \ - --argjson tasks "$2" ' + --argjson tasks "$2" \ + --argjson doing_cap "$FM_DOING_CHAR_CAP" ' def trunc($n): tostring | gsub("\\s+"; " ") | if length > $n then .[:$n] + "…" else . end; + # Word-boundary-aware cousin of trunc(), for the "doing" field only: a + # published status text should already fit a rendering surface without a + # mid-word cut. See FM_DOING_CHAR_CAP in + # bin/fm-classify-lib.sh for the sourced cap and the bash twin + # (fm_doing_truncate) this mirrors. + def doing_trunc($n): + tostring | gsub("\\s+"; " ") as $s + | if ($s | length) <= $n then $s + else ($s[0:$n]) as $cut + | ($cut | split(" ")) as $words + | (if ($words | length) > 1 then ($words[0:-1] | join(" ")) else "" end) as $boundary + | (if ($boundary | length) >= (($n * 3) / 5 | floor) and ($boundary | length) > 0 + then $boundary else $cut end) + "…" + end; ([ $backlog.records[]? | select((.state == "in_flight" or .state == "queued") and (.structured | not)) ]) as $unstructured_current | ([ $backlog.records[]? | select(.state == "in_flight" and .structured) ]) as $owned_in_flight @@ -663,7 +679,7 @@ secondmate_home_summary_json() { # | $tasks[] | select(.id == $work.id and .current_state.state == "working") | {id,kind,state:.current_state.state,source:.current_state.source, - doing:((.current_state.detail // "") | trunc(120))} ]) as $active_all + doing:((.current_state.detail // "") | doing_trunc($doing_cap))} ]) as $active_all | ($captain_holds_all + ([ $tasks[] as $t | ($t.hints.open_decisions // [])[] | {id:$t.id,key,verb,summary:(.summary | trunc(160)),reason:null,source:"status"} ])) as $decisions_all diff --git a/tests/fm-doing-cap.test.sh b/tests/fm-doing-cap.test.sh new file mode 100755 index 0000000000..5db7bb2b86 --- /dev/null +++ b/tests/fm-doing-cap.test.sh @@ -0,0 +1,192 @@ +#!/usr/bin/env bash +# Behavior tests for the "doing"/status-detail publish cap: the character +# budget a worker's published status text must already fit inside without +# needing a downstream renderer to elide it. The cap and its number are +# sourced in bin/fm-classify-lib.sh (FM_DOING_CHAR_CAP, from +# data/herdr-card-iteration-2/report.md's measured fit ladder). +# +# Two layers are covered: +# (a) fm_doing_truncate - the shared bash helper (fm-classify-lib.sh), +# exercised directly. +# (b) fm-fleet-snapshot.sh --json end to end - a task whose status line +# carries a too-long detail comes back with tasks[].current_state.detail +# already capped, while a normal-length one passes through unchanged. +set -u + +# shellcheck source=tests/lib.sh +# shellcheck disable=SC1091 +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +# shellcheck source=bin/fm-classify-lib.sh +# shellcheck disable=SC1091 +. "$ROOT/bin/fm-classify-lib.sh" + +SNAPSHOT="$ROOT/bin/fm-fleet-snapshot.sh" +TMP_ROOT=$(fm_test_tmproot fm-doing-cap) + +command -v jq >/dev/null 2>&1 || { echo "skip: jq not found"; exit 0; } + +# --- (a) fm_doing_truncate, direct ----------------------------------------- + +test_short_text_passes_through_unchanged() { + local input out + input="Refactor work cards with improved chip icons and typography" # 59 chars, the report's own longest real sample + [ "${#input}" -eq "$FM_DOING_CHAR_CAP" ] || fail "fixture no longer matches the sourced cap ($FM_DOING_CHAR_CAP): ${#input}" + out=$(fm_doing_truncate "$input") + [ "$out" = "$input" ] || fail "at-cap text must pass through byte-for-byte, got: $out" + pass "text at exactly the cap passes through unchanged" +} + +test_well_under_cap_passes_through_unchanged() { + local input out + input="Ship the thing" + out=$(fm_doing_truncate "$input") + [ "$out" = "$input" ] || fail "short text must pass through unchanged, got: $out" + pass "text well under the cap passes through unchanged" +} + +test_long_text_truncates_at_word_boundary() { + local input out + input="Refactor work cards with improved chip icons and typography and also update the changelog docs thoroughly" + out=$(fm_doing_truncate "$input") + [ "${#out}" -le $((FM_DOING_CHAR_CAP + 1)) ] || fail "truncated output must fit cap+ellipsis, got ${#out} chars: $out" + case "$out" in + *…) : ;; + *) fail "truncated output must end with an ellipsis marker, got: $out" ;; + esac + # Word-boundary cut: strip the ellipsis and confirm what remains is a + # whitespace-clean PREFIX of the original text (never a word sliced in + # half), which is only possible if the cut landed on a space. + local body=${out%…} + case "$input" in + "$body"*) : ;; + *) fail "expected a word-boundary prefix of the input, got: $body" ;; + esac + case "$input" in + "$body "*|"$body") : ;; + *) fail "cut must land exactly at a space, not mid-word: '$body' against '$input'" ;; + esac + pass "long text is shortened at the last whole word instead of hard-cut mid-word" +} + +test_single_long_token_falls_back_to_hard_cut() { + local input out expect + input="Supercalifragilisticexpialidocioussupercalifragilisticexpialidocioussupercalifragilisticexpialidocious" + out=$(fm_doing_truncate "$input") + expect="${input:0:$FM_DOING_CHAR_CAP}…" + [ "$out" = "$expect" ] || fail "no word boundary exists, so it must hard-cut at the cap, got: $out" + pass "a single token with no early space falls back to a hard cut at the cap" +} + +test_whitespace_is_collapsed_before_measuring() { + local input out + input=$'line one\nwith extra spaces' + out=$(fm_doing_truncate "$input") + [ "$out" = "line one with extra spaces" ] || fail "whitespace must collapse to single spaces, got: $out" + pass "runs of whitespace collapse to one space before the cap is measured" +} + +# --- (b) fm-fleet-snapshot.sh --json, end to end ---------------------------- + +make_fakebin() { # + local fb + fb=$(fm_fakebin "$1") + cat > "$fb/no-mistakes" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + cat > "$fb/tmux" <<'SH' +#!/usr/bin/env bash +case "${1:-}" in + display-message) printf '%%1\n' ;; + capture-pane) printf 'all quiet\n> \n' ;; +esac +exit 0 +SH + chmod +x "$fb/no-mistakes" "$fb/tmux" + printf '%s\n' "$fb" +} + +make_home() { # + local home=$TMP_ROOT/$1 + mkdir -p "$home/state" "$home/data" "$home/projects" "$home/config" + printf '%s\n' "$home" +} + +# Two secondmate tasks (secondmate kind skips the busy-pane check and reads +# straight off the status log, so the published detail is exactly the text +# after the status verb - the same free text a crewmate would write). +write_fixture() { # + local home=$1 + mkdir -p "$home/projects/short-wt" "$home/projects/long-wt" + cat > "$home/data/backlog.md" < "$home/state/short-doing.status" + fm_write_meta "$home/state/long-doing.meta" \ + "window=firstmate:fm-long-doing" \ + "worktree=$home/projects/long-wt" \ + "project=alpha" \ + "harness=codex" \ + "kind=secondmate" \ + "mode=secondmate" \ + "yolo=off" \ + "home=$home/projects/long-wt" \ + "projects=alpha" + printf 'working: Refactor work cards with improved chip icons and typography and also update the changelog docs thoroughly\n' \ + > "$home/state/long-doing.status" +} + +test_snapshot_passes_through_a_normal_length_detail() { + local home fakebin out detail + home=$(make_home normal) + write_fixture "$home" + fakebin=$(make_fakebin "$home") + out=$(PATH="$fakebin:$PATH" FM_HOME="$home" "$SNAPSHOT" --json) + detail=$(printf '%s' "$out" | jq -r '.tasks[] | select(.id == "short-doing") | .current_state.detail') + [ "$detail" = "Refactor work cards with improved chip icons and typography" ] \ + || fail "normal-length detail must pass through unchanged, got: $detail" + pass "fm-fleet-snapshot.sh passes a normal-length doing detail through unchanged" +} + +test_snapshot_shortens_a_too_long_detail() { + local home fakebin out detail + home=$(make_home overlong) + write_fixture "$home" + fakebin=$(make_fakebin "$home") + out=$(PATH="$fakebin:$PATH" FM_HOME="$home" "$SNAPSHOT" --json) + detail=$(printf '%s' "$out" | jq -r '.tasks[] | select(.id == "long-doing") | .current_state.detail') + [ "${#detail}" -le $((FM_DOING_CHAR_CAP + 1)) ] \ + || fail "published detail must respect the sourced cap, got ${#detail} chars: $detail" + case "$detail" in + *…) : ;; + *) fail "an over-cap detail must be marked as shortened, got: $detail" ;; + esac + case "$detail" in + *changelog*) fail "must not run past the sourced cap keeping full original text: $detail" ;; + esac + pass "fm-fleet-snapshot.sh shortens an over-cap doing detail to the sourced budget" +} + +test_short_text_passes_through_unchanged +test_well_under_cap_passes_through_unchanged +test_long_text_truncates_at_word_boundary +test_single_long_token_falls_back_to_hard_cut +test_whitespace_is_collapsed_before_measuring +test_snapshot_passes_through_a_normal_length_detail +test_snapshot_shortens_a_too_long_detail + +echo "all fm-doing-cap tests passed" From d081bc50de2c70cd3db73ec9f5c16034783a35f7 Mon Sep 17 00:00:00 2001 From: Bchue Date: Fri, 7 Aug 2026 21:17:41 -0400 Subject: [PATCH 2/3] no-mistakes(review): Pin fm_doing_truncate to UTF-8 locale for codepoint-safe truncation --- bin/fm-classify-lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index 0cc7cd1e1d..0739f2a5c6 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -435,6 +435,7 @@ FM_DOING_CHAR_CAP=${FM_DOING_CHAR_CAP:-59} # three in sync. fm_doing_truncate() { # [cap] local text=$1 cap=${2:-$FM_DOING_CHAR_CAP} collapsed cut boundary floor + local LC_ALL=C.UTF-8 collapsed=$(printf '%s' "$text" | tr -s '[:space:]' ' ') [ "${#collapsed}" -gt "$cap" ] || { printf '%s' "$collapsed"; return 0; } cut=${collapsed:0:cap} From 6fb847a9abab664f69786e16d8bd118e8e56a1f8 Mon Sep 17 00:00:00 2001 From: Bchue Date: Fri, 7 Aug 2026 21:58:32 -0400 Subject: [PATCH 3/3] no-mistakes: apply CI fixes --- .github/workflows/no-mistakes-required.yml | 31 +++- CONTRIBUTING.md | 1 + tests/no-mistakes-required-workflow.test.sh | 168 ++++++++++++++++++++ 3 files changed, 199 insertions(+), 1 deletion(-) create mode 100755 tests/no-mistakes-required-workflow.test.sh diff --git a/.github/workflows/no-mistakes-required.yml b/.github/workflows/no-mistakes-required.yml index f56afee418..9deff1f8d3 100644 --- a/.github/workflows/no-mistakes-required.yml +++ b/.github/workflows/no-mistakes-required.yml @@ -9,6 +9,9 @@ on: permissions: contents: read + # Needed to re-read the live PR body; the event payload only carries a + # snapshot taken when the event fired (see the verify step below). + 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 @@ -31,13 +34,38 @@ jobs: PR_BODY: ${{ github.event.pull_request.body }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} PR_NUMBER: ${{ github.event.pull_request.number }} + PR_REPO: ${{ github.repository }} + GH_TOKEN: ${{ github.token }} run: | set -eu marker='Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)' - if printf '%s' "${PR_BODY:-}" | grep -qF -- "$marker"; then + has_signature() { + printf '%s' "${1:-}" | grep -qF -- "$marker" + } + if has_signature "${PR_BODY:-}"; then echo "Found no-mistakes signature in PR #${PR_NUMBER} body." exit 0 fi + # The event payload carries the body as it stood when the event fired, + # and no-mistakes opens the PR before it writes the deterministic + # '## Pipeline' section into the body. An 'opened' run therefore + # observes an unsigned snapshot of a PR that is compliant moments + # later, and by design (see the concurrency note above) no later event + # can replace that run's verdict. Re-read the live body before failing + # so this check reflects the PR's current state, not a stale snapshot. + attempts=10 + attempt=1 + while [ "$attempt" -le "$attempts" ]; do + live_body=$(gh api "repos/${PR_REPO}/pulls/${PR_NUMBER}" --jq '.body // ""' 2>/dev/null) || live_body='' + if has_signature "$live_body"; then + echo "Found no-mistakes signature in the live PR #${PR_NUMBER} body (attempt ${attempt})." + exit 0 + fi + if [ "$attempt" -lt "$attempts" ]; then + sleep 6 + fi + attempt=$((attempt + 1)) + done { echo "::error::This PR was not raised through no-mistakes." echo @@ -50,5 +78,6 @@ jobs: echo "See CONTRIBUTING.md for setup and the full workflow." echo echo "PR author: ${PR_AUTHOR}" + echo "Live PR body re-read ${attempts} times; the signature never appeared." } >&2 exit 1 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7c3a1cab18..7876f7cbdb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,6 +11,7 @@ Pushing through it runs an AI-driven review/test/lint pipeline in an isolated wo A GitHub Actions check (`Require no-mistakes`) runs on PRs targeting `main` and fails if the body is missing the deterministic signature that no-mistakes writes. It evaluates every PR opening and body edit independently, so a later edit cannot replace an earlier pending compliance check. +Because no-mistakes opens the PR before it writes that signature into the body, the check re-reads the live PR body before failing, so an opening event never leaves a permanently red check on a PR that is signed moments later. GitHub Actions and Dependabot are exempt so their automation keeps working, but regular contributor PRs without the signature will not be reviewed or merged. ## Workflow diff --git a/tests/no-mistakes-required-workflow.test.sh b/tests/no-mistakes-required-workflow.test.sh new file mode 100755 index 0000000000..248d617d6b --- /dev/null +++ b/tests/no-mistakes-required-workflow.test.sh @@ -0,0 +1,168 @@ +#!/usr/bin/env bash +# Behavior tests for the "Require no-mistakes" PR-body compliance check +# (.github/workflows/no-mistakes-required.yml). +# +# The check used to read only github.event.pull_request.body - a snapshot taken +# when the event fired. no-mistakes opens the PR first and writes the +# deterministic '## Pipeline' section into the body immediately afterwards, so +# the 'opened' run always saw an unsigned snapshot of a PR that was compliant +# seconds later. The workflow deliberately gives each opened/edited event its +# own immutable concurrency group, so no later event can replace that verdict: +# the PR was left with a permanently red required check it could never turn +# green. The step must therefore re-read the LIVE body before failing. +# +# These tests extract the step's real shell body out of the workflow YAML and +# run it against stub `gh`/`sleep` binaries, so they exercise the exact script +# GitHub Actions runs rather than a re-spelled copy of it. +set -u + +# shellcheck source=tests/lib.sh +# shellcheck disable=SC1091 +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +WORKFLOW="$ROOT/.github/workflows/no-mistakes-required.yml" +TMP_ROOT=$(fm_test_tmproot no-mistakes-required-workflow) +mkdir -p "$TMP_ROOT" +trap 'fm_test_cleanup; rm -rf "$TMP_ROOT"' EXIT + +MARKER='Updates from [git push no-mistakes](https://github.com/kunchenguid/no-mistakes)' +SIGNED_BODY="## Summary +- something + +## Pipeline + +$MARKER" +UNSIGNED_BODY="## Summary +- something + +## Test plan +- [x] ran the tests" + +# --- extract the verify step's shell body from the workflow ----------------- + +STEP_SCRIPT="$TMP_ROOT/verify-step.sh" +awk ' + /^ run: \|$/ { collecting = 1; next } + collecting { + if ($0 == "") { print ""; next } + if ($0 !~ /^ /) { collecting = 0; next } + print substr($0, 11) + } +' "$WORKFLOW" >"$STEP_SCRIPT" + +[ -s "$STEP_SCRIPT" ] \ + || fail "could not extract the verify step's run: block from $WORKFLOW" +assert_grep "$MARKER" "$STEP_SCRIPT" \ + "extracted step body must contain the no-mistakes signature marker" + +# run_step : run the extracted step and set STEP_OUT +# (combined output), STEP_CODE, and STEP_GH_CALLS. Must NOT be called from a +# command substitution, or the results would be set in a discarded subshell. +# `gh` and `sleep` are stubbed so the retry loop is exercised without any real +# network or wall-clock cost. An empty makes the gh stub fail. +STEP_OUT= +STEP_CODE=0 +STEP_GH_CALLS=0 +STEP_GH_ARGS= +run_step() { + local pr_body=$1 live_body=$2 run_dir fakebin + run_dir=$(mktemp -d "$TMP_ROOT/run.XXXXXX") + fakebin=$(fm_fakebin "$run_dir") + printf '%s' "$live_body" >"$run_dir/live-body" + + cat >"$fakebin/gh" <<'SH' +#!/usr/bin/env bash +printf 'x\n' >>"$STUB_DIR/gh-calls" +printf '%s' "$*" >>"$STUB_DIR/gh-args" +printf '\n' >>"$STUB_DIR/gh-args" +[ -s "$STUB_DIR/live-body" ] || exit 1 +cat "$STUB_DIR/live-body" +SH + cat >"$fakebin/sleep" <<'SH' +#!/usr/bin/env bash +exit 0 +SH + chmod +x "$fakebin/gh" "$fakebin/sleep" + + STUB_DIR="$run_dir" \ + PATH="$fakebin:$PATH" \ + PR_BODY="$pr_body" \ + PR_AUTHOR=someone \ + PR_NUMBER=123 \ + PR_REPO=owner/repo \ + bash "$STEP_SCRIPT" >"$run_dir/out" 2>&1 + STEP_CODE=$? + STEP_OUT=$(cat "$run_dir/out") + if [ -f "$run_dir/gh-calls" ]; then + STEP_GH_CALLS=$(wc -l <"$run_dir/gh-calls" | tr -d ' ') + STEP_GH_ARGS=$(cat "$run_dir/gh-args") + else + STEP_GH_CALLS=0 + STEP_GH_ARGS= + fi +} + +# --- tests ------------------------------------------------------------------ + +test_signed_event_body_passes_without_api_call() { + run_step "$SIGNED_BODY" "" + expect_code 0 "$STEP_CODE" "signed event body must pass" + assert_contains "$STEP_OUT" "Found no-mistakes signature in PR #123 body." \ + "signed event body must report the signature" + [ "$STEP_GH_CALLS" -eq 0 ] \ + || fail "a signed event body must not need an API re-read (gh called $STEP_GH_CALLS times)" + pass "signed event body passes straight through with no live re-read" +} + +test_stale_event_body_passes_on_live_reread() { + run_step "$UNSIGNED_BODY" "$SIGNED_BODY" + expect_code 0 "$STEP_CODE" \ + "a stale unsigned snapshot must pass once the live body carries the signature" + assert_contains "$STEP_OUT" "Found no-mistakes signature in the live PR #123 body" \ + "live re-read success must be reported" + [ "$STEP_GH_CALLS" -ge 1 ] || fail "the live body was never re-read" + assert_contains "$STEP_GH_ARGS" "repos/owner/repo/pulls/123" \ + "the re-read must query this PR's own API endpoint" + pass "stale 'opened' snapshot passes once the live PR body is signed" +} + +test_unsigned_live_body_still_fails_with_guidance() { + run_step "$UNSIGNED_BODY" "$UNSIGNED_BODY" + expect_code 1 "$STEP_CODE" "a genuinely unsigned PR must still fail" + assert_contains "$STEP_OUT" "This PR was not raised through no-mistakes." \ + "failure must keep the contributor guidance" + assert_contains "$STEP_OUT" "PR author: someone" \ + "failure must still name the PR author" + [ "$STEP_GH_CALLS" -gt 1 ] \ + || fail "failing the check must retry the live re-read (gh called $STEP_GH_CALLS times)" + pass "unsigned live body still fails, after retrying the live re-read" +} + +test_api_failure_falls_back_to_failing_closed() { + # Empty live-body file makes the gh stub exit non-zero: a token/API problem + # must not crash the step under `set -eu`, it must fail the check cleanly. + run_step "$UNSIGNED_BODY" "" + expect_code 1 "$STEP_CODE" "an API failure must fail the check, not error out" + assert_contains "$STEP_OUT" "This PR was not raised through no-mistakes." \ + "API failure must still print the contributor guidance" + pass "live re-read failure fails the check closed with guidance" +} + +test_workflow_grants_pull_request_read() { + # The live re-read is only possible with this permission; without it the + # workflow would silently regress to snapshot-only behavior in CI. + assert_grep "pull-requests: read" "$WORKFLOW" \ + "workflow must grant pull-requests: read for the live body re-read" + # shellcheck disable=SC2016 # literal YAML expression, must not expand here + assert_grep 'GH_TOKEN: ${{ github.token }}' "$WORKFLOW" \ + "verify step must pass a token to gh" + pass "workflow grants the permission and token the live re-read needs" +} + +test_signed_event_body_passes_without_api_call +test_stale_event_body_passes_on_live_reread +test_unsigned_live_body_still_fails_with_guidance +test_api_failure_falls_back_to_failing_closed +test_workflow_grants_pull_request_read + +echo "all no-mistakes-required-workflow tests passed"