Skip to content

Audit: CI review automation and developer automation

What we check

Measured

These are the requirements the check decides a pass or a failure on, and the only ones that produce an issue. The rest of this section is part of the same standard; it is simply not decidable from the workflow files alone, so a reviewer confirms it instead.

  • Both developer workflows are present: pr-re-review.yml, which triggers another review, and pr-retest.yml, which re-runs the functional tests.
  • An automatic reviewer reaches Claude Code through the shared action shakenfist/actions/review-pr-with-claude@main, rather than a per-project script: some workflow other than the two developer workflows either calls shakenfist/actions/.github/workflows/pr-auto-review.yml@main, which reaches the action itself, or uses the action directly, on a line that is not a comment. pr-re-review.yml does not count. It uses the action too, so while it counted, a repository with no automatic reviewer at all passed.
  • pr-re-review.yml reaches shakenfist/actions/pr-bot-trigger@main rather than open-coding the phrase match, permission lookup, reaction and refusal reply.
  • No caller passes secrets: inherit.
  • In both developer workflows, the trigger job exports pr-bot-trigger's same-repo output, and the job that needs it requires that export to be 'true' in its if:.
  • pr-re-review.yml confirms that the tree it checked out is the commit its resolve step validated, on the head fallback path as well as the merge path. The confirm step has no if:, and it is either a call to shakenfist/actions/pr-review-ref@main with mode: confirm, as the template makes, or inline shell that compares HEAD^2 and also compares HEAD, as copies made before that action existed carry. A call's head-sha and merged inputs read the outputs of those names from the same action's mode: resolve step in that job.
  • The retired comment addresser is gone from the tree, as below.

Required, but confirmed by a reviewer

  • The reviewer job is a call to the reusable workflow shakenfist/actions/.github/workflows/pr-auto-review.yml@main, with the project's test jobs in its needs:. Hand-written automated_reviewer jobs are superseded; migrating deletes the project's check-bot-commit job too. A compliant hand-written job that uses the shared action still passes, because only the measurement above is made.
  • The calling job sets contents: read and pull-requests: write. issues: write is not needed, since the reviewer files no issues; a caller that still grants it is not at fault, because the reusable workflow narrows what it is given.
  • The automatic review does not pass force, and runs on same-repository pull requests only.
  • Optional, for suites prone to drift: pr-fix-tests.yml + test-drift-fix.yml.
  • The reviewer prompt asks it to check that docs/ was updated for user-visible changes.

The comment addresser is retired

Measured. None of these may be present, anywhere in the tree: .github/workflows/pr-address-comments.yml, address-comments-with-claude.sh, render-review.py, review-schema.json. Remove all four in one commit. A directory holding an action.yml/action.yaml is a composite action's own source and is exempt, which is what keeps the finding off shakenfist/actions/review-pr-with-claude/.

Why

No secrets: inherit on pr-auto-review.yml. It declares and reads no secrets -- it and review-pr-with-claude authenticate with github.token from the caller's permissions: block -- so inheriting buys nothing while putting every secret the repository holds, including publishing tokens, within reach of a workflow in another repository. Callers of smoke-cluster.yml do read secrets and inherit correctly. export-repo-config.yml reads none either, and the export-repo-config criterion makes the same finding for its callers.

The fork gate is stated twice. pr-bot-trigger already folds its fork check into authorized, so requiring same_repo on the work job again is redundant, and required anyway: both workflows put a pull request's code, or a dispatch against its ref, next to a write-scoped token, and the action is taken at @main, so a regression there must not be able to quietly widen what runs. Both halves are measured because either alone is no gate -- without the export the if: reads an empty string and the job silently never runs. The rule covers every job which needs: the trigger job, not only the one doing the work: an adopter who adds a third job downstream of it -- a cleanup or a notification -- gates that job too, or hangs it off the work job instead.

Confirm the checkout on both paths. The resolve step validates a sha and the checkout then names a ref, and a push between the two moves what the ref reaches. That is as true of refs/pull/N/head as of the merge ref, so an earlier template confirming only the merge path let a fallback review be of a commit nobody validated. Measured rather than left to the template because every adopter copied the earlier one (development#172).

The comparison itself lives in pr-review-ref, where its tests run every path against a stub gh and real git repositories, so a call to it is all that is measured, with what it is fed: a copy re-synced by hand from the inline shape keeps that shape's head_sha output name, which the action does not set, so the confirm step has no sha to confirm against and fails every run. The reverse half re-sync, an inline confirm step reading the action's resolve outputs by the old name, is worse: it compares the checkout against an empty sha and tells the requester the pull request moved, every run. So in a job that resolves through the action, any step reading an output the action does not set in resolve mode is a finding. The inline shape still passes because it works, and a repository should not be filed against for a copy that is correct; it is a re-sync to make when convenient, not a defect.

Same-repository pull requests only. The diff is untrusted input, and the reviewer reads it on a shared, long-lived runner pool in a job that holds a write-capable token. Claude Code runs there with read-only tools, confined to the checkout and without the token in its environment (actions#168), but a fork's diff would reach it with nobody having read it first. So the bot never reviews a fork pull request, whoever asks. To have a fork's change reviewed, a maintainer pushes the branch to the repository and opens the pull request from there.

The shared trigger action is a security requirement. pr-bot-trigger refuses fork pull requests; a hand-rolled copy does not inherit that. Its pr-ref output is .head.ref -- a branch name with nothing to say which repository it belongs to -- and callers hand it to actions/checkout and to git push origin HEAD:refs/heads/<ref> against their own repository. Fork PRs are commonly opened from the fork's default branch, so .head.ref is literally main: the bot commits to the target's main and pushes unreviewed. A maintainer typing the trigger phrase on a fork PR is enough; no malice required. Because the guard lives in the action, every user picked the fix up at @main -- which is the argument for the rule. Deployments predating the fix need replacing, not editing.

The retired addresser is not inert. It triggers on issue_comment, so it holds contents: write on the PR branch for a feature nobody wants, and it is the last caller of render-review.py, so the script and schema are dead weight the next project copies. Deleting the workflow but keeping the scripts leaves exactly the copy that propagates. Background: templates/ci-review-automation/README.md.

Template

Template: templates/ci-review-automation/ See: templates/ci-review-automation/README.md Docs: docs/ci-review-automation.md, docs/automated-pr-review.md

Test drift fixing template: templates/test-drift-fix/ See: templates/test-drift-fix/README.md

Projects

Per-project compliance for this criterion is regenerated on every run of the consistency audit: see the compliance page.

📝 Report an issue with this page