mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(sdk): honor canUseTool timeout in CLI control requests (#4491)
This commit is contained in:
parent
3cda1e26e7
commit
60dbbaaed4
9 changed files with 449 additions and 1 deletions
|
|
@ -35,6 +35,7 @@ export interface IControlContext {
|
|||
readonly settings: LoadedSettings;
|
||||
|
||||
permissionMode: PermissionMode;
|
||||
sdkCanUseToolTimeoutMs?: number;
|
||||
sdkMcpServers: Set<string>;
|
||||
mcpClients: Map<string, { client: Client; config: MCPServerConfig }>;
|
||||
inputClosed: boolean;
|
||||
|
|
@ -54,6 +55,7 @@ export class ControlContext implements IControlContext {
|
|||
readonly settings: LoadedSettings;
|
||||
|
||||
permissionMode: PermissionMode;
|
||||
sdkCanUseToolTimeoutMs?: number;
|
||||
sdkMcpServers: Set<string>;
|
||||
mcpClients: Map<string, { client: Client; config: MCPServerConfig }>;
|
||||
inputClosed: boolean;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,186 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen Team
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import {
|
||||
InputFormat,
|
||||
ToolConfirmationOutcome,
|
||||
} from '@qwen-code/qwen-code-core';
|
||||
import { createMinimalSettings } from '../../../config/settings.js';
|
||||
import type { StreamJsonOutputAdapter } from '../../io/StreamJsonOutputAdapter.js';
|
||||
import type { IControlContext } from '../ControlContext.js';
|
||||
import type { IPendingRequestRegistry } from './baseController.js';
|
||||
import { PermissionController } from './permissionController.js';
|
||||
|
||||
function createContext(canUseToolTimeoutMs?: number): IControlContext {
|
||||
const abortController = new AbortController();
|
||||
|
||||
return {
|
||||
config: {
|
||||
getDebugMode: vi.fn().mockReturnValue(false),
|
||||
getInputFormat: vi.fn().mockReturnValue(InputFormat.STREAM_JSON),
|
||||
} as unknown as IControlContext['config'],
|
||||
streamJson: {
|
||||
send: vi.fn(),
|
||||
} as unknown as StreamJsonOutputAdapter,
|
||||
sessionId: 'test-session-id',
|
||||
abortSignal: abortController.signal,
|
||||
debugMode: false,
|
||||
settings: createMinimalSettings(),
|
||||
permissionMode: 'default',
|
||||
sdkCanUseToolTimeoutMs: canUseToolTimeoutMs,
|
||||
sdkMcpServers: new Set<string>(),
|
||||
mcpClients: new Map(),
|
||||
inputClosed: false,
|
||||
};
|
||||
}
|
||||
|
||||
function createRegistry(): IPendingRequestRegistry {
|
||||
return {
|
||||
registerIncomingRequest: vi.fn(),
|
||||
deregisterIncomingRequest: vi.fn(),
|
||||
registerOutgoingRequest: vi.fn(),
|
||||
deregisterOutgoingRequest: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describe('PermissionController', () => {
|
||||
it('uses SDK canUseTool timeout for outgoing permission requests', async () => {
|
||||
const context = createContext(120_000);
|
||||
const controller = new PermissionController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'PermissionController',
|
||||
);
|
||||
const sendControlRequest = vi
|
||||
.spyOn(controller, 'sendControlRequest')
|
||||
.mockResolvedValue({
|
||||
subtype: 'success',
|
||||
request_id: 'request-1',
|
||||
response: { behavior: 'allow' },
|
||||
});
|
||||
const onConfirm = vi.fn();
|
||||
|
||||
controller.getToolCallUpdateCallback()([
|
||||
{
|
||||
status: 'awaiting_approval',
|
||||
request: {
|
||||
callId: 'tool-call-1',
|
||||
name: 'ask_user_question',
|
||||
args: { questions: [] },
|
||||
},
|
||||
confirmationDetails: {
|
||||
type: 'ask_user_question',
|
||||
title: 'Please answer',
|
||||
onConfirm,
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(sendControlRequest).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
subtype: 'can_use_tool',
|
||||
tool_name: 'ask_user_question',
|
||||
}),
|
||||
120_000,
|
||||
context.abortSignal,
|
||||
);
|
||||
});
|
||||
await vi.waitFor(() => {
|
||||
expect(onConfirm).toHaveBeenCalledWith(
|
||||
ToolConfirmationOutcome.ProceedOnce,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it('uses default timeout when SDK canUseTool timeout is undefined', async () => {
|
||||
const context = createContext(); // undefined timeout
|
||||
const controller = new PermissionController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'PermissionController',
|
||||
);
|
||||
const sendControlRequest = vi
|
||||
.spyOn(controller, 'sendControlRequest')
|
||||
.mockResolvedValue({
|
||||
subtype: 'success',
|
||||
request_id: 'request-2',
|
||||
response: { behavior: 'allow' },
|
||||
});
|
||||
const onConfirm = vi.fn();
|
||||
|
||||
controller.getToolCallUpdateCallback()([
|
||||
{
|
||||
status: 'awaiting_approval',
|
||||
request: {
|
||||
callId: 'tool-call-2',
|
||||
name: 'ask_user_question',
|
||||
args: { questions: [] },
|
||||
},
|
||||
confirmationDetails: {
|
||||
type: 'ask_user_question',
|
||||
title: 'Please answer',
|
||||
onConfirm,
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(sendControlRequest).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
subtype: 'can_use_tool',
|
||||
tool_name: 'ask_user_question',
|
||||
}),
|
||||
60_000, // DEFAULT_CAN_USE_TOOL_TIMEOUT_MS
|
||||
context.abortSignal,
|
||||
);
|
||||
});
|
||||
await vi.waitFor(() => {
|
||||
expect(onConfirm).toHaveBeenCalledWith(
|
||||
ToolConfirmationOutcome.ProceedOnce,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it('calls onConfirm with Cancel when sendControlRequest rejects', async () => {
|
||||
const context = createContext(120_000);
|
||||
const controller = new PermissionController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'PermissionController',
|
||||
);
|
||||
vi.spyOn(controller, 'sendControlRequest').mockRejectedValue(
|
||||
new Error('Request timeout'),
|
||||
);
|
||||
const onConfirm = vi.fn();
|
||||
|
||||
controller.getToolCallUpdateCallback()([
|
||||
{
|
||||
status: 'awaiting_approval',
|
||||
request: {
|
||||
callId: 'tool-call-3',
|
||||
name: 'ask_user_question',
|
||||
args: { questions: [] },
|
||||
},
|
||||
confirmationDetails: {
|
||||
type: 'ask_user_question',
|
||||
title: 'Please answer',
|
||||
onConfirm,
|
||||
},
|
||||
},
|
||||
]);
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(onConfirm).toHaveBeenCalledWith(
|
||||
ToolConfirmationOutcome.Cancel,
|
||||
expect.objectContaining({
|
||||
cancelMessage: expect.stringContaining('Request timeout'),
|
||||
}),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -36,6 +36,8 @@ import { BaseController } from './baseController.js';
|
|||
// Import ToolCallConfirmationDetails types for type alignment
|
||||
type ToolConfirmationType = 'edit' | 'exec' | 'mcp' | 'info' | 'plan';
|
||||
|
||||
const DEFAULT_CAN_USE_TOOL_TIMEOUT_MS = 60_000;
|
||||
|
||||
export class PermissionController extends BaseController {
|
||||
private pendingOutgoingRequests = new Set<string>();
|
||||
|
||||
|
|
@ -427,7 +429,7 @@ export class PermissionController extends BaseController {
|
|||
permission_suggestions: permissionSuggestions,
|
||||
blocked_path: null,
|
||||
} as CLIControlPermissionRequest,
|
||||
undefined, // use default timeout
|
||||
this.context.sdkCanUseToolTimeoutMs ?? DEFAULT_CAN_USE_TOOL_TIMEOUT_MS,
|
||||
this.context.abortSignal,
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,207 @@
|
|||
/**
|
||||
* @license
|
||||
* Copyright 2025 Qwen Team
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { InputFormat } from '@qwen-code/qwen-code-core';
|
||||
import { createMinimalSettings } from '../../../config/settings.js';
|
||||
import type { StreamJsonOutputAdapter } from '../../io/StreamJsonOutputAdapter.js';
|
||||
import type { IControlContext } from '../ControlContext.js';
|
||||
import type { IPendingRequestRegistry } from './baseController.js';
|
||||
import { SystemController } from './systemController.js';
|
||||
|
||||
function createContext(): IControlContext {
|
||||
const abortController = new AbortController();
|
||||
|
||||
return {
|
||||
config: {
|
||||
getDebugMode: vi.fn().mockReturnValue(false),
|
||||
getInputFormat: vi.fn().mockReturnValue(InputFormat.STREAM_JSON),
|
||||
setSdkMode: vi.fn(),
|
||||
getSessionId: vi.fn().mockReturnValue('test-session-id'),
|
||||
addMcpServers: vi.fn(),
|
||||
setSessionSubagents: vi.fn(),
|
||||
setApprovalMode: vi.fn(),
|
||||
setModel: vi.fn(),
|
||||
} as unknown as IControlContext['config'],
|
||||
streamJson: {
|
||||
send: vi.fn(),
|
||||
} as unknown as StreamJsonOutputAdapter,
|
||||
sessionId: 'test-session-id',
|
||||
abortSignal: abortController.signal,
|
||||
debugMode: false,
|
||||
settings: createMinimalSettings(),
|
||||
permissionMode: 'default',
|
||||
sdkCanUseToolTimeoutMs: undefined,
|
||||
sdkMcpServers: new Set<string>(),
|
||||
mcpClients: new Map(),
|
||||
inputClosed: false,
|
||||
};
|
||||
}
|
||||
|
||||
function createRegistry(): IPendingRequestRegistry {
|
||||
return {
|
||||
registerIncomingRequest: vi.fn(),
|
||||
deregisterIncomingRequest: vi.fn(),
|
||||
registerOutgoingRequest: vi.fn(),
|
||||
deregisterOutgoingRequest: vi.fn(),
|
||||
};
|
||||
}
|
||||
|
||||
describe('SystemController', () => {
|
||||
describe('initialize timeout validation', () => {
|
||||
it('accepts valid timeout within bounds', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: 120_000 },
|
||||
},
|
||||
'test-1',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBe(120_000);
|
||||
});
|
||||
|
||||
it('accepts timeout at maximum boundary (600_000ms)', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: 600_000 },
|
||||
},
|
||||
'test-2',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBe(600_000);
|
||||
});
|
||||
|
||||
it('ignores timeout exceeding maximum boundary', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: 600_001 },
|
||||
},
|
||||
'test-3',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('ignores Number.MAX_VALUE timeout', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: Number.MAX_VALUE },
|
||||
},
|
||||
'test-4',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('ignores negative timeout', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: -1000 },
|
||||
},
|
||||
'test-5',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('ignores zero timeout', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: 0 },
|
||||
},
|
||||
'test-6',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('ignores Infinity timeout', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: Infinity },
|
||||
},
|
||||
'test-7',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBeUndefined();
|
||||
});
|
||||
|
||||
it('ignores NaN timeout', async () => {
|
||||
const context = createContext();
|
||||
const controller = new SystemController(
|
||||
context,
|
||||
createRegistry(),
|
||||
'SystemController',
|
||||
);
|
||||
|
||||
await controller.handleRequest(
|
||||
{
|
||||
subtype: 'initialize',
|
||||
timeout: { canUseTool: NaN },
|
||||
},
|
||||
'test-8',
|
||||
);
|
||||
|
||||
expect(context.sdkCanUseToolTimeoutMs).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -31,6 +31,14 @@ import {
|
|||
|
||||
const debugLogger = createDebugLogger('SYSTEM_CONTROLLER');
|
||||
|
||||
/**
|
||||
* Maximum allowed timeout for canUseTool requests (10 minutes).
|
||||
* Node.js setTimeout coerces delays > 2^31-1 to 32-bit signed integers,
|
||||
* which can cause timeouts to fire immediately or never. This cap prevents
|
||||
* such edge cases while still allowing reasonable timeout values.
|
||||
*/
|
||||
const MAX_CAN_USE_TOOL_TIMEOUT_MS = 600_000;
|
||||
|
||||
export class SystemController extends BaseController {
|
||||
/**
|
||||
* Handle system control requests
|
||||
|
|
@ -132,6 +140,16 @@ export class SystemController extends BaseController {
|
|||
|
||||
this.context.config.setSdkMode(true);
|
||||
|
||||
const canUseToolTimeout = payload.timeout?.canUseTool;
|
||||
if (
|
||||
typeof canUseToolTimeout === 'number' &&
|
||||
Number.isFinite(canUseToolTimeout) &&
|
||||
canUseToolTimeout > 0 &&
|
||||
canUseToolTimeout <= MAX_CAN_USE_TOOL_TIMEOUT_MS
|
||||
) {
|
||||
this.context.sdkCanUseToolTimeoutMs = canUseToolTimeout;
|
||||
}
|
||||
|
||||
// Process SDK MCP servers
|
||||
if (
|
||||
payload.sdkMcpServers &&
|
||||
|
|
|
|||
|
|
@ -364,6 +364,9 @@ export interface CLIMcpServerConfig {
|
|||
export interface CLIControlInitializeRequest {
|
||||
subtype: 'initialize';
|
||||
hooks?: HookRegistration[] | null;
|
||||
timeout?: {
|
||||
canUseTool?: number;
|
||||
};
|
||||
/**
|
||||
* SDK MCP servers config
|
||||
* These are MCP servers running in the SDK process, connected via control plane.
|
||||
|
|
|
|||
|
|
@ -293,6 +293,9 @@ export class Query implements AsyncIterable<SDKMessage> {
|
|||
|
||||
await this.sendControlRequest(ControlRequestType.INITIALIZE, {
|
||||
hooks: null,
|
||||
timeout: this.options.timeout?.canUseTool
|
||||
? { canUseTool: this.options.timeout.canUseTool }
|
||||
: undefined,
|
||||
sdkMcpServers:
|
||||
Object.keys(sdkMcpServersForCli).length > 0
|
||||
? sdkMcpServersForCli
|
||||
|
|
|
|||
|
|
@ -334,6 +334,9 @@ export type WireSDKMcpServerConfig = Omit<SDKMcpServerConfig, 'instance'>;
|
|||
export interface CLIControlInitializeRequest {
|
||||
subtype: 'initialize';
|
||||
hooks?: HookRegistration[] | null;
|
||||
timeout?: {
|
||||
canUseTool?: number;
|
||||
};
|
||||
/**
|
||||
* SDK MCP servers config
|
||||
* These are MCP servers running in the SDK process, connected via control plane.
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import type {
|
|||
CLIControlRequest,
|
||||
CLIControlResponse,
|
||||
ControlCancelRequest,
|
||||
CLIControlInitializeRequest,
|
||||
} from '../../src/types/protocol.js';
|
||||
import { ControlRequestType } from '../../src/types/protocol.js';
|
||||
import { AbortError } from '../../src/types/errors.js';
|
||||
|
|
@ -313,6 +314,29 @@ describe('Query', () => {
|
|||
await query.close();
|
||||
});
|
||||
|
||||
it('should include canUseTool timeout in initialize request', async () => {
|
||||
const query = new Query(transport, {
|
||||
cwd: '/test',
|
||||
timeout: {
|
||||
canUseTool: 120_000,
|
||||
},
|
||||
});
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(transport.writtenMessages.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
const initRequest =
|
||||
transport.getLastWrittenMessage() as CLIControlRequest;
|
||||
expect(initRequest.request.subtype).toBe('initialize');
|
||||
expect(
|
||||
(initRequest.request as CLIControlInitializeRequest).timeout,
|
||||
).toEqual({ canUseTool: 120_000 });
|
||||
|
||||
await respondToInitialize(transport, query);
|
||||
await query.close();
|
||||
});
|
||||
|
||||
it('should generate unique session ID', async () => {
|
||||
const transport2 = new MockTransport();
|
||||
const query1 = new Query(transport, { cwd: '/test' });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue