Merge remote-tracking branch 'origin/main' into trig-178

This commit is contained in:
Molecule AI Core Platform Lead 2026-05-09 22:02:51 +00:00
commit e716d699e9
9 changed files with 404 additions and 26 deletions

View File

@ -1,6 +1,6 @@
"use client";
import { useCallback, useMemo } from "react";
import { useCallback, useEffect, useMemo, useRef } from "react";
import {
ReactFlow,
ReactFlowProvider,
@ -187,6 +187,23 @@ function CanvasInner() {
// Pan-to-node / zoom-to-team CustomEvent listeners + viewport save.
const { onMoveEnd } = useCanvasViewport();
// Screen-reader announcements — read liveAnnouncement from the store and
// immediately clear it so the same announcement doesn't re-fire on
// re-render. Using a ref avoids a setState loop while keeping the
// effect reactive to new announcement strings.
const liveAnnouncement = useCanvasStore((s) => s.liveAnnouncement);
const clearAnnouncement = useCanvasStore((s) => s.setLiveAnnouncement);
const prevAnnouncement = useRef("");
useEffect(() => {
if (liveAnnouncement && liveAnnouncement !== prevAnnouncement.current) {
prevAnnouncement.current = liveAnnouncement;
// Small delay so the DOM update lands before clearing, giving
// screen readers time to pick up the new text.
const timer = setTimeout(() => clearAnnouncement(""), 500);
return () => clearTimeout(timer);
}
}, [liveAnnouncement, clearAnnouncement]);
// Delete-confirmation lives in the store so the dialog survives ContextMenu
// unmounting — the prior local-in-ContextMenu state raced with the menu's
// outside-click handler.
@ -326,11 +343,21 @@ function CanvasInner() {
<DropTargetBadge />
</ReactFlow>
{/* Screen-reader live region: announces workspace count on canvas load or change */}
<div role="status" aria-live="polite" className="sr-only">
{nodes.filter((n) => !n.parentId).length === 0
? "No workspaces on canvas"
: `${nodes.filter((n) => !n.parentId).length} workspace${nodes.filter((n) => !n.parentId).length !== 1 ? "s" : ""} on canvas`}
{/* Screen-reader live region announces workspace count on initial load and
live status updates from WebSocket events (online, offline, provisioning, etc.).
The liveAnnouncement text is cleared after the screen reader has had time
to read it so the same message doesn't re-announce on re-render. */}
<div
role="status"
aria-live="polite"
aria-atomic="true"
className="sr-only"
>
{liveAnnouncement || (
nodes.filter((n) => !n.parentId).length === 0
? "No workspaces on canvas"
: `${nodes.filter((n) => !n.parentId).length} workspace${nodes.filter((n) => !n.parentId).length !== 1 ? "s" : ""} on canvas`
)}
</div>
{nodes.length === 0 && <EmptyState />}

View File

@ -513,7 +513,20 @@ function GroupedCommsView({
/>
<div className="flex-1 overflow-y-auto p-3 space-y-2">
{visible.map((msg) =>
msg.status === "error" ? (
// Only render the error UI when there is NO usable response
// content. A "error" status from the platform means the HTTP
// transport layer had a problem — but the agent response text
// may have arrived and been stored in response_body.text.
// Delegation results set responseText via extractResponseText
// once that function learned to parse body.text, so checking
// !msg.responseText here correctly identifies "no actual reply
// was received" vs. "reply arrived but status=error".
//
// Without this guard, successful delegation results were
// rendered as error banners, PMs saw "restart" prompts and
// restarted working agents, and retry storms formed as the
// platform re-delivered the same completed work (issue #159).
msg.status === "error" && !msg.responseText ? (
<ErrorMessage key={msg.id} msg={msg} />
) : (
<NormalMessage key={msg.id} msg={msg} />

View File

@ -4,9 +4,11 @@ import { render, screen, fireEvent, waitFor } from "@testing-library/react";
// API mock — tests can override per case via apiGetMock.mockImplementationOnce.
const apiGetMock = vi.fn<(url: string) => Promise<unknown>>();
const apiPostMock = vi.fn<(url: string, body?: unknown) => Promise<unknown>>();
vi.mock("@/lib/api", () => ({
api: {
get: (url: string) => apiGetMock(url),
post: (url: string, body?: unknown) => apiPostMock(url, body),
},
}));
@ -16,17 +18,23 @@ vi.mock("@/hooks/useSocketEvent", () => ({
useSocketEvent: () => {},
}));
// Canvas store — peer name resolution.
vi.mock("@/store/canvas", () => ({
useCanvasStore: {
getState: () => ({
nodes: [
{ id: "ws-self", data: { name: "Self" } },
{ id: "ws-peer", data: { name: "Peer Agent" } },
],
}),
},
}));
// Canvas store — peer name resolution + ErrorMessage requires selectNode
// (Zustand hook usage). The mock must support BOTH:
// useCanvasStore.getState().nodes (plain object with getState)
// useCanvasStore((s) => s.selectNode) (Zustand hook with selector)
vi.mock("@/store/canvas", () => {
const state = {
nodes: [
{ id: "ws-self", data: { name: "Self" } },
{ id: "ws-peer", data: { name: "Peer Agent" } },
],
selectNode: vi.fn(),
};
const hook = (selector?: (s: typeof state) => unknown) =>
selector ? selector(state) : state;
hook.getState = () => state;
return { useCanvasStore: hook };
});
// Toaster shim — AgentCommsPanel imports showToast.
vi.mock("../../Toaster", () => ({
@ -41,6 +49,8 @@ import { AgentCommsPanel } from "../AgentCommsPanel";
const scrollSpy = vi.fn<(opts?: ScrollIntoViewOptions | boolean) => void>();
beforeEach(() => {
apiGetMock.mockReset();
apiPostMock.mockReset();
apiPostMock.mockResolvedValue({});
scrollSpy.mockReset();
Element.prototype.scrollIntoView = scrollSpy as unknown as Element["scrollIntoView"];
});
@ -49,6 +59,81 @@ afterEach(() => {
vi.clearAllMocks();
});
// Regression test: when a delegation succeeds but the platform persisted
// status="error" (transport-layer HTTP failure, not agent failure), the
// canvas had the response text in msg.text but rendered ErrorMessage
// anyway, burying the real content in an "Underlying error" banner and
// prompting PMs to restart working agents (issue #159).
describe("AgentCommsPanel — error rendering guard (issue #159)", () => {
it("renders NormalMessage when status=error but msg.text is present (successful delegation)", async () => {
// Simulate a delegation result where status="error" (HTTP transport
// failed) but response_body.text carries the actual agent response.
// The correct behaviour: show the content as a normal inbound bubble,
// NOT an error banner.
apiGetMock.mockResolvedValueOnce([
{
id: "act-1",
activity_type: "delegation",
method: "delegate_result",
source_id: "ws-self",
target_id: "ws-peer",
summary: "Delegation completed",
request_body: null,
// delegation.go stores response_body as {text: "...", delegation_id: "..."}
response_body: {
text: "PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)",
delegation_id: "delg_01jx8q4n3k",
},
status: "error", // transport-layer error, not agent failure
created_at: "2026-04-25T18:00:00Z",
},
]);
render(<AgentCommsPanel workspaceId="ws-self" />);
// The response text should appear in a normal inbound bubble, NOT in
// an error banner. Specifically: no "Failed to deliver" or "returned
// an error" text should appear.
await waitFor(() => {
expect(screen.queryByText(/failed to deliver/i)).toBeNull();
expect(screen.queryByText(/returned an error/i)).toBeNull();
});
// The actual content must be visible.
await waitFor(() =>
expect(
screen.getByText(/tier-check fails NO REVIEWS/i),
).toBeDefined(),
);
});
it("renders ErrorMessage when status=error and msg.text is absent (true failure)", async () => {
// True delivery failure: no response body, no text. The error banner
// IS appropriate here.
apiGetMock.mockResolvedValueOnce([
{
id: "act-1",
activity_type: "a2a_send",
source_id: "ws-self",
target_id: "ws-peer",
method: "message/send",
summary: "A2A send failed",
request_body: null,
response_body: null,
status: "error",
created_at: "2026-04-25T18:00:00Z",
},
]);
render(<AgentCommsPanel workspaceId="ws-self" />);
// Error banner IS shown for true failures (no content).
// jsdom doesn't reliably match role="alert" in getByRole, so use
// getByText instead.
const errorBanner = await waitFor(() =>
screen.getByText(/failed to deliver/i),
);
expect(errorBanner).toBeDefined();
});
});
describe("AgentCommsPanel — initial-state parity with ChatTab my-chat", () => {
it("shows loading text while history fetch is in flight", () => {
apiGetMock.mockReturnValueOnce(new Promise(() => { /* never resolves */ }));

View File

@ -209,6 +209,43 @@ describe("extractResponseText", () => {
};
expect(extractResponseText(body)).toBe("Summary\nDetail block one\nDetail block two");
});
// Regression: delegation.go stores response_body as
// {"text": "...", "delegation_id": "..."} — no "result" wrapper.
// Without body.text handling, extractResponseText returns "" for
// delegate_result rows, causing the error UI to fire even when the
// delegation succeeded (issue #159).
it("extracts from body.text (delegation response_body shape)", () => {
const body = {
text: "PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)",
delegation_id: "delg_01jx8q4n3k",
};
expect(extractResponseText(body)).toBe(
"PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)"
);
});
it("prefers body.result over body.text when both present", () => {
const body = {
result: { parts: [{ kind: "text", text: "A2A result wins" }] },
text: "Delegation text",
};
// result path is checked first; A2A wins when both present.
expect(extractResponseText(body)).toBe("A2A result wins");
});
it("returns empty string when body.text is empty string", () => {
expect(extractResponseText({ text: "" })).toBe("");
});
it("extracts from body.response_preview (DELEGATION_COMPLETE WS event shape)", () => {
const body = {
response_preview: "PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)",
};
expect(extractResponseText(body)).toBe(
"PR #149: tier-check fails NO REVIEWS (author needs engineers/managers/ceo approval)"
);
});
});
describe("extractTextsFromParts", () => {

View File

@ -168,10 +168,10 @@ export function extractResponseText(body: Record<string, unknown>): string {
if (rootTexts.length > 0) collected.push(rootTexts.join("\n"));
// Task shape: {result: {artifacts: [{parts: [...]}]}}
const artifacts = result.artifacts as Array<Record<string, unknown>> | undefined;
const artifacts = result.artifacts as Array<Record<string, unknown> | undefined>;
if (artifacts) {
for (const a of artifacts) {
const t = extractTextsFromParts(a.parts);
const t = extractTextsFromParts(a?.parts);
if (t) collected.push(t);
}
}
@ -179,6 +179,20 @@ export function extractResponseText(body: Record<string, unknown>): string {
if (collected.length > 0) return collected.join("\n");
}
// Delegation results from delegation.go store response_body as
// {"text": "...", "delegation_id": "..."} — no "result" wrapper.
// Check this after the body.result path so A2A responses take
// precedence when both shapes are somehow present.
// Without this, responseText is always "" for delegate_result rows,
// causing the error UI to fire even when the delegation succeeded
// (issue #159).
if (typeof body.text === "string" && body.text) return body.text;
// DELEGATION_COMPLETE event (via canvas-events WS handler) stores
// response_body as {response_preview: "..."}. Handle this too.
if (typeof body.response_preview === "string" && body.response_preview) {
return body.response_preview;
}
// {task: "text"} — request body format, shouldn't be in response but handle it
if (typeof body.task === "string") return body.task;
} catch { /* ignore */ }

View File

@ -835,3 +835,181 @@ describe("handleCanvasEvent unknown event", () => {
).not.toThrow();
});
});
// ---------------------------------------------------------------------------
// Screen-reader live announcements
// ---------------------------------------------------------------------------
describe("handleCanvasEvent liveAnnouncement", () => {
it("announces WORKSPACE_ONLINE with node name", () => {
const node = makeNode("ws-1", { name: "Alpha" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_ONLINE", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Alpha is now online");
});
it("announces WORKSPACE_OFFLINE with node name", () => {
const node = makeNode("ws-1", { name: "Beta" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_OFFLINE", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Beta is now offline");
});
it("announces WORKSPACE_PAUSED with node name", () => {
const node = makeNode("ws-1", { name: "Gamma" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_PAUSED", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Gamma has been paused");
});
it("announces WORKSPACE_DEGRADED with node name", () => {
const node = makeNode("ws-1", { name: "Delta" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_DEGRADED",
workspace_id: "ws-1",
payload: { sample_error: "connection timeout" },
}),
get,
set
);
expect(state.liveAnnouncement).toBe("Delta is degraded");
});
it("announces WORKSPACE_PROVISIONING for new workspace with payload name", () => {
const { get, set, state } = makeStore([]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_PROVISIONING",
workspace_id: "ws-new",
payload: { name: "NewBot" },
}),
get,
set
);
expect(state.liveAnnouncement).toBe("NewBot is provisioning");
});
it("announces WORKSPACE_PROVISIONING for new workspace with default name", () => {
const { get, set, state } = makeStore([]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_PROVISIONING",
workspace_id: "ws-new",
payload: {},
}),
get,
set
);
expect(state.liveAnnouncement).toBe("New Workspace is provisioning");
});
it("announces WORKSPACE_REMOVED with node name", () => {
const node = makeNode("ws-1", { name: "Gamma" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({ event: "WORKSPACE_REMOVED", workspace_id: "ws-1" }),
get,
set
);
expect(state.liveAnnouncement).toBe("Gamma was removed");
});
it("announces WORKSPACE_PROVISION_FAILED with node name", () => {
const node = makeNode("ws-1", { name: "Delta" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_PROVISION_FAILED",
workspace_id: "ws-1",
payload: { error: "docker pull failed" },
}),
get,
set
);
expect(state.liveAnnouncement).toBe("Delta provisioning failed");
});
it("does not announce for TASK_UPDATED", () => {
const node = makeNode("ws-1", { name: "Alpha" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "TASK_UPDATED",
workspace_id: "ws-1",
payload: { current_task: "building release", active_tasks: 1 },
}),
get,
set
);
// TASK_UPDATED is noisy (every heartbeat); it should not announce
expect(state.liveAnnouncement ?? "").toBe("");
});
it("does not announce for AGENT_MESSAGE", () => {
const node = makeNode("ws-1", { name: "Alpha" });
const { get, set, state } = makeStore([node]);
handleCanvasEvent(
makeMsg({
event: "AGENT_MESSAGE",
workspace_id: "ws-1",
payload: { message: "hello from the agent" },
}),
get,
set
);
expect(state.liveAnnouncement ?? "").toBe("");
});
it("uses payload name for ONLINE when node not found in store", () => {
const { get, set, state } = makeStore([]);
handleCanvasEvent(
makeMsg({
event: "WORKSPACE_ONLINE",
workspace_id: "ws-1",
payload: { name: "FromPayload" },
}),
get,
set
);
// ONLINE when node doesn't exist just buffers _pendingOnline;
// no announcement should be set
expect(state.liveAnnouncement ?? "").toBe("");
});
});
});

View File

@ -80,6 +80,7 @@ export function handleCanvasEvent(
switch (msg.event) {
case "WORKSPACE_ONLINE": {
const existing = nodes.find((n) => n.id === msg.workspace_id);
const nodeName = existing?.data.name ?? (msg.payload.name as string) ?? "Workspace";
if (!existing) {
// PROVISIONING event hasn't been applied yet (WS reorder or
// this tab joined mid-deploy). Buffer so the later PROVISIONING
@ -105,6 +106,7 @@ export function handleCanvasEvent(
? { ...n, data: { ...n.data, status: "online" } }
: n,
),
liveAnnouncement: `${nodeName} is now online`,
});
// Remove the laser class after its keyframe ends so the edge
// settles into the app's default solid styling. Fire-and-forget.
@ -123,28 +125,36 @@ export function handleCanvasEvent(
}
case "WORKSPACE_OFFLINE": {
const offlineNode = nodes.find((n) => n.id === msg.workspace_id);
const offlineName = offlineNode?.data.name ?? "Workspace";
set({
nodes: nodes.map((n) =>
n.id === msg.workspace_id
? { ...n, data: { ...n.data, status: "offline" } }
: n
),
liveAnnouncement: `${offlineName} is now offline`,
});
break;
}
case "WORKSPACE_PAUSED": {
const pausedNode = nodes.find((n) => n.id === msg.workspace_id);
const pausedName = pausedNode?.data.name ?? "Workspace";
set({
nodes: nodes.map((n) =>
n.id === msg.workspace_id
? { ...n, data: { ...n.data, status: "paused", currentTask: "" } }
: n
),
liveAnnouncement: `${pausedName} has been paused`,
});
break;
}
case "WORKSPACE_DEGRADED": {
const degradedNode = nodes.find((n) => n.id === msg.workspace_id);
const degradedName = degradedNode?.data.name ?? "Workspace";
set({
nodes: nodes.map((n) =>
n.id === msg.workspace_id
@ -160,6 +170,7 @@ export function handleCanvasEvent(
}
: n
),
liveAnnouncement: `${degradedName} is degraded`,
});
break;
}
@ -230,6 +241,7 @@ export function handleCanvasEvent(
// removed per demo feedback. A2A edges (showA2AEdges) still
// render when enabled — those represent runtime traffic,
// which nesting doesn't express.
const newNodeName = (msg.payload.name as string) ?? "New Workspace";
set({
nodes: [
...nodes,
@ -244,7 +256,7 @@ export function handleCanvasEvent(
...(parentId ? { parentId } : {}),
className: "mol-deploy-spawn",
data: {
name: (msg.payload.name as string) ?? "New Workspace",
name: newNodeName,
status: "provisioning",
tier: (msg.payload.tier as number) ?? 1,
agentCard: null,
@ -261,6 +273,7 @@ export function handleCanvasEvent(
},
},
],
liveAnnouncement: `${newNodeName} is provisioning`,
});
// Grow the parent to fit the just-landed child. DEBOUNCED
@ -345,6 +358,7 @@ export function handleCanvasEvent(
case "WORKSPACE_REMOVED": {
const removedNode = nodes.find((n) => n.id === msg.workspace_id);
const removedName = removedNode?.data.name ?? "Workspace";
const parentOfRemoved = removedNode?.data.parentId ?? null;
set({
nodes: nodes
@ -363,6 +377,7 @@ export function handleCanvasEvent(
e.source !== msg.workspace_id && e.target !== msg.workspace_id
),
selectedNodeId: selectedNodeId === msg.workspace_id ? null : selectedNodeId,
liveAnnouncement: `${removedName} was removed`,
});
break;
}
@ -445,6 +460,8 @@ export function handleCanvasEvent(
case "WORKSPACE_PROVISION_FAILED": {
const errorMsg = (msg.payload.error as string) ?? "Unknown provisioning error";
const failedNode = nodes.find((n) => n.id === msg.workspace_id);
const failedName = failedNode?.data.name ?? "Workspace";
set({
nodes: nodes.map((n) =>
n.id === msg.workspace_id
@ -458,6 +475,7 @@ export function handleCanvasEvent(
}
: n
),
liveAnnouncement: `${failedName} provisioning failed`,
});
break;
}

View File

@ -225,6 +225,11 @@ interface CanvasState {
/** Whether the A2A topology overlay is visible. Persisted to localStorage. Default: true. */
showA2AEdges: boolean;
setShowA2AEdges: (show: boolean) => void;
/** Screen-reader announcement text. Set by handleCanvasEvent on significant
* status changes; consumed and cleared by the aria-live region in Canvas.tsx
* so the same announcement doesn't re-fire on re-render. */
liveAnnouncement: string;
setLiveAnnouncement: (msg: string) => void;
}
export const useCanvasStore = create<CanvasState>((set, get) => ({
@ -321,6 +326,8 @@ export const useCanvasStore = create<CanvasState>((set, get) => ({
localStorage.setItem("molecule:show-a2a-edges", String(show));
}
},
liveAnnouncement: "",
setLiveAnnouncement: (msg) => set({ liveAnnouncement: msg }),
viewport: { x: 0, y: 0, zoom: 1 },

View File

@ -41,11 +41,10 @@ canvas/src/
## Known Issues
### 🔴 HIGH: secrets-store.ts Performance
### ✅ MEDIUM: secrets-store.ts Performance (mitigated)
**File:** `canvas/src/stores/secrets-store.ts`
**Issue:** `getGrouped()` selector creates new objects every call (Object.fromEntries + arrays). Not memoized.
**Impact:** Causes unnecessary re-renders on frequent selector access.
**Fix needed:** Memoize the selector or use a proper Zustand selector pattern.
**Issue:** `getGrouped()` selector creates new objects every call. Not memoized.
**Impact:** Mitigated — `SecretsTab.tsx` wraps the call in `useMemo`, so no active re-render issues in the single consumer. The store-level fix (memoizing `getGrouped` itself) is optional but low priority now.
### 🟡 MEDIUM: Pre-commit Hook Verification
**Issue:** Pre-commit hook checks 'use client' on hook-using components but unclear if it actually fails on violations.
@ -108,7 +107,7 @@ canvas/src/
| Priority | Item | Files | Status |
|----------|------|-------|--------|
| HIGH | Screen reader announcements for canvas state changes | Canvas.tsx | Not started |
| ~~HIGH~~ | ~~Screen reader announcements for canvas state changes~~ | ~~Canvas.tsx, canvas-events.ts, canvas.ts~~ | ✅ Done — PR #172 |
| MEDIUM | Keyboard shortcut help dialog | useKeyboardShortcuts.ts | Not started |
| MEDIUM | Keyboard-accessible node drag | WorkspaceNode.tsx, useDragHandlers.ts | Not started |
| LOW | Edge anchor keyboard accessibility | A2AEdge.tsx | Not started |