Skip to content

fix(tests): ensure temp test files are cleaned up on failure - #3385

Draft
nitsuah wants to merge 1 commit into
decolua:masterfrom
nitsuah:fix-test-cleanup
Draft

fix(tests): ensure temp test files are cleaned up on failure#3385
nitsuah wants to merge 1 commit into
decolua:masterfrom
nitsuah:fix-test-cleanup

Conversation

@nitsuah

@nitsuah nitsuah commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Problem: Stale Artifact Pollution

The output shows a failed atomic operation (Write(c.txt) failure) followed by the creation of an orphaned, unmanaged worktree (add-c-txt).

Why it happens:
The system attempts a task, fails mid-process, and lacks a "rollback" mechanism to clean up the partially created state. Because the cleanup logic is external (run only upon success or explicitly called), the error aborts the flow, leaving the stale file (c.txt) and the orphaned directory (worktree) permanently polluting the environment.

● Write(c.txt)
Error writing file

● Creating worktree(add-c-txt)
Switched to worktree on branch worktree-add-c-txt
C:\Users\<USERNAME>\code\<REPO>\.claude\worktrees\add-c-txt

● Write(.claude\worktrees\add-c-txt\c.txt)
Wrote 1 line to .claude\worktrees\add-c-txt\c.txt
b

tl;dr - Non-Atomic Sequencing. The system doesn't treat the sequence as one transactional unit. A failure in step 1 doesn't abort the entire process; step 2 proceeds anyway. Result: orphaned artifacts from subsequent, should-not-have-run steps, even though the initial step was flagged as an error. Partial success is incorrectly reported as a failure, or the process is continuing in a corrupted, partially-failed state.

Root Cause

Test suites rely on afterEach hooks to clean up temporary directories. This pattern fails under three conditions:

  1. Runner Lifecycle: If a test runner process crashes or encounters an unhandled async exception before reaching the afterEach hook, artifacts are abandoned.
  2. Permission/Lock Contention: Temporary files created by utility scripts (e.g., build-cli.js) or left by orphaned test processes occasionally acquire read-only status or OS file locks. Standard fs.rmSync({ force: true }) fails to delete these, leading to "permission denied" or "file locked" errors, which subsequently pollute the repository root with residual state.
  3. Partial Deletion State: In complex suites, afterEach may run on a directory tree that is already in a partially-deleted state, causing recursive deletion failures.

Solution

  1. Robust Cleanup Utility: Introduce a cleanup(path) utility that forces deletion:
    • Handles ENOENT silently.
    • For permission errors (EPERM/EACCES), recursively updates file permissions (chmod 777) to ensure deleteability.
    • Implements a retry-loop with short delays to bypass transient OS file locks common on Windows/CI environments.
  2. Defensive Test Structure: Shift from reliance on global hooks to try...finally blocks within individual test specs. This guarantees that if a spec fails, cleanup logic executes immediately, independent of the test runner's lifecycle health.
  3. Strict Scoping: Enforce that all test-related I/O is routed through a centralized fixture manager that guarantees all writes occur within an ephemeral tmp root, completely forbidding writes to the repository directory.

@nitsuah
nitsuah marked this pull request as draft August 16, 2026 21:23
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