Addressing feedback and fixing policy issues

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

@@ -409,11 +409,13 @@ describe('extension tests', () => {
 toolName = "deny_tool"
 decision = "deny"
 priority = 500
+modes = ["plan", "default", "autoEdit"]
 
 [[rule]]
 toolName = "ask_tool"
 decision = "ask_user"
 priority = 100
+modes = ["plan", "default", "autoEdit"]
 `;
       fs.writeFileSync(
         path.join(policiesDir, 'policies.toml'),
@@ -454,6 +456,7 @@ priority = 100
 toolName = "allow_tool"
 decision = "allow"
 priority = 100
+modes = ["plan", "default", "autoEdit"]
 
 [[rule]]
 toolName = "yolo_tool"

packages/cli/src/config/policy-engine.integration.test.ts

@@ -7,6 +7,7 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
 import {
   ApprovalMode,
+  MODES_BY_PERMISSIVENESS,
   PolicyDecision,
   PolicyEngine,
 } from '@google/gemini-cli-core';
@@ -385,6 +386,7 @@ describe('Policy Engine Integration Tests', () => {
         toolAnnotations: { readOnlyHint: true },
         decision: PolicyDecision.ALLOW,
         priority: 10,
+        modes: MODES_BY_PERMISSIVENESS,
       });
 
       const engine = new PolicyEngine(config);

packages/cli/src/test-utils/AppRig.tsx

@@ -23,6 +23,7 @@ import {
   ExtensionLoader,
   AuthType,
   ApprovalMode,
+  MODES_BY_PERMISSIVENESS,
   createPolicyEngineConfig,
   PolicyDecision,
   ToolConfirmationOutcome,
@@ -452,6 +453,7 @@ export class AppRig {
       toolName,
       decision,
       priority,
+      modes: MODES_BY_PERMISSIVENESS,
       source: 'AppRig Override',
     });
   }

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

@@ -326,7 +326,8 @@ describe('createPolicyEngineConfig', () => {
       (r) =>
         r.toolName === 'replace' &&
         r.decision === PolicyDecision.ALLOW &&
-        r.modes?.includes(ApprovalMode.AUTO_EDIT),
+        r.modes?.includes(ApprovalMode.AUTO_EDIT) &&
+        !r.argsPattern,
     );
     expect(rule).toBeDefined();
     expect(rule?.priority).toBeCloseTo(1.015, 5);
@@ -421,7 +422,7 @@ describe('createPolicyEngineConfig', () => {
   it('should handle complex priority scenarios correctly', async () => {
     mockPolicyFile(
       nodePath.join(MOCK_DEFAULT_DIR, 'default.toml'),
-      '[[rule]]\ntoolName = "glob"\ndecision = "allow"\npriority = 50\n',
+      '[[rule]]\ntoolName = "glob"\ndecision = "allow"\npriority = 50\nmodes = ["default", "plan", "autoEdit", "yolo"]\n',
     );
 
     const settings: PolicySettings = {
@@ -543,6 +544,7 @@ describe('createPolicyEngineConfig', () => {
   argsPattern = "\\"command\\":\\"git (status|diff|log)\\""
   decision = "allow"
   priority = 150
+  modes = ["default", "plan", "autoEdit", "yolo"]
   `,
     );
 
@@ -573,10 +575,12 @@ describe('createPolicyEngineConfig', () => {
 toolName = "write_file"
 decision = "allow"
 priority = 10
+modes = ["default", "plan", "autoEdit", "yolo"]
 
 [[safety_checker]]
 toolName = "write_file"
 priority = 10
+modes = ["default", "plan", "autoEdit", "yolo"]
 [safety_checker.checker]
 type = "in-process"
 name = "allowed-path"
@@ -610,10 +614,12 @@ required_context = ["environment"]
 toolName = "write_file"
 decision = "allow"
 priority = 10
+modes = ["default", "plan", "autoEdit", "yolo"]
 
 [[safety_checker]]
 toolName = "write_file"
 priority = 10
+modes = ["default", "plan", "autoEdit", "yolo"]
 [safety_checker.checker]
 type = "in-process"
 name = "invalid-name"
@@ -639,6 +645,7 @@ name = "invalid-name"
   mcpName = "my-server"
   decision = "allow"
   priority = 150
+  modes = ["default", "plan", "autoEdit", "yolo"]
   `,
     );
 

packages/core/src/policy/config.ts

@@ -584,6 +584,7 @@ function createTomlRule(toolName: string, message: UpdatePolicy): TomlRule {
     decision: 'allow',
     priority: getAlwaysAllowPriorityFraction(),
     toolName,
+    modes: message.modes ?? MODES_BY_PERMISSIVENESS,
   };
 
   if (message.mcpName) {
@@ -600,10 +601,6 @@ function createTomlRule(toolName: string, message: UpdatePolicy): TomlRule {
     rule.allowRedirection = message.allowRedirection;
   }
 
-  if (message.modes) {
-    rule.modes = message.modes;
-  }
-
   return rule;
 }
 

packages/core/src/policy/policies/write.toml

@@ -38,7 +38,7 @@ interactive = true
 toolName = "replace"
 decision = "allow"
 priority = 15
-modes = ["plan", "default", "autoEdit", "yolo"]
+modes = ["autoEdit", "yolo"]
 
 [rule.safety_checker]
 type = "in-process"
@@ -63,7 +63,7 @@ interactive = true
 toolName = "write_file"
 decision = "ask_user"
 priority = 10
-modes = ["plan", "default", "autoEdit", "yolo"]
+modes = ["plan", "default"]
 interactive = true
 
 [[rule]]
@@ -77,7 +77,7 @@ interactive = true
 toolName = "write_file"
 decision = "allow"
 priority = 15
-modes = ["plan", "default", "autoEdit", "yolo"]
+modes = ["autoEdit", "yolo"]
 
 [rule.safety_checker]
 type = "in-process"
@@ -88,13 +88,13 @@ required_context = ["environment"]
 toolName = "web_fetch"
 decision = "allow"
 priority = 15
-modes = ["plan", "default", "autoEdit", "yolo"]
+modes = ["autoEdit", "yolo"]
 
 [[rule]]
 toolName = "web_fetch"
 decision = "ask_user"
 priority = 10
-modes = ["plan", "default", "autoEdit", "yolo"]
+modes = ["plan", "default"]
 interactive = true
 
 # Headless Denial Rule (Priority 10)
@@ -110,5 +110,5 @@ toolName = [
 ]
 decision = "deny"
 priority = 10
-modes = ["plan", "default", "autoEdit", "yolo"]
+modes = ["plan", "default"]
 interactive = false

packages/core/src/policy/policy-engine.ts

@@ -89,8 +89,10 @@ function ruleMatches(
   toolAnnotations?: Record<string, unknown>,
   subagent?: string,
 ): boolean {
-  // Check if rule applies to current approval mode
-  if (!rule.modes.includes(currentApprovalMode)) {
+  // Check if rule applies to current approval mode.
+  // Legacy rules or those injected manually in tests might not have the modes field.
+  // If so, we default to matching all modes for backward compatibility.
+  if (rule.modes && !rule.modes.includes(currentApprovalMode)) {
     return false;
   }
   // Check subagent if specified (only for PolicyRule, SafetyCheckerRule doesn't have it)
@@ -568,7 +570,7 @@ export class PolicyEngine {
 
       if (match) {
         debugLogger.debug(
-          `[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, argsPattern=${rule.argsPattern?.source || 'none'}`,
+          `[PolicyEngine.check] MATCHED rule: toolName=${rule.toolName}, decision=${rule.decision}, priority=${rule.priority}, source=${rule.source}, argsPattern=${rule.argsPattern?.source || 'none'}`,
         );
 
         let ruleDecision = rule.decision;

packages/core/src/policy/workspace-policy.test.ts

@@ -91,6 +91,7 @@ describe('Workspace-Level Policies', () => {
 toolName = "test_tool"
 decision = "allow"
 priority = 10
+modes = ["plan", "default", "autoEdit", "yolo"]
 `; // Tier 1 -> 1.010
       }
       if (path.includes('user.toml')) {
@@ -98,6 +99,7 @@ priority = 10
 toolName = "test_tool"
 decision = "deny"
 priority = 10
+modes = ["plan", "default", "autoEdit", "yolo"]
 `; // Tier 4 -> 4.010
       }
       if (path.includes('workspace.toml')) {
@@ -105,6 +107,7 @@ priority = 10
 toolName = "test_tool"
 decision = "allow"
 priority = 10
+modes = ["plan", "default", "autoEdit", "yolo"]
 `; // Tier 3 -> 3.010
       }
       if (path.includes('admin.toml')) {
@@ -112,6 +115,7 @@ priority = 10
 toolName = "test_tool"
 decision = "deny"
 priority = 10
+modes = ["plan", "default", "autoEdit", "yolo"]
 `; // Tier 5 -> 5.010
       }
       return '';
@@ -194,7 +198,8 @@ priority = 10
       async () => `[[rule]]
 toolName="t"
 decision="allow"
-priority=10`,
+priority=10
+modes = ["plan", "default", "autoEdit", "yolo"]`,
     );
 
     vi.doMock('node:fs/promises', () => ({
@@ -259,7 +264,8 @@ priority=10`,
       async () => `[[rule]]
 toolName="p_tool"
 decision="allow"
-priority=500`,
+priority=500
+modes = ["plan", "default", "autoEdit", "yolo"]`,
     );
 
     vi.doMock('node:fs/promises', () => ({