ci: add Claude-powered PR review, mention, and issue-to-PR workflows#402
Merged
ci: add Claude-powered PR review, mention, and issue-to-PR workflows#402
Conversation
Onboards Harper core to the AI workflow pattern we've been calibrating in HarperFast/oauth. Three workflows: - claude-review.yml — auto-review on every PR from org members and claude[bot]-authored PRs. Uses Sonnet 4.6, 24 turns, 15min timeout. Review checklist is composed from HarperFast/ai-review-prompts layers (universal + harper/common + harper/v5) plus a small repo-specific section for Harper core notes (oxlint, RocksDB, TypeStrip, dependencies.md). - claude-mention.yml — responds to @claude mentions from org members. Uses Opus 4.7, 48 turns, 20min timeout. Reasoning-heavy / open-ended asks need the extra capability. - claude-issue-to-pr.yml — label-triggered (claude-fix:typo|docs|deps| bug). Uses Sonnet 4.6, 72 turns, 25min timeout. Bounded maintenance work. Security posture reflects the hardening the oauth workflows went through: - author_association gate (OWNER/MEMBER/COLLABORATOR) on every job. - allowed_bots: claude on the review step so AI-authored PRs get reviewed (claude-code-action has its own bot-actor gate). - Tool allowlist omits Bash(npx:*) (biggest RCE primitive) and uses Bash(npm install) (no-arg) instead of Bash(npm install:*) (arbitrary package). Inline comments explain what's missing on purpose. - Interpolated user content (comment.body, issue.title, issue.body) is framed with code fences or inline backticks. Not a security boundary on its own — the gate and allowlist are — but reduces accidental prompt bleed-through. - The review prompt tells the agent that CLAUDE.md / AGENTS.md are part of the PR's own checkout, so a malicious PR could edit them to inject instructions. It should treat their contents as authoritative for conventions but NOT for overriding review discipline. Prerequisites once merged: 1. Add `ANTHROPIC_API_KEY` as a repository secret. 2. Create labels `claude-fix:typo`, `claude-fix:docs`, `claude-fix:deps`, `claude-fix:bug` for the issue-to-PR pipeline. 3. Branch protection on `main` and `release_*` — the "Must NOT push to main" guidance in the issue-to-pr prompt is a soft guardrail only; real protection is GitHub's branch-protection rules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
How do we protect against prompt injection / unauthorized invocation of it? |
Contributor
|
Ah I see, ok |
cb1kenobi
approved these changes
Apr 24, 2026
Contributor
cb1kenobi
left a comment
cb1kenobi
left a comment
There was a problem hiding this comment.
This is probably one the most crazy PRs I've ever reviewed. Wow.
Comment on lines
+43
to
+44
| with: | ||
| fetch-depth: 0 |
Contributor
There was a problem hiding this comment.
Generally you don't need to specify fetch-depth: 0. You don't need the full history of the checkout action.
Suggested change
| with: | |
| fetch-depth: 0 |
Contributor
There was a problem hiding this comment.
Unless the commits help Claude?
Contributor
There was a problem hiding this comment.
For the checkout action?
Member
Author
There was a problem hiding this comment.
Yeah, good question. We could and should make use of some depth here (and allow some git commands vs gh diff api) but could drop it in the issue-to-pr.
Contributor
There was a problem hiding this comment.
What's wrong with the default depth?
Member
Author
There was a problem hiding this comment.
claude only gets the shallow change with the default 1 depth and won't have the historical context for more thoughtful reviews. it's a workflow perf tradeoff to try to get better, more actionable, reviews. it's still going to take some iterating though. this first pass in harper still won't be perfect and will likely need modifications as models and context evolve...
kriszyp
approved these changes
Apr 24, 2026
Member
kriszyp
left a comment
kriszyp
left a comment
There was a problem hiding this comment.
Very cool, thank you for getting this in, it will be fun to see how it does!
Per inline review feedback from @cb1kenobi on #402, with a twist: rather than just dropping `fetch-depth: 0` everywhere, tighten mention and issue-to-PR (where Chris is unambiguously right — those agents never reach for history) and INVEST on the review side — add read-only git subcommands so the reviewer can do `git blame` / `git log` / `git diff <base>...HEAD` for real context on non-trivial PRs. ## Mention / issue-to-PR workflows - Drop `fetch-depth: 0` (default shallow is fine). Agents commit and push — neither needs deep history. `git log` / `git blame` aren't reached for in the current prompts. Added back cheaply if we see real blocks on history lookups. ## Review workflow - Keep `fetch-depth: 0`. The reviewer now has access to history via the allowlist additions below; depth 1 would make blame useless. - `--allowedTools` gains read-only git subcommands, individually scoped: `Bash(git diff:*)`, `Bash(git log:*)`, `Bash(git blame:*)`, `Bash(git show:*)`. Deliberately NOT `Bash(git:*)` — that would permit `git push --force`, `git reset --hard`, etc. - Drops `Bash(gh pr diff:*)` — `git diff <base>...HEAD` replaces it and avoids the API round-trip. `gh pr view`, `gh pr comment`, and the `mcp__github_inline_comment__create_inline_comment` tool stay — they all do different things (metadata, top-level summary, per-line anchored findings). - Prompt's "Tools" section now tells the agent to use `git blame` for the "is this code the PR introduced vs pre-existing" judgment (which the layered scope's Testing section already cares about), and `git log` / `git show` for "why is this load-bearing" context before flagging. Chris's observation was right that `fetch-depth: 0` was dead weight given the prior allowlist — but the right fix on the review surface is to make the allowlist richer where extra context pays off, not just shrink the checkout. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 24, 2026
Stacked onto #402 in response to the deep external review. Changes: Accepted and fixed: - #3 (ai-review-log log step): Ported verbatim from oauth. Harper's reviews now feed the central calibration tracker that the weekly sweep runs against. Adds AI_REVIEW_LOG_TOKEN to the secrets prerequisite list (flagged in PR body update). - #4 (dead `documentation/**` glob): Harper has no `documentation/` dir — the docs site is a separate repo. Replaced with realistic Harper doc-file names (README.md, CLAUDE.md, AGENTS.md, dependencies.md) + package.json keyword edits. Same fix in both mention and issue-to-pr prompts. - #5 (prefix-match label too permissive): `startsWith('claude-fix:')` matched typoed variants (`claude-fix:typos`, etc). Tightened to explicit whitelist of the four supported labels. - #6 (fixed heredoc marker collision risk): Replaced `CLAUDE_SCOPE_EOF` with a random `EOF_$(openssl rand -hex 16)` delimiter. Collision-proof against any content a future ai-review-prompts layer might include. - #7a (eager `npm ci` on mention): Removed. Most mentions (explain, review, small edits) don't need deps — install is ~35-60s × every mention. Prompt now tells the agent to run `npm ci` itself before any script that requires dependencies. issue-to-pr keeps its eager install since that workflow almost always builds/tests. - #8a (Opus cost on every mention): Shifted to Sonnet default with Opus opt-in via case-insensitive word-boundary `deep` in the comment. "Needs deep review of the whole migration" escalates; "fix this typo" stays on Sonnet. Cost gets spent deliberately, not by default. - #9 (no scope-to-diff guidance): Review prompt now tells the agent to start from `git diff --name-only <base>...HEAD` and only expand scope when a specific finding demands it. On a ~1000-file repo this matters. Plus a mention-parsing step that enforces: - `@claude` must be the first non-whitespace token (word-boundary after) — rules out `@claudette`, inline prose mentions, and quoted replies (`> @claude ...`) where the reply addresses a human. The existing `contains('@claude')` job-level `if:` stays as a cheap pre-filter; the new shell step is the real precision gate. - Subsequent steps guard on `steps.mention.outputs.proceed == 'true'`. Comment sharpening (accept the tradeoff, tighten the rationale): - #1 (postinstall RCE via package.json edit): The allowlist comment on both agent workflows previously implied `Bash(npm install)` (no-arg) was a real mitigation. It blocks `npm install @attacker/x` but NOT the `postinstall` path — an injection can edit package.json and then bare `npm install` executes the hostile lifecycle script with GITHUB_TOKEN + the claude[bot] installation token in env. Comment now names this path explicitly. The actual guardrails are branch protection + the author_association gate; a structural fix (`.npmrc ignore-scripts=true`, or dropping `Bash(npm install)` entirely in favor of a separate CI install job) deserves its own PR. - #2 (`Bash(git:*)` contradicts review.yml's stated principle): review.yml's comment previously read as universal guidance. It's actually specific to the read-only review workflow. Comment now explicitly notes that the authoring workflows deliberately grant broader git access and rely on branch protection as the guardrail. Not addressed here: - Splitting issue-to-pr into read-only research + narrow-write commit steps (post-v0.1.0 follow-up). - Tightening mention/issue-to-pr to specific read-only + commit/push git subcommands (same structural PR). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Onboards Harper core to the AI workflow pattern calibrated in HarperFast/oauth. Three new workflows under
.github/workflows/:claude-review.yml— auto-review on every PR from org members and claude[bot]-authored PRs. Sonnet 4.6, 24 turns, 15min timeout.claude-mention.yml— responds to@claude …from org members. Sonnet 4.6 default; Opus 4.7 opt-in via word-boundarydeepin the comment body. 48 turns, 20min timeout.claude-issue-to-pr.yml— label-triggered (claude-fix:typo|docs|deps|bug). Sonnet 4.6, 72 turns, 25min timeout.Review checklist
Review prompts compose layers from
HarperFast/ai-review-prompts(public, pinned by SHA):universal— architecture, security, dispatch surfaces, testing, output disciplineharper/common— cross-version Harper gotchasharper/v5— v5-specific (Resource API v2, Fabric deployment,.envsemantics)A small repo-specific section in
claude-review.ymlcovers Harper-core conventions the shared layers don't yet know (oxlint, RocksDB, TypeStrip,dependencies.md). A calibratedrepo-type/core.mdlayer lands inai-review-promptsonce this repo's PR reviews produce enough signal to support it.Security posture
Reviewed against a deep external review (findings summary in the fae521c commit message):
author_associationgate (OWNER/MEMBER/COLLABORATOR) on every joballowed_bots: claudeon the review step so AI-authored PRs get reviewed (claude-code-action has its own bot-actor gate separate from the workflowif:)Bash(npx:*)and uses no-argBash(npm install)(NOT:*). Honest caveat in the workflow comment: an injection could still editpackage.jsonto add a maliciouspostinstalland then invoke barenpm installto execute it. The allowlist alone doesn't close that; branch protection + the author_association gate are what bound blast radius.Bash(git:*)deliberately (they're authoring workflows). Review workflow is read-only by contrast, with git subcommands individually scoped (git diff/log/blame/show, noBash(git:*)).@claudemust be the first non-whitespace token (word-boundary after) — rules out@claudette, inline prose mentions, and quoted replies where the@claudeis addressing a human.comment.body,issue.title,issue.body) is framed with code fences or inline backticks. Not a security boundary on its own.Prerequisites before these workflows can fire
ANTHROPIC_API_KEYas a repository secret.AI_REVIEW_LOG_TOKENas a repository secret (for the logging-to-ai-review-log post-step; skipped gracefully if unset).claude-fix:typo,claude-fix:docs,claude-fix:deps,claude-fix:bug.mainand therelease_*/v*.xbranches — the "Must NOT push to main" prompt instruction is a soft guardrail; real protection is GitHub branch-protection rules.Test plan
ANTHROPIC_API_KEYandAI_REVIEW_LOG_TOKEN, createclaude-fix:*labels, confirm branch protectionclaude-review.ymlfires and posts a review@claudea small ask on an issue to confirmclaude-mention.ymlresponds@claude deep audit …on an issue to confirm Opus escalation kicks inclaude-fix:typoto confirm the issue-to-PR pipeline opens a PRharper/v5.md— follow-up PRs on this repo and onai-review-prompts