Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions internal/agent/loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,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.
Expand Down Expand Up @@ -542,7 +543,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}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}
}

Expand Down
80 changes: 80 additions & 0 deletions internal/agent/loop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,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)
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/exec_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,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 == "" {
Expand Down
29 changes: 21 additions & 8 deletions internal/providers/openai/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
})
}

Expand Down
82 changes: 82 additions & 0 deletions internal/providers/openai/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

func TestStreamCompletionEmitsErrorForMalformedJSON(t *testing.T) {
provider := newTestProvider(t, func(w http.ResponseWriter, r *http.Request) {
writeSSE(w, `{"choices":`)
Expand Down
48 changes: 48 additions & 0 deletions internal/sessions/error_payload.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package sessions

import (
"errors"

"github.com/Twigpine/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. A longer cause is cut on a
// rune boundary and marked, and the marked result, marker included, stays
// within maxErrorCauseBytes.
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
}
// The marker counts toward the bound, so the cut leaves room for it.
return truncateUTF8(cause, maxErrorCauseBytes-len(errorCauseTruncationMarker)) + errorCauseTruncationMarker
}
Loading
Loading