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 #404] [MERGED] feat(cursor-review): name the Anthropic lane's failure cause, gated and key-screened #404

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/anthropic-lane-failure-cause

📝 Commits (3)

16df226 feat(cursor-review): name the Anthropic lane's failure cause, under three gates
ba75547 docs(cursor-review): a no-submit cell can still print error objects; census-only is the no-error-object case
8457fe7 fix(cursor-review): gate the Anthropic cause on the real CLI shape, and screen it for the API key

📊 Changes

4 files changed (+259 additions, -22 deletions)

View changed files

📝 .github/cursor-review/README.md (+1 -1)
📝 .github/cursor-review/agent-error.py (+89 -0)
📝 .github/cursor-review/tests/test_agent_error.py (+144 -8)
📝 docs/callers/cursor-review.md (+25 -13)

📄 Description

Follow-up to #402. This commit missed that merge by two minutes — #402 merged at 02:40:07Z with head b537a26, and this landed on the branch at 02:42:02Z, so the squash (101466d) never picked it up.

It is not a new idea: it is the half of the panel's finding that the maintainer explicitly chose, and the only reason it is a separate PR is the race.

What #402 shipped, and what it left

The panel found that Claude Code reports an API-level failure as:

{"type":"result","subtype":"success","is_error":true,"result":"API Error: 400 credit balance is too low"}

subtype reads success on a failed call, and the cause sits only in result — a field the model also writes. #402 took the safe half: print is_error so the line cannot read as a success, and document the gap. Main therefore says, truthfully, that the Anthropic lane names no cause.

The reviewer offered two ways to close it: extract a bounded, sanitized slice of result, or limit the docs claim to the OpenAI lane. #402 did the second. This does the first.

The gates

result is printed only when all of these hold:

gate	why
the top-level `{"type":"result"}` object, `is_error` `true`	an ordinary result holds the agent's review; nested and codex-lane objects never qualify
the CLI marked it an API failure: a numeric `api_error_status`, or a value opening with `API Error: <3 ASCII digits>`	a CLI-written signal, not prose
no `sk-ant-` prefix and no 16-char slice of the step's own `ANTHROPIC_API_KEY`	the screen `findings.json` gets; a 120-char cut would slip past Actions' exact-value mask
cut to 120 chars, sanitized	bounded, and cannot carry a workflow command

api_error_status and terminal_reason (CLI-written scalars) join the field allowlist. Output from a real failure of the pinned CLI (@anthropic-ai/claude-code@2.1.286, invalid key):

error: type=result subtype=success is_error=True api_error_status=401 terminal_reason=api_error
error: result=Invalid API key ? Fix external API key

