connect_mcp_http skips _expand_agent_placeholders, so {agent_id}/{agent_dir} templates silently break for HTTP MCP servers

[huangzesen]

Summary

Agent.connect_mcp expands the per-agent placeholders {agent_id}, {agent_address}, and {agent_dir} in command, args, and env so a single shared MCP registry template can scope each agent to its own namespace. Agent.connect_mcp_http does not: url and headers are passed to HTTPMCPClient verbatim, so an HTTP registry entry containing {agent_id} sends the literal placeholder string over the wire with no warning. The two transports should honor the same placeholder contract.

Narrative

The placeholder mechanism exists so that operators can maintain one MCP configuration template and stamp it out across a fleet of agents. _expand_agent_placeholders (src/lingtai/agent.py:827-845) documents the intent directly:

def _expand_agent_placeholders(self, value):
    """Substitute per-agent placeholders in an MCP launch string.

    Lets a single shared MCP registry template scope each agent to its own
    namespace without per-agent hand-editing — e.g. a NoKV workbench root
    ``--workbench-root /agents/{agent_id}/wb``. ...
    """

connect_mcp (src/lingtai/agent.py:847-908) applies it consistently to every launch-relevant string:

# src/lingtai/agent.py:865-871
command = self._expand_agent_placeholders(command)
if args:
    args = [self._expand_agent_placeholders(a) for a in args]
if env:
    env = {k: self._expand_agent_placeholders(v) for k, v in env.items()}

connect_mcp_http (src/lingtai/agent.py:910-957) was added as the HTTP twin of connect_mcp, but the expansion step was never carried over:

# src/lingtai/agent.py:924-927
from .services.mcp import HTTPMCPClient

client = HTTPMCPClient(url=url, headers=headers)
client.start()

Both methods are fed from exactly the same configuration surface. _load_mcp_from_workdir (src/lingtai/agent.py:454 onward) reads MCP entries from working_dir/mcp/servers.json and from the init.json mcp field, and its inner _spawn helper (src/lingtai/agent.py:506-548) dispatches on cfg["type"]: "http" entries go to connect_mcp_http(url=cfg["url"], headers=cfg.get("headers")), everything else goes to connect_mcp. The docstring even shows the canonical HTTP shape with an Authorization header. So from the config author's point of view there is one registry format with one documented placeholder contract — but whether placeholders actually work depends on which type the entry has.

Concretely, a user hits this today by writing a perfectly reasonable multi-tenant HTTP entry:

{
  "team-memory": {
    "type": "http",
    "url": "https://mcp.internal.example/agents/{agent_id}/mcp",
    "headers": {"X-Agent-Scope": "{agent_id}"}
  }
}

The stdio equivalent of this pattern (e.g. --workbench-root /agents/{agent_id}/wb) works and is covered by test_expand_agent_placeholders_scopes_workbench_root in tests/test_mcp_capability.py. The HTTP version sends a request to the literal path /agents/%7Bagent_id%7D/mcp with a literal {agent_id} header value. The failure mode is at best a confusing connection error logged as a generic "failed to load" warning by _spawn, and at worst something quieter and more dangerous: if the server treats the unexpanded string as a valid opaque scope key, every agent in the fleet resolves to the same namespace ({agent_id} literally), silently merging per-agent state across agents. Nothing in the kernel warns that the placeholder was left unexpanded.

The same gap propagates to the retry path: _retry_failed_mcps (src/lingtai/agent.py:684) re-invokes connect_mcp_http with the recorded spec's url/headers, so dead-MCP respawns inherit the same behavior.

