Skip to content

Commit 15214b4

Browse files
AlbinoGeekAlbinoGeek
committed
test: clean up tmp dirs created by integration tests
Integration tests called mkdtempSync(tmpdir(), ...) per test without ever removing the resulting dirs. Each `bun test` run leaked ~60 dirs under $TMPDIR; 613 had accumulated locally before this fix. Add mkTmpDir/trackTmpPath/cleanupTmpPaths to the shared test harness and have every test file register `afterEach(cleanupTmpPaths)`. Hook registration is deliberately per-file — a module-scope afterEach in the harness only fires for the first-importing file because bun caches the module across test files in the same run.
1 parent db8f7d0 commit 15214b4

File tree

7 files changed

+98
-46
lines changed

7 files changed

+98
-46
lines changed

‎src/repo-paths.test.ts‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { describe, expect, test } from "bun:test";
2-
import { mkdirSync, mkdtempSync } from "node:fs";
1+
import { afterEach, describe, expect, test } from "bun:test";
2+
import { mkdirSync, mkdtempSync, rmSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join, resolve } from "node:path";
55
import {
@@ -8,27 +8,42 @@ import {
88
resolvePathForRepo,
99
} from "./repo-paths.js";
1010

11+
const created: string[] = [];
12+
13+
function mkTmp(prefix: string): string {
14+
const dir = mkdtempSync(join(tmpdir(), prefix));
15+
created.push(dir);
16+
return dir;
17+
}
18+
19+
afterEach(() => {
20+
while (created.length > 0) {
21+
const dir = created.pop();
22+
if (dir) rmSync(dir, { recursive: true, force: true });
23+
}
24+
});
25+
1126
describe("isStrictlyUnderGitTop", () => {
1227
test("allows same directory as top", () => {
13-
const top = mkdtempSync(join(tmpdir(), "rg-top-"));
28+
const top = mkTmp("rg-top-");
1429
expect(isStrictlyUnderGitTop(top, top)).toBe(true);
1530
});
1631

1732
test("allows child directory", () => {
18-
const top = mkdtempSync(join(tmpdir(), "rg-top-"));
33+
const top = mkTmp("rg-top-");
1934
const child = join(top, "packages", "a");
2035
mkdirSync(child, { recursive: true });
2136
expect(isStrictlyUnderGitTop(child, top)).toBe(true);
2237
});
2338

2439
test("rejects sibling outside top", () => {
25-
const top = mkdtempSync(join(tmpdir(), "rg-top-"));
26-
const outside = mkdtempSync(join(tmpdir(), "rg-out-"));
40+
const top = mkTmp("rg-top-");
41+
const outside = mkTmp("rg-out-");
2742
expect(isStrictlyUnderGitTop(outside, top)).toBe(false);
2843
});
2944

3045
test("rejects path with dotdot past top", () => {
31-
const top = mkdtempSync(join(tmpdir(), "rg-top-"));
46+
const top = mkTmp("rg-top-");
3247
const nested = join(top, "sub", "mod");
3348
mkdirSync(nested, { recursive: true });
3449
const escaped = resolve(join(nested, "..", "..", ".."));
@@ -38,14 +53,14 @@ describe("isStrictlyUnderGitTop", () => {
3853

3954
describe("resolvePathForRepo + assertRelativePathUnderTop", () => {
4055
test("rejects relative segment that escapes top", () => {
41-
const top = mkdtempSync(join(tmpdir(), "rg-top-"));
56+
const top = mkTmp("rg-top-");
4257
const rel = join("..", "..", "etc");
4358
const abs = resolvePathForRepo(rel, top);
4459
expect(assertRelativePathUnderTop(rel, abs, top)).toBe(false);
4560
});
4661

4762
test("allows normal nested relative path", () => {
48-
const top = mkdtempSync(join(tmpdir(), "rg-top-"));
63+
const top = mkTmp("rg-top-");
4964
const pkg = join(top, "pkg");
5065
mkdirSync(pkg);
5166
const abs = resolvePathForRepo("pkg", top);

‎src/server/batch-commit-tool.test.ts‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,17 @@
1616
* 7. Valid path that is exactly the git root is accepted
1717
*/
1818

19-
import { describe, expect, test } from "bun:test";
19+
import { afterEach, describe, expect, test } from "bun:test";
2020
import { type ExecSyncOptionsWithStringEncoding, execFileSync } from "node:child_process";
21-
import { mkdtempSync, writeFileSync } from "node:fs";
22-
import { tmpdir } from "node:os";
21+
import { writeFileSync } from "node:fs";
2322
import { join } from "node:path";
2423

2524
import { isStrictlyUnderGitTop, resolvePathForRepo } from "../repo-paths.js";
2625
import { registerBatchCommitTool } from "./batch-commit-tool.js";
2726
import { spawnGitAsync } from "./git.js";
28-
import { captureTool } from "./test-harness.js";
27+
import { captureTool, cleanupTmpPaths, mkTmpDir } from "./test-harness.js";
28+
29+
afterEach(cleanupTmpPaths);
2930

3031
// ---------------------------------------------------------------------------
3132
// Mirrors the SHA extraction regex from batch-commit-tool.ts
@@ -57,7 +58,7 @@ function gitCmd(cwd: string, ...args: string[]): string {
5758
}
5859

5960
function makeRepo(): string {
60-
const dir = mkdtempSync(join(tmpdir(), "mcp-batch-commit-test-"));
61+
const dir = mkTmpDir("mcp-batch-commit-test-");
6162
gitCmd(dir, "init", "-b", "main");
6263
gitCmd(dir, "config", "user.email", "test@example.com");
6364
gitCmd(dir, "config", "user.name", "Test User");
@@ -71,10 +72,10 @@ function makeRepo(): string {
7172
* `remote` is a bare repo used as `origin`.
7273
*/
7374
function makeRepoWithUpstream(): { work: string; remote: string } {
74-
const remote = mkdtempSync(join(tmpdir(), "mcp-batch-commit-remote-"));
75+
const remote = mkTmpDir("mcp-batch-commit-remote-");
7576
gitCmd(remote, "init", "--bare", "-b", "main");
7677

77-
const work = mkdtempSync(join(tmpdir(), "mcp-batch-commit-work-"));
78+
const work = mkTmpDir("mcp-batch-commit-work-");
7879
gitCmd(work, "init", "-b", "main");
7980
gitCmd(work, "config", "user.email", "test@example.com");
8081
gitCmd(work, "config", "user.name", "Test User");
@@ -124,27 +125,27 @@ describe("extractSha", () => {
124125

125126
describe("path escape detection", () => {
126127
test("dotdot path that escapes root is rejected", () => {
127-
const gitTop = mkdtempSync(join(tmpdir(), "mcp-top-"));
128+
const gitTop = mkTmpDir("mcp-top-");
128129
const rel = "../../etc/passwd";
129130
const abs = resolvePathForRepo(rel, gitTop);
130131
expect(isStrictlyUnderGitTop(abs, gitTop)).toBe(false);
131132
});
132133

133134
test("normal nested path is accepted", () => {
134-
const gitTop = mkdtempSync(join(tmpdir(), "mcp-top-"));
135+
const gitTop = mkTmpDir("mcp-top-");
135136
const rel = "src/foo.ts";
136137
const abs = resolvePathForRepo(rel, gitTop);
137138
expect(isStrictlyUnderGitTop(abs, gitTop)).toBe(true);
138139
});
139140

140141
test("absolute path outside root is rejected", () => {
141-
const gitTop = mkdtempSync(join(tmpdir(), "mcp-top-"));
142-
const outside = mkdtempSync(join(tmpdir(), "mcp-other-"));
142+
const gitTop = mkTmpDir("mcp-top-");
143+
const outside = mkTmpDir("mcp-other-");
143144
expect(isStrictlyUnderGitTop(outside, gitTop)).toBe(false);
144145
});
145146

146147
test("path equal to git root is accepted", () => {
147-
const gitTop = mkdtempSync(join(tmpdir(), "mcp-top-"));
148+
const gitTop = mkTmpDir("mcp-top-");
148149
expect(isStrictlyUnderGitTop(gitTop, gitTop)).toBe(true);
149150
});
150151
});
@@ -297,7 +298,7 @@ describe("batch_commit execute handler", () => {
297298
});
298299

299300
test("non-git workspaceRoot → not_a_git_repository error", async () => {
300-
const plain = mkdtempSync(join(tmpdir(), "mcp-plain-"));
301+
const plain = mkTmpDir("mcp-plain-");
301302

302303
const run = captureTool(registerBatchCommitTool);
303304
const text = await run({

‎src/server/git-cherry-pick-tool.test.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22
* Integration tests for git_cherry_pick.
33
*/
44

5-
import { describe, expect, test } from "bun:test";
5+
import { afterEach, describe, expect, test } from "bun:test";
66
import { type ExecSyncOptionsWithStringEncoding, execFileSync } from "node:child_process";
7-
import { existsSync, mkdtempSync, writeFileSync } from "node:fs";
8-
import { tmpdir } from "node:os";
7+
import { existsSync, writeFileSync } from "node:fs";
98
import { join } from "node:path";
109

1110
import { registerGitCherryPickTool } from "./git-cherry-pick-tool.js";
12-
import { captureTool } from "./test-harness.js";
11+
import { captureTool, cleanupTmpPaths, mkTmpDir } from "./test-harness.js";
12+
13+
afterEach(cleanupTmpPaths);
1314

1415
function gitCmd(cwd: string, ...args: string[]): string {
1516
const opts: ExecSyncOptionsWithStringEncoding = {
@@ -29,7 +30,7 @@ function gitCmd(cwd: string, ...args: string[]): string {
2930
}
3031

3132
function makeRepo(): string {
32-
const dir = mkdtempSync(join(tmpdir(), "mcp-cherry-pick-test-"));
33+
const dir = mkTmpDir("mcp-cherry-pick-test-");
3334
gitCmd(dir, "init", "-b", "main");
3435
gitCmd(dir, "config", "user.email", "test@example.com");
3536
gitCmd(dir, "config", "user.name", "Test User");

‎src/server/git-diff-summary-tool.test.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,15 @@
1414
* 7. Integration: real diff against a throwaway repo
1515
*/
1616

17-
import { describe, expect, test } from "bun:test";
17+
import { afterEach, describe, expect, test } from "bun:test";
1818
import { type ExecSyncOptionsWithStringEncoding, execFileSync } from "node:child_process";
19-
import { mkdtempSync, writeFileSync } from "node:fs";
20-
import { tmpdir } from "node:os";
19+
import { writeFileSync } from "node:fs";
2120
import { join, matchesGlob } from "node:path";
2221
import { isSafeGitUpstreamToken, spawnGitAsync } from "./git.js";
2322
import { registerGitDiffSummaryTool } from "./git-diff-summary-tool.js";
24-
import { captureTool } from "./test-harness.js";
23+
import { captureTool, cleanupTmpPaths, mkTmpDir } from "./test-harness.js";
24+
25+
afterEach(cleanupTmpPaths);
2526

2627
// ---------------------------------------------------------------------------
2728
// Local copies of private helpers (mirrors git-diff-summary-tool.ts)
@@ -152,7 +153,7 @@ function gitCmd(cwd: string, ...args: string[]): string {
152153
}
153154

154155
function makeRepo(): string {
155-
const dir = mkdtempSync(join(tmpdir(), "mcp-git-diff-test-"));
156+
const dir = mkTmpDir("mcp-git-diff-test-");
156157
gitCmd(dir, "init", "-b", "main");
157158
gitCmd(dir, "config", "user.email", "test@example.com");
158159
gitCmd(dir, "config", "user.name", "Test User");
@@ -588,7 +589,7 @@ describe("git_diff_summary execute handler", () => {
588589
});
589590

590591
test("non-git workspaceRoot → not_a_git_repository error", async () => {
591-
const plain = mkdtempSync(join(tmpdir(), "mcp-plain-diff-"));
592+
const plain = mkTmpDir("mcp-plain-diff-");
592593

593594
const run = captureTool(registerGitDiffSummaryTool);
594595
const text = await run({ workspaceRoot: plain, format: "json" });

‎src/server/git-log-tool.test.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,16 @@
1717
* 9. commit subject containing % is preserved intact
1818
*/
1919

20-
import { describe, expect, test } from "bun:test";
20+
import { afterEach, describe, expect, test } from "bun:test";
2121
import { type ExecSyncOptionsWithStringEncoding, execFileSync } from "node:child_process";
22-
import { mkdtempSync, writeFileSync } from "node:fs";
23-
import { tmpdir } from "node:os";
22+
import { writeFileSync } from "node:fs";
2423
import { join } from "node:path";
2524

2625
import { gitTopLevel, spawnGitAsync } from "./git.js";
2726
import { registerGitLogTool } from "./git-log-tool.js";
28-
import { captureTool } from "./test-harness.js";
27+
import { captureTool, cleanupTmpPaths, mkTmpDir } from "./test-harness.js";
28+
29+
afterEach(cleanupTmpPaths);
2930

3031
// ---------------------------------------------------------------------------
3132
// Separators — must match git-log-tool.ts constants
@@ -62,7 +63,7 @@ function gitCmd(cwd: string, ...args: string[]): string {
6263
}
6364

6465
function makeRepo(): string {
65-
const dir = mkdtempSync(join(tmpdir(), "mcp-git-log-test-"));
66+
const dir = mkTmpDir("mcp-git-log-test-");
6667
gitCmd(dir, "init", "-b", "main");
6768
gitCmd(dir, "config", "user.email", "test@example.com");
6869
gitCmd(dir, "config", "user.name", "Test User");
@@ -266,7 +267,7 @@ describe("git_log integration", () => {
266267
});
267268

268269
test("not_a_git_repo: gitTopLevel returns null for a plain directory", () => {
269-
const dir = mkdtempSync(join(tmpdir(), "mcp-not-git-"));
270+
const dir = mkTmpDir("mcp-not-git-");
270271
const top = gitTopLevel(dir);
271272
expect(top).toBeNull();
272273
});
@@ -367,7 +368,7 @@ describe("git_log execute handler", () => {
367368
});
368369

369370
test("non-git workspaceRoot → not_a_git_repo error in group", async () => {
370-
const plain = mkdtempSync(join(tmpdir(), "mcp-plain-log-"));
371+
const plain = mkTmpDir("mcp-plain-log-");
371372

372373
const run = captureTool(registerGitLogTool);
373374
const text = await run({ workspaceRoot: plain, format: "json" });

‎src/server/git-merge-tool.test.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,16 @@
55
* No network, no real upstream — every branch lives locally.
66
*/
77

8-
import { describe, expect, test } from "bun:test";
8+
import { afterEach, describe, expect, test } from "bun:test";
99
import { type ExecSyncOptionsWithStringEncoding, execFileSync } from "node:child_process";
10-
import { existsSync, mkdtempSync, readdirSync, writeFileSync } from "node:fs";
10+
import { existsSync, readdirSync, writeFileSync } from "node:fs";
1111
import { tmpdir } from "node:os";
1212
import { join } from "node:path";
1313

1414
import { registerGitMergeTool } from "./git-merge-tool.js";
15-
import { captureTool } from "./test-harness.js";
15+
import { captureTool, cleanupTmpPaths, mkTmpDir, trackTmpPath } from "./test-harness.js";
16+
17+
afterEach(cleanupTmpPaths);
1618

1719
// ---------------------------------------------------------------------------
1820
// Repo helpers
@@ -36,7 +38,7 @@ function gitCmd(cwd: string, ...args: string[]): string {
3638
}
3739

3840
function makeRepo(): string {
39-
const dir = mkdtempSync(join(tmpdir(), "mcp-git-merge-test-"));
41+
const dir = mkTmpDir("mcp-git-merge-test-");
4042
gitCmd(dir, "init", "-b", "main");
4143
gitCmd(dir, "config", "user.email", "test@example.com");
4244
gitCmd(dir, "config", "user.name", "Test User");
@@ -318,7 +320,7 @@ describe("git_merge cleanup", () => {
318320
const dir = makeRepo();
319321
// Create branch + worktree for it.
320322
gitCmd(dir, "branch", "feature/w", "HEAD");
321-
const wtPath = join(tmpdir(), `mcp-wt-${Date.now()}`);
323+
const wtPath = trackTmpPath(join(tmpdir(), `mcp-wt-${Date.now()}`));
322324
gitCmd(dir, "worktree", "add", wtPath, "feature/w");
323325
// Add a commit in the worktree so it's ahead.
324326
writeFileSync(join(wtPath, "w.txt"), "W\n");
@@ -395,7 +397,7 @@ describe("git_merge guardrails", () => {
395397
});
396398

397399
test("non-git workspaceRoot returns not_a_git_repository", async () => {
398-
const plain = mkdtempSync(join(tmpdir(), "mcp-plain-"));
400+
const plain = mkTmpDir("mcp-plain-");
399401
const run = captureTool(registerGitMergeTool);
400402
const text = await run({
401403
workspaceRoot: plain,

‎src/server/test-harness.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
* // result is string (markdown) or JSON-parseable string
1818
*/
1919

20+
import { mkdtempSync, rmSync } from "node:fs";
21+
import { tmpdir } from "node:os";
22+
import { join } from "node:path";
2023
import type { FastMCP } from "fastmcp";
2124

2225
// ---------------------------------------------------------------------------
@@ -44,6 +47,34 @@ const STUB_CONTEXT: AnyRecord = {
4447
session: undefined,
4548
};
4649

50+
// ---------------------------------------------------------------------------
51+
// Tmp-dir lifecycle — prevents accumulating thousands of leaked test dirs.
52+
// Each test file must register the hook itself:
53+
// afterEach(cleanupTmpPaths);
54+
// Module-scope afterEach(...) would only register once (first-importer wins)
55+
// because the module is cached across test files in the same bun test run.
56+
// ---------------------------------------------------------------------------
57+
58+
const tmpPaths: string[] = [];
59+
60+
export function mkTmpDir(prefix: string): string {
61+
const dir = mkdtempSync(join(tmpdir(), prefix));
62+
tmpPaths.push(dir);
63+
return dir;
64+
}
65+
66+
export function trackTmpPath(path: string): string {
67+
tmpPaths.push(path);
68+
return path;
69+
}
70+
71+
export function cleanupTmpPaths(): void {
72+
while (tmpPaths.length > 0) {
73+
const p = tmpPaths.pop();
74+
if (p) rmSync(p, { recursive: true, force: true });
75+
}
76+
}
77+
4778
// ---------------------------------------------------------------------------
4879
// Fake server
4980
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)