From c913ce3c0b3f578706db8a1a2615e847eca3323a Mon Sep 17 00:00:00 2001 From: Tayyab3245 Date: Tue, 30 Sep 2025 21:18:04 -0400 Subject: [PATCH] fix(cli): honor argv @path in interactive sessions (quoted + unquoted) (#5952) Co-authored-by: Allen Hutchison --- packages/cli/src/config/config.test.ts | 142 ++++++++++++++++++++++++- packages/cli/src/config/config.ts | 70 ++++++++---- packages/cli/src/gemini.test.tsx | 2 +- 3 files changed, 191 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index ea7b9d6b98..22e07cc5dd 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -207,6 +207,136 @@ describe('parseArguments', () => { expect(argv.prompt).toBeUndefined(); }); + it('should convert positional query argument to prompt by default', async () => { + process.argv = ['node', 'script.js', 'Hi Gemini']; + const argv = await parseArguments({} as Settings); + expect(argv.query).toBe('Hi Gemini'); + expect(argv.prompt).toBe('Hi Gemini'); + expect(argv.promptInteractive).toBeUndefined(); + }); + + it('should map @path to prompt (one-shot) when it starts with @', async () => { + process.argv = ['node', 'script.js', '@path ./file.md']; + const argv = await parseArguments({} as Settings); + expect(argv.query).toBe('@path ./file.md'); + expect(argv.prompt).toBe('@path ./file.md'); + expect(argv.promptInteractive).toBeUndefined(); + }); + + it('should map @path to prompt even when config flags are present', async () => { + // @path queries should now go to one-shot mode regardless of other flags + process.argv = [ + 'node', + 'script.js', + '@path', + './file.md', + '--model', + 'gemini-1.5-pro', + ]; + const argv = await parseArguments({} as Settings); + expect(argv.query).toBe('@path ./file.md'); + expect(argv.prompt).toBe('@path ./file.md'); // Should map to one-shot + expect(argv.promptInteractive).toBeUndefined(); + expect(argv.model).toBe('gemini-1.5-pro'); + }); + + it('maps unquoted positional @path + arg to prompt (one-shot)', async () => { + // Simulate: gemini @path ./file.md + process.argv = ['node', 'script.js', '@path', './file.md']; + const argv = await parseArguments({} as Settings); + // After normalization, query is a single string + expect(argv.query).toBe('@path ./file.md'); + // And it's mapped to one-shot prompt when no -p/-i flags are set + expect(argv.prompt).toBe('@path ./file.md'); + expect(argv.promptInteractive).toBeUndefined(); + }); + + it('should handle multiple @path arguments in a single command (one-shot)', async () => { + // Simulate: gemini @path ./file1.md @path ./file2.md + process.argv = [ + 'node', + 'script.js', + '@path', + './file1.md', + '@path', + './file2.md', + ]; + const argv = await parseArguments({} as Settings); + // After normalization, all arguments are joined with spaces + expect(argv.query).toBe('@path ./file1.md @path ./file2.md'); + // And it's mapped to one-shot prompt + expect(argv.prompt).toBe('@path ./file1.md @path ./file2.md'); + expect(argv.promptInteractive).toBeUndefined(); + }); + + it('should handle mixed quoted and unquoted @path arguments (one-shot)', async () => { + // Simulate: gemini "@path ./file1.md" @path ./file2.md "additional text" + process.argv = [ + 'node', + 'script.js', + '@path ./file1.md', + '@path', + './file2.md', + 'additional text', + ]; + const argv = await parseArguments({} as Settings); + // After normalization, all arguments are joined with spaces + expect(argv.query).toBe( + '@path ./file1.md @path ./file2.md additional text', + ); + // And it's mapped to one-shot prompt + expect(argv.prompt).toBe( + '@path ./file1.md @path ./file2.md additional text', + ); + expect(argv.promptInteractive).toBeUndefined(); + }); + + it('should map @path to prompt with ambient flags (debug, telemetry)', async () => { + // Ambient flags like debug, telemetry should NOT affect routing + process.argv = [ + 'node', + 'script.js', + '@path', + './file.md', + '--debug', + '--telemetry', + ]; + const argv = await parseArguments({} as Settings); + expect(argv.query).toBe('@path ./file.md'); + expect(argv.prompt).toBe('@path ./file.md'); // Should map to one-shot + expect(argv.promptInteractive).toBeUndefined(); + expect(argv.debug).toBe(true); + expect(argv.telemetry).toBe(true); + }); + + it('should map any @command to prompt (one-shot)', async () => { + // Test that all @commands now go to one-shot mode + const testCases = [ + '@path ./file.md', + '@include src/', + '@search pattern', + '@web query', + '@git status', + ]; + + for (const testQuery of testCases) { + process.argv = ['node', 'script.js', testQuery]; + const argv = await parseArguments({} as Settings); + expect(argv.query).toBe(testQuery); + expect(argv.prompt).toBe(testQuery); + expect(argv.promptInteractive).toBeUndefined(); + } + }); + + it('should handle @command with leading whitespace', async () => { + // Test that trim() + routing handles leading whitespace correctly + process.argv = ['node', 'script.js', ' @path ./file.md']; + const argv = await parseArguments({} as Settings); + expect(argv.query).toBe(' @path ./file.md'); + expect(argv.prompt).toBe(' @path ./file.md'); + expect(argv.promptInteractive).toBeUndefined(); + }); + it('should throw an error when both --yolo and --approval-mode are used together', async () => { process.argv = [ 'node', @@ -2702,7 +2832,7 @@ describe('loadCliConfig interactive', () => { 'script.js', '--model', 'gemini-1.5-pro', - '--sandbox', + '--yolo', 'Hello world', ]; const argv = await parseArguments({} as Settings); @@ -2717,6 +2847,9 @@ describe('loadCliConfig interactive', () => { argv, ); expect(config.isInteractive()).toBe(false); + // Verify the question is preserved for one-shot execution + expect(argv.prompt).toBe('Hello world'); + expect(argv.promptInteractive).toBeUndefined(); }); it('should be interactive if no positional prompt words are provided with flags', async () => { @@ -3155,10 +3288,13 @@ describe('parseArguments with positional prompt', () => { mockConsoleError.mockRestore(); }); - it('should correctly parse a positional prompt', async () => { + it('should correctly parse a positional prompt to query field', async () => { process.argv = ['node', 'script.js', 'positional', 'prompt']; const argv = await parseArguments({} as Settings); - expect(argv.promptWords).toEqual(['positional', 'prompt']); + expect(argv.query).toBe('positional prompt'); + // Since no explicit prompt flags are set and query doesn't start with @, should map to prompt (one-shot) + expect(argv.prompt).toBe('positional prompt'); + expect(argv.promptInteractive).toBeUndefined(); }); it('should correctly parse a prompt from the --prompt flag', async () => { diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index a3a294a00b..b3a51976a8 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -58,6 +58,7 @@ const logger = { }; export interface CliArgs { + query: string | undefined; model: string | undefined; sandbox: boolean | string | undefined; sandboxImage: string | undefined; @@ -85,12 +86,12 @@ export interface CliArgs { screenReader: boolean | undefined; useSmartEdit: boolean | undefined; useWriteTodos: boolean | undefined; - promptWords: string[] | undefined; outputFormat: string | undefined; } export async function parseArguments(settings: Settings): Promise { - const yargsInstance = yargs(hideBin(process.argv)) + const rawArgv = hideBin(process.argv); + const yargsInstance = yargs(rawArgv) .locale('en') .scriptName('gemini') .usage( @@ -166,8 +167,12 @@ export async function parseArguments(settings: Settings): Promise { 'proxy', 'Use the "proxy" setting in settings.json instead. This flag will be removed in a future version.', ) - .command('$0 [promptWords...]', 'Launch Gemini CLI', (yargsInstance) => + .command('$0 [query..]', 'Launch Gemini CLI', (yargsInstance) => yargsInstance + .positional('query', { + description: + 'Positional prompt. Defaults to one-shot; use -i/--prompt-interactive for interactive.', + }) .option('model', { alias: 'm', type: 'string', @@ -301,22 +306,28 @@ export async function parseArguments(settings: Settings): Promise { 'prompt', 'Use the positional prompt instead. This flag will be removed in a future version.', ) + // Ensure validation flows through .fail() for clean UX + .fail((msg, err, yargs) => { + console.error(msg || err?.message || 'Unknown error'); + yargs.showHelp(); + process.exit(1); + }) .check((argv) => { - const promptWords = argv['promptWords'] as string[] | undefined; - if (argv['prompt'] && promptWords && promptWords.length > 0) { - throw new Error( - 'Cannot use both a positional prompt and the --prompt (-p) flag together', - ); + // The 'query' positional can be a string (for one arg) or string[] (for multiple). + // This guard safely checks if any positional argument was provided. + const query = argv['query'] as string | string[] | undefined; + const hasPositionalQuery = Array.isArray(query) + ? query.length > 0 + : !!query; + + if (argv['prompt'] && hasPositionalQuery) { + return 'Cannot use both a positional prompt and the --prompt (-p) flag together'; } if (argv['prompt'] && argv['promptInteractive']) { - throw new Error( - 'Cannot use both --prompt (-p) and --prompt-interactive (-i) together', - ); + return 'Cannot use both --prompt (-p) and --prompt-interactive (-i) together'; } if (argv.yolo && argv['approvalMode']) { - throw new Error( - 'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.', - ); + return 'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.'; } return true; }), @@ -339,6 +350,8 @@ export async function parseArguments(settings: Settings): Promise { yargsInstance.wrap(yargsInstance.terminalWidth()); const result = await yargsInstance.parse(); + // If yargs handled --help/--version it will have exited; nothing to do here. + // Handle case where MCP subcommands are executed - they should exit the process // and not return to main CLI logic if ( @@ -349,6 +362,26 @@ export async function parseArguments(settings: Settings): Promise { process.exit(0); } + // Normalize query args: handle both quoted "@path file" and unquoted @path file + const queryArg = (result as { query?: string | string[] | undefined }).query; + const q: string | undefined = Array.isArray(queryArg) + ? queryArg.join(' ') + : queryArg; + + // Route positional args: explicit -i flag -> interactive; else -> one-shot (even for @commands) + if (q && !result['prompt']) { + const hasExplicitInteractive = + result['promptInteractive'] === '' || !!result['promptInteractive']; + if (hasExplicitInteractive) { + result['promptInteractive'] = q; + } else { + result['prompt'] = q; + } + } + + // Keep CliArgs.query as a string for downstream typing + (result as Record)['query'] = q || undefined; + // The import format is now only controlled by settings.memoryImportFormat // We no longer accept it as a CLI argument return result as unknown as CliArgs; @@ -475,8 +508,7 @@ export async function loadCliConfig( ); let mcpServers = mergeMcpServers(settings, activeExtensions); - const question = - argv.promptInteractive || argv.prompt || (argv.promptWords || []).join(' '); + const question = argv.promptInteractive || argv.prompt || ''; // Determine approval mode with backward compatibility let approvalMode: ApprovalMode; @@ -529,11 +561,11 @@ export async function loadCliConfig( const policyEngineConfig = createPolicyEngineConfig(settings, approvalMode); - // Fix: If promptWords are provided, always use non-interactive mode - const hasPromptWords = argv.promptWords && argv.promptWords.length > 0; + // Interactive mode: explicit -i flag or (TTY + no args + no -p flag) + const hasQuery = !!argv.query; const interactive = !!argv.promptInteractive || - (process.stdin.isTTY && !hasPromptWords && !argv.prompt); + (process.stdin.isTTY && !hasQuery && !argv.prompt); // In non-interactive mode, exclude tools that require a prompt. const extraExcludes: string[] = []; if (!interactive && !argv.experimentalAcp) { diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index 4bad3fd980..4d6b894393 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -307,6 +307,7 @@ describe('gemini.tsx main function kitty protocol', () => { debug: undefined, prompt: undefined, promptInteractive: undefined, + query: undefined, allFiles: undefined, showMemoryUsage: undefined, yolo: undefined, @@ -328,7 +329,6 @@ describe('gemini.tsx main function kitty protocol', () => { screenReader: undefined, useSmartEdit: undefined, useWriteTodos: undefined, - promptWords: undefined, outputFormat: undefined, });