fix(mcp): defensive registrar wrap + boot-line log + contract test (closes #88)#90
Merged
Merged
Conversation
…loses #88) One failing devagentic-side registrar (canvas / docs / mutations / github / lane-h) no longer kills peer registrars in create_mcp_server — each call is wrapped in try/except with a WARN log naming the family. Boot-line INFO log on server creation: "MCP server boot: registered N tools: [...]" — deployed-container stderr now exposes exactly which tools are present in that wheel, instead of the operator having to truncate-enumerate (which is what triggered the original #88 false alarm). Contract test asserts 14 named G1/G2/G3/G4 + canvas + docs tools are present after create_mcp_server returns. A future refactor that drops a registrar surfaces as a CI failure with a precise "Missing: {...}" diff. Also defensive tests for the wrap+WARN path and the boot-line emission, including when a registrar fails. Fixed a separate latent regression in TestToolRegistration::test_all_tools_registered — the exact-equality assertion against 10 messaging tool names had been failing silently since the G1-G4 cascade landed (now 32 tools registered). Switched to subset semantics; the new contract test in test_mcp_defensive_registrars.py covers the rest.
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
1 participant
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.
Summary
Closes #88. Defensive hardening + observability + contract test for
create_mcp_server.The original #88 bug report (G2/G3/G4 tools missing from
list_tools()) was a false alarm — but the triage surfaced three legitimate defensive asks. This PR ships all three plus fixes a latent regression in the existing test suite.Three changes
Defensive registrar wrap (
mcp_serve.py:875-893)Each devagentic-side registrar call (
canvas/docs/devagentic-mutations/github/lane-h) is now wrapped in try/except with a WARN log naming the failing family. One broken family no longer crashes peer registrations.Boot-line INFO log (
mcp_serve.py:895-908)MCP server boot: registered 32 tools: ['canvas_add_node', ...]on everycreate_mcp_servercall. Surfaces in deployed-container subprocess stderr, exposing exactly which tools are present in that wheel build. Removes the need to truncate-enumerate (which is what triggered the original G2/G3/G4 MCP tools missing frommcp_serve.list_tools()despite create_mcp_server() registration #88 false alarm).Contract test (
tests/test_mcp_defensive_registrars.py)Asserts 14 named G1/G2/G3/G4 + canvas + docs tools survive
create_mcp_server. A future refactor that drops a registrar or breaks the@mcp.tool()decorator path surfaces as a precise "Missing: {...}" CI failure instead of in production.Latent regression fixed
TestToolRegistration::test_all_tools_registeredhad a stale exact-equality assertion against a 10-tool messaging-only set — failing silently since the G1-G4 cascade landed (32 tools registered today). Switched to subset semantics (missing = expected - tool_names); the new contract test intest_mcp_defensive_registrars.pycovers the rest.Test plan
pytest tests/test_mcp_defensive_registrars.py— 5 new tests pass (contract + 2 defensive + 2 boot-line)pytest tests/test_mcp_serve.py::TestToolRegistration— 2 pass (was failing before this PR)test_mcp_serve.py+test_mcp_canvas.py+test_mcp_docs.py+test_mcp_subset_filter.py+test_internal_mcp_autowire.py+test_autowire_cli_path.py) — 163/163 greenMCP server boot: registered N tools: [...]line on every spawnComposition
Builds on the post-G4 cascade (#69/#72/#73/#75/#77/#79/#81/#83/#85/#87) shipped in v0.16.0. Doesn't change tool surface or behavior — pure observability + defensive hardening. No deploy gap implication beyond v0.16.0 itself.