test(mcp): restore os.homedir spy after each test to prevent cross-file leak by sahrizvi · Pull Request #1043 · AltimateAI/altimate-code

sahrizvi · 2026-07-23T22:53:32Z

Issue for this PR

Type of change

Bug fix
New feature
Refactor / code improvement
Documentation

What does this PR do?

The beforeEach in packages/opencode/test/mcp/lifecycle.test.ts:250 installs spyOn(os, "homedir").mockImplementation(...) and returns a fresh mkdtempSync(... "mcp-lifecycle-home-...") on every call. There is no matching afterEach that restores the spy. When bun runs the full test suite, the spy persists past this file into any downstream file in the same worker process. The next test that calls os.homedir() gets a different random path per invocation — inside a single expect().toEqual(...) call, the value returned to the "actual" side and the value used to build the "expected" side can differ, and the assertion fails on paths that visibly contain the mcp-lifecycle-home- prefix from this file.

The most visible casualty is test/permission/next.test.ts, where 6 tilde / $HOME-expansion tests fail intermittently on CI:

error: expect(received).toEqual(expected)
[
  {
    "action": "allow",
-   "pattern": "/tmp/mcp-lifecycle-home-Zz7Y0f/projects/*",
+   "pattern": "/tmp/mcp-lifecycle-home-VvysFV/projects/*",
    "permission": "external_directory",
  }
]

The failure is file-order dependent — it passes in isolation and only surfaces when the full suite runs in the leaking order.

Fix: import afterEach from bun:test and add afterEach(() => mock.restore()) right after the beforeEach block in lifecycle.test.ts. Blanket mock.restore() tears down the os.homedir spy (and any other spy installed during a test) between tests, so the mock cannot escape the file boundary. This matches the pattern already used elsewhere in the suite and keeps the fix a single import addition plus a 3-line hook.

How did you verify your code works?

