Skip to content

Drop recoverable SSE error text when a remote turn's done event is ok - #352

Merged
tylerreckart merged 3 commits into
mainfrom
fix/sse-done-ok-drops-stale-error
Sep 21, 2026
Merged

tylerreckart merged 3 commits into
mainfrom
fix/sse-done-ok-drops-stale-error

Conversation

@cursor

@cursor cursor Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Problem

RemoteSseTurnConsumer accumulates SSE error events into error_, then finish() copied that string even when the terminal done event had ok: true.

SSE error is documented as recoverable (the stream may continue). Production path: orchestrate emits error for a stored-agent catalog skip (skipping stored agent …) and still finishes with done.ok=true. The remote TUI (arbiter --connect) then returned ApiResponse.ok=true with a leftover error — the same leftover-error class as the 0.13.4 continuation-failure fix.

Fix

done is the terminal aggregate. On ok: true, clear accumulated error text. On ok: false, prefer done.error when present and fall back to prior error events when that field is empty. finish() does not copy error_ onto a successful result.

Tests

unit_remote_connect — catalog-skip error then done.ok=true leaves error empty; done.ok=false without an error field keeps the SSE message; done.error still wins over an earlier skip. Suite 25/25 locally (158 assertions), including ASan. arbiter compiles.

Independently mergeable against main (0456350). Code hunks (sse_turn.cpp / test_remote_connect.cpp) are disjoint from #321–#351. CHANGELOG [Unreleased] sibling-conflicts as usual after the first of those merges.

Open in Web View Automation 

Note

Low Risk
Narrow remote TUI SSE aggregation change with targeted unit tests; no auth, persistence, or API contract changes.

Overview
Fixes remote --connect turns reporting success with a stale error string when the SSE stream emitted recoverable error events but finished with done ok: true (e.g. skipping an invalid stored agent in the catalog).

RemoteSseTurnConsumer now treats done as authoritative: on success it clears accumulated SSE error text; on failure it still uses done.error when set and otherwise keeps earlier SSE error messages. finish() no longer copies error_ onto successful results.

Unit tests in test_remote_connect.cpp cover catalog-skip-then-success, failure without done.error, and done.error overriding a prior SSE error. CHANGELOG [Unreleased] documents the fix.

Reviewed by Cursor Bugbot for commit b2eda45. Bugbot is set up for automated code reviews on this repo. Configure here.

RemoteSseTurnConsumer accumulated SSE error events into the turn result
even after done.ok=true (catalog skip of a stored agent, etc.). done is
the terminal aggregate: success now clears the result error; failure
still prefers done.error and falls back to prior error events.

Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
@tylerreckart
tylerreckart marked this pull request as ready for review September 21, 2026 12:58
@tylerreckart
tylerreckart enabled auto-merge (squash) September 21, 2026 12:58
@tylerreckart
tylerreckart merged commit 932ef03 into main Sep 21, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants