test(user): restore real modules from a pre-mock snapshot

[0xfandom]

The failure

Four tests have been failing intermittently on main and on unrelated PRs:

(fail) task report generation > prints markdown task reports through the CLI handler
(fail) task report generation > writes markdown task reports through the CLI handler
(fail) /ads command > "off" disables earning and clears the stored code
(fail) /ads command > submitting the masked dialog enables earning and persists the code

with TypeError: undefined is not an object (evaluating 'inside.stderr.trim') at taskReport.ts:382. Seen on main at 6bef0e1 and 0ff1d1c, and simultaneously on five of my open PRs that touch none of that code.

Cause

src/utils/user.test.ts teardown reinstalls its own mocks instead of undoing them:

import * as realExeca from 'execa'
...
afterEach(() => {
  mock.restore()
  mock.module('execa', () => realExeca)   // <- realExeca is the mock by now

import * as is a live namespace binding. mock.module repoints it, so by the time afterEach runs, realExeca is the stub — and passing it back to mock.module reinstalls it. mock.module is process-global and is not undone by mock.restore(), so from that point every test file loaded afterwards gets the stub.

Reduced:

import * as realExeca from 'execa'
const before = realExeca.execa
mock.module('execa', () => ({ ...realExeca, execa: async () => ({ exitCode: 0, stdout: 'MOCKED' }) }))
realExeca.execa !== before          // true — the namespace was repointed
mock.module('execa', () => realExeca)          // "restore"
(await import('execa')).execa(...)  // -> { stdout: 'MOCKED' }, still the stub

The stub returns { exitCode, stdout } with no stderr, which is what makes it visible downstream: collectTaskReportGitMetadata reads inside.stderr.trim() and throws. Whether it bites depends on whether bun happens to order user.test.ts before the affected suites — hence the intermittency, and why adding or removing any test file anywhere moves it.

Fix

Snapshot each real module surface into a plain object at load, before any mock is installed, and restore through the snapshots. The stub definitions read from the snapshot too — a bare import('execa') inside the helper resolves to whatever mock is currently installed, so each stub was being built on top of the previous one.

This is the same shape as the fixes in #1667 and #1708.

Verification

Bisected the failure to this file: 162 test files in src/utils, halved down to user.test.ts alone reproducing it against reportTask.test.ts, and the four other candidates in the final group clean.

• bun test src/utils/user.test.ts src/utils/reportTask.test.ts — fails before, passes after.
• bun test src/utils/user.test.ts src/commands/ads* — passes after.
• Full suite (bun test --feature=UNATTENDED_RETRY --max-concurrency=1, 7569 tests / 637 files): 26 failures fixed, 0 newly failing.

The remaining failures are pre-existing and unrelated (the baseline itself is unstable — two runs on clean main gave 48 and 69). The two task-report tests are deterministically fixed, verified by the targeted repro above rather than by the full-run diff.

Note for a follow-up

22 other suites restore through a live namespace the same way — grep -rlE "mock\.module\([^,]+,\s*\(\)\s*=>\s*(real|original)[A-Za-z]*\s*\)". Most are probably harmless today because their stubs happen to be shape-complete, but the pattern is a latent version of this bug. I have not touched them here: I only fixed what I could reproduce, and a sweep of that size deserves its own PR.

Summary by CodeRabbit

• Tests
  • Improved test isolation by capturing and restoring original module namespaces before/after each test, preventing mocks from unintentionally chaining across test cases.
  • Strengthened abort/interrupt coverage with deterministic query “Stop hook regression” scenarios, asserting when the synthetic user cancellation message is present vs. absent depending on the abort reason.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 200444c4-cd5c-4296-b954-0f6cb763c515

📥 Commits

Reviewing files that changed from the base of the PR and between ff971e0 and 14aef0b.

📒 Files selected for processing (1)

• tests/sdk/query-lifecycle.test.ts

💤 Files with no reviewable changes (1)

• tests/sdk/query-lifecycle.test.ts

📜 Recent review details

⏰ Context from checks skipped due to timeout. (3)

• GitHub Check: smoke-and-tests (22)
• GitHub Check: typecheck
• GitHub Check: smoke-and-tests (24.11.x)

---

📝 Walkthrough

Walkthrough

The PR hardens module mocking in src/utils/user.test.ts and replaces the prior interruption test with deterministic queryLoop coverage for explicit and absent abort reasons.

Changes

Test reliability updates

Layer / File(s)	Summary
Stabilize user test module mocks src/utils/user.test.ts	Captures real dependency snapshots before mocking and reuses them for dependency loading, environment and execa mocks, and afterEach restoration.
Test abort-reason interruption flow tests/sdk/query-lifecycle.test.ts	Adds stop-hook regression tests verifying that "[Request interrupted by user]" is absent for reason "interrupt" and present when no abort reason is supplied.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• Gitlawb/openclaude#1030: Updates the same user-test mocking targets to prevent mock state leakage.
• Gitlawb/openclaude#1733: Covers abort-reason handling and related query interruption behavior.
• Gitlawb/openclaude#1850: Aligns stop-hook tests with abort-reason-dependent interruption messaging.

Suggested labels: bug

Suggested reviewers: jatmn, kevincodex1

🚥 Pre-merge checks | ✅ 7

✅ Passed checks (7 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately reflects the main fix in user.test.ts.
Description check	✅ Passed	The description is detailed and covers the bug, cause, fix, verification, and follow-up, though it doesn't use the exact template headings.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Risk Surface Disclosed	✅ Passed	PASS: The PR is test-only and explicitly calls out the process-global mock leak risk, scopes the fix, and reports no new blocker.
No Hidden Policy Change	✅ Passed	Only test harness snapshotting and abort-message regression tests changed; no product, trust, routing, telemetry/network, or permission-policy code paths were altered.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[jatmn]

I found an issue that needs to be addressed before this is ready.

Findings

• [P2] Remove the unrelated VCR fixture
  fixtures/734ad7.json:1
  This PR does not change the SDK lifecycle test that owns this capture. The fixture makes the existing test undefined reason path replay an empty response instead of executing its normal request path, so it can silently hide regressions; moreover, the test disables built-in agents while the committed input contains an environment-dependent agent-listing reminder. Keep this mock-teardown repair focused and remove the generated fixture (or make an intentional, tested lifecycle-fixture change in its own PR).

[0xfandom]

Good catch — that fixture was recorded by accident while I ran the SDK suite during verification; it isn't part of this change. Removed it in 59b25c8 so the PR stays scoped to the mock-teardown repair. Any intentional lifecycle-fixture change can go in its own PR.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/utils/user.test.ts`:
- Around line 82-83: Update the getHostPlatformForAnalytics mock in the affected
user test to return the valid Platform value win32 instead of windows, while
leaving the env.platform fixture unchanged unless required by the test contract.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 23ed3fd8-31fe-4d93-a007-4d381b3d43f1

📥 Commits

Reviewing files that changed from the base of the PR and between 01a01fb and 59b25c8.

📒 Files selected for processing (1)

• src/utils/user.test.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (2)

• GitHub Check: smoke-and-tests (22)
• GitHub Check: smoke-and-tests (24.11.x)

🧰 Additional context used

📓 Path-based instructions (4)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

TypeScript code in this repository must use strict mode and ESM imports.

**/*.{ts,tsx}: Follow the existing code style and architectural patterns in touched TypeScript and TSX files.
Add or update tests when TypeScript or TSX changes affect behavior.
Review AI-generated TypeScript and TSX changes for correctness beyond compilation, consistency with repository architecture and style, unnecessary generated noise, and subtle bugs before submission.

Files:

• src/utils/user.test.ts

**/*

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*: Keep pull requests focused on one problem or feature; do not mix unrelated cleanup, fixes, features, or refactors into the same change.
Preserve existing repository patterns unless intentionally refactoring them, and prefer small, readable changes over broad rewrites.
Do not reformat unrelated files, and keep comments useful and concise.
Update documentation when setup, commands, or user-facing behavior changes.
When changing provider behavior, avoid breaking third-party providers, test the exact provider/model path changed when possible, explicitly identify affected providers, and document limitations or follow-up work.
Do not assign or use provider tags; provider tags are controlled and applied by maintainers.
Run the relevant validation checks locally before submitting; CI-required checks include bun run check, bun run test:full, provider tests when applicable, typechecks, and bun run security:pr-scan. Web changes additionally require bun run web:typecheck and bun run web:build.
Dependency changes must have a concrete project benefit, such as fixing a bug, addressing a security issue, or supporting an approved feature; preference alone is insufficient.
Do not change the project's language, core runtime, dependency stack, or significantly restructure dependencies without prior maintainer agreement.
Before implementing a new feature or other non-trivial change, open an issue to establish scope and alignment with the project roadmap.

Files:

• src/utils/user.test.ts

---

⚙️ CodeRabbit configuration file

**/*: Apply the OpenClaude maintainer review rubric from AGENTS.md. Review the current diff, not stale discussion context. Separate real blockers from suggestions. Do not request changes for vague style churn. Treat approval as merge-ready from CodeRabbit's side, pending required human review and GitHub Checks. If checks are failing or unavailable, say so clearly instead of implying the PR is fully ready.

Files:

• src/utils/user.test.ts

**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Run focused tests for changed behavior and ensure provider-specific changes include the relevant provider tests.

Files:

• src/utils/user.test.ts

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}

⚙️ CodeRabbit configuration file

{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}: Review tests for meaningful coverage of the changed behavior, isolation of global/env/config state, async cleanup, fake timers, provider profile leaks, and Windows-compatible assumptions. Block when risky runtime changes lack focused regression coverage or tests assert implementation details while missing the user-visible behavior.

Files:

• src/utils/user.test.ts

🔇 Additional comments (1)

src/utils/user.test.ts (1)

12-23: LGTM!

Also applies to: 33-44, 86-87, 114-118

[jatmn]

@kevincodex1 LGTM