fix(mcp-sync): route tests never follow COPILOT_HOME, and docs name Copilot (#581)

The route test cleared only the registry CLIs' relocation vars, so with
COPILOT_HOME exported it wrote its fixture into that real Copilot config.
It now clears the sync-only tools' vars too, and Copilot's install probe goes
through the test's own installed set instead of the machine's PATH.

Docs: CLAUDE.md, the API reference and the Settings reference name COPILOT_HOME
and the sync-only table; a missing comma in docs/cli-registry.md; the
mcp-sync.ts overview and the Sync confirm mention Copilot.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

CLAUDE.md

@@ -248,7 +248,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
 
 ⚠️ **llama-swap endpoints** (one model at a time): the apply routes check `GET /running` and return `requiresConfirmation` before evicting a model another live session uses; `confirmedSwap` and `confirmedContext` are SEPARATE flags and must stay so. Claude alone gets a context floor (`CLAUDE_MIN_SAFE_CONTEXT_TOKENS`); context is parsed from `/running`'s `cmd`, never trusted from `/props`. Backend log lines come from llama-swap's `/api/events` `upstream` source, never `/logs`. → [architecture-invariants#custom-model-endpoint-profiles](docs/architecture-invariants.md#custom-model-endpoint-profiles)
 
-**MCP server sync** (opt-in, `mcpSyncEnabled`, SYNCED, default OFF; `src/mcp-sync.ts`, `GET`/`POST /api/mcp-sync`): copies each installed, enabled CLI's user-level MCP servers into the others. It is the ONE subsystem that writes another CLI's REAL user config (`~/.claude.json`, `~/.codex/config.toml`, `~/.gemini/*`, opencode's), which is why it is opt-in and admin-only in multi-user mode (both verbs 403 for a non-admin, and the Settings group is hidden for them). Where each CLI keeps the file is registry data, `capabilities.mcpConfig` (`{ path, format, relocation? }`), never a branch on the id. ⚠️ ADDITIVE only: a name already defined, in any shape, is never edited or removed (a different same-name definition is a reported conflict), and a server switched off in its own CLI is never copied. ⚠️ Never write a file that did not parse; re-parse the NEW text and require every added server to read back before the tmp+rename (written through a symlink, previous file kept as `<file>.codeman-bak`, one apply at a time, else 409). ⚠️ A file that receives copied `env`/`headers` (secrets) is left `0600`, and so is the backup. ⚠️ Responses carry server NAMES only, never env values, headers or file text: a parse failure is reported by line and column (`describeMcpSyncError`), never the parser's own message (smol-toml and V8 both quote source). ⚠️ `mcpConfig.relocation` names the env var the CLI reads to move its file (`CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `XDG_CONFIG_HOME`, `GEMINI_CLI_HOME`), resolved from the SERVER env at call time; a relative value reports the target `skipped`, never a guessed write, and a per-session `envOverrides` relocation is not followed. Tests must pass `home` (which drops the `process.env` default) or clear those vars first. → `docs/cli-registry.md` (MCP server sync), `docs/api-reference.md`, `docs/wiki/Settings-Reference.md`
+**MCP server sync** (opt-in, `mcpSyncEnabled`, SYNCED, default OFF; `src/mcp-sync.ts`, `GET`/`POST /api/mcp-sync`): copies each installed, enabled CLI's user-level MCP servers into the others. It is the ONE subsystem that writes another CLI's REAL user config (`~/.claude.json`, `~/.codex/config.toml`, `~/.gemini/*`, opencode's), which is why it is opt-in and admin-only in multi-user mode (both verbs 403 for a non-admin, and the Settings group is hidden for them). Where each CLI keeps the file is registry data, `capabilities.mcpConfig` (`{ path, format, relocation? }`), never a branch on the id. ⚠️ Tools that sync but are not run modes (GitHub Copilot CLI, the first) are plain data in `MCP_SYNC_ONLY_TOOLS` (`src/mcp-sync-targets.ts`), take part only when installed or already configured, and their companion `settings.json` (`disabledMcpServers`) is read, never written; a test must clear their relocation vars too. ⚠️ ADDITIVE only: a name already defined, in any shape, is never edited or removed (a different same-name definition is a reported conflict), and a server switched off in its own CLI is never copied. ⚠️ Never write a file that did not parse; re-parse the NEW text and require every added server to read back before the tmp+rename (written through a symlink, previous file kept as `<file>.codeman-bak`, one apply at a time, else 409). ⚠️ A file that receives copied `env`/`headers` (secrets) is left `0600`, and so is the backup. ⚠️ Responses carry server NAMES only, never env values, headers or file text: a parse failure is reported by line and column (`describeMcpSyncError`), never the parser's own message (smol-toml and V8 both quote source). ⚠️ `mcpConfig.relocation` names the env var the CLI reads to move its file (`CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `XDG_CONFIG_HOME`, `GEMINI_CLI_HOME`, and `COPILOT_HOME` for Copilot), resolved from the SERVER env at call time; a relative value reports the target `skipped`, never a guessed write, and a per-session `envOverrides` relocation is not followed. Tests must pass `home` (which drops the `process.env` default) or clear those vars first. → `docs/cli-registry.md` (MCP server sync), `docs/api-reference.md`, `docs/wiki/Settings-Reference.md`
 
 **Run launch synchronization**: the Run entrypoint holds an in-flight lock and disables `#runBtn` for the whole launch (≥500ms) so a double click cannot create duplicate `w<n>-<case>` sessions; `_ensureCreatedSessionVisible()` runs before `selectSession()` and `_onSessionCreated()` stays an idempotent upsert, so POST-first and SSE-first both render exactly one tab. ⚠️ **Closing has the mirror-image race**: `closeSession()` must read `wasActive` BEFORE its `await` and announce the delete via `_closingSessions`, and `_onSessionDeleted` skips the active-session handoff for ids in that set; never read `activeSessionId` after the fact. The fallback picks the first `sessionOrder` entry still in `sessions`. Tests: `test/session-close-fallback.test.ts`. → [architecture-invariants#run-launch-synchronization](docs/architecture-invariants.md#run-launch-synchronization)
 

docs/api-reference.md

@@ -836,7 +836,7 @@ Result (`data`):
 - `targets[]` — one per enabled CLI that declares an MCP config, plus GitHub Copilot CLI (`id: "copilot"`, a sync-only target that is not a run mode): `id`, `label`, `file`, `status`, `error?`, `servers` (names it already has), `added` (names added, or that would be), `skipped` (names its dialect cannot express, e.g. SSE for Codex and Antigravity).
   - `status`: `ok`; `absent` (not installed and no config file, so not read or created); `skipped` (the CLI's relocation env var, e.g. `CODEX_HOME`, is set to a relative path in the server's environment, so its file cannot be located safely and is neither read nor written); `unreadable` (the file exists but cannot be parsed safely, so it is not written); `failed` (a read or write error, the file may be unchanged).
   - `error` says why a target is not `ok`. A parse failure is reported by position only (`not valid TOML (line 3, column 21)`, `not valid JSON`), never with text from the file.
-  - `file` honours each CLI's own relocation env var as the server process sees it (`CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `XDG_CONFIG_HOME`, `GEMINI_CLI_HOME`); see `docs/cli-registry.md`.
+  - `file` honours each CLI's own relocation env var as the server process sees it (`CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `XDG_CONFIG_HOME`, `GEMINI_CLI_HOME`, and `COPILOT_HOME` for the sync-only Copilot CLI); see `docs/cli-registry.md`.
 - `conflicts[]` — names defined differently by different CLIs. Existing definitions are kept; the first CLI's is copied where the name is missing.
 - `disabled[]` — names left out because every definition is switched off in its own CLI (codex `enabled = false`, opencode `enabled: false`, antigravity `disabled: true`).
 - `unsupported[]` — labels of enabled agent CLIs with no known MCP config file (nothing is guessed).

docs/cli-registry.md

@@ -269,7 +269,7 @@ A module-level const freezes at first import, and the failure is asymmetric: a C
 
 **Tools that are not run modes.** GitHub Copilot CLI keeps an MCP list worth syncing but Codeman does not launch it, so it has no registry entry. `src/mcp-sync-targets.ts` declares such tools as plain data (`MCP_SYNC_ONLY_TOOLS`: id, label, config path, dialect, relocation var, the binary whose presence means "installed"). They join the registry CLIs as sync targets (listed after them, so a registry CLI's definition wins a same-name difference), under the same rules: installed or already configured, otherwise `absent`. Copilot's dialect is `copilot-json` (`~/.copilot/mcp-config.json`, relocated by `COPILOT_HOME`; checked against `copilot mcp add` 1.0.94): `mcpServers`, each entry with `tools` (`["*"]` = all), `type` `local` | `http` | `sse`, `command`/`args`/`env` or `url`/`headers`. `copilot mcp disable` does not mark the entry: it lists the name under `disabledMcpServers` in `settings.json` beside the config. Sync reads that list (never writes it) so a disabled server is not copied, and reports the target `unreadable` if `settings.json` is not valid JSON rather than guessing.
 
-`relocation` (`{ envVar, path }`) names the env var the CLI itself reads to move that file: claude `CLAUDE_CONFIG_DIR` (`.claude.json` under it), codex `CODEX_HOME` (`config.toml`), opencode `XDG_CONFIG_HOME` (`opencode/opencode.json`) gemini `GEMINI_CLI_HOME` (`.gemini/settings.json`) and, for the sync-only Copilot CLI, `COPILOT_HOME` (`mcp-config.json`); antigravity follows `$HOME` only, so it declares none. The var is read from the SERVER process env at call time, which is the env the CLIs Codeman spawns inherit. An absolute value moves the file to `<value>/<relocation.path>`, an empty one counts as unset (as it does for each CLI), and anything else reports the target `skipped` with the reason instead of writing a file the CLI never reads. A per-session relocation (a session's own `CLAUDE_CONFIG_DIR` in `envOverrides`) is not followed: the sync only knows the server's environment.
+`relocation` (`{ envVar, path }`) names the env var the CLI itself reads to move that file: claude `CLAUDE_CONFIG_DIR` (`.claude.json` under it), codex `CODEX_HOME` (`config.toml`), opencode `XDG_CONFIG_HOME` (`opencode/opencode.json`), gemini `GEMINI_CLI_HOME` (`.gemini/settings.json`) and, for the sync-only Copilot CLI, `COPILOT_HOME` (`mcp-config.json`); antigravity follows `$HOME` only, so it declares none. The var is read from the SERVER process env at call time, which is the env the CLIs Codeman spawns inherit. An absolute value moves the file to `<value>/<relocation.path>`, an empty one counts as unset (as it does for each CLI), and anything else reports the target `skipped` with the reason instead of writing a file the CLI never reads. A per-session relocation (a session's own `CLAUDE_CONFIG_DIR` in `envOverrides`) is not followed: the sync only knows the server's environment.
 
 The rules the module keeps and the tests pin: it only ADDS (a name already defined, in any shape, is never edited or removed; a same-name difference is reported as a conflict); a server switched off in its own CLI is not copied; it never writes a file it could not parse (opencode JSONC with comments, a TOML file with a duplicate table) and re-parses the new text before writing; codex TOML is read with a real parser (`smol-toml`), so CRLF files and inline tables are handled; names such as `__proto__` are ignored and every table keyed by an untrusted name has no prototype; a symlinked config is written through, not replaced; a file that receives `env`/`headers` is left `0600`; only one apply runs at a time; and its result carries server names only, never env values or headers, and never file text: a parse failure is reported by line and column, not by the parser's message (smol-toml prints a code frame of the offending lines and V8's JSON errors quote source, either of which can hold a secret). The schema restricts `path` and `relocation.path` to a relative path without `..`, since sync writes to it.
 

src/mcp-sync.ts

@@ -12,7 +12,8 @@
  *     does not understand) is never rewritten and nothing is ever removed. Same name with a
  *     different definition is reported as a conflict and left alone.
  *   - A server the user has switched off in its own CLI (codex `enabled = false`, opencode
- *     `enabled: false`, antigravity `disabled: true`) is not propagated: copying it would
+ *     `enabled: false`, antigravity `disabled: true`, Copilot's `disabledMcpServers` in its
+ *     `settings.json`) is not propagated: copying it would
  *     switch it on in every other CLI.
  *   - A file that does not parse (e.g. opencode JSONC with comments, a TOML file with a
  *     duplicate table) is never written, and a write is only made after the NEW text has been

src/web/public/settings-ui.js

@@ -1235,7 +1235,7 @@ Object.assign(CodemanApp.prototype, {
       show('Apply or Save settings to turn MCP sync on first, then preview or sync.', 'save-first');
       return;
     }
-    if (apply && !confirm('Add missing MCP servers to every installed, enabled CLI\'s config file? Env values and headers on those servers are copied too.')) return;
+    if (apply && !confirm('Add missing MCP servers to every installed, enabled CLI\'s config file (and GitHub Copilot CLI\'s, when it is installed)? Env values and headers on those servers are copied too.')) return;
     show('Working…');
     const res = apply ? await this._apiPost('/api/mcp-sync', {}) : await this._api('/api/mcp-sync');
     let body = null;

test/routes/mcp-sync-routes.test.ts

@@ -16,6 +16,7 @@ import { registerMcpSyncRoutes } from '../../src/web/routes/mcp-sync-routes.js';
 import { SETTINGS_PATH } from '../../src/web/route-helpers.js';
 import { registryFilePath, reloadCliRegistry } from '../../src/config/cli-registry/registry.js';
 import { STOCK_CLIS } from '../../src/config/cli-registry/stock.js';
+import { MCP_SYNC_ONLY_TOOLS } from '../../src/mcp-sync-targets.js';
 
 // Which CLIs are installed on the machine running the tests must not decide the outcome: nothing
 // is installed, so only a CLI whose config file exists takes part.
@@ -43,14 +44,26 @@ vi.mock('../../src/utils/cli-installed-probes.js', () => ({
   probeStockCliAvailability: async () => ({}),
   isCliEntryInstalled: (e: { id: string }) => installed.has(e.id),
 }));
+// The sync-only tools (Copilot) probe the real PATH for their binary; route that through the same set.
+vi.mock('../../src/mcp-sync-targets.js', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('../../src/mcp-sync-targets.js')>();
+  return {
+    ...actual,
+    mcpSyncOnlyTargets: (taken: ReadonlySet<string>) =>
+      actual.mcpSyncOnlyTargets(taken, (binary) => installed.has(binary)),
+  };
+});
 
 // The route follows each CLI's relocation env var (CODEX_HOME, CLAUDE_CONFIG_DIR, XDG_CONFIG_HOME,
 // ...) from process.env, so the runner's own values (CI images set XDG_CONFIG_HOME) must never
 // aim a test write outside the temp HOME. Cleared before every test, restored after the file.
-const RELOCATION_VARS = STOCK_CLIS.flatMap((e) => {
-  const envVar = e.capabilities.mcpConfig?.relocation?.envVar;
-  return envVar ? [envVar] : [];
-});
+const RELOCATION_VARS = [
+  ...STOCK_CLIS.flatMap((e) => {
+    const envVar = e.capabilities.mcpConfig?.relocation?.envVar;
+    return envVar ? [envVar] : [];
+  }),
+  ...MCP_SYNC_ONLY_TOOLS.flatMap((t) => (t.relocation ? [t.relocation.envVar] : [])),
+];
 const savedEnv = Object.fromEntries(RELOCATION_VARS.map((k) => [k, process.env[k]]));
 afterAll(() => {
   for (const [k, v] of Object.entries(savedEnv)) {