From 00bec0c446fbc18be7be5fc9720a639d5d5e6c64 Mon Sep 17 00:00:00 2001 From: tiandao <2388127316@qq.com> Date: Sun, 4 Oct 2026 16:47:26 +0800 Subject: [PATCH 1/2] fix(sessions): settle native dedupe ties deterministically Byte-identical copies share the content-derived updatedAt, so the "keep the newest write" order in list() tied for exactly the manual-copy case the dedup was added for. The tie fell through to walkJsonl order, which follows directory enumeration and flips per platform: on NTFS a backup/ copy won while ext4 kept the original, so the copy took the session over, the original's id was dropped from records, and the new regression test failed on Windows. Break ties deterministically: an id with an open runtime keeps the file it is already writing, and the last tie is settled by path order, which every platform computes the same way. fixes #1383 --- .../src/native-pi-session.test.ts | 77 +++++++++++++++++++ .../agent-runtime/src/native-pi-session.ts | 25 +++++- 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/packages/agent-runtime/src/native-pi-session.test.ts b/packages/agent-runtime/src/native-pi-session.test.ts index 770b3c1dd1..13ef82c9c9 100644 --- a/packages/agent-runtime/src/native-pi-session.test.ts +++ b/packages/agent-runtime/src/native-pi-session.test.ts @@ -79,6 +79,83 @@ describe("NativePiSessionService", () => { expect(sessions[0].id).toBe(original.id); }); + it("breaks copy ties by path order instead of directory enumeration (#1383)", async () => { + const f = fixture(); + const control = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot }); + const [original] = await control.list(); + + // A copy that sorts before the original's group wins by path order. + // The guarantee under test is determinism: whichever file wins, every + // platform picks the same one, so the id cannot flip between scans. + const backupDir = join(f.sessionRoot, "--backup--"); + mkdirSync(backupDir, { recursive: true }); + writeFileSync(join(backupDir, "fixture-copy.jsonl"), f.text); + + const service = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot }); + const sessions = await service.list(); + expect(sessions).toHaveLength(1); + expect(sessions[0].id).not.toBe(original.id); + }); + + it("keeps the open runtime's file when a copy ties on recency (#1383)", async () => { + const f = fixture(); + const model = { + id: "test-model", + name: "Test Model", + api: "openai-completions", + provider: "test-provider", + baseUrl: "http://127.0.0.1/unused", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 16_000, + maxTokens: 1_000, + }; + const response: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: "fixture response" }], + api: "openai-completions", + provider: "test-provider", + model: "test-model", + stopReason: "stop", + timestamp: Date.now(), + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, totalTokens: 2, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, + }; + const fauxRuntime = { + getModel: () => model, + hasConfiguredAuth: () => true, + streamSimple: () => { + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + stream.push({ type: "start", partial: response }); + stream.push({ type: "done", reason: "stop", message: response }); + stream.end(response); + }); + return stream; + }, + }; + const service = new NativePiSessionService({ + agentDir: f.agentDir, + sessionRoot: f.sessionRoot, + modelRuntimeFactory: async () => fauxRuntime as any, + }); + const [summary] = await service.list(); + await service.prompt(summary.id, "desktop prompt", () => undefined); + await expect.poll(() => service.status(summary.id).status.isRunning).toBe(false); + + // The copy is byte-identical to the file the open runtime just wrote, + // so recency ties, and it sorts before the original's group. Only the + // runtime preference keeps the live session's file canonical. + const backupDir = join(f.sessionRoot, "--backup--"); + mkdirSync(backupDir, { recursive: true }); + writeFileSync(join(backupDir, "fixture-copy.jsonl"), readFileSync(f.file, "utf8")); + + const sessions = await service.list(); + expect(sessions).toHaveLength(1); + expect(sessions[0].id).toBe(summary.id); + service.disposeAll(); + }); + it("searches native metadata and active-branch message text without rewriting JSONL", async () => { const f = fixture(); const service = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot }); diff --git a/packages/agent-runtime/src/native-pi-session.ts b/packages/agent-runtime/src/native-pi-session.ts index f6b74d197c..0b24055878 100644 --- a/packages/agent-runtime/src/native-pi-session.ts +++ b/packages/agent-runtime/src/native-pi-session.ts @@ -552,11 +552,30 @@ export class NativePiSessionService { // path. Keep the newest write per native id and drop the stale copies // from the records map so lookups cannot route to a dead duplicate // (#1359). + // + // Byte-identical copies share the content-derived `updatedAt`, so the + // recency order ties for exactly the manual-copy case this dedup is + // for. A tie used to fall through to `walkJsonl` order, which follows + // directory enumeration and differs per platform: on NTFS the copy + // under `backup/` won while ext4 kept the original, so the copy could + // take the session over and strand the original's id (#1383). Order + // ties deterministically instead: an id with an open runtime keeps + // the file it is already writing, and the last tie is settled by + // path order, which every platform computes the same way. const seenNativeIds = new Set(); const deduped: Array = []; - const sortedByRecency = [...summaries].sort((a, b) => - (b?.updatedAt ?? "").localeCompare(a?.updatedAt ?? ""), - ); + const rankTie = (a: SessionSummary, b: SessionSummary): number => { + const aLive = this.runtimes.has(a.id) ? 1 : 0; + const bLive = this.runtimes.has(b.id) ? 1 : 0; + if (aLive !== bLive) return bLive - aLive; + const aPath = this.records.get(a.id)?.path ?? a.id; + const bPath = this.records.get(b.id)?.path ?? b.id; + return aPath < bPath ? -1 : aPath > bPath ? 1 : 0; + }; + const sortedByRecency = [...summaries].sort((a, b) => { + const byRecency = (b?.updatedAt ?? "").localeCompare(a?.updatedAt ?? ""); + return byRecency !== 0 || !a || !b ? byRecency : rankTie(a, b); + }); const winnerIds = new Set(); for (const summary of sortedByRecency) { if (!summary) continue; From 16950eef168e2ad7d8c132ad3aab54db56f13d4f Mon Sep 17 00:00:00 2001 From: vastsa Date: Sun, 4 Oct 2026 17:16:10 +0800 Subject: [PATCH 2/2] test(sessions): pin the canonical file in the copy dedupe The #1359 regression test only scanned its "control" after the copy existed, so it compared the winner with itself: it passed on every platform even while the byte-identical copy won the dedup on NTFS and APFS. Scan the control before writing the copy so the assertion pins which file stays canonical for the native id, and let it fail if the winner ever follows enumeration order again. Also lift the inline faux model runtime that two tests duplicated into one `fauxModelRuntime()` helper and document the rankTie comparator, including the id fallback that only keeps it total. Verified locally: the strengthened assertion fails on main (the copy wins on APFS) and passes with the deterministic tie-break; the package suite is 1240/1240 and `tsc --noEmit` is clean. --- .../src/native-pi-session.test.ts | 128 +++++++----------- .../agent-runtime/src/native-pi-session.ts | 5 + 2 files changed, 57 insertions(+), 76 deletions(-) diff --git a/packages/agent-runtime/src/native-pi-session.test.ts b/packages/agent-runtime/src/native-pi-session.test.ts index 13ef82c9c9..6bbed1ea95 100644 --- a/packages/agent-runtime/src/native-pi-session.test.ts +++ b/packages/agent-runtime/src/native-pi-session.test.ts @@ -63,16 +63,20 @@ describe("NativePiSessionService", () => { it("collapses copied session files onto one entry per native id (#1359)", async () => { const f = fixture(); + + // Control scan without the copy: the group file is the canonical session. + // A byte-identical copy ties on the content-derived recency, so this is + // the assertion that fails when the winner follows enumeration order + // (#1383). + const control = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot }); + const [original] = await control.list(); + // A backup copy of the session under a subdirectory of the scan root: // same header id, different path. It must not become a second session. const backupDir = join(f.sessionRoot, "backup"); mkdirSync(backupDir, { recursive: true }); writeFileSync(join(backupDir, "fixture-copy.jsonl"), f.text); - // Control: without the copy, list() names the original file's id. - const control = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot }); - const [original] = await control.list(); - const service = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot }); const sessions = await service.list(); expect(sessions).toHaveLength(1); @@ -99,45 +103,10 @@ describe("NativePiSessionService", () => { it("keeps the open runtime's file when a copy ties on recency (#1383)", async () => { const f = fixture(); - const model = { - id: "test-model", - name: "Test Model", - api: "openai-completions", - provider: "test-provider", - baseUrl: "http://127.0.0.1/unused", - reasoning: false, - input: ["text"], - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, - contextWindow: 16_000, - maxTokens: 1_000, - }; - const response: AssistantMessage = { - role: "assistant", - content: [{ type: "text", text: "fixture response" }], - api: "openai-completions", - provider: "test-provider", - model: "test-model", - stopReason: "stop", - timestamp: Date.now(), - usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, totalTokens: 2, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, - }; - const fauxRuntime = { - getModel: () => model, - hasConfiguredAuth: () => true, - streamSimple: () => { - const stream = createAssistantMessageEventStream(); - queueMicrotask(() => { - stream.push({ type: "start", partial: response }); - stream.push({ type: "done", reason: "stop", message: response }); - stream.end(response); - }); - return stream; - }, - }; const service = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot, - modelRuntimeFactory: async () => fauxRuntime as any, + modelRuntimeFactory: async () => fauxModelRuntime() as any, }); const [summary] = await service.list(); await service.prompt(summary.id, "desktop prompt", () => undefined); @@ -237,45 +206,10 @@ describe("NativePiSessionService", () => { it("continues through AgentSession and appends the faux-model turn to the original file", async () => { const f = fixture(); - const model = { - id: "test-model", - name: "Test Model", - api: "openai-completions", - provider: "test-provider", - baseUrl: "http://127.0.0.1/unused", - reasoning: false, - input: ["text"], - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, - contextWindow: 16_000, - maxTokens: 1_000, - }; - const response: AssistantMessage = { - role: "assistant", - content: [{ type: "text", text: "fixture response" }], - api: "openai-completions", - provider: "test-provider", - model: "test-model", - stopReason: "stop", - timestamp: Date.now(), - usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, totalTokens: 2, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, - }; - const fauxRuntime = { - getModel: () => model, - hasConfiguredAuth: () => true, - streamSimple: () => { - const stream = createAssistantMessageEventStream(); - queueMicrotask(() => { - stream.push({ type: "start", partial: response }); - stream.push({ type: "done", reason: "stop", message: response }); - stream.end(response); - }); - return stream; - }, - }; const service = new NativePiSessionService({ agentDir: f.agentDir, sessionRoot: f.sessionRoot, - modelRuntimeFactory: async () => fauxRuntime as any, + modelRuntimeFactory: async () => fauxModelRuntime() as any, }); const [summary] = await service.list(); const before = readFileSync(f.file, "utf8"); @@ -346,6 +280,48 @@ function fauxStream(message = response()) { return stream; } +/** + * A minimal offline model runtime for the native continuation tests: one + * catalog entry, auth always present, one canned assistant turn per call. + */ +function fauxModelRuntime() { + const model = { + id: "test-model", + name: "Test Model", + api: "openai-completions", + provider: "test-provider", + baseUrl: "http://127.0.0.1/unused", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 16_000, + maxTokens: 1_000, + }; + const message: AssistantMessage = { + role: "assistant", + content: [{ type: "text", text: "fixture response" }], + api: "openai-completions", + provider: "test-provider", + model: "test-model", + stopReason: "stop", + timestamp: Date.now(), + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, totalTokens: 2, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, + }; + return { + getModel: () => model, + hasConfiguredAuth: () => true, + streamSimple: () => { + const stream = createAssistantMessageEventStream(); + queueMicrotask(() => { + stream.push({ type: "start", partial: message }); + stream.push({ type: "done", reason: "stop", message }); + stream.end(message); + }); + return stream; + }, + }; +} + describe("native continuation review regressions", () => { it("uses the real offline credential/catalog and reports missing exact auth/model", async () => { const f = await configuredFixture(); diff --git a/packages/agent-runtime/src/native-pi-session.ts b/packages/agent-runtime/src/native-pi-session.ts index 0b24055878..f0c10807c3 100644 --- a/packages/agent-runtime/src/native-pi-session.ts +++ b/packages/agent-runtime/src/native-pi-session.ts @@ -564,6 +564,11 @@ export class NativePiSessionService { // path order, which every platform computes the same way. const seenNativeIds = new Set(); const deduped: Array = []; + // Tie-break inside a recency group: the file an open runtime is already + // writing wins, then the smallest record path. Both keys are total orders + // over the group, so the winner cannot follow enumeration order. The id + // fallback only keeps the comparator total for a summary without a record + // (every summary above writes one, so it is not reachable in practice). const rankTie = (a: SessionSummary, b: SessionSummary): number => { const aLive = this.runtimes.has(a.id) ? 1 : 0; const bLive = this.runtimes.has(b.id) ? 1 : 0;