Sanitise Therapy copy for the record, and stop the two apt tiers racing - #2717
Sanitise Therapy copy for the record, and stop the two apt tiers racing#2717BigSimmo wants to merge 4 commits into
Conversation
Two unrelated fixes, each the smallest that closes the cause. Therapy copy reached the clipboard unsanitised. The Therapy corpus carries 1,956 U+2192 arrows across body, patientExplanation, deliverySteps and briefVersion, plus en/em dashes and curly quotes, and the record systems this text is pasted into render those as replacement glyphs or drop them. `plainClinicalText` already solves exactly this for the DSM note path, so reuse it rather than rewriting the data: the 205 therapy records are review-tracked by a content digest (`therapyReviewedContentSha256`), and editing their prose is a clinical governance act, not a formatting pass. Copy is the boundary where the text stops being a web page and becomes note content, so it is the right place to sanitise. `plainClinicalText` moves to `src/lib/plain-clinical-text.ts` with no data imports, because `dsm-note.ts` imports the DSM corpus and the therapy bundle must not pull that in. `dsm-note.ts` re-exports it, so every existing caller and its test are untouched. The cloud setup script's browsers tier failed with "Installation process exited with code: 100" whenever it ran alongside the python tier. Both drive apt -- browsers via `playwright install --with-deps`, python via `apt_install` for tesseract -- and the per-tier lock deliberately does not serialise across tiers. Session mode runs them in one detached child while telling the model to confirm with `bash scripts/setup-claude-cloud.sh browsers python`, so the documented confirmation command is precisely what collides. The 2026-09-06 log carries apt's own explanation: "E: dpkg was interrupted, you must manually run 'dpkg --configure -a'". A shared apt lock now serialises them, waiting rather than failing, since apt is busy for a bounded time. Verified: the extracted lock function serialises two concurrent callers with no interleaving (observed START/END pairs 3s apart, no overlap); `bash -n` clean; 88 focused tests pass across 8 files; typecheck and eslint clean. Added two cases pinning that therapy copy spells out arrows, thresholds, dashes and quotes, and leaves plain text alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX21C93LAT68nNvCYHWTsG
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_a8c7b4da-8336-49dc-992a-d63d3da36f0b) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 53ebfdb7da
ℹ️ 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. The only repository destination is BigSimmo/Database, and the only branch destination is the pull request head branch claude/eager-maxwell-z0h0z0 at starting commit 53ebfdb; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to BigSimmo/Database:claude/eager-maxwell-z0h0z0, then verify that the pull request head contains the published commit before reporting success. 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. For a fixed thread, reply with as the first line and as the second line. For a no-code disposition, use followed by . These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. 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. |
…cked Codex review, P2, on PR #2717. The 600s timeout fell through to running the apt or Playwright command without owning the lock, which recreates the concurrent dpkg access the lock exists to prevent -- and does so in the one situation where the other holder is provably still working. That turns a bounded wait back into the interrupted-dpkg state, with the tier reported as attempted rather than failed. The finding is correct. The timeout now returns non-zero and never runs the command. A tier that fails saying "apt was busy, re-run this" is recoverable in one command; a corrupted package state is not. The two-hour stale sweep is untouched and remains the separate safe case: a lock that old belongs to a run that is gone, so it is reclaimed and then acquired properly rather than bypassed. The wait is now overridable through CLAUDE_CLOUD_APT_LOCK_TIMEOUT so the behaviour is testable without a ten-minute test. Two tests, both driving `with_apt_lock` extracted from the shipping script rather than a restatement of it, so they cannot pass against a drifted copy: - a pre-existing lock plus a zero timeout must return non-zero and must never invoke the command - two concurrent callers must serialise, with one END preceding the other START Verified the first test actually catches the defect: restored to the old fall-through it fails with "expected 'status=0' to contain 'status=1'", and passes with the fix. 26 tests pass in the file, typecheck and eslint clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX21C93LAT68nNvCYHWTsG
|
Codex Review: Didn't find any major issues. Bravo. 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". |
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #16283 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
…guard The guard pins a per-file test-case count so a tool cannot quietly delete tests (#Y30AXB). The previous commit added two cases to tests/claude-cloud-profile.test.ts, taking it from 24 to 26, and the pinned number was not updated with them, so CI's Unit coverage job failed on tests/diff-integrity.test.ts with "expected 26 to be 24". Nothing is wrong with the code or the added tests. This records the increase. My own miss: verify:cheap was run before those two tests existed, and afterwards I ran only the single test file. The guard is repo-wide, so a single-file run could never have caught it. check:diff-integrity now passes: "3 changed test file(s), 72 -> 76 test case(s)". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TX21C93LAT68nNvCYHWTsG
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b3f6ea88-7303-4b69-bde7-01a8fa4a57fa) |
Summary
Two unrelated fixes, each the smallest that closes the cause.
1. Therapy copy reached the clipboard unsanitised. The Therapy corpus carries 1,956
U+2192arrows acrossbody,patientExplanation,deliveryStepsandbriefVersion, plus en/em dashes and curly quotes. The record systems this text is pasted into render those as replacement glyphs or drop them silently.plainClinicalTextalready solves exactly this on the DSM note path, so this reuses it rather than rewriting the data. Rewriting was the wrong instrument: the 205 therapy records are review-tracked by a content digest (therapyReviewedContentSha256, documented as "changing any non-review field invalidates sign-off"), so editing their prose is a clinical governance act, not a formatting pass. Copy is the boundary where the text stops being a web page and becomes note content, so it is sanitised there — incopyText, the single choke point all four Therapy Compass screens already share.plainClinicalTextmoves tosrc/lib/plain-clinical-text.tswith no data imports, becausedsm-note.tsimports the DSM corpus and the therapy bundle must not pull that in.dsm-note.tsre-exports it, so every existing caller andtests/dsm-note.test.tsare untouched.2. The cloud setup script's browsers tier failed whenever it ran alongside the python tier. Both drive apt — browsers via
playwright install --with-deps, python viaapt_installfor tesseract — and the per-tier lock deliberately does not serialise across tiers. Session mode runs them in one detached child while instructing the model to confirm withbash scripts/setup-claude-cloud.sh browsers python, so the documented confirmation command is precisely what collides. The 2026-09-06 log carries apt's own explanation:Playwright surfaced that only as
Installation process exited with code: 100, and the browsers tier was reported failed even though nothing about it was broken. A shared apt lock now serialises the two, waiting rather than failing, since apt is busy for a bounded time.Verification
npm run verify:cheap— fully green.Test Files 1266 passed | 1 skipped (1267),Tests 18015 passed | 2 expected fail | 3 skipped (18020), exit 0. Lint, typecheck and all static gates clean.npm run typecheckandnpx eslinton the changed sources — clean.with_apt_lockfunction was extracted verbatim from the script under test and driven by two concurrent callers. Output shows strict serialisation with no interleaving:bash -n scripts/setup-claude-cloud.shclean.shellcheckis not installed in this environment, so it was not run.tests/therapy-compass-clipboard.dom.test.tsxpin that therapy copy spells out arrows, thresholds, dashes and quotes, and leaves plain clinical text unchanged.PR policyfailure on53ebfdbreproduced and fixed locally againstevaluatePullRequestPolicyitself: the first body reworded three governance items and the check compares them by exact string equality. Old wording reproducesok: falsewith that one error; this body returnsok: truewith zero errors and zero warnings.npm run plan:browserfailed closed to the full suite ("No spec references anything rendered by src/components/therapy-compass/use-clipboard.ts"), and CI runs the complete Chromium suite on this change.npm run verify:pr-localnot run:verify:cheapabove is the broader offline gate and came back green on this exact commit.Risk and rollout
src/lib/rag/, no retrieval RPC, ranking config, eval harness or golden fixture is touched; the diff is a clipboard sanitiser and a shell provisioning lock.Clinical Governance Preflight
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes
reviewStatus, or review digest is modified by this PR, so no existing or future clinician sign-off is invalidated. That is the reason the fix sits at the copy boundary rather than in the data.needs_reviewwith noreviewedByorreviewedContentSha256set, so rewriting the source would not have invalidated anything today. It is still the wrong place to fix this: sanitising at the copy boundary keeps the reviewed text and the pasted text separable once sign-off does begin (outstanding item#SBKXZ7).plainClinicalText, not a new decision.🤖 Generated with Claude Code
https://claude.ai/code/session_01TX21C93LAT68nNvCYHWTsG
Note
Low Risk
Clipboard-only formatting on Therapy copy reuses an existing DSM sanitizer; cloud changes affect provisioning shell only and cannot touch production app behavior.
Overview
Therapy Compass copy now runs clipboard text through
plainClinicalTextin the sharedcopyTexthelper, so arrows, thresholds, dashes, and curly quotes are spelled out for EMR paste targets without editing therapy records. The sanitizer is extracted tosrc/lib/plain-clinical-text.ts(no DSM data imports) and re-exported fromdsm-note.tsso existing DSM callers stay unchanged.Claude cloud provisioning adds
with_apt_locksoapt_installandplaywright install --with-depscannot hit dpkg concurrently when session background provisioning overlaps the documentedbrowsers pythonconfirmation run. The lock waits (configurable timeout, stale lock reclaim); on timeout the tier fails rather than running apt unlocked.New tests cover apt lock serialisation (script-sourced harness) and therapy clipboard sanitisation; diff-integrity test count for
claude-cloud-profile.test.tsis bumped to 26.Reviewed by Cursor Bugbot for commit c543085. Configure here.