From 517bd0efc562e6789c8e4901556cbc2ed9b89583 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Fri, 1 May 2026 11:19:17 -0700 Subject: [PATCH] feat(canvas+workspace-server): data-driven Provider dropdown (#199) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Option B PR-5. Canvas Config tab now exposes a Provider override input that's adapter-driven from each runtime's template — no hardcoded provider list in the canvas. PUT /workspaces/:id/provider on Save when dirty; auto-restart suppression to avoid double-restart with the model handler's own restart. The dropdown's suggestion list comes from /templates → runtime_config.providers (the field added in molecule-ai-workspace-template-hermes PR #31). For templates that haven't migrated to the explicit providers list yet, suggestions derive from model[].id slug prefixes — still adapter-driven, just inferred. This keeps existing templates working while platform team migrates them one at a time. workspace-server changes: - Add Providers []string field to templateSummary JSON - Parse runtime_config.providers in /templates handler - 2 new tests pin the surfacing + omitempty behavior canvas changes: - Remove hardcoded PROVIDER_SUGGESTIONS constant - Add provider/originalProvider state + PUT-on-save logic - Add deriveProvidersFromModels() fallback helper - Wire RuntimeOption.providers from /templates response - 8 new tests pin the behavior end-to-end Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/tabs/ConfigTab.tsx | 192 ++++++++-- .../__tests__/ConfigTab.provider.test.tsx | 332 ++++++++++++++++++ .../internal/handlers/templates.go | 12 + .../internal/handlers/templates_test.go | 111 ++++++ 4 files changed, 627 insertions(+), 20 deletions(-) create mode 100644 canvas/src/components/tabs/__tests__/ConfigTab.provider.test.tsx diff --git a/canvas/src/components/tabs/ConfigTab.tsx b/canvas/src/components/tabs/ConfigTab.tsx index e1227d67..f46ff538 100644 --- a/canvas/src/components/tabs/ConfigTab.tsx +++ b/canvas/src/components/tabs/ConfigTab.tsx @@ -100,6 +100,42 @@ interface RuntimeOption { value: string; label: string; models: ModelSpec[]; + // providers is the declarative provider list each template ships in + // its config.yaml under runtime_config.providers. The /templates API + // surfaces it (workspace-server templates.go) so canvas stays + // adapter-driven: hermes ships ~20 slugs, claude-code ships + // ["anthropic"], gemini-cli ships ["gemini"], etc. Empty list → + // canvas falls back to deriving unique vendor prefixes from + // models[].id (still adapter-driven, just inferred). + providers: string[]; +} + +// deriveProvidersFromModels — when a template doesn't ship an explicit +// providers list, infer suggestions from the vendor prefixes of its +// model slugs. e.g. ["anthropic:claude-opus-4-7", "openai:gpt-4o", +// "anthropic:claude-sonnet-4-5"] → ["anthropic", "openai"]. +// +// This keeps the dropdown adapter-driven for older templates that +// haven't migrated to the explicit `providers:` field yet, AND +// continues to be a useful fallback for any future runtime whose +// derive-provider semantics happen to match the slug prefix. +function deriveProvidersFromModels(models: ModelSpec[]): string[] { + const seen = new Set(); + const out: string[] = []; + for (const m of models) { + if (!m.id) continue; + // Both ":" (anthropic:claude-opus-4-7) and "/" (nousresearch/hermes-4-70b) + // are valid vendor separators in our slug taxonomy. Take whichever + // appears first and split there. + const sep = m.id.match(/[:/]/)?.index ?? -1; + if (sep <= 0) continue; + const vendor = m.id.slice(0, sep); + if (!seen.has(vendor)) { + seen.add(vendor); + out.push(vendor); + } + } + return out; } // Fallback used when /templates can't be fetched (offline, older backend). @@ -118,14 +154,14 @@ interface RuntimeOption { const RUNTIMES_WITH_OWN_CONFIG = new Set(["external"]); const FALLBACK_RUNTIME_OPTIONS: RuntimeOption[] = [ - { value: "", label: "LangGraph (default)", models: [] }, - { value: "claude-code", label: "Claude Code", models: [] }, - { value: "crewai", label: "CrewAI", models: [] }, - { value: "autogen", label: "AutoGen", models: [] }, - { value: "deepagents", label: "DeepAgents", models: [] }, - { value: "openclaw", label: "OpenClaw", models: [] }, - { value: "hermes", label: "Hermes", models: [] }, - { value: "gemini-cli", label: "Gemini CLI", models: [] }, + { value: "", label: "LangGraph (default)", models: [], providers: [] }, + { value: "claude-code", label: "Claude Code", models: [], providers: [] }, + { value: "crewai", label: "CrewAI", models: [], providers: [] }, + { value: "autogen", label: "AutoGen", models: [], providers: [] }, + { value: "deepagents", label: "DeepAgents", models: [], providers: [] }, + { value: "openclaw", label: "OpenClaw", models: [], providers: [] }, + { value: "hermes", label: "Hermes", models: [], providers: [] }, + { value: "gemini-cli", label: "Gemini CLI", models: [], providers: [] }, ]; export function ConfigTab({ workspaceId }: Props) { @@ -138,6 +174,17 @@ export function ConfigTab({ workspaceId }: Props) { const [rawMode, setRawMode] = useState(false); const [rawDraft, setRawDraft] = useState(""); const [runtimeOptions, setRuntimeOptions] = useState(FALLBACK_RUNTIME_OPTIONS); + // Provider override (Option B PR-5): stored separately from config.yaml + // because the value lives in workspace_secrets (encrypted), not in the + // platform-managed config.yaml. The two endpoints are GET/PUT + // /workspaces/:id/provider on workspace-server (handlers/secrets.go). + // Empty = "auto-derive from model slug prefix" — pre-Option-B behavior + // and what most users want. Setting to a non-empty value writes + // LLM_PROVIDER into workspace_secrets and triggers an auto-restart so + // the workspace boots with the new provider in env (and via CP user- + // data, written into /configs/config.yaml on next provision too). + const [provider, setProvider] = useState(""); + const [originalProvider, setOriginalProvider] = useState(""); const successTimerRef = useRef>(undefined); useEffect(() => { @@ -168,6 +215,22 @@ export function ConfigTab({ workspaceId }: Props) { wsMetadataModel = (m.model || "").trim(); } catch { /* non-fatal */ } + // Load explicit provider override (Option B PR-5). Endpoint returns + // {provider: "", source: "default"} when no override is set, so the + // empty string is the legitimate "auto-derive" signal — don't treat + // it as a load error. Non-fatal: an older workspace-server that + // predates PR-2 returns 404 here; the form falls back to "" and + // Save just won't PUT the provider field. + try { + const p = await api.get<{ provider?: string }>(`/workspaces/${workspaceId}/provider`); + const loadedProvider = (p.provider || "").trim(); + setProvider(loadedProvider); + setOriginalProvider(loadedProvider); + } catch { + setProvider(""); + setOriginalProvider(""); + } + try { const res = await api.get<{ content: string }>(`/workspaces/${workspaceId}/files/config.yaml`); const parsed = parseYaml(res.content); @@ -209,11 +272,11 @@ export function ConfigTab({ workspaceId }: Props) { useEffect(() => { let cancelled = false; - api.get>("/templates") + api.get>("/templates") .then((rows) => { if (cancelled || !Array.isArray(rows)) return; const byRuntime = new Map(); - byRuntime.set("", { value: "", label: "LangGraph (default)", models: [] }); + byRuntime.set("", { value: "", label: "LangGraph (default)", models: [], providers: [] }); for (const r of rows) { const v = (r.runtime || "").trim(); if (!v || v === "langgraph") continue; @@ -221,8 +284,9 @@ export function ConfigTab({ workspaceId }: Props) { // one with the richer models list is probably newer. const existing = byRuntime.get(v); const models = Array.isArray(r.models) ? r.models : []; + const providers = Array.isArray(r.providers) ? r.providers : []; if (!existing || models.length > existing.models.length) { - byRuntime.set(v, { value: v, label: r.name || v, models }); + byRuntime.set(v, { value: v, label: r.name || v, models, providers }); } } if (byRuntime.size > 1) setRuntimeOptions(Array.from(byRuntime.values())); @@ -234,6 +298,16 @@ export function ConfigTab({ workspaceId }: Props) { // Models + env hints for the currently-selected runtime. const selectedRuntime = runtimeOptions.find((o) => o.value === (config.runtime || "")) ?? null; const availableModels: ModelSpec[] = selectedRuntime?.models ?? []; + // Provider suggestions: prefer the runtime's declarative providers + // list (sourced from its template config.yaml runtime_config.providers + // and surfaced via /templates), fall back to deriving from model slug + // prefixes when the template hasn't migrated to the explicit field + // yet. Either way the data flows from the adapter — no hardcoded + // canvas-side enum. + const providerSuggestions: string[] = + (selectedRuntime?.providers && selectedRuntime.providers.length > 0) + ? selectedRuntime.providers + : deriveProvidersFromModels(availableModels); const currentModelId = config.runtime_config?.model || config.model || ""; const currentModelSpec = availableModels.find((m) => m.id === currentModelId) ?? null; @@ -334,6 +408,24 @@ export function ConfigTab({ workspaceId }: Props) { } } + // Provider override save (Option B PR-5). PUT only when the user + // changed the dropdown — otherwise an unrelated Save (e.g. tier + // edit) would re-write the provider unchanged and the server- + // side auto-restart would fire on every Save, costing the user a + // ~30s reboot for a no-op change. Server endpoint accepts an + // empty string to clear the override (deletes the + // workspace_secrets row); we forward whatever the form holds. + let providerSaveError: string | null = null; + const providerChanged = provider !== originalProvider; + if (providerChanged) { + try { + await api.put(`/workspaces/${workspaceId}/provider`, { provider }); + setOriginalProvider(provider); + } catch (e) { + providerSaveError = e instanceof Error ? e.message : "Provider update was rejected"; + } + } + setOriginalYaml(content); if (rawMode) { const parsed = parseYaml(content); @@ -341,16 +433,30 @@ export function ConfigTab({ workspaceId }: Props) { } else { setRawDraft(content); } - if (restart) { + // SetProvider on the server already triggers an auto-restart for + // the workspace whenever the value actually changed (see + // workspace-server/internal/handlers/secrets.go:SetProvider). If + // the user also clicked Save+Restart we'd kick off a SECOND + // restart here and the two would race in the canvas store — + // suppress the redundant call and rely on the server-side one. + const providerWillAutoRestart = providerChanged && !providerSaveError; + if (restart && !providerWillAutoRestart) { await useCanvasStore.getState().restartWorkspace(workspaceId); - } else { - useCanvasStore.getState().updateNodeData(workspaceId, { needsRestart: true }); + } else if (!restart) { + useCanvasStore.getState().updateNodeData(workspaceId, { needsRestart: !providerWillAutoRestart }); } - if (modelSaveError) { - // Partial-save UX: surface the model rejection instead of - // showing "Saved" — the user would otherwise watch the model - // field revert on next reload with no explanation. - setError(`Other fields saved, but model update failed: ${modelSaveError}`); + // Aggregate partial-save errors. Both modelSaveError and + // providerSaveError describe rejected updates from independent + // endpoints — show whichever fired so the user knows which + // field reverts on next reload (otherwise they'd see "Saved" and + // be confused why Provider snapped back). + const partialError = providerSaveError + ? `Other fields saved, but provider update failed: ${providerSaveError}` + : modelSaveError + ? `Other fields saved, but model update failed: ${modelSaveError}` + : null; + if (partialError) { + setError(partialError); } else { setSuccess(true); clearTimeout(successTimerRef.current); @@ -371,7 +477,8 @@ export function ConfigTab({ workspaceId }: Props) { const taskBudgetId = useId(); const sandboxBackendId = useId(); - const isDirty = rawMode ? rawDraft !== originalYaml : toYaml(config) !== originalYaml; + const providerDirty = provider !== originalProvider; + const isDirty = (rawMode ? rawDraft !== originalYaml : toYaml(config) !== originalYaml) || providerDirty; if (loading) { return
Loading config...
; @@ -518,6 +625,51 @@ export function ConfigTab({ workspaceId }: Props) { )} + {/* Provider override (Option B PR-5). Free-text combobox so + operators can use any of the 30+ slugs hermes-agent's + derive-provider.sh recognizes — the suggestion list is + a hint, not a constraint. Empty = "auto-derive from + model slug prefix" which is correct for the common case + (model "anthropic:claude-opus-4-7" → provider derived + as "anthropic"). The override is needed when the model + alias has no clean vendor prefix (e.g. hermes default + "nousresearch/hermes-4-70b" → derive returns empty → + hermes errors "No LLM provider configured"). */} +
+ + 0 ? `${runtimeId}-providers` : undefined} + value={provider} + onChange={(e) => setProvider(e.target.value.trim())} + placeholder={ + providerSuggestions.length > 0 + ? `e.g. ${providerSuggestions.slice(0, 3).join(", ")} (empty = auto-derive)` + : "empty = auto-derive from model slug" + } + aria-label="LLM provider override" + data-testid="provider-input" + className="w-full bg-zinc-800 border border-zinc-700 rounded px-2 py-1 text-xs text-zinc-200 font-mono focus:outline-none focus:border-blue-500" + /> + {providerSuggestions.length > 0 && ( + + {providerSuggestions.map((p) => ( + + )} + {provider && provider !== originalProvider && ( +

+ Provider change → workspace will auto-restart on Save. +

+ )} +
({ + api: { + get: (path: string) => apiGet(path), + patch: (path: string, body: unknown) => apiPatch(path, body), + put: (path: string, body: unknown) => apiPut(path, body), + post: vi.fn(), + del: vi.fn(), + }, +})); + +vi.mock("@/store/canvas", () => ({ + useCanvasStore: Object.assign( + (selector: (s: unknown) => unknown) => selector({ restartWorkspace: vi.fn(), updateNodeData: vi.fn() }), + { getState: () => ({ restartWorkspace: vi.fn(), updateNodeData: vi.fn() }) }, + ), +})); + +vi.mock("../AgentCardSection", () => ({ + AgentCardSection: () =>
, +})); + +import { ConfigTab } from "../ConfigTab"; + +// wireApi — same shape as ConfigTab.hermes.test.tsx, extended with the +// /provider endpoint. Each test sets `providerValue` to the value the +// GET endpoint returns; "missing" means the endpoint rejects (older +// workspace-server pre-PR-2 — must not crash the tab). +function wireApi(opts: { + workspaceRuntime?: string; + workspaceModel?: string; + configYamlContent?: string | null; + templates?: Array<{ id: string; name?: string; runtime?: string; models?: unknown[]; providers?: string[] }>; + providerValue?: string | "missing"; +}) { + apiGet.mockImplementation((path: string) => { + if (path === `/workspaces/ws-test`) { + return Promise.resolve({ runtime: opts.workspaceRuntime ?? "" }); + } + if (path === `/workspaces/ws-test/model`) { + return Promise.resolve({ model: opts.workspaceModel ?? "" }); + } + if (path === `/workspaces/ws-test/provider`) { + if (opts.providerValue === "missing") { + return Promise.reject(new Error("404")); + } + return Promise.resolve({ provider: opts.providerValue ?? "", source: opts.providerValue ? "workspace_secrets" : "default" }); + } + if (path === `/workspaces/ws-test/files/config.yaml`) { + if (opts.configYamlContent === null) return Promise.reject(new Error("not found")); + return Promise.resolve({ content: opts.configYamlContent ?? "" }); + } + if (path === "/templates") { + return Promise.resolve(opts.templates ?? []); + } + return Promise.reject(new Error(`unmocked api.get: ${path}`)); + }); +} + +beforeEach(() => { + apiGet.mockReset(); + apiPatch.mockReset(); + apiPut.mockReset(); +}); + +describe("ConfigTab — Provider override (Option B PR-5)", () => { + // Empty provider on load is the legitimate default ("auto-derive + // from model slug prefix"), NOT an error. The endpoint returning + // {provider: "", source: "default"} is the documented happy-path + // shape — if the form treated that as "load failed" we'd lose the + // ability to render the input at all on fresh workspaces. + it("renders an empty Provider input when no override is set", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "nousresearch/hermes-4-70b", + configYamlContent: "name: ws\nruntime: hermes\n", + providerValue: "", + }); + + render(); + const input = await screen.findByTestId("provider-input"); + expect((input as HTMLInputElement).value).toBe(""); + }); + + // Pre-existing override loads back into the field on mount. Without + // this, an operator who set provider=openrouter yesterday would see + // the field blank today, conclude the value didn't stick, and + // re-save — the resulting PUT-with-same-value would auto-restart + // the workspace for nothing. + it("loads an existing provider override from the server", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "nousresearch/hermes-4-70b", + configYamlContent: "name: ws\nruntime: hermes\n", + providerValue: "openrouter", + }); + + render(); + const input = await screen.findByTestId("provider-input"); + await waitFor(() => expect((input as HTMLInputElement).value).toBe("openrouter")); + }); + + // Old workspace-server (pre-PR-2) returns a 404 on /provider. The + // tab must keep loading — the fallback is "" (auto-derive), same as + // a fresh workspace. + it("falls back to empty provider when the endpoint is missing", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "nousresearch/hermes-4-70b", + configYamlContent: "name: ws\nruntime: hermes\n", + providerValue: "missing", + }); + + render(); + const input = await screen.findByTestId("provider-input"); + expect((input as HTMLInputElement).value).toBe(""); + // Tab should be fully rendered, not stuck in loading or error state. + expect(screen.queryByText(/Loading config/i)).toBeNull(); + }); + + // Setting a value + Save must PUT to the right endpoint with the + // right body shape. Server-side handler (workspace-server + // handlers/secrets.go:SetProvider) reads body.provider — any other + // key gets silently ignored and the workspace_secrets row stays + // unset. This regression would manifest as "Save → Restart → + // workspace still says No LLM provider configured." + it("PUTs the new provider to /workspaces/:id/provider on Save", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "nousresearch/hermes-4-70b", + configYamlContent: "name: ws\nruntime: hermes\n", + providerValue: "", + }); + apiPut.mockResolvedValue({ status: "saved", provider: "anthropic" }); + + render(); + const input = await screen.findByTestId("provider-input"); + + fireEvent.change(input, { target: { value: "anthropic" } }); + expect((input as HTMLInputElement).value).toBe("anthropic"); + + const saveBtn = screen.getByRole("button", { name: /^save$/i }); + fireEvent.click(saveBtn); + + await waitFor(() => { + const providerCalls = apiPut.mock.calls.filter(([path]) => path === "/workspaces/ws-test/provider"); + expect(providerCalls.length).toBe(1); + expect(providerCalls[0][1]).toEqual({ provider: "anthropic" }); + }); + }); + + // No-change Save must NOT PUT /provider. The server-side SetProvider + // auto-restarts the workspace on every successful PUT — re-writing + // an unchanged value would cost the user a ~30s reboot every time + // they tweak some other field. + it("does not PUT /provider when the value is unchanged", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "nousresearch/hermes-4-70b", + configYamlContent: "name: ws\nruntime: hermes\ntier: 2\n", + providerValue: "openrouter", + }); + apiPut.mockResolvedValue({}); + + render(); + await screen.findByTestId("provider-input"); + + // Click Save without touching the provider field. Trigger another + // dirty-marker (tier change) so Save is enabled — the test is + // about NOT touching /provider, not about Save being disabled. + const tierSelect = screen.getByLabelText(/tier/i) as HTMLSelectElement; + fireEvent.change(tierSelect, { target: { value: "3" } }); + + const saveBtn = screen.getByRole("button", { name: /^save$/i }); + fireEvent.click(saveBtn); + + await waitFor(() => { + // Some PUT(s) may fire (e.g. /model). Just assert /provider is NOT among them. + const providerCalls = apiPut.mock.calls.filter(([path]) => path === "/workspaces/ws-test/provider"); + expect(providerCalls.length).toBe(0); + }); + }); + + // The dropdown's suggestion list MUST come from the runtime's own + // template (via /templates → runtime_config.providers), not a + // hardcoded canvas-side enum. This is the "Native + pluggable + // runtime" invariant: a new runtime declaring its own provider + // taxonomy in its config.yaml gets a working dropdown without ANY + // canvas-side change. + // + // Pinned by checking that suggestions surfaced in the datalist + // exactly mirror what the templates endpoint returned for the + // matching runtime. If a future contributor reintroduces a + // PROVIDER_SUGGESTIONS-style hardcoded list and the datalist + // contents don't follow the template, this test fails. + it("populates the provider datalist from the matched runtime's templates entry", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "nousresearch/hermes-4-70b", + configYamlContent: "name: ws\nruntime: hermes\n", + providerValue: "", + templates: [ + { + id: "hermes", + name: "Hermes", + runtime: "hermes", + models: [], + // The provider list every runtime adapter ships in its own + // config.yaml. Canvas must surface THIS, not its own list. + providers: ["nous", "openrouter", "anthropic", "minimax-cn"], + }, + ], + }); + + render(); + const input = await screen.findByTestId("provider-input"); + const listId = (input as HTMLInputElement).getAttribute("list"); + expect(listId).toBeTruthy(); + await waitFor(() => { + const datalist = document.getElementById(listId!); + expect(datalist).not.toBeNull(); + const optionValues = Array.from(datalist!.querySelectorAll("option")).map( + (o) => (o as HTMLOptionElement).value, + ); + // Order matters — most-common-first is part of the contract so + // the demo flow lands on a working choice without scrolling. + expect(optionValues).toEqual(["nous", "openrouter", "anthropic", "minimax-cn"]); + }); + }); + + // Fallback path: when a template hasn't migrated to the explicit + // `providers:` field yet, suggestions are derived from model slug + // prefixes. Still adapter-driven (the slugs come from the template's + // `models:` list), just inferred. This keeps existing templates + // working while the platform team migrates them one at a time. + it("falls back to model-slug prefixes when the runtime ships no providers list", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "anthropic:claude-opus-4-7", + configYamlContent: "name: ws\nruntime: hermes\n", + providerValue: "", + templates: [ + { + id: "hermes", + name: "Hermes", + runtime: "hermes", + models: [ + { id: "anthropic:claude-opus-4-7" }, + { id: "openai:gpt-4o" }, + { id: "anthropic:claude-sonnet-4-5" }, // dup vendor — must dedupe + { id: "nousresearch/hermes-4-70b" }, // "/" separator + ], + // No `providers:` field → fallback derivation kicks in. + }, + ], + }); + + render(); + const input = await screen.findByTestId("provider-input"); + const listId = (input as HTMLInputElement).getAttribute("list"); + expect(listId).toBeTruthy(); + await waitFor(() => { + const datalist = document.getElementById(listId!); + const optionValues = Array.from(datalist!.querySelectorAll("option")).map( + (o) => (o as HTMLOptionElement).value, + ); + // Order = first-appearance from models[]; dedup keeps anthropic + // once even though two model slugs use it. + expect(optionValues).toEqual(["anthropic", "openai", "nousresearch"]); + }); + }); + + // Empty string is a legitimate save target — it clears the override + // (the server-side endpoint deletes the workspace_secrets row). + // Operators who picked "anthropic" yesterday and want to revert to + // auto-derive today should be able to do so by clearing the field + // and clicking Save. Without this PUT path, the only way to clear + // would be a direct DB edit. + it("PUTs an empty string when the operator clears a previously-set provider", async () => { + wireApi({ + workspaceRuntime: "hermes", + workspaceModel: "anthropic:claude-opus-4-7", + configYamlContent: "name: ws\nruntime: hermes\n", + providerValue: "openrouter", + }); + apiPut.mockResolvedValue({ status: "cleared" }); + + render(); + const input = await screen.findByTestId("provider-input"); + await waitFor(() => expect((input as HTMLInputElement).value).toBe("openrouter")); + + fireEvent.change(input, { target: { value: "" } }); + + const saveBtn = screen.getByRole("button", { name: /^save$/i }); + fireEvent.click(saveBtn); + + await waitFor(() => { + const providerCalls = apiPut.mock.calls.filter(([path]) => path === "/workspaces/ws-test/provider"); + expect(providerCalls.length).toBe(1); + expect(providerCalls[0][1]).toEqual({ provider: "" }); + }); + }); +}); diff --git a/workspace-server/internal/handlers/templates.go b/workspace-server/internal/handlers/templates.go index e33c06d6..1279a524 100644 --- a/workspace-server/internal/handlers/templates.go +++ b/workspace-server/internal/handlers/templates.go @@ -59,6 +59,16 @@ type templateSummary struct { // preflight uses this as the fallback provider when `models` is empty // so provider picker stays data-driven instead of hardcoded in the UI. RequiredEnv []string `json:"required_env,omitempty"` + // Providers is the runtime's own list of supported provider slugs, + // sourced from runtime_config.providers in the template's config.yaml. + // The canvas Config tab surfaces this as the Provider override + // dropdown (Option B PR-5). Data-driven so each runtime owns its own + // taxonomy — hermes-agent supports 20+ providers; claude-code only + // "anthropic"; gemini-cli only "gemini" — and a future runtime with + // a different vendor list doesn't need a canvas edit. Empty list → + // canvas falls back to deriving suggestions from `models[].id` slug + // prefixes (still adapter-driven, just inferred). + Providers []string `json:"providers,omitempty"` Skills []string `json:"skills"` SkillCount int `json:"skill_count"` // ProvisionTimeoutSeconds lets a slow runtime declare its expected @@ -100,6 +110,7 @@ func (h *TemplatesHandler) List(c *gin.Context) { Model string `yaml:"model"` Models []modelSpec `yaml:"models"` RequiredEnv []string `yaml:"required_env"` + Providers []string `yaml:"providers"` ProvisionTimeoutSeconds int `yaml:"provision_timeout_seconds"` } `yaml:"runtime_config"` } @@ -122,6 +133,7 @@ func (h *TemplatesHandler) List(c *gin.Context) { Model: model, Models: raw.RuntimeConfig.Models, RequiredEnv: raw.RuntimeConfig.RequiredEnv, + Providers: raw.RuntimeConfig.Providers, Skills: raw.Skills, SkillCount: len(raw.Skills), ProvisionTimeoutSeconds: raw.RuntimeConfig.ProvisionTimeoutSeconds, diff --git a/workspace-server/internal/handlers/templates_test.go b/workspace-server/internal/handlers/templates_test.go index e40c6b16..6b85715c 100644 --- a/workspace-server/internal/handlers/templates_test.go +++ b/workspace-server/internal/handlers/templates_test.go @@ -197,6 +197,117 @@ skills: [] } } +// TestTemplatesList_SurfacesProviders pins the Option B PR-5 wiring: +// /templates must echo runtime_config.providers from the template's +// config.yaml into the JSON response. Canvas reads this list to +// populate the Provider override dropdown WITHOUT hardcoding any +// provider taxonomy on the frontend — that's the "data-driven from +// adapter" invariant. +// +// If a future yaml-tag rename or struct edit drops the field, every +// runtime would silently fall back to model-prefix derivation. For +// hermes specifically (default model has no clean prefix), that +// degrades the dropdown to empty and reintroduces the "No LLM +// provider configured" UX gap from 2026-05-01. +func TestTemplatesList_SurfacesProviders(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + + tmpDir := t.TempDir() + tmplDir := filepath.Join(tmpDir, "hermes-prov") + if err := os.MkdirAll(tmplDir, 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + configYaml := `name: Hermes +description: test +tier: 2 +runtime: hermes +runtime_config: + model: nousresearch/hermes-4-70b + providers: + - nous + - openrouter + - anthropic +skills: [] +` + if err := os.WriteFile(filepath.Join(tmplDir, "config.yaml"), []byte(configYaml), 0644); err != nil { + t.Fatalf("write: %v", err) + } + + handler := NewTemplatesHandler(tmpDir, nil) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/templates", nil) + handler.List(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", w.Code) + } + var resp []templateSummary + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("parse: %v", err) + } + if len(resp) != 1 { + t.Fatalf("expected 1 template, got %d", len(resp)) + } + got := resp[0] + want := []string{"nous", "openrouter", "anthropic"} + if len(got.Providers) != len(want) { + t.Fatalf("Providers: want %v, got %v", want, got.Providers) + } + for i, p := range want { + if got.Providers[i] != p { + t.Errorf("Providers[%d]: want %q, got %q", i, p, got.Providers[i]) + } + } + + // Cross-check the JSON wire shape directly — canvas reads the field + // as `providers` (lowercase) and a struct-tag rename here would + // break consumers without surfacing in the typed assertions above. + if !strings.Contains(w.Body.String(), `"providers":["nous","openrouter","anthropic"]`) { + t.Errorf("response missing providers JSON field: %s", w.Body.String()) + } +} + +// TestTemplatesList_OmitsProvidersWhenAbsent pins the omitempty +// behavior — older templates that haven't migrated to +// runtime_config.providers yet must NOT emit `providers: null` (which +// would break canvas's array-typed parser). A template that simply +// omits the field stays absent in the response and canvas falls back +// to deriving suggestions from model-slug prefixes. +func TestTemplatesList_OmitsProvidersWhenAbsent(t *testing.T) { + setupTestDB(t) + setupTestRedis(t) + + tmpDir := t.TempDir() + tmplDir := filepath.Join(tmpDir, "no-prov") + if err := os.MkdirAll(tmplDir, 0755); err != nil { + t.Fatalf("mkdir: %v", err) + } + configYaml := `name: Legacy +runtime: langgraph +runtime_config: + model: anthropic:claude-opus-4-7 +skills: [] +` + if err := os.WriteFile(filepath.Join(tmplDir, "config.yaml"), []byte(configYaml), 0644); err != nil { + t.Fatalf("write: %v", err) + } + + handler := NewTemplatesHandler(tmpDir, nil) + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + c.Request = httptest.NewRequest("GET", "/templates", nil) + handler.List(c) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d", w.Code) + } + if strings.Contains(w.Body.String(), `"providers":`) { + t.Errorf("response should omit providers when template has none, got: %s", w.Body.String()) + } +} + func TestTemplatesList_LegacyTopLevelModel(t *testing.T) { // Older templates (pre-runtime_config) declared `model:` at the top level. // The /templates endpoint should keep surfacing those for backward compat.