From 45ec7ec67258010255a48c7305b57edca129fef9 Mon Sep 17 00:00:00 2001
From: zeroaltitude <zeroaltitude@gmail.com>
Date: Mon, 9 Mar 2026 09:31:24 -0700
Subject: [PATCH] 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<voi
   // callbacks do not execute in this drain cycle. Without this, a self-
   // scheduling action (one that pushes another action) could loop infinitely
   // because Array's for...of iterator is live and re-reads length each step.
-  const pendingActions = [...(event.postHookActions ?? [])];
+  const pendingActions = [...event.postHookActions];
   // Clear the source array so re-draining the same event is a no-op.
   // Without this, passing an event twice would re-execute every action.
-  if (event.postHookActions) {
-    event.postHookActions.length = 0;
-  }
+  event.postHookActions.length = 0;
   for (const action of pendingActions) {
     try {
       await action();