diff --git a/build/win32/code.iss b/build/win32/code.iss index d889ca8c4d383..0d47e15103fa8 100644 --- a/build/win32/code.iss +++ b/build/win32/code.iss @@ -1453,6 +1453,12 @@ begin Result := not (IsBackgroundUpdate() and FileExists(Path)); end; +// Check if VS Code created a cancel file to signal that the update should be aborted +function CancelFileExists(): Boolean; +begin + Result := FileExists(ExpandConstant('{param:cancel}')) +end; + function ShouldRunAfterUpdate(): Boolean; begin if IsBackgroundUpdate() then @@ -1639,11 +1645,17 @@ begin Log('Checking whether application is still running...'); while (CheckForMutexes('{#AppMutex}')) do begin + if CancelFileExists() then + begin + Log('Cancel file detected, aborting background update.'); + DeleteFile(ExpandConstant('{app}\updating_version')); + Abort; + end; Sleep(1000) end; Log('Application appears not to be running.'); - if not SessionEndFileExists() then begin + if not SessionEndFileExists() and not CancelFileExists() then begin StopTunnelServiceIfNeeded(); Log('Invoking inno_updater for background update'); Exec(ExpandConstant('{app}\{#VersionedResourcesFolder}\tools\inno_updater.exe'), ExpandConstant('"{app}\{#ExeBasename}.exe" ' + BoolToStr(LockFileExists()) + ' "{cm:UpdatingVisualStudioCode}"'), '', SW_SHOW, ewWaitUntilTerminated, UpdateResultCode); @@ -1657,7 +1669,7 @@ begin end; #endif end else begin - Log('Skipping inno_updater.exe call because OS session is ending'); + Log('Skipping inno_updater.exe call because OS session is ending or cancel was requested'); end; end else begin if IsVersionedUpdate() then begin diff --git a/src/vs/editor/contrib/hover/browser/hoverUtils.ts b/src/vs/editor/contrib/hover/browser/hoverUtils.ts index 997d4512c1a94..1dc56a043b6ce 100644 --- a/src/vs/editor/contrib/hover/browser/hoverUtils.ts +++ b/src/vs/editor/contrib/hover/browser/hoverUtils.ts @@ -6,12 +6,16 @@ import * as dom from '../../../../base/browser/dom.js'; import { IEditorMouseEvent } from '../../../browser/editorBrowser.js'; +const enum PADDING { + VALUE = 3 +} + export function isMousePositionWithinElement(element: HTMLElement, posx: number, posy: number): boolean { const elementRect = dom.getDomNodePagePosition(element); - if (posx < elementRect.left - || posx > elementRect.left + elementRect.width - || posy < elementRect.top - || posy > elementRect.top + elementRect.height) { + if (posx < elementRect.left + PADDING.VALUE + || posx > elementRect.left + elementRect.width - PADDING.VALUE + || posy < elementRect.top + PADDING.VALUE + || posy > elementRect.top + elementRect.height - PADDING.VALUE) { return false; } return true; diff --git a/src/vs/editor/contrib/hover/browser/markdownHoverParticipant.ts b/src/vs/editor/contrib/hover/browser/markdownHoverParticipant.ts index e289a3dbca003..ba261eaa4a44a 100644 --- a/src/vs/editor/contrib/hover/browser/markdownHoverParticipant.ts +++ b/src/vs/editor/contrib/hover/browser/markdownHoverParticipant.ts @@ -36,8 +36,8 @@ import { HoverStartSource } from './hoverOperation.js'; import { ScrollEvent } from '../../../../base/common/scrollable.js'; const $ = dom.$; -const increaseHoverVerbosityIcon = registerIcon('hover-increase-verbosity', Codicon.add, nls.localize('increaseHoverVerbosity', 'Icon for increaseing hover verbosity.')); -const decreaseHoverVerbosityIcon = registerIcon('hover-decrease-verbosity', Codicon.remove, nls.localize('decreaseHoverVerbosity', 'Icon for decreasing hover verbosity.')); +const increaseHoverVerbosityIcon = registerIcon('hover-increase-verbosity', Codicon.addSmall, nls.localize('increaseHoverVerbosity', 'Icon for increaseing hover verbosity.')); +const decreaseHoverVerbosityIcon = registerIcon('hover-decrease-verbosity', Codicon.removeSmall, nls.localize('decreaseHoverVerbosity', 'Icon for decreasing hover verbosity.')); export class MarkdownHover implements IHoverPart { diff --git a/src/vs/editor/contrib/hover/test/browser/hoverUtils.test.ts b/src/vs/editor/contrib/hover/test/browser/hoverUtils.test.ts index e40987aeefeb7..593eb58d304bd 100644 --- a/src/vs/editor/contrib/hover/test/browser/hoverUtils.test.ts +++ b/src/vs/editor/contrib/hover/test/browser/hoverUtils.test.ts @@ -114,10 +114,10 @@ suite('Hover Utils', () => { test('returns true when mouse is on element edges', () => { const element = createMockElement(100, 100, 200, 100); - assert.strictEqual(isMousePositionWithinElement(element, 100, 100), true); // top-left corner - assert.strictEqual(isMousePositionWithinElement(element, 300, 100), true); // top-right corner - assert.strictEqual(isMousePositionWithinElement(element, 100, 200), true); // bottom-left corner - assert.strictEqual(isMousePositionWithinElement(element, 300, 200), true); // bottom-right corner + assert.strictEqual(isMousePositionWithinElement(element, 100, 100), false); // top-left corner + assert.strictEqual(isMousePositionWithinElement(element, 300, 100), false); // top-right corner + assert.strictEqual(isMousePositionWithinElement(element, 100, 200), false); // bottom-left corner + assert.strictEqual(isMousePositionWithinElement(element, 300, 200), false); // bottom-right corner }); test('returns false when mouse is left of element', () => { @@ -146,16 +146,16 @@ suite('Hover Utils', () => { test('handles element at origin (0,0)', () => { const element = createMockElement(0, 0, 100, 100); - assert.strictEqual(isMousePositionWithinElement(element, 0, 0), true); + assert.strictEqual(isMousePositionWithinElement(element, 0, 0), false); assert.strictEqual(isMousePositionWithinElement(element, 50, 50), true); - assert.strictEqual(isMousePositionWithinElement(element, 100, 100), true); + assert.strictEqual(isMousePositionWithinElement(element, 100, 100), false); assert.strictEqual(isMousePositionWithinElement(element, 101, 101), false); }); test('handles small elements (1x1)', () => { const element = createMockElement(100, 100, 1, 1); - assert.strictEqual(isMousePositionWithinElement(element, 100, 100), true); - assert.strictEqual(isMousePositionWithinElement(element, 101, 101), true); + assert.strictEqual(isMousePositionWithinElement(element, 100, 100), false); + assert.strictEqual(isMousePositionWithinElement(element, 101, 101), false); assert.strictEqual(isMousePositionWithinElement(element, 102, 102), false); }); }); diff --git a/src/vs/platform/extensions/common/extensionsApiProposals.ts b/src/vs/platform/extensions/common/extensionsApiProposals.ts index 8769eef63c20e..fa429fbf424ea 100644 --- a/src/vs/platform/extensions/common/extensionsApiProposals.ts +++ b/src/vs/platform/extensions/common/extensionsApiProposals.ts @@ -56,7 +56,7 @@ const _allApiProposals = { }, chatParticipantPrivate: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts', - version: 12 + version: 13 }, chatPromptFiles: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.chatPromptFiles.d.ts', diff --git a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts index 66d8a0aa918b5..8de7fdcc25ce3 100644 --- a/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts +++ b/src/vs/platform/terminal/common/xterm/shellIntegrationAddon.ts @@ -21,20 +21,17 @@ import { removeAnsiEscapeCodesFromPrompt } from '../../../../base/common/strings import { ShellEnvDetectionCapability } from '../capabilities/shellEnvDetectionCapability.js'; import { PromptTypeDetectionCapability } from '../capabilities/promptTypeDetectionCapability.js'; - -/** - * Shell integration is a feature that enhances the terminal's understanding of what's happening - * in the shell by injecting special sequences into the shell's prompt using the "Set Text - * Parameters" sequence (`OSC Ps ; Pt ST`). - * - * Definitions: - * - OSC: `\x1b]` - * - Ps: A single (usually optional) numeric parameter, composed of one or more digits. - * - Pt: A text parameter composed of printable characters. - * - ST: `\x7` - * - * This is inspired by a feature of the same name in the FinalTerm, iTerm2 and kitty terminals. - */ +// Shell integration is a feature that enhances the terminal's understanding of what's happening +// in the shell by injecting special sequences into the shell's prompt using the "Set Text +// Parameters" sequence (`OSC Ps ; Pt ST`). +// +// Definitions: +// - OSC: `\x1b]` +// - Ps: A single (usually optional) numeric parameter, composed of one or more digits. +// - Pt: A text parameter composed of printable characters. +// - ST: `\x7` +// +// This is inspired by a feature of the same name in the FinalTerm, iTerm2 and kitty terminals. /** * The identifier for the first numeric parameter (`Ps`) for OSC commands used by shell integration. @@ -174,6 +171,8 @@ const enum VSCodeOscPt { /** * Similar to prompt start but for line continuations. * + * Format: `OSC 633 ; F ST` + * * WARNING: This sequence is unfinalized, DO NOT use this in your shell integration script. */ ContinuationStart = 'F', @@ -181,6 +180,8 @@ const enum VSCodeOscPt { /** * Similar to command start but for line continuations. * + * Format: `OSC 633 ; G ST` + * * WARNING: This sequence is unfinalized, DO NOT use this in your shell integration script. */ ContinuationEnd = 'G', @@ -188,6 +189,8 @@ const enum VSCodeOscPt { /** * The start of the right prompt. * + * Format: `OSC 633 ; H ST` + * * WARNING: This sequence is unfinalized, DO NOT use this in your shell integration script. */ RightPromptStart = 'H', @@ -195,6 +198,8 @@ const enum VSCodeOscPt { /** * The end of the right prompt. * + * Format: `OSC 633 ; I ST` + * * WARNING: This sequence is unfinalized, DO NOT use this in your shell integration script. */ RightPromptEnd = 'I', @@ -224,7 +229,7 @@ const enum VSCodeOscPt { /** * Sets a mark/point-of-interest in the buffer. * - * Format: `OSC 633 ; SetMark [; Id=] [; Hidden]` + * Format: `OSC 633 ; SetMark [; Id=] [; Hidden] ST` * * `Id` - The identifier of the mark that can be used to reference it * `Hidden` - When set, the mark will be available to reference internally but will not visible @@ -236,7 +241,7 @@ const enum VSCodeOscPt { /** * Sends the shell's complete environment in JSON format. * - * Format: `OSC 633 ; EnvJson ; ; ` + * Format: `OSC 633 ; EnvJson ; ; ST` * * - `Environment` - A stringified JSON object containing the shell's complete environment. The * variables and values use the same encoding rules as the {@link CommandLine} sequence. @@ -250,7 +255,7 @@ const enum VSCodeOscPt { /** * Delete a single environment variable from cached environment. * - * Format: `OSC 633 ; EnvSingleDelete ; ; [; ]` + * Format: `OSC 633 ; EnvSingleDelete ; ; [; ] ST` * * - `Nonce` - An optional nonce can be provided which may be required by the terminal in order * to enable some features. This helps ensure no malicious command injection has occurred. @@ -262,7 +267,7 @@ const enum VSCodeOscPt { /** * The start of the collecting user's environment variables individually. * - * Format: `OSC 633 ; EnvSingleStart ; [; ]` + * Format: `OSC 633 ; EnvSingleStart ; [; ] ST` * * - `Clear` - An _mandatory_ flag indicating any cached environment variables will be cleared. * - `Nonce` - An optional nonce can be provided which may be required by the terminal in order @@ -275,7 +280,7 @@ const enum VSCodeOscPt { /** * Sets an entry of single environment variable to transactional pending map of environment variables. * - * Format: `OSC 633 ; EnvSingleEntry ; ; [; ]` + * Format: `OSC 633 ; EnvSingleEntry ; ; [; ] ST` * * - `Nonce` - An optional nonce can be provided which may be required by the terminal in order * to enable some features. This helps ensure no malicious command injection has occurred. @@ -288,7 +293,7 @@ const enum VSCodeOscPt { * The end of the collecting user's environment variables individually. * Clears any pending environment variables and fires an event that contains user's environment. * - * Format: `OSC 633 ; EnvSingleEnd [; ]` + * Format: `OSC 633 ; EnvSingleEnd [; ] ST` * * - `Nonce` - An optional nonce can be provided which may be required by the terminal in order * to enable some features. This helps ensure no malicious command injection has occurred. @@ -305,7 +310,7 @@ const enum ITermOscPt { /** * Sets a mark/point-of-interest in the buffer. * - * Format: `OSC 1337 ; SetMark` + * Format: `OSC 1337 ; SetMark ST` */ SetMark = 'SetMark', diff --git a/src/vs/platform/update/electron-main/abstractUpdateService.ts b/src/vs/platform/update/electron-main/abstractUpdateService.ts index 0c396b416812a..ed54d90f383b3 100644 --- a/src/vs/platform/update/electron-main/abstractUpdateService.ts +++ b/src/vs/platform/update/electron-main/abstractUpdateService.ts @@ -253,7 +253,15 @@ export abstract class AbstractUpdateService implements IUpdateService { if (isLatest === false && this._state.type === StateType.Ready) { this.logService.info('update#readyStateCheck: newer update available, restarting update machinery'); - await this.cancelPendingUpdate(); + + try { + await this.cancelPendingUpdate(); + } catch (error) { + this.logService.error('update#checkForOverwriteUpdates(): failed to cancel pending update, aborting overwrite'); + this.logService.error(error); + return false; + } + this._overwrite = true; this.setState(State.Overwriting(this._state.update, explicit)); this.doCheckForUpdates(explicit, pendingUpdateCommit); diff --git a/src/vs/platform/update/electron-main/updateService.win32.ts b/src/vs/platform/update/electron-main/updateService.win32.ts index 37c7f4e8ec1fe..7778a01ffa34d 100644 --- a/src/vs/platform/update/electron-main/updateService.win32.ts +++ b/src/vs/platform/update/electron-main/updateService.win32.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { spawn } from 'child_process'; +import { ChildProcess, spawn } from 'child_process'; import { existsSync, unlinkSync } from 'fs'; import { mkdir, readFile, unlink } from 'fs/promises'; import { tmpdir } from 'os'; @@ -18,6 +18,7 @@ import { transform } from '../../../base/common/stream.js'; import { URI } from '../../../base/common/uri.js'; import { checksum } from '../../../base/node/crypto.js'; import * as pfs from '../../../base/node/pfs.js'; +import { killTree } from '../../../base/node/processes.js'; import { IConfigurationService } from '../../configuration/common/configuration.js'; import { IEnvironmentMainService } from '../../environment/electron-main/environmentMainService.js'; import { IFileService } from '../../files/common/files.js'; @@ -40,6 +41,10 @@ async function pollUntil(fn: () => boolean, millis = 1000): Promise { interface IAvailableUpdate { packagePath: string; updateFilePath?: string; + /** File path used to signal the Inno Setup installer to cancel */ + cancelFilePath?: string; + /** The Inno Setup process that is applying the update in the background */ + updateProcess?: ChildProcess; } let _updateType: UpdateType | undefined = undefined; @@ -75,7 +80,7 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun @IProductService productService: IProductService, @IMeteredConnectionService meteredConnectionService: IMeteredConnectionService, ) { - super(lifecycleMainService, configurationService, environmentMainService, requestService, logService, productService, meteredConnectionService, false); + super(lifecycleMainService, configurationService, environmentMainService, requestService, logService, productService, meteredConnectionService, true); lifecycleMainService.setRelaunchHandler(this); } @@ -168,14 +173,18 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun return createUpdateURL(this.productService.updateUrl!, platform, quality, commit, options); } - protected doCheckForUpdates(explicit: boolean): void { + protected doCheckForUpdates(explicit: boolean, pendingCommit?: string): void { if (!this.quality) { return; } const background = !explicit && !this.shouldDisableProgressiveReleases(); - const url = this.buildUpdateFeedUrl(this.quality, this.productService.commit!, { background }); - this.setState(State.CheckingForUpdates(explicit)); + const url = this.buildUpdateFeedUrl(this.quality, pendingCommit ?? this.productService.commit!, { background }); + + // Only set CheckingForUpdates if we're not already in Overwriting state + if (this.state.type !== StateType.Overwriting) { + this.setState(State.CheckingForUpdates(explicit)); + } this.requestService.request({ url }, CancellationToken.None) .then(asJson) @@ -183,7 +192,14 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun const updateType = getUpdateType(); if (!update || !update.url || !update.version || !update.productVersion) { - this.setState(State.Idle(updateType)); + // If we were checking for an overwrite update and found nothing newer, + // restore the Ready state with the pending update + if (this.state.type === StateType.Overwriting) { + this._overwrite = false; + this.setState(State.Ready(this.state.update, this.state.explicit, false)); + } else { + this.setState(State.Idle(updateType)); + } return Promise.resolve(null); } @@ -245,13 +261,12 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun }); }).then(packagePath => { this.availableUpdate = { packagePath }; + this.saveUpdateMetadata(update); this.setState(State.Downloaded(update, explicit, this._overwrite)); const fastUpdatesEnabled = this.configurationService.getValue('update.enableWindowsBackgroundUpdates'); - if (fastUpdatesEnabled) { - if (this.productService.target === 'user') { - this.doApplyUpdate(); - } + if (fastUpdatesEnabled && this.productService.target === 'user') { + this.doApplyUpdate(); } else { this.setState(State.Ready(update, explicit, this._overwrite)); } @@ -264,7 +279,15 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun // only show message when explicitly checking for updates const message: string | undefined = explicit ? (err.message || err) : undefined; - this.setState(State.Idle(getUpdateType(), message)); + + // If we were checking for an overwrite update and it failed, + // restore the Ready state with the pending update + if (this.state.type === StateType.Overwriting) { + this._overwrite = false; + this.setState(State.Ready(this.state.update, this.state.explicit, false)); + } else { + this.setState(State.Idle(getUpdateType(), message)); + } }); } @@ -312,15 +335,36 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun const cachePath = await this.cachePath; const sessionEndFlagPath = path.join(cachePath, 'session-ending.flag'); + const cancelFilePath = path.join(cachePath, `cancel.flag`); + try { + await unlink(cancelFilePath); + } catch { + // ignore + } this.availableUpdate.updateFilePath = path.join(cachePath, `CodeSetup-${this.productService.quality}-${update.version}.flag`); + this.availableUpdate.cancelFilePath = cancelFilePath; await pfs.Promises.writeFile(this.availableUpdate.updateFilePath, 'flag'); - const child = spawn(this.availableUpdate.packagePath, ['/verysilent', '/log', `/update="${this.availableUpdate.updateFilePath}"`, `/sessionend="${sessionEndFlagPath}"`, '/nocloseapplications', '/mergetasks=runcode,!desktopicon,!quicklaunchicon'], { - detached: true, - stdio: ['ignore', 'ignore', 'ignore'], - windowsVerbatimArguments: true - }); + const child = spawn(this.availableUpdate.packagePath, + [ + '/verysilent', + '/log', + `/update="${this.availableUpdate.updateFilePath}"`, + `/sessionend="${sessionEndFlagPath}"`, + `/cancel="${cancelFilePath}"`, + '/nocloseapplications', + '/mergetasks=runcode,!desktopicon,!quicklaunchicon' + ], + { + detached: true, + stdio: ['ignore', 'ignore', 'ignore'], + windowsVerbatimArguments: true + } + ); + + // Track the process so we can cancel it if needed + this.availableUpdate.updateProcess = child; child.once('exit', () => { this.availableUpdate = undefined; @@ -335,6 +379,58 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun .then(() => this.setState(State.Ready(update, explicit, this._overwrite))); } + protected override async cancelPendingUpdate(): Promise { + if (!this.availableUpdate) { + return; + } + + this.logService.trace('update#cancelPendingUpdate: cancelling pending update'); + const { updateProcess, updateFilePath, cancelFilePath } = this.availableUpdate; + + if (updateProcess && updateProcess.exitCode === null) { + // Remove all listeners to prevent the exit handler from changing state + updateProcess.removeAllListeners(); + const exitPromise = new Promise(resolve => updateProcess.once('exit', () => resolve(true))); + + // Write the cancel file to signal Inno Setup to exit gracefully + if (cancelFilePath) { + try { + await pfs.Promises.writeFile(cancelFilePath, 'cancel'); + } catch (err) { + this.logService.warn('update#cancelPendingUpdate: failed to write cancel file', err); + } + } + + // Wait for the process to exit gracefully, then force-kill if needed + const pid = updateProcess.pid; + const exited = await Promise.race([exitPromise, timeout(30 * 1000).then(() => false)]); + if (pid && !exited) { + this.logService.trace('update#cancelPendingUpdate: process did not exit gracefully, killing process tree'); + await killTree(pid, true); + } + } + + // Clean up the flag file + if (updateFilePath) { + try { + await unlink(updateFilePath); + } catch (err) { + // ignore + } + } + + // Clean up the cancel file + if (cancelFilePath) { + try { + await unlink(cancelFilePath); + } catch (err) { + // ignore + } + } + + this.availableUpdate = undefined; + } + protected override doQuitAndInstall(): void { if (this.state.type !== StateType.Ready || !this.availableUpdate) { return; @@ -352,6 +448,30 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun } } + private async saveUpdateMetadata(update: IUpdate): Promise { + try { + const cachePath = await this.cachePath; + const metadataPath = path.join(cachePath, 'update-metadata.json'); + await pfs.Promises.writeFile(metadataPath, JSON.stringify(update)); + } catch (e) { + this.logService.error('update#saveUpdateMetadata: failed to save', e); + } + } + + private async loadUpdateMetadata(): Promise { + try { + const cachePath = await this.cachePath; + const metadataPath = path.join(cachePath, 'update-metadata.json'); + if (await pfs.Promises.exists(metadataPath)) { + const content = await readFile(metadataPath, 'utf8'); + return JSON.parse(content); + } + } catch (e) { + this.logService.error('update#loadUpdateMetadata: failed to load', e); + } + return undefined; + } + protected override getUpdateType(): UpdateType { return getUpdateType(); } @@ -362,16 +482,14 @@ export class Win32UpdateService extends AbstractUpdateService implements IRelaun } const fastUpdatesEnabled = this.configurationService.getValue('update.enableWindowsBackgroundUpdates'); - const update: IUpdate = { version: 'unknown', productVersion: 'unknown' }; + const update: IUpdate = await this.loadUpdateMetadata() ?? { version: 'unknown', productVersion: 'unknown' }; this.setState(State.Downloading(update, true, false)); this.availableUpdate = { packagePath }; this.setState(State.Downloaded(update, true, false)); - if (fastUpdatesEnabled) { - if (this.productService.target === 'user') { - this.doApplyUpdate(); - } + if (fastUpdatesEnabled && this.productService.target === 'user') { + this.doApplyUpdate(); } else { this.setState(State.Ready(update, true, false)); } diff --git a/src/vs/workbench/api/common/extHostLanguageModelTools.ts b/src/vs/workbench/api/common/extHostLanguageModelTools.ts index 68b95881b32ae..6095aca4be16a 100644 --- a/src/vs/workbench/api/common/extHostLanguageModelTools.ts +++ b/src/vs/workbench/api/common/extHostLanguageModelTools.ts @@ -127,6 +127,7 @@ export class ExtHostLanguageModelTools implements ExtHostLanguageModelToolsShape chatInteractionId: isProposedApiEnabled(extension, 'chatParticipantPrivate') ? options.chatInteractionId : undefined, subAgentInvocationId: isProposedApiEnabled(extension, 'chatParticipantPrivate') ? options.subAgentInvocationId : undefined, chatStreamToolCallId: isProposedApiEnabled(extension, 'chatParticipantAdditions') ? options.chatStreamToolCallId : undefined, + preToolUseResult: isProposedApiEnabled(extension, 'chatParticipantPrivate') ? options.preToolUseResult : undefined, }, token); const dto: Dto = result instanceof SerializableObjectWithBuffers ? result.value : result; diff --git a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts index 080b6871061ef..6978ffacbf0bd 100644 --- a/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts +++ b/src/vs/workbench/browser/parts/activitybar/activitybarPart.ts @@ -41,7 +41,7 @@ import { SwitchCompositeViewAction } from '../compositeBarActions.js'; export class ActivitybarPart extends Part { - static readonly ACTION_HEIGHT = 48; + static readonly ACTION_HEIGHT = 32; static readonly pinnedViewContainersKey = 'workbench.activity.pinnedViewlets2'; static readonly placeholderViewContainersKey = 'workbench.activity.placeholderViewlets'; @@ -49,8 +49,8 @@ export class ActivitybarPart extends Part { //#region IView - readonly minimumWidth: number = 48; - readonly maximumWidth: number = 48; + readonly minimumWidth: number = 36; + readonly maximumWidth: number = 36; readonly minimumHeight: number = 0; readonly maximumHeight: number = Number.POSITIVE_INFINITY; diff --git a/src/vs/workbench/browser/parts/activitybar/media/activityaction.css b/src/vs/workbench/browser/parts/activitybar/media/activityaction.css index 8cc8ca0e48422..6c19b80055bf2 100644 --- a/src/vs/workbench/browser/parts/activitybar/media/activityaction.css +++ b/src/vs/workbench/browser/parts/activitybar/media/activityaction.css @@ -12,7 +12,7 @@ .monaco-workbench .activitybar > .content .composite-bar > .monaco-action-bar .action-item::after { position: absolute; content: ''; - width: 48px; + width: 36px; height: 2px; display: none; background-color: transparent; @@ -46,8 +46,8 @@ z-index: 1; display: flex; overflow: hidden; - width: 48px; - height: 48px; + width: 36px; + height: 32px; margin-right: 0; box-sizing: border-box; @@ -55,12 +55,12 @@ .monaco-workbench .activitybar > .content :not(.monaco-menu) > .monaco-action-bar .action-label:not(.codicon) { font-size: 15px; - line-height: 40px; - padding: 0 0 0 48px; + line-height: 32px; + padding: 0 0 0 36px; } .monaco-workbench .activitybar > .content :not(.monaco-menu) > .monaco-action-bar .action-label.codicon { - font-size: 24px; + font-size: 16px; align-items: center; justify-content: center; } @@ -157,27 +157,28 @@ .monaco-workbench .activitybar > .content :not(.monaco-menu) > .monaco-action-bar .badge .badge-content { position: absolute; - top: 24px; - right: 8px; + top: 17px; + right: 6px; font-size: 9px; font-weight: 600; - min-width: 8px; - height: 16px; - line-height: 16px; - padding: 0 4px; - border-radius: 20px; + min-width: 9px; + height: 13px; + line-height: 13px; + padding: 0 2px; + border-radius: 13px; text-align: center; + border: none !important; } .monaco-workbench .activitybar > .content :not(.monaco-menu) > .monaco-action-bar .profile-badge .profile-text-overlay { position: absolute; font-weight: 600; - font-size: 9px; - line-height: 10px; - top: 24px; - right: 6px; - padding: 2px 3px; - border-radius: 7px; + font-size: 8px; + line-height: 8px; + top: 14px; + right: 2px; + padding: 2px 2px; + border-radius: 6px; background-color: var(--vscode-profileBadge-background); color: var(--vscode-profileBadge-foreground); border: 2px solid var(--vscode-activityBar-background); diff --git a/src/vs/workbench/browser/parts/activitybar/media/activitybarpart.css b/src/vs/workbench/browser/parts/activitybar/media/activitybarpart.css index abe1427ca20a9..452cc971b2486 100644 --- a/src/vs/workbench/browser/parts/activitybar/media/activitybarpart.css +++ b/src/vs/workbench/browser/parts/activitybar/media/activitybarpart.css @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ .monaco-workbench .part.activitybar { - width: 48px; + width: 36px; height: 100%; } diff --git a/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts index aba6e221b024e..4bc871cae8938 100644 --- a/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts @@ -20,6 +20,7 @@ import { derived, derivedOpts, IObservable, IReader, observableFromEventOpts, Ob import Severity from '../../../../../base/common/severity.js'; import { StopWatch } from '../../../../../base/common/stopwatch.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; +import { URI } from '../../../../../base/common/uri.js'; import { localize, localize2 } from '../../../../../nls.js'; import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../../platform/accessibilitySignal/browser/accessibilitySignalService.js'; @@ -35,8 +36,6 @@ import { Registry } from '../../../../../platform/registry/common/platform.js'; import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; import { ITelemetryService } from '../../../../../platform/telemetry/common/telemetry.js'; import { IExtensionService } from '../../../../services/extensions/common/extensions.js'; -import { IPostToolUseCallerInput, IPreToolUseCallerInput, IPreToolUseHookResult } from '../../common/hooks/hooksTypes.js'; -import { HookAbortError, IHooksExecutionService } from '../../common/hooks/hooksExecutionService.js'; import { ChatContextKeys } from '../../common/actions/chatContextKeys.js'; import { ChatRequestToolReferenceEntry, toToolSetVariableEntry, toToolVariableEntry } from '../../common/attachments/chatVariableEntries.js'; import { IVariableReference } from '../../common/chatModes.js'; @@ -45,12 +44,11 @@ import { ChatConfiguration } from '../../common/constants.js'; import { ILanguageModelChatMetadata } from '../../common/languageModels.js'; import { IChatModel, IChatRequestModel } from '../../common/model/chatModel.js'; import { ChatToolInvocation } from '../../common/model/chatProgressTypes/chatToolInvocation.js'; -import { ILanguageModelToolsConfirmationService } from '../../common/tools/languageModelToolsConfirmationService.js'; -import { CountTokensCallback, createToolSchemaUri, IBeginToolCallOptions, ILanguageModelToolsService, IPreparedToolInvocation, IToolAndToolSetEnablementMap, IToolData, IToolImpl, IToolInvocation, IToolResult, IToolResultInputOutputDetails, SpecedToolAliases, stringifyPromptTsxPart, isToolSet, ToolDataSource, toolContentToA11yString, toolMatchesModel, ToolSet, VSCodeToolReference, IToolSet, ToolSetForModel, IToolInvokedEvent } from '../../common/tools/languageModelToolsService.js'; -import { getToolConfirmationAlert } from '../accessibility/chatAccessibilityProvider.js'; -import { URI } from '../../../../../base/common/uri.js'; import { chatSessionResourceToId } from '../../common/model/chatUri.js'; import { HookType } from '../../common/promptSyntax/hookSchema.js'; +import { ILanguageModelToolsConfirmationService } from '../../common/tools/languageModelToolsConfirmationService.js'; +import { CountTokensCallback, createToolSchemaUri, IBeginToolCallOptions, IExternalPreToolUseHookResult, ILanguageModelToolsService, IPreparedToolInvocation, isToolSet, IToolAndToolSetEnablementMap, IToolData, IToolImpl, IToolInvocation, IToolInvokedEvent, IToolResult, IToolResultInputOutputDetails, IToolSet, SpecedToolAliases, stringifyPromptTsxPart, ToolDataSource, toolMatchesModel, ToolSet, ToolSetForModel, VSCodeToolReference } from '../../common/tools/languageModelToolsService.js'; +import { getToolConfirmationAlert } from '../accessibility/chatAccessibilityProvider.js'; const jsonSchemaRegistry = Registry.as(JSONContributionRegistry.Extensions.JSONContribution); @@ -126,7 +124,6 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo @IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService, @IStorageService private readonly _storageService: IStorageService, @ILanguageModelToolsConfirmationService private readonly _confirmationService: ILanguageModelToolsConfirmationService, - @IHooksExecutionService private readonly _hooksExecutionService: IHooksExecutionService, @ICommandService private readonly _commandService: ICommandService, ) { super(); @@ -367,74 +364,35 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo return undefined; } - /** - * Execute the preToolUse hook and handle denial. - * Returns an object containing: - * - denialResult: A tool result if the hook denied execution (caller should return early) - * - hookResult: The full hook result for use in auto-approval logic (allow/ask decisions) - * @param pendingInvocation If there's an existing streaming invocation from beginToolCall, pass it here to cancel it instead of creating a new one. - */ - private async _executePreToolUseHook( + private _handlePreToolUseDenial( dto: IToolInvocation, + hookResult: IExternalPreToolUseHookResult, toolData: IToolData | undefined, - request: IChatRequestModel | undefined, pendingInvocation: ChatToolInvocation | undefined, - token: CancellationToken - ): Promise<{ denialResult?: IToolResult; hookResult?: IPreToolUseHookResult }> { - // Skip hook if no session context or tool doesn't exist - if (!dto.context?.sessionResource || !toolData) { - return {}; - } - - const hookInput: IPreToolUseCallerInput = { - toolName: dto.toolId, - toolInput: dto.parameters, - toolCallId: dto.callId, - }; - let hookResult: IPreToolUseHookResult | undefined; - try { - hookResult = await this._hooksExecutionService.executePreToolUseHook(dto.context.sessionResource, hookInput, token); - } catch (e) { - if (e instanceof HookAbortError) { - this._logService.debug(`[LanguageModelToolsService#invokeTool] Tool ${dto.toolId} aborted by preToolUse hook: ${e.stopReason}`); - throw new CancellationError(); - } - throw e; - } - - if (hookResult?.permissionDecision === 'deny') { - const hookReason = hookResult.permissionDecisionReason ?? localize('hookDeniedNoReason', "Hook denied tool execution"); - const reason = localize('deniedByPreToolUseHook', "Denied by {0} hook: {1}", HookType.PreToolUse, hookReason); - this._logService.debug(`[LanguageModelToolsService#invokeTool] Tool ${dto.toolId} denied by preToolUse hook: ${hookReason}`); - - // Handle the tool invocation in cancelled state - if (toolData) { - if (pendingInvocation) { - // If there's an existing streaming invocation, cancel it - pendingInvocation.cancelFromStreaming(ToolConfirmKind.Denied, reason); - } else if (request) { - // Otherwise create a new cancelled invocation and add it to the chat model - const toolInvocation = ChatToolInvocation.createCancelled( - { toolCallId: dto.callId, toolId: dto.toolId, toolData, subagentInvocationId: dto.subAgentInvocationId, chatRequestId: dto.chatRequestId }, - dto.parameters, - ToolConfirmKind.Denied, - reason - ); - this._chatService.appendProgress(request, toolInvocation); - } + request: IChatRequestModel | undefined, + ): IToolResult { + const hookReason = hookResult.permissionDecisionReason ?? localize('hookDeniedNoReason', "Hook denied tool execution"); + const reason = localize('deniedByPreToolUseHook', "Denied by {0} hook: {1}", HookType.PreToolUse, hookReason); + this._logService.debug(`[LanguageModelToolsService#invokeTool] Tool ${dto.toolId} denied by preToolUse hook: ${hookReason}`); + + if (toolData) { + if (pendingInvocation) { + pendingInvocation.cancelFromStreaming(ToolConfirmKind.Denied, reason); + } else if (request) { + const cancelledInvocation = ChatToolInvocation.createCancelled( + { toolCallId: dto.callId, toolId: dto.toolId, toolData, subagentInvocationId: dto.subAgentInvocationId, chatRequestId: dto.chatRequestId }, + dto.parameters, + ToolConfirmKind.Denied, + reason + ); + this._chatService.appendProgress(request, cancelledInvocation); } - - const denialMessage = localize('toolExecutionDenied', "Tool execution denied: {0}", hookReason); - return { - denialResult: { - content: [{ kind: 'text', value: denialMessage }], - toolResultError: hookReason, - }, - hookResult, - }; } - return { hookResult }; + return { + content: [{ kind: 'text', value: `Tool execution denied: ${hookReason}` }], + toolResultError: hookReason, + }; } /** @@ -469,53 +427,6 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo return undefined; } - /** - * Execute the postToolUse hook after tool completion. - * If the hook returns a "block" decision, additional context is appended to the tool result - * as feedback for the agent indicating the block and reason. The tool has already run, - * so blocking only provides feedback. - */ - private async _executePostToolUseHook( - dto: IToolInvocation, - toolResult: IToolResult, - token: CancellationToken - ): Promise { - if (!dto.context?.sessionResource) { - return; - } - - const hookInput: IPostToolUseCallerInput = { - toolName: dto.toolId, - toolInput: dto.parameters, - getToolResponseText: () => toolContentToA11yString(toolResult.content), - toolCallId: dto.callId, - }; - let hookResult; - try { - hookResult = await this._hooksExecutionService.executePostToolUseHook(dto.context.sessionResource, hookInput, token); - } catch (e) { - if (e instanceof HookAbortError) { - this._logService.debug(`[LanguageModelToolsService#invokeTool] PostToolUse hook aborted for tool ${dto.toolId}: ${e.stopReason}`); - throw new CancellationError(); - } - throw e; - } - - if (hookResult?.decision === 'block') { - const hookReason = hookResult.reason ?? localize('postToolUseHookBlockedNoReason', "Hook blocked tool result"); - this._logService.debug(`[LanguageModelToolsService#invokeTool] PostToolUse hook blocked for tool ${dto.toolId}: ${hookReason}`); - const blockMessage = localize('postToolUseHookBlockedContext', "The PostToolUse hook blocked this tool result. Reason: {0}", hookReason); - toolResult.content.push({ kind: 'text', value: '\n\n' + blockMessage + '\n' }); - } - - if (hookResult?.additionalContext) { - // Append additional context from all hooks to the tool result content - for (const context of hookResult.additionalContext) { - toolResult.content.push({ kind: 'text', value: '\n\n' + context + '\n' }); - } - } - } - async invokeTool(dto: IToolInvocation, countTokens: CountTokensCallback, token: CancellationToken): Promise { this._logService.trace(`[LanguageModelToolsService#invokeTool] Invoking tool ${dto.toolId} with parameters ${JSON.stringify(dto.parameters)}`); @@ -563,14 +474,14 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo token = source.token; } - // Execute preToolUse hook - returns early if hook denies execution - const { denialResult: hookDenialResult, hookResult: preToolUseHookResult } = await this._executePreToolUseHook(dto, toolData, request, toolInvocation, token); - if (hookDenialResult) { - // Clean up pending tool call if it exists + // Handle preToolUse hook denial + const preToolUseHookResult = dto.preToolUseResult; + if (preToolUseHookResult?.permissionDecision === 'deny') { + const denialResult = this._handlePreToolUseDenial(dto, preToolUseHookResult, toolData, toolInvocation, request); if (pendingToolCallKey) { this._pendingToolCalls.delete(pendingToolCallKey); } - return hookDenialResult; + return denialResult; } // Apply updatedInput from preToolUse hook if provided, after validating against the tool's input schema @@ -730,9 +641,6 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo } } - // Execute postToolUse hook after successful tool execution - await this._executePostToolUseHook(dto, toolResult, token); - this._telemetryService.publicLog2( 'languageModelToolInvoked', { @@ -777,7 +685,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo } } - private async prepareToolInvocationWithHookResult(tool: IToolEntry, dto: IToolInvocation, hookResult: IPreToolUseHookResult | undefined, token: CancellationToken): Promise { + private async prepareToolInvocationWithHookResult(tool: IToolEntry, dto: IToolInvocation, hookResult: IExternalPreToolUseHookResult | undefined, token: CancellationToken): Promise { let forceConfirmationReason: string | undefined; if (hookResult?.permissionDecision === 'ask') { const hookMessage = localize('preToolUseHookRequiredConfirmation', "{0} required confirmation", HookType.PreToolUse); @@ -798,7 +706,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo * since when the hook returns 'ask' and preparedInvocation was undefined, we create one. */ private async resolveAutoConfirmFromHook( - hookResult: IPreToolUseHookResult | undefined, + hookResult: IExternalPreToolUseHookResult | undefined, tool: IToolEntry, dto: IToolInvocation, preparedInvocation: IPreparedToolInvocation | undefined, diff --git a/src/vs/workbench/contrib/chat/common/hooks/hooksCommandTypes.ts b/src/vs/workbench/contrib/chat/common/hooks/hooksCommandTypes.ts index b939a40204080..1b31c0fafd5af 100644 --- a/src/vs/workbench/contrib/chat/common/hooks/hooksCommandTypes.ts +++ b/src/vs/workbench/contrib/chat/common/hooks/hooksCommandTypes.ts @@ -9,10 +9,6 @@ * "External" means these types define the contract between VS Code and the external hook * command process. * - * Examples: - * - IPreToolUseCommandInput: sent TO the spawned command via stdin - * - IPreToolUseCommandOutput: received FROM the spawned command via stdout - * * Internal types (in hooksTypes.ts) are used within VS Code. */ @@ -69,59 +65,3 @@ export interface IHookCommandResult { } //#endregion - -//#region PreToolUse Hook Types - -/** - * Tool-specific command input fields for preToolUse hook. - * These are mixed with IHookCommandInput at runtime. - */ -export interface IPreToolUseCommandInput { - readonly tool_name: string; - readonly tool_input: unknown; - readonly tool_use_id: string; -} - -/** - * External command output for preToolUse hook. - * Extends common output with hookSpecificOutput wrapper. - */ -export interface IPreToolUseCommandOutput extends IHookCommandOutput { - readonly hookSpecificOutput?: { - readonly hookEventName?: string; - readonly permissionDecision?: 'allow' | 'deny'; - readonly permissionDecisionReason?: string; - readonly updatedInput?: object; - readonly additionalContext?: string; - }; -} - -//#endregion - -//#region PostToolUse Hook Types - -/** - * Tool-specific command input fields for postToolUse hook. - * These are mixed with IHookCommandInput at runtime. - */ -export interface IPostToolUseCommandInput { - readonly tool_name: string; - readonly tool_input: unknown; - readonly tool_response: string; - readonly tool_use_id: string; -} - -/** - * External command output for postToolUse hook. - * Extends common output with decision control fields. - */ -export interface IPostToolUseCommandOutput extends IHookCommandOutput { - readonly decision?: 'block'; - readonly reason?: string; - readonly hookSpecificOutput?: { - readonly hookEventName?: string; - readonly additionalContext?: string; - }; -} - -//#endregion diff --git a/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts b/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts index 266ce042a49d3..5e6f02e05b4db 100644 --- a/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts +++ b/src/vs/workbench/contrib/chat/common/hooks/hooksExecutionService.ts @@ -13,24 +13,15 @@ import { createDecorator } from '../../../../../platform/instantiation/common/in import { ILogService } from '../../../../../platform/log/common/log.js'; import { Registry } from '../../../../../platform/registry/common/platform.js'; import { Extensions, IOutputChannelRegistry, IOutputService } from '../../../../services/output/common/output.js'; -import { HookType, HookTypeValue, IChatRequestHooks, IHookCommand } from '../promptSyntax/hookSchema.js'; +import { HookTypeValue, IChatRequestHooks, IHookCommand } from '../promptSyntax/hookSchema.js'; import { HookCommandResultKind, IHookCommandInput, IHookCommandResult, - IPostToolUseCommandInput, - IPreToolUseCommandInput } from './hooksCommandTypes.js'; import { commonHookOutputValidator, IHookResult, - IPostToolUseCallerInput, - IPostToolUseHookResult, - IPreToolUseCallerInput, - IPreToolUseHookResult, - postToolUseOutputValidator, - PreToolUsePermissionDecision, - preToolUseOutputValidator } from './hooksTypes.js'; export const hooksOutputChannelId = 'hooksExecution'; @@ -101,21 +92,6 @@ export interface IHooksExecutionService { * Execute hooks of the given type for the given session */ executeHook(hookType: HookTypeValue, sessionResource: URI, options?: IHooksExecutionOptions): Promise; - - /** - * Execute preToolUse hooks with typed input and validated output. - * The execution service builds the full hook input from the caller input plus session context. - * Returns a combined result with common fields and permission decision. - */ - executePreToolUseHook(sessionResource: URI, input: IPreToolUseCallerInput, token?: CancellationToken): Promise; - - /** - * Execute postToolUse hooks with typed input and validated output. - * Called after a tool completes successfully. The execution service builds the full hook input - * from the caller input plus session context. - * Returns a combined result with decision and additional context. - */ - executePostToolUseHook(sessionResource: URI, input: IPostToolUseCallerInput, token?: CancellationToken): Promise; } /** @@ -439,6 +415,15 @@ export class HooksExecutionService extends Disposable implements IHooksExecution } } + // Emit aggregated warnings for any hook results that had warning messages + this._emitAggregatedWarnings(hookType, sessionResource, results); + + // If any hook set stopReason, emit progress so it's visible to the user + const stoppedResult = results.find(r => r.stopReason !== undefined); + if (stoppedResult?.stopReason) { + this._emitHookProgress(hookType, sessionResource, formatHookErrorMessage(stoppedResult.stopReason)); + } + return results; } finally { this._onDidExecuteHook.fire({ @@ -451,170 +436,6 @@ export class HooksExecutionService extends Disposable implements IHooksExecution } } - async executePreToolUseHook(sessionResource: URI, input: IPreToolUseCallerInput, token?: CancellationToken): Promise { - const toolSpecificInput: IPreToolUseCommandInput = { - tool_name: input.toolName, - tool_input: input.toolInput, - tool_use_id: input.toolCallId, - }; - - const results = await this.executeHook(HookType.PreToolUse, sessionResource, { - input: toolSpecificInput, - token: token ?? CancellationToken.None, - }); - - // Run all hooks and collapse results. Most restrictive decision wins: deny > ask > allow. - // Collect all additionalContext strings from every hook. - const allAdditionalContext: string[] = []; - let mostRestrictiveDecision: PreToolUsePermissionDecision | undefined; - let winningResult: IHookResult | undefined; - let winningReason: string | undefined; - let lastUpdatedInput: object | undefined; - - for (const result of results) { - if (result.resultKind === 'success' && typeof result.output === 'object' && result.output !== null) { - const validationResult = preToolUseOutputValidator.validate(result.output); - if (!validationResult.error) { - const hookSpecificOutput = validationResult.content.hookSpecificOutput; - if (hookSpecificOutput) { - // Validate hookEventName if present - must match the hook type - if (hookSpecificOutput.hookEventName !== undefined && hookSpecificOutput.hookEventName !== HookType.PreToolUse) { - this._logService.warn(`[HooksExecutionService] preToolUse hook returned invalid hookEventName '${hookSpecificOutput.hookEventName}', expected '${HookType.PreToolUse}'`); - continue; - } - - // Collect additionalContext from every hook - if (hookSpecificOutput.additionalContext) { - allAdditionalContext.push(hookSpecificOutput.additionalContext); - } - - // Track the last updatedInput (later hooks override earlier ones) - if (hookSpecificOutput.updatedInput) { - lastUpdatedInput = hookSpecificOutput.updatedInput; - } - - // Track the most restrictive decision: deny > ask > allow - const decision = hookSpecificOutput.permissionDecision; - if (decision && this._isMoreRestrictive(decision, mostRestrictiveDecision)) { - mostRestrictiveDecision = decision; - winningResult = result; - winningReason = hookSpecificOutput.permissionDecisionReason; - } - } - } else { - this._logService.warn(`[HooksExecutionService] preToolUse hook output validation failed: ${validationResult.error.message}`); - } - } - } - - const baseResult = winningResult ?? results[0]; - - // Emit hook progress for warning messages after all hooks have completed - this._emitAggregatedWarnings(HookType.PreToolUse, sessionResource, results); - - // If any hook set stopReason, throw HookAbortError after processing - const stoppedResult = results.find(r => r.stopReason !== undefined); - if (stoppedResult?.stopReason !== undefined) { - this._emitHookProgress(HookType.PreToolUse, sessionResource, formatHookErrorMessage(stoppedResult.stopReason)); - throw new HookAbortError(HookType.PreToolUse, stoppedResult.stopReason ?? 'Unknown error'); - } - - return { - ...baseResult, - permissionDecision: mostRestrictiveDecision, - permissionDecisionReason: winningReason, - updatedInput: lastUpdatedInput, - additionalContext: allAdditionalContext.length > 0 ? allAdditionalContext : undefined, - }; - } - - /** - * Returns true if `candidate` is more restrictive than `current`. - * Restriction order: deny > ask > allow. - */ - private _isMoreRestrictive(candidate: PreToolUsePermissionDecision, current: PreToolUsePermissionDecision | undefined): boolean { - const order: Record = { 'deny': 2, 'ask': 1, 'allow': 0 }; - return current === undefined || order[candidate] > order[current]; - } - - async executePostToolUseHook(sessionResource: URI, input: IPostToolUseCallerInput, token?: CancellationToken): Promise { - // Check if there are PostToolUse hooks registered before doing any work stringifying tool results - const hooks = this.getHooksForSession(sessionResource); - const hookCommands = hooks?.[HookType.PostToolUse]; - if (!hookCommands || hookCommands.length === 0) { - return undefined; - } - - // Lazily render tool response text only when hooks are registered - const toolResponseText = input.getToolResponseText(); - - const toolSpecificInput: IPostToolUseCommandInput = { - tool_name: input.toolName, - tool_input: input.toolInput, - tool_response: toolResponseText, - tool_use_id: input.toolCallId, - }; - - const results = await this.executeHook(HookType.PostToolUse, sessionResource, { - input: toolSpecificInput, - token: token ?? CancellationToken.None, - }); - - // Run all hooks and collapse results. Block is the most restrictive decision. - // Collect all additionalContext strings from every hook. - const allAdditionalContext: string[] = []; - let hasBlock = false; - let blockReason: string | undefined; - let blockResult: IHookResult | undefined; - - for (const result of results) { - if (result.resultKind === 'success' && typeof result.output === 'object' && result.output !== null) { - const validationResult = postToolUseOutputValidator.validate(result.output); - if (!validationResult.error) { - const validated = validationResult.content; - - // Validate hookEventName if present - if (validated.hookSpecificOutput?.hookEventName !== undefined && validated.hookSpecificOutput.hookEventName !== HookType.PostToolUse) { - this._logService.warn(`[HooksExecutionService] postToolUse hook returned invalid hookEventName '${validated.hookSpecificOutput.hookEventName}', expected '${HookType.PostToolUse}'`); - continue; - } - - // Collect additionalContext from every hook - if (validated.hookSpecificOutput?.additionalContext) { - allAdditionalContext.push(validated.hookSpecificOutput.additionalContext); - } - - // Track the first block decision (most restrictive) - if (validated.decision === 'block' && !hasBlock) { - hasBlock = true; - blockReason = validated.reason; - blockResult = result; - } - } else { - this._logService.warn(`[HooksExecutionService] postToolUse hook output validation failed: ${validationResult.error.message}`); - } - } - } - - const baseResult = blockResult ?? results[0]; - - // Emit hook progress for warning messages after all hooks have completed - this._emitAggregatedWarnings(HookType.PostToolUse, sessionResource, results); - - // If any hook set stopReason, throw HookAbortError after processing - const stoppedResult = results.find(r => r.stopReason !== undefined); - if (stoppedResult?.stopReason !== undefined) { - this._emitHookProgress(HookType.PostToolUse, sessionResource, formatHookErrorMessage(stoppedResult.stopReason)); - throw new HookAbortError(HookType.PostToolUse, stoppedResult.stopReason ?? 'Unknown error'); - } - - return { - ...baseResult, - decision: hasBlock ? 'block' : undefined, - reason: blockReason, - additionalContext: allAdditionalContext.length > 0 ? allAdditionalContext : undefined, - }; - } } /** diff --git a/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts b/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts index f35308d893582..1c4bceb531406 100644 --- a/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts +++ b/src/vs/workbench/contrib/chat/common/hooks/hooksTypes.ts @@ -9,14 +9,10 @@ * "Internal" means these types are used by VS Code code only - they never cross the * process boundary to external hook commands. They use camelCase for field names. * - * Examples: - * - IPreToolUseCallerInput: provided by VS Code callers (e.g., LanguageModelToolsService) - * - IPreToolUseHookResult: returned TO VS Code callers after processing command output - * * External types (in hooksCommandTypes.ts) define the contract with spawned commands. */ -import { vBoolean, vEnum, vObj, vObjAny, vOptionalProp, vString } from '../../../../../base/common/validation.js'; +import { vBoolean, vObj, vOptionalProp, vString } from '../../../../../base/common/validation.js'; //#region Common Hook Types @@ -58,87 +54,3 @@ export const commonHookOutputValidator = vObj({ }); //#endregion - -//#region PreToolUse Hook Types - -/** - * Input provided by VS Code callers when invoking the preToolUse hook. - */ -export interface IPreToolUseCallerInput { - readonly toolName: string; - readonly toolInput: unknown; - readonly toolCallId: string; -} - -export const preToolUseOutputValidator = vObj({ - hookSpecificOutput: vOptionalProp(vObj({ - hookEventName: vOptionalProp(vString()), - permissionDecision: vOptionalProp(vEnum('allow', 'deny', 'ask')), - permissionDecisionReason: vOptionalProp(vString()), - updatedInput: vOptionalProp(vObjAny()), - additionalContext: vOptionalProp(vString()), - })), -}); - -/** - * Valid permission decisions for preToolUse hooks. - * - 'allow': Auto-approve the tool execution (skip user confirmation) - * - 'deny': Deny the tool execution - * - 'ask': Always require user confirmation (never auto-approve) - */ -export type PreToolUsePermissionDecision = 'allow' | 'deny' | 'ask'; - -/** - * Result from preToolUse hooks with permission decision fields. - * Returned to VS Code callers. Represents the collapsed result of all hooks. - */ -export interface IPreToolUseHookResult extends IHookResult { - readonly permissionDecision?: PreToolUsePermissionDecision; - readonly permissionDecisionReason?: string; - /** - * Modified tool input parameters from the hook. - * When set, replaces the original tool input before execution. - * Combine with 'allow' to auto-approve, or 'ask' to show modified input to the user. - */ - readonly updatedInput?: object; - readonly additionalContext?: string[]; -} - -//#endregion - -//#region PostToolUse Hook Types - -/** - * Input provided by VS Code callers when invoking the postToolUse hook. - * The toolResponse is a lazy getter that renders the tool result content to a string. - * It is only called if there are PostToolUse hooks registered. - */ -export interface IPostToolUseCallerInput { - readonly toolName: string; - readonly toolInput: unknown; - readonly getToolResponseText: () => string; - readonly toolCallId: string; -} - -export const postToolUseOutputValidator = vObj({ - decision: vOptionalProp(vEnum('block')), - reason: vOptionalProp(vString()), - hookSpecificOutput: vOptionalProp(vObj({ - hookEventName: vOptionalProp(vString()), - additionalContext: vOptionalProp(vString()), - })), -}); - -export type PostToolUseDecision = 'block'; - -/** - * Result from postToolUse hooks with decision fields. - * Returned to VS Code callers. Represents the collapsed result of all hooks. - */ -export interface IPostToolUseHookResult extends IHookResult { - readonly decision?: PostToolUseDecision; - readonly reason?: string; - readonly additionalContext?: string[]; -} - -//#endregion diff --git a/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts index 05a5d1d17524a..9153ccc9facfb 100644 --- a/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/common/tools/languageModelToolsService.ts @@ -162,6 +162,16 @@ export namespace ToolDataSource { } } +/** + * Pre-tool-use hook result passed from the extension when the hook was executed externally. + */ +export interface IExternalPreToolUseHookResult { + permissionDecision?: 'allow' | 'deny' | 'ask'; + permissionDecisionReason?: string; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + updatedInput?: Record; +} + export interface IToolInvocation { callId: string; toolId: string; @@ -184,6 +194,8 @@ export interface IToolInvocation { userSelectedTools?: UserSelectedTools; /** The label of the custom button selected by the user during confirmation, if custom buttons were used. */ selectedCustomButton?: string; + /** Pre-tool-use hook result passed from the extension, if the hook was already executed externally. */ + preToolUseResult?: IExternalPreToolUseHookResult; } export interface IToolInvocationContext { diff --git a/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts b/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts index 3913649070d47..a5b61a2687935 100644 --- a/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/tools/languageModelToolsService.test.ts @@ -34,7 +34,7 @@ import { ILanguageModelToolsConfirmationService } from '../../../common/tools/la import { MockLanguageModelToolsConfirmationService } from '../../common/tools/mockLanguageModelToolsConfirmationService.js'; import { runWithFakedTimers } from '../../../../../../base/test/common/timeTravelScheduler.js'; import { ILanguageModelChatMetadata } from '../../../common/languageModels.js'; -import { IHookResult, IPostToolUseCallerInput, IPostToolUseHookResult, IPreToolUseCallerInput, IPreToolUseHookResult } from '../../../common/hooks/hooksTypes.js'; +import { IHookResult } from '../../../common/hooks/hooksTypes.js'; import { IHooksExecutionService, IHooksExecutionOptions, IHooksExecutionProxy } from '../../../common/hooks/hooksExecutionService.js'; import { HookTypeValue, IChatRequestHooks } from '../../../common/promptSyntax/hookSchema.js'; import { IDisposable } from '../../../../../../base/common/lifecycle.js'; @@ -69,10 +69,6 @@ class MockHooksExecutionService implements IHooksExecutionService { readonly _serviceBrand: undefined; readonly onDidExecuteHook = Event.None; readonly onDidHookProgress = Event.None; - public preToolUseHookResult: IPreToolUseHookResult | undefined = undefined; - public postToolUseHookResult: IPostToolUseHookResult | undefined = undefined; - public lastPreToolUseInput: IPreToolUseCallerInput | undefined = undefined; - public lastPostToolUseInput: IPostToolUseCallerInput | undefined = undefined; setProxy(_proxy: IHooksExecutionProxy): void { } registerHooks(_sessionResource: URI, _hooks: IChatRequestHooks): IDisposable { return { dispose: () => { } }; } @@ -80,14 +76,6 @@ class MockHooksExecutionService implements IHooksExecutionService { executeHook(_hookType: HookTypeValue, _sessionResource: URI, _options?: IHooksExecutionOptions): Promise { return Promise.resolve([]); } - async executePreToolUseHook(_sessionResource: URI, input: IPreToolUseCallerInput, _token?: CancellationToken): Promise { - this.lastPreToolUseInput = input; - return this.preToolUseHookResult; - } - async executePostToolUseHook(_sessionResource: URI, input: IPostToolUseCallerInput, _token?: CancellationToken): Promise { - this.lastPostToolUseInput = input; - return this.postToolUseHookResult; - } } function registerToolForTest(service: LanguageModelToolsService, store: any, id: string, impl: IToolImpl, data?: Partial) { @@ -3825,13 +3813,6 @@ suite('LanguageModelToolsService', () => { }); test('when hook denies, tool returns error and creates cancelled invocation', async () => { - mockHooksService.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'deny', - permissionDecisionReason: 'Destructive operations require approval', - }; - const tool = registerToolForTest(hookService, store, 'hookDenyTool', { invoke: async () => ({ content: [{ kind: 'text', value: 'should not run' }] }) }); @@ -3839,8 +3820,14 @@ suite('LanguageModelToolsService', () => { const capture: { invocation?: ChatToolInvocation } = {}; stubGetSession(hookChatService, 'hook-test', { requestId: 'req1', capture }); + const dto = tool.makeDto({ test: 1 }, { sessionId: 'hook-test' }); + dto.preToolUseResult = { + permissionDecision: 'deny', + permissionDecisionReason: 'Destructive operations require approval', + }; + const result = await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'hook-test' }), + dto, async () => 0, CancellationToken.None ); @@ -3863,12 +3850,6 @@ suite('LanguageModelToolsService', () => { }); test('when hook allows, tool executes normally', async () => { - mockHooksService.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'allow', - }; - const tool = registerToolForTest(hookService, store, 'hookAllowTool', { invoke: async () => ({ content: [{ kind: 'text', value: 'success' }] }) }); @@ -3876,8 +3857,13 @@ suite('LanguageModelToolsService', () => { const capture: { invocation?: ChatToolInvocation } = {}; stubGetSession(hookChatService, 'hook-test-allow', { requestId: 'req1', capture }); + const dto = tool.makeDto({ test: 1 }, { sessionId: 'hook-test-allow' }); + dto.preToolUseResult = { + permissionDecision: 'allow', + }; + const result = await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'hook-test-allow' }), + dto, async () => 0, CancellationToken.None ); @@ -3888,8 +3874,6 @@ suite('LanguageModelToolsService', () => { }); test('when hook returns undefined, tool executes normally', async () => { - mockHooksService.preToolUseHookResult = undefined; - const tool = registerToolForTest(hookService, store, 'hookUndefinedTool', { invoke: async () => ({ content: [{ kind: 'text', value: 'success' }] }) }); @@ -3906,38 +3890,7 @@ suite('LanguageModelToolsService', () => { assert.strictEqual((result.content[0] as IToolResultTextPart).value, 'success'); }); - test('hook receives correct input parameters', async () => { - mockHooksService.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'allow', - }; - - const tool = registerToolForTest(hookService, store, 'hookInputTool', { - invoke: async () => ({ content: [{ kind: 'text', value: 'success' }] }) - }); - - stubGetSession(hookChatService, 'hook-test-input', { requestId: 'req1' }); - - await hookService.invokeTool( - tool.makeDto({ param1: 'value1', param2: 42 }, { sessionId: 'hook-test-input' }), - async () => 0, - CancellationToken.None - ); - - assert.ok(mockHooksService.lastPreToolUseInput); - assert.strictEqual(mockHooksService.lastPreToolUseInput.toolName, 'hookInputTool'); - assert.deepStrictEqual(mockHooksService.lastPreToolUseInput.toolInput, { param1: 'value1', param2: 42 }); - }); - test('when hook denies, tool invoke is never called', async () => { - mockHooksService.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'deny', - permissionDecisionReason: 'Operation not allowed', - }; - let invokeCalled = false; const tool = registerToolForTest(hookService, store, 'hookNeverInvokeTool', { invoke: async () => { @@ -3949,8 +3902,14 @@ suite('LanguageModelToolsService', () => { const capture: { invocation?: unknown } = {}; stubGetSession(hookChatService, 'hook-test-no-invoke', { requestId: 'req1', capture }); + const dto = tool.makeDto({ test: 1 }, { sessionId: 'hook-test-no-invoke' }); + dto.preToolUseResult = { + permissionDecision: 'deny', + permissionDecisionReason: 'Operation not allowed', + }; + await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'hook-test-no-invoke' }), + dto, async () => 0, CancellationToken.None ); @@ -3959,13 +3918,6 @@ suite('LanguageModelToolsService', () => { }); test('when hook returns ask, tool is not auto-approved', async () => { - mockHooksService.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'ask', - permissionDecisionReason: 'Requires user confirmation', - }; - let invokeCompleted = false; const tool = registerToolForTest(hookService, store, 'hookAskTool', { invoke: async () => { @@ -3984,9 +3936,15 @@ suite('LanguageModelToolsService', () => { const capture: { invocation?: ChatToolInvocation } = {}; stubGetSession(hookChatService, 'hook-test-ask', { requestId: 'req1', capture }); + const dto = tool.makeDto({ test: 1 }, { sessionId: 'hook-test-ask' }); + dto.preToolUseResult = { + permissionDecision: 'ask', + permissionDecisionReason: 'Requires user confirmation', + }; + // Start invocation - it should wait for confirmation const invokePromise = hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'hook-test-ask' }), + dto, async () => 0, CancellationToken.None ); @@ -4007,12 +3965,6 @@ suite('LanguageModelToolsService', () => { }); test('when hook returns allow, tool is auto-approved', async () => { - mockHooksService.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'allow', - }; - let invokeCompleted = false; const tool = registerToolForTest(hookService, store, 'hookAutoApproveTool', { invoke: async () => { @@ -4031,9 +3983,14 @@ suite('LanguageModelToolsService', () => { const capture: { invocation?: ChatToolInvocation } = {}; stubGetSession(hookChatService, 'hook-test-auto-approve', { requestId: 'req1', capture }); + const dto = tool.makeDto({ test: 1 }, { sessionId: 'hook-test-auto-approve' }); + dto.preToolUseResult = { + permissionDecision: 'allow', + }; + // Invoke the tool - it should auto-approve due to hook const result = await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'hook-test-auto-approve' }), + dto, async () => 0, CancellationToken.None ); @@ -4046,12 +4003,6 @@ suite('LanguageModelToolsService', () => { test('when hook returns updatedInput, tool is invoked with replaced parameters', async () => { let receivedParameters: Record | undefined; - mockHooksService.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'allow', - updatedInput: { safeCommand: 'echo hello' }, - }; const tool = registerToolForTest(hookService, store, 'hookUpdatedInputTool', { invoke: async (dto) => { @@ -4069,8 +4020,14 @@ suite('LanguageModelToolsService', () => { stubGetSession(hookChatService, 'hook-test-updated-input', { requestId: 'req1' }); + const dto = tool.makeDto({ originalCommand: 'rm -rf /' }, { sessionId: 'hook-test-updated-input' }); + dto.preToolUseResult = { + permissionDecision: 'allow', + updatedInput: { safeCommand: 'echo hello' }, + }; + await hookService.invokeTool( - tool.makeDto({ originalCommand: 'rm -rf /' }, { sessionId: 'hook-test-updated-input' }), + dto, async () => 0, CancellationToken.None ); @@ -4095,12 +4052,6 @@ suite('LanguageModelToolsService', () => { }); let receivedParameters: Record | undefined; - mockHooks.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'allow', - updatedInput: { invalidField: 'wrong' }, - }; const tool = registerToolForTest(setup.service, store, 'hookValidationFailTool', { invoke: async (dto) => { @@ -4124,8 +4075,14 @@ suite('LanguageModelToolsService', () => { stubGetSession(setup.chatService, 'hook-test-validation-fail', { requestId: 'req1' }); + const dto = tool.makeDto({ command: 'original' }, { sessionId: 'hook-test-validation-fail' }); + dto.preToolUseResult = { + permissionDecision: 'allow', + updatedInput: { invalidField: 'wrong' }, + }; + await setup.service.invokeTool( - tool.makeDto({ command: 'original' }, { sessionId: 'hook-test-validation-fail' }), + dto, async () => 0, CancellationToken.None ); @@ -4151,12 +4108,6 @@ suite('LanguageModelToolsService', () => { }); let receivedParameters: Record | undefined; - mockHooks.preToolUseHookResult = { - output: undefined, - resultKind: 'success', - permissionDecision: 'allow', - updatedInput: { command: 'safe-command' }, - }; const tool = registerToolForTest(setup.service, store, 'hookValidationPassTool', { invoke: async (dto) => { @@ -4180,8 +4131,14 @@ suite('LanguageModelToolsService', () => { stubGetSession(setup.chatService, 'hook-test-validation-pass', { requestId: 'req1' }); + const dto = tool.makeDto({ command: 'original' }, { sessionId: 'hook-test-validation-pass' }); + dto.preToolUseResult = { + permissionDecision: 'allow', + updatedInput: { command: 'safe-command' }, + }; + await setup.service.invokeTool( - tool.makeDto({ command: 'original' }, { sessionId: 'hook-test-validation-pass' }), + dto, async () => 0, CancellationToken.None ); @@ -4190,154 +4147,4 @@ suite('LanguageModelToolsService', () => { assert.deepStrictEqual(receivedParameters, { command: 'safe-command' }); }); }); - - suite('postToolUse hooks', () => { - let mockHooksService: MockHooksExecutionService; - let hookService: LanguageModelToolsService; - let hookChatService: MockChatService; - - setup(() => { - mockHooksService = new MockHooksExecutionService(); - const setup = createTestToolsService(store, { - hooksExecutionService: mockHooksService - }); - hookService = setup.service; - hookChatService = setup.chatService; - }); - - test('when hook blocks, block context is appended to tool result', async () => { - mockHooksService.postToolUseHookResult = { - output: undefined, - resultKind: 'success', - decision: 'block', - reason: 'Lint errors detected', - }; - - const tool = registerToolForTest(hookService, store, 'postHookBlockTool', { - invoke: async () => ({ content: [{ kind: 'text', value: 'original output' }] }) - }); - - stubGetSession(hookChatService, 'post-hook-block', { requestId: 'req1' }); - - const result = await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'post-hook-block' }), - async () => 0, - CancellationToken.None - ); - - // Original content should still be present - assert.strictEqual(result.content[0].kind, 'text'); - assert.strictEqual((result.content[0] as IToolResultTextPart).value, 'original output'); - - // Block context should be appended wrapped in XML tags - assert.ok(result.content.length >= 2, 'Block context should be appended'); - const blockPart = result.content[1] as IToolResultTextPart; - assert.strictEqual(blockPart.kind, 'text'); - assert.ok(blockPart.value.includes(''), 'Block text should have opening tag'); - assert.ok(blockPart.value.includes(''), 'Block text should have closing tag'); - assert.ok(blockPart.value.includes('Lint errors detected'), 'Block text should include the reason'); - - // Should NOT set toolResultError - assert.strictEqual(result.toolResultError, undefined); - }); - - test('when hook returns additionalContext, it is appended to tool result', async () => { - mockHooksService.postToolUseHookResult = { - output: undefined, - resultKind: 'success', - additionalContext: ['Consider running tests after this change'], - }; - - const tool = registerToolForTest(hookService, store, 'postHookContextTool', { - invoke: async () => ({ content: [{ kind: 'text', value: 'original output' }] }) - }); - - stubGetSession(hookChatService, 'post-hook-context', { requestId: 'req1' }); - - const result = await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'post-hook-context' }), - async () => 0, - CancellationToken.None - ); - - assert.strictEqual(result.content[0].kind, 'text'); - assert.strictEqual((result.content[0] as IToolResultTextPart).value, 'original output'); - - assert.ok(result.content.length >= 2, 'Additional context should be appended'); - const contextPart = result.content[1] as IToolResultTextPart; - assert.strictEqual(contextPart.kind, 'text'); - assert.ok(contextPart.value.includes(''), 'Context text should have opening tag'); - assert.ok(contextPart.value.includes(''), 'Context text should have closing tag'); - assert.ok(contextPart.value.includes('Consider running tests after this change')); - }); - - test('when hook returns undefined, tool result is unchanged', async () => { - mockHooksService.postToolUseHookResult = undefined; - - const tool = registerToolForTest(hookService, store, 'postHookNoopTool', { - invoke: async () => ({ content: [{ kind: 'text', value: 'original output' }] }) - }); - - stubGetSession(hookChatService, 'post-hook-noop', { requestId: 'req1' }); - - const result = await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'post-hook-noop' }), - async () => 0, - CancellationToken.None - ); - - assert.strictEqual(result.content.length, 1); - assert.strictEqual(result.content[0].kind, 'text'); - assert.strictEqual((result.content[0] as IToolResultTextPart).value, 'original output'); - }); - - test('hook receives correct input including tool response text', async () => { - mockHooksService.postToolUseHookResult = undefined; - - const tool = registerToolForTest(hookService, store, 'postHookInputTool', { - invoke: async () => ({ content: [{ kind: 'text', value: 'file contents here' }] }) - }); - - stubGetSession(hookChatService, 'post-hook-input', { requestId: 'req1' }); - - await hookService.invokeTool( - tool.makeDto({ param1: 'value1' }, { sessionId: 'post-hook-input' }), - async () => 0, - CancellationToken.None - ); - - assert.ok(mockHooksService.lastPostToolUseInput); - assert.strictEqual(mockHooksService.lastPostToolUseInput.toolName, 'postHookInputTool'); - assert.deepStrictEqual(mockHooksService.lastPostToolUseInput.toolInput, { param1: 'value1' }); - assert.strictEqual(typeof mockHooksService.lastPostToolUseInput.getToolResponseText, 'function'); - }); - - test('when hook blocks with both decision and additionalContext, both are appended', async () => { - mockHooksService.postToolUseHookResult = { - output: undefined, - resultKind: 'success', - decision: 'block', - reason: 'Security issue found', - additionalContext: ['Please review the file permissions'], - }; - - const tool = registerToolForTest(hookService, store, 'postHookBlockContextTool', { - invoke: async () => ({ content: [{ kind: 'text', value: 'original' }] }) - }); - - stubGetSession(hookChatService, 'post-hook-block-ctx', { requestId: 'req1' }); - - const result = await hookService.invokeTool( - tool.makeDto({ test: 1 }, { sessionId: 'post-hook-block-ctx' }), - async () => 0, - CancellationToken.None - ); - - // Original + block message + additional context = 3 parts - assert.ok(result.content.length >= 3, 'Should have original, block message, and additional context'); - assert.strictEqual((result.content[0] as IToolResultTextPart).value, 'original'); - assert.ok((result.content[1] as IToolResultTextPart).value.includes('Security issue found')); - assert.ok((result.content[2] as IToolResultTextPart).value.includes('Please review the file permissions')); - }); - }); }); diff --git a/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts b/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts index 43f2f82410d76..1493f35a29a6c 100644 --- a/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/hooksExecutionService.test.ts @@ -389,684 +389,6 @@ suite('HooksExecutionService', () => { }); }); - suite('executePreToolUseHook', () => { - test('returns allow result when hook allows', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - permissionDecision: 'allow', - permissionDecisionReason: 'Tool is safe' - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'allow'); - assert.strictEqual(result.permissionDecisionReason, 'Tool is safe'); - }); - - test('returns ask result when hook requires confirmation', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - permissionDecision: 'ask', - permissionDecisionReason: 'Requires user approval' - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'ask'); - assert.strictEqual(result.permissionDecisionReason, 'Requires user approval'); - }); - - test('deny takes priority over ask and allow', async () => { - let callCount = 0; - const proxy = createMockProxy(() => { - callCount++; - // First hook returns allow, second returns ask, third returns deny - if (callCount === 1) { - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: 'allow' } } - }; - } else if (callCount === 2) { - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: 'ask' } } - }; - } else { - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: 'deny', permissionDecisionReason: 'Blocked' } } - }; - } - }); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook1'), cmd('hook2'), cmd('hook3')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'deny'); - assert.strictEqual(result.permissionDecisionReason, 'Blocked'); - }); - - test('ask takes priority over allow', async () => { - let callCount = 0; - const proxy = createMockProxy(() => { - callCount++; - // First hook returns allow, second returns ask - if (callCount === 1) { - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: 'allow' } } - }; - } else { - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: 'ask', permissionDecisionReason: 'Need confirmation' } } - }; - } - }); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook1'), cmd('hook2')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'ask'); - assert.strictEqual(result.permissionDecisionReason, 'Need confirmation'); - }); - - test('ignores results with wrong hookEventName', async () => { - let callCount = 0; - const proxy = createMockProxy(() => { - callCount++; - if (callCount === 1) { - // First hook returns allow but with wrong hookEventName - return { - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - hookEventName: 'PostToolUse', // Wrong hook type - permissionDecision: 'deny' - } - } - }; - } else { - // Second hook returns allow with correct hookEventName - return { - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - hookEventName: 'PreToolUse', - permissionDecision: 'allow' - } - } - }; - } - }); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook1'), cmd('hook2')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - // The deny with wrong hookEventName should be ignored - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'allow'); - }); - - test('allows results without hookEventName (optional field)', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - // No hookEventName - should be accepted - permissionDecision: 'allow' - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'allow'); - }); - - test('returns updatedInput when hook provides it', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - permissionDecision: 'allow', - updatedInput: { path: '/safe/path.ts' } - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: { path: '/original/path.ts' }, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'allow'); - assert.deepStrictEqual(result.updatedInput, { path: '/safe/path.ts' }); - }); - - test('later hook updatedInput overrides earlier one', async () => { - let callCount = 0; - const proxy = createMockProxy(() => { - callCount++; - if (callCount === 1) { - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: 'allow', updatedInput: { value: 'first' } } } - }; - } - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: 'allow', updatedInput: { value: 'second' } } } - }; - }); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook1'), cmd('hook2')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.deepStrictEqual(result.updatedInput, { value: 'second' }); - }); - - test('returns result with updatedInput even without permission decision', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - updatedInput: { modified: true } - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.deepStrictEqual(result.updatedInput, { modified: true }); - assert.strictEqual(result.permissionDecision, undefined); - }); - - test('updatedInput combined with ask shows modified input to user', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - permissionDecision: 'ask', - permissionDecisionReason: 'Modified input needs review', - updatedInput: { command: 'echo safe' } - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: { command: 'rm -rf /' }, toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.permissionDecision, 'ask'); - assert.strictEqual(result.permissionDecisionReason, 'Modified input needs review'); - assert.deepStrictEqual(result.updatedInput, { command: 'echo safe' }); - }); - }); - - suite('executePostToolUseHook', () => { - test('returns undefined when no hooks configured', async () => { - const proxy = createMockProxy(); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePostToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, getToolResponseText: () => 'tool output', toolCallId: 'call-1' } - ); - - assert.strictEqual(result, undefined); - }); - - test('returns block decision when hook blocks', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - decision: 'block', - reason: 'Lint errors found' - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePostToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, getToolResponseText: () => 'tool output', toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.decision, 'block'); - assert.strictEqual(result.reason, 'Lint errors found'); - }); - - test('returns additionalContext from hookSpecificOutput', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - hookEventName: 'PostToolUse', - additionalContext: 'File was modified successfully' - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePostToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, getToolResponseText: () => 'tool output', toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.deepStrictEqual(result.additionalContext, ['File was modified successfully']); - assert.strictEqual(result.decision, undefined); - }); - - test('block takes priority and collects all additionalContext', async () => { - let callCount = 0; - const proxy = createMockProxy(() => { - callCount++; - if (callCount === 1) { - return { - kind: HookCommandResultKind.Success, - result: { - decision: 'block', - reason: 'Tests failed' - } - }; - } else { - return { - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - additionalContext: 'Extra context from second hook' - } - } - }; - } - }); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('hook1'), cmd('hook2')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePostToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, getToolResponseText: () => 'tool output', toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.strictEqual(result.decision, 'block'); - assert.strictEqual(result.reason, 'Tests failed'); - assert.deepStrictEqual(result.additionalContext, ['Extra context from second hook']); - }); - - test('ignores results with wrong hookEventName', async () => { - let callCount = 0; - const proxy = createMockProxy(() => { - callCount++; - if (callCount === 1) { - return { - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - hookEventName: 'PreToolUse', - additionalContext: 'Should be ignored' - } - } - }; - } else { - return { - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - hookEventName: 'PostToolUse', - additionalContext: 'Correct context' - } - } - }; - } - }); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('hook1'), cmd('hook2')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePostToolUseHook( - sessionUri, - { toolName: 'test-tool', toolInput: {}, getToolResponseText: () => 'tool output', toolCallId: 'call-1' } - ); - - assert.ok(result); - assert.deepStrictEqual(result.additionalContext, ['Correct context']); - }); - - test('passes tool response text as string to external command', async () => { - let receivedInput: unknown; - const proxy = createMockProxy((_cmd, input) => { - receivedInput = input; - return { kind: HookCommandResultKind.Success, result: {} }; - }); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - await service.executePostToolUseHook( - sessionUri, - { toolName: 'my-tool', toolInput: { arg: 'val' }, getToolResponseText: () => 'file contents here', toolCallId: 'call-42' } - ); - - assert.ok(typeof receivedInput === 'object' && receivedInput !== null); - const input = receivedInput as Record; - assert.strictEqual(input['tool_name'], 'my-tool'); - assert.deepStrictEqual(input['tool_input'], { arg: 'val' }); - assert.strictEqual(input['tool_response'], 'file contents here'); - assert.strictEqual(input['tool_use_id'], 'call-42'); - assert.strictEqual(input['hookEventName'], HookType.PostToolUse); - }); - - test('does not call getter when no PostToolUse hooks registered', async () => { - const proxy = createMockProxy(); - service.setProxy(proxy); - - // Register hooks only for PreToolUse, not PostToolUse - const hooks = { [HookType.PreToolUse]: [cmd('hook')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - let getterCalled = false; - const result = await service.executePostToolUseHook( - sessionUri, - { - toolName: 'test-tool', - toolInput: {}, - getToolResponseText: () => { getterCalled = true; return ''; }, - toolCallId: 'call-1' - } - ); - - assert.strictEqual(result, undefined); - assert.strictEqual(getterCalled, false); - }); - }); - - suite('preToolUse smoke tests — input → output', () => { - test('single hook: allow', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - permissionDecision: 'allow', - permissionDecisionReason: 'Trusted tool', - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('lint-check')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const input = { toolName: 'readFile', toolInput: { path: '/src/index.ts' }, toolCallId: 'call-1' }; - const result = await service.executePreToolUseHook(sessionUri, input); - - assert.deepStrictEqual( - JSON.stringify({ permissionDecision: result?.permissionDecision, permissionDecisionReason: result?.permissionDecisionReason, additionalContext: result?.additionalContext }), - JSON.stringify({ permissionDecision: 'allow', permissionDecisionReason: 'Trusted tool', additionalContext: undefined }) - ); - }); - - test('single hook: deny', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - permissionDecision: 'deny', - permissionDecisionReason: 'Path is outside workspace', - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('path-guard')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const input = { toolName: 'writeFile', toolInput: { path: '/etc/passwd' }, toolCallId: 'call-2' }; - const result = await service.executePreToolUseHook(sessionUri, input); - - assert.deepStrictEqual( - JSON.stringify({ permissionDecision: result?.permissionDecision, permissionDecisionReason: result?.permissionDecisionReason }), - JSON.stringify({ permissionDecision: 'deny', permissionDecisionReason: 'Path is outside workspace' }) - ); - }); - - test('multiple hooks: deny wins over allow and ask', async () => { - // Three hooks return allow, ask, deny (in that order). - // deny must win regardless of ordering. - let callCount = 0; - const decisions = ['allow', 'ask', 'deny'] as const; - const proxy = createMockProxy(() => { - const decision = decisions[callCount++]; - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: decision, permissionDecisionReason: `hook-${callCount}` } } - }; - }); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('h1'), cmd('h2'), cmd('h3')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'runCommand', toolInput: { cmd: 'rm -rf /' }, toolCallId: 'call-3' } - ); - - assert.deepStrictEqual( - JSON.stringify({ permissionDecision: result?.permissionDecision, permissionDecisionReason: result?.permissionDecisionReason }), - JSON.stringify({ permissionDecision: 'deny', permissionDecisionReason: 'hook-3' }) - ); - }); - - test('multiple hooks: ask wins over allow', async () => { - let callCount = 0; - const decisions = ['allow', 'ask'] as const; - const proxy = createMockProxy(() => { - const decision = decisions[callCount++]; - return { - kind: HookCommandResultKind.Success, - result: { hookSpecificOutput: { permissionDecision: decision, permissionDecisionReason: `reason-${decision}` } } - }; - }); - service.setProxy(proxy); - - const hooks = { [HookType.PreToolUse]: [cmd('h1'), cmd('h2')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePreToolUseHook( - sessionUri, - { toolName: 'exec', toolInput: {}, toolCallId: 'call-4' } - ); - - assert.deepStrictEqual( - JSON.stringify({ permissionDecision: result?.permissionDecision, permissionDecisionReason: result?.permissionDecisionReason }), - JSON.stringify({ permissionDecision: 'ask', permissionDecisionReason: 'reason-ask' }) - ); - }); - }); - - suite('postToolUse smoke tests — input → output', () => { - test('single hook: block', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - decision: 'block', - reason: 'Lint errors found' - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('lint')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const input = { toolName: 'writeFile', toolInput: { path: 'foo.ts' }, getToolResponseText: () => 'wrote 42 bytes', toolCallId: 'call-5' }; - const result = await service.executePostToolUseHook(sessionUri, input); - - assert.deepStrictEqual( - JSON.stringify({ decision: result?.decision, reason: result?.reason, additionalContext: result?.additionalContext }), - JSON.stringify({ decision: 'block', reason: 'Lint errors found', additionalContext: undefined }) - ); - }); - - test('single hook: additionalContext only', async () => { - const proxy = createMockProxy(() => ({ - kind: HookCommandResultKind.Success, - result: { - hookSpecificOutput: { - additionalContext: 'Tests still pass after this edit' - } - } - })); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('test-runner')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const input = { toolName: 'editFile', toolInput: {}, getToolResponseText: () => 'ok', toolCallId: 'call-6' }; - const result = await service.executePostToolUseHook(sessionUri, input); - - assert.deepStrictEqual( - JSON.stringify({ decision: result?.decision, reason: result?.reason, additionalContext: result?.additionalContext }), - JSON.stringify({ decision: undefined, reason: undefined, additionalContext: ['Tests still pass after this edit'] }) - ); - }); - - test('multiple hooks: block wins and all hooks run', async () => { - let callCount = 0; - const proxy = createMockProxy(() => { - callCount++; - if (callCount === 1) { - return { kind: HookCommandResultKind.Success, result: { decision: 'block', reason: 'Tests failed' } }; - } - return { kind: HookCommandResultKind.Success, result: { hookSpecificOutput: { additionalContext: 'context from second hook' } } }; - }); - service.setProxy(proxy); - - const hooks = { [HookType.PostToolUse]: [cmd('test'), cmd('lint')] }; - store.add(service.registerHooks(sessionUri, hooks)); - - const result = await service.executePostToolUseHook( - sessionUri, - { toolName: 'writeFile', toolInput: {}, getToolResponseText: () => 'data', toolCallId: 'call-7' } - ); - - assert.deepStrictEqual( - JSON.stringify({ decision: result?.decision, reason: result?.reason, additionalContext: result?.additionalContext }), - JSON.stringify({ decision: 'block', reason: 'Tests failed', additionalContext: ['context from second hook'] }) - ); - }); - - test('no hooks registered → undefined (getter never called)', async () => { - const proxy = createMockProxy(); - service.setProxy(proxy); - - // Register PreToolUse only — no PostToolUse - store.add(service.registerHooks(sessionUri, { [HookType.PreToolUse]: [cmd('h')] })); - - let getterCalled = false; - const result = await service.executePostToolUseHook( - sessionUri, - { toolName: 't', toolInput: {}, getToolResponseText: () => { getterCalled = true; return ''; }, toolCallId: 'c' } - ); - - assert.strictEqual(result, undefined); - assert.strictEqual(getterCalled, false); - }); - }); - function createMockProxy(handler?: (cmd: IHookCommand, input: unknown, token: CancellationToken) => IHookCommandResult): IHooksExecutionProxy { return { runHookCommand: async (hookCommand, input, token) => { diff --git a/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts b/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts index 86dcf0337e464..ad37a1404ceea 100644 --- a/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts +++ b/src/vscode-dts/vscode.proposed.chatParticipantPrivate.d.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -// version: 12 +// version: 13 declare module 'vscode' { @@ -258,6 +258,8 @@ declare module 'vscode' { provideFileIgnored(uri: Uri, token: CancellationToken): ProviderResult; } + export type PreToolUsePermissionDecision = 'allow' | 'deny' | 'ask'; + export interface LanguageModelToolInvocationOptions { chatRequestId?: string; /** @deprecated Use {@link chatSessionResource} instead */ @@ -269,6 +271,16 @@ declare module 'vscode' { * Unique ID for the subagent invocation, used to group tool calls from the same subagent run together. */ subAgentInvocationId?: string; + /** + * Pre-tool-use hook result, if the hook was already executed by the caller. + * When provided, the tools service will skip executing its own preToolUse hook + * and use this result for permission decisions and input modifications instead. + */ + preToolUseResult?: { + permissionDecision?: PreToolUsePermissionDecision; + permissionDecisionReason?: string; + updatedInput?: object; + }; } export interface LanguageModelToolInvocationPrepareOptions {