test: stabilize aggregate and package suites (#255, #245)

[mohanagy]

Summary

• replace the long-lived serial Vitest fork between files while keeping file execution serial and module isolation enabled
• enforce the runner contract in tests/release-config.test.ts
• pre-arm the secret-plugin started-marker watcher so child startup cannot outrun the cancellation test observation
• verify V8 coverage stripping through the injected npm child boundary instead of a flaky real npm exec process tree

Root causes

The aggregate-suite failures migrated between process-backed files because singleFork: true retained one worker process across the entire suite. That allowed handles and lifecycle state from an earlier file to accumulate. The bundled fake upstream introduced after the original cold-start mitigation makes a fresh isolated fork per serial file viable again.

The Windows package-contract failure was a test-harness mismatch: a synchronous child-environment assertion depended on real npm startup and close delivery. The contract now captures the exact environment passed by runNpm through its existing injectable spawner, eliminating the unrelated npm lifecycle without weakening the assertion.

No product source, public API, timeout, coverage threshold, platform check, or containment behavior changes.

Security impact

Test-process isolation is strengthened: each serial file receives a fresh fork, preventing handles or lifecycle state from crossing file boundaries. The package contract now asserts the child coverage environment directly. Runtime redaction, audit guarantees, secret handling, Windows containment, subprocess policies, and public APIs are unchanged.

TDD evidence

• red: tightened the runner contract to require fork replacement and observed it fail against singleFork: true
• red: reproduced aggregate failures in setup-profile-readiness, audit-outcomes, and upstream-manager at different positions under the retained worker
• red: Windows Node 22 reproduced ci: diagnose Windows package-contract coverage subprocess timeout #245 at the real npm exec boundary with the unchanged 5-second test budget
• green: focused runner/readiness/wrapper contracts: 134 passed
• green: repeated readiness cancellation test: 5/5 clean
• green: deterministic V8 coverage child-environment assertion: 3 ms
• green: package contract: 26 passed
• green: full suite after the change: 1,769 passed, 33 skipped
• green: coverage suite: 1,769 passed, 33 skipped; 95.65% statements, 91.88% branches, 98.66% functions, 95.65% lines

Validation

• npm test
• npm run test:core
• npm run test:coverage
• npm run lint
• npm run typecheck
• npm run build
• npm run smoke:cli
• npm run check:pack
• npm run test:package

Closes #255
Closes #245

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b1dd0737-1260-4328-bacb-fb0645cf8952

📥 Commits

Reviewing files that changed from the base of the PR and between 54d3600 and 1994489.

📒 Files selected for processing (1)

• tests/package-contract.test.ts

📝 Walkthrough

Walkthrough

A shared filesystem path-arrival watcher replaces duplicated test logic, readiness synchronization now uses it with cleanup, and Vitest fork configuration changes from single-worker reuse to isolated process-backed execution. The related configuration contract test is updated.

Changes

Test reliability updates

Layer / File(s)	Summary
Shared path arrival synchronization tests/helpers/path-arrival.ts, tests/mcp-wrapper.test.ts, tests/setup-profile-readiness.test.ts	Adds a reusable fs.watch-based path waiter with fallback polling and cleanup, then uses it in MCP and plugin readiness tests.
Fork configuration contract vitest.config.ts, tests/release-config.test.ts	Sets poolOptions.forks.singleFork to false while retaining isolation, and updates the matching contract assertion.

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

Sequence Diagram(s)

sequenceDiagram
  participant ReadinessTest
  participant watchForPathArrival
  participant FileSystem
  ReadinessTest->>watchForPathArrival: Watch startedPath
  watchForPathArrival->>FileSystem: Watch parent directory
  watchForPathArrival->>FileSystem: Check path access
  FileSystem-->>watchForPathArrival: Path arrival or error
  watchForPathArrival-->>ReadinessTest: Resolve or reject wait
  ReadinessTest->>watchForPathArrival: Close watcher

Loading

Possibly related PRs

• mohanagy/miftah#187: Both update Vitest fork configuration and its contract test.
• mohanagy/miftah#227: Both use filesystem marker watching in tests/mcp-wrapper.test.ts.

Poem

A rabbit watched the marker glow,
Through watcher ears that softly know.
Forks now hop to processes new,
And cleanup closes when they’re through.
Tests spring fresh—boing, boing, hooray!

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Docstring Coverage	❌ Error	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title matches the main change: stabilizing test suites by adjusting Vitest isolation and related contracts.
Description check	✅ Passed	The description covers Summary, Security impact, TDD evidence, and Validation with concrete commands and results, so it is mostly complete.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/issue-255-aggregate-contention-root-cause

---

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

[mohanagy]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mohanagy]

CodeRabbit status is passing, but the fresh review request reported "Review rate limited". Per the review policy, an independent exact-head review was completed for 1994489ae0745c3bf430a0d9c55648b3b06ba91c.

No blocking findings. The review confirmed fresh per-file fork isolation without timeout/coverage weakening; the injected package test exercises the production NpmSpawner environment boundary; subprocess calls retain argument arrays and shell: false; watcher resources close safely; and no production security, containment, redaction, platform, public API, or compatibility code changed. Focused review tests and git diff --check passed.

Current-head CI run 30374998838 is green on Linux/macOS/Windows with Node 20/22/24, Linux quality, package checks, and Verify.