feat(mcp): finish the per-config connection lookups (#5701)

[ntdatt812]

Closes #5701.

The issue is right, but main has moved

connections::connect resolves per-config through host::for_config while the by-server-id lookups read the process-global host::try_service, so a host that connects through the facade can never see the connection it just made.

Since the issue was filed, main already grew two of the answers:

function	added
all_connected_tools_for_config	before the issue
disconnect_for_config	e34226cb0, 2026-08-23 16:35 UTC

The issue was opened at 19:14 UTC the same day, ~2h40m after disconnect_for_config landed, and lists disconnect among the broken seven. That is not a mistake on the reporter's part — I checked git cat-file -p 523ef352a:src/openhuman/mcp/registry/mod.rs, the revision the issue names, and neither _for_config variant exists there. The issue is accurate for its base and stale against main.

That changes what needs doing. This is not one odd function out of eight; it is a migration with two of seven done. disconnect_for_config's own doc comment already spells out the trap the issue spends a page reconstructing:

the by-server-id form resolves through the process default, which stops answering once a second workspace is open

So the three resolutions the issue proposes — take a config on all eight, take a &McpHost on all eight, or make connect read the global — are all breaking, while the maintainers had already chosen a fourth: add variants, leave the ambient forms alone. This follows that.

What this adds

Five functions, each next to its ambient counterpart, same shape as the two that exist:

connected_overview_for_config    server_tools_for_config    is_connected_for_config
auth_hint_for_config             last_error_for_config

Purely additive — no existing signature changes and no caller moves.

Each Err arm now logs the workspace-resolution failure rather than folding it silently into false / None / Vec::new(). That is the issue's second ask: "the service is not up" and "the server is not connected" are different facts and currently share a return value.

One naming judgment call for you to overrule if you disagree: server_tools_for_config, not tools_for_config. The mechanical suffix would put tools_for_config directly beneath all_connected_tools_for_config, where it reads as that function's every-server sibling rather than as the one-server form.

Test

per_config_lookups_see_a_per_config_connection in tests/mcp_registry_e2e.rs.

Making it discriminate was the interesting part. With a single workspace open, try_service happily returns that lone host, so a wrong implementation routed through the global would still pass. The test therefore opens two workspaces first and asserts the precondition explicitly:

assert!(host::try_service().is_none(), ...);

With two open and no default, try_service refuses to guess — which is the state an embedder holding its own per-config service is in permanently. It then connects through the facade in workspace A and asserts the per-config lookups see it, that workspace B does not, and that disconnect_for_config drops it.

Verified red against the wrong implementation: reimplementing is_connected_for_config over try_service() (the way the ambient form does) turns it red — panicked at tests/mcp_registry_e2e.rs:723: the workspace that connected must see its own connection.

Verification

• cargo test --features mcp --test mcp_registry_e2e — 17 passed.
• cargo check --lib --features mcp — clean.
• cargo check --lib --no-default-features — clean. stub.rs deliberately gets no additions: its docs say it mirrors only the surface always-compiled callers depend on, and these five have no such caller yet. The disabled build is the drift catcher and it is green.
• cargo fmt --all — clean.

Summary by CodeRabbit

• New Features

  • Added workspace-specific connection status and server lookup support.
  • Connection details, available tools, authentication hints, and recent errors can now be retrieved for a selected workspace.
  • Workspace connection actions remain isolated, even when no global connection is configured.

• Bug Fixes

  • Improved connection lookups to consistently use the selected workspace rather than a process-wide default.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 3ea4da77-ef6b-47f4-879a-d9ddb59cb971

📥 Commits

Reviewing files that changed from the base of the PR and between fa044d3 and a854a5e.

📒 Files selected for processing (2)

• src/openhuman/mcp/registry/mod.rs
• tests/mcp_registry_e2e.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• src/openhuman/mcp/registry/mod.rs
• tests/mcp_registry_e2e.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The MCP connections facade adds five configuration-scoped lookup functions. Each resolves the host for the supplied workspace configuration and returns a benign default when no host exists. An end-to-end test verifies isolation between two workspaces.

Changes

Per-config MCP connection lookups

Layer / File(s)	Summary
Per-config lookup facade and validation src/openhuman/mcp/registry/mod.rs, tests/mcp_registry_e2e.rs	Adds configuration-scoped overview, tools, connection, authentication hint, and error lookups. Missing hosts produce logged default results. The test confirms workspace isolation and disconnect behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to a854a

This PR adds per-configuration connection lookups without changing existing APIs or caller behavior, with focused test coverage and successful validation; no actionable merge-blocking risk remains.

Poem

A rabbit checks each workspace door
The right host answers, not one global store
Tools and hints stay safely in line
Errors remain where connections shine
Two burrows test the paths anew
Then every link is cleared from view

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR adds five per-config lookup functions and verifies workspace isolation, but it does not add a per-config equivalent for all_connected_tools. The existing ambient APIs also remain process-glob…	Add all_connected_tools_for_config or otherwise provide consistent per-config service resolution for every connection API covered by issue #5701. Extend the end-to-end test to cover this API and confirm all required operations use the sam…

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the MCP connection lookup changes and matches the primary objective in issue #5701.
Out of Scope Changes check	✅ Passed	The modified registry functions and end-to-end test directly support the per-config MCP connection lookup objectives. No unrelated code changes are identified.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 2 files.

Full details: Linked Issues check

Explanation

The PR adds five per-config lookup functions and verifies workspace isolation, but it does not add a per-config equivalent for all_connected_tools. The existing ambient APIs also remain process-global, so the linked issue's requirement for consistent service resolution across all connection APIs is incomplete [#5701].

Resolution

Add all_connected_tools_for_config or otherwise provide consistent per-config service resolution for every connection API covered by issue #5701. Extend the end-to-end test to cover this API and confirm all required operations use the same configured service.

• Fix all pre-merge checks with AI

Warning

Your free Security trial is over. An organization admin can upgrade to Advanced for continuous pull request security review or dismiss this notice.

---

_{Comment @coderabbitai help to get the list of available commands.}

[tinysweeper]

tinysweeper found nothing blocking. Approving.

$0.0000 · 0 in / 0 out

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[tinysweeper]

How this change flows

2 changed behaviours across 6 relationships. 5 surrounding behaviours are shown (60 graph nodes walked). 42 further behaviours left out to keep the diagram readable.

flowchart LR
  n0["connections<br/>changed"]:::changed
  n1["supervisor_skips_a_disabled_server<br/>changed"]:::changed
  n2["host"]:::impacted
  n3["make_installed_server"]:::impacted
  n4["fresh_workspace_config"]:::impacted
  n5["format"]:::impacted
  n6["format_connected_mcp_block"]:::impacted
  n1 -->|calls| n2
  n1 -->|calls| n3
  n1 -->|calls| n4
  n3 -->|calls| n5
  n3 -->|tests| n5
  n6 -->|uses| n0
  classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
  classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
  classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
  classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff

Loading

_{Green: changed behaviour. Grey: surrounding behaviour. Arrows name the call, use, implementation, or test relationship. Orange: has findings. Red: has a finding that blocks the merge.}

[M3gA-Mind]

Maintainer review (merge-readiness sweep).

Disclosure first: earlier in this sweep I ran GitHub's "Update branch" on this PR, which pushed a merge commit — a854a5e4 "Merge branch 'main' into fix/5701-connections-for-config", authored by the fleet account — onto your branch. That is the only change: no content of mine, no rebase, no force-push, your commits untouched. The sweep was re-scoped to review-only for external contributions after that, so nothing further will be pushed here. Apologies for the surprise commit; flagging it so you do not go looking for where it came from.

Assessment: sound, and I could not find anything wrong with it

Verified against current main rather than against the PR's original base:

• Nothing has landed that duplicates it — connected_overview_for_config, server_tools_for_config, is_connected_for_config and auth_hint_for_config are all absent from src/, so the gap mcp registry connections facade: connect() is per-config, the other seven read the process global #5701 describes is still open.
• Every symbol it builds on still exists: host::for_config (src/openhuman/mcp/host.rs), and connected_overview / tools_for / is_connected / auth_hint on the dynamic connections handle. The siblings it mirrors — all_connected_tools_for_config (registry/mod.rs:109) and disconnect_for_config (:212) — are both still there, so the naming and the error-shape stay consistent with their neighbours.
• Purely additive, no new callers. Every new function is a _for_config counterpart of an existing ambient one, each degrading the no-host case the same way its sibling does (log at debug, return the empty/None/false answer). Nothing existing changes behaviour.

The server_tools_for_config naming note in the doc comment — chosen so it cannot be misread as the every-server form two functions above — is a good call and worth keeping.

State

MERGEABLE, and the full check suite was green on the pre-update head (Rust Quality, Core Coverage, Feature-Gate Smoke, Test Inventory, RSS, the lot); the refreshed run against today's main is finishing now. The BLOCKED merge state is the repo's two-approval requirement, not a check failure — that is for a maintainer to clear, and I am deliberately not approving anything in this sweep.

Nothing needed from you that I can see.

[M3gA-Mind]

Approved after a maintainer-side verification pass.

Verified on the current head: MERGEABLE against main, zero failing and zero pending required checks, and no unresolved, non-outdated review threads.

This is one of two required approvals; a second maintainer review is still needed before merge.