mirror of
https://github.com/QwenLM/qwen-code.git
synced 2025-12-20 16:57:46 +00:00
feat: add --approval-mode parameter (#6024)
Co-authored-by: Jacob Richman <jacob314@gmail.com>
This commit is contained in:
Arya Gummadi
GitHub
@@ -156,6 +156,93 @@ describe('parseArguments', () => {
|
||||
expect(argv.promptInteractive).toBe('interactive prompt');
|
||||
expect(argv.prompt).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should throw an error when both --yolo and --approval-mode are used together', async () => {
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'--yolo',
|
||||
'--approval-mode',
|
||||
'default',
|
||||
];
|
||||
|
||||
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
|
||||
throw new Error('process.exit called');
|
||||
});
|
||||
|
||||
const mockConsoleError = vi
|
||||
.spyOn(console, 'error')
|
||||
.mockImplementation(() => {});
|
||||
|
||||
await expect(parseArguments()).rejects.toThrow('process.exit called');
|
||||
|
||||
expect(mockConsoleError).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.',
|
||||
),
|
||||
);
|
||||
|
||||
mockExit.mockRestore();
|
||||
mockConsoleError.mockRestore();
|
||||
});
|
||||
|
||||
it('should throw an error when using short flags -y and --approval-mode together', async () => {
|
||||
process.argv = ['node', 'script.js', '-y', '--approval-mode', 'yolo'];
|
||||
|
||||
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
|
||||
throw new Error('process.exit called');
|
||||
});
|
||||
|
||||
const mockConsoleError = vi
|
||||
.spyOn(console, 'error')
|
||||
.mockImplementation(() => {});
|
||||
|
||||
await expect(parseArguments()).rejects.toThrow('process.exit called');
|
||||
|
||||
expect(mockConsoleError).toHaveBeenCalledWith(
|
||||
expect.stringContaining(
|
||||
'Cannot use both --yolo (-y) and --approval-mode together. Use --approval-mode=yolo instead.',
|
||||
),
|
||||
);
|
||||
|
||||
mockExit.mockRestore();
|
||||
mockConsoleError.mockRestore();
|
||||
});
|
||||
|
||||
it('should allow --approval-mode without --yolo', async () => {
|
||||
process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit'];
|
||||
const argv = await parseArguments();
|
||||
expect(argv.approvalMode).toBe('auto_edit');
|
||||
expect(argv.yolo).toBe(false);
|
||||
});
|
||||
|
||||
it('should allow --yolo without --approval-mode', async () => {
|
||||
process.argv = ['node', 'script.js', '--yolo'];
|
||||
const argv = await parseArguments();
|
||||
expect(argv.yolo).toBe(true);
|
||||
expect(argv.approvalMode).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should reject invalid --approval-mode values', async () => {
|
||||
process.argv = ['node', 'script.js', '--approval-mode', 'invalid'];
|
||||
|
||||
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => {
|
||||
throw new Error('process.exit called');
|
||||
});
|
||||
|
||||
const mockConsoleError = vi
|
||||
.spyOn(console, 'error')
|
||||
.mockImplementation(() => {});
|
||||
|
||||
await expect(parseArguments()).rejects.toThrow('process.exit called');
|
||||
|
||||
expect(mockConsoleError).toHaveBeenCalledWith(
|
||||
expect.stringContaining('Invalid values:'),
|
||||
);
|
||||
|
||||
mockExit.mockRestore();
|
||||
mockConsoleError.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadCliConfig', () => {
|
||||
@@ -760,6 +847,211 @@ describe('mergeExcludeTools', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('Approval mode tool exclusion logic', () => {
|
||||
const originalIsTTY = process.stdin.isTTY;
|
||||
|
||||
beforeEach(() => {
|
||||
process.stdin.isTTY = false; // Ensure non-interactive mode
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.stdin.isTTY = originalIsTTY;
|
||||
});
|
||||
|
||||
it('should exclude all interactive tools in non-interactive mode with default approval mode', async () => {
|
||||
process.argv = ['node', 'script.js', '-p', 'test'];
|
||||
const argv = await parseArguments();
|
||||
const settings: Settings = {};
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
const config = await loadCliConfig(
|
||||
settings,
|
||||
extensions,
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
|
||||
const excludedTools = config.getExcludeTools();
|
||||
expect(excludedTools).toContain(ShellTool.Name);
|
||||
expect(excludedTools).toContain(EditTool.Name);
|
||||
expect(excludedTools).toContain(WriteFileTool.Name);
|
||||
});
|
||||
|
||||
it('should exclude all interactive tools in non-interactive mode with explicit default approval mode', async () => {
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'--approval-mode',
|
||||
'default',
|
||||
'-p',
|
||||
'test',
|
||||
];
|
||||
const argv = await parseArguments();
|
||||
const settings: Settings = {};
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
const config = await loadCliConfig(
|
||||
settings,
|
||||
extensions,
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
|
||||
const excludedTools = config.getExcludeTools();
|
||||
expect(excludedTools).toContain(ShellTool.Name);
|
||||
expect(excludedTools).toContain(EditTool.Name);
|
||||
expect(excludedTools).toContain(WriteFileTool.Name);
|
||||
});
|
||||
|
||||
it('should exclude only shell tools in non-interactive mode with auto_edit approval mode', async () => {
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'--approval-mode',
|
||||
'auto_edit',
|
||||
'-p',
|
||||
'test',
|
||||
];
|
||||
const argv = await parseArguments();
|
||||
const settings: Settings = {};
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
const config = await loadCliConfig(
|
||||
settings,
|
||||
extensions,
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
|
||||
const excludedTools = config.getExcludeTools();
|
||||
expect(excludedTools).toContain(ShellTool.Name);
|
||||
expect(excludedTools).not.toContain(EditTool.Name);
|
||||
expect(excludedTools).not.toContain(WriteFileTool.Name);
|
||||
});
|
||||
|
||||
it('should exclude no interactive tools in non-interactive mode with yolo approval mode', async () => {
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'--approval-mode',
|
||||
'yolo',
|
||||
'-p',
|
||||
'test',
|
||||
];
|
||||
const argv = await parseArguments();
|
||||
const settings: Settings = {};
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
const config = await loadCliConfig(
|
||||
settings,
|
||||
extensions,
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
|
||||
const excludedTools = config.getExcludeTools();
|
||||
expect(excludedTools).not.toContain(ShellTool.Name);
|
||||
expect(excludedTools).not.toContain(EditTool.Name);
|
||||
expect(excludedTools).not.toContain(WriteFileTool.Name);
|
||||
});
|
||||
|
||||
it('should exclude no interactive tools in non-interactive mode with legacy yolo flag', async () => {
|
||||
process.argv = ['node', 'script.js', '--yolo', '-p', 'test'];
|
||||
const argv = await parseArguments();
|
||||
const settings: Settings = {};
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
const config = await loadCliConfig(
|
||||
settings,
|
||||
extensions,
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
|
||||
const excludedTools = config.getExcludeTools();
|
||||
expect(excludedTools).not.toContain(ShellTool.Name);
|
||||
expect(excludedTools).not.toContain(EditTool.Name);
|
||||
expect(excludedTools).not.toContain(WriteFileTool.Name);
|
||||
});
|
||||
|
||||
it('should not exclude interactive tools in interactive mode regardless of approval mode', async () => {
|
||||
process.stdin.isTTY = true; // Interactive mode
|
||||
|
||||
const testCases = [
|
||||
{ args: ['node', 'script.js'] }, // default
|
||||
{ args: ['node', 'script.js', '--approval-mode', 'default'] },
|
||||
{ args: ['node', 'script.js', '--approval-mode', 'auto_edit'] },
|
||||
{ args: ['node', 'script.js', '--approval-mode', 'yolo'] },
|
||||
{ args: ['node', 'script.js', '--yolo'] },
|
||||
];
|
||||
|
||||
for (const testCase of testCases) {
|
||||
process.argv = testCase.args;
|
||||
const argv = await parseArguments();
|
||||
const settings: Settings = {};
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
const config = await loadCliConfig(
|
||||
settings,
|
||||
extensions,
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
|
||||
const excludedTools = config.getExcludeTools();
|
||||
expect(excludedTools).not.toContain(ShellTool.Name);
|
||||
expect(excludedTools).not.toContain(EditTool.Name);
|
||||
expect(excludedTools).not.toContain(WriteFileTool.Name);
|
||||
}
|
||||
});
|
||||
|
||||
it('should merge approval mode exclusions with settings exclusions in auto_edit mode', async () => {
|
||||
process.argv = [
|
||||
'node',
|
||||
'script.js',
|
||||
'--approval-mode',
|
||||
'auto_edit',
|
||||
'-p',
|
||||
'test',
|
||||
];
|
||||
const argv = await parseArguments();
|
||||
const settings: Settings = { excludeTools: ['custom_tool'] };
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
const config = await loadCliConfig(
|
||||
settings,
|
||||
extensions,
|
||||
'test-session',
|
||||
argv,
|
||||
);
|
||||
|
||||
const excludedTools = config.getExcludeTools();
|
||||
expect(excludedTools).toContain('custom_tool'); // From settings
|
||||
expect(excludedTools).toContain(ShellTool.Name); // From approval mode
|
||||
expect(excludedTools).not.toContain(EditTool.Name); // Should be allowed in auto_edit
|
||||
expect(excludedTools).not.toContain(WriteFileTool.Name); // Should be allowed in auto_edit
|
||||
});
|
||||
|
||||
it('should throw an error for invalid approval mode values in loadCliConfig', async () => {
|
||||
// Create a mock argv with an invalid approval mode that bypasses argument parsing validation
|
||||
const invalidArgv: Partial<CliArgs> & { approvalMode: string } = {
|
||||
approvalMode: 'invalid_mode',
|
||||
promptInteractive: '',
|
||||
prompt: '',
|
||||
yolo: false,
|
||||
};
|
||||
|
||||
const settings: Settings = {};
|
||||
const extensions: Extension[] = [];
|
||||
|
||||
await expect(
|
||||
loadCliConfig(settings, extensions, 'test-session', invalidArgv),
|
||||
).rejects.toThrow(
|
||||
'Invalid approval mode: invalid_mode. Valid values are: yolo, auto_edit, default',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadCliConfig with allowed-mcp-server-names', () => {
|
||||
const originalArgv = process.argv;
|
||||
const originalEnv = { ...process.env };
|
||||
@@ -1327,3 +1619,80 @@ describe('loadCliConfig interactive', () => {
|
||||
expect(config.isInteractive()).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('loadCliConfig approval mode', () => {
|
||||
const originalArgv = process.argv;
|
||||
const originalEnv = { ...process.env };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetAllMocks();
|
||||
vi.mocked(os.homedir).mockReturnValue('/mock/home/user');
|
||||
process.env.GEMINI_API_KEY = 'test-api-key';
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.argv = originalArgv;
|
||||
process.env = originalEnv;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should default to DEFAULT approval mode when no flags are set', async () => {
|
||||
process.argv = ['node', 'script.js'];
|
||||
const argv = await parseArguments();
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
|
||||
});
|
||||
|
||||
it('should set YOLO approval mode when --yolo flag is used', async () => {
|
||||
process.argv = ['node', 'script.js', '--yolo'];
|
||||
const argv = await parseArguments();
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.YOLO);
|
||||
});
|
||||
|
||||
it('should set YOLO approval mode when -y flag is used', async () => {
|
||||
process.argv = ['node', 'script.js', '-y'];
|
||||
const argv = await parseArguments();
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.YOLO);
|
||||
});
|
||||
|
||||
it('should set DEFAULT approval mode when --approval-mode=default', async () => {
|
||||
process.argv = ['node', 'script.js', '--approval-mode', 'default'];
|
||||
const argv = await parseArguments();
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
|
||||
});
|
||||
|
||||
it('should set AUTO_EDIT approval mode when --approval-mode=auto_edit', async () => {
|
||||
process.argv = ['node', 'script.js', '--approval-mode', 'auto_edit'];
|
||||
const argv = await parseArguments();
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.AUTO_EDIT);
|
||||
});
|
||||
|
||||
it('should set YOLO approval mode when --approval-mode=yolo', async () => {
|
||||
process.argv = ['node', 'script.js', '--approval-mode', 'yolo'];
|
||||
const argv = await parseArguments();
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.YOLO);
|
||||
});
|
||||
|
||||
it('should prioritize --approval-mode over --yolo when both would be valid (but validation prevents this)', async () => {
|
||||
// Note: This test documents the intended behavior, but in practice the validation
|
||||
// prevents both flags from being used together
|
||||
process.argv = ['node', 'script.js', '--approval-mode', 'default'];
|
||||
const argv = await parseArguments();
|
||||
// Manually set yolo to true to simulate what would happen if validation didn't prevent it
|
||||
argv.yolo = true;
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.DEFAULT);
|
||||
});
|
||||
|
||||
it('should fall back to --yolo behavior when --approval-mode is not set', async () => {
|
||||
process.argv = ['node', 'script.js', '--yolo'];
|
||||
const argv = await parseArguments();
|
||||
const config = await loadCliConfig({}, [], 'test-session', argv);
|
||||
expect(config.getApprovalMode()).toBe(ServerConfig.ApprovalMode.YOLO);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user