diff --git a/internal/llm/policy_test.go b/internal/llm/policy_test.go index eca74d3..7f0bf17 100644 --- a/internal/llm/policy_test.go +++ b/internal/llm/policy_test.go @@ -368,3 +368,44 @@ func TestRequestShapeErrorsAreTerminalButTimingErrorsWalk(t *testing.T) { }) } } + +// TestIsTerminalExcludesRecoverableCauses: IsTerminal decides whether a failure +// KILLS THE TURN, which is a stricter question than "should the fallback walk +// stop here?" — the question terminalForFallback answers. +// +// Several causes stop the walk precisely because something else recovers them. +// Aliasing the two would make a delegated worker's context overflow kill the +// parent turn instead of compacting and retrying: the recovery would be +// replaced by a hard failure, and the bug would look like the very hang this +// classification exists to prevent. +func TestIsTerminalExcludesRecoverableCauses(t *testing.T) { + shape := fmt.Errorf("gemini stream: %w", &genai.APIError{Code: 400, Message: "malformed"}) + + for _, tc := range []struct { + name string + err error + want bool + why string + }{ + {"malformed request", shape, true, "fails identically on every model"}, + {"unauthorized", wire.ErrUnauthorized, true, "needs the user to sign in"}, + {"insufficient credit", wire.ErrInsufficientCredit, true, "needs the user to top up"}, + {"context overflow", wire.ErrContextOverflow, false, "compact-and-retry recovers it"}, + {"incomplete stream", wire.ErrStreamIncomplete, false, "the same call is retried"}, + {"rate limited", fmt.Errorf("x: %w", &genai.APIError{Code: 429}), false, "timing, not shape"}, + {"provider down", fmt.Errorf("x: %w", &genai.APIError{Code: 503}), false, "infrastructure"}, + {"transport failure", errors.New("connection reset"), false, "infrastructure"}, + {"nil", nil, false, "no failure at all"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := IsTerminal(tc.err); got != tc.want { + t.Errorf("IsTerminal = %v, want %v — %s", got, tc.want, tc.why) + } + }) + } + + // The lane-exhausted card is the user's decision, not a turn kill. + if IsTerminal(&provider.ErrLaneExhausted{}) { + t.Error("an exhausted lane raises the fallback-choice card; it must not kill the turn") + } +} diff --git a/internal/llm/recover.go b/internal/llm/recover.go index ab1520a..7d79a68 100644 --- a/internal/llm/recover.go +++ b/internal/llm/recover.go @@ -41,7 +41,30 @@ const maxModelFallbacks = 2 // thought-signature 400 was retried roughly 150 times over 12 minutes, because // nothing in the loop could tell "this tool had a bad day" from "this will // never work". -func IsTerminal(err error) bool { return err != nil && terminalForFallback(err) } +func IsTerminal(err error) bool { + if err == nil { + return false + } + // NOT an alias for terminalForFallback. That answers "should the fallback + // walk stop here?", and several of its cases stop the walk precisely BECAUSE + // something else recovers them: a context overflow compacts and retries, an + // incomplete stream retries the same call, an exhausted lane raises the + // user's fallback-choice card. Killing the turn on those would break the + // recovery instead of surfacing a cause. + for _, recoverable := range []error{ + wire.ErrContextOverflow, // compact-and-retry, inside the worker + wire.ErrStreamIncomplete, // same-call transport retry + } { + if errors.Is(err, recoverable) { + return false + } + } + var exh *provider.ErrLaneExhausted + if errors.As(err, &exh) { + return false // raises the fallback-choice card; the user decides + } + return terminalForFallback(err) +} func terminalForFallback(err error) bool { for _, sentinel := range []error{