fix(core): guard searchProjectContent and listSessions against readdir TOCTOU

.husky/pre-push

@@ -26,9 +26,15 @@ if [ "$has_code_push" = false ]; then
 fi
 
 # run_quiet: silent on success, full output on failure
+# NOTE: husky runs this script under `sh -e` (errexit). `out=$("$@" 2>&1)` alone
+# inherits the wrapped command's exit code, so a failing command kills the
+# script on that assignment line before `status=$?`/`echo "$out"` ever run —
+# swallowing the diagnostic output entirely. `|| status=$?` keeps the failing
+# command from being the last element of the statement, so errexit does not
+# fire here, while `$?` is still captured accurately.
 run_quiet() {
-  out=$("$@" 2>&1)
-  status=$?
+  status=0
+  out=$("$@" 2>&1) || status=$?
   if [ $status -ne 0 ]; then
     echo "$out"
   fi

packages/core/src/paths.integration.test.ts

@@ -18,6 +18,10 @@ describe('getRealPathFromSession (mocked)', () => {
     vi.clearAllMocks()
   })
 
+  // Flaky under load: the dynamic re-import below occasionally exceeds vitest's
+  // default 5000ms test timeout on a cold module cache (observed independent of
+  // this test's own logic, on unmodified main). Raise the timeout rather than
+  // changing behavior.
   it('returns null for non-existent project', async () => {
     vi.mocked(fs.readdirSync).mockImplementation(() => {
       throw new Error('ENOENT')
@@ -27,7 +31,7 @@ describe('getRealPathFromSession (mocked)', () => {
     const { getRealPathFromSession } = await import('./paths.js')
     const result = getRealPathFromSession('-nonexistent-project')
     expect(result).toBeNull()
-  })
+  }, 15000)
 
   it('returns real cwd from session file when cwd matches folder name', async () => {
     const mockCwd = '/home/user/example.com'

packages/core/src/session/search.test.ts

@@ -4,6 +4,16 @@ import * as path from 'node:path'
 import * as os from 'node:os'
 import { Effect } from 'effect'
 
+vi.mock('node:fs/promises', async () => {
+  const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises')
+  const readdirMock = vi.fn(actual.readdir)
+  return {
+    ...actual,
+    readdir: readdirMock,
+    default: { ...actual, readdir: readdirMock },
+  }
+})
+
 vi.mock('../paths.js', async () => {
   const actual = await vi.importActual<typeof import('../paths.js')>('../paths.js')
   return {
@@ -135,6 +145,10 @@ describe('searchSessions', () => {
   let tempDir: string
 
   beforeEach(async () => {
+    // Restore readdir to the real implementation; individual tests may override.
+    const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises')
+    vi.mocked(fs.readdir).mockImplementation(actual.readdir as typeof fs.readdir)
+
     tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'search-test-'))
     vi.mocked(getSessionsDir).mockReturnValue(tempDir)
   })
@@ -260,4 +274,147 @@ describe('searchSessions', () => {
     const results = await Effect.runPromise(searchSessions('needle'))
     expect(results).toEqual([])
   })
+
+  // searchProjectContent TOCTOU guard — a project's folder may vanish between
+  // listProjects() returning it and searchProjectContent's own readdir (cross-PC
+  // sync, manual deletion). Mirrors listProjects' own guard, see Issue #103.
+  describe('searchProjectContent TOCTOU safety', () => {
+    function makeEnoent(p: string): NodeJS.ErrnoException {
+      const err = new Error(
+        `ENOENT: no such file or directory, scandir '${p}'`
+      ) as NodeJS.ErrnoException
+      err.code = 'ENOENT'
+      return err
+    }
+
+    it("skips a project whose folder vanishes mid-content-search, keeping other projects' results", async () => {
+      const projGone = '-Users-test-search-gone'
+      const projKeep = '-Users-test-search-keep'
+      await writeSessionWithDistinctTitleAndContent(
+        projGone,
+        'sess-gone',
+        'plain title',
+        'this has NEEDLE inside'
+      )
+      await writeSessionWithDistinctTitleAndContent(
+        projKeep,
+        'sess-keep',
+        'plain title',
+        'this also has NEEDLE inside'
+      )
+
+      // listProjects() also readdirs each project folder (for sessionCount) before
+      // searchProjectContent gets a turn. Succeed on that first call so projGone
+      // survives into targetProjects, then fail with ENOENT on the *next* readdir
+      // for the same path — simulating the folder vanishing between the two reads
+      // (the actual TOCTOU window this guard protects, not a same-instant miss).
+      const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises')
+      let goneCallCount = 0
+      vi.mocked(fs.readdir).mockImplementation(((p: unknown, opts: unknown) => {
+        if (typeof p === 'string' && p.endsWith(projGone)) {
+          goneCallCount += 1
+          if (goneCallCount > 1) {
+            return Promise.reject(makeEnoent(p))
+          }
+        }
+        return (actual.readdir as typeof fs.readdir)(
+          p as Parameters<typeof fs.readdir>[0],
+          opts as Parameters<typeof fs.readdir>[1]
+        )
+      }) as typeof fs.readdir)
+
+      // Before fix: the whole searchSessions Effect would die on projGone's ENOENT.
+      // After fix: projGone is skipped, projKeep's content match still returned.
+      const results = await Effect.runPromise(searchSessions('needle', { searchContent: true }))
+      const ids = results.map((r) => r.sessionId)
+      expect(ids).toContain('sess-keep')
+      expect(ids).not.toContain('sess-gone')
+    })
+
+    it('propagates a non-ENOENT readdir error (EACCES) instead of silently skipping', async () => {
+      const projDenied = '-Users-test-search-eacces'
+      await writeSessionWithDistinctTitleAndContent(
+        projDenied,
+        'sess-denied',
+        'plain title',
+        'this has NEEDLE inside'
+      )
+
+      // Same call-count trick as above, but this project sees *three* readdir
+      // calls before searchProjectContent's own turn: (1) listProjects' own
+      // readdir (for sessionCount), (2) Phase 1 title search's listSessions()
+      // readdir, (3) searchProjectContent's own readdir. Let the first two
+      // succeed so projDenied reaches Phase 2, then fail with EACCES
+      // specifically on call 3 — isolating this test to *searchProjectContent's*
+      // non-ENOENT passthrough. (Failing at call 2 instead would have the
+      // title-search phase's own catchAll re-throw the EACCES and abort
+      // Effect.all before Phase 2 is ever entered — see the dedicated
+      // "searchSessions title-search TOCTOU safety" block below, which
+      // exercises exactly that call-2 case.)
+      const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises')
+      let deniedCallCount = 0
+      vi.mocked(fs.readdir).mockImplementation(((p: unknown, opts: unknown) => {
+        if (typeof p === 'string' && p.endsWith(projDenied)) {
+          deniedCallCount += 1
+          if (deniedCallCount > 2) {
+            const err = new Error(
+              `EACCES: permission denied, scandir '${p}'`
+            ) as NodeJS.ErrnoException
+            err.code = 'EACCES'
+            return Promise.reject(err)
+          }
+        }
+        return (actual.readdir as typeof fs.readdir)(
+          p as Parameters<typeof fs.readdir>[0],
+          opts as Parameters<typeof fs.readdir>[1]
+        )
+      }) as typeof fs.readdir)
+
+      await expect(
+        Effect.runPromise(searchSessions('needle', { searchContent: true }))
+      ).rejects.toThrow()
+    })
+  })
+
+  // Phase 1 (title search) TOCTOU guard — a project's folder may vanish
+  // between listProjects() returning it and listSessions()'s own readdir.
+  // listSessions() intentionally throws on a missing project (see the guard's
+  // comment at its Effect.catchAll call site in search.ts), so non-ENOENT
+  // errors (EACCES, etc.) must still propagate here too, distinct from the
+  // ENOENT/ENOTDIR skip case. This coverage was previously only accidental
+  // (the searchProjectContent EACCES test above used to fail one readdir call
+  // too early and exercise this path instead of its own) — now isolated into
+  // its own test so it isn't lost if the call-count fix above ever changes.
+  describe('searchSessions title-search TOCTOU safety', () => {
+    it('propagates a non-ENOENT readdir error (EACCES) from the title-search phase', async () => {
+      const projDenied = '-Users-test-search-title-eacces'
+      await writeSessionFile(projDenied, 'sess-denied', 'plain title')
+
+      // listProjects() readdirs each project folder once (for sessionCount);
+      // let that succeed so projDenied reaches Phase 1, then fail with EACCES
+      // specifically on listSessions' own readdir (call 2) — isolating this
+      // test to the title-search guard, before Phase 2 (searchProjectContent)
+      // is ever entered.
+      const actual = await vi.importActual<typeof import('node:fs/promises')>('node:fs/promises')
+      let deniedCallCount = 0
+      vi.mocked(fs.readdir).mockImplementation(((p: unknown, opts: unknown) => {
+        if (typeof p === 'string' && p.endsWith(projDenied)) {
+          deniedCallCount += 1
+          if (deniedCallCount > 1) {
+            const err = new Error(
+              `EACCES: permission denied, scandir '${p}'`
+            ) as NodeJS.ErrnoException
+            err.code = 'EACCES'
+            return Promise.reject(err)
+          }
+        }
+        return (actual.readdir as typeof fs.readdir)(
+          p as Parameters<typeof fs.readdir>[0],
+          opts as Parameters<typeof fs.readdir>[1]
+        )
+      }) as typeof fs.readdir)
+
+      await expect(Effect.runPromise(searchSessions('needle'))).rejects.toThrow()
+    })
+  })
 })

packages/core/src/session/search.ts

@@ -5,11 +5,19 @@ import { Effect, pipe } from 'effect'
 import * as fs from 'node:fs/promises'
 import * as path from 'node:path'
 import { getSessionsDir } from '../paths.js'
-import { extractTextContent, extractTitle, tryParseJsonLine } from '../utils.js'
+import {
+  extractTextContent,
+  extractTitle,
+  isMissingFolderError,
+  tryParseJsonLine,
+} from '../utils.js'
+import { createLogger } from '../logger.js'
 import { listProjects } from './projects.js'
 import { listSessions } from './crud.js'
 import type { Message, SearchResult, Project } from '../types.js'
 
+const log = createLogger('search')
+
 // Pure function: extract snippet around match
 export const extractSnippet = (text: string, matchIndex: number, queryLength: number): string => {
   const start = Math.max(0, matchIndex - 50)
@@ -74,7 +82,10 @@ const searchSessionContent = (
 const searchProjectContent = (project: Project, queryLower: string, alreadyFoundIds: Set<string>) =>
   Effect.gen(function* () {
     const projectPath = path.join(getSessionsDir(), project.name)
-    const files = yield* Effect.tryPromise(() => fs.readdir(projectPath))
+    const files = yield* Effect.tryPromise({
+      try: () => fs.readdir(projectPath),
+      catch: (error) => error as NodeJS.ErrnoException,
+    })
     const sessionFiles = files.filter((f) => f.endsWith('.jsonl') && !f.startsWith('agent-'))
 
     // Filter out already found sessions and create search effects
@@ -90,7 +101,19 @@ const searchProjectContent = (project: Project, queryLower: string, alreadyFound
 
     const results = yield* Effect.all(searchEffects, { concurrency: 10 })
     return results.filter((r): r is NonNullable<typeof r> => r !== null)
-  })
+  }).pipe(
+    // Guard against TOCTOU: a project's folder may vanish between listProjects()
+    // returning it and this per-project readdir (cross-PC sync, manual deletion).
+    // Narrow to ENOENT/ENOTDIR so unrelated I/O failures (EACCES, EIO, etc.) still
+    // propagate. Mirrors listProjects' own guard — see Issue #103.
+    Effect.catchAll((error) => {
+      if (!isMissingFolderError(error)) {
+        return Effect.fail(error)
+      }
+      log.debug(`searchProjectContent: skipping missing project ${project.name}`, error)
+      return Effect.succeed([] as SearchResult[])
+    })
+  )
 
 // Pattern to detect potential session ID queries (hex chars and hyphens, 8+ chars)
 const SESSION_ID_PATTERN = /^[a-f0-9][a-f0-9-]{7,}$/i
@@ -196,7 +219,26 @@ export const searchSessions = (
                   timestamp: session.updatedAt,
                 }) satisfies SearchResult
             )
-        )
+        ),
+        // Guard against TOCTOU: `project` just came from listProjects(), but its
+        // folder may vanish before this per-project listSessions() call (cross-PC
+        // sync, manual deletion). listSessions() itself intentionally throws on a
+        // missing project (callers passing an arbitrary/typo'd name should see an
+        // error — see packages/mcp's "should throw error for non-existent project"
+        // test), so the guard lives here at the call site, scoped to this
+        // already-enumerated aggregation context only. Narrow to ENOENT/ENOTDIR so
+        // unrelated I/O failures (EACCES, EIO, etc.) still propagate. Mirrors
+        // listProjects' own guard — see Issue #103.
+        Effect.catchAll((error) => {
+          if (!isMissingFolderError(error)) {
+            return Effect.fail(error)
+          }
+          log.debug(
+            `searchSessions: skipping missing project ${project.name} in title search`,
+            error
+          )
+          return Effect.succeed([] as SearchResult[])
+        })
       )
     )
 

packages/core/src/utils.ts

@@ -33,10 +33,20 @@ export class FileWriteError extends Data.TaggedError('FileWriteError')<{
  * (ENOENT, ENOTDIR). Used to narrow TOCTOU-race recovery so unrelated
  * I/O failures (EACCES, EIO, EMFILE, EBUSY, etc.) still propagate.
  *
+ * Unwraps Effect's `UnknownException` (the wrapper `Effect.tryPromise(fn)`
+ * produces when called without an explicit `{ try, catch }` mapping — the
+ * original `NodeJS.ErrnoException` ends up on `.error`/`.cause` instead of
+ * the top level) so callers don't need to know which `tryPromise` form the
+ * effect they're guarding happens to use.
+ *
  * See Issue #103 + .claude/rules/async-io.md.
  */
 export const isMissingFolderError = (error: unknown): boolean => {
-  const code = (error as NodeJS.ErrnoException | null | undefined)?.code
+  const codeOf = (e: unknown) => (e as NodeJS.ErrnoException | null | undefined)?.code
+  const code =
+    codeOf(error) ??
+    codeOf((error as { error?: unknown } | null | undefined)?.error) ??
+    codeOf((error as { cause?: unknown } | null | undefined)?.cause)
   return code === 'ENOENT' || code === 'ENOTDIR'
 }