fix(acp): align permission flow across clients

This commit is contained in:
LaZzyMan 2026-03-26 23:25:04 +08:00
parent 52a573f10f
commit dd518de5b0
26 changed files with 1890 additions and 1082 deletions

View file

@ -57,7 +57,7 @@ import { buildAuthMethods } from './authMethods.js';
import { AcpFileSystemService } from './service/filesystem.js';
import { Readable, Writable } from 'node:stream';
import type { LoadedSettings } from '../config/settings.js';
import { SettingScope } from '../config/settings.js';
import { loadSettings, SettingScope } from '../config/settings.js';
import type { ApprovalModeValue } from './session/types.js';
import { z } from 'zod';
import type { CliArgs } from '../config/config.js';
@ -223,30 +223,18 @@ class QwenAgent implements Agent {
return sessionService.sessionExists(params.sessionId);
},
);
if (!exists) {
throw RequestError.invalidParams(
undefined,
`Session not found for id: ${params.sessionId}`,
);
}
const config = await this.newSessionConfig(
params.cwd,
params.mcpServers,
params.sessionId,
exists,
);
await this.ensureAuthenticated(config);
this.setupFileSystem(config);
const sessionData = config.getResumedSessionData();
if (!sessionData) {
throw RequestError.internalError(
undefined,
`Failed to load session data for id: ${params.sessionId}`,
);
}
await this.createAndStoreSession(config, sessionData.conversation);
await this.createAndStoreSession(config, sessionData?.conversation);
const modesData = this.buildModesData(config);
const availableModels = this.buildAvailableModels(config);
@ -380,7 +368,9 @@ class QwenAgent implements Agent {
cwd: string,
mcpServers: McpServer[],
sessionId?: string,
resume?: boolean,
): Promise<Config> {
this.settings = loadSettings(cwd);
const mergedMcpServers = { ...this.settings.merged.mcpServers };
for (const server of mcpServers) {
@ -402,11 +392,11 @@ class QwenAgent implements Agent {
const settings = { ...this.settings.merged, mcpServers: mergedMcpServers };
const argvForSession = {
...this.argv,
resume: sessionId,
...(resume ? { resume: sessionId } : { sessionId }),
continue: false,
};
const config = await loadCliConfig(settings, argvForSession, cwd);
const config = await loadCliConfig(settings, argvForSession, cwd, []);
await config.initialize();
return config;
}

View file

@ -34,6 +34,7 @@ describe('Session', () => {
let currentAuthType: AuthType;
let switchModelSpy: ReturnType<typeof vi.fn>;
let getAvailableCommandsSpy: ReturnType<typeof vi.fn>;
let mockToolRegistry: { getTool: ReturnType<typeof vi.fn> };
beforeEach(() => {
currentModel = 'qwen3-code-plus';
@ -50,7 +51,7 @@ describe('Session', () => {
addHistory: vi.fn(),
} as unknown as GeminiChat;
const toolRegistry = { getTool: vi.fn() };
mockToolRegistry = { getTool: vi.fn() };
const fileService = { shouldGitIgnoreFile: vi.fn().mockReturnValue(false) };
mockConfig = {
@ -65,8 +66,9 @@ describe('Session', () => {
getChatRecordingService: vi.fn().mockReturnValue({
recordUserMessage: vi.fn(),
recordUiTelemetryEvent: vi.fn(),
recordToolResult: vi.fn(),
}),
getToolRegistry: vi.fn().mockReturnValue(toolRegistry),
getToolRegistry: vi.fn().mockReturnValue(mockToolRegistry),
getFileService: vi.fn().mockReturnValue(fileService),
getFileFilteringRespectGitIgnore: vi.fn().mockReturnValue(true),
getEnableRecursiveFileSearch: vi.fn().mockReturnValue(false),
@ -275,5 +277,147 @@ describe('Session', () => {
expect.any(Function),
);
});
it('hides allow-always options when confirmation already forbids them', async () => {
const executeSpy = vi.fn().mockResolvedValue({
llmContent: 'ok',
returnDisplay: 'ok',
});
const onConfirmSpy = vi.fn().mockResolvedValue(undefined);
const invocation = {
params: { path: '/tmp/file.txt' },
getDefaultPermission: vi.fn().mockResolvedValue('ask'),
getConfirmationDetails: vi.fn().mockResolvedValue({
type: 'info',
title: 'Need permission',
prompt: 'Allow?',
hideAlwaysAllow: true,
onConfirm: onConfirmSpy,
}),
getDescription: vi.fn().mockReturnValue('Inspect file'),
toolLocations: vi.fn().mockReturnValue([]),
execute: executeSpy,
};
const tool = {
name: 'read_file',
kind: core.Kind.Read,
build: vi.fn().mockReturnValue(invocation),
};
mockToolRegistry.getTool.mockReturnValue(tool);
mockConfig.getApprovalMode = vi
.fn()
.mockReturnValue(ApprovalMode.DEFAULT);
mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
mockChat.sendMessageStream = vi.fn().mockResolvedValue(
(async function* () {
yield {
type: core.StreamEventType.CHUNK,
value: {
functionCalls: [
{
id: 'call-1',
name: 'read_file',
args: { path: '/tmp/file.txt' },
},
],
},
};
})(),
);
await session.prompt({
sessionId: 'test-session-id',
prompt: [{ type: 'text', text: 'run tool' }],
});
expect(mockClient.requestPermission).toHaveBeenCalledWith(
expect.objectContaining({
options: [
expect.objectContaining({ kind: 'allow_once' }),
expect.objectContaining({ kind: 'reject_once' }),
],
}),
);
const options = (mockClient.requestPermission as ReturnType<typeof vi.fn>)
.mock.calls[0][0].options as Array<{ kind: string }>;
expect(options.some((option) => option.kind === 'allow_always')).toBe(
false,
);
});
it('respects permission-request hook allow decisions without opening ACP permission dialog', async () => {
const hookSpy = vi
.spyOn(core, 'firePermissionRequestHook')
.mockResolvedValue({
hasDecision: true,
shouldAllow: true,
updatedInput: { path: '/tmp/updated.txt' },
denyMessage: undefined,
});
const executeSpy = vi.fn().mockResolvedValue({
llmContent: 'ok',
returnDisplay: 'ok',
});
const onConfirmSpy = vi.fn().mockResolvedValue(undefined);
const invocation = {
params: { path: '/tmp/original.txt' },
getDefaultPermission: vi.fn().mockResolvedValue('ask'),
getConfirmationDetails: vi.fn().mockResolvedValue({
type: 'info',
title: 'Need permission',
prompt: 'Allow?',
onConfirm: onConfirmSpy,
}),
getDescription: vi.fn().mockReturnValue('Inspect file'),
toolLocations: vi.fn().mockReturnValue([]),
execute: executeSpy,
};
const tool = {
name: 'read_file',
kind: core.Kind.Read,
build: vi.fn().mockReturnValue(invocation),
};
mockToolRegistry.getTool.mockReturnValue(tool);
mockConfig.getApprovalMode = vi
.fn()
.mockReturnValue(ApprovalMode.DEFAULT);
mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
mockConfig.getEnableHooks = vi.fn().mockReturnValue(true);
mockConfig.getMessageBus = vi.fn().mockReturnValue({});
mockChat.sendMessageStream = vi.fn().mockResolvedValue(
(async function* () {
yield {
type: core.StreamEventType.CHUNK,
value: {
functionCalls: [
{
id: 'call-2',
name: 'read_file',
args: { path: '/tmp/original.txt' },
},
],
},
};
})(),
);
try {
await session.prompt({
sessionId: 'test-session-id',
prompt: [{ type: 'text', text: 'run tool' }],
});
} finally {
hookSpy.mockRestore();
}
expect(mockClient.requestPermission).not.toHaveBeenCalled();
expect(onConfirmSpy).toHaveBeenCalledWith(
core.ToolConfirmationOutcome.ProceedOnce,
);
expect(invocation.params).toEqual({ path: '/tmp/updated.txt' });
expect(executeSpy).toHaveBeenCalled();
});
});
});

View file

@ -36,6 +36,13 @@ import {
readManyFiles,
Storage,
ToolNames,
buildPermissionCheckContext,
evaluatePermissionRules,
fireNotificationHook,
firePermissionRequestHook,
injectPermissionRulesIfMissing,
NotificationType,
persistPermissionOutcome,
} from '@qwen-code/qwen-code-core';
import { RequestError } from '@agentclientprotocol/sdk';
@ -43,7 +50,6 @@ import type {
AvailableCommand,
ContentBlock,
EmbeddedResourceResource,
PermissionOption,
PromptRequest,
PromptResponse,
RequestPermissionRequest,
@ -54,7 +60,6 @@ import type {
SetSessionModeResponse,
SetSessionModelRequest,
SetSessionModelResponse,
ToolCallContent,
AgentSideConnection,
} from '@agentclientprotocol/sdk';
import type { LoadedSettings } from '../../config/settings.js';
@ -79,6 +84,10 @@ import { ToolCallEmitter } from './emitters/ToolCallEmitter.js';
import { PlanEmitter } from './emitters/PlanEmitter.js';
import { MessageEmitter } from './emitters/MessageEmitter.js';
import { SubAgentTracker } from './SubAgentTracker.js';
import {
buildPermissionRequestContent,
toPermissionOptions,
} from './permissionUtils.js';
const debugLogger = createDebugLogger('SESSION');
@ -487,13 +496,34 @@ export class Session implements SessionContext {
await this.sendUpdate(update);
}
private async resolveIdeDiffForOutcome(
confirmationDetails: ToolCallConfirmationDetails,
outcome: ToolConfirmationOutcome,
): Promise<void> {
if (
confirmationDetails.type !== 'edit' ||
!confirmationDetails.ideConfirmation
) {
return;
}
const { IdeClient } = await import('@qwen-code/qwen-code-core');
const ideClient = await IdeClient.getInstance();
const cliOutcome =
outcome === ToolConfirmationOutcome.Cancel ? 'rejected' : 'accepted';
await ideClient.resolveDiffFromCli(
confirmationDetails.filePath,
cliOutcome as 'accepted' | 'rejected',
);
}
private async runTool(
abortSignal: AbortSignal,
promptId: string,
fc: FunctionCall,
): Promise<Part[]> {
const callId = fc.id ?? `${fc.name}-${Date.now()}`;
const args = (fc.args ?? {}) as Record<string, unknown>;
let args = (fc.args ?? {}) as Record<string, unknown>;
const startTime = Date.now();
@ -526,19 +556,49 @@ export class Session implements SessionContext {
];
};
const earlyErrorResponse = async (
error: Error,
toolName = fc.name ?? 'unknown_tool',
) => {
if (toolName !== TodoWriteTool.Name) {
await this.toolCallEmitter.emitError(callId, toolName, error);
}
const errorParts = errorResponse(error);
this.config.getChatRecordingService()?.recordToolResult(errorParts, {
callId,
status: 'error',
resultDisplay: undefined,
error,
errorType: undefined,
});
return errorParts;
};
if (!fc.name) {
return errorResponse(new Error('Missing function name'));
return earlyErrorResponse(new Error('Missing function name'));
}
const toolRegistry = this.config.getToolRegistry();
const tool = toolRegistry.getTool(fc.name as string);
if (!tool) {
return errorResponse(
return earlyErrorResponse(
new Error(`Tool "${fc.name}" not found in registry.`),
);
}
// ---- L1: Tool enablement check ----
const pm = this.config.getPermissionManager?.();
if (pm && !pm.isToolEnabled(fc.name as string)) {
return earlyErrorResponse(
new Error(
`Qwen Code requires permission to use "${fc.name}", but that permission was declined.`,
),
fc.name,
);
}
// Detect TodoWriteTool early - route to plan updates instead of tool_call events
const isTodoWriteTool = tool.name === TodoWriteTool.Name;
const isAgentTool = tool.name === AgentTool.Name;
@ -577,127 +637,238 @@ export class Session implements SessionContext {
);
}
// Use the new permission flow: getDefaultPermission + getConfirmationDetails
// ask_user_question must always go through confirmation even in YOLO mode
// so the user always has a chance to respond to questions.
// L3→L4→L5 Permission Flow (aligned with coreToolScheduler)
//
// L3: Tool's intrinsic default permission
// L4: PermissionManager rule override
// L5: ApprovalMode override (YOLO / AUTO_EDIT / PLAN)
//
// AUTO_EDIT auto-approval is handled HERE, same as coreToolScheduler.
// The VS Code extension is just a UI layer for requestPermission.
const isAskUserQuestionTool = fc.name === ToolNames.ASK_USER_QUESTION;
// ---- L3: Tool's default permission ----
// In YOLO mode, force 'allow' for everything except ask_user_question.
const defaultPermission =
this.config.getApprovalMode() !== ApprovalMode.YOLO ||
isAskUserQuestionTool
? await invocation.getDefaultPermission()
: 'allow';
const needsConfirmation = defaultPermission === 'ask';
// ---- L4: PermissionManager override (if relevant rules exist) ----
const toolParams = invocation.params as Record<string, unknown>;
const pmCtx = buildPermissionCheckContext(
fc.name,
toolParams,
this.config.getTargetDir?.() ?? '',
);
const { finalPermission, pmForcedAsk } = await evaluatePermissionRules(
pm,
defaultPermission,
pmCtx,
);
// Check for plan mode enforcement - block non-read-only tools
// but allow ask_user_question so users can answer clarification questions
const isPlanMode = this.config.getApprovalMode() === ApprovalMode.PLAN;
const needsConfirmation = finalPermission === 'ask';
// ---- L5: ApprovalMode overrides ----
const approvalMode = this.config.getApprovalMode();
const isPlanMode = approvalMode === ApprovalMode.PLAN;
// PLAN mode: block non-read-only tools
if (
isPlanMode &&
!isExitPlanModeTool &&
!isAskUserQuestionTool &&
needsConfirmation
) {
// In plan mode, block any tool that requires confirmation (write operations)
return errorResponse(
return earlyErrorResponse(
new Error(
`Plan mode is active. The tool "${fc.name}" cannot be executed because it modifies the system. ` +
'Please use the exit_plan_mode tool to present your plan and exit plan mode before making changes.',
),
fc.name,
);
}
if (defaultPermission === 'deny') {
return errorResponse(
if (finalPermission === 'deny') {
return earlyErrorResponse(
new Error(
`Tool "${fc.name}" is denied: command substitution is not allowed for security reasons.`,
defaultPermission === 'deny'
? `Tool "${fc.name}" is denied: command substitution is not allowed for security reasons.`
: `Tool "${fc.name}" is denied by permission rules.`,
),
fc.name,
);
}
let didRequestPermission = false;
if (needsConfirmation) {
const confirmationDetails =
await invocation.getConfirmationDetails(abortSignal);
const content: ToolCallContent[] = [];
if (confirmationDetails.type === 'edit') {
content.push({
type: 'diff',
path: confirmationDetails.fileName,
oldText: confirmationDetails.originalContent,
newText: confirmationDetails.newContent,
});
}
// Centralised rule injection (for display and persistence)
injectPermissionRulesIfMissing(confirmationDetails, pmCtx);
// Add plan content for exit_plan_mode
if (confirmationDetails.type === 'plan') {
content.push({
type: 'content',
content: {
type: 'text',
text: confirmationDetails.plan,
},
});
}
const messageBus = this.config.getMessageBus?.();
const hooksEnabled = this.config.getEnableHooks?.() ?? false;
let hookHandled = false;
// Map tool kind, using switch_mode for exit_plan_mode per ACP spec
const mappedKind = this.toolCallEmitter.mapToolKind(tool.kind, fc.name);
if (hooksEnabled && messageBus) {
const hookResult = await firePermissionRequestHook(
messageBus,
fc.name,
args,
String(approvalMode),
);
const params: RequestPermissionRequest = {
sessionId: this.sessionId,
options: toPermissionOptions(confirmationDetails),
toolCall: {
toolCallId: callId,
status: 'pending',
title: invocation.getDescription(),
content,
locations: invocation.toolLocations(),
kind: mappedKind,
rawInput: args,
},
};
if (hookResult.hasDecision) {
hookHandled = true;
if (hookResult.shouldAllow) {
if (hookResult.updatedInput) {
args = hookResult.updatedInput;
invocation.params =
hookResult.updatedInput as typeof invocation.params;
}
const output = (await this.client.requestPermission(
params,
)) as RequestPermissionResponse & {
answers?: Record<string, string>;
};
const outcome =
output.outcome.outcome === 'cancelled'
? ToolConfirmationOutcome.Cancel
: z
.nativeEnum(ToolConfirmationOutcome)
.parse(output.outcome.optionId);
await confirmationDetails.onConfirm(outcome, {
answers: output.answers,
});
// After exit_plan_mode confirmation, send current_mode_update notification
if (isExitPlanModeTool && outcome !== ToolConfirmationOutcome.Cancel) {
await this.sendCurrentModeUpdateNotification(outcome);
}
switch (outcome) {
case ToolConfirmationOutcome.Cancel:
return errorResponse(
new Error(`Tool "${fc.name}" was canceled by the user.`),
);
case ToolConfirmationOutcome.ProceedOnce:
case ToolConfirmationOutcome.ProceedAlways:
case ToolConfirmationOutcome.ProceedAlwaysProject:
case ToolConfirmationOutcome.ProceedAlwaysUser:
case ToolConfirmationOutcome.ProceedAlwaysServer:
case ToolConfirmationOutcome.ProceedAlwaysTool:
case ToolConfirmationOutcome.ModifyWithEditor:
break;
default: {
const resultOutcome: never = outcome;
throw new Error(`Unexpected: ${resultOutcome}`);
await this.resolveIdeDiffForOutcome(
confirmationDetails,
ToolConfirmationOutcome.ProceedOnce,
);
await confirmationDetails.onConfirm(
ToolConfirmationOutcome.ProceedOnce,
);
} else {
return earlyErrorResponse(
new Error(
hookResult.denyMessage ||
`Permission denied by hook for "${fc.name}"`,
),
fc.name,
);
}
}
}
} else if (!isTodoWriteTool) {
// Skip tool_call event for TodoWriteTool - use ToolCallEmitter
// AUTO_EDIT mode: auto-approve edit and info tools
// (same as coreToolScheduler L5 — NOT delegated to the extension)
if (
approvalMode === ApprovalMode.AUTO_EDIT &&
(confirmationDetails.type === 'edit' ||
confirmationDetails.type === 'info')
) {
// Auto-approve, skip requestPermission.
// didRequestPermission stays false → emitStart below.
} else if (!hookHandled) {
// Show permission dialog via ACP requestPermission
didRequestPermission = true;
const content = buildPermissionRequestContent(confirmationDetails);
// Map tool kind, using switch_mode for exit_plan_mode per ACP spec
const mappedKind = this.toolCallEmitter.mapToolKind(
tool.kind,
fc.name,
);
if (hooksEnabled && messageBus) {
void fireNotificationHook(
messageBus,
`Qwen Code needs your permission to use ${fc.name}`,
NotificationType.PermissionPrompt,
'Permission needed',
);
}
const params: RequestPermissionRequest = {
sessionId: this.sessionId,
options: toPermissionOptions(confirmationDetails, pmForcedAsk),
toolCall: {
toolCallId: callId,
status: 'pending',
title: invocation.getDescription(),
content,
locations: invocation.toolLocations(),
kind: mappedKind,
rawInput: args,
},
};
const output = (await this.client.requestPermission(
params,
)) as RequestPermissionResponse & {
answers?: Record<string, string>;
};
const outcome =
output.outcome.outcome === 'cancelled'
? ToolConfirmationOutcome.Cancel
: z
.nativeEnum(ToolConfirmationOutcome)
.parse(output.outcome.optionId);
await this.resolveIdeDiffForOutcome(confirmationDetails, outcome);
await confirmationDetails.onConfirm(outcome, {
answers: output.answers,
});
// Persist permission rules when user explicitly chose "Always Allow".
// This branch is only reached for tools that went through
// requestPermission (user saw dialog and made a choice).
// AUTO_EDIT auto-approved tools never reach here.
if (
outcome === ToolConfirmationOutcome.ProceedAlways ||
outcome === ToolConfirmationOutcome.ProceedAlwaysProject ||
outcome === ToolConfirmationOutcome.ProceedAlwaysUser
) {
await persistPermissionOutcome(
outcome,
confirmationDetails,
this.config.getOnPersistPermissionRule?.(),
this.config.getPermissionManager?.(),
{ answers: output.answers },
);
}
// After exit_plan_mode confirmation, send current_mode_update
if (
isExitPlanModeTool &&
outcome !== ToolConfirmationOutcome.Cancel
) {
await this.sendCurrentModeUpdateNotification(outcome);
}
// After edit tool ProceedAlways, notify the client about mode change
if (
confirmationDetails.type === 'edit' &&
outcome === ToolConfirmationOutcome.ProceedAlways
) {
await this.sendCurrentModeUpdateNotification(outcome);
}
switch (outcome) {
case ToolConfirmationOutcome.Cancel:
return errorResponse(
new Error(`Tool "${fc.name}" was canceled by the user.`),
);
case ToolConfirmationOutcome.ProceedOnce:
case ToolConfirmationOutcome.ProceedAlways:
case ToolConfirmationOutcome.ProceedAlwaysProject:
case ToolConfirmationOutcome.ProceedAlwaysUser:
case ToolConfirmationOutcome.ProceedAlwaysServer:
case ToolConfirmationOutcome.ProceedAlwaysTool:
case ToolConfirmationOutcome.ModifyWithEditor:
break;
default: {
const resultOutcome: never = outcome;
throw new Error(`Unexpected: ${resultOutcome}`);
}
}
}
}
if (!didRequestPermission && !isTodoWriteTool) {
// Auto-approved (L3 allow / L4 PM allow / L5 YOLO|AUTO_EDIT)
// → emit tool_call start notification
const startParams: ToolCallStartParams = {
callId,
toolName: fc.name,
@ -1041,113 +1212,3 @@ export class Session implements SessionContext {
}
}
}
// ============================================================================
// Helper functions
// ============================================================================
const basicPermissionOptions = [
{
optionId: ToolConfirmationOutcome.ProceedOnce,
name: 'Allow',
kind: 'allow_once',
},
{
optionId: ToolConfirmationOutcome.Cancel,
name: 'Reject',
kind: 'reject_once',
},
] as const;
function toPermissionOptions(
confirmation: ToolCallConfirmationDetails,
): PermissionOption[] {
switch (confirmation.type) {
case 'edit':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlways,
name: 'Allow All Edits',
kind: 'allow_always',
},
...basicPermissionOptions,
];
case 'exec':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: `Always Allow in project: ${confirmation.rootCommand}`,
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: `Always Allow for user: ${confirmation.rootCommand}`,
kind: 'allow_always',
},
...basicPermissionOptions,
];
case 'mcp':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: `Always Allow in project: ${confirmation.toolName}`,
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: `Always Allow for user: ${confirmation.toolName}`,
kind: 'allow_always',
},
...basicPermissionOptions,
];
case 'info':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: `Always Allow in project`,
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: `Always Allow for user`,
kind: 'allow_always',
},
...basicPermissionOptions,
];
case 'plan':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlways,
name: `Yes, and auto-accept edits`,
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedOnce,
name: `Yes, and manually approve edits`,
kind: 'allow_once',
},
{
optionId: ToolConfirmationOutcome.Cancel,
name: `No, keep planning (esc)`,
kind: 'reject_once',
},
];
case 'ask_user_question':
return [
{
optionId: ToolConfirmationOutcome.ProceedOnce,
name: 'Submit',
kind: 'allow_once',
},
{
optionId: ToolConfirmationOutcome.Cancel,
name: 'Cancel',
kind: 'reject_once',
},
];
default: {
const unreachable: never = confirmation;
throw new Error(`Unexpected: ${unreachable}`);
}
}
}

View file

@ -488,6 +488,9 @@ describe('SubAgentTracker', () => {
await vi.waitFor(() => {
expect(respondSpy).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
{
answers: undefined,
},
);
});
});
@ -528,7 +531,58 @@ describe('SubAgentTracker', () => {
eventEmitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, event);
await vi.waitFor(() => {
expect(respondSpy).toHaveBeenCalledWith(ToolConfirmationOutcome.Cancel);
expect(respondSpy).toHaveBeenCalledWith(
ToolConfirmationOutcome.Cancel,
{
answers: undefined,
},
);
});
});
it('should forward answers payload from ACP permission responses', async () => {
requestPermissionSpy.mockResolvedValue({
outcome: {
outcome: 'selected',
optionId: ToolConfirmationOutcome.ProceedOnce,
},
answers: {
answer: 'yes',
},
});
tracker.setup(eventEmitter, abortController.signal);
const respondSpy = vi.fn().mockResolvedValue(undefined);
const confirmationDetails = {
type: 'ask_user_question',
title: 'Question',
questions: [
{
question: 'Continue?',
header: 'Question',
options: [],
multiSelect: false,
},
],
} as unknown as AgentApprovalRequestEvent['confirmationDetails'];
const event = createApprovalEvent({
name: 'ask_user_question',
callId: 'call-ask',
confirmationDetails,
respond: respondSpy,
});
eventEmitter.emit(AgentEventType.TOOL_WAITING_APPROVAL, event);
await vi.waitFor(() => {
expect(respondSpy).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
{
answers: {
answer: 'yes',
},
},
);
});
});
});

