feat: add provider-aware summary error handling - #1817
Merged
peyton-alt merged 5 commits intoJul 21, 2026
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes summary-generation failures across multiple providers by introducing a richer, provider-aware TextGenerationError shape, updating both non-streaming and streaming execution paths to preserve meaningful causes, and improving entire explain --generate rendering while keeping auto-summary logs privacy-safe.
Changes:
- Introduces provider/kind-aware
TextGenerationErrorclassification in the shared isolated CLI runner and streaming template, and updates provider agents to use it. - Updates
explain --generateto render provider-typed failures (including “missing CLI”) consistently and preserve unwrap chains for cancellation/deadline behavior. - Adjusts condensation auto-summary logging to record only structured metadata and expands tests across error matrices and subprocess fakes.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cmd/entire/cli/strategy/manual_commit_condensation.go | Switches auto-summary failure logging to structured, privacy-safe attrs. |
| cmd/entire/cli/strategy/manual_commit_condensation_test.go | Adds a regression test ensuring provider content is excluded from failure log attrs. |
| cmd/entire/cli/explain.go | Adds provider-typed summary error formatting/rendering and preserves cause chains via a formatted wrapper. |
| cmd/entire/cli/explain_test.go | Expands tests to cover non-Claude provider matrices, precedence, and cause preservation. |
| cmd/entire/cli/explain_summary_provider.go | Returns typed TextGenerationError for “configured provider missing CLI” instead of a generic error. |
| cmd/entire/cli/explain_summary_provider_test.go | Adds coverage verifying missing CLI errors are typed. |
| cmd/entire/cli/agent/text_generator_cli.go | Refactors isolated CLI runner to be provider-aware, returning a typed error with classification and metadata. |
| cmd/entire/cli/agent/text_generator_cli_test.go | Reworks tests around a helper subprocess, adds classification/precedence assertions, and validates generator wrapping behavior. |
| cmd/entire/cli/agent/streaming_template.go | Aligns streaming error handling with the same typed provider-aware error shape and precedence rules. |
| cmd/entire/cli/agent/streaming_template_test.go | Rebuilds tests to use a subprocess helper and validates classification/precedence and error-chain properties. |
| cmd/entire/cli/agent/pi/pi.go | Adds injectable CommandRunner on PiAgent. |
| cmd/entire/cli/agent/pi/models.go | Updates list-models path to the new isolated runner signature. |
| cmd/entire/cli/agent/pi/generate.go | Updates Pi generation path to use provider-aware isolated runner with injected runner support. |
| cmd/entire/cli/agent/pi/generate_test.go | Adds Pi error-path tests (missing CLI/auth/empty output) validating classification and wrapper counts. |
| cmd/entire/cli/agent/hook_command_windows_exec_test.go | Minor constant rename usage for Windows-only execution test gating. |
| cmd/entire/cli/agent/hook_command_test.go | Aligns OS constant usage with production hook wrapper selection logic. |
| cmd/entire/cli/agent/geminicli/generate.go | Migrates Gemini generator to the new isolated runner return shape and relies on typed error wrapping. |
| cmd/entire/cli/agent/cursor/generate.go | Migrates Cursor generator to the new isolated runner return shape and relies on typed error wrapping. |
| cmd/entire/cli/agent/copilotcli/generate.go | Migrates Copilot CLI generator to the new isolated runner return shape and relies on typed error wrapping. |
| cmd/entire/cli/agent/codex/generate.go | Migrates Codex generator to the new isolated runner return shape and relies on typed error wrapping. |
Comment on lines
+641
to
+658
| kind := "unknown" | ||
| switch failure.Kind { //nolint:exhaustive // Unknown is the safe default. | ||
| case agent.TextGenerationErrorAuth: | ||
| kind = "auth" | ||
| case agent.TextGenerationErrorRateLimit: | ||
| kind = "rate_limit" | ||
| case agent.TextGenerationErrorConfig: | ||
| kind = "config" | ||
| case agent.TextGenerationErrorCLIMissing: | ||
| kind = "cli_missing" | ||
| } | ||
| return []any{ | ||
| slog.String("failure_kind", kind), | ||
| slog.String("provider", string(failure.Provider)), | ||
| slog.Int("exit_code", failure.ExitCode), | ||
| slog.Int("stdout_bytes", failure.StdoutBytes), | ||
| slog.Int("stderr_bytes", len(failure.Stderr)), | ||
| } |
Comment on lines
18
to
21
| func (a *PiAgent) ListModels(ctx context.Context) ([]agent.ModelInfo, error) { | ||
| out, _, _, err := agent.RunIsolatedTextGeneratorCLI(ctx, nil, "pi", "pi", []string{"--list-models"}, "") | ||
| out, err := agent.RunIsolatedTextGeneratorCLI(ctx, nil, agent.AgentNamePi, []string{"--list-models"}, "") | ||
| if err != nil { | ||
| return nil, fmt.Errorf("pi --list-models: %w", err) |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stack
This PR is stacked on #1230, which is stacked on #964. Merge in that order.
It supersedes the older implementation in #1005 and carries forward the intent reviewed in Entire trail 193 without adding that old branch directly to the active stack.
Verification
mise run check— lint, race-enabled unit/integration suite, Vogon 59/59, Roger-Roger 4/4Native Pi calls were attempted but could not complete because the available OpenAI credential was quota-limited and the Google credential was invalid; Pi's deterministic error-path coverage passes.
Note
Medium Risk
Touches core summary generation, streaming fallback, and user-visible explain errors across multiple providers; behavior changes are well-tested but affect failure UX and error precedence on timeouts/cancellation.
Overview
Summary generation failures from external CLIs are now normalized into a shared
TextGenerationErrorwith provider identity, failure kind (auth, rate limit, config, missing CLI), message, stderr, exit code, and stdout byte counts.RunIsolatedTextGeneratorCLItakes a provider enum instead of separate binary/display strings, returns(string, error)only, and builds typed errors viaclassifyTextGenerationFailure(contextual HTTP status patterns, Gemini auth phrase). Codex, Copilot, Cursor, Gemini, and PiGenerateTextpaths wrap those errors instead of assembling their own envelopes. Pi gains injectableCommandRunnerfor tests.StreamingGeneratorTemplateuses the samenewTextGenerationErrorshape, classifies start/exit failures, and refines precedence: specific decoded provider errors beat concurrent deadlines; unknown provider/parser failures still join context cancellation/deadline with subprocess causes.Explain and
ensureSummaryProviderPresentsurface missing or misconfigured providers as typed failures with user-facing labels viaformatProviderSummaryError. Auto-summary condensation logs only structured metadata (no stderr/message content). Tests expand across isolated CLI helpers, streaming template subprocess fakes, and provider error matrices.Reviewed by Cursor Bugbot for commit 4ee08c3. Configure here.