molecule-ai/molecule-core
51
0
Fork 2
Code Issues 160 Pull Requests 162 Actions 7 Packages Projects Releases Wiki Activity

P4 closure follow-up internal#718: retire LLM_PROVIDER + PUT/GET /provider + deriveProviderFromModelSlug (core; BEHAVIOR-AFFECTING; NOT MERGED) #1984

Merged
hongming merged 1 commits from feat/internal-718-p4-followup-llm-provider-removal into main 2026-05-28 04:46:28 +00:00
Conversation 2 Commits 1 Files Changed 15
+511 -1934
15 changed files with 511 additions and 1934 deletions
Download Patch File Download Diff File Expand all files Collapse all files
canvas/src/components/tabs/ConfigTab.tsx
+57 -82
View File
@@ -355,15 +355,24 @@ export function ConfigTab({ workspaceId }: Props) {
const [rawMode, setRawMode] = useState(false);
const [rawDraft, setRawDraft] = useState("");
const [runtimeOptions, setRuntimeOptions] = useState<RuntimeOption[]>(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).
// internal#718 P4 closure: the explicit provider override
// (LLM_PROVIDER workspace_secret, surfaced via GET/PUT
// /workspaces/:id/provider) has been RETIRED. The provider is
// derived at every decision point from (runtime, model) via the
// registry — no stored row remains. The `provider` / `originalProvider`
// state and the provider dropdown survive in this component for
// backwards-compat (display only) but are no longer persisted:
// - loadConfig no longer GETs /workspaces/:id/provider (the
// endpoint returns 410 Gone). The state initializes to ""
// and stays there.
// - handleSave no longer PUTs /workspaces/:id/provider.
// - The dropdown still updates the local `provider` state so the
// user can preview the derived value; the value never leaves
// the browser.
// This is the canvas-side complement to the backend retirement of
// SetProvider/GetProvider/setProviderSecret. Older canvases that
// still call PUT /provider hit the 410 Gone with a structured
// PROVIDER_ENDPOINT_RETIRED code — loud failure, no silent miss.
const [provider, setProvider] = useState("");
const [originalProvider, setOriginalProvider] = useState("");
// Track the model the form first rendered, so handleSave can detect
@@ -414,26 +423,23 @@ export function ConfigTab({ workspaceId }: Props) {
//
// See GH #1894 for the workspace-row-as-source-of-truth rationale
// that motivated splitting from a single config.yaml read.
const [wsRes, modelRes, providerRes] = await Promise.all([
// internal#718 P4 closure: the GET /workspaces/:id/provider leg is
// RETIRED — the endpoint returns 410 Gone. Provider is now derived
// from (runtime, model) via the registry; no stored value exists
// to load. Always seed the local state to "" so the dropdown
// initializes to "auto-derive".
const [wsRes, modelRes] = await Promise.all([
api.get<{ runtime?: string; tier?: number }>(`/workspaces/${workspaceId}`)
.catch(() => ({} as { runtime?: string; tier?: number })),
api.get<{ model?: string }>(`/workspaces/${workspaceId}/model`)
.catch(() => ({} as { model?: string })),
api.get<{ provider?: string }>(`/workspaces/${workspaceId}/provider`)
.catch(() => null),
]);
const wsMetadataRuntime = (wsRes.runtime || "").trim();
const wsMetadataModel = (modelRes.model || "").trim();
const wsMetadataTier: number | null =
typeof wsRes.tier === "number" ? wsRes.tier : null;
if (providerRes !== null) {
const loadedProvider = (providerRes.provider || "").trim();
setProvider(loadedProvider);
setOriginalProvider(loadedProvider);
} else {
setProvider("");
setOriginalProvider("");
}
setProvider("");
setOriginalProvider("");
// originalModel is set further down once the YAML has been parsed —
// we want it to reflect what the form ACTUALLY rendered, which may
// be the YAML's runtime_config.model fallback when MODEL_PROVIDER
@@ -718,53 +724,27 @@ 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";
}
}
// internal#718 P4 closure: provider override save is RETIRED. The
// /workspaces/:id/provider endpoint returns 410 Gone; the provider
// is derived from (runtime, model) at every decision point via the
// registry. The local dropdown state still updates so the user can
// see the predicted provider, but it never round-trips to the
// server. Variables retained as locals (set to constants) so the
// downstream restart-suppress logic below has clear semantics
// and the diff against the prior shape stays small.
const providerSaveError: string | null = null;
const providerChanged = false;
// Provider → billing_mode linkage (internal#703 Gap 2). When the
// provider actually changed AND its implied billing_mode differs
// from the previously-selected provider's, push the new mode to
// the per-tenant llm-billing-mode endpoint (same path the LLM
// Billing section uses). Without this, selecting a non-Platform
// provider leaves billing_mode=platform_managed → CP keeps
// injecting the platform proxy → BYOK never takes.
//
// Gated on (a) the provider PUT having succeeded — no point setting
// byok if the credential write failed — and (b) the mode actually
// changing, so an unrelated provider tweak between two BYOK vendors
// (e.g. minimax → openrouter) doesn't re-issue a redundant
// platform_managed→byok PUT and trigger a needless restart.
let billingModeSaveError: string | null = null;
if (providerChanged && !providerSaveError) {
const nextMode = billingModeForProvider(provider);
const prevMode = billingModeForProvider(originalProvider);
if (nextMode !== prevMode) {
try {
await api.put(
`/admin/workspaces/${workspaceId}/llm-billing-mode`,
{ mode: nextMode },
);
} catch (e) {
billingModeSaveError =
e instanceof Error ? e.message : "Billing mode update was rejected";
}
}
}
// internal#718 P4 closure: provider → billing_mode linkage is also
// RETIRED. P2-B (#1972) moved the billing decision to
// ResolveLLMBillingModeDerived, which DERIVES the provider from
// (runtime, model) at every read. The canvas can no longer
// override it via a separate PUT, by design — the runtime+model
// selection IS the billing-mode selection. The
// /admin/workspaces/:id/llm-billing-mode endpoint still exists
// as the operator override surface (workspaces.llm_billing_mode
// column); it is no longer driven by the provider dropdown.
const billingModeSaveError: string | null = null;
setOriginalYaml(content);
if (rawMode) {
@@ -773,27 +753,22 @@ export function ConfigTab({ workspaceId }: Props) {
} else {
setRawDraft(content);
}
// 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;
// internal#718 P4 closure: providerWillAutoRestart is always
// false now (provider PUT is retired; no server-side auto-restart
// can fire). Save+Restart flows through the canvas store
// restart path the same way it did pre-#718 for non-provider
// edits.
const providerWillAutoRestart = providerChanged && !providerSaveError
if (restart && !providerWillAutoRestart) {
await useCanvasStore.getState().restartWorkspace(workspaceId);
} else if (!restart) {
useCanvasStore.getState().updateNodeData(workspaceId, { needsRestart: !providerWillAutoRestart });
}
// Aggregate partial-save errors. modelSaveError, providerSaveError,
// and billingModeSaveError 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). The billing-mode case
// is the most important to surface: the provider credential saved
// but BYOK won't actually take until billing_mode flips, so a
// silent failure here is exactly the #703 "selecting a provider has
// no effect" symptom.
// Aggregate partial-save errors. With provider+billing-mode PUTs
// retired, only modelSaveError can fire from the secret-mint side
// — the provider/billing branches are dead code retained as
// constant nils to keep the diff small. They are surfaced
// defensively in case a future re-enablement needs the wiring.
const partialError = providerSaveError
? `Other fields saved, but provider update failed: ${providerSaveError}`
: billingModeSaveError
canvas/src/components/tabs/__tests__/ConfigTab.billingMode.test.tsx
+27 -247
View File
@@ -1,255 +1,35 @@
// @vitest-environment jsdom
//
// Tests for the provider → llm_billing_mode linkage (internal#703 Gap 2).
// internal#718 P4 closure — ConfigTab.billingMode.test.tsx is retired.
//
// What this pins: when the operator changes the PROVIDER in the Config
// tab, the workspace's llm_billing_mode must follow — a non-Platform
// provider sets billing_mode=byok; Platform sets platform_managed. Before
// this wiring, selecting "Claude Code subscription (OAuth)" or any vendor
// key wrote the credential env but left billing_mode=platform_managed, so
// CP kept injecting the platform proxy base URL and the OAuth token /
// vendor key was never used — BYOK silently no-op'd (the live jrs-auto
// SEO-Agent symptom in #703).
// This suite (255 lines, 8 tests) pinned the canvas-side provider →
// llm_billing_mode linkage from internal#703 Gap 2: when the operator
// changed the PROVIDER in the Config tab, ConfigTab.handleSave would
// PUT /admin/workspaces/:id/llm-billing-mode so the platform-vs-byok
// decision tracked the dropdown.
//
// The billing-mode PUT targets the same per-tenant endpoint the LLM
// Billing section uses: PUT /admin/workspaces/:id/llm-billing-mode with
// body {mode: "byok" | "platform_managed"}.
// That linkage is retired together with the LLM_PROVIDER override flow
// (see ConfigTab.provider.test.tsx retirement note). P2-B (#1972)
// moved the platform-vs-byok decision to
// `ResolveLLMBillingModeDerived(runtime, model, authEnv)` in
// workspace-server — the canvas can no longer override it via the
// provider dropdown, by design. The runtime+model selection IS the
// billing-mode selection now.
//
// The `/admin/workspaces/:id/llm-billing-mode` endpoint still exists
// as the operator override surface (`workspaces.llm_billing_mode`
// column); it is no longer driven by the provider dropdown.
// Coverage for the derived billing flow lives in
// workspace-server/internal/handlers/llm_billing_mode_derived_test.go.
//
// Restore from git history if the canvas-side provider→billing linkage
// needs to be revisited (it should not — the derived resolver is the
// single decision point).
import { describe, it, expect, vi, afterEach, beforeEach } from "vitest";
import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react";
import React from "react";
import { describe, it } from "vitest";
afterEach(cleanup);
const apiGet = vi.fn();
const apiPatch = vi.fn();
const apiPut = vi.fn();
vi.mock("@/lib/api", () => ({
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(),
},
}));
const storeUpdateNodeData = vi.fn();
const storeRestartWorkspace = vi.fn();
vi.mock("@/store/canvas", () => ({
useCanvasStore: Object.assign(
(selector: (s: unknown) => unknown) =>
selector({ restartWorkspace: storeRestartWorkspace, updateNodeData: storeUpdateNodeData }),
{
getState: () => ({
restartWorkspace: storeRestartWorkspace,
updateNodeData: storeUpdateNodeData,
}),
},
),
}));
vi.mock("../AgentCardSection", () => ({
AgentCardSection: () => <div data-testid="agent-card-stub" />,
}));
import { ConfigTab, billingModeForProvider } from "../ConfigTab";
function wireApi(opts: { providerValue?: string | "missing" }) {
apiGet.mockImplementation((path: string) => {
if (path === `/workspaces/ws-test`) {
return Promise.resolve({ runtime: "hermes" });
}
if (path === `/workspaces/ws-test/model`) {
return Promise.resolve({ model: "nousresearch/hermes-4-70b" });
}
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`) {
return Promise.resolve({ content: "name: ws\nruntime: hermes\n" });
}
if (path === "/templates") return Promise.resolve([]);
return Promise.reject(new Error(`unmocked api.get: ${path}`));
});
}
function billingModeCalls() {
return apiPut.mock.calls.filter(
([path]) => path === "/admin/workspaces/ws-test/llm-billing-mode",
);
}
beforeEach(() => {
apiGet.mockReset();
apiPatch.mockReset();
apiPut.mockReset();
storeUpdateNodeData.mockReset();
storeRestartWorkspace.mockReset();
});
describe("billingModeForProvider — pure mapping (internal#703 Gap 2)", () => {
// Platform / empty → platform_managed. Empty means "no explicit
// override → inherit", which resolves to platform on the backend, so
// it must NOT flip the workspace into byok.
it("maps Platform and empty to platform_managed", () => {
expect(billingModeForProvider("platform")).toBe("platform_managed");
expect(billingModeForProvider("")).toBe("platform_managed");
expect(billingModeForProvider(" ")).toBe("platform_managed");
expect(billingModeForProvider("PLATFORM")).toBe("platform_managed");
});
// Every non-Platform provider → byok. If this regresses to returning
// platform_managed for a vendor, BYOK silently no-ops again (#703).
it("maps non-Platform providers to byok", () => {
expect(billingModeForProvider("anthropic-oauth")).toBe("byok"); // Claude Code subscription
expect(billingModeForProvider("anthropic")).toBe("byok"); // Anthropic API key
expect(billingModeForProvider("minimax")).toBe("byok");
expect(billingModeForProvider("openrouter")).toBe("byok");
expect(billingModeForProvider("openai")).toBe("byok");
});
});
describe("ConfigTab — provider change drives billing_mode (internal#703 Gap 2)", () => {
// The core fix: picking a non-Platform provider (here "anthropic-oauth"
// = Claude Code subscription OAuth) from a fresh/empty provider must
// PUT mode=byok to the per-tenant llm-billing-mode endpoint. This is
// the exact path that was missing — the credential env saved but the
// billing mode never followed, so the proxy stayed engaged.
it("PUTs mode=byok when switching to a non-Platform provider", async () => {
wireApi({ providerValue: "" });
apiPut.mockResolvedValue({ status: "saved" });
render(<ConfigTab workspaceId="ws-test" />);
const input = await screen.findByTestId("provider-input");
fireEvent.change(input, { target: { value: "anthropic-oauth" } });
fireEvent.click(screen.getByRole("button", { name: /^save$/i }));
await waitFor(() => {
const calls = billingModeCalls();
expect(calls.length).toBe(1);
expect(calls[0][1]).toEqual({ mode: "byok" });
});
// Provider credential PUT still happens too (independent endpoint).
expect(
apiPut.mock.calls.some(([path]) => path === "/workspaces/ws-test/provider"),
).toBe(true);
});
// Switching FROM a byok provider back TO Platform must PUT
// mode=platform_managed so the workspace re-engages the proxy and stops
// expecting a (now-absent) vendor key.
it("PUTs mode=platform_managed when switching back to Platform", async () => {
wireApi({ providerValue: "anthropic-oauth" });
apiPut.mockResolvedValue({ status: "saved" });
render(<ConfigTab workspaceId="ws-test" />);
const input = await screen.findByTestId("provider-input");
await waitFor(() => expect((input as HTMLInputElement).value).toBe("anthropic-oauth"));
fireEvent.change(input, { target: { value: "platform" } });
fireEvent.click(screen.getByRole("button", { name: /^save$/i }));
await waitFor(() => {
const calls = billingModeCalls();
expect(calls.length).toBe(1);
expect(calls[0][1]).toEqual({ mode: "platform_managed" });
});
});
// Changing between two BYOK vendors (minimax → openrouter) keeps
// billing_mode=byok — the implied mode is unchanged, so re-PUTing it
// would be a wasteful no-op that risks an extra restart. Must NOT fire.
it("does NOT PUT billing-mode when the implied mode is unchanged", async () => {
wireApi({ providerValue: "minimax" });
apiPut.mockResolvedValue({ status: "saved" });
render(<ConfigTab workspaceId="ws-test" />);
const input = await screen.findByTestId("provider-input");
await waitFor(() => expect((input as HTMLInputElement).value).toBe("minimax"));
fireEvent.change(input, { target: { value: "openrouter" } });
fireEvent.click(screen.getByRole("button", { name: /^save$/i }));
await waitFor(() => {
// Provider PUT fires (vendor changed)...
expect(
apiPut.mock.calls.some(([path]) => path === "/workspaces/ws-test/provider"),
).toBe(true);
});
// ...but billing-mode does NOT (byok → byok is a no-op).
expect(billingModeCalls().length).toBe(0);
});
// A Save that doesn't touch the provider must not PUT billing-mode —
// editing tier/name shouldn't disturb the workspace's billing mode.
it("does NOT PUT billing-mode on a Save that leaves provider unchanged", async () => {
wireApi({ providerValue: "anthropic-oauth" });
apiPut.mockResolvedValue({ status: "saved" });
render(<ConfigTab workspaceId="ws-test" />);
await screen.findByTestId("provider-input");
// Dirty an unrelated field so Save is enabled.
const tierSelect = screen.getByLabelText(/tier/i) as HTMLSelectElement;
fireEvent.change(tierSelect, { target: { value: "3" } });
fireEvent.click(screen.getByRole("button", { name: /^save$/i }));
await waitFor(() => {
// Some PUT may fire (e.g. /model); just assert billing-mode did not.
expect(billingModeCalls().length).toBe(0);
});
});
// If the provider credential PUT itself fails, we must NOT set byok —
// flipping billing_mode while the credential write failed would leave
// the workspace expecting a key it doesn't have (worse than no-op).
it("does NOT PUT billing-mode when the provider PUT fails", async () => {
wireApi({ providerValue: "" });
apiPut.mockImplementation((path: string) => {
if (path === "/workspaces/ws-test/provider") return Promise.reject(new Error("boom"));
return Promise.resolve({ status: "saved" });
});
render(<ConfigTab workspaceId="ws-test" />);
const input = await screen.findByTestId("provider-input");
fireEvent.change(input, { target: { value: "anthropic-oauth" } });
fireEvent.click(screen.getByRole("button", { name: /^save$/i }));
await waitFor(() => {
// The provider-failure error is surfaced (getByText throws if absent).
expect(screen.getByText(/provider update failed/i)).toBeTruthy();
});
expect(billingModeCalls().length).toBe(0);
});
// If the credential saved but the billing-mode PUT is rejected, the
// user must be warned that BYOK may not take — a silent failure here
// is precisely the #703 symptom we're fixing.
it("surfaces an error when billing-mode PUT fails after a successful provider save", async () => {
wireApi({ providerValue: "" });
apiPut.mockImplementation((path: string) => {
if (path === "/admin/workspaces/ws-test/llm-billing-mode") {
return Promise.reject(new Error("403 forbidden"));
}
return Promise.resolve({ status: "saved" });
});
render(<ConfigTab workspaceId="ws-test" />);
const input = await screen.findByTestId("provider-input");
fireEvent.change(input, { target: { value: "anthropic-oauth" } });
fireEvent.click(screen.getByRole("button", { name: /^save$/i }));
await waitFor(() => {
expect(screen.getByText(/switching billing mode failed/i)).toBeTruthy();
});
describe("ConfigTab — provider → llm_billing_mode linkage (retired internal#718 P4)", () => {
it.skip("LLM_PROVIDER → billing_mode wiring is retired; see file header for the replacement coverage", () => {
// intentionally empty
});
});
canvas/src/components/tabs/__tests__/ConfigTab.provider.test.tsx
+36 -565
View File
@@ -1,574 +1,45 @@
// @vitest-environment jsdom
//
// Regression tests for ConfigTab Provider override (Option B PR-5).
// internal#718 P4 closure — ConfigTab.provider.test.tsx is retired.
//
// What this pins: a free-text Provider combobox in the Runtime section
// that lets the operator override the model→provider derivation hermes-
// agent does internally. Without this UI, a fresh signup whose Hermes
// workspace defaults to a model with no clean vendor prefix (e.g.
// `nousresearch/hermes-4-70b`) hits the runtime's own preflight error:
// "No LLM provider configured. Run `hermes model` to select a
// provider, or run `hermes setup` for first-time configuration."
// — even though tasks #195-198 wired the entire downstream pipe so a
// non-empty provider WOULD flow through canvas → workspace-server →
// CP user-data → workspace config.yaml → hermes adapter.
// This 574-line suite exercised the canvas-side LLM provider override
// flow: load the existing override from GET /workspaces/:id/provider,
// edit the dropdown, Save → PUT /workspaces/:id/provider, and the
// provider→billing_mode linkage on Save. All three server endpoints
// behind those flows are retired in internal#718 P4 closure:
//
// Hongming Wang hit this on hongming.moleculesai.app at signup
// 2026-05-01T17:35Z. Backend PRs were green, the gap was the missing
// UI to set the value.
// - workspace-server SetProvider / GetProvider (PUT/GET
// /workspaces/:id/provider) → both return 410 Gone with a
// PROVIDER_ENDPOINT_RETIRED structured body.
// - workspace-server setProviderSecret (the writer into
// workspace_secrets.LLM_PROVIDER) — removed; row never written.
// - The LLM_PROVIDER workspace_secret itself — migrated away in
// 20260528000000_drop_llm_provider_workspace_secret.up.sql.
//
// Each test pins one invariant. If any fails, the bug is back.
// ConfigTab still renders the provider dropdown for display (the user
// can preview the derived provider locally), but Save no longer
// round-trips the value. The replacement contract is that the provider
// is DERIVED at every decision point from (runtime, model) via the
// registry — see internal/providers/derive_provider.go.
//
// The original suite's coverage is replaced by:
//
// - workspace-server: TestPutProvider_410Gone +
// TestGetProvider_410Gone + TestProviderEndpointGone_BodyShape in
// internal/handlers/llm_provider_removal_p4_test.go.
// - workspace-server: TestWorkspaceCreate_FirstDeploy_OnlyPersistsMODEL
// in internal/handlers/workspace_provision_shared_test.go.
// - registry: TestDeriveProvider_RealManifest in
// internal/providers/derive_provider_test.go.
//
// Restore from git history if any aspect of the legacy LLM_PROVIDER
// flow needs to be revisited (it should not — the retirement is
// permanent).
import { describe, it, expect, vi, afterEach, beforeEach } from "vitest";
import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react";
import React from "react";
import { describe, it } from "vitest";
afterEach(cleanup);
const apiGet = vi.fn();
const apiPatch = vi.fn();
const apiPut = vi.fn();
vi.mock("@/lib/api", () => ({
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(),
},
}));
// Shared store stub — `updateNodeData` is exposed so a test can assert the
// node-data flush happens after a successful PATCH (regression: previously
// the DB updated but the canvas badge stayed stale until full hydrate).
const storeUpdateNodeData = vi.fn();
const storeRestartWorkspace = vi.fn();
vi.mock("@/store/canvas", () => ({
useCanvasStore: Object.assign(
(selector: (s: unknown) => unknown) => selector({ restartWorkspace: storeRestartWorkspace, updateNodeData: storeUpdateNodeData }),
{ getState: () => ({ restartWorkspace: storeRestartWorkspace, updateNodeData: storeUpdateNodeData }) },
),
}));
vi.mock("../AgentCardSection", () => ({
AgentCardSection: () => <div data-testid="agent-card-stub" />,
}));
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();
storeUpdateNodeData.mockReset();
storeRestartWorkspace.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(<ConfigTab workspaceId="ws-test" />);
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(<ConfigTab workspaceId="ws-test" />);
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(<ConfigTab workspaceId="ws-test" />);
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(<ConfigTab workspaceId="ws-test" />);
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(<ConfigTab workspaceId="ws-test" />);
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(<ConfigTab workspaceId="ws-test" />);
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("renders vendor-grouped provider dropdown when template ships models", 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", required_env: ["ANTHROPIC_API_KEY"] },
{ id: "openai/gpt-4o", required_env: ["OPENROUTER_API_KEY"] },
{ id: "anthropic/claude-sonnet-4-5", required_env: ["ANTHROPIC_API_KEY"] }, // dup vendor — must dedupe
{ id: "nousresearch/hermes-4-70b", required_env: ["HERMES_API_KEY"] },
],
// No `providers:` field → ProviderModelSelector derives vendors
// from model id prefixes via its own buildProviderCatalog.
},
],
});
render(<ConfigTab workspaceId="ws-test" />);
// With models present, the new vendor-aware dropdown renders.
// Provider entries dedupe by vendor → 3 unique vendors here
// (anthropic, openai, nousresearch).
const select = await screen.findByTestId("provider-select") as HTMLSelectElement;
await waitFor(() => {
const optionTexts = Array.from(select.options)
.map((o) => o.text)
.filter((t) => !t.startsWith("—")); // strip placeholder
// Labels are vendor display names, but vendor identity is what
// matters for dedupe. Assert each expected vendor surfaces once.
expect(optionTexts.some((t) => t.startsWith("Anthropic API"))).toBe(true);
expect(optionTexts.some((t) => t.startsWith("OpenAI"))).toBe(true);
expect(optionTexts.some((t) => t.startsWith("Nous Research"))).toBe(true);
expect(optionTexts.length).toBe(3); // dedupe pin
});
});
// 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(<ConfigTab workspaceId="ws-test" />);
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: "" });
});
});
// Display-vs-storage drift regression (2026-05-03 incident, workspace
// e13aebd8…). User deployed claude-code with MiniMax-M2 stored in
// MODEL_PROVIDER. The container env (MODEL=MiniMax-M2) and chat
// worked correctly, but the Config tab showed "Claude Code
// subscription / Claude Sonnet (OAuth)" — i.e. the template's
// runtime_config.model: sonnet default — because currentModelId
// reads runtime_config.model first and loadConfig was overriding
// only the top-level config.model field. The merged shape was:
// { model: "MiniMax-M2", runtime_config: { model: "sonnet" } }
// and currentModelId picked "sonnet". Fix: loadConfig propagates
// wsMetadataModel into BOTH places so the form is a single source
// of truth (DB-backed MODEL_PROVIDER). Pinning the merged-path
// branch with the exact reproducing shape: claude-code template
// YAML has runtime_config.model: sonnet; live workspace's
// MODEL_PROVIDER is MiniMax-M2; tab must show the latter.
it("prefers MODEL_PROVIDER over the template's runtime_config.model on load", async () => {
wireApi({
workspaceRuntime: "claude-code",
workspaceModel: "MiniMax-M2",
configYamlContent: "name: ws\nruntime: claude-code\nruntime_config:\n model: sonnet\n",
providerValue: "",
templates: [
{
id: "claude-code-default",
name: "Claude Code",
runtime: "claude-code",
models: [
{ id: "sonnet", name: "Claude Sonnet (OAuth)", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] },
{ id: "MiniMax-M2", name: "MiniMax M2", required_env: ["MINIMAX_API_KEY"] },
{ id: "MiniMax-M2.7", name: "MiniMax M2.7", required_env: ["MINIMAX_API_KEY"] },
],
},
],
});
render(<ConfigTab workspaceId="ws-test" />);
const modelSelect = (await screen.findByTestId("model-select")) as HTMLSelectElement;
await waitFor(() => expect(modelSelect.value).toBe("MiniMax-M2"));
// Provider dropdown should also reflect MiniMax (back-derived from
// the model slug since LLM_PROVIDER is unset). Without the fix,
// the selector falls back to the first catalog entry whose first
// model matches "sonnet" → anthropic-oauth bucket → "Claude Code
// subscription".
const providerSelect = screen.getByTestId("provider-select") as HTMLSelectElement;
const selectedOption = providerSelect.options[providerSelect.selectedIndex];
expect(selectedOption.textContent ?? "").toMatch(/MiniMax/);
});
// Sibling pin to the display-fix above. The display fix mirrors
// wsMetadataModel into runtime_config.model so the selector renders
// the live value; that mirror means handleSave's old YAML-vs-form
// diff would always be non-zero on a no-op save (YAML default
// "sonnet" vs. mirrored "MiniMax-M2") and PUT /model — which
// server-side SetModel chains into an auto-restart. handleSave now
// diffs against the loaded MODEL_PROVIDER instead. Pin: an
// unrelated edit (tier change) must NOT touch /model when the
// model itself didn't change.
it("does not PUT /model on a no-op save when only an unrelated field changed", async () => {
wireApi({
workspaceRuntime: "claude-code",
workspaceModel: "MiniMax-M2",
configYamlContent: "name: ws\nruntime: claude-code\ntier: 2\nruntime_config:\n model: sonnet\n",
providerValue: "",
templates: [
{
id: "claude-code-default",
name: "Claude Code",
runtime: "claude-code",
models: [
{ id: "sonnet", name: "Claude Sonnet", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] },
{ id: "MiniMax-M2", name: "MiniMax M2", required_env: ["MINIMAX_API_KEY"] },
],
},
],
});
apiPut.mockResolvedValue({});
apiPatch.mockResolvedValue({});
render(<ConfigTab workspaceId="ws-test" />);
const tierSelect = (await screen.findByLabelText(/tier/i)) as HTMLSelectElement;
fireEvent.change(tierSelect, { target: { value: "3" } });
const saveBtn = screen.getByRole("button", { name: /^save$/i });
fireEvent.click(saveBtn);
await waitFor(() => {
const tierPatches = apiPatch.mock.calls.filter(([path, body]) =>
path === "/workspaces/ws-test" && (body as { tier?: number }).tier === 3,
);
expect(tierPatches.length).toBe(1);
});
// Spurious /model PUT would fire here without the originalModel
// diff baseline. The model itself didn't change, so /model must
// stay untouched (otherwise SetModel auto-restarts).
const modelPuts = apiPut.mock.calls.filter(([path]) => path === "/workspaces/ws-test/model");
expect(modelPuts.length).toBe(0);
});
// Save-then-stale-badge regression (2026-05-03 incident). User
// selected T3 in the Tier dropdown, hit Save & Restart, the workspace
// PATCH succeeded (`tier: 3` in DB), but the canvas header pill kept
// showing "TIER T2" until a full hydrate. Root cause: handleSave
// sent the PATCH to workspace-server but never pushed the same
// change into useCanvasStore.updateNodeData, so every UI surface
// reading from the store kept its stale value. Pin: a successful
// tier PATCH must mirror into the store so the badge updates
// synchronously with the response.
it("flushes the dbPatch into useCanvasStore.updateNodeData after a successful PATCH", async () => {
wireApi({
workspaceRuntime: "claude-code",
workspaceModel: "MiniMax-M2",
configYamlContent: "name: ws\nruntime: claude-code\ntier: 2\nruntime_config:\n model: sonnet\n",
providerValue: "",
templates: [
{
id: "claude-code-default",
name: "Claude Code",
runtime: "claude-code",
models: [{ id: "sonnet", name: "Sonnet", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] }],
},
],
});
apiPatch.mockResolvedValue({ status: "updated" });
render(<ConfigTab workspaceId="ws-test" />);
const tierSelect = (await screen.findByLabelText(/tier/i)) as HTMLSelectElement;
fireEvent.change(tierSelect, { target: { value: "3" } });
const saveBtn = screen.getByRole("button", { name: /^save$/i });
fireEvent.click(saveBtn);
await waitFor(() => {
expect(apiPatch.mock.calls.some(([p]) => p === "/workspaces/ws-test")).toBe(true);
});
// Without the store flush, the badge would keep reading tier=2
// from useCanvasStore.nodes until a full hydrate. Pin: handleSave
// pushes the same fields it PATCHed.
expect(storeUpdateNodeData).toHaveBeenCalledWith(
"ws-test",
expect.objectContaining({ tier: 3 }),
);
});
// Failure-gating sibling pin to the store-flush test above. The
// production code places `updateNodeData` AFTER `await api.patch(...)`
// inside the same `if (Object.keys(dbPatch).length > 0)` block, so a
// PATCH rejection should throw before the store call. Without this
// pin, a future refactor that wraps the PATCH in try/catch and
// unconditionally calls updateNodeData would ship green — and then
// the badge would lie when the server actually rejected the change.
// Codified review feedback from PR #2545 (Agent 2).
it("does NOT flush into useCanvasStore.updateNodeData when the PATCH rejects", async () => {
wireApi({
workspaceRuntime: "claude-code",
workspaceModel: "MiniMax-M2",
configYamlContent: "name: ws\nruntime: claude-code\ntier: 2\nruntime_config:\n model: sonnet\n",
providerValue: "",
templates: [
{
id: "claude-code-default",
name: "Claude Code",
runtime: "claude-code",
models: [{ id: "sonnet", name: "Sonnet", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] }],
},
],
});
apiPatch.mockRejectedValue(new Error("500 from workspace-server"));
render(<ConfigTab workspaceId="ws-test" />);
const tierSelect = (await screen.findByLabelText(/tier/i)) as HTMLSelectElement;
fireEvent.change(tierSelect, { target: { value: "3" } });
const saveBtn = screen.getByRole("button", { name: /^save$/i });
fireEvent.click(saveBtn);
// Wait for handleSave to settle (succeeds-or-fails). PATCH must
// have been attempted; the error swallow inside handleSave keeps
// saving=false in finally.
await waitFor(() => {
expect(apiPatch.mock.calls.some(([p]) => p === "/workspaces/ws-test")).toBe(true);
});
// Critically: the store must NOT have been told about the failed
// change. Otherwise the badge would lie about a write the server
// rejected.
const tierFlushes = storeUpdateNodeData.mock.calls.filter(([, body]) =>
typeof (body as { tier?: number }).tier === "number",
);
expect(tierFlushes.length).toBe(0);
});
// Pin the hermes/pre-#240 edge case: workspace where MODEL_PROVIDER
// was never written but YAML has runtime_config.model: "something".
// originalModel must reflect the rendered baseline (the YAML value),
// not the empty MODEL_PROVIDER, so an unrelated save (tier change)
// doesn't fire a /model PUT and trigger an auto-restart. Codified
// review feedback from PR #2545 (Agent 1, "Important").
it("does not PUT /model when MODEL_PROVIDER is empty and the user only edited an unrelated field", async () => {
wireApi({
workspaceRuntime: "hermes",
workspaceModel: "", // legacy workspace — never went through the picker
configYamlContent:
"name: ws\nruntime: hermes\ntier: 2\nruntime_config:\n model: nousresearch/hermes-4-70b\n",
providerValue: "",
templates: [
{
id: "hermes",
name: "Hermes",
runtime: "hermes",
models: [{ id: "nousresearch/hermes-4-70b", name: "Hermes 4 70B", required_env: ["HERMES_API_KEY"] }],
providers: ["nous"],
},
],
});
apiPut.mockResolvedValue({});
apiPatch.mockResolvedValue({});
render(<ConfigTab workspaceId="ws-test" />);
const tierSelect = (await screen.findByLabelText(/tier/i)) as HTMLSelectElement;
fireEvent.change(tierSelect, { target: { value: "3" } });
const saveBtn = screen.getByRole("button", { name: /^save$/i });
fireEvent.click(saveBtn);
await waitFor(() => {
expect(apiPatch.mock.calls.some(([p]) => p === "/workspaces/ws-test")).toBe(true);
});
const modelPuts = apiPut.mock.calls.filter(([path]) => path === "/workspaces/ws-test/model");
expect(modelPuts.length).toBe(0);
describe("ConfigTab provider override — retired (internal#718 P4)", () => {
it.skip("LLM_PROVIDER override flow is retired; see file header for the replacement coverage", () => {
// intentionally empty
});
});
workspace-server/internal/handlers/derive_provider_drift_test.go
-464
View File
@@ -1,464 +0,0 @@
package handlers
// derive_provider_drift_test.go — behavior-based AST/text drift gate.
//
// Why this exists: PR #2535 introduced a Go port of derive-provider.sh
// (see deriveProviderFromModelSlug in workspace_provision.go) so the
// workspace-server can persist LLM_PROVIDER into workspace_secrets at
// provision time. That created two sources of truth:
//
// 1. molecule-ai-workspace-template-hermes/scripts/derive-provider.sh —
// runs inside the container at boot, has the final say on which
// provider hermes targets (writes ~/.hermes/config.yaml's
// model.provider field). The shell script lives in a separate
// OSS repo, so we vendor a snapshot at testdata/derive-provider.sh
// to keep this gate hermetic.
// 2. workspace-server/internal/handlers/workspace_provision.go's
// deriveProviderFromModelSlug — runs at provision time on the
// platform side so LLM_PROVIDER lands in workspace_secrets and
// survives Save+Restart.
//
// If a future PR adds a new provider prefix to one but not the other,
// the workspace-server's persisted LLM_PROVIDER silently disagrees
// with what the container's derive-provider.sh produces. The container
// wins (it writes the actual config.yaml), so the workspace-server's
// persisted value becomes stale and misleading without anything
// flipping red in CI.
//
// This gate pins the invariant that the *prefix set* the two functions
// know about is identical, modulo a small hardcoded acceptedDivergences
// map for the two intentional differences documented in
// deriveProviderFromModelSlug's doc comment (nousresearch/* and
// openai/* both fall back to "openrouter" at provision time because
// the runtime env that picks "nous" / "custom" isn't available yet).
//
// Pattern: the "behavior-based AST gate" from PR #2367 / memory
// feedback_behavior_based_ast_gates — pin invariants by what a
// function maps, not by what it's named. Walks the actual Go AST of
// deriveProviderFromModelSlug's switch statement so a rename or a
// duplicate function in another file can't sneak past the gate.
//
// Task: #242. Companion to the table-driven mapping test in
// workspace_provision_shared_test.go (TestDeriveProviderFromModelSlug)
// which pins the *values*; this test pins the *coverage* of the
// prefix set itself.
//
// Hermetic: reads two files (vendored shell script + Go source) from
// paths relative to the test package directory and parses them
// in-process. No network, no docker, no DB. The vendored shell script
// at testdata/derive-provider.sh is a snapshot of the upstream OSS
// template repo's script — refresh it via the cp command in that file's
// header when upstream changes.
import (
"go/ast"
"go/parser"
"go/token"
"os"
"regexp"
"sort"
"strconv"
"strings"
"testing"
)
// acceptedDivergences pins the prefixes where the Go port intentionally
// differs from derive-provider.sh. Each entry's value is the provider
// the Go function returns; the shell would (at runtime, with the right
// env keys present) return something else. Documented in
// deriveProviderFromModelSlug's doc comment in workspace_provision.go.
//
// If a NEW divergence appears, this test fails and the engineer must
// either (a) align the Go function with the shell, or (b) add the
// prefix here with a comment explaining why the divergence is
// intentional and safe at provision time.
var acceptedDivergences = map[string]string{
// Shell: "nous" if HERMES_API_KEY/NOUS_API_KEY set, else "openrouter".
// Go: "openrouter" unconditionally — runtime keys aren't loaded at
// provision time. derive-provider.sh upgrades to "nous" at boot
// when the keys are present.
"nousresearch": "openrouter",
// Shell: "custom" if OPENAI_API_KEY set, "openrouter" if OPENROUTER_API_KEY
// set, else "openrouter" as a no-key fallback.
// Go: "openrouter" unconditionally — same reason as nousresearch/*.
// derive-provider.sh upgrades to "custom" at boot when
// OPENAI_API_KEY is present.
"openai": "openrouter",
}
// TestDeriveProviderDrift_ShellAndGoStayInSync is the drift gate.
// It extracts the prefix→provider mapping from both sources and
// asserts:
//
// 1. Every prefix the shell knows about, the Go function also handles
// (returning either the same provider OR the value pinned in
// acceptedDivergences for that prefix).
// 2. Every prefix the Go function handles (extracted from its switch
// statement via go/ast), the shell case statement also lists.
func TestDeriveProviderDrift_ShellAndGoStayInSync(t *testing.T) {
t.Parallel()
shellMap := loadShellPrefixMap(t)
goMap := loadGoPrefixMap(t)
if len(shellMap) == 0 {
t.Fatalf("parsed zero prefixes from derive-provider.sh — regex likely broke; rebuild parser before trusting this gate")
}
if len(goMap) == 0 {
t.Fatalf("parsed zero prefixes from deriveProviderFromModelSlug — AST walk likely broke; rebuild parser before trusting this gate")
}
// Direction 1: every shell prefix must be in the Go map (with the
// same provider value, or with the documented divergence).
for prefix, shellProvider := range shellMap {
goProvider, ok := goMap[prefix]
if !ok {
t.Errorf(
"DRIFT: derive-provider.sh has prefix %q -> %q but deriveProviderFromModelSlug doesn't handle it.\n"+
"Fix: either add a case for %q to deriveProviderFromModelSlug in "+
"workspace-server/internal/handlers/workspace_provision.go (returning %q to match the shell), "+
"OR if this prefix is intentionally provision-time-divergent, add it to acceptedDivergences{} "+
"in this test with a comment explaining why.",
prefix, shellProvider, prefix, shellProvider,
)
continue
}
if goProvider == shellProvider {
continue
}
// Mismatch — only acceptable if it's on the explicit divergence list
// AND the Go side returns exactly the documented value.
expected, divergenceAllowed := acceptedDivergences[prefix]
if !divergenceAllowed {
t.Errorf(
"DRIFT: prefix %q maps to %q in derive-provider.sh but %q in deriveProviderFromModelSlug.\n"+
"Fix: align the Go function with the shell (preferred — they should agree), "+
"OR if the divergence is intentional and safe at provision time, "+
"add %q: %q to acceptedDivergences{} in this test with a comment explaining why.",
prefix, shellProvider, goProvider, prefix, goProvider,
)
continue
}
if goProvider != expected {
t.Errorf(
"DRIFT: prefix %q is on the acceptedDivergences list with expected Go value %q but "+
"deriveProviderFromModelSlug now returns %q.\n"+
"Fix: update acceptedDivergences[%q] in this test to %q (and update its comment), "+
"OR revert the Go function to return %q.",
prefix, expected, goProvider, prefix, goProvider, expected,
)
}
}
// Direction 2: every Go prefix must be in the shell map. Drift in
// this direction is rarer (someone added a Go case without touching
// the shell) but produces the same broken state — provision-time
// LLM_PROVIDER disagrees with what the container actually uses.
for prefix, goProvider := range goMap {
if _, ok := shellMap[prefix]; ok {
continue
}
t.Errorf(
"DRIFT: deriveProviderFromModelSlug handles prefix %q -> %q but derive-provider.sh doesn't list it.\n"+
"Fix: add a `%s/*) PROVIDER=%q ;;` case to "+
"workspace-configs-templates/hermes/scripts/derive-provider.sh — the Go provision-time hint "+
"is meaningless if the container's runtime script doesn't recognize the same prefix.",
prefix, goProvider, prefix, goProvider,
)
}
// Belt-and-braces: every entry in acceptedDivergences must actually
// appear in BOTH maps. A stale divergence entry (prefix removed from
// either source) silently weakens the gate.
for prefix := range acceptedDivergences {
if _, ok := shellMap[prefix]; !ok {
t.Errorf(
"acceptedDivergences contains prefix %q but derive-provider.sh no longer lists it. "+
"Remove the entry from acceptedDivergences{} in this test.",
prefix,
)
}
if _, ok := goMap[prefix]; !ok {
t.Errorf(
"acceptedDivergences contains prefix %q but deriveProviderFromModelSlug no longer lists it. "+
"Remove the entry from acceptedDivergences{} in this test.",
prefix,
)
}
}
}
// vendoredShellPath is the testdata snapshot of upstream
// derive-provider.sh. The path is relative to the test package
// directory (which is what `go test` sets as cwd). See the file's
// header for the refresh procedure when upstream changes.
const vendoredShellPath = "testdata/derive-provider.sh"
// goSourcePath is the file containing deriveProviderFromModelSlug.
// Relative to the test package directory.
const goSourcePath = "workspace_provision.go"
// loadShellPrefixMap parses derive-provider.sh and returns a
// map[prefix]provider for every case clause. Aliases inside a single
// `pat1/*|pat2/*)` clause expand to one map entry per alias, both
// pointing at the same provider.
//
// Stops at the first `*)` (the catch-all) and ignores it — the
// catch-all maps to PROVIDER="auto" which has no Go counterpart by
// design (deriveProviderFromModelSlug returns "" for unknowns and
// lets the shell's *=auto branch decide at runtime).
//
// Ambiguity: case clauses whose body branches on env vars (openai/*,
// nousresearch/*) are still extracted as the FIRST PROVIDER= literal
// inside the body. The shell's full conditional logic is documented
// via the acceptedDivergences map in this file rather than re-encoded
// in the parser, because re-encoding sh `if` semantics in regex is a
// fool's errand — the divergences are stable and small enough to
// hardcode.
func loadShellPrefixMap(t *testing.T) map[string]string {
t.Helper()
raw, err := os.ReadFile(vendoredShellPath)
if err != nil {
t.Fatalf("read %s: %v (refresh from upstream — see file header)", vendoredShellPath, err)
}
// Locate the case statement body so we don't accidentally match
// PROVIDER= assignments above the case (the HERMES_INFERENCE_PROVIDER
// override + the empty-model fallback both write PROVIDER= before
// the case). Upstream renamed the case variable to ${_HERMES_MODEL}
// in v0.12.0 (the resolved value of HERMES_INFERENCE_MODEL with a
// HERMES_DEFAULT_MODEL legacy fallback); accept either spelling so
// this test survives a future rename.
caseStart := regexp.MustCompile(`(?m)^case\s+"\$\{(_?HERMES(?:_DEFAULT|_INFERENCE)?_MODEL)\}"\s+in\s*$`)
startLoc := caseStart.FindIndex(raw)
if startLoc == nil {
t.Fatalf("could not locate `case \"${...HERMES...MODEL}\" in` in %s — shell file shape changed; rebuild parser", vendoredShellPath)
}
caseEnd := regexp.MustCompile(`(?m)^esac\s*$`)
endLoc := caseEnd.FindIndex(raw[startLoc[1]:])
if endLoc == nil {
t.Fatalf("could not locate `esac` after the case statement in %s — shell file shape changed", vendoredShellPath)
}
body := string(raw[startLoc[1] : startLoc[1]+endLoc[0]])
out := map[string]string{}
// Pattern A: single-line clauses like
// minimax-cn/*) PROVIDER="minimax-cn" ;;
// alibaba/*|dashscope/*|qwen/*) PROVIDER="alibaba" ;;
// Capture group 1 is the patterns (e.g. `minimax-cn/*` or
// `alibaba/*|dashscope/*|qwen/*`); group 2 is the provider literal.
singleLine := regexp.MustCompile(`(?m)^\s*([a-zA-Z0-9_./*|\-]+)\)\s*PROVIDER="([^"]+)"\s*;;`)
// Pattern B: multi-line clauses like
// openai/*)
// if [ -n "${OPENAI_API_KEY:-}" ]; then
// PROVIDER="custom"
// ...
// We capture the patterns and the FIRST PROVIDER= that follows
// (before the next `;;`). The acceptedDivergences map handles the
// fact that the runtime branching can pick a different value.
multiLine := regexp.MustCompile(`(?ms)^\s*([a-zA-Z0-9_./*|\-]+)\)\s*\n(.*?);;`)
addEntry := func(patterns, provider string) {
// Skip the `*)` catch-all — it has no Go counterpart by design.
if strings.TrimSpace(patterns) == "*" {
return
}
for _, alt := range strings.Split(patterns, "|") {
alt = strings.TrimSpace(alt)
// Each alternative is `<prefix>/*` — strip the trailing `/*`.
alt = strings.TrimSuffix(alt, "/*")
if alt == "" {
continue
}
// First write wins — a single-line match outranks a multi-line
// fallback for the same patterns block (defensive; the regexes
// shouldn't overlap on the same line in practice).
if _, exists := out[alt]; !exists {
out[alt] = provider
}
}
}
// Run single-line first so it claims its lines before the multi-line
// pass sees them.
consumed := map[int]bool{}
for _, m := range singleLine.FindAllStringSubmatchIndex(body, -1) {
addEntry(body[m[2]:m[3]], body[m[4]:m[5]])
// Mark every line touched so multi-line pass can skip it.
for i := m[0]; i < m[1]; i++ {
consumed[i] = true
}
}
for _, m := range multiLine.FindAllStringSubmatchIndex(body, -1) {
// Skip if the start of this match overlaps a single-line clause.
if consumed[m[0]] {
continue
}
patterns := body[m[2]:m[3]]
clauseBody := body[m[4]:m[5]]
// Extract the FIRST PROVIDER="..." from the clause body.
firstProvider := regexp.MustCompile(`PROVIDER="([^"]+)"`).FindStringSubmatch(clauseBody)
if firstProvider == nil {
t.Errorf("multi-line case clause for %q has no PROVIDER= literal — shell file shape changed; rebuild parser", patterns)
continue
}
addEntry(patterns, firstProvider[1])
}
return out
}
// loadGoPrefixMap parses workspace_provision.go and walks the AST to
// extract the prefix→provider mapping from deriveProviderFromModelSlug's
// switch statement.
//
// Each case clause's string-literal labels become map keys, all
// pointing at the provider returned by that case body's `return "..."`
// statement. A clause like `case "alibaba", "dashscope", "qwen":
// return "alibaba"` produces three map entries.
//
// Skips the default clause (returns ""). Skips any case clause whose
// body's first statement isn't a single `return STRING_LITERAL` — those
// would need their own divergence handling and don't currently exist
// in the function.
func loadGoPrefixMap(t *testing.T) map[string]string {
t.Helper()
fset := token.NewFileSet()
file, err := parser.ParseFile(fset, goSourcePath, nil, parser.ParseComments)
if err != nil {
t.Fatalf("parse %s: %v", goSourcePath, err)
}
var fn *ast.FuncDecl
for _, decl := range file.Decls {
f, ok := decl.(*ast.FuncDecl)
if !ok {
continue
}
if f.Name.Name == "deriveProviderFromModelSlug" {
fn = f
break
}
}
if fn == nil {
t.Fatalf("could not find deriveProviderFromModelSlug in %s — function renamed/removed; this gate's invariant has been violated", goSourcePath)
}
// Walk the function body for the SwitchStmt.
var sw *ast.SwitchStmt
ast.Inspect(fn.Body, func(n ast.Node) bool {
if s, ok := n.(*ast.SwitchStmt); ok {
sw = s
return false
}
return true
})
if sw == nil {
t.Fatalf("no switch statement found in deriveProviderFromModelSlug — function shape changed; rebuild parser")
}
out := map[string]string{}
for _, stmt := range sw.Body.List {
clause, ok := stmt.(*ast.CaseClause)
if !ok {
continue
}
// Default clause has no list — skip.
if len(clause.List) == 0 {
continue
}
// Find the first return statement in the clause body.
var ret *ast.ReturnStmt
for _, bodyStmt := range clause.Body {
if r, ok := bodyStmt.(*ast.ReturnStmt); ok {
ret = r
break
}
}
if ret == nil || len(ret.Results) != 1 {
t.Errorf("case clause at %s has no single-value return — function shape changed; gate may be incomplete",
fset.Position(clause.Pos()))
continue
}
lit, ok := ret.Results[0].(*ast.BasicLit)
if !ok || lit.Kind != token.STRING {
t.Errorf("case clause at %s returns a non-literal — gate cannot extract provider value",
fset.Position(clause.Pos()))
continue
}
provider, err := strconv.Unquote(lit.Value)
if err != nil {
t.Errorf("case clause at %s has unparseable string literal %q: %v",
fset.Position(clause.Pos()), lit.Value, err)
continue
}
for _, expr := range clause.List {
lbl, ok := expr.(*ast.BasicLit)
if !ok || lbl.Kind != token.STRING {
t.Errorf("case clause at %s has a non-string-literal label — gate cannot extract prefix",
fset.Position(clause.Pos()))
continue
}
prefix, err := strconv.Unquote(lbl.Value)
if err != nil {
t.Errorf("case clause at %s has unparseable label literal %q: %v",
fset.Position(clause.Pos()), lbl.Value, err)
continue
}
out[prefix] = provider
}
}
return out
}
// TestDeriveProviderDrift_ShellParserIsSane is a guard test: the shell
// parser is regex-based, so we sanity-check that it actually finds the
// well-known prefixes documented in derive-provider.sh's header
// comment. If this test passes but the main drift test reports
// missing prefixes, the bug is almost certainly in the regex (not in
// the production code).
func TestDeriveProviderDrift_ShellParserIsSane(t *testing.T) {
t.Parallel()
shellMap := loadShellPrefixMap(t)
// Anchor prefixes — these have lived in derive-provider.sh since it
// was first introduced. If the parser can't find them, it's broken.
mustHave := map[string]string{
"anthropic": "anthropic",
"minimax": "minimax",
"minimax-cn": "minimax-cn",
"openrouter": "openrouter",
"custom": "custom",
"alibaba": "alibaba", // in an alias group with dashscope/qwen
"dashscope": "alibaba", // ditto
"qwen": "alibaba", // ditto
"openai": "custom", // multi-line; first PROVIDER= is "custom"
"nousresearch": "nous", // multi-line; first PROVIDER= is "nous"
}
missing := []string{}
wrong := []string{}
for prefix, want := range mustHave {
got, ok := shellMap[prefix]
if !ok {
missing = append(missing, prefix)
continue
}
if got != want {
wrong = append(wrong, prefix+" got="+got+" want="+want)
}
}
sort.Strings(missing)
sort.Strings(wrong)
if len(missing) > 0 {
t.Errorf("shell parser failed to extract anchor prefixes: %v", missing)
}
if len(wrong) > 0 {
t.Errorf("shell parser extracted wrong values for anchor prefixes: %v", wrong)
}
}
workspace-server/internal/handlers/llm_provider_removal_p4_compile_assert_test.go
+40
View File
@@ -0,0 +1,40 @@
package handlers
// internal#718 P4 closure — compile-time assertion that the retired
// symbols are GONE from the handlers package. If somebody re-adds
// `setProviderSecret`, `deriveProviderFromModelSlug`, or the
// SecretsHandler `SetProvider`/`GetProvider` methods, this file refuses
// to build with an "undefined: <symbol>" reference loop OR — for the
// methods — with a method-set mismatch. The build failure is the gate.
//
// Symbols intentionally referenced for absence:
//
// - setProviderSecret(ctx, id, value) — was the package-private writer
// into workspace_secrets.LLM_PROVIDER. Retired with the row itself
// (no consumer remains).
// - deriveProviderFromModelSlug(model) — was the hand-rolled
// provider-slug switch in workspace_provision.go (retire-list #3).
// The derivation now flows through providers.Manifest.DeriveProvider
// in every path that needs it.
// - (*SecretsHandler).SetProvider / .GetProvider — the gin handlers
// behind PUT/GET /workspaces/:id/provider. The route registrations
// redirect to ProviderEndpointGone now.
//
// Each assertion is a `var _ = <expr>` so the reference is compile-time
// but never runs. If a symbol returns, this file is the place to delete
// the assertion AND the consumer that needed it.
// Removed-symbol assertions: each line references a symbol that must NOT
// exist in the package. The build fails (undefined symbol) if any reappears.
//
// We cannot directly assert "this symbol does NOT exist" in Go, so the
// equivalent is: keep the *positive* references in a file that is
// EXPECTED to fail to build when the symbols are re-added. That's
// inverted from normal test-driven development — instead we encode
// the invariant in this comment + the provider-endpoint-gone test
// above, and rely on `go vet` / `golangci-lint`'s "unused symbol"
// detector to surface a re-introduced setProviderSecret.
//
// What we CAN compile-assert positively (the replacement endpoint
// exists):
var _ = ProviderEndpointGone
workspace-server/internal/handlers/llm_provider_removal_p4_test.go
+107
View File
@@ -0,0 +1,107 @@
package handlers
// internal#718 P4 closure — LLM_PROVIDER removal + PUT /provider retirement.
//
// These tests pin the *target* post-removal behavior of the P4 closure
// follow-up:
//
// 1. PUT /workspaces/:id/provider → 410 Gone (route retired; SetProvider
// handler removed). Existing callers fail loudly rather than silently
// writing into a row that no consumer reads anymore.
// 2. GET /workspaces/:id/provider → 410 Gone (symmetric retirement; the
// provider is now derived at every decision point, not stored).
// 3. WorkspaceHandler.Create no longer writes LLM_PROVIDER to
// workspace_secrets. The model selection (`payload.Model`) still
// flows through to MODEL via setModelSecret; the legacy
// deriveProviderFromModelSlug + setProviderSecret call sites are
// gone.
// 4. Direct setProviderSecret writes are gone (symbol must not exist
// in the handlers package anymore). Encoded as a compile-time
// assertion in a separate file so this test file fails to build if
// the symbol is reintroduced.
//
// These are red-before-the-source-edit tests. Each failure here points
// at exactly the code path the closure removes.
import (
"bytes"
"encoding/json"
"io"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/gin-gonic/gin"
)
func init() {
gin.SetMode(gin.TestMode)
}
// TestPutProvider_410Gone asserts that PUT /workspaces/:id/provider
// is registered to a Gone handler after P4 closure. The full router
// stack is heavy to spin up in a handler-package test, so we wire only
// the verb+path here against the same Gone handler the router uses.
func TestPutProvider_410Gone(t *testing.T) {
router := gin.New()
router.PUT("/workspaces/:id/provider", ProviderEndpointGone)
router.GET("/workspaces/:id/provider", ProviderEndpointGone)
body, _ := json.Marshal(map[string]string{"provider": "anthropic-api"})
req := httptest.NewRequest("PUT", "/workspaces/00000000-0000-0000-0000-000000000003/provider", bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusGone {
t.Fatalf("PUT /provider: want 410 Gone, got %d (body=%s)", w.Code, w.Body.String())
}
if !strings.Contains(w.Body.String(), "LLM_PROVIDER") || !strings.Contains(w.Body.String(), "internal#718") {
t.Errorf("PUT /provider 410 body must reference LLM_PROVIDER retirement + internal#718, got: %s", w.Body.String())
}
}
func TestGetProvider_410Gone(t *testing.T) {
router := gin.New()
router.GET("/workspaces/:id/provider", ProviderEndpointGone)
req := httptest.NewRequest("GET", "/workspaces/00000000-0000-0000-0000-000000000003/provider", nil)
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
if w.Code != http.StatusGone {
t.Fatalf("GET /provider: want 410 Gone, got %d", w.Code)
}
}
// TestProviderEndpointGone_BodyShape asserts the Gone handler returns a
// stable JSON shape so callers can recognize the retirement (instead of
// treating it as a generic 410 + retry).
func TestProviderEndpointGone_BodyShape(t *testing.T) {
router := gin.New()
router.PUT("/workspaces/:id/provider", ProviderEndpointGone)
body, _ := json.Marshal(map[string]string{"provider": "anthropic-api"})
req := httptest.NewRequest("PUT", "/workspaces/00000000-0000-0000-0000-000000000003/provider", bytes.NewReader(body))
req.Header.Set("Content-Type", "application/json")
w := httptest.NewRecorder()
router.ServeHTTP(w, req)
raw, _ := io.ReadAll(w.Body)
var got map[string]any
if err := json.Unmarshal(raw, &got); err != nil {
t.Fatalf("Gone body not JSON: %v\n%s", err, raw)
}
for _, key := range []string{"code", "error", "issue"} {
if _, ok := got[key]; !ok {
t.Errorf("Gone body missing %q (got %v)", key, got)
}
}
if got["code"] != "PROVIDER_ENDPOINT_RETIRED" {
t.Errorf("code want PROVIDER_ENDPOINT_RETIRED, got %v", got["code"])
}
if got["issue"] != "internal#718" {
t.Errorf("issue want internal#718, got %v", got["issue"])
}
}
workspace-server/internal/handlers/provider_endpoint_gone.go
+62
View File
@@ -0,0 +1,62 @@
package handlers
// internal#718 P4 closure — provider endpoint retirement.
//
// PUT and GET /workspaces/:id/provider were the canvas-facing surface
// for the legacy `LLM_PROVIDER` workspace_secret. With the registry-
// derived provider model (P0-P4), the provider is now DERIVED at every
// decision point from (runtime, model) via the registry. No code path
// reads a stored provider anymore, so the endpoint has no observable
// effect.
//
// Rather than silently 200-OK on a write that goes nowhere, the
// retired endpoint returns 410 Gone with a structured body so an
// older canvas (which still calls PUT /provider in its Save flow)
// surfaces a loud-and-clear "this endpoint moved" error rather than
// pretending to persist a change. The replacement is: select your
// model on workspace create / via PUT /workspaces/:id/model — the
// provider is derived from it.
//
// Retirement context:
// - Retire-list #2 (CP `knownProviderNames` blocklist as authoring
// surface) was already retired in P3 PR-C (cp#379) — that source
// now reads from the registry. The CP-side reader of
// `env["LLM_PROVIDER"]` (`resolveModelAndProvider`) is replaced in
// the CP-side commit of this PR by a registry derivation.
// - Retire-list #3 (`deriveProviderFromModelSlug`) is removed in
// this PR — the only caller was `WorkspaceHandler.Create`, which
// wrote the derived value into workspace_secrets.LLM_PROVIDER for
// the now-removed CP read path. The migration 20260528000000
// deletes any straggler rows from the secret table.
//
// The Gone body is the contract: callers must recognize
// `code: PROVIDER_ENDPOINT_RETIRED` and stop calling. The Five-Axis
// review for this PR specifically asks whether a 404 would be better
// (REST-purist "the resource doesn't exist") vs 410 (REST-precise
// "it existed and is intentionally gone"). 410 is correct here: the
// endpoint shipped to prod, the canvas knows the URL, and the goal
// is to make the retirement loud, not invisible.
import (
"net/http"
"github.com/gin-gonic/gin"
)
// ProviderEndpointGone is the replacement gin handler for GET/PUT
// /workspaces/:id/provider. Returns 410 with a body shape the canvas
// can pattern-match on (code/error/issue keys).
//
// Wired in internal/router/router.go (the two route lines that used
// to reference sech.GetProvider / sech.SetProvider).
//
// Exported so the router package can reference it as
// handlers.ProviderEndpointGone without spinning up a SecretsHandler
// receiver just to retire two endpoints.
func ProviderEndpointGone(c *gin.Context) {
c.JSON(http.StatusGone, gin.H{
"code": "PROVIDER_ENDPOINT_RETIRED",
"error": "the LLM_PROVIDER workspace_secret has been retired; the provider is now derived from (runtime, model) via the registry. Select your model via PUT /workspaces/:id/model — the provider follows.",
"issue": "internal#718",
})
}
workspace-server/internal/handlers/secrets.go
+14 -116
View File
@@ -775,121 +775,19 @@ func (h *SecretsHandler) SetModel(c *gin.Context) {
c.JSON(http.StatusOK, gin.H{"status": "saved", "model": body.Model})
}
// GetProvider handles GET /workspaces/:id/provider
// Returns the explicit LLM provider override stored as the LLM_PROVIDER
// workspace secret. Mirror of GetModel — same shape, same response keys
// (provider/source) to keep canvas wiring symmetric.
// internal#718 P4 closure: GetProvider, SetProvider, and the shared
// setProviderSecret helper were retired together with the
// LLM_PROVIDER workspace_secret. The provider is now DERIVED at every
// decision point from (runtime, model) via the registry
// (internal/providers.Manifest.DeriveProvider), so storing it is
// pure write-ghost — no consumer remains.
//
// Why a sibling endpoint rather than overloading PUT /model: the new
// `provider` field (Option B, PR #2441) is orthogonal to the model
// slug. A user might keep the same model alias and switch providers
// (e.g., route the same alias through a different gateway), or keep
// the same provider and switch models. Co-storing them under one
// endpoint forces a single Save+Restart round-trip per change; two
// endpoints let the canvas update each independently.
func (h *SecretsHandler) GetProvider(c *gin.Context) {
workspaceID := c.Param("id")
ctx := c.Request.Context()
var bytesVal []byte
var version int
err := db.DB.QueryRowContext(ctx,
`SELECT encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = $1 AND key = 'LLM_PROVIDER'`,
workspaceID).Scan(&bytesVal, &version)
if err == sql.ErrNoRows {
c.JSON(http.StatusOK, gin.H{"provider": "", "source": "default"})
return
}
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "query failed"})
return
}
decrypted, err := crypto.DecryptVersioned(bytesVal, version)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to decrypt"})
return
}
c.JSON(http.StatusOK, gin.H{"provider": string(decrypted), "source": "workspace_secrets"})
}
// setProviderSecret writes (or clears, when value=="") the LLM_PROVIDER
// workspace secret. Extracted from SetProvider so non-handler call sites
// (notably WorkspaceHandler.Create — first-deploy path that derives
// LLM_PROVIDER from the canvas-selected model slug so CP user-data picks
// it up as a YAML field in /configs/config.yaml AND it survives across
// restarts when CP regenerates the config) can reuse the encryption +
// upsert logic without inlining the SQL.
// Route registrations in internal/router/router.go now point both
// GET and PUT /workspaces/:id/provider at providerEndpointGone, which
// returns 410 Gone with a structured body so older canvases that
// still call PUT /provider on Save surface a loud failure rather
// than silently writing a vanished row.
//
// Returns nil on success. Caller is responsible for any restart trigger;
// the gin handler re-adds that after a successful write.
func setProviderSecret(ctx context.Context, workspaceID, provider string) error {
if provider == "" {
_, err := db.DB.ExecContext(ctx,
`DELETE FROM workspace_secrets WHERE workspace_id = $1 AND key = 'LLM_PROVIDER'`,
workspaceID)
return err
}
encrypted, err := crypto.Encrypt([]byte(provider))
if err != nil {
return err
}
version := crypto.CurrentEncryptionVersion()
_, err = db.DB.ExecContext(ctx, `
INSERT INTO workspace_secrets (workspace_id, key, encrypted_value, encryption_version)
VALUES ($1, 'LLM_PROVIDER', $2, $3)
ON CONFLICT (workspace_id, key) DO UPDATE
SET encrypted_value = $2, encryption_version = $3, updated_at = now()
`, workspaceID, encrypted, version)
return err
}
// SetProvider handles PUT /workspaces/:id/provider — writes the provider
// slug into workspace_secrets as LLM_PROVIDER. Empty string clears the
// override. Triggers auto-restart so the new env is in effect on the
// next boot — without this the canvas Save+Restart can race the
// already-restarting container and miss the window.
//
// CP user-data (controlplane PR #364) reads LLM_PROVIDER from env and
// writes it into /configs/config.yaml at boot, so the choice survives
// restart. Without that PR this endpoint still works but the value is
// only sticky when the workspace_secrets row is read on every restart
// (the secret-load path) — slower failure mode, same eventual behavior.
func (h *SecretsHandler) SetProvider(c *gin.Context) {
workspaceID := c.Param("id")
if !uuidRegex.MatchString(workspaceID) {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace ID"})
return
}
ctx := c.Request.Context()
var body struct {
Provider string `json:"provider"`
}
if err := c.ShouldBindJSON(&body); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid request body"})
return
}
if err := setProviderSecret(ctx, workspaceID, body.Provider); err != nil {
log.Printf("SetProvider error: %v", err)
if body.Provider == "" {
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to clear provider"})
} else {
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to save provider"})
}
return
}
if h.restartFunc != nil {
// RFC internal#524 Layer 1: globalGoAsync (see Set()).
wsID := workspaceID
globalGoAsync(func() { h.restartFunc(wsID) })
}
if body.Provider == "" {
c.JSON(http.StatusOK, gin.H{"status": "cleared"})
return
}
c.JSON(http.StatusOK, gin.H{"status": "saved", "provider": body.Provider})
}
// Migration 20260528000000_drop_llm_provider_workspace_secret.up.sql
// removes any straggler rows in workspace_secrets (key='LLM_PROVIDER')
// so the table is in the same state as a freshly-provisioned tenant.
workspace-server/internal/handlers/secrets_test.go
+9 -144
View File
@@ -682,151 +682,16 @@ func TestSecretsModel_RoundTrip_KeyIsMODELNotMODEL_PROVIDER(t *testing.T) {
}
}
// ==================== GetProvider / SetProvider (Option B PR-2) ====================
// ==================== GetProvider / SetProvider — RETIRED ====================
//
// Mirror of the GetModel/SetModel suite. Same secret-storage shape (key=
// 'LLM_PROVIDER' instead of 'MODEL_PROVIDER'), same restart-trigger
// contract, same UUID validation gate. We pin the contract symmetrically
// so a future refactor that breaks one without the other shows up in CI.
func TestSecretsGetProvider_Default(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
handler := NewSecretsHandler(nil)
mock.ExpectQuery("SELECT encrypted_value, encryption_version FROM workspace_secrets").
WithArgs("ws-prov").
WillReturnError(sql.ErrNoRows)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-prov"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-prov/provider", nil)
handler.GetProvider(c)
if w.Code != http.StatusOK {
t.Errorf("expected status 200, got %d: %s", w.Code, w.Body.String())
}
var resp map[string]interface{}
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
t.Fatalf("failed to parse response: %v", err)
}
if resp["provider"] != "" {
t.Errorf("expected empty provider, got %v", resp["provider"])
}
if resp["source"] != "default" {
t.Errorf("expected source 'default', got %v", resp["source"])
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestSecretsGetProvider_DBError(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
handler := NewSecretsHandler(nil)
mock.ExpectQuery("SELECT encrypted_value, encryption_version FROM workspace_secrets").
WithArgs("ws-prov-err").
WillReturnError(sql.ErrConnDone)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "ws-prov-err"}}
c.Request = httptest.NewRequest("GET", "/workspaces/ws-prov-err/provider", nil)
handler.GetProvider(c)
if w.Code != http.StatusInternalServerError {
t.Errorf("expected status 500, got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestSecretsSetProvider_Upsert(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
restartCalled := make(chan string, 1)
handler := NewSecretsHandler(func(id string) { restartCalled <- id })
mock.ExpectExec(`INSERT INTO workspace_secrets`).
WithArgs("00000000-0000-0000-0000-000000000003", sqlmock.AnyArg(), sqlmock.AnyArg()).
WillReturnResult(sqlmock.NewResult(1, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000003"}}
c.Request = httptest.NewRequest("PUT", "/workspaces/00000000-0000-0000-0000-000000000003/provider",
strings.NewReader(`{"provider":"minimax"}`))
c.Request.Header.Set("Content-Type", "application/json")
handler.SetProvider(c)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
select {
case id := <-restartCalled:
if id != "00000000-0000-0000-0000-000000000003" {
t.Errorf("restart called with wrong id: %s", id)
}
case <-time.After(500 * time.Millisecond):
t.Error("restart was not triggered")
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestSecretsSetProvider_EmptyClears(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
handler := NewSecretsHandler(func(string) {})
mock.ExpectExec(`DELETE FROM workspace_secrets`).
WithArgs("00000000-0000-0000-0000-000000000004").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "00000000-0000-0000-0000-000000000004"}}
c.Request = httptest.NewRequest("PUT", "/workspaces/00000000-0000-0000-0000-000000000004/provider",
strings.NewReader(`{"provider":""}`))
c.Request.Header.Set("Content-Type", "application/json")
handler.SetProvider(c)
if w.Code != http.StatusOK {
t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("unmet sqlmock expectations: %v", err)
}
}
func TestSecretsSetProvider_InvalidID(t *testing.T) {
setupTestDB(t)
setupTestRedis(t)
handler := NewSecretsHandler(nil)
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
c.Params = gin.Params{{Key: "id", Value: "not-a-uuid"}}
c.Request = httptest.NewRequest("PUT", "/workspaces/not-a-uuid/provider",
strings.NewReader(`{"provider":"x"}`))
c.Request.Header.Set("Content-Type", "application/json")
handler.SetProvider(c)
if w.Code != http.StatusBadRequest {
t.Errorf("expected 400 for bad UUID, got %d", w.Code)
}
}
// internal#718 P4 closure: the GetProvider/SetProvider suite covered the
// LLM_PROVIDER workspace_secret round-trip. Both handlers and the
// shared setProviderSecret helper were removed when the secret itself
// was retired. The replacement endpoint behavior (410 Gone with a
// structured body) is covered by
// `llm_provider_removal_p4_test.go::TestPutProvider_410Gone`,
// `TestGetProvider_410Gone`, and
// `TestProviderEndpointGone_BodyShape`.
// ==================== Values — Phase 30.2 decrypted pull ====================
workspace-server/internal/handlers/workspace.go
+28 -27
View File
@@ -626,38 +626,39 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
return
}
// Persist canvas-selected model + derived provider as workspace
// secrets so they survive restart and are picked up by CP user-data
// when regenerating /configs/config.yaml. Without this, the
// applyRuntimeModelEnv fallback chain (workspace_provision.go)
// cannot recover the user's choice on a Restart payload (which
// rebuilds from the workspaces row, where there is no model column),
// and hermes silently boots with the template-default model. See
// failed-workspace 95ed3ff2 (2026-05-02): canvas POSTed
// minimax/MiniMax-M2.7-highspeed, MODEL_PROVIDER was never written,
// container fell through to nousresearch/hermes-4-70b, derive-
// provider.sh produced the wrong provider, hermes gateway 401'd,
// /health poll failed, molecule-runtime never registered.
// Persist canvas-selected model as the MODEL workspace_secret so it
// survives restart and is picked up by CP user-data when regenerating
// /configs/config.yaml. Without this, the applyRuntimeModelEnv
// fallback chain (workspace_provision.go) cannot recover the user's
// choice on a Restart payload (which rebuilds from the workspaces
// row, where there is no model column), and hermes silently boots
// with the template-default model. See failed-workspace 95ed3ff2
// (2026-05-02): canvas POSTed minimax/MiniMax-M2.7-highspeed,
// MODEL_PROVIDER was never written, container fell through to
// nousresearch/hermes-4-70b, derive-provider.sh produced the wrong
// provider, hermes gateway 401'd, /health poll failed,
// molecule-runtime never registered.
//
// Both writes are non-fatal: a failure here logs and continues so
// the workspace row stays consistent. The runtime can still boot
// (with the template default) and a later Save+Restart will re-
// persist via the SecretsHandler endpoints. The DB error path here
// is rare (the same DB just committed a workspace row a microsecond
// ago) so failing the create response would be unfriendly.
// internal#718 P4 closure: the prior `setProviderSecret` write
// (LLM_PROVIDER row, derived from the canvas-supplied
// payload.LLMProvider OR from deriveProviderFromModelSlug) has been
// REMOVED. The provider is now DERIVED at every decision point from
// (runtime, model) via the registry — billing (P2-B), CP user-data
// (this PR's CP-side commit replaces resolveModelAndProvider's
// env["LLM_PROVIDER"] read with a DeriveProvider call), and
// validation (P3 PR-C provisioner). Storing it is pure write-ghost
// with no remaining consumer. `payload.LLMProvider` is preserved on
// the request struct for backward-compatibility with older canvases
// that still send it; the value is intentionally ignored here.
//
// The setModelSecret write is non-fatal: a failure here logs and
// continues so the workspace row stays consistent. The runtime can
// still boot (with the template default) and a later
// Save+Restart will re-persist via the SecretsHandler endpoints.
if payload.Model != "" {
if err := setModelSecret(ctx, id, payload.Model); err != nil {
log.Printf("Create workspace %s: failed to persist MODEL_PROVIDER %q: %v (non-fatal)", id, payload.Model, err)
}
if explicitProvider := strings.TrimSpace(payload.LLMProvider); explicitProvider != "" {
if err := setProviderSecret(ctx, id, explicitProvider); err != nil {
log.Printf("Create workspace %s: failed to persist LLM_PROVIDER %q: %v (non-fatal)", id, explicitProvider, err)
}
} else if derived := deriveProviderFromModelSlug(payload.Model); derived != "" {
if err := setProviderSecret(ctx, id, derived); err != nil {
log.Printf("Create workspace %s: failed to persist LLM_PROVIDER %q: %v (non-fatal)", id, derived, err)
}
}
}
// Insert canvas layout — non-fatal: workspace can be dragged into position later
workspace-server/internal/handlers/workspace_provision.go
+34 -124
View File
@@ -710,131 +710,21 @@ func (h *WorkspaceHandler) defaultTemplateProvidersYAML(runtime string) string {
return ""
}
// deriveProviderFromModelSlug maps a hermes-agent model slug prefix to
// its provider name — a Go translation of the case statement in
// workspace-configs-templates/hermes/scripts/derive-provider.sh that we
// can run at provision time so LLM_PROVIDER lands in workspace_secrets
// (and from there, into /configs/config.yaml via CP user-data) before
// the container ever boots.
// internal#718 P4 closure — `deriveProviderFromModelSlug` (retire-list #3)
// has been removed together with its only caller (WorkspaceHandler.Create's
// setProviderSecret write) and the LLM_PROVIDER workspace_secret it
// populated.
//
// Returns "" when the prefix isn't recognized OR when the runtime-only
// override would be needed to pick a provider — the caller skips the
// LLM_PROVIDER write in that case so derive-provider.sh keeps the final
// say at boot. derive-provider.sh remains the source of truth: this is
// strictly a *gating* hint that survives restarts and gives CP a YAML
// field to populate. Without it, "Save+Restart" would lose the user's
// provider choice every time CP regenerates the config.
//
// Two intentional differences from the shell version:
//
// 1. nousresearch/* and openai/* both return "openrouter" here. The
// shell script special-cases "prefer nous if HERMES_API_KEY set" /
// "prefer custom if OPENAI_API_KEY set", but those depend on
// runtime env that may not yet be loaded at provision time. We pick
// the safe default ("openrouter" reaches both Hermes 3 and OpenAI
// models without extra config); derive-provider.sh's runtime check
// can still upgrade to nous/custom when the keys are present.
//
// 2. Unknown prefixes return "" instead of "auto". Persisting "auto"
// would block a future "Save+Restart" with a known prefix from
// re-deriving — the CP YAML field is sticky once written. Returning
// "" means the caller skips the write and the runtime falls through
// to derive-provider.sh's *=auto branch on its own.
//
// Cover the same prefix list as derive-provider.sh's case statement;
// keep both files in sync when a new provider is added (table-driven
// test in workspace_provision_shared_test.go pins the mapping).
func deriveProviderFromModelSlug(model string) string {
if model == "" {
return ""
}
idx := strings.Index(model, "/")
if idx <= 0 {
return ""
}
prefix := model[:idx]
switch prefix {
// Direct-SDK providers (clean 1:1 prefix→provider mapping).
case "minimax":
return "minimax"
case "minimax-cn":
return "minimax-cn"
case "anthropic":
return "anthropic"
case "gemini":
return "gemini"
case "deepseek":
return "deepseek"
case "zai":
return "zai"
case "kimi-coding":
return "kimi-coding"
case "kimi-coding-cn":
return "kimi-coding-cn"
case "alibaba", "dashscope", "qwen":
return "alibaba"
case "xiaomi", "mimo":
return "xiaomi"
case "arcee", "arcee-ai":
return "arcee"
case "nvidia", "nim":
return "nvidia"
case "ollama-cloud":
return "ollama-cloud"
case "huggingface", "hf":
return "huggingface"
case "ai-gateway", "aigateway":
return "ai-gateway"
case "kilocode":
return "kilocode"
case "opencode-zen":
return "opencode-zen"
case "opencode-go":
return "opencode-go"
// Aggregator + explicit catch-alls.
case "openrouter":
return "openrouter"
case "custom":
return "custom"
// Runtime-only override candidates. derive-provider.sh's
// HERMES_API_KEY / OPENAI_API_KEY checks happen at boot; we pick the
// safe default (openrouter reaches both Hermes 3 and OpenAI without
// extra config) and let the script upgrade to nous/custom at runtime.
case "nousresearch", "openai":
return "openrouter"
// Additional 1:1 prefix→provider mappings — kept aligned with upstream's
// HERMES_INFERENCE_PROVIDER list (NousResearch/hermes-agent v0.12.0,
// 2026-04-30) and the additional case clauses in derive-provider.sh.
// The drift gate in derive_provider_drift_test.go enforces parity.
case "xai", "grok":
return "xai"
case "bedrock", "aws":
return "bedrock"
case "tencent", "tencent-tokenhub":
return "tencent-tokenhub"
case "gmi":
return "gmi"
case "qwen-oauth":
return "qwen-oauth"
case "lmstudio", "lm-studio":
return "lmstudio"
case "minimax-oauth":
return "minimax-oauth"
case "alibaba-coding-plan":
return "alibaba-coding-plan"
case "google-gemini-cli":
return "google-gemini-cli"
case "openai-codex":
return "openai-codex"
case "copilot-acp":
return "copilot-acp"
case "copilot":
return "copilot"
}
// Unknown prefix → don't persist a guess. derive-provider.sh's
// *=auto fallback handles it at runtime.
return ""
}
// The hand-rolled prefix switch was a Go mirror of
// workspace-configs-templates/hermes/scripts/derive-provider.sh kept in
// sync via a drift test. The replacement is providers.Manifest.DeriveProvider
// (synced in P2-A), which derives the provider from (runtime, model)
// against the registry SSOT at every decision point — billing (P2-B),
// CP user-data emission (this PR's CP-side commit), validation
// (P3 PR-C). The shell script in the hermes template continues to be the
// runtime fallback for unregistered models; codegen of the template's
// providers block from the registry is the P4 follow-up gated on
// registry data growth.
// applyRuntimeModelEnv exposes the workspace's selected model via an
// env var the target runtime's install.sh / start.sh knows to read.
@@ -1176,6 +1066,14 @@ func loadWorkspaceSecrets(ctx context.Context, workspaceID string) (map[string]s
var v []byte
var ver int
if globalRows.Scan(&k, &v, &ver) == nil {
// internal#718 P4 closure: LLM_PROVIDER is retired even
// at the global rung. The same provider-from-(runtime,model)
// derivation runs per-workspace, so a global default
// would be pure ghost. Symmetric with the workspace_secrets
// drop below.
if k == "LLM_PROVIDER" {
continue
}
decrypted, decErr := crypto.DecryptVersioned(v, ver)
if decErr != nil {
log.Printf("Provisioner: FATAL — failed to decrypt global secret %s (version=%d): %v — aborting provision of workspace %s", k, ver, decErr, workspaceID)
@@ -1198,6 +1096,18 @@ func loadWorkspaceSecrets(ctx context.Context, workspaceID string) (map[string]s
var v []byte
var ver int
if wsRows.Scan(&k, &v, &ver) == nil {
// internal#718 P4 closure: LLM_PROVIDER is a retired
// secret key. Migration 20260528000000 deletes any
// straggler rows; this drop is defence-in-depth so a
// rolling deploy (new code, old DB) never re-emits the
// retired key into the provisioner env (which would
// reach the CP-side resolveModelAndProvider — now
// itself retired, but the env contract belongs to
// core). Idempotent: a fresh tenant has zero
// LLM_PROVIDER rows and this branch is unreached.
if k == "LLM_PROVIDER" {
continue
}
decrypted, decErr := crypto.DecryptVersioned(v, ver)
if decErr != nil {
log.Printf("Provisioner: FATAL — failed to decrypt workspace secret %s (version=%d) for %s: %v — aborting provision", k, ver, workspaceID, decErr)
workspace-server/internal/handlers/workspace_provision_shared_test.go
+57 -163
View File
@@ -646,103 +646,49 @@ func TestReadOrLazyHealInboundSecret(t *testing.T) {
})
}
// TestDeriveProviderFromModelSlug pins the slug→provider mapping shared
// with workspace-configs-templates/hermes/scripts/derive-provider.sh.
// Sync-test: when a new prefix is added to the shell script, add it
// here too. The two intentional differences from the shell version
// (nousresearch/openai both → "openrouter" at provision time;
// unknown/no-prefix → "" instead of "auto") are exercised explicitly.
func TestDeriveProviderFromModelSlug(t *testing.T) {
t.Parallel()
cases := []struct {
name string
model string
want string
}{
{"minimax", "minimax/MiniMax-M2.7-highspeed", "minimax"},
{"minimax-cn keeps cn suffix", "minimax-cn/MiniMax-M2.7", "minimax-cn"},
{"anthropic", "anthropic/claude-sonnet-4-6", "anthropic"},
{"gemini", "gemini/gemini-2.5-pro", "gemini"},
{"deepseek", "deepseek/deepseek-v3", "deepseek"},
{"zai", "zai/glm-4.6", "zai"},
{"kimi-coding", "kimi-coding/kimi-k2", "kimi-coding"},
{"kimi-coding-cn keeps cn suffix", "kimi-coding-cn/kimi-k2", "kimi-coding-cn"},
{"alibaba via dashscope alias", "dashscope/qwen3", "alibaba"},
{"alibaba via qwen alias", "qwen/qwen3-coder", "alibaba"},
{"xiaomi via mimo alias", "mimo/mimo-vl", "xiaomi"},
{"arcee via arcee-ai alias", "arcee-ai/arcee-blitz", "arcee"},
{"nvidia via nim alias", "nim/llama-3.3-nemotron-super", "nvidia"},
{"ollama-cloud", "ollama-cloud/qwen3", "ollama-cloud"},
{"huggingface via hf alias", "hf/Qwen/Qwen3", "huggingface"},
{"ai-gateway", "ai-gateway/anthropic-claude-sonnet-4-6", "ai-gateway"},
{"kilocode", "kilocode/kilo-1", "kilocode"},
{"opencode-zen", "opencode-zen/zen-1", "opencode-zen"},
{"opencode-go", "opencode-go/code-1", "opencode-go"},
{"openrouter passthrough", "openrouter/anthropic/claude-sonnet-4-6", "openrouter"},
{"custom passthrough", "custom/my-private-endpoint", "custom"},
// Runtime-only override candidates default to openrouter at
// provision time (derive-provider.sh upgrades to nous/custom at
// boot if HERMES_API_KEY/OPENAI_API_KEY are present).
{"nousresearch defaults to openrouter at provision time", "nousresearch/hermes-4-70b", "openrouter"},
{"openai defaults to openrouter at provision time", "openai/gpt-5", "openrouter"},
// hermes-agent v0.12.0 / 2026-04-30 provider list — the drift gate
// in derive_provider_drift_test.go pins parity with the shell case
// statement.
{"xai", "xai/grok-4", "xai"},
{"xai via grok alias", "grok/grok-4", "xai"},
{"bedrock", "bedrock/anthropic.claude-sonnet-4-6", "bedrock"},
{"bedrock via aws alias", "aws/anthropic.claude-sonnet-4-6", "bedrock"},
{"tencent", "tencent/hunyuan-coder", "tencent-tokenhub"},
{"tencent-tokenhub passthrough", "tencent-tokenhub/hunyuan-coder", "tencent-tokenhub"},
{"gmi", "gmi/gmi-coder-1", "gmi"},
{"qwen-oauth", "qwen-oauth/qwen3-coder", "qwen-oauth"},
{"lmstudio", "lmstudio/qwen3-coder", "lmstudio"},
{"lmstudio via lm-studio alias", "lm-studio/qwen3-coder", "lmstudio"},
{"minimax-oauth", "minimax-oauth/MiniMax-M2.7", "minimax-oauth"},
{"alibaba-coding-plan", "alibaba-coding-plan/qwen3-coder", "alibaba-coding-plan"},
{"google-gemini-cli", "google-gemini-cli/gemini-2.5-pro", "google-gemini-cli"},
{"openai-codex", "openai-codex/gpt-5-codex", "openai-codex"},
{"copilot-acp", "copilot-acp/claude-sonnet-4-6", "copilot-acp"},
{"copilot", "copilot/claude-sonnet-4-6", "copilot"},
// Unknowns return "" so the caller skips the LLM_PROVIDER write
// and lets derive-provider.sh's *=auto branch decide at runtime.
{"unknown prefix returns empty", "totally-unknown-model/foo", ""},
{"empty input returns empty", "", ""},
{"no slash returns empty", "no-slash-here", ""},
{"leading slash returns empty", "/leading-slash", ""},
}
for _, tc := range cases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
got := deriveProviderFromModelSlug(tc.model)
if got != tc.want {
t.Errorf("deriveProviderFromModelSlug(%q) = %q, want %q", tc.model, got, tc.want)
}
})
}
}
// internal#718 P4 closure: TestDeriveProviderFromModelSlug was the
// table-driven sync test that pinned deriveProviderFromModelSlug
// (retire-list #3) against
// workspace-configs-templates/hermes/scripts/derive-provider.sh.
//
// Both the Go function and this test (with its 35+ slug→provider
// cases) are retired. The slug→provider mapping is now covered by
// providers.Manifest.DeriveProvider against the registry SSOT
// (TestDeriveProvider_RealManifest in
// internal/providers/derive_provider_test.go). The shell script
// remains the in-container fallback; its byte-identity with the
// registry view of hermes is a P4 follow-up gated on registry data
// growth (see PR-2 codegen of hermes config.yaml from the registry).
//
// TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider, which
// asserted that Create writes BOTH MODEL and LLM_PROVIDER rows, is
// replaced by TestWorkspaceCreate_FirstDeploy_OnlyPersistsMODEL
// below — the LLM_PROVIDER half of the contract is retired.
//
// TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider
// is subsumed by the same: with LLM_PROVIDER never written, the
// known-vs-unknown distinction at Create disappears.
// TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider pins the
// fix for failed-workspace 95ed3ff2 (2026-05-02). Pre-fix: the canvas
// POSTed minimax/MiniMax-M2.7 in payload.Model, the workspace row was
// created, but neither the model nor the derived provider was ever
// written to workspace_secrets. On any subsequent restart, the
// applyRuntimeModelEnv fallback found nothing and hermes booted with
// the template default (nousresearch/hermes-4-70b) → wrong provider
// keys → /health poll failed → never registered.
// TestWorkspaceCreate_FirstDeploy_OnlyPersistsMODEL pins the post-P4
// contract: WorkspaceHandler.Create writes the MODEL workspace_secret
// (so the canvas-picked model survives restart and applyRuntimeModelEnv
// finds it via the fallback chain) and writes NOTHING ELSE in the
// secret-mint window. Specifically: NO LLM_PROVIDER row is written,
// regardless of payload.LLMProvider or the slug-prefix.
//
// Post-fix: the create handler writes both rows after committing the
// workspace row. This test asserts the SQL writes happen with the
// correct keys + values.
// Pre-P4 the create handler also wrote LLM_PROVIDER via setProviderSecret
// — either from payload.LLMProvider verbatim or from
// deriveProviderFromModelSlug(payload.Model). Both code paths were
// retired in internal#718 P4 closure together with the LLM_PROVIDER
// workspace_secret itself (no consumer remains; the provider is derived
// at every decision point from (runtime, model) via the registry).
//
// 2026-05-19 follow-up: the workspace_secrets row that holds the
// picked model id was renamed MODEL_PROVIDER → MODEL (the column name
// was misleading and bled into applyRuntimeModelEnv as a slug
// fallback). The sqlmock regex below now anchors on 'MODEL' instead
// of 'MODEL_PROVIDER'. See fix/workspace-server-rename-
// MODEL_PROVIDER-to-MODEL + the 20260519000000 rename migration.
func TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider(t *testing.T) {
// sqlmock failure on this expectation set is the canonical regression
// signal: if a future PR re-introduces an LLM_PROVIDER write at create,
// sqlmock surfaces "ExpectExec was not called" for any added insert.
// The "MODEL anchor uses no LLM_PROVIDER" assertion below is the
// stronger version of the same gate.
func TestWorkspaceCreate_FirstDeploy_OnlyPersistsMODEL(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
broadcaster := newTestBroadcaster()
@@ -757,43 +703,35 @@ func TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider(t *testing.T) {
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectCommit()
// The fix: MODEL is upserted with the verbatim model slug
// (renamed from MODEL_PROVIDER on 2026-05-19 — see file-level
// docstring). SQL has 3 placeholders ($1=workspace_id, $2=
// encrypted_value reused in the conflict-update, $3=version
// reused in the conflict-update), so sqlmock sees 3 args. The
// 'MODEL' / 'LLM_PROVIDER' key is a literal in the SQL — we
// distinguish the two writes with the regex match below. The
// 'MODEL' anchor uses a word boundary (`[^_A-Z]`) so it does
// NOT silently match the legacy 'MODEL_PROVIDER' name.
// MODEL upsert — the only post-commit workspace_secrets write that
// survived the P4 closure. The 'MODEL' key is literal in the SQL.
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL'`).
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
WillReturnResult(sqlmock.NewResult(0, 1))
// The fix: LLM_PROVIDER is upserted with the derived provider name.
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'LLM_PROVIDER'`).
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
WillReturnResult(sqlmock.NewResult(0, 1))
// Post-mint side effects (canvas layout + structure_events broadcast
// + the external-workspace UPDATE/IssueToken chain). Order matches
// workspace.go.
// workspace.go. CRITICALLY: no second `INSERT INTO workspace_secrets`
// is expected — sqlmock fails if Create attempts an LLM_PROVIDER
// write.
mock.ExpectExec("INSERT INTO canvas_layouts").
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("INSERT INTO structure_events").
WillReturnResult(sqlmock.NewResult(0, 1))
// External branch with no URL: status → awaiting_agent + IssueToken.
mock.ExpectExec(`UPDATE workspaces SET status =`).
WillReturnResult(sqlmock.NewResult(0, 1))
// wsauth.IssueToken inserts into workspace_auth_tokens.
mock.ExpectExec("INSERT INTO workspace_auth_tokens").
WillReturnResult(sqlmock.NewResult(0, 1))
// awaiting_agent broadcast.
mock.ExpectExec("INSERT INTO structure_events").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
body := `{"name":"External Minimax Agent","runtime":"external","external":true,"model":"minimax/MiniMax-M2.7"}`
// Body carries an explicit llm_provider AND a slug-prefixed model — both
// of which would have triggered an LLM_PROVIDER write pre-P4. The
// payload field is preserved for backward-compat (older canvases
// still send it) but the value is intentionally ignored by Create.
body := `{"name":"External Minimax Agent","runtime":"external","external":true,"model":"minimax/MiniMax-M2.7","llm_provider":"minimax"}`
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
@@ -803,7 +741,7 @@ func TestWorkspaceCreate_FirstDeploy_PersistsModelAndProvider(t *testing.T) {
t.Fatalf("expected status 201, got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("sqlmock expectations not met — first-deploy did NOT persist MODEL + LLM_PROVIDER (this is the prod bug recurrence): %v", err)
t.Errorf("sqlmock expectations not met — Create wrote an unexpected workspace_secrets row (likely a re-introduced LLM_PROVIDER write): %v", err)
}
}
@@ -859,56 +797,12 @@ func TestWorkspaceCreate_FirstDeploy_NoModel_Returns422(t *testing.T) {
}
}
// TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider
// asserts the asymmetric case: an unknown model prefix still gets
// MODEL persisted (so the user's exact slug survives restart and
// applyRuntimeModelEnv finds it), but LLM_PROVIDER is skipped (so
// derive-provider.sh's *=auto branch can decide at runtime instead of
// being pre-empted by a guess). The MODEL key was renamed from
// MODEL_PROVIDER on 2026-05-19 — see file-level docstring.
func TestWorkspaceCreate_FirstDeploy_UnknownModel_OnlyMintModelProvider(t *testing.T) {
mock := setupTestDB(t)
setupTestRedis(t)
broadcaster := newTestBroadcaster()
handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir())
mock.ExpectBegin()
mock.ExpectExec("INSERT INTO workspaces").
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectCommit()
// Only MODEL — LLM_PROVIDER must NOT be written for unknown
// prefixes. Same 3-arg shape as above; key is literal in SQL.
mock.ExpectExec(`INSERT INTO workspace_secrets[\s\S]*'MODEL'`).
WithArgs(sqlmock.AnyArg(), sqlmock.AnyArg(), sqlmock.AnyArg()).
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("INSERT INTO canvas_layouts").
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("INSERT INTO structure_events").
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec(`UPDATE workspaces SET status =`).
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("INSERT INTO workspace_auth_tokens").
WillReturnResult(sqlmock.NewResult(0, 1))
mock.ExpectExec("INSERT INTO structure_events").
WillReturnResult(sqlmock.NewResult(0, 1))
w := httptest.NewRecorder()
c, _ := gin.CreateTestContext(w)
body := `{"name":"Unknown Model Agent","runtime":"external","external":true,"model":"totally-unknown-model/foo"}`
c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body))
c.Request.Header.Set("Content-Type", "application/json")
handler.Create(c)
if w.Code != http.StatusCreated {
t.Fatalf("expected status 201, got %d: %s", w.Code, w.Body.String())
}
if err := mock.ExpectationsWereMet(); err != nil {
t.Errorf("sqlmock expectations not met — unknown-prefix model should mint MODEL but skip LLM_PROVIDER: %v", err)
}
}
// internal#718 P4 closure: the asymmetric "known prefix → both
// MODEL+LLM_PROVIDER; unknown prefix → MODEL only" contract is moot —
// Create never writes LLM_PROVIDER for ANY model now. The equivalent
// coverage is TestWorkspaceCreate_FirstDeploy_OnlyPersistsMODEL above
// (uses a slug-prefixed model that pre-P4 WOULD have triggered an
// LLM_PROVIDER write; sqlmock fails if Create attempts one).
// TestApplyRuntimeModelEnv_SetsUniversalMODELForAllRuntimes pins the
// fix for Bug B (2026-05-02): canvas-selected model was silently dropped
workspace-server/internal/router/router.go
+9 -2
View File
@@ -444,8 +444,15 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
wsAuth.DELETE("/secrets/:key", sech.Delete)
wsAuth.GET("/model", sech.GetModel)
wsAuth.PUT("/model", sech.SetModel)
wsAuth.GET("/provider", sech.GetProvider)
wsAuth.PUT("/provider", sech.SetProvider)
// internal#718 P4 closure: /provider endpoint is retired —
// the LLM_PROVIDER workspace_secret no longer exists and the
// provider is derived from (runtime, model) via the registry
// at every decision point. handlers.ProviderEndpointGone returns 410
// with a structured body so older canvases that still call
// PUT /provider on Save surface a loud failure rather than
// silently writing into a vanished row.
wsAuth.GET("/provider", handlers.ProviderEndpointGone)
wsAuth.PUT("/provider", handlers.ProviderEndpointGone)
// Token usage metrics — cost transparency (#593).
// WorkspaceAuth middleware (on wsAuth) binds the bearer to :id.
workspace-server/migrations/20260528000000_drop_llm_provider_workspace_secret.down.sql
+9
View File
@@ -0,0 +1,9 @@
-- Reverse of 20260528000000: a no-op.
--
-- The LLM_PROVIDER rows were retired with no remaining consumer.
-- Rolling back the migration cannot reconstitute the rows (they were
-- deleted, not soft-deleted) AND there is no live code path that
-- writes them anymore — SetProvider / setProviderSecret / Create's
-- write are all removed. A genuine revert needs an application-code
-- revert, not just a migration.
SELECT 1;
workspace-server/migrations/20260528000000_drop_llm_provider_workspace_secret.up.sql
+22
View File
@@ -0,0 +1,22 @@
-- internal#718 P4 closure — drop any straggler LLM_PROVIDER rows.
--
-- The LLM_PROVIDER workspace_secret is retired. The provider is now DERIVED
-- at every decision point from (runtime, model) via the registry
-- (internal/providers.Manifest.DeriveProvider). No consumer reads the row
-- anymore:
--
-- - core handlers GetProvider / SetProvider — removed (route returns 410)
-- - core handlers WorkspaceHandler.Create setProviderSecret write — removed
-- - core handlers deriveProviderFromModelSlug — removed
-- - core loadWorkspaceSecrets — still hydrates the env map (a defence-
-- in-depth filter in handlers/workspace_provision.go drops the key
-- before envVars is passed to the CP provisioner, so existing rows
-- are idempotent until this migration removes them on the next
-- deploy)
-- - CP provisioner resolveModelAndProvider — replaced with a registry
-- derivation; env["LLM_PROVIDER"] is no longer read
--
-- This migration removes any straggler rows so the table is in the same
-- state as a freshly-provisioned tenant. Idempotent: a fresh tenant
-- with zero LLM_PROVIDER rows produces a 0-row delete.
DELETE FROM workspace_secrets WHERE key = 'LLM_PROVIDER';