Skip to content

feat(harness): run MCP servers per session and declare their tools - #80

Draft
sanchitrk wants to merge 6 commits into
claude/sleepy-gates-ce83d0from
claude/cod-114-stacked-pr-92a28d
Draft

sanchitrk wants to merge 6 commits into
claude/sleepy-gates-ce83d0from
claude/cod-114-stacked-pr-92a28d

Conversation

@sanchitrk

@sanchitrk sanchitrk commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Stacked on #79 (MCP client). Turns the client into something a session uses: servers from mcp.json, kept connected across a session's exchanges, with their tools declared to the model. The runtime is built on Effect scopes.

Config: mcp.json

  • Read next to settings.jsonc: the user layer (--user-config-dir or --home), then the project's .codework/mcp.json from the session's hostDir.
  • A project entry replaces a user entry of the same name, or overrides only enabled / exposure / toolExposure of it without restating it, so credentials stay in the user file.
  • A broken file or entry is reported and skipped; other servers still connect.
  • env and headers values resolve ${VAR} / $VAR and !command when the server connects.

Plugins: a ctx.plugin.mcp bucket

  • A new mcp plugin domain runs first: { mcp: 1, tool: 2, prompt: 3 }.
  • The bucket is seeded from mcp.json; mcp plugins can list / get / set / update / remove servers, checked with the same rules as the file.
  • It is settled before the tool domain runs. MCP tools are then in the tool bucket, so tool plugins can patch, override or hook them, and prompt plugins index them. Later domains can read the bucket but not write it.
  • Only configuration lives in the bucket; connection state never does.

Runtime: src/mcp/mcp.ts

  • Each session's servers connect in the background. An exchange waits up to 10 s for the servers it starts, and only for servers with direct tools.
  • Each connection lives in its own child scope; lifecycle operations are serialized per server, and startups to one URL are serialized.
  • Tool definitions are frozen per exchange; a call uses the server's current connection when it runs:
    • A connection that drops fails the calls running on it (reason and stderr tail); the next call reconnects first, even within the same exchange.
    • An expired HTTP session is replaced, and calls that hit it run once more on the new one; the old connection closes when its last call finishes.
    • A tool call is never retried otherwise.
    • A withdrawn tool says it is no longer available; a changed tool list takes effect at the next exchange.
  • A config change restarts only servers whose connection settings changed; exposure changes apply without reconnecting.

Tools

  • Named mcp__<server>__<tool> (non-[A-Za-z0-9_] → _, max 64), with a hash suffix on collisions or overflow. Names stay with their tool for the session.
  • Exposure is direct or hidden (per server, or per tool with patterns).
  • Tool.fromJsonSchema (in @codeworksh/plugin) sends the server's JSON Schema to the provider unchanged and validates arguments against its Effect translation. A schema the translation cannot express validates nothing. The server receives the arguments as the model wrote them.
  • isError results and failed calls reach the model as error results.

Other changes

  • JSONC parsing shared by settings and mcp.json (util/jsonc.ts).
  • stdio servers start in their configured cwd, resolved against the session's base directory, with a leading ~/ expanded in command, args and cwd.
  • The client's Connection gains dropped(), so a call notices a drop before the onClose handler has run.

Not in this PR (follow-ups)

  • Resources (resource tools) and truncation / spill-to-file of large and binary results.
  • space / app scopes, idle timeouts and leases. Until then, a session's servers live until the process exits.
  • OAuth sign-in; deferred / codemode exposure.

Review fixes (2026-10-15)

Rebased onto #79's latest head, so it includes onLog / McpLog, condensed validation errors and the cleanup fixes.

  • Idle exit deadlock. A server exiting with no call running retired its connection from inside the connection's own scope, which waited on itself and left the server locked for good. Connection callbacks now run in the service's scope.
  • Connect attempts run on their own. Callers share one attempt, and closing a server interrupts it instead of waiting behind it.
  • Restarted servers keep their tools. An exchange waits up to 10 s for every server it starts (new or restarted), not only on the first exchange.
  • Frozen configs. Server configs in ctx.plugin.mcp are copied and deep-frozen, so a plugin can't change a settled server.
  • Names. A hashed tool name grows until it's free, so a tool named like the hash can't replace another.
  • Logging. Server log messages go to <home>/mcp.log.
  • New test. mcp.session.test.ts gains a lifecycle test, with artifact mcp.session.lifecycle.json. It covers:
    • an idle exit, then a reconnect (it hangs without the deadlock fix)
    • an exposure-only change, which keeps the same process
    • an env change, which starts a new process
    • an HTTP session expiring once (retried on a new session) and twice in a row (the call fails)

