diff --git a/cmd/entire/cli/agent/claudecode/claude_test.go b/cmd/entire/cli/agent/claudecode/claude_test.go index 875926bc5..5dc87df40 100644 --- a/cmd/entire/cli/agent/claudecode/claude_test.go +++ b/cmd/entire/cli/agent/claudecode/claude_test.go @@ -2,6 +2,7 @@ package claudecode import ( "context" + "errors" "os/exec" "testing" ) @@ -26,6 +27,7 @@ func TestProtectedDirs(t *testing.T) { } func TestGenerateText_ArrayResponse(t *testing.T) { + t.Parallel() ag := &ClaudeCodeAgent{ CommandRunner: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { response := `[{"type":"system","subtype":"init"},{"type":"assistant","message":"Working on it"},{"type":"result","result":"final generated text"}]` @@ -42,3 +44,55 @@ func TestGenerateText_ArrayResponse(t *testing.T) { t.Fatalf("GenerateText() = %q, want %q", result, "final generated text") } } + +func TestGenerateText_EnvelopeErrorReturnsClaudeError(t *testing.T) { + t.Parallel() + ag := &ClaudeCodeAgent{ + CommandRunner: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { + response := `{"type":"result","subtype":"success","is_error":true,"api_error_status":401,"result":"Auth required"}` + return exec.CommandContext(ctx, "sh", "-c", "printf '%s' '"+response+"'") + }, + } + _, err := ag.GenerateText(context.Background(), "prompt", "") + var ce *ClaudeError + if !errors.As(err, &ce) { + t.Fatalf("err = %v; want *ClaudeError", err) + } + if ce.Kind != ClaudeErrorAuth { + t.Fatalf("Kind = %v; want %v", ce.Kind, ClaudeErrorAuth) + } +} + +func TestGenerateText_CLIMissing(t *testing.T) { + t.Parallel() + ag := &ClaudeCodeAgent{ + CommandRunner: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { + return exec.CommandContext(ctx, "/nonexistent/binary/that/does/not/exist") + }, + } + _, err := ag.GenerateText(context.Background(), "prompt", "") + var ce *ClaudeError + if !errors.As(err, &ce) { + t.Fatalf("err = %v; want *ClaudeError", err) + } + if ce.Kind != ClaudeErrorCLIMissing { + t.Fatalf("Kind = %v; want %v", ce.Kind, ClaudeErrorCLIMissing) + } +} + +func TestGenerateText_StderrAuthFallback(t *testing.T) { + t.Parallel() + ag := &ClaudeCodeAgent{ + CommandRunner: func(ctx context.Context, _ string, _ ...string) *exec.Cmd { + return exec.CommandContext(ctx, "sh", "-c", "printf 'Invalid API key' 1>&2; exit 2") + }, + } + _, err := ag.GenerateText(context.Background(), "prompt", "") + var ce *ClaudeError + if !errors.As(err, &ce) { + t.Fatalf("err = %v; want *ClaudeError", err) + } + if ce.Kind != ClaudeErrorAuth { + t.Fatalf("Kind = %v; want %v", ce.Kind, ClaudeErrorAuth) + } +} diff --git a/cmd/entire/cli/agent/claudecode/error.go b/cmd/entire/cli/agent/claudecode/error.go new file mode 100644 index 000000000..a4187abdf --- /dev/null +++ b/cmd/entire/cli/agent/claudecode/error.go @@ -0,0 +1,116 @@ +package claudecode + +import ( + "fmt" + "strings" +) + +// ClaudeErrorKind classifies a typed Claude CLI error so callers can +// produce actionable user-facing messages without parsing strings. +type ClaudeErrorKind string + +const ( + // ClaudeErrorAuth indicates an authentication or authorization failure + // (HTTP 401/403 in the CLI envelope, or recognized stderr substring). + ClaudeErrorAuth ClaudeErrorKind = "auth" + // ClaudeErrorRateLimit indicates the request was rejected for rate-limit + // or quota reasons (HTTP 429). + ClaudeErrorRateLimit ClaudeErrorKind = "rate_limit" + // ClaudeErrorConfig indicates a client-side request error other than + // auth or rate-limit (e.g., HTTP 4xx for invalid model or malformed args). + ClaudeErrorConfig ClaudeErrorKind = "config" + // ClaudeErrorCLIMissing indicates the claude binary was not found on PATH. + ClaudeErrorCLIMissing ClaudeErrorKind = "cli_missing" + // ClaudeErrorUnknown is the catch-all for failures we cannot classify. + ClaudeErrorUnknown ClaudeErrorKind = "unknown" +) + +// ClaudeError is a typed error returned by ClaudeCodeAgent's text generation +// methods. APIStatus and ExitCode use zero to mean "not applicable." +type ClaudeError struct { + Kind ClaudeErrorKind + Message string // user-safe text extracted from the CLI envelope or stderr + APIStatus int + ExitCode int + Cause error +} + +func (e *ClaudeError) Error() string { + if e.Message == "" { + if e.ExitCode != 0 { + return fmt.Sprintf("claude CLI error (kind=%s, exit=%d)", e.Kind, e.ExitCode) + } + return fmt.Sprintf("claude CLI error (kind=%s)", e.Kind) + } + return fmt.Sprintf("claude CLI error (kind=%s): %s", e.Kind, e.Message) +} + +func (e *ClaudeError) Unwrap() error { return e.Cause } + +const stderrMessageMaxLen = 500 + +// authStderrPhrases is intentionally small. The primary auth-detection path +// is the structured envelope (classifyEnvelopeError); these phrases are a +// best-effort fallback for crashes that exit non-zero before the envelope +// is produced. +var authStderrPhrases = []string{ + "invalid api key", + "not logged in", +} + +// classifyEnvelopeError converts a Claude CLI is_error:true envelope into a +// typed ClaudeError. The result text is treated as user-safe (the CLI +// produces it for human consumption). +func classifyEnvelopeError(resultText string, apiStatus *int, exitCode int) *ClaudeError { + e := &ClaudeError{ + Message: resultText, + ExitCode: exitCode, + } + if apiStatus != nil { + e.APIStatus = *apiStatus + } + switch { + case e.APIStatus == 401, e.APIStatus == 403: + e.Kind = ClaudeErrorAuth + case e.APIStatus == 429: + e.Kind = ClaudeErrorRateLimit + case e.APIStatus >= 400 && e.APIStatus < 500: + e.Kind = ClaudeErrorConfig + case e.APIStatus == 0 && hasAuthPhrase(resultText): + // No structured status (older CLI builds / internal errors) — fall + // back to the same phrase heuristic the stderr path uses so users + // still get auth-specific guidance. + e.Kind = ClaudeErrorAuth + default: + e.Kind = ClaudeErrorUnknown + } + return e +} + +// classifyStderrError is a fallback classifier used when the subprocess exited +// non-zero without producing a parseable envelope. It only attempts to +// recognize a small, stable set of auth phrases; everything else becomes +// ClaudeErrorUnknown with the (truncated) stderr as the message. +func classifyStderrError(stderr string, exitCode int) *ClaudeError { + msg := strings.TrimSpace(stderr) + if len(msg) > stderrMessageMaxLen { + msg = msg[:stderrMessageMaxLen] + } + e := &ClaudeError{Message: msg, ExitCode: exitCode} + if hasAuthPhrase(msg) { + e.Kind = ClaudeErrorAuth + return e + } + e.Kind = ClaudeErrorUnknown + return e +} + +func hasAuthPhrase(s string) bool { + lower := strings.ToLower(s) + for _, phrase := range authStderrPhrases { + if strings.Contains(lower, phrase) { + return true + } + } + return false +} diff --git a/cmd/entire/cli/agent/claudecode/error_test.go b/cmd/entire/cli/agent/claudecode/error_test.go new file mode 100644 index 000000000..37d4c9386 --- /dev/null +++ b/cmd/entire/cli/agent/claudecode/error_test.go @@ -0,0 +1,153 @@ +package claudecode + +import ( + "errors" + "fmt" + "strings" + "testing" +) + +func TestClaudeError_ErrorIncludesKindAndMessage(t *testing.T) { + t.Parallel() + e := &ClaudeError{Kind: ClaudeErrorAuth, Message: "Invalid API key"} + s := e.Error() + if !strings.Contains(s, "auth") { + t.Errorf("Error() = %q; want to contain kind 'auth'", s) + } + if !strings.Contains(s, "Invalid API key") { + t.Errorf("Error() = %q; want to contain message", s) + } +} + +func TestClaudeError_UnwrapReturnsCause(t *testing.T) { + t.Parallel() + cause := errors.New("underlying") + e := &ClaudeError{Kind: ClaudeErrorUnknown, Cause: cause} + if got := errors.Unwrap(e); !errors.Is(got, cause) { + t.Errorf("Unwrap() = %v; want %v", got, cause) + } +} + +func TestClaudeError_UnwrapNilCause(t *testing.T) { + t.Parallel() + e := &ClaudeError{Kind: ClaudeErrorAuth} + if got := errors.Unwrap(e); got != nil { + t.Errorf("Unwrap() = %v; want nil", got) + } +} + +func TestClaudeError_ErrorEmptyMessageFallback(t *testing.T) { + t.Parallel() + e := &ClaudeError{Kind: ClaudeErrorRateLimit} + s := e.Error() + want := "claude CLI error (kind=rate_limit)" + if s != want { + t.Errorf("Error() = %q; want %q", s, want) + } +} + +func TestClaudeError_ErrorEmptyMessageIncludesExitCode(t *testing.T) { + t.Parallel() + e := &ClaudeError{Kind: ClaudeErrorUnknown, ExitCode: 137} + s := e.Error() + want := "claude CLI error (kind=unknown, exit=137)" + if s != want { + t.Errorf("Error() = %q; want %q", s, want) + } +} + +func TestClaudeError_ErrorsAsIntegration(t *testing.T) { + t.Parallel() + cause := errors.New("timeout") + wrapped := fmt.Errorf("operation failed: %w", &ClaudeError{ + Kind: ClaudeErrorCLIMissing, + Message: "claude not found", + Cause: cause, + }) + + var ce *ClaudeError + if !errors.As(wrapped, &ce) { + t.Fatal("errors.As did not find *ClaudeError in wrapped chain") + } + if ce.Kind != ClaudeErrorCLIMissing { + t.Errorf("Kind = %q; want %q", ce.Kind, ClaudeErrorCLIMissing) + } + if !errors.Is(ce, cause) { + t.Error("errors.Is did not find cause through ClaudeError.Unwrap()") + } +} + +func TestClassifyEnvelopeError(t *testing.T) { + t.Parallel() + intPtr := func(n int) *int { return &n } + tests := []struct { + name string + result string + status *int + exitCode int + wantKind ClaudeErrorKind + wantAPI int + wantExit int + }{ + {"Auth401", "Authentication required", intPtr(401), 0, ClaudeErrorAuth, 401, 0}, + {"Auth403", "forbidden", intPtr(403), 0, ClaudeErrorAuth, 403, 0}, + {"RateLimit429", "Too many requests", intPtr(429), 0, ClaudeErrorRateLimit, 429, 0}, + {"Config404", "model not found", intPtr(404), 0, ClaudeErrorConfig, 404, 0}, + {"Config400", "invalid_request_error", intPtr(400), 0, ClaudeErrorConfig, 400, 0}, + {"UnknownNoStatus", "something blew up", nil, 0, ClaudeErrorUnknown, 0, 0}, + {"Unknown5xx", "upstream error", intPtr(503), 0, ClaudeErrorUnknown, 503, 0}, + {"ExitCodePropagated", "internal error", intPtr(500), 2, ClaudeErrorUnknown, 500, 2}, + {"AuthFromResultWhenStatusNil", "Invalid API key provided", nil, 0, ClaudeErrorAuth, 0, 0}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := classifyEnvelopeError(tc.result, tc.status, tc.exitCode) + if got.Kind != tc.wantKind { + t.Errorf("Kind = %v; want %v", got.Kind, tc.wantKind) + } + if got.Message != tc.result { + t.Errorf("Message = %q; want %q", got.Message, tc.result) + } + if got.APIStatus != tc.wantAPI { + t.Errorf("APIStatus = %d; want %d", got.APIStatus, tc.wantAPI) + } + if got.ExitCode != tc.wantExit { + t.Errorf("ExitCode = %d; want %d", got.ExitCode, tc.wantExit) + } + }) + } +} + +func TestClassifyStderrError(t *testing.T) { + t.Parallel() + tests := []struct { + name string + stderr string + exitCode int + wantKind ClaudeErrorKind + wantExit int + maxMsgLen int // 0 means no length check + }{ + {"AuthFromInvalidKey", "error: Invalid API key", 1, ClaudeErrorAuth, 1, 0}, + {"AuthFromNotLoggedIn", "Please run claude login first; you are not logged in", 1, ClaudeErrorAuth, 1, 0}, + {"AuthCaseInsensitive", "INVALID API KEY", 1, ClaudeErrorAuth, 1, 0}, + {"UnknownPreservesMessage", "segfault", 134, ClaudeErrorUnknown, 134, 0}, + {"TruncatesLongStderr", strings.Repeat("x", 800), 1, ClaudeErrorUnknown, 1, 500}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := classifyStderrError(tc.stderr, tc.exitCode) + if got.Kind != tc.wantKind { + t.Errorf("Kind = %v; want %v", got.Kind, tc.wantKind) + } + if got.ExitCode != tc.wantExit { + t.Errorf("ExitCode = %d; want %d", got.ExitCode, tc.wantExit) + } + if tc.maxMsgLen > 0 && len(got.Message) > tc.maxMsgLen { + t.Errorf("len(Message) = %d; want <= %d", len(got.Message), tc.maxMsgLen) + } + }) + } +} diff --git a/cmd/entire/cli/agent/claudecode/generate.go b/cmd/entire/cli/agent/claudecode/generate.go index 973938e54..647a83b6e 100644 --- a/cmd/entire/cli/agent/claudecode/generate.go +++ b/cmd/entire/cli/agent/claudecode/generate.go @@ -1,9 +1,13 @@ package claudecode import ( + "bytes" "context" + "errors" "fmt" + "os" "os/exec" + "strings" "github.com/entireio/cli/cmd/entire/cli/agent" ) @@ -12,6 +16,14 @@ import ( // Implements the agent.TextGenerator interface. // The model parameter hints which model to use (e.g., "haiku", "sonnet"). // If empty, defaults to "haiku" for fast, cheap generation. +// +// Unlike most agents, this implementation runs the subprocess directly rather +// than through agent.RunIsolatedTextGeneratorCLI. The shared helper collapses +// stderr + exit code into a single formatted error string, but Claude CLI +// returns operational errors (auth, rate limit, invalid model) as exit 0 +// with is_error:true in the JSON envelope on stdout — so we need structured +// access to stdout, stderr and the exit code to produce the typed ClaudeError +// values that formatCheckpointSummaryError maps to actionable messages. func (c *ClaudeCodeAgent) GenerateText(ctx context.Context, prompt string, model string) (string, error) { claudePath := "claude" if model == "" { @@ -23,19 +35,79 @@ func (c *ClaudeCodeAgent) GenerateText(ctx context.Context, prompt string, model commandRunner = exec.CommandContext } - args := []string{ + cmd := commandRunner(ctx, claudePath, "--print", "--output-format", "json", - "--model", model, "--setting-sources", "", - } - stdoutText, err := agent.RunIsolatedTextGeneratorCLI(ctx, commandRunner, claudePath, "claude", args, prompt) - if err != nil { - return "", fmt.Errorf("claude text generation failed: %w", err) + "--model", model, "--setting-sources", "") + + // Isolate from the user's git repo to prevent recursive hook triggers + // and index pollution (matches agent.RunIsolatedTextGeneratorCLI behavior). + cmd.Dir = os.TempDir() + cmd.Env = agent.StripGitEnv(os.Environ()) + cmd.Stdin = strings.NewReader(prompt) + + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + if err := cmd.Run(); err != nil { + // Prefer a structured Claude error over bare ctx sentinels: if the CLI + // already emitted an is_error envelope on stdout, surface that even when + // the context happens to be done. Otherwise the user loses actionable + // auth/rate-limit/config diagnostics whenever ctx and the subprocess + // both fail at roughly the same time. + if _, env, parseErr := parseGenerateTextResponse(stdout.Bytes()); parseErr == nil && env != nil && env.IsError { + result := "" + if env.Result != nil { + result = *env.Result + } + exitCode := 0 + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + exitCode = exitErr.ExitCode() + } + return "", classifyEnvelopeError(result, env.APIErrorStatus, exitCode) + } + // No structured signal on stdout — ctx cancellation is next most + // informative, since the rest is a guess. + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return "", context.DeadlineExceeded + } + if errors.Is(ctx.Err(), context.Canceled) { + return "", context.Canceled + } + if isExecNotFound(err) { + return "", &ClaudeError{Kind: ClaudeErrorCLIMissing, Cause: err} + } + exitCode := -1 + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + exitCode = exitErr.ExitCode() + } + return "", classifyStderrError(stderr.String(), exitCode) } - result, err := parseGenerateTextResponse([]byte(stdoutText)) + // Exit 0: parse the response and check for is_error (Claude CLI returns + // most operational errors as exit 0 with is_error:true in the envelope). + result, env, err := parseGenerateTextResponse(stdout.Bytes()) if err != nil { - return "", fmt.Errorf("failed to parse claude CLI response: %w", err) + return "", &ClaudeError{Kind: ClaudeErrorUnknown, Message: fmt.Sprintf("failed to parse claude CLI response: %v", err), Cause: err} + } + if env != nil && env.IsError { + return "", classifyEnvelopeError(result, env.APIErrorStatus, 0) } return result, nil } + +// isExecNotFound returns true only when err indicates the binary was not +// found on PATH or at the given absolute path. It intentionally excludes +// other *exec.Error causes (permission denied, invalid executable format), +// which should surface as a generic failure so operators aren't misdirected +// to a reinstall when the real problem is a broken/inaccessible binary. +func isExecNotFound(err error) bool { + var execErr *exec.Error + if errors.As(err, &execErr) && errors.Is(execErr.Err, exec.ErrNotFound) { + return true + } + return errors.Is(err, exec.ErrNotFound) || errors.Is(err, os.ErrNotExist) +} diff --git a/cmd/entire/cli/agent/claudecode/response.go b/cmd/entire/cli/agent/claudecode/response.go index a1e10ee1d..f54334fb7 100644 --- a/cmd/entire/cli/agent/claudecode/response.go +++ b/cmd/entire/cli/agent/claudecode/response.go @@ -7,28 +7,48 @@ import ( ) type responseEnvelope struct { - Type string `json:"type"` - Result *string `json:"result"` + Type string `json:"type"` + Subtype string `json:"subtype"` + IsError bool `json:"is_error"` + APIErrorStatus *int `json:"api_error_status"` + Result *string `json:"result"` } -// parseGenerateTextResponse extracts the raw text payload from Claude CLI JSON output. -// Claude may return either a legacy single object or a newer array of events. -func parseGenerateTextResponse(stdout []byte) (string, error) { +// parseGenerateTextResponse extracts the raw text payload and envelope metadata +// from Claude CLI JSON output. Claude may return either a single object or an array of events. +// The returned envelope allows callers to check IsError and APIErrorStatus. +func parseGenerateTextResponse(stdout []byte) (string, *responseEnvelope, error) { var response responseEnvelope - if err := json.Unmarshal(stdout, &response); err == nil && response.Result != nil { - return *response.Result, nil + if err := json.Unmarshal(stdout, &response); err == nil { + if response.Result != nil { + return *response.Result, &response, nil + } + // is_error:true with null result is a structured CLI failure; callers + // need the envelope (IsError, APIErrorStatus) for classification. + if response.IsError { + return "", &response, nil + } } var responses []responseEnvelope if err := json.Unmarshal(stdout, &responses); err != nil { - return "", fmt.Errorf("unsupported Claude CLI JSON response: %w", err) + return "", nil, fmt.Errorf("unsupported Claude CLI JSON response: %w", err) } for i := len(responses) - 1; i >= 0; i-- { - if responses[i].Type == "result" && responses[i].Result != nil { - return *responses[i].Result, nil + if responses[i].Type != "result" { + continue + } + if responses[i].Result != nil { + return *responses[i].Result, &responses[i], nil + } + // Mirror the object-path behavior: is_error:true with null result is + // a structured failure whose envelope (IsError, APIErrorStatus) must + // reach classifyEnvelopeError. + if responses[i].IsError { + return "", &responses[i], nil } } - return "", errors.New("unsupported Claude CLI JSON response: missing result item") + return "", nil, errors.New("unsupported Claude CLI JSON response: missing result item") } diff --git a/cmd/entire/cli/agent/claudecode/response_test.go b/cmd/entire/cli/agent/claudecode/response_test.go index 776e34f00..c65dcee45 100644 --- a/cmd/entire/cli/agent/claudecode/response_test.go +++ b/cmd/entire/cli/agent/claudecode/response_test.go @@ -5,6 +5,66 @@ import ( "testing" ) +func TestParseGenerateTextResponse_IsErrorEnvelope(t *testing.T) { + t.Parallel() + stdout := `{"type":"result","subtype":"success","is_error":true,"api_error_status":404,"result":"model not found"}` + result, env, err := parseGenerateTextResponse([]byte(stdout)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result != "model not found" { + t.Errorf("result = %q; want %q", result, "model not found") + } + if env == nil { + t.Fatal("envelope = nil; want non-nil") + } + if !env.IsError { + t.Error("IsError = false; want true") + } + if env.APIErrorStatus == nil || *env.APIErrorStatus != 404 { + t.Errorf("APIErrorStatus = %v; want *404", env.APIErrorStatus) + } +} + +func TestParseGenerateTextResponse_IsErrorEnvelopeNullResult(t *testing.T) { + t.Parallel() + // Claude CLI can emit is_error:true with result:null on internal failures. + // The envelope's IsError / APIErrorStatus signal must survive even though + // Result is absent — otherwise classification falls through to a generic + // "missing result item" parse error and the upstream typed-error path + // loses the api_error_status. + // + // Both wire shapes (single object and event array) must behave the same. + tests := []struct { + name string + stdout string + }{ + {"single object", `{"type":"result","subtype":"error_during_execution","is_error":true,"api_error_status":500,"result":null}`}, + {"event array", `[{"type":"system"},{"type":"result","subtype":"error_during_execution","is_error":true,"api_error_status":500,"result":null}]`}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + result, env, err := parseGenerateTextResponse([]byte(tc.stdout)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if result != "" { + t.Errorf("result = %q; want empty string", result) + } + if env == nil { + t.Fatal("envelope = nil; want non-nil") + } + if !env.IsError { + t.Error("IsError = false; want true") + } + if env.APIErrorStatus == nil || *env.APIErrorStatus != 500 { + t.Errorf("APIErrorStatus = %v; want *500", env.APIErrorStatus) + } + }) + } +} + func TestParseGenerateTextResponse(t *testing.T) { t.Parallel() @@ -50,7 +110,7 @@ func TestParseGenerateTextResponse(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - got, err := parseGenerateTextResponse([]byte(tt.stdout)) + got, _, err := parseGenerateTextResponse([]byte(tt.stdout)) if tt.wantErr != "" { if err == nil { t.Fatal("expected error") diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index eaef81571..d2d122af5 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -14,6 +14,7 @@ import ( "time" "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/claudecode" "github.com/entireio/cli/cmd/entire/cli/agent/geminicli" "github.com/entireio/cli/cmd/entire/cli/agent/opencode" "github.com/entireio/cli/cmd/entire/cli/agent/types" @@ -519,9 +520,9 @@ func generateCheckpointSummary(ctx context.Context, w, errW io.Writer, v1Store * fmt.Fprintln(errW, "Generating checkpoint summary...") } - summary, err := generateCheckpointAISummary(ctx, scopedTranscript, cpSummary.FilesTouched, content.Metadata.Agent, provider.Generator) + summary, appliedDeadline, err := generateCheckpointAISummary(ctx, scopedTranscript, cpSummary.FilesTouched, content.Metadata.Agent, provider.Generator) if err != nil { - return fmt.Errorf("failed to generate summary: %w", err) + return formatCheckpointSummaryError(err, appliedDeadline) } // Persist to both stores; at least one must succeed. @@ -555,7 +556,13 @@ func generateCheckpointSummary(ctx context.Context, w, errW io.Writer, v1Store * return nil } -func generateCheckpointAISummary(ctx context.Context, scopedTranscript []byte, filesTouched []string, agentType types.AgentType, generator summarize.Generator) (*checkpoint.Summary, error) { +// generateCheckpointAISummary returns the generated summary, the effective +// deadline applied to the underlying call (which may be shorter than +// checkpointSummaryTimeout if the parent context had an earlier deadline), +// and any error. The effective deadline is returned so the caller can render +// the true timeout value in user-facing error messages instead of always +// showing the package default. +func generateCheckpointAISummary(ctx context.Context, scopedTranscript []byte, filesTouched []string, agentType types.AgentType, generator summarize.Generator) (*checkpoint.Summary, time.Duration, error) { timeoutCtx, cancel := context.WithTimeout(ctx, checkpointSummaryTimeout) timeoutDuration := checkpointSummaryTimeout if deadline, ok := timeoutCtx.Deadline(); ok { @@ -566,16 +573,94 @@ func generateCheckpointAISummary(ctx context.Context, scopedTranscript []byte, f // scopedTranscript is read from checkpoint storage, which redacts on write. summary, err := generateTranscriptSummary(timeoutCtx, redact.AlreadyRedacted(scopedTranscript), filesTouched, agentType, generator) if err != nil { - if errors.Is(err, context.Canceled) || errors.Is(timeoutCtx.Err(), context.Canceled) { - return nil, fmt.Errorf("summary generation canceled: %w", context.Canceled) + // Only classify as ctx cancel/deadline when the error chain actually + // contains the sentinel. Relying on timeoutCtx.Err() here loses typed + // errors (e.g. *ClaudeError) when the subprocess returned a real + // structured failure while timeoutCtx.Err() is non-nil for any reason + // (parent cancelled, deadline already elapsed, etc.). + if errors.Is(err, context.Canceled) { + return nil, timeoutDuration, fmt.Errorf("summary generation canceled: %w", err) } - if errors.Is(err, context.DeadlineExceeded) || errors.Is(timeoutCtx.Err(), context.DeadlineExceeded) { - return nil, fmt.Errorf("summary generation timed out after %s: %w", formatSummaryTimeout(timeoutDuration), context.DeadlineExceeded) + if errors.Is(err, context.DeadlineExceeded) { + return nil, timeoutDuration, fmt.Errorf("summary generation timed out after %s: %w", formatSummaryTimeout(timeoutDuration), err) } - return nil, err + return nil, timeoutDuration, err } - return summary, nil + return summary, timeoutDuration, nil +} + +// formatCheckpointSummaryError maps typed Claude CLI errors and context +// sentinels to user-facing messages. +func formatCheckpointSummaryError(err error, deadline time.Duration) error { + var claudeErr *claudecode.ClaudeError + switch { + case errors.As(err, &claudeErr): + switch claudeErr.Kind { //nolint:exhaustive // ClaudeErrorUnknown handled by default + case claudecode.ClaudeErrorAuth: + return fmt.Errorf("Claude authentication failed%s\nRun `claude login` and retry", formatMessageSuffix(claudeErr.Message)) //nolint:staticcheck // ST1005: capitalized because Claude is a proper noun + case claudecode.ClaudeErrorRateLimit: + return fmt.Errorf("Claude rejected the summary request due to rate limits or quota%s\nWait and retry", formatMessageSuffix(claudeErr.Message)) //nolint:staticcheck // ST1005 + case claudecode.ClaudeErrorConfig: + return fmt.Errorf("Claude rejected the summary request%s\nCheck your Claude CLI config and selected model", formatMessageSuffix(claudeErr.Message)) //nolint:staticcheck // ST1005 + case claudecode.ClaudeErrorCLIMissing: + return errors.New("Claude CLI is not installed or not on PATH") //nolint:staticcheck // ST1005 + default: + return fmt.Errorf("Claude failed to generate the summary%s", formatClaudeErrorSuffix(claudeErr)) //nolint:staticcheck // ST1005 + } + case errors.Is(err, context.DeadlineExceeded): + // Deliberately provider-neutral: explain --generate supports multiple + // summary providers (claude-code, codex, gemini, ...), so hardcoding + // "Claude" / "sonnet" / "Anthropic" here would misdirect users who + // selected a different provider in .entire/settings.json. + return fmt.Errorf( + "summary generation did not return within the %s safety deadline. This usually means one of:\n"+ + " - the selected model is taking longer than expected on a large transcript\n"+ + " - the summary provider's CLI cannot reach its API (network, VPN, firewall)\n"+ + " Try: run the provider CLI directly to confirm it works\n"+ + " - the provider's API is degraded", + formatSummaryTimeout(deadline)) + case errors.Is(err, context.Canceled): + return errors.New("summary generation canceled") + default: + return fmt.Errorf("failed to generate summary: %w", err) + } +} + +// formatMessageSuffix formats ": " when msg is non-empty and "" otherwise. +// Used by the Auth / RateLimit / Config branches of formatCheckpointSummaryError +// to avoid rendering a bare colon when ClaudeError.Message is empty (reachable +// when the CLI envelope is is_error:true with result:null but a real status). +func formatMessageSuffix(msg string) string { + if msg == "" { + return "" + } + return ": " + msg +} + +// formatClaudeErrorSuffix builds a diagnostic suffix for user-facing output +// when we fall through to the default "failed to generate the summary" path. +// Prefers the envelope Message, falls back to HTTP status, then exit code, +// so the user never sees a bare "Claude failed to generate the summary:" +// with nothing after the colon (which happens when Claude returns +// is_error:true with result:null, or when the subprocess crashes with no +// stderr output). ExitCode < 0 means the subprocess did not produce a real +// exit code (e.g. launch failure) — render that as "abnormal termination" +// rather than the misleading "exited with code -1". +func formatClaudeErrorSuffix(e *claudecode.ClaudeError) string { + if e.Message != "" { + return ": " + e.Message + } + switch { + case e.APIStatus != 0: + return fmt.Sprintf(" (Anthropic API returned HTTP %d)", e.APIStatus) + case e.ExitCode > 0: + return fmt.Sprintf(" (claude CLI exited with code %d)", e.ExitCode) + case e.ExitCode < 0: + return " (claude CLI terminated abnormally — no exit code captured)" + default: + return " (no diagnostic detail available from Claude CLI)" + } } func formatSummaryTimeout(d time.Duration) string { diff --git a/cmd/entire/cli/explain_test.go b/cmd/entire/cli/explain_test.go index cb94c3133..1905efc39 100644 --- a/cmd/entire/cli/explain_test.go +++ b/cmd/entire/cli/explain_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -12,6 +13,7 @@ import ( "time" "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/claudecode" "github.com/entireio/cli/cmd/entire/cli/agent/types" "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" @@ -69,6 +71,137 @@ func TestExplainCmd_SearchAllFlag(t *testing.T) { } } +func TestFormatCheckpointSummaryError_Auth(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(&claudecode.ClaudeError{Kind: claudecode.ClaudeErrorAuth, Message: "Invalid API key"}, 0) + msg := err.Error() + if !strings.Contains(strings.ToLower(msg), "authentication failed") { + t.Errorf("missing 'authentication failed' in %q", msg) + } + if !strings.Contains(msg, "Invalid API key") { + t.Errorf("missing envelope message in %q", msg) + } +} + +func TestFormatCheckpointSummaryError_RateLimit(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(&claudecode.ClaudeError{Kind: claudecode.ClaudeErrorRateLimit, Message: "429"}, 0) + if !strings.Contains(err.Error(), "rate limit") { + t.Errorf("missing rate-limit phrasing: %q", err) + } +} + +func TestFormatCheckpointSummaryError_Config(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(&claudecode.ClaudeError{Kind: claudecode.ClaudeErrorConfig, Message: "model not found"}, 0) + if !strings.Contains(err.Error(), "model not found") { + t.Errorf("envelope message not surfaced: %q", err) + } +} + +func TestFormatCheckpointSummaryError_CLIMissing(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(&claudecode.ClaudeError{Kind: claudecode.ClaudeErrorCLIMissing}, 0) + if !strings.Contains(err.Error(), "not installed") { + t.Errorf("missing cli-missing phrasing: %q", err) + } +} + +// TestFormatCheckpointSummaryError_TypedBranchesHandleEmptyMessage guards against +// the null-result-envelope regression: Claude can emit is_error:true with a real +// HTTP status (401/429/4xx) but result:null, producing a ClaudeError with Message="". +// The Auth/RateLimit/Config branches must not render a bare colon in that case. +func TestFormatCheckpointSummaryError_TypedBranchesHandleEmptyMessage(t *testing.T) { + t.Parallel() + kinds := []claudecode.ClaudeErrorKind{ + claudecode.ClaudeErrorAuth, + claudecode.ClaudeErrorRateLimit, + claudecode.ClaudeErrorConfig, + } + for _, kind := range kinds { + t.Run(string(kind), func(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(&claudecode.ClaudeError{Kind: kind}, 0) + msg := err.Error() + // Must not end any line with a bare colon (the classic regression + // of rendering "...: " with nothing after it). + for _, line := range strings.Split(msg, "\n") { + if strings.HasSuffix(strings.TrimSpace(line), ":") { + t.Errorf("line ends with bare colon: %q (full: %q)", line, msg) + } + } + }) + } +} + +func TestFormatCheckpointSummaryError_DeadlineExceeded(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(fmt.Errorf("wrapped: %w", context.DeadlineExceeded), 5*time.Minute) + msg := err.Error() + for _, want := range []string{"5m", "safety deadline"} { + if !strings.Contains(msg, want) { + t.Errorf("missing %q in %q", want, msg) + } + } + // Negative guards against regressions: + // - summary_timeout_seconds advice was removed because the setting is + // not wired yet (follow-up PR). Reintroducing it would re-mislead users. + // - Hardcoded "Claude" / "sonnet" / "Anthropic" would misdirect users of + // alternate summary providers (codex, gemini). + for _, unwanted := range []string{"summary_timeout_seconds", "Claude", "sonnet", "Anthropic", "anthropic.com"} { + if strings.Contains(msg, unwanted) { + t.Errorf("unexpected %q in provider-neutral timeout message: %q", unwanted, msg) + } + } +} + +func TestFormatCheckpointSummaryError_Canceled(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(fmt.Errorf("wrapped: %w", context.Canceled), 0) + if !strings.Contains(err.Error(), "canceled") { + t.Errorf("missing canceled: %q", err) + } +} + +func TestFormatCheckpointSummaryError_Passthrough(t *testing.T) { + t.Parallel() + err := formatCheckpointSummaryError(errors.New("something else"), 0) + if !strings.Contains(err.Error(), "something else") { + t.Errorf("original error not preserved: %q", err) + } +} + +// TestFormatCheckpointSummaryError_Unknown covers the three branches of the +// default-case suffix builder. Guards against users seeing +// "Claude failed to generate the summary:" with nothing after the colon +// (the null-result and no-stderr-OOM scenarios). +func TestFormatCheckpointSummaryError_Unknown(t *testing.T) { + t.Parallel() + tests := []struct { + name string + err *claudecode.ClaudeError + want string // substring that must appear in the rendered message + }{ + {"APIStatus when Message empty", &claudecode.ClaudeError{Kind: claudecode.ClaudeErrorUnknown, APIStatus: 500}, "500"}, + {"ExitCode when Message empty", &claudecode.ClaudeError{Kind: claudecode.ClaudeErrorUnknown, ExitCode: 137}, "137"}, + {"Negative ExitCode renders as abnormal, not -1", &claudecode.ClaudeError{Kind: claudecode.ClaudeErrorUnknown, ExitCode: -1}, "abnormal"}, + {"All-zero fields render a diagnostic sentinel, not empty", &claudecode.ClaudeError{Kind: claudecode.ClaudeErrorUnknown}, "no diagnostic detail"}, + {"Message takes precedence", &claudecode.ClaudeError{Kind: claudecode.ClaudeErrorUnknown, Message: "something weird"}, "something weird"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + msg := formatCheckpointSummaryError(tc.err, 0).Error() + if strings.HasSuffix(strings.TrimSpace(msg), ":") { + t.Errorf("user-facing message ends with bare colon: %q", msg) + } + if !strings.Contains(msg, tc.want) { + t.Errorf("missing %q in %q", tc.want, msg) + } + }) + } +} + func TestExplainCmd_RejectsPositionalArgs(t *testing.T) { tests := []struct { name string @@ -130,7 +263,7 @@ func TestGenerateCheckpointAISummary_AddsDefaultTimeoutWithoutParentDeadline(t * } start := time.Now() - summary, err := generateCheckpointAISummary(context.Background(), []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + summary, _, err := generateCheckpointAISummary(context.Background(), []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) if err != nil { t.Fatalf("generateCheckpointAISummary() error = %v", err) } @@ -172,7 +305,7 @@ func TestGenerateCheckpointAISummary_UsesParentDeadlineAndWrapsSentinel(t *testi return nil, ctx.Err() } - _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + _, appliedDeadline, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) if err == nil { t.Fatal("expected timeout error") } @@ -182,6 +315,13 @@ func TestGenerateCheckpointAISummary_UsesParentDeadlineAndWrapsSentinel(t *testi if gotDeadline.IsZero() { t.Fatal("expected deadline to be captured") } + // The applied deadline must reflect the shorter parent-ctx deadline, + // not the package-default checkpointSummaryTimeout. Otherwise + // formatCheckpointSummaryError would report the wrong timeout to users. + if appliedDeadline >= checkpointSummaryTimeout { + t.Fatalf("appliedDeadline = %s; want shorter than %s (parent had tighter deadline)", + appliedDeadline, checkpointSummaryTimeout) + } if delta := gotDeadline.Sub(parentDeadline); delta < -5*time.Millisecond || delta > 5*time.Millisecond { t.Fatalf("deadline delta = %s, want near 0", delta) } @@ -190,6 +330,47 @@ func TestGenerateCheckpointAISummary_UsesParentDeadlineAndWrapsSentinel(t *testi } } +// TestGenerateCheckpointAISummary_PreservesClaudeErrorWhenCtxIsDone guards +// against the race where the underlying summarizer returns a typed +// *ClaudeError AND the context happens to be done. Prior code checked +// timeoutCtx.Err() and unconditionally wrapped with %w context.DeadlineExceeded, +// which discarded the typed error and routed the user to the wrong +// "safety deadline" guidance instead of the auth/rate-limit message. +func TestGenerateCheckpointAISummary_PreservesClaudeErrorWhenCtxIsDone(t *testing.T) { + tmpTimeout := checkpointSummaryTimeout + tmpGenerator := generateTranscriptSummary + t.Cleanup(func() { + checkpointSummaryTimeout = tmpTimeout + generateTranscriptSummary = tmpGenerator + }) + + checkpointSummaryTimeout = 30 * time.Second + + // Cancel the parent before we even call — ctx.Err() will be non-nil. + parentCtx, cancel := context.WithCancel(context.Background()) + cancel() + + claudeErr := &claudecode.ClaudeError{Kind: claudecode.ClaudeErrorAuth, Message: "Invalid API key"} + generateTranscriptSummary = func( + context.Context, + redact.RedactedBytes, + []string, + types.AgentType, + summarize.Generator, + ) (*checkpoint.Summary, error) { + return nil, claudeErr + } + + _, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + var ce *claudecode.ClaudeError + if !errors.As(err, &ce) { + t.Fatalf("errors.As did not recover *ClaudeError; got %v", err) + } + if ce.Kind != claudecode.ClaudeErrorAuth { + t.Errorf("Kind = %v; want auth", ce.Kind) + } +} + func TestGenerateCheckpointAISummary_ClampsLongParentDeadlineToDefaultTimeout(t *testing.T) { tmpTimeout := checkpointSummaryTimeout tmpGenerator := generateTranscriptSummary @@ -220,7 +401,7 @@ func TestGenerateCheckpointAISummary_ClampsLongParentDeadlineToDefaultTimeout(t } start := time.Now() - summary, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + summary, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) if err != nil { t.Fatalf("generateCheckpointAISummary() error = %v", err) } @@ -257,7 +438,7 @@ func TestGenerateCheckpointAISummary_UsesCancellationSentinel(t *testing.T) { return nil, ctx.Err() } - _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) + _, _, err := generateCheckpointAISummary(parentCtx, []byte("transcript"), nil, agent.AgentTypeClaudeCode, nil) if err == nil { t.Fatal("expected cancellation error") } diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index 995d00b1b..d3bf99629 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -11,6 +11,7 @@ import ( "os" "path/filepath" "strings" + "time" "github.com/entireio/cli/cmd/entire/cli/jsonutil" "github.com/entireio/cli/cmd/entire/cli/paths" @@ -80,6 +81,13 @@ type EntireSettings struct { // should include a vercel.json that disables deployments for Entire branches. Vercel bool `json:"vercel,omitempty"` + // SummaryTimeoutSeconds is an optional hard deadline (in seconds) for + // `entire explain --generate` summary generation. Zero or negative means + // "unset" -- the caller picks the default. Not yet consumed by the + // generate path; present so settings round-trip for a follow-up change + // that wires it into the deadline selection. + SummaryTimeoutSeconds int `json:"summary_timeout_seconds,omitempty"` + // Deprecated: no longer used. Exists to tolerate old settings files // that still contain "strategy": "auto-commit" or similar. Strategy string `json:"strategy,omitempty"` @@ -155,6 +163,16 @@ func (s *EntireSettings) GetCommitLinking() string { return CommitLinkingPrompt } +// SummaryTimeoutValue returns the configured hard deadline for +// `entire explain --generate` summary generation. Zero means "unset" -- +// the caller picks the default. Negative values are treated as unset. +func (s *EntireSettings) SummaryTimeoutValue() time.Duration { + if s.SummaryTimeoutSeconds < 1 { + return 0 + } + return time.Duration(s.SummaryTimeoutSeconds) * time.Second +} + // Load loads the Entire settings from .entire/settings.json, // then applies any overrides from .entire/settings.local.json if it exists. // Returns default settings if neither file exists. @@ -418,6 +436,15 @@ func mergeJSON(settings *EntireSettings, data []byte) error { settings.Vercel = vercel } + // Override summary_timeout_seconds if present + if summaryTimeoutRaw, ok := raw["summary_timeout_seconds"]; ok { + var st int + if err := json.Unmarshal(summaryTimeoutRaw, &st); err != nil { + return fmt.Errorf("parsing summary_timeout_seconds field: %w", err) + } + settings.SummaryTimeoutSeconds = st + } + return nil } diff --git a/cmd/entire/cli/settings/settings_test.go b/cmd/entire/cli/settings/settings_test.go index ab9816c0a..8b24ee7f9 100644 --- a/cmd/entire/cli/settings/settings_test.go +++ b/cmd/entire/cli/settings/settings_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" "testing" + "time" ) const ( @@ -840,6 +841,28 @@ func TestIsFilteredFetchesEnabled_WrongType(t *testing.T) { } } +func TestSummaryTimeoutValue(t *testing.T) { + t.Parallel() + tests := []struct { + name string + seconds int + want time.Duration + }{ + {"Unset", 0, 0}, + {"Negative", -5, 0}, + {"Positive", 90, 90 * time.Second}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + s := &EntireSettings{SummaryTimeoutSeconds: tc.seconds} + if got := s.SummaryTimeoutValue(); got != tc.want { + t.Errorf("SummaryTimeoutValue() = %v; want %v", got, tc.want) + } + }) + } +} + // containsUnknownField checks if the error message indicates an unknown field func containsUnknownField(msg string) bool { // Go's json package reports unknown fields with this message format diff --git a/cmd/entire/cli/summarize/claude.go b/cmd/entire/cli/summarize/claude.go index cebc85a66..ec7f09576 100644 --- a/cmd/entire/cli/summarize/claude.go +++ b/cmd/entire/cli/summarize/claude.go @@ -93,7 +93,7 @@ func (g *ClaudeGenerator) Generate(ctx context.Context, input Input) (*checkpoin resultJSON, err := textGenerator.GenerateText(ctx, prompt, model) if err != nil { - return nil, fmt.Errorf("failed to generate summary text: %w", err) + return nil, err //nolint:wrapcheck // preserve *ClaudeError for errors.As at the explain layer } return parseSummaryText(resultJSON) diff --git a/cmd/entire/cli/summarize/claude_test.go b/cmd/entire/cli/summarize/claude_test.go index bf2c0b27e..b73d39811 100644 --- a/cmd/entire/cli/summarize/claude_test.go +++ b/cmd/entire/cli/summarize/claude_test.go @@ -2,6 +2,7 @@ package summarize import ( "context" + "errors" "strings" "testing" @@ -70,8 +71,8 @@ func TestClaudeGenerator_TextGeneratorError(t *testing.T) { t.Fatal("expected error") } - if !strings.Contains(err.Error(), "failed to generate summary text") { - t.Fatalf("unexpected error: %v", err) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("expected DeadlineExceeded, got: %v", err) } } diff --git a/cmd/entire/cli/summarize/summarize.go b/cmd/entire/cli/summarize/summarize.go index 8160f8363..bf2756874 100644 --- a/cmd/entire/cli/summarize/summarize.go +++ b/cmd/entire/cli/summarize/summarize.go @@ -56,7 +56,7 @@ func GenerateFromTranscript(ctx context.Context, transcriptBytes redact.Redacted summary, err := generator.Generate(ctx, input) if err != nil { - return nil, fmt.Errorf("failed to generate summary: %w", err) + return nil, err //nolint:wrapcheck // preserve *ClaudeError for errors.As at the explain layer } return summary, nil diff --git a/cmd/entire/cli/summarize/summarize_test.go b/cmd/entire/cli/summarize/summarize_test.go index 6ace80935..0a0d326f1 100644 --- a/cmd/entire/cli/summarize/summarize_test.go +++ b/cmd/entire/cli/summarize/summarize_test.go @@ -3,10 +3,13 @@ package summarize import ( "context" "encoding/json" + "errors" + "fmt" "strings" "testing" "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/claudecode" "github.com/entireio/cli/cmd/entire/cli/agent/types" "github.com/entireio/cli/cmd/entire/cli/transcript" "github.com/entireio/cli/redact" @@ -670,6 +673,40 @@ func TestGenerateFromTranscript(t *testing.T) { } } +// TestGenerateFromTranscript_PreservesClaudeError pins both //nolint:wrapcheck +// contracts in one shot: the stub TextGenerator returns a *ClaudeError, which +// must survive ClaudeGenerator.Generate (claude.go) AND GenerateFromTranscript +// (summarize.go) so the explain layer can map it to actionable user messaging +// via errors.As. A regression at either layer that flattens the typed error +// (e.g. fmt.Errorf("...: %v", err)) would fail this test. +func TestGenerateFromTranscript_PreservesClaudeError(t *testing.T) { + t.Parallel() + + claudeErr := &claudecode.ClaudeError{ + Kind: claudecode.ClaudeErrorRateLimit, + Message: "Rate limit exceeded", + APIStatus: 429, + } + gen := &ClaudeGenerator{TextGenerator: &stubTextGenerator{err: claudeErr}} + + transcript := []byte(`{"type":"user","message":{"content":"Hello"}} +{"type":"assistant","message":{"content":[{"type":"text","text":"Hi there"}]}}`) + + _, err := GenerateFromTranscript(context.Background(), redact.AlreadyRedacted(transcript), []string{}, "", gen) + wrapped := fmt.Errorf("explain generate call: %w", err) + + var ce *claudecode.ClaudeError + if !errors.As(wrapped, &ce) { + t.Fatalf("errors.As could not recover *ClaudeError from chain: %v", wrapped) + } + if ce.Kind != claudecode.ClaudeErrorRateLimit { + t.Errorf("Kind = %v; want %v", ce.Kind, claudecode.ClaudeErrorRateLimit) + } + if ce.APIStatus != 429 { + t.Errorf("APIStatus = %d; want 429", ce.APIStatus) + } +} + func TestGenerateFromTranscript_EmptyTranscript(t *testing.T) { mockGenerator := &ClaudeGenerator{}