diff --git a/data/outstanding-issues-snapshot.json b/data/outstanding-issues-snapshot.json index b585fb5eb3..fb9ed7a162 100644 --- a/data/outstanding-issues-snapshot.json +++ b/data/outstanding-issues-snapshot.json @@ -2,7 +2,7 @@ "version": "outstanding-issues-snapshot-v1", "ledger_revision": { "sha": "6085a0a59aca4c1bb9e19fb4d490fd34dec950cd", - "committed_at": "2026-08-22T20:52:39+00:00" + "committed_at": "2026-08-22T20:52:39Z" }, "counts": { "open": 73, @@ -10,7 +10,7 @@ "p2": 40, "p3": 33, "queued": 10, - "pending": 128, + "pending": 138, "resolved": 376 }, "queue": [ @@ -851,12 +851,24 @@ "summary": "#4XBMMR: Resolved by retaining the non-sensitive public design comps while making their indexing policy truthful and enforceable: next.config.ts now serves X-Robots-Tag: noindex, nofollow for exact /mockups/:path* assets, and mockups/README.md now distinguishes public retrieval from indexing and access control. tests/mockup-crawler-policy.test.ts imports the effective Next config, validates the route grammar, proves nested mockup assets match, and proves unrelated assets do not. Focused combined policy verification passed 3 files and 26 tests; the focused proxy production-block test passed 1 test with 14 skipped on 2026-08-23.", "created_at": "2026-08-23" }, + { + "request_id": "1838b99a-9323-4e3b-84a5-8e52535ecec8", + "action": "add", + "summary": "Caring Contacts: the governed-message validator has zero production callers", + "created_at": "2026-08-24" + }, { "request_id": "183965e3-ec47-48cf-a99f-8b037311ed2d", "action": "add", "summary": "Patient-profile alerts surface unassessed gates only for contraindication rows, so 266 caution, dose-adjust and monitor gates silently no-op", "created_at": "2026-08-23" }, + { + "request_id": "18b32d38-540f-4d7c-af35-3f4f067e4998", + "action": "add", + "summary": "Caring Contacts: nothing checks screen wording against the prohibited-vocabulary list", + "created_at": "2026-08-24" + }, { "request_id": "1902cbe5-1188-4e3e-b96e-7578419ab071", "action": "queue", @@ -1199,6 +1211,12 @@ "summary": "docs:check-links passes over a broken relative markdown link in a spec, so a binding document went missing unnoticed", "created_at": "2026-08-18" }, + { + "request_id": "854ca9ba-ba3b-442b-be6d-6f5873ed60ed", + "action": "add", + "summary": "Caring Contacts: connection-unavailable and permission-unavailable have no runtime caller", + "created_at": "2026-08-24" + }, { "request_id": "8f2c28e6-aeef-4648-bb23-c1bd2222226b", "action": "done", @@ -1235,12 +1253,24 @@ "summary": "Cancel request d627018c-e5ce-44c9-a9cc-5d798f807c93: Correction: the queued repair must identify or establish the canonical manifest generation path before changing a data manifest; direct manifest editing is not an approved implementation path.", "created_at": "2026-08-24" }, + { + "request_id": "9da200a8-0ae0-451b-bef8-d1636b9e3cff", + "action": "add", + "summary": "Caring Contacts: postgres-repository.ts is ~2,080 lines and holds five self-contained clusters", + "created_at": "2026-08-24" + }, { "request_id": "9dd3e06a-ec5c-439e-bdf5-f092d59706cd", "action": "done", "summary": "#XCAX01: Resolved 2026-08-23 for the repository fleet-sweep surfaces only. scripts/clean-worktree.mjs and scripts/worktree-inventory.mjs are now permanently report-only across CLI, exported, and programmatic paths: any remove/apply-shaped option is rejected before adapters; Git is confined to an exact read-only allowlist with optional locks and lazy fetch suppressed, and prune is permitted only as --dry-run -v; inspection and Windows reparse-boundary uncertainty fail closed; liveness is tri-state and absence remains unknown; explicit-root inventory has no removal API and immutable mutation counts stay zero. For worktree tooling, verify:preflight invokes only worktrees:report -- --self-test rather than inspecting the fleet. Synthetic proof: tests/clean-worktree.test.ts plus tests/worktree-inventory.test.ts, 46/46 passed, plus both pure self-tests and git diff --check. No live fleet report, cleanup, prune, or file/ref/object/registration/worktree/directory removal occurred. This closure does not change or claim to remove guard-push.mjs's separate exact temporary-worktree lifecycle. Exact-path fleet cleanup remains deferred under #6GW95D.", "created_at": "2026-08-23" }, + { + "request_id": "a0548b2c-2c80-44f9-9440-5b983e169a13", + "action": "add", + "summary": "Caring Contacts: the closing-message refusal is a guard that cannot fire", + "created_at": "2026-08-24" + }, { "request_id": "a06c9a8d-8c82-47a0-be9f-58f2a2a0273f", "action": "add", @@ -1259,6 +1289,12 @@ "summary": "Caring Contacts Phase 2B — the screens", "created_at": "2026-08-22" }, + { + "request_id": "a4b0610d-ef92-4d1b-bcc8-3da0eb646d5a", + "action": "add", + "summary": "Caring Contacts: four bare foreign keys onto plans/contacts predate the composite same-team rule", + "created_at": "2026-08-24" + }, { "request_id": "a539a411-4648-4f71-8b40-90d49646e8a4", "action": "done", @@ -1331,6 +1367,12 @@ "summary": "Caring Contacts: three unmitigated hazards block any real-patient pilot (safety officer, lived-experience review, Aboriginal health review)", "created_at": "2026-08-18" }, + { + "request_id": "ba655ad0-9934-4885-a959-5efd375a9bf1", + "action": "add", + "summary": "Caring Contacts: Ruling 60's 640-767px overlay-modality band is pinned on one side only", + "created_at": "2026-08-24" + }, { "request_id": "ba668e51-5db9-473c-9104-d424fc8e239e", "action": "cancel", @@ -1403,6 +1445,12 @@ "summary": "#5MMK5R: Documented in docs/testing.md that npm run verify:phone-chrome inspects working-tree diffs relative to merge-base and selects zero browser stages against clean trees by design, with guidance for explicit --files or --full=always execution.", "created_at": "2026-08-23" }, + { + "request_id": "cd97d402-7af1-473e-a2eb-dbac597cd03d", + "action": "add", + "summary": "Caring Contacts: the frozen interaction matrix's 'Recovery action only' wording is ambiguous", + "created_at": "2026-08-24" + }, { "request_id": "d4452409-69bf-4bb6-9ad7-dab654822940", "action": "add", @@ -1421,6 +1469,12 @@ "summary": "Caring Contacts: the desktop session gate renders as a 1392x170 letterbox, not a gate", "created_at": "2026-08-22" }, + { + "request_id": "d7725691-5968-48f1-9912-bca3a5942731", + "action": "add", + "summary": "Caring Contacts: the interface prohibited-language helper has the same 'lead' job-title collision B2 just fixed for messages", + "created_at": "2026-08-24" + }, { "request_id": "d9c78396-1cea-45bd-a4a7-067865151dd3", "action": "add", @@ -1523,6 +1577,12 @@ "summary": "Cancel request 0f9238c1-8add-450c-92d1-917376761248: Resolved on feature branch: verified patient-visible automated reply strings for therapeutic neutrality, emergency escalation, and GSM-7 limit in message-copy.ts with tests.", "created_at": "2026-08-23" }, + { + "request_id": "fa126adf-ade8-4027-a029-d59b4bef8967", + "action": "add", + "summary": "Caring Contacts: the same-team write serialisation is accidental and still unpinned", + "created_at": "2026-08-24" + }, { "request_id": "fa89c68a-3769-483f-9670-900012471425", "action": "done", diff --git a/docs/caring-contacts/PROGRESS-LEDGER.md b/docs/caring-contacts/PROGRESS-LEDGER.md index 7c0d736d49..03834b7b34 100644 --- a/docs/caring-contacts/PROGRESS-LEDGER.md +++ b/docs/caring-contacts/PROGRESS-LEDGER.md @@ -4,11 +4,26 @@ a second copy of the detail: each row points at the file that holds the reasoning. Where this file and a detailed record disagree, **the detailed record wins** — this one is a summary and can go stale. -Last updated at head `05584f9b5`, 2026-08-23. Branch `claude/suicide-contact-mockup-b5aaa0`, pushed. - -> **The branch is SHARED.** On 2026-08-22 a commit (`c3ef20c3f`) authored elsewhere — not from this -> machine's clone — landed on it while a session was mid-task. `git fetch` before every push, and treat -> any full-suite result taken on a moving tree as a hypothesis. See Ruling 66. +Last updated 2026-08-24. + +> **PHASE 2A HAS LANDED ON `main`, AND THE FEATURE BRANCH IS RETIRED.** Verified 2026-08-24: the whole +> of Phase 2A was squash-merged as `e4cbe8d3a` — "Claude/suicide contact mockup b5aaa0 (#2279)", +> 2026-08-23 — and every `docs/caring-contacts/**`, `src/lib/caring-contacts/**`, +> `src/components/caring-contacts/**`, `caring-contacts/supabase/migrations/**` and +> `tests/ui-caring-contacts-workspace.spec.ts` path on `main` now matches the old branch tip +> `cf03f99a4`, except where later `main` work is NEWER (the design-system token consolidation touched +> three component files). **`main` is the source of truth. Do the closing work on a fresh worktree off +> `origin/main`, not on `claude/suicide-contact-mockup-b5aaa0`.** No local remote-tracking ref for that +> branch remains, which is consistent with the PR branch having been deleted after the merge — not +> verified against GitHub, which needs approval. + +> **Sections below that instruct you to work on, push, or fetch the feature branch are superseded by +> the paragraph above.** They are kept because their _reasoning_ about durability and about shared-tree +> measurement is still correct and still paid for; only the branch name is stale. In particular the +> historical note that the branch was SHARED — commit `c3ef20c3f` landed on it on 2026-08-22 from a +> clone not on this machine, mid-task — is why any full-suite result taken on a moving tree is a +> hypothesis and not a result. That lesson generalises to `main`, which many sessions touch. See +> Ruling 66. --- @@ -39,52 +54,62 @@ experience and clinical sign-off are required before any real use. | 1 | Phase 1 + early 2A | Built the sealed domain and database. 13 owner-behalf decisions. Phase 1 gate passed. | `D:\Repos\caring-contacts-handoff-2026-08-20\` | | 2 | Phase 2A controller | Plan written; Tasks 1-10 and 11a built and reviewed. Rulings 1-31. Died on an account limit mid-fix-round-2. | same bundle | | 3 | Phase 2A recovery | Verified the abandoned commit, re-reviewed it, Rulings 32-34, survived a worktree deletion, rebuilt and re-proved | `D:\Repos\caring-contacts-handoff-2026-08-21\` | +| 4 | Phase 2A completion | Task 11b, Tasks 12-19, the final whole-branch review, Rulings 35-66, the copy review, the condensed bar | build record, Session 4 onward | +| 5 | Phase 2A closing | Found the phase already merged to `main`; browser gate green; mutation proofs; issues sweep; 2B planning begun | build record, Session 5 | ## 4. Task status — Phase 2A (19 tasks, 5 groups) -| Task | What it is | State | -| ---- | ------------------------------------------------ | ------------------------------------------------------- | -| 1 | Patient-visible copy into the sealed domain | Complete, reviewed clean | -| 2 | Roles and actions | Complete, reviewed clean | -| 3 | Service safety stop | Complete, 1 fix round | -| 4 | Pathway versions and dual approval | Complete, 1 fix round | -| 5 | Referrals | Complete (batched 5-7) | -| 6 | Plan ownership, reassignment, coverage | Complete (batched 5-7) | -| 7 | Moving a contact / changing its date | Complete (batched 5-7) | -| 8 | Auditing a view, not only a write | Complete, 1 fix round (a CRITICAL finding) | -| 9 | Notification preferences and training | Complete, reviewed clean | -| — | **Checkpoint 1** | **PASSED** — 7,604 tests, typecheck and lint green | -| 10 | Storage contract + in-memory store (~21 methods) | Complete, 1 fix round (7 findings), 101 tests | -| 11a | Migration 0003 + row-level security | Complete, **3 fix rounds**, 96 database tests | -| 11b | Shared-contract move + 22 Postgres methods | Complete, 2 fix rounds, review clean — typecheck GREEN | -| — | **Checkpoint 2** | **PASSED** — see the build record | -| 12 | Database config that can never hit Clinical KB | Complete, batched with 13, 1 fix round, review clean | -| 13 | Demo role switcher | Complete, batched with 12, 1 fix round, review clean | -| 14 | Route handlers that audit every view | Complete, 2 fix rounds, review clean | -| 15 | Route group, four width states, inbound link | Complete, 1 fix round, review clean | -| 16 | Service-state banner | Complete, 1 fix round, review clean | -| 17 | The frozen 24-row overlay definition table | Complete, 1 fix round, review clean — 0 Important | -| 18 | One renderer, twenty-four overlays | Complete, 2 fix rounds, review clean | -| 19 | Browser proof at six widths + plan closing steps | Complete, 1 fix round, review clean | -| — | Final whole-branch review | **Done** — three parallel reviewers, distinct lenses | -| — | Post-review fixes (Rulings 60-65) | Complete — incl. a CRITICAL patient-data finding | -| — | Condensed pinned safety bar (owner-requested) | Built, 1 fix round; **2 mutation proofs still unrun** | -| — | Copy review document for the owner | **Delivered** — `copy-review.md`, 7 items need his call | +| Task | What it is | State | +| ---- | ------------------------------------------------ | ------------------------------------------------------ | +| 1 | Patient-visible copy into the sealed domain | Complete, reviewed clean | +| 2 | Roles and actions | Complete, reviewed clean | +| 3 | Service safety stop | Complete, 1 fix round | +| 4 | Pathway versions and dual approval | Complete, 1 fix round | +| 5 | Referrals | Complete (batched 5-7) | +| 6 | Plan ownership, reassignment, coverage | Complete (batched 5-7) | +| 7 | Moving a contact / changing its date | Complete (batched 5-7) | +| 8 | Auditing a view, not only a write | Complete, 1 fix round (a CRITICAL finding) | +| 9 | Notification preferences and training | Complete, reviewed clean | +| — | **Checkpoint 1** | **PASSED** — 7,604 tests, typecheck and lint green | +| 10 | Storage contract + in-memory store (~21 methods) | Complete, 1 fix round (7 findings), 101 tests | +| 11a | Migration 0003 + row-level security | Complete, **3 fix rounds**, 96 database tests | +| 11b | Shared-contract move + 22 Postgres methods | Complete, 2 fix rounds, review clean — typecheck GREEN | +| — | **Checkpoint 2** | **PASSED** — see the build record | +| 12 | Database config that can never hit Clinical KB | Complete, batched with 13, 1 fix round, review clean | +| 13 | Demo role switcher | Complete, batched with 12, 1 fix round, review clean | +| 14 | Route handlers that audit every view | Complete, 2 fix rounds, review clean | +| 15 | Route group, four width states, inbound link | Complete, 1 fix round, review clean | +| 16 | Service-state banner | Complete, 1 fix round, review clean | +| 17 | The frozen 24-row overlay definition table | Complete, 1 fix round, review clean — 0 Important | +| 18 | One renderer, twenty-four overlays | Complete, 2 fix rounds, review clean | +| 19 | Browser proof at six widths + plan closing steps | Complete, 1 fix round, review clean | +| — | Final whole-branch review | **Done** — three parallel reviewers, distinct lenses | +| — | Post-review fixes (Rulings 60-65) | Complete — incl. a CRITICAL patient-data finding | +| — | Condensed pinned safety bar (owner-requested) | Built, 1 fix round; browser gate green 2026-08-24 | +| — | Copy review document for the owner | **Delivered** — `copy-review.md`; recommendations now | +| | | tracked in `copy-decisions-recommended.md`. 13 open, | +| | | 9 clinical/policy + 4 engineering (the "7" was an | +| | | undercount and is corrected) | +| — | Deferred-findings `/issues` sweep | **Done 2026-08-24** — 7 request files queued in | +| | | `docs/outstanding-issues-inbox/`, awaiting reconcile | +| — | Phase 2B plan | **In progress** — no plan existed; being written | ## 5. Verification evidence, as recorded -| Gate | Result | -| ---------------------------- | --------------------------------------------------------------------------------------- | -| Phase 1 gate | 7,531 tests / 682 files; tsc silent; lint 0 warnings; 55 database tests | -| Phase 2A Checkpoint 1 | 7,604 tests passed; typecheck and lint green | -| Task 10 | 101 tests (up from 84) | -| Task 11a, through 3 rounds | 55 → 71 → 87 → 93 → **96 passed** | -| Task 11b, through 2 rounds | 96 → 159 → 162 → **163 passed** database; full suite **7671 passed, 0 failed** | -| Full suite, 2026-08-23 | `Test Files 2 failed \| 701 passed \| 2 skipped (705)`; `Tests 3 failed \| 7841 passed` | -| Current known-red (expected) | **NONE in this work.** Both 2026-08-23 failures were artefacts, proven so: the | -| | caring-contacts one passed 22/22 on re-run of the same commit (a concurrent agent | -| | held a source file mid-edit), and the other was a 120 s timeout under machine load. | -| Browser gate | **Was fully red — 32/32 — for a reason outside this work**, then unblocked. See §5b. | +| Gate | Result | +| ----------------------------- | ---------------------------------------------------------------------------------------- | +| Phase 1 gate | 7,531 tests / 682 files; tsc silent; lint 0 warnings; 55 database tests | +| Phase 2A Checkpoint 1 | 7,604 tests passed; typecheck and lint green | +| Task 10 | 101 tests (up from 84) | +| Task 11a, through 3 rounds | 55 → 71 → 87 → 93 → **96 passed** | +| Task 11b, through 2 rounds | 96 → 159 → 162 → **163 passed** database; full suite **7671 passed, 0 failed** | +| Full suite, 2026-08-23 | `Test Files 2 failed \| 701 passed \| 2 skipped (705)`; `Tests 3 failed \| 7841 passed` | +| Current known-red (expected) | **NONE in this work.** Both 2026-08-23 failures were artefacts, proven so: the | +| | caring-contacts one passed 22/22 on re-run of the same commit (a concurrent agent | +| | held a source file mid-edit), and the other was a 120 s timeout under machine load. | +| Browser gate | **GREEN 2026-08-24 on `main`: `32 passed (55.5s)`, exit 0**, no ECONNRESET in a 341-line | +| | log. The 2026-08-23 residual failure at 1440px was LOAD, not a defect — see §5b. | +| Condensed-bar mutation proofs | Run 2026-08-24 against `main`. See §5c. | ### 5b. The production lock, and why the browser gate went red @@ -115,6 +140,23 @@ a re-sent undelivered contact; a cross-team row leak; a duplicate active plan; a committed cross-team write; and a silently rewritable safety incident. **Four tests have been found unable to fail and rewritten.** +### 5c. The condensed bar's two mutation proofs — both run 2026-08-24, both discriminate + +| Mutation | Result | What it proves | +| --------------------------------------------------------- | ------------------------ | ------------------------------------------------ | +| Pin: `top-full` -> `top-0` | 13 failed / 19 passed | The 1440px pin assertion is REACHED and fails at | +| | | line 877 on `barBox.top` 64 -> 0, with the two | +| | | preceding assertions passing first. | +| Dark colours: danger tokens -> fixed light-theme literals | **1 failed** / 31 passed | Only the dark-scheme comparison reddens, at line | +| | | 931, naming the injected literal. | + +**And the trap that nearly produced a false proof.** The first attempt at the colour mutation used an +anchor matching TWO elements. A uniqueness assertion refused the edit — but the script ran the whole +browser gate anyway, on an unmutated tree, and reported `32 passed`, exit 0. Read without the abort +line, that is a real, green, strongest-looking gate run supporting exactly the opposite conclusion. +**A mutation proof has two results, not one: prove the mutation is in the tree before believing the +gate.** Full account in the build record. + ## 6. Decisions taken on the owner's behalf **Phase 1 — 13 numbered decisions**, each with why and cost-if-wrong: `phase-1-handoff.md` §"Decisions". diff --git a/docs/caring-contacts/copy-decisions-recommended.md b/docs/caring-contacts/copy-decisions-recommended.md new file mode 100644 index 0000000000..1ae41be98b --- /dev/null +++ b/docs/caring-contacts/copy-decisions-recommended.md @@ -0,0 +1,259 @@ +# Caring Contacts — the copy decisions, with a recommendation for each + +> ## OWNER APPROVED ALL THIRTEEN, 2026-08-24 +> +> The owner read the thirteen items in chat and answered "go ahead with your recommendations." +> That is the decision of record for every item below. Two qualifications, because approval of a +> recommendation is not the same as the recommendation being executable today: +> +> - **A9 (adding Lifeline `13 11 14`) CANNOT be executed yet, by its own terms.** The recommendation +> was to add Lifeline _and drop the `Fictional Support Line` line once a real crisis number is +> chosen_ — because the message is roughly nine characters from its two-segment maximum, so nothing +> can be added until something comes out. No real crisis number exists. **A9 is APPROVED IN +> PRINCIPLE and BLOCKED ON a real crisis number.** Do not implement it by removing some other +> sentence chosen by an implementer; the owner was explicitly asked to name what goes, and the +> answer that arrived approves the shape, not a specific removal. Re-ask when a real number exists. +> - **A4 (the closing message) is approved as a DEFERRAL, not as text.** The approved recommendation +> is that the wording be written in a later phase with a lived-experience representative, and that +> the system refuse loudly in the meantime. The refusal half is buildable now; the wording is not, +> and no implementer should draft it. +> +> Everything else is approved for implementation. Patient-visible strings are therefore NO LONGER +> FROZEN — but each change must cite the item number it implements, and the sealed domain's +> `message-copy` module remains the single place they live. + +**Written 2026-08-24, approved the same day, and NOT YET IMPLEMENTED.** The freeze that used to sit +here is lifted — see the banner above. Approval and implementation are different things, and the +status table at the foot of this file is the one that says which items are built. + +## Why this file exists + +`copy-review.md` sets out every concern but deliberately offers no recommendations — it is a mark-up +document, and guessing at intent was explicitly ruled out when it was written. Recommendations were +given to the owner on 2026-08-23 **in conversation only, and were never written to a tracked file**, +so they did not survive the session that made them. This file closes that gap. It is the +recommendation half; `copy-review.md` stays the evidence half and wins on any question of what the +current wording actually says. + +**Two counts, and the disagreement is worth stating rather than smoothing over.** `PROGRESS-LEDGER.md` +and the continuation prompt both say **seven** items need the owner's call. `copy-review.md` Part 7 +lists **ten** concerns and Part 5 lists **three** more — thirteen in total. Reading all thirteen, +**nine are genuinely clinical or policy decisions only the owner can take** and four are engineering +work needing no clinical input. The "seven" does not reconcile against the document and appears to be +an undercount carried forward from an earlier draft, so this file uses the thirteen and marks which +is which. + +**"Cost if wrong" means the cost of having followed my recommendation and been mistaken** — that is +the thing worth weighing, not the recommendation itself. + +--- + +## A. The nine that need the owner's clinical or policy decision + +### A1 — A patient is given a crisis number labelled "Fictional Support Line" + +Current wording, in every message: `Fictional Support Line: +61 491 570 158`. + +**Recommend:** keep the fictional label, and add a machine check that refuses to send any message +whose crisis-line text still contains the word "Fictional". Not a comment and not a note in a +runbook — a test that goes red, in the same place the two-segment length limit is already enforced. + +**Why:** the label is correct today and is the honest thing for a prototype to say. The risk is not +the wording, it is that nothing forces it to be replaced. The programme already proves this pattern +works: the required-fragment checker rejects a message missing `In an emergency call 000`, so the +same mechanism can reject one that still says `Fictional`. + +**Cost if wrong:** near zero. A check that fires on the day a real number arrives is a two-line edit +to remove. + +### A2 — "Your message has not been seen by anyone and has not been kept" may not be true + +**Recommend:** narrow it to what this system can actually promise, and say who is not reading rather +than what is not stored — something in the shape of _"No one at Example Aftercare Team reads this +number."_ Do not restore any claim about storage until a telephony provider is chosen and its +retention terms have been read. + +**Why:** this is the highest-risk sentence in the programme. It is a firm factual claim about data +handling, made to a person in distress, about a system that has no telephony provider yet — so +nobody can currently know whether it is true. The same reasoning already forced one narrowing here +on 2026-08-19, when "Replies are not received, stored, analysed or monitored" became untrue the +moment the number was made able to receive. This is the identical mistake one step further down. + +**Cost if wrong:** a patient is told slightly less than the eventual truth. That is the safe +direction of error. The other direction is a false promise about confidentiality to a suicidal +person, which no later correction reaches. + +### A3 — "No one reads replies to this number" says what does not happen, not what does + +**Recommend:** say both, in that order — nobody reads it, and something does come back +automatically — so the patient knows the auto-reply is not a person. + +**Why:** a patient who reads "no one reads replies" and then receives a message may reasonably +conclude somebody did read it after all. That is worse than the original ambiguity, because it +teaches them the stated boundary is unreliable at exactly the moment the boundary matters. + +**Cost if wrong:** characters. Message A is already at its two-segment ceiling of 252 — about nine +characters from rejection — so this may not fit in Message A and may have to live only in Message B. +Decide it as a pair with A9. + +### A4 — The required closing message does not exist + +The checker requires a final message to contain `This is the final message in this programme`. No +final message has been written, so a plan reaching its end today sends nothing. + +**Recommend:** treat the wording as a Phase 2B deliverable and write it with a lived-experience +representative, not before. In the meantime make the gap loud rather than silent: a plan whose last +contact has no message body should refuse and raise, not pass quietly. + +**Why:** the end of a caring-contacts series is a clinically loaded moment — it is the point the +patient loses the contact — and drafting that text is not a job for a coding session. But a silent +no-op is the worst available behaviour, because it looks like success. + +**Cost if wrong:** the refusal fires during a demo and needs explaining. Cheap. A silently missing +final message means a patient's series stops with no closure and nothing recorded that it happened. + +### A5 — A patient is never told when sending stops + +During a service-wide stop, a pause, or a contact-changed block, clinicians are told in detail and +the patient is told nothing. + +**Recommend:** keep it that way for a service-wide safety stop, and record it as a deliberate +decision rather than leave it an omission. Revisit only for a **withdrawal**, where the patient +asked. + +**Why:** a service-wide stop is triggered by a serious incident affecting somebody else. A message +saying "your messages have stopped" to a person who did not ask, at a moment nobody can explain to +them, invites the reading that they did something wrong or that something has happened to their +clinician. Silence is the more conservative option and it is reversible — nothing prevents adding a +notice later. + +**Cost if wrong:** a patient notices the messages stopped and does not know why. That is real, and it +is exactly why this needs the owner's decision rather than a controller ruling. + +### A6 — "Contacts that fall inside the pause are skipped for good" + +**Recommend:** confirm the behaviour is intended, and change the clinician-facing wording to state +the consequence as a number rather than a fact — for example _"3 contacts fall inside this pause and +will not be sent later."_ + +**Why:** in caring contacts the schedule is the intervention. Silently and permanently removing +contacts from it is a clinical act, and "skipped for good" is easy to read past when you are pausing +for an ordinary administrative reason. + +**Cost if wrong:** none — showing the count is strictly more information. Whether pausing _should_ +drop contacts is the owner's question, and this recommendation does not settle it. + +### A7 — Withdrawal is immediate, irreversible, and needs nobody else's agreement + +Restarting the service after a stop needs three approvals from three people. Withdrawing a patient +needs none and cannot be undone. + +**Recommend:** keep the asymmetry and record why. Add a confirmation step that names what is lost. +Do not add an approver. + +**Why:** the asymmetry is defensible and I think correct, because the two actions are not comparable. +A restart resumes sending to everybody after an incident, so the risk is in acting too readily. A +withdrawal is a patient exercising a choice about contact they receive, and putting a second +clinician in front of that turns a patient's decision into a request. Irreversibility is the part +worth softening, and a confirmation naming the consequence does that without an approval gate. + +**Cost if wrong:** a withdrawal made in error cannot be reversed and the patient must be re-enrolled +from the start. The confirmation step is what makes that acceptable, so it should not be dropped +from this recommendation. + +### A8 — "All three attempts in the original window are finished and there is no later retry" + +A patient whose contact fails receives nothing that day and nothing later. + +**Recommend:** confirm as clinical policy, and surface it where a clinician will actually meet it — +on the patient's plan, not only inside a button panel. + +**Why:** no-later-retry is a reasonable design. A caring contact arriving days late is a different +intervention, and stacking retries turns a non-demanding contact into a demanding one. But it is a +clinical policy currently stated only in a place a clinician sees once something has already gone +wrong. If a patient is missing contacts, that belongs on their plan. + +**Cost if wrong:** a clinician assumes a failed contact will be retried and does not follow up +manually. That is a real gap in a suicide-prevention programme, which is why the visibility half +matters more than the policy half. + +### A9 — "000" is the only emergency direction given + +No Lifeline, no 13YARN, no after-hours mental health line. + +**Recommend:** decide this as a pair with A3, because they compete for the same nine spare +characters. If only one thing can be added: **add Lifeline `13 11 14`, and drop the +`Fictional Support Line` line once a real crisis number is chosen** — the fictional line is already +occupying the space a real one would need. + +**Why:** 000 alone directs a person in distress to an emergency-services response. That is the right +answer for an emergency in progress and the wrong answer for someone distressed and not in immediate +danger, and Lifeline is the standard Australian answer to that second state. 13YARN matters for +Aboriginal and Torres Strait Islander patients, and the schema already carries cultural identity, so +it could be conditional rather than universal — but that is a Phase 2B capability, not a wording +change. + +**Cost if wrong:** the length ceiling is hard at about nine characters, so anything added means +something removed, and removing the wrong thing is worse than adding nothing. This is the one item +where I would not act on my own recommendation without the owner's explicit choice of what goes. + +--- + +## B. The four that need no clinical input + +None of these changes a patient-visible string. All can proceed as soon as the owner says go. + +### B1 — Two panels describe content they do not show + +"Preview the message the patient would see" shows no message; "Plan activation recorded" records +nothing. **Recommend:** change the words now to match what exists, and let Phase 2B change them back +when the content lands. A true statement about a smaller product beats a false one about a larger +one. + +### B2 — "lead" appears in visible wording in its ordinary English sense + +"the incident lead", "the clinical programme lead". **Recommend:** narrow the prohibited-vocabulary +ban to the commercial sense rather than exempting the sentence — an exemption would have to be +re-argued every time the sentence changes. These are people's job titles. + +### B3 — The prohibited-vocabulary ban is not enforced on screen wording + +It runs on outgoing messages and the 24 frozen overlay rows only. **Recommend:** extend it to a +static scan over interface strings. Captured in the issues inbox on 2026-08-24 as a P2 issue. + +### B4 — One banned word is rendered, in the frozen design-scratch prototype + +"Delivered is a transport receipt only and never means the message was read or the patient is safe." +**Recommend:** leave it. Those screens 404 in production and Phase 2B replaces them, and the sentence +uses "safe" in order to deny it, which is the defensible use. Decide it when the wording is carried +across, and let B3's scan flag it at that point. + +--- + +## What happens next + +1. The owner marks this file up, or answers item by item. +2. Only then does any patient-visible string change, and each change carries its decision reference. +3. A1's machine check, B1, B2 and B3 can start as soon as he says go; none touches patient wording. + +--- + +## Implementation status, 2026-08-24 + +Approval is not implementation. Nothing below has been built yet. + +| Item | Approved outcome | Where it gets built | +| ---- | ------------------------------------------------------------------- | ----------------------------------------- | +| A1 | Machine check refusing any message containing "Fictional" | Small change now, beside the length check | +| A2 | Narrow the storage promise to who is not reading | Small change now, `message-copy` | +| A3 | Say nobody reads it AND something automatic comes back | Small change now, `message-copy` | +| A4 | Refuse loudly when a final message is missing; wording deferred | Refusal now; wording a later phase | +| A5 | Patient not told during a service-wide stop — DELIBERATE | Decision only; nothing to build | +| A6 | Confirm intended; show the count of contacts a pause discards | Phase 2B, Group 1 | +| A7 | Keep the asymmetry; add a confirmation naming what is lost | Phase 2B, Group 1 | +| A8 | Confirm no-later-retry; surface it on the plan, not only in a panel | Phase 2B, Groups 1-2 | +| A9 | Add Lifeline — **BLOCKED**, needs a real crisis number first | Re-ask when one exists | +| B1 | Make the two panels describe what they actually show | Phase 2B, Group 3 | +| B2 | Narrow the "lead" ban to the commercial sense | Small change now | +| B3 | Extend the prohibited-word scan to interface strings | Small change now (issues inbox P2) | +| B4 | Leave the design-scratch sentence alone | No action | diff --git a/docs/caring-contacts/phase-2a-build-record.md b/docs/caring-contacts/phase-2a-build-record.md index cb2bdc24a2..4e81d29a9b 100644 --- a/docs/caring-contacts/phase-2a-build-record.md +++ b/docs/caring-contacts/phase-2a-build-record.md @@ -2666,3 +2666,200 @@ failure as a regression, run the other way. **Also still unrun, and still recorded as unrun:** the condensed bar's 1440px pin mutation and its dark-mode colour mutation. Both were blocked on the browser gate; the gate is now available, so the next session can run them. + +# Session 5 — 2026-08-24 — closing work, and the branch turned out to be merged + +Working copy `D:\Repos\Database\.claude\worktrees\browser-test-gate-handoff-d5c1db`, on `main` at +`6299857df`. Dependencies came from `node scripts/setup-codex-worktree.mjs`, which found every +`package-lock.json` on this machine byte-identical and reused `D:\Repos\Database`'s `node_modules` in +seconds rather than the 15-58 minutes `npm ci` costs here. Worth knowing before anyone budgets an hour +for a fresh worktree again. + +Ruling: [67] This session appends to THIS tracked file rather than opening a `.superpowers/sdd/` +ledger, even though it is running the subagent-driven-development method which asks for one. +— Why: the programme already ruled that the SDD workspace is a GENERATED MIRROR and never a source, +after the original git-ignored workspace was destroyed on 2026-08-21 and took the only copy of a +session ledger with it. A second ledger in git-ignored scratch would recreate exactly that loss, and +two ledgers disagreeing is worse than one that is occasionally terse. — Cost if wrong: the SDD +scripts' `progress.md` conventions are not used, so a future controller resuming by those conventions +finds no ledger where the skill says to look. That is why this ruling is written here, where that +controller is told to read first. + +## The finding that reframes everything below: Phase 2A ALREADY MERGED + +The handoff, the ledger and the continuation prompt all named branch `claude/suicide-contact-mockup-b5aaa0` +as the source of truth and told the next session to build a worktree from it. **That is stale.** Verified +before any work was done: + +- `e4cbe8d3a` on `main` is "Claude/suicide contact mockup b5aaa0 (#2279)", dated 2026-08-23 — a squash + merge of the whole phase. `main` has advanced 18 commits since. +- Every caring-contacts path on `main` matches the old branch tip `cf03f99a4`. `git diff origin/main +claude/suicide-contact-mockup-b5aaa0` over `src/lib/caring-contacts`, `caring-contacts/` and + `tests/ui-caring-contacts-workspace.spec.ts` is EMPTY; `docs/caring-contacts/` differs by two lines in + one archive file; `src/components/caring-contacts/` differs only where **`main` is newer** — the + design-system consolidation replaced a shadow literal with a token, swapped a local `SectionHeading` + for the shared one, and added a `focusTimerRef` teardown the branch never had. +- No local remote-tracking ref for that branch survives, which is what a deleted PR branch looks like + after a prune. Not verified against GitHub — that needs approval and buys nothing here. + +So the branch is retired and `main` is the source of truth. The whole "push after every task / +`SKIP_STATIC_GUARD=1`" apparatus in the older records was correct for its moment and is now noise. The +records have been corrected in place rather than deleted, because the _reasoning_ about durability and +about measuring a moving tree still holds — and holds harder on `main`, which far more sessions touch. + +**The general lesson, and it is the same shape as the idempotency-table one:** a handoff document +describes where work _was_, and no part of it updates when the work moves. Four documents agreed with +each other and all four were wrong together, because they were written in one session and copied from +one another. **Agreement between records that share an ancestor is not corroboration.** Check the claim +against git, not against the other records. + +## Closing item 1 — the browser gate is GREEN, and the residual failure was load + +Re-run against `main` content, via the repository runner, whole log kept to a file and not tailed: + +``` +Running 32 tests using 1 worker + ok 30 [chromium] > ui-caring-contacts-workspace.spec.ts:822:9 > caring-contacts service stop, stated on + every screen > pins the condensed bar under the header once the banner has gone at 1440px (898ms) + 32 passed (55.5s) +EXIT=0 +``` + +341-line log, no `ECONNRESET` anywhere in it. **The exact test that failed on 2026-08-23 ran and passed.** +The disposition recorded then — "unresolved, re-run it on a quiet machine" — therefore resolves to +**load, not a defect**, and the condensed bar's fix round is closed on that count. Note also that the +count is **32**, not the 33 the continuation prompt predicted; the prompt has been corrected. + +Two corrections to how that failure was read, both worth more than the incident: + +- **The machine was NOT quiet for this run** — 73 `node` processes and 23 Claude processes were live. A + pass under load is _stronger_ evidence than a pass on a quiet machine, not weaker, because load is the + very hypothesis being tested. Waiting for quiet would have bought less and cost hours. +- **`:822` was never the failing line.** Playwright reports a failure at the test's DECLARATION line, and + 822 is the `test(...)` line. The `apiRequestContext.post: read ECONNRESET` came from + `arrangeServiceStop`'s setup POST at line **672**, before a single pin assertion executed. The previous + session's honest worry — "it is precisely the assertion the fix round strengthened, so the one test most + likely to be genuinely wrong is the one that failed" — could not have been true as stated: a dropped + HTTP connection during arrange cannot be caused by wrong geometry in an assertion that never ran. That + did not make the re-run unnecessary, and running it was still right — the difference between narrowing + a hypothesis and confirming one. But **read which line the runner is actually naming before inferring + what a failure means.** + +## Closing item 2 — the condensed bar's mutation proofs + +### Mutation A — the 1440px pin assertion IS reachable and DOES discriminate + +`top-full` -> `top-0` on the condensed bar's className, one line, nothing else touched. The bar then +sits at the header's top edge instead of hanging off its bottom, which is precisely the "buried behind +the header" defect the pin exists to prevent. + +Result: **13 failed, 19 passed** (against 32/0 unmutated). The failure at 1440px, verbatim: + +``` +12) ui-caring-contacts-workspace.spec.ts:822:9 > pins the condensed bar under the header ... at 1440px + Error: the condensed bar is behind the header at 1440px + expect(received).toBeGreaterThanOrEqual(expected) + Expected: >= 64 + Received: 0 + > 877 | expect(geometry.barBox!.top, `the condensed bar is behind the header at ${width}px`) +``` + +Three things make this a proof rather than a red light: + +- **It is the pin assertion itself that failed** — line 877 — not an earlier one standing in for it. +- **The two assertions before it passed first**: the round-1 guard at 869 (`the banner is still on +screen ... nothing about the handover is being measured`) and `the condensed bar did not appear` at 874. So the pin is REACHED, which is exactly what round 1's degenerate version was not. +- **The mutation moved a value the assertion reads**: `barBox.top` went 64 -> 0. A mutation that leaves + every asserted value unchanged proves nothing however red the suite goes, and three proposed proofs + on this branch already failed that test. + +The other twelve failures are honest collateral, not scope creep: the six `keeps the stop stated exactly +once at every scroll position` cases and the forced-colours case all read the bar's on-screen position, +and an unpinned bar changes it. Worth stating explicitly so a later reader does not treat thirteen +failures from a one-line change as evidence the mutation was too broad. + +### THE TRAP, and it is the most valuable thing in this section + +The first attempt at mutation B was written to swap the danger tokens for theme-invariant literals. Its +anchor, `bg-[color:var(--danger-bg)]`, appears **twice** in the file — once on the full banner and once +on the condensed bar. A uniqueness assertion caught it and refused to edit: + +``` +AssertionError: mutation B anchor not unique for bg-[color:var(--danger-bg)]: 2 +``` + +But the surrounding script was not written to stop there, and it **ran the whole browser gate anyway, +on a completely unmutated tree**. That run reported: + +``` +32 passed (52.7s) +EXIT=0 +``` + +**Read that the way it would have been read without the abort line: a mutation applied, the suite still +green, therefore the dark-mode assertion cannot fail and the test is worthless.** The conclusion would +have been exactly backwards, it would have been recorded against a test that is in fact fine, and the +"evidence" would have been a genuine 32-passed gate run — the strongest-looking kind. + +This is the same family as the two traps already recorded here — `npx playwright test` exiting 0 having +run nothing, and the EPERM lock failure that produces no summary line — but it is worse than either, +because there is no missing output to notice. The gate really ran, really passed, and really measured +the unmutated code. + +**The rule, and it is now a standing one for this programme: a mutation proof has TWO results, not one. +Prove the mutation is in the tree before believing anything the gate says about it.** Concretely: assert +the anchor is unique, assert the replacement is present after writing, print `git diff` of the mutated +file into the same log as the gate output, and refuse to launch the gate at all if any of that fails. +The corrected script does all four, which is why its log opens with the applied-mutation line and the +diff before a single test runs. + +A quieter lesson sits underneath it. The uniqueness assertion was cheap insurance added almost as an +afterthought, and it is the only reason this was caught. **Guard the mutation, not just the assertion** +— the thing being manipulated is as capable of silent failure as the thing being tested. + +### Mutation B — the dark-mode colour assertion IS falsifiable, and reddens ONLY itself + +The corrected mutation swaps the condensed bar's `--danger-bg` / `--danger-text` for fixed literals +carrying the light theme's own values, so the bar renders identically in both schemes. This is exactly +the mutation the test's own comment names: "swapping `--danger-text` for a token whose value is +identical in both themes reddens this and would have sailed through the old assertion." + +No theme-invariant token exists to swap in — every colour token in `globals.css` that is defined before +the dark block is also redefined inside it. That is a good property of the design system and it is why +the mutation uses literals; recorded so nobody later reads the literals as sloppiness. + +Anchoring had to be precise, because `bg-[color:var(--danger-bg)]` matches the full banner too. The +unique anchors are the longer runs `bg-[color:var(--danger-bg)] px-4 py-2 text-sm font-semibold` and +`text-sm font-semibold text-[color:var(--danger-text)] data-[full-banner-out-of-view=true]:flex`, each +appearing exactly once, and the script asserts that count before writing. + +Result: **1 failed, 31 passed.** The single failure, verbatim: + +``` +31) ui-caring-contacts-workspace.spec.ts:913:7 > re-resolves the condensed bar's own colours in dark + Error: the condensed bar's ink did not change in dark + expect(received).not.toBe(expected) + Expected: not "rgb(163, 25, 15)" + > 931 | expect(dark.colour, "the condensed bar's ink did not change in dark").not.toBe(light.colour) +``` + +Everything a proof needs is in those four lines: + +- **Exactly one test reddened**, and it is the intended one. Mutation A's thirteen failures were honest + collateral from moving the bar; this one changes only colour, and only the colour assertion notices. + A mutation whose blast radius matches its intent is itself evidence the assertion is measuring what + it claims. +- **The failing assertion is the scheme-comparison at line 931**, not the `display !== "none"` guard at + 924 — which passed first, so the assertion is reached. +- **`Expected: not "rgb(163, 25, 15)"` is the literal the mutation injected.** The failure is + attributable to the mutation by value, not merely by timing. + +The `surface` assertion on the next line never ran, because `expect` throws on the first failure. That +is not a gap: the test is proven able to fail, which is the claim. Proving the second assertion +independently would need its own mutation, and nothing depends on it that the first does not already +establish. + +### Both closing proofs are now run, and the earlier "unrun" record is discharged + +The final review recorded these two as **unrun, not passed**, and blocked on the browser gate. The gate +is green, both are run, and both discriminate. The condensed pinned safety bar's fix round is closed. diff --git a/docs/caring-contacts/phase-2a-continuation-prompt.md b/docs/caring-contacts/phase-2a-continuation-prompt.md index 6263695249..e54f4a1f6e 100644 --- a/docs/caring-contacts/phase-2a-continuation-prompt.md +++ b/docs/caring-contacts/phase-2a-continuation-prompt.md @@ -14,20 +14,31 @@ writing anything. ═══ WHERE THE WORK IS ═══ Repository: D:\Repos\Database (remote: github.com/BigSimmo/Database) -Branch: claude/suicide-contact-mockup-b5aaa0 — PUSHED. origin holds it. That is the source of truth. - -THE BRANCH IS SHARED. It is not yours alone. On 2026-08-22 commit c3ef20c3f landed on it from a clone -that is NOT on this machine (this repo's branch reflog never held it), while a session was mid-task. -So: `git fetch` BEFORE every push, not after a rejection; never force-push it; and treat any full-suite -result taken while another agent is editing as a hypothesis, not a result. A phantom failure and a -phantom pass are equally possible — see "the phantom failures" in the build record. - -FIRST ACTION — make yourself a working copy. Do NOT assume one already exists: +Branch: main. PHASE 2A HAS ALREADY MERGED. Verified 2026-08-24: the whole phase went in as + e4cbe8d3a, "Claude/suicide contact mockup b5aaa0 (#2279)", on 2026-08-23. Every + caring-contacts path on main matches the old branch tip cf03f99a4 except where later + main work is newer. The old feature branch claude/suicide-contact-mockup-b5aaa0 is + RETIRED — do not work on it, do not push it, do not resurrect it. + +MEASUREMENT DISCIPLINE CARRIES OVER, AND MATTERS MORE ON main. The retired branch was shared: on +2026-08-22 commit c3ef20c3f landed on it from a clone not on this machine, mid-task, and invented both +a phantom failure and a phantom green. main is touched by many more sessions than that branch ever was. +So: record the exact commit any full-suite or browser result was taken against, re-run a single file +alone against a named commit before believing a failure, and treat green under concurrency as worth no +more than red under concurrency. See "the phantom failures" in the build record. + +FIRST ACTION — make yourself a working copy off main. Do NOT assume one already exists: cd D:\Repos\Database git fetch origin - git worktree add D:\Worktrees\Database\ claude/suicide-contact-mockup-b5aaa0 -Then confirm you are where you think you are: `git rev-parse --abbrev-ref HEAD` must print -claude/suicide-contact-mockup-b5aaa0. + git worktree add D:\Worktrees\Database\ -b claude/ origin/main +Then confirm you are where you think you are: `git rev-parse --abbrev-ref HEAD` must print your new +branch, and `git log --oneline -1` must show a commit at or after e4cbe8d3a. + +DEPENDENCIES ARE FREE IF A COMPLETE INSTALL EXISTS. `npm ci` is 15-58 minutes here, but every +package-lock.json on this machine was byte-identical on 2026-08-24, so +`node scripts/setup-codex-worktree.mjs` reused D:\Repos\Database's node_modules in seconds and +reported "PASS: worktree dependencies match package-lock.json." Run that FIRST, before reaching for +npm ci. Check with `--dry-run` if you want to see what it would do. WORKING DIRECTORIES ON THIS MACHINE DO NOT SURVIVE. On 2026-08-21 four were destroyed by another process — under .claude\worktrees\ AND under D:\Worktrees\, one of them holding this exact work, and @@ -76,22 +87,37 @@ idempotency table. Rulings now run to 66. OPEN, in the order I would take them: - 1. THE BROWSER GATE WAS RUNNING WHEN THE SESSION ENDED, and its result is unknown. Re-run it: - npm run test:e2e -- tests/ui-caring-contacts-workspace.spec.ts --project=chromium - Redirect the whole log to a file; do NOT pipe it through `tail`, which destroys the per-failure - detail. NEVER invoke `npx playwright test` directly — the repo refuses it with an `Error:` line - and EXIT CODE 0, so it looks like a pass and ran nothing. Expect 33 tests. - 2. TWO MUTATION PROOFS ARE UNRUN and are recorded as unrun, not as passed — the condensed bar's - pin assertion at 1440px, and its dark-mode colour assertion. Both were blocked on the browser - gate. Run them once (1) is green, or the bar's fix round is not closed. - 3. THE SEVEN COPY ITEMS await the owner. Recommendations are already written and given to him. + 1. DONE 2026-08-24 — THE BROWSER GATE IS GREEN ON main. `32 passed (55.5s)`, exit 0, no + ECONNRESET anywhere in a 341-line log. The one test that failed on 2026-08-23 — + `:822 pins the condensed bar under the header once the banner has gone at 1440px` — ran and + passed in 898ms. So the residual failure was load, not a defect, and the condensed bar's fix + round IS closed on that count. Two things made the earlier reading harder than it needed to be + and are worth keeping: Playwright reports a failure at the test's DECLARATION line, so + `:822` named the `test(...)` line and not the failing statement — the ECONNRESET was actually + in `arrangeServiceStop`'s setup POST at line 672, before any pin assertion ran, which is why a + transport error could never have been the pin being wrong. And `npm run test:e2e` is still the + only correct invocation: `npx playwright test` refuses with an `Error:` line and EXIT CODE 0, + so it reads as a pass having run nothing. Redirect the whole log to a file; never pipe a gate + through `tail`. Expect 32 tests, not 33. + 2. DONE 2026-08-24 — BOTH MUTATION PROOFS RUN, and both discriminate. Pin (`top-full` -> `top-0`): + 13 failed / 19 passed, failing at line 877 on `barBox.top` 64 -> 0 with the two preceding + assertions passing first, so the 1440px pin is REACHED. Dark colours (danger tokens -> fixed + literals): exactly 1 failed / 31 passed, at line 931, naming the injected literal — no + collateral at all. The condensed bar's fix round is CLOSED. + Read the build record's account of the near-miss before writing your own mutation proof: the + first colour mutation silently failed to apply and its gate reported `32 passed`, exit 0. + 3. THE COPY DECISIONS await the owner. `docs/caring-contacts/copy-decisions-recommended.md` + (new, 2026-08-24) carries a recommendation, a reason and a cost-if-wrong for each. Note the + count: this prompt and the ledger both said SEVEN, but `copy-review.md` actually raises + THIRTEEN — nine clinical or policy, four pure engineering. The seven was an undercount. + Do not change patient-visible wording until he has answered. 4. PHASE 2B has no plan yet. The owner's stated order: patients and their plans -> schedule and what's due -> message templates -> team, workload and coverage. - 5. An `/issues capture` sweep has NOT been run. Deferred items currently survive only in the build - record: the accidental same-team serialisation; postgres-repository.ts at ~2,080 lines; four - bare foreign keys onto plans/contacts; the prohibited-language gate covering only the 24 overlay - rows; the frozen matrix's ambiguous "Recovery action only" wording; Ruling 60's 640-767px band; - `connection-unavailable` and `permission-unavailable` having no runtime caller. + 5. DONE 2026-08-24 — the `/issues capture` sweep ran. All seven deferred items are now immutable + request files under `docs/outstanding-issues-inbox/`, so they no longer survive only in the + build record. They reach `docs/outstanding-issues.md` when someone runs + `npm run issues:reconcile` from a serialized fresh-base branch; until then they are queued, + not filed. THE AUTHORITIES, if the brief sends you there or a conflict arises - docs/superpowers/plans/2026-08-19-caring-contact-phase-2a-foundations.md @@ -105,19 +131,16 @@ THE AUTHORITIES, if the brief sends you there or a conflict arises changes against most training data. Reading beats reasoning; more thinking does not repair a wrong prior. -THE CODE TASK 11b TOUCHES — three files, all large; let a subagent read them - - tests/helpers/caring-contacts-repository-contract.ts receives the moved tests — do this FIRST - - tests/caring-contacts-repository.test.ts loses the moved tests - - src/lib/caring-contacts/db/postgres-repository.ts the ~22 missing methods - Reference for intended behaviour (read as specification, do NOT copy its structure): - - src/lib/caring-contacts/in-memory-repository.ts +THE STORAGE LAYER — Task 11b finished this; the map is here because Phase 2B extends it - src/lib/caring-contacts/repository.ts the interface: 38 methods - Also relevant: - - tests/caring-contacts-postgres-repository.test.ts carries temporary scaffolding 11b removes + - src/lib/caring-contacts/in-memory-repository.ts implements all 38 + - src/lib/caring-contacts/db/postgres-repository.ts implements all 38; ~2,080 lines + - tests/helpers/caring-contacts-repository-contract.ts the SHARED contract BOTH stores run. + New behaviour goes HERE, not in one store's + own file, or the two stores drift. - tests/helpers/caring-contacts-postgres.ts harness; CARING_CONTACTS_DATA_TABLES is a hand-maintained truncation list every new table must join - - tests/caring-contacts-retention.test.ts the one known-red test; Ruling 26, Step 0b THE SEALED DOMAIN — src/lib/caring-contacts/ (27 modules). The store PERSISTS decisions; it must never re-derive a rule a module already owns, even if the answer matches today: @@ -162,19 +185,20 @@ THE SCRATCH WORKSPACE — generated, never a source record instead. The original was git-ignored, was destroyed, and took the only copy of the session ledger with it. That is why it is now regenerable. -═══ STATE — nothing is outstanding ═══ -Tasks 1-10 and Task 11a are COMPLETE and reviewed. Task 11a went through three fix rounds; Rulings -27-34 are implemented and verified. The caring-contact database suite is at 96 passed, and its three -newest tests are proven falsifiable by deliberate mutation, not merely green. - -Two failures are EXPECTED and must NOT be "fixed" by weakening anything: - - `npm run typecheck` is RED on src/lib/caring-contacts/db/postgres-repository.ts. The interface - declares 38 methods, the in-memory store implements 38, Postgres implements 16 — a gap of 22. - Task 11b closes it, and restoring typecheck is the task's headline deliverable. Do NOT narrow the - interface and do NOT stub methods: a stub that satisfies the compiler while failing at runtime - turns a visible failure into a hidden one. - - `npm run test` has exactly one failure, tests/caring-contacts-retention.test.ts. Ruling 26 - specifies the fix and it is Step 0b of your brief. +═══ STATE — all 19 tasks built; NOTHING is knowingly red ═══ +Every Phase 2A task is complete and reviewed, both checkpoints passed, the final whole-branch review +ran, and its findings were fixed through Ruling 65. Rulings run to 66. + +THE TWO EXPECTED FAILURES THIS SECTION USED TO NAME ARE BOTH GONE, and neither was fixed by +weakening anything — that matters, because a green tree reached by softening an assertion is worse +than the red one it replaced: + - `npm run typecheck` was RED on db/postgres-repository.ts, a 22-method gap against the + 38-method interface. Task 11b implemented all 22. Green. + - `npm run test` had exactly one failure, tests/caring-contacts-retention.test.ts. Ruling 26's + fix landed as Step 0b of Task 11b. Green, and the retention suite went 23 -> 24 tests. +So a red typecheck or a red retention test is now a REGRESSION, not the documented baseline. If you +meet one, do not reach for `SKIP_STATIC_GUARD=1` — that override existed only while the red above +was expected. ═══ WHAT THIS IS ═══ A suicide-prevention caring-contacts workspace: patients discharged from hospital receive a fixed @@ -249,7 +273,11 @@ Sonnet 5 at medium-high for ordinary implementer work. Opus 5 at high for: migra security, Tasks 17-18 (the 24-overlay modality contract), anything displaying delivery or clinical state, and the final whole-branch review. -═══ STOP AFTER Checkpoint 2. Do not start Task 12. ═══ +═══ WHERE TO STOP ═══ +Phase 2A is finished, so there is no task boundary left to stop at. Stop instead when the closing +items above are done, and DO NOT begin building Phase 2B screens: 2B needs its own written plan and +the owner's copy decisions first, and starting it early is how the wording gets settled by an +implementer instead of by him. ═══ TELL ME AT THE END ═══ What was built, what you decided on my behalf and what each costs if wrong, and anything you could not diff --git a/docs/caring-contacts/phase-2a-handoff.md b/docs/caring-contacts/phase-2a-handoff.md index 6da37a0039..1ab51fafa1 100644 --- a/docs/caring-contacts/phase-2a-handoff.md +++ b/docs/caring-contacts/phase-2a-handoff.md @@ -4,8 +4,15 @@ new session, a new machine, or a new account. Everything below is either in this repository or named with an exact path on the workstation. -Written at head `6322017ce` on branch `claude/suicide-contact-mockup-b5aaa0`. Nothing has been pushed; there -is no pull request; the branch exists only locally. +> **SUPERSEDED IN ONE RESPECT, 2026-08-24: Phase 2A HAS MERGED and the branch named below is RETIRED.** +> It went in as `e4cbe8d3a` — "Claude/suicide contact mockup b5aaa0 (#2279)" — on 2026-08-23, and `main` +> is now the source of truth. Everything else in this document still stands. Work from a fresh worktree +> off `origin/main`; do not resurrect `claude/suicide-contact-mockup-b5aaa0`. +> +> Note also that this file used to contradict itself: the line here said the branch had never been pushed +> while §4 said it was pushed to origin. Both were written truthfully at different moments and neither was +> updated when the other changed. **A record that disagrees with itself is telling you its update +> discipline failed, not which half to believe** — check git. --- @@ -97,17 +104,23 @@ contains `*`), so it remains disposable by design. ## 4. Exactly where the work stopped -**Branch:** `claude/suicide-contact-mockup-b5aaa0` — **PUSHED to origin.** GitHub holds it; that is the -source of truth, not any directory on this workstation. +**Branch:** MERGED AND RETIRED as of 2026-08-23. `claude/suicide-contact-mockup-b5aaa0` was squash-merged +into `main` as `e4cbe8d3a` (#2279); every caring-contacts path on `main` matches the old branch tip, and no +local remote-tracking ref for the branch survives. **`main` is the source of truth**, not that branch and +not any directory on this workstation. -**Working copy:** make your own. Do not assume one exists: +**Working copy:** make your own, off `main`. Do not assume one exists: ``` cd D:\Repos\Database git fetch origin -git worktree add D:\Worktrees\Database\ claude/suicide-contact-mockup-b5aaa0 +git worktree add D:\Worktrees\Database\ -b claude/ origin/main +node scripts/setup-codex-worktree.mjs ``` +That last line matters: `npm ci` costs 15-58 minutes here, but on 2026-08-24 every `package-lock.json` on +this machine was byte-identical, so the setup script reused an existing `node_modules` in seconds. + **Working directories on this machine do not survive.** On 2026-08-21 four were destroyed by another process — under `.claude\worktrees\` **and** under `D:\Worktrees\`, one of them holding this exact work, and one through an explicit `git worktree lock`. **Relocating is not protection.** The `.git` pointer file @@ -116,7 +129,7 @@ No warning, and the cause is not identified. Commit often, **push after every ta needed to resume in a **tracked** file — git-ignored scratch dies with the directory. This branch survived a destruction today only because it had been pushed. -**Head at last push:** `32bfbdae5`. Nothing merged, no pull request. +**Head at last push:** `32bfbdae5` — historical. The phase has since merged; see the banner at the top. ### Done and reviewed clean diff --git a/docs/caring-contacts/phase-2b-build-record.md b/docs/caring-contacts/phase-2b-build-record.md new file mode 100644 index 0000000000..8d377171a2 --- /dev/null +++ b/docs/caring-contacts/phase-2b-build-record.md @@ -0,0 +1,725 @@ +# SDD ledger — plan: docs/superpowers/plans/2026-08-24-caring-contact-phase-2b-screens.md + +**This is the Phase 2B build record and the SDD ledger, in one tracked file.** Per Phase 2A's +Ruling 67, this programme does not keep a ledger in git-ignored `.superpowers/sdd/` scratch — a +git-ignored session ledger was destroyed once already and took the only copy of its session's record +with it. The build record IS the ledger. + +**Where Phase 2A's record ends and this one begins:** `phase-2a-build-record.md` holds Rulings 1–67 +and every Phase 2A task. Ruling numbering CONTINUES here from 68 so a ruling number is unique across +the whole programme. + +Base commit for this plan: `875c8b604`. + +--- + +## Pre-flight scan of the plan + +Run before dispatching Task 1, per the method. The output is a table, not a verdict. + +### Task pairs sharing a file or an interface + +| Tasks | Shared surface | What one produces / the other consumes | Finding | +| ---------------------- | ----------------------------------- | -------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 4 → 5, 13, 15, 18 | `shell.tsx` destination lists | Task 4 adds the first `href`; each screen task adds its own | **Sequential edits to one file.** No contradiction. Implementers are never dispatched in parallel, so this is a merge risk only if that rule is broken. | +| 1 → 5, 13, 15, 18 | the empty-state component | Task 1 produces it; four list screens consume it | Clean. Task 1 must land before any consumer. | +| 2 → 5, 12, 17 | the list-read API pattern | Task 2 produces the helper + contract test; three routes consume it | Clean, and this is the whole reason Group 0 exists. | +| 3 → 11, 14, 16, 18, 20 | the overlay trigger | Task 3 produces it; five tasks wire overlays with it | Clean. Ruling 69 keeps wiring with the owning screen. | +| C → 16 | `message-copy.ts` | Task C rewrites the reply wording (items A2/A3); Task 16 renders it | **Ordering constraint.** C must land first, or Task 16 renders wording that is about to change. Recorded, not a conflict. | +| 7–9 ↔ 13 | the nine-contacts / closing-message | Corrections #3 and #4 touch the activation review AND every schedule | **Genuine cross-task requirement.** Whichever lands first sets the shape; the second must not re-derive it. Both must read the same source of truth in `schedule.ts`. | +| 5 ↔ 6 | `getEpisode` vs `listPlans` | Task 5 must NOT call `getEpisode`; Task 6 is the one screen that may | Clean, and stated in the plan. Worth re-stating in both briefs. | + +### Per-task self-consistency + +| Task(s) | Its own text agrees with itself? | +| ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| C | Yes — six named edits in two named modules. | +| 1–4 | Yes. | +| 5–11 | Yes, with the `getEpisode` restriction stated. | +| 12–14 | Yes. | +| 15–16 | **NO — defect found, see Ruling 73.** The design-corrections table routes correction #2 to "Group 3, Task 11", but Task 11 is Group 1's overlay wiring; Group 3 is Tasks 15–16. | +| 17–18 | Yes, with Ruling 72's scope limit stated. | +| 19–21 | Yes. | + +### Anything the plan mandates that the review rubric treats as a defect + +None found. The plan mandates no test that asserts nothing and no verbatim duplication of a logic +block. + +--- + +## Rulings + +**Ruling [73] — the design-corrections table's "Group 3, Task 11" is a typo for Task 16; corrected in +the plan.** — Why: Task 11 is Group 1's overlay wiring and cannot carry a Group 3 copy correction. The +correction is the reply-handling wording, which belongs to the message-preview surface built in Task 16. — Cost if wrong: had it stood, Task 11's implementer would have received a requirement it had no +surface for, and either implemented it in the wrong place or reported BLOCKED — a wasted dispatch +either way. This is exactly what the pre-flight scan is for and it is the first thing the scan found. + +**Ruling [74] — Group 4 is built at the approved roster-table depth, and the owner is told plainly +rather than asked again.** — Why: this is Ruling 72 carried into execution. The owner was asked +directly whether "workload and coverage" means a staff list or something richer, and answered "go +ahead" to the plan without narrowing it. The method's standing instruction is to rule rather than +stall, and only the roster table has an approved design — inventing a capacity view would be design +work done by an implementer, which is worse than delivering the designed thing. — Cost if wrong: if +he meant rosters, leave and caseload, Phase 2B delivers a thinner group 4 than he expected. It costs a +design pass and one more group later, not rework: nothing built at roster depth becomes wrong. +**Flagged to him in the closing report, not buried here.** + +**Ruling [75] — Guidance and Reports (Task 19) are deferred to the END of Phase 2B, and may be cut to +Phase 3 without blocking anything.** — Why: both sit outside the owner's stated four groups, both +already have approved designs, and neither is a dependency of any other task. Deferring them costs +nothing and protects the four groups he actually asked for. — Cost if wrong: if he wanted Reports +early — the equity reach section is the one part with external interest — it arrives later than he +hoped. Reversible at any point by moving one task. + +**Ruling [76] — the approved copy changes are executed as ONE batched task (Task C) ahead of Group 0.** — Why: the method says to batch small same-shape work into one dispatch rather than one subagent +per item. All six approved edits are small, independent, and land in two adjacent modules +(`message-copy.ts`, `message-policy.ts`). They also unblock nothing else, so they are cheap to do +first and get the owner's approved wording into the tree before any screen renders it. — Cost if +wrong: one review surface covers six changes, so a weak review could let one through. Mitigated by +requiring a separate covering test per item, named by item number. + +**Ruling [77] — A9 (add Lifeline) is NOT dispatched, in spite of being approved.** — Why: the approved +recommendation is conditional on a real crisis number existing, because the message is ~9 characters +from its two-segment maximum and nothing can be added until something is removed. No real number +exists. Dispatching it would force an implementer to choose which patient-facing sentence to delete — +precisely the decision the owner was asked to make and which his "go ahead" does not answer. — Cost if +wrong: the message carries no Lifeline number for now, which is the status quo and is the safe +direction. **Re-ask when a real crisis number exists.** + +**Ruling [78] — no push, no pull request, at any point in this plan without the owner saying so.** +— Why: he was asked directly and has not answered; the method's own stop conditions name a push to a +shared branch as something to ask about. Commits accumulate locally, which is what protected this work +before. — Cost if wrong: the work sits on one machine, which is the machine that has destroyed four +working directories. Mitigated because commits on a worktree branch live in the shared object store +and survive the worktree itself. + +--- + +**Ruling [79] — A1 is implemented as an acknowledged validator issue, not as a prohibited term.** +— Why: the owner approved "refuse any message still containing the word Fictional", but BOTH approved +patient messages contain `Fictional Support Line` today, so adding it to `prohibitedTerms` makes every +existing message invalid and the check would have to be disabled to ship. A disabled check is worse +than no check. Instead `validateGovernedMessage` gains a `fictional-contact-detail-present` issue and +`GovernedMessageInput` gains an explicit `syntheticFictionalContactsAcknowledged` flag; the prototype's +callers pass it, so the acknowledgement is greppable and attached to each call site. The day a real +send path is built, someone must consciously pass a flag whose name says it is synthetic, or remove +the fictional numbers. — Cost if wrong: more machinery than a one-line string check, and one more +field on a widely used input type. The alternative was a check that could not coexist with the +messages it guards. + +**Ruling [80] — A3's "something automatic comes back" goes ONLY in the reply message, not in the +first message.** — Why: Message A measures 252 septets against a two-segment ceiling — about nine +characters of headroom — and the addition does not fit. Message B has room and is where a patient who +has just replied actually reads. Measured with the repository's own `calculateGsm7`, not estimated: +Message A 252, current Message B 218, proposed Message B **210**, all two segments and GSM-7 valid. +— Cost if wrong: a patient reading only the first message is not told an automatic reply exists. They +learn it the moment they reply, which is the only moment it matters. + +## Task progress + +Task C: dispatched (sonnet), base `ac87293f2`. Returned **DONE_WITH_CONCERNS** at `9a4cf055c`, seven +commits. Full suite `Tests 2 failed | 9778 passed | 74 skipped (9854)`; typecheck and lint clean. + +**The two failures are NOT this diff, and I verified that myself rather than accepting the report.** +They are `tests/gate-receipts.test.ts` > "gate receipts — file modes", both failing inside +`chmodSync(..., 0o755)`. Run alone: `Tests 2 failed | 32 passed (34)`. The diff does not touch that +file, touches nothing filesystem-related, and the failing group is specifically about **file modes** — +which this workstation cannot represent, being a Windows ReFS Dev Drive with `core.fileMode=false`. +Environmental, and now a known third local failure alongside the session-start-hook and +worker-observability ones. + +**Implementer concern 1, confirmed and consequential: `validateGovernedMessage` has ZERO production +callers.** The brief told it to "update the prototype's existing callers" to pass the new +acknowledgement flag; there are none. `grep -rn validateGovernedMessage src/ worker/` returns only its +own definition. That is coherent — nothing is ever sent, so nothing validates — but it means the A1 +guard protects a path that does not exist yet. **The owner should not read "the validator refuses an +unacknowledged fictional number" as "the system refuses it".** Captured to the issues inbox as P2. + +**Implementer concern 3, out of scope and captured:** `tests/helpers/caring-contacts-prohibited-language.ts` +carries the same `\bleads?\b` job-title collision for interface copy that B2 just fixed for messages. +Captured as P3. The implementer reported it rather than fixing it, which is what the brief asked for. + +Task C: task review dispatched (opus), with four named questions — B2's narrowing of what is now +permitted, whether A1's derived marker tracks the owner's intent, whether an unwired A4 refusal +achieves the approved outcome, and whether B3's scan can actually fail. + +### Task C review — spec ✅, quality approved with findings + +Reviewed on opus with four named questions. **Spec ✅**: all six items, exact values verbatim, nothing +extra. **No assertion deleted or loosened** — the only removed test lines are the two `septets: 218 → +210` pins, and six existing tests that gained the acknowledgement flag kept their `toEqual` +expectations intact. Domain isolation holds. Mutation proofs judged genuine on internal evidence +rather than trust: the A2/A3 proof reports septets moving 210 → 221 and the injected `" definitely"` +is exactly 11 septets; the B3 proof quotes Vitest's exact `expected 0 to be greater than 0`. + +Three Important, four Minor. **The two findings worth carrying forward as lessons:** + +- **An allowlist cannot close an open-ended set.** B2 narrowed the "lead" prohibition by enumerating + nine commercial modifiers and six companions. Every phrasing not thought of is now permitted — + `lead magnet`, `lead nurturing`, `qualify this lead`, `convert the lead` all pass and all previously + failed. The correct shape is the inverse: refuse `\bleads?\b` by default and exempt the job-title + collocations, because THAT set really is closed in this domain (`incident lead`, `programme lead`, + `clinical lead`, `team lead`, `service lead`). **Enumerate the safe set, never the dangerous one.** +- **A guard on a chokepoint fires; a guard beside one does not.** A1 and A4 look like the same kind of + delivery and are not. A1's check lives inside `validateGovernedMessage`, which any future sender + must pass through, so it fires automatically the day a sender exists. A4's + `resolveClosingContactMessageBody` is a standalone function nothing obliges anyone to call, so a + future author can resolve a closing body any other way and never meet it. Same "unwired" label, + opposite futures. + +**Ruling [82] — the A1 marker finding is PROMOTED from Minor to Important and enters the fix round.** +— Why: as built the marker is `crisisSupportContact.split(":")[0]` = the literal "Fictional Support +Line", so a message carrying the reserved NUMBER with no label raises nothing — and that is precisely +the shape that would reach a sender. A1's whole purpose is stopping a fictional crisis number reaching +a real patient; the number is the artefact that matters and the label is the harmless one. Two +realistic reformattings of the crisis contact also silently disable it without reddening a test. +— Cost if wrong: one extra item in a fix round that was happening anyway. The reviewer graded it Minor +on scope grounds and was not wrong to; I am weighting it by what it is guarding. + +**Ruling [83] — A4 is NOT recorded as closed, and the owner is told so.** — Why: the approved outcome +was "refuse loudly rather than send nothing". Nothing refuses today, and nothing can be made to +refuse without a future author choosing to call a function they are not obliged to call. Recording it +closed would convert an open safety gap into a solved one in the only document anyone will read later. +— Cost if wrong: the item stays open in the ledger slightly longer than a generous reading needs. + +Minors 5, 6 and 7 were bundled into the same fix round rather than deferred — each is a one-line +assertion or a comment reconciliation, and a round was happening anyway, so bundling extends nothing. +Recorded because the method's default is that Minors do not enter the loop. + +Task C: fix round 1/5 dispatched — resumed the original implementer with 7 findings (3 Important, 1 +promoted, 3 bundled Minors). + +### Task C fix round 1 — returned DONE at `a865d6aa9`, and the B2 inversion verified independently + +Full suite `Tests 2 failed | 9785 passed | 74 skipped (9861)` — the same two pre-existing +`gate-receipts` file-mode failures, untouched by this round. Typecheck and lint clean. + +**I tested the inverted "lead" pattern myself rather than accepting the report**, because it is the +change most able to fail quietly in the permissive direction. The new pattern is a negative lookbehind +— `/(?/i` built from +`DESIGNATED_FICTIONAL_MOBILE_NUMBERS`, so the label, the bare number, a relabelling and a reordering +are each independently sufficient to fire it. `message-rules.ts` gained its first import +(`./synthetic-contacts`) — inside the sealed domain, and `synthetic-contacts.ts` imports nothing, so +there is no cycle and no isolation breach. Both checked directly. + +**The lesson the inversion confirms, stated as a rule for the rest of this plan: when a check must +distinguish a safe set from a dangerous one, enumerate whichever set is CLOSED.** Here the job titles +this domain uses are five; commercial vocabulary for "lead" is unbounded. The first attempt enumerated +the unbounded side and was permissive in exactly the places nobody thought of. The same test applies +to every allowlist, ignore list and exemption this plan will add. + +Task C: fix round 1/5 (7 addressed, 0 open by the implementer's account; commits `9a4cf055c`..`a865d6aa9`). +Scoped re-review dispatched. + +### Task C scoped re-review — ALL SEVEN ADDRESSED, no new Critical or Important + +The re-reviewer re-derived every claim by executing the regexes and re-running the B3 scan against the +real tree rather than reading the report's assertions. Verified independently of my own check: the +inverted pattern refuses all six leaked phrasings plus `lead score`, and exempts all five job titles; +the A1 marker catches the relabelled, reordered and bare-number shapes; B3's new pass finds +`inbox`/`campaign` in a fixture written as bare JSX text with no quotes anywhere, and finds 0 in the +real tree across 15 files. + +**Two mechanism-level checks worth keeping, because both are the kind that a test-level check would +have passed over:** + +- **The A1/patient-mobile double report is benign, and for a reason.** The patient-mobile check is + independent of `syntheticFictionalContactsAcknowledged`, which gates only the fictional check. So a + caller that acknowledges synthetic contacts silences the noisy code and keeps the safety-critical + one. The changed expectation is a tightening — the original assertion survives with a second true + code beside it, in the order the validator actually pushes them. +- **The global-regex handling is correct**, which is easy to get wrong: the prohibited-language regex + stays non-global for `.test()`, and the `g` copy is used only with `matchAll`, which never advances + the original's `lastIndex`. A shared global regex with `.test()` would have skipped every second + match. + +**Mutation proofs judged genuine, 5 of 5, and B2's is two-directional as asked**: reverting to the +allowlist makes `lead nurturing` valid and reddens the refusal test; widening to a bare `\bleads?\b` +makes `the incident lead` invalid and reddens the exemption test. Those bracket the behaviour from +opposite sides rather than being two views of one assertion, which is what "prove it discriminates" +actually requires. + +**Deferred minors — pointed at the final whole-branch review, not fixed now:** + +1. **New Minor 8 — the job-title exemption requires WHITESPACE adjacency, but this domain writes + `team-lead`** (`contact-rescheduling.ts`, `repository.ts`, and the mockup overlay copy). Inert + today: those files are outside B3's two scan roots, neither approved message contains "lead", and + the validator has no production callers. Conservative direction. One-character fix (`[\s-]`) if + ever needed. The verb sense (`can lead to relief`) is also refused and I would NOT change that + without the owner — refusing it is defensible in a safety vocabulary. +2. `stripCommentsAndClassNameValues` strips `//` and `/* */` inside string literals too; a future URL + literal would blank the rest of its line. Narrow, because the quoted pass still covers quoted text. +3. The raw-prose pass scans identifiers and JSX attribute names, not only prose. Zero hits today; + fail-closed, so acceptable. +4. `scanRootForProhibitedLanguage` is now used only by fixture tests while the real-tree test inlines + its own walk — two code paths that must stay in step. +5. The A1 marker matches literal number strings, so `+61491570158` or `0491 570 158` still evade. +6. Minor 7's floor is aggregate across both roots, so one root emptying would still pass. + +Task C: fix round 1/5 (7 addressed, 0 open; commits `9a4cf055c`..`a865d6aa9`). +**Task C: COMPLETE (commits `ac87293f2`..`a865d6aa9`, review clean, 6 minors deferred).** + +## Reading the API layer properly shrank the plan — three rulings + +Written while Task 1 ran. I went to write Task 2's brief, read the code it was meant to extract a +pattern from, and found the pattern already there. Two more of the plan's premises fell over the same +way. **All three came from recon reports that were factually correct and whose implications I had +carried too far** — a distinction worth naming, because the reports are not at fault and the same +mistake is available on every remaining task. + +**Ruling [84] — Task 2 (the list-read API pattern) is CUT.** — Why: `readHandler` in +`caring-contacts-server/handler.ts` already IS the pattern. It gates on the demo flag, resolves the +actor, opens the store, wraps the read in `auditedRead`, and maps outcomes to responses — including +the subtle ones: `access-audit-unavailable` produces 503 (a read nobody can prove happened is worse +than a read refused), failed produces 500, denied produces `not-found`. **Eight routes already use +it** and four already share the `COLLECTION = "all"` objectId convention for a collection read. +Extracting a helper from call sites that already share a factory would deliver a wrapper around a +wrapper. — Cost if wrong: the three new list routes have no shared helper of their own — but they +still share `readHandler`, which is where the audit and refusal semantics actually live, so the thing +worth holding together is already held. + +**What survives the cut, and must move into the first list route's brief:** one contract test pinning +that an **empty list is 200 with an empty array, never a 404**. That is not obvious from the code — +`auditedRead` maps a null-or-undefined release to `denied`, which becomes `not-found`, and an empty +array is neither. The factory's own comment says the trail cannot distinguish "you may not see these" +from "there are none" for a list, which is exactly why the HTTP shape must be pinned by a test rather +than left to reading. + +**Ruling [85] — Task 5 does NOT build a patients read; it consumes the one that exists.** — Why: +`GET /api/caring-contacts/plans` already lists the team's plans via `readHandler` +(`objectType: "plan"`, `kind: "search"`). The domain recon noted a `patientDirectory` access-object +type "whose repository read isn't confirmed wired", and I read that as a gap. It is not: that object +type is already used by the **referrals** route, deliberately, because a referral names a patient who +may not yet have a plan. Both reads exist and they are different reads. — Cost if wrong: if a patients +directory eventually needs patients with no plan and no referral, a new read appears then — but +nothing built now becomes wrong, because listing plans is what a caseload screen shows. + +**Ruling [86] — design correction #1 is already implemented in the domain; only the control is +missing.** — Why: spec section 2.3 (the coordinator sets the first contact date) is recorded in +`phase-2a-visual-differences.md` as a difference the mockup does not reflect, and the plan routes it +to Task 6 as though it were unbuilt. In fact `schedule.ts` takes `firstContactDate` and validates it, +and the plans POST schema already accepts `firstContactDate` and `firstContactReason`. The correction +is therefore a **screen** change only. — Cost if wrong: none identified; if the domain's handling +turns out incomplete, Task 6 discovers it against a real API rather than building a second path. + +**The lesson, and it is the one to carry into every remaining brief.** A reconnaissance report tells +you what it looked at. Three times here I turned "X was not confirmed wired" or "X is recorded as a +difference" into "X does not exist", and each time the thing existed. **Before a brief says "build +this", open the file and look.** The cost of not doing so is not a wasted task — it is a SECOND +implementation of something that already works, sitting beside the first, both maintained. + +## Ruling 87 — Task 3 cannot ship a trigger without the commit contract + +Verified in the code before writing Task 3's brief, applying the lesson from Rulings 84-86. + +**What already exists:** `workspace-overlays.tsx` is already a Client Component, and +`openWorkspaceOverlay(id)` is already exported and covered by DOM tests. It pushes +`?overlay=` onto history so Back closes the overlay. So Task 3 is NOT "build an overlay +opening mechanism" — that is built. What is missing is a small client-side control a Server +Component screen can render to call it. + +**What reading it also exposed, and this is the part that matters.** `WorkspaceOverlays`' +`commit` callback currently **closes the overlay and records nothing**, with an honest comment +saying so: the screens that raise these overlays and the stores their decisions are written to +are later tasks, and "nothing in the workspace opens an overlay yet, so no control in the +interface currently advertises an action this does not perform." + +**That last clause is load-bearing, and Task 3 is precisely what would break it.** The moment a +screen can open an overlay, its confirm button becomes a control that advertises an action the +system does not perform — which is exactly what this repository's button-wiring gate exists to +forbid, and what the "Language and region" defect of 2026-07-21 was. The overlays are currently +safe only because they are unreachable. + +Ruling: [87] Task 3 delivers the trigger **and** the commit contract together. The trigger +component must **require** a commit handler from its caller — not default it to a no-op, and not +accept an optional one. A screen must be unable to open an overlay it has not wired. Where an +overlay's action genuinely is not built yet, the caller passes an explicit unavailable-state +handler in the shape `unavailable-destination.tsx` already uses (`aria-disabled`, an inert +handler, a stated reason), so the control still says what it is. — Why: splitting them would +ship, for the length of one task, an interface whose confirm buttons do nothing — and a later +task would have to find every one of them. Requiring the handler at the type level means the +compiler finds them instead. — Cost if wrong: Task 3 is larger than the plan sized it, and the +first screen to use it carries more wiring than "open a panel". That is the honest cost of the +overlays being decision surfaces rather than dialogs. + +**The general shape, worth keeping:** a mechanism that is safe only because nothing reaches it is +not safe, it is unreached. Before making something reachable, check what its arrival makes true. + +## Task 1 — the shared EmptyState component + +Dispatched sonnet, base `ff79cb6ce`. Returned **DONE** at `97a7ff782`. +New file `tests/caring-contacts-empty-state.dom.test.tsx`: `Tests 9 passed (9)`. Full suite +`Tests 2 failed | 9794 passed | 74 skipped (9870)` — the two known `gate-receipts` file-mode +failures and no others. Typecheck and lint clean, both after real lease acquisition rather than a +lock-contention exit. + +**The lock incident, and the correction it produced.** The implementer paused mid-task waiting on the +repository's heavy-run lease, held by a concurrent session's Playwright run — with its work +UNCOMMITTED. On a machine that has destroyed four working directories mid-session, that is the +expensive shape of an ordinary delay. Resumed with an explicit instruction ordering: **commit first, +then retry the gate, bounded.** It committed, retried twice, acquired the lease and finished for real. + +The instruction is now standing for every remaining brief: _commit before waiting on any gate, and a +lock-acquisition failure is neither a pass nor a failure — if the output carries no summary line the +run did not happen, whatever the exit code says._ Machine health was checked rather than assumed: +`node --version` returned in 0.083s, so this was ordinary lease contention and not the +process-spawn starvation that has previously made everything slow. + +**Task C's B3 scan caught a defect in Task 1, one task after being built.** The first draft used +lucide-react's `Inbox` icon; `caring-contacts-interface-vocabulary.test.ts` rejected it, because +"inbox" is banned as reply-monitoring language. The implementer switched to `FolderOpen`. + +This is worth recording precisely, because it cuts both ways. It **fired on a bare identifier, not on +user-facing prose** — which is exactly deferred item 2 from Task C's re-review ("the raw-prose pass +scans identifiers, imports and JSX attribute names, not only prose … a future `const conversation = …` +would fail the test with a confusing message"). That deferred concern materialised within one task, +so it is real and will recur. Whether it was a _false_ positive is a judgement I have put to Task 1's +reviewer rather than settled myself: an icon named `Inbox` is not text a patient or clinician reads, +but it is also the kind of name that leaks into a `data-testid` or an `aria-label` without anyone +noticing. If the reviewer judges it over-reach, narrow the scan; if not, leave it and accept the +occasional harmless rename. **Do not narrow it merely because it was inconvenient once.** + +Task 1: task review dispatched (sonnet), with four open questions — the missing `role="group"` +wrapper that the sibling `AutomatedState` has, whether typing `action` as `ReactNode` weakens the +button-wiring guarantee, whether the discriminated union delivers its promise at the type level, and +whether the mutation proof is genuine. + +### Task 1 review — spec ✅, quality NOT approved as-is; two Important findings + +**The mutation narrative was wrong, and the reviewer found it by re-running both mutations rather +than reading the report.** The report described mutating the `"filtered"` branch and claimed +`Tests 2 failed | 7 passed`. Applying exactly that edit produces `Tests 1 failed | 8 passed` — with +the guard hardcoded `true` the else-branch is dead code, so touching it cannot affect the `"no-data"` +case. Putting the placeholder in the branch that ALWAYS executes reproduces the reported numbers +exactly, including the two failure messages quoted. + +So the evidence is genuine and reproducible; the account of **which branch was mutated** is not, and +the mutation actually run is coarser than the brief asked for — it broke both kinds' content equally +rather than making `"filtered"` render as `"no-data"`. The distinction is still proven. But a report +that misdescribes its own proof is a report whose other proofs cannot be taken on trust, which is why +this is Important rather than a note. **Self-reported mutation results need independent +re-derivation, and this is the second time on this programme that re-deriving one changed the +answer.** + +**Ruling [88] — the component is renamed `ListEmptyState`, and the naming collision was MY defect, +not the implementer's.** — Why: `src/components/ui-primitives.tsx` already exports an `EmptyState`, +a registered design-system primitive; `EmptyState` appears across 43 files. My brief mandated the +colliding name. The consequence is not cosmetic: `scripts/generate-design-system-adoption.mjs` +matches test files to components with a bare `new RegExp("\b" + name + "\b")` over raw file text, +with **no import-path awareness** — verified at line 1562. So the regenerated +`docs/design-system/adoption-manifest.json` now credits `tests/caring-contacts-empty-state.dom.test.tsx` +as proof coverage for a shared primitive that test never imports. **That is false evidence about test +coverage in a governance artifact**, which is precisely the class of defect this programme keeps +finding, and it cannot be fixed at the call site because the matcher never looks at imports. +`ListEmptyState` is unused anywhere in `src/` or `tests/`. — Cost if wrong: a rename across one +component, one test file and the regenerated manifest. Cheaper than a manifest that overstates +coverage for a component used in 13 places. + +**The accessibility gap is real and is fixed in the same round.** `AutomatedState` wraps its three +pieces in `role="group"` + `aria-label` so a screen reader reaching the state reaches the reason and +remedy without hunting. `ListEmptyState`'s `"filtered"` branch has the identical three-piece shape — +name, why, what-changes-it — and no role, no grouping, no heading element. Ruling 81 forbade +rendering `AutomatedState`; it never forbade reusing its accessible structure, and I should have said +so explicitly in the brief. + +**Two of the reviewer's answers settle open questions and are recorded as settled:** + +- **`action: ReactNode` is the right trade, not a weakening.** An `onClick`-shaped prop cannot cross a + Server-to-Client boundary at all, so it would force this component to become a Client Component — + exactly what Ruling 13 forbids. Enforcement lives in `eslint-rules/require-button-wiring.mjs`, a + repo-wide AST scan that fires wherever a ` + ); +} diff --git a/src/components/caring-contacts/workspace/overlays/workspace-overlays.tsx b/src/components/caring-contacts/workspace/overlays/workspace-overlays.tsx index 349571863e..c0aed37e3a 100644 --- a/src/components/caring-contacts/workspace/overlays/workspace-overlays.tsx +++ b/src/components/caring-contacts/workspace/overlays/workspace-overlays.tsx @@ -1,7 +1,20 @@ "use client"; -import { useCallback, useSyncExternalStore } from "react"; +import { useCallback, useEffect, useState, useSyncExternalStore } from "react"; +import type { WorkspaceOverlayDefinition } from "./definitions"; +import { + clearStagedWorkspaceOverlayCommit, + commitForHistoryEntry, + consumeWorkspaceOverlayCommit, + commitRefusalFor, + nextWorkspaceOverlayCommitToken, + noStagedWorkspaceOverlayCommit, + readStagedWorkspaceOverlayCommit, + stageWorkspaceOverlayCommit, + subscribeToStagedWorkspaceOverlayCommit, + type WorkspaceOverlayCommit, +} from "./overlay-commits"; import { OverlayHost } from "./overlay-host"; /** @@ -91,18 +104,61 @@ function overlayUrl(id: string | null) { */ const OVERLAY_HISTORY_MARKER = "caringContactsOverlayEntry"; -function currentEntryWasPushedByThisModule(): boolean { +/** + * The token naming the staged commit this entry was opened with, if a control + * opened it. + * + * It lives beside the marker above and for the identical reason, which is worth + * stating rather than inheriting: it is per-ENTRY. A module variable would + * describe the top of the stack only, and Back, Forward, a second mount or a test + * traversing history directly would each leave it stale — and a stale token means + * a commit answering an overlay it was never staged for. `history.state` brings + * its own answer along with every traversal, so there is nothing to reset. + * + * Absent on an entry nobody opened from a control: a deep link, the entry the user + * arrived on, or the entry `history.back()` unwinds to. That absence IS the + * deep-link case, and the host reads it as "nothing is staged for this". + */ +const OVERLAY_COMMIT_TOKEN = "caringContactsOverlayCommitToken"; + +function overlayHistoryState(): Record | null { const state: unknown = window.history.state; - return typeof state === "object" && state !== null && OVERLAY_HISTORY_MARKER in state; + return typeof state === "object" && state !== null ? (state as Record) : null; +} + +function currentEntryWasPushedByThisModule(): boolean { + const state = overlayHistoryState(); + return state !== null && OVERLAY_HISTORY_MARKER in state; +} + +/** The commit token the current history entry carries, or null. */ +function readEntryCommitToken(): string | null { + const token = overlayHistoryState()?.[OVERLAY_COMMIT_TOKEN]; + return typeof token === "string" ? token : null; +} + +/** The server has no history to read, so no entry ever carries a token there. */ +function noEntryCommitToken(): string | null { + return null; +} + +function pushOverlayEntry(id: string, commitToken: string | null) { + const state: Record = { [OVERLAY_HISTORY_MARKER]: id }; + if (commitToken !== null) state[OVERLAY_COMMIT_TOKEN] = commitToken; + window.history.pushState(state, "", overlayUrl(id)); + window.dispatchEvent(new Event(OVERLAY_URL_CHANGED_EVENT)); } /** * Opening pushes, so Back closes the overlay — that is the browser-history * support rule 7 asks for. + * + * This form carries NO commit, so the overlay it opens is in the same position as + * a deep link: its decision cannot be recorded, and a recording row says so. Use + * `openWorkspaceOverlayWithCommit` from a control. */ export function openWorkspaceOverlay(id: string) { - window.history.pushState({ [OVERLAY_HISTORY_MARKER]: id }, "", overlayUrl(id)); - window.dispatchEvent(new Event(OVERLAY_URL_CHANGED_EVENT)); + pushOverlayEntry(id, null); } /** @@ -131,25 +187,120 @@ export function closeWorkspaceOverlay() { window.dispatchEvent(new Event(OVERLAY_URL_CHANGED_EVENT)); } +/** + * Opens an overlay AND states what confirming it does, in that order. + * + * This is the only way a control in the workspace opens an overlay: `commit` is + * required, so a control cannot raise a decision surface it has not wired + * (Ruling 87). `openWorkspaceOverlay` above stays available unchanged for the + * URL-only case its own tests cover, and is deliberately NOT the trigger's route. + * + * Staging first is the ordering `overlay-commits.ts` documents: both writes are + * synchronous, so the host's first render carrying the new entry already carries + * its commit and never passes through a frame where the overlay is open with + * nothing staged. The token binds the two together, so the commit answers THIS + * entry and no other. + */ +export function openWorkspaceOverlayWithCommit(id: string, commit: WorkspaceOverlayCommit) { + const token = nextWorkspaceOverlayCommitToken(); + stageWorkspaceOverlayCommit(token, commit); + pushOverlayEntry(id, token); +} + export function WorkspaceOverlays() { const openOverlayId = useSyncExternalStore(subscribeToOverlayParam, readOverlayParam, noOverlayParam); + const entryCommitToken = useSyncExternalStore(subscribeToOverlayParam, readEntryCommitToken, noEntryCommitToken); + const slot = useSyncExternalStore( + subscribeToStagedWorkspaceOverlayCommit, + readStagedWorkspaceOverlayCommit, + noStagedWorkspaceOverlayCommit, + ); - const close = useCallback(() => { - closeWorkspaceOverlay(); - }, []); + /** + * A failure from an asynchronous `record`, held so it can be raised during + * render (fix round 1, Important 4). + * + * A rejection cannot be allowed to stay in the promise: the overlay has already + * closed by then, so the clinician would be looking at a screen that gave every + * appearance of having recorded the decision while nothing was written and + * nothing was said. Re-raising it during render is what puts it in front of + * `src/app/caring-contacts/error.tsx`, which states plainly that nothing was + * sent and nothing was changed. It is stored wrapped, because a promise may + * reject with `undefined` and a bare `unknown` could not then be told apart + * from "no failure". + */ + const [commitFailure, setCommitFailure] = useState<{ readonly error: unknown } | null>(null); + if (commitFailure !== null) throw commitFailure.error; /** - * Confirming an overlay closes it, and records nothing yet. + * What the control that opened THIS history entry said confirming it does — or + * null, when nothing was staged for it and the decision cannot be recorded. + */ + const commit = commitForHistoryEntry(slot, entryCommitToken); + const commitRefusal = commitRefusalFor(commit); + + /** + * The slot belongs to one history entry, and this is the ONE place it is + * emptied (fix round 1, Importants 2 and 3). + * + * Reconciling here rather than clearing inline is what makes both of those + * true at once. Inline clearing at the end of a confirm handler ran while the + * URL still named the overlay — `history.back()` fires `popstate` + * asynchronously — so React re-rendered the still-open overlay with an empty + * slot and flashed the refusal at a clinician who had just confirmed a + * withdrawal. And clearing in `close` covered only the Sheet's own dismissals: + * Back, the workspace's primary route out, closes through `popstate` and never + * calls `onClose` at all, so the slot outlived it. * - * Stated plainly rather than left to look finished: the screens that raise - * these overlays, and the stores their decisions are written to, are later - * tasks. Nothing in the workspace opens an overlay yet either — `?overlay=` - * is reachable only by typing it — so no control in the interface currently - * advertises an action this does not perform. + * A traversal changes the entry, the entry changes the token, and a slot that no + * longer names the current entry is emptied — whatever route got us here. */ - const commit = useCallback(() => { + useEffect(() => { + if (slot !== null && slot.token !== entryCommitToken) clearStagedWorkspaceOverlayCommit(); + }, [slot, entryCommitToken]); + + const close = useCallback(() => { closeWorkspaceOverlay(); }, []); - return ; + /** + * Read-only rows are exits and recovery actions, not records. Mutating rows + * atomically claim the staged commit before invoking it, so a second activation + * while history.back() is still pending cannot produce a duplicate write. + */ + const recordDecision = useCallback( + (definition: WorkspaceOverlayDefinition) => { + if (!definition.mutatesState) { + closeWorkspaceOverlay(); + return; + } + + const activeCommit = consumeWorkspaceOverlayCommit(entryCommitToken); + // A consumed or stale token means another activation has already started + // closing this entry. It is intentionally a no-op. + if (activeCommit === null) return; + if (activeCommit.kind !== "record") { + throw new Error(`The overlay "${definition.id}" attempted to record from a non-recording commit.`); + } + + // Promise.resolve rather than instanceof Promise: a Server Action's return + // value need only be thenable, and a synchronous record returning undefined + // costs one already-resolved promise. + void Promise.resolve(activeCommit.record(definition.id)).catch((error: unknown) => { + setCommitFailure({ error }); + }); + closeWorkspaceOverlay(); + }, + [entryCommitToken], + ); + + return ( + + ); } diff --git a/src/lib/caring-contacts/message-copy.ts b/src/lib/caring-contacts/message-copy.ts index acdab353d1..022075a966 100644 --- a/src/lib/caring-contacts/message-copy.ts +++ b/src/lib/caring-contacts/message-copy.ts @@ -14,9 +14,28 @@ export const EXACT_PATIENT_VISIBLE_MESSAGE = `Hi Rowan, Alex from Example Afterc // PROVISIONAL — not clinically approved. Required by production-build spec §2.1: the automated response // sent to anyone who replies. It must name where a person IS available, immediately after saying that -// nobody reads this channel, so that reaching out is answered rather than met with silence. Content is -// discarded after this response is sent; nothing is stored, counted per patient, or shown to staff. -export const AUTOMATED_REPLY_RESPONSE = `This number is not read. Your message has not been seen by anyone and has not been kept. To talk to someone, call ${FICTIONAL_CONTACTS_BY_ROLE.programmeStaffedLine}, 9 am-6 pm every day. In an emergency call 000. Fictional Support Line: ${FICTIONAL_CONTACTS_BY_ROLE.crisisSupportContact}.`; +// nobody reads this channel, so that reaching out is answered rather than met with silence. +// +// Content is INTENDED to be discarded after this response is sent -- nothing stored, counted per +// patient, or shown to staff -- as a requirement on whatever sender is eventually built. This is a +// design contract, not a claim about current system behaviour: exactly as the next paragraph notes, +// there is no telephony provider yet to make it true or false, so do not read this line as evidence +// that anything is presently being discarded. Fixed round 1 (Minor 6, 2026-08-24): this line used to +// read as a settled operational fact, which is precisely the kind of unverifiable storage claim A2 +// removed from the patient-visible text below -- reworded here so the module's own rationale does +// not silently reintroduce it. +// +// Corrected 2026-08-24 under the owner's approved copy decisions (items A2 + A3, see +// docs/caring-contacts/phase-2b-sdd-archive/task-c-brief.md). Two defects, fixed together because +// they are one sentence: A2 -- "has not been seen by anyone and has not been kept" was a firm claim +// about storage, made to a person in distress, about a system with no telephony provider yet, so +// nobody could currently know whether it was true. The replacement says only what this system can +// actually know: who is not reading. A3 -- a patient told "no one reads replies" who then receives +// this very message could reasonably conclude somebody read theirs first. "and this reply is +// automatic" closes that. EXACT_PATIENT_VISIBLE_MESSAGE is deliberately NOT touched: it is 252 +// septets against the 2-segment ceiling with no room left, so this fact lives only here, where +// there is room -- see caring-contacts-message-copy.test.ts. +export const AUTOMATED_REPLY_RESPONSE = `No one at Example Aftercare Team reads this number, and this reply is automatic. To talk to someone, call ${FICTIONAL_CONTACTS_BY_ROLE.programmeStaffedLine}, 9 am-6 pm every day. In an emergency call 000. Fictional Support Line: ${FICTIONAL_CONTACTS_BY_ROLE.crisisSupportContact}.`; export const EXACT_MESSAGE_GSM7: Gsm7Evidence = calculateGsm7(EXACT_PATIENT_VISIBLE_MESSAGE); export const AUTOMATED_REPLY_GSM7: Gsm7Evidence = calculateGsm7(AUTOMATED_REPLY_RESPONSE); diff --git a/src/lib/caring-contacts/message-policy.ts b/src/lib/caring-contacts/message-policy.ts index b8ef7689fc..d848054d20 100644 --- a/src/lib/caring-contacts/message-policy.ts +++ b/src/lib/caring-contacts/message-policy.ts @@ -52,11 +52,20 @@ export type GovernedMessageInput = { messageType: MessageType; /** The recipient's own mobile number, if known, so it can be checked for leakage into the text. */ patientMobileNumber?: string; + /** + * Explicit, greppable acknowledgement that a reserved fictional contact detail (see + * message-rules.ts's `fictionalContactMarkerPattern`) in `text` is known to be synthetic. + * Ruling 79 (item A1, 2026-08-24): defaults to false/absent, which means the message is + * REFUSED whenever it matches that pattern. There is deliberately no way to silence the check + * other than passing this flag at the call site. + */ + syntheticFictionalContactsAcknowledged?: boolean; }; export type MessageValidationIssue = | { code: "exceeds-two-segments"; septets: number; segments: number } | { code: "prohibited-term"; term: string } + | { code: "fictional-contact-detail-present" } | { code: "first-message-missing-support-information" } | { code: "closing-message-missing-ending-statement" } | { code: "closing-message-missing-support-information" } @@ -81,11 +90,22 @@ export function validateGovernedMessage(input: GovernedMessageInput): Validation const lowerText = text.toLowerCase(); for (const term of rules.prohibitedTerms) { - if (lowerText.includes(term.toLowerCase())) { + // B2: a term with a pattern override (today, only "lead") is matched by that pattern instead + // of plain substring inclusion. Every other term's behaviour is unchanged. + const override = rules.prohibitedTermPatternOverrides[term]; + const matched = override ? override.test(text) : lowerText.includes(term.toLowerCase()); + if (matched) { issues.push({ code: "prohibited-term", term }); } } + // Ruling 79 (item A1): always reported when the marker pattern matches, unless explicitly + // acknowledged at the call site. See message-rules.ts's `fictionalContactMarkerPattern` doc + // comment for why this is a pattern (label OR number) rather than a single string. + if (!input.syntheticFictionalContactsAcknowledged && rules.fictionalContactMarkerPattern.test(text)) { + issues.push({ code: "fictional-contact-detail-present" }); + } + if (input.messageType === "first") { const hasFullSupportInformation = text.includes(rules.programmeLine) && @@ -117,3 +137,35 @@ export function validateGovernedMessage(input: GovernedMessageInput): Validation return issues.length === 0 ? { valid: true } : { valid: false, issues }; } + +export type ClosingMessageBodyIssue = { code: "closing-message-body-not-authored" }; + +export type ClosingMessageBodyResolution = { ok: true; body: string } | { ok: false; issue: ClosingMessageBodyIssue }; + +/** + * Resolves the outgoing text for a `closing` contact (item A4, 2026-08-24). + * + * No closing message has ever been written -- final wording is a clinical decision the owner has + * deferred to a lived-experience representative, not an implementation gap. A plan reaching its + * end today has nothing to send, and the only acceptable response to that is a loud, identifiable + * refusal: never an empty string, never a silent fall-back to some other message's text, and never + * a silently skipped contact. This is the refusal only -- it drafts no closing-message wording of + * its own, deliberately, because an implementer doing so would be the exact failure this exists to + * prevent. This is distinct from `closing-message-missing-ending-statement`: that code means a body + * exists but is wrong; this one means no body exists to check at all. + * + * No existing seam resolves a contact's message body anywhere in this domain today (checked + * schedule.ts, simulation.ts, repository.ts, model.ts) -- `PlannedContact` carries a `messageType` + * but no body content, and nothing yet supplies one. This function is the mechanism a future + * sender must call once that seam is built; it is not itself wired into the schedule or the + * simulation driver, because doing so would require inventing where an authored closing body comes + * from, which is exactly the decision this task defers. + */ +export function resolveClosingContactMessageBody( + authoredClosingBody: string | undefined, +): ClosingMessageBodyResolution { + if (!authoredClosingBody || authoredClosingBody.trim().length === 0) { + return { ok: false, issue: { code: "closing-message-body-not-authored" } }; + } + return { ok: true, body: authoredClosingBody }; +} diff --git a/src/lib/caring-contacts/message-rules.ts b/src/lib/caring-contacts/message-rules.ts index af20e99702..555c478d06 100644 --- a/src/lib/caring-contacts/message-rules.ts +++ b/src/lib/caring-contacts/message-rules.ts @@ -3,6 +3,12 @@ // constants, and the two-segment GSM-7 limit. Replace this file wholesale when the clinical programme // lead and lived-experience representative approve the real content style guide. Do not edit // message-policy.ts to accommodate a rule change — the mechanism is stable, the rules are data. +import { DESIGNATED_FICTIONAL_MOBILE_NUMBERS } from "./synthetic-contacts"; + +/** Escapes `value` for literal use inside a `RegExp` source string. */ +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} export type ProvisionalMessageRules = { /** Above this many GSM-7 segments, a message fails exceeds-two-segments. */ @@ -17,10 +23,63 @@ export type ProvisionalMessageRules = { emergencyDirection: string; /** The one crisis-support contact; required in first and closing messages. */ crisisSupportContact: string; + /** + * Identifies a reserved fictional contact detail inside a message. NOT in `prohibitedTerms` -- + * Ruling 79 (item A1, 2026-08-24): both approved patient-visible messages contain + * `crisisSupportContact` today, so a bare prohibition on this text would make every existing + * message invalid. message-policy.ts instead reports `fictional-contact-detail-present` + * whenever a message matches this pattern, unless the caller explicitly acknowledges the + * number is synthetic. See task-c-brief.md, "A1". + * + * Fix round 1 (promoted finding, 2026-08-24): the first version was `crisisSupportContact. + * split(":")[0]` -- the LABEL "Fictional Support Line" only. A message carrying the reserved + * NUMBER with no label raised nothing, which is precisely the shape that would reach a real + * sender: the number is the dangerous artefact, not the label. This is now a pattern matching + * "Fictional" (any case) OR any one of `synthetic-contacts.ts`'s reserved fictional numbers, so + * relabelling ("Fictional Support Line (24h): …"), reordering ("… (Fictional Support Line)"), + * or dropping the label entirely (just the bare number) are all still caught -- either half of + * the pair alone is sufficient. + */ + fictionalContactMarkerPattern: RegExp; + /** + * Per-term overrides for how a `prohibitedTerms` entry is matched, keyed by the exact term + * string. A term with no entry here keeps the default plain-substring match. B2 (2026-08-24): + * "lead" is the only entry -- `lowerText.includes("lead")` also matched the ordinary English + * "the incident lead" / "the clinical programme lead" (job titles in the service-stop wording). + * This is deliberately NOT extended to the rest of the list: several other terms are multi-word + * phrases whose substring behaviour is intentional. + * + * Fix round 1: the first version of this override ALLOWLISTED nine commercial + * modifiers/companions ("sales lead", "lead generation", ...). Commercial vocabulary for "lead" + * is open-ended, so that allowlist was itself a defect -- "lead nurturing", "lead magnet", + * "qualify this lead" and similar phrasing all passed silently. The current version inverts + * this: it refuses "lead"/"leads" as a whole word BY DEFAULT, and exempts only the closed, + * small set of job titles this domain's own wording ever uses. See task-c-brief.md, "B2", and + * task-c-report.md's "fix round 1" section. + */ + prohibitedTermPatternOverrides: Readonly>>; /** States that the message is the last one the recipient will receive. */ closingStatement: string; }; +const CRISIS_SUPPORT_CONTACT = "Fictional Support Line: +61 491 570 158"; + +// A1, fix round 1: "Fictional" (label, any case) OR any reserved fictional number from +// synthetic-contacts.ts. Either half alone is sufficient, so relabelling the crisis contact, +// reordering it, or dropping the label and keeping only the bare number are all still caught. +const FICTIONAL_CONTACT_MARKER_PATTERN = new RegExp( + ["Fictional", ...DESIGNATED_FICTIONAL_MOBILE_NUMBERS.map(escapeRegExp)].join("|"), + "i", +); + +// B2, fix round 1: refuses "lead"/"leads" as a whole word BY DEFAULT (a negative lookbehind, not +// an allowlist of commercial phrasing), exempting only this domain's closed set of job titles -- +// "incident lead", "programme lead", "clinical lead", "team lead", "service lead". A job title is +// exempted only when the qualifying word sits IMMEDIATELY before "lead"/"leads" (so "clinical +// programme lead" is still exempt: "programme lead" is the qualifying pair actually adjacent to +// the word), never merely because one of those words appears anywhere earlier in the message. +const COMMERCIAL_LEAD_PATTERN = /(? { expect(EXACT_MESSAGE_GSM7).toEqual({ invalidCharacters: [], segments: 2, septets: 252, valid: true }); // The automated reply (production-build spec §2.1) is patient-visible too, so it carries the same // two-segment ceiling and the same prohibition on echoing a patient mobile number. + // 218 -> 210 septets, owner-approved 2026-08-24 (items A2 + A3, see + // docs/caring-contacts/phase-2b-sdd-archive/task-c-brief.md). expect(calculateGsm7(AUTOMATED_REPLY_RESPONSE)).toEqual({ invalidCharacters: [], segments: 2, - septets: 218, + septets: 210, valid: true, }); expect(AUTOMATED_REPLY_RESPONSE).toContain(FICTIONAL_CONTACTS_BY_ROLE.programmeStaffedLine); diff --git a/tests/caring-contacts-empty-state.dom.test.tsx b/tests/caring-contacts-empty-state.dom.test.tsx new file mode 100644 index 0000000000..27e184459a --- /dev/null +++ b/tests/caring-contacts-empty-state.dom.test.tsx @@ -0,0 +1,165 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; + +import { ListEmptyState } from "@/components/caring-contacts/workspace/list-empty-state"; + +describe("ListEmptyState — no-data", () => { + it("renders the heading and the explanation, and no Why/What-changes-it pair", () => { + const { container } = render( + , + ); + expect(screen.getByText("No patients yet")).toBeInTheDocument(); + expect(screen.getByText("Add the first patient to get started.")).toBeInTheDocument(); + // The "filtered" wording shape must never leak onto a genuinely empty list — + // that would misstate a caseload of zero as a caseload hidden by a filter. + expect(container.textContent ?? "").not.toContain("Why:"); + expect(container.textContent ?? "").not.toContain("What changes it:"); + }); + + it("renders no action when none is given", () => { + const { container } = render( + , + ); + expect(container.querySelector("a, button")).toBeNull(); + }); + + it("renders the given action, and it is genuinely actionable", () => { + render( + Add a patient} + />, + ); + const action = screen.getByRole("link", { name: "Add a patient" }); + expect(action).toHaveAttribute("href", "/caring-contacts/patients/new"); + }); + + it("names the whole state with a role=group, the same way automated-state.tsx does", () => { + render(); + const region = screen.getByRole("group", { name: "No patients yet" }); + expect(region).toHaveTextContent("Nothing here."); + }); +}); + +describe("ListEmptyState — filtered", () => { + it("renders both the reason and the remedy in the page, never in a title alone", () => { + // Mirrors how tests/caring-contacts-explained-automation.dom.test.tsx proves + // AutomatedState's reason and remedy are not tooltip-only: nothing here may + // be reachable only by hovering a `title`. + const { container } = render( + , + ); + expect(container.textContent ?? "").toContain("Why:"); + expect(container.textContent ?? "").toContain("The status filter is set to Discharged this week."); + expect(container.textContent ?? "").toContain("What changes it:"); + expect(container.textContent ?? "").toContain("Clear the status filter to see the rest of the caseload."); + for (const node of container.querySelectorAll("[title]")) { + expect(node.getAttribute("title")).not.toContain("The status filter is set to Discharged this week."); + expect(node.getAttribute("title")).not.toContain("Clear the status filter to see the rest of the caseload."); + } + }); + + it("cannot be constructed without a reason and a remedy", () => { + // A type-level guarantee, checked by `tsc --noEmit` rather than at runtime: + // the discriminated union makes `because`/`changedBy` required the moment + // `kind` is "filtered", so an omission is a compile error, not a judgement + // call left to whoever writes the next list screen. + const omittedBecause = ( + // @ts-expect-error "filtered" requires `because` — it cannot be left out. + + ); + const omittedChangedBy = ( + // @ts-expect-error "filtered" requires `changedBy` — it cannot be left out. + + ); + expect(omittedBecause).toBeTruthy(); + expect(omittedChangedBy).toBeTruthy(); + }); + + it("renders the given action, and it is genuinely actionable", () => { + render( + Clear filter} + />, + ); + const action = screen.getByRole("link", { name: "Clear filter" }); + expect(action).toHaveAttribute("href", "/caring-contacts/patients"); + }); + + it("groups the heading, the reason and the remedy under one role=group named by the heading", () => { + // Ruling 81 forbade rendering AutomatedState, not reusing the accessible + // structure that makes AutomatedState's reason and remedy reachable + // together: a screen reader that reaches this named group finds "Why:" + // and "What changes it:" without hunting elsewhere on the page. + render( + , + ); + const region = screen.getByRole("group", { name: "No patients match" }); + expect(region).toHaveTextContent("The status filter is set to Discharged this week."); + expect(region).toHaveTextContent("Clear the status filter to see the rest of the caseload."); + }); +}); + +describe("ListEmptyState — rendering", () => { + it("renders correctly inside a 320px-wide container", () => { + const { container } = render( +
+ +
, + ); + // Text wraps rather than escaping its measure — the same contract + // `automated-state.tsx` holds with `max-w-[var(--measure)]`. + for (const paragraph of container.querySelectorAll("p")) { + if (paragraph.textContent && paragraph.textContent.length > 40) { + expect(paragraph.className).toContain("max-w-[var(--measure)]"); + } + } + expect(screen.getByText("No patients match")).toBeInTheDocument(); + }); + + it("keeps its border visible under forced colours, the way automated-state.tsx does", () => { + // jsdom cannot emulate `forced-colors: active`; real browser proof for this + // family of guarantee lives in tests/ui-caring-contacts-workspace.spec.ts. + // What this DOM test can and does prove is that the override class this + // repo's forced-colors contract depends on is actually present in the + // rendered markup, not merely written somewhere in the source. + const { container } = render( + , + ); + const region = container.firstElementChild; + expect(region).not.toBeNull(); + expect(region!.className).toContain("forced-colors:border-[CanvasText]"); + }); + + it("never carries the state by colour alone: the icon is decorative and the words are the state", () => { + const { container } = render( + , + ); + const icons = container.querySelectorAll("svg"); + expect(icons, "no-data renders no icon").toHaveLength(1); + for (const icon of icons) { + expect(icon.getAttribute("aria-hidden")).toBe("true"); + } + }); +}); diff --git a/tests/caring-contacts-explained-automation.dom.test.tsx b/tests/caring-contacts-explained-automation.dom.test.tsx index cbecf61de6..6827a5c068 100644 --- a/tests/caring-contacts-explained-automation.dom.test.tsx +++ b/tests/caring-contacts-explained-automation.dom.test.tsx @@ -248,6 +248,14 @@ const ALLOWED_CLIENT_COMPONENTS = [ // props cannot cross a Server → Client boundary. It takes no props at all, which is // what keeps the service-state record on the server side of this seam. "overlays/workspace-overlays.tsx", + // Task 3's control: the button a screen renders to raise one of the 24 overlays. A click + // handler is by definition a client capability, so this cannot be a Server Component. + // Added on the same three conditions as the entries above: its props are an overlay id, a + // class name, children, and a `WorkspaceOverlayCommit` — an intent union of a callback and + // a plain-words reason string, never a state object and nothing derived from the record; + // the companion test below proves its source and everything it reaches never name that + // module or type; and it is here deliberately rather than to clear a red test. + "overlays/overlay-trigger.tsx", // Decides WHEN the condensed stop bar is shown, and never what it says. A scroll position // and two element rectangles are browser facts, so this one cannot be answered on the // server — and the header is not the height of its token (87.5px at 320/390, 65px above, diff --git a/tests/caring-contacts-interface-vocabulary.test.ts b/tests/caring-contacts-interface-vocabulary.test.ts new file mode 100644 index 0000000000..2c3bf9df2b --- /dev/null +++ b/tests/caring-contacts-interface-vocabulary.test.ts @@ -0,0 +1,243 @@ +// tests/caring-contacts-interface-vocabulary.test.ts +// +// Item B3 (2026-08-24): extend the prohibited-word scan to interface strings. +// +// Until now, the prohibition on words like "campaign", "engagement score", and "inbox" ran only +// against outgoing messages (message-policy.ts) and the 24 frozen overlay definition rows +// (caring-contacts-overlay-definitions.test.ts). Nothing checked the words on a SCREEN, so the ban +// on interface wording was policy held by people rather than by software. This scans every string +// and template literal under the workspace and caring-contacts app trees for the same wider +// interface vocabulary (CARING_CONTACTS_PROHIBITED_LANGUAGE) already used by the overlay tests. +// +// See docs/caring-contacts/phase-2b-sdd-archive/task-c-brief.md, "B3". +import { mkdtempSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +import { CARING_CONTACTS_PROHIBITED_LANGUAGE } from "./helpers/caring-contacts-prohibited-language"; + +// Deliberately narrow roots, not "all of src/components/caring-contacts": this excludes +// src/components/caring-contacts/mockups/** without needing a special-case skip. Mockups are +// frozen design scratch that 404 in production and knowingly contain one prohibited phrase the +// owner ruled (B4) to leave alone -- see task-c-brief.md, "B3", "Scope limits, deliberate". +const SCAN_ROOTS = [ + path.join(process.cwd(), "src", "components", "caring-contacts", "workspace"), + path.join(process.cwd(), "src", "app", "caring-contacts"), +]; + +function walk(dir: string): string[] { + return readdirSync(dir).flatMap((entry) => { + const full = path.join(dir, entry); + return statSync(full).isDirectory() ? walk(full) : full.endsWith(".ts") || full.endsWith(".tsx") ? [full] : []; + }); +} + +/** + * Every quoted string and template literal in `source`, with line/block comments skipped. + * + * A naive quote-matching regex over the raw source is unsound here: JSDoc comments in this tree + * use single backticks for inline code (`` `useSearchParams` ``), and an odd number of them across + * a comment block pairs a backtick from one inline-code span with one from a much later, unrelated + * span, capturing the entire stretch between as a single fake "template literal". A small + * character-by-character scan that recognises both line comments and block comments avoids that, + * and handles escaped quote characters inside real literals. + */ +function extractStringAndTemplateLiterals(source: string): string[] { + const literals: string[] = []; + let i = 0; + const n = source.length; + while (i < n) { + const two = source.slice(i, i + 2); + if (two === "//") { + const end = source.indexOf("\n", i); + i = end === -1 ? n : end + 1; + continue; + } + if (two === "/*") { + const end = source.indexOf("*/", i + 2); + i = end === -1 ? n : end + 2; + continue; + } + const ch = source[i]; + if (ch === '"' || ch === "'" || ch === "`") { + const quote = ch; + let j = i + 1; + let content = ""; + while (j < n) { + if (source[j] === "\\") { + content += source[j] + (source[j + 1] ?? ""); + j += 2; + continue; + } + if (source[j] === quote) { + j += 1; + break; + } + content += source[j]; + j += 1; + } + literals.push(content); + i = j; + continue; + } + i += 1; + } + return literals; +} + +/** + * `className` attribute VALUES are excluded before extraction. This is narrowing which literals + * reach the scan, not adding a file to an ignore list: a `className` value is CSS/Tailwind tokens, + * including custom-property names like `var(--safe-area-bottom)`, which contains "safe" as a + * substring of a CSS identifier with no relationship to interface prose a patient or clinician + * would read. Both real occurrences of `--safe-area-bottom` in this tree are inside `className` + * attributes (shell.tsx, overlay-host.tsx) -- confirmed while building this scan. + */ +const CLASSNAME_ATTRIBUTE_VALUE = /className\s*=\s*(?:"[^"]*"|'[^']*'|\{`[^`]*`\})/g; + +function extractInterfaceStrings(source: string): string[] { + return extractStringAndTemplateLiterals(source.replace(CLASSNAME_ATTRIBUTE_VALUE, "")); +} + +/** + * `source` with only comments and `className` attribute values removed -- everything else is + * left exactly as written, including plain JSX text nodes that carry no quotes at all. + * + * Fix round 1 (Important 3): `extractInterfaceStrings` above only sees quoted/template-literal + * strings, but this tree writes copy the OTHER way too -- as plain JSX text between tags (e.g. + * shell.tsx's `Caring Contacts`, loading.tsx's `

