From f89a1566ac12356831b6fbbaed1e6d975cb5a2d5 Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Sat, 17 Oct 2026 18:28:56 +0800 Subject: [PATCH 1/6] clinical:review: let the owner sign sources reviewed only in prose A source marked locally_reviewed with no attestation field at all (the review lives only in dispositionReason/notes) is now offered by clinical:review --kind source like a drafted source, so the owner can record attestedBy/attestedAt/attestedAgainstSha256 for it. Signing leaves validationStatus as locally_reviewed. Any partial or complete attestation, approved or rejected records are still refused. The TTY requirement and full-name signer rule are unchanged. The developer-hub ledger and owner queue now count and list these as ready to sign (the two OCP WA standards). Owner decision, 17 Oct 2026. --- docs/clinical-sign-off-how-to.md | 34 ++++ docs/source-acquisition-protocol.md | 6 +- scripts/lib/signoff-kinds/sources.mjs | 25 ++- src/lib/developer-area/sign-off-queue.ts | 81 +++++++-- tests/developer-sign-off-page.dom.test.tsx | 4 +- tests/sign-off-queue.test.ts | 32 +++- tests/sign-off-today.test.ts | 8 +- tests/signoff-sources.test.ts | 202 ++++++++++++++++++++- 8 files changed, 351 insertions(+), 41 deletions(-) diff --git a/docs/clinical-sign-off-how-to.md b/docs/clinical-sign-off-how-to.md index 826f73e141..23c2e3fd0e 100644 --- a/docs/clinical-sign-off-how-to.md +++ b/docs/clinical-sign-off-how-to.md @@ -255,6 +255,34 @@ npm run therapy:review -- --write --walk --reviewed-by "Dr " 47 Therapy records list no references yet. The tool leaves them out, because the source correspondence question cannot be answered yes without a source; they stay awaiting review until one is adopted. Type `skip` at any question to move on to the next record without saving the one on screen. +## Signing off sources + +Candidate clinical sources in the source acquisition ledger are signed with the same tool. +Sign one source by its id (the tool shows it), or walk the whole queue: + +```bash +npm run clinical:review -- --write --kind source --code "" --reviewed-by "Dr " +``` + +```bash +npm run clinical:review -- --write --walk --kind source --reviewed-by "Dr " +``` + +The tool offers two kinds of source: + +- **Unverified sources** that nobody has reviewed yet. Signing one marks it `locally_reviewed`. +- **Sources already marked `locally_reviewed` whose review was written down only in words**, + with no recorded signer, date or content pin (owner decision, 2026-10-17). These show as + waiting in the sign-off ledger. Signing one records your name, the time and a pin of the + record's current content; its status stays `locally_reviewed`. Read the record on screen + again before you sign: you are vouching for it as it reads today, not as it read when it was + first reviewed. + +A source that already carries any part of a recorded sign-off (a signer, a date or a content +pin) is not offered and the tool refuses it by its id, unless its content has changed since it +was signed. Signing a source never adopts it: whether clinical content cites it is a separate +decision. To send a source sign-off, add `src/data/source-acquisitions.json` in step 11 below. + ## Indigenous content is never signed off here Owner rule, 2026-09-26. Any record that mentions Aboriginal or Torres Strait Islander people, @@ -352,6 +380,12 @@ a session: git add src/data/formulation-concepts.json src/data/formulation-content.json ``` + For sources: + + ```bash + git add src/data/source-acquisitions.json + ``` + For Therapy: ```bash diff --git a/docs/source-acquisition-protocol.md b/docs/source-acquisition-protocol.md index a0ff041dd4..1b8d056be1 100644 --- a/docs/source-acquisition-protocol.md +++ b/docs/source-acquisition-protocol.md @@ -119,7 +119,11 @@ A record may also carry `attestedBy`, `attestedAt` and `attestedAgainstSha256`. optional, and all three are the owner's to write; an agent never records a sign-off on the owner's behalf. The owner writes them with `npm run clinical:review -- --kind source` (one at a time with `--walk`, or a whole review pack with `--pack` then `--write --batch`), which sets -`validationStatus` to `locally_reviewed` and computes the digest. Signing a source does not adopt +`validationStatus` to `locally_reviewed` and computes the digest. The tool also offers a source +already marked `locally_reviewed` whose review lives only in prose, meaning it has no +attestation field at all; signing it records the three fields and leaves `validationStatus` as +it is (owner decision, 2026-10-17). A record with any attestation field, complete or partial, +is not offered this way. Signing a source does not adopt it: `disposition` stays as it is until a separate adoption decision. Sources with Aboriginal, Torres Strait Islander or other Indigenous content are never offered (owner rule 2026-09-26). diff --git a/scripts/lib/signoff-kinds/sources.mjs b/scripts/lib/signoff-kinds/sources.mjs index c847fcc1b5..21157d2569 100644 --- a/scripts/lib/signoff-kinds/sources.mjs +++ b/scripts/lib/signoff-kinds/sources.mjs @@ -23,9 +23,12 @@ * cannot carry them. * * Status: a record in `acquisitionReviewQueue()` (not rejected, still unverified) is - * `drafted`; one that is locally_reviewed or approved AND names its reviewer in attestedBy - * is `reviewed`; everything else (rejections, and reviews recorded only in prose) is - * `pending`. Walk order is the queue's: most local rung first, then title. + * `drafted`, and so is one marked locally_reviewed with no attestation field at all (a review + * recorded only in prose, which the owner can now sign properly; signing leaves its + * validationStatus as it is). One that is locally_reviewed or approved AND names its reviewer + * in attestedBy is `reviewed`; everything else (rejections, approved records reviewed only in + * prose, and any partial attestation) is `pending`. Walk order: most local rung first, then + * title. * * Signing a source does not adopt it. `disposition` stays `candidate` until clinical content * is written to cite it, which is a separate, deliberate edit. @@ -75,8 +78,24 @@ const presentText = (value) => (typeof value === "string" && value.trim() ? valu /** What signing leaves in validationStatus; mirrors signedValidationStatus in the rating helper. */ const signedValidationStatus = (status) => (status === "unverified" ? "locally_reviewed" : status); +/** + * Marked locally_reviewed with the review recorded only in prose: no attestation field at all + * (not even an empty or null one). The owner may sign it with this tool like a drafted source, + * which records the structured attestation the ledger lacks (owner decision, 2026-10-17). A + * record carrying any attestation field, complete or partial, is never treated this way. + */ +export function isUnattestedLocalReview(native) { + return ( + isPlainRecord(native) && + native.disposition !== "rejected" && + native.validationStatus === "locally_reviewed" && + ATTESTATION_FIELDS.every((key) => !Object.hasOwn(native, key)) + ); +} + function sourceStatus(native) { if (native.disposition !== "rejected" && native.validationStatus === "unverified") return "drafted"; + if (isUnattestedLocalReview(native)) return "drafted"; if (REVIEWED_VALIDATION.has(native.validationStatus) && presentText(native.attestedBy)) return "reviewed"; return "pending"; } diff --git a/src/lib/developer-area/sign-off-queue.ts b/src/lib/developer-area/sign-off-queue.ts index 5983c27616..1736809fca 100644 --- a/src/lib/developer-area/sign-off-queue.ts +++ b/src/lib/developer-area/sign-off-queue.ts @@ -27,6 +27,7 @@ import { acquisitionAttestedContentSha256, acquisitionReviewQueue, sourceAcquisitionRecords, + type SourceAcquisitionRecord, } from "@/lib/sources/acquisition-ledger"; import { conceptReviewState, @@ -516,29 +517,67 @@ function therapyFamily(): SignOffFamily { }; } +const SOURCE_ATTESTATION_FIELDS = ["attestedBy", "attestedAt", "attestedAgainstSha256"] as const; +const SOURCE_TITLE_COLLATOR = new Intl.Collator("en-AU"); + +/** + * Marked locally_reviewed, not rejected, with no attestation field at all: the review lives only + * in prose. Mirrors isUnattestedLocalReview in scripts/lib/signoff-kinds/sources.mjs, which is + * what lets clinical:review sign it. + */ +function isUnattestedLocalSourceReview(record: SourceAcquisitionRecord): boolean { + return ( + record.disposition !== "rejected" && + record.validationStatus === "locally_reviewed" && + !SOURCE_ATTESTATION_FIELDS.some((field) => Object.hasOwn(record, field)) + ); +} + function sourcesFamily(): SignOffFamily { - // Reuses acquisitionReviewQueue rather than re-deriving the filter: that + // Reuses acquisitionReviewQueue rather than re-deriving its filter: that // function already encodes which records count as awaiting sign-off (not // rejected, still unverified) and the worst-rung-first order the source // acquisition protocol reads them in. - const rows = acquisitionReviewQueue().map((record) => ({ - family: "sources", - key: `source-acquisition:${record.id}`, - id: record.id, - title: record.title, - nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung})`, - statusLabel: "Captured, not yet verified", - requires: - "The owner verifies the source against the acquisition protocol before any clinical content may cite it; an adopted source with validationStatus unverified is already a ledger error.", - href: null, - signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), - })); + // Reviews recorded only in prose are offered by clinical:review too (owner decision, + // 2026-10-17), so the owner can record the structured attestation they lack. They are + // merged into the queue's own order (worst rung first, then title), as the tool walks them. + const ordered = [...acquisitionReviewQueue(), ...sourceAcquisitionRecords.filter(isUnattestedLocalSourceReview)].sort( + (left, right) => left.rung - right.rung || SOURCE_TITLE_COLLATOR.compare(left.title, right.title), + ); + const rows = ordered.map((record) => + isUnattestedLocalSourceReview(record) + ? { + family: "sources", + key: `source-acquisition:${record.id}`, + id: record.id, + title: record.title, + nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung}), no attestation`, + statusLabel: "Reviewed in prose only, not yet signed", + requires: + "The owner re-reads the source as it reads today and signs it, which records attestedBy, attestedAt and attestedAgainstSha256; validationStatus stays locally_reviewed.", + href: null, + signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), + } + : { + family: "sources", + key: `source-acquisition:${record.id}`, + id: record.id, + title: record.title, + nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung})`, + statusLabel: "Captured, not yet verified", + requires: + "The owner verifies the source against the acquisition protocol before any clinical content may cite it; an adopted source with validationStatus unverified is already a ledger error.", + href: null, + signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), + }, + ); return { id: "sources", name: "Source acquisitions", source: "src/data/source-acquisitions.json", - nativeField: "disposition + validationStatus, via acquisitionReviewQueue()", - note: "Worst rung first, the order the source acquisition protocol reads them in. Rejected records are excluded by that queue, not by this page.", + nativeField: + "disposition + validationStatus, via acquisitionReviewQueue(), plus locally_reviewed records with no attestation field", + note: "Worst rung first, then title: the order the source acquisition protocol reads them in, with reviews recorded only in prose merged in. Rejected records are excluded.", unrouted: true, rows, }; @@ -1534,18 +1573,20 @@ function sourcesDomain(): SignOffDomainSummary { reason: "Held for Aboriginal and Torres Strait Islander governance review.", href: null, }); - } else if (record.validationStatus === "unverified") { + } else if (record.validationStatus === "unverified" || isUnattestedLocalSourceReview(record)) { + // Unverified, or locally_reviewed in prose only with no attestation field at all: + // clinical:review offers both as drafted (scripts/lib/signoff-kinds/sources.mjs). signableNow += 1; } else { - // Reviewed only in prose: the source sign-off kind reads this as `pending`, and - // clinical:review signs drafted (unverified) sources only, so it is not signable now. + // Approved in prose only, or a partial or stale attestation: the source sign-off kind + // does not offer it, so it is not signable now. prerequisiteRows.push({ domain: "sources", domainName: "Source acquisitions", key: `prereq-source:${record.id}`, id: record.id, title: record.title, - reason: `Marked ${record.validationStatus} without a structured attestation (attestedBy, attestedAt, attestedAgainstSha256 over its current content), so it does not count as signed; clinical:review only signs unverified sources.`, + reason: `Marked ${record.validationStatus} without a complete, current structured attestation (attestedBy, attestedAt, attestedAgainstSha256 over its current content), so it does not count as signed; clinical:review signs only unverified sources and locally_reviewed ones with no attestation field.`, href: null, }); } @@ -1562,7 +1603,7 @@ function sourcesDomain(): SignOffDomainSummary { indigenousHoldRows, awaitingPrerequisites: prerequisiteRows.length, prerequisiteRows, - needsSigningSummary: `${signableNow} source(s) ready to sign now; ${indigenousHoldRows.length} Indigenous health and SEWB source(s) held for Aboriginal governance review; ${prerequisiteRows.length} reviewed only in prose and awaiting a structured attestation.`, + needsSigningSummary: `${signableNow} source(s) ready to sign now; ${indigenousHoldRows.length} Indigenous health and SEWB source(s) held for Aboriginal governance review; ${prerequisiteRows.length} awaiting a complete structured attestation the tool cannot yet record.`, queueFamilyId: "sources", }); } diff --git a/tests/developer-sign-off-page.dom.test.tsx b/tests/developer-sign-off-page.dom.test.tsx index 67999a2c5b..279c5464f8 100644 --- a/tests/developer-sign-off-page.dom.test.tsx +++ b/tests/developer-sign-off-page.dom.test.tsx @@ -90,9 +90,9 @@ describe("clinical sign-off queue page", () => { render(); expect(screen.getByTestId("developer-sign-off-count-signed-value")).toHaveTextContent(String(ledger.totalSigned)); - expect(screen.getByTestId("developer-sign-off-count-signable-now-value")).toHaveTextContent("0"); + expect(screen.getByTestId("developer-sign-off-count-signable-now-value")).toHaveTextContent("2"); expect(screen.getByTestId("developer-sign-off-count-indigenous-hold-value")).toHaveTextContent("26"); - expect(screen.getByTestId("developer-sign-off-count-prerequisites-value")).toHaveTextContent("610"); + expect(screen.getByTestId("developer-sign-off-count-prerequisites-value")).toHaveTextContent("608"); for (const domain of ledger.domains) { const card = screen.getByTestId(`developer-sign-off-ledger-domain-${domain.id}`); diff --git a/tests/sign-off-queue.test.ts b/tests/sign-off-queue.test.ts index 1d7dd93438..c3887ab0e1 100644 --- a/tests/sign-off-queue.test.ts +++ b/tests/sign-off-queue.test.ts @@ -15,7 +15,7 @@ import { dictionaryDefinitionReviews, isDefinitionReviewClinicallyApproved, } from "@/lib/dictionary-editorial/definition-reviews"; -import { acquisitionReviewQueue } from "@/lib/sources/acquisition-ledger"; +import { acquisitionReviewQueue, sourceAcquisitionRecords } from "@/lib/sources/acquisition-ledger"; import { isSpecifierClinicianReviewed, loadSpecifiersContent, specifierCatalogItems } from "@/lib/specifiers-content"; import { formulationConcepts, formulationGuides } from "@/lib/formulation-concepts"; import { conceptReviewState } from "@/lib/formulation-review-status"; @@ -57,8 +57,9 @@ const EXPECTED: Record = { // no references yet (47 originally unsigned - 22 that gained references + 1 wake-therapy). therapy: 26, // All 93 signable candidates and the 3 adopted medicine sources were reviewed; six Indigenous - // sources remain held. - sources: 6, + // sources remain held, and two OCP WA standards reviewed only in prose wait for the owner to + // sign them with clinical:review (owner decision, 17 Oct 2026). + sources: 8, }; describe("clinical sign-off queue", () => { @@ -134,7 +135,23 @@ describe("clinical sign-off queue", () => { it("reuses the source acquisition ledger's own queue rather than re-deriving the filter", () => { const sources = queue.families.find((family) => family.id === "sources")!; - expect(sources.rows.map((row) => row.id)).toEqual(acquisitionReviewQueue().map((record) => record.id)); + const proseOnly = sourceAcquisitionRecords.filter( + (record) => + record.disposition !== "rejected" && + record.validationStatus === "locally_reviewed" && + !["attestedBy", "attestedAt", "attestedAgainstSha256"].some((field) => Object.hasOwn(record, field)), + ); + // The ledger's queue plus the reviews recorded only in prose (which clinical:review also + // offers), in the queue's own order: most local rung first, then title. + const collator = new Intl.Collator("en-AU"); + expect(sources.rows.map((row) => row.id)).toEqual( + [...acquisitionReviewQueue(), ...proseOnly] + .sort((left, right) => left.rung - right.rung || collator.compare(left.title, right.title)) + .map((record) => record.id), + ); + expect( + sources.rows.filter((row) => row.statusLabel === "Reviewed in prose only, not yet signed").map((row) => row.id), + ).toEqual(["ocp-wa-standard-authorisation-of-hospitals", "ocp-wa-standards-for-clinical-care"]); }); it("carries the whole dictionary editorial layer, which nothing else in the app renders", () => { @@ -301,7 +318,8 @@ const EXPECTED_SIGNED_BY_DOMAIN: Record { PsychSift: 477, }); expect(ledger.waitingByCategory).toEqual({ - signableNow: 0, + signableNow: 2, indigenousHold: 26, - awaitingPrerequisites: 610, + awaitingPrerequisites: 608, awaitingTool: 565, }); }); diff --git a/tests/sign-off-today.test.ts b/tests/sign-off-today.test.ts index 7c2af99954..0f8cef05ac 100644 --- a/tests/sign-off-today.test.ts +++ b/tests/sign-off-today.test.ts @@ -120,7 +120,13 @@ describe("the real queue", () => { // 16 Oct 2026: the 27 corrected Therapy records and the 3 adopted medicine sources // have been signed off by "Psych Sift", so no signable rows remain; everything still // waiting is an Indigenous governance hold or has no sign-off tool / references. - expect(signable.map((item) => item.family)).toEqual([]); + // 17 Oct 2026: clinical:review now offers sources reviewed only in prose (owner decision), + // so the two OCP WA standards are signable by the owner. + expect(signable.map((item) => item.family)).toEqual(["sources", "sources"]); + expect(signable.map((item) => item.id)).toEqual([ + "ocp-wa-standard-authorisation-of-hospitals", + "ocp-wa-standards-for-clinical-care", + ]); expect(today.signable).toBe(signable.length); expect(today.rows.length).toBe(Math.min(SIGN_OFF_TODAY_SIZE, signable.length)); for (const item of today.rows) { diff --git a/tests/signoff-sources.test.ts b/tests/signoff-sources.test.ts index a881d72a4d..6e50a865b3 100644 --- a/tests/signoff-sources.test.ts +++ b/tests/signoff-sources.test.ts @@ -1,4 +1,9 @@ -import { describe, expect, it } from "vitest"; +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { PassThrough } from "node:stream"; +import { afterEach, describe, expect, it } from "vitest"; import { applyClinicalReview, @@ -14,10 +19,12 @@ import { import { display, duplicateSources, + isUnattestedLocalReview, loadContext, possibleDuplicateSources, } from "../scripts/lib/signoff-kinds/sources.mjs"; -import { renderClinicalPack } from "../scripts/review-clinical-record.mjs"; +import { removePathSync } from "../scripts/retryable-fs.mjs"; +import { main, renderClinicalPack } from "../scripts/review-clinical-record.mjs"; import sourceAcquisitions from "../src/data/source-acquisitions.json"; @@ -84,15 +91,28 @@ describe("source sign-offs on disk", () => { it("offers exactly the acquisition review queue, in its order, less the records held from sign-off", () => { const queue = acquisitionReviewQueue().map((record) => record.id); + // Reviews recorded only in prose are also drafted (owner decision, 2026-10-17); the + // ledger's own queue lists unverified records only, so compare without them. const drafted = views() - .filter((record) => record.status === "drafted") + .filter((record) => record.status === "drafted" && record.validationStatus === "unverified") .map((record) => record.id); expect(drafted).toEqual(queue); - const held = views().filter((record) => record.status === "drafted" && signOffEligibilityProblem(record, "source")); - const offered = signOffQueue("source", views()); - expect(offered).toEqual(queue.filter((id) => !held.some((record) => record.id === id))); - expect(offered.length + held.length).toBe(acquisitionReviewQueue().length); + const held = views().filter( + (record) => + record.status === "drafted" && + record.validationStatus === "unverified" && + signOffEligibilityProblem(record, "source"), + ); + const unattested = views() + .filter((record) => record.status === "drafted" && record.validationStatus === "locally_reviewed") + .map((record) => record.id as string); + const offered = signOffQueue("source", views()) as string[]; + expect(offered.filter((id) => !unattested.includes(id))).toEqual( + queue.filter((id) => !held.some((record) => record.id === id)), + ); + expect(offered.filter((id) => unattested.includes(id))).toEqual(unattested); + expect(offered.length - unattested.length + held.length).toBe(acquisitionReviewQueue().length); // The owner rule of 2026-09-26 holds Indigenous content back; this one is the named example. expect(held.map((record) => record.id)).toContain("therapy-wa-aboriginal-impact-policy"); for (const record of held) expect(indigenousContentIn(record, "source")).not.toBeNull(); @@ -299,3 +319,171 @@ describe("source sign-off ratings", () => { } }); }); + +/** + * Owner decision, 2026-10-17: a source marked locally_reviewed whose review was recorded only in + * prose (no attestation field at all) can be signed with clinical:review like a drafted source, + * so the owner can record the structured attestation it lacks. Every other guard stays. + */ +describe("signing a source reviewed only in prose", () => { + const PROSE_ONLY = ["ocp-wa-standard-authorisation-of-hospitals", "ocp-wa-standards-for-clinical-care"]; + const LEDGER = join("src", "data", "source-acquisitions.json"); + // A person's full name, which the --write attribution rule accepts. + const SIGNER = "Dr Alex Morgan"; + const temporaryDirectories: string[] = []; + afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) removePathSync(directory, { recursive: true }); + }); + + const native = (id: string) => clone(ledger.find((record) => record.id === id)!); + const attestedNative = () => + clone( + ledger.find( + (record) => + record.validationStatus === "locally_reviewed" && + typeof record.attestedBy === "string" && + record.disposition !== "rejected" && + !indigenousContentIn(record, "source"), + )!, + ); + const viewOf = (record: Json) => (collectionOf("source", [record]) as Json[])[0]!; + + function fixtureRoot(records: Json[]) { + const root = mkdtempSync(join(tmpdir(), "source-signoff-")); + temporaryDirectories.push(root); + mkdirSync(join(root, "src", "data"), { recursive: true }); + writeFileSync(join(root, LEDGER), `${JSON.stringify(records, null, 2)}\n`); + return root; + } + + async function runWrite(root: string, code: string, answers: string[]) { + const input = Object.assign(new PassThrough(), { isTTY: true }); + const output = Object.assign(new PassThrough(), { isTTY: true, columns: 80 }); + const text: string[] = []; + output.on("data", (chunk: Buffer) => { + const value = chunk.toString(); + text.push(value); + if (/(Type yes, no, or quit: |type its code \([^)]*\), or quit to stop: )$/.test(value)) { + const next = answers.shift(); + if (next !== undefined) setImmediate(() => input.write(`${next}\n`)); + } + }); + const errors = new PassThrough(); + errors.resume(); + const exit = await main(["--write", "--kind", "source", "--code", code, "--reviewed-by", SIGNER], { + root, + input, + output, + errorOutput: errors, + }); + return { exit, text: text.join("") }; + } + + it("finds exactly the two OCP WA standards in the ledger today, and offers them for sign-off", () => { + const proseOnly = ledger.filter((record) => isUnattestedLocalReview(record)).map((record) => record.id); + expect([...proseOnly].sort()).toEqual(PROSE_ONLY); + for (const id of PROSE_ONLY) { + const view = views().find((record) => record.id === id)!; + expect(view).toMatchObject({ status: "drafted", validationStatus: "locally_reviewed", reviewedBy: null }); + expect(signOffEligibilityProblem(view, "source")).toBeNull(); + expect(signOffQueue("source", views())).toContain(id); + } + }); + + it("accepts a reviewed-but-unsigned source for --write and records only the attestation", async () => { + const id = "ocp-wa-standards-for-clinical-care"; + const before = [native(id), attestedNative()]; + const root = fixtureRoot(before); + const { exit, text } = await runWrite(root, id, ["yes", "yes", "yes", id]); + expect(exit).toBe(0); + expect(text).toContain(`Saved: Source ${id} is signed off.`); + + const after = JSON.parse(readFileSync(join(root, LEDGER), "utf8")) as Json[]; + expect(after[1]).toEqual(before[1]); + const changed = Object.keys({ ...before[0], ...after[0] }).filter( + (key) => JSON.stringify(after[0]![key]) !== JSON.stringify(before[0]![key]), + ); + expect(changed.sort()).toEqual(["attestedAgainstSha256", "attestedAt", "attestedBy"]); + expect(after[0]).toMatchObject({ validationStatus: "locally_reviewed", attestedBy: SIGNER }); + expect(after[0]!.attestedAgainstSha256).toBe( + acquisitionAttestedContentSha256(after[0] as unknown as SourceAcquisitionRecord), + ); + expect(acquisitionLedgerIssues(after as unknown as SourceAcquisitionRecord[])).toEqual([]); + const view = viewOf(after[0]!); + expect(view.status).toBe("reviewed"); + expect(recordPinState(view, "source")).toBe("current"); + }, 60_000); + + it("refuses a source that already carries an attestation, before asking anything", async () => { + const attested = attestedNative(); + expect(isUnattestedLocalReview(attested)).toBe(false); + expect(signOffEligibilityProblem(viewOf(attested), "source")).toMatch(/already signed off/); + const root = fixtureRoot([attested]); + const raw = readFileSync(join(root, LEDGER), "utf8"); + await expect(runWrite(root, attested.id as string, [])).rejects.toThrow(/already signed off/); + expect(readFileSync(join(root, LEDGER), "utf8")).toBe(raw); + }); + + it.each([ + ["attestedBy only", { attestedBy: "Dr Someone Else" }], + ["attestedAt only", { attestedAt: "2026-09-06T00:00:00.000Z" }], + ["attestedAgainstSha256 only", { attestedAgainstSha256: "0".repeat(64) }], + ["a null attestation field", { attestedBy: null }], + ["an empty attestation field", { attestedAt: "" }], + ])("refuses a locally_reviewed source with a partial attestation (%s)", (_label, fields) => { + const record = { ...native("ocp-wa-standards-for-clinical-care"), ...fields }; + expect(isUnattestedLocalReview(record)).toBe(false); + expect(signOffEligibilityProblem(viewOf(record), "source")).not.toBeNull(); + }); + + it.each([ + ["approved", { validationStatus: "approved" }], + ["rejected", { disposition: "rejected" }], + ])("does not offer a prose-only review that is %s", (_label, fields) => { + const record = { ...native("ocp-wa-standards-for-clinical-care"), ...fields }; + expect(isUnattestedLocalReview(record)).toBe(false); + expect(signOffEligibilityProblem(viewOf(record), "source")).toMatch(/Only a drafted source/); + }); + + it("still refuses --write for a reviewed-but-unsigned source without an interactive TTY", () => { + const path = join(process.cwd(), LEDGER); + const before = readFileSync(path, "utf8"); + for (const id of PROSE_ONLY) { + const refused = spawnSync( + process.execPath, + [ + join(process.cwd(), "scripts", "review-clinical-record.mjs"), + "--write", + "--kind", + "source", + "--code", + id, + "--reviewed-by", + SIGNER, + ], + { cwd: process.cwd(), encoding: "utf8", input: `yes\nyes\nyes\n${id}\n` }, + ); + expect(refused.status).not.toBe(0); + expect(refused.stderr).toContain("interactive TTY"); + } + expect(readFileSync(path, "utf8")).toBe(before); + }); + + it("still refuses a product name as the signer for a reviewed-but-unsigned source", async () => { + const id = "ocp-wa-standards-for-clinical-care"; + const root = fixtureRoot([native(id)]); + const raw = readFileSync(join(root, LEDGER), "utf8"); + const input = Object.assign(new PassThrough(), { isTTY: true }); + const output = Object.assign(new PassThrough(), { isTTY: true, columns: 80 }); + output.resume(); + await expect( + main(["--write", "--kind", "source", "--code", id, "--reviewed-by", "PsychSift"], { + root, + input, + output, + errorOutput: output, + }), + ).rejects.toThrow(); + expect(readFileSync(join(root, LEDGER), "utf8")).toBe(raw); + }); +}); From 755f16615c469d07f0fd9a548f571efed2806f1a Mon Sep 17 00:00:00 2001 From: "qodo-code-review[bot]" <151058649+qodo-code-review[bot]@users.noreply.github.com> Date: Sat, 17 Oct 2026 10:34:52 +0000 Subject: [PATCH 2/6] =?UTF-8?q?fix:=202=20findings=20=E2=80=94=20Count=20s?= =?UTF-8?q?tale=20attestations=20as=20signable;=20Reuse=20acquisitio?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Count stale attestations as signable - Reuse acquisition attestation fields --- src/lib/developer-area/sign-off-queue.ts | 68 +++++++++++++++++------- 1 file changed, 48 insertions(+), 20 deletions(-) diff --git a/src/lib/developer-area/sign-off-queue.ts b/src/lib/developer-area/sign-off-queue.ts index 1736809fca..3af7b1ef0a 100644 --- a/src/lib/developer-area/sign-off-queue.ts +++ b/src/lib/developer-area/sign-off-queue.ts @@ -24,6 +24,7 @@ import { therapyNeedsReview, therapyRecords } from "@/lib/therapies"; import signableTherapySlugList from "@/data/therapy-signable-slugs.json"; import { isIndigenousContent, signedOffReviewer } from "@/lib/forms-reference-sign-off"; import { + ACQUISITION_ATTESTATION_FIELDS, acquisitionAttestedContentSha256, acquisitionReviewQueue, sourceAcquisitionRecords, @@ -517,7 +518,6 @@ function therapyFamily(): SignOffFamily { }; } -const SOURCE_ATTESTATION_FIELDS = ["attestedBy", "attestedAt", "attestedAgainstSha256"] as const; const SOURCE_TITLE_COLLATOR = new Intl.Collator("en-AU"); /** @@ -529,7 +529,19 @@ function isUnattestedLocalSourceReview(record: SourceAcquisitionRecord): boolean return ( record.disposition !== "rejected" && record.validationStatus === "locally_reviewed" && - !SOURCE_ATTESTATION_FIELDS.some((field) => Object.hasOwn(record, field)) + !ACQUISITION_ATTESTATION_FIELDS.some((field) => Object.hasOwn(record, field)) + ); +} + +/** A reviewed source whose content changed after its structured sign-off. */ +function isStaleAttestedSourceReview(record: SourceAcquisitionRecord): boolean { + return ( + record.disposition !== "rejected" && + (record.validationStatus === "locally_reviewed" || record.validationStatus === "approved") && + typeof record.attestedBy === "string" && + record.attestedBy.trim().length > 0 && + typeof record.attestedAgainstSha256 === "string" && + record.attestedAgainstSha256 !== acquisitionAttestedContentSha256(record) ); } @@ -538,27 +550,43 @@ function sourcesFamily(): SignOffFamily { // function already encodes which records count as awaiting sign-off (not // rejected, still unverified) and the worst-rung-first order the source // acquisition protocol reads them in. - // Reviews recorded only in prose are offered by clinical:review too (owner decision, - // 2026-10-17), so the owner can record the structured attestation they lack. They are - // merged into the queue's own order (worst rung first, then title), as the tool walks them. - const ordered = [...acquisitionReviewQueue(), ...sourceAcquisitionRecords.filter(isUnattestedLocalSourceReview)].sort( + // Reviews recorded only in prose and stale attestations are offered by clinical:review too, + // so the owner can record or refresh the structured attestation. They are merged into the + // queue's own order (worst rung first, then title), as the tool walks them. + const ordered = [ + ...acquisitionReviewQueue(), + ...sourceAcquisitionRecords.filter((record) => isUnattestedLocalSourceReview(record) || isStaleAttestedSourceReview(record)), + ].sort( (left, right) => left.rung - right.rung || SOURCE_TITLE_COLLATOR.compare(left.title, right.title), ); const rows = ordered.map((record) => - isUnattestedLocalSourceReview(record) + isStaleAttestedSourceReview(record) ? { family: "sources", key: `source-acquisition:${record.id}`, id: record.id, title: record.title, - nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung}), no attestation`, - statusLabel: "Reviewed in prose only, not yet signed", + nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung}), stale attestation`, + statusLabel: "Edited since sign-off, needs signing again", requires: - "The owner re-reads the source as it reads today and signs it, which records attestedBy, attestedAt and attestedAgainstSha256; validationStatus stays locally_reviewed.", + "The source changed after its last sign-off; the owner re-reads it as it reads today and signs it again.", href: null, signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), } - : { + : isUnattestedLocalSourceReview(record) + ? { + family: "sources", + key: `source-acquisition:${record.id}`, + id: record.id, + title: record.title, + nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung}), no attestation`, + statusLabel: "Reviewed in prose only, not yet signed", + requires: + "The owner re-reads the source as it reads today and signs it, which records attestedBy, attestedAt and attestedAgainstSha256; validationStatus stays locally_reviewed.", + href: null, + signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), + } + : { family: "sources", key: `source-acquisition:${record.id}`, id: record.id, @@ -576,8 +604,8 @@ function sourcesFamily(): SignOffFamily { name: "Source acquisitions", source: "src/data/source-acquisitions.json", nativeField: - "disposition + validationStatus, via acquisitionReviewQueue(), plus locally_reviewed records with no attestation field", - note: "Worst rung first, then title: the order the source acquisition protocol reads them in, with reviews recorded only in prose merged in. Rejected records are excluded.", + "disposition + validationStatus, via acquisitionReviewQueue(), plus prose-only reviews and stale attestations", + note: "Worst rung first, then title: the order the source acquisition protocol reads them in, with prose-only reviews and stale attestations merged in. Rejected records are excluded.", unrouted: true, rows, }; @@ -1573,20 +1601,20 @@ function sourcesDomain(): SignOffDomainSummary { reason: "Held for Aboriginal and Torres Strait Islander governance review.", href: null, }); - } else if (record.validationStatus === "unverified" || isUnattestedLocalSourceReview(record)) { - // Unverified, or locally_reviewed in prose only with no attestation field at all: - // clinical:review offers both as drafted (scripts/lib/signoff-kinds/sources.mjs). + } else if (record.validationStatus === "unverified" || isUnattestedLocalSourceReview(record) || isStaleAttestedSourceReview(record)) { + // Unverified, locally_reviewed in prose only, or reviewed with a stale attestation: + // clinical:review offers all three for signing (scripts/lib/signoff-kinds/sources.mjs). signableNow += 1; } else { - // Approved in prose only, or a partial or stale attestation: the source sign-off kind - // does not offer it, so it is not signable now. + // Approved in prose only or a partial attestation: the source sign-off kind does + // not offer it, so it is not signable now. prerequisiteRows.push({ domain: "sources", domainName: "Source acquisitions", key: `prereq-source:${record.id}`, id: record.id, title: record.title, - reason: `Marked ${record.validationStatus} without a complete, current structured attestation (attestedBy, attestedAt, attestedAgainstSha256 over its current content), so it does not count as signed; clinical:review signs only unverified sources and locally_reviewed ones with no attestation field.`, + reason: `Marked ${record.validationStatus} without a complete, current structured attestation (attestedBy, attestedAt, attestedAgainstSha256 over its current content), so it does not count as signed; clinical:review does not offer approved prose-only or partially attested sources.`, href: null, }); } @@ -1603,7 +1631,7 @@ function sourcesDomain(): SignOffDomainSummary { indigenousHoldRows, awaitingPrerequisites: prerequisiteRows.length, prerequisiteRows, - needsSigningSummary: `${signableNow} source(s) ready to sign now; ${indigenousHoldRows.length} Indigenous health and SEWB source(s) held for Aboriginal governance review; ${prerequisiteRows.length} awaiting a complete structured attestation the tool cannot yet record.`, + needsSigningSummary: `${signableNow} source(s) ready to sign now; ${indigenousHoldRows.length} Indigenous health and SEWB source(s) held for Aboriginal governance review; ${prerequisiteRows.length} awaiting a complete structured attestation or a supported review status.`, queueFamilyId: "sources", }); } From e0f898b0fee2f933c6d0c7bbce0b80100703f872 Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Sat, 17 Oct 2026 18:40:32 +0800 Subject: [PATCH 3/6] style: Prettier on sign-off-queue.ts after the review-bot fix --- src/lib/developer-area/sign-off-queue.ts | 36 +++++++++++++----------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/lib/developer-area/sign-off-queue.ts b/src/lib/developer-area/sign-off-queue.ts index 3af7b1ef0a..c27ec39486 100644 --- a/src/lib/developer-area/sign-off-queue.ts +++ b/src/lib/developer-area/sign-off-queue.ts @@ -555,10 +555,10 @@ function sourcesFamily(): SignOffFamily { // queue's own order (worst rung first, then title), as the tool walks them. const ordered = [ ...acquisitionReviewQueue(), - ...sourceAcquisitionRecords.filter((record) => isUnattestedLocalSourceReview(record) || isStaleAttestedSourceReview(record)), - ].sort( - (left, right) => left.rung - right.rung || SOURCE_TITLE_COLLATOR.compare(left.title, right.title), - ); + ...sourceAcquisitionRecords.filter( + (record) => isUnattestedLocalSourceReview(record) || isStaleAttestedSourceReview(record), + ), + ].sort((left, right) => left.rung - right.rung || SOURCE_TITLE_COLLATOR.compare(left.title, right.title)); const rows = ordered.map((record) => isStaleAttestedSourceReview(record) ? { @@ -587,17 +587,17 @@ function sourcesFamily(): SignOffFamily { signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), } : { - family: "sources", - key: `source-acquisition:${record.id}`, - id: record.id, - title: record.title, - nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung})`, - statusLabel: "Captured, not yet verified", - requires: - "The owner verifies the source against the acquisition protocol before any clinical content may cite it; an adopted source with validationStatus unverified is already a ledger error.", - href: null, - signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), - }, + family: "sources", + key: `source-acquisition:${record.id}`, + id: record.id, + title: record.title, + nativeStatus: `${record.disposition} / validationStatus: ${record.validationStatus} (rung ${record.rung})`, + statusLabel: "Captured, not yet verified", + requires: + "The owner verifies the source against the acquisition protocol before any clinical content may cite it; an adopted source with validationStatus unverified is already a ledger error.", + href: null, + signOff: toolUnlessIndigenous({ script: "clinical:review", kind: "source", code: record.id }, record), + }, ); return { id: "sources", @@ -1601,7 +1601,11 @@ function sourcesDomain(): SignOffDomainSummary { reason: "Held for Aboriginal and Torres Strait Islander governance review.", href: null, }); - } else if (record.validationStatus === "unverified" || isUnattestedLocalSourceReview(record) || isStaleAttestedSourceReview(record)) { + } else if ( + record.validationStatus === "unverified" || + isUnattestedLocalSourceReview(record) || + isStaleAttestedSourceReview(record) + ) { // Unverified, locally_reviewed in prose only, or reviewed with a stale attestation: // clinical:review offers all three for signing (scripts/lib/signoff-kinds/sources.mjs). signableNow += 1; From 51b8840d61d2246ba2430ab1021dd914db8baeba Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Sat, 17 Oct 2026 18:48:58 +0800 Subject: [PATCH 4/6] Address review: decision entry, complete-attestation stale rule, accurate copy - Add docs/decisions/2026-10-17-sign-prose-only-source-reviews.md and a retired-rules entry for the old 'clinical:review only signs unverified sources' rule; link the decision from the code and docs - Developer hub: a pin mismatch counts as a stale (re-signable) sign-off only when all three attestation fields are present; a partial attestation stays awaiting prerequisites - Sign-off screen: for a source already locally_reviewed, say signing keeps validationStatus and does not change its rating - How-to: scope the 'already carries a sign-off' sentence to locally_reviewed sources --- docs/clinical-sign-off-how-to.md | 9 +++++---- .../2026-10-17-sign-prose-only-source-reviews.md | 14 ++++++++++++++ docs/organisation/retired-rules.json | 8 ++++++++ docs/source-acquisition-protocol.md | 2 +- scripts/lib/signoff-kinds/sources.mjs | 12 +++++++++--- src/lib/developer-area/sign-off-queue.ts | 14 ++++++++++---- tests/signoff-sources.test.ts | 4 ++++ 7 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 docs/decisions/2026-10-17-sign-prose-only-source-reviews.md diff --git a/docs/clinical-sign-off-how-to.md b/docs/clinical-sign-off-how-to.md index 23c2e3fd0e..58bdec748c 100644 --- a/docs/clinical-sign-off-how-to.md +++ b/docs/clinical-sign-off-how-to.md @@ -272,15 +272,16 @@ The tool offers two kinds of source: - **Unverified sources** that nobody has reviewed yet. Signing one marks it `locally_reviewed`. - **Sources already marked `locally_reviewed` whose review was written down only in words**, - with no recorded signer, date or content pin (owner decision, 2026-10-17). These show as + with no recorded signer, date or content pin ([owner decision, 2026-10-17](decisions/2026-10-17-sign-prose-only-source-reviews.md)). These show as waiting in the sign-off ledger. Signing one records your name, the time and a pin of the record's current content; its status stays `locally_reviewed`. Read the record on screen again before you sign: you are vouching for it as it reads today, not as it read when it was first reviewed. -A source that already carries any part of a recorded sign-off (a signer, a date or a content -pin) is not offered and the tool refuses it by its id, unless its content has changed since it -was signed. Signing a source never adopts it: whether clinical content cites it is a separate +A source marked `locally_reviewed` that already carries any part of a recorded sign-off (a +signer, a date or a content pin) is not offered this way, and the tool refuses it by its id +unless its content has changed since it was signed. (A partial sign-off is a ledger error that +`npm run check:source-acquisitions` reports.) Signing a source never adopts it: whether clinical content cites it is a separate decision. To send a source sign-off, add `src/data/source-acquisitions.json` in step 11 below. ## Indigenous content is never signed off here diff --git a/docs/decisions/2026-10-17-sign-prose-only-source-reviews.md b/docs/decisions/2026-10-17-sign-prose-only-source-reviews.md new file mode 100644 index 0000000000..e6379faa24 --- /dev/null +++ b/docs/decisions/2026-10-17-sign-prose-only-source-reviews.md @@ -0,0 +1,14 @@ +# Decision: The owner can sign sources reviewed only in prose + +- **Status:** decided 2026-10-17 +- **Source:** `scripts/lib/signoff-kinds/sources.mjs`, find "The owner may sign it with this tool like a drafted source" +- **Retires:** `clinical-review-signs-only-unverified-sources` + +`npm run clinical:review -- --kind source` now also offers a source that is marked `locally_reviewed`, is not +rejected, and has no attestation field at all, because its review was recorded only in prose. Signing it +writes attestedBy, attestedAt and attestedAgainstSha256 and leaves validationStatus as it is. Sources with +any attestation field, approved or rejected sources, and Indigenous content are still not offered this way. +The interactive-terminal requirement and the full-name signer rule are unchanged. The owner approved this +in chat on 17 Oct 2026 so they can sign the two OCP WA standards themselves. + +If this summary and the source ever differ, the source wins. diff --git a/docs/organisation/retired-rules.json b/docs/organisation/retired-rules.json index d7802db99d..09be5cb59f 100644 --- a/docs/organisation/retired-rules.json +++ b/docs/organisation/retired-rules.json @@ -56,6 +56,14 @@ "pattern": "npm run arbiter|gate[- ]arbiter|GATE_ARBITER|consult the arbiter|arbiter'?s? verdict", "allowedContext": "no longer|removed|removal|retir(ed|ement)|deleted|there is no|formerly|previously|superseded|dropped|replaced|deprecated|gone|do not wait|no need", "record": "diff-integrity.json: scripts/gate-arbiter.mjs deleted in the owner-approved governance cut of 2026-09-17; gate receipts kept" + }, + { + "id": "clinical-review-signs-only-unverified-sources", + "retired": "2026-10-17", + "summary": "clinical:review --kind source no longer signs only unverified sources: a locally_reviewed source with no attestation field at all is offered too, so the owner can record the structured attestation it lacks.", + "pattern": "clinical:review (only signs|signs only) (unverified|drafted) sources|clinical:review only signs unverified", + "allowedContext": "no longer|retir(ed|ement)|removed|formerly|previously|superseded|dropped|replaced|historical", + "decision": "docs/decisions/2026-10-17-sign-prose-only-source-reviews.md" } ] } diff --git a/docs/source-acquisition-protocol.md b/docs/source-acquisition-protocol.md index 1b8d056be1..0ab3aeb1cf 100644 --- a/docs/source-acquisition-protocol.md +++ b/docs/source-acquisition-protocol.md @@ -122,7 +122,7 @@ behalf. The owner writes them with `npm run clinical:review -- --kind source` (o `validationStatus` to `locally_reviewed` and computes the digest. The tool also offers a source already marked `locally_reviewed` whose review lives only in prose, meaning it has no attestation field at all; signing it records the three fields and leaves `validationStatus` as -it is (owner decision, 2026-10-17). A record with any attestation field, complete or partial, +it is ([owner decision, 2026-10-17](decisions/2026-10-17-sign-prose-only-source-reviews.md)). A record with any attestation field, complete or partial, is not offered this way. Signing a source does not adopt it: `disposition` stays as it is until a separate adoption decision. Sources with Aboriginal, Torres Strait Islander or other Indigenous content are never offered (owner rule 2026-09-26). diff --git a/scripts/lib/signoff-kinds/sources.mjs b/scripts/lib/signoff-kinds/sources.mjs index 21157d2569..c349fc1d8b 100644 --- a/scripts/lib/signoff-kinds/sources.mjs +++ b/scripts/lib/signoff-kinds/sources.mjs @@ -83,6 +83,7 @@ const signedValidationStatus = (status) => (status === "unverified" ? "locally_r * (not even an empty or null one). The owner may sign it with this tool like a drafted source, * which records the structured attestation the ledger lacks (owner decision, 2026-10-17). A * record carrying any attestation field, complete or partial, is never treated this way. + * Decision entry: docs/decisions/2026-10-17-sign-prose-only-source-reviews.md */ export function isUnattestedLocalReview(native) { return ( @@ -311,9 +312,14 @@ export const display = { ...ratingRows(record, context), [ "What signing does", - "Sets validationStatus to locally_reviewed and records your name, the time and a pin of every field above. " + - "It takes the source off the owner review queue and changes its rating as shown above. It does not adopt " + - "the source: disposition stays candidate until clinical content is written to cite it.", + record.validationStatus === "locally_reviewed" + ? "Records your name, the time and a pin of every field above. validationStatus is already " + + "locally_reviewed and stays so, so its rating does not change; the review stops being prose only and " + + "counts as signed. It does not adopt the source: disposition stays as it is until clinical content is " + + "written to cite it." + : "Sets validationStatus to locally_reviewed and records your name, the time and a pin of every field above. " + + "It takes the source off the owner review queue and changes its rating as shown above. It does not adopt " + + "the source: disposition stays candidate until clinical content is written to cite it.", ], ]; }, diff --git a/src/lib/developer-area/sign-off-queue.ts b/src/lib/developer-area/sign-off-queue.ts index c27ec39486..6c9ac702bb 100644 --- a/src/lib/developer-area/sign-off-queue.ts +++ b/src/lib/developer-area/sign-off-queue.ts @@ -533,14 +533,20 @@ function isUnattestedLocalSourceReview(record: SourceAcquisitionRecord): boolean ); } -/** A reviewed source whose content changed after its structured sign-off. */ +/** + * A reviewed source with a complete structured sign-off (all three attestation fields present) + * whose content changed after it was signed. A partial attestation is never treated as stale: it + * is a ledger error (acquisitionLedgerIssues) and stays awaiting prerequisites. + */ function isStaleAttestedSourceReview(record: SourceAcquisitionRecord): boolean { + const complete = ACQUISITION_ATTESTATION_FIELDS.every((field) => { + const value = record[field]; + return typeof value === "string" && value.trim().length > 0; + }); return ( record.disposition !== "rejected" && (record.validationStatus === "locally_reviewed" || record.validationStatus === "approved") && - typeof record.attestedBy === "string" && - record.attestedBy.trim().length > 0 && - typeof record.attestedAgainstSha256 === "string" && + complete && record.attestedAgainstSha256 !== acquisitionAttestedContentSha256(record) ); } diff --git a/tests/signoff-sources.test.ts b/tests/signoff-sources.test.ts index 6e50a865b3..d651e4fcfb 100644 --- a/tests/signoff-sources.test.ts +++ b/tests/signoff-sources.test.ts @@ -385,6 +385,10 @@ describe("signing a source reviewed only in prose", () => { for (const id of PROSE_ONLY) { const view = views().find((record) => record.id === id)!; expect(view).toMatchObject({ status: "drafted", validationStatus: "locally_reviewed", reviewedBy: null }); + // The screen must not promise a rating change: validationStatus is already locally_reviewed. + const text = rowsText(display.source(view, context)); + expect(text).toContain("stays so, so its rating does not change"); + expect(text).not.toContain("Sets validationStatus to locally_reviewed"); expect(signOffEligibilityProblem(view, "source")).toBeNull(); expect(signOffQueue("source", views())).toContain(id); } From a6e0854024fb51cb579542af08d2f267f5b7e0fd Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Sat, 17 Oct 2026 19:03:55 +0800 Subject: [PATCH 5/6] chore: refresh repo-awareness snapshot for the new decision entry --- data/repo-awareness-snapshot.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/data/repo-awareness-snapshot.json b/data/repo-awareness-snapshot.json index 27260505f0..5587411a59 100644 --- a/data/repo-awareness-snapshot.json +++ b/data/repo-awareness-snapshot.json @@ -4384,6 +4384,11 @@ "section": "archive", "catalogued": true }, + { + "path": "docs/decisions/2026-10-17-sign-prose-only-source-reviews.md", + "section": "decisions", + "catalogued": true + }, { "path": "docs/brand/psychsift-logo.md", "section": "brand", From 0ae8329602ab45d8461aca0ee57cff552941e551 Mon Sep 17 00:00:00 2001 From: BigSimmo Date: Sat, 17 Oct 2026 19:14:20 +0800 Subject: [PATCH 6/6] Address review: empty attestation fields reported, accurate re-sign copy - acquisitionLedgerIssues names an attestation field present but null or empty when no sign-off is recorded (it blocks the prose-only path, so it must not pass silently); regression tests for each field - Sign-off screen: any already-reviewed source (locally_reviewed or approved) is told its status is kept and its rating does not change - How-to: only a complete sign-off whose content changed is re-offered; partial or empty ones never are --- docs/clinical-sign-off-how-to.md | 7 ++++--- scripts/lib/signoff-kinds/sources.mjs | 10 +++++----- src/lib/sources/acquisition-ledger.ts | 10 ++++++++++ tests/signoff-sources.test.ts | 9 ++++++++- tests/source-acquisition-ledger.test.ts | 10 ++++++++++ 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/docs/clinical-sign-off-how-to.md b/docs/clinical-sign-off-how-to.md index 58bdec748c..32242806b3 100644 --- a/docs/clinical-sign-off-how-to.md +++ b/docs/clinical-sign-off-how-to.md @@ -279,9 +279,10 @@ The tool offers two kinds of source: first reviewed. A source marked `locally_reviewed` that already carries any part of a recorded sign-off (a -signer, a date or a content pin) is not offered this way, and the tool refuses it by its id -unless its content has changed since it was signed. (A partial sign-off is a ledger error that -`npm run check:source-acquisitions` reports.) Signing a source never adopts it: whether clinical content cites it is a separate +signer, a date or a content pin, even an empty one) is not offered this way. The one exception is +a complete sign-off (signer, date and pin all recorded) whose content has changed since: the tool +offers that for signing again. A partial or empty sign-off is never offered, even after the +content changes; it is a ledger error that `npm run check:source-acquisitions` reports. Signing a source never adopts it: whether clinical content cites it is a separate decision. To send a source sign-off, add `src/data/source-acquisitions.json` in step 11 below. ## Indigenous content is never signed off here diff --git a/scripts/lib/signoff-kinds/sources.mjs b/scripts/lib/signoff-kinds/sources.mjs index c349fc1d8b..4259ec891b 100644 --- a/scripts/lib/signoff-kinds/sources.mjs +++ b/scripts/lib/signoff-kinds/sources.mjs @@ -312,11 +312,11 @@ export const display = { ...ratingRows(record, context), [ "What signing does", - record.validationStatus === "locally_reviewed" - ? "Records your name, the time and a pin of every field above. validationStatus is already " + - "locally_reviewed and stays so, so its rating does not change; the review stops being prose only and " + - "counts as signed. It does not adopt the source: disposition stays as it is until clinical content is " + - "written to cite it." + record.validationStatus !== "unverified" + ? `Records your name, the time and a pin of every field above. validationStatus is already ` + + `${record.validationStatus} and stays so, so its rating does not change; the source counts as signed ` + + "against its content as it reads now. It does not adopt the source: disposition stays as it is until " + + "clinical content is written to cite it." : "Sets validationStatus to locally_reviewed and records your name, the time and a pin of every field above. " + "It takes the source off the owner review queue and changes its rating as shown above. It does not adopt " + "the source: disposition stays candidate until clinical content is written to cite it.", diff --git a/src/lib/sources/acquisition-ledger.ts b/src/lib/sources/acquisition-ledger.ts index a77ef71ac1..273d1178e0 100644 --- a/src/lib/sources/acquisition-ledger.ts +++ b/src/lib/sources/acquisition-ledger.ts @@ -417,6 +417,16 @@ export function acquisitionLedgerIssues( `${id}: an attestation must record attestedBy, attestedAt and attestedAgainstSha256; missing ${missing.join(", ")}`, ); } + // An attestation field that is present but null or empty records no sign-off, yet it still + // keeps a prose-only review out of `clinical:review`, so name it rather than pass it silently. + const blank = ACQUISITION_ATTESTATION_FIELDS.filter( + (field) => Object.hasOwn(record, field) && presentText(record[field]) === null, + ); + if (attestation.length === 0 && blank.length > 0) { + issues.push( + `${id}: ${blank.join(", ")} present but empty; remove the field, or record a complete attestation with clinical:review`, + ); + } // `npm run clinical:review -- --kind source` writes the exact UTC instant of the // sign-off, the same rule every other sign-off kind holds; a bare date is still // accepted for an attestation written by hand. diff --git a/tests/signoff-sources.test.ts b/tests/signoff-sources.test.ts index d651e4fcfb..82b7768471 100644 --- a/tests/signoff-sources.test.ts +++ b/tests/signoff-sources.test.ts @@ -387,7 +387,7 @@ describe("signing a source reviewed only in prose", () => { expect(view).toMatchObject({ status: "drafted", validationStatus: "locally_reviewed", reviewedBy: null }); // The screen must not promise a rating change: validationStatus is already locally_reviewed. const text = rowsText(display.source(view, context)); - expect(text).toContain("stays so, so its rating does not change"); + expect(text).toContain("already locally_reviewed and stays so, so its rating does not change"); expect(text).not.toContain("Sets validationStatus to locally_reviewed"); expect(signOffEligibilityProblem(view, "source")).toBeNull(); expect(signOffQueue("source", views())).toContain(id); @@ -440,6 +440,13 @@ describe("signing a source reviewed only in prose", () => { expect(signOffEligibilityProblem(viewOf(record), "source")).not.toBeNull(); }); + it("tells the owner an approved source keeps its status when a stale sign-off is redone", () => { + const record = { ...attestedNative(), validationStatus: "approved" }; + const text = rowsText(display.source(viewOf(record), context)); + expect(text).toContain("already approved and stays so"); + expect(text).not.toContain("Sets validationStatus to locally_reviewed"); + }); + it.each([ ["approved", { validationStatus: "approved" }], ["rejected", { disposition: "rejected" }], diff --git a/tests/source-acquisition-ledger.test.ts b/tests/source-acquisition-ledger.test.ts index e1081e5882..8715e139d5 100644 --- a/tests/source-acquisition-ledger.test.ts +++ b/tests/source-acquisition-ledger.test.ts @@ -497,6 +497,16 @@ describe("source acquisition attestation", () => { ]); }); + it.each([ + ["attestedBy", null], + ["attestedAt", ""], + ["attestedAgainstSha256", " "], + ])("reports an attestation field %s that is present but empty, with no sign-off recorded", (field, value) => { + expect(issuesFor({ [field]: value })).toEqual([ + `ocp-wa-test-guideline: ${field} present but empty; remove the field, or record a complete attestation with clinical:review`, + ]); + }); + it("rejects a malformed digest rather than calling it stale", () => { expect(issuesFor({ attestedBy: "Owner", attestedAt: "2026-09-18", attestedAgainstSha256: "not-a-digest" })).toEqual( ["ocp-wa-test-guideline: attestedAgainstSha256 must be a lower-case 64-character SHA-256 digest"],