fix(mcp): preserve recoverable stale transports#19
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
WalkthroughThis PR refines daemon ingress stale-connection cleanup logic to preserve sockets with recoverable initialized MCP state, introduces a new public API method to query that state, bumps the version to 0.6.1 across package metadata and implementation files, updates specification and documentation to reflect the narrowed cleanup behavior, and adds integration tests validating the new socket preservation logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
Greptile SummaryThis PR fixes a bug where long-running initialized MCP transport connections could be incorrectly closed by a second daemon ingress cleanup pass after their session registry entry had been removed. The fix adds a Confidence Score: 5/5Safe to merge — targeted, well-tested fix with no regressions identified. The change is minimal and focused: a new boolean predicate on No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant C as MCP Client
participant I as daemon_ingress_server
participant S as mcp_protocol_session
participant R as session_registry
C->>I: WebSocket connect
I->>S: create session
C->>I: initialize request
I->>S: handle_line
S-->>S: sets initialized_auth_mode
S-->>I: on_agent_session_bound(session_id)
I-->>I: bind agent_session_id to connection
Note over I,R: 1st cleanup tick
I->>R: run_stale_session_cleanup
I->>I: unbind_connections_for_missing_sessions
Note right of I: reset last_activity to now
I->>I: close_stale_unbound_connections
Note right of I: conn in skipped_ids, skip
Note over I,S: 2nd cleanup tick
I->>I: close_stale_unbound_connections
I->>S: has_recoverable_initialized_state
S-->>I: true, socket preserved
C->>I: next tool call, implicit rebind
Reviews (1): Last reviewed commit: "fix(mcp): preserve recoverable stale tra..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR fixes daemon/proxy stale-connection cleanup so that initialized MCP transports remain usable (and can implicitly rebind) even after their session registry entry has been soft-reaped and later removed, while still reclaiming truly abandoned pre-initialize ingress sockets.
Changes:
- Preserve unbound daemon ingress sockets during stale cleanup when the connection’s protocol session has recoverable initialized state.
- Add/extend integration coverage for the preserved-vs-closed unbound socket behavior.
- Bump local release/runtime version identifiers to
0.6.1and update spec/docs/changelog accordingly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
local-mcp-bun/src/daemon_ingress_server.ts |
Skips stale unbound socket closure when the associated protocol session is initialized and recoverable. |
local-mcp-bun/src/mcp_protocol_session.ts |
Adds a predicate for “recoverable initialized state” and bumps serverInfo.version to 0.6.1. |
local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts |
Extends coverage to ensure recoverable initialized sockets stay open, and adds a test ensuring non-initialized stale sockets still close. |
specs/local-multiplexed-browser-mcp-spec.md |
Updates requirements language to reflect the recoverable initialized-state exception for unbound ingress cleanup. |
specs/local-multiplexed-browser-mcp-changelog.md |
Records the behavioral change, verification commands, and version bump. |
local-mcp-bun/README.md |
Clarifies cleanup semantics for initialized transports vs pre-initialize unbound socket leaks. |
local-mcp-bun/package.json |
Version bump to 0.6.1. |
local-mcp-bun/chrome-extension/manifest.json |
Extension version bump to 0.6.1. |
local-mcp-bun/src/bridge_transport.ts |
Synthetic extension fixture version bump to 0.6.1. |
agent-logs/2026-04-30T004933-0500-recoverable-stale-transport.md |
Adds supporting investigation/validation notes for the change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Root Cause
The previous stale cleanup recovery path kept live transports open during soft-reap, but a later daemon ingress pass could still classify an initialized transport as a stale unbound socket after its registry binding had been removed. That closed long-running agent transports that were otherwise capable of implicit rebind.
Validation
bun test local-mcp-bun/tests/integration/daemon_ingress_cleanup.test.ts local-mcp-bun/tests/unit/mcp_protocol_session.test.tsbun run --cwd local-mcp-bun lint:specbun run --cwd local-mcp-bun lint:docsbun run --cwd local-mcp-bun lint:compliancebun run --cwd local-mcp-bun release:check-version -- --version v0.6.1bun run --cwd local-mcp-bun package:release -- --mode=server --version v0.6.1 --cleanserverInfo.version="0.6.1".Summary by CodeRabbit
Release Notes – v0.6.1
Bug Fixes
Documentation
Chores