Skip to content

Commit 30cccb9

Browse files
mattmilleraiclaude
andauthored
feat(cursor-review): probe the direct Anthropic key and fall back to Cursor when it is capped (#431)
* feat(cursor-review): probe the direct Anthropic key before a round and fall back to Cursor when it is capped A key whose organization reached its usage limit is present but refused, so every direct cell, the direct judge and the direct axis runner errored and rounds ended with no decision until someone deleted the caller's direct-model variables by hand. The workflows checked presence only. - gate: `Probe the direct Anthropic key` makes one 1-token Messages call per configured model (hosted, no checkout, never prints the key or a header) and classifies it: 2xx usable; the usage-limit 400, billing, 401/403 unusable; 429/529/5xx/no answer retried twice then unusable; 404 or a 400 naming an unknown model invalid. Best effort: a crash leaves the outputs empty, which preflight reads as "not probed". - preflight: unusable reads exactly like an absent key (the Cursor `claude-*` cells and the Cursor judge run), with a warning per lane and a job summary line; invalid fails the run, never a fallback on a typo. - post-review: `--runner-note` says under the review header that the direct lanes ran on Cursor this round. - cursor-axis-base: a new `axis-probe` job (hosted, no checkout) runs the same byte-identical probe and picks the Cursor `axis` job or `axis-direct`; an absent key, a non-plain or unknown id, a non-checkout axis and context_sources still fail the axis. - docs: cursor-review and cursor-approve caller guides and the cursor-review README describe the fallback and how to see it in a run. No new inputs or secrets: today's callers work unchanged. * fix(cursor-review): name each fallen lane's own reason in the fallback note When the cells and judge models fail the probe differently (billing vs. unavailable), the note named the first lane's reason for both. One shared reason still reads once; differing reasons are listed per lane. Applied to both byte-identical probe copies (gate + axis-probe). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cursor-review): harden the Anthropic key probe per the panel's review round - Write every probe output once, at the end, and catch every exception per call, so a probe that dies midway leaves all lanes unprobed instead of one lane decided and the note missing. Retries also stop at a wall-clock PROBE_BUDGET_SECONDS inside the step timeout (urlopen's timeout bounds a socket op, not a request). - Refuse redirects: the request carries x-api-key, so a 3xx is now an unusable refusal, never followed to another host. - `invalid` is a 404 / not_found_error only; a 400's wording is no longer guessed at. Any other refusal still falls back, now documented. - A dash-free CLI model alias (`opus`, `sonnet`) that the raw API 404s is not probed, so a config the `--model` CLI resolves keeps working. - preflight checks the openai/anthropic id collision before a probe verdict can fail the run. - When the cells fall back to a panel list with no Cursor claude-* id, the warning, job summary and posted note now say no Anthropic cell ran. - axis-direct's comment names axis-probe as the other key-holding job. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cursor-review): publish only the probe's reason class, never the API's raw message The fallback note lands in the posted PR review, and the per-lane reasons in ::warning:: lines and job summaries, all world-readable on a public repo. An Anthropic 429 message can name the organization and a billing one its credit state, and clean() redacts only the exact key. Keep the reason class plus, for a usage limit, the parsed reset time; drop the rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 68dd712 commit 30cccb9

10 files changed

Lines changed: 994 additions & 38 deletions

File tree

