Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/vs/base/common/htmlContent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,18 @@ export function escapeMarkdownSyntaxTokens(text: string): string {
return text.replace(/[\\`*_{}[\]()#+\-!~]/g, '\\$&'); // CodeQL [SM02383] Backslash is escaped in the character class
}

/**
* Escapes only the characters that would break out of markdown link text
* (`[label](url)`) syntax: `\` and `]`. Use this when the escaped string is
* displayed as the visible label of a link, since renderers that extract the
* link text without re-parsing markdown (e.g. the chat inline anchor / skill
* pill) would otherwise show full `escapeMarkdownSyntaxTokens` backslashes
* (`\-`, `\.`, ...) verbatim.
*/
export function escapeMarkdownLinkLabel(text: string): string {
return text.replace(/[\\\]]/g, '\\$&');
}

/**
* @see https://github.com/microsoft/vscode/issues/193746
*/
Expand Down
23 changes: 22 additions & 1 deletion src/vs/base/test/common/htmlContent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import assert from 'assert';
import { appendEscapedMarkdownInlineCode } from '../../common/htmlContent.js';
import { appendEscapedMarkdownInlineCode, escapeMarkdownLinkLabel } from '../../common/htmlContent.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from './utils.js';

suite('htmlContent', () => {
Expand Down Expand Up @@ -37,4 +37,25 @@ suite('htmlContent', () => {
assert.strictEqual(appendEscapedMarkdownInlineCode('``'), '``` `` ```');
});
});

suite('escapeMarkdownLinkLabel', () => {
test('passes plain text through unchanged', () => {
assert.strictEqual(escapeMarkdownLinkLabel('hello'), 'hello');
assert.strictEqual(escapeMarkdownLinkLabel(''), '');
assert.strictEqual(escapeMarkdownLinkLabel('heap-snapshot-analysis'), 'heap-snapshot-analysis');
assert.strictEqual(escapeMarkdownLinkLabel('foo.bar_baz'), 'foo.bar_baz');
});

test('escapes only `\\` and `]`', () => {
assert.strictEqual(escapeMarkdownLinkLabel('a]b'), 'a\\]b');
assert.strictEqual(escapeMarkdownLinkLabel('a\\b'), 'a\\\\b');
assert.strictEqual(escapeMarkdownLinkLabel(']]'), '\\]\\]');
});

test('does not escape characters that are safe in link text', () => {
// these would be escaped by escapeMarkdownSyntaxTokens but must
// pass through here since they render literally inside `[...]`.
assert.strictEqual(escapeMarkdownLinkLabel('a*b_c#d-e.f!g~h+i(j)k{l}m'), 'a*b_c#d-e.f!g~h+i(j)k{l}m');
});
});
});
13 changes: 12 additions & 1 deletion src/vs/platform/agentHost/node/copilot/copilotAgentSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import type { FileEdit, ToolDefinition } from '../../common/state/protocol/state
import { SessionInputAnswerState, SessionInputAnswerValueKind, SessionInputQuestionKind, SessionInputResponseKind, ToolResultContentType, type PendingMessage, type SessionInputAnswer, type SessionInputRequest, type ToolCallResult, type ToolResultContent } from '../../common/state/sessionState.js';
import { CopilotSessionWrapper } from './copilotSessionWrapper.js';
import type { ShellManager } from './copilotShellTools.js';
import { getEditFilePath, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isShellTool, tryStringify, type ITypedPermissionRequest } from './copilotToolDisplay.js';
import { getEditFilePath, getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, isShellTool, synthesizeSkillToolEvents, tryStringify, type ITypedPermissionRequest } from './copilotToolDisplay.js';
import { FileEditTracker } from './fileEditTracker.js';
import { mapSessionEvents } from './mapSessionEvents.js';
import { buildPendingEditContentUri } from './pendingEditContentStore.js';
Expand Down Expand Up @@ -793,6 +793,17 @@ export class CopilotAgentSession extends Disposable {
this._onDidSessionProgress.fire({ session, type: 'idle' });
}));

// The SDK emits a `skill` tool call (which we hide) and a richer
// `skill.invoked` event with the resolved SKILL.md path. Synthesize a
// tool-start/complete pair from the latter so the UI can render a
// clickable file link, matching the `view`-tool display style.
this._register(wrapper.onSkillInvoked(e => {
this._logService.info(`[Copilot:${sessionId}] Skill invoked: ${e.data.name} (${e.data.path})`);
const { start, complete } = synthesizeSkillToolEvents(session, e.data, e.id);
this._onDidSessionProgress.fire(start);
this._onDidSessionProgress.fire(complete);
}));

this._register(wrapper.onSubagentStarted(e => {
this._logService.info(`[Copilot:${sessionId}] Subagent started: toolCallId=${e.data.toolCallId}, agent=${e.data.agentName}`);
this._onDidSessionProgress.fire({
Expand Down
98 changes: 96 additions & 2 deletions src/vs/platform/agentHost/node/copilot/copilotToolDisplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
import type { PermissionRequest } from '@github/copilot-sdk';
import { hasKey } from '../../../../base/common/types.js';
import { URI } from '../../../../base/common/uri.js';
import { appendEscapedMarkdownInlineCode } from '../../../../base/common/htmlContent.js';
import { appendEscapedMarkdownInlineCode, escapeMarkdownLinkLabel } from '../../../../base/common/htmlContent.js';
import { hash } from '../../../../base/common/hash.js';
import { localize } from '../../../../nls.js';
import type { IAgentToolReadyEvent } from '../../common/agentService.js';
import type { IAgentToolCompleteEvent, IAgentToolReadyEvent, IAgentToolStartEvent } from '../../common/agentService.js';
import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js';
import { StringOrMarkdown } from '../../common/state/protocol/state.js';
import { basename } from '../../../../base/common/resources.js';
Expand Down Expand Up @@ -55,6 +56,7 @@ const enum CopilotToolName {
WebFetch = 'web_fetch',
AskUser = 'ask_user',
ReportIntent = 'report_intent',
Skill = 'skill',
}

/** Parameters for the `bash` / `powershell` shell tools. */
Expand Down Expand Up @@ -170,9 +172,16 @@ const SUBAGENT_TOOL_NAMES: ReadonlySet<string> = new Set([
/**
* Tools that should not be shown to the user. These are internal tools
* used by the CLI for its own purposes (e.g., reporting intent to the model).
*
* `skill` is hidden because the SDK already emits a richer `skill.invoked`
* lifecycle event with the resolved skill file path; the agent session
* synthesizes a tool-start/complete pair from that event so the UI can
* render a clickable file link instead of just the skill name. See
* {@link synthesizeSkillToolEvents}.
*/
const HIDDEN_TOOL_NAMES: ReadonlySet<string> = new Set([
CopilotToolName.ReportIntent,
CopilotToolName.Skill,
]);

/**
Expand Down Expand Up @@ -398,6 +407,91 @@ export function getPastTenseMessage(toolName: string, displayName: string, param
}
}

// =============================================================================
// Skill event synthesis
//
// The Copilot SDK emits a `skill` tool call (which we hide) and, separately, a
// `skill.invoked` lifecycle event with the resolved skill file path. We turn
// the latter into a synthesized tool-start/complete pair so clients can render
// a clickable file link to the SKILL.md the agent loaded -- matching the
// existing `view`-tool display style. Live and replay paths share this helper
// so they stay in lock-step (see also the mirrored-pair gotcha for tool-call
// display in this file).
// =============================================================================

/** Subset of the SDK's `skill.invoked` payload that the synth helper needs. */
export interface ICopilotSkillInvokedData {
readonly name: string;
readonly path?: string;
readonly description?: string;
}

/**
* Builds a stable synthetic tool call id for a `skill.invoked` event so
* reconnect/replay produces the same id as the original live emit. The id
* is used unencoded as a path segment (e.g. by `ChatResponseResource.createUri`),
* so it must not contain characters like `/` -- we hash any fallback values
* that could carry filesystem paths or arbitrary text.
*/
export function getSkillSyntheticToolCallId(eventId: string | undefined, data: ICopilotSkillInvokedData): string {
if (eventId) {
return `synth-skill-${eventId}`;
}
const seed = data.path ?? data.name;
return `synth-skill-${hash(seed).toString(16)}`;
}

/**
* Synthesizes the `tool_start` and `tool_complete` agent progress events that
* represent a successful `skill.invoked` lifecycle event. Used by both the
* live session handler and the history-replay mapper so the two paths render
* identically.
*/
export function synthesizeSkillToolEvents(
session: URI,
data: ICopilotSkillInvokedData,
eventId: string | undefined,
): { start: IAgentToolStartEvent; complete: IAgentToolCompleteEvent } {
const toolCallId = getSkillSyntheticToolCallId(eventId, data);
const displayName = localize('toolName.skill', "Read Skill");
// Use the skill name as the link text rather than the basename: every skill
// file is named SKILL.md, so `Reading skill [plan]` reads better than the
// always-identical `Reading skill [SKILL.md]`. The client may further upgrade
// this link to a rich pill based on the `SKILL.md` basename. Skill names and
// paths come from the SDK / agent host and are escaped to prevent markdown
// injection from a malicious skill author.
// Escape only the characters that would break out of markdown link text
// syntax (`\` and `]`); a full markdown escape would leave visible
// backslashes in renderers (like the skill pill) that extract link text
// without re-parsing markdown.
const escapedName = escapeMarkdownLinkLabel(data.name);
const skillLink = data.path ? `[${escapedName}](${URI.file(data.path)})` : undefined;
const invocationMessage: StringOrMarkdown = skillLink
? md(localize('toolInvoke.skill', "Reading skill {0}", skillLink))
: localize('toolInvoke.skillName', "Reading skill {0}", data.name);
const pastTenseMessage: StringOrMarkdown = skillLink
? md(localize('toolComplete.skill', "Read skill {0}", skillLink))
: localize('toolComplete.skillName', "Read skill {0}", data.name);
Comment thread
roblourens marked this conversation as resolved.
const start: IAgentToolStartEvent = {
session,
type: 'tool_start',
toolCallId,
toolName: CopilotToolName.Skill,
displayName,
invocationMessage,
};
const complete: IAgentToolCompleteEvent = {
session,
type: 'tool_complete',
toolCallId,
result: {
success: true,
pastTenseMessage,
},
};
return { start, complete };
}

export function getToolInputString(toolName: string, parameters: Record<string, unknown> | undefined, rawArguments: string | undefined): string | undefined {
if (!parameters && !rawArguments) {
return undefined;
Expand Down
45 changes: 43 additions & 2 deletions src/vs/platform/agentHost/node/copilot/mapSessionEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { IAgentMessageEvent, IAgentSubagentStartedEvent, IAgentToolCompleteEvent
import { stripRedundantCdPrefix } from '../../common/commandLineHelpers.js';
import { IFileEditRecord, ISessionDatabase } from '../../common/sessionDataService.js';
import { ToolResultContentType, type ToolResultContent } from '../../common/state/sessionState.js';
import { getInvocationMessage, getPastTenseMessage, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool } from './copilotToolDisplay.js';
import { getInvocationMessage, getPastTenseMessage, getShellLanguage, getSubagentMetadata, getToolDisplayName, getToolInputString, getToolKind, isEditTool, isHiddenTool, synthesizeSkillToolEvents } from './copilotToolDisplay.js';
import { buildSessionDbUri } from './fileEditTracker.js';

function tryStringify(value: unknown): string | undefined {
Expand Down Expand Up @@ -58,11 +58,45 @@ export interface ISessionEventMessage {
reasoningText?: string;
encryptedContent?: string;
parentToolCallId?: string;
/**
* Origin of this message. The SDK sets this to a non-`'user'` value
* (e.g. `'skill-pdf'`) for messages it injects on behalf of a skill or
* other internal mechanism. We filter those out so they don't render
* as user turns.
*/
source?: string;
};
}

/** Minimal event shape for `skill.invoked`, used to synthesize a tool-style render. */
export interface ISessionEventSkillInvoked {
type: 'skill.invoked';
id?: string;
data: {
name: string;
path?: string;
description?: string;
};
}

/**
* Returns true if the event is a SDK-injected `user.message` that should not
* be shown to the user (e.g. skill-content injection).
*
* The SDK marks these via a non-`'user'` `source` field. Older sessions
* persisted before `source` existed will not be filtered; that is accepted
* leakage rather than guessed-at content sniffing.
*/
export function isSyntheticUserMessage(event: ISessionEvent): boolean {
if (event.type !== 'user.message') {
return false;
}
const source = (event as ISessionEventMessage).data?.source;
return !!source && source.toLowerCase() !== 'user';
}

/** Minimal event shape for session history mapping. */
export type ISessionEvent = ISessionEventToolStart | ISessionEventToolComplete | ISessionEventMessage | ISessionEventSubagentStarted | { type: string; data?: unknown };
export type ISessionEvent = ISessionEventToolStart | ISessionEventToolComplete | ISessionEventMessage | ISessionEventSubagentStarted | ISessionEventSkillInvoked | { type: string; data?: unknown };

export interface ISessionEventSubagentStarted {
type: 'subagent.started';
Expand Down Expand Up @@ -143,6 +177,9 @@ export async function mapSessionEvents(
// Second pass: build result events
for (const e of events) {
if (e.type === 'assistant.message' || e.type === 'user.message') {
if (isSyntheticUserMessage(e)) {
continue;
}
const d = (e as ISessionEventMessage).data;
result.push({
session,
Expand Down Expand Up @@ -253,6 +290,10 @@ export async function mapSessionEvents(
agentDisplayName: d.agentDisplayName,
agentDescription: d.agentDescription,
});
} else if (e.type === 'skill.invoked') {
const skillEvent = e as ISessionEventSkillInvoked;
const { start, complete } = synthesizeSkillToolEvents(session, skillEvent.data, skillEvent.id);
result.push(start, complete);
}
}
return result;
Expand Down
48 changes: 47 additions & 1 deletion src/vs/platform/agentHost/test/node/copilotToolDisplay.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import assert from 'assert';
import { URI } from '../../../../base/common/uri.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
import { getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getToolInputString, getToolKind, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js';
import { getInvocationMessage, getPastTenseMessage, getPermissionDisplay, getShellLanguage, getToolInputString, getToolKind, isHiddenTool, synthesizeSkillToolEvents, type ITypedPermissionRequest } from '../../node/copilot/copilotToolDisplay.js';

suite('getPermissionDisplay — cd-prefix stripping', () => {

Expand Down Expand Up @@ -293,3 +293,49 @@ suite('copilotToolDisplay — write_/read_ shell tools', () => {
});
});
});

suite('skill events', () => {

ensureNoDisposablesAreLeakedInTestSuite();

const session = URI.parse('agent://copilot/test');

test('hides the raw `skill` tool call and synthesizes a tool-start/complete pair from `skill.invoked`', () => {
const withPath = synthesizeSkillToolEvents(
session,
{ name: 'plan', path: '/abs/repo/skills/plan/SKILL.md' },
'evt-123',
);
const noPath = synthesizeSkillToolEvents(
session,
{ name: 'plan' },
undefined,
);

assert.deepStrictEqual({
skillIsHidden: isHiddenTool('skill'),
withPathToolCallId: withPath.start.toolCallId,
withPathSameIdOnComplete: withPath.start.toolCallId === withPath.complete.toolCallId,
withPathToolName: withPath.start.toolName,
withPathDisplayName: withPath.start.displayName,
withPathInvocation: withPath.start.invocationMessage,
withPathPastTense: withPath.complete.result.pastTenseMessage,
withPathSuccess: withPath.complete.result.success,
noPathToolCallId: noPath.start.toolCallId,
noPathInvocation: noPath.start.invocationMessage,
noPathPastTense: noPath.complete.result.pastTenseMessage,
}, {
skillIsHidden: true,
withPathToolCallId: 'synth-skill-evt-123',
withPathSameIdOnComplete: true,
withPathToolName: 'skill',
withPathDisplayName: 'Read Skill',
withPathInvocation: { markdown: 'Reading skill [plan](file:///abs/repo/skills/plan/SKILL.md)' },
withPathPastTense: { markdown: 'Read skill [plan](file:///abs/repo/skills/plan/SKILL.md)' },
withPathSuccess: true,
noPathToolCallId: 'synth-skill-2108d652',
noPathInvocation: 'Reading skill plan',
noPathPastTense: 'Read skill plan',
});
});
});
Loading
Loading