Merge pull request #197 from panda-z519/fix/subagent-model-picker

fix(subagents): choose a model only from the configured providers

Author: vastsa

apps/desktop/src/components/settings/SubagentEditorSheet.tsx

@@ -22,6 +22,7 @@ import {
   groupSubagentModelChoices,
   subagentModelChoices,
   subagentModelOrphanPin,
+  subagentModelPinParts,
   subagentModelSelectValue,
 } from "./subagent-models";
 
@@ -180,9 +181,14 @@ export function subagentDraftError(draft: SubagentDraft): string | null {
   if (!subagentSlug(draft.name)) return "extensions.subagents.errorSlug";
   if (!draft.description.trim()) return "extensions.subagents.errorDescription";
   if (draft.tools.length === 0) return "extensions.subagents.errorTools";
-  // `provider/model` is the only shape main can resolve; a bare model id has no
-  // provider to look up, so it would be dropped with a diagnostic nobody reads.
-  if (draft.model.trim() && !/^[^/\s]+\/.+$/.test(draft.model.trim())) {
+  // `provider/model` is the only shape the runtime can resolve; a bare model id
+  // has no provider to look up, so it would be dropped with a diagnostic nobody
+  // reads. Only the slash is structural: the provider half is matched by a
+  // normalized alias, and a custom endpoint's display name may contain spaces —
+  // the picker offers those, so rejecting them here would make a selectable
+  // option impossible to save. This shares the picker's own splitter so the two
+  // can never disagree.
+  if (draft.model.trim() && !subagentModelPinParts(draft.model.trim())) {
     return "extensions.subagents.errorModel";
   }
   // 0 is the cleared state, not an invalid one: a definition may leave the turn
@@ -326,9 +332,14 @@ function ManagementScope({
   );
 }
 
-const CUSTOM_SUBAGENT_MODEL_VALUE = "__custom__";
-
-/** Model and thinking controls for a subagent definition. */
+/**
+ * Model and thinking controls for a subagent definition.
+ *
+ * The model list offers only models the user already configured, so the value
+ * saved is always resolvable in Settings; there is no free-text escape hatch.
+ * When no provider offers a runnable model the field explains that and links to
+ * Models instead of accepting a hand-typed id the runtime could not resolve.
+ */
 function ModelField({
   draft,
   setDraft,
@@ -343,10 +354,7 @@ function ModelField({
   orphanModel: string | null;
 }) {
   const { t } = useTranslation();
-  const [customModel, setCustomModel] = useState(false);
-  const modelValue = customModel
-    ? CUSTOM_SUBAGENT_MODEL_VALUE
-    : subagentModelSelectValue(draft.model, modelChoices);
+  const modelValue = subagentModelSelectValue(draft.model, modelChoices);
 
   return (
     <>
@@ -360,28 +368,25 @@ function ModelField({
           }
         >
           {modelChoices.length === 0 ? (
-            <Input
-              value={draft.model}
-              placeholder={t("extensions.subagents.modelPickPlaceholder")}
-              aria-label={t("extensions.subagents.model")}
-              onChange={(event) =>
-                setDraft({ ...draft, model: event.target.value })
-              }
-            />
+            <div className="ext-field-empty">
+              <Button
+                variant="secondary"
+                size="sm"
+                onClick={() => {
+                  const store = useAppStore.getState();
+                  store.setSettingsTab("agent");
+                }}
+              >
+                {t("extensions.subagents.modelPickEmptyAction")}
+              </Button>
+            </div>
           ) : (
             <Select
               value={modelValue}
               aria-label={t("extensions.subagents.model")}
-              onChange={(event) => {
-                const value = event.target.value;
-                if (value === CUSTOM_SUBAGENT_MODEL_VALUE) {
-                  setCustomModel(true);
-                  setDraft({ ...draft, model: "" });
-                } else {
-                  setCustomModel(false);
-                  setDraft({ ...draft, model: value });
-                }
-              }}
+              onChange={(event) =>
+                setDraft({ ...draft, model: event.target.value })
+              }
             >
               <option value="">{t("extensions.subagents.modelInherit")}</option>
               {modelGroups.map((group) => (
@@ -396,9 +401,6 @@ function ModelField({
               {orphanModel ? (
                 <option value={orphanModel}>{orphanModel}</option>
               ) : null}
-              <option value={CUSTOM_SUBAGENT_MODEL_VALUE}>
-                {t("extensions.subagents.modelPickCustom")}
-              </option>
             </Select>
           )}
         </Field>
@@ -427,21 +429,6 @@ function ModelField({
           </Select>
         </Field>
       </div>
-      {modelChoices.length > 0 && customModel ? (
-        <Field
-          label={t("extensions.subagents.modelPickCustom")}
-          hint={t("extensions.subagents.modelPickCustomHint")}
-        >
-          <Input
-            value={draft.model}
-            placeholder={t("extensions.subagents.modelPickPlaceholder")}
-            aria-label={t("extensions.subagents.modelPickCustom")}
-            onChange={(event) =>
-              setDraft({ ...draft, model: event.target.value })
-            }
-          />
-        </Field>
-      ) : null}
     </>
   );
 }

apps/desktop/src/components/settings/subagent-models.ts

@@ -54,7 +54,19 @@ function providerAlias(value: string): string {
   return value.trim().toLowerCase().replace(/[^a-z0-9]+/g, "");
 }
 
-function pinParts(pin: string): { providerPart: string; modelId: string } | null {
+/**
+ * Split a pin into its provider half and model half, or null when it is not a
+ * pin at all.
+ *
+ * Only the slash is structural. The provider half is matched by a normalized
+ * alias at both ends of the app (`findProvider` in `agent-runtime` and
+ * `providerAlias` here), so a display name is a valid spelling — and a custom
+ * endpoint's display name may contain spaces. The editor validates drafts with
+ * this same function so the picker can never offer an option it would reject.
+ */
+export function subagentModelPinParts(
+  pin: string,
+): { providerPart: string; modelId: string } | null {
   const trimmed = pin.trim();
   const slash = trimmed.indexOf("/");
   if (slash < 1 || slash === trimmed.length - 1) return null;
@@ -65,13 +77,13 @@ function pinParts(pin: string): { providerPart: string; modelId: string } | null
 }
 
 export function pinMatchesChoice(pin: string, choice: SubagentModelChoice): boolean {
-  const parts = pinParts(pin);
+  const parts = subagentModelPinParts(pin);
   if (!parts) return false;
   if (!modelIdsMatch(parts.modelId, choice.modelId)) return false;
   if (parts.providerPart === choice.providerId) return true;
   const alias = providerAlias(parts.providerPart);
   if (!alias) return false;
-  const canonicalProviderPart = pinParts(choice.value)?.providerPart;
+  const canonicalProviderPart = subagentModelPinParts(choice.value)?.providerPart;
   if (canonicalProviderPart && providerAlias(canonicalProviderPart) === alias) {
     return true;
   }
@@ -122,17 +134,6 @@ export function subagentModelChoices(
   for (const { provider, modelId } of defaultModelOptions(
     runnableProviders,
   )) {
-    // Delegation is an explicit per-binding opt-in. The Composer can use every
-    // runnable model, but a subagent must not be able to select a binding that
-    // the user has not exposed to AI delegation in Models.
-    if (
-      !provider.models?.some(
-        (binding) =>
-          modelIdsMatch(binding.id, modelId) && binding.availableForSubagents === true,
-      )
-    ) {
-      continue;
-    }
     const value = `${uniqueProviderPart(provider, runnableProviders)}/${modelId}`;
     if (seen.has(value.toLowerCase())) continue;
     seen.add(value.toLowerCase());

apps/desktop/src/styles/extensions.css

@@ -1167,6 +1167,12 @@
   min-width: 0;
 }
 
+/* A field with nothing to pick yet: the action replaces the control. */
+.ext-field-empty {
+  display: flex;
+  align-items: center;
+}
+
 /* The transport choice is two cards, not a select: it changes the whole form. */
 .ext-transport-pick {
   display: grid;

apps/desktop/test/subagent-editor-presets.test.mjs

@@ -100,10 +100,14 @@ test("the model picker keeps existing pins visible", () => {
   assert.match(editorSource, /orphanModel \? \(/);
 });
 
-test("the model picker keeps custom ids available without exposing opted-out bindings", () => {
-  assert.match(editorSource, /CUSTOM_SUBAGENT_MODEL_VALUE/);
-  assert.match(editorSource, /extensions\.subagents\.modelPickCustom/);
-  assert.match(editorSource, /extensions\.subagents\.modelPickCustomHint/);
+test("the model picker offers every configured provider model, with no free-text path", () => {
+  // The picker is the only way to set a model: every option comes from the
+  // configured provider catalog, so a saved pin is always resolvable.
+  assert.match(editorSource, /subagentModelChoices\(providers\)/);
+  assert.match(editorSource, /groupSubagentModelChoices\(modelChoices\)/);
+  assert.match(editorSource, /subagentModelOrphanPin\(draft\.model, modelChoices\)/);
+  assert.doesNotMatch(editorSource, /CUSTOM_SUBAGENT_MODEL_VALUE/);
+  assert.doesNotMatch(editorSource, /modelPickCustom/);
 });
 
 test("the editor styles ship with the picker", () => {

apps/desktop/test/subagent-models.test.mjs

@@ -19,6 +19,7 @@ const {
   subagentModelChoices,
   subagentModelOrphanPin,
   subagentModelPin,
+  subagentModelPinParts,
   subagentModelSelectValue,
 } = await import("../src/components/settings/subagent-models.ts");
 
@@ -55,6 +56,46 @@ test("a custom endpoint uses its display name instead of the generic vendor key"
     subagentModelPin({ vendorKey: "custom", name: "My Gateway" }, "local-model"),
     "My Gateway/local-model",
   );
+
+test("every option the picker offers is a pin the editor will save", () => {
+  // A custom endpoint's display name contains spaces, and the picker offers it.
+  // The editor's draft check must accept exactly what the picker produces, or
+  // the user can pick an option and then never save the sheet.
+  const choices = subagentModelChoices([
+    provider(),
+    provider({ id: "p2", name: "My Gateway", vendorKey: "custom" }),
+    provider({ id: "p3", name: "My Ollama", vendorKey: "" }),
+  ]);
+  assert.ok(choices.length > 0);
+  for (const choice of choices) {
+    assert.notEqual(
+      subagentModelPinParts(choice.value),
+      null,
+      `${choice.value} is offered but not saveable`,
+    );
+  }
+});
+
+test("a pin needs a provider half, a model half, and keeps its own slashes", () => {
+  assert.deepEqual(subagentModelPinParts("anthropic/claude-haiku-4-5"), {
+    providerPart: "anthropic",
+    modelId: "claude-haiku-4-5",
+  });
+  assert.deepEqual(subagentModelPinParts("  My Gateway/local-model  "), {
+    providerPart: "My Gateway",
+    modelId: "local-model",
+  });
+  // An openrouter-style model id carries its own slashes; only the first one
+  // separates the provider.
+  assert.deepEqual(subagentModelPinParts("openrouter/deepseek/deepseek-chat"), {
+    providerPart: "openrouter",
+    modelId: "deepseek/deepseek-chat",
+  });
+  // A bare id has no provider to look up, so it is not a pin.
+  assert.equal(subagentModelPinParts("claude-haiku-4-5"), null);
+  assert.equal(subagentModelPinParts("/claude-haiku-4-5"), null);
+  assert.equal(subagentModelPinParts("anthropic/"), null);
+});
 });
 
 test("the sheet lists configured models from enabled, credentialed providers", () => {
@@ -89,7 +130,10 @@ test("the sheet lists configured models from enabled, credentialed providers", (
   );
 });
 
-test("the sheet only lists bindings explicitly enabled for subagents", () => {
+test("the sheet lists every configured binding regardless of the delegation flag", () => {
+  // The editor no longer consults `availableForSubagents`: a model the user
+  // configured in Settings is selectable, so the list cannot be empty while a
+  // runnable provider exists.
   const choices = subagentModelChoices([
     provider({
       models: [
@@ -104,7 +148,7 @@ test("the sheet only lists bindings explicitly enabled for subagents", () => {
   ]);
   assert.deepEqual(
     choices.map((choice) => choice.modelId),
-    ["claude-haiku-4-5"],
+    ["claude-haiku-4-5", "claude-sonnet-4-6", "claude-opus-4-6"],
   );
 });
 
@@ -222,7 +266,7 @@ test("a pin that is no longer configured stays selectable", () => {
   assert.equal(subagentModelOrphanPin("openai/gpt-5", choices), "openai/gpt-5");
 });
 
-test("the editor model field offers configured, custom, and empty-list paths", async () => {
+test("the editor model field is a configured-only select with an empty-state action", async () => {
   const source = await readFile(
     new URL("../src/components/settings/SubagentEditorSheet.tsx", import.meta.url),
     "utf8",
@@ -234,11 +278,13 @@ test("the editor model field offers configured, custom, and empty-list paths", a
   assert.match(modelField, /<Select/);
   assert.match(modelField, /<optgroup/);
   assert.match(modelField, /extensions\.subagents\.modelInherit/);
-  assert.match(modelField, /extensions\.subagents\.modelPickCustom/);
   assert.match(modelField, /extensions\.subagents\.modelPickEmpty/);
+  assert.match(modelField, /extensions\.subagents\.modelPickEmptyAction/);
+  assert.match(modelField, /setSettingsTab\("agent"\)/);
+  // No hand-typed model id: the field never renders a free-text input.
+  assert.doesNotMatch(modelField, /<Input/);
+  assert.doesNotMatch(modelField, /modelPickCustom/);
   assert.match(source, /subagentModelChoices\(providers\)/);
-  assert.match(source, /CUSTOM_SUBAGENT_MODEL_VALUE/);
-  assert.match(source, /extensions\.subagents\.modelPickCustomHint/);
   assert.match(source, /resetSubagentTemplate\(draft\)/);
 });