cursor-review: fast=false, no synchronize, review-now re-request, P0/P1/P2 class rule - #125
Conversation
… fast=false On 2026-09-19 the estate ran 165 cursor-review runs over 80 distinct heads (2.1 launches per PR) and exhausted the Cursor Models pool for seven and a half hours. The callable had no author filter, no path filter, and no notion of how much a pull request is worth reviewing. Three changes, all in the callable so no caller has to reason about them: - `fast` now defaults to FALSE. The job already budgets `wait-minutes` and blocks nobody while it waits, so a reviewer is never the latency-critical lane; implementation lanes keep `fast: true`. - Callers stop listening on `synchronize` and start listening on `labeled`. A push no longer re-reviews; a lane adds `review-now`, the job launches and clears the label again so the next add is a fresh event, and every other label addition (`bot-inbox`, triage labels) skips before any network call. - The P0/P1/P2 class rule decides whether an agent launches at all. P0 is `.github/workflows/**`, `.github/actions/**`, `docs/agents/**`, `AGENTS.md`/`CLAUDE.md`, or the `review-p0` label. P1 is ordinary code. P2 is an automation author (dependabot, github-actions, renovate), a `changeset-release/*` head, a chore/docs/nit-marked title, a metadata-only diff, or the `review-p2` label, and P2 never launches an agent. `review-now` overrides every P2 signal. Unknown never means P2: an unreadable file list, or a diff past the page bound, classifies as reviewable, and the title and metadata-only signals stay silent while the paths are unknown — a "chore:" title on a workflow change is exactly the mismatch that hides a deleted `on:` block. No launch counter and no ledger file: Logan chose the class rule alone over a numeric cap (2026-09-19). The ledger is `gh run list --repo narduk-enterprises/<repo> --workflow cursor-review.yml`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Cursor review of |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-b93ffff1-c833-432e-80df-f6396f65a238 · 5m 49s · head 67894e98a9e9
The callable drops push re-reviews, defaults fast to false, and adds a P0/P1/P2 launch rule with review-now as the re-request. The class-rule tests are internally consistent and the cheap gates I ran are green, but the new labeled trigger is wired too late: job concurrency with cancel-in-progress will cancel an in-flight review when any other label is added, after which the Python skip exits 0 and the PR is left unreviewed. Fix that concurrency/if gate first; then either pin this SHA in cursor-review-self.yml or do not add labeled there while the caller still points at workflows#120.
Findings: 2 blocking, 3 consider, 0 nit (5 posted inline).
Checks the reviewer ran:
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> exit 0
- python3 scripts/lint_callables.py -> 12 files, 0 findings
- python3 scripts/test_cursor_review.py -> 45 tests OK
- remaining 24 scripts/test_*.py (the rest of the local gate) -> all passed
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
…f not the title Answers the five findings of the Cursor review on 67894e9. blocking — an ignored label cancelled the live review. `preconditions()` was the only filter, and it runs long after GitHub has claimed the job's concurrency group: a `bot-inbox` or triage label started a run, cancelled the in-flight opened/ready review, and only then printed its skip, leaving the PR unreviewed. The allow-list moves into the job `if:`, so an ignored label never queues the job and never enters the group. A caller that declares workflow-level concurrency needs the same discrimination in its GROUP NAME, because a run-level cancel happens before any job condition is evaluated; the README caller shape now carries it. blocking — the self-caller was about to ship `labeled` while still pinned to workflows#120, which has no label filter and no class rule, so every label add would have launched a full review. Its trigger set reverts here and changes in the caller wave that pins this commit. The trigger set moves with the pin, never before it. consider — a conventional `chore:`/`docs:`/`nit:` title classified an ordinary code change P2. That is an author-controlled skip of the merge-gating reviewer, and estate lanes use those prefixes on code pull requests daily. The title signal is gone entirely rather than narrowed: with it applied only to metadata diffs it decided nothing the path check had not already decided, and `review-p2` is the honest way to say "nit". consider — a rename carries its old path in `previous_filename`, which `changed_paths()` dropped, so moving `.github/workflows/ci.yml` to `docs/old-ci.md` read as a metadata-only P2. Both names now count. consider — the moved-head skip still promised "the new head gets its own review", which stopped being true when `synchronize` went away. It, the marker comment, and both module docstrings now name the `review-now` re-request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-b82ef555-e68c-4911-b642-e2bf61caf924 · 5m 46s · head b39d17eea19f
The callable stops push re-reviews, defaults fast to false, and adds a P0/P1/P2 launch rule, with b39d17e correctly moving the re-request allow-list into the job if: so a triage label no longer cancels an in-flight review. That direction is sound and the cheap gates I ran are green, but two holes remain against the PR's own invariants. Look first at review_class(): review-p2 and automation-author skips still fire before always-review paths, so a dependabot Actions bump (or a review-p2 label) on .github/workflows/** never launches the merge-gating reviewer.
Findings: 2 blocking, 1 consider, 0 nit (3 posted inline).
Checks the reviewer ran:
- python3 scripts/test_cursor_review.py -q -> 47 tests OK
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_*.py (all 25 scripts) -> each exited 0
- actionlint -> not installed in this environment, not run
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
The title signal was deleted in b39d17e (Cursor review finding 3: a chore:/docs:/nit: title on an ordinary code change was an author-controlled skip of the merge-gating reviewer). Four places still advertised it. Replace each with the reason the title is never a signal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cursor review of b39d17e, two blocking findings: 1. review-p2, automation authors and changeset-release/* heads were all evaluated before the P0 path check, so dependabot bumping a pinned action inside .github/workflows/, or anyone labelling a workflow change review-p2, exited 0 with no agent launched -- the deleted-`on:`-block class this reviewer exists to gate (company-hq D-AGENT-REVIEW-2). Labels may escalate a class, never lower a P0. review-p0 and the critical-path check now run first, and two new tests pin both directions. 2. The caller concurrency group discriminated with !startsWith(label.name, 'review-'), which treats review-p2 as a re-request: the run would claim the -review group, cancel a live review, and then be skipped by the callable's job if:. Same cancel-then-skip hole b39d17e closed for bot-inbox. The group now uses the same explicit allow-list as the job if:. Also: README item 3 said a push cancels and re-reviews the new head, which this PR removes; it now describes the review-now re-request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
loganrenz
changed the title
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-dd59a76a-8709-4dce-8618-c1a7b1ed316e · 6m 14s · head 5bc1dc34c9d4
The callable drops synchronize, defaults fast to false, and adds a P0/P1/P2 launch rule with review-now as the re-request. The earlier cancel-then-skip, self-caller pin, title-skip, rename, and P0-vs-P2 holes are fixed on this head, and the cheap gates I ran are green. Look first at review_class(): when changed_paths() returns None it still applies the automation-author, release-head, and review-p2 skips, so a dependabot workflow change past the 300-file bound (or a files-API failure) is classified P2 and never launches — the same deleted-on:-block class 5bc1dc3 said P0 must always win.
Findings: 1 blocking, 2 consider, 0 nit (3 posted inline).
Checks the reviewer ran:
- python3 scripts/test_cursor_review.py -q -> 49 tests OK
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_*.py (all 25 scripts) -> each exited 0
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> exit 0
- offline reproduction: files API 500 + dependabot[bot] -> skip P2, no POST /v1/agents
- offline reproduction: 3x100 file pages with .github/workflows/ci.yml on page 1 + dependabot[bot] -> skip P2, no POST /v1/agents
- offline reproduction: .github/CODEOWNERS-only and .gitignore-only diffs -> skip P2, no POST /v1/agents
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
…e code Cursor review of 5bc1dc3, one blocking and two consider findings: 1. blocking -- changed_paths() documents that None means UNKNOWN and prints "classifying this pull request as reviewable", but review_class() then applied the review-p2 label, the automation-author and the changeset-release/* head skips anyway. A dependabot pull request whose diff exceeds the three-page bound, or one where the files API fails, was classified P2 and never launched -- the same deleted-`on:`-block class the previous commit said P0 must always win. `paths is None` now returns P1 before any skip, with two tests: every P2 signal against a failing files API, and a diff past the page bound. 2. A CODEOWNERS edit can drop required reviewers and a .gitignore edit can stop ignoring secret material, so neither is prose. METADATA_NAMES keeps LICENSE and NOTICE only. 3. README item 3 recommended dismiss_stale_reviews_on_push, which with `synchronize` gone would let a push clear a REQUEST_CHANGES while nothing launches to replace it -- verify-pr-gate.py would then print green on a head no reviewer saw. The recommendation is dropped and the hazard stated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-f80977cb-79a1-45fa-aab1-421acf7b31c8 · 7m 2s · head 2426cc140ccc
The callable drops synchronize, defaults fast to false, and adds a P0/P1/P2 launch rule with review-now as the re-request. The earlier cancel-then-skip, self-caller pin, title-skip, P0-vs-P2, unknown-file-list, and CODEOWNERS holes are fixed on this head, and the cheap gates I ran are green. Look first at review-p1: it is in the job if and the caller concurrency allow-list as a re-request, but review_class still applies the automation-author and review-p2 skips, so adding it can cancel an in-flight waiter and then exit 0.
Findings: 0 blocking, 3 consider, 0 nit (3 posted inline).
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_cursor_review.py -q -> 52 tests OK
- python3 scripts/test_*.py (all 25 scripts) -> each exited 0
- actionlint -> not installed in this environment, not run
- offline reproduction: labeled review-p1 + dependabot[bot] -> skip P2, no POST /v1/agents
- offline reproduction: labeled review-p1 + README.md-only -> class P1, POST /v1/agents
- offline reproduction: dependabot Skip with a prior agent marker -> cancelled=[], only GET /pulls/7/files
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
Superseded by the Cursor review of 2426cc1.
Cursor review of 2426cc1, three consider findings, all accepted: 1. review-p1 is in RE_REQUEST_LABELS, the job if: and the caller concurrency allow-list, so adding it cancels the in-flight waiter -- but review_class still skipped on dependabot and review-p2, making the add a cancel-then-skip. An explicit class label is a human statement about the pull request; an author name and a branch prefix are guesses, so the label wins. 2. A P2 Skip returned before cancel_previous, leaving the previous head's Cursor agent running with nothing left to post its review. The skip now owes the same cleanup a launch does. 3. The callable's own header still told callers to use an undiscriminated caller group, the bug 5bc1dc3 fixed in the README. Both now carry the same allow-list suffix. 54 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-f5f5a2ce-e750-41bc-b386-451e5eeb1c75 · 5m 54s · head 3d17249a06da
grok-4.6 (effort xhigh). The callable drops synchronize, defaults fast to false, and adds a P0/P1/P2 launch rule with review-now as the re-request. The earlier cancel-then-skip, self-caller pin, title-skip, rename, P0-vs-P2, unknown-file-list, CODEOWNERS, and review-p1 holes are fixed on this head, and the cheap gates I ran are green. Look first at the new skip cleanup: cancel_previous still returns when the marker is for this head, so the same-head leak 3d17249 named is unfixed, and find_marker_comment can turn a contracted-green P2 skip red.
Findings: 0 blocking, 2 consider, 0 nit (2 posted inline).
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_cursor_review.py -q -> 54 tests OK
- python3 scripts/test_*.py (all 25 scripts) -> each exited 0
- actionlint -> not installed in this environment, not run
- offline reproduction: P2 metadata skip + marker for current HEAD -> cancelled=[], exit 0
- offline reproduction: labeled review-now + marker for current HEAD -> cancelled=[], POST /v1/agents
- offline reproduction: P2 skip + comments GET 500 -> ReviewError (job would go red)
- offline reproduction: P2 skip + 5x100 comment pages -> ReviewError exceeded the 5-page bound
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
Cursor review of 3d17249, two consider findings: 1. cancel_previous still exempted a marker whose headSha equalled the current head, so the cleanup 3d17249 added did nothing in the case its own docstring named: review-now is the same-head re-request that replaced synchronize, so the run that most needs the cancel is exactly the one the SHA comparison skipped. The guard is gone -- one agent per pull request at a time -- and test_a_same_head_agent_is_cancelled_too pins both the skip and the review-now relaunch. find_marker_comment could also raise out of the skip handler and turn a contracted-green P2 job red; it is now a notice, pinned by test_a_skip_stays_green_when_the_comments_api_fails. 2. README item 3 claimed review-now cancels the in-flight agent run, which was false under the guard. With the guard dropped it is true, and the sentence now says what is cancelled and why the same head is included. 56 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-2114df0d-1dfb-4eff-a408-b3e011aa42b4 · 7m 28s · head e1f38ae70b70
The callable stops push-driven re-reviews in the recommended caller shape, defaults fast to false, and launches only under a P0/P1/P2 class rule with review-now as the same-head re-request. The earlier cancel-then-skip, unknown-file-list, P0-vs-P2, review-p1, CODEOWNERS, and same-head cancel holes are fixed on e1f38ae, and the cheap gates I ran are green. Look first at the job if: it still treats every non-labeled action as a launch, so a leftover synchronize (every current enrolled caller still has one) will keep burning the pool after a SHA-only pin.
Findings: 0 blocking, 2 consider, 0 nit (2 posted inline).
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_cursor_review.py -q -> 56 tests OK
- python3 scripts/test_*.py (all 25 scripts) -> each exited 0
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> exit 0
- offline: preconditions() accepts synchronize; review_class() P2-skips SKILL.md, harness/claude.md, DECISIONS.md, docs/SECRETS-BULK-READ-BAN.md; review-p2 skips src/a.ts
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
Cursor review of e1f38ae, two consider findings: 1. The job `if:` was a deny-list for unwanted `labeled` events, so `synchronize`, `edited` and a bare `on: pull_request` all passed it, claimed the concurrency group and reached the script. Every enrolled caller still carries `synchronize` today, so a pin-only follow-up would have recreated the 165-run storm this whole change exists to stop. Both the job `if:` and `preconditions()` are now allow-lists of opened / reopened / ready_for_review / a re-request label, and a refused event costs no network call. 2. ALWAYS_REVIEW_NAMES was AGENTS.md/CLAUDE.md, case-sensitive, so `skills/*/SKILL.md`, `harness/claude.md` and `DECISIONS.md` were prose and never launched -- the same miss CODEOWNERS had. The names are now matched case-insensitively and include CODEX.md, SKILL.md and DECISIONS.md; ordinary prose (README.md, CHANGELOG.md, docs/architecture.md) is still P2. 60 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-27cf4cf3-b449-4739-9c0c-6a4e627724ac · 6m 46s · head 6f20f98a34a5
The callable now defaults fast to false, allow-lists the events that may start a review, re-requests via review-now, and applies a P0-first P0/P1/P2 class rule with solid offline tests (unknown diffs fail open, renames keep the old path, review-p1 beats inferred P2, same-head cancel, skip stays green). The class rule and job if: match the stated 2026-09-19 budget tradeoff and D-AGENT-REVIEW-2’s merge-gate shape. Look first at leftover synchronize on enrolled callers: the job if: stops the launch, but a caller that still has synchronize plus undiscriminated cancel-in-progress will kill the opened review on the first push and never run cancel_previous.
Findings: 0 blocking, 2 consider, 0 nit (2 posted inline).
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 files, 0 findings
- python3 scripts/test_cursor_review.py -> 60 tests, OK
- python3 scripts/test_*.py (each file) -> all passed
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> clean; shellcheck was not on PATH so run-block linting was skipped
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
Cursor review of 6f20f98, two consider findings, both documentation: 1. The job `if:` stops the launch but not the caller-level cancel. A caller that keeps `synchronize` and one cancel-in-progress group kills the opened review's waiter before any job condition runs, is then skipped, and so never reaches cancel_previous -- the agent keeps burning the pool and the PR is left unreviewed. The documented caller group now MIRRORS the job `if:` allow-list rather than only excluding unwanted labels, so synchronize, edited and an ignored label all land in one `other` bucket that no live review is ever in. README and the callable header both say a pin-only bump of a caller still listing synchronize is unsafe. 2. verify-pr-gate.py reads reviewDecision and does not bind it to headRefOid, so an APPROVE or COMMENT on head N still greens head N+1 even with dismiss_stale_reviews_on_push OFF. The README said review-now after a push was mandatory only when that setting was on; it is mandatory either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-1fc0281e-f112-44ba-8fcd-87107f4cf160 · 6m 16s · head 127980ff8f78
The callable drops synchronize, defaults fast to false, re-requests via review-now, and launches only under a P0-first P0/P1/P2 rule. On 127980f the earlier holes (job-if allow-list, same-head cancel, unknown file lists, policy basenames, pin-only-bump caller group) are fixed and the cheap gates I ran are green. Look first at clear_review_now sitting after launch(): a usage_limit_exceeded — the failure this change exists to survive — leaves the label on, so the next add is a silent no-op.
Findings: 0 blocking, 3 consider, 0 nit (3 posted inline).
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_cursor_review.py -> 60 tests, OK
- python3 scripts/test_*.py (all 25 scripts) -> each exited 0
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> clean (shellcheck 0.9.0 on PATH)
- offline: labeled review-now + POST /v1/agents 400 usage_limit_exceeded -> labels_removed=[], ReviewError
- offline: PR_LABELS=[review-p2] on default src/a.ts files -> skip P2, no POST /v1/agents
- offline: labeled review-now + PR_OPTED_OUT=true with same-head marker -> cancelled=[], 0 network calls
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
…f the group Cursor review of 127980f, three consider findings: 1. clear_review_now ran only after launch() and upsert_marker() both succeeded, so a usage_limit_exceeded -- the failure this change exists to survive, 77 consecutive refusals on 2026-09-19 -- left the label in place and re-adding it raised no labeled event. The lane would have had no way to ask again. The clear now happens immediately before the launch, pinned by test_review_now_is_cleared_even_when_cursor_refuses. 2. The job if: started on a wake label even for a draft or a no-ai-review pull request, so the add claimed the concurrency group, cancelled a live waiter, and was then refused by preconditions() with no cancel_previous -- the same cancel-then-skip shape. Both conditions are now in the if: as defence in depth; the script keeps its own checks. 3. review-p2 as an author-controlled skip: deferred to the orchestrator, see the thread. no-ai-review already grants exactly that power, so removing the review-p2 skip would not remove the capability, and the class rule Logan approved on 2026-09-19 names P2 explicitly. 61 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-5d4101bc-a98b-4b74-b127-0dd3587d9d3c · 10m 11s · head 96848b2a631a
The allow-list, P0-before-P2 class rule, review-now clear-before-launch, and the extracted-script tests are sound after the cancel-then-skip series. Look first at the documented caller concurrency group: 96848b2 put draft==false and !no-ai-review on the job if: but left the shipped group expression matching only the action/label half, so a wake label on a draft or no-ai-review PR still enters the caller review bucket, cancels the in-flight waiter, then the job skips and cancel_previous never runs. Second, METADATA_SUFFIXES treats every .txt path as prose, so a requirements.txt-only head skips the merge-gating reviewer the CODEOWNERS carve-out was written to keep.
Findings: 0 blocking, 2 consider, 0 nit (2 posted inline).
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 files checked, 0 findings
- python3 scripts/test_cursor_review.py -> 61 tests OK in 0.060s
- remaining scripts/test_*.py (loop, excluding test_cursor_review.py) -> all exited 0
- actionlint -> not installed (command not found), not run
- shellcheck -> not on PATH, not run
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
Cursor review of 96848b2, two consider findings: 1. 96848b2 added draft and no-ai-review to the job if: but not to the documented caller group, so the README's own claim that the group "mirrors the job if: exactly" stopped being true in the commit that made it. A wake label on a now-draft or opted-out PR would enter the caller `review` bucket, cancel the in-flight waiter, and then be skipped -- the cancel-then-skip shape again. Both predicates are now in the group, and WorkflowShapeTests pins the README group line against every predicate in the if:. 2. `.txt` was classified as prose, so a requirements.txt-, constraints.txt- or CMakeLists.txt-only pull request never launched the merge-gating reviewer. Same carve-out as CODEOWNERS and .gitignore. METADATA_SUFFIXES keeps .md/.mdx/.rst. 62 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-a6930db0-8533-474c-a301-53c6dd874d66 · 9m 20s · head 35eed735ee07
The callable drops synchronize, defaults fast to false, re-requests via review-now, and launches only under a P0-first P0/P1/P2 class rule. The job if: allow-list, same-head cancel, unknown-file fail-open, and the 62 extracted-script tests are internally consistent after the cancel-then-skip series, and the cheap gates I ran are green. Look first at clear_review_now still sitting after find_marker_comment: a comments-API failure leaves review-now on, so the next add is a silent no-op — the same hole 96848b2 closed for usage_limit_exceeded.
Findings: 0 blocking, 3 consider, 0 nit (3 posted inline).
Checks the reviewer ran:
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> exit 0 (shellcheck 0.9.0 on PATH)
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_cursor_review.py -q -> 62 tests OK in 0.060s
- remaining 24 scripts/test_*.py (each file, -q) -> each exited 0
- yaml.safe_load of .github/workflows/*.yml -> parsed 12 file(s), 0 bad
- offline: PR_LABELS='"review-now"' + labeled review-now + dependabot[bot] -> skip P2, labels_removed=[], no POST /v1/agents
- offline: PR_LABELS='["review-now"]' + labeled review-now + dependabot[bot] -> launch, labels_removed=['review-now']
- offline: labeled review-now + comments GET 500 -> ReviewError, labels_removed=[], no POST /v1/agents
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
Cursor review of 35eed73, three consider findings: 1. clear_review_now ran after find_marker_comment, which can raise on its own (HTTP 500, or the 5-page bound). The labeled event would then be unconsumed and unrepeatable: re-adding a label already present raises no event. The clear is now the first thing the run does after classification. 2. label_names() returned an empty set for a JSON string, so if the toJSON(labels.*.name) splat ever collapses for a one-label pull request -- the common shape for an agent PR that adds only review-now -- the label would neither override P2 nor consume its own event. A string now reads as one name, and new effective_labels() also counts the `labeled` event's own label, which is on the pull request by definition. Both review_class and clear_review_now use it. 3. The job if: refuses to wake while no-ai-review is on, and removing it is an `unlabeled` event nothing listens for, so a review-now added underneath is never consumed. Documented in README: clear the opt-out first, then add review-now. 64 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-4768f953-011b-41bd-b46b-69cd0da19841 · 6m 53s · head e9cf7b76bf67
The callable now refuses synchronize, defaults fast to false, and launches only for P0/P1 or an explicit wake label under the 2026-09-19 class rule. The job if: allow-list, caller-group mirror, fail-open unknown diffs, and review-now consumption tests are sound; I am not re-opening the deferred review-p2 actor check (workflows#126) or the verify-pr-gate head binding (agent-infrastructure#1589). Look first at ALWAYS_REVIEW_NAMES: the reviewer's own brief is still classified as prose, so a brief-only change never gets the merge-gating review this file exists to run.
Findings: 0 blocking, 3 consider, 0 nit (2 posted inline).
Findings the diff could not anchor (file or line outside this PR's changes):
- consider
scripts/cursor_review.py:806— A preconditions skip leaves review-now on the PR
e9cf7b7 moved clear_review_now to the first step after classification so a failed launch still consumes the labeled event (test_review_now_is_cleared_even_when_cursor_refuses, test_review_now_is_cleared_before_anything_that_can_fail). A preconditions Skip still returns here first, with no GitHub client and no DELETE. The job if: does not filter forks or a missing CURSOR_CLOUD_AGENTS_API_KEY, so a review-now add on a fork or on a caller that does not yet have the secret stays on the PR. Adding the label again raises no event. test_each_skip_reason_is_green_and_launches_nothing pins those skips to zero network calls, which is why this is untested. On a review-now event, build GitHub with GITHUB_TOKEN (present on both skips) and call clear_review_now before returning; keep Cursor calls at zero.
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 files, 0 findings
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> exit 0 (pinned binary; shellcheck was not on PATH)
- python3 scripts/test_cursor_review.py -> 64 tests, OK
- all scripts/test_*.py -> OK
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
…ew-now Cursor review of e9cf7b7, two consider findings: 1. scripts/cursor_review_brief.md is the merge-gating reviewer's instruction contract (untrusted-data rule, verdict vocabulary) and was classified prose by its .md suffix, so a brief-only pull request would never launch once this repo's caller pins the callable. Added to ALWAYS_REVIEW_NAMES and pinned in test_policy_markdown_is_never_prose. 2. The failed-review marker still told the lane to push a new head, the trigger this PR removes. It now says what the Skip sibling says: re-run the job or add review-now. Pinned both ways in the errored-run test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor review · grok-4.6 (effort xhigh, fast true) · class P0 · agent bc-d9e5bdeb-e4d8-4b6d-b12e-fe1ae8054525 · 6m 32s · head 3c7456678e0b
The callable drops synchronize, defaults fast to false, re-requests via review-now, and launches only under a P0-first P0/P1/P2 class rule. On 3c74566 the earlier cancel-then-skip, unknown-file, P0-vs-P2, same-head cancel, allow-list, and brief-as-prose holes are fixed, and the cheap gates I ran are green. I am not re-opening the deferred review-p2 actor check (workflows#126) or the verify-pr-gate head binding (agent-infrastructure#1589). Look first at clear_review_now: a preconditions skip still leaves review-now on the PR, so the next add is a silent no-op.
Findings: 0 blocking, 1 consider, 1 nit (2 posted inline).
Checks the reviewer ran:
- python3 scripts/lint_callables.py -> 12 file(s) checked, 0 finding(s)
- python3 scripts/test_cursor_review.py -q -> 64 tests OK in 0.060s
- remaining scripts/test_*.py (each file, -q) -> each exited 0
- actionlint 1.7.12 -no-color -oneline .github/workflows/*.yml -> exit 0 (pinned binary; shellcheck was not on PATH so run-block linting was skipped)
- offline: labeled review-now + fork head -> skip, labels_removed=[], 0 network calls
- offline: labeled review-now + empty CURSOR_CLOUD_AGENTS_API_KEY -> skip, labels_removed=[], 0 network calls
How to respond: fix and push, or reply in the thread with disposition: <accept|reject|defer> - <reason> and resolve it. A push no longer re-reviews on its own — add the review-now label when the new head needs another review. A blocking finding requests changes; the merge gate honours GitHub's review decision.
| # 2026-09-19), and `find_marker_comment` can raise on its own; if the label | ||
| # outlived either, re-adding it would raise no `labeled` event and the lane | ||
| # would have no way to ask again. | ||
| clear_review_now(github, number, env) |
There was a problem hiding this comment.
consider — A preconditions skip leaves review-now on the PR
clear_review_now is the consumption for the new labeled re-request, and it now runs before launch/find_marker_comment so usage_limit_exceeded still consumes the event (test_review_now_is_cleared_even_when_cursor_refuses, test_review_now_is_cleared_before_anything_that_can_fail). A preconditions Skip still returns earlier, with no GitHub client and no DELETE. The job if: does not filter forks or a missing CURSOR_CLOUD_AGENTS_API_KEY, so a review-now add on a fork (public callers never get the secret) or on a caller that does not yet have the secret stays on the PR. Adding the label again raises no event. test_each_skip_reason_is_green_and_launches_nothing pins those skips to zero network calls, which is why this is untested. Offline: labeled review-now + fork, and labeled review-now + empty key, both exit 0 with labels_removed=[]. On a review-now event, build GitHub with GITHUB_TOKEN (present on both skips) and call clear_review_now before returning; keep Cursor calls at zero.
| the inferred P2 signals too. Inside that, Logan's answer of | ||
| 2026-09-19 governs volume, in his words: *"No numeric cap, only the P0/P1/P2 | ||
| class rule"*. P0 (`.github/workflows/**`, `.github/actions/**`, `docs/agents/**`, a policy | ||
| basename — `AGENTS.md`, `CLAUDE.md`, `CODEX.md`, `SKILL.md`, `DECISIONS.md`, |
There was a problem hiding this comment.
nit — P0 policy-name list omits cursor_review_brief.md
3c74566 added cursor_review_brief.md to ALWAYS_REVIEW_NAMES and test_policy_markdown_is_never_prose because the brief is the merge-gating reviewer's instruction contract. The shipped P0 lists in README (this line) and the callable header still name only AGENTS.md/CLAUDE.md/CODEX.md/SKILL.md/DECISIONS.md. Behavior is correct; the copies will rot the next time someone treats the list as complete. Add the basename to both lists, or have WorkflowShapeTests pin those names against ALWAYS_REVIEW_NAMES.
Proposed change:
basename — `AGENTS.md`, `CLAUDE.md`, `CODEX.md`, `SKILL.md`, `DECISIONS.md`, `cursor_review_brief.md`,
… set (#127) The trigger change was deliberately held back from #125 so it would not ship `labeled` against the old callable, which had no label filter and would have launched a full review on every label add. This is the caller-wave commit that pin says to make. A push no longer re-reviews; a lane adds `review-now`. The concurrency group mirrors the callable's job `if:`. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 participants
What
The Cursor reviewer stops re-reviewing on every push, stops reviewing work nobody is waiting on, and drops the
fastpremium.On 2026-09-19 the estate ran 165
cursor-reviewruns over 80 distinct heads (2.1 launches/PR, 8x the previous day) and exhausted the Cursor Models pool for seven and a half hours — 77 consecutivePOST /v1/agents -> HTTP 400 usage_limit_exceeded. The callable had no author filter, no path filter, and no notion of how much a pull request is worth reviewing. Evidence:~/.agents/programs/master/reports/reviewer-untangle-1853.md§1.Changes
All three live in the callable, so no caller has to reason about them.
fastdefaults tofalse. The job already budgetswait-minutesand blocks nobody while it waits, so a reviewer is never the latency-critical lane. Implementation lanes keepfast: true.synchronizeout,labeledin. A push no longer re-reviews. A lane adds thereview-nowlabel; the job launches, then clears the label so the next add is a freshlabeledevent. Every other label addition (bot-inbox, triage labels) skips before any network call..github/workflows/**,.github/actions/**,docs/agents/**,AGENTS.md/CLAUDE.md, or thereview-p0label. Always reviewed, never deferred.dependabot[bot],github-actions[bot],renovate[bot]), achangeset-release/*head, a metadata-only diff, or thereview-p2label. Never launches an agent; the orchestrating session self-reviews it.review-nowoverrides every P2 signal, and P0 is decided first: a label or an automation author may escalate a class, never lower a P0. Dependabot bumping a pinned action inside.github/workflows/, or areview-p2label on a workflow change, is still reviewed.chore:/docs:on code pull requests every day, and a free skip of a merge-gating reviewer is exactly the wrong lever to hand an author.Unknown never means P2. An unreadable file list or a diff past the page bound classifies as reviewable, and the metadata-only signal stays silent while the paths are unknown — hiding a deleted
on:block behind an unreadable file list is exactly the failure this reviewer exists to catch (docs/agents/pr-review-and-merge.md).No launch counter and no ledger file, on purpose. The ledger is one line:
gh run list --repo narduk-enterprises/<repo> --workflow cursor-review.yml.This repo's own caller (
cursor-review-self.yml) takes the new trigger set in the same commit; the other enrolled callers follow in their own PRs pinned to this merge SHA.Authorization
Merge default: company-hq D-ORG-1 (g), 2026-09-02.
Logan's verbatim answers, 2026-09-19 (askme round 4):
Validation
actionlint -no-color -oneline .github/workflows/*.yml— cleanpython3 scripts/lint_callables.py— 12 files, 0 findingsscripts/test_*.pygreen;scripts/test_cursor_review.pygrew from 37 to 62 tests, addingClassRuleTests: P2 launches nothing across every signal,review-nowandreview-p1defeat each of them, P0 outranks every P2 skip signal in both directions, an unknown or page-bounded file list fails open to P1, a rename out of an always-review path is still P0, achore:title on a code change is still P1, policy markdown (SKILL.md,harness/claude.md,DECISIONS.md) and build input (requirements.txt,CMakeLists.txt) andCODEOWNERS/.gitignoreare never prose whileREADME.md/CHANGELOG.mdstill are,synchronizeand every other non-review action cost zero network calls,review-nowis cleared even when Cursor refuses withusage_limit_exceeded, a skipped class still cancels the previous agent and still exits 0 when the comments API fails, andWorkflowShapeTestspins the README caller group against every predicate in the jobif:.🤖 Generated with Claude Code
Review loop
Eight Cursor rounds on this branch, every finding dispositioned in-thread and every thread resolved: 3 blocking and 15 consider accepted, 1 consider deferred with reasons. The blocking three were a cancel-then-skip that killed live reviews, P2 skip signals outranking P0 paths, and unknown file lists taking inferred P2 skips.
Two follow-ups were filed rather than folded in:
verify-pr-gate.pyreadsreviewDecisionwithout binding it toheadRefOid, so an APPROVE on head N greens head N+1. Mitigated here by documentingreview-now-after-every-push as mandatory either way.no-ai-reviewandreview-p2de-escalate with no actor check. One policy, needs Logan's answer, not a quiet edit.