From 2509356e0832c4dda0c1c04205d01850fa59f568 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 17:04:25 -0700 Subject: [PATCH] fix(session-memory): restore pre-existing file on late-block retraction instead of deleting When a later hook sets blockSessionSave=true, the retraction previously unconditionally deleted memoryFilePath. If the filename collided with a pre-existing memory file (e.g. same LLM slug on the same day), the prior session's content was erased. Now snapshots any pre-existing file content before the inline write and restores it on retraction instead of deleting. Non-colliding writes still get deleted as before. --- .../bundled/session-memory/handler.test.ts | 55 +++++++++++++++++++ src/hooks/bundled/session-memory/handler.ts | 26 ++++++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 4c429b12596..10c1ab98a62 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -643,6 +643,61 @@ describe("session-memory hook", () => { expect(memoryFiles).toHaveLength(0); }); + it("late-block retraction restores pre-existing file instead of deleting", async () => { + const tempDir = await createCaseWorkspace("block-save-restore"); + const sessionsDir = path.join(tempDir, "sessions"); + await fs.mkdir(sessionsDir, { recursive: true }); + const sessionFile = await writeWorkspaceFile({ + dir: sessionsDir, + name: "test-session.jsonl", + content: createMockSessionContent([{ role: "user", content: "new session" }]), + }); + + // Run the handler to create the memory file (captures the generated filename). + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + await handler(event); + await drainPostHookActions(event); + + const memoryDir = path.join(tempDir, "memory"); + const files1 = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(files1).toHaveLength(1); + const existingFile = files1[0]; + const existingPath = path.join(memoryDir, existingFile); + + // Replace the file content with known "prior" content to simulate a + // pre-existing memory file that would be at risk during slug collision. + await fs.writeFile(existingPath, "prior session content"); + + // Now run a second handler that writes to the SAME filename. + // We simulate this by using the same session (same slug output). + const event2 = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s2", sessionFile }, + }); + await handler(event2); + + // Verify inline write happened (file content changed from "prior session content"). + // Note: if slugs differ, this test just validates the non-collision retraction path. + // The fix ensures correctness in both cases — collision restores, no-collision deletes. + + // Late-block: a later hook sets blockSessionSave + event2.context.blockSessionSave = true; + await drainPostHookActions(event2); + + // The SECOND handler's file should be retracted. + // If it was the same filename (collision), the prior content should be restored. + // If different filename, that file should be deleted and the original file untouched. + const files2 = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + + // The original file must survive regardless of collision + expect(files2).toContain(existingFile); + const content = await fs.readFile(existingPath, "utf-8"); + expect(content).toBe("prior session content"); + }); + it("sessionSaveContent (pre-set) overrides saved content", async () => { const tempDir = await createCaseWorkspace("custom-content"); const sessionsDir = path.join(tempDir, "sessions"); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 4de19361545..cb1d811e051 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -382,10 +382,20 @@ const saveSessionToMemory: HookHandler = async (event) => { // Write inline (fail-safe: if postHookActions never drains, the file // is preserved on disk with the best content available at this point). // If blockSessionSave was already set by an upstream hook, skip the write. + // + // Before writing, snapshot any pre-existing file content so that late-block + // retraction can restore it instead of deleting — preventing accidental + // erasure of prior memory files when LLM slugs collide on the same day. + let preExistingContent: string | null = null; if (context.blockSessionSave === true) { log.debug("Session save blocked by upstream hook (inline check)"); } else { await fs.mkdir(memoryDir, { recursive: true }); + try { + preExistingContent = await fs.readFile(memoryFilePath, "utf-8"); + } catch { + // File doesn't exist yet — normal case, nothing to preserve. + } await writeFileWithinRoot({ rootDir: memoryDir, relativePath: filename, @@ -407,10 +417,22 @@ const saveSessionToMemory: HookHandler = async (event) => { // and per-action isolation. event.postHookActions.push(async () => { // If a later hook blocked the save, retract the file we just wrote. + // If the file existed before our write (slug collision), restore the + // original content instead of deleting — avoids erasing prior history. if (event.context.blockSessionSave === true && writtenEntry !== null) { try { - await fs.unlink(memoryFilePath); - log.debug("Session save retracted by post-hook (blockSessionSave)"); + if (preExistingContent !== null) { + await writeFileWithinRoot({ + rootDir: memoryDir, + relativePath: filename, + data: preExistingContent, + encoding: "utf-8", + }); + log.debug("Session save retracted by post-hook — pre-existing file restored"); + } else { + await fs.unlink(memoryFilePath); + log.debug("Session save retracted by post-hook (blockSessionSave)"); + } } catch (err) { // File may not exist if inline write also didn't happen — that's fine. if ((err as NodeJS.ErrnoException).code !== "ENOENT") {