Merge pull request #2460 from Molecule-AI/feat/template-always-ask-provider

feat(canvas): always ask for provider+model when deploying multi-provider templates
This commit is contained in:
Hongming Wang 2026-05-01 20:45:37 +00:00 committed by GitHub
commit d294f15c88
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 418 additions and 32 deletions

View File

@ -16,14 +16,35 @@ interface Props {
/** Runtime slug used only for the "The <runtime> runtime "
* headline; behavior is driven by providers/missingKeys. */
runtime: string;
/** Called when all required keys for the chosen provider are saved. */
onKeysAdded: () => void;
/** Called when all required keys for the chosen provider are saved.
* Receives the model slug if the modal collected one (template-deploy
* flow); legacy callers ignore it. */
onKeysAdded: (model?: string) => void;
/** Called when the user cancels the deploy. */
onCancel: () => void;
/** Optional — open the Settings Panel (Config tab → Secrets). */
onOpenSettings?: () => void;
/** If provided, secrets save at workspace scope instead of global. */
workspaceId?: string;
/** Set of env var names already configured in the relevant scope
* (global or workspace). When provided, entries whose key is already
* in this set start as `saved: true` so the user can confirm without
* re-entering. Used by the template-deploy "always ask" flow so a
* user can pick a different provider even when global env covers
* the default one. */
configuredKeys?: Set<string>;
/** Model slug suggestions (datalist) populated from the template's
* models[]. When non-empty the picker renders a model input above
* the API-key fields. The picker passes the entered slug back via
* onKeysAdded. */
modelSuggestions?: string[];
/** Pre-fill the model input. */
initialModel?: string;
/** Override the modal's title + description copy. The default
* "Missing API Keys" title misreads when the modal is opened to
* pick provider/model with keys already configured. */
title?: string;
description?: string;
}
interface KeyEntry {
@ -60,6 +81,11 @@ export function MissingKeysModal({
onCancel,
onOpenSettings,
workspaceId,
configuredKeys,
modelSuggestions,
initialModel,
title,
description,
}: Props) {
const pickerProviders = providers ?? [];
const pickerMode = pickerProviders.length > 1;
@ -74,6 +100,11 @@ export function MissingKeysModal({
onCancel={onCancel}
onOpenSettings={onOpenSettings}
workspaceId={workspaceId}
configuredKeys={configuredKeys}
modelSuggestions={modelSuggestions}
initialModel={initialModel}
title={title}
description={description}
/>
);
}
@ -108,17 +139,41 @@ function ProviderPickerModal({
onCancel,
onOpenSettings,
workspaceId,
configuredKeys,
modelSuggestions,
initialModel,
title,
description,
}: {
open: boolean;
providers: ProviderChoice[];
runtime: string;
onKeysAdded: () => void;
onKeysAdded: (model?: string) => void;
onCancel: () => void;
onOpenSettings?: () => void;
workspaceId?: string;
configuredKeys?: Set<string>;
modelSuggestions?: string[];
initialModel?: string;
title?: string;
description?: string;
}) {
const [selectedId, setSelectedId] = useState(providers[0].id);
// Prefer the first provider whose env vars are already satisfied by
// the configured set — pre-selecting "the option the user already has
// keys for" matches expected UX. Falls back to providers[0] otherwise.
const initialSelected = useMemo(() => {
if (configuredKeys) {
const satisfied = providers.find((p) =>
p.envVars.every((k) => configuredKeys.has(k)),
);
if (satisfied) return satisfied.id;
}
return providers[0].id;
}, [providers, configuredKeys]);
const [selectedId, setSelectedId] = useState(initialSelected);
const [entries, setEntries] = useState<KeyEntry[]>([]);
const [model, setModel] = useState(initialModel ?? "");
const firstInputRef = useRef<HTMLInputElement>(null);
const selected = useMemo(
@ -126,10 +181,13 @@ function ProviderPickerModal({
[providers, selectedId],
);
const showModelInput = (modelSuggestions?.length ?? 0) > 0 || initialModel !== undefined;
useEffect(() => {
if (!open) return;
setSelectedId(providers[0].id);
}, [open, providers]);
setSelectedId(initialSelected);
setModel(initialModel ?? "");
}, [open, initialSelected, initialModel]);
useEffect(() => {
if (!open) return;
@ -137,12 +195,15 @@ function ProviderPickerModal({
selected.envVars.map((key) => ({
key,
value: "",
saved: false,
// Pre-mark as saved when the key is already in the configured
// set (global or workspace scope). Lets the user click Deploy
// without re-entering a key the platform already holds.
saved: configuredKeys?.has(key) ?? false,
saving: false,
error: null,
})),
);
}, [open, selected]);
}, [open, selected, configuredKeys]);
useEffect(() => {
if (!open) return;
@ -243,16 +304,52 @@ function ProviderPickerModal({
</svg>
</div>
<h3 id="missing-keys-title" className="text-sm font-semibold text-zinc-100">
Missing API Keys
{title ?? "Missing API Keys"}
</h3>
</div>
<p className="text-[12px] text-zinc-400 leading-relaxed">
The <span className="text-amber-300 font-medium">{runtimeLabel}</span>{" "}
runtime supports multiple providers. Pick one and paste its API key.
{description ?? (
<>
The <span className="text-amber-300 font-medium">{runtimeLabel}</span>{" "}
runtime supports multiple providers. Pick one and paste its API key.
</>
)}
</p>
</div>
<div className="px-5 py-4 space-y-3">
{showModelInput && (
<div>
<label
htmlFor="provider-picker-model-input"
className="text-[10px] uppercase tracking-wide text-zinc-500 font-semibold mb-1.5 block"
>
Model{" "}
<span aria-hidden="true" className="text-red-400">*</span>
<span className="sr-only"> (required)</span>
</label>
<input
id="provider-picker-model-input"
type="text"
value={model}
onChange={(e) => setModel(e.target.value)}
placeholder="e.g. minimax/MiniMax-M2.7"
aria-label="Model slug"
autoComplete="off"
spellCheck={false}
list="provider-picker-model-suggestions"
className="w-full bg-zinc-900 border border-zinc-600 rounded px-2 py-1.5 text-[11px] text-zinc-100 font-mono focus:outline-none focus:border-blue-500 focus:ring-1 focus:ring-blue-500/20 transition-colors"
/>
<datalist id="provider-picker-model-suggestions">
{modelSuggestions?.map((m) => (
<option key={m} value={m} />
))}
</datalist>
<p className="text-[9px] text-zinc-500 mt-1 leading-relaxed">
Slug determines provider routing at install time.
</p>
</div>
)}
<fieldset className="space-y-1.5">
<legend className="text-[10px] uppercase tracking-wide text-zinc-500 font-semibold mb-1.5">
Provider
@ -364,8 +461,12 @@ function ProviderPickerModal({
Cancel Deploy
</button>
<button
onClick={onKeysAdded}
disabled={!allSaved || anySaving}
onClick={() => onKeysAdded(showModelInput ? model.trim() : undefined)}
disabled={
!allSaved ||
anySaving ||
(showModelInput && model.trim() === "")
}
className="px-3.5 py-1.5 text-[12px] bg-blue-600 hover:bg-blue-500 text-white rounded-lg transition-colors disabled:opacity-40"
>
{allSaved ? "Deploy" : entries.length > 1 ? "Add Keys" : "Add Key"}

View File

@ -27,16 +27,16 @@ import { renderHook } from "@testing-library/react";
import type { Template } from "@/lib/deploy-preflight";
// ── Hoisted mocks ────────────────────────────────────────────────────────────
const { mockApiPost, mockCheckDeploySecrets, mockResolveRuntime } = vi.hoisted(
() => ({
const { mockApiPost, mockApiGet, mockCheckDeploySecrets, mockResolveRuntime } =
vi.hoisted(() => ({
mockApiPost: vi.fn(),
mockApiGet: vi.fn(),
mockCheckDeploySecrets: vi.fn(),
mockResolveRuntime: vi.fn(),
}),
);
}));
vi.mock("@/lib/api", () => ({
api: { post: mockApiPost },
api: { post: mockApiPost, get: mockApiGet },
}));
vi.mock("@/lib/deploy-preflight", async () => {
@ -51,20 +51,44 @@ vi.mock("@/lib/deploy-preflight", async () => {
};
});
// MissingKeysModal: render a minimal stand-in that exposes the two
// callbacks the hook wires up. The real modal pulls in radix + the
// secrets store, neither of which is relevant to this hook's behavior.
// MissingKeysModal: render a minimal stand-in that exposes the
// callbacks the hook wires up + dumps the new template-deploy props
// (configuredKeys size, modelSuggestions, initialModel) into the
// DOM so tests can assert on them. The real modal pulls in radix +
// the secrets store, neither of which is relevant to this hook's
// behavior.
vi.mock("@/components/MissingKeysModal", () => ({
MissingKeysModal: (props: {
open: boolean;
onKeysAdded: () => void;
onKeysAdded: (model?: string) => void;
onCancel: () => void;
configuredKeys?: Set<string>;
modelSuggestions?: string[];
initialModel?: string;
title?: string;
}) =>
props.open ? (
<div data-testid="missing-keys-modal">
<button data-testid="modal-keys-added" onClick={props.onKeysAdded}>
<span data-testid="modal-configured-size">
{props.configuredKeys?.size ?? 0}
</span>
<span data-testid="modal-model-suggestions">
{(props.modelSuggestions ?? []).join(",")}
</span>
<span data-testid="modal-initial-model">{props.initialModel ?? ""}</span>
<span data-testid="modal-title">{props.title ?? ""}</span>
<button
data-testid="modal-keys-added"
onClick={() => props.onKeysAdded()}
>
keys added
</button>
<button
data-testid="modal-keys-added-with-model"
onClick={() => props.onKeysAdded("minimax/MiniMax-M2.7")}
>
keys added with model
</button>
<button data-testid="modal-cancel" onClick={props.onCancel}>
cancel
</button>
@ -95,6 +119,7 @@ function makeTemplate(over: Partial<Template> = {}): Template {
beforeEach(() => {
mockApiPost.mockReset();
mockApiGet.mockReset();
mockCheckDeploySecrets.mockReset();
mockResolveRuntime.mockReset();
// Default: identity-mapped runtime, preflight passes.
@ -104,8 +129,12 @@ beforeEach(() => {
missingKeys: [],
providers: [],
runtime: "claude-code",
configuredKeys: new Set(),
});
mockApiPost.mockResolvedValue({ id: "ws-new" });
// Default: secrets endpoint returns nothing so the picker
// renders every entry as input. Multi-provider tests override.
mockApiGet.mockResolvedValue([]);
});
afterEach(() => {
@ -204,6 +233,7 @@ describe("useTemplateDeploy — preflight failure modes", () => {
missingKeys: ["ANTHROPIC_API_KEY"],
providers: [],
runtime: "claude-code",
configuredKeys: new Set(),
});
const onDeployed = vi.fn();
@ -231,6 +261,7 @@ describe("useTemplateDeploy — modal lifecycle", () => {
missingKeys: ["ANTHROPIC_API_KEY"],
providers: [],
runtime: "claude-code",
configuredKeys: new Set(),
});
const onDeployed = vi.fn();
const { result, rerender } = renderHook(() =>
@ -265,6 +296,7 @@ describe("useTemplateDeploy — modal lifecycle", () => {
missingKeys: ["ANTHROPIC_API_KEY"],
providers: [],
runtime: "claude-code",
configuredKeys: new Set(),
});
const { result, rerender } = renderHook(() => useTemplateDeploy());
@ -287,6 +319,169 @@ describe("useTemplateDeploy — modal lifecycle", () => {
});
});
describe("useTemplateDeploy — multi-provider always-ask flow", () => {
// The user-reported bug: clicking a hermes template (which has
// multiple provider options) deployed silently when global env
// covered the API key, producing "No LLM provider configured" 500
// because the workspace booted with no explicit model. Fix:
// always open the picker for multi-provider templates so the
// user picks provider + model per workspace, even when keys are
// already saved.
function multiProviderTemplate(): Template {
return makeTemplate({
id: "hermes-template",
name: "Hermes",
runtime: "hermes",
model: "anthropic/claude-sonnet-4-5",
models: [
{ id: "minimax/MiniMax-M2.7", required_env: ["MINIMAX_API_KEY"] },
{ id: "anthropic/claude-sonnet-4-5", required_env: ["ANTHROPIC_API_KEY"] },
],
});
}
it("opens picker even when preflight.ok=true (≥2 providers)", async () => {
mockCheckDeploySecrets.mockResolvedValueOnce({
ok: true, // every key is in global env
missingKeys: [],
providers: [
{ id: "MINIMAX_API_KEY", label: "MiniMax", envVars: ["MINIMAX_API_KEY"] },
{ id: "ANTHROPIC_API_KEY", label: "Anthropic", envVars: ["ANTHROPIC_API_KEY"] },
],
runtime: "hermes",
configuredKeys: new Set(["MINIMAX_API_KEY", "ANTHROPIC_API_KEY"]),
});
const { result, rerender } = renderHook(() => useTemplateDeploy());
await act(async () => {
await result.current.deploy(multiProviderTemplate());
});
rerender();
render(<>{result.current.modal}</>);
expect(screen.getByTestId("missing-keys-modal")).toBeTruthy();
// Both global keys flowed into the modal as `configuredKeys` so
// entries can render as Saved without re-prompting.
expect(screen.getByTestId("modal-configured-size").textContent).toBe("2");
// Confirm POST has NOT fired yet — the user must explicitly
// confirm in the picker even though preflight passed.
expect(mockApiPost).not.toHaveBeenCalled();
// Title shifts to "Configure Workspace" since keys aren't missing.
expect(screen.getByTestId("modal-title").textContent).toBe(
"Configure Workspace",
);
});
it("threads template.models[].id as model suggestions + template.model as initial value", async () => {
mockCheckDeploySecrets.mockResolvedValueOnce({
ok: true,
missingKeys: [],
providers: [
{ id: "MINIMAX_API_KEY", label: "MiniMax", envVars: ["MINIMAX_API_KEY"] },
{ id: "ANTHROPIC_API_KEY", label: "Anthropic", envVars: ["ANTHROPIC_API_KEY"] },
],
runtime: "hermes",
configuredKeys: new Set(),
});
const { result, rerender } = renderHook(() => useTemplateDeploy());
await act(async () => {
await result.current.deploy(multiProviderTemplate());
});
rerender();
render(<>{result.current.modal}</>);
expect(screen.getByTestId("modal-model-suggestions").textContent).toBe(
"minimax/MiniMax-M2.7,anthropic/claude-sonnet-4-5",
);
expect(screen.getByTestId("modal-initial-model").textContent).toBe(
"anthropic/claude-sonnet-4-5",
);
});
it("POST /workspaces includes model when picker confirms with one", async () => {
mockCheckDeploySecrets.mockResolvedValueOnce({
ok: true,
missingKeys: [],
providers: [
{ id: "MINIMAX_API_KEY", label: "MiniMax", envVars: ["MINIMAX_API_KEY"] },
{ id: "ANTHROPIC_API_KEY", label: "Anthropic", envVars: ["ANTHROPIC_API_KEY"] },
],
runtime: "hermes",
configuredKeys: new Set(),
});
const { result, rerender } = renderHook(() => useTemplateDeploy());
await act(async () => {
await result.current.deploy(multiProviderTemplate());
});
rerender();
render(<>{result.current.modal}</>);
await act(async () => {
fireEvent.click(screen.getByTestId("modal-keys-added-with-model"));
await Promise.resolve();
await Promise.resolve();
});
expect(mockApiPost).toHaveBeenCalledWith(
"/workspaces",
expect.objectContaining({
template: "hermes-template",
model: "minimax/MiniMax-M2.7",
}),
);
});
it("single-provider template still skips picker when preflight.ok", async () => {
// Default preflight mock: ok=true, providers=[]. claude-code is
// single-provider — there's nothing to choose, so the picker
// SHOULD remain hidden. Regression guard: don't accidentally
// make every deploy require a click-through.
const onDeployed = vi.fn();
const { result } = renderHook(() => useTemplateDeploy({ onDeployed }));
await act(async () => {
await result.current.deploy(makeTemplate());
});
expect(mockApiPost).toHaveBeenCalledTimes(1);
expect(onDeployed).toHaveBeenCalledWith("ws-new");
});
it("empty configuredKeys (preflight defensive fallback) still opens picker", async () => {
// checkDeploySecrets falls back to an empty Set when the
// /settings/secrets endpoint errors — the modal must still
// open so the user isn't blocked, just with every entry
// rendered as input rather than Saved.
mockCheckDeploySecrets.mockResolvedValueOnce({
ok: true,
missingKeys: [],
providers: [
{ id: "MINIMAX_API_KEY", label: "MiniMax", envVars: ["MINIMAX_API_KEY"] },
{ id: "ANTHROPIC_API_KEY", label: "Anthropic", envVars: ["ANTHROPIC_API_KEY"] },
],
runtime: "hermes",
configuredKeys: new Set(),
});
const { result, rerender } = renderHook(() => useTemplateDeploy());
await act(async () => {
await result.current.deploy(multiProviderTemplate());
});
rerender();
render(<>{result.current.modal}</>);
expect(screen.getByTestId("missing-keys-modal")).toBeTruthy();
expect(screen.getByTestId("modal-configured-size").textContent).toBe("0");
expect(mockApiPost).not.toHaveBeenCalled();
});
});
describe("useTemplateDeploy — POST failure", () => {
it("POST rejection sets error and clears deploying", async () => {
mockApiPost.mockRejectedValueOnce(new Error("server 500"));

View File

@ -44,7 +44,11 @@ export interface UseTemplateDeployOptions {
/** Paired template + preflight result carried through the "user
* clicked deploy modal opens keys saved retry" loop. Named
* so the `useState` generic and any future signature change have
* a single place to track. */
* a single place to track. `preflight.configuredKeys` lets the
* modal mark pre-saved entries without re-prompting the
* template-deploy "always ask" flow surfaces the picker even when
* preflight.ok is true so the user can pick a different provider
* per workspace. */
interface MissingKeysInfo {
template: Template;
preflight: PreflightResult;
@ -81,9 +85,14 @@ export function useTemplateDeploy(
/** Actually execute the POST /workspaces call. Split from `deploy`
* so the "modal → keys added → retry" path can reuse it without
* re-running preflight (the user just proved the keys are now set). */
* re-running preflight (the user just proved the keys are now set).
*
* `model` (optional) is the user-picked model slug from the picker
* modal. When the template is multi-provider, hermes-style routing
* reads the slug prefix at install time to pick the upstream
* endpoint, so the slug must reach the workspace verbatim. */
const executeDeploy = useCallback(
async (template: Template) => {
async (template: Template, model?: string) => {
setDeploying(template.id);
setError(null);
try {
@ -98,6 +107,7 @@ export function useTemplateDeploy(
template: template.id,
tier: template.tier,
canvas: coords,
...(model ? { model } : {}),
});
onDeployed?.(ws.id);
} catch (e) {
@ -133,7 +143,18 @@ export function useTemplateDeploy(
setDeploying(null);
return;
}
if (!preflight.ok) {
// Open the picker modal whenever preflight failed OR the
// template offers ≥2 provider options. Multi-provider
// templates (e.g. hermes) need an explicit per-workspace
// pick — silently inheriting whichever global key matches
// produces the "No LLM provider configured" 500 because the
// adapter falls back to its compiled-in default model when
// the user hasn't asserted a slug. The "always ask" rule
// skips claude-code / langgraph-style single-provider
// templates where there's nothing to choose between.
const shouldShowPicker =
!preflight.ok || preflight.providers.length >= 2;
if (shouldShowPicker) {
setMissingKeysInfo({ template, preflight });
setDeploying(null);
return;
@ -147,19 +168,43 @@ export function useTemplateDeploy(
// (it's placed inline in JSX), and useCallback's deps would
// invalidate on every state change, making the memoisation a wash.
// Plain ReactNode is simpler and equally performant.
const isMultiProvider = (missingKeysInfo?.preflight.providers.length ?? 0) >= 2;
// Suggestions for the model field — pull declared model ids from the
// template. Templates without `models` declared (e.g. claude-code)
// pass [] which suppresses the model field entirely.
const modelSuggestions =
missingKeysInfo?.template.models?.map((m) => m.id) ?? [];
// Pre-fill the model input with the template's default `model` so
// confirming without changing it preserves today's behaviour.
const initialModel = missingKeysInfo?.template.model;
// When the user has keys configured (preflight.ok) we re-purpose the
// modal as a "confirm provider/model" prompt — adjust copy
// accordingly so it doesn't claim keys are missing.
const allConfigured = missingKeysInfo?.preflight.ok ?? false;
const modalTitle = allConfigured
? "Configure Workspace"
: undefined;
const modalDescription = allConfigured
? "Pick the provider and model for this workspace. Saved API keys are reused automatically."
: undefined;
const modal: ReactNode = (
<MissingKeysModal
open={!!missingKeysInfo}
missingKeys={missingKeysInfo?.preflight.missingKeys ?? []}
providers={missingKeysInfo?.preflight.providers ?? []}
runtime={missingKeysInfo?.preflight.runtime ?? ""}
onKeysAdded={() => {
configuredKeys={missingKeysInfo?.preflight.configuredKeys}
modelSuggestions={isMultiProvider ? modelSuggestions : undefined}
initialModel={isMultiProvider ? initialModel : undefined}
title={modalTitle}
description={modalDescription}
onKeysAdded={(model?: string) => {
if (missingKeysInfo) {
const template = missingKeysInfo.template;
setMissingKeysInfo(null);
// Intentional fire-and-forget — executeDeploy manages
// its own error state via setError.
void executeDeploy(template);
void executeDeploy(template, model);
}
}}
onCancel={() => setMissingKeysInfo(null)}

View File

@ -244,5 +244,26 @@ describe("checkDeploySecrets", () => {
const result = await checkDeploySecrets(LANGGRAPH);
expect(result.ok).toBe(false);
expect(result.missingKeys).toEqual(["OPENAI_API_KEY"]);
// Empty Set on fetch failure — useTemplateDeploy relies on this
// so the picker still opens with every entry rendered as input.
expect(result.configuredKeys).toEqual(new Set());
});
it("surfaces configuredKeys (has_value=true entries only) so callers skip a second fetch", async () => {
(global.fetch as ReturnType<typeof vi.fn>).mockResolvedValueOnce({
ok: true,
json: () =>
Promise.resolve([
{ key: "ANTHROPIC_API_KEY", has_value: true, created_at: "", updated_at: "" },
{ key: "OPENROUTER_API_KEY", has_value: false, created_at: "", updated_at: "" },
{ key: "RANDOM_OTHER_KEY", has_value: true, created_at: "", updated_at: "" },
]),
} as Response);
const result = await checkDeploySecrets(HERMES);
// Only has_value=true entries belong in the set.
expect(result.configuredKeys).toEqual(
new Set(["ANTHROPIC_API_KEY", "RANDOM_OTHER_KEY"]),
);
});
});

View File

@ -91,6 +91,12 @@ export interface PreflightResult {
* required (AllKeysModal renders the N envVars inline). */
providers: ProviderChoice[];
runtime: string;
/** Set of env var names already configured (i.e. `has_value: true`) at
* the relevant scope (workspace if `workspaceId` was passed, otherwise
* global). Surfaced so callers can mark pre-saved entries in the
* picker without making a second `/settings/secrets` round trip.
* Empty Set on secrets-endpoint failure (treated as "nothing set"). */
configuredKeys: Set<string>;
}
/* ---------- Provider options ---------- */
@ -235,7 +241,13 @@ export async function checkDeploySecrets(
if (providers.length === 0) {
// Template declares no env requirements — nothing to preflight.
return { ok: true, missingKeys: [], providers: [], runtime };
return {
ok: true,
missingKeys: [],
providers: [],
runtime,
configuredKeys: new Set(),
};
}
let configured: Set<string>;
@ -254,7 +266,13 @@ export async function checkDeploySecrets(
}
if (findSatisfiedProvider(providers, configured)) {
return { ok: true, missingKeys: [], providers, runtime };
return {
ok: true,
missingKeys: [],
providers,
runtime,
configuredKeys: configured,
};
}
// Nothing configured — surface every candidate env var so the modal
@ -262,5 +280,11 @@ export async function checkDeploySecrets(
const missingKeys = Array.from(
new Set(providers.flatMap((p) => p.envVars)),
);
return { ok: false, missingKeys, providers, runtime };
return {
ok: false,
missingKeys,
providers,
runtime,
configuredKeys: configured,
};
}