Skip to content

Commit f64da4a

Browse files
committed
refactor(cli): centralize and constantize workspace policy auto-accept flag
Refactor AUTO_ACCEPT_WORKSPACE_POLICIES from a mutable variable in policy.ts to a constant in config.ts, addressing review feedback to improve code maintainability and centralize configuration. - Move AUTO_ACCEPT_WORKSPACE_POLICIES constant to config.ts with updated documentation comment for clarity. - Update resolveWorkspacePolicyState to accept autoAcceptWorkspacePolicies as an explicit option, removing dependency on global state. - Update LoadCliConfigOptions to include autoAcceptWorkspacePolicies, enabling precise control over the flag during configuration loading. - Refactor unit and integration tests to pass the flag directly instead of using monkey patching, ensuring cleaner test isolation. - Clean up unused imports and setter functions in policy logic. This change eliminates mutable global state for the policy auto-accept behavior while maintaining the new default UX of automatically accepting workspace policies.
1 parent b6475aa commit f64da4a

4 files changed

Lines changed: 90 additions & 108 deletions

File tree

packages/cli/src/config/config.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,11 @@ import { promptForSetting } from './extensions/extensionSettings.js';
6868
import type { EventEmitter } from 'node:stream';
6969
import { runExitCleanup } from '../utils/cleanup.js';
7070

