simplify(mcp): delete the tool compatibility dispatch layer - #2366
Conversation
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: b20c6b3fe3
ℹ️ 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".
| pub fn invocation_client_for_current_client( | ||
| handshake: DaemonHandshake, | ||
| ) -> Result<tracedecay_daemon_protocol::DaemonInvocationClient> { | ||
| let socket_path = default_available_socket_path(&handshake)?; |
There was a problem hiding this comment.
When a CLI call lands after DaemonAuthority::acquire has published the new authority record but before bootstrap has rebound the Unix socket, default_available_socket_path rejects the absent path immediately. The previous invocation_client_for_current path handed the recorded endpoint to DaemonInvocationClient, whose connection helper waits through DAEMON_RESTART_GRACE; this change therefore makes every typed tracedecay tool call spuriously exit 69 during that normal restart window instead of retrying.
Useful? React with 👍 / 👎.
| tracedecay::daemon::handshake_for_current_client(profile, project, None, false, false)?; | ||
| let executor = FamilyToolExecutor { | ||
| client: tracedecay_daemon_identity::invocation_client_for_current(handshake)?, | ||
| client: tracedecay::daemon::invocation_client_for_current_client(handshake)?, |
There was a problem hiding this comment.
--json failures machine-readable
For a newly migrated multi-root call using --json, any socket or authority failure here is returned before handle_multi_root can render a tool result, leaving stdout empty and only printing an error to stderr. The deleted compatibility route explicitly converted the same project-route failure to {"problem": ...} on stdout, so machine callers that parse the promised JSON output now fail precisely when the daemon is unavailable; preserve that structured error behavior in the typed route before deleting the compatibility support.
AGENTS.md reference: AGENTS.md:L178-L180
Useful? React with 👍 / 👎.
1 participant
BEGIN_COMMIT_OVERRIDE
simplify(mcp)!: delete the tool compatibility dispatch layer (#2366)
BREAKING CHANGE: every
tracedecay toolroute now connects to the daemon socket named by TRACEDECAY_DAEMON_SOCKET when it is set, instead of the daemon the client profile's authority record names. A missing socket is the typed daemon-unreachable refusal with exit code 69 for every tool (previously typed tools such asstatusstill answered). The MCP generic compatibility fallback is gone: a tool name no typed owner serves isunknown tool.END_COMMIT_OVERRIDE
This finishes the MCP typed migration. Every public tool group and all four internal admin tools now answer through a typed daemon owner:
This PR deletes the compatibility layer that sat under them.
Deleted
LegacyToolCompatibilityOwnerand its cached advertised-name set (mcp/tools/mod.rs).handle_tool_call_with_registry_options. That covers the catalog-binding recheck, the owner admission, the universal-ceiling wrapper, the dead group match, and the boundary freshness trailer. A name no typed owner claims is nowunknown tool. The internal branch-add tool is still served by the daemon before MCP dispatch.resolve_catalog_tool_binding. The typed path'sresolve_application_bindingabsorbs theresolve_named_bindinghelper, which had only that caller left.validate_current_application_binding. The typed dispatch still calls it (application_surface/dispatch.rs:80) to re-check the binding identity and schemas before execution.CatalogBindingResolverdirectly:v2_surface_mount_conformance,native_integration_surface_mount, and the CLI transport-equivalence test.dispatch_compatibility_tooland its support code:DaemonToolDispatch::call,tool_timeout_error,map_tool_deadline_error, andprint_project_route_problemrecover_truncated_mcp_result,reject_tool_result_truncation, and that function's unit test.tool_cli_skips_daemon_notifications_until_matching_responsetest and its sentinel daemon. They only exercisedtracedecay tool'stools/calltransport.rgconfirms none remain.Total: 15 files, +226 / −620.
What replaces the last compat routes
tracedecay tool multi_root_*was the last advertised family on the CLI compat path. It now runs through the typed family executor, as Work and Workflow do, by calling the samehandle_multi_rootthe MCP path uses.tracedecay toolroutes connect through the newtracedecay::daemon::invocation_client_for_current_client. It resolves the socket thatTRACEDECAY_DAEMON_SOCKETnames, the same way thetools/calltransport does. A missing socket gives the typed daemon-unreachable refusal and exit code 69, which the Pi extension branches on.v2_surface_mount_conformancenow treats internal owner operations as served by name, and fails if one is listed. Since refactor(mcp): serve admin sync through the project owner #2343 it had reported the four internal operations as unmounted.Fail-before / pass-after
core_cli_suite tool_daemon_test::tool_cli_without_daemon_socket_reports_daemon_unavailablenow probes both the multi-root read and an owner-served graph tool (tool status --json) withTRACEDECAY_DAEMON_SOCKETpointing at a missing socket.On
origin/mastersource (fd10010) with only this test file changed:With this change:
ok. Both probes exit 69 withproject route error (daemon_connect_down): TraceDecay daemon socket '…/missing.sock' is not available.rg
Runtime journey
Setup: debug
tracedecayat this branch's head (b20c6b3), isolated HOME,TRACEDECAY_DATA_DIR,TRACEDECAY_GLOBAL_DB, andXDG_CONFIG_HOME, and one daemon undersystemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1G. The daemon was stopped afterwards (inactive).Checks (local, on fd10010)
mcp_suite(with the CLI built first; the test process ran outside the 6 GB scope): 588 passed, 0 failed.core_cli_suite: 146 passed, 0 failed.tracedecay-cli --bins: 333 passed.tracedecay-mcp380,tracedecay-tool-catalog7,tracedecay-daemon-service322 (theadoption_observationcensus passed), contracts 429, mcp-catalog 28.--lib -- mcp:: daemon::: 735 passed, 0 failed.transport_acceptance_suite v2_surface_mount2 passed;product_surface_suite native_integration_surface_mount7 passed.cargo clippy -D warnings --all-targetsover tracedecay, mcp, contracts, daemon-protocol, api, mcp-catalog, tool-catalog, cli, daemon-service, and agent-hosts, with and withouttracedecay/test-transport,test-helpers: clean.cargo fmt --all -- --check: clean.pnpm run contracts:check: up to date.cargo check --workspace --all-targets --target x86_64-pc-windows-gnu --features tracedecay/test-transport,tracedecay/test-helpers,tracedecay-cli/test-transportexited 0 (272 warnings, all pre-existing).