diff --git a/canvas/src/components/tabs/chat/AgentCommsPanel.tsx b/canvas/src/components/tabs/chat/AgentCommsPanel.tsx index 9aae504de..1ec6059c9 100644 --- a/canvas/src/components/tabs/chat/AgentCommsPanel.tsx +++ b/canvas/src/components/tabs/chat/AgentCommsPanel.tsx @@ -649,7 +649,17 @@ function WaitingBubbles({ visible }: { visible: CommMessage[] }) { if (!prev || m.timestamp > prev.timestamp) tailByPeer.set(m.peerId, m); } const waitingPeers = Array.from(tailByPeer.values()).filter( - (m) => m.flow === "out" && (m.status === "pending" || m.status === "queued"), + // Task #227 — also light the indicator for status="dispatched": that's + // the platform's marker for a poll-mode delegation that's been + // recorded into the peer's inbox but not yet picked up. Without this + // arm, external/MCP peer threads showed an outbound bubble and then + // dead silence until the eventual reply landed — no parity with the + // native push-path "pending" indicator. + (m) => + m.flow === "out" && + (m.status === "pending" || + m.status === "queued" || + m.status === "dispatched"), ); if (waitingPeers.length === 0) return null; return ( @@ -688,7 +698,9 @@ function WaitingBubbles({ visible }: { visible: CommMessage[] }) { {m.status === "queued" ? `${m.peerName} is busy — reply will arrive when they're free` - : `Waiting for ${m.peerName}…`} + : m.status === "dispatched" + ? `Queued — ${m.peerName} will pick up on next poll` + : `Waiting for ${m.peerName}…`} diff --git a/canvas/src/components/tabs/chat/hooks/__tests__/useChatSend.pollMode.test.tsx b/canvas/src/components/tabs/chat/hooks/__tests__/useChatSend.pollMode.test.tsx new file mode 100644 index 000000000..089b015c9 --- /dev/null +++ b/canvas/src/components/tabs/chat/hooks/__tests__/useChatSend.pollMode.test.tsx @@ -0,0 +1,178 @@ +// @vitest-environment jsdom +// +// Task #227 — external/MCP workspace progress UX parity. +// +// ws-server's `proxyA2ARequest` poll-mode short-circuit +// (workspace-server/internal/handlers/a2a_proxy.go:402-432) returns a +// synthetic `{status:"queued", delivery_mode:"poll", method:"message/send"}` +// HTTP 200 within ~50ms when the target workspace is registered with +// `delivery_mode=poll` — i.e. an operator's laptop running +// `molecule-mcp-claude-channel`, a hermes/codex MCP bridge, or a Cursor +// MCP client. The real agent reply arrives separately via the +// AGENT_MESSAGE WebSocket event after the agent's next +// `wait_for_message` poll (could be 1s, could be 60s). +// +// Pre-#227 behaviour: useChatSend treated the queued-200 as a successful +// round-trip — extractReplyText returned "", no agent bubble was +// created, `releaseSendGuards` flipped `sending` off, and the user saw +// dead silence between their user bubble and the eventual reply with +// NO progress indicator. That's the user-reported gap this task fixes. +// +// These tests pin the new behaviour: on a queued-200, the hook MUST NOT +// call onAgentMessage (no empty bubble) AND MUST NOT call +// releaseSendGuards (spinner persists). The eventual AGENT_MESSAGE WS +// event is what clears the spinner — that path is covered by +// useChatSocket.test.tsx already. + +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; + +// Capture the api.post invocations + control responses per-test. +const apiPostMock = vi.fn< + (url: string, body?: unknown, opts?: unknown) => Promise +>(); +vi.mock("@/lib/api", () => ({ + api: { + post: (url: string, body?: unknown, opts?: unknown) => + apiPostMock(url, body, opts), + get: vi.fn(), + }, +})); + +// uploads — tests don't go through the upload path; stub the helpers +// useChatSend imports so the module loads. +vi.mock("../../uploads", () => ({ + uploadChatFiles: vi.fn(), + FileTooLargeError: class FileTooLargeError extends Error {}, +})); + +// types — re-export the createMessage helper unchanged; only the +// uploads stub matters above. +import { useChatSend } from "../useChatSend"; + +beforeEach(() => { + apiPostMock.mockReset(); +}); + +describe("useChatSend — poll-mode (external/MCP) queued-200 handling — task #227", () => { + it("does NOT call onAgentMessage when the synthetic {status:'queued'} response lands (no empty bubble)", async () => { + // Mock the platform's poll-mode short-circuit response shape exactly + // as ws-server's `proxyA2ARequest` returns it (a2a_proxy.go:420-431). + apiPostMock.mockResolvedValueOnce({ + status: "queued", + delivery_mode: "poll", + method: "message/send", + }); + + const onUserMessage = vi.fn(); + const onAgentMessage = vi.fn(); + + const { result } = renderHook(() => + useChatSend("ws-poll-target", { + getHistoryMessages: () => [], + onUserMessage, + onAgentMessage, + }), + ); + + await act(async () => { + await result.current.sendMessage("hello external workspace"); + // Yield one microtask so the .then runs. + await Promise.resolve(); + }); + + // User bubble fires — the user typed, that part is unconditional. + expect(onUserMessage).toHaveBeenCalledTimes(1); + // CRITICAL: no agent bubble. extractReplyText on a queued envelope + // returns "" — the pre-#227 code would still have hit the + // "releaseSendGuards + no bubble" path, BUT it would have ended + // `sending`. The new code returns early BEFORE that release, so the + // contract under test is "no synthesised empty bubble". + expect(onAgentMessage).not.toHaveBeenCalled(); + }); + + it("keeps `sending` true after a queued-200 — the spinner must persist until the real AGENT_MESSAGE arrives", async () => { + apiPostMock.mockResolvedValueOnce({ + status: "queued", + delivery_mode: "poll", + method: "message/send", + }); + + const { result } = renderHook(() => + useChatSend("ws-poll-target", { + getHistoryMessages: () => [], + }), + ); + + await act(async () => { + await result.current.sendMessage("waiting for the operator laptop"); + await Promise.resolve(); + }); + + // The spinner-driving state is `sending`. On a queued-200, it must + // remain true — clearing it here is the exact bug task #227 + // resurfaces (collapsing the spinner before the agent has even seen + // the message). + expect(result.current.sending).toBe(true); + }); + + it("ALSO keeps `sending` true even after a follow-up microtask flush — guards against an accidental late release", async () => { + // Defense: ensure no chained .then / .finally accidentally calls + // releaseSendGuards on the queued path. Run several microtask + // ticks and re-assert. + apiPostMock.mockResolvedValueOnce({ + status: "queued", + delivery_mode: "poll", + }); + + const { result } = renderHook(() => + useChatSend("ws-poll-target", { + getHistoryMessages: () => [], + }), + ); + + await act(async () => { + await result.current.sendMessage("late-release-guard"); + // Flush multiple microtask ticks. + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(result.current.sending).toBe(true); + }); + + it("push-mode (real reply parts) still flips sending=false + creates an agent bubble — non-regression for the default path", async () => { + // Sanity-check the push path still works: a real reply must call + // onAgentMessage and flip sending=false. Without this assertion an + // overzealous "return early on any non-result body" would silently + // break the dominant push-mode path. + apiPostMock.mockResolvedValueOnce({ + result: { + parts: [{ kind: "text", text: "hi from native workspace" }], + }, + }); + + const onAgentMessage = vi.fn(); + const { result } = renderHook(() => + useChatSend("ws-native-push", { + getHistoryMessages: () => [], + onAgentMessage, + }), + ); + + await act(async () => { + await result.current.sendMessage("native push test"); + await Promise.resolve(); + }); + + expect(onAgentMessage).toHaveBeenCalledTimes(1); + const msg = onAgentMessage.mock.calls[0][0] as { + role: string; + content: string; + }; + expect(msg.role).toBe("agent"); + expect(msg.content).toBe("hi from native workspace"); + expect(result.current.sending).toBe(false); + }); +}); diff --git a/canvas/src/components/tabs/chat/hooks/__tests__/useChatSocket.test.tsx b/canvas/src/components/tabs/chat/hooks/__tests__/useChatSocket.test.tsx index b0319c281..428ea944f 100644 --- a/canvas/src/components/tabs/chat/hooks/__tests__/useChatSocket.test.tsx +++ b/canvas/src/components/tabs/chat/hooks/__tests__/useChatSocket.test.tsx @@ -116,6 +116,77 @@ describe("useChatSocket — surface error_detail to onSendError (internal#212)", expect(reason.length).toBeGreaterThan(0); }); + // Task #227 — external/MCP (poll-mode) workspace progress UX. + // + // ws-server's `proxyA2ARequest` poll-mode short-circuit fires the + // ACTIVITY_LOGGED a2a_receive with status="ok" and NO duration_ms (no + // reply yet — the request is queued for the agent's next poll). Before + // task #227 the (status==="ok" && durationMs) guard silently dropped + // this row, so the chat UI had ZERO progress signal between "user + // typed" and "agent eventually polled and replied". Lock the queued + // line in so future refactors don't regress to the silent-drop state. + it("emits a 'queued — will pick up on next poll' activity line when a2a_receive status=ok has no duration_ms (poll-mode)", () => { + const onActivityLog = vi.fn(); + renderHook(() => + useChatSocket("ws-self", { + onActivityLog, + }), + ); + + expect(capturedHandler).not.toBeNull(); + act(() => { + capturedHandler!({ + event: "ACTIVITY_LOGGED", + workspace_id: "ws-self", + payload: { + activity_type: "a2a_receive", + method: "message/send", + status: "ok", + target_id: "ws-self", + // No duration_ms — this is the queued-for-poll signal. + }, + timestamp: "2026-05-20T00:00:00Z", + }); + }); + + expect(onActivityLog).toHaveBeenCalledTimes(1); + const line = onActivityLog.mock.calls[0][0] as string; + // The line MUST be present (not the empty-string silent-drop pattern) + // and MUST mention the queued state so the user has actionable signal. + expect(line.length).toBeGreaterThan(0); + expect(line.toLowerCase()).toMatch(/queued|poll/); + }); + + // Pair with the above: poll-mode acknowledgement must NOT prematurely + // call onSendComplete — the spinner has to stay up until the actual + // AGENT_MESSAGE reply lands. (The reply-success path with duration_ms + // still calls onSendComplete; that's the push-mode case.) + it("does NOT call onSendComplete on a poll-mode queued a2a_receive (spinner must persist)", () => { + const onSendComplete = vi.fn(); + renderHook(() => + useChatSocket("ws-self", { + onSendComplete, + }), + ); + + act(() => { + capturedHandler!({ + event: "ACTIVITY_LOGGED", + workspace_id: "ws-self", + payload: { + activity_type: "a2a_receive", + method: "message/send", + status: "ok", + target_id: "ws-self", + // No duration_ms. + }, + timestamp: "2026-05-20T00:00:00Z", + }); + }); + + expect(onSendComplete).not.toHaveBeenCalled(); + }); + it("ignores errors targeted at a different workspace's peer", () => { // Defense against a race where the WS hub fans out to all clients — // each chat panel must only react when target_id matches its own diff --git a/canvas/src/components/tabs/chat/hooks/useChatSend.ts b/canvas/src/components/tabs/chat/hooks/useChatSend.ts index 59ff0c9b9..beb9724c0 100644 --- a/canvas/src/components/tabs/chat/hooks/useChatSend.ts +++ b/canvas/src/components/tabs/chat/hooks/useChatSend.ts @@ -22,6 +22,28 @@ interface A2AResponse { parts?: A2APart[]; artifacts?: Array<{ parts: A2APart[] }>; }; + /** Set by ws-server's poll-mode short-circuit in `proxyA2ARequest` + * (a2a_proxy.go:416-431) when the target workspace is registered as + * `delivery_mode=poll` — e.g. an operator's laptop running + * `molecule-mcp-claude-channel`, a hermes/codex MCP bridge, or a + * Cursor MCP client. The HTTP 200 carries the synthetic envelope + * `{status:"queued", delivery_mode:"poll", method:"message/send"}` + * immediately (~50ms), BEFORE the agent has produced a reply. + * + * Task #227 routing: when this field is "queued" the caller must NOT + * treat the 200 as "agent done" — there are no `result.parts` yet + * (the reply will arrive separately via the AGENT_MESSAGE WS event + * after the agent's next poll). Keep the spinner up; the eventual + * AGENT_MESSAGE flips `sending` off via the existing useChatSocket + * `onSendComplete` path. Without this distinction the spinner + * disappeared immediately and external/MCP workspaces had no progress + * UX between send and reply. */ + status?: string; + /** Companion to `status` — "poll" when the queued short-circuit fired. + * Defensive: we key the poll-mode-skip decision on status==="queued" + * (the canonical signal) rather than on this field, but it's surfaced + * here so future debugging / tests can assert on the full envelope. */ + delivery_mode?: string; } export function extractReplyText(resp: A2AResponse): string { @@ -195,6 +217,30 @@ export function useChatSend(workspaceId: string, options: UseChatSendOptions) { sendInFlightRef.current = false; return; } + // Task #227 — poll-mode (external/MCP workspace) queued-200 + // short-circuit. ws-server's `proxyA2ARequest` returns + // `{status:"queued", delivery_mode:"poll", ...}` immediately + // when the target has no URL (delivery_mode=poll), BEFORE the + // agent has produced any reply. There is no `result.parts` + // payload here — the actual reply will arrive separately via + // the AGENT_MESSAGE WebSocket event after the agent's next + // `wait_for_message` poll. + // + // Keep the spinner up by deliberately NOT calling + // releaseSendGuards: the user-facing "thinking" state must + // persist until the AGENT_MESSAGE lands (handled by the + // useChatSocket `onAgentMessage`/`onSendComplete` path) or an + // explicit error fires (`onSendError` from an ACTIVITY_LOGGED + // status="error"). Don't synthesise an empty agent bubble. + // + // sendInFlightRef stays true intentionally — it's the dedup + // guard for the user typing two messages back-to-back; for + // poll mode the second message would race the first agent's + // reply, so blocking is correct (matches push-mode behaviour + // where `sending` blocks the textarea). + if (resp?.status === "queued") { + return; + } const replyText = extractReplyText(resp); const replyFiles = extractFilesFromTask( (resp?.result ?? {}) as Record, diff --git a/canvas/src/components/tabs/chat/hooks/useChatSocket.ts b/canvas/src/components/tabs/chat/hooks/useChatSocket.ts index 4494efda9..46fc85619 100644 --- a/canvas/src/components/tabs/chat/hooks/useChatSocket.ts +++ b/canvas/src/components/tabs/chat/hooks/useChatSocket.ts @@ -62,6 +62,25 @@ export function useChatSocket( line = `← ${targetName} responded (${sec}s)`; const own = (targetId || msg.workspace_id) === workspaceId; if (own) callbacksRef.current.onSendComplete?.(); + } else if (status === "ok" && !durationMs) { + // Task #227 — poll-mode (external/MCP workspace) queued receipt. + // ws-server `logA2AReceiveQueued` writes a "received but no + // reply yet" row with status="ok" and NO duration_ms, then + // immediately returns the synthetic {status:"queued"} 200 to + // the caller. Before this branch the row was silently dropped + // by the (status==="ok" && durationMs) guard above — leaving + // the chat UI with zero progress signal for the entire window + // between "user typed" and "agent eventually polled and + // replied". Surface the queued state explicitly so the user + // sees acknowledgement (matches the queued-delegation + // indicator in AgentCommsPanel.WaitingBubbles). + // + // We intentionally do NOT call onSendComplete here: the + // outbound is not done — only acknowledged. The MyChatPanel + // spinner stays up until the actual AGENT_MESSAGE reply lands + // (poll path) or an explicit error fires (which still hits + // the status==="error" branch below). + line = `⧗ ${targetName} queued — agent will pick up on next poll`; } else if (status === "error") { line = `⚠ ${targetName} error`; const own = (targetId || msg.workspace_id) === workspaceId; diff --git a/canvas/src/store/canvas-topology.ts b/canvas/src/store/canvas-topology.ts index 1bed943bf..ec67282f2 100644 --- a/canvas/src/store/canvas-topology.ts +++ b/canvas/src/store/canvas-topology.ts @@ -523,6 +523,9 @@ export function buildNodesAndEdges( // that don't yet include these columns in the GET response. broadcastEnabled: ws.broadcast_enabled ?? false, talkToUserEnabled: ws.talk_to_user_enabled ?? true, + // A2A delivery mode (task #227). Absent on older ws-server builds + // — leave undefined so the chat UI's "?? 'push'" fallback applies. + deliveryMode: ws.delivery_mode, }, }; if (hasParent) { diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index 1baa0e660..fd8117982 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -106,6 +106,28 @@ export interface WorkspaceNodeData extends Record { * send_message_to_user / POST /notify return 403 and the canvas * shows a "not enabled" state with a button to re-enable. Default true. */ talkToUserEnabled?: boolean; + /** A2A inbound delivery mode for this workspace — "push" (default — + * synchronous HTTP dispatch by ws-server `proxyA2ARequest`) or "poll" + * (workspace has no URL; ws-server logs the request and the agent + * consumes it via `wait_for_message` / GET /activity?since_id=). + * + * Why surfaced to the UI: poll-mode targets (external/MCP workspaces: + * `molecule-mcp-claude-channel` on an operator laptop, hermes/codex + * bridge clients, Cursor MCP) acknowledge a canvas `message/send` with + * a synthetic `{status:"queued"}` 200 within ~50ms. Without this flag + * the chat UI cannot tell that gap from a real round-trip — the + * spinner disappears immediately and the user sees dead silence until + * the agent eventually polls and replies via the AGENT_MESSAGE WS + * event (could be seconds, could be minutes). Task #227 — render a + * "queued — agent will pick up on next poll" state for poll-mode + * sends so external/MCP workspaces have progress UX parity with + * native runtimes (claude-code / codex / hermes / openclaw). + * + * Sourced from the GET /workspaces response (`delivery_mode` snake_case + * field, mapped here in canvas-topology.ts). Absent on older platform + * builds — that fallthrough is treated as "push" to match + * ws-server's `lookupDeliveryMode` default. */ + deliveryMode?: string; } export type PanelTab = "details" | "skills" | "chat" | "terminal" | "config" | "schedule" | "channels" | "files" | "memory" | "traces" | "events" | "activity" | "audit"; diff --git a/canvas/src/store/socket.ts b/canvas/src/store/socket.ts index d82f06762..a4d1f97d9 100644 --- a/canvas/src/store/socket.ts +++ b/canvas/src/store/socket.ts @@ -342,6 +342,16 @@ export interface WorkspaceData { /** Workspace ability flags (migration 20260514). */ broadcast_enabled?: boolean; talk_to_user_enabled?: boolean; + /** A2A delivery mode for inbound messages — "push" (default, synchronous + * HTTP dispatch to `url`) or "poll" (queued to activity_logs, agent + * picks up via `wait_for_message` / GET /activity?since_id=). Surfaced + * in the GET /workspaces response since #2339 PR 1; older platform + * versions return it absent so the canvas treats absent as "push" (the + * documented default in `lookupDeliveryMode`). Used by the chat UI to + * render an "agent will pick up on next poll" indicator instead of + * collapsing the spinner the moment the synchronous queued-200 returns + * (task #227 — external/MCP workspaces had no progress UX). */ + delivery_mode?: string; } let socket: ReconnectingSocket | null = null;