From 1d3e263e4bb3db8f1c1de2a2de418c21ed92f70d Mon Sep 17 00:00:00 2001 From: svector-anu Date: Sun, 9 Aug 2026 07:52:21 +0100 Subject: [PATCH 1/6] Persist status code and cause on provider error events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Session events of type "error" only ever stored the top-level message string, e.g. {"type":"error","payload":{"message":"provider error: provider returned error"}}. The HTTP status code and upstream cause were discarded, leaving no way to determine why a provider call failed from the CLI or its logs. - zeroruntime.StreamEvent gains StatusCode/Cause fields, set at the provider's two response-driven error sites (HTTP error response, streamed error payload). - zeroruntime.CollectedStream carries them through as ErrorStatusCode/ErrorCause. - agent.Run now returns a *zeroruntime.StreamError (implements error) instead of a flat errors.New(...), so the detail survives to the call site instead of being flattened into a string. - New sessions.ErrorEventPayload(err) builds the EventError payload, adding "statusCode"/"cause" via errors.As when present (unwrapping wrapped errors too). All three session-event call sites (exec.go, exec_spec.go, tui/model.go) now use it. Cause is built from the existing provider.redact(...) helper — the same scrubbing already applied to the classified Error string — so nothing new is ever persisted unredacted. Scoped to the OpenAI-compatible provider (matches the issue's repro: provider=gitlawb-opengateway, model=tencent/hy3). Anthropic and Gemini have the identical classifiedError/redact pattern at their own StreamEventError sites and can get the same two-field addition as a fast follow. Fixes issue 1 of #674. Issues 2 (turn resume) and 3 (duplicate terminal detection) are separate, not addressed here per the maintainer's split. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BLkz6WZYMD3AoiMHQLGvmB --- internal/agent/loop.go | 2 +- internal/cli/exec.go | 2 +- internal/cli/exec_spec.go | 2 +- internal/providers/openai/provider.go | 12 ++-- internal/providers/openai/provider_test.go | 51 ++++++++++++++ internal/sessions/error_payload.go | 31 +++++++++ internal/sessions/error_payload_test.go | 79 ++++++++++++++++++++++ internal/tui/model.go | 2 +- internal/zeroruntime/helpers.go | 7 ++ internal/zeroruntime/types.go | 25 +++++++ 10 files changed, 205 insertions(+), 8 deletions(-) create mode 100644 internal/sessions/error_payload.go create mode 100644 internal/sessions/error_payload_test.go diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 7c3d17813..83be70964 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -490,7 +490,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) } if collected.Error != "" { result.Messages = copyMessages(messages) - return result, errors.New(collected.Error) + return result, &zeroruntime.StreamError{Message: collected.Error, StatusCode: collected.ErrorStatusCode, Cause: collected.ErrorCause} } } diff --git a/internal/cli/exec.go b/internal/cli/exec.go index 2d1fe542a..23c3a48ff 100644 --- a/internal/cli/exec.go +++ b/internal/cli/exec.go @@ -761,7 +761,7 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in } return exitInterrupted } - sessionRecorder.append(sessions.EventError, map[string]any{"message": err.Error()}) + sessionRecorder.append(sessions.EventError, sessions.ErrorEventPayload(err)) if options.outputFormat == execOutputStreamJSON { writer.errorEvent("provider_error", err.Error(), false) writer.runEnd("error", exitProvider) diff --git a/internal/cli/exec_spec.go b/internal/cli/exec_spec.go index fc22eed35..cd7b873ce 100644 --- a/internal/cli/exec_spec.go +++ b/internal/cli/exec_spec.go @@ -198,7 +198,7 @@ func runExecSpecDraft(run execSpecDraftRun) int { } return exitInterrupted } - sessionRecorder.append(sessions.EventError, map[string]any{"message": err.Error()}) + sessionRecorder.append(sessions.EventError, sessions.ErrorEventPayload(err)) return writeExecSpecDraftProviderError(&writer, run.options.outputFormat, err.Error()) } if result.StopReason != agent.StopReasonSpecReviewRequired || draftInfo.SpecID == "" || draftInfo.SpecFilePath == "" { diff --git a/internal/providers/openai/provider.go b/internal/providers/openai/provider.go index 8bf2bc5ba..38d0c48a8 100644 --- a/internal/providers/openai/provider.go +++ b/internal/providers/openai/provider.go @@ -312,8 +312,10 @@ func (provider *Provider) emitPayload(ctx context.Context, data string, state *t } } sendEvent(ctx, events, zeroruntime.StreamEvent{ - Type: zeroruntime.StreamEventError, - Error: provider.classifiedError(statusCode, chunk.Error.Message), + Type: zeroruntime.StreamEventError, + Error: provider.classifiedError(statusCode, chunk.Error.Message), + StatusCode: statusCode, + Cause: provider.redact(chunk.Error.Message), }) state.done = true return false @@ -403,8 +405,10 @@ func (provider *Provider) emitHTTPError(ctx context.Context, response *http.Resp return } sendEvent(ctx, events, zeroruntime.StreamEvent{ - Type: zeroruntime.StreamEventError, - Error: provider.classifiedError(response.StatusCode, message), + Type: zeroruntime.StreamEventError, + Error: provider.classifiedError(response.StatusCode, message), + StatusCode: response.StatusCode, + Cause: provider.redact(message), }) } diff --git a/internal/providers/openai/provider_test.go b/internal/providers/openai/provider_test.go index b88f1714d..69ab6807f 100644 --- a/internal/providers/openai/provider_test.go +++ b/internal/providers/openai/provider_test.go @@ -412,6 +412,35 @@ func TestStreamCompletionClassifiesHTTPErrorsAndRedactsToken(t *testing.T) { } } +// TestStreamCompletionHTTPErrorCarriesStatusCodeAndCause verifies the StreamEvent +// for an HTTP-level provider failure carries the raw status code and a redacted +// cause separately from the classified, user-facing Error string — the detail +// a session's "error" event needs to actually diagnose why a provider call +// failed instead of only storing the flattened message (#674). +func TestStreamCompletionHTTPErrorCarriesStatusCodeAndCause(t *testing.T) { + provider := newTestProviderWithKey(t, "sk-secret", func(w http.ResponseWriter, r *http.Request) { + http.Error(w, `{"error":{"message":"upstream saw Bearer sk-secret"}}`, http.StatusInternalServerError) + }) + stream, err := provider.StreamCompletion(context.Background(), zeroruntime.CompletionRequest{}) + if err != nil { + t.Fatalf("StreamCompletion returned setup error: %v", err) + } + events := readAll(stream) + if len(events) != 1 || events[0].Type != zeroruntime.StreamEventError { + t.Fatalf("events = %#v, want one error", events) + } + event := events[0] + if event.StatusCode != http.StatusInternalServerError { + t.Fatalf("StatusCode = %d, want %d", event.StatusCode, http.StatusInternalServerError) + } + if !strings.Contains(event.Cause, "upstream saw Bearer") { + t.Fatalf("Cause = %q, want it to contain the upstream detail", event.Cause) + } + if strings.Contains(event.Cause, "sk-secret") { + t.Fatalf("Cause leaked token: %q", event.Cause) + } +} + func TestStreamCompletionHumanizesUpstreamUnreachableGatewayError(t *testing.T) { // A local Ollama daemon serving a "-cloud" model answers on localhost but // returns HTTP 502 with an opaque proxied transport error when it cannot reach @@ -493,6 +522,28 @@ func TestStreamCompletionClassifiesStreamErrorCode(t *testing.T) { } } +// TestStreamCompletionStreamedErrorCarriesStatusCodeAndCause mirrors +// TestStreamCompletionHTTPErrorCarriesStatusCodeAndCause for the other error +// path: an error arriving inside a 200 OK SSE payload's "code" field rather +// than the HTTP status (#674). +func TestStreamCompletionStreamedErrorCarriesStatusCodeAndCause(t *testing.T) { + provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) { + writeSSE(w, `{"error":{"message":"rate limited, see Bearer token docs","code":"429"}}`) + }) + + events := collectProviderEvents(t, provider) + if len(events) != 1 || events[0].Type != zeroruntime.StreamEventError { + t.Fatalf("events = %#v, want one error", events) + } + event := events[0] + if event.StatusCode != http.StatusTooManyRequests { + t.Fatalf("StatusCode = %d, want %d", event.StatusCode, http.StatusTooManyRequests) + } + if !strings.Contains(event.Cause, "rate limited") { + t.Fatalf("Cause = %q, want it to contain the upstream detail", event.Cause) + } +} + func TestStreamCompletionEmitsErrorForMalformedJSON(t *testing.T) { provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) { writeSSE(w, `{"choices":`) diff --git a/internal/sessions/error_payload.go b/internal/sessions/error_payload.go new file mode 100644 index 000000000..288443cec --- /dev/null +++ b/internal/sessions/error_payload.go @@ -0,0 +1,31 @@ +package sessions + +import ( + "errors" + + "github.com/Gitlawb/zero/internal/zeroruntime" +) + +// ErrorEventPayload builds the payload for an EventError session event. It +// always includes the flattened message (unchanged, for backward +// compatibility with existing consumers of events.jsonl), and additionally +// persists the HTTP status code and upstream cause when err is (or wraps) a +// *zeroruntime.StreamError — the structured error a failed provider stream +// returns. Without this, a provider error left no way to diagnose *why* a +// call failed from the CLI or its logs — only the generic top-level message +// was ever recorded (#674). Cause has already been redacted for secrets by +// the provider before reaching here (see zeroruntime.StreamEvent.Cause) — it +// is never re-scrubbed or stored raw at this layer. +func ErrorEventPayload(err error) map[string]any { + payload := map[string]any{"message": err.Error()} + var streamErr *zeroruntime.StreamError + if errors.As(err, &streamErr) { + if streamErr.StatusCode != 0 { + payload["statusCode"] = streamErr.StatusCode + } + if streamErr.Cause != "" { + payload["cause"] = streamErr.Cause + } + } + return payload +} diff --git a/internal/sessions/error_payload_test.go b/internal/sessions/error_payload_test.go new file mode 100644 index 000000000..1318a9b6b --- /dev/null +++ b/internal/sessions/error_payload_test.go @@ -0,0 +1,79 @@ +package sessions + +import ( + "errors" + "fmt" + "testing" + + "github.com/Gitlawb/zero/internal/zeroruntime" +) + +func TestErrorEventPayloadPlainError(t *testing.T) { + payload := ErrorEventPayload(errors.New("provider error: provider returned error")) + + if got, want := payload["message"], "provider error: provider returned error"; got != want { + t.Fatalf("message = %v, want %v", got, want) + } + if _, ok := payload["statusCode"]; ok { + t.Fatalf("statusCode should be absent for a plain error, got %v", payload["statusCode"]) + } + if _, ok := payload["cause"]; ok { + t.Fatalf("cause should be absent for a plain error, got %v", payload["cause"]) + } +} + +func TestErrorEventPayloadStreamError(t *testing.T) { + err := &zeroruntime.StreamError{ + Message: "provider error: provider returned error", + StatusCode: 500, + Cause: "upstream returned an empty completion", + } + + payload := ErrorEventPayload(err) + + if got, want := payload["message"], "provider error: provider returned error"; got != want { + t.Fatalf("message = %v, want %v", got, want) + } + if got, want := payload["statusCode"], 500; got != want { + t.Fatalf("statusCode = %v, want %v", got, want) + } + if got, want := payload["cause"], "upstream returned an empty completion"; got != want { + t.Fatalf("cause = %v, want %v", got, want) + } +} + +func TestErrorEventPayloadWrappedStreamError(t *testing.T) { + // A StreamError wrapped by another layer (e.g. fmt.Errorf("...: %w", err)) + // must still surface its status/cause via errors.As, not just a direct type + // assertion — this is the shape #674's real repro chain produces. + inner := &zeroruntime.StreamError{Message: "rate limit error: too many requests", StatusCode: 429, Cause: "retry after 30s"} + wrapped := fmt.Errorf("turn failed: %w", inner) + + payload := ErrorEventPayload(wrapped) + + if got, want := payload["statusCode"], 429; got != want { + t.Fatalf("statusCode = %v, want %v", got, want) + } + if got, want := payload["cause"], "retry after 30s"; got != want { + t.Fatalf("cause = %v, want %v", got, want) + } + if got, want := payload["message"], "turn failed: rate limit error: too many requests"; got != want { + t.Fatalf("message = %v, want %v", got, want) + } +} + +func TestErrorEventPayloadZeroStatusCodeOmitted(t *testing.T) { + // A transport-level failure (e.g. context cancellation) never got an HTTP + // response, so StatusCode is legitimately 0 — that must stay omitted rather + // than be persisted as a misleading "statusCode": 0. + err := &zeroruntime.StreamError{Message: "context deadline exceeded"} + + payload := ErrorEventPayload(err) + + if _, ok := payload["statusCode"]; ok { + t.Fatalf("statusCode should be omitted when zero, got %v", payload["statusCode"]) + } + if _, ok := payload["cause"]; ok { + t.Fatalf("cause should be omitted when empty, got %v", payload["cause"]) + } +} diff --git a/internal/tui/model.go b/internal/tui/model.go index 916b26221..9c778f0d3 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -5524,7 +5524,7 @@ func (m model) runAgentWithOptions(runID int, runCtx context.Context, prompt str flushReasoning(m.now()) sessionEvents = append(sessionEvents, pendingSessionEvent{ Type: sessions.EventError, - Payload: map[string]any{"message": err.Error()}, + Payload: sessions.ErrorEventPayload(err), }) return agentResponseMsg{runID: runID, rows: rows, usageEvents: usageEvents, usageModelID: usageModelID, sessionEvents: sessionEvents, err: err, goalAware: goalAwareRun, turnTools: toolCalls, turnElapsed: m.activeTurnElapsed(started)} } diff --git a/internal/zeroruntime/helpers.go b/internal/zeroruntime/helpers.go index 0d7a2ec51..03888db84 100644 --- a/internal/zeroruntime/helpers.go +++ b/internal/zeroruntime/helpers.go @@ -12,6 +12,11 @@ type CollectedStream struct { ToolCalls []ToolCall Usage Usage Error string + // ErrorStatusCode and ErrorCause mirror StreamEvent.StatusCode/Cause for a + // StreamEventError — see those fields for what each carries. Both are zero + // value when Error came from a context cancellation or other non-HTTP source. + ErrorStatusCode int + ErrorCause string DroppedToolCalls int // malformed tool calls the provider could not dispatch // FinishReason is the provider's normalized terminal stop reason when the // response did not end normally (FinishReasonLength / FinishReasonContentFilter). @@ -153,6 +158,8 @@ func CollectStreamWithOptions(ctx context.Context, events <-chan StreamEvent, op usageSeen = true case StreamEventError: collected.Error = event.Error + collected.ErrorStatusCode = event.StatusCode + collected.ErrorCause = event.Cause return finish() case StreamEventDone: return finish() diff --git a/internal/zeroruntime/types.go b/internal/zeroruntime/types.go index dc76ef79a..ad9d28fc8 100644 --- a/internal/zeroruntime/types.go +++ b/internal/zeroruntime/types.go @@ -184,6 +184,17 @@ type StreamEvent struct { ArgumentsFragment string Usage Usage Error string + // StatusCode is the HTTP status code behind Error, when the failure came from + // an HTTP response (0 for transport-level/context errors that never received + // one). Providers set it alongside Error on a StreamEventError so the status + // survives past the flattened error string (#674). + StatusCode int + // Cause is the raw, secret-redacted upstream detail behind Error — the + // provider's own error message or response body — kept separate from Error's + // curated/classified wording so both are preserved rather than one + // overwriting the other. Always passed through the same redaction already + // applied to Error (see provider.redact); never stored unredacted. + Cause string // FinishReason carries the provider's normalized terminal stop reason when a // response did not end normally (e.g. FinishReasonLength when the output hit // the token cap, or FinishReasonContentFilter when it was filtered). It is @@ -195,6 +206,20 @@ type StreamEvent struct { ReasoningBlocks []ReasoningBlock } +// StreamError is the terminal error returned for a failed provider stream. It +// implements error via Message, so existing `err.Error()` call sites are +// unaffected by this type; callers that need the underlying HTTP status code or +// upstream cause recover them with errors.As instead of re-parsing the message +// string. This is what lets a session's "error" event persist real diagnostic +// detail instead of only the flattened message (#674). +type StreamError struct { + Message string + StatusCode int + Cause string +} + +func (e *StreamError) Error() string { return e.Message } + // CompletionRequest groups provider input messages and available tools. type CompletionRequest struct { Messages []Message From 8f712b6a8e3a369232c17bcbdd918e5d3d415016 Mon Sep 17 00:00:00 2001 From: svector-anu Date: Sun, 9 Aug 2026 08:10:16 +0100 Subject: [PATCH 2/6] Address CodeRabbit review: upstream-unreachable metadata + test coverage - emitHTTPError's UpstreamUnreachable branch was dropping StatusCode/ Cause (only the humanized Error string was set), unlike the classifiedError branch right below it. A proxy connectivity failure (e.g. a local Ollama daemon serving a "-cloud" model) lost its response metadata even though response.StatusCode was available. Now sets both, same as the main path. - TestStreamCompletionStreamedErrorCarriesStatusCodeAndCause now includes a secret-shaped value in the SSE error message and asserts it's absent from Cause, matching the redaction check already done for the HTTP-error path. - TestStreamCompletionHumanizesUpstreamUnreachableGatewayError now asserts StatusCode/Cause on the upstream-unreachable path (the fix above). - New TestRunReturnsStreamErrorWithStatusCodeAndCause in internal/agent covers the collector-to-agent propagation boundary end-to-end: a provider emitting StreamEventError with StatusCode/ Cause set results in Run returning an error errors.As can still recover those fields from. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01BLkz6WZYMD3AoiMHQLGvmB --- internal/agent/loop_test.go | 40 ++++++++++++++++++++++ internal/providers/openai/provider.go | 7 +++- internal/providers/openai/provider_test.go | 16 +++++++-- 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index 46f0affb1..f0b5938d2 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -3800,3 +3800,43 @@ func TestRunNilTraceForwardsUsage(t *testing.T) { t.Fatal("OnUsage not forwarded when Trace is nil") } } + +// TestRunReturnsStreamErrorWithStatusCodeAndCause covers the collector-to-agent +// propagation boundary: a provider stream that terminates with a StreamEventError +// carrying StatusCode/Cause must have Run return an error that errors.As can +// still recover those fields from — the detail a session's "error" event needs +// to actually diagnose why a provider call failed (#674). +func TestRunReturnsStreamErrorWithStatusCodeAndCause(t *testing.T) { + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{{ + { + Type: zeroruntime.StreamEventError, + Error: "provider error: provider returned error", + StatusCode: 500, + Cause: "upstream returned an empty completion", + }, + }}} + + _, err := Run(context.Background(), "hi", provider, Options{ + SessionID: "stream-error-session", + Cwd: t.TempDir(), + ProviderName: "test-provider", + Model: "test-model", + }) + if err == nil { + t.Fatal("Run: expected an error, got nil") + } + + var streamErr *zeroruntime.StreamError + if !errors.As(err, &streamErr) { + t.Fatalf("Run error %v (%T) does not unwrap to *zeroruntime.StreamError", err, err) + } + if streamErr.StatusCode != 500 { + t.Fatalf("StatusCode = %d, want 500", streamErr.StatusCode) + } + if streamErr.Cause != "upstream returned an empty completion" { + t.Fatalf("Cause = %q, want %q", streamErr.Cause, "upstream returned an empty completion") + } + if err.Error() != "provider error: provider returned error" { + t.Fatalf("Error() = %q, want the flattened message unchanged", err.Error()) + } +} diff --git a/internal/providers/openai/provider.go b/internal/providers/openai/provider.go index 38d0c48a8..373883d58 100644 --- a/internal/providers/openai/provider.go +++ b/internal/providers/openai/provider.go @@ -401,7 +401,12 @@ func (provider *Provider) emitHTTPError(ctx context.Context, response *http.Resp // localhost but returns a gateway error when it cannot reach its own backend. // Surface that as a clear connectivity message instead of the raw proxied body. if humanized, ok := providerio.UpstreamUnreachable(message); ok { - sendEvent(ctx, events, zeroruntime.StreamEvent{Type: zeroruntime.StreamEventError, Error: provider.redact(humanized)}) + sendEvent(ctx, events, zeroruntime.StreamEvent{ + Type: zeroruntime.StreamEventError, + Error: provider.redact(humanized), + StatusCode: response.StatusCode, + Cause: provider.redact(message), + }) return } sendEvent(ctx, events, zeroruntime.StreamEvent{ diff --git a/internal/providers/openai/provider_test.go b/internal/providers/openai/provider_test.go index 69ab6807f..2f042005f 100644 --- a/internal/providers/openai/provider_test.go +++ b/internal/providers/openai/provider_test.go @@ -466,6 +466,15 @@ func TestStreamCompletionHumanizesUpstreamUnreachableGatewayError(t *testing.T) t.Fatalf("error = %q, want it to contain %q", got, want) } } + // The humanized "upstream unreachable" message must not drop the response + // metadata a session error event needs — StatusCode/Cause must still reflect + // the real HTTP response, matching the classified-error path below it (#674). + if events[0].StatusCode != http.StatusBadGateway { + t.Fatalf("StatusCode = %d, want %d", events[0].StatusCode, http.StatusBadGateway) + } + if !strings.Contains(events[0].Cause, "TLS handshake timeout") { + t.Fatalf("Cause = %q, want it to contain the raw upstream detail", events[0].Cause) + } } func TestStreamCompletionEmitsStreamErrorObject(t *testing.T) { @@ -527,8 +536,8 @@ func TestStreamCompletionClassifiesStreamErrorCode(t *testing.T) { // path: an error arriving inside a 200 OK SSE payload's "code" field rather // than the HTTP status (#674). func TestStreamCompletionStreamedErrorCarriesStatusCodeAndCause(t *testing.T) { - provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) { - writeSSE(w, `{"error":{"message":"rate limited, see Bearer token docs","code":"429"}}`) + provider := newTestProviderWithKey(t, "sk-secret", func(w http.ResponseWriter, r *http.Request) { + writeSSE(w, `{"error":{"message":"rate limited sk-secret, see Bearer token docs","code":"429"}}`) }) events := collectProviderEvents(t, provider) @@ -542,6 +551,9 @@ func TestStreamCompletionStreamedErrorCarriesStatusCodeAndCause(t *testing.T) { if !strings.Contains(event.Cause, "rate limited") { t.Fatalf("Cause = %q, want it to contain the upstream detail", event.Cause) } + if strings.Contains(event.Cause, "sk-secret") { + t.Fatalf("Cause leaked token: %q", event.Cause) + } } func TestStreamCompletionEmitsErrorForMalformedJSON(t *testing.T) { From c76201399c7c0c4345331aab330c7bf3a6551c90 Mon Sep 17 00:00:00 2001 From: svector-anu Date: Mon, 10 Aug 2026 08:39:30 +0100 Subject: [PATCH 3/6] fix(zeroruntime): gofmt struct alignment in CollectedStream --- internal/zeroruntime/helpers.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/zeroruntime/helpers.go b/internal/zeroruntime/helpers.go index 03888db84..4a192f4b3 100644 --- a/internal/zeroruntime/helpers.go +++ b/internal/zeroruntime/helpers.go @@ -8,10 +8,10 @@ import ( // CollectedStream is the non-streaming summary of provider events. type CollectedStream struct { - Text string - ToolCalls []ToolCall - Usage Usage - Error string + Text string + ToolCalls []ToolCall + Usage Usage + Error string // ErrorStatusCode and ErrorCause mirror StreamEvent.StatusCode/Cause for a // StreamEventError — see those fields for what each carries. Both are zero // value when Error came from a context cancellation or other non-HTTP source. From 08e6a61562864a2d9e3f720ab01de3c865e9cb84 Mon Sep 17 00:00:00 2001 From: svector-anu Date: Mon, 10 Aug 2026 08:42:37 +0100 Subject: [PATCH 4/6] test(sessions): assert error event round-trips statusCode/cause through Store --- internal/sessions/error_payload_test.go | 48 +++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/internal/sessions/error_payload_test.go b/internal/sessions/error_payload_test.go index 1318a9b6b..4fe9450f8 100644 --- a/internal/sessions/error_payload_test.go +++ b/internal/sessions/error_payload_test.go @@ -1,6 +1,7 @@ package sessions import ( + "encoding/json" "errors" "fmt" "testing" @@ -77,3 +78,50 @@ func TestErrorEventPayloadZeroStatusCodeOmitted(t *testing.T) { t.Fatalf("cause should be omitted when empty, got %v", payload["cause"]) } } + +func TestStoreAppendEventPersistsErrorEventStatusCodeAndCause(t *testing.T) { + // Closes the seam between ErrorEventPayload (unit tested above) and Run + // returning a *StreamError (tested in internal/agent): this asserts the + // two actually meet at the Store, i.e. a *StreamError survives a real + // AppendEvent/ReadEvents round trip with statusCode/cause intact. + store := NewStore(StoreOptions{RootDir: t.TempDir(), Now: fixedClock("2026-08-10T12:00:00Z")}) + session, err := store.Create(CreateInput{SessionID: "error_join"}) + if err != nil { + t.Fatalf("Create returned error: %v", err) + } + + streamErr := &zeroruntime.StreamError{ + Message: "provider error: provider returned error", + StatusCode: 503, + Cause: "upstream returned an empty completion", + } + + if _, err := store.AppendEvent(session.SessionID, AppendEventInput{ + Type: EventError, + Payload: ErrorEventPayload(streamErr), + }); err != nil { + t.Fatalf("AppendEvent returned error: %v", err) + } + + events, err := store.ReadEvents(session.SessionID) + if err != nil { + t.Fatalf("ReadEvents returned error: %v", err) + } + if len(events) != 1 || events[0].Type != EventError { + t.Fatalf("expected a single error event, got %#v", events) + } + + var payload map[string]any + if err := json.Unmarshal(events[0].Payload, &payload); err != nil { + t.Fatalf("failed to unmarshal persisted payload: %v", err) + } + if got, want := payload["message"], "provider error: provider returned error"; got != want { + t.Fatalf("persisted message = %v, want %v", got, want) + } + if got, want := payload["statusCode"], float64(503); got != want { + t.Fatalf("persisted statusCode = %v, want %v", got, want) + } + if got, want := payload["cause"], "upstream returned an empty completion"; got != want { + t.Fatalf("persisted cause = %v, want %v", got, want) + } +} From ba92df91245f41d087de0a7bbe96da4894f38ca9 Mon Sep 17 00:00:00 2001 From: svector-anu Date: Thu, 8 Oct 2026 15:50:01 +0100 Subject: [PATCH 5/6] fix(agent): keep StreamError through image-rejection recovery The 400 image-rejection branch returned a plain fmt.Errorf, so errors.As could not recover StatusCode/Cause and the session error event persisted only the message. Wrap the StreamError with %w; the user-facing message is unchanged. --- internal/agent/loop.go | 3 ++- internal/agent/loop_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 0ae3fc83f..417231f32 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -404,7 +404,8 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) // collected and a non-nil stop error when the run must end now. recoverStreamError := func(collected zeroruntime.CollectedStream) (zeroruntime.CollectedStream, error) { if isImageRejectionError(errors.New(collected.Error)) { - return collected, fmt.Errorf("model %s rejected the image: %s. The model may not support image input — try switching to a vision-capable model (claude, gpt-4o, gemini)", options.Model, collected.Error) + cause := &zeroruntime.StreamError{Message: collected.Error, StatusCode: collected.ErrorStatusCode, Cause: collected.ErrorCause} + return collected, fmt.Errorf("model %s rejected the image: %w. The model may not support image input — try switching to a vision-capable model (claude, gpt-4o, gemini)", options.Model, cause) } // REACTIVE compaction: the streamed error may also be a context limit // (some providers surface it mid-stream). Compact and retry once. diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index ba78f69a1..29e5975de 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -4233,6 +4233,46 @@ func TestRunReturnsStreamErrorWithStatusCodeAndCause(t *testing.T) { } } +// TestRunImageRejectionKeepsStreamErrorStatusCodeAndCause covers the 400 +// image-rejection recovery path: the friendly message is kept, but the returned +// error must still unwrap to the *StreamError so StatusCode and Cause reach the +// session's "error" event (#674). +func TestRunImageRejectionKeepsStreamErrorStatusCodeAndCause(t *testing.T) { + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{{ + { + Type: zeroruntime.StreamEventError, + Error: "provider error 400: image input is not supported", + StatusCode: 400, + Cause: "this model does not support image input", + }, + }}} + + _, err := Run(context.Background(), "hi", provider, Options{ + SessionID: "image-rejection-session", + Cwd: t.TempDir(), + ProviderName: "test-provider", + Model: "test-model", + }) + if err == nil { + t.Fatal("Run: expected an error, got nil") + } + + want := "model test-model rejected the image: provider error 400: image input is not supported. The model may not support image input — try switching to a vision-capable model (claude, gpt-4o, gemini)" + if err.Error() != want { + t.Fatalf("Error() = %q, want the friendly message unchanged: %q", err.Error(), want) + } + var streamErr *zeroruntime.StreamError + if !errors.As(err, &streamErr) { + t.Fatalf("Run error %v (%T) does not unwrap to *zeroruntime.StreamError", err, err) + } + if streamErr.StatusCode != 400 { + t.Fatalf("StatusCode = %d, want 400", streamErr.StatusCode) + } + if streamErr.Cause != "this model does not support image input" { + t.Fatalf("Cause = %q, want the upstream cause", streamErr.Cause) + } +} + // TestRunSuppressesAdvisoryHooksInPlanMode: plan mode promises a read-only // turn for advisory hooks (sessionStart/sessionEnd/afterTool), which execute // configured host commands outside the advertised-tool and sandbox gates. From 571675db9ceda30a12d57dc252ffefe21d2319d3 Mon Sep 17 00:00:00 2001 From: svector-anu Date: Thu, 8 Oct 2026 15:50:33 +0100 Subject: [PATCH 6/6] fix(providers): stop persisting a bucketed 500 and bound error cause A streamed error arrives on a 200 response, so the local 500 default used for classification is not an observed status; report StatusCode only when the payload code maps to a known status. Cap the persisted cause at 8 KiB (rune-safe, with a truncation marker) since provider bodies read up to 64 KiB. --- internal/providers/openai/provider.go | 12 +++++--- internal/providers/openai/provider_test.go | 19 +++++++++++++ internal/sessions/error_payload.go | 19 +++++++++++-- internal/sessions/error_payload_test.go | 32 ++++++++++++++++++++++ 4 files changed, 76 insertions(+), 6 deletions(-) diff --git a/internal/providers/openai/provider.go b/internal/providers/openai/provider.go index 5ef563bcc..c5937f542 100644 --- a/internal/providers/openai/provider.go +++ b/internal/providers/openai/provider.go @@ -298,27 +298,31 @@ func (provider *Provider) emitPayload(ctx context.Context, data string, state *t if chunk.Error != nil { state.flushContent(ctx, events) state.closeOpen(ctx, events) + // statusCode buckets the error for classification only. The HTTP response + // was 200, so observedStatus stays 0 unless the payload's code maps to a + // known status; a bucket default must not be persisted as a real status. statusCode := http.StatusInternalServerError + observedStatus := 0 if chunk.Error.Code != nil { switch c := chunk.Error.Code.(type) { case string: if code, ok := openAIStreamErrorStatusByCode[c]; ok { - statusCode = code + statusCode, observedStatus = code, code } case float64: if code, ok := openAIStreamErrorStatusByCode[strconv.Itoa(int(c))]; ok { - statusCode = code + statusCode, observedStatus = code, code } case int: if code, ok := openAIStreamErrorStatusByCode[strconv.Itoa(c)]; ok { - statusCode = code + statusCode, observedStatus = code, code } } } sendEvent(ctx, events, zeroruntime.StreamEvent{ Type: zeroruntime.StreamEventError, Error: provider.classifiedError(statusCode, chunk.Error.Message), - StatusCode: statusCode, + StatusCode: observedStatus, Cause: provider.redact(chunk.Error.Message), }) state.done = true diff --git a/internal/providers/openai/provider_test.go b/internal/providers/openai/provider_test.go index 92f028e9a..dd3c897b0 100644 --- a/internal/providers/openai/provider_test.go +++ b/internal/providers/openai/provider_test.go @@ -647,6 +647,25 @@ func TestStreamCompletionStreamedErrorCarriesStatusCodeAndCause(t *testing.T) { } } +// An error inside a 200 OK SSE payload whose code is not a known status was +// never an observed HTTP status, so none must be reported (#674 review). +func TestStreamCompletionStreamedErrorWithUnknownCodeReportsNoStatus(t *testing.T) { + provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) { + writeSSE(w, `{"error":{"message":"model overloaded","code":"server_error"}}`) + }) + + events := collectProviderEvents(t, provider) + if len(events) != 1 || events[0].Type != zeroruntime.StreamEventError { + t.Fatalf("events = %#v, want one error", events) + } + if events[0].StatusCode != 0 { + t.Fatalf("StatusCode = %d, want 0 (no HTTP status was observed)", events[0].StatusCode) + } + if !strings.Contains(events[0].Cause, "model overloaded") { + t.Fatalf("Cause = %q, want the upstream detail", events[0].Cause) + } +} + func TestStreamCompletionEmitsErrorForMalformedJSON(t *testing.T) { provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) { writeSSE(w, `{"choices":`) diff --git a/internal/sessions/error_payload.go b/internal/sessions/error_payload.go index 288443cec..efb61898e 100644 --- a/internal/sessions/error_payload.go +++ b/internal/sessions/error_payload.go @@ -6,6 +6,13 @@ import ( "github.com/Gitlawb/zero/internal/zeroruntime" ) +// maxErrorCauseBytes bounds the persisted cause. Provider error bodies are read +// up to 64 KiB, which is far more than a diagnostic needs and would otherwise be +// copied whole into every error event in events.jsonl. +const maxErrorCauseBytes = 8 << 10 + +const errorCauseTruncationMarker = "… [truncated]" + // ErrorEventPayload builds the payload for an EventError session event. It // always includes the flattened message (unchanged, for backward // compatibility with existing consumers of events.jsonl), and additionally @@ -15,7 +22,8 @@ import ( // call failed from the CLI or its logs — only the generic top-level message // was ever recorded (#674). Cause has already been redacted for secrets by // the provider before reaching here (see zeroruntime.StreamEvent.Cause) — it -// is never re-scrubbed or stored raw at this layer. +// is never re-scrubbed or stored raw at this layer. Cause is cut to +// maxErrorCauseBytes (on a rune boundary) with a truncation marker appended. func ErrorEventPayload(err error) map[string]any { payload := map[string]any{"message": err.Error()} var streamErr *zeroruntime.StreamError @@ -24,8 +32,15 @@ func ErrorEventPayload(err error) map[string]any { payload["statusCode"] = streamErr.StatusCode } if streamErr.Cause != "" { - payload["cause"] = streamErr.Cause + payload["cause"] = boundErrorCause(streamErr.Cause) } } return payload } + +func boundErrorCause(cause string) string { + if len(cause) <= maxErrorCauseBytes { + return cause + } + return truncateUTF8(cause, maxErrorCauseBytes) + errorCauseTruncationMarker +} diff --git a/internal/sessions/error_payload_test.go b/internal/sessions/error_payload_test.go index 4fe9450f8..4f0b106c9 100644 --- a/internal/sessions/error_payload_test.go +++ b/internal/sessions/error_payload_test.go @@ -4,7 +4,9 @@ import ( "encoding/json" "errors" "fmt" + "strings" "testing" + "unicode/utf8" "github.com/Gitlawb/zero/internal/zeroruntime" ) @@ -125,3 +127,33 @@ func TestStoreAppendEventPersistsErrorEventStatusCodeAndCause(t *testing.T) { t.Fatalf("persisted cause = %v, want %v", got, want) } } + +func TestErrorEventPayloadBoundsCause(t *testing.T) { + // "é" is two bytes; an odd cap boundary must not split it. + cause := strings.Repeat("é", maxErrorCauseBytes) + payload := ErrorEventPayload(&zeroruntime.StreamError{Message: "m", StatusCode: 500, Cause: cause}) + + got, ok := payload["cause"].(string) + if !ok { + t.Fatalf("cause = %#v, want string", payload["cause"]) + } + if !strings.HasSuffix(got, errorCauseTruncationMarker) { + t.Fatalf("cause missing truncation marker: ...%q", got[len(got)-20:]) + } + body := strings.TrimSuffix(got, errorCauseTruncationMarker) + if len(body) > maxErrorCauseBytes { + t.Fatalf("bounded cause body is %d bytes, want <= %d", len(body), maxErrorCauseBytes) + } + if !utf8.ValidString(got) { + t.Fatal("bounded cause is not valid UTF-8") + } +} + +func TestErrorEventPayloadKeepsCauseAtCap(t *testing.T) { + cause := strings.Repeat("a", maxErrorCauseBytes) + payload := ErrorEventPayload(&zeroruntime.StreamError{Message: "m", Cause: cause}) + + if got := payload["cause"]; got != cause { + t.Fatalf("cause at exactly the cap was altered (len %d)", len(got.(string))) + } +}