diff --git a/packages/cli/src/acp-integration/session/Session.test.ts b/packages/cli/src/acp-integration/session/Session.test.ts index 750b5e34869..24aa3cd99d8 100644 --- a/packages/cli/src/acp-integration/session/Session.test.ts +++ b/packages/cli/src/acp-integration/session/Session.test.ts @@ -26,6 +26,22 @@ import type { LoadedSettings } from '../../config/settings.js'; import * as nonInteractiveCliCommands from '../../nonInteractiveCliCommands.js'; import { CommandKind } from '../../ui/commands/types.js'; +const debugLoggerWarnSpy = vi.hoisted(() => vi.fn()); + +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + createDebugLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: debugLoggerWarnSpy, + error: vi.fn(), + }), + }; +}); + vi.mock('../../nonInteractiveCliCommands.js', () => ({ ALLOWED_BUILTIN_COMMANDS_NON_INTERACTIVE: [ 'init', @@ -2203,6 +2219,105 @@ describe('Session', () => { expect(executeSpy).toHaveBeenCalled(); }); + it('resets AUTO denial counters when a permission-request hook approves a denialTracking fallback prompt', async () => { + const hookSpy = vi + .spyOn(core, 'firePermissionRequestHook') + .mockResolvedValue({ + hasDecision: true, + shouldAllow: true, + updatedInput: undefined, + denyMessage: undefined, + }); + const executeSpy = vi.fn().mockResolvedValue({ + llmContent: 'ok', + returnDisplay: 'ok', + }); + const onConfirmSpy = vi.fn().mockResolvedValue(undefined); + const setAutoModeDenialState = vi.fn(); + const invocation = { + params: { command: 'python -c "print(1)"' }, + getDefaultPermission: vi.fn().mockResolvedValue('ask'), + getConfirmationDetails: vi.fn().mockResolvedValue({ + type: 'exec', + title: 'Need permission', + command: 'python', + rootCommand: 'python', + onConfirm: onConfirmSpy, + }), + getDescription: vi.fn().mockReturnValue('Run command'), + toolLocations: vi.fn().mockReturnValue([]), + execute: executeSpy, + }; + const tool = { + name: core.ToolNames.SHELL, + kind: core.Kind.Execute, + build: vi.fn().mockReturnValue(invocation), + }; + + mockToolRegistry.getTool.mockReturnValue(tool); + mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.AUTO); + mockConfig.getPermissionManager = vi.fn().mockReturnValue(null); + mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(false); + mockConfig.getMessageBus = vi.fn().mockReturnValue({}); + mockConfig.getAutoModeDenialState = vi.fn().mockReturnValue({ + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 20, + totalUnavailable: 0, + }); + mockConfig.setAutoModeDenialState = setAutoModeDenialState; + ( + mockGeminiClient as unknown as { + getHistoryTail: ReturnType; + } + ).getHistoryTail = vi.fn().mockReturnValue([]); + mockChat.sendMessageStream = vi.fn().mockResolvedValue( + createStreamWithChunks([ + { + type: core.StreamEventType.CHUNK, + value: { + functionCalls: [ + { + id: 'call-auto-fallback-hook-approved', + name: core.ToolNames.SHELL, + args: { command: 'python -c "print(1)"' }, + }, + ], + }, + }, + ]), + ); + debugLoggerWarnSpy.mockClear(); + + try { + await session.prompt({ + sessionId: 'test-session-id', + prompt: [{ type: 'text', text: 'run tool' }], + }); + + expect(mockClient.requestPermission).not.toHaveBeenCalled(); + await vi.waitFor(() => { + expect(onConfirmSpy).toHaveBeenCalledWith( + core.ToolConfirmationOutcome.ProceedOnce, + ); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 0, + totalUnavailable: 0, + }); + expect(executeSpy).toHaveBeenCalled(); + }); + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + expect.stringContaining( + 'Auto mode denial counters reset after fallback approval', + ), + ); + } finally { + hookSpy.mockRestore(); + } + }); + describe('hooks', () => { describe('PermissionDenied hook', () => { it('fires PermissionDenied hooks for AUTO classifier blocks', async () => { @@ -2223,7 +2338,11 @@ describe('Session', () => { stage: 'fast', durationMs: 20, }, - { kind: 'blocked', errorMessage: 'blocked' }, + { + kind: 'blocked', + errorMessage: 'blocked', + reason: 'classifier_blocked', + }, core.ToolNames.SHELL, { command: 'rm -rf /tmp/example' }, 'auto-denied-acp', @@ -2256,7 +2375,11 @@ describe('Session', () => { stage: 'fast', durationMs: 3000, }, - { kind: 'blocked', errorMessage: 'blocked' }, + { + kind: 'blocked', + errorMessage: 'blocked', + reason: 'classifier_unavailable', + }, core.ToolNames.SHELL, { command: 'rm -rf /tmp/example' }, 'auto-denied-acp', @@ -2289,7 +2412,11 @@ describe('Session', () => { stage: 'fast', durationMs: 20, }, - { kind: 'blocked', errorMessage: 'blocked' }, + { + kind: 'blocked', + errorMessage: 'blocked', + reason: 'classifier_blocked', + }, core.ToolNames.SHELL, { command: 'rm -rf /tmp/example' }, 'auto-denied-acp', @@ -2316,7 +2443,7 @@ describe('Session', () => { stage: 'fast', durationMs: 20, }, - { kind: 'fallback' }, + { kind: 'fallback', reason: 'safety_check' }, core.ToolNames.SHELL, { command: 'rm -rf /tmp/example' }, 'auto-denied-acp', diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index df4be4e7df3..b43239f1813 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -64,8 +64,10 @@ import { formatStopHookBlockingCapWarning, applyAutoModeDecision, evaluateAutoMode, + formatDenialStateLog, getAutoModePermissionDeniedReason, isApproveOutcome, + isDenialFallbackReason, MAX_TRANSCRIPT_MESSAGES, recordAllow, recordFallbackApprove, @@ -1976,6 +1978,7 @@ export class Session implements SessionContext { recordAllow(this.config.getAutoModeDenialState()), ); } + let wasAutoModeDenialFallback = false; // ── L5: AUTO mode three-layer filter (duplicated from // coreToolScheduler.ts; ACP routes through this Session path). @@ -1984,6 +1987,7 @@ export class Session implements SessionContext { // existing manual-approval flow below. if (!autoModeAllowed && shouldRunAutoModeForCall(approvalMode, fc.name)) { const denialState = this.config.getAutoModeDenialState(); + const fallback = shouldFallback(denialState); // `buildClassifierContents` retains only the most recent // MAX_TRANSCRIPT_MESSAGES messages; ask the chat client for // exactly that tail rather than triggering a `structuredClone` @@ -2000,7 +2004,7 @@ export class Session implements SessionContext { messages, config: this.config, signal: abortSignal, - skipClassifier: shouldFallback(denialState).fallback, + skipClassifierReason: fallback.fallback ? fallback.reason : undefined, }); // Apply decision via shared helper — eliminates ~40 lines of @@ -2027,9 +2031,20 @@ export class Session implements SessionContext { autoModeAllowed = true; break; case 'blocked': + debugLogger.warn( + `Auto mode blocked (${outcome.reason}): tool=${fc.name}, ` + + formatDenialStateLog(denialState), + ); return earlyErrorResponse(new Error(outcome.errorMessage), fc.name); case 'fallback': // Drop through to the manual-approval flow below. + wasAutoModeDenialFallback = isDenialFallbackReason(outcome.reason); + if (wasAutoModeDenialFallback) { + debugLogger.warn( + `Auto mode fallback to manual approval (${outcome.reason}): ` + + formatDenialStateLog(denialState), + ); + } break; default: { const _exhaustive: never = outcome; @@ -2040,6 +2055,33 @@ export class Session implements SessionContext { let didRequestPermission = false; let confirmationDetails: ToolCallConfirmationDetails | undefined; + const recordAutoModeFallbackResolution = ( + outcome: ToolConfirmationOutcome, + ) => { + // Reset AUTO-mode fallback counters when approval resolves a prompt + // raised because denialTracking forced fallback. This covers both ACP + // requestPermission and PermissionRequest hook approvals. + if ( + approvalMode === ApprovalMode.AUTO && + wasAutoModeDenialFallback && + isApproveOutcome(outcome) + ) { + const before = this.config.getAutoModeDenialState(); + const after = recordFallbackApprove(before); + if (after === before) { + debugLogger.warn( + `Auto mode denial counters already clear after fallback approval: ` + + formatDenialStateLog(before), + ); + return; + } + debugLogger.warn( + `Auto mode denial counters reset after fallback approval: ` + + `${formatDenialStateLog(before)} -> ${formatDenialStateLog(after)}`, + ); + this.config.setAutoModeDenialState(after); + } + }; if ( !autoModeAllowed && @@ -2092,6 +2134,9 @@ export class Session implements SessionContext { await confirmationDetails.onConfirm( ToolConfirmationOutcome.ProceedOnce, ); + recordAutoModeFallbackResolution( + ToolConfirmationOutcome.ProceedOnce, + ); } else { return earlyErrorResponse( new Error( @@ -2159,18 +2204,7 @@ export class Session implements SessionContext { .nativeEnum(ToolConfirmationOutcome) .parse(output.outcome.optionId); - // Reset the AUTO-mode fallback streak when the user manually - // approves a prompt that was raised because denialTracking forced - // fallback. Without this, a single block-streak permanently - // downgrades the rest of the session to manual approval until the - // mode is toggled. Parallels coreToolScheduler.ts:1705-1717. - // Cancel / abort do NOT reset — treating rejection as a signal - // the classifier was right to block. - if (approvalMode === ApprovalMode.AUTO && isApproveOutcome(outcome)) { - this.config.setAutoModeDenialState( - recordFallbackApprove(this.config.getAutoModeDenialState()), - ); - } + recordAutoModeFallbackResolution(outcome); await confirmationDetails.onConfirm(outcome, { answers: output.answers, diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index 830a766f18e..55cd754ee00 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -545,7 +545,12 @@ export const InputPrompt: React.FC = ({ setLivePanelFocused(false); return true; } - if (key.sequence && key.sequence.length === 1 && !key.ctrl && !key.meta) { + if ( + key.sequence && + key.sequence.length === 1 && + !key.ctrl && + !key.meta + ) { setLivePanelFocused(false); return false; } diff --git a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx index 0805b177307..b240f037d27 100644 --- a/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx +++ b/packages/cli/src/ui/components/background-view/BackgroundTasksDialog.tsx @@ -951,8 +951,7 @@ export const BackgroundTasksDialog: React.FC = ({ const selectedAgentIdForActivity = selectedEntry?.kind === 'agent' ? selectedEntry.agentId : undefined; useEffect(() => { - if (!dialogOpen || !isDetailMode || !selectedAgentIdForActivity) - return; + if (!dialogOpen || !isDetailMode || !selectedAgentIdForActivity) return; const registry = config.getBackgroundTaskRegistry(); const onActivity = (entry: AgentTask) => { if (entry.agentId !== selectedAgentIdForActivity) return; @@ -960,7 +959,13 @@ export const BackgroundTasksDialog: React.FC = ({ }; registry.setActivityChangeCallback(onActivity); return () => registry.setActivityChangeCallback(undefined); - }, [dialogOpen, dialogMode, isDetailMode, config, selectedAgentIdForActivity]); + }, [ + dialogOpen, + dialogMode, + isDetailMode, + config, + selectedAgentIdForActivity, + ]); // Wall-clock tick for the running agent's duration. Activity callbacks // fire when tools run, but duration needs to advance even when the agent @@ -1021,7 +1026,14 @@ export const BackgroundTasksDialog: React.FC = ({ ) { exitDetail(); } - }, [dialogOpen, dialogMode, isDetailMode, selectedEntryId, selectedStatus, exitDetail]); + }, [ + dialogOpen, + dialogMode, + isDetailMode, + selectedEntryId, + selectedStatus, + exitDetail, + ]); // Encapsulates the cancel flow with the foreground confirm-step. // Foreground entries: first `x` arms; second `x` confirms. Background diff --git a/packages/cli/src/ui/components/hooks/constants.test.ts b/packages/cli/src/ui/components/hooks/constants.test.ts index 58778c98f09..f9100667ae3 100644 --- a/packages/cli/src/ui/components/hooks/constants.test.ts +++ b/packages/cli/src/ui/components/hooks/constants.test.ts @@ -71,6 +71,13 @@ describe('hooks constants', () => { expect(exitCodes).toHaveLength(2); }); + it('should return exit codes for PermissionDenied event', () => { + const exitCodes = getHookExitCodes(HookEventName.PermissionDenied); + expect(exitCodes).toHaveLength(2); + expect(exitCodes[0].code).toBe(0); + expect(exitCodes[1].code).toBe('Other'); + }); + it('should return exit codes for SessionStart event', () => { const exitCodes = getHookExitCodes(HookEventName.SessionStart); expect(exitCodes).toHaveLength(2); @@ -142,6 +149,13 @@ describe('hooks constants', () => { expect(desc).toContain('Stop'); }); + 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', + ); + }); + it('should return empty string for unknown event', () => { const desc = getHookShortDescription('unknown_event' as HookEventName); expect(desc).toBe(''); @@ -160,6 +174,12 @@ describe('hooks constants', () => { expect(desc).toContain('response'); }); + it('should return description for PermissionDenied', () => { + const desc = getHookDescription(HookEventName.PermissionDenied); + expect(desc).toContain('tool_name'); + expect(desc).toContain('reason'); + }); + it('should return empty string for Stop event', () => { const desc = getHookDescription(HookEventName.Stop); expect(desc).toBe(''); @@ -299,6 +319,18 @@ describe('hooks constants', () => { expect(info.matcherGroups).toEqual([]); }); + it('should create empty info for PermissionDenied', () => { + const info = createEmptyHookEventInfo(HookEventName.PermissionDenied); + + expect(info.event).toBe(HookEventName.PermissionDenied); + expect(info.shortDescription).toBe( + 'When a tool call is denied before a permission dialog is displayed', + ); + expect(info.description).toContain('tool_use_id'); + expect(info.exitCodes).toHaveLength(2); + expect(info.matcherGroups).toEqual([]); + }); + it('should create empty info for TodoCreated', () => { const info = createEmptyHookEventInfo(HookEventName.TodoCreated); diff --git a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx index e7bba2a825a..43007342eff 100644 --- a/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx +++ b/packages/cli/src/ui/contexts/BackgroundTaskViewContext.tsx @@ -29,7 +29,11 @@ const debugLogger = createDebugLogger('BG_TASK_VIEW'); // ─── Types ────────────────────────────────────────────────── -export type BackgroundDialogMode = 'closed' | 'list' | 'detail' | 'detail-from-panel'; +export type BackgroundDialogMode = + | 'closed' + | 'list' + | 'detail' + | 'detail-from-panel'; export interface BackgroundTaskViewState { /** @@ -288,7 +292,15 @@ export function BackgroundTaskViewProvider({ livePanelFocused, livePanelSelectedIndex, }), - [entries, selectedIndex, dialogMode, dialogOpen, pillFocused, livePanelFocused, livePanelSelectedIndex], + [ + entries, + selectedIndex, + dialogMode, + dialogOpen, + pillFocused, + livePanelFocused, + livePanelSelectedIndex, + ], ); const actions: BackgroundTaskViewActions = useMemo( diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index caa363ded10..f3eda272d8e 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -83,8 +83,23 @@ type ToolSpanRecord = { const toolSpanRecords = vi.hoisted((): ToolSpanRecord[] => []); const shouldThrowToolSpanSetAttribute = vi.hoisted(() => ({ value: false })); const shouldThrowToolSpanSetStatus = vi.hoisted(() => ({ value: false })); +const debugLoggerWarnSpy = vi.hoisted(() => vi.fn()); const runSideQueryMock = vi.hoisted(() => vi.fn()); +vi.mock('../utils/debugLogger.js', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + createDebugLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: debugLoggerWarnSpy, + error: vi.fn(), + }), + }; +}); + vi.mock('../telemetry/tracer.js', () => ({ safeSetStatus: ( span: { setStatus: (status: { code: number; message?: string }) => void }, @@ -488,6 +503,131 @@ describe('CoreToolScheduler', () => { runSideQueryMock.mockReset(); }); + type SchedulerDenialTrackingInternals = { + toolCalls: ToolCall[]; + autoModeFallbackCallIds: Set; + _handleConfirmationResponseInner: ( + callId: string, + toolCall: ToolCall, + originalOnConfirm: ( + outcome: ToolConfirmationOutcome, + payload?: ToolConfirmationPayload, + ) => Promise, + outcome: ToolConfirmationOutcome, + signal: AbortSignal, + payload?: ToolConfirmationPayload, + ) => Promise; + }; + + function createSchedulerForDenialTrackingApprovalTest() { + const denialState = { + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 20, + totalUnavailable: 0, + }; + const setAutoModeDenialState = vi.fn(); + const scheduler = new CoreToolScheduler({ + config: { + getSessionId: () => 'test-session-id', + getApprovalMode: () => ApprovalMode.AUTO, + getAutoModeDenialState: () => denialState, + setAutoModeDenialState, + getToolRegistry: () => + ({ + getTool: () => undefined, + }) as unknown as ToolRegistry, + getUsageStatisticsEnabled: () => false, + getDebugMode: () => false, + getChatRecordingService: () => undefined, + } as unknown as Config, + onAllToolCallsComplete: vi.fn(), + onToolCallsUpdate: vi.fn(), + getPreferredEditor: () => 'vscode', + onEditorClose: vi.fn(), + }); + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'exec', + title: 'Run command', + command: 'python', + rootCommand: 'python', + onConfirm: vi.fn().mockResolvedValue(undefined), + }; + const toolCall = { + status: 'awaiting_approval', + request: { + callId: 'call-1', + name: ToolNames.SHELL, + args: {}, + isClientInitiated: false, + prompt_id: 'prompt-1', + }, + tool: {}, + confirmationDetails, + } as unknown as ToolCall; + const internals = scheduler as unknown as SchedulerDenialTrackingInternals; + internals.toolCalls = [toolCall]; + return { internals, toolCall, setAutoModeDenialState }; + } + + it('does not reset total denial counters for unrelated AUTO approvals', async () => { + const { internals, toolCall, setAutoModeDenialState } = + createSchedulerForDenialTrackingApprovalTest(); + + await internals._handleConfirmationResponseInner( + 'call-1', + toolCall, + vi.fn().mockResolvedValue(undefined), + ToolConfirmationOutcome.ProceedOnce, + new AbortController().signal, + ); + + expect(setAutoModeDenialState).not.toHaveBeenCalled(); + }); + + it('resets denial counters after approving a denialTracking fallback prompt', async () => { + const { internals, toolCall, setAutoModeDenialState } = + createSchedulerForDenialTrackingApprovalTest(); + internals.autoModeFallbackCallIds.add('call-1'); + debugLoggerWarnSpy.mockClear(); + + await internals._handleConfirmationResponseInner( + 'call-1', + toolCall, + vi.fn().mockResolvedValue(undefined), + ToolConfirmationOutcome.ProceedOnce, + new AbortController().signal, + ); + + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 0, + totalUnavailable: 0, + }); + expect(debugLoggerWarnSpy).toHaveBeenCalledWith( + expect.stringContaining( + 'Auto mode denial counters reset after fallback approval', + ), + ); + }); + + it('does not reset denial counters after cancelling a denialTracking fallback prompt', async () => { + const { internals, toolCall, setAutoModeDenialState } = + createSchedulerForDenialTrackingApprovalTest(); + internals.autoModeFallbackCallIds.add('call-1'); + + await internals._handleConfirmationResponseInner( + 'call-1', + toolCall, + vi.fn().mockResolvedValue(undefined), + ToolConfirmationOutcome.Cancel, + new AbortController().signal, + ); + + expect(setAutoModeDenialState).not.toHaveBeenCalled(); + }); + function createSchedulerForLegacyToolTests(options: { toolsByName: Map; approvalMode?: ApprovalMode; @@ -497,6 +637,13 @@ describe('CoreToolScheduler', () => { firePermissionDeniedEvent: ReturnType; }; disableHooks?: boolean; + autoModeDenialState?: { + consecutiveBlock: number; + consecutiveUnavailable: number; + totalBlock: number; + totalUnavailable: number; + }; + setAutoModeDenialState?: ReturnType; onAllToolCallsComplete?: ReturnType; onToolCallsUpdate?: ReturnType; memoryMonitor?: { scheduleCheck: () => void }; @@ -556,13 +703,14 @@ describe('CoreToolScheduler', () => { getDisableAllHooks: vi .fn() .mockReturnValue(options.disableHooks ?? true), - getAutoModeDenialState: vi.fn().mockReturnValue({ - consecutiveBlock: 0, - consecutiveUnavailable: 0, - totalBlock: 0, - totalUnavailable: 0, - }), - setAutoModeDenialState: vi.fn(), + getAutoModeDenialState: () => + options.autoModeDenialState ?? { + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 0, + totalUnavailable: 0, + }, + setAutoModeDenialState: options.setAutoModeDenialState ?? vi.fn(), getAutoModeSettings: () => ({}), getWorkspaceContext: () => ({ isPathWithinWorkspace: () => false, @@ -1025,6 +1173,96 @@ describe('CoreToolScheduler', () => { }, ); + it('resets denial counters when PermissionRequest hook approves a denialTracking fallback prompt', async () => { + const setAutoModeDenialState = vi.fn(); + const onConfirmSpy = vi.fn().mockResolvedValue(undefined); + const execute = vi.fn().mockResolvedValue({ + llmContent: 'executed', + returnDisplay: 'executed', + }); + const toolsByName = new Map([ + [ + ToolNames.SHELL, + new MockTool({ + name: ToolNames.SHELL, + kind: Kind.Execute, + getDefaultPermission: MOCK_TOOL_GET_DEFAULT_PERMISSION, + getConfirmationDetails: vi.fn().mockResolvedValue({ + type: 'exec', + title: 'Run command', + command: 'python', + rootCommand: 'python', + onConfirm: onConfirmSpy, + }), + execute, + }), + ], + ]); + const messageBus = { + request: vi.fn().mockImplementation( + async (request: { + eventName: string; + }): Promise => ({ + type: MessageBusType.HOOK_EXECUTION_RESPONSE, + correlationId: `${request.eventName}-hook`, + success: true, + output: + request.eventName === 'PermissionRequest' + ? { + hookSpecificOutput: { + decision: { + behavior: 'allow', + }, + }, + } + : { decision: 'allow' }, + }), + ), + }; + const { scheduler, onAllToolCallsComplete } = + createSchedulerForLegacyToolTests({ + toolsByName, + approvalMode: ApprovalMode.AUTO, + messageBus, + disableHooks: false, + autoModeDenialState: { + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 20, + totalUnavailable: 0, + }, + setAutoModeDenialState, + }); + + await scheduler.schedule( + [ + { + callId: 'hook-approved-denial-fallback', + name: ToolNames.SHELL, + args: { command: 'python -c "print(1)"' }, + isClientInitiated: false, + prompt_id: 'prompt-hook-approved-denial-fallback', + }, + ], + new AbortController().signal, + ); + + await vi.waitFor(() => { + expect(onAllToolCallsComplete).toHaveBeenCalled(); + }); + + expect(onConfirmSpy).toHaveBeenCalledWith( + ToolConfirmationOutcome.ProceedOnce, + ); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 0, + totalUnavailable: 0, + }); + expect(execute).toHaveBeenCalledOnce(); + }); + it('should cancel a tool call if the signal is aborted before confirmation', async () => { const mockTool = new MockTool({ name: 'mockTool', diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 534eda6672f..e7b3623b516 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -74,7 +74,9 @@ import { } from '../permissions/autoMode.js'; import { MAX_TRANSCRIPT_MESSAGES } from '../permissions/classifier-transcript.js'; import { + formatDenialStateLog, isApproveOutcome, + isDenialFallbackReason, recordAllow, recordFallbackApprove, shouldFallback, @@ -801,6 +803,7 @@ export class CoreToolScheduler { private isFinalizingToolCalls = false; private isScheduling = false; private validationRetryCounts = new Map(); + private autoModeFallbackCallIds = new Set(); // Tool span lifecycle now spans validating → awaiting_approval → executing // → terminal, so we hold the span across method boundaries by callId. // Decoupling from ToolCall identity is intentional — setStatusInternal @@ -1726,7 +1729,9 @@ export class CoreToolScheduler { messages, config: this.config, signal, - skipClassifier: fallback.fallback, + skipClassifierReason: fallback.fallback + ? fallback.reason + : undefined, }); const outcome = applyAutoModeDecision( @@ -1757,6 +1762,10 @@ export class CoreToolScheduler { this.setStatusInternal(reqInfo.callId, 'scheduled'); continue; case 'blocked': + debugLogger.warn( + `Auto mode blocked (${outcome.reason}): tool=${canonicalName}, ` + + formatDenialStateLog(denialState), + ); this.setStatusInternal( reqInfo.callId, 'error', @@ -1773,9 +1782,11 @@ export class CoreToolScheduler { // operators see the cause in the debug log (only when // fallback was specifically armed by denialTracking — // a pmForcedAsk fallback isn't an audit-worthy event). - if (fallback.fallback) { + if (isDenialFallbackReason(outcome.reason)) { + this.autoModeFallbackCallIds.add(reqInfo.callId); debugLogger.warn( - `Auto mode fallback to manual approval (${fallback.reason}): consecutiveBlock=${denialState.consecutiveBlock}, consecutiveUnavailable=${denialState.consecutiveUnavailable}`, + `Auto mode fallback to manual approval (${outcome.reason}): ` + + formatDenialStateLog(denialState), ); } break; @@ -1907,6 +1918,10 @@ export class CoreToolScheduler { await confirmationDetails.onConfirm( ToolConfirmationOutcome.ProceedOnce, ); + this.recordAutoModeFallbackResolution( + reqInfo.callId, + ToolConfirmationOutcome.ProceedOnce, + ); this.setToolCallOutcome( reqInfo.callId, ToolConfirmationOutcome.ProceedOnce, @@ -1921,6 +1936,10 @@ export class CoreToolScheduler { ToolConfirmationOutcome.Cancel, cancelPayload, ); + this.recordAutoModeFallbackResolution( + reqInfo.callId, + ToolConfirmationOutcome.Cancel, + ); this.setToolCallOutcome( reqInfo.callId, ToolConfirmationOutcome.Cancel, @@ -2231,22 +2250,7 @@ export class CoreToolScheduler { this.setToolCallOutcome(callId, outcome); - // AUTO-mode denialTracking recovery: when the user manually approves a - // call that fell back from AUTO (either by streak threshold or by an - // explicit ask rule), reset the consecutiveBlock counter so subsequent - // calls return to classifier flow. Without this, a session that hit - // the denial threshold once would stay in fallback for the rest of - // the session even after the user explicitly approves the next call. - // Cancel / abort do NOT reset — spec §9.1.4 treats rejection as a - // signal that the classifier was correct to block. - if ( - this.config.getApprovalMode() === ApprovalMode.AUTO && - isApproveOutcome(outcome) - ) { - this.config.setAutoModeDenialState( - recordFallbackApprove(this.config.getAutoModeDenialState()), - ); - } + this.recordAutoModeFallbackResolution(callId, outcome); if (outcome === ToolConfirmationOutcome.Cancel || signal.aborted) { // Use custom cancel message from payload if provided, otherwise use default @@ -2364,6 +2368,40 @@ export class CoreToolScheduler { // (#4321 review-9 wenshao Critical). } + private recordAutoModeFallbackResolution( + callId: string, + outcome: ToolConfirmationOutcome, + ): void { + const wasAutoModeFallback = this.autoModeFallbackCallIds.delete(callId); + + // AUTO-mode denialTracking recovery: when the user manually approves a + // call that fell back because denialTracking was armed, clear the armed + // counters so subsequent calls return to classifier flow. Ordinary AUTO + // approvals for ask rules must not clear cumulative denial totals. + // Cancel / abort do NOT reset — spec §9.1.4 treats rejection as a + // signal that the classifier was correct to block. + if ( + this.config.getApprovalMode() === ApprovalMode.AUTO && + wasAutoModeFallback && + isApproveOutcome(outcome) + ) { + const before = this.config.getAutoModeDenialState(); + const after = recordFallbackApprove(before); + if (after === before) { + debugLogger.warn( + `Auto mode denial counters already clear after fallback approval: ` + + formatDenialStateLog(before), + ); + return; + } + debugLogger.warn( + `Auto mode denial counters reset after fallback approval: ` + + `${formatDenialStateLog(before)} -> ${formatDenialStateLog(after)}`, + ); + this.config.setAutoModeDenialState(after); + } + } + /** * Opens an IDE diff view for edit-type tools when IDE mode is active. * The IDE resolution is handled asynchronously — if the user accepts or diff --git a/packages/core/src/hooks/hookSystem.test.ts b/packages/core/src/hooks/hookSystem.test.ts index 37087c1b602..6f4cbc98658 100644 --- a/packages/core/src/hooks/hookSystem.test.ts +++ b/packages/core/src/hooks/hookSystem.test.ts @@ -1453,6 +1453,26 @@ describe('HookSystem', () => { expect(result).toBeUndefined(); }); + it('should return DefaultHookOutput when finalOutput exists', async () => { + const mockResult = createMockAggregatedResult(true, { + decision: 'block' as HookDecision, + reason: 'Observed denial', + }); + vi.mocked( + mockHookEventHandler.firePermissionDeniedEvent, + ).mockResolvedValue(mockResult); + + const result = await hookSystem.firePermissionDeniedEvent( + 'ReadFile', + { path: '/secret.txt' }, + 'tool-use-2', + 'classifier_unavailable', + ); + + expect(result).toBeDefined(); + expect(result?.isBlockingDecision()).toBe(true); + }); + it('should return PermissionDenied hook output when present', async () => { const mockAggregated = createMockAggregatedResult(true); mockAggregated.finalOutput = { diff --git a/packages/core/src/permissions/autoMode.test.ts b/packages/core/src/permissions/autoMode.test.ts index da8e51b3f3f..2a495c28f70 100644 --- a/packages/core/src/permissions/autoMode.test.ts +++ b/packages/core/src/permissions/autoMode.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, vi } from 'vitest'; import { SAFE_TOOL_ALLOWLIST, + applyAutoModeDecision, evaluateAutoMode, formatClassifierBlockMessage, getAutoModePermissionDeniedReason, @@ -284,18 +285,17 @@ describe('evaluateAutoMode — fast-path gating', () => { config: baseConfig, signal: new AbortController().signal, }); - expect(decision.via).toBe('fallback'); + expect(decision).toEqual({ via: 'fallback', reason: 'ask_rule' }); }); - it('routes to fallback when skipClassifier=true (denialTracking armed)', async () => { + it('routes to fallback with the denialTracking reason when armed', async () => { // Regression guard: when denialTracking has already armed a fallback // (3 consecutive blocks / 2 consecutive unavailables), the scheduler - // passes `skipClassifier: true` so the in-progress call drops to - // manual approval without burning another classifier request. Fast - // paths still fire — only the classifier dispatch is suppressed. - // Tool here is SHELL (not on the allowlist, not an edit), so neither - // fast-path applies; without skipClassifier this would dispatch the - // classifier. + // passes the specific reason so the in-progress call drops to manual + // approval without burning another classifier request. Fast paths still + // fire — only the classifier dispatch is suppressed. Tool here is SHELL + // (not on the allowlist, not an edit), so neither fast-path applies; + // without skipClassifierReason this would dispatch the classifier. const decision = await evaluateAutoMode({ ctx: { toolName: ToolNames.SHELL, command: 'rm -rf /' }, pmForcedAsk: false, @@ -303,9 +303,115 @@ describe('evaluateAutoMode — fast-path gating', () => { messages: [], config: baseConfig, signal: new AbortController().signal, - skipClassifier: true, + skipClassifierReason: 'total_denial', }); - expect(decision.via).toBe('fallback'); + expect(decision).toEqual({ via: 'fallback', reason: 'total_denial' }); + }); +}); + +// ─── applyAutoModeDecision reason mapping ──────────────────────────────── + +describe('applyAutoModeDecision — blocked reason mapping', () => { + const denialState = { + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 0, + totalUnavailable: 0, + }; + + it('maps classifier policy blocks to classifier_blocked', () => { + 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, + denialState, + ); + + expect(result).toMatchObject({ + kind: 'blocked', + reason: 'classifier_blocked', + }); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 1, + consecutiveUnavailable: 0, + totalBlock: 1, + totalUnavailable: 0, + }); + }); + + it('maps classifier infrastructure failures to classifier_unavailable', () => { + const setAutoModeDenialState = vi.fn(); + const result = applyAutoModeDecision( + { + via: 'classifier', + shouldBlock: true, + reason: 'timeout', + unavailable: true, + stage: 'thinking', + durationMs: 10, + }, + { setAutoModeDenialState } as unknown as Config, + denialState, + ); + + expect(result).toMatchObject({ + kind: 'blocked', + reason: 'classifier_unavailable', + }); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 0, + consecutiveUnavailable: 1, + totalBlock: 0, + totalUnavailable: 1, + }); + }); + + it('allows classifier approvals and resets consecutive counters', () => { + const setAutoModeDenialState = vi.fn(); + const result = applyAutoModeDecision( + { + via: 'classifier', + shouldBlock: false, + reason: 'safe command', + unavailable: false, + stage: 'fast', + durationMs: 10, + }, + { setAutoModeDenialState } as unknown as Config, + { + consecutiveBlock: 1, + consecutiveUnavailable: 2, + totalBlock: 3, + totalUnavailable: 4, + }, + ); + + expect(result).toEqual({ kind: 'approved' }); + expect(setAutoModeDenialState).toHaveBeenCalledWith({ + consecutiveBlock: 0, + consecutiveUnavailable: 0, + totalBlock: 3, + totalUnavailable: 4, + }); + }); + + it('passes through fallback reason without mutating denial state', () => { + const setAutoModeDenialState = vi.fn(); + const result = applyAutoModeDecision( + { via: 'fallback', reason: 'consecutive_block' }, + { setAutoModeDenialState } as unknown as Config, + denialState, + ); + + expect(result).toEqual({ kind: 'fallback', reason: 'consecutive_block' }); + expect(setAutoModeDenialState).not.toHaveBeenCalled(); }); }); @@ -372,6 +478,7 @@ describe('PermissionDenied hook gating', () => { shouldFirePermissionDeniedForAutoMode(classifierBlock, { kind: 'blocked', errorMessage: 'blocked', + reason: 'classifier_blocked', }), ).toBe(true); @@ -384,8 +491,8 @@ describe('PermissionDenied hook gating', () => { expect( shouldFirePermissionDeniedForAutoMode( - { via: 'fallback' }, - { kind: 'fallback' }, + { via: 'fallback', reason: 'safety_check' }, + { kind: 'fallback', reason: 'safety_check' }, ), ).toBe(false); }); diff --git a/packages/core/src/permissions/autoMode.ts b/packages/core/src/permissions/autoMode.ts index 612dd946615..370a90a6360 100644 --- a/packages/core/src/permissions/autoMode.ts +++ b/packages/core/src/permissions/autoMode.ts @@ -20,6 +20,7 @@ import type { Content } from '@google/genai'; import { ApprovalMode, type Config } from '../config/config.js'; import type { PermissionDeniedReason } from '../hooks/types.js'; +export type { PermissionDeniedReason } from '../hooks/types.js'; import { ToolNames } from '../tools/tool-names.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { classifyAction, type ClassifierResult } from './classifier.js'; @@ -28,6 +29,7 @@ import { recordBlock, recordUnavailable, type AutoModeDenialState, + type DenialFallbackReason, } from './denialTracking.js'; import type { PermissionCheckContext } from './types.js'; @@ -168,7 +170,27 @@ export type AutoModeDecision = stage: 'fast' | 'thinking'; durationMs: number; } - | { via: 'fallback' }; + | { via: 'fallback'; reason: FallbackToAskReason }; + +/** + * Reasons AUTO mode itself is unavailable before a per-call decision can run. + * Kept distinct from per-call fallback and classifier-denial reasons. + */ +export type AutoModeUnavailableReason = + | 'circuit-breaker' + | 'disabled' + | 'policy'; + +/** + * Reasons a call falls through to manual approval even though AUTO mode is on. + * This is not a denial: the user may still approve the pending request. + */ +export type FallbackToAskReason = + | 'safety_check' + | 'ask_rule' + | 'plan_mode_floor' + | 'org_ask_ceiling' + | DenialFallbackReason; /** * Outcome of {@link applyAutoModeDecision}. Boils the union of @@ -179,8 +201,12 @@ export type AutoModeDecision = */ export type AutoModeOutcome = | { kind: 'approved' } - | { kind: 'blocked'; errorMessage: string } - | { kind: 'fallback' }; + | { + kind: 'blocked'; + errorMessage: string; + reason: PermissionDeniedReason; + } + | { kind: 'fallback'; reason: FallbackToAskReason }; /** * Apply an {@link AutoModeDecision} to denial-tracking state and return @@ -217,12 +243,15 @@ export function applyAutoModeDecision( return { kind: 'blocked', errorMessage: formatClassifierBlockMessage(decision), + reason: decision.unavailable + ? 'classifier_unavailable' + : 'classifier_blocked', }; } config.setAutoModeDenialState(recordAllow(denialState)); return { kind: 'approved' }; case 'fallback': - return { kind: 'fallback' }; + return { kind: 'fallback', reason: decision.reason }; default: { const _exhaustive: never = decision; // Surface drift at runtime — TS exhaustiveness can be bypassed @@ -233,7 +262,7 @@ export function applyAutoModeDecision( `Auto mode: unrecognised decision.via "${(decision as { via: string }).via}" — falling through to manual approval`, ); void _exhaustive; - return { kind: 'fallback' }; + return { kind: 'fallback', reason: 'safety_check' }; } } } @@ -301,13 +330,13 @@ export interface EvaluateAutoModeInput { config: Config; signal: AbortSignal; /** - * When true, the L5.3 classifier is skipped and an unmatched call - * resolves to `{ via: 'fallback' }`. Used by the scheduler to short- - * circuit classifier dispatch when denialTracking has already armed a - * fallback to manual approval — while still letting safe tools take - * the L5.1 / L5.2 fast-paths. + * When present, the L5.3 classifier is skipped and an unmatched call + * resolves to `{ via: 'fallback', reason: skipClassifierReason }`. + * Used by the scheduler to short-circuit classifier dispatch when + * denialTracking has already armed a fallback to manual approval — + * while still letting safe tools take the L5.1 / L5.2 fast-paths. */ - skipClassifier?: boolean; + skipClassifierReason?: DenialFallbackReason; } /** @@ -343,14 +372,14 @@ export async function evaluateAutoMode( // the same reason; the classifier path was the missing leg. // (auto-mode.md documents this as "ask rules force manual confirmation".) if (input.pmForcedAsk) { - return { via: 'fallback' }; + return { via: 'fallback', reason: 'ask_rule' }; } // Caller (scheduler) has detected an armed fallback state; surface that // so the call drops to manual approval instead of burning a classifier // request that would deepen the denial streak. - if (input.skipClassifier) { - return { via: 'fallback' }; + if (input.skipClassifierReason) { + return { via: 'fallback', reason: input.skipClassifierReason }; } // L5.3: two-stage LLM classifier. diff --git a/packages/core/src/permissions/denialTracking.test.ts b/packages/core/src/permissions/denialTracking.test.ts index 331590e83da..b08ba2f4049 100644 --- a/packages/core/src/permissions/denialTracking.test.ts +++ b/packages/core/src/permissions/denialTracking.test.ts @@ -8,7 +8,9 @@ import { describe, it, expect } from 'vitest'; import { AUTO_MODE_DENIAL_LIMITS, createDenialState, + formatDenialStateLog, isApproveOutcome, + isDenialFallbackReason, recordAllow, recordBlock, recordFallbackApprove, @@ -31,6 +33,35 @@ describe('createDenialState', () => { }); }); +describe('formatDenialStateLog', () => { + it('formats every denial counter in a stable order', () => { + expect( + formatDenialStateLog({ + consecutiveBlock: 1, + consecutiveUnavailable: 2, + totalBlock: 3, + totalUnavailable: 4, + }), + ).toBe( + 'consecutiveBlock=1, consecutiveUnavailable=2, totalBlock=3, totalUnavailable=4', + ); + }); +}); + +describe('isDenialFallbackReason', () => { + it('accepts denial-tracking fallback reasons', () => { + expect(isDenialFallbackReason('consecutive_block')).toBe(true); + expect(isDenialFallbackReason('consecutive_unavailable')).toBe(true); + expect(isDenialFallbackReason('total_denial')).toBe(true); + }); + + it('rejects non-denial fallback reasons', () => { + expect(isDenialFallbackReason('ask_rule')).toBe(false); + expect(isDenialFallbackReason('safety_check')).toBe(false); + expect(isDenialFallbackReason('')).toBe(false); + }); +}); + describe('recordBlock', () => { it('increments consecutiveBlock and totalBlock', () => { const s = recordBlock(FRESH); @@ -114,14 +145,34 @@ describe('shouldFallback', () => { }); }); - it('does NOT trigger on totalBlock alone (telemetry-only)', () => { + it('triggers fallback after 20 total denials even when they are not consecutive', () => { let s: AutoModeDenialState = FRESH; - for (let i = 0; i < 50; i++) { - s = recordBlock(s); + for (let i = 0; i < AUTO_MODE_DENIAL_LIMITS.maxTotalDenials - 1; i++) { + s = i % 2 === 0 ? recordBlock(s) : recordUnavailable(s); s = recordAllow(s); // cycle block→allow so consecutive resets each round } - expect(s.totalBlock).toBe(50); + expect(s.totalBlock + s.totalUnavailable).toBe(19); expect(shouldFallback(s)).toEqual({ fallback: false }); + s = recordBlock(s); + expect(s.totalBlock + s.totalUnavailable).toBe(20); + expect(shouldFallback(s)).toEqual({ + fallback: true, + reason: 'total_denial', + }); + }); + + it('gives total-denial fallback precedence over consecutive thresholds', () => { + const s: AutoModeDenialState = { + consecutiveBlock: AUTO_MODE_DENIAL_LIMITS.maxConsecutiveBlock, + consecutiveUnavailable: 0, + totalBlock: AUTO_MODE_DENIAL_LIMITS.maxTotalDenials, + totalUnavailable: 0, + }; + + expect(shouldFallback(s)).toEqual({ + fallback: true, + reason: 'total_denial', + }); }); }); @@ -153,7 +204,7 @@ describe('recordFallbackApprove', () => { expect(shouldFallback(s).fallback).toBe(false); }); - it('preserves total counters (telemetry)', () => { + it('preserves total counters below the total denial cap', () => { let s: AutoModeDenialState = FRESH; s = recordBlock(s); s = recordUnavailable(s); @@ -163,6 +214,37 @@ describe('recordFallbackApprove', () => { expect(s.totalUnavailable).toBe(2); }); + it('resets total counters after the user approves a total-cap fallback prompt', () => { + let s: AutoModeDenialState = FRESH; + for (let i = 0; i < AUTO_MODE_DENIAL_LIMITS.maxTotalDenials; i++) { + s = recordBlock(s); + s = recordAllow(s); + } + expect(shouldFallback(s)).toEqual({ + fallback: true, + reason: 'total_denial', + }); + + s = recordFallbackApprove(s); + + expect(s.consecutiveBlock).toBe(0); + expect(s.consecutiveUnavailable).toBe(0); + expect(s.totalBlock).toBe(0); + expect(s.totalUnavailable).toBe(0); + expect(shouldFallback(s)).toEqual({ fallback: false }); + }); + + it('resets all counters when total and consecutive caps overlap', () => { + const s: AutoModeDenialState = { + consecutiveBlock: AUTO_MODE_DENIAL_LIMITS.maxConsecutiveBlock, + consecutiveUnavailable: 0, + totalBlock: AUTO_MODE_DENIAL_LIMITS.maxTotalDenials, + totalUnavailable: 0, + }; + + expect(recordFallbackApprove(s)).toEqual(FRESH); + }); + it('is a no-op when both consecutive counters are already zero', () => { const s: AutoModeDenialState = FRESH; expect(recordFallbackApprove(s)).toBe(s); @@ -203,5 +285,6 @@ describe('AUTO_MODE_DENIAL_LIMITS', () => { it('is frozen at the documented values', () => { expect(AUTO_MODE_DENIAL_LIMITS.maxConsecutiveBlock).toBe(3); expect(AUTO_MODE_DENIAL_LIMITS.maxConsecutiveUnavailable).toBe(2); + expect(AUTO_MODE_DENIAL_LIMITS.maxTotalDenials).toBe(20); }); }); diff --git a/packages/core/src/permissions/denialTracking.ts b/packages/core/src/permissions/denialTracking.ts index d1f23fb0731..e10b29435ba 100644 --- a/packages/core/src/permissions/denialTracking.ts +++ b/packages/core/src/permissions/denialTracking.ts @@ -7,23 +7,25 @@ * * Protects users from infinite loops when the classifier persistently blocks * (LLM stuck in a dead-end) or persistently fails (infrastructure problem). - * After the consecutive thresholds 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. + * 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. * * Block and unavailable counters cross-reset: they represent different * failure modes and should not accumulate together. Switching ApprovalMode * resets all counters. * - * `total*` counters are telemetry-only — they do NOT trigger fallback. - * A long session naturally accumulates blocks; forcing manual approval after - * an absolute total would harm UX. + * `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. */ /** Reasons the orchestrator may choose to fall back to manual approval. */ export type DenialFallbackReason = | 'consecutive_block' - | 'consecutive_unavailable'; + | 'consecutive_unavailable' + | 'total_denial'; export interface AutoModeDenialState { consecutiveBlock: number; @@ -35,8 +37,16 @@ export interface AutoModeDenialState { export const AUTO_MODE_DENIAL_LIMITS = { maxConsecutiveBlock: 3, maxConsecutiveUnavailable: 2, + maxTotalDenials: 20, } as const; +function hasReachedTotalCap(state: AutoModeDenialState): boolean { + return ( + state.totalBlock + state.totalUnavailable >= + AUTO_MODE_DENIAL_LIMITS.maxTotalDenials + ); +} + /** Freshly-initialised state with all counters zero. */ export function createDenialState(): AutoModeDenialState { return { @@ -47,6 +57,27 @@ export function createDenialState(): AutoModeDenialState { }; } +/** Format denial counters for AUTO-mode debug logs. */ +export function formatDenialStateLog(state: AutoModeDenialState): string { + return ( + `consecutiveBlock=${state.consecutiveBlock}, ` + + `consecutiveUnavailable=${state.consecutiveUnavailable}, ` + + `totalBlock=${state.totalBlock}, ` + + `totalUnavailable=${state.totalUnavailable}` + ); +} + +/** True when a fallback reason came from denial tracking. */ +export function isDenialFallbackReason( + reason: string, +): reason is DenialFallbackReason { + return ( + reason === 'consecutive_block' || + 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) { @@ -91,11 +122,15 @@ 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. + * 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, ): { fallback: true; reason: DenialFallbackReason } | { fallback: false } { + if (hasReachedTotalCap(state)) { + return { fallback: true, reason: 'total_denial' }; + } if (state.consecutiveBlock >= AUTO_MODE_DENIAL_LIMITS.maxConsecutiveBlock) { return { fallback: true, reason: 'consecutive_block' }; } @@ -111,6 +146,8 @@ export function shouldFallback( /** * 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 @@ -126,6 +163,9 @@ export function shouldFallback( export function recordFallbackApprove( state: AutoModeDenialState, ): AutoModeDenialState { + if (hasReachedTotalCap(state)) { + return createDenialState(); + } 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 eacf1ec1b08..c896cfef25b 100644 --- a/packages/core/src/permissions/index.ts +++ b/packages/core/src/permissions/index.ts @@ -14,10 +14,13 @@ export { applyAutoModeDecision, evaluateAutoMode, formatClassifierBlockMessage, + type AutoModeUnavailableReason, getAutoModePermissionDeniedReason, type AutoModeDecision, + type FallbackToAskReason, type AutoModeOutcome, type EvaluateAutoModeInput, + type PermissionDeniedReason, SAFE_TOOL_ALLOWLIST, isInSafeToolAllowlist, passesAcceptEditsFastPath, @@ -29,7 +32,9 @@ export { type DenialFallbackReason, AUTO_MODE_DENIAL_LIMITS, createDenialState, + formatDenialStateLog, isApproveOutcome, + isDenialFallbackReason, recordAllow, recordBlock, recordFallbackApprove,