fix: preserve upstream containment after transport close errors

[mohanagy]

Fixes #112.

Root cause

MiftahServer.close previously stopped at the first downstream transport-close rejection, skipping upstream manager shutdown and potentially leaving a stdio provider process alive.

Change

Each shutdown phase now runs independently. Miftah preserves and rethrows the first failure after resource-subscription cleanup, MCP transport shutdown, and upstream process-manager containment have all been attempted.

Regression coverage

The focused test starts a real fake stdio upstream, injects a rejecting downstream transport close, verifies the original error still reaches the caller, and verifies manager health is stopped with no retained PID.

Validation

• focused lifecycle regression passed
• npm run lint passed
• npm run typecheck passed
• git diff --check passed

A separate coverage-only doctor readiness cascade was observed and is tracked in #117; no coverage threshold, timeout, or test behavior was weakened here.

Summary by CodeRabbit

• Bug Fixes
  • Improved server shutdown reliability by continuing cleanup even when one shutdown step fails.
  • Ensured upstream processes are stopped when downstream transport closure encounters an error.
  • Shutdown errors are still reported after all cleanup attempts complete.

[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: 12 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: 74d39656-9f3f-4797-853a-b631d00c415f

📥 Commits

Reviewing files that changed from the base of the PR and between 80347ab and dd98314.

📒 Files selected for processing (2)

• src/mcp/server/miftah-server.ts
• tests/mcp-wrapper.test.ts

📝 Walkthrough

Walkthrough

MiftahServer.close() now attempts subscription, MCP server, and upstream cleanup independently, rethrowing the first error afterward. Tests add a rejecting transport and verify upstream processes stop despite downstream transport closure failure.

Changes

Shutdown lifecycle

Layer / File(s)	Summary
Best-effort shutdown and lifecycle regression coverage src/mcp/server/miftah-server.ts, tests/mcp-wrapper.test.ts	MiftahServer.close() continues all cleanup steps while preserving the first error; lifecycle tests simulate transport close failure and verify upstream processes stop.

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

Possibly related PRs

• mohanagy/miftah#70: Both changes address MiftahServer shutdown and upstream cleanup behavior.

Poem

A rabbit watched the shutdown trail,
Each cleanup step stayed on the rail.
Though transport closure cried,
Upstreams stopped inside,
Then the first error hopped outside.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	It covers summary, root cause, change, regression coverage, and validation, but omits the required Security impact section and template-style validation checklist.	Add a Security impact section and list exact validation commands/results in the repository's template format.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title concisely matches the main change: preserving upstream cleanup when transport close fails.
Linked Issues check	✅ Passed	The code fixes the shared shutdown boundary, preserves cleanup, and adds a focused regression test matching #112's core acceptance criteria.
Out of Scope Changes check	✅ Passed	All changes stay within the server shutdown path and its regression test; no unrelated code or behavior changes are introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/112-process-backed-test-cascade

---

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

[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 `@tests/mcp-wrapper.test.ts`:
- Around line 222-225: Update the regression tests using RejectingCloseTransport
so they create one shared Error instance for the simulated close failure, inject
that instance into the transport, and assert MiftahServer.close() rejects with
the identical object via rejects.toBe(closeError) rather than checking only its
message.

🪄 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: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a1017302-d05c-41da-a157-f5073d58a3b4

📥 Commits

Reviewing files that changed from the base of the PR and between 6f0a17d and 80347ab.

📒 Files selected for processing (2)

• src/mcp/server/miftah-server.ts
• tests/mcp-wrapper.test.ts