Skip to content

Commit 27f3381

Browse files
Stop workflow-controller tests from leaking into ~/.corbits/projects (#603)
* Stop workflow-controller tests from leaking into ~/.corbits/projects 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. * 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.
1 parent e140500 commit 27f3381

4 files changed

Lines changed: 105 additions & 12 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"build": "bun build ./src/index.ts --outdir ./dist --target bun --external '@opentui/core-*' && bun scripts/copy-repo-plugins.ts",
3333
"build:bin": "bun build ./src/index.ts --compile --minify --define process.env.NODE_ENV='\"production\"' --outfile ./dist/corbits && bun scripts/copy-repo-plugins.ts",
3434
"typecheck": "tsc --noEmit",
35-
"test": "bun test ./src ./tests ./evals",
35+
"test": "bun scripts/guard-real-projects-dir.ts ./src ./tests ./evals",
3636
"lint": "prettier --check --cache . && eslint --cache .",
3737
"check": "bun run lint && bun run typecheck && bun run build && bun run test",
3838
"start": "bun run build && bun ./dist/index.js",

scripts/guard-real-projects-dir.ts

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
import { randomUUID } from "node:crypto";
2+
import { homedir, tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
import { mkdir, readdir, rm } from "node:fs/promises";
5+
import { spawn } from "node:child_process";
6+
7+
// Runs `bun test` and fails the run if any test wrote into the real
8+
// ~/.corbits/projects directory. Tests must sandbox state under a temp
9+
// `home` (see src/session/index.ts's `home` overrides); nothing running
10+
// under this wrapper is allowed to fall back to the developer's own
11+
// session history.
12+
//
13+
// This is a backstop, not a substitute for threading `home` correctly: a
14+
// leak is only caught after it already wrote into a real directory once,
15+
// which this script then reports and leaves in place for inspection.
16+
//
17+
// Attribution: a plain before/after snapshot of the whole directory also
18+
// picks up entries from other checkouts on this machine running their own
19+
// `bun run check` concurrently — a routine part of working across several
20+
// worktrees, and not something this run's suite is responsible for. To tell
21+
// the two apart, this run's own temp dirs are pointed at a unique,
22+
// per-invocation scratch directory (via TMPDIR) whose name carries this
23+
// run's id. `src/session/project-key.ts` derives a project key from the
24+
// realpath of the test's `cwd`/`home`, and since those are mkdtemp'd inside
25+
// our scratch dir here, a real leak's project key inherits our run id as a
26+
// substring. Only entries that carry it are ours to fail on; anything else
27+
// is a sibling checkout's own business.
28+
29+
const projectsDir = join(homedir(), ".corbits", "projects");
30+
31+
async function listEntries(): Promise<Set<string>> {
32+
try {
33+
return new Set(await readdir(projectsDir));
34+
} catch (err) {
35+
if ((err as NodeJS.ErrnoException).code === "ENOENT") return new Set();
36+
throw err;
37+
}
38+
}
39+
40+
async function main(): Promise<void> {
41+
const before = await listEntries();
42+
43+
const runId = randomUUID();
44+
const runTmpDir = join(tmpdir(), `corbits-test-guard-${runId}`);
45+
await mkdir(runTmpDir, { recursive: true });
46+
47+
const args = process.argv.slice(2);
48+
const child = spawn("bun", ["test", ...args], {
49+
stdio: "inherit",
50+
env: { ...process.env, TMPDIR: runTmpDir, TMP: runTmpDir, TEMP: runTmpDir },
51+
});
52+
const testExitCode = await new Promise<number>((resolve) => {
53+
child.on("exit", (code) => resolve(code ?? 1));
54+
});
55+
56+
await rm(runTmpDir, { recursive: true, force: true }).catch(() => {});
57+
58+
const after = await listEntries();
59+
const newEntries = [...after].filter((name) => !before.has(name));
60+
const leaked = newEntries.filter((name) => name.includes(runId));
61+
const unattributed = newEntries.filter((name) => !name.includes(runId));
62+
63+
if (unattributed.length > 0) {
64+
process.stderr.write(
65+
`\nguard-real-projects-dir: ignoring ${unattributed.length} new ${projectsDir} ` +
66+
"entries not created by this run (likely another checkout's concurrent " +
67+
`test/check run):\n${unattributed.map((name) => ` ${name}`).join("\n")}\n`,
68+
);
69+
}
70+
71+
if (leaked.length > 0) {
72+
process.stderr.write(
73+
`\nguard-real-projects-dir: ${leaked.length} test run wrote into the real ` +
74+
`${projectsDir} instead of a sandboxed temp dir:\n` +
75+
leaked.map((name) => ` ${name}`).join("\n") +
76+
"\n\nA test must pass an explicit `home` (mkdtemp'd) through to any " +
77+
"function that otherwise defaults to node:os homedir() — see " +
78+
"tests/unit/workflow-controller.test.ts for the pattern.\n",
79+
);
80+
process.exit(1);
81+
}
82+
83+
process.exit(testExitCode);
84+
}
85+
86+
void main();

src/tui/workflow-controller.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ export interface WorkflowControllerArgs {
5555
// The live chat director; the workflow coordinator is attached to it when a
5656
// workflow starts. Returns undefined before the director is built.
5757
getDirector: () => { setWorkflowCoordinator: SetCoordinator } | undefined;
58+
// Overrides the state-tree home (defaults to the real user home). Tests
59+
// pass a sandboxed dir here so persist()/resume() never touch ~/.corbits.
60+
home?: string;
5861
}
5962

6063
// Owns the workflow lifecycle for the TUI: starting, capability overrides,
@@ -116,13 +119,15 @@ export class WorkflowController {
116119
const runtime = this.runtime;
117120
if (runtime === undefined) return;
118121
const sessionId = this.args.getSessionId();
119-
void saveWorkflowState(this.args.cwd, sessionId, runtime.state()).catch((err: unknown) => {
120-
const reason = err instanceof Error ? err.message : String(err);
121-
warnWorkflowPersistenceFailure(
122-
join(sessionDir(this.args.cwd, sessionId), "workflow.json"),
123-
reason,
124-
);
125-
});
122+
void saveWorkflowState(this.args.cwd, sessionId, runtime.state(), this.args.home).catch(
123+
(err: unknown) => {
124+
const reason = err instanceof Error ? err.message : String(err);
125+
warnWorkflowPersistenceFailure(
126+
join(sessionDir(this.args.cwd, sessionId, this.args.home), "workflow.json"),
127+
reason,
128+
);
129+
},
130+
);
126131
}
127132

128133
private attach(workflow: Workflow): void {
@@ -178,7 +183,7 @@ export class WorkflowController {
178183

179184
// Restore a persisted workflow for the current session, if any.
180185
async resume(): Promise<void> {
181-
const state = await loadWorkflowState(this.args.cwd, this.args.getSessionId());
186+
const state = await loadWorkflowState(this.args.cwd, this.args.getSessionId(), this.args.home);
182187
if (state === null || state.completed || state.stack.length === 0) return;
183188
const rootName = state.stack[0]?.workflow;
184189
const workflow = rootName !== undefined ? findWorkflow(rootName) : undefined;

tests/unit/workflow-controller.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ async function withController(
2222
c: WorkflowController,
2323
director: { coordinator: WorkflowCoordinator | undefined },
2424
cwd: string,
25+
home: string,
2526
) => void | Promise<void>,
2627
): Promise<void> {
2728
const cwd = await mkdtemp(join(tmpdir(), "wf-controller-"));
@@ -38,9 +39,10 @@ async function withController(
3839
director.coordinator = c;
3940
},
4041
}),
42+
home,
4143
});
4244
try {
43-
await fn(controller, director, cwd);
45+
await fn(controller, director, cwd, home);
4446
} finally {
4547
await flushWorkflowStateWrites(cwd, "session-1", home);
4648
await rm(cwd, { recursive: true, force: true });
@@ -127,13 +129,13 @@ test("history() entry after workflow completion contains the workflow name and s
127129
});
128130

129131
test("resume() restores an on-disk workflow snapshot for the session", async () => {
130-
await withController([], async (controller, director, cwd) => {
132+
await withController([], async (controller, director, cwd, home) => {
131133
const workflow = findWorkflow("review");
132134
expect(workflow).toBeDefined();
133135
const runtime = new WorkflowRuntime(new Map());
134136
runtime.start(workflow!);
135137
runtime.advance();
136-
await saveWorkflowState(cwd, "session-1", runtime.state());
138+
await saveWorkflowState(cwd, "session-1", runtime.state(), home);
137139

138140
await controller.resume();
139141
expect(controller.isActive()).toBe(true);

0 commit comments

Comments
 (0)