‎.github/cursor-approve/tests/test_axis_direct.py‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@
55
66
* plumbing — the wrappers declare the input (default '') and the optional
77
ANTHROPIC_API_KEY, and pass both to the base unchanged;
8-
* exactly one runner — the Cursor job is gated on the input being empty, the
9-
separate `axis-direct` job (hard-pinned to `ubuntu-latest`) on it being
10-
set, and nothing falls back;
8+
* exactly one runner — the Cursor job runs when the input is empty or
9+
`axis-probe` found the direct key unusable, the separate `axis-direct` job
10+
(hard-pinned to `ubuntu-latest`) when the probe picked direct; nothing
11+
falls back once the direct runner runs;
1112
* confinement — the cells' flags: Read/Grep/Glob, `--restricted`,
1213
`--setting-sources ""`, /proc and /sys denied, never `--bare`;
13-
* the key is referenced by the `axis-direct` job alone, never the Cursor job;
14+
* the key is referenced by the `axis-direct` and `axis-probe` jobs alone,
15+
never the Cursor job;
1416
* usage — `usage-direct-axis-<axis>-<model>`, the cells' script byte for byte
1517
but for the result path, best effort and time-boxed;
1618
* the API-error diagnostic — agent-error.py runs on the OUTCOME, not on the
@@ -30,8 +32,10 @@
3032
HERE = os.path.dirname(os.path.abspath(__file__))
3133
ROOT = os.path.normpath(os.path.join(HERE, "..", "..", ".."))
3234
WORKFLOWS = os.path.join(ROOT, ".github", "workflows")
33-
DIRECT_IF = "inputs.axis_direct_model != ''"
34-
CURSOR_IF = "inputs.axis_direct_model == ''"
35+
PROBE_IF = "inputs.axis_direct_model != ''"
36+
DIRECT_IF = "${{ !cancelled() && needs.axis-probe.outputs.runner == 'direct' }}"
37+
CURSOR_IF = ("${{ !cancelled() && (inputs.axis_direct_model == '' "
38+
"|| needs.axis-probe.outputs.runner == 'cursor') }}")
3539

3640

3741
def read(name):
@@ -68,6 +72,7 @@ def run_block(step_text):
6872
BASE = read("cursor-axis-base.yml")
6973
CURSOR_JOB = job(BASE, "axis")
7074
DIRECT_JOB = job(BASE, "axis-direct")
75+
PROBE_JOB = job(BASE, "axis-probe")
7176
# Without comment lines, for the "never mentioned" checks below.
7277
CURSOR_CODE = "\n".join(ln for ln in CURSOR_JOB.split("\n") if not ln.lstrip().startswith("#"))
7378
DIRECT_CODE = "\n".join(ln for ln in DIRECT_JOB.split("\n") if not ln.lstrip().startswith("#"))
@@ -100,7 +105,8 @@ def test_the_id_is_validated_and_never_combined_with_context_tools(self):
100105
# The run names its runner, on both paths.
101106
self.assertIn("::notice::", check)
102107
self.assertEqual(check.count("GITHUB_STEP_SUMMARY"), 1)
103-
self.assertEqual(CURSOR["Check inputs"].count("GITHUB_STEP_SUMMARY"), 1)
108+
# Once on each branch: the plain line, or the one naming a fallback.
109+
self.assertEqual(CURSOR["Check inputs"].count("GITHUB_STEP_SUMMARY"), 2)
104110
# The direct job reads neither, so it does not gate on them.
105111
self.assertNotIn("inputs.model", DIRECT_JOB)
106112
self.assertNotIn("inputs.runs_on", DIRECT_JOB)
@@ -128,10 +134,13 @@ def test_the_check_executes(self):
128134

129135
class ExactlyOneRunner(unittest.TestCase):
130136
def test_each_job_is_gated_on_the_input(self):
137+
self.assertIn(f"\n if: {PROBE_IF}\n", PROBE_JOB)
131138
self.assertIn(f"\n if: {CURSOR_IF}\n", CURSOR_JOB)
132139
self.assertIn(f"\n if: {DIRECT_IF}\n", DIRECT_JOB)
140+
for body in (CURSOR_JOB, DIRECT_JOB):
141+
self.assertIn("\n needs: axis-probe\n", body)
133142
self.assertEqual(re.findall(r"^ ([A-Za-z0-9_-]+):$", BASE[BASE.index("\njobs:\n"):], re.MULTILINE),
134-
["axis", "axis-direct"])
143+
["axis-probe", "axis", "axis-direct"])
135144

136145
def test_the_direct_job_is_hosted_and_named_apart(self):
137146
# Never the caller's `runs_on` pool, as cursor-review's direct cells.
@@ -144,6 +153,9 @@ def test_each_job_carries_only_its_own_runner(self):
144153
self.assertNotIn("cursor-agent", DIRECT_CODE)
145154
self.assertNotIn("install-cursor-cli", DIRECT_CODE)
146155
self.assertNotIn("axis_direct_model", CURSOR_CODE.replace(f"if: {CURSOR_IF}", ""))
156+
# The probe runs no reviewer and checks nothing out.
157+
for word in ("claude", "cursor-agent", "actions/checkout", "uses:"):
158+
self.assertNotIn(word, PROBE_JOB.replace("claude-", ""), word)
147159

