Persist status code and cause on provider error events (#674)

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.
@@ -500,7 +501,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}
 			}
 		}
 

internal/agent/loop_test.go

@@ -4193,6 +4193,86 @@ func TestRunNilTraceForwardsUsage(t *testing.T) {
 	}
 }
 
+// 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())
+	}
+}
+
+// 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.

internal/cli/exec.go

@@ -778,7 +778,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)

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 == "" {

internal/providers/openai/provider.go

@@ -298,26 +298,32 @@ 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),
+			Type:       zeroruntime.StreamEventError,
+			Error:      provider.classifiedError(statusCode, chunk.Error.Message),
+			StatusCode: observedStatus,
+			Cause:      provider.redact(chunk.Error.Message),
 		})
 		state.done = true
 		return false
@@ -404,12 +410,19 @@ 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{
-		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),
 	})
 }
 

internal/providers/openai/provider_test.go

@@ -500,6 +500,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) {
 	t.Parallel()
 	// A local Ollama daemon serving a "-cloud" model answers on localhost but
@@ -526,6 +555,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) {
@@ -584,6 +622,50 @@ 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 := 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)
+	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)
+	}
+	if strings.Contains(event.Cause, "sk-secret") {
+		t.Fatalf("Cause leaked token: %q", event.Cause)
+	}
+}
+
+// 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":`)

internal/sessions/error_payload.go

@@ -0,0 +1,46 @@
+package sessions
+
+import (
+	"errors"
+
+	"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
+// 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. 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
+	if errors.As(err, &streamErr) {
+		if streamErr.StatusCode != 0 {
+			payload["statusCode"] = streamErr.StatusCode
+		}
+		if 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
+}