From 2ae019c330485155a1f48e75de4d5deaeb85b734 Mon Sep 17 00:00:00 2001
From: zeroaltitude <zeroaltitude@gmail.com>
Date: Mon, 9 Mar 2026 23:21:03 -0700
Subject: [PATCH] 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;
       }