From 9eb22333a53c5be51110a2483acfa1c00b9267ca Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sat, 2 May 2026 19:01:13 -0700 Subject: [PATCH] fix(deploy-modal): snap provider radio when model resolves to a provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The TemplatePalette deploy modal (MissingKeysModal → ProviderPickerModal) let the model field and provider radio drift apart. When a hermes template defaulted the model to "MiniMax-M2.7-highspeed" but the radio defaulted to providers[0] (Anthropic), the env-var input below asked for ANTHROPIC_API_KEY. A user pasting their MINIMAX_API_KEY there (or just dismissing the dialog) ended up with a workspace whose runtime_config.model=MiniMax + ANTHROPIC_API_KEY env — the hermes adapter then crashed during boot before /registry/register, surfacing as WORKSPACE_PROVISION_FAILED 12 minutes later. Caught 2026-05-02 on hongming/Hermes Agent (workspace 95ed3ff2-… ended with: "container started but never called /registry/register"). Sibling of the ConfigTab cascade fix in PR #2516 (task #236) — same pattern, different surface. Plumbs the template's full ModelSpec[] (with required_env per model) into the picker. When the typed model matches a registry entry, snap the radio so the env-var fields underneath match what the model actually needs. Free-text models (typed slug not in the registry) and models with no required_env (local/self-hosted endpoints) leave the radio alone — the user can still pick a provider manually. Backwards-compat: callers that don't pass `models` get the pre-cascade behavior, pinned by a regression test. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/MissingKeysModal.tsx | 56 +++- .../MissingKeysModal.cascade.test.tsx | 282 ++++++++++++++++++ canvas/src/hooks/useTemplateDeploy.tsx | 6 + 3 files changed, 343 insertions(+), 1 deletion(-) create mode 100644 canvas/src/components/__tests__/MissingKeysModal.cascade.test.tsx diff --git a/canvas/src/components/MissingKeysModal.tsx b/canvas/src/components/MissingKeysModal.tsx index 1c3ef3cf..13da6ed0 100644 --- a/canvas/src/components/MissingKeysModal.tsx +++ b/canvas/src/components/MissingKeysModal.tsx @@ -3,7 +3,11 @@ import { useState, useEffect, useCallback, useRef, useMemo } from "react"; import { createPortal } from "react-dom"; import { api } from "@/lib/api"; -import { getKeyLabel, type ProviderChoice } from "@/lib/deploy-preflight"; +import { + getKeyLabel, + type ModelSpec, + type ProviderChoice, +} from "@/lib/deploy-preflight"; interface Props { open: boolean; @@ -38,6 +42,14 @@ interface Props { * the API-key fields. The picker passes the entered slug back via * onKeysAdded. */ modelSuggestions?: string[]; + /** Full model specs from the template (with required_env per model). + * When provided, the picker auto-snaps the provider radio to the + * matching provider as the user changes the model — fixes the + * "type MiniMax model, see ANTHROPIC_API_KEY field" cascade bug + * (sibling of the ConfigTab cascade fix in #2516). Optional so + * callers without model→provider mapping data can still use the + * picker as-is. */ + models?: ModelSpec[]; /** Pre-fill the model input. */ initialModel?: string; /** Override the modal's title + description copy. The default @@ -83,6 +95,7 @@ export function MissingKeysModal({ workspaceId, configuredKeys, modelSuggestions, + models, initialModel, title, description, @@ -102,6 +115,7 @@ export function MissingKeysModal({ workspaceId={workspaceId} configuredKeys={configuredKeys} modelSuggestions={modelSuggestions} + models={models} initialModel={initialModel} title={title} description={description} @@ -131,6 +145,22 @@ export function MissingKeysModal({ // Provider-picker mode — choose one option, save its env var(s), deploy. // ----------------------------------------------------------------------------- +/** Provider id derived from a model spec — sorted+joined required_env, + * matching the formula in providersFromTemplate(). When the model has + * no required_env (local/self-hosted endpoints) returns null, since + * there's no provider option the radio could snap to. Exported for + * the cascade-snap test. */ +export function providerIdForModel( + modelId: string, + models: ModelSpec[] | undefined, +): string | null { + const trimmed = modelId.trim(); + if (!trimmed || !models) return null; + const m = models.find((x) => x.id === trimmed); + if (!m?.required_env || m.required_env.length === 0) return null; + return [...m.required_env].sort().join("|"); +} + function ProviderPickerModal({ open, providers, @@ -141,6 +171,7 @@ function ProviderPickerModal({ workspaceId, configuredKeys, modelSuggestions, + models, initialModel, title, description, @@ -154,6 +185,7 @@ function ProviderPickerModal({ workspaceId?: string; configuredKeys?: Set; modelSuggestions?: string[]; + models?: ModelSpec[]; initialModel?: string; title?: string; description?: string; @@ -189,6 +221,28 @@ function ProviderPickerModal({ setModel(initialModel ?? ""); }, [open, initialSelected, initialModel]); + // Cascade: when the model resolves to a known provider via its + // required_env, snap the radio so the env-var fields below match + // the model the user picked. Without this, picking + // "MiniMax-M2.7-highspeed" leaves the radio on whatever default + // was first (e.g. Anthropic) and surfaces ANTHROPIC_API_KEY as + // the required key — saving that and deploying produces a + // workspace with model=MiniMax + ANTHROPIC_API_KEY which then + // fails to call /registry/register and times out. Caught + // 2026-05-02 on hongming/Hermes Agent (workspace + // 95ed3ff2-… ended in WORKSPACE_PROVISION_FAILED). + // Free-text models not in `models` (or models without + // required_env) fall through and leave the radio alone. + useEffect(() => { + if (!open) return; + const targetId = providerIdForModel(model, models); + if (!targetId) return; + const matching = providers.find((p) => p.id === targetId); + if (matching && matching.id !== selectedId) { + setSelectedId(matching.id); + } + }, [open, model, models, providers, selectedId]); + useEffect(() => { if (!open) return; setEntries( diff --git a/canvas/src/components/__tests__/MissingKeysModal.cascade.test.tsx b/canvas/src/components/__tests__/MissingKeysModal.cascade.test.tsx new file mode 100644 index 00000000..32dfd62b --- /dev/null +++ b/canvas/src/components/__tests__/MissingKeysModal.cascade.test.tsx @@ -0,0 +1,282 @@ +// @vitest-environment jsdom +/** + * Provider→model cascade in the deploy modal (sibling of the ConfigTab + * cascade fix shipped in PR #2516, task #236). + * + * The user-reported bug (2026-05-02 hongming Hermes Agent): + * + * 1. User opens TemplatePalette → Deploy on a hermes template. + * 2. Modal shows MODEL field pre-filled with template default + * (e.g. "MiniMax-M2.7-highspeed") AND a list of provider radios + * (Anthropic, OpenRouter, MiniMax, …). + * 3. The provider radio defaults to whichever entry was first in + * `preflight.providers` (Anthropic in the hermes case). + * 4. The env-var input below shows ANTHROPIC_API_KEY. + * 5. User pastes whatever key they have, clicks Deploy. + * 6. Workspace is created with model=MiniMax-M2.7-highspeed + + * ANTHROPIC_API_KEY → hermes adapter tries to call Anthropic + * with a MiniMax model id → crashes before /registry/register + * → workspace ends in WORKSPACE_PROVISION_FAILED with + * "container started but never called /registry/register". + * + * Fix: when the model resolves to a known provider via its + * `required_env`, snap the radio so the env-var fields below match + * the model the user picked. Free-text models not in `models` (or + * models without required_env) leave the radio alone — the user can + * still manually pick a provider. + */ +import { describe, it, expect, vi, afterEach } from "vitest"; +import { render, screen, fireEvent, cleanup } from "@testing-library/react"; + +import { MissingKeysModal, providerIdForModel } from "../MissingKeysModal"; +import type { ModelSpec, ProviderChoice } from "@/lib/deploy-preflight"; + +vi.mock("@/lib/api", () => ({ + api: { get: vi.fn(), put: vi.fn() }, +})); + +vi.mock("@/lib/deploy-preflight", async () => { + const actual = await vi.importActual( + "@/lib/deploy-preflight", + ); + return actual; +}); + +// Hermes-shaped fixture: 3 providers, multiple models per provider, one +// "no required_env" local model that should never block a deploy. +const HERMES_PROVIDERS: ProviderChoice[] = [ + { + id: "ANTHROPIC_API_KEY", + label: "Anthropic (8 models)", + envVars: ["ANTHROPIC_API_KEY"], + }, + { + id: "MINIMAX_API_KEY", + label: "MiniMax (2 models)", + envVars: ["MINIMAX_API_KEY"], + }, + { + id: "OPENROUTER_API_KEY", + label: "OpenRouter (14 models)", + envVars: ["OPENROUTER_API_KEY"], + }, +]; + +const HERMES_MODELS: ModelSpec[] = [ + { id: "claude-sonnet-4-6", required_env: ["ANTHROPIC_API_KEY"] }, + { id: "claude-opus-4-7", required_env: ["ANTHROPIC_API_KEY"] }, + { id: "MiniMax-M2.7-highspeed", required_env: ["MINIMAX_API_KEY"] }, + { id: "MiniMax-M2.7", required_env: ["MINIMAX_API_KEY"] }, + { id: "openrouter/anthropic/claude-3.5-sonnet", required_env: ["OPENROUTER_API_KEY"] }, + // Local/self-hosted endpoint — no required_env. Picker should + // never snap on this one because there's no provider to snap to. + { id: "local-llama3", required_env: [] }, +]; + +describe("providerIdForModel", () => { + it("returns the provider id (sorted+joined required_env) for a known model", () => { + expect(providerIdForModel("MiniMax-M2.7-highspeed", HERMES_MODELS)).toBe( + "MINIMAX_API_KEY", + ); + expect(providerIdForModel("claude-opus-4-7", HERMES_MODELS)).toBe( + "ANTHROPIC_API_KEY", + ); + }); + + // The id formula sorts envVars before joining. A model that needs + // two keys together (rare today, but the shape supports it) maps + // to a deterministic id regardless of the order in required_env. + it("sorts required_env so the id matches providersFromTemplate's formula", () => { + const models: ModelSpec[] = [ + { id: "weird", required_env: ["Z_KEY", "A_KEY"] }, + ]; + expect(providerIdForModel("weird", models)).toBe("A_KEY|Z_KEY"); + }); + + it("trims whitespace before lookup so a stray space doesn't miss a match", () => { + expect(providerIdForModel(" MiniMax-M2.7 ", HERMES_MODELS)).toBe( + "MINIMAX_API_KEY", + ); + }); + + it("returns null for empty / undefined / whitespace-only model id", () => { + expect(providerIdForModel("", HERMES_MODELS)).toBeNull(); + expect(providerIdForModel(" ", HERMES_MODELS)).toBeNull(); + }); + + it("returns null when models are not provided (free-text mode)", () => { + expect(providerIdForModel("anything", undefined)).toBeNull(); + }); + + it("returns null when model isn't in the registry (free-text)", () => { + expect(providerIdForModel("not-a-listed-model", HERMES_MODELS)).toBeNull(); + }); + + it("returns null when the model has no required_env (local endpoint)", () => { + expect(providerIdForModel("local-llama3", HERMES_MODELS)).toBeNull(); + }); +}); + +describe("ProviderPickerModal — model→provider cascade", () => { + afterEach(() => cleanup()); + + // The headline bug: opening the modal with the MiniMax default + // pre-filled should NOT leave the radio on Anthropic just because + // Anthropic was first in providers[]. The cascade snaps the radio + // to MINIMAX_API_KEY on first paint. + it("snaps provider radio to MiniMax when initialModel is a MiniMax model", () => { + render( + m.id)} + models={HERMES_MODELS} + initialModel="MiniMax-M2.7-highspeed" + onKeysAdded={vi.fn()} + onCancel={vi.fn()} + />, + ); + const minimaxRadio = screen.getByRole("radio", { + name: /MiniMax \(2 models\)/i, + }) as HTMLInputElement; + expect(minimaxRadio.checked).toBe(true); + // The env-var input underneath should be for MINIMAX_API_KEY, + // not ANTHROPIC_API_KEY — that's the load-bearing UX win. The + // entry uses a password input with a fixed "sk-..." placeholder + // when the key name contains "API_KEY"; assert exactly ONE such + // input exists, which proves only the selected provider's envVars + // were rendered into entries[]. (The provider-radio subtitles + // also mention each envVar name as Mono text — that's why we + // can't use getByText("MINIMAX_API_KEY") here, it would match + // both the radio label and the entry label.) + const apiKeyInputs = screen.getAllByPlaceholderText("sk-..."); + expect(apiKeyInputs).toHaveLength(1); + }); + + // Mid-flow change: user starts with the pre-filled MiniMax model, + // edits it to a Claude model, the radio re-snaps to Anthropic. This + // matches user expectation — picking a different model shouldn't + // leave the wrong env-var input showing. + it("re-snaps when the user edits the model field to a different provider's model", () => { + render( + m.id)} + models={HERMES_MODELS} + initialModel="MiniMax-M2.7-highspeed" + onKeysAdded={vi.fn()} + onCancel={vi.fn()} + />, + ); + const modelInput = screen.getByLabelText(/Model slug/i) as HTMLInputElement; + fireEvent.change(modelInput, { target: { value: "claude-opus-4-7" } }); + const anthropicRadio = screen.getByRole("radio", { + name: /Anthropic \(8 models\)/i, + }) as HTMLInputElement; + expect(anthropicRadio.checked).toBe(true); + // Same shape-pin as the previous test — exactly one + // password input means only the selected provider's envVars + // landed in entries[]. + expect(screen.getAllByPlaceholderText("sk-...")).toHaveLength(1); + }); + + // Free-text models (typed slug not in the registry) should NOT + // change the radio — the user may know about a model the template + // doesn't list. Falling back to the previously-selected provider + // keeps the form in a usable state. + it("leaves the radio alone when the typed model is not in the registry", () => { + render( + m.id)} + models={HERMES_MODELS} + initialModel="MiniMax-M2.7-highspeed" + onKeysAdded={vi.fn()} + onCancel={vi.fn()} + />, + ); + // Snapped to MiniMax by initial cascade. + expect( + (screen.getByRole("radio", { + name: /MiniMax \(2 models\)/i, + }) as HTMLInputElement).checked, + ).toBe(true); + + // Type something the registry doesn't know — radio stays on MiniMax. + const modelInput = screen.getByLabelText(/Model slug/i) as HTMLInputElement; + fireEvent.change(modelInput, { + target: { value: "some-future-model-not-in-registry" }, + }); + expect( + (screen.getByRole("radio", { + name: /MiniMax \(2 models\)/i, + }) as HTMLInputElement).checked, + ).toBe(true); + }); + + // Backwards-compat: callers that don't pass `models` (legacy + // call sites) keep the pre-cascade behavior — radio defaults to + // providers[0] (or to a satisfied configuredKeys match). The + // cascade is purely additive. + it("falls back to providers[0] when models prop is omitted", () => { + render( + m.id)} + // models intentionally omitted — legacy caller shape. + initialModel="MiniMax-M2.7-highspeed" + onKeysAdded={vi.fn()} + onCancel={vi.fn()} + />, + ); + // Without `models`, no cascade: radio sits on providers[0] + // (Anthropic), reproducing the bug the cascade fixes. Pinned + // here so anyone removing the `models` prop sees the regression. + expect( + (screen.getByRole("radio", { + name: /Anthropic \(8 models\)/i, + }) as HTMLInputElement).checked, + ).toBe(true); + }); + + // configuredKeys interaction: when a provider's keys are already + // saved globally, the picker pre-selects that satisfied provider. + // The model cascade should still override — the user explicitly + // picked a model that needs a different provider, that intent + // wins over "you already have this key". + it("model cascade beats configuredKeys-satisfied default", () => { + render( + m.id)} + models={HERMES_MODELS} + initialModel="MiniMax-M2.7-highspeed" + onKeysAdded={vi.fn()} + onCancel={vi.fn()} + />, + ); + expect( + (screen.getByRole("radio", { + name: /MiniMax \(2 models\)/i, + }) as HTMLInputElement).checked, + ).toBe(true); + }); +}); diff --git a/canvas/src/hooks/useTemplateDeploy.tsx b/canvas/src/hooks/useTemplateDeploy.tsx index 4f746c98..41bf9000 100644 --- a/canvas/src/hooks/useTemplateDeploy.tsx +++ b/canvas/src/hooks/useTemplateDeploy.tsx @@ -197,6 +197,12 @@ export function useTemplateDeploy( runtime={missingKeysInfo?.preflight.runtime ?? ""} configuredKeys={missingKeysInfo?.preflight.configuredKeys} modelSuggestions={isMultiProvider ? modelSuggestions : undefined} + // Pass full model specs (id + required_env) so the picker can + // auto-snap the provider radio when the user picks a model — fixes + // the "type MiniMax model, see ANTHROPIC_API_KEY" cascade bug. + // Only relevant in multi-provider mode where the model field is + // shown. + models={isMultiProvider ? missingKeysInfo?.template.models : undefined} initialModel={isMultiProvider ? initialModel : undefined} title={modalTitle} description={modalDescription}