fix(canvas#2594): surface env-derived model/provider hint + block Save until picked #2887
@@ -443,6 +443,12 @@ export function ConfigTab({ workspaceId }: Props) {
|
||||
// save propagate-and-restart even when the user didn't touch the model.
|
||||
// Capturing the actual rendered value covers both.
|
||||
const [originalModel, setOriginalModel] = useState("");
|
||||
// core#2594: source of the loaded model value. "workspace_secrets" means
|
||||
// the model is persisted in the DB; "unresolved" means the workspace is
|
||||
// running with a runtime-env-derived model and the canvas has no stored
|
||||
// value to display. Used to show a "derived from environment" hint and to
|
||||
// block Save from appearing to wipe env-derived routing.
|
||||
const [modelSource, setModelSource] = useState<"workspace_secrets" | "unresolved" | null>(null);
|
||||
const successTimerRef = useRef<ReturnType<typeof setTimeout>>(undefined);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -480,11 +486,12 @@ export function ConfigTab({ workspaceId }: Props) {
|
||||
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<{ model?: string; source?: "workspace_secrets" | "unresolved" }>(`/workspaces/${workspaceId}/model`)
|
||||
.catch(() => ({} as { model?: string; source?: "workspace_secrets" | "unresolved" })),
|
||||
]);
|
||||
const wsMetadataRuntime = (wsRes.runtime || "").trim();
|
||||
const wsMetadataModel = (modelRes.model || "").trim();
|
||||
setModelSource(modelRes.source ?? (wsMetadataModel ? "workspace_secrets" : "unresolved"));
|
||||
const wsMetadataTier: number | null =
|
||||
typeof wsRes.tier === "number" ? wsRes.tier : null;
|
||||
setProvider("");
|
||||
@@ -915,6 +922,12 @@ export function ConfigTab({ workspaceId }: Props) {
|
||||
|
||||
const providerDirty = provider !== originalProvider;
|
||||
const isDirty = (rawMode ? rawDraft !== originalYaml : toYaml(config) !== originalYaml) || providerDirty;
|
||||
// core#2594: an env-resolved workspace has no stored model. Saving while
|
||||
// the model dropdown is still empty can't "wipe" routing (handleSave skips
|
||||
// an empty /model PUT), but it is confusing and can stall the user on other
|
||||
// fields without surfacing why. Block save until a model is picked.
|
||||
const modelUnresolved = modelSource === "unresolved" && !currentModelId;
|
||||
const canSave = isDirty && !modelUnresolved;
|
||||
|
||||
if (loading) {
|
||||
return <div className="p-4 text-xs text-ink-mid">Loading config...</div>;
|
||||
@@ -1001,6 +1014,16 @@ export function ConfigTab({ workspaceId }: Props) {
|
||||
))}
|
||||
</select>
|
||||
</div>
|
||||
{/* core#2594: env-resolved workspaces have no stored model/provider.
|
||||
Surface that clearly so users don't see empty required dropdowns
|
||||
on a healthy workspace, and can't hit Save expecting it to stay
|
||||
empty (which would otherwise look like it "wiped" routing). */}
|
||||
{modelSource === "unresolved" && !currentModelId && (
|
||||
<div className="px-2 py-1.5 bg-surface-sunken/50 border border-line/60 rounded text-[10px] text-ink-mid">
|
||||
<span className="font-medium text-ink">Provider and model are derived from the workspace runtime environment.</span>{" "}
|
||||
They are not stored in config.yaml. Select a model below to persist them.
|
||||
</div>
|
||||
)}
|
||||
{/* Shared Provider→Model selector. Same component renders in
|
||||
MissingKeysModal (deploy onboarding) so the dropdown UX is
|
||||
identical across all three surfaces. Provider field maps
|
||||
@@ -1311,7 +1334,7 @@ export function ConfigTab({ workspaceId }: Props) {
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => handleSave(true)}
|
||||
disabled={!isDirty || saving}
|
||||
disabled={!canSave || saving}
|
||||
// Same accent-LIGHTER fix shipped on every other tab.
|
||||
className="px-3 py-1.5 bg-accent hover:bg-accent-strong text-xs rounded text-white disabled:opacity-30 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1 focus-visible:ring-offset-surface"
|
||||
>
|
||||
@@ -1320,7 +1343,7 @@ export function ConfigTab({ workspaceId }: Props) {
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => handleSave(false)}
|
||||
disabled={!isDirty || saving}
|
||||
disabled={!canSave || saving}
|
||||
className="px-3 py-1.5 bg-surface-card hover:bg-surface-card text-xs rounded text-ink-mid disabled:opacity-30 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1"
|
||||
>
|
||||
Save
|
||||
|
||||
@@ -0,0 +1,113 @@
|
||||
// @vitest-environment jsdom
|
||||
//
|
||||
// core#2594: env-resolved workspaces have no stored model/provider (DB MODEL is
|
||||
// NULL and config.yaml is empty). The Config tab must not show empty required
|
||||
// dropdowns as if the workspace is broken; it should explain that values are
|
||||
// derived from the runtime environment and block Save until the user picks a
|
||||
// persisted model.
|
||||
|
||||
import { describe, it, expect, vi, afterEach, beforeEach } from "vitest";
|
||||
import { render, screen, cleanup, waitFor, fireEvent } from "@testing-library/react";
|
||||
import React from "react";
|
||||
|
||||
afterEach(cleanup);
|
||||
|
||||
const apiGet = vi.fn();
|
||||
vi.mock("@/lib/api", () => ({
|
||||
api: {
|
||||
get: (path: string) => apiGet(path),
|
||||
patch: vi.fn(),
|
||||
put: vi.fn(),
|
||||
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 } from "../ConfigTab";
|
||||
|
||||
beforeEach(() => {
|
||||
apiGet.mockReset();
|
||||
apiGet.mockImplementation((path: string) => {
|
||||
if (path === `/workspaces/ws-env`) {
|
||||
return Promise.resolve({ runtime: "claude-code", status: "online" });
|
||||
}
|
||||
if (path === `/workspaces/ws-env/model`) {
|
||||
return Promise.resolve({ model: "", source: "unresolved" });
|
||||
}
|
||||
if (path === `/workspaces/ws-env/files/config.yaml`) {
|
||||
return Promise.resolve({ content: "name: env-resolved\nruntime: claude-code\n" });
|
||||
}
|
||||
if (path === "/templates") {
|
||||
return Promise.resolve([
|
||||
{
|
||||
id: "claude-code",
|
||||
name: "Claude Code",
|
||||
runtime: "claude-code",
|
||||
models: [
|
||||
{ id: "claude-sonnet-4-6", name: "Claude Sonnet 4.6", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] },
|
||||
{ id: "claude-opus-4-7", name: "Claude Opus 4.7", required_env: ["CLAUDE_CODE_OAUTH_TOKEN"] },
|
||||
],
|
||||
providers: ["anthropic-oauth"],
|
||||
},
|
||||
]);
|
||||
}
|
||||
return Promise.reject(new Error(`unmocked api.get: ${path}`));
|
||||
});
|
||||
});
|
||||
|
||||
describe("ConfigTab env-resolved workspace (core#2594)", () => {
|
||||
it("renders a 'derived from environment' hint when model source is unresolved", async () => {
|
||||
render(<ConfigTab workspaceId="ws-env" />);
|
||||
await waitFor(() => expect(apiGet).toHaveBeenCalledWith(`/workspaces/ws-env/model`));
|
||||
expect(screen.getByText(/derived from the workspace runtime environment/i)).toBeTruthy();
|
||||
});
|
||||
|
||||
it("disables Save buttons while model is unresolved and empty", async () => {
|
||||
render(<ConfigTab workspaceId="ws-env" />);
|
||||
await waitFor(() => expect(apiGet).toHaveBeenCalledWith(`/workspaces/ws-env/model`));
|
||||
const saveBtn = screen.getByRole("button", { name: /Save$/ });
|
||||
const saveRestartBtn = screen.getByRole("button", { name: /Save & Restart/i });
|
||||
expect(saveBtn.hasAttribute("disabled")).toBe(true);
|
||||
expect(saveRestartBtn.hasAttribute("disabled")).toBe(true);
|
||||
});
|
||||
|
||||
it("enables Save after the user selects a model", async () => {
|
||||
render(<ConfigTab workspaceId="ws-env" />);
|
||||
await waitFor(() => expect(apiGet).toHaveBeenCalledWith(`/workspaces/ws-env/model`));
|
||||
|
||||
// Open provider dropdown and pick the only provider.
|
||||
const providerSelect = screen.getByTestId("provider-select") as HTMLSelectElement;
|
||||
fireEvent.change(providerSelect, { target: { value: "anthropic|CLAUDE_CODE_OAUTH_TOKEN" } });
|
||||
await waitFor(() => expect(providerSelect.value).toBe("anthropic|CLAUDE_CODE_OAUTH_TOKEN"));
|
||||
|
||||
// Open model dropdown and pick a model.
|
||||
const modelSelect = screen.getByTestId("model-select") as HTMLSelectElement;
|
||||
await waitFor(() => expect(modelSelect.disabled).toBe(false));
|
||||
fireEvent.change(modelSelect, { target: { value: "claude-sonnet-4-6" } });
|
||||
await waitFor(() => expect(modelSelect.value).toBe("claude-sonnet-4-6"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole("button", { name: /Save$/ }).hasAttribute("disabled")).toBe(false);
|
||||
expect(screen.getByRole("button", { name: /Save & Restart/i }).hasAttribute("disabled")).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user