diff --git a/canvas/src/components/tabs/ChatTab.tsx b/canvas/src/components/tabs/ChatTab.tsx index c4b326e7..bc551bef 100644 --- a/canvas/src/components/tabs/ChatTab.tsx +++ b/canvas/src/components/tabs/ChatTab.tsx @@ -10,9 +10,10 @@ import { closeWebSocketGracefully } from "@/lib/ws-close"; import { type ChatMessage, type ChatAttachment, createMessage, appendMessageDeduped } from "./chat/types"; import { uploadChatFiles, downloadChatFile } from "./chat/uploads"; import { AttachmentChip, PendingAttachmentPill } from "./chat/AttachmentViews"; -import { extractResponseText, extractRequestText, extractFilesFromTask } from "./chat/message-parser"; +import { extractResponseText, extractFilesFromTask } from "./chat/message-parser"; import { AgentCommsPanel } from "./chat/AgentCommsPanel"; import { appendActivityLine } from "./chat/activityLog"; +import { activityRowToMessages, type ActivityRowForHydration } from "./chat/historyHydration"; import { runtimeDisplayName } from "@/lib/runtime-names"; import { ConfirmDialog } from "@/components/ConfirmDialog"; @@ -125,42 +126,17 @@ function extractReplyText(resp: A2AResponse): string { */ async function loadMessagesFromDB(workspaceId: string): Promise<{ messages: ChatMessage[]; error: string | null }> { try { - const activities = await api.get | null; - response_body: Record | null; - }>>(`/workspaces/${workspaceId}/activity?type=a2a_receive&source=canvas&limit=50`); + const activities = await api.get( + `/workspaces/${workspaceId}/activity?type=a2a_receive&source=canvas&limit=50`, + ); const messages: ChatMessage[] = []; - // Activities are newest-first, reverse for chronological order + // Activities are newest-first, reverse for chronological order. + // Per-row mapping lives in chat/historyHydration.ts so it can be + // unit-tested without spinning up the full ChatTab component + // (regression cover for the timestamp-collapse bug). for (const a of [...activities].reverse()) { - // Extract user message from request_body. Pin the timestamp to - // the activity row's created_at — without this override every - // historical user bubble re-stamps to "now" on each chat reload, - // and ALL user messages collapse to the same render-time clock - // (same as the agent path on line 157). - const userText = extractRequestText(a.request_body); - if (userText && !isInternalSelfMessage(userText)) { - messages.push({ ...createMessage("user", userText), timestamp: a.created_at }); - } - - // Extract agent response — text AND any file attachments so a - // chat reload surfaces historical download chips, not just plain - // text. `result` is nested on successful A2A responses; some - // older rows stored the raw `result` payload at the top level, - // so fall back to the body itself when `.result` is absent. - if (a.response_body) { - const text = extractResponseText(a.response_body); - const attachments = extractFilesFromTask( - (a.response_body.result ?? a.response_body) as Record, - ); - if (text || attachments.length > 0) { - const role = a.status === "error" || text.toLowerCase().startsWith("agent error") ? "system" : "agent"; - messages.push({ ...createMessage(role, text, attachments), timestamp: a.created_at }); - } - } + messages.push(...activityRowToMessages(a, isInternalSelfMessage)); } return { messages, error: null }; } catch (err) { diff --git a/canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts b/canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts new file mode 100644 index 00000000..fa164621 --- /dev/null +++ b/canvas/src/components/tabs/chat/__tests__/historyHydration.test.ts @@ -0,0 +1,192 @@ +// @vitest-environment jsdom +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { activityRowToMessages, type ActivityRowForHydration } from "../historyHydration"; + +const NEVER_INTERNAL = (_text: string) => false; + +function makeRow(overrides: Partial = {}): ActivityRowForHydration { + return { + activity_type: "a2a_receive", + status: "ok", + created_at: "2026-04-25T18:00:00.000Z", + request_body: null, + response_body: null, + ...overrides, + }; +} + +describe("activityRowToMessages", () => { + // The bug that prompted extracting this helper: every call to + // createMessage() inside the loader stamped `timestamp: new Date()`, + // so every reload re-stamped every historical user bubble to the + // render moment. Two messages sent hours apart both rendered with + // the same "now" clock. These tests pin the override so a future + // refactor that drops the spread-and-override fails loudly. + + describe("timestamp preservation (regression cover)", () => { + beforeEach(() => { + // Freeze the wall clock to a value that's CLEARLY different from + // any row.created_at the tests use, so any test that doesn't + // override the timestamp will mint "2030-…" and the assertion + // against "2026-…" will fail unmistakably. + vi.useFakeTimers(); + vi.setSystemTime(new Date("2030-01-01T00:00:00.000Z")); + }); + afterEach(() => vi.useRealTimers()); + + it("user message timestamp pins to row.created_at, NOT new Date()", () => { + const row = makeRow({ + created_at: "2026-04-25T18:00:00.000Z", + request_body: { params: { message: { parts: [{ kind: "text", text: "hello from earlier today" }] } } }, + }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + const user = msgs.find((m) => m.role === "user")!; + expect(user.timestamp).toBe("2026-04-25T18:00:00.000Z"); + // Negative assertion: the wall clock is set to 2030. If the + // override regresses, timestamp will start with "2030-". + expect(user.timestamp.startsWith("2030")).toBe(false); + }); + + it("agent message timestamp pins to row.created_at, NOT new Date()", () => { + const row = makeRow({ + created_at: "2026-04-25T18:05:00.000Z", + response_body: { result: "agent reply" }, + }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + const agent = msgs.find((m) => m.role === "agent")!; + expect(agent.timestamp).toBe("2026-04-25T18:05:00.000Z"); + expect(agent.timestamp.startsWith("2030")).toBe(false); + }); + + it("two rows with distinct created_at produce two distinct timestamps (the user-visible bug)", () => { + // The actual screenshot symptom: two user messages sent hours + // apart both rendered with the same render-moment timestamp. + // If activityRowToMessages defers to new Date(), both messages + // here will share the frozen 2030 wall clock instead of their + // distinct created_at values. + const a = activityRowToMessages( + makeRow({ + created_at: "2026-04-25T14:00:00.000Z", + request_body: { params: { message: { parts: [{ kind: "text", text: "first" }] } } }, + }), + NEVER_INTERNAL, + ); + const b = activityRowToMessages( + makeRow({ + created_at: "2026-04-25T21:01:58.000Z", + request_body: { params: { message: { parts: [{ kind: "text", text: "second" }] } } }, + }), + NEVER_INTERNAL, + ); + expect(a[0].timestamp).toBe("2026-04-25T14:00:00.000Z"); + expect(b[0].timestamp).toBe("2026-04-25T21:01:58.000Z"); + expect(a[0].timestamp).not.toBe(b[0].timestamp); + }); + }); + + describe("user-message extraction", () => { + it("emits a user message when request_body has text", () => { + const row = makeRow({ + request_body: { params: { message: { parts: [{ kind: "text", text: "hi agent" }] } } }, + }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + expect(msgs).toHaveLength(1); + expect(msgs[0].role).toBe("user"); + expect(msgs[0].content).toBe("hi agent"); + }); + + it("drops user messages flagged as internal self-messages", () => { + // The heartbeat self-trigger ("Delegation results are ready...") + // gets recorded as a canvas-source row but isn't a real user + // message — the predicate filters it out so it doesn't render + // as the user talking to themselves. + const row = makeRow({ + request_body: { params: { message: { parts: [{ kind: "text", text: "Delegation results are ready..." }] } } }, + }); + const msgs = activityRowToMessages(row, (t) => t.startsWith("Delegation results are ready")); + expect(msgs.find((m) => m.role === "user")).toBeUndefined(); + }); + + it("emits no user message when request_body is null", () => { + const row = makeRow({ request_body: null }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + expect(msgs.find((m) => m.role === "user")).toBeUndefined(); + }); + }); + + describe("agent-message extraction", () => { + it("emits an agent message from response_body.result string", () => { + const row = makeRow({ response_body: { result: "agent says hi" } }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + expect(msgs).toHaveLength(1); + expect(msgs[0].role).toBe("agent"); + expect(msgs[0].content).toBe("agent says hi"); + }); + + it("emits role=system when status=error", () => { + // System role gets distinct rendering (red bubble) for failed + // calls; if this regresses errors will look like normal agent + // replies and the user won't realise something went wrong. + const row = makeRow({ + status: "error", + response_body: { result: "delegation failed" }, + }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + expect(msgs[0].role).toBe("system"); + }); + + it("attaches file parts hydrated from response_body (parts at root)", () => { + // The notify-with-attachments shape: response_body = + // {result: "", parts: [{kind:"file", ...}]}. If + // extractFilesFromTask doesn't see the attachments here, a chat + // reload after an agent attached a file would lose the chips. + const row = makeRow({ + response_body: { + result: "Done — see attached.", + parts: [ + { kind: "file", file: { name: "build.zip", uri: "workspace:/tmp/build.zip", size: 12345 } }, + ], + }, + }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + const agent = msgs.find((m) => m.role === "agent")!; + expect(agent.attachments).toEqual([ + { name: "build.zip", uri: "workspace:/tmp/build.zip", size: 12345 }, + ]); + }); + + it("emits no agent message when response_body is null", () => { + const row = makeRow({ response_body: null }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + expect(msgs.find((m) => m.role === "agent" || m.role === "system")).toBeUndefined(); + }); + + it("emits no agent message when response_body has neither text nor files", () => { + const row = makeRow({ response_body: { unrelated: "metadata" } }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + expect(msgs.find((m) => m.role === "agent")).toBeUndefined(); + }); + }); + + describe("end-to-end shape", () => { + it("a single row with both user request and agent reply emits two messages with the same timestamp", () => { + // Mirrors the canonical canvas-source row: user types something, + // agent replies, both stored on the same activity_logs row. UI + // renders them as the user bubble immediately followed by the + // agent bubble — keep that pairing intact. + const row = makeRow({ + created_at: "2026-04-25T18:00:00.000Z", + request_body: { params: { message: { parts: [{ kind: "text", text: "what's 2+2?" }] } } }, + response_body: { result: "4" }, + }); + const msgs = activityRowToMessages(row, NEVER_INTERNAL); + expect(msgs).toHaveLength(2); + expect(msgs[0].role).toBe("user"); + expect(msgs[0].content).toBe("what's 2+2?"); + expect(msgs[0].timestamp).toBe("2026-04-25T18:00:00.000Z"); + expect(msgs[1].role).toBe("agent"); + expect(msgs[1].content).toBe("4"); + expect(msgs[1].timestamp).toBe("2026-04-25T18:00:00.000Z"); + }); + }); +}); diff --git a/canvas/src/components/tabs/chat/historyHydration.ts b/canvas/src/components/tabs/chat/historyHydration.ts new file mode 100644 index 00000000..b6feab5b --- /dev/null +++ b/canvas/src/components/tabs/chat/historyHydration.ts @@ -0,0 +1,65 @@ +import { type ChatMessage, createMessage } from "./types"; +import { extractResponseText, extractRequestText, extractFilesFromTask } from "./message-parser"; + +/** Activity row shape the chat history loader consumes. Only the fields + * it actually reads are listed — the platform sends more (id, target_id, + * method, summary, etc.) but the hydration is defined by these four. */ +export interface ActivityRowForHydration { + activity_type: string; + status: string; + created_at: string; + request_body: Record | null; + response_body: Record | null; +} + +/** Map a single activity_logs row to the chat messages it represents. + * + * An a2a_receive row can produce up to two messages: + * 1. A user-side bubble derived from request_body (the message the + * user sent), unless the request was an internal self-message. + * 2. An agent-side bubble derived from response_body (text + + * file attachments), with role=system when status=error. + * + * CRITICAL: both messages MUST adopt `row.created_at` as their + * timestamp. createMessage() defaults to new Date() — appropriate for + * freshly-typed messages, wrong for hydrated history because every + * reload would re-stamp every bubble to the render moment. The + * regression that prompted extracting this helper showed up as every + * user message in the chat collapsing to the same "now" clock after + * reload (see test_user_messages_pin_timestamps_to_created_at). + */ +export function activityRowToMessages( + row: ActivityRowForHydration, + isInternalSelfMessage: (text: string) => boolean, +): ChatMessage[] { + const out: ChatMessage[] = []; + + const userText = extractRequestText(row.request_body); + if (userText && !isInternalSelfMessage(userText)) { + out.push({ ...createMessage("user", userText), timestamp: row.created_at }); + } + + if (row.response_body) { + const text = extractResponseText(row.response_body); + // Pick the right object to feed extractFilesFromTask: + // - Task-shape: {result: {parts: [...]}} → unwrap result + // - Notify-shape: {result: "", parts: [...]} → use the body + // Naively doing `result ?? body` would pass the string "" to + // the file extractor for the notify case, returning [] and dropping + // the file chips on reload. Only unwrap when result is an object. + const filesSource: Record = + row.response_body.result && typeof row.response_body.result === "object" + ? (row.response_body.result as Record) + : row.response_body; + const attachments = extractFilesFromTask(filesSource); + if (text || attachments.length > 0) { + const role: ChatMessage["role"] = + row.status === "error" || text.toLowerCase().startsWith("agent error") + ? "system" + : "agent"; + out.push({ ...createMessage(role, text, attachments), timestamp: row.created_at }); + } + } + + return out; +}