[PR #388] [MERGED] fix(cursor-review): retry a panel cell on Cursor's transient resource_exhausted error #387

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: be-19849-cursor-review-retry-resource-exhausted

📝 Commits (3)

1a6a8ce fix(cursor-review): retry a panel cell on Cursor's transient resource_exhausted error
c7a5eeb fix(cursor-review): rewind the prompt before a retried attempt
454e25e fix(cursor-review): report a signal that lands while the agent's stderr drains

📊 Changes

4 files changed (+576 additions, -23 deletions)

View changed files

📝 .github/cursor-review/README.md (+1 -1)
📝 .github/cursor-review/stop-on-submit.py (+253 -20)
📝 .github/cursor-review/tests/test_stop_on_submit.py (+308 -2)
📝 .github/workflows/cursor-review.yml (+14 -0)

📄 Description

ELI-5

Sometimes Cursor says "I'm too busy right now, try again" (RetriableError: [resource_exhausted]) about 25 seconds into a review cell. Today the cell gives up straight away and the panel posts with reviewers missing. With this change the cell waits a bit and tries again, at most twice, before it gives up.

What changed

stop-on-submit.py (the wrapper around each panel cell's cursor-agent) gets a retry loop:
- Trigger: the agent exits by itself with a code > 0, and the last 64 KiB of its stderr contains [resource_exhausted] or RetriableError. stderr passes through the wrapper and is copied on to its own stderr byte for byte, so /tmp/review-stderr.txt gets the same content as before. stdout is inherited untouched.
- Prompt replay: stdin (< /tmp/prompt.txt) is rewound to its starting offset before every retry; a stdin that cannot seek rules the retry out.
- Backoff: --retry-delays 30,90, so at most 2 retries.
- Never re-runs a submitted or partial cell: a retry needs findings.json to parse as an object whose status is not ok and whose findings list is empty. review-output-mcp.py appends each recorded finding, so re-running a cell that already recorded some would duplicate them. Those cells, and an unreadable file, fail exactly as they do today.
- Fits the step cap: no retry starts more than --retry-window 300 seconds after the wrapper started (delay included). The last attempt therefore still has about 10 of the 15 minutes for a full review. If it runs past that, the step cap kills it as before.
- Never retried: exit 0, any other error, a death by signal, and a forwarded signal (the step cap or a cancel, including one that arrives during the backoff or while the agent's stderr drains, which ends the wrapper with 128+signal).
- Logging: each retry prints one ::warning::Reviewer cell <review_type>/<model>: attempt N/3 failed with Cursor's retriable error [resource_exhausted] (exit 1); retrying in 30s. The warning names only the fixed marker, never text the agent printed, and the cell label is sanitized.
cursor-review.yml Run cursor review passes --retry-delays 30,90 --retry-window 300 --cell "$REVIEW_TYPE/$MODEL" (it adds REVIEW_TYPE to the step env) and has a comment explaining why. A cell that uses up its retries still ends status=error, and Fail the leg when the cell did not submit still turns it red.
README row for stop-on-submit.py updated.

Judgment calls

The retry lives in the Python wrapper, not the step shell. That makes it unit-testable against a stub agent, and it keeps the existing signal forwarding and process-group SIGKILL working for each attempt.
Only stderr is scanned (changed in review, see Review fixes). The first version scanned stdout too, but stdout is the model's own text, which the PR under review can steer.
"No structured findings were submitted" is read strictly as no ok status and no recorded findings, so partial reviews are never duplicated.

Provenance

Authored by: agent-work loop
Verified:
- Tests: the full .github/cursor-review/tests suite (1188 tests, Python 3.12) passes on the current head, including test_stop_on_submit.py (41 tests)
- RetryTest (real wrapper over a stub agent that reads its whole stdin): retry then success, retries used up, every attempt gets the full prompt (also when stdin starts partway in), a pipe on stdin rules the retry out, a marker on stdout isn't retried, the warning starts its own line, other errors, exit 0 and signal death not retried, submitted or partially recorded findings not re-run, retry window, signal during the backoff
- A signal during the stderr drain exits 128+signal within 2s; this test fails against the previous head
- Lints: check_workflow_pins.py and check_agents_md.py pass on the current head; check-org-repo-literals.sh, shellcheck and actionlint were run on earlier rounds
- PR CI was green on the previous head; the current head is awaiting CI
- Not run: a live panel run against Cursor (it can't be triggered from here)
Deviations: none from the acceptance criteria. See Residual for what was not exercised live.

Residual

Not checked live against Cursor. None of the six sampled failing ci-cursor-review.yml runs on the consumer repo were re-read here. The matcher is based on the quoted log tail RetriableError: [resource_exhausted] Error. After the next cursor-review bump, check a consumer run with a Kimi (kimi-k3-high) failure for the new ::warning::Reviewer cell … line and a second attempt in the Run cursor review log.
This doesn't help if Cursor's capacity stays exhausted for more than about 2 minutes. In that case the cell still ends status=error, as the ticket allows. The model swap and counting the direct-OpenAI cell toward the gate are the separate follow-ups the ticket names.
The direct-OpenAI Codex cell (review-openai-direct) and the judge/consolidate step get no retry. Both are out of scope per the ticket.

Review fixes

A retry now gets the whole prompt again. The step feeds the prompt as < /tmp/prompt.txt, and every attempt shares that one open file and its read position. The first attempt read it to the end, so a retried cursor-agent got an empty prompt and failed at once (No prompt provided for print mode). The retry could never save a cell. The wrapper now notes where stdin starts and moves it back there before every retry. If stdin can't be rewound (a pipe or a terminal), it doesn't retry and logs one line saying why. A workflow comment and the wiring test now pin the file redirect.
The ::warning:: always starts on its own line. When the agent's last stderr write had no trailing newline, the warning was glued onto it and the runner didn't pick it up. The wrapper now starts a new line first when needed.
Only stderr can trigger a retry. stdout is the model's own text, which the PR under review can influence, so the marker is no longer looked for there, and stdout goes straight to /tmp/review-raw.txt again, as before this PR. This replaces the "both stdout and stderr are scanned" judgment call above.
A signal that lands while the agent's stderr drains is reported as a signal. A step-cap signal that arrived after the agent exited (exit 0, say) but before its stderr finished draining was recorded and then ignored. The wrapper returned the agent's own code, and the drain could still run for its full 5s. Now the drain stops as soon as a signal is pending, and the wrapper exits 128+signal.

_{🔄 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/388 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `be-19849-cursor-review-retry-resource-exhausted` --- ### 📝 Commits (3) - [`1a6a8ce`](https://github.com/Comfy-Org/github-workflows/commit/1a6a8ceb01eac8ca073d9a775ec42fc47d482c88) fix(cursor-review): retry a panel cell on Cursor's transient resource_exhausted error - [`c7a5eeb`](https://github.com/Comfy-Org/github-workflows/commit/c7a5eebcfb11067b29deee2d6b772e4c266e9e82) fix(cursor-review): rewind the prompt before a retried attempt - [`454e25e`](https://github.com/Comfy-Org/github-workflows/commit/454e25e8092c28e91976d1250bc3be77616ceedb) fix(cursor-review): report a signal that lands while the agent's stderr drains ### 📊 Changes **4 files changed** (+576 additions, -23 deletions) <details> <summary>View changed files</summary> 📝 `.github/cursor-review/README.md` (+1 -1) 📝 `.github/cursor-review/stop-on-submit.py` (+253 -20) 📝 `.github/cursor-review/tests/test_stop_on_submit.py` (+308 -2) 📝 `.github/workflows/cursor-review.yml` (+14 -0) </details> ### 📄 Description ## ELI-5 Sometimes Cursor says "I'm too busy right now, try again" (`RetriableError: [resource_exhausted]`) about 25 seconds into a review cell. Today the cell gives up straight away and the panel posts with reviewers missing. With this change the cell waits a bit and tries again, at most twice, before it gives up. ## What changed - **`stop-on-submit.py`** (the wrapper around each panel cell's `cursor-agent`) gets a retry loop: - **Trigger:** the agent exits by itself with a code > 0, and the last 64 KiB of its **stderr** contains `[resource_exhausted]` or `RetriableError`. stderr passes through the wrapper and is copied on to its own stderr byte for byte, so `/tmp/review-stderr.txt` gets the same content as before. stdout is inherited untouched. - **Prompt replay:** stdin (`< /tmp/prompt.txt`) is rewound to its starting offset before every retry; a stdin that cannot seek rules the retry out. - **Backoff:** `--retry-delays 30,90`, so at most 2 retries. - **Never re-runs a submitted or partial cell:** a retry needs `findings.json` to parse as an object whose status is not `ok` **and** whose `findings` list is empty. `review-output-mcp.py` appends each recorded finding, so re-running a cell that already recorded some would duplicate them. Those cells, and an unreadable file, fail exactly as they do today. - **Fits the step cap:** no retry starts more than `--retry-window 300` seconds after the wrapper started (delay included). The last attempt therefore still has about 10 of the 15 minutes for a full review. If it runs past that, the step cap kills it as before. - **Never retried:** exit 0, any other error, a death by signal, and a forwarded signal (the step cap or a cancel, including one that arrives during the backoff or while the agent's stderr drains, which ends the wrapper with 128+signal). - **Logging:** each retry prints one `::warning::Reviewer cell <review_type>/<model>: attempt N/3 failed with Cursor's retriable error [resource_exhausted] (exit 1); retrying in 30s`. The warning names only the fixed marker, never text the agent printed, and the cell label is sanitized. - **`cursor-review.yml`** `Run cursor review` passes `--retry-delays 30,90 --retry-window 300 --cell "$REVIEW_TYPE/$MODEL"` (it adds `REVIEW_TYPE` to the step env) and has a comment explaining why. A cell that uses up its retries still ends `status=error`, and `Fail the leg when the cell did not submit` still turns it red. - README row for `stop-on-submit.py` updated. ## Judgment calls - **The retry lives in the Python wrapper, not the step shell.** That makes it unit-testable against a stub agent, and it keeps the existing signal forwarding and process-group SIGKILL working for each attempt. - **Only stderr is scanned** (changed in review, see Review fixes). The first version scanned stdout too, but stdout is the model's own text, which the PR under review can steer. - **"No structured findings were submitted" is read strictly** as no `ok` status *and* no recorded findings, so partial reviews are never duplicated. ## Provenance - **Authored by:** agent-work loop - **Verified:** - Tests: the full `.github/cursor-review/tests` suite (1188 tests, Python 3.12) passes on the current head, including `test_stop_on_submit.py` (41 tests) - `RetryTest` (real wrapper over a stub agent that reads its whole stdin): retry then success, retries used up, every attempt gets the full prompt (also when stdin starts partway in), a pipe on stdin rules the retry out, a marker on stdout isn't retried, the warning starts its own line, other errors, exit 0 and signal death not retried, submitted or partially recorded findings not re-run, retry window, signal during the backoff - A signal during the stderr drain exits 128+signal within 2s; this test fails against the previous head - Lints: `check_workflow_pins.py` and `check_agents_md.py` pass on the current head; `check-org-repo-literals.sh`, shellcheck and actionlint were run on earlier rounds - PR CI was green on the previous head; the current head is awaiting CI - Not run: a live panel run against Cursor (it can't be triggered from here) - **Deviations:** none from the acceptance criteria. See Residual for what was not exercised live. ## Residual - **Not checked live against Cursor.** None of the six sampled failing `ci-cursor-review.yml` runs on the consumer repo were re-read here. The matcher is based on the quoted log tail `RetriableError: [resource_exhausted] Error`. After the next cursor-review bump, check a consumer run with a Kimi (`kimi-k3-high`) failure for the new `::warning::Reviewer cell …` line and a second attempt in the `Run cursor review` log. - **This doesn't help if Cursor's capacity stays exhausted for more than about 2 minutes.** In that case the cell still ends `status=error`, as the ticket allows. The model swap and counting the direct-OpenAI cell toward the gate are the separate follow-ups the ticket names. - **The direct-OpenAI Codex cell (`review-openai-direct`) and the judge/consolidate step get no retry.** Both are out of scope per the ticket. ## Review fixes - **A retry now gets the whole prompt again.** The step feeds the prompt as `< /tmp/prompt.txt`, and every attempt shares that one open file and its read position. The first attempt read it to the end, so a retried `cursor-agent` got an empty prompt and failed at once (`No prompt provided for print mode`). The retry could never save a cell. The wrapper now notes where stdin starts and moves it back there before every retry. If stdin can't be rewound (a pipe or a terminal), it doesn't retry and logs one line saying why. A workflow comment and the wiring test now pin the file redirect. - **The `::warning::` always starts on its own line.** When the agent's last stderr write had no trailing newline, the warning was glued onto it and the runner didn't pick it up. The wrapper now starts a new line first when needed. - **Only stderr can trigger a retry.** stdout is the model's own text, which the PR under review can influence, so the marker is no longer looked for there, and stdout goes straight to `/tmp/review-raw.txt` again, as before this PR. This replaces the "both stdout and stderr are scanned" judgment call above. - **A signal that lands while the agent's stderr drains is reported as a signal.** A step-cap signal that arrived after the agent exited (exit 0, say) but before its stderr finished draining was recorded and then ignored. The wrapper returned the agent's own code, and the drain could still run for its full 5s. Now the drain stops as soon as a signal is pending, and the wrapper exits 128+signal. --- <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:17 +00:00