Skip to content

feat(cli): forward ask_user_question answers from SDK can_use_tool #6655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 6 commits into from
Jul 10, 2026
Merged

feat(cli): forward ask_user_question answers from SDK can_use_tool #6655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8705ee5
feat(cli): forward ask_user_question answers from SDK can_use_tool
TianYuan1024 Jul 10, 2026
7cd5f91
fix(cli): forward ask_user_question answers on teammate approval path
TianYuan1024 Jul 10, 2026
e3fe0a4
Merge remote-tracking branch 'origin/main' into feat/sdk-ask-user-que…
TianYuan1024 Jul 10, 2026
d78d7ac
test(web-shell): stub Range client-rect methods to fix flaky CI
TianYuan1024 Jul 10, 2026
9c07d95
fix(web-shell): resolve merge conflict in test setup Range stubs
qwen-code-dev-bot Jul 10, 2026
c13214e
refactor(cli): use ToolNames constant and broaden permission tests
TianYuan1024 Jul 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
288 changes: 288 additions & 0 deletions packages/cli/src/nonInteractive/control/controllers/permissionController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,200 @@ describe('PermissionController', () => {
});
});

it('routes ask_user_question answers from updatedInput into the confirmation payload', async () => {
const context = createContext(120_000);
const controller = new PermissionController(
context,
createRegistry(),
'PermissionController',
);
const answers = { '0': 'PostgreSQL', '1': 'REST' };
vi.spyOn(controller, 'sendControlRequest').mockResolvedValue({
subtype: 'success',
request_id: 'request-answers',
response: {
behavior: 'allow',
updatedInput: { questions: [], answers },
},
});
const onConfirm = vi.fn();
const toolCall = {
status: 'awaiting_approval',
request: {
callId: 'tool-call-answers',
name: 'ask_user_question',
args: { questions: [] } as Record<string, unknown>,
},
confirmationDetails: {
type: 'ask_user_question',
title: 'Please answer',
onConfirm,
},
};

controller.getToolCallUpdateCallback()([toolCall]);

await vi.waitFor(() => {
expect(onConfirm).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
expect.objectContaining({ answers }),
);
});

// The leader path overrides the tool's in-process args with the
// host's sanitized updatedInput before confirming.
expect(toolCall.request.args).toEqual({ questions: [], answers });
});

it('omits answers from the payload when updatedInput has none', async () => {
const context = createContext(120_000);
const controller = new PermissionController(
context,
createRegistry(),
'PermissionController',
);
vi.spyOn(controller, 'sendControlRequest').mockResolvedValue({
subtype: 'success',
request_id: 'request-no-answers',
response: {
behavior: 'allow',
updatedInput: { command: 'ls -a' },
},
});
const onConfirm = vi.fn();

controller.getToolCallUpdateCallback()([
{
status: 'awaiting_approval',
request: {
callId: 'tool-call-no-answers',
name: 'run_shell_command',
args: { command: 'ls' },
},
confirmationDetails: {
type: 'exec',
title: 'Run command',
onConfirm,
},
},
]);

await vi.waitFor(() => {
expect(onConfirm).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
{ updatedInput: { command: 'ls -a' } },
);
});
});

it('does not promote a same-named answers field for non-ask_user_question tools', async () => {
const context = createContext(120_000);
const controller = new PermissionController(
context,
createRegistry(),
'PermissionController',
);
vi.spyOn(controller, 'sendControlRequest').mockResolvedValue({
subtype: 'success',
request_id: 'request-foreign-answers',
response: {
behavior: 'allow',
// A non-ask_user_question tool happens to carry an `answers` field;
// it must not leak into the confirmation payload.
updatedInput: { command: 'ls', answers: { '0': 'leak' } },
},
});
const onConfirm = vi.fn();

controller.getToolCallUpdateCallback()([
{
status: 'awaiting_approval',
request: {
callId: 'tool-call-foreign-answers',
name: 'run_shell_command',
args: { command: 'ls' },
},
confirmationDetails: {
type: 'exec',
title: 'Run command',
onConfirm,
},
},
]);

await vi.waitFor(() => {
expect(onConfirm).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
{ updatedInput: { command: 'ls', answers: { '0': 'leak' } } },
);
});
expect(onConfirm).not.toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
expect.objectContaining({ answers: expect.anything() }),
);
});