Review fixes (2026-10-24)

  • Secrets in values: env and headers values with a line break or NUL are refused, naming the key but not the value.
  • !command: runs with stdin closed; its output is reused for the life of the process (failures run again next time).
  • auth / oauth: reported as not supported yet instead of being silently ignored.
  • Arguments: the nulls strict sampling writes for omitted optional arguments are dropped before the call; JSON Schema patterns are validated instead of turning validation off.
  • Lifecycle:
    • a changed tool list is fetched without holding the server's lock;
    • a server whose connect failed is retried at the next exchange, in the background;
    • removed or changed servers stop without making the prompt wait.
  • Names: a hashed tool name stays within 64 characters.
  • Tests: mcp.session.test.ts covers the null argument, a pattern rejection, an oauth entry and a header value with a newline (the secret appears nowhere).
  • Second pass (Codex):
    • an attempt that fails at once (an unset variable) no longer blocks every later retry;
    • connections close in parallel on shutdown, after connect attempts are interrupted;
    • overlapping list_changed refreshes keep only the latest listing;
    • a !command that ignores SIGTERM is killed, and concurrent connects share one run;
    • env values may span lines (a PEM key); headers still refuse line breaks;
    • extra arguments to a closed JSON Schema object are rejected instead of passed on.
  • Third pass: server log writes run on the service's fibers, so a failed handshake keeps the messages that explain it; a failed !command is forgotten as soon as it fails.

