diff --git a/.github/skills/vscode-dev-workbench/SKILL.md b/.github/skills/vscode-dev-workbench/SKILL.md index 01589c1716cb1..93716893ca0d9 100644 --- a/.github/skills/vscode-dev-workbench/SKILL.md +++ b/.github/skills/vscode-dev-workbench/SKILL.md @@ -21,12 +21,20 @@ If your paths differ, check `server/` in `vscode-dev` for the source root resolu ## Start the dev server +**Critical:** Run `npm run dev` from the **`vscode-dev`** folder, NOT from `vscode`. The `vscode` repo has no `dev` script and will fail with `npm error Missing script: "dev"`. Terminal tools that simplify/strip leading `cd` into separate commands will silently keep the cwd of a previous terminal — always use an absolute `pushd` or verify with `pwd` before `npm run dev`. + ```bash -cd vscode-dev -npm run dev # runs watch + nodemon; serves https://127.0.0.1:3000 +cd /path/to/vscode-dev # NOT /path/to/vscode +npm run dev # runs watch + nodemon; serves https://127.0.0.1:3000 ``` -On first start you may see one crash like `Cannot find module './indexes'` — it's the watcher racing the first build. nodemon restarts automatically once `out/` finishes compiling. +If you're driving this through an agent/terminal tool, prefer: + +```bash +pushd /absolute/path/to/vscode-dev >/dev/null && pwd && npm run dev +``` + +On first start you may see one crash like `Cannot find module './indexes'` — it's the watcher racing the first build. nodemon restarts automatically once `out/` finishes compiling. The server is ready when `curl -sk -o /dev/null -w "%{http_code}" https://127.0.0.1:3000/` returns `200`. ## URLs @@ -97,15 +105,19 @@ For a true mobile viewport, drive a standalone Playwright script with `devices[' ## Testing the Agents window against a local mock agent host +If the scenario touches the Agents window (`/agents` route), you almost always need the mock agent host running. Without it, the Agents window will sit on the sign-in / tunnel-discovery screen and block any real interaction. Start it **in addition to** the dev server — it's a second terminal, not a replacement. + `vscode-dev` supports a `?mock-agent-host=ws://…` URL parameter that short-circuits tunnel discovery and wires the Agents window to a raw WebSocket. Pair it with the mock agent host binary from `microsoft/vscode`: ```bash -cd vscode +cd /path/to/vscode node out/vs/platform/agentHost/node/agentHostServerMain.js \ --enable-mock-agent --quiet --without-connection-token --port 8765 # Listens on ws://localhost:8765 ``` +Prerequisite: `out/` in the `vscode` repo must be populated by the `VS Code - Build` task (or `npm run watch`). If `out/vs/platform/agentHost/node/agentHostServerMain.js` is missing, start that task first. + `--enable-mock-agent` registers the `ScriptedMockAgent` from `src/vs/platform/agentHost/test/node/mockAgent.ts` with one pre-existing session. Seed additional sessions via the `VSCODE_AGENT_HOST_MOCK_SEED_SESSIONS` env var, using a comma-separated list of session URIs (for example, `VSCODE_AGENT_HOST_MOCK_SEED_SESSIONS=mock://pre-1,mock://pre-2`). Scripted prompts include `hello`, `use-tool`, `error`, `permission`, `write-file`, `run-safe-command`, `slow`, `client-tool`, `subagent`, etc. (see `mockAgent.ts` for the full list). Then open: diff --git a/extensions/copilot/package.json b/extensions/copilot/package.json index 5fbdd80f6a69d..3da56f266cb62 100644 --- a/extensions/copilot/package.json +++ b/extensions/copilot/package.json @@ -235,6 +235,27 @@ ] } }, + { + "name": "skill", + "toolReferenceName": "skill", + "displayName": "%copilot.tools.skill.name%", + "icon": "$(book)", + "userDescription": "%copilot.tools.skill.description%", + "modelDescription": "Invoke a skill to handle a user's request with specialized instructions and workflows.\n\nSkills are domain-specific capabilities discovered from SKILL.md files. When a user's task matches an available skill, call this tool to load and apply it. If the user types a slash command (e.g. \"/deploy\", \"/test\"), treat it as a skill invocation.\n\nUsage:\n- Pass the skill name only (no arguments).\n- Examples: skill: \"docx\", skill: \"deploy\", skill: \"fix-ci-failures\"\n\nRules:\n- Available skills appear in system-reminder messages earlier in the conversation.\n- BLOCKING: When a matching skill exists, you MUST call this tool before producing any other output about the task.\n- Never reference a skill without calling this tool.\n- Do not call this tool for a skill that is already active in the current turn (indicated by a tag).\n- Do not use this tool for built-in commands such as /help or /clear.", + "when": "config.github.copilot.chat.skillTool.enabled", + "inputSchema": { + "type": "object", + "properties": { + "skill": { + "type": "string", + "description": "The skill name. E.g., \"commit\", \"review-pr\", or \"pdf\"" + } + }, + "required": [ + "skill" + ] + } + }, { "name": "copilot_searchWorkspaceSymbols", "toolReferenceName": "symbols", @@ -1265,7 +1286,8 @@ "problems", "readFile", "viewImage", - "readNotebookCellOutput" + "readNotebookCellOutput", + "skill" ] }, { @@ -4411,6 +4433,15 @@ "experimental" ] }, + "github.copilot.chat.skillTool.enabled": { + "type": "boolean", + "default": false, + "markdownDescription": "%github.copilot.config.skill.enabled%", + "tags": [ + "advanced", + "experimental" + ] + }, "github.copilot.chat.executionSubagent.enabled": { "type": "boolean", "default": false, diff --git a/extensions/copilot/package.nls.json b/extensions/copilot/package.nls.json index 6d6f23d955764..5eb1a8178da2d 100644 --- a/extensions/copilot/package.nls.json +++ b/extensions/copilot/package.nls.json @@ -490,6 +490,9 @@ "copilot.tools.runSubagent.description": "Runs a task within an isolated subagent context. Enables efficient organization of tasks and context window management.", "copilot.tools.searchSubagent.name": "Search Subagent", "copilot.tools.searchSubagent.description": "Launch an iterative search-focused subagent to find relevant code in your workspace.", + "copilot.tools.skill.name": "Skill", + "copilot.tools.skill.description": "Execute a skill by name. Skills provide specialized capabilities, domain knowledge, and refined workflows.", + "github.copilot.config.skill.enabled": "Enable the skill tool in Copilot Chat. When enabled, skills are invoked via a dedicated skill tool instead of readFile.", "github.copilot.config.searchSubagent.enabled": "Enable the search subagent tool for iterative code exploration in the workspace.", "github.copilot.config.searchSubagent.useAgenticProxy": "Use the agentic proxy for the search subagent tool.", "github.copilot.config.searchSubagent.model": "Model to use for the search subagent. When useAgenticProxy is enabled, defaults to 'vscode-agentic-search-router-a'. Otherwise defaults to the main agent model.", diff --git a/extensions/copilot/src/extension/intents/node/agentIntent.ts b/extensions/copilot/src/extension/intents/node/agentIntent.ts index bd6186c9d6965..853eb96833503 100644 --- a/extensions/copilot/src/extension/intents/node/agentIntent.ts +++ b/extensions/copilot/src/extension/intents/node/agentIntent.ts @@ -120,6 +120,9 @@ export const getAgentTools = async (accessor: ServicesAccessor, request: vscode. const executionSubagentEnabled = configurationService.getExperimentBasedConfig(ConfigKey.Advanced.ExecutionSubagentToolEnabled, experimentationService); allowTools[ToolName.ExecutionSubagent] = isGptOrAnthropic && executionSubagentEnabled; + const skillToolEnabled = configurationService.getExperimentBasedConfig(ConfigKey.Advanced.SkillToolEnabled, experimentationService); + allowTools[ToolName.Skill] = skillToolEnabled; + allowTools[CUSTOM_TOOL_SEARCH_NAME] = !!model.supportsToolSearch; if (model.family.includes('grok-code')) { diff --git a/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts b/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts index 03e30a22219ed..403a3ae61c145 100644 --- a/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts +++ b/extensions/copilot/src/extension/intents/node/toolCallingLoop.ts @@ -383,12 +383,29 @@ export abstract class ToolCallingLoop 0) { + this._logService.info('[ToolCallingLoop] Autopilot: model produced a text-only response, treating as done'); + return undefined; + } + // safety valve — only give up after exhausting all continuation attempts if (this.autopilotIterationCount >= ToolCallingLoop.MAX_AUTOPILOT_ITERATIONS) { this._logService.info(`[ToolCallingLoop] Autopilot: hit max iterations (${ToolCallingLoop.MAX_AUTOPILOT_ITERATIONS}), letting it stop`); return undefined; } + // If we already nudged once and the model still produced no tool calls, the model + // is effectively done — further nudges just waste tokens. Bail out and let the + // loop stop. + if (this.autopilotStopHookActive && result.round.toolCalls.length === 0) { + this._logService.info('[ToolCallingLoop] Autopilot: prior nudge produced no tool calls, stopping to avoid wasted requests'); + return undefined; + } + this.autopilotIterationCount++; return 'You have not yet marked the task as complete using the task_complete tool. ' + 'You must call task_complete when done — whether the task involved code changes, answering a question, or any other interaction.\n\n' + @@ -900,7 +917,7 @@ export abstract class ToolCallingLoop ({ id: generateUuid(), @@ -150,10 +150,11 @@ describe('ToolCallingLoop autopilot', () => { vi.restoreAllMocks(); }); - function createLoop(permissionLevel?: string): AutopilotTestToolCallingLoop { + function createLoop(permissionLevel?: string, requestOverrides: Partial = {}): AutopilotTestToolCallingLoop { const conversation = createTestConversation(1); const request = createMockChatRequest({ permissionLevel, + ...requestOverrides, } as Partial); const loop = instantiationService.createInstance( AutopilotTestToolCallingLoop, @@ -196,15 +197,24 @@ describe('ToolCallingLoop autopilot', () => { expect(msg).toBeUndefined(); }); - it('should keep nudging even with autopilotStopHookActive set', () => { + it('should bail when prior nudge produced no tool calls', () => { const loop = createLoop('autopilot'); // Simulate that we already nudged once and set the flag loop.setAutopilotStopHookActive(true); - // Should still return a nudge — autopilotStopHookActive no longer causes early bail + // Should bail — the previous nudge produced no tool calls, so further nudges + // would just waste tokens (the model is effectively done). const result = loop.testShouldAutopilotContinue(createMockSingleResult()); - expect(result).toContain('task_complete'); + expect(result).toBeUndefined(); + }); + + it('should skip the nudge when the model returned a text-only response (no tool calls)', () => { + const loop = createLoop('autopilot'); + const result = loop.testShouldAutopilotContinue(createMockSingleResult({ + round: createMockRound([], 'Here is a summary of what I did.'), + })); + expect(result).toBeUndefined(); }); it('should allow another nudge after autopilotStopHookActive is reset', () => { diff --git a/extensions/copilot/src/extension/prompts/node/agent/agentPrompt.tsx b/extensions/copilot/src/extension/prompts/node/agent/agentPrompt.tsx index 502ca65ad021c..e0ed7841dfc90 100644 --- a/extensions/copilot/src/extension/prompts/node/agent/agentPrompt.tsx +++ b/extensions/copilot/src/extension/prompts/node/agent/agentPrompt.tsx @@ -525,6 +525,8 @@ class SkillAdherenceReminder extends PromptElement constructor( props: SkillAdherenceReminderProps, @ICustomInstructionsService private readonly customInstructionsService: ICustomInstructionsService, + @IConfigurationService private readonly configurationService: IConfigurationService, + @IExperimentationService private readonly experimentationService: IExperimentationService, ) { super(props); } @@ -541,6 +543,14 @@ class SkillAdherenceReminder extends PromptElement return undefined; } + const skillToolEnabled = this.configurationService.getExperimentBasedConfig(ConfigKey.Advanced.SkillToolEnabled, this.experimentationService); + + if (skillToolEnabled) { + return + Always check if any skills apply to the user's request. If so, use the {ToolName.Skill} tool to invoke the skill by name. Multiple skill files may be needed for a single request. These files contain best practices built from testing that are needed for high-quality outputs.
+ ; + } + return Always check if any skills apply to the user's request. If so, use the {ToolName.ReadFile} tool to read the corresponding SKILL.md files. Multiple skill files may be needed for a single request. These files contain best practices built from testing that are needed for high-quality outputs.
; diff --git a/extensions/copilot/src/extension/tools/common/skillTelemetry.ts b/extensions/copilot/src/extension/tools/common/skillTelemetry.ts new file mode 100644 index 0000000000000..b5e25e0e15f55 --- /dev/null +++ b/extensions/copilot/src/extension/tools/common/skillTelemetry.ts @@ -0,0 +1,54 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { ICustomInstructionsService, ISkillInfo } from '../../../platform/customInstructions/common/customInstructionsService'; +import { IExtensionsService } from '../../../platform/extensions/common/extensionsService'; +import { ITelemetryService } from '../../../platform/telemetry/common/telemetry'; +import { hash } from '../../../util/vs/base/common/hash'; +import { URI } from '../../../util/vs/base/common/uri'; + +/** + * Sends `skillContentRead` telemetry for a skill invocation. + * Shared between the skill tool and the readFile tool to ensure consistent + * telemetry when skills are loaded through either path. + * + * TODO: Add pluginNameHash and pluginVersion properties once vscode core's + * extensionPromptFileProvider command exposes IAgentPluginService metadata. + */ +export function sendSkillContentReadTelemetry( + telemetryService: ITelemetryService, + customInstructionsService: ICustomInstructionsService, + extensionsService: IExtensionsService, + uri: URI, + skillInfo: ISkillInfo, + content: string, +): void { + const extensionSkillInfo = customInstructionsService.getExtensionSkillInfo(uri); + const extensionId = extensionSkillInfo?.extensionId ?? ''; + const extensionVersion = extensionId ? extensionsService.getExtension(extensionId)?.packageJSON?.version ?? '' : ''; + const contentHash = content ? String(hash(content)) : ''; + + const plaintextProps = { + skillName: skillInfo.skillName, + skillPath: uri.toString(), + skillExtensionId: extensionId, + skillExtensionVersion: extensionVersion, + skillStorage: skillInfo.storage, + skillContentHash: contentHash, + }; + + telemetryService.sendGHTelemetryEvent('skillContentRead', + { + skillNameHash: String(hash(skillInfo.skillName)), + skillExtensionIdHash: extensionId ? String(hash(extensionId)) : '', + skillExtensionVersion: plaintextProps.skillExtensionVersion, + skillStorage: plaintextProps.skillStorage, + skillContentHash: contentHash, + } + ); + + telemetryService.sendEnhancedGHTelemetryEvent('skillContentRead', plaintextProps); + telemetryService.sendInternalMSFTTelemetryEvent('skillContentRead', plaintextProps); +} diff --git a/extensions/copilot/src/extension/tools/common/toolNames.ts b/extensions/copilot/src/extension/tools/common/toolNames.ts index 412c1e50e0914..e2ba597305e82 100644 --- a/extensions/copilot/src/extension/tools/common/toolNames.ts +++ b/extensions/copilot/src/extension/tools/common/toolNames.ts @@ -74,6 +74,7 @@ export enum ToolName { ToolSearch = 'tool_search', ResolveMemoryFileUri = 'resolve_memory_file_uri', ExecutionSubagent = 'execution_subagent', + Skill = 'skill', SessionStoreSql = 'session_store_sql', CoreOpenBrowserPage = 'open_browser_page', CoreClickElement = 'click_element', @@ -266,6 +267,7 @@ export const toolCategories: Record = { [ToolName.Memory]: ToolCategory.VSCodeInteraction, [ToolName.ToolSearch]: ToolCategory.Core, [ToolName.ResolveMemoryFileUri]: ToolCategory.Core, + [ToolName.Skill]: ToolCategory.Core, [ToolName.SessionStoreSql]: ToolCategory.Core, } as const; diff --git a/extensions/copilot/src/extension/tools/node/allTools.ts b/extensions/copilot/src/extension/tools/node/allTools.ts index 8f1170fb9f182..88d8e495a96b4 100644 --- a/extensions/copilot/src/extension/tools/node/allTools.ts +++ b/extensions/copilot/src/extension/tools/node/allTools.ts @@ -20,6 +20,7 @@ import './githubTextSearchTool'; import './insertEditTool'; import './installExtensionTool'; import './listDirTool'; +import './skillTool'; import './manageTodoListTool'; import './memoryTool'; import './multiReplaceStringTool'; diff --git a/extensions/copilot/src/extension/tools/node/readFileTool.tsx b/extensions/copilot/src/extension/tools/node/readFileTool.tsx index 4c98cf8e79bad..38d453bcc2780 100644 --- a/extensions/copilot/src/extension/tools/node/readFileTool.tsx +++ b/extensions/copilot/src/extension/tools/node/readFileTool.tsx @@ -20,9 +20,9 @@ import { IExperimentationService } from '../../../platform/telemetry/common/null import { ITelemetryService } from '../../../platform/telemetry/common/telemetry'; import { IWorkspaceService } from '../../../platform/workspace/common/workspaceService'; import { getCachedSha256Hash } from '../../../util/common/crypto'; -import { hash } from '../../../util/vs/base/common/hash'; import { clamp } from '../../../util/vs/base/common/numbers'; import { dirname, extUriBiasedIgnorePathCase } from '../../../util/vs/base/common/resources'; +import { sendSkillContentReadTelemetry } from '../common/skillTelemetry'; import { URI } from '../../../util/vs/base/common/uri'; import { IInstantiationService } from '../../../util/vs/platform/instantiation/common/instantiation'; import { LanguageModelPromptTsxPart, LanguageModelToolResult, Location, MarkdownString, Range } from '../../../vscodeTypes'; @@ -374,37 +374,9 @@ export class ReadFileTool implements ICopilotTool { // Send separate skillContentRead event only for successful skill file reads. // Reuses extensionSkillInfo/skillInfo already computed above. - // TODO: Add pluginNameHash and pluginVersion properties once vscode core's - // extensionPromptFileProvider command exposes IAgentPluginService metadata. if (skillInfo && documentSnapshot && uri && this.customInstructionsService.isSkillMdFile(uri)) { const content = documentSnapshot instanceof TextDocumentSnapshot ? documentSnapshot.getText() : ''; - const extensionId = extensionSkillInfo?.extensionId ?? ''; - const extensionVersion = extensionId ? this.extensionsService.getExtension(extensionId)?.packageJSON?.version ?? '' : ''; - const contentHash = content ? String(hash(content)) : ''; - - // Plaintext properties shared by enhanced GH and internal MSFT events - const plaintextProps = { - skillName: skillInfo.skillName, - skillPath: uri.toString(), - skillExtensionId: extensionId, - skillExtensionVersion: extensionVersion, - skillStorage: skillInfo.storage, - skillContentHash: contentHash, - }; - - this.telemetryService.sendGHTelemetryEvent('skillContentRead', - { - skillNameHash: String(hash(skillInfo.skillName)), - skillExtensionIdHash: extensionId ? String(hash(extensionId)) : '', - skillExtensionVersion: plaintextProps.skillExtensionVersion, - skillStorage: plaintextProps.skillStorage, - skillContentHash: contentHash, - } - ); - - this.telemetryService.sendEnhancedGHTelemetryEvent('skillContentRead', plaintextProps); - - this.telemetryService.sendInternalMSFTTelemetryEvent('skillContentRead', plaintextProps); + sendSkillContentReadTelemetry(this.telemetryService, this.customInstructionsService, this.extensionsService, uri, skillInfo, content); } } diff --git a/extensions/copilot/src/extension/tools/node/skillTool.ts b/extensions/copilot/src/extension/tools/node/skillTool.ts new file mode 100644 index 0000000000000..bfa2e7a108b15 --- /dev/null +++ b/extensions/copilot/src/extension/tools/node/skillTool.ts @@ -0,0 +1,305 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as l10n from '@vscode/l10n'; +import type * as vscode from 'vscode'; +import { ICustomInstructionsService } from '../../../platform/customInstructions/common/customInstructionsService'; +import { SKILL_FILENAME } from '../../../platform/customInstructions/common/promptTypes'; +import { TextDocumentSnapshot } from '../../../platform/editing/common/textDocumentSnapshot'; +import { IExtensionsService } from '../../../platform/extensions/common/extensionsService'; +import { IFileSystemService } from '../../../platform/filesystem/common/fileSystemService'; +import { FileType } from '../../../platform/filesystem/common/fileTypes'; +import { ITelemetryService } from '../../../platform/telemetry/common/telemetry'; +import { IWorkspaceService } from '../../../platform/workspace/common/workspaceService'; +import { extUriBiasedIgnorePathCase } from '../../../util/vs/base/common/resources'; +import { equalsIgnoreCase } from '../../../util/vs/base/common/strings'; +import { isString } from '../../../util/vs/base/common/types'; +import { URI } from '../../../util/vs/base/common/uri'; +import { ExtendedLanguageModelToolResult, LanguageModelTextPart, MarkdownString } from '../../../vscodeTypes'; +import { isCustomizationsIndex } from '../../prompt/common/chatVariablesCollection'; +import { IBuildPromptContext } from '../../prompt/common/intents'; +import { ToolName } from '../common/toolNames'; +import { CopilotToolMode, ICopilotTool, ToolRegistry } from '../common/toolsRegistry'; +import { IToolsService } from '../common/toolsService'; +import { formatUriForFileWidget } from '../common/toolUtils'; +import { sendSkillContentReadTelemetry } from '../common/skillTelemetry'; + +export interface ISkillParams { + /** The skill name. E.g., "commit", "review-pr", or "pdf" */ + skill: string; +} + +/** Maximum number of related files to list in skill context */ +const MAX_RELATED_FILES = 50; + +/** Directories to skip when listing related files */ +const SKILL_SKIP_DIRS = new Set([ + '.git', 'node_modules', 'dist', 'build', 'out', '.cache', + 'coverage', '__pycache__', 'target', 'bin', 'obj', '.venv', 'venv', +]); + +class SkillTool implements ICopilotTool { + public static readonly toolName = ToolName.Skill; + public static readonly nonDeferred = true; + private _inputContext: IBuildPromptContext | undefined; + + constructor( + @IWorkspaceService private readonly workspaceService: IWorkspaceService, + @ICustomInstructionsService private readonly customInstructionsService: ICustomInstructionsService, + @IFileSystemService private readonly fileSystemService: IFileSystemService, + @IToolsService private readonly toolsService: IToolsService, + @ITelemetryService private readonly telemetryService: ITelemetryService, + @IExtensionsService private readonly extensionsService: IExtensionsService, + ) { } + + async invoke(options: vscode.LanguageModelToolInvocationOptions, token: vscode.CancellationToken) { + const uri = this.resolveSkillUri(options.input.skill); + + // Read the skill file content + const document = await this.workspaceService.openTextDocument(uri); + const snapshot = TextDocumentSnapshot.create(document); + const skillContent = snapshot.getText(); + + // Emit skill content read telemetry + const skillInfo = this.customInstructionsService.getSkillInfo(uri); + if (skillInfo) { + sendSkillContentReadTelemetry(this.telemetryService, this.customInstructionsService, this.extensionsService, uri, skillInfo, skillContent); + } + + const mode = parseSkillContext(skillContent); + + if (mode === 'fork') { + return this.invokeFork(skillContent, uri, options, token); + } else { + return this.invokeInline(skillContent, uri); + } + } + + private async invokeInline(skillContent: string, uri: URI) { + const skillInfo = this.customInstructionsService.getSkillInfo(uri); + const skillLabel = skillInfo?.skillName ?? 'skill'; + const skillFolderUri = skillInfo?.skillFolderUri ?? extUriBiasedIgnorePathCase.dirname(uri); + + // List related files in the skill directory + const relatedFiles = await this.listRelatedFiles(skillFolderUri); + const relatedFilesSection = relatedFiles.length > 0 + ? `\nRelated files (use read_file tool to read):\n${relatedFiles.map(f => ` - ${f}`).join('\n')}\n` + : ''; + + const resultText = ` +Base directory: ${skillFolderUri.fsPath} +${relatedFilesSection} +${skillContent} +`; + + const result = new ExtendedLanguageModelToolResult([new LanguageModelTextPart(resultText)]); + result.toolResultMessage = new MarkdownString(l10n.t`Loaded skill: ${skillLabel}`); + result.toolMetadata = { + skill: skillLabel, + skillUri: uri.toString(), + agentName: 'skill' + }; + return result; + } + + private async invokeFork(skillContent: string, uri: URI, options: vscode.LanguageModelToolInvocationOptions, token: vscode.CancellationToken) { + const skillInfo = this.customInstructionsService.getSkillInfo(uri); + const skillLabel = skillInfo?.skillName ?? options.input.skill; + + // Use the user's original message as the task for the subagent + const userMessage = this._inputContext?.conversation?.turns[0]?.request.message; + const query = userMessage ?? `Run the ${skillLabel} skill`; + + // Embed skill instructions in the prompt for the subagent + const prompt = `You have been loaded with the following skill instructions. Follow them carefully to complete the task. + + +${skillContent} + + +Task: ${query}`; + + // Delegate to the runSubagent tool which goes through the full VS Code chat + // pipeline (automatic instructions, hooks, model resolution, nesting depth, etc.) + const subagentResult = await this.toolsService.invokeTool(ToolName.CoreRunSubagent, { + ...options, + input: { + prompt, + description: `Skill: ${skillLabel}`, + }, + }, token); + + // Extract text from the subagent result + const parts: string[] = []; + for (const part of subagentResult.content) { + if (part instanceof LanguageModelTextPart) { + parts.push(part.value); + } + } + const subagentResponse = parts.join('') || 'Skill completed with no output'; + + // Frame the result as skill output (not as another agent's response) so the + // parent agent summarizes the content naturally without treating it as a + // conversation with a separate agent. + const result = new ExtendedLanguageModelToolResult([new LanguageModelTextPart( + `Result from the "${skillLabel}" skill:\n\n${subagentResponse}` + )]); + result.toolMetadata = { + skill: options.input.skill, + skillUri: uri.toString(), + agentName: 'skill' + }; + result.toolResultMessage = new MarkdownString(l10n.t`Skill complete: ${skillLabel}`); + return result; + } + + async prepareInvocation(options: vscode.LanguageModelToolInvocationPrepareOptions, _token: vscode.CancellationToken): Promise { + let uri: URI; + try { + uri = this.resolveSkillUri(options.input.skill); + } catch { + return { + invocationMessage: new MarkdownString(l10n.t`Loading skill: ${options.input.skill}`), + }; + } + + await this.customInstructionsService.refreshExtensionPromptFiles(); + const skillInfo = this.customInstructionsService.getSkillInfo(uri); + const skillLabel = skillInfo?.skillName ?? options.input.skill; + + // Read the skill file to check frontmatter for context: fork + const document = await this.workspaceService.openTextDocument(uri); + const snapshot = TextDocumentSnapshot.create(document); + const mode = parseSkillContext(snapshot.getText()); + + if (mode === 'fork') { + return { + invocationMessage: new MarkdownString(l10n.t`Running skill ${formatUriForFileWidget(uri, { vscodeLinkType: 'skill', linkText: skillLabel })}`), + pastTenseMessage: new MarkdownString(l10n.t`Ran skill ${formatUriForFileWidget(uri, { vscodeLinkType: 'skill', linkText: skillLabel })}`), + }; + } + + return { + invocationMessage: new MarkdownString(l10n.t`Loading skill ${formatUriForFileWidget(uri, { vscodeLinkType: 'skill', linkText: skillLabel })}`), + pastTenseMessage: new MarkdownString(l10n.t`Loaded skill ${formatUriForFileWidget(uri, { vscodeLinkType: 'skill', linkText: skillLabel })}`), + }; + } + + async resolveInput(input: ISkillParams, promptContext: IBuildPromptContext, _mode: CopilotToolMode): Promise { + this._inputContext = promptContext; + return input; + } + + /** + * Resolve a skill name to its SKILL.md URI by searching the instruction index. + * If not found, throws with a list of available skills. + */ + private resolveSkillUri(skillName: string): URI { + const indexVariable = this._inputContext?.chatVariables.find(isCustomizationsIndex); + const indexValue = indexVariable && isString(indexVariable.value) ? indexVariable.value : undefined; + return resolveSkillUri( + skillName, + indexValue, + text => this.customInstructionsService.parseInstructionIndexFile(text), + uri => this.customInstructionsService.getSkillInfo(uri), + ); + } + + /** + * List files in a skill directory, excluding SKILL.md and skipped directories. + */ + private async listRelatedFiles(skillFolderUri: URI): Promise { + return listRelatedFiles(skillFolderUri, uri => this.fileSystemService.readDirectory(uri)); + } +} + +ToolRegistry.registerTool(SkillTool); + +export type ReadDirectoryFn = (uri: URI) => Promise<[string, FileType][]>; + +/** + * Resolve a skill name to its SKILL.md URI by searching the instruction index. + * If not found, throws with a list of available skills. + */ +export function resolveSkillUri( + skillName: string, + indexValue: string | undefined, + parseInstructionIndexFile: (text: string) => { readonly skills: Iterable }, + getSkillInfo: (uri: URI) => { readonly skillName: string } | undefined, +): URI { + const availableSkills: string[] = []; + + if (isString(indexValue)) { + const indexFile = parseInstructionIndexFile(indexValue); + for (const skillUri of indexFile.skills) { + const info = getSkillInfo(skillUri); + if (info) { + if (info.skillName === skillName) { + return skillUri; + } + availableSkills.push(info.skillName); + } + } + } + + const skillListMessage = availableSkills.length > 0 + ? ` Available skills: ${availableSkills.join(', ')}` + : ''; + throw new Error(`Skill "${skillName}" not found.${skillListMessage}`); +} + +/** + * List files in a skill directory, excluding SKILL.md and skipped directories. + */ +export async function listRelatedFiles(skillFolderUri: URI, readDirectory: ReadDirectoryFn): Promise { + try { + const files: string[] = []; + await listRelatedFilesRecursive(skillFolderUri, skillFolderUri, files, readDirectory); + return files; + } catch { + return []; + } +} + +export async function listRelatedFilesRecursive(baseUri: URI, currentUri: URI, files: string[], readDirectory: ReadDirectoryFn, depth: number = 0): Promise { + if (files.length >= MAX_RELATED_FILES || depth > 5) { + return; + } + + const entries = await readDirectory(currentUri); + + for (const [name, type] of entries) { + if (files.length >= MAX_RELATED_FILES) { + break; + } + + if (type === FileType.Directory) { + if (!SKILL_SKIP_DIRS.has(name)) { + await listRelatedFilesRecursive(baseUri, extUriBiasedIgnorePathCase.joinPath(currentUri, name), files, readDirectory, depth + 1); + } + } else if (type === FileType.File && !equalsIgnoreCase(name, SKILL_FILENAME)) { + const relativePath = extUriBiasedIgnorePathCase.relativePath(baseUri, extUriBiasedIgnorePathCase.joinPath(currentUri, name)); + if (relativePath) { + files.push(relativePath); + } + } + } +} + +/** + * Parse the `context` field from SKILL.md YAML frontmatter. + * Returns 'fork' if frontmatter contains `context: fork`, otherwise 'inline'. + */ +export function parseSkillContext(content: string): 'inline' | 'fork' { + const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/m); + if (!frontmatterMatch) { + return 'inline'; + } + const contextMatch = frontmatterMatch[1].match(/^context:\s*(.+)$/m); + if (contextMatch && contextMatch[1].trim() === 'fork') { + return 'fork'; + } + return 'inline'; +} diff --git a/extensions/copilot/src/extension/tools/node/test/skillTool.spec.ts b/extensions/copilot/src/extension/tools/node/test/skillTool.spec.ts new file mode 100644 index 0000000000000..e73dbfb092f99 --- /dev/null +++ b/extensions/copilot/src/extension/tools/node/test/skillTool.spec.ts @@ -0,0 +1,19 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { expect, suite, test } from 'vitest'; +import { toolCategories, ToolCategory, ToolName } from '../../common/toolNames'; +import { ToolRegistry } from '../../common/toolsRegistry'; + +// Ensure side-effect registration +import '../skillTool'; + +suite('SkillTool', () => { + test('is registered and categorized as Core', () => { + const isRegistered = ToolRegistry.getTools().some(t => t.toolName === ToolName.Skill); + expect(isRegistered).toBe(true); + expect(toolCategories[ToolName.Skill]).toBe(ToolCategory.Core); + }); +}); diff --git a/extensions/copilot/src/extension/tools/vscode-node/test/skillTool.test.ts b/extensions/copilot/src/extension/tools/vscode-node/test/skillTool.test.ts new file mode 100644 index 0000000000000..c7a5da0c2a701 --- /dev/null +++ b/extensions/copilot/src/extension/tools/vscode-node/test/skillTool.test.ts @@ -0,0 +1,433 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { FileType } from '../../../../platform/filesystem/common/fileTypes'; +import { URI } from '../../../../util/vs/base/common/uri'; +import { listRelatedFiles, listRelatedFilesRecursive, parseSkillContext, ReadDirectoryFn, resolveSkillUri } from '../../node/skillTool'; + +suite('parseSkillContext', () => { + test('returns inline when no frontmatter', () => { + assert.strictEqual(parseSkillContext('# My Skill\nSome content'), 'inline'); + }); + + test('returns inline when frontmatter has no context field', () => { + const content = '---\nname: test\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns fork when context is fork', () => { + const content = '---\ncontext: fork\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); + + test('returns inline when context is not fork', () => { + const content = '---\ncontext: inline\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns fork with extra whitespace around value', () => { + const content = '---\ncontext: fork \n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); + + test('returns inline for empty content', () => { + assert.strictEqual(parseSkillContext(''), 'inline'); + }); + + test('handles context field among other frontmatter fields', () => { + const content = '---\nname: my-skill\ncontext: fork\ndescription: A skill\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); + + test('returns inline when frontmatter delimiters are missing closing', () => { + const content = '---\ncontext: fork\n# No closing delimiter'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline when --- appears only once', () => { + const content = '---\nSome text without closing frontmatter'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline for context value that contains fork as substring', () => { + const content = '---\ncontext: forked\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline when context field is in body not frontmatter', () => { + const content = '---\nname: test\n---\ncontext: fork\n# Not in frontmatter'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline for context: Fork (case-sensitive)', () => { + const content = '---\ncontext: Fork\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline for context: FORK (case-sensitive)', () => { + const content = '---\ncontext: FORK\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline when context key is part of another key', () => { + const content = '---\nmycontext: fork\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('handles context as first field in frontmatter', () => { + const content = '---\ncontext: fork\nname: test\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); + + test('handles context as last field in frontmatter', () => { + const content = '---\nname: test\ndescription: A skill\ncontext: fork\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); + + test('returns inline for empty frontmatter', () => { + const content = '---\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline for context with no value', () => { + const content = '---\ncontext:\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('returns inline when --- appears in body after valid frontmatter', () => { + const content = '---\nname: test\n---\nSome body\n---\ncontext: fork\n---'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('handles content with only frontmatter and no body', () => { + const content = '---\ncontext: fork\n---'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); + + test('handles multiline frontmatter values before context', () => { + const content = '---\ndescription: |\n This is a long\n multiline description\ncontext: fork\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); + + test('returns inline for context with quoted value', () => { + const content = '---\ncontext: "fork"\n---\n# My Skill'; + assert.strictEqual(parseSkillContext(content), 'inline'); + }); + + test('handles tab-indented context value', () => { + const content = '---\ncontext:\tfork\n---\n# Skill'; + assert.strictEqual(parseSkillContext(content), 'fork'); + }); +}); + +suite('listRelatedFilesRecursive', () => { + function makeReadDirectory(tree: Record): ReadDirectoryFn { + return async (uri: URI) => { + return tree[uri.path] ?? []; + }; + } + + const baseUri = URI.file('/skills/my-skill'); + + test('lists files excluding SKILL.md', async () => { + const readDir = makeReadDirectory({ + '/skills/my-skill': [ + ['SKILL.md', FileType.File], + ['helper.py', FileType.File], + ['README.md', FileType.File], + ], + }); + + const files: string[] = []; + await listRelatedFilesRecursive(baseUri, baseUri, files, readDir); + assert.deepStrictEqual(files, ['helper.py', 'README.md']); + }); + + test('excludes SKILL.md case-insensitively', async () => { + const readDir = makeReadDirectory({ + '/skills/my-skill': [ + ['skill.md', FileType.File], + ['Skill.MD', FileType.File], + ['data.json', FileType.File], + ], + }); + + const files: string[] = []; + await listRelatedFilesRecursive(baseUri, baseUri, files, readDir); + assert.deepStrictEqual(files, ['data.json']); + }); + + test('recurses into subdirectories', async () => { + const readDir = makeReadDirectory({ + '/skills/my-skill': [ + ['SKILL.md', FileType.File], + ['lib', FileType.Directory], + ], + '/skills/my-skill/lib': [ + ['utils.ts', FileType.File], + ], + }); + + const files: string[] = []; + await listRelatedFilesRecursive(baseUri, baseUri, files, readDir); + assert.deepStrictEqual(files, ['lib/utils.ts']); + }); + + test('skips blacklisted directories', async () => { + const readDir = makeReadDirectory({ + '/skills/my-skill': [ + ['index.ts', FileType.File], + ['node_modules', FileType.Directory], + ['.git', FileType.Directory], + ['src', FileType.Directory], + ], + '/skills/my-skill/node_modules': [ + ['pkg.json', FileType.File], + ], + '/skills/my-skill/.git': [ + ['HEAD', FileType.File], + ], + '/skills/my-skill/src': [ + ['main.ts', FileType.File], + ], + }); + + const files: string[] = []; + await listRelatedFilesRecursive(baseUri, baseUri, files, readDir); + assert.deepStrictEqual(files, ['index.ts', 'src/main.ts']); + }); + + test('respects max depth of 5', async () => { + // Build a tree 7 levels deep + const tree: Record = {}; + for (let i = 0; i <= 7; i++) { + const path = '/skills/my-skill' + '/d'.repeat(i); + const entries: [string, FileType][] = [ + [`file${i}.txt`, FileType.File], + ]; + if (i < 7) { + entries.push(['d', FileType.Directory]); + } + tree[path] = entries; + } + + const readDir = makeReadDirectory(tree); + const files: string[] = []; + await listRelatedFilesRecursive(baseUri, baseUri, files, readDir); + + // depth 0 (root) through depth 5 should have files (6 files total) + // depth 6+ should be skipped + assert.strictEqual(files.length, 6); + assert.ok(files.includes('file0.txt')); + assert.ok(files.includes('d/d/d/d/d/file5.txt')); + assert.ok(!files.some(f => f.includes('file6.txt'))); + }); + + test('stops at MAX_RELATED_FILES (50)', async () => { + const entries: [string, FileType][] = []; + for (let i = 0; i < 60; i++) { + entries.push([`file${i}.txt`, FileType.File]); + } + + const readDir = makeReadDirectory({ + '/skills/my-skill': entries, + }); + + const files: string[] = []; + await listRelatedFilesRecursive(baseUri, baseUri, files, readDir); + assert.strictEqual(files.length, 50); + }); +}); + +suite('listRelatedFiles', () => { + test('returns empty array on error', async () => { + const readDir: ReadDirectoryFn = async () => { + throw new Error('ENOENT'); + }; + + const result = await listRelatedFiles(URI.file('/nonexistent'), readDir); + assert.deepStrictEqual(result, []); + }); + + test('returns files from flat directory', async () => { + const readDir: ReadDirectoryFn = async () => [ + ['SKILL.md', FileType.File], + ['template.txt', FileType.File], + ]; + + const result = await listRelatedFiles(URI.file('/skills/test'), readDir); + assert.deepStrictEqual(result, ['template.txt']); + }); +}); + +suite('resolveSkillUri', () => { + const skillA = URI.file('/skills/alpha/SKILL.md'); + const skillB = URI.file('/skills/beta/SKILL.md'); + const skillC = URI.file('/skills/gamma/SKILL.md'); + + function makeSkillIndex(...uris: URI[]): { readonly skills: Iterable } { + return { skills: uris }; + } + + function makeGetSkillInfo(map: Map): (uri: URI) => { readonly skillName: string } | undefined { + return (uri: URI) => { + const name = map.get(uri.toString()); + return name ? { skillName: name } : undefined; + }; + } + + test('resolves matching skill by name', () => { + const infoMap = new Map([ + [skillA.toString(), 'alpha'], + [skillB.toString(), 'beta'], + ]); + const result = resolveSkillUri( + 'beta', + 'index-content', + () => makeSkillIndex(skillA, skillB), + makeGetSkillInfo(infoMap), + ); + assert.strictEqual(result.toString(), skillB.toString()); + }); + + test('resolves first skill when only one exists', () => { + const infoMap = new Map([[skillA.toString(), 'alpha']]); + const result = resolveSkillUri( + 'alpha', + 'index-content', + () => makeSkillIndex(skillA), + makeGetSkillInfo(infoMap), + ); + assert.strictEqual(result.toString(), skillA.toString()); + }); + + test('throws when skill name not found, lists available skills', () => { + const infoMap = new Map([ + [skillA.toString(), 'alpha'], + [skillB.toString(), 'beta'], + [skillC.toString(), 'gamma'], + ]); + assert.throws( + () => resolveSkillUri( + 'nonexistent', + 'index-content', + () => makeSkillIndex(skillA, skillB, skillC), + makeGetSkillInfo(infoMap), + ), + (err: Error) => { + assert.ok(err.message.includes('"nonexistent"')); + assert.ok(err.message.includes('Available skills: alpha, beta, gamma')); + return true; + } + ); + }); + + test('throws when indexValue is undefined', () => { + assert.throws( + () => resolveSkillUri( + 'any-skill', + undefined, + () => makeSkillIndex(), + () => undefined, + ), + (err: Error) => { + assert.ok(err.message.includes('"any-skill" not found')); + assert.ok(!err.message.includes('Available skills')); + return true; + } + ); + }); + + test('throws with no available skills when index has no skills', () => { + assert.throws( + () => resolveSkillUri( + 'missing', + 'index-content', + () => makeSkillIndex(), + () => undefined, + ), + (err: Error) => { + assert.ok(err.message.includes('"missing" not found')); + assert.ok(!err.message.includes('Available skills')); + return true; + } + ); + }); + + test('skips skills where getSkillInfo returns undefined', () => { + const infoMap = new Map([ + [skillB.toString(), 'beta'], + ]); + // skillA has no info, only skillB does + assert.throws( + () => resolveSkillUri( + 'nonexistent', + 'index-content', + () => makeSkillIndex(skillA, skillB), + makeGetSkillInfo(infoMap), + ), + (err: Error) => { + // Only beta should appear in available skills + assert.ok(err.message.includes('Available skills: beta')); + assert.ok(!err.message.includes('alpha')); + return true; + } + ); + }); + + test('returns first match when multiple skills have same name', () => { + const uri1 = URI.file('/skills/a/SKILL.md'); + const uri2 = URI.file('/skills/b/SKILL.md'); + const result = resolveSkillUri( + 'duplicate', + 'index-content', + () => makeSkillIndex(uri1, uri2), + () => ({ skillName: 'duplicate' }), + ); + assert.strictEqual(result.toString(), uri1.toString()); + }); + + test('passes index text to parseInstructionIndexFile', () => { + let receivedText: string | undefined; + const infoMap = new Map([[skillA.toString(), 'alpha']]); + resolveSkillUri( + 'alpha', + 'my-special-index-content', + (text) => { receivedText = text; return makeSkillIndex(skillA); }, + makeGetSkillInfo(infoMap), + ); + assert.strictEqual(receivedText, 'my-special-index-content'); + }); + + test('does not call parseInstructionIndexFile when indexValue is undefined', () => { + let wasCalled = false; + assert.throws( + () => resolveSkillUri( + 'skill', + undefined, + () => { wasCalled = true; return makeSkillIndex(); }, + () => undefined, + ), + ); + assert.strictEqual(wasCalled, false); + }); + + test('matches skill name exactly (case-sensitive)', () => { + const infoMap = new Map([[skillA.toString(), 'Alpha']]); + assert.throws( + () => resolveSkillUri( + 'alpha', + 'index-content', + () => makeSkillIndex(skillA), + makeGetSkillInfo(infoMap), + ), + (err: Error) => { + assert.ok(err.message.includes('Available skills: Alpha')); + return true; + } + ); + }); +}); diff --git a/extensions/copilot/src/platform/configuration/common/configurationService.ts b/extensions/copilot/src/platform/configuration/common/configurationService.ts index 8857f9e44ffde..4ce1093b53469 100644 --- a/extensions/copilot/src/platform/configuration/common/configurationService.ts +++ b/extensions/copilot/src/platform/configuration/common/configurationService.ts @@ -662,6 +662,8 @@ export namespace ConfigKey { export const SearchSubagentThoroughnessEnabled = defineSetting('chat.searchSubagent.thoroughnessEnabled', ConfigType.ExperimentBased, false); export const ExecutionSubagentToolEnabled = defineSetting('chat.executionSubagent.enabled', ConfigType.ExperimentBased, false); + export const SkillToolEnabled = defineSetting('chat.skillTool.enabled', ConfigType.ExperimentBased, false); + /** Model to use for the execution subagent */ /** Use the agentic proxy for the execution subagent */ export const ExecutionSubagentUseAgenticProxy = defineSetting('chat.executionSubagent.useAgenticProxy', ConfigType.ExperimentBased, false); /** Model to use for the execution subagent. When useAgenticProxy is true, defaults to 'exec-subagent-router-a'. When false, defaults to the main agent model. */ diff --git a/src/vs/sessions/browser/parts/media/sidebarPart.css b/src/vs/sessions/browser/parts/media/sidebarPart.css index 43e3e446665cc..28f63b4d66872 100644 --- a/src/vs/sessions/browser/parts/media/sidebarPart.css +++ b/src/vs/sessions/browser/parts/media/sidebarPart.css @@ -70,18 +70,42 @@ /* ---- Phone Layout: Sidebar Drawer Overlay ---- */ -/* On phone, the sidebar is a drawer that slides over the chat. - It takes 85% width (max 360px) and sits on top of everything. */ +/* On phone, the sidebar is a full-width drawer that slides over the chat. + It covers the full viewport below the mobile top bar; the top bar's + sidebar toggle button remains visible and is used to dismiss it. + + The drawer slides in/out with a transition (not a keyframe animation) so + that interrupted toggles retarget smoothly from the current position + rather than restarting. The split-view wrapper toggles `display: none` + via a `.visible` class; `transition-behavior: allow-discrete` defers + the discrete `display` change until the slide-out completes, and + `@starting-style` provides the offscreen origin for the slide-in. */ .agent-sessions-workbench.phone-layout .split-view-view:has(> .part.sidebar) { position: absolute !important; top: 0 !important; left: 0 !important; bottom: 0 !important; - width: 85% !important; - max-width: 360px !important; + width: 100% !important; height: 100% !important; z-index: 250; - animation: sidebar-slide-in 200ms ease-out; + transform: translateX(0); + transition: + transform 260ms cubic-bezier(0.32, 0.72, 0, 1), + display 260ms allow-discrete; +} + +/* Slide-in starting state (applies on each transition into .visible) */ +@starting-style { + .agent-sessions-workbench.phone-layout .split-view-view.visible:has(> .part.sidebar) { + transform: translateX(-100%); + } +} + +/* Slide-out target: when `.visible` is removed, splitview's own rule sets + `display: none`. With `allow-discrete` above, the transform animates first + and the discrete `display` swap happens at the end of the transition. */ +.agent-sessions-workbench.phone-layout .split-view-view:not(.visible):has(> .part.sidebar) { + transform: translateX(-100%); } /* The sidebar Part inside fills its container */ @@ -90,32 +114,12 @@ height: 100%; } -@keyframes sidebar-slide-in { - from { - transform: translateX(-100%); - } - to { - transform: translateX(0); +@media (prefers-reduced-motion: reduce) { + .agent-sessions-workbench.phone-layout .split-view-view:has(> .part.sidebar) { + transition: none; } } -/* Sidebar backdrop — applied via JS when sidebar is open on phone */ -.mobile-sidebar-backdrop { - position: absolute; - top: 0; - left: 0; - right: 0; - bottom: 0; - background: rgba(0, 0, 0, 0.5); - z-index: 240; - animation: backdrop-fade-in 200ms ease-out; -} - -@keyframes backdrop-fade-in { - from { opacity: 0; } - to { opacity: 1; } -} - /* Increase sidebar footer action button height for touch */ .agent-sessions-workbench.phone-layout .part.sidebar > .sidebar-footer .sidebar-action-button { min-height: 44px; diff --git a/src/vs/sessions/browser/parts/mobile/mobileChatShell.css b/src/vs/sessions/browser/parts/mobile/mobileChatShell.css index 72eddfcfe05a4..2f79a84a73e25 100644 --- a/src/vs/sessions/browser/parts/mobile/mobileChatShell.css +++ b/src/vs/sessions/browser/parts/mobile/mobileChatShell.css @@ -3,6 +3,21 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +/* + * `!important` policy for this file: + * + * Only use `!important` when fighting one of: + * 1. SplitView.View.layoutContainer — sets inline position/top/left/width/height + * on every `.split-view-view` (src/vs/base/browser/ui/splitview/splitview.ts). + * 2. Part.layoutContents — inlines width/height on `.part > .content` via size(). + * 3. An equal-specificity desktop rule in the main workbench stylesheet. + * + * Rules that only face lower-specificity desktop CSS (e.g. a single `.part.X` + * selector) do NOT need `!important` — the `.phone-layout` class on the + * workbench root already wins. When adding a new rule, omit `!important` + * first and only add it if DevTools shows an actual override. + */ + /* ---- Mobile Top Bar ---- */ .mobile-top-bar { @@ -114,52 +129,21 @@ /* On phone, stack the mobile top bar and grid vertically */ .agent-sessions-workbench.phone-layout { - display: flex !important; - flex-direction: column !important; - overflow: hidden !important; -} - -/* On phone, split-view-views that directly contain a Part fill the full - grid area. Uses :has(> .part) to target only part containers — NOT - nested split-views inside parts' own content. */ -.agent-sessions-workbench.phone-layout .split-view-view:has(> .part) { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; -} - -/* The grid's own branch nodes (NOT those inside parts) need full sizing. - Target only direct children of the grid root. */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; -} - -/* Split-view-views inside the grid root that contain branch nodes */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view:has(> .monaco-grid-branch-node) { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; -} - -/* Second-level grid branch nodes */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; + display: flex; + flex-direction: column; + overflow: hidden; } -/* Third-level (top-right section) */ -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view:has(> .monaco-grid-branch-node) { +/* On phone, all split-view-view wrappers inside the grid — both those + wrapping parts AND those wrapping nested grid branch nodes — fill the + full grid area. This collapses the multi-axis grid into a single + full-screen viewport so parts overlap rather than share horizontal + space. The sidebar uses its own z-index + transform to slide over + the chat (see sidebarPart.css). The :has() conditions scope strictly + to grid wrappers so splitviews used inside individual parts' content + (e.g. a sidebar's view list) are not affected. */ +.agent-sessions-workbench.phone-layout .split-view-view:has(> .part), +.agent-sessions-workbench.phone-layout .split-view-view:has(> .monaco-grid-branch-node) { position: absolute !important; top: 0 !important; left: 0 !important; @@ -167,42 +151,52 @@ height: 100% !important; } -.agent-sessions-workbench.phone-layout > .monaco-grid-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node > .monaco-split-view2 > .split-view-container > .split-view-view > .monaco-grid-branch-node { - position: absolute !important; - top: 0 !important; - left: 0 !important; - width: 100% !important; - height: 100% !important; +/* All grid branch nodes fill their parent. `.monaco-grid-branch-node` is + specific to the grid widget, so this descendant selector won't hit + splitviews used inside individual parts' content. The grid widget + does not write inline geometry to branch nodes (only to the wrapping + `.split-view-view`), so plain CSS suffices here. */ +.agent-sessions-workbench.phone-layout .monaco-grid-branch-node { + position: absolute; + top: 0; + left: 0; + width: 100%; + height: 100%; } -/* Remove card appearance from ALL parts on phone */ +/* Remove card appearance from ALL parts on phone. + Specificity wins over the desktop card rule in style.css without !important; + width/height match what the mobile Part.layout() already inlines. */ .agent-sessions-workbench.phone-layout .part.chatbar, .agent-sessions-workbench.phone-layout .part.sidebar, .agent-sessions-workbench.phone-layout .part.auxiliarybar, .agent-sessions-workbench.phone-layout .part.panel { - margin: 0 !important; - border: none !important; - border-radius: 0 !important; - box-shadow: none !important; - --part-border-color: transparent !important; - width: 100% !important; - height: 100% !important; + margin: 0; + border: none; + border-radius: 0; + box-shadow: none; + --part-border-color: transparent; + width: 100%; + height: 100%; } -/* Force content div inside parts to fill the part on phone. - Part.layoutContents() sets inline width/height via size(), which - may use the grid-allocated dimensions rather than the CSS-overridden - 100% dimensions. Override with !important. */ +/* Pin Part content to the wrapper's full width on phone. + `!important` is required because `Part.layoutContents()` inlines the + width on `.part > .content` based on the splitview size (rule #2 in the + policy above). Without this, opening the sidebar — which makes the + splitview share space between sidebar and chatbar — would shrink the + chatbar's content during the drawer slide animation. */ .agent-sessions-workbench.phone-layout .part.chatbar > .content, .agent-sessions-workbench.phone-layout .part.sidebar > .content, .agent-sessions-workbench.phone-layout .part.auxiliarybar > .content, .agent-sessions-workbench.phone-layout .part.panel > .content { width: 100% !important; + height: 100% !important; } /* Hide the session composite bar (Copilot CLI / Approvals / Branch) on phone */ .agent-sessions-workbench.phone-layout .session-composite-bar { - display: none !important; + display: none; } /* Ensure the grid view element doesn't overflow — flex child must shrink */ @@ -224,8 +218,8 @@ } .agent-sessions-workbench.phone-layout .interactive-session .interactive-input-part { - max-width: none !important; - padding-bottom: calc(10px + env(safe-area-inset-bottom)) !important; + max-width: none !important; /* fights equal-specificity rule in style.css */ + padding-bottom: calc(10px + env(safe-area-inset-bottom)); } /* Chat input minimum font size to prevent iOS auto-zoom */ @@ -247,7 +241,7 @@ } .agent-sessions-workbench.phone-layout .part.sidebar > .composite.title { - display: none !important; + display: none; } .agent-sessions-workbench.phone-layout .part.sidebar > .content { @@ -260,7 +254,7 @@ /* Customization toolbar: hidden on phone (opens editors, not mobile-compatible) */ .agent-sessions-workbench.phone-layout .part.sidebar .ai-customization-toolbar { - display: none !important; + display: none; } /* Make sidebar footer touch-friendly */ @@ -271,7 +265,7 @@ /* Hide the "+ Session" button in the sidebar on phone — replaced by top bar + button */ .agent-sessions-workbench.phone-layout .agent-sessions-new-button-container { - display: none !important; + display: none; } /* Hide sashes on phone */ @@ -291,7 +285,7 @@ /* On phone, push the chat input to the bottom of the chat area */ .agent-sessions-workbench.phone-layout .interactive-session .interactive-input-and-execute-toolbar { - margin-top: auto !important; + margin-top: auto; } /* ---- Phone Layout: Chat Welcome Page ---- */ @@ -351,7 +345,7 @@ } .agent-sessions-workbench.phone-layout .session-workspace-picker-label { - font-size: 18px !important; + font-size: 18px; opacity: 0.6; } @@ -370,12 +364,12 @@ /* Hide the local mode bar (Copilot CLI / Default Approvals / Branch) on phone */ .agent-sessions-workbench.phone-layout .new-chat-bottom-container { - display: none !important; + display: none; } /* Also hide the sessions-chat-widget's DnD overlay on phone */ .agent-sessions-workbench.phone-layout .sessions-chat-dnd-overlay { - display: none !important; + display: none; } /* Chat widget fills full width on phone */ diff --git a/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts b/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts index 550ac0944a315..38c4cb598138f 100644 --- a/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts +++ b/src/vs/sessions/browser/parts/mobile/mobileTitlebarPart.ts @@ -16,6 +16,7 @@ import { IInstantiationService } from '../../../../platform/instantiation/common import { HiddenItemStrategy, MenuWorkbenchToolBar } from '../../../../platform/actions/browser/toolbar.js'; import { ISessionsManagementService } from '../../../services/sessions/common/sessionsManagement.js'; import { IsNewChatSessionContext } from '../../../common/contextkeys.js'; +import { SideBarVisibleContext } from '../../../../workbench/common/contextkeys.js'; import { Menus } from '../../menus.js'; /** @@ -72,13 +73,28 @@ export class MobileTitlebarPart extends Disposable { this._register(toDisposable(() => this.element.remove())); parent.prepend(this.element); - // Hamburger button + // Sidebar toggle button. Uses the same icon as the desktop/web + // agents-app sidebar toggle and reflects open/closed state via the + // SideBarVisibleContext key. const hamburger = append(this.element, $('button.mobile-top-bar-button')); hamburger.setAttribute('aria-label', localize('mobileTopBar.openSessions', "Open sessions")); const hamburgerIcon = append(hamburger, $('span')); - hamburgerIcon.classList.add(...ThemeIcon.asClassNameArray(Codicon.menu)); + const closedIconClasses = ThemeIcon.asClassNameArray(Codicon.layoutSidebarLeftOff); + const openIconClasses = ThemeIcon.asClassNameArray(Codicon.layoutSidebarLeft); + hamburgerIcon.classList.add(...closedIconClasses); this._register(addDisposableListener(hamburger, EventType.CLICK, () => this._onDidClickHamburger.fire())); + const sidebarVisibleKeySet = new Set([SideBarVisibleContext.key]); + const updateSidebarIcon = () => { + const isOpen = !!SideBarVisibleContext.getValue(contextKeyService); + hamburgerIcon.classList.remove(...closedIconClasses, ...openIconClasses); + hamburgerIcon.classList.add(...(isOpen ? openIconClasses : closedIconClasses)); + hamburger.setAttribute('aria-label', isOpen + ? localize('mobileTopBar.closeSessions', "Close sessions") + : localize('mobileTopBar.openSessions', "Open sessions")); + }; + updateSidebarIcon(); + // Center slot: title and/or actions container (mutually exclusive) const center = append(this.element, $('div.mobile-top-bar-center')); @@ -126,6 +142,9 @@ export class MobileTitlebarPart extends Disposable { if (e.affectsSome(newChatKeySet)) { updateCenterMode(); } + if (e.affectsSome(sidebarVisibleKeySet)) { + updateSidebarIcon(); + } })); this._register(toolbar.onDidChangeMenuItems(() => updateCenterMode())); } diff --git a/src/vs/sessions/browser/web.main.ts b/src/vs/sessions/browser/web.main.ts index ca88efb89aee3..4dcabc35177ff 100644 --- a/src/vs/sessions/browser/web.main.ts +++ b/src/vs/sessions/browser/web.main.ts @@ -3,9 +3,28 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { joinPath } from '../../base/common/resources.js'; +import { onUnexpectedError } from '../../base/common/errors.js'; import { ServiceCollection } from '../../platform/instantiation/common/serviceCollection.js'; import { ILogService } from '../../platform/log/common/log.js'; +import { IRemoteAuthorityResolverService } from '../../platform/remote/common/remoteAuthorityResolver.js'; +import { IStorageService } from '../../platform/storage/common/storage.js'; +import { IUriIdentityService } from '../../platform/uriIdentity/common/uriIdentity.js'; +import { IAnyWorkspaceIdentifier, IWorkspaceContextService } from '../../platform/workspace/common/workspace.js'; +import { IWorkspaceTrustEnablementService, IWorkspaceTrustManagementService } from '../../platform/workspace/common/workspaceTrust.js'; +import { IBrowserWorkbenchEnvironmentService } from '../../workbench/services/environment/browser/environmentService.js'; +import { IWorkbenchConfigurationService } from '../../workbench/services/configuration/common/configuration.js'; +import { IUserDataProfileService } from '../../workbench/services/userDataProfile/common/userDataProfile.js'; +import { BrowserUserDataProfilesService } from '../../platform/userDataProfile/browser/userDataProfile.js'; +import { FileService } from '../../platform/files/common/fileService.js'; +import { IPolicyService } from '../../platform/policy/common/policy.js'; +import { IRemoteAgentService } from '../../workbench/services/remote/common/remoteAgentService.js'; +import { IWorkspaceEditingService } from '../../workbench/services/workspaces/common/workspaceEditing.js'; +import { WorkspaceTrustEnablementService, WorkspaceTrustManagementService } from '../../workbench/services/workspaces/common/workspaceTrust.js'; import { BrowserMain, IBrowserMainWorkbench } from '../../workbench/browser/web.main.js'; +import { getWorkspaceIdentifier } from '../../workbench/services/workspaces/browser/workspaces.js'; +import { SessionsWorkspaceContextService } from '../services/workspace/browser/workspaceContextService.js'; +import { ConfigurationService } from '../services/configuration/browser/configurationService.js'; import { Workbench as SessionsWorkbench } from './workbench.js'; export class SessionsBrowserMain extends BrowserMain { @@ -13,4 +32,63 @@ export class SessionsBrowserMain extends BrowserMain { protected override createWorkbench(domElement: HTMLElement, serviceCollection: ServiceCollection, logService: ILogService): IBrowserMainWorkbench { return new SessionsWorkbench(domElement, undefined, serviceCollection, logService); } + + protected override async createWorkspaceConfigAndStorageServices( + serviceCollection: ServiceCollection, + _workspace: IAnyWorkspaceIdentifier, + environmentService: IBrowserWorkbenchEnvironmentService, + userDataProfileService: IUserDataProfileService, + _userDataProfilesService: BrowserUserDataProfilesService, + fileService: FileService, + _remoteAgentService: IRemoteAgentService, + uriIdentityService: IUriIdentityService, + policyService: IPolicyService, + logService: ILogService, + remoteAuthorityResolverService: IRemoteAuthorityResolverService, + ): Promise<{ configurationService: IWorkbenchConfigurationService; storageService: IStorageService }> { + // Use sessions workspace/configuration services instead of the standard + // WorkspaceService. This mirrors what SessionsMain does on desktop: + // the agents window manages workspace folders in-memory without creating + // workspace file watchers or other resources. + + // Workspace — use a stable synthetic workspace identifier for agents + const sessionsWorkspaceUri = joinPath(environmentService.userRoamingDataHome, 'agent-sessions.code-workspace'); + const workspaceIdentifier = getWorkspaceIdentifier(sessionsWorkspaceUri); + const workspaceContextService = new SessionsWorkspaceContextService(workspaceIdentifier, uriIdentityService); + + serviceCollection.set(IWorkspaceContextService, workspaceContextService); + serviceCollection.set(IWorkspaceEditingService, workspaceContextService); + + // Configuration — the sessions ConfigurationService works against the + // in-memory workspace model rather than a real .code-workspace file on disk. + const configurationService = new ConfigurationService( + userDataProfileService, + workspaceContextService, + uriIdentityService, + fileService, + policyService, + logService + ); + + try { + await configurationService.initialize(); + } catch (error) { + onUnexpectedError(error); + } + + serviceCollection.set(IWorkbenchConfigurationService, configurationService); + + // Storage + const storageService = await this.createStorageService(workspaceIdentifier, logService, userDataProfileService); + serviceCollection.set(IStorageService, storageService); + + // Workspace Trust Service + const workspaceTrustEnablementService = new WorkspaceTrustEnablementService(configurationService, environmentService); + serviceCollection.set(IWorkspaceTrustEnablementService, workspaceTrustEnablementService); + + const workspaceTrustManagementService = new WorkspaceTrustManagementService(configurationService, remoteAuthorityResolverService, storageService, uriIdentityService, environmentService, workspaceContextService, workspaceTrustEnablementService, fileService); + serviceCollection.set(IWorkspaceTrustManagementService, workspaceTrustManagementService); + + return { configurationService, storageService }; + } } diff --git a/src/vs/sessions/browser/workbench.ts b/src/vs/sessions/browser/workbench.ts index de3c9c25d0f43..01bd31f30a891 100644 --- a/src/vs/sessions/browser/workbench.ts +++ b/src/vs/sessions/browser/workbench.ts @@ -7,7 +7,7 @@ import '../../workbench/browser/style.js'; import './media/style.css'; import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../base/common/lifecycle.js'; import { Emitter, Event, setGlobalLeakWarningThreshold } from '../../base/common/event.js'; -import { getActiveDocument, getActiveElement, getClientArea, getWindowId, getWindows, IDimension, isAncestorUsingFlowTo, isHTMLElement, size, Dimension, runWhenWindowIdle, addDisposableListener, EventType } from '../../base/browser/dom.js'; +import { getActiveDocument, getActiveElement, getClientArea, getWindowId, getWindows, IDimension, isAncestorUsingFlowTo, isHTMLElement, size, Dimension, runWhenWindowIdle } from '../../base/browser/dom.js'; import { DeferredPromise, RunOnceScheduler } from '../../base/common/async.js'; import { isFullscreen, onDidChangeFullscreen, isChrome, isFirefox, isSafari } from '../../base/browser/browser.js'; import { mark } from '../../base/common/performance.js'; @@ -697,9 +697,6 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic })); } - private sidebarDrawerBackdrop: HTMLElement | undefined; - private readonly sidebarDrawerBackdropDisposables = this._register(new DisposableStore()); - private toggleMobileSidebarDrawer(): void { const isOpen = this.partVisibility.sidebar; if (isOpen) { @@ -710,17 +707,6 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic } private openMobileSidebarDrawer(): void { - // Show backdrop — created fresh each open so its click listener is - // tracked by a DisposableStore and cleaned up on close. - if (!this.sidebarDrawerBackdrop) { - const backdrop = document.createElement('div'); - backdrop.className = 'mobile-sidebar-backdrop'; - this.sidebarDrawerBackdropDisposables.add(addDisposableListener(backdrop, EventType.CLICK, () => this.closeMobileSidebarDrawer())); - this.sidebarDrawerBackdropDisposables.add(toDisposable(() => backdrop.remove())); - this.sidebarDrawerBackdrop = backdrop; - } - this.mainContainer.appendChild(this.sidebarDrawerBackdrop); - // Push a history entry so the Android back button dismisses the drawer. // Must come before setSideBarHidden(false) so layoutMobileSidebar() sees // the drawer state. @@ -729,16 +715,13 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic } // Show sidebar in grid — the actual drawer dimensions are applied by - // layoutMobileSidebar() from within layout(), which respects the - // "drawer" shape on phone (85% width, below the mobile top bar). + // layoutMobileSidebar() from within layout(), which uses the full + // viewport width below the mobile top bar on phone. The toggle button + // in the top bar remains visible and is used to close the drawer. this.setSideBarHidden(false); } private closeMobileSidebarDrawer(): void { - // Remove backdrop and dispose its listener. - this.sidebarDrawerBackdropDisposables.clear(); - this.sidebarDrawerBackdrop = undefined; - // Hide sidebar in grid this.setSideBarHidden(true); @@ -1272,32 +1255,27 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic return; } - // Only phone uses the overlay drawer shape. Tablet/desktop let the - // grid position the sidebar normally, so clear any inline styles. + // On phone the sidebar renders as a full-viewport overlay drawer. + // Geometry is fully expressed in CSS — see + // `mobileChatShell.css` (split-view-view fills the grid) and + // `sidebarPart.css` (drawer animation, z-index). We avoid setting + // inline position/size styles here because writing them after the + // grid has already laid out and painted the sidebar causes a + // visible one-frame snap on toggle. const isPhone = this.layoutPolicy.viewportClass.get() === 'phone'; if (!isPhone || !this.partVisibility.sidebar) { sidebarContainer.classList.remove('mobile-overlay-sidebar'); - sidebarContainer.style.position = ''; - sidebarContainer.style.top = ''; - sidebarContainer.style.left = ''; - sidebarContainer.style.width = ''; - sidebarContainer.style.height = ''; - sidebarContainer.style.zIndex = ''; return; } - // Phone drawer: 85% width (capped at 360px), positioned below the - // mobile top bar (the grid titlebar is hidden on phone). + sidebarContainer.classList.add('mobile-overlay-sidebar'); + + // Re-layout the sidebar Part with the drawer's content dimensions + // so its internal composite/list sizing matches the CSS-positioned + // drawer (grid area minus the mobile top bar). const topBarHeight = this.mobileTopBarElement?.offsetHeight ?? 48; - const drawerWidth = Math.min(Math.floor(this._mainContainerDimension.width * 0.85), 360); + const drawerWidth = this._mainContainerDimension.width; const drawerHeight = Math.max(0, this._mainContainerDimension.height - topBarHeight); - sidebarContainer.classList.add('mobile-overlay-sidebar'); - sidebarContainer.style.position = 'fixed'; - sidebarContainer.style.top = `${topBarHeight}px`; - sidebarContainer.style.left = '0'; - sidebarContainer.style.width = `${drawerWidth}px`; - sidebarContainer.style.height = `${drawerHeight}px`; - sidebarContainer.style.zIndex = '30'; sidebarPart.layout(drawerWidth, drawerHeight, topBarHeight, 0); } diff --git a/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts b/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts index 6a1fe3d3ae258..293ec68110f07 100644 --- a/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts +++ b/src/vs/sessions/contrib/accountMenu/browser/account.contribution.ts @@ -757,6 +757,50 @@ class TitleBarUpdateWidget extends BaseActionViewItem { // --- Register custom view item --- // +// These actions are registered at module level (not lazily inside AccountWidgetContribution) +// so that Menus.TitleBarRightLayout is non-empty when the MenuWorkbenchToolBar is first +// constructed. If they were registered lazily (WorkbenchPhase.AfterRestored), the toolbar's +// IntersectionObserver would have already observed an empty, display:none container and +// stopped listening for menu changes — causing the widgets to never appear. +// +// The actual widget rendering and interaction is handled by TitleBarUpdateWidget / +// TitleBarAccountWidget, which are custom view items registered via IActionViewItemService +// in AccountWidgetContribution. Those widgets attach their own DOM click handlers, so +// run() here is intentionally a no-op. +registerAction2(class extends Action2 { + constructor() { + super({ + id: SessionsTitleBarUpdateWidgetAction, + title: localize2('agentsUpdateTitleBar', "Agents Update"), + menu: { + id: Menus.TitleBarRightLayout, + group: 'navigation', + order: 99, + when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated()), + } + }); + } + + run(): void { } +}); + +registerAction2(class extends Action2 { + constructor() { + super({ + id: SessionsTitleBarAccountWidgetAction, + title: localize2('agentsAccountStatusTitleBar', "Agents Account and Status"), + menu: { + id: Menus.TitleBarRightLayout, + group: 'navigation', + order: 100, + when: IsAuxiliaryWindowContext.toNegated(), + } + }); + } + + run(): void { } +}); + class AccountWidgetContribution extends Disposable implements IWorkbenchContribution { static readonly ID = 'workbench.contrib.sessionsWidget'; @@ -768,57 +812,14 @@ class AccountWidgetContribution extends Disposable implements IWorkbenchContribu super(); // Titlebar update widget (to the right of separator, left of account badge) - let updateWidget: TitleBarUpdateWidget | undefined; this._register(actionViewItemService.register(Menus.TitleBarRightLayout, SessionsTitleBarUpdateWidgetAction, (action, options) => { - updateWidget = instantiationService.createInstance(TitleBarUpdateWidget, action, options); - return updateWidget; + return instantiationService.createInstance(TitleBarUpdateWidget, action, options); }, undefined)); - this._register(registerAction2(class extends Action2 { - constructor() { - super({ - id: SessionsTitleBarUpdateWidgetAction, - title: localize2('agentsUpdateTitleBar', "Agents Update"), - menu: { - id: Menus.TitleBarRightLayout, - group: 'navigation', - order: 99, - when: ContextKeyExpr.and(IsAuxiliaryWindowContext.toNegated(), SessionsWelcomeVisibleContext.toNegated()), - } - }); - } - - async run(): Promise { - updateWidget?.onClick(); - } - })); - // Titlebar account widget (rightmost in titlebar) - let accountWidget: TitleBarAccountWidget | undefined; this._register(actionViewItemService.register(Menus.TitleBarRightLayout, SessionsTitleBarAccountWidgetAction, (action, options) => { - accountWidget = instantiationService.createInstance(TitleBarAccountWidget, action, options); - return accountWidget; + return instantiationService.createInstance(TitleBarAccountWidget, action, options); }, undefined)); - - this._register(registerAction2(class extends Action2 { - constructor() { - super({ - id: SessionsTitleBarAccountWidgetAction, - title: localize2('agentsAccountStatusTitleBar', "Agents Account and Status"), - menu: { - id: Menus.TitleBarRightLayout, - group: 'navigation', - order: 100, - when: IsAuxiliaryWindowContext.toNegated(), - } - }); - } - - async run(): Promise { - accountWidget?.onClick(); - } - })); - } } diff --git a/src/vs/workbench/browser/web.main.ts b/src/vs/workbench/browser/web.main.ts index cff8e2c38be4c..1dfedf4e6ec76 100644 --- a/src/vs/workbench/browser/web.main.ts +++ b/src/vs/workbench/browser/web.main.ts @@ -368,27 +368,27 @@ export class BrowserMain extends Disposable { serviceCollection.set(IPolicyService, policyService); serviceCollection.set(IAccountPolicyGateService, policyService); - // Long running services (workspace, config, storage) - const [configurationService, storageService] = await Promise.all([ - this.createWorkspaceService(workspace, environmentService, userDataProfileService, userDataProfilesService, fileService, remoteAgentService, uriIdentityService, policyService, logService).then(service => { - - // Workspace - serviceCollection.set(IWorkspaceContextService, service); - - // Configuration - serviceCollection.set(IWorkbenchConfigurationService, service); - - return service; - }), - - this.createStorageService(workspace, logService, userDataProfileService).then(service => { + const configurationService = await this.createWorkspaceAndDependentServices(serviceCollection, workspace, environmentService, userDataProfileService, userDataProfilesService, fileService, remoteAgentService, uriIdentityService, policyService, logService, loggerService, remoteAuthorityResolverService, productService); - // Storage - serviceCollection.set(IStorageService, service); + return { serviceCollection, configurationService, logService }; + } - return service; - }) - ]); + private async createWorkspaceAndDependentServices( + serviceCollection: ServiceCollection, + workspace: IAnyWorkspaceIdentifier, + environmentService: IBrowserWorkbenchEnvironmentService, + userDataProfileService: IUserDataProfileService, + userDataProfilesService: BrowserUserDataProfilesService, + fileService: FileService, + remoteAgentService: IRemoteAgentService, + uriIdentityService: IUriIdentityService, + policyService: IPolicyService, + logService: ILogService, + loggerService: ILoggerService, + remoteAuthorityResolverService: IRemoteAuthorityResolverService, + productService: IProductService, + ): Promise { + const { configurationService, storageService } = await this.createWorkspaceConfigAndStorageServices(serviceCollection, workspace, environmentService, userDataProfileService, userDataProfilesService, fileService, remoteAgentService, uriIdentityService, policyService, logService, remoteAuthorityResolverService); // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! // @@ -399,17 +399,6 @@ export class BrowserMain extends Disposable { // // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - // Workspace Trust Service - const workspaceTrustEnablementService = new WorkspaceTrustEnablementService(configurationService, environmentService); - serviceCollection.set(IWorkspaceTrustEnablementService, workspaceTrustEnablementService); - - const workspaceTrustManagementService = new WorkspaceTrustManagementService(configurationService, remoteAuthorityResolverService, storageService, uriIdentityService, environmentService, configurationService, workspaceTrustEnablementService, fileService); - serviceCollection.set(IWorkspaceTrustManagementService, workspaceTrustManagementService); - - // Update workspace trust so that configuration is updated accordingly - configurationService.updateWorkspaceTrust(workspaceTrustManagementService.isWorkspaceTrusted()); - this._register(workspaceTrustManagementService.onDidChangeTrust(() => configurationService.updateWorkspaceTrust(workspaceTrustManagementService.isWorkspaceTrusted()))); - // Request Service const requestService = new BrowserRequestService(remoteAgentService, configurationService, loggerService); serviceCollection.set(IRequestService, requestService); @@ -436,7 +425,7 @@ export class BrowserMain extends Disposable { // Userdata Initialize Service const userDataInitializers: IUserDataInitializer[] = []; userDataInitializers.push(new UserDataSyncInitializer(environmentService, secretStorageService, userDataSyncStoreManagementService, fileService, userDataProfilesService, storageService, productService, requestService, logService, uriIdentityService)); - if (environmentService.options.profile) { + if (environmentService.options?.profile) { userDataInitializers.push(new UserDataProfileInitializer(environmentService, fileService, userDataProfileService, storageService, logService, uriIdentityService, requestService)); } const userDataInitializationService = new UserDataInitializationService(userDataInitializers); @@ -452,10 +441,55 @@ export class BrowserMain extends Disposable { logService.error(error); } - return { serviceCollection, configurationService, logService }; + return configurationService; } - private async initializeUserData(userDataInitializationService: UserDataInitializationService, configurationService: WorkspaceService) { + /** + * Creates and registers the workspace context, configuration, storage, + * and workspace trust services. Override this to provide different + * workspace/configuration implementations (e.g., sessions in-memory + * workspace). Overrides must also register workspace trust services. + */ + protected async createWorkspaceConfigAndStorageServices( + serviceCollection: ServiceCollection, + workspace: IAnyWorkspaceIdentifier, + environmentService: IBrowserWorkbenchEnvironmentService, + userDataProfileService: IUserDataProfileService, + userDataProfilesService: BrowserUserDataProfilesService, + fileService: FileService, + remoteAgentService: IRemoteAgentService, + uriIdentityService: IUriIdentityService, + policyService: IPolicyService, + logService: ILogService, + remoteAuthorityResolverService: IRemoteAuthorityResolverService, + ): Promise<{ configurationService: IWorkbenchConfigurationService; storageService: IStorageService }> { + const [configurationService, storageService] = await Promise.all([ + this.createWorkspaceService(workspace, environmentService, userDataProfileService, userDataProfilesService, fileService, remoteAgentService, uriIdentityService, policyService, logService).then(service => { + serviceCollection.set(IWorkspaceContextService, service); + serviceCollection.set(IWorkbenchConfigurationService, service); + return service; + }), + this.createStorageService(workspace, logService, userDataProfileService).then(service => { + serviceCollection.set(IStorageService, service); + return service; + }) + ]); + + // Workspace Trust Service + const workspaceTrustEnablementService = new WorkspaceTrustEnablementService(configurationService, environmentService); + serviceCollection.set(IWorkspaceTrustEnablementService, workspaceTrustEnablementService); + + const workspaceTrustManagementService = new WorkspaceTrustManagementService(configurationService, remoteAuthorityResolverService, storageService, uriIdentityService, environmentService, configurationService, workspaceTrustEnablementService, fileService); + serviceCollection.set(IWorkspaceTrustManagementService, workspaceTrustManagementService); + + // Update workspace trust so that configuration is updated accordingly + configurationService.updateWorkspaceTrust(workspaceTrustManagementService.isWorkspaceTrusted()); + this._register(workspaceTrustManagementService.onDidChangeTrust(() => configurationService.updateWorkspaceTrust(workspaceTrustManagementService.isWorkspaceTrusted()))); + + return { configurationService, storageService }; + } + + private async initializeUserData(userDataInitializationService: UserDataInitializationService, configurationService: IWorkbenchConfigurationService) { if (await userDataInitializationService.requiresInitialization()) { mark('code/willInitRequiredUserData'); @@ -464,7 +498,9 @@ export class BrowserMain extends Disposable { // Important: Reload only local user configuration after initializing // Reloading complete configuration blocks workbench until remote configuration is loaded. - await configurationService.reloadLocalUserConfiguration(); + if (configurationService instanceof WorkspaceService) { + await configurationService.reloadLocalUserConfiguration(); + } mark('code/didInitRequiredUserData'); } @@ -555,7 +591,7 @@ export class BrowserMain extends Disposable { })); } - private async createStorageService(workspace: IAnyWorkspaceIdentifier, logService: ILogService, userDataProfileService: IUserDataProfileService): Promise { + protected async createStorageService(workspace: IAnyWorkspaceIdentifier, logService: ILogService, userDataProfileService: IUserDataProfileService): Promise { const storageService = new BrowserStorageService(workspace, userDataProfileService, logService); try { diff --git a/src/vs/workbench/contrib/browserView/common/browserEditorInput.ts b/src/vs/workbench/contrib/browserView/common/browserEditorInput.ts index d20b820a2ee9a..bdfab1228a085 100644 --- a/src/vs/workbench/contrib/browserView/common/browserEditorInput.ts +++ b/src/vs/workbench/contrib/browserView/common/browserEditorInput.ts @@ -131,6 +131,14 @@ export class BrowserEditorInput extends EditorInput { return this._model ? this._model.favicon : this._initialData.favicon; } + /** + * Whether this editor was opened via a default localhost link open (setting + * not explicitly configured by the user). Transient — not serialized. + */ + get isDefaultLinkOpen(): boolean { + return !!this._initialData.isDefaultLinkOpen; + } + navigate(url: string): void { if (this._model) { void this._model.loadURL(url); diff --git a/src/vs/workbench/contrib/browserView/common/browserView.ts b/src/vs/workbench/contrib/browserView/common/browserView.ts index fcf544b9db45d..671d41a5658da 100644 --- a/src/vs/workbench/contrib/browserView/common/browserView.ts +++ b/src/vs/workbench/contrib/browserView/common/browserView.ts @@ -85,6 +85,14 @@ export interface IBrowserEditorViewState { readonly url?: string; readonly title?: string; readonly favicon?: string; + + /** + * When true, indicates that this browser tab was opened via the localhost + * link opener while the user has not explicitly configured the setting + * (i.e. the default value was used). This is a transient flag and is not + * serialized. + */ + readonly isDefaultLinkOpen?: boolean; } export const IBrowserViewWorkbenchService = createDecorator('browserViewWorkbenchService'); diff --git a/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts b/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts index 88a2eb5da0a23..5db998e533d2d 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts @@ -67,10 +67,10 @@ export abstract class BrowserEditorContribution extends Disposable { constructor(protected readonly editor: BrowserEditor) { super(); - this._register(editor.onDidChangeModel(model => { + this._register(editor.onDidChangeModel(({ model, isNew }) => { this._modelStore.clear(); if (model) { - this.subscribeToModel(model, this._modelStore); + this.subscribeToModel(model, this._modelStore, isNew); } else { this.clear(); } @@ -80,7 +80,7 @@ export abstract class BrowserEditorContribution extends Disposable { /** * Called whenever the editor model changes to update state. */ - protected subscribeToModel(_model: IBrowserViewModel, _store: DisposableStore): void { } + protected subscribeToModel(_model: IBrowserViewModel, _store: DisposableStore, _isNew: boolean): void { } /** * Called when the model is cleared to reset state. @@ -339,7 +339,10 @@ export class BrowserEditor extends EditorPane { private _model: IBrowserViewModel | undefined; get model(): IBrowserViewModel | undefined { return this._model; } - private readonly _onDidChangeModel = this._register(new Emitter()); + private readonly _onDidChangeModel = this._register(new Emitter<{ + model: IBrowserViewModel | undefined; + isNew: boolean; + }>()); readonly onDidChangeModel = this._onDidChangeModel.event; // -- State ---------------------------------------------------------- @@ -540,6 +543,7 @@ export class BrowserEditor extends EditorPane { this._inputDisposables.clear(); let model = input.model; + const isNew = !model; if (!model) { // Set initial navigation state from the input so that the UI is populated while the model is loading. this.updateNavigationState({ @@ -559,7 +563,7 @@ export class BrowserEditor extends EditorPane { } this._model = model; - this._onDidChangeModel.fire(model); + this._onDidChangeModel.fire({ model, isNew }); // Initialize UI state and context keys from model this.updateNavigationState({ @@ -1044,7 +1048,7 @@ export class BrowserEditor extends EditorPane { void this._model?.setVisible(false); this._model = undefined; - this._onDidChangeModel.fire(undefined); + this._onDidChangeModel.fire({ model: undefined, isNew: false }); this._canGoBackContext.reset(); this._canGoForwardContext.reset(); diff --git a/src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorChatFeatures.ts b/src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorChatFeatures.ts index 1820fcae23ea9..b10ee84e71a0d 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorChatFeatures.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorChatFeatures.ts @@ -227,7 +227,7 @@ export class BrowserEditorChatIntegration extends BrowserEditorContribution { } override get urlBarWidgets(): readonly IBrowserEditorWidgetContribution[] { - return [{ element: this._shareButtonContainer, order: 100 }]; + return [{ element: this._shareButtonContainer, order: 50 }]; } protected override subscribeToModel(model: IBrowserViewModel, store: DisposableStore): void { diff --git a/src/vs/workbench/contrib/browserView/electron-browser/features/browserTabManagementFeatures.ts b/src/vs/workbench/contrib/browserView/electron-browser/features/browserTabManagementFeatures.ts index 3b40e69a1273f..c4b8d60d0ec77 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/features/browserTabManagementFeatures.ts +++ b/src/vs/workbench/contrib/browserView/electron-browser/features/browserTabManagementFeatures.ts @@ -12,7 +12,7 @@ import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from '../../../../services/e import { IEditorGroupsService, GroupsOrder } from '../../../../services/editor/common/editorGroupsService.js'; import { EditorsOrder, GroupIdentifier } from '../../../../common/editor.js'; import { IQuickInputService, IQuickInputButton, IQuickPickItem, IQuickPickSeparator, QuickInputButtonLocation, IQuickPick } from '../../../../../platform/quickinput/common/quickInput.js'; -import { Disposable, DisposableStore } from '../../../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore, MutableDisposable } from '../../../../../base/common/lifecycle.js'; import { URI } from '../../../../../base/common/uri.js'; import { Codicon } from '../../../../../base/common/codicons.js'; import { ThemeIcon } from '../../../../../base/common/themables.js'; @@ -25,17 +25,26 @@ import { ITelemetryService } from '../../../../../platform/telemetry/common/tele import { ContextKeyExpr, IContextKeyService, RawContextKey } from '../../../../../platform/contextkey/common/contextkey.js'; import { BrowserViewCommandId } from '../../../../../platform/browserView/common/browserView.js'; import { IWorkbenchContribution, registerWorkbenchContribution2, WorkbenchPhase } from '../../../../common/contributions.js'; -import { IBrowserViewWorkbenchService } from '../../common/browserView.js'; +import { IBrowserViewModel, IBrowserViewWorkbenchService } from '../../common/browserView.js'; import { IConfigurationRegistry, Extensions as ConfigurationExtensions } from '../../../../../platform/configuration/common/configurationRegistry.js'; import { workbenchConfigurationNodeBase } from '../../../../common/configuration.js'; import { IExternalOpener, IOpenerService } from '../../../../../platform/opener/common/opener.js'; import { isLocalhostAuthority } from '../../../../../platform/url/common/trustedDomains.js'; -import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; +import { IConfigurationService, isConfigured } from '../../../../../platform/configuration/common/configuration.js'; import { ICommandService } from '../../../../../platform/commands/common/commands.js'; import { CancellationToken } from '../../../../../base/common/cancellation.js'; import { ToggleTitleBarConfigAction } from '../../../../browser/parts/titlebar/titlebarActions.js'; import { Registry } from '../../../../../platform/registry/common/platform.js'; import { match } from '../../../../../base/common/glob.js'; +import { $, addDisposableListener, EventType } from '../../../../../base/browser/dom.js'; +import { BrowserEditor, BrowserEditorContribution, IBrowserEditorWidgetContribution } from '../browserEditor.js'; +import { IHoverService } from '../../../../../platform/hover/browser/hover.js'; +import { HoverPosition } from '../../../../../base/browser/ui/hover/hoverWidget.js'; +import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js'; +import { IPreferencesService } from '../../../../services/preferences/common/preferences.js'; +import { disposableTimeout } from '../../../../../base/common/async.js'; +import { MarkdownString } from '../../../../../base/common/htmlContent.js'; +import { IsSessionsWindowContext } from '../../../../common/contextkeys.js'; const CONTEXT_BROWSER_EDITOR_OPEN = new RawContextKey('browserEditorOpen', false, localize('browser.editorOpen', "Whether any browser editor is currently open")); @@ -530,14 +539,139 @@ class LocalhostLinkOpenerContribution extends Disposable implements IWorkbenchCo logBrowserOpen(this.telemetryService, 'localhostLinkOpener'); + // Check whether the setting was explicitly set by the user or is still at its default value. + // When it is a default, tag the viewState so that the hint pill can be shown. + const isDefaultLinkOpen = !isConfigured(this.configurationService.inspect('workbench.browser.openLocalhostLinks')); + const browserUri = BrowserViewUri.forId(generateUuid()); - await this.editorService.openEditor({ resource: browserUri, options: { pinned: true, viewState: { url: href } } }); + await this.editorService.openEditor({ resource: browserUri, options: { pinned: true, viewState: { url: href, isDefaultLinkOpen } } }); return true; } } registerWorkbenchContribution2(LocalhostLinkOpenerContribution.ID, LocalhostLinkOpenerContribution, WorkbenchPhase.BlockStartup); +// ---- Link opened hint pill (URL bar widget) -------------------------------- + +const LOCALHOST_HINT_DISMISSED_KEY = 'workbench.browser.linkOpenedHintDismissed'; + +/** + * A small pill shown in the URL bar that informs the user their link was opened + * in the Integrated Browser by default. Clicking it shows a tooltip + * with an explanation and options to open settings or dismiss permanently. + */ +class LinkOpenedHintPill extends BrowserEditorContribution { + + private readonly _pill: HTMLElement; + private readonly _attentionTimeout = this._register(new MutableDisposable()); + + constructor( + editor: BrowserEditor, + @IHoverService private readonly hoverService: IHoverService, + @IStorageService private readonly storageService: IStorageService, + @IPreferencesService private readonly preferencesService: IPreferencesService, + @IContextKeyService private readonly contextKeyService: IContextKeyService + ) { + super(editor); + + this._pill = $('.browser-link-opened-hint-pill'); + this._pill.tabIndex = 0; + this._pill.role = 'button'; + this._pill.ariaLabel = localize('browser.linkOpenedHint.ariaLabel', "This link opened in the integrated browser"); + this._pill.ariaHidden = 'true'; + + const icon = $('span'); + icon.className = ThemeIcon.asClassName(Codicon.info); + const label = $('span'); + label.textContent = localize('browser.linkOpenedHint.label', "Link opened here"); + + this._pill.appendChild(icon); + this._pill.appendChild(label); + + const hoverOptions = () => ({ + content: new MarkdownString(localize('browser.linkOpenedHint.detail', "**Integrated Browser**\n\nLocalhost links automatically open in the integrated browser.")), + actions: [ + { + label: localize('browser.linkOpenedHint.openSettings', "Open Settings"), + commandId: 'workbench.action.openSettings', + iconClass: ThemeIcon.asClassName(Codicon.settingsGear), + run: () => { + this.preferencesService.openUserSettings({ query: 'workbench.browser.openLocalhostLinks' }); + } + }, + { + label: localize('browser.linkOpenedHint.dismiss', "Don't Show Again"), + commandId: '', + run: () => { + this._dismiss(); + } + } + ], + position: { hoverPosition: HoverPosition.BELOW } + }); + + this._register(this.hoverService.setupDelayedHover(this._pill, hoverOptions, { setupKeyboardEvents: true })); + this._register(addDisposableListener(this._pill, EventType.CLICK, () => { + this.hoverService.showInstantHover({ ...hoverOptions(), target: this._pill, persistence: { sticky: true } }, true); + })); + } + + override get urlBarWidgets(): readonly IBrowserEditorWidgetContribution[] { + return [{ element: this._pill, order: 100 }]; + } + + protected override subscribeToModel(_model: IBrowserViewModel, _store: DisposableStore, isNew: boolean): void { + if (IsSessionsWindowContext.getValue(this.contextKeyService)) { + this._setVisible(false); + return; + } + + const input = this.editor.input; + if (input instanceof BrowserEditorInput && input.isDefaultLinkOpen) { + const dismissed = this.storageService.getBoolean(LOCALHOST_HINT_DISMISSED_KEY, StorageScope.APPLICATION, false); + this._setVisible(!dismissed); + if (!dismissed && isNew) { + this._callAttention(); + } + } else { + this._setVisible(false); + } + } + + override clear(): void { + this._attentionTimeout.clear(); + this._setVisible(false); + } + + private _setVisible(visible: boolean): void { + if (!visible) { + this._attentionTimeout.clear(); + this._pill.classList.remove('attention'); + } + this._pill.classList.toggle('visible', visible); + this._pill.ariaHidden = visible ? 'false' : 'true'; + } + + private _callAttention(): void { + this._attentionTimeout.clear(); + this._pill.classList.remove('attention'); + // Start collapsed (icon only), expand after 300ms, then collapse back after another 2s + this._attentionTimeout.value = disposableTimeout(() => { + this._pill.classList.add('attention'); + this._attentionTimeout.value = disposableTimeout(() => { + this._pill.classList.remove('attention'); + }, 2000); + }, 300); + } + + private _dismiss(): void { + this.storageService.store(LOCALHOST_HINT_DISMISSED_KEY, true, StorageScope.APPLICATION, StorageTarget.USER); + this._setVisible(false); + } +} + +BrowserEditor.registerContribution(LinkOpenedHintPill); + Registry.as(ConfigurationExtensions.Configuration).registerConfiguration({ ...workbenchConfigurationNodeBase, properties: { diff --git a/src/vs/workbench/contrib/browserView/electron-browser/media/browser.css b/src/vs/workbench/contrib/browserView/electron-browser/media/browser.css index ba7f6e9d8ecb1..d90a81b267bfe 100644 --- a/src/vs/workbench/contrib/browserView/electron-browser/media/browser.css +++ b/src/vs/workbench/contrib/browserView/electron-browser/media/browser.css @@ -88,13 +88,15 @@ display: flex; align-items: center; flex-shrink: 0; + padding: 2px; + gap: 2px; } .browser-zoom-pill { display: none; align-items: center; gap: 2px; - margin: 0 2px; + margin: 0; padding: 0 2px; flex-shrink: 0; border-radius: var(--vscode-cornerRadius-small); @@ -113,10 +115,72 @@ } } + .browser-link-opened-hint-pill { + display: none; + align-items: center; + gap: 4px; + margin: 0; + padding: 3px 4px; + flex-shrink: 0; + border-radius: 9999px; + border: 1px solid var(--vscode-button-secondaryBorder, transparent); + background-color: var(--vscode-button-secondaryBackground); + color: var(--vscode-button-secondaryForeground); + font-size: 12px; + line-height: 1; + white-space: nowrap; + cursor: pointer; + overflow: hidden; + max-width: 14px; + transition: max-width 0.3s ease, padding 0.3s ease, background-color 0.3s ease, color 0.3s ease, border-color 0.3s ease; + position: relative; + + &.attention { + border-color: var(--vscode-button-border, transparent); + background-color: var(--vscode-button-background); + color: var(--vscode-button-foreground); + max-width: 200px; + padding: 3px 6px; + + span { + opacity: 1; + } + + &:hover { + background-color: var(--vscode-button-hoverBackground); + } + } + + &.visible { + display: flex; + } + + &:hover { + background-color: var(--vscode-button-secondaryHoverBackground); + } + + &:focus-visible { + outline: 1px solid var(--vscode-focusBorder); + outline-offset: 1px; + } + + span { + opacity: 0; + transition: opacity 0.3s ease; + } + + .codicon { + color: inherit; + font-size: 14px; + flex-shrink: 0; + opacity: 1; + } + } + .browser-share-toggle-container { display: flex; align-items: center; - margin: 2px 4px; + margin: 0; flex-shrink: 0; .browser-share-toggle { @@ -480,8 +544,8 @@ display: flex; align-items: center; justify-content: center; - width: 23px; - height: 23px; + width: 24px; + height: 24px; border-radius: var(--vscode-cornerRadius-small); cursor: pointer; background-color: var(--vscode-statusBarItem-errorBackground); diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts index 50d0b03e73125..84e5f7acd72d7 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/permissionPickerActionItem.ts @@ -17,7 +17,7 @@ import { IContextKeyService } from '../../../../../../platform/contextkey/common import { IKeybindingService } from '../../../../../../platform/keybinding/common/keybinding.js'; import { ITelemetryService } from '../../../../../../platform/telemetry/common/telemetry.js'; import { ChatConfiguration, ChatPermissionLevel } from '../../../common/constants.js'; -import { IChatSessionProviderOptionItem } from '../../../common/chatSessionsService.js'; +import { IChatSessionProviderOptionItem, SessionType } from '../../../common/chatSessionsService.js'; import { MenuItemAction } from '../../../../../../platform/actions/common/actions.js'; import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { IDialogService } from '../../../../../../platform/dialogs/common/dialogs.js'; @@ -275,7 +275,11 @@ export class PermissionPickerActionItem extends ChatInputPickerActionViewItem { class: undefined, enabled: true, run: async () => { - await openerService.open(URI.parse('https://code.visualstudio.com/docs/copilot/agents/agent-tools#_permission-levels')); + const ext = delegate.getExtensionPermissions?.(); + const url = ext?.sessionType === SessionType.ClaudeCode + ? 'https://code.claude.com/docs/en/permission-modes#available-modes' + : 'https://code.visualstudio.com/docs/copilot/agents/agent-tools#_permission-levels'; + await openerService.open(URI.parse(url)); } }], reporter: { id: 'ChatPermissionPicker', name: 'ChatPermissionPicker', includeOptions: true }, diff --git a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css index dc5a29920e3e8..235cd6b6a4a89 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css @@ -981,6 +981,15 @@ have to be updated for changes to the rules above, or to support more deeply nes overflow: visible; } +.monaco-workbench .interactive-session .chat-input-container.working.focused { + border-color: color-mix(in srgb, var(--vscode-focusBorder) 40%, transparent); +} + +.monaco-workbench .interactive-session .chat-input-container.working .interactive-input-editor .monaco-editor, +.monaco-workbench .interactive-session .chat-input-container.working .interactive-input-editor .monaco-editor .monaco-editor-background { + background-color: transparent; +} + .monaco-workbench .interactive-session .chat-input-container.working::before { opacity: 1; animation: chat-input-working-border-spin var(--chat-input-anim-duration) linear infinite; @@ -1513,7 +1522,8 @@ have to be updated for changes to the rules above, or to support more deeply nes across every view-line on each typed character. */ .chat-editor-container .monaco-editor [class^="ced-chat-session-detail"] { display: inline-block; - max-width: 100%; /* fallback for environments without container query units */ + max-width: 100%; + /* fallback for environments without container query units */ max-width: 100cqi; overflow: hidden; text-overflow: ellipsis; diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts index bb2d7dd5d67a4..a1f75a77843ab 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/computeAutomaticInstructions.ts @@ -342,6 +342,7 @@ export class ComputeAutomaticInstructions { private async _getCustomizationsIndex(instructionFiles: readonly IInstructionFile[], _existingVariables: ChatRequestVariableSet, telemetryEvent: InstructionsCollectionEvent, debugInfo: InstructionsCollectionDebugInfo, token: CancellationToken): Promise { const readTool = this._getTool('readFile'); const runSubagentTool = this._getTool(VSCodeToolReference.runSubagent); + const skillTool = this._getTool('skill'); const currentSessionType = this._currentSessionType; const remoteEnv = await this._remoteAgentService.getEnvironment(); @@ -428,34 +429,78 @@ export class ComputeAutomaticInstructions { } const useSkillAdherencePrompt = this._configurationService.getValue(PromptsConfig.USE_SKILL_ADHERENCE_PROMPT); + // When the skill tool is available, direct the model to use it by name + // instead of reading SKILL.md files directly. This keeps file paths out of + // the listing and routes through the proper skill loading pipeline. + const skillLoadTool = skillTool ?? readTool; entries.push(''); if (useSkillAdherencePrompt) { // Stronger skill adherence prompt for experimental feature entries.push('Skills provide specialized capabilities, domain knowledge, and refined workflows for producing high-quality outputs. Each skill folder contains tested instructions for specific domains like testing strategies, API design, or performance optimization. Multiple skills can be combined when a task spans different domains.'); - entries.push(`BLOCKING REQUIREMENT: When a skill applies to the user's request, you MUST load and read the SKILL.md file IMMEDIATELY as your first action, BEFORE generating any other response or taking action on the task. Use ${readTool.variable} to load the relevant skill(s).`); - entries.push('NEVER just mention or reference a skill in your response without actually reading it first. If a skill is relevant, load it before proceeding.'); + if (skillTool) { + entries.push(`BLOCKING REQUIREMENT: When a skill applies to the user's request, you MUST invoke it IMMEDIATELY as your first action, BEFORE generating any other response or taking action on the task. Use ${skillTool.variable} with the skill name to load the relevant skill(s).`); + } else { + entries.push(`BLOCKING REQUIREMENT: When a skill applies to the user's request, you MUST load and read the SKILL.md file IMMEDIATELY as your first action, BEFORE generating any other response or taking action on the task. Use ${readTool.variable} to load the relevant skill(s).`); + } + entries.push('NEVER just mention or reference a skill in your response without actually loading it first. If a skill is relevant, load it before proceeding.'); entries.push('How to determine if a skill applies:'); entries.push('1. Review the available skills below and match their descriptions against the user\'s request'); entries.push('2. If any skill\'s domain overlaps with the task, load that skill immediately'); entries.push('3. When multiple skills apply (e.g., a flowchart in documentation), load all relevant skills'); entries.push('Examples:'); - entries.push(`- "Help me write unit tests for this module" -> Load the testing skill via ${readTool.variable} FIRST, then proceed`); - entries.push(`- "Optimize this slow function" -> Load the performance-profiling skill via ${readTool.variable} FIRST, then proceed`); + entries.push(`- "Help me write unit tests for this module" -> Load the testing skill via ${skillLoadTool.variable} FIRST, then proceed`); + entries.push(`- "Optimize this slow function" -> Load the performance-profiling skill via ${skillLoadTool.variable} FIRST, then proceed`); entries.push(`- "Add a discount code field to checkout" -> Load both the checkout-flow and form-validation skills FIRST`); entries.push('Available skills:'); } else { - entries.push('Here is a list of skills that contain domain specific knowledge on a variety of topics.'); - entries.push('Each skill comes with a description of the topic and a file path that contains the detailed instructions.'); - entries.push(`When a user asks you to perform a task that falls within the domain of a skill, use the ${readTool.variable} tool to acquire the full instructions from the file URI.`); + if (skillTool) { + entries.push('Here is a list of skills that contain domain specific knowledge on a variety of topics.'); + entries.push(`When a user asks you to perform a task that falls within the domain of a skill, use the ${skillTool.variable} tool with the skill name to load it.`); + } else { + entries.push('Here is a list of skills that contain domain specific knowledge on a variety of topics.'); + entries.push('Each skill comes with a description of the topic and a file path that contains the detailed instructions.'); + entries.push(`When a user asks you to perform a task that falls within the domain of a skill, use the ${readTool.variable} tool to acquire the full instructions from the file URI.`); + } } - for (const skill of modelInvocableSkills) { - entries.push(''); - entries.push(`${skill.name}`); + const SKILL_DESCRIPTION_CHAR_BUDGET = 15000; + const TRUNCATED_NAMES_CHAR_BUDGET = 5000; + let skillCharCount = 0; + let truncatedAtIndex = modelInvocableSkills.length; + for (let i = 0; i < modelInvocableSkills.length; i++) { + const skill = modelInvocableSkills[i]; + const skillEntry = [``, `${skill.name}`]; if (skill.description) { - entries.push(`${skill.description}`); + skillEntry.push(`${skill.description}`); + } + skillEntry.push(`${filePath(skill.uri)}`); + skillEntry.push(``); + const entryLength = skillEntry.join('\n').length + 1; // +1 for joining newline + if (skillCharCount + entryLength > SKILL_DESCRIPTION_CHAR_BUDGET) { + truncatedAtIndex = i; + break; + } + skillCharCount += entryLength; + entries.push(...skillEntry); + } + // When skills are truncated by the character budget, include remaining + // skill names so the model can still discover and invoke them. + if (truncatedAtIndex < modelInvocableSkills.length) { + const truncatedSkills = modelInvocableSkills.slice(truncatedAtIndex); + const names: string[] = []; + let nameListLength = 0; + for (const skill of truncatedSkills) { + const addition = (names.length > 0 ? 2 : 0) + skill.name.length; + if (nameListLength + addition > TRUNCATED_NAMES_CHAR_BUDGET) { + break; + } + nameListLength += addition; + names.push(skill.name); } - entries.push(`${filePath(skill.uri)}`); - entries.push(''); + const remaining = truncatedSkills.length - names.length; + const nameList = names.join(', '); + entries.push(remaining > 0 + ? `Additional skills available (invoke by name): ${nameList}... and ${remaining} more` + : `Additional skills available (invoke by name): ${nameList}`); } entries.push('', '', ''); // add trailing newline } @@ -526,6 +571,7 @@ export class ComputeAutomaticInstructions { }; collectToolReference(readTool); collectToolReference(runSubagentTool); + collectToolReference(skillTool); return toPromptTextVariableEntry(content, true, toolReferences); } diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptFileAttributes.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptFileAttributes.ts index f2ca02874b4f4..108cb40e30830 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptFileAttributes.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptFileAttributes.ts @@ -204,6 +204,11 @@ export const skillAttributes: Record = { type: 'map', description: localize('promptHeader.skill.metadata', 'Additional metadata for the skill.'), }, + [PromptHeaderAttributes.context]: { + type: 'scalar', + description: localize('promptHeader.skill.context', 'Controls how the skill is loaded. Set to \'fork\' to spawn a subagent with the skill instructions instead of returning them inline.'), + enums: [{ name: 'fork', description: localize('promptHeader.skill.context.fork', 'Spawn a subagent with the skill instructions injected as system context.') }], + }, }; const allAttributeNames: Record = { diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts index d92b57bbf8d46..f44449e2ddc66 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/languageProviders/promptValidator.ts @@ -15,6 +15,7 @@ import { ILanguageModelToolsService, SpecedToolAliases } from '../../tools/langu import { PromptsType, Target } from '../promptTypes.js'; import { ISequenceValue, IHeaderAttribute, IScalarValue, parseCommaSeparatedList, ParsedPromptFile, PromptHeader, IValue, PromptHeaderAttributes } from '../promptFileParser.js'; import { IFileService } from '../../../../../../platform/files/common/files.js'; +import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js'; import { IPromptsService } from '../service/promptsService.js'; import { ILabelService } from '../../../../../../platform/label/common/label.js'; import { AGENTS_SOURCE_FOLDER, CLAUDE_AGENTS_SOURCE_FOLDER, isInClaudeRulesFolder, isSkillFilename, LEGACY_MODE_FILE_EXTENSION, VALID_SKILL_NAME_REGEX } from '../config/promptFileLocations.js'; @@ -37,6 +38,7 @@ export class PromptValidator { @ILabelService private readonly labelService: ILabelService, @IPromptsService private readonly promptsService: IPromptsService, @ILogService private readonly logger: ILogService, + @IConfigurationService private readonly configurationService: IConfigurationService, ) { } public async validate(promptAST: ParsedPromptFile, promptType: PromptsType, report: (markers: IMarkerData) => void): Promise { @@ -132,6 +134,19 @@ export class PromptValidator { } } } + + // Validate context: fork — requires the skill tool to be enabled + const contextAttribute = promptAST.header?.getAttribute(PromptHeaderAttributes.context); + if (contextAttribute && contextAttribute.value.type === 'scalar' && contextAttribute.value.value.trim() === 'fork') { + const skillToolEnabled = this.configurationService.getValue('github.copilot.chat.skillTool.enabled'); + if (!skillToolEnabled) { + report(toMarker( + localize('promptValidator.contextForkNotSupported', "The 'context: fork' attribute requires the skill tool to be enabled (github.copilot.chat.skillTool.enabled)."), + contextAttribute.value.range, + MarkerSeverity.Warning + )); + } + } } private async validateBody(promptAST: ParsedPromptFile, target: Target, report: (markers: IMarkerData) => void): Promise { @@ -925,7 +940,7 @@ const allAttributeNames: Record = { [PromptsType.prompt]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.model, PromptHeaderAttributes.tools, PromptHeaderAttributes.mode, PromptHeaderAttributes.agent, PromptHeaderAttributes.argumentHint], [PromptsType.instructions]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.applyTo, PromptHeaderAttributes.excludeAgent], [PromptsType.agent]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.model, PromptHeaderAttributes.tools, PromptHeaderAttributes.advancedOptions, PromptHeaderAttributes.handOffs, PromptHeaderAttributes.argumentHint, PromptHeaderAttributes.target, PromptHeaderAttributes.infer, PromptHeaderAttributes.agents, PromptHeaderAttributes.hooks, PromptHeaderAttributes.userInvocable, PromptHeaderAttributes.disableModelInvocation, GithubPromptHeaderAttributes.github], - [PromptsType.skill]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.license, PromptHeaderAttributes.compatibility, PromptHeaderAttributes.metadata, PromptHeaderAttributes.argumentHint, PromptHeaderAttributes.userInvocable, PromptHeaderAttributes.disableModelInvocation], + [PromptsType.skill]: [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.license, PromptHeaderAttributes.compatibility, PromptHeaderAttributes.metadata, PromptHeaderAttributes.argumentHint, PromptHeaderAttributes.userInvocable, PromptHeaderAttributes.disableModelInvocation, PromptHeaderAttributes.context], [PromptsType.hook]: [], // hooks are JSON files, not markdown with YAML frontmatter }; const githubCopilotAgentAttributeNames = [PromptHeaderAttributes.name, PromptHeaderAttributes.description, PromptHeaderAttributes.tools, PromptHeaderAttributes.target, GithubPromptHeaderAttributes.mcpServers, GithubPromptHeaderAttributes.github, PromptHeaderAttributes.infer]; diff --git a/src/vs/workbench/contrib/chat/common/promptSyntax/promptFileParser.ts b/src/vs/workbench/contrib/chat/common/promptSyntax/promptFileParser.ts index 0387470e98ba4..66e0b6dfea912 100644 --- a/src/vs/workbench/contrib/chat/common/promptSyntax/promptFileParser.ts +++ b/src/vs/workbench/contrib/chat/common/promptSyntax/promptFileParser.ts @@ -84,6 +84,7 @@ export namespace PromptHeaderAttributes { export const userInvocable = 'user-invocable'; export const disableModelInvocation = 'disable-model-invocation'; export const hooks = 'hooks'; + export const context = 'context'; } export class PromptHeader { @@ -316,6 +317,10 @@ export class PromptHeader { return this.getBooleanAttribute(PromptHeaderAttributes.disableModelInvocation); } + public get context(): string | undefined { + return this.getStringAttribute(PromptHeaderAttributes.context); + } + /** * Gets the raw 'hooks' attribute value from the header. * Returns the YAML map value if present, or undefined. The caller is