Loading the + * Caring Contacts workspace

`). `

Check your inbox for the latest campaign.

` extracted + * nothing and scored zero offences under the quote-only scan, while the same words wrapped as + * `

{"..."}

` were caught -- this function is the second pass that closes that gap. + */ +function stripCommentsAndClassNameValues(source: string): string { + const withoutClassNames = source.replace(CLASSNAME_ATTRIBUTE_VALUE, ""); + let result = ""; + let i = 0; + const n = withoutClassNames.length; + while (i < n) { + const two = withoutClassNames.slice(i, i + 2); + if (two === "//") { + const end = withoutClassNames.indexOf("\n", i); + i = end === -1 ? n : end + 1; + continue; + } + if (two === "/*") { + const end = withoutClassNames.indexOf("*/", i + 2); + i = end === -1 ? n : end + 2; + continue; + } + result += withoutClassNames[i]; + i += 1; + } + return result; +} + +// A `g`-flagged copy of the shared vocabulary regex, so `matchAll` can enumerate every hit in the +// raw-prose pass rather than only reporting the first (`CARING_CONTACTS_PROHIBITED_LANGUAGE` +// itself stays non-global, matching how the quoted-literal pass above uses `.test()` on it). +const CARING_CONTACTS_PROHIBITED_LANGUAGE_GLOBAL = new RegExp( + CARING_CONTACTS_PROHIBITED_LANGUAGE.source, + CARING_CONTACTS_PROHIBITED_LANGUAGE.flags.includes("g") + ? CARING_CONTACTS_PROHIBITED_LANGUAGE.flags + : `${CARING_CONTACTS_PROHIBITED_LANGUAGE.flags}g`, +); + +/** Every prohibited-vocabulary match found directly in the comment/className-stripped source. */ +function scanRawProseForProhibitedLanguage(source: string): string[] { + const stripped = stripCommentsAndClassNameValues(source); + return [...stripped.matchAll(CARING_CONTACTS_PROHIBITED_LANGUAGE_GLOBAL)].map((match) => match[0]); +} + +/** Every offence in one file: quoted/template-literal strings, plus raw prose (JSX text). */ +function scanOneFileForProhibitedLanguage(file: string): string[] { + const source = readFileSync(file, "utf8"); + const relativePath = path.relative(process.cwd(), file); + const offences: string[] = []; + for (const literal of extractInterfaceStrings(source)) { + if (CARING_CONTACTS_PROHIBITED_LANGUAGE.test(literal)) { + offences.push(`${relativePath}: ${JSON.stringify(literal)}`); + } + } + for (const match of scanRawProseForProhibitedLanguage(source)) { + offences.push(`${relativePath} (raw prose, e.g. JSX text): ${JSON.stringify(match)}`); + } + return offences; +} + +/** Every `root -> file -> offending literal` combination found under `root`. */ +function scanRootForProhibitedLanguage(root: string): string[] { + return walk(root).flatMap((file) => scanOneFileForProhibitedLanguage(file)); +} + +describe("caring-contacts interface vocabulary (B3)", () => { + it("finds a deliberately planted prohibited string in a fixture -- proving the scan can fail", () => { + // Runs the real file-scanning code path (walk + read + extract + match) against a temporary + // fixture, not just the extraction function in isolation. "A scan that cannot fail is worse + // than no scan" (task-c-brief.md) -- this is that proof. + const fixtureDir = mkdtempSync(path.join(tmpdir(), "caring-contacts-interface-vocabulary-fixture-")); + try { + writeFileSync( + path.join(fixtureDir, "planted-banner.tsx"), + [ + "export function PlantedBanner() {", + ' return

{"Following up on your sales lead from this campaign."}

;', + "}", + "", + ].join("\n"), + "utf8", + ); + + const offences = scanRootForProhibitedLanguage(fixtureDir); + expect(offences.length).toBeGreaterThan(0); + expect(offences.some((offence) => offence.includes("campaign"))).toBe(true); + } finally { + rmSync(fixtureDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + // Fix round 1 (Important 3): the extractor above only sees quoted/template-literal strings, but + // this tree writes copy the OTHER way too -- as plain JSX text between tags with no quotes at + // all (e.g. shell.tsx's `Caring Contacts`, loading.tsx's `

+ // Loading the Caring Contacts workspace

`). `

Check your inbox for the latest + // campaign.

` extracted nothing and scored zero offences before this fix, while the same + // words wrapped as `

{"..."}

` were caught -- an inconsistency the fixture below is + // deliberately shaped to close, not to lean on. + it("finds a prohibited word planted as PLAIN JSX TEXT, not just inside quotes", () => { + const fixtureDir = mkdtempSync(path.join(tmpdir(), "caring-contacts-interface-vocabulary-jsx-text-fixture-")); + try { + writeFileSync( + path.join(fixtureDir, "planted-plain-text-banner.tsx"), + [ + "export function PlantedPlainTextBanner() {", + " return

Check your inbox for the latest campaign.

;", + "}", + "", + ].join("\n"), + "utf8", + ); + + const offences = scanRootForProhibitedLanguage(fixtureDir); + expect(offences.length).toBeGreaterThan(0); + expect(offences.some((offence) => offence.includes("inbox"))).toBe(true); + expect(offences.some((offence) => offence.includes("campaign"))).toBe(true); + } finally { + rmSync(fixtureDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + }); + + it("finds nothing in the real workspace and caring-contacts app tree", () => { + // Minor 7: a root that exists but holds no .ts/.tsx file would otherwise pass this test + // vacuously -- close that with a floor on how many files were actually read. + let filesScanned = 0; + const offences = SCAN_ROOTS.flatMap((root) => { + const files = walk(root); + filesScanned += files.length; + return files.flatMap((file) => scanOneFileForProhibitedLanguage(file)); + }); + expect(filesScanned).toBeGreaterThan(0); + expect(offences).toEqual([]); + }); +}); diff --git a/tests/caring-contacts-message-copy.test.ts b/tests/caring-contacts-message-copy.test.ts index fe8900163b..da62f3337d 100644 --- a/tests/caring-contacts-message-copy.test.ts +++ b/tests/caring-contacts-message-copy.test.ts @@ -16,7 +16,9 @@ import { describe("caring-contacts patient-visible copy", () => { it("keeps the pinned GSM-7 evidence for both patient-visible strings", () => { expect(EXACT_MESSAGE_GSM7).toEqual({ invalidCharacters: [], segments: 2, septets: 252, valid: true }); - expect(AUTOMATED_REPLY_GSM7).toEqual({ invalidCharacters: [], segments: 2, septets: 218, valid: true }); + // 218 -> 210 septets, owner-approved 2026-08-24 (items A2 + A3): the first sentence was + // replaced, see the "A2 + A3" describe block below for the full covering tests. + expect(AUTOMATED_REPLY_GSM7).toEqual({ invalidCharacters: [], segments: 2, septets: 210, valid: true }); }); it("derives its evidence from the single domain GSM-7 calculator", () => { @@ -56,3 +58,42 @@ describe("caring-contacts patient-visible copy", () => { expect(DESIGNATED_FICTIONAL_MOBILE_NUMBERS).toHaveLength(4); }); }); + +// --------------------------------------------------------------------------- +// A2 + A3 (owner-approved 2026-08-24) — the automated reply no longer makes a storage claim +// nobody can currently verify, and it now tells the recipient the reply they are reading is +// automatic, so a person who was just told "no one reads this" cannot mistake a reply for a +// human response. See docs/caring-contacts/phase-2b-sdd-archive/task-c-brief.md. +// --------------------------------------------------------------------------- +describe("caring-contacts automated reply wording (A2 + A3, 2026-08-24)", () => { + it("matches the owner-approved text exactly", () => { + expect(AUTOMATED_REPLY_RESPONSE).toBe( + "No one at Example Aftercare Team reads this number, and this reply is automatic. To talk to someone, " + + `call ${FICTIONAL_CONTACTS_BY_ROLE.programmeStaffedLine}, 9 am-6 pm every day. In an emergency call 000. ` + + `Fictional Support Line: ${FICTIONAL_CONTACTS_BY_ROLE.crisisSupportContact}.`, + ); + }); + + it("stays within the two-segment GSM-7 ceiling (measured, not assumed)", () => { + const evidence = calculateGsm7(AUTOMATED_REPLY_RESPONSE); + expect(evidence.segments).toBe(2); + expect(evidence.valid).toBe(true); + expect(evidence.septets).toBe(210); + }); + + it("A2: drops the storage claim nobody can currently verify", () => { + expect(AUTOMATED_REPLY_RESPONSE).not.toContain("has not been kept"); + expect(AUTOMATED_REPLY_RESPONSE).not.toContain("has not been seen by anyone"); + }); + + it("A3: tells the recipient this reply itself is automatic", () => { + expect(AUTOMATED_REPLY_RESPONSE).toContain("automatic"); + }); + + it("leaves EXACT_PATIENT_VISIBLE_MESSAGE untouched by the A2/A3 wording change", () => { + // Message A is 252 septets against the 2-segment ceiling -- deliberately not touched here. + // See message-copy.ts's own comment and task-c-brief.md, "A2 + A3", for why. + expect(EXACT_PATIENT_VISIBLE_MESSAGE).not.toContain("this reply is automatic"); + expect(calculateGsm7(EXACT_PATIENT_VISIBLE_MESSAGE).septets).toBe(252); + }); +}); diff --git a/tests/caring-contacts-message-policy.test.ts b/tests/caring-contacts-message-policy.test.ts index f4ffb87677..d5543cf764 100644 --- a/tests/caring-contacts-message-policy.test.ts +++ b/tests/caring-contacts-message-policy.test.ts @@ -4,12 +4,15 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; +import { AUTOMATED_REPLY_RESPONSE, EXACT_PATIENT_VISIBLE_MESSAGE } from "@/lib/caring-contacts/message-copy"; import { PROVISIONAL_MESSAGE_RULES } from "@/lib/caring-contacts/message-rules"; import { calculateGsm7, + resolveClosingContactMessageBody, validateGovernedMessage, type GovernedMessageInput, } from "@/lib/caring-contacts/message-policy"; +import { DESIGNATED_FICTIONAL_MOBILE_NUMBERS } from "@/lib/caring-contacts/synthetic-contacts"; const rules = PROVISIONAL_MESSAGE_RULES; @@ -131,31 +134,241 @@ describe("rule 3: prohibited-term", () => { }); }); +// --------------------------------------------------------------------------- +// Rule 3b — fictional-contact-detail-present (Ruling 79 / item A1, 2026-08-24) +// +// "Fictional" is deliberately NOT in prohibitedTerms: both approved patient-visible messages +// contain "Fictional Support Line" today, so that would make every existing message invalid and +// the check would have to be disabled to ship -- a disabled check is worse than no check. Instead +// this issue is always reported unless the caller explicitly acknowledges the number is synthetic, +// so the day a real send path is built, someone must consciously pass a flag whose name says it is +// synthetic, or remove the fictional numbers. See docs/caring-contacts/phase-2b-sdd-archive/ +// task-c-brief.md, "A1". +// --------------------------------------------------------------------------- + +describe("rule 3b: fictional-contact-detail-present", () => { + it("fails with exactly that issue code when the fictional crisis contact is present and unacknowledged", () => { + const input: GovernedMessageInput = { text: rules.crisisSupportContact, messageType: "standard" }; + const result = validateGovernedMessage(input); + expect(result).toEqual({ valid: false, issues: [{ code: "fictional-contact-detail-present" }] }); + }); + + it("passes the same message when syntheticFictionalContactsAcknowledged is true", () => { + const input: GovernedMessageInput = { + text: rules.crisisSupportContact, + messageType: "standard", + syntheticFictionalContactsAcknowledged: true, + }; + expect(validateGovernedMessage(input)).toEqual({ valid: true }); + }); + + it("does not raise the issue for a message with no fictional contact marker, acknowledged or not", () => { + const plain: GovernedMessageInput = { text: "Thinking of you today.", messageType: "standard" }; + expect(validateGovernedMessage(plain)).toEqual({ valid: true }); + expect(validateGovernedMessage({ ...plain, syntheticFictionalContactsAcknowledged: true })).toEqual({ + valid: true, + }); + }); + + it("the two approved patient-visible messages pass once the fictional-contact acknowledgement is given", () => { + // The prototype's real callers: both approved messages name the fictional numbers on purpose + // (see message-copy.ts), so both must pass validateGovernedMessage only when the caller + // explicitly acknowledges that -- never silently. + for (const text of [EXACT_PATIENT_VISIBLE_MESSAGE, AUTOMATED_REPLY_RESPONSE]) { + const unacknowledged = validateGovernedMessage({ text, messageType: "standard" }); + expect(unacknowledged.valid).toBe(false); + if (unacknowledged.valid) throw new Error("unreachable"); + expect(unacknowledged.issues).toContainEqual({ code: "fictional-contact-detail-present" }); + + expect( + validateGovernedMessage({ text, messageType: "standard", syntheticFictionalContactsAcknowledged: true }).valid, + ).toBe(true); + } + }); + + // Fix round 1 (promoted finding): the original marker was `crisisSupportContact.split(":")[0]`, + // i.e. the LABEL "Fictional Support Line" only. A message carrying the bare reserved NUMBER with + // no label raised nothing -- exactly the shape that would reach a real sender. The marker is now + // a pattern matching "Fictional" (any case) OR any reserved fictional number from + // synthetic-contacts.ts, so either half alone is still caught. + it("still refuses a relabelled crisis contact that keeps both 'Fictional' and the number", () => { + const text = "Fictional Support Line (24h): +61 491 570 158"; + const result = validateGovernedMessage({ text, messageType: "standard" }); + expect(result).toEqual({ valid: false, issues: [{ code: "fictional-contact-detail-present" }] }); + }); + + it("still refuses a reordered crisis contact with no colon after the label", () => { + const text = "+61 491 570 158 (Fictional Support Line)"; + const result = validateGovernedMessage({ text, messageType: "standard" }); + expect(result).toEqual({ valid: false, issues: [{ code: "fictional-contact-detail-present" }] }); + }); + + it("refuses the bare reserved number with the 'Fictional' label removed entirely -- the dangerous shape", () => { + const text = "Call +61 491 570 158 for support."; + const result = validateGovernedMessage({ text, messageType: "standard" }); + expect(result).toEqual({ valid: false, issues: [{ code: "fictional-contact-detail-present" }] }); + }); + + it("refuses a message carrying any one of the four reserved fictional numbers, labelled or not", () => { + for (const number of DESIGNATED_FICTIONAL_MOBILE_NUMBERS) { + const result = validateGovernedMessage({ text: `Reach out on ${number}.`, messageType: "standard" }); + expect(result).toEqual({ valid: false, issues: [{ code: "fictional-contact-detail-present" }] }); + } + }); + + it("all of the above pass once acknowledged", () => { + for (const text of [ + "Fictional Support Line (24h): +61 491 570 158", + "+61 491 570 158 (Fictional Support Line)", + "Call +61 491 570 158 for support.", + ]) { + expect( + validateGovernedMessage({ text, messageType: "standard", syntheticFictionalContactsAcknowledged: true }), + ).toEqual({ valid: true }); + } + }); +}); + +// --------------------------------------------------------------------------- +// Rule 3c — B2 (2026-08-24, fix round 1): "lead" refuses by default, exempting only a closed +// set of job titles +// +// `lowerText.includes("lead")` also matches the ordinary English "the incident lead" and "the +// clinical programme lead" -- job titles that appear in the service-stop wording -- which would +// reject a message for using a word correctly. +// +// The FIRST version of this fix was itself defective: it allowlisted nine commercial +// modifiers/companions ("sales lead", "lead generation", ...), and commercial vocabulary is +// open-ended, so anything not on that list passed silently ("lead nurturing", "lead magnet", +// "qualify this lead", ...). Fix round 1 inverts it: "lead"/"leads" is refused by default (a +// whole-word match), and only the closed, small set of job titles this domain actually uses is +// exempted -- "incident lead", "programme lead", "clinical lead", "team lead", "service lead". +// That set is closed in a way commercial phrasing for "lead" never is. +// +// Only this one term is narrowed; every other prohibited term keeps its deliberate substring +// behaviour (several are multi-word phrases whose substring matching is intentional). See +// docs/caring-contacts/phase-2b-sdd-archive/task-c-brief.md, "B2", and task-c-report.md's +// "fix round 1" section. +// --------------------------------------------------------------------------- + +describe('rule 3c: "lead" refuses by default, exempting only known job titles (B2)', () => { + it("accepts every job title in the closed exemption set", () => { + for (const text of [ + "Please contact the incident lead for an update.", + "This was escalated to the programme lead.", + "This was escalated to the clinical programme lead.", + "Speak to the clinical lead about this.", + "The team lead approved the change.", + "Contact the service lead for details.", + ]) { + expect(validateGovernedMessage({ text, messageType: "standard" })).toEqual({ valid: true }); + } + }); + + it("rejects open-ended commercial/CRM phrasing that a fixed allowlist would have missed", () => { + // Every one of these was newly PERMITTED by the fix-round-1 review's allowlist version of + // this override -- none of them contain "sales", "marketing", "generation", "conversion", or + // any of the other nine modifiers/companions that version enumerated. + for (const text of [ + "We are focused on lead nurturing this quarter.", + "Check out our new lead magnet.", + "The lead source was social media.", + "Update the leads database today.", + "Our lead gen numbers are strong.", + "Please qualify this lead.", + "Please convert the lead.", + "This lead is hot.", + "Reach out to your lead.", + // Still rejects the phrasings the original (allowlist) version DID catch, too. + "Following up on your sales lead from last week.", + "This campaign generated 50 new leads.", + "Our lead generation numbers are up this quarter.", + // "lead score" (no "g") -- the allowlist version's "scoring?" alternative matched + // "scoring" but not "score", missing this exact phrase. Confirms the typo did not survive + // the inversion: there is no companion-word list left to carry it. + "This platform assigns a lead score to every contact.", + ]) { + const result = validateGovernedMessage({ text, messageType: "standard" }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("unreachable"); + expect(result.issues).toContainEqual({ code: "prohibited-term", term: "lead" }); + } + }); + + it('pins the override map to exactly one entry -- "lead" -- so a future override for another term cannot land unnoticed', () => { + expect(Object.keys(rules.prohibitedTermPatternOverrides)).toEqual(["lead"]); + }); + + it("leaves every OTHER prohibited term's substring behaviour exactly as it was", () => { + // One case per term (excluding "lead", covered above), because narrowing one term is + // precisely the change most likely to quietly widen what the rest of the list allows. + const exampleTextByTerm: Record = { + "high risk": "This patient is at high risk today.", + // "safe" as a substring of "unsafe" -- proves substring matching, not word-boundary matching. + safe: "The unsafe practice was flagged.", + "engagement score": "Your engagement score has changed.", + campaign: "Part of a campaign this month.", + // "conversion" as a substring of "reconversion" -- same proof. + conversion: "The reconversion rate improved.", + "best match": "This is the best match available.", + inbox: "Check your inbox for updates.", + conversation: "Let's have a conversation about this.", + }; + for (const term of rules.prohibitedTerms) { + if (term === "lead") continue; // deliberately narrowed above -- not part of this proof + const text = exampleTextByTerm[term]; + if (text === undefined) { + throw new Error(`no example text seeded for prohibited term ${JSON.stringify(term)} in this test`); + } + const result = validateGovernedMessage({ text, messageType: "standard" }); + expect(result.valid).toBe(false); + if (result.valid) throw new Error("unreachable"); + expect(result.issues).toContainEqual({ code: "prohibited-term", term }); + } + }); +}); + // --------------------------------------------------------------------------- // Rule 4 — a first message must carry full support information // --------------------------------------------------------------------------- describe("rule 4: first-message-missing-support-information", () => { it("passes a first message that contains the programme line, hours, emergency direction, and a crisis contact", () => { - const input: GovernedMessageInput = { text: compliantFirstMessage, messageType: "first" }; + const input: GovernedMessageInput = { + text: compliantFirstMessage, + messageType: "first", + syntheticFictionalContactsAcknowledged: true, + }; expect(validateGovernedMessage(input)).toEqual({ valid: true }); }); it("fails when the programme line is missing", () => { const text = [rules.operatingHours, rules.emergencyDirection, rules.crisisSupportContact].join(". "); - const result = validateGovernedMessage({ text, messageType: "first" }); + const result = validateGovernedMessage({ + text, + messageType: "first", + syntheticFictionalContactsAcknowledged: true, + }); expect(result).toEqual({ valid: false, issues: [{ code: "first-message-missing-support-information" }] }); }); it("fails when the hours are missing", () => { const text = [rules.programmeLine, rules.emergencyDirection, rules.crisisSupportContact].join(". "); - const result = validateGovernedMessage({ text, messageType: "first" }); + const result = validateGovernedMessage({ + text, + messageType: "first", + syntheticFictionalContactsAcknowledged: true, + }); expect(result).toEqual({ valid: false, issues: [{ code: "first-message-missing-support-information" }] }); }); it("fails when the emergency direction is missing", () => { const text = [rules.programmeLine, rules.operatingHours, rules.crisisSupportContact].join(". "); - const result = validateGovernedMessage({ text, messageType: "first" }); + const result = validateGovernedMessage({ + text, + messageType: "first", + syntheticFictionalContactsAcknowledged: true, + }); expect(result).toEqual({ valid: false, issues: [{ code: "first-message-missing-support-information" }] }); }); @@ -177,13 +390,21 @@ describe("rule 4: first-message-missing-support-information", () => { describe("rule 5: closing-message-missing-ending-statement / closing-message-missing-support-information", () => { it("passes a closing message with the ending statement, programme line, and crisis contact", () => { - const input: GovernedMessageInput = { text: compliantClosingMessage, messageType: "closing" }; + const input: GovernedMessageInput = { + text: compliantClosingMessage, + messageType: "closing", + syntheticFictionalContactsAcknowledged: true, + }; expect(validateGovernedMessage(input)).toEqual({ valid: true }); }); it("fails with closing-message-missing-ending-statement when the final-message statement is absent", () => { const text = [rules.programmeLine, rules.crisisSupportContact].join(". "); - const result = validateGovernedMessage({ text, messageType: "closing" }); + const result = validateGovernedMessage({ + text, + messageType: "closing", + syntheticFictionalContactsAcknowledged: true, + }); expect(result).toEqual({ valid: false, issues: [{ code: "closing-message-missing-ending-statement" }], @@ -192,7 +413,11 @@ describe("rule 5: closing-message-missing-ending-statement / closing-message-mis it("fails with closing-message-missing-support-information when the programme line is absent", () => { const text = [rules.closingStatement, rules.crisisSupportContact].join(". "); - const result = validateGovernedMessage({ text, messageType: "closing" }); + const result = validateGovernedMessage({ + text, + messageType: "closing", + syntheticFictionalContactsAcknowledged: true, + }); expect(result).toEqual({ valid: false, issues: [{ code: "closing-message-missing-support-information" }], @@ -220,18 +445,88 @@ describe("rule 5: closing-message-missing-ending-statement / closing-message-mis }); }); +// --------------------------------------------------------------------------- +// Rule 5b — resolveClosingContactMessageBody (item A4, 2026-08-24) +// +// No closing message has ever been written -- that wording is a clinical decision deferred to a +// lived-experience representative, not an implementation gap. This is the refusal ONLY: no +// closing-message wording is drafted here or anywhere in this task. See +// docs/caring-contacts/phase-2b-sdd-archive/task-c-brief.md, "A4". +// --------------------------------------------------------------------------- + +describe("rule 5b: resolveClosingContactMessageBody / closing-message-body-not-authored", () => { + it("refuses with its own identifiable reason when no authored body exists", () => { + expect(resolveClosingContactMessageBody(undefined)).toEqual({ + ok: false, + issue: { code: "closing-message-body-not-authored" }, + }); + }); + + it("refuses the same way for an empty or whitespace-only authored body -- never an empty string, never silent", () => { + expect(resolveClosingContactMessageBody("")).toEqual({ + ok: false, + issue: { code: "closing-message-body-not-authored" }, + }); + expect(resolveClosingContactMessageBody(" ")).toEqual({ + ok: false, + issue: { code: "closing-message-body-not-authored" }, + }); + }); + + it("does not refuse a closing contact that does have an authored body", () => { + const result = resolveClosingContactMessageBody(compliantClosingMessage); + expect(result).toEqual({ ok: true, body: compliantClosingMessage }); + }); + + it("never falls back to the ordinary (non-closing) message text", () => { + // A body-not-authored refusal must never resolve to some other message's text -- it has no + // `body` field on the failure branch at all. + const result = resolveClosingContactMessageBody(undefined); + expect(result.ok).toBe(false); + if (result.ok) throw new Error("unreachable"); + expect(result).not.toHaveProperty("body"); + }); + + it("is a different failure from closing-message-missing-ending-statement (missing body vs. wrong body)", () => { + // A body that EXISTS but is wrong (lacks the required ending statement) is caught later, by + // validateGovernedMessage, with a different code -- proving the two failures are distinguishable. + const noBody = resolveClosingContactMessageBody(undefined); + expect(noBody).toEqual({ ok: false, issue: { code: "closing-message-body-not-authored" } }); + + const wrongBody = "Some closing text with no ending statement."; + const resolved = resolveClosingContactMessageBody(wrongBody); + expect(resolved).toEqual({ ok: true, body: wrongBody }); + const validated = validateGovernedMessage({ text: wrongBody, messageType: "closing" }); + expect(validated).toEqual({ + valid: false, + issues: [ + { code: "closing-message-missing-ending-statement" }, + { code: "closing-message-missing-support-information" }, + ], + }); + }); +}); + // --------------------------------------------------------------------------- // Rule 6 — a patient mobile number must never appear in the message // --------------------------------------------------------------------------- describe("rule 6: contains-patient-mobile", () => { it("fails when the message contains the patient's mobile number", () => { + // +61 491 570 006 is deliberately one of synthetic-contacts.ts's four reserved fictional + // numbers (miraPatientMobile) -- since fix round 1, that means it ALSO trips + // fictional-contact-detail-present (A1), because the marker pattern covers "every reserved + // number", not just the crisis/staffed-line ones. Both codes are correct here; this proves + // the two rules coexist rather than one silently masking the other. const input: GovernedMessageInput = { text: "Call us back on +61 491 570 006 any time.", messageType: "standard", patientMobileNumber: "+61 491 570 006", }; - expect(validateGovernedMessage(input)).toEqual({ valid: false, issues: [{ code: "contains-patient-mobile" }] }); + expect(validateGovernedMessage(input)).toEqual({ + valid: false, + issues: [{ code: "fictional-contact-detail-present" }, { code: "contains-patient-mobile" }], + }); }); it("passes when the patient's mobile number is known but absent from the text", () => { diff --git a/tests/caring-contacts-overlay-host.dom.test.tsx b/tests/caring-contacts-overlay-host.dom.test.tsx index 2600792003..8d16c064ac 100644 --- a/tests/caring-contacts-overlay-host.dom.test.tsx +++ b/tests/caring-contacts-overlay-host.dom.test.tsx @@ -1,7 +1,7 @@ import { readFileSync } from "node:fs"; import { resolve } from "node:path"; -import { act, cleanup, render, screen, waitFor } from "@testing-library/react"; +import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { useState } from "react"; import { describe, expect, it, vi } from "vitest"; @@ -16,6 +16,7 @@ import { WORKSPACE_OVERLAY_DEFINITIONS } from "@/components/caring-contacts/work import { closeWorkspaceOverlay, openWorkspaceOverlay, + openWorkspaceOverlayWithCommit, WorkspaceOverlays, } from "@/components/caring-contacts/workspace/overlays/workspace-overlays"; import { WORKSPACE_WIDTH_BREAKPOINTS, widthStateFor } from "@/components/caring-contacts/workspace/width-state"; @@ -62,6 +63,7 @@ function OverlayHarness({ overlayId }: { overlayId: string }) { onClose={() => setOpenOverlayId(null)} onCommit={noop} blockReason={null} + commitRefusal={null} /> ); @@ -76,7 +78,13 @@ describe("the overlay host", () => { ] as const) { setViewportWidth(width); const { unmount } = render( - , + , ); const body = screen.getByTestId("workspace-overlay-content"); expect(body, `${definition.id} at ${width}px`).toHaveAttribute("data-overlay-id", definition.id); @@ -110,6 +118,7 @@ describe("the overlay host", () => { onClose={noop} onCommit={noop} blockReason="permission-unavailable" + commitRefusal={null} />, ); expect( @@ -119,7 +128,13 @@ describe("the overlay host", () => { refused.unmount(); const live = render( - , + , ); if (definition.requiresFreshAuthentication) { await userEvent.click(screen.getByTestId("workspace-overlay-action")); @@ -243,7 +258,15 @@ describe("the overlay host", () => { (definition) => definition.phoneModality === "bottom-sheet", ); expect(bottomSheetRow, "the frozen table no longer has a bottom-sheet row").toBeDefined(); - render(); + render( + , + ); expect(screen.getByTestId("workspace-overlay-content")).toHaveAttribute("data-overlay-modality", "bottom-sheet"); }); @@ -263,7 +286,15 @@ describe("the overlay host", () => { it("keeps the session gate open through Escape", async () => { setViewportWidth(1440); const onClose = vi.fn(); - render(); + render( + , + ); await userEvent.keyboard("{Escape}"); // The `onClose` assertion is the discriminating one, and it is the only one here. // A second line asserting the content is still in the document was REMOVED (Minor 5, @@ -282,7 +313,15 @@ describe("the overlay host", () => { // the opening focus lands on `document.body` -- on the ONE overlay a person cannot dismiss and // must act on, which a screen reader announces as nothing having happened at all. setViewportWidth(1440); - render(); + render( + , + ); const action = screen.getByTestId("workspace-overlay-action"); await waitFor(() => expect(action).toHaveFocus()); @@ -291,7 +330,15 @@ describe("the overlay host", () => { it("never traps focus in the offline status banner", () => { setViewportWidth(1440); - render(); + render( + , + ); expect(screen.getByRole("status")).toBeInTheDocument(); expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); @@ -299,7 +346,15 @@ describe("the overlay host", () => { it("commits a withdrawal only on the second activation", async () => { setViewportWidth(1440); const onCommit = vi.fn(); - render(); + render( + , + ); await userEvent.click(screen.getByRole("button", { name: /withdraw/i })); expect(onCommit).not.toHaveBeenCalled(); expect(screen.getByText(/fresh authentication checkpoint/i)).toBeInTheDocument(); @@ -311,7 +366,13 @@ describe("the overlay host", () => { setViewportWidth(1440); const onCommit = vi.fn(); render( - , + , ); const action = screen.getByRole("button", { name: /pause/i }); expect(action).toHaveAttribute("aria-disabled", "true"); @@ -332,6 +393,7 @@ describe("the overlay host", () => { onClose={noop} onCommit={readOnlyCommit} blockReason="permission-unavailable" + commitRefusal={null} />, ); expect(screen.getByRole("button", { name: /close/i })).not.toHaveAttribute("aria-disabled"); @@ -374,6 +436,36 @@ describe("the overlay URL", () => { expect(window.history.state, "the seeded workspace entry must carry no marker").toBeNull(); } + it("lets an unstaged recovery action close its URL-opened overlay", async () => { + setViewportWidth(1440); + seedHistory(); + render(); + + act(() => openWorkspaceOverlay("session-expiry")); + const action = await screen.findByTestId("workspace-overlay-action"); + await userEvent.click(action); + + await waitFor(() => expect(screen.queryByTestId("workspace-overlay-content")).toBeNull()); + expect(window.location.search).not.toContain("overlay="); + }); + + it("records a staged mutating action only once while its close traversal is pending", async () => { + setViewportWidth(1440); + seedHistory(); + render(); + const record = vi.fn(); + + act(() => openWorkspaceOverlayWithCommit("pause", { kind: "record", record })); + const action = await screen.findByTestId("workspace-overlay-action"); + act(() => { + fireEvent.click(action); + fireEvent.click(action); + }); + + expect(record).toHaveBeenCalledTimes(1); + await waitFor(() => expect(screen.queryByTestId("workspace-overlay-content")).toBeNull()); + }); + it("does not reopen a dismissed overlay when the browser goes back", async () => { setViewportWidth(1440); seedHistory(); diff --git a/tests/caring-contacts-overlay-trigger.dom.test.tsx b/tests/caring-contacts-overlay-trigger.dom.test.tsx new file mode 100644 index 0000000000..125372e5f7 --- /dev/null +++ b/tests/caring-contacts-overlay-trigger.dom.test.tsx @@ -0,0 +1,458 @@ +import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { Component, type ReactNode } from "react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { WORKSPACE_OVERLAY_DEFINITIONS } from "@/components/caring-contacts/workspace/overlays/definitions"; +import { + clearStagedWorkspaceOverlayCommit, + commitForHistoryEntry, + commitRefusalFor, + nextWorkspaceOverlayCommitToken, + NO_STAGED_COMMIT_REASON, + readStagedWorkspaceOverlayCommit, + stageWorkspaceOverlayCommit, +} from "@/components/caring-contacts/workspace/overlays/overlay-commits"; +import { WorkspaceOverlayTrigger } from "@/components/caring-contacts/workspace/overlays/overlay-trigger"; +import { + openWorkspaceOverlay, + WorkspaceOverlays, +} from "@/components/caring-contacts/workspace/overlays/workspace-overlays"; + +import { CARING_CONTACTS_PROHIBITED_LANGUAGE } from "./helpers/caring-contacts-prohibited-language"; + +/** + * Task 3: the control that raises an overlay, and the commit contract that had to + * ship with it. + * + * Ruling 87 is the whole reason this file is not simply "a button opens a panel". + * The 24 overlays are decision surfaces, every one of them renders a decision + * control, and until this trigger existed none of them was reachable from any + * control — which is the only reason a confirm that recorded nothing was + * tolerable. + * + * Ruling 90 is the reason the assertions come in three groups rather than two. + * Ruling 87's domain is the sixteen rows that RECORD something; the other eight + * carry exits, and a refusal reading "nothing can be recorded here" is false about + * a control whose whole action is to leave. So: the trigger opens what it names; + * a recording row never offers a decision control the system will not honour; and + * a non-recording row keeps its exit. + */ + +/** jsdom reports a fixed 1024px viewport; the host needs a width to choose a modality at all. */ +function setViewportWidth(width: number) { + Object.defineProperty(window, "innerWidth", { configurable: true, value: width }); + window.dispatchEvent(new Event("resize")); +} + +const WORKSPACE_PATH = "/caring-contacts"; +/** A distinguishable prior entry, so what Back lands on is unambiguous. */ +const PRIOR_PATH = "/caring-contacts/somewhere-before"; + +function seedHistory() { + window.history.pushState(null, "", `${PRIOR_PATH}?marker=before`); + window.history.pushState(null, "", WORKSPACE_PATH); +} + +/** + * The staged commit lives in a module-scoped slot, so it outlives a render the way + * the browser tab does. Emptying it between tests is what stops one test's staged + * intent silently satisfying the next test's assertion. + */ +beforeEach(() => { + clearStagedWorkspaceOverlayCommit(); + setViewportWidth(1440); + seedHistory(); +}); + +afterEach(() => { + cleanup(); + clearStagedWorkspaceOverlayCommit(); +}); + +function contentFor(overlayId: string) { + return document.querySelector(`[data-testid="workspace-overlay-content"][data-overlay-id="${overlayId}"]`); +} + +describe("the overlay trigger", () => { + it("opens the overlay it names, and Back closes it", async () => { + render( + <> + {} }}> + Pause this plan + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Pause this plan" })); + expect(contentFor("pause")).not.toBeNull(); + expect(window.location.search).toContain("overlay=pause"); + + act(() => window.history.back()); + await waitFor(() => expect(contentFor("pause")).toBeNull()); + expect(window.location.search).not.toContain("overlay="); + }); + + it("fails loudly for an id the frozen table does not carry, rather than opening nothing", () => { + // A control that opens an empty overlay is the silent version of exactly the + // defect the commit contract exists to prevent, so it throws at render. + expect(() => + render( + {} }}> + Pause this plan + , + ), + ).toThrow(/No overlay is defined for the id "pause-plan"/); + }); + + it("cannot be constructed without a commit", () => { + // A type-level guarantee checked by `tsc --noEmit`, not at runtime: `commit` + // is required, so a screen that opens an overlay it has not wired fails to + // compile. `@ts-expect-error` fails the typecheck if the error ever stops + // being raised — which is what makes this a proof rather than a comment. + const withoutCommit = ( + // @ts-expect-error `commit` is required — an overlay cannot be opened unwired. + Pause this plan + ); + expect(withoutCommit).toBeTruthy(); + }); + + it("carries the workspace's 48px tap floor and a surface of its own", () => { + render( + + Pause this plan + , + ); + const trigger = screen.getByRole("button", { name: "Pause this plan" }); + + // `min-h-tap` is `--spacing-tap` (3rem). Never `min-h-11`: 44px reintroduces a + // known `ui-smoke` sub-pixel flake. + expect(trigger.className).toContain("min-h-tap"); + + // Fix round 1, M-3: with no `className` — the shape every usage takes before a + // screen styles it — the control must still have a surface rather than being + // effectively invisible. Tokens only, no hex. + expect(trigger.className, "the default rendering has no background").toMatch(/bg-\[color:var\(--[a-z-]+\)\]/); + expect(trigger.className, "the default rendering has no text colour").toMatch(/text-\[color:var\(--[a-z-]+\)\]/); + expect(trigger.className, "the default rendering has no border colour").toMatch( + /border-\[color:var\(--[a-z-]+\)\]/, + ); + }); +}); + +describe("the commit contract", () => { + it("records the screen's decision through the host mounted by the shell", async () => { + const record = vi.fn(); + render( + <> + + Pause this plan + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Pause this plan" })); + const action = screen.getByTestId("workspace-overlay-action"); + expect(action).not.toHaveAttribute("aria-disabled"); + + await userEvent.click(action); + // The overlay the trigger named, not whatever happened to be open. + expect(record).toHaveBeenCalledTimes(1); + expect(record).toHaveBeenCalledWith("pause"); + + await waitFor(() => expect(contentFor("pause")).toBeNull()); + // The entry the commit belonged to has been unwound, so the slot no longer + // names the current entry and the host has emptied it. + await waitFor(() => expect(readStagedWorkspaceOverlayCommit()).toBeNull()); + }); + + it("never shows the refusal in the frame the decision was confirmed in", async () => { + // Fix round 1, Important 2. Clearing the slot inside the confirm handler + // emptied it while the URL still named the overlay — `history.back()` fires + // `popstate` asynchronously — so React re-rendered the still-open overlay with + // nothing staged and flashed "nothing can be recorded here" at someone who had + // just confirmed a withdrawal. + // + // `fireEvent`, not `userEvent`, and no `waitFor`: `fireEvent` flushes React + // inside the click while the `popstate` is still a queued task, so this reads + // the exact frame the flash would appear in. Every assertion in the test above + // waits for the settled state and steps straight over it. + const record = vi.fn(); + render( + <> + + Withdraw this patient + + + , + ); + + fireEvent.click(screen.getByRole("button", { name: "Withdraw this patient" })); + fireEvent.click(screen.getByTestId("workspace-overlay-action")); + // `withdrawal` requires fresh authentication, so the checkpoint is raised first + // and the second activation is the one that records. + fireEvent.click(screen.getByTestId("workspace-overlay-action")); + expect(record).toHaveBeenCalledTimes(1); + + const action = screen.getByTestId("workspace-overlay-action"); + expect(action, "the confirmed frame showed the action refused").not.toHaveAttribute("aria-disabled"); + expect( + screen.queryByText(NO_STAGED_COMMIT_REASON), + "the confirmed frame showed the unstaged refusal", + ).not.toBeInTheDocument(); + }); + + it("still runs the fresh-authentication checkpoint before recording", async () => { + const record = vi.fn(); + render( + <> + + Withdraw this patient + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Withdraw this patient" })); + await userEvent.click(screen.getByTestId("workspace-overlay-action")); + expect(record, "the first activation raised the checkpoint; it must record nothing").not.toHaveBeenCalled(); + expect(screen.getByText(/fresh authentication checkpoint/i)).toBeInTheDocument(); + + await userEvent.click(screen.getByTestId("workspace-overlay-action")); + expect(record).toHaveBeenCalledTimes(1); + }); + + it("refuses the decision in the aria-disabled shape when the caller states it is unavailable", async () => { + const reason = "Pausing a plan is not built yet, so nothing can be changed from here."; + render( + <> + + Pause this plan + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Pause this plan" })); + const action = screen.getByTestId("workspace-overlay-action"); + + expect(action).toHaveAttribute("aria-disabled", "true"); + // Never both: native `disabled` removes the tab stop, so the stated reason + // could never be reached by keyboard, and lint fails on the pair. + expect(action).not.toHaveAttribute("disabled"); + + // Reachable, and NOT in a `title`: a title is reached by hover and may never + // be announced at all. + expect(action).not.toHaveAttribute("title"); + const describedBy = action.getAttribute("aria-describedby"); + expect(describedBy, "the refused control points at no reason").not.toBeNull(); + expect(document.getElementById(describedBy!)?.textContent).toBe(reason); + + await userEvent.click(action); + expect(contentFor("pause"), "an inert action must not close the overlay either").not.toBeNull(); + }); + + it("carries a caller-stated refusal onto a read-only row as well", async () => { + // Scope `every-row`: a screen that says the decision is unbuilt has said so + // about THIS row, whatever the row does — an exit nobody has built is still an + // exit that would go nowhere. This is the half of the refusal Ruling 90 leaves + // reaching every row. + const readOnly = WORKSPACE_OVERLAY_DEFINITIONS.find((definition) => !definition.mutatesState); + expect(readOnly, "the frozen table carries at least one read-only row").toBeDefined(); + const reason = "This preview is not wired to a screen yet."; + render( + <> + + Open the preview + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Open the preview" })); + expect(screen.getByTestId("workspace-overlay-action")).toHaveAttribute("aria-disabled", "true"); + expect(screen.getByText(reason)).toBeInTheDocument(); + }); + + it("refuses a recording overlay reached by address rather than from a control", async () => { + render(); + act(() => openWorkspaceOverlay("pause")); + await screen.findByTestId("workspace-overlay-content"); + + const action = screen.getByTestId("workspace-overlay-action"); + expect(action).toHaveAttribute("aria-disabled", "true"); + expect(screen.getByText(NO_STAGED_COMMIT_REASON)).toBeInTheDocument(); + }); +}); + +/** + * Ruling 90. The eight `mutatesState: false` rows carry EXITS, not confirmations, + * so "nothing can be recorded here" is not a statement that can be made about + * them — and on the two `recovery-only` rows, refusing the single control leaves a + * person inside an overlay they cannot dismiss with nothing to do at all. + */ +describe("a row that records nothing keeps its way out", () => { + const NON_RECORDING = WORKSPACE_OVERLAY_DEFINITIONS.filter((definition) => !definition.mutatesState); + + it("covers every non-recording row in the frozen table", () => { + // Guards the loop below against silently shrinking to nothing if the flag ever + // moves in the table. + expect(NON_RECORDING.length).toBe(8); + }); + + for (const definition of NON_RECORDING) { + it(`leaves "${definition.id}" usable when it is deep-linked with nothing staged`, async () => { + render(); + act(() => openWorkspaceOverlay(definition.id)); + await screen.findByTestId("workspace-overlay-content"); + + const action = screen.getByTestId("workspace-overlay-action"); + expect(action, `${definition.id}: its exit was refused`).not.toHaveAttribute("aria-disabled"); + expect( + screen.queryByText(NO_STAGED_COMMIT_REASON), + `${definition.id}: it renders a refusal that is false about an exit`, + ).not.toBeInTheDocument(); + }); + } + + it("leaves the recovery-only session gate with something to do", async () => { + // The sharpest case, and the one that made the first version harmful: + // `session-expiry` ignores Escape and the backdrop by design, so its single + // control is the only way out of it. + const gate = WORKSPACE_OVERLAY_DEFINITIONS.find((definition) => definition.id === "session-expiry"); + expect(gate?.dismissal, "session-expiry is the recovery-only row this test is about").toBe("recovery-only"); + + render(); + act(() => openWorkspaceOverlay("session-expiry")); + await screen.findByTestId("workspace-overlay-content"); + + const action = screen.getByTestId("workspace-overlay-action"); + expect(action).not.toHaveAttribute("aria-disabled"); + expect(action).not.toHaveAttribute("disabled"); + }); +}); + +/** + * Fix round 1, Important 3. The slot is bound to the history ENTRY that staged it, + * not to the overlay id — the id match narrowed these failures without closing + * them. + */ +describe("a staged commit belongs to the entry that opened it", () => { + it("never answers an entry it was not staged for", () => { + const commit = { kind: "record", record: () => {} } as const; + const token = nextWorkspaceOverlayCommitToken(); + stageWorkspaceOverlayCommit(token, commit); + const slot = readStagedWorkspaceOverlayCommit(); + + expect(commitForHistoryEntry(slot, token)).toBe(commit); + // A later opening mints a new token, so an older slot cannot answer it — this + // is the ten-`Pause`-rows case, where one row's commit must never answer an + // overlay raised from another. + expect(commitForHistoryEntry(slot, nextWorkspaceOverlayCommitToken())).toBeNull(); + // A deep link, and the entry `history.back()` unwinds to, carry no token. + expect(commitForHistoryEntry(slot, null)).toBeNull(); + expect(commitForHistoryEntry(null, token)).toBeNull(); + }); + + it("mints a distinct token per opening", () => { + const tokens = new Set([ + nextWorkspaceOverlayCommitToken(), + nextWorkspaceOverlayCommitToken(), + nextWorkspaceOverlayCommitToken(), + ]); + expect(tokens.size).toBe(3); + }); + + it("is emptied when the browser goes Back, which never calls the Sheet's close", async () => { + // The workspace's PRIMARY dismissal route, and the one the first version + // missed entirely: Back closes through `popstate`, so `onClose` never runs. + const record = vi.fn(); + render( + <> + + Pause this plan + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Pause this plan" })); + expect(readStagedWorkspaceOverlayCommit()).not.toBeNull(); + + act(() => window.history.back()); + await waitFor(() => expect(contentFor("pause")).toBeNull()); + await waitFor(() => + expect(readStagedWorkspaceOverlayCommit(), "the commit outlived the Back that dismissed it").toBeNull(), + ); + expect(record).not.toHaveBeenCalled(); + }); +}); + +/** Fix round 1, Important 4: a rejected recording must not disappear into the promise. */ +class CommitFailureBoundary extends Component<{ children: ReactNode }, { message: string | null }> { + state = { message: null as string | null }; + static getDerivedStateFromError(error: unknown) { + return { message: error instanceof Error ? error.message : String(error) }; + } + render() { + return this.state.message === null ? this.props.children :

Nothing was sent: {this.state.message}

; + } +} + +describe("an asynchronous recording", () => { + it("accepts a promise-returning record and closes once it is issued", async () => { + const record = vi.fn(() => Promise.resolve()); + render( + <> + + Pause this plan + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Pause this plan" })); + await userEvent.click(screen.getByTestId("workspace-overlay-action")); + expect(record).toHaveBeenCalledWith("pause"); + await waitFor(() => expect(contentFor("pause")).toBeNull()); + }); + + it("raises a rejection where an error boundary can state that nothing was written", async () => { + // The minimum that is not silent. Without this the rejection is an unhandled + // promise, the overlay has already closed, and the clinician is looking at a + // screen that appears to have recorded the decision. + const failure = new Error("the store refused the write"); + render( + + Promise.reject(failure) }}> + Pause this plan + + + , + ); + + await userEvent.click(screen.getByRole("button", { name: "Pause this plan" })); + await userEvent.click(screen.getByTestId("workspace-overlay-action")); + + await waitFor(() => expect(screen.getByText(/the store refused the write/)).toBeInTheDocument()); + }); +}); + +describe("the refusal rule", () => { + it("answers every state of the slot, and scopes each refusal to the rows it is true of", () => { + // Total by construction, so the rule can be read here rather than inferred + // from a rendered button. + expect(commitRefusalFor(null)).toEqual({ reason: NO_STAGED_COMMIT_REASON, scope: "recording-rows-only" }); + expect(commitRefusalFor({ kind: "unavailable", reason: "Not built yet." })).toEqual({ + reason: "Not built yet.", + scope: "every-row", + }); + expect(commitRefusalFor({ kind: "record", record: () => {} })).toBeNull(); + }); + + it("states the unstaged refusal in permitted vocabulary", () => { + expect(NO_STAGED_COMMIT_REASON).not.toMatch(CARING_CONTACTS_PROHIBITED_LANGUAGE); + }); +});