From b17165f982c1b51f8784a206ecf2a6be46685722 Mon Sep 17 00:00:00 2001
From: zeroaltitude <zeroaltitude@gmail.com>
Date: Mon, 9 Mar 2026 22:52:13 -0700
Subject: [PATCH] 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<voi
  * 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.
+ * @param onError - Per-action error handler. Required to force callers to be explicit
+ *   about error handling — pass a logger in production, a rethrower in tests, or
+ *   `() => {}` only when intentionally swallowing errors.
  */
 export async function drainPostHookActions(
   actions: Array<() => Promise<void> | void>,
-  onError?: (err: unknown) => void,
+  onError: (err: unknown) => void,
 ): Promise<void> {
   const pending = [...actions];
   actions.length = 0;
@@ -335,7 +337,7 @@ export async function drainPostHookActions(
     try {
       await action();
     } catch (err) {
-      onError?.(err);
+      onError(err);
     }
   }
 }