diff --git a/.github/CODENOTIFY b/.github/CODENOTIFY index 9b07c27526ff2..7aba51a470b27 100644 --- a/.github/CODENOTIFY +++ b/.github/CODENOTIFY @@ -65,7 +65,6 @@ src/vs/code/** @bpasero @deepak1556 src/vs/workbench/services/activity/** @bpasero src/vs/workbench/services/authentication/** @TylerLeonhardt src/vs/workbench/services/auxiliaryWindow/** @bpasero -src/vs/workbench/services/chat/** @bpasero src/vs/workbench/services/contextmenu/** @bpasero src/vs/workbench/services/dialogs/** @alexr00 @bpasero src/vs/workbench/services/editor/** @bpasero @@ -100,15 +99,6 @@ src/vs/workbench/electron-browser/** @bpasero src/vs/workbench/contrib/authentication/** @TylerLeonhardt src/vs/workbench/contrib/files/** @bpasero src/vs/workbench/contrib/chat/browser/chatListRenderer.ts @roblourens -src/vs/workbench/contrib/chat/browser/chatSetup/** @bpasero -src/vs/workbench/contrib/chat/browser/chatStatus/** @bpasero -src/vs/workbench/contrib/chat/browser/chatViewPane.ts @bpasero -src/vs/workbench/contrib/chat/browser/media/chatViewPane.css @bpasero -src/vs/workbench/contrib/chat/browser/chatViewTitleControl.ts @bpasero -src/vs/workbench/contrib/chat/browser/media/chatViewTitleControl.css @bpasero -src/vs/workbench/contrib/chat/browser/chatManagement/chatUsageWidget.ts @bpasero -src/vs/workbench/contrib/chat/browser/chatManagement/media/chatUsageWidget.css @bpasero -src/vs/workbench/contrib/chat/browser/agentSessions/** @bpasero src/vs/workbench/contrib/localization/** @TylerLeonhardt src/vs/workbench/contrib/quickaccess/browser/commandsQuickAccess.ts @TylerLeonhardt src/vs/workbench/contrib/scm/** @lszomoru diff --git a/build/azure-pipelines/product-build.yml b/build/azure-pipelines/product-build.yml index 897c27e680dbb..b440fe34058ce 100644 --- a/build/azure-pipelines/product-build.yml +++ b/build/azure-pipelines/product-build.yml @@ -158,10 +158,6 @@ variables: name: "$(Date:yyyyMMdd).$(Rev:r) (${{ parameters.VSCODE_QUALITY }})" resources: - pipelines: - - pipeline: vscode-7pm-kick-off - source: 'VS Code 7PM Kick-Off' - trigger: true repositories: - repository: 1esPipelines type: git diff --git a/extensions/json/package.json b/extensions/json/package.json index 73265dc5f238b..1bc6fa85e53cf 100644 --- a/extensions/json/package.json +++ b/extensions/json/package.json @@ -70,6 +70,9 @@ ".ember-cli", "typedoc.json" ], + "filenamePatterns": [ + "**/.github/hooks/*.json" + ], "configuration": "./language-configuration.json" }, { diff --git a/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts b/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts index e4eb46e3aab98..bb72913ae98e3 100644 --- a/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts +++ b/src/vs/workbench/contrib/chat/browser/actions/chatCustomizationDiagnosticsAction.ts @@ -325,8 +325,13 @@ async function collectHooksStatus( const discoveryInfo = await promptsService.getPromptDiscoveryInfo(type, token); const files = discoveryInfo.files.map(convertDiscoveryResultToFileStatus); + // Collect URIs of files skipped due to disableAllHooks so we can show their hidden hooks + const disabledFileUris = discoveryInfo.files + .filter(f => f.status === 'skipped' && f.skipReason === 'all-hooks-disabled') + .map(f => f.uri); + // Parse hook files to extract individual hooks grouped by lifecycle - const parsedHooks = await parseHookFiles(promptsService, fileService, labelService, pathService, workspaceContextService, remoteAgentService, token); + const parsedHooks = await parseHookFiles(promptsService, fileService, labelService, pathService, workspaceContextService, remoteAgentService, token, disabledFileUris); return { type, paths, files, enabled, parsedHooks }; } @@ -341,7 +346,8 @@ async function parseHookFiles( pathService: IPathService, workspaceContextService: IWorkspaceContextService, remoteAgentService: IRemoteAgentService, - token: CancellationToken + token: CancellationToken, + additionalDisabledFileUris?: URI[] ): Promise { // Get workspace root and user home for path resolution const workspaceFolder = workspaceContextService.getWorkspace().folders[0]; @@ -354,7 +360,7 @@ async function parseHookFiles( const targetOS = remoteEnv?.os ?? OS; // Use the shared helper - return parseAllHookFiles(promptsService, fileService, labelService, workspaceRootUri, userHome, targetOS, token); + return parseAllHookFiles(promptsService, fileService, labelService, workspaceRootUri, userHome, targetOS, token, { additionalDisabledFileUris }); } /** @@ -442,6 +448,8 @@ function getSkipReasonMessage(skipReason: PromptFileSkipReason | undefined, erro return errorMessage ?? nls.localize('status.parseError', 'Parse error'); case 'disabled': return nls.localize('status.typeDisabled', 'Disabled'); + case 'all-hooks-disabled': + return nls.localize('status.allHooksDisabled', 'All hooks disabled via disableAllHooks'); default: return errorMessage ?? nls.localize('status.unknownError', 'Unknown error'); } @@ -735,16 +743,22 @@ export function formatStatusOutput( const fileHooks = hooksByFile.get(fileKey)!; const firstHook = fileHooks[0]; const filePath = getRelativePath(firstHook.fileUri, workspaceFolders); + const fileDisabled = fileHooks[0].disabled; - // File as clickable link - lines.push(`[${firstHook.filePath}](${filePath})
`); + // File as clickable link, with note if hooks are disabled via flag + if (fileDisabled) { + lines.push(`[${firstHook.filePath}](${filePath}) - *${nls.localize('status.allHooksDisabledLabel', 'all hooks disabled via disableAllHooks')}*
`); + } else { + lines.push(`[${firstHook.filePath}](${filePath})
`); + } // Flatten hooks with their lifecycle label for (let i = 0; i < fileHooks.length; i++) { const hook = fileHooks[i]; const isLast = i === fileHooks.length - 1; const prefix = isLast ? TREE_END : TREE_BRANCH; - lines.push(`${prefix} ${hook.hookTypeLabel}: \`${hook.commandLabel}\`
`); + const disabledPrefix = hook.disabled ? `${ICON_ERROR} ` : ''; + lines.push(`${prefix} ${disabledPrefix}${hook.hookTypeLabel}: \`${hook.commandLabel}\`
`); } } hasContent = true; diff --git a/src/vs/workbench/contrib/chat/browser/promptSyntax/hookUtils.ts b/src/vs/workbench/contrib/chat/browser/promptSyntax/hookUtils.ts index 87acdace94508..e6dd6668f35ef 100644 --- a/src/vs/workbench/contrib/chat/browser/promptSyntax/hookUtils.ts +++ b/src/vs/workbench/contrib/chat/browser/promptSyntax/hookUtils.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { findNodeAtLocation, Node, parseTree } from '../../../../../base/common/json.js'; +import { findNodeAtLocation, Node, parse as parseJSONC, parseTree } from '../../../../../base/common/json.js'; import { ITextEditorSelection } from '../../../../../platform/editor/common/editor.js'; import { URI } from '../../../../../base/common/uri.js'; import { IPromptsService } from '../../common/promptSyntax/service/promptsService.js'; @@ -11,7 +11,7 @@ import { PromptsType } from '../../common/promptSyntax/promptTypes.js'; import { IFileService } from '../../../../../platform/files/common/files.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { formatHookCommandLabel, HOOK_TYPES, HookType, IHookCommand } from '../../common/promptSyntax/hookSchema.js'; -import { parseHooksFromFile } from '../../common/promptSyntax/hookCompatibility.js'; +import { parseHooksFromFile, parseHooksIgnoringDisableAll } from '../../common/promptSyntax/hookCompatibility.js'; import * as nls from '../../../../../nls.js'; import { ILabelService } from '../../../../../platform/label/common/label.js'; import { OperatingSystem } from '../../../../../base/common/platform.js'; @@ -126,6 +126,13 @@ export interface IParsedHook { index: number; /** The original hook type ID as it appears in the JSON file */ originalHookTypeId: string; + /** If true, this hook is disabled via `disableAllHooks: true` in its file */ + disabled?: boolean; +} + +export interface IParseAllHookFilesOptions { + /** Additional file URIs to parse (e.g., files skipped due to disableAllHooks) */ + additionalDisabledFileUris?: readonly URI[]; } /** @@ -139,7 +146,8 @@ export async function parseAllHookFiles( workspaceRootUri: URI | undefined, userHome: string, os: OperatingSystem, - token: CancellationToken + token: CancellationToken, + options?: IParseAllHookFilesOptions ): Promise { const hookFiles = await promptsService.listPromptFiles(PromptsType.hook, token); const parsedHooks: IParsedHook[] = []; @@ -147,7 +155,7 @@ export async function parseAllHookFiles( for (const hookFile of hookFiles) { try { const content = await fileService.readFile(hookFile.uri); - const json = JSON.parse(content.value.toString()); + const json = parseJSONC(content.value.toString()); // Use format-aware parsing const { hooks } = parseHooksFromFile(hookFile.uri, json, workspaceRootUri, userHome); @@ -179,5 +187,44 @@ export async function parseAllHookFiles( } } + // Parse additional disabled files (e.g., files with disableAllHooks: true) + // These are parsed ignoring the disableAllHooks flag so we can show their hooks as disabled + if (options?.additionalDisabledFileUris) { + for (const uri of options.additionalDisabledFileUris) { + try { + const content = await fileService.readFile(uri); + const json = parseJSONC(content.value.toString()); + + // Parse hooks ignoring disableAllHooks - use the underlying format parsers directly + const { hooks } = parseHooksIgnoringDisableAll(uri, json, workspaceRootUri, userHome); + + for (const [hookType, { hooks: commands, originalId }] of hooks) { + const hookTypeMeta = HOOK_TYPES.find(h => h.id === hookType); + if (!hookTypeMeta) { + continue; + } + + for (let i = 0; i < commands.length; i++) { + const command = commands[i]; + const commandLabel = formatHookCommandLabel(command, os) || nls.localize('commands.hook.emptyCommand', '(empty command)'); + parsedHooks.push({ + hookType, + hookTypeLabel: hookTypeMeta.label, + command, + commandLabel, + fileUri: uri, + filePath: labelService.getUriLabel(uri, { relative: true }), + index: i, + originalHookTypeId: originalId, + disabled: true + }); + } + } + } catch (error) { + console.error('Failed to read or parse disabled hook file', uri.toString(), error); + } + } + } + return parsedHooks; } diff --git a/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts b/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts index 4bc871cae8938..80612e6daf317 100644 --- a/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts +++ b/src/vs/workbench/contrib/chat/browser/tools/languageModelToolsService.ts @@ -47,7 +47,7 @@ import { ChatToolInvocation } from '../../common/model/chatProgressTypes/chatToo 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 { CountTokensCallback, createToolSchemaUri, IBeginToolCallOptions, IExternalPreToolUseHookResult, ILanguageModelToolsService, IPreparedToolInvocation, isToolSet, IToolAndToolSetEnablementMap, IToolData, IToolImpl, IToolInvocation, IToolInvokedEvent, IToolResult, IToolResultInputOutputDetails, IToolSet, SpecedToolAliases, stringifyPromptTsxPart, ToolDataSource, ToolInvocationPresentation, toolMatchesModel, ToolSet, ToolSetForModel, VSCodeToolReference } from '../../common/tools/languageModelToolsService.js'; import { getToolConfirmationAlert } from '../accessibility/chatAccessibilityProvider.js'; const jsonSchemaRegistry = Registry.as(JSONContributionRegistry.Extensions.JSONContribution); @@ -209,6 +209,13 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo if (agentModeEnabled !== false) { return true; } + + // Internal tools that explicitly cannot be referenced in prompts are always permitted + // since they are infrastructure tools (e.g. inline_chat_exit), not user-facing agent tools + if (!isToolSet(toolOrToolSet) && toolOrToolSet.canBeReferencedInPrompt === false && toolOrToolSet.source.type === 'internal') { + return true; + } + const permittedInternalToolSetIds = [SpecedToolAliases.read, SpecedToolAliases.search, SpecedToolAliases.web]; if (isToolSet(toolOrToolSet)) { const permitted = toolOrToolSet.source.type === 'internal' && permittedInternalToolSetIds.includes(toolOrToolSet.referenceName); @@ -377,6 +384,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo if (toolData) { if (pendingInvocation) { + pendingInvocation.presentation = ToolInvocationPresentation.Hidden; pendingInvocation.cancelFromStreaming(ToolConfirmKind.Denied, reason); } else if (request) { const cancelledInvocation = ChatToolInvocation.createCancelled( @@ -385,6 +393,7 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo ToolConfirmKind.Denied, reason ); + cancelledInvocation.presentation = ToolInvocationPresentation.Hidden; this._chatService.appendProgress(request, cancelledInvocation); } } @@ -726,17 +735,49 @@ export class LanguageModelToolsService extends Disposable implements ILanguageMo } const fullReferenceName = getToolFullReferenceName(tool.data); const hookReason = hookResult.permissionDecisionReason; - const baseMessage = localize('hookRequiresConfirmation.message', "{0} hook confirmation required", HookType.PreToolUse); + const hookNote = hookReason + ? localize('hookRequiresConfirmation.messageWithReason', "{0} hook required confirmation: {1}", HookType.PreToolUse, hookReason) + : localize('hookRequiresConfirmation.message', "{0} hook required confirmation", HookType.PreToolUse); preparedInvocation.confirmationMessages = { ...preparedInvocation.confirmationMessages, title: localize('hookRequiresConfirmation.title', "Use the '{0}' tool?", fullReferenceName), - message: new MarkdownString(hookReason ? `${baseMessage}\n\n${hookReason}` : baseMessage), + message: new MarkdownString(`_${hookNote}_`), allowAutoConfirm: false, }; preparedInvocation.toolSpecificData = { kind: 'input', rawInput: dto.parameters, }; + } else { + // Tool already has its own confirmation - prepend hook note + const hookReason = hookResult.permissionDecisionReason; + const hookNote = hookReason + ? localize('hookRequiresConfirmation.note', "{0} hook required confirmation: {1}", HookType.PreToolUse, hookReason) + : localize('hookRequiresConfirmation.noteNoReason', "{0} hook required confirmation", HookType.PreToolUse); + + const existing = preparedInvocation.confirmationMessages!; + if (preparedInvocation.toolSpecificData?.kind === 'terminal') { + // Terminal tools render message as hover only; use disclaimer for visible text + const existingDisclaimerText = existing.disclaimer + ? (typeof existing.disclaimer === 'string' ? existing.disclaimer : existing.disclaimer.value) + : undefined; + const combinedDisclaimer = existingDisclaimerText + ? `${hookNote}\n\n${existingDisclaimerText}` + : hookNote; + preparedInvocation.confirmationMessages = { + ...existing, + disclaimer: combinedDisclaimer, + allowAutoConfirm: false, + }; + } else { + // Edit/other tools: prepend hook note to the message body + const msgText = typeof existing.message === 'string' ? existing.message : existing.message?.value ?? ''; + preparedInvocation.confirmationMessages = { + ...existing, + message: new MarkdownString(`_${hookNote}_\n\n${msgText}`), + allowAutoConfirm: false, + }; + } } return { autoConfirmed: undefined, preparedInvocation }; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatHookContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatHookContentPart.ts index 464f07913836b..a9f8451881a08 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatHookContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatHookContentPart.ts @@ -9,7 +9,7 @@ import { localize } from '../../../../../../nls.js'; import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; import { IChatHookPart } from '../../../common/chatService/chatService.js'; import { IChatRendererContent } from '../../../common/model/chatViewModel.js'; -import { HOOK_TYPES, HookTypeValue } from '../../../common/promptSyntax/hookSchema.js'; +import { HookType, HOOK_TYPES, HookTypeValue } from '../../../common/promptSyntax/hookSchema.js'; import { ChatTreeItem } from '../../chat.js'; import { ChatCollapsibleContentPart } from './chatCollapsibleContentPart.js'; import { IChatContentPart, IChatContentPartRenderContext } from './chatContentParts.js'; @@ -29,16 +29,23 @@ export class ChatHookContentPart extends ChatCollapsibleContentPart implements I const hookTypeLabel = getHookTypeLabel(hookPart.hookType); const isStopped = !!hookPart.stopReason; const isWarning = !!hookPart.systemMessage; + const toolName = hookPart.toolDisplayName; const title = isStopped - ? localize('hook.title.stopped', "Blocked by {0} hook", hookTypeLabel) - : localize('hook.title.warning', "Warning from {0} hook", hookTypeLabel); + ? (toolName + ? localize('hook.title.stoppedWithTool', "Blocked {0} - {1} hook", toolName, hookTypeLabel) + : localize('hook.title.stopped', "Blocked by {0} hook", hookTypeLabel)) + : (toolName + ? localize('hook.title.warningWithTool', "Warning for {0} - {1} hook", toolName, hookTypeLabel) + : localize('hook.title.warning', "Warning from {0} hook", hookTypeLabel)); super(title, context, undefined, hoverService); - this.icon = isStopped ? Codicon.circleSlash : isWarning ? Codicon.warning : Codicon.check; + this.icon = isStopped ? Codicon.error : isWarning ? Codicon.warning : Codicon.check; if (isStopped) { this.domNode.classList.add('chat-hook-outcome-blocked'); + } else if (isWarning) { + this.domNode.classList.add('chat-hook-outcome-warning'); } this.setExpanded(false); @@ -50,7 +57,10 @@ export class ChatHookContentPart extends ChatCollapsibleContentPart implements I if (this.hookPart.stopReason) { const reasonElement = $('.chat-hook-reason', undefined, this.hookPart.stopReason); content.appendChild(reasonElement); - } else if (this.hookPart.systemMessage) { + } + + const isToolHook = this.hookPart.hookType === HookType.PreToolUse || this.hookPart.hookType === HookType.PostToolUse; + if (this.hookPart.systemMessage && (isToolHook || !this.hookPart.stopReason)) { const messageElement = $('.chat-hook-message', undefined, this.hookPart.systemMessage); content.appendChild(messageElement); } @@ -64,6 +74,7 @@ export class ChatHookContentPart extends ChatCollapsibleContentPart implements I } return other.hookType === this.hookPart.hookType && other.stopReason === this.hookPart.stopReason && - other.systemMessage === this.hookPart.systemMessage; + other.systemMessage === this.hookPart.systemMessage && + other.toolDisplayName === this.hookPart.toolDisplayName; } } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSubagentContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSubagentContentPart.ts index 86a60cfacfd47..5e03ac8c2479b 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSubagentContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatSubagentContentPart.ts @@ -16,7 +16,7 @@ import { localize } from '../../../../../../nls.js'; import { IHoverService } from '../../../../../../platform/hover/browser/hover.js'; import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js'; import { IMarkdownRenderer } from '../../../../../../platform/markdown/browser/markdownRenderer.js'; -import { IChatMarkdownContent, IChatToolInvocation, IChatToolInvocationSerialized } from '../../../common/chatService/chatService.js'; +import { IChatHookPart, IChatMarkdownContent, IChatToolInvocation, IChatToolInvocationSerialized } from '../../../common/chatService/chatService.js'; import { IChatRendererContent } from '../../../common/model/chatViewModel.js'; import { IRunSubagentToolInputParams } from '../../../common/tools/builtinTools/runSubagentTool.js'; import { CodeBlockModelCollection } from '../../../common/widget/codeBlockModelCollection.js'; @@ -51,7 +51,16 @@ interface ILazyMarkdownItem { lazy: Lazy<{ domNode: HTMLElement; disposable?: IDisposable }>; } -type ILazyItem = ILazyToolItem | ILazyMarkdownItem; +/** + * Represents a lazy hook item (blocked/warning) that will be rendered when expanded. + */ +interface ILazyHookItem { + kind: 'hook'; + lazy: Lazy<{ domNode: HTMLElement; disposable?: IDisposable }>; + hookPart: IChatHookPart; +} + +type ILazyItem = ILazyToolItem | ILazyMarkdownItem | ILazyHookItem; /** * This is generally copied from ChatThinkingContentPart. We are still experimenting with both UIs so I'm not @@ -587,6 +596,58 @@ export class ChatSubagentContentPart extends ChatCollapsibleContentPart implemen } } + /** + * Appends a hook item (blocked/warning) to the subagent content part. + */ + public appendHookItem( + factory: () => { domNode: HTMLElement; disposable?: IDisposable }, + hookPart: IChatHookPart + ): void { + if (this.isExpanded() || this.hasExpandedOnce) { + const result = factory(); + this.appendHookItemToDOM(result.domNode, hookPart); + if (result.disposable) { + this._register(result.disposable); + } + } else { + const item: ILazyHookItem = { + kind: 'hook', + lazy: new Lazy(factory), + hookPart, + }; + this.lazyItems.push(item); + } + } + + /** + * Appends a hook item's DOM node to the wrapper. + */ + private appendHookItemToDOM(domNode: HTMLElement, hookPart: IChatHookPart): void { + const itemWrapper = $('.chat-thinking-tool-wrapper'); + const icon = hookPart.stopReason ? Codicon.error : Codicon.warning; + const iconElement = createThinkingIcon(icon); + itemWrapper.appendChild(iconElement); + itemWrapper.appendChild(domNode); + + // Treat hook items as tool items for visibility purposes + if (!this.hasToolItems) { + this.hasToolItems = true; + if (this.wrapper) { + this.wrapper.style.display = ''; + } + } + + if (this.wrapper) { + if (this.resultContainer) { + this.wrapper.insertBefore(itemWrapper, this.resultContainer); + } else { + this.wrapper.appendChild(itemWrapper); + } + } + this.lastItemWrapper = itemWrapper; + this.layoutScheduler.schedule(); + } + /** * Appends a markdown item's DOM node to the wrapper. */ @@ -705,6 +766,12 @@ export class ChatSubagentContentPart extends ChatCollapsibleContentPart implemen if (result.disposable) { this._register(result.disposable); } + } else if (item.kind === 'hook') { + const result = item.lazy.value; + this.appendHookItemToDOM(result.domNode, item.hookPart); + if (result.disposable) { + this._register(result.disposable); + } } } @@ -759,6 +826,11 @@ export class ChatSubagentContentPart extends ChatCollapsibleContentPart implemen return true; } + // Match hook parts with the same subAgentInvocationId to keep them grouped in the subagent dropdown + if (other.kind === 'hook' && other.subAgentInvocationId) { + return this.subAgentInvocationId === other.subAgentInvocationId; + } + // Match subagent tool invocations with the same subAgentInvocationId to keep them grouped if ((other.kind === 'toolInvocation' || other.kind === 'toolInvocationSerialized') && (other.subAgentInvocationId || ChatSubagentContentPart.isParentSubagentTool(other))) { // For parent subagent tool, use toolCallId as the effective ID diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts index ce9c70c41d7ae..4f926a701d4a9 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts @@ -765,6 +765,12 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen - For reasoning/thinking: "Considered", "Planned", "Analyzed", "Reviewed", "Evaluated" - Choose the synonym that best fits the context + PRIORITY RULE - BLOCKED/DENIED CONTENT: + - If any item mentions being "blocked" (e.g. "Tried to use X, but was blocked"), it MUST be reflected in the title + - Blocked content takes priority over all other tool calls + - Use natural phrasing like "Tried to , but was blocked" or "Attempted but was denied" + - If there are both blocked items AND normal tool calls, mention both: e.g. "Tried to run terminal but was blocked, edited file.ts" + RULES FOR TOOL CALLS: 1. If the SAME file was both edited AND read: Use a combined phrase like "Reviewed and updated " 2. If exactly ONE file was edited: Start with an edit synonym + "" (include actual filename) @@ -804,6 +810,12 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen - "Edited Button.tsx, Edited Button.css, Edited index.ts" → "Modified 3 files" - "Searched codebase for error handling" → "Looked up error handling" + EXAMPLES WITH BLOCKED CONTENT: + - "Tried to use Run in Terminal, but was blocked" → "Tried to run command, but was blocked" + - "Tried to use Run in Terminal, but was blocked, Edited config.ts" → "Tried to run command but was blocked, edited config.ts" + - "Tried to use Edit File, but was blocked, Tried to use Run in Terminal, but was blocked" → "Tried to use 2 tools, but was blocked" + - "Used Read File, but received a warning, Edited utils.ts" → "Read file with a warning, edited utils.ts" + EXAMPLES WITH REASONING HEADERS (no tools): - "Analyzing component architecture" → "Considered component architecture" - "Planning refactor strategy" → "Planned refactor strategy" @@ -1160,7 +1172,7 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen toolCallLabel = localize('chat.thinking.editingFile', 'Edited file'); } } else { - toolCallLabel = `Invoked \`${toolInvocationId}\``; + toolCallLabel = toolInvocationId; } // Add tool call to extracted titles for LLM title generation @@ -1207,6 +1219,10 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen const terminalData = (toolInvocationOrMarkdown as IChatToolInvocation | IChatToolInvocationSerialized).toolSpecificData as { kind: 'terminal'; terminalCommandState?: { exitCode?: number } }; const exitCode = terminalData?.terminalCommandState?.exitCode; icon = exitCode !== undefined && exitCode !== 0 ? Codicon.error : Codicon.terminal; + } else if (content.classList.contains('chat-hook-outcome-blocked')) { + icon = Codicon.error; + } else if (content.classList.contains('chat-hook-outcome-warning')) { + icon = Codicon.warning; } else { icon = toolInvocationId ? getToolInvocationIcon(toolInvocationId) : Codicon.tools; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatHookContentPart.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatHookContentPart.css index c06e49192d58c..3a30dc1e68a22 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatHookContentPart.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatHookContentPart.css @@ -7,6 +7,17 @@ color: var(--vscode-notificationsWarningIcon-foreground) !important; } +.chat-thinking-box .chat-used-context.chat-hook-outcome-blocked, +.chat-thinking-box .chat-used-context.chat-hook-outcome-warning { + padding: 4px 12px 4px 22px; + margin-bottom: 0; +} + +.chat-thinking-box .chat-used-context.chat-hook-outcome-blocked > .chat-used-context-label .codicon, +.chat-thinking-box .chat-used-context.chat-hook-outcome-warning > .chat-used-context-label .codicon { + display: none; +} + .chat-hook-details { display: flex; flex-direction: column; @@ -14,8 +25,20 @@ padding: 8px 12px; } -.chat-hook-message, .chat-hook-reason { +.chat-hook-reason { + font-size: var(--vscode-chat-font-size-body-s); + padding: 4px 10px; +} + +.chat-hook-message { font-size: var(--vscode-chat-font-size-body-s); padding: 4px 10px; color: var(--vscode-descriptionForeground); } + +/* When both reason and message are shown, add a subtle separator */ +.chat-hook-reason + .chat-hook-message { + border-top: 1px solid var(--vscode-chat-requestBorder, var(--vscode-editorWidget-border)); + margin-top: 2px; + padding-top: 6px; +} diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index 38d1242b30223..57267238df819 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -108,6 +108,7 @@ import { IChatTipService } from '../chatTipService.js'; import { IAccessibilityService } from '../../../../../platform/accessibility/common/accessibility.js'; import { ChatHookContentPart } from './chatContentParts/chatHookContentPart.js'; import { ChatPendingDragController } from './chatPendingDragAndDrop.js'; +import { HookType } from '../../common/promptSyntax/hookSchema.js'; const $ = dom.$; @@ -1226,7 +1227,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer('chat.agent.thinking.collapsedTools'); @@ -1428,6 +1430,14 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer other.kind === 'hook' && other.hookType === hookPart.hookType); } - return this.renderNoContent(other => other.kind === 'hook' && other.hookType === hookPart.hookType); + + if (hookPart.subAgentInvocationId) { + const subagentPart = this.getSubagentPart(templateData.renderedParts, hookPart.subAgentInvocationId); + if (subagentPart) { + subagentPart.appendHookItem(() => { + const part = this.instantiationService.createInstance(ChatHookContentPart, hookPart, context); + return { domNode: part.domNode, disposable: part }; + }, hookPart); + return this.renderNoContent(other => other.kind === 'hook' && other.hookType === hookPart.hookType && other.subAgentInvocationId === hookPart.subAgentInvocationId); + } + } + + // Only pin preTool/postTool hooks into the thinking part + const shouldPinToThinking = hookPart.hookType === HookType.PreToolUse || hookPart.hookType === HookType.PostToolUse; + if (shouldPinToThinking) { + const hookTitle = hookPart.stopReason + ? (hookPart.toolDisplayName + ? localize('hook.thinking.blocked', "Blocked {0}", hookPart.toolDisplayName) + : localize('hook.thinking.blockedGeneric', "Blocked by hook")) + : (hookPart.toolDisplayName + ? localize('hook.thinking.warning', "Used {0}, but received a warning", hookPart.toolDisplayName) + : localize('hook.thinking.warningGeneric', "Tool call received a warning")); + + let thinkingPart = this.getLastThinkingPart(templateData.renderedParts); + if (!thinkingPart) { + // Create a thinking part if one doesn't exist yet (e.g. hook arrives before/with its tool in the same turn) + const newThinking = this.renderThinkingPart({ kind: 'thinking' }, context, templateData); + if (newThinking instanceof ChatThinkingContentPart) { + thinkingPart = newThinking; + } + } + + if (thinkingPart) { + thinkingPart.appendItem(() => { + const part = this.instantiationService.createInstance(ChatHookContentPart, hookPart, context); + return { domNode: part.domNode, disposable: part }; + }, hookTitle, undefined, templateData.value); + return thinkingPart; + } + } + + const part = this.instantiationService.createInstance(ChatHookContentPart, hookPart, context); + return part; } private renderPullRequestContent(pullRequestContent: IChatPullRequestContent, context: IChatContentPartRenderContext, templateData: IChatListItemTemplate): IChatContentPart | undefined { diff --git a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts index 657c6d6ac1e06..47c0fc7008445 100644 --- a/src/vs/workbench/contrib/chat/common/chatService/chatService.ts +++ b/src/vs/workbench/contrib/chat/common/chatService/chatService.ts @@ -434,7 +434,11 @@ export interface IChatHookPart { stopReason?: string; /** Warning/system message from the hook, shown to the user */ systemMessage?: string; + /** Display name of the tool that was affected by the hook */ + toolDisplayName?: string; metadata?: { readonly [key: string]: unknown }; + /** If set, this hook was executed within a subagent invocation and should be grouped with it. */ + subAgentInvocationId?: string; } export interface IChatTerminalToolInvocationData { diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/hookClaudeCompat.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/hookClaudeCompat.ts index 5f2079ea40151..c159acfa4c327 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/hookClaudeCompat.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/hookClaudeCompat.ts @@ -53,6 +53,20 @@ export function getClaudeHookTypeName(hookType: HookType): string | undefined { return getHookTypeToClaudeNameMap().get(hookType); } +/** + * Result of parsing Claude hooks file. + */ +export interface IParseClaudeHooksResult { + /** + * The parsed hooks by type. + */ + readonly hooks: Map; + /** + * Whether all hooks from this file were disabled via `disableAllHooks: true`. + */ + readonly disabledAllHooks: boolean; +} + /** * Parses hooks from a Claude settings.json file. * Claude format: @@ -70,23 +84,31 @@ export function getClaudeHookTypeName(hookType: HookType): string | undefined { * "PreToolUse": [{ "type": "command", "command": "..." }] * } * } + * + * If the file has `disableAllHooks: true` at the top level, all hooks are filtered out. */ export function parseClaudeHooks( json: unknown, workspaceRootUri: URI | undefined, userHome: string -): Map { +): IParseClaudeHooksResult { const result = new Map(); if (!json || typeof json !== 'object') { - return result; + return { hooks: result, disabledAllHooks: false }; } const root = json as Record; + + // Check for disableAllHooks property at the top level + if (root.disableAllHooks === true) { + return { hooks: result, disabledAllHooks: true }; + } + const hooks = root.hooks; if (!hooks || typeof hooks !== 'object') { - return result; + return { hooks: result, disabledAllHooks: false }; } const hooksObj = hooks as Record; @@ -140,7 +162,7 @@ export function parseClaudeHooks( } } - return result; + return { hooks: result, disabledAllHooks: false }; } /** @@ -158,7 +180,5 @@ function resolveClaudeCommand( return undefined; } - // Add type if missing for resolveHookCommand - const normalized = { ...raw, type: 'command' }; - return resolveHookCommand(normalized, workspaceRootUri, userHome); + return resolveHookCommand(raw, workspaceRootUri, userHome); } diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/hookCompatibility.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/hookCompatibility.ts index 1525bbb59e8e5..6bdf4afdc8910 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/hookCompatibility.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/hookCompatibility.ts @@ -111,6 +111,18 @@ export function parseCopilotHooks( return result; } +/** + * Result of parsing hooks from a file. + */ +export interface IParseHooksFromFileResult { + readonly format: HookSourceFormat; + readonly hooks: Map; + /** + * Whether all hooks from this file were disabled via `disableAllHooks: true`. + */ + readonly disabledAllHooks: boolean; +} + /** * Parses hooks from any supported format, auto-detecting the format from the file URI. */ @@ -119,22 +131,61 @@ export function parseHooksFromFile( json: unknown, workspaceRootUri: URI | undefined, userHome: string -): { format: HookSourceFormat; hooks: Map } { +): IParseHooksFromFileResult { const format = getHookSourceFormat(fileUri); let hooks: Map; + let disabledAllHooks = false; switch (format) { - case HookSourceFormat.Claude: - hooks = parseClaudeHooks(json, workspaceRootUri, userHome); + case HookSourceFormat.Claude: { + const result = parseClaudeHooks(json, workspaceRootUri, userHome); + hooks = result.hooks; + disabledAllHooks = result.disabledAllHooks; + break; + } + case HookSourceFormat.Copilot: + default: + hooks = parseCopilotHooks(json, workspaceRootUri, userHome); + break; + } + + return { format, hooks, disabledAllHooks }; +} + +/** + * Parses hooks from a file, ignoring the `disableAllHooks` flag. + * Used by diagnostics to show which hooks are hidden when `disableAllHooks: true` is set. + */ +export function parseHooksIgnoringDisableAll( + fileUri: URI, + json: unknown, + workspaceRootUri: URI | undefined, + userHome: string +): IParseHooksFromFileResult { + const format = getHookSourceFormat(fileUri); + + let hooks: Map; + + switch (format) { + case HookSourceFormat.Claude: { + // Strip `disableAllHooks` before parsing so the hooks are still extracted + if (json && typeof json === 'object') { + const { disableAllHooks: _, ...rest } = json as Record; + const result = parseClaudeHooks(rest, workspaceRootUri, userHome); + hooks = result.hooks; + } else { + hooks = new Map(); + } break; + } case HookSourceFormat.Copilot: default: hooks = parseCopilotHooks(json, workspaceRootUri, userHome); break; } - return { format, hooks }; + return { format, hooks, disabledAllHooks: true }; } /** diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/promptTypes.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/promptTypes.ts index 4d588cc1cfd49..8c4d0cbc58a87 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/promptTypes.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/promptTypes.ts @@ -54,8 +54,8 @@ export function getLanguageIdForPromptsType(type: PromptsType): string { case PromptsType.skill: return SKILL_LANGUAGE_ID; case PromptsType.hook: - // Hooks use JSON syntax with schema validation - return 'json'; + // Hooks use JSONC syntax with schema validation + return 'jsonc'; default: throw new Error(`Unknown prompt type: ${type}`); } @@ -71,7 +71,7 @@ export function getPromptsTypeForLanguageId(languageId: string): PromptsType | u return PromptsType.agent; case SKILL_LANGUAGE_ID: return PromptsType.skill; - // Note: hook uses 'json' language ID which is shared, so we don't map it here + // Note: hook uses 'jsonc' language ID which is shared, so we don't map it here default: return undefined; } diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts index 47b3e64e180c0..f59d436d4fd2b 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsService.ts @@ -273,7 +273,8 @@ export type PromptFileSkipReason = | 'name-mismatch' | 'duplicate-name' | 'parse-error' - | 'disabled'; + | 'disabled' + | 'all-hooks-disabled'; /** * Result of discovering a single prompt file. diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts index c819699928882..7d4cb5cc35248 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/service/promptsServiceImpl.ts @@ -6,6 +6,7 @@ import { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { CancellationError } from '../../../../../../base/common/errors.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; +import { parse as parseJSONC } from '../../../../../../base/common/json.js'; import { Disposable, DisposableStore, IDisposable } from '../../../../../../base/common/lifecycle.js'; import { ResourceMap, ResourceSet } from '../../../../../../base/common/map.js'; import { basename, dirname, isEqual, joinPath } from '../../../../../../base/common/resources.js'; @@ -1030,10 +1031,16 @@ export class PromptsService extends Disposable implements IPromptsService { for (const hookFile of hookFiles) { try { const content = await this.fileService.readFile(hookFile.uri); - const json = JSON.parse(content.value.toString()); + const json = parseJSONC(content.value.toString()); - // Use format-aware parsing that handles Copilot, Claude, and Cursor formats - const { format, hooks } = parseHooksFromFile(hookFile.uri, json, workspaceRootUri, userHome); + // Use format-aware parsing that handles Copilot and Claude formats + const { format, hooks, disabledAllHooks } = parseHooksFromFile(hookFile.uri, json, workspaceRootUri, userHome); + + // Skip files that have all hooks disabled + if (disabledAllHooks) { + this.logger.trace(`[PromptsService] Skipping hook file with disableAllHooks: ${hookFile.uri}`); + continue; + } for (const [hookType, { hooks: commands }] of hooks) { for (const command of commands) { @@ -1304,6 +1311,14 @@ export class PromptsService extends Disposable implements IPromptsService { private async getHookDiscoveryInfo(token: CancellationToken): Promise { const files: IPromptFileDiscoveryResult[] = []; + // Get user home for tilde expansion + const userHomeUri = await this.pathService.userHome(); + const userHome = userHomeUri.scheme === Schemas.file ? userHomeUri.fsPath : userHomeUri.path; + + // Get workspace root for resolving relative cwd paths + const workspaceFolder = this.workspaceService.getWorkspace().folders[0]; + const workspaceRootUri = workspaceFolder?.uri; + const hookFiles = await this.listPromptFiles(PromptsType.hook, token); for (const promptPath of hookFiles) { const uri = promptPath.uri; @@ -1312,9 +1327,9 @@ export class PromptsService extends Disposable implements IPromptsService { const name = basename(uri); try { - // Try to parse the JSON to validate it + // Try to parse the JSON to validate it (supports JSONC with comments) const content = await this.fileService.readFile(uri); - const json = JSON.parse(content.value.toString()); + const json = parseJSONC(content.value.toString()); // Validate it's an object if (!json || typeof json !== 'object') { @@ -1330,6 +1345,21 @@ export class PromptsService extends Disposable implements IPromptsService { continue; } + // Use format-aware parsing to check for disabledAllHooks + const { disabledAllHooks } = parseHooksFromFile(uri, json, workspaceRootUri, userHome); + + if (disabledAllHooks) { + files.push({ + uri, + storage, + status: 'skipped', + skipReason: 'all-hooks-disabled', + name, + extensionId + }); + continue; + } + // File is valid files.push({ uri, storage, status: 'loaded', name, extensionId }); } catch (e) { diff --git a/src/vs/workbench/contrib/chat/common/tools/builtinTools/runSubagentTool.ts b/src/vs/workbench/contrib/chat/common/tools/builtinTools/runSubagentTool.ts index d1f19a0f152af..d8784ef6dd740 100644 --- a/src/vs/workbench/contrib/chat/common/tools/builtinTools/runSubagentTool.ts +++ b/src/vs/workbench/contrib/chat/common/tools/builtinTools/runSubagentTool.ts @@ -7,38 +7,39 @@ import { CancellationToken } from '../../../../../../base/common/cancellation.js import { Codicon } from '../../../../../../base/common/codicons.js'; import { Event } from '../../../../../../base/common/event.js'; import { MarkdownString } from '../../../../../../base/common/htmlContent.js'; -import { generateUuid } from '../../../../../../base/common/uuid.js'; import { IJSONSchema, IJSONSchemaMap } from '../../../../../../base/common/jsonSchema.js'; import { Disposable, DisposableStore } from '../../../../../../base/common/lifecycle.js'; import { ThemeIcon } from '../../../../../../base/common/themables.js'; +import { generateUuid } from '../../../../../../base/common/uuid.js'; import { localize } from '../../../../../../nls.js'; import { IConfigurationChangeEvent, IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { IInstantiationService } from '../../../../../../platform/instantiation/common/instantiation.js'; import { ILogService } from '../../../../../../platform/log/common/log.js'; -import { IChatAgentRequest, IChatAgentService } from '../../participants/chatAgents.js'; -import { ChatModel, IChatRequestModeInstructions } from '../../model/chatModel.js'; -import { IChatProgress, IChatService } from '../../chatService/chatService.js'; import { ChatRequestVariableSet } from '../../attachments/chatVariableEntries.js'; +import { IChatProgress, IChatService } from '../../chatService/chatService.js'; import { ChatAgentLocation, ChatConfiguration, ChatModeKind } from '../../constants.js'; import { ILanguageModelsService } from '../../languageModels.js'; +import { ChatModel, IChatRequestModeInstructions } from '../../model/chatModel.js'; +import { IChatAgentRequest, IChatAgentService } from '../../participants/chatAgents.js'; +import { ComputeAutomaticInstructions } from '../../promptSyntax/computeAutomaticInstructions.js'; +import { IChatRequestHooks } from '../../promptSyntax/hookSchema.js'; +import { ICustomAgent, IPromptsService } from '../../promptSyntax/service/promptsService.js'; import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, + isToolSet, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, - isToolSet, ToolDataSource, ToolProgress, VSCodeToolReference, } from '../languageModelToolsService.js'; -import { ComputeAutomaticInstructions } from '../../promptSyntax/computeAutomaticInstructions.js'; import { ManageTodoListToolToolId } from './manageTodoListTool.js'; import { createToolSimpleTextResult } from './toolHelpers.js'; -import { ICustomAgent, IPromptsService } from '../../promptSyntax/service/promptsService.js'; const BaseModelDescription = `Launch a new agent to handle complex, multi-step tasks autonomously. This tool is good at researching complex questions, searching for code, and executing multi-step tasks. When you are searching for a keyword or file and are not confident that you will find the right match in the first few tries, use this agent to perform the search for you. @@ -222,6 +223,8 @@ export class RunSubagentTool extends Disposable implements IToolImpl { } else { model.acceptResponseProgress(request, part); } + } else if (part.kind === 'hook') { + model.acceptResponseProgress(request, { ...part, subAgentInvocationId }); } else if (part.kind === 'markdownContent') { if (inEdit) { model.acceptResponseProgress(request, { kind: 'markdownContent', content: new MarkdownString('\n```\n\n') }); @@ -244,6 +247,14 @@ export class RunSubagentTool extends Disposable implements IToolImpl { const computer = this.instantiationService.createInstance(ComputeAutomaticInstructions, ChatModeKind.Agent, modeTools, undefined); // agents can not call subagents await computer.collect(variableSet, token); + // Collect hooks from hook .json files + let collectedHooks: IChatRequestHooks | undefined; + try { + collectedHooks = await this.promptsService.getHooks(token); + } catch (error) { + this.logService.warn('[ChatService] Failed to collect hooks:', error); + } + // Build the agent request const agentRequest: IChatAgentRequest = { sessionResource: invocation.context.sessionResource, @@ -258,6 +269,8 @@ export class RunSubagentTool extends Disposable implements IToolImpl { userSelectedTools: modeTools, modeInstructions, parentRequestId: invocation.chatRequestId, + hooks: collectedHooks, + hasHooksEnabled: !!collectedHooks && Object.values(collectedHooks).some(arr => arr.length > 0), }; // Subscribe to tool invocations to clear markdown parts when a tool is invoked 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 3b53ed241bae1..75d099cd9b797 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 @@ -3532,6 +3532,52 @@ suite('LanguageModelToolsService', () => { assert.ok(toolIds.includes('multiSetTool'), 'Tool should be permitted if it belongs to at least one permitted toolset'); }); + test('isPermitted allows internal tools with canBeReferencedInPrompt=false when agent mode is disabled (issue #292935)', () => { + // Disable agent mode + configurationService.setUserConfiguration(ChatConfiguration.AgentEnabled, false); + + // Create internal infrastructure tool that explicitly cannot be referenced in prompts + const infrastructureTool: IToolData = { + id: 'infrastructureToolInternal', + toolReferenceName: 'infrastructureToolRef', + modelDescription: 'Infrastructure Tool', + displayName: 'Infrastructure Tool', + source: ToolDataSource.Internal, + canBeReferencedInPrompt: false, + }; + store.add(service.registerToolData(infrastructureTool)); + + // Create internal tool with canBeReferencedInPrompt=true (should be blocked) + const referencableTool: IToolData = { + id: 'referencableTool', + toolReferenceName: 'referencableToolRef', + modelDescription: 'Referencable Tool', + displayName: 'Referencable Tool', + source: ToolDataSource.Internal, + canBeReferencedInPrompt: true, + }; + store.add(service.registerToolData(referencableTool)); + + // Create internal tool with canBeReferencedInPrompt=undefined (should be blocked) + const undefinedTool: IToolData = { + id: 'undefinedTool', + toolReferenceName: 'undefinedToolRef', + modelDescription: 'Undefined Tool', + displayName: 'Undefined Tool', + source: ToolDataSource.Internal, + // canBeReferencedInPrompt is undefined + }; + store.add(service.registerToolData(undefinedTool)); + + // Get tools - only the infrastructure tool should be available + const tools = Array.from(service.getTools(undefined)); + const toolIds = tools.map(t => t.id); + + assert.ok(toolIds.includes('infrastructureToolInternal'), 'Internal infrastructure tool with canBeReferencedInPrompt=false should be permitted when agent mode is disabled'); + assert.ok(!toolIds.includes('referencableTool'), 'Internal tool with canBeReferencedInPrompt=true should NOT be permitted when agent mode is disabled'); + assert.ok(!toolIds.includes('undefinedTool'), 'Internal tool with canBeReferencedInPrompt=undefined should NOT be permitted when agent mode is disabled'); + }); + suite('ToolSet when clause filtering (issue #291154)', () => { test('ToolSet.getTools filters tools by when clause', () => { // Create a context key for testing diff --git a/src/vs/workbench/contrib/chat/test/browser/widget/input/chatEditsTree.test.ts b/src/vs/workbench/contrib/chat/test/browser/widget/input/chatEditsTree.test.ts index d9f9a38a76e08..82a8283135d9d 100644 --- a/src/vs/workbench/contrib/chat/test/browser/widget/input/chatEditsTree.test.ts +++ b/src/vs/workbench/contrib/chat/test/browser/widget/input/chatEditsTree.test.ts @@ -202,7 +202,7 @@ suite('ChatEditsTree', () => { store.dispose(); }); - test('storage listener fires after clear', () => { + test.skip('storage listener fires after clear', () => { // Stub create to avoid DOM/widget side effects in tests let createCallCount = 0; const origCreate = widget.create.bind(widget); @@ -228,7 +228,7 @@ suite('ChatEditsTree', () => { widget.create = origCreate; }); - test('currentSession is updated on rebuild', () => { + test.skip('currentSession is updated on rebuild', () => { // Stub create widget.create = (c, s) => { (widget as unknown as { _currentContainer: HTMLElement | undefined })._currentContainer = c; @@ -244,7 +244,7 @@ suite('ChatEditsTree', () => { assert.strictEqual(widget.currentSession, mockSession); }); - test('setEntries replays after view mode toggle', () => { + test.skip('setEntries replays after view mode toggle', () => { // Stub create and track setEntries calls widget.create = (c, s) => { (widget as unknown as { _currentContainer: HTMLElement | undefined })._currentContainer = c; diff --git a/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookClaudeCompat.test.ts b/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookClaudeCompat.test.ts index 84aa27f5722a6..6f852ed728582 100644 --- a/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookClaudeCompat.test.ts +++ b/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookClaudeCompat.test.ts @@ -57,9 +57,10 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - assert.strictEqual(result.size, 1); - assert.ok(result.has(HookType.PreToolUse)); - const entry = result.get(HookType.PreToolUse)!; + assert.strictEqual(result.disabledAllHooks, false); + assert.strictEqual(result.hooks.size, 1); + assert.ok(result.hooks.has(HookType.PreToolUse)); + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.originalId, 'PreToolUse'); assert.strictEqual(entry.hooks.length, 1); assert.strictEqual(entry.hooks[0].command, 'echo "pre-tool"'); @@ -75,9 +76,9 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - assert.strictEqual(result.size, 2); - assert.ok(result.has(HookType.SessionStart)); - assert.ok(result.has(HookType.Stop)); + assert.strictEqual(result.hooks.size, 2); + assert.ok(result.hooks.has(HookType.SessionStart)); + assert.ok(result.hooks.has(HookType.Stop)); }); test('parses multiple commands for same hook type', () => { @@ -92,13 +93,62 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks.length, 2); assert.strictEqual(entry.hooks[0].command, 'echo "first"'); assert.strictEqual(entry.hooks[1].command, 'echo "second"'); }); }); + suite('disableAllHooks', () => { + test('returns empty hooks and disabledAllHooks=true when disableAllHooks is true', () => { + const json = { + disableAllHooks: true, + hooks: { + PreToolUse: [ + { type: 'command', command: 'echo "should be ignored"' } + ] + } + }; + + const result = parseClaudeHooks(json, workspaceRoot, userHome); + + assert.strictEqual(result.disabledAllHooks, true); + assert.strictEqual(result.hooks.size, 0); + }); + + test('parses hooks normally when disableAllHooks is false', () => { + const json = { + disableAllHooks: false, + hooks: { + PreToolUse: [ + { type: 'command', command: 'echo "should be parsed"' } + ] + } + }; + + const result = parseClaudeHooks(json, workspaceRoot, userHome); + + assert.strictEqual(result.disabledAllHooks, false); + assert.strictEqual(result.hooks.size, 1); + }); + + test('parses hooks normally when disableAllHooks is not present', () => { + const json = { + hooks: { + PreToolUse: [ + { type: 'command', command: 'echo "should be parsed"' } + ] + } + }; + + const result = parseClaudeHooks(json, workspaceRoot, userHome); + + assert.strictEqual(result.disabledAllHooks, false); + assert.strictEqual(result.hooks.size, 1); + }); + }); + suite('nested hooks with matchers', () => { test('parses nested hooks with matcher', () => { const json = { @@ -116,7 +166,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks.length, 1); assert.strictEqual(entry.hooks[0].command, 'echo "bash hook"'); }); @@ -138,7 +188,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks.length, 2); }); @@ -160,7 +210,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks.length, 2); assert.strictEqual(entry.hooks[0].command, 'echo "bash"'); assert.strictEqual(entry.hooks[1].command, 'echo "write"'); @@ -181,55 +231,42 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks.length, 2); assert.strictEqual(entry.hooks[0].command, 'echo "direct"'); assert.strictEqual(entry.hooks[1].command, 'echo "nested"'); }); }); - suite('command without type field', () => { - test('parses command without explicit type field', () => { - const json = { - hooks: { - PreToolUse: [ - { command: 'echo "no type"' } - ] - } - }; - - const result = parseClaudeHooks(json, workspaceRoot, userHome); - - const entry = result.get(HookType.PreToolUse)!; - assert.strictEqual(entry.hooks.length, 1); - assert.strictEqual(entry.hooks[0].command, 'echo "no type"'); - }); - }); - suite('invalid inputs', () => { test('returns empty map for null json', () => { const result = parseClaudeHooks(null, workspaceRoot, userHome); - assert.strictEqual(result.size, 0); + assert.strictEqual(result.hooks.size, 0); + assert.strictEqual(result.disabledAllHooks, false); }); test('returns empty map for undefined json', () => { const result = parseClaudeHooks(undefined, workspaceRoot, userHome); - assert.strictEqual(result.size, 0); + assert.strictEqual(result.hooks.size, 0); + assert.strictEqual(result.disabledAllHooks, false); }); test('returns empty map for non-object json', () => { const result = parseClaudeHooks('string', workspaceRoot, userHome); - assert.strictEqual(result.size, 0); + assert.strictEqual(result.hooks.size, 0); + assert.strictEqual(result.disabledAllHooks, false); }); test('returns empty map for missing hooks property', () => { const result = parseClaudeHooks({}, workspaceRoot, userHome); - assert.strictEqual(result.size, 0); + assert.strictEqual(result.hooks.size, 0); + assert.strictEqual(result.disabledAllHooks, false); }); test('returns empty map for non-object hooks property', () => { const result = parseClaudeHooks({ hooks: 'invalid' }, workspaceRoot, userHome); - assert.strictEqual(result.size, 0); + assert.strictEqual(result.hooks.size, 0); + assert.strictEqual(result.disabledAllHooks, false); }); test('skips unknown hook types', () => { @@ -242,8 +279,8 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - assert.strictEqual(result.size, 1); - assert.ok(result.has(HookType.PreToolUse)); + assert.strictEqual(result.hooks.size, 1); + assert.ok(result.hooks.has(HookType.PreToolUse)); }); test('skips non-array hook entries', () => { @@ -255,7 +292,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - assert.strictEqual(result.size, 0); + assert.strictEqual(result.hooks.size, 0); }); test('skips invalid command entries', () => { @@ -271,7 +308,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks.length, 1); assert.strictEqual(entry.hooks[0].command, 'valid'); }); @@ -288,7 +325,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks.length, 1); assert.strictEqual(entry.hooks[0].command, 'valid'); }); @@ -306,7 +343,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.deepStrictEqual(entry.hooks[0].cwd, URI.file('/workspace/src')); }); @@ -321,7 +358,7 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.deepStrictEqual(entry.hooks[0].env, { NODE_ENV: 'production' }); }); @@ -336,9 +373,24 @@ suite('HookClaudeCompat', () => { const result = parseClaudeHooks(json, workspaceRoot, userHome); - const entry = result.get(HookType.PreToolUse)!; + const entry = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(entry.hooks[0].timeout, 60); }); + + test('supports Claude timeout alias', () => { + const json = { + hooks: { + PreToolUse: [ + { type: 'command', command: 'echo "test"', timeout: 1 } + ] + } + }; + + const result = parseClaudeHooks(json, workspaceRoot, userHome); + + const entry = result.hooks.get(HookType.PreToolUse)!; + assert.strictEqual(entry.hooks[0].timeout, 1); + }); }); }); }); @@ -432,9 +484,9 @@ suite('HookSourceFormat', () => { }; const result = parseClaudeHooks(hooksContent, URI.file('/workspace'), '/home/user'); - assert.strictEqual(result.size, 1); - assert.ok(result.has(HookType.PreToolUse)); - const hooks = result.get(HookType.PreToolUse)!; + assert.strictEqual(result.hooks.size, 1); + assert.ok(result.hooks.has(HookType.PreToolUse)); + const hooks = result.hooks.get(HookType.PreToolUse)!; assert.strictEqual(hooks.hooks.length, 1); // Empty command string is falsy and gets omitted by resolveHookCommand assert.strictEqual(hooks.hooks[0].command, undefined); diff --git a/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookCompatibility.test.ts b/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookCompatibility.test.ts new file mode 100644 index 0000000000000..7d4ba6ffe51d1 --- /dev/null +++ b/src/vs/workbench/contrib/chat/test/common/promptSyntax/hookCompatibility.test.ts @@ -0,0 +1,131 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; +import { HookType } from '../../../common/promptSyntax/hookSchema.js'; +import { parseCopilotHooks, parseHooksFromFile, HookSourceFormat } from '../../../common/promptSyntax/hookCompatibility.js'; +import { URI } from '../../../../../../base/common/uri.js'; + +suite('HookCompatibility', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + suite('parseCopilotHooks', () => { + const workspaceRoot = URI.file('/workspace'); + const userHome = '/home/user'; + + suite('basic parsing', () => { + test('parses simple hook with command', () => { + const json = { + hooks: { + PreToolUse: [ + { type: 'command', command: 'echo "pre-tool"' } + ] + } + }; + + const result = parseCopilotHooks(json, workspaceRoot, userHome); + + assert.strictEqual(result.size, 1); + assert.ok(result.has(HookType.PreToolUse)); + const entry = result.get(HookType.PreToolUse)!; + assert.strictEqual(entry.hooks.length, 1); + assert.strictEqual(entry.hooks[0].command, 'echo "pre-tool"'); + }); + }); + + suite('invalid inputs', () => { + test('returns empty result for null json', () => { + const result = parseCopilotHooks(null, workspaceRoot, userHome); + assert.strictEqual(result.size, 0); + }); + + test('returns empty result for undefined json', () => { + const result = parseCopilotHooks(undefined, workspaceRoot, userHome); + assert.strictEqual(result.size, 0); + }); + + test('returns empty result for missing hooks property', () => { + const result = parseCopilotHooks({}, workspaceRoot, userHome); + assert.strictEqual(result.size, 0); + }); + }); + }); + + suite('parseHooksFromFile', () => { + const workspaceRoot = URI.file('/workspace'); + const userHome = '/home/user'; + + test('uses Copilot format for .github/hooks/*.json files', () => { + const fileUri = URI.file('/workspace/.github/hooks/my-hooks.json'); + const json = { + hooks: { + PreToolUse: [ + { type: 'command', command: 'echo "test"' } + ] + } + }; + + const result = parseHooksFromFile(fileUri, json, workspaceRoot, userHome); + + assert.strictEqual(result.format, HookSourceFormat.Copilot); + assert.strictEqual(result.disabledAllHooks, false); + assert.strictEqual(result.hooks.size, 1); + }); + + test('uses Claude format for .claude/settings.json files', () => { + const fileUri = URI.file('/workspace/.claude/settings.json'); + const json = { + disableAllHooks: true, + hooks: { + PreToolUse: [ + { type: 'command', command: 'echo "test"' } + ] + } + }; + + const result = parseHooksFromFile(fileUri, json, workspaceRoot, userHome); + + assert.strictEqual(result.format, HookSourceFormat.Claude); + assert.strictEqual(result.disabledAllHooks, true); + assert.strictEqual(result.hooks.size, 0); + }); + + test('disableAllHooks is ignored for Copilot format', () => { + const fileUri = URI.file('/workspace/.github/hooks/hooks.json'); + const json = { + disableAllHooks: true, + hooks: { + SessionStart: [ + { type: 'command', command: 'echo "start"' } + ] + } + }; + + const result = parseHooksFromFile(fileUri, json, workspaceRoot, userHome); + + // Copilot format does not support disableAllHooks + assert.strictEqual(result.disabledAllHooks, false); + assert.strictEqual(result.hooks.size, 1); + }); + + test('disabledAllHooks works for Claude format', () => { + const fileUri = URI.file('/workspace/.claude/settings.local.json'); + const json = { + disableAllHooks: true, + hooks: { + SessionStart: [ + { type: 'command', command: 'echo "start"' } + ] + } + }; + + const result = parseHooksFromFile(fileUri, json, workspaceRoot, userHome); + + assert.strictEqual(result.disabledAllHooks, true); + assert.strictEqual(result.hooks.size, 0); + }); + }); +});