diff --git a/packages/agent-core-v2/src/agent/externalHooks/externalHooksService.ts b/packages/agent-core-v2/src/agent/externalHooks/externalHooksService.ts index 0ecf9049c..89bd23850 100644 --- a/packages/agent-core-v2/src/agent/externalHooks/externalHooksService.ts +++ b/packages/agent-core-v2/src/agent/externalHooks/externalHooksService.ts @@ -28,7 +28,6 @@ import { import { IAgentLoopService, type TurnAfterStepContext } from '#/agent/loop'; import { IAgentPermissionGate, - type PermissionApprovalResultContext, } from '#/agent/permissionGate'; import { IAgentPromptService, @@ -173,14 +172,7 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter permission.hooks.onDidRequestApproval.register('externalHooks', async (ctx, next) => { void this.engine()?.fireAndForgetTrigger('PermissionRequest', { matcherValue: ctx.toolName, - inputData: { - turnId: ctx.turnId, - toolCallId: ctx.toolCallId, - toolName: ctx.toolName, - action: ctx.action, - toolInput: ctx.toolInput, - display: ctx.display, - }, + inputData: { ...ctx }, }); await next(); }), @@ -189,7 +181,7 @@ export class AgentExternalHooksService extends Disposable implements IAgentExter permission.hooks.onDidResolveApproval.register('externalHooks', async (ctx, next) => { void this.engine()?.fireAndForgetTrigger('PermissionResult', { matcherValue: ctx.toolName, - inputData: permissionResultInputData(ctx), + inputData: { ...ctx }, }); await next(); }), @@ -469,31 +461,6 @@ function toolOutputText(output: ExecutableToolResult['output']): string { .join(''); } -function permissionResultInputData( - payload: PermissionApprovalResultContext, -): Record { - if (payload.decision === 'error') { - return { - turnId: payload.turnId, - toolCallId: payload.toolCallId, - toolName: payload.toolName, - action: payload.action, - decision: payload.decision, - error: payload.error, - }; - } - return { - turnId: payload.turnId, - toolCallId: payload.toolCallId, - toolName: payload.toolName, - action: payload.action, - decision: payload.decision, - scope: payload.scope, - feedback: payload.feedback, - selectedLabel: payload.selectedLabel, - }; -} - registerScopedService( LifecycleScope.Agent, IAgentExternalHooksService, diff --git a/packages/agent-core-v2/src/agent/permissionGate/permissionGate.ts b/packages/agent-core-v2/src/agent/permissionGate/permissionGate.ts index e82ca374d..1ea5af9c6 100644 --- a/packages/agent-core-v2/src/agent/permissionGate/permissionGate.ts +++ b/packages/agent-core-v2/src/agent/permissionGate/permissionGate.ts @@ -1,4 +1,6 @@ import type { + ApprovalRequest, + ApprovalResponse, PermissionData, } from '#/agent/permissionPolicy'; import { createDecorator } from "#/_base/di"; @@ -7,40 +9,26 @@ import type { ResolvedToolExecutionHookContext, } from '#/agent/tool'; import type { Hooks } from '#/hooks'; -import type { ToolInputDisplay } from '@moonshot-ai/protocol'; export interface PermissionGateOptions { readonly agentId?: string; } -export interface PermissionApprovalRequestContext { +export type PermissionApprovalRequestContext = ApprovalRequest & { + readonly sessionId?: string; + readonly agentId?: string; readonly turnId: number; - readonly toolCallId: string; - readonly toolName: string; - readonly action: string; readonly toolInput: unknown; - readonly display: ToolInputDisplay; -} +}; -export type PermissionApprovalResultContext = - | { - readonly turnId: number; - readonly toolCallId: string; - readonly toolName: string; - readonly action: string; - readonly decision: 'approved' | 'rejected' | 'cancelled'; - readonly scope?: 'session'; - readonly feedback?: string; - readonly selectedLabel?: string; - } - | { - readonly turnId: number; - readonly toolCallId: string; - readonly toolName: string; - readonly action: string; - readonly decision: 'error'; - readonly error: string; - }; +export type PermissionApprovalResultContext = PermissionApprovalRequestContext & + ( + | ApprovalResponse + | { + readonly decision: 'error'; + readonly error: string; + } + ); export interface IAgentPermissionGate { readonly _serviceBrand: undefined; diff --git a/packages/agent-core-v2/src/agent/permissionGate/permissionGateService.ts b/packages/agent-core-v2/src/agent/permissionGate/permissionGateService.ts index db71a8964..f6c0ad89c 100644 --- a/packages/agent-core-v2/src/agent/permissionGate/permissionGateService.ts +++ b/packages/agent-core-v2/src/agent/permissionGate/permissionGateService.ts @@ -1,6 +1,9 @@ -import type { - ApprovalResponse, - PermissionData, +import { + IAgentPermissionPolicyService, + type ApprovalResponse, + type PermissionData, + type PermissionPolicyResolution, + type PermissionPolicyResult, } from '#/agent/permissionPolicy'; import { Disposable, @@ -14,11 +17,6 @@ import type { import type { ToolInputDisplay } from '@moonshot-ai/protocol'; import { ISessionApprovalService } from "#/session/approval/approval"; import { IAgentPermissionModeService } from '#/agent/permissionMode'; -import { - IAgentPermissionPolicyService, - type PermissionPolicyResolution, - type PermissionPolicyResult, -} from '#/agent/permissionPolicy'; import { IAgentPermissionRulesService } from '#/agent/permissionRules'; import { ISessionContext } from '#/session/sessionContext'; import { ITelemetryService } from '#/app/telemetry'; @@ -129,6 +127,19 @@ export class AgentPermissionGate extends Disposable implements IAgentPermissionG summary: action, detail: context.args, } as ToolInputDisplay); + const approvalRequest = { + sessionId: this.session.sessionId, + agentId: this.options.agentId ?? 'main', + turnId: context.turnId, + toolCallId: context.toolCall.id, + toolName: name, + action, + display, + }; + const approvalContext = { + ...approvalRequest, + toolInput: context.args, + } satisfies PermissionApprovalRequestContext; const startedAt = Date.now(); let response: ApprovalResponse; @@ -136,25 +147,10 @@ export class AgentPermissionGate extends Disposable implements IAgentPermissionG if (approvalService === undefined) { response = { decision: 'approved' }; } else { - void this.hooks.onDidRequestApproval.run({ - turnId: context.turnId, - toolCallId: context.toolCall.id, - toolName: name, - action, - toolInput: context.args, - display, - }).catch(() => undefined); + void this.hooks.onDidRequestApproval.run(approvalContext).catch(() => undefined); try { response = await abortable( - approvalService.request({ - sessionId: this.session.sessionId, - agentId: this.options.agentId ?? 'main', - turnId: context.turnId, - toolCallId: context.toolCall.id, - toolName: name, - action, - display, - }), + approvalService.request(approvalRequest), context.signal, ); context.signal.throwIfAborted(); @@ -171,10 +167,7 @@ export class AgentPermissionGate extends Disposable implements IAgentPermissionG has_feedback: false, }); void this.hooks.onDidResolveApproval.run({ - turnId: context.turnId, - toolCallId: context.toolCall.id, - toolName: name, - action, + ...approvalContext, decision: 'error', error: error instanceof Error ? error.message : String(error), }).catch(() => undefined); @@ -192,14 +185,8 @@ export class AgentPermissionGate extends Disposable implements IAgentPermissionG : undefined; if (approvalService !== undefined) { void this.hooks.onDidResolveApproval.run({ - turnId: context.turnId, - toolCallId: context.toolCall.id, - toolName: name, - action, - decision: response.decision, - scope: response.scope, - feedback: response.feedback, - selectedLabel: response.selectedLabel, + ...approvalContext, + ...response, }).catch(() => undefined); } this.rulesService.recordApprovalResult({ diff --git a/packages/agent-core-v2/test/externalHooks/integration.test.ts b/packages/agent-core-v2/test/externalHooks/integration.test.ts index b6de18352..5fc0a29a8 100644 --- a/packages/agent-core-v2/test/externalHooks/integration.test.ts +++ b/packages/agent-core-v2/test/externalHooks/integration.test.ts @@ -35,7 +35,7 @@ import { stubBootstrap } from '../bootstrap/stubs'; import { stubLoopWithHooks, stubToolExecutor, stubTurnWithHooks } from '../turn/stubs'; function nodeCommand(source: string): string { - return `node -e ${JSON.stringify(source.replace(/\s*\n\s*/g, ' '))}`; + return `node -e ${JSON.stringify(source.replaceAll(/\s*\n\s*/g, ' '))}`; } function stdinScript(body: string): string { @@ -228,6 +228,104 @@ describe('HookEngine integration', () => { } }); + it('passes permission approval contexts through to PermissionRequest and PermissionResult hooks', async () => { + const disposables = new DisposableStore(); + let ix: TestInstantiationService | undefined; + try { + const permissionHooks = createHooks([ + 'onDidRequestApproval', + 'onDidResolveApproval', + ]) as IAgentPermissionGate['hooks']; + const fired: Array<{ + event: string; + matcherValue?: unknown; + inputData?: unknown; + }> = []; + const hookEngine = { + trigger: async () => [], + triggerBlock: async () => undefined, + fireAndForgetTrigger: async ( + event: string, + args: { matcherValue?: unknown; inputData?: unknown }, + ) => { + fired.push({ + event, + matcherValue: args.matcherValue, + inputData: args.inputData, + }); + }, + }; + + ix = createServices(disposables, { + strict: true, + additionalServices: (reg) => { + reg.defineInstance(IBootstrapService, stubBootstrap()); + reg.definePartialInstance(IConfigService, {}); + reg.definePartialInstance(IPluginService, {}); + reg.defineInstance(IAgentContextMemoryService, stubContextMemory()); + reg.defineInstance(IAgentRecordService, stubRecord()); + reg.defineInstance(IAgentLoopService, stubLoopWithHooks()); + reg.definePartialInstance(IAgentPromptService, { + hooks: createHooks(['onWillSubmitPrompt']), + }); + reg.defineInstance(IAgentTurnService, stubTurnWithHooks()); + reg.defineInstance(IAgentToolExecutorService, stubToolExecutor()); + reg.definePartialInstance(IAgentPermissionGate, { + hooks: permissionHooks, + }); + reg.definePartialInstance(IAgentFullCompactionService, { + hooks: createHooks(['onWillCompact', 'onDidCompact']), + }); + reg.definePartialInstance(IAgentTaskService, { + hooks: createHooks(['onDidNotify']), + }); + }, + }); + ix.set( + IAgentExternalHooksService, + new SyncDescriptor(AgentExternalHooksService, [{ hookEngine }]), + ); + ix.get(IAgentExternalHooksService); + + const requestContext = { + sessionId: 'session-1', + agentId: 'main', + turnId: 7, + toolCallId: 'call-bash', + toolName: 'Bash', + action: 'Run command', + toolInput: { command: 'pwd' }, + display: { kind: 'command' as const, command: 'pwd' }, + }; + await permissionHooks.onDidRequestApproval.run(requestContext); + await permissionHooks.onDidResolveApproval.run({ + ...requestContext, + decision: 'approved', + selectedLabel: 'Approve once', + }); + + expect(fired).toEqual([ + { + event: 'PermissionRequest', + matcherValue: 'Bash', + inputData: requestContext, + }, + { + event: 'PermissionResult', + matcherValue: 'Bash', + inputData: { + ...requestContext, + decision: 'approved', + selectedLabel: 'Approve once', + }, + }, + ]); + } finally { + ix?.dispose(); + disposables.dispose(); + } + }); + it('fires a Notification hook only when its matcher equals the notification matcher value', async () => { const engine = new HookEngine([ { diff --git a/packages/agent-core-v2/test/permission/permissionGate.test.ts b/packages/agent-core-v2/test/permission/permissionGate.test.ts index 2c32e2bc9..53a4606ed 100644 --- a/packages/agent-core-v2/test/permission/permissionGate.test.ts +++ b/packages/agent-core-v2/test/permission/permissionGate.test.ts @@ -6,9 +6,11 @@ import { createServices } from '#/_base/di/test'; import type { TestInstantiationService } from '#/_base/di/test'; import { createHooks } from '#/hooks'; import type { Hooks } from '#/hooks'; -import type { ApprovalResponse } from '#/session/approval/approval'; -import type { ApprovalRequest } from '#/session/approval/approval'; -import { ISessionApprovalService } from '#/session/approval/approval'; +import { + ISessionApprovalService, + type ApprovalRequest, + type ApprovalResponse, +} from '#/session/approval/approval'; import { IHostEnvironment } from '#/os/interface/hostEnvironment'; import type { ResolvedToolExecutionHookContext } from '#/agent/tool'; import { IAgentPermissionGate, AgentPermissionGate } from '#/agent/permissionGate'; @@ -17,12 +19,11 @@ import { IAgentPermissionModeService } from '#/agent/permissionMode'; import type { PermissionMode, PermissionPolicyEvaluation } from '#/agent/permissionPolicy'; import { IAgentPermissionPolicyService } from '#/agent/permissionPolicy'; import { AgentPermissionPolicyService } from '#/agent/permissionPolicy/permissionPolicyService'; -import type { - IAgentPermissionRulesService as PermissionRulesServiceContract, - PermissionApprovalResultRecord, +import { + IAgentPermissionRulesService, + type PermissionApprovalResultRecord, + type PermissionRule, } from '#/agent/permissionRules'; -import type { PermissionRule } from '#/agent/permissionRules'; -import { IAgentPermissionRulesService } from '#/agent/permissionRules'; import { IAgentPlanService } from '#/agent/plan'; import { IAgentProfileService, type ProfileData } from '#/agent/profile'; import { ISessionContext, makeSessionContext } from '#/session/sessionContext'; @@ -400,6 +401,8 @@ describe('AgentPermissionGate', () => { expect(request).toHaveBeenCalledTimes(1); expect(permissionRequest).toHaveBeenCalledWith({ + sessionId: 'test-session', + agentId: 'main', turnId: 1, toolCallId: 'call-Bash', toolName: 'Bash', @@ -412,10 +415,18 @@ describe('AgentPermissionGate', () => { }, }); expect(permissionResult).toHaveBeenCalledWith({ + sessionId: 'test-session', + agentId: 'main', turnId: 1, toolCallId: 'call-Bash', toolName: 'Bash', action: 'Approve Bash', + toolInput: { command: 'printf first' }, + display: { + kind: 'generic', + summary: 'Approve Bash', + detail: { command: 'printf first' }, + }, decision: 'approved', selectedLabel: 'Approve once', }); @@ -529,7 +540,7 @@ interface MutableRulesOptions { function mutablePermissionRulesService( options: MutableRulesOptions, -): PermissionRulesServiceContract { +): IAgentPermissionRulesService { return { _serviceBrand: undefined, get rules() {