Skip to content

Commit 735c737

Browse files
dileepyavanCopilot
andauthored
Fix WSL resource links in Agent Host chat responses (#333635)
* changes Signed-off-by: Dileep Yavanmandha <dileepy@microsoft.com> * Fix Agent Host file link hover labels Format default Markdown link hovers with the host-aware label service while preserving navigation targets and custom titles. Add coverage for WSL paths, Windows host formatting, and existing link behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * changes Signed-off-by: Dileep Yavanmandha <dileepy@microsoft.com> --------- Signed-off-by: Dileep Yavanmandha <dileepy@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 33451f4 commit 735c737

8 files changed

Lines changed: 187 additions & 6 deletions

File tree

src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6811,7 +6811,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC
68116811
}
68126812

68136813
resolveChatResponseUri(_sessionResource: URI, href: string, _kind: 'link' | 'image'): string {
6814-
return rewriteAgentHostLinkTarget(href, this._config.connectionAuthority);
6814+
return rewriteAgentHostLinkTarget(href, this._config.connectionAuthority, this._config.connection.resourceUris);
68156815
}
68166816

68176817
/**

src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2126,7 +2126,7 @@ function normalizeFileUriSelection(uri: URI, href: string): URI {
21262126
}
21272127

21282128
/** Wraps an absolute path or internal URI target for the owning Agent Host connection. */
2129-
export function rewriteAgentHostLinkTarget(href: string, connectionAuthority: string): string {
2129+
export function rewriteAgentHostLinkTarget(href: string, connectionAuthority: string, resourceUris: IAgentHostResourceUriMapper = createAgentHostResourceUriMapper(connectionAuthority)): string {
21302130
let parsed = parseAbsoluteFileLinkTarget(href);
21312131
if (!parsed) {
21322132
try {
@@ -2146,7 +2146,10 @@ export function rewriteAgentHostLinkTarget(href: string, connectionAuthority: st
21462146

21472147
let agentHostUri: URI;
21482148
try {
2149-
agentHostUri = toAgentHostUri(parsed, connectionAuthority);
2149+
agentHostUri = resourceUris.fromAgentHost(parsed);
2150+
if (parsed.scheme !== Schemas.file && isEqual(agentHostUri, parsed)) {
2151+
agentHostUri = toAgentHostUri(parsed, connectionAuthority);
2152+
}
21502153
} catch {
21512154
return href;
21522155
}

src/vs/workbench/contrib/chat/browser/widget/chatContentMarkdownRenderer.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import { getDefaultHoverDelegate } from '../../../../../base/browser/ui/hover/ho
99
import { IMarkdownString } from '../../../../../base/common/htmlContent.js';
1010
import { DisposableStore } from '../../../../../base/common/lifecycle.js';
1111
import { type MarkedExtension } from '../../../../../base/common/marked/marked.js';
12+
import { URI } from '../../../../../base/common/uri.js';
13+
import { ILabelService } from '../../../../../platform/label/common/label.js';
1214
import { IMarkdownRenderer, IMarkdownRendererService } from '../../../../../platform/markdown/browser/markdownRenderer.js';
1315
import { ILanguageService } from '../../../../../editor/common/languages/language.js';
1416
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
@@ -124,6 +126,7 @@ export class ChatContentMarkdownRenderer implements IMarkdownRenderer {
124126
@IConfigurationService configurationService: IConfigurationService,
125127
@IHoverService private readonly hoverService: IHoverService,
126128
@IMarkdownRendererService private readonly markdownRendererService: IMarkdownRendererService,
129+
@ILabelService private readonly labelService: ILabelService,
127130
) { }
128131

129132
render(markdown: IMarkdownString, options?: MarkdownRenderOptions, outElement?: HTMLElement): IRenderedMarkdown {
@@ -162,7 +165,12 @@ export class ChatContentMarkdownRenderer implements IMarkdownRenderer {
162165
// eslint-disable-next-line no-restricted-syntax
163166
result.element.querySelectorAll('a').forEach((element) => {
164167
if (element.title) {
165-
const title = element.title;
168+
let title = element.title;
169+
if (title === element.dataset.href && title.startsWith(`${AGENT_HOST_SCHEME}:`)) {
170+
const uri = URI.parse(title);
171+
const label = this.labelService.getUriLabel(uri);
172+
title = uri.fragment ? `${label}#${uri.fragment}` : label;
173+
}
166174
element.title = '';
167175
store.add(this.hoverService.setupManagedHover(getDefaultHoverDelegate('element'), element, title));
168176
}

src/vs/workbench/contrib/chat/test/browser/agentSessions/agentHostChatContribution.test.ts

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { ILogService, NullLogService } from '../../../../../../platform/log/comm
2626
import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js';
2727
import { IAgentCreateSessionConfig, IAgentHostService, IAgentSessionMetadata, AgentSession } from '../../../../../../platform/agentHost/common/agentService.js';
2828
import type { ChatInputRequestWithPlanReview } from '../../../../../../platform/agentHost/common/agentHostPlanReview.js';
29-
import { createAgentHostResourceUriMapper, identityAgentHostResourceUriMapper, toAgentHostUri } from '../../../../../../platform/agentHost/common/agentHostUri.js';
29+
import { agentHostAuthority, createAgentHostResourceUriMapper, fromAgentHostUri, identityAgentHostResourceUriMapper, toAgentHostUri } from '../../../../../../platform/agentHost/common/agentHostUri.js';
3030
import { AgentFeedbackAttachmentDisplayKind, AgentFeedbackAttachmentMetadataKey } from '../../../../../../platform/agentHost/common/meta/agentFeedbackAttachments.js';
3131
import { VSCODE_EPHEMERAL_SESSION_META_KEY } from '../../../../../../platform/agentHost/common/meta/agentEphemeralSessionMeta.js';
3232
import { getElementAttachmentCorrelationId, toElementAttachmentMeta } from '../../../../../../platform/agentHost/common/meta/agentElementAttachments.js';
@@ -1271,6 +1271,90 @@ suite('AgentHostChatContribution', () => {
12711271

12721272
});
12731273

1274+
suite('response resource links', () => {
1275+
test('uses the WSL connection for file links despite a local session authority', () => {
1276+
const { sessionHandler, agentHostService } = createContribution(disposables);
1277+
const authority = agentHostAuthority('vscode-remote://wsl+Ubuntu');
1278+
agentHostService.resourceUris = createAgentHostResourceUriMapper(authority);
1279+
const session = URI.parse('agent-host-copilot:/session');
1280+
const file = URI.file('/home/user/project/src/file.ts').with({ fragment: 'L42,7' });
1281+
const targets = [
1282+
'/home/user/project/src/file.ts:42:7',
1283+
'file:///home/user/project/src/file.ts#L42,7',
1284+
];
1285+
1286+
assert.deepStrictEqual(targets.map(href => {
1287+
const resolved = URI.parse(sessionHandler.resolveChatResponseUri(session, href, 'link'));
1288+
return { resolved: resolved.toString(), hostUri: fromAgentHostUri(resolved).toString() };
1289+
}), targets.map(() => ({
1290+
resolved: toAgentHostUri(file, authority).toString(),
1291+
hostUri: file.toString(),
1292+
})));
1293+
});
1294+
1295+
test('uses the WSL connection for image paths and preserves encoded path characters', () => {
1296+
const { sessionHandler, agentHostService } = createContribution(disposables);
1297+
const authority = agentHostAuthority('vscode-remote://wsl+Ubuntu');
1298+
agentHostService.resourceUris = createAgentHostResourceUriMapper(authority);
1299+
const session = URI.parse('agent-host-copilot:/session');
1300+
const image = URI.file('/home/user/my project/image.png');
1301+
1302+
assert.strictEqual(
1303+
sessionHandler.resolveChatResponseUri(session, '/home/user/my%20project/image.png', 'image'),
1304+
toAgentHostUri(image, authority).toString(),
1305+
);
1306+
});
1307+
1308+
for (const host of ['local', 'WSL']) {
1309+
test(`routes ${host} internal resource links and images through the owning Agent Host`, () => {
1310+
const { sessionHandler, agentHostService } = createContribution(disposables);
1311+
const authority = host === 'local' ? 'local' : agentHostAuthority('vscode-remote://wsl+Ubuntu');
1312+
agentHostService.resourceUris = host === 'local' ? identityAgentHostResourceUriMapper : createAgentHostResourceUriMapper(authority);
1313+
const session = URI.parse('agent-host-copilot:/session');
1314+
const resources = [
1315+
URI.parse('agenthost-content:///session/my%20result.txt?view=raw#L42,7'),
1316+
URI.parse('git-blob:///project/src/file.ts?ref=HEAD#L7'),
1317+
];
1318+
1319+
assert.deepStrictEqual(resources.map(resource => {
1320+
const href = resource.toString();
1321+
const link = sessionHandler.resolveChatResponseUri(session, href, 'link');
1322+
return {
1323+
link,
1324+
image: sessionHandler.resolveChatResponseUri(session, href, 'image'),
1325+
unwrapped: fromAgentHostUri(URI.parse(link)).toString(),
1326+
alreadyMapped: sessionHandler.resolveChatResponseUri(session, toAgentHostUri(resource, authority).toString(), 'link'),
1327+
};
1328+
}), resources.map(resource => ({
1329+
link: toAgentHostUri(resource, authority).toString(),
1330+
image: toAgentHostUri(resource, authority).toString(),
1331+
unwrapped: resource.toString(),
1332+
alreadyMapped: toAgentHostUri(resource, authority).toString(),
1333+
})));
1334+
});
1335+
}
1336+
1337+
test('preserves local file links and external or already mapped links', () => {
1338+
const { sessionHandler, agentHostService } = createContribution(disposables);
1339+
const session = URI.parse('agent-host-copilot:/session');
1340+
const local = sessionHandler.resolveChatResponseUri(session, '/project/file.ts:42', 'link');
1341+
const authority = agentHostAuthority('vscode-remote://wsl+Ubuntu');
1342+
agentHostService.resourceUris = createAgentHostResourceUriMapper(authority);
1343+
const mapped = toAgentHostUri(URI.file('/home/user/file.ts'), authority).toString();
1344+
const external = 'https://example.com/file.ts';
1345+
1346+
assert.deepStrictEqual({
1347+
local,
1348+
mapped: sessionHandler.resolveChatResponseUri(session, mapped, 'link'),
1349+
external: sessionHandler.resolveChatResponseUri(session, external, 'link'),
1350+
}, {
1351+
local: URI.file('/project/file.ts').with({ fragment: 'L42' }).toString(),
1352+
mapped,
1353+
external,
1354+
});
1355+
});
1356+
});
1357+
12741358
// ---- Download progress notification (editor window) -----------------
12751359

12761360
suite('download progress', () => {

src/vs/workbench/contrib/chat/test/browser/widget/chatMarkdownRenderer.test.ts

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,93 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import assert from 'assert';
7+
import sinon from 'sinon';
78
import { MarkdownString } from '../../../../../../base/common/htmlContent.js';
9+
import { OperatingSystem } from '../../../../../../base/common/platform.js';
10+
import { URI } from '../../../../../../base/common/uri.js';
811
import { assertSnapshot } from '../../../../../../base/test/common/snapshot.js';
912
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js';
13+
import { AGENT_HOST_LABEL_FORMATTER, agentHostAuthority, agentHostLabelFormatter, toAgentHostUri } from '../../../../../../platform/agentHost/common/agentHostUri.js';
14+
import { IHoverService } from '../../../../../../platform/hover/browser/hover.js';
15+
import { NullHoverService } from '../../../../../../platform/hover/test/browser/nullHoverService.js';
16+
import { ILabelService } from '../../../../../../platform/label/common/label.js';
1017
import { ChatContentMarkdownRenderer } from '../../../browser/widget/chatContentMarkdownRenderer.js';
1118
import { workbenchInstantiationService } from '../../../../../test/browser/workbenchTestServices.js';
1219

1320
suite('ChatMarkdownRenderer', () => {
1421
const store = ensureNoDisposablesAreLeakedInTestSuite();
1522

1623
let testRenderer: ChatContentMarkdownRenderer;
24+
let instantiationService: ReturnType<typeof workbenchInstantiationService>;
1725
setup(() => {
18-
const instantiationService = store.add(workbenchInstantiationService(undefined, store));
26+
instantiationService = store.add(workbenchInstantiationService(undefined, store));
1927
testRenderer = instantiationService.createInstance(ChatContentMarkdownRenderer);
2028
});
2129

30+
suite('link hovers', () => {
31+
let setupManagedHover: sinon.SinonSpy<Parameters<IHoverService['setupManagedHover']>, ReturnType<IHoverService['setupManagedHover']>>;
32+
33+
setup(() => {
34+
setupManagedHover = sinon.spy(NullHoverService.setupManagedHover);
35+
instantiationService.stub(IHoverService, { ...NullHoverService, setupManagedHover });
36+
store.add(instantiationService.get(ILabelService).registerFormatter(AGENT_HOST_LABEL_FORMATTER));
37+
testRenderer = instantiationService.createInstance(ChatContentMarkdownRenderer);
38+
});
39+
40+
test('shows host paths for transformed and already mapped links without changing their targets', () => {
41+
const authority = agentHostAuthority('vscode-remote://wsl+Ubuntu');
42+
store.add(instantiationService.get(ILabelService).registerFormatter(agentHostLabelFormatter(authority, OperatingSystem.Linux)));
43+
const file = URI.file('/home/user/my project/a&b.ts').with({ fragment: 'L42,7' });
44+
const target = toAgentHostUri(file, authority).toString();
45+
const links = [
46+
{ href: '/home/user/my%20project/a&b.ts:42:7', transformUri: () => target },
47+
{ href: target, transformUri: undefined },
48+
];
49+
50+
const actual = links.map(({ href, transformUri }) => {
51+
const result = store.add(testRenderer.render(new MarkdownString(`[file](${href})`), { transformUri }));
52+
const link = result.element.querySelector('a');
53+
return {
54+
hover: setupManagedHover.lastCall.args[2],
55+
target: link?.dataset.href,
56+
text: link?.textContent,
57+
nativeTitle: link?.title,
58+
};
59+
});
60+
61+
assert.deepStrictEqual(actual, links.map(() => ({
62+
hover: '/home/user/my project/a&b.ts#L42,7',
63+
target,
64+
text: 'file',
65+
nativeTitle: '',
66+
})));
67+
});
68+
69+
test('uses the remote host operating system for path formatting', () => {
70+
const labelService = instantiationService.get(ILabelService);
71+
store.add(labelService.registerFormatter(agentHostLabelFormatter('windows-host', OperatingSystem.Windows)));
72+
const target = toAgentHostUri(URI.file('C:/my project/file.ts'), 'windows-host').toString();
73+
store.add(testRenderer.render(new MarkdownString(`[file](${target})`)));
74+
75+
assert.strictEqual(setupManagedHover.lastCall.args[2], 'C:\\my project\\file.ts');
76+
});
77+
78+
test('preserves explicit titles and ordinary file, external, and command link behavior', () => {
79+
const target = toAgentHostUri(URI.file('/home/user/file.ts'), 'remote-host').toString();
80+
const file = URI.file('/my project/file.ts').with({ fragment: 'L7' });
81+
const markdown = new MarkdownString(`[custom](${target} "Custom title") [file](${file}) [web](https://example.com/) [command](command:example)`, { isTrusted: true });
82+
const result = store.add(testRenderer.render(markdown));
83+
84+
assert.deepStrictEqual({
85+
hovers: setupManagedHover.getCalls().map(call => call.args[2]),
86+
targets: Array.from(result.element.querySelectorAll('a'), link => link.dataset.href),
87+
}, {
88+
hovers: ['Custom title', `${file.fsPath}#L7`, 'https://example.com/'],
89+
targets: [target, file.toString(), 'https://example.com/', 'command:example'],
90+
});
91+
});
92+
});
93+
2294
test('simple', async () => {
2395
const md = new MarkdownString('a');
2496
const result = store.add(testRenderer.render(md));

src/vs/workbench/test/browser/componentFixtures/chat/chatAgentMerge.fixture.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { buildAgentMergePrompt, IAgentMergePromptSummary, parseAgentMergePrompt
1111
import { CommandsRegistry, ICommandService } from '../../../../../platform/commands/common/commands.js';
1212
import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js';
1313
import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js';
14+
import { ILabelService } from '../../../../../platform/label/common/label.js';
1415
import { IMarkdownRendererService, MarkdownRendererService } from '../../../../../platform/markdown/browser/markdownRenderer.js';
1516
import { ChatContentMarkdownRenderer } from '../../../../contrib/chat/browser/widget/chatContentMarkdownRenderer.js';
1617
import { ChatAgentMergeContentPart } from '../../../../contrib/chat/browser/widget/chatContentParts/chatAgentMergeContentPart.js';
@@ -178,6 +179,9 @@ function renderAgentMerge({ container, disposableStore, theme }: ComponentFixtur
178179
const instantiationService = createEditorServices(disposableStore, {
179180
colorTheme: theme,
180181
additionalServices: (reg) => {
182+
reg.defineInstance(ILabelService, new class extends mock<ILabelService>() {
183+
override getUriLabel(uri: URI): string { return uri.path; }
184+
}());
181185
reg.define(IMarkdownRendererService, MarkdownRendererService);
182186
reg.defineInstance(ICommandService, commandService);
183187
},

src/vs/workbench/test/browser/componentFixtures/chat/chatAgentMergeNotice.fixture.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import * as dom from '../../../../../base/browser/dom.js';
7+
import { URI } from '../../../../../base/common/uri.js';
78
import { mock } from '../../../../../base/test/common/mock.js';
89
import { agentMergeDisableReasons, agentMergeDisabledNotice, agentMergeEnabledNotice } from '../../../../../platform/agentHost/common/agentMerge.js';
910
import { AgentSystemNotificationKind, toAgentSystemNotificationMeta } from '../../../../../platform/agentHost/common/meta/agentSystemNotificationMeta.js';
11+
import { ILabelService } from '../../../../../platform/label/common/label.js';
1012
import { IMarkdownRendererService, MarkdownRendererService } from '../../../../../platform/markdown/browser/markdownRenderer.js';
1113
import { systemNotificationToChatPart } from '../../../../contrib/chat/browser/agentSessions/agentHost/stateToProgressAdapter.js';
1214
import { ChatContentMarkdownRenderer } from '../../../../contrib/chat/browser/widget/chatContentMarkdownRenderer.js';
@@ -35,6 +37,9 @@ function renderNotice(context: ComponentFixtureContext, content: string, kind: A
3537
const instantiationService = createEditorServices(disposableStore, {
3638
colorTheme: context.theme,
3739
additionalServices: (reg) => {
40+
reg.defineInstance(ILabelService, new class extends mock<ILabelService>() {
41+
override getUriLabel(uri: URI): string { return uri.path; }
42+
}());
3843
reg.define(IMarkdownRendererService, MarkdownRendererService);
3944
reg.defineInstance(IChatMarkdownAnchorService, anchorService);
4045
},

src/vs/workbench/test/browser/componentFixtures/chat/chatProgressContentPart.fixture.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@ import { Event } from '../../../../../base/common/event.js';
99
import { observableValue } from '../../../../../base/common/observable.js';
1010
import { Codicon } from '../../../../../base/common/codicons.js';
1111
import { ThemeIcon } from '../../../../../base/common/themables.js';
12+
import { URI } from '../../../../../base/common/uri.js';
1213
import { mock, upcastPartial } from '../../../../../base/test/common/mock.js';
14+
import { ILabelService } from '../../../../../platform/label/common/label.js';
1315
import { IMarkdownRendererService, MarkdownRendererService } from '../../../../../platform/markdown/browser/markdownRenderer.js';
1416
import { ChatProgressContentPart } from '../../../../contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.js';
1517
import { ChatContentMarkdownRenderer } from '../../../../contrib/chat/browser/widget/chatContentMarkdownRenderer.js';
@@ -68,6 +70,9 @@ function renderProgressPart(
6870
const instantiationService = createEditorServices(disposableStore, {
6971
colorTheme: context.theme,
7072
additionalServices: (reg) => {
73+
reg.defineInstance(ILabelService, new class extends mock<ILabelService>() {
74+
override getUriLabel(uri: URI): string { return uri.path; }
75+
}());
7176
reg.define(IMarkdownRendererService, MarkdownRendererService);
7277
reg.defineInstance(IChatMarkdownAnchorService, mockAnchorService);
7378
},

0 commit comments

Comments
 (0)