From 6ea01acdfbf8ca27464f10a51ea8b856961b594b Mon Sep 17 00:00:00 2001 From: Amp Date: Fri, 21 Aug 2026 15:27:13 +0000 Subject: [PATCH] fix(doctor): atomically repair session transcripts --- .../doctor-session-transcripts.test.ts | 109 ++++++++++++++++++ src/commands/doctor-session-transcripts.ts | 107 ++++++++++------- 2 files changed, 174 insertions(+), 42 deletions(-) diff --git a/src/commands/doctor-session-transcripts.test.ts b/src/commands/doctor-session-transcripts.test.ts index 78dd97876856..a46bba4fd084 100644 --- a/src/commands/doctor-session-transcripts.test.ts +++ b/src/commands/doctor-session-transcripts.test.ts @@ -14,11 +14,31 @@ const repairCanonicalSessionKeys = vi.hoisted(() => vi.fn()); const migrateLegacyMainSessionKeys = vi.hoisted(() => vi.fn()); const runDoctorSessionSqlite = vi.hoisted(() => vi.fn()); const withDoctorSqliteMaintenanceLock = vi.hoisted(() => vi.fn()); +const atomicWriteControl = vi.hoisted(() => ({ + beforeRename: undefined as undefined | ((filePath: string) => Promise), +})); vi.mock("../../packages/terminal-core/src/note.js", () => ({ note, })); +vi.mock("../infra/json-files.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + writeTextAtomic: async (...args: Parameters) => { + const [filePath, content, options] = args; + return await actual.writeTextAtomic(filePath, content, { + ...options, + beforeRename: async (params) => { + await options?.beforeRename?.(params); + await atomicWriteControl.beforeRename?.(filePath); + }, + }); + }, + }; +}); + vi.mock("./doctor-session-sqlite.js", () => ({ runDoctorSessionSqlite, })); @@ -115,6 +135,7 @@ describe("doctor session transcript repair", () => { let root: string; beforeEach(async () => { + atomicWriteControl.beforeRename = undefined; note.mockClear(); repairReservedIncognitoSessionKeys.mockReset().mockReturnValue({ found: 0, repaired: 0 }); repairCanonicalSessionDeliveryStates @@ -224,6 +245,94 @@ describe("doctor session transcript repair", () => { ).toEqual(["parent", "plain-user", "plain-assistant"]); }); + it.each([ + { + name: "prompt-rewrite branch", + entries: [ + { type: "session", version: 3, id: "session-1", timestamp: "2026-04-25T00:00:00Z" }, + { + type: "message", + id: "parent", + parentId: null, + message: { role: "assistant", content: "previous" }, + }, + { + type: "message", + id: "runtime-user", + parentId: "parent", + message: { + role: "user", + content: + "visible ask\n\n<<>>\nsecret\n<<>>", + }, + }, + { + type: "message", + id: "runtime-assistant", + parentId: "runtime-user", + message: { role: "assistant", content: "stale" }, + }, + { + type: "message", + id: "plain-user", + parentId: "parent", + message: { role: "user", content: "visible ask" }, + }, + { + type: "message", + id: "plain-assistant", + parentId: "plain-user", + message: { role: "assistant", content: "answer" }, + }, + ], + }, + { + name: "legacy OpenAI Codex metadata", + entries: [ + { type: "session", version: 3, id: "session-1", timestamp: "2026-04-25T00:00:00Z" }, + { + type: "message", + id: "legacy-assistant", + parentId: null, + message: { + role: "assistant", + provider: "openai-codex", + api: "openai-codex-responses", + content: [{ type: "text", text: "hello" }], + }, + }, + ], + }, + ])("preserves and reports $name when atomic publication fails", async ({ entries }) => { + const filePath = await writeTranscript(entries); + const original = await fs.readFile(filePath, "utf-8"); + atomicWriteControl.beforeRename = async () => { + throw new Error("injected transcript rename failure"); + }; + + await noteSessionTranscriptHealth({ + sessionDirs: [path.dirname(filePath)], + shouldRepair: true, + }); + + expect(await fs.readFile(filePath, "utf-8")).toBe(original); + const backupName = (await fs.readdir(path.dirname(filePath))).find( + (entry) => entry.startsWith("session.jsonl.pre-doctor-") && entry.endsWith(".bak"), + ); + expect(backupName).toEqual(expect.any(String)); + if (!backupName) { + throw new Error("expected transcript repair backup"); + } + expect(await fs.readFile(path.join(path.dirname(filePath), backupName), "utf-8")).toBe( + original, + ); + const transcriptNote = note.mock.calls.find((call) => call[1] === "Session transcripts")?.[0]; + expect(transcriptNote).toContain("needs repair"); + expect(transcriptNote).toContain("backup="); + expect(transcriptNote).toContain("injected transcript rename failure"); + expect(transcriptNote).not.toContain("Repaired 1 transcript file"); + }); + it("reports affected transcripts without rewriting outside repair mode", async () => { const filePath = await writeTranscript([ { type: "session", version: 3, id: "session-1", timestamp: "2026-04-25T00:00:00Z" }, diff --git a/src/commands/doctor-session-transcripts.ts b/src/commands/doctor-session-transcripts.ts index ae3f2cb5f4c3..a766a2de1ebf 100644 --- a/src/commands/doctor-session-transcripts.ts +++ b/src/commands/doctor-session-transcripts.ts @@ -19,6 +19,7 @@ import { } from "../config/sessions/transcript-tree.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; import type { HealthFinding, HealthRepairEffect } from "../flows/health-checks.js"; +import { writeTextAtomic } from "../infra/json-files.js"; import { shortenHomePath } from "../utils.js"; import { repairCanonicalSessionKeys, @@ -255,15 +256,11 @@ async function writeActiveTranscript(params: { filePath: string; entries: TranscriptEntry[]; activePath: ActiveTranscriptPath; -}): Promise { +}): Promise { const header = params.entries.find((entry) => entry.type === "session"); if (!header) { throw new Error("missing session header"); } - const backupPath = `${params.filePath}.pre-doctor-branch-repair-${new Date() - .toISOString() - .replace(/[:.]/g, "-")}.bak`; - await fs.copyFile(params.filePath, backupPath); const lastPersistedId = getEntryId(params.activePath.entriesToPersist.at(-1) ?? {}); const terminalLeafControl = params.activePath.terminalLeafControl ? { @@ -279,21 +276,45 @@ async function writeActiveTranscript(params: { ] .map((entry) => JSON.stringify(entry)) .join("\n"); - await fs.writeFile(params.filePath, `${next}\n`, "utf-8"); - return backupPath; + await writeTextAtomic(params.filePath, next, { + tempPrefix: `${path.basename(params.filePath)}.doctor-repair`, + trailingNewline: true, + }); } async function writeTranscriptEntries(params: { filePath: string; entries: TranscriptEntry[]; -}): Promise { - const backupPath = `${params.filePath}.pre-doctor-openai-codex-repair-${new Date() - .toISOString() - .replace(/[:.]/g, "-")}.bak`; - await fs.copyFile(params.filePath, backupPath); +}): Promise { const next = params.entries.map((entry) => JSON.stringify(entry)).join("\n"); - await fs.writeFile(params.filePath, `${next}\n`, "utf-8"); - return backupPath; + await writeTextAtomic(params.filePath, next, { + tempPrefix: `${path.basename(params.filePath)}.doctor-repair`, + trailingNewline: true, + }); +} + +async function publishTranscriptRepair(params: { + result: SessionTranscriptHealthIssue; + backupLabel: "branch-repair" | "openai-codex-repair"; + write: () => Promise; +}): Promise { + let backupPath: string | undefined; + try { + const nextBackupPath = `${params.result.filePath}.pre-doctor-${params.backupLabel}-${new Date() + .toISOString() + .replace(/[:.]/g, "-")}.bak`; + await fs.copyFile(params.result.filePath, nextBackupPath); + backupPath = nextBackupPath; + await params.write(); + return { ...params.result, repaired: true, backupPath }; + } catch (err) { + return { + ...params.result, + repaired: false, + ...(backupPath ? { backupPath } : {}), + reason: String(err), + }; + } } /** Repairs one transcript file by keeping the active branch and backing up the original file. */ @@ -308,17 +329,19 @@ async function repairBrokenSessionTranscriptFile(params: { const activePath = selectActivePath(entries); if (!activePath) { if (legacyOpenAICodexEntries > 0 && params.shouldRepair) { - const backupPath = await writeTranscriptEntries({ filePath: params.filePath, entries }); - return { - filePath: params.filePath, - broken: true, - repaired: true, - originalEntries: entries.length, - activeEntries: 0, - legacyOpenAICodexEntries, - backupPath, - reason: "no active branch", - }; + return await publishTranscriptRepair({ + result: { + filePath: params.filePath, + broken: true, + repaired: false, + originalEntries: entries.length, + activeEntries: 0, + legacyOpenAICodexEntries, + reason: "no active branch", + }, + backupLabel: "openai-codex-repair", + write: () => writeTranscriptEntries({ filePath: params.filePath, entries }), + }); } return { filePath: params.filePath, @@ -351,22 +374,21 @@ async function repairBrokenSessionTranscriptFile(params: { legacyOpenAICodexEntries, }; } - const backupPath = broken - ? await writeActiveTranscript({ - filePath: params.filePath, - entries, - activePath, - }) - : await writeTranscriptEntries({ filePath: params.filePath, entries }); - return { - filePath: params.filePath, - broken: true, - repaired: true, - originalEntries: entries.length, - activeEntries: activePath.entries.length, - legacyOpenAICodexEntries, - backupPath, - }; + return await publishTranscriptRepair({ + result: { + filePath: params.filePath, + broken: true, + repaired: false, + originalEntries: entries.length, + activeEntries: activePath.entries.length, + legacyOpenAICodexEntries, + }, + backupLabel: broken ? "branch-repair" : "openai-codex-repair", + write: () => + broken + ? writeActiveTranscript({ filePath: params.filePath, entries, activePath }) + : writeTranscriptEntries({ filePath: params.filePath, entries }), + }); } catch (err) { return { filePath: params.filePath, @@ -483,11 +505,12 @@ export async function noteSessionTranscriptHealth(params?: { ...broken.slice(0, 20).map((result) => { const backup = result.backupPath ? ` backup=${shortenHomePath(result.backupPath)}` : ""; const status = result.repaired ? "repaired" : "needs repair"; + const error = !result.repaired && result.reason ? ` error=${result.reason}` : ""; const metadata = result.legacyOpenAICodexEntries > 0 ? ` openai-codex=${result.legacyOpenAICodexEntries}` : ""; - return `- ${shortenHomePath(result.filePath)} ${status} entries=${result.originalEntries}->${result.activeEntries + 1}${metadata}${backup}`; + return `- ${shortenHomePath(result.filePath)} ${status} entries=${result.originalEntries}->${result.activeEntries + 1}${metadata}${backup}${error}`; }), ]; if (broken.length > 20) {