Preserve unavailable MCP selections on unrelated admin saves

CHANGELOG.md

@@ -18,6 +18,9 @@ product overview.
 
 ## Unreleased
 
+- Preserve existing unavailable Cloud MCP selections when saving unrelated admin settings.
+  Removing a selection refreshes the channel picker and prevents later saves from restoring it.
+
 - Show enabled optional providers in the admin channel Runtime picker and preserve their saved
   engine/model selection when changing unrelated channel settings.
 

FEATURES.md

@@ -182,6 +182,10 @@
   operator definitions before saving, including stale cached discoveries. Previously stored
   selections remain removable; unrelated saves preserve them and runtime admission still drops
   unusable optional connectors with a notice while allowing the turn to continue.
+- The shared admin MCP picker round-trips the exact saved reference for an unavailable selection,
+  even when discovery redacts its transport metadata. Channel saves refresh the picker from the
+  committed record, so removing an unavailable grant disables re-addition and later unrelated saves
+  cannot restore it. New selections still require current backend admission.
 
 ## Drive sync status for unlinked channels
 

TEST-PLAN.md

@@ -447,6 +447,13 @@ must run applicable gates before stable promotion; no live pass is implied by au
 
 ## Claude Cloud MCP admission (2026-10-08)
 
+- [x] Browser regression: with `CG_BROWSER_MODULE` pointing to Playwright, run
+  `node --test test/mcp-legacy-selection-browser.test.js`. The actual admin form/router preserves
+  an existing credentialed MCP's exact saved reference when discovery returns a redacted unavailable
+  row and an unrelated setting is saved, with both Claude and Codex channel pins. Removal refreshes
+  the current picker, disables re-addition and survives another save; a forged new invalid selection
+  still returns 400 without changing grants. The test fails before the fix on the missing `match`
+  in the native form payload. This isolated browser regression does not complete the live gates below.
 - [x] Regression: `node --test test/claude-mcp-selection-admission.test.js
   test/mcp-discovery.test.js test/claude-selected-mcp.test.js test/channel-settings-modal.test.js
   test/access-grants.test.js test/folders-settings.test.js test/mcp-config.test.js

public/app.js

@@ -1715,7 +1715,14 @@ function selectedMcpEntries(box, engine) {
   captureMcpSelection(box);
   const selected = mcpBoxState(box)[engine];
   const available = catalogWithSavedEntries(box, engine);
-  return selected.map((id) => available.find((entry) => mcpEntryId(entry) === id)).filter(Boolean);
+  return selected.map((id) => {
+    const entry = available.find((candidate) => mcpEntryId(candidate) === id);
+    // Unavailable catalog rows redact transport references. Round-trip an existing grant
+    // verbatim rather than serializing that display-only stub and silently dropping it.
+    // New selections still use the catalog and every save is revalidated by the backend.
+    const original = box._mcpOriginals?.[engine]?.find((candidate) => mcpEntryId(candidate) === id);
+    return entry?.selectable === false && original ? original : entry;
+  }).filter(Boolean);
 }
 
 function wireGrantTierNavigation(select, currentTier) {
@@ -2407,6 +2414,9 @@ function renderChannelDetail(ch) {
       // The server response is the validated, committed record. Reconcile the cached channel from
       // that whole record so a later SPA re-render cannot resurrect stale MCP/skill selections.
       ch.meta = reconcileChannelMeta(ch.meta, result.meta);
+      initializeMcpBox(mcpsBox, { claude: ch.meta.allowedMcps || [], codex: ch.meta.allowedCodexMcps || [] });
+      if (!mcpsBox.dataset.mcpLoading) paintAllMcpBoxes(mcpsBox);
+      updateChecksCount(mcpsBox, mcpsCount);
       void refreshVpn();
       skillsPicker?.update({ selected: ch.meta.skills || [] });
       const acceptedGuests = channelGuestAcceptedIds(

test/channel-workdir-ui.test.js

@@ -56,11 +56,12 @@ function fixture({ workDir = "/home/operator/project", saveError } = {}) {
   const calls = [];
   const context = {
     card, ch, meta: ch.meta, Event, detailDirty: false, SELF_SAVING_CONTROLS: ".channel-env-card, .ch-vpn-controls",
-    engineSelect: { value: "codex" }, usersBox: { dataset: { ready: "" } }, mcpsBox: {}, skillsPicker: null,
+    engineSelect: { value: "codex" }, usersBox: { dataset: { ready: "" } }, mcpsBox: { dataset: {} }, mcpsCount: {}, skillsPicker: null,
     makeToolboxKeyInput: control(".ch-make-toolbox-key"),
     makeToolboxUrlInput: control(".ch-make-toolbox-url"), makeToolboxState: new Control(),
     clearMakeToolbox: false,
     tokenValue: () => "", selectedMcpEntries: () => [], checkedValues: () => [],
+    initializeMcpBox() {}, paintAllMcpBoxes() {}, updateChecksCount() {},
     explicitCheckedValues: () => [], channelGuestSavePatch: () => ({}),
     channelGuestAcceptedIds: () => null, reconcileChannelMeta,
     attachReveal() {}, revealSecret() {}, paintModePill() {}, renderConvList() {}, refreshVpn() {}, async refreshConversationRows() { return true; }, setTimeout() {},

test/mcp-legacy-selection-browser.test.js

@@ -0,0 +1,84 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import express from "express";
+import path from "node:path";
+import { writeFileSync } from "node:fs";
+import { fileURLToPath } from "node:url";
+import { ensureTestEnv } from "./helpers.js";
+
+ensureTestEnv();
+
+test("unrelated admin saves retain unavailable MCP selections and removals cannot restore them", { skip: !process.env.CG_BROWSER_MODULE }, async (t) => {
+  const { chromium } = await import(process.env.CG_BROWSER_MODULE);
+  const { createAdminRouter } = await import("../src/web/routes/admin.js");
+  const { saveSettings } = await import("../src/config/settings.js");
+  const { upsertChannelEntry, defaultChannelMeta, saveChannelMeta, getChannelMeta } = await import("../src/config/store.js");
+  const { catalogFromClaudeList } = await import("../src/gateway/mcp-discovery.js");
+  const original = { name: "credentialed", namespace: "mcp__credentialed", match: { serverName: "credentialed" } };
+  writeFileSync(path.join(process.env.CLAUDE_CONFIG_DIR, ".claude.json"), JSON.stringify({ mcpServers: {
+    credentialed: { command: "node", args: ["fixture-echo.mjs"], env: { TOKEN: "test-private-mcp-value" } },
+  } }));
+  const catalog = await catalogFromClaudeList("credentialed: node fixture-echo.mjs - ✓ Connected");
+  assert.equal(catalog[0].selectable, false);
+  assert.equal(catalog[0].match, undefined, "unavailable catalog intentionally redacts transport references");
+  saveSettings({ engineEnabled: { claude: true, codex: true }, engineFallback: false });
+  const app = express();
+  app.use(express.json());
+  app.get("/api/mcp/available", (req, res) => res.json({ servers: req.query.engine === "claude" ? catalog : [] }));
+  app.get("/api/health", (_req, res) => res.json({ slack: { connected: false }, engines: {} }));
+  app.use("/api", createAdminRouter({ slack: { getClient: () => null, snapshot: () => ({ connected: false }) } }));
+  const publicDir = fileURLToPath(new URL("../public", import.meta.url));
+  app.use(express.static(publicDir, { dotfiles: "allow" }));
+  app.get("/conversations/channel/:id", (_req, res) => res.sendFile(path.join(publicDir, "index.html"), { dotfiles: "allow" }));
+  const server = await new Promise((resolve) => { const s = app.listen(0, "127.0.0.1", () => resolve(s)); });
+  t.after(() => new Promise((resolve) => { server.closeAllConnections(); server.close(resolve); }));
+  const browser = await chromium.launch({ headless: true });
+  t.after(() => browser.close());
+  const page = await browser.newPage();
+  page.setDefaultTimeout(10000);
+  const errors = [];
+  page.on("pageerror", (error) => errors.push(error.message));
+  for (const engine of ["claude", "codex"]) {
+    const id = `C_MCP_LEGACY_${engine.toUpperCase()}`;
+    const entry = await upsertChannelEntry(id, { name: `mcp-legacy-${engine}`, type: "channel", isDM: false });
+    await saveChannelMeta(entry.slug, { ...defaultChannelMeta({ channelId: id, name: entry.name, type: "channel", isDM: false }), engine, allowedMcps: [original], allowedCodexMcps: [], allowNetwork: false });
+    await page.goto(`http://127.0.0.1:${server.address().port}/conversations/channel/${id}`);
+    await page.locator('[data-pane="mcps"].subtab').click();
+    const selected = page.locator('.ch-mcps input[data-mcp-engine="claude"][value="credentialed"]');
+    await selected.waitFor();
+    assert.equal(await selected.isChecked(), true);
+    assert.equal(await selected.isDisabled(), false, "legacy selection remains removable");
+    assert.ok(!(await page.locator(".ch-mcps").innerText()).includes("test-private-mcp-value"));
+    const save = async () => {
+      const pending = page.waitForResponse((r) => r.url().endsWith(`/channels/${id}/meta`) && r.request().method() === "PUT");
+      await page.locator(".save-channel").click();
+      const response = await pending;
+      assert.equal(response.status(), 200, await response.text());
+      await page.locator(".detail-savebar .msg").filter({ hasText: /^Saved$/ }).waitFor();
+      return response.request().postDataJSON();
+    };
+    await page.locator('[data-pane="runtime"].subtab').click();
+    await page.locator(".ch-memory").setChecked(!(await page.locator(".ch-memory").isChecked()));
+    const body = await save();
+    assert.deepEqual(body.allowedMcps, [original], `${engine} unrelated save round-trips the exact saved reference`);
+    assert.deepEqual((await getChannelMeta(entry.slug)).allowedMcps, [original]);
+    assert.equal((await getChannelMeta(entry.slug)).engine, engine);
+    assert.deepEqual((await getChannelMeta(entry.slug)).allowedCodexMcps, []);
+    await page.locator('[data-pane="mcps"].subtab').click();
+    await selected.uncheck();
+    await save();
+    assert.deepEqual((await getChannelMeta(entry.slug)).allowedMcps, []);
+    assert.equal(await selected.isDisabled(), true, "committed removal refreshes the original selection and disables unavailable re-add");
+    await page.locator('[data-pane="runtime"].subtab').click();
+    await page.locator(".ch-memory").setChecked(!(await page.locator(".ch-memory").isChecked()));
+    assert.deepEqual((await save()).allowedMcps, [], "later saves cannot resurrect removed originals");
+    const rejected = await page.evaluate(async ({ id, original }) => {
+      const r = await fetch(`/api/channels/${id}/meta`, { method: "PUT", headers: { "content-type": "application/json", "X-CG-Request": "1" }, body: JSON.stringify({ allowedMcps: [original] }) });
+      return { status: r.status, body: await r.json() };
+    }, { id, original });
+    assert.equal(rejected.status, 400, "backend still rejects a newly added unavailable reference");
+    assert.ok(!JSON.stringify(rejected).includes("test-private-mcp-value"));
+    assert.deepEqual((await getChannelMeta(entry.slug)).allowedMcps, []);
+  }
+  assert.deepEqual(errors, []);
+});