diff --git a/src/config/path-probe.ts b/src/config/path-probe.ts index 3c11a82d..e69e9642 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 33f9c618..5f9eabac 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 5b2e7e58..34f32adc 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 8b8483f1..3edabb0a 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 64b93f61..ff85bf12 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 5a78272e..c0fdcce8 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 a9da2b54..909f4625 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);