feat: repoint MCP tools and UI project calls to core#1329
Conversation
geoffjay
left a comment
There was a problem hiding this comment.
Review: feat: repoint MCP tools and UI project calls to core
Stack position: This PR (issue-1312) is stacked on issue-1311 (PR #1328). PR #1328 was just given a needs-rework label in its own review. This PR's code is clean and correct, but it cannot merge until #1328 is addressed and its fixes propagate up the stack.
Non-blocking: stale JSDoc comment in ProjectPicker.tsx
The file-level comment at lines 1–4 still describes the component as fetching from the orchestrator:
/**
* ProjectPicker — dropdown that lists all projects from the orchestrator and
* lets the user select one as the active knowledge context.
*/
Should be updated to reference core now that the import switched from orchestratorClient to coreClient. Easy one-liner fix.
Non-blocking: no integration test coverage for the new core routing
tests/common/mod.rs correctly adds core_url: "http://127.0.0.1:1".to_string(), // unused to the test client so it compiles, but run_list_projects and run_get_project have no integration tests against a mock core server. This gap predates this PR — the functions weren't tested before either — so it's not a regression, but a follow-up test using a mock_core_server() modelled on the existing mock_orch_server() would be worthwhile.
Looks good
- ENV var naming is consistent.
AGENTD_CORE_URLin the MCP crate andAGENTD_MCP_CORE_URLin common config follow the exact same pattern used for all other services (AGENTD_ORCHESTRATOR_URL/AGENTD_MCP_ORCHESTRATOR_URL, etc.). No inconsistency. - URL construction is correct.
CoreClientsetsbaseUrl = serviceConfig.coreServiceUrl(defaulthttp://localhost:17000) and each method appends/api/v1/projects.ApiClientconstructs the final URL as${this.baseUrl}${path}→http://localhost:17000/api/v1/projects. ✓ withAuthpattern followed correctly.CoreClientmatches the established pattern used by every other service client (ask,communicate,knowledge,monitor,memory). ✓ProjectDetailcleanup is correct. After removingagent_count/workflow_count, the struct hasid,name,description,created_at,updated_at— which exactly matches the coreProjectResponsefields. Serde deserialization will work without any extra attributes needed. ✓- MCP config wiring is complete.
AgentdMcpConfig::from_env(),ValidateConfig,Default, the env-clear lists in all three tests, andtest_validate_bad_url_failsare all updated consistently. ✓ orchestratorClient.listProjectsis now dead code in the UI (line 324 oforchestrator.tshas no remaining callers after this PR). Not a problem for this PR but worth a follow-up cleanup.
Update MCP
run_list_projectsandrun_get_projecttools to target the core service URL (/api/v1/projects). Addcore_urltoAgentdMcpConfig(env:AGENTD_CORE_URL) andAgentdClient. Createui/src/services/core.tswith aCoreClientand updateProjectPickerto use it. Removeagent_count/workflow_countfromProjectDetailsince core does not compute them.Closes #1312