mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-06 15:39:41 +02:00
fix(cases): bound linked-workspace path probes so an unreachable mount cannot freeze the server
A linked case can live on a network mount. When that mount goes away, a hard mount makes stat() wait indefinitely, and the existsSync() probes in the case routes and the workspace hook/statusline helpers ran on the event loop, so a single GET /api/cases (or a session create in that workspace) froze the whole web server until the mount came back. Add boundedPathExists() (src/utils/bounded-path-probe.ts): an async stat that answers "absent" after 1.5 s, shares one in-flight probe per path, remembers a timed-out path until its stat finally settles, and refuses to start new probes while two stalled ones still hold libuv threadpool workers. Route the read-side probes in case-routes.ts and hooks-config.ts through it. The settings writers in hooks-config.ts use an async lstat that treats only ENOENT as missing, so an unreachable workspace is never mistaken for an empty one and has its settings recreated.
This commit is contained in:
committed by
Aamer Akhter
parent
ffaa5ee80c
commit
00b935abe6
+30
-12
@@ -30,7 +30,6 @@
|
||||
*/
|
||||
|
||||
import { randomBytes } from 'node:crypto';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { readFile, writeFile, mkdir, lstat, readdir, realpath, rename, unlink, rmdir, chmod } from 'node:fs/promises';
|
||||
import { homedir } from 'node:os';
|
||||
import { join, dirname } from 'node:path';
|
||||
@@ -40,6 +39,25 @@ import type { HookEventType } from './types.js';
|
||||
import { HOOK_TIMEOUT_SECONDS } from './config/auth-config.js';
|
||||
import { dataPath } from './config/instance.js';
|
||||
import { readJsonConfig, SETTINGS_PATH } from './web/route-helpers.js';
|
||||
import { boundedPathExists } from './utils/bounded-path-probe.js';
|
||||
|
||||
/**
|
||||
* Existence check for a WRITER. Unlike `boundedPathExists`, which answers
|
||||
* "absent" for a path it could not reach in time, this tells "missing" apart
|
||||
* from "unreachable": only ENOENT reads as absent, anything else throws, so a
|
||||
* stalled or unreadable workspace can never be mistaken for an empty one and
|
||||
* have its settings recreated over the top. It is async, so a dead mount ties
|
||||
* up a threadpool worker rather than the event loop.
|
||||
*/
|
||||
async function pathExistsForWrite(path: string): Promise<boolean> {
|
||||
try {
|
||||
await lstat(path);
|
||||
return true;
|
||||
} catch (err) {
|
||||
if ((err as NodeJS.ErrnoException).code === 'ENOENT') return false;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Serializes read-modify-write access to a `settings.local.json` path. Every
|
||||
@@ -558,7 +576,7 @@ export async function stripCaseEnvKeys(casePath: string, keysToRemove: readonly
|
||||
if (keysToRemove.length === 0) return;
|
||||
|
||||
await withSafeSettingsWrite(casePath, 'env-key removal', async (_claudeDir, settingsPath) => {
|
||||
if (!existsSync(settingsPath)) return;
|
||||
if (!(await boundedPathExists(settingsPath))) return;
|
||||
|
||||
let existing: Record<string, unknown>;
|
||||
try {
|
||||
@@ -590,7 +608,7 @@ export async function stripCaseEnvKeys(casePath: string, keysToRemove: readonly
|
||||
*/
|
||||
export async function updateCaseEnvVars(casePath: string, envVars: Record<string, string>): Promise<void> {
|
||||
await withSafeSettingsWrite(casePath, 'env vars', async (claudeDir, settingsPath) => {
|
||||
if (!existsSync(claudeDir)) {
|
||||
if (!(await pathExistsForWrite(claudeDir))) {
|
||||
await mkdir(claudeDir, { recursive: true });
|
||||
}
|
||||
|
||||
@@ -621,7 +639,7 @@ export async function updateCaseEnvVars(casePath: string, envVars: Record<string
|
||||
*/
|
||||
export async function updateCaseModel(casePath: string, model: string | null): Promise<void> {
|
||||
await withSafeSettingsWrite(casePath, 'model', async (claudeDir, settingsPath) => {
|
||||
if (!existsSync(claudeDir)) {
|
||||
if (!(await pathExistsForWrite(claudeDir))) {
|
||||
await mkdir(claudeDir, { recursive: true });
|
||||
}
|
||||
|
||||
@@ -650,7 +668,7 @@ export async function updateCaseModel(casePath: string, model: string | null): P
|
||||
*/
|
||||
export async function writeHooksConfig(casePath: string): Promise<void> {
|
||||
await withSafeSettingsWrite(casePath, 'hooks', async (claudeDir, settingsPath) => {
|
||||
if (!existsSync(claudeDir)) {
|
||||
if (!(await pathExistsForWrite(claudeDir))) {
|
||||
await mkdir(claudeDir, { recursive: true });
|
||||
}
|
||||
|
||||
@@ -698,7 +716,7 @@ export async function writeHooksConfig(casePath: string): Promise<void> {
|
||||
*/
|
||||
export async function ensureCodemanHooks(casePath: string): Promise<void> {
|
||||
await withSafeSettingsWrite(casePath, 'hooks (ensure)', async (claudeDir, settingsPath) => {
|
||||
if (!existsSync(claudeDir)) {
|
||||
if (!(await pathExistsForWrite(claudeDir))) {
|
||||
await mkdir(claudeDir, { recursive: true });
|
||||
}
|
||||
|
||||
@@ -738,7 +756,7 @@ export async function ensureCodemanHooks(casePath: string): Promise<void> {
|
||||
* when the hooks aren't ours, so it is cheap enough to call on every Claude spawn.
|
||||
*/
|
||||
export async function refreshStaleCodemanHooks(casePath: string): Promise<void> {
|
||||
if (!existsSync(join(casePath, '.claude', 'settings.local.json'))) return;
|
||||
if (!(await boundedPathExists(join(casePath, '.claude', 'settings.local.json')))) return;
|
||||
await withSafeSettingsWrite(casePath, 'hooks (refresh)', async (_claudeDir, settingsPath) => {
|
||||
let existing: Record<string, unknown>;
|
||||
try {
|
||||
@@ -820,7 +838,7 @@ export async function refreshStaleCodemanHooks(casePath: string): Promise<void>
|
||||
*/
|
||||
export async function applyWorkspaceHooks(workspace: string, install?: boolean): Promise<void> {
|
||||
try {
|
||||
if (!existsSync(workspace)) return;
|
||||
if (!(await boundedPathExists(workspace))) return;
|
||||
const shouldInstall = install ?? (await readWorkspaceHooksEnabled());
|
||||
await (shouldInstall ? ensureCodemanHooks(workspace) : refreshStaleCodemanHooks(workspace));
|
||||
} catch {
|
||||
@@ -883,7 +901,7 @@ export function generateStatusLineCommand(): string {
|
||||
export async function applyStatusLineConfig(casePath: string, enabled: boolean): Promise<void> {
|
||||
await withSafeSettingsWrite(casePath, 'statusLine', async (claudeDir, settingsPath) => {
|
||||
let existing: Record<string, unknown> = {};
|
||||
if (existsSync(settingsPath)) {
|
||||
if (await pathExistsForWrite(settingsPath)) {
|
||||
try {
|
||||
existing = JSON.parse(await readFile(settingsPath, 'utf-8'));
|
||||
} catch {
|
||||
@@ -898,7 +916,7 @@ export async function applyStatusLineConfig(casePath: string, enabled: boolean):
|
||||
const desired = generateStatusLineCommand();
|
||||
if (isOurs && current?.command === desired) return; // already current — skip rewrite
|
||||
if (current && !isOurs) return; // user has their OWN statusLine — never clobber it
|
||||
if (!existsSync(claudeDir)) await mkdir(claudeDir, { recursive: true });
|
||||
if (!(await pathExistsForWrite(claudeDir))) await mkdir(claudeDir, { recursive: true });
|
||||
existing.statusLine = { type: 'command', command: desired }; // add, or update an out-of-date ours
|
||||
} else {
|
||||
if (!isOurs) return; // nothing of ours to remove (leave a user's own statusLine alone)
|
||||
@@ -957,7 +975,7 @@ function statusLineExporterScriptContent(): string {
|
||||
}
|
||||
|
||||
async function readStatusLineCommandFromFile(settingsPath: string): Promise<string | undefined> {
|
||||
if (!existsSync(settingsPath)) return undefined;
|
||||
if (!(await boundedPathExists(settingsPath))) return undefined;
|
||||
try {
|
||||
const parsed = JSON.parse(await readFile(settingsPath, 'utf-8'));
|
||||
const current = parsed.statusLine as { command?: unknown } | undefined;
|
||||
@@ -1103,7 +1121,7 @@ export async function resolveStatusLineCliCommand(
|
||||
): Promise<string | undefined> {
|
||||
const settingsPath = join(casePath, '.claude', 'settings.local.json');
|
||||
let userHasOwnStatusLine = false;
|
||||
if (existsSync(settingsPath)) {
|
||||
if (await boundedPathExists(settingsPath)) {
|
||||
try {
|
||||
const existing = JSON.parse(await readFile(settingsPath, 'utf-8'));
|
||||
const current = existing.statusLine as { command?: unknown } | undefined;
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
/**
|
||||
* @fileoverview Bounded existence probe for user-chosen paths.
|
||||
*
|
||||
* A linked case can live on a network mount (NFS, SMB, sshfs). When that mount
|
||||
* goes unreachable, a hard mount makes `stat()` wait forever. A synchronous
|
||||
* probe (`existsSync`) on such a path blocks the event loop and freezes the
|
||||
* whole web server; even an async `stat()` never settles and permanently holds
|
||||
* one of libuv's few threadpool workers, which every other `fs`, `dns.lookup`
|
||||
* and `crypto` call in the process shares.
|
||||
*
|
||||
* `boundedPathExists()` therefore:
|
||||
* - probes asynchronously and answers `false` after `PROBE_TIMEOUT_MS`, so a
|
||||
* request never waits on a dead mount for longer than that;
|
||||
* - shares one in-flight probe per path, and keeps answering `false` for a path
|
||||
* whose probe timed out until that probe finally settles (so a dead path is
|
||||
* not re-probed on every request, and is re-probed once the mount recovers);
|
||||
* - stops starting new probes once `MAX_STALLED_PROBES` timed-out probes are
|
||||
* still pending, so stalled stats cannot drain the threadpool. Probes that are
|
||||
* merely in flight do not count, so concurrent healthy probes never get a
|
||||
* false negative.
|
||||
*
|
||||
* Like `existsSync`, it follows symlinks and reports any error as "absent". It
|
||||
* is meant for READ decisions (is it there, show it or not). A writer that must
|
||||
* tell "missing" apart from "unreachable" should not treat its `false` as
|
||||
* permission to create or overwrite anything.
|
||||
*
|
||||
* @module utils/bounded-path-probe
|
||||
*/
|
||||
|
||||
import fs from 'node:fs/promises';
|
||||
|
||||
/** How long a caller waits for one probe before treating the path as absent. */
|
||||
export const PROBE_TIMEOUT_MS = 1_500;
|
||||
/** Timed-out probes allowed to remain pending before new probes are refused. */
|
||||
export const MAX_STALLED_PROBES = 2;
|
||||
|
||||
const inFlight = new Map<string, Promise<boolean>>();
|
||||
const stalled = new Set<string>();
|
||||
|
||||
async function statExists(path: string): Promise<boolean> {
|
||||
try {
|
||||
await fs.stat(path);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve whether `path` exists without letting an unresponsive filesystem
|
||||
* block the caller for longer than `PROBE_TIMEOUT_MS`.
|
||||
*/
|
||||
export async function boundedPathExists(path: string): Promise<boolean> {
|
||||
if (stalled.has(path)) return false;
|
||||
|
||||
let probe = inFlight.get(path);
|
||||
if (!probe) {
|
||||
if (stalled.size >= MAX_STALLED_PROBES) return false;
|
||||
probe = statExists(path);
|
||||
inFlight.set(path, probe);
|
||||
void probe.finally(() => {
|
||||
inFlight.delete(path);
|
||||
stalled.delete(path);
|
||||
});
|
||||
}
|
||||
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
return await Promise.race([
|
||||
probe,
|
||||
new Promise<boolean>((resolve) => {
|
||||
timer = setTimeout(() => {
|
||||
if (inFlight.get(path) === probe) stalled.add(path);
|
||||
resolve(false);
|
||||
}, PROBE_TIMEOUT_MS);
|
||||
timer.unref?.();
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
if (timer) clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
@@ -50,6 +50,7 @@ import {
|
||||
} from '../../git-clone.js';
|
||||
import type { GitRemoteProbe, GitUrlParse } from '../../git-clone.js';
|
||||
import { generateClaudeMd } from '../../templates/claude-md.js';
|
||||
import { boundedPathExists } from '../../utils/bounded-path-probe.js';
|
||||
import { readAgentCaseMarker, type AgentCaseMarker } from '../../agent-case-marker.js';
|
||||
import { settingsWriteBlocker, writeHooksConfig } from '../../hooks-config.js';
|
||||
import {
|
||||
@@ -162,8 +163,11 @@ function gitDiagnosticLine(stderr: string): string {
|
||||
* hooks, which run on the user's machine when a session starts in the case, so
|
||||
* the clone response says so out loud instead of silently merging into them.
|
||||
*/
|
||||
function repoShipsClaudeSettings(casePath: string): boolean {
|
||||
return ['settings.json', 'settings.local.json'].some((file) => existsSync(join(casePath, '.claude', file)));
|
||||
async function repoShipsClaudeSettings(casePath: string): Promise<boolean> {
|
||||
for (const file of ['settings.json', 'settings.local.json']) {
|
||||
if (await boundedPathExists(join(casePath, '.claude', file))) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -266,7 +270,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
cases.push({
|
||||
name: e.name,
|
||||
path: casePath,
|
||||
hasClaudeMd: existsSync(join(casePath, 'CLAUDE.md')),
|
||||
hasClaudeMd: await boundedPathExists(join(casePath, 'CLAUDE.md')),
|
||||
location: 'local',
|
||||
...(marker ? { agentCreated: agentCreatedInfo(marker) } : {}),
|
||||
});
|
||||
@@ -281,11 +285,11 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
const existingNames = new Set(cases.map((c) => c.name));
|
||||
if (admin) {
|
||||
for (const [name, path] of Object.entries(linkedCases)) {
|
||||
if (!existingNames.has(name) && SAFE_CASE_NAME.test(name) && existsSync(path)) {
|
||||
if (!existingNames.has(name) && SAFE_CASE_NAME.test(name) && (await boundedPathExists(path))) {
|
||||
cases.push({
|
||||
name,
|
||||
path,
|
||||
hasClaudeMd: existsSync(join(path, 'CLAUDE.md')),
|
||||
hasClaudeMd: await boundedPathExists(join(path, 'CLAUDE.md')),
|
||||
linked: true,
|
||||
location: 'linked-local',
|
||||
});
|
||||
@@ -333,7 +337,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
const dockerCaseInfo: CaseInfo = {
|
||||
name: dockerCase.name,
|
||||
path: dockerDisplayPath({ container, path: dockerCase.hostWorkspacePath }),
|
||||
hasClaudeMd: existsSync(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')),
|
||||
hasClaudeMd: await boundedPathExists(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')),
|
||||
location: 'docker',
|
||||
docker: {
|
||||
hostId: host.id,
|
||||
@@ -615,7 +619,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
} else {
|
||||
warnings.push('Kept the repository’s own CLAUDE.md.');
|
||||
}
|
||||
if (repoShipsClaudeSettings(casePath)) {
|
||||
if (await repoShipsClaudeSettings(casePath)) {
|
||||
warnings.push(
|
||||
'This repository ships its own .claude/settings files. Codeman merged its hooks alongside them without removing anything — review them before starting a session, since repo-supplied hooks run on this machine.'
|
||||
);
|
||||
@@ -1618,7 +1622,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
return {
|
||||
name,
|
||||
path: dockerDisplayPath({ container, path: dockerCase.hostWorkspacePath }),
|
||||
hasClaudeMd: existsSync(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')),
|
||||
hasClaudeMd: await boundedPathExists(join(dockerCase.hostWorkspacePath, 'CLAUDE.md')),
|
||||
location: 'docker',
|
||||
docker: {
|
||||
hostId: host.id,
|
||||
@@ -1634,7 +1638,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
|
||||
const casePath = await resolveCasePath(name, getAuthUser(req));
|
||||
|
||||
if (!existsSync(casePath)) {
|
||||
if (!(await boundedPathExists(casePath))) {
|
||||
return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Case not found');
|
||||
}
|
||||
|
||||
@@ -1642,7 +1646,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
return {
|
||||
name,
|
||||
path: casePath,
|
||||
hasClaudeMd: existsSync(join(casePath, 'CLAUDE.md')),
|
||||
hasClaudeMd: await boundedPathExists(join(casePath, 'CLAUDE.md')),
|
||||
...(linked && { linked: true }),
|
||||
};
|
||||
});
|
||||
@@ -1660,7 +1664,7 @@ export function registerCaseRoutes(app: FastifyInstance, ctx: EventPort & Config
|
||||
|
||||
const fixPlanPath = join(casePath, '@fix_plan.md');
|
||||
|
||||
if (!existsSync(fixPlanPath)) {
|
||||
if (!(await boundedPathExists(fixPlanPath))) {
|
||||
return { exists: false, content: null, todos: [] };
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user