fix(core): prevent duplicate tool approval entries in auto-saved.toml (#19487)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

packages/core/src/agents/remote-invocation.ts

@@ -117,8 +117,8 @@ export class RemoteAgentInvocation extends BaseToolInvocation<
       type: 'info',
       title: `Call Remote Agent: ${this.definition.displayName ?? this.definition.name}`,
       prompt: `Calling remote agent: "${this.params.query}"`,
-      onConfirm: async (outcome: ToolConfirmationOutcome) => {
-        await this.publishPolicyUpdate(outcome);
+      onConfirm: async (_outcome: ToolConfirmationOutcome) => {
+        // Policy updates are now handled centrally by the scheduler
       },
     };
   }

packages/core/src/tools/confirmation-policy.test.ts

@@ -136,17 +136,17 @@ describe('Tool Confirmation Policy Updates', () => {
     it.each([
       {
         outcome: ToolConfirmationOutcome.ProceedAlways,
-        shouldPublish: false,
+        _shouldPublish: false,
         expectedApprovalMode: ApprovalMode.AUTO_EDIT,
       },
       {
         outcome: ToolConfirmationOutcome.ProceedAlwaysAndSave,
-        shouldPublish: true,
-        persist: true,
+        _shouldPublish: true,
+        _persist: true,
       },
     ])(
       'should handle $outcome correctly',
-      async ({ outcome, shouldPublish, persist, expectedApprovalMode }) => {
+      async ({ outcome, expectedApprovalMode }) => {
         const tool = create(mockConfig, mockMessageBus);
 
         // For file-based tools, ensure the file exists if needed
@@ -172,26 +172,19 @@ describe('Tool Confirmation Policy Updates', () => {
         if (confirmation) {
           await confirmation.onConfirm(outcome);
 
-          if (shouldPublish) {
-            expect(mockMessageBus.publish).toHaveBeenCalledWith(
-              expect.objectContaining({
-                type: MessageBusType.UPDATE_POLICY,
-                persist,
-              }),
-            );
-          } else {
-            // Should not publish UPDATE_POLICY message for ProceedAlways
-            const publishCalls = (mockMessageBus.publish as any).mock.calls;
-            const hasUpdatePolicy = publishCalls.some(
-              (call: any) => call[0].type === MessageBusType.UPDATE_POLICY,
-            );
-            expect(hasUpdatePolicy).toBe(false);
-          }
+          // Policy updates are no longer published by onConfirm; they are
+          // handled centrally by the schedulers.
+          const publishCalls = (mockMessageBus.publish as any).mock.calls;
+          const hasUpdatePolicy = publishCalls.some(
+            (call: any) => call[0].type === MessageBusType.UPDATE_POLICY,
+          );
+          expect(hasUpdatePolicy).toBe(false);
 
           if (expectedApprovalMode !== undefined) {
-            expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
-              expectedApprovalMode,
-            );
+            // expectedApprovalMode in this test (AUTO_EDIT) is now handled
+            // by updatePolicy in the scheduler, so it should not be called
+            // here either.
+            expect(mockConfig.setApprovalMode).not.toHaveBeenCalled();
           }
         }
       },

packages/core/src/tools/edit.ts

@@ -14,7 +14,7 @@ import {
   BaseToolInvocation,
   Kind,
   type ToolCallConfirmationDetails,
-  ToolConfirmationOutcome,
+  type ToolConfirmationOutcome,
   type ToolEditConfirmationDetails,
   type ToolInvocation,
   type ToolLocation,
@@ -725,14 +725,9 @@ class EditToolInvocation
       fileDiff,
       originalContent: editData.currentContent,
       newContent: editData.newContent,
-      onConfirm: async (outcome: ToolConfirmationOutcome) => {
-        if (outcome === ToolConfirmationOutcome.ProceedAlways) {
-          // No need to publish a policy update as the default policy for
-          // AUTO_EDIT already reflects always approving edit.
-          this.config.setApprovalMode(ApprovalMode.AUTO_EDIT);
-        } else {
-          await this.publishPolicyUpdate(outcome);
-        }
+      onConfirm: async (_outcome: ToolConfirmationOutcome) => {
+        // Mode transitions (e.g. AUTO_EDIT) and policy updates are now
+        // handled centrally by the scheduler.
 
         if (ideConfirmation) {
           const result = await ideConfirmation;

packages/core/src/tools/enter-plan-mode.ts

@@ -105,7 +105,7 @@ export class EnterPlanModeInvocation extends BaseToolInvocation<
         'This will restrict the agent to read-only tools to allow for safe planning.',
       onConfirm: async (outcome: ToolConfirmationOutcome) => {
         this.confirmationOutcome = outcome;
-        await this.publishPolicyUpdate(outcome);
+        // Policy updates are now handled centrally by the scheduler
       },
     };
   }

packages/core/src/tools/mcp-tool.ts

@@ -130,7 +130,7 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation<
           DiscoveredMCPToolInvocation.allowlist.add(toolAllowListKey);
         } else if (outcome === ToolConfirmationOutcome.ProceedAlwaysAndSave) {
           DiscoveredMCPToolInvocation.allowlist.add(toolAllowListKey);
-          await this.publishPolicyUpdate(outcome);
+          // Persistent policy updates are now handled centrally by the scheduler
         }
       },
     };

packages/core/src/tools/memoryTool.ts

@@ -204,7 +204,7 @@ class MemoryToolInvocation extends BaseToolInvocation<
         if (outcome === ToolConfirmationOutcome.ProceedAlways) {
           MemoryToolInvocation.allowlist.add(allowlistKey);
         }
-        await this.publishPolicyUpdate(outcome);
+        // Policy updates are now handled centrally by the scheduler
       },
     };
     return confirmationDetails;

packages/core/src/tools/shell.ts

@@ -140,8 +140,8 @@ export class ShellToolInvocation extends BaseToolInvocation<
       command: this.params.command,
       rootCommand: rootCommandDisplay,
       rootCommands,
-      onConfirm: async (outcome: ToolConfirmationOutcome) => {
-        await this.publishPolicyUpdate(outcome);
+      onConfirm: async (_outcome: ToolConfirmationOutcome) => {
+        // Policy updates are now handled centrally by the scheduler
       },
     };
     return confirmationDetails;

packages/core/src/tools/tools.ts

@@ -173,8 +173,8 @@ export abstract class BaseToolInvocation<
       type: 'info',
       title: `Confirm: ${this._toolDisplayName || this._toolName}`,
       prompt: this.getDescription(),
-      onConfirm: async (outcome: ToolConfirmationOutcome) => {
-        await this.publishPolicyUpdate(outcome);
+      onConfirm: async (_outcome: ToolConfirmationOutcome) => {
+        // Policy updates are now handled centrally by the scheduler
       },
     };
     return confirmationDetails;

packages/core/src/tools/web-fetch.test.ts

@@ -390,7 +390,7 @@ describe('WebFetchTool', () => {
       expect(confirmationDetails).toBe(false);
     });
 
-    it('should call setApprovalMode when onConfirm is called with ProceedAlways', async () => {
+    it('should NOT call setApprovalMode when onConfirm is called with ProceedAlways (now handled by scheduler)', async () => {
       const tool = new WebFetchTool(mockConfig, bus);
       const params = { prompt: 'fetch https://example.com' };
       const invocation = tool.build(params);
@@ -408,9 +408,8 @@ describe('WebFetchTool', () => {
         );
       }
 
-      expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(
-        ApprovalMode.AUTO_EDIT,
-      );
+      // Schedulers are now responsible for mode transitions via updatePolicy
+      expect(mockConfig.setApprovalMode).not.toHaveBeenCalled();
     });
   });
 

packages/core/src/tools/web-fetch.ts

@@ -13,7 +13,7 @@ import {
   BaseDeclarativeTool,
   BaseToolInvocation,
   Kind,
-  ToolConfirmationOutcome,
+  type ToolConfirmationOutcome,
 } from './tools.js';
 import type { MessageBus } from '../confirmation-bus/message-bus.js';
 import { ToolErrorType } from './tool-error.js';
@@ -246,14 +246,9 @@ ${textContent}
       title: `Confirm Web Fetch`,
       prompt: this.params.prompt,
       urls,
-      onConfirm: async (outcome: ToolConfirmationOutcome) => {
-        if (outcome === ToolConfirmationOutcome.ProceedAlways) {
-          // No need to publish a policy update as the default policy for
-          // AUTO_EDIT already reflects always approving web-fetch.
-          this.config.setApprovalMode(ApprovalMode.AUTO_EDIT);
-        } else {
-          await this.publishPolicyUpdate(outcome);
-        }
+      onConfirm: async (_outcome: ToolConfirmationOutcome) => {
+        // Mode transitions (e.g. AUTO_EDIT) and policy updates are now
+        // handled centrally by the scheduler.
       },
     };
     return confirmationDetails;

packages/core/src/tools/write-file.ts

@@ -25,7 +25,7 @@ import {
   BaseDeclarativeTool,
   BaseToolInvocation,
   Kind,
-  ToolConfirmationOutcome,
+  type ToolConfirmationOutcome,
 } from './tools.js';
 import { ToolErrorType } from './tool-error.js';
 import { makeRelative, shortenPath } from '../utils/paths.js';
@@ -228,14 +228,9 @@ class WriteFileToolInvocation extends BaseToolInvocation<
       fileDiff,
       originalContent,
       newContent: correctedContent,
-      onConfirm: async (outcome: ToolConfirmationOutcome) => {
-        if (outcome === ToolConfirmationOutcome.ProceedAlways) {
-          // No need to publish a policy update as the default policy for
-          // AUTO_EDIT already reflects always approving write-file.
-          this.config.setApprovalMode(ApprovalMode.AUTO_EDIT);
-        } else {
-          await this.publishPolicyUpdate(outcome);
-        }
+      onConfirm: async (_outcome: ToolConfirmationOutcome) => {
+        // Mode transitions (e.g. AUTO_EDIT) and policy updates are now
+        // handled centrally by the scheduler.
 
         if (ideConfirmation) {
           const result = await ideConfirmation;