Skip to content

Latest commit

 

History

History
4815 lines (4420 loc) · 279 KB

File metadata and controls

4815 lines (4420 loc) · 279 KB

CLAUDE.md

Guidance for Claude Code when working in this repository.

Memory storage policy

  • Persist standing notes and memories only in repo-tracked files (via commits/PRs); do not save them to non-repo local paths.

Standing merge policy (mwc)

  • Standing mwc is active by default in Morrison-Lab/gha: AI agent sessions working in this repository have standing permission to squash-merge pull requests once they reach fully clean (all CI checks green and zero outstanding review findings), unless explicitly instructed otherwise for a specific PR or session.

Standing tag-slide policy

  • Sliding this repo's major tag is a standing grant: AI agent sessions may dispatch slide-major-tag.yml on their own judgment, without asking first (user directive, 2026-08-27), unless explicitly instructed otherwise for a specific occasion or session. Like the mwc grant above, it removes the asking and not the judgment.

  • The readiness bar is the same one mwc uses, on the commit being slid. Every check run on that exact commit is green or skipped, with none pending, read from the paginated check-runs endpoint rather than gh pr checks. "Merged work that consumers need" is the motivation, not the gate.

  • Green is necessary and not sufficient: check the callee's permissions: blocks before every slide. README.md's "Widening permissions is a breaking change" section is the authority and states the rule, the parse-time failure mode, and the remedy: prefer a major bump, and where one is disproportionate, find the callers and PR the caller-side grant before sliding. Follow that remedy; this bullet only adds the check that makes it fire, because the rule's failure mode is that nobody consults it.

    That is the measured lesson rather than a hypothetical. gha#685 recorded the identical incident when gha#638 added issues: read to ai-code-review.yml. The rule was written down, with a worked precedent, and gha#830 added checks: read anyway; every check was green, the slide onto c07f7d45 was correct under the readiness bar above, and 17 of the 18 repositories pinning that workflow at @v2 lost review dispatch (gha#831, "Same class as #685"). A green readiness bar is what made it feel safe, so the bar is where the check belongs.

    The reason it stays invisible: the callee's own checks all pass, and this repo's dogfood caller is typically updated in the same PR, so the one repository anyone would check first is immunized against the very regression being shipped. Widening permissions: is one of a family of breaking changes CI cannot see, which includes renaming or removing a workflow_call input, changing an OPTIONAL input's default (a required input's default is unreachable, so changing it breaks nobody), making an existing optional input required, requiring a new secret, and renaming or removing a JOB --- this repo's own reference pages tell consumers to put review / require-review and review / require-clean-verdict in branch protection, so a rename blocks every merge in every consumer.

    Before sliding, diff the callees' job permissions: blocks against the currently-tagged commit. Treat an ADDED key, or a WIDENED value --- read to write, or a whole block collapsing to permissions: write-all, neither of which adds a key and both of which startup-fail a narrower caller just the same --- as a stop --- meaning go find the callers and PR their grants, not merely eyeball the hit. Find them per workflow, and run the query BOTH unscoped and owner-scoped, taking the union. The owner list is hand-maintained and has gone stale before, and a push to a caller's file can drop that file out of the code-search index until it is reindexed --- measured 2026-09-07, a caller whose matched line had been unchanged for six weeks vanished from results after an unrelated edit at 01:55 PDT, was still absent at 02:24, and was back by 02:53. So union the two forms, treat any count as a floor, and re-run after a delay when any caller may have been touched recently. See REVDEPS.md for the measurements and the owner list:

    # Derive the major rather than hard-coding v2: gha#833 cuts a v3.
    # An empty $major here silently widens the search to every tag, pulling in
    # @v1 callers, so assign it first. Measured 2026-09-07 unscoped, the
    # unpinned form returned more hits than the pinned one.
    major=$(git ls-remote --tags origin 'v*.*.*' \
      | sed 's#.*refs/tags/##; s/\^{}$//' \
      | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -1 | cut -d. -f1)
    # --limit: a truncated gh search is silent. Re-raise the cap and re-run
    # if the hit count comes back EQUAL to it (see REVDEPS.md). Do not trust a
    # remembered ceiling -- counts move, and the pinned and unpinned forms of
    # the same query return different ones.
    gh search code "Morrison-Lab/gha/.github/workflows/<name>.yml@$major" \
      --json repository,path --limit 100
    # ... and again with --owner for each owner in REVDEPS.md; union the two.
    major=$(git ls-remote --tags origin 'v*.*.*' \
      | sed 's#.*refs/tags/##; s/\^{}$//' \
      | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -1 | cut -d. -f1)
    # Resolve the tag from the REMOTE: a plain fetch will not move an existing
    # local tag, so a local rev-parse reports the PRE-slide commit (see
    # "Re-running failed jobs cannot verify a tag slide" below).
    # --tags --force fetches the tag OBJECT, not just its sha. ls-remote reads
    # the remote without fetching anything, so on a shallow clone -- which is
    # what actions/checkout gives you by default -- diffing against that sha
    # dies with "fatal: bad object". --force because a slide moves the tag.
    # Reproduced on a --depth 1 clone, 2026-09-07.
    git fetch -q --tags --force origin main
    tagsha="refs/tags/$major"
    # Both extensions: a *.yml-only glob is the drift this repo has been
    # bitten by twice (see workflow_discovery.py), and it is untestable here
    # because the tree currently holds no *.yaml workflow.
    # Callees only: a caller's own grants are not part of anyone's contract.
    for wf in $(grep -rl 'workflow_call:' .github/workflows \
        --include='*.yml' --include='*.yaml'); do
      # --diff-filter=M: a workflow ADDED since the tag has no callers yet,
      # so every permission line in it would be a false positive. No workflow
      # is added in the current v2..main range, so the filter drops nothing
      # today; it is here for the ranges where one is. To see the shape,
      # diff across a range that adds one -- 402d17a3~1..402d17a3, which
      # added check-code-similarity.yml (gha#728) -- with and without it.
      git diff --diff-filter=M "$tagsha" FETCH_HEAD -- "$wf" \
        | grep -E '^\+ +[a-z-]+: (read|write)' && echo "  ^^ in $wf"
    done

    A removed key is fine; additions and widenings break callers. Read this as a prompt rather than a gate; gha#836 carries the reasoning and tracks replacing it with a parsed per-job set comparison. It greps ADDED DIFF LINES, so a key whose only change is its trailing comment shows up as a hit, a genuine addition to a job that previously had no permissions: block at all shows up the same as any other, and it cannot say WHICH job gained the key. --diff-filter=M drops workflows added since the tag, which have no callers to break; a workflow RENAMED since the tag is dropped with them, so check any rename by hand. It also enumerates callees from the WORKING TREE while diffing FETCH_HEAD, so a callee that exists on main but not in your checkout is skipped silently. The value pattern is unanchored, so write matches the prefix of write-all and an indented permissions: write-all is caught --- but a WORKFLOW-level one at column 0 is not, since the pattern requires leading space. No callee has a workflow-level block today (every callee job declares its own, which would override one anyway), so that gap is currently unreachable rather than merely unlikely. Confirm each hit against the two commits before treating it as a stop.

  • Re-read main's tip immediately before dispatching, and again after. slide-major-tag.yml tags $GITHUB_SHA --- whatever main points at when the run executes --- rather than a SHA you nominate. The standing mwc grant above means another session may squash-merge while you are deciding, so the commit you vetted and the commit that gets tagged are not guaranteed to be the same one. Confirm afterwards that the tag landed on a commit you actually checked.

  • This matters more here than the wording suggests. Consumers pin the floating tag, so advancing it is what rolls a change out to everyone at once --- which is precisely why the slide is workflow_dispatch-only rather than automatic on merge (slide-major-tag.yml's own header states this). An unslid tag therefore means merged review-infrastructure fixes reach nothing, gha's own PRs included. Measured 2026-08-27: gha#674 merged, and all seven then-open PRs stayed unreviewable until the tag moved --- each of the seven edits a top-level workflow file, which is what the pre-#674 guard skipped on.

  • Verify the slide from the remote, never a local tag. A plain git fetch --tags refuses to move an existing tag, so a local git rev-parse reports the pre-slide SHA and reads exactly like a slide that did not happen --- see Re-running failed jobs cannot verify a tag slide for the mechanism. Derive the tag name rather than hard-coding v2: resolve-major-tag.sh takes it from the latest vX.Y.Z tag, so a v3.0.0 would make this slide v3. Derive it from the REMOTE too, for the same reason the check itself reads the remote --- a local git tag --list is subject to exactly the staleness this bullet is about. Note the repo publishes no GitHub Releases, so gh release list returns [] here and cannot be the source:

    major=$(git ls-remote --tags origin 'v*.*.*' \
      | sed 's#.*refs/tags/##; s/\^{}$//' \
      | grep -E '^v[0-9]+\.[0-9]+\.[0-9]+$' | sort -V | tail -1 | cut -d. -f1)
    git ls-remote origin "refs/tags/$major" "refs/tags/$major^{}"

    Both refspecs, because an annotated tag's bare line is the tag object rather than the commit; they agree today only because this workflow writes lightweight tags.

  • Say what moved, because a slide is effectively one-way. There is no un-slide workflow: slide-major-tag.yml only ever force-moves the tag forward to main's tip. Reverting means a manual git push --force by someone holding contents: write, and any consumer run that already resolved the tag during a bad window cannot be recalled at all. So name the old SHA, the new SHA, and which merged PRs the tag now carries --- that report is what makes a bad slide detectable, which is the most the reporting can buy.

  • Do: slide when the commit is green, the callees' permissions: blocks gained no key and widened no value since the tagged commit, and consumers need it --- then report both SHAs.

  • Don't: read a green readiness bar as the whole gate; it was green when gha#830 shipped the outage.

  • Don't: read this as covering a release or version bump, another repository, or a slide over a commit whose checks you have not read.

About this repo

Central, reusable GitHub Actions for d-morrison / UCD-SERG / ucdavis R-package and Quarto repositories (see README.md). Each capability ships as a composite action plus a workflow_call reusable workflow. Consumers pin the major tag each capability's own reference page documents (@v1 for most, @v2 for preview, preview-deploy, cleanup-pr-previews, quarto-publish, test-coverage, check-equation-renders, check-bibliography-dois, check-phi, check-junk-files, check-links, check-non-standard-chars, claude, claude-code-review, update-snapshots, lint-yaml, lint-markdown, lint-qmd, lint-changed-lines, lint-changed-files, check-new-line-breaks, check-secrets, request-dependabot-review, sync-upstream, check-news, altdoc-multiversion-docs, report-failure, gemini, gemini-code-review, antigravity-code-review, cursor-code-review, ai-code-review, opencode-code-review, bump-dev-version, version-check, small-model-agent, check-ai-tells, lint-workflows, spellcheck, check-typos, check-extra, check-formatting, claude-manage-project, r-cmd-check, check-code-similarity, and check-one-function-per-file -- see the Versioning section of README.md). @v1 was frozen at the pre-2.0.0 snapshot and has picked up no fixes since, which is why the capabilities above moved to @v2.

Layout

  • conductor/ -- A pure-documentation top-level directory holding the Morrison-Lab "Conductor" orchestration tool scaffold (internal tooling for AI-agent workflow management). It contains no reusable workflows or composite actions; it is a repository of markdown templates (workflow.md, plan.md, product.md) used by agents operating in this repository to track complex implementation tracks. Note that conductor/archive/*/plan.md files hold historical tracking data for completed tracks, and their cited commit SHAs may reflect divergence from main due to out-of-band delivery.

  • Per-capability composite-action directories at the repo root, each with an action.yml and, for R/Python capabilities, a language-specific helper script -- e.g. check-bibliography-dois/ (R), check-extra/ (R), check-non-standard-chars/, check-phi/, and check-new-line-breaks/ (Python; the last mirrors check-phi's diff-scoped-by-base-ref pattern, but skips the check entirely rather than falling back to a whole-tree scan when the diff can't be computed, since a whole-tree scan here would reflag a corpus's pre-existing long-line drift, which is exactly what the diff-scoping exists to avoid; since gha#684 an added line whose exact text was also deleted in the same diff is exempted as moved-not-new, so a file split does not reflag relocated content either). check-typos/ (Python wrapping the crate-ci/typos CLI) uses that same skip-not-fallback for misspellings: a whole-tree first run would reflag every known misspelling the corpus already carries, and unknown jargon is not an error, so there is no inst/WORDLIST to grow into the way spellcheck.yml does. check-formatting/ wraps posit-dev/setup-air and air format --check -- <path>. It has no helper script: Air is a Rust binary, so there is no R session. Whole-tree (not diff-scoped): the check is check-only, and consumers opt in with their own reformat commit, so a first run going red on an unformatted tree is the intended adoption cost rather than pre-existing drift to tolerate. check-junk-files/ (shell) is a third scoping: it scans neither the diff nor the history but the index (git ls-files -i -c -X), for tracked operating-system and editor detritus. Diff-scoping is wrong here for the opposite reason it is right for check-new-line-breaks: a .DS_Store committed long ago is still a live defect rather than pre-existing drift to tolerate, and clearing it costs one command. It passes no --exclude-standard, so a file the caller force-added despite its own .gitignore is not second-guessed, and its paths-ignore becomes git pathspec exclusions rather than gitignore ! lines -- negation is matched per pattern against the full path, so !vendor/ re-includes vendor/.DS_Store not at all and would exempt nothing silently (verified against git 2.50.1, both directions). Its patterns default is deliberately the set usethis::git_vaccinate() writes (read from r-lib/usethis's own git_ignore_lines, not from the rendered reference page), plus ._*, Thumbs.db, and desktop.ini, which vaccination does not cover -- so the remedy the failure recommends is never narrower than the check's own scope, and the gap is stated rather than left silent. check-code-similarity/ (Python wrapping the JPlag jar) is scoped differently again from all of these: neither the diff, the index, nor the history, but a caller-supplied corpus. It compares one root of submissions against another, where JPlag treats each child directory as one submission --- so the population is whatever the caller assembled, and a root of loose files is refused rather than compared against nothing. Diff-scoping would be actively wrong here, since a copied file the current PR never touched is exactly what the check exists to find. JPlag was chosen over MOSS because it computes locally, where MOSS submits source to Stanford's servers, and over Dolos because R is not among Dolos's default tree-sitter parsers. The jar is pinned by version and SHA-256, since the tool reads every line of the caller's source; a mismatch refuses to run, and deletes the file only when it came from our own cache rather than from the jar input. That input exists so the distinction is reachable through the action rather than only through the script --- review caught the prose describing a caller-facing feature that, at the time, only a direct script call could use. check-secrets/ (shell) is the deliberate counter-example to that pattern: it is the one check that scans history rather than a diff, because a secret committed and later removed stays fetchable through the API, so diff-scoping would miss the case the capability exists for. It refuses a shallow clone rather than reporting a partial scan clean, and it wraps the MIT gitleaks CLI rather than gitleaks/gitleaks-action, which is proprietary and needs a paid licence for organization accounts. check-links/ bundles lychee.default.toml; check-one-function-per-file/ bundles the composite action, parser script, and pytest suite for enforcing single function definitions per file; preview/, quarto-publish/, open-sync-pr/, and resolve-pr-info/ are action-only (the last two are shared internal helpers: open-sync-pr for push-and-open-PR used by bump-submodule, sync-shared-fragments, and sync-upstream; resolve-pr-info for PR branch/head-repo/fork lookup used by ai-code-review, gemini, and dispatch-review).

  • .github/workflows/ -- the workflow_call reusable workflows that wrap the composites (one per consumer-facing capability -- the shared internal open-sync-pr composite has no wrapper), plus the claude.yml and claude-code-review.yml reusable wrappers, and _selftest.yml, which exercises composites on every PR -- local ./ refs for pre-release capabilities, and @v1 through the reusable-workflow wrappers for stable ones. claude-bot.yml and claude-review.yml are event-triggered workflows that run the Claude bot in this repo, not workflow_call wrappers.

  • Several workflows have no corresponding root composite: check-news.yml, summary.yml, and preview-deploy.yml are workflow_call reusable workflows that wrap external actions; cleanup-pr-previews.yml is a self-contained workflow_call reusable workflow (inline shell logic, no external composite); r-cmd-check.yml wraps r-lib/actions check steps in two mutually exclusive jobs (full OS x R-version matrix, plus a hard-dependencies-only job) because the matrix, the job if:, and the optional Linux container are job-level and cannot live in a composite; altdoc-multiversion-docs.yml is also self-contained but pairs inline shell logic with the three internal composites below -- generate-altdoc-version-dropdown, generate-altdoc-landing-page, and resolve-altdoc-base-url -- no top-level render/deploy composite of their own (the render+deploy sequence is inherently stateful/ordered, so splitting it into a separate composite the way quarto-publish/preview do would add indirection without reuse value, per the one-genuine-consumer-pattern reasoning in shared/principles/README.md's "How the principles relate" section of Morrison-Lab/ai-config); bump-submodule.yml, sync-shared-fragments.yml, and sync-upstream.yml are workflow_call reusable workflows that call the shared internal open-sync-pr composite (sync-upstream merges an upstream repo's branch into a fork via git merge --squash -- which stays out of a MERGE_HEAD state so open-sync-pr's git switch -C works -- then lets open-sync-pr commit the merge and open the PR); request-dependabot-review.yml similarly calls the internal build-reviewer-args composite (see below) for its reviewer-list split/trim logic; slide-major-tag.yml is dispatch-triggered and runs only in this repo.

  • .github/actions/checkout-submodules/ -- a small shared composite reused by the reusable workflows.

  • .github/actions/parse-workflow-ref/ -- a small composite action that parses a github.workflow_ref/github.job_workflow_ref-shaped string (owner/repo/.github/workflows/<file>@ref) into its repo/path/ref parts, shared by every claude-code-review.yml and claude-review.yml step that needs to pick one of these strings apart instead of duplicating the sed logic inline. It has to be a composite action rather than a plain checked-out script (like check-review-execution.sh below) because some call sites run before any checkout has happened -- a composite action's own files are available via uses: regardless of checkout state, which a bare script path is not.

  • .github/actions/detect-pr-workflow-edits/ -- wraps scripts/detect-pr-workflow-edits.sh, which classifies whether a PR's changed-file list includes top-level .github/workflows/*.yml / .yaml (not scripts/ nested under that directory, and not composite action.yml files). claude-code-review.yml uses it to restore default-branch workflow copies instead of skipping the review (gha#598); dispatch-review.sh uses it to omit --ref so GitHub executes the default-branch caller rather than the PR head's YAML. A missing PR_CHANGED_FILES variable fails closed (exit 2) rather than reporting a clean tree. Listing the PR's files goes through list-pr-changed-files.sh, which fails closed when GitHub's files endpoint returns fewer paths than the PR's changed_files count (that endpoint caps at 3000 files; a 200 with a short list is not a complete tree).

  • .github/actions/restore-default-branch-workflows/ -- wraps scripts/restore-default-branch-workflows.sh, which deletes .github/workflows/ then checks it out from origin/<default-branch>. A pathspec checkout alone does not delete a workflow the PR added, so the rm -rf is load-bearing (pinned by the suite's extra-file case). The script probes git cat-file -e "$ref:.github/workflows" before deleting, so a missing tree on the trusted ref fails rather than wiping the working copy. On restore, it drops .github/workflows/.restored-from-default-branch so workflow-parsing test suites and audits skip with a notice rather than asserting against default-branch files (gha#765).

  • .github/actions/run-review-guard/ -- a thin composite-action wrapper around check-review-execution.sh (below), invoked from claude-code-review.yml's "Fail the check if the review did not complete (attempt 1)" step (and again from its retry counterpart -- see run-claude-review-attempt below). #191 tried to locate that script by resolving Morrison-Lab/gha's own repo/ref from github.job_workflow_ref and checking it out into a side directory, but that context var came back empty at runtime on real consumer runs even though the calling step passed it correctly (gha#196) -- the #191 fix was only unit-tested via the sed-parsing logic in isolation, never exercised end-to-end. github.action_path doesn't have that failure mode: a composite action's own files are always reachable through it regardless of how the calling reusable workflow was invoked, the same reasoning parse-workflow-ref itself relies on.

  • .github/actions/run-claude-review-attempt/ -- wraps the single anthropics/claude-code-action call claude-code-review.yml uses to review a PR (allowedTools/disallowedTools, the review prompt). Extracted into a composite action so claude-code-review.yml can invoke it a second time, unchanged, as a same-prompt retry when the first attempt completes without an SDK error but never states a verdict -- the "stub review" signature (gha#185): check-review-execution.sh surfaces this specific case via a stub_review output (through run-review-guard), and the workflow retries once before failing the check for real. Keeping the claude-code-action call itself in one place (rather than duplicating its ~100-line with: block between two near-identical steps) follows this file's own DRY guidance below. claude-code-review.yml's "Resolve final review outcome" step is the single point that decides which attempt's output to use and the only step that actually fails the job when neither attempt produces a usable review -- both "Fail the check" steps are continue-on-error: true so a recovered retry doesn't leave the job red.

  • .github/actions/upload-review-execution/ -- resolves claude-code-action's execution_file output (with its temp-path fallback) and uploads it as a workflow artifact, in one composite action shared between attempt 1 and the run-claude-review-attempt retry above -- the same DRY rationale that motivated extracting that (much larger) composite action, just at a smaller scale (gha#201 review).

  • .github/actions/pack-review-payload/ -- writes the files claude-code-review.yml's posting job needs (payload.json plus optional review.txt / denied_tools.txt) and uploads them as a workflow artifact (gha#580). The model job (claude-review) holds contents: read and cannot post; post-review downloads this artifact and holds pull-requests: write / issues: write. Located via github.action_path because callers checkout their own repo. _selftest.yml exercises it via a local ./ ref (upload: false); the reusable workflow's @v2 call is the usual bootstrap gap until the tag slides.

  • .github/actions/build-quota-skip-notice/ -- wraps scripts/build-quota-skip-notice.sh, which renders the PR notice claude-code-review.yml posts on a graceful quota skip (gha#804). The guard already told three cases apart in the run log -- no secret configured, the API rejecting the first request at zero cost (gha#396), and a 429 part-way through a review that had spent real turns (gha#520) -- but the notice stated one disjunction for all of them, so after a mid-run 429 it told a maintainer who had just repaired the secret that no secret was configured. check-review-execution.sh now emits quota_reason (rejected-at-door or midrun-429) and a single-line quota_message beside quota_exhausted=true, run-review-guard re-declares both as outputs, the pre-flight step emits missing-secret, resolve-final passes all of it through env:, pack-review-payload packs it, and the posting job hands it to this composite. Five things constrain any change to it. The run-review-guard output block is the hop no offline test crosses. A composite's step outputs are invisible to its caller unless re-declared in outputs:, and the first draft omitted that block, so every value arrived empty and the notice fell back to the old wording for both real cases while every script test stayed green. run-review-job-split-tests.py now asserts that every steps.fail-check*.outputs.<name> the workflow reads is declared there. The message is redacted before it leaves the guard. A door rejection is exactly where the SDK quotes credential context (the gha#686 entry above records one), and the comment is not masked, so the message runs through the same chain denied_tools does -- one jq def redact: prelude (redact_jq) shared by both, rather than a second copy of five patterns. run-fixture-tests.sh builds token-bearing door and mid-run fixtures at run time and asserts both the log line and the quota_message output carry *** on each path; a _selftest.yml uses: ./ call against the door fixture is what proves the value: mappings themselves. The headline always begins Claude review skipped, because classify-review-delivery.sh keys on that phrase; its test suite generates a body per reason from the real builder so the two cannot drift. An unrecognized or empty reason renders the pre-gha#804 disjunction rather than erroring, so a guard at an older tag still gets a notice. And quota_message is API-authored free text, so it takes the same env: route denied_tools does (gha#541). The guard writes it key=value, as it does denied_tools, which is safe because the same file collapses it to one line two lines earlier; the two downstream re-emissions (resolve-final and Load review payload) use the delimiter form, because their single-line guarantee lives in another file. The builder collapses it again before rendering, so a newline in it cannot escape the blockquote. run-build-quota-skip-notice-tests.sh pins the per-reason wording, the negative claims (a mid-run 429 must not say no secret is configured), the verbatim message, and the collapse step's run-id capture; run-fixture-tests.sh asserts quota_reason and quota_message per skip fixture (including a multi-line door message, the only fixture that can see a broken door-path extraction) and that a passing run carries no stale reason.

  • .github/actions/extract-total-cost/ -- wraps scripts/extract-total-cost.sh, which extracts total_cost_usd from a claude-code-action execution-output file's last result event. claude.yml calls it once, right after "Run Claude Code", and both its comment-posting steps ("Post Claude's response if no code was committed" and "Finalize PR for issue trigger") read the shared steps.cost.outputs.cost -- a single extraction instead of duplicating the jq filter at both call sites (gha#219 review finding 1).

  • .github/actions/sum-costs/ -- wraps scripts/sum-costs.sh, which sums two (each optionally empty) total_cost_usd values. claude-code-review.yml's "Sum attempt costs" step calls it once, combining the initial attempt's cost with the gha#185 stub-retry attempt's cost when one ran, so the arithmetic has offline test coverage instead of being an inline awk one-liner only exercised by a live two-attempt review run (gha#219 review finding 5).

  • .github/actions/detect-review-request/ -- wraps scripts/detect-review-request.sh, which decides whether a comment/review body is an explicit @claude review request. claude.yml calls it twice: once on the trigger comment, and once (via its bodies-file input) on every comment posted after the trigger, for the late-arrival rescan. Those two paths previously each carried their own copy of the pattern -- one a bash regex, one a jq test() -- which is how they drifted apart, and how a consumer ended up adding a local dispatch job that double-dispatched every plain @claude review (UCD-SERG/serodynamics#277). Two things to know before widening the pattern: a false positive is the expensive error, because match == 'true' suppresses claude.yml's "Post Claude's response if no code was committed" step, so a misfire swallows the answer to a question the user actually asked -- which is why the lead-ins between @claude and review are a closed set of function words (please, can/could/would/will you, kindly, pls/plz) rather than "any few words". Both sides of review need that closed set, not just the lead-in. gha#341 constrained only what may precede the keyword, which left @claude can you review this and fix the failing test? matching: the object of review names a topic to examine, so the comment is a question for the agent, and dispatching it suppressed the answer. So a second closed set governs what may follow -- deictic references to the PR under discussion (this, the latest changes, again) and trailing politeness -- and the request must then end its line (gha#346). That is also what keeps review a whole word, so the older [^[:alnum:]]|$ guard against @claude reviewer is gone rather than duplicated. The trade is that a pure review request with an unlisted object (@claude review the changes I just pushed) now self-reviews instead of dispatching, which is the cheap error by the same asymmetry. Both known cases are pinned in the test table, so widening TAIL_WORD to recover them stays a deliberate decision. A third portability note sits alongside the jq one below: the script normalizes CRLF with tr -d '\r' because GitHub delivers comment bodies with CRLF and the pattern anchors on a bare newline. The sed 's/\r$//' it replaced only worked under GNU sed -- BSD/macOS sed reads \r as a literal r -- and the composite probes base64 -d vs -D for the same reason. And bodies-file takes base64-encoded lines, not raw or NUL-separated ones: comment bodies are multi-line, and jq --raw-output0 needs jq 1.7 while runs-on is a consumer-settable input, so a runner with jq 1.6 would have failed into the || : fallback and silently reported "no late review". The composite decodes those lines into a pipe, NUL-separated, and the script reads stdin -- never argv. Linux caps a single argument at MAX_ARG_STRLEN (131072 bytes) independently of the far larger aggregate ARG_MAX, while GitHub allows 65536-character comments, which in mostly-4-byte UTF-8 is 256 KiB. So one emoji-heavy comment from any non-bot commenter fails execve with E2BIG, and under the composite's set -euo pipefail that reddens the whole calling job over an optional late-dispatch nicety (caught in gha#341's review; the test table's oversized-body case is the regression guard). Finally, the script does not match the raw body: it pipes each one through scripts/strip-non-invoking-markup.sh first, which removes blockquote lines, fenced code blocks, indented code blocks, and inline code spans. All four are standard Markdown for "this is a literal string, not something I mean", and treating them as text meant a comment documenting the accepted phrasings dispatched a review by quoting them (gha#344). It tracks CommonMark closely rather than approximately, because the two directions of error land on different callers: under-stripping dispatches a review off quoted text, while over-stripping drops a genuine request in the mention gate that shares the script (gha#342). Four things constrain any change to that stripper. A code span becomes the placeholder word elided rather than being deleted, because deleting it lets its neighbours close up into a request the author never wrote: a span sitting between the mention and the keyword would collapse into a dispatch. The placeholder also has to be letters that no caller's pattern accepts, which rules out the obvious [code] -- code is one of the TAIL_WORD alternatives, so the placeholder itself would have completed a match. And a code span is closed by a backtick run of equal length, so the scan measures runs rather than matching `[^`]*` -- that pattern matches the empty span between the two opening backticks of a ``...`` span and leaks the contents through, the same bug the Tests section records for check-new-line-breaks's strip_inline_markup. Third, the span scan runs over the whole body at once, not per line, because a span may contain a line break -- CommonMark closes it on the next run of matching length wherever that appears. That is also why indentation limits matter rather than being tidied away: a fence is capped at three spaces of indentation at both ends, so trimming indentation wholesale before the close test let a 4-space-indented delimiter (which is fence content) close the block early, and four columns after a blank line opens an indented code block in the first place. The blank-line precondition on that last one is load-bearing: without it an indented list continuation would be stripped, which drops a genuine request. Fourth, never write an interval expression ({m,n}) into that awk. mawk is Debian's and Ubuntu's default awk, selected through the awk alternatives link, so any image that has not installed and selected another implementation resolves awk to it. mawk 1.3.4 20240123 aborts the whole process on an interval --- REcompile() - panic: values still on machine stack --- rather than returning a verdict, so the abort is absorbed into a false for every input and a genuine review request is silently never dispatched. This defect was fixed in gha#457 (formerly tracked by gha#448 and gha#451): strip-non-invoking-markup.sh previously read if (bare ~ /^#{1,6}([ \t]|$)/) return 1. Always express the CommonMark 1-6 heading limit as ^#+([ \t]|$) plus a length check (or ^##?#?#?#?#?([ \t]|$)) rather than using interval quantifiers like {1,6}. Two things make this regression risk easy to miss. _selftest.yml is green on main throughout, so whatever awk the ubuntu-latest runner provides does not hit the panic --- which is not a claim about which awk that is, since actions/runner-images' Ubuntu2404-Readme.md names neither mawk nor gawk, and a container's own readlink -f /usr/bin/awk reports only that container. The guarantee stops at that runner either way: runs-on is a consumer-settable input, so a consumer whose runner resolves awk to mawk would get the abort if interval quantifiers were reintroduced, which is the same portability class as the three notes above. And bracketing the braces (the fix for a literal {} that mawk misreads as an interval) does not help here: the interval is the thing being asked for, so express the limit as a length check or unrolled quantifiers instead. Also, detect-review-request.sh previously swallowed stripper failures inside if condition evaluations (gha#451); body normalization now evaluates strip-non-invoking-markup.sh outside if constructs so script failures propagate under set -e, and claude.yml step Detect @claude review request carries continue-on-error: true so workflow runs tolerate stripper/engine failures safely. It lives in its own script rather than inline because the same constructs gate whether the agent runs at all (gha#342).

  • .github/actions/detect-bot-mention/ -- wraps scripts/detect-bot-mention.sh, which decides whether a body carries an @claude mention that is actually addressed to the bot rather than quoted while writing about it. claude.yml calls it from a cheap mention-filter job, for all four reactive events, and gates the expensive agent job on that job's proceed output (gha#554). It shares strip-non-invoking-markup.sh with detect-review-request (gha#342). Its bias is the opposite of that script's, and the two must not be harmonized on this point. There a false positive suppresses the agent's reply to a real question, so the matcher is deliberately narrow; here a false negative means a genuine request is silently ignored, so the gate answers "run" whenever any mention survives stripping, and treats an empty result (the step did not run, or failed) as "run" too. That is also why the matching stays plain-substring and case-insensitive, mirroring the contains() call it backs: a word-boundary rule would buy very little and risk exactly the false negative this bias rules out. The caller-side job if: and mention-filter's own if: still test the raw body, because a GitHub expression cannot strip Markdown, so a quoted mention still starts the filter job (a billed runner minute). What it no longer starts is the expensive claude job: no caller checkout, no model invocation, no review re-dispatch. An allowlisted issues.assigned event is exempt from the mention check (gha#552) and still proceeds with no mention anywhere in the issue.

  • .github/actions/report-push-failure/ -- wraps scripts/classify-push-failure.sh, which reads a failed git push's output and names the failure kind (workflows-permission, push-protection, non-fast-forward, other, plus no-push-attempt which the composite assigns when no log exists) plus advice for it. The composite adds what the script deliberately leaves out: it redacts any credential git echoed back in the remote URL, emits the ::error::, generates a git format-patch of the commits that could not be pushed, and comments all of it on the issue or PR. claude.yml calls it from two steps, one per push site -- "Push PR branch if Claude committed" and "Push branch for issue trigger". That second one is a step this PR split out of the old combined push-and- finalize step, so the push's own outcome can gate the finalize that follows it. Three things to know before changing it. The redaction is not belt-and-braces: Actions masks secrets in a run log and not in a comment body, so without it the push token would be published rather than starred out -- which is also why dry-run exists, so _selftest.yml can assert the redaction holds against a real call. The classifier keys on the refusing to allow ... to create or update workflow clause rather than on the trailing scope name, because GitHub words that tail differently per credential: a GitHub App is rejected for lacking the workflows permission, a PAT for lacking the workflow scope. And the fenced blocks in the comment body measure the longest backtick run in their content and open with one more, the same reasoning strip-non-invoking-markup.sh uses -- a patch that touches a Markdown file carries ``` lines of its own, which a fixed three-backtick fence would let close the block early. Two further behaviours are load-bearing rather than incidental, both found by gha#361's review. A missing push log is reported as the no-push-attempt kind rather than skipped: the calling step can die before its push (the auto-commit sweep, the fork lookup), and since `claude.yml` gates its response-post step off on that same failure, standing down here would leave the thread with no comment at all -- the exact outcome gha#360 exists to prevent. And the patch is truncated by reading a file, never a pipe: `printf ... | head -c` leaves printf writing to a closed pipe once head has its bytes, so any patch past the ~64 KiB pipe buffer raised SIGPIPE, which `pipefail` promotes to the pipeline's status and `set -e` turns into an aborted report -- losing the comment precisely for the large patches that most need preserving. Two more, from gha#361's second round. A rejection carrying GitHub secret-scanning markers (`GH013` and friends) gets its patch suppressed rather than posted: those commits carry the secret the push was blocked to contain, so rendering them into a public comment -- or the run log -- would republish it, and Actions' masking does not apply because a scanned secret is commit content rather than a configured `secrets.*` value. That suppression is a second output, `withhold-patch`, decided independently of `kind` -- do not re-key it on `kind`. `kind` is a first-match chain, so it answers "which explanation does the reader get", one rejection and one story. Whether the commits may be published is a different question, and tying it to `kind` made it answerable only for whichever clause happened to win: a push that both edits a workflow file and carries a secret matches the workflows-permission clause first, so a kind-keyed gate never fired and the patch went out with the live credential in it (round 5). The markers are therefore tested on their own, before the chain, and the composite gates publication on that -- erring toward withholding, since a needless withhold costs a re-run while a published credential cannot be recalled. When the markers fire but another kind wins, the classifier appends the no-patch explanation to that kind's advice, so the omission is never silent. And the byte budget bounds the whole body, not just the patch: the log gets a fixed slice and the patch takes the remainder, because capping only the patch let a verbose rejection carry the total past GitHub's comment limit on its own, which 422s the post and drops the report entirely -- the silent-thread outcome gha#360 exists to prevent.

  • .github/actions/report-gemini-failure/ -- wraps scripts/classify-gemini-failure.sh, which reads a failed google-github-actions/run-gemini-cli call's error output and names the failure kind (quota-or-auth -- rate-limit, auth rejection, or a suspended project, all graceful-skip -- or other, a genuine failure) plus advice. Deliberately simpler than classify-push-failure.sh: there is no patch to withhold and no credential to redact, since the input is API error text rather than a git push log, and the classifier itself does not embed the raw error output -- that stays the composite's job. The composite's fence_for()/truncate_to_bytes() helpers live in scripts/fence-and-truncate.sh, sourced by both this composite and report-push-failure -- extracted rather than pasted twice, per this file's own "factor shared logic into reusable units rather than copying it between files" guidance (gha#380 review finding 3). classify-gemini-failure.sh's own quota/auth regex only matches distinctive markers (RESOURCE_EXHAUSTED, PERMISSION_DENIED, a "code" JSON key or HTTP status line paired with 401/403/429, etc.), never a bare status code or a generic word like disabled/billing on its own -- run-gemini-cli's error output is raw stderr when stderr isn't valid JSON, so an unanchored alternative matches ordinary text in a realistic multi-line stack trace (a Node stack trace's own line:column numbers, an unrelated MCP log line) and would misclassify a genuine bug as a graceful skip -- exactly the failure mode this script's header comment says must never happen (gha#380 review finding 1). gemini.yml and gemini-code-review.yml both call it from a step gated on steps.gemini-run.outcome == 'failure': a quota-or-auth classification posts a > [!WARNING] comment and stops there, deliberately never retried -- retrying a suspended or rate-limited key wastes CI time and can look like continued automated abuse to Google, which is the opposite of what should happen. An other classification still posts (with a > [!CAUTION] framing) but the calling workflow fails the check for real, the same two-tier structure report-push-failure's kind decides. gemini-code-review.yml additionally gates a require-review job (mirroring claude-code-review.yml's own) on this: it skips gray rather than red when the review was a graceful quota/auth skip, or when a dispatch-guard block left the underlying review job's own result at success with no review having actually run (dispatch_guard_blocked output -- unlike claude-code-review.yml's dispatch-guard, which lives in a separate job and gates claude-review's job-level if: directly, gemini-code-review.yml's dispatch-guard is a step inside the same job as the review itself, so blocking it only skips downstream steps rather than the job as a whole). Consumers gating merges on this workflow should use review / require-review, not review / review, for the same reason claude-code-review.yml's own header documents. Its review job shares a cancel-in-progress concurrency group across the automatic pull_request trigger and gemini.yml's @gemini review dispatch, the same race CLAUDE.md's "A canceled review skips require-review gracefully" documents for claude-code-review.yml (gha#585). (Added after the "Default Gemini Project" API-key suspension incident, 2026-07-30, gha#379 -- see the Tests section below for the offline coverage and the _selftest.yml end-to-end proof.)

  • .github/actions/report-review-failure/ -- wraps scripts/compose-review-failure-report.sh, which builds the comment claude-code-review.yml posts when a review run finishes without a usable verdict (gha#543). Before it, every posting step in that workflow was gated on steps.resolve-final.outcome == 'success', so a no-verdict review skipped all four together -- the quota notice, the review comment, the cost comment, and the collapse step. The run spent real money, reddened require-review, and left the PR thread silent, which from the thread is indistinguishable from a reviewer that has not started yet. The same silent-thread class gha#360 fixed for the push path and gha#379 for the Gemini path. Five things constrain any change to it. The script composes; it does not classify. Its two siblings are handed raw error text and must work out what happened, whereas here check-review-execution.sh has already decided and Resolve final review outcome has already picked which attempt's decision stands. So failure_kind is an input, and re-deriving it in the workflow would be a second copy of one classification, free to drift out of step with the first. The script's only judgment about the kind is to normalize an unrecognized value to unknown, which is why kind= is echoed back rather than dropped: the caller reports the kind actually used, so a normalization is visible instead of silent. The denied-tool line is four-valued, not two. Names known, a real zero, a known count with unavailable names, and no denial data at all are different facts, and each licenses a different statement. A short-circuited run exits before the guard ever counts denials, so the count arrives empty there -- rendering that as "none" would assert the reviewer was not blocked by permissions on a run where nothing about permissions is known, and would send a triager to the wrong place. When a non-zero count is known but tool names are unavailable (such as a scalar count without array or a lost sidecar), reporting "not recorded" would falsely claim no denial data was produced; the true statement is that the count is known with names unavailable (gha#764). This is the same distinction check-review-execution.sh draws with its own denials_known flag. The threshold is passed through, never restated. STUB_RETRY_MAX_DENIALS is overridable, so a hard-coded 5 here would be right only until someone overrode it -- the two-declarations-of-one-default problem gha#303 pinned a test against. The guard emits max_denials, and an empty value drops the threshold clause rather than substituting a number nobody compared against. denied_tools reaches the calling script through env:, never through ${{ }} in a run: body. It is assembled from the commands the reviewer attempted, so it is agent-authored free text that routinely carries quotes, $, ;, and redirects -- gha#541's whole subject was a reviewer reaching for gh pr diff ... > /tmp/pr.diff. The first draft of Resolve final review outcome interpolated it inline beside the outcome and stub_review reads already there, which is safe for those two (the runner and the guard constrain them to a fixed vocabulary) and is arbitrary command execution for this one: a sample carrying $(...) runs inside a job holding CLAUDE_CODE_OAUTH_TOKEN, and needs no quote-breaking to do it, since command substitution happens inside the double-quoted assignment. Reproduced directly rather than reasoned about, and closed by an env: assignment, which the runner substitutes rather than bash parsing it. denials, max_denials, and failure_kind ride along under the same rule even though none of them needs it, so nobody has to re-derive which member of the group was the dangerous one. The YAML half cannot be unit-tested, so the composer's suite pins the half that can: such a value must render verbatim, neither executed nor mangled. The run URL is load-bearing rather than decoration. claude-code-review.yml's collapse step matches comments by the actions/runs/<id> link in their body, so without it every failed round would leave its own permanent, unfoldable copy on the PR. That collapse step is gated on this action's posted output rather than on the failure itself, mirroring gha#434's own fix: folding the previous run's notice after a failed post would leave the PR with no explanation at all, which is the outcome this action exists to prevent.

  • .github/actions/trigger-bugbot-review/ -- wraps scripts/trigger-bugbot-review.sh, which POSTs a PR URL to https://api.cursor.com/bugbot/review and fails unless the API returns "outcome":"success" plus a request_id. cursor-code-review.yml calls it so the reusable workflow can queue a Cursor Bugbot review without the caller's checkout containing this repo's scripts (github.action_path, same reason as build-reviewer-args). Success means queued: Bugbot posts comments and the Cursor Bugbot check itself, asynchronously. The API is Enterprise-scoped (admin:*); Team/individual installs enable Bugbot in the Cursor dashboard instead. A Team-plan key is not "almost Enterprise": the queue step fails with HTTP 401: Invalid Team API Key (dogfood cursor-review.yml, run 32694255358, 2026-08-24, gha#601). That error means the secret reached Cursor and was rejected; it is not a missing secrets: mapping. The key is sent as an Authorization: Basic header via curl --config, not on argv, and the script never prints it. --header is not enough: it still puts the base64 credential on curl's argv (ps / /proc/<pid>/cmdline). jq's // treats JSON false as empty, so .dry_run // empty drops the common "dry_run":false response and falls back to the locally requested value (gha#511); parse with a null check instead.

  • .github/actions/classify-review-verdict/ -- wraps scripts/classify-review-verdict.sh, which classifies an extracted review text as clean or not and emits clean and verdict (gha#812). It exists for the reason every other composite here does, and the reason is worth stating because the bug it fixes was invisible in this repo: the posting job of the reusable claude-code-review.yml runs in the CONSUMER's checkout, which carries none of this repository's script tree, so the plain run: bash .github/workflows/scripts/... form that #790 shipped exited 127 in every consumer repo. The failure was maximally confusing there, since the reviewer had already posted a clean verdict: post-review went red on the classify step and require-review and require-clean-verdict cascaded from it, so a PR with an approving review read as a failed review pipeline (measured on ucdavis/bcs#876, run 33622719245). _selftest.yml exercises it through two real uses: ./ calls, one Ready-for-merge and one Needs-more-work, asserting BOTH outputs on each: clean alone is a boolean that a composite hardcoding its outputs would also satisfy, so the second call is what proves the passthrough carries the script's real answer (gha#813 review).

  • .github/actions/check-credential-shape/ -- wraps scripts/check-credential-shape.sh, which decides whether a configured API credential secret is structurally usable as an HTTP Authorization header value. claude-code-review.yml calls it from a pre-flight step, before the review spends anything (gha#686). Three things constrain any change to it. The rule is INTERIOR whitespace, not any whitespace. A header value may carry none at all, so the strictest reading would reject a trailing newline too --- which gh secret set < file produces routinely, and which consumers may well be running on successfully today if the action trims before sending. Rejecting that turns a hardening change into an outage, and the asymmetry runs one way: a missed detection costs the badly-worded hard-error comment consumers already get, while a false detection blocks review entirely. The verdict is "every configured credential is unusable", never "some credential is". Which of the two secrets claude-code-action actually sends is its own precedence rule rather than something this workflow knows, so blocking while a well-formed alternative is configured would be a guess. The selftest's mixed good/bad case is what pins that, and it is the one case that passes under a narrowed verdict while every other case still passes. It never repairs the value. The observed case was 2931 characters on 62 lines, which is not a token with a stray newline but different content entirely, so stripping the whitespace would send a credential nobody chose and turn a nameable configuration defect into an opaque rejection. It fails fast and names the remedy instead. The detail line it emits reaches a PR comment, so it carries counts and a position and never any part of the value --- the same figures the SDK's own rejection message already prints.

  • .github/actions/build-reviewer-args/ -- wraps scripts/build-reviewer-args.sh, which splits a comma-separated reviewers list into a JSON array of trimmed, non-empty usernames. request-dependabot-review.yml calls it once to build its gh api -f reviewers[]=... arguments, so the split/trim logic has offline test coverage instead of only being exercised by a live Dependabot PR (gha#253 review: a bare IFS=',' read -ra doesn't trim whitespace, so "alice, bob" sent an invalid reviewers[]= bob and failed the job).

  • .github/actions/install-gha-scripts/ -- copies named scripts out of .github/workflows/scripts/ into a runner temp directory and outputs that directory, so a reusable workflow can call one from inside a run: block. The deliberate exception to the wrap-and-run shape every other helper here uses: ai-code-review.yml's candidate loop makes its decision inside a shell loop over a dynamic agent list, and a composite cannot be invoked mid-loop, so the alternative was inlining the matcher beyond the reach of the offline table tests this repo relies on for exactly that class of logic (gha#362). It resolves via github.action_path for the same reason run-review-guard does. Two behaviours are load-bearing: a name carrying a path separator is refused rather than sanitized, since every real caller names a bare filename; and a missing script is an error rather than a silent no-op, which would leave the calling loop invoking a file that is not there.

  • .github/actions/open-failure-issue/ -- wraps two scripts: scripts/select-existing-issue.sh, which picks the open issue an automated failure report should be appended to (exact, case-sensitive title match; lowest number wins when duplicates already exist), and scripts/split-csv-list.sh, which splits and trims the comma-separated labels input so each name is passed as its own --label. That second one is the gha#253 bug class again: gh issue create --label is a Cobra StringSlice, which splits on commas without trimming, so a natural bug, automated yields a second item beginning with a space, which matches no label and fails the whole call. build-reviewer-args.sh delegates its own split/trim to the same script, so the repo has one CSV splitter rather than two. The composite does the gh calls around it: list open issues, then either comment on the match or file a new issue. report-failure.yml and check-links.yml both call it. Two behaviors worth knowing before changing it: a label the calling repository does not define is dropped with a warning and the issue is filed anyway, since losing a failure report over a missing label is the worse outcome; and dry-run exists so _selftest.yml can exercise the action for real without filing an issue on every selftest run (the lookup still runs, so it needs only issues: read).

  • .github/actions/generate-altdoc-version-dropdown/ and .github/actions/generate-altdoc-landing-page/ - Python composites wrapping the scripts altdoc-multiversion-docs.yml needs (rewrite the navbar "Versions" dropdown; generate the root redirect landing page, and -- when the legacy-paths input is set -- a root 404.html redirecting retired version directories to their replacements, gha#301). Ported from d-morrison/rpt's bespoke .github/scripts/ copies, generalized to derive the docs base URL and default branch from the caller's own context instead of a hard-coded repo (see UCD-SERG/serocalculator#504). Both invoke .github/actions/resolve-altdoc-base-url/resolve_base_url.py directly via a github.action_path-relative path (the build-reviewer-args idiom described above) rather than nesting a uses: ./... step -- a relative local path inside a composite action resolves against the top-level workflow's own checkout, not the repo the enclosing composite was fetched from, so a nested uses: step would fail to find action.yml for any real consumer (gha#284 review). Sharing the script this way still gives both composites one source of truth for the base-URL derivation instead of each carrying its own copy. Within generate-altdoc-landing-page, its two generator scripts (the landing page and the gha#301 legacy-path 404.html) share _site_output.py for the OUTPUT_DIR/DOCS_BASE_URL plumbing they both need -- including the site-root default, which has to agree with action.yml's own output-dir default and is asserted to by a test rather than left to a comment (gha#303 review). generate-altdoc-version-dropdown likewise splits its version-labeling and navbar-rewriting helpers into navbar_version.py (gha#307): those are pure functions, so they get unit tests, while importing the script itself would run its whole top-level flow (git lookups, a required DOCS_BASE_URL). That module also owns the two label suffixes (" (stable)", " (dev)"), so the menu's own label is built in the same shape as the entries it sits above rather than drifting into a second format. The version in that label can still differ from the (dev) entry's, and deliberately does on a PR preview: the label comes from the rendered checkout's own DESCRIPTION (local_version), while the /dev/ entry always names the default branch's version (dev_version, read from origin/<default-branch>). A PR that bumps DESCRIPTION therefore shows its own version in the navbar while the menu still points /dev/ at what is actually deployed there -- both correct, since the reader is looking at the PR's build, not /dev/.

  • .github/actions/inject-canonical-urls/ -- wraps inject_canonical_urls.py, which adds a <link rel="canonical"> to every indexable page of a rendered altdoc/Quarto tree before altdoc-multiversion-docs.yml deploys it (gha#332). That workflow publishes the same site to /dev/, /latest-tag/, /vX.Y.Z/, and /pr-preview/pr-<N>/, so without this every page exists N times with nothing naming the authoritative copy. Four things constrain any change to it. A canonical is emitted only when the target exists: the composite reads the pages currently under /latest-tag/ off the deploy branch (git ls-tree, the same interrogation the /latest-tag/ bootstrap step already does) and a page with no counterpart there self-canonicalizes, because a canonical pointing at a 404 asks the indexer to credit a page that is not there -- strictly worse than emitting none. PR previews get noindex rather than a canonical, since a preview is ephemeral and is not the authoritative copy of anything. 404.html is excluded from the indexable set, as canonicalizing the page served for missing URLs would point every miss at a real page. And the verification pass re-reads the files rather than trusting the insertion pass's own report, because an off-by-one in the insertion index produces a plausible log and a broken page; only a re-read separates them. The step is wired in after every subdir= assignment and before every deploy, which is load-bearing rather than incidental -- placed earlier (the obvious spot, right after the sibling "Report an issue" rewrite) env.subdir is not yet set, so the self-canonical fallback silently receives an empty path.

  • .github/actions/bump-dev-version/ and .github/actions/check-dev-version/ -- composite actions wrapping description-version.R's pure read_version/versions_equal/bump_dev_version logic via the github.action_path-relative pattern described above (the build-reviewer-args idiom), since bump-dev-version.yml and version-check.yml are reusable workflows referencing this repo's own scripts and cannot rely on their caller's checkout containing them. bump-dev-version bumps the dev-version counter (the 4th .90NN component of DESCRIPTION's Version:) and hands off to open-sync-pr@v2; check-dev-version fails a PR whose DESCRIPTION differs from the base branch's at all, inverting the older RMI-PACTA/actions-derived version-check.yaml copies each repo used to carry, so no PR branch ever holds a competing version to conflict on (gha#390).

  • .github/actions/check-tag-drift/ -- composite action wrapping check-tag-drift.sh (which sources shared resolve-major-tag.sh, also used by slide-major-tag.yml) via github.action_path, deriving the active major tag from the latest semver release tag (vX.Y.Z) and emitting a GitHub Actions notice and job summary when main has unreleased commits ahead of the major tag (gha#309).

  • examples/ -- caller stubs consumers copy into their own repos.

  • README.md, CHANGELOG.md -- top-level project docs; REVDEPS.md -- lists registered downstream consumer repos. Every PR that

    changes user-facing behavior should add a changelog fragment under changelog.d/ (a <slug>.<category>.md file -- see changelog.d/README.md) rather than editing CHANGELOG.md directly, so parallel PRs never conflict on the same ## [Unreleased] lines. changelog.d/assemble.sh collates the fragments into CHANGELOG.md at release time. This is not CI-enforced (require-changelog.yml was removed).

When editing a consumer-facing capability, change the composite (<name>/action.yml, plus its helper script if one exists) and keep the wrapping reusable workflow and its examples/<name>.yml stub in sync. Internal-only composites (like open-sync-pr) have no wrapper or example stub to update. New .github/workflows/ changes are exercised by _selftest.yml; because brand-new actions aren't at the @v1 tag yet, the selftest runs them via the local ./<name> ref until release.

Migrating a consumer's bespoke workflow to a reusable one here needs a feature-by-feature diff, not just a structural read. A reusable capability can look like a superset of the bespoke version it replaces -- more inputs, more hardening, more edge-case handling -- while still missing one specific step the bespoke version had, because that step covered something the reusable capability's original author's own repos never needed. Read the bespoke workflow line-by-line and confirm each step (not just each job) has an equivalent in the composite/reusable version before treating the migration as drop-in; don't infer parity from the reusable version's inputs table or its being "the canonical, more capable version" in general. When a gap turns up, file it upstream (here) and defer that one file's migration rather than silently dropping the feature or hand-duplicating it in the consumer's stub. (UCD-SERG/serocalculator#548/#549: test-coverage.yml looked like a straightforward superset of serocalculator's bespoke test-coverage.yaml -- same coverage measurement, same testthat-output and failure-artifact steps -- but was missing the JUnit-report upload to Codecov Test Analytics (codecov/test-results-action) the bespoke version also did; gha#234 tracks closing that gap, and the consumer left that one file unmigrated in the meantime rather than lose the feature.)

A new composite action cannot gain its first @v2 caller in the same PR that introduces it, and whether that bites depends on whether the caller is dogfooded here. A uses: ref is resolved when the job is prepared, before any step runs and before any step-level if: is evaluated, so a reference to Morrison-Lab/gha/.github/actions/<new-action>@v2 fails the whole job with Can't find 'action.yml' ... @v2 until @v2 is advanced past the merge -- even when the step is gated on failure() and would never have run.

The Tests section's build-reviewer-args paragraph records the same bootstrapping gap, and treats it as a coverage limitation: that workflow's own reusable layer cannot be exercised end to end until the tag advances. That reading is right there, because request-dependabot-review.yml only runs on Dependabot PRs, so nothing goes red in the meantime. When the new caller is something _selftest.yml invokes through a local ./ ref on every PR (check-links.yml is the one that does), the gap stops being a coverage footnote and becomes a red check on every PR in the repo, with no way to fix it inside that PR: a relative local path is not a workaround either, since inside a reusable workflow it resolves against the caller's checkout (gha#284).

So split the work: land the action plus its non-dogfooded callers first, and migrate a per-PR-dogfooded caller in a follow-up once the tag has moved. (gha#326: check-links.yml's migration and website-publish.yml's dogfood job were both cut from that PR for this reason and landed in gha#327, after links / link-checker went red on exactly this.)

A brand-new capability that ships at a tag newer than @v1 (because @v1 was frozen before it existed -- see slide-major-tag.yml / the Versioning section of README.md) needs its major tag updated at two distinct kinds of site, not just the obvious one:

  1. Capability-specific refs -- the new capability's own caller stub (examples/<name>.yml) and reference-page example (website/reference/<name>.qmd).
  2. Blanket-rule prose -- any general "pin every reference to @v1" statement elsewhere (README.md's Versioning section, website/workflows.qmd) needs an exception clause, even though it never names the new capability.

Grep the repo for @v1 rather than relying on memory of where it appears. Missing either kind surfaces as a workflow-not-found error for consumers who copy that spot literally (gha#148, caught across two review rounds).

When narrowing an already-fixed blanket claim, re-grep the WHOLE repo after every edit -- not just the files you already know about. The same versioning

convention gets restated in multiple, independently-worded spots: not just once per file, but in separate sections of the same file (e.g. README.md's ## Versioning section and its nested ### Pinning third-party actions subsection both needed the same @v1/@v2 exception clause), and across sibling pages that all describe the tag scheme (website/index.qmd's nav blurb, website/versioning.qmd, website/workflows.qmd, CLAUDE.md's own "About this repo"). Fixing the first instance you find and moving on invites the reviewer to find the next one in a later round -- gha#181 took six review rounds to fully sweep this exact pattern (@v1/@v2 scoping) because each fix only searched the files already in the diff. Before considering a versioning-prose fix complete, grep -rn "@v1\|@v2" (or whatever pattern is narrowing) across the entire repo, not just the files already touched.

That rule is not about versioning, and its own wording is what hides that. Every example above names @v1/@v2, and the closing sentence says "versioning-prose fix", so a sweep of some other repeated string does not read as covered -- and the identical failure then repeats verbatim. The d-morrison/gha -> Morrison-Lab/gha retarget hit it twice: one pass missed website/_quarto.yml entirely, and gha#374's own first pass left five more live sites, four of them the exact categories that PR was fixing elsewhere (the generated-consumer-PR-body link, the "not d-morrison/gha's own tree" comment). Read it as applying to any repeated string being retargeted -- an owner, a URL, a tag, a renamed input -- and grep for that string rather than for @v1.

A completeness claim in a changelog fragment is the one claim neither a reviewer nor a check can verify from the diff, so it needs the grep before it ships. "The last stale X references are gone" is an assertion about the tree rather than about the diff, so a reviewer reading the diff has nothing to check it against, and no CI job tests it either. It also reads as settled, which is what stops anyone from re-running the grep later. So when a fragment claims a sweep is complete, run the whole-repo grep and either make the claim true or enumerate the carve-outs explicitly. gha#374's review caught exactly this, citing the paragraph above as the governing principle -- which is the evidence that the rule was right and only its stated scope was too narrow.

Adding a new workflow_call input to an existing reusable workflow needs its own doc sync at three sites beyond the workflow file itself, or the input is invisible to a consumer skimming the docs:

  1. README.md's per-workflow table row's "Key inputs" cell.
  2. website/workflows.qmd's equivalent table row -- a separate table, not generated from README.md, so it drifts independently.
  3. website/reference/<name>.qmd's Inputs table, plus a commented usage line in its ## Example block.

Grep the repo for the workflow's filename (e.g. claude-code-review.yml) across README.md, website/workflows.qmd, and website/reference/ rather than assuming only one needs the update. Caught across four review rounds on gha#161 -- the fix for round 2's finding (missing composite) surfaced round 3's finding (docs out of sync), whose fix left one more untouched table row that round 3 flagged as out-of-scope, fixed anyway before round 4 confirmed clean.

Widening a job's trusted-author if: gate to admit a new event type needs a three-way audit across everything that decides something about the new path, not just the gate itself. A productive audit of one direction is not evidence that other directions are clean (gha#552/#553):

  • Downstream -- steps in the same job whose if:/env: read event-shaped context (github.event.issue.number, github.event.comment.body, etc.). A step that looks safely scoped -- e.g. gated on a steps.dedup.outputs.skip != 'true' flag -- can still run and fail under the newly-admitted event, because that flag was never set for it either; the flag and the missing context are two independent gaps, and fixing the top-level if: closes neither. Grep the same job for every step reading event context and add the same admit condition (or an explicit exclusion) to each one. (gha#245/#246: widening claude.yml's job if: to admit workflow_dispatch/schedule left two post-steps -- Acknowledge @claude mention and Post Claude's response if no code was committed -- still ungated for those events; both ran and attempted gh issue comment "" on every unattended run, one failing visibly, contrary to the PR's own prompt text claiming no post-step would post a reply. Caught by review, not by the author's own initial self-check.)

  • Upstream -- the caller-side job if: in examples/ and in this repo's own dogfood caller workflows. A reusable workflow is never invoked if the caller's gate doesn't admit the event, and the new input is then silently inert for any consumer following the docs (gha#552 review: adding dispatch-on-assignee left examples/claude.yml's gate dropping issues.assigned runs before the workflow could see them).

  • Sideways -- sibling branches of conditionals referencing the event predating the new case (prompt text, format() ternaries, step names). (gha#552 review: the prompt's opening ternary told every assignment run it was "triggered by an @claude mention", which for that path does not exist.)

Tests

check-phi/tests/test_detectors.py is a pytest suite pinning each PHI detector's positive and negative behavior. Run it with python3 -m pytest check-phi/tests/ -q; CI runs it as the phi-tests job in _selftest.yml. There's no broader unit-test harness -- most capabilities are validated end-to-end by _selftest.yml, running against this repo itself or small throwaway fixtures (stable capabilities via @v1, pre-release ones from local source).

check-new-line-breaks/tests/test_check_new_line_breaks.py is a pytest suite covering the sentence-splitter/block-detector functions directly and, via small throwaway git repos (tmp_path fixtures, generated at test time -- nothing committed), the diff-scoping behavior itself: a newly-added multi-sentence line is flagged, a pre-existing one in an untouched line is not, and a diff that can't be computed skips rather than falling back to a whole-tree scan. gha#825's working-tree-aware scope has its own cases: an uncommitted violation is flagged under auto scope with the same-bytes committed case as its control, a clean tree of either kind still reports the examined line/file count, NLB_SCOPE=committed forces the old behavior even when the tree is dirty, and a base branch that advances after the current branch diverged is still not flagged, pinning that the merge-base anchor is unchanged. Run it with python3 -m pytest check-new-line-breaks/tests/ -q; CI runs it as the new-line-breaks-tests job in _selftest.yml, alongside a new-line-breaks job that exercises the real composite (base-ref diff mode) against this repo's own tree, the same "local composite, not yet the @v1-pinned reusable-workflow chain" precedent phi uses above.

Running that check locally before a push used to prove nothing about files you had not committed yet, and it reported them as clean rather than as unchecked -- fixed by gha#825 (NLB_SCOPE, default auto). _added_line_numbers (called from find_violations) used to always diff "$base_ref"..."HEAD", so its population was the commit graph -- a staged file was invisible to it, and an untracked one doubly so. Nothing in the output distinguished "checked, no violations" from "there was nothing to check", which made it misread rather than merely miss:

git add -A
NLB_BASE_REF=origin/main python3 check-new-line-breaks/check-new-line-breaks.py
# before gha#825: No lines missing semantic breaks. (it examined zero added lines)

Now the check widens its own scope when it detects one is needed: with NLB_SCOPE left at its default of auto, a working tree or index carrying changes to a matched file is diffed against the merge base directly (folding staged and unstaged content into one diff), and a clean tree -- CI's own state, always -- keeps the old committed-only diff. Either way, every run now prints how many added lines and files it examined and under which scope, so a run over zero lines reads differently from a run that examined everything and found nothing:

NLB_BASE_REF=origin/main python3 check-new-line-breaks/check-new-line-breaks.py
# -> Examined 3 added line(s) across 2 file(s) (scope: working tree).

A brand-new untracked file is a disclosed gap even under auto/ worktree scope: plain git diff never shows untracked content, so stage it (git add) to be examined -- the same one-command fix committing used to be for everything. NLB_SCOPE=committed forces the old, CI-matching behavior explicitly, for a local run that wants to reproduce exactly what CI will see. The ref itself needs no special care in either scope: the merge base of base_ref and HEAD is resolved once, explicitly, via git merge-base, so NLB_BASE_REF=origin/main means what the new-line-breaks job's own merge-base SHA means, and a base branch that has advanced since the current branch diverged does not widen what gets checked. Measured on gha#544, before this fix: a local run over a staged changelog fragment reported clean, and CI flagged three lines in that same file on the very next push. That was the general verify-the-right-artifact trap in Morrison-Lab/ai-config wearing local clothes: the working tree was an adjacent artifact to the one the instrument actually read, and checking it thoroughly was still checking the wrong thing. The same reasoning still applies to the other diff-scoped checks here -- check-phi takes a base-ref with no working-tree awareness of its own, and check-secrets scans history -- so neither can see an uncommitted change.

A second, independent way that check reports clean over content it never examined: NLB_GLOBS defaults to *.md, unless overridden. The trap above is about which commits the check sees. This one is about which files, and it is wider than it looks.

check-new-line-breaks.py reads globs = os.environ.get("NLB_GLOBS", "*.md").split() or ["*.md"], and the composite exposes that as a globs input. In this repo, _selftest.yml's new-line-breaks jobs pass globs: '*.md *.qmd' and check-diff-scoped.sh defaults NLB_GLOBS to *.md *.qmd (gha#750), so newly-added prose across the 53 .qmd pages under website/ is scanned. Composite descriptions in action.yml and workflows remain outside the check's population.

That is worth stating because this repo's own review checklist (item 7, "Suggest semantic line breaks in prose") names "action descriptions" explicitly, so the written rule and the instrument that would enforce it disagree about scope, and only the prose says so.

It also defeats the obvious negative control. Appending a deliberate two-sentence line to a .qmd and re-running the checker prints No lines missing semantic breaks --- which reads as "the control failed, so the checker is broken" rather than as "that file is not in the population":

# after committing a two-sentence line into a .qmd
NLB_BASE_REF=origin/main python3 check-new-line-breaks/check-new-line-breaks.py
# -> No lines missing semantic breaks.        (the .qmd was never scanned)

NLB_GLOBS='*.md *.qmd' NLB_BASE_REF=origin/main \
  python3 check-new-line-breaks/check-new-line-breaks.py
# -> ::error file=...qmd,line=122::Line packs more than one sentence: ...

So set NLB_GLOBS explicitly to the extensions your diff actually touches before trusting a direct local run of check-new-line-breaks.py, and read a silent run as a question about the population rather than as a pass.

The reproduction above runs against any checkout and needs no history: pick any .qmd, commit a two-sentence line into it, and compare the two invocations. That is the form to trust, because the case that prompted this paragraph is not reproducible, for a reason worth naming. gha#748's diff was entirely .qmd and .yml, and its action.yml description did carry a packed line -- but the pre-push adversarial review caught it, the fix was amended into the first push, and the merged commit is therefore clean under every glob. Following the process this file recommends is what erased the evidence for it. So do not read gha#748 as a citation you can check; the merged diff shows nothing, which is the correct outcome rather than a contradiction. Widening the checked set to .qmd in CI and check-diff-scoped.sh landed in gha#750.

A third way, and the one the two above cannot warn you about: the local run and the CI run are the same script under a DIFFERENT configuration. The two lessons above are about a check whose population is narrower than you assumed. This one is about a check whose population is narrower than CI's, which is a different question and has the opposite tell -- there is no default to misread, because the default is exactly what makes the local run wrong.

_selftest.yml's phi job passes PHI_DETECTORS: ssn,mrn,dob,csv_phi_header,study_id,phone,email. check-phi's own default omits phone and email. So running check-phi/check-phi.py locally with no environment exercises five detectors where CI exercises seven, and its No PHI-like content detected is a true statement about a smaller question than the one CI asks.

Measured on gha#763: a local diff-scoped run reported clean, and CI failed on four phi:email hits in the same diff -- a fixture's synthetic GIT_AUTHOR_EMAIL, which the two missing detectors exist to find. The remedy was one phi-allow comment per line, following the precedent in every other tests/ script here, rather than adding preview/tests/** to the job's PHI_PATHS_IGNORE: these fixtures carry no PHI-shaped content by design, so the directory stays scanned.

The general form is worth stating, because it reaches every capability here whose selftest job overrides an input: read the job's with: block before trusting a local run of the script it calls. check-diff-scoped.sh inherits this for most checks (though it defaults NLB_GLOBS to *.md *.qmd), invoking each check with the caller's environment.

# Not the composite's defaults -- the ones the `phi` job actually passes.
PHI_DETECTORS='ssn,mrn,dob,csv_phi_header,study_id,phone,email' \
  PHI_BASE_REF=origin/main PHI_FAIL=true python3 check-phi/check-phi.py

The suite also covers the gha#336 clause check (a long line carrying a mid-line semicolon, as a proxy for SemBr rule 5), including that it is on by default -- and pins the two defaults that are declared in three places at once. _DEFAULT_CLAUSE_BREAKS/_DEFAULT_CLAUSE_MIN_LENGTH in the script are the single source, but action.yml and .github/workflows/check-new-line-breaks.yml each re-declare them for their own inputs, so a parametrized test reads both YAML files and asserts they agree with the script -- the same gha#303 precedent that pinned generate-altdoc-landing-page's site-root default rather than leaving it to a comment. The first draft of #336 proved why: find_violations() kept a stale False default while classify_line() and main() had moved to True, and only the test caught the drift. That test parses the YAML with a line scan rather than a YAML library, because the new-line-breaks-tests job installs only pytest.

A selftest step that sets no fail: cannot prove the input reached the script, however it is worded. _selftest.yml's new-line-breaks job does call the composite a second time with clause-breaks: 'false', and that is worth having as a real uses: exercise -- but main() now returns 1 by default when violations are found (NLB_FAIL defaults to true), so the step fails on findings rather than proving whether a specific input arrived or was dropped. What actually pins the env var -> main() -> exit code path is a set of pytest cases that set NLB_FAIL=true around a real main() call on a throwaway git repo, asserting exit 1 with the clause check on and exit 0 both with NLB_CLAUSE_BREAKS=false and with the length gate raised past the line. (gha#337 review round 2: the step's original comment, and this paragraph, both claimed the step proved the plumbing; neither could.) Round 3 added the converse caveat, since "cannot prove the input arrived" is not "proves nothing": the step still pins that action.yml parses and that the opt-out code path runs to completion, which is why it stayed rather than being deleted as dead weight. Round 5 narrowed that caveat in turn -- it had also claimed the step pins that the input is declared. Declaration is pinned by the defaults-agreement test instead, which reads each YAML file for the input's default: and fails outright when there is none (gha#337 review round 5).

Markup stripping is where this check's false verdicts come from, in both directions. The clause check keys on a semicolon in the stripped line, so every pattern in strip_inline_markup decides two things at once: whether a ; is prose, and whether the line is long enough to look at. Both of gha#337's round-3 findings were one pattern each. A code-span pattern of `[^`]*` matches the empty span between the two opening backticks of a ``...`` span, so an N-backtick span kept its contents and a ;-separated shell command read as prose -- the exact case the stripping exists to remove. And a bare-URL pattern of https?://\S+ runs to the next whitespace, so a ; immediately after a URL was deleted along with it, silencing a genuine break. The rule that catches both: a pattern must remove the construct and nothing adjacent to it, so backreference a delimiter's opening run rather than matching to the next one, and stop a URL before trailing sentence punctuation.

The sentence regex has two independently-breakable halves, and a fix to one does not touch the other. _SENT_BREAK_RE is [.!?] plus a closing-character class, then whitespace, then a lookahead at what starts the next sentence. Each half fails silently and in the same direction -- a missed boundary means the line reads as one sentence, so the check passes it clean. gha#397 was the closing class omitting * and _, which swallowed every **Claim.** Explanation. line. Measured on 2026-08-03, adding the two characters took the multi-sentence lines detected across Morrison-Lab/ai-config's Markdown from 2837 to 3398, and across this repo's from 719 to 784 --- increases of 19.8% and 9.0% over the old counts. Stated as a share instead, which is the figure that says how much was hidden: the 561 lines ai-config gained are about one in six of what the fixed check finds (561/3398 = 16.5%). Those two denominators are easy to mix up, and only the second answers "how much was the blind spot hiding". Note also that at that measurement the lookahead half still missed a whole class of sentence, so 3398 was itself an undercount and the true hidden share was lower still --- which is an argument about the size of the number, not about whether it is worth fixing. That gap was gha#389: the lookahead required [A-Z"'`*\[] and so missed a sentence opening with a bare lowercase identifier (renv restored the lockfile.), the exact shape our prose writes most. gha#425 closed it with a second branch, _SENT_BREAK_LOWER_RE, that accepts a lowercase follower under two structural guards that share the work rather than one lookbehind carrying all of it. Get the division of labour exactly right, because this block is the map future widenings are read against, and three review rounds on gha#425 corrected earlier guesses here --- each attribution below is what a mutation test (remove a guard, see which case starts splitting) actually shows, not what reads plausibly. The (?<=[a-z][a-z]) lookbehind requires two lowercase letters immediately before the terminal punctuation. It is the guard that refuses a single-letter initial (U.S., the . follows S), a dotted abbreviation (a.m., the . follows .m), a one-letter token (option a.), a digit- or version-ending token (plan9., and v2.1. at a clause end where the . does have a following space), and the ellipsis (wait... foo): the only dot with a following space is the third, and the two characters before it are both dots, so the lookbehind fails there. The branch also has no closing-character class at all, so the terminal [.!?] must be immediately followed by whitespace --- which is what keeps mid-sentence emphasis (**critical.** yet) and a quoted or parenthesized fragment (he said "stop." then) on one line. Separately from both guards, an internal decimal or version dot (the . in 0.9012 or v2.1 between the digits) never reaches a split attempt at all, since it has no following space for the \s+ to match. A closing class was tried on the lowercase branch and removed. The uppercase branch safely carries emphasis and quote closers --- #397 added */_ to its class so a **bold.** sentence end is caught rather than swallowed --- because its uppercase-follower lookahead still refuses a mid-construct lowercase continuation. The lowercase branch's follower is lowercase, so any closer would fire on exactly those mid-construct cases ("stop." then, **critical.** yet) and re-introduce an over-split, which is why it has no closing class at all. So when either branch is widened, ask what the other guards now block before concluding the construction is covered --- and pair the widening with a negative case, since the guards are each other's backstops. The lowercase branch also made the pre-existing abbreviation list reachable for lowercase forms (3 sec. then), and getting that right took three review rounds because the abbreviation protection reaches both branches by default: _ABBREV_RE runs once, up front. The trap each round hit is that an abbreviation edit made for one branch silently regresses the other --- dropping No un-split Item No. Three on the uppercase branch, then registering every lowercase form un-split It took 300 ms. The next ... on it too. The disambiguator is the follower's case: a lowercase unit before a lowercase word (3 sec. then) is mid-sentence, but before an uppercase word (300 ms. The) it is a genuine boundary that must still split. So gha#425 protects the conventional-case abbreviations on both branches (No., Sec., unchanged from before), and protects the lowercase forms in a second pass (_ABBREV_LOWER_RE) applied only after the uppercase branch has run --- so the lowercase forms suppress the lowercase branch without ever reaching the uppercase one. That second list excludes no (a lowercase no. is the word, and should split) and adds min/hr/hrs; it is curated rather than exhaustive, so an unlisted lowercase abbreviation before a lowercase word can still false-split on this check. The whole regex is duplicated in Morrison-Lab/ai-config's scripts/semantic-line-breaks.py, the reformatter this check is the detector half of, so a fix to either is owed to the other (porting gha#425's fix there is tracked in Morrison-Lab/ai-config#1212).

.github/workflows/scripts/tests/run-assemble-news-tests.sh is a shell suite over assemble-news.sh, covering the heading map, category validation, and bullet-marker normalization. CI runs it as the assemble-news job in _selftest.yml.

A repeat assembly merges into the development block's existing sections (gha#810), and Test 29 pins the data-loss direction, not only the tidy one. Before it, every assembly prepended a fresh set of category headings under the top-level heading, so the second monthly run left two ## Bug fixes siblings under one H1 (ucdavis/bcs#862, failed by the consumer's MD024 gate) plus an MD012 double blank at the seam. The script now pre-scans the FIRST top-level section only, splices new bullets under a heading already present there ahead of its existing ones, and emits only the absent headings as a fresh block; an older release block carrying the same heading is not a target (Test 28 diffs it before and after). The seam is handled by a need_blank state flag in the awk rather than by reading the next record with getline: a record read that way never reaches the rules below it, so the first draft silently dropped the bullets of a heading sitting directly under the H1 while still deleting their fragments. Test 29 is that shape, and the assertion is that the bullet exists. The heading-to-bullets map crosses into the awk as pairs of lines rather than delimited records, because the map constrains a heading only to one line and a tab-separated manifest lost a tab-bearing heading's fragment the same silent way (Test 30, from the review); "blank" means whitespace-only at both seams, as markdownlint reads it (Test 31); an empty fragment adds no stray blank (Test 32); a merged section stays one tight list, the held-back blank being dropped before a bullet and printed before anything else (Test 27's adjacency assertion); a whitespace-only line after the target heading is one blank (Test 33); a column-0 # followed by a space inside a fenced code block is not a heading in either pass (Test 34); a fence line arriving at a splice seam still drains the pending blank, since the fence short-circuit runs after the seam rules (Test 35); and a tight heading followed directly by a fence or paragraph gets a synthesized blank between the new bullets and that content (Test 36). Test 27 also diffs the older release block before and after, which is the only assertion that sees the awk-side in_first guard and the delete after a merge together. One trap ... EXIT, installed right after mktemp -d, removes every temp path on every exit.

