Route the closing-message refusal through the validateGovernedMessage chokepoint (#59JT7W)

src/lib/caring-contacts/db/postgres-repository.ts

@@ -186,6 +186,33 @@ const PLAN_COLUMNS = `id, team_id, patient_id, referral_id, pathway_version_id,
   discharge_at, created_at, completed_at, sending_preference, patient_name, patient_mobile_number,
   patient_identifiers`;
 
+/**
+ * What a LIST read of plans selects: `PLAN_COLUMNS` minus every patient column (#RZVMPD).
+ *
+ * `PLAN_COLUMNS` above is right for `readPlanRecord` and `selectPlanForUpdate`, whose callers
+ * genuinely need the patient detail -- `getEpisode` projects all three and `markRetentionCleared`
+ * clears them. `listPlans` needs none of them: `toPlanRecord` maps the plan, the ids, the dates,
+ * the outcome, the contacts and the attestations, and touches no patient column at all. So the
+ * caseload read was pulling every patient's name, mobile number and identifier list for the whole
+ * team, on every render, to throw all three away.
+ *
+ * NOTHING WAS RELEASED, AND THAT IS NOT THE POINT. `PlanRecord` excludes `patientDetail`
+ * structurally, so the guarantee held; what did not hold is that the data need never have entered
+ * the process. This narrows in the QUERY, not only in the mapping afterwards -- the standard
+ * `listPatientNames` below already sets.
+ *
+ * `patient_name` is dropped here too, not only the mobile number and the identifiers. Names have
+ * their own read with its own capability check and its own `patientNameDirectory` access-audit
+ * object type (Ruling 91), which exists so "who read patients' names, and when" is answerable.
+ * Pulling names inside a read audited as `plan` under-counts that trail.
+ *
+ * A SEPARATE CONSTANT RATHER THAN A NARROWED `PLAN_COLUMNS`. Narrowing the shared one in place
+ * would leave `getEpisode` projecting `undefined` for three patient fields, which no type checks
+ * and which is a worse defect than the one this fixes.
+ */
+const PLAN_LIST_COLUMNS = `id, team_id, patient_id, referral_id, pathway_version_id, state, version,
+  outcome, discharge_at, created_at, completed_at, sending_preference`;
+
 const CONTACT_COLUMNS = `id, plan_id, team_id, sequence, state, version, cadence_label, calendar_day,
   send_at, message_type, suppressed_reason`;
 
@@ -2329,7 +2356,7 @@ export function createPostgresRepository(
     async listPlans(context: ReadContext) {
       if (!mayReadOwnTeam(context, READ_ACTIONS.plan)) return [];
       return runRead(context, async (connection) => {
-        const plans = await connection.query(`select ${PLAN_COLUMNS} from caring_contacts.plans order by id`);
+        const plans = await connection.query(`select ${PLAN_LIST_COLUMNS} from caring_contacts.plans order by id`);
         const contacts = await connection.query(
           `select ${CONTACT_COLUMNS} from caring_contacts.contacts order by plan_id, sequence`,
         );
@@ -2371,13 +2398,13 @@ export function createPostgresRepository(
      * the mobile number or the identifier list into the process at all -- its narrowing is in the
      * query, not only in the mapping afterwards.
      *
-     * Read that as a claim about this method, NOT about the page. `PLAN_COLUMNS` includes
-     * `patient_mobile_number` and `patient_identifiers`, and `listPlans` selects it verbatim, so the
-     * Patients directory still pulls both for its whole caseload on every render and discards them
-     * in `toPlanRecord`. What the projection changes is what is RELEASED, which is the substance:
-     * nothing outside this file can obtain those fields through it. Narrowing `listPlans`' own
-     * column list is a real privacy improvement on a hot path and is tracked separately, because it
-     * deserves its own review rather than riding along with a names read.
+     * That claim now holds of the PAGE as well, not only of this method (#RZVMPD, 2026-09-02).
+     * `listPlans` used to select `PLAN_COLUMNS` verbatim, so the Patients directory pulled every
+     * patient's name, mobile number and identifier list for its whole caseload on every render and
+     * discarded all three in `toPlanRecord`. It selects `PLAN_LIST_COLUMNS` instead, which carries
+     * no patient column at all -- so this projection is once again the only list-shaped read in
+     * this store that touches a name, which is what makes its `patientNameDirectory` access-audit
+     * entry a complete answer to "who read patients' names".
      */
     async listPatientNames(context: ReadContext) {
       if (!mayReadAllOwnTeam(context, PATIENT_NAME_READ_ACTIONS)) return [];

src/lib/caring-contacts/message-policy.ts

@@ -72,7 +72,16 @@ export function calculateGsm7(text: string): Gsm7Evidence {
 }
 
 export type GovernedMessageInput = {
-  text: string;
+  /**
+   * The fully substituted outgoing message, or `undefined` when no body was resolved at all.
+   *
+   * IT IS OPTIONAL SO THAT "NOTHING WAS AUTHORED" CAN REACH THIS FUNCTION (item A4, 2026-09-02).
+   * A required `string` left a sender holding no body with only two moves: pass `""`, which is
+   * refused for the WRONG reason (see `closing-message-body-not-authored` below), or skip the
+   * chokepoint entirely and decide for itself -- the bypass this widening exists to close. An
+   * absent body is a fact about the message, so the type says so.
+   */
+  text: string | undefined;
   messageType: MessageType;
   /** The recipient's own mobile number, if known, so it can be checked for leakage into the text. */
   patientMobileNumber?: string;
@@ -104,6 +113,20 @@ export type MessageValidationIssue =
   | { code: "first-message-missing-support-information" }
   | { code: "closing-message-missing-ending-statement" }
   | { code: "closing-message-missing-support-information" }
+  /**
+   * No closing body exists to check at all -- distinct from the two codes above, which mean a body
+   * exists and is wrong. See `resolveClosingContactMessageBody` for why the distinction is the
+   * whole point rather than a nicety.
+   */
+  | { code: "closing-message-body-not-authored" }
+  /**
+   * The same fact for a `standard` or `first` message: no body exists to send.
+   *
+   * Its OWN code rather than the closing one, because the closing case carries a specific meaning --
+   * no closing wording has ever been clinically authored (item A4) -- that says nothing about an
+   * ordinary message whose body a caller simply failed to supply.
+   */
+  | { code: "message-body-not-authored" }
   | { code: "contains-patient-mobile" }
   | { code: "solicits-reply" }
   | { code: "terminated-contact-dispatch-refused"; state: ContactState | PlanState };
@@ -117,6 +140,34 @@ export type ValidationResult = { valid: true } | { valid: false; issues: Message
 export function validateGovernedMessage(input: GovernedMessageInput): ValidationResult {
   const rules = PROVISIONAL_MESSAGE_RULES;
   const issues: MessageValidationIssue[] = [];
+
+  // NO BODY, NO SEND -- FOR EVERY MESSAGE TYPE (item A4, 2026-09-02).
+  //
+  // It returns rather than accumulating, because a message with no body has nothing for any of the
+  // TEXT checks below to read: running them anyway would report
+  // `closing-message-missing-ending-statement` -- "the body you wrote is wrong" -- about a body
+  // nobody wrote. That is a refusal either way; it is the DIAGNOSIS that would be false, and this
+  // module's reason for separating the two codes is that a maintainer reading the first one goes
+  // looking for wording to fix. There is none to fix.
+  //
+  // IT COVERS `standard` AND `first`, NOT ONLY `closing`. Widening `text` to `string | undefined`
+  // made "nothing was authored" expressible for the first time, and a rule that answered only for
+  // closing messages would have made the chokepoint say `valid: true` -- an explicit "this may be
+  // sent" -- for a standard message with no body at all. That would be a NEW permission, granted by
+  // the very change whose purpose is closing a bypass.
+  //
+  // THE STATE REFUSALS ARE STILL REPORTED. They do not read the text, so they are appended before
+  // returning. Without that, a cancelled plan with no body reported only "write a body": the
+  // recoverable condition masking the unrecoverable one, which is the mirror image of the false
+  // diagnosis above.
+  if (!messageBodyIsAuthored(input.text)) {
+    issues.push({
+      code: input.messageType === "closing" ? "closing-message-body-not-authored" : "message-body-not-authored",
+    });
+    appendStateIssues(input, issues);
+    return { valid: false, issues };
+  }
+
   const text = input.text;
 
   const gsm7 = calculateGsm7(text);
@@ -171,15 +222,39 @@ export function validateGovernedMessage(input: GovernedMessageInput): Validation
     issues.push({ code: "solicits-reply" });
   }
 
+  appendStateIssues(input, issues);
+
+  return issues.length === 0 ? { valid: true } : { valid: false, issues };
+}
+
+/**
+ * The refusals that depend on the RECORD rather than on the text -- a contact already past dispatch,
+ * or a plan that has ended.
+ *
+ * Extracted so BOTH exits from `validateGovernedMessage` report them, including the unauthored-body
+ * return above. They read no text, so there is nothing about a missing body that makes them
+ * unanswerable.
+ */
+function appendStateIssues(input: GovernedMessageInput, issues: MessageValidationIssue[]): void {
   if (input.contactState && TERMINAL_DISPATCH_REFUSED_CONTACT_STATES.includes(input.contactState)) {
     issues.push({ code: "terminated-contact-dispatch-refused", state: input.contactState });
   }
 
   if (input.planState && TERMINAL_PLAN_STATES.includes(input.planState)) {
     issues.push({ code: "terminated-contact-dispatch-refused", state: input.planState });
   }
+}
 
-  return issues.length === 0 ? { valid: true } : { valid: false, issues };
+/**
+ * Whether a body was authored at all. Blank-or-absent is the same answer, because a
+ * whitespace-only body sends whitespace.
+ *
+ * Declared here so `validateGovernedMessage` above and `resolveClosingContactMessageBody` below ask
+ * the question once. Two copies of "is anything actually written here" is how the chokepoint and
+ * its adapter would come to disagree, which is the failure this whole change closes.
+ */
+function messageBodyIsAuthored(body: string | undefined): body is string {
+  return body !== undefined && body.trim().length > 0;
 }
 
 export type ClosingMessageBodyIssue = { code: "closing-message-body-not-authored" };
@@ -200,15 +275,33 @@ export type ClosingMessageBodyResolution = { ok: true; body: string } | { ok: fa
  *
  * 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.
+ * but no body content, and nothing yet supplies one. This function is a convenience for a future
+ * sender that wants the body back on the success branch; 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.
+ *
+ * IT NO LONGER OWNS THE RULE (#59JT7W, 2026-09-02). It used to hold the refusal alone, which made
+ * the guarantee conditional on a caller CHOOSING to ask -- and a sender that resolved a closing
+ * body some other way met no refusal at all. Unlike the A1 fictional-contact check, which rides
+ * `validateGovernedMessage`, nothing obliged anyone to come here. The rule now lives in that
+ * chokepoint, which every sender must pass whatever it did to obtain a body, and this function
+ * delegates to it: it reads the single `closing-message-body-not-authored` issue and maps it back
+ * to the resolution shape. It deliberately ignores every OTHER issue the chokepoint may report --
+ * a body that exists but is wrong is a real body, and saying whether it may be SENT is the
+ * caller's own `validateGovernedMessage` call to make, not this one's.
  */
 export function resolveClosingContactMessageBody(
   authoredClosingBody: string | undefined,
 ): ClosingMessageBodyResolution {
-  if (!authoredClosingBody || authoredClosingBody.trim().length === 0) {
+  const validated = validateGovernedMessage({ text: authoredClosingBody, messageType: "closing" });
+  if (!validated.valid && validated.issues.some((issue) => issue.code === "closing-message-body-not-authored")) {
+    return { ok: false, issue: { code: "closing-message-body-not-authored" } };
+  }
+  // Unreachable unless the chokepoint stops reporting the issue for an unauthored body, which the
+  // contract test in tests/caring-contacts-message-policy.test.ts pins. Narrowed rather than
+  // asserted non-null so the impossible case still refuses rather than returning `undefined` as a
+  // body -- "never an empty string, never a silent fall-back" applies to this line too.
+  if (authoredClosingBody === undefined) {
     return { ok: false, issue: { code: "closing-message-body-not-authored" } };
   }
   return { ok: true, body: authoredClosingBody };

src/lib/caring-contacts/schedule-view.ts

@@ -29,7 +29,10 @@
 //
 // IT RELEASES NO PATIENT IDENTITY. Every field below comes from a `PlanRecord`, which carries none
 // -- the patient is named only by the synthetic `patientId`. A screen that needs names reads them
-// through `listPatientNames`, which is audited in its own right.
+// through `listPatientNames`, which is audited in its own right. Since #RZVMPD that is true of the
+// QUERY too and not only of the type: `listPlans` selects `PLAN_LIST_COLUMNS`, which names no
+// patient column, so a caseload render no longer pulls names, mobile numbers and identifiers into
+// the process merely to discard them.
 //
 // Pure and deterministic: no clock, no storage, no ambient time. The day being asked about is an
 // argument.

tests/caring-contacts-domain-isolation.test.ts

@@ -149,8 +149,9 @@ describe("caring-contacts properties that only a source scan can hold", () => {
 
   it("never fetches the first-contact reason for a list read", () => {
     // `first_contact_reason` is free text a clinician wrote about one patient. It is deliberately
-    // absent from `PLAN_COLUMNS` -- the list `readPlanRecord` and `listPlans` select -- so rendering
-    // a caseload never pulls it into the process at all.
+    // absent from `PLAN_COLUMNS` -- the list `readPlanRecord` and `selectPlanForUpdate` select -- so
+    // rendering a caseload never pulls it into the process at all. (`listPlans` selects the narrower
+    // `PLAN_LIST_COLUMNS`, guarded separately below.)
     //
     // Nothing observable through the repository can hold that. `toPlanRecord` maps field by field,
     // so adding the column to `PLAN_COLUMNS` fetches a clinical note for every plan in the team and
@@ -255,6 +256,43 @@ describe("caring-contacts properties that only a source scan can hold", () => {
     expect(declaration?.[1]).not.toContain("preferred_name");
   });
 
+  it("never fetches any patient column for a list read (#RZVMPD)", () => {
+    // The same argument as the two scans above, applied to the columns those two took as GIVEN.
+    // `first_contact_reason` and `preferred_name` were kept out of the list read because they are
+    // patient content -- while the list read went on selecting the patient's name, mobile number
+    // and identifier list for the whole caseload on every render, and `toPlanRecord` discarded all
+    // three. The narrowing was in the mapping, which releases nothing but fetches everything; it
+    // is now in the query.
+    //
+    // Nothing observable through the repository can hold this, for exactly the reason the other
+    // two scans exist: widening `PLAN_LIST_COLUMNS` back out changes no behaviour and leaves every
+    // behavioural test green. Only a scan of the query text can fail.
+    const source = postgresStore();
+    const listDeclaration = /const PLAN_LIST_COLUMNS = `([\s\S]*?)`;/.exec(source);
+
+    // Positive control: the constant was found and really is a plan column list. `state` is chosen
+    // deliberately -- it must survive any future narrowing, unlike the patient columns asserted
+    // absent below, so this control cannot be satisfied by an empty or truncated match.
+    expect(listDeclaration).not.toBeNull();
+    expect(listDeclaration?.[1]).toContain("state");
+    expect(listDeclaration?.[1]).toContain("discharge_at");
+
+    for (const column of [
+      "patient_name",
+      "patient_mobile_number",
+      "patient_identifiers",
+      "first_contact_reason",
+      "preferred_name",
+    ]) {
+      expect(listDeclaration?.[1]).not.toContain(column);
+    }
+
+    // And the list read really does use it. Without this the constant could be narrowed correctly
+    // and never wired up, which is the failure mode that reads as a fix.
+    expect(source).toMatch(/select \$\{PLAN_LIST_COLUMNS\} from caring_contacts\.plans order by id/);
+    expect(source).not.toMatch(/select \$\{PLAN_COLUMNS\} from caring_contacts\.plans order by id/);
+  });
+
   it("never derives the preferred name from the stored patient name", () => {
     // Owner decision, 2026-08-26, and the one a later "simplification" is most likely to undo: the
     // preferred name is ASKED FOR. A store that split `patient_name` would greet `Mr John Smith` as