Repository navigation
fix(mcpdo): fail connections/list and MRU inference on an over-limit socket path - #2849
Merged
Merged
Conversation
…socket path (#2823) `connections/list` and `servers/show` with no name read a `daemon_unreachable` error as "nothing is connected". When the daemon socket path is over the platform's sun_path limit the daemon can never bind, so that read was wrong: the list printed `Connections (0)` and exited 0, and MRU inference said there was no most-recently-used connection, while `connect` refused with the real cause. Both now run the same `assertSocketPathWithinLimit()` check that `ensureDaemon()` runs before treating an unreachable daemon as empty, so they fail with connect's error and exit code. Neither starts the daemon or creates its directory, and an absent daemon on a valid path still reports an empty list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: cliffhall <cliff@futurescale.com>
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues were identified for the socket-path validation change.
No open issues 🎉
What changed in this PR
Fixes #2823 by making mcpdo distinguish an absent daemon from an unusable socket path, without starting the daemon.
Changes:
- Shares the socket-path validation between
connections/listand namelessservers/show. - Preserves empty-list behavior when the daemon is absent and the path is valid.
- Adds regression coverage for error reporting, exit-code parity with
connect, and avoiding daemon-directory creation.
| File | Description |
|---|---|
clients/mcpdo/src/connection/mcp.ts |
Validates socket-path length before treating an unreachable daemon as no connections. |
clients/mcpdo/__tests__/mcp-coverage.test.ts |
Tests over-limit failures for both commands, skipping Windows named pipes. |
Review effort: Balanced
Member
Author
|
Copilot review loop closed: round 1 was clean (no inline comments, no headline findings, no suppressed comments), so no further round was requested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2823
What changed
connections/listandservers/showwith no name both read adaemon_unreachableerror as "nothing is connected". When the daemon socket path is over the platform'ssun_pathlimit, the daemon can never bind, so that reading was wrong. The list printedConnections (0)and exited 0, and MRU inference reported "no most-recently-used connection", whileconnectrefused with the real cause.A small helper in
clients/mcpdo/src/connection/mcp.ts,assertNoDaemonMeansNothingConnected(), rethrows anything that isn'tdaemon_unreachable. It then runs the sameassertSocketPathWithinLimit(getDaemonSocketPath())check thatensureDaemon()runs. Both call sites use it, so they now fail with connect's exact error and exit code.connections/liststill does not start the daemon or create its directory.servers/listand the auth-name index also tolerate an unreachable daemon. They are left alone because the issue scopes the fix to these two commands. Those commands only use connections to annotate output, and they work without a daemon.Tests
clients/mcpdo/__tests__/mcp-coverage.test.tshas a new case, skipped on win32 because named pipes have nosun_pathlimit. With an over-limitMCP_INSPECTOR_DAEMON_DIRit checks that:connections/listand namelessservers/showboth fail with the socket-path error.connect's.The test fails without the fix (exit 0). The existing "do not auto-spawn" test still covers the empty-list case.
Hand smoke (built
mcpdo, macOS)npm run local:gatepassed.🤖 Generated with Claude Code