71+
/**
72+
* Temporary flag to automatically accept workspace policies to reduce UX friction.
73+
*/
74+
export const AUTO_ACCEPT_WORKSPACE_POLICIES = true;
75+
7176
export interface CliArgs {
7277
query: string | undefined;
7378
model: string | undefined;
@@ -439,6 +444,7 @@ export interface LoadCliConfigOptions {
439444
projectHooks?: { [K in HookEventName]?: HookDefinition[] } & {
440445
disabled?: string[];
441446
};
447+
autoAcceptWorkspacePolicies?: boolean;
442448
}
443449

444450
export async function loadCliConfig(
@@ -700,6 +706,8 @@ export async function loadCliConfig(
700706
cwd,
701707
trustedFolder,
702708
interactive,
709+
autoAcceptWorkspacePolicies:
710+
options.autoAcceptWorkspacePolicies ?? AUTO_ACCEPT_WORKSPACE_POLICIES,
703711
});
704712

705713
const policyEngineConfig = await createPolicyEngineConfig(

packages/cli/src/config/policy.test.ts

Lines changed: 39 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
88
import * as fs from 'node:fs';
99
import * as path from 'node:path';
1010
import * as os from 'node:os';
11-
import {
12-
resolveWorkspacePolicyState,
13-
AUTO_ACCEPT_WORKSPACE_POLICIES,
14-
setAutoAcceptWorkspacePolicies,
15-
} from './policy.js';
11+
import { resolveWorkspacePolicyState } from './policy.js';
1612
import { writeToStderr } from '@google/gemini-cli-core';
1713

1814
// Mock debugLogger to avoid noise in test output
@@ -59,6 +55,7 @@ describe('resolveWorkspacePolicyState', () => {
5955
cwd: workspaceDir,
6056
trustedFolder: false,
6157
interactive: true,
58+
autoAcceptWorkspacePolicies: true,
6259
});
6360

6461
expect(result).toEqual({
@@ -77,6 +74,7 @@ describe('resolveWorkspacePolicyState', () => {
7774
cwd: workspaceDir,
7875
trustedFolder: true,
7976
interactive: true,
77+
autoAcceptWorkspacePolicies: true,
8078
});
8179
expect(firstResult.workspacePoliciesDir).toBe(policiesDir);
8280
expect(firstResult.policyUpdateConfirmationRequest).toBeUndefined();
@@ -88,6 +86,7 @@ describe('resolveWorkspacePolicyState', () => {
8886
cwd: workspaceDir,
8987
trustedFolder: true,
9088
interactive: true,
89+
autoAcceptWorkspacePolicies: true,
9190
});
9291

9392
expect(result.workspacePoliciesDir).toBe(policiesDir);
@@ -99,46 +98,42 @@ describe('resolveWorkspacePolicyState', () => {
9998
cwd: workspaceDir,
10099
trustedFolder: true,
101100
interactive: true,
101+
autoAcceptWorkspacePolicies: true,
102102
});
103103

104104
expect(result.workspacePoliciesDir).toBeUndefined();
105105
expect(result.policyUpdateConfirmationRequest).toBeUndefined();
106106
});
107107

108-
it('should return confirmation request if changed in interactive mode when AUTO_ACCEPT is false', async () => {
109-
const originalValue = AUTO_ACCEPT_WORKSPACE_POLICIES;
110-
setAutoAcceptWorkspacePolicies(false);
111-
112-
try {
113-
fs.mkdirSync(policiesDir, { recursive: true });
114-
fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []');
108+
it('should return confirmation request if changed in interactive mode when autoAcceptWorkspacePolicies is false', async () => {
109+
fs.mkdirSync(policiesDir, { recursive: true });
110+
fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []');
115111

116-
const result = await resolveWorkspacePolicyState({
117-
cwd: workspaceDir,
118-
trustedFolder: true,
119-
interactive: true,
120-
});
112+
const result = await resolveWorkspacePolicyState({
113+
cwd: workspaceDir,
114+
trustedFolder: true,
115+
interactive: true,
116+
autoAcceptWorkspacePolicies: false,
117+
});
121118

122-
expect(result.workspacePoliciesDir).toBeUndefined();
123-
expect(result.policyUpdateConfirmationRequest).toEqual({
124-
scope: 'workspace',
125-
identifier: workspaceDir,
126-
policyDir: policiesDir,
127-
newHash: expect.any(String),
128-
});
129-
} finally {
130-
setAutoAcceptWorkspacePolicies(originalValue);
131-
}
119+
expect(result.workspacePoliciesDir).toBeUndefined();
120+
expect(result.policyUpdateConfirmationRequest).toEqual({
121+
scope: 'workspace',
122+
identifier: workspaceDir,
123+
policyDir: policiesDir,
124+
newHash: expect.any(String),
125+
});
132126
});
133127

134-
it('should warn and auto-accept if changed in non-interactive mode when AUTO_ACCEPT is true', async () => {
128+
it('should warn and auto-accept if changed in non-interactive mode when autoAcceptWorkspacePolicies is true', async () => {
135129
fs.mkdirSync(policiesDir, { recursive: true });
136130
fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []');
137131

138132
const result = await resolveWorkspacePolicyState({
139133
cwd: workspaceDir,
140134
trustedFolder: true,
141135
interactive: false,
136+
autoAcceptWorkspacePolicies: true,
142137
});
143138

144139
expect(result.workspacePoliciesDir).toBe(policiesDir);
@@ -148,28 +143,22 @@ describe('resolveWorkspacePolicyState', () => {
148143
);
149144
});
150145

151-
it('should warn and auto-accept if changed in non-interactive mode when AUTO_ACCEPT is false', async () => {
152-
const originalValue = AUTO_ACCEPT_WORKSPACE_POLICIES;
153-
setAutoAcceptWorkspacePolicies(false);
154-
155-
try {
156-
fs.mkdirSync(policiesDir, { recursive: true });
157-
fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []');
146+
it('should warn and auto-accept if changed in non-interactive mode when autoAcceptWorkspacePolicies is false', async () => {
147+
fs.mkdirSync(policiesDir, { recursive: true });
148+
fs.writeFileSync(path.join(policiesDir, 'policy.toml'), 'rules = []');
158149

159-
const result = await resolveWorkspacePolicyState({
160-
cwd: workspaceDir,
161-
trustedFolder: true,
162-
interactive: false,
163-
});
150+
const result = await resolveWorkspacePolicyState({
151+
cwd: workspaceDir,
152+
trustedFolder: true,
153+
interactive: false,
154+
autoAcceptWorkspacePolicies: false,
155+
});
164156

165-
expect(result.workspacePoliciesDir).toBe(policiesDir);
166-
expect(result.policyUpdateConfirmationRequest).toBeUndefined();
167-
expect(writeToStderr).toHaveBeenCalledWith(
168-
expect.stringContaining('Automatically accepting and loading'),
169-
);
170-
} finally {
171-
setAutoAcceptWorkspacePolicies(originalValue);
172-
}
157+
expect(result.workspacePoliciesDir).toBe(policiesDir);
158+
expect(result.policyUpdateConfirmationRequest).toBeUndefined();
159+
expect(writeToStderr).toHaveBeenCalledWith(
160+
expect.stringContaining('Automatically accepting and loading'),
161+
);
173162
});
174163

175164
it('should not return workspace policies if cwd is the home directory', async () => {
@@ -182,6 +171,7 @@ describe('resolveWorkspacePolicyState', () => {
182171
cwd: tempDir,
183172
trustedFolder: true,
184173
interactive: true,
174+
autoAcceptWorkspacePolicies: true,
185175
});
186176

187177
expect(result.workspacePoliciesDir).toBeUndefined();
@@ -206,6 +196,7 @@ describe('resolveWorkspacePolicyState', () => {
206196
cwd: symlinkDir,
207197
trustedFolder: true,
208198
interactive: true,
199+
autoAcceptWorkspacePolicies: true,
209200
});
210201

211202
expect(result.workspacePoliciesDir).toBeUndefined();

packages/cli/src/config/policy.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,6 @@ import {
2121
} from '@google/gemini-cli-core';
2222
import { type Settings } from './settings.js';
2323

24-
/**
25-
* Temporary flag to automatically accept workspace policies to reduce friction.
26-
* Exported as 'let' to allow monkey patching in tests via the setter.
27-
*/
28-
export let AUTO_ACCEPT_WORKSPACE_POLICIES = true;
29-
30-
/**
31-
* Sets the AUTO_ACCEPT_WORKSPACE_POLICIES flag.
32-
* Used primarily for testing purposes.
33-
*/
34-
export function setAutoAcceptWorkspacePolicies(value: boolean) {
35-
AUTO_ACCEPT_WORKSPACE_POLICIES = value;
36-
}
37-
3824
export async function createPolicyEngineConfig(
3925
settings: Settings,
4026
approvalMode: ApprovalMode,
@@ -73,8 +59,14 @@ export async function resolveWorkspacePolicyState(options: {
7359
cwd: string;
7460
trustedFolder: boolean;
7561
interactive: boolean;
62+
autoAcceptWorkspacePolicies?: boolean;
7663
}): Promise<WorkspacePolicyState> {
77-
const { cwd, trustedFolder, interactive } = options;
64+
const {
65+
cwd,
66+
trustedFolder,
67+
interactive,
68+
autoAcceptWorkspacePolicies = false,
69+
} = options;
7870

7971
let workspacePoliciesDir: string | undefined;
8072
let policyUpdateConfirmationRequest:
@@ -106,7 +98,7 @@ export async function resolveWorkspacePolicyState(options: {
10698
) {
10799
// No workspace policies found
108100
workspacePoliciesDir = undefined;
109-
} else if (interactive && !AUTO_ACCEPT_WORKSPACE_POLICIES) {
101+
} else if (interactive && !autoAcceptWorkspacePolicies) {
110102
// Policies changed or are new, and we are in interactive mode and auto-accept is disabled
111103
policyUpdateConfirmationRequest = {
112104
scope: 'workspace',

packages/cli/src/config/workspace-policy-cli.test.ts

Lines changed: 35 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import { loadCliConfig, type CliArgs } from './config.js';
1010
import { createTestMergedSettings } from './settings.js';
1111
import * as ServerConfig from '@google/gemini-cli-core';
1212
import { isWorkspaceTrusted } from './trustedFolders.js';
13-
import * as Policy from './policy.js';
1413

1514
// Mock dependencies
1615
vi.mock('./trustedFolders.js', () => ({
@@ -239,48 +238,40 @@ describe('Workspace-Level Policy CLI Integration', () => {
239238
);
240239
});
241240

242-
it('should set policyUpdateConfirmationRequest if integrity MISMATCH in interactive mode when AUTO_ACCEPT is false', async () => {
243-
// Monkey patch AUTO_ACCEPT_WORKSPACE_POLICIES using setter
244-
const originalValue = Policy.AUTO_ACCEPT_WORKSPACE_POLICIES;
245-
Policy.setAutoAcceptWorkspacePolicies(false);
246-
247-
try {
248-
vi.mocked(isWorkspaceTrusted).mockReturnValue({
249-
isTrusted: true,
250-
source: 'file',
251-
});
252-
mockCheckIntegrity.mockResolvedValue({
253-
status: 'mismatch',
254-
hash: 'new-hash',
255-
fileCount: 1,
256-
});
257-
vi.mocked(ServerConfig.isHeadlessMode).mockReturnValue(false); // Interactive
258-
259-
const settings = createTestMergedSettings();
260-
const argv = {
261-
query: 'test',
262-
promptInteractive: 'test',
263-
} as unknown as CliArgs;
264-
265-
const config = await loadCliConfig(settings, 'test-session', argv, {
266-
cwd: MOCK_CWD,
267-
});
268-
269-
expect(config.getPolicyUpdateConfirmationRequest()).toEqual({
270-
scope: 'workspace',
271-
identifier: MOCK_CWD,
272-
policyDir: expect.stringContaining(path.join('.gemini', 'policies')),
273-
newHash: 'new-hash',
274-
});
275-
expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith(
276-
expect.objectContaining({
277-
workspacePoliciesDir: undefined,
278-
}),
279-
expect.anything(),
280-
);
281-
} finally {
282-
// Restore for other tests
283-
Policy.setAutoAcceptWorkspacePolicies(originalValue);
284-
}
241+
it('should set policyUpdateConfirmationRequest if integrity MISMATCH in interactive mode when autoAcceptWorkspacePolicies is false', async () => {
242+
vi.mocked(isWorkspaceTrusted).mockReturnValue({
243+
isTrusted: true,
244+
source: 'file',
245+
});
246+
mockCheckIntegrity.mockResolvedValue({
247+
status: 'mismatch',
248+
hash: 'new-hash',
249+
fileCount: 1,
250+
});
251+
vi.mocked(ServerConfig.isHeadlessMode).mockReturnValue(false); // Interactive
252+
253+
const settings = createTestMergedSettings();
254+
const argv = {
255+
query: 'test',
256+
promptInteractive: 'test',
257+
} as unknown as CliArgs;
258+
259+
const config = await loadCliConfig(settings, 'test-session', argv, {
260+
cwd: MOCK_CWD,
261+
autoAcceptWorkspacePolicies: false,
262+
});
263+
264+
expect(config.getPolicyUpdateConfirmationRequest()).toEqual({
265+
scope: 'workspace',
266+
identifier: MOCK_CWD,
267+
policyDir: expect.stringContaining(path.join('.gemini', 'policies')),
268+
newHash: 'new-hash',
269+
});
270+
expect(ServerConfig.createPolicyEngineConfig).toHaveBeenCalledWith(
271+
expect.objectContaining({
272+
workspacePoliciesDir: undefined,
273+
}),
274+
expect.anything(),
275+
);
285276
});
286277
});

0 commit comments

Comments
 (0)