Repository navigation
Conversation
Signed-off-by: kevin9327 <5299031+kevin9327@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@kevin9327 Thanks for the submission. Outside of the core cause that's being addressed in another PR, are there other cases that this is an issue? |
|
Yes, I found one more, and #9853 doesn't fix it.
Here's what I checked on
On the client, that 500 goes through the generic 5xx branch (runtime.ts#L1389-L1404), so the run is re-queued with no limit. I reproduced this at the API level. I haven't checked whether the workflow editor lets you draw that edge. More generally, any other unhandled exception inside |
Summary
When
enqueue_batchanswers 5xx, webv2 treats the error as transient and re-queues the run with no attempt limit (features/queue/runtime.ts). A 500 caused by the request itself (e.g. the graph-validation crash in #9852) never succeeds, so the run was resubmitted forever, filling the server log, and the loop came back after every reload because the run is persisted.With this change, a run whose submission gets 3 consecutive 5xx responses is marked
failedwith the server's error, the same way a 4xx is handled:POST enqueue_batch → 500, retried every ~1–5 s with no end.502, 503 and 504 still retry with no limit, since they usually mean the server is restarting or behind a proxy, and a lost/ambiguous response (a non-
ApiError) still goes through reconciliation as before. The counter is per run and resets as soon as a submission succeeds or fails with a different kind of error.Related Issues / Discussions
Closes #9854. This covers the unbounded-retry part. It doesn't change Queue panel visibility of runs that are still being retried; that's UI work that can follow separately. The server-side trigger is #9852 / #9853.
QA Instructions
pnpm vitest run src/features/queue/runtime.test.ts: 66 passed. Two new tests:ApiError(500)→ the run is failed after 3 enqueue attempts and isn't submitted again (the first test fails onmain);ApiError(503)then success → still submitted, never failed.pnpm test: 620 files / 10175 tests passed.format:check,lint:oxcandlint:tscpass. The architecture vitest suites (src/architecture) pass. Thearchitecture:checkinventory scripts need Node ≥ 22 (--experimental-strip-types), so I couldn't run them on the Node 20 here; this change doesn't touch imports or ownership.Review
Remaining limitation: the retry count lives in memory, so after a page reload a run that is still pending gets up to 3 more attempts before it fails. Once it's marked failed, that state is persisted and the retries stop.
Compatibility / Rollout
No persisted-state or API changes.
Checklist
What's Newcopy (if doing a release after this PR)Need help on this PR? Tag
@codesmith-botwith what you need.