View file

@ -26,44 +26,15 @@ import { ToolCallEmitter } from './emitters/ToolCallEmitter.js';
import { MessageEmitter } from './emitters/MessageEmitter.js';
import type {
AgentSideConnection,
PermissionOption,
RequestPermissionRequest,
ToolCallContent,
} from '@agentclientprotocol/sdk';
import {
buildPermissionRequestContent,
toPermissionOptions,
} from './permissionUtils.js';
const debugLogger = createDebugLogger('ACP_SUBAGENT_TRACKER');
/**
* Permission option kind type matching ACP schema.
*/
type PermissionKind =
| 'allow_once'
| 'reject_once'
| 'allow_always'
| 'reject_always';
/**
* Configuration for permission options displayed to users.
*/
interface PermissionOptionConfig {
optionId: ToolConfirmationOutcome;
name: string;
kind: PermissionKind;
}
const basicPermissionOptions: readonly PermissionOptionConfig[] = [
{
optionId: ToolConfirmationOutcome.ProceedOnce,
name: 'Allow',
kind: 'allow_once',
},
{
optionId: ToolConfirmationOutcome.Cancel,
name: 'Reject',
kind: 'reject_once',
},
] as const;
/**
* Tracks and emits events for sub-agent tool calls within AgentTool execution.
*
@ -219,23 +190,6 @@ export class SubAgentTracker {
if (abortSignal.aborted) return;
const state = this.toolStates.get(event.callId);
const content: ToolCallContent[] = [];
// Handle edit confirmation type - show diff
if (event.confirmationDetails.type === 'edit') {
const editDetails = event.confirmationDetails as unknown as {
type: 'edit';
fileName: string;
originalContent: string | null;
newContent: string;
};
content.push({
type: 'diff',
path: editDetails.fileName,
oldText: editDetails.originalContent ?? '',
newText: editDetails.newContent,
});
}
// Build permission request
const fullConfirmationDetails = {
@ -250,12 +204,12 @@ export class SubAgentTracker {
const params: RequestPermissionRequest = {
sessionId: this.ctx.sessionId,
options: this.toPermissionOptions(fullConfirmationDetails),
options: toPermissionOptions(fullConfirmationDetails),
toolCall: {
toolCallId: event.callId,
status: 'pending',
title,
content,
content: buildPermissionRequestContent(fullConfirmationDetails),
locations,
kind,
rawInput: state?.args,
@ -273,7 +227,9 @@ export class SubAgentTracker {
.parse(output.outcome.optionId);
// Respond to subagent with the outcome
await event.respond(outcome);
await event.respond(outcome, {
answers: 'answers' in output ? output.answers : undefined,
});
} catch (error) {
// If permission request fails, cancel the tool call
debugLogger.error(
@ -323,92 +279,4 @@ export class SubAgentTracker {
);
};
}
/**
* Converts confirmation details to permission options for the client.
*/
private toPermissionOptions(
confirmation: ToolCallConfirmationDetails,
): PermissionOption[] {
const hideAlwaysAllow =
'hideAlwaysAllow' in confirmation && confirmation.hideAlwaysAllow;
switch (confirmation.type) {
case 'edit':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlways,
name: 'Allow All Edits',
kind: 'allow_always',
},
...basicPermissionOptions,
];
case 'exec':
return [
...(hideAlwaysAllow
? []
: [
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: `Always Allow in project: ${(confirmation as { rootCommand?: string }).rootCommand ?? 'command'}`,
kind: 'allow_always' as const,
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: `Always Allow for user: ${(confirmation as { rootCommand?: string }).rootCommand ?? 'command'}`,
kind: 'allow_always' as const,
},
]),
...basicPermissionOptions,
];
case 'mcp':
return [
...(hideAlwaysAllow
? []
: [
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: `Always Allow in project: ${(confirmation as { toolName?: string }).toolName ?? 'tool'}`,
kind: 'allow_always' as const,
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: `Always Allow for user: ${(confirmation as { toolName?: string }).toolName ?? 'tool'}`,
kind: 'allow_always' as const,
},
]),
...basicPermissionOptions,
];
case 'info':
return [
...(hideAlwaysAllow
? []
: [
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: 'Always Allow in project',
kind: 'allow_always' as const,
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: 'Always Allow for user',
kind: 'allow_always' as const,
},
]),
...basicPermissionOptions,
];
case 'plan':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlways,
name: 'Always Allow Plans',
kind: 'allow_always',
},
...basicPermissionOptions,
];
default: {
// Fallback for unknown types
return [...basicPermissionOptions];
}
}
}
}

