From 00b935abe6e2f5d131dfa4ea51ececca1a5409be Mon Sep 17 00:00:00 2001 From: Saqeb Akhter Date: Thu, 1 Oct 2026 13:58:55 -0400 Subject: [PATCH 1/3] fix(cases): bound linked-workspace path probes so an unreachable mount cannot freeze the server A linked case can live on a network mount. When that mount goes away, a hard mount makes stat() wait indefinitely, and the existsSync() probes in the case routes and the workspace hook/statusline helpers ran on the event loop, so a single GET /api/cases (or a session create in that workspace) froze the whole web server until the mount came back. Add boundedPathExists() (src/utils/bounded-path-probe.ts): an async stat that answers "absent" after 1.5 s, shares one in-flight probe per path, remembers a timed-out path until its stat finally settles, and refuses to start new probes while two stalled ones still hold libuv threadpool workers. Route the read-side probes in case-routes.ts and hooks-config.ts through it. The settings writers in hooks-config.ts use an async lstat that treats only ENOENT as missing, so an unreachable workspace is never mistaken for an empty one and has its settings recreated. --- src/hooks-config.ts | 42 +++++++++---- src/utils/bounded-path-probe.ts | 82 +++++++++++++++++++++++++ src/web/routes/case-routes.ts | 26 ++++---- test/bounded-path-probe.test.ts | 103 ++++++++++++++++++++++++++++++++ test/routes/case-routes.test.ts | 44 ++++++++++++++ 5 files changed, 274 insertions(+), 23 deletions(-) create mode 100644 src/utils/bounded-path-probe.ts create mode 100644 test/bounded-path-probe.test.ts diff --git a/src/hooks-config.ts b/src/hooks-config.ts index fda526c7e..568d1c27f 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -30,7 +30,6 @@ */ import { randomBytes } from 'node:crypto'; -import { existsSync } from 'node:fs'; import { readFile, writeFile, mkdir, lstat, readdir, realpath, rename, unlink, rmdir, chmod } from 'node:fs/promises'; import { homedir } from 'node:os'; import { join, dirname } from 'node:path'; @@ -40,6 +39,25 @@ import type { HookEventType } from './types.js'; import { HOOK_TIMEOUT_SECONDS } from './config/auth-config.js'; import { dataPath } from './config/instance.js'; import { readJsonConfig, SETTINGS_PATH } from './web/route-helpers.js'; +import { boundedPathExists } from './utils/bounded-path-probe.js'; + +/** + * Existence check for a WRITER. Unlike `boundedPathExists`, which answers + * "absent" for a path it could not reach in time, this tells "missing" apart + * from "unreachable": only ENOENT reads as absent, anything else throws, so a + * stalled or unreadable workspace can never be mistaken for an empty one and + * have its settings recreated over the top. It is async, so a dead mount ties + * up a threadpool worker rather than the event loop. + */ +async function pathExistsForWrite(path: string): Promise { + try { + await lstat(path); + return true; + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return false; + throw err; + } +} /** * Serializes read-modify-write access to a `settings.local.json` path. Every @@ -558,7 +576,7 @@ export async function stripCaseEnvKeys(casePath: string, keysToRemove: readonly if (keysToRemove.length === 0) return; await withSafeSettingsWrite(casePath, 'env-key removal', async (_claudeDir, settingsPath) => { - if (!existsSync(settingsPath)) return; + if (!(await boundedPathExists(settingsPath))) return; let existing: Record; try { @@ -590,7 +608,7 @@ export async function stripCaseEnvKeys(casePath: string, keysToRemove: readonly */ export async function updateCaseEnvVars(casePath: string, envVars: Record): Promise { await withSafeSettingsWrite(casePath, 'env vars', async (claudeDir, settingsPath) => { - if (!existsSync(claudeDir)) { + if (!(await pathExistsForWrite(claudeDir))) { await mkdir(claudeDir, { recursive: true }); } @@ -621,7 +639,7 @@ export async function updateCaseEnvVars(casePath: string, envVars: Record { await withSafeSettingsWrite(casePath, 'model', async (claudeDir, settingsPath) => { - if (!existsSync(claudeDir)) { + if (!(await pathExistsForWrite(claudeDir))) { await mkdir(claudeDir, { recursive: true }); } @@ -650,7 +668,7 @@ export async function updateCaseModel(casePath: string, model: string | null): P */ export async function writeHooksConfig(casePath: string): Promise { await withSafeSettingsWrite(casePath, 'hooks', async (claudeDir, settingsPath) => { - if (!existsSync(claudeDir)) { + if (!(await pathExistsForWrite(claudeDir))) { await mkdir(claudeDir, { recursive: true }); } @@ -698,7 +716,7 @@ export async function writeHooksConfig(casePath: string): Promise { */ export async function ensureCodemanHooks(casePath: string): Promise { await withSafeSettingsWrite(casePath, 'hooks (ensure)', async (claudeDir, settingsPath) => { - if (!existsSync(claudeDir)) { + if (!(await pathExistsForWrite(claudeDir))) { await mkdir(claudeDir, { recursive: true }); } @@ -738,7 +756,7 @@ export async function ensureCodemanHooks(casePath: string): Promise { * when the hooks aren't ours, so it is cheap enough to call on every Claude spawn. */ export async function refreshStaleCodemanHooks(casePath: string): Promise { - if (!existsSync(join(casePath, '.claude', 'settings.local.json'))) return; + if (!(await boundedPathExists(join(casePath, '.claude', 'settings.local.json')))) return; await withSafeSettingsWrite(casePath, 'hooks (refresh)', async (_claudeDir, settingsPath) => { let existing: Record; try { @@ -820,7 +838,7 @@ export async function refreshStaleCodemanHooks(casePath: string): Promise */ export async function applyWorkspaceHooks(workspace: string, install?: boolean): Promise { try { - if (!existsSync(workspace)) return; + if (!(await boundedPathExists(workspace))) return; const shouldInstall = install ?? (await readWorkspaceHooksEnabled()); await (shouldInstall ? ensureCodemanHooks(workspace) : refreshStaleCodemanHooks(workspace)); } catch { @@ -883,7 +901,7 @@ export function generateStatusLineCommand(): string { export async function applyStatusLineConfig(casePath: string, enabled: boolean): Promise { await withSafeSettingsWrite(casePath, 'statusLine', async (claudeDir, settingsPath) => { let existing: Record = {}; - if (existsSync(settingsPath)) { + if (await pathExistsForWrite(settingsPath)) { try { existing = JSON.parse(await readFile(settingsPath, 'utf-8')); } catch { @@ -898,7 +916,7 @@ export async function applyStatusLineConfig(casePath: string, enabled: boolean): const desired = generateStatusLineCommand(); if (isOurs && current?.command === desired) return; // already current — skip rewrite if (current && !isOurs) return; // user has their OWN statusLine — never clobber it - if (!existsSync(claudeDir)) await mkdir(claudeDir, { recursive: true }); + if (!(await pathExistsForWrite(claudeDir))) await mkdir(claudeDir, { recursive: true }); existing.statusLine = { type: 'command', command: desired }; // add, or update an out-of-date ours } else { if (!isOurs) return; // nothing of ours to remove (leave a user's own statusLine alone) @@ -957,7 +975,7 @@ function statusLineExporterScriptContent(): string { } async function readStatusLineCommandFromFile(settingsPath: string): Promise { - if (!existsSync(settingsPath)) return undefined; + if (!(await boundedPathExists(settingsPath))) return undefined; try { const parsed = JSON.parse(await readFile(settingsPath, 'utf-8')); const current = parsed.statusLine as { command?: unknown } | undefined; @@ -1103,7 +1121,7 @@ export async function resolveStatusLineCliCommand( ): Promise { const settingsPath = join(casePath, '.claude', 'settings.local.json'); let userHasOwnStatusLine = false; - if (existsSync(settingsPath)) { + if (await boundedPathExists(settingsPath)) { try { const existing = JSON.parse(await readFile(settingsPath, 'utf-8')); const current = existing.statusLine as { command?: unknown } | undefined; diff --git a/src/utils/bounded-path-probe.ts b/src/utils/bounded-path-probe.ts new file mode 100644 index 000000000..28928b0ce --- /dev/null +++ b/src/utils/bounded-path-probe.ts @@ -0,0 +1,82 @@ +/** + * @fileoverview Bounded existence probe for user-chosen paths. + * + * A linked case can live on a network mount (NFS, SMB, sshfs). When that mount + * goes unreachable, a hard mount makes `stat()` wait forever. A synchronous + * probe (`existsSync`) on such a path blocks the event loop and freezes the + * whole web server; even an async `stat()` never settles and permanently holds + * one of libuv's few threadpool workers, which every other `fs`, `dns.lookup` + * and `crypto` call in the process shares. + * + * `boundedPathExists()` therefore: + * - probes asynchronously and answers `false` after `PROBE_TIMEOUT_MS`, so a + * request never waits on a dead mount for longer than that; + * - shares one in-flight probe per path, and keeps answering `false` for a path + * whose probe timed out until that probe finally settles (so a dead path is + * not re-probed on every request, and is re-probed once the mount recovers); + * - stops starting new probes once `MAX_STALLED_PROBES` timed-out probes are + * still pending, so stalled stats cannot drain the threadpool. Probes that are + * merely in flight do not count, so concurrent healthy probes never get a + * false negative. + * + * Like `existsSync`, it follows symlinks and reports any error as "absent". It + * is meant for READ decisions (is it there, show it or not). A writer that must + * tell "missing" apart from "unreachable" should not treat its `false` as + * permission to create or overwrite anything. + * + * @module utils/bounded-path-probe + */ + +import fs from 'node:fs/promises'; + +/** How long a caller waits for one probe before treating the path as absent. */ +export const PROBE_TIMEOUT_MS = 1_500; +/** Timed-out probes allowed to remain pending before new probes are refused. */ +export const MAX_STALLED_PROBES = 2; + +const inFlight = new Map>(); +const stalled = new Set(); + +async function statExists(path: string): Promise { + try { + await fs.stat(path); + return true; + } catch { + return false; + } +} + +/** + * Resolve whether `path` exists without letting an unresponsive filesystem + * block the caller for longer than `PROBE_TIMEOUT_MS`. + */ +export async function boundedPathExists(path: string): Promise { + if (stalled.has(path)) return false; + + let probe = inFlight.get(path); + if (!probe) { + if (stalled.size >= MAX_STALLED_PROBES) return false; + probe = statExists(path); + inFlight.set(path, probe); + void probe.finally(() => { + inFlight.delete(path); + stalled.delete(path); + }); + } + + let timer: ReturnType | undefined; + try { + return await Promise.race([ + probe, + new Promise((resolve) => { + timer = setTimeout(() => { + if (inFlight.get(path) === probe) stalled.add(path); + resolve(false); + }, PROBE_TIMEOUT_MS); + timer.unref?.(); + }), + ]); + } finally { + if (timer) clearTimeout(timer); + } +} diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index b48023443..7aed82ac2 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -50,6 +50,7 @@ import { } from '../../git-clone.js'; import type { GitRemoteProbe, GitUrlParse } from '../../git-clone.js'; import { generateClaudeMd } from '../../templates/claude-md.js'; +import { boundedPathExists } from '../../utils/bounded-path-probe.js'; import { readAgentCaseMarker, type AgentCaseMarker } from '../../agent-case-marker.js'; import { settingsWriteBlocker, writeHooksConfig } from '../../hooks-config.js'; import { @@ -162,8 +163,11 @@ function gitDiagnosticLine(stderr: string): string { * hooks, which run on the user's machine when a session starts in the case, so * the clone response says so out loud instead of silently merging into them. */ -function repoShipsClaudeSettings(casePath: string): boolean { - return ['settings.json', 'settings.local.json'].some((file) => existsSync(join(casePath, '.claude', file))); +async function repoShipsClaudeSettings(casePath: string): Promise { + for (const file of ['settings.json', 'settings.local.json']) { + if (await boundedPathExists(join(casePath, '.claude', file))) return true; + } + return false; } /** @@ -266,7 +270,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config cases.push({ name: e.name, path: casePath, - hasClaudeMd: existsSync(join(casePath, 'CLAUDE.md')), + hasClaudeMd: await boundedPathExists(join(casePath, 'CLAUDE.md')), location: 'local', ...(marker ? { agentCreated: agentCreatedInfo(marker) } : {}), }); @@ -281,11 +285,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const existingNames = new Set(cases.map((c) => c.name)); if (admin) { for (const [name, path] of Object.entries(linkedCases)) { - if (!existingNames.has(name) && SAFE_CASE_NAME.test(name) && existsSync(path)) { + if (!existingNames.has(name) && SAFE_CASE_NAME.test(name) && (await boundedPathExists(path))) { cases.push({ name, path, - hasClaudeMd: existsSync(join(path, 'CLAUDE.md')), + hasClaudeMd: await boundedPathExists(join(path, 'CLAUDE.md')), linked: true, location: 'linked-local', }); @@ -333,7 +337,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const dockerCaseInfo: CaseInfo = { name: dockerCase.name, path: dockerDisplayPath({ container, path: dockerCase.hostWorkspacePath }), - hasClaudeMd: existsSync(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')), + hasClaudeMd: await boundedPathExists(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')), location: 'docker', docker: { hostId: host.id, @@ -615,7 +619,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } else { warnings.push('Kept the repository’s own CLAUDE.md.'); } - if (repoShipsClaudeSettings(casePath)) { + if (await repoShipsClaudeSettings(casePath)) { warnings.push( 'This repository ships its own .claude/settings files. Codeman merged its hooks alongside them without removing anything — review them before starting a session, since repo-supplied hooks run on this machine.' ); @@ -1618,7 +1622,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return { name, path: dockerDisplayPath({ container, path: dockerCase.hostWorkspacePath }), - hasClaudeMd: existsSync(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')), + hasClaudeMd: await boundedPathExists(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')), location: 'docker', docker: { hostId: host.id, @@ -1634,7 +1638,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const casePath = await resolveCasePath(name, getAuthUser(req)); - if (!existsSync(casePath)) { + if (!(await boundedPathExists(casePath))) { return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Case not found'); } @@ -1642,7 +1646,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return { name, path: casePath, - hasClaudeMd: existsSync(join(casePath, 'CLAUDE.md')), + hasClaudeMd: await boundedPathExists(join(casePath, 'CLAUDE.md')), ...(linked && { linked: true }), }; }); @@ -1660,7 +1664,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const fixPlanPath = join(casePath, '@fix_plan.md'); - if (!existsSync(fixPlanPath)) { + if (!(await boundedPathExists(fixPlanPath))) { return { exists: false, content: null, todos: [] }; } diff --git a/test/bounded-path-probe.test.ts b/test/bounded-path-probe.test.ts new file mode 100644 index 000000000..c44e46bf9 --- /dev/null +++ b/test/bounded-path-probe.test.ts @@ -0,0 +1,103 @@ +/** + * @fileoverview Tests for boundedPathExists (src/utils/bounded-path-probe.ts): + * a stat() that never settles (an unreachable hard network mount) must not hold + * the caller past the timeout, must not be re-issued while it is still pending, + * and must not let stalled probes pile up in libuv's shared threadpool. + */ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('node:fs/promises', () => ({ + default: { stat: vi.fn() }, +})); + +import fs from 'node:fs/promises'; +import { boundedPathExists, PROBE_TIMEOUT_MS } from '../src/utils/bounded-path-probe.js'; + +const stat = vi.mocked(fs.stat); + +/** + * Make the first stat() of each given path hang until released (the mount is + * down); every later stat, and every other path, answers "exists". + */ +function hangOn(paths: string[]): Map void> { + const releases = new Map void>(); + stat.mockImplementation((path) => { + if (!paths.includes(String(path)) || releases.has(String(path))) return Promise.resolve({} as never); + return new Promise((resolve) => { + releases.set(String(path), () => resolve({} as never)); + }); + }); + return releases; +} + +afterEach(() => { + vi.useRealTimers(); + stat.mockReset(); +}); + +describe('boundedPathExists', () => { + it('reports an existing path as present and a missing one as absent', async () => { + stat.mockImplementation(async (path) => { + if (String(path) === '/present') return {} as never; + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + }); + expect(await boundedPathExists('/present')).toBe(true); + expect(await boundedPathExists('/missing')).toBe(false); + }); + + it('answers false after the timeout when stat never settles, and does not re-probe until it does', async () => { + vi.useFakeTimers(); + const releases = hangOn(['/mnt/stalled/case']); + + const result = boundedPathExists('/mnt/stalled/case'); + await vi.advanceTimersByTimeAsync(PROBE_TIMEOUT_MS); + expect(await result).toBe(false); + + // A second caller gets the cached verdict immediately, without another stat. + expect(await boundedPathExists('/mnt/stalled/case')).toBe(false); + expect(stat).toHaveBeenCalledTimes(1); + + // Once the mount answers, the path is probed afresh. + releases.get('/mnt/stalled/case')!(); + await vi.advanceTimersByTimeAsync(0); + expect(await boundedPathExists('/mnt/stalled/case')).toBe(true); + expect(stat).toHaveBeenCalledTimes(2); + }); + + it('shares one in-flight stat between concurrent callers of the same path', async () => { + const releases = hangOn(['/slow']); + const a = boundedPathExists('/slow'); + const b = boundedPathExists('/slow'); + expect(stat).toHaveBeenCalledTimes(1); + releases.get('/slow')!(); + expect(await a).toBe(true); + expect(await b).toBe(true); + }); + + it('does not give concurrent healthy probes a false negative', async () => { + stat.mockImplementation(async () => ({}) as never); + const results = await Promise.all(['/a', '/b', '/c', '/d', '/e'].map((p) => boundedPathExists(p))); + expect(results).toEqual([true, true, true, true, true]); + }); + + it('stops issuing new stats once stalled probes would tie up the threadpool', async () => { + vi.useFakeTimers(); + const releases = hangOn(['/mnt/stalled/one', '/mnt/stalled/two']); + + const first = boundedPathExists('/mnt/stalled/one'); + const second = boundedPathExists('/mnt/stalled/two'); + await vi.advanceTimersByTimeAsync(PROBE_TIMEOUT_MS); + expect(await first).toBe(false); + expect(await second).toBe(false); + + // Both slots are held by stats that never returned: refuse a third. + expect(await boundedPathExists('/healthy/three')).toBe(false); + expect(stat).toHaveBeenCalledTimes(2); + + // Once the stalled stats settle, probing resumes normally. + releases.forEach((release) => release()); + await vi.advanceTimersByTimeAsync(0); + expect(await boundedPathExists('/healthy/three')).toBe(true); + expect(stat).toHaveBeenCalledTimes(3); + }); +}); diff --git a/test/routes/case-routes.test.ts b/test/routes/case-routes.test.ts index 56a89a2fb..4315e0a81 100644 --- a/test/routes/case-routes.test.ts +++ b/test/routes/case-routes.test.ts @@ -35,6 +35,7 @@ vi.mock('node:fs', async (importOriginal) => { vi.mock('node:fs/promises', () => ({ default: { + stat: vi.fn(), readdir: vi.fn(async () => []), readFile: vi.fn(async () => { const err = new Error('ENOENT') as NodeJS.ErrnoException; @@ -74,6 +75,7 @@ const mockedReaddirSync = vi.mocked(readdirSync); const mockedReaddir = vi.mocked(fs.readdir); const mockedReadFile = vi.mocked(fs.readFile); const mockedWriteFile = vi.mocked(fs.writeFile); +const mockedStat = vi.mocked(fs.stat); const mockedCheckRemoteTmux = vi.mocked(checkRemoteTmuxAvailable); interface CaseRouteHarness { @@ -127,6 +129,12 @@ describe('case-routes', () => { // Default: existsSync returns false, readFile throws ENOENT mockedExistsSync.mockReturnValue(false); mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + // Async stat (the bounded path probe) follows the mocked existsSync, so a + // test that sets up a path's presence via existsSync drives both the same way. + mockedStat.mockImplementation(async (path) => { + if (mockedExistsSync(path)) return { isDirectory: () => true } as never; + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + }); }); afterEach(async () => { @@ -210,6 +218,42 @@ describe('case-routes', () => { // Should have both regular and linked cases expect(body.data.length).toBeGreaterThanOrEqual(1); }); + + it('still answers promptly when a linked case sits on an unreachable mount', async () => { + // A hard network mount that went away: a synchronous probe blocks the + // thread (simulated by a busy-wait), and an async stat never settles. + const stalledPath = '/mnt/unreachable/linked-nfs'; + const BLOCK_MS = 4_000; + mockedReaddir.mockResolvedValue([] as never); + mockedReadFile.mockResolvedValueOnce(JSON.stringify({ 'linked-nfs': stalledPath }) as never); + mockedExistsSync.mockImplementation((p) => { + if (String(p) !== stalledPath) return false; + const until = Date.now() + BLOCK_MS; + while (Date.now() < until) { + // spin: the event loop is frozen for as long as the mount does not answer + } + return true; + }); + let release: (() => void) | undefined; + mockedStat.mockImplementation((p) => { + if (String(p) !== stalledPath) { + return Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + } + return new Promise((resolve) => { + release = () => resolve({ isDirectory: () => true } as never); + }); + }); + + const started = Date.now(); + const res = await harness.app.inject({ method: 'GET', url: '/api/cases' }); + const elapsed = Date.now() - started; + release?.(); + + expect(res.statusCode).toBe(200); + expect(elapsed).toBeLessThan(BLOCK_MS - 1_000); + // The unreachable case is left out rather than holding the list hostage. + expect(JSON.parse(res.body).data).toEqual([]); + }); }); describe('remote host and remote case routes', () => { From d1bfbb4fcfdb199959b0080863a2ccb970ac6707 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sun, 4 Oct 2026 20:30:40 -0400 Subject: [PATCH 2/3] fix(cases): tell an unreachable path from an absent one, scope the stall cap The bounded path probe answered "absent" both when a path did not exist and when it simply did not answer, so a stalled linked case 404'd and the Run button scaffolded a stray local case over it, and two stalled paths anywhere made every unrelated path read as absent (hooks skipped, statusLine overridden, the clone warning lost). - probePath()/probePathKind() are tri-state: present (or directory/file), absent (ENOENT/ENOTDIR only) and unknown (timeout, other errors, refusal). boundedPathExists() stays as the display-only boolean. - A stalled path takes only its own mount out of probing (deepest mount point from /proc/self/mounts, never /; just the path itself when there is no mount table). Unrelated paths keep probing. The process-wide cap is a backstop that answers unknown, and a single-path user request can probe past it ({ pastCap: true }), still bounded and still recorded as stalled. One console.warn when a path first stalls and one when the cap engages. - GET /api/cases/:name keeps NOT_FOUND for definite absence only. An unreachable linked case answers with its registered path and unreachable: true; a local one answers OPERATION_FAILED. runClaude and runShell create a case only on errorCode NOT_FOUND. The case list keeps an unreachable linked case, marked unreachable, instead of dropping it, and fix-plan reports an unreadable plan as an error, not "no plan". - applyWorkspaceHooks and the statusLine helpers skip only a workspace that is absent or on the stalled mount; a capacity refusal no longer stops hooks being installed elsewhere, and an unreadable settings file never lets the exporter override a user's own statusLine. - The clone flow's repo-settings warning is back on its synchronous check, and stripCaseEnvKeys uses pathExistsForWrite. - POST /api/sessions (workingDir) and POST /api/quick-start (case folder) probe with the bounded probe instead of statSync/existsSync. Missing and non-directory keep INVALID_INPUT; unknown is OPERATION_FAILED, and quick-start never scaffolds over a folder that did not answer. - PATH_PROBE_TIMEOUT_MS and MAX_STALLED_PATH_PROBES move to src/config/path-probe.ts, overridable via CODEMAN_PATH_PROBE_TIMEOUT_MS (default 1500) and CODEMAN_PATH_PROBE_MAX_STALLED (default 3), and are documented in the Settings Reference. - The probe is exported from the utils barrel and imported from there. --- docs/wiki/Settings-Reference.md | 2 + src/config/path-probe.ts | 35 +++ src/hooks-config.ts | 52 +++- src/types/api.ts | 5 + src/utils/bounded-path-probe.ts | 186 +++++++++++--- src/utils/index.ts | 2 + src/web/public/session-ui.js | 16 +- src/web/routes/case-routes.ts | 61 +++-- src/web/routes/session-routes.ts | 36 ++- test/bounded-path-probe.test.ts | 226 +++++++++++++++--- test/routes/case-clone-routes.test.ts | 44 +++- test/routes/case-routes.test.ts | 96 +++++++- .../session-create-unreachable-path.test.ts | 174 ++++++++++++++ test/run-mode-ui.test.ts | 62 +++++ .../workspace-hooks-unreachable-mount.test.ts | 145 +++++++++++ 15 files changed, 1022 insertions(+), 120 deletions(-) create mode 100644 src/config/path-probe.ts create mode 100644 test/routes/session-create-unreachable-path.test.ts create mode 100644 test/workspace-hooks-unreachable-mount.test.ts diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index 4892d3be8..58114914b 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -181,6 +181,8 @@ Some things are configured before the server starts, not in the UI: | `CODEMAN_BASE_URL` | Mounts Codeman under a sub-path behind a reverse proxy that forwards the prefix unchanged. See [Remote Access](Remote-Access). | | `CODEMAN_MAX_DOWNLOAD_BYTES` | Cap on raw file bodies and downloads. 2 GB by default, `0` for none. | | `CODEMAN_MAX_REMOTE_FILE_SSH` | Concurrent ssh reads for files in remote cases. 4 by default. | +| `CODEMAN_PATH_PROBE_TIMEOUT_MS` | How long a linked case's folder may take to answer before it is shown as unreachable. 1500 ms by default; raise it for a slow but healthy mount. | +| `CODEMAN_PATH_PROBE_MAX_STALLED` | Unanswered folder checks allowed to pile up before new ones are refused. 3 by default. | ## Gotchas diff --git a/src/config/path-probe.ts b/src/config/path-probe.ts new file mode 100644 index 000000000..3c11a82d2 --- /dev/null +++ b/src/config/path-probe.ts @@ -0,0 +1,35 @@ +/** + * @fileoverview Limits for the bounded path probe (`src/utils/bounded-path-probe.ts`). + * + * A linked case can live on a network mount, and a hard mount that went away makes + * `stat()` wait until the mount comes back. The probe gives up on such a path after + * `PATH_PROBE_TIMEOUT_MS` and answers "unknown", and it stops starting new probes + * once `MAX_STALLED_PATH_PROBES` timed-out stats are still holding libuv threadpool + * workers (the pool is shared by every `fs`, `dns.lookup` and `crypto` call in the + * process, and holds 4 workers unless `UV_THREADPOOL_SIZE` says otherwise). + * + * Both are env-overridable, in the same style as the other config modules. A slow + * but healthy mount (an sshfs that needs a couple of seconds on first touch) may want + * a longer timeout; a server started with a larger `UV_THREADPOOL_SIZE` can afford a + * higher stall cap. + * + * @module config/path-probe + */ + +function envInt(name: string, fallback: number, min: number, max: number): number { + const raw = parseInt(process.env[name] || '', 10); + if (!Number.isFinite(raw) || raw <= 0) return fallback; + return Math.max(min, Math.min(max, raw)); +} + +/** How long a caller waits for one path probe before the answer is "unknown". */ +export const PATH_PROBE_TIMEOUT_MS = envInt('CODEMAN_PATH_PROBE_TIMEOUT_MS', 1_500, 100, 60_000); + +/** + * Timed-out probes allowed to stay pending before new probes are refused (answered + * "unknown" without a stat). This is a backstop, not the main defence: a stalled + * path already takes its neighbours (same parent directory) out of probing, so the + * cap only engages once three UNRELATED places have stopped answering. The default + * leaves one of libuv's default four workers free for the rest of the process. + */ +export const MAX_STALLED_PATH_PROBES = envInt('CODEMAN_PATH_PROBE_MAX_STALLED', 3, 1, 64); diff --git a/src/hooks-config.ts b/src/hooks-config.ts index 568d1c27f..33f9c618b 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -39,12 +39,12 @@ import type { HookEventType } from './types.js'; import { HOOK_TIMEOUT_SECONDS } from './config/auth-config.js'; import { dataPath } from './config/instance.js'; import { readJsonConfig, SETTINGS_PATH } from './web/route-helpers.js'; -import { boundedPathExists } from './utils/bounded-path-probe.js'; +import { isNearStalledPath, probePath } from './utils/index.js'; /** - * Existence check for a WRITER. Unlike `boundedPathExists`, which answers - * "absent" for a path it could not reach in time, this tells "missing" apart - * from "unreachable": only ENOENT reads as absent, anything else throws, so a + * Existence check for a WRITER. Unlike the bounded read-side probe (`probePath`), + * which gives up after a timeout and answers "unknown", this waits for the real + * answer: only ENOENT reads as absent, anything else throws, so a * stalled or unreadable workspace can never be mistaken for an empty one and * have its settings recreated over the top. It is async, so a dead mount ties * up a threadpool worker rather than the event loop. @@ -59,6 +59,20 @@ async function pathExistsForWrite(path: string): Promise { } } +/** + * Whether a READ-side helper should leave `path` alone: it is definitely absent, or + * it sits on a mount that is not answering (near a stalled probe). An "unknown" + * that is NOT near a stalled probe (the probe was refused for capacity, or the stat + * failed with something other than ENOENT) is not a reason to skip: the caller goes + * on, and its own async read or write settles the question for that one path. + */ +async function absentOrUnreachable(path: string): Promise<'absent' | 'unreachable' | false> { + const state = await probePath(path); + if (state === 'absent') return 'absent'; + if (state === 'unknown' && isNearStalledPath(path)) return 'unreachable'; + return false; +} + /** * Serializes read-modify-write access to a `settings.local.json` path. Every * writer in this module (hooks, env, model, statusLine) shares this map, so @@ -576,7 +590,7 @@ export async function stripCaseEnvKeys(casePath: string, keysToRemove: readonly if (keysToRemove.length === 0) return; await withSafeSettingsWrite(casePath, 'env-key removal', async (_claudeDir, settingsPath) => { - if (!(await boundedPathExists(settingsPath))) return; + if (!(await pathExistsForWrite(settingsPath))) return; let existing: Record; try { @@ -756,7 +770,7 @@ export async function ensureCodemanHooks(casePath: string): Promise { * when the hooks aren't ours, so it is cheap enough to call on every Claude spawn. */ export async function refreshStaleCodemanHooks(casePath: string): Promise { - if (!(await boundedPathExists(join(casePath, '.claude', 'settings.local.json')))) return; + if (await absentOrUnreachable(join(casePath, '.claude', 'settings.local.json'))) return; await withSafeSettingsWrite(casePath, 'hooks (refresh)', async (_claudeDir, settingsPath) => { let existing: Record; try { @@ -838,7 +852,13 @@ export async function refreshStaleCodemanHooks(casePath: string): Promise */ export async function applyWorkspaceHooks(workspace: string, install?: boolean): Promise { try { - if (!(await boundedPathExists(workspace))) return; + const skip = await absentOrUnreachable(workspace); + if (skip === 'unreachable') { + console.warn( + `[hooks] ${workspace} is not responding (unreachable mount?); Codeman hooks not checked or installed` + ); + } + if (skip) return; const shouldInstall = install ?? (await readWorkspaceHooksEnabled()); await (shouldInstall ? ensureCodemanHooks(workspace) : refreshStaleCodemanHooks(workspace)); } catch { @@ -975,7 +995,7 @@ function statusLineExporterScriptContent(): string { } async function readStatusLineCommandFromFile(settingsPath: string): Promise { - if (!(await boundedPathExists(settingsPath))) return undefined; + if (await absentOrUnreachable(settingsPath)) return undefined; try { const parsed = JSON.parse(await readFile(settingsPath, 'utf-8')); const current = parsed.statusLine as { command?: unknown } | undefined; @@ -1121,9 +1141,21 @@ export async function resolveStatusLineCliCommand( ): Promise { const settingsPath = join(casePath, '.claude', 'settings.local.json'); let userHasOwnStatusLine = false; - if (await boundedPathExists(settingsPath)) { + const skip = await absentOrUnreachable(settingsPath); + // Unreachable: whether the user configured their own statusLine there cannot be + // told, and this must never override a real one, so inject nothing. + if (skip === 'unreachable') return undefined; + if (!skip) { + let raw: string; + try { + raw = await readFile(settingsPath, 'utf-8'); + } catch (err) { + // Gone since the probe: nothing to respect. Unreadable: same reason as above. + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') return undefined; + raw = ''; + } try { - const existing = JSON.parse(await readFile(settingsPath, 'utf-8')); + const existing = raw ? JSON.parse(raw) : {}; const current = existing.statusLine as { command?: unknown } | undefined; if (current && typeof current.command === 'string') { if (current.command.includes(STATUSLINE_MARKER)) { diff --git a/src/types/api.ts b/src/types/api.ts index 1a6193c76..52a076401 100644 --- a/src/types/api.ts +++ b/src/types/api.ts @@ -157,6 +157,11 @@ export interface CaseInfo { location?: 'local' | 'linked-local' | 'remote' | 'docker'; /** Whether this is a linked local folder */ linked?: boolean; + /** + * The case folder did not answer (an unreachable network mount, or an error other + * than "no such file"), so whether it still exists is unknown. Absent = it answered. + */ + unreachable?: boolean; /** * Present when Codeman scaffolded this case directory for an AGENT-spawned session * (the packaged skill's workers, or any spawn naming a parent session), read back diff --git a/src/utils/bounded-path-probe.ts b/src/utils/bounded-path-probe.ts index 28928b0ce..5b2e7e58c 100644 --- a/src/utils/bounded-path-probe.ts +++ b/src/utils/bounded-path-probe.ts @@ -8,59 +8,145 @@ * one of libuv's few threadpool workers, which every other `fs`, `dns.lookup` * and `crypto` call in the process shares. * - * `boundedPathExists()` therefore: - * - probes asynchronously and answers `false` after `PROBE_TIMEOUT_MS`, so a - * request never waits on a dead mount for longer than that; - * - shares one in-flight probe per path, and keeps answering `false` for a path - * whose probe timed out until that probe finally settles (so a dead path is - * not re-probed on every request, and is re-probed once the mount recovers); - * - stops starting new probes once `MAX_STALLED_PROBES` timed-out probes are - * still pending, so stalled stats cannot drain the threadpool. Probes that are - * merely in flight do not count, so concurrent healthy probes never get a - * false negative. + * The probe therefore answers one of THREE things, never two: + * - `'present'` / `'absent'`: the filesystem answered (ENOENT and ENOTDIR are + * the only errors that mean absent); + * - `'unknown'`: it did not answer in `PATH_PROBE_TIMEOUT_MS`, it answered with + * some other error (EIO from a soft mount that gave up, EACCES), or the probe + * was refused (below). "Unknown" is NOT "absent": a caller that would create, + * scaffold or 404 on absence must not do so on unknown. * - * Like `existsSync`, it follows symlinks and reports any error as "absent". It - * is meant for READ decisions (is it there, show it or not). A writer that must - * tell "missing" apart from "unreachable" should not treat its `false` as - * permission to create or overwrite anything. + * And it keeps a dead mount from draining the threadpool: + * - one in-flight probe per path, shared by concurrent callers; + * - a path whose probe timed out is "stalled" until that stat finally settles. + * Paths NEAR a stalled one are answered "unknown" without a new stat, so one + * dead mount costs one worker, not one per case and file on it. "Near" means on + * the same mount: under the deepest mount point holding the stalled path, read + * from `/proc/self/mounts` (procfs, which never waits on the dead filesystem). + * Where that table is unavailable (not Linux), or the deepest mount is `/`, it + * narrows to the stalled path and everything under it. Unrelated paths are + * probed normally; + * - once `MAX_STALLED_PATH_PROBES` stalled stats are pending, new probes are + * refused process-wide (answered "unknown"), since each would risk another + * worker. Probes merely in flight do not count, so concurrent healthy probes + * never get refused. A caller acting on ONE path at a user's explicit request + * (opening a case, starting a session in it) may pass `{ pastCap: true }`: its + * probe is still bounded and still recorded as stalled if it hangs (so a dead + * path costs at most one worker however often it is retried), but it is not + * refused just because unrelated mounts are dead. Bulk scans (the case list) + * and per-spawn helpers keep the cap. + * + * Both events are logged once (`console.warn`): a path's first stall, and the + * cap engaging, so "my case vanished" and "hooks stopped firing" leave a trace. + * + * Writers should not use this at all: a writer that must tell "missing" apart + * from "unreachable" wants an ENOENT-aware async `lstat` (see + * `pathExistsForWrite` in hooks-config.ts). * * @module utils/bounded-path-probe */ +import { readFileSync } from 'node:fs'; import fs from 'node:fs/promises'; +import { resolve, sep } from 'node:path'; +import { MAX_STALLED_PATH_PROBES, PATH_PROBE_TIMEOUT_MS } from '../config/path-probe.js'; -/** How long a caller waits for one probe before treating the path as absent. */ -export const PROBE_TIMEOUT_MS = 1_500; -/** Timed-out probes allowed to remain pending before new probes are refused. */ -export const MAX_STALLED_PROBES = 2; +/** What a probe could establish about a path. */ +export type PathProbeState = 'present' | 'absent' | 'unknown'; +/** Like {@link PathProbeState}, with "present" split by whether it is a directory. */ +export type PathProbeKind = 'directory' | 'file' | 'absent' | 'unknown'; -const inFlight = new Map>(); -const stalled = new Set(); +const inFlight = new Map>(); +/** Stalled path -> the directory whose subtree is answered "unknown" while it stays stalled. */ +const stalled = new Map(); +let capWarned = false; -async function statExists(path: string): Promise { +async function statKind(path: string): Promise { try { - await fs.stat(path); - return true; + return (await fs.stat(path)).isDirectory() ? 'directory' : 'file'; + } catch (err) { + const code = (err as NodeJS.ErrnoException)?.code; + return code === 'ENOENT' || code === 'ENOTDIR' ? 'absent' : 'unknown'; + } +} + +function isWithin(path: string, root: string): boolean { + if (path === root) return true; + return path.startsWith(root.endsWith(sep) ? root : root + sep); +} + +/** Deepest mount point holding `abs`, from the kernel's mount table; undefined when unreadable. */ +function mountPointOf(abs: string): string | undefined { + let table: string; + try { + table = readFileSync('/proc/self/mounts', 'utf-8'); } catch { - return false; + return undefined; + } + let best: string | undefined; + for (const line of table.split('\n')) { + const field = line.split(' ')[1]; + if (!field) continue; + // The table octal-escapes space, tab, newline and backslash in mount points. + const mountPoint = field.replace(/\\([0-7]{3})/g, (_m, oct: string) => String.fromCharCode(parseInt(oct, 8))); + if (isWithin(abs, mountPoint) && (!best || mountPoint.length > best.length)) best = mountPoint; } + return best; +} + +/** The subtree a stalled path takes down with it: its mount, else just itself (see the module comment). */ +function stallScope(abs: string): string { + const mountPoint = mountPointOf(abs); + return mountPoint && mountPoint !== '/' ? mountPoint : abs; } /** - * Resolve whether `path` exists without letting an unresponsive filesystem - * block the caller for longer than `PROBE_TIMEOUT_MS`. + * Whether `path` is near a path whose probe is still stalled (see the module + * comment), i.e. whether the probe would answer "unknown" for it without a stat. + * Lets a caller tell "this workspace sits on the dead mount" apart from "the + * probe was refused for capacity". */ -export async function boundedPathExists(path: string): Promise { - if (stalled.has(path)) return false; +export function isNearStalledPath(path: string): boolean { + const abs = resolve(path); + for (const scope of stalled.values()) { + if (isWithin(abs, scope)) return true; + } + return false; +} + +/** Options for {@link probePathKind} / {@link probePath}. */ +export interface PathProbeOptions { + /** Probe even while the stall cap is engaged (see the module comment). */ + pastCap?: boolean; +} - let probe = inFlight.get(path); +/** + * Probe `path` without letting an unresponsive filesystem block the caller for + * longer than `PATH_PROBE_TIMEOUT_MS`. Follows symlinks, like `stat()`. + */ +export async function probePathKind(path: string, options: PathProbeOptions = {}): Promise { + const abs = resolve(path); + if (isNearStalledPath(abs)) return 'unknown'; + + let probe = inFlight.get(abs); if (!probe) { - if (stalled.size >= MAX_STALLED_PROBES) return false; - probe = statExists(path); - inFlight.set(path, probe); - void probe.finally(() => { - inFlight.delete(path); - stalled.delete(path); + if (stalled.size >= MAX_STALLED_PATH_PROBES && !options.pastCap) { + if (!capWarned) { + capWarned = true; + console.warn( + `[path-probe] ${stalled.size} path probes are stalled on unresponsive filesystems; ` + + 'not starting new ones until one answers (paths read as unknown meanwhile)' + ); + } + return 'unknown'; + } + probe = statKind(abs); + const started = probe; + inFlight.set(abs, started); + void started.finally(() => { + inFlight.delete(abs); + stalled.delete(abs); + if (stalled.size < MAX_STALLED_PATH_PROBES) capWarned = false; }); } @@ -68,11 +154,17 @@ export async function boundedPathExists(path: string): Promise { try { return await Promise.race([ probe, - new Promise((resolve) => { + new Promise((resolveTimeout) => { timer = setTimeout(() => { - if (inFlight.get(path) === probe) stalled.add(path); - resolve(false); - }, PROBE_TIMEOUT_MS); + if (inFlight.get(abs) === probe && !stalled.has(abs)) { + stalled.set(abs, stallScope(abs)); + console.warn( + `[path-probe] ${abs} did not answer within ${PATH_PROBE_TIMEOUT_MS} ms ` + + '(unreachable mount?); treating it and its neighbours as unknown until it does' + ); + } + resolveTimeout('unknown'); + }, PATH_PROBE_TIMEOUT_MS); timer.unref?.(); }), ]); @@ -80,3 +172,19 @@ export async function boundedPathExists(path: string): Promise { if (timer) clearTimeout(timer); } } + +/** Tri-state probe of `path`; see the module comment for what "unknown" means. */ +export async function probePath(path: string, options: PathProbeOptions = {}): Promise { + const kind = await probePathKind(path, options); + return kind === 'directory' || kind === 'file' ? 'present' : kind; +} + +/** + * `true` only when `path` is known to exist. For DISPLAY decisions only (does a + * case have a CLAUDE.md): it folds "unknown" into `false`, so never use it to + * decide that something is absent and may be created, scaffolded or reported + * missing; use {@link probePath} for that. + */ +export async function boundedPathExists(path: string): Promise { + return (await probePath(path)) === 'present'; +} diff --git a/src/utils/index.ts b/src/utils/index.ts index 502c5c35a..2c30f417e 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -68,3 +68,5 @@ export type { DeepSeekProfile, DeepSeekProfileKind } from './deepseek-cli-resolv export { compileFileQuery, matchFileQuery } from './file-query.js'; export type { FileQueryMatcher } from './file-query.js'; export { resolveOmpDir, isOmpAvailable, getOmpNotFoundMessage, getOmpCliVersion } from './omp-cli-resolver.js'; +export { boundedPathExists, probePath, probePathKind, isNearStalledPath } from './bounded-path-probe.js'; +export type { PathProbeState, PathProbeKind, PathProbeOptions } from './bounded-path-probe.js'; diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 27a74ec8b..5aa69ae44 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -1874,10 +1874,14 @@ Object.assign(CodemanApp.prototype, { try { // Get case path first const caseRes = await fetch(`/api/cases/${caseName}`); - let caseData = (await caseRes.json())?.data ?? {}; + const caseLookup = await caseRes.json(); + let caseData = caseLookup?.data ?? {}; - // Create the case if it doesn't exist + // Create the case only when the server says it does not exist. Any other + // failure (a linked folder on a mount that is not answering) must not + // scaffold a same-name local case that would then shadow the real one. if (!caseData.path) { + if (caseLookup?.errorCode !== 'NOT_FOUND') throw new Error(caseLookup?.error || 'Case lookup failed'); const createCaseRes = await fetch('/api/cases', { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -2084,10 +2088,14 @@ Object.assign(CodemanApp.prototype, { try { // Get the case path const caseRes = await fetch(`/api/cases/${caseName}`); - let caseData = (await caseRes.json())?.data ?? {}; + const caseLookup = await caseRes.json(); + let caseData = caseLookup?.data ?? {}; - // Create the case if it doesn't exist + // Create the case only when the server says it does not exist. Any other + // failure (a linked folder on a mount that is not answering) must not + // scaffold a same-name local case that would then shadow the real one. if (!caseData.path) { + if (caseLookup?.errorCode !== 'NOT_FOUND') throw new Error(caseLookup?.error || 'Case lookup failed'); const createCaseRes = await fetch('/api/cases', { method: 'POST', headers: { 'Content-Type': 'application/json' }, diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 7aed82ac2..8b8483f19 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -50,7 +50,7 @@ import { } from '../../git-clone.js'; import type { GitRemoteProbe, GitUrlParse } from '../../git-clone.js'; import { generateClaudeMd } from '../../templates/claude-md.js'; -import { boundedPathExists } from '../../utils/bounded-path-probe.js'; +import { boundedPathExists, probePath } from '../../utils/index.js'; import { readAgentCaseMarker, type AgentCaseMarker } from '../../agent-case-marker.js'; import { settingsWriteBlocker, writeHooksConfig } from '../../hooks-config.js'; import { @@ -163,11 +163,11 @@ function gitDiagnosticLine(stderr: string): string { * hooks, which run on the user's machine when a session starts in the case, so * the clone response says so out loud instead of silently merging into them. */ -async function repoShipsClaudeSettings(casePath: string): Promise { - for (const file of ['settings.json', 'settings.local.json']) { - if (await boundedPathExists(join(casePath, '.claude', file))) return true; - } - return false; +function repoShipsClaudeSettings(casePath: string): boolean { + // Deliberately NOT the bounded path probe: the tree was just cloned into the + // local case space (and lstat'ed synchronously moments ago), so a bound protects + // nothing here, while a probe answering "unknown" could silently drop this warning. + return ['settings.json', 'settings.local.json'].some((file) => existsSync(join(casePath, '.claude', file))); } /** @@ -285,15 +285,19 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const existingNames = new Set(cases.map((c) => c.name)); if (admin) { for (const [name, path] of Object.entries(linkedCases)) { - if (!existingNames.has(name) && SAFE_CASE_NAME.test(name) && (await boundedPathExists(path))) { - cases.push({ - name, - path, - hasClaudeMd: await boundedPathExists(join(path, 'CLAUDE.md')), - linked: true, - location: 'linked-local', - }); - } + if (existingNames.has(name) || !SAFE_CASE_NAME.test(name)) continue; + const state = await probePath(path); + if (state === 'absent') continue; + // An unreachable linked case (a dead network mount) stays listed and says + // so: dropping it would read as "deleted" and invite a same-name local case. + cases.push({ + name, + path, + hasClaudeMd: state === 'present' && (await boundedPathExists(join(path, 'CLAUDE.md'))), + linked: true, + location: 'linked-local', + ...(state === 'unknown' ? { unreachable: true } : {}), + }); } } @@ -619,7 +623,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } else { warnings.push('Kept the repository’s own CLAUDE.md.'); } - if (await repoShipsClaudeSettings(casePath)) { + if (repoShipsClaudeSettings(casePath)) { warnings.push( 'This repository ships its own .claude/settings files. Codeman merged its hooks alongside them without removing anything — review them before starting a session, since repo-supplied hooks run on this machine.' ); @@ -1637,12 +1641,27 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } const casePath = await resolveCasePath(name, getAuthUser(req)); + const linked = casePath !== join(resolveCasesDir(getAuthUser(req)), name); - if (!(await boundedPathExists(casePath))) { + // NOT_FOUND means DEFINITELY absent: the Run button creates a case on it, so + // a path that merely did not answer (a dead network mount) must never get it. + // One path, asked for explicitly: probe it even while unrelated mounts are dead. + const state = await probePath(casePath, { pastCap: true }); + if (state === 'absent') { return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Case not found'); } + if (state === 'unknown') { + // The linked registry knows where the case lives, so say where, and that + // it is not answering. A local case has no such record to fall back on. + if (!linked) { + return createErrorResponse( + ApiErrorCode.OPERATION_FAILED, + `Case folder is not responding or not readable: ${casePath}` + ); + } + return { name, path: casePath, hasClaudeMd: false, linked: true, unreachable: true }; + } - const linked = casePath !== join(resolveCasesDir(getAuthUser(req)), name); return { name, path: casePath, @@ -1664,7 +1683,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const fixPlanPath = join(casePath, '@fix_plan.md'); - if (!(await boundedPathExists(fixPlanPath))) { + const fixPlanState = await probePath(fixPlanPath, { pastCap: true }); + if (fixPlanState === 'unknown') { + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, 'Case folder is not responding or not readable'); + } + if (fixPlanState === 'absent') { return { exists: false, content: null, todos: [] }; } diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index bb4bdbe0c..eeabfa969 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -174,6 +174,7 @@ import { toSessionDocker, } from '../../docker-hosts.js'; import { LRUMap } from '../../utils/lru-map.js'; +import { probePathKind } from '../../utils/index.js'; import { findLatestOmpSessionId } from '../../utils/omp-session-resolver.js'; import { scanOmpSessionsHistory } from '../../omp-transcript.js'; import { scanCodexSessionsHistory, codexThreadBySessionId } from '../../codex-transcript.js'; @@ -971,16 +972,23 @@ export function registerSessionRoutes( return createErrorResponse(ApiErrorCode.FORBIDDEN, 'workingDir is outside your workspace'); } - // Validate workingDir exists and is a directory + // Validate workingDir exists and is a directory. Bounded: a workingDir on a + // network mount that stopped answering must not freeze the event loop, and + // "did not answer" is reported as such, never as "does not exist". if (body.workingDir) { - try { - const stat = statSync(workingDir); - if (!stat.isDirectory()) { - return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'workingDir is not a directory'); - } - } catch { + const kind = await probePathKind(workingDir, { pastCap: true }); + if (kind === 'unknown') { + return createErrorResponse( + ApiErrorCode.OPERATION_FAILED, + `workingDir is not responding or not readable: ${workingDir}` + ); + } + if (kind === 'absent') { return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'workingDir does not exist'); } + if (kind !== 'directory') { + return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'workingDir is not a directory'); + } } // envOverrides flow through Session → tmux setenv (ephemeral, per-session). @@ -3694,9 +3702,21 @@ export function registerSessionRoutes( return createErrorResponse(ApiErrorCode.FORBIDDEN, 'case path is outside your workspace'); } + // Bounded probe of a local case folder: a linked case can sit on a network mount + // that stopped answering, and a synchronous check there froze the whole server. + // Only a DEFINITE absence may scaffold a new case; "did not answer" must not + // create one over the top of where the real case is mounted. + const localCaseState = remote || docker ? undefined : await probePathKind(resolvedCasePath, { pastCap: true }); + if (localCaseState === 'unknown') { + return createErrorResponse( + ApiErrorCode.OPERATION_FAILED, + `Case folder is not responding or not readable: ${resolvedCasePath}` + ); + } + // Create case folder and CLAUDE.md if it doesn't exist (only for non-linked, non-remote, // non-docker cases — docker workspaces are scaffolded in their own block below) - if (!remote && !docker && !existsSync(resolvedCasePath)) { + if (localCaseState === 'absent') { try { mkdirSync(resolvedCasePath, { recursive: true }); mkdirSync(join(resolvedCasePath, 'src'), { recursive: true }); diff --git a/test/bounded-path-probe.test.ts b/test/bounded-path-probe.test.ts index c44e46bf9..64b93f612 100644 --- a/test/bounded-path-probe.test.ts +++ b/test/bounded-path-probe.test.ts @@ -1,103 +1,257 @@ /** - * @fileoverview Tests for boundedPathExists (src/utils/bounded-path-probe.ts): + * @fileoverview Tests for the bounded path probe (src/utils/bounded-path-probe.ts): * a stat() that never settles (an unreachable hard network mount) must not hold - * the caller past the timeout, must not be re-issued while it is still pending, - * and must not let stalled probes pile up in libuv's shared threadpool. + * the caller past the timeout, must read as "unknown" rather than "absent", must + * not be re-issued while it is still pending, must not let stalled probes pile up + * in libuv's shared threadpool, and must not make unrelated healthy paths unknown. */ -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; vi.mock('node:fs/promises', () => ({ default: { stat: vi.fn() }, })); +// The kernel mount table the probe scopes a stall by. `/mnt/nas` and `/mnt/nas b` +// (a mount point with a space, octal-escaped in the table) are network mounts; +// everything else sits on the root filesystem. `null` = no table (not Linux). +const mounts = vi.hoisted(() => ({ + table: null as string | null, + default: [ + 'sysfs /sys sysfs rw 0 0', + '/dev/sda1 / ext4 rw 0 0', + 'nas:/export /mnt/nas nfs rw,hard 0 0', + 'nas:/other /mnt/nas\\040b nfs rw,hard 0 0', + '', + ].join('\n'), +})); +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + const readFileSync = ((path: unknown, ...rest: unknown[]) => { + if (String(path) === '/proc/self/mounts') { + if (mounts.table === null) throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); + return mounts.table; + } + return (actual.readFileSync as (...a: unknown[]) => unknown)(path, ...rest); + }) as typeof actual.readFileSync; + return { ...actual, readFileSync, default: { ...actual, readFileSync } }; +}); + import fs from 'node:fs/promises'; -import { boundedPathExists, PROBE_TIMEOUT_MS } from '../src/utils/bounded-path-probe.js'; +import { boundedPathExists, isNearStalledPath, probePath, probePathKind } from '../src/utils/bounded-path-probe.js'; +import { MAX_STALLED_PATH_PROBES, PATH_PROBE_TIMEOUT_MS } from '../src/config/path-probe.js'; const stat = vi.mocked(fs.stat); +const dirStats = { isDirectory: () => true } as never; +const fileStats = { isDirectory: () => false } as never; + +let releases: Map void>; +let warn: ReturnType; /** * Make the first stat() of each given path hang until released (the mount is - * down); every later stat, and every other path, answers "exists". + * down); every later stat, and every other path, answers "a directory exists". */ function hangOn(paths: string[]): Map void> { - const releases = new Map void>(); stat.mockImplementation((path) => { - if (!paths.includes(String(path)) || releases.has(String(path))) return Promise.resolve({} as never); + if (!paths.includes(String(path)) || releases.has(String(path))) return Promise.resolve(dirStats); return new Promise((resolve) => { - releases.set(String(path), () => resolve({} as never)); + releases.set(String(path), () => resolve(dirStats)); }); }); return releases; } -afterEach(() => { +/** Start probes for `paths` and let them time out, leaving each one stalled. */ +async function stall(paths: string[]): Promise { + const pending = paths.map((p) => probePath(p)); + await vi.advanceTimersByTimeAsync(PATH_PROBE_TIMEOUT_MS); + expect(await Promise.all(pending)).toEqual(paths.map(() => 'unknown')); +} + +beforeEach(() => { + mounts.table = mounts.default; + releases = new Map(); + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(async () => { + // Settle every stalled stat so module state does not leak into the next test. + releases.forEach((release) => release()); + if (vi.isFakeTimers()) await vi.advanceTimersByTimeAsync(0); + else await new Promise((r) => setTimeout(r, 0)); vi.useRealTimers(); stat.mockReset(); + warn.mockRestore(); }); -describe('boundedPathExists', () => { - it('reports an existing path as present and a missing one as absent', async () => { +describe('probePath', () => { + it('tells present, absent and unreadable apart', async () => { stat.mockImplementation(async (path) => { - if (String(path) === '/present') return {} as never; + if (String(path) === '/present') return dirStats; + if (String(path) === '/eio') throw Object.assign(new Error('EIO'), { code: 'EIO' }); + if (String(path) === '/notdir/child') throw Object.assign(new Error('ENOTDIR'), { code: 'ENOTDIR' }); throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }); }); + expect(await probePath('/present')).toBe('present'); + expect(await probePath('/missing')).toBe('absent'); + expect(await probePath('/notdir/child')).toBe('absent'); + // A soft mount that gave up answers EIO: that is not proof the path is gone. + expect(await probePath('/eio')).toBe('unknown'); expect(await boundedPathExists('/present')).toBe(true); expect(await boundedPathExists('/missing')).toBe(false); + expect(await boundedPathExists('/eio')).toBe(false); }); - it('answers false after the timeout when stat never settles, and does not re-probe until it does', async () => { + it('reports whether a present path is a directory', async () => { + stat.mockImplementation(async (path) => (String(path) === '/dir' ? dirStats : fileStats)); + expect(await probePathKind('/dir')).toBe('directory'); + expect(await probePathKind('/file')).toBe('file'); + }); + + it('answers unknown (not absent) after the timeout, and does not re-probe until the stat settles', async () => { vi.useFakeTimers(); - const releases = hangOn(['/mnt/stalled/case']); + hangOn(['/mnt/stalled/case']); - const result = boundedPathExists('/mnt/stalled/case'); - await vi.advanceTimersByTimeAsync(PROBE_TIMEOUT_MS); - expect(await result).toBe(false); + const result = probePath('/mnt/stalled/case'); + await vi.advanceTimersByTimeAsync(PATH_PROBE_TIMEOUT_MS); + expect(await result).toBe('unknown'); // A second caller gets the cached verdict immediately, without another stat. - expect(await boundedPathExists('/mnt/stalled/case')).toBe(false); + expect(await probePath('/mnt/stalled/case')).toBe('unknown'); expect(stat).toHaveBeenCalledTimes(1); // Once the mount answers, the path is probed afresh. releases.get('/mnt/stalled/case')!(); await vi.advanceTimersByTimeAsync(0); - expect(await boundedPathExists('/mnt/stalled/case')).toBe(true); + expect(await probePath('/mnt/stalled/case')).toBe('present'); expect(stat).toHaveBeenCalledTimes(2); }); it('shares one in-flight stat between concurrent callers of the same path', async () => { - const releases = hangOn(['/slow']); - const a = boundedPathExists('/slow'); + hangOn(['/slow']); + const a = probePath('/slow'); const b = boundedPathExists('/slow'); expect(stat).toHaveBeenCalledTimes(1); releases.get('/slow')!(); - expect(await a).toBe(true); + expect(await a).toBe('present'); expect(await b).toBe(true); }); it('does not give concurrent healthy probes a false negative', async () => { - stat.mockImplementation(async () => ({}) as never); + stat.mockImplementation(async () => dirStats); const results = await Promise.all(['/a', '/b', '/c', '/d', '/e'].map((p) => boundedPathExists(p))); expect(results).toEqual([true, true, true, true, true]); }); - it('stops issuing new stats once stalled probes would tie up the threadpool', async () => { + it('still probes a healthy path as present while two unrelated paths are stalled', async () => { + vi.useFakeTimers(); + hangOn(['/mnt/nas-a/project', '/mnt/nas-b/project']); + await stall(['/mnt/nas-a/project', '/mnt/nas-b/project']); + + expect(await probePath('/home/user/codeman-cases/healthy')).toBe('present'); + expect(await boundedPathExists('/home/user/codeman-cases/healthy/CLAUDE.md')).toBe(true); + expect(isNearStalledPath('/home/user/codeman-cases/healthy')).toBe(false); + }); + + it('answers unknown, without a stat, for paths near a stalled one', async () => { vi.useFakeTimers(); - const releases = hangOn(['/mnt/stalled/one', '/mnt/stalled/two']); + hangOn(['/mnt/nas/project-one']); + await stall(['/mnt/nas/project-one']); + stat.mockClear(); - const first = boundedPathExists('/mnt/stalled/one'); - const second = boundedPathExists('/mnt/stalled/two'); - await vi.advanceTimersByTimeAsync(PROBE_TIMEOUT_MS); - expect(await first).toBe(false); - expect(await second).toBe(false); + // Its own files, and a sibling linked case on the same mount. + expect(await probePath('/mnt/nas/project-one/CLAUDE.md')).toBe('unknown'); + expect(await probePath('/mnt/nas/project-two')).toBe('unknown'); + expect(isNearStalledPath('/mnt/nas/project-two/.claude/settings.local.json')).toBe(true); + expect(stat).not.toHaveBeenCalled(); - // Both slots are held by stats that never returned: refuse a third. - expect(await boundedPathExists('/healthy/three')).toBe(false); - expect(stat).toHaveBeenCalledTimes(2); + releases.get('/mnt/nas/project-one')!(); + await vi.advanceTimersByTimeAsync(0); + expect(isNearStalledPath('/mnt/nas/project-two')).toBe(false); + expect(await probePath('/mnt/nas/project-two')).toBe('present'); + }); + + it('reads octal-escaped mount points from the table', async () => { + vi.useFakeTimers(); + hangOn(['/mnt/nas b/one']); + await stall(['/mnt/nas b/one']); + expect(isNearStalledPath('/mnt/nas b/two')).toBe(true); + expect(isNearStalledPath('/mnt/nas/two')).toBe(false); + }); + + it('never takes the root filesystem down with a stalled path on it, only that path', async () => { + vi.useFakeTimers(); + hangOn(['/srv/projects/stuck']); + await stall(['/srv/projects/stuck']); + + expect(await probePath('/srv/projects/stuck/CLAUDE.md')).toBe('unknown'); + expect(await probePath('/srv/projects/other')).toBe('present'); + expect(await probePath('/home/user/codeman-cases/one')).toBe('present'); + }); + + it('narrows a stall to the stalled path when there is no mount table', async () => { + vi.useFakeTimers(); + mounts.table = null; + hangOn(['/mnt/nas/project-one']); + await stall(['/mnt/nas/project-one']); + + expect(await probePath('/mnt/nas/project-one/CLAUDE.md')).toBe('unknown'); + expect(await probePath('/mnt/nas/project-two')).toBe('present'); + }); + + it('refuses new stats once stalled probes would tie up the threadpool, answering unknown', async () => { + vi.useFakeTimers(); + const dead = Array.from({ length: MAX_STALLED_PATH_PROBES }, (_, i) => `/mnt/dead-${i}/case`); + hangOn(dead); + await stall(dead); + stat.mockClear(); + + // Every slot is held by a stat that never returned: refuse another, but never + // claim the path is absent. + expect(await probePath('/healthy/elsewhere')).toBe('unknown'); + expect(stat).not.toHaveBeenCalled(); // Once the stalled stats settle, probing resumes normally. releases.forEach((release) => release()); await vi.advanceTimersByTimeAsync(0); - expect(await boundedPathExists('/healthy/three')).toBe(true); - expect(stat).toHaveBeenCalledTimes(3); + expect(await probePath('/healthy/elsewhere')).toBe('present'); + expect(stat).toHaveBeenCalledTimes(1); + }); + + it('lets a pastCap probe through the cap, still bounded and still recorded as stalled', async () => { + vi.useFakeTimers(); + const dead = Array.from({ length: MAX_STALLED_PATH_PROBES }, (_, i) => `/mnt/full-${i}/case`); + hangOn([...dead, '/mnt/another-dead/case']); + await stall(dead); + stat.mockClear(); + + expect(await probePath('/healthy/explicit', { pastCap: true })).toBe('present'); + + const hung = probePath('/mnt/another-dead/case', { pastCap: true }); + await vi.advanceTimersByTimeAsync(PATH_PROBE_TIMEOUT_MS); + expect(await hung).toBe('unknown'); + // A retry is answered from the stall record, not with another stat. + expect(await probePath('/mnt/another-dead/case', { pastCap: true })).toBe('unknown'); + expect(stat).toHaveBeenCalledTimes(2); + }); + + it('warns once when a path first stalls and once when the cap engages', async () => { + vi.useFakeTimers(); + const dead = Array.from({ length: MAX_STALLED_PATH_PROBES }, (_, i) => `/mnt/gone-${i}/case`); + hangOn(dead); + await stall([dead[0]]); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain('/mnt/gone-0/case'); + + // Asking again about the same stalled path does not warn again. + await probePath(dead[0]); + expect(warn).toHaveBeenCalledTimes(1); + + await stall(dead.slice(1)); + warn.mockClear(); + await probePath('/healthy/one'); + await probePath('/healthy/two'); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toMatch(/stalled/i); }); }); diff --git a/test/routes/case-clone-routes.test.ts b/test/routes/case-clone-routes.test.ts index ab0c3414a..915020b61 100644 --- a/test/routes/case-clone-routes.test.ts +++ b/test/routes/case-clone-routes.test.ts @@ -17,7 +17,28 @@ * Port: N/A (app.inject). */ -import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeAll, afterAll, beforeEach, afterEach, vi } from 'vitest'; + +// Unreachable-mount seam: `stat()` of a path under this root never settles (a hard +// network mount that went away), so the bounded path probe can be driven to its +// stall cap. Every other stat is the real one. The short timeout is read at import. +const deadMount = vi.hoisted(() => { + process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS = '200'; + return { root: '/mnt/codeman-clone-test-dead', releases: [] as Array<() => void> }; +}); +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + const stat = ((path: string, ...rest: unknown[]) => { + if (String(path).startsWith(deadMount.root + '/')) { + return new Promise((resolve) => deadMount.releases.push(() => resolve({} as never))); + } + return (actual.stat as (...a: unknown[]) => unknown)(path, ...rest); + }) as typeof actual.stat; + return { ...actual, stat, default: { ...actual, stat } }; +}); +afterAll(() => { + delete process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS; +}); import Fastify, { type FastifyInstance } from 'fastify'; import fastifyCookie from '@fastify/cookie'; import { execFileSync } from 'node:child_process'; @@ -38,6 +59,8 @@ import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js'; import { registerCaseRoutes } from '../../src/web/routes/case-routes.js'; import { isGitAvailable } from '../../src/git-clone.js'; +import { probePath } from '../../src/utils/index.js'; +import { MAX_STALLED_PATH_PROBES } from '../../src/config/path-probe.js'; const CASES_DIR = join(homedir(), 'codeman-cases'); const gitPresent = isGitAvailable(); @@ -252,6 +275,25 @@ describe.skipIf(!gitPresent)('POST /api/cases/clone — real clone', () => { expect(body.data.warnings.join(' ')).toMatch(/ships its own \.claude/); }); + it('still warns about repo-supplied .claude settings while unrelated mounts are unreachable', async () => { + const dead = Array.from({ length: MAX_STALLED_PATH_PROBES }, (_, i) => `${deadMount.root}/nas-${i}/project`); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + // Engage the probe's stall cap: every new bounded probe is now refused. + expect(await Promise.all(dead.map((p) => probePath(p)))).toEqual(dead.map(() => 'unknown')); + + created.push('warns-under-cap'); + const res = await clone({ name: 'warns-under-cap', repository: origin }); + const body = JSON.parse(res.body); + expect(body.success).toBe(true); + expect(body.data.warnings.join(' ')).toMatch(/ships its own \.claude/); + } finally { + deadMount.releases.splice(0).forEach((release) => release()); + await new Promise((r) => setTimeout(r, 0)); + warn.mockRestore(); + } + }); + it('installs Codeman hooks alongside whatever the repo shipped', async () => { created.push('hooked'); await clone({ name: 'hooked', repository: origin }); diff --git a/test/routes/case-routes.test.ts b/test/routes/case-routes.test.ts index 4315e0a81..5a78272eb 100644 --- a/test/routes/case-routes.test.ts +++ b/test/routes/case-routes.test.ts @@ -13,13 +13,24 @@ * behavior matches production exactly). */ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, afterAll, vi } from 'vitest'; import Fastify, { type FastifyInstance } from 'fastify'; import fastifyCookie from '@fastify/cookie'; import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js'; import { registerCaseRoutes } from '../../src/web/routes/case-routes.js'; +import { probePath } from '../../src/utils/index.js'; +import { MAX_STALLED_PATH_PROBES } from '../../src/config/path-probe.js'; + +// A short path-probe timeout keeps the unreachable-mount tests quick. Read when the +// probe's config module is first imported, so it is set before any import runs. +vi.hoisted(() => { + process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS = '300'; +}); +afterAll(() => { + delete process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS; +}); // Mock filesystem modules vi.mock('node:fs', async (importOriginal) => { @@ -251,8 +262,19 @@ describe('case-routes', () => { expect(res.statusCode).toBe(200); expect(elapsed).toBeLessThan(BLOCK_MS - 1_000); - // The unreachable case is left out rather than holding the list hostage. - expect(JSON.parse(res.body).data).toEqual([]); + // The unreachable case is listed as such rather than holding the list + // hostage, or vanishing as though it had been deleted. + expect(JSON.parse(res.body).data).toEqual([ + { + name: 'linked-nfs', + path: stalledPath, + hasClaudeMd: false, + linked: true, + location: 'linked-local', + unreachable: true, + }, + ]); + await new Promise((r) => setTimeout(r, 0)); // let the released stat clear its stall }); }); @@ -714,6 +736,64 @@ describe('case-routes', () => { expect(body.data.name).toBe('regular-case'); }); + it('answers a linked case on an unreachable mount with its registered path, not NOT_FOUND', async () => { + // The timeout path: the mount does not answer at all. + const stalledPath = '/mnt/unreachable/linked-get'; + mockedReadFile.mockResolvedValue(JSON.stringify({ 'linked-get': stalledPath }) as never); + let release: (() => void) | undefined; + mockedStat.mockImplementation((p) => { + if (String(p) !== stalledPath) { + return Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + } + return new Promise((resolve) => { + release = () => resolve({ isDirectory: () => true } as never); + }); + }); + + const res = await harness.app.inject({ method: 'GET', url: '/api/cases/linked-get' }); + release?.(); + await new Promise((r) => setTimeout(r, 0)); // let the released stat clear its stall + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body); + expect(body.success).toBe(true); + expect(body.data).toMatchObject({ name: 'linked-get', path: stalledPath, linked: true, unreachable: true }); + }); + + it('still answers a healthy case while unrelated mounts are stalled past the cap', async () => { + const dead = Array.from({ length: MAX_STALLED_PATH_PROBES }, (_, i) => `/mnt/dead-${i}/linked`); + const releases: Array<() => void> = []; + mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + mockedStat.mockImplementation((p) => { + if (dead.includes(String(p))) { + return new Promise((resolve) => releases.push(() => resolve({ isDirectory: () => true } as never))); + } + return Promise.resolve({ isDirectory: () => true } as never); + }); + expect(await Promise.all(dead.map((p) => probePath(p)))).toEqual(dead.map(() => 'unknown')); + + const res = await harness.app.inject({ method: 'GET', url: '/api/cases/healthy-local' }); + releases.forEach((release) => release()); + await new Promise((r) => setTimeout(r, 0)); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).data).toMatchObject({ name: 'healthy-local' }); + expect(JSON.parse(res.body).data.unreachable).toBeUndefined(); + }); + + it('answers a local case it cannot read with a non-NOT_FOUND error', async () => { + // A soft mount that gave up (EIO) is not proof the case is gone, and the Run + // button creates a case on NOT_FOUND. + mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + mockedStat.mockRejectedValue(Object.assign(new Error('EIO'), { code: 'EIO' })); + + const res = await harness.app.inject({ method: 'GET', url: '/api/cases/eio-case' }); + const body = JSON.parse(res.body); + expect(body.success).toBe(false); + expect(body.errorCode).toBe('OPERATION_FAILED'); + expect(res.statusCode).not.toBe(404); + }); + it('returns error when case not found anywhere', async () => { mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); mockedExistsSync.mockReturnValue(false); @@ -748,6 +828,16 @@ describe('case-routes', () => { expect(body.data.todos).toEqual([]); }); + it('reports an unreadable fix plan as an error, not as "no plan"', async () => { + mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + mockedStat.mockRejectedValue(Object.assign(new Error('EIO'), { code: 'EIO' })); + + const res = await harness.app.inject({ method: 'GET', url: '/api/cases/my-case/fix-plan' }); + const body = JSON.parse(res.body); + expect(body.success).toBe(false); + expect(body.errorCode).toBe('OPERATION_FAILED'); + }); + it('parses fix plan with todos and stats', async () => { const fixPlanContent = [ '# Fix Plan', diff --git a/test/routes/session-create-unreachable-path.test.ts b/test/routes/session-create-unreachable-path.test.ts new file mode 100644 index 000000000..2160674d0 --- /dev/null +++ b/test/routes/session-create-unreachable-path.test.ts @@ -0,0 +1,174 @@ +/** + * @fileoverview Session creation must not freeze the server on a workspace whose + * network mount has gone away (`POST /api/sessions` with a `workingDir` on it, and + * `POST /api/quick-start` for a linked case that lives there), and must not treat + * "did not answer" as "does not exist" (quick-start would scaffold a fresh case + * over the top of where the real one is mounted). + * + * A hard mount that stopped answering is simulated two ways, matching how each + * API behaves on one: a synchronous probe (`existsSync`/`statSync`/`mkdirSync`) + * busy-waits, freezing the event loop, and an async `stat()` never settles. + * + * Uses app.inject(), so no real HTTP port is needed. + */ +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; + +const dead = vi.hoisted(() => { + // Short probe timeout so a stalled stat costs ~200 ms here. Read at import. + process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS = '200'; + return { + root: '/mnt/codeman-test-dead-mount', + blockMs: 3_000, + syncTouches: [] as string[], + releases: [] as Array<() => void>, + }; +}); + +function onDeadMount(path: unknown): boolean { + const p = String(path); + return p === dead.root || p.startsWith(dead.root + '/'); +} + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + const freezeOn = + unknown>(fn: T) => + (...args: Parameters): ReturnType => { + if (onDeadMount(args[0])) { + dead.syncTouches.push(String(args[0])); + const until = Date.now() + dead.blockMs; + while (Date.now() < until) { + // spin: the event loop is frozen for as long as the mount does not answer + } + throw Object.assign(new Error('EIO'), { code: 'EIO' }); + } + return fn(...args) as ReturnType; + }; + const existsSync = freezeOn(actual.existsSync); + const statSync = freezeOn(actual.statSync as (...args: never[]) => unknown); + const mkdirSync = freezeOn(actual.mkdirSync as (...args: never[]) => unknown); + return { + ...actual, + existsSync, + statSync, + mkdirSync, + default: { ...actual, existsSync, statSync, mkdirSync }, + }; +}); + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + const stat = ((path: string, ...rest: unknown[]) => { + if (onDeadMount(path)) { + return new Promise((_resolve, reject) => { + dead.releases.push(() => reject(Object.assign(new Error('EIO'), { code: 'EIO' }))); + }); + } + return (actual.stat as (...a: unknown[]) => unknown)(path, ...rest); + }) as typeof actual.stat; + return { ...actual, stat, default: { ...actual, stat } }; +}); + +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { createMockRouteContext } from '../mocks/index.js'; +import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; +import { registerSessionRoutes } from '../../src/web/routes/session-routes.js'; +import { dataPath } from '../../src/config/instance.js'; + +describe('session creation on an unreachable mount', () => { + let app: FastifyInstance; + let scratch: string; + let warn: ReturnType; + + beforeEach(async () => { + dead.syncTouches.length = 0; + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + scratch = await mkdtemp(join(tmpdir(), 'codeman-unreachable-create-')); + app = Fastify({ logger: false }); + await app.register(fastifyCookie); + registerSessionRoutes(app, createMockRouteContext() as never); + installRouteErrorHandler(app); + await app.ready(); + }); + + afterEach(async () => { + await app.close(); + dead.releases.splice(0).forEach((release) => release()); + await new Promise((r) => setTimeout(r, 0)); + await rm(scratch, { recursive: true, force: true }); + await rm(dataPath('linked-cases.json'), { force: true }); + warn.mockRestore(); + }); + + afterAll(() => { + delete process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS; + }); + + it('POST /api/sessions answers promptly, and not as "does not exist", for a workingDir on a dead mount', async () => { + const started = Date.now(); + const res = await app.inject({ + method: 'POST', + url: '/api/sessions', + payload: { name: 'dead-mount', mode: 'shell', workingDir: `${dead.root}/project` }, + }); + const elapsed = Date.now() - started; + + expect(elapsed).toBeLessThan(dead.blockMs - 1_000); + expect(dead.syncTouches).toEqual([]); + const body = JSON.parse(res.body); + expect(body.success).toBe(false); + expect(body.errorCode).toBe('OPERATION_FAILED'); + expect(body.error).toMatch(/not responding/i); + }); + + it('POST /api/sessions keeps INVALID_INPUT for a missing workingDir and for a file', async () => { + const file = join(scratch, 'a-file.txt'); + await writeFile(file, 'x'); + + const missing = await app.inject({ + method: 'POST', + url: '/api/sessions', + payload: { name: 'missing', mode: 'shell', workingDir: join(scratch, 'nope') }, + }); + expect(JSON.parse(missing.body)).toMatchObject({ + success: false, + errorCode: 'INVALID_INPUT', + error: 'workingDir does not exist', + }); + + const notDir = await app.inject({ + method: 'POST', + url: '/api/sessions', + payload: { name: 'file', mode: 'shell', workingDir: file }, + }); + expect(JSON.parse(notDir.body)).toMatchObject({ + success: false, + errorCode: 'INVALID_INPUT', + error: 'workingDir is not a directory', + }); + }); + + it('POST /api/quick-start refuses, promptly and without scaffolding, a linked case on a dead mount', async () => { + await writeFile(dataPath('linked-cases.json'), JSON.stringify({ 'nas-linked': `${dead.root}/linked` })); + + const started = Date.now(); + const res = await app.inject({ + method: 'POST', + url: '/api/quick-start', + payload: { caseName: 'nas-linked', mode: 'shell' }, + }); + const elapsed = Date.now() - started; + + expect(elapsed).toBeLessThan(dead.blockMs - 1_000); + // Neither probed nor created synchronously on the dead mount. + expect(dead.syncTouches).toEqual([]); + const body = JSON.parse(res.body); + expect(body.success).toBe(false); + expect(body.errorCode).toBe('OPERATION_FAILED'); + expect(body.error).toMatch(/not responding/i); + }); +}); diff --git a/test/run-mode-ui.test.ts b/test/run-mode-ui.test.ts index 7f8224791..5b96f3891 100644 --- a/test/run-mode-ui.test.ts +++ b/test/run-mode-ui.test.ts @@ -1227,4 +1227,66 @@ describe('Grok quick start', () => { expect(names).toEqual(['w1-grok-case', 'w2-grok-case', 'w3-grok-case']); expect(selected).toEqual(['sess-gk-0']); }); + + describe('case lookup before a local launch', () => { + function loadLaunchHarness(caseAnswer: Record) { + const elements: Record = { + quickStartCase: { value: 'nas-case' }, + shellCount: { value: '1' }, + tabCount: { value: '1' }, + }; + const requests: Array<{ url: string; method?: string }> = []; + const written: string[] = []; + const CodemanApp = function CodemanApp(this: any) {}; + const context = vm.createContext({ + CodemanApp, + localStorage: { getItem: () => null, setItem: () => {} }, + document: { getElementById: (id: string) => elements[id] ?? null }, + fetch: async (url: string, init?: { method?: string }) => { + requests.push({ url, method: init?.method }); + if (url === '/api/cases/nas-case') return { json: async () => caseAnswer }; + if (url === '/api/cases' && init?.method === 'POST') { + return { + json: async () => ({ + success: true, + data: { case: { name: 'nas-case', path: '/home/u/codeman-cases/nas-case' } }, + }), + }; + } + // Anything past the case lookup is out of scope here: stop the launch. + throw new Error(`stop: ${url}`); + }, + console, + }); + const sessionUi = readFileSync(resolve(import.meta.dirname, '../src/web/public/session-ui.js'), 'utf8'); + vm.runInContext(sessionUi, context, { filename: 'session-ui.js' }); + const app = new (CodemanApp as any)(); + app.terminal = { clear: () => {}, writeln: (line: string) => written.push(line), focus: () => {} }; + app.sessions = new Map(); + app.cases = []; + app.getTerminalDimensions = () => null; + app._readTabCount = () => 1; + app.loadAppSettingsFromStorage = () => ({}); + app.getCaseSettings = () => ({}); + return { app, requests, written }; + } + + const unreachable = { success: false, error: 'Case folder is not responding', errorCode: 'OPERATION_FAILED' }; + const missing = { success: false, error: 'Case not found', errorCode: 'NOT_FOUND' }; + + for (const launcher of ['runClaude', 'runShell'] as const) { + it(`${launcher} never creates a case when the lookup could not tell whether it exists`, async () => { + const { app, requests, written } = loadLaunchHarness(unreachable); + await app[launcher](); + expect(requests.some((r) => r.url === '/api/cases' && r.method === 'POST')).toBe(false); + expect(written.join('\n')).toContain('Case folder is not responding'); + }); + + it(`${launcher} creates the case when the lookup says it does not exist`, async () => { + const { app, requests } = loadLaunchHarness(missing); + await app[launcher](); + expect(requests.some((r) => r.url === '/api/cases' && r.method === 'POST')).toBe(true); + }); + } + }); }); diff --git a/test/workspace-hooks-unreachable-mount.test.ts b/test/workspace-hooks-unreachable-mount.test.ts new file mode 100644 index 000000000..a9da2b543 --- /dev/null +++ b/test/workspace-hooks-unreachable-mount.test.ts @@ -0,0 +1,145 @@ +/** + * @fileoverview How the workspace hook and statusLine helpers in hooks-config.ts + * read an "unknown" answer from the bounded path probe. A dead network mount + * elsewhere on the machine (enough of them to engage the probe's stall cap) must + * not stop Codeman's hooks from being installed in a healthy workspace, and must + * not let the plan-usage exporter be injected over a user's own statusLine. A + * workspace that IS on the dead mount is skipped without hanging the caller. + * + * Real temp directories; only `stat()` of the chosen dead paths is made to hang. + * Port: none. + */ +import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { existsSync, mkdtempSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const probe = vi.hoisted(() => { + // Short probe timeout so the stalls below cost ~100 ms each, read at import. + process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS = '100'; + return { dead: new Set(), releases: [] as Array<() => void> }; +}); + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + const stat = ((path: string, ...rest: unknown[]) => { + for (const dead of probe.dead) { + if (String(path) === dead || String(path).startsWith(dead + '/')) { + return new Promise((resolve, reject) => { + probe.releases.push(() => reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))); + void resolve; + }); + } + } + return (actual.stat as (...a: unknown[]) => unknown)(path, ...rest); + }) as typeof actual.stat; + return { ...actual, stat, default: { ...actual, stat } }; +}); + +import { applyWorkspaceHooks, resolveStatusLineCliCommand, stripCaseEnvKeys } from '../src/hooks-config.js'; +import { probePath } from '../src/utils/index.js'; +import { MAX_STALLED_PATH_PROBES } from '../src/config/path-probe.js'; + +const root = mkdtempSync(join(tmpdir(), 'codeman-unreachable-mount-')); + +/** Stall `count` paths on unrelated "mounts" until afterEach releases them. */ +async function stallUnrelatedMounts(count: number): Promise { + const paths = Array.from({ length: count }, (_, i) => `/mnt/dead-nas-${i}/project`); + paths.forEach((p) => probe.dead.add(p)); + expect(await Promise.all(paths.map((p) => probePath(p)))).toEqual(paths.map(() => 'unknown')); +} + +let warn: ReturnType; + +beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(async () => { + probe.dead.clear(); + probe.releases.splice(0).forEach((release) => release()); + await new Promise((r) => setTimeout(r, 0)); + warn.mockRestore(); +}); + +afterAll(() => { + delete process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS; +}); + +describe('workspace helpers while other mounts are unreachable', () => { + it('installs hooks in a healthy workspace while the stall cap is engaged', async () => { + await stallUnrelatedMounts(MAX_STALLED_PATH_PROBES); + const workspace = join(root, 'healthy-a'); + mkdirSync(workspace); + + await applyWorkspaceHooks(workspace, true); + + const settings = join(workspace, '.claude', 'settings.local.json'); + expect(existsSync(settings)).toBe(true); + expect(readFileSync(settings, 'utf-8')).toContain('/api/hook-event'); + }); + + it('installs hooks in a healthy workspace while two unrelated paths are stalled', async () => { + await stallUnrelatedMounts(2); + const workspace = join(root, 'healthy-b'); + mkdirSync(workspace); + + await applyWorkspaceHooks(workspace, true); + + expect(existsSync(join(workspace, '.claude', 'settings.local.json'))).toBe(true); + }); + + it('skips, without hanging, a workspace that sits on the dead mount', async () => { + const workspace = '/mnt/dead-nas-x/project'; + probe.dead.add('/mnt/dead-nas-x'); + expect(await probePath(workspace)).toBe('unknown'); + + const started = Date.now(); + await applyWorkspaceHooks(join('/mnt/dead-nas-x', 'project'), true); + expect(Date.now() - started).toBeLessThan(1_000); + expect( + warn.mock.calls.some( + (c: unknown[]) => /hooks/i.test(String(c[0])) && String(c[0]).includes('/mnt/dead-nas-x/project') + ) + ).toBe(true); + }); + + it('removes a superseded env key from a healthy workspace while the stall cap is engaged', async () => { + await stallUnrelatedMounts(MAX_STALLED_PATH_PROBES); + const workspace = join(root, 'strip-env'); + mkdirSync(join(workspace, '.claude'), { recursive: true }); + const settings = join(workspace, '.claude', 'settings.local.json'); + writeFileSync(settings, JSON.stringify({ env: { CLAUDE_CODE_STALE: '1', USER_KEEP: '2' } })); + + await stripCaseEnvKeys(workspace, ['CLAUDE_CODE_STALE']); + + expect(JSON.parse(readFileSync(settings, 'utf-8')).env).toEqual({ USER_KEEP: '2' }); + }); + + it("never injects the exporter over a user's own statusLine while the cap is engaged", async () => { + await stallUnrelatedMounts(MAX_STALLED_PATH_PROBES); + const workspace = join(root, 'own-statusline'); + mkdirSync(join(workspace, '.claude'), { recursive: true }); + writeFileSync( + join(workspace, '.claude', 'settings.local.json'), + JSON.stringify({ statusLine: { type: 'command', command: 'my-own-statusline' } }) + ); + + expect(await resolveStatusLineCliCommand(workspace, true)).toBeUndefined(); + }); + + it('still injects the exporter in a healthy workspace without one while the cap is engaged', async () => { + await stallUnrelatedMounts(MAX_STALLED_PATH_PROBES); + const workspace = join(root, 'no-statusline'); + mkdirSync(workspace); + + expect(await resolveStatusLineCliCommand(workspace, true)).toMatch(/statusline-exporter\.sh$/); + }); + + it('does not inject the exporter into a workspace on the dead mount', async () => { + probe.dead.add('/mnt/dead-nas-y'); + expect(await probePath('/mnt/dead-nas-y/project')).toBe('unknown'); + + expect(await resolveStatusLineCliCommand('/mnt/dead-nas-y/project', true)).toBeUndefined(); + }); +}); From 9fa44109b87adf389e23a77b26c721b40a79f963 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Mon, 5 Oct 2026 09:51:27 -0400 Subject: [PATCH 3/3] fix(cases): keep deleted workspaces deleted, cap pastCap, scope stalls to network mounts - applyWorkspaceHooks: an "unknown" probe that is not near a stalled path (refused by the stall cap, or an unexpected stat error) no longer reads as "go ahead". It checks existence with pathExistsForWrite first, so a deleted workspace is not recreated by the mkdir -p in ensureCodemanHooks. - pastCap gets a hard ceiling, PATH_PROBE_STALL_CEILING = UV_THREADPOOL_SIZE (default 4) minus one, so explicit requests against several dead paths can never take the last libuv worker. The bulk cap now defaults to one below the ceiling (2 with the default pool), leaving a slot for an explicit request. - A stall widens to its mount only for network and FUSE filesystem types read from /proc/self/mounts; on a local mount (a path typed under a local /home that reaches a NAS through a symlink) it narrows to the stalled path. - GET /api/cases/:name probes CLAUDE.md with pastCap, like the folder probe. - Comment in config/path-probe.ts describes the mount-scoped stall. --- src/config/path-probe.ts | 29 +++++-- src/hooks-config.ts | 19 +++-- src/utils/bounded-path-probe.ts | 64 +++++++++++----- src/web/routes/case-routes.ts | 3 +- test/bounded-path-probe.test.ts | 75 ++++++++++++++++++- test/routes/case-routes.test.ts | 2 + .../workspace-hooks-unreachable-mount.test.ts | 17 ++++- 7 files changed, 171 insertions(+), 38 deletions(-) diff --git a/src/config/path-probe.ts b/src/config/path-probe.ts index 3c11a82d2..e69e9642b 100644 --- a/src/config/path-probe.ts +++ b/src/config/path-probe.ts @@ -10,8 +10,8 @@ * * Both are env-overridable, in the same style as the other config modules. A slow * but healthy mount (an sshfs that needs a couple of seconds on first touch) may want - * a longer timeout; a server started with a larger `UV_THREADPOOL_SIZE` can afford a - * higher stall cap. + * a longer timeout. The stall limits follow `UV_THREADPOOL_SIZE` on their own, so a + * server started with a larger pool gets a higher ceiling without further setup. * * @module config/path-probe */ @@ -26,10 +26,23 @@ function envInt(name: string, fallback: number, min: number, max: number): numbe export const PATH_PROBE_TIMEOUT_MS = envInt('CODEMAN_PATH_PROBE_TIMEOUT_MS', 1_500, 100, 60_000); /** - * Timed-out probes allowed to stay pending before new probes are refused (answered - * "unknown" without a stat). This is a backstop, not the main defence: a stalled - * path already takes its neighbours (same parent directory) out of probing, so the - * cap only engages once three UNRELATED places have stopped answering. The default - * leaves one of libuv's default four workers free for the rest of the process. + * Hard ceiling on timed-out probes left pending, for every caller, `pastCap` ones + * included: the threadpool size minus one, so a dead mount can never take the last + * worker. libuv sizes the pool from `UV_THREADPOOL_SIZE` (4 when unset). A pool of + * one cannot keep a worker free at all, so the ceiling never drops below one. */ -export const MAX_STALLED_PATH_PROBES = envInt('CODEMAN_PATH_PROBE_MAX_STALLED', 3, 1, 64); +export const PATH_PROBE_STALL_CEILING = Math.max(1, (Number(process.env.UV_THREADPOOL_SIZE) || 4) - 1); + +/** + * Timed-out probes allowed to stay pending before new BULK probes are refused + * (answered "unknown" without a stat). This is a backstop, not the main defence: a + * stalled path on a network or FUSE mount already takes the rest of that mount out + * of probing (a stall anywhere else takes out only the stalled path), so the cap + * only engages once that many UNRELATED places have stopped answering. It defaults + * to one below {@link PATH_PROBE_STALL_CEILING} (2 with the default pool), leaving a + * slot a `pastCap` probe may still use, and is never allowed above the ceiling. + */ +export const MAX_STALLED_PATH_PROBES = Math.min( + PATH_PROBE_STALL_CEILING, + envInt('CODEMAN_PATH_PROBE_MAX_STALLED', Math.max(1, PATH_PROBE_STALL_CEILING - 1), 1, 64) +); diff --git a/src/hooks-config.ts b/src/hooks-config.ts index 33f9c618b..5f9eabac7 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -852,13 +852,20 @@ export async function refreshStaleCodemanHooks(casePath: string): Promise */ export async function applyWorkspaceHooks(workspace: string, install?: boolean): Promise { try { - const skip = await absentOrUnreachable(workspace); - if (skip === 'unreachable') { - console.warn( - `[hooks] ${workspace} is not responding (unreachable mount?); Codeman hooks not checked or installed` - ); + const state = await probePath(workspace); + if (state === 'absent') return; + if (state === 'unknown') { + if (isNearStalledPath(workspace)) { + console.warn( + `[hooks] ${workspace} is not responding (unreachable mount?); Codeman hooks not checked or installed` + ); + return; + } + // Any other "unknown" (the stall cap refused the probe, or the stat failed + // with something other than ENOENT) proves nothing about existence, and the + // install below would mkdir -p a deleted repo back into being: ask directly. + if (!(await pathExistsForWrite(workspace))) return; } - if (skip) return; const shouldInstall = install ?? (await readWorkspaceHooksEnabled()); await (shouldInstall ? ensureCodemanHooks(workspace) : refreshStaleCodemanHooks(workspace)); } catch { diff --git a/src/utils/bounded-path-probe.ts b/src/utils/bounded-path-probe.ts index 5b2e7e58c..34f32adc7 100644 --- a/src/utils/bounded-path-probe.ts +++ b/src/utils/bounded-path-probe.ts @@ -21,11 +21,14 @@ * - a path whose probe timed out is "stalled" until that stat finally settles. * Paths NEAR a stalled one are answered "unknown" without a new stat, so one * dead mount costs one worker, not one per case and file on it. "Near" means on - * the same mount: under the deepest mount point holding the stalled path, read - * from `/proc/self/mounts` (procfs, which never waits on the dead filesystem). - * Where that table is unavailable (not Linux), or the deepest mount is `/`, it - * narrows to the stalled path and everything under it. Unrelated paths are - * probed normally; + * the same mount when that mount is a network or FUSE filesystem (NFS, SMB, + * sshfs and the like): under the deepest mount point holding the stalled path, + * with its type, read from `/proc/self/mounts` (procfs, which never waits on the + * dead filesystem). Otherwise it narrows to the stalled path and everything under + * it: when the deepest mount is local (a path typed under a local `/home` can + * reach a NAS through a symlink, and must not take the rest of `/home` with it), + * is `/`, or the table is unavailable (not Linux). Unrelated paths are probed + * normally; * - once `MAX_STALLED_PATH_PROBES` stalled stats are pending, new probes are * refused process-wide (answered "unknown"), since each would risk another * worker. Probes merely in flight do not count, so concurrent healthy probes @@ -34,7 +37,9 @@ * probe is still bounded and still recorded as stalled if it hangs (so a dead * path costs at most one worker however often it is retried), but it is not * refused just because unrelated mounts are dead. Bulk scans (the case list) - * and per-spawn helpers keep the cap. + * and per-spawn helpers keep the cap. `pastCap` still stops at + * `PATH_PROBE_STALL_CEILING` (the threadpool size minus one), so explicit + * requests against several dead paths can never take the last worker. * * Both events are logged once (`console.warn`): a path's first stall, and the * cap engaging, so "my case vanished" and "hooks stopped firing" leave a trace. @@ -49,7 +54,7 @@ import { readFileSync } from 'node:fs'; import fs from 'node:fs/promises'; import { resolve, sep } from 'node:path'; -import { MAX_STALLED_PATH_PROBES, PATH_PROBE_TIMEOUT_MS } from '../config/path-probe.js'; +import { MAX_STALLED_PATH_PROBES, PATH_PROBE_STALL_CEILING, PATH_PROBE_TIMEOUT_MS } from '../config/path-probe.js'; /** What a probe could establish about a path. */ export type PathProbeState = 'present' | 'absent' | 'unknown'; @@ -75,29 +80,53 @@ function isWithin(path: string, root: string): boolean { return path.startsWith(root.endsWith(sep) ? root : root + sep); } -/** Deepest mount point holding `abs`, from the kernel's mount table; undefined when unreadable. */ -function mountPointOf(abs: string): string | undefined { +/** Filesystem types whose stall means the whole mount is gone (network and FUSE). */ +const REMOTE_FS_TYPES = new Set([ + 'nfs', + 'nfs4', + 'cifs', + 'smb3', + 'smbfs', + '9p', + 'ceph', + 'glusterfs', + 'afs', + 'lustre', + 'davfs', +]); + +function isRemoteFsType(fsType: string): boolean { + return REMOTE_FS_TYPES.has(fsType) || fsType.startsWith('fuse.'); +} + +/** Deepest mount holding `abs`, from the kernel's mount table; undefined when unreadable. */ +function mountOf(abs: string): { mountPoint: string; fsType: string } | undefined { let table: string; try { table = readFileSync('/proc/self/mounts', 'utf-8'); } catch { return undefined; } - let best: string | undefined; + let best: { mountPoint: string; fsType: string } | undefined; for (const line of table.split('\n')) { - const field = line.split(' ')[1]; - if (!field) continue; + const [, field, fsType] = line.split(' '); + if (!field || !fsType) continue; // The table octal-escapes space, tab, newline and backslash in mount points. const mountPoint = field.replace(/\\([0-7]{3})/g, (_m, oct: string) => String.fromCharCode(parseInt(oct, 8))); - if (isWithin(abs, mountPoint) && (!best || mountPoint.length > best.length)) best = mountPoint; + if (isWithin(abs, mountPoint) && (!best || mountPoint.length > best.mountPoint.length)) { + best = { mountPoint, fsType }; + } } return best; } -/** The subtree a stalled path takes down with it: its mount, else just itself (see the module comment). */ +/** + * The subtree a stalled path takes down with it (see the module comment): its + * mount when that is a network or FUSE filesystem, else just the path itself. + */ function stallScope(abs: string): string { - const mountPoint = mountPointOf(abs); - return mountPoint && mountPoint !== '/' ? mountPoint : abs; + const mount = mountOf(abs); + return mount && mount.mountPoint !== '/' && isRemoteFsType(mount.fsType) ? mount.mountPoint : abs; } /** @@ -130,7 +159,8 @@ export async function probePathKind(path: string, options: PathProbeOptions = {} let probe = inFlight.get(abs); if (!probe) { - if (stalled.size >= MAX_STALLED_PATH_PROBES && !options.pastCap) { + // pastCap lifts the bulk cap, never the ceiling that keeps one worker free. + if (stalled.size >= (options.pastCap ? PATH_PROBE_STALL_CEILING : MAX_STALLED_PATH_PROBES)) { if (!capWarned) { capWarned = true; console.warn( diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 8b8483f19..3edabb0a5 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -1665,7 +1665,8 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config return { name, path: casePath, - hasClaudeMd: await boundedPathExists(join(casePath, 'CLAUDE.md')), + // Probed like the folder above, or a healthy case reads as having no CLAUDE.md under the cap. + hasClaudeMd: (await probePath(join(casePath, 'CLAUDE.md'), { pastCap: true })) === 'present', ...(linked && { linked: true }), }; }); diff --git a/test/bounded-path-probe.test.ts b/test/bounded-path-probe.test.ts index 64b93f612..ff85bf120 100644 --- a/test/bounded-path-probe.test.ts +++ b/test/bounded-path-probe.test.ts @@ -38,7 +38,7 @@ vi.mock('node:fs', async (importOriginal) => { import fs from 'node:fs/promises'; import { boundedPathExists, isNearStalledPath, probePath, probePathKind } from '../src/utils/bounded-path-probe.js'; -import { MAX_STALLED_PATH_PROBES, PATH_PROBE_TIMEOUT_MS } from '../src/config/path-probe.js'; +import { MAX_STALLED_PATH_PROBES, PATH_PROBE_STALL_CEILING, PATH_PROBE_TIMEOUT_MS } from '../src/config/path-probe.js'; const stat = vi.mocked(fs.stat); const dirStats = { isDirectory: () => true } as never; @@ -143,10 +143,11 @@ describe('probePath', () => { expect(results).toEqual([true, true, true, true, true]); }); - it('still probes a healthy path as present while two unrelated paths are stalled', async () => { + it('still probes a healthy path as present while fewer unrelated paths are stalled than the cap', async () => { vi.useFakeTimers(); - hangOn(['/mnt/nas-a/project', '/mnt/nas-b/project']); - await stall(['/mnt/nas-a/project', '/mnt/nas-b/project']); + const dead = Array.from({ length: MAX_STALLED_PATH_PROBES - 1 }, (_, i) => `/mnt/nas-${i}/project`); + hangOn(dead); + await stall(dead); expect(await probePath('/home/user/codeman-cases/healthy')).toBe('present'); expect(await boundedPathExists('/home/user/codeman-cases/healthy/CLAUDE.md')).toBe(true); @@ -189,6 +190,41 @@ describe('probePath', () => { expect(await probePath('/home/user/codeman-cases/one')).toBe('present'); }); + it('narrows a stall on a local mount to the stalled path, even when that mount is not /', async () => { + // /home is its own local filesystem; ~/nas is a symlink to a network mount, so + // the stalled path is typed under /home. Only network and FUSE mounts widen. + vi.useFakeTimers(); + mounts.table = [mounts.default, '/dev/sdb1 /home ext4 rw,relatime 0 0', ''].join('\n'); + hangOn(['/home/user/nas/project']); + await stall(['/home/user/nas/project']); + stat.mockClear(); + + expect(await probePath('/home/user/nas/project/CLAUDE.md')).toBe('unknown'); + expect(isNearStalledPath('/home/user/codeman-cases/one')).toBe(false); + expect(await probePath('/home/user/codeman-cases/one')).toBe('present'); + expect(await probePath('/home/user/nas/other')).toBe('present'); + expect(stat).toHaveBeenCalledTimes(2); + }); + + it('widens a stall to the whole mount for network and FUSE filesystems', async () => { + vi.useFakeTimers(); + mounts.table = [ + mounts.default, + 'nas:/four /srv/nas4 nfs4 rw,hard 0 0', + 'user@host:/ /srv/sshfs fuse.sshfs rw 0 0', + '//nas/share /srv/smb cifs rw 0 0', + '', + ].join('\n'); + const dead = ['/srv/nas4/one', '/srv/sshfs/one']; + hangOn(dead); + await stall(dead); + + expect(isNearStalledPath('/srv/nas4/two')).toBe(true); + expect(isNearStalledPath('/srv/sshfs/two')).toBe(true); + expect(isNearStalledPath('/srv/smb/two')).toBe(false); + expect(isNearStalledPath('/srv/elsewhere')).toBe(false); + }); + it('narrows a stall to the stalled path when there is no mount table', async () => { vi.useFakeTimers(); mounts.table = null; @@ -235,6 +271,37 @@ describe('probePath', () => { expect(stat).toHaveBeenCalledTimes(2); }); + it('stops pastCap probes at the threadpool ceiling, answering unknown without a stat', async () => { + vi.useFakeTimers(); + // Fill the bulk cap, then let pastCap probes stall until the ceiling is reached. + const dead = Array.from({ length: PATH_PROBE_STALL_CEILING + 1 }, (_, i) => `/mnt/ceiling-${i}/case`); + hangOn(dead); + await stall(dead.slice(0, MAX_STALLED_PATH_PROBES)); + for (const path of dead.slice(MAX_STALLED_PATH_PROBES, PATH_PROBE_STALL_CEILING)) { + const hung = probePath(path, { pastCap: true }); + await vi.advanceTimersByTimeAsync(PATH_PROBE_TIMEOUT_MS); + expect(await hung).toBe('unknown'); + } + stat.mockClear(); + + // One worker must stay free: no new stat, even for an explicit request. + const refused = probePath(dead[PATH_PROBE_STALL_CEILING], { pastCap: true }); + await vi.advanceTimersByTimeAsync(PATH_PROBE_TIMEOUT_MS); + expect(await refused).toBe('unknown'); + expect(stat).not.toHaveBeenCalled(); + expect(await probePath('/healthy/explicit', { pastCap: true })).toBe('unknown'); + expect(stat).not.toHaveBeenCalled(); + + // Once one stalled stat settles, an explicit request is probed again. + releases.get(dead[0])!(); + await vi.advanceTimersByTimeAsync(0); + expect(await probePath('/healthy/explicit', { pastCap: true })).toBe('present'); + }); + + it('keeps the bulk cap below the ceiling, so a pastCap probe has room', () => { + expect(MAX_STALLED_PATH_PROBES).toBeLessThan(PATH_PROBE_STALL_CEILING); + }); + it('warns once when a path first stalls and once when the cap engages', async () => { vi.useFakeTimers(); const dead = Array.from({ length: MAX_STALLED_PATH_PROBES }, (_, i) => `/mnt/gone-${i}/case`); diff --git a/test/routes/case-routes.test.ts b/test/routes/case-routes.test.ts index 5a78272eb..c0fdcce86 100644 --- a/test/routes/case-routes.test.ts +++ b/test/routes/case-routes.test.ts @@ -779,6 +779,8 @@ describe('case-routes', () => { expect(res.statusCode).toBe(200); expect(JSON.parse(res.body).data).toMatchObject({ name: 'healthy-local' }); expect(JSON.parse(res.body).data.unreachable).toBeUndefined(); + // Its CLAUDE.md is probed the same way as its folder, so it is not misreported missing. + expect(JSON.parse(res.body).data.hasClaudeMd).toBe(true); }); it('answers a local case it cannot read with a non-NOT_FOUND error', async () => { diff --git a/test/workspace-hooks-unreachable-mount.test.ts b/test/workspace-hooks-unreachable-mount.test.ts index a9da2b543..909f4625f 100644 --- a/test/workspace-hooks-unreachable-mount.test.ts +++ b/test/workspace-hooks-unreachable-mount.test.ts @@ -79,8 +79,21 @@ describe('workspace helpers while other mounts are unreachable', () => { expect(readFileSync(settings, 'utf-8')).toContain('/api/hook-event'); }); - it('installs hooks in a healthy workspace while two unrelated paths are stalled', async () => { - await stallUnrelatedMounts(2); + it('keeps a deleted workspace deleted while the stall cap is engaged', async () => { + // The cap refuses the probe ("unknown" without a stat), which must not read as + // "go ahead": installing would mkdir -p the deleted repo back into existence. + await stallUnrelatedMounts(MAX_STALLED_PATH_PROBES); + const workspace = join(root, 'deleted-repo'); + expect(await probePath(workspace)).toBe('unknown'); + + await applyWorkspaceHooks(workspace, true); + await applyWorkspaceHooks(workspace, false); + + expect(existsSync(workspace)).toBe(false); + }); + + it('installs hooks in a healthy workspace while fewer unrelated paths are stalled than the cap', async () => { + await stallUnrelatedMounts(MAX_STALLED_PATH_PROBES - 1); const workspace = join(root, 'healthy-b'); mkdirSync(workspace);