mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-06 15:39:41 +02:00
fix(cases): tell an unreachable path from an absent one, scope the stall cap
The bounded path probe answered "absent" both when a path did not exist and
when it simply did not answer, so a stalled linked case 404'd and the Run
button scaffolded a stray local case over it, and two stalled paths anywhere
made every unrelated path read as absent (hooks skipped, statusLine
overridden, the clone warning lost).
- probePath()/probePathKind() are tri-state: present (or directory/file),
absent (ENOENT/ENOTDIR only) and unknown (timeout, other errors, refusal).
boundedPathExists() stays as the display-only boolean.
- A stalled path takes only its own mount out of probing (deepest mount
point from /proc/self/mounts, never /; just the path itself when there is
no mount table). Unrelated paths keep probing. The process-wide cap is a
backstop that answers unknown, and a single-path user request can probe
past it ({ pastCap: true }), still bounded and still recorded as stalled.
One console.warn when a path first stalls and one when the cap engages.
- GET /api/cases/:name keeps NOT_FOUND for definite absence only. An
unreachable linked case answers with its registered path and
unreachable: true; a local one answers OPERATION_FAILED. runClaude and
runShell create a case only on errorCode NOT_FOUND. The case list keeps an
unreachable linked case, marked unreachable, instead of dropping it, and
fix-plan reports an unreadable plan as an error, not "no plan".
- applyWorkspaceHooks and the statusLine helpers skip only a workspace that
is absent or on the stalled mount; a capacity refusal no longer stops
hooks being installed elsewhere, and an unreadable settings file never
lets the exporter override a user's own statusLine.
- The clone flow's repo-settings warning is back on its synchronous check,
and stripCaseEnvKeys uses pathExistsForWrite.
- POST /api/sessions (workingDir) and POST /api/quick-start (case folder)
probe with the bounded probe instead of statSync/existsSync. Missing and
non-directory keep INVALID_INPUT; unknown is OPERATION_FAILED, and
quick-start never scaffolds over a folder that did not answer.
- PATH_PROBE_TIMEOUT_MS and MAX_STALLED_PATH_PROBES move to
src/config/path-probe.ts, overridable via CODEMAN_PATH_PROBE_TIMEOUT_MS
(default 1500) and CODEMAN_PATH_PROBE_MAX_STALLED (default 3), and are
documented in the Settings Reference.
- The probe is exported from the utils barrel and imported from there.
This commit is contained in:
@@ -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<typeof import('node:fs/promises')>();
|
||||
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 });
|
||||
|
||||
@@ -13,13 +13,24 @@
|
||||
* behavior matches production exactly).
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { describe, it, expect, beforeEach, afterEach, afterAll, vi } from 'vitest';
|
||||
import Fastify, { type FastifyInstance } from 'fastify';
|
||||
import fastifyCookie from '@fastify/cookie';
|
||||
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
|
||||
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
|
||||
import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js';
|
||||
import { registerCaseRoutes } from '../../src/web/routes/case-routes.js';
|
||||
import { probePath } from '../../src/utils/index.js';
|
||||
import { MAX_STALLED_PATH_PROBES } from '../../src/config/path-probe.js';
|
||||
|
||||
// A short path-probe timeout keeps the unreachable-mount tests quick. Read when the
|
||||
// probe's config module is first imported, so it is set before any import runs.
|
||||
vi.hoisted(() => {
|
||||
process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS = '300';
|
||||
});
|
||||
afterAll(() => {
|
||||
delete process.env.CODEMAN_PATH_PROBE_TIMEOUT_MS;
|
||||
});
|
||||
|
||||
// Mock filesystem modules
|
||||
vi.mock('node:fs', async (importOriginal) => {
|
||||
@@ -251,8 +262,19 @@ describe('case-routes', () => {
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(elapsed).toBeLessThan(BLOCK_MS - 1_000);
|
||||
// The unreachable case is left out rather than holding the list hostage.
|
||||
expect(JSON.parse(res.body).data).toEqual([]);
|
||||
// The unreachable case is listed as such rather than holding the list
|
||||
// hostage, or vanishing as though it had been deleted.
|
||||
expect(JSON.parse(res.body).data).toEqual([
|
||||
{
|
||||
name: 'linked-nfs',
|
||||
path: stalledPath,
|
||||
hasClaudeMd: false,
|
||||
linked: true,
|
||||
location: 'linked-local',
|
||||
unreachable: true,
|
||||
},
|
||||
]);
|
||||
await new Promise((r) => setTimeout(r, 0)); // let the released stat clear its stall
|
||||
});
|
||||
});
|
||||
|
||||
@@ -714,6 +736,64 @@ describe('case-routes', () => {
|
||||
expect(body.data.name).toBe('regular-case');
|
||||
});
|
||||
|
||||
it('answers a linked case on an unreachable mount with its registered path, not NOT_FOUND', async () => {
|
||||
// The timeout path: the mount does not answer at all.
|
||||
const stalledPath = '/mnt/unreachable/linked-get';
|
||||
mockedReadFile.mockResolvedValue(JSON.stringify({ 'linked-get': stalledPath }) as never);
|
||||
let release: (() => void) | undefined;
|
||||
mockedStat.mockImplementation((p) => {
|
||||
if (String(p) !== stalledPath) {
|
||||
return Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
|
||||
}
|
||||
return new Promise((resolve) => {
|
||||
release = () => resolve({ isDirectory: () => true } as never);
|
||||
});
|
||||
});
|
||||
|
||||
const res = await harness.app.inject({ method: 'GET', url: '/api/cases/linked-get' });
|
||||
release?.();
|
||||
await new Promise((r) => setTimeout(r, 0)); // let the released stat clear its stall
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.success).toBe(true);
|
||||
expect(body.data).toMatchObject({ name: 'linked-get', path: stalledPath, linked: true, unreachable: true });
|
||||
});
|
||||
|
||||
it('still answers a healthy case while unrelated mounts are stalled past the cap', async () => {
|
||||
const dead = Array.from({ length: MAX_STALLED_PATH_PROBES }, (_, i) => `/mnt/dead-${i}/linked`);
|
||||
const releases: Array<() => void> = [];
|
||||
mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
|
||||
mockedStat.mockImplementation((p) => {
|
||||
if (dead.includes(String(p))) {
|
||||
return new Promise((resolve) => releases.push(() => resolve({ isDirectory: () => true } as never)));
|
||||
}
|
||||
return Promise.resolve({ isDirectory: () => true } as never);
|
||||
});
|
||||
expect(await Promise.all(dead.map((p) => probePath(p)))).toEqual(dead.map(() => 'unknown'));
|
||||
|
||||
const res = await harness.app.inject({ method: 'GET', url: '/api/cases/healthy-local' });
|
||||
releases.forEach((release) => release());
|
||||
await new Promise((r) => setTimeout(r, 0));
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(JSON.parse(res.body).data).toMatchObject({ name: 'healthy-local' });
|
||||
expect(JSON.parse(res.body).data.unreachable).toBeUndefined();
|
||||
});
|
||||
|
||||
it('answers a local case it cannot read with a non-NOT_FOUND error', async () => {
|
||||
// A soft mount that gave up (EIO) is not proof the case is gone, and the Run
|
||||
// button creates a case on NOT_FOUND.
|
||||
mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
|
||||
mockedStat.mockRejectedValue(Object.assign(new Error('EIO'), { code: 'EIO' }));
|
||||
|
||||
const res = await harness.app.inject({ method: 'GET', url: '/api/cases/eio-case' });
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.success).toBe(false);
|
||||
expect(body.errorCode).toBe('OPERATION_FAILED');
|
||||
expect(res.statusCode).not.toBe(404);
|
||||
});
|
||||
|
||||
it('returns error when case not found anywhere', async () => {
|
||||
mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
|
||||
mockedExistsSync.mockReturnValue(false);
|
||||
@@ -748,6 +828,16 @@ describe('case-routes', () => {
|
||||
expect(body.data.todos).toEqual([]);
|
||||
});
|
||||
|
||||
it('reports an unreadable fix plan as an error, not as "no plan"', async () => {
|
||||
mockedReadFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' }));
|
||||
mockedStat.mockRejectedValue(Object.assign(new Error('EIO'), { code: 'EIO' }));
|
||||
|
||||
const res = await harness.app.inject({ method: 'GET', url: '/api/cases/my-case/fix-plan' });
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.success).toBe(false);
|
||||
expect(body.errorCode).toBe('OPERATION_FAILED');
|
||||
});
|
||||
|
||||
it('parses fix plan with todos and stats', async () => {
|
||||
const fixPlanContent = [
|
||||
'# Fix Plan',
|
||||
|
||||
@@ -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<typeof import('node:fs')>();
|
||||
const freezeOn =
|
||||
<T extends (...args: never[]) => unknown>(fn: T) =>
|
||||
(...args: Parameters<T>): ReturnType<T> => {
|
||||
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<T>;
|
||||
};
|
||||
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<typeof import('node:fs/promises')>();
|
||||
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<typeof vi.spyOn>;
|
||||
|
||||
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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user