From 5c80b9c3d693f9ab6a6a8568bb92a4aedb50c4d5 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Mon, 4 May 2026 15:14:40 -0700 Subject: [PATCH] feat(canvas): render misconfigured workspaces with the configuration_status from agent_card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes molecule-controlplane#467 (issue filed against CP, but resolution landed canvas-side because the workspace-server ALREADY returns the agent_card JSONB blob with configuration_status / configuration_error fields populated by molecule-core PR #2756). No CP-side change needed — the gap was the canvas's blindness to those fields. Before this PR, a workspace whose adapter.setup() failed (typically missing/rotated LLM credential) appeared identical to a healthy one in the canvas tile: green "Online" status, no error indication. The operator had to dig into workspace logs to discover the env var to set. This PR surfaces the state via the existing status-pill UX: 1. STATUS_CONFIG gains a "not_configured" entry — amber dot/glow, "Not configured" label. Distinct from "online" (emerald) and "failed" (red) — the workspace is reachable, it just needs config. 2. canvas-topology exposes getConfigurationStatus / getConfigurationError helpers — strict equality on the JSONB field so unknown values pass through as null instead of crashing the tile renderer. 3. WorkspaceNode derives an `effectiveStatus` that overrides data.status with "not_configured" when (status === "online" AND agent_card.configuration_status === "not_configured"). The override only applies on top of "online" — a genuinely offline / failed / provisioning workspace keeps its existing treatment. 4. The configuration_error string surfaces in two places: the tile's aria-label (screen reader access) + a truncated preview row at the bottom of the tile (same visual as the existing "degraded error preview" — mirrors the established pattern for in-tile error surfacing). Test coverage: 11 new in canvas-topology-configuration-status.test.ts. Each helper covered for the happy path, missing fields, defensive ignores of unknown values, and an end-to-end "stale ready overrides old error" guard. Once this lands + canvas redeploys, operators see "Not configured: Neither OPENAI_API_KEY nor MINIMAX_API_KEY is set" right on the workspace tile instead of a confused-looking green "online" workspace that silently 503s every JSON-RPC request. Pairs with: molecule-core PR #2756 (decouple agent-card from setup), #2775 (boot_routes pin), #2778 (secret_redactor) Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/WorkspaceNode.tsx | 52 ++++++++- canvas/src/lib/design-tokens.ts | 7 ++ ...nvas-topology-configuration-status.test.ts | 103 ++++++++++++++++++ canvas/src/store/canvas-topology.ts | 39 +++++++ 4 files changed, 195 insertions(+), 6 deletions(-) create mode 100644 canvas/src/store/__tests__/canvas-topology-configuration-status.test.ts diff --git a/canvas/src/components/WorkspaceNode.tsx b/canvas/src/components/WorkspaceNode.tsx index b2154dd3..2579d8fb 100644 --- a/canvas/src/components/WorkspaceNode.tsx +++ b/canvas/src/components/WorkspaceNode.tsx @@ -3,6 +3,7 @@ import { useCallback, useMemo } from "react"; import { Handle, NodeResizer, Position, type NodeProps, type Node } from "@xyflow/react"; import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; +import { getConfigurationError, getConfigurationStatus } from "@/store/canvas-topology"; import { showToast } from "@/components/Toaster"; import { Tooltip } from "@/components/Tooltip"; import { STATUS_CONFIG, TIER_CONFIG } from "@/lib/design-tokens"; @@ -35,8 +36,28 @@ function EjectIcon(props: React.SVGProps) { } export function WorkspaceNode({ id, data }: NodeProps>) { - const statusCfg = STATUS_CONFIG[data.status] || STATUS_CONFIG.offline; + // Configuration-status overlay (PR #2756 / #467 chain). When the + // workspace is reachable but adapter.setup() failed (typically a + // missing/rotated LLM credential), the agent_card carries + // configuration_status: "not_configured". Surface this as a distinct + // tile state so the operator sees a useful error instead of an + // ambiguous "online but silent" workspace. + // + // The override only applies when the underlying status is "online" — + // a workspace that's actually offline / failed / provisioning gets + // its own treatment. "online + not_configured" is the gap PR #2756 + // introduced; everything else was already covered. + const isMisconfigured = + data.status === "online" && + getConfigurationStatus(data.agentCard) === "not_configured"; + const configurationError = getConfigurationError(data.agentCard); + const effectiveStatus = isMisconfigured ? "not_configured" : data.status; + const statusCfg = STATUS_CONFIG[effectiveStatus] || STATUS_CONFIG.offline; const tierCfg = TIER_CONFIG[data.tier] || { label: `T${data.tier}`, color: "text-ink-mid bg-surface-card border border-line" }; + const tooltipExtra = isMisconfigured && configurationError + ? `Agent not configured: ${configurationError}` + : null; + void tooltipExtra; // wired in via aria-label below; reserved here for future tooltip surface. // Org-deploy context — four derived flags off one store subscription. // Drives the shimmer while provisioning, the dimmed/non-draggable // treatment on locked descendants, and the Cancel pill on the root. @@ -75,7 +96,12 @@ export function WorkspaceNode({ id, data }: NodeProps>)
{ e.stopPropagation(); @@ -283,11 +309,12 @@ export function WorkspaceNode({ id, data }: NodeProps>) {/* Bottom row: status / active tasks */}
- {data.status !== "online" ? ( + {effectiveStatus !== "online" ? (
{statusCfg.label} @@ -313,6 +340,19 @@ export function WorkspaceNode({ id, data }: NodeProps>) {data.lastSampleError}
)} + + {/* Configuration error preview — same visual as the degraded + * error preview but keyed off the agent_card's configuration_status. + * Tells the operator which env var is missing so they can fix it + * without having to dig into the workspace logs. */} + {isMisconfigured && configurationError && ( +
+ {configurationError} +
+ )}
{ + it("returns null when agentCard is null", () => { + expect(getConfigurationStatus(null)).toBe(null); + }); + + it("returns null when agentCard has no configuration_status", () => { + expect(getConfigurationStatus({ name: "x" })).toBe(null); + }); + + it("returns 'ready' when agent reports configuration ok", () => { + expect( + getConfigurationStatus({ configuration_status: "ready" }), + ).toBe("ready"); + }); + + it("returns 'not_configured' when agent reports setup failed", () => { + expect( + getConfigurationStatus({ configuration_status: "not_configured" }), + ).toBe("not_configured"); + }); + + it("ignores unknown values defensively", () => { + // A future agent reporting a status string we don't yet recognise + // shouldn't crash the canvas — we treat it as 'no info' (null). + expect( + getConfigurationStatus({ configuration_status: "starting" }), + ).toBe(null); + expect( + getConfigurationStatus({ configuration_status: 42 }), + ).toBe(null); + expect( + getConfigurationStatus({ configuration_status: null }), + ).toBe(null); + }); +}); + +describe("getConfigurationError", () => { + it("returns null when agentCard is null", () => { + expect(getConfigurationError(null)).toBe(null); + }); + + it("returns null when status is 'ready' even if error string present", () => { + // Defensive: if the agent somehow ships configuration_status=ready + // alongside a stale configuration_error from a previous boot, we + // trust the live status flag and don't surface the stale error. + expect( + getConfigurationError({ + configuration_status: "ready", + configuration_error: "stale: was unset", + }), + ).toBe(null); + }); + + it("returns the error string when status is 'not_configured'", () => { + expect( + getConfigurationError({ + configuration_status: "not_configured", + configuration_error: + "RuntimeError: Neither OPENAI_API_KEY nor MINIMAX_API_KEY is set", + }), + ).toBe( + "RuntimeError: Neither OPENAI_API_KEY nor MINIMAX_API_KEY is set", + ); + }); + + it("returns null when status is 'not_configured' but error is missing", () => { + expect( + getConfigurationError({ configuration_status: "not_configured" }), + ).toBe(null); + }); + + it("returns null when error is empty string", () => { + // Empty string isn't actionable for the operator — treat same as + // missing. + expect( + getConfigurationError({ + configuration_status: "not_configured", + configuration_error: "", + }), + ).toBe(null); + }); + + it("returns null when error is non-string", () => { + expect( + getConfigurationError({ + configuration_status: "not_configured", + configuration_error: { reason: "object" }, + }), + ).toBe(null); + }); +}); diff --git a/canvas/src/store/canvas-topology.ts b/canvas/src/store/canvas-topology.ts index 396b89ff..334dcff7 100644 --- a/canvas/src/store/canvas-topology.ts +++ b/canvas/src/store/canvas-topology.ts @@ -564,3 +564,42 @@ export function extractSkillNames(agentCard: Record | null): st .map((skill: Record) => String(skill.name || skill.id || "")) .filter(Boolean); } + +/** + * Returns the configuration status reported by the workspace, or null + * when the agent card doesn't carry one (older runtime, or pre-PR #2756 + * worker). + * + * Pairs with molecule-core PR #2756: when adapter.setup() fails, the + * runtime mounts a not-configured handler AND advertises the failure + * via agent_card.configuration_status = "not_configured" + + * configuration_error = "". Canvas reads both to render a + * "needs config" tile instead of a confused "online but silent" state. + * + * Returns null (not undefined) so callers can distinguish "no info" + * from explicit values via a strict equality check. + */ +export function getConfigurationStatus( + agentCard: Record | null, +): "ready" | "not_configured" | null { + if (!agentCard) return null; + const raw = agentCard.configuration_status; + if (raw === "ready" || raw === "not_configured") return raw; + return null; +} + +/** + * Returns the configuration error string from the agent card when + * configuration_status is "not_configured", or null otherwise. + * + * Already redacted server-side via secret_redactor (PR #2778) — safe to + * render in the UI verbatim. + */ +export function getConfigurationError( + agentCard: Record | null, +): string | null { + if (!agentCard) return null; + if (getConfigurationStatus(agentCard) !== "not_configured") return null; + const raw = agentCard.configuration_error; + return typeof raw === "string" && raw.length > 0 ? raw : null; +}