That real message has no API Error: preamble, so a preamble-only gate (this PR's first revision) would have missed it. The review panel caught that, and the captured object is now pinned as a fixture.

Being straight about the trade

This is the one place a model-written field reaches the run log. A prompt-injected agent can forge the preamble, so it is a bounded, documented channel — roughly 120 sanitized characters that cannot open a ::workflow command:: or a legacy ##[command] and cannot carry the API key, but can carry other text — not a safe one.

It was taken knowingly. The alternative, which main currently ships, is a lane that reports subtype=success and names nothing: the cause appears nowhere else in the file, and the file is deliberately not uploaded (the Anthropic lane screens findings.json for a leaked API key before publishing it, and shipping the unscreened transcript would route the agent's prose past that guard).

A failure with neither marker prints no cause rather than a guess. Widen the gate against a real message, never speculatively.

The module docstring, the README row and the caller page all asserted that no model-written field reaches the log. All three are corrected here rather than left to mislead — that overstatement is exactly what the panel caught the first time round.

Tests

Each gate is pinned separately, because a gate that quietly stopped holding would widen the channel without changing any other test. The pinned cases are review prose without a marker, a message with no status code, is_error false, a non-result type, a nested or list-wrapped result, Unicode digits, a boolean api_error_status, a structured result, the length cap, the injection case, and the key screen (prefix, full key, offset slice, mid-key slice).

One of those tests first passed for the wrong reason. With is_error false, the fixture no longer looked like an error at all, so the extractor was never reached and the gate went untested while appearing covered. It now carries an independent error signal and asserts that the object was recognised as an error.

Provenance

Authored by: agent-work loop
Verified:

python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py': 1416 pass; the cursor-approve suite passes
A mutation of each gate (type/top-level, depth, ASCII digits, bool status, CLI marker, key screen, key prefix, key window) is caught by the suite
agent-error.py run against a freshly captured invalid-key claude --output-format json file prints the status, reason and cause shown above
check_workflow_pins.py, check_agents_md.py --root . and check-org-repo-literals.sh pass

Deviations:

JSON-body parsing of API Error: <status> {...} was not adopted: a forged JSON body is as easy to write as forged prose, so it does not narrow the channel

🤖 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/404 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/10/2026 **Status:** ✅ Merged **Merged:** 10/10/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/anthropic-lane-failure-cause` --- ### 📝 Commits (3) - [`16df226`](https://github.com/Comfy-Org/github-workflows/commit/16df226eb84cd45cff12282fffb9fa37691d393c) feat(cursor-review): name the Anthropic lane's failure cause, under three gates - [`ba75547`](https://github.com/Comfy-Org/github-workflows/commit/ba75547c2375a22c838858da82ee8030bcc1d07b) docs(cursor-review): a no-submit cell can still print error objects; census-only is the no-error-object case - [`8457fe7`](https://github.com/Comfy-Org/github-workflows/commit/8457fe72bb912ea4eb14a974119005b3bb442747) fix(cursor-review): gate the Anthropic cause on the real CLI shape, and screen it for the API key ### 📊 Changes **4 files changed** (+259 additions, -22 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+1 -1) 📝 `.github/cursor-review/agent-error.py` (+89 -0) 📝 `.github/cursor-review/tests/test_agent_error.py` (+144 -8) 📝 `docs/callers/cursor-review.md` (+25 -13) </details> ### 📄 Description Follow-up to #402. This commit **missed that merge by two minutes** — #402 merged at `02:40:07Z` with head `b537a26`, and this landed on the branch at `02:42:02Z`, so the squash (`101466d`) never picked it up. It is not a new idea: it is the half of the panel's finding that the maintainer explicitly chose, and the only reason it is a separate PR is the race. ## What #402 shipped, and what it left The panel found that Claude Code reports an API-level failure as: ```json {"type":"result","subtype":"success","is_error":true,"result":"API Error: 400 credit balance is too low"} ``` `subtype` reads **success** on a failed call, and the cause sits only in `result` — a field the model also writes. #402 took the safe half: print `is_error` so the line cannot read as a success, and document the gap. Main therefore says, truthfully, that the Anthropic lane names no cause. The reviewer offered two ways to close it: extract a bounded, sanitized slice of `result`, or limit the docs claim to the OpenAI lane. #402 did the second. This does the first. ## The gates `result` is printed only when **all** of these hold: | gate | why | |---|---| | the top-level `{"type":"result"}` object, `is_error` `true` | an ordinary result holds the agent's review; nested and codex-lane objects never qualify | | the CLI marked it an API failure: a numeric `api_error_status`, or a value opening with `API Error: <3 ASCII digits> ` | a CLI-written signal, not prose | | no `sk-ant-` prefix and no 16-char slice of the step's own `ANTHROPIC_API_KEY` | the screen `findings.json` gets; a 120-char cut would slip past Actions' exact-value mask | | cut to 120 chars, sanitized | bounded, and cannot carry a workflow command | `api_error_status` and `terminal_reason` (CLI-written scalars) join the field allowlist. Output from a real failure of the pinned CLI (`@anthropic-ai/claude-code@2.1.286`, invalid key): ``` error: type=result subtype=success is_error=True api_error_status=401 terminal_reason=api_error error: result=Invalid API key ? Fix external API key ``` That real message has **no** `API Error:` preamble, so a preamble-only gate (this PR's first revision) would have missed it. The review panel caught that, and the captured object is now pinned as a fixture. ## Being straight about the trade **This is the one place a model-written field reaches the run log.** A prompt-injected agent *can* forge the preamble, so it is a bounded, documented channel — roughly 120 sanitized characters that cannot open a `::workflow command::` or a legacy `##[command]` and cannot carry the API key, but can carry other text — not a safe one. It was taken knowingly. The alternative, which main currently ships, is a lane that reports `subtype=success` and names nothing: the cause appears nowhere else in the file, and the file is deliberately not uploaded (the Anthropic lane screens `findings.json` for a leaked API key before publishing it, and shipping the unscreened transcript would route the agent's prose past that guard). A failure with neither marker prints no cause rather than a guess. Widen the gate against a real message, never speculatively. The module docstring, the README row and the caller page all asserted that no model-written field reaches the log. All three are corrected here rather than left to mislead — that overstatement is exactly what the panel caught the first time round. ## Tests Each gate is pinned **separately**, because a gate that quietly stopped holding would widen the channel without changing any other test. The pinned cases are review prose without a marker, a message with no status code, `is_error` false, a non-`result` type, a nested or list-wrapped result, Unicode digits, a boolean `api_error_status`, a structured `result`, the length cap, the injection case, and the key screen (prefix, full key, offset slice, mid-key slice). One of those tests first passed for the **wrong reason**. With `is_error` false, the fixture no longer looked like an error at all, so the extractor was never reached and the gate went untested while appearing covered. It now carries an independent error signal and asserts that the object *was* recognised as an error. ## Provenance **Authored by:** agent-work loop **Verified:** - `python3 -m unittest discover -s .github/cursor-review/tests -p 'test_*.py'`: 1416 pass; the cursor-approve suite passes - A mutation of each gate (type/top-level, depth, ASCII digits, bool status, CLI marker, key screen, key prefix, key window) is caught by the suite - `agent-error.py` run against a freshly captured invalid-key `claude --output-format json` file prints the status, reason and cause shown above - `check_workflow_pins.py`, `check_agents_md.py --root .` and `check-org-repo-literals.sh` pass **Deviations:** - JSON-body parsing of `API Error: <status> {...}` was not adopted: a forged JSON body is as easy to write as forged prose, so it does not narrow the channel 🤖 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:25 +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#404

No description provided.

Rows
Columns