fix(gateway): surface resolved chat errors#84953
Conversation
|
Codex review: needs maintainer review before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. by source inspection. Current main can receive resolved idle-timeout payloads with PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land this gateway fix after normal maintainer and CI review, keeping the terminal-error handling in the gateway post-dispatch branch and the focused replay coverage with it. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection. Current main can receive resolved idle-timeout payloads with Is this the best way to solve the issue? Yes. The gateway post-dispatch branch is the narrow ownership boundary for live chat broadcasts and dedupe replay, and the latest PR head covers both the initial terminal error and cached replay semantics. Label changes:
Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against b77f36fb1cbd. |
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Brave Diff Drake Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
Broadcast delivered error payloads from completed agent runs so clients see terminal failures instead of silently stopping. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
f7e0bf3 to
380c233
Compare
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Maintainer proof before landing: Behavior addressed: resolved gateway chat reply payloads marked isError emit a terminal chat error instead of leaving the run in progress |
|
Thanks for tracking this down and for the detailed proof. I'm going to close this as superseded by current Proof on current main:
Appreciate the clean repro and tests here; the fix shape landed through the newer PR, so we do not need to carry this conflicting branch forward. |
Summary
state: "error"when a completed agent run deliveredisErrorreply payloads, covering idle-timeout failures that resolve instead of throw.Fixes #84945
Real behavior proof (required for external PRs)
isError: true(for exampleLLM idle timeout (120s): no response from model). After this patch, that path emits a terminal chat event withstate: "error"instead of leaving the web chat stuck as an in-progress run, and it still emits the user transcript update before the terminal error broadcast.feat/issue-84945, headc26ee4098bc851a0f1ee5ef3f36888f69b2f3db6, Node via repository scripts. The repro exercised the gateway chat send path with an ACP-style delivered error payload.node scripts/run-vitest.mjs src/gateway/server-methods/chat.directive-tags.test.tsnode scripts/run-vitest.mjs src/gateway/server.chat.gateway-server-chat.test.tsgit diff --checknode scripts/check-changed.mjschatbroadcast withstate: "error"and the exact idle-timeout message, and the user transcript update is observed before the error broadcast.Test plan
node scripts/run-vitest.mjs src/gateway/server-methods/chat.directive-tags.test.tsnode scripts/run-vitest.mjs src/gateway/server.chat.gateway-server-chat.test.tsgit diff --checknode scripts/check-changed.mjs