diff --git a/packages/core/src/agents/remote-invocation.ts b/packages/core/src/agents/remote-invocation.ts index 10301f30900..41564944ecd 100644 --- a/packages/core/src/agents/remote-invocation.ts +++ b/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 }, }; } diff --git a/packages/core/src/tools/confirmation-policy.test.ts b/packages/core/src/tools/confirmation-policy.test.ts index 30213ac4d98..72b6e11e210 100644 --- a/packages/core/src/tools/confirmation-policy.test.ts +++ b/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(); } } }, diff --git a/packages/core/src/tools/edit.ts b/packages/core/src/tools/edit.ts index 8a481616627..130a05a8fec 100644 --- a/packages/core/src/tools/edit.ts +++ b/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; diff --git a/packages/core/src/tools/enter-plan-mode.ts b/packages/core/src/tools/enter-plan-mode.ts index feccb81089c..9e1bed23a6d 100644 --- a/packages/core/src/tools/enter-plan-mode.ts +++ b/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 }, }; } diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index c4d7a320384..280af4589a4 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/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 } }, }; diff --git a/packages/core/src/tools/memoryTool.ts b/packages/core/src/tools/memoryTool.ts index 31cc35077d5..33cb9483e18 100644 --- a/packages/core/src/tools/memoryTool.ts +++ b/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; diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index ff20b8a7b2a..76db302f426 100644 --- a/packages/core/src/tools/shell.ts +++ b/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; diff --git a/packages/core/src/tools/tools.ts b/packages/core/src/tools/tools.ts index 3d90e80699f..4e9972e37c3 100644 --- a/packages/core/src/tools/tools.ts +++ b/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; diff --git a/packages/core/src/tools/web-fetch.test.ts b/packages/core/src/tools/web-fetch.test.ts index ac483fccd91..f0c6ff2c7ee 100644 --- a/packages/core/src/tools/web-fetch.test.ts +++ b/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(); }); }); diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index 41d4b7a09dc..214cf4916b5 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/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; diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index 9f4abfdf557..4d521db33c5 100644 --- a/packages/core/src/tools/write-file.ts +++ b/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;