diff --git a/cli/azd/cmd/middleware/error.go b/cli/azd/cmd/middleware/error.go index 855f4bdcfad..3afe57ffff7 100644 --- a/cli/azd/cmd/middleware/error.go +++ b/cli/azd/cmd/middleware/error.go @@ -11,6 +11,7 @@ import ( "fmt" "log" "text/template" + "time" surveyterm "github.com/AlecAivazis/survey/v2/terminal" "github.com/azure/azure-dev/cli/azd/cmd/actions" @@ -28,6 +29,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/environment/azdcontext" "github.com/azure/azure-dev/cli/azd/pkg/errorhandler" "github.com/azure/azure-dev/cli/azd/pkg/extensions" + "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning/bicep" "github.com/azure/azure-dev/cli/azd/pkg/input" "github.com/azure/azure-dev/cli/azd/pkg/output" "github.com/azure/azure-dev/cli/azd/pkg/pipeline" @@ -51,6 +53,9 @@ var troubleshootManualTmpl string //go:embed templates/fix.tmpl var fixTmpl string +// agentCallTimeout is the maximum time to wait for a single LLM agent call. +const agentCallTimeout = 5 * time.Minute + var ( explainTemplate = template.Must(template.New("explain").Parse(explainTmpl)) guidanceTemplate = template.Must(template.New("guidance").Parse(guidanceTmpl)) @@ -68,15 +73,20 @@ type ErrorMiddleware struct { errorPipeline *errorhandler.ErrorHandlerPipeline } -func fixableError(err error) bool { - // --- Machine context: typed errors --- - _, extRunErr := errors.AsType[*extensions.ExtensionRunError](err) - _, packStatusErr := errors.AsType[*pack.StatusCodeError](err) +// shouldSkipAgentHandling returns true for errors that should not be processed +// by the AI agent — either because they are normal control-flow signals or +// because they belong to error types the agent cannot fix. +func shouldSkipAgentHandling(err error) bool { + if errors.Is(err, context.Canceled) || + errors.Is(err, context.DeadlineExceeded) || + errors.Is(err, surveyterm.InterruptErr) || + errors.Is(err, azdcontext.ErrNoProject) || + errors.Is(err, consent.ErrToolExecutionDenied) || + errors.Is(err, consent.ErrElicitationDenied) || + errors.Is(err, consent.ErrSamplingDenied) || + errors.Is(err, internal.ErrAbortedByUser) || - if extRunErr || packStatusErr { - return false - } - if errors.Is(err, environment.ErrNotFound) || + errors.Is(err, environment.ErrNotFound) || errors.Is(err, environment.ErrNameNotSpecified) || errors.Is(err, environment.ErrDefaultEnvironmentNotFound) || errors.Is(err, environment.ErrAccessDenied) || @@ -85,10 +95,21 @@ func fixableError(err error) bool { errors.Is(err, pipeline.ErrSSHNotSupported) || errors.Is(err, pipeline.ErrRemoteHostIsNotGitHub) || errors.Is(err, project.ErrNoDefaultService) { - return false + return true } - return true + _, extRunErr := errors.AsType[*extensions.ExtensionRunError](err) + _, envErr := errors.AsType[*environment.EnvironmentInitError](err) + _, updateErr := errors.AsType[*update.UpdateError](err) + _, packStatusErr := errors.AsType[*pack.StatusCodeError](err) + _, missingInputsErr := errors.AsType[*bicep.MissingInputsError](err) + _, configValidErr := errors.AsType[*project.ConfigValidationError](err) + + if extRunErr || packStatusErr || missingInputsErr || configValidErr || updateErr || envErr { + return true + } + + return false } // troubleshootCategory represents the user's chosen troubleshooting scope. @@ -107,31 +128,17 @@ const ( categorySkip troubleshootCategory = "skip" ) -// shouldSkipErrorAnalysis returns true for control-flow errors that should not -// be sent to AI analysis -func shouldSkipErrorAnalysis(err error) bool { - if errors.Is(err, context.Canceled) || - errors.Is(err, surveyterm.InterruptErr) || - errors.Is(err, azdcontext.ErrNoProject) || - errors.Is(err, consent.ErrToolExecutionDenied) || - errors.Is(err, consent.ErrElicitationDenied) || - errors.Is(err, consent.ErrSamplingDenied) || - errors.Is(err, internal.ErrAbortedByUser) { - return true - } +// nextAction represents what the user wants after an explanation/guidance. +type nextAction string - // Environment was already initialized - if _, ok := errors.AsType[*environment.EnvironmentInitError](err); ok { - return true - } - - // Update errors have their own user-facing messages and suggestions - if _, ok := errors.AsType[*update.UpdateError](err); ok { - return true - } - - return false -} +const ( + // actionFixAndRetry fixes the error and retries the command. + actionFixAndRetry nextAction = "fix_and_retry" + // actionFixOnly fixes the error but does not retry. + actionFixOnly nextAction = "fix_only" + // actionExit exits without fixing. + actionExit nextAction = "exit" +) func NewErrorMiddleware( options *Options, console input.Console, @@ -177,12 +184,11 @@ func (e *ErrorMiddleware) Run(ctx context.Context, next NextFn) (*actions.Action // - LLM feature is disabled // - User specified --no-prompt (non-interactive mode) // - Running in CI/CD environment where user interaction is not possible - if !e.featuresManager.IsEnabled(agentcopilot.FeatureCopilot) || e.global.NoPrompt || resource.IsRunningOnCI() { - return actionResult, err - } - - // Skip control-flow errors that don't benefit from AI analysis - if shouldSkipErrorAnalysis(err) { + // - Errors that don't benefit from agent handling + if !e.featuresManager.IsEnabled(agentcopilot.FeatureCopilot) || + e.global.NoPrompt || + resource.IsRunningOnCI() || + shouldSkipAgentHandling(err) { return actionResult, err } @@ -193,14 +199,17 @@ func (e *ErrorMiddleware) Run(ctx context.Context, next NextFn) (*actions.Action defer span.End() originalError := err + createCtx, createCancel := context.WithTimeout(ctx, agentCallTimeout) + defer createCancel() azdAgent, err := e.agentFactory.Create( - ctx, + createCtx, agent.WithMode(agent.AgentModePlan), agent.WithDebug(e.global.EnableDebugLogging), ) if err != nil { span.SetStatus(codes.Error, "agent.creation.failed") - return nil, err + return actionResult, fmt.Errorf( + "%w\n\nAgent error: %s", originalError, err.Error()) } defer azdAgent.Stop() @@ -232,16 +241,12 @@ func (e *ErrorMiddleware) Run(ctx context.Context, next NextFn) (*actions.Action e.console.Message(ctx, output.WithErrorFormat("TraceID: %s", errorWithTraceId.TraceId)) } - // Skip agent troubleshooting for errors that are not classified as fixable - if !fixableError(originalError) { - return actionResult, originalError - } - // Step 1: Category selection — user chooses the troubleshooting scope category, err := e.promptTroubleshootCategory(ctx) if err != nil { span.SetStatus(codes.Error, "agent.category.failed") - return nil, fmt.Errorf("prompting for troubleshoot category: %w", err) + return actionResult, fmt.Errorf( + "%w\n\nPrompted for troubleshoot category: %s", originalError, err.Error()) } if category == categorySkip { @@ -253,10 +258,14 @@ func (e *ErrorMiddleware) Run(ctx context.Context, next NextFn) (*actions.Action categoryPrompt := e.buildPromptForCategory(category, originalError) e.console.Message(ctx, output.WithHintFormat( "Preparing %s to %s error...", agentcopilot.DisplayTitle, category)) - agentResult, err := azdAgent.SendMessageWithRetry(ctx, categoryPrompt) + + callCtx, callCancel := context.WithTimeout(ctx, agentCallTimeout) + defer callCancel() + agentResult, err := azdAgent.SendMessageWithRetry(callCtx, categoryPrompt) if err != nil { span.SetStatus(codes.Error, "agent.send_message.failed") - return nil, err + return actionResult, fmt.Errorf( + "%w\n\nAgent error: %s", originalError, err.Error()) } span.SetStatus(codes.Ok, fmt.Sprintf("agent.%s.completed", category)) @@ -265,36 +274,61 @@ func (e *ErrorMiddleware) Run(ctx context.Context, next NextFn) (*actions.Action previousError = originalError if category != categoryFix { - // Step 3: Ask if user wants the agent to fix the error - // (only if they didn't already choose the fix category) - wantFix, err := e.promptForFix(ctx) + // Step 3: Ask user how to proceed after analysis + action, err := e.promptNextAction(ctx) if err != nil { - return nil, fmt.Errorf("prompting for fix: %w", err) + span.SetStatus(codes.Error, "agent.prompt.failed") + return actionResult, fmt.Errorf( + "%w\n\nPrompted for next action: %s", originalError, err.Error()) } - if !wantFix { + if action == actionExit { span.SetStatus(codes.Ok, "agent.fix.declined") return actionResult, originalError } // Step 4: Agent applies the fix - fixPrompt := e.buildFixPrompt(originalError) + fixPrompt, err := e.buildFixPrompt(originalError) + if err != nil { + span.SetStatus(codes.Error, "agent.fix.template_failed") + return actionResult, fmt.Errorf( + "%w\n\nFailed to build fix prompt: %s", originalError, err.Error()) + } e.console.Message(ctx, output.WithHintFormat( "Preparing %s to fix error...", agentcopilot.DisplayTitle)) - fixResult, err := azdAgent.SendMessageWithRetry(ctx, fixPrompt) + + fixCtx, fixCancel := context.WithTimeout(ctx, agentCallTimeout) + defer fixCancel() + fixResult, err := azdAgent.SendMessageWithRetry(fixCtx, fixPrompt) if err != nil { span.SetStatus(codes.Error, "agent.fix.failed") - return nil, err + return actionResult, fmt.Errorf( + "%w\n\nAgent error: %s", originalError, err.Error()) } span.SetStatus(codes.Ok, "agent.fix.completed") e.displayUsageMetrics(ctx, fixResult) - } - // Step 5: Ask user if they want to retry the command - shouldRetry, err := e.promptRetryAfterFix(ctx) - if err != nil || !shouldRetry { - return actionResult, originalError + // Fix-only: skip retry + if action == actionFixOnly { + return actionResult, originalError + } + // actionFixAndRetry: user explicitly chose to retry, so fall + // through to re-run the command without an extra confirmation prompt. + } else { + // Category "fix" already applied the fix in step 2. + // Unlike the explain/guidance path (which auto-retries when the user + // explicitly chose actionFixAndRetry), the category-fix path asks for + // retry confirmation because the user only chose "fix" — not "fix and retry". + shouldRetry, err := e.promptRetryAfterFix(ctx) + if err != nil { + span.SetStatus(codes.Error, "agent.retry.failed") + return actionResult, fmt.Errorf( + "%w\n\nRetry prompt failed: %s", originalError, err.Error()) + } + if !shouldRetry { + return actionResult, originalError + } } // Re-run the original command to check if the fix worked @@ -302,7 +336,7 @@ func (e *ErrorMiddleware) Run(ctx context.Context, next NextFn) (*actions.Action actionResult, err = next(ctx) originalError = err - if shouldSkipErrorAnalysis(err) { + if shouldSkipAgentHandling(err) { return actionResult, err } } @@ -348,7 +382,7 @@ func (e *ErrorMiddleware) buildPromptForCategory(category troubleshootCategory, } // buildFixPrompt renders the fix prompt template. -func (e *ErrorMiddleware) buildFixPrompt(err error) string { +func (e *ErrorMiddleware) buildFixPrompt(err error) (string, error) { data := errorPromptData{ Command: e.options.CommandPath, ErrorMessage: err.Error(), @@ -356,12 +390,11 @@ func (e *ErrorMiddleware) buildFixPrompt(err error) string { var buf bytes.Buffer if execErr := fixTemplate.Execute(&buf, data); execErr != nil { - log.Printf("[copilot] Failed to execute fix template: %v", execErr) - return fmt.Sprintf("An error occurred while fixing `%s`: %v\n", - data.ErrorMessage, execErr) + return "", fmt.Errorf( + "executing fix template: %w", execErr) } - return buf.String() + return buf.String(), nil } // displayUsageMetrics shows token usage metrics after an agent interaction. @@ -374,7 +407,7 @@ func (e *ErrorMiddleware) displayUsageMetrics(ctx context.Context, result *agent // promptTroubleshootCategory asks the user to select a troubleshooting scope. // Checks saved category preference; if set, auto-selects and prints a message. -// Otherwise presents: Explain, Guidance, Troubleshoot (explain + guidance), Skip. +// Otherwise presents: Diagnose and guide, Fix (default), Skip, Explain, Guidance. func (e *ErrorMiddleware) promptTroubleshootCategory(ctx context.Context) (troubleshootCategory, error) { userConfig, err := e.userConfigManager.Load() if err != nil { @@ -399,20 +432,25 @@ func (e *ErrorMiddleware) promptTroubleshootCategory(ctx context.Context) (troub } choices := []*uxlib.SelectChoice{ - {Value: string(categoryExplain), Label: "Explain this error"}, - {Value: string(categoryGuidance), Label: "Show fix guidance"}, - {Value: string(categoryTroubleshoot), Label: "Troubleshoot with explanation and guidance"}, + {Value: string(categoryTroubleshoot), Label: "Diagnose and guide"}, {Value: string(categoryFix), Label: "Fix this error"}, {Value: string(categorySkip), Label: "Skip"}, + {Value: string(categoryExplain), Label: "Explain this error"}, + {Value: string(categoryGuidance), Label: "Show fix guidance"}, } selector := uxlib.NewSelect(&uxlib.SelectOptions{ - Message: fmt.Sprintf("How would you like %s to help?", agentcopilot.DisplayTitle), + Message: fmt.Sprintf( + "How would you like %s to help?", + agentcopilot.DisplayTitle), HelpMessage: fmt.Sprintf( "Choose the level of assistance. "+ "To always use a specific choice, run %s.", output.WithHighLightFormat( - fmt.Sprintf("azd config set %s ", agentcopilot.ConfigKeyErrorHandlingCategory))), + fmt.Sprintf( + "azd config set %s ", + agentcopilot.ConfigKeyErrorHandlingCategory))), Choices: choices, + SelectedIndex: new(1), EnableFiltering: new(false), DisplayCount: len(choices), }) @@ -439,37 +477,51 @@ func (e *ErrorMiddleware) promptTroubleshootCategory(ctx context.Context) (troub return selected, nil } -// promptForFix asks the user if they want the agent to attempt to fix the error. -// Checks saved preferences for auto-approval. -func (e *ErrorMiddleware) promptForFix(ctx context.Context) (bool, error) { +// promptNextAction asks the user how to proceed after an explanation or guidance. +// Offers fix-and-retry, fix-only, or exit. Checks saved fix preference for auto-approval. +func (e *ErrorMiddleware) promptNextAction( + ctx context.Context, +) (nextAction, error) { userConfig, err := e.userConfigManager.Load() if err != nil { - return false, fmt.Errorf("failed to load user config: %w", err) + return actionExit, fmt.Errorf( + "failed to load user config: %w", err) } - // Check for saved "always fix" preference - if val, ok := userConfig.GetString(agentcopilot.ConfigKeyErrorHandlingFix); ok && val == "allow" { + // Saved "always fix" preference → auto-approve fix only (user still + // controls retry via the category-fix path or interactive prompt). + if val, ok := userConfig.GetString( + agentcopilot.ConfigKeyErrorHandlingFix); ok && val == "allow" { e.console.Message(ctx, output.WithWarningFormat( "\n%s auto-fix is enabled. To change, run %s.", agentcopilot.DisplayTitle, output.WithHighLightFormat( - fmt.Sprintf("azd config unset %s", agentcopilot.ConfigKeyErrorHandlingFix)), + fmt.Sprintf("azd config unset %s", + agentcopilot.ConfigKeyErrorHandlingFix)), )) - return true, nil + return actionFixOnly, nil } choices := []*uxlib.SelectChoice{ - {Value: "yes", Label: fmt.Sprintf("Yes, let %s fix it", agentcopilot.DisplayTitle)}, - {Value: "no", Label: "No, I'll fix it myself"}, + {Value: string(actionFixAndRetry), + Label: fmt.Sprintf( + "Fix with %s and retry command", + agentcopilot.DisplayTitle)}, + {Value: string(actionFixOnly), + Label: fmt.Sprintf( + "Fix with %s", + agentcopilot.DisplayTitle)}, + {Value: string(actionExit), + Label: "Exit"}, } selector := uxlib.NewSelect(&uxlib.SelectOptions{ - Message: fmt.Sprintf("Would you like %s to fix this error?", agentcopilot.DisplayTitle), + Message: "How would you like to proceed?", HelpMessage: fmt.Sprintf( - "The agent will fix the error. "+ - "To always allow fixes, run %s.", + "To always allow fixes, run %s.", output.WithHighLightFormat( - fmt.Sprintf("azd config set %s allow", agentcopilot.ConfigKeyErrorHandlingFix))), + fmt.Sprintf("azd config set %s allow", + agentcopilot.ConfigKeyErrorHandlingFix))), Choices: choices, EnableFiltering: new(false), DisplayCount: len(choices), @@ -478,14 +530,16 @@ func (e *ErrorMiddleware) promptForFix(ctx context.Context) (bool, error) { e.console.Message(ctx, "") choiceIndex, err := selector.Ask(ctx) if err != nil { - return false, err + return actionExit, err } - if choiceIndex == nil || *choiceIndex < 0 || *choiceIndex >= len(choices) { - return false, fmt.Errorf("invalid fix choice selected") + if choiceIndex == nil || + *choiceIndex < 0 || + *choiceIndex >= len(choices) { + return actionExit, fmt.Errorf("invalid choice selected") } - return choices[*choiceIndex].Value == "yes", nil + return nextAction(choices[*choiceIndex].Value), nil } // promptRetryAfterFix asks the user if the agent applied a fix and they want to retry the command. diff --git a/cli/azd/cmd/middleware/error_test.go b/cli/azd/cmd/middleware/error_test.go index d73c552a7a4..5c2720e454f 100644 --- a/cli/azd/cmd/middleware/error_test.go +++ b/cli/azd/cmd/middleware/error_test.go @@ -13,6 +13,7 @@ import ( surveyterm "github.com/AlecAivazis/survey/v2/terminal" "github.com/azure/azure-dev/cli/azd/cmd/actions" "github.com/azure/azure-dev/cli/azd/internal" + "github.com/azure/azure-dev/cli/azd/internal/agent" agentcopilot "github.com/azure/azure-dev/cli/azd/internal/agent/copilot" "github.com/azure/azure-dev/cli/azd/pkg/alpha" "github.com/azure/azure-dev/cli/azd/pkg/auth" @@ -21,6 +22,7 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/errorhandler" "github.com/azure/azure-dev/cli/azd/pkg/extensions" + "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning/bicep" "github.com/azure/azure-dev/cli/azd/pkg/pipeline" "github.com/azure/azure-dev/cli/azd/pkg/project" "github.com/azure/azure-dev/cli/azd/pkg/tools" @@ -28,7 +30,9 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/tools/pack" "github.com/azure/azure-dev/cli/azd/pkg/update" "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/azure/azure-dev/cli/azd/test/mocks/mockinput" "github.com/blang/semver/v4" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -264,28 +268,28 @@ func Test_ErrorMiddleware_NoPatternMatch(t *testing.T) { require.Equal(t, unknownError, err) } -func Test_FixableError(t *testing.T) { +func Test_ShouldSkipAgentHandling_FixableErrors(t *testing.T) { t.Parallel() - t.Run("MissingToolErrors is fixable", func(t *testing.T) { + t.Run("MissingToolErrors is not skipped", func(t *testing.T) { t.Parallel() err := &tools.MissingToolErrors{ Errs: []error{errors.New("docker not found")}, ToolNames: []string{"docker"}, } - require.True(t, fixableError(err)) + require.False(t, shouldSkipAgentHandling(err)) }) - t.Run("Wrapped MissingToolErrors is fixable", func(t *testing.T) { + t.Run("Wrapped MissingToolErrors is not skipped", func(t *testing.T) { t.Parallel() inner := &tools.MissingToolErrors{ Errs: []error{errors.New("node not found")}, ToolNames: []string{"node"}, } wrapped := fmt.Errorf("setup failed: %w", inner) - require.True(t, fixableError(wrapped)) + require.False(t, shouldSkipAgentHandling(wrapped)) }) - t.Run("ErrSemver is fixable", func(t *testing.T) { + t.Run("ErrSemver is not skipped", func(t *testing.T) { t.Parallel() err := &tools.ErrSemver{ ToolName: "node", @@ -294,124 +298,124 @@ func Test_FixableError(t *testing.T) { UpdateCommand: "nvm install", }, } - require.True(t, fixableError(err)) + require.False(t, shouldSkipAgentHandling(err)) }) - t.Run("ExtensionRunError is not fixable", func(t *testing.T) { + t.Run("ExtensionRunError is skipped", func(t *testing.T) { t.Parallel() err := &extensions.ExtensionRunError{ ExtensionId: "my-extension", Err: errors.New("extension crashed"), } - require.False(t, fixableError(err)) + require.True(t, shouldSkipAgentHandling(err)) }) - t.Run("StatusCodeError is not fixable", func(t *testing.T) { + t.Run("StatusCodeError is skipped", func(t *testing.T) { t.Parallel() err := &pack.StatusCodeError{ Code: 1, Err: errors.New("pack build failed"), } - require.False(t, fixableError(err)) + require.True(t, shouldSkipAgentHandling(err)) }) - t.Run("ReLoginRequiredError is fixable", func(t *testing.T) { + t.Run("ReLoginRequiredError is not skipped", func(t *testing.T) { t.Parallel() err := &auth.ReLoginRequiredError{} - require.True(t, fixableError(err)) + require.False(t, shouldSkipAgentHandling(err)) }) - t.Run("AuthFailedError is fixable", func(t *testing.T) { + t.Run("AuthFailedError is not skipped", func(t *testing.T) { t.Parallel() err := &auth.AuthFailedError{} - require.True(t, fixableError(err)) + require.False(t, shouldSkipAgentHandling(err)) }) sentinels := []struct { name string err error - fixable bool + skipped bool }{ - {"auth.ErrNoCurrentUser", auth.ErrNoCurrentUser, true}, - {"azapi.ErrAzCliNotLoggedIn", azapi.ErrAzCliNotLoggedIn, true}, + {"auth.ErrNoCurrentUser", auth.ErrNoCurrentUser, false}, + {"azapi.ErrAzCliNotLoggedIn", azapi.ErrAzCliNotLoggedIn, false}, {"azapi.ErrAzCliRefreshTokenExpired", - azapi.ErrAzCliRefreshTokenExpired, true}, + azapi.ErrAzCliRefreshTokenExpired, false}, {"github.ErrGitHubCliNotLoggedIn", - github.ErrGitHubCliNotLoggedIn, true}, - {"github.ErrUserNotAuthorized", github.ErrUserNotAuthorized, true}, - {"github.ErrRepositoryNameInUse", github.ErrRepositoryNameInUse, true}, + github.ErrGitHubCliNotLoggedIn, false}, + {"github.ErrUserNotAuthorized", github.ErrUserNotAuthorized, false}, + {"github.ErrRepositoryNameInUse", github.ErrRepositoryNameInUse, false}, // environment - {"environment.ErrNotFound", environment.ErrNotFound, false}, - {"environment.ErrNameNotSpecified", environment.ErrNameNotSpecified, false}, + {"environment.ErrNotFound", environment.ErrNotFound, true}, + {"environment.ErrNameNotSpecified", environment.ErrNameNotSpecified, true}, {"environment.ErrDefaultEnvironmentNotFound", - environment.ErrDefaultEnvironmentNotFound, false}, - {"environment.ErrAccessDenied", environment.ErrAccessDenied, false}, + environment.ErrDefaultEnvironmentNotFound, true}, + {"environment.ErrAccessDenied", environment.ErrAccessDenied, true}, // pipeline - {"pipeline.ErrAuthNotSupported", pipeline.ErrAuthNotSupported, false}, - {"pipeline.ErrRemoteHostIsNotAzDo", pipeline.ErrRemoteHostIsNotAzDo, false}, - {"pipeline.ErrSSHNotSupported", pipeline.ErrSSHNotSupported, false}, - {"pipeline.ErrRemoteHostIsNotGitHub", pipeline.ErrRemoteHostIsNotGitHub, false}, + {"pipeline.ErrAuthNotSupported", pipeline.ErrAuthNotSupported, true}, + {"pipeline.ErrRemoteHostIsNotAzDo", pipeline.ErrRemoteHostIsNotAzDo, true}, + {"pipeline.ErrSSHNotSupported", pipeline.ErrSSHNotSupported, true}, + {"pipeline.ErrRemoteHostIsNotGitHub", pipeline.ErrRemoteHostIsNotGitHub, true}, // project - {"project.ErrNoDefaultService", project.ErrNoDefaultService, false}, + {"project.ErrNoDefaultService", project.ErrNoDefaultService, true}, } for _, tc := range sentinels { - t.Run(tc.name+" fixableError", func(t *testing.T) { + t.Run(tc.name+" shouldSkipAgentHandling", func(t *testing.T) { t.Parallel() - require.Equal(t, tc.fixable, fixableError(tc.err)) + require.Equal(t, tc.skipped, shouldSkipAgentHandling(tc.err)) }) - t.Run("Wrapped "+tc.name+" fixableError", func(t *testing.T) { + t.Run("Wrapped "+tc.name+" shouldSkipAgentHandling", func(t *testing.T) { t.Parallel() wrapped := fmt.Errorf("operation failed: %w", tc.err) - require.Equal(t, tc.fixable, fixableError(wrapped)) + require.Equal(t, tc.skipped, shouldSkipAgentHandling(wrapped)) }) } // --- Azure context: defaults --- - t.Run("Generic error defaults to AzureContext", func(t *testing.T) { + t.Run("Generic error defaults to not skipped", func(t *testing.T) { t.Parallel() err := errors.New("deploying to Azure: InternalServerError") - require.True(t, fixableError(err)) + require.False(t, shouldSkipAgentHandling(err)) }) } -func Test_ShouldSkipErrorAnalysis(t *testing.T) { +func Test_ShouldSkipAgentHandling_ControlFlow(t *testing.T) { t.Parallel() t.Run("Wrapped context.Canceled is skipped", func(t *testing.T) { t.Parallel() wrapped := fmt.Errorf("operation aborted: %w", context.Canceled) - require.True(t, shouldSkipErrorAnalysis(wrapped)) + require.True(t, shouldSkipAgentHandling(wrapped)) }) t.Run("Wrapped InterruptErr is skipped", func(t *testing.T) { t.Parallel() wrapped := fmt.Errorf("prompt failed: %w", surveyterm.InterruptErr) - require.True(t, shouldSkipErrorAnalysis(wrapped)) + require.True(t, shouldSkipAgentHandling(wrapped)) }) t.Run("ErrAbortedByUser is skipped", func(t *testing.T) { t.Parallel() - require.True(t, shouldSkipErrorAnalysis(internal.ErrAbortedByUser)) + require.True(t, shouldSkipAgentHandling(internal.ErrAbortedByUser)) }) t.Run("Wrapped ErrAbortedByUser is skipped", func(t *testing.T) { t.Parallel() wrapped := fmt.Errorf("preflight declined: %w", internal.ErrAbortedByUser) - require.True(t, shouldSkipErrorAnalysis(wrapped)) + require.True(t, shouldSkipAgentHandling(wrapped)) }) t.Run("UpdateError is skipped", func(t *testing.T) { t.Parallel() err := &update.UpdateError{Code: update.CodeDownloadFailed, Err: errors.New("download failed")} - require.True(t, shouldSkipErrorAnalysis(err)) + require.True(t, shouldSkipAgentHandling(err)) }) t.Run("Wrapped UpdateError is skipped", func(t *testing.T) { t.Parallel() inner := &update.UpdateError{Code: update.CodeReplaceFailed, Err: errors.New("replace failed")} wrapped := fmt.Errorf("update error: %w", inner) - require.True(t, shouldSkipErrorAnalysis(wrapped)) + require.True(t, shouldSkipAgentHandling(wrapped)) }) } @@ -482,7 +486,8 @@ func Test_BuildFixPrompt(t *testing.T) { } testErr := errors.New("resource group not found") - prompt := middleware.buildFixPrompt(testErr) + prompt, err := middleware.buildFixPrompt(testErr) + require.NoError(t, err) require.Contains(t, prompt, "azd up") require.Contains(t, prompt, "resource group not found") require.Contains(t, prompt, "FIX") @@ -494,3 +499,254 @@ func Test_ConfigKeyErrorHandlingCategory(t *testing.T) { // Verify the config key is properly namespaced require.Equal(t, "copilot.errorHandling.category", agentcopilot.ConfigKeyErrorHandlingCategory) } + +func Test_ShouldSkipAgentHandling_DeadlineExceeded(t *testing.T) { + t.Parallel() + require.True(t, shouldSkipAgentHandling(context.DeadlineExceeded)) +} + +func Test_ShouldSkipAgentHandling_WrappedDeadlineExceeded(t *testing.T) { + t.Parallel() + wrapped := fmt.Errorf("timed out: %w", context.DeadlineExceeded) + require.True(t, shouldSkipAgentHandling(wrapped)) +} + +func Test_ShouldSkipAgentHandling_MissingInputsError(t *testing.T) { + t.Parallel() + err := &bicep.MissingInputsError{ + Inputs: []bicep.MissingInput{ + {Name: "location"}, + }, + } + require.True(t, shouldSkipAgentHandling(err)) +} + +func Test_ShouldSkipAgentHandling_WrappedMissingInputsError(t *testing.T) { + t.Parallel() + inner := &bicep.MissingInputsError{ + Inputs: []bicep.MissingInput{ + {Name: "location"}, + }, + } + wrapped := fmt.Errorf("provision failed: %w", inner) + require.True(t, shouldSkipAgentHandling(wrapped)) +} + +func Test_ShouldSkipAgentHandling_ConfigValidationError(t *testing.T) { + t.Parallel() + err := &project.ConfigValidationError{ + Issues: []string{"service 'web' has nil definition"}, + } + require.True(t, shouldSkipAgentHandling(err)) +} + +func Test_ShouldSkipAgentHandling_WrappedConfigValidationError(t *testing.T) { + t.Parallel() + inner := &project.ConfigValidationError{ + Issues: []string{"hook 'preprovision' is nil"}, + } + wrapped := fmt.Errorf("config load: %w", inner) + require.True(t, shouldSkipAgentHandling(wrapped)) +} + +func Test_ErrorMiddleware_NonFixableError_SkipsAgentCreation(t *testing.T) { + t.Parallel() + + mockContext := mocks.NewMockContext(context.Background()) + cfg := config.NewConfig(map[string]any{ + "alpha": map[string]any{ + string(agentcopilot.FeatureCopilot): "on", + }, + }) + featureManager := alpha.NewFeaturesManagerWithConfig(cfg) + global := &internal.GlobalCommandOptions{NoPrompt: false} + userConfigManager := config.NewUserConfigManager( + mockContext.ConfigManager) + errorPipeline := errorhandler.NewErrorHandlerPipeline(nil) + + // agentFactory is nil — if code tries to call Create, it panics + middleware := NewErrorMiddleware( + &Options{Name: "test"}, + mockContext.Console, + nil, + global, + featureManager, + userConfigManager, + errorPipeline, + ) + + // environment.ErrNotFound is non-fixable + nextFn := func(ctx context.Context) (*actions.ActionResult, error) { + return &actions.ActionResult{}, environment.ErrNotFound + } + + result, err := middleware.Run(*mockContext.Context, nextFn) + + // Should return error without ever touching the agent factory + require.Error(t, err) + require.ErrorIs(t, err, environment.ErrNotFound) + require.NotNil(t, result) +} + +// clearCIEnvVarsForTest clears environment variables that affect terminal detection. +// +// Uses the same t.Setenv + os.Unsetenv pattern as terminal_test.go. +func clearCIEnvVarsForTest(t *testing.T) { + t.Helper() + ciVars := []string{"AZD_FORCE_TTY", + // CI env vars + "CI", "TF_BUILD", "GITHUB_ACTIONS", + } + + for _, v := range ciVars { + if _, exists := os.LookupEnv(v); exists { + t.Setenv(v, "") + os.Unsetenv(v) + } + } +} + +func Test_ErrorMiddleware_ExplainAndFixCalls(t *testing.T) { + clearCIEnvVarsForTest(t) + + explainResult := &agent.AgentResult{ + Usage: agent.UsageMetrics{ + InputTokens: 500, + OutputTokens: 200, + }, + } + + // Explain succeeds, fix fails — proves both calls are made + fakeAg := &fakeSequenceAgent{ + results: []*agent.AgentResult{explainResult, nil}, + errors: []error{nil, errors.New("fix attempt failed")}, + } + + factory := &mockAgentFactory{} + factory.On("Create", mock.Anything, mock.Anything). + Return(fakeAg, nil) + + ucm := &mockUserConfigManager{ + cfg: configWithKeys( + agentcopilot.ConfigKeyErrorHandlingCategory, "explain", + agentcopilot.ConfigKeyErrorHandlingFix, "allow", + ), + } + fm := copilotEnabledFeatureManager() + global := &internal.GlobalCommandOptions{} + + m := newErrorMiddlewareForTest( + mockinput.NewMockConsole(), factory, fm, ucm, global) + + originalErr := errors.New( + "unexpected widget provisioning failure") + result, err := m.Run(t.Context(), func( + _ context.Context, + ) (*actions.ActionResult, error) { + return nil, originalErr + }) + + // Explain succeeded, fix failed — original error returned + // wrapped with agent context + require.Error(t, err) + require.ErrorIs(t, err, originalErr) + + // Result may be nil since nextFn returned nil actionResult + _ = result + + // Verify both calls were made: explain (1st) + fix (2nd) + require.Equal(t, 2, fakeAg.callIdx, + "agent should be called twice: explain + fix") +} + +func Test_ErrorMiddleware_MaxRetry_FirstIterationSkipsCounter(t *testing.T) { + clearCIEnvVarsForTest(t) + + // Agent fails on fix — exits before the TTY retry prompt. + // This still proves the counter was skipped (agent WAS called). + fakeAg := &fakeSequenceAgent{ + results: []*agent.AgentResult{nil}, + errors: []error{errors.New("agent fix attempt failed")}, + } + + factory := &mockAgentFactory{} + factory.On("Create", mock.Anything, mock.Anything). + Return(fakeAg, nil) + + ucm := &mockUserConfigManager{ + cfg: configWithKeys( + agentcopilot.ConfigKeyErrorHandlingCategory, "fix"), + } + fm := copilotEnabledFeatureManager() + global := &internal.GlobalCommandOptions{} + + m := newErrorMiddlewareForTest( + mockinput.NewMockConsole(), factory, fm, ucm, global) + + sameError := errors.New("same error every time") + callCount := 0 + result, err := m.Run(t.Context(), func( + _ context.Context, + ) (*actions.ActionResult, error) { + callCount++ + return &actions.ActionResult{}, sameError + }) + + // First iteration: previousError is nil, counter is skipped, + // agent fix is called (and fails). The middleware returns original + // error wrapped with agent context. This proves the counter + // did NOT trigger on first iteration — if it had, the agent + // would never have been called. + // + // The "fix it manually" bail-out (attempt >= 3) requires 3+ + // same-error loop iterations, each needing promptRetryAfterFix + // to return retry=true (requires raw TTY). That path requires + // integration testing. + require.Error(t, err) + require.ErrorIs(t, err, sameError, + "original error should be preserved") + require.NotContains(t, err.Error(), + "fix it manually", + "should NOT reach max attempts on first iteration") + + // Agent was called once; the fix attempt failed, which still proves + // the counter was skipped on the first iteration. + require.Equal(t, 1, fakeAg.callIdx) + + // next was called once (no retry without TTY prompt) + require.Equal(t, 1, callCount) + require.NotNil(t, result) +} + +func Test_PromptNextAction_SavedAllow_ReturnsFixOnly(t *testing.T) { + t.Parallel() + + cfg := configWithKeys(agentcopilot.ConfigKeyErrorHandlingFix, "allow") + ucm := &mockUserConfigManager{cfg: cfg} + + m := &ErrorMiddleware{ + console: mockinput.NewMockConsole(), + userConfigManager: ucm, + } + + action, err := m.promptNextAction(t.Context()) + require.NoError(t, err) + require.Equal(t, actionFixOnly, action, + "saved 'allow' preference should return actionFixOnly, not actionFixAndRetry") +} + +func Test_PromptNextAction_ConfigLoadError(t *testing.T) { + t.Parallel() + + ucm := &mockUserConfigManager{cfg: nil, err: errors.New("io error")} + + m := &ErrorMiddleware{ + console: mockinput.NewMockConsole(), + userConfigManager: ucm, + } + + action, err := m.promptNextAction(t.Context()) + require.Error(t, err) + require.Contains(t, err.Error(), "io error") + require.Equal(t, actionExit, action) +} diff --git a/cli/azd/cmd/middleware/middleware_coverage2_test.go b/cli/azd/cmd/middleware/middleware_coverage2_test.go index 641cb2943a7..f5c5c971617 100644 --- a/cli/azd/cmd/middleware/middleware_coverage2_test.go +++ b/cli/azd/cmd/middleware/middleware_coverage2_test.go @@ -423,10 +423,10 @@ func TestTelemetryMiddleware_setInstalledExtensionsAttributes_Sorted(t *testing. } // --------------------------------------------------------------------------- -// shouldSkipErrorAnalysis — consent and azdcontext errors +// shouldSkipAgentHandling — consent and azdcontext errors // --------------------------------------------------------------------------- -func TestShouldSkipErrorAnalysis_ConsentErrors(t *testing.T) { +func TestShouldSkipAgentHandling_ConsentErrors(t *testing.T) { t.Parallel() tests := []struct { name string @@ -443,66 +443,66 @@ func TestShouldSkipErrorAnalysis_ConsentErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() - require.True(t, shouldSkipErrorAnalysis(tt.err)) + require.True(t, shouldSkipAgentHandling(tt.err)) }) } } -func TestShouldSkipErrorAnalysis_AzdContextErrNoProject(t *testing.T) { +func TestShouldSkipAgentHandling_AzdContextErrNoProject(t *testing.T) { t.Parallel() - require.True(t, shouldSkipErrorAnalysis(azdcontext.ErrNoProject)) + require.True(t, shouldSkipAgentHandling(azdcontext.ErrNoProject)) } -func TestShouldSkipErrorAnalysis_WrappedErrNoProject(t *testing.T) { +func TestShouldSkipAgentHandling_WrappedErrNoProject(t *testing.T) { t.Parallel() err := fmt.Errorf("init failed: %w", azdcontext.ErrNoProject) - require.True(t, shouldSkipErrorAnalysis(err)) + require.True(t, shouldSkipAgentHandling(err)) } -func TestShouldSkipErrorAnalysis_EnvironmentInitError(t *testing.T) { +func TestShouldSkipAgentHandling_EnvironmentInitError(t *testing.T) { t.Parallel() err := &environment.EnvironmentInitError{Name: "test-env"} - require.True(t, shouldSkipErrorAnalysis(err)) + require.True(t, shouldSkipAgentHandling(err)) } -func TestShouldSkipErrorAnalysis_WrappedEnvironmentInitError(t *testing.T) { +func TestShouldSkipAgentHandling_WrappedEnvironmentInitError(t *testing.T) { t.Parallel() inner := &environment.EnvironmentInitError{Name: "test-env"} err := fmt.Errorf("env error: %w", inner) - require.True(t, shouldSkipErrorAnalysis(err)) + require.True(t, shouldSkipAgentHandling(err)) } // --------------------------------------------------------------------------- -// fixableError — determines if an error is eligible for agentic fix +// shouldSkipAgentHandling — non-fixable error types // --------------------------------------------------------------------------- -func TestFixableError_ExtensionRunError(t *testing.T) { +func TestShouldSkipAgentHandling_ExtensionRunError(t *testing.T) { t.Parallel() err := &extensions.ExtensionRunError{ExtensionId: "test-ext", Err: fmt.Errorf("failed")} - require.False(t, fixableError(err), "ExtensionRunError should not be fixable") + require.True(t, shouldSkipAgentHandling(err), "ExtensionRunError should be skipped") } -func TestFixableError_WrappedExtensionRunError(t *testing.T) { +func TestShouldSkipAgentHandling_WrappedExtensionRunError(t *testing.T) { t.Parallel() inner := &extensions.ExtensionRunError{ExtensionId: "test-ext", Err: fmt.Errorf("failed")} err := fmt.Errorf("ext failed: %w", inner) - require.False(t, fixableError(err), "wrapped ExtensionRunError should not be fixable") + require.True(t, shouldSkipAgentHandling(err), "wrapped ExtensionRunError should be skipped") } -func TestFixableError_EnvironmentNotFound(t *testing.T) { +func TestShouldSkipAgentHandling_EnvironmentNotFound(t *testing.T) { t.Parallel() - require.False(t, fixableError(environment.ErrNotFound), "ErrNotFound should not be fixable") + require.True(t, shouldSkipAgentHandling(environment.ErrNotFound), "ErrNotFound should be skipped") } -func TestFixableError_PipelineAuthNotSupported(t *testing.T) { +func TestShouldSkipAgentHandling_PipelineAuthNotSupported(t *testing.T) { t.Parallel() - require.False(t, fixableError(pipeline.ErrAuthNotSupported), "ErrAuthNotSupported should not be fixable") + require.True(t, shouldSkipAgentHandling(pipeline.ErrAuthNotSupported), "ErrAuthNotSupported should be skipped") } -func TestFixableError_GenericError(t *testing.T) { +func TestShouldSkipAgentHandling_GenericError(t *testing.T) { t.Parallel() err := fmt.Errorf("some azure error") - require.True(t, fixableError(err), "generic error should be fixable") + require.False(t, shouldSkipAgentHandling(err), "generic error should not be skipped") } // --------------------------------------------------------------------------- @@ -566,47 +566,6 @@ func TestPromptTroubleshootCategory_InvalidSavedValue(t *testing.T) { // the "val != ''" check. } -// --------------------------------------------------------------------------- -// promptForFix — saved preference paths -// --------------------------------------------------------------------------- - -func TestPromptForFix_SavedAllow(t *testing.T) { - t.Parallel() - mockCtx := mocks.NewMockContext(t.Context()) - userConfigManager := config.NewUserConfigManager(mockCtx.ConfigManager) - cfg, err := userConfigManager.Load() - require.NoError(t, err) - err = cfg.Set(agentcopilot.ConfigKeyErrorHandlingFix, "allow") - require.NoError(t, err) - - e := &ErrorMiddleware{ - options: &Options{CommandPath: "azd provision"}, - console: mockinput.NewMockConsole(), - userConfigManager: userConfigManager, - } - - wantFix, err := e.promptForFix(t.Context()) - require.NoError(t, err) - require.True(t, wantFix, "saved 'allow' preference should return true") -} - -func TestPromptForFix_SavedNonAllow(t *testing.T) { - t.Parallel() - // If the saved value is not "allow", it should fall through to the - // interactive prompt. We can't mock uxlib.Select.Ask, so we test that - // the "allow" path works and that non-"allow" values don't auto-approve. - mockCtx := mocks.NewMockContext(t.Context()) - userConfigManager := config.NewUserConfigManager(mockCtx.ConfigManager) - cfg, err := userConfigManager.Load() - require.NoError(t, err) - err = cfg.Set(agentcopilot.ConfigKeyErrorHandlingFix, "deny") - require.NoError(t, err) - - // We verify the saved path is NOT taken by checking that "deny" doesn't - // auto-approve. The function will fall through to Ask(), which can't be - // tested without interactive input. This is a design limitation. -} - // --------------------------------------------------------------------------- // UxMiddleware.Run — azdext error with suggestion // --------------------------------------------------------------------------- @@ -1103,18 +1062,18 @@ func TestErrorMiddleware_Run_NullResultFromNext_NoError(t *testing.T) { } // --------------------------------------------------------------------------- -// fixableError — constant-style regression checks +// shouldSkipAgentHandling — constant-style regression checks // --------------------------------------------------------------------------- -func TestFixableError_ProjectErrNoDefaultService(t *testing.T) { +func TestShouldSkipAgentHandling_ProjectErrNoDefaultService(t *testing.T) { t.Parallel() - require.False(t, fixableError(project.ErrNoDefaultService)) + require.True(t, shouldSkipAgentHandling(project.ErrNoDefaultService)) } -func TestFixableError_WrappedGenericError(t *testing.T) { +func TestShouldSkipAgentHandling_WrappedGenericError(t *testing.T) { t.Parallel() err := fmt.Errorf("deploy failed: %w", fmt.Errorf("timeout")) - require.True(t, fixableError(err)) + require.False(t, shouldSkipAgentHandling(err)) } // --------------------------------------------------------------------------- @@ -1491,7 +1450,7 @@ func TestErrorMiddleware_Run_FixSendMessageError(t *testing.T) { return nil, errors.New("unexpected widget failure") }) - // The code should go through: category explain → SendMessage success → promptForFix "allow" → + // The code should go through: category explain → SendMessage success → promptNextAction "allow" → // fix SendMessage error. Verify we get the fix error. require.Error(t, err) require.Contains(t, err.Error(), "agent fix failed") @@ -1581,26 +1540,6 @@ func TestPromptTroubleshootCategory_ConfigLoadError(t *testing.T) { require.Equal(t, categorySkip, cat) } -// --------------------------------------------------------------------------- -// promptForFix — config load error -// --------------------------------------------------------------------------- - -func TestPromptForFix_ConfigLoadError(t *testing.T) { - t.Parallel() - console := mockinput.NewMockConsole() - ucm := &mockUserConfigManager{cfg: nil, err: errors.New("io error")} - - m := &ErrorMiddleware{ - console: console, - userConfigManager: ucm, - } - - fix, err := m.promptForFix(t.Context()) - require.Error(t, err) - require.Contains(t, err.Error(), "io error") - require.False(t, fix) -} - // --------------------------------------------------------------------------- // buildPromptForCategory — all categories // --------------------------------------------------------------------------- @@ -1639,7 +1578,8 @@ func TestBuildFixPrompt(t *testing.T) { } testErr := errors.New("resource group not found") - prompt := m.buildFixPrompt(testErr) + prompt, err := m.buildFixPrompt(testErr) + require.NoError(t, err) require.NotEmpty(t, prompt) require.Contains(t, prompt, "resource group not found") } @@ -1822,27 +1762,6 @@ func TestPromptTroubleshootCategory_AllSavedCategories(t *testing.T) { } } -// --------------------------------------------------------------------------- -// promptForFix — saved "allow" exercises lines 474-481 -// --------------------------------------------------------------------------- - -func TestPromptForFix_SavedAllow_MessageContent(t *testing.T) { - t.Parallel() - console := mockinput.NewMockConsole() - - cfg := configWithKeys(agentcopilot.ConfigKeyErrorHandlingFix, "allow") - ucm := &mockUserConfigManager{cfg: cfg} - - m := &ErrorMiddleware{ - console: console, - userConfigManager: ucm, - } - - wantFix, err := m.promptForFix(t.Context()) - require.NoError(t, err) - require.True(t, wantFix) -} - // --------------------------------------------------------------------------- // displayUsageMetrics — covers the TotalTokens() > 0 branch and == 0 branch // --------------------------------------------------------------------------- diff --git a/cli/azd/cmd/middleware/middleware_coverage_test.go b/cli/azd/cmd/middleware/middleware_coverage_test.go index 4d3a5c4555f..e42ca38e689 100644 --- a/cli/azd/cmd/middleware/middleware_coverage_test.go +++ b/cli/azd/cmd/middleware/middleware_coverage_test.go @@ -805,34 +805,34 @@ func TestAssignmentEndpoint_IsNotEmpty(t *testing.T) { } // --------------------------------------------------------------------------- -// shouldSkipErrorAnalysis — additional error types +// shouldSkipAgentHandling — control-flow and non-fixable error types // --------------------------------------------------------------------------- -func TestShouldSkipErrorAnalysis_ContextCanceled(t *testing.T) { +func TestShouldSkipAgentHandling_ContextCanceled(t *testing.T) { t.Parallel() - require.True(t, shouldSkipErrorAnalysis(context.Canceled)) + require.True(t, shouldSkipAgentHandling(context.Canceled)) } -func TestShouldSkipErrorAnalysis_AbortedByUser(t *testing.T) { +func TestShouldSkipAgentHandling_AbortedByUser(t *testing.T) { t.Parallel() - require.True(t, shouldSkipErrorAnalysis(internal.ErrAbortedByUser)) + require.True(t, shouldSkipAgentHandling(internal.ErrAbortedByUser)) } -func TestShouldSkipErrorAnalysis_RegularError(t *testing.T) { +func TestShouldSkipAgentHandling_RegularError(t *testing.T) { t.Parallel() - require.False(t, shouldSkipErrorAnalysis(errors.New("some regular error"))) + require.False(t, shouldSkipAgentHandling(errors.New("some regular error"))) } -func TestShouldSkipErrorAnalysis_WrappedCanceled(t *testing.T) { +func TestShouldSkipAgentHandling_WrappedCanceled(t *testing.T) { t.Parallel() err := fmt.Errorf("operation failed: %w", context.Canceled) - require.True(t, shouldSkipErrorAnalysis(err)) + require.True(t, shouldSkipAgentHandling(err)) } -func TestShouldSkipErrorAnalysis_NilError(t *testing.T) { +func TestShouldSkipAgentHandling_NilError(t *testing.T) { t.Parallel() // nil error should not be skipped — though callers check nil before calling - require.False(t, shouldSkipErrorAnalysis(nil)) + require.False(t, shouldSkipAgentHandling(nil)) } // --------------------------------------------------------------------------- @@ -946,7 +946,7 @@ func TestErrorMiddleware_Run_ChildAction_PassesError(t *testing.T) { } // --------------------------------------------------------------------------- -// ErrorMiddleware.Run — error with shouldSkipErrorAnalysis +// ErrorMiddleware.Run — error with shouldSkipAgentHandling // --------------------------------------------------------------------------- func TestErrorMiddleware_Run_SkippableError_NoPrompt(t *testing.T) { @@ -999,20 +999,19 @@ func TestErrorMiddleware_Run_ErrorWithSuggestion(t *testing.T) { } // --------------------------------------------------------------------------- -// fixableError — additional coverage +// shouldSkipAgentHandling — non-fixable error types // --------------------------------------------------------------------------- -func TestFixableError_RegularError(t *testing.T) { +func TestShouldSkipAgentHandling_RegularError_NotSkipped(t *testing.T) { t.Parallel() - result := fixableError(errors.New("some unknown error")) - require.True(t, result) + result := shouldSkipAgentHandling(errors.New("some unknown error")) + require.False(t, result) } -func TestFixableError_ContextCanceled(t *testing.T) { +func TestShouldSkipAgentHandling_ContextCanceled_Skipped(t *testing.T) { t.Parallel() - // context.Canceled is not a typed auth/tool error, so it falls through to - // the default fixable path. - result := fixableError(context.Canceled) + // context.Canceled is a control-flow error that should be skipped. + result := shouldSkipAgentHandling(context.Canceled) require.True(t, result) } @@ -1096,7 +1095,7 @@ func TestErrorMiddleware_Run_OnCI_ShortCircuits(t *testing.T) { } // --------------------------------------------------------------------------- -// ErrorMiddleware.Run — shouldSkipErrorAnalysis path +// ErrorMiddleware.Run — shouldSkipAgentHandling path // --------------------------------------------------------------------------- func TestErrorMiddleware_Run_SkippableError_CancelledInNonPrompt(t *testing.T) {