From 8507bddb05467bac80038b6d4c7fadbc6aae4ae2 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Fri, 28 Aug 2026 09:36:03 +0200 Subject: [PATCH] refactor: centralize NUL-delimited composite keys --- AGENTS.md | 1 + packages/core/src/kilocode/zero-id.ts | 5 ++ packages/core/test/kilocode/zero-id.test.ts | 58 +++++++++++++++++++ .../src/agent-manager/local-diff-cache.ts | 13 +++-- .../src/agent-manager/project/route.ts | 4 +- .../services/cli-backend/explicit-abort.ts | 5 +- .../unit/agent-manager-worktree-diffs.test.ts | 32 +++++++++- .../tests/unit/agent-project-route.test.ts | 7 +++ .../tests/unit/explicit-abort.test.ts | 24 ++++++++ .../kilo-vscode/tests/unit/local-diff.test.ts | 20 +++++++ .../agent-manager/worktree-diffs.ts | 5 +- .../opencode/src/kilocode/indexing-warning.ts | 3 +- .../src/kilocode/indexing-worker-client.ts | 5 +- .../test/kilocode/indexing-warning.test.ts | 8 +++ .../test/kilocode/indexing-worker.test.ts | 32 ++++++++++ 15 files changed, 206 insertions(+), 16 deletions(-) create mode 100644 packages/core/src/kilocode/zero-id.ts create mode 100644 packages/core/test/kilocode/zero-id.test.ts diff --git a/AGENTS.md b/AGENTS.md index 45963d7b0ab..3e2954cb776 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,6 +93,7 @@ Examples: `fix(tui): simplify thinking toggle styling`, `docs: update contributi - Prefer single word variable names where possible - Use Bun APIs when possible, like `Bun.file()` - Rely on type inference when possible; avoid explicit type annotations or interfaces unless necessary for exports or clarity +- Prefer `Promise.withResolvers()` for deferreds when runtime/types support it; allow callback/event executors, not async executors or redundant Promise wrapping. ### Avoid let statements diff --git a/packages/core/src/kilocode/zero-id.ts b/packages/core/src/kilocode/zero-id.ts new file mode 100644 index 00000000000..b182d40724d --- /dev/null +++ b/packages/core/src/kilocode/zero-id.ts @@ -0,0 +1,5 @@ +export function zeroID(...parts: (string | number | boolean)[]) { + if (parts.length === 2) return `${parts[0]}\0${parts[1]}` + if (parts.length === 3) return `${parts[0]}\0${parts[1]}\0${parts[2]}` + return parts.join("\0") +} diff --git a/packages/core/test/kilocode/zero-id.test.ts b/packages/core/test/kilocode/zero-id.test.ts new file mode 100644 index 00000000000..9790f1e66e4 --- /dev/null +++ b/packages/core/test/kilocode/zero-id.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, expectTypeOf, test } from "bun:test" +import { zeroID } from "@opencode-ai/core/kilocode/zero-id" + +describe("zeroID", () => { + test("accepts only string, number, and boolean parts", () => { + expectTypeOf>().toEqualTypeOf<(string | number | boolean)[]>() + expectTypeOf>().toEqualTypeOf() + }) + + test("matches template literals for short composite keys", () => { + const values = ["", "a", "a\0b", "路径", "null", "undefined", 0, -0, -1, 0.5, NaN, Infinity, -Infinity, true, false] + for (const first of values) { + for (const second of values) { + expect(zeroID(first, second)).toBe(`${first}\0${second}`) + for (const third of values) { + expect(zeroID(first, second, third)).toBe(`${first}\0${second}\0${third}`) + } + } + } + }) + + test("matches array joins across arities and preserves empty parts", () => { + const cases: Parameters[] = [ + [], + [""], + [false], + [0], + ["", ""], + ["", "", ""], + ["", "", "", ""], + ["prefix", "", 0, false, "suffix"], + ["/repo", "", "ancestor", "file.ts", true, "modified", 3, 0, false, ""], + ["\0", "a\0b", "", "路径", NaN, -0, -Infinity, true, false], + ] + for (const parts of cases) { + const expected = parts.join("\0") + expect(zeroID(...parts)).toBe(expected) + expect(Buffer.from(zeroID(...parts))).toEqual(Buffer.from(expected)) + } + }) + + test("preserves namespace prefixes, suffixes, and nested keys", () => { + expect(zeroID("project", "")).toBe("project\0") + expect(zeroID("", "file.ts")).toBe("\0file.ts") + expect(zeroID("error", "message")).toBe("error\0message") + expect(zeroID(zeroID("project", "session"), "file.ts")).toBe("project\0session\0file.ts") + expect(zeroID("ab", "c")).not.toBe(zeroID("a", "bc")) + expect(zeroID("project", "session").startsWith(zeroID("project", ""))).toBe(true) + expect(zeroID("project-other", "session").startsWith(zeroID("project", ""))).toBe(false) + }) + + test("supports explicit caller-specific nullish coercion", () => { + for (const value of [null, undefined]) { + expect(zeroID("scope", String(value))).toBe(`scope\0${value}`) + expect(zeroID("scope", value ?? "")).toBe(["scope", value].join("\0")) + } + }) +}) diff --git a/packages/kilo-vscode/src/agent-manager/local-diff-cache.ts b/packages/kilo-vscode/src/agent-manager/local-diff-cache.ts index 04f72352525..601c119c865 100644 --- a/packages/kilo-vscode/src/agent-manager/local-diff-cache.ts +++ b/packages/kilo-vscode/src/agent-manager/local-diff-cache.ts @@ -1,3 +1,4 @@ +import { zeroID } from "@opencode-ai/core/kilocode/zero-id" import { imageMime } from "../diff/shared/image" import type { Batch, Meta } from "./local-diff-batch" import type { WorktreeDiffEntry } from "./types" @@ -58,7 +59,7 @@ export function createDiffCache(load: Loader) { } const identity = (dir: string, base: string, anc: string, meta: Meta) => - [ + zeroID( dir, base, anc, @@ -69,7 +70,7 @@ export function createDiffCache(load: Loader) { meta.deletions, meta.binary, meta.stamp, - ].join("\0") + ) const cached = (id: string) => { const value = details.get(id) @@ -236,8 +237,8 @@ export function createDiffCache(load: Loader) { } const queued = (id: string, dir: string, base: string, anc: string, meta: Meta, signal?: AbortSignal) => { - const scope = `${dir}\0${base}` - const key = `${scope}\0${anc}` + const scope = zeroID(dir, base) + const key = zeroID(scope, anc) let queue = queues.get(key) if (!queue) { queue = new Map() @@ -270,7 +271,7 @@ export function createDiffCache(load: Loader) { const file = (dir: string, base: string, path: string, signal?: AbortSignal): Promise => { if (signal?.aborted) return Promise.reject(new Error("Diff detail aborted")) - const state = states.get(`${dir}\0${base}`) + const state = states.get(zeroID(dir, base)) if (!state) return load.file(dir, base, path, signal) const meta = state.metas.get(path) if (!meta) return Promise.resolve(null) @@ -292,7 +293,7 @@ export function createDiffCache(load: Loader) { return { summary: async (dir: string, base: string): Promise => { - const id = `${dir}\0${base}` + const id = zeroID(dir, base) const generation = (generations.get(id) ?? 0) + 1 generations.set(id, generation) const result = await load.summary(dir, base) diff --git a/packages/kilo-vscode/src/agent-manager/project/route.ts b/packages/kilo-vscode/src/agent-manager/project/route.ts index 8504e65227f..90c93079fee 100644 --- a/packages/kilo-vscode/src/agent-manager/project/route.ts +++ b/packages/kilo-vscode/src/agent-manager/project/route.ts @@ -1,5 +1,7 @@ /** Strict project/resource ownership for Agent Manager multi-project routing. */ +import { zeroID as key } from "@opencode-ai/core/kilocode/zero-id" + export interface ProjectRef { projectId: string } @@ -45,8 +47,6 @@ interface SessionRoute { generation: number } -const key = (projectId: string, id: string) => `${projectId}\0${id}` - export class ProjectRouteService { private readonly projects = new Map() private readonly sessions = new Map() diff --git a/packages/kilo-vscode/src/services/cli-backend/explicit-abort.ts b/packages/kilo-vscode/src/services/cli-backend/explicit-abort.ts index 93987b0db32..23ae7e86a6a 100644 --- a/packages/kilo-vscode/src/services/cli-backend/explicit-abort.ts +++ b/packages/kilo-vscode/src/services/cli-backend/explicit-abort.ts @@ -1,4 +1,5 @@ import path from "node:path" +import { zeroID } from "@opencode-ai/core/kilocode/zero-id" import type { SSEPayload } from "./sdk-sse-adapter" type Buffered = { event: SSEPayload; directory?: string } @@ -75,12 +76,12 @@ export class ExplicitAbortState { const key = scope(sessionID, directory) return this.states.has(key) ? [key] : [] } - const prefix = `${sessionID}\0` + const prefix = zeroID(sessionID, "") return [...this.states.keys()].filter((key) => key.startsWith(prefix)) } } function scope(sessionID: string, directory: string) { const dir = path.resolve(directory) - return `${sessionID}\0${process.platform === "win32" ? dir.toLowerCase() : dir}` + return zeroID(sessionID, process.platform === "win32" ? dir.toLowerCase() : dir) } diff --git a/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts b/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts index c647fbcbda6..d9b2e2dd196 100644 --- a/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-manager-worktree-diffs.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "bun:test" import { createRoot } from "solid-js" -import { createWorktreeDiffs } from "../../webview-ui/agent-manager/worktree-diffs" +import { createWorktreeDiffs, diffDataKey } from "../../webview-ui/agent-manager/worktree-diffs" import type { WorktreeFileDiff } from "../../webview-ui/src/types/messages" const diff = (file: string, additions = 1): WorktreeFileDiff => ({ @@ -30,7 +30,37 @@ const withDiffs = (fn: (diffs: ReturnType, sent: Sen }) } +describe("diffDataKey", () => { + it("preserves the nullish fallback without replacing an empty project", () => { + expect(diffDataKey(undefined, "s1")).toBe("single\0s1") + expect(diffDataKey("single", "s1")).toBe("single\0s1") + expect(diffDataKey("", "s1")).toBe("\0s1") + expect(diffDataKey("project", "")).toBe("project\0") + expect(diffDataKey("project", "s1\0file.ts")).toBe("project\0s1\0file.ts") + }) +}) + describe("createWorktreeDiffs", () => { + it.each([undefined, "", "project"])("prunes only the complete project namespace %j", (project) => { + createRoot((dispose) => { + const store = createWorktreeDiffs(vscode([]), () => project) + const sibling = `${project ?? "single"}-other` + for (const owner of [project, sibling]) { + store.onWorktreeDiff({ + type: "agentManager.worktreeDiff", + projectId: owner, + sessionId: "gone#branch", + diffs: [diff("a.ts")], + }) + } + + store.prune(new Set()) + expect(store.diffDatas()[`${project ?? "single"}\0gone#branch`]).toBeUndefined() + expect(store.diffDatas()[`${sibling}\0gone#branch`]).toHaveLength(1) + dispose() + }) + }) + it("stores full diffs per session", () => { withDiffs((diffs) => { diffs.onWorktreeDiff({ type: "agentManager.worktreeDiff", sessionId: "s1", diffs: [diff("a.ts")] }) diff --git a/packages/kilo-vscode/tests/unit/agent-project-route.test.ts b/packages/kilo-vscode/tests/unit/agent-project-route.test.ts index 9ee54dc8fc9..e1487d094ff 100644 --- a/packages/kilo-vscode/tests/unit/agent-project-route.test.ts +++ b/packages/kilo-vscode/tests/unit/agent-project-route.test.ts @@ -50,6 +50,13 @@ describe("ProjectRouteService", () => { ) }) + it("preserves fallback, precedence, and empty fields in composite UI keys", () => { + expect(ProjectRouteService.key({ projectId: "a" })).toBe("a\0local") + expect(ProjectRouteService.key({ projectId: "a", sessionId: "s", worktreeId: "wt" })).toBe("a\0s") + expect(ProjectRouteService.key({ projectId: "a", sessionId: "", worktreeId: "wt" })).toBe("a\0") + expect(ProjectRouteService.key({ projectId: "", worktreeId: "" })).toBe("\0") + }) + describe("safe resolution (non-throwing)", () => { it("trySessionDirectory returns the exact dir for an unambiguous raw id", () => { const routes = new ProjectRouteService() diff --git a/packages/kilo-vscode/tests/unit/explicit-abort.test.ts b/packages/kilo-vscode/tests/unit/explicit-abort.test.ts index 1d2f8d06f91..dd3c93fbe59 100644 --- a/packages/kilo-vscode/tests/unit/explicit-abort.test.ts +++ b/packages/kilo-vscode/tests/unit/explicit-abort.test.ts @@ -78,6 +78,30 @@ describe("explicit abort state", () => { expect(state.event(close("interrupted"), "/repo/a")).toBe(false) }) + it.each(["", "session"])("removes only the exact session prefix %j across directories", (session) => { + const state = new ExplicitAbortState() + for (const directory of ["/repo/a", "/repo/b"]) { + const id = state.begin(session, directory) + state.finish(session, directory, id, true) + } + const sibling = `${session}-other` + const id = state.begin(sibling, "/repo/a") + state.finish(sibling, "/repo/a", id, true) + + state.remove(session) + expect(state.event(close("interrupted", session), "/repo/a")).toBe(true) + expect(state.event(close("interrupted", session), "/repo/b")).toBe(true) + expect(state.event(close("interrupted", sibling))).toBe(false) + }) + + it("normalizes directories before building scope keys", () => { + const state = new ExplicitAbortState() + const id = state.begin("session", "/repo/nested/..") + state.finish("session", "/repo", id, true) + + expect(state.event(close("interrupted"), "/repo/.")).toBe(false) + }) + it("clears suppression when an idle session becomes busy again", () => { const state = new ExplicitAbortState() const id = state.begin("session", "/repo") diff --git a/packages/kilo-vscode/tests/unit/local-diff.test.ts b/packages/kilo-vscode/tests/unit/local-diff.test.ts index dfaf6679d3b..352f2679d34 100644 --- a/packages/kilo-vscode/tests/unit/local-diff.test.ts +++ b/packages/kilo-vscode/tests/unit/local-diff.test.ts @@ -467,6 +467,26 @@ describe("diffFile", () => { }) }) + it("keeps empty and named base cache identities separate", async () => { + await withRepo(async (dir, base) => { + await fs.writeFile(path.join(dir, "seed.txt"), "seed\ncommitted\n") + runSync(dir, ["commit", "-am", "change seed"]) + await fs.writeFile(path.join(dir, "seed.txt"), "seed\ncommitted\nworking\n") + const local = createLocalDiff(git()) + await local.summary(dir, "") + await local.summary(dir, base) + + const current = await local.file(dir, "", "seed.txt") + const ancestor = await local.file(dir, base, "seed.txt") + expect(current?.before).toBe("seed\ncommitted\n") + expect(ancestor?.before).toBe("seed\n") + expect(current?.after).toBe("seed\ncommitted\nworking\n") + expect(ancestor?.after).toBe(current?.after) + expect(await local.file(dir, "", "seed.txt")).toBe(current) + expect(await local.file(dir, base, "seed.txt")).toBe(ancestor) + }) + }) + it("does not cache detail that is aborted before Git completes", async () => { await withRepo(async (dir, base) => { await fs.writeFile(path.join(dir, "seed.txt"), "seed\ncached\n") diff --git a/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts b/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts index 24cc476a44f..0e50fb9e934 100644 --- a/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts +++ b/packages/kilo-vscode/webview-ui/agent-manager/worktree-diffs.ts @@ -8,6 +8,7 @@ */ import { createSignal, type Accessor } from "solid-js" +import { zeroID } from "@opencode-ai/core/kilocode/zero-id" import { mergeWorktreeDiffs } from "../diff-viewer/diff-state" import { parseDiffId } from "./diff-scope-state" import type { useVSCode } from "../src/context/vscode" @@ -33,7 +34,7 @@ export function wireDiffId(id: string) { } export function diffDataKey(project: string | undefined, id: string): string { - return `${project ?? "single"}\0${id}` + return zeroID(project ?? "single", id) } export function createWorktreeDiffs( @@ -79,7 +80,7 @@ export function createWorktreeDiffs( } const prune = (ids: Set) => { - const prefix = `${project() ?? "single"}\0` + const prefix = key("") const keys = new Set([ ...Object.keys(diffDatas()), ...Object.keys(diffLoadings()), diff --git a/packages/opencode/src/kilocode/indexing-warning.ts b/packages/opencode/src/kilocode/indexing-warning.ts index 1223efb9cc9..e4dd2054d4f 100644 --- a/packages/opencode/src/kilocode/indexing-warning.ts +++ b/packages/opencode/src/kilocode/indexing-warning.ts @@ -1,4 +1,5 @@ import type { IndexingStatus } from "@kilocode/kilo-indexing/status" +import { zeroID } from "@opencode-ai/core/kilocode/zero-id" export const INDEXING_WARNING_CODES = ["qdrant.version-incompatible", "qdrant.version-unavailable"] as const @@ -21,7 +22,7 @@ export function parseQdrantWarning(value: unknown): IndexingWarning | undefined } export function indexingWarningKey(warning: IndexingWarning): string { - return `${warning.code}\u0000${warning.message}` + return zeroID(warning.code, warning.message) } export function indexingErrorMessage(status: IndexingStatus): string | undefined { diff --git a/packages/opencode/src/kilocode/indexing-worker-client.ts b/packages/opencode/src/kilocode/indexing-worker-client.ts index 0f055c5db13..1ba8f7b2210 100644 --- a/packages/opencode/src/kilocode/indexing-worker-client.ts +++ b/packages/opencode/src/kilocode/indexing-worker-client.ts @@ -4,6 +4,7 @@ import type { VectorStoreSearchResult, } from "@kilocode/kilo-indexing/engine" import type { IndexingStatus } from "@kilocode/kilo-indexing/status" +import { zeroID } from "@opencode-ai/core/kilocode/zero-id" import { withTimeout } from "@/util/timeout" import type { Event, Log, Message, Request, Result } from "./indexing-worker-protocol" import type { IndexingWarning } from "./indexing-warning" @@ -123,7 +124,7 @@ export namespace IndexingWorker { } const worker = (directory: string, root: string, hooks: Hooks): Host => { - const key = `${directory}\0${root}` + const key = zeroID(directory, root) const state = channel() let active = true let callbacks = hooks @@ -205,7 +206,7 @@ export namespace IndexingWorker { export function create(directory: string, root: string, hooks: Hooks) { if (factory) return factory(directory, root, hooks) - const key = `${directory}\0${root}` + const key = zeroID(directory, root) const existing = pool.get(key) if (existing) { existing.use(hooks) diff --git a/packages/opencode/test/kilocode/indexing-warning.test.ts b/packages/opencode/test/kilocode/indexing-warning.test.ts index ad97671aca1..4255dac9669 100644 --- a/packages/opencode/test/kilocode/indexing-warning.test.ts +++ b/packages/opencode/test/kilocode/indexing-warning.test.ts @@ -41,3 +41,11 @@ test("indexingWarningKey includes the warning code and message", () => { "qdrant.version-unavailable\u0000warning", ) }) + +test("indexingWarningKey preserves empty messages and embedded delimiters", () => { + for (const code of ["qdrant.version-unavailable", "qdrant.version-incompatible"] as const) { + for (const message of ["", "\0", "warning\0detail", "null", "undefined", "警告"]) { + expect(indexingWarningKey({ code, message })).toBe(`${code}\0${message}`) + } + } +}) diff --git a/packages/opencode/test/kilocode/indexing-worker.test.ts b/packages/opencode/test/kilocode/indexing-worker.test.ts index 12214b0a078..a8b409705c0 100644 --- a/packages/opencode/test/kilocode/indexing-worker.test.ts +++ b/packages/opencode/test/kilocode/indexing-worker.test.ts @@ -57,6 +57,38 @@ test("routes multiple directories through the shared indexing worker", async () expect(failures).toEqual([]) }) +test("pools workers by both directory and root", async () => { + await using tmp = await tmpdir() + await using root = await tmpdir() + const failures: unknown[] = [] + const hooks: IndexingWorker.Hooks = { + status() {}, + telemetry() {}, + warning() {}, + log() {}, + failure(err) { + failures.push(err) + }, + } + const first = IndexingWorker.create(tmp.path, tmp.path, hooks) + const again = IndexingWorker.create(tmp.path, tmp.path, hooks) + const second = IndexingWorker.create(tmp.path, root.path, hooks) + + try { + expect(again).toBe(first) + expect(second).not.toBe(first) + const statuses = await Promise.all([ + first.init({ enabled: false, embedderProvider: "openai" }), + second.init({ enabled: false, embedderProvider: "openai" }), + ]) + expect(statuses.map((status) => status.state)).toEqual(["Disabled", "Disabled"]) + } finally { + await Promise.all([first.dispose(), second.dispose()]) + } + + expect(failures).toEqual([]) +}) + test("waits for the primary index instead of scanning a worktree independently", async () => { await using tmp = await tmpdir() const main = path.join(tmp.path, "main")