fix(cli): honor argv @path in interactive sessions (quoted + unquoted) (#5952)

Co-authored-by: Allen Hutchison <adh@google.com>
This commit is contained in:
Tayyab3245
2025-09-30 21:18:04 -04:00
committed by GitHub
parent f2aa9d283a
commit c913ce3c0b
3 changed files with 191 additions and 23 deletions

View File

@@ -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 () => {

View File

@@ -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<CliArgs> {
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<CliArgs> {
'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<CliArgs> {
'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<CliArgs> {
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<CliArgs> {
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<string, unknown>)['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) {

View File

@@ -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,
});