Skip to content

test: clean up isolated config homes after integration runs#509

Merged
benvinegar merged 1 commit into
mainfrom
chore/config-home-cleanup
Jul 3, 2026
Merged

test: clean up isolated config homes after integration runs#509
benvinegar merged 1 commit into
mainfrom
chore/config-home-cleanup

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Addresses the Greptile P2 on #506: createTestConfigHome() is called at module scope in four test files and the temp dirs were never removed, leaking one directory per file per run.

The suggested per-consumer afterAll is exactly what this does — a central sweep was attempted first but Bun's test runner never emits process.on("exit") (verified with a probe), so the helper now exports cleanupTestConfigHomes() and each consumer registers it with afterAll.

Verified: /tmp is clean after bun test test/session/ and bun run test:tty-smoke (previously each run left 4 dirs). One caveat: a spawned session daemon can in rare cases recreate a dir just after the sweep — a transient race that leaves at most one near-empty dir, versus the previous guaranteed leak.

Empty changeset: test-only maintenance.

🤖 Generated with Claude Code

Addresses Greptile's review note on #506: the per-file XDG_CONFIG_HOME
temp dirs were created at module scope and never removed. Bun's test
runner never emits process 'exit', so the helper exports an explicit
cleanupTestConfigHomes() that each consumer registers via afterAll.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a temp-directory leak introduced in #506 by exporting a cleanupTestConfigHomes() helper and registering it via afterAll in each of the four test files that call createTestConfigHome() at module scope. The approach is straightforward given Bun's per-file worker isolation, which prevents a single process.on("exit") sweep from working.

  • test/helpers/config-home.ts: adds a module-level createdDirs array and a cleanupTestConfigHomes export that pops and recursively removes each tracked directory using rmSync({recursive:true, force:true}).
  • Four test files (broker-e2e, cli, daemon, tty): each imports cleanupTestConfigHomes and registers it with afterAll at module scope.

Confidence Score: 5/5

Test-only cleanup with no production code changes; the new cleanup function is idempotent and safe to merge.

All changes are confined to test helpers and test files. The cleanupTestConfigHomes logic is straightforward — a pop-loop backed by rmSync({force:true}) — and each test file correctly pairs its createTestConfigHome() call with afterAll(cleanupTestConfigHomes). Bun's per-file worker isolation ensures the module-level createdDirs singleton is not shared across files, so there is no cross-file cleanup race. The only rough edge (a spawned daemon recreating a dir after the sweep) is already documented in the PR description and leaves at worst one near-empty directory, which is far better than the previous guaranteed leak.

No files require special attention.

Important Files Changed

Filename Overview
test/helpers/config-home.ts Adds module-level createdDirs tracking and a cleanupTestConfigHomes export; logic is correct and the pop-loop with rmSync({force:true}) is safe.
test/session/broker-e2e.test.ts Adds afterAll(cleanupTestConfigHomes) at module scope; minimal change, correct placement.
test/session/cli.test.ts Adds afterAll(cleanupTestConfigHomes) at module scope; minimal change, correct placement.
test/session/daemon.test.ts Adds afterAll(cleanupTestConfigHomes) at module scope; minimal change, correct placement.
test/smoke/tty.test.ts Adds afterAll(cleanupTestConfigHomes) at module scope; minimal change, correct placement.
.changeset/config-home-sweep.md Empty changeset (no package entries, no description); intentional for a test-only change but unusually bare.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant F as Test File (module scope)
    participant H as config-home.ts
    participant OS as /tmp

    F->>H: createTestConfigHome()
    H->>OS: mkdtempSync(join(tmpdir(), prefix))
    OS-->>H: dir path
    H->>H: createdDirs.push(dir)
    H-->>F: dir path

    F->>F: afterAll(cleanupTestConfigHomes) registered

    Note over F: ... tests run ...

    F->>H: cleanupTestConfigHomes() [via afterAll]
    loop "while createdDirs.length > 0"
        H->>H: "dir = createdDirs.pop()"
        H->>OS: "rmSync(dir, { recursive: true, force: true })"
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant F as Test File (module scope)
    participant H as config-home.ts
    participant OS as /tmp

    F->>H: createTestConfigHome()
    H->>OS: mkdtempSync(join(tmpdir(), prefix))
    OS-->>H: dir path
    H->>H: createdDirs.push(dir)
    H-->>F: dir path

    F->>F: afterAll(cleanupTestConfigHomes) registered

    Note over F: ... tests run ...

    F->>H: cleanupTestConfigHomes() [via afterAll]
    loop "while createdDirs.length > 0"
        H->>H: "dir = createdDirs.pop()"
        H->>OS: "rmSync(dir, { recursive: true, force: true })"
    end
Loading

Reviews (1): Last reviewed commit: "test: clean up isolated config homes aft..." | Re-trigger Greptile

@benvinegar benvinegar merged commit f2f555e into main Jul 3, 2026
9 checks passed
@benvinegar benvinegar deleted the chore/config-home-cleanup branch July 3, 2026 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant