From 53ebfdb7da057d704f33e7a6a4258eae17269449 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 09:16:32 +0000 Subject: [PATCH 1/3] Sanitise Therapy copy for the record, and stop the two apt tiers racing 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 Claude-Session: https://claude.ai/code/session_01TX21C93LAT68nNvCYHWTsG --- scripts/setup-claude-cloud.sh | 44 +++++++++++++++++-- .../therapy-compass/use-clipboard.ts | 8 +++- src/lib/dsm-note.ts | 39 ++-------------- src/lib/plain-clinical-text.ts | 39 ++++++++++++++++ tests/therapy-compass-clipboard.dom.test.tsx | 28 ++++++++++++ 5 files changed, 118 insertions(+), 40 deletions(-) create mode 100644 src/lib/plain-clinical-text.ts diff --git a/scripts/setup-claude-cloud.sh b/scripts/setup-claude-cloud.sh index 23f7e48292..d0246bb596 100755 --- a/scripts/setup-claude-cloud.sh +++ b/scripts/setup-claude-cloud.sh @@ -120,12 +120,49 @@ mark_tier_done() { have() { command -v "$1" >/dev/null 2>&1; } +# Serialises everything that drives apt, which the per-tier lock deliberately does not. +# +# The browsers tier shells out to `playwright install --with-deps` and the python tier calls +# apt_install for tesseract, so the two reach the same dpkg lock from different tiers. Session mode +# runs them in one detached child while telling the model to confirm with +# `bash scripts/setup-claude-cloud.sh browsers python`, and that confirmation command is exactly what +# collides. Observed 2026-09-06: the confirmation run's Playwright step died with "Installation +# process exited with code: 100" while the background child was unpacking tesseract, and the log +# carried apt's own explanation — "E: dpkg was interrupted, you must manually run 'dpkg --configure +# -a'". The browsers tier was then reported as failed even though nothing about it was broken. +# +# Waiting rather than failing is the point: apt is genuinely busy for a bounded time, so a caller that +# waits gets the install it asked for. Ten minutes is well past the slowest observed apt step and short +# enough that a truly wedged lock still surfaces instead of hanging the container. +with_apt_lock() { + local lock="$marker_dir/apt.lock" waited=0 + while ! mkdir "$lock" 2>/dev/null; do + if [ -n "$(find "$lock" -maxdepth 0 -mmin +120 2>/dev/null)" ]; then + warn "clearing a stale apt lock" + rm -rf "$lock" + continue + fi + if [ "$waited" -ge 600 ]; then + warn "apt is still locked by another run after 600s; proceeding without the lock" + "$@" + return + fi + [ "$waited" -eq 0 ] && log "waiting for another run's apt step to finish" + sleep 5 + waited=$((waited + 5)) + done + "$@" + local status=$? + rm -rf "$lock" 2>/dev/null + return "$status" +} + apt_install() { have apt-get || { warn "apt-get is unavailable; cannot install: $*"; return 1; } if [ "$(id -u)" = "0" ]; then - apt-get update -qq && apt-get install -y --no-install-recommends "$@" + with_apt_lock sh -c 'apt-get update -qq && apt-get install -y --no-install-recommends "$@"' _ "$@" elif have sudo; then - sudo apt-get update -qq && sudo apt-get install -y --no-install-recommends "$@" + with_apt_lock sh -c 'sudo apt-get update -qq && sudo apt-get install -y --no-install-recommends "$@"' _ "$@" else warn "neither root nor sudo; cannot install: $*" return 1 @@ -240,8 +277,9 @@ tier_deno() { tier_browsers() { [ -x ./node_modules/.bin/playwright ] || { warn "playwright is not installed; run npm ci first"; return 1; } + # `--with-deps` runs apt, so it takes the shared apt lock like apt_install does; see with_apt_lock. # shellcheck disable=SC2086 - ./node_modules/.bin/playwright install --with-deps ${CLAUDE_CLOUD_BROWSERS:-chromium firefox webkit} + with_apt_lock ./node_modules/.bin/playwright install --with-deps ${CLAUDE_CLOUD_BROWSERS:-chromium firefox webkit} } tier_python() { diff --git a/src/components/therapy-compass/use-clipboard.ts b/src/components/therapy-compass/use-clipboard.ts index 4996004d20..4694cbb881 100644 --- a/src/components/therapy-compass/use-clipboard.ts +++ b/src/components/therapy-compass/use-clipboard.ts @@ -2,6 +2,7 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { copyTextToClipboard } from "@/lib/copy-to-clipboard"; +import { plainClinicalText } from "@/lib/plain-clinical-text"; /** * Write text to the clipboard, guarded for SSR / unavailable API. Resolves to @@ -9,11 +10,16 @@ import { copyTextToClipboard } from "@/lib/copy-to-clipboard"; * 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. + * + * Everything copied here is destined for a progress note, so it goes through + * `plainClinicalText` first. The Therapy corpus carries 1,956 `\u2192` arrows plus + * en/em dashes and curly quotes, and the record systems this is pasted into + * render those as replacement glyphs or drop them silently. */ export async function copyText(text: string): Promise { if (typeof navigator === "undefined" || !text) return false; try { - await copyTextToClipboard(text); + await copyTextToClipboard(plainClinicalText(text)); return true; } catch { return false; diff --git a/src/lib/dsm-note.ts b/src/lib/dsm-note.ts index a0ddb63ded..6114ab716c 100644 --- a/src/lib/dsm-note.ts +++ b/src/lib/dsm-note.ts @@ -1,4 +1,7 @@ import { dsmCriteria, dsmSpecifierSplit, type DsmDiagnosis, type DsmLabeledText, type DsmSpecifier } from "@/lib/dsm"; +import { plainClinicalText } from "@/lib/plain-clinical-text"; + +export { plainClinicalText }; /** * Note text generation for the DSM diagnosis page's note builder. @@ -36,42 +39,6 @@ export type DsmNoteInput = { includeCriterionText: boolean; }; -/** - * Characters that survive a copy into a clinical record system. - * - * The vendored DSM export uses typographic characters throughout — 216 uses of - * `≥` alone, plus `≤ ≈ × ² – — → ↑` and curly quotes. Several of the systems - * this text is pasted into render those as replacement glyphs or drop them, and - * `≥4` silently becoming `4` reverses the meaning of a threshold. Each one is - * therefore spelled out rather than stripped. - * - * `â` (khyâl) and `é` (Guillain-Barré) are left alone: both are ordinary Latin-1 - * letters inside a correctly spelled clinical term, not typography. - * - * Semicolons are folded to commas because every semicolon in this corpus joins - * list items or an "or" clause, where a comma reads identically and matches how - * notes are written. - */ -const PLAIN_TEXT_REPLACEMENTS: ReadonlyArray = [ - [/≥\s*/g, "at least "], - [/≤\s*/g, "no more than "], - [/≈\s*/g, "approximately "], - [/↑\s*/g, "increased "], - [/\s*→\s*/g, " leading to "], - [/×/g, "x"], - [/²/g, "2"], - [/[–—]/g, "-"], - [/[‘’]/g, "'"], - [/[“”]/g, '"'], - [/;\s*/g, ", "], -]; - -export function plainClinicalText(value: string): string { - let text = value; - for (const [pattern, replacement] of PLAIN_TEXT_REPLACEMENTS) text = text.replace(pattern, replacement); - return text.replace(/[ \t]{2,}/g, " ").trim(); -} - /** * The specifier rows that are safe to offer as a tick box. * diff --git a/src/lib/plain-clinical-text.ts b/src/lib/plain-clinical-text.ts new file mode 100644 index 0000000000..8d5bc82ec8 --- /dev/null +++ b/src/lib/plain-clinical-text.ts @@ -0,0 +1,39 @@ +/** + * Characters that survive a copy into a clinical record system. + * + * The vendored DSM export uses typographic characters throughout — 216 uses of + * `≥` alone, plus `≤ ≈ × ² – — → ↑` and curly quotes. Several of the systems + * this text is pasted into render those as replacement glyphs or drop them, and + * `≥4` silently becoming `4` reverses the meaning of a threshold. Each one is + * therefore spelled out rather than stripped. + * + * `â` (khyâl) and `é` (Guillain-Barré) are left alone: both are ordinary Latin-1 + * letters inside a correctly spelled clinical term, not typography. + * + * Semicolons are folded to commas because every semicolon in this corpus joins + * list items or an "or" clause, where a comma reads identically and matches how + * notes are written. + * + * This module holds no data imports on purpose. The Therapy catalogue reaches a + * record through the same paste and needs the same guarantee, and importing it + * from `dsm-note.ts` would pull the whole DSM corpus into the therapy bundle. + */ +const PLAIN_TEXT_REPLACEMENTS: ReadonlyArray = [ + [/≥\s*/g, "at least "], + [/≤\s*/g, "no more than "], + [/≈\s*/g, "approximately "], + [/↑\s*/g, "increased "], + [/\s*→\s*/g, " leading to "], + [/×/g, "x"], + [/²/g, "2"], + [/[–—]/g, "-"], + [/[‘’]/g, "'"], + [/[“”]/g, '"'], + [/;\s*/g, ", "], +]; + +export function plainClinicalText(value: string): string { + let text = value; + for (const [pattern, replacement] of PLAIN_TEXT_REPLACEMENTS) text = text.replace(pattern, replacement); + return text.replace(/[ \t]{2,}/g, " ").trim(); +} diff --git a/tests/therapy-compass-clipboard.dom.test.tsx b/tests/therapy-compass-clipboard.dom.test.tsx index 7093e174e3..9c164fcb7a 100644 --- a/tests/therapy-compass-clipboard.dom.test.tsx +++ b/tests/therapy-compass-clipboard.dom.test.tsx @@ -34,6 +34,34 @@ afterEach(() => { } }); +describe("copyText record safety", () => { + // The Therapy corpus carries 1,956 arrows across body/patientExplanation/ + // deliverySteps/briefVersion. Copy is the boundary where that text stops being + // a web page and becomes note content, so it is sanitised here rather than by + // rewriting the reviewed source records. + it("spells out arrows and typographic characters before writing", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + setWriteText(writeText); + + await copyText( + "Build engagement \u2192 set goals \u2192 review; \u2265 4 sessions \u2014 \u201Cas tolerated\u201D", + ); + + expect(writeText).toHaveBeenCalledWith( + 'Build engagement leading to set goals leading to review, at least 4 sessions - "as tolerated"', + ); + }); + + it("leaves plain clinical text unchanged", async () => { + const writeText = vi.fn().mockResolvedValue(undefined); + setWriteText(writeText); + + await copyText("Behavioural activation, then graded exposure"); + + expect(writeText).toHaveBeenCalledWith("Behavioural activation, then graded exposure"); + }); +}); + describe("copyText", () => { it("resolves true and writes when the clipboard accepts the text", async () => { const writeText = vi.fn().mockResolvedValue(undefined); From 3ce87c7e60d0fdc2381703e1b040bc02730907e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 09:52:19 +0000 Subject: [PATCH 2/3] Fail the tier when the apt lock wait expires, instead of running unlocked 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 Claude-Session: https://claude.ai/code/session_01TX21C93LAT68nNvCYHWTsG --- scripts/setup-claude-cloud.sh | 17 +++++-- tests/claude-cloud-profile.test.ts | 82 ++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 4 deletions(-) diff --git a/scripts/setup-claude-cloud.sh b/scripts/setup-claude-cloud.sh index d0246bb596..f6fd026445 100755 --- a/scripts/setup-claude-cloud.sh +++ b/scripts/setup-claude-cloud.sh @@ -134,18 +134,27 @@ have() { command -v "$1" >/dev/null 2>&1; } # Waiting rather than failing is the point: apt is genuinely busy for a bounded time, so a caller that # waits gets the install it asked for. Ten minutes is well past the slowest observed apt step and short # enough that a truly wedged lock still surfaces instead of hanging the container. +# +# The timeout FAILS THE TIER; it never runs the command unlocked. Falling through to an unlocked run +# would recreate the exact concurrent dpkg access this function exists to prevent, and it would do so +# in the one situation where the other holder is provably still working — turning a bounded wait back +# into the interrupted-dpkg state, with the tier reported as attempted. 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 above is 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. with_apt_lock() { local lock="$marker_dir/apt.lock" waited=0 + local timeout="${CLAUDE_CLOUD_APT_LOCK_TIMEOUT:-600}" while ! mkdir "$lock" 2>/dev/null; do if [ -n "$(find "$lock" -maxdepth 0 -mmin +120 2>/dev/null)" ]; then warn "clearing a stale apt lock" rm -rf "$lock" continue fi - if [ "$waited" -ge 600 ]; then - warn "apt is still locked by another run after 600s; proceeding without the lock" - "$@" - return + if [ "$waited" -ge "$timeout" ]; then + warn "apt is still held by another run after ${timeout}s; not running it unlocked" + warn "re-run this tier once the other run finishes" + return 1 fi [ "$waited" -eq 0 ] && log "waiting for another run's apt step to finish" sleep 5 diff --git a/tests/claude-cloud-profile.test.ts b/tests/claude-cloud-profile.test.ts index 0eba3bab32..09936acc63 100644 --- a/tests/claude-cloud-profile.test.ts +++ b/tests/claude-cloud-profile.test.ts @@ -402,6 +402,88 @@ describe("SessionStart registration", () => { }); }); +describe("setup-claude-cloud apt serialisation", () => { + // The browsers tier shells out to `playwright install --with-deps` and the python tier calls + // apt_install for tesseract. Both drive apt, and the per-tier lock deliberately does not serialise + // across tiers, so on 2026-09-06 the documented confirmation command + // (`bash scripts/setup-claude-cloud.sh browsers python`) collided with the session hook's own + // background child: the browsers tier died with Playwright's opaque "Installation process exited + // with code: 100" while the log carried apt's real explanation, "E: dpkg was interrupted". + // + // `with_apt_lock` is extracted from the real script rather than restated here, so these tests + // cannot pass against a copy that has drifted from the code that ships. + function aptLockHarness(home: string, body: string) { + const source = readFileSync(provisioner, "utf8"); + const start = source.indexOf("with_apt_lock() {"); + expect(start).toBeGreaterThan(-1); + const end = source.indexOf("\n}\n", start) + 3; + const fn = source.slice(start, end); + expect(fn).toContain('mkdir "$lock"'); + + const harness = join(home, "harness.sh"); + writeFileSync(harness, ['marker_dir="$1"', "log() { :; }", "warn() { :; }", fn, body].join("\n") + "\n", "utf8"); + return harness; + } + + it("fails the caller instead of running the command unlocked when the wait expires", () => { + const home = makeSandboxHome(); + const markers = join(home, "markers"); + mkdirSync(join(markers, "apt.lock"), { recursive: true }); + const ran = join(home, "ran"); + const harness = aptLockHarness(home, `with_apt_lock touch "${ran.replace(/\\/g, "/")}"; echo "status=$?"`); + + const result = spawnSync(bashCommand, [harness, markers], { + encoding: "utf8", + env: { ...process.env, CLAUDE_CLOUD_APT_LOCK_TIMEOUT: "0" }, + }); + + // Falling through to an unlocked run is the whole bug: it would reproduce the concurrent dpkg + // access precisely when the other holder is provably still working. + expect(result.stdout).toContain("status=1"); + expect(existsSync(ran)).toBe(false); + }); + + it("serialises two concurrent callers rather than letting them overlap", () => { + const home = makeSandboxHome(); + const markers = join(home, "markers"); + mkdirSync(markers, { recursive: true }); + const out = join(home, "out").replace(/\\/g, "/"); + const harness = aptLockHarness( + home, + `with_apt_lock sh -c 'echo "START $2" >> "$1"; sleep 1; echo "END $2" >> "$1"' _ "${out}" "$2"`, + ); + + const runs = ["A", "B"].map((label) => + spawnSync( + bashCommand, + ["-c", `bash "${harness.replace(/\\/g, "/")}" "${markers.replace(/\\/g, "/")}" ${label} &`], + { + encoding: "utf8", + env: { ...process.env, CLAUDE_CLOUD_APT_LOCK_TIMEOUT: "60" }, + }, + ), + ); + for (const run of runs) expect(run.status).toBe(0); + + // Give both detached callers time to finish their 1s critical sections plus the 5s poll. + const deadline = Date.now() + 30_000; + let lines: string[] = []; + while (Date.now() < deadline) { + lines = existsSync(out) ? readFileSync(out, "utf8").trim().split(/\r?\n/).filter(Boolean) : []; + if (lines.length >= 4) break; + spawnSync(bashCommand, ["-c", "sleep 0.5"]); + } + + expect(lines).toHaveLength(4); + // Whichever won, its END must precede the other's START; interleaving is the failure. + expect(lines[0]).toMatch(/^START /); + expect(lines[1]).toBe(lines[0].replace("START", "END")); + expect(lines[2]).toMatch(/^START /); + expect(lines[2]).not.toBe(lines[0]); + expect(lines[3]).toBe(lines[2].replace("START", "END")); + }); +}); + describe("profile snapshot fidelity", () => { it.skipIf(!hasVendoredSkills)("copies skills as real directories, not as the workstation's symlinks", () => { // ~/.claude/skills is mostly symlinks into ~/.agents/skills. A snapshot that preserved them would From 55f6fa9ff797030a38d778959111d1063c440b4c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 10:07:23 +0000 Subject: [PATCH 3/3] Record the new claude-cloud-profile test count in the diff-integrity 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 Claude-Session: https://claude.ai/code/session_01TX21C93LAT68nNvCYHWTsG --- tests/diff-integrity.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/diff-integrity.test.ts b/tests/diff-integrity.test.ts index 1d99b196fc..8041195579 100644 --- a/tests/diff-integrity.test.ts +++ b/tests/diff-integrity.test.ts @@ -97,7 +97,7 @@ describe("countTestCases", () => { const counts: Record = { "tests/guard-push-no-merge-base.test.ts": 1, "tests/pdf-extractor.test.ts": 6, - "tests/claude-cloud-profile.test.ts": 24, + "tests/claude-cloud-profile.test.ts": 26, }; for (const [path, expected] of Object.entries(counts)) { const source = readFileSync(resolve(REPOSITORY_ROOT, path), "utf8");