Repository navigation
fix(context-mcp): surface describeRole failure instead of dropping task tools silently - #144
Conversation
…sk tools silently Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
zooidbot
left a comment
There was a problem hiding this comment.
Lead review: no blocking findings. Merging once CI is green.
bin.ts: the try/catch now logs the socket path and the error, which is what #133 asked for.zooid_context_statusis registered only on failure, and the same notice is added to the instructions. Agents that read instructions get it, and the tool covers clients that drop instructions. Good trade-off.- Both tests cover the right cases (failure, and success with no status tool).
Out of scope, filed as #145: a second daemon on the same data dir still orphans the live daemon's sockets, so this PR makes that failure visible but doesn't prevent it.
ZOD093/ZOD084 knowledge note: this is additive behaviour on a failure path, and the specs don't say otherwise. No spec issue needed.
zooidbot
left a comment
There was a problem hiding this comment.
Lead review, round 2: blocking. CI build-test fails in the socket tier (pnpm test:socket, src/integration.test.ts), which pnpm test doesn't run.
end-to-end tools/call → daemon socket → provider → tool result expects the exact tool list and now gets an extra zooid_context_status. The log shows why: that daemon is reachable but has no task actions, so describeRole rejects with task actions unavailable. This PR treats that the same as a dead socket.
That's a correctness problem as well as a test problem. A daemon that answers with an error isn't "unreachable", and telling the agent to ask for a restart would be wrong. Fix:
- Set
roleError(and the status tool and notice) only on a transport failure: connect error, ENOENT, ECONNREFUSED, or timeout. A daemon-side error reply should keep the old behaviour (no task tools, still logged to stderr). - Add a socket-tier test for the dead-socket case, and keep the existing end-to-end assertion unchanged.
- Run
pnpm test:socketinpackages/context-mcpas well aspnpm testbefore reporting.
A daemon that answers describeRole with an error (e.g. task actions unavailable) is reachable; only socket errors set roleError. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
zooidbot
left a comment
There was a problem hiding this comment.
Lead review, round 3: no blocking findings. isTransportError keys on the errno code. callDaemon rejects daemon replies with new Error(parsed.error), which has no code, so the split is correct. The new socket-tier test covers both sides, and CI is green. Merging.
Non-blocking: callDaemon has no timeout, so a hung daemon still blocks MCP startup. That predates this PR.
Closes #133
Repro: ran the built
bin.jswithZOOID_DAEMON_SOCKpointing at a nonexistent socket. Before: starts, logs nothing about the failure, registers no task tools. (Reproduced by reading the swallowing.catch(() => undefined); the original unlinked-socket incident was not reproduced.)Change (context-mcp only):
bin.ts: a faileddescribeRoleis logged to stderr with the socket path and error.mcp-server.ts: newroleErroroption. When set, the server instructions carry an "unreachable daemon" notice and azooid_context_statustool (returnsisError) reports the error, so the agent can say "context daemon unreachable" instead of "tool missing". Task tools stay unregistered.mcp-server.test.ts; patch changeset added.Out of scope: the second-daemon-unlinks-sockets root cause (needs a data-dir lock; separate issue).
Tests:
pnpm testin packages/context-mcp: 59 passed;pnpm typecheckclean. Built bin smoke-run against a missing socket printsdescribeRole failed (sock=…): connect ENOENT …; task tools disabled.Not verified: behaviour in a live daemon/agent session (sandbox has no daemon); prettier/lint not run (prettier not installed in the worktree,
pnpm lintprinted nothing).Knowledge impact: ZOD093 / ZOD084 (role gating) — may want a note that an unreachable daemon now yields a status tool + notice.
Plan feedback: none beyond: issue pointed at line 97 but the file's current line is ~100; comment 2 says two fixes but only the first was in scope.
🤖 Generated with Claude Code