Repository navigation
feat(agent/x/agentmcp): load plugin mcp.json with per-entry isolation - #29325
jscottmiller wants to merge 1 commit into
Conversation
c054f33 to
6b3fb44
Compare
6b3fb44 to
f6be262
Compare
f6be262 to
e36194c
Compare
ad8f1a6 to
aa68b08
Compare
aa68b08 to
f5c9052
Compare
f5c9052 to
39a2fcc
Compare
39a2fcc to
be6460d
Compare
be6460d to
b6c86aa
Compare
b6c86aa to
eb6eaaf
Compare
3f83e08 to
04a9511
Compare
04a9511 to
3bdcb68
Compare
3bdcb68 to
7094c14
Compare
7094c14 to
539a9cd
Compare
539a9cd to
072ee10
Compare
072ee10 to
9ec22f8
Compare
9ec22f8 to
e31d950
Compare
2bf7309 to
eac2b1a
Compare
eac2b1a to
b7768cb
Compare
b7768cb to
9edd87d
Compare
9edd87d to
59ecf64
Compare
|
/coder-agents-review |
There was a problem hiding this comment.
This round reviews the plugin loader after the round 2 fixes and the new plugin header origin scoping. New findings: 5 P3, 16 P4, 8 Nit, and three earlier findings are re-raised on their threads.
Addressed: all other round 2 findings, including credential redaction in rejected rows, ${PLUGIN_DATA} subdirectory cwd, unknown-field rejection, legacy-first name claiming, header validation, and the legacy first-wins test. Re-raised: no Windows junction test for the symlinked-ancestor check, a lowercase plugin_root still overriding PLUGIN_ROOT on Windows when the agent environment already sets it, and the 89-column commit body line. The plugin rows sharing Name downstream remain deferred to #29326.
Out of scope (needs a ticket or explicit acceptance by a human):
agent/x/agentmcp/plugin.go:250:canonicalExistingduplicatesagent/agentcontext/plugin.gocanonicalExistingDir, and the two now differ (each has a check the other lacks), so plugin skills and plugin MCP servers enforce containment differently.agent/x/agentmcp/mcphttpclient.go:16: legacy.mcp.jsonheaders, such asAuthorization, are sent to every origin a server redirects to or names in an SSE endpoint event, and thehttpClientWithHeadersdoc does not say so.agent/x/agentmcp/manager.go:901: aListToolserror is copied into the catalog withoutredactErrorURLs; a transport error quoting a plugin URL with a query credential would reach coderd (unverified).agent/x/agentmcp/manager.go:213: theSetOnReloaddoc says "before the first Reload" and omitsReloadSources.agent/x/agentmcp/manager_internal_test.go:455: the literal-32601should bejsonrpc.CodeMethodNotFound.agent/x/agentmcp/config.go:131: the legacy entry decode error names themcpServerEntryGo struct.coderd/x/chatd/mcpclient/mcpclient.go:1602:isLoopbackHostis the same function the PR adds inplugin.go;agentcannot importcoderd, so the copies can diverge.- Earlier items remain:
agent/mcpcatalog.go:17andagent/agentcontext/mcp.go:16.
Notes:
- The round 2 note on plugin stdio servers receiving
CODER_AGENT_TOKENstill needs a human decision.
🤖 This review was automatically generated with Coder Agents.
ibetitsmike
left a comment
There was a problem hiding this comment.
Reviewed f76e6bf against its base 1fdd825 (#29324), with the same multi-agent setup as #29321: 10 reviewers (correctness, security, concurrency, tests, and design, each on two models) with throwaway probe tests and mutation runs at the exact head, then 2 adjudicators that re-ran the probe for every P1 to P3 finding and checked them against the 72 resolved bot threads and the later layers. Build, vet, go test -race of the package, and golangci-lint were clean at this head.
Overall: go with fixes. Per-entry isolation is careful, entry typing is thorough, and static containment held across every symlink layout the security reviewers tried. The problems are a fallback that crosses owners, the redirect policy, SSE sessions, and edge paths.
What I'd fix before merge:
- P2: a failed connect keeps the previous owner's client across sources, so a removed plugin's server keeps serving tool calls under another plugin's name (
manager.go, a three-line fix). - P2: plugin HTTP and SSE clients follow off-origin redirects and SSE endpoint events, including to plain
http, while the ticket says off-origin redirects are refused (mcphttpclient.go). - P2, predates this PR: every SSE session closes right after connect, so the new plugin
ssetype can't work (manager.go). - P2:
nullinsideargs,env, orheadersloads as"", although the description says nulls are rejected (plugin.go).
The P3 comments cover a FIFO or oversized mcp.json, IDNA in the origin check, lexical .., Windows command resolution, "cwd": "", and test gaps found by mutation. The P4, Obs, and Nit comments are optional.
Smaller notes, no inline comment:
- Connect and
ListToolserrors can carry the URL query into logs and catalog rows (manager.go#L686, #L902). That predates the PR for legacy servers, and plugin URLs are literal package data. - #29324 (outside this diff):
PluginDataDirhashes the install directory, so a plugin updated into a new directory gets an emptyPLUGIN_DATA, against spec 9.1 ("MUST preserve its contents across plugin updates"). Connect failures there still show a genericfailed to connectrow, although the ticket credits #29324 with reporting connect errors.
Not covered: nobody ran Windows, so the Windows comment rests on a standard-library trace. Later layers were checked for fixes and consumers, not re-reviewed.
Xum wrote this review on behalf of Mike.
| st := ServerStatus{Name: name} | ||
| catalog := make([]ServerStatus, 0, len(wanted)+len(rejected)) | ||
| for name, cfg := range wanted { | ||
| st := ServerStatus{Name: name, PluginName: pluginName(cfg.Plugin)} |
There was a problem hiding this comment.
P2: a failed connect keeps the previous owner's client across sources, so a removed plugin's server keeps running and serving tool calls under another plugin's name.
classifyServers keys entries by server name, and a changed PluginScope asks for a reconnect. When that connect fails, installServers keeps diff.prev[name] whatever its owner (manager.go#L745-L748, "Connect failed; retain the old client."). This line takes PluginName from the wanted config while the tools come from the retained client, and the new snapshot is installed, so SnapshotChanged is false and nothing retries.
Repro: plugin a declares a working x, and plugin b declares x with a missing command. After removing a, the catalog shows x from plugin b as connected with a's tools, and x__env runs in a's process (its cwd and PLUGIN_ROOT). Legacy-to-plugin and plugin-to-legacy behave the same.
The same-source fallback is CODAGT-1339 and accepted as is. What's new is keeping an uninstalled owner under another plugin's identity, now that the source is part of server identity. It needs a cross-source name collision plus a failing new owner, but collisions on common names such as github make that plausible. The code is unchanged in #29326 and #30577.
Fix: keep prev only when pluginScopeEqual(prev.config.Plugin, wantCfg.Plugin); otherwise append it to replaced so the caller closes it. With that change the repro passes, and so does the package under -race.
Test: the repro above, asserting that the (x, b) row is disconnected, m.servers["x"] is absent, and the old session is closed, plus one legacy-to-plugin case.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 012e51b: when a reconnect fails, the old client is kept only if its plugin scope (name, root, and data dir) is unchanged. Otherwise it is closed, including for a server moving to or from a legacy file, and the new row shows failed to connect. Covered by TestReloadSources_FailedConnectClosesClientFromOtherScope (another plugin, from legacy, to legacy, and the same plugin with a new data dir). The fallback for an unchanged scope stays as CODAGT-1339. (Coder Agents generated)
| base := mcpBaseTransport() | ||
| if len(headers) == 0 { | ||
| return &http.Client{Transport: base}, nil | ||
| } | ||
| return &http.Client{Transport: &sameOriginHeaderRoundTripper{ | ||
| base: base, | ||
| logger: logger, | ||
| origin: originOf(u), | ||
| headers: headers, | ||
| }}, nil |
There was a problem hiding this comment.
P2: plugin HTTP and SSE clients follow off-origin redirects and SSE endpoint events, including to plain http.
sameOriginHeaderClient sets no CheckRedirect. With no headers configured it returns a bare client with no origin check at all (line 82); otherwise the round tripper forwards off-origin requests and only withholds the headers. validatePluginURL checks only the configured URL. A 307 or 308, or an SSE endpoint event, can therefore move JSON-RPC bodies (tool-call arguments) to another origin, including plain http on a non-loopback host, which defeats the https rule after one hop. In probes, an initialize body arrived at http://<eth0-ip>, and a TLS-to-HTTP 307 resent a tools/call body, with and without configured headers.
The description says such a request "goes without them", so following is deliberate, but CODAGT-1101 says "off-origin redirects are refused", and CrossOriginRedirectFollowedWithoutHeader pins the follow behavior. The chatd MCP client already refuses them (CheckRedirect = safedial.CheckSameOriginRedirect).
Fix: for plugin clients, always wrap the transport and fail any off-origin request (that also covers SSE endpoints, for which the SDK has no hook), set CheckRedirect, and apply the https-or-loopback rule to every request. If following is the intended product choice, change the ticket criterion and the test name, but still close the https downgrade.
Test: a 307 to another port and an SSE endpoint event naming another port, each with and without headers; the connect fails and the other server records zero requests.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Pre-existing behavior, tracked as a follow-up. Following redirects and SSE endpoint events off-origin is main's behavior for legacy servers (httpClientWithHeaders sets no CheckRedirect). This PR keeps that for plugin servers and only withholds headers off-origin. Refusing off-origin redirects, including https to http, for every HTTP and SSE MCP server is tracked in CODAGT-1383. That ticket was split out after the CODAGT-1101 criterion was written, so the criterion is out of date. (Coder Agents generated)
| if cfg.Transport == "sse" { | ||
| return &mcp.SSEClientTransport{Endpoint: cfg.URL, HTTPClient: client}, nil |
There was a problem hiding this comment.
P2 (predates this PR): every SSE session closes as soon as connectServer returns, so the new plugin sse type can't work.
connectServer passes connectCtx, which it cancels on return, to c.Connect. go-sdk v1.8.0's SSEClientTransport.Connect opens the long-lived GET with that context (sse.go#L413) and closes the transport when the GET exits (#L469), so every SSE session, legacy or plugin, closes right after connect. Against mcp.NewSSEHandler, at this head and at the base, the first call fails with calling "tools/list": EOF, while an SDK client connected with a live context works against the same server. streamable-http is unaffected.
This PR adds plugin sse as a supported type, but its only SSE launch tests expect initialization to fail, so nothing runs a working SSE session.
Fix: run the SSE Connect on a cancelable child of ctx, enforce the handshake deadline with a timer that's stopped on success, and tie cancel to session close. Fix it here, or file a follow-up that blocks advertising plugin sse support.
Test: a successful plugin sse load followed by a tool call.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Pre-existing on main and tracked in CODAGT-1268. The connect-scoped context closes SSE sessions for legacy .mcp.json servers and for chatd's org-level SSE servers too. The ticket covers the fix for both paths and an end-to-end SSE test that calls a tool after connect. (Coder Agents generated)
| for _, key := range slices.Sorted(maps.Keys(fields)) { | ||
| if string(bytes.TrimSpace(fields[key])) == "null" { | ||
| return ServerConfig{}, xerrors.Errorf("server %q: field %q must not be null", name, key) | ||
| } | ||
| } |
There was a problem hiding this comment.
P2: null inside args, env, or headers loads as an empty string.
This check covers only top-level entry fields. Decoding into []string and map[string]string turns a null element or value into "", so "args":["x",null], "env":{"PATH":null}, and "headers":{"X-A":null} all load with no error. The description claims null rejection, and this is the unfinished part of the CRF-64 fix. The result is a changed argument list, or a blank PATH that fails at launch with a confusing error instead of an entry error.
Fix: decode as []*string and map[string]*string (or check the raw elements) and reject nil.
Test: three table rows, each expecting must not be null, while a valid sibling entry still loads.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 012e51b: args, env and headers decode into pointer types, so a null element or value is rejected (for example field "env": key "PATH" must not be null) while sibling entries still load. Three new rows in TestParseSource_PluginEntries. (Coder Agents generated)
| func readInRoot(root, rel string) ([]byte, error) { | ||
| r, err := os.OpenRoot(root) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer r.Close() | ||
| return r.ReadFile(rel) |
There was a problem hiding this comment.
P3: readInRoot has no regular-file check and no size cap, so a FIFO mcp.json stops all MCP for the agent.
os.Root.ReadFile blocks on a FIFO inside the reload singleflight, so every later reload joins the stuck one, and all MCP stops, legacy servers included. An 8 MiB mcp.json is accepted too. agentcontext's plugin.json reader checks both. Git can't carry a FIFO, but tar archives and setup scripts can, and the legacy .mcp.json reader on main has the same exposure, hence P3.
Fix: open non-blocking, stat the handle, reject non-regular files, and read through io.LimitReader(f, limit+1); report failures as the existing mcp.json not loaded row.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Pre-existing: the legacy .mcp.json reader on main has the same exposure, so this is tracked for both readers in CODAGT-1429: reject non-regular files, cap the size, and report the failure as the existing mcp.json not loaded row. (Coder Agents generated)
| assert.True(t, info.IsDir()) | ||
| if runtime.GOOS != "windows" { | ||
| // Windows does not map directory ACLs onto Unix permission bits. | ||
| assert.Equal(t, os.FileMode(0o700), info.Mode().Perm()) |
There was a problem hiding this comment.
Obs: this 0o700 assertion is vacuous under umask 077, where a 0o755 mkdir also ends up 0o700.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 012e51b: the test probes the umask with a 0755 directory and asserts 0700 only when the umask keeps a group or other bit; otherwise it logs that the mode is not checked. Under umask 022, a 0755 MkdirAll fails it. (Coder Agents generated)
| name: "HTTPTypeLiteralRejected", | ||
| entry: map[string]any{"type": "http", "url": "https://example.com/mcp"}, | ||
| errContains: `unsupported type "http"`, |
There was a problem hiding this comment.
Nit: asserting use "streamable-http" here would pin the hint branch; deleting case "http" keeps this test green today.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Done in 012e51b: HTTPTypeLiteralRejected now asserts unsupported type "http"; use "streamable-http", and HTTPTypeWithNullHeaderReportsTypeHint checks the hint still wins over a null header value. (Coder Agents generated)
| // buildPluginEnv sets PLUGIN_ROOT and PLUGIN_DATA after the entry's | ||
| // env, so they come last even where environment keys are matched | ||
| // case-insensitively and the last duplicate wins. |
There was a problem hiding this comment.
Nit: this says the plugin variables "come last", but mergeEnv replaces existing keys in place.
"are applied after the entry's env" would be accurate.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 012e51b: the comment now says PLUGIN_ROOT and PLUGIN_DATA are applied after the entry's env, replacing an inherited value with the same name. (Coder Agents generated)
|
|
||
| for key := range entry.Env { | ||
| if strings.EqualFold(key, pluginRootEnv) || strings.EqualFold(key, pluginDataEnv) { | ||
| return xerrors.Errorf("env may not set %s", key) |
There was a problem hiding this comment.
Nit: "env may not set" reads as permission; "must not" is clearer.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 012e51b: the error is now env must not set <name>. (Coder Agents generated)
| var args []string | ||
| if entry.Args != nil { | ||
| args = make([]string, 0, len(entry.Args)) | ||
| for _, arg := range entry.Args { | ||
| args = append(args, expandPluginPlaceholders(arg, scope)) | ||
| } | ||
| } | ||
| var env map[string]string | ||
| if entry.Env != nil { | ||
| env = make(map[string]string, len(entry.Env)) | ||
| for key, value := range entry.Env { | ||
| env[key] = expandPluginPlaceholders(value, scope) | ||
| } | ||
| } |
There was a problem hiding this comment.
Nit: fillPluginStdio copies args and env that it already owns; expanding in place removes eight lines.
Xum wrote this comment on behalf of Mike.
There was a problem hiding this comment.
Fixed in 012e51b as part of the null fix: fillPluginStdio expands env in place and no longer copies it. (Coder Agents generated)
|
Following up on the smaller notes in your review:
(Coder Agents generated) |
Teach agentmcp to load Agent Plugins mcp.json files alongside the
legacy .mcp.json files it already handles.
New input types: ConfigSource{Path, Plugin *PluginScope} and
PluginScope{Name, Root, DataDir}. Manager.ReloadSources accepts
sources, deduplicated by path with the first source winning;
Manager.Reload keeps its []string signature and wraps ReloadSources
through LegacySources. ParseSource replaces ParseConfig, which had no
other caller; its legacy branch is ParseConfig's body. The file
snapshot records the scope a path was read under, so a plugin rename
or data dir change with an untouched mcp.json still reloads.
ServerConfig gains Plugin and Cwd, both part of the identity
classifyServers compares, so a server moving between a legacy file
and a plugin is reconnected instead of kept. When that reconnect
fails, the old client is kept only if its plugin scope is unchanged;
another owner's client is closed. ServerStatus gains
PluginName so the catalog can carry a rejected plugin entry next to a
same-named legacy server.
Plugin files (plugin.go): Root and DataDir must be absolute, the source
path must be mcp.json at Root, and the file is read through os.Root, so
a symlink leading outside Root is refused. Root and DataDir are
symlink-resolved once, and those paths are used for commands, cwd, the
PLUGIN_ROOT and PLUGIN_DATA variables, and their placeholders; a DataDir
that cannot be resolved is used unresolved and fails at launch. Server
names must be non-empty. Invalid UTF-8 is replaced with U+FFFD, as
encoding/json does. The top level must be exactly $schema and mcpServers
with the 1.0.0 mcp schema URL, otherwise the file is rejected and the
plugin has no MCP servers; the error lists the keys found. Each entry is
validated independently: an invalid entry is logged and reported as a
disconnected ServerStatus with Err, and its siblings still load. Entries
require type in {stdio, streamable-http, sse}; "http" is not accepted,
and streamable-http is stored as the http transport. As the spec
requires, an entry with a field its type does not allow, including an
unknown field or one from another type, is invalid, as is a null
mcpServers, field, args element, or env or headers value, or a field of
the wrong type. stdio commands are a single token, either a bare name or
a ./-relative path that is resolved against the plugin root without
lexical cleaning, so a .. applies after the symlinks before it, then
symlink-resolved, containment-checked, must not be a directory, and
stored absolute. On Windows the ./ path is first resolved with
exec.LookPath, as os/exec does, so the file that runs is the one
checked, and a drive-relative bare name such as C:tool is rejected. cwd
must not be empty, and is ./-relative or is ${PLUGIN_ROOT} or
${PLUGIN_DATA}, alone or followed by /, and must resolve to a directory
inside the root, or inside DataDir for the last form, where it need not
exist. An entry env that sets PLUGIN_ROOT or PLUGIN_DATA is rejected, in
any letter case on Windows, where environment names are
case-insensitive, as is a malformed env key. A ./ cwd has its
placeholders expanded before it is resolved. streamable-http and sse
URLs must be absolute with a host name, https unless the host is
localhost or a loopback IP, and carry no userinfo or fragment. Headers
must be valid HTTP field names and values, and no two names may differ
only in case. Only ${PLUGIN_ROOT} and ${PLUGIN_DATA} are expanded in
args, env values, and cwd, in a single pass; plugin files never go
through os.Expand. When EvalSymlinks fails, which it does on Windows
substituted drives, canonicalization of the plugin root, commands, and
cwd falls back to the cleaned path only if the path has no .. component
and no component from the plugin root (or DataDir) down to the path is a
symlink or other reparse point.
Plugin streamable-http and sse servers add their configured headers only
to requests whose scheme, host, and port match the server URL, with
default ports normalized and non-ASCII hosts compared in their IDNA
ASCII form, as net/http dials them, and never replace a header the MCP
client sets or that net/http manages, such as Accept-Encoding and
hop-by-hop headers. A request to another origin, such as a redirect
target or an SSE message endpoint on another host, is sent without them,
and the first such request per connection is logged at warn level with
the server and request origins. A request host that is not valid IDNA
also gets no headers. A plugin server URL that cannot be parsed, or
whose host is not valid IDNA when headers are set, fails with a fixed
"invalid server url" error. Legacy .mcp.json servers keep sending their
headers to every origin.
Plugin stdio launch: DataDir, and a cwd inside it, are created 0700
before launch; cmd.Dir is the resolved cwd; the environment is the
same enriched environment a .mcp.json stdio server receives, plus the
entry env, with PLUGIN_ROOT and PLUGIN_DATA applied last.
A plugin mcp.json that fails whole-file validation is reported as a
rejected catalog entry named "mcp.json" and attributed to the plugin,
so the plugin's missing servers have a visible cause instead of only
an agent log line. URL validation errors do not quote the URL, so
rejected-entry errors in the catalog carry no URL. Legacy sources claim
server names before plugin sources, whatever the input order. A
plugin server that loses a name collision names the config that
claimed the name ("server name already in use by <path>" for a legacy
file, "by plugin <name>" for a plugin).
Legacy .mcp.json parsing and header handling are unchanged: a file
with any invalid entry is skipped as a whole, and duplicate names
across legacy files keep the silent first-wins behavior.
tl;dr: The agent's MCP engine can load an Agent Plugins
mcp.jsonalongside legacy.mcp.json, applying the spec's rules to plugin files.Reloads now take
ConfigSource{Path, Plugin *PluginScope}, and the file snapshot and reconnect identity include the plugin scope. Plugin files follow the Agent Plugins 1.0.0 specification: they must contain exactly$schemaandmcpServers, with entries of typestdio,streamable-http, orsse, and an entry with a field its type does not allow is invalid. stdio commands must be a bare name or a./-relative path inside the plugin, andcwdmust stay inside the plugin root or its data directory. Only${PLUGIN_ROOT}and${PLUGIN_DATA}are expanded, URLs must be https unless loopback, and headers must be valid and unique ignoring case. Pluginstreamable-httpandsseservers send their headers only to the server URL's origin, comparing non-ASCII hosts in their IDNA form, and never replace a header the MCP client sets; a request to another origin, such as a redirect or an SSE message endpoint on another host, goes without them, and the first one per connection is logged as a warning. Plugin stdio servers get the same environment as legacy servers plusPLUGIN_ROOTandPLUGIN_DATA. Choices this PR makes beyond the spec: rejecting the literal typehttpcomes with a hint to usestreamable-http; plugin headers that net/http manages, such asAccept-Encoding, are not sent; legacy.mcp.jsonservers claim names before plugin servers; URL validation errors do not quote the URL.Each plugin entry fails on its own: an invalid entry, including one with a
nullinsideargs,env, orheaders, is listed as a disconnected server with its error, and its siblings still load. A pluginmcp.jsonthat fails whole-file validation shows up as an error row attributed to the plugin, and a plugin server that loses a name collision names the config that claimed it. When a reconnect fails, the old client is kept only if its plugin scope is unchanged, so a server never keeps running under another owner's name.Legacy
.mcp.jsonbehavior is unchanged, including headers, which are still sent to every origin. A legacy file with any invalid entry is still skipped as a whole, and duplicate legacy names keep first-wins. Per-entry isolation for legacy files is tracked in CODAGT-1338.PR map
scott/agent-plugins/5-agentmcp-helpers..4d0a3443c7agent/x/agentmcp/plugin.go(+533/-0, new): parses a pluginmcp.json(exact top level, per-entry type and field rules, rejection of nulls, including insideargs,env, andheaders, and of wrong types) and enforces containment for commands and cwd (canonicalExisting,canonicalAllowMissing,hasLinkInside,requireInside). Paths are joined without cleaning so..applies after symlinks, and on Windows a./command is checked as the PATHEXT file that runs. It also validates URLs and headers, and createsDataDirbefore a stdio launch.agent/x/agentmcp/manager.go(+263/-112): reloads take[]ConfigSource. Legacy sources claim names before plugin sources, rejected plugin entries and failed files become catalog rows, and the snapshot and reconnect identity include the plugin scope. A failed reconnect keeps the old client only for the same plugin scope. Plugin stdio servers getPLUGIN_ROOTandPLUGIN_DATA, and plugin http/sse servers usesameOriginHeaderClient.Reconnectreloads the last sources with their plugin scopes.agent/x/agentmcp/mcphttpclient.go(+139/-4, concurrency):sameOriginHeaderClientsends headers only to the server origin, comparing non-ASCII hosts in IDNA form, skips headers the client or net/http set, and warns once (sync.Once) when it withholds them. LegacyhttpClientWithHeadersis unchanged.agent/x/agentmcp/config.go(+91/-6):ConfigSource,PluginScope,LegacySources, andParseSource, which requires<Root>/mcp.json, resolvesRootandDataDironce, and reads throughos.Root.agent/x/agentmcp/expand.go(+26/-0, new): single-pass${PLUGIN_ROOT}/${PLUGIN_DATA}expansion.This PR was generated by Coder Agents on behalf of @jscottmiller.