From aafca9be7fbcb9efd7c6dde039d318016f7c5733 Mon Sep 17 00:00:00 2001 From: KO Ho Tin Date: Sun, 26 Jul 2026 17:11:30 +0800 Subject: [PATCH 1/4] fix(vscode): make markdown file links in assistant output clickable Markdown links pointing at local files rendered as plain anchors, which the webview never follows: react-markdown's sanitizer empties file:// and vscode://file/ hrefs, and the openFile bridge rejected absolute paths and :line suffixes anyway. Only http(s) links worked. Route path-like hrefs (plain paths, file:// and vscode://file/ URIs, with an optional trailing :line(:col) suffix) through the existing FileLink / openFile bridge instead of an anchor, preserving those schemes with a custom urlTransform. The openFile handler now accepts absolute paths that resolve inside the selected working directory (relativized before the existing containment check, so out-of-workspace paths still fail) and reveals the requested line via the editor selection. Bare absolute paths in plain text (outside a markdown link) are unchanged; text enrichment still only links workspace-relative paths. Fixes #2194 --- apps/vscode/shared/bridge.ts | 4 ++ apps/vscode/src/handlers/file.handler.ts | 27 ++++++- apps/vscode/test/file-link-parsing.test.ts | 71 +++++++++++++++++++ apps/vscode/test/workspace-paths.test.ts | 55 ++++++++++++++ apps/vscode/tsconfig.json | 2 +- .../webview-ui/src/components/Markdown.tsx | 27 ++++--- apps/vscode/webview-ui/src/lib/link-utils.ts | 71 +++++++++++++++++++ apps/vscode/webview-ui/src/services/bridge.ts | 4 +- apps/vscode/webview-ui/tsconfig.json | 2 +- 9 files changed, 247 insertions(+), 16 deletions(-) create mode 100644 apps/vscode/test/file-link-parsing.test.ts create mode 100644 apps/vscode/webview-ui/src/lib/link-utils.ts diff --git a/apps/vscode/shared/bridge.ts b/apps/vscode/shared/bridge.ts index d7f9bd88ae..2ebab25b37 100644 --- a/apps/vscode/shared/bridge.ts +++ b/apps/vscode/shared/bridge.ts @@ -207,6 +207,10 @@ function validateParams(method: RpcMethod, params: unknown): boolean { || (Number.isInteger(params["maxCount"]) && (params["maxCount"] as number) >= 0)) && isOptionalType(params["includeVideo"], "boolean"); case Methods.OpenFile: + return hasString(params, "filePath") + && isPlainObject(params) + && (params["line"] === undefined + || (Number.isInteger(params["line"]) && (params["line"] as number) >= 1)); case Methods.OpenFileDiff: case Methods.CheckFileExists: case Methods.GetImageDataUri: diff --git a/apps/vscode/src/handlers/file.handler.ts b/apps/vscode/src/handlers/file.handler.ts index abc49cabf2..09be1ca4ea 100644 --- a/apps/vscode/src/handlers/file.handler.ts +++ b/apps/vscode/src/handlers/file.handler.ts @@ -4,6 +4,7 @@ import * as vscode from "vscode"; import { Events, Methods } from "../../shared/bridge"; import type { FileChange, ProjectFile } from "../../shared/types"; import type { BaselineSession } from "../managers/baseline.manager"; +import { relativeFsPath } from "../utils/fs-path"; import { isWorkspacePathContained, resolveWorkspacePath, @@ -17,6 +18,7 @@ interface GetProjectFilesParams { } interface PickMediaParams { maxCount?: number; includeVideo?: boolean } interface FilePathParams { filePath: string } +interface OpenFileParams { filePath: string; line?: number } interface OptionalFilePathParams { filePath?: string } interface PathsParams { paths: string[] } interface CheckFilesExistParams { paths: string[] } @@ -68,13 +70,32 @@ const pickMedia: Handler = async (params) => { return results; }; -const openFile: Handler = async ({ filePath }, ctx) => { - const resolved = await resolveExistingWorkspaceFile(ctx.requireWorkDirUri(), filePath); +const openFile: Handler = async ({ filePath, line }, ctx) => { + const workDirUri = ctx.requireWorkDirUri(); + // Assistant output also links files by absolute path; relativize when the + // target lies inside the working directory so those links open, while + // out-of-workspace paths keep failing the containment check below. + const target = isAbsolutePathInput(filePath) + ? relativeFsPath(workDirUri.fsPath, filePath) + : filePath; + if (target === undefined) return { ok: false }; + const resolved = await resolveExistingWorkspaceFile(workDirUri, target); if (resolved === undefined) return { ok: false }; - await vscode.commands.executeCommand("vscode.open", resolved.uri); + await vscode.commands.executeCommand( + "vscode.open", + resolved.uri, + line === undefined ? undefined : { selection: new vscode.Range(line - 1, 0, line - 1, 0) }, + ); return { ok: true }; }; +function isAbsolutePathInput(input: string): boolean { + // Same detection as resolveWorkspacePath's rejection branch. + return path.posix.isAbsolute(input.replaceAll("\\", "/")) + || path.win32.isAbsolute(input) + || /^[A-Za-z]:/.test(input); +} + const openFileDiff: Handler = async ({ filePath }, ctx) => { const sessionId = ctx.getSessionId(); const resolved = await resolveExistingWorkspaceFile(ctx.requireWorkDirUri(), filePath); diff --git a/apps/vscode/test/file-link-parsing.test.ts b/apps/vscode/test/file-link-parsing.test.ts new file mode 100644 index 0000000000..2e7d6e3c85 --- /dev/null +++ b/apps/vscode/test/file-link-parsing.test.ts @@ -0,0 +1,71 @@ +/** + * Scenario: markdown link hrefs in assistant output → local file-link targets. + * Responsibilities: external links stay anchors, local paths (plain, file://, + * vscode://file/, with optional :line suffix) resolve to an openFile target, + * and the url transform preserves the schemes the sanitizer would drop. + * Wiring: pure helpers, no VS Code host involved. + * Run: pnpm --filter kimi-code exec vitest run --config vitest.config.ts test/file-link-parsing.test.ts + */ +import { describe, expect, it } from "vitest"; +import { fileAwareUrlTransform, parseFileLink } from "@/lib/link-utils"; + +describe("parseFileLink", () => { + it("parses an absolute path with a line suffix", () => { + expect(parseFileLink("/abs/path/report.md:33")).toEqual({ path: "/abs/path/report.md", line: 33 }); + }); + + it("parses an absolute path without a line suffix", () => { + expect(parseFileLink("/abs/path/report.md")).toEqual({ path: "/abs/path/report.md" }); + }); + + it("parses a relative path with a line:column suffix", () => { + expect(parseFileLink("src/app.ts:12:5")).toEqual({ path: "src/app.ts", line: 12 }); + }); + + it("parses a file URI with a line suffix", () => { + expect(parseFileLink("file:///abs/path/report.md:33")).toEqual({ path: "/abs/path/report.md", line: 33 }); + }); + + it("parses a Windows drive file URI with percent-encoding", () => { + expect(parseFileLink("file:///C:/proj/file%20name.ts")).toEqual({ path: "C:/proj/file name.ts" }); + }); + + it("parses a vscode file URI", () => { + expect(parseFileLink("vscode://file/abs/path/report.md:33")).toEqual({ path: "/abs/path/report.md", line: 33 }); + }); + + it("parses a native Windows path with a line suffix", () => { + expect(parseFileLink("C:\\proj\\file.ts:7")).toEqual({ path: "C:\\proj\\file.ts", line: 7 }); + }); + + it("returns null for external and non-file links", () => { + expect(parseFileLink("https://example.com/report.md")).toBeNull(); + expect(parseFileLink("http://example.com")).toBeNull(); + expect(parseFileLink("mailto:someone@example.com")).toBeNull(); + expect(parseFileLink("javascript:alert(1)")).toBeNull(); + expect(parseFileLink("data:text/plain;base64,eA==")).toBeNull(); + expect(parseFileLink("vscode://settings/editor")).toBeNull(); + expect(parseFileLink("//example.com/report.md")).toBeNull(); + expect(parseFileLink("#section")).toBeNull(); + expect(parseFileLink("")).toBeNull(); + expect(parseFileLink(undefined)).toBeNull(); + }); +}); + +describe("fileAwareUrlTransform", () => { + it("preserves file and vscode-file URLs the default sanitizer would drop", () => { + expect(fileAwareUrlTransform("file:///abs/report.md:33")).toBe("file:///abs/report.md:33"); + expect(fileAwareUrlTransform("vscode://file/abs/report.md")).toBe("vscode://file/abs/report.md"); + }); + + it("keeps http(s) links and plain paths intact", () => { + expect(fileAwareUrlTransform("https://example.com/x")).toBe("https://example.com/x"); + expect(fileAwareUrlTransform("src/app.ts:12")).toBe("src/app.ts:12"); + expect(fileAwareUrlTransform("/abs/path.md:3")).toBe("/abs/path.md:3"); + expect(fileAwareUrlTransform("C:\\proj\\file.ts:7")).toBe("C:\\proj\\file.ts:7"); + }); + + it("still drops javascript URLs", () => { + expect(fileAwareUrlTransform("javascript:alert(1)")).toBe(""); + }); +}); diff --git a/apps/vscode/test/workspace-paths.test.ts b/apps/vscode/test/workspace-paths.test.ts index 1e64f43381..a98d00a460 100644 --- a/apps/vscode/test/workspace-paths.test.ts +++ b/apps/vscode/test/workspace-paths.test.ts @@ -70,6 +70,15 @@ const vscodeHost = vi.hoisted(() => { } } + class Range { + constructor( + readonly startLine: number, + readonly startCharacter: number, + readonly endLine: number, + readonly endCharacter: number, + ) {} + } + class RelativePattern { readonly baseUri: Uri; readonly base: string; @@ -89,6 +98,7 @@ const vscodeHost = vi.hoisted(() => { return { Uri, + Range, RelativePattern, readDirectory, stat, @@ -102,6 +112,7 @@ const vscodeHost = vi.hoisted(() => { vi.mock("vscode", () => ({ Uri: vscodeHost.Uri, + Range: vscodeHost.Range, RelativePattern: vscodeHost.RelativePattern, FileType: { Unknown: 0, File: 1, Directory: 2, SymbolicLink: 64 }, QuickPickItemKind: { Separator: -1 }, @@ -280,6 +291,50 @@ describe("Webview workspace paths (selected-directory containment)", () => { expect(vscodeHost.executeCommand).not.toHaveBeenCalled(); }); + it("opens an absolute path inside the selected working directory", async () => { + const workDir = join(root, "project"); + await mkdir(workDir); + const inside = join(workDir, "notes.md"); + await writeFile(inside, "notes"); + const ctx = createContext(vscodeHost.Uri.file(workDir)); + + const result = await fileHandlers[Methods.OpenFile]!({ filePath: inside }, ctx); + + expect(result).toEqual({ ok: true }); + const [command, uri] = vscodeHost.executeCommand.mock.calls[0]!; + expect(command).toBe("vscode.open"); + expect(areSameFsPath((uri as vscode.Uri).fsPath, inside)).toBe(true); + }); + + it("refuses an absolute path outside the selected working directory", async () => { + const workDir = join(root, "project"); + await mkdir(workDir); + const outside = join(root, "outside.md"); + await writeFile(outside, "secret"); + const ctx = createContext(vscodeHost.Uri.file(workDir)); + + const result = await fileHandlers[Methods.OpenFile]!({ filePath: outside }, ctx); + + expect(result).toEqual({ ok: false }); + expect(vscodeHost.executeCommand).not.toHaveBeenCalled(); + }); + + it("opens a file at the requested line", async () => { + const workDir = join(root, "project"); + await mkdir(join(workDir, "sub"), { recursive: true }); + await writeFile(join(workDir, "sub", "file.md"), "content"); + const ctx = createContext(vscodeHost.Uri.file(workDir)); + + const result = await fileHandlers[Methods.OpenFile]!({ filePath: "sub/file.md", line: 33 }, ctx); + + expect(result).toEqual({ ok: true }); + const call = vscodeHost.executeCommand.mock.calls[0]!; + expect(call[0]).toBe("vscode.open"); + expect(call[2]).toMatchObject({ + selection: { startLine: 32, startCharacter: 0, endLine: 32, endCharacter: 0 }, + }); + }); + it("omits an outside symlink when an SDK Write event requests baseline capture", async () => { const workDir = join(root, "project"); const outsideRoot = await mkdtemp(join(tmpdir(), "kimi-vscode-baseline-outside-")); diff --git a/apps/vscode/tsconfig.json b/apps/vscode/tsconfig.json index bc9d284638..6f6205a1e8 100644 --- a/apps/vscode/tsconfig.json +++ b/apps/vscode/tsconfig.json @@ -15,5 +15,5 @@ } }, "include": ["src/**/*", "shared/**/*", "test/**/*"], - "exclude": ["dist", "node_modules", "webview-ui", "test/settings-store.test.ts"] + "exclude": ["dist", "node_modules", "webview-ui", "test/settings-store.test.ts", "test/file-link-parsing.test.ts"] } diff --git a/apps/vscode/webview-ui/src/components/Markdown.tsx b/apps/vscode/webview-ui/src/components/Markdown.tsx index f9d04a5a45..a2bc7f0824 100644 --- a/apps/vscode/webview-ui/src/components/Markdown.tsx +++ b/apps/vscode/webview-ui/src/components/Markdown.tsx @@ -9,6 +9,7 @@ import { useRequest } from "ahooks"; import { IconVideo } from "@tabler/icons-react"; import type { Components } from "react-markdown"; import { parseSegments, parseColorSegments, extractPaths, checkFilesExist, hasColors, isLocalPath } from "@/lib/text-enrichment"; +import { parseFileLink, fileAwareUrlTransform } from "@/lib/link-utils"; import { CopyButton } from "@/components/CopyButton"; import { MediaPreviewModal, StreamImagePreview, ImageLoadFail } from "@/components/MediaPreviewModal"; import { getMediaTypeFromSrc } from "@/lib/media-utils"; @@ -37,13 +38,13 @@ function ColorSwatch({ color }: { color: string }) { return ; } -export function FileLink({ path, display }: { path: string; display: string }) { +export function FileLink({ path, line, display }: { path: string; line?: number; display: React.ReactNode }) { const onClick = useCallback( (e: React.MouseEvent) => { e.preventDefault(); - void bridge.openFile(path); + void bridge.openFile(path, line); }, - [path], + [path, line], ); return (