refactor(mcp): answer info and runtime reads through their owners - #2335
Conversation
tracedecay_status, tracedecay_active_project, tracedecay_remote_status and tracedecay_runtime are graph-tool operations the project's graph-tool owner answers, and tracedecay_project_list, tracedecay_project_search and tracedecay_project_context are profile registry reads the daemon's profile owner answers for every connection, with or without a project. Each decodes its arguments against a typed request, so an unknown key or a wrong type is refused instead of ignored, and each result is a typed catalog result that serializes to the JSON these tools already emitted. The projectless connection's hand-written registry path is folded into the profile owner, and the per-server registry read port is gone. The doctor and remote-status types derive JsonSchema. A route refusal from an owner now keeps its reason code instead of reading as a config error.
|
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: 01daa780ed
ℹ️ 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".
| #[serde(untagged)] | ||
| pub enum StatusResultV1 { | ||
| Admission(StatusAdmissionV1), | ||
| Project(Box<ProjectStatusV1>), | ||
| } |
There was a problem hiding this comment.
When the first tracedecay_status request arrives while project opening is incomplete, serve_core_doctor_runtime_request returns project_open_status_value, which contains project_open, uses schema_convergence.status = "unavailable", and omits required ProjectStatusV1 fields such as memory and code_index_freshness. Neither variant of this new untagged result accepts that routine startup response, so the advertised/generated result contract cannot decode an actual production status result; add a typed project-opening variant or translate the core response into the canonical shape.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)] | ||
| #[serde(deny_unknown_fields)] | ||
| pub struct RuntimeResultV1 { |
There was a problem hiding this comment.
For tracedecay_runtime startup-health/full-doctor requests, and for doctor_report requests before the owner report is ready, the daemon core returns doctor_runtime_value directly. That payload omits required fields such as captured_at and host_os, adds doctor_runtime, project_open, and git_watcher, and its fallback doctor report has a reason while omitting fields required by RuntimeDoctorReportV1::Unknown; consequently this new deny_unknown_fields result type and generated clients reject a production response for the same catalog operation. The core path needs its own typed variant or normalization into RuntimeResultV1.
AGENTS.md reference: AGENTS.md:L9-L12
Useful? React with 👍 / 👎.
| Some(cancellation), | ||
| ) | ||
| .await?; | ||
| let response_handle_root = cli_response_handle_root(profile, dispatch.project_path.as_deref())?; |
There was a problem hiding this comment.
When a profile-registry CLI read names an initialized project whose repository identity marker is malformed or unreadable, the daemon profile owner can successfully answer from the registry, but this new post-response call re-reads that project marker through resolve_persisted_layout and replaces the valid result with a local error. Registry list/search/context are profile-owned and previously rendered projectlessly without a response-handle root—listing rendering still explicitly ignores the root—so these calls should render with no project layout lookup rather than reacquiring an unrelated project authority.
Useful? React with 👍 / 👎.
1 participant
What changed
The info and health tools that still went through
LegacyToolCompatibilityOwnerand the generic dispatch fallback now run through typed owners:tracedecay_status,tracedecay_active_project,tracedecay_remote_status,tracedecay_runtime.tracedecay_project_list,tracedecay_project_search,tracedecay_project_context. They travel as a newProfileGraphToolinvocation (InvocationTarget::Profile). The handshake's project, if there is one, only marks which listing entry is active.Each tool decodes its arguments into a typed request (
deny_unknown_fields) and returns a typed catalog result (project_info_surface.rs) that serializes to the JSON these tools already emitted. An unknown or mistyped argument is now refused by the owner, not silently ignored.statusstill carrieswait_for(#2179, and thestructuredContent.waitfrom #2314 that the CLI uses for its exit code),memory(#2194/#2220, including the #2301 holder shape),github_source(#2221/#2260),reset_required_stores, and therestoringfreshness state from #2293.Deleted:
project_*path indaemon/projectless.rs, now folded into the profile owner.project_registry_readsport.ProjectRegistryListingOutcomeand theRegistryUnavailablevariants.dispatch_groups/health_dispatch.rsand theHealthdispatch group.server_stats/scope_prefixparameters ofhandle_tool_call*. The owner reads them from the serving server.JsonSchema derives added: the remote-status types (
remote/status.rs), doctor sources (LanguageServerReadV1,RemoteListenerReadV1, the language-server state and analyzer enums,ResidentMemoryHolderReadV1),DoctorReportV1,TableGrowthDoctorEvidenceV1, the domain remote authority types,AuthorityEpoch, andStoreResetRequiredV1. Contracts and the TypeScript SDK are regenerated.Compat symbols are left in place for the deletion PR. Sibling tools are untouched.
Proof
Fail before / pass after through the real MCP server (
mcp_suite info_health_request_test). Onorigin/master,status {"include_diagnostics": true}answered markdown and the test failed. Now each tool refuses with its owner's problem record, for example:{"kind":"invalid_request","code":"application.surface.invalid_request","message":"invalid arguments for tracedecay_status: unknown field \include_diagnostics`, expected one of `admission_only`, `include_branch_diagnostics`, `include_storage_health`, `include_session_ingest`, `include_staleness`, `wait_for`"}`The same holds for
active_project {"project"},remote_status {"kind"},runtime {"doctor"},project_list {"limt"}and{"limit":2.5},project_search {"query":12}, andproject_context {"project_selector":{"project":…}}.The projectless socket test gained a refused call. With the
mark_semantic_tool_errorfix removed it fails (isError: null), and it passes with the fix.Journey on the built binary (isolated HOME, one daemon under
systemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1G, stopped afterwards):tracedecay servetools/list: 230.status:admission_onlyreturnsproject_admitted: true;include_diagnosticsreturnsisError: true.active_project:proj_…,active_project,code_project.remote_status:{"kind":"unconfigured"}.project_list:ok, with the project markedis_active: true;limtreturnsisError: true.project_context:ok,is_active: true.runtime {"doctor_report":true}: doctor reportkind: observed, keys[kind, language_servers, report, schema_convergences, table_growth_evidence], report keys[coverage, entries];doctorreturnsisError: true.statusserves memory owners in the perf(code-index): share one decode across linked worktrees #2301 holder shape.Counts (last runs, on the final base):
tracedecay-mcp387, contracts 420, daemon-protocol 65, daemon-service 322, domain 223, mcp-catalog 29, tool-catalog 6, api 53.mcp_suitefiltered to status/project/runtime/remote/info_health/admin/protocol/dashboard/graph_analysis/port_status/automation/analytics: 206 passed. The one failure (a process-sample timeout at load average ~150) passes alone.core_cli_suitetool_first_touch/tool_daemon: 34.-D warningsis clean with and withouttest-transport;cargo fmtis clean;contracts:checkis up to date.Reviewer notes
statusis no longer a compat tool, so the CLI tests that probe the compat transport now usemulti_root_scope_set_readwith an absent id. Its concealednot_found_or_not_authorizedanswer only comes from an admitted project server. The fix(cli): type readiness waits and projectless refusals end to end #2314 fake-daemon wait-outcome test became a unit test oftool_result_process_outcome; the owner'sstructuredContent.waitis pinned indispatch_tests.problemat the top level, like the existing retained projectless path, rather than instructuredContent. That is pre-existing behavior of the projectless route, not changed here.