Skip to content

chore(ci): delegate AI review to the org-shared reusable workflow - #122

Merged
starknetdev merged 3 commits into
mainfrom
chore/shared-code-review
Jul 29, 2026
Merged

chore(ci): delegate AI review to the org-shared reusable workflow#122
starknetdev merged 3 commits into
mainfrom
chore/shared-code-review

Conversation

@starknetdev

@starknetdev starknetdev commented Jul 29, 2026

Copy link
Copy Markdown
Member

Summary

Ports this repo to the org-shared review workflow that denshokan-sdk already uses. Four hand-rolled review jobs (claude-review-packages, codex-review-packages, claude-review-general, codex-review-general) collapse into one call to Provable-Games/.github/.github/workflows/code-review.yml@v1.

Review logic — matrix build, prompt assembly, the codex sandbox workaround, comment upserting, Claude transcript extraction — plus model and effort config now live in one place. A model deprecation becomes an org-variable change rather than an edit in every repo. This repo supplies only .github/review-agents.json and the existing .github/prompts/*.md, which are unchanged.

Net −577 lines in pr-ci.yml.

Changes

  • .github/review-agents.json (new) — two agents matching the old review scopes: packages (Cairo review over packages/) and general (everything else).
  • changes job — drops the packages_review / general_review / review_automation_changed filters and the whole "Determine AI review availability" step. The shared workflow owns fork skipping. The test-matrix computation is untouched.
  • review job — the shared-workflow call. Grants issues: write, pull-requests: write, id-token: write, since a reusable workflow's job permissions can't exceed the caller's and the default token is read-only.
  • pr-ci aggregate — now needs review instead of the four review jobs, and is back to a plain did-any-job-fail check.

Upstream fixes this PR depends on

Porting surfaced two gaps in the shared workflow, both now fixed and live at v1:

  • fix(code-review): make the documented "." catch-all actually match .github#1 — the documented diff_paths: ["."] catch-all only matched dotfile paths, because entries are compared with startswith(). A repo following the README got no review at all on a README.md-only PR, silently. This PR briefly enumerated every top-level path as a workaround; now that the matcher is fixed, general uses the documented two-line catch-all again. That also picks up files the enumeration missed — LICENSE and docs/** matched no agent under the workaround.
  • feat(code-review): add opt-in block_on_severity merge gate .github#2 — the shared workflow never failed a build on findings, so the port would have traded "a [CRITICAL] blocks merge" for "a [CRITICAL] is a comment". It now takes an opt-in block_on_severity input, and this PR sets it true.

Agent matching under the current v1 matcher:

changed files agents
packages/embeddable_game_standard/… packages
README.md, Scarb.toml, codecov.yml, LICENSE, docs/** general
.github/**, .tool-versions, scripts/**, src/** general
packages/… + README.md packages, general

Behavior changes to be aware of

  • Check names change. claude-review-packages etc. become Claude - Cairo Packages, Claude - General Engineering, Codex - …. If any of the old names are required status checks in branch protection, they need updating — pr-ci still aggregates everything and is unaffected.
  • Severity gating is preserved via block_on_severity: true, matching the inline jobs' grep -qE '\[(CRITICAL|HIGH)\]' && exit 1.
  • The missing-secrets guard is gone. The old changes job failed the build when CLAUDE_CODE_OAUTH_TOKEN / CODEX_AUTH_DOT_JSON were unavailable on a first-party PR with reviewable changes. The shared workflow has no equivalent signal to key that off, so a secrets outage now degrades to reviews silently not running rather than a red build.
  • review has no needs: — deliberately. A skipped dependency skips its dependents, and lint is conditional on packages/ changing, so gating review on it would drop reviews on docs-only PRs. This matches denshokan-sdk.

Test plan

  • npx yaml@2.8.1 valid on pr-ci.yml — the same validator infra-validate runs
  • jq -e . on review-agents.json
  • Parsed with js-yaml and asserted job graph: review resolves to the shared workflow with the right permissions, pr-ci.needs includes review
  • No dangling references to the removed changes outputs
  • Both prompt_file paths exist
  • Agent matching simulated with the real matcher from code-review.yml@v1 (table above)

The end-to-end proof is this PR's own checks: it touches .github/** only, so general should run and packages should not.

🤖 Generated with Claude Code

Replaces four hand-rolled review jobs (claude/codex x packages/general)
with a single call to Provable-Games/.github code-review.yml@v1. Review
logic, prompt assembly, comment upserting and model config now live in
one place, so a model deprecation is an org-variable change rather than
an edit here. This repo supplies only .github/review-agents.json plus
the existing .github/prompts/*.md.

Net -577 lines. Also drops the local AI-availability gate: the shared
workflow owns fork skipping, so the `changes` job no longer computes
can_run_ai_reviews and the pr-ci aggregate is back to a plain
did-any-job-fail check.

The general agent enumerates top-level paths rather than using the
documented ["."] catch-all: the shared workflow's matcher is prefix
based, so ["."] only matches dotfiles and would silently skip review on
README.md / Scarb.toml / codecov.yml-only PRs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@starknetdev, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 16 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ef281e4a-0562-47b8-9f02-c52f9f104bc7

📥 Commits

Reviewing files that changed from the base of the PR and between e24bf41 and d2a591b.

📒 Files selected for processing (2)
  • .github/review-agents.json
  • .github/workflows/pr-ci.yml

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

Claude Code Review

Now I have enough to assess. Let me analyze the key concern: the PR body's own table shows general agent matching for .github/** etc., but the JSON uses diff_paths: ["."] + exclude_paths: ["packages/"]. The PR body claims the matcher was fixed so "." catches everything.

However there's a notable discrepancy: the PR body describes the workaround using enumeration and then "now uses the documented two-line catch-all again" — but the JSON here has both diff_paths: ["."] and exclude_paths: ["packages/"]. The general agent overlapping with packages is intentional per the table (both run when both change). The exclude_paths for general is what prevents general from reviewing packages files.

The main verifiable risk: exclude_paths is a field whose support I cannot confirm against the upstream schema without fetching it. If unsupported, general would review packages/ files too — but that's a soft concern. Let me focus on concrete, verifiable findings.

Let me double check the critical behavior-change the PR itself admits: the missing-secrets guard removal, and whether pr-ci still handles review being skipped correctly.

The pr-ci job now only fails on failure/cancelled. If review is skipped (e.g., fork, or the shared workflow skips), pr-ci passes. That matches the documented intent. No bug there.

Let me assess the findings I can concretely back with evidence in the allowed files.

Based on my review of the two scoped files:

Analysis summary:

  • pr-ci.yml: changes job intact, no dangling refs to removed outputs, review job correctly wired into pr-ci.needs, permissions/secrets rationale documented inline. YAML structure sound.
  • review-agents.json: valid JSON, both prompt_file paths exist on disk, diff_paths/exclude_paths match the intended scoping.

The one thing I cannot independently verify (WebFetch not permitted) is whether the upstream code-review.yml@v1 schema honors exclude_paths and the block_on_severity input — but these are described as fixed/live upstream in the PR body, and I have no contradicting evidence in the scoped files.

I have one actionable finding backed by evidence in the diff:

[LOW] .github/workflows/pr-ci.yml:42-45 - The infra_ci paths-filter still lists .github/prompts/** and .github/workflows/**, which previously fed the now-deleted "review_automation_modified" gate (that gate self-disabled AI review when review automation changed). With that logic removed, this PR (which edits .github/workflows/pr-ci.yml) will now run the review job against its own uncommitted-to-v1 config — exactly the self-review the old gate prevented. This is the intended new behavior per the PR, so it's informational: confirm you're comfortable losing the "don't let a PR review-gate itself with its own modified prompts/workflow" protection. infra_ci itself is still correctly consumed by infra-validate, so no dangling output.

Impact: A PR that weakens/breaks a review prompt no longer bypasses gating; the modified prompt is used to review the same PR. Acceptable if intentional.
Fix: None required if intentional. Otherwise, re-add a guard input to the shared workflow call to skip review when infra_ci == 'true'.

Everything else in the scoped diff (removed dead outputs, review wiring, pr-ci aggregation, review-agents.json structure and prompt paths) checks out.

@github-actions

github-actions Bot commented Jul 29, 2026

Copy link
Copy Markdown

GPT Code Review

[MEDIUM] .github/workflows/pr-ci.yml:430 - secrets: inherit forwards every caller secret to the reusable workflow, not just the two review credentials this job needs.
Impact: Any upstream change at the mutable @v1 ref can access unrelated repo/org secrets during first-party PR runs.
Fix: Pass only the required secrets explicitly: CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} and CODEX_AUTH_DOT_JSON: ${{ secrets.CODEX_AUTH_DOT_JSON }}. GitHub docs: https://docs.github.com/en/enterprise-cloud@latest/actions/how-tos/reuse-automations/reuse-workflows#passing-inputs-and-secrets-to-a-reusable-workflow

[MEDIUM] .github/workflows/pr-ci.yml:447 - The aggregate now accepts a skipped review job and the removed caller-side secret guard is not replaced.
Impact: On first-party PRs, a missing/rotated review secret can skip AI review while pr-ci still passes, so the required review gate silently disappears.
Fix: Add a small caller-side preflight job for non-fork, non-trusted-bot PRs that fails when either review secret is empty, and include it in pr-ci.needs.

starknetdev and others added 2 commits July 29, 2026 09:55
Both depend on shared-workflow changes that have now landed at v1.

review-agents.json: the general agent goes back to the documented
diff_paths ["."] + exclude_paths ["packages/"]. The enumerated top-level
list was a workaround for the prefix matcher treating "." as a dotfile
prefix (fixed in Provable-Games/.github#1), and it silently missed any
new root file — LICENSE and docs/** matched no agent at all.

pr-ci.yml: block_on_severity: true restores the merge gate the inline
review jobs had (Provable-Games/.github#2). Without it a [CRITICAL]
finding would only post a comment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both were flagged as reproducibility/least-privilege risks in review.
Keeping them, deliberately: @v1 is the org convention every caller uses
and is what makes an upstream model change a one-place fix, and the
callee is first-party and declares exactly the two secrets it uses.
Pinning a SHA or enumerating secrets here would diverge from the other
six callers for no practical gain.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@starknetdev
starknetdev merged commit d26f27f into main Jul 29, 2026
10 checks passed
starknetdev added a commit that referenced this pull request Aug 5, 2026
Brings the branch up to date with v1.1.11 and, more usefully, onto the
org-shared AI review workflow adopted in #122 — this branch predates it and
was still running the retired hand-rolled review jobs.
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.

1 participant