Skip to content
This repository was archived by the owner on Oct 7, 2026. It is now read-only.

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

Merged
wojtha merged 2 commits into
masterfrom
chore/migrate-pr-review-harness-v2
Oct 6, 2026
Merged

wojtha merged 2 commits into
masterfrom
chore/migrate-pr-review-harness-v2

Conversation

@invacuo

@invacuo invacuo commented Oct 6, 2026

Copy link
Copy Markdown

Stakeholder Overview

Moves dm-core's PR review from a standalone anthropics/claude-code-action workflow (claude-code-review.yml, which assembled the prompt and diff itself) to the shared CustomInk review harness (customink/agent-first-standards, pinned @v2). The workflow is now a thin caller stub, .github/workflows/code-review.yml, with job key review. The repo's own .claude/review-prompt.md and CLAUDE.md stay as the repo-specific review context.

Risk Estimate

✅ Negligible risk!

CI configuration only; no library code changes. The only user-visible effect is how PR reviews are produced and posted.

  • Branch protection: gh api repos/customink/dm-core/branches/master/protection/required_status_checks returns 404 "Required status checks not enabled", and gh api repos/customink/dm-core/rules/branches/master returns [] (no rulesets). No required check matches the old job key (review) or workflow name (Claude Code Review), so nothing is orphaned.
  • The review job key is kept; the check will render as review / code.
Click to expand!
  • .github/workflows/claude-code-review.yml renamed to .github/workflows/code-review.yml (git records it as delete + add, since the content is almost entirely replaced) and replaced with the canonical stub from the harness's docs/ADOPTING.md (## The stub, read at ?ref=v2):
    • uses: customink/agent-first-standards/.github/workflows/pr-review.yml@v2
    • on: pull_request with types: [opened, synchronize, ready_for_review, reopened, closed] (closed is what the harness needs for lifecycle telemetry)
    • permissions exactly actions: read, contents: read, pull-requests: write, issues: write
    • verdict_policy: deterministic set explicitly
    • the five secrets forwarded: ANTHROPIC_API_KEY, CUSTOM_INK_CODE_REVIEW_APP_ID, CUSTOM_INK_CODE_REVIEW_PRIVATE_KEY, NEW_RELIC_ACCOUNT_ID, NEW_RELIC_INSERT_KEY
  • .claude/review-prompt.md: removed the paragraph "Only comment when you have actionable feedback. Never post "looks good"..." (separate commit). The base prompt already owns review behavior. All domain sections are untouched.

Notes

Trust boundary. The harness skips the real review on a PR that edits the review stub, and it restores CLAUDE.md from the base branch. So review / code on this PR going green does not show the setup works (its review steps are skipped). The new configuration is first exercised on the first PR opened after this merges.

Dropped from the legacy workflow (all harness-owned now):

  • issue_comment, pull_request_review_comment and pull_request_review triggers: @claude in a comment or review body no longer re-triggers a review. A team that wants that should run it as a separate interactive-assistant workflow.
  • --allowedTools list.
  • The "Resolve claude_args" step, including the vars.CLAUDE_REVIEW_MODEL model selection. The repo variable is no longer read; the harness's default model applies. Add model: under with: if a specific model is wanted.
  • --max-turns 10. The harness default is 25. Add max_turns: back if the lower budget was a deliberate cost decision.
  • Draft-skip and @claude-mention if: conditions, concurrency block, and continue-on-error. The harness has its own concurrency group and draft/bot handling.
  • id-token: write (the harness workflow does not request an OIDC token) and issues: read (now issues: write).
  • The inline "Resolve PR number", "Checkout", "Fetch PR diff" and "Build review prompt" steps. The harness fetches the diff and builds the prompt from CLAUDE.md and .claude/review-prompt.md itself. (The legacy step also read AGENTS.md; no AGENTS.md exists in this repo.)

Non-default inputs carried over: none. The only with: input is verdict_policy: deterministic, written explicitly per org policy. There was no previous verdict_policy (legacy workflow), so no behavior change is being overridden. With it, blocking findings request changes, non-blocking findings post as comments, and a clean review approves.

Path filters: none. review / code can be made a required check once findings are trustworthy.

Sub-directory CLAUDE.md files: none (only the root CLAUDE.md), so claude_md_glob is not set.

Local dry-run (skill Step 6) was not run.

Review prompt: the remaining "Review Behavior" section ends with "If the entire PR looks good, do not post a comment at all", which may conflict with the harness's deterministic policy (a clean review posts an APPROVE). The team may want to trim that section; it was left as-is here.

CODEOWNERS: * @customink/fenix covers .github/. Branch protection on master requires 1 approving review; require_code_owner_reviews is false.

Rollout after merge: consider promoting review / code to a required check once findings look trustworthy, and review_mode: blocking if suggestion churn becomes a problem.

🤖 Generated with Claude Code

invacuo and others added 2 commits October 6, 2026 07:53
Replace the standalone claude-code-action review workflow with the thin
caller stub for the shared agent-first-standards PR review harness,
pinned to @v2. Renamed to code-review.yml; job key stays `review`.

Dropped: @claude comment triggers, --allowedTools, the CLAUDE_REVIEW_MODEL
model-selection step, --max-turns 10, draft/comment `if:` conditions,
id-token: write, and the inline prompt/diff assembly steps.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The shared harness base prompt already owns review behavior (when to
comment, no summary-only comments), so the repo prompt keeps domain
conventions only.

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 11:55
@wojtha

wojtha commented Oct 6, 2026

Copy link
Copy Markdown

This repo is going to be archived soon, so I'll merge as it is, without fixing the CI.

@wojtha
wojtha merged commit 94875de into master Oct 6, 2026
1 check failed
@wojtha
wojtha deleted the chore/migrate-pr-review-harness-v2 branch October 6, 2026 12:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants