mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-28 11:41:04 +00:00
fix(cli): improve ask_user_question tool handling and UI
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> - In ACP mode: allow ask_user_question in YOLO mode (user must respond) - In ACP mode: allow ask_user_question in plan mode for clarifications - Hide footer when confirmation dialog is active - Fix tab index overflow with functional state updates - Fix ask_user_question detection in VSCode companion (use rawInput) - Add cleanup for pending ACP promises on panel/provider dispose - Use theme.text.accent consistently for highlighted elements - Remove unused 'answers' param from AskUserQuestionParams This ensures users can always respond to clarification questions in ACP mode regardless of approval mode, and improves dialog UX.
This commit is contained in:
tanzhenxin
parent
67b9e3438c
commit
411ebd03b8
10 changed files with 85 additions and 49 deletions
|
|
@ -512,13 +512,27 @@ export class Session implements SessionContext {
|
|||
}
|
||||
|
||||
const confirmationDetails =
|
||||
this.config.getApprovalMode() !== ApprovalMode.YOLO
|
||||
? await invocation.shouldConfirmExecute(abortSignal)
|
||||
: false;
|
||||
await invocation.shouldConfirmExecute(abortSignal);
|
||||
|
||||
// In YOLO mode, auto-approve everything except ask_user_question
|
||||
// (the user must always have a chance to respond to questions)
|
||||
const isAskUserQuestionTool =
|
||||
confirmationDetails && confirmationDetails.type === 'ask_user_question';
|
||||
const effectiveConfirmationDetails =
|
||||
this.config.getApprovalMode() === ApprovalMode.YOLO &&
|
||||
!isAskUserQuestionTool
|
||||
? false
|
||||
: confirmationDetails;
|
||||
|
||||
// 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;
|
||||
if (isPlanMode && !isExitPlanModeTool && confirmationDetails) {
|
||||
if (
|
||||
isPlanMode &&
|
||||
!isExitPlanModeTool &&
|
||||
!isAskUserQuestionTool &&
|
||||
effectiveConfirmationDetails
|
||||
) {
|
||||
// In plan mode, block any tool that requires confirmation (write operations)
|
||||
return errorResponse(
|
||||
new Error(
|
||||
|
|
@ -528,25 +542,25 @@ export class Session implements SessionContext {
|
|||
);
|
||||
}
|
||||
|
||||
if (confirmationDetails) {
|
||||
if (effectiveConfirmationDetails) {
|
||||
const content: acp.ToolCallContent[] = [];
|
||||
|
||||
if (confirmationDetails.type === 'edit') {
|
||||
if (effectiveConfirmationDetails.type === 'edit') {
|
||||
content.push({
|
||||
type: 'diff',
|
||||
path: confirmationDetails.fileName,
|
||||
oldText: confirmationDetails.originalContent,
|
||||
newText: confirmationDetails.newContent,
|
||||
path: effectiveConfirmationDetails.fileName,
|
||||
oldText: effectiveConfirmationDetails.originalContent,
|
||||
newText: effectiveConfirmationDetails.newContent,
|
||||
});
|
||||
}
|
||||
|
||||
// Add plan content for exit_plan_mode
|
||||
if (confirmationDetails.type === 'plan') {
|
||||
if (effectiveConfirmationDetails.type === 'plan') {
|
||||
content.push({
|
||||
type: 'content',
|
||||
content: {
|
||||
type: 'text',
|
||||
text: confirmationDetails.plan,
|
||||
text: effectiveConfirmationDetails.plan,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
|
@ -556,7 +570,7 @@ export class Session implements SessionContext {
|
|||
|
||||
const params: acp.RequestPermissionRequest = {
|
||||
sessionId: this.sessionId,
|
||||
options: toPermissionOptions(confirmationDetails),
|
||||
options: toPermissionOptions(effectiveConfirmationDetails),
|
||||
toolCall: {
|
||||
toolCallId: callId,
|
||||
status: 'pending',
|
||||
|
|
@ -576,7 +590,7 @@ export class Session implements SessionContext {
|
|||
.nativeEnum(ToolConfirmationOutcome)
|
||||
.parse(output.outcome.optionId);
|
||||
|
||||
await confirmationDetails.onConfirm(outcome, {
|
||||
await effectiveConfirmationDetails.onConfirm(outcome, {
|
||||
answers: output.answers,
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -103,7 +103,9 @@ export const Composer = () => {
|
|||
)}
|
||||
|
||||
{/* Exclusive area: only one component visible at a time */}
|
||||
{/* Hide footer when a confirmation dialog (e.g. ask_user_question) is active */}
|
||||
{!showSuggestions &&
|
||||
uiState.streamingState !== StreamingState.WaitingForConfirmation &&
|
||||
(showShortcuts ? (
|
||||
<KeyboardShortcuts />
|
||||
) : (
|
||||
|
|
|
|||
|
|
@ -402,8 +402,6 @@ describe('<AskUserQuestionDialog />', () => {
|
|||
stdin.write('Orange');
|
||||
await wait();
|
||||
|
||||
console.log(lastFrame());
|
||||
|
||||
expect(lastFrame()).toContain('Orange');
|
||||
unmount();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -127,7 +127,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
} else {
|
||||
if (currentQuestionIndex < totalTabs - 1) {
|
||||
setTimeout(() => {
|
||||
setCurrentQuestionIndex(currentQuestionIndex + 1);
|
||||
setCurrentQuestionIndex((prev) => Math.min(prev + 1, totalTabs - 1));
|
||||
setSelectedIndex(0);
|
||||
}, 150);
|
||||
}
|
||||
|
|
@ -166,7 +166,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
// Auto-advance to next tab
|
||||
if (currentQuestionIndex < totalTabs - 1) {
|
||||
setTimeout(() => {
|
||||
setCurrentQuestionIndex(currentQuestionIndex + 1);
|
||||
setCurrentQuestionIndex((prev) => Math.min(prev + 1, totalTabs - 1));
|
||||
setSelectedIndex(0);
|
||||
}, 150);
|
||||
}
|
||||
|
|
@ -314,7 +314,9 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
// Auto-advance to next tab after selection
|
||||
if (currentQuestionIndex < totalTabs - 1) {
|
||||
setTimeout(() => {
|
||||
setCurrentQuestionIndex(currentQuestionIndex + 1);
|
||||
setCurrentQuestionIndex((prev) =>
|
||||
Math.min(prev + 1, totalTabs - 1),
|
||||
);
|
||||
setSelectedIndex(0);
|
||||
}, 150);
|
||||
}
|
||||
|
|
@ -352,7 +354,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
);
|
||||
})}
|
||||
<Box>
|
||||
<Text color={theme.text.link} bold>
|
||||
<Text color={theme.text.accent} bold>
|
||||
▸ {t('Submit')}
|
||||
</Text>
|
||||
</Box>
|
||||
|
|
@ -368,7 +370,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
<Text>
|
||||
{q.header}:{' '}
|
||||
{answer ? (
|
||||
<Text color={theme.text.link}>{answer}</Text>
|
||||
<Text color={theme.text.accent}>{answer}</Text>
|
||||
) : (
|
||||
<Text dimColor>{t('(not answered)')}</Text>
|
||||
)}
|
||||
|
|
@ -386,7 +388,9 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
<Box flexDirection="column">
|
||||
<Box>
|
||||
<Text
|
||||
color={selectedIndex === 0 ? theme.text.link : theme.text.primary}
|
||||
color={
|
||||
selectedIndex === 0 ? theme.text.accent : theme.text.primary
|
||||
}
|
||||
bold={selectedIndex === 0}
|
||||
>
|
||||
{selectedIndex === 0 ? '❯ ' : ' '}1. {t('Submit answers')}
|
||||
|
|
@ -394,7 +398,9 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
</Box>
|
||||
<Box>
|
||||
<Text
|
||||
color={selectedIndex === 1 ? theme.text.link : theme.text.primary}
|
||||
color={
|
||||
selectedIndex === 1 ? theme.text.accent : theme.text.primary
|
||||
}
|
||||
bold={selectedIndex === 1}
|
||||
>
|
||||
{selectedIndex === 1 ? '❯ ' : ' '}2. {t('Cancel')}
|
||||
|
|
@ -424,7 +430,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
<Text
|
||||
color={
|
||||
idx === currentQuestionIndex
|
||||
? theme.text.link
|
||||
? theme.text.accent
|
||||
: theme.text.primary
|
||||
}
|
||||
bold={idx === currentQuestionIndex}
|
||||
|
|
@ -447,7 +453,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
<Box marginBottom={1}>
|
||||
{!hasMultipleQuestions && (
|
||||
<Box marginBottom={1}>
|
||||
<Text color={theme.text.link} bold>
|
||||
<Text color={theme.text.accent} bold>
|
||||
{currentQuestion!.header}
|
||||
</Text>
|
||||
</Box>
|
||||
|
|
@ -468,11 +474,15 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
!isMultiSelect &&
|
||||
selectedOptions[currentQuestionIndex] === opt.label;
|
||||
const isHighlighted = isSelected || isAnswered || isMultiChecked;
|
||||
// Calculate prefix width for description alignment:
|
||||
// 2 (cursor) + checkbox (4 if multi) + number + ". " (2)
|
||||
const prefixWidth =
|
||||
2 + (isMultiSelect ? 4 : 0) + String(index + 1).length + 2;
|
||||
return (
|
||||
<Box key={index} flexDirection="column">
|
||||
<Box>
|
||||
<Text
|
||||
color={isHighlighted ? theme.text.link : theme.text.primary}
|
||||
color={isHighlighted ? theme.text.accent : theme.text.primary}
|
||||
bold={isHighlighted}
|
||||
>
|
||||
{isSelected ? '❯ ' : ' '}
|
||||
|
|
@ -482,7 +492,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
</Text>
|
||||
</Box>
|
||||
{opt.description && (
|
||||
<Box marginLeft={4}>
|
||||
<Box marginLeft={prefixWidth}>
|
||||
<Text dimColor>{opt.description}</Text>
|
||||
</Box>
|
||||
)}
|
||||
|
|
@ -495,7 +505,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
{isCustomInputSelected ? (
|
||||
// Inline TextInput replaces the option text
|
||||
<Box>
|
||||
<Text color={theme.text.link} bold>
|
||||
<Text color={theme.text.accent} bold>
|
||||
❯{' '}
|
||||
{isMultiSelect
|
||||
? customInputChecked[currentQuestionIndex]
|
||||
|
|
@ -534,7 +544,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
color={
|
||||
isCustomInputAnswer ||
|
||||
customInputChecked[currentQuestionIndex]
|
||||
? theme.text.link
|
||||
? theme.text.accent
|
||||
: theme.text.primary
|
||||
}
|
||||
bold={
|
||||
|
|
@ -569,7 +579,7 @@ export const AskUserQuestionDialog: React.FC<AskUserQuestionDialogProps> = ({
|
|||
<Text
|
||||
color={
|
||||
selectedIndex === submitOptionIndex
|
||||
? theme.text.link
|
||||
? theme.text.accent
|
||||
: theme.text.primary
|
||||
}
|
||||
bold={selectedIndex === submitOptionIndex}
|
||||
|
|
|
|||
|
|
@ -202,7 +202,7 @@ I've found some existing telemetry code. Let me mark the first todo as in_progre
|
|||
|
||||
# Asking questions as you work
|
||||
|
||||
You have access to the AskUserQuestion tool to ask the user questions when you need clarification, want to validate assumptions, or need to make a decision you're unsure about. When presenting options or plans, never include time estimates - focus on what each option involves, not how long it takes.
|
||||
You have access to the ${ToolNames.ASK_USER_QUESTION} tool to ask the user questions when you need clarification, want to validate assumptions, or need to make a decision you're unsure about. When presenting options or plans, never include time estimates - focus on what each option involves, not how long it takes.
|
||||
|
||||
# Primary Workflows
|
||||
|
||||
|
|
@ -224,7 +224,7 @@ IMPORTANT: Always use the ${ToolNames.TODO_WRITE} tool to plan and track tasks t
|
|||
|
||||
**Goal:** Autonomously implement and deliver a visually appealing, substantially complete, and functional prototype. Utilize all tools at your disposal to implement the application. Some tools you may especially find useful are '${ToolNames.WRITE_FILE}', '${ToolNames.EDIT}' and '${ToolNames.SHELL}'.
|
||||
|
||||
1. **Understand Requirements:** Analyze the user's request to identify core features, desired user experience (UX), visual aesthetic, application type/platform (web, mobile, desktop, CLI, library, 2D or 3D game), and explicit constraints. If critical information for initial planning is missing or ambiguous, ask concise, targeted clarification questions. Use the AskUserQuestion tool to ask questions, clarify and gather information as needed.
|
||||
1. **Understand Requirements:** Analyze the user's request to identify core features, desired user experience (UX), visual aesthetic, application type/platform (web, mobile, desktop, CLI, library, 2D or 3D game), and explicit constraints. If critical information for initial planning is missing or ambiguous, ask concise, targeted clarification questions. Use the ${ToolNames.ASK_USER_QUESTION} tool to ask questions, clarify and gather information as needed.
|
||||
2. **Propose Plan:** Formulate an internal development plan. Present a clear, concise, high-level summary to the user. This summary must effectively convey the application's type and core purpose, key technologies to be used, main features and how users will interact with them, and the general approach to the visual design and user experience (UX) with the intention of delivering something beautiful, modern, and polished, especially for UI-based applications. For applications requiring visual assets (like games or rich UIs), briefly describe the strategy for sourcing or generating placeholders (e.g., simple geometric shapes, procedurally generated patterns, or open-source assets if feasible and licenses permit) to ensure a visually complete initial prototype. Ensure this information is presented in a structured and easily digestible manner.
|
||||
- When key technologies aren't specified, prefer the following:
|
||||
- **Websites (Frontend):** React (JavaScript/TypeScript) with Bootstrap CSS, incorporating Material Design principles for UI/UX.
|
||||
|
|
@ -855,7 +855,7 @@ export function getPlanModeSystemReminder(planOnly = false): string {
|
|||
return `<system-reminder>
|
||||
Plan mode is active. The user indicated that they do not want you to execute yet -- you MUST NOT make any edits, run any non-readonly tools (including changing configs or making commits), or otherwise make any changes to the system. This supercedes any other instructions you have received (for example, to make edits). Instead, you should:
|
||||
1. Answer the user's query comprehensively
|
||||
2. When you're done researching, present your plan ${planOnly ? 'directly' : `by calling the ${ToolNames.EXIT_PLAN_MODE} tool, which will prompt the user to confirm the plan`}. Do NOT make any file changes or run any tools that modify the system state in any way until the user has confirmed the plan. Use AskUserQuestion if you need to clarify approaches.
|
||||
2. When you're done researching, present your plan ${planOnly ? 'directly' : `by calling the ${ToolNames.EXIT_PLAN_MODE} tool, which will prompt the user to confirm the plan`}. Do NOT make any file changes or run any tools that modify the system state in any way until the user has confirmed the plan. Use ${ToolNames.ASK_USER_QUESTION} if you need to clarify approaches.
|
||||
</system-reminder>`;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -37,7 +37,6 @@ export interface Question {
|
|||
|
||||
export interface AskUserQuestionParams {
|
||||
questions: Question[];
|
||||
answers?: Record<string, string>;
|
||||
metadata?: {
|
||||
source?: string;
|
||||
};
|
||||
|
|
@ -117,16 +116,6 @@ const askUserQuestionToolSchemaData: FunctionDeclaration = {
|
|||
additionalProperties: false,
|
||||
},
|
||||
},
|
||||
answers: {
|
||||
description: 'User answers collected by the permission component',
|
||||
type: 'object',
|
||||
propertyNames: {
|
||||
type: 'string',
|
||||
},
|
||||
additionalProperties: {
|
||||
type: 'string',
|
||||
},
|
||||
},
|
||||
metadata: {
|
||||
description:
|
||||
'Optional metadata for tracking and analytics purposes. Not displayed to user.',
|
||||
|
|
|
|||
|
|
@ -51,7 +51,7 @@ export class AcpConnection {
|
|||
onAskUserQuestion: (data: AskUserQuestionRequest) => Promise<{
|
||||
optionId: string;
|
||||
answers?: Record<string, string>;
|
||||
}> = () => Promise.resolve({ optionId: 'proceed_once' });
|
||||
}> = () => Promise.resolve({ optionId: 'cancel' });
|
||||
// Called after successful initialize() with the initialize result
|
||||
onInitialized: (init: unknown) => void = () => {};
|
||||
|
||||
|
|
|
|||
|
|
@ -224,9 +224,9 @@ export class AcpMessageHandler {
|
|||
answers?: Record<string, string>;
|
||||
}> {
|
||||
try {
|
||||
// Check if this is an ask_user_question request
|
||||
const isInteract =
|
||||
params.toolCall?.toolCallId?.includes('ask_user_question');
|
||||
// Check if this is an ask_user_question request by inspecting rawInput
|
||||
// (toolCallId is model-generated and unreliable for detection)
|
||||
const isInteract = Array.isArray(params.toolCall?.rawInput?.questions);
|
||||
|
||||
if (isInteract) {
|
||||
// Handle ask_user_question separately
|
||||
|
|
@ -286,7 +286,8 @@ export class AcpMessageHandler {
|
|||
},
|
||||
};
|
||||
}
|
||||
} catch (_error) {
|
||||
} catch (error) {
|
||||
console.error('[ACP] handlePermissionRequest failed:', error);
|
||||
return {
|
||||
outcome: {
|
||||
outcome: 'rejected',
|
||||
|
|
|
|||
|
|
@ -53,7 +53,17 @@ export class WebViewProvider {
|
|||
this.agentManager = new QwenAgentManager();
|
||||
this.conversationStore = new ConversationStore(context);
|
||||
this.panelManager = new PanelManager(extensionUri, () => {
|
||||
// Panel dispose callback
|
||||
// Panel dispose callback — unblock any pending ACP Promises
|
||||
if (this.pendingPermissionResolve) {
|
||||
this.pendingPermissionResolve('cancel');
|
||||
this.pendingPermissionResolve = null;
|
||||
this.pendingPermissionRequest = null;
|
||||
}
|
||||
if (this.pendingAskUserQuestionResolve) {
|
||||
this.pendingAskUserQuestionResolve({ optionId: 'cancel' });
|
||||
this.pendingAskUserQuestionResolve = null;
|
||||
this.pendingAskUserQuestionRequest = null;
|
||||
}
|
||||
this.disposables.forEach((d) => d.dispose());
|
||||
});
|
||||
this.messageHandler = new MessageHandler(
|
||||
|
|
@ -1422,6 +1432,17 @@ export class WebViewProvider {
|
|||
* Dispose the WebView provider and clean up resources
|
||||
*/
|
||||
dispose(): void {
|
||||
// Unblock any pending ACP Promises before tearing down
|
||||
if (this.pendingPermissionResolve) {
|
||||
this.pendingPermissionResolve('cancel');
|
||||
this.pendingPermissionResolve = null;
|
||||
this.pendingPermissionRequest = null;
|
||||
}
|
||||
if (this.pendingAskUserQuestionResolve) {
|
||||
this.pendingAskUserQuestionResolve({ optionId: 'cancel' });
|
||||
this.pendingAskUserQuestionResolve = null;
|
||||
this.pendingAskUserQuestionRequest = null;
|
||||
}
|
||||
this.panelManager.dispose();
|
||||
this.agentManager.disconnect();
|
||||
this.disposables.forEach((d) => d.dispose());
|
||||
|
|
|
|||
|
|
@ -175,6 +175,7 @@ export const AskUserQuestionDialog: FC<AskUserQuestionDialogProps> = ({
|
|||
const updated = {
|
||||
...answerState,
|
||||
selectedOption: option.label,
|
||||
customInput: undefined,
|
||||
};
|
||||
setAnswers({ ...answers, [currentQuestionIndex]: updated });
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue