feat(mcp): Dim 5 leftovers 2/2 — .mcp.json layered scopes + client resources#116
feat(mcp): Dim 5 leftovers 2/2 — .mcp.json layered scopes + client resources#116sweetcornna wants to merge 1 commit into
Conversation
…sources
Closes the last two verified-open Dim 5 gaps
(audit/GAP_CLOSE_2026-07-08.md A2 + A3).
A2 — .mcp.json scopes/precedence. New corlinman_mcp_server.scoped_config:
claude-code-style layered server discovery — local
(<project>/.mcp.local.json, gitignored) > project (<project>/.mcp.json,
checked in) > user (~/.corlinman/mcp.json) > inline [mcp] gateway
config; dedup by server name, strongest scope wins, shadowed servers
keep their original listing position. File shape matches claude-code's
{"mcpServers": {...}} with mcp_servers/servers accepted as aliases;
every entry parses through the existing McpServerSpec.from_mapping.
Total-function: malformed files/entries are logged and skipped, never
raised. Wired at BOTH bring-up points (gateway lifespan + embedded
console), each with an inline-config fallback.
A3 — client resources. McpClientManager discovers resources/list at
bring-up (paged like tools/list; a server without the capability folds
to [] and cannot fail a tools-only bring-up) and exposes
read_resource(server, uri) -> McpToolCallOutcome (text contents folded,
blob parts become a placeholder note; never raises). Agent surface:
each resource-bearing server gains a synthetic {server}_read_resource
tool through the EXISTING advertise/route seams (schema enumerates up
to 10 resources to bound per-turn bloat); the bridge routes it to
resources/read. Literal-wins on both sides: a server exposing a real
read_resource tool keeps normal tools/call dispatch and the synthetic
is never advertised. The allowedMcpServers/deniedMcpServers policy
applies to the resources map too.
Tests: 6 scoped-config, 8 client-resources, 7 advertise/bridge routing
(incl. the literal-wins dispatch proof both ways). mcp-server + gateway
mcp + console suites 457 passed; ruff/mypy/lint-imports green.
github-actions
Bot
added
codex:needs-review
![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: 953342c443
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| managed.resources = await asyncio.wait_for( | ||
| self._list_resources(peer), | ||
| timeout=spec.handshake_timeout_s, | ||
| ) |
There was a problem hiding this comment.
For an MCP server that exposes resources but no tools, a tools/list method-not-found is raised by the earlier required tool discovery and _bring_up returns before this resources discovery runs. The new advertisement path supports resources-only servers, but the manager will mark real resources-only servers as error, so discovered_resources() never surfaces them; make tools/list best-effort (or capability-gated) when resources can still make the server useful.
Useful? React with 👍 / 👎.
| bare = _bare_tool_name(server, target_tool) | ||
| if ( | ||
| bare == "read_resource" | ||
| and not _server_has_tool(self._manager, server, "read_resource") |
There was a problem hiding this comment.
If a server named srv exposes a real tool named srv_read_resource but not read_resource, the advertised call is srv_srv_read_resource; _strip_server_namespace correctly resolves that to the literal srv_read_resource, but this branch then strips the prefix again for classification and hijacks the call to resources/read. Limit the synthetic-resource branch to the exact synthetic advertised tool instead of any target tool whose literal name ends up as {server}_read_resource.
Useful? React with 👍 / 👎.
| # synthetic ``{server}_read_resource`` tool (tolerant getattr: | ||
| # older/mock managers without the surface contribute none). | ||
| _res_fn = getattr(mcp_manager, "discovered_resources", None) | ||
| _resources = _res_fn() if callable(_res_fn) else None |
There was a problem hiding this comment.
When a connected MCP server changes its resources after boot, this re-advertisement still reads the boot-time discovered_resources() snapshot; the manager only handles tools/list_changed, so notifications/resources/list_changed never re-lists resources or triggers this path. A server that starts with no resources and later publishes one will never get {server}_read_resource advertised until restart, and changed URI lists leave stale tool descriptions; mirror the tools refresh for resources before using this snapshot.
Useful? React with 👍 / 👎.
| user = Path(user_dir) if user_dir is not None else Path.home() / ".corlinman" | ||
| return { | ||
| "user": user / "mcp.json", | ||
| "project": project / ".mcp.json", |
There was a problem hiding this comment.
When a cloned repository contains .mcp.json, this adds that project file to the auto-loaded scopes, and connect_all() later spawns any stdio commands from it with no trust or approval gate. Claude Code's MCP docs state project-scoped .mcp.json servers require approval before use (https://code.claude.com/docs/en/mcp), so simply starting the console/gateway inside an untrusted checkout can execute repository-controlled commands; require a persisted approval/trust record or keep project scope disabled until approved.
Useful? React with 👍 / 👎.
| specs: list[McpServerSpec] = [] | ||
| for name, body in servers.items(): | ||
| try: | ||
| specs.append(McpServerSpec.from_mapping(str(name), body)) |
There was a problem hiding this comment.
For .mcp.json entries copied from Claude Code such as {"type":"http","url":"..."} or type: "sse", this passes the raw body into from_mapping, which only reads transport; it ignores type, infers ws from the URL, and dials HTTP/SSE endpoints as WebSockets. Claude Code's MCP docs require URL entries to carry a type field (http, sse, or ws) (https://code.claude.com/docs/en/mcp), so imported remote-server configs fail to connect unless type is translated or unsupported transports are skipped explicitly.
Useful? React with 👍 / 👎.
| specs: list[McpServerSpec] = [] | ||
| for name, body in servers.items(): | ||
| try: | ||
| specs.append(McpServerSpec.from_mapping(str(name), body)) |
There was a problem hiding this comment.
Claude Code .mcp.json supports ${VAR} and ${VAR:-default} expansion in command, args, env, url, and headers, but this path parses file entries directly into McpServerSpec, which stringifies those values without expansion. Shared project configs that rely on ${API_KEY} or ${CLAUDE_PROJECT_DIR:-.} will launch with literal placeholders or fail authentication; expand these fields before from_mapping and skip entries with missing required variables.
Useful? React with 👍 / 👎.
github-actions
Bot
added
codex:reviewed
1 participant
Summary
Closes the last two verified-open Dim 5 gaps (
audit/GAP_CLOSE_2026-07-08.mdA2 + A3). Stacked on #115 (part 1/2) — base branch isfeat/dim5-mcp-leftovers; retarget tomainafter #115 merges.A2 —
.mcp.jsonlayered scopesNew scoped_config.py: claude-code-style layered discovery —
local(<project>/.mcp.local.json) >project(<project>/.mcp.json) >user(~/.corlinman/mcp.json) > inline[mcp]config; name-keyed dedup, strongest scope wins, shadowed servers keep their listing position. Accepts claude-code's{"mcpServers": {…}}plusmcp_servers/serversaliases. Malformed files/entries log-and-skip (a config file must never kill a boot). Wired at both bring-up points (gateway lifespan + embedded console) with inline-config fallback.A3 — client resources
McpClientManagerdiscoversresources/listat bring-up (paged; capability-less servers fold to[]and can't fail a tools-only bring-up) +read_resource(server, uri)(text folded, blobs become a placeholder note, never raises).{server}_read_resourcetool through the existing advertise/route seams — schema enumerates ≤10 resources to bound per-turn bloat.McpToolBridgegains theresources/readbranch.read_resourcetool keeps normaltools/calldispatch; the synthetic is never advertised for it (dispatch proof tested in both directions).allowedMcpServers/deniedMcpServerspolicy applies to the resources map too.Tests
6 scoped-config + 8 client-resources + 7 advertise/bridge routing; mcp-server + gateway-mcp + console suites 457 passed; ruff/mypy/lint-imports green.