148160
def test_no_fallback_a_missing_key_fails_the_axis(self):
149161
body = run_block(STEPS["Check the Anthropic API key"])
@@ -181,7 +193,11 @@ def test_the_key_is_only_in_the_direct_job(self):
181193
if "secrets.ANTHROPIC_API_KEY" in body:
182194
self.assertIn(name, ("Check the Anthropic API key", "Run the axis (direct)",
183195
"Extract the verdict"), name)
184-
self.assertEqual(BASE.count("secrets.ANTHROPIC_API_KEY"), DIRECT_JOB.count("secrets.ANTHROPIC_API_KEY"))
196+
self.assertEqual(BASE.count("secrets.ANTHROPIC_API_KEY"),
197+
DIRECT_JOB.count("secrets.ANTHROPIC_API_KEY") + PROBE_JOB.count("secrets.ANTHROPIC_API_KEY"))
198+
# The probe job is hosted and checks nothing out (asserted above).
199+
self.assertIn("\n runs-on: ubuntu-latest\n", PROBE_JOB)
200+
self.assertNotIn("CURSOR_API_KEY", PROBE_JOB)
185201
self.assertNotIn("CURSOR_API_KEY", DIRECT_JOB)
186202

187203
def test_every_stdin_python_with_the_key_is_isolated(self):

‎.github/cursor-review/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ jobs:
328328
|---|---|---|---|
329329
| Secret | `CURSOR_API_KEY` | **yes** | Bills the panel + judge `cursor-agent` calls. |
330330
| Secret | `OPENAI_API_KEY` | no | Only with `openai_direct_model`: bills the optional direct-OpenAI cells. Use a project-scoped service-account key with a spend limit. |
331-
| Secret | `ANTHROPIC_API_KEY` | no | Only with `anthropic_direct_model` or `judge_direct_model`: bills the optional direct-Anthropic cells and direct judge. Use a key scoped to its own Anthropic workspace with a spend limit, never a personal key. |
331+
| Secret | `ANTHROPIC_API_KEY` | no | Only with `anthropic_direct_model` or `judge_direct_model`: bills the optional direct-Anthropic cells and direct judge. Use a key scoped to its own Anthropic workspace with a spend limit, never a personal key. A key that hits that limit falls back to Cursor for the run (see `anthropic_direct_model`). |
332332
| Secret | `SLACK_BOT_TOKEN` | no | Enables start/complete DMs to the triggerer. |
333333
| Variable | `CURSOR_REVIEW_DM_EMAIL_MAP` | no | Maps GitHub logins → emails for Slack DM lookup. |
334334

