ci: enroll this repo in its own cursor-review and unreviewed-merge gates#37
Conversation
github-workflows ships the org's review panel, unreviewed-merge detector, and AGENTS.md integrity check — and until now consumed none of them: PRs here merged with no AI review and no compliance trail, in the one repo whose files become every other repo's required checks. Adds two thin callers, SHA-pinned like any external consumer (a local uses: path would run the PR's own copy of the reviewer — pinning keeps the panel tamper-proof): - ci-cursor-review.yml — direct pull_request trigger (no label dance; every non-draft PR here is worth a review). CURSOR_API_KEY is already provisioned. - ci-detect-unreviewed-merge.yml — verbatim template copy, renamed because the canonical path hosts the reusable itself. Needs UNREVIEWED_MERGES_TOKEN before it goes green; fails loudly until then. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
… reusable gate The gate only honors label events, so the self-caller's direct pull_request trigger ran the shell and skipped the panel (verified live on this PR's first push). Two-step fix, pin-ordering aware: - cursor-review.yml gains an opt-in `run_without_label` input (default false — zero behavior change for existing callers): plain PR events (opened/reopened/ready_for_review/synchronize) fire the panel, with the fork skip, skip-cursor-review veto, and same-commit dedupe all still applying. For repos with no labeling automation. - ci-cursor-review.yml switches to the sibling label-triggered shape, which works at its current pin (the pinned SHA predates the new input, so passing it would be an invalid-input error). When the auto-bump advances the pin past this commit, the bump PR should add `run_without_label: true` and widen the trigger types — noted in the file header. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mattmillerai
added
cursor-review
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Cursor Review — Consolidated panel
Triggered by @mattmillerai.
Found 7 finding(s).
| Severity | Count |
|---|---|
| 🟡 Medium | 5 |
| 🟢 Low | 2 |
Panel: 6/8 reviewers contributed findings.
Reviewers that did not contribute: kimi-k2.5:adversarial (empty), kimi-k2.5:edge-case (empty)
| # veto — and the same-commit dedupe below still prevents re-reviews. | ||
| if [ "$RUN_WITHOUT_LABEL" = "true" ]; then | ||
| case "$GH_EVENT_ACTION" in | ||
| opened|reopened|ready_for_review|synchronize) |
There was a problem hiding this comment.
🟡 Medium — In run_without_label mode the gate matches opened/synchronize (plus reopened/ready_for_review) with no github.event.pull_request.draft check, and GitHub fires opened/synchronize for draft PRs. Enrolled repos will run the full 8-cell panel + judge on every draft and every push to a draft, burning paid CURSOR_API_KEY quota and contradicting the caller comment's stated 'every non-draft PR' intent. Add a draft guard (or force callers to trigger only on ready_for_review). Raised by 5 of 8 reviewers (claude-opus-4-8-thinking-max adversarial, gemini-3.1-pro adversarial, gpt-5.6-sol-max edge-case, claude-opus-4-8-thinking-max edge-case, gemini-3.1-pro edge-case).
| # removed or re-applied. Scoping by label name ensures that removing an | ||
| # unrelated label does not kill a running cursor review. | ||
| concurrency: | ||
| group: cursor-review-pr-${{ github.event.pull_request.number }}-${{ github.event.label.name }} |
There was a problem hiding this comment.
🟡 Medium — The concurrency group embeds github.event.label.name, so a run started by the cursor-review label (group ...-cursor-review) and the labeled event adding skip-cursor-review (group ...-skip-cursor-review) fall into different groups. With cancel-in-progress: true the veto label therefore cannot cancel an already-running review, which keeps consuming secrets and posts anyway. In run_without_label mode it is worse: non-label events like synchronize produce an empty label segment (...-) that won't serialize against label events either. Raised by 3 of 8 reviewers (gpt-5.6-sol-max adversarial, gemini-3.1-pro edge-case, gpt-5.6-sol-max edge-case).
| # label events. Once the pin advances past the commit adding the reusable's | ||
| # `run_without_label` input, the auto-bump PR for this file should also add | ||
| # `run_without_label: true` under `with:` and widen the trigger types to | ||
| # [opened, reopened, ready_for_review, labeled, unlabeled] — this repo has no |
There was a problem hiding this comment.
🟡 Medium — The recommended future trigger list [opened, reopened, ready_for_review, labeled, unlabeled] omits synchronize, yet the reusable's run_without_label gate and the run_without_label input description both explicitly support synchronize. Because specifying types overrides GitHub's defaults, an auto-bump PR that applies this guidance would never re-review new commits pushed to an open PR, leaving the latest code unanalyzed. Raised by 3 of 8 reviewers (gpt-5.6-sol-max adversarial, gemini-3.1-pro edge-case, gpt-5.6-sol-max edge-case).
| # labeling automation opt in to running on plain PR events. Sits | ||
| # BELOW the fork skip and skip-cursor-review checks — those still | ||
| # veto — and the same-commit dedupe below still prevents re-reviews. | ||
| if [ "$RUN_WITHOUT_LABEL" = "true" ]; then |
There was a problem hiding this comment.
🟡 Medium — Same-repo Dependabot PRs pass the fork check, but GitHub withholds Actions secrets and downgrades GITHUB_TOKEN for Dependabot events. When a caller enables run_without_label, this branch sets should_run=true for Dependabot opened/synchronize events, so the panel runs with an empty CURSOR_API_KEY and the review job fails on every Dependabot update — blocking merges if it is a required check. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).
| :!**/*-lock.json | ||
| :!**/dist/** | ||
| secrets: | ||
| CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }} |
There was a problem hiding this comment.
🟡 Medium — CURSOR_API_KEY is handed to a cursor-agent --trust process that ingests attacker-controlled same-repository PR content; the fork check only blocks forks, not same-repo contributors. A contributor who can push a branch (and, in label mode, apply the trigger label) could use prompt injection or a malicious workspace config to execute commands and exfiltrate the key — a risk run_without_label broadens by auto-triggering on any same-repo PR. Raised by 1 of 8 reviewers (gpt-5.6-sol-max adversarial).
| # BELOW the fork skip and skip-cursor-review checks — those still | ||
| # veto — and the same-commit dedupe below still prevents re-reviews. | ||
| if [ "$RUN_WITHOUT_LABEL" = "true" ]; then | ||
| case "$GH_EVENT_ACTION" in |
There was a problem hiding this comment.
🟢 Low — In run_without_label mode this case handles only opened/reopened/ready_for_review/synchronize, so an unlabeled event (e.g. removing skip-cursor-review to unblock a PR) falls through to the legacy check that requires labeled + the trigger label and no-ops. Since the recommended trigger types include unlabeled, removing the veto label silently fails to re-trigger a review until another qualifying event occurs. Raised by 3 of 8 reviewers (gpt-5.6-sol-max adversarial, gemini-3.1-pro edge-case, gpt-5.6-sol-max edge-case).
| # labeling automation opt in to running on plain PR events. Sits | ||
| # BELOW the fork skip and skip-cursor-review checks — those still | ||
| # veto — and the same-commit dedupe below still prevents re-reviews. | ||
| if [ "$RUN_WITHOUT_LABEL" = "true" ]; then |
There was a problem hiding this comment.
🟢 Low — run_without_label lets multiple activity types fire on one HEAD SHA (e.g. opened then synchronize, or open-as-draft then ready_for_review); the same-commit dedupe is a separate, non-atomic step, so two near-simultaneous events can both pass it (neither has posted yet) and emit duplicate reviews. The reusable defines no concurrency of its own and the example caller group keyed on github.event.label.name is empty for these non-label events, so rapid synchronize pushes can also queue unbounded expensive panels — run_without_label callers need explicit concurrency guidance. Raised by 2 of 8 reviewers (claude-opus-4-8-thinking-max adversarial, claude-opus-4-8-thinking-max edge-case).
1 participant
What
github-workflows ships the org's cursor-review panel, its unreviewed-merge detector, and (since #34) the AGENTS.md integrity check — and consumed none of them on itself. PRs here (e.g. #34, #36) merge with no AI review, no approval requirement, and no compliance trail, in the one repo whose files become every other repo's required checks. This enrolls the repo in its own gates:
ci-cursor-review.yml— SHA-pinned to this repo's own merged main, exactly like an external consumer. A localuses: ./…path would run the PR's own copy of the reviewer, letting a PR weaken the panel judging it — pinning keeps the reviewer tamper-proof. Label-triggered for now (the pinned gate only honors label events — verified live on this PR's first push, where a direct trigger ran the shell and skipped the panel).cursor-review.ymlgains an opt-inrun_without_labelinput (default false — zero change for existing callers): plain PR events fire the panel, with the fork skip,skip-cursor-reviewveto, and same-commit dedupe still applying. Once the auto-bump advances this repo's own pin past this merge, the bump PR should flip the caller torun_without_label: trueand widen its trigger types (noted in the caller header) — this repo has no labeling automation.ci-detect-unreviewed-merge.yml— verbatim copy of the canonical template fromComfy-Org/unreviewed-merges/detector/, renamed because the canonical filename is occupied here by the reusable itself. Operator step remaining: provisionsecrets.UNREVIEWED_MERGES_TOKEN— until then the job fails loudly on pushes to main (deliberate: enrollment should never be silently inert).Housekeeping already done outside this PR:
CURSOR_API_KEYrepo secret provisioned; repo added toCURSOR_REVIEW_CALLERS(13 entries).🤖 Generated with Claude Code