diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index 4892d3be..58114914 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 00000000..3c11a82d --- /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 fda526c7..33f9c618 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,39 @@ 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 { isNearStalledPath, probePath } from './utils/index.js'; + +/** + * 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. + */ +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; + } +} + +/** + * 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 @@ -558,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 (!existsSync(settingsPath)) return; + if (!(await pathExistsForWrite(settingsPath))) return; let existing: Record; try { @@ -590,7 +622,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 +653,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 +682,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 +730,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 +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 (!existsSync(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 { @@ -820,7 +852,13 @@ export async function refreshStaleCodemanHooks(casePath: string): Promise */ export async function applyWorkspaceHooks(workspace: string, install?: boolean): Promise { try { - if (!existsSync(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 { @@ -883,7 +921,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 +936,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 +995,7 @@ function statusLineExporterScriptContent(): string { } async function readStatusLineCommandFromFile(settingsPath: string): Promise { - if (!existsSync(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; @@ -1103,9 +1141,21 @@ export async function resolveStatusLineCliCommand( ): Promise { const settingsPath = join(casePath, '.claude', 'settings.local.json'); let userHasOwnStatusLine = false; - if (existsSync(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 1a6193c7..52a07640 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 new file mode 100644 index 00000000..5b2e7e58 --- /dev/null +++ b/src/utils/bounded-path-probe.ts @@ -0,0 +1,190 @@ +/** + * @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. + * + * 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. + * + * 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'; + +/** 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>(); +/** Stalled path -> the directory whose subtree is answered "unknown" while it stays stalled. */ +const stalled = new Map(); +let capWarned = false; + +async function statKind(path: string): Promise { + try { + 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 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; +} + +/** + * 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 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; +} + +/** + * 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_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; + }); + } + + let timer: ReturnType | undefined; + try { + return await Promise.race([ + probe, + new Promise((resolveTimeout) => { + timer = setTimeout(() => { + 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?.(); + }), + ]); + } finally { + 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 502c5c35..2c30f417 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 27a74ec8..5aa69ae4 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 b4802344..8b8483f1 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, probePath } from '../../utils/index.js'; import { readAgentCaseMarker, type AgentCaseMarker } from '../../agent-case-marker.js'; import { settingsWriteBlocker, writeHooksConfig } from '../../hooks-config.js'; import { @@ -163,6 +164,9 @@ function gitDiagnosticLine(stderr: string): string { * the clone response says so out loud instead of silently merging into them. */ 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))); } @@ -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,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) && existsSync(path)) { - cases.push({ - name, - path, - hasClaudeMd: existsSync(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 } : {}), + }); } } @@ -333,7 +341,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, @@ -1618,7 +1626,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, @@ -1633,16 +1641,31 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config } const casePath = await resolveCasePath(name, getAuthUser(req)); + const linked = casePath !== join(resolveCasesDir(getAuthUser(req)), name); - if (!existsSync(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, - hasClaudeMd: existsSync(join(casePath, 'CLAUDE.md')), + hasClaudeMd: await boundedPathExists(join(casePath, 'CLAUDE.md')), ...(linked && { linked: true }), }; }); @@ -1660,7 +1683,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const fixPlanPath = join(casePath, '@fix_plan.md'); - if (!existsSync(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 bb4bdbe0..eeabfa96 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 new file mode 100644 index 00000000..64b93f61 --- /dev/null +++ b/test/bounded-path-probe.test.ts @@ -0,0 +1,257 @@ +/** + * @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 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, 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, 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 "a directory exists". + */ +function hangOn(paths: string[]): Map void> { + stat.mockImplementation((path) => { + if (!paths.includes(String(path)) || releases.has(String(path))) return Promise.resolve(dirStats); + return new Promise((resolve) => { + releases.set(String(path), () => resolve(dirStats)); + }); + }); + return releases; +} + +/** 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('probePath', () => { + it('tells present, absent and unreadable apart', async () => { + stat.mockImplementation(async (path) => { + 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('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(); + hangOn(['/mnt/stalled/case']); + + 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 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 probePath('/mnt/stalled/case')).toBe('present'); + expect(stat).toHaveBeenCalledTimes(2); + }); + + it('shares one in-flight stat between concurrent callers of the same path', async () => { + hangOn(['/slow']); + const a = probePath('/slow'); + const b = boundedPathExists('/slow'); + expect(stat).toHaveBeenCalledTimes(1); + releases.get('/slow')!(); + expect(await a).toBe('present'); + expect(await b).toBe(true); + }); + + it('does not give concurrent healthy probes a false negative', async () => { + 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('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(); + hangOn(['/mnt/nas/project-one']); + await stall(['/mnt/nas/project-one']); + stat.mockClear(); + + // 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(); + + 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 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 ab0c3414..915020b6 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 56a89a2f..5a78272e 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) => { @@ -35,6 +46,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 +86,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 +140,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 +229,53 @@ 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 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 + }); }); describe('remote host and remote case routes', () => { @@ -670,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); @@ -704,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 00000000..2160674d --- /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 7f822479..5b96f389 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 00000000..a9da2b54 --- /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(); + }); +});