fix(runtime-core): close partially initialized MCP clients when startup fails by tkislan · Pull Request #452 · deepnote/deepnote

tkislan · 2026-08-08T09:56:12Z

Summary

Fixes #449. executeAgentBlock created MCP clients with Promise.all, which rejects as soon as one createMCPClient call fails — before the result is ever assigned to mcpClients. Clients that had already resolved were then invisible to the cleanup loop in the finally block, leaking their stdio subprocesses.

What changed

packages/runtime-core/src/agent-handler.ts: switch to Promise.allSettled, retain every fulfilled client into mcpClients, then re-throw the first rejection so the existing (untouched) finally block closes what was actually started. The original startup error still propagates unwrapped.

Test plan

Added a regression test: one client starts successfully, one fails to start → the successful client's close() is called exactly once, the original error still propagates via rejects.toBe, and the agent never runs.
Confirmed the new test fails on the pre-fix code at the close() call-count assertion, then passes after the fix.
pnpm test (2785 passed), pnpm typecheck, and pnpm biome:check all clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Aip97YnPuwVwg4zpNLkfMc

Summary by CodeRabbit

Bug Fixes
- Improved reliability when connecting to multiple MCP servers during startup.
- Successfully connected servers remain available even if another server connection fails.
- Improved cleanup after partial startup failures to prevent resource leaks.
- Failure messages now identify the affected server more accurately, including when cleanup encounters an error.

…up fails executeAgentBlock created MCP clients with Promise.all, which rejects as soon as one createMCPClient call fails — before the result is ever assigned to mcpClients. Clients that had already resolved were then invisible to the cleanup loop in the finally block, leaking their stdio subprocesses. Switch to Promise.allSettled: retain every fulfilled client into mcpClients first, then re-throw the first rejection so the existing finally block closes what was actually started. The original startup error still propagates unwrapped; the finally block itself is untouched. Adds a regression test with one client that starts successfully and one that fails, asserting the successful client's close() is called and the agent never runs. Closes #449 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Aip97YnPuwVwg4zpNLkfMc

codecov · 2026-08-08T09:58:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.30%. Comparing base (1a2baed) to head (5021fa5).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #452      +/-   ##
==========================================
+ Coverage   88.24%   88.30%   +0.06%     
==========================================
  Files         191      191              
  Lines       10691    10697       +6     
  Branches     2993     2996       +3     
==========================================
+ Hits         9434     9446      +12     
+ Misses       1255     1249       -6     
  Partials        2        2

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ilure attribution When a failing MCP server precedes a successful one, the compacted mcpClients array loses index alignment with mergedMcpConfig, causing close-failure warnings to name the wrong server. Pair each client with its config name at assignment time so the finally block always reports the correct server. Adds a regression test with failure-first ordering that asserts the warning message contains the correct server name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MH71aBtxrizLCVrWUhSAzj

tkislan · 2026-08-10T19:55:17Z

@coderabbitai full review

coderabbitai · 2026-08-10T19:55:23Z

✅ Action performed

Full review finished.

coderabbitai · 2026-08-10T19:55:39Z

📝 Walkthrough

Walkthrough

MCP client initialization now uses Promise.allSettled to retain successful clients when another startup fails. The original failure is rethrown after successful clients are recorded. Tool loading and cleanup use stored client and server-name pairs. Tests verify cleanup after partial startup failure and correct server attribution for close-failure warnings.

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

Sequence Diagram(s)

sequenceDiagram
  participant executeAgentBlock
  participant MCP initialization
  participant MCP client
  executeAgentBlock->>MCP initialization: start configured clients
  MCP initialization-->>executeAgentBlock: settled client results
  executeAgentBlock->>MCP client: load tools from successful clients
  executeAgentBlock->>MCP client: close retained clients during cleanup
  MCP client-->>executeAgentBlock: close result or warning

Possibly related issues

deepnote/deepnote issue 450 — Shares the MCP initialization, tool discovery, cancellation, and cleanup scope.

Possibly related PRs

deepnote/deepnote#342 — Introduces related MCP client initialization, loading, and cleanup logic.
deepnote/deepnote#445 — Contains related MCP lifecycle handling and tests.

Suggested reviewers: m1so, jamesbhobbs

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes retain successful clients, rethrow initialization errors, close retained clients, and add the required regression tests for issue [`#449`].
Out of Scope Changes check	✅ Passed	The changes are limited to MCP initialization cleanup and related regression tests described in issue [`#449`].
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Updates Docs	✅ Passed	This PR fixes internal MCP cleanup and adds no user-facing feature or public API; existing Agent/MCP docs remain accurate. Verify the private roadmap separately because that repo is not visible here.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main fix: closing partially initialized MCP clients after startup failure.

_{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 `@packages/runtime-core/src/agent-handler.ts`:
- Around line 192-194: Update the startup handling around the client-settling
logic in agent-handler.ts to record each rejection as it occurs and preserve the
first rejection observed in time, rather than selecting the first rejected
result by configuration order. Throw the recorded error after all startup
promises settle, and add a regression test covering two servers that fail in
reverse configuration order.

🪄 Autofix

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: CHILL

Plan: Pro

Run ID: 9e17d696-1d12-45fe-b723-78a1e0dc3cb2

📥 Commits

Reviewing files that changed from the base of the PR and between 0ea255d and b86edc6.

📒 Files selected for processing (2)

packages/runtime-core/src/agent-handler.ts
packages/runtime-core/src/execute-agent-block.test.ts

tkislan and others added 2 commits August 10, 2026 18:56

Remove redundant comment

b86edc6

coderabbitai Bot requested changes Aug 10, 2026

View reviewed changes

Comment thread packages/runtime-core/src/agent-handler.ts

coderabbitai Bot approved these changes Aug 11, 2026

View reviewed changes

tkislan marked this pull request as ready for review August 11, 2026 05:37

tkislan requested a review from a team as a code owner August 11, 2026 05:37

Merge branch 'main' into fix/issue-449-mcp-client-cleanup

b0b3dd4

dinohamzic approved these changes Aug 11, 2026

View reviewed changes

tkislan enabled auto-merge (squash) August 11, 2026 15:16

Merge branch 'main' into fix/issue-449-mcp-client-cleanup

5021fa5

tkislan merged commit 3a3a15c into main Aug 12, 2026
21 checks passed

tkislan deleted the fix/issue-449-mcp-client-cleanup branch August 12, 2026 20:25

tkislan mentioned this pull request Aug 13, 2026

chore(release): bump package versions #444

Merged

coderabbitai Bot mentioned this pull request Aug 14, 2026

fix(runtime-core): avoid MCP tool discovery after agent cancellation (#450) #457

Draft

Navigation Menu

Uh oh!

fix(runtime-core): close partially initialized MCP clients when startup fails - #452

fix(runtime-core): close partially initialized MCP clients when startup fails#452
tkislan merged 5 commits into
mainfrom
fix/issue-449-mcp-client-cleanup

tkislan commented Aug 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

codecov Bot commented Aug 8, 2026 •

edited

Loading

Uh oh!

tkislan commented Aug 10, 2026

Uh oh!

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

Walkthrough

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

tkislan commented Aug 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What changed

Test plan

Summary by CodeRabbit

Uh oh!

codecov Bot commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tkislan commented Aug 10, 2026

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Walkthrough

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tkislan commented Aug 8, 2026 •

edited by coderabbitai Bot

Loading

codecov Bot commented Aug 8, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading

coderabbitai Bot commented Aug 10, 2026 •

edited

Loading