mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
feat(core): surface PreToolUse hook 'ask' as a TUI confirmation (#5629)
* feat(core): surface PreToolUse hook 'ask' as a TUI confirmation A PreToolUse hook returning permissionDecision:'ask' was treated the same as 'deny'. The hook fires in the execution phase (_executeToolCallBody), after the confirmation flow in _schedule has finished, so an 'ask' could only block the tool as EXECUTION_DENIED instead of prompting the user. Bounce the tool from the execution phase back to awaiting_approval when a hook asks: build a synthetic 'info' confirmation whose onConfirm routes through handleConfirmationResponse (ProceedOnce re-executes, Cancel cancels). PreToolUse keeps its "before execution" timing — only the 'ask' branch is new; 'denied'/'stop' keep deny-as-error. A non-interactive CLI or background agent cannot prompt, so 'ask' falls back to deny there. The re-execution after approval skips both the hook re-fire (no infinite re-ask loop) and the non-idempotent path-unescape prelude. A walk-away abort sets a terminal status so the turn cannot hang, and the tool span survives the bounce so it is finalized exactly once. Tests: add coverage for ask->awaiting_approval, approve->execute-once (no re-ask loop), decline->cancelled, non-interactive/background deny, walk-away abort, single span finalize, and no double path-unescape. * fix(core): handle PreToolUse 'ask' bounce edge cases from review Round 1 review of #5629 surfaced edge cases in the bounce mechanism: - Multi-tool batch hang: a bounced tool approved while a sibling was still executing stayed stuck in 'scheduled'. attemptExecutionOfScheduledCalls now loops, re-checking for newly-scheduled bounce-approved calls after each batch drains. - Orphaned hook events: the post-approval re-execution generated a fresh tool_use_id, leaving PreToolUse(old)/PostToolUse(new) unpaired. Preserve and reuse the original id across the bounce. - ModifyWithEditor double-unescape: request.args is unescaped in place before the hook fires, so the ModifyWithEditor branch must skip its own unescape for a bounced tool (it would double-strip escaped metacharacters). - Missing signal.aborted re-check before bouncing: mirror the confirmation-phase guard so an aborted signal falls through to deny instead of flashing a confirmation nobody can answer. Tests: multi-tool-hang regression (RED before the loop fix), non-interactive STREAM_JSON and Zed bounce paths, and span-finalize assertions on the walk-away abort test. * fix(core): keep PreToolUse 'ask' gate when a sibling is auto-approved Round 2 review: autoApproveCompatiblePendingTools auto-approved every awaiting_approval tool when a sibling was approved with ProceedAlways — including tools bounced by a PreToolUse 'ask'. The bounced tool would be auto-approved and re-executed with the hook skipped (isPostAskReexecution), silently defeating the hook's confirmation gate. Exclude bounced callIds from the auto-approve filter so a hook 'ask' always requires explicit confirmation. Test: a sibling's ProceedAlways no longer auto-approves a bounced ask (RED before the filter guard). * fix(cli): preserve hook ask prompts on approval mode change * fix(core): handle PreToolUse ask edge cases * fix(core): cancel scheduled calls during ask abort drain --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
顾盼
GitHub
parent
42921028fb
commit
7281eb58fc
4 changed files with 1139 additions and 51 deletions
|
|
@ -6010,6 +6010,47 @@ describe('useGeminiStream', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should not auto-approve prompts that hide always allow', async () => {
|
||||
const mockOnConfirm = vi.fn().mockResolvedValue(undefined);
|
||||
const awaitingApprovalToolCalls: TrackedToolCall[] = [
|
||||
{
|
||||
request: {
|
||||
callId: 'call1',
|
||||
name: 'replace',
|
||||
args: { old_string: 'old', new_string: 'new' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-id-1',
|
||||
},
|
||||
status: 'awaiting_approval',
|
||||
responseSubmittedToGemini: false,
|
||||
confirmationDetails: {
|
||||
onConfirm: mockOnConfirm,
|
||||
onCancel: vi.fn(),
|
||||
message: 'Confirm hook ask?',
|
||||
displayedText: 'Hook requested confirmation',
|
||||
hideAlwaysAllow: true,
|
||||
},
|
||||
tool: {
|
||||
name: 'replace',
|
||||
displayName: 'replace',
|
||||
description: 'Replace text',
|
||||
build: vi.fn(),
|
||||
} as any,
|
||||
invocation: {
|
||||
getDescription: () => 'Mock description',
|
||||
} as unknown as AnyToolInvocation,
|
||||
} as unknown as TrackedWaitingToolCall,
|
||||
];
|
||||
|
||||
const { result } = renderTestHook(awaitingApprovalToolCalls);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleApprovalModeChange(ApprovalMode.YOLO);
|
||||
});
|
||||
|
||||
expect(mockOnConfirm).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should only auto-approve edit tools when switching to AUTO_EDIT mode', async () => {
|
||||
const mockOnConfirmReplace = vi.fn().mockResolvedValue(undefined);
|
||||
const mockOnConfirmWrite = vi.fn().mockResolvedValue(undefined);
|
||||
|
|
|
|||
|
|
@ -2758,8 +2758,17 @@ export const useGeminiStream = (
|
|||
newApprovalMode === ApprovalMode.AUTO_EDIT
|
||||
) {
|
||||
let awaitingApprovalCalls = toolCalls.filter(
|
||||
(call): call is TrackedWaitingToolCall =>
|
||||
call.status === 'awaiting_approval',
|
||||
(call): call is TrackedWaitingToolCall => {
|
||||
if (call.status !== 'awaiting_approval') {
|
||||
return false;
|
||||
}
|
||||
const { confirmationDetails } = call;
|
||||
return !(
|
||||
confirmationDetails &&
|
||||
'hideAlwaysAllow' in confirmationDetails &&
|
||||
confirmationDetails.hideAlwaysAllow === true
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
// For AUTO_EDIT mode, only approve edit tools (edit/replace, write_file, notebook_edit)
|
||||
|
|
|
|||
|
|
@ -54,6 +54,8 @@ import {
|
|||
import { MessageBusType } from '../confirmation-bus/types.js';
|
||||
import type { HookExecutionResponse } from '../confirmation-bus/types.js';
|
||||
import { type NotificationType } from '../hooks/types.js';
|
||||
import { InputFormat } from '../output/types.js';
|
||||
import { unescapePath } from '../utils/paths.js';
|
||||
import type { MessageBus } from '../confirmation-bus/message-bus.js';
|
||||
import { IdeClient } from '../ide/ide-client.js';
|
||||
import { WriteFileTool } from '../tools/write-file.js';
|
||||
|
|
@ -1051,7 +1053,6 @@ describe('CoreToolScheduler', () => {
|
|||
}
|
||||
});
|
||||
|
||||
|
||||
it('executes only the first request for duplicate callIds in one batch', async () => {
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'first result',
|
||||
|
|
@ -6842,37 +6843,48 @@ describe('CoreToolScheduler telemetry spans', () => {
|
|||
signal?: AbortSignal,
|
||||
updateOutput?: (output: string) => void,
|
||||
) => Promise<ToolResult>;
|
||||
tools?: MockTool[];
|
||||
messageBus?: { request: ReturnType<typeof vi.fn> };
|
||||
disableHooks?: boolean;
|
||||
canUpdateOutput?: boolean;
|
||||
isInteractive?: boolean;
|
||||
inputFormat?: InputFormat;
|
||||
shouldAvoidPermissionPrompts?: boolean;
|
||||
experimentalZedIntegration?: boolean;
|
||||
includeSensitiveSpanAttributes?: boolean;
|
||||
sensitiveSpanAttributeMaxLength?: number;
|
||||
}): {
|
||||
scheduler: CoreToolScheduler;
|
||||
onAllToolCallsComplete: ReturnType<typeof vi.fn>;
|
||||
onToolCallsUpdate: ReturnType<typeof vi.fn>;
|
||||
} {
|
||||
const mockTool = new MockTool({
|
||||
name: 'mockTool',
|
||||
canUpdateOutput: options.canUpdateOutput,
|
||||
execute:
|
||||
options.execute ??
|
||||
vi.fn().mockResolvedValue({
|
||||
llmContent: 'ok',
|
||||
returnDisplay: 'ok',
|
||||
}),
|
||||
});
|
||||
const tools = options.tools ?? [
|
||||
new MockTool({
|
||||
name: 'mockTool',
|
||||
canUpdateOutput: options.canUpdateOutput,
|
||||
execute:
|
||||
options.execute ??
|
||||
vi.fn().mockResolvedValue({
|
||||
llmContent: 'ok',
|
||||
returnDisplay: 'ok',
|
||||
}),
|
||||
}),
|
||||
];
|
||||
const toolsByName = new Map(tools.map((t) => [t.name, t]));
|
||||
const lookup = (name?: string) =>
|
||||
(name ? toolsByName.get(name) : undefined) ?? tools[0];
|
||||
const mockToolRegistry = {
|
||||
getTool: () => mockTool,
|
||||
ensureTool: async () => mockTool,
|
||||
getTool: (n?: string) => lookup(n),
|
||||
ensureTool: async (n?: string) => lookup(n),
|
||||
getFunctionDeclarations: () => [],
|
||||
tools: new Map(),
|
||||
discovery: {},
|
||||
registerTool: () => {},
|
||||
getToolByName: () => mockTool,
|
||||
getToolByDisplayName: () => mockTool,
|
||||
getTools: () => [],
|
||||
getToolByName: (n?: string) => lookup(n),
|
||||
getToolByDisplayName: (n?: string) => lookup(n),
|
||||
getTools: () => tools,
|
||||
discoverTools: async () => {},
|
||||
getAllTools: () => [],
|
||||
getAllTools: () => tools,
|
||||
getToolsByServer: () => [],
|
||||
} as unknown as ToolRegistry;
|
||||
|
||||
|
|
@ -6900,6 +6912,14 @@ describe('CoreToolScheduler telemetry spans', () => {
|
|||
getChatRecordingService: () => undefined,
|
||||
getMessageBus: vi.fn().mockReturnValue(options.messageBus),
|
||||
getDisableAllHooks: vi.fn().mockReturnValue(options.disableHooks ?? true),
|
||||
// Confirmation-prompt capability stubs — consumed by
|
||||
// canPromptForAskBounce when a PreToolUse hook returns 'ask'.
|
||||
isInteractive: () => options.isInteractive ?? true,
|
||||
getInputFormat: () => options.inputFormat ?? InputFormat.TEXT,
|
||||
getExperimentalZedIntegration: () =>
|
||||
options.experimentalZedIntegration ?? false,
|
||||
getShouldAvoidPermissionPrompts: () =>
|
||||
options.shouldAvoidPermissionPrompts ?? false,
|
||||
getTelemetryIncludeSensitiveSpanAttributes: () =>
|
||||
options.includeSensitiveSpanAttributes ?? false,
|
||||
getTelemetrySensitiveSpanAttributeMaxLength: () =>
|
||||
|
|
@ -6907,14 +6927,15 @@ describe('CoreToolScheduler telemetry spans', () => {
|
|||
} as unknown as Config;
|
||||
|
||||
const onAllToolCallsComplete = vi.fn();
|
||||
const onToolCallsUpdate = vi.fn();
|
||||
const scheduler = new CoreToolScheduler({
|
||||
config: mockConfig,
|
||||
onAllToolCallsComplete,
|
||||
onToolCallsUpdate: vi.fn(),
|
||||
onToolCallsUpdate,
|
||||
getPreferredEditor: () => 'vscode',
|
||||
onEditorClose: vi.fn(),
|
||||
});
|
||||
return { scheduler, onAllToolCallsComplete };
|
||||
return { scheduler, onAllToolCallsComplete, onToolCallsUpdate };
|
||||
}
|
||||
|
||||
async function runSingleTool(
|
||||
|
|
@ -7726,6 +7747,797 @@ describe('CoreToolScheduler telemetry spans', () => {
|
|||
expect(getBlockedSpans()).toHaveLength(0);
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// PreToolUse hook permissionDecision:'ask' — bounce the tool from the
|
||||
// EXECUTION phase back to awaiting_approval for a native TUI confirmation
|
||||
// instead of denying it (the historical behavior). When we cannot prompt
|
||||
// (non-interactive / background agent) it still falls back to deny.
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
function askMessageBus(reason = 'please confirm'): {
|
||||
request: ReturnType<typeof vi.fn>;
|
||||
} {
|
||||
return {
|
||||
request: vi.fn().mockResolvedValue({
|
||||
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
|
||||
correlationId: 'pre-hook',
|
||||
success: true,
|
||||
output: { decision: 'ask', reason },
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
async function scheduleWithAsk(options: {
|
||||
messageBus: { request: ReturnType<typeof vi.fn> };
|
||||
execute?: () => Promise<ToolResult>;
|
||||
isInteractive?: boolean;
|
||||
inputFormat?: InputFormat;
|
||||
shouldAvoidPermissionPrompts?: boolean;
|
||||
experimentalZedIntegration?: boolean;
|
||||
args?: Record<string, unknown>;
|
||||
abortController?: AbortController;
|
||||
tools?: MockTool[];
|
||||
}): Promise<{
|
||||
scheduler: CoreToolScheduler;
|
||||
onAllToolCallsComplete: ReturnType<typeof vi.fn>;
|
||||
onToolCallsUpdate: ReturnType<typeof vi.fn>;
|
||||
abortController: AbortController;
|
||||
}> {
|
||||
toolSpanRecords.length = 0;
|
||||
const built = buildScheduler({ disableHooks: false, ...options });
|
||||
const abortController = options.abortController ?? new AbortController();
|
||||
await built.scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'ask-call',
|
||||
name: 'mockTool',
|
||||
args: options.args ?? { input: 'x' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-ask',
|
||||
},
|
||||
],
|
||||
abortController.signal,
|
||||
);
|
||||
return { ...built, abortController };
|
||||
}
|
||||
|
||||
// Count only PreToolUse fires — the same messageBus mock also serves
|
||||
// PostToolUse/PostToolBatch, so a raw call count would be ambiguous.
|
||||
function preToolUseCallCount(messageBus: {
|
||||
request: ReturnType<typeof vi.fn>;
|
||||
}): number {
|
||||
return messageBus.request.mock.calls.filter(
|
||||
(call) => (call[0] as { eventName?: string })?.eventName === 'PreToolUse',
|
||||
).length;
|
||||
}
|
||||
|
||||
it('bounces a PreToolUse ask to awaiting_approval with an info confirmation', async () => {
|
||||
const messageBus = askMessageBus('confirm deploy 38111');
|
||||
const { onToolCallsUpdate } = await scheduleWithAsk({ messageBus });
|
||||
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
|
||||
expect(waiting.confirmationDetails.type).toBe('info');
|
||||
// The hook re-evaluates on every call, so "always allow" is hidden.
|
||||
expect(
|
||||
(waiting.confirmationDetails as { hideAlwaysAllow?: boolean })
|
||||
.hideAlwaysAllow,
|
||||
).toBe(true);
|
||||
expect(
|
||||
(waiting.confirmationDetails as { prompt: string }).prompt,
|
||||
).toContain('confirm deploy 38111');
|
||||
// One open blocked_on_user span; the tool span stays open across the
|
||||
// bounce (it is NOT finalized until the confirmation resolves).
|
||||
const blocked = getBlockedSpans();
|
||||
expect(blocked).toHaveLength(1);
|
||||
expect(blocked[0].ended).toBe(false);
|
||||
expect(getToolSpans()[0].ended).toBe(false);
|
||||
});
|
||||
|
||||
it('executes the tool exactly once when the user approves an ask (no re-ask loop)', async () => {
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'ok',
|
||||
returnDisplay: 'ok',
|
||||
});
|
||||
const messageBus = askMessageBus();
|
||||
const { onToolCallsUpdate, onAllToolCallsComplete } = await scheduleWithAsk(
|
||||
{
|
||||
messageBus,
|
||||
execute,
|
||||
},
|
||||
);
|
||||
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
await waiting.confirmationDetails.onConfirm(
|
||||
ToolConfirmationOutcome.ProceedOnce,
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
const completed = onAllToolCallsComplete.mock.calls.at(
|
||||
-1,
|
||||
)?.[0] as ToolCall[];
|
||||
expect(completed[0].status).toBe('success');
|
||||
expect(execute).toHaveBeenCalledTimes(1);
|
||||
// The re-execution skips the hook → PreToolUse fired exactly once.
|
||||
expect(preToolUseCallCount(messageBus)).toBe(1);
|
||||
|
||||
// Tool span finalized exactly once; blocked span ended.
|
||||
const toolSpans = getToolSpans();
|
||||
expect(toolSpans).toHaveLength(1);
|
||||
expect(toolSpans[0].ended).toBe(true);
|
||||
const blocked = getBlockedSpans();
|
||||
expect(blocked).toHaveLength(1);
|
||||
expect(blocked[0].ended).toBe(true);
|
||||
});
|
||||
|
||||
it('cancels the tool without executing when the user declines an ask', async () => {
|
||||
const execute = vi.fn();
|
||||
const messageBus = askMessageBus();
|
||||
const { onToolCallsUpdate, onAllToolCallsComplete } = await scheduleWithAsk(
|
||||
{
|
||||
messageBus,
|
||||
execute,
|
||||
},
|
||||
);
|
||||
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
await waiting.confirmationDetails.onConfirm(ToolConfirmationOutcome.Cancel);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
const completed = onAllToolCallsComplete.mock.calls.at(
|
||||
-1,
|
||||
)?.[0] as ToolCall[];
|
||||
expect(completed[0].status).toBe('cancelled');
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('denies a PreToolUse ask (no bounce) in non-interactive mode', async () => {
|
||||
const execute = vi.fn();
|
||||
const messageBus = askMessageBus();
|
||||
const { onAllToolCallsComplete } = await scheduleWithAsk({
|
||||
messageBus,
|
||||
execute,
|
||||
isInteractive: false,
|
||||
inputFormat: InputFormat.TEXT,
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
const completed = onAllToolCallsComplete.mock.calls.at(
|
||||
-1,
|
||||
)?.[0] as ToolCall[];
|
||||
expect(completed[0].status).toBe('error');
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
// Never bounced → no blocked span.
|
||||
expect(getBlockedSpans()).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('denies a PreToolUse ask for background agents', async () => {
|
||||
const execute = vi.fn();
|
||||
const messageBus = askMessageBus();
|
||||
const { onAllToolCallsComplete } = await scheduleWithAsk({
|
||||
messageBus,
|
||||
execute,
|
||||
shouldAvoidPermissionPrompts: true,
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
const completed = onAllToolCallsComplete.mock.calls.at(
|
||||
-1,
|
||||
)?.[0] as ToolCall[];
|
||||
expect(completed[0].status).toBe('error');
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
expect(getBlockedSpans()).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('cancels a pending ask (no hang) when the signal aborts', async () => {
|
||||
const execute = vi.fn();
|
||||
const messageBus = askMessageBus();
|
||||
const abortController = new AbortController();
|
||||
const { onToolCallsUpdate, onAllToolCallsComplete } = await scheduleWithAsk(
|
||||
{
|
||||
messageBus,
|
||||
execute,
|
||||
abortController,
|
||||
},
|
||||
);
|
||||
|
||||
await waitForStatus(onToolCallsUpdate, 'awaiting_approval');
|
||||
abortController.abort();
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
const completed = onAllToolCallsComplete.mock.calls.at(
|
||||
-1,
|
||||
)?.[0] as ToolCall[];
|
||||
expect(completed[0].status).toBe('cancelled');
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
// The drainSpansForBatch safety net must also END both spans (not just
|
||||
// leave the tool stuck) — guards against accidental removal of the
|
||||
// terminal setStatusInternal added to drainSpansForBatch.
|
||||
expect(getBlockedSpans()[0]?.ended).toBe(true);
|
||||
expect(getToolSpans()[0]?.ended).toBe(true);
|
||||
});
|
||||
|
||||
it('does not double-unescape path args across an ask bounce', async () => {
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'ok',
|
||||
returnDisplay: 'ok',
|
||||
});
|
||||
const messageBus = askMessageBus();
|
||||
// Two backslashes before the space: unescaping once → `a\ b`, twice →
|
||||
// `a b`. The re-execution must skip the unescape prelude so the path is
|
||||
// unescaped exactly once.
|
||||
const rawPath = 'a\\\\ b';
|
||||
const { onToolCallsUpdate, onAllToolCallsComplete } = await scheduleWithAsk(
|
||||
{
|
||||
messageBus,
|
||||
execute,
|
||||
args: { file_path: rawPath },
|
||||
},
|
||||
);
|
||||
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
await waiting.confirmationDetails.onConfirm(
|
||||
ToolConfirmationOutcome.ProceedOnce,
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
const completed = onAllToolCallsComplete.mock.calls.at(
|
||||
-1,
|
||||
)?.[0] as ToolCall[];
|
||||
expect(completed[0].status).toBe('success');
|
||||
expect(
|
||||
(completed[0].request.args as Record<string, unknown>)['file_path'],
|
||||
).toBe(unescapePath(rawPath));
|
||||
});
|
||||
|
||||
it('approving a bounced ask runs the tool even while a sibling is still executing', async () => {
|
||||
toolSpanRecords.length = 0;
|
||||
// toolA bounces (PreToolUse 'ask'); toolB's execute stays pending so it
|
||||
// is still 'executing' when the user approves A. The
|
||||
// attemptExecutionOfScheduledCalls guard fails on that pass — without a
|
||||
// re-check after toolB drains, toolA would hang in 'scheduled' forever.
|
||||
let resolveB!: (r: ToolResult) => void;
|
||||
const bDone = new Promise<ToolResult>((res) => {
|
||||
resolveB = res;
|
||||
});
|
||||
const aExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'A ok',
|
||||
returnDisplay: 'A ok',
|
||||
});
|
||||
const bExecute = vi.fn().mockReturnValue(bDone);
|
||||
const tools = [
|
||||
new MockTool({ name: 'toolA', kind: Kind.Read, execute: aExecute }),
|
||||
new MockTool({ name: 'toolB', kind: Kind.Read, execute: bExecute }),
|
||||
];
|
||||
const messageBus = {
|
||||
request: vi
|
||||
.fn()
|
||||
.mockImplementation(
|
||||
async (req: {
|
||||
eventName?: string;
|
||||
input?: { tool_name?: string };
|
||||
}) => ({
|
||||
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
|
||||
correlationId: 'pre-hook',
|
||||
success: true,
|
||||
output:
|
||||
req.eventName === 'PreToolUse' && req.input?.tool_name === 'toolA'
|
||||
? { decision: 'ask', reason: 'confirm A' }
|
||||
: {},
|
||||
}),
|
||||
),
|
||||
};
|
||||
const { scheduler, onAllToolCallsComplete, onToolCallsUpdate } =
|
||||
buildScheduler({ tools, messageBus, disableHooks: false });
|
||||
|
||||
// Not awaited: schedule stays pending while toolB executes.
|
||||
const schedulePromise = scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'a',
|
||||
name: 'toolA',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
{
|
||||
callId: 'b',
|
||||
name: 'toolB',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
// Approve A while toolB's execute is still pending.
|
||||
await waiting.confirmationDetails.onConfirm(
|
||||
ToolConfirmationOutcome.ProceedOnce,
|
||||
);
|
||||
// Let toolB finish — toolA must now run rather than stay stuck.
|
||||
resolveB({ llmContent: 'B ok', returnDisplay: 'B ok' });
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
await schedulePromise;
|
||||
const completed = onAllToolCallsComplete.mock.calls.at(
|
||||
-1,
|
||||
)?.[0] as ToolCall[];
|
||||
expect(aExecute).toHaveBeenCalledTimes(1);
|
||||
expect(completed.find((c) => c.request.callId === 'a')?.status).toBe(
|
||||
'success',
|
||||
);
|
||||
expect(completed.find((c) => c.request.callId === 'b')?.status).toBe(
|
||||
'success',
|
||||
);
|
||||
});
|
||||
|
||||
it('bounces a non-interactive STREAM_JSON ask (client can answer control requests)', async () => {
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'ok',
|
||||
returnDisplay: 'ok',
|
||||
});
|
||||
const messageBus = askMessageBus();
|
||||
const { onToolCallsUpdate } = await scheduleWithAsk({
|
||||
messageBus,
|
||||
execute,
|
||||
isInteractive: false,
|
||||
inputFormat: InputFormat.STREAM_JSON,
|
||||
});
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
expect(waiting.confirmationDetails.type).toBe('info');
|
||||
expect(getBlockedSpans()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('bounces a non-interactive ask under the Zed integration', async () => {
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'ok',
|
||||
returnDisplay: 'ok',
|
||||
});
|
||||
const messageBus = askMessageBus();
|
||||
const { onToolCallsUpdate } = await scheduleWithAsk({
|
||||
messageBus,
|
||||
execute,
|
||||
isInteractive: false,
|
||||
experimentalZedIntegration: true,
|
||||
});
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
expect(waiting.confirmationDetails.type).toBe('info');
|
||||
expect(getBlockedSpans()).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("a sibling's ProceedAlways must not auto-approve a bounced ask", async () => {
|
||||
toolSpanRecords.length = 0;
|
||||
// Both tools bounce on a PreToolUse 'ask'. Approving toolB with
|
||||
// ProceedAlways runs autoApproveCompatiblePendingTools, which must NOT
|
||||
// auto-approve the bounced toolA — its hook 'ask' requires explicit
|
||||
// confirmation, otherwise the hook gate is silently defeated.
|
||||
const aExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'A',
|
||||
returnDisplay: 'A',
|
||||
});
|
||||
const bExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'B',
|
||||
returnDisplay: 'B',
|
||||
});
|
||||
const tools = [
|
||||
new MockTool({ name: 'toolA', kind: Kind.Read, execute: aExecute }),
|
||||
new MockTool({ name: 'toolB', kind: Kind.Read, execute: bExecute }),
|
||||
];
|
||||
const messageBus = {
|
||||
request: vi
|
||||
.fn()
|
||||
.mockImplementation(async (req: { eventName?: string }) => ({
|
||||
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
|
||||
correlationId: 'pre-hook',
|
||||
success: true,
|
||||
output:
|
||||
req.eventName === 'PreToolUse'
|
||||
? { decision: 'ask', reason: 'confirm' }
|
||||
: {},
|
||||
})),
|
||||
};
|
||||
const { scheduler, onToolCallsUpdate } = buildScheduler({
|
||||
tools,
|
||||
messageBus,
|
||||
disableHooks: false,
|
||||
});
|
||||
|
||||
await scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'a',
|
||||
name: 'toolA',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
{
|
||||
callId: 'b',
|
||||
name: 'toolB',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
// Both tools bounce to awaiting_approval.
|
||||
await vi.waitFor(() => {
|
||||
const awaiting = onToolCallsUpdate.mock.calls
|
||||
.flatMap((c) => c[0] as ToolCall[])
|
||||
.filter((tc) => tc.status === 'awaiting_approval')
|
||||
.map((tc) => tc.request.callId);
|
||||
expect(awaiting).toContain('a');
|
||||
expect(awaiting).toContain('b');
|
||||
});
|
||||
|
||||
const toolBWaiting = onToolCallsUpdate.mock.calls
|
||||
.flatMap((c) => c[0] as ToolCall[])
|
||||
.find(
|
||||
(tc) => tc.request.callId === 'b' && tc.status === 'awaiting_approval',
|
||||
) as WaitingToolCall;
|
||||
// Programmatic ProceedAlways on toolB runs autoApproveCompatiblePendingTools
|
||||
// synchronously inside handleConfirmationResponse (awaited here).
|
||||
await toolBWaiting.confirmationDetails.onConfirm(
|
||||
ToolConfirmationOutcome.ProceedAlways,
|
||||
);
|
||||
|
||||
// toolA's hook 'ask' must still gate it: autoApprove skipped the bounced
|
||||
// tool, so it never ran (and stays awaiting the user's own confirmation).
|
||||
// toolB also doesn't run yet — the batch waits while toolA is non-terminal.
|
||||
expect(aExecute).not.toHaveBeenCalled();
|
||||
const latestA = onToolCallsUpdate.mock.calls
|
||||
.flatMap((c) => c[0] as ToolCall[])
|
||||
.filter((tc) => tc.request.callId === 'a')
|
||||
.at(-1);
|
||||
expect(latestA?.status).toBe('awaiting_approval');
|
||||
});
|
||||
|
||||
it('ignores ModifyWithEditor for a bounced ask info confirmation', async () => {
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'ok',
|
||||
returnDisplay: 'ok',
|
||||
});
|
||||
const getModifyContext = vi.fn(() => {
|
||||
throw new Error('info confirmation must not enter editor modify flow');
|
||||
});
|
||||
const tool = Object.assign(
|
||||
new MockTool({
|
||||
name: 'mockTool',
|
||||
kind: Kind.Edit,
|
||||
execute,
|
||||
}),
|
||||
{ getModifyContext },
|
||||
);
|
||||
const messageBus = askMessageBus();
|
||||
const { onToolCallsUpdate } = await scheduleWithAsk({
|
||||
messageBus,
|
||||
execute,
|
||||
tools: [tool],
|
||||
});
|
||||
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
await expect(
|
||||
waiting.confirmationDetails.onConfirm(
|
||||
ToolConfirmationOutcome.ModifyWithEditor,
|
||||
),
|
||||
).resolves.toBeUndefined();
|
||||
|
||||
expect(getModifyContext).not.toHaveBeenCalled();
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
const latest = onToolCallsUpdate.mock.calls
|
||||
.flatMap((c) => c[0] as ToolCall[])
|
||||
.filter((tc) => tc.request.callId === 'ask-call')
|
||||
.at(-1);
|
||||
expect(latest?.status).toBe('awaiting_approval');
|
||||
});
|
||||
|
||||
it('pauses later unsafe batches while a bounced ask awaits approval', async () => {
|
||||
const aExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'A ok',
|
||||
returnDisplay: 'A ok',
|
||||
});
|
||||
const bExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'B ok',
|
||||
returnDisplay: 'B ok',
|
||||
});
|
||||
const tools = [
|
||||
new MockTool({ name: 'toolA', kind: Kind.Edit, execute: aExecute }),
|
||||
new MockTool({ name: 'toolB', kind: Kind.Edit, execute: bExecute }),
|
||||
];
|
||||
const messageBus = {
|
||||
request: vi
|
||||
.fn()
|
||||
.mockImplementation(
|
||||
async (req: {
|
||||
eventName?: string;
|
||||
input?: { tool_name?: string };
|
||||
}) => ({
|
||||
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
|
||||
correlationId: 'pre-hook',
|
||||
success: true,
|
||||
output:
|
||||
req.eventName === 'PreToolUse' && req.input?.tool_name === 'toolA'
|
||||
? { decision: 'ask', reason: 'confirm A' }
|
||||
: {},
|
||||
}),
|
||||
),
|
||||
};
|
||||
const { scheduler, onAllToolCallsComplete, onToolCallsUpdate } =
|
||||
buildScheduler({ tools, messageBus, disableHooks: false });
|
||||
|
||||
await scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'a',
|
||||
name: 'toolA',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
{
|
||||
callId: 'b',
|
||||
name: 'toolB',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
],
|
||||
new AbortController().signal,
|
||||
);
|
||||
|
||||
expect(aExecute).not.toHaveBeenCalled();
|
||||
expect(bExecute).not.toHaveBeenCalled();
|
||||
|
||||
const waiting = onToolCallsUpdate.mock.calls
|
||||
.flatMap((c) => c[0] as ToolCall[])
|
||||
.find(
|
||||
(tc) => tc.request.callId === 'a' && tc.status === 'awaiting_approval',
|
||||
) as WaitingToolCall;
|
||||
expect(waiting).toBeDefined();
|
||||
|
||||
await waiting.confirmationDetails.onConfirm(
|
||||
ToolConfirmationOutcome.ProceedOnce,
|
||||
);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
});
|
||||
expect(aExecute).toHaveBeenCalledTimes(1);
|
||||
expect(bExecute).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('abort drain leaves executing siblings to finish their own abort path', async () => {
|
||||
let resolveB!: (r: ToolResult) => void;
|
||||
const bDone = new Promise<ToolResult>((resolve) => {
|
||||
resolveB = resolve;
|
||||
});
|
||||
const bExecute = vi.fn().mockReturnValue(bDone);
|
||||
const tools = [
|
||||
new MockTool({
|
||||
name: 'toolA',
|
||||
kind: Kind.Read,
|
||||
execute: vi.fn().mockResolvedValue({
|
||||
llmContent: 'A ok',
|
||||
returnDisplay: 'A ok',
|
||||
}),
|
||||
}),
|
||||
new MockTool({ name: 'toolB', kind: Kind.Read, execute: bExecute }),
|
||||
];
|
||||
const messageBus = {
|
||||
request: vi
|
||||
.fn()
|
||||
.mockImplementation(
|
||||
async (req: {
|
||||
eventName?: string;
|
||||
input?: { tool_name?: string };
|
||||
}) => ({
|
||||
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
|
||||
correlationId: 'pre-hook',
|
||||
success: true,
|
||||
output:
|
||||
req.eventName === 'PreToolUse' && req.input?.tool_name === 'toolA'
|
||||
? { decision: 'ask', reason: 'confirm A' }
|
||||
: {},
|
||||
}),
|
||||
),
|
||||
};
|
||||
const abortController = new AbortController();
|
||||
const { scheduler, onToolCallsUpdate } = buildScheduler({
|
||||
tools,
|
||||
messageBus,
|
||||
disableHooks: false,
|
||||
});
|
||||
|
||||
const schedulePromise = scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'a',
|
||||
name: 'toolA',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
{
|
||||
callId: 'b',
|
||||
name: 'toolB',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
],
|
||||
abortController.signal,
|
||||
);
|
||||
|
||||
await waitForStatus(onToolCallsUpdate, 'awaiting_approval');
|
||||
await vi.waitFor(() => {
|
||||
expect(bExecute).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
abortController.abort();
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
const latestB = onToolCallsUpdate.mock.calls
|
||||
.flatMap((c) => c[0] as ToolCall[])
|
||||
.filter((tc) => tc.request.callId === 'b')
|
||||
.at(-1);
|
||||
expect(latestB?.status).toBe('executing');
|
||||
|
||||
resolveB({ llmContent: 'B done', returnDisplay: 'B done' });
|
||||
await schedulePromise;
|
||||
});
|
||||
|
||||
it('abort drain cancels scheduled siblings behind a bounced ask', async () => {
|
||||
const aExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'A ok',
|
||||
returnDisplay: 'A ok',
|
||||
});
|
||||
const bExecute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'B ok',
|
||||
returnDisplay: 'B ok',
|
||||
});
|
||||
const tools = [
|
||||
new MockTool({ name: 'toolA', kind: Kind.Edit, execute: aExecute }),
|
||||
new MockTool({ name: 'toolB', kind: Kind.Edit, execute: bExecute }),
|
||||
];
|
||||
const messageBus = {
|
||||
request: vi
|
||||
.fn()
|
||||
.mockImplementation(
|
||||
async (req: {
|
||||
eventName?: string;
|
||||
input?: { tool_name?: string };
|
||||
}) => ({
|
||||
type: MessageBusType.HOOK_EXECUTION_RESPONSE,
|
||||
correlationId: 'pre-hook',
|
||||
success: true,
|
||||
output:
|
||||
req.eventName === 'PreToolUse' && req.input?.tool_name === 'toolA'
|
||||
? { decision: 'ask', reason: 'confirm A' }
|
||||
: {},
|
||||
}),
|
||||
),
|
||||
};
|
||||
const abortController = new AbortController();
|
||||
const { scheduler, onAllToolCallsComplete, onToolCallsUpdate } =
|
||||
buildScheduler({
|
||||
tools,
|
||||
messageBus,
|
||||
disableHooks: false,
|
||||
});
|
||||
|
||||
await scheduler.schedule(
|
||||
[
|
||||
{
|
||||
callId: 'a',
|
||||
name: 'toolA',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
{
|
||||
callId: 'b',
|
||||
name: 'toolB',
|
||||
args: {},
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p',
|
||||
},
|
||||
],
|
||||
abortController.signal,
|
||||
);
|
||||
|
||||
await waitForStatus(onToolCallsUpdate, 'awaiting_approval');
|
||||
|
||||
abortController.abort();
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
const latestB = onToolCallsUpdate.mock.calls
|
||||
.flatMap((c) => c[0] as ToolCall[])
|
||||
.filter((tc) => tc.request.callId === 'b')
|
||||
.at(-1);
|
||||
expect(latestB?.status).toBe('cancelled');
|
||||
expect(onAllToolCallsComplete).toHaveBeenCalled();
|
||||
expect(aExecute).not.toHaveBeenCalled();
|
||||
expect(bExecute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('cleans bounce markers when post-ask re-execution fails before body runs', async () => {
|
||||
const tracing = await import('../telemetry/session-tracing.js');
|
||||
const runInToolSpanContext = vi.mocked(tracing.runInToolSpanContext);
|
||||
const messageBus = askMessageBus();
|
||||
const { scheduler, onToolCallsUpdate } = await scheduleWithAsk({
|
||||
messageBus,
|
||||
});
|
||||
const waiting = (await waitForStatus(
|
||||
onToolCallsUpdate,
|
||||
'awaiting_approval',
|
||||
)) as WaitingToolCall;
|
||||
|
||||
runInToolSpanContext.mockImplementationOnce(() => {
|
||||
throw new Error('context failed before callback');
|
||||
});
|
||||
|
||||
await expect(
|
||||
waiting.confirmationDetails.onConfirm(
|
||||
ToolConfirmationOutcome.ProceedOnce,
|
||||
),
|
||||
).rejects.toThrow('context failed before callback');
|
||||
|
||||
expect(
|
||||
(scheduler as unknown as { bouncedAwaitingApproval: Set<string> })
|
||||
.bouncedAwaitingApproval.size,
|
||||
).toBe(0);
|
||||
expect(
|
||||
(scheduler as unknown as { bouncedToolUseId: Map<string, string> })
|
||||
.bouncedToolUseId.size,
|
||||
).toBe(0);
|
||||
expect(
|
||||
(scheduler as unknown as { toolSpans: Map<string, unknown> }).toolSpans
|
||||
.size,
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
it('blocked_on_user span ends with cancel when the user rejects (#3731 Phase 2)', async () => {
|
||||
// Reuses MockEditTool — same setup as the existing edit-cancellation
|
||||
// test in `CoreToolScheduler edit cancellation`, just instrumented for
|
||||
|
|
|
|||
|
|
@ -222,7 +222,6 @@ const TOOL_FAILURE_KIND_BACKGROUND_AGENT_DENIED = 'background_agent_denied';
|
|||
|
||||
const TOOL_SPAN_STATUS_PRE_HOOK_BLOCKED = 'Tool execution blocked by hook';
|
||||
|
||||
|
||||
const TOOL_SPAN_STATUS_POST_HOOK_STOPPED = 'Tool execution stopped by hook';
|
||||
const TOOL_SPAN_STATUS_PERMISSION_DENIED = 'Permission denied for tool';
|
||||
const TOOL_SPAN_STATUS_PERMISSION_HOOK_DENIED =
|
||||
|
|
@ -1210,6 +1209,20 @@ export class CoreToolScheduler {
|
|||
// callIdToBatch is drained earlier when spans end, so it cannot be used
|
||||
// to recover the PostToolBatch AbortSignal reliably.
|
||||
private callIdToPostToolBatchSignal = new Map<string, AbortSignal>();
|
||||
// Tool calls that a PreToolUse 'ask' hook bounced from the EXECUTION
|
||||
// phase back to awaiting_approval. Tracked so that, once the user
|
||||
// approves, the re-execution skips BOTH the PreToolUse hook (otherwise
|
||||
// the hook would return 'ask' again → infinite confirmation loop) and
|
||||
// the path-unescape prelude (unescapePath is not idempotent — running
|
||||
// it twice corrupts paths containing escaped metacharacters). Cleared
|
||||
// on terminal state via finalizeToolSpan.
|
||||
private readonly bouncedAwaitingApproval = new Set<string>();
|
||||
// Original tool_use_id captured when a tool is bounced by a PreToolUse
|
||||
// 'ask', keyed by callId. The first (bounced) attempt fires PreToolUse
|
||||
// with this id; the post-approval re-execution skips PreToolUse but fires
|
||||
// PostToolUse — reusing this id keeps the Pre/Post pair correlated instead
|
||||
// of orphaning two events. Cleared on terminal state via finalizeToolSpan.
|
||||
private readonly bouncedToolUseId = new Map<string, string>();
|
||||
private requestQueue: Array<{
|
||||
request: ToolCallRequestInfo | ToolCallRequestInfo[];
|
||||
signal: AbortSignal;
|
||||
|
|
@ -1499,6 +1512,12 @@ export class CoreToolScheduler {
|
|||
* `setToolSpan{Failure,Cancelled,Ok}` before this call (#4321 review).
|
||||
*/
|
||||
private finalizeToolSpan(callId: string): void {
|
||||
// Terminal-state cleanup: drop any PreToolUse 'ask' bounce markers so
|
||||
// they never leak past the tool call's lifetime. Done unconditionally
|
||||
// (before the span guard) so a bounced call is cleared even on the
|
||||
// defensive no-span path.
|
||||
this.bouncedAwaitingApproval.delete(callId);
|
||||
this.bouncedToolUseId.delete(callId);
|
||||
const span = this.toolSpans.get(callId);
|
||||
if (!span) return;
|
||||
this.toolSpans.delete(callId);
|
||||
|
|
@ -1587,6 +1606,22 @@ export class CoreToolScheduler {
|
|||
// remaining entries — the timer callback would otherwise surface
|
||||
// an unhandled exception (#4321 review-3 wenshao Suggestion).
|
||||
try {
|
||||
const call = this.toolCalls.find((c) => c.request.callId === callId);
|
||||
if (
|
||||
call?.status !== 'awaiting_approval' &&
|
||||
call?.status !== 'scheduled'
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
// Safety-net for a tool stuck awaiting approval, or still scheduled
|
||||
// behind such a tool, at abort time. Do not force-terminalize
|
||||
// executing siblings; their own abort path owns live output, failure
|
||||
// hooks, and final status.
|
||||
this.setStatusInternal(
|
||||
callId,
|
||||
'cancelled',
|
||||
'Tool call cancelled by user.',
|
||||
);
|
||||
if (this.blockedSpans.has(callId)) {
|
||||
this.finalizeBlockedSpan(callId, 'aborted', 'system');
|
||||
}
|
||||
|
|
@ -2882,7 +2917,10 @@ export class CoreToolScheduler {
|
|||
this.finalizeToolSpan(callId);
|
||||
} else if (outcome === ToolConfirmationOutcome.ModifyWithEditor) {
|
||||
const waitingToolCall = toolCall as WaitingToolCall;
|
||||
if (isModifiableDeclarativeTool(waitingToolCall.tool)) {
|
||||
if (
|
||||
waitingToolCall.confirmationDetails.type === 'edit' &&
|
||||
isModifiableDeclarativeTool(waitingToolCall.tool)
|
||||
) {
|
||||
const modifyContext = waitingToolCall.tool.getModifyContext(signal);
|
||||
const editorType = this.getPreferredEditor();
|
||||
if (!editorType) {
|
||||
|
|
@ -2921,15 +2959,21 @@ export class CoreToolScheduler {
|
|||
|
||||
// Normalize shell-escaped paths so the editor receives actual
|
||||
// filesystem paths (request.args may still hold escaped values
|
||||
// since buildInvocation normalizes a structuredClone).
|
||||
// since buildInvocation normalizes a structuredClone) — UNLESS this
|
||||
// tool was bounced by a PreToolUse 'ask', in which case
|
||||
// _executeToolCallBody already unescaped request.args in place
|
||||
// before the hook fired. Unescaping again here would double-strip
|
||||
// and corrupt paths containing escaped metacharacters.
|
||||
const normalizedArgs = {
|
||||
...waitingToolCall.request.args,
|
||||
} as typeof waitingToolCall.request.args;
|
||||
for (const key of PATH_ARG_KEYS) {
|
||||
if (typeof normalizedArgs[key] === 'string') {
|
||||
(normalizedArgs as Record<string, unknown>)[key] = unescapePath(
|
||||
String(normalizedArgs[key]).trim(),
|
||||
);
|
||||
if (!this.bouncedAwaitingApproval.has(callId)) {
|
||||
for (const key of PATH_ARG_KEYS) {
|
||||
if (typeof normalizedArgs[key] === 'string') {
|
||||
(normalizedArgs as Record<string, unknown>)[key] = unescapePath(
|
||||
String(normalizedArgs[key]).trim(),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
const { updatedParams, updatedDiff } = await modifyWithEditor<
|
||||
|
|
@ -3124,18 +3168,34 @@ export class CoreToolScheduler {
|
|||
private async attemptExecutionOfScheduledCalls(
|
||||
signal: AbortSignal,
|
||||
): Promise<void> {
|
||||
const allCallsFinalOrScheduled = this.toolCalls.every(
|
||||
(call) =>
|
||||
call.status === 'scheduled' ||
|
||||
call.status === 'cancelled' ||
|
||||
call.status === 'success' ||
|
||||
call.status === 'error',
|
||||
);
|
||||
// Loop rather than execute once: a tool bounced to awaiting_approval by a
|
||||
// PreToolUse 'ask' can be approved (→ 'scheduled') while a sibling in the
|
||||
// same batch is still executing. The guard below fails on that pass, and
|
||||
// nothing else retriggers execution once the sibling finishes — so after
|
||||
// each batch drains, re-check for a newly-scheduled bounce-approved tool.
|
||||
// Each iteration either drains ≥1 'scheduled' call or returns, so this
|
||||
// cannot spin: a re-bounce lands back in awaiting_approval (guard fails →
|
||||
// return), and a clean run leaves nothing 'scheduled' (length 0 → return).
|
||||
while (true) {
|
||||
const allCallsFinalOrScheduled = this.toolCalls.every(
|
||||
(call) =>
|
||||
call.status === 'scheduled' ||
|
||||
call.status === 'cancelled' ||
|
||||
call.status === 'success' ||
|
||||
call.status === 'error',
|
||||
);
|
||||
if (!allCallsFinalOrScheduled) {
|
||||
// Something is still executing or awaiting approval; its own
|
||||
// completion path (or handleConfirmationResponse) re-enters here.
|
||||
return;
|
||||
}
|
||||
|
||||
if (allCallsFinalOrScheduled) {
|
||||
const callsToExecute = this.toolCalls.filter(
|
||||
(call): call is ScheduledToolCall => call.status === 'scheduled',
|
||||
);
|
||||
if (callsToExecute.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Partition tool calls into consecutive batches by concurrency safety.
|
||||
// Consecutive safe tools are grouped into parallel batches; unsafe
|
||||
|
|
@ -3146,15 +3206,28 @@ export class CoreToolScheduler {
|
|||
for (const batch of batches) {
|
||||
if (batch.concurrent && batch.calls.length > 1) {
|
||||
await this.runConcurrently(batch.calls, signal);
|
||||
if (this.hasExecutingOrAwaitingApprovalCall()) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
for (const call of batch.calls) {
|
||||
await this.executeSingleToolCall(call, signal);
|
||||
if (this.hasExecutingOrAwaitingApprovalCall()) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private hasExecutingOrAwaitingApprovalCall(): boolean {
|
||||
return this.toolCalls.some(
|
||||
(call) =>
|
||||
call.status === 'executing' || call.status === 'awaiting_approval',
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Execute multiple tool calls concurrently with a concurrency cap.
|
||||
*/
|
||||
|
|
@ -3211,6 +3284,8 @@ export class CoreToolScheduler {
|
|||
this._executeToolCallBody(scheduledCall, signal, toolSpan),
|
||||
);
|
||||
} catch (error) {
|
||||
this.bouncedAwaitingApproval.delete(callId);
|
||||
this.bouncedToolUseId.delete(callId);
|
||||
// _executeToolCallBody pre-sets span status (OK / FAILURE /
|
||||
// CANCELLED) only AFTER its main try/catch is entered. Throws
|
||||
// from the prelude — getMessageBus,
|
||||
|
|
@ -3244,13 +3319,114 @@ export class CoreToolScheduler {
|
|||
);
|
||||
throw error;
|
||||
} finally {
|
||||
// _executeToolCallBody pre-sets status (OK / FAILURE / CANCELLED) via
|
||||
// setToolSpan*; finalize without metadata to preserve that.
|
||||
this.finalizeToolSpan(callId);
|
||||
// A PreToolUse 'ask' hook can bounce this tool back to
|
||||
// awaiting_approval (see bounceToAwaitingApprovalForAsk). The tool
|
||||
// span must then stay open until handleConfirmationResponse resolves
|
||||
// the confirmation — finalizing here would orphan it and the
|
||||
// re-execution would open a second span. The re-execution consumes
|
||||
// the marker before running, so the post-approval finally finalizes
|
||||
// normally. Checking the marker (not a re-read of tool status) avoids
|
||||
// a race where a STREAM_JSON client answers the confirmation
|
||||
// synchronously and flips status to 'scheduled' before this runs.
|
||||
if (!this.bouncedAwaitingApproval.has(callId)) {
|
||||
// _executeToolCallBody pre-sets status (OK / FAILURE / CANCELLED)
|
||||
// via setToolSpan*; finalize without metadata to preserve that.
|
||||
this.finalizeToolSpan(callId);
|
||||
}
|
||||
this.memoryMonitor?.scheduleCheck();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a PreToolUse 'ask' decision can be surfaced as an interactive
|
||||
* TUI confirmation. Mirrors the confirmation-phase guards: a
|
||||
* non-interactive CLI (unless STREAM_JSON, which can answer control
|
||||
* requests) and background agents cannot prompt, so an 'ask' there must
|
||||
* fall back to deny rather than hang forever in awaiting_approval.
|
||||
*/
|
||||
private canPromptForAskBounce(): boolean {
|
||||
const isNonInteractive =
|
||||
!this.config.isInteractive() &&
|
||||
!this.config.getExperimentalZedIntegration() &&
|
||||
this.config.getInputFormat() !== InputFormat.STREAM_JSON;
|
||||
if (isNonInteractive) {
|
||||
return false;
|
||||
}
|
||||
if (this.config.getShouldAvoidPermissionPrompts?.()) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Bounce a tool from the EXECUTION phase back to awaiting_approval so the
|
||||
* user can confirm a PreToolUse 'ask' decision in the TUI. Reuses the
|
||||
* standard confirmation machinery: a synthetic 'info' confirmation whose
|
||||
* onConfirm routes through handleConfirmationResponse (ProceedOnce →
|
||||
* re-execute, Cancel → cancelled). `hideAlwaysAllow` is set because the
|
||||
* hook re-evaluates on every call, so an "always allow" rule is
|
||||
* meaningless. The callId is added to `bouncedAwaitingApproval` BEFORE
|
||||
* the status change so executeSingleToolCall's finally keeps the tool
|
||||
* span open across the bounce and the re-execution skips the hook +
|
||||
* prelude (see `_executeToolCallBody`).
|
||||
*/
|
||||
private bounceToAwaitingApprovalForAsk(
|
||||
scheduledCall: ScheduledToolCall,
|
||||
reason: string | undefined,
|
||||
toolSpan: Span,
|
||||
signal: AbortSignal,
|
||||
): void {
|
||||
const { callId, name: toolName } = scheduledCall.request;
|
||||
const canonicalName = canonicalToolName(toolName);
|
||||
|
||||
this.bouncedAwaitingApproval.add(callId);
|
||||
|
||||
const confirmationDetails: ToolCallConfirmationDetails = {
|
||||
type: 'info',
|
||||
title: `Hook requested confirmation to run ${toolName}`,
|
||||
prompt:
|
||||
reason ||
|
||||
`A PreToolUse hook requested confirmation before running ${toolName}.`,
|
||||
hideAlwaysAllow: true,
|
||||
onConfirm: (outcome, payload) =>
|
||||
this.handleConfirmationResponse(
|
||||
callId,
|
||||
// No real tool onConfirm — for this synthetic prompt all of the
|
||||
// approve/deny handling lives in handleConfirmationResponse.
|
||||
async () => {},
|
||||
outcome,
|
||||
signal,
|
||||
payload,
|
||||
),
|
||||
};
|
||||
|
||||
this.setStatusInternal(callId, 'awaiting_approval', confirmationDetails);
|
||||
|
||||
// blocked_on_user span as a child of the tool span — mirrors the
|
||||
// confirmation-phase setup so walk-away aborts and finalize paths
|
||||
// behave identically.
|
||||
const blockedSpan = startToolBlockedOnUserSpan(toolSpan, {
|
||||
tool_name: canonicalName,
|
||||
call_id: callId,
|
||||
});
|
||||
this.blockedSpans.set(callId, blockedSpan);
|
||||
|
||||
// Surface the prompt the same way the confirmation phase does.
|
||||
const messageBus = this.config.getMessageBus() as MessageBus | undefined;
|
||||
if (!this.config.getDisableAllHooks() && messageBus) {
|
||||
fireNotificationHook(
|
||||
messageBus,
|
||||
`Qwen Code needs your permission to use ${toolName}`,
|
||||
NotificationType.PermissionPrompt,
|
||||
'Permission needed',
|
||||
).catch((error) => {
|
||||
debugLogger.warn(
|
||||
`Permission prompt notification hook failed: ${error instanceof Error ? error.message : String(error)}`,
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
private safelyAddToolInputAttributes(
|
||||
span: Span,
|
||||
toolName: string,
|
||||
|
|
@ -3285,11 +3461,22 @@ export class CoreToolScheduler {
|
|||
const invocation = scheduledCall.invocation;
|
||||
const toolInput = scheduledCall.request.args as Record<string, unknown>;
|
||||
|
||||
// Normalize shell-escaped path params so hooks operate on actual filesystem
|
||||
// paths, matching the normalization done in tool validation.
|
||||
for (const key of PATH_ARG_KEYS) {
|
||||
if (typeof toolInput[key] === 'string') {
|
||||
toolInput[key] = unescapePath(String(toolInput[key]).trim());
|
||||
// Re-execution after the user approved a PreToolUse 'ask' bounce: the
|
||||
// hook already ran and the user already confirmed. Consuming the marker
|
||||
// here (a) lets executeSingleToolCall's finally finalize the span
|
||||
// normally for this run, and (b) signals that we must skip BOTH the
|
||||
// hook re-fire below (otherwise the hook returns 'ask' again → infinite
|
||||
// confirmation loop) and the path-unescape prelude (unescapePath is not
|
||||
// idempotent — running it twice corrupts paths with escaped metachars).
|
||||
const isPostAskReexecution = this.bouncedAwaitingApproval.delete(callId);
|
||||
|
||||
if (!isPostAskReexecution) {
|
||||
// Normalize shell-escaped path params so hooks operate on actual
|
||||
// filesystem paths, matching the normalization done in tool validation.
|
||||
for (const key of PATH_ARG_KEYS) {
|
||||
if (typeof toolInput[key] === 'string') {
|
||||
toolInput[key] = unescapePath(String(toolInput[key]).trim());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -3304,15 +3491,22 @@ export class CoreToolScheduler {
|
|||
);
|
||||
}
|
||||
|
||||
// Generate unique tool_use_id for hook tracking
|
||||
const toolUseId = generateToolUseId();
|
||||
// Generate unique tool_use_id for hook tracking. On a post-'ask'
|
||||
// re-execution, reuse the id from the first (bounced) attempt so the
|
||||
// PreToolUse event fired then pairs with the PostToolUse event fired now
|
||||
// — a fresh id would leave both events orphaned for consumers that
|
||||
// correlate Pre/Post by tool_use_id (audit trails, metrics).
|
||||
const toolUseId = isPostAskReexecution
|
||||
? (this.bouncedToolUseId.get(callId) ?? generateToolUseId())
|
||||
: generateToolUseId();
|
||||
|
||||
// Get MessageBus for hook execution
|
||||
const messageBus = this.config.getMessageBus() as MessageBus | undefined;
|
||||
const hooksEnabled = !this.config.getDisableAllHooks();
|
||||
|
||||
// PreToolUse Hook
|
||||
if (hooksEnabled && messageBus) {
|
||||
// PreToolUse Hook — skipped on a post-'ask' re-execution (the hook
|
||||
// already ran and the user already confirmed; re-firing would loop).
|
||||
if (hooksEnabled && messageBus && !isPostAskReexecution) {
|
||||
// Convert ApprovalMode to permission_mode string for hooks
|
||||
const permissionMode = this.config.getApprovalMode();
|
||||
const preHookResult = await this.withHookSpan(
|
||||
|
|
@ -3349,7 +3543,33 @@ export class CoreToolScheduler {
|
|||
},
|
||||
);
|
||||
if (!preHookResult.shouldProceed) {
|
||||
// Hook blocked the execution
|
||||
// A PreToolUse hook returning permissionDecision:'ask' wants the
|
||||
// user to confirm in the TUI before the tool runs. When we can
|
||||
// prompt, bounce the tool into the existing awaiting_approval flow
|
||||
// instead of denying it. 'denied'/'stop' (and 'ask' in a
|
||||
// non-interactive/background context where we cannot prompt) keep
|
||||
// the original deny-as-error behavior.
|
||||
if (
|
||||
preHookResult.blockType === 'ask' &&
|
||||
!signal.aborted &&
|
||||
this.canPromptForAskBounce()
|
||||
) {
|
||||
// Mirror the confirmation-phase abort re-check: never open a
|
||||
// transient awaiting_approval (flashing a confirmation nobody can
|
||||
// answer) on an already-aborted signal — fall through to deny.
|
||||
// Preserve the tool_use_id so the post-approval re-execution
|
||||
// reuses it (see the toolUseId comment above).
|
||||
this.bouncedToolUseId.set(callId, toolUseId);
|
||||
this.bounceToAwaitingApprovalForAsk(
|
||||
scheduledCall,
|
||||
preHookResult.blockReason,
|
||||
span,
|
||||
signal,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Hook blocked the execution.
|
||||
const blockMessage =
|
||||
preHookResult.blockReason || 'Tool execution blocked by hook';
|
||||
const errorResponse = createErrorResponse(
|
||||
|
|
@ -4485,7 +4705,13 @@ export class CoreToolScheduler {
|
|||
const pendingTools = this.toolCalls.filter(
|
||||
(call) =>
|
||||
call.status === 'awaiting_approval' &&
|
||||
call.request.callId !== triggeringCallId,
|
||||
call.request.callId !== triggeringCallId &&
|
||||
// A tool bounced by a PreToolUse 'ask' must NOT be auto-approved as a
|
||||
// side effect of approving a sibling: the hook explicitly requested
|
||||
// confirmation, and re-execution skips the hook — auto-approving here
|
||||
// would silently defeat the hook's gate. It requires its own explicit
|
||||
// user confirmation.
|
||||
!this.bouncedAwaitingApproval.has(call.request.callId),
|
||||
) as WaitingToolCall[];
|
||||
|
||||
for (const pendingTool of pendingTools) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue