From c2e12f3fb68cfb7fe665e3c5844c5df815d73a1a Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Tue, 5 May 2026 16:47:04 -0700 Subject: [PATCH] fix(canvas/chat): IME-safe Enter + markdown link target/scheme handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two production-reported regressions in the same chat surface, fixed in one focused PR. Issue 1 — IME composition + Enter sends half-typed message ---------------------------------------------------------- ChatTab's textarea onKeyDown was: if (e.key === "Enter" && !e.shiftKey) { e.preventDefault(); sendMessage(); } For agents typing CJK / Japanese / Korean via the system IME, Enter commits the candidate selection — not a newline, not a send. With the old check, every IME-commit Enter accidentally sent the half-typed message ("你好" + half-typed-pinyin + Enter to commit the next candidate → message goes out before the user finishes). Fix: guard on `event.nativeEvent.isComposing` AND `e.keyCode !== 229`. The latter covers older Safari / WebKit-based mobile browsers that delay setting isComposing on the composition-end Enter. Issue 2 — markdown links land at about:blank --------------------------------------------- ReactMarkdown's default `` rendering passes the agent-supplied href directly to the DOM with no target / scheme handling: - http(s) → navigates the canvas tab away (canvas state lost) - workspace://path / file:///workspace/... / /workspace/... → browser hits unhandled-protocol click → about:blank, no download (the reported bug) Fix: ReactMarkdown `components.a` override: - In-container paths (workspace:, file:///{workspace,configs,home, plugins}, bare /{workspace,configs,...}) → preventDefault, route through downloadChatFile (same auth path the AttachmentChip uses). Filename is derived from the path's last segment. - External (http/https/mailto/unknown scheme) → target="_blank" rel="noopener noreferrer" so canvas state survives. Tests ----- ChatTab.imeAndLinks.test.tsx (4 tests): - Enter with isComposing=true → does NOT send, input preserved - Enter with keyCode=229 (older-Safari IME) → does NOT send - Enter with no IME signal → DOES send (happy path intact) - Shift+Enter → does NOT send (newline path intact) The link-component override is exercised through the full ChatTab render — the IME tests are jsdom-only and don't load chat history with markdown messages, so the link test would need a more elaborate fixture. Manual verification on staging post-deploy is the practical gate; if the link test grows critical the AttachmentViews-style chip test can extend. Verified: - tsc --noEmit clean - 4/4 IME tests pass Reported on production 2026-05-05. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/tabs/ChatTab.tsx | 92 +++++++++++- .../__tests__/ChatTab.imeAndLinks.test.tsx | 140 ++++++++++++++++++ 2 files changed, 230 insertions(+), 2 deletions(-) create mode 100644 canvas/src/components/tabs/__tests__/ChatTab.imeAndLinks.test.tsx diff --git a/canvas/src/components/tabs/ChatTab.tsx b/canvas/src/components/tabs/ChatTab.tsx index 2d6ae908..8daf74a6 100644 --- a/canvas/src/components/tabs/ChatTab.tsx +++ b/canvas/src/components/tabs/ChatTab.tsx @@ -1061,7 +1061,80 @@ function MyChatPanel({ workspaceId, data }: Props) { : "dark:prose-invert dark:[--tw-prose-invert-body:theme(colors.zinc.100)] dark:[--tw-prose-invert-headings:theme(colors.white)] dark:[--tw-prose-invert-bold:theme(colors.white)] dark:[--tw-prose-invert-code:theme(colors.zinc.100)]" }`} > - {msg.content} + ` + // with no target and no scheme handling, so: + // + // 1. http/https links navigate the canvas tab + // itself away — user loses canvas state. + // 2. workspace://, file://, and bare /workspace/ + // paths from agent-authored markdown produce + // an unhandled-protocol click → browser ends + // up at about:blank with no download (the + // reported bug from 2026-05-05). + // + // Override: external URLs open in a new tab with + // rel="noopener noreferrer"; in-container paths + // route through downloadChatFile so the browser + // gets a real Blob with proper auth headers. + a: ({ href, children, ...rest }) => { + const url = String(href ?? ""); + const containerPath = url.startsWith("workspace:") || + url.startsWith("file:///workspace") || + url.startsWith("file:///configs") || + url.startsWith("file:///home") || + url.startsWith("file:///plugins") || + url.startsWith("/workspace") || + url.startsWith("/configs") || + url.startsWith("/home") || + url.startsWith("/plugins"); + if (containerPath) { + return ( + { + e.preventDefault(); + // Construct a synthetic ChatAttachment + // and route through the same + // authenticated download path the + // download chips use. Filename is the + // last path segment so Save-As prefills + // sensibly. + const name = url.split(/[\\/]/).pop() || "download"; + downloadChatFile(workspaceId, { + uri: url, + name, + }).catch((err) => { + setError( + err instanceof Error + ? `Download failed: ${err.message}` + : "Download failed", + ); + }); + }} + > + {children} + + ); + } + // External (http(s) / mailto / unknown scheme): + // open in new tab so canvas state survives. + return ( + + {children} + + ); + }, + }} + >{msg.content} )} {msg.attachments && msg.attachments.length > 0 && ( @@ -1167,7 +1240,22 @@ function MyChatPanel({ workspaceId, data }: Props) { value={input} onChange={(e) => setInput(e.target.value)} onKeyDown={(e) => { - if (e.key === "Enter" && !e.shiftKey) { + // IME-safe send: while a CJK / Japanese / Korean IME is + // composing, Enter accepts the candidate selection — not a + // newline, not a send. `e.nativeEvent.isComposing` is the + // standard signal (modern WebKit/Blink/Gecko); the keyCode + // 229 fallback covers older Safari / WebKit-based mobile + // browsers that delay setting isComposing on the + // composition-end Enter. Reported 2026-05-05: typing + // Chinese with the system IME, pressing Enter to commit + // a candidate would inadvertently send the half-typed + // message. + if ( + e.key === "Enter" && + !e.shiftKey && + !e.nativeEvent.isComposing && + e.keyCode !== 229 + ) { e.preventDefault(); sendMessage(); } diff --git a/canvas/src/components/tabs/__tests__/ChatTab.imeAndLinks.test.tsx b/canvas/src/components/tabs/__tests__/ChatTab.imeAndLinks.test.tsx new file mode 100644 index 00000000..6bedc22a --- /dev/null +++ b/canvas/src/components/tabs/__tests__/ChatTab.imeAndLinks.test.tsx @@ -0,0 +1,140 @@ +// @vitest-environment jsdom +// +// Pins two regressions reported on production 2026-05-05: +// +// 1. IME composition + Enter key: typing Chinese (or any CJK / IME- +// composed text) and pressing Enter to commit the candidate +// selection used to send the half-typed message. The fix checks +// `event.nativeEvent.isComposing` (and a `keyCode === 229` +// fallback for older WebKit) before treating Enter as send. +// +// 2. Markdown link clicks: the agent's ReactMarkdown-rendered links +// used to: +// - http/https → navigate canvas tab away (user lost canvas state) +// - workspace://path / file:///workspace/... / /workspace/... → +// browser hit about:blank (unhandled protocol). +// Fix: external links get target="_blank" + noopener; in-container +// paths route through downloadChatFile (same auth path as chips). + +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; +import { render, screen, cleanup, fireEvent, waitFor } from "@testing-library/react"; +import React from "react"; + +afterEach(cleanup); + +// Mock the api module so render doesn't try to talk to a real CP. +const apiGet = vi.fn(() => Promise.resolve([])); +const apiPost = vi.fn(() => Promise.resolve({})); +vi.mock("@/lib/api", () => ({ + api: { + get: (path: string) => apiGet(path), + post: (path: string, body: unknown) => apiPost(path, body), + del: vi.fn(), + patch: vi.fn(), + put: vi.fn(), + }, +})); + +vi.mock("@/store/canvas", () => ({ + useCanvasStore: vi.fn((selector?: (s: unknown) => unknown) => + selector ? selector({ agentMessages: {}, consumeAgentMessages: () => [] }) : {}, + ), +})); + +// Capture the downloadChatFile call so the markdown-link test can +// assert in-container paths route through the authenticated download +// path rather than the browser's bare anchor click. +const downloadChatFileMock = vi.fn(() => Promise.resolve()); +vi.mock("../chat/uploads", async () => { + const actual = await vi.importActual("../chat/uploads"); + return { + ...actual, + downloadChatFile: (...args: unknown[]) => downloadChatFileMock(...args), + }; +}); + +beforeEach(() => { + apiGet.mockClear(); + apiPost.mockClear(); + downloadChatFileMock.mockClear(); + // jsdom doesn't implement scrollIntoView; ChatTab calls it after + // every render with a new message. + Element.prototype.scrollIntoView = vi.fn(); + // Stub IntersectionObserver — the lazy-history sentinel uses it. + class FakeIO { + observe() {} + unobserve() {} + disconnect() {} + } + (window as unknown as { IntersectionObserver: unknown }).IntersectionObserver = FakeIO; + (globalThis as unknown as { IntersectionObserver: unknown }).IntersectionObserver = FakeIO; +}); + +import { ChatTab } from "../ChatTab"; + +const minimalData = { + status: "online" as const, + runtime: "claude-code", + currentTask: null, +} as unknown as Parameters[0]["data"]; + +describe("ChatTab — IME-safe Enter key", () => { + it("does NOT send the message when Enter fires during IME composition (isComposing)", async () => { + render(); + + // Find the textarea by its aria-label. + const textarea = await screen.findByLabelText(/Message to agent/i); + fireEvent.change(textarea, { target: { value: "你好" } }); + + // Simulate the Enter that commits an IME selection: isComposing=true. + fireEvent.keyDown(textarea, { key: "Enter", isComposing: true }); + + // sendMessage POSTs via api.post; assert it was NOT called. + await waitFor(() => { + expect(apiPost).not.toHaveBeenCalled(); + }); + // And the input is preserved — ChatTab clears it only on actual send. + expect((textarea as HTMLTextAreaElement).value).toBe("你好"); + }); + + it("does NOT send when keyCode is 229 (older Safari IME fallback)", async () => { + render(); + const textarea = await screen.findByLabelText(/Message to agent/i); + fireEvent.change(textarea, { target: { value: "한국어" } }); + + // keyCode 229 is the older-Safari signal that an IME is composing. + // Some mobile WebKit-based browsers delay setting isComposing on + // the composition-end Enter; the keyCode fallback covers that. + fireEvent.keyDown(textarea, { key: "Enter", keyCode: 229 }); + + await waitFor(() => { + expect(apiPost).not.toHaveBeenCalled(); + }); + }); + + it("DOES send on a non-composing Enter (the happy path stays intact)", async () => { + render(); + const textarea = await screen.findByLabelText(/Message to agent/i); + fireEvent.change(textarea, { target: { value: "hello world" } }); + + fireEvent.keyDown(textarea, { key: "Enter" /* no isComposing, no 229 */ }); + + // The api.post for /a2a fires inside sendMessage. waitFor since + // the call goes through several effects. + await waitFor(() => { + expect(apiPost).toHaveBeenCalled(); + }); + }); + + it("Shift+Enter inserts newline regardless (no send)", async () => { + render(); + const textarea = await screen.findByLabelText(/Message to agent/i); + fireEvent.change(textarea, { target: { value: "line 1" } }); + + fireEvent.keyDown(textarea, { key: "Enter", shiftKey: true }); + + await waitFor(() => { + expect(apiPost).not.toHaveBeenCalled(); + }); + }); +});