RT-299: sweep read-only Go module caches out of leftover test dirs

[m4ttheweric]

RT-299. The glitter pty gate builds rt-ui under the run's throwaway HOME, so Go left a read-only module cache there. The next bun test run's preload sweep threw EACCES on it and no test in that run started.

What changed

• e2e/pty/glitter.test.ts builds with GOFLAGS=-modcacherw, so the cache stays deletable
• test-setup.ts's sweep makes a stale dir's subdirs writable and retries when the plain rm fails
• the afterAll detached rm -rf runs chmod -R u+w first
• lib/__tests__/test-home-isolation.test.ts plants a dead run's dir with a read-only module cache and asserts the next run sweeps it (failed with EACCES before the fix)

Verification

bun run test:pty then bun run test back to back with no cleanup: the pty gate passed 7/7, zero read-only dirs were left under rt-tests, and the unit suite started and ran all 9891 tests with no EACCES/ENOTEMPTY. Its four failures (worktree provision timeouts, OAuth listen) come from machine load and pass 99/99 when run alone.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added coverage for cleanup of stale temporary test directories containing read-only Go cache files.
  • Improved automated test cleanup so it can remove read-only files and directories, including after an interrupted run. These changes make test-environment cleanup more reliable; no end-user features or behavior changed.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 29d98dbe-42ff-4dfd-ba6a-609296d4e8d0

📥 Commits

Reviewing files that changed from the base of the PR and between aa083f5 and 31e02a4.

📒 Files selected for processing (3)

• e2e/pty/glitter.test.ts
• lib/__tests__/test-home-isolation.test.ts
• test-setup.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

---

📝 Walkthrough

Walkthrough

The UI build sets GOFLAGS with -modcacherw. Test cleanup makes directories writable before removal. A new integration test checks removal of a stale home directory containing a read-only Go module cache.

Changes

Temporary home cleanup

Layer / File(s)	Summary
Writable cache and stale-home cleanup e2e/pty/glitter.test.ts, test-setup.ts, lib/__tests__/test-home-isolation.test.ts	The UI build appends -modcacherw to GOFLAGS. Cleanup makes directories writable before removal. The integration test creates a stale home with a read-only Go module cache and checks that the probe removes it.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 31e02

The regression test creates its stale directory where the next run will sweep it. No actionable merge-blocking risk remains after normal checks.

Architecture Summary

Architecture risk: 🟡 Medium · up to 31e02

The change affects 3 systems.

Changed systems: test-setup.ts, e2e, lib

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

• observed — test-setup.ts (service) was modified; 1 changed file maps to changed impact.
• observed — e2e (service) was modified; 1 changed file maps to changed impact.
• observed — lib (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

• observed — Modified behavior in e2e/pty/glitter.test.ts: The ui:build invocation now sets GOFLAGS to the existing value with -modcacherw appended, filtering out an empty existing value; it retains the same command, working directory, and piped output.
• observed — Modified behavior in lib/tests/test-home-isolation.test.ts: The imports add filesystem operations for creating, checking, changing permissions on, and removing the stale directory, plus dirname for deriving its parent path.
• observed — Modified behavior in lib/tests/test-home-isolation.test.ts: A new integration test creates a completed child process’s stale home directory and a read-only Go module-cache path beneath it, then runs the probe with TMPDIR set to the test-root parent. It fails with captured output if the probe exits nonzero, expects the stale directory to be absent afterward, and restores permissions and removes the directory in finally if it remains.
• observed — Modified behavior in test-setup.ts: Adds the chmodSync filesystem import for the writable-cleanup helpers.

Reliability and maintainability

• inferred — Risk-relevant change factors for test-setup.ts: blast_radius_1; direct_dependents_1

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: removing read-only Go module caches from leftover test directories. It is specific and concise.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}