Skip to content

chore(ci): migrate PR review to the shared review harness (v2) - #8

Closed
invacuo wants to merge 4 commits into
masterfrom
chore/migrate-pr-review-harness-v2
Closed

invacuo wants to merge 4 commits into
masterfrom
chore/migrate-pr-review-harness-v2

Conversation

@invacuo

@invacuo invacuo commented Oct 6, 2026 •

Copy link
Copy Markdown

Stakeholder Overview

Replaces moqueue's standalone claude-code-action review workflow with the shared CustomInk PR review harness (customink/agent-first-standards, pinned to @v2). The harness owns the diff and previous-review fetching, prompt assembly, posting and trust boundary that this repo's workflow was doing by hand.

Trust boundary / first exercise: the harness skips the real review on a PR that edits the review stub, so review / code on this PR is not evidence the setup works. The new configuration is first exercised on the first PR after merge.

Risk Estimate

✅ Negligible risk!

CI-only change to a gem's review tooling; no library code changes. Branch protection on master, verified with gh api:

  • required_status_checks: none (checks: [], contexts: [], enforcement off); /rules/branches/master returns []. Nothing requires the old or new check name.
  • required_approving_review_count: 0, require_code_owner_reviews: false, enforce_admins: true.
  • Not verified: whether a REQUEST_CHANGES review posted by the harness would block merging under these settings.
Click to expand!

Commit 1: migrate PR review to the shared review harness (v2)

  • .github/workflows/claude-code-review.yml removed and .github/workflows/code-review.yml added with the canonical stub from the harness's ADOPTING.md (## The stub), plus verdict_policy: deterministic. Git shows this as a delete plus an add, not a rename, because the content is almost entirely different.
  • Job key stays review, so the check reads review / code. closed is in pull_request types (lifecycle telemetry). Permissions are actions: read, contents: read, pull-requests: write, issues: write.

Commits 2 and 3: trim .claude/review-prompt.md

  • Removes Only comment when you have actionable feedback. Never post "looks good"..., which restates reviewer behavior the base prompt owns. A second commit removes the bullet "If the entire PR looks good, do not post a comment at all" (it conflicts with the harness posting an APPROVE for a clean review under verdict_policy: deterministic). A third commit trims the rest of the file down to its heading and intro line: the prompt was an unmodified template with no repo-specific rules, and the harness base prompt already covers its content (severity tiers, review behavior, confidence scoring, previous-review handling, generated-file filtering). Parts of it conflicted with the base prompt (the Critical/Question severities, and "phrase uncertainty as a question" versus the base prompt dropping low-confidence findings). The file is kept rather than deleted, so repo context now comes only from CLAUDE.md. Also dropped with it: the generic "What to Evaluate" list, the PR Context line, and the "flag change hygiene" and "call out good choices" bullets, which the base prompt does not cover.

Notes

Dropped from the legacy workflow

  • issue_comment / pull_request_review_comment / pull_request_review triggers: @claude comment-triggered re-review no longer works. The stub triggers only on pull_request.
  • --allowedTools list, --max-turns 10, and the vars.CLAUDE_REVIEW_MODEL model-selection step (harness owns tools, turns and model; no model, max_turns or other with: inputs are carried over besides verdict_policy).
  • Plumbing steps now done by the harness: Resolve PR number, Checkout, Fetch PR diff, Fetch previous reviews, Build review prompt, Resolve claude_args, plus the inline anti-repetition prompt text and the workflow-level concurrency block.
  • Draft/merged if: conditions and id-token: write. issues: read became issues: write.
  • continue-on-error: true: previously a failed review never failed the job. With verdict_policy: deterministic the harness can post REQUEST_CHANGES or APPROVE (blocking findings give REQUEST_CHANGES, clean gives APPROVE). See the branch-protection facts above.
  • allowed_bots: "custom-ink-triton": the harness uses allowed_bots: "*" with denied_bots defaulting to dependabot[bot],renovate[bot], so other non-denied bots' PRs (including custom-ink-triton) are now also reviewed.

AGENTS.md: the legacy "Build review prompt" step concatenated AGENTS.md, CLAUDE.md and .claude/review-prompt.md. The root CLAUDE.md only contains @AGENTS.md, and AGENTS.md holds the repo context. Whether the harness resolves that @AGENTS.md import was NOT verified (AGENTS.md is not named in the harness's pr-review.yml or ADOPTING.md). It will be first exercised on the first PR after merge; if reviews lack repo context, fold the essentials into .claude/review-prompt.md.

Other workflows: .github/workflows/rubocop.yml (reviewdog RuboCop lint reporter) is not an AI reviewer and is left unchanged. stale.yml is unrelated.

Not run: the skill's Step 6 local dry-run was not run.

Surviving path filters: none. No claude_md_glob is set (no sub-directory CLAUDE.md files). CODEOWNERS is * @customink/premium-blend.

Rollout: do not make review / code a required status check (per the harness's docs/ADOPTING.md: the harness skips drafts, stub-modifying PRs, denied bots and duplicate SHAs, and a skipped workflow never reports a status, which blocks merging when required). review_mode: blocking is available if suggestion churn becomes a problem.

🤖 Generated with Claude Code

invacuo and others added 2 commits October 6, 2026 11:22
Replace the standalone claude-code-action review workflow with the
canonical caller stub for the shared harness, pinned to @v2, with
verdict_policy: deterministic.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The shared review harness base prompt owns review behavior, so the
'only comment when actionable' line restates it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@invacuo
invacuo requested a review from a team as a code owner October 6, 2026 15:24
invacuo and others added 2 commits October 6, 2026 12:23
…w prompt

The harness posts an APPROVE verdict for a clean review under
verdict_policy: deterministic, so a repo-prompt instruction to post no
comment at all on a clean PR conflicts with it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The base prompt already owns severity tiers, review behavior, confidence
scoring, previous-review handling and generated-file filtering. This
prompt was an unmodified template with no repo-specific rules, and parts
of it (Critical/Question severities, "phrase uncertainty as a question")
conflict with the base prompt, so only the heading and intro remain.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@invacuo

invacuo commented Oct 6, 2026

Copy link
Copy Markdown
Author

Closing: moqueue is a public repo, and GitHub does not allow public repos to call reusable workflows stored in private repos, so the shared review harness (customink/agent-first-standards, private) cannot run here. Merging this would replace the working legacy review workflow with one that fails to start.

@invacuo invacuo closed this Oct 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants