Claude/suicide contact mockup b5aaa0 - #2279
Conversation
Records the nine decision-lock revisions agreed 19 August 2026 (auto-reply to inbound messages, closing message at month 12, coordinator-set first contact date, third-party pause, cultural-identity reach reporting, configurable retention with real deletion, message rules as data, the enforced repository seam, and one bounded clinical-record document). Specifies the domain rules layer, a dedicated caring-contact Supabase project kept hard-separated from the Clinical KB project, the seven screens required by existing decisions but never designed, four recommended screens, the design non-regression contract, the elevation brief, and the open governance register. Synthetic data only. No SMS provider, no real patient data, no migration against the Clinical KB project, no production deployment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…governance artefacts Renames the service from Callback (41 occurrences across the rollout plan, the coordination spec, the production build spec and the prototype shell header). Callback named a promise the service never keeps: it never calls back, never receives a reply and never responds. A patient enrolled in "Callback" could reasonably expect a telephone call, which is the same expectation-mismatch hazard as the silent-reply problem corrected on 19 August. Recorded as decision revision 2.10. No test asserted the old name; the 38 focused caring-contact tests pass unchanged. Adds five artefacts the programme lacked: - hazard-log.md: 30 clinical, patient-facing, scheduling, privacy and operational hazards with severity, controls, status and named owner. Six are unmitigated and block a pilot. - evidence-brief.md: the honest evidence position for a sponsor, including the equivocal meta-analyses and null replications. Citations are explicitly marked unverified and must be checked before use. - referral-feasibility.md: the questions to ask about the WA hospital referral feed, who to ask, and the manual-entry fallback. Largest programme risk. - message-review-pack.md: how to run the lived-experience review, and the reply-boundary wording correction it must settle first. - demo-script.md: the five-minute path through the built system. Repairs the binding reference to design-handoff.md, a file that never existed, repointing both mentions at interaction-matrix.md, which holds the 24-row modality matrix. The documentation link gate passed over this for days; queued as its own ledger request. Queues six outstanding-work requests covering the blocking hazards, the now inaccurate reply wording, referral feasibility, build status, the Australian hosting gap and the link-gate defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e, and plan the domain build Reply wording. Decision revision 2.1 made the number receiving-capable, which made "Replies are not received, stored, analysed or monitored" untrue: replies are received, then discarded unread. Stating something false about a safety boundary is the failure this programme can least afford, so the notice now claims only what remains true — "No one reads replies to this number" — and a new AUTOMATED_REPLY_RESPONSE constant supplies the message a person gets at the moment they reach out. Both are marked provisional pending the lived-experience and dual-approval gate. GSM-7 evidence recomputed independently: the patient message drops from 272 to 252 septets, still two segments; the automated reply is 218 septets, two segments. Both are now pinned by test, as is the absence of any patient mobile number in the automated reply. Evidence brief. Every citation verified against journal records rather than recalled. The material find is Stevens et al. (Br J Psychiatry 2024;224(3): 106-113), an Australian randomised trial of automated SMS brief contact after hospital-treated self-harm reporting a significant 22% reduction in repeat event rates at 12 and 24 months, on a nine-contact schedule nearly identical to this pathway's. Comtois et al. (JAMA Psychiatry 2019) is corrected: both primary outcomes were null and only secondary outcomes reached significance, so the common summary of it as simply positive overstates it. Milner et al. (2015) retained as the strongest sceptical citation, with the note that it predates both SMS trials. Adds outreach-drafts.md with two ready-to-send approaches — the lived-experience message review and the hospital referral feed feasibility conversation — since those are the two actions that need a person rather than a keyboard. Adds the part-one implementation plan: eleven test-first tasks covering the sealed domain layer, the twelve-month simulation and the team-scoped Postgres schema, with three design decisions recorded (Week 1 collision suppression, cancellation always permitted, safety stop ungated by role). Focused caring-contact suites: 38 passed. Typecheck: exit 0. Link gate: 1910 references resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…p non-building material Replaces the twelve-phase framing with three phases, every one of which can be built now on invented patients without a sponsor, a contract, or anyone's permission: (1) the rules and the database, (2) the working screens, (3) the demonstrable system. Most of the previous twelve boundaries were not real — nothing outside the code changed between them, and several described work that cannot be done from a keyboard at all. States the out-of-scope set plainly: no message sent to any number real or test, no SMS provider, no hosting change, no hospital system connection, no enterprise sign-on, no real patient, and no migration against the Clinical KB Supabase project. The build runs against local Postgres. Removes six governance and sponsor documents from the working tree — hazard log, evidence brief, referral feasibility, outreach drafts, message review pack and demonstration script. They are not building material. All six remain in git history at 32d408c and restore with a single checkout when a sponsoring service exists. Trims the specification's open register to the four decisions that actually affect the build, and drops the companion-document index whose every entry pointed at a removed file. Applies the Task 1 finding: the plan advertised toAwstParts(date, clock?) while the implemented function takes one argument. Corrected before Task 2 so no later task builds against a signature that does not exist. Link gate: 1907 references resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…clamping and collision suppression
… cancellation Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…amed refusals Adds canPerformCaringContactAction (team-scope check before role grant, frozen ROLE_ACTIONS map, named refusal reasons) and canApproveOwnAuthoredVersion (self-approval-denied). A table-driven test over the frozen ALL_ACTIONS constant asserts every CaringContactAction is explicitly granted or explicitly ungranted, so a future action cannot silently default to allowed.
… provisional rulebook
…y audited writes
Adds the storage contract every caring-contact change passes through, plus the
in-memory reference implementation and one shared contract suite that Task 11's
Postgres store will run unchanged.
Guarantees, each proved by a test that fails when the mechanism is removed:
* a replayed idempotency key returns the original result and writes nothing;
* a write appends exactly one audit event in the same uninterrupted commit as
the change, so a write that throws part-way leaves neither;
* a write against a stale version is refused by name, including when two
simultaneous writes race;
* a patient may hold only one non-terminal plan, checked across every team;
* reads are team- and capability-scoped and return empty rather than a
refusal that would reveal a record exists.
Absorbed contacts are stored in the terminal suppressed state, so dispatch
cannot key off sendAt. A recorded death cancels every non-terminal contact
outright, with no comparison of any send instant to now.
The episode shape moves from retention.ts to a new episode.ts and is re-exported
unchanged, so the store can project retention's own Episode type without
tripping the Task 8 sibling guard. There is still exactly one episode shape.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task 9 could not record that a caring contact went out: permissions.ts had no concept of a non-human actor, so there was no attributable author for a provider status and Episode.counts.contactsSent/contactsDelivered could only ever report 0. Adds a SystemActor with one system role (contactDispatcher), granted exactly four contact-status actions and nothing else. The human and system grant tables are pinned disjoint by test: a dispatcher can never activate, pause, withdraw, reassign, approve or read anything, and no human role can write a delivery receipt by hand. Human roles smuggled onto a system actor are ignored. The store gains startContactDispatch, recordContactSent, recordContactProviderStatus and recordContactMissed, each an ordinary idempotent, version-checked, atomically audited write against objectType "contact". Only the write that BEGINS a dispatch is gated on an active plan: once a contact is `processing` the send is committed, and the contact lifecycle has no exit from `processing` other than sending, so gating the later writes would strand a contact rather than protect anyone. A recorded death is not carried by that gate at all — it cancels every unsent contact outright. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t as a safety stop Task 9 routed death and death-correction through triggerServiceSafetyStop because no action in CaringContactAction covered hospital events at all. The capability model therefore said a recorded death was a service safety stop, which is not what happened. Adds recordHospitalStatusEvent and routes every hospital status event through it. It is a clinical write, granted to coordinator and teamLead; the auditor stays confined to reading (rule 3), and is refused a readmission or a mobile-number change. A death and its correction additionally accept triggerServiceSafetyStop, the one capability every role holds. That preserves the property the old routing was reaching for: recording a death must never be blocked by a permission check, because a refusal would leave a plan sending to someone who has died. Proved by a contract test in which an auditor is refused a readmission and still succeeds in recording a death. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…zero duplicate sends The integration proof for Tasks 1-9: one twelve-month episode driven through the real store, permissions, lifecycles and calendar, with ten scenarios plus the suppression collision case. simulation.ts adds no rules. What may be sent comes from the store's listSendableContacts (contact state, never sendAt); whether a send is still on time comes from schedule.ts's approved window, now exported as APPROVED_SEND_WINDOW / isWithinApprovedSendWindow rather than restated here; whether a send may happen at all is the store's refusal, which the driver records instead of pre-empting. The one policy no module owns — how many times a transient provider failure may be retried, and how far apart — is a required input, not a constant invented in the driver. Proved: ten dispatches ascending with no duplicates; three attempts and never a fourth; a retry that would leave the window (or roll into the next day's) abandoned and marked missed rather than sent late; a pause permanently skipping months 2 and 3 with the calendar untouched; a withdrawal cancelling everything later; a readmission that neither resumes nor rebases nor admits a competing plan; no dispatch at or after a recorded death under three separate retry states; a pause landing mid-dispatch yielding exactly one outcome; identical dispatch days under a +/-5 minute clock skew; one audit event per write with no mobile number, message body or patient name; and the collision case sending nine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… audit and RLS The caring-contact schema, its row-level security, and the Postgres store, proven against a real Postgres 17 in a disposable local container. Schema (caring-contacts/supabase/migrations/, never the repository's live supabase/migrations/): - teams, actors, referrals, plans, contacts, pathway_versions, audit_events, service_state, retention_state, plus contact_dispatches, idempotency_records, and cultural_identity_reports. - Cultural identity lives ONLY in the reporting projection; the patient row has no such column. - plans_one_non_terminal_per_patient: a unique PARTIAL index over patient_id alone, so one team cannot start a second plan for a person another team is already contacting. - contact_dispatches_unique_attempt: unique (contact_id, attempt). - No CREATE INDEX CONCURRENTLY; every migration replays as a no-op. Row-level security is enabled and forced on every table, deny by default: each policy compares the row's team to a transaction-local setting that resolves to NULL when unset, so an unscoped session matches nothing. A cross-team select returns zero rows rather than an error that would confirm the row exists, and the anonymous role is granted SELECT deliberately so its denial proves policy rather than a missing grant. A DEFERRABLE constraint trigger fires at commit and fails any change to a patient-bearing table that carries no audit event in the same transaction, so a direct update cannot bypass the audit path. The store reproduces the in-memory store's version-check ordering and its active-plan dispatch gate, and the Task 9 contract suite now runs against both implementations from one definition rather than being duplicated. Also carried from Task 10: markMissed now accepts `processing` as well as `scheduled`, so a contact abandoned after a provider timeout is no longer stranded; and the retry policy (2 retries, 3 attempts, 45 minutes apart) has a governed home in service-rules.ts instead of living in test fixtures. tests/test-runner-safety.test.ts's live-test discovery guard was rewritten to assert the loaded config's node project rather than one literal line of source, and confirmed still able to fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sses at zero warnings PAUSE_EVENT_TYPES was an `as const` array used only in a type position, which this repository's zero-warning lint correctly flagged as an unused runtime value. It is now a union type, which is what the code actually needed. Found by re-reading the phase-gate output rather than trusting its exit code: the gate had been piped through `tail`, so the shell reported 0 while lint had failed on one warning and prettier had flagged files. Exit code alone is not proof when a pipe is in the way. Gate now, each with a real exit code: lint 0, prettier 0 across src, tests, scripts, caring-contacts, docs and config, tsc 0, and the two affected suites 59/59. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…session scratch The build ran through a ledger under .superpowers/, which is git-ignored. Every decision taken on the owner's behalf lived only there and in one long conversation, and both are losable. This records them where the repository keeps them. Contains: the three-phase scope and what Phase 1 actually built; the thirteen decisions with their reasoning and what each costs if wrong; the deliberate sabotage results, including the two tests found unable to fail and rewritten; six open items for Phase 2; three open decisions for the owner; and the exact command that restores the six governance documents from history. Phase gate, each verified with a real exit code: 7,531 tests across 682 files, tsc silent, lint zero warnings, Prettier clean, and 55 database tests against Postgres 17 in a disposable container. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
19 test-first tasks covering the rules layer the seven undesigned screens need, the production route group and four-state shell, the API boundary that audits every view, and all 24 overlays against the frozen modality matrix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…domain so production can use it
The three patient-visible strings (PATIENT_VISIBLE_NO_REPLY_NOTICE,
EXACT_PATIENT_VISIBLE_MESSAGE, AUTOMATED_REPLY_RESPONSE) and the fictional
contact numbers previously lived only in a mockup component
(src/components/caring-contacts/mockups/personalisation-screen.tsx and
types.ts). Production code can never import from a mockup path
(eslint no-restricted-imports), so this moved them into the sealed domain
at src/lib/caring-contacts/:
- New src/lib/caring-contacts/synthetic-contacts.ts: FICTIONAL_CONTACTS_BY_ROLE,
DESIGNATED_FICTIONAL_MOBILE_NUMBERS and their types.
- New src/lib/caring-contacts/message-copy.ts: the three provisional
patient-visible strings (byte-identical, PROVISIONAL comments carried
across verbatim) plus their GSM-7 evidence, derived from the single
calculateGsm7 in message-policy.ts.
- The mockup's duplicated calculateGsm7/Gsm7Evidence was deleted; both
mockup files now import from the domain and re-export the same names so
every existing pinned test (tests/caring-contact-mockups.dom.test.tsx,
tests/caring-contact-product-redesign.dom.test.tsx) keeps passing
unchanged.
- Fixed two spots where the brief's plain "export { X } from '...'"
re-export would not have created a usable local binding: personalisation-
screen.tsx uses EXACT_MESSAGE_GSM7/EXACT_PATIENT_VISIBLE_MESSAGE/
PATIENT_VISIBLE_NO_REPLY_NOTICE internally (MessagePreview,
CompactMessagePreview, PersonalisationScreen), and types.ts uses
SyntheticPatientMobile as a field type on SyntheticPatient. Both files
now import-then-locally-export instead, which typecheck caught
(TS2304: Cannot find name 'SyntheticPatientMobile').
Mutation proof: temporarily removed one space from
EXACT_PATIENT_VISIBLE_MESSAGE (252 septets became 251). This correctly
turned red: tests/caring-contacts-message-copy.test.ts "keeps the pinned
GSM-7 evidence..." and tests/caring-contact-mockups.dom.test.tsx "derives
exact GSM-7 septets...". A same-length digit swap ("6 pm" -> "7 pm", the
brief's suggested example) does NOT change septets since both characters
are GSM-7 basic-set digits costing 1 septet each, so it does not exercise
any assertion — recorded here rather than used as the proof. Reverted the
space-removal mutation; final diff is byte-identical to the original
strings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e workspace performs Adds the two dual-approval roles (clinicalProgrammeLead, livedExperienceRepresentative) and the ten action names the Phase 2 screens need, none of which existed yet. Because canPerformCaringContactAction denies by default, an unnamed action could not be granted, so this is the gate every later Group 1 task passes through. - publishPathwayVersion is granted only to clinicalProgrammeLead, never teamLead: publication is the clinical act; the team lead approves and retires but does not publish. - triggerServiceSafetyStop stays granted to every human role, including both new ones. - The auditor gains only read/no-op actions (viewPatientRecord, manageNotificationPreferences, enterTrainingMode) and stays confined from every plan-mutating action. - UNGRANTED_ACTIONS stays a frozen empty array; every new action is granted to at least one role. Extended tests/caring-contacts-permissions.test.ts with the new roles/actions describe block and widened its ROLES constant to include both new roles, which the pre-existing completeness test needs since publishPathwayVersion is granted only to a role outside the old three-role list. Verified test-first: the appended test failed for the right reason (undefined ROLE_ACTIONS lookups for the not-yet-existing roles) before the registry changes, and passed (90/90) after. Mutation check: granting publishPathwayVersion to teamLead flipped exactly one assertion red (89/90), naming teamLead's decision; reverted. Full unit suite (685 files) and typecheck both pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…approval
Spec 4.2: a confirmed wrong-recipient message, duplicate send, unauthorised
content, privacy/security incident, or loss of audit integrity halts ALL sending
across every patient and team. The first recorded stop is permanent -- a second
stop is refused rather than allowed to overwrite the original reason, actor,
time or note.
The restart cannot be one person's decision. All three of incidentLead,
privacySecurityOwner and clinicalProgrammeLead must be recorded, and the module
refuses a repeat ROLE (restart-approval-role-already-recorded) as well as a
repeat ACTOR in a different role (restart-approval-actor-already-recorded), so
three approvals mean three people. The service returns to running only on the
approval that completes the third distinct role; two approvals leave it stopped
with both approvals recorded. There is no force flag or override path.
describeServiceStop is banner text rendered on screens showing no patient, so it
deliberately excludes the free-text incident note, which a responder can write a
name or a number into.
Pure transitions: injected Clock, no storage, no permission check (the caller has
already asked canPerformCaringContactAction), no imports outside the sealed
domain.
The AWST +08:00 timestamp formatter moves from a private helper in audit.ts to
clock.ts as awstIsoTimestamp, and audit.ts now calls it. Behaviour is unchanged
(caring-contacts-audit.test.ts stays green); this keeps one timestamp format in
the domain rather than a second one drifting alongside the audit trail.
Mutation testing -- each applied alone, the named test observed red, then reverted:
1. restart on `approvals.length >= 2` instead of all three required roles
-> caught by "requires all three approval roles before it restarts"
(AssertionError: expected false to be true)
2. same-actor guard deleted from applyServiceRestartApproval
-> caught by "refuses a single person supplying more than one approval"
(expected { ok: true } to deeply equal { ok: false })
3. serviceStopBlocksDispatch hardcoded to false
-> caught by "stops the whole service and blocks dispatch"
(AssertionError: expected false to be true)
Verified: node scripts/run-vitest.mjs run tests/caring-contacts-service-state.test.ts
-> Tests 8 passed (8); full caring-contacts suite (15 files) -> Tests 316 passed (316);
npm run typecheck clean; prettier clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…, and name the stop service-wide
Review round 1.
describeServiceStop now takes ServiceStopBannerFacts -- { stopped: false } |
{ stopped: true; reason; restartApprovals } -- instead of the whole ServiceState.
The incident note is free text a responder types mid-incident and is the one
field guaranteed to hold patient data; the banner renders on every screen,
including ones showing no patient. Previously only a doc comment held it out
while it sat in scope on every line. Now the compiler does.
The discriminated shape was kept rather than a bare { reason, restartApprovals }
| null so that the existing assertion `describeServiceStop(runningService(team))
=== null` keeps testing the running-service contract instead of degrading to
null-maps-to-null. ServiceState stays structurally assignable, so no caller
changes.
New test: a stop whose note names a patient and a +61 mobile, asserting neither
substring reaches the banner while the reason and the "0 of 3" count still do.
ServiceState.teamId is renamed reportedByTeamId on both variants and on
runningService's parameter. The spec halts sending "across every patient and
team", but a field called teamId makes a per-team storage table the natural
implementation, which would leave every other team sending through the incident.
The type now says in a doc comment that the field is provenance only and that
storage must persist a single service-wide record, not one row per team.
Removed the unreachable "All approvals are in." branch: a stopped state can
never hold three approvals, because the third restarts the service.
Mutation testing, both halves run and reverted:
A. interpolate ${state.note} with the parameter left narrow
-> does not compile: TS2339 Property 'note' does not exist on type
'{ stopped: true; reason: ServiceStopReason; restartApprovals: ... }'
B. widen the parameter back to ServiceState, then interpolate ${state.note}
-> compiles, and is caught by "never leaks the incident note into the
banner, even when the note names a patient" (expected ... not to
contain 'Rowan'; the received banner carried the full note)
Restore verified byte-identical by diff before committing.
Verified: focused + audit + clock + isolation -> Tests 32 passed (32);
full caring-contacts suite -> Tests 317 passed (317); typecheck no diagnostics;
eslint clean; prettier "All matched files use Prettier code style!".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nd urgent retirement Adds applyPathwayVersionTransition (draft -> inReview -> approved -> retired, plus publish) and retirementPausesFutureContacts, per Phase 2A task 4. Self-approval is delegated to the existing canApproveOwnAuthoredVersion in permissions.ts rather than re-implemented; its self-approval-denied reason is surfaced unchanged. Two independent approvals are required (role and actor each checked against prior approvals) before a version reaches approved, and snapshot is never spread into or replaced by any transition. Mutation proof (each reverted after confirming the expected failure): - Changed the approved-state condition from "both required roles recorded" to "approvals.length >= 1" -> 4 of 6 tests in tests/caring-contacts-pathway-versions.test.ts went red (the two-role gating test, the one-person-both-approvals test, the urgent-retirement test, and the snapshot-immutability test all depend on reaching approved only via the real two-role path). - Removed the canApproveOwnAuthoredVersion delegation call -> exactly "refuses the author approving their own version, with the shared reason" went red, confirming that check is load-bearing and not decorative. Test: node scripts/run-vitest.mjs run tests/caring-contacts-pathway-versions.test.ts -> Test Files 1 passed (1) / Tests 6 passed (6) Domain isolation: node scripts/run-vitest.mjs run tests/caring-contacts-domain-isolation.test.ts -> Test Files 1 passed (1) / Tests 3 passed (3) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…amps
Fix round 1 for task 4: review found retire's legality gate had zero test
coverage (nothing called retire outside "approved") and that publish/retire
success paths set publishedAt/retiredAt but never asserted the value.
- Add "refuses retirement from every state except approved", exercising
draft, inReview, and retired, each asserting the exact
{ ok: false, reason: "pathway-not-retirable" } object.
- Assert published.publishedAt and routine/urgent.retiredAt are non-null
AWST timestamps ending "+08:00".
Mutation proof (both reverted, pathway-versions.ts diff empty afterward):
- Dropped the retire legality guard -> exactly the new retirement test went
red (1 failed, 6 passed), draft state returned ok:true instead of the
refusal.
- Left publishedAt as null on publish -> exactly the snapshot-immutability
test (which now asserts publishedAt) went red on
"expected null not to be null" (1 failed, 6 passed).
Test: node scripts/run-vitest.mjs run tests/caring-contacts-pathway-versions.test.ts
-> Test Files 1 passed (1) / Tests 7 passed (7)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…efusals, and close the guard's route-level blind spot Fix round 1 for Task 18. Closing an overlay no longer pushes a second entry. It unwinds the one opening pushed with history.back(), falling back to replaceState only for a deep link this module did not push, so Back after dismissing can no longer walk forward into the dismissed modal and open/close cycles no longer grow the stack. blockReason is stated in plain words through an explicit, hand-written, total lookup that throws on an unmapped key (Ruling 61) - no default branch, no wording derived from the identifier, and the machine identifier never reaches the screen. The client-boundary guard now also scans src/app/caring-contacts, where the route file that actually reads the record lives. It sat outside the workspace scan, so a client boundary there would have serialised the incident note with the guard still green. Only Next's mandatory error boundary is allowlisted. data-overlay-modality now carries a comment stating exactly what it claims, and a test pins the 640-767 band where the frozen mapping's 768 and the shared Sheet's own 640 disagree (Ruling 60) - no behaviour change, the divergence is recorded rather than fought. Also: the prohibited-vocabulary regex moves to one shared helper instead of two byte-identical copies; the vocabulary check now reaches the fresh-authentication checkpoint copy; the Ruling 58 fail-loud branch is exercised directly; and one assertion that could not fail was removed from the session-gate test, replaced by a discriminating one, and disclosed in the report. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…fixed bug The Ruling 61 lookup was not total for inherited keys, so an unknown reason returns a function instead of throwing, React drops it, and a blocked control shows a clinician no explanation at all. That is the identical defect Task 17 fixed a week earlier in a different file, with Object.hasOwn. The fix did not travel. When a defect class is found on this branch, 'where else does this shape exist?' is now part of the fix rather than an optional extra. Also records a history test that neither discriminates nor executes the branch it names, and a band pin that holds one of the two breakpoints it claims. An honest one-sided pin is fine; a two-sided claim over a one-sided pin is not. Carries the guard's remaining gap to the final review: it cannot see a client component imported from outside the scanned directories and handed the record inside the shell. The earlier misses were about depth; this one is about direction.
…l lookup, and make two pins actually pin Fix round 2 for Task 18. BLOCK_REASON_WORDING is an object literal, so map[reason] === undefined is not a membership test: toString, constructor, valueOf, hasOwnProperty and __proto__ all answered with a function typed string. React renders a function as nothing, so a blocked control ended up with aria-describedby pointing at an empty paragraph - unavailable, with no reason given and no throw. Fixed with Object.hasOwn. This is the identical defect Task 17 fixed in its matrix normalisation; the audit of every other string-keyed lookup in these files found one more of the same shape, SHEET_GEOMETRY, which is safe because its key is a closed union rather than a caller string, and now says so at its use site. The deep-link history test could not discriminate: both seeded entries shared a pathname, so replaceState and an unconditional back() produced outcomes every assertion accepted. It also ran the wrong branch, because the push-branch flag leaked in from the preceding test. The prior entry now has a distinct pathname and the assertions are a discriminating pair; the flag is gone entirely, replaced by a per-entry history.state marker that no traversal, mount or test can leave stale. The Ruling 60 band pin held only its rail edge - the Sheet's 640 was a literal read from nothing. Source-text assertions now hold that edge too: the exact class string sheet.tsx flips its default geometry on, and the absence of a --breakpoint-sm override that would move what sm: means. Also: the client-boundary scan now accepts every extension a use client module could ship under, not only .ts and .tsx. The report's two overstated claims are corrected in place beside the sentences that made them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The travelling-fix rule worked first time it was applied: the implementer audited its own files for the same lookup shape, found one more, and the re-reviewer traced the type flow rather than accepting the judgement - confirming no runtime path lets a caller string reach that index. On the broken history test it went further than asked and better: removed the leaky module flag rather than resetting it, putting the marker on the history entry so cross-test leakage is structurally impossible rather than cleaned up after. Records an honest verdict on a substitute proof. Mutating an expected string proves an assertion is not vacuous; it does not prove a genuine future edit would be caught. Acceptable when the constraint is real and the gap is stated - which it was, by the implementer itself.
…4 overlays at six widths Extends the Task 15 shell spec rather than replacing it; nothing already in it is weakened. Adds brief item 5 and the remainder of item 6: - all 24 overlays deep-linked by ?overlay=<id> at 390 and 1440, with the expected modality and dismissal read off WORKSPACE_OVERLAY_DEFINITIONS and widthStateFor rather than written into the spec by hand, so the proof and the frozen table cannot drift apart; - per-modality geometry: a full-screen-stage and a phone session gate fill the viewport, an inspection-drawer is right-anchored and at most 56% of the width, a dialog is at most 640px, a bottom-sheet and the status banner are anchored to the bottom edge, and no overlay has an edge outside the viewport; - Escape closes an escape-backdrop-close row and clears the id from the URL, while a recovery-only row survives it -- driven from the table, which covers offline-banner as well as session-expiry; - focus returns to the control an overlay was opened from; - a visible focus ring in dark, under forced colours, and at the 400% zoom equivalent of a 1280px viewport, with no horizontal overflow there. Ruling 60's 640-767 band is deliberately not sampled. 400% zoom is emulated as a divided viewport because CSS zoom on the root leaves the media-query viewport at 1280 in this Chromium, which would make the overflow check unable to fail. The adoption manifest is regenerated by the pre-commit hook: the spec now names the Sheet and overlay surfaces it drives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…check-links Task 19's atlas step, under Ruling 62. Phase 2A builds one production screen plus the twenty-four overlays, so 18 of the 44 atlas images have a production counterpart and 26 do not; the document says which, and never implies a 44-image comparison happened. Seven differences are recorded on the screens that do exist, each justified against a frozen contract, a recorded ruling, or the declared scope of Phase 2A. Nothing unexplained was found, so nothing was fixed under this step. `npm run verify:pr-local` was already failing on `docs:check-links` before this task's commits, on three references in the Task 15 and Task 18 evidence records: - `tests/caring-contacts-width-state.ts:10` was a typo for the real `tests/caring-contacts-width-state.test.ts:10`; corrected. - A nested not-found route Task 15 considered and decided against, and a temporary mutation probe Task 18 created, quoted the failure of, and deleted, are both deliberately absent and both named in prose that should stay byte-stable. They go on the checker's existing allowlist for exactly that case rather than having the evidence records rewritten around them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task 19's closing step. Phase 2A ends with one production screen and the frozen 24-overlay renderer; the thirteen remaining screens, and the Today dashboard body itself, are Plan 2B. 26 of the 44 mockup atlas images have no production counterpart today and are that plan's visual specification. Queued through the inbox, which is the only supported route: `docs/outstanding-issues.md` is untouched and `npm run issues:reconcile` runs from its own branch after this one lands. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ing on The report was committed with its verify:pr-local section still pending, because the gate had not finished. It now carries all four runs verbatim: the exit-75 lease block (blocked, not red), this report's own format and docs-link defects, and the final run's `lint, typecheck` green with two 30-second child-process timeouts in tests/codex-cloud-setup.test.ts and tests/design-sync-contract.test.ts that reproduce green on their own (41 passed) and that nothing here touches. The four steps that run never reached were then run directly and all passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…und the geometry, scope the suppression Six review items. Nothing existing was deleted or loosened: the spec diff against the previous commit has zero removed lines. Important 1 — three /issues rows, queued through the inbox, never by editing docs/outstanding-issues.md. The confirmed focus defect (a non-dismissible overlay opens with focus outside its own aria-modal dialog, failing WCAG 2.4.3) carries the whole chain the reviewer traced and says plainly that the fix belongs in the shared Sheet rather than in this workspace. Plus the desktop session-gate shape as an owner decision, and the unreproduced run-playwright build failure. Important 2 — the M1 evidence claim is corrected beside the rows that make it. expectedModalityAt duplicates the host's decision from the same import, so with a DOM-level mutation nothing in the browser proof establishes that the stamp derives from the frozen table; it establishes the assertion is wired to the attribute and that the load-time width resolves correctly. The contract is held by the definitions test, which is cited. Important 3 — verify:pr-local is now stated plainly as RED with cause, naming the two timing-out files, the evidence they are unrelated, and what does not exist: a single end-to-end invocation that exits 0. The gate was not re-run, as directed. Minor 4 — the shard entry said 10.0s for 9 tests; it is 18 tests at 34.2s. The new number alone would have pushed the post-critical spread to 32.4s against a 10s ceiling, so two of the smallest specs moved shard to lift the other two groups instead: 245.9 / 254.4 / 254.4, spread 8.5. Minor 5 — desktop session-gate and dialog each stated only one side of their bound. Closed with MIN_SURFACE_WIDTH 320 and MIN_SURFACE_HEIGHT 96 applied to every modality, since bottom-sheet and status-banner heights had the same gap. Proved by two new mutations; M8's first attempt was silently dropped by tailwind-merge and went green, which is recorded because that is the failure mode this discipline exists to catch. Minor 6 — the docs-link allowlist suppressed a path repo-wide. Added a scoped map keyed by document, moved my two entries onto it, and verified the scoping bites by repointing a key at a filename that does not exist. Browser proof re-run after the spec change: 18 passed (42.7s), exit 0, no admission-busy marker. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 540b7876cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…empotency table Confirmed independently. fingerprintOf produces a canonical string, not a hash, and both stores insert it verbatim - so createPlan writes the patient's name and mobile number, and stopService writes the responder's free-text incident note. It survived nineteen task reviews because every guard and every review pointed at the audit trail. The table written in the same transaction, one table over, has no guard, and appears in the whole test corpus only as a truncate target and a table-exists entry. It crosses teams too: restart approvals are service-wide, so one team's approver stores another team's incident note under their own team_id, readable under row-level security. The lesson: a privacy guard protects the table it names, and the review verifying it inherits that scope. Ask what a mechanism stores incidentally, not only what it is for.
|
Unblock update Final head: Blocker: GitHub reports this PR unmergeable. It is 277 commits behind No sync or code changes were made. These conflicts overlap a clinical Caring Contacts change, and the PR also retains three unresolved exact-head findings: P1 unauthenticated production role switching, P2 invalid audit timestamp acceptance, and P2 invalid calendar coverage windows. Resolving or “taking a side” automatically would be unsafe and outside the targeted unblock scope. Required CI: no exact-head workflow run is present while the PR remains unmergeable. Review threads: three blocking threads remain open. Ledger record deferred by anti-churn policy. Required human action: resolve the main conflicts with product/security review, implement and test the P1/P2 API protections, then re-run the exact-head required CI. Merge left to the user. BLOCKED — 12 current-main conflicts plus one P1 and two P2 unresolved review findings require a human-led security and conflict resolution. |
… close the denylist Half B of the final whole-branch review — the API surface, the schema, and the static gates. The sealed domain and both stores are another agent's half and are untouched here, including for the two mutation proofs, which run against copies. Finding 1 — the service-state POST reply leaked the incident note. writeHandler serialises the store's return value, and approveServiceRestart hands back the still-stopped ServiceState for the first and second approvals. The write's capability is checked against the actor's own team; the note is releasable only to the reporting team, so those are different questions and a second team's teamLead could legitimately approve and receive a note they may not read. Both writes now return the narrowed view rather than the record. Emptying the reply was rejected: the approver needs to see the stop still standing and their own approval among restartApprovals. Finding 2 — a recovery-only overlay opened with focus on document.body (WCAG 2.4.3), because it renders no header and so has no close control for the shared Sheet's fallback to find. Fixed with the hook Sheet already exposes: data-sheet-autofocus on the action control when the overlay is not dismissible. sheet.tsx is unchanged, so no blast radius. The filed request assumed a shared design-system change; an immutable cancellation records that its premise was wrong and where the fix landed. Finding 3 — the session route suppressed logging for its whole catch, so the only genuine 500 it can produce was the one fault that never reached the logs. The suppression is now scoped to the expected PublicApiError. Finding 4 — migration 0004 makes audit_events append-only: a before update or delete trigger that raises for everyone including the schema owner, plus the table's grant narrowed to select, insert. audit-integrity-loss halts the whole service, so the trail could not stay editable by the thing it audits. One transaction, no concurrent index build. TRUNCATE fires statement-level triggers only, so the suite's truncation is untouched — asserted, not assumed. Finding 5 — a source-text guard pins ensureTeam as an unconditional insert and as the first statement of runWrite's transaction. That accident serialises same-team writers, and removing it would widen the concurrency surface of every write with no behavioural test going red. Finding 7 — domain isolation was a denylist that let twilio, redis and stripe through, with a regex that could not see require() or dynamic import(). Now an allowlist of node: and relative specifiers, over a widened extractor. Finding 6 — deliberately not fixed. The suite is not unexecuted (174 passed); what is missing is automation. caring-contacts:db:test hard-fails without a Postgres container, so wiring it into any offline verify:* chain would break every offline run, and a dedicated alias would be a check that cannot fail. The CI job needs filing; the contract test becomes honest once it exists.
The row-level-security suite is collected by no automated gate. It is NOT unexecuted - it ran repeatedly during Phase 2A, most recently at 174 passed - but nothing would catch a future regression. Deliberately not fixed inside the final fix wave: the suite hard-fails without a Postgres container, so wiring it into an offline verify chain would break every offline run, and a dedicated alias would be a check that cannot fail. It needs a CI job with a service container, which is a repository-wide change with its own review.
…erform the clearance
Half A of the final whole-branch review — the sealed domain and both stores.
The API surface, migrations and static gates are another agent's half and are
untouched here, including for the mutation proofs, which ran against copies and
were reverted.
Finding 1 (CRITICAL) — fingerprintOf returned a canonical STRING and both stores
inserted it verbatim into idempotency_records.fingerprint, so createPlan wrote
the patient's name, mobile number, identifiers and cultural identity in
plaintext, and stopService/resolveDispatchDiscrepancy wrote the responder's
incident note. Now SHA-256 inside the shared definition, so the two stores
cannot drift. Equality is the only comparison either store makes, and the new
fingerprint test pins replay semantics from both directions rather than only
asserting the plaintext is gone. Proof: with the hash temporarily reverted, the
new Postgres row test fails on the real row —
expected '[{"fingerprint":"{input:{patientId:st…' not to contain 'Jordan Nguyen'.
The stored `result` was NOT stripped: it is the answer a replay must return, and
no assertion pinned it, which makes silent stripping more dangerous not less.
ServiceState.note in approveServiceRestart's row is recorded as a known residual
in repository.ts and the report; the fix spans this contract and wave B's route.
Finding 2 (CRITICAL) — a coverage window was validated only by `until > from`, a
lexical compare that "cherry" > "banana" satisfies, so nonsense was accepted and
stored by the in-memory store while Postgres's regex check raised and escaped as
a throw. schedule.ts now exports isAwstCalendarDay (stricter than the SQL check:
2026-02-30 is refused too) and applyAssignmentAction refuses
coverage-window-not-calendar-day before the ordering test. The route schema is
wave B's file and still needs tightening.
Finding 3 (CRITICAL) — the in-memory store stored the caller's snapshot object
and returned it through a shallow copy, so every reader held the live governed
message text, rewritable with no version bump and no audit event. Postgres
round-trips through jsonb and copied for free, so the two stores genuinely
differed on the one type carrying clinical content. clonePathwayVersion now
deep-copies and freezes on the write path, the transition result and both reads.
Finding 4 — the permissions suite's ROLES was a hand-written array typed as a
readonly array rather than a tuple, so a sixth role was forced into the grant
table by the compiler and silently absent from every "for every role" claim.
Derived from Object.keys now. Proof both ways: with a sixth role added, the
death-unblockable safety-stop test reddens; with the same mutation and the old
hand-written list, it passes and only the new derivation guard catches it.
Finding 5 — canPerformCaringContactAction indexed frozen object literals, so an
unknown-shaped role threw a TypeError instead of refusing, and the `?? []` beside
it was not a guard because a function is not nullish. Third occurrence on this
branch; now one shared Object.hasOwn helper at both sites. The audit of the rest
of the domain found the same shape in the Postgres store's constraint-refusal map
(fixed) and in two wording tables whose fix needs a product decision (reported).
Finding 6 — markRetentionCleared wrote cleared_at and nothing else, so every
identifying field survived and getEpisode returned all of them, while anything
reading the record concluded otherwise. Both stores now de-identify in the same
transaction that records the clearance, against one shared CLEARED_PATIENT_DETAIL,
including the cultural-identity projection that lives outside the plan row. No
existing assertion was changed; two contract cases were added.
Finding 7 — TERMINAL_PLAN_STATES was declared three times and READ_ACTIONS,
PATHWAY_VERSION_READ_ACTIONS, DISPATCHED_CONTACT_STATES, isTerminalPlan and
outcomeFor twice each, the Postgres copies under a comment saying they were kept
identical on purpose. READ_ACTIONS is an access-control rule, not a value list.
All lifted to model.ts and repository.ts; retention.ts's parallel list left alone.
Gates: npm run test — Test Files 703 passed | 2 skipped (705), Tests 7818 passed
| 29 skipped (7847). caring-contacts:db:test — Test Files 2 passed (2), Tests 174
passed (174). tsc --noEmit and eslint clean for every file in this scope.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PR #2279 unblock summary
Ledger record deferred by anti-churn policy. Merge left to the user BLOCKED |
…note is never stored
Closing round on the wave-A review, authorised against the "no second fix
wave" rule because the exposure was created by the wave split.
Ruling 65 — approveServiceRestart returned the whole ServiceState, and both
stores persist a write's return value as its idempotency replay result under
the WRITING team's id. Restart approvals are service-wide, so a TEAM-SOUTH
teamLead legitimately approves a TEAM-NORTH incident and the reporting team's
note came to rest in a row TEAM-SOUTH can select under row-level security --
while narrowServiceStateForActor withheld that same note from that same actor
at the API boundary. A boundary cannot reach a stored row; only the return type
can. The domain now has ServiceRestartOutcome (ServiceState minus note,
stoppedBy and reportedByTeamId) and restartOutcomeOf, so both stores narrow
identically, and the reply and the stored result are clean by construction
rather than by filtering -- a replay stays truthful because the original answer
never carried it either. Checked before building on it: no consumer needed the
note. The route was the only production reader and already discarded it,
describeServiceStop cannot see it by type, and no component reads the POST
reply. ServiceRestartView takes no actor, deliberately: there is nothing left
to gate. Proof: with the postgres store's narrowing reverted, the new
cross-team row test fails on the real row -- expected
'[{"fingerprint":"11a35125e1c762b50e14…' not to contain 'Rowan'. The
fingerprint column is already an opaque hash; the leak was the result column
beside it.
Concern 3, decided as harden not fallback — STOP_REASON_WORDING and
ALERT_CLASS_LABELS are frozen object literals keyed by closed unions, so
["constructor"] was a function about to be interpolated into a sentence. Both
now guard with Object.hasOwn and throw naming the key, per Ruling 61: an
unwritten wording is a programming error, and a fallback would render a
plausible sentence for a value nobody defined -- on the banner whose whole job
is to say truthfully why sending has halted. Twelve cases red first with
"expected [Function] to throw an error". Fifth and sixth sites of this pattern
closed on this branch; each needed its own fix.
Concern 2 — the assignment route accepted from/until as any non-empty string,
so the domain's named refusal was the only thing between nonsense and the
database check. The schema now refines on the same isAwstCalendarDay predicate
the domain uses, imported rather than restated. Red first at 422 (the store's
refusal coming back through the handler, i.e. the malformed request reaching
the store); 400 now, with the store asserted never called.
Gates: npm run test — Test Files 703 passed | 2 skipped (705), Tests 7834
passed | 29 skipped (7863). caring-contacts:db:test — Test Files 2 passed (2),
Tests 177 passed (177). tsc --noEmit clean, npm run lint clean at
--max-warnings 0 (the reported in-memory-repository.ts:29 warning was in-flight
and is gone), prettier --check clean. Both usually-environmental tests passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Final head SHA (remote, unchanged): Blocker found and disposition
While proving the merge locally I also found and fixed two real (pre-existing on the PR's own commits, unrelated to the conflict) problems that would otherwise fail required CI once unblocked:
I could not publish this commit. Pushing it hit this repository's local What's needed from a human: push the locally-resolved commit for me, or explicitly authorize Sync / merge-tree statusReal conflict (not staleness) — resolved locally as described above; not yet pushed. Targeted verification performed (local, on the resolved merge commit)
No provider-backed checks ( Required-CI status and review-thread statusRequired CI has not run on any new SHA because nothing new has been pushed; the last observed required-CI state at Residual risk / required human action
Merge left to the user. BLOCKED Generated by Claude Code |
Pull request was converted to draft
…roll away Task 19's browser proof measured the full service-state banner scrolling completely out of view at 320, 390, 430 and 768px (y from -285 to -602), because it sits in normal flow beneath a sticky header. Spec 4.2 requires the stop stated on every screen while it is active. The owner's decision: a condensed one-line bar, pinned only once the full banner has gone -- not the full banner pinned, which costs about a quarter of a phone screen at all times. The bar is an absolutely positioned child of the sticky header at top-full, so it rides the header with no pin offset (the header measures 87.5px at 320/390 and 65px above, against a 64px --header-h token), moves no content as it appears, and inherits the header's stacking context so it introduces no new z-index at all. The incident note stays structurally unreachable: the bar renders from ServiceStopBannerFacts, which omits `note` by construction, and the one new Client Component takes no props at all -- it decides when the bar shows and never what it says. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…tate honestly Adds the full-suite section the report was missing: `npm run test` is not green on this workstation, two runs of the same tree gave different failure sets, and four of those failures reproduce with the change reverted. None is in caring-contacts. Reported rather than rounded to green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n commit landed mid-task
Summary
Verification
npm run verify:pr-localDuring development, use
npm run verify:cheapas the faster iteration gate before the final PR-local preflight.npm run verify:uiwhen UI, routing, styling, browser behavior, reduced-motion, or forced-colors behavior changednpm run verify:releasebefore release or handoff confidence claimsFor retrieval, ranking, selection, chunking, source/citation rendering, or answer-contract changes,
verify:pr-localrunseval:rag:offlineautomatically. Run the offline command directly during iteration before spending a live eval.npm run eval:retrieval:quality(must stay 36/36) when retrieval, ranking, selection, chunking, or scoring behavior changed — CI cannot run it (needs live keys), so run it locally and paste the summary. A metadata/governance-weighting change once buried correct docs (recall 1.0→0.76) and only this eval caught it.npm run eval:rag -- --limit 15+npm run eval:quality -- --rag-onlywhen answer generation, the synthesis prompt, or answer post-processing changed (grounded-supported must not drop; citation-failure 0)npm run check:production-readinesswhen clinical workflow, privacy, environment, Supabase, source governance, or deployment behavior changednpm run check:deployment-readinesswhen deployment startup, hosting, or rollout behavior changedRisk and rollout
Complete this section for clinical, data, API, auth/privacy, workflow, dependency, build, or deployment changes.
src/lib/rag/, retrieval RPCs, golden fixtures, ranking tests; seedocs/rag-behaviour/safeguards.md). Must use one of:RAG impact: no retrieval behaviour change — <reason>RAG impact: behaviour change — canary pair <baseline run> -> <post run>RAG impact: noneor omit for non-RAG PRs)Clinical Governance Preflight
Complete this section when the change touches ingestion, answer generation, search/ranking, source rendering, document access, privacy, production env, or clinical output.
Clinical KB Database(sjrfecxgysukkwxsowpy)Notes