Its bullet-style cases are where the subtle failures live, and they fail silently in one direction. resolve_bullet_style() picks the marker markdownlint's MD004 will hold the whole assembled file to, so a wrong answer flips that requirement for every bullet already in the file rather than erroring. The candidate scan matches "a marker followed by a space", which is also what a spaced CommonMark thematic break looks like, so the suite's job is to pin the boundary between the two. Tests 15 and 18 pin the break cases (- - -, * * *), and 16, 17, 19, and 20 pin the list cases that merely resemble one: an empty list item (a * and the space after it), a spaced run of + (never a break character), two markers, and a run mixing marker characters. Test 16's empty item must use * rather than -: the no-bullet-found fallback is itself -, so a - empty item makes the buggy and fixed answers coincide and the regression goes undetected.

Prose about this check trips lint-markdown, because the construct it describes is the construct MD038 forbids. A bullet marker followed by a trailing space cannot be written inside a code span: MD038/no-space-in-code rejects it, so the sentence explaining the rule fails the linter the rule is about. It bit twice on gha#742 -- once in a changelog fragment, caught by CI, and again in this very section, caught locally. The fix is to render the example so it cannot match, naming the marker in a span and the space in prose, rather than rewording around the meaning or reaching for an inline lint exemption. This is the self-implicating-example problem Morrison-Lab/ai-config's examples-are-scanned.md describes, in the one shape this repo's own docs keep meeting it. Run it over a Markdown file you just wrote, rather than waiting for the job -- and pin the version to the one lint-markdown/package.json declares, rather than letting npx resolve whatever is current, since an unpinned run is a different tool from the one CI runs:

