From 65fae19fc8389764a072183abb55d65d04558eb0 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Fri, 6 Mar 2026 09:43:52 -0700 Subject: [PATCH 01/45] feat(hooks): add blockSessionSave and sessionSaveContent to session memory handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new context fields for upstream hooks (e.g. security plugins) to control session memory persistence: - blockSessionSave (boolean): prevent session from being saved to memory - sessionSaveContent (string): override saved content with custom text (empty string is valid — persists a blank marker without transcript) When sessionSaveContent is set, LLM slug generation and session content loading are skipped (unnecessary when content is overridden). Split from #35567 — sessionSaveRedirectPath follows separately as it requires path canonicalization, symlink resolution, and filesystem write policy review. --- .../bundled/session-memory/handler.test.ts | 77 +++++++++++++++++++ src/hooks/bundled/session-memory/handler.ts | 16 +++- 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index fb7e9ca0a4d..b5b5ef3ce13 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -572,4 +572,81 @@ describe("session-memory hook", () => { expect(memoryContent).toContain("user: Only message 1"); expect(memoryContent).toContain("assistant: Only message 2"); }); + + it("blockSessionSave prevents memory file creation", async () => { + const tempDir = await createCaseWorkspace("block-save"); + 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: "secret" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + event.context.blockSessionSave = true; + + await handler(event); + + const memoryDir = path.join(tempDir, "memory"); + const memoryFiles = await fs.readdir(memoryDir).catch(() => [] as string[]); + expect(memoryFiles.filter((f) => f.endsWith(".md"))).toHaveLength(0); + }); + + it("sessionSaveContent overrides saved content", async () => { + const tempDir = await createCaseWorkspace("custom-content"); + 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: "original" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + event.context.sessionSaveContent = "Custom summary from upstream hook"; + + await handler(event); + + const memoryDir = path.join(tempDir, "memory"); + const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(files.length).toBeGreaterThan(0); + const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); + expect(content).toContain("Custom summary from upstream hook"); + expect(content).not.toContain("original"); + }); + + it("sessionSaveContent empty string writes blank marker file", async () => { + const tempDir = await createCaseWorkspace("empty-content"); + 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: "sensitive data" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + // Empty string is a valid override — persists a blank marker without + // loading the transcript or generating an LLM slug. + event.context.sessionSaveContent = ""; + + await handler(event); + + const memoryDir = path.join(tempDir, "memory"); + const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(files.length).toBeGreaterThan(0); + const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); + // Should be truly empty — blank marker file + expect(content).toBe(""); + }); }); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 32fc36b23f0..c6bfa6d0a9a 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -207,6 +207,13 @@ const saveSessionToMemory: HookHandler = async (event) => { log.debug("Hook triggered for reset/new command", { action: event.action }); const context = event.context || {}; + + // Check if another hook (e.g., security plugin) blocked the save. + if (context.blockSessionSave === true) { + log.debug("Session save blocked by upstream hook"); + return; + } + const cfg = context.cfg as OpenClawConfig | undefined; const contextWorkspaceDir = typeof context.workspaceDir === "string" && context.workspaceDir.trim().length > 0 @@ -279,8 +286,9 @@ const saveSessionToMemory: HookHandler = async (event) => { let slug: string | null = null; let sessionContent: string | null = null; + const hasCustomContent = typeof context.sessionSaveContent === "string"; - if (sessionFile) { + if (sessionFile && !hasCustomContent) { // Get recent conversation content, with fallback to rotated reset transcript. sessionContent = await getRecentSessionContentWithResetFallback(sessionFile, messageCount); log.debug("Session content loaded", { @@ -341,7 +349,11 @@ const saveSessionToMemory: HookHandler = async (event) => { entryParts.push("## Conversation Summary", "", sessionContent, ""); } - const entry = entryParts.join("\n"); + // Use custom content from upstream hook if available, otherwise use built entry. + // An empty string is a valid redaction signal — hooks may intentionally + // set it to persist a blank marker while avoiding transcript retention. + const customContent = context.sessionSaveContent; + const entry = typeof customContent === "string" ? customContent : entryParts.join("\n"); // Write under memory root with alias-safe file validation. await writeFileWithinRoot({ From 7a81cc94d3c9d73fc261e91980dd57b49b609c41 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Fri, 6 Mar 2026 15:58:37 -0700 Subject: [PATCH 02/45] fix(session-memory): add debug log for custom content, tighten test assertion - Add log.debug when sessionSaveContent override is used (symmetric with blockSessionSave) - Change override test assertion from toContain to toBe for precision --- src/hooks/bundled/session-memory/handler.test.ts | 2 +- src/hooks/bundled/session-memory/handler.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index b5b5ef3ce13..c2a7e07b79c 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -618,7 +618,7 @@ describe("session-memory hook", () => { const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); expect(files.length).toBeGreaterThan(0); const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); - expect(content).toContain("Custom summary from upstream hook"); + expect(content).toBe("Custom summary from upstream hook"); expect(content).not.toContain("original"); }); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index c6bfa6d0a9a..94cbf9791d7 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -353,6 +353,11 @@ const saveSessionToMemory: HookHandler = async (event) => { // An empty string is a valid redaction signal — hooks may intentionally // set it to persist a blank marker while avoiding transcript retention. const customContent = context.sessionSaveContent; + if (typeof customContent === "string") { + log.debug("Using custom session content from upstream hook", { + length: customContent.length, + }); + } const entry = typeof customContent === "string" ? customContent : entryParts.join("\n"); // Write under memory root with alias-safe file validation. From 9400e537f0b750686864020e6a1b006511d56262 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Fri, 6 Mar 2026 18:10:51 -0700 Subject: [PATCH 03/45] fix(session-memory): consolidate hasCustomContent / typeof redundancy Use the hasCustomContent boolean (already computed for gating session loading) instead of re-checking typeof at the write site. --- src/hooks/bundled/session-memory/handler.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 94cbf9791d7..4993d95381d 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -352,13 +352,17 @@ const saveSessionToMemory: HookHandler = async (event) => { // Use custom content from upstream hook if available, otherwise use built entry. // An empty string is a valid redaction signal — hooks may intentionally // set it to persist a blank marker while avoiding transcript retention. - const customContent = context.sessionSaveContent; - if (typeof customContent === "string") { + // Use custom content from upstream hook if available, otherwise use built entry. + // hasCustomContent (set above) already gates session loading + slug generation. + let entry: string; + if (hasCustomContent) { + entry = context.sessionSaveContent as string; log.debug("Using custom session content from upstream hook", { - length: customContent.length, + length: entry.length, }); + } else { + entry = entryParts.join("\n"); } - const entry = typeof customContent === "string" ? customContent : entryParts.join("\n"); // Write under memory root with alias-safe file validation. await writeFileWithinRoot({ From 1010fb90028340e3235318e79ffea87d0d668148 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Fri, 6 Mar 2026 18:36:27 -0700 Subject: [PATCH 04/45] fix(session-memory): remove duplicate comment, skip entryParts when custom content, clarify hook ordering - Remove copy-paste duplicate 'Use custom content...' comment - Move entryParts construction inside else branch (skip when hasCustomContent) - Add comment explaining FIFO hook ordering for blockSessionSave consumers --- src/hooks/bundled/session-memory/handler.ts | 39 +++++++++++---------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 4993d95381d..ee6eacbf17f 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -209,6 +209,10 @@ const saveSessionToMemory: HookHandler = async (event) => { const context = event.context || {}; // Check if another hook (e.g., security plugin) blocked the save. + // Internal hooks execute sequentially in FIFO registration order. + // Managed/workspace hooks that set blockSessionSave or sessionSaveContent + // must register for the same event (command:new / command:reset) and will + // run before this bundled handler as long as they are loaded first. if (context.blockSessionSave === true) { log.debug("Session save blocked by upstream hook"); return; @@ -334,33 +338,30 @@ const saveSessionToMemory: HookHandler = async (event) => { const sessionId = (sessionEntry.sessionId as string) || "unknown"; const source = (context.commandSource as string) || "unknown"; - // Build Markdown entry - const entryParts = [ - `# Session: ${dateStr} ${timeStr} UTC`, - "", - `- **Session Key**: ${displaySessionKey}`, - `- **Session ID**: ${sessionId}`, - `- **Source**: ${source}`, - "", - ]; - - // Include conversation content if available - if (sessionContent) { - entryParts.push("## Conversation Summary", "", sessionContent, ""); - } - - // Use custom content from upstream hook if available, otherwise use built entry. - // An empty string is a valid redaction signal — hooks may intentionally - // set it to persist a blank marker while avoiding transcript retention. - // Use custom content from upstream hook if available, otherwise use built entry. + // Use custom content from upstream hook if available, otherwise build entry. // hasCustomContent (set above) already gates session loading + slug generation. let entry: string; if (hasCustomContent) { + // An empty string is a valid redaction signal — hooks may intentionally + // set it to persist a blank marker while avoiding transcript retention. entry = context.sessionSaveContent as string; log.debug("Using custom session content from upstream hook", { length: entry.length, }); } else { + const entryParts = [ + `# Session: ${dateStr} ${timeStr} UTC`, + "", + `- **Session Key**: ${event.sessionKey}`, + `- **Session ID**: ${sessionId}`, + `- **Source**: ${source}`, + "", + ]; + + if (sessionContent) { + entryParts.push("## Conversation Summary", "", sessionContent, ""); + } + entry = entryParts.join("\n"); } From 423dce7ccd55cf97d89f99bc6770ad8c99ec5607 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Fri, 6 Mar 2026 18:47:20 -0700 Subject: [PATCH 05/45] docs(session-memory): clarify hook ordering limitation for blockSessionSave Bundled hooks register before managed/workspace hooks in FIFO order, so blockSessionSave only works when set by typed plugin hooks (which fire earlier in the lifecycle) or extraDirs hooks. Document this limitation honestly rather than claiming incorrect ordering. --- src/hooks/bundled/session-memory/handler.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index ee6eacbf17f..39bc547f5e1 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -209,10 +209,13 @@ const saveSessionToMemory: HookHandler = async (event) => { const context = event.context || {}; // Check if another hook (e.g., security plugin) blocked the save. - // Internal hooks execute sequentially in FIFO registration order. - // Managed/workspace hooks that set blockSessionSave or sessionSaveContent - // must register for the same event (command:new / command:reset) and will - // run before this bundled handler as long as they are loaded first. + // NOTE: Internal hooks execute sequentially in FIFO registration order. + // Bundled hooks register before managed/workspace hooks (see workspace.ts + // loadHookEntries merge order), so this check only works when the policy + // decision is set by a typed plugin hook (registerTypedHook) that fires + // earlier in the agent lifecycle, or by a hook loaded via extraDirs that + // registers before bundled hooks. Managed/workspace internal hooks that + // set blockSessionSave on the same event will run AFTER this handler. if (context.blockSessionSave === true) { log.debug("Session save blocked by upstream hook"); return; From 6b998ff7466b5847dd2688aa80e07da1ebd147ab Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Fri, 6 Mar 2026 19:15:50 -0700 Subject: [PATCH 06/45] fix(session-memory): use HHMMSS slug to prevent same-minute overwrites Custom content and no-session paths fell through to HHMM (4-char) timestamp slug. Two /new events in the same minute would overwrite. HHMMSS makes collisions require same-second timing. --- src/hooks/bundled/session-memory/handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 39bc547f5e1..660934416fa 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -322,7 +322,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // If no slug, use timestamp if (!slug) { const timeSlug = now.toISOString().split("T")[1].split(".")[0].replace(/:/g, ""); - slug = timeSlug.slice(0, 4); // HHMM + slug = timeSlug.slice(0, 6); // HHMMSS — seconds prevent same-minute overwrites log.debug("Using fallback timestamp slug", { slug }); } From 1ea1aa75df2f147ebbba83b020830bb163f583d1 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 09:29:41 -0700 Subject: [PATCH 07/45] feat(hooks): add postHookActions for order-independent hook coordination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a generic postHookActions mechanism to InternalHookEvent: handlers push deferred callbacks that triggerInternalHook drains after all handlers complete. This eliminates FIFO registration-order dependencies — any hook can set context flags regardless of when it registered. Session-memory handler updated to use fail-safe pattern: - Writes file inline (preserves data if postHookActions never runs) - Pushes post-hook action for retraction (blockSessionSave) and content replacement (sessionSaveContent) - Pre-set flags still work inline; late-set flags work via post-hook New tests: - 5 tests for postHookActions mechanism (ordering, error isolation, late-context visibility) - 2 tests for late-set blockSessionSave/sessionSaveContent - 1 fail-safe test (file preserved when postHookActions not drained) --- .../bundled/session-memory/handler.test.ts | 107 +++++++++++++++++- src/hooks/bundled/session-memory/handler.ts | 86 ++++++++++---- src/hooks/internal-hooks.test.ts | 94 +++++++++++++++ src/hooks/internal-hooks.ts | 22 ++++ 4 files changed, 280 insertions(+), 29 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index c2a7e07b79c..9bb2edaf68d 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -573,7 +573,16 @@ describe("session-memory hook", () => { expect(memoryContent).toContain("assistant: Only message 2"); }); - it("blockSessionSave prevents memory file creation", async () => { + // Helper to drain postHookActions (simulates what triggerInternalHook does) + async function drainPostHookActions(event: { + postHookActions: Array<() => Promise | void>; + }) { + for (const action of event.postHookActions) { + await action(); + } + } + + it("blockSessionSave (pre-set) prevents memory file creation", async () => { const tempDir = await createCaseWorkspace("block-save"); const sessionsDir = path.join(tempDir, "sessions"); await fs.mkdir(sessionsDir, { recursive: true }); @@ -590,13 +599,46 @@ describe("session-memory hook", () => { event.context.blockSessionSave = true; await handler(event); + await drainPostHookActions(event); const memoryDir = path.join(tempDir, "memory"); const memoryFiles = await fs.readdir(memoryDir).catch(() => [] as string[]); expect(memoryFiles.filter((f) => f.endsWith(".md"))).toHaveLength(0); }); - it("sessionSaveContent overrides saved content", async () => { + it("blockSessionSave (late-set) retracts memory file via postHookActions", async () => { + const tempDir = await createCaseWorkspace("block-save-late"); + 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: "secret" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + + // Handler writes the file inline (fail-safe) + await handler(event); + + const memoryDir = path.join(tempDir, "memory"); + let memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(memoryFiles.length).toBeGreaterThan(0); // file exists after inline write + + // A later hook sets blockSessionSave + event.context.blockSessionSave = true; + + // Post-hook action retracts the file + await drainPostHookActions(event); + + memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(memoryFiles).toHaveLength(0); + }); + + it("sessionSaveContent (pre-set) overrides saved content", async () => { const tempDir = await createCaseWorkspace("custom-content"); const sessionsDir = path.join(tempDir, "sessions"); await fs.mkdir(sessionsDir, { recursive: true }); @@ -613,6 +655,7 @@ describe("session-memory hook", () => { event.context.sessionSaveContent = "Custom summary from upstream hook"; await handler(event); + await drainPostHookActions(event); const memoryDir = path.join(tempDir, "memory"); const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); @@ -622,6 +665,37 @@ describe("session-memory hook", () => { expect(content).not.toContain("original"); }); + it("sessionSaveContent (late-set) overwrites file via postHookActions", async () => { + const tempDir = await createCaseWorkspace("late-custom-content"); + 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: "original" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + + // Handler writes default content inline + await handler(event); + + // A later hook sets custom content + event.context.sessionSaveContent = "Redacted by policy"; + + // Post-hook action overwrites + await drainPostHookActions(event); + + const memoryDir = path.join(tempDir, "memory"); + const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(files.length).toBeGreaterThan(0); + const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); + expect(content).toBe("Redacted by policy"); + }); + it("sessionSaveContent empty string writes blank marker file", async () => { const tempDir = await createCaseWorkspace("empty-content"); const sessionsDir = path.join(tempDir, "sessions"); @@ -636,17 +710,40 @@ describe("session-memory hook", () => { cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, previousSessionEntry: { sessionId: "s1", sessionFile }, }); - // Empty string is a valid override — persists a blank marker without - // loading the transcript or generating an LLM slug. event.context.sessionSaveContent = ""; await handler(event); + await drainPostHookActions(event); const memoryDir = path.join(tempDir, "memory"); const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); expect(files.length).toBeGreaterThan(0); const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); - // Should be truly empty — blank marker file expect(content).toBe(""); }); + + it("fail-safe: file is preserved if postHookActions never drain", async () => { + const tempDir = await createCaseWorkspace("fail-safe"); + 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: "important data" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + + await handler(event); + // Deliberately do NOT drain postHookActions — simulates a system failure + + const memoryDir = path.join(tempDir, "memory"); + const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(files.length).toBeGreaterThan(0); + const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); + expect(content).toContain("important data"); + }); }); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 660934416fa..868c8752088 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -208,18 +208,12 @@ const saveSessionToMemory: HookHandler = async (event) => { const context = event.context || {}; - // Check if another hook (e.g., security plugin) blocked the save. - // NOTE: Internal hooks execute sequentially in FIFO registration order. - // Bundled hooks register before managed/workspace hooks (see workspace.ts - // loadHookEntries merge order), so this check only works when the policy - // decision is set by a typed plugin hook (registerTypedHook) that fires - // earlier in the agent lifecycle, or by a hook loaded via extraDirs that - // registers before bundled hooks. Managed/workspace internal hooks that - // set blockSessionSave on the same event will run AFTER this handler. - if (context.blockSessionSave === true) { - log.debug("Session save blocked by upstream hook"); - return; - } + // NOTE: blockSessionSave and sessionSaveContent are checked in a + // postHookActions callback (see bottom of this handler) so that hooks + // registered after this bundled handler can still set them. The file + // is written inline (fail-safe: if postHookActions never runs, data is + // preserved on disk). The post-hook callback handles retraction + // (blockSessionSave) and content replacement (sessionSaveContent). const cfg = context.cfg as OpenClawConfig | undefined; const contextWorkspaceDir = @@ -238,7 +232,6 @@ const saveSessionToMemory: HookHandler = async (event) => { sessionKey: event.sessionKey, }); const memoryDir = path.join(workspaceDir, "memory"); - await fs.mkdir(memoryDir, { recursive: true }); // Get today's date for filename const now = new Date(event.timestamp); @@ -368,18 +361,63 @@ const saveSessionToMemory: HookHandler = async (event) => { entry = entryParts.join("\n"); } - // Write under memory root with alias-safe file validation. - await writeFileWithinRoot({ - rootDir: memoryDir, - relativePath: filename, - data: entry, - encoding: "utf-8", - }); - log.debug("Memory file written successfully"); + // 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. + if (context.blockSessionSave === true) { + log.debug("Session save blocked by upstream hook (inline check)"); + } else { + await fs.mkdir(memoryDir, { recursive: true }); + await writeFileWithinRoot({ + rootDir: memoryDir, + relativePath: filename, + data: entry, + encoding: "utf-8", + }); + log.debug("Memory file written successfully"); - // Log completion (but don't send user-visible confirmation - it's internal housekeeping) - const relPath = memoryFilePath.replace(os.homedir(), "~"); - log.info(`Session context saved to ${relPath}`); + const relPath = memoryFilePath.replace(os.homedir(), "~"); + log.info(`Session context saved to ${relPath}`); + } + + // Defer retraction/replacement to post-hook phase so that hooks + // registered after this handler can set blockSessionSave or + // sessionSaveContent and still have them honored. + const writtenEntry = context.blockSessionSave === true ? null : entry; + event.postHookActions.push(async () => { + try { + // If a later hook blocked the save, retract the file we just wrote. + if (event.context.blockSessionSave === true && writtenEntry !== null) { + try { + 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") { + throw err; + } + } + return; + } + + // If a later hook set sessionSaveContent, overwrite with new content. + const postContent = event.context.sessionSaveContent; + if (typeof postContent === "string" && postContent !== writtenEntry) { + await writeFileWithinRoot({ + rootDir: memoryDir, + relativePath: filename, + data: postContent, + encoding: "utf-8", + }); + log.debug("Session save content replaced by post-hook (sessionSaveContent)", { + length: postContent.length, + }); + } + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + log.error(`Post-hook session-memory action failed: ${message}`); + } + }); } catch (err) { if (err instanceof Error) { log.error("Failed to save session memory", { diff --git a/src/hooks/internal-hooks.test.ts b/src/hooks/internal-hooks.test.ts index 8f71c6b80cf..c0a87b014bb 100644 --- a/src/hooks/internal-hooks.test.ts +++ b/src/hooks/internal-hooks.test.ts @@ -445,6 +445,100 @@ describe("hooks", () => { }); }); + describe("postHookActions", () => { + it("executes post-hook actions after all handlers complete", async () => { + const order: string[] = []; + + registerInternalHook("command:new", async (event) => { + order.push("handler-1"); + event.postHookActions.push(async () => { + order.push("post-action-1"); + }); + }); + + registerInternalHook("command:new", async () => { + order.push("handler-2"); + }); + + const event = createInternalHookEvent("command", "new", "test-session"); + await triggerInternalHook(event); + + expect(order).toEqual(["handler-1", "handler-2", "post-action-1"]); + }); + + it("post-hook actions see context set by later handlers", async () => { + let sawFlag = false; + + registerInternalHook("command:new", async (event) => { + event.postHookActions.push(async () => { + sawFlag = event.context.myFlag === true; + }); + }); + + registerInternalHook("command:new", async (event) => { + event.context.myFlag = true; + }); + + const event = createInternalHookEvent("command", "new", "test-session"); + await triggerInternalHook(event); + + expect(sawFlag).toBe(true); + }); + + it("post-hook action errors don't prevent other post-hook actions", async () => { + const executed: string[] = []; + + registerInternalHook("command:new", async (event) => { + event.postHookActions.push(async () => { + throw new Error("boom"); + }); + event.postHookActions.push(async () => { + executed.push("second-action"); + }); + }); + + const event = createInternalHookEvent("command", "new", "test-session"); + await triggerInternalHook(event); + + expect(executed).toEqual(["second-action"]); + }); + + it("multiple handlers can push post-hook actions; all execute in order", async () => { + const order: string[] = []; + + registerInternalHook("command:new", async (event) => { + event.postHookActions.push(() => { + order.push("from-handler-1"); + }); + }); + + registerInternalHook("command:new", async (event) => { + event.postHookActions.push(() => { + order.push("from-handler-2"); + }); + }); + + const event = createInternalHookEvent("command", "new", "test-session"); + await triggerInternalHook(event); + + expect(order).toEqual(["from-handler-1", "from-handler-2"]); + }); + + it("initializes postHookActions as empty array", () => { + const event = createInternalHookEvent("command", "new", "test-session"); + expect(event.postHookActions).toEqual([]); + }); + + it("does not run post-hook actions when no handlers are registered", async () => { + const event = createInternalHookEvent("command", "new", "test-session"); + event.postHookActions.push(() => { + throw new Error("should not run"); + }); + // triggerInternalHook returns early when no handlers — post-hooks don't run + await expect(triggerInternalHook(event)).resolves.not.toThrow(); + }); + }); + describe("getRegisteredEventKeys", () => { it("should return all registered event keys", () => { registerInternalHook("command:new", vi.fn()); diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index b73dcb75fab..a8d0736fbf0 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -169,6 +169,12 @@ export interface InternalHookEvent { timestamp: Date; /** Messages to send back to the user (hooks can push to this array) */ messages: string[]; + /** Deferred actions to run after all handlers complete. + * Handlers push async callbacks here; triggerInternalHook drains them + * sequentially after the main handler loop. This eliminates FIFO + * registration-order dependencies: a handler that runs early can defer + * work that depends on context set by later handlers. */ + postHookActions: Array<() => Promise | void>; } export type InternalHookHandler = (event: InternalHookEvent) => Promise | void; @@ -285,6 +291,21 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise 0) { + for (const action of event.postHookActions) { + try { + await action(); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`); + } + } + } } /** @@ -308,6 +329,7 @@ export function createInternalHookEvent( context, timestamp: new Date(), messages: [], + postHookActions: [], }; } From 0621de477343c59c888db98dcc76c2c369353d17 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 09:45:52 -0700 Subject: [PATCH 08/45] fix(session-memory): blockSessionSave takes precedence over sessionSaveContent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When both flags are set, blockSessionSave must win — a blocked save should never create a file, even if sessionSaveContent is also present. Bug: if blockSessionSave was pre-set (writtenEntry=null), the post-hook sessionSaveContent check would pass and create a new file, violating the block intent. Fix: guard the sessionSaveContent overwrite with blockSessionSave !== true. Tests: 2 new tests covering both-flags-pre-set and both-flags-late-set. Credit: Greptile review. --- .../bundled/session-memory/handler.test.ts | 54 +++++++++++++++++++ src/hooks/bundled/session-memory/handler.ts | 8 ++- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 9bb2edaf68d..85bd757727a 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -746,4 +746,58 @@ describe("session-memory hook", () => { const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); expect(content).toContain("important data"); }); + + it("blockSessionSave takes precedence over sessionSaveContent (both pre-set)", async () => { + const tempDir = await createCaseWorkspace("block-beats-content-pre"); + 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: "secret" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + event.context.blockSessionSave = true; + event.context.sessionSaveContent = "Should not appear"; + + await handler(event); + await drainPostHookActions(event); + + const memoryDir = path.join(tempDir, "memory"); + const memoryFiles = await fs.readdir(memoryDir).catch(() => [] as string[]); + expect(memoryFiles.filter((f) => f.endsWith(".md"))).toHaveLength(0); + }); + + it("blockSessionSave takes precedence over sessionSaveContent (both late-set)", async () => { + const tempDir = await createCaseWorkspace("block-beats-content-late"); + 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: "secret" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + + // Handler writes inline (no flags set yet) + await handler(event); + + // Later hooks set both flags + event.context.blockSessionSave = true; + event.context.sessionSaveContent = "Should not appear"; + + await drainPostHookActions(event); + + const memoryDir = path.join(tempDir, "memory"); + const memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(memoryFiles).toHaveLength(0); + }); }); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 868c8752088..e4b84218242 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -401,8 +401,14 @@ const saveSessionToMemory: HookHandler = async (event) => { } // If a later hook set sessionSaveContent, overwrite with new content. + // blockSessionSave takes precedence — never create/overwrite a file that + // was blocked, even if sessionSaveContent is also set. const postContent = event.context.sessionSaveContent; - if (typeof postContent === "string" && postContent !== writtenEntry) { + if ( + event.context.blockSessionSave !== true && + typeof postContent === "string" && + postContent !== writtenEntry + ) { await writeFileWithinRoot({ rootDir: memoryDir, relativePath: filename, From 64d52cf2ea73ca58b0dee15ca54618d729e0952f Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 09:52:48 -0700 Subject: [PATCH 09/45] fix(session-memory): skip transcript loading and LLM call when blockSessionSave is pre-set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When blockSessionSave is already true, the handler was still loading session content and potentially calling generateSlugViaLLM — sending sensitive transcript text to a model provider despite an explicit block. Short-circuit before transcript loading when the flag is set. Credit: Codex review. --- src/hooks/bundled/session-memory/handler.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index e4b84218242..fa038880051 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -288,7 +288,12 @@ const saveSessionToMemory: HookHandler = async (event) => { let sessionContent: string | null = null; const hasCustomContent = typeof context.sessionSaveContent === "string"; - if (sessionFile && !hasCustomContent) { + // Short-circuit transcript loading and LLM slug generation when + // blockSessionSave is already set — no point loading sensitive content + // or sending it to a model provider when saving is explicitly blocked. + const blockPreSet = context.blockSessionSave === true; + + if (sessionFile && !hasCustomContent && !blockPreSet) { // Get recent conversation content, with fallback to rotated reset transcript. sessionContent = await getRecentSessionContentWithResetFallback(sessionFile, messageCount); log.debug("Session content loaded", { From 4040a25bb19d3cd4783a59503a675b0ed74f779e Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 10:09:43 -0700 Subject: [PATCH 10/45] fix(session-memory): ensure memoryDir exists in post-hook write path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When blockSessionSave is true initially, the inline write is skipped — including the fs.mkdir that creates memoryDir. If a later hook clears blockSessionSave and sets sessionSaveContent, the post-hook writeFileWithinRoot call would fail with ENOENT because the directory was never created. The error was silently swallowed, causing the content override to be lost. Add fs.mkdir(memoryDir, { recursive: true }) before the post-hook write. Add regression test for the block-then-clear-with-content scenario. --- .../bundled/session-memory/handler.test.ts | 43 +++++++++++++++++++ src/hooks/bundled/session-memory/handler.ts | 4 ++ 2 files changed, 47 insertions(+) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 85bd757727a..fbee8e9eba6 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -747,6 +747,49 @@ describe("session-memory hook", () => { expect(content).toContain("important data"); }); + it("blockSessionSave pre-set then cleared with sessionSaveContent creates file (mkdir edge case)", async () => { + // Regression: when blockSessionSave is true initially, the inline write + // is skipped — including the fs.mkdir. If a later hook clears the flag + // and sets sessionSaveContent, the post-hook write must create the + // directory itself or it fails with ENOENT. + const tempDir = await createCaseWorkspace("block-then-clear"); + 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: "secret" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + event.context.blockSessionSave = true; + + // Handler runs — inline write is skipped, memoryDir never created + await handler(event); + + const memoryDir = path.join(tempDir, "memory"); + const existsBefore = await fs + .stat(memoryDir) + .then(() => true) + .catch(() => false); + expect(existsBefore).toBe(false); + + // A later hook clears blockSessionSave and sets custom content + event.context.blockSessionSave = false; + event.context.sessionSaveContent = "Replacement content from policy hook"; + + // Post-hook should create the directory and write the file + await drainPostHookActions(event); + + const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(files.length).toBeGreaterThan(0); + const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8"); + expect(content).toBe("Replacement content from policy hook"); + }); + it("blockSessionSave takes precedence over sessionSaveContent (both pre-set)", async () => { const tempDir = await createCaseWorkspace("block-beats-content-pre"); 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 fa038880051..91ca2ad698b 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -414,6 +414,10 @@ const saveSessionToMemory: HookHandler = async (event) => { typeof postContent === "string" && postContent !== writtenEntry ) { + // Ensure memoryDir exists — the inline write may have been + // skipped (e.g. blockSessionSave was true initially) so mkdir + // might never have run. + await fs.mkdir(memoryDir, { recursive: true }); await writeFileWithinRoot({ rootDir: memoryDir, relativePath: filename, From 51fdf17867c0917e29ce4f501e44b36a87abb15d Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 10:16:02 -0700 Subject: [PATCH 11/45] fix: drain postHookActions even with no registered handlers Remove early return in triggerInternalHook that skipped the post-hook drain when allHandlers was empty. This was a latent footgun: any postHookActions pre-populated on the event before calling triggerInternalHook would be silently dropped if no handlers were registered for that event type. Update test to verify post-hooks now drain in the no-handlers case. --- src/hooks/internal-hooks.test.ts | 10 ++++++---- src/hooks/internal-hooks.ts | 4 ---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/hooks/internal-hooks.test.ts b/src/hooks/internal-hooks.test.ts index c0a87b014bb..a2b51c803c7 100644 --- a/src/hooks/internal-hooks.test.ts +++ b/src/hooks/internal-hooks.test.ts @@ -529,13 +529,15 @@ describe("hooks", () => { expect(event.postHookActions).toEqual([]); }); - it("does not run post-hook actions when no handlers are registered", async () => { + it("drains post-hook actions even when no handlers are registered", async () => { const event = createInternalHookEvent("command", "new", "test-session"); + let ran = false; event.postHookActions.push(() => { - throw new Error("should not run"); + ran = true; }); - // triggerInternalHook returns early when no handlers — post-hooks don't run - await expect(triggerInternalHook(event)).resolves.not.toThrow(); + // No handlers registered — post-hooks should still drain + await triggerInternalHook(event); + expect(ran).toBe(true); }); }); diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index a8d0736fbf0..2feb80e8af1 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -279,10 +279,6 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise Date: Sat, 7 Mar 2026 10:31:08 -0700 Subject: [PATCH 12/45] cleanup: address greptile review nits for 5/5 - Remove redundant length guard on postHookActions drain loop - Add per-action error isolation to drainPostHookActions test helper, matching actual triggerInternalHook behaviour - Remove double-catch in session-memory post-hook callback; let errors propagate to the framework's per-action catch for consistent logging --- .../bundled/session-memory/handler.test.ts | 9 ++- src/hooks/bundled/session-memory/handler.ts | 74 +++++++++---------- src/hooks/internal-hooks.ts | 14 ++-- 3 files changed, 49 insertions(+), 48 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index fbee8e9eba6..4c429b12596 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -573,12 +573,17 @@ describe("session-memory hook", () => { expect(memoryContent).toContain("assistant: Only message 2"); }); - // Helper to drain postHookActions (simulates what triggerInternalHook does) + // Helper to drain postHookActions with per-action error isolation, + // matching triggerInternalHook's actual drain behaviour. async function drainPostHookActions(event: { postHookActions: Array<() => Promise | void>; }) { for (const action of event.postHookActions) { - await action(); + try { + await action(); + } catch { + // Per-action isolation — one failure doesn't block others. + } } } diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 91ca2ad698b..fcb04eb1c28 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -389,48 +389,46 @@ const saveSessionToMemory: HookHandler = async (event) => { // registered after this handler can set blockSessionSave or // sessionSaveContent and still have them honored. const writtenEntry = context.blockSessionSave === true ? null : entry; + // Post-hook callback — errors propagate to the framework's per-action + // catch in triggerInternalHook, which provides consistent log formatting + // and per-action isolation. event.postHookActions.push(async () => { - try { - // If a later hook blocked the save, retract the file we just wrote. - if (event.context.blockSessionSave === true && writtenEntry !== null) { - try { - 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") { - throw err; - } + // If a later hook blocked the save, retract the file we just wrote. + if (event.context.blockSessionSave === true && writtenEntry !== null) { + try { + 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") { + throw err; } - return; } + return; + } - // If a later hook set sessionSaveContent, overwrite with new content. - // blockSessionSave takes precedence — never create/overwrite a file that - // was blocked, even if sessionSaveContent is also set. - const postContent = event.context.sessionSaveContent; - if ( - event.context.blockSessionSave !== true && - typeof postContent === "string" && - postContent !== writtenEntry - ) { - // Ensure memoryDir exists — the inline write may have been - // skipped (e.g. blockSessionSave was true initially) so mkdir - // might never have run. - await fs.mkdir(memoryDir, { recursive: true }); - await writeFileWithinRoot({ - rootDir: memoryDir, - relativePath: filename, - data: postContent, - encoding: "utf-8", - }); - log.debug("Session save content replaced by post-hook (sessionSaveContent)", { - length: postContent.length, - }); - } - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - log.error(`Post-hook session-memory action failed: ${message}`); + // If a later hook set sessionSaveContent, overwrite with new content. + // blockSessionSave takes precedence — never create/overwrite a file that + // was blocked, even if sessionSaveContent is also set. + const postContent = event.context.sessionSaveContent; + if ( + event.context.blockSessionSave !== true && + typeof postContent === "string" && + postContent !== writtenEntry + ) { + // Ensure memoryDir exists — the inline write may have been + // skipped (e.g. blockSessionSave was true initially) so mkdir + // might never have run. + await fs.mkdir(memoryDir, { recursive: true }); + await writeFileWithinRoot({ + rootDir: memoryDir, + relativePath: filename, + data: postContent, + encoding: "utf-8", + }); + log.debug("Session save content replaced by post-hook (sessionSaveContent)", { + length: postContent.length, + }); } }); } catch (err) { diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 2feb80e8af1..1d02a4aa2b3 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -292,14 +292,12 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise 0) { - for (const action of event.postHookActions) { - try { - await action(); - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`); - } + for (const action of event.postHookActions) { + try { + await action(); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`); } } } From decdddbe3eee5f783db80e436afd6b25cce1f8a2 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 10:49:04 -0700 Subject: [PATCH 13/45] security: disable tools for LLM slug generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The slug generator embeds up to 2000 chars of raw conversation content in its prompt. Without disableTools, the embedded agent inherits the full tool set (exec, file write, messaging), meaning a crafted conversation could prompt-inject the slug call into executing arbitrary side-effects before the slug text is extracted. Slug generation is pure text — it never needs tool access. Add disableTools: true to close this injection surface. --- src/hooks/llm-slug-generator.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/hooks/llm-slug-generator.ts b/src/hooks/llm-slug-generator.ts index eb355fc3289..cbfeaf032cf 100644 --- a/src/hooks/llm-slug-generator.ts +++ b/src/hooks/llm-slug-generator.ts @@ -50,6 +50,14 @@ Reply with ONLY the slug, nothing else. Examples: "vendor-pitch", "api-design", const provider = parsed?.provider ?? DEFAULT_PROVIDER; const model = parsed?.model ?? DEFAULT_MODEL; + // Security: disable tools for this one-shot call. The prompt embeds + // up to 2 000 chars of raw conversation content, which is attacker- + // controllable. Without disableTools the embedded agent inherits the + // full tool set (exec, file write, messaging, …), so a crafted + // conversation could prompt-inject the slug-generation call into + // executing arbitrary side-effects *before* the (well-sanitised) slug + // text is extracted. Slug generation is pure text — it never needs + // tool access. const result = await runEmbeddedPiAgent({ sessionId: `slug-generator-${Date.now()}`, sessionKey: "temp:slug-generator", @@ -61,6 +69,7 @@ Reply with ONLY the slug, nothing else. Examples: "vendor-pitch", "api-design", prompt, provider, model, + disableTools: true, timeoutMs: 15_000, // 15 second timeout runId: `slug-gen-${Date.now()}`, }); From 1e11385cbdfa93c3fe8e16a3040ed12a10c4dfe8 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 10:54:03 -0700 Subject: [PATCH 14/45] fix: add random suffix to fallback slug to prevent same-second collisions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-resolution (HHMMSS) fallback slugs can collide when automated or multi-channel setups emit rapid /new or /reset commands within the same second — both writes target the same filename and the later one silently overwrites the earlier memory entry. Append a 4-char random alphanumeric suffix (e.g. 103022-x7f2) to make collisions effectively impossible without LLM slug generation. --- src/hooks/bundled/session-memory/handler.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index fcb04eb1c28..56321d26b1d 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -317,10 +317,15 @@ const saveSessionToMemory: HookHandler = async (event) => { } } - // If no slug, use timestamp + // If no slug, use timestamp with a random suffix to avoid collisions. + // Second-resolution (HHMMSS) alone can collide when automated or + // multi-channel setups emit rapid /new or /reset commands within the + // same second — both writes target the same filename and the later + // one silently overwrites the earlier memory entry. if (!slug) { const timeSlug = now.toISOString().split("T")[1].split(".")[0].replace(/:/g, ""); - slug = timeSlug.slice(0, 6); // HHMMSS — seconds prevent same-minute overwrites + const rand = Math.random().toString(36).slice(2, 6); // 4-char alphanumeric + slug = `${timeSlug.slice(0, 6)}-${rand}`; log.debug("Using fallback timestamp slug", { slug }); } From ff014f96e4033afd31ea4c6742915c8772ec430e Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 11:07:54 -0700 Subject: [PATCH 15/45] fix: add postHookActions to boot-md test fixture The InternalHookEvent interface now requires postHookActions (added by this PR). The boot-md test's makeEvent helper was missing it, causing a tsc error that fails CI. --- src/hooks/bundled/boot-md/handler.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hooks/bundled/boot-md/handler.test.ts b/src/hooks/bundled/boot-md/handler.test.ts index de2abd6475f..46466b254b8 100644 --- a/src/hooks/bundled/boot-md/handler.test.ts +++ b/src/hooks/bundled/boot-md/handler.test.ts @@ -37,6 +37,7 @@ function makeEvent(overrides?: Partial): InternalHookEvent { context: {}, timestamp: new Date(), messages: [], + postHookActions: [], ...overrides, }; } From 71609fa108021ca6071996852cdc5978b09113ce Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 11:14:41 -0700 Subject: [PATCH 16/45] ci: retrigger CI (oxfmt check may be stale) From 5a135d855cf0ef073c9ba6458103f1724de38de6 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sat, 7 Mar 2026 13:06:20 -0700 Subject: [PATCH 17/45] docs: document known limitation for cleared sessionSaveContent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an earlier hook pre-sets sessionSaveContent and a later hook clears it, the transcript is not available for fallback — it was never loaded. Hooks wanting to override should set their own content, not clear it. --- src/hooks/bundled/session-memory/handler.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 56321d26b1d..fe4736c5bbc 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -293,6 +293,14 @@ const saveSessionToMemory: HookHandler = async (event) => { // or sending it to a model provider when saving is explicitly blocked. const blockPreSet = context.blockSessionSave === true; + // Known limitation: if an earlier hook pre-sets sessionSaveContent and + // a later hook *clears* it (expecting a revert to the default + // transcript), the transcript is not available — it was never loaded + // because hasCustomContent was true at this point. The post-hook + // cannot fall back to the default entry without re-reading the session + // file and re-running slug generation. In practice, hooks that want + // to override earlier custom content should set their own + // sessionSaveContent rather than clearing it. if (sessionFile && !hasCustomContent && !blockPreSet) { // Get recent conversation content, with fallback to rotated reset transcript. sessionContent = await getRecentSessionContentWithResetFallback(sessionFile, messageCount); From 40a6a3ce76039ab93c2a3360e46616d2fc70e4a3 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 16:58:34 -0700 Subject: [PATCH 18/45] fix(session-memory): use displaySessionKey instead of event.sessionKey in entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolveDisplaySessionKey function correctly resolved workspace-based agent IDs (e.g. agent:main:main → agent:navi:main when workspaceDir matches the navi agent), but the entry template used event.sessionKey directly, bypassing the resolution. --- src/hooks/bundled/session-memory/handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index fe4736c5bbc..4de19361545 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -366,7 +366,7 @@ const saveSessionToMemory: HookHandler = async (event) => { const entryParts = [ `# Session: ${dateStr} ${timeStr} UTC`, "", - `- **Session Key**: ${event.sessionKey}`, + `- **Session Key**: ${displaySessionKey}`, `- **Session ID**: ${sessionId}`, `- **Source**: ${source}`, "", From 2509356e0832c4dda0c1c04205d01850fa59f568 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 17:04:25 -0700 Subject: [PATCH 19/45] 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") { From 25093b7024bbb0b2696d7628b97fbb2a3835de43 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 17:37:43 -0700 Subject: [PATCH 20/45] fix: warn when blockSessionSave is cleared without sessionSaveContent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When blockSessionSave is pre-set, the handler skips transcript loading and LLM slug generation (avoiding unnecessary I/O and model calls). If a later hook clears blockSessionSave without setting sessionSaveContent, no file can be produced because the transcript was never loaded. Previously this was a silent no-op. Now emits log.warn so plugin authors know to supply sessionSaveContent when un-blocking a pre-set block. Adds test: 'blockSessionSave pre-set then cleared without sessionSaveContent warns and writes nothing' — verifies the edge case produces no file. --- .../bundled/session-memory/handler.test.ts | 32 +++++++++++++++++++ src/hooks/bundled/session-memory/handler.ts | 13 ++++++++ 2 files changed, 45 insertions(+) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 10c1ab98a62..98b0966629c 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -903,4 +903,36 @@ describe("session-memory hook", () => { const memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); expect(memoryFiles).toHaveLength(0); }); + + it("blockSessionSave pre-set then cleared without sessionSaveContent warns and writes nothing", async () => { + const tempDir = await createCaseWorkspace("block-cleared-no-content"); + 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: "will not be saved" }]), + }); + + const event = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + + // Pre-set blockSessionSave — handler skips transcript loading + inline write + event.context.blockSessionSave = true; + + await handler(event); + + // A later hook clears blockSessionSave but forgets to set sessionSaveContent. + // Since the transcript was never loaded, no file can be produced. + event.context.blockSessionSave = false; + + await drainPostHookActions(event); + + // No memory file should exist — the transcript was never loaded + const memoryDir = path.join(tempDir, "memory"); + const memoryFiles = await fs.readdir(memoryDir).catch(() => [] as string[]); + expect(memoryFiles.filter((f) => f.endsWith(".md"))).toHaveLength(0); + }); }); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index cb1d811e051..b32d990db4f 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -464,6 +464,19 @@ const saveSessionToMemory: HookHandler = async (event) => { log.debug("Session save content replaced by post-hook (sessionSaveContent)", { length: postContent.length, }); + } else if ( + event.context.blockSessionSave !== true && + writtenEntry === null && + typeof postContent !== "string" + ) { + // blockSessionSave was pre-set (causing writtenEntry=null and no inline + // write), then a later hook cleared it without providing sessionSaveContent. + // The transcript was never loaded, so we cannot produce a file. Warn so + // plugin authors know to supply content when un-blocking. + log.warn( + "blockSessionSave was cleared but no sessionSaveContent provided — " + + "no memory file written (transcript was not loaded during pre-set block)", + ); } }); } catch (err) { From c6110a3b152d0395debdd11c63fbeef53222f88d Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 17:54:20 -0700 Subject: [PATCH 21/45] fix: separate ENOENT handling for unlink vs writeFileWithinRoot in retraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the single catch block into two distinct error-handling paths: 1. Pre-existing content restore (writeFileWithinRoot): errors are NOT swallowed — ENOENT here means memoryDir was removed after our inline write, which is a real filesystem inconsistency that must surface. 2. Unlink (no pre-existing content): ENOENT is expected when the inline write didn't happen. Non-ENOENT errors (EACCES, EROFS) are re-thrown for triggerInternalHook to log. Added comment clarifying that the retraction guarantee is best-effort under adversarial filesystem conditions (per-action isolation absorbs the error). --- src/hooks/bundled/session-memory/handler.ts | 40 +++++++++++++-------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index b32d990db4f..e9aea24aa20 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -420,23 +420,33 @@ const saveSessionToMemory: HookHandler = async (event) => { // 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 { - 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 { + if (preExistingContent !== null) { + // Slug collision: another entry already existed at this filename + // before our inline write. Restore the original content rather + // than deleting — preserves the prior session's history. + // writeFileWithinRoot errors (e.g. ENOENT if memoryDir was + // removed after our inline write) are NOT swallowed — they + // indicate a real filesystem inconsistency that must surface. + await writeFileWithinRoot({ + rootDir: memoryDir, + relativePath: filename, + data: preExistingContent, + encoding: "utf-8", + }); + log.debug("Session save retracted by post-hook — pre-existing file restored"); + } else { + try { 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") { - throw err; + } catch (err) { + // ENOENT is expected when the inline write didn't happen + // (e.g. blockSessionSave was set before writeFileWithinRoot). + // Re-throw so triggerInternalHook logs it. Note: the error + // is caught per-action and does NOT propagate to the caller; + // the file may remain on disk if unlink fails (e.g. EACCES). + if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + throw err; + } } } return; From a0073bfb9b6d2b276eede26ef1dcc7ef8a5bf743 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 18:16:03 -0700 Subject: [PATCH 22/45] =?UTF-8?q?fix:=20address=20remaining=20greptile=204?= =?UTF-8?q?/5=20items=20=E2=80=94=20random=20guard,=20ENOENT=20comment,=20?= =?UTF-8?q?array=20snapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Math.random() === 0 guard: (0).toString(36).slice(2,6) returns '' producing a trailing-hyphen slug. Added || '0000' fallback. 2. Misleading ENOENT comment: said 'when blockSessionSave was set before writeFileWithinRoot' but that branch requires writtenEntry !== null (blockSessionSave was false). Real scenario: external file deletion between inline write and post-hook drain. Comment corrected. 3. Live-array drain: for...of on postHookActions is a live iterator — a self-scheduling action could loop infinitely. Snapshot the array with [...(event.postHookActions ?? [])] before draining. --- src/hooks/bundled/session-memory/handler.ts | 13 +++++++------ src/hooks/internal-hooks.ts | 8 +++++++- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index e9aea24aa20..323ed371e71 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -332,7 +332,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // one silently overwrites the earlier memory entry. if (!slug) { const timeSlug = now.toISOString().split("T")[1].split(".")[0].replace(/:/g, ""); - const rand = Math.random().toString(36).slice(2, 6); // 4-char alphanumeric + const rand = Math.random().toString(36).slice(2, 6) || "0000"; // 4-char alphanumeric slug = `${timeSlug.slice(0, 6)}-${rand}`; log.debug("Using fallback timestamp slug", { slug }); } @@ -439,11 +439,12 @@ const saveSessionToMemory: HookHandler = async (event) => { await fs.unlink(memoryFilePath); log.debug("Session save retracted by post-hook (blockSessionSave)"); } catch (err) { - // ENOENT is expected when the inline write didn't happen - // (e.g. blockSessionSave was set before writeFileWithinRoot). - // Re-throw so triggerInternalHook logs it. Note: the error - // is caught per-action and does NOT propagate to the caller; - // the file may remain on disk if unlink fails (e.g. EACCES). + // ENOENT can occur if the file was externally deleted between + // the inline write and the post-hook drain — not a concern. + // Re-throw non-ENOENT errors (e.g. EACCES, EROFS) so + // triggerInternalHook logs them. Note: errors are caught + // per-action and do NOT propagate to the session caller; + // the file may remain on disk under adversarial FS conditions. if ((err as NodeJS.ErrnoException).code !== "ENOENT") { throw err; } diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 1d02a4aa2b3..4dcf3040d89 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -292,7 +292,13 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise Date: Sun, 8 Mar 2026 21:50:57 -0700 Subject: [PATCH 23/45] fix(test): snapshot postHookActions array in test helper to match production drain The test drainPostHookActions helper iterated the live array, but triggerInternalHook snapshots with [...(event.postHookActions ?? [])] before draining. Align test helper to match production semantics. --- src/hooks/bundled/session-memory/handler.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 98b0966629c..c192ff5f624 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -578,7 +578,11 @@ describe("session-memory hook", () => { async function drainPostHookActions(event: { postHookActions: Array<() => Promise | void>; }) { - for (const action of event.postHookActions) { + // Snapshot before draining — matches triggerInternalHook's production + // semantics (prevents self-scheduling actions from executing in the + // same drain cycle). + const pending = [...event.postHookActions]; + for (const action of pending) { try { await action(); } catch { From 419249fabb0666f89ff5871ea9b23fb54f339467 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 22:57:26 -0700 Subject: [PATCH 24/45] fix: pad random slug suffix to guarantee 4 characters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Math.random().toString(36).slice(2,6) can produce fewer than 4 chars for short base-36 representations (e.g. 0.5 → '0.i' → 'i'). Append '0000' before slicing to guarantee the suffix is always 4 chars. --- src/hooks/bundled/session-memory/handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 323ed371e71..ed70e42c9a7 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -332,7 +332,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // one silently overwrites the earlier memory entry. if (!slug) { const timeSlug = now.toISOString().split("T")[1].split(".")[0].replace(/:/g, ""); - const rand = Math.random().toString(36).slice(2, 6) || "0000"; // 4-char alphanumeric + const rand = (Math.random().toString(36) + "0000").slice(2, 6); // guaranteed 4-char slug = `${timeSlug.slice(0, 6)}-${rand}`; log.debug("Using fallback timestamp slug", { slug }); } From abd7c877aae2b06f938d8be7e05a9960bd0bcee8 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 23:08:27 -0700 Subject: [PATCH 25/45] fix: use padEnd for truly guaranteed 4-char random slug suffix Previous approach ((random + '0000').slice(2,6)) still produced 3 chars when Math.random() === 0 ('0'.toString(36) = '0', '00000'.slice(2,6) = '000'). padEnd(4, '0') guarantees exactly 4 characters for all inputs. --- src/hooks/bundled/session-memory/handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index ed70e42c9a7..f85450a16c9 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -332,7 +332,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // one silently overwrites the earlier memory entry. if (!slug) { const timeSlug = now.toISOString().split("T")[1].split(".")[0].replace(/:/g, ""); - const rand = (Math.random().toString(36) + "0000").slice(2, 6); // guaranteed 4-char + const rand = Math.random().toString(36).slice(2, 6).padEnd(4, "0"); slug = `${timeSlug.slice(0, 6)}-${rand}`; log.debug("Using fallback timestamp slug", { slug }); } From c643651199aa87d865680e1d23ce0fa25b7b5946 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Sun, 8 Mar 2026 23:19:42 -0700 Subject: [PATCH 26/45] test: exercise slug-collision restoration path with deterministic Math.random MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test couldn't trigger a slug collision because the random suffix made same-filename generation virtually impossible. Pin Math.random to a fixed value so both handler calls produce the same fallback slug, exercising the preExistingContent !== null restoration branch. Verifies: second handler overwrites first file → late blockSessionSave retracts second write → first session's content is restored (not deleted). --- .../bundled/session-memory/handler.test.ts | 84 ++++++++++--------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index c192ff5f624..8b8326ed300 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -647,59 +647,65 @@ describe("session-memory hook", () => { expect(memoryFiles).toHaveLength(0); }); - it("late-block retraction restores pre-existing file instead of deleting", async () => { + it("late-block retraction restores pre-existing file instead of deleting (slug collision)", 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" }]), + content: createMockSessionContent([{ role: "user", content: "first 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); + // Pin Math.random to force deterministic slug — both handler calls + // produce the same fallback filename, exercising the slug-collision + // restoration path (preExistingContent !== null). + const origRandom = Math.random; + Math.random = () => 0.5; - 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); + try { + // First handler: creates memory file with deterministic slug. + const event1 = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s1", sessionFile }, + }); + await handler(event1); + await drainPostHookActions(event1); - // 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"); + const memoryDir = path.join(tempDir, "memory"); + const files1 = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); + expect(files1).toHaveLength(1); + const collidingFile = files1[0]; + const collidingPath = path.join(memoryDir, collidingFile); + const originalContent = await fs.readFile(collidingPath, "utf-8"); + expect(originalContent).toContain("first session"); - // 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); + // Second handler: same deterministic slug → overwrites the file (collision). + const sessionFile2 = await writeWorkspaceFile({ + dir: sessionsDir, + name: "test-session2.jsonl", + content: createMockSessionContent([{ role: "user", content: "second session" }]), + }); + const event2 = createHookEvent("command", "new", "agent:main:main", { + cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, + previousSessionEntry: { sessionId: "s2", sessionFile: sessionFile2 }, + }); + 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. + // Verify the file was overwritten by second handler. + const overwrittenContent = await fs.readFile(collidingPath, "utf-8"); + expect(overwrittenContent).toContain("second session"); - // Late-block: a later hook sets blockSessionSave - event2.context.blockSessionSave = true; - await drainPostHookActions(event2); + // Late-block: retraction should restore the FIRST session's content. + 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"); + const restoredContent = await fs.readFile(collidingPath, "utf-8"); + expect(restoredContent).toContain("first session"); + expect(restoredContent).not.toContain("second session"); + } finally { + Math.random = origRandom; + } }); it("sessionSaveContent (pre-set) overrides saved content", async () => { From 83817718278a9dfe15f851ed9097358f9e577ba3 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 00:25:33 -0700 Subject: [PATCH 27/45] fix(hooks): make postHookActions drain idempotent, clarify block-then-clear warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Clear postHookActions array after draining (both production triggerInternalHook and test helper). Re-triggering the same event is now a safe no-op instead of re-executing every action. 2. Expand warning log in block-then-clear scenario to explain that transcript was skipped because sessionSaveContent OR blockSessionSave was pre-set, and that plugin authors must provide sessionSaveContent when clearing blockSessionSave. 3. Add test: 'clears postHookActions after drain — re-trigger is a no-op' verifying the idempotency contract. --- src/hooks/bundled/session-memory/handler.test.ts | 2 ++ src/hooks/bundled/session-memory/handler.ts | 5 ++++- src/hooks/internal-hooks.test.ts | 14 ++++++++++++++ src/hooks/internal-hooks.ts | 5 +++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 8b8326ed300..4ed0c286aff 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -582,6 +582,8 @@ describe("session-memory hook", () => { // semantics (prevents self-scheduling actions from executing in the // same drain cycle). const pending = [...event.postHookActions]; + // Clear source array so re-drain is a no-op (matches production). + event.postHookActions.length = 0; for (const action of pending) { try { await action(); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index f85450a16c9..a8d4725cc09 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -486,7 +486,10 @@ const saveSessionToMemory: HookHandler = async (event) => { // plugin authors know to supply content when un-blocking. log.warn( "blockSessionSave was cleared but no sessionSaveContent provided — " + - "no memory file written (transcript was not loaded during pre-set block)", + "no memory file written. Transcript was not loaded because " + + "sessionSaveContent or blockSessionSave was pre-set during handler " + + "execution. To write a file after clearing blockSessionSave, also " + + "provide sessionSaveContent with the desired content.", ); } }); diff --git a/src/hooks/internal-hooks.test.ts b/src/hooks/internal-hooks.test.ts index a2b51c803c7..ffa7639bcb0 100644 --- a/src/hooks/internal-hooks.test.ts +++ b/src/hooks/internal-hooks.test.ts @@ -539,6 +539,20 @@ describe("hooks", () => { await triggerInternalHook(event); expect(ran).toBe(true); }); + + it("clears postHookActions after drain — re-trigger is a no-op", async () => { + const event = createInternalHookEvent("command", "new", "test-session"); + let count = 0; + event.postHookActions.push(() => { + count++; + }); + await triggerInternalHook(event); + expect(count).toBe(1); + // Second trigger on same event should not re-execute + await triggerInternalHook(event); + expect(count).toBe(1); + expect(event.postHookActions).toEqual([]); + }); }); describe("getRegisteredEventKeys", () => { diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 4dcf3040d89..f7ac8c802a6 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -298,6 +298,11 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise Date: Mon, 9 Mar 2026 00:48:41 -0700 Subject: [PATCH 28/45] fix(hooks): pin timestamp in collision test, avoid dead entry construction, reuse blockPreSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Slug-collision test: Pin event.timestamp to a fixed Date in addition to Math.random, preventing flaky behavior when wall-clock crosses a second boundary between event1 and event2. 2. handler.ts: Skip entry construction when blockPreSet is true (the value is discarded — writtenEntry will be null regardless). Avoids misleading dead code. 3. handler.ts: Reuse blockPreSet in the inline-write guard instead of re-evaluating context.blockSessionSave === true. Single source of truth for the pre-set block condition. --- src/hooks/bundled/session-memory/handler.test.ts | 11 ++++++++--- src/hooks/bundled/session-memory/handler.ts | 8 ++++++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 4ed0c286aff..af95c00d41f 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -659,11 +659,14 @@ describe("session-memory hook", () => { content: createMockSessionContent([{ role: "user", content: "first session" }]), }); - // Pin Math.random to force deterministic slug — both handler calls - // produce the same fallback filename, exercising the slug-collision - // restoration path (preExistingContent !== null). + // Pin Math.random AND timestamp to force deterministic slug — both + // handler calls produce the same fallback filename, exercising the + // slug-collision restoration path (preExistingContent !== null). + // Without pinning the clock, a wall-clock second boundary between + // event1 and event2 would produce different HHMMSS prefixes → no collision. const origRandom = Math.random; Math.random = () => 0.5; + const fixedTimestamp = new Date("2024-01-15T12:34:56.000Z"); try { // First handler: creates memory file with deterministic slug. @@ -671,6 +674,7 @@ describe("session-memory hook", () => { cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, previousSessionEntry: { sessionId: "s1", sessionFile }, }); + event1.timestamp = fixedTimestamp; await handler(event1); await drainPostHookActions(event1); @@ -692,6 +696,7 @@ describe("session-memory hook", () => { cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig, previousSessionEntry: { sessionId: "s2", sessionFile: sessionFile2 }, }); + event2.timestamp = fixedTimestamp; await handler(event2); // Verify the file was overwritten by second handler. diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index a8d4725cc09..329fec97b48 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -354,6 +354,8 @@ const saveSessionToMemory: HookHandler = async (event) => { // Use custom content from upstream hook if available, otherwise build entry. // hasCustomContent (set above) already gates session loading + slug generation. + // When blockPreSet is true, skip entry construction entirely — the inline + // write won't happen and the value would be discarded. let entry: string; if (hasCustomContent) { // An empty string is a valid redaction signal — hooks may intentionally @@ -362,7 +364,7 @@ const saveSessionToMemory: HookHandler = async (event) => { log.debug("Using custom session content from upstream hook", { length: entry.length, }); - } else { + } else if (!blockPreSet) { const entryParts = [ `# Session: ${dateStr} ${timeStr} UTC`, "", @@ -377,6 +379,8 @@ const saveSessionToMemory: HookHandler = async (event) => { } entry = entryParts.join("\n"); + } else { + entry = ""; // Block pre-set — writtenEntry will be null regardless. } // Write inline (fail-safe: if postHookActions never drains, the file @@ -387,7 +391,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // 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) { + if (blockPreSet) { log.debug("Session save blocked by upstream hook (inline check)"); } else { await fs.mkdir(memoryDir, { recursive: true }); From 45ec7ec67258010255a48c7305b57edca129fef9 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 09:31:24 -0700 Subject: [PATCH 29/45] fix: use vi.spyOn for Math.random, simplify postHookActions null-guards - Replace direct Math.random mutation with vi.spyOn(Math, 'random').mockReturnValue(0.5) for idiomatic Vitest cleanup integration - Fix comment: collision is driven by identical LLM slug, not timestamp fallback; Math.random pin is a backstop for null sessionContent edge case - Remove unnecessary nullish-coalescing and conditional guard on postHookActions (field is required in interface and always initialized by createInternalHookEvent) Addresses greptile review feedback for confidence score improvement. --- src/hooks/bundled/session-memory/handler.test.ts | 16 +++++++++------- src/hooks/internal-hooks.ts | 6 ++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index af95c00d41f..dc996ad6d89 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -660,12 +660,14 @@ describe("session-memory hook", () => { }); // Pin Math.random AND timestamp to force deterministic slug — both - // handler calls produce the same fallback filename, exercising the - // slug-collision restoration path (preExistingContent !== null). - // Without pinning the clock, a wall-clock second boundary between - // event1 and event2 would produce different HHMMSS prefixes → no collision. - const origRandom = Math.random; - Math.random = () => 0.5; + // handler calls produce the same LLM-generated slug ("simple-math" + // via the mock), exercising the slug-collision restoration path + // (preExistingContent !== null). Math.random is pinned as a backstop + // for the edge case where sessionContent is null and the LLM path + // never fires. Without pinning the clock, a wall-clock second + // boundary between event1 and event2 would produce different HHMMSS + // prefixes → no collision. + vi.spyOn(Math, "random").mockReturnValue(0.5); const fixedTimestamp = new Date("2024-01-15T12:34:56.000Z"); try { @@ -711,7 +713,7 @@ describe("session-memory hook", () => { expect(restoredContent).toContain("first session"); expect(restoredContent).not.toContain("second session"); } finally { - Math.random = origRandom; + vi.restoreAllMocks(); } }); diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index f7ac8c802a6..7099f9f7f3d 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -297,12 +297,10 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise Date: Mon, 9 Mar 2026 11:42:20 -0700 Subject: [PATCH 30/45] fix: add diagnostic for cleared sessionSaveContent, export drainPostHookActions - Add log.debug when a post-hook clears pre-set sessionSaveContent, making the silent no-op visible to plugin authors (symmetric with blockSessionSave cleared warning) - Extract drainPostHookActions as an exported utility from internal-hooks.ts so handler tests share the exact production drain semantics instead of maintaining a divergent copy - Update handler.test.ts to import and use the shared drain utility Addresses greptile review feedback for confidence score improvement. --- .../bundled/session-memory/handler.test.ts | 46 +++++++------------ src/hooks/bundled/session-memory/handler.ts | 16 +++++++ src/hooks/internal-hooks.ts | 36 ++++++++++++--- 3 files changed, 62 insertions(+), 36 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index dc996ad6d89..79e47ffba3b 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -573,24 +573,12 @@ describe("session-memory hook", () => { expect(memoryContent).toContain("assistant: Only message 2"); }); - // Helper to drain postHookActions with per-action error isolation, - // matching triggerInternalHook's actual drain behaviour. - async function drainPostHookActions(event: { - postHookActions: Array<() => Promise | void>; - }) { - // Snapshot before draining — matches triggerInternalHook's production - // semantics (prevents self-scheduling actions from executing in the - // same drain cycle). - const pending = [...event.postHookActions]; - // Clear source array so re-drain is a no-op (matches production). - event.postHookActions.length = 0; - for (const action of pending) { - try { - await action(); - } catch { - // Per-action isolation — one failure doesn't block others. - } - } + // Uses the exported drain utility from internal-hooks.ts so tests share + // the exact same drain semantics as production (snapshot → clear → sequential + // await → per-action error isolation). + async function drainActions(event: { postHookActions: Array<() => Promise | void> }) { + const { drainPostHookActions } = await import("../../internal-hooks.js"); + await drainPostHookActions(event.postHookActions); } it("blockSessionSave (pre-set) prevents memory file creation", async () => { @@ -610,7 +598,7 @@ describe("session-memory hook", () => { event.context.blockSessionSave = true; await handler(event); - await drainPostHookActions(event); + await drainActions(event); const memoryDir = path.join(tempDir, "memory"); const memoryFiles = await fs.readdir(memoryDir).catch(() => [] as string[]); @@ -643,7 +631,7 @@ describe("session-memory hook", () => { event.context.blockSessionSave = true; // Post-hook action retracts the file - await drainPostHookActions(event); + await drainActions(event); memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); expect(memoryFiles).toHaveLength(0); @@ -678,7 +666,7 @@ describe("session-memory hook", () => { }); event1.timestamp = fixedTimestamp; await handler(event1); - await drainPostHookActions(event1); + await drainActions(event1); const memoryDir = path.join(tempDir, "memory"); const files1 = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); @@ -707,7 +695,7 @@ describe("session-memory hook", () => { // Late-block: retraction should restore the FIRST session's content. event2.context.blockSessionSave = true; - await drainPostHookActions(event2); + await drainActions(event2); const restoredContent = await fs.readFile(collidingPath, "utf-8"); expect(restoredContent).toContain("first session"); @@ -734,7 +722,7 @@ describe("session-memory hook", () => { event.context.sessionSaveContent = "Custom summary from upstream hook"; await handler(event); - await drainPostHookActions(event); + await drainActions(event); const memoryDir = path.join(tempDir, "memory"); const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); @@ -766,7 +754,7 @@ describe("session-memory hook", () => { event.context.sessionSaveContent = "Redacted by policy"; // Post-hook action overwrites - await drainPostHookActions(event); + await drainActions(event); const memoryDir = path.join(tempDir, "memory"); const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); @@ -792,7 +780,7 @@ describe("session-memory hook", () => { event.context.sessionSaveContent = ""; await handler(event); - await drainPostHookActions(event); + await drainActions(event); const memoryDir = path.join(tempDir, "memory"); const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); @@ -861,7 +849,7 @@ describe("session-memory hook", () => { event.context.sessionSaveContent = "Replacement content from policy hook"; // Post-hook should create the directory and write the file - await drainPostHookActions(event); + await drainActions(event); const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); expect(files.length).toBeGreaterThan(0); @@ -887,7 +875,7 @@ describe("session-memory hook", () => { event.context.sessionSaveContent = "Should not appear"; await handler(event); - await drainPostHookActions(event); + await drainActions(event); const memoryDir = path.join(tempDir, "memory"); const memoryFiles = await fs.readdir(memoryDir).catch(() => [] as string[]); @@ -916,7 +904,7 @@ describe("session-memory hook", () => { event.context.blockSessionSave = true; event.context.sessionSaveContent = "Should not appear"; - await drainPostHookActions(event); + await drainActions(event); const memoryDir = path.join(tempDir, "memory"); const memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md")); @@ -947,7 +935,7 @@ describe("session-memory hook", () => { // Since the transcript was never loaded, no file can be produced. event.context.blockSessionSave = false; - await drainPostHookActions(event); + await drainActions(event); // No memory file should exist — the transcript was never loaded const memoryDir = path.join(tempDir, "memory"); diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 329fec97b48..1ccf017451d 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -495,6 +495,22 @@ const saveSessionToMemory: HookHandler = async (event) => { "execution. To write a file after clearing blockSessionSave, also " + "provide sessionSaveContent with the desired content.", ); + } else if ( + event.context.blockSessionSave !== true && + writtenEntry !== null && + typeof postContent !== "string" && + hasCustomContent + ) { + // sessionSaveContent was pre-set (inline write used custom content), + // then a later hook cleared it. The file retains the pre-set content. + // This is a no-op — to revert to transcript content, the clearing hook + // must provide its own sessionSaveContent. Log for diagnostics so + // plugin authors know their clearing was silently ignored. + log.debug( + "sessionSaveContent was cleared by a post-hook but the inline write " + + "already used the pre-set content. File retains pre-set content. " + + "To override, set sessionSaveContent to the desired replacement.", + ); } }); } catch (err) { diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 7099f9f7f3d..5ae1e127695 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -297,16 +297,38 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise { + const message = err instanceof Error ? err.message : String(err); + log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`); + }); +} + +/** + * Drain an array of post-hook action callbacks. + * + * Snapshots the array before iterating so that self-scheduling actions + * (ones that push new callbacks) do not execute in the same drain cycle. + * Clears the source array after snapshotting so re-draining is a no-op. + * Errors are caught per-action so one failure doesn't block others. + * + * Exported for use in tests that need to drain post-hook actions without + * going through the full triggerInternalHook pipeline, ensuring they + * share the same drain semantics as production. + * + * @param actions - The postHookActions array to drain (mutated: cleared after snapshot). + * @param onError - Optional per-action error handler. Defaults to swallowing errors silently. + */ +export async function drainPostHookActions( + actions: Array<() => Promise | void>, + onError?: (err: unknown) => void, +): Promise { + const pending = [...actions]; + actions.length = 0; + for (const action of pending) { try { await action(); } catch (err) { - const message = err instanceof Error ? err.message : String(err); - log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`); + onError?.(err); } } } From 65e0158bf86af2f32fa2ca8f5008fb192a5e2c32 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 12:15:25 -0700 Subject: [PATCH 31/45] fix: rethrow in test drain helper, fix slug comment, clarify drain JSDoc - drainActions test helper now rethrows errors so test failures surface the actual error instead of a confusing downstream assertion - Fix misleading comment: slug collision is via fallback timestamp+random path (LLM slug generation disabled in VITEST=true), not LLM mock - Clarify drainPostHookActions JSDoc: re-drain is only a no-op when no actions push new callbacks during execution; mid-drain pushes persist Addresses greptile review feedback for confidence score improvement. --- .../bundled/session-memory/handler.test.ts | 22 ++++++++++--------- src/hooks/internal-hooks.ts | 6 ++++- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 79e47ffba3b..61aa3e2205d 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -575,10 +575,13 @@ describe("session-memory hook", () => { // Uses the exported drain utility from internal-hooks.ts so tests share // the exact same drain semantics as production (snapshot → clear → sequential - // await → per-action error isolation). + // await). Errors are rethrown (rather than swallowed) so test failures surface + // the actual error message instead of a confusing downstream assertion failure. async function drainActions(event: { postHookActions: Array<() => Promise | void> }) { const { drainPostHookActions } = await import("../../internal-hooks.js"); - await drainPostHookActions(event.postHookActions); + await drainPostHookActions(event.postHookActions, (err) => { + throw err; + }); } it("blockSessionSave (pre-set) prevents memory file creation", async () => { @@ -647,14 +650,13 @@ describe("session-memory hook", () => { content: createMockSessionContent([{ role: "user", content: "first session" }]), }); - // Pin Math.random AND timestamp to force deterministic slug — both - // handler calls produce the same LLM-generated slug ("simple-math" - // via the mock), exercising the slug-collision restoration path - // (preExistingContent !== null). Math.random is pinned as a backstop - // for the edge case where sessionContent is null and the LLM path - // never fires. Without pinning the clock, a wall-clock second - // boundary between event1 and event2 would produce different HHMMSS - // prefixes → no collision. + // Pin Math.random AND timestamp to force deterministic fallback slug — + // both handler calls produce the same HHMMSS prefix (fixed timestamp) + // and the same random suffix (pinned Math.random). LLM slug generation + // is disabled in the test environment (VITEST=true), so the collision + // is exercised entirely through the fallback path. Without pinning + // the clock, a wall-clock second boundary between event1 and event2 + // would produce different HHMMSS prefixes → no collision. vi.spyOn(Math, "random").mockReturnValue(0.5); const fixedTimestamp = new Date("2024-01-15T12:34:56.000Z"); diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 5ae1e127695..6d2610c670b 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -308,7 +308,11 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise Date: Mon, 9 Mar 2026 12:31:10 -0700 Subject: [PATCH 32/45] fix: rethrow non-ENOENT in preExistingContent read, clarify drain JSDoc - preExistingContent try/catch now only swallows ENOENT; rethrows EACCES, EISDIR, etc. to prevent data-loss where late-block retraction would delete a file instead of restoring prior content - drainPostHookActions JSDoc clarifies that per-action isolation only holds when onError does not rethrow; callers wanting fail-fast (tests) can use a rethrowing onError intentionally Addresses greptile review: data-loss risk + docstring precision. --- src/hooks/bundled/session-memory/handler.ts | 12 +++++++++++- src/hooks/internal-hooks.ts | 5 ++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 1ccf017451d..2478770e26c 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -397,8 +397,18 @@ const saveSessionToMemory: HookHandler = async (event) => { await fs.mkdir(memoryDir, { recursive: true }); try { preExistingContent = await fs.readFile(memoryFilePath, "utf-8"); - } catch { + } catch (err: unknown) { // File doesn't exist yet — normal case, nothing to preserve. + // Rethrow non-ENOENT errors (EACCES, EISDIR, etc.) to avoid silently + // losing preExistingContent, which would cause late-block retraction + // to delete the file instead of restoring a prior session's history. + if ( + err instanceof Error && + "code" in err && + (err as NodeJS.ErrnoException).code !== "ENOENT" + ) { + throw err; + } } await writeFileWithinRoot({ rootDir: memoryDir, diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 6d2610c670b..8bcbb567aed 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -313,7 +313,10 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise Date: Mon, 9 Mar 2026 16:47:57 -0700 Subject: [PATCH 33/45] fix: use crypto.randomUUID for slug suffix, use blockPreSet variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace Math.random().toString(36) with crypto.randomUUID() for uniformly-distributed 4-char hex suffix — eliminates weak entropy from floating-point base-36 conversion edge cases - Use already-captured blockPreSet variable instead of re-reading context.blockSessionSave for consistency and clarity - Update test to mock crypto.randomUUID instead of Math.random Addresses greptile review: weak entropy risk + redundant context read. --- src/hooks/bundled/session-memory/handler.test.ts | 8 ++++---- src/hooks/bundled/session-memory/handler.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index 61aa3e2205d..e7c85d8f603 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -650,14 +650,14 @@ describe("session-memory hook", () => { content: createMockSessionContent([{ role: "user", content: "first session" }]), }); - // Pin Math.random AND timestamp to force deterministic fallback slug — + // Pin crypto.randomUUID AND timestamp to force deterministic fallback slug — // both handler calls produce the same HHMMSS prefix (fixed timestamp) - // and the same random suffix (pinned Math.random). LLM slug generation - // is disabled in the test environment (VITEST=true), so the collision + // and the same random suffix (pinned UUID). LLM slug generation is + // disabled in the test environment (VITEST=true), so the collision // is exercised entirely through the fallback path. Without pinning // the clock, a wall-clock second boundary between event1 and event2 // would produce different HHMMSS prefixes → no collision. - vi.spyOn(Math, "random").mockReturnValue(0.5); + vi.spyOn(crypto, "randomUUID").mockReturnValue("aaaa1111-2222-3333-4444-555566667777"); const fixedTimestamp = new Date("2024-01-15T12:34:56.000Z"); try { diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 2478770e26c..398786c8390 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -332,7 +332,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // one silently overwrites the earlier memory entry. if (!slug) { const timeSlug = now.toISOString().split("T")[1].split(".")[0].replace(/:/g, ""); - const rand = Math.random().toString(36).slice(2, 6).padEnd(4, "0"); + const rand = crypto.randomUUID().replace(/-/g, "").slice(0, 4); slug = `${timeSlug.slice(0, 6)}-${rand}`; log.debug("Using fallback timestamp slug", { slug }); } @@ -425,7 +425,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // Defer retraction/replacement to post-hook phase so that hooks // registered after this handler can set blockSessionSave or // sessionSaveContent and still have them honored. - const writtenEntry = context.blockSessionSave === true ? null : entry; + const writtenEntry = blockPreSet ? null : entry; // Post-hook callback — errors propagate to the framework's per-action // catch in triggerInternalHook, which provides consistent log formatting // and per-action isolation. From 9de1adbce2837b194994d18f5a1d851b50f3055a Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 16:59:40 -0700 Subject: [PATCH 34/45] fix: explicit inlineWriteHappened sentinel, diagnostic for dual pre-set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Introduce inlineWriteHappened boolean to decouple write-outcome from write-intent (blockPreSet) — makes retraction condition unambiguous - Add log.debug when both blockSessionSave and sessionSaveContent are pre-set simultaneously, surfacing that custom content is discarded (blockSessionSave takes precedence) Addresses greptile review: sentinel coupling + silent discard of dual pre-set sessionSaveContent. --- src/hooks/bundled/session-memory/handler.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 398786c8390..d9dd26fd4d6 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -392,7 +392,14 @@ const saveSessionToMemory: HookHandler = async (event) => { // erasure of prior memory files when LLM slugs collide on the same day. let preExistingContent: string | null = null; if (blockPreSet) { - log.debug("Session save blocked by upstream hook (inline check)"); + if (hasCustomContent) { + log.debug( + "blockSessionSave pre-set — sessionSaveContent was also set but will be ignored " + + "(blockSessionSave takes precedence over sessionSaveContent)", + ); + } else { + log.debug("Session save blocked by upstream hook (inline check)"); + } } else { await fs.mkdir(memoryDir, { recursive: true }); try { @@ -425,7 +432,8 @@ const saveSessionToMemory: HookHandler = async (event) => { // Defer retraction/replacement to post-hook phase so that hooks // registered after this handler can set blockSessionSave or // sessionSaveContent and still have them honored. - const writtenEntry = blockPreSet ? null : entry; + const inlineWriteHappened = !blockPreSet; + const writtenEntry = inlineWriteHappened ? entry : null; // Post-hook callback — errors propagate to the framework's per-action // catch in triggerInternalHook, which provides consistent log formatting // and per-action isolation. @@ -433,7 +441,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // 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) { + if (event.context.blockSessionSave === true && inlineWriteHappened) { if (preExistingContent !== null) { // Slug collision: another entry already existed at this filename // before our inline write. Restore the original content rather From cca4fde3d21b906ac1658de22ce64608a4e16ea6 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 22:22:21 -0700 Subject: [PATCH 35/45] fix: warn on late-set blockSessionSave privacy boundary, consistent error narrowing - Add log.warn when blockSessionSave is late-set: file is retracted but transcript may have already been sent to LLM for slug generation. Surfaces the privacy boundary so plugin authors know to pre-set for full confidentiality. - Align fs.unlink catch to use same defensive err instanceof Error && 'code' in err pattern as the fs.readFile catch above. Addresses greptile review: privacy boundary documentation + error narrowing consistency. --- src/hooks/bundled/session-memory/handler.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index d9dd26fd4d6..4c67f31d5bf 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -442,6 +442,16 @@ const saveSessionToMemory: HookHandler = async (event) => { // 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 && inlineWriteHappened) { + // Privacy note: late-set blockSessionSave retracts the file but does NOT + // prevent transcript content from having already been sent to the LLM + // provider for slug generation. To prevent transcript processing entirely, + // set blockSessionSave before the session-memory handler runs (pre-set path). + log.warn( + "blockSessionSave was set by a late hook — memory file will be retracted, but " + + "transcript content may have already been sent to the LLM provider for slug generation. " + + "To prevent transcript processing entirely, set blockSessionSave before the " + + "session-memory handler runs.", + ); if (preExistingContent !== null) { // Slug collision: another entry already existed at this filename // before our inline write. Restore the original content rather @@ -467,7 +477,11 @@ const saveSessionToMemory: HookHandler = async (event) => { // triggerInternalHook logs them. Note: errors are caught // per-action and do NOT propagate to the session caller; // the file may remain on disk under adversarial FS conditions. - if ((err as NodeJS.ErrnoException).code !== "ENOENT") { + if ( + !(err instanceof Error) || + !("code" in err) || + (err as NodeJS.ErrnoException).code !== "ENOENT" + ) { throw err; } } From b17165f982c1b51f8784a206ecf2a6be46685722 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 22:52:13 -0700 Subject: [PATCH 36/45] fix: split dual-purpose writtenEntry comparison, require onError in drain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Split postContent !== writtenEntry into explicit (writtenEntry === null || postContent !== writtenEntry) — makes the two intents unambiguous: write if no inline write happened OR if content changed - Make onError required in drainPostHookActions — forces callers to be explicit about error handling (logger in production, rethrower in tests) instead of silently swallowing by default Addresses greptile review: comparison clarity + silent error swallow. --- src/hooks/bundled/session-memory/handler.ts | 4 +++- src/hooks/internal-hooks.ts | 8 +++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 4c67f31d5bf..3057d78f170 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -496,7 +496,9 @@ const saveSessionToMemory: HookHandler = async (event) => { if ( event.context.blockSessionSave !== true && typeof postContent === "string" && - postContent !== writtenEntry + // Two distinct intents: write if no inline write happened (writtenEntry + // is null because blockPreSet was true) OR if the content changed. + (writtenEntry === null || postContent !== writtenEntry) ) { // Ensure memoryDir exists — the inline write may have been // skipped (e.g. blockSessionSave was true initially) so mkdir diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 8bcbb567aed..522406e5d7a 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -323,11 +323,13 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise {}` only when intentionally swallowing errors. */ export async function drainPostHookActions( actions: Array<() => Promise | void>, - onError?: (err: unknown) => void, + onError: (err: unknown) => void, ): Promise { const pending = [...actions]; actions.length = 0; @@ -335,7 +337,7 @@ export async function drainPostHookActions( try { await action(); } catch (err) { - onError?.(err); + onError(err); } } } From 8ccb58796595c8be073a9e525a44a21bc0b1d69d Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 23:04:26 -0700 Subject: [PATCH 37/45] fix: gate privacy warning on !hasCustomContent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The late-set blockSessionSave privacy warning incorrectly fired when custom content was pre-set (transcript was never loaded or sent to LLM). Now only warns when transcript was actually loaded — matching the condition that guards transcript loading at line 304. --- src/hooks/bundled/session-memory/handler.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 3057d78f170..6aeb0ddcdda 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -444,14 +444,17 @@ const saveSessionToMemory: HookHandler = async (event) => { if (event.context.blockSessionSave === true && inlineWriteHappened) { // Privacy note: late-set blockSessionSave retracts the file but does NOT // prevent transcript content from having already been sent to the LLM - // provider for slug generation. To prevent transcript processing entirely, - // set blockSessionSave before the session-memory handler runs (pre-set path). - log.warn( - "blockSessionSave was set by a late hook — memory file will be retracted, but " + - "transcript content may have already been sent to the LLM provider for slug generation. " + - "To prevent transcript processing entirely, set blockSessionSave before the " + - "session-memory handler runs.", - ); + // provider for slug generation — but only when the transcript was actually + // loaded (i.e. no custom content was pre-set). When hasCustomContent is + // true, transcript loading and LLM calls were skipped entirely. + if (!hasCustomContent) { + log.warn( + "blockSessionSave was set by a late hook — memory file will be retracted, but " + + "transcript content may have already been sent to the LLM provider for slug generation. " + + "To prevent transcript processing entirely, set blockSessionSave before the " + + "session-memory handler runs.", + ); + } if (preExistingContent !== null) { // Slug collision: another entry already existed at this filename // before our inline write. Restore the original content rather From d9cce17a751661efe4fc35ebbe741be75429ec70 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 23:15:47 -0700 Subject: [PATCH 38/45] fix: check blockPreSet before hasCustomContent to prevent misleading log MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restructured entry construction to test blockPreSet first — eliminates the misleading 'Using custom session content' debug log that fired before the 'will be ignored' correction when both flags were pre-set. Removed redundant duplicate blockPreSet check in the write-decision block. --- src/hooks/bundled/session-memory/handler.ts | 27 ++++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 6aeb0ddcdda..5b89d0c5b13 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -357,14 +357,26 @@ const saveSessionToMemory: HookHandler = async (event) => { // When blockPreSet is true, skip entry construction entirely — the inline // write won't happen and the value would be discarded. let entry: string; - if (hasCustomContent) { + if (blockPreSet) { + // Block takes precedence — skip entry construction entirely since the + // inline write won't happen and the value would be discarded. + entry = ""; + if (hasCustomContent) { + log.debug( + "blockSessionSave pre-set — sessionSaveContent was also set but will be ignored " + + "(blockSessionSave takes precedence over sessionSaveContent)", + ); + } else { + log.debug("Session save blocked by upstream hook (inline check)"); + } + } else if (hasCustomContent) { // An empty string is a valid redaction signal — hooks may intentionally // set it to persist a blank marker while avoiding transcript retention. entry = context.sessionSaveContent as string; log.debug("Using custom session content from upstream hook", { length: entry.length, }); - } else if (!blockPreSet) { + } else { const entryParts = [ `# Session: ${dateStr} ${timeStr} UTC`, "", @@ -379,8 +391,6 @@ const saveSessionToMemory: HookHandler = async (event) => { } entry = entryParts.join("\n"); - } else { - entry = ""; // Block pre-set — writtenEntry will be null regardless. } // Write inline (fail-safe: if postHookActions never drains, the file @@ -392,14 +402,7 @@ const saveSessionToMemory: HookHandler = async (event) => { // erasure of prior memory files when LLM slugs collide on the same day. let preExistingContent: string | null = null; if (blockPreSet) { - if (hasCustomContent) { - log.debug( - "blockSessionSave pre-set — sessionSaveContent was also set but will be ignored " + - "(blockSessionSave takes precedence over sessionSaveContent)", - ); - } else { - log.debug("Session save blocked by upstream hook (inline check)"); - } + // Already logged above — nothing to write. } else { await fs.mkdir(memoryDir, { recursive: true }); try { From 2ae019c330485155a1f48e75de4d5deaeb85b734 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 23:21:03 -0700 Subject: [PATCH 39/45] fix: verify own write before retraction to prevent concurrent save clobber MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The retraction path now reads current file content and compares it to writtenEntry before restoring preExistingContent or unlinking. If the content differs (another session wrote to the same file concurrently), retraction is skipped with a warning — preventing stale preExistingContent from overwriting a newer concurrent save. Also removes the now-redundant ENOENT catch in the unlink path since the pre-read already handles missing files. --- src/hooks/bundled/session-memory/handler.ts | 51 +++++++++++++-------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 5b89d0c5b13..b09736af70e 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -458,6 +458,37 @@ const saveSessionToMemory: HookHandler = async (event) => { "session-memory handler runs.", ); } + // Verify we're reverting our own write before touching the file. + // A concurrent session (e.g. /new, /reset) may have written to the + // same filename between our inline write and this post-hook drain. + // If the current content doesn't match what we wrote, skip retraction + // to avoid clobbering the other session's data. + let currentContent: string | null = null; + try { + currentContent = await fs.readFile(memoryFilePath, "utf-8"); + } catch (err: unknown) { + if ( + err instanceof Error && + "code" in err && + (err as NodeJS.ErrnoException).code === "ENOENT" + ) { + // File was externally deleted — nothing to retract. + log.debug("Session save retraction skipped — file already removed"); + return; + } + throw err; + } + + if (currentContent !== writtenEntry) { + // File content differs from what we wrote — another session has + // written to this file since our inline write. Do not clobber. + log.warn( + "Session save retraction skipped — file was modified by another " + + "session since our inline write (concurrent save detected)", + ); + return; + } + if (preExistingContent !== null) { // Slug collision: another entry already existed at this filename // before our inline write. Restore the original content rather @@ -473,24 +504,8 @@ const saveSessionToMemory: HookHandler = async (event) => { }); log.debug("Session save retracted by post-hook — pre-existing file restored"); } else { - try { - await fs.unlink(memoryFilePath); - log.debug("Session save retracted by post-hook (blockSessionSave)"); - } catch (err) { - // ENOENT can occur if the file was externally deleted between - // the inline write and the post-hook drain — not a concern. - // Re-throw non-ENOENT errors (e.g. EACCES, EROFS) so - // triggerInternalHook logs them. Note: errors are caught - // per-action and do NOT propagate to the session caller; - // the file may remain on disk under adversarial FS conditions. - if ( - !(err instanceof Error) || - !("code" in err) || - (err as NodeJS.ErrnoException).code !== "ENOENT" - ) { - throw err; - } - } + await fs.unlink(memoryFilePath); + log.debug("Session save retracted by post-hook (blockSessionSave)"); } return; } From 9744fa0b06edcc29a77cda1ac063465521ea96d9 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 23:24:28 -0700 Subject: [PATCH 40/45] fix: default postHookActions to [] for legacy callers Legacy callers constructing hook events without postHookActions would hit a TypeError when drainPostHookActions spreads undefined. Now defaults to empty array at trigger time for backwards compatibility. --- src/hooks/internal-hooks.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index 522406e5d7a..fcc46806132 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -297,7 +297,10 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise { + // Default to empty array for legacy callers that construct hook events + // without the postHookActions field (pre-dates this PR's event shape change). + const actions = event.postHookActions ?? []; + await drainPostHookActions(actions, (err) => { const message = err instanceof Error ? err.message : String(err); log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`); }); From e034064ee8e8ca3443050dad06645e306ca4029e Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 23:40:36 -0700 Subject: [PATCH 41/45] fix: add explicit crypto import, tighten privacy warning to sessionContent check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Import crypto from 'node:crypto' for consistency with codebase conventions (every other file uses explicit import, not global) - Tighten late-block privacy warning to only fire when sessionContent was actually loaded (non-null) — prevents misleading warning when no transcript was ever read from disk or sent to LLM - Add matching crypto import in test file so vi.spyOn mocks the correct module reference --- src/hooks/bundled/session-memory/handler.test.ts | 1 + src/hooks/bundled/session-memory/handler.ts | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/hooks/bundled/session-memory/handler.test.ts b/src/hooks/bundled/session-memory/handler.test.ts index e7c85d8f603..a94efde7c3b 100644 --- a/src/hooks/bundled/session-memory/handler.test.ts +++ b/src/hooks/bundled/session-memory/handler.test.ts @@ -1,3 +1,4 @@ +import crypto from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index b09736af70e..b4b21a15df7 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -5,6 +5,7 @@ * Creates a new dated memory file with LLM-generated slug */ +import crypto from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -450,7 +451,10 @@ const saveSessionToMemory: HookHandler = async (event) => { // provider for slug generation — but only when the transcript was actually // loaded (i.e. no custom content was pre-set). When hasCustomContent is // true, transcript loading and LLM calls were skipped entirely. - if (!hasCustomContent) { + if (!hasCustomContent && sessionContent) { + // Only warn when transcript was actually loaded and potentially + // sent to the LLM for slug generation. When sessionFile was null + // or sessionContent failed to load, no data left the device. log.warn( "blockSessionSave was set by a late hook — memory file will be retracted, but " + "transcript content may have already been sent to the LLM provider for slug generation. " + From b6583a379d869cc0bf79f9ac9fe52ceee645128f Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 23:50:44 -0700 Subject: [PATCH 42/45] fix: use crypto.randomUUID() for slug generator sessionId/runId Replace Date.now() with crypto.randomUUID() for collision-proof identifiers under concurrent slug-generation calls. Two separate Date.now() calls could resolve to the same millisecond in burst scenarios, causing shared sessionId between concurrent runs. --- src/hooks/llm-slug-generator.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hooks/llm-slug-generator.ts b/src/hooks/llm-slug-generator.ts index cbfeaf032cf..dd562642ebd 100644 --- a/src/hooks/llm-slug-generator.ts +++ b/src/hooks/llm-slug-generator.ts @@ -2,6 +2,7 @@ * LLM-based slug generator for session memory filenames */ +import crypto from "node:crypto"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; @@ -59,7 +60,7 @@ Reply with ONLY the slug, nothing else. Examples: "vendor-pitch", "api-design", // text is extracted. Slug generation is pure text — it never needs // tool access. const result = await runEmbeddedPiAgent({ - sessionId: `slug-generator-${Date.now()}`, + sessionId: `slug-generator-${crypto.randomUUID()}`, sessionKey: "temp:slug-generator", agentId, sessionFile: tempSessionFile, @@ -71,7 +72,7 @@ Reply with ONLY the slug, nothing else. Examples: "vendor-pitch", "api-design", model, disableTools: true, timeoutMs: 15_000, // 15 second timeout - runId: `slug-gen-${Date.now()}`, + runId: `slug-gen-${crypto.randomUUID()}`, }); // Extract text from payloads From 28f44b1736b0aae252ff2366ab6cdcfffa58d626 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 23:56:33 -0700 Subject: [PATCH 43/45] fix: add TOCTOU ownership check to post-hook content overwrite path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sessionSaveContent replacement path now verifies the file still contains our writtenEntry before overwriting — same guard as the late-block retraction path. Prevents concurrent runs with the same filename from clobbering each other's valid saves. --- src/hooks/bundled/session-memory/handler.ts | 28 +++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index b4b21a15df7..60043293e84 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -525,6 +525,34 @@ const saveSessionToMemory: HookHandler = async (event) => { // is null because blockPreSet was true) OR if the content changed. (writtenEntry === null || postContent !== writtenEntry) ) { + // Verify ownership before overwriting — if another concurrent run wrote + // to the same file since our inline write, do not clobber their content. + // Same TOCTOU guard as the late-block retraction path. + if (writtenEntry !== null) { + let currentContent: string | null = null; + try { + currentContent = await fs.readFile(memoryFilePath, "utf-8"); + } catch (err: unknown) { + if ( + err instanceof Error && + "code" in err && + (err as NodeJS.ErrnoException).code === "ENOENT" + ) { + // File was externally deleted — safe to recreate with new content. + currentContent = null; + } else { + throw err; + } + } + if (currentContent !== null && currentContent !== writtenEntry) { + log.warn( + "Session save content replacement skipped — file was modified by another " + + "session since our inline write (concurrent save detected)", + ); + return; + } + } + // Ensure memoryDir exists — the inline write may have been // skipped (e.g. blockSessionSave was true initially) so mkdir // might never have run. From c0792e53761a2b4737dd3d9e5d7459ddba553e71 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Tue, 10 Mar 2026 00:19:54 -0700 Subject: [PATCH 44/45] fix: restore pre-existing content on ENOENT during late-block retraction When a slug collision exists (preExistingContent !== null) and the file is externally deleted before the post-hook drain, the ENOENT handler now restores the prior session's content instead of returning early. The inline overwrite already destroyed the original file, so early return would permanently lose the prior session's memory. --- src/hooks/bundled/session-memory/handler.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/hooks/bundled/session-memory/handler.ts b/src/hooks/bundled/session-memory/handler.ts index 60043293e84..1a8fb597e05 100644 --- a/src/hooks/bundled/session-memory/handler.ts +++ b/src/hooks/bundled/session-memory/handler.ts @@ -476,8 +476,23 @@ const saveSessionToMemory: HookHandler = async (event) => { "code" in err && (err as NodeJS.ErrnoException).code === "ENOENT" ) { - // File was externally deleted — nothing to retract. - log.debug("Session save retraction skipped — file already removed"); + if (preExistingContent !== null) { + // Our inline write overwrote a pre-existing entry (slug collision), + // and the file was subsequently deleted externally. Restore the + // prior session's content — it was lost to our inline overwrite. + await writeFileWithinRoot({ + rootDir: memoryDir, + relativePath: filename, + data: preExistingContent, + encoding: "utf-8", + }); + log.debug( + "Session save retracted by post-hook — pre-existing file restored after external deletion", + ); + } else { + // No prior content existed — file was externally deleted, nothing to restore. + log.debug("Session save retraction skipped — file already removed"); + } return; } throw err; From 79c06d71607b087e7b230fc6517d849d935f3268 Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Tue, 10 Mar 2026 00:21:30 -0700 Subject: [PATCH 45/45] fix: initialize postHookActions before handler loop, not just at drain Move postHookActions ??= [] to before the handler loop so handlers can safely push() during execution. Legacy callers without the field would otherwise TypeError on .push() inside the session-memory handler. Removed the now-redundant ?? [] at drain time. --- src/hooks/internal-hooks.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index fcc46806132..de3a7a79c0a 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -274,6 +274,11 @@ export function getRegisteredEventKeys(): string[] { * @param event - The event to trigger */ export async function triggerInternalHook(event: InternalHookEvent): Promise { + // Normalize postHookActions before entering the handler loop — handlers + // may push to this array during execution. Legacy callers that construct + // hook events without the field would otherwise hit a TypeError on .push(). + event.postHookActions ??= []; + const typeHandlers = handlers.get(event.type) ?? []; const specificHandlers = handlers.get(`${event.type}:${event.action}`) ?? []; @@ -297,10 +302,7 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise { + await drainPostHookActions(event.postHookActions, (err) => { const message = err instanceof Error ? err.message : String(err); log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`); });