Comfy-Org/github-workflows

Fork 0

mirror of https://github.com/Comfy-Org/github-workflows.git synced 2026-11-05 00:17:37 +00:00

[PR #431] [MERGED] feat(cursor-review): probe the direct Anthropic key and fall back to Cursor when it is capped #432

New issue

Closed

opened 2026-10-26 22:39:46 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:39:46 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/431
Author: @mattmillerai
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-20888-anthropic-direct-probe

📝 Commits (4)

51386e5 feat(cursor-review): probe the direct Anthropic key before a round and fall back to Cursor when it is capped
ce1a871 fix(cursor-review): name each fallen lane's own reason in the fallback note
886c548 fix(cursor-review): harden the Anthropic key probe per the panel's review round
6649e7f fix(cursor-review): publish only the probe's reason class, never the API's raw message

📊 Changes

10 files changed (+994 additions, -38 deletions)

View changed files

📝 .github/cursor-approve/tests/test_axis_direct.py (+25 -9)
📝 .github/cursor-review/README.md (+3 -3)
📝 .github/cursor-review/post-review.py (+12 -0)
➕ .github/cursor-review/tests/test_anthropic_probe.py (+466 -0)
📝 .github/cursor-review/tests/test_auto_approve.py (+1 -1)
📝 .github/cursor-review/tests/test_post_review.py (+10 -0)
📝 .github/workflows/cursor-axis-base.yml (+241 -12)
📝 .github/workflows/cursor-review.yml (+229 -6)
📝 docs/callers/cursor-approve.md (+4 -4)
📝 docs/callers/cursor-review.md (+3 -3)

📄 Description

ELI-5

The review panel, its judge and two approval axes can talk to Anthropic directly with an API key. When that key hits its organization's monthly spend limit, every direct call fails, the round ends with no decision, and a human had to delete repo variables to put everything back on Cursor. Now each round first makes one tiny (1-token) call with the key. If Anthropic refuses it for a usage limit, billing, a bad key, or keeps saying "overloaded" after retries, that run quietly uses the Cursor lanes instead, and says so loudly in the run. A typo'd model id still fails the run.

What changed

cursor-review.yml › gate: new step Probe the direct Anthropic key (GitHub-hosted, checks out nothing, runs only for a round that will actually run and only when the key is present). One POST /v1/messages with max_tokens: 1 per distinct configured model (anthropic_direct_model, judge_direct_model). Classification: 2xx → usable; 400 "usage limit" / billing error / credit balance / 402 → unusable; 401/403 → unusable; 408/429/5xx/529/no answer (or any exception) → retried after 15 s and 45 s, within a wall-clock PROBE_BUDGET_SECONDS, then unusable; 404 / not_found_error → invalid, except a dash-free CLI model alias (opus, sonnet), which is not probed; any other refusal (an unrecognised 4xx, or a redirect, which is never followed with the key) → unusable. Never prints the key, request headers or response headers, and never publishes the API's raw message (it can name the organization): only the reason class, plus a usage limit's parsed reset time. All outputs are written once, at the end, and continue-on-error applies: if the probe itself crashes or times out, the outputs stay empty and preflight behaves exactly as today.
preflight › Define panel models: an unusable lane is handled exactly like an absent key. The direct cells stay off and the Cursor claude-* cells stay in the panel; the Cursor judge runs. Each lane gets one ::warning:: with the reason class (and reset time), plus one job-summary line. If the panel list has no Cursor claude-* id, the warning, summary and posted note say no Anthropic review cell ran. invalid → ::error:: + exit 1 (no fallback on a typo), checked after the openai/anthropic id-collision warning.
post-review.py: new optional --runner-note, rendered as an italic banner under the review header (same as --ledger-note). The post step passes gate's anthropic_fallback_note, so a round whose direct lanes fell back says so on the PR.
cursor-axis-base.yml: new axis-probe job (GitHub-hosted, contents: read, no checkout, no uses:) ahead of the two runners. It first runs the checks a fallback must not skip (checkout axis, checkout: true, plain id, no context_sources, key present), then the byte-identical probe script (a test pins the parity), then Pick the runner → runner=direct|cursor. axis runs when the input is empty or the probe picked cursor; axis-direct runs only when it picked direct. Unknown model → the probe job fails, so neither runner runs and the axis has no verdict, same as today.
Docs: docs/callers/cursor-review.md, docs/callers/cursor-approve.md, .github/cursor-review/README.md and the workflow input/secret descriptions now say that a capped or unusable key falls back to Cursor for that run, and how to see that in the run.

No new inputs or secrets, so today's caller files work unchanged.

Judgment calls

Absent key on an axis still fails (it does not fall back). The ticket asks to keep the rest of the axis no-fallback rule and names only usability as the fallback trigger. The absent-key check moved into axis-probe; the old check in axis-direct stays as a backstop.
An "other" 4xx falls back instead of failing. Only a 404 / not_found_error counts as a config error; a 400's wording is never guessed at ("invalid max_tokens for this model" must not fail a valid config). Any other 4xx is treated as unusable so that it can't take the approver down.
Only the two configured models are probed, and a model shared by the cells and the judge is probed once. A key at its usage limit is refused org-wide, so a second call would tell us nothing.
Gate step timeout 5 min: the worst case for two models is 2 × (3 × 20 s + 60 s) = 4 min, and retries stop at a 240 s budget. The axis probe step uses 3 min and a 150 s budget for its one model.
The axis key confinement widened by one job. The ticket says the key may appear only in jobs that already reference it, but its own design (§5) calls for a new probe job in cursor-axis-base.yml. That job is GitHub-hosted, checks out nothing, uses no actions and never runs on runs_on. The confinement test now asserts exactly that.

How has this been tested?

python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py': all OK. New test_anthropic_probe.py runs the real gate step against a local HTTP stub that replays recorded responses: 200, the usage-limit 400 (reset time kept), the billing 400, 401, 429 then 200 (2 calls, direct kept), 529 on every attempt (3 calls, unusable), 404 (invalid), a 404 on an alias (not probed), any 400 including model-worded ones (unusable, never invalid), a 302 (refused, target never contacted), a truncated error body (retried, no crash), the retry budget, a key or org id in the error message (never published), one model shared by both lanes (1 call), a non-plain id (not probed). It also runs preflight's real Define panel models: unusable → Cursor claude cells and Cursor judge, 2 warnings, 2 summary lines; unusable with no claude-* in the panel → says no Anthropic cell ran; usable or not probed → unchanged; invalid → exit 1, but the id-collision warning first. It runs the axis Pick the runner and Check the direct inputs steps, checks gate/axis script parity, and checks the post step's --runner-note wiring. test_post_review.py adds a test that renders the runner note.
python3 -m unittest discover -s .github/cursor-approve/tests -p 'test_*.py': 328 OK (test_axis_direct.py updated for the three-job layout and the key confinement).
.github/workflow-pins suite 551 OK; check_workflow_pins.py OK; check-org-repo-literals.sh OK; agents-md-integrity and public-repo-hygiene suites OK; both workflows parse as YAML.
Doc-drift sweep: 60 identifiers flagged; ~25 meaningful references judged by targeted grep (axis_direct_model, anthropic_direct_model, judge_direct_model, ANTHROPIC_API_KEY, "no fallback"/"never falls back"), 9 updated. The remaining ~1,700 lines it found are matches on common words (content, text, body, bash, …) and were not reviewed one by one.

Residual

Not exercised against the live Anthropic API. There was no key in the sandbox, so classification was verified only against recorded response shapes: the ticket's usage-limit text and the documented error types. If the API rejects max_tokens: 1 for some future model with a 400, that round falls back to Cursor rather than running direct. Watch the first real round after the direct variables are restored.
The approve card doesn't name a runner, so it can't show the fallback. The fallback is visible through the probe job's ::warning::, the job summaries, and the check that ran (axis: <axis> instead of axis: <axis> (direct)). Adding a runner field to the verdict artifact/card is a separate change.
A panel list with no Cursor claude-* id gets no Anthropic cell on a fallback. That is the same panel an absent key gives; this PR only makes it say so. Adding a Cursor fallback model is a separate design call.
Probe coverage gaps: runs created before a cap still have no probe until callers bump to this SHA. The OpenAI direct lane has no probe (out of scope per the ticket). The probe happens once at round start, so a cap reached mid-round still fails that round's direct lanes.
Operator step after merge and caller bump: restore the direct model repo variables on the consuming repos. Not done here.

Provenance

Authored by: agent-work loop
Verified:
- Tests at the current head: cursor-review suite OK (1719+ incl. the review-round probe cases), cursor-approve 328 OK, workflow-pins OK
- Lints: workflow pin check, org-repo-literal lint, AGENTS.md integrity: clean
- Probe classification executed against a local stub for every branch, incl. redirect refusal, retry budget, alias and no-raw-message cases
- The new probe tests were confirmed to fail against the previous head's workflows
- Not run: a live Anthropic call (no key available), a real Actions run
Deviations: the approve card does not name the runner (no card change; the probe warning, job summary and check name show it instead)

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/431 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-20888-anthropic-direct-probe` --- ### 📝 Commits (4) - [`51386e5`](https://github.com/Comfy-Org/github-workflows/commit/51386e599e8b453e9c393e65ad684a4abc390bc8) feat(cursor-review): probe the direct Anthropic key before a round and fall back to Cursor when it is capped - [`ce1a871`](https://github.com/Comfy-Org/github-workflows/commit/ce1a8712d88e51fbf12aa970b26cd8474c0e524d) fix(cursor-review): name each fallen lane's own reason in the fallback note - [`886c548`](https://github.com/Comfy-Org/github-workflows/commit/886c548584cd5b31f9274688f5f9636b9db5b4ac) fix(cursor-review): harden the Anthropic key probe per the panel's review round - [`6649e7f`](https://github.com/Comfy-Org/github-workflows/commit/6649e7fdb8244eaaf84cb21c608bbef46883a241) fix(cursor-review): publish only the probe's reason class, never the API's raw message ### 📊 Changes **10 files changed** (+994 additions, -38 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-approve/tests/test_axis_direct.py` (+25 -9) 📝 `.github/cursor-review/README.md` (+3 -3) 📝 `.github/cursor-review/post-review.py` (+12 -0) ➕ `.github/cursor-review/tests/test_anthropic_probe.py` (+466 -0) 📝 `.github/cursor-review/tests/test_auto_approve.py` (+1 -1) 📝 `.github/cursor-review/tests/test_post_review.py` (+10 -0) 📝 `.github/workflows/cursor-axis-base.yml` (+241 -12) 📝 `.github/workflows/cursor-review.yml` (+229 -6) 📝 `docs/callers/cursor-approve.md` (+4 -4) 📝 `docs/callers/cursor-review.md` (+3 -3) </details> ### 📄 Description ## ELI-5 The review panel, its judge and two approval axes can talk to Anthropic directly with an API key. When that key hits its organization's monthly spend limit, every direct call fails, the round ends with no decision, and a human had to delete repo variables to put everything back on Cursor. Now each round first makes one tiny (1-token) call with the key. If Anthropic refuses it for a usage limit, billing, a bad key, or keeps saying "overloaded" after retries, that run quietly uses the Cursor lanes instead, and says so loudly in the run. A typo'd model id still fails the run. ## What changed - **`cursor-review.yml` › `gate`**: new step `Probe the direct Anthropic key` (GitHub-hosted, checks out nothing, runs only for a round that will actually run and only when the key is present). One `POST /v1/messages` with `max_tokens: 1` per distinct configured model (`anthropic_direct_model`, `judge_direct_model`). Classification: 2xx → usable; 400 "usage limit" / billing error / credit balance / 402 → unusable; 401/403 → unusable; 408/429/5xx/529/no answer (or any exception) → retried after 15 s and 45 s, within a wall-clock `PROBE_BUDGET_SECONDS`, then unusable; 404 / `not_found_error` → invalid, except a dash-free CLI model alias (`opus`, `sonnet`), which is not probed; any other refusal (an unrecognised 4xx, or a redirect, which is never followed with the key) → unusable. Never prints the key, request headers or response headers, and never publishes the API's raw message (it can name the organization): only the reason class, plus a usage limit's parsed reset time. All outputs are written once, at the end, and `continue-on-error` applies: if the probe itself crashes or times out, the outputs stay empty and preflight behaves exactly as today. - **`preflight` › `Define panel models`**: an `unusable` lane is handled exactly like an absent key. The direct cells stay off and the Cursor `claude-*` cells stay in the panel; the Cursor judge runs. Each lane gets one `::warning::` with the reason class (and reset time), plus one job-summary line. If the panel list has no Cursor `claude-*` id, the warning, summary and posted note say no Anthropic review cell ran. `invalid` → `::error::` + exit 1 (no fallback on a typo), checked after the openai/anthropic id-collision warning. - **`post-review.py`**: new optional `--runner-note`, rendered as an italic banner under the review header (same as `--ledger-note`). The post step passes `gate`'s `anthropic_fallback_note`, so a round whose direct lanes fell back says so on the PR. - **`cursor-axis-base.yml`**: new `axis-probe` job (GitHub-hosted, `contents: read`, no checkout, no `uses:`) ahead of the two runners. It first runs the checks a fallback must not skip (checkout axis, `checkout: true`, plain id, no `context_sources`, key present), then the **byte-identical** probe script (a test pins the parity), then `Pick the runner` → `runner=direct|cursor`. `axis` runs when the input is empty **or** the probe picked cursor; `axis-direct` runs only when it picked direct. Unknown model → the probe job fails, so neither runner runs and the axis has no verdict, same as today. - **Docs**: `docs/callers/cursor-review.md`, `docs/callers/cursor-approve.md`, `.github/cursor-review/README.md` and the workflow input/secret descriptions now say that a capped or unusable key falls back to Cursor for that run, and how to see that in the run. No new inputs or secrets, so today's caller files work unchanged. ## Judgment calls - **Absent key on an axis still fails** (it does not fall back). The ticket asks to keep the rest of the axis no-fallback rule and names only usability as the fallback trigger. The absent-key check moved into `axis-probe`; the old check in `axis-direct` stays as a backstop. - **An "other" 4xx falls back instead of failing.** Only a 404 / `not_found_error` counts as a config error; a 400's wording is never guessed at ("invalid max_tokens for this model" must not fail a valid config). Any other 4xx is treated as unusable so that it can't take the approver down. - **Only the two configured models are probed**, and a model shared by the cells and the judge is probed once. A key at its usage limit is refused org-wide, so a second call would tell us nothing. - **Gate step timeout 5 min**: the worst case for two models is 2 × (3 × 20 s + 60 s) = 4 min, and retries stop at a 240 s budget. The axis probe step uses 3 min and a 150 s budget for its one model. - **The axis key confinement widened by one job.** The ticket says the key may appear only in jobs that already reference it, but its own design (§5) calls for a new probe job in `cursor-axis-base.yml`. That job is GitHub-hosted, checks out nothing, uses no actions and never runs on `runs_on`. The confinement test now asserts exactly that. ## How has this been tested? - `python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py'`: all OK. New `test_anthropic_probe.py` runs the real gate step against a local HTTP stub that replays recorded responses: 200, the usage-limit 400 (reset time kept), the billing 400, 401, 429 then 200 (2 calls, direct kept), 529 on every attempt (3 calls, unusable), 404 (invalid), a 404 on an alias (not probed), any 400 including model-worded ones (unusable, never invalid), a 302 (refused, target never contacted), a truncated error body (retried, no crash), the retry budget, a key or org id in the error message (never published), one model shared by both lanes (1 call), a non-plain id (not probed). It also runs preflight's real `Define panel models`: unusable → Cursor claude cells and Cursor judge, 2 warnings, 2 summary lines; unusable with no `claude-*` in the panel → says no Anthropic cell ran; usable or not probed → unchanged; invalid → exit 1, but the id-collision warning first. It runs the axis `Pick the runner` and `Check the direct inputs` steps, checks gate/axis script parity, and checks the post step's `--runner-note` wiring. `test_post_review.py` adds a test that renders the runner note. - `python3 -m unittest discover -s .github/cursor-approve/tests -p 'test_*.py'`: 328 OK (`test_axis_direct.py` updated for the three-job layout and the key confinement). - `.github/workflow-pins` suite 551 OK; `check_workflow_pins.py` OK; `check-org-repo-literals.sh` OK; `agents-md-integrity` and `public-repo-hygiene` suites OK; both workflows parse as YAML. - Doc-drift sweep: 60 identifiers flagged; ~25 meaningful references judged by targeted grep (`axis_direct_model`, `anthropic_direct_model`, `judge_direct_model`, `ANTHROPIC_API_KEY`, "no fallback"/"never falls back"), 9 updated. The remaining ~1,700 lines it found are matches on common words (`content`, `text`, `body`, `bash`, …) and were not reviewed one by one. ## Residual - **Not exercised against the live Anthropic API.** There was no key in the sandbox, so classification was verified only against recorded response shapes: the ticket's usage-limit text and the documented error types. If the API rejects `max_tokens: 1` for some future model with a 400, that round falls back to Cursor rather than running direct. Watch the first real round after the direct variables are restored. - **The approve card doesn't name a runner**, so it can't show the fallback. The fallback is visible through the probe job's `::warning::`, the job summaries, and the check that ran (`axis: <axis>` instead of `axis: <axis> (direct)`). Adding a runner field to the verdict artifact/card is a separate change. - **A panel list with no Cursor `claude-*` id** gets no Anthropic cell on a fallback. That is the same panel an absent key gives; this PR only makes it say so. Adding a Cursor fallback model is a separate design call. - **Probe coverage gaps:** runs created before a cap still have no probe until callers bump to this SHA. The OpenAI direct lane has no probe (out of scope per the ticket). The probe happens once at round start, so a cap reached mid-round still fails that round's direct lanes. - **Operator step after merge and caller bump:** restore the direct model repo variables on the consuming repos. Not done here. ## Provenance - **Authored by:** agent-work loop - **Verified:** - Tests at the current head: cursor-review suite OK (1719+ incl. the review-round probe cases), cursor-approve 328 OK, workflow-pins OK - Lints: workflow pin check, org-repo-literal lint, AGENTS.md integrity: clean - Probe classification executed against a local stub for every branch, incl. redirect refusal, retry budget, alias and no-raw-message cases - The new probe tests were confirmed to fail against the previous head's workflows - Not run: a live Anthropic call (no key available), a real Actions run - **Deviations:** the approve card does not name the runner (no card change; the probe warning, job summary and check name show it instead) 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:39:46 +00:00

closed this issue
added the
pull-request
label

No labels

cursor-review-catalog-drift

groom

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/github-workflows#432

No description provided.

Rows
Columns