Reduce boilerplate.

This commit is contained in:
Christian Gunderman
2026-03-06 13:48:40 -08:00
parent 9a71caa781
commit 489e7bfbef
21 changed files with 230 additions and 151 deletions
@@ -6,7 +6,6 @@
import {
BaseToolInvocation,
type ForcedToolDecision,
type ToolConfirmationOutcome,
type ToolResult,
type ToolCallConfirmationDetails,
@@ -135,7 +134,6 @@ export class RemoteAgentInvocation extends BaseToolInvocation<
protected override async getConfirmationDetails(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
// For now, always require confirmation for remote agents until we have a policy system for them.
return {
@@ -112,7 +112,6 @@ describe('SubAgentInvocation', () => {
expect(result).toBe(false);
expect(mockInnerInvocation.shouldConfirmExecute).toHaveBeenCalledWith(
abortSignal,
undefined,
);
expect(MockSubagentToolWrapper).toHaveBeenCalledWith(
testDefinition,
@@ -157,7 +156,6 @@ describe('SubAgentInvocation', () => {
expect(result).toBe(confirmationDetails);
expect(mockInnerInvocation.shouldConfirmExecute).toHaveBeenCalledWith(
abortSignal,
undefined,
);
expect(MockSubagentToolWrapper).toHaveBeenCalledWith(
testRemoteDefinition,
+1 -3
View File
@@ -9,7 +9,6 @@ import {
Kind,
type ToolInvocation,
type ToolResult,
type ForcedToolDecision,
BaseToolInvocation,
type ToolCallConfirmationDetails,
isTool,
@@ -146,13 +145,12 @@ class SubAgentInvocation extends BaseToolInvocation<AgentInputs, ToolResult> {
override async shouldConfirmExecute(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
const invocation = this.buildSubInvocation(
this.definition,
this.withUserHints(this.params),
);
return invocation.shouldConfirmExecute(abortSignal, forcedDecision);
return invocation.shouldConfirmExecute(abortSignal);
}
async execute(
@@ -52,15 +52,18 @@ export class MessageBus extends EventEmitter {
}
if (message.type === MessageBusType.TOOL_CONFIRMATION_REQUEST) {
const { decision } = await this.policyEngine.check(
const { decision: policyDecision } = await this.policyEngine.check(
message.toolCall,
message.serverName,
message.toolAnnotations,
message.subagent,
);
const decision = message.forcedDecision ?? policyDecision;
switch (decision) {
case PolicyDecision.ALLOW:
case 'allow':
// Directly emit the response instead of recursive publish
this.emitMessage({
type: MessageBusType.TOOL_CONFIRMATION_RESPONSE,
@@ -69,6 +72,7 @@ export class MessageBus extends EventEmitter {
});
break;
case PolicyDecision.DENY:
case 'deny':
// Emit both rejection and response messages
this.emitMessage({
type: MessageBusType.TOOL_POLICY_REJECTION,
@@ -81,6 +85,7 @@ export class MessageBus extends EventEmitter {
});
break;
case PolicyDecision.ASK_USER:
case 'ask_user':
// Pass through to UI for user confirmation if any listeners exist.
// If no listeners are registered (e.g., headless/ACP flows),
// immediately request user confirmation to avoid long timeouts.
@@ -46,6 +46,10 @@ export interface ToolConfirmationRequest {
* Optional rich details for the confirmation UI (diffs, counts, etc.)
*/
details?: SerializableConfirmationDetails;
/**
* Optional decision to force for this tool call, bypassing the policy engine.
*/
forcedDecision?: 'allow' | 'deny' | 'ask_user';
}
export interface ToolConfirmationResponse {
-2
View File
@@ -16,7 +16,6 @@ import {
import type {
ToolCallConfirmationDetails,
ToolResult,
ForcedToolDecision,
} from '../tools/tools.js';
import { getResponseText } from '../utils/partUtils.js';
import { reportError } from '../utils/errorReporting.js';
@@ -47,7 +46,6 @@ export interface ServerTool {
shouldConfirmExecute(
params: Record<string, unknown>,
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false>;
}
+1
View File
@@ -616,6 +616,7 @@ export class Scheduler {
if (beforeOutput.isAskDecision()) {
hookDecision = 'ask';
hookSystemMessage = beforeOutput.systemMessage;
toolCall.request.forcedAsk = true;
}
if (beforeOutput instanceof BeforeToolHookOutput) {
+1 -10
View File
@@ -12,7 +12,6 @@ import {
BaseDeclarativeTool,
BaseToolInvocation,
Kind,
type ForcedToolDecision,
type ToolCallConfirmationDetails,
type ToolInvocation,
type ToolLiveOutput,
@@ -31,7 +30,6 @@ interface MockToolOptions {
shouldConfirmExecute?: (
params: { [key: string]: unknown },
signal: AbortSignal,
forcedDecision?: ForcedToolDecision,
) => Promise<ToolCallConfirmationDetails | false>;
execute?: (
params: { [key: string]: unknown },
@@ -70,13 +68,8 @@ class MockToolInvocation extends BaseToolInvocation<
override shouldConfirmExecute(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
return this.tool.shouldConfirmExecute(
this.params,
abortSignal,
forcedDecision,
);
return this.tool.shouldConfirmExecute(this.params, abortSignal);
}
getDescription(): string {
@@ -94,7 +87,6 @@ export class MockTool extends BaseDeclarativeTool<
readonly shouldConfirmExecute: (
params: { [key: string]: unknown },
signal: AbortSignal,
forcedDecision?: ForcedToolDecision,
) => Promise<ToolCallConfirmationDetails | false>;
readonly execute: (
@@ -177,7 +169,6 @@ export class MockModifiableToolInvocation extends BaseToolInvocation<
override async shouldConfirmExecute(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
if (this.tool.shouldConfirm) {
return {
-2
View File
@@ -7,7 +7,6 @@
import {
BaseDeclarativeTool,
BaseToolInvocation,
type ForcedToolDecision,
type ToolResult,
Kind,
type ToolAskUserConfirmationDetails,
@@ -127,7 +126,6 @@ export class AskUserInvocation extends BaseToolInvocation<
override async shouldConfirmExecute(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolAskUserConfirmationDetails | false> {
const normalizedQuestions = this.params.questions.map((q) => ({
...q,
@@ -191,5 +191,39 @@ describe('Tool Confirmation Policy Updates', () => {
}
},
);
it('should skip confirmation in AUTO_EDIT mode', async () => {
vi.spyOn(mockConfig, 'getApprovalMode').mockReturnValue(
ApprovalMode.AUTO_EDIT,
);
const tool = create(mockConfig, mockMessageBus);
const invocation = tool.build(params as any);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
);
expect(confirmation).toBe(false);
});
it('should NOT skip confirmation in AUTO_EDIT mode if forcedDecision is ask_user', async () => {
vi.spyOn(mockConfig, 'getApprovalMode').mockReturnValue(
ApprovalMode.AUTO_EDIT,
);
const tool = create(mockConfig, mockMessageBus);
const invocation = tool.build(params as any);
// Mock getMessageBusDecision to return ask_user
vi.spyOn(invocation as any, 'getMessageBusDecision').mockResolvedValue(
'ask_user',
);
const confirmation = await invocation.shouldConfirmExecute(
new AbortController().signal,
'ask_user',
);
expect(confirmation).not.toBe(false);
});
});
});
+12 -13
View File
@@ -13,7 +13,6 @@ import {
BaseDeclarativeTool,
BaseToolInvocation,
Kind,
type ForcedToolDecision,
type ToolCallConfirmationDetails,
type ToolConfirmationOutcome,
type ToolEditConfirmationDetails,
@@ -27,7 +26,7 @@ import { ToolErrorType } from './tool-error.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { isNodeError } from '../utils/errors.js';
import type { Config } from '../config/config.js';
import { ApprovalMode } from '../policy/types.js';
import { type ApprovalMode } from '../policy/types.js';
import { CoreToolCallStatus } from '../scheduler/types.js';
import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js';
@@ -700,26 +699,26 @@ class EditToolInvocation
);
}
protected override get respectsAutoEdit(): boolean {
return true;
}
protected override getApprovalMode(): ApprovalMode {
return this.config.getApprovalMode();
}
/**
* Handles the confirmation prompt for the Edit tool in the CLI.
* It needs to calculate the diff to show the user.
*/
protected override async getConfirmationDetails(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
_abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails | false> {
if (
this.config.getApprovalMode() === ApprovalMode.AUTO_EDIT &&
forcedDecision !== 'ask_user'
) {
return false;
}
let editData: CalculatedEdit;
try {
editData = await this.calculateEdit(this.params, abortSignal);
editData = await this.calculateEdit(this.params, _abortSignal);
} catch (error) {
if (abortSignal.aborted) {
if (_abortSignal.aborted) {
throw error;
}
const errorMsg = error instanceof Error ? error.message : String(error);
+1 -4
View File
@@ -7,7 +7,6 @@
import {
BaseDeclarativeTool,
BaseToolInvocation,
type ForcedToolDecision,
type ToolResult,
Kind,
type ToolInfoConfirmationDetails,
@@ -86,10 +85,8 @@ export class EnterPlanModeInvocation extends BaseToolInvocation<
override async shouldConfirmExecute(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ToolInfoConfirmationDetails | false> {
const decision =
forcedDecision ?? (await this.getMessageBusDecision(abortSignal));
const decision = await this.getMessageBusDecision(abortSignal);
if (decision === 'allow') {
return false;
}
+1 -4
View File
@@ -7,7 +7,6 @@
import {
BaseDeclarativeTool,
BaseToolInvocation,
type ForcedToolDecision,
type ToolResult,
Kind,
type ToolExitPlanModeConfirmationDetails,
@@ -119,7 +118,6 @@ export class ExitPlanModeInvocation extends BaseToolInvocation<
override async shouldConfirmExecute(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ToolExitPlanModeConfirmationDetails | false> {
const resolvedPlanPath = this.getResolvedPlanPath();
@@ -139,8 +137,7 @@ export class ExitPlanModeInvocation extends BaseToolInvocation<
return false;
}
const decision =
forcedDecision ?? (await this.getMessageBusDecision(abortSignal));
const decision = await this.getMessageBusDecision(abortSignal);
if (decision === 'deny') {
throw new Error(
`Tool execution for "${
@@ -10,7 +10,6 @@ import {
Kind,
type ToolInvocation,
type ToolResult,
type ForcedToolDecision,
type ToolCallConfirmationDetails,
} from './tools.js';
import { GET_INTERNAL_DOCS_TOOL_NAME } from './tool-names.js';
@@ -86,7 +85,6 @@ class GetInternalDocsInvocation extends BaseToolInvocation<
override async shouldConfirmExecute(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
return false;
}
-2
View File
@@ -11,7 +11,6 @@ import {
BaseToolInvocation,
Kind,
ToolConfirmationOutcome,
type ForcedToolDecision,
type ToolCallConfirmationDetails,
type ToolInvocation,
type ToolMcpConfirmationDetails,
@@ -193,7 +192,6 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation<
protected override async getConfirmationDetails(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
const serverAllowListKey = this.serverName;
const toolAllowListKey = `${this.serverName}.${this.serverToolName}`;
-2
View File
@@ -9,7 +9,6 @@ import {
BaseToolInvocation,
Kind,
ToolConfirmationOutcome,
type ForcedToolDecision,
type ToolEditConfirmationDetails,
type ToolResult,
} from './tools.js';
@@ -164,7 +163,6 @@ class MemoryToolInvocation extends BaseToolInvocation<
protected override async getConfirmationDetails(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolEditConfirmationDetails | false> {
const memoryFilePath = getGlobalMemoryFilePath();
const allowlistKey = memoryFilePath;
-2
View File
@@ -16,7 +16,6 @@ import {
BaseToolInvocation,
ToolConfirmationOutcome,
Kind,
type ForcedToolDecision,
type ToolInvocation,
type ToolResult,
type ToolCallConfirmationDetails,
@@ -110,7 +109,6 @@ export class ShellToolInvocation extends BaseToolInvocation<
protected override async getConfirmationDetails(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
const command = stripShellWrapper(this.params.command);
+32 -6
View File
@@ -18,7 +18,7 @@ import {
type ToolConfirmationResponse,
type Question,
} from '../confirmation-bus/types.js';
import { type ApprovalMode } from '../policy/types.js';
import { ApprovalMode } from '../policy/types.js';
import type { SubagentProgress } from '../agents/types.js';
/**
@@ -58,10 +58,10 @@ export interface ToolInvocation<
* @param abortSignal An AbortSignal that can be used to cancel the confirmation request.
* @returns A ToolCallConfirmationDetails object if confirmation is required, or false if not.
*/
shouldConfirmExecute: (
shouldConfirmExecute(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
) => Promise<ToolCallConfirmationDetails | false>;
): Promise<ToolCallConfirmationDetails | false>;
/**
* Executes the tool with the validated parameters.
@@ -111,6 +111,14 @@ export abstract class BaseToolInvocation<
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
if (
this.respectsAutoEdit &&
this.getApprovalMode() === ApprovalMode.AUTO_EDIT &&
forcedDecision !== 'ask_user'
) {
return false;
}
const decision =
forcedDecision ?? (await this.getMessageBusDecision(abortSignal));
if (decision === 'allow') {
@@ -126,11 +134,28 @@ export abstract class BaseToolInvocation<
}
if (decision === 'ask_user') {
return this.getConfirmationDetails(abortSignal, forcedDecision);
return this.getConfirmationDetails(abortSignal);
}
// Default to confirmation details if decision is unknown (should not happen with exhaustive policy)
return this.getConfirmationDetails(abortSignal, forcedDecision);
return this.getConfirmationDetails(abortSignal);
}
/**
* Whether this tool respects the AUTO_EDIT approval mode.
* Subclasses should override this and return true if they want to skip
* confirmation when the session is in AUTO_EDIT mode.
*/
protected get respectsAutoEdit(): boolean {
return false;
}
/**
* Returns the current approval mode from the tool configuration.
* Subclasses should override this and return the actual approval mode.
*/
protected getApprovalMode(): ApprovalMode {
return ApprovalMode.DEFAULT;
}
/**
@@ -174,7 +199,6 @@ export abstract class BaseToolInvocation<
*/
protected async getConfirmationDetails(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
if (!this.messageBus) {
return false;
@@ -193,6 +217,7 @@ export abstract class BaseToolInvocation<
protected getMessageBusDecision(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ForcedToolDecision> {
if (!this.messageBus || !this._toolName) {
// If there's no message bus, we can't make a decision, so we allow.
@@ -211,6 +236,7 @@ export abstract class BaseToolInvocation<
},
serverName: this._serverName,
toolAnnotations: this._toolAnnotations,
forcedDecision,
};
return new Promise<ForcedToolDecision>((resolve) => {
+9 -9
View File
@@ -8,7 +8,6 @@ import {
BaseDeclarativeTool,
BaseToolInvocation,
Kind,
type ForcedToolDecision,
type ToolCallConfirmationDetails,
type ToolInvocation,
type ToolResult,
@@ -18,7 +17,7 @@ import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { ToolErrorType } from './tool-error.js';
import { getErrorMessage } from '../utils/errors.js';
import type { Config } from '../config/config.js';
import { ApprovalMode } from '../policy/types.js';
import { type ApprovalMode } from '../policy/types.js';
import { getResponseText } from '../utils/partUtils.js';
import { fetchWithTimeout, isPrivateIp } from '../utils/fetch.js';
import { truncateString } from '../utils/textUtils.js';
@@ -292,16 +291,17 @@ ${textContent}
return `Processing URLs and instructions from prompt: "${displayPrompt}"`;
}
protected override get respectsAutoEdit(): boolean {
return true;
}
protected override getApprovalMode(): ApprovalMode {
return this.config.getApprovalMode();
}
protected override async getConfirmationDetails(
_abortSignal: AbortSignal,
_forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
// Check for AUTO_EDIT approval mode. This tool has a specific behavior
// where ProceedAlways switches the entire session to AUTO_EDIT.
if (this.config.getApprovalMode() === ApprovalMode.AUTO_EDIT) {
return false;
}
let urls: string[] = [];
let prompt = this.params.prompt || '';
+12 -13
View File
@@ -11,13 +11,12 @@ import os from 'node:os';
import * as Diff from 'diff';
import { WRITE_FILE_TOOL_NAME, WRITE_FILE_DISPLAY_NAME } from './tool-names.js';
import type { Config } from '../config/config.js';
import { ApprovalMode } from '../policy/types.js';
import { type ApprovalMode } from '../policy/types.js';
import {
BaseDeclarativeTool,
BaseToolInvocation,
Kind,
type ForcedToolDecision,
type FileDiff,
type ToolCallConfirmationDetails,
type ToolEditConfirmationDetails,
@@ -173,22 +172,22 @@ class WriteFileToolInvocation extends BaseToolInvocation<
return `Writing to ${shortenPath(relativePath)}`;
}
protected override async getConfirmationDetails(
abortSignal: AbortSignal,
forcedDecision?: ForcedToolDecision,
): Promise<ToolCallConfirmationDetails | false> {
if (
this.config.getApprovalMode() === ApprovalMode.AUTO_EDIT &&
forcedDecision !== 'ask_user'
) {
return false;
}
protected override get respectsAutoEdit(): boolean {
return true;
}
protected override getApprovalMode(): ApprovalMode {
return this.config.getApprovalMode();
}
protected override async getConfirmationDetails(
_abortSignal: AbortSignal,
): Promise<ToolCallConfirmationDetails | false> {
const correctedContentResult = await getCorrectedFileContent(
this.config,
this.resolvedPath,
this.params.content,
abortSignal,
_abortSignal,
);
if (correctedContentResult.error) {