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.
This commit is contained in:
zeroaltitude 2026-03-09 09:31:24 -07:00
parent 8d7bd9e461
commit 45ec7ec672
No known key found for this signature in database
GPG Key ID: 77592FB1C703882E
2 changed files with 11 additions and 11 deletions

View File

@ -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();
}
});

View File

@ -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();