fix(mockups): report Therapy Compass clipboard copy only on success#667
Conversation
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJaXJntdH7Q98ejoSZF46K
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughClipboard copying is now asynchronous, reports actual write success, and handles rejected writes. The hook coordinates successful feedback, unmount cleanup, timeout cleanup, and out-of-order requests. New DOM tests cover utility and hook behavior. ChangesClipboard async safety
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant useClipboard
participant ClipboardAPI
Caller->>useClipboard: copy(key)
useClipboard->>ClipboardAPI: writeText(key)
ClipboardAPI-->>useClipboard: resolve success or failure
useClipboard-->>Caller: update copied feedback for latest request
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: be05af92f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJaXJntdH7Q98ejoSZF46K
Summary
Follow-up to #664 (merged) addressing a Codex review finding that landed after auto-merge — Codex and CodeRabbit are advisory here, so the squash merge didn't wait for the auto-resolve pass to commit a fix.
In
src/components/therapy-compass/use-clipboard.ts,copyTextfirednavigator.clipboard.writeTextand returnedtruesynchronously, so a rejected write (permission denied, lost focus, blocked gesture) still flipped the button to "Copied" and left the promise rejection unhandled.copyTextis now async: it awaits the write and resolvesfalseon rejection (no throw), so callers never signal a copy that didn't happen.useClipboardsetscopiedonly after a successful write, and guards against a state update after unmount.copyTextwas only consumed internally byuseClipboard; the screens use the hook'scopy(still fire-and-forgetvoid).Dev-only mockup route (
/mockups/therapy-compass, production 404 +robotsdisallow); no production surface, data, schema, or governance change. 2 files (+105/−14).Verification
tests/therapy-compass-clipboard.dom.test.tsx(jsdom) — 5/5 pass:copyTextresolvestrue/falsefor accept/reject and never throws;copiedis set on success and stays unset whenwriteTextrejects.npm run verify:pr-local— not run to completion in this sandbox (optional deps). Instead ran typecheck, lint (--max-warnings 0), Prettier--check, and the new test directly — all green. CIBuild/Static PR checks/Unit coveragecover the full install.npm run verify:ui— CICritical UI smoke/UI regressioncover the Chromium suite; the change is clipboard-behavior only (no visual change).npm run eval:retrieval:quality— N/A. No retrieval, ranking, selection, chunking, or scoring change.npm run check:production-readiness— N/A. No clinical workflow, privacy, environment, Supabase, source-governance, or deployment change.Clinical Governance Preflight
N/A for production surfaces — static, dev-only mockup. It does not touch ingestion, answer generation, search/ranking of live documents, document access, privacy, production env, Supabase, or clinical output. No schema, RLS, or service-role changes.
Notes
The finding was a valid P2 from Codex on #664; auto-merge (which only gates on required checks, not advisory reviews) merged before the Codex auto-resolve pass could commit. This PR is that fix, with a behavioral test guarding the copy-success contract.
🤖 Generated with Claude Code
https://claude.ai/code/session_01TJaXJntdH7Q98ejoSZF46K
Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
Tests