View file

@ -0,0 +1,54 @@
/**
* @license
* Copyright 2025 Qwen
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, expect, it } from 'vitest';
import { ToolConfirmationOutcome } from '@qwen-code/qwen-code-core';
import { toPermissionOptions } from './permissionUtils.js';
describe('permissionUtils', () => {
describe('toPermissionOptions', () => {
it('uses permissionRules for exec always-allow labels when available', () => {
const options = toPermissionOptions({
type: 'exec',
title: 'Confirm Shell Command',
command: 'git add package.json',
rootCommand: 'git',
permissionRules: ['Bash(git add *)'],
onConfirm: async () => undefined,
});
expect(options).toContainEqual(
expect.objectContaining({
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: 'Always Allow in project: git add *',
}),
);
expect(options).toContainEqual(
expect.objectContaining({
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: 'Always Allow for user: git add *',
}),
);
});
it('falls back to rootCommand when exec permissionRules are unavailable', () => {
const options = toPermissionOptions({
type: 'exec',
title: 'Confirm Shell Command',
command: 'git add package.json',
rootCommand: 'git',
onConfirm: async () => undefined,
});
expect(options).toContainEqual(
expect.objectContaining({
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: 'Always Allow in project: git',
}),
);
});
});
});

View file

@ -0,0 +1,208 @@
/**
* @license
* Copyright 2025 Qwen
* SPDX-License-Identifier: Apache-2.0
*/
import type { ToolCallConfirmationDetails } from '@qwen-code/qwen-code-core';
import { ToolConfirmationOutcome } from '@qwen-code/qwen-code-core';
import type {
PermissionOption,
ToolCallContent,
} from '@agentclientprotocol/sdk';
const basicPermissionOptions = [
{
optionId: ToolConfirmationOutcome.ProceedOnce,
name: 'Allow',
kind: 'allow_once',
},
{
optionId: ToolConfirmationOutcome.Cancel,
name: 'Reject',
kind: 'reject_once',
},
] as const satisfies readonly PermissionOption[];
function supportsHideAlwaysAllow(
confirmation: ToolCallConfirmationDetails,
): confirmation is Exclude<
ToolCallConfirmationDetails,
{ type: 'ask_user_question' }
> {
return confirmation.type !== 'ask_user_question';
}
function filterAlwaysAllowOptions(
confirmation: ToolCallConfirmationDetails,
options: PermissionOption[],
forceHideAlwaysAllow = false,
): PermissionOption[] {
const hideAlwaysAllow =
forceHideAlwaysAllow ||
(supportsHideAlwaysAllow(confirmation) &&
confirmation.hideAlwaysAllow === true);
return hideAlwaysAllow
? options.filter((option) => option.kind !== 'allow_always')
: options;
}
function formatExecPermissionScopeLabel(
confirmation: Extract<ToolCallConfirmationDetails, { type: 'exec' }>,
): string {
const permissionRules = confirmation.permissionRules ?? [];
const bashRules = permissionRules
.map((rule) => {
const match = /^Bash\((.*)\)$/.exec(rule.trim());
return match?.[1]?.trim() || undefined;
})
.filter((rule): rule is string => Boolean(rule));
const uniqueRules = [...new Set(bashRules)];
if (uniqueRules.length === 1) {
return uniqueRules[0];
}
if (uniqueRules.length > 1) {
return uniqueRules.join(', ');
}
return confirmation.rootCommand;
}
export function buildPermissionRequestContent(
confirmation: ToolCallConfirmationDetails,
): ToolCallContent[] {
const content: ToolCallContent[] = [];
if (confirmation.type === 'edit') {
content.push({
type: 'diff',
path: confirmation.fileName,
oldText: confirmation.originalContent ?? '',
newText: confirmation.newContent,
});
}
if (confirmation.type === 'plan') {
content.push({
type: 'content',
content: {
type: 'text',
text: confirmation.plan,
},
});
}
return content;
}
export function toPermissionOptions(
confirmation: ToolCallConfirmationDetails,
forceHideAlwaysAllow = false,
): PermissionOption[] {
switch (confirmation.type) {
case 'edit':
return filterAlwaysAllowOptions(
confirmation,
[
{
optionId: ToolConfirmationOutcome.ProceedAlways,
name: 'Allow All Edits',
kind: 'allow_always',
},
...basicPermissionOptions,
],
forceHideAlwaysAllow,
);
case 'exec': {
const label = formatExecPermissionScopeLabel(confirmation);
return filterAlwaysAllowOptions(
confirmation,
[
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: `Always Allow in project: ${label}`,
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: `Always Allow for user: ${label}`,
kind: 'allow_always',
},
...basicPermissionOptions,
],
forceHideAlwaysAllow,
);
}
case 'mcp':
return filterAlwaysAllowOptions(
confirmation,
[
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: `Always Allow in project: ${confirmation.toolName}`,
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: `Always Allow for user: ${confirmation.toolName}`,
kind: 'allow_always',
},
...basicPermissionOptions,
],
forceHideAlwaysAllow,
);
case 'info':
return filterAlwaysAllowOptions(
confirmation,
[
{
optionId: ToolConfirmationOutcome.ProceedAlwaysProject,
name: 'Always Allow in project',
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedAlwaysUser,
name: 'Always Allow for user',
kind: 'allow_always',
},
...basicPermissionOptions,
],
forceHideAlwaysAllow,
);
case 'plan':
return [
{
optionId: ToolConfirmationOutcome.ProceedAlways,
name: 'Yes, and auto-accept edits',
kind: 'allow_always',
},
{
optionId: ToolConfirmationOutcome.ProceedOnce,
name: 'Yes, and manually approve edits',
kind: 'allow_once',
},
{
optionId: ToolConfirmationOutcome.Cancel,
name: 'No, keep planning (esc)',
kind: 'reject_once',
},
];
case 'ask_user_question':
return [
{
optionId: ToolConfirmationOutcome.ProceedOnce,
name: 'Submit',
kind: 'allow_once',
},
{
optionId: ToolConfirmationOutcome.Cancel,
name: 'Cancel',
kind: 'reject_once',
},
];
default: {
const unreachable: never = confirmation;
throw new Error(`Unexpected: ${unreachable}`);
}
}
}

