Skip to content

Claude PR review runs one generic pass: split it into parallel lenses defined as repo skills #997

Description

@tobyhede

Background

This repository is adding an automatic code review by Claude (Anthropic's AI model) on every pull request, in #974. It runs as a GitHub Actions workflow using Anthropic's official claude-code-action. It signs in to Anthropic with OIDC (OpenID Connect — GitHub hands the job a short-lived identity token, and Anthropic trusts that token instead of a stored API key). The review is advisory: it leaves comments, it never approves, blocks, or merges.

Problem Statement

As merged in #974, the review is one generic pass. Maintainers want several focused reviews running at the same time — for example correctness, security, and this repository's own rules — and they want each review's instructions to live in a skill that is easy to read and edit, not buried in workflow YAML. They also want to run Anthropic's official /code-review plugin as one of those reviews.

Building that safely depends on several facts about the action that are not visible from the workflow file and are easy to get wrong. Some were got wrong while #974 was being reviewed:

  1. A prompt on a pull-request event puts the action in "agent mode". Agent mode gives Claude no pull-request context and creates no comment. With shell commands blocked and a one-commit checkout, the original review could not see the diff, and its summary was thrown away — while the check still went green.
  2. The action replaces .claude/ and CLAUDE.md with the base branch's copies before Claude starts. So skills, agents, settings, and hooks always come from main, and a pull request cannot rewrite its own reviewer. But CLAUDE.md imports AGENTS.md, and AGENTS.md is not replaced. Without an extra step, main's CLAUDE.md loads the pull request's AGENTS.md as trusted instructions.
  3. "Tag mode" (track_progress: true) looks like the easy fix and is not. It provides a progress comment, but it also lets Claude commit, push, and edit files without asking.
  4. The action skips itself on any pull request that edits the workflow file, because the file must match main. It reports this as success unless the workflow checks for it. So no change to this workflow can be tested on its own pull request.
  5. Plugins cannot be pinned by URL. The action only accepts a marketplace URL ending in .git, so installing code-review@claude-code-plugins the documented way runs whatever is on anthropics/claude-code's main branch, inside a job that holds an identity token and pull-request write access. That breaks this repository's rule that every third-party action is pinned to a commit.

None of this is written down anywhere a contributor would find it. The next person to add a review will rediscover it, or not.

Solution

Turn the review into a matrix of parallel review lenses, each defined by a repository skill. Write the rules down so adding a lens is a routine change.

  • Each lens is one GitHub Actions matrix entry and one skill. It gets its own check on the pull request, its own inline comments, and one summary comment that it updates on each push.
  • Lens instructions live in skills. Lenses that apply to every CipherStash repository live in the organisation skills repository, cipherstash/skills (the company-skills plugin), so every repository's review shares them. Lenses specific to this repository live in its own .claude/skills/, which the action loads from main.
  • The organisation skills are installed from a checkout of cipherstash/skills pinned to a commit, so a pull request cannot change them and an upstream change only arrives through a reviewed bump of that commit.
  • Claude only reads. It reads the diff with scoped gh pr diff / gh pr view commands and posts inline comments. It hands its summary back as structured data, and a plain shell step — not Claude — publishes it.
  • A review that skipped or produced no summary fails its check. A green check means a review happened.
  • The official /code-review plugin runs as one more lens. It is installed from a local checkout of anthropics/claude-code pinned to a commit.
  • The rules are written down in AGENTS.md and in an ADR (architecture decision record), so they are found by the next contributor and by agents working in this repository.

User Stories

  1. As a maintainer, I want several focused Claude reviews to run in parallel on each pull request, so that each one looks deeply at one concern instead of skimming everything.
  2. As a maintainer, I want each review lens to show as its own check, so that I can see at a glance which lenses ran and which failed.
  3. As a maintainer, I want one lens failing not to cancel the others, so that a flaky lens does not hide the other results.
  4. As a maintainer, I want each lens to keep a single summary comment that it updates on every push, so that the pull request is not buried in repeated bot comments.
  5. As a maintainer, I want each inline comment to say which lens left it, so that I know which concern it comes from.
  6. As a maintainer, I want lens instructions in a skill file, so that I can read and change what a lens checks without editing workflow YAML.
  7. As a maintainer, I want to add a lens by adding one skill and one matrix entry, so that growing the review is routine.
  8. As a maintainer, I want a test that fails when a matrix lens has no skill or a lens skill has no matrix entry, so that the two cannot drift apart silently.
  9. As a maintainer, I want a lens for correctness — logic errors, regressions, compatibility breaks, missing tests for changed behaviour — so that bugs are caught before a human looks.
  10. As a maintainer, I want a lens for security — plaintext or secrets in logs or errors, weakened encryption, auth or lock-context flows, injection, GitHub Actions privilege problems, supply-chain regressions — so that security issues get dedicated attention.
  11. As a maintainer, I want a lens for this repository's rules that CI does not enforce — stale customer-facing skills, missing or wrongly scoped changesets, stale repository-layout docs, Linear IDs on GitHub — so that the rules agents most often miss are checked.
  12. As a maintainer, I want Anthropic's official /code-review plugin to run as a lens, so that we also get its multi-agent bug hunt with per-finding validation.
  13. As a maintainer, I want that plugin pinned to a specific commit, so that an upstream change cannot alter what runs in a job holding an identity token and pull-request write access.
  14. As a security reviewer, I want a pull request to be unable to change the instructions its own reviewer follows, so that a contributor cannot tell the review to ignore their change.
  15. As a security reviewer, I want every file CLAUDE.md imports to be restored from the base branch, so that the gap left by the action's own restore is closed.
  16. As a security reviewer, I want that restore list derived from CLAUDE.md's imports, so that a new import cannot slip through unrestored.
  17. As a security reviewer, I want Claude to be unable to edit files, commit, push, approve, label, or merge, so that a manipulated review can do no more than leave a misleading comment.
  18. As a security reviewer, I want shell access limited to read-only gh pr commands, plus the commands a pinned plugin lens declares, so that review cannot run arbitrary commands.
  19. As a security reviewer, I want the summary comment published by a shell step rather than by Claude, so that Claude needs no general comment-writing ability.
  20. As a security reviewer, I want the summary step to find its own comment by author and by a marker at the very start of the comment, so that a summary that quotes another lens's marker cannot overwrite that lens's comment.
  21. As a security reviewer, I want the summary sent to GitHub as a JSON document, so that a summary like true or 42 is not turned into a boolean or a number.
  22. As a security reviewer, I want reviews to run only on non-draft pull requests from branches in this repository, not forks or bots, so that the identity token is never minted for untrusted code.
  23. As a maintainer, I want a lens that skipped, crashed, or returned no summary to fail its check, so that a green check always means a review happened.
  24. As a maintainer, I want that failure message to name the likely cause when the action skipped because the workflow file changed, so that the red check on a workflow-editing pull request is not a mystery.
  25. As a maintainer, I want rapid pushes debounced and earlier runs cancelled, so that we pay for one review of the latest commit, not one per push.
  26. As a maintainer, I want each lens bounded in turns and time, so that a confused review cannot run up cost indefinitely.
  27. As a contributor, I want the rules for changing the review workflow written in AGENTS.md, so that I know about agent mode, tag mode, the restore gap, and the self-skip before I start.
  28. As a contributor, I want an ADR recording why lens instructions are skills restored from the base branch and why Claude is read-only, so that the reasoning survives when someone proposes a "simpler" setup.
  29. As a contributor, I want to know that workflow changes cannot be tested on their own pull request, so that I plan a check on a test pull request after merging.
  30. As a maintainer, I want the post-merge check on a test pull request to be part of finishing this work, so that "CI is green" is not mistaken for "the review works".
  31. As an engineering lead, I want review lenses that apply to every repository to live in the organisation skills repository, so that improving one lens improves every repository's review.
  32. As a maintainer, I want lenses specific to this repository to stay in this repository, so that its rules are reviewed alongside the code they govern.
  33. As a security reviewer, I want the organisation skills pinned to a full commit SHA, so that a change in the skills repository cannot alter this repository's review without a pull request here.
  34. As a maintainer, I want the organisation skills checkout to need no credentials, so that the review workflow keeps holding no stored secrets.
  35. As a developer, I want to be able to install the same organisation skills locally, so that I can run a lens on my branch before pushing.

Implementation Decisions

  • Shape. One job, review, with a matrix of lenses and fail-fast: false. The job name stays review, so the existing repo-wide guards that key on job names (the fork-guard list and the OIDC token-holder classification) do not change. Each matrix entry names its lens and the per-lens settings that differ — at minimum its allowed tools and its turn limit.
  • Lens definition. Each lens has a skill named review-<lens>. The workflow prompt carries only the plumbing: repository, pull-request number, head commit, which skill to use (by its full name), how to read the diff, and the output contract. What to look for lives in the skill. Starting lenses:
    • correctness and security: organisation-wide, authored in cipherstash/skills and invoked as company-skills:review-correctness and company-skills:review-security.
    • repo-rules: specific to this repository, authored in its .claude/skills/.
    • code-review: Anthropic's plugin (see Plugin lens).
      Each matrix entry records where its skill comes from (organisation or repository), so the test can check the right place.
  • Organisation skills. cipherstash/skills is a Claude Code plugin marketplace (company) publishing one plugin (company-skills). Each lens job checks it out at a pinned full commit SHA into a scratch directory with persist-credentials: false, and installs it through the action's plugin_marketplaces (that local path) and plugins (company-skills@company) inputs. A local path is the only way the action allows a pinned install. The repository is public, so the checkout needs no token and the workflow keeps holding no stored secrets. Plugins install into the runner's user configuration, outside the pull request's checkout, so the action's base-branch restore does not touch them and a pull request cannot alter them. Updating the organisation lenses means a pull request here that bumps the pinned SHA; Dependabot does not track checkout refs. Organisation review skills follow that repository's own CONTRIBUTING.md and pass its scripts/validate.py.
  • Trusted instructions. Keep the action's restore of .claude/ and CLAUDE.md from the base branch. Add workflow steps that sparse-check-out the base commit and copy every file CLAUDE.md imports (today just AGENTS.md) over the pull request's copy before Claude starts, then delete the temporary checkout. Do not pass --setting-sources user: it would stop the restored instructions and the lens skills from loading at all, and the action's restore already deals with the hooks risk it was meant to address.
  • Mode. Agent mode only: track_progress: false, no use_sticky_comment (it does nothing in agent mode).
  • Tools. Default allow-list per lens: the inline-comment tool, gh pr diff, gh pr view. Always disallowed: Edit, Write, NotebookEdit, WebFetch, WebSearch. Never put a blanket Bash in the disallow list — it overrides the scoped Bash(gh pr …) allows. Task (subagents) is allowed only for a lens that needs it; today that is only code-review. The code-review lens additionally gets exactly the gh commands its pinned command file declares. Whether the Skill tool must be allow-listed for skills to load is unverified — settle it on the post-merge test pull request.
  • Output contract. Claude returns { "summary": string } through the action's --json-schema option; the action already marks the run failed if structured output is missing. A shell step then publishes <!-- claude-review:<lens> -->, a heading naming the lens, and the summary. It finds an existing comment by author and that marker at the start of the comment, then updates it or creates one. It sends the body as JSON. Inline comments start with the lens name in brackets.
  • Fail closed. An always-run step fails the job unless the action reported conclusion=success and returned a non-empty summary. When the conclusion is unset, the message says the likely cause is a workflow file that differs from main.
  • Plugin lens. Check out anthropics/claude-code at a pinned commit into a scratch directory, and install code-review from that local path through the action's plugin_marketplaces and plugins inputs. Run it as /code-review <PR> --comment. Do not copy its command into this repository: its licence is "All rights reserved". Its first step stops if Claude has already commented on the pull request, so it may review only the first push, or may skip entirely because of the other lenses' summary comments. Check this on the post-merge test pull request and record the result (see Further Notes).
  • Unchanged. The trigger set, fork/draft/bot condition, workflow-level contents: read, the job-level contents: read / pull-requests: write / id-token: write, the federation preflight, the debounce, the action pinned to a commit, and persist-credentials: false on every checkout.
  • Documentation. Add a "Claude PR review" section to AGENTS.md covering: agent mode versus tag mode, the base-branch restore and its AGENTS.md gap, the self-skip on workflow edits, the read-only tool rules, the output contract, and how to add a lens. Add an ADR recording two decisions and why: lens instructions are skills restored from the base branch, and Claude is read-only with publishing done by a shell step.

Testing Decisions

  • One seam: the workflow-shape test that already exists for this workflow. It reads the workflow YAML and the repository tree and asserts on structure, not on wording beyond the fixed contracts. A good test here fails when a security property or a contract breaks, and does not fail on a reworded prompt.
  • That test should assert:
    • the set of matrix lenses equals the set of review-* skills, both ways round. For repository lenses this is checked against this repository's .claude/skills/. For organisation lenses the test checks that the matrix names a company-skills: skill; it cannot see the skills repository's tree;
    • the organisation skills checkout targets cipherstash/skills at a full 40-character SHA, keeps no credentials, passes no token, and is installed from that local path, never from a URL;
    • every lens's allowed tools are within the permitted set, Task appears only on lenses that declare subagents, and no lens can edit, write, or fetch from the web;
    • no blanket Bash appears in any disallow list;
    • tag mode is off and --setting-sources is not restricted;
    • the base-branch restore list equals CLAUDE.md's imports, and the restore runs before the review and cleans up afterwards;
    • the plugin checkout is pinned to a full commit SHA and the plugin is installed from that local path, never from a URL;
    • the fail-closed step runs always, comes after the review, and checks both the conclusion and the summary;
    • the publish step keys on the per-lens marker and sends a JSON body.
  • Prior art: the existing Claude-review workflow test (permission pinning, a job-set equality, the CLAUDE.md-import derivation), the repo-wide fork-guard job list, and the OIDC token-holder classification test, which should keep passing unchanged.
  • Acceptance outside CI. The action skips on any pull request that edits the workflow, so CI cannot prove the review works. After merge, open a small test pull request with a planted bug and a planted rule violation, then confirm: every lens check runs; each lens posts one summary and updates it on a second push; inline comments carry lens prefixes; the planted issues are found; and the code-review lens behaves as recorded. Record the result on this issue.

Out of Scope

  • Reviewing pull requests from forks. The identity token must not be minted for untrusted code.
  • Making any review lens a required or blocking check.
  • Letting Claude apply fixes, push commits, or use tag mode.
  • Running the built-in /code-review command that ships inside the Claude Code CLI. The action does not document headless use of built-in commands; the plugin is the supported route.
  • Changing the Anthropic federation setup or the four GitHub Actions variables it uses.
  • Deduplicating inline comments across pushes.
  • Sharing the whole review workflow across repositories as a reusable workflow. The Anthropic federation rule binds to the calling repository and workflow, and the action's "workflow must match the default branch" check has not been tested with reusable workflows. Sharing the skills comes first.
  • Automating bumps of the pinned organisation skills SHA.

Further Notes

  • Depends on Add federated Claude PR review #974. That pull request introduces the single-pass workflow this spec turns into lenses, including scoped diff reading. Land Add federated Claude PR review #974 first, then implement this.
  • Prerequisites in cipherstash/skills: make the repository public; settle its default branch (it is setup/cross-platform-skills today, while main exists and is newer); and add the review-correctness and review-security skills there. Today it contains no skills. Pin to a commit on the default branch once those land.
  • Local use. Developers can install the same plugin with /plugin marketplace add cipherstash/skills followed by /plugin install company-skills@company, as that repository's README describes. Whether this repository's project settings can point everyone at the marketplace automatically is unverified.
  • Open question for the code-review lens: review only the first push (the plugin's default) or every push? Every push means keeping a pinned, modified copy of the command, which the licence may not allow. Decide from what the post-merge test pull request shows; until then, accept the plugin's default.
  • Cost. Each push runs every lens after a five-minute debounce. The code-review lens alone launches eight or more subagents, several of them on Opus.
  • Evidence. Verified against anthropics/claude-code-action at the pinned commit bf38e86e58df9ebf3420326d019f955bb3be64dd (v1.0.225):
    • src/modes/detector.ts — a prompt on an opened, synchronize, ready-for-review, or reopened event selects agent mode.
    • src/modes/agent/index.ts — agent mode writes the prompt verbatim and passes no tracking comment.
    • src/mcp/install-mcp-server.ts — in agent mode, comment tools are installed only when explicitly allowed.
    • src/modes/tag/index.ts — tag mode adds git add, git commit, and a push wrapper to the allowed tools, and runs with acceptEdits.
    • src/entrypoints/run.ts:263 and src/github/operations/restore-config.ts — on pull-request events, .claude, CLAUDE.md, CLAUDE.local.md, .mcp.json, .claude.json, .gitmodules, .ripgreprc, and .husky are restored from the base branch. AGENTS.md is not.
    • src/entrypoints/run.ts:179 — the workflow-validation skip returns without setting conclusion.
    • base-action/src/run-claude-sdk.ts — with --json-schema, missing structured output sets conclusion=failure.
    • base-action/src/install-plugins.ts — a marketplace must be a URL ending in .git or a local path, so there is no commit pin by URL.
    • Plugin source: anthropics/claude-code plugins/code-review/commands/code-review.md.
  • Verified in cipherstash/skills: the marketplace is named company, the plugin's manifest name is company-skills (which prefixes its skills), and it is MIT-licensed.
  • Plausible but unverified: that skills load with only Skill allow-listed; that plugin skills installed through plugin_marketplaces are available in agent mode; how the code-review plugin's "already commented" check treats summaries posted by github-actions[bot].

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    SDKenhancementNew feature or requestgithub-actionsPull request modifies GitHub Actionsready-for-agentFully specified and ready for an AFK agent

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions