From 7bd47062601fc89d33be0d297508d33c12b0edce Mon Sep 17 00:00:00 2001 From: Imanol Maiztegui Date: Wed, 1 Apr 2026 14:20:08 +0200 Subject: [PATCH] fix(vscode): normalize migrated session paths (#8062) * feat(vscode): add legacy migration path normalization utility Introduce `normalizeLegacyPath` and supporting helpers for the legacy session migration path. The utility resolves and normalizes raw path inputs into a stable absolute form, including uppercasing Windows drive letters to match canonical casing used by path filters downstream. - `normalizeLegacyPath`: trims, resolves, normalizes, and calls `fs.realpath`, falling back to the normalized path on failure - `isWindowsDrivePath`: detects Windows-style drive paths cross-platform - `normalizeWindowsDriveLetter`: uppercases the drive letter prefix Unit tests cover empty input, Windows drive detection, drive-letter uppercasing, and `realpath` failure fallback. * refactor(legacy-migration): make `createSession` async to resolve workspace path Wire `normalizeLegacyPath` into `createSession` by converting it to an async function. The session directory is now resolved through the path normalization utility instead of using the raw workspace string directly. Update `parseSession` to await the async `createSession` call, and update all unit tests to handle the now-async API. * fix(legacy-migration): lift path normalization out of `createSession` into `parseSession` Move the `normalizeLegacyPath` call from `createSession` to `parseSession` so that path resolution happens once at the top of the pipeline. The resolved `dir` is then threaded explicitly into `createSession`, `createProject`, and `parseMessagesFromConversation` as a parameter, removing the need for `createSession` to be async. * refactor(messages): make `dir` parameter optional via overloaded signature Allow `parseMessagesFromConversation` to accept either a `string` dir plus an optional `LegacyHistoryItem`, or a `LegacyHistoryItem` directly from which the workspace path is extracted. This removes the requirement for callers to destructure the item before passing it in. Also tighten the path normalization tests to use `path.normalize` and `path.resolve` for cross-platform correctness and drop the empty `afterEach` hook. * test(legacy-migration): fix Windows drive letter uppercasing assertions Replace hardcoded backslash expectations with `path.normalize`/`path.resolve` so the test behaves correctly across platforms. Also align the mock to return its input unchanged and use a lowercase drive letter as the starting value to properly exercise the uppercasing logic. * fix:formatting --- .../legacy-migration/sessions/lib/messages.ts | 11 +++-- .../src/legacy-migration/sessions/lib/path.ts | 25 ++++++++++ .../legacy-migration/sessions/lib/project.ts | 7 +-- .../legacy-migration/sessions/lib/session.ts | 3 +- .../src/legacy-migration/sessions/parser.ts | 9 ++-- .../unit/legacy-migration/messages.test.ts | 7 +-- .../unit/legacy-migration/parser.test.ts | 4 +- .../tests/unit/legacy-migration/path.test.ts | 48 +++++++++++++++++++ .../unit/legacy-migration/session.test.ts | 7 +-- 9 files changed, 103 insertions(+), 18 deletions(-) create mode 100644 packages/kilo-vscode/src/legacy-migration/sessions/lib/path.ts create mode 100644 packages/kilo-vscode/tests/unit/legacy-migration/path.test.ts diff --git a/packages/kilo-vscode/src/legacy-migration/sessions/lib/messages.ts b/packages/kilo-vscode/src/legacy-migration/sessions/lib/messages.ts index e59616216df..c0ddc3d75bb 100644 --- a/packages/kilo-vscode/src/legacy-migration/sessions/lib/messages.ts +++ b/packages/kilo-vscode/src/legacy-migration/sessions/lib/messages.ts @@ -10,11 +10,15 @@ type Assistant = Extract export function parseMessagesFromConversation( conversation: LegacyApiMessage[], id: string, + dirOrItem?: string | LegacyHistoryItem, item?: LegacyHistoryItem, ): Array> { + const dir = typeof dirOrItem === "string" ? dirOrItem : (dirOrItem?.workspace ?? "") + const next = typeof dirOrItem === "string" ? item : dirOrItem + return conversation .filter((entry) => entry.role === "user" || entry.role === "assistant") - .map((entry, index) => parseMessage(entry, index, id, item)) + .map((entry, index) => parseMessage(entry, index, id, dir, next)) .filter((message): message is NonNullable => Boolean(message)) } @@ -22,6 +26,7 @@ function parseMessage( entry: LegacyApiMessage, index: number, id: string, + dir: string, item?: LegacyHistoryItem, ): NonNullable | undefined { const created = entry.ts ?? item?.ts ?? 0 @@ -55,8 +60,8 @@ function parseMessage( mode: item?.mode ?? "code", agent: "main", path: { - cwd: item?.workspace ?? "", - root: item?.workspace ?? "", + cwd: dir, + root: dir, }, cost: 0, tokens: { diff --git a/packages/kilo-vscode/src/legacy-migration/sessions/lib/path.ts b/packages/kilo-vscode/src/legacy-migration/sessions/lib/path.ts new file mode 100644 index 00000000000..4b841725400 --- /dev/null +++ b/packages/kilo-vscode/src/legacy-migration/sessions/lib/path.ts @@ -0,0 +1,25 @@ +import * as fs from "fs/promises" +import * as path from "path" + +export async function normalizeLegacyPath(input?: string): Promise { + const raw = input?.trim() + if (!raw) return "" + + // Collapse legacy paths into one stable absolute form before importing them. + const normalized = path.normalize(path.resolve(raw)) + const canonical = normalizeWindowsDriveLetter(normalized) + + return fs.realpath(canonical).catch(() => canonical) +} + +export function isWindowsDrivePath(input: string): boolean { + return /^[a-z]:[\\/]/i.test(input) +} + +function normalizeWindowsDriveLetter(input: string): string { + // Match the canonical drive-letter casing used later by Windows path filters. + if (!isWindowsDrivePath(input)) return input + const head = input[0] + if (!head) return input + return head.toUpperCase() + input.slice(1) +} diff --git a/packages/kilo-vscode/src/legacy-migration/sessions/lib/project.ts b/packages/kilo-vscode/src/legacy-migration/sessions/lib/project.ts index b073f535d19..6a06b364796 100644 --- a/packages/kilo-vscode/src/legacy-migration/sessions/lib/project.ts +++ b/packages/kilo-vscode/src/legacy-migration/sessions/lib/project.ts @@ -4,12 +4,13 @@ import { createProjectID } from "./ids" export function createProject(item?: LegacyHistoryItem): NonNullable { const project = makeProject() + const dir = item?.workspace ?? "" - project.id = createProjectID(item?.workspace) + project.id = createProjectID(dir) - project.worktree = item?.workspace ?? "" + project.worktree = dir - project.sandboxes = item?.workspace ? [item.workspace] : [] + project.sandboxes = dir ? [dir] : [] project.timeCreated = item?.ts ?? 0 diff --git a/packages/kilo-vscode/src/legacy-migration/sessions/lib/session.ts b/packages/kilo-vscode/src/legacy-migration/sessions/lib/session.ts index 14058c6e94f..725aacf6059 100644 --- a/packages/kilo-vscode/src/legacy-migration/sessions/lib/session.ts +++ b/packages/kilo-vscode/src/legacy-migration/sessions/lib/session.ts @@ -6,6 +6,7 @@ export function createSession( id: string, item: LegacyHistoryItem | undefined, projectID: string, + dir: string, ): NonNullable { const session = makeSession() @@ -15,7 +16,7 @@ export function createSession( session.slug = id - session.directory = item?.workspace ?? "" + session.directory = dir session.title = item?.task ?? id diff --git a/packages/kilo-vscode/src/legacy-migration/sessions/parser.ts b/packages/kilo-vscode/src/legacy-migration/sessions/parser.ts index 76a063b3f80..966ba46bf41 100644 --- a/packages/kilo-vscode/src/legacy-migration/sessions/parser.ts +++ b/packages/kilo-vscode/src/legacy-migration/sessions/parser.ts @@ -8,6 +8,7 @@ import type { import { getApiConversationHistory, parseFile } from "./lib/legacy-conversation" import { parseMessagesFromConversation } from "./lib/messages" import { parsePartsFromConversation } from "./lib/parts/parts" +import { normalizeLegacyPath } from "./lib/path" import { createProject } from "./lib/project" import { createSession } from "./lib/session" @@ -19,11 +20,13 @@ export interface NormalizedSession { } export async function parseSession(id: string, dir: string, item?: LegacyHistoryItem): Promise { - const project = createProject(item) - const session = createSession(id, item, project.id) + const root = await normalizeLegacyPath(item?.workspace) + const next = item ? { ...item, workspace: root } : undefined + const project = createProject(next) + const session = createSession(id, next, project.id, root) const file = await getApiConversationHistory(id, dir) const conversation = parseFile(file) - const messages = parseMessagesFromConversation(conversation, id, item) + const messages = parseMessagesFromConversation(conversation, id, root, next) const parts = parsePartsFromConversation(conversation, id, item) return { diff --git a/packages/kilo-vscode/tests/unit/legacy-migration/messages.test.ts b/packages/kilo-vscode/tests/unit/legacy-migration/messages.test.ts index 69450d78025..f89e2fc330e 100644 --- a/packages/kilo-vscode/tests/unit/legacy-migration/messages.test.ts +++ b/packages/kilo-vscode/tests/unit/legacy-migration/messages.test.ts @@ -43,7 +43,7 @@ function sample(): LegacyApiMessage[] { describe("legacy migration messages", () => { it("parses a basic legacy conversation into ordered user and assistant messages with stable ids", () => { - const list = parseMessagesFromConversation(sample(), id, item) + const list = parseMessagesFromConversation(sample(), id, item.workspace, item) expect(list).toHaveLength(2) expect(list[0]?.data.role).toBe("user") @@ -53,7 +53,7 @@ describe("legacy migration messages", () => { }) it("creates valid assistant message metadata for the SDK/backend shape", () => { - const list = parseMessagesFromConversation(sample(), id, item) + const list = parseMessagesFromConversation(sample(), id, item.workspace, item) const msg = list.find((x) => x.data.role === "assistant") expect(msg?.data.role).toBe("assistant") @@ -66,7 +66,7 @@ describe("legacy migration messages", () => { }) it("ignores unsupported legacy entries instead of producing broken messages", () => { - const list = parseMessagesFromConversation(sample(), id, item) + const list = parseMessagesFromConversation(sample(), id, item.workspace, item) expect(list).toHaveLength(2) expect(list.some((x) => x.data.role !== "user" && x.data.role !== "assistant")).toBe(false) @@ -92,6 +92,7 @@ describe("legacy migration messages", () => { }, ], id, + item.workspace, item, ) diff --git a/packages/kilo-vscode/tests/unit/legacy-migration/parser.test.ts b/packages/kilo-vscode/tests/unit/legacy-migration/parser.test.ts index db2477b265d..0d3d9fa34ea 100644 --- a/packages/kilo-vscode/tests/unit/legacy-migration/parser.test.ts +++ b/packages/kilo-vscode/tests/unit/legacy-migration/parser.test.ts @@ -15,14 +15,14 @@ const item = { describe("legacy migration parser", () => { it("uses the final deterministic ids expected for migration", () => { const project = createProject(item) - const session = createSession(id, item, project.id) + const session = createSession(id, item, project.id, item.workspace) const msg1 = createMessageID(id, 0) const msg2 = createMessageID(id, 1) const prt1 = createPartID(id, 0, 0) const prt2 = createPartID(id, 1, 0) expect(project.id).toBe(createProject(item).id) - expect(session.id).toBe(createSession(id, item, project.id).id) + expect(session.id).toBe(createSession(id, item, project.id, item.workspace).id) expect(session.id.startsWith("ses_migrated_")).toBe(true) expect(msg1).toBe(createMessageID(id, 0)) expect(msg2).toBe(createMessageID(id, 1)) diff --git a/packages/kilo-vscode/tests/unit/legacy-migration/path.test.ts b/packages/kilo-vscode/tests/unit/legacy-migration/path.test.ts new file mode 100644 index 00000000000..71e807c1560 --- /dev/null +++ b/packages/kilo-vscode/tests/unit/legacy-migration/path.test.ts @@ -0,0 +1,48 @@ +import { beforeEach, describe, expect, it, mock } from "bun:test" +import * as path from "path" + +const realpath = mock(async (input: string) => input) + +mock.module("fs/promises", () => ({ + realpath, +})) + +const { isWindowsDrivePath, normalizeLegacyPath } = await import("../../../src/legacy-migration/sessions/lib/path") + +describe("legacy migration path", () => { + beforeEach(() => { + realpath.mockReset() + realpath.mockImplementation(async (input: string) => input) + }) + + it("returns an empty string for empty legacy paths", async () => { + expect(await normalizeLegacyPath(" ")).toBe("") + expect(realpath).not.toHaveBeenCalled() + }) + + it("detects Windows drive paths without relying on runtime platform", () => { + expect(isWindowsDrivePath("c:\\repo")).toBe(true) + expect(isWindowsDrivePath("C:/repo")).toBe(true) + expect(isWindowsDrivePath("/repo")).toBe(false) + }) + + it("uppercases the Windows drive letter before resolving the final path", async () => { + realpath.mockImplementation(async (input: string) => input) + + const value = await normalizeLegacyPath("c:/repo/../repo/file.txt") + const expected = path.normalize(path.resolve("c:/repo/../repo/file.txt")) + + expect(realpath).toHaveBeenCalledTimes(1) + expect(realpath.mock.calls[0]?.[0]).toBe(expected.replace(/^c:/, "C:")) + expect(value).toBe(expected.replace(/^c:/, "C:")) + }) + + it("falls back to the normalized path when realpath fails", async () => { + realpath.mockRejectedValueOnce(new Error("missing")) + + const input = "C:/repo/./child" + const value = await normalizeLegacyPath(input) + + expect(value).toBe(path.normalize(path.resolve(input))) + }) +}) diff --git a/packages/kilo-vscode/tests/unit/legacy-migration/session.test.ts b/packages/kilo-vscode/tests/unit/legacy-migration/session.test.ts index 7764b88ea4d..4faceb315cb 100644 --- a/packages/kilo-vscode/tests/unit/legacy-migration/session.test.ts +++ b/packages/kilo-vscode/tests/unit/legacy-migration/session.test.ts @@ -13,6 +13,7 @@ describe("legacy migration session", () => { mode: "code", }, "project-1", + "/workspace/testing", ) expect(session.projectID).toBe("project-1") @@ -24,15 +25,15 @@ describe("legacy migration session", () => { }) it("creates a deterministic session id from the legacy task id", () => { - const a = createSession("legacy-task-1", undefined, "project-1") - const b = createSession("legacy-task-1", undefined, "project-1") + const a = createSession("legacy-task-1", undefined, "project-1", "") + const b = createSession("legacy-task-1", undefined, "project-1", "") expect(a.id).toBe(b.id) expect(a.id.startsWith("ses_migrated_")).toBe(true) }) it("falls back to the legacy id as title when task metadata is missing", () => { - const session = createSession("legacy-task-1", undefined, "project-1") + const session = createSession("legacy-task-1", undefined, "project-1", "") expect(session.slug).toBe("legacy-task-1") expect(session.title).toBe("legacy-task-1")