mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-16 09:01:17 -07:00
Re-submission: Make --allowed-tools work in non-interactive mode (#10289)
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: matt korwel <matt.korwel@gmail.com>
This commit is contained in:
@@ -2773,6 +2773,78 @@ describe('loadCliConfig tool exclusions', () => {
|
||||
expect(config.getExcludeTools()).not.toContain('replace');
|
||||
expect(config.getExcludeTools()).not.toContain('write_file');
|
||||
});
|
||||
|
||||
it('should not exclude shell tool in non-interactive mode when --allowed-tools="ShellTool" is set', async () => {
|
||||
process.stdin.isTTY = false;
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'-p',
|
||||
'test',
|
||||
'--allowed-tools',
|
||||
'ShellTool',
|
||||
];
|
||||
const argv = await parseArguments({} as Settings);
|
||||
const config = await loadCliConfig(
|
||||
{},
|
||||
[],
|
||||
new ExtensionEnablementManager(
|
||||
ExtensionStorage.getUserExtensionsDir(),
|
||||
argv.extensions,
|
||||
),
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
expect(config.getExcludeTools()).not.toContain(ShellTool.Name);
|
||||
});
|
||||
|
||||
it('should not exclude shell tool in non-interactive mode when --allowed-tools="run_shell_command" is set', async () => {
|
||||
process.stdin.isTTY = false;
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'-p',
|
||||
'test',
|
||||
'--allowed-tools',
|
||||
'run_shell_command',
|
||||
];
|
||||
const argv = await parseArguments({} as Settings);
|
||||
const config = await loadCliConfig(
|
||||
{},
|
||||
[],
|
||||
new ExtensionEnablementManager(
|
||||
ExtensionStorage.getUserExtensionsDir(),
|
||||
argv.extensions,
|
||||
),
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
expect(config.getExcludeTools()).not.toContain(ShellTool.Name);
|
||||
});
|
||||
|
||||
it('should not exclude shell tool in non-interactive mode when --allowed-tools="ShellTool(wc)" is set', async () => {
|
||||
process.stdin.isTTY = false;
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'-p',
|
||||
'test',
|
||||
'--allowed-tools',
|
||||
'ShellTool(wc)',
|
||||
];
|
||||
const argv = await parseArguments({} as Settings);
|
||||
const config = await loadCliConfig(
|
||||
{},
|
||||
[],
|
||||
new ExtensionEnablementManager(
|
||||
ExtensionStorage.getUserExtensionsDir(),
|
||||
argv.extensions,
|
||||
),
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
expect(config.getExcludeTools()).not.toContain(ShellTool.Name);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadCliConfig interactive', () => {
|
||||
|
||||
@@ -31,6 +31,7 @@ import {
|
||||
ShellTool,
|
||||
EditTool,
|
||||
WriteFileTool,
|
||||
SHELL_TOOL_NAMES,
|
||||
resolveTelemetrySettings,
|
||||
FatalConfigError,
|
||||
} from '@google/gemini-cli-core';
|
||||
@@ -430,6 +431,36 @@ export async function loadHierarchicalGeminiMemory(
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates a filter function to determine if a tool should be excluded.
|
||||
*
|
||||
* In non-interactive mode, we want to disable tools that require user
|
||||
* interaction to prevent the CLI from hanging. This function creates a predicate
|
||||
* that returns `true` if a tool should be excluded.
|
||||
*
|
||||
* A tool is excluded if it's not in the `allowedToolsSet`. The shell tool
|
||||
* has a special case: it's not excluded if any of its subcommands
|
||||
* are in the `allowedTools` list.
|
||||
*
|
||||
* @param allowedTools A list of explicitly allowed tool names.
|
||||
* @param allowedToolsSet A set of explicitly allowed tool names for quick lookups.
|
||||
* @returns A function that takes a tool name and returns `true` if it should be excluded.
|
||||
*/
|
||||
function createToolExclusionFilter(
|
||||
allowedTools: string[],
|
||||
allowedToolsSet: Set<string>,
|
||||
) {
|
||||
return (tool: string): boolean => {
|
||||
if (tool === ShellTool.Name) {
|
||||
// If any of the allowed tools is ShellTool (even with subcommands), don't exclude it.
|
||||
return !allowedTools.some((allowed) =>
|
||||
SHELL_TOOL_NAMES.some((shellName) => allowed.startsWith(shellName)),
|
||||
);
|
||||
}
|
||||
return !allowedToolsSet.has(tool);
|
||||
};
|
||||
}
|
||||
|
||||
export function isDebugMode(argv: CliArgs): boolean {
|
||||
return (
|
||||
argv.debug ||
|
||||
@@ -562,6 +593,9 @@ export async function loadCliConfig(
|
||||
|
||||
const policyEngineConfig = createPolicyEngineConfig(settings, approvalMode);
|
||||
|
||||
const allowedTools = argv.allowedTools || settings.tools?.allowed || [];
|
||||
const allowedToolsSet = new Set(allowedTools);
|
||||
|
||||
// Interactive mode: explicit -i flag or (TTY + no args + no -p flag)
|
||||
const hasQuery = !!argv.query;
|
||||
const interactive =
|
||||
@@ -570,14 +604,22 @@ export async function loadCliConfig(
|
||||
// In non-interactive mode, exclude tools that require a prompt.
|
||||
const extraExcludes: string[] = [];
|
||||
if (!interactive && !argv.experimentalAcp) {
|
||||
const defaultExcludes = [ShellTool.Name, EditTool.Name, WriteFileTool.Name];
|
||||
const autoEditExcludes = [ShellTool.Name];
|
||||
|
||||
const toolExclusionFilter = createToolExclusionFilter(
|
||||
allowedTools,
|
||||
allowedToolsSet,
|
||||
);
|
||||
|
||||
switch (approvalMode) {
|
||||
case ApprovalMode.DEFAULT:
|
||||
// In default non-interactive mode, all tools that require approval are excluded.
|
||||
extraExcludes.push(ShellTool.Name, EditTool.Name, WriteFileTool.Name);
|
||||
extraExcludes.push(...defaultExcludes.filter(toolExclusionFilter));
|
||||
break;
|
||||
case ApprovalMode.AUTO_EDIT:
|
||||
// In auto-edit non-interactive mode, only tools that still require a prompt are excluded.
|
||||
extraExcludes.push(ShellTool.Name);
|
||||
extraExcludes.push(...autoEditExcludes.filter(toolExclusionFilter));
|
||||
break;
|
||||
case ApprovalMode.YOLO:
|
||||
// No extra excludes for YOLO mode.
|
||||
@@ -649,7 +691,7 @@ export async function loadCliConfig(
|
||||
question,
|
||||
fullContext: argv.allFiles || false,
|
||||
coreTools: settings.tools?.core || undefined,
|
||||
allowedTools: argv.allowedTools || settings.tools?.allowed || undefined,
|
||||
allowedTools: allowedTools.length > 0 ? allowedTools : undefined,
|
||||
policyEngineConfig,
|
||||
excludeTools,
|
||||
toolDiscoveryCommand: settings.tools?.discoveryCommand,
|
||||
|
||||
@@ -873,4 +873,57 @@ describe('runNonInteractive', () => {
|
||||
|
||||
expect(processStdoutSpy).toHaveBeenCalledWith('Acknowledged');
|
||||
});
|
||||
|
||||
it('should allow a normally-excluded tool when --allowed-tools is set', async () => {
|
||||
// By default, ShellTool is excluded in non-interactive mode.
|
||||
// This test ensures that --allowed-tools overrides this exclusion.
|
||||
vi.mocked(mockConfig.getToolRegistry).mockReturnValue({
|
||||
getTool: vi.fn().mockReturnValue({
|
||||
name: 'ShellTool',
|
||||
description: 'A shell tool',
|
||||
run: vi.fn(),
|
||||
}),
|
||||
getFunctionDeclarations: vi.fn().mockReturnValue([{ name: 'ShellTool' }]),
|
||||
} as unknown as ToolRegistry);
|
||||
|
||||
const toolCallEvent: ServerGeminiStreamEvent = {
|
||||
type: GeminiEventType.ToolCallRequest,
|
||||
value: {
|
||||
callId: 'tool-shell-1',
|
||||
name: 'ShellTool',
|
||||
args: { command: 'ls' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'prompt-id-allowed',
|
||||
},
|
||||
};
|
||||
const toolResponse: Part[] = [{ text: 'file.txt' }];
|
||||
mockCoreExecuteToolCall.mockResolvedValue({ responseParts: toolResponse });
|
||||
|
||||
const firstCallEvents: ServerGeminiStreamEvent[] = [toolCallEvent];
|
||||
const secondCallEvents: ServerGeminiStreamEvent[] = [
|
||||
{ type: GeminiEventType.Content, value: 'file.txt' },
|
||||
{
|
||||
type: GeminiEventType.Finished,
|
||||
value: { reason: undefined, usageMetadata: { totalTokenCount: 10 } },
|
||||
},
|
||||
];
|
||||
|
||||
mockGeminiClient.sendMessageStream
|
||||
.mockReturnValueOnce(createStreamFromEvents(firstCallEvents))
|
||||
.mockReturnValueOnce(createStreamFromEvents(secondCallEvents));
|
||||
|
||||
await runNonInteractive(
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
'List the files',
|
||||
'prompt-id-allowed',
|
||||
);
|
||||
|
||||
expect(mockCoreExecuteToolCall).toHaveBeenCalledWith(
|
||||
mockConfig,
|
||||
expect.objectContaining({ name: 'ShellTool' }),
|
||||
expect.any(AbortSignal),
|
||||
);
|
||||
expect(processStdoutSpy).toHaveBeenCalledWith('file.txt');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user