Merge pull request #356 from timkjr/pr/test-isolation

fix(test): isolate route tests from the production ~/.codeman data dir
This commit is contained in:
Ark0N
2026-09-04 13:50:03 +02:00
committed by GitHub
16 changed files with 166 additions and 97 deletions
+7
View File
@@ -23,6 +23,13 @@ export default defineConfig({
include: ['test/**/*.test.ts'],
exclude: [...configDefaults.exclude, ...NON_CI_TEST_GLOBS],
setupFiles: ['./test/setup.ts'],
// SAFETY: force every worker's data dir away from prod `~/.codeman`. Route
// tests (e.g. session-routes-workspace-hooks) write remote-hosts.json into
// `getDataDir()`; without this a bare run clobbers the production host
// registry (found 2026-08-29). `/tmp` is fine here — the tree is throwaway.
env: {
CODEMAN_DATA_DIR: '/tmp/codeman-vitest-data',
},
fileParallelism: false,
testTimeout: 30000,
teardownTimeout: 60000,
+7
View File
@@ -21,6 +21,13 @@ export default defineConfig({
environment: 'node',
include: ['test/**/*.test.ts'],
setupFiles: ['./test/setup.ts'],
// SAFETY: force every worker's data dir away from prod `~/.codeman`. Route
// tests (e.g. session-routes-workspace-hooks) write remote-hosts.json into
// `getDataDir()`; without this a bare run clobbers the production host
// registry (found 2026-08-29). `/tmp` is fine here — the tree is throwaway.
env: {
CODEMAN_DATA_DIR: '/tmp/codeman-vitest-data',
},
// Run test files sequentially to respect mux session limits
// Individual tests within files still run in parallel where safe
fileParallelism: false,
+9 -4
View File
@@ -18,6 +18,7 @@ import { mkdirSync, rmSync, writeFileSync, existsSync } from 'node:fs';
import { homedir } from 'node:os';
import { join } from 'node:path';
import { dataPath } from '../src/config/instance.js';
import { safeRmHomeTree } from './mocks/index.js';
import { program, resolveCliCasePath, resolveSkillTargetPath } from '../src/cli.js';
import { getCasesDir } from '../src/config/cases-dir.js';
@@ -37,14 +38,18 @@ function writeLinkedCases(content: string): void {
beforeEach(() => {
rmSync(LINKED_CASES_FILE, { force: true });
rmSync(CASES_DIR, { recursive: true, force: true });
rmSync(LINKED_ROOT, { recursive: true, force: true });
safeRmHomeTree(CASES_DIR);
safeRmHomeTree(LINKED_ROOT);
});
afterEach(() => {
// LINKED_CASES_FILE is dataPath('linked-cases.json') → CODEMAN_DATA_DIR,
// which test/setup.ts points at a throwaway /tmp dir, so a plain delete is
// safe here. Only homedir()-derived paths (CASES_DIR/LINKED_ROOT) need the
// containment gate.
rmSync(LINKED_CASES_FILE, { force: true });
rmSync(CASES_DIR, { recursive: true, force: true });
rmSync(LINKED_ROOT, { recursive: true, force: true });
safeRmHomeTree(CASES_DIR);
safeRmHomeTree(LINKED_ROOT);
});
describe('resolveSkillTargetPath (global)', () => {
+7 -14
View File
@@ -1,8 +1,8 @@
import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { existsSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { homedir } from 'node:os';
import { safeRmHomeTree } from './mocks/index.js';
const TEST_PORT = 3110;
const CASES_DIR = join(homedir(), 'codeman-cases');
@@ -19,13 +19,12 @@ describe('Edge Cases and Error Handling', () => {
});
afterEach(() => {
// Clean up cases created during this test
// Clean up cases created during this test. SAFETY: CASES_DIR is
// homedir()-derived, which on some platforms ignores the test HOME — the
// containment gate refuses to delete anything not under the temp HOME.
while (createdCases.length > 0) {
const caseName = createdCases.pop()!;
const casePath = join(CASES_DIR, caseName);
if (existsSync(casePath)) {
rmSync(casePath, { recursive: true, force: true });
}
safeRmHomeTree(join(CASES_DIR, caseName));
}
});
@@ -349,15 +348,9 @@ describe('Concurrent Session Handling', () => {
}
}
// Cleanup
const { rmSync, existsSync } = await import('node:fs');
const { join } = await import('node:path');
const { homedir } = await import('node:os');
// Cleanup (containment-gated: never touch prod ~/codeman-cases)
for (const name of createdCases) {
const path = join(homedir(), 'codeman-cases', name);
if (existsSync(path)) {
rmSync(path, { recursive: true, force: true });
}
safeRmHomeTree(join(homedir(), 'codeman-cases', name));
}
});
});
+6 -11
View File
@@ -1,8 +1,8 @@
import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { existsSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { homedir } from 'node:os';
import { safeRmHomeTree } from './mocks/index.js';
const TEST_PORT = 3115;
const CASES_DIR = join(homedir(), 'codeman-cases');
@@ -24,13 +24,11 @@ describe('Integration Flows', () => {
});
afterEach(() => {
// Clean up cases created during this test
// Clean up cases created during this test (containment-gated: never
// delete a case dir outside the temp HOME, e.g. prod ~/codeman-cases on
// platforms where os.homedir() ignores $HOME).
while (createdCases.length > 0) {
const caseName = createdCases.pop()!;
const casePath = join(CASES_DIR, caseName);
if (existsSync(casePath)) {
rmSync(casePath, { recursive: true, force: true });
}
safeRmHomeTree(join(CASES_DIR, createdCases.pop()!));
}
});
@@ -296,10 +294,7 @@ describe('SSE Event Flow', () => {
} catch {}
}
for (const caseName of createdCases) {
const casePath = join(CASES_DIR, caseName);
if (existsSync(casePath)) {
rmSync(casePath, { recursive: true, force: true });
}
safeRmHomeTree(join(CASES_DIR, caseName));
}
await server.stop();
}, 60000);
+1 -1
View File
@@ -7,5 +7,5 @@
export { MockSession, createMockSession, terminalOutputs } from './mock-session.js';
export { MockStateStore } from './mock-state-store.js';
export { waitForEvent, createDeferred } from './test-helpers.js';
export { waitForEvent, createDeferred, safeRmHomeTree, isUnderTestHome } from './test-helpers.js';
export { createMockRouteContext, type MockRouteContext } from './mock-route-context.js';
+39
View File
@@ -2,6 +2,9 @@
* Reusable async test helpers.
*/
import { rmSync } from 'node:fs';
import { resolve } from 'node:path';
/** Wait for an EventEmitter to emit a specific event, with timeout */
export function waitForEvent(
emitter: { once: (event: string, listener: (...args: unknown[]) => void) => void },
@@ -34,3 +37,39 @@ export function createDeferred<T = void>(): {
});
return { promise, resolve, reject };
}
/**
* Delete a directory tree, but ONLY when it lives inside the test HOME.
*
* SAFETY (2026-08-29): `test/setup.ts` redirects `process.env.HOME` to a
* throwaway dir, but code that resolves paths via `os.homedir()` does NOT
* follow that redirect on every Linux build/Node version — some read
* /etc/passwd instead of $HOME. A test that `rmSync(CASES_DIR, recursive)` can
* therefore delete the PRODUCTION `~/codeman-cases` (or any home-anchored
* tree) on those platforms. This gate refuses to delete anything not under the
* (redirected) `process.env.HOME`. Lexical `resolve()` is used because the leaf
* often does not exist and `realpathSync` would throw.
*/
export function safeRmHomeTree(path: string): void {
const home = process.env.HOME;
if (!home) return;
const target = resolve(path);
const root = resolve(home);
if (target === root || target.startsWith(root + '/')) {
rmSync(target, { recursive: true, force: true });
}
}
/**
* True when `path` resolves strictly inside `process.env.HOME` (or to it).
* Same rationale as `safeRmHomeTree`; use for guarded non-recursive deletes
* (single files like `linked-cases.json`) so they never touch prod state on
* platforms where `os.homedir()` ignores `$HOME`.
*/
export function isUnderTestHome(path: string): boolean {
const home = process.env.HOME;
if (!home) return false;
const target = resolve(path);
const root = resolve(home);
return target === root || target.startsWith(root + '/');
}
+5 -10
View File
@@ -12,7 +12,9 @@
*/
import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { safeRmHomeTree } from './mocks/index.js';
import { homedir } from 'node:os';
import { join } from 'node:path';
const TEST_PORT = 3215;
// Helper to parse SSE events from raw text
@@ -1043,15 +1045,8 @@ describe('Operation Lightspeed', () => {
const caseEvent = events.find((e) => e.event === 'case:created');
expect(caseEvent).toBeDefined();
// Cleanup
const { rmSync } = await import('node:fs');
const { join } = await import('node:path');
const { homedir } = await import('node:os');
try {
rmSync(join(homedir(), 'codeman-cases', caseName), { recursive: true });
} catch {
/* may not exist */
}
// Cleanup (containment-gated: never touch prod ~/codeman-cases)
safeRmHomeTree(join(homedir(), 'codeman-cases', caseName));
});
it('should deliver session:created to every client, even those with a mismatched filter', async () => {
+4 -7
View File
@@ -13,9 +13,9 @@
import { describe, it, expect, beforeAll, afterAll, afterEach } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { existsSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { homedir } from 'node:os';
import { safeRmHomeTree } from './mocks/index.js';
const TEST_PORT = 3125;
const CASES_DIR = join(homedir(), 'codeman-cases');
@@ -33,13 +33,10 @@ describe('Ralph Integration Tests', () => {
});
afterEach(() => {
// Clean up cases created during this test
// Clean up cases created during this test (containment-gated: never
// delete a case dir outside the temp HOME).
while (createdCases.length > 0) {
const caseName = createdCases.pop()!;
const casePath = join(CASES_DIR, caseName);
if (existsSync(casePath)) {
rmSync(casePath, { recursive: true, force: true });
}
safeRmHomeTree(join(CASES_DIR, createdCases.pop()!));
}
});
+7 -5
View File
@@ -9,8 +9,10 @@
* working tree in the case directory, that scaffolding does not overwrite the
* repository's own files, and that a rejected URL never reaches git.
*
* `test/setup.ts` points HOME at a per-file temp dir, so CASES_DIR resolves
* inside the fixture and nothing touches the developer's real ~/codeman-cases.
* `test/setup.ts` points HOME at a per-file temp dir, but CASES_DIR is
* `join(homedir(), 'codeman-cases')` and `os.homedir()` ignores the HOME
* override on some platforms/Node builds — so cleanup below goes through
* `safeRmHomeTree`, which refuses to delete anything outside the temp HOME.
*
* Port: N/A (app.inject).
*/
@@ -31,7 +33,7 @@ import {
} from 'node:fs';
import { homedir, tmpdir } from 'node:os';
import { join } from 'node:path';
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
import { createMockRouteContext, safeRmHomeTree, 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';
@@ -147,7 +149,7 @@ describe('POST /api/cases/clone — input rejection', () => {
expect(res.statusCode).toBe(httpStatusForErrorCode(ApiErrorCode.ALREADY_EXISTS));
expect(JSON.parse(res.body).error).toMatch(/already exists/i);
} finally {
rmSync(join(CASES_DIR, 'taken'), { recursive: true, force: true });
safeRmHomeTree(join(CASES_DIR, 'taken'));
}
});
});
@@ -217,7 +219,7 @@ describe.skipIf(!gitPresent)('POST /api/cases/clone — real clone', () => {
afterAll(() => {
rmSync(root, { recursive: true, force: true });
for (const name of created) rmSync(join(CASES_DIR, name), { recursive: true, force: true });
for (const name of created) safeRmHomeTree(join(CASES_DIR, name));
});
beforeEach(buildApp);
@@ -19,14 +19,14 @@
* including the sweep's deleted-workspace guard.
*/
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import Fastify, { type FastifyInstance } from 'fastify';
import fastifyCookie from '@fastify/cookie';
import { mkdtemp, rm, readFile, mkdir, writeFile } from 'node:fs/promises';
import { existsSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
import { createMockRouteContext, safeRmHomeTree, type MockRouteContext } from '../mocks/index.js';
import { installRouteErrorHandler } from '../../src/web/route-error-handler.js';
import { registerSessionRoutes } from '../../src/web/routes/session-routes.js';
import { generateHooksConfig, applyWorkspaceHooks } from '../../src/hooks-config.js';
@@ -168,6 +168,16 @@ describe('POST /api/sessions workspace hooks', () => {
// `user@host:session` — locally a RELATIVE path, so a mkdir would create it
// as a junk directory under the server cwd. statusLineTelemetry rides along:
// applyStatusLineConfig mkdirs the same way and used to run for remote attaches.
// SAFETY (2026-08-29): write straight to `getDataDir()` — `test/setup.ts`
// already sandboxes CODEMAN_DATA_DIR for the whole file (same convention as
// the docker-hosts fixtures below). A prior version of this test stubbed
// CODEMAN_DATA_DIR to a SEPARATE throwaway dir for just this write, but
// `session-routes.ts`'s `CODEMAN_CONFIG_DIR` is a module-load-time constant
// (frozen at the sandboxed dir before this test ever runs), so that fixture
// landed somewhere the route handler could never read it — the remote host
// lookup silently failed and the test passed for the wrong reason (Fastify
// defaults an unset reply code to 200, so the NOT_FOUND branch and the
// intended success branch were indistinguishable by status code alone).
await mkdir(getDataDir(), { recursive: true });
await writeFile(
join(getDataDir(), 'remote-hosts.json'),
@@ -248,7 +258,11 @@ describe('POST /api/quick-start workspace hooks', () => {
// Docker fixtures + case dirs must not leak into the next test.
await rm(join(getDataDir(), 'docker-hosts.json'), { force: true });
await rm(join(getDataDir(), 'docker-cases.json'), { force: true });
await rm(CASES_DIR, { recursive: true, force: true });
// SAFETY (2026-08-29): CASES_DIR is `join(homedir(), 'codeman-cases')`, and
// on environments where `os.homedir()` ignores `$HOME` it resolves to the
// PROD case tree. `safeRmHomeTree` refuses to delete anything not under the
// redirected test HOME, so a run can never nuke the real `~/codeman-cases`.
safeRmHomeTree(CASES_DIR);
});
it('installs hooks into an EXISTING case directory (a linked case / cloned repo)', async () => {
+22 -14
View File
@@ -19,12 +19,33 @@ import Fastify, { type FastifyInstance } from 'fastify';
import fastifyWebsocket from '@fastify/websocket';
import WebSocket, { WebSocketServer } from 'ws';
import { mkdirSync, writeFileSync, rmSync } from 'node:fs';
import { homedir } from 'node:os';
import { join } from 'node:path';
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
import { registerVoiceRoutes, _resetVoiceStreamCountForTesting } from '../../src/web/routes/voice-routes.js';
import { MAX_CONCURRENT_STREAMS } from '../../src/config/voice.js';
// SAFETY (2026-08-29): anchor on the REDIRECTED test HOME (process.env.HOME,
// which test/setup.ts points at a throwaway dir) instead of os.homedir().
// On some Linux builds os.homedir() reads /etc/passwd and would resolve to the
// REAL home, clobbering the user's ~/.claude/.credentials.json.
function testHome(): string {
if (!process.env.HOME) throw new Error('process.env.HOME unset — test/setup.ts must run first');
return process.env.HOME;
}
function writeCredentials(expiresAt: number | undefined): void {
const dir = join(testHome(), '.claude');
mkdirSync(dir, { recursive: true });
writeFileSync(
join(dir, '.credentials.json'),
JSON.stringify({ claudeAiOauth: { accessToken: TOKEN, expiresAt, subscriptionType: 'max' } })
);
}
function removeCredentials(): void {
rmSync(join(testHome(), '.claude', '.credentials.json'), { force: true });
}
const PORT = 3230;
const UPSTREAM_PORT = 3231;
const TOKEN = 'sk-ant-oat01-voice-route-test';
@@ -38,19 +59,6 @@ interface UpstreamCapture {
socket: WebSocket | null;
}
function writeCredentials(expiresAt: number | undefined): void {
const dir = join(homedir(), '.claude');
mkdirSync(dir, { recursive: true });
writeFileSync(
join(dir, '.credentials.json'),
JSON.stringify({ claudeAiOauth: { accessToken: TOKEN, expiresAt, subscriptionType: 'max' } })
);
}
function removeCredentials(): void {
rmSync(join(homedir(), '.claude', '.credentials.json'), { force: true });
}
function waitForClose(ws: WebSocket, timeoutMs = 3000): Promise<{ code: number; reason: string }> {
return new Promise((resolve, reject) => {
const timer = setTimeout(() => reject(new Error('WS close timeout')), timeoutMs);
+5 -11
View File
@@ -1,8 +1,9 @@
import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { existsSync, mkdtempSync, rmSync } from 'node:fs';
import { mkdtempSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { homedir, tmpdir } from 'node:os';
import { safeRmHomeTree } from './mocks/index.js';
const TEST_PORT = 3120;
const CASES_DIR = join(homedir(), 'codeman-cases');
@@ -27,13 +28,9 @@ describe('Session Cleanup', () => {
});
afterEach(() => {
// Clean up cases created during this test
// Clean up cases created during this test (containment-gated).
while (createdCases.length > 0) {
const caseName = createdCases.pop()!;
const casePath = join(CASES_DIR, caseName);
if (existsSync(casePath)) {
rmSync(casePath, { recursive: true, force: true });
}
safeRmHomeTree(join(CASES_DIR, createdCases.pop()!));
}
});
@@ -228,10 +225,7 @@ describe('Resource Management', () => {
afterAll(async () => {
for (const caseName of createdCases) {
const casePath = join(CASES_DIR, caseName);
if (existsSync(casePath)) {
rmSync(casePath, { recursive: true, force: true });
}
safeRmHomeTree(join(CASES_DIR, caseName));
}
await server.stop();
}, 60000);
+20 -1
View File
@@ -18,7 +18,9 @@ const originalHome = process.env.HOME;
const originalUserProfile = process.env.USERPROFILE;
const originalVitest = process.env.VITEST;
const originalPlaywrightBrowsersPath = process.env.PLAYWRIGHT_BROWSERS_PATH;
const originalCodemanDataDir = process.env.CODEMAN_DATA_DIR;
const testHome = mkdtempSync(join(tmpdir(), 'codeman-vitest-'));
const testDataDir = join(tmpdir(), `codeman-vitest-data-${process.pid}`);
if (originalPlaywrightBrowsersPath === undefined && originalHome) {
process.env.PLAYWRIGHT_BROWSERS_PATH =
@@ -32,6 +34,16 @@ process.env.HOME = testHome;
process.env.USERPROFILE = testHome;
process.env.VITEST = 'true';
// SAFETY: `getDataDir()` resolves via `homedir()` → `~/.codeman<INSTANCE_SUFFIX>`.
// Overriding HOME above is NOT enough: on Linux `os.homedir()` reads /etc/passwd,
// not $HOME, so without this a route test that writes `remote-hosts.json` (or
// any state file) into `getDataDir()` silently clobbers the PRODUCTION
// `~/.codeman` tree (found 2026-08-29: `session-routes-workspace-hooks.test.ts`
// overwrote prod `remote-hosts.json` with an `h1/box/10.0.0.5` fixture during a
// bare full-suite run, wiping every user-defined remote host and emptying the
// launch case dropdown). Point every test at a throwaway data dir instead.
process.env.CODEMAN_DATA_DIR = testDataDir;
delete process.env.CODEMAN_PASSWORD;
delete process.env.CODEMAN_USERNAME;
// Gesture availability changes renderIndexHtml output (injects the
@@ -50,7 +62,9 @@ afterAll(async () => {
// "onUserConsoleLog" call is still pending, and that single unhandled
// EnvironmentTeardownError fails the run after every test has passed
// (observed twice on the PR #175/#176 merge commit; never locally).
await new Promise((resolve) => setTimeout(resolve, 50));
const { promise: drained, resolve: drainDone } = Promise.withResolvers<void>();
setTimeout(drainDone, 50);
await drained;
if (originalHome === undefined) delete process.env.HOME;
else process.env.HOME = originalHome;
@@ -64,7 +78,11 @@ afterAll(async () => {
if (originalPlaywrightBrowsersPath === undefined) delete process.env.PLAYWRIGHT_BROWSERS_PATH;
else process.env.PLAYWRIGHT_BROWSERS_PATH = originalPlaywrightBrowsersPath;
if (originalCodemanDataDir === undefined) delete process.env.CODEMAN_DATA_DIR;
else process.env.CODEMAN_DATA_DIR = originalCodemanDataDir;
rmSync(testHome, { recursive: true, force: true });
rmSync(testDataDir, { recursive: true, force: true });
});
// afterAll never fires for a fully-skipped test file (no tests execute), which
@@ -72,4 +90,5 @@ afterAll(async () => {
// with force is a no-op when afterAll already removed it.
process.on('exit', () => {
rmSync(testHome, { recursive: true, force: true });
rmSync(testDataDir, { recursive: true, force: true });
});
+5 -7
View File
@@ -1,6 +1,9 @@
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { EventEmitter } from 'node:events';
import { safeRmHomeTree } from './mocks/index.js';
import { homedir } from 'node:os';
import { join } from 'node:path';
const TEST_PORT = 3107;
@@ -295,13 +298,8 @@ describe('SSE Event Types', () => {
expect(caseCreated).toBeDefined();
expect((caseCreated?.data as any).name).toBe(caseName);
// Cleanup
const { rmSync } = await import('node:fs');
const { join } = await import('node:path');
const { homedir } = await import('node:os');
try {
rmSync(join(homedir(), 'codeman-cases', caseName), { recursive: true });
} catch {}
// Cleanup (containment-gated)
safeRmHomeTree(join(homedir(), 'codeman-cases', caseName));
});
});
});
+5 -9
View File
@@ -1,5 +1,8 @@
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { WebServer } from '../src/web/server.js';
import { safeRmHomeTree } from './mocks/index.js';
import { homedir } from 'node:os';
import { join } from 'node:path';
const TEST_PORT = 3212;
@@ -437,14 +440,7 @@ describe('SSE Subscription Filtering', () => {
expect(caseCreated).toBeDefined();
expect((caseCreated?.data as any).name).toBe(caseName);
// Cleanup
const { rmSync } = await import('node:fs');
const { join } = await import('node:path');
const { homedir } = await import('node:os');
try {
rmSync(join(homedir(), 'codeman-cases', caseName), { recursive: true });
} catch {
/* may not exist */
}
// Cleanup (containment-gated)
safeRmHomeTree(join(homedir(), 'codeman-cases', caseName));
});
});