fix: disambiguate colliding builtin model ids in configOptions

src/config/options.ts

@@ -164,21 +164,51 @@ export function isBuiltinProvider(providerId: string): boolean {
 /**
  * Encode a provider+model pair into a configOption `value` string.
  *
- * Builtin providers encode as the bare modelId (legacy form, keeps the dropdown
- * clean for the common case). Third-party providers encode as
- * `providerId\modelId` — `\` is unambiguous because providerIds (UUIDs) and
- * modelIds (`/`-separated) never contain it.
+ * Always `providerId\modelId` — builtins included. A collision-only prefix
+ * would advertise different id shapes depending on how many coding plans the
+ * user has enabled. `\` is unambiguous because providerIds (UUIDs / builtin:
+ * slugs) and modelIds (`/`-separated) never contain it.
+ *
+ * Inbound, `parseModelValue` still accepts a legacy bare modelId.
  */
 export function formatModelValue(providerId: string, modelId: string): string {
-  if (isBuiltinProvider(providerId)) return modelId;
   return `${providerId}\\${modelId}`;
 }
 
+function collidingModelIds(models: ModelRef[]): Set<string> {
+  const counts = new Map<string, number>();
+  for (const model of models) {
+    counts.set(model.modelId, (counts.get(model.modelId) ?? 0) + 1);
+  }
+  const colliding = new Set<string>();
+  for (const [modelId, count] of counts) {
+    if (count > 1) colliding.add(modelId);
+  }
+  return colliding;
+}
+
+function buildModelSelectOptions(models: ModelRef[]): Array<{ value: string; name: string }> {
+  const collidingIds = collidingModelIds(models);
+  const options: Array<{ value: string; name: string }> = [];
+  const seen = new Set<string>();
+  for (const model of models) {
+    const value = formatModelValue(model.providerId, model.modelId);
+    if (seen.has(value)) continue;
+    seen.add(value);
+    const qualify = collidingIds.has(model.modelId) || !isBuiltinProvider(model.providerId);
+    options.push({
+      value,
+      name: qualify ? `${model.providerName} › ${model.modelId}` : model.modelId,
+    });
+  }
+  return options;
+}
+
 /**
  * Parse a configOption `value` back into { providerId, modelId }.
  *
- * A value without `\` is a builtin modelId (legacy form) → resolve to the first
- * enabled builtin provider. A value with `\` is a third-party provider+model.
+ * A value without `\` is a legacy bare modelId → resolve to the first enabled
+ * builtin provider. A value with `\` is the current provider+model encoding.
  */
 export function parseModelValue(value: string): { providerId: string; modelId: string } {
   const idx = value.indexOf("\\");
@@ -336,18 +366,14 @@ export async function buildConfigOptions(
   // currentValue encodes provider+model so the switch handler can locate the
   // right provider (and its apiKey). Fall back to the first enabled provider
   // when settings omits providerId (legacy sessions).
-  const currentModel = formatModelValue(
-    currentProviderId || loadAllModels()[0]?.providerId || DEFAULT_PROVIDER_ID,
-    currentModelId,
-  );
+  const allModels = loadAllModels();
+  const currentProvider = currentProviderId || allModels[0]?.providerId || DEFAULT_PROVIDER_ID;
+  const currentModel = formatModelValue(currentProvider, currentModelId);
 
-  // Model options: config.json enabled providers are authoritative. Builtin
-  // models show as the bare modelId (clean dropdown for the common case);
-  // third-party models prefix the provider name so they're distinguishable.
-  let modelOptions = loadAllModels().map((m) => ({
-    value: formatModelValue(m.providerId, m.modelId),
-    name: isBuiltinProvider(m.providerId) ? m.modelId : `${m.providerName} › ${m.modelId}`,
-  }));
+  // Model options: config.json enabled providers are authoritative. Values
+  // are always providerId\modelId. Builtin labels stay the bare modelId
+  // unless two providers ship the same id; third-party labels always qualify.
+  let modelOptions = buildModelSelectOptions(allModels);
   if (!modelOptions.some((o) => o.value === currentModel)) {
     // The current model isn't from an enabled provider (e.g. the session was
     // created with a now-disabled provider). Append it so the dropdown still

tests/model-select-options.test.ts

@@ -0,0 +1,120 @@
+/**
+ * ACP model option values always encode as providerId\modelId, including
+ * builtins. Bare GLM-5.3 collided when two coding plans shipped the same id
+ * and crashed clients that key on uniqueness (Paseo Command Center). A
+ * collision-only prefix would have advertised different id shapes depending
+ * on the user's enabled-provider set.
+ *
+ * parseModelValue still accepts a legacy bare modelId (first enabled builtin).
+ * Dropdown labels stay the bare modelId for a single builtin; colliding
+ * modelIds are qualified with the provider name.
+ */
+
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import { ZCODE_CREDS_PATH } from "../src/utils.js";
+import { ZcodeAcpServer } from "../src/server.js";
+
+function codingPlan(name: string, baseURL: string) {
+  return {
+    name,
+    kind: "anthropic",
+    enabled: true,
+    options: { apiKey: "plan-token", apiKeyRequired: true, baseURL },
+    models: {
+      "GLM-5.3": { limit: { context: 200000 } },
+      "GLM-5.3-Flash": { limit: { context: 200000 } },
+    },
+  };
+}
+
+function collidingPlansConfig() {
+  return {
+    provider: {
+      "builtin:zai-coding-plan": codingPlan("Z.ai - Coding Plan", "https://api.z.ai/api/anthropic"),
+      "builtin:zai-start-plan": codingPlan(
+        "Z.ai - Start Plan",
+        "https://zcode.z.ai/api/v1/zcode-plan/anthropic",
+      ),
+    },
+  };
+}
+
+let fakeConfig: unknown = collidingPlansConfig();
+
+vi.mock("node:fs", async () => {
+  const actual = await vi.importActual<typeof import("node:fs")>("node:fs");
+  return {
+    ...actual,
+    readFileSync: (p: string) => {
+      if (p === ZCODE_CREDS_PATH) return JSON.stringify(fakeConfig);
+      return actual.readFileSync(p);
+    },
+  };
+});
+
+const { buildConfigOptions, formatModelValue, loadAllModels, parseModelValue } =
+  await import("../src/config/options.js");
+
+afterEach(() => {
+  fakeConfig = collidingPlansConfig();
+});
+
+describe("model configOptions uniqueness", () => {
+  it("does not advertise duplicate values when two builtins share GLM-5.3", async () => {
+    const options = await buildConfigOptions(new ZcodeAcpServer(), null);
+    const model = options.find((option) => option.id === "model");
+    const values = model?.options.map((option) => option.value) ?? [];
+
+    expect(values).toEqual([...new Set(values)]);
+    expect(values).toHaveLength(4);
+    expect(model?.currentValue).toBeDefined();
+    expect(values).toContain(model?.currentValue);
+  });
+
+  it("keeps both colliding coding plans selectable", async () => {
+    const options = await buildConfigOptions(new ZcodeAcpServer(), null);
+    const model = options.find((option) => option.id === "model");
+    const parsed = (model?.options ?? []).map((option) => parseModelValue(option.value));
+
+    expect(parsed).toEqual([
+      { providerId: "builtin:zai-coding-plan", modelId: "GLM-5.3" },
+      { providerId: "builtin:zai-coding-plan", modelId: "GLM-5.3-Flash" },
+      { providerId: "builtin:zai-start-plan", modelId: "GLM-5.3" },
+      { providerId: "builtin:zai-start-plan", modelId: "GLM-5.3-Flash" },
+    ]);
+    expect(model?.options.map((option) => option.name)).toEqual([
+      "Z.ai - Coding Plan › GLM-5.3",
+      "Z.ai - Coding Plan › GLM-5.3-Flash",
+      "Z.ai - Start Plan › GLM-5.3",
+      "Z.ai - Start Plan › GLM-5.3-Flash",
+    ]);
+  });
+
+  it("encodes a single builtin as providerId\\modelId too", async () => {
+    fakeConfig = {
+      provider: {
+        "builtin:zai-coding-plan": codingPlan(
+          "Z.ai - Coding Plan",
+          "https://api.z.ai/api/anthropic",
+        ),
+      },
+    };
+
+    const options = await buildConfigOptions(new ZcodeAcpServer(), null);
+    const model = options.find((option) => option.id === "model");
+    const values = model?.options.map((option) => option.value) ?? [];
+
+    expect(values).toEqual([
+      "builtin:zai-coding-plan\\GLM-5.3",
+      "builtin:zai-coding-plan\\GLM-5.3-Flash",
+    ]);
+    expect(model?.options.map((option) => option.name)).toEqual(["GLM-5.3", "GLM-5.3-Flash"]);
+    expect(model?.currentValue).toBe(formatModelValue("builtin:zai-coding-plan", "GLM-5.3"));
+    expect(parseModelValue("GLM-5.3")).toEqual({
+      providerId: "builtin:zai-coding-plan",
+      modelId: "GLM-5.3",
+    });
+    expect(loadAllModels()).toHaveLength(2);
+  });
+});

tests/runtime-model.test.ts

@@ -10,8 +10,8 @@
  * buildRuntimeModel() inlines apiKey as {source:"inline",value}
  * for third-party providers (the backend resolves model-call auth from the
  * overlay itself; omitting it yields HTTP 401) but omits it for builtins.
- * Builtin models encode as bare modelIds, and third-party models carry their
- * providerId prefix.
+ * Every model encodes as providerId\modelId. A legacy bare modelId still
+ * parses as the first enabled builtin.
  */
 
 import { describe, expect, it, vi } from "vitest";
@@ -205,10 +205,9 @@ describe("modelContextWindow", () => {
 });
 
 describe("parseModelValue / formatModelValue", () => {
-  it("builtin providers encode as bare modelId (no prefix)", () => {
-    // The common case stays clean — builtin models show just the modelId.
+  it("builtin providers encode as providerId\\modelId", () => {
     const value = formatModelValue("builtin:primary", "model-a");
-    expect(value).toBe("model-a");
+    expect(value).toBe("builtin:primary\\model-a");
     expect(parseModelValue(value)).toEqual({
       providerId: "builtin:primary",
       modelId: "model-a",