From 513133a0d5cfe0299be96b171213c20a961c6394 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 23 Aug 2026 22:47:52 -0700 Subject: [PATCH 1/2] Stop workflow-controller tests from leaking into ~/.corbits/projects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WorkflowController never threaded an override for the state-tree home, so persist()/resume() always fell back to the real user home even when a test passed a sandboxed one. tests/unit/workflow-controller.test.ts was the concrete leaker: every start()/resume() call it exercised wrote a real session directory (t-wf-controller-*) into ~/.corbits/projects. Add an optional `home` to WorkflowControllerArgs and thread it through persist() and resume(), then pass the test's mkdtemp'd home through the controller and the one direct saveWorkflowState() call that skipped it. Add scripts/guard-real-projects-dir.ts, wired into `bun run test`, which snapshots ~/.corbits/projects before and after the suite and fails the run if anything new appears — a backstop against this class of leak recurring in any test, not just this file. --- package.json | 2 +- scripts/guard-real-projects-dir.ts | 54 ++++++++++++++++++++++++++ src/tui/workflow-controller.ts | 21 ++++++---- tests/unit/workflow-controller.test.ts | 8 ++-- 4 files changed, 73 insertions(+), 12 deletions(-) create mode 100644 scripts/guard-real-projects-dir.ts 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..0e87bb0a1 --- /dev/null +++ b/scripts/guard-real-projects-dir.ts @@ -0,0 +1,54 @@ +import { homedir } from "node:os"; +import { join } from "node:path"; +import { readdir } 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. + +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 args = process.argv.slice(2); + const child = spawn("bun", ["test", ...args], { stdio: "inherit" }); + const testExitCode = await new Promise((resolve) => { + child.on("exit", (code) => resolve(code ?? 1)); + }); + + const after = await listEntries(); + const leaked = [...after].filter((name) => !before.has(name)); + + 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); From 90691803247fcc31a6b4bc608f41532bba26f83c Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 23 Aug 2026 23:50:00 -0700 Subject: [PATCH 2/2] Attribute guard-real-projects-dir leaks to this test run Compare against the real ~/.corbits/projects only for entries this run's own project keys account for, instead of any new entry: a plain before/after snapshot also picks up sibling checkouts running their own bun run check concurrently, which is our normal multi-worktree workflow and not something this suite is responsible for. Point TMPDIR/TMP/TEMP at a per-invocation scratch dir carrying this run's id before spawning bun test. project-key.ts derives a project key from the realpath of a test's mkdtemp'd cwd/home, so a real leak's key inherits the run id as a substring; only those entries fail the guard. --- scripts/guard-real-projects-dir.ts | 40 +++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/scripts/guard-real-projects-dir.ts b/scripts/guard-real-projects-dir.ts index 0e87bb0a1..c80acaa1e 100644 --- a/scripts/guard-real-projects-dir.ts +++ b/scripts/guard-real-projects-dir.ts @@ -1,6 +1,7 @@ -import { homedir } from "node:os"; +import { randomUUID } from "node:crypto"; +import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; -import { readdir } from "node:fs/promises"; +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 @@ -12,6 +13,18 @@ import { spawn } from "node:child_process"; // 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"); @@ -27,14 +40,33 @@ async function listEntries(): Promise> { 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" }); + 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 leaked = [...after].filter((name) => !before.has(name)); + 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(