@@ -393,10 +393,10 @@ All optional except `workflows_ref` (required, no default) — pass them under
393393
| `panel_models` | `''` (built-in list) | JSON array of Cursor model ids that **replaces** the panel list; each still runs both review types and is validated against the live catalog by preflight. For per-repo experiments (e.g. `-xhigh` vs `-max` tiers). |
394394
| `openai_direct_model` | `''` (off) | OPT-IN. An OpenAI model id reviewed through the OpenAI API directly (pinned Codex CLI, `package.json` here) as two cells (**adversarial** + **edge-case**, the Cursor cells' prompts) on GitHub-hosted runners, by default **replacing** the Cursor `gpt-*` cells (next row). Needs the `OPENAI_API_KEY` secret and a plain id (`[A-Za-z0-9._-]`); without either, preflight warns and leaves the cells off and the panel unchanged. Not part of `panel_models` (not in the Cursor catalog). Whether the cells gate depends on the next row. |
395395
| `openai_direct_replaces_cursor_openai` | `true` | When the direct cells are in effect, drop the Cursor `gpt-*` cell(s) so the OpenAI lane is reviewed once — the direct cells then **gate** like the Cursor cells they replace: counted in `Panel integrity` and the auto-approve gate, a failed one red in the rollup and named `(direct, <status>)` in the review's "did not contribute" line. `false` runs both for a side-by-side backend comparison, with the direct cells **advisory** (findings reach the judge; never counted, never red). Preflight fails rather than leave the Cursor panel empty. |
396-
| `anthropic_direct_model` | `''` (off) | OPT-IN. The Anthropic twin of `openai_direct_model`: an Anthropic model id (e.g. `claude-opus-5-5`) reviewed through the Anthropic API directly (pinned Claude Code CLI, `package.json` here) as two cells on GitHub-hosted runners, by default **replacing** the Cursor `claude-*` cells. Needs the `ANTHROPIC_API_KEY` secret and a plain id other than `openai_direct_model`'s; without any of those, preflight warns and leaves the cells off and the panel unchanged. The agent also reads a **read-only PR checkout** (symlinks removed) with Read, Grep and Glob only — no Bash or code-running tool — confined to it by `--restricted`, with `/proc` and `/sys` also denied by explicit deny rules. This panel reviews private-repo diffs and the workflow cannot check the Anthropic organization's data-retention terms: callers on private repos must use a key from an organization with zero data retention. Each cell also uploads a `usage-direct-<review_type>-<model>` artifact (kept 30 days) with the API's token counts for that cell, so cost can be measured without an Admin API key. |
396+
| `anthropic_direct_model` | `''` (off) | OPT-IN. The Anthropic twin of `openai_direct_model`: an Anthropic model id (e.g. `claude-opus-5-5`) reviewed through the Anthropic API directly (pinned Claude Code CLI, `package.json` here) as two cells on GitHub-hosted runners, by default **replacing** the Cursor `claude-*` cells. Needs the `ANTHROPIC_API_KEY` secret and a plain id other than `openai_direct_model`'s; without any of those, preflight warns and leaves the cells off and the panel unchanged. **A capped or unusable key falls back to Cursor for that run:** before each round the hosted `gate` job makes one 1-token Anthropic call per configured model; a usage-limit or billing refusal, a 401/403, or a 429/529/5xx still failing after retries makes preflight treat the key as absent — the Cursor `claude-*` cells and the Cursor judge run. You can tell from the run: a `::warning::` per lane on the `preflight` job naming the reason class and, for a usage limit, its reset time (never the API's raw message, which can name the organization), a `fell back to Cursor` line in its job summary, and an italic note under the posted review's header. Any other refusal (an unrecognised 4xx; a redirect, never followed) falls back too. An unknown model id (404) fails the run instead; a typo never falls back. A dash-free Claude Code alias (`opus`, `sonnet`) is not probed — the raw API does not know it — so it runs direct as before. If `panel_models` has no `claude-*` id, there is no Cursor cell to fall back to: the warning, summary and note say no Anthropic review cell ran. The agent also reads a **read-only PR checkout** (symlinks removed) with Read, Grep and Glob only — no Bash or code-running tool — confined to it by `--restricted`, with `/proc` and `/sys` also denied by explicit deny rules. This panel reviews private-repo diffs and the workflow cannot check the Anthropic organization's data-retention terms: callers on private repos must use a key from an organization with zero data retention. Each cell also uploads a `usage-direct-<review_type>-<model>` artifact (kept 30 days) with the API's token counts for that cell, so cost can be measured without an Admin API key. |
397397
| `anthropic_direct_effort` | `xhigh` | `--effort` for the direct Anthropic cells: `low`, `medium`, `high`, `xhigh` or `max`; anything else warns and falls back to `xhigh`. |
398398
| `anthropic_direct_replaces_cursor_anthropic` | `true` | As `openai_direct_replaces_cursor_openai`, for the Cursor `claude-*` cells. Each lab counts by its own flag. Never touches `judge_model`. |
399-
| `judge_direct_model` | `''` (off) | OPT-IN. An Anthropic model id (e.g. `claude-opus-5-5`) that runs the **judge** through the Anthropic API directly instead of `judge_model` through Cursor, with the same pinned Claude Code CLI and `ANTHROPIC_API_KEY` as `anthropic_direct_model`. Needs that secret and a plain id (`[A-Za-z0-9._-]`, at most 100); without either, preflight warns and the Cursor judge runs. Same prompt, structured-output tool, step timeout and degraded panel-union fallback as the Cursor judge; the agent reads the PR checkout (symlinks removed) with Read, Grep and Glob only — no Bash or code-running tool — confined to it by `--restricted`, with `/proc` and `/sys` also denied by explicit deny rules. A final review containing the key is replaced by the error seed, so the panel union posts instead; the log shows the judge's status and finding count only. The judge runs in its own `judge-direct` job on GitHub-hosted `ubuntu-latest`, whatever `runs_on` says, so the key never reaches `consolidate`. This judge reads private-repo diffs and the workflow cannot check the Anthropic organization's data-retention terms: callers on private repos must use a key from an organization with zero data retention. The judge also uploads a `usage-direct-judge-<model>` artifact (kept 30 days) with the API's token counts for the judge, the same record the direct cells upload. |
399+
| `judge_direct_model` | `''` (off) | OPT-IN. An Anthropic model id (e.g. `claude-opus-5-5`) that runs the **judge** through the Anthropic API directly instead of `judge_model` through Cursor, with the same pinned Claude Code CLI and `ANTHROPIC_API_KEY` as `anthropic_direct_model`. Needs that secret and a plain id (`[A-Za-z0-9._-]`, at most 100); without either, preflight warns and the Cursor judge runs — and likewise when the key is capped or unusable for the run (see `anthropic_direct_model`). Same prompt, structured-output tool, step timeout and degraded panel-union fallback as the Cursor judge; the agent reads the PR checkout (symlinks removed) with Read, Grep and Glob only — no Bash or code-running tool — confined to it by `--restricted`, with `/proc` and `/sys` also denied by explicit deny rules. A final review containing the key is replaced by the error seed, so the panel union posts instead; the log shows the judge's status and finding count only. The judge runs in its own `judge-direct` job on GitHub-hosted `ubuntu-latest`, whatever `runs_on` says, so the key never reaches `consolidate`. This judge reads private-repo diffs and the workflow cannot check the Anthropic organization's data-retention terms: callers on private repos must use a key from an organization with zero data retention. The judge also uploads a `usage-direct-judge-<model>` artifact (kept 30 days) with the API's token counts for the judge, the same record the direct cells upload. |
400400
| `judge_direct_effort` | `xhigh` | `--effort` for the direct judge: `low`, `medium`, `high`, `xhigh` or `max`; anything else warns and falls back to `xhigh`. |
401401
| `cell_timeout_minutes` | `15` | Wall-clock cap on one reviewer cell's agent step (Cursor or direct). Over it the cell exits 130, uploads `status: error` with no findings, reddens its leg and fails `Panel integrity`. Shared by the cell's in-step retries, so it bounds attempts + backoff. Derives three more caps that cannot be set alone: cell job = this + 15, `Run judge` step = max(30, this × 2), judge job = that + 10. At the default: 15 / 30 / 30 / 40, today's values exactly. Preflight bounds it 10–45. |
402402
| `skip_bot_branch_prefixes` | `ci/bump- chore/refresh- auto/refresh-` | Skip the panel when the PR author is a Bot **and** the head branch starts with one of these prefixes (machine pin bumps / catalog refreshes). `''` reviews every bot PR. |