ver=$(node -p "require('./lint-markdown/package.json').dependencies['markdownlint-cli2']")
npx "markdownlint-cli2@$ver" --config lint-markdown/.markdownlint.default.jsonc '*.md'

Deriving the version rather than writing it here keeps one declaration, per the gha#303 precedent. The difference is observable rather than theoretical: measured on gha#744, an unpinned invocation reported 0 issues in 0 files where the pinned 0.23.0 reported 0 error(s) -- the same verdict through a different output contract, and a rule-behaviour difference would be exactly as quiet.

Pin which script a pre/post-fix measurement actually loaded, because the obvious baseline is wrong exactly when the fix is already committed. git show HEAD~1:<path> reads the previous commit, not the pre-fix state, so the moment a second commit lands on the branch that baseline silently contains the fix. The measurement then reports the fixed behaviour under both labels, which looks like "the fix changes nothing" rather than like a loading error. Measured on gha#742: an MD004 comparison read identical numbers for both arms until a grep -c sanity assertion on the loaded script showed the baseline was the fixed one. Diff against origin/<default-branch> for a pre-fix baseline, and assert the distinguishing symbol is absent from it before trusting any number. Five mutations were confirmed to turn a named case red rather than assumed to -- relaxing the length gate to two, accepting a run mixing - and *, ignoring the marker argument to hardcode -, treating + as a break character, and dropping the length check that separates an empty item from a break.

