diff --git a/.changeset/file-mention-spaces-unicode.md b/.changeset/file-mention-spaces-unicode.md new file mode 100644 index 00000000000..da2b09b6410 --- /dev/null +++ b/.changeset/file-mention-spaces-unicode.md @@ -0,0 +1,5 @@ +--- +"kilo-code": patch +--- + +Fix file @mentions for filenames containing spaces or non-ASCII characters. Such mentions now stay fully highlighted in the sent message and are read correctly instead of failing with "File not found". diff --git a/packages/kilo-ui/src/components/message-highlight.test.ts b/packages/kilo-ui/src/components/message-highlight.test.ts index c752b412efa..a30335c7b3b 100644 --- a/packages/kilo-ui/src/components/message-highlight.test.ts +++ b/packages/kilo-ui/src/components/message-highlight.test.ts @@ -80,4 +80,325 @@ describe("message highlight", () => { { text: "@src/index.ts", type: "file" }, ]) }) + + test("highlights filename with a space when source offsets are provided", () => { + const text = "check @org data.xlsx now" + const segments = buildHighlightedTextSegments( + text, + [ + { + source: { + type: "file", + path: "org data.xlsx", + text: { value: "@org data.xlsx", start: 6, end: 20 }, + }, + }, + ], + [], + ) + + expect(segments).toEqual([ + { text: "check " }, + { text: "@org data.xlsx", type: "file" }, + { text: " now" }, + ]) + }) + + test("fallback regex does not match filenames containing spaces (requires source offsets)", () => { + // The fallback regex intentionally excludes spaces: a pattern permissive enough to + // span space-separated path segments would also swallow ordinary prose following + // any unrelated @mention. Highlighting space-containing paths relies on source.text. + const text = "check @org data.xlsx now" + expect(buildHighlightedTextSegments(text, [], [])).toEqual([{ text }]) + }) + + test("fallback regex does not over-match ordinary prose following an unrelated @mention", () => { + const text = "@code-reviewer check the report for v1.2 details" + expect(buildHighlightedTextSegments(text, [], [])).toEqual([{ text }]) + }) + + test("fallback regex still detects a plain mention without swallowing trailing prose", () => { + const text = "see @src/index.ts for v1.2 details" + expect(buildHighlightedTextSegments(text, [], [])).toEqual([ + { text: "see " }, + { text: "@src/index.ts", type: "file" }, + { text: " for v1.2 details" }, + ]) + }) + + test("highlights every repeated occurrence of a plain mention when only one ref exists", () => { + // mentionedPaths is a Set, so buildFileAttachments only ever produces one + // ref per unique path even when it's mentioned twice in the same message. + const text = "compare @src/a.ts with @src/a.ts" + const segments = buildHighlightedTextSegments( + text, + [{ source: { type: "file", path: "src/a.ts", text: { value: "@src/a.ts", start: 8, end: 17 } } }], + [], + ) + + expect(segments).toEqual([ + { text: "compare " }, + { text: "@src/a.ts", type: "file" }, + { text: " with " }, + { text: "@src/a.ts", type: "file" }, + ]) + }) + + test("does not let a shorter mention's repeat-search truncate a longer mention that starts the same way", () => { + // "@a.ts" is a literal prefix of "@a.tsx". A naive repeat-search for the + // first ref's value would match inside the second, distinct mention and + // drop its highlight (or worse, wrongly highlight only part of it). + const text = "see @a.ts and also @a.tsx" + const segments = buildHighlightedTextSegments( + text, + [ + { source: { type: "file", path: "a.ts", text: { value: "@a.ts", start: 4, end: 9 } } }, + { source: { type: "file", path: "a.tsx", text: { value: "@a.tsx", start: 19, end: 25 } } }, + ], + [], + ) + + expect(segments).toEqual([ + { text: "see " }, + { text: "@a.ts", type: "file" }, + { text: " and also " }, + { text: "@a.tsx", type: "file" }, + ]) + }) + + test("highlights a repeated mention directly followed by a comma", () => { + const text = "check @file.ts, then @file.ts, done" + const segments = buildHighlightedTextSegments( + text, + [{ source: { type: "file", path: "file.ts", text: { value: "@file.ts", start: 6, end: 14 } } }], + [], + ) + + expect(segments).toEqual([ + { text: "check " }, + { text: "@file.ts", type: "file" }, + { text: ", then " }, + { text: "@file.ts", type: "file" }, + { text: ", done" }, + ]) + }) + + test("highlights a repeated mention directly followed by a sentence-ending period", () => { + const text = "check @file.ts, then @file.ts." + const segments = buildHighlightedTextSegments( + text, + [{ source: { type: "file", path: "file.ts", text: { value: "@file.ts", start: 6, end: 14 } } }], + [], + ) + + expect(segments).toEqual([ + { text: "check " }, + { text: "@file.ts", type: "file" }, + { text: ", then " }, + { text: "@file.ts", type: "file" }, + { text: "." }, + ]) + }) + + test("highlights a repeated mention directly followed by a closing paren", () => { + const text = "see (@a.ts) and (@a.ts) again" + const segments = buildHighlightedTextSegments( + text, + [{ source: { type: "file", path: "a.ts", text: { value: "@a.ts", start: 5, end: 10 } } }], + [], + ) + + expect(segments).toEqual([ + { text: "see (" }, + { text: "@a.ts", type: "file" }, + { text: ") and (" }, + { text: "@a.ts", type: "file" }, + { text: ") again" }, + ]) + }) + + test("still rejects a repeat that is actually a prefix of a longer compound extension", () => { + // "@report.csv" is a literal prefix of "@report.csv.bak" — the second + // dot is followed by a word character, so it counts as a continuation + // and the shorter mention must not falsely match inside the longer one. + const text = "compare @report.csv with backup @report.csv.bak" + const segments = buildHighlightedTextSegments( + text, + [{ source: { type: "file", path: "report.csv", text: { value: "@report.csv", start: 8, end: 19 } } }], + [], + ) + + expect(segments).toEqual([{ text: "compare " }, { text: "@report.csv", type: "file" }, { text: " with backup @report.csv.bak" }]) + }) + + test("does not let a shorter Cyrillic mention's repeat-search collide with a longer, distinct one", () => { + // "@файл" is a literal prefix of "@файлы" (Cyrillic "ы" appended), the + // same structural collision as "@a.ts" inside "@a.tsx" but for a + // non-ASCII continuation character that \w alone would not recognize. + const text = "open @файл and also @файлы" + const segments = buildHighlightedTextSegments( + text, + [ + { source: { type: "file", path: "файл", text: { value: "@файл", start: 5, end: 10 } } }, + { source: { type: "file", path: "файлы", text: { value: "@файлы", start: 20, end: 26 } } }, + ], + [], + ) + + expect(segments).toEqual([ + { text: "open " }, + { text: "@файл", type: "file" }, + { text: " and also " }, + { text: "@файлы", type: "file" }, + ]) + }) + + test("does not let a shorter CJK mention's repeat-search collide with a longer, distinct one", () => { + // "@文件" is a literal prefix of "@文件夹" ("夹" appended). + const text = "check @文件 and also @文件夹 folder" + const segments = buildHighlightedTextSegments( + text, + [ + { source: { type: "file", path: "文件", text: { value: "@文件", start: 6, end: 9 } } }, + { source: { type: "file", path: "文件夹", text: { value: "@文件夹", start: 19, end: 23 } } }, + ], + [], + ) + + expect(segments).toEqual([ + { text: "check " }, + { text: "@文件", type: "file" }, + { text: " and also " }, + { text: "@文件夹", type: "file" }, + { text: " folder" }, + ]) + }) + + test("highlights every repeated occurrence of a mention containing a space when only one ref exists", () => { + const text = "a @dup name.ts b @dup name.ts c" + const segments = buildHighlightedTextSegments( + text, + [{ source: { type: "file", path: "dup name.ts", text: { value: "@dup name.ts", start: 2, end: 14 } } }], + [], + ) + + expect(segments).toEqual([ + { text: "a " }, + { text: "@dup name.ts", type: "file" }, + { text: " b " }, + { text: "@dup name.ts", type: "file" }, + { text: " c" }, + ]) + }) + + test("highlights Cyrillic filename when source offsets are provided", () => { + const text = "open @файл.txt please" + const segments = buildHighlightedTextSegments( + text, + [ + { + source: { + type: "file", + path: "файл.txt", + text: { value: "@файл.txt", start: 5, end: 14 }, + }, + }, + ], + [], + ) + + expect(segments).toEqual([{ text: "open " }, { text: "@файл.txt", type: "file" }, { text: " please" }]) + }) + + test("highlights Chinese filename when source offsets are provided", () => { + const text = "read @文件.txt" + const segments = buildHighlightedTextSegments( + text, + [ + { + source: { + type: "file", + path: "文件.txt", + text: { value: "@文件.txt", start: 5, end: 11 }, + }, + }, + ], + [], + ) + + expect(segments).toEqual([{ text: "read " }, { text: "@文件.txt", type: "file" }]) + }) + + test("does not let a repeated mention hide a distinct mention that sits between the repeats", () => { + // Regression: for "@a.ts @b.ts @a.ts", locating the first "@a.ts" ref must + // not swallow "@b.ts" while scanning forward for the second "@a.ts" repeat. + const text = "@a.ts @b.ts @a.ts" + const segments = buildHighlightedTextSegments( + text, + [ + { source: { type: "file", path: "a.ts", text: { value: "@a.ts", start: 0, end: 5 } } }, + { source: { type: "file", path: "b.ts", text: { value: "@b.ts", start: 6, end: 11 } } }, + ], + [], + ) + + expect(segments).toEqual([ + { text: "@a.ts", type: "file" }, + { text: " " }, + { text: "@b.ts", type: "file" }, + { text: " " }, + { text: "@a.ts", type: "file" }, + ]) + }) + + test("does not let a repeated shorter mention with spaces collide with a longer, distinct mention that starts the same way", () => { + // "@a.txt" is a literal prefix of the space-containing "@a.txt backup.txt". + // A generic continuation-character heuristic would treat the space after + // "@a.txt" as a valid boundary, since paths may now legitimately contain + // spaces. Checking against the other ref's actual mention text catches this. + const text = "@a.txt @a.txt backup.txt" + const segments = buildHighlightedTextSegments( + text, + [ + { source: { type: "file", path: "a.txt", text: { value: "@a.txt", start: 0, end: 6 } } }, + { + source: { + type: "file", + path: "a.txt backup.txt", + text: { value: "@a.txt backup.txt", start: 7, end: 24 }, + }, + }, + ], + [], + ) + + expect(segments).toEqual([ + { text: "@a.txt", type: "file" }, + { text: " " }, + { text: "@a.txt backup.txt", type: "file" }, + ]) + }) + + test("highlights filename with space in directory and source offsets", () => { + const text = "using @my folder/report.xlsx here" + const segments = buildHighlightedTextSegments( + text, + [ + { + source: { + type: "file", + path: "my folder/report.xlsx", + text: { value: "@my folder/report.xlsx", start: 6, end: 28 }, + }, + }, + ], + [], + ) + + expect(segments).toEqual([ + { text: "using " }, + { text: "@my folder/report.xlsx", type: "file" }, + { text: " here" }, + ]) + }) }) diff --git a/packages/kilo-ui/src/components/message-highlight.ts b/packages/kilo-ui/src/components/message-highlight.ts index 7bbfc7762e9..3c0e16ac9ab 100644 --- a/packages/kilo-ui/src/components/message-highlight.ts +++ b/packages/kilo-ui/src/components/message-highlight.ts @@ -21,7 +21,16 @@ type Ref = { type: "file" | "agent" } -/** Match @path mentions: `@` followed by a path-like token (contains `/` or `.`). */ +/** + * Match @path mentions: `@` followed by a path-like token (contains `/` or `.`). + * This regex is the fallback used only when no source position data is available + * (e.g. messages sent before file attachments carried source.text). It intentionally + * does not match spaces: a pattern permissive enough to span space-separated path + * segments also matches ordinary prose following any @mention (e.g. `@agent check + * the report for v1.2 details` would swallow everything up to `v1.2`). Paths with + * spaces are highlighted correctly via the source.text-based resolve() path instead, + * which locates the exact known mention text rather than pattern-matching prose. + */ const MENTION_RE = /@([\w./-]+\.[\w]+|[\w.-]+\/[\w./-]+)/g function detect(text: string): Ref[] { @@ -31,35 +40,128 @@ function detect(text: string): Ref[] { })) } -function locate(text: string, ref: Ref, index: number): Ref | undefined { +function locate(text: string, ref: Ref, claimed: { start: number; end: number }[]): Ref | undefined { const source = ref.source if (!source.value) return undefined + const free = (start: number, end: number) => !claimed.some((c) => start < c.end && end > c.start) + if (Number.isFinite(source.start) && Number.isFinite(source.end)) { const start = Math.min(text.length, Math.max(0, source.start)) const end = Math.min(text.length, Math.max(0, source.end)) - if (start >= index && start <= end && text.slice(start, end) === source.value) { + if (text.slice(start, end) === source.value && free(start, end)) { return { ...ref, source: { ...source, start, end } } } } - const hint = Number.isFinite(source.start) ? Math.min(text.length, Math.max(index, source.start)) : index - const found = text.indexOf(source.value, hint) - const start = found === -1 ? text.indexOf(source.value, index) : found - if (start === -1) return undefined - return { ...ref, source: { ...source, start, end: start + source.value.length } } + const firstFree = (from: number) => { + let search = from + while (true) { + const found = text.indexOf(source.value, search) + if (found === -1) return undefined + const end = found + source.value.length + if (free(found, end)) return { start: found, end } + search = found + 1 + } + } + + // Prefer the first unclaimed occurrence at or after this ref's own recorded + // position, falling back to the first unclaimed occurrence anywhere. Each + // ref is searched against its own hint rather than a cursor shared across + // every other ref, so locating one ref can't skip past a distinct ref's real + // occurrence that sits between two repeats of an earlier one (see the + // interleaved-mentions regression test). + const hint = Number.isFinite(source.start) ? Math.min(text.length, Math.max(0, source.start)) : 0 + const match = firstFree(hint) ?? firstFree(0) + if (!match) return undefined + return { ...ref, source: { ...source, start: match.start, end: match.end } } +} + +// Any letter, digit, underscore, slash, or hyphen unambiguously continues a +// path token. Uses Unicode property escapes rather than \w, which matches +// ASCII letters/digits only in JavaScript regex — without this, a Cyrillic +// or CJK mention (e.g. "@файл") would not be recognized as continuing into a +// longer, distinct mention that starts the same way (e.g. "@файлы"), +// reintroducing the same collision this check exists to prevent. A dot is +// handled separately (see continuesPath) since it is both a common +// sentence-ending character and a path/extension separator. +const PATH_CONTINUATION = /[\p{L}\p{N}_/-]/u + +/** + * Whether `text[end]` extends a match into a longer, different path rather + * than ending it. A dot only counts as a continuation when another + * letter/digit follows (e.g. "@report.csv" + ".bak", or the "x" in "@a.tsx" + * itself is already caught by PATH_CONTINUATION) — a lone trailing dot, as + * in an ordinary sentence ending, does not. + */ +function continuesPath(text: string, end: number): boolean { + const char = text[end] + if (char === undefined) return false + if (PATH_CONTINUATION.test(char)) return true + return char === "." && /[\p{L}\p{N}_]/u.test(text[end + 1] ?? "") +} + +/** + * Find every position at or after `from` where `value` occurs as a complete + * token, not immediately preceded by a character that could extend it into a + * longer, different path, and not immediately claimed by `others` — the exact + * mention text of every other known ref. A plain substring search would let a + * shorter mention match as a prefix of a longer, distinct one that starts the + * same way (e.g. "@a.ts" inside "@a.tsx", or "@a.txt" inside the space-containing + * "@a.txt backup.txt"). Checking against the other refs' actual mention text, + * rather than only a generic continuation-character heuristic, is required + * because a space can no longer be assumed to end a mention now that paths may + * contain spaces — `continuesPath` alone would treat the boundary before + * "backup.txt" as valid. Ordinary punctuation such as a trailing comma, + * sentence-ending period, or closing paren is not a continuation character, + * so a repeat directly followed by it is still accepted. + */ +function repeats(text: string, value: string, others: string[], claimed: { start: number; end: number }[]): number[] { + const result: number[] = [] + let search = 0 + + while (true) { + const found = text.indexOf(value, search) + if (found === -1) break + + const end = found + value.length + const before = found === 0 || !PATH_CONTINUATION.test(text[found - 1] ?? "") + const after = !continuesPath(text, end) + const collides = others.some((other) => other !== value && other.length > value.length && text.startsWith(other, found)) + const free = !claimed.some((c) => found < c.end && end > c.start) + if (before && after && !collides && free) result.push(found) + search = found + 1 + } + + return result } function resolve(text: string, refs: Ref[]): Ref[] { + const others = refs.map((ref) => ref.source.value) + const claimed: { start: number; end: number }[] = [] const result: Ref[] = [] - let index = 0 + // Locate each ref's own primary occurrence first, independently of every + // other ref, so a distinct ref's real occurrence sitting between two + // repeats of an earlier one is never skipped over (see the + // interleaved-mentions regression test). for (const ref of [...refs].sort((a, b) => a.source.start - b.source.start || b.source.end - a.source.end)) { - const next = locate(text, ref, index) + const next = locate(text, ref, claimed) if (!next) continue - result.push(next) - index = next.source.end + claimed.push({ start: next.source.start, end: next.source.end }) + } + + // mentionedPaths is a Set, so a path mentioned more than once in the same + // message only produces a single attachment/ref. Highlight any later + // boundary-delimited repeats of each located ref's mention text too, so + // every occurrence stays highlighted, not just the first. + for (const ref of [...result]) { + for (const start of repeats(text, ref.source.value, others, claimed)) { + const end = start + ref.source.value.length + result.push({ ...ref, source: { ...ref.source, start, end } }) + claimed.push({ start, end }) + } } return result diff --git a/packages/kilo-vscode/tests/unit/file-mention-utils.test.ts b/packages/kilo-vscode/tests/unit/file-mention-utils.test.ts index cd5a1903b34..64d2bbea554 100644 --- a/packages/kilo-vscode/tests/unit/file-mention-utils.test.ts +++ b/packages/kilo-vscode/tests/unit/file-mention-utils.test.ts @@ -155,6 +155,24 @@ describe("syncMentionedPaths", () => { const result = syncMentionedPaths(paths, "@foo.ts is important") expect(result.has("foo.ts")).toBe(true) }) + + it("does not false-match a stale shorter path against a longer, space-containing path that starts the same way", () => { + // "a.txt" is a known path from an earlier, unrelated mention. A space + // genuinely follows "a.txt" in the current text, but only because it's + // the start of the longer, distinct "a.txt backup.txt" -- a whitespace-only + // boundary check would incorrectly treat that as a valid, separate match. + const paths = new Set(["a.txt", "a.txt backup.txt"]) + const result = syncMentionedPaths(paths, "@a.txt backup.txt") + expect(result.has("a.txt backup.txt")).toBe(true) + expect(result.has("a.txt")).toBe(false) + }) + + it("keeps a shorter path when it also has its own genuine, separate occurrence", () => { + const paths = new Set(["a.txt", "a.txt backup.txt"]) + const result = syncMentionedPaths(paths, "@a.txt backup.txt and also @a.txt") + expect(result.has("a.txt backup.txt")).toBe(true) + expect(result.has("a.txt")).toBe(true) + }) }) describe("buildTextAfterMentionSelect", () => { @@ -300,6 +318,90 @@ describe("buildFileAttachments", () => { const result = buildFileAttachments("@foo.ts", paths, "C:\\Users\\workspace") expect(result[0]!.url).not.toContain("\\") }) + + it("includes source.text with correct position for a plain mention", () => { + const paths = new Set(["src/foo.ts"]) + const text = "check @src/foo.ts here" + const result = buildFileAttachments(text, paths, "/workspace") + expect(result[0]!.source).toEqual({ + type: "file", + path: "src/foo.ts", + text: { value: "@src/foo.ts", start: 6, end: 17 }, + }) + }) + + it("includes source.text for a filename with spaces", () => { + const paths = new Set(["org data.xlsx"]) + const text = "see @org data.xlsx now" + const result = buildFileAttachments(text, paths, "/workspace") + expect(result).toHaveLength(1) + expect(result[0]!.source).toEqual({ + type: "file", + path: "org data.xlsx", + text: { value: "@org data.xlsx", start: 4, end: 18 }, + }) + }) + + it("includes source.text for a Cyrillic filename", () => { + const paths = new Set(["файл.txt"]) + const text = "open @файл.txt" + const result = buildFileAttachments(text, paths, "/workspace") + expect(result).toHaveLength(1) + expect(result[0]!.source?.text.value).toBe("@файл.txt") + expect(result[0]!.source?.text.start).toBe(5) + }) + + it("includes source.text for a Chinese filename", () => { + const paths = new Set(["文件.txt"]) + const text = "@文件.txt" + const result = buildFileAttachments(text, paths, "/workspace") + expect(result).toHaveLength(1) + expect(result[0]!.source?.text.value).toBe("@文件.txt") + expect(result[0]!.source?.text.start).toBe(0) + }) + + it("includes source.text for a path with spaces in both dir and filename", () => { + const paths = new Set(["my folder/org data.xlsx"]) + const text = "using @my folder/org data.xlsx here" + const result = buildFileAttachments(text, paths, "/workspace") + expect(result).toHaveLength(1) + expect(result[0]!.source).toEqual({ + type: "file", + path: "my folder/org data.xlsx", + text: { value: "@my folder/org data.xlsx", start: 6, end: 30 }, + }) + }) + + it("percent-encodes spaces in the file URL so the server can decode it correctly", () => { + const paths = new Set(["org data.xlsx"]) + const result = buildFileAttachments("@org data.xlsx", paths, "/workspace") + expect(result).toHaveLength(1) + expect(result[0]!.url).not.toContain(" ") + expect(result[0]!.url).toContain("%20") + }) + + it("percent-encodes spaces in nested path segments", () => { + const paths = new Set(["my folder/my file.txt"]) + const result = buildFileAttachments("@my folder/my file.txt", paths, "/workspace") + expect(result).toHaveLength(1) + expect(result[0]!.url).not.toContain(" ") + expect(result[0]!.url).toContain("my%20folder") + expect(result[0]!.url).toContain("my%20file.txt") + }) + + it("round-trips a filename containing a literal percent-encoded-looking sequence", () => { + // Only escaping spaces before assigning to url.pathname is not enough: a + // real filename like "100%20real.txt" already contains the literal text + // "%20". If "%" itself isn't escaped first, the URL's "%20" is + // indistinguishable from an actually-encoded space, and decoding it (as + // Bun's fileURLToPath does server-side) would produce "100 real.txt" -- + // a different, wrong filename. + const paths = new Set(["100%20real.txt"]) + const result = buildFileAttachments("@100%20real.txt", paths, "/workspace") + expect(result).toHaveLength(1) + const decoded = decodeURIComponent(new URL(result[0]!.url).pathname) + expect(decoded).toBe("/workspace/100%20real.txt") + }) }) describe("getMentionRemovalRange", () => { diff --git a/packages/kilo-vscode/tests/unit/use-file-mention.test.ts b/packages/kilo-vscode/tests/unit/use-file-mention.test.ts index 06364592ee0..45a2834e787 100644 --- a/packages/kilo-vscode/tests/unit/use-file-mention.test.ts +++ b/packages/kilo-vscode/tests/unit/use-file-mention.test.ts @@ -213,6 +213,92 @@ describe("useFileMention", () => { dispose.fn?.() }) + it("seedFromText truncates a mention path at the first space (known limitation, see seedFromParts)", () => { + // Documents why seedFromParts exists: seedFromText re-derives candidate + // paths from raw text via a regex that stops at whitespace. For a path + // containing a space, it discovers only the prefix before the space, and + // that truncated candidate then incorrectly passes syncMentionedPaths' + // boundary check too, since a real space genuinely follows it in the text. + const ctx = { + postMessage: () => {}, + onMessage: () => () => {}, + } + + const dispose: { fn?: () => void } = {} + const mention = createRoot((root) => { + dispose.fn = root + return useFileMention(ctx, undefined, () => false) + }) + + mention.seedFromText("Say hi to @mention-test/my quarterly report.txt !") + + expect(mention.mentionedPaths().has("mention-test/my")).toBe(true) + expect(mention.mentionedPaths().has("mention-test/my quarterly report.txt")).toBe(false) + + dispose.fn?.() + }) + + it("seedFromParts seeds an exact path correctly even when it contains a space", () => { + const ctx = { + postMessage: () => {}, + onMessage: () => () => {}, + } + + const dispose: { fn?: () => void } = {} + const mention = createRoot((root) => { + dispose.fn = root + return useFileMention(ctx, undefined, () => false) + }) + + const text = "Say hi to @mention-test/my quarterly report.txt !" + mention.seedFromParts(["mention-test/my quarterly report.txt"], text) + + expect(mention.mentionedPaths().has("mention-test/my quarterly report.txt")).toBe(true) + expect(mention.mentionedPaths().has("mention-test/my")).toBe(false) + + dispose.fn?.() + }) + + it("seedFromParts prunes paths no longer present in the text", () => { + const ctx = { + postMessage: () => {}, + onMessage: () => () => {}, + } + + const dispose: { fn?: () => void } = {} + const mention = createRoot((root) => { + dispose.fn = root + return useFileMention(ctx, undefined, () => false) + }) + + mention.seedFromParts(["mention-test/gone.ts"], "no mentions here anymore") + + expect(mention.mentionedPaths().size).toBe(0) + + dispose.fn?.() + }) + + it("seedFromParts seeds multiple exact paths from a single message", () => { + const ctx = { + postMessage: () => {}, + onMessage: () => () => {}, + } + + const dispose: { fn?: () => void } = {} + const mention = createRoot((root) => { + dispose.fn = root + return useFileMention(ctx, undefined, () => false) + }) + + const text = "Compare @a data.ts and @b data.ts please" + mention.seedFromParts(["a data.ts", "b data.ts"], text) + + expect(mention.mentionedPaths().has("a data.ts")).toBe(true) + expect(mention.mentionedPaths().has("b data.ts")).toBe(true) + + dispose.fn?.() + }) + it("filters visible results synchronously while a new search is pending", async () => { const posted: WebviewMessage[] = [] const handlers = new Set<(message: ExtensionMessage) => void>() diff --git a/packages/kilo-vscode/webview-ui/src/components/chat/PromptInput.tsx b/packages/kilo-vscode/webview-ui/src/components/chat/PromptInput.tsx index 13e6518816b..b80e70f0bbe 100644 --- a/packages/kilo-vscode/webview-ui/src/components/chat/PromptInput.tsx +++ b/packages/kilo-vscode/webview-ui/src/components/chat/PromptInput.tsx @@ -629,7 +629,12 @@ export const PromptInput: Component = (props) => { if (message.type === "setChatBoxMessage") { setText(message.text) - mention.seedFromText(message.text) + // Prefer the exact attachment paths when available (e.g. reverting to a + // message with @mentions) — seedFromText re-derives candidate mentions + // from raw text via regex, which truncates at the first space in a + // filename and cannot be relied on to reconstruct spaced paths correctly. + if (message.paths?.length) mention.seedFromParts(message.paths, message.text) + else mention.seedFromText(message.text) if (textareaRef) { textareaRef.value = message.text adjustHeight() diff --git a/packages/kilo-vscode/webview-ui/src/context/session.tsx b/packages/kilo-vscode/webview-ui/src/context/session.tsx index eb4b9ffc5d2..06ee1173e2b 100644 --- a/packages/kilo-vscode/webview-ui/src/context/session.tsx +++ b/packages/kilo-vscode/webview-ui/src/context/session.tsx @@ -2781,7 +2781,15 @@ export const SessionProvider: ParentComponent = (props) => { .filter((p) => p.type === "text" && !(p as { synthetic?: boolean }).synthetic) .map((p) => (p as { text: string }).text ?? "") .join("") - if (text) window.postMessage({ type: "setChatBoxMessage", text }, "*") + // Pass the original attachments' exact paths alongside the restored text + // so PromptInput can seed them directly rather than re-deriving mentions + // from the text via regex, which truncates at the first space in a + // filename (see PromptInput's setChatBoxMessage handler). + const paths = parts + .filter((p): p is Extract => p.type === "file") + .map((p) => p.source?.path) + .filter((p): p is string => !!p) + if (text) window.postMessage({ type: "setChatBoxMessage", text, paths }, "*") } vscode.postMessage({ type: "revertSession", sessionID: id, messageID, partID }) } diff --git a/packages/kilo-vscode/webview-ui/src/hooks/file-mention-utils.ts b/packages/kilo-vscode/webview-ui/src/hooks/file-mention-utils.ts index c2d9ecb1273..2a6b2c3b666 100644 --- a/packages/kilo-vscode/webview-ui/src/hooks/file-mention-utils.ts +++ b/packages/kilo-vscode/webview-ui/src/hooks/file-mention-utils.ts @@ -33,13 +33,6 @@ export const FILE_PICKER_RESULT: MentionResult = { description: "Select a file outside the workspace", } -/** - * Escape special regex characters in a string so it can be used in a RegExp. - */ -function escape(str: string): string { - return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") -} - export function getTerminalMentionResult(query: string): MentionResult[] { const normalized = query.toLowerCase() if (!TERMINAL_MENTION.startsWith(normalized)) return [] @@ -84,14 +77,38 @@ export function filterMentionResults(query: string, items: MentionResult[]): Men * * Uses boundary-aware matching (whitespace or start/end of string) and processes * paths longest-first to prevent `@src/a.ts` from false-matching `@src/a.tsx`. + * + * A trailing space can no longer be assumed to end a mention now that paths + * may contain spaces: `@a.txt` is a literal, whitespace-bounded prefix of the + * space-containing `@a.txt backup.txt`. Checking each candidate occurrence + * against every longer path already accepted at the same position (rather + * than relying on whitespace alone) prevents a stale, unrelated `a.txt` from + * a prior mention surviving just because it happens to collide with the + * start of a longer path mentioned in the current text. */ export function syncMentionedPaths(prev: Set, text: string): Set { const next = new Set() // Sort longest-first so e.g. "src/a.tsx" is checked before "src/a.ts" const sorted = [...prev].sort((a, b) => b.length - a.length) + const accepted: string[] = [] for (const path of sorted) { - const pattern = new RegExp(`(?:^|\\s)@${escape(path)}(?:\\s|$)`) - if (pattern.test(text)) next.add(path) + const token = `@${path}` + let search = 0 + const valid = (() => { + while (true) { + const idx = text.indexOf(token, search) + if (idx === -1) return false + const before = idx === 0 || /\s/.test(text[idx - 1] ?? "") + const end = idx + token.length + const after = end >= text.length || /\s/.test(text[end] ?? "") + const collides = accepted.some((other) => other !== path && text.startsWith(`@${other}`, idx)) + if (before && after && !collides) return true + search = idx + 1 + } + })() + if (!valid) continue + accepted.push(path) + next.add(path) } return next } @@ -228,6 +245,10 @@ function isInsideWorkspace(abs: string, dir: string): boolean { * which enforces the normal external-directory permission checks. Every * resolved path (relative or absolute) is normalized before the containment * check so a "../" sequence can't slip past a literal string-prefix match. + * + * Includes source.text position data so the message renderer can highlight + * the full mention span (including paths with spaces or non-ASCII characters) + * without falling back to the regex-based detection that stops at spaces. */ export function buildFileAttachments( text: string, @@ -237,13 +258,32 @@ export function buildFileAttachments( const result: FileAttachment[] = [] const dir = normalizeAbsolutePath(workspaceDir.replaceAll("\\", "/")).replace(/\/+$/, "") for (const path of mentionedPaths) { - if (text.includes(`@${path}`)) { + const token = `@${path}` + const idx = text.indexOf(token) + if (idx !== -1) { const raw = isAbsolutePath(path) ? path.replaceAll("\\", "/") : `${dir}/${path}` const abs = normalizeAbsolutePath(raw) if (!isInsideWorkspace(abs, dir)) continue const url = new URL("file://") - url.pathname = abs.startsWith("/") ? abs : `/${abs}` - result.push({ mime: "text/plain", url: url.href }) + // Pre-encode spaces and literal percent signs before assigning to + // pathname: VS Code's webview (Chromium) does not percent-encode spaces + // in file:// URL pathnames, which causes Bun's fileURLToPath on the + // server to truncate the path at the first space. A literal "%" in the + // filename must also be escaped first (to "%25"), otherwise a name like + // "100%20real.txt" would be indistinguishable from an already-encoded + // space and get decoded back to "100 real.txt" server-side. Other + // non-ASCII characters are encoded correctly by the URL class, so only + // "%" and " " need this explicit treatment. + url.pathname = (abs.startsWith("/") ? abs : `/${abs}`).replace(/%/g, "%25").replace(/ /g, "%20") + result.push({ + mime: "text/plain", + url: url.href, + source: { + type: "file", + path, + text: { value: token, start: idx, end: idx + token.length }, + }, + }) } } return result diff --git a/packages/kilo-vscode/webview-ui/src/hooks/useFileMention.ts b/packages/kilo-vscode/webview-ui/src/hooks/useFileMention.ts index eec16d67afd..dd9d396da93 100644 --- a/packages/kilo-vscode/webview-ui/src/hooks/useFileMention.ts +++ b/packages/kilo-vscode/webview-ui/src/hooks/useFileMention.ts @@ -68,6 +68,13 @@ export interface FileMention { seedFromText: (text: string) => void /** Insert a file-picker result at the stored cursor position. Ignored unless requestId matches the pending request. */ insertFilePickerResult: (path: string, requestId: string) => void + /** + * Seed known paths from a set of already-confirmed exact paths (e.g. a + * reverted message's file attachments), then prune against `text`. Prefer + * this over seedFromText when exact paths are available, since seedFromText + * cannot correctly rediscover paths containing spaces from raw text alone. + */ + seedFromParts: (paths: string[], text: string) => void } export function useFileMention( @@ -430,6 +437,19 @@ export function useFileMention( state.onSelect?.() } + // Seed known paths from a set of already-confirmed exact paths (e.g. the + // file attachments of a message being restored after a revert), then prune + // mentionedPaths against the current text. Unlike seedFromText, this does + // not re-derive candidate paths from the text via regex: that regex cannot + // distinguish a complete mention from a truncated prefix when the real + // path contains a space (e.g. it would discover only "dir/my" from + // "@dir/my report.txt", which then passes syncMentionedPaths' boundary + // check too, since a real space genuinely follows "my" in the full name). + const seedFromParts = (paths: string[], text: string) => { + for (const p of paths) knownPaths.add(p) + syncMentionedPaths(text) + } + return { mentionedPaths, mentionResults, @@ -447,5 +467,6 @@ export function useFileMention( snapSelection, seedFromText, insertFilePickerResult, + seedFromParts, } } diff --git a/packages/kilo-vscode/webview-ui/src/types/messages/extension-messages.ts b/packages/kilo-vscode/webview-ui/src/types/messages/extension-messages.ts index 8f492cdabe7..c3132d9d77c 100644 --- a/packages/kilo-vscode/webview-ui/src/types/messages/extension-messages.ts +++ b/packages/kilo-vscode/webview-ui/src/types/messages/extension-messages.ts @@ -269,6 +269,14 @@ export interface ActionMessage { export interface SetChatBoxMessage { type: "setChatBoxMessage" text: string + /** + * Exact relative paths of the file attachments carried by the restored + * message, if known (e.g. when reverting to a message that had @mentions). + * When present, PromptInput seeds these directly instead of re-deriving + * candidate mentions from the text via regex, which cannot tell a complete + * mention from a truncated prefix when the real path contains a space. + */ + paths?: string[] } export interface AppendChatBoxMessage {