diff --git a/package.json b/package.json index d3b102bce..d788077b0 100644 --- a/package.json +++ b/package.json @@ -32,7 +32,7 @@ "build": "bun build ./src/index.ts --outdir ./dist --target bun --external '@opentui/core-*' && bun scripts/copy-repo-plugins.ts", "build:bin": "bun build ./src/index.ts --compile --minify --define process.env.NODE_ENV='\"production\"' --outfile ./dist/corbits && bun scripts/copy-repo-plugins.ts", "typecheck": "tsc --noEmit", - "test": "bun test ./src ./tests ./evals", + "test": "bun scripts/guard-real-projects-dir.ts ./src ./tests ./evals", "lint": "prettier --check --cache . && eslint --cache .", "check": "bun run lint && bun run typecheck && bun run build && bun run test", "start": "bun run build && bun ./dist/index.js", diff --git a/scripts/guard-real-projects-dir.ts b/scripts/guard-real-projects-dir.ts new file mode 100644 index 000000000..c80acaa1e --- /dev/null +++ b/scripts/guard-real-projects-dir.ts @@ -0,0 +1,86 @@ +import { randomUUID } from "node:crypto"; +import { homedir, tmpdir } from "node:os"; +import { join } from "node:path"; +import { mkdir, readdir, rm } from "node:fs/promises"; +import { spawn } from "node:child_process"; + +// Runs `bun test` and fails the run if any test wrote into the real +// ~/.corbits/projects directory. Tests must sandbox state under a temp +// `home` (see src/session/index.ts's `home` overrides); nothing running +// under this wrapper is allowed to fall back to the developer's own +// session history. +// +// This is a backstop, not a substitute for threading `home` correctly: a +// leak is only caught after it already wrote into a real directory once, +// which this script then reports and leaves in place for inspection. +// +// Attribution: a plain before/after snapshot of the whole directory also +// picks up entries from other checkouts on this machine running their own +// `bun run check` concurrently — a routine part of working across several +// worktrees, and not something this run's suite is responsible for. To tell +// the two apart, this run's own temp dirs are pointed at a unique, +// per-invocation scratch directory (via TMPDIR) whose name carries this +// run's id. `src/session/project-key.ts` derives a project key from the +// realpath of the test's `cwd`/`home`, and since those are mkdtemp'd inside +// our scratch dir here, a real leak's project key inherits our run id as a +// substring. Only entries that carry it are ours to fail on; anything else +// is a sibling checkout's own business. + +const projectsDir = join(homedir(), ".corbits", "projects"); + +async function listEntries(): Promise> { + try { + return new Set(await readdir(projectsDir)); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") return new Set(); + throw err; + } +} + +async function main(): Promise { + const before = await listEntries(); + + const runId = randomUUID(); + const runTmpDir = join(tmpdir(), `corbits-test-guard-${runId}`); + await mkdir(runTmpDir, { recursive: true }); + + const args = process.argv.slice(2); + const child = spawn("bun", ["test", ...args], { + stdio: "inherit", + env: { ...process.env, TMPDIR: runTmpDir, TMP: runTmpDir, TEMP: runTmpDir }, + }); + const testExitCode = await new Promise((resolve) => { + child.on("exit", (code) => resolve(code ?? 1)); + }); + + await rm(runTmpDir, { recursive: true, force: true }).catch(() => {}); + + const after = await listEntries(); + const newEntries = [...after].filter((name) => !before.has(name)); + const leaked = newEntries.filter((name) => name.includes(runId)); + const unattributed = newEntries.filter((name) => !name.includes(runId)); + + if (unattributed.length > 0) { + process.stderr.write( + `\nguard-real-projects-dir: ignoring ${unattributed.length} new ${projectsDir} ` + + "entries not created by this run (likely another checkout's concurrent " + + `test/check run):\n${unattributed.map((name) => ` ${name}`).join("\n")}\n`, + ); + } + + if (leaked.length > 0) { + process.stderr.write( + `\nguard-real-projects-dir: ${leaked.length} test run wrote into the real ` + + `${projectsDir} instead of a sandboxed temp dir:\n` + + leaked.map((name) => ` ${name}`).join("\n") + + "\n\nA test must pass an explicit `home` (mkdtemp'd) through to any " + + "function that otherwise defaults to node:os homedir() — see " + + "tests/unit/workflow-controller.test.ts for the pattern.\n", + ); + process.exit(1); + } + + process.exit(testExitCode); +} + +void main(); diff --git a/src/tui/workflow-controller.ts b/src/tui/workflow-controller.ts index 5ca6080a8..c0e74ab8a 100644 --- a/src/tui/workflow-controller.ts +++ b/src/tui/workflow-controller.ts @@ -55,6 +55,9 @@ export interface WorkflowControllerArgs { // The live chat director; the workflow coordinator is attached to it when a // workflow starts. Returns undefined before the director is built. getDirector: () => { setWorkflowCoordinator: SetCoordinator } | undefined; + // Overrides the state-tree home (defaults to the real user home). Tests + // pass a sandboxed dir here so persist()/resume() never touch ~/.corbits. + home?: string; } // Owns the workflow lifecycle for the TUI: starting, capability overrides, @@ -116,13 +119,15 @@ export class WorkflowController { const runtime = this.runtime; if (runtime === undefined) return; const sessionId = this.args.getSessionId(); - void saveWorkflowState(this.args.cwd, sessionId, runtime.state()).catch((err: unknown) => { - const reason = err instanceof Error ? err.message : String(err); - warnWorkflowPersistenceFailure( - join(sessionDir(this.args.cwd, sessionId), "workflow.json"), - reason, - ); - }); + void saveWorkflowState(this.args.cwd, sessionId, runtime.state(), this.args.home).catch( + (err: unknown) => { + const reason = err instanceof Error ? err.message : String(err); + warnWorkflowPersistenceFailure( + join(sessionDir(this.args.cwd, sessionId, this.args.home), "workflow.json"), + reason, + ); + }, + ); } private attach(workflow: Workflow): void { @@ -178,7 +183,7 @@ export class WorkflowController { // Restore a persisted workflow for the current session, if any. async resume(): Promise { - const state = await loadWorkflowState(this.args.cwd, this.args.getSessionId()); + const state = await loadWorkflowState(this.args.cwd, this.args.getSessionId(), this.args.home); if (state === null || state.completed || state.stack.length === 0) return; const rootName = state.stack[0]?.workflow; const workflow = rootName !== undefined ? findWorkflow(rootName) : undefined; diff --git a/tests/unit/workflow-controller.test.ts b/tests/unit/workflow-controller.test.ts index 3f85b3426..668c58475 100644 --- a/tests/unit/workflow-controller.test.ts +++ b/tests/unit/workflow-controller.test.ts @@ -22,6 +22,7 @@ async function withController( c: WorkflowController, director: { coordinator: WorkflowCoordinator | undefined }, cwd: string, + home: string, ) => void | Promise, ): Promise { const cwd = await mkdtemp(join(tmpdir(), "wf-controller-")); @@ -38,9 +39,10 @@ async function withController( director.coordinator = c; }, }), + home, }); try { - await fn(controller, director, cwd); + await fn(controller, director, cwd, home); } finally { await flushWorkflowStateWrites(cwd, "session-1", home); await rm(cwd, { recursive: true, force: true }); @@ -127,13 +129,13 @@ test("history() entry after workflow completion contains the workflow name and s }); test("resume() restores an on-disk workflow snapshot for the session", async () => { - await withController([], async (controller, director, cwd) => { + await withController([], async (controller, director, cwd, home) => { const workflow = findWorkflow("review"); expect(workflow).toBeDefined(); const runtime = new WorkflowRuntime(new Map()); runtime.start(workflow!); runtime.advance(); - await saveWorkflowState(cwd, "session-1", runtime.state()); + await saveWorkflowState(cwd, "session-1", runtime.state(), home); await controller.resume(); expect(controller.isActive()).toBe(true);