diff --git a/CLAUDE.md b/CLAUDE.md index 8199f78e..f5e52c57 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -146,6 +146,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph - **Default bind is loopback-only; non-loopback without a password starts but warns** — the server defaults to `--host 127.0.0.1`. Binding non-loopback (`--host`/`-H`/`CODEMAN_HOST`) without `CODEMAN_PASSWORD` starts anyway but prints a loud warning; `--allow-unauthenticated-network` / `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1` acknowledges it. ⚠️ The production systemd unit passes no `--host`, so prod binds **localhost only**: reach it via `tailscale serve`/tunnel to `127.0.0.1`. A loopback bind is reachable through a same-host tunnel but NOT by a browser hitting the box's LAN IP. `install.sh` is separate and prompts for the binding (defaulting to LAN + a password), and preserves the existing binding on re-runs. → [architecture-invariants#default-bind-and-the-non-loopback-warning-path](docs/architecture-invariants.md#default-bind-and-the-non-loopback-warning-path), `docs/security-architecture.md` - **Instance isolation / multi-instance attach danger** — the data dir (`~/.codeman`) and tmux socket (`tmux -L codeman`) are PROCESS-WIDE and shared by every Codeman on the machine, derived from `CODEMAN_INSTANCE` via `src/config/instance.ts`. ⚠️ A 2nd instance on the SAME socket **discovers and attaches PTYs to the first instance's live sessions**, resizing and mutating them. `$HOME` isolation is NOT enough because tmux is system-global. To run two instances, give each a distinct `CODEMAN_INSTANCE` (scopes dir + socket together), or set `CODEMAN_TMUX_SOCKET` + `CODEMAN_DATA_DIR` individually; `scripts/run-beta.sh` does this for a beta alongside prod. **Any new `~/.codeman/...` path MUST go through `dataPath()`**, never `join(homedir(), '.codeman', …)`, and **any new `tmux -L` caller through `resolveTmuxSocketName()`** (both in `config/instance.ts`): the TUI shells out to tmux from a second process, and a hardcoded `codeman` there would point a beta instance at prod's panes. → [architecture-invariants#instance-isolation-and-the-multi-instance-attach-danger](docs/architecture-invariants.md#instance-isolation-and-the-multi-instance-attach-danger) - **node-pty's macOS `spawn-helper` ships without `+x`** (issues #6, #204): `node-pty@1.1.0` publishes `prebuilds/darwin-/spawn-helper` as mode 0644, and macOS launches every PTY through it, so a stock macOS install fails every session start with `Error: posix_spawnp failed.` **Linux can never reproduce it**: `spawn-helper` is an `OS=="mac"` gyp target and node-pty ships no Linux prebuild, so node-gyp always emits an executable helper there. ⚠️ The flip side of that: since Linux has no prebuild, `npm install` **needs a C/C++ toolchain there** (`make`, `g++`, `python3`), so `install.sh` checks for and installs one alongside Node/tmux/git — a stock Ubuntu 24 server has none and died inside node-gyp with `not found: make`. Do not drop that step. ⚠️ Look in **`prebuilds/-/`**, not just `build/Release/`, which does not exist on macOS. Repair is a chmod, never a mandatory rebuild (that would require Xcode CLI tools and deletes `prebuilds/` before compiling): `npm run fix:node-pty` chmods every helper then proves it by really opening a PTY. `spawnPtyWithHelperRepair()` (`utils/node-pty-repair.ts`) wraps every `pty.spawn()` in `session.ts` and self-heals a broken install on the first failure. → [architecture-invariants#node-ptys-macos-spawn-helper-must-be-executable](docs/architecture-invariants.md#node-ptys-macos-spawn-helper-must-be-executable) +- **User-chosen paths are probed BOUNDED, never with `existsSync`/`statSync`** (#516): a linked case or a `workingDir` can sit on a network mount that stopped answering. A synchronous check there freezes the whole server, and an unbounded async `stat`/`lstat`/`readFile` holds one of libuv's threadpool workers (4 by default, shared with every `fs`, `dns.lookup` and `crypto` call) until the mount returns. On a request or spawn path use `probePath()`/`probePathKind()` (`utils/bounded-path-probe.ts`, read its `@fileoverview`). ⚠️ `unknown` is NEVER `absent`: nothing is created, scaffolded or 404'd on it. Bulk scans keep the stall cap; a request for ONE path the user named may pass `{ pastCap: true }`, which still stops at the ceiling that keeps one worker free; and a helper that would otherwise touch the path skips whatever is still `unknown`. User-facing errors for it go through `describeUnknownPath()`, so a refused probe is not reported as a broken folder. - **Headless screenshots: `deviceScaleFactor` MUST be 1, and write unique filenames** — under DSF=2 xterm's WebGL renderer draws glyphs at ~2× nominal size while still *reporting* nominal cell dims, so only the pixels reveal it and only the terminal font looks wrong. And overwriting a fixed output path leaves OS image viewers showing the old render, which reads as "the fix didn't work"; `scripts/capture-real-overview.mjs` mints a timestamped filename per run. Seed the per-device `localStorage` keys (`codeman:skin`, `codeman-font-size`, `codeman-app-settings`) so the capture matches a real device. → [architecture-invariants#headless-screenshot-capture](docs/architecture-invariants.md#headless-screenshot-capture) **Import conventions**: Utils from `./utils`, types from `./types` (barrel), config from specific `./config/*` files. diff --git a/docs/api-reference.md b/docs/api-reference.md index f0e41dd0..89a873b7 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -709,7 +709,7 @@ The target is judged before anything is written: - It must be absolute with no `..` and none of the shell metacharacters a session working directory is rejected for (spaces are fine). `400 INVALID_INPUT` otherwise. - It must not be a system directory (`/etc`, `/usr`, `/proc`, ...), the home folder itself, Codeman's own data folder, or a credential/config tree (`~/.ssh`, `~/.aws`, `~/.claude`, ...). Judged on the path as typed and on its symlink-resolved form, against both the given and the symlink-resolved roots. `400`. - It must not be, or be inside, the cases directory (the caller's own and the shared one): a case there is a plain create without `path`. `400`. -- Its parent must already exist (one folder is created, never a chain): `404 NOT_FOUND`. +- Its parent must already exist (one folder is created, never a chain): `404 NOT_FOUND`. A parent that does not answer (an unreachable network mount) or cannot be read is `422 OPERATION_FAILED`, checked through the bounded path probe before anything else touches it. - The folder must not exist, or must be an **empty** directory; a folder with contents is Link Existing's job: `409 ALREADY_EXISTS`. A symlink or a plain file at the target is `400`. - `409 ALREADY_EXISTS` also for a case name already in use (in the cases dir or the registry) and for a folder that is already a case. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 7fe8e59c..cd49d869 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -321,7 +321,7 @@ So: `_confirmIdle()` (session.ts) requires the pane to go quiet, and then asks t **Hook events**: Claude Code hooks trigger via `/api/hook-event`. Key events: `permission_prompt`, `elicitation_dialog`, `elicitation_complete`, `elicitation_response`, `idle_prompt`, `stop`, `teammate_idle`, `task_completed`, `prompt_submitted` (UserPromptSubmit, #367: a Claude pane reports its live conversation id first-hand). See `src/hooks-config.ts`; upstream hook semantics mirrored in `docs/claude-code-hooks-reference.md`. -⚠️ **Every claude session INSTALLS the hooks block into its workspace** (`applyWorkspaceHooks` in hooks-config.ts → `ensureCodemanHooks`, an add-only merge that keeps a user's own handlers), from EVERY claude create path — both interactive routes, cron fires, legacy scheduled runs, the plan-orchestrator one-shots — and from `restoreMuxSessions()` for sessions recovered on server start (that boot sweep skips a workspace that no longer exists, so a deleted repo with a surviving tmux session is never resurrected as an empty dir). Before 2026-08-15 hooks were written ONLY when Codeman created the case DIRECTORY, so a linked case / cloned repo — where most sessions actually run — had no hooks at all and every hook-driven surface was silently dead there: an AskUserQuestion dialog blocked the pane while the tab and the phone overview both read a calm `idle`, with no Approvals Inbox item, no push, no definitive `stop`/`idle_prompt` for respawn and no `stop`/`blocked` for the wait endpoints. +⚠️ **Every claude session INSTALLS the hooks block into its workspace** (`applyWorkspaceHooks` in hooks-config.ts → `ensureCodemanHooks`, an add-only merge that keeps a user's own handlers), from EVERY claude create path — both interactive routes, cron fires, legacy scheduled runs, the plan-orchestrator one-shots — and from `restoreMuxSessions()` for sessions recovered on server start (that boot sweep skips a workspace that no longer exists, so a deleted repo with a surviving tmux session is never resurrected as an empty dir, and skips one that does not answer, such as a linked case on an unreachable mount, without waiting on it: `applyWorkspaceHooks` asks the bounded path probe, retries one capacity-refused probe past the bulk cap, and skips whatever is still "unknown" rather than touching it with an unbounded `lstat`). Before 2026-08-15 hooks were written ONLY when Codeman created the case DIRECTORY, so a linked case / cloned repo — where most sessions actually run — had no hooks at all and every hook-driven surface was silently dead there: an AskUserQuestion dialog blocked the pane while the tab and the phone overview both read a calm `idle`, with no Approvals Inbox item, no push, no definitive `stop`/`idle_prompt` for respawn and no `stop`/`blocked` for the wait endpoints. The escape hatch is the synced `workspaceHooksEnabled` setting (App Settings → Agents & CLIs → Claude, **default ON**); OFF restores the old behavior, where a Codeman block that is already there is still refreshed when stale (COD-91) but one is never added. ⚠️ Route the decision through `applyWorkspaceHooks` rather than calling `ensureCodemanHooks` at a new site, or the setting silently stops applying to that path. diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index b9c3c771..63335691 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -191,7 +191,7 @@ Some things are configured before the server starts, not in the UI: | `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. | +| `CODEMAN_PATH_PROBE_MAX_STALLED` | Unanswered folder checks allowed to pile up before new ones are refused. 2 by default: one below the threadpool size minus one, so it follows `UV_THREADPOOL_SIZE` (4 unless set), and it is never allowed above that ceiling. A check you start by opening one case or session may use the one slot left above it. | ## Gotchas diff --git a/plugins/codeman/skills/codeman/reference/verbs.md b/plugins/codeman/skills/codeman/reference/verbs.md index 46eecc8d..db9d7d7e 100644 --- a/plugins/codeman/skills/codeman/reference/verbs.md +++ b/plugins/codeman/skills/codeman/reference/verbs.md @@ -95,6 +95,9 @@ Differences from `quick-start` worth knowing before you debug one: - the id is at `.data.session.id`, not `.data.sessionId`; - `workingDir` must already exist (400 `INVALID_INPUT`, "workingDir does not exist"), and in multi-user mode must be inside the caller's own workspace (403 `FORBIDDEN`); + one that does not answer or cannot be read (an unreachable network mount, a + permission error) is 422 `OPERATION_FAILED`, never "does not exist", so do not + create a replacement for it; - hitting the session cap here is `OPERATION_FAILED`, where `quick-start` returns `SESSION_BUSY` for the identical condition. diff --git a/skills/codeman/reference/verbs.md b/skills/codeman/reference/verbs.md index 46eecc8d..db9d7d7e 100644 --- a/skills/codeman/reference/verbs.md +++ b/skills/codeman/reference/verbs.md @@ -95,6 +95,9 @@ Differences from `quick-start` worth knowing before you debug one: - the id is at `.data.session.id`, not `.data.sessionId`; - `workingDir` must already exist (400 `INVALID_INPUT`, "workingDir does not exist"), and in multi-user mode must be inside the caller's own workspace (403 `FORBIDDEN`); + one that does not answer or cannot be read (an unreachable network mount, a + permission error) is 422 `OPERATION_FAILED`, never "does not exist", so do not + create a replacement for it; - hitting the session cap here is `OPERATION_FAILED`, where `quick-start` returns `SESSION_BUSY` for the identical condition. diff --git a/src/hooks-config.ts b/src/hooks-config.ts index 5f9eabac..19c2f788 100644 --- a/src/hooks-config.ts +++ b/src/hooks-config.ts @@ -59,17 +59,30 @@ async function pathExistsForWrite(path: string): Promise { } } +/** + * Probe a path a per-spawn helper is about to touch. An "unknown" that is NOT near a + * stalled probe (the bulk cap refused it, or the stat failed with something other + * than ENOENT) gets ONE more bounded probe past the bulk cap, so a healthy path still + * answers while unrelated mounts are dead. Whatever is still "unknown" after that + * must be skipped by the caller, never touched with an unbounded `lstat`/`readFile`: + * on a dead mount those never settle, and each would hold a threadpool worker the + * probe's ceiling does not count. + */ +async function probeBeforeTouching(path: string) { + const state = await probePath(path); + if (state !== 'unknown' || isNearStalledPath(path)) return state; + return probePath(path, { pastCap: true }); +} + /** * 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. + * it did not answer (a mount that is not responding, a refused probe, an unreadable + * path). See `probeBeforeTouching` for why "unknown" is a skip. */ async function absentOrUnreachable(path: string): Promise<'absent' | 'unreachable' | false> { - const state = await probePath(path); + const state = await probeBeforeTouching(path); if (state === 'absent') return 'absent'; - if (state === 'unknown' && isNearStalledPath(path)) return 'unreachable'; + if (state === 'unknown') return 'unreachable'; return false; } @@ -852,19 +865,16 @@ export async function refreshStaleCodemanHooks(casePath: string): Promise */ export async function applyWorkspaceHooks(workspace: string, install?: boolean): Promise { try { - const state = await probePath(workspace); + // "absent" stays absent: the install below would mkdir -p a deleted repo back + // into being. "unknown" is skipped too, never asked again with an unbounded + // lstat (see probeBeforeTouching). + const state = await probeBeforeTouching(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; + console.warn( + `[hooks] ${workspace} is not responding or not readable (unreachable mount?); Codeman hooks not checked or installed` + ); + return; } const shouldInstall = install ?? (await readWorkspaceHooksEnabled()); await (shouldInstall ? ensureCodemanHooks(workspace) : refreshStaleCodemanHooks(workspace)); diff --git a/src/types/api.ts b/src/types/api.ts index 52a07640..c3cadc03 100644 --- a/src/types/api.ts +++ b/src/types/api.ts @@ -159,7 +159,9 @@ export interface CaseInfo { 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. + * than "no such file"), or its probe was refused because folders on other unreachable + * mounts are still not answering, so whether it still exists is unknown. A refused + * probe can set this on a healthy linked case. Absent = it answered. */ unreachable?: boolean; /** diff --git a/src/utils/bounded-path-probe.ts b/src/utils/bounded-path-probe.ts index 34f32adc..bd1d6933 100644 --- a/src/utils/bounded-path-probe.ts +++ b/src/utils/bounded-path-probe.ts @@ -37,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. `pastCap` still stops at + * keep the cap; the per-spawn hook and statusLine helpers retry one refused + * probe past it and then skip a path that still answers "unknown", rather than + * touch it with an unbounded call. `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. * @@ -203,6 +205,31 @@ export async function probePathKind(path: string, options: PathProbeOptions = {} } } +/** + * Why a probe of `path` answers "unknown" right now: its mount is not answering + * (`'stalled'`, it is near a stalled probe), new probes are refused because enough + * UNRELATED paths are stalled (`'refused'`; `pastCap` picks which limit applies), or + * neither, so the filesystem answered with an error such as EACCES or EIO + * (`'unreadable'`). For messages only: it reads the state now, not at probe time. + */ +export function unknownPathReason(path: string, options: PathProbeOptions = {}): 'stalled' | 'refused' | 'unreadable' { + if (isNearStalledPath(path)) return 'stalled'; + if (stalled.size >= (options.pastCap ? PATH_PROBE_STALL_CEILING : MAX_STALLED_PATH_PROBES)) return 'refused'; + return 'unreadable'; +} + +/** + * User-facing sentence for an "unknown" probe of `path` (`label` names it, e.g. + * "workingDir"). A refused probe says so, rather than blaming a folder that was never + * checked: at the ceiling every new folder reads "unknown" until a dead mount answers. + */ +export function describeUnknownPath(label: string, path: string, options: PathProbeOptions = {}): string { + return unknownPathReason(path, options) === 'refused' + ? `${label} was not checked: folders on other unreachable mounts are still not answering, ` + + `so Codeman is not checking new folders until one does (see the server log): ${path}` + : `${label} is not responding or not readable: ${path}`; +} + /** 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); diff --git a/src/utils/index.ts b/src/utils/index.ts index 2c30f417..c7763ee5 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -68,5 +68,12 @@ 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 { + boundedPathExists, + describeUnknownPath, + probePath, + probePathKind, + isNearStalledPath, + unknownPathReason, +} from './bounded-path-probe.js'; export type { PathProbeState, PathProbeKind, PathProbeOptions } from './bounded-path-probe.js'; diff --git a/src/web/case-path.ts b/src/web/case-path.ts index 27b68386..fb784412 100644 --- a/src/web/case-path.ts +++ b/src/web/case-path.ts @@ -28,6 +28,7 @@ import { promises as fs } from 'node:fs'; import { basename, dirname, join, resolve, sep } from 'node:path'; import { isValidWorkingDir } from './schemas.js'; +import { describeUnknownPath, probePath } from '../utils/index.js'; /** System trees nobody creates a project in; creating one here is a mistake or an attack. */ const BLOCKED_SYSTEM_ROOTS = [ @@ -61,7 +62,7 @@ export interface NewCasePathContext { export type NewCasePathResult = | { ok: true; path: string; existedEmpty: boolean } - | { ok: false; code: 'INVALID' | 'BLOCKED' | 'NOT_FOUND' | 'EXISTS'; reason: string }; + | { ok: false; code: 'INVALID' | 'BLOCKED' | 'NOT_FOUND' | 'EXISTS' | 'UNREACHABLE'; reason: string }; const isWithin = (child: string, root: string): boolean => child === root || child.startsWith(root.endsWith(sep) ? root : root + sep); @@ -145,6 +146,21 @@ export async function prepareNewCasePath(raw: string, ctx: NewCasePathContext): const typedBlock = blockedReason(target, ctx); if (typedBlock) return { ok: false, code: 'BLOCKED', reason: typedBlock }; + // Bounded first: the parent can sit on a network mount that stopped answering, where the + // realpath/stat/lstat/readdir below would each hold a threadpool worker until it returns. + // It is one folder the user named, so the probe may pass the bulk cap (never the ceiling). + const parentState = await probePath(dirname(target), { pastCap: true }); + if (parentState === 'absent') { + return { ok: false, code: 'NOT_FOUND', reason: `The parent folder ${dirname(target)} does not exist` }; + } + if (parentState === 'unknown') { + return { + ok: false, + code: 'UNREACHABLE', + reason: describeUnknownPath('The parent folder', dirname(target), { pastCap: true }), + }; + } + // Resolve the parent's symlinks, then judge again: a link into a blocked tree must not pass. let realParent: string; try { diff --git a/src/web/routes/case-routes.ts b/src/web/routes/case-routes.ts index 066fc121..7515e1da 100644 --- a/src/web/routes/case-routes.ts +++ b/src/web/routes/case-routes.ts @@ -51,7 +51,7 @@ import { import type { GitRemoteProbe, GitUrlParse } from '../../git-clone.js'; import { generateClaudeMd } from '../../templates/claude-md.js'; import { prepareNewCasePath } from '../case-path.js'; -import { boundedPathExists, probePath } from '../../utils/index.js'; +import { boundedPathExists, describeUnknownPath, probePath } from '../../utils/index.js'; import { readAgentCaseMarker, type AgentCaseMarker } from '../../agent-case-marker.js'; import { settingsWriteBlocker, writeHooksConfig } from '../../hooks-config.js'; import { @@ -464,6 +464,12 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config const casesDirs = [...new Set([ownCasesDir, resolveCasesDir()])]; const prepared = await prepareNewCasePath(customPath, { home: homedir(), dataDir: getDataDir(), casesDirs }); if (!prepared.ok) { + // A parent that did not answer is not a bad request: OPERATION_FAILED (422), like + // POST /api/sessions for a workingDir on a dead mount. + if (prepared.code === 'UNREACHABLE') { + reply.code(422); + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, prepared.reason); + } const status = prepared.code === 'NOT_FOUND' ? 404 : prepared.code === 'EXISTS' ? 409 : 400; reply.code(status); const code = @@ -1752,7 +1758,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config if (!linked) { return createErrorResponse( ApiErrorCode.OPERATION_FAILED, - `Case folder is not responding or not readable: ${casePath}` + describeUnknownPath('Case folder', casePath, { pastCap: true }) ); } return { name, path: casePath, hasClaudeMd: false, linked: true, unreachable: true }; diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index eeabfa96..a21b19f4 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -174,7 +174,7 @@ import { toSessionDocker, } from '../../docker-hosts.js'; import { LRUMap } from '../../utils/lru-map.js'; -import { probePathKind } from '../../utils/index.js'; +import { describeUnknownPath, 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'; @@ -980,7 +980,7 @@ export function registerSessionRoutes( if (kind === 'unknown') { return createErrorResponse( ApiErrorCode.OPERATION_FAILED, - `workingDir is not responding or not readable: ${workingDir}` + describeUnknownPath('workingDir', workingDir, { pastCap: true }) ); } if (kind === 'absent') { @@ -3710,7 +3710,7 @@ export function registerSessionRoutes( if (localCaseState === 'unknown') { return createErrorResponse( ApiErrorCode.OPERATION_FAILED, - `Case folder is not responding or not readable: ${resolvedCasePath}` + describeUnknownPath('Case folder', resolvedCasePath, { pastCap: true }) ); } diff --git a/test/bounded-path-probe.test.ts b/test/bounded-path-probe.test.ts index ff85bf12..56281f8d 100644 --- a/test/bounded-path-probe.test.ts +++ b/test/bounded-path-probe.test.ts @@ -37,7 +37,14 @@ 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 { + boundedPathExists, + describeUnknownPath, + isNearStalledPath, + probePath, + probePathKind, + unknownPathReason, +} from '../src/utils/bounded-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); @@ -100,6 +107,9 @@ describe('probePath', () => { expect(await boundedPathExists('/present')).toBe(true); expect(await boundedPathExists('/missing')).toBe(false); expect(await boundedPathExists('/eio')).toBe(false); + // An error answer is neither a stall nor a refusal. + expect(unknownPathReason('/eio')).toBe('unreadable'); + expect(describeUnknownPath('Case folder', '/eio')).toBe('Case folder is not responding or not readable: /eio'); }); it('reports whether a present path is a directory', async () => { @@ -291,6 +301,15 @@ describe('probePath', () => { expect(stat).not.toHaveBeenCalled(); expect(await probePath('/healthy/explicit', { pastCap: true })).toBe('unknown'); expect(stat).not.toHaveBeenCalled(); + // ...and the message says the folder was never checked, rather than blaming it. + expect(unknownPathReason('/healthy/explicit', { pastCap: true })).toBe('refused'); + expect(unknownPathReason(dead[0], { pastCap: true })).toBe('stalled'); + expect(describeUnknownPath('workingDir', '/healthy/explicit', { pastCap: true })).toMatch( + /^workingDir was not checked: .*not answering.*: \/healthy\/explicit$/ + ); + expect(describeUnknownPath('workingDir', dead[0], { pastCap: true })).toBe( + `workingDir is not responding or not readable: ${dead[0]}` + ); // Once one stalled stat settles, an explicit request is probed again. releases.get(dead[0])!(); @@ -298,8 +317,34 @@ describe('probePath', () => { 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('keeps the bulk cap below the ceiling, so a pastCap probe has room', async () => { + // Read under a controlled environment: the limits are computed at import from + // UV_THREADPOOL_SIZE and CODEMAN_PATH_PROBE_MAX_STALLED, which the test process + // could otherwise inherit. + const saved = { uv: process.env.UV_THREADPOOL_SIZE, max: process.env.CODEMAN_PATH_PROBE_MAX_STALLED }; + const limitsUnder = async (uv: string | undefined, max: string | undefined) => { + if (uv === undefined) delete process.env.UV_THREADPOOL_SIZE; + else process.env.UV_THREADPOOL_SIZE = uv; + if (max === undefined) delete process.env.CODEMAN_PATH_PROBE_MAX_STALLED; + else process.env.CODEMAN_PATH_PROBE_MAX_STALLED = max; + vi.resetModules(); + return import('../src/config/path-probe.js'); + }; + try { + const defaults = await limitsUnder(undefined, undefined); + expect([defaults.MAX_STALLED_PATH_PROBES, defaults.PATH_PROBE_STALL_CEILING]).toEqual([2, 3]); + const bigPool = await limitsUnder('8', undefined); + expect([bigPool.MAX_STALLED_PATH_PROBES, bigPool.PATH_PROBE_STALL_CEILING]).toEqual([6, 7]); + // An override may reach the ceiling but never pass it. + const overridden = await limitsUnder(undefined, '64'); + expect(overridden.MAX_STALLED_PATH_PROBES).toBe(overridden.PATH_PROBE_STALL_CEILING); + } finally { + if (saved.uv === undefined) delete process.env.UV_THREADPOOL_SIZE; + else process.env.UV_THREADPOOL_SIZE = saved.uv; + if (saved.max === undefined) delete process.env.CODEMAN_PATH_PROBE_MAX_STALLED; + else process.env.CODEMAN_PATH_PROBE_MAX_STALLED = saved.max; + vi.resetModules(); + } }); it('warns once when a path first stalls and once when the cap engages', async () => { diff --git a/test/case-path-unreachable.test.ts b/test/case-path-unreachable.test.ts new file mode 100644 index 00000000..ea71012c --- /dev/null +++ b/test/case-path-unreachable.test.ts @@ -0,0 +1,91 @@ +/** + * @fileoverview "Create in a custom folder" (#535) on a parent folder that does not + * answer (#516): `prepareNewCasePath` must ask the bounded path probe first, and give + * up with UNREACHABLE within the probe timeout instead of reaching the realpath / stat / + * lstat / readdir calls that would wait on a hard mount forever. + * + * Only the chosen dead paths hang; everything else is the real filesystem. + * Port: none. + */ +import { afterAll, afterEach, describe, expect, it, vi } from 'vitest'; +import { mkdtempSync, realpathSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const probe = vi.hoisted(() => { + // Short probe timeout so a stall costs ~100 ms, read at import. + process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS = '100'; + return { dead: '/mnt/dead-nas-case-parent', releases: [] as Array<() => void>, touched: [] as string[] }; +}); + +/** A call on the dead path never settles (a hard mount), until afterEach releases it. */ +function hangOnDead unknown>(name: string, real: F): F { + return ((path: string, ...rest: unknown[]) => { + if (String(path) === probe.dead || String(path).startsWith(probe.dead + '/')) { + probe.touched.push(`${name} ${String(path)}`); + return new Promise((_resolve, reject) => { + probe.releases.push(() => reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }))); + }); + } + return (real as unknown as (...a: unknown[]) => unknown)(path, ...rest); + }) as unknown as F; +} + +vi.mock('node:fs/promises', async (importOriginal) => { + const actual = await importOriginal(); + const stat = hangOnDead('stat', actual.stat); + return { ...actual, stat, default: { ...actual, stat } }; +}); + +vi.mock('node:fs', async (importOriginal) => { + const actual = await importOriginal(); + const promises = { + ...actual.promises, + realpath: hangOnDead('realpath', actual.promises.realpath), + stat: hangOnDead('stat', actual.promises.stat), + lstat: hangOnDead('lstat', actual.promises.lstat), + readdir: hangOnDead('readdir', actual.promises.readdir), + }; + return { ...actual, promises, default: { ...actual, promises } }; +}); + +import { prepareNewCasePath } from '../src/web/case-path.js'; + +const root = realpathSync(mkdtempSync(join(tmpdir(), 'case-path-unreachable-'))); +const ctx = { home: join(root, 'home'), dataDir: join(root, 'home', '.codeman'), casesDirs: [join(root, 'cases')] }; + +afterEach(async () => { + probe.releases.splice(0).forEach((release) => release()); + probe.touched.length = 0; + await new Promise((r) => setTimeout(r, 0)); +}); + +afterAll(() => { + rmSync(root, { recursive: true, force: true }); + delete process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS; +}); + +describe('prepareNewCasePath on a parent folder that does not answer', () => { + it('answers UNREACHABLE within the probe timeout and never touches the path unbounded', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const result = await Promise.race([ + prepareNewCasePath(`${probe.dead}/new-case`, ctx), + new Promise<'hung'>((resolve) => setTimeout(() => resolve('hung'), 2_000)), + ]); + warn.mockRestore(); + + expect(result).toMatchObject({ ok: false, code: 'UNREACHABLE' }); + expect((result as { reason: string }).reason).toBe( + `The parent folder is not responding or not readable: ${probe.dead}` + ); + // Only the bounded probe's own stat reached the dead mount. + expect(probe.touched).toEqual([`stat ${probe.dead}`]); + }); + + it('still reports a parent that definitely does not exist as NOT_FOUND', async () => { + expect(await prepareNewCasePath(join(root, 'no-such-parent', 'new-case'), ctx)).toMatchObject({ + ok: false, + code: 'NOT_FOUND', + }); + }); +}); diff --git a/test/run-mode-ui.test.ts b/test/run-mode-ui.test.ts index 5b96f389..b290cf8f 100644 --- a/test/run-mode-ui.test.ts +++ b/test/run-mode-ui.test.ts @@ -1227,66 +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 }; - } +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' }; + 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'); - }); + 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); - }); - } - }); + 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 index 909f4625..84c1a210 100644 --- a/test/workspace-hooks-unreachable-mount.test.ts +++ b/test/workspace-hooks-unreachable-mount.test.ts @@ -22,18 +22,26 @@ const probe = vi.hoisted(() => { 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; - }); + /** A call on a dead path never settles (a hard mount), until afterEach releases it. */ + const hangOnDead = unknown>(real: F): F => + ((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 } }; + return (real as unknown as (...a: unknown[]) => unknown)(path, ...rest); + }) as unknown as F; + const hung = { + stat: hangOnDead(actual.stat), + lstat: hangOnDead(actual.lstat), + readFile: hangOnDead(actual.readFile), + realpath: hangOnDead(actual.realpath), + }; + return { ...actual, ...hung, default: { ...actual, ...hung } }; }); import { applyWorkspaceHooks, resolveStatusLineCliCommand, stripCaseEnvKeys } from '../src/hooks-config.js'; @@ -149,6 +157,21 @@ describe('workspace helpers while other mounts are unreachable', () => { expect(await resolveStatusLineCliCommand(workspace, true)).toMatch(/statusline-exporter\.sh$/); }); + it('skips, within the probe timeout, dead workspaces whose probes the bulk cap refused', async () => { + // Two unrelated stalls engage the bulk cap, so probes of these two paths are + // refused without a stat. Their lstat and readFile hang too: a helper that + // touched them directly would never return and would hold a threadpool worker. + await stallUnrelatedMounts(MAX_STALLED_PATH_PROBES); + probe.dead.add('/mnt/dead-nas-c'); + probe.dead.add('/mnt/dead-nas-d'); + const within = (work: Promise) => + Promise.race([work, new Promise<'hung'>((resolve) => setTimeout(() => resolve('hung'), 2_000))]); + + expect(await within(applyWorkspaceHooks('/mnt/dead-nas-c/project', true))).toBeUndefined(); + expect(await within(resolveStatusLineCliCommand('/mnt/dead-nas-d/project', true))).toBeUndefined(); + expect(existsSync('/mnt/dead-nas-c/project')).toBe(false); + }); + 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');