it.each([
['updatedInput is an array', ['ls'], undefined],
['updatedInput is a string', 'ls', undefined],
['answers is an array', { questions: [], answers: ['x'] }, undefined],
['answers is null', { questions: [], answers: null }, undefined],
['answers is an empty object', { questions: [], answers: {} }, {}],
])(
'omits answers from the payload when %s',
async (_desc, updatedInput, expectedAnswers) => {
const context = createContext(120_000);
const controller = new PermissionController(
context,
createRegistry(),
'PermissionController',
);
vi.spyOn(controller, 'sendControlRequest').mockResolvedValue({
subtype: 'success',
request_id: 'request-guard',
response: { behavior: 'allow', updatedInput },
});
const onConfirm = vi.fn();

controller.getToolCallUpdateCallback()([
{
status: 'awaiting_approval',
request: {
callId: 'tool-call-guard',
name: 'ask_user_question',
args: { questions: [] },
},
confirmationDetails: {
type: 'ask_user_question',
title: 'Please answer',
onConfirm,
},
},
]);

await vi.waitFor(() => {
expect(onConfirm).toHaveBeenCalled();
});

const [outcome, payload] = onConfirm.mock.calls[0];
expect(outcome).toBe(ToolConfirmationOutcome.ProceedOnce);
const isPlainObject =
updatedInput !== null &&
typeof updatedInput === 'object' &&
!Array.isArray(updatedInput);
if (!isPlainObject) {
// A non-object updatedInput (array or primitive) is rejected
// wholesale — plain confirm, no payload.
expect(payload).toBeUndefined();
} else if (expectedAnswers === undefined) {
expect(payload).toEqual({ updatedInput });
expect(payload).not.toHaveProperty('answers');
} else {
expect(payload).toEqual({ updatedInput, answers: expectedAnswers });
}
},
);

it('uses default timeout when SDK canUseTool timeout is undefined', async () => {
const context = createContext(); // undefined timeout
const controller = new PermissionController(
Expand Down Expand Up @@ -184,6 +378,100 @@ describe('PermissionController', () => {
});
});

it('forwards ask_user_question answers to a teammate approval', async () => {
const context = createContext(120_000);
const controller = new PermissionController(
context,
createRegistry(),
'PermissionController',
);
const answers = { '0': 'PostgreSQL', '1': 'REST' };
vi.spyOn(controller, 'sendControlRequest').mockResolvedValue({
subtype: 'success',
request_id: 'teammate-request',
response: {
behavior: 'allow',
updatedInput: { questions: [], answers },
},
});
const respond = vi.fn().mockResolvedValue(undefined);

await controller.handleTeammateApproval({
teammateName: 'worker',
toolName: 'ask_user_question',
toolInput: { questions: [] },
respond,
timestamp: 123,
});

expect(respond).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
expect.objectContaining({ answers }),
);
});

it('does not promote a same-named answers field for a non-ask_user_question teammate approval', async () => {
const context = createContext(120_000);
const controller = new PermissionController(
context,
createRegistry(),
'PermissionController',
);
vi.spyOn(controller, 'sendControlRequest').mockResolvedValue({
subtype: 'success',
request_id: 'teammate-request-foreign',
response: {
behavior: 'allow',
updatedInput: { command: 'ls', answers: { '0': 'leak' } },
},
});
const respond = vi.fn().mockResolvedValue(undefined);

await controller.handleTeammateApproval({
teammateName: 'worker',
toolName: 'run_shell_command',
toolInput: { command: 'ls' },
respond,
timestamp: 456,
});

expect(respond).toHaveBeenCalledWith(ToolConfirmationOutcome.ProceedOnce, {
updatedInput: { command: 'ls', answers: { '0': 'leak' } },
});
expect(respond).not.toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
expect.objectContaining({ answers: expect.anything() }),
);
});

it('confirms a teammate approval with no payload when updatedInput is absent', async () => {
const context = createContext(120_000);
const controller = new PermissionController(
context,
createRegistry(),
'PermissionController',
);
vi.spyOn(controller, 'sendControlRequest').mockResolvedValue({
subtype: 'success',
request_id: 'teammate-request-no-input',
response: { behavior: 'allow' },
});
const respond = vi.fn().mockResolvedValue(undefined);

await controller.handleTeammateApproval({
teammateName: 'worker',
toolName: 'run_shell_command',
toolInput: { command: 'ls' },
respond,
timestamp: 789,
});

expect(respond).toHaveBeenCalledWith(
ToolConfirmationOutcome.ProceedOnce,
undefined,
);
});

@wenshao wenshao Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The leader path's else branch (no updatedInput → bare onConfirm(ProceedOnce)) has no explicit test in this PR. The teammate path has one ("confirms a teammate approval with no payload when updatedInput is absent"), and the pre-existing timeout test exercises it implicitly, but a leader-specific assertion would pin the branch down and document the intent.

— qwen3.7-max via Qwen Code /review


it('omits modify suggestions when edit confirmation hides modify actions', () => {
const controller = new PermissionController(
createContext(),
Expand Down
Loading
Loading