fix(test): mock fs dependencies instead of self-module spy and add afterEach cleanup

Author: potb

src/shared/session-utils.test.ts

@@ -1,62 +1,96 @@
-import { describe, test, expect, beforeEach, mock, spyOn } from "bun:test"
+import { describe, test, expect, beforeEach, afterEach, spyOn, mock } from "bun:test"
+import * as fs from "node:fs"
 import { initializeAgentNameAliases, resetAgentNameAliases } from "./agent-name-aliases"
+
+//#given mocked fs module to control getMessageDir behavior
+mock.module("node:fs", () => ({
+  existsSync: mock(() => true),
+  readdirSync: mock(() => []),
+}))
+
 import * as sessionUtils from "./session-utils"
 
 describe("session-utils", () => {
-  let getMessageDirSpy: ReturnType<typeof spyOn>
-  let findNearestSpy: ReturnType<typeof mock>
+  //#given mocked hook-message-injector dependencies
+  //#when testing isCallerOrchestrator
+  //#then all tests should pass with proper cleanup
+
+  let injectorSpy: ReturnType<typeof spyOn> | null = null
 
   beforeEach(() => {
     resetAgentNameAliases()
-    getMessageDirSpy = spyOn(sessionUtils, "getMessageDir").mockReturnValue("/tmp/fake-dir")
+  })
+
+  afterEach(() => {
+    //#then restore all mocks to prevent test pollution
+    if (injectorSpy) {
+      injectorSpy.mockRestore()
+      injectorSpy = null
+    }
   })
 
   test("returns false when sessionID is undefined", () => {
+    //#when sessionID is undefined
+    //#then isCallerOrchestrator returns false
     expect(sessionUtils.isCallerOrchestrator(undefined)).toBe(false)
   })
 
   test("returns false when no message directory exists", () => {
-    getMessageDirSpy.mockReturnValue(null)
+    //#given getMessageDir returns null (no message directory found)
+    //#when calling isCallerOrchestrator with a sessionID
+    //#then it returns false
+    const injectorModule = require("../features/hook-message-injector")
+    injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue(null)
     expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(false)
   })
 
   test("returns true when nearest message agent is atlas", () => {
-    const injector = require("../features/hook-message-injector")
-    spyOn(injector, "findNearestMessageWithFields").mockReturnValue({ agent: "atlas" })
-
+    //#given findNearestMessageWithFields returns an object with agent: "atlas"
+    const injectorModule = require("../features/hook-message-injector")
+    injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "atlas" })
+    //#when calling isCallerOrchestrator
+    //#then it returns true
     expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(true)
   })
 
   test("returns true for Atlas with different casing", () => {
-    const injector = require("../features/hook-message-injector")
-    spyOn(injector, "findNearestMessageWithFields").mockReturnValue({ agent: "Atlas" })
-
+    //#given findNearestMessageWithFields returns an object with agent: "Atlas" (uppercase)
+    const injectorModule = require("../features/hook-message-injector")
+    injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "Atlas" })
+    //#when calling isCallerOrchestrator
+    //#then it returns true (case-insensitive comparison)
     expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(true)
   })
 
   test("returns false when nearest message agent is not atlas", () => {
-    const injector = require("../features/hook-message-injector")
-    spyOn(injector, "findNearestMessageWithFields").mockReturnValue({ agent: "sisyphus" })
-
+    //#given findNearestMessageWithFields returns an object with agent: "sisyphus"
+    const injectorModule = require("../features/hook-message-injector")
+    injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "sisyphus" })
+    //#when calling isCallerOrchestrator
+    //#then it returns false
     expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(false)
   })
 
   test("canonicalizes renamed atlas agent when checking orchestrator", () => {
+    //#given agent name aliases are initialized with atlas -> "Master Orchestrator"
     initializeAgentNameAliases(
       { atlas: "Master Orchestrator" },
       ["atlas", "sisyphus", "prometheus"],
     )
-
-    const injector = require("../features/hook-message-injector")
-    spyOn(injector, "findNearestMessageWithFields").mockReturnValue({ agent: "Master Orchestrator" })
-
+    //#and findNearestMessageWithFields returns the renamed agent name
+    const injectorModule = require("../features/hook-message-injector")
+    injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({ agent: "Master Orchestrator" })
+    //#when calling isCallerOrchestrator
+    //#then it canonicalizes the name and returns true
     expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(true)
   })
 
   test("returns false when agent field is missing from nearest message", () => {
-    const injector = require("../features/hook-message-injector")
-    spyOn(injector, "findNearestMessageWithFields").mockReturnValue({})
-
+    //#given findNearestMessageWithFields returns an object without agent field
+    const injectorModule = require("../features/hook-message-injector")
+    injectorSpy = spyOn(injectorModule, "findNearestMessageWithFields").mockReturnValue({})
+    //#when calling isCallerOrchestrator
+    //#then it returns false (agent defaults to empty string, not "atlas")
     expect(sessionUtils.isCallerOrchestrator("ses_test")).toBe(false)
   })
 })