From be05af92f75d27d27fd25a7f44a936ba5368b44c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 14:29:17 +0000 Subject: [PATCH 1/2] fix(mockups): report Therapy Compass clipboard copy only on success Addresses a Codex review finding on #664's `use-clipboard.ts`: `copyText` fired `navigator.clipboard.writeText` and returned `true` synchronously, so a rejected write (permission denied, lost focus, blocked gesture) still flipped the button to "Copied" and left the promise rejection unhandled. - `copyText` is now async: it awaits the write and resolves `false` on rejection instead of throwing, so callers never signal a copy that didn't happen. - `useClipboard` sets `copied` only after a successful write, and guards against a state update after unmount. - Adds tests that mock `writeText` resolving/rejecting: `copyText` reports real success/failure without throwing, and `copied` stays unset when the write rejects. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TJaXJntdH7Q98ejoSZF46K --- .../therapy-compass/use-clipboard.ts | 44 +++++++---- tests/therapy-compass-clipboard.dom.test.tsx | 75 +++++++++++++++++++ 2 files changed, 105 insertions(+), 14 deletions(-) create mode 100644 tests/therapy-compass-clipboard.dom.test.tsx diff --git a/src/components/therapy-compass/use-clipboard.ts b/src/components/therapy-compass/use-clipboard.ts index 4d62dcf676..dc1850e25b 100644 --- a/src/components/therapy-compass/use-clipboard.ts +++ b/src/components/therapy-compass/use-clipboard.ts @@ -2,35 +2,51 @@ import { useCallback, useEffect, useRef, useState } from "react"; -/** Write text to the clipboard, guarded for SSR / unavailable API. Returns whether the write was attempted. */ -export function copyText(text: string): boolean { +/** + * Write text to the clipboard, guarded for SSR / unavailable API. Resolves to + * whether the write actually succeeded: a rejected write (permission denied, + * lost focus, a blocked user gesture) resolves to `false` instead of throwing, + * so callers never signal success for a copy that didn't happen and no unhandled + * promise rejection escapes. + */ +export async function copyText(text: string): Promise { if (typeof navigator === "undefined" || !navigator.clipboard || !text) return false; - void navigator.clipboard.writeText(text); - return true; + try { + await navigator.clipboard.writeText(text); + return true; + } catch { + return false; + } } /** * Clipboard copy with transient "copied" feedback. `copied` holds the key of the * most recently copied item (or null) and resets after `resetMs`, so a caller can - * flip a single button's label/icon without tracking its own timer. + * flip a single button's label/icon without tracking its own timer. The key is + * set only once the write actually succeeds, and never after the component has + * unmounted. */ export function useClipboard(resetMs = 1400) { const [copied, setCopied] = useState(null); const timer = useRef | null>(null); + const mounted = useRef(true); - useEffect( - () => () => { + useEffect(() => { + mounted.current = true; + return () => { + mounted.current = false; if (timer.current) clearTimeout(timer.current); - }, - [], - ); + }; + }, []); const copy = useCallback( (text: string, key = "default") => { - if (!copyText(text)) return; - setCopied(key); - if (timer.current) clearTimeout(timer.current); - timer.current = setTimeout(() => setCopied(null), resetMs); + void copyText(text).then((ok) => { + if (!ok || !mounted.current) return; + setCopied(key); + if (timer.current) clearTimeout(timer.current); + timer.current = setTimeout(() => setCopied(null), resetMs); + }); }, [resetMs], ); diff --git a/tests/therapy-compass-clipboard.dom.test.tsx b/tests/therapy-compass-clipboard.dom.test.tsx new file mode 100644 index 0000000000..42cdc62c57 --- /dev/null +++ b/tests/therapy-compass-clipboard.dom.test.tsx @@ -0,0 +1,75 @@ +import { act, renderHook } from "@testing-library/react"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { copyText, useClipboard } from "@/components/therapy-compass/use-clipboard"; + +// The clipboard helper must only report success when the browser actually +// accepts the write. A rejected `writeText` (permission denied, lost focus, +// blocked gesture) must resolve to `false` without throwing, so callers never +// flip to "Copied" for a copy that didn't happen. + +const originalClipboard = Object.getOwnPropertyDescriptor(globalThis.navigator, "clipboard"); + +function setWriteText(writeText: ((text: string) => Promise) | null) { + Object.defineProperty(globalThis.navigator, "clipboard", { + value: writeText ? { writeText } : undefined, + configurable: true, + writable: true, + }); +} + +// Let the two-microtask copyText().then() chain settle inside act(). +async function settle() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +afterEach(() => { + vi.restoreAllMocks(); + if (originalClipboard) { + Object.defineProperty(globalThis.navigator, "clipboard", originalClipboard); + } else { + setWriteText(null); + } +}); + +describe("copyText", () => { + it("resolves true and writes when the clipboard accepts the text", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + setWriteText(writeText); + await expect(copyText("hello")).resolves.toBe(true); + expect(writeText).toHaveBeenCalledWith("hello"); + }); + + it("resolves false — never throws — when the write rejects", async () => { + setWriteText(vi.fn().mockRejectedValue(new Error("NotAllowedError"))); + await expect(copyText("hello")).resolves.toBe(false); + }); + + it("resolves false for empty text without touching the clipboard", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + setWriteText(writeText); + await expect(copyText("")).resolves.toBe(false); + expect(writeText).not.toHaveBeenCalled(); + }); +}); + +describe("useClipboard", () => { + it("sets copied to the key only after a successful write", async () => { + setWriteText(vi.fn().mockResolvedValue(undefined)); + const { result } = renderHook(() => useClipboard()); + expect(result.current.copied).toBeNull(); + act(() => result.current.copy("hello", "step-1")); + await settle(); + expect(result.current.copied).toBe("step-1"); + }); + + it("leaves copied unset when the write rejects", async () => { + setWriteText(vi.fn().mockRejectedValue(new Error("NotAllowedError"))); + const { result } = renderHook(() => useClipboard()); + act(() => result.current.copy("hello", "step-1")); + await settle(); + expect(result.current.copied).toBeNull(); + }); +}); From 5a89a521add5a02dc4f6dd640b393c5bdd690183 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 14 Jul 2026 14:42:54 +0000 Subject: [PATCH 2/2] fix(mockups): ignore stale out-of-order clipboard completions Addresses a second Codex review finding on #667: useClipboard is shared across multiple copy controls (e.g. Brief's per-step buttons plus the intervention copy action), so if two writes are in flight and their promises resolve out of order, an older completion could overwrite the newer copy's feedback with the wrong key. - Tag each copy with a monotonic request id; a completion only updates `copied` when it is still the latest request, so stale / out-of-order successes are ignored and the hook contract (copied reflects the most recent copy) holds. - Adds a test that resolves two in-flight writes out of order and asserts the latest key wins and the stale completion does not overwrite it. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01TJaXJntdH7Q98ejoSZF46K --- .../therapy-compass/use-clipboard.ts | 7 +++++- tests/therapy-compass-clipboard.dom.test.tsx | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/components/therapy-compass/use-clipboard.ts b/src/components/therapy-compass/use-clipboard.ts index dc1850e25b..c966a90e91 100644 --- a/src/components/therapy-compass/use-clipboard.ts +++ b/src/components/therapy-compass/use-clipboard.ts @@ -30,6 +30,7 @@ export function useClipboard(resetMs = 1400) { const [copied, setCopied] = useState(null); const timer = useRef | null>(null); const mounted = useRef(true); + const latestRequest = useRef(0); useEffect(() => { mounted.current = true; @@ -41,8 +42,12 @@ export function useClipboard(resetMs = 1400) { const copy = useCallback( (text: string, key = "default") => { + // One hook instance is shared across several controls, so tag each request + // and ignore stale completions: a later copy supersedes this one, and its + // (possibly out-of-order) success must not overwrite the newer feedback. + const request = ++latestRequest.current; void copyText(text).then((ok) => { - if (!ok || !mounted.current) return; + if (!ok || !mounted.current || request !== latestRequest.current) return; setCopied(key); if (timer.current) clearTimeout(timer.current); timer.current = setTimeout(() => setCopied(null), resetMs); diff --git a/tests/therapy-compass-clipboard.dom.test.tsx b/tests/therapy-compass-clipboard.dom.test.tsx index 42cdc62c57..7093e174e3 100644 --- a/tests/therapy-compass-clipboard.dom.test.tsx +++ b/tests/therapy-compass-clipboard.dom.test.tsx @@ -72,4 +72,29 @@ describe("useClipboard", () => { await settle(); expect(result.current.copied).toBeNull(); }); + + it("ignores a stale out-of-order completion and keeps the most recent copy's key", async () => { + const resolvers: Array<() => void> = []; + setWriteText(vi.fn().mockImplementation(() => new Promise((resolve) => resolvers.push(resolve)))); + const { result } = renderHook(() => useClipboard()); + + act(() => result.current.copy("first", "k-first")); + act(() => result.current.copy("second", "k-second")); + expect(resolvers).toHaveLength(2); + + // The most recent (second) write resolves first — its key wins. + await act(async () => { + resolvers[1](); + await Promise.resolve(); + }); + expect(result.current.copied).toBe("k-second"); + + // The stale (first) write resolves later — it must NOT overwrite the newer feedback. + await act(async () => { + resolvers[0](); + await Promise.resolve(); + await Promise.resolve(); + }); + expect(result.current.copied).toBe("k-second"); + }); });