diff --git a/docs/users/features/hooks.md b/docs/users/features/hooks.md index 9aa3075b529..540b0dfce94 100644 --- a/docs/users/features/hooks.md +++ b/docs/users/features/hooks.md @@ -337,25 +337,25 @@ When `ok` is `false`, Qwen Code will continue working and use the `reason` as co Hooks fire at specific points during a Qwen Code session. Different events support different matchers to filter trigger conditions. -| Event | Triggered When | Matcher Target | -| :------------------- | :---------------------------------------------- | :------------------------------------------------------------- | -| `PreToolUse` | Before tool execution | Tool id (`write_file`, `read_file`, `run_shell_command`, etc.) | -| `PostToolUse` | After successful tool execution | Tool id | -| `PostToolUseFailure` | After tool execution fails | Tool id | -| `UserPromptSubmit` | Before supported model invocations | None | -| `SessionStart` | When session starts or resumes | Source (`startup`, `resume`, `clear`, `compact`) | -| `SessionEnd` | When session ends | Reason (`clear`, `logout`, `prompt_input_exit`, etc.) | -| `SessionDelete` | After an explicitly selected session is deleted | None | -| `MessageDisplay` | Repeatedly, as the reply streams | None (always fires) | -| `Stop` | When Claude prepares to conclude response | None (always fires) | -| `SubagentStart` | When subagent starts | Agent type (`Bash`, `Explorer`, `Plan`, etc.) | -| `SubagentStop` | When subagent stops | Agent type | -| `PreCompact` | Before conversation compaction | Trigger (`manual`, `auto`) | -| `Notification` | When notifications are sent | Type (`permission_prompt`, `idle_prompt`, `auth_success`) | -| `PermissionRequest` | When permission dialog is shown | Tool id | -| `PermissionDenied` | When tool permission is denied | Tool id | -| `TodoCreated` | When a new todo item is created | None (always fires) | -| `TodoCompleted` | When a todo item is marked as completed | None (always fires) | +| Event | Triggered When | Matcher Target | +| :------------------- | :----------------------------------------------- | :------------------------------------------------------------- | +| `PreToolUse` | Before tool execution | Tool id (`write_file`, `read_file`, `run_shell_command`, etc.) | +| `PostToolUse` | After successful tool execution | Tool id | +| `PostToolUseFailure` | After tool execution fails | Tool id | +| `UserPromptSubmit` | Before supported model invocations | None | +| `SessionStart` | When session starts or resumes | Source (`startup`, `resume`, `clear`, `compact`) | +| `SessionEnd` | When session ends | Reason (`clear`, `logout`, `prompt_input_exit`, etc.) | +| `SessionDelete` | After an explicitly selected session is deleted | None | +| `MessageDisplay` | Repeatedly, as the reply streams | None (always fires) | +| `Stop` | When Claude prepares to conclude response | None (always fires) | +| `SubagentStart` | When subagent starts | Agent type (`Bash`, `Explorer`, `Plan`, etc.) | +| `SubagentStop` | When subagent stops | Agent type | +| `PreCompact` | Before conversation compaction | Trigger (`manual`, `auto`) | +| `Notification` | When notifications are sent | Type (`permission_prompt`, `idle_prompt`, `auth_success`) | +| `PermissionRequest` | When permission dialog is shown | Tool id | +| `PermissionDenied` | When AUTO-mode classification denies a tool call | Tool id | +| `TodoCreated` | When a new todo item is created | None (always fires) | +| `TodoCompleted` | When a todo item is marked as completed | None (always fires) | ### Matcher Patterns diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 851c239d9b7..f462f3ed484 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -27531,6 +27531,203 @@ describe('Session', () => { ); }); + function configureAutoModeShellFallback(options: { + callId: string; + command: string; + denialState: core.AutoModeDenialState; + classifierResults?: Array>; + }) { + let denialState = options.denialState; + const generateJson = vi.fn(); + for (const result of options.classifierResults ?? []) { + generateJson.mockResolvedValueOnce(result); + } + const execute = vi.fn().mockResolvedValue({ + llmContent: 'ok', + returnDisplay: 'ok', + }); + const onConfirm = vi.fn().mockResolvedValue(undefined); + const invocation = { + params: { command: options.command }, + getDefaultPermission: vi.fn().mockResolvedValue('ask'), + getConfirmationDetails: vi.fn().mockResolvedValue({ + type: 'exec', + title: 'Need permission', + command: options.command, + rootCommand: 'python', + onConfirm, + }), + getDescription: vi.fn().mockReturnValue('Run command'), + toolLocations: vi.fn().mockReturnValue([]), + execute, + }; + mockToolRegistry.getTool.mockReturnValue({ + name: core.ToolNames.SHELL, + kind: core.Kind.Execute, + build: vi.fn().mockReturnValue(invocation), + }); + mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.AUTO); + mockConfig.getCwd = vi.fn().mockReturnValue('/repo'); + mockConfig.getPermissionManager = vi.fn().mockReturnValue(null); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(true); + mockConfig.getMessageBus = vi.fn().mockReturnValue(undefined); + mockConfig.getAutoModeSettings = vi.fn().mockReturnValue({}); + mockConfig.getBaseLlmClient = vi.fn().mockReturnValue({ generateJson }); + mockConfig.getAutoModeDenialState = vi + .fn() + .mockImplementation(() => denialState); + mockConfig.setAutoModeDenialState = vi + .fn() + .mockImplementation((next: core.AutoModeDenialState) => { + denialState = next; + }); + ( + mockLlmClient as unknown as { + getHistoryTail: ReturnType; + } + ).getHistoryTail = vi.fn().mockReturnValue([]); + vi.mocked(mockClient.requestPermission).mockResolvedValueOnce({ + outcome: { + outcome: 'selected', + optionId: core.ToolConfirmationOutcome.ProceedOnce, + }, + }); + mockChat.sendMessageStream = vi.fn().mockResolvedValue( + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { + functionCalls: [ + { + id: options.callId, + name: core.ToolNames.SHELL, + args: { command: options.command }, + }, + ], + }, + }, + ]), + ); + + return { + execute, + generateJson, + getDenialState: () => denialState, + onConfirm, + }; + } + + it('routes an exact ACP retry to manual approval without reclassifying it', async () => { + const command = 'python -c "print(1)"'; + const { execute, generateJson, getDenialState, onConfirm } = + configureAutoModeShellFallback({ + callId: 'call-exact-auto-retry', + command, + denialState: { + consecutiveBlock: 1, + consecutiveUnavailable: 0, + totalBlock: 1, + totalUnavailable: 0, + pendingManualRetryFingerprint: core.getAutoModeActionFingerprint( + core.ToolNames.SHELL, + { command }, + '/repo', + ), + }, + }); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'retry tool' }], + }); + + expect(generateJson).not.toHaveBeenCalled(); + expect(mockClient.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + toolCall: expect.objectContaining({ + content: expect.arrayContaining([ + expect.objectContaining({ + content: expect.objectContaining({ + text: expect.stringContaining('previously blocked'), + }), + }), + ]), + }), + }), + ); + const permissionRequest = vi.mocked(mockClient.requestPermission).mock + .calls[0][0]; + expect(permissionRequest.options).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ + optionId: + core.ToolConfirmationOutcome.ProceedOnceAndSwitchToDefault, + }), + ]), + ); + expect(onConfirm).toHaveBeenCalledWith( + core.ToolConfirmationOutcome.ProceedOnce, + { answers: undefined }, + ); + expect(execute).toHaveBeenCalledOnce(); + expect(getDenialState()).toEqual({ + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 1, + totalUnavailable: 0, + }); + }); + + it('routes the current ACP threshold block to manual approval', async () => { + const command = 'python -c "print(1)"'; + const { execute, generateJson, getDenialState } = + configureAutoModeShellFallback({ + callId: 'call-current-threshold', + command, + denialState: { + consecutiveBlock: 2, + consecutiveUnavailable: 0, + totalBlock: 2, + totalUnavailable: 0, + }, + classifierResults: [ + { shouldBlock: true }, + { + thinking: 'confirmed', + shouldBlock: true, + reason: 'unsafe command', + }, + ], + }); + + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run tool' }], + }); + + expect(generateJson).toHaveBeenCalledTimes(2); + expect(mockClient.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + toolCall: expect.objectContaining({ + content: expect.arrayContaining([ + expect.objectContaining({ + content: expect.objectContaining({ + text: expect.stringContaining('consecutive denial limit'), + }), + }), + ]), + }), + }), + ); + expect(execute).toHaveBeenCalledOnce(); + expect(getDenialState()).toEqual({ + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 3, + totalUnavailable: 0, + }); + }); + describe('in-session cron MessageDisplay', () => { /** Mock scheduler that delivers exactly one in-session job through `start`. */ function schedulerFiring(job: { prompt: string }) { diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 07e823fd9b2..7c17c149607 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -35,6 +35,7 @@ import type { ChatCompressionInfo, AutoModeDecision, AutoModeOutcome, + AutoModeFallbackConfirmation, GoalRecord, GoalRuntime, GoalSnapshotV2, @@ -133,16 +134,17 @@ import { getStopHookContinuationReason, formatStopHookBlockingCapWarning, applyAutoModeDecision, - decorateClassifierUnavailableConfirmation, + decorateAutoModeFallbackConfirmation, evaluateAutoMode, + getAutoModeActionFingerprint, getAutoModePermissionDeniedReason, + prepareAutoModeFallback, isApproveOutcome, isDenialFallbackReason, MAX_TRANSCRIPT_MESSAGES, formatDenialStateLog, recordAllow, recordFallbackApprove, - shouldFallback, shouldClassifyAllShellForAutoMode, finalizeToolResponses, shouldForceAutoModeReviewForAllow, @@ -11909,12 +11911,20 @@ export class Session implements SessionContext { !forceAutoReviewForAllow && !planShellRequiresConfirmation; if (autoModeAllowed && approvalMode === ApprovalMode.AUTO) { + const actionFingerprint = getAutoModeActionFingerprint( + policyToolName, + toolParams, + this.config.getCwd(), + ); this.config.setAutoModeDenialState( - recordAllow(this.config.getAutoModeDenialState()), + recordAllow( + this.config.getAutoModeDenialState(), + actionFingerprint, + ), ); } let wasAutoModeManualFallback = false; - let autoModeFallbackMessage: string | undefined; + let autoModeFallback: AutoModeFallbackConfirmation | undefined; // ── L5: AUTO mode three-layer filter (duplicated from // coreToolScheduler.ts; ACP routes through this Session path). @@ -11926,8 +11936,15 @@ export class Session implements SessionContext { !requiresUserInteraction && shouldRunAutoModeForCall(approvalMode, policyToolName) ) { - const denialState = this.config.getAutoModeDenialState(); - const fallback = shouldFallback(denialState); + const actionFingerprint = getAutoModeActionFingerprint( + policyToolName, + toolParams, + this.config.getCwd(), + ); + const { denialState, fallback } = prepareAutoModeFallback( + this.config, + actionFingerprint, + ); // `buildClassifierContents` retains only the most recent // MAX_TRANSCRIPT_MESSAGES messages; ask the chat client for // exactly that tail rather than triggering a `structuredClone` @@ -11961,6 +11978,7 @@ export class Session implements SessionContext { decision, this.config, denialState, + actionFingerprint, ); await fireSessionPermissionDeniedForAutoMode( this.config, @@ -12002,10 +12020,15 @@ export class Session implements SessionContext { outcome.reason === 'external_write'; if ( - outcome.reason === 'classifier_unavailable' || - outcome.reason === 'external_write' + outcome.message && + (outcome.reason === 'classifier_unavailable' || + outcome.reason === 'external_write' || + isDenialFallbackReason(outcome.reason)) ) { - autoModeFallbackMessage = outcome.message; + autoModeFallback = { + reason: outcome.reason, + message: outcome.message, + }; } if (wasAutoModeManualFallback) { @@ -12110,10 +12133,11 @@ export class Session implements SessionContext { return confirmationDetailsCancellation; } - if (autoModeFallbackMessage) { - confirmationDetails = decorateClassifierUnavailableConfirmation( + if (autoModeFallback && confirmationDetails) { + confirmationDetails = decorateAutoModeFallbackConfirmation( confirmationDetails, - autoModeFallbackMessage, + autoModeFallback.reason, + autoModeFallback.message, ); } diff --git a/packages/cli/src/acp-integration/session/permissionUtils.test.ts b/packages/cli/src/acp-integration/session/permissionUtils.test.ts index 71726b3f7d0..459272569ec 100644 --- a/packages/cli/src/acp-integration/session/permissionUtils.test.ts +++ b/packages/cli/src/acp-integration/session/permissionUtils.test.ts @@ -130,6 +130,51 @@ describe('permissionUtils', () => { ]); }); + it('offers switch-to-Default after consecutive classifier failures', () => { + const options = toPermissionOptions({ + type: 'exec', + title: 'Confirm Shell Command', + command: 'touch /tmp/marker', + rootCommand: 'touch', + autoModeFallback: { + reason: 'consecutive_unavailable', + message: 'Auto Mode could not classify consecutive actions.', + }, + onConfirm: async () => undefined, + }); + + expect(options).toContainEqual({ + optionId: ToolConfirmationOutcome.ProceedOnceAndSwitchToDefault, + name: 'Switch to Default Mode and allow once (recommended)', + kind: 'allow_once', + }); + }); + + it('keeps blocked retries in Auto Mode and hides persistent choices', () => { + const options = toPermissionOptions({ + type: 'exec', + title: 'Confirm Shell Command', + command: 'touch /tmp/marker', + rootCommand: 'touch', + autoModeFallback: { + reason: 'classifier_blocked_retry', + message: 'This exact action was previously blocked.', + }, + onConfirm: async () => undefined, + }); + + expect(options).toEqual([ + expect.objectContaining({ + optionId: ToolConfirmationOutcome.ProceedOnce, + kind: 'allow_once', + }), + expect.objectContaining({ + optionId: ToolConfirmationOutcome.Cancel, + kind: 'reject_once', + }), + ]); + }); + it('can hide project persistence while keeping user persistence', () => { const options = toPermissionOptions( { diff --git a/packages/cli/src/acp-integration/session/permissionUtils.ts b/packages/cli/src/acp-integration/session/permissionUtils.ts index 60be2d4a10b..d25489ef0f7 100644 --- a/packages/cli/src/acp-integration/session/permissionUtils.ts +++ b/packages/cli/src/acp-integration/session/permissionUtils.ts @@ -76,7 +76,12 @@ function filterAlwaysAllowOptions( const visibleOptions = hideAlwaysAllow ? options.filter((option) => option.kind !== 'allow_always') : options; - if (!confirmation.autoModeFallback) return visibleOptions; + if ( + confirmation.autoModeFallback?.reason !== 'classifier_unavailable' && + confirmation.autoModeFallback?.reason !== 'consecutive_unavailable' + ) { + return visibleOptions; + } const switchOption: PermissionOption = { optionId: ToolConfirmationOutcome.ProceedOnceAndSwitchToDefault, diff --git a/packages/cli/src/ui/components/hooks/constants.test.ts b/packages/cli/src/ui/components/hooks/constants.test.ts index ccc33960697..2d19c5afa37 100644 --- a/packages/cli/src/ui/components/hooks/constants.test.ts +++ b/packages/cli/src/ui/components/hooks/constants.test.ts @@ -191,9 +191,7 @@ describe('hooks constants', () => { it('should return description for PermissionDenied', () => { const desc = getHookShortDescription(HookEventName.PermissionDenied); - expect(desc).toBe( - 'When a tool call is denied before a permission dialog is displayed', - ); + expect(desc).toBe('When AUTO-mode classification denies a tool call'); }); it('should return empty string for unknown event', () => { @@ -388,7 +386,7 @@ describe('hooks constants', () => { expect(info.event).toBe(HookEventName.PermissionDenied); expect(info.shortDescription).toBe( - 'When a tool call is denied before a permission dialog is displayed', + 'When AUTO-mode classification denies a tool call', ); expect(info.description).toContain('tool_use_id'); expect(info.exitCodes).toHaveLength(2); diff --git a/packages/cli/src/ui/components/hooks/constants.ts b/packages/cli/src/ui/components/hooks/constants.ts index fd03b36dcc5..01ae8d3937c 100644 --- a/packages/cli/src/ui/components/hooks/constants.ts +++ b/packages/cli/src/ui/components/hooks/constants.ts @@ -211,7 +211,7 @@ export function getHookShortDescription(eventName: string): string { 'When a permission dialog is displayed', ), [HookEventName.PermissionDenied]: t( - 'When a tool call is denied before a permission dialog is displayed', + 'When AUTO-mode classification denies a tool call', ), [HookEventName.TodoCreated]: t('When a new todo item is created'), [HookEventName.TodoCompleted]: t('When a todo item is marked as completed'), diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index afc7975c342..eda35e16a5e 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -313,6 +313,64 @@ describe('ToolConfirmationMessage', () => { ); }); + it('renders blocked retry guidance without offering a mode switch', () => { + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'exec', + title: 'Confirm Shell Command', + command: 'touch /tmp/marker', + rootCommand: 'touch', + hideAlwaysAllow: true, + autoModeFallback: { + reason: 'classifier_blocked_retry', + message: 'This exact action was previously blocked.', + }, + onConfirm: vi.fn(), + }; + + const { lastFrame } = renderWithProviders( + , + ); + + const frame = lastFrame() ?? ''; + expect(frame).toContain('This exact action was previously blocked.'); + expect(frame).toContain('Yes, allow once'); + expect(frame).not.toContain('Switch to Default Mode'); + expect(frame).not.toContain('Always allow'); + }); + + it('offers a mode switch after consecutive classifier failures', () => { + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'exec', + title: 'Confirm Shell Command', + command: 'touch /tmp/marker', + rootCommand: 'touch', + hideAlwaysAllow: true, + autoModeFallback: { + reason: 'consecutive_unavailable', + message: 'Auto Mode could not classify consecutive actions.', + }, + onConfirm: vi.fn(), + }; + + const { lastFrame } = renderWithProviders( + , + ); + + expect(lastFrame() ?? '').toContain( + 'Switch to Default Mode and allow once (recommended)', + ); + }); + // Regression coverage for the round-1 review on PR #4386 (PR #4386 round-2 // self-review SR-1): the warnings block sits outside the MaxSizedBox // cap, so its footprint has to be reserved from `bodyContentHeight` @@ -969,6 +1027,40 @@ describe('ToolConfirmationMessage', () => { expect(frame).not.toContain('Allow always'); }); + it('budgets the two-option blocked retry layout on a tight terminal', () => { + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'exec', + title: 'Confirm Execution', + command: ['line-1', 'line-2', 'line-3', 'line-4'].join('\n'), + rootCommand: 'line-1', + hideAlwaysAllow: true, + autoModeFallback: { + reason: 'classifier_blocked_retry', + message: 'This exact action was previously blocked.', + }, + onConfirm: vi.fn(), + }; + + const { lastFrame } = renderWithProviders( + , + ); + + const frame = lastFrame() ?? ''; + expect(frame).toContain('previously blocked'); + expect(frame).toContain('line-1'); + expect(frame).toContain('last 2 lines hidden'); + expect(frame).toContain('Yes, allow once'); + expect(frame).toContain('No'); + expect(frame).not.toContain('Switch to Default Mode'); + expect(frame).not.toContain('Allow always'); + }); + it('renders the command and exec-specific question for exec confirmations', () => { const confirmationDetails: ToolCallConfirmationDetails = { type: 'exec', diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index a3915caad37..064e6f3be2e 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -62,6 +62,12 @@ export const ToolConfirmationMessage: React.FC< }) => { const { onConfirm } = confirmationDetails; const autoModeFallback = confirmationDetails.autoModeFallback; + const offersSwitchToDefault = + autoModeFallback?.reason === 'classifier_unavailable' || + autoModeFallback?.reason === 'consecutive_unavailable'; + const hidesAlwaysAllow = + 'hideAlwaysAllow' in confirmationDetails && + confirmationDetails.hideAlwaysAllow === true; const settings = useSettings(); const preferredEditor = settings.merged.general?.preferredEditor as @@ -177,15 +183,15 @@ export const ToolConfirmationMessage: React.FC< // Calculate the vertical space (in lines) consumed by UI elements // surrounding the main body content. Compact mode drops outer padding - // and inter-section margins, and renders a fixed 3-option list rather + // and inter-section margins, and renders a reduced option list rather // than the full options array. const PADDING_OUTER_Y = compactMode ? 0 : 2; const MARGIN_BODY_BOTTOM = compactMode ? 0 : 1; const HEIGHT_QUESTION = 1; const MARGIN_QUESTION_BOTTOM = compactMode ? 0 : 1; const HEIGHT_OPTIONS = compactMode - ? 3 - : options.length + (autoModeFallback ? 1 : 0); + ? 2 + (offersSwitchToDefault ? 1 : 0) + (hidesAlwaysAllow ? 0 : 1) + : options.length + (offersSwitchToDefault ? 1 : 0); const AUTO_MODE_FALLBACK_HEIGHT = autoModeFallback ? wrapAnsi(`⚠ ${autoModeFallback.message}`, warningContentWidth, { trim: false, @@ -670,7 +676,7 @@ export const ToolConfirmationMessage: React.FC< }); } - if (autoModeFallback) { + if (offersSwitchToDefault) { const cancelIndex = options.findIndex( (option) => option.value === ToolConfirmationOutcome.Cancel, ); @@ -684,6 +690,9 @@ export const ToolConfirmationMessage: React.FC< 0, switchOption, ); + } + + if (autoModeFallback) { bodyContent = ( @@ -718,7 +727,7 @@ export const ToolConfirmationMessage: React.FC< label: t('Yes, allow once'), value: ToolConfirmationOutcome.ProceedOnce, }, - ...(autoModeFallback + ...(offersSwitchToDefault ? [ { key: 'switch-default-and-proceed-once', diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 5b9d9f5e382..4c2a840ac09 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -29,6 +29,7 @@ import { BaseToolInvocation, Kind, ToolConfirmationOutcome, + getAutoModeActionFingerprint, DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES, DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD, MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS, @@ -816,6 +817,7 @@ describe('CoreToolScheduler', () => { consecutiveUnavailable: number; totalBlock: number; totalUnavailable: number; + pendingManualRetryFingerprint?: string; }; setAutoModeDenialState?: ReturnType; setApprovalMode?: ReturnType; @@ -841,6 +843,16 @@ describe('CoreToolScheduler', () => { findMatchingDenyRule: (ctx: unknown) => string | undefined; }; }) { + let autoModeDenialState = options.autoModeDenialState ?? { + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 0, + totalUnavailable: 0, + }; + const setAutoModeDenialState = (state: typeof autoModeDenialState) => { + autoModeDenialState = state; + options.setAutoModeDenialState?.(state); + }; const ensureTool = vi.fn( async (name: string) => options.toolsByName.get(name) as AnyDeclarativeTool, @@ -920,14 +932,8 @@ describe('CoreToolScheduler', () => { getDisableAllHooks: vi.fn( () => !(options.hooksEnabled?.() ?? !(options.disableHooks ?? true)), ), - getAutoModeDenialState: () => - options.autoModeDenialState ?? { - consecutiveBlock: 0, - consecutiveUnavailable: 0, - totalBlock: 0, - totalUnavailable: 0, - }, - setAutoModeDenialState: options.setAutoModeDenialState ?? vi.fn(), + getAutoModeDenialState: () => autoModeDenialState, + setAutoModeDenialState, getAutoModeSettings: () => ({}), getWorkspaceContext: () => ({ isPathWithinWorkspace: () => false, @@ -4116,6 +4122,293 @@ describe('CoreToolScheduler', () => { expect(toolSpan?.ended).toBe(true); }); + it('routes only an exact blocked-action retry to one manual confirmation', async () => { + runSideQueryMock + .mockResolvedValueOnce({ shouldBlock: true }) + .mockResolvedValueOnce({ + shouldBlock: true, + reason: 'dangerous shell command', + }) + .mockResolvedValueOnce({ shouldBlock: true }) + .mockResolvedValueOnce({ + shouldBlock: true, + reason: 'different dangerous shell command', + }); + const execute = vi.fn().mockResolvedValue({ + llmContent: 'executed', + returnDisplay: 'executed', + }); + const originalOnConfirm = vi.fn().mockResolvedValue(undefined); + const tool = new MockTool({ + name: ToolNames.SHELL, + getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, + getConfirmationDetails: async () => ({ + type: 'exec', + title: 'Confirm shell command', + command: 'dangerous command', + rootCommand: 'dangerous', + onConfirm: originalOnConfirm, + }), + execute, + }); + const { scheduler, onAllToolCallsComplete, onToolCallsUpdate } = + createSchedulerForLegacyToolTests({ + toolsByName: new Map([[tool.name, tool]]), + approvalMode: ApprovalMode.AUTO, + }); + const signal = new AbortController().signal; + const schedule = async (callId: string, command: string): Promise => { + await scheduler.schedule( + { + callId, + name: ToolNames.SHELL, + args: { command }, + isClientInitiated: false, + prompt_id: `prompt-${callId}`, + }, + signal, + ); + }; + + await schedule('blocked-a', 'dangerous-a'); + await vi.waitFor(() => expect(onAllToolCallsComplete).toHaveBeenCalled()); + expect(runSideQueryMock).toHaveBeenCalledTimes(2); + + onAllToolCallsComplete.mockClear(); + onToolCallsUpdate.mockClear(); + await schedule('blocked-b', 'dangerous-b'); + await vi.waitFor(() => expect(onAllToolCallsComplete).toHaveBeenCalled()); + expect(runSideQueryMock).toHaveBeenCalledTimes(4); + + onToolCallsUpdate.mockClear(); + await schedule('retry-b', 'dangerous-b'); + const waiting = (await waitForStatus( + onToolCallsUpdate, + 'awaiting_approval', + )) as WaitingToolCall; + expect(runSideQueryMock).toHaveBeenCalledTimes(4); + expect(waiting.confirmationDetails).toMatchObject({ + hideAlwaysAllow: true, + autoModeFallback: { + reason: 'classifier_blocked_retry', + message: expect.stringContaining('previously blocked'), + }, + }); + expect(execute).not.toHaveBeenCalled(); + + await waiting.confirmationDetails.onConfirm( + ToolConfirmationOutcome.ProceedOnce, + ); + await vi.waitFor(() => expect(execute).toHaveBeenCalledOnce()); + expect(originalOnConfirm).toHaveBeenCalledWith( + ToolConfirmationOutcome.ProceedOnce, + undefined, + ); + }); + + it('reclassifies an exact action after its one-shot retry is rejected', async () => { + runSideQueryMock + .mockResolvedValueOnce({ shouldBlock: true }) + .mockResolvedValueOnce({ + shouldBlock: true, + reason: 'dangerous shell command', + }) + .mockResolvedValueOnce({ shouldBlock: true }) + .mockResolvedValueOnce({ + shouldBlock: true, + reason: 'still dangerous', + }); + const originalOnConfirm = vi.fn().mockResolvedValue(undefined); + const tool = new MockTool({ + name: ToolNames.SHELL, + getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, + getConfirmationDetails: async () => ({ + type: 'exec', + title: 'Confirm shell command', + command: 'dangerous command', + rootCommand: 'dangerous', + onConfirm: originalOnConfirm, + }), + execute: vi.fn(), + }); + const { scheduler, onAllToolCallsComplete, onToolCallsUpdate } = + createSchedulerForLegacyToolTests({ + toolsByName: new Map([[tool.name, tool]]), + approvalMode: ApprovalMode.AUTO, + }); + const signal = new AbortController().signal; + const schedule = async (callId: string): Promise => { + await scheduler.schedule( + { + callId, + name: ToolNames.SHELL, + args: { command: 'dangerous command' }, + isClientInitiated: false, + prompt_id: `prompt-${callId}`, + }, + signal, + ); + }; + + await schedule('blocked-before-retry'); + await vi.waitFor(() => + expect(onAllToolCallsComplete).toHaveBeenCalledOnce(), + ); + expect(runSideQueryMock).toHaveBeenCalledTimes(2); + + onAllToolCallsComplete.mockClear(); + onToolCallsUpdate.mockClear(); + await schedule('rejected-retry'); + const waiting = (await waitForStatus( + onToolCallsUpdate, + 'awaiting_approval', + )) as WaitingToolCall; + expect(runSideQueryMock).toHaveBeenCalledTimes(2); + + await waiting.confirmationDetails.onConfirm(ToolConfirmationOutcome.Cancel); + await vi.waitFor(() => + expect(onAllToolCallsComplete).toHaveBeenCalledOnce(), + ); + + onAllToolCallsComplete.mockClear(); + onToolCallsUpdate.mockClear(); + await schedule('blocked-after-rejection'); + await vi.waitFor(() => + expect(onAllToolCallsComplete).toHaveBeenCalledOnce(), + ); + expect(runSideQueryMock).toHaveBeenCalledTimes(4); + const completedCall = onAllToolCallsComplete.mock + .calls[0][0][0] as CompletedToolCall; + expect(completedCall.status).toBe('error'); + expect(originalOnConfirm).toHaveBeenCalledWith( + ToolConfirmationOutcome.Cancel, + undefined, + ); + }); + + it.each([ + { + name: 'consecutive limit', + initialState: { + consecutiveBlock: 2, + consecutiveUnavailable: 0, + totalBlock: 2, + totalUnavailable: 0, + }, + reason: 'consecutive_block', + }, + { + name: 'total limit', + initialState: { + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 19, + totalUnavailable: 0, + }, + reason: 'total_denial', + }, + ])( + 'routes the current classifier block to manual confirmation at the $name', + async ({ initialState, reason }) => { + runSideQueryMock + .mockResolvedValueOnce({ shouldBlock: true }) + .mockResolvedValueOnce({ + shouldBlock: true, + reason: 'dangerous shell command', + }); + const hookSystem = { + firePermissionDeniedEvent: vi.fn().mockResolvedValue(undefined), + }; + const setAutoModeDenialState = vi.fn(); + const tool = new MockTool({ + name: ToolNames.SHELL, + getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, + getConfirmationDetails: MOCK_TOOL_GET_CONFIRMATION_DETAILS, + execute: vi.fn(), + }); + const { scheduler, onToolCallsUpdate } = + createSchedulerForLegacyToolTests({ + toolsByName: new Map([[tool.name, tool]]), + approvalMode: ApprovalMode.AUTO, + autoModeDenialState: initialState, + setAutoModeDenialState, + hookSystem, + disableHooks: false, + }); + + await scheduler.schedule( + { + callId: `threshold-${reason}`, + name: ToolNames.SHELL, + args: { command: 'dangerous command' }, + isClientInitiated: false, + prompt_id: `prompt-${reason}`, + }, + new AbortController().signal, + ); + + const waiting = (await waitForStatus( + onToolCallsUpdate, + 'awaiting_approval', + )) as WaitingToolCall; + expect(waiting.confirmationDetails).toMatchObject({ + autoModeFallback: { reason }, + }); + expect(hookSystem.firePermissionDeniedEvent).toHaveBeenCalledOnce(); + expect(setAutoModeDenialState).toHaveBeenLastCalledWith( + expect.objectContaining({ + totalBlock: initialState.totalBlock + 1, + }), + ); + }, + ); + + it('routes a consecutive classifier outage to manual confirmation without re-querying', async () => { + runSideQueryMock.mockReset(); + const hookSystem = { + firePermissionDeniedEvent: vi.fn().mockResolvedValue(undefined), + }; + const tool = new MockTool({ + name: ToolNames.SHELL, + getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, + getConfirmationDetails: MOCK_TOOL_GET_CONFIRMATION_DETAILS, + execute: vi.fn(), + }); + const { scheduler, onToolCallsUpdate } = createSchedulerForLegacyToolTests({ + toolsByName: new Map([[tool.name, tool]]), + approvalMode: ApprovalMode.AUTO, + autoModeDenialState: { + consecutiveBlock: 0, + consecutiveUnavailable: 2, + totalBlock: 0, + totalUnavailable: 2, + }, + hookSystem, + disableHooks: false, + }); + + await scheduler.schedule( + { + callId: 'consecutive-unavailable', + name: ToolNames.SHELL, + args: { command: 'dangerous command' }, + isClientInitiated: false, + prompt_id: 'prompt-consecutive-unavailable', + }, + new AbortController().signal, + ); + + const waiting = (await waitForStatus( + onToolCallsUpdate, + 'awaiting_approval', + )) as WaitingToolCall; + expect(waiting.confirmationDetails).toMatchObject({ + autoModeFallback: { reason: 'consecutive_unavailable' }, + }); + expect(runSideQueryMock).not.toHaveBeenCalled(); + expect(hookSystem.firePermissionDeniedEvent).not.toHaveBeenCalled(); + }); + it('marks invalid PermissionRequest rewrites as pre-execution span failures', async () => { const execute = vi.fn(); const onConfirm = vi.fn().mockResolvedValue(undefined); @@ -8769,6 +9062,7 @@ describe('CoreToolScheduler request queueing', () => { consecutiveUnavailable: number; totalBlock: number; totalUnavailable: number; + pendingManualRetryFingerprint?: string; }; function createPendingProtectedWriteHarness(options?: { @@ -9049,6 +9343,94 @@ describe('CoreToolScheduler request queueing', () => { ).autoModeFallbackCallIds.has('pending-protected-write'), ).toBe(true); }); + + it('routes an exact retry through manual approval during pending re-evaluation', async () => { + const command = "echo '{}' > .qwen/settings.json"; + runSideQueryMock.mockReset(); + const { scheduler, setAutoModeDenialState } = + createPendingProtectedWriteHarness({ + denialState: { + consecutiveBlock: 1, + consecutiveUnavailable: 0, + totalBlock: 1, + totalUnavailable: 0, + pendingManualRetryFingerprint: getAutoModeActionFingerprint( + ToolNames.SHELL, + { command }, + '/repo', + ), + }, + }); + + await ( + scheduler as unknown as { + autoApproveCompatiblePendingTools: ( + signal: AbortSignal, + triggeringCallId: string, + ) => Promise; + } + ).autoApproveCompatiblePendingTools( + new AbortController().signal, + 'approved-sibling', + ); + + expect(runSideQueryMock).not.toHaveBeenCalled(); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 1, + consecutiveUnavailable: 0, + totalBlock: 1, + totalUnavailable: 0, + }); + const toolCalls = (scheduler as unknown as { toolCalls: ToolCall[] }) + .toolCalls; + expect(toolCalls[0]).toMatchObject({ + status: 'awaiting_approval', + confirmationDetails: { + autoModeFallback: { reason: 'classifier_blocked_retry' }, + }, + }); + }); + + it('keeps the current threshold block pending for manual approval', async () => { + runSideQueryMock + .mockResolvedValueOnce({ shouldBlock: true }) + .mockResolvedValueOnce({ + shouldBlock: true, + reason: 'protected write', + thinking: 'confirmed', + }); + const { scheduler, hookSystem } = createPendingProtectedWriteHarness({ + denialState: { + consecutiveBlock: 2, + consecutiveUnavailable: 0, + totalBlock: 2, + totalUnavailable: 0, + }, + disableHooks: false, + }); + + await ( + scheduler as unknown as { + autoApproveCompatiblePendingTools: ( + signal: AbortSignal, + triggeringCallId: string, + ) => Promise; + } + ).autoApproveCompatiblePendingTools( + new AbortController().signal, + 'approved-sibling', + ); + + const toolCalls = (scheduler as unknown as { toolCalls: ToolCall[] }) + .toolCalls; + expect(toolCalls[0]).toMatchObject({ + status: 'awaiting_approval', + confirmationDetails: { + autoModeFallback: { reason: 'consecutive_block' }, + }, + }); + expect(hookSystem.firePermissionDeniedEvent).toHaveBeenCalledOnce(); + }); }); describe('CoreToolScheduler truncated output protection', () => { diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 2af601ce4f3..165464b52b5 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -10,6 +10,7 @@ import type { ToolExecutionStatus, } from './turn.js'; import type { + AutoModeFallbackConfirmation, ToolCallConfirmationDetails, ToolResult, ToolResultDisplay, @@ -116,9 +117,11 @@ import { } from './plan-mode-entry-policy.js'; import { applyAutoModeDecision, - decorateClassifierUnavailableConfirmation, + decorateAutoModeFallbackConfirmation, evaluateAutoMode, + getAutoModeActionFingerprint, getAutoModePermissionDeniedReason, + prepareAutoModeFallback, shouldClassifyAllShellForAutoMode, shouldForceAutoModeReviewForAllow, shouldFirePermissionDeniedForAutoMode, @@ -131,7 +134,6 @@ import { isDenialFallbackReason, recordAllow, recordFallbackApprove, - shouldFallback, } from '../permissions/denialTracking.js'; import { getResponseTextFromParts, @@ -2976,8 +2978,16 @@ export class CoreToolScheduler { // manual approval — confusing UX given the previous allow-rule // call just worked silently. if (approvalMode === ApprovalMode.AUTO) { + const actionFingerprint = getAutoModeActionFingerprint( + canonicalName, + toolParams, + this.config.getCwd(), + ); this.config.setAutoModeDenialState( - recordAllow(this.config.getAutoModeDenialState()), + recordAllow( + this.config.getAutoModeDenialState(), + actionFingerprint, + ), ); } this.setToolCallOutcome( @@ -2993,13 +3003,20 @@ export class CoreToolScheduler { // Grep, LS, in-cwd Edit, …) short-circuit even in a denial-streak // fallback state — otherwise every trivially safe tool would // force manual approval until the user toggles modes. - let autoModeFallbackMessage: string | undefined; + let autoModeFallback: AutoModeFallbackConfirmation | undefined; if ( !requiresUserInteraction && shouldRunAutoModeForCall(approvalMode, canonicalName) ) { - const denialState = this.config.getAutoModeDenialState(); - const fallback = shouldFallback(denialState); + const actionFingerprint = getAutoModeActionFingerprint( + canonicalName, + toolParams, + this.config.getCwd(), + ); + const { denialState, fallback } = prepareAutoModeFallback( + this.config, + actionFingerprint, + ); // `buildClassifierContents` retains only the most recent // MAX_TRANSCRIPT_MESSAGES messages; ask the chat client for // exactly that tail rather than triggering a @@ -3032,6 +3049,7 @@ export class CoreToolScheduler { decision, this.config, denialState, + actionFingerprint, ); if ( !this.config.getDisableAllHooks() && @@ -3097,18 +3115,28 @@ export class CoreToolScheduler { // operators see recovery fallbacks in the debug log. A // pmForcedAsk fallback isn't an audit-worthy event. if ( - isDenialFallbackReason(outcome.reason) || - outcome.reason === 'classifier_unavailable' + outcome.message && + (isDenialFallbackReason(outcome.reason) || + outcome.reason === 'classifier_unavailable') ) { this.autoModeFallbackCallIds.add(reqInfo.callId); - autoModeFallbackMessage = outcome.message; + autoModeFallback = { + reason: outcome.reason, + message: outcome.message, + }; debugLogger.warn( `Auto mode fallback to manual approval (${outcome.reason}): ` + formatDenialStateLog(denialState), ); - } else if (outcome.reason === 'external_write') { + } else if ( + outcome.reason === 'external_write' && + outcome.message + ) { this.autoModeFallbackCallIds.add(reqInfo.callId); - autoModeFallbackMessage = outcome.message; + autoModeFallback = { + reason: outcome.reason, + message: outcome.message, + }; debugLogger.warn( `Auto mode fallback to manual approval (external_write): Write attempted outside workspace.`, ); @@ -3152,10 +3180,11 @@ export class CoreToolScheduler { continue; } - if (autoModeFallbackMessage) { - confirmationDetails = decorateClassifierUnavailableConfirmation( + if (autoModeFallback) { + confirmationDetails = decorateAutoModeFallbackConfirmation( confirmationDetails, - autoModeFallbackMessage, + autoModeFallback.reason, + autoModeFallback.message, ); } @@ -6413,8 +6442,15 @@ export class CoreToolScheduler { debugLogger.info( `Auto mode: pending L4 allow overridden by protected-write guard or classifyAllShell for ${pendingTool.request.name}`, ); - const denialState = this.config.getAutoModeDenialState(); - const fallback = shouldFallback(denialState); + const actionFingerprint = getAutoModeActionFingerprint( + pendingTool.request.name, + toolParams, + this.config.getCwd(), + ); + const { denialState, fallback } = prepareAutoModeFallback( + this.config, + actionFingerprint, + ); const messages = this.config .getLlmClient?.() @@ -6444,6 +6480,7 @@ export class CoreToolScheduler { decision, this.config, denialState, + actionFingerprint, ); if ( !this.config.getDisableAllHooks() && @@ -6528,13 +6565,23 @@ export class CoreToolScheduler { ); } - if (outcome.message) { + if ( + outcome.message && + (isDenialFallbackReason(outcome.reason) || + outcome.reason === 'classifier_unavailable' || + outcome.reason === 'external_write') + ) { + const autoModeFallback: AutoModeFallbackConfirmation = { + reason: outcome.reason, + message: outcome.message, + }; this.setStatusInternal( pendingTool.request.callId, 'awaiting_approval', - decorateClassifierUnavailableConfirmation( + decorateAutoModeFallbackConfirmation( pendingTool.confirmationDetails, - outcome.message, + autoModeFallback.reason, + autoModeFallback.message, ), ); } diff --git a/packages/core/src/hooks/hookEventHandler.ts b/packages/core/src/hooks/hookEventHandler.ts index eb252be2664..d0cd42d0a4c 100644 --- a/packages/core/src/hooks/hookEventHandler.ts +++ b/packages/core/src/hooks/hookEventHandler.ts @@ -568,10 +568,9 @@ export class HookEventHandler { } /** - * Fire a PermissionDenied event for tool calls rejected before manual - * permission handling starts. Unlike PermissionRequest, this event does not - * ask hooks to approve or modify the call; it reports AUTO-mode denials that - * happen before any permission dialog would be shown. + * Fire a PermissionDenied event for AUTO-mode classifier denials. Unlike + * PermissionRequest, this event does not ask hooks to approve or modify the + * call. A threshold fallback may still show a manual dialog afterward. */ async firePermissionDeniedEvent( toolName: string, diff --git a/packages/core/src/hooks/types.ts b/packages/core/src/hooks/types.ts index ef9d97aa491..b662223683b 100644 --- a/packages/core/src/hooks/types.ts +++ b/packages/core/src/hooks/types.ts @@ -56,7 +56,7 @@ export enum HookEventName { SessionDelete = 'SessionDelete', // When a permission dialog is displayed PermissionRequest = 'PermissionRequest', - // When a tool call is denied before a permission dialog is displayed + // When AUTO-mode classification denies a tool call PermissionDenied = 'PermissionDenied', // StopFailure - When the turn ends due to an API error (instead of Stop) StopFailure = 'StopFailure', diff --git a/packages/core/src/permissions/autoMode.test.ts b/packages/core/src/permissions/autoMode.test.ts index dd8d5a7a8ec..e0f2c56db29 100644 --- a/packages/core/src/permissions/autoMode.test.ts +++ b/packages/core/src/permissions/autoMode.test.ts @@ -11,13 +11,16 @@ import path from 'node:path'; import { SAFE_TOOL_ALLOWLIST, applyAutoModeDecision, + decorateAutoModeFallbackConfirmation, decorateClassifierUnavailableConfirmation, evaluateAutoMode, formatClassifierBlockMessage, formatClassifierUnavailableFallbackMessage, + getAutoModeActionFingerprint, getAutoModePermissionDeniedReason, isAutoModeProtectedWritePath, isInSafeToolAllowlist, + prepareAutoModeFallback, shouldFirePermissionDeniedForAutoMode, passesAcceptEditsFastPath, shouldClassifyAllShellForAutoMode, @@ -1218,6 +1221,72 @@ describe('applyAutoModeDecision — blocked reason mapping', () => { }); }); + it('routes the block that reaches the consecutive limit to manual approval', () => { + const setAutoModeDenialState = vi.fn(); + const result = applyAutoModeDecision( + { + via: 'classifier', + shouldBlock: true, + reason: 'unsafe command', + unavailable: false, + stage: 'fast', + durationMs: 10, + }, + { setAutoModeDenialState } as unknown as Config, + { + consecutiveBlock: 2, + consecutiveUnavailable: 0, + totalBlock: 2, + totalUnavailable: 0, + }, + 'blocked-action', + ); + + expect(result).toMatchObject({ + kind: 'fallback', + reason: 'consecutive_block', + }); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 3, + consecutiveUnavailable: 0, + totalBlock: 3, + totalUnavailable: 0, + }); + }); + + it('routes the block that reaches the total limit to manual approval', () => { + const setAutoModeDenialState = vi.fn(); + const result = applyAutoModeDecision( + { + via: 'classifier', + shouldBlock: true, + reason: 'unsafe command', + unavailable: false, + stage: 'fast', + durationMs: 10, + }, + { setAutoModeDenialState } as unknown as Config, + { + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 19, + totalUnavailable: 0, + }, + 'blocked-action', + ); + + expect(result).toMatchObject({ + kind: 'fallback', + reason: 'total_denial', + }); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 1, + consecutiveUnavailable: 0, + totalBlock: 20, + totalUnavailable: 0, + }); + }); + it('routes classifier infrastructure failures to manual approval', () => { const setAutoModeDenialState = vi.fn(); const result = applyAutoModeDecision( @@ -1283,9 +1352,47 @@ describe('applyAutoModeDecision — blocked reason mapping', () => { denialState, ); - expect(result).toEqual({ kind: 'fallback', reason: 'consecutive_block' }); + expect(result).toMatchObject({ + kind: 'fallback', + reason: 'consecutive_block', + }); expect(setAutoModeDenialState).not.toHaveBeenCalled(); }); + + it('consumes a matching retry token when a threshold fallback takes precedence', () => { + const setAutoModeDenialState = vi.fn(); + const actionFingerprint = 'same-action'; + const result = applyAutoModeDecision( + { via: 'fallback', reason: 'consecutive_block' }, + { setAutoModeDenialState } as unknown as Config, + { + ...denialState, + consecutiveBlock: 3, + pendingManualRetryFingerprint: actionFingerprint, + }, + actionFingerprint, + ); + + expect(result).toMatchObject({ + kind: 'fallback', + reason: 'consecutive_block', + }); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + ...denialState, + consecutiveBlock: 3, + }); + }); +}); + +describe('getAutoModeActionFingerprint', () => { + it('matches canonical args only within the same working directory', () => { + expect(getAutoModeActionFingerprint('shell', { b: 2, a: 1 }, '/repo')).toBe( + getAutoModeActionFingerprint('shell', { a: 1, b: 2 }, '/repo'), + ); + expect(getAutoModeActionFingerprint('shell', { a: 1 }, '/repo')).not.toBe( + getAutoModeActionFingerprint('shell', { a: 1 }, '/other'), + ); + }); }); // ─── formatClassifierBlockMessage ──────────────────────────────────────── @@ -1309,7 +1416,7 @@ describe('formatClassifierBlockMessage', () => { unavailable: false, }), ).toBe( - 'Blocked by auto mode policy: Irreversible filesystem destruction\nDo not try to complete the denied action through another tool, shell indirection, generated script, alias, symlink, config change, hook, command file, MCP configuration, encoded payload, or equivalent path. If that action is required, stop and ask the user for explicit approval. You may continue with unrelated safe work or a genuinely safer alternative that does not accomplish the denied action.', + 'Blocked by auto mode policy: Irreversible filesystem destruction\nDo not try to complete the denied action through another tool, shell indirection, generated script, alias, symlink, config change, hook, command file, MCP configuration, encoded payload, or equivalent path. To request manual approval for this exact action, retry the same tool call without changing its arguments. You may continue with unrelated safe work or a genuinely safer alternative that does not accomplish the denied action.', ); }); }); @@ -1335,7 +1442,7 @@ describe('classifier unavailable confirmation', () => { }); it('decorates the confirmation and suppresses persistent approval', () => { - const confirmation = decorateClassifierUnavailableConfirmation( + const confirmation = decorateAutoModeFallbackConfirmation( { type: 'exec', title: 'Run command', @@ -1343,6 +1450,7 @@ describe('classifier unavailable confirmation', () => { rootCommand: 'touch', onConfirm: vi.fn(), }, + 'classifier_unavailable', 'Classifier unavailable.', ); @@ -1354,6 +1462,22 @@ describe('classifier unavailable confirmation', () => { }, }); }); + + it('keeps the classifier-unavailable decorator compatible', () => { + const confirmation = decorateClassifierUnavailableConfirmation( + { + type: 'info', + title: 'Run tool', + prompt: 'Run?', + onConfirm: vi.fn(), + }, + 'Classifier unavailable.', + ); + + expect(confirmation.autoModeFallback?.reason).toBe( + 'classifier_unavailable', + ); + }); }); // ─── PermissionDenied hook gating ──────────────────────────────────────── @@ -1368,7 +1492,7 @@ describe('PermissionDenied hook gating', () => { durationMs: 20, }; - it('fires only for classifier blocks that produce a blocked outcome', () => { + it('fires for classifier policy blocks, including threshold fallbacks', () => { expect( shouldFirePermissionDeniedForAutoMode(classifierBlock, { kind: 'blocked', @@ -1395,6 +1519,37 @@ describe('PermissionDenied hook gating', () => { ), ).toBe(false); + expect( + shouldFirePermissionDeniedForAutoMode(classifierBlock, { + kind: 'fallback', + reason: 'consecutive_block', + message: 'Review manually.', + }), + ).toBe(true); + + expect( + shouldFirePermissionDeniedForAutoMode(classifierBlock, { + kind: 'fallback', + reason: 'total_denial', + message: 'Review manually.', + }), + ).toBe(true); + + expect( + shouldFirePermissionDeniedForAutoMode(classifierBlock, { + kind: 'fallback', + reason: 'classifier_blocked_retry', + message: 'Review manually.', + }), + ).toBe(false); + + expect( + shouldFirePermissionDeniedForAutoMode(classifierBlock, { + kind: 'fallback', + reason: 'safety_check', + }), + ).toBe(false); + expect( shouldFirePermissionDeniedForAutoMode( { via: 'fallback', reason: 'safety_check' }, @@ -1600,6 +1755,50 @@ describe('evaluateAutoMode — L5.2.5 destructive command guard', () => { expect(decision.via).not.toBe('blocked:destructive-command'); }); + it('preserves an armed retry when the destructive guard preempts it', async () => { + const actionFingerprint = getAutoModeActionFingerprint( + ToolNames.SHELL, + { command: 'git reset --hard' }, + cwd, + ); + let denialState = { + consecutiveBlock: 1, + consecutiveUnavailable: 0, + totalBlock: 1, + totalUnavailable: 0, + pendingManualRetryFingerprint: actionFingerprint, + }; + const config = { + ...baseConfig, + getAutoModeDenialState: () => denialState, + setAutoModeDenialState: (next: typeof denialState) => { + denialState = next; + }, + } as unknown as Config; + const prepared = prepareAutoModeFallback(config, actionFingerprint); + + const decision = await evaluateAutoMode({ + ctx: { toolName: ToolNames.SHELL, command: 'git reset --hard' }, + pmForcedAsk: false, + toolParams: { command: 'git reset --hard' }, + messages: [{ role: 'user', parts: [{ text: 'fix the bug' }] }], + config, + signal: new AbortController().signal, + skipClassifierReason: prepared.fallback.fallback + ? prepared.fallback.reason + : undefined, + }); + const outcome = applyAutoModeDecision( + decision, + config, + prepared.denialState, + actionFingerprint, + ); + + expect(outcome.kind).toBe('blocked'); + expect(denialState.pendingManualRetryFingerprint).toBe(actionFingerprint); + }); + it('does not block non-shell tools', async () => { const decision = await evaluateAutoMode({ ctx: { toolName: ToolNames.READ_FILE, filePath: '/any/file.txt' }, @@ -1647,6 +1846,10 @@ describe('evaluateAutoMode — L5.2.5 destructive command guard', () => { if (result.kind === 'blocked') { expect(result.errorMessage).toContain('Blocked destructive git command'); expect(result.errorMessage).toContain('Do not try to complete'); + expect(result.errorMessage).not.toContain('retry the same tool call'); + expect(result.errorMessage).toContain( + 'ask the user for explicit approval', + ); } expect(setAutoModeDenialState).toHaveBeenCalled(); }); diff --git a/packages/core/src/permissions/autoMode.ts b/packages/core/src/permissions/autoMode.ts index 20d8d0c26ec..03d784ac143 100644 --- a/packages/core/src/permissions/autoMode.ts +++ b/packages/core/src/permissions/autoMode.ts @@ -28,15 +28,22 @@ import { import type { PermissionDeniedReason } from '../hooks/types.js'; export type { PermissionDeniedReason } from '../hooks/types.js'; import { ToolNames } from '../tools/tool-names.js'; -import type { ToolCallConfirmationDetails } from '../tools/tools.js'; +import { getToolCallRepeatKey } from '../tools/tool-call-repeat-key.js'; +import type { + AutoModeFallbackConfirmation, + ToolCallConfirmationDetails, +} from '../tools/tools.js'; import { normalizeMonitorCommand } from '../utils/shell-utils.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { classifyAction, type ClassifierResult } from './classifier.js'; import { extractShellOperationsAcrossCommand } from './shell-semantics.js'; import { + consumePendingManualRetry, + isDenialFallbackReason, recordAllow, recordBlock, recordUnavailable, + shouldFallback, type AutoModeDenialState, type DenialFallbackReason, } from './denialTracking.js'; @@ -537,6 +544,28 @@ export type AutoModeOutcome = message?: string; }; +/** Stable identity for an AUTO-mode action within its filesystem context. */ +export function getAutoModeActionFingerprint( + toolName: string, + toolParams: Record, + cwd: string, +): string { + return getToolCallRepeatKey(toolName, { cwd, toolParams }); +} + +/** Resolve a fallback without consuming its one-shot retry yet. */ +export function prepareAutoModeFallback( + config: Config, + actionFingerprint: string, +): { + denialState: AutoModeDenialState; + fallback: ReturnType; +} { + const denialState = config.getAutoModeDenialState(); + const fallback = shouldFallback(denialState, actionFingerprint); + return { denialState, fallback }; +} + /** * Apply an AUTO decision and denial-tracking update. Shared by the scheduler * and ACP paths; callers still handle their integration-specific responses. @@ -545,17 +574,20 @@ export function applyAutoModeDecision( decision: AutoModeDecision, config: Config, denialState: AutoModeDenialState, + actionFingerprint?: string, ): AutoModeOutcome { switch (decision.via) { case 'fast-path:accept-edits': case 'fast-path:allowlist': - config.setAutoModeDenialState(recordAllow(denialState)); + config.setAutoModeDenialState( + recordAllow(denialState, actionFingerprint), + ); return { kind: 'approved' }; case 'blocked:destructive-command': config.setAutoModeDenialState(recordBlock(denialState)); return { kind: 'blocked', - errorMessage: `${decision.reason}\n${AUTO_MODE_DENIAL_GUIDANCE}`, + errorMessage: `${decision.reason}\n${AUTO_MODE_DESTRUCTIVE_DENIAL_GUIDANCE}`, reason: 'classifier_blocked', }; case 'classifier': @@ -568,16 +600,39 @@ export function applyAutoModeDecision( message: formatClassifierUnavailableFallbackMessage(decision), }; } - config.setAutoModeDenialState(recordBlock(denialState)); + const blockedState = recordBlock(denialState, actionFingerprint); + const fallback = shouldFallback(blockedState); + if (fallback.fallback) { + config.setAutoModeDenialState( + consumePendingManualRetry(blockedState), + ); + return { + kind: 'fallback', + reason: fallback.reason, + message: formatDenialFallbackMessage( + fallback.reason, + decision.reason, + ), + }; + } + config.setAutoModeDenialState(blockedState); return { kind: 'blocked', errorMessage: formatClassifierBlockMessage(decision), reason: 'classifier_blocked', }; } - config.setAutoModeDenialState(recordAllow(denialState)); + config.setAutoModeDenialState( + recordAllow(denialState, actionFingerprint), + ); return { kind: 'approved' }; case 'fallback': + if ( + actionFingerprint !== undefined && + denialState.pendingManualRetryFingerprint === actionFingerprint + ) { + config.setAutoModeDenialState(consumePendingManualRetry(denialState)); + } if (decision.reason === 'external_write') { return { kind: 'fallback', @@ -586,7 +641,13 @@ export function applyAutoModeDecision( 'Writes outside the workspace require manual approval in AUTO mode.', }; } - return { kind: 'fallback', reason: decision.reason }; + return { + kind: 'fallback', + reason: decision.reason, + ...(isDenialFallbackReason(decision.reason) + ? { message: formatDenialFallbackMessage(decision.reason) } + : {}), + }; default: { const _exhaustive: never = decision; // Make unexpected JS/interop values visible at runtime. @@ -608,7 +669,11 @@ export function shouldFirePermissionDeniedForAutoMode( return ( decision.via === 'classifier' && decision.shouldBlock && - outcome.kind === 'blocked' + !decision.unavailable && + (outcome.kind === 'blocked' || + (outcome.kind === 'fallback' && + (outcome.reason === 'consecutive_block' || + outcome.reason === 'total_denial'))) ); } @@ -620,12 +685,35 @@ export function getAutoModePermissionDeniedReason( /** * Trailing guidance appended to classifier policy-denial tool results. - * Centralised so the policy boundary (no silent retries, no equivalent-path - * workarounds, stop and ask the user) stays in sync with the main system - * prompt's Denied Tool Calls rule. + * Centralised so the policy boundary (no equivalent-path workarounds, with an + * exact-action retry available for manual review) stays in sync with the main + * system prompt's Denied Tool Calls rule. */ export const AUTO_MODE_DENIAL_GUIDANCE = - 'Do not try to complete the denied action through another tool, shell indirection, generated script, alias, symlink, config change, hook, command file, MCP configuration, encoded payload, or equivalent path. If that action is required, stop and ask the user for explicit approval. You may continue with unrelated safe work or a genuinely safer alternative that does not accomplish the denied action.'; + 'Do not try to complete the denied action through another tool, shell indirection, generated script, alias, symlink, config change, hook, command file, MCP configuration, encoded payload, or equivalent path. To request manual approval for this exact action, retry the same tool call without changing its arguments. You may continue with unrelated safe work or a genuinely safer alternative that does not accomplish the denied action.'; + +const AUTO_MODE_DESTRUCTIVE_DENIAL_GUIDANCE = + 'Do not try to complete the denied action through another tool, shell indirection, generated script, alias, symlink, config change, hook, command file, MCP configuration, encoded payload, or equivalent path. If this action is required, stop and ask the user for explicit approval. You may continue with unrelated safe work or a genuinely safer alternative that does not accomplish the denied action.'; + +function formatDenialFallbackMessage( + reason: DenialFallbackReason, + classifierReason?: string, +): string { + switch (reason) { + case 'classifier_blocked_retry': + return 'Auto mode previously blocked this exact action. Review it manually.'; + case 'consecutive_block': + return `Auto mode reached its consecutive denial limit on this action${classifierReason ? ` (${classifierReason})` : ''}. Review it manually.`; + case 'consecutive_unavailable': + return 'Auto mode could not classify consecutive actions. Review this action manually.'; + case 'total_denial': + return 'Auto mode reached its session denial limit. Review this action manually.'; + default: { + const _exhaustive: never = reason; + return _exhaustive; + } + } +} export function formatClassifierUnavailableFallbackMessage( decision: Extract, @@ -634,8 +722,9 @@ export function formatClassifierUnavailableFallbackMessage( return `Auto Mode couldn't classify this action${detail}. Review it manually. Switching to Default Mode is recommended if you want to continue without the classifier.`; } -export function decorateClassifierUnavailableConfirmation( +export function decorateAutoModeFallbackConfirmation( confirmation: ToolCallConfirmationDetails, + reason: AutoModeFallbackConfirmation['reason'], message: string, ): ToolCallConfirmationDetails { return { @@ -644,12 +733,23 @@ export function decorateClassifierUnavailableConfirmation( ? {} : { hideAlwaysAllow: true }), autoModeFallback: { - reason: 'classifier_unavailable', + reason, message, }, } as ToolCallConfirmationDetails; } +export function decorateClassifierUnavailableConfirmation( + confirmation: ToolCallConfirmationDetails, + message: string, +): ToolCallConfirmationDetails { + return decorateAutoModeFallbackConfirmation( + confirmation, + 'classifier_unavailable', + message, + ); +} + /** * Build the tool-error message the scheduler / ACP session returns when the * classifier supplies a policy block. Keeping it here gives both paths the diff --git a/packages/core/src/permissions/denialTracking.test.ts b/packages/core/src/permissions/denialTracking.test.ts index d635d621e9e..a58d5258a8f 100644 --- a/packages/core/src/permissions/denialTracking.test.ts +++ b/packages/core/src/permissions/denialTracking.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect } from 'vitest'; import { AUTO_MODE_DENIAL_LIMITS, + consumePendingManualRetry, createDenialState, formatDenialStateLog, isApproveOutcome, @@ -52,6 +53,7 @@ describe('formatDenialStateLog', () => { describe('isDenialFallbackReason', () => { it('accepts denial-tracking fallback reasons', () => { expect(isDenialFallbackReason('consecutive_block')).toBe(true); + expect(isDenialFallbackReason('classifier_blocked_retry')).toBe(true); expect(isDenialFallbackReason('consecutive_unavailable')).toBe(true); expect(isDenialFallbackReason('total_denial')).toBe(true); }); @@ -79,6 +81,12 @@ describe('recordBlock', () => { // Total counters are independent. expect(s.totalUnavailable).toBe(1); }); + + it('records only a digest for the exact action eligible for manual retry', () => { + expect(recordBlock(FRESH, 'action-digest')).toMatchObject({ + pendingManualRetryFingerprint: 'action-digest', + }); + }); }); describe('recordUnavailable', () => { @@ -135,6 +143,60 @@ describe('shouldFallback', () => { }); }); + it('routes only an exact retry of the last blocked action to manual approval', () => { + const blocked = recordBlock(FRESH, 'blocked-action'); + + expect(shouldFallback(blocked, 'blocked-action')).toEqual({ + fallback: true, + reason: 'classifier_blocked_retry', + }); + expect(shouldFallback(blocked, 'changed-action')).toEqual({ + fallback: false, + }); + }); + + it('consumes the exact-action retry before manual confirmation', () => { + const blocked = recordBlock(FRESH, 'blocked-action'); + const consumed = consumePendingManualRetry(blocked); + + expect(consumed.pendingManualRetryFingerprint).toBeUndefined(); + expect(shouldFallback(consumed, 'blocked-action')).toEqual({ + fallback: false, + }); + }); + + it('preserves an exact-action retry across unrelated allowed work', () => { + const blocked = recordBlock(FRESH, 'blocked-action'); + const allowed = recordAllow(blocked, 'allowed-action'); + + expect(shouldFallback(allowed, 'blocked-action')).toEqual({ + fallback: true, + reason: 'classifier_blocked_retry', + }); + }); + + it('clears the retry when that exact action is allowed', () => { + const blocked = recordBlock(FRESH, 'blocked-action'); + const allowed = recordAllow(blocked, 'blocked-action'); + + expect(shouldFallback(allowed, 'blocked-action')).toEqual({ + fallback: false, + }); + }); + + it.each([ + ['a fingerprint-less block', recordBlock], + ['classifier unavailability', recordUnavailable], + ['an unrelated fallback approval', recordFallbackApprove], + ])('preserves an exact-action retry across %s', (_name, transition) => { + const blocked = recordBlock(FRESH, 'blocked-action'); + + expect(shouldFallback(transition(blocked), 'blocked-action')).toEqual({ + fallback: true, + reason: 'classifier_blocked_retry', + }); + }); + it('triggers fallback after 2 consecutive unavailable', () => { let s: AutoModeDenialState = FRESH; s = recordUnavailable(s); @@ -235,6 +297,21 @@ describe('recordFallbackApprove', () => { expect(shouldFallback(s)).toEqual({ fallback: false }); }); + it('preserves an unrelated retry when resetting the total denial cap', () => { + const state = { + ...FRESH, + totalBlock: AUTO_MODE_DENIAL_LIMITS.maxTotalDenials, + pendingManualRetryFingerprint: 'blocked-action', + }; + + expect( + shouldFallback(recordFallbackApprove(state), 'blocked-action'), + ).toEqual({ + fallback: true, + reason: 'classifier_blocked_retry', + }); + }); + it('resets all counters when total and consecutive caps overlap', () => { const s: AutoModeDenialState = { consecutiveBlock: AUTO_MODE_DENIAL_LIMITS.maxConsecutiveBlock, @@ -257,7 +334,7 @@ describe('resetDenialState', () => { let s: AutoModeDenialState = FRESH; s = recordBlock(s); s = recordUnavailable(s); - s = recordBlock(s); + s = recordBlock(s, 'blocked-action'); s = resetDenialState(); expect(s).toEqual(FRESH); }); diff --git a/packages/core/src/permissions/denialTracking.ts b/packages/core/src/permissions/denialTracking.ts index 3a6361b7b9c..e9c392c2810 100644 --- a/packages/core/src/permissions/denialTracking.ts +++ b/packages/core/src/permissions/denialTracking.ts @@ -7,10 +7,10 @@ * * Protects users from infinite loops when the classifier persistently blocks * (LLM stuck in a dead-end) or persistently fails (infrastructure problem). - * After either the consecutive thresholds or the total denial cap are - * exceeded, the orchestrator falls back to DEFAULT-mode confirmation flow - * for the next tool call. The session itself stays in AUTO; only the single - * offending call is downgraded. + * The exact blocked action may be retried once through DEFAULT-mode + * confirmation. Calls that reach a consecutive threshold or the total denial + * cap also fall back on that same call. The session itself stays in AUTO; only + * the single offending call is downgraded. * * Block and unavailable counters cross-reset: they represent different * failure modes and should not accumulate together. Switching ApprovalMode @@ -19,10 +19,13 @@ * `total*` counters are cumulative for the session and trigger a total * denial cap even when the model avoids consecutive-cap thresholds by * alternating blocks, unavailable verdicts, and allowed calls. + * Only one pending retry fingerprint is stored; a later classifier block for + * another action replaces it. */ /** Reasons the orchestrator may choose to fall back to manual approval. */ export type DenialFallbackReason = + | 'classifier_blocked_retry' | 'consecutive_block' | 'consecutive_unavailable' | 'total_denial'; @@ -32,6 +35,7 @@ export interface AutoModeDenialState { consecutiveUnavailable: number; totalBlock: number; totalUnavailable: number; + pendingManualRetryFingerprint?: string; } export const AUTO_MODE_DENIAL_LIMITS = { @@ -73,33 +77,53 @@ export function isDenialFallbackReason( ): reason is DenialFallbackReason { return ( reason === 'consecutive_block' || + reason === 'classifier_blocked_retry' || reason === 'consecutive_unavailable' || reason === 'total_denial' ); } /** Record a successful (allow) decision. Resets both consecutive counters. */ -export function recordAllow(state: AutoModeDenialState): AutoModeDenialState { - if (state.consecutiveBlock === 0 && state.consecutiveUnavailable === 0) { +export function recordAllow( + state: AutoModeDenialState, + actionFingerprint?: string, +): AutoModeDenialState { + const clearsPendingRetry = + actionFingerprint !== undefined && + state.pendingManualRetryFingerprint === actionFingerprint; + if ( + state.consecutiveBlock === 0 && + state.consecutiveUnavailable === 0 && + !clearsPendingRetry + ) { return state; // no-op } - return { + const next = { ...state, consecutiveBlock: 0, consecutiveUnavailable: 0, }; + if (clearsPendingRetry) delete next.pendingManualRetryFingerprint; + return next; } /** * Record a classifier-policy block. Increments `consecutiveBlock` and * `totalBlock`; cross-resets `consecutiveUnavailable`. */ -export function recordBlock(state: AutoModeDenialState): AutoModeDenialState { +export function recordBlock( + state: AutoModeDenialState, + actionFingerprint?: string, +): AutoModeDenialState { return { + ...state, consecutiveBlock: state.consecutiveBlock + 1, consecutiveUnavailable: 0, totalBlock: state.totalBlock + 1, totalUnavailable: state.totalUnavailable, + ...(actionFingerprint + ? { pendingManualRetryFingerprint: actionFingerprint } + : {}), }; } @@ -112,6 +136,7 @@ export function recordUnavailable( state: AutoModeDenialState, ): AutoModeDenialState { return { + ...state, consecutiveBlock: 0, consecutiveUnavailable: state.consecutiveUnavailable + 1, totalBlock: state.totalBlock, @@ -120,13 +145,14 @@ export function recordUnavailable( } /** - * Decide whether the next tool call should bypass the classifier and fall - * back to DEFAULT-mode confirmation. The fallback applies to a single call - * only; the session remains in AUTO. The total denial cap takes precedence - * over consecutive caps so alternating denial modes cannot avoid fallback. + * Decide whether a tool call should bypass the classifier and fall back to + * DEFAULT-mode confirmation. The fallback applies to a single call only; the + * session remains in AUTO. The total denial cap takes precedence over + * consecutive caps so alternating denial modes cannot avoid fallback. */ export function shouldFallback( state: AutoModeDenialState, + actionFingerprint?: string, ): { fallback: true; reason: DenialFallbackReason } | { fallback: false } { if (hasReachedTotalCap(state)) { return { fallback: true, reason: 'total_denial' }; @@ -140,22 +166,37 @@ export function shouldFallback( ) { return { fallback: true, reason: 'consecutive_unavailable' }; } + if ( + actionFingerprint !== undefined && + state.pendingManualRetryFingerprint === actionFingerprint + ) { + return { fallback: true, reason: 'classifier_blocked_retry' }; + } return { fallback: false }; } +/** Consume the one-shot manual retry before its confirmation is displayed. */ +export function consumePendingManualRetry( + state: AutoModeDenialState, +): AutoModeDenialState { + if (state.pendingManualRetryFingerprint === undefined) return state; + const next = { ...state }; + delete next.pendingManualRetryFingerprint; + return next; +} + /** * Called after the user manually approves a fallback-prompted tool call. * Resets BOTH consecutive counters so the agent can resume normal AUTO flow. * If the total denial cap was reached, also clears total counters so the * session does not stay permanently pinned to manual prompts. * - * Symmetric with `recordAllow`: a manual approval signals the user accepted - * the action, and the next call should re-engage the classifier. If the - * classifier or its infrastructure is still degraded, the next call's - * verdict will simply re-arm the appropriate counter (one block / one - * unavailable) — same recovery curve as initial onset, no permanent - * lock-out. Resetting only `consecutiveBlock` (the original v1 behaviour) - * created an asymmetry: a transient API blip past + * A manual approval signals the user accepted the action, and the next call + * should re-engage the classifier. If the classifier or its infrastructure is + * still degraded, the next call's verdict will simply re-arm the appropriate + * counter (one block / one unavailable) — same recovery curve as initial + * onset, no permanent lock-out. Resetting only `consecutiveBlock` (the + * original v1 behaviour) created an asymmetry: a transient API blip past * `maxConsecutiveUnavailable` would permanently downgrade the rest of the * session to manual approval even after the user approved the fallback * prompt, until ApprovalMode toggled. @@ -164,7 +205,13 @@ export function recordFallbackApprove( state: AutoModeDenialState, ): AutoModeDenialState { if (hasReachedTotalCap(state)) { - return createDenialState(); + return { + ...state, + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 0, + totalUnavailable: 0, + }; } if (state.consecutiveBlock === 0 && state.consecutiveUnavailable === 0) { return state; diff --git a/packages/core/src/permissions/index.ts b/packages/core/src/permissions/index.ts index a13c72ed9dc..c40ff3d4707 100644 --- a/packages/core/src/permissions/index.ts +++ b/packages/core/src/permissions/index.ts @@ -12,8 +12,11 @@ export { extractShellOperations } from './shell-semantics.js'; export type { ShellOperation } from './shell-semantics.js'; export { applyAutoModeDecision, + decorateAutoModeFallbackConfirmation, decorateClassifierUnavailableConfirmation, evaluateAutoMode, + getAutoModeActionFingerprint, + prepareAutoModeFallback, formatClassifierBlockMessage, formatClassifierUnavailableFallbackMessage, type AutoModeUnavailableReason, diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 0f2766e7693..8447c40c314 100644 --- a/packages/core/src/tools/tools.ts +++ b/packages/core/src/tools/tools.ts @@ -1016,7 +1016,13 @@ export interface ToolInfoConfirmationDetails { } export interface AutoModeFallbackConfirmation { - reason: 'classifier_unavailable'; + reason: + | 'classifier_blocked_retry' + | 'classifier_unavailable' + | 'consecutive_block' + | 'consecutive_unavailable' + | 'total_denial' + | 'external_write'; message: string; }