View file

@ -33,8 +33,8 @@ import {
} from '@qwen-code/qwen-code-core';
import { extensionsCommand } from '../commands/extensions.js';
import { hooksCommand } from '../commands/hooks.js';
import type { Settings, LoadedSettings } from './settings.js';
import { SettingScope } from './settings.js';
import type { Settings } from './settings.js';
import { loadSettings, SettingScope } from './settings.js';
import { authCommand } from '../commands/auth.js';
import {
resolveCliGenerationConfig,
@ -704,7 +704,6 @@ export async function loadCliConfig(
argv: CliArgs,
cwd: string = process.cwd(),
overrideExtensions?: string[],
loadedSettings?: LoadedSettings,
): Promise<Config> {
const debugMode = isDebugMode(argv);
@ -1042,20 +1041,19 @@ export async function loadCliConfig(
deny: mergedDeny.length > 0 ? mergedDeny : undefined,
},
// Permission rule persistence callback (writes to settings files).
onPersistPermissionRule: loadedSettings
? async (scope, ruleType, rule) => {
const settingScope =
scope === 'project' ? SettingScope.Workspace : SettingScope.User;
const key = `permissions.${ruleType}`;
const currentRules: string[] =
loadedSettings.forScope(settingScope).settings.permissions?.[
ruleType
] ?? [];
if (!currentRules.includes(rule)) {
loadedSettings.setValue(settingScope, key, [...currentRules, rule]);
}
}
: undefined,
onPersistPermissionRule: async (scope, ruleType, rule) => {
const currentSettings = loadSettings(cwd);
const settingScope =
scope === 'project' ? SettingScope.Workspace : SettingScope.User;
const key = `permissions.${ruleType}`;
const currentRules: string[] =
currentSettings.forScope(settingScope).settings.permissions?.[
ruleType
] ?? [];
if (!currentRules.includes(rule)) {
currentSettings.setValue(settingScope, key, [...currentRules, rule]);
}
},
toolDiscoveryCommand: settings.tools?.discoveryCommand,
toolCallCommand: settings.tools?.callCommand,
mcpServerCommand: settings.mcp?.serverCommand,

View file

@ -358,7 +358,6 @@ export async function main() {
argv,
process.cwd(),
argv.extensions,
settings,
);
// Register cleanup for MCP clients as early as possible

View file

@ -133,12 +133,12 @@ export class ShellProcessor implements IPromptProcessor {
// Security check on the final, escaped command string.
const { allAllowed, disallowedCommands, blockReason, isHardDenial } =
checkCommandPermissions(command, config, sessionShellAllowlist);
await checkCommandPermissions(command, config, sessionShellAllowlist);
// Determine if this command is explicitly auto-approved via PermissionManager
const pm = config.getPermissionManager?.();
const isAllowedBySettings = pm
? pm.isCommandAllowed(command) === 'allow'
? (await pm.isCommandAllowed(command)) === 'allow'
: false;
if (!allAllowed) {

View file

@ -74,6 +74,12 @@ export const ToolConfirmationMessage: React.FC<
}, [config]);
const handleConfirm = async (outcome: ToolConfirmationOutcome) => {
// Call onConfirm before resolving the IDE diff so that the CLI outcome
// (e.g. ProceedAlways) is processed first. resolveDiffFromCli would
// otherwise trigger the scheduler's ideConfirmation .then() handler
// with ProceedOnce, racing with the intended CLI outcome.
onConfirm(outcome);
if (confirmationDetails.type === 'edit') {
if (config.getIdeMode() && isDiffingEnabled) {
const cliOutcome =
@ -84,7 +90,6 @@ export const ToolConfirmationMessage: React.FC<
);
}
}
onConfirm(outcome);
};
const isTrustedFolder = config.isTrustedFolder();