fix(webui): reap the mcode acp process group and group models by provider

packages/webui/acp.mjs

@@ -93,6 +93,10 @@ export class McodeAcpClient extends EventEmitter {
       stdio: ['pipe', 'pipe', 'pipe'],
       windowsHide: true,
       shell: false,
+      // POSIX 下让子进程当进程组长 —— stop() 才能杀整组。引擎的 acp
+      // 入口会自己拉起 MCP 插件链 (proxy → mcp-server → …)，这些孙进程收不到
+      // 针对直接子进程的信号，于是每次重启都留下一棵孤儿插件树。
+      detached: process.platform !== 'win32',
     })
     // Node 24 does NOT emit 'exit' on spawn failure (ENOENT when mcode is
     // not installed) — only 'error' + 'close'. If pending requests were
@@ -323,7 +327,20 @@ export class McodeAcpClient extends EventEmitter {
 
   stop() {
     if (this.child) {
-      try { this.child.kill() } catch {}
+      const pid = this.child.pid
+      if (process.platform !== 'win32' && pid) {
+        // 进程组 SIGTERM → 3s 兜底 SIGKILL (整组, 含 MCP 插件链)。
+        // detached:true (见 start()) 让 pid 即进程组 id, 取负号即整组。
+        try { process.kill(-pid, 'SIGTERM') } catch { try { this.child.kill('SIGTERM') } catch {} }
+        const c = this.child
+        const t = setTimeout(() => {
+          try { process.kill(-pid, 'SIGKILL') } catch {}
+          try { c.kill('SIGKILL') } catch {}
+        }, 3000)
+        if (typeof t.unref === 'function') t.unref()
+      } else {
+        try { this.child.kill() } catch {}
+      }
       this.child = null
     }
     this.started = false

packages/webui/server/lib/acp-client.js

@@ -21,6 +21,13 @@ let _mcodeAcpInitPromise = null; // 防止并发 init 同一个 client
 export async function getMcodeAcpClient() {
   if (_mcodeAcpSingleton && _mcodeAcpSingleton.alive) return _mcodeAcpSingleton;
   if (_mcodeAcpInitPromise) return _mcodeAcpInitPromise;
+  // 覆盖引用前先 stop 掉旧实例: `alive` 判定为假的 client 仍可能持有子进程
+  // (例如 child 已置空但 _alive 未同步, 或子进程仍在收尾), 直接覆盖引用会把它
+  // 和它的 MCP 插件树一起漏掉。与下面 init 失败路径的 stop-before-null 同理。
+  if (_mcodeAcpSingleton && !_mcodeAcpSingleton.alive) {
+    try { _mcodeAcpSingleton.stop(); } catch {}
+    _mcodeAcpSingleton = null;
+  }
   _mcodeAcpInitPromise = (async () => {
     const client = new McodeAcpClient({ debug: false });
     try {

packages/webui/server/routes/model.js

@@ -1,6 +1,9 @@
 // webui/server/routes/model.js
 // GET /api/models, POST /api/set-model, POST /api/permissions, POST /api/answer (legacy)
 
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+
 import { pushStateFor } from "../lib/state-bus.js";
 import {
   mcodePermissionToWebui,
@@ -20,6 +23,78 @@ function configOption(cs, id) {
 import { webuiModeToLabel } from "../lib/interaction/permission-presets.js";
 import { readJson } from "../lib/read-json.js";
 
+/**
+ * Optional model-catalogue overlay, re-read on every request so editing the
+ * file takes effect without a restart.
+ *
+ * Path: `MCODE_WEBUI_MODELS_CONFIG`, else `models.json` under the server's cwd.
+ * Shape: `{ providers: [{ id, label, models: [{ id, label, contextLimit? }] }] }`.
+ *
+ * This is an *overlay*, not a replacement. The engine's session config option
+ * stays the authority on which models exist and which one is current — an
+ * overlay entry that the engine does not list is not offered, and an engine
+ * entry the overlay does not mention is still offered. The overlay only
+ * supplies display metadata (label, contextLimit) and lets an operator name
+ * the provider groups. Reading a missing or malformed file is not an error;
+ * it just means no overlay.
+ */
+function readModelsOverlay() {
+  const path =
+    process.env.MCODE_WEBUI_MODELS_CONFIG || join(process.cwd(), "models.json");
+  try {
+    const parsed = JSON.parse(readFileSync(path, "utf8"));
+    if (!parsed || !Array.isArray(parsed.providers)) return null;
+    return parsed;
+  } catch {
+    return null;
+  }
+}
+
+/**
+ * Provider for an engine model id.
+ *
+ * The engine encodes a selection as `m:<provider>:<model>:v:<variant>`, so the
+ * provider is the second colon-delimited field. Ids that are not in that
+ * encoding fall back to the `provider/` prefix form, and anything else reports
+ * an empty provider (the caller renders those as one unnamed group).
+ */
+function providerOf(modelId) {
+  if (typeof modelId !== "string") return "";
+  const encoded = /^m:([^:]+):/.exec(modelId);
+  if (encoded) return encoded[1];
+  const slash = /^([^/]+)\//.exec(modelId);
+  return slash ? slash[1] : "";
+}
+
+/** Model name within an engine id: `m:<provider>:<model>:v:<variant>` → `<model>`. */
+function bareNameOf(modelId) {
+  if (typeof modelId !== "string") return "";
+  const encoded = /^m:[^:]+:([^:]+):/.exec(modelId);
+  if (encoded) return encoded[1];
+  const slash = /^[^/]+\/(.+)$/.exec(modelId);
+  return slash ? slash[1] : modelId;
+}
+
+/**
+ * Index the overlay by provider, then by the model ids it can supply metadata
+ * for. A model may be written bare (`MiniMax-M3`) or fully qualified
+ * (`minimax_api/MiniMax-M3`); both resolve to the same entry.
+ */
+function indexOverlay(overlay) {
+  const byProvider = new Map();
+  for (const p of overlay.providers) {
+    if (!p || typeof p.id !== "string" || !p.id) continue;
+    const byModel = new Map();
+    for (const m of Array.isArray(p.models) ? p.models : []) {
+      if (!m || typeof m.id !== "string" || !m.id) continue;
+      byModel.set(m.id, m);
+      const bare = m.id.includes("/") ? m.id.slice(m.id.indexOf("/") + 1) : m.id;
+      byModel.set(bare, m);
+    }
+    byProvider.set(p.id, { label: typeof p.label === "string" && p.label ? p.label : p.id, byModel });
+  }
+  return byProvider;
+}
 
 // GET /api/models
 // The catalogue is the engine's `model` config option (the same list the TUI's
@@ -34,21 +109,51 @@ import { readJson } from "../lib/read-json.js";
 // answers `null` now, and the caller shows a neutral label. Nothing is written
 // back into `cs.model` either — that backfill is what put the invented name into
 // the state a later prompt would use.
+//
+// `groups` partitions that same catalogue by provider so a client can render
+// one section per provider instead of one flat list. It is derived from the
+// engine ids, so it is always consistent with `models`; the optional
+// `models.json` overlay may rename a group and enrich an entry.
 export function handleGetModels(_req, res, ctx) {
   const cs = ctx.cs;
   const option = configOption(cs, "model");
-  const models = (option && Array.isArray(option.options) ? option.options : []).map((o) => ({
-    id: o.value,
-    name: o.name,
-  }));
+  const overlay = readModelsOverlay();
+  const overlayIndex = overlay ? indexOverlay(overlay) : null;
+
+  const groups = new Map();
+  const models = (option && Array.isArray(option.options) ? option.options : []).map((o) => {
+    const provider = providerOf(o.value);
+    const providerEntry = overlayIndex?.get(provider);
+    const overlayModel = providerEntry?.byModel.get(bareNameOf(o.value))
+      ?? providerEntry?.byModel.get(o.value);
+    const entry = {
+      id: o.value,
+      name: o.name,
+      // `label` is the overlay's display name when it has one, else the
+      // engine's. Both spellings ship so older clients reading `name` keep
+      // working and newer ones can prefer `label`.
+      label: typeof overlayModel?.label === "string" && overlayModel.label ? overlayModel.label : o.name,
+      provider,
+    };
+    if (typeof overlayModel?.contextLimit === "number" && overlayModel.contextLimit > 0) {
+      entry.contextLimit = overlayModel.contextLimit;
+    }
+    if (!groups.has(provider)) {
+      groups.set(provider, { id: provider, label: providerEntry?.label ?? provider, models: [] });
+    }
+    groups.get(provider).models.push(entry);
+    return entry;
+  });
+
   const current = (option && option.currentValue) || null;
   res.writeHead(200, { "Content-Type": "application/json; charset=utf-8" });
   return res.end(
     JSON.stringify({
       ok: true,
       models,
+      groups: [...groups.values()],
       current,
-      source: "acp-session-config",
+      source: overlayIndex ? "acp-session-config+overlay" : "acp-session-config",
       // listModels is per-session, so there is nothing to report until the
       // engine has created one.
       ...(models.length === 0 ? { reason: "no_session_config" } : {}),

packages/webui/test/lib/acp-process-group.test.js

@@ -0,0 +1,129 @@
+// webui/test/lib/acp-process-group.test.js
+// Regression test: stopping the mcode acp child must take down the whole
+// process group, not just the direct child.
+//
+// The bug: `stop()` called `child.kill()`, which signals only the direct child.
+// The engine's acp entry spawns its own plugin chain (codex-mcp-proxy →
+// codex mcp-server → …); none of those receive the signal, so every restart
+// orphans a full plugin tree. This test reproduces exactly that shape — a
+// direct child with one grandchild — and asserts both die.
+//
+// It fails on the old `child.kill()`-only implementation (the grandchild
+// survives) and passes once the child is spawned `detached` and `stop()` kills
+// the group, escalating to SIGKILL if the group ignores SIGTERM.
+//
+// POSIX-only: Windows has no process groups to signal, and `stop()` there keeps
+// the direct-child kill.
+
+import { test, describe, before, after } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, writeFileSync, readFileSync, existsSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+const posix = process.platform !== "win32";
+
+/** A stand-in for the engine's acp entry: spawns one long-lived grandchild. */
+const FAKE_ACP = `
+// Spawns a grandchild that never exits on its own — the leak this test
+// guards against. Both pids are published so the test can check on them.
+const { spawn } = require("node:child_process");
+const { writeFileSync } = require("node:fs");
+
+const grandchild = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000);"], {
+  stdio: "ignore",
+});
+writeFileSync(process.env.PID_FILE, JSON.stringify({ child: process.pid, grandchild: grandchild.pid }));
+
+// Answer the initialize handshake so McodeAcpClient.start() can resolve.
+let buf = "";
+process.stdin.setEncoding("utf8");
+process.stdin.on("data", (chunk) => {
+  buf += chunk;
+  let i;
+  while ((i = buf.indexOf("\\n")) >= 0) {
+    const line = buf.slice(0, i).trim();
+    buf = buf.slice(i + 1);
+    if (!line) continue;
+    let msg;
+    try { msg = JSON.parse(line); } catch { continue; }
+    if (msg && msg.id !== undefined) {
+      process.stdout.write(JSON.stringify({ jsonrpc: "2.0", id: msg.id, result: {} }) + "\\n");
+    }
+  }
+});
+process.stdin.resume();
+`;
+
+function isAlive(pid) {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch (e) {
+    // EPERM means the process exists but is not ours to signal.
+    return e.code === "EPERM";
+  }
+}
+
+async function waitFor(predicate, { timeoutMs = 5000, stepMs = 50 } = {}) {
+  const deadline = Date.now() + timeoutMs;
+  while (Date.now() < deadline) {
+    if (await predicate()) return true;
+    await new Promise((r) => setTimeout(r, stepMs));
+  }
+  return await predicate();
+}
+
+describe("McodeAcpClient.stop() — process group teardown", { skip: !posix && "POSIX-only" }, () => {
+  let dir;
+  let entry;
+  let McodeAcpClient;
+
+  before(async () => {
+    dir = mkdtempSync(join(tmpdir(), "webui-acp-pg-"));
+    entry = join(dir, "fake-acp.js");
+    writeFileSync(entry, FAKE_ACP);
+    const mod = await import("../../acp.mjs");
+    McodeAcpClient = mod.McodeAcpClient;
+  });
+
+  after(() => {
+    if (dir) rmSync(dir, { recursive: true, force: true });
+    delete process.env.MCODE_CMD;
+    delete process.env.PID_FILE;
+  });
+
+  test("a grandchild of the acp child does not outlive stop()", async () => {
+    const pidFile = join(dir, `pids-${Date.now()}.json`);
+    process.env.MCODE_CMD = entry;
+    process.env.PID_FILE = pidFile;
+
+    const client = new McodeAcpClient();
+    await client.start();
+    assert.equal(client.alive, true);
+
+    // The fake publishes its pids synchronously on spawn, but read with a
+    // short poll so a slow filesystem cannot flake the test.
+    const gotFile = await waitFor(() => existsSync(pidFile));
+    assert.ok(gotFile, "fake acp never published its pids");
+    const { child, grandchild } = JSON.parse(readFileSync(pidFile, "utf8"));
+
+    // Both are running right now — the precondition for the assertion below.
+    assert.equal(isAlive(child), true, "direct child should be running");
+    assert.equal(isAlive(grandchild), true, "grandchild should be running");
+
+    client.stop();
+
+    // The direct child dies on the SIGTERM; the grandchild only dies if the
+    // signal reached the whole group. Poll rather than sleep a fixed amount so
+    // this tracks real process teardown, not scheduling luck.
+    const gone = await waitFor(() => !isAlive(grandchild) && !isAlive(child), {
+      timeoutMs: 8000,
+    });
+    assert.equal(
+      gone,
+      true,
+      `process group survived stop(): child(alive=${isAlive(child)}) grandchild(alive=${isAlive(grandchild)})`,
+    );
+  });
+});