Skip to content

fix(safety-plan): clear copy-feedback timer on unmount - #1717

Merged
BigSimmo merged 21 commits into
mainfrom
cursor/safety-plan-copy-timer-a650
Aug 8, 2026
Merged

fix(safety-plan): clear copy-feedback timer on unmount#1717
BigSimmo merged 21 commits into
mainfrom
cursor/safety-plan-copy-timer-a650

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • Extract the production-only Safety Plan fix from draft mockup PR feat(mockups): clinician workflow features 07–15 design study #1662 before that design-scratch PR was closed.
  • Clear the clipboard copy-feedback setTimeout on unmount (and before scheduling a replacement) so jsdom teardown / remount cannot call setState after the environment tears down (window is not defined).
  • Guard async clipboard completion with mountedRef so a deferred navigator.clipboard.writeText() cannot setCopied or schedule the 1600ms reset after unmount.
  • Add a fake-timer DOM test proving the scheduled 1600ms reset is cleared on unmount and advancing past it after teardown does not throw.
  • No mockup routes or clinician-workflow design studies included.

Verification

  • npx vitest run --project jsdom tests/patient-safety-plan.dom.test.tsx tests/patient-safety-plan-privacy.dom.test.tsxTest Files 2 passed (2) / Tests 15 passed (15)
  • npm run verify:cheapTest Files 523 passed (523) / Tests 5583 passed | 4 skipped (5587)
  • npm run verify:pr-local — unit suite + build + Client bundle secret surface check passed + Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites)
  • UI verification not run: no routing/styling/browser chrome change; clipboard reset timing only
  • Provider/release/live gates not run: no approval

Risk and rollout

  • Risk: Low — clears a dangling timeout and ignores post-unmount clipboard completion; copy UX unchanged when the component stays mounted
  • Rollback: Revert the commits on this branch
  • Provider or production effects: None

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed — timer cleanup only; no clinical decision-support behaviour change

Notes

  • Cherry-picked from feat(mockups): clinician workflow features 07–15 design study #1662 commit 09b1d2b8e59eae162aa2451b34293f0d9da82cec, then hardened for the async-unmount Codex finding.
  • Branches for closed mockup PRs were left in place for recovery.
  • Review-and-fix pass: no product P0–P2; timer-clear coverage added; one late main sync after required CI green on 04cb18e4.
Open in WebOpen in Cursor

Avoids ReferenceError: window is not defined when the clipboard
copy reset fires after jsdom tears down the test environment.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@coderabbitai

coderabbitaiBot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:26 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b2688ccc-9012-4a28-994d-c149a899323f

📥 Commits

Reviewing files that changed from the base of the PR and between e71b23b and 36e1ea4.

📒 Files selected for processing (3)
  • docs/branch-review-ledger.md
  • src/components/patient-safety-plan.tsx
  • tests/patient-safety-plan.dom.test.tsx

Comment @coderabbitai help to get the list of available commands.

@supabase

supabaseBot commented Aug 8, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@BigSimmo
BigSimmo marked this pull request as ready for review August 8, 2026 06:03

@chatgpt-codex-connectorchatgpt-codex-connectorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:41e7f90399

ℹ️ 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".

Comment threadsrc/components/patient-safety-plan.tsx
@BigSimmo
BigSimmo enabled auto-merge (squash) August 8, 2026 06:25
@BigSimmo
BigSimmo disabled auto-merge August 8, 2026 06:26
cursoragentand others added 3 commits August 8, 2026 06:27
Guard the copy-feedback path with a mounted ref so a deferred
clipboard promise cannot setState or schedule the 1600ms reset after
teardown. Cover with a deferred-promise unmount test.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@cursoragent Work the current open PR end-to-end. Confirm the PR number and GitHub head first from context. If more than one open PR could apply, stop and say which one you would use and why.

Fetch and start from the remote tip that matches that GitHub head. If the named branch ref is missing or stale, use the PR head ref. Preserve unrelated local WIP, including any local-only ledger commits; do not discard dirty work, and do not treat a local-ahead commit as the reviewed tip. Do not merge the PR, force-push, rebase, or arm auto-merge unless I explicitly ask. No provider-backed gates without separate approval. If you cannot push or resolve threads, diagnose and comment only; if inline replies fail, resolve when possible and put dispositions in the summary comment. If auto-merge is already armed, push only for a real blocker, and avoid pushes that would cancel in-flight required CI unless the push itself clears that blocker.

If the PR is already merged or closed: confirm the head and merge commit, note required-CI outcome, post one summary, and stop.

Goal: deep review plus Bugbot, fix actionable issues with the smallest correct changes, clear merge / required-CI / thread blockers, run strong local offline verification, push fixes, append the review ledger, and post one PR summary. Prefer thoroughness over speed. Regenerate large assets only when a fix requires it; then run the asset check and keep compatibility aliases byte-identical where the repo uses them.

Snapshot the GitHub head SHA: tip, base, behind/ahead, mergeable state, merge-tree versus origin/main (real conflict versus behind-but-clean), required checks on that tip including Production UI when selected, advisory separately, unresolved actionable threads. Missing checks while dirty are not green. If the tip moves mid-work, re-snapshot and continue from the new head.

Ledger-lookup against that GitHub head under the heavy review-and-fix scope for this PR. Already reviewed at this head with clean merge-tree, green required checks, and no new actionable threads → summarize, comment, stop unless I asked for a fresh superseding pass. Follow the repo review protocol.

Unblock once: real conflict → merge origin/main (prefer main’s shared queues; keep this PR’s notes); behind-but-clean → one sync, late if required CI is in flight; clean → leave. Labels like skip-branch-sync do not block a manual sync needed to clear a real blocker. No sync thrash. After any sync or push, re-snapshot tip, merge-tree, and required checks before declaring done. Dedupe the ledger if a merge touched it.

