fix(mcp): reclaim stale stdio bridge clients - #5
Conversation
Expire idle sockets, surface capacity rejection to stdio clients, and expose privacy-safe connection diagnostics plus exact disconnects. Cover disconnect cleanup, the 16-client limit, and idle bridge exit with real socket integration tests. Co-Authored-By: Codex <codex@openai.com>
Raise the default connection budget to 64 and keep idle expiry opt-in, since a library cannot assume every MCP client will respawn a deliberately closed stdio child. Preserve the 16-client saturation fixture through an injected test limit. Co-Authored-By: Codex <codex@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de5c67393a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while !remaining.isEmpty { | ||
| let written = remaining.withUnsafeBytes { raw -> Int in | ||
| guard let base = raw.baseAddress else { return 0 } | ||
| return Darwin.write(fd, base, raw.count) |
There was a problem hiding this comment.
When the listener is full and the rejected client closes before this new write, Darwin.write can raise SIGPIPE; without SO_NOSIGPIPE or an equivalent suppression, the default signal action terminates the entire host process rather than merely returning EPIPE. This library explicitly owns no app lifecycle, so it cannot rely on every host having globally ignored that signal.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| connections[id] = record | ||
| let snapshot = connectionSnapshotLocked() | ||
| stateLock.unlock() | ||
| publishConnectionChange(snapshot, at: at) |
There was a problem hiding this comment.
Whenever a client moves bytes, this calls the shared publisher directly from that client's private connection queue. As a result, the documented onConnectionChange callback—previously limited to membership changes on the accept queue—now fires for every activity update and can execute concurrently across clients, so hosts relying on serialized count notifications can race or perform expensive UI work per chunk. Publish only onConnectionsChange for activity, or serialize the legacy callback on the accept queue.
Useful? React with 👍 / 👎.
| if !undecided.isEmpty { | ||
| _ = writeAll([UInt8](undecided), count: undecided.count, to: destination) | ||
| } | ||
| if count == 0 { return .ended } | ||
| if errno == EINTR { continue } |
There was a problem hiding this comment.
If part of the capacity frame has been buffered and the next blocking read returns -1 with EINTR, this block writes the partial frame to stdout before checking errno, then continues and can still recognize the completed frame as a connection-limit response. The bridge therefore exits with code 2 but has already emitted malformed JSON to the MCP client; handle EINTR before flushing undecided, reserving the flush for EOF or a fatal read error.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification