From bfa49e8e749551d0ebd6717e250a8b197c165a6c Mon Sep 17 00:00:00 2001
From: zeroaltitude <zeroaltitude@gmail.com>
Date: Mon, 9 Mar 2026 12:31:10 -0700
Subject: [PATCH] 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<voi
  * *during* its own execution, those entries land in the already-cleared source
  * array and will persist after this function returns — a subsequent drain
  * call will execute them.
- * Errors are caught per-action so one failure doesn't block others.
+ * Errors are caught per-action via try/catch.  Per-action isolation (one
+ * failure doesn't block others) holds only when `onError` does not rethrow.
+ * If `onError` rethrows, subsequent actions are skipped — callers that want
+ * fail-fast semantics (e.g. tests) can use this intentionally.
  *
  * Exported for use in tests that need to drain post-hook actions without
  * going through the full triggerInternalHook pipeline, ensuring they