Review high-confidence delta risks only. Separate PR-introduced defects from pre-existing re-emitted debt. For generated assets, review contracts, aliases, cache, and manifests—not every generated line. If protected RAG or ranking surfaces are touched, say so before editing; fix PR-body policy text only when wrong or missing. Ignore bot noise. No nit spam or broad rewrites. Escalate verification by touched risk: clinical / RAG / privacy / migrations / auth → domain check plus production-readiness when warranted; UI / phone-chrome / routing / styling → ensure plus phone-chrome or UI gates when warranted; generated assets or docs inventory/links → asset or docs checks when warranted.

Fix P0/P1 always; clear scoped P2 when locally provable; else disposition and resolve. Required-check failures on this tip only; ignore advisory. Prefer reverting a bad autofix commit. After fixes: format and commit before push; smallest targeted proof; repo cheap gate; repo PR-local gate. No release, lighthouse, live eval, or live provider gates without approval.

Push only this PR’s fix commits. Append ledger for the final GitHub head under the heavy scope. Never push a ledger-only tip—include ledger in a real fix push, or leave a no-change ledger append local and unpushed.

One PR comment: tip, sync/merge-tree, fixed versus dispositioned, threads, required CI, decisive local gate lines, residual risks; merge left to me. Inline only for remaining human-needed P0/P1.

Stop when merge-tree is clean on the current GitHub tip, actionable threads are clear, required checks are green or clearly in progress, heavy gates for scope passed, summary posted, merge left to me.

cursoragentand others added 2 commits August 8, 2026 08:45
Cover the original timer-clear failure path with fake timers so unmount
clears the 1600ms reset and advancing past it after teardown does not throw.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@cursor

cursorBot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

PR #1717 review-and-fix summary

Tip:20aae0377baaab3235ecd81e299e188c30483bd4 on cursor/safety-plan-copy-timer-a650
Base:origin/main (synced this pass)
Ahead/behind: 7 / 0
Merge-tree vs origin/main: clean (behind-but-clean → one late sync after required CI settled on pre-sync tip 04cb18e4)
Auto-merge: not armed · Merge left to you

Findings

  • No product P0/P1/P2 in the copy/unmount delta (mountedRef + timer clear is sound).
  • Fixed (coverage): added fake-timer DOM test proving the scheduled 1600ms copy-feedback timer is cleared on unmount and advancing past it after teardown does not throw (04cb18e4).
  • Dispositioned: prior Codex P2 (async clipboard after unmount) already resolved at 579513bb; thread remains resolved. No new actionable threads.
  • Bugbot: no high-confidence product defects; residual risk was the timer-clear test gap (now covered).

Required CI

  • Pre-sync tip 04cb18e4: PR required pass; Production UI critical/(1)/(2)/(3) all pass.
  • Post-sync tip 20aae037: required checks restarting/in progress after the main sync (expected).
  • Advisory only: Lighthouse budget fail (ignored).

Local gates (decisive lines)

  • npx vitest run --project jsdom tests/patient-safety-plan.dom.test.tsx tests/patient-safety-plan-privacy.dom.test.tsxTest Files 2 passed (2) / Tests 15 passed (15)
  • npm run verify:cheapTest Files 523 passed (523) / Tests 5583 passed | 4 skipped (5587)
  • npm run verify:pr-local → same unit suite + ✓ Compiled successfully + Client bundle secret surface check passed + Offline RAG fixture and manifest validation passed (36 golden cases, 23 suites)

Not run

  • Provider-backed / release / live eval / lighthouse remediation (no approval).
  • verify:ui / phone-chrome: no routing/styling/chrome change.

Residual risks

  • Silent clipboard catch (pre-existing; no failure UI).
  • Ledger row for this pass is local-only / unpushed (no ledger-only tip).

Threads

  • Unresolved actionable: 0

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the Safety Plan copy-to-clipboard UX against unmount/remount scenarios by preventing post-unmount state updates, addressing jsdom teardown errors during tests.

Changes:

  • Track component mount status and the copy-feedback timeout handle, clearing the timeout on unmount and preventing late async clipboard completion from scheduling state updates.
  • Reset/replace the 1600ms “Copied” feedback timer safely (clearing any prior timer before scheduling a new one).
  • Add jsdom tests covering (a) clipboard promise settling after unmount and (b) the 1600ms reset timer being cleared on unmount.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

FileDescription
src/components/patient-safety-plan.tsxAdds unmount-safe handling for async clipboard completion and the “Copied” feedback reset timer.
tests/patient-safety-plan.dom.test.tsxAdds fake-timer coverage to prove the feedback timer is not scheduled/does not fire after unmount.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadtests/patient-safety-plan.dom.test.tsx
BigSimmoand others added 2 commits August 9, 2026 01:13
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

github-actionsBot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

CI triage

CI failed on this PR. Automated classification of the 3 failed job(s):

  • Unit coverageneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • Static PR checksneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
  • PR requiredneeds investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #9281 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.

@BigSimmo
BigSimmo enabled auto-merge (squash) August 8, 2026 18:10
cursoragentand others added 4 commits August 8, 2026 18:27
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Copilot's clipboard-descriptor try/finally dropped the it() closer,
which broke Static PR formatting and Unit coverage parse.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
Replace Copilot autofix's (navigator as any) with a narrow typed
delete so Static PR eslint passes.
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
@BigSimmo
BigSimmo merged commit aa6cf68 into mainAug 8, 2026
26 checks passed
@BigSimmo
BigSimmo deleted the cursor/safety-plan-copy-timer-a650 branch August 8, 2026 18:56
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@BigSimmo@cursoragent