Skip to content

test: stabilize resource subscription handshake observation - #276

Merged
mohanagy merged 1 commit into
developmentfrom
fix/124-resource-subscription-lifecycle
Jul 27, 2026
Merged

test: stabilize resource subscription handshake observation#276
mohanagy merged 1 commit into
developmentfrom
fix/124-resource-subscription-lifecycle

Conversation

@mohanagy

@mohanagy mohanagy commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Summary

Relates to #124.

The fresh aggregate-only recurrence was an observation failure: this test used Vitest's default one-second expect.poll deadline to wait for the child fixture's synchronous subscribe-entry marker. When the child reached that marker after the deadline, cleanup closed a still-pending client request and produced the secondary connection-close rejection.

This PR:

  • adds a test-only pre-marker subscribe delay to reproduce that boundary deterministically;
  • arms the existing filesystem watcher before the subscription request;
  • races the marker with early request settlement for a phase-specific failure;
  • keeps the existing test timeout, product runtime, containment, policy, and coverage thresholds unchanged.

The original restart-specific timeout in #124 has not been independently reproduced by this correction, so this PR intentionally does not close the issue.

Validation

  • Negative control: old polling assertion failed with Matcher did not succeed in time and the expected cleanup-side connection-close rejection.
  • npx vitest run tests/mcp-wrapper.test.ts -t "does not retain a subscription invalidated during its upstream handshake"
  • npx vitest run tests/mcp-wrapper.test.ts — 97 passed
  • npx vitest run --coverage tests/mcp-wrapper.test.ts — 97 tests passed; command then reports unchanged repository-wide coverage thresholds because it is a focused run
  • npx vitest run tests/package-contract.test.ts — 24 passed
  • npm run lint
  • npm run typecheck
  • regenerated tests/fixtures/fake-upstream-bundled.mjs twice with identical SHA-256

Summary by CodeRabbit

  • Tests

    • Improved coverage for subscription lifecycle behavior during upstream connection handshakes.
    • Added deterministic timing scenarios to better detect race conditions when subscriptions are invalidated before startup completes.
    • Strengthened test failure reporting and cleanup for more reliable results.
  • Maintenance

    • Refreshed bundled test fixtures without changing user-facing functionality.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: de02019a-0415-48ac-8e50-96ffcb247f92

📥 Commits

Reviewing files that changed from the base of the PR and between 2fe8131 and 0bed03e.

📒 Files selected for processing (3)
  • tests/fixtures/fake-upstream-bundled.mjs
  • tests/fixtures/fake-upstream-runtime.mjs
  • tests/mcp-wrapper.test.ts

📝 Walkthrough

Walkthrough

The upstream runtime now supports configurable subscription-start delays. The handshake test uses that delay and deterministic marker observation to verify invalidated subscriptions are not retained. The bundled upstream fixture was regenerated with minified output changes.

Changes

Subscription handshake synchronization

Layer / File(s) Summary
Upstream fixture timing and regeneration
tests/fixtures/fake-upstream-runtime.mjs, tests/fixtures/fake-upstream-bundled.mjs
The runtime delays subscription startup using TEST_SUBSCRIBE_START_DELAY_MS, and the bundled fixture contains regenerated minified output.
Deterministic handshake assertions
tests/mcp-wrapper.test.ts
The test waits for either the startup marker or subscription completion, rejects premature completion, preserves lifecycle assertions, and closes the watcher.

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

Possibly related PRs

  • mohanagy/miftah#70: Adds the subscription lifecycle behavior exercised by these deterministic fixture and wrapper-test changes.
  • mohanagy/miftah#227: Uses the same watcher-based synchronization pattern for the subscription-started marker.

Poem

A bunny watched the handshake start,
With timed-up ears and careful heart.
The marker came; the race was won,
The watcher closed when work was done.
No stale subscription stayed to hop!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the main change: stabilizing subscription handshake observation in tests.
Description check ✅ Passed The description is detailed and covers summary plus validation; it only omits the Security impact section from the template.
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.
✨ 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/124-resource-subscription-lifecycle

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

@mohanagy
mohanagy merged commit 9d69b07 into development Jul 27, 2026
21 of 23 checks passed
@mohanagy
mohanagy deleted the fix/124-resource-subscription-lifecycle branch July 27, 2026 21:33
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