Test plan

  • test/mcp.session.test.ts: the full harness with a scripted model and a real stdio fixture server, two exchanges in one session. It covers:
    • Config: user and project layers, a project override, a name clash, an invalid entry, an override with no base.
    • The plugin bucket: an mcp-domain edit, and a tool-domain write being refused.
    • Tools: the JSON Schema passed through, a hidden tool left out, a sanitized name, structured content, an isError result, arguments rejected by validation.
    • A crash mid-exchange, then the next call in the same exchange reconnecting to a new process.
    • Ends with __artifacts__/mcp.session.json, stable across runs.
  • Full harness suite: 514 passed, 34 skipped (live/remote)
  • vp check (the only errors are in sites/www, which doesn't generate its Astro types in this checkout) and vp run typecheck

🤖 Generated with Claude Code

@sanchitrk

sanchitrk commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor Author

Review (2026-10-24)

should-fix: a server whose first connect fails never connects again in that session

  • Where: mcp.ts reconcile (around L389–399 / L439). It calls start() only for new servers or a changed key.
  • Why it sticks: a failed server keeps tools: []. No tool is declared, so no call ever reaches open() to reconnect.
  • Scenario:
    1. An HTTP server is down when the session starts, or a stdio server crashes on its first launch.
    2. Its tools are missing for the rest of the session. On the desktop/daemon process that can be days.
    3. Only logWarning records it.
  • Fix: in reconcile, call start(server) again for servers that are failed or disconnected with no live and no attempt, and add them to started so the 10 s wait covers them. feat(harness): share MCP servers by scope and close idle ones #83 makes this worse for shared space / app instances (see feat(harness): share MCP servers by scope and close idle ones #83).

should-fix: a list_changed refresh holds the server lock during a network call

  • Where: current() runs connection.tools() inside server.lock.withPermits(1). That call can take 60 s plus retries. open() and close() take the same lock.
  • Scenario: a server sends tools/list_changed and then answers tools/list slowly. Every tool call to it, and any reconcile that closes it, waits.
  • Fix: fetch outside the lock, then take the lock only to check server.live === live and assign server.tools.

should-fix: closing servers blocks the next prompt

  • Where: reconcile closes servers one at a time while holding session.lock.
  • What it costs: each close can take a 2 s stdin grace, a 2 s SIGTERM wait and a 1 s HTTP DELETE. Changing env on two servers that ignore stdin EOF delays the next prompt by about 8 s.
  • Fix: fork the old servers' teardown on the service's runtime. Their scope still guarantees the kill.

PR body

  • The summary still says "The first exchange waits up to 10 s". The code and the review-fixes section say every exchange waits for the servers it starts.
  • The body names a vendored reference project and a peer-review tool. Please drop both, since PR bodies are public.

Already handled later in the stack, so no action needed here: per-session server shutdown (#83's idle close), the codemode / deferred exposure values (accepted in #83), and hidden-only servers spawning a process (needed for #81's resources).

@sanchitrk
sanchitrk force-pushed the claude/sleepy-gates-ce83d0 branch from 84e21d0 to f64f0c1 Compare October 24, 2026 14:19
@sanchitrk
sanchitrk force-pushed the claude/cod-114-stacked-pr-92a28d branch 2 times, most recently from 403cdd6 to 94bb0b9 Compare October 24, 2026 15:41
sanchitrk and others added 6 commits October 24, 2026 22:22
Reads mcp.json from the user and project layers, keeps each session's
servers connected across exchanges, and declares their tools to the model.

- mcp.json: project entries replace user ones or override only enabled,
  exposure and toolExposure; broken files and entries are reported and
  skipped. env and headers resolve ${VAR} and !command values.
- Plugins get a ctx.plugin.mcp bucket in a new mcp domain that runs first;
  it is settled before the tool domain, so tool plugins see MCP tools.
- Tool definitions are frozen per exchange; each call uses the server's
  current connection. A dropped connection reconnects on the next call, an
  expired HTTP session is replaced and the call retried once, tool calls
  are never retried otherwise.
- Tools are named mcp__<server>__<tool> with a hash suffix on collisions;
  exposure is direct or hidden.
- Tool.fromJsonSchema sends a server's JSON Schema to the provider as
  written and validates arguments against its Effect translation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A server's own relative cwd now resolves against the session's base
directory, and a leading ~/ is expanded in command, args and cwd.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tled

- Connection callbacks run in the service's scope. A server exiting while
  idle used to retire its connection from inside the connection's own
  scope, which waited on itself and left the server locked for good.
- Connect attempts run on their own and are shared by callers; closing a
  server interrupts one in progress instead of waiting behind it.
- An exchange waits up to 10 s for every server it starts, not only on
  the first exchange, so a restarted server keeps its tools declared.
- Server configs in ctx.plugin.mcp are copied and deep-frozen, so a
  plugin cannot change a settled server afterwards.
- A hashed tool name grows until it is free, so a server tool named like
  the hash cannot replace another tool.
- Server log messages go to <home>/mcp.log.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Refuse env and header values with a line break or NUL without quoting them.
- Run a !command with stdin closed, and reuse its output for the life of the process.
- Report auth/oauth entries as unsupported instead of ignoring the keys.
- Drop the nulls strict sampling writes for omitted optional arguments before the call.
- Validate JSON Schema patterns instead of skipping the whole schema.
- Refresh a changed tool list without holding the server lock.
- Retry a server whose connect failed at the next exchange, in the background.
- Stop removed or changed servers without making the prompt wait.
- Keep hashed tool names within 64 characters.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aces

- Start a connect attempt after it is recorded, so one that fails at once (an unset
  variable, say) no longer blocks every later retry.
- Close server connections in parallel on shutdown, after interrupting connect attempts.
- Keep only the latest tool listing when list_changed notifications overlap.
- Write server log messages from the handshake on.
- Kill a !command that ignores SIGTERM, and share one run between concurrent connects.
- Allow line breaks in env values (a PEM key); headers still refuse them.
- Reject arguments a closed JSON Schema object does not declare, instead of passing them on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Write server log messages on the service's fibers, so a failed handshake closing the
  connection no longer drops the messages that explain it.
- Forget a failed !command as soon as it fails, so the next connect runs it again even
  when every caller waiting on it gave up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sanchitrk
sanchitrk force-pushed the claude/cod-114-stacked-pr-92a28d branch from 94bb0b9 to 4cd6ee1 Compare October 24, 2026 16:55

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant