Skip to content

fix(mcp): broadcast roots/list_changed when session cwd changes#831

Open
apoc wants to merge 1 commit intocan1357:mainfrom
apoc:fix/mcp-roots-list-changed-on-move
Open

fix(mcp): broadcast roots/list_changed when session cwd changes#831
apoc wants to merge 1 commit intocan1357:mainfrom
apoc:fix/mcp-roots-list-changed-on-move

Conversation

@apoc
Copy link
Copy Markdown
Contributor

@apoc apoc commented Apr 27, 2026

Problem

When the user changes the session's working directory via /move <path>, MCP servers connected to the session keep operating against the original repo root forever. Two reinforcing root causes:

  1. MCPManager.cwd is set at construction and never updated. Its roots/list handler answers from this.cwd, so connected servers see a stale path.
  2. The client declares roots.listChanged: false in the initialize handshake, which per MCP spec forfeits the ability to push notifications/roots/list_changed. Even if the manager's cwd were updated, there's no notification mechanism to tell servers to re-query.

Net effect: file-aware MCP servers (code search, indexers, navigation) never learn about a /move and continue indexing/searching the original directory.

Fix

  • MCPManager: add getCwd() / setCwd(newCwd). setCwd normalizes the path, no-ops on no-change, and broadcasts notifications/roots/list_changed to every connected server (best-effort fan-out; per-connection failures logged at debug, never abort the broadcast).
  • mcp/client: declare roots.listChanged: true in initialize capabilities (spec prerequisite for sending the notification). Extracted CLIENT_CAPABILITIES const so tests can pin the contract.
  • mcp/types: added ROOTS_LIST_CHANGED: "notifications/roots/list_changed" to MCPNotificationMethods. Annotated every entry with direction + intended client behaviour for clarity.
  • mcp/roots (new): shared buildRootsList(cwd) and notifyRootsChanged(connections) helpers. Both MCPManager.#getRoots and defaultRequestHandler (the probe-only fallback) now use the same builder so they cannot diverge.
  • pi-utils: extracted normalizeProjectPath helper. Both setProjectDir and MCPManager.setCwd now go through it, so they cannot drift on macOS /private/... paths (a single /move to /private/tmp/foo previously produced two different file:// URIs depending on which code path served roots/list).
  • /move handler: calls this.ctx.mcpManager?.setCwd(resolvedPath) immediately after setProjectDir.

Tests

17 cases across 3 files (all passing, full type-check clean):

  • test/mcp-roots.test.ts (12) — buildRootsList, notifyRootsChanged (fan-out, disconnected-skip, single-failure isolation), MCPManager.setCwd/getCwd (state, relative paths, no-op).
  • test/mcp-client-capabilities.test.ts (2) — pins CLIENT_CAPABILITIES.roots.listChanged === true.
  • test/modes/controllers/command-controller-move.test.ts (5) — wiring of /move -> mcpManager.setCwd, no-op when mcpManager undefined, no-op for invalid/missing path, no-op while streaming.

E2E with a real subprocess MCP server was prototyped but excluded from the commit as too heavy for CI.

Limitations / out of scope

These are pre-existing or deliberate; not regressions of this PR. Could be follow-ups:

  • Subagent isolation backends (fuse-overlay, fuse-projfs, worktree) reuse the parent MCPManager via proxy tools, so MCP servers still see the parent cwd, not the per-task isolation directory. The trade-off (fresh MCP connections per task vs. race-free shared state) is left as a known limitation.
  • setCwd no-op check is strict-equality on the resolved path. Equivalent paths via arbitrary symlinks (other than the /private/... macOS case handled by normalizeProjectPath) or case-insensitive filesystems could miss the no-op and produce a redundant broadcast. Acceptable since the broadcast is idempotent.

Review

Three review rounds (P0 wire-method spelling, P3 const centralization, P2 macOS path drift, P3 doc-comment symmetry) — all findings resolved. Final pass: APPROVE (confidence 0.9).

Files changed

File Lines
packages/coding-agent/src/mcp/manager.ts +22/-11
packages/coding-agent/src/mcp/client.ts +12/-11
packages/coding-agent/src/mcp/types.ts +7/-1
packages/coding-agent/src/mcp/roots.ts (new) +56
packages/coding-agent/src/modes/controllers/command-controller.ts +3/-0
packages/coding-agent/CHANGELOG.md +9/-0
packages/utils/src/dirs.ts +16/-2
3 test files (new) +338

When the user runs /move, MCP servers connected to the parent session
were not notified of the new working directory because:
  - The client declared roots.listChanged: false in the initialize handshake
  - MCPManager.cwd was set at construction and never updated

After /move, file-aware MCP servers continued to operate against the
original repo root instead of the moved-to directory.

Changes:
- pi-utils: extract normalizeProjectPath helper so setProjectDir and
  MCPManager.setCwd cannot drift on macOS /private/... paths
- MCPManager: add getCwd/setCwd; setCwd broadcasts
  notifications/roots/list_changed via best-effort fan-out
- mcp/client: declare roots.listChanged: true (capability prerequisite);
  expose CLIENT_CAPABILITIES const for test pinning
- mcp/types: add ROOTS_LIST_CHANGED to MCPNotificationMethods; annotate
  every entry with direction
- mcp/roots: new shared helpers buildRootsList, notifyRootsChanged used
  by both manager and client default request handler
- /move command: call mcpManager.setCwd after setProjectDir

Tests: 17 cases across roots helpers, capabilities, and command wiring.
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.

2 participants