From 8d7bd9e461e6cfbc27d9474a35f6dc32f1eba0ba Mon Sep 17 00:00:00 2001 From: zeroaltitude Date: Mon, 9 Mar 2026 00:48:41 -0700 Subject: [PATCH] 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 });