How the design got here is easy to reconstruct: placeholder expansion was introduced for the stdio launch path (its docstring's motivating example is a subprocess CLI flag), and connect_mcp_http mirrors connect_mcp's structure (client construction, _mcp_clients tracking, tool registration loop) but predates or simply missed the expansion block. There is no principled reason for the asymmetry — per-agent scoping via a URL path segment or an auth/scope header is exactly the HTTP-native analog of a per-agent CLI flag.

Better looks like: connect_mcp_http expands url and header values through the same _expand_agent_placeholders helper before constructing HTTPMCPClient, so the documented placeholder contract holds regardless of transport, at boot, on refresh-retry, and for direct programmatic callers.

Current behavior

• Agent._expand_agent_placeholders (src/lingtai/agent.py:827-845) replaces {agent_id}/{agent_address} with the agent's working-dir name and {agent_dir} with the absolute working directory; non-strings and strings without { pass through unchanged.
• Agent.connect_mcp (src/lingtai/agent.py:847-908) applies it to command, each element of args, and each value of env (lines 867-871) before constructing MCPClient.
• Agent.connect_mcp_http (src/lingtai/agent.py:910-957) constructs HTTPMCPClient(url=url, headers=headers) at line 926 with no expansion of either argument.
• Both are reached from the same config via _load_mcp_from_workdir's _spawn (src/lingtai/agent.py:506-548; HTTP branch at 514-520) and from the respawn path _retry_failed_mcps (HTTP branch around src/lingtai/agent.py:684).
• Result: {agent_id}-style templates work for type: "stdio" registry entries and silently fail (literal placeholder sent to the server) for type: "http" entries.

Detailed implementation plan

1. Expand placeholders in connect_mcp_http (src/lingtai/agent.py, function at line 910). Mirror the block in connect_mcp, expanding the URL and header values (values only — header names, like env keys in connect_mcp, are not expanded):

   # Expand per-agent placeholders (e.g. {agent_id}) so a shared registry
   # template gives each agent its own scope — same contract as connect_mcp.
   url = self._expand_agent_placeholders(url)
   if headers:
       headers = {k: self._expand_agent_placeholders(v) for k, v in headers.items()}
   
   client = HTTPMCPClient(url=url, headers=headers)

   Place it immediately after the from .services.mcp import HTTPMCPClient import, matching connect_mcp's layout. Note the dict comprehension also copies headers, so the caller's dict (e.g. the spec stored in self._mcp_init_specs) is never mutated — the retry path keeps re-expanding from the pristine template, which is idempotent anyway since expanded values contain no placeholders.

2. Update the docstring of connect_mcp_http to state that url and header values undergo per-agent placeholder expansion (referencing _expand_agent_placeholders), so the contract is discoverable next to the code the same way it is for connect_mcp.

3. No changes needed in _load_mcp_from_workdir or _retry_failed_mcps (src/lingtai/agent.py:506-548 and ~658-700): both call connect_mcp_http with the raw spec, so doing the expansion inside connect_mcp_http covers boot, registry-gated init.json entries, legacy mcp/servers.json entries, and refresh-time respawns in one place. This is the main reason to fix it in the method rather than at the call sites.

4. No schema or data-format changes. The registry/init.json MCP entry shape is untouched; this only changes how existing string fields are interpreted, aligning them with the already-documented stdio behavior.

5. Back-compat consideration: a pre-existing HTTP entry whose URL or header value contains a literal {agent_id}-like substring that was intended verbatim would change meaning. This is judged acceptable: _expand_agent_placeholders only touches the three documented placeholder tokens, strings without { are untouched, and an intentional literal {agent_id} in a URL or auth header is far-fetched compared to the template use case the mechanism was built for. Mention it in the changelog. (If maintainers want extra caution, an escape convention like {{agent_id}} could be added to _expand_agent_placeholders for both transports — but that is a separate, orthogonal enhancement.)

6. Optional hardening (small, recommended): in _expand_agent_placeholders, nothing to change; but consider a debug-level log line in connect_mcp_http when expansion changed the URL, to make per-agent scoping visible in logs without leaking header values. Header values must not be logged (they commonly carry bearer tokens).

Risks and alternatives

• Risk: meaning change for literal {...} strings in existing HTTP configs — see step 5; scope is limited to the three exact tokens, and the stdio path has always behaved this way, so this brings HTTP to parity rather than introducing a novel hazard.
• Risk: leaking expanded secrets into logs. The fix itself adds no logging; if the optional debug line is added, log the URL only, never headers.
• Alternative: expand at the _spawn call site in _load_mcp_from_workdir. Rejected: it would leave direct programmatic callers of connect_mcp_http and the _retry_failed_mcps respawn path inconsistent, and would split the placeholder contract across two locations. connect_mcp sets the precedent of expanding inside the connect method.
• Alternative: expand inside HTTPMCPClient (src/lingtai/services/mcp.py:354). Rejected: the client has no knowledge of the agent's working dir; placeholder resolution is agent identity logic and belongs on Agent, exactly where _expand_agent_placeholders lives.
• Alternative: document the asymmetry instead of fixing it. Rejected: the config surface is shared (one JSON shape, one docstring at _load_mcp_from_workdir showing both types), so "placeholders work only for stdio" is a trap, not a contract.

Testing plan

Add to tests/test_mcp_capability.py, alongside the existing test_expand_agent_placeholders_scopes_workbench_root (line 174):

1. test_connect_mcp_http_expands_placeholders_in_url_and_headers — monkeypatch lingtai.services.mcp.HTTPMCPClient with a fake that records its constructor args and returns an empty list_tools(). Call agent.connect_mcp_http(url="https://h/agents/{agent_id}/mcp", headers={"Authorization": "Bearer {agent_id}", "X-Dir": "{agent_dir}"}). Assert the fake received the URL with the working-dir name substituted, Authorization value Bearer <agent_id>, and X-Dir equal to the absolute working dir.
2. test_connect_mcp_http_headers_none_passthrough — same fake; call with headers=None and a placeholder-free URL; assert the fake received headers=None and the URL byte-for-byte unchanged (guards the if headers: branch and the no-{ fast path).
3. test_connect_mcp_http_does_not_mutate_caller_headers — pass a headers dict containing {agent_id}, then assert the caller's dict still contains the literal placeholder after the call (protects the _retry_failed_mcps respawn path, which re-uses the stored spec).
4. test_load_mcp_http_entry_expands_placeholders — integration-flavored: write an init.json/registry HTTP entry whose url contains {agent_id}, monkeypatch Agent.connect_mcp_http (pattern already used at tests/test_mcp_capability.py:402-414 for connect_mcp) or the fake HTTPMCPClient, boot the agent, and assert the expanded URL reached the client — proving the end-to-end registry path honors the contract.

Generated by a Claude Fable 5 deep review of the lingtai-kernel codebase.