feat(hooks): implement STOP_EXECUTION and enhance hook decision handling (#15685)

packages/core/src/core/coreToolHookTriggers.test.ts

@@ -6,6 +6,7 @@
 
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { executeToolWithHooks } from './coreToolHookTriggers.js';
+import { ToolErrorType } from '../tools/tool-error.js';
 import {
   BaseToolInvocation,
   type ToolResult,
@@ -17,8 +18,8 @@ import {
   type HookExecutionResponse,
 } from '../confirmation-bus/types.js';
 
-class MockInvocation extends BaseToolInvocation<{ key: string }, ToolResult> {
-  constructor(params: { key: string }) {
+class MockInvocation extends BaseToolInvocation<{ key?: string }, ToolResult> {
+  constructor(params: { key?: string }) {
     super(params);
   }
   getDescription() {
@@ -26,8 +27,10 @@ class MockInvocation extends BaseToolInvocation<{ key: string }, ToolResult> {
   }
   async execute() {
     return {
-      llmContent: `key: ${this.params.key}`,
-      returnDisplay: `key: ${this.params.key}`,
+      llmContent: this.params.key ? `key: ${this.params.key}` : 'success',
+      returnDisplay: this.params.key
+        ? `key: ${this.params.key}`
+        : 'success display',
     };
   }
 }
@@ -39,12 +42,145 @@ describe('executeToolWithHooks', () => {
   beforeEach(() => {
     messageBus = {
       request: vi.fn(),
+      publish: vi.fn(),
+      subscribe: vi.fn(),
+      unsubscribe: vi.fn(),
     } as unknown as MessageBus;
     mockTool = {
       build: vi.fn().mockImplementation((params) => new MockInvocation(params)),
     } as unknown as AnyDeclarativeTool;
   });
 
+  it('should prioritize continue: false over decision: block in BeforeTool', async () => {
+    const invocation = new MockInvocation({});
+    const abortSignal = new AbortController().signal;
+
+    vi.mocked(messageBus.request).mockResolvedValue({
+      type: MessageBusType.HOOK_EXECUTION_RESPONSE,
+      correlationId: 'test-id',
+      success: true,
+      output: {
+        continue: false,
+        stopReason: 'Stop immediately',
+        decision: 'block',
+        reason: 'Should be ignored because continue is false',
+      },
+    } as HookExecutionResponse);
+
+    const result = await executeToolWithHooks(
+      invocation,
+      'test_tool',
+      abortSignal,
+      messageBus,
+      true,
+      mockTool,
+    );
+
+    expect(result.error?.type).toBe(ToolErrorType.STOP_EXECUTION);
+    expect(result.error?.message).toBe('Stop immediately');
+  });
+
+  it('should block execution in BeforeTool if decision is block', async () => {
+    const invocation = new MockInvocation({});
+    const abortSignal = new AbortController().signal;
+
+    vi.mocked(messageBus.request).mockResolvedValue({
+      type: MessageBusType.HOOK_EXECUTION_RESPONSE,
+      correlationId: 'test-id',
+      success: true,
+      output: {
+        decision: 'block',
+        reason: 'Execution blocked',
+      },
+    } as HookExecutionResponse);
+
+    const result = await executeToolWithHooks(
+      invocation,
+      'test_tool',
+      abortSignal,
+      messageBus,
+      true,
+      mockTool,
+    );
+
+    expect(result.error?.type).toBe(ToolErrorType.EXECUTION_FAILED);
+    expect(result.error?.message).toBe('Execution blocked');
+  });
+
+  it('should handle continue: false in AfterTool', async () => {
+    const invocation = new MockInvocation({});
+    const abortSignal = new AbortController().signal;
+    const spy = vi.spyOn(invocation, 'execute');
+
+    // BeforeTool allow
+    vi.mocked(messageBus.request)
+      .mockResolvedValueOnce({
+        type: MessageBusType.HOOK_EXECUTION_RESPONSE,
+        correlationId: 'test-id',
+        success: true,
+        output: { decision: 'allow' },
+      } as HookExecutionResponse)
+      // AfterTool stop
+      .mockResolvedValueOnce({
+        type: MessageBusType.HOOK_EXECUTION_RESPONSE,
+        correlationId: 'test-id',
+        success: true,
+        output: {
+          continue: false,
+          stopReason: 'Stop after execution',
+        },
+      } as HookExecutionResponse);
+
+    const result = await executeToolWithHooks(
+      invocation,
+      'test_tool',
+      abortSignal,
+      messageBus,
+      true,
+      mockTool,
+    );
+
+    expect(result.error?.type).toBe(ToolErrorType.STOP_EXECUTION);
+    expect(result.error?.message).toBe('Stop after execution');
+    expect(spy).toHaveBeenCalled();
+  });
+
+  it('should block result in AfterTool if decision is deny', async () => {
+    const invocation = new MockInvocation({});
+    const abortSignal = new AbortController().signal;
+
+    // BeforeTool allow
+    vi.mocked(messageBus.request)
+      .mockResolvedValueOnce({
+        type: MessageBusType.HOOK_EXECUTION_RESPONSE,
+        correlationId: 'test-id',
+        success: true,
+        output: { decision: 'allow' },
+      } as HookExecutionResponse)
+      // AfterTool deny
+      .mockResolvedValueOnce({
+        type: MessageBusType.HOOK_EXECUTION_RESPONSE,
+        correlationId: 'test-id',
+        success: true,
+        output: {
+          decision: 'deny',
+          reason: 'Result denied',
+        },
+      } as HookExecutionResponse);
+
+    const result = await executeToolWithHooks(
+      invocation,
+      'test_tool',
+      abortSignal,
+      messageBus,
+      true,
+      mockTool,
+    );
+
+    expect(result.error?.type).toBe(ToolErrorType.EXECUTION_FAILED);
+    expect(result.error?.message).toBe('Result denied');
+  });
+
   it('should apply modified tool input from BeforeTool hook', async () => {
     const params = { key: 'original' };
     const invocation = new MockInvocation(params);

packages/core/src/core/coreToolHookTriggers.ts

@@ -273,6 +273,19 @@ export async function executeToolWithHooks(
       toolInput,
     );
 
+    // Check if hook requested to stop entire agent execution
+    if (beforeOutput?.shouldStopExecution()) {
+      const reason = beforeOutput.getEffectiveReason();
+      return {
+        llmContent: `Agent execution stopped by hook: ${reason}`,
+        returnDisplay: `Agent execution stopped by hook: ${reason}`,
+        error: {
+          type: ToolErrorType.STOP_EXECUTION,
+          message: reason,
+        },
+      };
+    }
+
     // Check if hook blocked the tool execution
     const blockingError = beforeOutput?.getBlockingError();
     if (blockingError?.blocked) {
@@ -286,19 +299,6 @@ export async function executeToolWithHooks(
       };
     }
 
-    // Check if hook requested to stop entire agent execution
-    if (beforeOutput?.shouldStopExecution()) {
-      const reason = beforeOutput.getEffectiveReason();
-      return {
-        llmContent: `Agent execution stopped by hook: ${reason}`,
-        returnDisplay: `Agent execution stopped by hook: ${reason}`,
-        error: {
-          type: ToolErrorType.EXECUTION_FAILED,
-          message: `Agent execution stopped: ${reason}`,
-        },
-      };
-    }
-
     // Check if hook requested to update tool input
     if (beforeOutput instanceof BeforeToolHookOutput) {
       const modifiedInput = beforeOutput.getModifiedToolInput();
@@ -386,9 +386,22 @@ export async function executeToolWithHooks(
       return {
         llmContent: `Agent execution stopped by hook: ${reason}`,
         returnDisplay: `Agent execution stopped by hook: ${reason}`,
+        error: {
+          type: ToolErrorType.STOP_EXECUTION,
+          message: reason,
+        },
+      };
+    }
+
+    // Check if hook blocked the tool result
+    const blockingError = afterOutput?.getBlockingError();
+    if (blockingError?.blocked) {
+      return {
+        llmContent: `Tool result blocked: ${blockingError.reason}`,
+        returnDisplay: `Tool result blocked: ${blockingError.reason}`,
         error: {
           type: ToolErrorType.EXECUTION_FAILED,
-          message: `Agent execution stopped: ${reason}`,
+          message: blockingError.reason,
         },
       };
     }

packages/core/src/hooks/types.ts

@@ -180,7 +180,7 @@ export class DefaultHookOutput implements HookOutput {
    * Get the effective reason for blocking or stopping
    */
   getEffectiveReason(): string {
-    return this.reason || this.stopReason || 'No reason provided';
+    return this.stopReason || this.reason || 'No reason provided';
   }
 
   /**

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

@@ -71,6 +71,9 @@ export enum ToolErrorType {
 
   // WebSearch-specific Errors
   WEB_SEARCH_FAILED = 'web_search_failed',
+
+  // Hook-specific Errors
+  STOP_EXECUTION = 'stop_execution',
 }
 
 /**