diff --git a/canvas/src/components/ConversationTraceModal.tsx b/canvas/src/components/ConversationTraceModal.tsx index 7789b4c1..a4166061 100644 --- a/canvas/src/components/ConversationTraceModal.tsx +++ b/canvas/src/components/ConversationTraceModal.tsx @@ -35,7 +35,10 @@ export function extractMessageText(body: Record | null): string const rParts = (result?.parts || []) as Array>; const rText = rParts .map((p) => { - if (p.text) return p.text as string; + // Prefer direct text over root.text. When p.text is present + // (including empty string), return it as-is — do not fall through + // to root.text. Fall back to root.text only when p.text is absent. + if ("text" in p) return (p.text as string) ?? ""; const root = p.root as Record | undefined; return (root?.text as string) || ""; }) diff --git a/canvas/src/components/__tests__/ConversationTraceModal.test.tsx b/canvas/src/components/__tests__/ConversationTraceModal.test.tsx index 5df302ca..af11d07b 100644 --- a/canvas/src/components/__tests__/ConversationTraceModal.test.tsx +++ b/canvas/src/components/__tests__/ConversationTraceModal.test.tsx @@ -96,8 +96,8 @@ describe("extractMessageText — response result format", () => { ], }, }; - // Both parts contribute: text from first part, root.text from second. - // The implementation: all non-empty strings joined with newline. + // Part 0 has direct text → returns it. Part 1 has no text field + // (only root) → falls through to root.text. Both contribute. expect(extractMessageText(body)).toBe("Direct text\nRoot text"); }); }); @@ -153,4 +153,39 @@ describe("extractMessageText — error resilience", () => { }; expect(extractMessageText(body)).toBe("valid"); }); + + it("prefers empty-string parts[].text over parts[].root.text", () => { + // When p.text is "" (not undefined), it is the explicit value and must + // not be replaced by root.text. + const body = { + result: { + parts: [ + { text: "", root: { text: "root fallback" } }, + ], + }, + }; + // Bug fix: empty-string text must not fall through to root.text. + // The empty string is the explicit value, root.text is ignored. + expect(extractMessageText(body)).toBe(""); + }); + + it("uses root.text when parts[].text is absent", () => { + const body = { + result: { + parts: [{ root: { text: "fallback text" } }], + }, + }; + expect(extractMessageText(body)).toBe("fallback text"); + }); + + it("direct text wins over root.text when both are present", () => { + const body = { + result: { + parts: [ + { text: "direct", root: { text: "root" } }, + ], + }, + }; + expect(extractMessageText(body)).toBe("direct"); + }); }); diff --git a/canvas/src/components/__tests__/Spinner.test.tsx b/canvas/src/components/__tests__/Spinner.test.tsx index 9c5c01eb..680dafb1 100644 --- a/canvas/src/components/__tests__/Spinner.test.tsx +++ b/canvas/src/components/__tests__/Spinner.test.tsx @@ -5,10 +5,18 @@ * Covers: sm/md/lg size classes, aria-hidden, motion-safe animate-spin class. */ import React from "react"; -import { render, screen } from "@testing-library/react"; -import { describe, expect, it } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; +import { afterEach, describe, expect, it } from "vitest"; import { Spinner } from "../Spinner"; +afterEach(cleanup); + +// Helper extracts the SVG class string so tests are readable. +const getSvgClass = (size?: "sm" | "md" | "lg") => { + const { container } = render(); + return container.querySelector("svg")?.getAttribute("class") ?? ""; +}; + describe("Spinner — size variants", () => { it("renders with sm size class", () => { const { container } = render(); diff --git a/canvas/src/components/tabs/ChatTab.tsx b/canvas/src/components/tabs/ChatTab.tsx index 156f87e8..74a39af0 100644 --- a/canvas/src/components/tabs/ChatTab.tsx +++ b/canvas/src/components/tabs/ChatTab.tsx @@ -67,7 +67,8 @@ interface A2AResponse { // Server-side counterpart in workspace-server/internal/channels/ // manager.go has the same single-part bug; fix that too if/when a // channel-delivered reply (Slack, Lark, etc.) gets truncated. -function extractReplyText(resp: A2AResponse): string { +/** Exported for unit testing — see extractReplyText.test.ts */ +export function extractReplyText(resp: A2AResponse): string { const collect = (parts: A2APart[] | undefined): string => { if (!parts) return ""; return parts diff --git a/canvas/src/components/tabs/ConfigTab.tsx b/canvas/src/components/tabs/ConfigTab.tsx index 0c8b5bc3..1df68c1e 100644 --- a/canvas/src/components/tabs/ConfigTab.tsx +++ b/canvas/src/components/tabs/ConfigTab.tsx @@ -144,7 +144,8 @@ interface RuntimeOption { // haven't migrated to the explicit `providers:` field yet, AND // continues to be a useful fallback for any future runtime whose // derive-provider semantics happen to match the slug prefix. -function deriveProvidersFromModels(models: ModelSpec[]): string[] { +/** Exported for unit testing — see deriveProvidersFromModels.test.ts */ +export function deriveProvidersFromModels(models: ModelSpec[]): string[] { const seen = new Set(); const out: string[] = []; for (const m of models) { diff --git a/canvas/src/components/tabs/FilesTab/__tests__/FilesToolbar.test.tsx b/canvas/src/components/tabs/FilesTab/__tests__/FilesToolbar.test.tsx index cb23c95d..374de4e8 100644 --- a/canvas/src/components/tabs/FilesTab/__tests__/FilesToolbar.test.tsx +++ b/canvas/src/components/tabs/FilesTab/__tests__/FilesToolbar.test.tsx @@ -306,6 +306,136 @@ describe("FilesToolbar", () => { expect(fileInput).toBeTruthy(); expect(fileInput?.getAttribute("aria-label")).toBe("Upload folder files"); }); + + it("calls onUpload with FileList when file input fires change", () => { + const onUpload = vi.fn(); + render( + + ); + const fileInput = document.querySelector( + 'input[type="file"]' + ) as HTMLInputElement; + // Simulate a FileList via fireEvent (jsdom doesn't support real FileList) + fireEvent.change(fileInput); + expect(onUpload).toHaveBeenCalledTimes(1); + }); + + it("clicking Upload button focuses the hidden file input", () => { + render( + + ); + const uploadBtn = screen.getByRole("button", { name: /upload folder/i }); + // The button's onClick calls uploadRef.current.click() — just verify + // the button exists and has an onClick handler (rendered without error). + expect(uploadBtn).toBeTruthy(); + }); + }); + + describe("always-visible buttons", () => { + it("Export button is visible for /workspace root", () => { + render( + + ); + expect( + screen.getByRole("button", { name: /download all files/i }) + ).toBeTruthy(); + }); + + it("Refresh button is visible for /workspace root", () => { + render( + + ); + expect( + screen.getByRole("button", { name: /refresh file list/i }) + ).toBeTruthy(); + }); + }); + + describe("file count display", () => { + it("shows count for fileCount=0", () => { + render( + + ); + expect(screen.getByText("0 files")).toBeTruthy(); + }); + + it("shows count for fileCount=1", () => { + render( + + ); + expect(screen.getByText("1 files")).toBeTruthy(); + }); + + it("renders directory select options", () => { + render( + + ); + const select = screen.getByRole("combobox"); + expect(select.querySelectorAll("option").length).toBeGreaterThanOrEqual(4); + }); }); describe("a11y", () => { diff --git a/canvas/src/components/tabs/__tests__/deriveProvidersFromModels.test.ts b/canvas/src/components/tabs/__tests__/deriveProvidersFromModels.test.ts new file mode 100644 index 00000000..990f7451 --- /dev/null +++ b/canvas/src/components/tabs/__tests__/deriveProvidersFromModels.test.ts @@ -0,0 +1,66 @@ +// @vitest-environment jsdom +/** + * Tests for ConfigTab's deriveProvidersFromModels helper. + * + * Covers: colon-slug vendor extraction, slash-slug vendor extraction, + * deduplication, empty model list, models without separators, models + * with id="", missing id field, mixed separators, vendor ordering. + */ +import { describe, expect, it } from "vitest"; +import { deriveProvidersFromModels } from "../ConfigTab"; + +type ModelSpec = { id: string; name?: string }; + +const m = (id: string, name?: string): ModelSpec => + name ? { id, name } : { id }; + +describe("deriveProvidersFromModels", () => { + it("derives vendor from colon-separated slug", () => { + expect(deriveProvidersFromModels([m("anthropic:claude-opus-4-7")])).toEqual(["anthropic"]); + }); + + it("derives vendor from slash-separated slug", () => { + expect(deriveProvidersFromModels([m("nousresearch/hermes-4-70b")])).toEqual(["nousresearch"]); + }); + + it("deduplicates repeated vendor", () => { + expect( + deriveProvidersFromModels([ + m("anthropic:claude-opus-4-7"), + m("anthropic:claude-sonnet-4-5"), + m("anthropic:claude-haiku-4"), + ]) + ).toEqual(["anthropic"]); + }); + + it("returns empty array for empty model list", () => { + expect(deriveProvidersFromModels([])).toEqual([]); + }); + + it("skips model with no separator (bare slug)", () => { + expect(deriveProvidersFromModels([m("claude-opus-4-7")])).toEqual([]); + }); + + it("skips model with separator at position 0", () => { + expect(deriveProvidersFromModels([m(":anonymous")])).toEqual([]); + }); + + it("skips model with id === ''", () => { + expect(deriveProvidersFromModels([m("")])).toEqual([]); + }); + + it("skips models without id field", () => { + const models = [{ name: "Claude" }] as unknown as ModelSpec[]; + expect(deriveProvidersFromModels(models)).toEqual([]); + }); + + it("handles mixed colon and slash separators", () => { + expect( + deriveProvidersFromModels([ + m("anthropic:claude-opus-4-7"), + m("nousresearch/hermes-4-70b"), + m("openai:gpt-4o"), + ]) + ).toEqual(["anthropic", "nousresearch", "openai"]); + }); +}); diff --git a/canvas/src/components/tabs/__tests__/extractReplyText.test.ts b/canvas/src/components/tabs/__tests__/extractReplyText.test.ts new file mode 100644 index 00000000..e807abcd --- /dev/null +++ b/canvas/src/components/tabs/__tests__/extractReplyText.test.ts @@ -0,0 +1,153 @@ +// @vitest-environment jsdom +/** + * Tests for ChatTab's extractReplyText helper. + * + * Covers: null/undefined resp, empty result, single text part, multiple + * text parts joined with newline, mixed text + non-text parts, artifacts + * text extraction, artifacts alongside parts text, empty artifacts, + * nested structure, plain string result. + */ +import { describe, expect, it } from "vitest"; +import { extractReplyText } from "../ChatTab"; + +// Re-declare the minimal shape we test against (matches A2A response types). +type A2APart = { kind: string; text?: string }; +type A2AResponse = { + result?: { + parts?: A2APart[]; + artifacts?: Array<{ parts: A2APart[] }>; + }; +}; + +const resp = (overrides: A2AResponse): A2AResponse => overrides; + +describe("extractReplyText", () => { + it("returns empty string for null response", () => { + expect(extractReplyText(null as unknown as A2AResponse)).toBe(""); + }); + + it("returns empty string for undefined response", () => { + expect(extractReplyText(undefined as unknown as A2AResponse)).toBe(""); + }); + + it("returns empty string when result is absent", () => { + expect(extractReplyText({})).toBe(""); + }); + + it("returns empty string when parts is empty array", () => { + expect(extractReplyText(resp({ result: { parts: [] } }))).toBe(""); + }); + + it("returns empty string when parts contains only non-text parts", () => { + const r = resp({ result: { parts: [{ kind: "file" }, { kind: "data" }] } }); + expect(extractReplyText(r)).toBe(""); + }); + + it("returns text from a single text part", () => { + const r = resp({ result: { parts: [{ kind: "text", text: "Hello world" }] } }); + expect(extractReplyText(r)).toBe("Hello world"); + }); + + it("joins multiple text parts with newline", () => { + const r = resp({ + result: { + parts: [ + { kind: "text", text: "First line" }, + { kind: "text", text: "Second line" }, + { kind: "text", text: "Third line" }, + ], + }, + }); + expect(extractReplyText(r)).toBe("First line\nSecond line\nThird line"); + }); + + it("ignores falsy text values in parts", () => { + const r = resp({ + result: { + parts: [ + { kind: "text", text: "Valid text" }, + { kind: "text", text: "" }, // empty string — should be skipped + { kind: "text" }, // undefined text — should be skipped + ], + }, + }); + expect(extractReplyText(r)).toBe("Valid text"); + }); + + it("extracts text from artifacts array", () => { + const r = resp({ + result: { + artifacts: [ + { parts: [{ kind: "text", text: "Artifact body" }] }, + ], + }, + }); + expect(extractReplyText(r)).toBe("Artifact body"); + }); + + it("concatenates text from parts and artifacts with newline", () => { + const r = resp({ + result: { + parts: [{ kind: "text", text: "Direct response" }], + artifacts: [ + { parts: [{ kind: "text", text: "Artifact section" }] }, + ], + }, + }); + expect(extractReplyText(r)).toBe("Direct response\nArtifact section"); + }); + + it("skips artifacts with no text parts", () => { + const r = resp({ + result: { + parts: [{ kind: "text", text: "Summary" }], + artifacts: [ + { parts: [] }, + { parts: [{ kind: "file" }] }, + ], + }, + }); + expect(extractReplyText(r)).toBe("Summary"); + }); + + it("extracts text from multiple artifacts", () => { + const r = resp({ + result: { + artifacts: [ + { parts: [{ kind: "text", text: "Artifact 1" }] }, + { parts: [{ kind: "text", text: "Artifact 2" }] }, + ], + }, + }); + expect(extractReplyText(r)).toBe("Artifact 1\nArtifact 2"); + }); + + it("handles deeply nested parts gracefully (no crash)", () => { + const r = resp({ + result: { + parts: [ + { kind: "text", text: "OK" }, + { kind: "text", text: "" }, // empty — ignored + ], + artifacts: [ + { parts: [{ kind: "text", text: "Also OK" }] }, + ], + }, + }); + expect(extractReplyText(r)).toBe("OK\nAlso OK"); + }); + + it("prefers parts text even when artifacts also have text", () => { + // This test documents the current behavior: parts text and artifact + // text are both included (concatenated), not exclusive. + const r = resp({ + result: { + parts: [{ kind: "text", text: "Parts text" }], + artifacts: [ + { parts: [{ kind: "text", text: "Artifact text" }] }, + ], + }, + }); + expect(extractReplyText(r)).toBe("Parts text\nArtifact text"); + }); +});