bun test test/mcp/lifecycle.test.ts — 31 pass, 0 fail (fix's own file still green)
bun test test/permission/next.test.ts — 80 pass, 0 fail (previously affected file green in isolation, same as before the fix)
bun test test/mcp/lifecycle.test.ts test/permission/next.test.ts — 111 pass, 0 fail (both files together in one bun invocation — the shared-worker scenario that reproduced the leak)
bun run typecheck — clean (exit 0)
bun run script/upstream/analyze.ts --markers --base origin/main --strict — clean
bun run script/upstream/analyze.ts --branding — 0 leaks

Screenshots / recordings

n/a — no UI change.

Checklist

I have tested my changes locally
I have not included unrelated changes in this PR

Summary by cubic

Fix flaky tests by restoring the os.homedir spy after each test in packages/opencode/test/mcp/lifecycle.test.ts. Adds afterEach(() => mock.restore()) from bun:test to stop cross-file leaks that broke tilde/$HOME expansion checks in test/permission/next.test.ts.

^{Written for commit fec5f66. Summary will update on new commits.}

Summary by CodeRabbit

Tests
- Improved test isolation by automatically restoring mocks after each test.
- Prevented environment-related assertions from being affected by previous tests.

…le leak The `beforeEach` in `test/mcp/lifecycle.test.ts` installs a `spyOn(os, "homedir")` that returns a fresh `mkdtempSync(... "mcp-lifecycle-home-...")` on every call. Without a matching restore, the spy persists past this file into subsequent files in the same bun worker process. Downstream tests that call `os.homedir()` then get a different random path on every invocation — most visibly the tilde and `$HOME` expansion assertions in `test/permission/next.test.ts`, which see two different `mkdtempSync` paths within the same `expect().toEqual()` call and fail 6 times. The failure was file-order dependent (passes in isolation, fails when the full suite runs in the wrong order), which manifested as a required `CI/TypeScript` check failing intermittently on PRs — including v0.9.3. Fix: add `afterEach(() => mock.restore())` in `lifecycle.test.ts` so the spy is torn down between tests and cannot leak past the file boundary. Blanket `mock.restore()` matches the pattern already used elsewhere in the suite and cleans up any other spies installed during a test. Closes #1042

claude

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

github-actions · 2026-07-23T22:53:44Z

👋 This PR was automatically closed by our quality checks.

Common reasons:

New GitHub account with limited contribution history
PR description doesn't meet our guidelines
Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

coderabbitai · 2026-07-23T22:53:50Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c11584a6-a1d0-4ec0-b653-fced8b4c264a

📥 Commits

Reviewing files that changed from the base of the PR and between a441d17 and fec5f66.

📒 Files selected for processing (1)

packages/opencode/test/mcp/lifecycle.test.ts

📝 Walkthrough

Walkthrough

The MCP lifecycle test imports afterEach and restores Bun mocks after every test, preventing spies such as the os.homedir mock from persisting between tests.

Changes

MCP test isolation

Layer / File(s)	Summary
Per-test mock restoration `packages/opencode/test/mcp/lifecycle.test.ts`	Adds an `afterEach` hook that calls `mock.restore()` after each test.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: anandgupta42

Poem

A rabbit twitched its whiskers bright,
“Mocks now vanish after night.”
Each test hops clean, the paths stay true,
No borrowed homes seep through the queue.
Thump, thump—green tests anew!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and accurately summarizes the main fix: restoring the os.homedir spy after each test to prevent leakage.
Description check	✅ Passed	The description follows the template and includes the issue, change type, implementation details, verification steps, screenshots note, and checklist.
Linked Issues check	✅ Passed	The change directly addresses `#1042` by restoring the leaked os.homedir mock after each test, matching the issue's required fix.
Out of Scope Changes check	✅ Passed	The PR is narrowly scoped to the test isolation fix and adds no unrelated code changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/mcp-homedir-spy-leak

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

github-actions · 2026-07-23T22:54:07Z

👋 This PR was automatically closed by our quality checks.

Common reasons:

New GitHub account with limited contribution history
PR description doesn't meet our guidelines
Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

github-actions · 2026-07-23T22:54:25Z

👋 This PR was automatically closed by our quality checks.

Common reasons:

New GitHub account with limited contribution history
PR description doesn't meet our guidelines
Contribution appears to be AI-generated without meaningful review

If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you.

kilo-code-bot · 2026-07-23T22:56:46Z

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The change adds afterEach(() => mock.restore()) to tear down the spyOn(os, "homedir") spy between tests, fixing the cross-file test-isolation leak described in #1042. The fix is minimal and correct: mock.restore() reliably reverts the in-place spy, and the mock.module() SDK mocks earlier in the file stay effective because the already-imported MCP module (line 271) retains live bindings to the mock classes. The added comment clearly explains the rationale.

Files Reviewed (1 file)

packages/opencode/test/mcp/lifecycle.test.ts

_{Reviewed by glm-5.2 · Input: 50.4K · Output: 6.8K · Cached: 323.3K}

_{Review guidance: REVIEW.md from base branch main}

cubic-dev-ai

No issues found across 1 file

_{Re-trigger cubic}

dev-punia-altimate · 2026-07-24T13:39:10Z

🤖 Code Review — OpenCodeReview (Gemini) — No Issues Found

No supported files changed.

claude Bot reviewed Jul 23, 2026

View reviewed changes

github-actions Bot added contributor needs-review:blocked labels Jul 23, 2026

cubic-dev-ai Bot reviewed Jul 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

test(mcp): restore os.homedir spy after each test to prevent cross-file leak#1043

test(mcp): restore os.homedir spy after each test to prevent cross-file leak#1043
sahrizvi wants to merge 1 commit into
mainfrom
fix/mcp-homedir-spy-leak

sahrizvi commented Jul 23, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

claude Bot left a comment

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

kilo-code-bot Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

cubic-dev-ai Bot left a comment

Uh oh!

dev-punia-altimate commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sahrizvi commented Jul 23, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue for this PR

Type of change

What does this PR do?

How did you verify your code works?

Screenshots / recordings

Checklist

Summary by cubic

Summary by CodeRabbit

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Claude Code Review

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

coderabbitai Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

kilo-code-bot Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Summary

Uh oh!

cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

dev-punia-altimate commented Jul 24, 2026

🤖 Code Review — OpenCodeReview (Gemini) — No Issues Found

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sahrizvi commented Jul 23, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

kilo-code-bot Bot commented Jul 23, 2026 •

edited

Loading