‎.github/cursor-review/post-review.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3006,6 +3006,14 @@ def main():
30063006
"or the 'context unavailable' banner. Empty/absent renders nothing."
30073007
),
30083008
)
3009+
parser.add_argument(
3010+
"--runner-note",
3011+
default=None,
3012+
help=(
3013+
"Which runner a lane fell back to, e.g. the direct Anthropic lanes on "
3014+
"Cursor after the key probed unusable. Empty/absent renders nothing."
3015+
),
3016+
)
30093017
args = parser.parse_args()
30103018

30113019
attribution = f"\n\n_Triggered by @{args.triggered_by}._" if args.triggered_by else ""
@@ -3035,6 +3043,10 @@ def main():
30353043
# banner case matters most: a re-review that ran WITHOUT prior context
30363044
# must never look identical to a genuine first-round review.
30373045
banners += f"\n\n_{neutralize_mentions(args.ledger_note.strip())}_"
3046+
if args.runner_note and args.runner_note.strip():
3047+
# A fallback is a different panel than the caller configured: say so,
3048+
# or a Cursor-judged round reads as a direct-judged one.
3049+
banners += f"\n\n_{neutralize_mentions(args.runner_note.strip())}_"
30383050
header += banners
30393051
review_header += banners
30403052

0 commit comments

Comments
 (0)