A fixture that discriminates the code path can still demonstrate none of the harm, and Test 16's first draft did exactly that. It placed a later * Existing bullet. beside the empty item, which made the file MD004-inconsistent before assembly -- so the error count was 1 under both the pre-fix and post-fix answers, while the test's own comment claimed it showed an MD004 flip. The fixture now carries the empty item as its only bullet, measured at 0 errors unassembled, 1 pre-fix, and 0 post-fix. Read that as the general shape: when a test's stated rationale is about a downstream tool's verdict, measure that verdict under both answers rather than asserting the resolution alone (gha#741 review).

lint-markdown/check_list_item_splices.mjs (tested by node lint-markdown/tests/test_list_item_splices.mjs) flags list-item merge splices: a list item spliced directly onto a previous item's continuation line with no intervening blank line (gha#324). CI runs it as part of the lint-markdown composite action and job in _selftest.yml.

Its commonest trigger is an ordinary wrapped bullet list. The condition flags a list item whose immediately-preceding line is non-blank and is not itself a list item, heading, blockquote, table row, or thematic break. The continuation line of a previous wrapped bullet satisfies that non-blank preceding-line condition. So a wrapped bullet followed immediately by the next bullet --- without a blank line between them --- is flagged as a splice. The repo's house style requires multi-line wrapped list items to be separated from adjacent items by an intervening blank line. The check is diff-scoped via LIST_ITEM_SPLICE_BASE_REF on PRs (set to all for a full scan, or skipped with a warning when empty). Negative controls in test_list_item_splices.mjs verify that wrapped bullets separated by blank lines pass cleanly, and that fenced code blocks (```, ~~~), table rows, headings, blockquotes, and thematic breaks are exempt from triggering splice errors.

lint-markdown/check_table_splits.mjs (tested by node lint-markdown/tests/test_table_splits.mjs) flags split GFM tables (gha#558). A GFM table whose body rows are split across a blank line without repeating the delimiter row (|---|---|) or a table with a blank line between the header row and delimiter row loses its tabular rendering in GitHub Flavored Markdown. The check detects orphaned table rows and split table headers across tracked Markdown files, with negative controls asserting that standalone tables with delimiter rows, single pipes in prose/code spans, and fenced code blocks pass cleanly.

lint-markdown/tests/test_pathspec.mjs tests lint-markdown/_pathspec.mjs (and its shared pattern mirrored across lint-qmd and lint-yaml), asserting correct ignore compilation, list splitting, and cross-platform path matching for both POSIX and Windows backslash paths.

.github/workflows/scripts/tests/run-check-diff-scoped-tests.sh covers check-diff-scoped.sh, the contributor-facing wrapper that runs this repo's diff-scoped checks over committed content (gha#740). CI runs it as the diff-scoped-guard job in _selftest.yml.

It exists because three checks here share one silent failure, and the warning prose about it was not enough. check-new-line-breaks, check-phi and check-typos all read the commit graph, so none of them sees a staged or untracked file -- and each then prints no findings over content it never examined. The gha#544 paragraph earlier in this section -- "Running that check locally before a push proves nothing about files you have not committed yet" -- records that trap and its measured recurrence. The recurrence after that paragraph was written is what argued for a mechanical guard over a third restatement.

A second silent path turned up while building it, which the issue did not anticipate: the three checks disagree about an empty base ref. check-new-line-breaks and check-typos skip entirely, while check-phi scans the whole tree. So a misspelled ref does not error anywhere -- it produces a different wrong answer per check. The wrapper refuses rather than passing one through.

"Could not run" is a third outcome, and collapsing it either way is the same bug. check-typos needs an installed binary, so calling its absence a failure cries wolf and calling it a pass is the exact lie the wrapper exists to prevent. It gets exit status 3, distinct from 0 (all runnable checks clean), 1 (a finding) and 2 (refused). A real finding outranks an incomplete run, since the finding is actionable now.

24 assertions across 15 cases. Nine mutations are confirmed to turn a named case red: dropping the dirty guard, narrowing the status scan to --untracked-files=no, dropping the merge-base gate, letting the tool-unavailable arm read as clean, letting the script-absent arm read as clean, letting an incomplete run outrank a finding, no longer honouring .gitignore, dropping the no-base-ref block, and probing the typos check for its binary without its interpreter. One deliberately survives, and it is information rather than a gap: removing the base-ref resolvability check changes no verdict, since git merge-base also rejects a nonexistent ref, so that check earns its place by naming the problem rather than by catching it -- which a message assertion pins instead.

The "examined nothing" heuristic is guarded twice, so no SINGLE mutation of it turns a case red, and reporting either gate as confirmed would be false. Removing the status gate leaves the anchored pattern, which a violation line (printed as ::error) cannot match. Removing the anchor leaves the status gate, which a finding's non-zero exit already fails. Case 12 is killed by removing both together, which is the defect as it actually shipped. Read a survivor here as "the other gate still holds", not as missing coverage -- and note that this is the opposite reading from the mis-aimed mutations recorded above, so the two are told apart by whether a second mechanism covers the same outcome.

The .gitignore case is the one keep-if-trimmed assertion pointing the opposite way from the rest: refusing on ignored build output would make the wrapper unusable, and an unusable guard gets bypassed, which is worse than no guard.

A fix for a review finding is the likeliest place for the next one, and gha#745 produced two in a row. Closing B1 -- the guard blessing a check that examined nothing -- meant adding a read of each check's own "Skipping" admission. That read scanned the check's whole output before testing its exit status, and these checks print up to 77 characters of the offending line, so a genuine violation whose text quoted the phrase was reclassified as "examined nothing" and downgraded from a blocking exit 1 to a non-blocking 3. The fix for the unearned-clean-verdict defect produced an unearned clean verdict. Closing that in turn introduced a set -e regression, since out=$(cmd) is a simple assignment and a failing check terminated the script there.

Two things make this worth a rule rather than an anecdote. A fix is written under the belief that its region is now the best-understood part of the diff, which is exactly the belief that stops it being re-read adversarially. And it arrives late, when the reviewer has already approved everything around it, so it gets the least scrutiny of anything in the change.

  • Do: re-run the full mutation sweep after a fix, not only the case the finding named.

  • Do: ask what NEW failure the fix's own mechanism can produce, in the same terms as the finding it closes.

  • Don't: treat a fix as smaller than the code it replaces -- both of these were a few lines.

The self-implicating-example hazard has a RUNTIME form, not just a lint-time one. The two references above are about a checker scanning a file, where the remedy is to render the example so it cannot match. Here a running guard matched on scanning the whole tree instead, and on Skipping the plus the space after it -- strings this repo's own prose quotes, in that script's header and in this file. Writing this very paragraph tripped MD038 on that trailing space, which is the lint-time form biting inside the entry about the runtime form. So a PR editing the documentation could downgrade its own findings, and the mechanism is the corpus describing its own detector's vocabulary. The remedy is the same shape but applies to matching rather than to writing: anchor on the emitting form (^::warning::), and gate on a signal the prose cannot forge -- here the exit status.

  • Do: ask, of any heuristic keyed on a string, whether this corpus documents that string.

  • Don't: rely on a free-text match when the text is something the repo writes about.

An earlier revision of this paragraph claimed six mutations were confirmed when one of them was not, and the gap is worth keeping rather than quietly correcting. The tool-unavailable arm and the script-absent arm both end in "this check did not run", so mutating the second felt like covering both; it does not, and the suite stayed fully green while the wrapper returned 0 over a check that never ran. That is this file's own mis-aimed-mutation lesson, committed against the paragraph that states it (gha#745 review).

The harness absolutizes SCRIPT deliberately. Every case cds into a throwaway fixture repo before invoking the script, so a relative override resolves against the fixture instead and fails every case at once -- which reads as a broken implementation rather than as a harness pointed at nothing. Measured while landing gha#740 against an implementation that was fine. An earlier revision of this paragraph put a specific count on that measurement and explained it by saying the cases refusing before invoking the script still passed; no case does that, so the explanation was invented and the count was stale (gha#745 review).

.githooks/pre-push runs the wrapper before a push, and nothing installs it: git only honours it once a contributor opts in with git config core.hooksPath .githooks. That is deliberate, since whether this repo wants committed hooks at all is a policy question rather than a technical one. The hook warns and pushes on exit 3 rather than blocking, because a hook that blocks on the contributor's toolchain rather than on their code gets disabled, and then guards nothing.

check-secrets/tests/test-build-config.sh is a shell suite over build-gitleaks-config.sh, the script that turns the paths-ignore, allowlist-file, and config inputs into the gitleaks TOML the scan runs under. That generator is the piece worth testing because its failure mode is one-directional: a bug there widens an allowlist and quietly suppresses real findings, so the check goes green rather than breaking. Run it with bash check-secrets/tests/test-build-config.sh; CI runs it as the secrets-tests job in _selftest.yml, alongside a secrets job that exercises the real composite against this repo's own full history -- the same "local composite, not yet the @v2-pinned reusable-workflow chain" precedent phi and new-line-breaks use above.

Two of its cases are the ones to keep if the suite is ever trimmed. A pattern carrying ''' is refused rather than written, since a TOML literal string has no escapes and one would truncate the array, changing which findings are suppressed; that case was confirmed to fail when the guard is stubbed out. And a named-but-missing config/allowlist-file is an error rather than a silent fall back to the default ruleset -- a typo'd path must not read as "no allowlist".

check-secrets/tests/test-scan-secrets.sh covers the other half, and exists because the secrets job structurally cannot. That job runs against this repo's own history, which is clean by design and by measurement, so it always takes the zero-findings early return -- meaning the fail-keyed branches, the annotation loop, the step summary, and the exit code never execute in CI. Round 1 of gha#385's review found a fail-open fail bug in exactly that region while both selftest jobs stayed green, which is the same shape as the check-new-line-breaks lesson two paragraphs up: a step that cannot fail proves nothing about the code past the point it returns.

The fix is a stub gitleaks on GITLEAKS_BIN_DIR that writes a canned report and exits 0, so the real branching runs offline with no download and no network. Reintroducing either of the two bugs those tests were written for -- the fail-open comparison, or truncating the fingerprint to a 12-character SHA -- turns two assertions red each; both were confirmed by mutation rather than assumed.

Its fixtures carry no credential-shaped strings, deliberately. The secrets job scans this repo's own history, so a realistic dummy token in a committed fixture would trip it forever after -- the committed-fixture trap the paragraph below describes, in the one form no runtime generation can undo, since the commit that added it stays in history. The canned report the stub writes carries only RuleID, File, StartLine, Commit, and Fingerprint, never Match or Secret, for the same reason.

check-junk-files/tests/test-check-junk-files.sh is a shell suite driving check-junk-files.sh against throwaway git repos built in $TMPDIR -- nothing committed, for the same reason the test-coverage fixture is generated: a committed .DS_Store would be swept into this repo's own junk-files selftest job forever after. Every fixture is force-added (git add -Af), because a developer running the suite on a vaccinated machine has a global gitignore that silently skips the very files under test. CI runs it as the junk-files-tests job in _selftest.yml, alongside a junk-files job that exercises the real composite: once against this repo's clean tree, once against a .DS_Store staged into the index (the scan reads the index, so no commit is needed) with continue-on-error plus an outcome assertion, and once with that file exempted through paths-ignore -- the two input paths a wiring typo would silently turn into "checks nothing" and "exempts nothing". The four cases to keep if the suite is ever trimmed are the negative ones, because each pins a decision that is silent when reversed: a force-added file listed in the repo's own .gitignore is not reported (passing --exclude-standard reports it), paths-ignore: 'vendor/' really exempts the directory (implementing it as gitignore ! lines exempts nothing), an empty pattern set is an error rather than a green check that examined nothing, and a filename merely containing .DS_Store is not a match. All five mutations were confirmed to turn the suite red rather than assumed to, the sixth being the defaults-agreement check that action.yml and .github/workflows/check-junk-files.yml declare the same patterns string -- the gha#303 precedent, and here a drift would hand a consumer of the reusable workflow a different pattern set from a consumer of the composite with nothing red.

check-typos/tests/test_check_typos.py is a pytest suite driving check-typos.py against throwaway git repos and a stub typos binary that writes canned JSONL -- no download, the same remedy check-secrets records for its scan script. The cases to keep if the suite is ever trimmed are the negative ones: empty / unresolvable base-ref skips rather than scanning the whole tree (a whole-tree fallback would reflag every known misspelling the corpus already carries), a pre-existing typo on an untouched line is not flagged, a filename typo on a content-only edit of an already-named file is not flagged (the finding is produced and then dropped, not skipped), a rename into a misspelled name is flagged, fail: yes still blocks, a named-but-missing config is an error rather than a silent fall back to defaults, a stub exit other than 0 or 2 is a tool error even when fail is false, an added line starting ++ plus a space is not parsed as a diff file header, and a checksum mismatch refuses to install the binary. CI runs it as the typos-tests job in _selftest.yml, alongside a typos job that exercises the real composite (real installer, real typos 1.49.0): a no-base-ref call against this repo's own tree (which carried a handful of pre-existing hits as of 2026-08-26, so a broken skip would fail it), a fixture whose only typo is pre-existing (diff-scoped pass, base-ref: all fail), and a newly-added .qmd typo that spellcheck.yml cannot see, plus that file exempted through paths-ignore. The fixture checkout is generated at runtime (check-typos/tests/make-fixture.sh). The misspelling is also used as fixture payload in the pytest sources, so a later whole-tree dogfood of this repo should paths-ignore check-typos/tests/.

check-code-similarity/tests/test_check_code_similarity.py is a pytest suite driving check_code_similarity.py against a stub java that writes a canned JPlag results CSV --- no 80 MB jar and no JDK, the same remedy check-secrets records for its scan script. The stub parses -r out of its own argv, so the result-path plumbing is exercised rather than assumed.

check-one-function-per-file/tests/test_check_one_function_per_file.py is a pytest suite testing the top-level function parsers across Python (AST), R (brace/paren depth, backtick names, lambdas), Shell (parameter expansion protection), JavaScript/TypeScript (generics, block comment stripping with line preservation), and Julia (multiple dispatch deduplication), along with opt-out header directives and defaults-agreement. Run it with python3 -m pytest check-one-function-per-file/tests/ -v.

The refusal cases are the ones to keep if the suite is ever trimmed, and they all fail in one direction. A similarity check that could not run prints no findings, which is indistinguishable from a check that ran and found none --- so a missing corpus, an empty corpus, a corpus of loose files rather than submission directories, a JPlag crash, a missing results.csv, an empty one, a missing column, an unparseable similarity, and a digest mismatch are each an error rather than a quiet pass. Two of them are less obvious and were found by review rather than by writing the script: two roots sharing a directory name are ambiguous, because JPlag labels every submission by that name, and a comparison that ran but produced no pair involving the submission under review evaluated nothing that mattered. A clean run prints how many pairs it examined, which is what makes a real pass distinguishable from a vacuous one. Nine mutations were confirmed to turn a named case red rather than assumed to, including one that is easy to get backwards: a digest mismatch must delete a jar from our cache and must not delete one the caller passed with --jar, which is a file the action does not own.

CI runs it as the code-similarity job in _selftest.yml, unit tests first, then three real uses: ./check-code-similarity calls against a generated fixture --- a renamed-identifier copy that must be flagged, an unrelated corpus that must not be, and an empty-but-present corpus that must error. All three directions are asserted, because a job running only the first would pass just as well if the action flagged everything. The fixture is generated at runtime (check-code-similarity/tests/make-fixture.sh), per the committed-fixture rule above --- a committed R fixture would be swept into the bib, phi, and typos jobs' own scans.

JPlag's R grammar does not parse R's native pipe |>. Measured on gha#296, detection degraded rather than failed: a copy with every identifier renamed still scored 1.0, an unrelated submission 0.0, and a copy whose pipe style had been rewritten from |> to %>% also 1.0. The parse errors are surfaced as a counted warning rather than swallowed, because dropped tokens make the reported similarity a lower bound.

check-extra/tests/test-check-extra.sh is a shell+R suite over check-extra.R. The bash half pins that action.yml and .github/workflows/check-extra.yml declare the same check-readme-freshness default -- the gha#303 precedent, because a drift here would hand a consumer of the reusable workflow a different freshness gate from a consumer of the composite with nothing red. The R half pins the decisions that are silent when reversed: an unknown check name is an error, a missing tests/testthat or vignettes/ inside the combined warnings job is a skip rather than a failure, a missing README.Rmd or tests/testthat on the dedicated README / random-order jobs is a failure, a dirty or untracked README.md fails freshness, parse_flag is fail-closed, and a warning() in a test file fails the warnings sweep (when pkgload and testthat are installed). CI runs it as a step in the check-extra job in _selftest.yml, after three real uses: ./check-extra calls against a generated fixture (one per check) and before a fourth call against a warning-in-example fixture that must fail. The fixture is generated at runtime (check-extra/tests/make-fixture.sh).

lint-changed-files/tests/test-lint-r-scope.R is an R suite over lint-r-scope.R: scope validation, PR-file filtering (removed files dropped; walking 101 files only proves the walker does not drop list elements), a grep of lint-changed-files.R for .limit = Inf (that is the pagination pin -- pr_changed_paths never calls gh::gh), dotfile exclusions (an unchanged .lintr.R is excluded; .git/ is not listed), and -- when lintr is installed from CRAN -- the changed-files exclusion pattern plus the DESCRIPTION vs no-DESCRIPTION split (lint_package vs lint_dir). Those lintr cases setwd() into the fixture and pass GitHub-shaped repo-relative filenames (dirty.R, pkg/R/bad.R), because rel_to_path() prefix-matches against path and an absolute tempfile() path would make every changed-files case return empty lints before lintr runs. Run it with Rscript lint-changed-files/tests/test-lint-r-scope.R; CI runs it as the lint-changed-files-tests job in _selftest.yml, alongside a lint-changed-files job that exercises the real composite (uses: ./lint-changed-files, not @v2) against a generated project fixture with no DESCRIPTION -- the setup path lint-changed-lines does not cover. That composite job uses scope: project only: untracked fixtures are invisible to the PR-files API, so changed-files gh::gh wiring is not an end-to-end selftest (the same gap lint-changed-lines has). The two fixtures differ in exactly one thing (<- vs = under a .lintr.R that enables only assignment_linter), so a default-config change on CRAN cannot turn the clean variant red. The fixture is generated at runtime (lint-changed-files/tests/make-fixture.sh).

Generate selftest fixtures at runtime; don't commit them. A fixture committed under a composite's tests/ dir (e.g. a minimal R package for test-coverage) gets swept into OTHER selftest jobs' repo-wide scans: the bib job's dependency resolution tries to treat it as a real package, and the phi job's PHI scanner flags any synthetic identifier in it (a fake maintainer email, etc.). Generate the fixture in a small script (test-coverage/tests/make-fixture.sh is the pattern) that the coverage selftest job runs before invoking the composite, instead of committing R package source files (gha#148).

.github/workflows/scripts/tests/run-r-cmd-check-workflow-tests.py parses r-cmd-check.yml and examples/r-cmd-check.yml and asserts the contracts that a live R CMD check in this repo cannot: cache: false on the hard job, that job gated to pull_request, _R_CHECK_FORCE_SUGGESTS_ hard-coded false there, no linux-container on that job, error-on defaulting to '"note"' on the full matrix and forwarded to that job's check-r-package step, the hard job omitting error-on so r-lib's '"warning"' default applies, both jobs setting _R_CHECK_CRAN_INCOMING_ from cran-incoming-remote (r-lib forces it false when unset), the full job's Quarto skip keyed on verse rather than every ubuntu cell, the issue-required inputs present, and the example stub's concurrency group keyed on github.ref rather than github.head_ref alone. This repo is not an R package, so _selftest.yml does not run the reusable workflow end-to-end. A uses: job cannot materialize an R-package fixture the called workflow's own checkout would see. Run it with python3 .github/workflows/scripts/tests/run-r-cmd-check-workflow-tests.py --self-test; CI runs it as the r-cmd-check-tests job. Nine mutations are confirmed to turn it red: flipping cache: false, dropping the pull_request gate, restoring upstream's github.head_ref-only concurrency group, forwarding inputs.error-on on the hard job, dropping error-on from the full matrix Check step, dropping _R_CHECK_CRAN_INCOMING_, dropping _R_CHECK_FORCE_SUGGESTS_ from the full job, restoring rpt's skip-Quarto-on-every-ubuntu condition, and a verse-only skip that is not limited to ubuntu-latest.

.github/workflows/scripts/tests/run-version-check-workflow-tests.py pins version-check.yml's live-label exemption the same way, and for the same reason: that workflow needs a live pull_request event and a labelled PR, so _selftest.yml cannot exercise it end to end. CI runs it as a step in the dev-version job.

What it pins is the set of changes that are silent when reversed, which is the whole hazard here. Reading the label from github.event.pull_request.labels returns the labels frozen into the triggering event's payload, so a label applied afterwards is invisible and an approved or re-run job reuses the same stale payload --- the exemption never fires, and a check that never exempts is indistinguishable from one whose exemption nobody asked for (gha#722). So the assertions are: the payload path stays gone, pull-requests: read is granted, the read is paginated, the read is a plain assignment and not inside an if/while condition (where set -e is inert by design), the step runs under set -euo pipefail, both sides are case-folded, no hardcoded label spelling competes with the configurable input, and the bump-branch bypass survives. Nine mutations are confirmed to turn a named case red.

pull-requests: read, not issues: read, and the distinction is measured rather than reasoned. GitHub authorizes a label read on an issue object that is a pull request against the pull-requests permission: issues: read alone returned 403 Resource not accessible by integration on check-news's first consumer run (gha#724, fixed in gha#725, which grants both there defensively). version-check already granted pull-requests: read, so gha#726 added no scope at all --- the tracking issue's own prescription of issues: read predates that measurement.

The suite strips whole-line comments before scanning. The passage explaining why the stale-payload path is wrong has to name that path, so a scan over the raw file flags the comment documenting the rule --- the self-implicating-example problem Morrison-Lab/ai-config's examples-are-scanned.md describes. Teaching the checker about comment regions is the fix; rewording the prose so it cannot say what it means is not. Whole lines only, so a live read cannot be hidden from the scan by appending a # to its line.

The same coverage job exercises the new composite inputs (coverage-type, comment-donttest, comment-dontrun, min-coverage, upload-coverage, failure-artifact-name) rather than only the happy path. It cannot exercise the reusable workflow's examples-coverage input: that layer calls Morrison-Lab/gha/test-coverage@v2, which does not resolve the new keys until @v2 slides (the same bootstrapping gap dependabot-review and failure-issue record). The fixture exports add() (unit-tested) plus from_donttest() and from_dontrun() (covered only by those Rd example wrappers). A min-coverage: '100' call on the default tests type must fail; otherwise the threshold is a no-op. A coverage-type: 'examples,vignettes' call with both comment-donttest/comment-dontrun set false and the same 100% bar must pass. That is the only combination that actually executes those blocks. There is no vignettes/ directory in the fixture: type=examples,vignettes here proves the comma-split and the examples comment flags, not vignette execution. The fail-path assertion greps ${RUNNER_TEMP}/min-coverage-failure.txt for below the required, so a composite crash that merely reddens outcome does not count as pinning the threshold. Parser-level cases (empty min-coverage disables the threshold, comma-separated types, invalid values) live in test-coverage/tests/test-coverage-helpers.R and run after the first composite call, once setup-r has put Rscript on PATH.

.github/workflows/scripts/check-review-execution.sh holds claude-code-review.yml's fail-check guard logic (stub/placeholder-review detection, quota-exhaustion skip) as a standalone script, so it can run offline against canned execution-output fixtures instead of requiring a live Claude API call. .github/workflows/scripts/tests/run-fixture-tests.sh feeds each fixture under scripts/tests/fixtures/ through the script and asserts the expected pass/fail/skip/fail-stub outcome (fail-stub -- gha#185 -- is a fail fixture that must ALSO write stub_review=true, the signature claude-code-review.yml retries on); CI runs it as the review-fail-check job in _selftest.yml. These fixtures ARE committed rather than generated at runtime -- unlike the R-package/PHI-shaped fixtures the rule above warns about, they're plain JSON execution-output data with no content that would trip the bib or phi jobs' repo-wide scans (gha#174).

Quota exhaustion arrives in two shapes, and the second one has real cost behind it. The original quota-exhausted.json is a request rejected at the door -- total_cost_usd: 0, num_turns: 1 with an error_* subtype (an is_error: true run with subtype: "success" is an execution failure rather than quota exhaustion, gha#561) -- and the guard keys the graceful skip on that shape. An account can also run out mid-review, which the pair cannot see: gha#520 was observed at 13 turns and $4.10, with api_error_status: 429 and is_error: true alongside subtype: "success". Before the fix that fell through to the hard is_error exit and reddened claude-review over an account condition the PR's author could not act on, with no comment on the thread saying so. Two things constrain any change here. The detection keys on the structured api_error_status field and never on the result message's prose, for the reason classify-gemini-failure.sh's own header gives (gha#380 finding 1) -- that message is ordinary text, and matching prose against a transcript is how a genuine failure gets misclassified as a graceful skip. And the check sits after the gha#391 verdict check rather than beside the zero-cost one, so a run that stated its verdict and only then hit the limit still passes; quota-exhausted-midrun-with-verdict.json is the fixture that pins that ordering, and it passes with or without the fix by design -- the fixture that actually fails when the fix is removed is quota-exhausted-midrun.json, confirmed by mutation rather than assumed.

A reviewer that redrafts its final message defeats the gha#710 span rule, and the heading form is what tells the two shapes apart. gha#710 widened the posted text to the whole span from the first verdict-bearing block to the last, because a review split across blocks was being posted as its tail alone. gha#805 is that rule's own failure: a reviewer re-ran an instrument between drafts and produced three complete reviews in a row, each with its own ### Verdict heading and structured-review-data block, and the span rule concatenated all three (Morrison-Lab/ai-config#2966, run 33594599768). A complete draft carries a verdict heading; the gha#710 follow-up tail writes a line-start Verdict: line, never a heading (a mid-sentence "my verdict stands unchanged" matches neither regex, since both anchor at line start). So when more than one block carries an authored heading, the span starts at the last such block and still runs to the last verdict-bearing block, which drops the superseded drafts and keeps the tail; one heading leaves gha#710's behaviour untouched. Authored means outside a fence, outside a blockquote, and outside an indented code block. This corpus quotes the literal heading constantly, and a review of this very script does too, so a later block that only shows the heading shape in a code fence or blockquotes the previous verdict must not read as a new draft; with the bare regex it did, and the span then started at the quotation and dropped the entire real review, which is the failure gha#710 exists to prevent (gha#808 review). Both the jq detector and the bash invariant skip fenced and blockquoted lines. A fence closes only on the same character, at least as long as the opener, with nothing but whitespace after it (CommonMark, as strip-non-invoking-markup.sh does); a first draft closed on any fence line, so a backtick fence holding a tilde line leaked its heading, and a second closed on a run followed by text. Both accept at most three leading spaces before the # or before a fence run (spaces only: a tab is four columns, so a tab-led backtick line is indented code rather than a fence), so a line at four columns or a tab is never a heading, whether indented code or a lazy paragraph continuation; round 3 of the same review reproduced the drop with indentation, and no state machine is needed for it. The awk that counts headings carries no interval expression, per this file's mawk rule, spells the jq's word boundary as a trailing class, so a plural "Verdicts" heading counts in neither, and mirrors the jq's one-to-six hash limit by run length and its space-after-hashes requirement, so ####### Verdict and ###Verdict count in neither. An unclosed fence runs to the end of its block, so a redraft whose own code sample is never closed hides its own heading and falls back to the gha#710 span rule; that is malformed input rendered as GitHub renders it, and ignoring unclosed fences would re-admit the quoted-heading drop. verdict-redrafted-thrice.json pins the rule with a must-contain on the third draft, a second must-contain on the tail after it, and a must-not-contain on the first draft; verdict-then-quoted-heading.json pins that a quoted heading is not a draft, verdict-then-mismatched-fence.json that a tilde line does not close a backtick fence, and verdict-then-trailing-text-closer.json that a run followed by text does not close one either, and verdict-then-indented-heading.json that an indented heading is not a draft, verdict-redraft-after-tab-fence.json that a tab-led backtick line opens no fence, and verdict-then-tab-inside-fence.json that one inside a real fence closes none (there a tab-admitting extractor un-fences the quoted heading and drops the real review, the same silent-drop class as the fence and blockquote cases, and a tab-admitting awk counts two headings in the correct span, so the one fixture pins both halves); and assert_pass holds every posted review to at most one authored heading, so a future concatenation fails on whichever fixture produces it. Nine mutations turn a named case red: disabling the multi-heading branch, narrowing the span end to the last heading block, dropping the fence-and-blockquote exclusion, closing a fence on any delimiter, closing on a run followed by text, widening the heading indent back to any whitespace, admitting a tab into the fence indentation, lifting the six-hash limit, and dropping the awk's trailing word-boundary class. That count is a shape check on our own extraction, not a verdict parse: it never reads which verdict was stated.

A fast path inserted before an existing sanitizer inherits none of that sanitizer's protections, and classify-review-verdict.sh's own gha#710/gha#805/gha#808 quoted-verdict guard is exactly what the new path bypassed. gha#845 (Lacaedemon/sparta#1547) reported verdict=unrecognized on a review whose review-data payload already carried "verdict": "CLEAN", unrecognized because the prose was the triage-exemption template ("No action -- automated, trivial PR that does not need code review"). gha#846's first cut read that payload before the prose scan but from the raw, unstripped body -- before strip_machine_payloads had run -- so a payload quoted inside a blockquote or a fenced example overrode the live verdict, the identical quoted-verdict bypass the redraft rule above already exists to prevent, reintroduced one layer up. The fix shares one fence/blockquote tracker (_iter_fence_and_quote_state, via _open_fence/_fence_closes) between the payload scan and strip_machine_payloads, so the two cannot disagree about what is fenced or quoted.

  • Do: route a new fast path over the same untrusted text through the existing sanitizer's own state, not a fresh copy of its logic.

  • Don't: add a fast path ahead of an existing sanitizer and assume the sanitizer still protects it -- it has already returned by the time the sanitizer runs.

Two more local adversarial rounds found three further defects in that fast path: a non-greedy (.*?)\s*--> regex truncated on a --> inside a JSON string and fell back silently (fixed with raw_decode(), which parses one JSON value from an offset regardless of string contents); a bare "no action" on a later line beat an earlier rejection under the scan's last-match-wins rule (fixed by anchoring the phrase to the first non-empty verdict line and voiding it when that line still carries open-work or rejection vocabulary); and a CLEAN payload with non-empty findings was trusted outright (fixed by falling through to the prose scan instead; NOT_CLEAN still trusts regardless of findings).

Seven review rounds found these: three local adversarial rounds (round one authored the fast path with its own tests green; round two found the blockquote/fence bypass in the payload scan, the regex and findings gaps, and the first unanchored "no action" keyword; round three tightened that anchor and found the indented-fence gap), then four Copilot rounds. Copilot's first round found a real defect the local rounds had missed: strip_machine_payloads itself still kept blockquoted lines, so a quoted heading or keyword could win the heading and prose scans (fixed in 5793322, which is where the shared tracker reached both scans); its other rounds raised comment and doc accuracy findings. Tests went 138 -> 160. Round one's own tests all passed, and it would have shipped the quoted-payload bypass unreviewed -- the argument for the adversarial round over a green suite alone.

permission_denials_count can be absent from the real execution file even though claude-code-action prints it to the job log, because the log line is a display value the action computes, not a field it always writes to disk. gha#531 was observed on a real run: the printed log read "permission_denials_count": 5, but the saved execution file's result object carried no such key at all -- only permission_denials, an array of 5 denial-detail objects. Reading .permission_denials_count // "MISSING" alone therefore read MISSING and defaulted to the 999999 sentinel, which wrongly excluded the run from the gha#185 stub-retry even though the real count (5) sat right at the threshold. The fix falls back to permission_denials | length when the scalar is absent/null but the array is present, tried only after the scalar so every scalar-only fixture is unaffected; permission-denials-array-only-low-count.json and permission-denials-array-only-high-count.json pin both directions (within and above max_denials), confirmed to fail pre-fix by running them against the pre-#531 script.

That same array is what the log was throwing away, and the count alone reads as a different failure than it is. gha#540: a permission_denials_count=24 warning names no tool, so two readers of one log reached a wrong cause for the same failure (Morrison-Lab/ai-config#1773), and a 12-denial run on Morrison-Lab/wai#83 left a hypothesis -- sub-agent spawns, or file reads? -- that one line of log would have settled. A red check with only a number on it reads as "the reviewer gave up" rather than as a permissions gap with a specific fix. So check-review-execution.sh now summarizes permission_denials beside the count (Denied tools: Taskx6 Bashx3 WebFetchx2) with one argument sample per tool, and repeats both in the over-threshold annotation, which is the line a triager reads without opening the job log. Four things constrain any change to it. The summary is emitted where the count is computed, not inside the over-threshold branch the issue proposed: the low-count case is retried and can stub a second time, so it needs the same diagnostic. Summary and sample come from one jq pass emitting two lines, because they share the grouping and the ordering -- computing them separately meant two traversals and two copies of group_by | sort_by that could drift into disagreeing about which tool leads, which is detect-review-request's two-copies-of-one-pattern problem at expression scale. The sample takes one entry per tool group, ordered like the summary and capped at the leading three groups, rather than the first three distinct arguments overall -- a globally-unique list is ordered by the argument text, so the commonest tool can drop out of its own sample entirely (the first draft summarized six Task denials and then showed none of them). The summary itself stays uncapped, so a fourth tool is still counted even though it is not quoted. Token-shaped literals are redacted from the sample, because Actions masks a configured secrets.* value in a run log but not a credential the agent constructed itself. And a result carrying a scalar count with no array -- the exact mirror of the gha#531 case above, and what stub-gha198-high-denial-count.json already was -- reports names unavailable rather than an empty list, which would read as "nothing was denied".

denials is three-valued, and the reporting is the first consumer that has to care. It is a real 0, a real positive count, or the 999999 sentinel, which means unknown rather than "a great many". Both pre-existing gates want unknown to read as unsafe, so collapsing it into the positive case is exactly right for them -- which is what makes the conflation invisible to a reader of that code, and what the gha#540 reporting then inherited by writing its own guard as denials != 0. The two gates ask "may this be trusted?", where erring toward no is free. A log line asserts something about the run, and there erring is not free: an unparseable count says nothing about whether any denial occurred, so reporting it as the no-array case told a clean passing run it had denials (is-error-success-with-verdict.json, whose count is JSON null). So knownness is tracked as its own denials_known flag rather than re-derived from the magic number, the unknown case gets its own wording, and one denied_note feeds both the log line and the annotation so they cannot describe the same run differently. Note what the negative test could not see: must_not_log's original entry was genuine-finished-review.json, whose count is a literal 0, so the branch never fired for it under either version and the assertion passed while the bug was live. A fixture with a null count is the one that discriminates, which is the same lesson as the vacuous names unavailable needle below -- a negative assertion is only worth the fixture that can actually reach the branch. (gha#544 review, caught by the reviewer rather than by this suite.)

Its coverage needed a new kind of assertion, and the first draft of one passed vacuously. Everything run-fixture-tests.sh asserted before keyed on the exit code, a GITHUB_OUTPUT line, or the posted review_text_file; what gha#540 changed is what the script logs, which none of those can see. must_log/must_not_log are that assertion -- substrings the combined stdout/stderr must and must not contain -- and they matter more than their size suggests, since a dropped tool name regresses into a quiet log rather than a broken check. All five mutations were confirmed to turn the suite red (dropping the redaction, sorting alphabetically instead of by count, making the sample globally unique, deleting the names unavailable line, and emitting the summary only above the threshold), which is how the fourth one was caught not being: the assertion had been the bare phrase names unavailable, and the over-threshold annotation carries that phrase too as its own ${denied_summary:-...} fallback, so it passed with the log line deleted. It is anchored on the whole line now. Read that as the general shape rather than as one fixture's detail: a needle short enough to appear in a second, unrelated code path tests neither.

A test for one redaction pattern must be reachable by ONLY that pattern, and getting this wrong three times in one session is what makes it worth a rule. gha#543 moved denied_tools from a masked run log to an unmasked PR comment, so gha#548's review round 2 widened the chain to six patterns. Each new one was measured leaking before the fix and redacted after. The hard part was not the patterns; it was the tests. A credential written into an Authorization: header is caught by the generic header backstop whatever vendor prefix it carries, and one written into URL userinfo is caught by the userinfo pattern -- so a case built either way passes with the pattern it exists to test deleted. That happened to the Anthropic case, then again to the modern-PAT case after the first fix, then again when the PAT was moved into userinfo. Only a mutation sweep found it each time, because every version looked plausible and every version passed. So carry each credential in a form no other pattern can reach: a bare assignment or a file write, never a header and never a URL. A sibling pattern is one way to make such a case vacuous. The other is a later stage of the same path (gha#571, gha#574): a guard that rejects for a second reason -- a missing file, an empty value, a type check -- fails the input whether or not the alternative under test exists. So a negative case must be built so the code succeeds when the thing being tested is removed, not merely fails differently. Simulate both ways before trusting it. The same sweep found github_pat_ had no case at all -- a pre-existing pattern whose deletion turned nothing red. And pair the set with a benign command that must survive untouched, since a redaction that ate the diagnostic would satisfy every leak assertion while destroying what gha#540 added the names for.

The redaction mutation is the one whose fixture cannot be committed, so run-fixture-tests.sh builds that one at run time and assembles the token from a literal prefix plus a generated body. The committed-fixture rule two paragraphs down is the reason, in the form that has no undo: check-secrets scans this repo's own history, so a credential-shaped string in a committed file trips that scan for good, and deleting the file later does not reach the commit that introduced it. Generating it is the same move the test-coverage R-package fixture makes for a different scanner, applied to the one input that must never land in a commit at all.

A sixth mutation is about the summarizer itself rather than its output, and it is the one worth reading if only one is. Every jq lookup in both filters is ?-suppressed. That is not defensive habit: a denial entry whose tool_input is a string rather than an object makes a bare .tool_input.command raise Cannot index string with string, and under this script's set -e that aborts the whole run at exit 5 -- before the verdict check, before review_text_file, before anything. So the check goes red carrying a jq error in place of whatever the review actually concluded, which is strictly worse than the missing-names problem gha#540 set out to fix. Reproduced directly (exit 5, empty GITHUB_OUTPUT past the count) rather than reasoned about, and pinned by permission-denials-malformed-entries.json, whose expected outcome is the ordinary fail-stub -- the assertion is that the review still gets classified, not that some log line appears. The general shape is the one detect-review-request already records for its oversized-body E2BIG: an optional diagnostic must not be able to redden the job it is diagnosing. One further constraint follows from where those comments live. Both filters are single-quoted shell strings, so a comment inside them must carry no apostrophe -- one closes the string, and the first draft of that very comment failed bash -n by writing a possessive.

Fixing the guard alone leaves the check red, which is why gha#520 touched the workflow too. claude-code-action exits 1 on any is_error result, quota exhaustion included, and the Run Claude Code Review step carried no continue-on-error -- so the job failed on that step regardless of what the guard concluded afterwards. gha#350 had already made the guard run in this case; what it did not do was stop the step's own failure from deciding the job. Read a green fail-check step alongside a red claude-review job as this shape rather than as a contradiction: the guard's verdict and the job's conclusion were reached by different steps. Resolve final review outcome is documented as the only step that genuinely fails the job, and the retry attempt already carried the flag, so attempt 1 was the odd one out. Note that outcome is read before continue-on-error applies, which is what lets the retry keep gating on steps.claude-review.outcome == 'success' unchanged.

Four fixtures pin the gha#551 background-agent kind, and the fourth is the one the first three cannot replace. A no-verdict run whose transcript carries an EXECUTED background Agent/Task spawn -- a tool_use block whose input's run_in_background is absent or true, with no matching permission_denials entry -- classifies failure_kind=background-agent and does NOT write stub_review, so the gha#185 retry never fires for it. stub-background-agents-executed.json (explicit true) and stub-background-agents-omitted-param.json (parameter omitted; it defaults to true, and an omitted parameter is exactly what the gha#550 deny rule cannot match) pin the != false test's two positive arms. stub-sync-agents-only.json is the over-matching negative: an all-synchronous fan-out that stubbed for an unrelated reason must stay a retryable stub. stub-denied-bg-spawns-in-transcript.json is the subtraction's own discriminator -- both spawns appear in the transcript AND in permission_denials, which is what a real denied call looks like post-#550, so none executed and the retry survives. That fourth fixture exists because a mutation sweep proved the first three cannot see a dropped subtraction: the gha#550 spawn-denial fixtures carry no transcript tool_use blocks at all, so executed = uses and executed = uses - denied agree on every fixture that predates it. One meta-lesson from the same sweep, recorded because it produced three vacuously-green mutation results in one sitting: a mutation applied by string replacement must be CONFIRMED to have changed the file (git diff --quiet before running the suite), and the restore step must restore COMMITTED work -- a git checkout after each mutation, run against a not-yet-committed implementation, silently reverts the implementation itself, and every later "mutation" then tests the detector's absence rather than the targeted edit. Commit first; mutate second.

Applying a mutation is not aiming it, and per-case branches are where the two come apart. The note above catches a mutation that never reached the file. A mutation can reach the file, change it, and still tell you nothing -- because the line it changed is not the line the test exercises. That happens the moment an implementation grows per-case branches: mutating one arm of a case leaves every test whose input takes a different arm green, which reads exactly like "the suite does not pin this" when the suite pins it fine. Measured on gha#742, where two of five mutations "survived" for this reason alone -- one mutated the - arm while the test's candidate took the * arm, and one replaced a guard's return 1 with an assignment that happened to stay correct for the input under test. So state which case each mutation is supposed to break BEFORE running it, and read a survivor as a question about the mutation first and the test second.

Two fixtures pin the gha#550 spawn-denial exclusion, and the pair is the point rather than either one. spawn-denials-only-retryable.json carries an 8-spawn fan-out whose denials are all deliberate: the raw count clears the threshold while the starvation-relevant count is zero, so it must stay a retryable stub. spawn-denials-plus-starved-calls.json carries those same 8 beside 6 genuinely-starved gh api calls and must still classify high-denial. Without the second, an exclusion that simply zeroed the count would pass. Confirmed by mutation rather than assumed: reverting the gate to the raw count turns the first red and leaves the second green.

.github/actions/parse-workflow-ref/tests/run-tests.sh exercises the extracted parse-workflow-ref.sh (see Layout above) offline against a tag, a branch, and a full-SHA ref; CI runs it as a step in the same review-fail-check job in _selftest.yml (gha#191).

review-fail-check also runs run-review-guard (see Layout above) itself via a real uses: ./.github/actions/run-review-guard step against the genuine-finished-review.json fixture, asserting it produces a non-empty review_text_file output -- and a second such step against a stub fixture, asserting the stub_review output comes back true. Unlike the fixture tests above (which invoke check-review-execution.sh directly), these prove the composite action's github.action_path-relative resolution of that script, and its output passthrough wiring, actually work -- a gap that let gha#191's job_workflow_ref regression (gha#196) go unnoticed: the sed-parsing logic was unit-tested, but nothing exercised the real uses: call end-to-end. run-claude-review-attempt (see Layout above) has no equivalent selftest coverage -- it wraps a live anthropics/claude-code-action call, which isn't something a selftest job can exercise offline; it's validated the same way the inline step it replaced always was, by real PR reviews once merged and released.

.github/workflows/scripts/tests/run-sum-costs-tests.sh exercises sum-costs.sh (see Layout above) offline against a table of (cost-a, cost-b) pairs, including both-empty and one-empty cases; CI runs it as a step in the same review-fail-check job. review-fail-check also runs extract-total-cost and sum-costs themselves via real uses: steps (the same github.action_path-resolution proof the run-review-guard e2e steps give), asserting extract-total-cost surfaces the right cost for a real fixture and stays silent for a missing file, and sum-costs surfaces the right total for a real uses: call (gha#219 review finding 5).

.github/workflows/scripts/tests/run-detect-review-request-tests.sh exercises detect-review-request.sh (see Layout above) offline against a table of comment bodies: the phrasings that must dispatch a review, the ones that must not (a quote-reply, an @claude request that merely contains the word "review", reviewer as a whole different word), a multi-body call, and stripper-failure propagation under set -e (gha#451). CI runs it as a step in the review-fail-check job, which also calls detect-review-request itself through two real uses: steps -- the same github.action_path-resolution proof the run-review-guard / extract-total-cost / sum-costs e2e steps give, plus the one thing the offline tests cannot reach: the base64 round-trip on the bodies-file path, built by that job with the same jq ... | @base64 pipeline claude.yml uses. As with request-dependabot-review below, claude.yml's own layer above the composite is not covered -- it calls the action via Morrison-Lab/gha/...@v2, which does not resolve until @v2 is advanced past this capability's merge.

.github/workflows/scripts/tests/run-detect-bot-mention-tests.sh covers the quoted-mention gate the same way, and review-fail-check also calls detect-bot-mention through two real uses: steps -- one quoted mention, one genuine -- for the same github.action_path-resolution proof. Both directions are asserted deliberately: the only behaviour this gate can cause is a skip, so the negative case is the bug and the positive case is what stops the fix from silencing real requests. Read its true rows as the guard rails rather than as filler.

.github/workflows/scripts/tests/run-mention-filter-tests.py pins the gha#554 job split that moved that decision out of the agent job. _selftest.yml cannot invoke claude.yml (that would be a live agent run), so the suite reads the workflow YAML and executes the proceed step's own script against a table: a real mention dispatches, a match=false quoted / code-span / fence mention does not start the agent job, and an allowlisted assignment still dispatches with no mention. It also asserts the wiring that would silently undo the split -- claude needing mention-filter, the agent if: requiring proceed == 'true' rather than != 'false' (!= 'false' fail-opens an empty output from a successful filter job, and is the second half of always() && != 'false'), outputs.proceed reading the proceed step rather than match (which would kill assignment-without-mention), the detect step's if: omitting workflow_dispatch/schedule (four empty bodies print false), the proceed step having no if: (a match-nonempty gate would skip it on dispatch/schedule and fail-close unattended runs), those two events still admitted at the job if: (gha#245), the trusted-author association gate living on mention-filter now that the agent job's only if: is proceed, ubuntu-latest rather than inputs.runs-on, no caller checkout, no concurrency group on the filter, and detect-bot-mention living only in the filter job. CI runs it as a step in the same review-fail-check job. The composite already exists at @v2; this suite pins the workflow job split by reading claude.yml, not by invoking it. claude.yml@v2 itself picks the split up only when the major tag slides, the usual reusable-workflow lag, not a new-composite bootstrapping gap.

.github/workflows/scripts/tests/run-strip-non-invoking-markup-tests.sh covers the stripper that matcher now pipes its bodies through, as a table of (input, expected output) pairs rather than of verdicts. CI runs it as a step in the same review-fail-check job in _selftest.yml. It is a separate suite because the stripper already has two consumers (detect-review-request and detect-bot-mention / mention-filter) and because its failure modes run in both directions: under-stripping dispatches on quoted text, over-stripping swallows a real request. So the table pins both, and the cases that matter most are the ones a verdict-level test cannot distinguish -- an unclosed backtick run left alone, a shorter run failing to close a longer fence, a dropped block not joining its neighbouring lines. Three of its cases exist because gha#345's first review round found the corresponding gaps by hand-tracing the awk against CommonMark: a span whose delimiters sit on different lines, a 4-space-indented delimiter that must not close a fence, and an indented code block, which the first draft did not handle at all. Each was reproduced end to end through detect-review-request.sh before the fix and again after, since all three were under-stripping -- the direction that dispatches a review off quoted text. Their counterweight is the pair of cases asserting that an indented list continuation and an indented line with no blank line before it are kept: that is the over-stripping direction, which drops a genuine request. Two of the new cases in the matcher's own table were checked against main's pre-fix script to confirm they fail there, per the regression-test rule in Morrison-Lab/ai-config's shared/workflow/ardi.md; the first draft of the double-backtick case passed pre-fix for an unrelated reason (trailing prose after the keyword already blocked the match), so it was rewritten to end the line and re-checked.

.github/workflows/scripts/tests/run-classify-push-failure-tests.sh exercises classify-push-failure.sh (see Layout above) offline against a table of push outputs: the verbatim GitHub App rejection gha#360 was filed over, its PAT and OAuth App wordings, two non-fast-forward phrasings, and cases that must fall through to other (a protected-branch decline, an auth failure, an empty log). It also pins the four-part output contract the composite parses -- kind=, withhold-patch=, headline=, blank line, advice -- and that the headline stays a single line, since it reaches an ::error:: annotation. The composite reads each field by fixed line offset, so a reordering breaks it silently; that is why the shape is asserted rather than only the values. The one assertion worth reading as a guard rail rather than filler is that a generic failure's advice does not name WORKFLOW_TOKEN: the value of naming the secret comes entirely from naming it only when it is the cause. CI runs the suite as a step in the review-fail-check job, which also calls report-push-failure itself through four real uses: steps with dry-run: true -- one per classified kind (workflows-permission, a backtick-free other case, no-push-attempt, and the push-protection case, whose assertion is that no patch is rendered) -- the same github.action_path-resolution proof the other e2e steps give, plus the two things the offline table cannot reach: the patch generated from a live checkout, and the credential redaction. Those steps run last in that job because they commit a throwaway file to the checkout, which is what gives git format-patch a real one-commit range to render (a merge commit would render as nothing, since format-patch skips merges). As with detect-review-request, claude.yml's own layer above the composite is not covered -- it calls the action via Morrison-Lab/gha/...@v2, which does not resolve until @v2 is advanced past this capability's merge.

.github/workflows/scripts/tests/run-classify-gemini-failure-tests.sh exercises classify-gemini-failure.sh (see Layout above) offline against a table of run-gemini-cli error outputs: a 429/RESOURCE_EXHAUSTED quota blob, a 403/PERMISSION_DENIED auth rejection, plain suspended-project and rate-limit wording with no JSON envelope, and cases that must fall through to other (a malformed-request/INVALID_ARGUMENT blob, a network timeout, an empty error output) -- the last group matters as much as the first, since a genuine bug swallowed into the graceful-skip path would silently stop failing the check it should fail. Three more regression fixtures pin the anchoring fix from gha#380 review finding 1: a stack trace whose line:column number contains a bare 429 substring, an MCP log line carrying the bare word disabled, and a bare HTTP 429 status line with no other marker present -- the first two must classify other (the pre-fix regex matched either as a substring), the third must still classify quota-or-auth via the anchored HTTP-status-line alternative. It pins the four-part output contract (kind=, headline=, blank line, advice) by fixed line offset, the same reasoning run-classify-push-failure-tests.sh gives, and separately asserts the advice never embeds the raw error output itself -- that split is deliberate (see Layout above), so a test asserting the opposite would be asserting a regression. CI runs this suite as a step in the new gemini-review-fail-check job, which also calls report-gemini-failure itself through two real uses: steps with dry-run: true -- one quota-or-auth fixture, one genuine (other) fixture -- proving github.action_path resolution end-to-end, the same proof run-review-guard's/report-push-failure's own e2e steps give. Kept as its own job rather than folded into review-fail-check above, so a failure here is attributable at a glance -- the same one-capability-per-job split phi-tests/new-line-breaks-tests already use. As with report-push-failure, gemini.yml's/gemini-code-review.yml's own consumption of this composite via @v2 is not covered here -- it does not resolve until @v2 is advanced past this capability's merge.

.github/workflows/scripts/tests/run-compose-review-failure-report-tests.sh exercises compose-review-failure-report.sh (see Layout above) offline. The script emits prose, and asserting prose word-for-word produces a suite that fails on every wording change while catching nothing, so the table asserts two things only: the four-part output contract by fixed line offset (the same reasoning run-classify-push-failure-tests.sh gives for its own), and the claims a reader would act on -- which kind was chosen, and whether the denied-tool line says names, none, count with unavailable names, or not recorded. The four-case denied-tools group is the part to keep if the suite is ever trimmed, and its middle case is the trap: an empty denial count must not render as "none", since that asserts something about permissions on a run that never measured them. Seven mutations were confirmed to turn it red rather than assumed to -- collapsing "not recorded" into "none", dropping the contract's blank line, restoring the duplicated 5 default, accepting any kind verbatim, giving two kinds the same headline, printing the 999999 sentinel as a count, and eval-ing the denied-command text instead of rendering it verbatim. That last one is pinned by a single POSITIVE assertion. A negative counterpart was written and removed: a needle naming the substituted result is either the bare username, which may legitimately appear elsewhere in a report, or a marker string no version of the script can emit -- and the marker form passes under every mutation, which is the same vacuous shape this file records two paragraphs down for must_not_log. CI runs it as the review-failure-report job in _selftest.yml, kept separate from review-fail-check so a failure is attributable at a glance -- the same one-capability-per-job split phi-tests and gemini-review-fail-check use. That job also calls report-review-failure through two real uses: steps with dry-run: true, for the github.action_path-resolution proof the other composite e2e steps give plus the one thing the offline table cannot reach, since it calls the script directly and so cannot see whether action.yml wires its inputs through at all. The second of those two steps is not a duplicate of the first: it pins normalization, the only judgment the composite makes for itself. As with report-push-failure, claude-code-review.yml's own consumption of this composite via @v2 is not covered here -- it does not resolve until @v2 is advanced past this capability's merge.

.github/workflows/scripts/tests/run-check-credential-shape-tests.sh exercises check-credential-shape.sh (see Layout above) offline against a table of credential values. The negative cases are the ones to keep if the suite is ever trimmed, which inverts this file's usual advice for a detector: everywhere else an over-cautious verdict is free, and here it is the expensive error, because a true answered too eagerly blocks review for a consumer whose credential works today. So a plain token, a trailing newline, a leading newline, surrounding spaces, and an empty value must all pass through untouched. Three mutations were confirmed to turn it red rather than assumed to --- dropping the trimming (4 failures), disabling the detector outright (7), and matching only newlines rather than any whitespace (3). Read those counts off the suite's own N of 14 summary line rather than by counting ::error:: lines, which is how the first draft of this paragraph reported each one inflated by one: the summary is itself an ::error:: line, so a grep -c over them counts a fourteenth thing that is not a case (gha#687 review finding 4). The two-line contract assertion compares the line count arithmetically, which is portability rather than taste: BSD/macOS wc -l pads its output with leading spaces, so a string comparison against "2" failed all 14 cases on a maintainer's own machine while CI stayed green (gha#688). A suite meant to be runnable off-runner failing off-runner is the least visible failure available, and a red local run reads as the change under test having broken something. No other site in the repo compares a raw wc -l result as a string, but the enumeration needs three buckets rather than two: three sites compare arithmetically (run-classify-opencode-run-tests.sh, run-classify-gemini-failure-tests.sh, run-classify-push-failure-tests.sh), four strip the padding first with tr -d (check-junk-files.sh twice, claude.yml, inject-canonical-urls/action.yml), and three never compare at all, interpolating the count into a message where the padding is harmless (_selftest.yml twice, claude.yml). That third bucket is why "every other site compares safely" would have been false: those three avoid the bug by not being a comparison. CI runs it as the credential-shape job in _selftest.yml --- on a [ubuntu-latest, macos-latest] matrix since gha#690, so the BSD/GNU wc padding class the suite exists to catch is exercised on BSD userland rather than only narrated --- kept separate from review-fail-check so a failure is attributable at a glance --- the same one-capability-per-job split phi-tests and gemini-review-fail-check use. That job also calls the composite through three real uses: ./... steps, for the github.action_path-resolution proof the other composite e2e steps give plus the one decision the composite makes for itself, which the offline table cannot reach: combining two secrets into a single verdict. claude-code-review.yml's own @v2 consumption of the new composite is the usual bootstrap gap until the tag slides.

run-compose-review-failure-report-tests.sh gained the bad-credential kind alongside it, and its two negative assertions are the ones worth reading. That kind is the only one where no review process ever started, so the shared opening paragraph's "finished without producing" would be false of it, and both the denied-tools line and the cost line would describe a run that never happened --- "not recorded" would send a triager to look at permissions on a run that never reached them. Both suppressions were confirmed load-bearing by mutation, which matters more than usual here: a negative assertion written against an empty output passes under every mutation, and the first draft of these did exactly that (the test helper is $COMPOSE, and $compose_script silently produced no output under the suite's set -uo pipefail).

.github/workflows/scripts/tests/run-pack-review-payload-tests.sh and .github/workflows/scripts/tests/run-review-job-split-tests.py pin the gha#580 credential split. The pack suite asserts payload.json's key set and that sidecar files are omitted when the corresponding input is empty (a missing review.txt must not look like a present empty review). The YAML suite reads claude-code-review.yml and run-claude-review-attempt and asserts the facts a future edit could reverse silently: the model job requests EXACTLY the keys contents/pull-requests/issues/actions and no others (the set is over KEYS; separate per-key assertions pin the values against write) --- so any future addition fails offline instead of at a consumer's next PR, which is what gha#830 did not (gha#831, gha#832) --- so it grants no forge-write, no id-token: write, and no checks: read, the posting job holds pull-requests: write / issues: write / actions: read and does not invoke the model, github_token is forwarded so the App-token write exchange is skipped, the inline-comment MCP tool is not allowlisted, pack still runs after a failed resolve-final (!cancelled() plus success/failure outcomes; a default success() gate would skip it), the gha#543 failure notice still posts when the packed artifact is missing (!cancelled() plus download.outcome != 'success' ORed with the loaded resolve_outcome == 'failure' path, including a successful review so Require exiting 1 does not skip the notice), caller grant lists (the example stub, README, permissions page, reference Permissions and Example) include actions: read, and post-review stale-checks against event-pinned reviewed-head (github.event.pull_request.head.sha), falling back to gather-context's stash-head on dispatch, rather than a later API fetch from the model job. CI runs both, plus a real uses: ./ call to pack-review-payload with upload: false, as the review-job-split job in _selftest.yml -- kept separate from review-fail-check so a failure is attributable at a glance. claude-code-review.yml's own @v2 consumption of the new composite is the usual bootstrap gap until the tag slides.

run-fixture-tests.sh gained three assertions alongside it, and the last two are the ones worth reading. failure_kind is asserted for every fixture rather than only the failing ones, because a stale kind left on a clean run is what would let the comment describe a failure that did not happen; the exit code and stub_review are identical across hard-error, no-output, and high-denial, so nothing else in that suite can tell those three apart. max_denials is asserted as an invariant instead of per fixture -- it is written immediately after denials, unconditionally, so the two must always co-occur. Keying on that relationship rather than on a table means a fixture added later cannot forget to declare it, and it pins the value, so a caller reading an empty output and silently falling back to a hard-coded 5 is caught here rather than on a live review. The first draft asserted failure_kind alone, and dropping the max_denials write was confirmed to pass under it -- an unasserted new output is exactly what regresses in silence.

assert_denied_tools_presence is the third, and it pins a contract that had been asserted only in prose, and asserted wrongly: run-review-guard's docs claimed denied_tools was set on every exit path, which is false for the two short-circuit exits that return before the denial count exists. It asserts presence rather than content -- the value is legitimately empty on a zero-denial run, so denied_tools must be present exactly when denials is, and writing it on the short-circuit path turns the suite red. The distinction it protects reaches the PR comment: an ABSENT value means "never counted", an EMPTY-but-present one means "counted, and there were none", and only the second licenses saying the reviewer was not blocked by permissions.

.github/workflows/scripts/tests/run-detect-pr-workflow-edits-tests.sh exercises detect-pr-workflow-edits.sh (see Layout above) offline against a table of changed-file lists: a top-level workflow, the caller workflow, a nested workflows/scripts/ path, a composite action.yml, and an unset PR_CHANGED_FILES (must fail closed). CI runs it as a step in review-fail-check, which also calls detect-pr-workflow-edits through a real uses: ./... step on pull_request events for the github.action_path-resolution proof. That e2e asserts the composite's workflow_edits matches an independent classify of the same PR (the scripts the composite wraps), not merely that the output is the string true or false --- a detector that always returned false would still pass the boolean-only check on a workflow-editing PR.

.github/workflows/scripts/tests/run-list-pr-changed-files-tests.sh exercises list-pr-changed-files.sh against a stub gh: a complete two-file list succeeds, a list shorter than changed_files fails closed, and a missing or non-numeric changed_files fails closed. CI runs it in the same review-fail-check job. The truncated-list case is the one to keep if the suite is ever trimmed: GitHub's files endpoint caps at 3000 and still returns 200, so treating that response as complete would dispatch --ref at an unknown tree.

.github/workflows/scripts/tests/run-restore-default-branch-workflows-tests.sh exercises restore-default-branch-workflows.sh against throwaway git repos in $TMPDIR: a modified workflow plus a PR-only file must be replaced/deleted, a missing DEFAULT_BRANCH fails closed, a ref with no .github/workflows tree fails before deleting, and a pathspec-only checkout is shown not to delete the PR-only file (so the rm -rf is load-bearing). CI runs it in the same review-fail-check job. There is no live uses: of the restore composite against this checkout: restoring this repo's own .github/workflows/ mid-selftest would clobber later steps.

.github/workflows/scripts/tests/run-workflow-audit-tests.py covers the two workflow-wide audits _selftest.yml runs and the discovery module beneath them (gha#716, gha#720). Both audits used to be inline run: blocks in _selftest.yml grepping .github/workflows/*.yml; they are now audit_workflow_token_usage.py and audit_workflow_action_pins.py, sharing workflow_discovery.py with run-permissions-docs-tests.py and run-workflow-job-guard-tests.py. That is one copy of the discovery rule in the repo rather than four places for it to drift back to *.yml only --- which is the drift #712 and #716 each fixed separately, in two of those four.

Parsing replaced grepping because a line anchor cannot see either thing that matters here. ^\s*uses: matches the continuation form and not - uses: ..., and this repo writes both --- three action references, all in altdoc-multiversion-docs.yml, were exempt from the pin audit on that basis alone (measured 2026-08-28 against main at 7719d04; two further - uses: lines match textually but sit inside _selftest.yml heredocs and are not references). Widening the anchor is not the fix: two of the five lines it newly matches are _selftest.yml's heredoc-written fixture workflows, which are text inside a run: block rather than references GitHub resolves. A walk over parsed jobs.*.steps[].uses and jobs.*.uses sees both spellings by construction and cannot reach heredoc content at all. The same argument applies to the token audit, which now tells token: from submodules-token: by key rather than by what precedes the colon.

Testing the discovery is not testing the audits, and this repo's own tree is why. It carries 63 .yml workflows and zero .yaml ones, so a consumer reverted to a *.yml-only glob leaves every check green --- the audits pass because there is nothing for the wider glob to find, not because discovery works. Each audit therefore gets a fixture whose violation lives in a .yaml file. Those cases are the ones to keep if the suite is ever trimmed, alongside the negative ones: an unparsable workflow is an error rather than a clean file, an empty or missing directory fails closed rather than returning an empty list, submodules-token: is not flagged, and a uses: inside a run: block is not a reference.

run-permissions-docs-tests.py gained two cases of its own (10 and 11). The first is end-to-end: a .yaml reusable workflow that discovery misses is absent from the expected read-only set, so a doc correctly listing it reads as "listed but the workflow does not exist". The second asserts discover_workflows directly, so reverting it cannot be "fixed" by widening the glob to everything. Every case that predates them names only .yml fixtures, which is why none of them could see the gap.

Both audits refuse a malformed workflow rather than walking past it. An absent or wrongly-typed jobs, steps, with, or step entry means the audit examined nothing in that file, which is not the same as finding nothing in it --- the parsed-walk version of reading grep's exit 2 as exit 1. actionlint catches these too, so this is defence in depth rather than the only detector, but an audit that reports clean over a file it never walked is the exact failure both of these exist to prevent.

The token audit is deliberately not scoped to actions/checkout. That action is the canonical case rather than the only one: any action handed this secret through a token: input is being trusted to authenticate against the caller's own repository, which is precisely what it cannot do. Scoping to one action name would also miss a fork, a wrapper composite, or a rename, and the two errors are not symmetric --- a false negative ships a broken checkout to a consumer, a false positive is a one-line conversation on a PR. The reported line names the step's action so a genuine exception is recognizable at a glance, and a test pins the breadth so narrowing it later is a decision rather than a silent regression.

Two matches are anchored deliberately, and a bare in gets each wrong in opposite directions. The self-exemption is Morrison-Lab/gha or Morrison-Lab/gha/..., not a bare prefix, which would also exempt Morrison-Lab/gha-evil --- somebody else's repository, under an exemption that exists to say the code is ours. The secret test is the identifier SUBMODULES_TOKEN on word boundaries, not a substring, so NOT_SUBMODULES_TOKEN is a different secret rather than a blocked workflow.

The reference is classified before its pin is validated. A repository action or reusable workflow is pinned by a 40-character Git commit; a docker:// image by an @sha256: digest. Accepting either form everywhere is the easy mistake and passes actions/checkout@sha256:... and docker://alpine@<40hex>, neither of which resolves to anything. A docker:// ref pinned only by tag still fails.

The token audit walks job-level with: and secrets:, not only steps. A reusable-workflow caller passes values down through those blocks, and the regex this replaced matched any line-leading token: and so covered them incidentally. A parsed walk that visits only steps would have been a coverage regression wearing a refactor's clothes --- which is the general risk when replacing a text scan with a structural one, since the text scan's reach was never written down anywhere. Input keys are compared case-insensitively, because GitHub's runner resolves an action's inputs that way --- so a Token: that reads as a different key here is the same input there, and matching case-sensitively would leave a one-keystroke bypass of the whole audit. secrets: inherit is the one legitimate scalar in either block and is skipped; a scalar with:, or a secrets: naming anything else, is refused, because skipping every string would leave a block the audit never examined reported as clean. A token: whose value is a list or mapping is refused for the same reason, while a number or boolean --- which an input may legitimately be --- is not.

The suite mutes each audit's own output. An expected failure still prints ::error::, and GitHub renders every one as an annotation, so an unmuted suite decorates a passing job with a dozen errors it deliberately provoked.

CI runs all of this in the lint-checkout-tokens job, unit tests first. Nineteen mutations were confirmed to turn a named case red rather than assumed to, enumerated so the count is checkable against the list rather than asserted over it:

  1. a *.yml-only discovery, read by the audit suite;
  2. the same, read by the permissions-docs suite;
  3. a dropped empty-directory guard;
  4. a swallowed parse error;
  5. a pin regex accepting any @ref;
  6. a skipped step-level uses: walk;
  7. a token lookup matching any key containing token;
  8. a bare-prefix self-exemption;
  9. a substring secret match;
  10. a skipped malformed steps;
  11. a tolerated missing jobs;
  12. a skipped non-string step-level uses:;
  13. a pin regex rejecting a docker:// image digest;
  14. a pin test accepting either form everywhere;
  15. a token walk skipping job-level blocks;
  16. a refused secrets: inherit;
  17. a skip of every scalar block rather than of secrets: inherit alone;
  18. a tolerated container-valued token:;
  19. a case-sensitive input-key match.

.github/workflows/scripts/audit_example_concurrency.py closes gha#809: a caller-level concurrency: group that the called reusable workflow also declares deadlocks the run, and the failure is unusually hard to diagnose. How it presents is not settled, and the two recorded instances disagree. gha#437's presented as a cancelled run, which is how GitHub documents the condition -- it cancels with a message naming the group and both sides (Canceling since a deadlock for concurrency group '...' was detected between 'top level workflow' and '...', quoted verbatim in rhysd/actionlint#538, which asks actionlint to detect the condition statically). The ucdavis/hac.sap instance instead presented as a nested job with no runner, no steps, and no log. Whether that is a second failure mode, a difference between the caller's and the callee's view of the same cancellation, or a misreading of the run page has not been established here, so do not repeat either observable as the general case without checking the run in front of you. gha#437 recorded the mechanism for the review family; gha#654 then added group: gh-pages to quarto-publish.yml's deploy job without touching examples/quarto-publish.yml, which still told consumers to declare the same group at the top level, so every publish run on a consumer that copied the stub failed silently and the site stopped updating (ucdavis/hac.sap, run 33604968678). The audit walks every stub under examples/, resolves each uses: to the workflow file it names, and fails on any caller-level group that the called workflow also declares, at either of ITS two placements -- the callee's jobs and the callee's own top level; a stub naming a workflow file this repo does not carry is an error rather than a skip. Caller-level means two placements, not one. A top-level block covers the whole run and so covers the calling job, and a block on the calling job itself is the same collision written one level down -- concurrency: is valid on a job that uses: a reusable workflow, gha#811's review reproduced the audit exiting 0 over such a block on the live tree -- that the two then deadlock is inferred from the mechanism (a group is a group, wherever the caller declares it) rather than observed, and actionlint#538's record covers a caller top level against a group the callee supplies, not a caller-authored job-level block. Only the CALLING job's own group counts: a matching group on some other job of the stub serializes those two jobs and never waits on the callee, so reporting it would be a false positive, and a test pins that. The summary line counts calls actually COMPARED rather than walked past, since most stubs carry no caller-level group at all and a count that included them would read the same after the comparison was gutted. The CALLEE side has two placements too, and checking only its jobs missed one: a reusable workflow's own top-level concurrency: applies to the calling job, so a caller group matching it deadlocks exactly as a job-level one does. bump-dev-version.yml is this repo's live example of a workflow_call workflow carrying one, and before gha#811's review the audit walked past it. The audit is one level deep, while GitHub permits four. A callee job that itself calls another of our workflows is not compared. Eleven workflows here do call another of ours, but none of those eleven declares workflow_call:, so none can be a callee and no nesting is reachable from a stub -- a limit rather than a live gap. The docstring states it too, so a reader of the script alone gets it. And the comparison is literal string equality, so it sees constant group names only. That covers the gh-pages family, whose groups are the constant gh-pages. It does NOT cover a group written as an expression: the review family's groups are ${{ }}-valued, so no review stub can ever be flagged, and altdoc-multiversion-docs.yml's build job holds a group whose ${{ }} part evaluates to github.ref outside a pull request, so the group as a whole becomes altdoc-multiversion-docs-refs/heads/main -- identical to what a caller writing altdoc-multiversion-docs-${{ github.ref }} requests, while the two strings never match textually. gha#822 tracks normalizing expressions before comparing. EVERY non-string scalar group is refused rather than compared, not the subset that happens to survive str(). YAML's scalar resolution is lossy, so the audit cannot recover what the author wrote: measured against yaml.safe_load, 010 arrives as 8, 0x10 as 16, 1_000 as 1000, +5 as 5, 1:30 as 90, true as True and 1.10 as 1.1. The suite tries nine spellings; all nine resolve to a non-string and are refused, and the seven listed here are the ones that come back from str() as a plausible name the author never wrote, which is what makes them dangerous rather than merely wrong. Comparing any of those against a callee group written the same way is a silent false negative, in the one function whose contract is to refuse what it cannot evaluate, and the remedy is one pair of quotes, which the message asks for. Its first live run found a second collision the issue had not listed, examples/report-failure.yml, which is why the check is derived from the tree rather than from the issue's list. run-audit-example-concurrency-tests.py builds throwaway stub-and-workflow pairs per case, since the repo's own tree must never carry the collision; the negative cases (no top-level block, a different group, a callee with no job-level group, and a group on a NON-calling job) are the ones to keep, because without them the audit would fail every stub the moment any workflow gained a concurrency block. One case exists purely to pin the *.yaml half of the population, which neither another case nor the live run can reach -- examples/ holds only *.yml, so reverting the glob leaves everything else green, verbatim the class this file documents for workflow_discovery above. The non-mapping-job guard is declared TWICE (in job_groups and in callee_calls), so a single-site mutation survives the suite and only mutating both turns its case red; read that survivor as the other site still holding rather than as missing coverage. CI runs it as the example-concurrency job in _selftest.yml, unit tests first, then the live audit.

.github/workflows/scripts/audit_capability_versioning_docs.py closes gha#730: every capability that ships past the frozen @v1 snapshot must be named in each hand-restated versioning-list region -- README.md's ## Versioning section and its nested ### Pinning third-party actions subsection, website/versioning.qmd, website/workflows.qmd, and this file's own "About this repo" -- and that restatement has drifted on four separate occasions (gha#181, gha#374, gha#728 rounds 1 and 2) because every prior fix checked the lists against each other, or against whichever files happened to already be in that round's diff, rather than against a ground truth outside all of them. The population is derived from .github/workflows/*.yml file existence, never from any list: deriving it from a list only checks lists against each other, which is exactly the failure mode that let gha#728 through (every list agreed with every other list, and all of them were wrong). A capability's actual pinned tag is read from its own examples/<name>.yml self-reference, since that stub is what a consumer copy-pastes and a wrong tag there is self-defeating independent of anything this script checks -- picking the right line among several in one stub (claude-code-review.yml and report-failure.yml each reference an unrelated Morrison-Lab/gha/... path before their own) is done by matching the path segment naming the capability, not by taking the first self-reference in the file. Each prose location is a hand-registered (file, start heading, end heading) region rather than a free-text scan of the whole file, because a README capability table entry and a README versioning-list entry are different sections of the same file that must not be conflated -- a whole-file presence check would have missed gha#728's own case, since check-code-similarity was present in the table while absent from the list. Both markers must be found or region extraction refuses outright, so a renamed heading surfaces as a loud failure rather than silently narrowing the blind spot to nothing. Only the missing direction is checked, never the reverse: a region also names some @v1 capabilities on purpose, to say they were audited and confirmed unchanged, and a first draft that also flagged those as "stale" produced exactly the false-positive noise the issue's own abandoned prototype (21 false positives per file) was scrapped over. Presence within a region is an exact backtick-delimited token match, never a word-boundary regex -- \bgemini\b matches inside gemini-code-review too, since a word boundary sits on both sides of the substring there, and matching the literal `gemini.yml` or `gemini` token sidesteps that without a lookaround. Both spellings are checked at every site (bare name and name.yml) because that is exactly what gha#728's second round missed: the grep used to find round 1's gaps was keyed to the .yml spelling, and this file's own list names capabilities bare. .github/workflows/scripts/tests/run-audit-capability-versioning-docs-tests.py builds a fresh fixture tree per case from the module's own REGIONS registry (never a second hard-coded copy of it), and its cases are the usual mix of positive and negative: a capability listed everywhere passes, one missing from a single region produces exactly one finding naming that region, a @v1 capability is never required to be listed (and being listed anyway still isn't flagged), the gemini / gemini-code-review substring trap is asserted directly against is_listed, extract_pin picks the right line out of several self-references and raises on an ambiguous or absent one, discover_population excludes a workflow file with no matching example and an example with no matching workflow file (the assemble-news shape) and refuses an empty overlap outright, and extract_region refuses when either heading marker is missing. Three mutations were confirmed to turn the suite red rather than assumed to: is_listed always returning True, extract_pin taking the first self-reference instead of matching by name, and extract_region falling back to end-of-file instead of refusing when the end marker is absent. CI runs the suite as the versioning-docs job in _selftest.yml, unit tests first, then a real invocation of the script against this repo's own tree -- the same "unit tests, then the live self-check" structure lint-checkout-tokens uses just above. This audit run against the live tree at PR time found four capabilities (ai-code-review.yml, antigravity-code-review.yml, gemini.yml, gemini-code-review.yml) missing from README's main ## Versioning paragraph -- present in website/workflows.qmd's equivalent, so the drift had already happened silently -- plus small-model-agent.yml missing from the ### Pinning third-party actions subsection; both are fixed in the same PR.

.github/workflows/scripts/tests/run-trigger-bugbot-review-tests.sh exercises trigger-bugbot-review.sh (see Layout above) offline against a stub curl: a successful queue, DRY_RUN=true in the JSON body, HTTP 400 when Bugbot is disabled for the repo, HTTP 401, a curl transport failure, a 2xx body with "outcome":"error", and missing CURSOR_API_KEY / PR_URL. The stub is the coverage that matters, because _selftest.yml cannot call api.cursor.com and a live queue would bill. Two of its cases are the ones to keep if the suite is ever trimmed (gha#511): a "dry_run":false API body must win over a local DRY_RUN=true, and the stub's captured argv must not contain Authorization or the raw key -- the --header form put the base64 credential on argv even though the comments claimed otherwise. CI runs the suite as the cursor-review-check job, which also calls trigger-bugbot-review through a real uses: step with that same stub on CURL_BIN, proving github.action_path resolution. cursor-code-review.yml's own consumption of the composite via @v2 is not covered here -- it does not resolve until @v2 is advanced past this capability's merge.

.github/workflows/scripts/tests/run-classify-review-delivery-tests.sh exercises classify-review-delivery.sh (gha#362) offline against a table of comment bodies. That script decides whether a dispatched review actually produced a verdict, because a run CONCLUSION of success is not the same as "produced a verdict" -- claude-code-review.yml deliberately succeeds on a graceful quota skip (gha#520) and surfaces the skip through a comment, so the run-level conclusion cannot see the commonest runtime failure there is. It tests for FAILURE markers rather than for a positive verdict, and that direction is the design rather than caution. Requiring each agent's success marker means a wrong pattern makes every review by that agent fall through to a second agent, which costs a duplicate paid review and, on workflows sharing a per-PR cancel-in-progress group, can cancel the review it was checking. The failure direction degrades to today's accept-on-success behaviour instead. It is also complete for the two agents that emit markers, which is what makes the safe direction the correct one: gha#548 made every no-verdict path in claude-code-review.yml post a failure comment, and report-gemini-failure (gha#379) does the equivalent, so "no failure marker" and "produced a verdict" coincide there. The case to keep if the suite is ever trimmed is the discriminating negative: a failure marker on a different run must not decide this one. Without it, scoping the match to comments naming this run could be dropped and every other case would still pass. Four mutations were confirmed to turn the suite red rather than assumed to -- dropping the run-URL scoping, collapsing the two delivered=true reasons into one, deleting a marker from the table, and dropping the self-mod marker specifically. That last marker is the one the list most easily omits, and #571's review caught it missing: the self_mod and dispatch-guard skips report a job conclusion of success with every post-guard step skipped, so nothing about them looks like a failure, and their skip notices carry the run URL without matching failure wording. The pre-fix classification was delivered=true reason=no-failure-marker, which is the green-check-no-verdict case the capability exists to close. gemini-code-review.yml's dispatch guard posts a skip notice on fork/Dependabot PRs so delivery classification matches it as well (gha#573). CI runs the suite as the review-delivery job in _selftest.yml, which also calls install-gha-scripts through two real uses: ./... steps: one installing the classifier and running it on a marker body, for the github.action_path-resolution proof the other composite e2e steps give, and one asserting a path-bearing filename is refused. ai-code-review.yml does not call any of this yet -- a new composite cannot gain its first @v2 caller in the PR that introduces it, so the wiring waits on the tag slide and is tracked in gha#569.

.github/workflows/scripts/tests/run-save-pr-diff-tests.sh exercises save-pr-diff.sh offline against stubbed gh outcomes, asserting successful diff saving, empty diff handling, command failure, and partial output cleanup; mutation tests verify rm -f and non-empty -s checks are load-bearing. CI runs it as the save-pr-diff job in _selftest.yml, calling the .github/actions/save-pr-diff composite action through a real uses: step and asserting a non-empty saved diff path (gha#568).

.github/workflows/scripts/tests/run-resolve-major-tag-tests.sh exercises resolve-major-tag.sh (see Layout above) offline against throwaway git repos; .github/workflows/scripts/tests/run-check-tag-drift-tests.sh exercises check-tag-drift.sh (see Layout above) offline against throwaway git repos, asserting zero-drift, N-commit-drift, no-semver-tag, and missing-major-tag behavior; CI runs both in the tag-drift job of _selftest.yml.

.github/workflows/scripts/tests/run-build-reviewer-args-tests.sh exercises build-reviewer-args.sh (see Layout above) offline against a table of reviewer-list inputs, including comma-only, whitespace-padded, and doubled-comma cases; CI runs it as a step in the dependabot-review job. That job also runs build-reviewer-args itself via a real uses: step (the same github.action_path-resolution proof the run-review-guard / extract-total-cost / sum-costs e2e steps above give), asserting it surfaces the correctly trimmed and split JSON array for a real call. Unlike those other e2e steps, this one can't also exercise request-dependabot-review.yml's own reusable-workflow layer end-to-end yet: that workflow calls build-reviewer-args via Morrison-Lab/gha/...@v2, which won't resolve until @v2 is advanced past this capability's merge (the same test-coverage bootstrapping gap the Layout section's _selftest.yml/ local-ref paragraph describes) -- so dependabot-review tests the composite directly, the same "local composite, not the full reusable-workflow chain" precedent coverage below uses for test-coverage.yml (gha#253 review: missing selftest coverage for a new workflow with real side effects, precedented by the sync-pr job's open-sync-pr no-op test).

.github/workflows/scripts/tests/run-select-existing-issue-tests.sh exercises select-existing-issue.sh (see Layout above) offline against a table of (title, open-issues) pairs, including the no-match, prefix-is-not-a-match, case-sensitivity, and already-duplicated cases; run-split-csv-list-tests.sh does the same for split-csv-list.sh, covering the space-after-comma case that motivated it and asserting that a label's internal spaces survive trimming (good first issue is a real label). CI runs both in the failure-issue job of _selftest.yml. That job also calls open-failure-issue itself through a real uses: step with dry-run: true -- the same github.action_path-resolution proof the run-review-guard / build-reviewer-args e2e steps give, and the reason dry-run exists at all: without it the only end-to-end call would file an issue on this repo every selftest run. As with request-dependabot-review, the report-failure.yml reusable-workflow layer above the composite is not covered -- it calls the action via Morrison-Lab/gha/...@v2, which does not resolve until @v2 is advanced past this capability's merge -- so the job tests the composite directly, the same "local composite, not the full reusable-workflow chain" precedent coverage and dependabot-review use.

.github/workflows/scripts/tests/test-description-version.R exercises description-version.R's read_version/versions_equal/bump_dev_version functions offline against 15 cases, including the 3-vs-4-component version boundary (a freshly-cut release starts its next dev cycle at .9000 rather than bumping a 4th component that doesn't exist yet) and the .9999 -> .10000 carry, mutation-verified against a deliberately broken bump rule so the suite is confirmed to actually catch a regression rather than pass vacuously. CI runs it, plus real uses: calls to both bump-dev-version and check-dev-version with dry-run: true, as the dev-version job in _selftest.yml -- the same github.action_path-resolution proof the other composite e2e steps above give, and asserting check-dev-version's outcome differs between a matching and a mismatched DESCRIPTION pair (the check-equation-renders precedent for a pass/fail composite). Neither reusable workflow's own @v2 composite reference resolves pre-merge (the bootstrapping gap dependabot-review/open-failure-issue above hit too), and bump-dev-version.yml's real run is a write side effect selftest can't exercise, matching sync-pr's own no-op-only precedent -- so this job covers the composites directly rather than the reusable workflows as a whole, which is exactly the gap gha#390's own review found in version-check.yml (a checkout resolving against the calling repo rather than gha's own; see Layout above).

.github/actions/inject-canonical-urls/tests/test_inject_canonical_urls.py covers the canonical injector (see Layout above) offline, generating its HTML fixtures into tmp_path rather than committing them -- committed HTML under a tests directory gets swept into the bib and phi jobs' repo-wide scans, the same trap the test-coverage R-package fixture records below. CI runs it as the canonical-urls job in _selftest.yml, kept separate from altdoc-docs so a failure is attributable at a glance. That job also calls the composite through two real uses: ./... steps -- one release-style, one preview-style -- the github.action_path-resolution proof run-review-guard's own e2e steps give, and the one thing the unit tests cannot reach, since they call the script directly and so cannot see whether action.yml wires its inputs through at all. Six mutations were confirmed to turn the suite red, and the first is the one to keep if it is ever trimmed: making the canonical always point at /latest-tag/ regardless of whether the page exists there. That is the issue's own open decision point, and the failure it produces is silent -- a canonical to a 404 looks fine in the generated HTML and is only wrong at the indexer. The others cover dropping the 404.html exclusion, dropping the already-tagged skip, canonicalizing a preview instead of marking it noindex, dropping the base-URL trailing-slash normalization, and dropping the https:// guard.

The altdoc-docs job in _selftest.yml exercises generate-altdoc-version-dropdown, generate-altdoc-landing-page, and resolve-altdoc-base-url (see Layout above) directly against a throwaway fixture: a separate git-init'd package directory (not this checkout) with two release tags, asserting the composite picks the correct latest/previous tags and dev version and rewrites the navbar "Versions" block and root-redirect HTML correctly.

It calls generate-altdoc-version-dropdown four times, over three fixtures. Twice over the one above: first with no current-version, covering the inference path and the dev-build labeling, then with current-version: v0.1.0, covering an explicit release build. That second call also proves two things only a re-run can: that the generated-by marker keeps the block findable after the first run replaced its - text: Versions anchor, and that the navbar badge is replaced rather than stacked.

The third call uses a clone of that fixture, whose own DESCRIPTION is bumped to 0.2.0.9000 while origin/main still holds 0.1.0.9000. That is the only way to pin the PR-preview divergence described in the Layout section above -- the menu's label and badge naming the branch's version while the /dev/ entry names the default branch's. A unit test cannot reach it: resolve_current_version() takes one version, and the split between local_version and dev_version lives in generate_version_dropdown.py itself (gha#308 review). navbar_version.py's own pytest suite (generate-altdoc-version-dropdown/tests/test_navbar_version.py) covers the label resolution and YAML rewriting offline; the job runs it alongside generate-altdoc-landing-page's.

A fourth call, over a fresh clone of the same fixture, pins the release-tags input (gha#287): passed v9.9.9 (a tag the fixture repo was never actually tagged with, alongside its real v0.1.0), the composite must use that list as-is rather than falling back to its own gh api/git tag discovery, so latest-tag comes back v9.9.9. This is what proves the release-tags passthrough is load-bearing rather than a no-op default: every other call in this job leaves release-tags empty and no github-token, so it exercises the composite's own git tag fallback instead -- a different mechanism from altdoc-multiversion-docs.yml's "Determine latest stable release tag" step, which only ever calls gh api with no git tag fallback of its own. On the production path (the real workflow calling the real composite) only the workflow step's gh api call ever runs; the composite's own discovery, in either form, never fires there once release-tags is set.

The same job also covers the legacy-paths 404 redirect at three levels: generate-altdoc-landing-page/tests/test_legacy_redirects.py (pytest, the old=new parsing and its fail-fast validation), a real uses: call to the composite with legacy-paths set, and generate-altdoc-landing-page/tests/run-redirect-js-tests.mjs, which executes the generated page's redirect script under node with a stubbed window against a table of request paths. That last one exists because the Python tests can only assert the mapping reaches the page as text -- whether a given request then lands in the right place is a separate question, and getting it wrong is silent (a bad redirect still renders a plausible not-found page). Its table includes the paths that must not redirect: a genuinely missing page under /dev/ would otherwise bounce forever.

It does not exercise the full altdoc-multiversion-docs.yml reusable workflow end-to-end (the render + multi-target gh-pages deploy is a real write side effect selftest can't run); that layer is validated by real consumer usage once merged and released, the same precedent dependabot-review's reusable-workflow layer follows just above.

GitHub access in remote / web sessions

Claude Code on the web (and other remote/CI sessions) runs in a sandbox where the gh and glab CLIs are not installed and there is no direct GitHub API access. Skills and built-in commands that tell you to "use gh" -- /review,

/code-review --comment, /security-review, /verify, PR babysitting, PR creation -- only work if their GitHub steps are translated to the GitHub MCP tools (mcp__github__*). When a skill or command instructs a gh/glab command in such a session, substitute the equivalent MCP tool below. (In a local session where gh is on PATH, use gh as the skill describes.)

This repo is Morrison-Lab/gha (moved there from d-morrison/gha), so MCP calls use owner: Morrison-Lab, repo: gha.

Use whichever owner the session was scoped with, not whichever one is current. A session's GitHub access is pinned to the repository name it was started with, and the two names are not interchangeable at the tool layer even though they are the same repository:

  • A session scoped to the old d-morrison/gha keeps working, because the API follows the transfer redirect server-side. Passing owner: Morrison-Lab to that session fails with Access denied: repository "morrison-lab/gha" is not configured for this session, and add_repo cannot rescue it -- it refuses the cross-owner add outright.
  • Some endpoints return 301 Moved Permanently to the old name rather than following it, so a call can fail on the redirect alone. If one does, the answer is usually a different route to the same fact, not a different owner string.

So read the allowed-repositories list in the session's own context before assuming an owner, and if the scoped name is the old one, keep using it.

A Cursor Cloud Agent's @claude review / /review comment is not a human collaborator comment. It posts as cursor[bot] with author_association: NONE, so the default OWNER/MEMBER/COLLABORATOR gate skips the run (the workflow may still wake and report every job skipped). This repo's dogfood callers admit cursor[bot] on the caller-side if:; claude.yml's trusted-bot-logins input is the matching reusable-workflow allowlist (default []). Until @v2 carries that input, do not pass it in with: -- an unknown workflow_call input fails the job at the call gate for every mention. Prefer /review from cursor[bot] once claude-review.yml on main admits that login: that path is entirely in the caller and does not wait on a tag slide. A human OWNER/MEMBER/COLLABORATOR /review or @claude review remains the reliable workaround on any older pin.

Some of these sessions have no local git checkout at all (not just a missing gh CLI) -- there is no working tree to run git commit/git push against, so every change (branch, file edit, PR) must go through the MCP write tools below. Editing a file means: mcp__github__get_file_contents first to get its current blob sha (required on every update, not just the first -- re-fetch it after each write since it changes on every commit), then mcp__github__create_or_update_file with the full new file content (it replaces the whole file, there is no patch/diff mode) and that sha. A stale sha (from before another commit landed) fails the write -- re-fetch and retry rather than guessing.

Operation / gh/glab command GitHub MCP equivalent
gh pr list mcp__github__list_pull_requests
gh pr view <n> mcp__github__pull_request_read (method: get)
gh pr diff <n> mcp__github__pull_request_read (method: get_diff)
changed files in a PR mcp__github__pull_request_read (method: get_files)
gh pr status / gh pr checks mcp__github__pull_request_read (method: get_status / get_check_runs)
gh pr create mcp__github__create_pull_request
read PR conversation comments mcp__github__pull_request_read (method: get_comments)
read inline review comments mcp__github__pull_request_read (method: get_review_comments) -- also returns threadIds
post a top-level PR comment mcp__github__add_issue_comment
post inline review comments mcp__github__pull_request_review_write (method: create, no event) → mcp__github__add_comment_to_pending_review per comment → mcp__github__pull_request_review_write (method: submit_pending)
reply to a review comment mcp__github__add_reply_to_pull_request_comment
approve / request changes mcp__github__pull_request_review_write (method: create with event)
resolve a review thread mcp__github__pull_request_review_write (method: resolve_thread, threadId: <id from get_review_comments>)
gh issue list / gh issue view <n> mcp__github__list_issues / mcp__github__issue_read
read a file / repo contents mcp__github__get_file_contents
create/edit a file (no local checkout) mcp__github__create_or_update_file -- needs the target branch, full new file content, and the file's current blob sha (from get_file_contents) if it already exists
create a branch (no local checkout) mcp__github__create_branch
CI runs & job logs mcp__github__actions_list, mcp__github__actions_get, mcp__github__get_job_logs
watch / stop watching PR activity mcp__github__subscribe_pr_activity / mcp__github__unsubscribe_pr_activity
glab mr ... (GitLab) N/A -- this repo is on GitHub; use the tools above

Posting inline comments requires a pending review to already exist before mcp__github__add_comment_to_pending_review; create the pending review first, add each comment, then submit once at the end. Watch and respond to PR activity with mcp__github__subscribe_pr_activity / mcp__github__unsubscribe_pr_activity (not gh pr checks --watch).

Reading repos outside the session's MCP scope

A task often needs files from a sibling repo (e.g. d-morrison/qwt) that the session's GitHub MCP tools aren't scoped to -- those calls fail with Access denied: repository … is not configured for this session. Don't report the repo as inaccessible from that alone. First try the raw HTTP URL directly: any public repo's files are fetchable with curl (or WebFetch) at https://raw.githubusercontent.com/<owner>/<repo>/<branch>/<path>, which works even when gh and the MCP tools don't. (This is how qwt's standalone workflows were obtained to port them faithfully into the reusable workflows for #44/#45.) Only fall back to "can't access it" -- or to whatever session tooling can add a repo to scope, if any -- after the raw fetch also fails (private repo, or the network policy blocks the host).

A 403 from a rendered docs site is not the same as the content being inaccessible. A GitHub Pages / Quarto-rendered site (e.g. ucd-serg.github.io/lab-manual/coding-style.html) can reject WebFetch (for reasons unclear -- possibly anti-scraping) even though the source file it was built from is a plain file in a public repo. Don't conclude the content is unreachable -- find the source path (often the same repo, e.g. coding-style.qmd for coding-style.html, sometimes with _-prefixed included fragments) and raw-fetch that instead using the same <path>-includes-its-extension template above, e.g. https://raw.githubusercontent.com/<owner>/<repo>/<branch>/coding-style.qmd. (Confirmed this way that ettbc's .lintr.R predates UCD-SERG/lab-manual's move to a shared lms linter package (source: UCD-SERG/lab-manual/.lintr.R, which calls lms::default_linters() from a package defined in that repo's own lms/ subdirectory) -- the manual's own docs page 403'd, but its .qmd source and the referenced .lintr.R file both fetched cleanly.)

A canceled review skips require-review and require-clean-verdict gracefully (gha#585, gha#767)

claude-code-review.yml's model path is concurrency-grouped per PR (claude-review-<PR>, cancel-in-progress: true) on preempt-previous and claude-review, across BOTH the automatic pull_request-triggered review and claude.yml's comment-triggered (@claude review) re-dispatch. Stash/restore (gather-context / post-review) use a separate non-canceling group so a cancelled run's restore cannot cancel the successor's model job. preempt-previous is an echo in the canceling group; it does not wait for the predecessor's post-review. When a push and an @claude review comment land close together -- or claude.yml's agent run finishes and re-dispatches a review a minute or two later, landing on top of the next push's auto-review -- the two reviews race and one cancels the other.

The require-review and require-clean-verdict gate jobs treat a cancelled run as a graceful skip rather than an outright failure (gha#585, gha#767), allowing surviving and subsequent reviews to proceed cleanly without leaving a false-negative red check on superseded runs. To avoid causing unnecessary cancellations: don't post @claude review immediately after pushing a commit on a PR using this workflow; let the automatic review run alone, or wait for any in-flight dispatched review to finish first. (See the claude-review job's concurrency: comment in .github/workflows/claude-code-review.yml for the full mechanism.)

The same race fires from two plain pushes close together, not just a push plus an @claude review comment. Pushing two commits back-to-back (e.g. a code fix, then a small follow-up doc/memory commit) triggers two separate pull_request-type review runs; the second cancels the first via the same concurrency group. Don't chase this either -- and don't bother "fixing" the

workflow to prevent it: cancel-in-progress on a stale commit's review is the correct, intended behavior (only the latest commit's review matters, and canceling a stale run saves CI time), not a defect. A debounce to coalesce rapid pushes would trade away review latency on every normal single-push PR just to suppress a cosmetic, self-resolving non-issue on the rare double-push. The fix is behavioral: batch closely-related changes into one commit/push instead of two in quick succession.

gha#342's in-job gate is already at @v2; this PR's leftover is a runner, not a re-dispatch. claude-bot.yml still gates on contains(body, '@claude'), so a comment writing the mention inside backticks -- exactly what explaining any of this requires -- still starts a job: the full agent job on current @v2 pins, and only mention-filter after this PR's tag slides. The in-job stripper already withholds the billed agent run and the review re-dispatch, so quoting no longer cancels an in-flight claude-review the way the pre-#342 world did. Until @v2 slides past this PR, quoting still spends that agent-job runner minute (and the issue/PR concurrency slot). After the slide, only mention-filter starts, and the defang mitigation can be dropped.

Cheap self-check before investigating a post-push require-review/require-clean-verdict/claude-review failure: compare the failing check's commit SHA against the PR's current head SHA (pull_request_read method get, its head.sha field). If they don't match, the event is almost certainly this exact race on a now-superseded commit -- skip straight to "wait for the head commit's review" instead of spending a tool call fetching the workflow run to confirm cancelled vs failure. This shortcut is scoped to the two jobs that actually run under cancel-in-progress: true (claude-review/preview) -- _selftest.yml's jobs (check-links, phi-tests, bib, coverage, review-fail-check, etc.) have no concurrency: block, so a failure there on a non-head SHA is a real result the reader hasn't re-checked yet, not this race; don't apply the shortcut outside claude-review/require-review/require-clean-verdict/preview. (Lacaedemon/sparta PR #780, 2026-07-12: two pushes 3 minutes apart triggered exactly this on require-review; confirmed via actions_get get_workflow_run that the failing check's conclusion was cancelled on the non-head SHA, matching this pattern.)

require-clean-verdict: Opt-in server-side verdict gate (gha#767)

claude-code-review.yml provides two sibling gate jobs:

  • require-review: Asserts review delivery (the review ran and posted). It goes green on any completed, posted review (even one with findings) and fails red only on review crashes/errors.

  • require-clean-verdict: Asserts an affirmatively clean verdict ("Ready for merge", "Clean", "Approved"). It goes green when the review completed, was posted for the current PR head, and produced a clean verdict without unaddressed blocking findings; it goes red on "Needs more work", "Changes requested", "Blocked", or review errors.

Both gate jobs skip gray on the exact same graceful-skip paths:

  • claude-review was skipped (draft PR, fork, bot author, or dispatch guard blocked);
  • claude-review was cancelled by a newer run (cancel-in-progress);
  • Account API quota was exhausted mid-run (quota_exhausted=true);
  • Default-branch workflow restore failed (self_mod=true);
  • post-review reported the review stale because PR head moved (stale=true).

Consumers requiring server-side merge blocking can add review / require-clean-verdict to branch protection / repository ruleset required checks.

Green require-review is not fully clean - read the review comment body

Fully clean (the bar for standing mwc merge and for ARDI/GII session summaries) requires both green CI and a substantive clean review on the PR's current head SHA - see Morrison-Lab/ai-config's shared/workflow/fully-clean.md. Green review / claude-review plus green review / require-review satisfies criterion 1's review job half only. It is not sufficient for criterion 2.

Before declaring a PR clean, zero findings, or ready to merge, fetch and read the latest @claude review comment on the thread and confirm all of the following:

  1. The comment's created_at brackets inside the review job that ran on the current head commit. Use created_at, not updated_at. Later rounds fold earlier verdict comments and advance updated_at without editing their bodies; see review-verdict-pitfalls.md).

  2. The body contains a real ### Verdict (or Verdict:) line naming approval. It must not be a stub, a quota skip, or a self-mod skip.

  3. The verdict is not a deferral or refusal. Any of these mean the PR was not reviewed and is not clean, even when both review checks are green:

    • Deferred - author requested reviewers hold off
    • honoring that request and stopping here without conducting
    • without conducting the review
    • No verdict section at all (stub review - gha#185)
  4. Read to the end of the comment and count findings under every heading. Criterion 2's test is the absence of findings, not the presence of a positive verdict line - a "Ready" verdict above a findings list loses to the findings. Zero inline review threads is not a substitute. Deferrals and stubs often post as top-level comments only.

Session-lock claim comments are not a reason to skip review or call a PR clean. Comments such as Driving this PR to clean - back off until done or paws off until I'm done (from the claim-pr / ardi rituals) tell other write sessions not to push in parallel. They do not instruct automated review to stand down. They do not mean a review already ran. An agent that posted such a claim and then saw green review checks must still read the review body. Mistaking the claim for "don't review" produced gha#527: PR 527 was declared clean with zero findings while the bot posted only Deferred - author requested reviewers hold off and never reviewed the diff. PR #528 hardens the reviewer prompt and fails that deferral pattern in check-review-execution.sh. This paragraph is the matching guardrail for agent sessions driving ARDI/GII loops in this repo.

Cheap self-check before marking ✅ Clean in a GII summary or invoking mwc:

HEAD=$(gh pr view <N> --json headRefOid --jq .headRefOid)
REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner)
REVIEW_OK=$(gh api "repos/$REPO/commits/$HEAD/check-runs" --paginate \
  --jq '[.check_runs[] | select(.name == "review / claude-review" and .conclusion == "success")] | length')
gh pr view <N> --json comments --jq --arg head "$HEAD" --argjson reviewOk "$REVIEW_OK" '
  if ($reviewOk | tonumber) == 0 then empty
  else
    [.comments[]
     | select(.author.login | test("claude|github-actions"; "i"))
     | select(.body | test("### Verdict|Verdict:"))
     | select(.body | test("Deferred.*hold off|without conducting the review"; "i") | not)
    ] | last | {createdAt, bodyPreview: (.body[:200]), head: $head}
  end'

REVIEW_OK is zero when no successful review / claude-review check ran on the current head commit, so an empty result means not clean even if an older verdict comment exists on the thread. An empty last or a body matching the deferral patterns above also means not clean. Re-trigger review (@claude review after the in-flight run finishes, or a no-op push) and read the new comment before merging or advancing the loop.

Updating a Dependabot PR's branch is what makes it reviewable

The bot-exclusion gate above keys on github.event.sender.type != 'Bot' (.github/workflows/claude-code-review.yml lines 226, 253, and 512 as of 2026-09-07). sender is the actor whose action triggered the pull_request event, not the PR's author -- so a synchronize event fired by a human's API call carries a human sender even on a PR GitHub still lists as authored by app/dependabot. That distinction is what makes the update-branch trick work: it does not change who authored the PR, only who triggered the next synchronize event.

Measured on Morrison-Lab/gha#838, 2026-09-07: on head ca6ce946, review / claude-review was skipped under the gate above, so no verdict existed and scripts/check-pr-fully-clean.py reported the PR not fully clean ("No automated review comments or reviews found"). Grep for the right string when reading a log: the check-runs API reports conclusion: "skipped", while gh pr checks renders that same state as skipping. Running:

gh api -X PUT repos/Morrison-Lab/gha/pulls/838/update-branch \
  -f expected_head_sha=ca6ce946...

produced merge commit e16986e3, whose author.login is d-morrison (committer web-flow). On that new head, review / claude-review ran for real and produced a clean verdict, and check-pr-fully-clean.py exited 0. The PR merged as d5a7abba.

This is what lets a Dependabot PR clear the mwc Scope Limit's requirement for a clean automated review on the current head -- without it, that requirement's letter forbids merging any Dependabot PR, since the bot sender never clears the gate above.

  • Do: update a genuinely-behind Dependabot branch (update-branch) when its head has never carried a real review, and re-check the new head for a clean verdict before merging.

  • Do: attribute the un-skip to the sender field on the triggering event, not to any change in the PR's own authorship.

  • Don't: treat this as a way to force review on an arbitrary bot PR whose branch is already current -- update-branch only helps when the branch is genuinely behind and an update is warranted anyway.

  • Don't: assume the PR's author.login changed; the merge commit's author.login reads d-morrison only because a human triggered the update, and Dependabot still owns the PR itself.

A verification instrument that a duplicate key silently passes is not verifying anything

While fixing gha#823, a sweep of examples/*.yml stubs checked whether each stub's commented with: block would uncomment into valid YAML, using yaml.safe_load plus isinstance(job.get('with'), dict) and len(job['with']) >= 1. That reported exactly one broken file.

It was wrong: PyYAML's non-strict loader silently keeps the last of a pair of duplicate mapping keys, so a stub whose uncommented block declares with: (or an inner key) twice still parses to a valid one-key mapping and passes the check. Re-running with a duplicate-key-rejecting loader found five broken files: claude-code-review.yml, cursor-code-review.yml, gemini-code-review.yml, opencode-code-review.yml, and small-model-agent.yml (caught by review on PR #841, 2026-09-07).

The general lesson: when an instrument's pass condition is satisfiable by the exact defect it exists to detect, a green result from it is not evidence. Here the defect (a duplicate key) produces a valid one-key mapping, which is exactly how "OK" was defined, so the check could not distinguish a healthy stub from a broken one. GitHub Actions and PyYAML both resolve a duplicate key by taking the last occurrence silently, so the consequence in production is quiet input loss rather than a parse error -- nothing red anywhere.

  • Do: ask what a check's pass condition actually asserts, and whether the target defect can produce that same condition by accident.

  • Do: use a duplicate-key-rejecting YAML loader (or an equivalent stricter parse) whenever validity is being inferred from safe_load succeeding on hand-authored or uncommented YAML.

  • Don't: trust a verification script's "N broken files" count without checking whether its pass condition and the defect it is hunting can coincide.

  • Don't: assume yaml.safe_load rejects duplicate keys -- it does not; it silently keeps the last one.

Check a claim against the artifact it is about, not an adjacent one

Three review findings landed in one session (2026-09-07, PRs #841 and #842), and all three have the same shape: an assertion about an artifact that was never checked against the artifact itself. Each time the author had reasoned thoroughly about something adjacent, and each time the reviewer found the gap by fetching the live object instead. This is the gha-specific instance of Morrison-Lab/ai-config's shared/workflow/verify-the-right-artifact.md.

  1. A YAML sweep's own pass condition, not the YAML. The section directly above this one is the first case: a duplicate-key check whose success condition (yaml.safe_load plus a one-key mapping) was satisfiable by the exact defect it existed to detect, so the reported count (one broken stub) was wrong; the real count is five (gha#839, caught in PR #841 round 1).

  2. A code comment's claim about an upstream pipeline, not the pipeline. classify-review-verdict.sh protected an intra-word underscore with a \x00 sentinel and a comment asserting "NUL is stripped from the review text by the time it reaches here." That was never traced and is false: check-review-execution.sh extracts the review body with jq -r, which passes a literal NUL byte straight through, and Python's errors="replace" does not touch it either, since NUL is a valid single-byte UTF-8 codepoint rather than an invalid sequence. A real NUL between two words of a genuine rejection collided with the sentinel and scored the review clean (gha#842 round 2, fixed by reading the file through .replace("\x00", " ") instead of assuming the precondition).

  3. A PR's claim about a linked issue's current state, not the issue. Issue #839 was retitled from "one file" to "five stubs" and carried a correction comment within the hour, but its body was left arguing the original single-file narrative for close to three hours afterward. PR #841's own body then asserted #839 "was corrected" while that body mismatch was still live. The reviewer fetched #839's body directly rather than trusting the PR's account of it (round 3), and the body was edited to match minutes later.

The common remedy is not "be more careful" --- each case involved a thorough check, just of the wrong object. It is: when a claim is about a specific artifact (a parsed file, an upstream script's actual behavior, an issue's live body), fetch that artifact and compare the claim to it, rather than to a description of it, a summary of it, or an assumption about how its neighbor behaves.

Mechanization status differs across the three, and that is worth recording rather than glossing over. Case 1 is lexically decidable (a non-strict YAML load paired with a bare key-count assertion), and Morrison-Lab/ai-config#3344 tracks a hook for it. Case 3's shape (a gh issue edit --title with no accompanying body edit, or a PR body asserting another issue "was corrected") is a narrower lexical pattern with no confirmed tracking issue or hook as of this writing; check Morrison-Lab/ai-config's hooks and open issues before filing a duplicate, and file one if none exists. Case 2 was judged not mechanizable: it is a domain-correctness claim embedded in a code comment, with no decidable lexical signal separating a checked claim from an unchecked one. Saying so explicitly is part of the record, not a gap in it --- not every instance of this failure class reduces to a hook.

  • Do: fetch the parsed structure, the upstream script, or the live issue/PR body a claim is about, before writing the claim down.

  • Do: name, when a fix is not mechanizable, what makes it so (no lexical signal, judgment-dependent) rather than leaving it unaddressed with no note.

  • Don't: infer an issue's current body from its title, a comment on it, or another PR's description of it --- fetch the body.

  • Don't: let a validator's own success condition go unexamined merely because it is "just" the check, rather than the code under review.

A PR fixing claude-code-review.yml (or claude.yml) itself can't self-verify before merge

This repo's own dogfood workflow (.github/workflows/claude-review.yml) calls Morrison-Lab/gha/.github/workflows/claude-code-review.yml@v2 -- the released, floating tag, not a local ./ ref (unlike _selftest.yml's handling of brand-new pre-release capabilities; see "About this repo" above). .github/workflows/claude-bot.yml similarly calls Morrison-Lab/gha/.github/workflows/claude.yml@v2. @v2 only advances to include a fix once that fix's PR merges to main and slide-major-tag.yml runs.

So a PR that fixes a bug in either reusable workflow cannot exercise its own fix via this repo's automatic review or agent dispatch -- every review (or agent re-dispatch) of that PR runs the pre-fix version, and will keep hitting the exact bug being fixed until after merge. Seeing review / claude-review or review / require-review fail on such a PR with the bug's own signature is expected, not a regression in the diff; don't debug the new code as the cause. The current workaround is to re-trigger the review (push, or @claude review -- see the race-avoidance note above) as many times as needed, or just proceed to merge on the strength of a manual/offline review once CI's other jobs and a careful read of the diff are clean. (Hit on gha#201, whose diff fixed claude-code-review.yml's stub-review bug directly: every review of the PR itself failed with that exact signature -- is_error:false, a low permission_denials_count, no verdict -- right up until merge. gha#202 (a different fix, allowlisting WebFetch/Bash(curl:*)) hit the identical signature as a bystander while it still edited claude-code-review.yml's inline claude_args block directly, before a rebase onto #201 relocated that edit into the new run-claude-review-attempt composite action -- confirmed via that run's own execution output: permission_denials_count:1, no verdict. Once @v2 picked up #201's fix, both PRs' subsequent reviews went clean.

The claude.yml side of this hit on gha#286, fixing gha#285's gh workflow run-without---ref bug: a plain @claude review comment on PR #286 dispatched through claude-bot.yml's claude.yml@v2 -- the released, pre-fix tag -- and reproduced the exact #285 symptom live (the re-dispatched review's check-run landed on main's SHA, not the PR's head) even though the fix had already been pushed to the PR branch itself. Not a regression; the same "can't self-verify" gap, one layer up.)

Re-running failed jobs cannot verify a tag slide

The section above ends at the merge; this one covers the step after it. Once slide-major-tag.yml moves v2, the obvious way to confirm the fix reached consumers is to re-run the run that failed. Which re-run you pick decides whether that works, and the wrong one fails in a way that looks like the fix itself is broken.

GitHub resolves a uses: reusable-workflow reference when the run is first created and records the resolved commit in the run's referenced_workflows. Whether a re-run reuses that record depends on the mode, per GitHub's own docs ("Behavior of reusable workflows when re-running jobs"):

  • Re-running all jobs in a workflow will use the reusable workflow from the specified reference.
  • Re-running failed jobs or a specific job in a workflow will use the reusable workflow from the same commit SHA of the first attempt.

So Re-run failed jobs, Re-run this job, and their programmatic equivalents all replay the pre-slide workflow, however long ago the tag moved. Re-run all jobs re-resolves the reference and does pick the slide up. The failed-jobs call is spelled differently on every surface, so none of these is the name for it: REST is POST /repos/{owner}/{repo}/actions/runs/{run_id}/rerun-failed-jobs, the CLI is gh run rerun --failed, and the GitHub MCP server's actions_run_trigger takes method: rerun_failed_jobs. Note the docs' own precondition: this only applies where the reference is a tag or branch rather than a SHA, which is exactly how consumers pin @v2.

What makes the stale mode deceptive is that the two layers behave differently within it. Composite actions nested inside the reusable workflow (uses: Morrison-Lab/gha/.github/actions/...@v2) resolve at job-preparation time, so those do come back at the new tag even on a failed-jobs re-run. That mixes new-version composites with the old reusable workflow's logic, inputs, and defaults -- which reads as "the fix is live and didn't work" rather than "the fix isn't live". It also means such a re-run can surface a slide whose only substantive change lives in a composite, so what a failed-jobs re-run cannot verify is specifically a change to the reusable workflow's own content.

Decide it mechanically instead of inferring it from step output: read referenced_workflows[].sha on the run (actions_get get_workflow_run) and compare it against the tag's current commit.

Read that tag's commit with git ls-remote, and do not try to read it off a plain git fetch --tags. Sliding a major tag force-moves it, and a plain git fetch --tags will not move an existing local tag, so the one operation this section exists to verify is the one it cannot perform.

git does say so. Measured on git 2.43.0 against this repo's own slid v2, a plain git fetch origin --tags printed ! [rejected] v2 -> v2 (would clobber existing tag) on stderr and exited 1.

Both signals are easy to lose, though, and the habits that lose them are ordinary ones. -q suppresses the message. Piping the combined output through tail can drop it as well, since git prints ref-update lines in refname order and a slide follows cutting a vX.Y.Z that sorts after v2. And a pipeline reports tail's exit status rather than the fetch's, so && guards nothing once a pipe is involved. Measured on a throwaway remote carrying a rejected v2 beside a newly created v2.6.0:

$ git fetch origin --tags 2>&1 | tail -1 && git rev-parse 'v2^{}'
 * [new tag]         v2.6.0     -> v2.6.0
3dbbb1e95f5026c06799e548b05b820df0573581     # stale, and the chain did not stop

So do not reason about which invocation preserves the warning. Read the tag directly, or force the local update:

# Ask for both refspecs and take the ^{} line when there is one: an ANNOTATED
# tag's bare line is the tag object's sha rather than the commit. See
# ai-config's memories/git-tags.md. slide-major-tag.yml writes lightweight
# tags, so both forms agree for v1/v2 today.
git ls-remote origin 'refs/tags/v2' 'refs/tags/v2^{}'
git fetch origin --tags --force        # when the local checkout must be updated

Comparing two API-derived values sidesteps the question entirely, since referenced_workflows[].sha has no local-staleness failure mode. (Measured 2026-08-19, minutes after #521 merged and v2 slid to 3ec11c0: git rev-parse 'v2^{}' after a -q fetch returned the previous 3b09703, and that stale read was reported twice as "the fix has not reached consumers" while d-morrison/rme run 32298939967 was demonstrating the fix working -- that run's referenced_workflows[].sha reads 3ec11c0, its claude-review job's conclusion is success, and its Run Claude Code Review step's conclusion is success over an inner step that exited 1, which is #521's continue-on-error. Morrison-Lab/ai-config's memories/git-tags.md records the git behavior itself, and gha#522 is the pointer from here.) To verify a slide, prefer a fresh run -- push a commit, open a PR, or workflow_dispatch -- since it resolves the tag unambiguously and needs no reasoning about which re-run mode you are in. "Re-run all jobs" works too; "Re-run failed jobs" never does.

(UCD-SERG/serodynamics run 30471653690, 2026-07-29: after v2 was slid to c50e847 to pick up #359's ai-config@Morrison-Lab retarget, a failed-jobs re-run failed with the identical Failed to install plugin 'ai-config@d-morrison'. The job log showed both layers at once -- INPUT_PLUGINS: ai-config@d-morrison from the old reusable workflow, alongside a detect-review-request: match=false line that only exists at c50e847 -- and referenced_workflows still read sha: 6ee996b on attempt 2. A dispatched run on a PR branch confirmed the fix immediately.)

A push touching .github/workflows/ can fail even though WORKFLOW_TOKEN is wired correctly in code

If a push from claude.yml (or claude-bot.yml's dispatch of it) fails with:

! [remote rejected] ... (refusing to allow a GitHub App to create or
  update workflow `.github/workflows/<file>` without `workflows` permission)

this means the WORKFLOW_TOKEN repository secret is unset or lacks contents:write + workflows:write scope for this repo -- it is not a bug in claude.yml's own token-resolution code. claude.yml already resolves PUSH_TOKEN as ${{ secrets.WORKFLOW_TOKEN || secrets.GITHUB_TOKEN }}, and claude-bot.yml already passes WORKFLOW_TOKEN through; GITHUB_TOKEN alone can never push a .github/workflows/ change, so the fallback reproduces this exact rejection whenever WORKFLOW_TOKEN isn't configured with the right scope. Only someone with admin access to this repo's Settings -> Secrets and variables -> Actions can fix it (a classic PAT with repo + workflow scopes, or an equivalent GitHub App installation token) -- an @claude session has no path to set repository secrets itself. If you hit this, don't debug claude.yml's PUSH_TOKEN wiring; recover by pushing the already-committed local branch from a differently-credentialed session/human, and flag that WORKFLOW_TOKEN needs to be (re)configured. (Hit twice on 2026-07-24: PR #286 fixing #285, and PR #290 fixing #289, both editing .github/workflows/*.yml -- both required manual recovery; see gha#292.)

Recovery is cheaper than that paragraph implies once @v2 carries gha#360: the run now posts the commits back as a patch. claude.yml captures the push output, hands it to report-push-failure (see Layout above), and comments the explanation plus a git format-patch on the thread, so the work comes back with git am instead of being redone from scratch. Read the comment before re-doing the work; the diagnosis of the cause above is unchanged, and still needs a human with admin access.

The same change closed a second, worse defect. The rejection left the PR head SHA where it started, which is indistinguishable from "Claude committed nothing" -- so the "Post Claude's response if no code was committed" step ran and posted Claude's prose describing the fixes it had just made, onto a branch carrying none of them (gha#360, seen on Morrison-Lab/ai-config#805). That step is now gated on the push not having failed. When reading an older thread, treat a "here is what I did" comment as a claim about the branch to verify rather than as a record of it, per the SHA-comparison rule in Morrison-Lab/ai-config's shared/workflow/ardi.md.

A third, more direct mechanism used to produce the identical symptom without @v2 even entering the picture. claude-code-review.yml used to skip the review whenever the PR edited the caller workflow (and, on dispatch, any top-level .github/workflows/*.yml -- gha#386). review / claude-review reported success with every post-guard step skipped, and no verdict was produced. Since gha#440 that skip is a self_mod job output and require-review is gray rather than green. gha#598 keeps the gray skip only when restoring default-branch workflow files fails. A successful restore continues into the review. The skip is this workflow's own policy rather than an action 401: gha#580 forwards github_token, which skips the App-token exchange that used to fail workflow-content validation until merge. A green claude-review check is still easy to mistake for a real review.

Reviews of workflow-editing PRs restore the default-branch .github/workflows/ tree after checkout (gha#598). detect-pr-workflow-edits classifies top-level workflow YAML (not workflows/scripts/, not composite action.yml). restore-default-branch-workflows then rm -rfs .github/workflows/ and checks it out from origin/<default-branch>, so a workflow the PR added is deleted rather than left on disk. run-claude-review-attempt passes github_token (GITHUB_TOKEN) on that path so claude-code-action skips its OIDC App-token exchange and the workflow-content check that exchange runs. setupGitHubToken in src/github/token.ts returns early on OVERRIDE_GITHUB_TOKEN and never calls exchangeForAppToken, which is where the skip is thrown (anthropics/claude-code-action, read 2026-08-26). prepare.ts reads that env var only for the write-permission path. The prompt tells the reviewer that on-disk workflow files are the default-branch copies and to take workflow diffs from the saved PR diff. The restore drops .github/workflows/.restored-from-default-branch so workflow-parsing test suites and audits detect the restore and skip rather than measuring the default-branch copy (gha#765).

Dispatched reviews omit --ref when the PR edits workflow YAML, so GitHub executes the default-branch caller rather than the PR head's copy. That is the trusted-YAML half; the restore is the trusted-on-disk half. Fork PRs already omitted --ref (gha#289). A no---ref dispatch's check-runs land on the default branch (gha#285); the review comment still posts on the PR. Before gha#598 the guard instead skipped outright, which is what gha#286 recorded: an @claude review comment produced only a $0.60 cost comment and no verdict, because the PR touched claude-review.yml itself. Check the job's step list rather than its conclusion when reading any run from that era.

This does not switch to pull_request_target. pull_request executes the PR's triggering workflow YAML. pull_request_target executes the base copy and hands the job secrets plus a write token -- a pwn-request if the job then runs code from the PR head. Same-repo pull_request already has secrets. The safe default-branch YAML for dispatch is "omit --ref". For an automatic pull_request review of a PR that edits the caller, GitHub has already selected that caller YAML before any step runs, so the restore cannot un-execute it. That path stays same-repo only (forks are skipped). Do not "fix" it by flipping the trigger to pull_request_target.

self_mod now means the restore failed, not "the PR edits a workflow file". require-review still grays that case out. A later re-run can recover if fetching the default branch was the problem. Until @v2 slides past this merge, consumers (this repo's dogfood caller included) keep the pre-#598 skip: a green or gray require-review on an older pin is that tag lag, not a contradiction.

The action still carries its own workflow-content validation, and it does not print a literal 401. If restore fails, or github_token is not passed, a remaining content mismatch still hits:

Splitting the offending line into a follow-up PR would clear the guard, at the cost of leaving the sweep incomplete and its own docs overclaiming for a release cycle. (gha#329: a timeout-minutes hardening sweep across all 36 workflows touched claude-review.yml for exactly one inserted line, and its review was silently skipped -- caught only by noticing the job finished in 4 seconds. Copilot, requested as a fallback, refused separately for quota, so the PR merged on CI plus a self-review with no external verdict at all.)

gha#580 is why the restore is paired with a token. run-claude-review-attempt forwards github_token (${{ github.token }} from the read-only model job), so setupGitHubToken() returns that token without calling getIDToken() or exchangeForAppToken() (anthropics/claude-code-action v1.0.196 src/github/token.ts, measured 2026-08-26). That exchange is where the workflow-content skip was thrown, so forwarding the job token is what lets a restored checkout review at all.

Exchanging OIDC token for app token...
##[warning]Skipping action due to workflow validation: Workflow validation
failed. The workflow file must exist and have identical content to the
version on the repository's default branch...
Exiting due to workflow validation skip

The action STEP reports outcome=success (it "gracefully skips"), runs only ~4-11s, and writes NO execution output -- so check-review-execution.sh reports Claude review produced no execution output -- treating as a failed review, and claude-review + require-review go RED with no verdict. That is why gha#417 (bypass the skip without a token) was abandoned, and why gha#598 pairs the restore with github_token.

Diagnostic tells for that historical validation skip, so an older run log is not misdiagnosed as a 401:

  • The "Run Claude Code Review" STEP finishes in seconds (~4-11s measured) while writing no execution output; total job time is not a reliable tell, since checkout, submodules, and package installs run first and can dominate.

  • Grepping the log for 401 finds nothing -- the auth failure the parenthetical above calls "401s" does not surface as a literal 401 string; grep for workflow validation / Exiting due to workflow validation skip, or just READ the "Run Claude Code Review" step's own output rather than grepping for a guessed string (per Morrison-Lab/ai-config's shared/principles/fail-fast.md).

  • The validation keys on workflow CONTENT vs. the default branch, independent of trigger type.

The Test changes against a template repo section used to hit that same OIDC content-validation abort; github_token forwarding is what skips it now (gha#580).

claude.yml has four review-dispatch sites, and one is not the composite

Making a consumer's reviewer on-request only is a change to this repo, not to the consumer's caller. Removing a caller-side trigger stops the caller from starting a run; it does nothing about the runs claude.yml starts on its own once the agent job is already going.

At 838011e, .github/workflows/claude.yml dispatches claude-code-review.yml from four places:

line step condition
1223 Dispatch code review steps.review_request.outputs.match == 'true'
1291 Dispatch review for a late @claude review comment steps.late_review_request.outputs.match == 'true'
1308 Dispatch code review after Claude pushed commits head SHA changed
1532 inside Finalize PR for issue trigger none

The first three call the .github/actions/dispatch-review composite. The fourth is an inline gh workflow run "$REVIEW_WF" ... -f pr_number= in the middle of the step that opens or updates the PR for an issue-triggered run, so it is invisible to anyone enumerating uses: .../dispatch-review, and it carries its own copy of the gha#598 --ref logic rather than the composite's. The last two dispatch under no review-request condition at all, so an input gating only the request-matched sites leaves a review running on every push the agent makes and on every issue-triggered PR.

Enumerate the dispatches from the primitive, never from the composite or the step names. grep -n 'gh workflow run' .github/workflows/claude.yml .github/workflows/scripts/dispatch-review.sh reaches all of them in one pass; a grep for dispatch-review returns three, and reading the step names returns that same three by a different route, since each composite call sits under a step named Dispatch ... while the inline site sits under Finalize PR for issue trigger. Both are non-empty and plausible, and both miss the fourth.

  • Do: derive the dispatch set with gh workflow run before proposing any gate over it, and say which sites the gate covers.

  • Don't: count uses: .../dispatch-review occurrences, or steps named like a dispatch, as the population.

(Morrison-Lab/gha#778, 2026-08-31, filed while making UCD-SERG/shigella's reviewer on-request only in shigella#44 / shigella#46. That issue was written saying there are three sites and proposing an input gating one of them, so its stated outcome was not reachable by its own proposed fix. Both undercounts were caught by an adversarial review rather than by the author.)

dispatch-review.sh omits --ref in four cases; its header names three

Any documentation of the dispatch command has to carry those cases, because one of them is a trust boundary rather than a convenience. At 838011e, .github/workflows/scripts/dispatch-review.sh drops --ref when:

  1. PR_BRANCH cannot be resolved;

  2. PR_HEAD_REPO differs from REPO, that is, the PR is from a fork (gha#289);

  3. detect-pr-workflow-edits.sh reports the PR edits top-level .github/workflows/*.yml (gha#598);

  4. list-pr-changed-files.sh cannot produce a complete file set, which sets FORCE_DEFAULT_BRANCH_WORKFLOWS and forces the same omission.

The script's own header comment names only the first three, so case 4 is derivable from the code alone --- read the branches, not the comment.

Case 3 is why the flat form is unsafe to document: with --ref pointing at the PR branch, GitHub executes the PR head's own unreviewed caller YAML under this repository's model credentials, which is exactly what gha#598 exists to prevent. So write the conditional form, or say which case the given form covers, per Morrison-Lab/ai-config's shared/writing/fact-check-prose.md ("A command written into documentation is a condensation of the code that builds it"). Having applied the carve-out in a dispatch you just ran is not the same as having written it down; both acts happened in one session on shigella#46, and only the first one got it right.

A prompt instruction is a request; a permission rule is a constraint

run-claude-review-attempt denies Agent and Task in --disallowedTools and its --append-system-prompt instructs the reviewer to complete the review directly inline, because a background spawn in a headless CI run ends the turn waiting for completion notifications that no later turn will deliver (gha#392). That prompt instruction was originally added after gha#392, but was live, verbatim, when the same failure recurred on Morrison-Lab/ai-config#1744 (run 32347489886): four background agents spawned, turn ended, no verdict, $4.21 (gha#532).

The general point is worth keeping separate from the incident. A prompt tells the model what to do, and the model may not. When a constraint matters, look for a mechanical form of it before concluding that a better-worded prompt is the ceiling.

Claude Code has one here, and it is easy to miss because it is newer than the tool-name and command-prefix rules everyone knows. Per code.claude.com/docs/en/permissions, "Match by input parameter":

Deny and ask rules can match a top-level input parameter on any tool with Tool(param:value).

In gha#532, Agent(run_in_background:true) was deployed as a parameter-scoped deny rule. However, in gha#756, when the model omitted run_in_background in tool calls, it defaulted to true (backgrounding) while bypassing the parameter-scoped rule (measured on Morrison-Lab/ai-config#2808, runs 33526978807 [$3.20] and 33527218652 [$5.75] on 2026-09-01: all 4 Agent spawns omitted run_in_background and stalled without a verdict). Therefore, Agent and Task are denied outright in --disallowedTools. In a headless non-interactive review run, the model must complete the entire review inline in its primary context.

Five things constrain any change here.

It is a DENY on Agent,Task outright. allow rules continue to use each tool's own specifier syntax; an allow on Agent(run_in_background:false) does not work. Denying Agent,Task outright mechanistically blocks both explicit-true and omitted-parameter spawns.

The prompt instruction reinforces the constraint. The prompt instruction explicitly tells the model that subagents are disallowed and to complete the review inline, preventing stalled delegations.

A parameter rule cannot reach a tool's primary content field. The same section lists them (command for Bash, file_path for Read, url for WebFetch, and so on) and says Claude Code ignores such a rule and warns at startup, because Bash(command:rm *) would be bypassable by a compound command. Use the tool's own specifier for those.

The CLI flag parses --disallowedTools tool names directly. On Claude Code 2.1.238, Agent and Task in --disallowedTools are accepted with no startup warnings and prevent subagent invocations. (The negative control on the same version answered Bash(command:rm *) with Permission deny rule "Bash(command:rm *)" targets command as a raw string and will not match.)

The denials these rules produce are excluded from the stub-retry gate, and that exclusion is part of the fix rather than a refinement of it. check-review-execution.sh's threshold treats a high denial count as evidence the reviewer was starved of tools it needed -- its own comment says gha#198's pattern "has repeatedly NOT recovered", which is why crossing the threshold withholds the retry. A denial produced by a rule this repo added on purpose is not evidence for that, and it is not a small distortion at the sizes actually observed: the two incidents motivating these rules were a 4-spawn and an 8-spawn fan-out, so the second alone clears the default threshold of 5 before any genuinely-starved call is counted. Shipping the deny without the exclusion would therefore flip a retryable gha#185 stub into a hard-failed gha#198 classification in precisely the scenario the deny exists to serve. So the gate reads a count with the intended denials removed, while every reporting path keeps the true total -- a PR comment saying the reviewer was denied nothing when it was denied eight times would be false, and the denied-tools summary is what a triager acts on. The subtraction needs the permission_denials array, since that is what names tools; where only the scalar count survives (the gha#531 shape) no subtraction is possible and the gate falls back to the raw count, classifying such a run exactly as it is classified today. That direction is deliberate, since assuming unnamed denials were ours would weaken the gha#198 gate on evidence we do not have.

Never just theorize -- investigate empirically

A hypothesis that is cheap to test must be tested before it is asserted, and certainly before it is acted on or reported to anyone. Naming a plausible cause is the start of the work, not the end of it. The failure mode is not being wrong; it is being wrong and confident, because a stated hypothesis reads to everyone else like a finding.

This matters most when diagnosing CI, where the authoritative answer is almost always one call away and the plausible answer is almost always slightly wrong:

  • Read the failure's own output before theorizing about its cause. A job that fails with no logs still has an error banner on its job page, reachable with WebFetch on the run URL even when the API will not serve it. gha#351/#352: an org-wide job failure was attributed to an Actions spending limit, then -- after that was disproved -- to anything but billing, when the banner said The job was not started because your account is locked due to a billing issue all along.
  • Read the tool's config before modelling its behavior. A guess at lychee's redirect handling was wrong because 301 is in check-links/lychee.default.toml's accept list; two successive guesses at markdownlint's MD013 flagged 269 and then 31 lines against the linter's actual 1, because lint-qmd/.markdownlint.qmd.jsonc sets { line_length: 80, code_blocks: false, tables: false } and markdownlint ignores a line with no space past the limit. A model of a checker is only worth using once it reproduces that checker's known result on a known input.
  • Prefer the run's own artifacts to your inference about them. Which repositories moved in the org transfer was answerable from the lychee run's redirect and error lists -- qwt, rme, and rpt appeared in neither at the time, so they resolved cleanly then -- rather than from reasoning about which ones "probably" moved. (rpt has since moved to Morrison-Lab/rpt; the point here is the method, and the list is a snapshot of what that specific run found.)

A wall of access failures is not evidence that something cannot be investigated. In the same work, get_check_run returned 301, the MCP tools refused the new owner as out of scope, and the agent proxy returned 403 for api.github.com -- three failures in a row, after which a plain public https://github.com/... URL answered the question immediately. Exhausting the authenticated routes is a reason to try an unauthenticated one, not a reason to report the question as unanswerable. The same principle already appears above for reading files out of repositories this session is not scoped to.

Test changes against a template repo before declaring ready to merge

Before declaring a PR ready to merge -- or reporting a clean / ready-for-merge verdict -- for a change in this repo (gha) that touches a GitHub Action or a component action/workflow, test that change against one of the lab's template repos.

Running the unit tests or _selftest.yml in gha alone is not sufficient for such a change, because _selftest.yml exercises local composites and throwaway fixtures rather than the full downstream project structures -- an R package's vignettes, a Quarto site build -- that pin the @v2 reusable workflows. Instead, point a template repo's uses: at the PR's branch or SHA and confirm the workflow succeeds there:

Repointing the template's top-level uses: exercises a change to a reusable workflow's own YAML, but not a change to a composite action (.github/actions/<x>/, where most of this repo's capabilities live). A reusable workflow pins its internal composite calls to a literal @v2 (e.g. claude-code-review.yml has ~10 such uses: .../actions/<x>@v2 sites), and those resolve at job-preparation time from the released tag regardless of the ref the parent workflow file was fetched from (see Re-running failed jobs cannot verify a tag slide). So a template test of a composite-only change passes vacuously against the old released code. To exercise a composite change, also repoint the nested @v2 refs to the PR branch on the branch-pinned reusable workflow, or invoke the composite directly.

A PR fixing a reusable review workflow (claude-code-review.yml or claude.yml) used to abort template tests at the action's OIDC workflow-content check (confirmed empirically 2026-08-05 via a throwaway dispatched review; the failure surfaces as a fast no execution output, not a literal 401). run-claude-review-attempt now forwards github_token (gha#580), which skips that exchange and its content check. What still blocks dogfood review of such a PR in this repo is the caller-side self_mod skip: it fires when the PR edits the caller stub, and on workflow_dispatch when any top-level workflow YAML is in the diff. Fall back to the manual/offline path in A PR fixing claude-code-review.yml (or claude.yml) itself can't self-verify before merge, or give the action a github_token override that skips the OIDC exchange (wired on the gha#598 workflow-fallback path in run-claude-review-attempt).

Code review guidelines

When reviewing a pull request (e.g. via /review, /code-review, or as a Claude PR bot), evaluate the diff against all of the following, in addition to correctness:

1. The SERG lab manual

The UCD-SERG lab manual is the lab's authority on coding conventions. Hold changes to its standards, especially:

  • Coding style -- object naming, line breaks/formatting, function documentation, comments, message/communication style, and Quarto code-reference conventions (backticked pkg::fn(), markdown package links -- no raw HTML in .qmd).
  • Coding practices -- function decomposition and length limits, testing requirements, the QA checklist, documentation, {here} for paths, and tidyverse idioms.
  • Code repositories -- repository organization and version-control practices.

The manual defers to the tidyverse style guide for R; prefer tidyverse idioms and the native |> pipe.

2. d-morrison's review priorities

Above all, code should be highly modular and idiomatic:

  • Modular / decomposed. Favor small, single-purpose functions over long monolithic blocks. Flag duplicated logic (DRY), functions that do too much, deep nesting, and steps that should be extracted and named. In workflows and composite actions, factor shared logic into reusable units rather than copying it between files.
  • Idiomatic. Code should read like the surrounding code and like the ecosystem's conventions -- idiomatic R (tidyverse), idiomatic YAML/GitHub Actions, idiomatic shell. Prefer the standard, well-known way over a clever or bespoke one. Match existing naming, structure, and formatting in the file.
  • Keep these front-of-mind: surface modularity and idiom issues even when the code is otherwise correct.

Be specific and cite the relevant manual section or principle when raising a point. Distinguish blocking issues from optional suggestions.

3. Challenge ambiguous phrasing and terminology

Flag ambiguous terms and phrasing rather than accepting a plausible-sounding reading -- a name that could mean more than one thing, a claim that cites a value or construct without confirming it exists in the actual code. This is a global standing rule from the Morrison-Lab/ai-config corpus. Ambiguity accepted at face value is how a factually wrong claim (e.g. documentation citing a nonexistent enum value) slips through review unchallenged.

4. Fact-check prose against domain knowledge and external sources

When a diff touches prose (README.md, CHANGELOG.md, website/, action descriptions), assess the accuracy and clarity of its claims -- check each against domain knowledge and, where checkable, an external source (the referenced tool's own docs, a linked spec) -- and check any document-internal reasoning the prose makes (e.g. a justification for why a workflow does something a particular way). Also check that every factual claim is defended, separately from whether it's accurate: it needs either reasoning in the surrounding text or a citation, and a bare assertion with neither is a finding even when it turns out to be true. State which claims are inaccurate or undefended, cite the specific source checked for each judgment, and proactively suggest additional citations where they'd help. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/writing/fact-check-prose.md).

5. Check for AI-generated prose tells

When a diff touches prose (README.md, CHANGELOG.md, website/, action descriptions), scan it for the telltale signs of AI/LLM authorship -- overused vocabulary (delve, leverage, robust, seamless, tapestry, testament…), the "it's not just X, it's Y" antithesis, mechanical rule-of-three lists, hedging stacks, signposting filler, em-dash overuse, bold-leading bullets, emoji headers, and promotional register. Flag each tell found with its location and a de-slopped suggested revision -- weigh clustering, not an isolated instance. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/writing/ai-tells.md).

6. Hyperlink technical terms/results; no forward references

When a diff touches prose that defines technical terms or named results via Quarto's theorem-like crossref divs (::: {#def-...}, {#thm-...}, {#lem-...}, {#cor-...}, {#prp-...}, {#cnj-...}, {#exm-...}, {#exr-...}), check that every mention of a term or result links to the div that defines it, and that the div appears before its first mention in reading order -- a link to a definition the reader hasn't reached yet is a forward reference. This scope is per rendered file: cross-chapter ordering in a multi-file Quarto book is out of scope, check it manually. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/writing/definition-crossrefs.md).

The same problem also shows up as plain-text signposting -- "as discussed below", "in the following section", "we'll cover this later" -- pointing at content the reader hasn't reached yet, in any prose, not just documents with crossref divs. Flag these too: confirm each hit is a genuine reference (not an idiom like "values below the threshold") and that the target really comes later, then suggest reordering the content earlier or rewording the pointer into a working link. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/writing/forward-references.md, added in ai-config#507), with a dedicated fix-forward-references (ffr) skill there that applies the fix directly rather than only flagging it in review.

7. Suggest semantic line breaks in prose

When a diff touches prose (README.md, CHANGELOG.md, website/, action descriptions), check that lines break at clause/sentence boundaries (roughly 60-80 characters) instead of reflowing into long unbroken lines -- a semantic break keeps a diff scoped to the changed sentence. Raise violations as a suggestion, not a blocking requirement, and don't re-raise it if the author declines. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/writing/semantic-line-breaks.md).

8. Check code and math for strategic and tactical correctness

Beyond style, check whether the diff's code -- and any math or statistics embedded in it -- is right, not just correctly styled:

  • Strategic correctness. Is this the right algorithm or design for the problem? A clean implementation of the wrong approach (wrong data structure for the scale, a statistical method whose assumptions don't hold for this data, a concurrency strategy prone to races) is still wrong.
  • Tactical correctness. Given the chosen approach, does the code correctly execute it -- no off-by-one errors, sign errors, wrong comparison operators, mis-transcribed formulas, unit/dimension mismatches, or numerical instability.
  • Math/stats in code. Verify a formula, statistical test, or model against its source (a paper, a spec, a package's reference implementation) with the same rigor that item 4 applies to a derivation in prose.

Distinguish a strategic finding (needs a different approach) from a tactical one (needs a correction within the existing approach) -- the fix differs. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/coding/fact-check-code-logic.md, added in ai-config#455).

9. Challenge unnecessary complexity

When reviewing prose, math, or code, check whether it is more complex than the problem requires -- not just whether it's correct or clear. Flag

needlessly convoluted control flow, abstraction layers that add indirection without earning it, an overcomplicated derivation or an unnecessarily general result when a simpler equivalent exists, and prose that restates a point through more clauses or jargon than a plain rewrite needs. For each finding, propose the concrete simplification rather than just naming the complexity, and confirm it doesn't drop a feature, an edge case, or a meaning the original carried. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/workflow/challenge-unnecessary-complexity.md).

10. Question redundant content

When a diff touches prose, math, or code, check for content that could be consolidated without losing completeness or generality -- a claim or explanation restated in two places, a formula re-derived as a special case the general form already covers, duplicated logic across functions/files. Flag it only when nothing would be lost by merging; genuinely distinct content that merely looks similar should stay separate. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/workflow/challenge-redundant-content.md).

11. Write and recommend tidy, concise code

Beyond style-guide compliance, check whether the diff's code is genuinely tidy -- no leftover debug output, no dead branches, no function doing three unrelated things at once. In R code specifically, flag verbose base R or {rlang} constructs where a concise tidyverse equivalent (dplyr, purrr, the {{ }} embrace) does the same job more clearly, unless the tidyverse form would pull in a heavy dependency for a one-liner, the surrounding file is consistently base-R, or a hot loop needs base R's performance. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/coding/tidy-code.md).

12. Reuse function documentation and argument lists

Flag R code that copy-pastes a @param description or a prose section between roxygen blocks instead of using roxygen2's tag-reuse tags (@inheritParams, @inheritDotParams, @inheritSection) -- reused docs stay in sync when the source function's docs change; copy-pasted docs silently drift. Also flag a wrapper function that manually re-declares and relays arguments it never touches itself instead of forwarding ... straight to the subfunction (documented via @inheritDotParams). This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/coding/reuse-docs-and-args.md, added in ai-config#474).

13. Flag skipped steps in math derivations

When a diff touches a mathematical derivation (an algebraic manipulation, a proof, a statistical argument), check that every step is shown -- no two or more operations (distribution, cancellation, substitution, applying a named identity or assumption) combined into a single displayed line. When a step is missing, name the exact gap (the last line before the jump and the first line after it), name the specific operation that closes it, and draft the missing line(s) where feasible rather than only flagging "skipped steps" in general. This is distinct from item 8's derivation-validity check (whether each stated step follows correctly) -- this one catches a step that isn't stated at all. This is a global standing rule from the Morrison-Lab/ai-config corpus (shared/writing/math-derivation-steps.md).

14. Don't reinvent the wheel

When a diff adds a new function or feature, check whether that functionality has already been done -- in one of the lab's own repos (the lab packages, this repo's reusable workflows and actions), or in a trustworthy external source the code could depend on instead (base R, r-lib, tidyverse, a focused, well-maintained CRAN package, a vetted, well-maintained GitHub Actions marketplace action -- SHA-pinned per README.md's "Pinning third-party actions" subsection). Flag a hand-rolled equivalent of functionality that already exists: name the existing implementation, and prefer depending on it -- or forking and/or contributing to it -- over re-building from scratch. Accept the custom version when the existing option is genuinely unfit (wrong API, unmaintained, license-incompatible, or a heavy dependency for a one-liner), and ask for a note in the PR description or a code comment -- "checked existing options, nothing fit" -- when it's missing. This is a global standing rule from the

Morrison-Lab/ai-config corpus (shared/coding/prefer-packaged-functions.md states the R-function case); its umbrella statement lives at shared/principles/dont-reinvent-wheel.md there, added in ai-config#603.