Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2026-05-22 08:09:42 +01:00
Code Issues Packages Projects Releases Wiki Activity

Add stopReason and warningMessage support for hooks (#3548)

Browse Source 
* wip

* wip

* cleanups

* nit

* fix test

* update

* update

* updates

* updates

* fix

* Update src/extension/intents/node/toolCallingLoop.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This commit is contained in:
Paul
2026-02-07 08:56:27 -08:00
committed by GitHub
parent 2717881dfd
commit cc8aeb72ca
8 changed files with 1038 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/copilot/package.json
+1 -1
View File
@@ -90,7 +90,7 @@
"l10n": "./l10n",
"enabledApiProposals": [
"agentSessionsWorkspace",
"chatHooks@2",
"chatHooks@3",
"extensionsAny",
"newSymbolNamesProvider",
"interactive",
extensions/copilot/src/extension/intents/node/hookResultProcessor.ts
+146
View File
@@ -0,0 +1,146 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import * as l10n from '@vscode/l10n';
import type { ChatResponseStream } from 'vscode';
import { ILogService } from '../../../platform/log/common/logService';
import { ChatHookType } from '../../../vscodeTypes';
/**
* Error thrown when a hook requests the agent to abort processing.
* The message should be shown to the user.
*/
export class HookAbortError extends Error {
constructor(
public readonly hookType: string,
public readonly stopReason: string
) {
super(`Hook ${hookType} aborted: ${stopReason}`);
this.name = 'HookAbortError';
}
}
/**
* Type guard to check if an error is a HookAbortError.
*/
export function isHookAbortError(error: unknown): error is HookAbortError {
return error instanceof HookAbortError;
}
/**
* A hook result from the chat hook service.
*/
export interface HookResult {
stopReason?: string;
resultKind: 'success' | 'error' | 'warning';
warningMessage?: string;
output: unknown;
}
/**
* Options for processing hook results.
*/
export interface ProcessHookResultsOptions {
/** The type of hook being processed */
hookType: ChatHookType;
/** The hook results to process */
results: readonly HookResult[];
/** The output stream for displaying messages */
outputStream: ChatResponseStream | undefined;
/** The log service for logging */
logService: ILogService;
/** Callback for handling successful hook results. Called with the output for each success. */
onSuccess: (output: unknown) => void;
/**
* When true, errors and stopReason are completely ignored (no throw, no warning, no hookProgress).
* Use for hooks like SessionStart/SubagentStart where blocking errors should be silently ignored.
*/
ignoreErrors?: boolean;
/**
* Callback for handling error results. When provided, errors are passed to this callback
* instead of being shown to the user. Use for Stop/SubagentStop hooks where errors
* should be collected as blocking reasons.
*/
onError?: (errorMessage: string) => void;
}
/**
* Processes hook results, handling aborts, warnings, errors, and success cases.
* Warnings are aggregated and displayed together via hookProgress after processing all results.
*
* @param options The processing options
* @throws HookAbortError if any result contains a stopReason or an error result is encountered
*/
export function processHookResults(options: ProcessHookResultsOptions): void {
const { hookType, results, outputStream, logService, onSuccess, ignoreErrors, onError } = options;
const warnings: string[] = [];
for (const result of results) {
// Check for stopReason - abort immediately (unless ignoreErrors is set)
if (result.stopReason) {
if (ignoreErrors) {
logService.trace(`[ToolCallingLoop] ${hookType} hook stopReason ignored: ${result.stopReason}`);
continue;
}
logService.info(`[ToolCallingLoop] ${hookType} hook requested abort: ${result.stopReason}`);
outputStream?.hookProgress(hookType, result.stopReason);
throw new HookAbortError(hookType, result.stopReason);
}
// Collect warnings
if (result.resultKind === 'warning' && result.warningMessage) {
logService.trace(`[ToolCallingLoop] ${hookType} hook warning: ${result.warningMessage}`);
warnings.push(result.warningMessage);
}
// Handle success
if (result.resultKind === 'success') {
if (result.warningMessage) {
warnings.push(result.warningMessage);
}
onSuccess(result.output);
}
// Handle error - abort unless ignoreErrors is set or onError is provided
if (result.resultKind === 'error') {
const errorMessage = typeof result.output === 'string' && result.output ? result.output : '';
logService.error(`[ToolCallingLoop] ${hookType} hook error: ${errorMessage}`);
if (onError) {
// Pass error to callback (for Stop/SubagentStop to collect as blocking reason)
onError(errorMessage);
return;
} else if (ignoreErrors) {
// Completely ignore error - no throw, no hookProgress (silently continue)
continue;
} else {
outputStream?.hookProgress(hookType, formatHookErrorMessage(errorMessage));
throw new HookAbortError(hookType, errorMessage);
}
}
}
// Show aggregated warnings via hookProgress
if (warnings.length > 0 && outputStream) {
if (warnings.length === 1) {
outputStream.hookProgress(hookType, undefined, warnings[0]);
} else {
const formattedWarnings = warnings.map((w, i) => `${i + 1}. ${w}`).join('\n');
outputStream.hookProgress(hookType, undefined, formattedWarnings);
}
}
}
/**
* Formats a localized error message for a failed hook.
* @param errorMessage The error message from the hook
* @returns A localized error message string
*/
export function formatHookErrorMessage(errorMessage: string): string {
if (errorMessage) {
return l10n.t('A hook failed with a fatal error. Please check the Hooks output channel for more details. Error message: {0}', errorMessage);
}
return l10n.t('A hook failed with a fatal error. Please check the Hooks output channel for more details.');
}
extensions/copilot/src/extension/intents/node/toolCallingLoop.ts
+86 -50
View File
@@ -45,6 +45,7 @@ import { ToolFailureEncountered, ToolResultMetadata } from '../../prompts/node/p
import { ToolName } from '../../tools/common/toolNames';
import { ToolCallCancelledError } from '../../tools/common/toolsService';
import { ReadFileParams } from '../../tools/node/readFileTool';
import { isHookAbortError, processHookResults } from './hookResultProcessor';
export const enum ToolCallLimitBehavior {
Confirm,
@@ -95,6 +96,13 @@ export interface IToolCallingBuiltPromptEvent {
export type ToolCallingLoopFetchOptions = Required<Pick<IMakeChatRequestOptions, 'messages' | 'finishedCb' | 'requestOptions' | 'userInitiatedRequest'>> & Pick<IMakeChatRequestOptions, 'disableThinking'>;
interface StartHookResult {
/**
* Additional context to add to the agent's context, if any.
*/
readonly additionalContext?: string;
}
interface StopHookResult {
/**
* Whether the agent should continue (not stop).
@@ -243,39 +251,47 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
* @param token Cancellation token
* @returns Result indicating whether to continue and the reasons
*/
protected async executeStopHook(input: StopHookInput, outputStream: ChatResponseStream | undefined, sessionId: string, token: CancellationToken): Promise<StopHookResult> {
protected async executeStopHook(input: StopHookInput, sessionId: string, outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<StopHookResult> {
try {
const results = await this._chatHookService.executeHook('Stop', {
toolInvocationToken: this.options.request.toolInvocationToken,
input: input
}, sessionId, token);
// Collect all blocking reasons (deduplicated)
const blockingReasons = new Set<string>();
for (const result of results) {
if (result.success === true) {
// Output may be a parsed object or a JSON string
const output = result.output;
processHookResults({
hookType: 'Stop',
results,
outputStream,
logService: this._logService,
onSuccess: (output) => {
if (typeof output === 'object' && output !== null) {
const hookOutput = output as StopHookOutput;
this._logService.trace(`[DefaultToolCallingLoop] Checking hook output: decision=${hookOutput.decision}, reason=${hookOutput.reason}`);
this._logService.trace(`[ToolCallingLoop] Checking hook output: decision=${hookOutput.decision}, reason=${hookOutput.reason}`);
if (hookOutput.decision === 'block' && hookOutput.reason) {
this._logService.trace(`[DefaultToolCallingLoop] Stop hook blocked: ${hookOutput.reason}`);
this._logService.trace(`[ToolCallingLoop] Stop hook blocked: ${hookOutput.reason}`);
blockingReasons.add(hookOutput.reason);
}
}
} else if (result.success === false) {
const errorMessage = typeof result.output === 'string' ? result.output : 'Unknown error';
this._logService.error(`[DefaultToolCallingLoop] Stop hook error: ${errorMessage}`);
}
}
},
// Collect errors as blocking reasons (stderr from exit code != 0)
onError: (errorMessage) => {
if (errorMessage) {
this._logService.trace(`[ToolCallingLoop] Stop hook error collected as blocking reason: ${errorMessage}`);
blockingReasons.add(errorMessage);
}
},
});
if (blockingReasons.size > 0) {
return { shouldContinue: true, reasons: [...blockingReasons] };
}
return { shouldContinue: false };
} catch (error) {
this._logService.error('[DefaultToolCallingLoop] Error executing Stop hook', error);
if (isHookAbortError(error)) {
throw error;
}
this._logService.error('[ToolCallingLoop] Error executing Stop hook', error);
return { shouldContinue: false };
}
}
@@ -301,21 +317,24 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
/**
* Called when a session starts to allow hooks to provide additional context.
* @param input The session start hook input containing source
* @param outputStream The output stream for displaying messages
* @param token Cancellation token
* @returns Result containing additional context from hooks
*/
protected async executeSessionStartHook(input: SessionStartHookInput, sessionId: string, token: CancellationToken): Promise<SubagentStartHookResult> {
protected async executeSessionStartHook(input: SessionStartHookInput, sessionId: string, outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<StartHookResult> {
try {
const results = await this._chatHookService.executeHook('SessionStart', {
toolInvocationToken: this.options.request.toolInvocationToken,
input: input
}, sessionId, token);
// Collect additionalContext from all successful hook results
const additionalContexts: string[] = [];
for (const result of results) {
if (result.success === true) {
const output = result.output;
processHookResults({
hookType: 'SessionStart',
results,
outputStream,
logService: this._logService,
onSuccess: (output) => {
if (typeof output === 'object' && output !== null) {
const hookOutput = output as SessionStartHookOutput;
if (hookOutput.additionalContext) {
@@ -323,16 +342,18 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
this._logService.trace(`[ToolCallingLoop] SessionStart hook provided context: ${hookOutput.additionalContext.substring(0, 100)}...`);
}
}
} else if (result.success === false) {
const errorMessage = typeof result.output === 'string' ? result.output : 'Unknown error';
this._logService.error(`[ToolCallingLoop] SessionStart hook error: ${errorMessage}`);
}
}
},
// SessionStart blocking errors and stopReason are silently ignored
ignoreErrors: true,
});
return {
additionalContext: additionalContexts.length > 0 ? additionalContexts.join('\n') : undefined
};
} catch (error) {
if (isHookAbortError(error)) {
throw error;
}
this._logService.error('[ToolCallingLoop] Error executing SessionStart hook', error);
return {};
}
@@ -341,21 +362,24 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
/**
* Called when a subagent starts to allow hooks to provide additional context.
* @param input The subagent start hook input containing agent_id and agent_type
* @param outputStream The output stream for displaying messages
* @param token Cancellation token
* @returns Result containing additional context from hooks
*/
protected async executeSubagentStartHook(input: SubagentStartHookInput, sessionId: string, token: CancellationToken): Promise<SubagentStartHookResult> {
protected async executeSubagentStartHook(input: SubagentStartHookInput, sessionId: string, outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<SubagentStartHookResult> {
try {
const results = await this._chatHookService.executeHook('SubagentStart', {
toolInvocationToken: this.options.request.toolInvocationToken,
input: input
}, sessionId, token);
// Collect additionalContext from all successful hook results
const additionalContexts: string[] = [];
for (const result of results) {
if (result.success === true) {
const output = result.output;
processHookResults({
hookType: 'SubagentStart',
results,
outputStream,
logService: this._logService,
onSuccess: (output) => {
if (typeof output === 'object' && output !== null) {
const hookOutput = output as SubagentStartHookOutput;
if (hookOutput.additionalContext) {
@@ -363,16 +387,18 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
this._logService.trace(`[ToolCallingLoop] SubagentStart hook provided context: ${hookOutput.additionalContext.substring(0, 100)}...`);
}
}
} else if (result.success === false) {
const errorMessage = typeof result.output === 'string' ? result.output : 'Unknown error';
this._logService.error(`[ToolCallingLoop] SubagentStart hook error: ${errorMessage}`);
}
}
},
// SubagentStart blocking errors and stopReason are silently ignored
ignoreErrors: true,
});
return {
additionalContext: additionalContexts.length > 0 ? additionalContexts.join('\n') : undefined
};
} catch (error) {
if (isHookAbortError(error)) {
throw error;
}
this._logService.error('[ToolCallingLoop] Error executing SubagentStart hook', error);
return {};
}
@@ -385,18 +411,20 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
* @param token Cancellation token
* @returns Result indicating whether to continue and the reasons
*/
protected async executeSubagentStopHook(input: SubagentStopHookInput, outputStream: ChatResponseStream | undefined, sessionId: string, token: CancellationToken): Promise<SubagentStopHookResult> {
protected async executeSubagentStopHook(input: SubagentStopHookInput, sessionId: string, outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<SubagentStopHookResult> {
try {
const results = await this._chatHookService.executeHook('SubagentStop', {
toolInvocationToken: this.options.request.toolInvocationToken,
input: input
}, sessionId, token);
// Collect all blocking reasons (deduplicated)
const blockingReasons = new Set<string>();
for (const result of results) {
if (result.success === true) {
const output = result.output;
processHookResults({
hookType: 'SubagentStop',
results,
outputStream,
logService: this._logService,
onSuccess: (output) => {
if (typeof output === 'object' && output !== null) {
const hookOutput = output as SubagentStopHookOutput;
this._logService.trace(`[ToolCallingLoop] Checking SubagentStop hook output: decision=${hookOutput.decision}, reason=${hookOutput.reason}`);
@@ -405,17 +433,24 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
blockingReasons.add(hookOutput.reason);
}
}
} else if (result.success === false) {
const errorMessage = typeof result.output === 'string' ? result.output : 'Unknown error';
this._logService.error(`[ToolCallingLoop] SubagentStop hook error: ${errorMessage}`);
}
}
},
// Collect errors as blocking reasons (stderr from exit code != 0)
onError: (errorMessage) => {
if (errorMessage) {
this._logService.trace(`[ToolCallingLoop] SubagentStop hook error collected as blocking reason: ${errorMessage}`);
blockingReasons.add(errorMessage);
}
},
});
if (blockingReasons.size > 0) {
return { shouldContinue: true, reasons: [...blockingReasons] };
}
return { shouldContinue: false };
} catch (error) {
if (isHookAbortError(error)) {
throw error;
}
this._logService.error('[ToolCallingLoop] Error executing SubagentStop hook', error);
return { shouldContinue: false };
}
@@ -452,8 +487,9 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
*
* - For subagents: Always executes SubagentStart hook
* - For regular sessions: Only executes SessionStart hook on the first turn
* @throws HookAbortError if a hook requests the session/subagent to abort
*/
public async runStartHooks(token: CancellationToken): Promise<void> {
public async runStartHooks(outputStream: ChatResponseStream | undefined, token: CancellationToken): Promise<void> {
const sessionId = this.options.conversation.sessionId;
const hasHooks = this.options.request.hasHooksEnabled;
@@ -462,7 +498,7 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
const startHookResult = await this.executeSubagentStartHook({
agent_id: this.options.request.subAgentInvocationId,
agent_type: this.options.request.subAgentName ?? 'default',
}, sessionId, token);
}, sessionId, outputStream, token);
if (startHookResult.additionalContext) {
this.additionalHookContext = startHookResult.additionalContext;
this._logService.info(`[ToolCallingLoop] SubagentStart hook provided context for subagent ${this.options.request.subAgentInvocationId}`);
@@ -497,7 +533,7 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
if (isFirstTurn) {
const startHookResult = await this.executeSessionStartHook({
source: 'new',
}, sessionId, token);
}, sessionId, outputStream, token);
if (startHookResult.additionalContext) {
this.additionalHookContext = startHookResult.additionalContext;
this._logService.info('[ToolCallingLoop] SessionStart hook provided context for session');
@@ -524,7 +560,7 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
const startHookResult = await this.executeSubagentStartHook({
agent_id: this.options.request.subAgentInvocationId,
agent_type: this.options.request.subAgentName ?? 'default',
}, sessionId, token);
}, sessionId, outputStream, token);
if (startHookResult.additionalContext) {
this.additionalHookContext = startHookResult.additionalContext;
this._logService.info(`[ToolCallingLoop] SubagentStart hook provided context for subagent ${this.options.request.subAgentInvocationId}`);
@@ -563,7 +599,7 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
agent_id: this.options.request.subAgentInvocationId,
agent_type: this.options.request.subAgentName ?? 'default',
stop_hook_active: stopHookActive,
}, outputStream, sessionId, token);
}, sessionId, outputStream, token);
const joinedReasons = stopHookResult.reasons?.join('; ');
this._logService.info(`[ToolCallingLoop] Subagent stop hook result: shouldContinue=${stopHookResult.shouldContinue}, reasons=${joinedReasons}`);
if (stopHookResult.shouldContinue && stopHookResult.reasons?.length) {
@@ -578,7 +614,7 @@ export abstract class ToolCallingLoop<TOptions extends IToolCallingLoopOptions =
continue;
}
} else {
const stopHookResult = await this.executeStopHook({ stop_hook_active: stopHookActive }, outputStream, sessionId, token);
const stopHookResult = await this.executeStopHook({ stop_hook_active: stopHookActive }, sessionId, outputStream, token);
const joinedReasons = stopHookResult.reasons?.join('; ');
this._logService.info(`[ToolCallingLoop] Stop hook result: shouldContinue=${stopHookResult.shouldContinue}, reasons=${joinedReasons}`);
if (stopHookResult.shouldContinue && stopHookResult.reasons?.length) {
extensions/copilot/src/extension/intents/test/node/hookResultProcessor.spec.ts
+716
View File
@@ -0,0 +1,716 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { ChatResponseStream } from 'vscode';
import { TestLogService } from '../../../../platform/testing/common/testLogService';
import { ChatHookType } from '../../../../vscodeTypes';
import { formatHookErrorMessage, HookAbortError, HookResult, isHookAbortError, processHookResults, ProcessHookResultsOptions } from '../../node/hookResultProcessor';
/**
* Mock implementation of ChatResponseStream that tracks hookProgress calls.
*/
class MockChatResponseStream {
readonly hookProgressCalls: Array<{ hookType: ChatHookType; stopReason?: string; systemMessage?: string }> = [];
hookProgress(hookType: ChatHookType, stopReason?: string, systemMessage?: string): void {
this.hookProgressCalls.push({ hookType, stopReason, systemMessage });
}
}
describe('hookResultProcessor', () => {
let logService: TestLogService;
let mockStream: MockChatResponseStream;
beforeEach(() => {
logService = new TestLogService();
mockStream = new MockChatResponseStream();
});
describe('HookAbortError', () => {
it('should create error with hookType and stopReason', () => {
const error = new HookAbortError('UserPromptSubmit', 'Build failed');
expect(error.hookType).toBe('UserPromptSubmit');
expect(error.stopReason).toBe('Build failed');
expect(error.message).toBe('Hook UserPromptSubmit aborted: Build failed');
expect(error.name).toBe('HookAbortError');
});
it('should be identifiable via isHookAbortError', () => {
const hookError = new HookAbortError('Stop', 'reason');
expect(isHookAbortError(hookError)).toBe(true);
expect(isHookAbortError(new Error('regular error'))).toBe(false);
expect(isHookAbortError(null)).toBe(false);
expect(isHookAbortError(undefined)).toBe(false);
});
});
describe('stopReason handling for all hook types', () => {
const hookTypes: ChatHookType[] = [
'UserPromptSubmit',
'SessionStart',
'Stop',
'SubagentStart',
'SubagentStop',
];
hookTypes.forEach((hookType) => {
describe(`${hookType} hook`, () => {
it('should throw HookAbortError when stopReason is present', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: {},
stopReason: 'Build failed, fix errors before continuing',
},
];
const onSuccess = vi.fn();
const options: ProcessHookResultsOptions = {
hookType,
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
};
expect(() => processHookResults(options)).toThrow(HookAbortError);
expect(() => processHookResults(options)).toThrow(
`Hook ${hookType} aborted: Build failed, fix errors before continuing`
);
// Verify hookProgress is called with the stopReason
expect(mockStream.hookProgressCalls.length).toBeGreaterThan(0);
expect(mockStream.hookProgressCalls[0].hookType).toBe(hookType);
expect(mockStream.hookProgressCalls[0].stopReason).toBe('Build failed, fix errors before continuing');
});
it('should not call onSuccess when stopReason is present', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { someData: 'value' },
stopReason: 'Processing blocked',
},
];
const onSuccess = vi.fn();
const options: ProcessHookResultsOptions = {
hookType,
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
};
try {
processHookResults(options);
} catch {
// Expected to throw
}
expect(onSuccess).not.toHaveBeenCalled();
});
it('should throw HookAbortError immediately and stop processing remaining results', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { index: 0 },
stopReason: 'First hook aborted',
},
{
resultKind: 'success',
output: { index: 1 },
},
];
const onSuccess = vi.fn();
const options: ProcessHookResultsOptions = {
hookType,
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
};
expect(() => processHookResults(options)).toThrow('First hook aborted');
expect(onSuccess).not.toHaveBeenCalled();
// Verify hookProgress is called with the stopReason
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].hookType).toBe(hookType);
expect(mockStream.hookProgressCalls[0].stopReason).toBe('First hook aborted');
});
});
});
});
describe('UserPromptSubmit exit codes', () => {
// Exit code 0 - stdout shown to Claude (onSuccess called)
it('should call onSuccess with output on exit code 0 (success)', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: 'Additional context for Claude',
},
];
const onSuccess = vi.fn();
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).toHaveBeenCalledWith('Additional context for Claude');
});
// Exit code 2 - block processing, erase original prompt, and show stderr to user only
it('should throw HookAbortError and push hookProgress on exit code 2 (error)', () => {
const results: HookResult[] = [
{
resultKind: 'error',
output: 'Validation failed: missing required field',
},
];
const onSuccess = vi.fn();
expect(() =>
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
})
).toThrow(HookAbortError);
expect(onSuccess).not.toHaveBeenCalled();
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].hookType).toBe('UserPromptSubmit');
expect(mockStream.hookProgressCalls[0].stopReason).toContain('Validation failed: missing required field');
});
// Other exit codes - show stderr to user only (warnings flow)
it('should show warning to user on other exit codes', () => {
const results: HookResult[] = [
{
resultKind: 'warning',
warningMessage: 'Process exited with code 1: Some warning',
output: undefined,
},
];
const onSuccess = vi.fn();
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).not.toHaveBeenCalled();
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].hookType).toBe('UserPromptSubmit');
expect(mockStream.hookProgressCalls[0].systemMessage).toBe('Process exited with code 1: Some warning');
});
it('should aggregate multiple warnings', () => {
const results: HookResult[] = [
{ resultKind: 'warning', warningMessage: 'Warning 1', output: undefined },
{ resultKind: 'warning', warningMessage: 'Warning 2', output: undefined },
];
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: () => { },
});
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].systemMessage).toContain('1. Warning 1');
expect(mockStream.hookProgressCalls[0].systemMessage).toContain('2. Warning 2');
});
});
describe('SessionStart exit codes', () => {
// Exit code 0 - stdout shown to Claude
it('should call onSuccess with output on exit code 0 (success)', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { additionalContext: 'Session context data' },
},
];
const onSuccess = vi.fn();
processHookResults({
hookType: 'SessionStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).toHaveBeenCalledWith({ additionalContext: 'Session context data' });
});
// Blocking errors are silently ignored (ignoreErrors: true) - no throw, no hookProgress
it('should silently ignore errors when ignoreErrors is true (no throw, no hookProgress)', () => {
const results: HookResult[] = [
{
resultKind: 'error',
output: 'Session hook error',
},
];
const onSuccess = vi.fn();
expect(() =>
processHookResults({
hookType: 'SessionStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
ignoreErrors: true,
})
).not.toThrow();
expect(onSuccess).not.toHaveBeenCalled();
// hookProgress should NOT be called when ignoreErrors is true
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// stopReason (continue: false) is silently ignored (ignoreErrors: true)
it('should silently ignore stopReason when ignoreErrors is true', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { additionalContext: 'Some context' },
stopReason: 'Build failed, should be ignored',
},
];
const onSuccess = vi.fn();
expect(() =>
processHookResults({
hookType: 'SessionStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
ignoreErrors: true,
})
).not.toThrow();
// stopReason means the result is ignored entirely, so onSuccess is NOT called
expect(onSuccess).not.toHaveBeenCalled();
// hookProgress should NOT be called when ignoreErrors is true
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// Other exit codes - show stderr to user only (warnings)
it('should show warning to user on other exit codes', () => {
const results: HookResult[] = [
{
resultKind: 'warning',
warningMessage: 'Session start warning',
output: undefined,
},
];
processHookResults({
hookType: 'SessionStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: () => { },
});
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].systemMessage).toBe('Session start warning');
});
});
describe('Stop exit codes', () => {
// Exit code 0 - stdout/stderr not shown (success silently processed)
it('should call onSuccess with output on exit code 0 (success)', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { decision: 'allow' },
},
];
const onSuccess = vi.fn();
processHookResults({
hookType: 'Stop',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).toHaveBeenCalledWith({ decision: 'allow' });
// No hookProgress for success
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// Exit code 2 - show stderr to model and continue conversation (onError callback)
it('should call onError callback on exit code 2 (error) instead of throwing', () => {
const results: HookResult[] = [
{
resultKind: 'error',
output: 'Stop hook blocking reason',
},
];
const onSuccess = vi.fn();
const onError = vi.fn();
processHookResults({
hookType: 'Stop',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
onError,
});
expect(onSuccess).not.toHaveBeenCalled();
expect(onError).toHaveBeenCalledWith('Stop hook blocking reason');
// hookProgress should NOT be called when onError is provided
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// Other exit codes - show stderr to user only (warnings)
it('should show warning to user on other exit codes', () => {
const results: HookResult[] = [
{
resultKind: 'warning',
warningMessage: 'Stop hook warning',
output: undefined,
},
];
processHookResults({
hookType: 'Stop',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: () => { },
});
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].systemMessage).toBe('Stop hook warning');
});
});
describe('SubagentStart exit codes', () => {
// Exit code 0 - stdout shown to subagent
it('should call onSuccess with output on exit code 0 (success)', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { additionalContext: 'Subagent context' },
},
];
const onSuccess = vi.fn();
processHookResults({
hookType: 'SubagentStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).toHaveBeenCalledWith({ additionalContext: 'Subagent context' });
});
// Blocking errors are silently ignored (ignoreErrors: true) - no throw, no hookProgress
it('should silently ignore errors when ignoreErrors is true (no throw, no hookProgress)', () => {
const results: HookResult[] = [
{
resultKind: 'error',
output: 'Subagent start error',
},
];
const onSuccess = vi.fn();
expect(() =>
processHookResults({
hookType: 'SubagentStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
ignoreErrors: true,
})
).not.toThrow();
expect(onSuccess).not.toHaveBeenCalled();
// hookProgress should NOT be called when ignoreErrors is true
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// stopReason (continue: false) is silently ignored (ignoreErrors: true)
it('should silently ignore stopReason when ignoreErrors is true', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { additionalContext: 'Subagent context' },
stopReason: 'Blocking condition, should be ignored',
},
];
const onSuccess = vi.fn();
expect(() =>
processHookResults({
hookType: 'SubagentStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
ignoreErrors: true,
})
).not.toThrow();
// stopReason means the result is ignored entirely, so onSuccess is NOT called
expect(onSuccess).not.toHaveBeenCalled();
// hookProgress should NOT be called when ignoreErrors is true
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// Other exit codes - show stderr to user only (warnings)
it('should show warning to user on other exit codes', () => {
const results: HookResult[] = [
{
resultKind: 'warning',
warningMessage: 'Subagent start warning',
output: undefined,
},
];
processHookResults({
hookType: 'SubagentStart',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: () => { },
});
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].systemMessage).toBe('Subagent start warning');
});
});
describe('SubagentStop exit codes', () => {
// Exit code 0 - stdout/stderr not shown (success silently processed)
it('should call onSuccess with output on exit code 0 (success)', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: { decision: 'allow' },
},
];
const onSuccess = vi.fn();
processHookResults({
hookType: 'SubagentStop',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).toHaveBeenCalledWith({ decision: 'allow' });
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// Exit code 2 - show stderr to subagent and continue having it run (onError callback)
it('should call onError callback on exit code 2 (error) instead of throwing', () => {
const results: HookResult[] = [
{
resultKind: 'error',
output: 'Subagent stop blocking reason',
},
];
const onSuccess = vi.fn();
const onError = vi.fn();
processHookResults({
hookType: 'SubagentStop',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
onError,
});
expect(onSuccess).not.toHaveBeenCalled();
expect(onError).toHaveBeenCalledWith('Subagent stop blocking reason');
// hookProgress should NOT be called when onError is provided
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
// Other exit codes - show stderr to user only (warnings)
it('should show warning to user on other exit codes', () => {
const results: HookResult[] = [
{
resultKind: 'warning',
warningMessage: 'Subagent stop warning',
output: undefined,
},
];
processHookResults({
hookType: 'SubagentStop',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: () => { },
});
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].systemMessage).toBe('Subagent stop warning');
});
});
describe('formatHookErrorMessage', () => {
it('should format error message with details', () => {
const message = formatHookErrorMessage('Connection failed');
expect(message).toContain('Connection failed');
expect(message).toContain('hook failed');
});
it('should format error message without details', () => {
const message = formatHookErrorMessage('');
expect(message).toContain('hook failed');
expect(message).not.toContain('Error message:');
});
});
describe('edge cases', () => {
it('should handle empty results array', () => {
const onSuccess = vi.fn();
processHookResults({
hookType: 'UserPromptSubmit',
results: [],
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).not.toHaveBeenCalled();
expect(mockStream.hookProgressCalls).toHaveLength(0);
});
it('should handle undefined outputStream', () => {
const results: HookResult[] = [
{ resultKind: 'warning', warningMessage: 'Warning message', output: undefined },
];
// Should not throw when outputStream is undefined
expect(() =>
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: undefined,
logService,
onSuccess: () => { },
})
).not.toThrow();
});
it('should include warnings from success results', () => {
const results: HookResult[] = [
{
resultKind: 'success',
output: 'some output',
warningMessage: 'Warning from success result',
},
];
const onSuccess = vi.fn();
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess,
});
expect(onSuccess).toHaveBeenCalledWith('some output');
expect(mockStream.hookProgressCalls).toHaveLength(1);
expect(mockStream.hookProgressCalls[0].systemMessage).toBe('Warning from success result');
});
it('should handle error result with empty output', () => {
const results: HookResult[] = [
{
resultKind: 'error',
output: '',
},
];
expect(() =>
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: () => { },
})
).toThrow(HookAbortError);
expect(mockStream.hookProgressCalls).toHaveLength(1);
});
it('should handle error result with non-string output', () => {
const results: HookResult[] = [
{
resultKind: 'error',
output: { complex: 'object' },
},
];
expect(() =>
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: () => { },
})
).toThrow(HookAbortError);
// Empty error message when output is not a string
expect(mockStream.hookProgressCalls).toHaveLength(1);
});
it('should process multiple results in order', () => {
const results: HookResult[] = [
{ resultKind: 'success', output: 'first' },
{ resultKind: 'success', output: 'second' },
{ resultKind: 'success', output: 'third' },
];
const outputs: unknown[] = [];
processHookResults({
hookType: 'UserPromptSubmit',
results,
outputStream: mockStream as unknown as ChatResponseStream,
logService,
onSuccess: (output) => outputs.push(output),
});
expect(outputs).toEqual(['first', 'second', 'third']);
});
});
});
extensions/copilot/src/extension/intents/test/node/toolCallingLoopHooks.spec.ts
+57 -17
View File
@@ -103,7 +103,7 @@ class TestToolCallingLoop extends ToolCallingLoop<IToolCallingLoopOptions> {
// Expose the protected method for testing
public async testRunStartHooks(token: CancellationToken): Promise<void> {
await this.runStartHooks(token);
await this.runStartHooks(undefined, token);
}
// Expose additionalHookContext for verification
@@ -252,7 +252,7 @@ describe('ToolCallingLoop SessionStart hook', () => {
mockChatHookService.setHookResults('SessionStart', [
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 1' },
},
]);
@@ -279,15 +279,15 @@ describe('ToolCallingLoop SessionStart hook', () => {
mockChatHookService.setHookResults('SessionStart', [
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 1' },
},
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 2' },
},
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 3' },
},
]);
@@ -314,15 +314,15 @@ describe('ToolCallingLoop SessionStart hook', () => {
mockChatHookService.setHookResults('SessionStart', [
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 1' },
},
{
success: true,
resultKind: 'success',
output: {}, // No additionalContext
},
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 3' },
},
]);
@@ -343,21 +343,21 @@ describe('ToolCallingLoop SessionStart hook', () => {
expect(additionalContext).toBe('Context from hook 1\nContext from hook 3');
});
it('should ignore failed hook results', async () => {
it('should silently ignore failed hook results (blocking errors are ignored)', async () => {
const conversation = createTestConversation(1);
const request = createMockChatRequest();
mockChatHookService.setHookResults('SessionStart', [
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 1' },
},
{
success: false,
resultKind: 'error',
output: 'Hook error message',
},
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Context from hook 3' },
},
]);
@@ -372,8 +372,48 @@ describe('ToolCallingLoop SessionStart hook', () => {
);
disposables.add(loop);
await loop.testRunStartHooks(tokenSource.token);
// Should NOT throw - blocking errors are silently ignored for SessionStart
await expect(loop.testRunStartHooks(tokenSource.token)).resolves.not.toThrow();
// Only non-error results should be processed
const additionalContext = loop.getAdditionalHookContext();
expect(additionalContext).toBe('Context from hook 1\nContext from hook 3');
});
it('should silently ignore stopReason (continue: false) from hook results', async () => {
const conversation = createTestConversation(1);
const request = createMockChatRequest();
mockChatHookService.setHookResults('SessionStart', [
{
resultKind: 'success',
output: { additionalContext: 'Context from hook 1' },
},
{
resultKind: 'success',
output: { additionalContext: 'Context from hook 2' },
stopReason: 'Build failed, should be ignored',
},
{
resultKind: 'success',
output: { additionalContext: 'Context from hook 3' },
},
]);
const loop = instantiationService.createInstance(
TestToolCallingLoop,
{
conversation,
toolCallLimit: 10,
request,
}
);
disposables.add(loop);
// Should NOT throw - stopReason is silently ignored for SessionStart
await expect(loop.testRunStartHooks(tokenSource.token)).resolves.not.toThrow();
// Results with stopReason are skipped, only other results are processed
const additionalContext = loop.getAdditionalHookContext();
expect(additionalContext).toBe('Context from hook 1\nContext from hook 3');
});
@@ -434,7 +474,7 @@ describe('ToolCallingLoop SessionStart hook', () => {
mockChatHookService.setHookResults('SessionStart', [
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Custom context for prompt' },
},
]);
@@ -548,7 +588,7 @@ describe('ToolCallingLoop SubagentStart hook', () => {
mockChatHookService.setHookResults('SubagentStart', [
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Subagent-specific context' },
},
]);
@@ -578,11 +618,11 @@ describe('ToolCallingLoop SubagentStart hook', () => {
mockChatHookService.setHookResults('SubagentStart', [
{
success: true,
resultKind: 'success',
output: { additionalContext: 'First subagent context' },
},
{
success: true,
resultKind: 'success',
output: { additionalContext: 'Second subagent context' },
},
]);
extensions/copilot/src/extension/prompt/node/defaultIntentRequestHandler.ts
+15 -10
View File
@@ -37,6 +37,7 @@ import { IInstantiationService } from '../../../util/vs/platform/instantiation/c
import { ChatResponseMarkdownPart, ChatResponseProgressPart, ChatResponseTextEditPart, LanguageModelToolResult2 } from '../../../vscodeTypes';
import { CodeBlocksMetadata, CodeBlockTrackingChatResponseStream } from '../../codeBlocks/node/codeBlockProcessor';
import { CopilotInteractiveEditorResponse, InteractionOutcomeComputer } from '../../inlineChat/node/promptCraftingTypes';
import { isHookAbortError, processHookResults } from '../../intents/node/hookResultProcessor';
import { EmptyPromptError, IToolCallingBuiltPromptEvent, IToolCallingLoopOptions, IToolCallingResponseEvent, IToolCallLoopResult, ToolCallingLoop, ToolCallingLoopFetchOptions, ToolCallLimitBehavior } from '../../intents/node/toolCallingLoop';
import { UnknownIntent } from '../../intents/node/unknownIntent';
import { ResponseStreamWithLinkification } from '../../linkify/common/responseStreamWithLinkification';
@@ -169,6 +170,9 @@ export class DefaultIntentRequestHandler {
return CanceledResult;
} else if (err instanceof EmptyPromptError) {
return {};
} else if (isHookAbortError(err)) {
this._logService.info(`[DefaultIntentRequestHandler] Hook ${err.hookType} aborted: ${err.stopReason}`);
return {};
}
this._logService.error(err);
@@ -342,16 +346,17 @@ export class DefaultIntentRequestHandler {
try {
// Execute start hooks first (SessionStart/SubagentStart), then UserPromptSubmit
try {
await loop.runStartHooks(this.token);
} catch (error) {
this._logService.error('[DefaultIntentRequestHandler] Error executing start hooks', error);
}
try {
await this._chatHookService.executeHook('UserPromptSubmit', { toolInvocationToken: this.request.toolInvocationToken, input: { prompt: this.request.prompt } satisfies UserPromptSubmitHookInput }, this.conversation.sessionId);
} catch (error) {
this._logService.error('[DefaultIntentRequestHandler] Error executing UserPromptSubmit hook', error);
}
await loop.runStartHooks(this.stream, this.token);
const userPromptSubmitResults = await this._chatHookService.executeHook('UserPromptSubmit', { toolInvocationToken: this.request.toolInvocationToken, input: { prompt: this.request.prompt } satisfies UserPromptSubmitHookInput }, this.conversation.sessionId, this.token);
processHookResults({
hookType: 'UserPromptSubmit',
results: userPromptSubmitResults,
outputStream: this.stream,
logService: this._logService,
onSuccess: () => { /* UserPromptSubmit has no output to process */ },
});
const result = await loop.run(this.stream, this.token);
if (!result.round.toolCalls.length || result.response.type !== ChatFetchResponseType.Success) {
loop.telemetry.sendToolCallingTelemetry(result.toolCallRounds, result.availableTools, this.token.isCancellationRequested ? 'cancelled' : result.response.type);
extensions/copilot/src/extension/prompts/node/agent/summarizedConversationHistory.tsx
+1 -1
View File
@@ -498,7 +498,7 @@ class ConversationHistorySummarizer {
}, this.props.promptContext.conversation?.sessionId, this.token ?? CancellationToken.None);
for (const result of results) {
if (result.success === false) {
if (result.resultKind === 'error') {
const errorMessage = typeof result.output === 'string' ? result.output : 'Unknown error';
this.logService.error(`[ConversationHistorySummarizer] PreCompact hook error: ${errorMessage}`);
}
extensions/copilot/src/extension/vscode.proposed.chatHooks.d.ts
Vendored
+16 -8
View File
@@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
// version: 2
// version: 3
declare module 'vscode' {
@@ -27,29 +27,37 @@ declare module 'vscode' {
readonly toolInvocationToken: ChatParticipantToolToken;
}
/**
* The kind of result from executing a hook command.
* - 'success': Hook executed successfully (exit code 0)
* - 'error': Blocking error shown to model (exit code 2)
* - 'warning': Non-blocking warning shown to user only (other exit codes)
*/
export type ChatHookResultKind = 'success' | 'error' | 'warning';
/**
* Result of executing a hook command.
* Contains common flow control fields and the hook's output.
*/
export interface ChatHookResult {
/**
* The kind of result from executing the hook.
*/
readonly resultKind: ChatHookResultKind;
/**
* If set, the agent should stop processing entirely after this hook.
* The message is shown to the user but not to the agent.
*/
readonly stopReason?: string;
/**
* Message shown to the user.
* Warning message shown to the user.
*/
readonly messageForUser?: string;
readonly warningMessage?: string;
/**
* The hook's output (hook-specific fields only).
* For errors, this is the error message string.
*/
readonly output: unknown;
/**
* Whether the hook command executed successfully (exit code 0).
*/
readonly success: boolean;
}
export namespace chat {
@@ -64,4 +72,4 @@ declare module 'vscode' {
*/
export function executeHook(hookType: ChatHookType, options: ChatHookExecutionOptions, token?: CancellationToken): Thenable<ChatHookResult[]>;
}
}
}