mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-11 06:31:01 -07:00
fix(plan): make question type required in AskUser tool (#18959)
This commit is contained in:
@@ -37,6 +37,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which authentication method should we use?',
|
||||
header: 'Auth',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'OAuth 2.0', description: 'Industry standard, supports SSO' },
|
||||
{ label: 'JWT tokens', description: 'Stateless, good for APIs' },
|
||||
@@ -74,6 +75,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which features?',
|
||||
header: 'Features',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'TypeScript', description: '' },
|
||||
{ label: 'ESLint', description: '' },
|
||||
@@ -171,6 +173,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which authentication method?',
|
||||
header: 'Auth',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'OAuth 2.0', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -228,6 +231,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Choose an option',
|
||||
header: 'Scroll Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: Array.from({ length: 15 }, (_, i) => ({
|
||||
label: `Option ${i + 1}`,
|
||||
description: `Description ${i + 1}`,
|
||||
@@ -296,6 +300,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which database should we use?',
|
||||
header: 'Database',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'PostgreSQL', description: 'Relational database' },
|
||||
{ label: 'MongoDB', description: 'Document database' },
|
||||
@@ -305,6 +310,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which ORM do you prefer?',
|
||||
header: 'ORM',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'Prisma', description: 'Type-safe ORM' },
|
||||
{ label: 'Drizzle', description: 'Lightweight ORM' },
|
||||
@@ -359,12 +365,14 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which testing framework?',
|
||||
header: 'Testing',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Vitest', description: 'Fast unit testing' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
{
|
||||
question: 'Which CI provider?',
|
||||
header: 'CI',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'GitHub Actions', description: 'Built into GitHub' },
|
||||
],
|
||||
@@ -402,12 +410,14 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which package manager?',
|
||||
header: 'Package',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'pnpm', description: 'Fast, disk efficient' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
{
|
||||
question: 'Which bundler?',
|
||||
header: 'Bundler',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Vite', description: 'Next generation bundler' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -465,6 +475,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which framework?',
|
||||
header: 'Framework',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'React', description: 'Component library' },
|
||||
{ label: 'Vue', description: 'Progressive framework' },
|
||||
@@ -474,6 +485,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which styling?',
|
||||
header: 'Styling',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'Tailwind', description: 'Utility-first CSS' },
|
||||
{ label: 'CSS Modules', description: 'Scoped styles' },
|
||||
@@ -500,12 +512,14 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Create tests?',
|
||||
header: 'Tests',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: 'Generate test files' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
{
|
||||
question: 'Add documentation?',
|
||||
header: 'Docs',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: 'Generate JSDoc comments' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -545,12 +559,14 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which license?',
|
||||
header: 'License',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'MIT', description: 'Permissive license' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
{
|
||||
question: 'Include README?',
|
||||
header: 'README',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: 'Generate README.md' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -580,12 +596,14 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Target Node version?',
|
||||
header: 'Node',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Node 20', description: 'LTS version' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
{
|
||||
question: 'Enable strict mode?',
|
||||
header: 'Strict',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: 'Strict TypeScript' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -727,6 +745,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Should it be async?',
|
||||
header: 'Async',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'Yes', description: 'Use async/await' },
|
||||
{ label: 'No', description: 'Synchronous hook' },
|
||||
@@ -773,6 +792,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which styling approach?',
|
||||
header: 'Style',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'CSS Modules', description: 'Scoped CSS' },
|
||||
{ label: 'Tailwind', description: 'Utility classes' },
|
||||
@@ -895,6 +915,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Choice Q?',
|
||||
header: 'Choice',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Option 1', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -952,12 +973,14 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Question 1?',
|
||||
header: 'Q1',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'A1', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
{
|
||||
question: 'Question 2?',
|
||||
header: 'Q2',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'A2', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -1008,6 +1031,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Which option do you prefer?',
|
||||
header: 'Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -1036,6 +1060,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Is **this** working?',
|
||||
header: 'Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -1067,6 +1092,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Is **this** working?',
|
||||
header: 'Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -1096,6 +1122,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Run `npm start`?',
|
||||
header: 'Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Yes', description: '' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -1126,6 +1153,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Choose an option',
|
||||
header: 'Context Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: Array.from({ length: 10 }, (_, i) => ({
|
||||
label: `Option ${i + 1}`,
|
||||
description: `Description ${i + 1}`,
|
||||
@@ -1162,6 +1190,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: longQuestion,
|
||||
header: 'Alternate Buffer Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'Option 1', description: 'Desc 1' }],
|
||||
multiSelect: false,
|
||||
},
|
||||
@@ -1195,6 +1224,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Select your preferred language:',
|
||||
header: 'Language',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'TypeScript', description: '' },
|
||||
{ label: 'JavaScript', description: '' },
|
||||
@@ -1228,6 +1258,7 @@ describe('AskUserDialog', () => {
|
||||
{
|
||||
question: 'Select your preferred language:',
|
||||
header: 'Language',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'TypeScript', description: '' },
|
||||
{ label: 'JavaScript', description: '' },
|
||||
|
||||
@@ -9,7 +9,7 @@ import { act } from 'react';
|
||||
import { renderWithProviders } from '../../test-utils/render.js';
|
||||
import { waitFor } from '../../test-utils/async.js';
|
||||
import { AskUserDialog } from './AskUserDialog.js';
|
||||
import type { Question } from '@google/gemini-cli-core';
|
||||
import { QuestionType, type Question } from '@google/gemini-cli-core';
|
||||
|
||||
describe('Key Bubbling Regression', () => {
|
||||
afterEach(() => {
|
||||
@@ -20,6 +20,7 @@ describe('Key Bubbling Regression', () => {
|
||||
{
|
||||
question: 'Choice Q?',
|
||||
header: 'Choice',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'Option 1', description: '' },
|
||||
{ label: 'Option 2', description: '' },
|
||||
|
||||
@@ -147,9 +147,9 @@ export enum QuestionType {
|
||||
export interface Question {
|
||||
question: string;
|
||||
header: string;
|
||||
/** Question type: 'choice' renders selectable options, 'text' renders free-form input, 'yesno' renders a binary Yes/No choice. Defaults to 'choice'. */
|
||||
type?: QuestionType;
|
||||
/** Selectable choices. REQUIRED when type='choice' or omitted. IGNORED for 'text' and 'yesno'. */
|
||||
/** Question type: 'choice' renders selectable options, 'text' renders free-form input, 'yesno' renders a binary Yes/No choice. */
|
||||
type: QuestionType;
|
||||
/** Selectable choices. REQUIRED when type='choice'. IGNORED for 'text' and 'yesno'. */
|
||||
options?: QuestionOption[];
|
||||
/** Allow multiple selections. Only applies when type='choice'. */
|
||||
multiSelect?: boolean;
|
||||
|
||||
@@ -131,6 +131,7 @@ describe('AskUserTool', () => {
|
||||
const questions = Array(5).fill({
|
||||
question: 'Test?',
|
||||
header: 'Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'A', description: 'A' },
|
||||
{ label: 'B', description: 'B' },
|
||||
@@ -156,7 +157,13 @@ describe('AskUserTool', () => {
|
||||
|
||||
it('should return error if header exceeds max length', () => {
|
||||
const result = tool.validateToolParams({
|
||||
questions: [{ question: 'Test?', header: 'This is way too long' }],
|
||||
questions: [
|
||||
{
|
||||
question: 'Test?',
|
||||
header: 'This is way too long',
|
||||
type: QuestionType.CHOICE,
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(result).toContain('must NOT have more than 16 characters');
|
||||
});
|
||||
@@ -167,6 +174,7 @@ describe('AskUserTool', () => {
|
||||
{
|
||||
question: 'Test?',
|
||||
header: 'Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [{ label: 'A', description: 'A' }],
|
||||
},
|
||||
],
|
||||
@@ -182,6 +190,7 @@ describe('AskUserTool', () => {
|
||||
{
|
||||
question: 'Test?',
|
||||
header: 'Test',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'A', description: 'A' },
|
||||
{ label: 'B', description: 'B' },
|
||||
@@ -201,6 +210,7 @@ describe('AskUserTool', () => {
|
||||
{
|
||||
question: 'Which approach?',
|
||||
header: 'Approach',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'A', description: 'Option A' },
|
||||
{ label: 'B', description: 'Option B' },
|
||||
@@ -224,18 +234,16 @@ describe('AskUserTool', () => {
|
||||
expect(result).toContain("type='choice' requires 'options'");
|
||||
});
|
||||
|
||||
it('should return error if type is omitted and options missing (defaults to choice)', () => {
|
||||
it('should return error if type is missing', () => {
|
||||
const result = tool.validateToolParams({
|
||||
questions: [
|
||||
{
|
||||
question: 'Pick one?',
|
||||
header: 'Choice',
|
||||
// type omitted, defaults to 'choice'
|
||||
// options missing
|
||||
},
|
||||
} as unknown as Question,
|
||||
],
|
||||
});
|
||||
expect(result).toContain("type='choice' requires 'options'");
|
||||
expect(result).toContain("must have required property 'type'");
|
||||
});
|
||||
|
||||
it('should accept text type without options', () => {
|
||||
@@ -288,6 +296,7 @@ describe('AskUserTool', () => {
|
||||
{
|
||||
question: 'Pick one?',
|
||||
header: 'Choice',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: '', description: 'Empty label' },
|
||||
{ label: 'B', description: 'Option B' },
|
||||
@@ -304,6 +313,7 @@ describe('AskUserTool', () => {
|
||||
{
|
||||
question: 'Pick one?',
|
||||
header: 'Choice',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'A' } as { label: string; description: string },
|
||||
{ label: 'B', description: 'Option B' },
|
||||
@@ -318,7 +328,13 @@ describe('AskUserTool', () => {
|
||||
describe('validateBuildAndExecute', () => {
|
||||
it('should hide validation errors from returnDisplay', async () => {
|
||||
const params = {
|
||||
questions: [{ question: 'Test?', header: 'This is way too long' }],
|
||||
questions: [
|
||||
{
|
||||
question: 'Test?',
|
||||
header: 'This is way too long',
|
||||
type: QuestionType.TEXT,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const result = await tool.validateBuildAndExecute(
|
||||
@@ -337,7 +353,9 @@ describe('AskUserTool', () => {
|
||||
.mockReturnValue(null);
|
||||
|
||||
const params = {
|
||||
questions: [{ question: 'Valid?', header: 'Valid' }],
|
||||
questions: [
|
||||
{ question: 'Valid?', header: 'Valid', type: QuestionType.TEXT },
|
||||
],
|
||||
};
|
||||
|
||||
const mockInvocation = {
|
||||
@@ -366,10 +384,11 @@ describe('AskUserTool', () => {
|
||||
|
||||
describe('shouldConfirmExecute', () => {
|
||||
it('should return confirmation details with normalized questions', async () => {
|
||||
const questions = [
|
||||
const questions: Question[] = [
|
||||
{
|
||||
question: 'How should we proceed with this task?',
|
||||
header: 'Approach',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{
|
||||
label: 'Quick fix (Recommended)',
|
||||
@@ -394,12 +413,7 @@ describe('AskUserTool', () => {
|
||||
expect(details).not.toBe(false);
|
||||
if (details && details.type === 'ask_user') {
|
||||
expect(details.title).toBe('Ask User');
|
||||
expect(details.questions).toEqual(
|
||||
questions.map((q) => ({
|
||||
...q,
|
||||
type: QuestionType.CHOICE,
|
||||
})),
|
||||
);
|
||||
expect(details.questions).toEqual(questions);
|
||||
expect(typeof details.onConfirm).toBe('function');
|
||||
} else {
|
||||
// Type guard for TypeScript
|
||||
@@ -407,11 +421,12 @@ describe('AskUserTool', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('should normalize question type to CHOICE when omitted', async () => {
|
||||
const questions = [
|
||||
it('should use provided question type', async () => {
|
||||
const questions: Question[] = [
|
||||
{
|
||||
question: 'Which approach?',
|
||||
header: 'Approach',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'Option A', description: 'First option' },
|
||||
{ label: 'Option B', description: 'Second option' },
|
||||
@@ -432,10 +447,11 @@ describe('AskUserTool', () => {
|
||||
|
||||
describe('execute', () => {
|
||||
it('should return user answers after confirmation', async () => {
|
||||
const questions = [
|
||||
const questions: Question[] = [
|
||||
{
|
||||
question: 'How should we proceed with this task?',
|
||||
header: 'Approach',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{
|
||||
label: 'Quick fix (Recommended)',
|
||||
@@ -484,10 +500,11 @@ describe('AskUserTool', () => {
|
||||
});
|
||||
|
||||
it('should display message when user submits without answering', async () => {
|
||||
const questions = [
|
||||
const questions: Question[] = [
|
||||
{
|
||||
question: 'Which approach?',
|
||||
header: 'Approach',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{ label: 'Option A', description: 'First option' },
|
||||
{ label: 'Option B', description: 'Second option' },
|
||||
@@ -528,6 +545,7 @@ describe('AskUserTool', () => {
|
||||
{
|
||||
question: 'Which sections of the documentation should be updated?',
|
||||
header: 'Docs',
|
||||
type: QuestionType.CHOICE,
|
||||
options: [
|
||||
{
|
||||
label: 'User Guide',
|
||||
|
||||
@@ -42,7 +42,7 @@ export class AskUserTool extends BaseDeclarativeTool<
|
||||
maxItems: 4,
|
||||
items: {
|
||||
type: 'object',
|
||||
required: ['question', 'header'],
|
||||
required: ['question', 'header', 'type'],
|
||||
properties: {
|
||||
question: {
|
||||
type: 'string',
|
||||
@@ -111,7 +111,7 @@ export class AskUserTool extends BaseDeclarativeTool<
|
||||
|
||||
for (let i = 0; i < params.questions.length; i++) {
|
||||
const q = params.questions[i];
|
||||
const questionType = q.type ?? QuestionType.CHOICE;
|
||||
const questionType = q.type;
|
||||
|
||||
// Validate that 'choice' type has options
|
||||
if (questionType === QuestionType.CHOICE) {
|
||||
@@ -186,7 +186,7 @@ export class AskUserInvocation extends BaseToolInvocation<
|
||||
): Promise<ToolAskUserConfirmationDetails | false> {
|
||||
const normalizedQuestions = this.params.questions.map((q) => ({
|
||||
...q,
|
||||
type: q.type ?? QuestionType.CHOICE,
|
||||
type: q.type,
|
||||
}));
|
||||
|
||||
return {
|
||||
@@ -210,9 +210,7 @@ export class AskUserInvocation extends BaseToolInvocation<
|
||||
}
|
||||
|
||||
async execute(_signal: AbortSignal): Promise<ToolResult> {
|
||||
const questionTypes = this.params.questions.map(
|
||||
(q) => q.type ?? QuestionType.CHOICE,
|
||||
);
|
||||
const questionTypes = this.params.questions.map((q) => q.type);
|
||||
|
||||
if (this.confirmationOutcome === ToolConfirmationOutcome.Cancel) {
|
||||
return {
|
||||
|
||||
Reference in New Issue
Block a user