test(mcp): prove remote status across the tools/call boundary - #1816
Conversation
`tracedecay_remote_status` was proved only below the transport. The handler unit test and `remote_status_dispatch_tests` call the dispatch function directly, and `daemon::remote_protocol_tests` reads the session runtime registry. Nothing exercised the tool through a real `tools/call`, so the two typed no-plane outcomes could diverge at the boundary without a failing test. Add the transport-level cases the existing suites do not cover: - A production composition harness mounts the Remote Brain reader with no listener and no registered node, so `tools/call` must render `unconfigured` in both markdown and JSON. - A direct protocol server never installs the reader, so the same call over JSON-RPC must render `unavailable`, with no `isError`. Both cases also assert the response carries exactly one content block, pinning that a typed read attaches no banner or token footer. The configured (`observed`) plane is left to the two suites that can mount one cheaply: the harness exposes no registry accessor, so provisioning a node here would duplicate `remote_protocol_tests` at the cost of reproducing daemon authority acquisition, a signed grant, and enrollment admission in an integration fixture. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.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: 391c6a4ddd
ℹ️ 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".
|
|
||
| let markdown = response_with_id(&responses, json!(1)); | ||
| assert_eq!( | ||
| successful_tool_text(&markdown, "markdown remote status"), |
There was a problem hiding this comment.
If only the default-markdown path regresses to return isError: true while preserving the expected text, this test still passes: successful_tool_text checks the JSON-RPC error and first text block, while the explicit isError assertion applies only to json_response. Assert the markdown result's semantic-success state as well so this case proves that an unmounted authority remains a typed result.
AGENTS.md reference: AGENTS.md:L192-L193
Useful? React with 👍 / 👎.
| ); | ||
| let json_response = response_with_id(&responses, json!(2)); | ||
| assert_eq!( | ||
| successful_tool_text(&json_response, "json remote status"), |
There was a problem hiding this comment.
If the direct transport appends a banner or token footer as another content block, both assertions still pass because successful_tool_text reads only result.content[0]. The daemon half uses status_text to require exactly one block, but this direct-server half therefore does not enforce the same response-identity contract; validate the content array and its length for both direct responses.
AGENTS.md reference: AGENTS.md:L177-L178
Useful? React with 👍 / 👎.
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. |
1 participant
Supersedes #1645. Same goal — prove
tracedecay_remote_statusat thetools/callboundary — carrying only the test and itsmodregistration,rebased onto current
master.What landed
crates/tracedecay/tests/mcp_suite/mcp_handler_test/remote_status_test.rsplus its alphabetical
mod remote_status_test;line. Two cases:ProductionProjectCompositionHarnessV1(no listener, no node)unconfigured, markdown + JSONunavailable, markdown + JSON, noisErrorBoth assert the response carries exactly one content block, which pins that
a typed read attaches no banner or token footer.
Why this gap is real
tracedecay_remote_statuswas already proved below the transport, in threeplaces: the handler unit test in
tracedecay-mcp,remote_status_dispatch_tests(dispatch function, observed + unavailable), and
daemon::remote_protocol_tests(session runtime registry, unconfigured + observed with a provisioned node and
serving listener). None of them crosses
tools/call, so the two typed no-planeoutcomes could diverge at the boundary without a failing test.
unconfiguredin particular had no coverage anywhere above the registry.
Why no configured-plane case here
The production harness exposes no registry accessor, so mounting an
observedplane in an integration test would mean reproducing daemon authority
acquisition, a database scope, a signed enrollment grant, and admission — to
re-prove what
remote_protocol_tests(registry side) andremote_status_dispatch_tests(dispatch side) already cover. Left to those twoand noted in the module doc comment.
What was dropped from #1645
Everything else on
cursor/prove-remote-status-f89fis a stale divergent forkof the already-merged #1789 and is not carried:
.github/workflows/ci.ymlpolicy change (pull_requesttrigger +ci-fulllabel gate)
(
code_index_schedulerreconcile/serving,generation_retention_test,lcm_preserved_profile_journey_test,daemon_fixture,session_search_test,and others)
runtime_acceptance_suite/advisory_runtime_acceptance.rsgit_intelligence.rs,tracedecay-global-db,tracedecay-privacy, andignored_dependency_admission_testsmcp_handler_test.rs, which deletes 17 module registrationsThe test itself is also trimmed: the
assert_ne!lines that restated theassert_eq!immediately above them (!= UNAVAILABLE_*,!= "{}",!= "_No results._\n") and a re-parse of a string already compared to its JSONliteral are gone. The
isErrorassertion on the direct-server response stays —successful_tool_textdoes not check it.#1645 is left open for its author to close.
Verification
cargo test -p tracedecay --features tracedecay/test-transport --test mcp_suite -- remote_status_testcargo clippy --workspace --all-targets --locked -- -D warningscargo fmt --all -- --checkscripts/lint-commit-range.mjs origin/master HEADCI not dispatched.
🤖 Generated with Claude Code