From 7539d4dc33add11dffb1e4997cd40c1b78d321a5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 17 Sep 2026 18:14:03 +0000 Subject: [PATCH] Drop recoverable SSE error text when the remote turn's done event is ok 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 --- CHANGELOG.md | 3 +++ src/remote/sse_turn.cpp | 7 ++++++- tests/test_remote_connect.cpp | 39 +++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dae12b43..72723696 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +### Fixed +- **Remote TUI: recoverable SSE `error` is not a failed turn.** `RemoteSseTurnConsumer::finish` copied accumulated `error` event text even when the terminal `done` event had `ok: true` (e.g. catalog skip of a stored agent whose JSON failed validation). `done` is authoritative: success clears the result error; failure still prefers `done.error` and falls back to prior `error` events when that field is empty. + ## [0.13.4] — 2026-09-10 ### Fixed diff --git a/src/remote/sse_turn.cpp b/src/remote/sse_turn.cpp index 14605389..1898160f 100644 --- a/src/remote/sse_turn.cpp +++ b/src/remote/sse_turn.cpp @@ -225,6 +225,11 @@ void RemoteSseTurnConsumer::handle_done(const JsonValue& payload) { if (!ok_) { const std::string err = payload.get_string("error"); if (!err.empty()) error_ = err; + } else { + // SSE `error` events are recoverable (catalog skip, transient + // upstream note). `done` is the terminal aggregate — a successful + // turn must not keep those strings as the result error. + error_.clear(); } input_tokens_ = static_cast(payload.get_number("input_tokens", 0)); output_tokens_ = static_cast(payload.get_number("output_tokens", 0)); @@ -251,7 +256,7 @@ RemoteTurnResult RemoteSseTurnConsumer::finish(bool transport_cancelled) { if (done_seen_) { r.ok = ok_; r.content = content_; - r.error = error_; + r.error = ok_ ? std::string{} : error_; return r; } // Stream closed without done — treat as failure. diff --git a/tests/test_remote_connect.cpp b/tests/test_remote_connect.cpp index e24f9208..88fc2732 100644 --- a/tests/test_remote_connect.cpp +++ b/tests/test_remote_connect.cpp @@ -122,6 +122,45 @@ TEST_CASE("RemoteSseTurnConsumer surfaces done.ok=false") { CHECK(result.error == "upstream failed"); } +TEST_CASE("RemoteSseTurnConsumer drops recoverable error on done.ok=true") { + OutputQueue queue; + StreamRenderer renderer(kMasterStream, queue); + RemoteSseTurnConsumer consumer(renderer, queue); + consumer.on_event("error", + R"({"message":"skipping stored agent 'broken' for tenant 1: invalid json"})"); + consumer.on_event("text", R"({"delta":"ok"})"); + consumer.on_event("done", + R"({"ok":true,"content":"ok","input_tokens":3,"output_tokens":1})"); + auto result = consumer.finish(false); + CHECK(result.ok); + CHECK(result.content == "ok"); + CHECK(result.error.empty()); + CHECK(result.input_tokens == 3); + CHECK(result.output_tokens == 1); +} + +TEST_CASE("RemoteSseTurnConsumer keeps SSE error when done.ok=false has no error") { + OutputQueue queue; + StreamRenderer renderer(kMasterStream, queue); + RemoteSseTurnConsumer consumer(renderer, queue); + consumer.on_event("error", R"({"message":"history load failed"})"); + consumer.on_event("done", R"({"ok":false,"content":""})"); + auto result = consumer.finish(false); + CHECK_FALSE(result.ok); + CHECK(result.error == "history load failed"); +} + +TEST_CASE("RemoteSseTurnConsumer prefers done.error over earlier SSE error") { + OutputQueue queue; + StreamRenderer renderer(kMasterStream, queue); + RemoteSseTurnConsumer consumer(renderer, queue); + consumer.on_event("error", R"({"message":"skipping stored agent 'x'"})"); + consumer.on_event("done", R"({"ok":false,"error":"agent not found","content":""})"); + auto result = consumer.finish(false); + CHECK_FALSE(result.ok); + CHECK(result.error == "agent not found"); +} + TEST_CASE("RemoteSseTurnConsumer cancel without done") { OutputQueue queue; StreamRenderer renderer(kMasterStream, queue);