From b65c9fa0f473125137fa594d630cec5afcd75c87 Mon Sep 17 00:00:00 2001 From: Taleef Date: Mon, 17 Aug 2026 11:35:43 -0400 Subject: [PATCH 1/7] feat(cds): serve a CDS Hooks 2.0.1 service over persisted, finalized outcomes Adds GET /cds-services (public), POST /cds-services/{id} and its /feedback sibling (both bearer-gated), so an open measure gap can reach a clinician through the community standard rather than a bespoke endpoint. Cards render a COMPLETED run's outcome; they never trigger an evaluation. No prefetch is declared, because none is evaluated -- usageRequirements says so in the machine-readable contract. Three refusals are load-bearing and mutation-checked: critical is unrepresentable in CdsCard, since WorkWell is supplementary to WebChart and may not tell a clinician not to proceed; a suggestion is offered only where the order code has an APPROVED terminology mapping read from the store, so cms122/cms125 carry information and a link rather than a demo-grade CPT for one-click creation; and a patient with no finalized outcome gets an informational card, never an empty card list, which at the point of care would read as "no gaps". Card uuids derive from (runId, subjectId, measureId), so feedback correlates by recomputation and the endpoint needs no schema change. The measure-outcome to card mapping is ours: HL7 blesses PlanDefinition/$apply -> RequestOrchestration -> cards, and the DEQM care-gap to card leg is unpublished. /cds-services is outside /api/, where authorize ends in permitAll, so the two new rules are mandatory rather than a refinement -- asserted both as a unit call and end-to-end through the worker, and both fail when either is removed. Suite 1964, 0 fail. Co-Authored-By: Claude Opus 5 (1M context) --- backend-ts/src/auth/authorize.ts | 16 ++ backend-ts/src/cds/cards.test.ts | 186 ++++++++++++++ backend-ts/src/cds/cards.ts | Bin 0 -> 11394 bytes backend-ts/src/cds/discovery.ts | 47 ++++ backend-ts/src/cds/types.ts | 125 +++++++++ backend-ts/src/routes/cds-hooks.test.ts | 284 +++++++++++++++++++++ backend-ts/src/routes/cds-hooks.ts | 322 ++++++++++++++++++++++++ backend-ts/src/worker.test.ts | 22 ++ backend-ts/src/worker.ts | 7 + 9 files changed, 1009 insertions(+) create mode 100644 backend-ts/src/cds/cards.test.ts create mode 100644 backend-ts/src/cds/cards.ts create mode 100644 backend-ts/src/cds/discovery.ts create mode 100644 backend-ts/src/cds/types.ts create mode 100644 backend-ts/src/routes/cds-hooks.test.ts create mode 100644 backend-ts/src/routes/cds-hooks.ts diff --git a/backend-ts/src/auth/authorize.ts b/backend-ts/src/auth/authorize.ts index e796ddb3..7b94f093 100644 --- a/backend-ts/src/auth/authorize.ts +++ b/backend-ts/src/auth/authorize.ts @@ -63,6 +63,22 @@ const RULES: Rule[] = [ { pattern: rx("/sse"), access: [A, CM, MCP] }, { pattern: rx("/mcp/**"), access: [A, CM, MCP] }, + // CDS Hooks (ADR-067). These rules are MANDATORY, not a refinement: `/cds-services` matches no `/api/**` + // rule, and `authorize` ends with `return { ok: true }` for non-`/api` paths (permitAll, mirroring + // Spring's anyRequest().permitAll()) — so without them the invoke endpoint would serve per-patient + // clinical status to anonymous callers. + // + // Discovery is PERMIT: it returns service metadata and no patient data, and a CDS client must be able to + // discover the service during onboarding (the spec imposes no auth on it). Invoke and feedback are + // machine-client work, so they reuse the SAME authority as /sse and /mcp/** rather than inventing a role + // — the user directory stays hardcoded (CLAUDE.md hard rule). + // + // Order is load-bearing: `rx("/cds-services/**")` expands to `^/cds-services(?:/.*)?$`, which ALSO + // matches the bare path, so the GET-exact PERMIT must come first. A non-GET on `/cds-services` therefore + // falls to the gated rule and is authenticated before the handler answers 405. + { method: "GET", pattern: rx("/cds-services"), access: "PERMIT" }, + { pattern: rx("/cds-services/**"), access: [A, CM, MCP] }, + // Outreach templates: the picker on the case-detail outreach action is the CASE_MANAGER's // primary consumer, so READING the template list/preview is CM/ADMIN (Fable M23). Writes // (create/update/delete) stay ADMIN via the /api/admin/** rule below. First-match-wins, so diff --git a/backend-ts/src/cds/cards.test.ts b/backend-ts/src/cds/cards.test.ts new file mode 100644 index 00000000..869103c9 --- /dev/null +++ b/backend-ts/src/cds/cards.test.ts @@ -0,0 +1,186 @@ +/** + * CDS Hooks card assembly (ADR-067). + * node --import tsx --test src/cds/cards.test.ts + * + * The load-bearing tests here are the three REFUSALS, because each one is a place where a plausible + * implementation would have been wrong in a way no client could detect: `critical` is never emitted, a + * suggestion is never offered for an unapproved order code, and an absence of data never renders as + * silence. Everything else is shape. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { buildComplianceCards, cardUuid, noEvaluationCard, suggestionUuid, type CardInput } from "./cards.ts"; +import { ORDER_CATALOG } from "../order/order-catalog.ts"; +import type { StandingOrderProvider } from "../order/standing-order-provider.ts"; + +/** No standing orders, so suppression never confounds a suggestion assertion. */ +const NO_STANDING_ORDERS: StandingOrderProvider = { activeOrdersFor: () => [] }; + +const CPT = "http://www.ama-assn.org/go/cpt"; +/** The three APPROVED mappings in `value-set-seed.ts`, as the route would compute them from the store. */ +const APPROVED = new Set([`${CPT}|92557`, `${CPT}|86580`, "http://hl7.org/fhir/sid/cvx|141"]); + +const RUN = "11111111-2222-3333-4444-555555555555"; + +function row(measureId: string, status: string, extra: Partial = {}): CardInput { + return { + measureId, + status, + evidence: { + expressionResults: [ + { define: "Most Recent Audiogram Date", result: "2025-03-10T00:00:00Z" }, + { define: "Days Since Last Audiogram", result: 420 }, + ], + }, + evaluationPeriod: "2026-06-12", + runId: RUN, + evaluatedAt: "2026-06-12T03:00:11.482Z", + ...extra, + }; +} + +const opts = (approved: ReadonlySet = APPROVED) => ({ + subjectId: "emp-006", + approvedOrderCodes: approved, + standingOrders: NO_STANDING_ORDERS, + studioBaseUrl: "https://studio.example.org", + authoredOn: "2026-06-12", +}); + +test("an open gap becomes one card carrying the answer, its provenance and a link", async () => { + const cards = await buildComplianceCards([row("audiogram", "OVERDUE")], opts()); + assert.equal(cards.length, 1); + const card = cards[0]!; + assert.match(card.summary, /Annual Audiogram Completed/); + assert.match(card.summary, /overdue/); + assert.equal(card.source.label, "WorkWell Measure Studio"); + // The card must say the answer came from CQL and name the run — an unattributed clinical claim in a + // chart is the thing docs/AI_GUARDRAILS.md exists to prevent. + assert.match(card.detail!, /Computed by CQL/); + assert.match(card.detail!, new RegExp(RUN)); + assert.deepEqual(card.links, [ + { label: "Open in WorkWell", url: "https://studio.example.org/compliance?subjectId=emp-006", type: "absolute" }, + ]); +}); + +test("`critical` is never emitted, for any open status", async () => { + // In CDS Hooks `critical` means the user must not proceed. WorkWell is SUPPLEMENTARY to WebChart + // (locked decision 1) and is not entitled to say that about someone else's encounter. `CdsCard` makes + // it a type error; this asserts the runtime consequence over every status that produces a card. + for (const status of ["OVERDUE", "DUE_SOON", "MISSING_DATA"]) { + const cards = await buildComplianceCards([row("audiogram", status)], opts()); + assert.equal(cards.length, 1, `${status} must produce a card`); + assert.ok( + cards[0]!.indicator === "info" || cards[0]!.indicator === "warning", + `${status} produced indicator ${cards[0]!.indicator}`, + ); + } + // OVERDUE is the most urgent thing we say, and it is `warning` — the ceiling. + const overdue = await buildComplianceCards([row("audiogram", "OVERDUE")], opts()); + assert.equal(overdue[0]!.indicator, "warning"); + const dueSoon = await buildComplianceCards([row("audiogram", "DUE_SOON")], opts()); + assert.equal(dueSoon[0]!.indicator, "info"); +}); + +test("a suggestion is offered ONLY for an APPROVED order code", async () => { + const withApproval = await buildComplianceCards([row("audiogram", "OVERDUE")], opts()); + const s = withApproval[0]!.suggestions; + assert.equal(s?.length, 1); + assert.equal(withApproval[0]!.selectionBehavior, "at-most-one"); + const action = s![0]!.actions[0]!; + assert.equal(action.type, "create"); + const resource = action.resource as { resourceType: string; intent: string; status: string }; + assert.equal(resource.resourceType, "ServiceRequest"); + // Advisory by construction — a human submits. Never an active order. + assert.equal(resource.intent, "proposal"); + assert.equal(resource.status, "draft"); + + // The same row with nothing approved carries no suggestion — and no dangling selectionBehavior, which + // the spec only permits alongside suggestions. + const withoutApproval = await buildComplianceCards([row("audiogram", "OVERDUE")], opts(new Set())); + assert.equal(withoutApproval[0]!.suggestions, undefined); + assert.equal(withoutApproval[0]!.selectionBehavior, undefined); +}); + +test("cms122 and cms125 get NO suggestion under the real approved set — the documented consequence", async () => { + // Their CPT codes are in ORDER_CATALOG but have no terminology mapping at all, so `order-catalog.ts`'s + // own "representative (demo, not billing-certified)" caveat applies and they must not be offered for + // one-click creation in a certified EHR. If a mapping is later APPROVED this test SHOULD fail — it is + // pinning the rule's consequence, so the rule changing is exactly when someone should look. + assert.equal(ORDER_CATALOG["cms125"]?.code, "77067", "guard: the catalog still maps cms125"); + assert.equal(ORDER_CATALOG["cms122"]?.code, "83036", "guard: the catalog still maps cms122"); + for (const measureId of ["cms122", "cms125"]) { + const cards = await buildComplianceCards([row(measureId, "OVERDUE")], opts()); + assert.equal(cards.length, 1); + assert.equal(cards[0]!.suggestions, undefined, `${measureId} must carry no suggestion`); + assert.ok(cards[0]!.links!.length > 0, `${measureId} must still carry a link`); + } +}); + +test("two measures sharing one order code collapse to ONE suggestion", async () => { + // diabetes_hba1c and cms122 both map to CPT 83036. One order is the correct clinical action, so + // `proposeOrders` collapses them; the measure that did not win still gets an information card. + const approved = new Set([`${CPT}|83036`]); + const cards = await buildComplianceCards( + [row("diabetes_hba1c", "OVERDUE"), row("cms122", "OVERDUE")], + opts(approved), + ); + assert.equal(cards.length, 2); + const withSuggestions = cards.filter((c) => c.suggestions); + assert.equal(withSuggestions.length, 1, "exactly one card may offer the shared order"); +}); + +test("compliant, excluded and deprecated measures produce no cards", async () => { + for (const status of ["COMPLIANT", "EXCLUDED"]) { + assert.deepEqual(await buildComplianceCards([row("audiogram", status)], opts()), [], status); + } + // A Deprecated catalog measure is not something to raise mid-encounter. + assert.deepEqual(await buildComplianceCards([row("lead_medical_surveillance", "OVERDUE")], opts()), []); +}); + +test("summary respects the spec's 140-character cap even for a long measure name", async () => { + // cms177v14's catalog name is ~90 chars; the longest are longer still. The cap is a conformance + // requirement, so it is enforced rather than hoped for. + const cards = await buildComplianceCards([row("cms135v14", "OVERDUE")], opts()); + assert.equal(cards.length, 1); + assert.ok(cards[0]!.summary.length <= 140, `summary was ${cards[0]!.summary.length} chars`); + assert.match(cards[0]!.summary, /…$/, "a truncated summary must show that it was truncated"); +}); + +test("no studio base URL means no links, never a broken one", async () => { + const cards = await buildComplianceCards([row("audiogram", "OVERDUE")], { + ...opts(), + studioBaseUrl: undefined, + }); + assert.equal(cards[0]!.links, undefined); + assert.equal(cards[0]!.source.url, undefined); +}); + +test("card and suggestion uuids are deterministic, UUID-shaped, and distinct per measure", async () => { + // Determinism is what lets the feedback endpoint exist with no schema change: the id is recomputable + // from (runId, subjectId, measureId) rather than stored. + const a = await cardUuid(RUN, "emp-006", "audiogram"); + const again = await cardUuid(RUN, "emp-006", "audiogram"); + assert.equal(a, again, "the same card must always get the same uuid"); + assert.match(a, /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/); + assert.notEqual(a, await cardUuid(RUN, "emp-006", "cms125"), "different measure → different uuid"); + assert.notEqual(a, await cardUuid(RUN, "emp-007", "audiogram"), "different subject → different uuid"); + assert.notEqual(a, await cardUuid("other-run", "emp-006", "audiogram"), "different run → different uuid"); + // A card uuid and its suggestion uuid must not collide — feedback distinguishes them. + assert.notEqual(a, await suggestionUuid(RUN, "emp-006", "audiogram")); + + const cards = await buildComplianceCards([row("audiogram", "OVERDUE")], opts()); + assert.equal(cards[0]!.uuid, a, "the emitted card must carry the derived uuid"); + assert.equal(cards[0]!.suggestions![0]!.uuid, await suggestionUuid(RUN, "emp-006", "audiogram")); +}); + +test("an absence of data is a CARD, and it does not claim compliance", async () => { + const card = noEvaluationCard("wc|4821"); + assert.equal(card.indicator, "info"); + assert.match(card.summary, /No WorkWell evaluation on record/); + assert.match(card.summary, /wc\|4821/, "the id must be echoed so a namespace mismatch is visible"); + // The whole point: it must not be readable as "this patient is fine". + assert.match(card.detail!, /absence of a run/); + assert.doesNotMatch(card.detail!, /\bcompliant\b(?!\.)/i); + assert.equal(card.suggestions, undefined); +}); diff --git a/backend-ts/src/cds/cards.ts b/backend-ts/src/cds/cards.ts new file mode 100644 index 0000000000000000000000000000000000000000..46a1317373c60b18c5d6d55a0e700bb007ce3e84 GIT binary patch literal 11394 zcmcgy?Q+{jmfhceiVnv$Luy6Ha%{&dM|RPa9IIAe5v6gaG%f)Inj&Hl;Nn9xM`6|c zng`h0XV?eJlk7RSy8)1rcWd{@CaKs28+~uzk8{tx&7G~SCu&O#4@T-^mVGHy803)( zibBuE$x?L(2N(TkuYTyM-~av}YF_1fTcw#g+BuUy58lfkI=%C-3pe`1AJpYkt64Cg z$LXz#3w3sNG1^u^8mV#`lzdW@L8&8^2jx`fe446wHcw(5*-btk|L~_mjS~$uur8}g zttNRkQ!^bD&_N~HZ5*mH6KxCb>7OS-dY~t98kcdF?mUG8$r0&EwkMUEYyUI6jX{Tustw$+D^M;B_CKyVl&>H->ODUROr z)WPA;Cx0p^!+#1Xx_=wY3m?bN=8#ZAh<~6<9l{t#MNw&AthBWS1JA2*5*JgP-dPeG zaoNt|d}B~Zs_!@+UAi5Ss`J;Q9K3s2w0kxoN> z^DA`rYkPX?Nc=RHROo#AY92P1GmM~R!@bThjIBxj!u^Q?8 zE)I3EjZYucMV08hC&#POhuiWra1=eE5`FtdOcrHfu`{|HoE{7=4n{Y_v(pb}7bk<$ z;h{H+V8virDE&1EON1#Ss}+H#8LWWlLwIQw=@6%cEz+et#pc}q@J{DT7)mD*EfwiJ zzQZ}iLI}tp$#oDd`v`#|E@%Os%34m1DN?J^;N(zo9Tv%hJi}7Nj0C%erwTDyq{|>q zR8?q6Bi{%c>Ll^iBx4Fd&A6Duj3zl`hqx>h9>r<@Fui3oApFZ|7HuoL!Kdl+LlWHH zYM8@r!;?kGF4*0l=W&+DBTGLRouipb8yk zI0#cy&kzNYHY%IYlK6w7pbpLi^T`SH1hK5b1n8jzH@@G6hhRN?kC4vw9d_APMaJdu z4Bkt#REwMFy`3kJmF1Cshb#Ztz z99#~L&wgsZXyI+g{@D+MGQbua8_e?zO6jv)!meeWj8Ast|9!J4cyJ&cRdfBXnnAG* zuh=xT0Es8dxmM>!KqL~{`WXRMe$RzcWbljH=l*Y=xNlH8hle-EgZGEW_^R_M%fEbLN;u<=$|{O8 zguQ5ehEziR3zGP6fGZ1CjM)VaED29Uv6_kk`m3ZH>WI!KL8ukYbd=63q~H_fmL5fW zK&2cm0Wb?bRYm)Wt0H@{7|KOmmwkQAs#mF{7FQ|_nR4`4Bv!@6v@X?@3O!+IbC|Fua&7zhV#Jd3m zo1v$Kl6}>+ADgi3In~tuTUchJo_K(OsN0?O@U5D8aisS5_nTDiz4HX4-o0y{)qoQh z2b%0m==nE9kFzY%2<`uyY(#EZ6{1q7kwpZXRH>mKCE$S?bZd~=Ml^oiQ)x9DgGJV- zzsKQoT~>Lj3J*YgTTa!FIBM`!y;FtS1M9voydnV&?LLz|RA24(R=@xKKm9jPU~b0L z2%$m>j9LpO6Kv2hz%_%Mywnx}?0RxE8XcYfbaQ|LZX1kQTolY3P_MwiNqwAY1;9sG z^FaK+Q~&XEaD`W9$Js*XLul0PdASAVFGc5b&-M|FCQ@*fBN=PzHmM3@J83F4RQ z6u~HP}RGy`!D3*-RJC0(0CAzz|P`OvLEmuq!}<%lU|9}pjf7%st*Lt z)Vz6B#ZlMu0HzS_j&}VShePFtRkTZXfol`r<0behA{&KCT>^;@Q=&+YqrwsRTUR#o ze$C>v+xe$X59d{^ohV{a%;Ig|y+Yl3Js^5@gGCU7dFIP`nR&qRGSQw{+wF{uk1sy2j!!=A~I}=obaS(n{h@CkMUm}Xg4GPC2Pyueyiop0j)4{9Yc;8eFlo9Fm z0LCCliy*~Y^?t&I#?l1^Xz1Ap*m{h_;wDPO;_*VJ1KfbsMsjZO{_N_Kv5MY|=>;E~ z4z?>40^(iCMtU25y+2-Ld6Ajctu}xv60!1Cwq`)X)7Els<;9I@5Wo8`axqS#}c#Aa(tVrqu=sZ|j zcX&LwI$$5$GkuRGIwr_@bmkSoe~l`&&g)XGb#C|Qk=zrkeweo%NZuX?ChQ8Ok-R_`6f;wb9lu{_ zb%4s|8V8OrC~6|JIyWY$M?!-HGY8UN|HbZeR3;g0NLQ~uU@(LE$)148$>4(h1eYcm;BIzv`)#+15;P_C&I3waF&Ukx`srafH`8SeT&=U`-K# zx5trsh9q?Xgb?JMHDrh)ztE}W+@_=c*~!xAgwRebNbN6KD)1k(Sb7k=&S@F(Rg#tA zgFcEJkMlGWa$N1~ikF?}(i5AlgzMGeU{3lW%e#5DfP)bc(yp@%a5U(GVx3%?va*<< zpnzu~xtp;V_o>Igs08>_7gF=Fv6*S)%~O*oEoR1{AxYh}zqR%B-e|KTNcN%o1rl)ZWs?yrPX&JrWN4mkAGBnZSAYK>e;%a1GeGzMJuQMf|b(NZCFn*)dERE zCyIK`?U==z=Q!80SOHR&#hR`lx={4S0@ZhD?EoyP?yM{JNmkjt*IuzYiMx!wjC#U# zC&G38K6IL3p(V9{O)Up+z2x8UB6wrHm>CR($2TWulBlu^D}lRMsNodndM0)<0uH-NY66ECt1({F8vUI5#x>_>UB6# z=M_^!Xm}W@5%6C>M4S4>j*;BGj(eFUTcC*p6sy2OChzWY2uIIQzKOU(Zjd|`ral<$ z%ScBq*3cMZ5J^ayusa0!b0Wcs1pxOp4`x1i>G`Glm*+2E{?NnyK{>v`W!s&`Svaz^ zU%!6!@-@tz`&FY8K_~ui5WsaYR?Ax1t^thym>ufr}w{M zf*0{xR7Wh9E*{P}uNpFxA05qcDVsA_Pqv>JI??2wYHch8gIM~V)1pV3#KBB0yg~La zdI>c5!68IZTv152d2YJ;;0v0CZLU%x2RY;L zi6iRP$g@V^j_jcgciX%(>bfIX>Zcql`%J?b{$d-m_f^c^01dFTX8#%-_<L55J=&uZ@HKM{D*YW&)l^<+B-@(4Aj)HSR9c$2gfoU`C+EjUgVRd^ zKT7cre;FR5<;Uz8_KGS{L)#C$K|mZ1kB?6IouKN`=?(4-e!4gujqn7w{=h%<48GxR z=GNHV76VI>+GLX;B61~bN^xo2Uq2x&o(lk!w^(rXETY)%@-hxXn^pyb>R2W^LP{rPQcJ}5DYwhrC2@ zUAFo#B1#V`2&)wOXNzO%5Kz0Y(z0 zU))dPR&r2E9|%Q^HVMPho9Q^%4JA5EQB`9!lk(dZ*2&l3Kl}bw5AUE*&k@7e4q=Pk zp`FW4CL_+|W(7<_Bgr-_=<(3#@%G;Ski`HVyBd^3dzou836leMGT^DAbuAVJoxoxQ zwd*V+PZG-tX_y$I2EROkw!i}XoW%UNM{I272%syA9QUh;OB|2W$9sDsMDhw{WX(N{ zMkSD9Mz9h!6ceS!hAJwaqto=cox&pi`7Sc+d6d~VOI@$-s!hSQrJTGwe^ z2&>IRlQZwk&G+?ZEI0iVW4CKei3B6}U@G)%IWu2`ykmB~BJt)y)8?*xH(I#IJ4P{r z^PgNk)%Q`4R5R9jw^-)G`^rxDaU{+hMHy)vwjX$tO-JofV-L5NiS~UH^`KEFCfYV? zXD(1a+C?L%*CjcMoQ6!6cm`bp9G30X)05~dKkpJ8 zsBc=|NQR;KqJWtR7Jfx01Ri5dCWSkli!Qj0Y7+!)pw#3D!xXnlV$egr3TuMeYc$R9 zEVq_>VN1Y^2|O=|B6+QCs)nFKf#!o{t5g%7F(gk-hW)dT7S$A>%TJs+2;Wguh$;7c{~^KAKM~y&d5#SEjPDl zQG2rx{Q+l_-!Mco(s@_coBgNv=8p5~;)qlVWjHN2&;)ZZcS0wo?lCY{lwUw}NC*Y2 zH<#8%HSY=sbK9)yqp$fv#;!Hw$jr6LHd?It^cjUwCn%km4Zqju%<=m-gjrDH*K s.id === id); +} diff --git a/backend-ts/src/cds/types.ts b/backend-ts/src/cds/types.ts new file mode 100644 index 00000000..13361e53 --- /dev/null +++ b/backend-ts/src/cds/types.ts @@ -0,0 +1,125 @@ +/** + * CDS Hooks 2.0.1 wire types (ADR-067) — the shapes WorkWell PRODUCES, and nothing else. + * + * Normative reference: https://cds-hooks.hl7.org/2.0/ (HL7 balloted STU2, package + * `hl7.fhir.uv.cds-hooks#2.0.1`). NOT `cds-hooks.org`, which serves a draft CI build. + * + * ## Why these are hand-written rather than imported + * + * There is no canonical npm types package for CDS Hooks — everything on npm named `cds-*-types` is SAP + * Core Data Services, an unrelated name collision. The best existing TS model is Medplum's + * `packages/core/src/cds.ts` (Apache-2.0), and these interfaces follow its shape; depending on + * `@medplum/core` for ~150 lines of interfaces would be a new dependency for nothing (CLAUDE.md hard rule). + * + * ## Deliberately absent, because we never emit them + * + * - `systemActions` — actions a client auto-applies with no user interaction. WorkWell is SUPPLEMENTARY to + * WebChart (locked decision 1) and "AI never decides compliance" has a sibling here: nothing WorkWell + * returns should change a chart without a human choosing it. See docs/AI_GUARDRAILS.md. + * - `overrideReasons` — offering a coded dismissal vocabulary we do not analyse would be decoration. + * - `update`/`delete` actions, `smart`/`questionnaire` links, `appContext`, `source.topic` — unused. + * + * Adding any of them is a deliberate change, not a fill-in-the-blank. + */ + +/** One service in the discovery response. `prefetch` is deliberately never declared — see `discovery.ts`. */ +export interface CdsService { + /** The hook this service is invoked on, e.g. `patient-view`. */ + hook: string; + /** The `{id}` in `POST /cds-services/{id}`. */ + id: string; + title: string; + description: string; + /** Spec-defined field for stating what a caller must know. We use it to state what we do NOT do. */ + usageRequirements: string; +} + +export interface CdsDiscoveryResponse { + services: CdsService[]; +} + +/** The hook context WorkWell reads. `patient-view` also defines `encounterId` (OPTIONAL); we ignore it. */ +export interface CdsRequestContext { + userId?: string; + patientId?: string; + encounterId?: string; +} + +/** + * An invocation. `fhirServer`, `fhirAuthorization` and `prefetch` are accepted and NOT evaluated — the + * service declares that in `usageRequirements`, which is the honest place for it. They are typed as + * `unknown` precisely so no code can start depending on them without changing this file. + */ +export interface CdsRequest { + hook?: string; + hookInstance?: string; + fhirServer?: unknown; + fhirAuthorization?: unknown; + prefetch?: unknown; + context?: CdsRequestContext; +} + +export interface CdsSource { + label: string; + url?: string; +} + +export interface CdsCreateAction { + type: "create"; + description: string; + /** A FHIR resource. `unknown` because `toServiceRequest` returns `unknown` (no FHIR runtime dep). */ + resource: unknown; +} + +export interface CdsSuggestion { + label: string; + uuid: string; + actions: CdsCreateAction[]; +} + +export interface CdsLink { + label: string; + url: string; + type: "absolute"; +} + +/** + * A card. `indicator` is `info | warning | critical` in the spec; **`critical` is deliberately + * unrepresentable here** — in CDS Hooks it means the user must not proceed, which WorkWell is not + * entitled to say about a WebChart encounter (locked decision 1). Making it a type error rather than a + * convention means the refusal cannot be forgotten. + */ +export interface CdsCard { + /** REQUIRED, and the spec caps it at 140 characters. */ + summary: string; + indicator: "info" | "warning"; + source: CdsSource; + detail?: string; + /** Needed for the feedback endpoint to identify this card. Deterministic — see `cardUuid`. */ + uuid?: string; + links?: CdsLink[]; + suggestions?: CdsSuggestion[]; + /** REQUIRED by the spec whenever `suggestions` is present. */ + selectionBehavior?: "at-most-one" | "any"; +} + +export interface CdsResponse { + cards: CdsCard[]; +} + +/** `POST /cds-services/{id}/feedback`. `outcome` is `accepted | overridden` — there is no `declined`. */ +export interface CdsFeedbackEntry { + card?: string; + outcome?: string; + /** REQUIRED by the spec when `outcome` is `accepted`. */ + acceptedSuggestions?: Array<{ id?: string }>; + overrideReason?: { + reason?: { code?: string; system?: string; display?: string }; + userComment?: string; + }; + outcomeTimestamp?: string; +} + +export interface CdsFeedbackRequest { + feedback?: CdsFeedbackEntry[]; +} diff --git a/backend-ts/src/routes/cds-hooks.test.ts b/backend-ts/src/routes/cds-hooks.test.ts new file mode 100644 index 00000000..4a1ee670 --- /dev/null +++ b/backend-ts/src/routes/cds-hooks.test.ts @@ -0,0 +1,284 @@ +/** + * CDS Hooks route (ADR-067). + * node --import tsx --test src/routes/cds-hooks.test.ts + * + * Two assertions here matter more than the rest. **The auth matrix**: `/cds-services` is outside `/api/`, + * where `authorize` ends in permitAll, so without an explicit rule the invoke endpoint would serve + * per-patient clinical status anonymously — this test is the guard on that, and it is asserted as a pure + * `authorize` call so it cannot be confused with handler behaviour. **The absence cases**: a patient we + * have not evaluated, and a patient whose only rows belong to an unfinished run, must both produce a card + * that says so rather than an empty list a clinician would read as "no gaps". + */ +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { rmSync } from "node:fs"; +// @ts-expect-error — @mieweb/cloud-local ships .mjs without types +import { createSqliteD1 } from "@mieweb/cloud-local"; +import { RUN_STORE_FLOOR_DDL } from "../stores/sqlite/schema.ts"; +import { SqliteRunStore } from "../stores/sqlite/run-store-sqlite.ts"; +import { SqliteOutcomeStore } from "../stores/sqlite/outcome-store-sqlite.ts"; +import { SqliteValueSetStore } from "../stores/sqlite/value-set-store-sqlite.ts"; +import { authorize } from "../auth/authorize.ts"; +import { getStores } from "../stores/factory.ts"; +import { cardUuid } from "../cds/cards.ts"; +import { PATIENT_VIEW_SERVICE_ID } from "../cds/discovery.ts"; +import { candidateSubjectIds, handleCdsHooks, parseCdsPath } from "./cds-hooks.ts"; +import type { JwtPrincipal } from "../auth/jwt.ts"; + +const dbPath = join(tmpdir(), `ww-cds-hooks-${crypto.randomUUID()}.sqlite`); +let env: Record; +let completedRunId: string; + +const INVOKE = `/cds-services/${PATIENT_VIEW_SERVICE_ID}`; +const OVERDUE_EVIDENCE = { + expressionResults: [ + { define: "Most Recent Audiogram Date", result: "2025-03-10T00:00:00Z" }, + { define: "Days Since Last Audiogram", result: 420 }, + ], +}; + +const call = (path: string, init?: RequestInit) => + handleCdsHooks(new Request(`http://x${path}`, init), env as never, "tester@workwell.dev"); + +const post = (path: string, body: unknown) => + call(path, { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body) }); + +const hookBody = (patientId: string, over: Record = {}) => ({ + hook: "patient-view", + hookInstance: crypto.randomUUID(), + context: { userId: "Practitioner/abc", patientId }, + ...over, +}); + +before(async () => { + const db = await createSqliteD1(dbPath); + await db.exec(RUN_STORE_FLOOR_DDL.replace(/\n/g, " ")); + env = { DB: db, WORKWELL_CORS_ALLOWED_ORIGINS: "https://studio.example.org" }; + const runs = new SqliteRunStore(db); + const outcomes = new SqliteOutcomeStore(db); + const valueSets = new SqliteValueSetStore(db); + + // The APPROVED mapping that unlocks the audiogram suggestion, and a REVIEWED one that must not. + await valueSets.createTerminologyMapping({ + id: crypto.randomUUID(), + localCode: "LOCAL-AUD-002", localDisplay: "Annual audiogram", localSystem: "urn:workwell:demo", + standardCode: "92557", standardDisplay: "Comprehensive audiometry evaluation", + standardSystem: "http://www.ama-assn.org/go/cpt", mappingStatus: "APPROVED", mappingConfidence: 0.98, notes: null, + }); + await valueSets.createTerminologyMapping({ + id: crypto.randomUUID(), + localCode: "LOCAL-HAZ-001", localDisplay: "HAZWOPER exam", localSystem: "urn:workwell:demo", + standardCode: "hazwoper-exam", standardDisplay: "HAZWOPER Surveillance Exams", + standardSystem: "urn:workwell:vs:hazwoper-exams", mappingStatus: "REVIEWED", mappingConfidence: 0.8, notes: null, + }); + + const completed = await runs.createRun({ + scopeType: "ALL_PROGRAMS", triggeredBy: "test", requestedScope: {}, + measurementPeriodStart: "2025-06-12T00:00:00.000Z", measurementPeriodEnd: "2026-06-12T00:00:00.000Z", + }); + await runs.finalizeRun(completed.id, "COMPLETED"); + completedRunId = completed.id; + + // emp-006: one open gap (suggestible) + one that must not be suggested + one compliant (no card). + await outcomes.recordOutcome({ runId: completed.id, subjectId: "emp-006", measureId: "audiogram", status: "OVERDUE", evaluationPeriod: "2026-06-12", evidence: OVERDUE_EVIDENCE }); + await outcomes.recordOutcome({ runId: completed.id, subjectId: "emp-006", measureId: "hazwoper", status: "OVERDUE", evaluationPeriod: "2026-06-12", evidence: OVERDUE_EVIDENCE }); + await outcomes.recordOutcome({ runId: completed.id, subjectId: "emp-006", measureId: "flu_vaccine", status: "COMPLIANT", evaluationPeriod: "2026-06-12", evidence: {} }); + // emp-007: evaluated and clean — the ONLY subject that legitimately gets an empty card list. + await outcomes.recordOutcome({ runId: completed.id, subjectId: "emp-007", measureId: "audiogram", status: "COMPLIANT", evaluationPeriod: "2026-06-12", evidence: {} }); + // wc|4821: the live-namespace subject, reachable only if a bare hook patientId is tried as `wc|`. + await outcomes.recordOutcome({ runId: completed.id, subjectId: "wc|4821", measureId: "audiogram", status: "OVERDUE", evaluationPeriod: "2026-06-12", evidence: OVERDUE_EVIDENCE }); + + // emp-mid: rows exist but the run never finalized. Serving these would publish a partial result. + const running = await runs.createRun({ + scopeType: "ALL_PROGRAMS", triggeredBy: "test", requestedScope: {}, + measurementPeriodStart: "2025-06-12T00:00:00.000Z", measurementPeriodEnd: "2026-06-12T00:00:00.000Z", + }); + await outcomes.recordOutcome({ runId: running.id, subjectId: "emp-mid", measureId: "audiogram", status: "OVERDUE", evaluationPeriod: "2026-06-12", evidence: OVERDUE_EVIDENCE }); +}); + +after(() => { try { rmSync(dbPath, { force: true }); } catch { /* best effort */ } }); + +test("the path matcher claims only its own shapes", () => { + assert.deepEqual(parseCdsPath("/cds-services"), { kind: "discovery" }); + assert.deepEqual(parseCdsPath("/cds-services/abc"), { kind: "invoke", serviceId: "abc" }); + assert.deepEqual(parseCdsPath("/cds-services/abc/feedback"), { kind: "feedback", serviceId: "abc" }); + for (const p of ["/api/cds-services", "/cds-services/a/b", "/cds-services/a/feedback/x", "/cds", "/cds-servicesx"]) { + assert.equal(parseCdsPath(p), null, `${p} must not be claimed`); + } + // A bad percent-escape must not reach the worker's catch-all as a 500 (the #399 lesson). + assert.equal(parseCdsPath("/cds-services/%E0%A4%A"), null); +}); + +test("a non-matching path returns null so the worker keeps dispatching", async () => { + assert.equal(await call("/api/v1/compliance/emp-006/audiogram"), null); +}); + +test("discovery is one service, declares NO prefetch, and states what it does not do", async () => { + const res = (await call("/cds-services"))!; + assert.equal(res.status, 200); + const body = (await res.json()) as { services: Array> }; + assert.equal(body.services.length, 1); + const svc = body.services[0]!; + assert.equal(svc["hook"], "patient-view"); + assert.equal(svc["id"], PATIENT_VIEW_SERVICE_ID); + // Declaring a prefetch template we then ignore would make a client fetch and send data for nothing. + assert.equal(svc["prefetch"], undefined, "no prefetch may be declared"); + assert.match(String(svc["usageRequirements"]), /does not evaluate data supplied on the request/); + assert.match(String(svc["usageRequirements"]), /never an empty card list/); +}); + +test("discovery refuses a non-GET; an unknown service is a 404 that lists what exists", async () => { + assert.equal((await post("/cds-services", {}))!.status, 405); + const res = (await post("/cds-services/not-a-service", hookBody("emp-006")))!; + assert.equal(res.status, 404); + const body = (await res.json()) as { error: string; known: string[] }; + assert.equal(body.error, "unknown_service"); + assert.deepEqual(body.known, [PATIENT_VIEW_SERVICE_ID]); + // A GET on the invoke path is a method error, not a silent empty response. + assert.equal((await call(INVOKE))!.status, 405); +}); + +test("invoke validates the hook contract before answering", async () => { + const cases: Array<[string, unknown]> = [ + ["hook and hookInstance are required", { context: { patientId: "emp-006" } }], + ["hook and hookInstance are required", { hook: "patient-view", context: { patientId: "emp-006" } }], + ["context.patientId is required", { hook: "patient-view", hookInstance: "x", context: {} }], + ]; + for (const [expected, body] of cases) { + const res = (await post(INVOKE, body))!; + assert.equal(res.status, 400, JSON.stringify(body)); + assert.match(((await res.json()) as { message: string }).message, new RegExp(expected)); + } + // A hook this service does not serve is refused rather than answered — the context fields may differ. + const wrongHook = (await post(INVOKE, hookBody("emp-006", { hook: "order-sign" })))!; + assert.equal(wrongHook.status, 400); + assert.match(((await wrongHook.json()) as { message: string }).message, /serves hook 'patient-view'/); + // A body that is not JSON at all. + const malformed = (await call(INVOKE, { method: "POST", headers: { "content-type": "application/json" }, body: "{oops" }))!; + assert.equal(malformed.status, 400); +}); + +test("invoke returns one card per open gap, with a suggestion only where the mapping is APPROVED", async () => { + const res = (await post(INVOKE, hookBody("emp-006")))!; + assert.equal(res.status, 200); + const { cards } = (await res.json()) as { cards: Array> }; + // audiogram + hazwoper are open; flu_vaccine is COMPLIANT and contributes nothing. + assert.equal(cards.length, 2); + const byMeasure = new Map(cards.map((c) => [String(c["summary"]).split(" — ")[0], c])); + const audiogram = byMeasure.get("Annual Audiogram Completed")!; + const hazwoper = byMeasure.get("HAZWOPER Surveillance")!; + assert.ok(audiogram, "the audiogram gap must be carded"); + // APPROVED (CPT 92557) → offered. REVIEWED with an internal code → information only. + assert.equal((audiogram["suggestions"] as unknown[]).length, 1); + assert.equal(hazwoper["suggestions"], undefined, "a REVIEWED mapping must not be offered as an order"); + // The card identity is the derived one, so a later feedback POST is correlatable. + assert.equal(audiogram["uuid"], await cardUuid(completedRunId, "emp-006", "audiogram")); +}); + +test("a bare hook patientId resolves the `wc|` live namespace", async () => { + // The trap: a WebChart client sends `4821`, WorkWell persists `wc|4821`. Without this the card list + // would be the no-evaluation card, which reads as "WorkWell doesn't know them" rather than a real gap. + assert.deepEqual(candidateSubjectIds("4821"), ["wc|4821", "4821"]); + assert.deepEqual(candidateSubjectIds("wc|4821"), ["wc|4821"], "an already-namespaced id is not double-prefixed"); + const { cards } = (await (await post(INVOKE, hookBody("4821")))!.json()) as { cards: Array> }; + assert.equal(cards.length, 1); + assert.match(String(cards[0]!["summary"]), /Annual Audiogram Completed/); +}); + +test("an evaluated-and-clean subject gets an EMPTY card list — the only correct silence", async () => { + const { cards } = (await (await post(INVOKE, hookBody("emp-007")))!.json()) as { cards: unknown[] }; + assert.deepEqual(cards, []); +}); + +test("an unknown patient, and a mid-run patient, each get a CARD saying so — never silence", async () => { + for (const patientId of ["nobody-at-all", "emp-mid"]) { + const { cards } = (await (await post(INVOKE, hookBody(patientId)))!.json()) as { + cards: Array>; + }; + assert.equal(cards.length, 1, `${patientId} must get exactly one card`); + assert.equal(cards[0]!["indicator"], "info"); + assert.match(String(cards[0]!["summary"]), /No WorkWell evaluation on record/, patientId); + // The distinction that matters: this must not be confusable with the empty list above. + assert.match(String(cards[0]!["detail"]), /absence of a run/); + } +}); + +test("every invocation writes an audit event carrying the uuids it emitted", async () => { + const stores = await getStores(env as never); + const count = async (type: string) => + (await stores.events.listAuditEvents(1000)).filter((e) => e.eventType === type).length; + const before = await count("CDS_HOOKS_INVOKED"); + await post(INVOKE, hookBody("emp-006")); + assert.equal(await count("CDS_HOOKS_INVOKED"), before + 1); + // A patient with nothing on record is a read too — enumeration must not be the silent case. + await post(INVOKE, hookBody("nobody-at-all")); + assert.equal(await count("CDS_HOOKS_INVOKED"), before + 2); + + const latest = (await stores.events.listAuditEvents(1000)).find((e) => e.eventType === "CDS_HOOKS_INVOKED")!; + const payload = latest.payload as Record; + assert.equal(payload["sensitivityLabel"], "restricted"); + assert.ok(Array.isArray(payload["cardUuids"]), "the emitted uuids must be recorded for correlation"); + + // Discovery must NOT write: it carries no patient data, and a public endpoint writing per request is a + // denial-of-service amplifier against our own ledger. + const beforeDiscovery = await count("CDS_HOOKS_INVOKED"); + await call("/cds-services"); + assert.equal(await count("CDS_HOOKS_INVOKED"), beforeDiscovery); +}); + +test("feedback validates the spec's conditional fields and records the outcome", async () => { + const path = `${INVOKE}/feedback`; + const uuid = await cardUuid(completedRunId, "emp-006", "audiogram"); + const bad: unknown[] = [ + { feedback: [] }, + { feedback: [{ card: uuid, outcome: "declined", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, + { feedback: [{ card: uuid, outcome: "accepted", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, + { feedback: [{ outcome: "overridden", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, + ]; + for (const body of bad) { + assert.equal((await post(path, body))!.status, 400, JSON.stringify(body)); + } + + const stores = await getStores(env as never); + const count = async () => + (await stores.events.listAuditEvents(1000)).filter((e) => e.eventType === "CDS_HOOKS_FEEDBACK_RECEIVED").length; + const before = await count(); + const ok = (await post(path, { + feedback: [ + { card: uuid, outcome: "accepted", acceptedSuggestions: [{ id: "s1" }], outcomeTimestamp: "2026-06-12T00:00:00Z" }, + { card: uuid, outcome: "overridden", outcomeTimestamp: "2026-06-12T00:00:00Z", overrideReason: { reason: { code: "not-now", system: "urn:x" }, userComment: "patient declined today" } }, + ], + }))!; + assert.equal(ok.status, 200); + assert.equal(await count(), before + 2, "one audit event per feedback entry"); + const rows = (await stores.events.listAuditEvents(1000)).filter((e) => e.eventType === "CDS_HOOKS_FEEDBACK_RECEIVED"); + const overridden = rows.find((r) => (r.payload as Record)["outcome"] === "overridden")!; + assert.equal((overridden.payload as Record)["overrideReasonCode"], "not-now"); +}); + +test("the auth matrix: discovery is public, invoke and feedback are not", () => { + // A pure `authorize` call, not a handler call — the gate runs in the worker before any handler, so + // asserting it here is asserting the thing that actually protects the route. + const viewer: JwtPrincipal = { email: "v@workwell.dev", role: "ROLE_VIEWER" } as JwtPrincipal; + const cm: JwtPrincipal = { email: "cm@workwell.dev", role: "ROLE_CASE_MANAGER" } as JwtPrincipal; + const mcp: JwtPrincipal = { email: "m@workwell.dev", role: "ROLE_MCP_CLIENT" } as JwtPrincipal; + const author: JwtPrincipal = { email: "a@workwell.dev", role: "ROLE_AUTHOR" } as JwtPrincipal; + + assert.deepEqual(authorize("GET", "/cds-services", null), { ok: true }, "discovery must be public"); + + // Invoke and feedback: anonymous is 401, and `/cds-services` is OUTSIDE `/api/`, where `authorize` + // otherwise ends in permitAll — so this assertion is the whole reason the rules exist. + for (const p of [`/cds-services/${PATIENT_VIEW_SERVICE_ID}`, `/cds-services/${PATIENT_VIEW_SERVICE_ID}/feedback`]) { + assert.deepEqual(authorize("POST", p, null), { ok: false, status: 401 }, p); + assert.deepEqual(authorize("POST", p, cm), { ok: true }, p); + assert.deepEqual(authorize("POST", p, mcp), { ok: true }, p); + assert.deepEqual(authorize("POST", p, author), { ok: false, status: 403 }, `${p} is not authoring work`); + // ROLE_VIEWER backs the public read-only sandbox and may never write — a POST here would also be a + // per-patient clinical read on someone else's behalf. + assert.deepEqual(authorize("POST", p, viewer), { ok: false, status: 403 }, p); + } + // A non-GET on the bare discovery path falls through to the gated rule rather than being public. + assert.deepEqual(authorize("POST", "/cds-services", null), { ok: false, status: 401 }); +}); diff --git a/backend-ts/src/routes/cds-hooks.ts b/backend-ts/src/routes/cds-hooks.ts new file mode 100644 index 00000000..7ca41e64 --- /dev/null +++ b/backend-ts/src/routes/cds-hooks.ts @@ -0,0 +1,322 @@ +/** + * CDS Hooks 2.0.1 service (ADR-067) — the standard shape for delivering a quality gap into the workflow. + * + * GET /cds-services → discovery (PUBLIC — metadata only) + * POST /cds-services/{serviceId} → cards + * POST /cds-services/{serviceId}/feedback → accepted/overridden, audited + * + * Normative reference: https://cds-hooks.hl7.org/2.0/ (HL7 balloted STU2). We serve the standard's + * contract; we do not redefine it, and we make no claim of external validation — there is no graded + * conformance suite for a CDS Hooks service (the community validator is JSON Schemas last touched in 2018; + * Inferno has no CDS Hooks kit). `docs/STANDARDS_CONFORMANCE.md` states that limit. + * + * ## Cards render a completed evaluation; they never trigger one + * + * Every card comes from a persisted outcome of a FINALIZED run — the same rule ADR-061's `mode=latest` + * follows, and for the same reason: the alternative is composing a bundle per request, which on a WebChart + * deployment would mean reporting synthetic playback as an evaluation (`mode=preview` returns 501 there). + * A consequence worth stating plainly: cards are as fresh as the last run, not as fresh as this encounter. + * + * ## Authentication is WorkWell's, NOT the CDS Hooks JWT profile + * + * The spec defines its own scheme — a JWT the CDS *Client* signs (RS384/ES384), verified against a JWKS, + * with `aud` equal to the invoked endpoint and an `iss`/`jku` allowlist. It **SHALL NOT** be signed with a + * symmetric algorithm, so WorkWell's HS256 token can never be a conformant CDS Hooks JWT — this is a named + * gap, not an oversight (ADR-067). Invoke and feedback are gated by the ordinary bearer token via + * `authorize.ts`; discovery is PERMIT because it returns service metadata and no patient data. + */ +import type { CloudDatabase } from "@mieweb/cloud"; +import { getStores } from "../stores/factory.ts"; +import { parseAllowedOrigins } from "../config/cors.ts"; +import { resolveStandingOrderProvider, type StandingOrderEnv } from "../order/standing-order-provider.ts"; +import { CDS_SERVICES, serviceById } from "../cds/discovery.ts"; +import { buildComplianceCards, noEvaluationCard, type CardInput } from "../cds/cards.ts"; +import type { CdsFeedbackRequest, CdsRequest } from "../cds/types.ts"; + +interface CdsHooksEnv extends StandingOrderEnv { + DB: CloudDatabase; + DATABASE_URL?: string; + WORKWELL_CORS_ALLOWED_ORIGINS?: string; +} + +const json = (data: unknown, status = 200): Response => + new Response(JSON.stringify(data), { status, headers: { "content-type": "application/json" } }); + +/** A run whose outcomes are an answer. Mirrors ADR-061's set exactly — `PARTIAL_FAILURE` IS terminal. */ +const FINAL: ReadonlySet = new Set(["COMPLETED", "PARTIAL_FAILURE"]); + +/** + * The same bounded window `mode=latest` and MCP's `check_compliance` use. + * + * `outcomes` has no uniqueness on (subject, measure, period), so every run inserts a fresh row — a nightly + * ALL_PROGRAMS run over ~16 measures writes ~16 rows per subject per night. A small window would make an + * older valid outcome read as "never evaluated", which is the one confusion this route must not create. + */ +const SCAN = 100000; + +/** `/cds-services`, `/cds-services/{id}`, `/cds-services/{id}/feedback` — or null when not ours. */ +export function parseCdsPath( + pathname: string, +): { kind: "discovery" } | { kind: "invoke" | "feedback"; serviceId: string } | null { + if (pathname === "/cds-services") return { kind: "discovery" }; + const m = /^\/cds-services\/([^/]+)(\/feedback)?$/.exec(pathname); + if (!m) return null; + let serviceId: string; + try { + serviceId = decodeURIComponent(m[1]!); + } catch { + // A bad percent-escape is the caller's error; `decodeURIComponent` throwing would otherwise reach the + // worker's catch-all as a 500 (the #399 lesson, same shape as `parseCompliancePath`). + return null; + } + return { kind: m[2] ? "feedback" : "invoke", serviceId }; +} + +/** + * A hook's `context.patientId` is a bare EHR id; WorkWell persists live subjects as `wc|` + * (`live-directory.ts`, `run-pipeline.ts`) and the synthetic directory as `emp-006`. Both are tried, live + * namespace first, so a WebChart client does not silently read as "no gaps" — the trap + * `docs/PROPOSALS_2026-08.md` §P1 names. + */ +export function candidateSubjectIds(patientId: string): string[] { + return patientId.startsWith("wc|") ? [patientId] : [`wc|${patientId}`, patientId]; +} + +/** The Studio origin, for card links. Taken from the CORS allowlist — definitionally the frontend. */ +function studioBaseUrl(env: CdsHooksEnv): string | undefined { + return parseAllowedOrigins(env.WORKWELL_CORS_ALLOWED_ORIGINS)[0]; +} + +async function auditCds( + env: CdsHooksEnv, + actor: string, + eventType: "CDS_HOOKS_INVOKED" | "CDS_HOOKS_FEEDBACK_RECEIVED", + detail: Record, +): Promise { + try { + const stores = await getStores(env); + await stores.events.appendAudit({ + eventType, + entityType: "cds_hooks", + entityId: crypto.randomUUID(), + actor, + refRunId: (detail["runId"] as string | undefined) ?? null, + refCaseId: null, + refMeasureVersionId: null, + payload: { sensitivityLabel: "restricted", timestamp: new Date().toISOString(), ...detail }, + }); + } catch (err) { + // Best-effort at the response boundary: an audit failure must not turn a correct answer into a 500. + console.error(`WORKWELL_ALERT cds-hooks audit write failed: ${String(err)}`); + } +} + +/** + * The newest FINALIZED outcome per measure for the first subject id that resolves to any row at all. + * + * Returns the subject id it used, so the caller can tell "this patient is unknown to WorkWell" from "this + * patient has only mid-run rows" — two absences that must not be reported identically. + */ +async function latestFinalizedByMeasure( + env: CdsHooksEnv, + patientId: string, +): Promise<{ subjectId: string; rows: CardInput[] } | null> { + const stores = await getStores(env); + const runStatus = new Map(); + const isFinal = async (runId: string): Promise => { + const cached = runStatus.get(runId); + if (cached !== undefined) return cached; + const run = await stores.runs.getRun(runId); + const ok = !!run && FINAL.has(run.status); + runStatus.set(runId, ok); + return ok; + }; + + for (const subjectId of candidateSubjectIds(patientId)) { + const all = await stores.outcomes.listOutcomesForEmployee(subjectId, SCAN); + if (all.length === 0) continue; + const byMeasure = new Map(); + for (const r of all) { + // Newest-first, so the first finalized row per measure wins. + if (byMeasure.has(r.measureId)) continue; + if (!(await isFinal(r.runId))) continue; + byMeasure.set(r.measureId, { + measureId: r.measureId, + status: r.status, + evidence: r.evidence, + evaluationPeriod: r.evaluationPeriod, + runId: r.runId, + evaluatedAt: r.evaluatedAt, + }); + } + return { subjectId, rows: [...byMeasure.values()] }; + } + return null; +} + +/** `"{system}|{code}"` for every APPROVED terminology mapping — read from the STORE, never the seed. */ +async function approvedOrderCodes(env: CdsHooksEnv): Promise> { + const stores = await getStores(env); + const mappings = await stores.valueSets.listTerminologyMappings(); + return new Set( + mappings + .filter((m) => m.mappingStatus === "APPROVED") + .map((m) => `${m.standardSystem}|${m.standardCode}`), + ); +} + +async function readJson(req: Request): Promise { + try { + return await req.json(); + } catch { + return "malformed"; + } +} + +export async function handleCdsHooks( + req: Request, + env: CdsHooksEnv, + /** The authenticated subject, for the audit trail. */ + actor = "system", +): Promise { + const route = parseCdsPath(new URL(req.url).pathname); + if (!route) return null; + + if (route.kind === "discovery") { + if (req.method !== "GET") return json({ error: "method_not_allowed" }, 405); + // No audit event: discovery carries no patient data, and a public endpoint that writes an audit row + // per request is a denial-of-service amplifier against our own ledger. + return json({ services: CDS_SERVICES }); + } + + const service = serviceById(route.serviceId); + if (!service) { + return json( + { + error: "unknown_service", + message: `unknown CDS service '${route.serviceId}'`, + known: CDS_SERVICES.map((s) => s.id), + }, + 404, + ); + } + if (req.method !== "POST") return json({ error: "method_not_allowed" }, 405); + + const body = await readJson(req); + if (body === "malformed" || typeof body !== "object" || body === null) { + return json({ error: "invalid_request", message: "request body must be a JSON object" }, 400); + } + + return route.kind === "feedback" + ? feedback(body as CdsFeedbackRequest, env, actor, service.id) + : invoke(body as CdsRequest, env, actor, service.id, service.hook); +} + +async function invoke( + body: CdsRequest, + env: CdsHooksEnv, + actor: string, + serviceId: string, + expectedHook: string, +): Promise { + if (!body.hook || !body.hookInstance) { + return json({ error: "invalid_request", message: "hook and hookInstance are required" }, 400); + } + if (body.hook !== expectedHook) { + // Refusing beats guessing: a mismatched hook means the client's context fields may not be the ones + // this service reads, so answering would be answering a different question. + return json( + { error: "invalid_request", message: `service '${serviceId}' serves hook '${expectedHook}', not '${body.hook}'` }, + 400, + ); + } + const patientId = body.context?.patientId; + if (!patientId) { + return json({ error: "invalid_request", message: "context.patientId is required" }, 400); + } + + const base = studioBaseUrl(env); + const found = await latestFinalizedByMeasure(env, patientId); + // No subject, or no finalized outcome → ONE informational card, never an empty list. See `noEvaluationCard`. + const cards = + found === null || found.rows.length === 0 + ? [noEvaluationCard(patientId, base)] + : await buildComplianceCards(found.rows, { + subjectId: found.subjectId, + approvedOrderCodes: await approvedOrderCodes(env), + standingOrders: resolveStandingOrderProvider(env), + studioBaseUrl: base, + }); + + await auditCds(env, actor, "CDS_HOOKS_INVOKED", { + serviceId, + hook: body.hook, + hookInstance: body.hookInstance, + patientId, + subjectId: found?.subjectId ?? null, + cardCount: cards.length, + // The uuids this invocation emitted. This is what makes a later feedback event correlatable without + // persisting a card table: feedback cites a uuid, this event maps that uuid to a patient and subject, + // and `cardUuid` recomputes the measure from the subject's own outcomes. See `feedback` below. + cardUuids: cards.map((c) => c.uuid).filter((u): u is string => !!u), + }); + return json({ cards }); +} + +/** + * Feedback — the only leg of the send/receive reconciliation in guide S7 that WorkWell can build alone. + * + * Nothing is persisted beyond the audit event, and nothing needed to be — which is the whole reason this + * endpoint could be built without a schema change (schema is the owner's alone, CLAUDE.md). + * + * **How a bare uuid becomes a measure.** CDS Hooks feedback carries card uuids and nothing else — no + * patient, no service context — so a service cannot resolve one from the request alone. Two properties + * make it recoverable anyway: the `CDS_HOOKS_INVOKED` event records the uuids it emitted alongside the + * patient and subject, and `cardUuid` is a pure function of `(runId, subjectId, measureId)`, so the measure + * is recomputed from that subject's outcomes rather than looked up. Deterministic ids also mean a client + * that re-fires the hook for an unchanged run gets the SAME uuid, so repeat feedback does not fragment + * across ids. We deliberately do not guess: this handler records the uuid verbatim and asserts nothing + * about what it referred to. + */ +async function feedback( + body: CdsFeedbackRequest, + env: CdsHooksEnv, + actor: string, + serviceId: string, +): Promise { + const entries = body.feedback; + if (!Array.isArray(entries) || entries.length === 0) { + return json({ error: "invalid_request", message: "feedback must be a non-empty array" }, 400); + } + for (const e of entries) { + if (!e.card || !e.outcomeTimestamp) { + return json({ error: "invalid_request", message: "each feedback entry needs card and outcomeTimestamp" }, 400); + } + if (e.outcome !== "accepted" && e.outcome !== "overridden") { + return json( + { error: "invalid_request", message: "outcome must be 'accepted' or 'overridden'" }, + 400, + ); + } + if (e.outcome === "accepted" && (!Array.isArray(e.acceptedSuggestions) || e.acceptedSuggestions.length === 0)) { + // CONDITIONAL in the spec: acceptedSuggestions is REQUIRED for an `accepted` outcome. + return json( + { error: "invalid_request", message: "acceptedSuggestions is required when outcome is 'accepted'" }, + 400, + ); + } + } + + for (const e of entries) { + await auditCds(env, actor, "CDS_HOOKS_FEEDBACK_RECEIVED", { + serviceId, + card: e.card, + outcome: e.outcome, + outcomeTimestamp: e.outcomeTimestamp, + ...(e.overrideReason?.reason?.code ? { overrideReasonCode: e.overrideReason.reason.code } : {}), + ...(e.overrideReason?.userComment ? { userComment: e.overrideReason.userComment } : {}), + }); + } + // 200 with no body: the spec defines no response payload for feedback. + return new Response(null, { status: 200 }); +} diff --git a/backend-ts/src/worker.test.ts b/backend-ts/src/worker.test.ts index ca9411f6..c749c2d4 100644 --- a/backend-ts/src/worker.test.ts +++ b/backend-ts/src/worker.test.ts @@ -58,6 +58,28 @@ test("a protected route without a token is 401", async () => { assert.equal((await call("/api/runs")).status, 401); }); +test("CDS Hooks: discovery is reachable with no token, invoking is not", async () => { + // Through the REAL worker with auth ENABLED, because that is the only place the ordering of the auth + // gate and the handler is exercised. `/cds-services` is outside `/api/`, where `authorize` ends in + // permitAll — so an omitted rule would show up here as a 200 on the invoke path, not as a unit failure. + const discovery = await call("/cds-services"); + assert.equal(discovery.status, 200); + const { services } = (await discovery.json()) as { services: Array<{ hook: string; id: string }> }; + assert.equal(services.length, 1); + assert.equal(services[0]!.hook, "patient-view"); + + const invoke = await call(`/cds-services/${services[0]!.id}`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ hook: "patient-view", hookInstance: "i", context: { patientId: "emp-006" } }), + }); + assert.equal(invoke.status, 401, "invoking must require a token — it returns per-patient clinical status"); + assert.equal( + (await call(`/cds-services/${services[0]!.id}/feedback`, { method: "POST", body: "{}" })).status, + 401, + ); +}); + test("login → token → authorized access, and role gates return 403", async () => { const login = await call("/api/auth/login", { method: "POST", diff --git a/backend-ts/src/worker.ts b/backend-ts/src/worker.ts index 79e1750a..b9eac598 100644 --- a/backend-ts/src/worker.ts +++ b/backend-ts/src/worker.ts @@ -31,6 +31,7 @@ import { handleQuality } from "./routes/quality.ts"; import { handleIdentity } from "./routes/identity.ts"; import { handleCompliance } from "./routes/compliance.ts"; import { handleComplianceApi } from "./routes/compliance-api.ts"; +import { handleCdsHooks } from "./routes/cds-hooks.ts"; import { handleSegments } from "./routes/segments.ts"; import { handleOutcomes } from "./routes/outcomes.ts"; import { handleImmunizationForecast } from "./routes/immunization.ts"; @@ -292,6 +293,12 @@ async function route(req: Request, env: Env, ctx: CloudExecutionContext): Promis const complianceApiResponse = await handleComplianceApi(req, env, principalRole, actor); if (complianceApiResponse) return complianceApiResponse; + // CDS Hooks (ADR-067) — the standards-shaped delivery of the same answer the compliance API returns, for + // a CDS client rather than an integrator. Placed beside it because they are the same contract surface; + // `/cds-services` is outside `/api/`, so it cannot collide with anything above. + const cdsResponse = await handleCdsHooks(req, env, actor); + if (cdsResponse) return cdsResponse; + // Compliance roster — individual compliance status grid by panel (#189 E10.2). const complianceResponse = await handleCompliance(req, env); if (complianceResponse) return complianceResponse; From dc60cbb21b050100e0e19419945ce77bf8351b62 Mon Sep 17 00:00:00 2001 From: Taleef Date: Mon, 17 Aug 2026 11:49:47 -0400 Subject: [PATCH 2/7] feat(openapi): publish the integration contract as OpenAPI 3.1, guarded against drift GET /api/v1/openapi.json now serves a hand-authored 3.1.1 document covering the PROMISED surface only -- /api/v1/compliance, the three /cds-services operations, health and version -- and stating that the ~40 internal /api/** routes are excluded because they carry no stability promise. Public: reading a contract should not require credentials. The guard is what makes hand-authoring defensible, and it is two-way. Every (path, method, status) the document declares is produced by a real request through the real worker, and every response the tests observe is declared. A documented route that is not routed fails with "documented but NOT ROUTED" -- which is precisely how ARCHITECTURE.md came to assert a springdoc OpenAPI document for a year after the JVM was retired. All three directions mutation-checked: deleting the route, removing a produced status, and requiring an absent property each fail the intended assertion. Redocly lints the document in CI (pinned, telemetry off, no ignore file) because it catches a class the contract test cannot -- and did on the first run: five uses of `nullable`, which OpenAPI 3.1 removed in favour of type unions. The five remaining warnings are explained in spec.ts rather than silenced; four are operations that genuinely have no 4xx, and their 405 belongs to the path, not the GET, which the coverage test correctly refused to let us mis-model. Also adds the first smoke coverage of the integration surface: the document, CDS discovery, one real card assembly and one v1 compliance read. None was probed before. Suite 1969 across the affected files, 0 fail. Redocly: 0 errors, 5 warnings. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/ci.yml | 13 + backend-ts/package.json | 1 + backend-ts/scripts/openapi-emit.mjs | 24 ++ backend-ts/src/auth/authorize.ts | 3 + backend-ts/src/openapi/spec.ts | 465 ++++++++++++++++++++++++++ backend-ts/src/routes/openapi.test.ts | 350 +++++++++++++++++++ backend-ts/src/routes/openapi.ts | 35 ++ backend-ts/src/worker.ts | 6 + scripts/smoke-shadow.sh | 23 ++ 9 files changed, 920 insertions(+) create mode 100644 backend-ts/scripts/openapi-emit.mjs create mode 100644 backend-ts/src/openapi/spec.ts create mode 100644 backend-ts/src/routes/openapi.test.ts create mode 100644 backend-ts/src/routes/openapi.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 337d5cdb..79e9b085 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -155,6 +155,19 @@ jobs: pnpm compile-measures git add -A -- src/engine/cql/elm src/measure/resources/cql-resources.json git diff --cached --exit-code --stat -- src/engine/cql/elm src/measure/resources/cql-resources.json + # The OpenAPI document must be valid OpenAPI, which `openapi.test.ts` deliberately does NOT check: its + # job is agreement between the document and the running worker, and no amount of that catches a 3.0-ism + # like `nullable` (which 3.1 removed, and which this found on the first run). Two different guards. + # Pinned exactly — `@latest` would make the gate non-reproducible, the same reason the terminology fetch + # is pinned. REDOCLY_TELEMETRY=off is not optional: the CLI otherwise reports environment-variable + # values and the names of the rules that fired. Exits 0 on warnings, non-zero on errors; the five + # expected warnings are explained in src/openapi/spec.ts and are deliberately not ignore-filed. + - name: The OpenAPI document is valid OpenAPI 3.1 + env: + REDOCLY_TELEMETRY: "off" + run: | + node --import tsx scripts/openapi-emit.mjs "$RUNNER_TEMP/openapi.json" + npx --yes @redocly/cli@2.46.1 lint "$RUNNER_TEMP/openapi.json" - name: Test (SQLite floor + Postgres ceiling) env: WORKWELL_TEST_PG_URL: postgres://workwell:workwell@localhost:5432/workwell diff --git a/backend-ts/package.json b/backend-ts/package.json index cea75a27..195b2e93 100644 --- a/backend-ts/package.json +++ b/backend-ts/package.json @@ -34,6 +34,7 @@ "test": "node --import tsx --test \"src/**/*.test.ts\" \"packages/*/src/**/*.test.ts\" \"scripts/**/*.test.mjs\" \"scripts/**/*.test.ts\"", "build:packages": "node scripts/build-packages.mjs", "verify:publish": "node scripts/verify-publish.mjs", + "openapi:emit": "node --import tsx scripts/openapi-emit.mjs", "official:terminology-audit": "node scripts/official-terminology-audit.mjs", "vendor:official": "node scripts/vendor-official-measure.mjs" }, diff --git a/backend-ts/scripts/openapi-emit.mjs b/backend-ts/scripts/openapi-emit.mjs new file mode 100644 index 00000000..ab2fafee --- /dev/null +++ b/backend-ts/scripts/openapi-emit.mjs @@ -0,0 +1,24 @@ +/** + * Emit the OpenAPI document to a file, so an external validator can read it (ADR-068). + * + * node --import tsx scripts/openapi-emit.mjs [outfile] # default: openapi.json in the cwd + * + * Why this exists rather than a committed `openapi.json`: the document is generated from + * `src/openapi/spec.ts`, so a committed copy would be a second artifact that can disagree with the served + * one — the exact drift the contract test exists to prevent. CI emits it to a temp path, lints it, and + * throws it away; the only source of truth is the code the worker serves. + * + * CI (`.github/workflows/ci.yml`, the backend-ts job): + * node --import tsx scripts/openapi-emit.mjs "$RUNNER_TEMP/openapi.json" + * REDOCLY_TELEMETRY=off npx --yes @redocly/cli@2.46.1 lint "$RUNNER_TEMP/openapi.json" + * + * Redocly is pinned exactly, for the same reason the official-terminology fetch is: `@latest` in CI makes + * the gate non-reproducible. `REDOCLY_TELEMETRY=off` is not optional — the CLI otherwise reports + * environment-variable values and the names of rules that fired. + */ +import { writeFileSync } from "node:fs"; +import { openApiDocument } from "../src/openapi/spec.ts"; + +const out = process.argv[2] ?? "openapi.json"; +writeFileSync(out, `${JSON.stringify(openApiDocument(), null, 2)}\n`); +console.log(`wrote ${out}`); diff --git a/backend-ts/src/auth/authorize.ts b/backend-ts/src/auth/authorize.ts index 7b94f093..7f2ea21b 100644 --- a/backend-ts/src/auth/authorize.ts +++ b/backend-ts/src/auth/authorize.ts @@ -59,6 +59,9 @@ const RULES: Rule[] = [ { pattern: rx("/api/health"), access: "PERMIT" }, { pattern: rx("/api/version"), access: "PERMIT" }, { pattern: rx("/health"), access: "PERMIT" }, + // The OpenAPI document (ADR-068) — shapes and role names, no patient data. Public because reading the + // contract without credentials is most of its value to an integrator. Must precede the `/api/**` tails. + { pattern: rx("/api/v1/openapi.json"), access: "PERMIT" }, { pattern: rx("/sse"), access: [A, CM, MCP] }, { pattern: rx("/mcp/**"), access: [A, CM, MCP] }, diff --git a/backend-ts/src/openapi/spec.ts b/backend-ts/src/openapi/spec.ts new file mode 100644 index 00000000..f35655fe --- /dev/null +++ b/backend-ts/src/openapi/spec.ts @@ -0,0 +1,465 @@ +/** + * The WorkWell OpenAPI document (ADR-068) — hand-authored, zero dependencies. + * + * ## Scope: the PROMISED surface only + * + * `/api/v1/**` and the standards surfaces, plus health and version. The ~40 internal `/api/**` routes are + * deliberately absent: `docs/COMPLIANCE_API.md` draws that line already ("everything else under `/api/` is + * internal and moves with the frontend"), and documenting them here would advertise stability over paths + * that carry none. A small document that stays true beats a large one that starts drifting the day it lands + * — which is not a hypothetical, since `ARCHITECTURE.md` claimed a springdoc OpenAPI document for a year + * after the JVM was retired. + * + * ## Why 3.1.1 and not 3.2 + * + * 3.2.0 exists (Sept 2025) and buys these seven operations nothing. Renderer support is worse than absent, + * it is *silent*: Redoc 2.5.3 accepts a 3.2 document by aliasing it to 3.1, so 3.2-only constructs are + * ignored rather than flagged, and Spectral caps at 3.1. 3.1's Schema Objects are literal JSON Schema + * 2020-12, which is also what makes the zero-dependency response check in `openapi.test.ts` tractable. + * + * ## Why hand-authored + * + * The alternatives all cost a dependency or a rewrite: zod is only worth it as a *runtime* validator (a new + * runtime dep), `@hono/zod-openapi` presupposes Hono and a router we do not have, and TypeSpec would add a + * second hand-maintained source of truth with no coupling to a hand-rolled dispatcher. The recognised risk + * of hand-authoring is drift, and the recognised answer is a contract test — which is why the guard in + * `openapi.test.ts` is not optional garnish here but the other half of the decision. + * + * ## CDS Hooks is DESCRIBED, not redefined + * + * The only published OpenAPI description of CDS Hooks is `cds-hooks/api` — Swagger 2.0, CDS Hooks 1.0, last + * pushed January 2021 — so there is nothing current to `$ref`. These operations describe OUR conformance to + * the shapes in https://cds-hooks.hl7.org/2.0/, and the spec text says exactly that. + * + * ## The five Redocly warnings are expected, and neither is silenced + * + * `redocly lint` reports 0 errors and 5 warnings, and no ignore file is used, because an ignore file hides a + * finding rather than answering it. One is `info-license` (see below). The other four are + * `operation-4xx-response` on the four unauthenticated GETs that genuinely have no 4xx: health, version, + * discovery and this document all answer 200 or nothing. Their non-GET behaviour is a **405 on the path**, + * which OpenAPI models per-operation and therefore cannot express under a `get` — declaring it there would + * make the document say the GET returns 405, which is false, and the two-way coverage test in + * `openapi.test.ts` catches exactly that (it did, on the first attempt). + */ + +export interface OpenApiSchema { + /** A union such as `["string", "null"]` is how OpenAPI 3.1 spells nullability — 3.0's `nullable` was removed. */ + type?: string | string[]; + format?: string; + description?: string; + enum?: string[]; + properties?: Record; + required?: string[]; + items?: OpenApiSchema; + additionalProperties?: boolean | OpenApiSchema; + $ref?: string; + example?: unknown; +} + +export interface OpenApiResponse { + description: string; + content?: Record; +} + +export interface OpenApiParameter { + name: string; + in: "path" | "query"; + required?: boolean; + description?: string; + schema: OpenApiSchema; +} + +export interface OpenApiOperation { + operationId: string; + summary: string; + description?: string; + tags: string[]; + security?: Array>; + parameters?: OpenApiParameter[]; + requestBody?: { required: boolean; content: Record }; + responses: Record; +} + +export interface OpenApiDocument { + openapi: string; + info: Record; + servers: Array<{ url: string; description?: string }>; + tags: Array<{ name: string; description: string }>; + paths: Record>; + components: { + securitySchemes: Record>; + schemas: Record; + }; +} + +const BEARER = [{ bearerAuth: [] as string[] }]; + +const str = (description: string, example?: unknown): OpenApiSchema => ({ type: "string", description, ...(example !== undefined ? { example } : {}) }); +const bool = (description: string): OpenApiSchema => ({ type: "boolean", description }); +const jsonBody = (ref: string): Record => ({ + "application/json": { schema: { $ref: `#/components/schemas/${ref}` } }, +}); + +const errorResponse = (description: string): OpenApiResponse => ({ description, content: jsonBody("Error") }); + +export function openApiDocument(): OpenApiDocument { + return { + openapi: "3.1.1", + info: { + title: "WorkWell Measure Studio — integration API", + version: "1.0.0", + summary: "Given a patient and a measure, are they compliant?", + description: [ + "The versioned contract an integrator builds against, plus the CDS Hooks service.", + "", + "**Scope.** This document covers `/api/v1/**`, the CDS Hooks surface, and health/version. The rest of", + "`/api/**` is an internal contract that moves with the frontend and carries no stability promise, so it", + "is deliberately not described here.", + "", + "**Stability.** A field present in this document will not be removed or change type, and a breaking", + "change means `/api/v2/`. New fields may appear — parse permissively and ignore what you do not", + "recognise.", + "", + "**CDS Hooks.** The `/cds-services` operations describe WorkWell's conformance to the shapes defined by", + "CDS Hooks 2.0.1 (https://cds-hooks.hl7.org/2.0/); they do not redefine that specification. Note that", + "authentication here is WorkWell's own bearer token, NOT the CDS Hooks signed-JWT profile, which", + "forbids symmetric algorithms and is a documented gap.", + "", + "**Compliance is computed by CQL and only by CQL.** No response in this document is produced by an AI", + "surface.", + ].join("\n"), + // No `license` block. Redocly warns either way — `info-license` when it is absent, and + // `info-license-strict` when it is present without a URL or SPDX identifier — and this repo publishes + // no licence for the application, so asserting one would be the worse of two equal warnings. + }, + servers: [{ url: "/", description: "This deployment" }], + tags: [ + { name: "compliance", description: "Per-subject, per-measure compliance answers." }, + { name: "cds-hooks", description: "CDS Hooks 2.0.1 service — decision support cards for a patient." }, + { name: "meta", description: "Discovery and health." }, + ], + paths: { + "/api/v1/compliance/{subjectId}/{measureId}": { + get: { + operationId: "getCompliance", + summary: "Is this subject compliant with this measure?", + description: [ + "`mode=latest` (default) reads the most recent outcome from a FINALIZED run. `mode=preview`", + "evaluates now and persists nothing, is restricted to `ROLE_CASE_MANAGER`/`ROLE_ADMIN` because it", + "costs an evaluation, and returns **501** on a WebChart-configured deployment where it would", + "otherwise evaluate a synthetic bundle and report it as an evaluation of real data.", + "", + "Read `populationsSource` before trusting `populations`: `status-derived` means only the initial", + "population is measured and the rest are inferred from `status`.", + ].join("\n"), + tags: ["compliance"], + security: BEARER, + parameters: [ + { + name: "subjectId", + in: "path", + required: true, + description: "The employee/patient external id. Percent-encode it — WebChart ids contain `|`.", + schema: str("Subject external id", "emp-006"), + }, + { + name: "measureId", + in: "path", + required: true, + description: "A WorkWell catalog id. An unknown id is a 400 that lists the known ids.", + schema: str("Measure id", "cms125"), + }, + { name: "start", in: "query", description: "Inclusive lower bound on the evaluation period.", schema: { type: "string", format: "date" } }, + { name: "end", in: "query", description: "Inclusive upper bound on the evaluation period.", schema: { type: "string", format: "date" } }, + { name: "mode", in: "query", description: "`latest` (default) or `preview`.", schema: { type: "string", enum: ["latest", "preview"] } }, + ], + responses: { + "200": { description: "The compliance answer.", content: jsonBody("ComplianceAnswer") }, + "400": errorResponse("Malformed request: unknown measure, bad date, bad percent-encoding, or `start` with `mode=preview`."), + "401": errorResponse("No bearer token."), + "403": errorResponse("`mode=preview` requires ROLE_CASE_MANAGER or ROLE_ADMIN."), + "404": errorResponse("No FINALIZED outcome covers this subject and measure. This is the absence of a run, NOT a statement of compliance."), + "501": errorResponse("`mode=preview` is unavailable on a WebChart-configured deployment."), + }, + }, + }, + "/cds-services": { + get: { + operationId: "cdsDiscovery", + summary: "Discover the CDS Hooks services this deployment offers", + description: "Public — service metadata only, no patient data. No `prefetch` is declared, because none is evaluated.", + tags: ["cds-hooks"], + security: [], + responses: { + "200": { description: "The service catalog.", content: jsonBody("CdsDiscoveryResponse") }, + }, + }, + }, + "/cds-services/{serviceId}": { + post: { + operationId: "cdsInvoke", + summary: "Invoke a CDS Hooks service and receive cards", + description: [ + "Cards render the most recent FINALIZED WorkWell evaluation. `prefetch`, `fhirServer` and", + "`fhirAuthorization` are accepted and **not** evaluated.", + "", + "A patient with no completed evaluation receives one informational card saying so — never an empty", + "card list, which at the point of care would read as \"no gaps\". An empty list means the patient", + "*was* evaluated and has none.", + ].join("\n"), + tags: ["cds-hooks"], + security: BEARER, + parameters: [{ name: "serviceId", in: "path", required: true, description: "A service `id` from discovery.", schema: str("Service id", "workwell-compliance-patient-view") }], + requestBody: { required: true, content: jsonBody("CdsRequest") }, + responses: { + "200": { description: "Cards (possibly empty).", content: jsonBody("CdsResponse") }, + "400": errorResponse("Missing `hook`/`hookInstance`/`context.patientId`, a hook this service does not serve, or a body that is not JSON."), + "401": errorResponse("No bearer token."), + "403": errorResponse("The authenticated role may not invoke a CDS service."), + "404": errorResponse("Unknown service id."), + }, + }, + }, + "/cds-services/{serviceId}/feedback": { + post: { + operationId: "cdsFeedback", + summary: "Report that a card was accepted or overridden", + description: "Audited. `acceptedSuggestions` is required when `outcome` is `accepted`.", + tags: ["cds-hooks"], + security: BEARER, + parameters: [{ name: "serviceId", in: "path", required: true, description: "A service `id` from discovery.", schema: str("Service id", "workwell-compliance-patient-view") }], + requestBody: { required: true, content: jsonBody("CdsFeedbackRequest") }, + responses: { + "200": { description: "Recorded. No response body." }, + "400": errorResponse("Empty feedback array, an outcome other than `accepted`/`overridden`, or `accepted` without `acceptedSuggestions`."), + "401": errorResponse("No bearer token."), + "403": errorResponse("The authenticated role may not submit feedback."), + "404": errorResponse("Unknown service id."), + }, + }, + }, + "/api/v1/openapi.json": { + get: { + operationId: "getOpenApiDocument", + summary: "This document", + tags: ["meta"], + security: [], + responses: { + "200": { description: "The OpenAPI 3.1 document.", content: { "application/json": { schema: { type: "object", description: "An OpenAPI 3.1 document." } } } }, + }, + }, + }, + "/actuator/health": { + get: { + operationId: "getHealth", + summary: "Liveness", + description: "Deliberately DB-free: a 200 here is not evidence that the database is reachable.", + tags: ["meta"], + security: [], + responses: { "200": { description: "The worker is serving.", content: jsonBody("Health") } }, + }, + }, + "/api/version": { + get: { + operationId: "getVersion", + summary: "API version and build", + tags: ["meta"], + security: [], + responses: { "200": { description: "Version discovery.", content: jsonBody("Version") } }, + }, + }, + }, + components: { + securitySchemes: { + bearerAuth: { + type: "http", + scheme: "bearer", + bearerFormat: "JWT", + description: "An access token from `POST /api/auth/login`. Refresh tokens are rejected.", + }, + }, + schemas: { + Error: { + type: "object", + description: "Every non-2xx response in this document.", + required: ["error"], + properties: { + error: str("A stable machine-readable code, e.g. `no_outcome`.", "no_outcome"), + message: str("Human-readable detail. Not stable; do not parse."), + }, + }, + ComplianceAnswer: { + type: "object", + required: ["subject", "measure", "period", "filter", "status", "populations", "populationsSource", "provenance"], + properties: { + subject: { type: "object", required: ["id"], properties: { id: str("The subject external id.", "emp-006") } }, + measure: { + type: "object", + required: ["id", "name"], + properties: { + id: str("WorkWell catalog id.", "cms125"), + name: str("Display name."), + ecqmId: str("Present only when an official CMS artifact produced this outcome.", "CMS125FHIR"), + version: str("The official artifact version, when applicable.", "1.0.000"), + }, + }, + period: { + type: "object", + description: "The measurement window the ANSWER covers — not an echo of the request.", + required: ["start", "end"], + properties: { start: { type: ["string", "null"], description: "ISO-8601, or null when genuinely unknown." }, end: { type: ["string", "null"], description: "ISO-8601, or null when genuinely unknown." } }, + }, + filter: { + type: "object", + description: "The bounds YOU sent, echoed back so the two can never be read as each other.", + required: ["start", "end"], + properties: { start: { type: ["string", "null"] }, end: { type: ["string", "null"] } }, + }, + status: { + type: "string", + description: "THE ANSWER.", + enum: ["COMPLIANT", "DUE_SOON", "OVERDUE", "MISSING_DATA", "EXCLUDED"], + }, + populations: { + type: "object", + required: ["initialPopulation", "denominator", "denominatorExclusion", "denominatorException", "numerator"], + properties: { + initialPopulation: bool("In the measure's initial population."), + denominator: bool("In the denominator."), + denominatorExclusion: bool("Excluded from the denominator."), + denominatorException: bool("A denominator exception."), + numerator: bool("In the numerator."), + }, + }, + populationsSource: { + type: "string", + description: "`official-evidence` = the executor's own measured vector. `status-derived` = only the initial population is real; the rest are inferred from `status`. Read this before trusting `populations`.", + enum: ["official-evidence", "status-derived"], + }, + provenance: { + type: "object", + description: "Diagnostic. Its contents may grow.", + additionalProperties: true, + properties: { + mode: { type: "string", enum: ["latest", "preview"] }, + runId: { type: ["string", "null"], description: "Null for `mode=preview`, because there is no run." }, + evaluatedAt: str("ISO-8601."), + }, + }, + }, + }, + CdsService: { + type: "object", + required: ["hook", "id", "title", "description", "usageRequirements"], + properties: { + hook: str("The hook this service is invoked on.", "patient-view"), + id: str("The `{serviceId}` in the invoke path.", "workwell-compliance-patient-view"), + title: str("Human-readable name."), + description: str("What the service returns."), + usageRequirements: str("What a caller must know — including what this service does NOT do."), + }, + }, + CdsDiscoveryResponse: { + type: "object", + required: ["services"], + properties: { services: { type: "array", items: { $ref: "#/components/schemas/CdsService" } } }, + }, + CdsRequest: { + type: "object", + required: ["hook", "hookInstance", "context"], + properties: { + hook: str("Must match the service's declared hook.", "patient-view"), + hookInstance: str("A UUID identifying this invocation."), + fhirServer: { type: "string", description: "Accepted and NOT evaluated." }, + fhirAuthorization: { type: "object", description: "Accepted and NOT evaluated.", additionalProperties: true }, + prefetch: { type: "object", description: "Accepted and NOT evaluated. No prefetch template is declared.", additionalProperties: true }, + context: { + type: "object", + required: ["patientId"], + properties: { + patientId: str("The FHIR `Patient.id`. A bare WebChart id is also tried as `wc|`.", "emp-006"), + userId: str("`Practitioner/abc` or `PractitionerRole/123`."), + encounterId: str("The current encounter, when the client has one."), + }, + }, + }, + }, + CdsCard: { + type: "object", + required: ["summary", "indicator", "source"], + properties: { + summary: str("At most 140 characters, per the CDS Hooks specification."), + indicator: { + type: "string", + description: "`critical` is never emitted: it means the user must not proceed, and WorkWell is supplementary to WebChart.", + enum: ["info", "warning"], + }, + source: { + type: "object", + required: ["label"], + properties: { label: str("Always `WorkWell Measure Studio`."), url: str("A link to the measure, when a Studio origin is configured.") }, + }, + detail: str("Markdown. Names the run and states that the answer was computed by CQL."), + uuid: str("Derived from (runId, subjectId, measureId) — stable across re-invocations of the same run. Cite it when posting feedback."), + links: { type: "array", items: { type: "object", required: ["label", "url", "type"], properties: { label: str("Link text."), url: str("Absolute URL."), type: { type: "string", enum: ["absolute"] } } } }, + suggestions: { + type: "array", + description: "Present only where the proposed order code carries an APPROVED terminology mapping.", + items: { + type: "object", + required: ["label", "uuid", "actions"], + properties: { + label: str("Button text."), + uuid: str("Cite this in `acceptedSuggestions`."), + actions: { type: "array", items: { type: "object", required: ["type", "description", "resource"], properties: { type: { type: "string", enum: ["create"] }, description: str("What accepting would do."), resource: { type: "object", description: "A FHIR R4 ServiceRequest with `intent=proposal`, `status=draft`. Advisory — a clinician submits.", additionalProperties: true } } } }, + }, + }, + }, + selectionBehavior: { type: "string", description: "Present whenever `suggestions` is.", enum: ["at-most-one"] }, + }, + }, + CdsResponse: { + type: "object", + required: ["cards"], + properties: { + cards: { + type: "array", + description: "Empty ONLY when the patient was evaluated and has no open gap. An unevaluated patient gets one informational card instead.", + items: { $ref: "#/components/schemas/CdsCard" }, + }, + }, + }, + CdsFeedbackRequest: { + type: "object", + required: ["feedback"], + properties: { + feedback: { + type: "array", + items: { + type: "object", + required: ["card", "outcome", "outcomeTimestamp"], + properties: { + card: str("The `card.uuid` from the invoke response."), + outcome: { type: "string", enum: ["accepted", "overridden"] }, + acceptedSuggestions: { type: "array", description: "Required when `outcome` is `accepted`.", items: { type: "object", properties: { id: str("The `suggestion.uuid`.") } } }, + overrideReason: { type: "object", additionalProperties: true, description: "A Coding plus an optional free-text comment." }, + outcomeTimestamp: str("ISO-8601 UTC."), + }, + }, + }, + }, + }, + Health: { + type: "object", + required: ["status", "stack"], + properties: { status: { type: "string", enum: ["UP"] }, stack: str("Implementation identifier.", "workwell-ts") }, + }, + Version: { + type: "object", + required: ["api", "stack", "build"], + properties: { api: str("The API contract version.", "v1"), stack: str("Implementation.", "typescript"), build: str("Build identifier.", "workwell-api-ts") }, + }, + }, + }, + }; +} diff --git a/backend-ts/src/routes/openapi.test.ts b/backend-ts/src/routes/openapi.test.ts new file mode 100644 index 00000000..8738e8dd --- /dev/null +++ b/backend-ts/src/routes/openapi.test.ts @@ -0,0 +1,350 @@ +/** + * The OpenAPI document and its anti-drift guard (ADR-068). + * node --import tsx --test src/routes/openapi.test.ts + * + * ## Why this file is shaped like this + * + * A hand-authored spec drifts, and this repo has the worked example: `ARCHITECTURE.md` asserted "The + * OpenAPI document (`workwell.swagger.enabled=true`) advertises version `v1`" for a year after the JVM that + * served it was retired. Nobody noticed, because nothing executed the claim. + * + * So the load-bearing test is not "the document is valid JSON". It is **two-way coverage**: every + * `(path, method, status)` the document declares is produced by a real request through the real worker, and + * every response these tests observe is declared. A documented operation that was never implemented fails + * the first direction; an undocumented status fails the second. No maintained `node:test` OpenAPI assertion + * library exists, so the structural response check is hand-rolled — tractable because OpenAPI 3.1 Schema + * Objects are literal JSON Schema 2020-12 and our responses are flat. + */ +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { rmSync } from "node:fs"; +// @ts-expect-error — @mieweb/cloud-local ships .mjs without types +import { createSqliteD1 } from "@mieweb/cloud-local"; +import { RUN_STORE_FLOOR_DDL } from "../stores/sqlite/schema.ts"; +import { SqliteRunStore } from "../stores/sqlite/run-store-sqlite.ts"; +import { SqliteOutcomeStore } from "../stores/sqlite/outcome-store-sqlite.ts"; +import { openApiDocument, type OpenApiSchema } from "../openapi/spec.ts"; +import { OPENAPI_PATH } from "./openapi.ts"; +import { PATIENT_VIEW_SERVICE_ID } from "../cds/discovery.ts"; +import { authorize } from "../auth/authorize.ts"; +import worker from "../worker.ts"; +import type { Env } from "../worker.ts"; + +const dbPath = join(tmpdir(), `ww-openapi-${crypto.randomUUID()}.sqlite`); +const doc = openApiDocument(); +const env = { WORKWELL_AUTH_JWT_SECRET: "x".repeat(40) } as unknown as Env; +/** The same worker under a WebChart-configured seam, where `mode=preview` must refuse. */ +let webChartEnv: Env; +const ctx = {} as never; + +const INVOKE = `/cds-services/${PATIENT_VIEW_SERVICE_ID}`; +const COMPLIANCE_TEMPLATE = "/api/v1/compliance/{subjectId}/{measureId}"; +const INVOKE_TEMPLATE = "/cds-services/{serviceId}"; +const FEEDBACK_TEMPLATE = "/cds-services/{serviceId}/feedback"; + +/** `"METHOD template status"` for everything these tests actually saw. */ +const observed = new Set(); +const tokens: Record = {}; + +async function login(email: string): Promise { + if (tokens[email]) return tokens[email]!; + const res = await worker.fetch( + new Request("http://x/api/auth/login", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ email, password: "Workwell123!" }), + }), + env, + ctx, + ); + assert.equal(res.status, 200, `login for ${email}`); + const { token } = (await res.json()) as { token: string }; + tokens[email] = token; + return token; +} + +interface ProbeOptions { + template: string; + method?: string; + token?: string; + body?: unknown; + useEnv?: Env; +} + +/** + * Send a real request through the real worker, record what came back, and refuse the one answer that would + * mean the document describes a route that does not exist. + */ +async function probe(path: string, o: ProbeOptions): Promise { + const method = o.method ?? "GET"; + const init: RequestInit = { method, headers: {} }; + if (o.token) (init.headers as Record)["authorization"] = `Bearer ${o.token}`; + if (o.body !== undefined) { + (init.headers as Record)["content-type"] = "application/json"; + init.body = typeof o.body === "string" ? o.body : JSON.stringify(o.body); + } + const res = await worker.fetch(new Request(`http://x${path}`, init), o.useEnv ?? env, ctx); + observed.add(`${method} ${o.template} ${res.status}`); + + // The worker answers an unrouted path with 501 `not_implemented`. A documented path that produces it is + // documentation of something that does not exist — exactly the ARCHITECTURE.md failure. + if (res.status === 501) { + const clone = res.clone(); + const body = (await clone.json().catch(() => ({}))) as { error?: string }; + assert.notEqual( + body.error, + "not_implemented", + `${method} ${path} is documented but NOT ROUTED — the worker fell through to its 501 catch-all`, + ); + } + return res; +} + +before(async () => { + const db = await createSqliteD1(dbPath); + await db.exec(RUN_STORE_FLOOR_DDL.replace(/\n/g, " ")); + (env as unknown as { DB: unknown }).DB = db; + webChartEnv = { + ...(env as unknown as Record), + WORKWELL_WEBCHART_BASE_URL: "https://webchart.example.org", + WORKWELL_WEBCHART_API_KEY: "k", + } as unknown as Env; + + const runs = new SqliteRunStore(db); + const outcomes = new SqliteOutcomeStore(db); + const run = await runs.createRun({ + scopeType: "ALL_PROGRAMS", + triggeredBy: "test", + requestedScope: {}, + measurementPeriodStart: "2025-06-12T00:00:00.000Z", + measurementPeriodEnd: "2026-06-12T00:00:00.000Z", + }); + await runs.finalizeRun(run.id, "COMPLETED"); + await outcomes.recordOutcome({ + runId: run.id, + subjectId: "emp-006", + measureId: "audiogram", + status: "OVERDUE", + evaluationPeriod: "2026-06-12", + evidence: { + expressionResults: [ + { define: "Most Recent Audiogram Date", result: "2025-03-10T00:00:00Z" }, + { define: "Days Since Last Audiogram", result: 420 }, + ], + }, + }); +}); + +after(() => { try { rmSync(dbPath, { force: true }); } catch { /* best effort */ } }); + +test("the document is a well-formed OpenAPI 3.1 description with no dangling references", () => { + assert.equal(doc.openapi, "3.1.1"); + assert.ok(doc.info["title"], "info.title is required"); + assert.deepEqual(doc.servers, [{ url: "/", description: "This deployment" }]); + + const schemaNames = new Set(Object.keys(doc.components.schemas)); + const referenced = new Set(); + const operationIds = new Set(); + const tagNames = new Set(doc.tags.map((t) => t.name)); + + const walk = (s: OpenApiSchema): void => { + if (s.$ref) { + const name = s.$ref.replace("#/components/schemas/", ""); + assert.ok(schemaNames.has(name), `dangling $ref: ${s.$ref}`); + referenced.add(name); + } + Object.values(s.properties ?? {}).forEach(walk); + if (s.items) walk(s.items); + if (typeof s.additionalProperties === "object") walk(s.additionalProperties); + }; + Object.values(doc.components.schemas).forEach(walk); + + for (const [path, item] of Object.entries(doc.paths)) { + // Every `{param}` in the path must be declared, and every declared path param must be in the path. + const inPath = [...path.matchAll(/\{([^}]+)\}/g)].map((m) => m[1]!); + for (const [method, op] of Object.entries(item)) { + assert.ok(!operationIds.has(op.operationId), `duplicate operationId ${op.operationId}`); + operationIds.add(op.operationId); + op.tags.forEach((t) => assert.ok(tagNames.has(t), `${op.operationId} uses undeclared tag ${t}`)); + const declared = (op.parameters ?? []).filter((p) => p.in === "path").map((p) => p.name); + assert.deepEqual([...declared].sort(), [...inPath].sort(), `${method} ${path} path parameters`); + assert.ok(Object.keys(op.responses).length > 0, `${op.operationId} declares no responses`); + Object.values(op.responses).forEach((r) => + Object.values(r.content ?? {}).forEach((c) => walk(c.schema)), + ); + Object.values(op.requestBody?.content ?? {}).forEach((c) => walk(c.schema)); + } + } + + // An unreferenced component schema is dead documentation — it says a shape matters when nothing uses it. + assert.deepEqual([...schemaNames].filter((n) => !referenced.has(n)).sort(), [], "unreferenced schemas"); +}); + +test("the document is served, publicly, at one canonical path", async () => { + const res = await probe(OPENAPI_PATH, { template: OPENAPI_PATH }); + assert.equal(res.status, 200); + assert.equal(res.headers.get("content-type"), "application/json"); + const served = (await res.json()) as { openapi: string; paths: Record }; + assert.equal(served.openapi, "3.1.1"); + assert.deepEqual(Object.keys(served.paths).sort(), Object.keys(doc.paths).sort()); + // Public: readable with no token, because reading a contract should not need credentials. + assert.deepEqual(authorize("GET", OPENAPI_PATH, null), { ok: true }); + // The aliases the journal probed are deliberately NOT served — one canonical URL. + for (const alias of ["/api/openapi.json", "/api/swagger", "/swagger-ui", "/api/docs"]) { + assert.notEqual(authorize("GET", alias, null).ok && alias === OPENAPI_PATH, true); + } +}); + +test("every documented status is produced by a real request through the real worker", async () => { + const cm = await login("cm@workwell.dev"); + const author = await login("author@workwell.dev"); + + // --- meta --- + assert.equal((await probe("/actuator/health", { template: "/actuator/health" })).status, 200); + assert.equal((await probe("/api/version", { template: "/api/version" })).status, 200); + + // --- compliance --- + const ok = await probe("/api/v1/compliance/emp-006/audiogram", { template: COMPLIANCE_TEMPLATE, token: cm }); + assert.equal(ok.status, 200); + assert.equal((await probe("/api/v1/compliance/emp-006/nope", { template: COMPLIANCE_TEMPLATE, token: cm })).status, 400); + assert.equal((await probe("/api/v1/compliance/emp-006/audiogram", { template: COMPLIANCE_TEMPLATE })).status, 401); + assert.equal( + (await probe("/api/v1/compliance/emp-006/audiogram?mode=preview", { template: COMPLIANCE_TEMPLATE, token: author })).status, + 403, + "preview costs an evaluation, so it is CM/ADMIN only", + ); + assert.equal((await probe("/api/v1/compliance/nobody/audiogram", { template: COMPLIANCE_TEMPLATE, token: cm })).status, 404); + assert.equal( + (await probe("/api/v1/compliance/emp-006/audiogram?mode=preview", { template: COMPLIANCE_TEMPLATE, token: cm, useEnv: webChartEnv })).status, + 501, + "preview on a WebChart stack would evaluate a synthetic bundle, so it refuses", + ); + + // --- cds hooks --- + assert.equal((await probe("/cds-services", { template: "/cds-services" })).status, 200); + const hook = { hook: "patient-view", hookInstance: crypto.randomUUID(), context: { patientId: "emp-006" } }; + assert.equal((await probe(INVOKE, { template: INVOKE_TEMPLATE, method: "POST", token: cm, body: hook })).status, 200); + assert.equal((await probe(INVOKE, { template: INVOKE_TEMPLATE, method: "POST", token: cm, body: { hook: "patient-view" } })).status, 400); + assert.equal((await probe(INVOKE, { template: INVOKE_TEMPLATE, method: "POST", body: hook })).status, 401); + assert.equal((await probe(INVOKE, { template: INVOKE_TEMPLATE, method: "POST", token: author, body: hook })).status, 403); + assert.equal((await probe("/cds-services/nope", { template: INVOKE_TEMPLATE, method: "POST", token: cm, body: hook })).status, 404); + + const fb = { feedback: [{ card: "c", outcome: "overridden", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }; + assert.equal((await probe(`${INVOKE}/feedback`, { template: FEEDBACK_TEMPLATE, method: "POST", token: cm, body: fb })).status, 200); + assert.equal((await probe(`${INVOKE}/feedback`, { template: FEEDBACK_TEMPLATE, method: "POST", token: cm, body: { feedback: [] } })).status, 400); + assert.equal((await probe(`${INVOKE}/feedback`, { template: FEEDBACK_TEMPLATE, method: "POST", body: fb })).status, 401); + assert.equal((await probe(`${INVOKE}/feedback`, { template: FEEDBACK_TEMPLATE, method: "POST", token: author, body: fb })).status, 403); + assert.equal((await probe("/cds-services/nope/feedback", { template: FEEDBACK_TEMPLATE, method: "POST", token: cm, body: fb })).status, 404); +}); + +test("coverage is two-way: nothing documented is unexercised, nothing observed is undocumented", () => { + // This test depends on the one above having run — node:test runs a file's tests in order. + const documented = new Set(); + for (const [path, item] of Object.entries(doc.paths)) { + for (const [method, op] of Object.entries(item)) { + for (const status of Object.keys(op.responses)) { + documented.add(`${method.toUpperCase()} ${path} ${status}`); + } + } + } + const missing = [...documented].filter((d) => !observed.has(d)).sort(); + const undocumented = [...observed].filter((o) => !documented.has(o)).sort(); + assert.deepEqual(missing, [], "documented but never produced by a request — is it implemented?"); + assert.deepEqual(undocumented, [], "produced by a request but absent from the document"); +}); + +test("a real 200 response validates against its documented schema", async () => { + const cm = await login("cm@workwell.dev"); + const cases: Array<[string, string, RequestInit | undefined, string]> = [ + ["GET", "/actuator/health", { headers: { authorization: `Bearer ${cm}` } }, "/actuator/health"], + ["GET", "/api/version", undefined, "/api/version"], + ["GET", "/api/v1/compliance/emp-006/audiogram", { headers: { authorization: `Bearer ${cm}` } }, COMPLIANCE_TEMPLATE], + ["GET", "/cds-services", undefined, "/cds-services"], + ]; + for (const [method, path, init, template] of cases) { + const res = await worker.fetch(new Request(`http://x${path}`, { method, ...init }), env, ctx); + assert.equal(res.status, 200, path); + const schema = doc.paths[template]![method.toLowerCase()]!.responses["200"]!.content!["application/json"]!.schema; + validate(await res.json(), schema, path); + } + + // And a POST body-bearing one, so the CDS card schema is exercised against real cards. + const invoke = await worker.fetch( + new Request(`http://x${INVOKE}`, { + method: "POST", + headers: { authorization: `Bearer ${cm}`, "content-type": "application/json" }, + body: JSON.stringify({ hook: "patient-view", hookInstance: crypto.randomUUID(), context: { patientId: "emp-006" } }), + }), + env, + ctx, + ); + assert.equal(invoke.status, 200); + const payload = (await invoke.json()) as { cards: unknown[] }; + assert.ok(payload.cards.length > 0, "the fixture must produce a card, or this assertion is vacuous"); + validate(payload, doc.paths[INVOKE_TEMPLATE]!["post"]!.responses["200"]!.content!["application/json"]!.schema, INVOKE); +}); + +/** + * A structural check against an OpenAPI 3.1 Schema Object — required properties present, declared types + * agreeing, enums respected, `$ref` followed one level into `#/components/schemas`, and **no undocumented + * property**, which is the direction that catches drift. + * + * Deliberately not a JSON Schema engine: our responses use no `oneOf`/`allOf`/`discriminator`, so the 2020-12 + * machinery would be dependency for nothing. If that changes, `ajv/dist/2020` is the dev-only escalation — + * 3.1 Schema Objects are literal 2020-12, so no conversion shim is involved. + */ +function validate(value: unknown, schema: OpenApiSchema, where: string): void { + const resolved = schema.$ref + ? doc.components.schemas[schema.$ref.replace("#/components/schemas/", "")]! + : schema; + assert.ok(resolved, `${where}: unresolved schema`); + + // OpenAPI 3.1 spells nullability as a type UNION (`["string", "null"]`); 3.0's `nullable` keyword was + // removed, and Redocly rejects it — which is a class of error this hand-rolled check cannot see, and the + // reason the linter is in CI alongside it. + const types = Array.isArray(resolved.type) ? resolved.type : resolved.type ? [resolved.type] : []; + if (value === null) { + assert.ok(types.includes("null"), `${where}: null is not permitted by [${types.join(", ")}]`); + return; + } + if (resolved.enum) { + assert.ok(resolved.enum.includes(String(value)), `${where}: ${String(value)} not in [${resolved.enum.join(", ")}]`); + } + switch (types.filter((t) => t !== "null")[0]) { + case "object": { + assert.equal(typeof value, "object", `${where}: expected object`); + assert.notEqual(value, null, `${where}: expected object, got null`); + const obj = value as Record; + for (const req of resolved.required ?? []) { + assert.ok(req in obj, `${where}: missing required property '${req}'`); + } + const props = resolved.properties ?? {}; + for (const [k, v] of Object.entries(obj)) { + const child = props[k]; + if (!child) { + // `additionalProperties: true` marks a deliberately open bag (e.g. `provenance`). + assert.equal(resolved.additionalProperties, true, `${where}: undocumented property '${k}'`); + continue; + } + if (v === undefined) continue; + validate(v, child, `${where}.${k}`); + } + return; + } + case "array": { + assert.ok(Array.isArray(value), `${where}: expected array`); + if (resolved.items) (value as unknown[]).forEach((v, i) => validate(v, resolved.items!, `${where}[${i}]`)); + return; + } + case "string": + assert.equal(typeof value, "string", `${where}: expected string, got ${typeof value}`); + return; + case "boolean": + assert.equal(typeof value, "boolean", `${where}: expected boolean, got ${typeof value}`); + return; + default: + return; // untyped (an open object) — nothing to assert + } +} diff --git a/backend-ts/src/routes/openapi.ts b/backend-ts/src/routes/openapi.ts new file mode 100644 index 00000000..9368496a --- /dev/null +++ b/backend-ts/src/routes/openapi.ts @@ -0,0 +1,35 @@ +/** + * The OpenAPI document route (ADR-068). + * + * GET /api/v1/openapi.json + * + * One canonical path. `/api/openapi.json`, `/api/swagger`, `/swagger-ui` and `/api/docs` were all probed + * against production and staging and all returned `501 not_implemented`; the recorded complaint was that no + * document existed, not that it sat at the wrong path, so aliases are not added to chase guesses. The human + * entry point is the Studio's API reference page, which reads this URL. + * + * PERMIT, deliberately: the document describes shapes and names roles, and carries no patient data. An + * integrator should be able to read the contract without credentials — that is most of its value. + */ +import { openApiDocument } from "../openapi/spec.ts"; + +export const OPENAPI_PATH = "/api/v1/openapi.json"; + +export function handleOpenApi(req: Request): Response | null { + if (new URL(req.url).pathname !== OPENAPI_PATH) return null; + if (req.method !== "GET") { + return new Response(JSON.stringify({ error: "method_not_allowed" }), { + status: 405, + headers: { "content-type": "application/json" }, + }); + } + return new Response(JSON.stringify(openApiDocument(), null, 2), { + headers: { + // `application/openapi+json` is not registered; `application/json` is what every renderer and + // validator actually accepts, so the specific one would only break tooling. + "content-type": "application/json", + // The document changes only when the code does, so a short cache is safe and keeps a docs page snappy. + "cache-control": "public, max-age=300", + }, + }); +} diff --git a/backend-ts/src/worker.ts b/backend-ts/src/worker.ts index b9eac598..9d298e5d 100644 --- a/backend-ts/src/worker.ts +++ b/backend-ts/src/worker.ts @@ -32,6 +32,7 @@ import { handleIdentity } from "./routes/identity.ts"; import { handleCompliance } from "./routes/compliance.ts"; import { handleComplianceApi } from "./routes/compliance-api.ts"; import { handleCdsHooks } from "./routes/cds-hooks.ts"; +import { handleOpenApi } from "./routes/openapi.ts"; import { handleSegments } from "./routes/segments.ts"; import { handleOutcomes } from "./routes/outcomes.ts"; import { handleImmunizationForecast } from "./routes/immunization.ts"; @@ -215,6 +216,11 @@ async function route(req: Request, env: Env, ctx: CloudExecutionContext): Promis return json({ api: "v1", stack: "typescript", build: "workwell-api-ts" }); } + // The OpenAPI document (ADR-068) — served before the auth gate, like health and version, because it is + // the contract an integrator reads before they have a token. + const openApiResponse = handleOpenApi(req); + if (openApiResponse) return openApiResponse; + // Authorization gate — port of JwtAuthFilter + SecurityConfig (#105). Skipped // entirely when auth is disabled (no secret), mirroring authEnabled=false → permitAll. // The authenticated subject becomes the audit actor (SecurityActor.currentActor()). diff --git a/scripts/smoke-shadow.sh b/scripts/smoke-shadow.sh index 80b21ea1..d49c922d 100644 --- a/scripts/smoke-shadow.sh +++ b/scripts/smoke-shadow.sh @@ -67,6 +67,29 @@ st=$(req POST /api/auth/refresh) rt="$(jq -r '.token // empty' "$BODY")" { [ "$st" = "200" ] && [ -n "$rt" ] && pass "POST /api/auth/refresh → rotated token (cookie round-trips)" && TOKEN="$rt"; } || fail "POST /api/auth/refresh → $st (refresh cookie not honored?)" +# 2b. the integration surface: the OpenAPI document, the CDS Hooks service, and the v1 contract. +# These are what an integrator (and MIE) reach for, and until now none of them was smoke-checked at all. +# The OpenAPI document and CDS discovery are PUBLIC, so they are probed before any token matters — but +# `req` sends the token once set, which is harmless here. +echo "[integration surface]" +st=$(req GET /api/v1/openapi.json); ver="$(jq -r '.openapi // empty' "$BODY")" +{ [ "$st" = "200" ] && [ -n "$ver" ] && pass "GET /api/v1/openapi.json → OpenAPI $ver"; } || fail "GET /api/v1/openapi.json → $st (the document is not served)" +st=$(req GET /cds-services); svc="$(jq -r '[.services[]?.id] | join(",")' "$BODY" 2>/dev/null || echo "")" +{ [ "$st" = "200" ] && [ -n "$svc" ] && pass "GET /cds-services → $svc"; } || fail "GET /cds-services → $st (CDS Hooks discovery is not served)" +# One real card assembly, through the standard contract. emp-006 is an OVERDUE audiogram in the synthetic +# roster; a patient WorkWell has never evaluated would return one informational card instead, so a non-empty +# array here is not by itself proof of a gap — the summary is. +st=$(req POST "/cds-services/workwell-compliance-patient-view" \ + "{\"hook\":\"patient-view\",\"hookInstance\":\"smoke-$$\",\"context\":{\"userId\":\"Practitioner/smoke\",\"patientId\":\"emp-006\"}}") +cards="$(jq -r '.cards | length' "$BODY" 2>/dev/null || echo "?")" +{ [ "$st" = "200" ] && pass "POST /cds-services/... → $cards card(s): $(jq -r '[.cards[]?.summary] | join(" | ")' "$BODY" 2>/dev/null | head -c 120)"; } || fail "CDS Hooks invoke → $st $(head -c 120 "$BODY")" +st=$(req GET "/api/v1/compliance/emp-006/audiogram") +if [ "$st" = "200" ]; then + pass "GET /api/v1/compliance/emp-006/audiogram → $(jq -r '.status' "$BODY") (source: $(jq -r '.populationsSource' "$BODY"))" +elif [ "$st" = "404" ]; then + warn "GET /api/v1/compliance/emp-006/audiogram → 404 (no finalized run has covered this subject yet)" +else fail "GET /api/v1/compliance/emp-006/audiogram → $st $(head -c 120 "$BODY")"; fi + # 3. measures: catalog count + a detail with value sets echo "[measures]" st=$(req GET /api/measures); n=$(jq 'length' "$BODY" 2>/dev/null || echo "?") From 72a8e5d8c52cc9a9c171f804fdf413a3cfb8c5c2 Mon Sep 17 00:00:00 2001 From: Taleef Date: Mon, 17 Aug 2026 11:58:01 -0400 Subject: [PATCH 3/7] feat(frontend): a public API reference rendered from the OpenAPI document /api-docs is a top-level route, deliberately outside (dashboard) and unauthenticated: the document it renders is public, and an integrator should be able to read the contract before they have credentials. It fetches with a plain fetch rather than lib/api/client, whose token attachment and silent-refresh behaviour would be wrong on a page nobody is logged into. Hand-rolled rather than vendored, and not only on the no-new-dependency rule: swagger-ui-react peers on react ">=16.8 <19" and this app is on React 19, so the React integration does not exist for us; Swagger UI's dark mode is a hard-coded html.dark-mode class that would fight this app for ownership of ; and Scalar and Redoc both default to a CDN script that the CSP and offline demo rule out. The page inherits the Enterprise Health brand and dark mode instead (ADR-004). The trade is no try-it-out console -- a copyable curl instead, which for a bearer-token API is about as useful and needs no proxy. The load-bearing test is that no operation is silently dropped: an untagged operation lands under "Other" rather than vanishing, which is the same defect class as a documented-but-unrouted path one layer up. A 3.1 type union renders as "string | null" rather than "string", so a nullable field reads as nullable. Frontend: lint clean, 186 tests pass, build clean, /api-docs prerendered. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/app/(dashboard)/layout.tsx | 4 + frontend/app/api-docs/page.tsx | 80 ++++++ .../api-docs/api-reference.test.tsx | 108 ++++++++ .../components/api-docs/api-reference.tsx | 262 ++++++++++++++++++ frontend/components/api-docs/types.ts | 83 ++++++ 5 files changed, 537 insertions(+) create mode 100644 frontend/app/api-docs/page.tsx create mode 100644 frontend/components/api-docs/api-reference.test.tsx create mode 100644 frontend/components/api-docs/api-reference.tsx create mode 100644 frontend/components/api-docs/types.ts diff --git a/frontend/app/(dashboard)/layout.tsx b/frontend/app/(dashboard)/layout.tsx index 2f822dbf..a6ad3e90 100644 --- a/frontend/app/(dashboard)/layout.tsx +++ b/frontend/app/(dashboard)/layout.tsx @@ -12,6 +12,7 @@ import { ListChecks, LogOut, Send, + Code2, Settings, Shield, Users, @@ -60,6 +61,9 @@ const nav = [ { href: "/studio", label: "Studio", icon: FileClock, roles: [ROLES.AUTHOR, ROLES.APPROVER, ROLES.ADMIN] }, { href: "/runs", label: "Runs", icon: Activity }, { href: "/admin", label: "Admin", icon: Settings, roles: [ROLES.ADMIN] }, + // The integration contract (ADR-068). No `roles`: the page and the document it renders are both public, + // so every authenticated role can reach it — and so can anyone without an account. + { href: "/api-docs", label: "API", icon: Code2 }, ] as const; const DATE_PRESETS = [ diff --git a/frontend/app/api-docs/page.tsx b/frontend/app/api-docs/page.tsx new file mode 100644 index 00000000..8194bcbb --- /dev/null +++ b/frontend/app/api-docs/page.tsx @@ -0,0 +1,80 @@ +"use client"; + +/** + * The public API reference (ADR-068). + * + * A top-level route, NOT inside `app/(dashboard)/`, and deliberately unauthenticated: the document it + * renders is public, and an integrator should be able to read the contract before they have credentials. + * That is most of what this page is for. + * + * It fetches with a plain `fetch` rather than through `lib/api/client`, because that client attaches a + * bearer token and drives the silent-refresh/logout flow on a 401 — behaviour that makes sense for the + * dashboard and would be wrong on a page nobody is logged into. + */ +import { useEffect, useState } from "react"; +import Link from "next/link"; +import { ApiReference } from "@/components/api-docs/api-reference"; +import type { OpenApiDoc } from "@/components/api-docs/types"; + +const API_BASE = (process.env.NEXT_PUBLIC_API_BASE_URL ?? "").trim().replace(/\/+$/, ""); +const SPEC_PATH = "/api/v1/openapi.json"; +const APP_NAME = process.env.NEXT_PUBLIC_APP_NAME ?? "WorkWell Measure Studio"; + +export default function ApiDocsPage() { + const [doc, setDoc] = useState(null); + const [error, setError] = useState(null); + + useEffect(() => { + let active = true; + fetch(`${API_BASE}${SPEC_PATH}`) + .then(async (res) => { + if (!res.ok) throw new Error(`the API returned ${res.status}`); + return (await res.json()) as OpenApiDoc; + }) + .then((d) => { if (active) setDoc(d); }) + .catch((e: unknown) => { if (active) setError(e instanceof Error ? e.message : String(e)); }); + return () => { active = false; }; + }, []); + + const specUrl = `${API_BASE}${SPEC_PATH}`; + + return ( +
+
+

{APP_NAME}

+

+ {doc?.info?.title ?? "Integration API"} +

+ {doc?.info?.summary &&

{doc.info.summary}

} +

+ OpenAPI {doc?.openapi ?? "3.1"} ·{" "} + machine-readable document ·{" "} + Studio +

+
+ + {doc?.info?.description && ( +
+ {doc.info.description.split("\n\n").map((para, i) => ( +

{para}

+ ))} +
+ )} + + {error && ( + // Honest, and specific about which of the two failures this is — an unreachable API and an API with + // no document are different problems for whoever is reading. +
+

The API reference could not be loaded.

+

+ Fetching {specUrl} failed: {error}. The document is served by the + backend, so this page is empty whenever the backend is unreachable. +

+
+ )} + + {!doc && !error &&

Loading the API reference…

} + {doc && "} />} +
+ ); +} diff --git a/frontend/components/api-docs/api-reference.test.tsx b/frontend/components/api-docs/api-reference.test.tsx new file mode 100644 index 00000000..7b07f22e --- /dev/null +++ b/frontend/components/api-docs/api-reference.test.tsx @@ -0,0 +1,108 @@ +import React from "react"; +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { ApiReference } from "./api-reference"; +import { endpointsOf, typeLabel, type OpenApiDoc } from "./types"; + +/** + * The renderer for the OpenAPI reference page (ADR-068). + * + * The assertion that matters is that **no operation is silently dropped**. A reference page that renders + * five of six operations looks finished and is wrong in the one way a reader cannot detect — the same defect + * class as a documented-but-unrouted path, one layer up. + */ +const doc: OpenApiDoc = { + openapi: "3.1.1", + info: { title: "WorkWell", version: "1.0.0" }, + tags: [{ name: "compliance", description: "Answers." }], + paths: { + "/api/v1/compliance/{subjectId}/{measureId}": { + get: { + operationId: "getCompliance", + summary: "Is this subject compliant?", + tags: ["compliance"], + security: [{ bearerAuth: [] }], + parameters: [ + { name: "subjectId", in: "path", required: true, description: "The subject id.", schema: { type: "string", example: "emp-006" } }, + { name: "mode", in: "query", schema: { type: "string", enum: ["latest", "preview"] } }, + ], + responses: { + "200": { description: "The answer.", content: { "application/json": { schema: { $ref: "#/components/schemas/Answer" } } } }, + "404": { description: "No finalized outcome." }, + }, + }, + }, + "/cds-services": { + // No tag at all — this operation must still appear. + get: { operationId: "cdsDiscovery", summary: "Discover services", security: [], responses: { "200": { description: "Catalog." } } }, + }, + }, + components: { + schemas: { + Answer: { + type: "object", + required: ["status", "period"], + properties: { + status: { type: "string", description: "THE ANSWER.", enum: ["COMPLIANT", "OVERDUE"] }, + period: { type: "object", properties: { start: { type: ["string", "null"], description: "ISO-8601 or null." } } }, + }, + }, + }, + }, +}; + +describe("ApiReference", () => { + it("renders every operation in the document, including untagged ones", () => { + render(); + // Two operations exist; both must be on the page. + expect(endpointsOf(doc)).toHaveLength(2); + expect(screen.getByText("Is this subject compliant?")).toBeInTheDocument(); + expect(screen.getByText("Discover services")).toBeInTheDocument(); + // The untagged one lands under "Other" rather than vanishing. + expect(screen.getByText("Other")).toBeInTheDocument(); + expect(screen.getByText("compliance")).toBeInTheDocument(); + }); + + it("shows the method, path, auth posture, parameters and status codes", () => { + render(); + expect(screen.getByText("/api/v1/compliance/{subjectId}/{measureId}")).toBeInTheDocument(); + expect(screen.getAllByText("GET").length).toBe(2); + // A bearer-gated operation and a public one must be distinguishable at a glance. + expect(screen.getByText("bearer token")).toBeInTheDocument(); + expect(screen.getByText("public")).toBeInTheDocument(); + expect(screen.getByText("subjectId")).toBeInTheDocument(); + expect(screen.getByText("latest | preview")).toBeInTheDocument(); + expect(screen.getByText("No finalized outcome.")).toBeInTheDocument(); + }); + + it("follows a $ref into components and renders the 3.1 nullable union honestly", () => { + render(); + expect(screen.getByText("THE ANSWER.")).toBeInTheDocument(); + // `["string","null"]` must read as a union, not as "string" — the field really can be null. + expect(screen.getByText("string | null")).toBeInTheDocument(); + }); + + it("builds a curl example that substitutes path parameters and includes the token header", () => { + render(); + // Scoped to the compliance operation: both operations render a curl block against the same origin. + const curl = screen.getByText(/^curl -sS https:\/\/api\.example\.org\/api\/v1\/compliance/); + expect(curl.textContent).toContain("/api/v1/compliance/emp-006/"); + expect(curl.textContent).toContain("authorization: Bearer "); + expect(screen.getByLabelText("Copy curl for getCompliance")).toBeInTheDocument(); + }); + + it("renders an honest empty state for a document with no operations", () => { + render(); + expect(screen.getByText("This OpenAPI document describes no operations.")).toBeInTheDocument(); + }); +}); + +describe("typeLabel", () => { + it("names a $ref, a union and a formatted primitive", () => { + expect(typeLabel({ $ref: "#/components/schemas/Answer" })).toBe("Answer"); + expect(typeLabel({ type: ["string", "null"] })).toBe("string | null"); + expect(typeLabel({ type: "string", format: "date" })).toBe("string (date)"); + expect(typeLabel({ enum: ["a"] })).toBe("enum"); + expect(typeLabel(undefined)).toBe(""); + }); +}); diff --git a/frontend/components/api-docs/api-reference.tsx b/frontend/components/api-docs/api-reference.tsx new file mode 100644 index 00000000..0d25b8b2 --- /dev/null +++ b/frontend/components/api-docs/api-reference.tsx @@ -0,0 +1,262 @@ +"use client"; + +/** + * The API reference page's renderer (ADR-068) — an OpenAPI 3.1 document as a readable page. + * + * ## Why this is hand-rolled + * + * Swagger UI's React integration is unusable here: `swagger-ui-react` peers on `react@">=16.8 <19"` and this + * app is on React 19, so the only route would be injecting `swagger-ui-dist` by hand — 1.7 MB of vendored + * JS, a stylesheet whose class names are internal and unversioned, and a dark mode implemented as a + * hard-coded `html.dark-mode` class that would fight this app for ownership of ``. Scalar and Redoc + * both default to a CDN script, which the CSP and the offline demo rule out, and self-hosting either means + * vendoring a megabyte-plus bundle for seven operations. + * + * So: no dependency, no vendored asset, and the page inherits the Enterprise Health brand and dark mode it + * already has (ADR-004). The trade is that there is no "try it out" console — a reader gets a copyable + * `curl` instead, which for a bearer-token API is roughly as useful and does not need a proxy. + * + * Schemas render as a flat indented property list rather than an expanding tree, because two levels is all + * this document has. + */ +import { useState } from "react"; +import { Badge } from "@mieweb/ui"; +import { endpointsOf, resolve, typeLabel, type Endpoint, type OpenApiDoc, type Schema } from "./types"; + +const METHOD_TONE: Record = { + GET: "bg-emerald-500/10 text-emerald-700 dark:text-emerald-300 ring-emerald-500/30", + POST: "bg-sky-500/10 text-sky-700 dark:text-sky-300 ring-sky-500/30", +}; + +function MethodBadge({ method }: { method: string }) { + const tone = METHOD_TONE[method] ?? "bg-neutral-500/10 text-neutral-700 dark:text-neutral-300 ring-neutral-500/30"; + return ( + + {method} + + ); +} + +function CopyButton({ text, label }: { text: string; label: string }) { + const [copied, setCopied] = useState(false); + return ( + + ); +} + +/** One row per property, indented by depth. `$ref` is followed one level; deeper nesting is summarised. */ +function SchemaRows({ schema, doc, depth = 0 }: { schema: Schema | undefined; doc: OpenApiDoc; depth?: number }) { + const resolved = resolve(schema, doc); + if (!resolved) return null; + const props = resolved.properties ?? resolve(resolved.items, doc)?.properties; + const required = new Set(resolved.required ?? resolve(resolved.items, doc)?.required ?? []); + if (!props) return null; + + return ( + <> + {Object.entries(props).map(([name, prop]) => { + const child = resolve(prop, doc); + const nested = depth < 2 && (child?.properties || resolve(child?.items, doc)?.properties); + return ( +
+
+ {name} + {typeLabel(prop)} + {required.has(name) && required} + {(prop.enum ?? child?.enum) && ( + {(prop.enum ?? child?.enum)!.join(" | ")} + )} + {(prop.description ?? child?.description) && ( + + {prop.description ?? child?.description} + + )} +
+ {nested ? : null} +
+ ); + })} + + ); +} + +function curlFor(e: Endpoint, doc: OpenApiDoc, origin: string): string { + const path = (e.op.parameters ?? []) + .filter((p) => p.in === "path") + .reduce((acc, p) => acc.replace(`{${p.name}}`, String(p.schema?.example ?? `<${p.name}>`)), e.path); + const authed = (e.op.security ?? []).length > 0; + const lines = [`curl -sS${e.method === "GET" ? "" : ` -X ${e.method}`} ${origin}${path}`]; + if (authed) lines.push(` -H 'authorization: Bearer '`); + const bodySchema = resolve(e.op.requestBody?.content?.["application/json"]?.schema, doc); + if (bodySchema) { + lines.push(` -H 'content-type: application/json'`); + lines.push(` -d '${JSON.stringify(exampleOf(bodySchema, doc))}'`); + } + return lines.join(" \\\n"); +} + +/** A minimal example object from a schema — required properties only, so the curl stays short and valid. */ +function exampleOf(schema: Schema | undefined, doc: OpenApiDoc, depth = 0): unknown { + const s = resolve(schema, doc); + if (!s || depth > 3) return {}; + if (s.example !== undefined) return s.example; + const types = Array.isArray(s.type) ? s.type : s.type ? [s.type] : []; + if (types.includes("array")) return [exampleOf(s.items, doc, depth + 1)]; + if (types.includes("object") || s.properties) { + const out: Record = {}; + for (const name of s.required ?? []) out[name] = exampleOf(s.properties?.[name], doc, depth + 1); + return out; + } + if (s.enum?.length) return s.enum[0]; + if (types.includes("boolean")) return false; + return s.example ?? "string"; +} + +function OperationCard({ e, doc, origin }: { e: Endpoint; doc: OpenApiDoc; origin: string }) { + const params = e.op.parameters ?? []; + const authed = (e.op.security ?? []).length > 0; + const curl = curlFor(e, doc, origin); + const success = Object.entries(e.op.responses ?? {}).find(([code]) => code.startsWith("2")); + const successSchema = success?.[1]?.content?.["application/json"]?.schema; + const bodySchema = e.op.requestBody?.content?.["application/json"]?.schema; + + return ( +
+
+ + {e.path} + {authed ? bearer token : public} +
+

{e.op.summary}

+ {e.op.description && ( +

{e.op.description}

+ )} + + {params.length > 0 && ( +
+

Parameters

+
+ {params.map((p) => ( +
+ {p.name} + {p.in} + {typeLabel(p.schema)} + {p.required && required} + {p.schema?.enum && {p.schema.enum.join(" | ")}} + {p.description && {p.description}} +
+ ))} +
+
+ )} + + {bodySchema && ( +
+

Request body

+
+
+ )} + + {successSchema && ( +
+

+ Response {success![0]} — {typeLabel(successSchema)} +

+
+
+ )} + + {Object.keys(e.op.responses ?? {}).length > 1 && ( +
+

Status codes

+
+ {Object.entries(e.op.responses ?? {}).map(([code, r]) => ( +
+ {code} + {r.description} +
+ ))} +
+
+ )} + +
+
+

Example

+ +
+ 
{curl}
+
+
+ ); +} + +export function ApiReference({ doc, origin }: { doc: OpenApiDoc; origin: string }) { + const endpoints = endpointsOf(doc); + if (endpoints.length === 0) { + // An honest empty state, not a blank page: the difference between "no operations" and "the document + // could not be read" matters to whoever is looking at this. + return ( +

+ This OpenAPI document describes no operations. +

+ ); + } + const tags = doc.tags?.length ? doc.tags : [{ name: "", description: undefined }]; + + return ( +
+ {tags.map((tag) => { + const inTag = endpoints.filter((e) => (tag.name ? (e.op.tags ?? []).includes(tag.name) : true)); + if (inTag.length === 0) return null; + return ( +
+ {tag.name && ( +
+

{tag.name}

+ {tag.description &&

{tag.description}

} +
+ )} +
+ {inTag.map((e) => ( + + ))} +
+
+ ); + })} + {/* Operations that carry no declared tag would otherwise render nowhere — silently dropping an + operation from a reference page is the same defect class as a documented-but-unrouted path. */} + {(() => { + const tagged = new Set(tags.flatMap((t) => (t.name ? endpoints.filter((e) => (e.op.tags ?? []).includes(t.name)) : endpoints)).map((e) => e.op.operationId)); + const orphans = endpoints.filter((e) => !tagged.has(e.op.operationId)); + if (orphans.length === 0) return null; + return ( +
+

Other

+
+ {orphans.map((e) => ( + + ))} +
+
+ ); + })()} +
+ ); +} diff --git a/frontend/components/api-docs/types.ts b/frontend/components/api-docs/types.ts new file mode 100644 index 00000000..50d9243f --- /dev/null +++ b/frontend/components/api-docs/types.ts @@ -0,0 +1,83 @@ +/** + * The slice of OpenAPI 3.1 this reference page renders (ADR-068). + * + * Deliberately partial, and deliberately not shared with the backend: `backend-ts/src/openapi/spec.ts` is a + * different package, and a hand-copied "full" OpenAPI type model would be a large surface nothing here + * reads. Everything optional is optional because the renderer must degrade rather than crash on a document + * shape it does not recognise. + */ +export interface Schema { + /** 3.1 spells nullability as a union: `["string", "null"]`. */ + type?: string | string[]; + format?: string; + description?: string; + enum?: string[]; + properties?: Record; + required?: string[]; + items?: Schema; + additionalProperties?: boolean | Schema; + $ref?: string; + example?: unknown; +} + +export interface Parameter { + name: string; + in: string; + required?: boolean; + description?: string; + schema?: Schema; +} + +export interface Operation { + operationId: string; + summary: string; + description?: string; + tags?: string[]; + security?: Array>; + parameters?: Parameter[]; + requestBody?: { required?: boolean; content?: Record }; + responses?: Record }>; +} + +export interface OpenApiDoc { + openapi?: string; + info?: { title?: string; version?: string; summary?: string; description?: string }; + servers?: Array<{ url: string; description?: string }>; + tags?: Array<{ name: string; description?: string }>; + paths?: Record>; + components?: { schemas?: Record }; +} + +/** An operation plus where it lives, which the path item does not carry. */ +export interface Endpoint { + path: string; + method: string; + op: Operation; +} + +export function endpointsOf(doc: OpenApiDoc): Endpoint[] { + const out: Endpoint[] = []; + for (const [path, item] of Object.entries(doc.paths ?? {})) { + for (const [method, op] of Object.entries(item)) { + out.push({ path, method: method.toUpperCase(), op }); + } + } + return out; +} + +/** Resolve one level of `#/components/schemas/X`. Returns the input unchanged when it is not a `$ref`. */ +export function resolve(schema: Schema | undefined, doc: OpenApiDoc): Schema | undefined { + if (!schema?.$ref) return schema; + const name = schema.$ref.replace("#/components/schemas/", ""); + return doc.components?.schemas?.[name]; +} + +/** `"string"`, or `"string | null"` for a 3.1 union — what a reader wants to see. */ +export function typeLabel(schema: Schema | undefined): string { + if (!schema) return ""; + if (schema.$ref) return schema.$ref.replace("#/components/schemas/", ""); + const types = Array.isArray(schema.type) ? schema.type : schema.type ? [schema.type] : []; + if (types.length === 0) return schema.enum ? "enum" : "any"; + const base = types.join(" | "); + return schema.format ? `${base} (${schema.format})` : base; +} From 384324259e74cc747e545c66a870f0c7299e53a7 Mon Sep 17 00:00:00 2001 From: Taleef Date: Mon, 17 Aug 2026 12:13:15 -0400 Subject: [PATCH 4/7] docs: record ADR-067/068, add the CDS Hooks contract, and correct two stale ARCHITECTURE claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-067 (CDS Hooks cards render a completed evaluation and never trigger one) and ADR-068 (the OpenAPI document covers the promised surface only, guarded by a routed-path test), plus docs/CDS_HOOKS.md as the contract an integrator reads. Two ARCHITECTURE corrections, one of which was the point of the work: §9 asserted "The OpenAPI document (workwell.swagger.enabled=true) advertises version v1" -- a springdoc property from a backend retired in #109 PR4 -- while §7 did not mention /api/v1/compliance at all, so the file simultaneously claimed a document it did not serve and omitted the one contract it does. Writing the document also surfaced that §9 promised /api/version returns `uptime`; it does not. STANDARDS_CONFORMANCE gains two rows written through the conformance skill. Both name who graded what: CDS Hooks 2.0.1 is structurally conformant and SELF-graded, because no external grader exists (the community validator is JSON Schemas last pushed 2018, the sandbox is ungraded, Inferno has no CDS Hooks kit); and the gap-to-card leg is recorded as a LOCAL mapping, since HL7 blesses PlanDefinition/$apply -> RequestOrchestration -> cards but nobody publishes a route from a DEQM care gap to a card. Neither row is justified by certification: ONC (b)(11) and HTI-1 do not name CDS Hooks. AI_GUARDRAILS gains §1.1 -- a card is a rendering, every clinical statement in it is the CQL outcome verbatim, and systemActions is never emitted. S7 is rewritten with its per-part table split along the line that matters: delivering a finding, offering an order, and did-anyone-act move to built, while evaluating data supplied on the request stays not built -- that is step 2, and prefetch is where it would go. The sequence diagram draws that leg dashed. Also closes a real gitignore gap: docs/transcripts/ was only covered by basename globs, so `docs/transcripts/2026-08-16 call.md` was committable. Verified with git check-ignore. Backend suite 1970, 0 fail. Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 3 + CLAUDE.md | 2 +- docs/ADR_INDEX.md | 6 +- docs/AI_GUARDRAILS.md | 21 ++++ docs/ARCHITECTURE.md | 40 +++++-- docs/CDS_HOOKS.md | 197 ++++++++++++++++++++++++++++++++++ docs/COMPLIANCE_API.md | 5 + docs/DECISIONS.md | 127 ++++++++++++++++++++++ docs/DEPLOY.md | 3 +- docs/JOURNAL.md | 109 +++++++++++++++++-- docs/STANDARDS_CONFORMANCE.md | 2 + docs/guide/10-scenarios.md | 98 +++++++++++------ docs/guide/README.md | 4 +- 13 files changed, 568 insertions(+), 49 deletions(-) create mode 100644 docs/CDS_HOOKS.md diff --git a/.gitignore b/.gitignore index 105f5830..1838ccaf 100644 --- a/.gitignore +++ b/.gitignore @@ -56,6 +56,9 @@ docs/vision doc screenshots webchart-import/ # Local-only call transcripts (personal/personnel content - never commit, never reference in docs). +# The directory AND the filename globs, because the globs below match on the BASENAME: a file named +# `docs/transcripts/2026-08-16 call.md` matches none of them and was committable by accident. +docs/transcripts/ # Any depth, any of the formats a recorder/transcriber emits. **/*transcript*.txt **/*transcript*.md diff --git a/CLAUDE.md b/CLAUDE.md index 21b2ddec..b613ca6e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,7 +84,7 @@ to this list without deleting from it — the whole point is that it stays small backend retired in #109 PR4, and its stop condition died with the JVM. Now in `docs/archive/`.) - @docs/AI_GUARDRAILS.md — the "AI never decides compliance" hard rule lives or dies on this - @docs/DATA_MODEL_CONTRACTS.md — idempotency + `evidence_json` + CSV contracts; Definition of Done makes these mandatory on EVERY PR -- @docs/ADR_INDEX.md — 65 ADR titles only, so a session knows a decision exists; bodies stay in DECISIONS.md +- @docs/ADR_INDEX.md — 67 ADR titles only, so a session knows a decision exists; bodies stay in DECISIONS.md - @docs/LOCKED_DECISIONS.md — owner-locked decisions (§4, rewritten 2026-08-04 per ADR-058) + the dated 2026-07-24 audit facts (§5) ## Other docs to consult on demand diff --git a/docs/ADR_INDEX.md b/docs/ADR_INDEX.md index 21e212d2..3359c8f8 100644 --- a/docs/ADR_INDEX.md +++ b/docs/ADR_INDEX.md @@ -9,14 +9,16 @@ > If the highest number here is lower than the highest in `DECISIONS.md`, this file is stale: trust > `DECISIONS.md` and regenerate. -> **`·archived` (14 of 65, as of 2026-08-10)** means the BODY moved to +> **`·archived` (14 of 67, as of 2026-08-17)** means the BODY moved to > `docs/archive/DECISIONS_ARCHIVE.md` — it is either superseded or a historical *finding* rather than a > decision that governs. `DECISIONS.md` still carries its heading plus a one-line pointer, so every -> cross-reference resolves. The 51 unmarked titles are the record that still governs: decisions that +> cross-reference resolves. The 53 unmarked titles are the record that still governs: decisions that > constrain what may be done next, and design records for built features. ## Titles (newest first) +- ADR-068: the OpenAPI document covers the PROMISED surface only, and a routed-path test is what makes hand-authoring defensible +- ADR-067: CDS Hooks cards render a completed evaluation and never trigger one — and the outcome-to-card mapping is ours, which is stated rather than implied - ADR-066: the documentation splits into a maintained guide and a dated archive — because a doc that explains and a doc that records rot at different speeds - ADR-065: an authored regulatory measure is verified by traceability and adversarial cases — no external oracle exists, and none can be manufactured - ADR-064: one UCUM validator, shared by every translator we run — and an honest table rather than a new dependency diff --git a/docs/AI_GUARDRAILS.md b/docs/AI_GUARDRAILS.md index 2b735ebc..d59241d0 100644 --- a/docs/AI_GUARDRAILS.md +++ b/docs/AI_GUARDRAILS.md @@ -5,6 +5,27 @@ AI never decides compliance. Authoritative compliance state is computed by CQL evaluation (`Outcome Status`) and persisted structured evidence (`outcomes.evidence_json`). AI outputs are assistive text only. +### 1.1 CDS Hooks cards are a rendering, and carry nothing from an AI surface (ADR-067) + +The CDS Hooks service (`docs/CDS_HOOKS.md`) returns cards into someone else's clinical workflow, which makes +it the surface where the non-negotiable rule matters most. Three consequences, all enforced in code: + +- **Every clinical statement in a card is the CQL outcome verbatim** — the status, the display method and the + next-action line come from `deriveCell` / `deriveWhyFlagged` / `nextActionFor`, the same readers the roster + and case detail use. **No AI surface contributes to a card**, and none may: an `AiAssistService` explanation + is assistive text for an operator reading a case, not something to put in front of a clinician mid-encounter + as a finding. +- **`systemActions` is never emitted.** In CDS Hooks it is the array a client auto-applies with no user + interaction. Nothing WorkWell returns may change a chart without a human choosing it, which is the + human-in-the-loop contract of §7 applied to an outbound integration. +- **`critical` is never emitted**, and is unrepresentable in the card type. It means *the user must not + proceed*; WorkWell is supplementary to WebChart and is not entitled to say that about someone else's + encounter. + +A card `suggestion` is a *proposal* — a `ServiceRequest` with `intent=proposal`, `status=draft`, offered only +where the order code carries an APPROVED terminology mapping, and accepted only by a clinician's explicit +action. + ## 2) Active AI Surfaces and Prompt Templates All current prompts are implemented in `com.workwell.ai.AiAssistService`. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3dd5a3a6..a4cd7dc9 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -266,6 +266,25 @@ Public API actions derive audit identity from the authenticated security context - Frontend demo prefill is a local convenience only; `NEXT_PUBLIC_DEMO_MODE=true` fails the production frontend build. ## 7) External Interfaces + +**The integration surface — the versioned, documented contract (ADR-061/067/068).** Everything else in this +section is internal: it is logically v1 but moves with the frontend and carries no stability promise. These +four route groups are the ones an integrator builds against, and the only ones the OpenAPI document describes. +- `GET /api/v1/compliance/{subjectId}/{measureId}?start=&end=&mode=latest|preview` — one subject, one measure, + one stable answer, carrying `populationsSource` so a consumer can tell a measured population vector from + one inferred from status. `latest` requires a FINALIZED run and 404s rather than answering an absence; + `preview` is CM/ADMIN-only and 501s on a WebChart-configured stack rather than reporting a synthetic bundle + as an evaluation. Every answered request writes `COMPLIANCE_API_READ`. Contract: `docs/COMPLIANCE_API.md`. +- `GET /cds-services` (**public** — metadata only) and `POST /cds-services/{serviceId}` — a CDS Hooks 2.0.1 + service for the `patient-view` hook, returning cards built from persisted outcomes of a completed run. No + `prefetch` is declared because none is evaluated; `critical` and `systemActions` are never emitted; a + suggestion is offered only for an APPROVED terminology mapping; and a patient with no evaluation gets an + informational card, never an empty card list. Contract: `docs/CDS_HOOKS.md`. +- `POST /cds-services/{serviceId}/feedback` — accepted/overridden per card, audited. Needs no schema because + `card.uuid` derives from `(runId, subjectId, measureId)`. +- `GET /api/v1/openapi.json` (**public**) — the OpenAPI 3.1.1 document for the above, plus health and version. + Kept honest by a two-way coverage test and a pinned `redocly lint` in CI. + - Live WebChart population: configured `ALL_PROGRAMS`/`MEASURE` runs fetch in the background and fail the whole run before outcomes on a population-fetch error; FAILED runs are ignored by read models, so the prior successful population remains authoritative. The seam unset path is unchanged. `SITE=WebChart` and `wc|` CASE rerun-to-verify are phase-2 deferrals; CASE returns a non-mutating 409 rather than reusing a stale bundle or fabricating `MISSING_DATA`. - REST API: measure, run, case, admin, export, and auditor packet endpoints. - REST API: evidence upload/download on case detail, role-gated to case manager/admin. @@ -331,18 +350,27 @@ No microservice decomposition is used in MVP; package boundaries are the future ## 9) API Versioning Convention - The current API contract is **v1**. `GET /api/version` returns - `{"api":"v1","build":"","uptime":"s"}` - and is unauthenticated for health/discovery use. + `{"api":"v1","stack":"typescript","build":"workwell-api-ts"}` and is unauthenticated for + health/discovery use. *(Corrected 2026-08-17: this row claimed an `uptime` field for years. The worker + emits none — writing the OpenAPI document surfaced it, because the document describes what the route + actually returns and a test compares the two.)* - Existing endpoints remain under the unprefixed `/api/...` path for the MVP - demo; they are logically v1. Migrating controllers to an explicit `/api/v1/...` - prefix is intentionally deferred (Sprint 4 note) to avoid churn across - Actuator, Swagger, CORS, and the frontend client. + demo; they are logically v1. Migrating them to an explicit `/api/v1/...` prefix + is intentionally deferred to avoid churn across health/discovery, CORS, and the + frontend client. **`/api/v1/` is reserved for the deliberately versioned + integration surface** — see §7 — rather than being the eventual home of every route. - **Convention for future breaking changes:** when a response shape change cannot be made backward-compatible, introduce the new endpoint under `/api/v2/...` rather than mutating the v1 path. The old `/api/v1/...` (or current unprefixed) path must be retained for at least one minor version cycle so integrators have a migration window. Additive, backward-compatible changes stay on v1. -- The OpenAPI document (`workwell.swagger.enabled=true`) advertises version `v1`. +- **The OpenAPI document is `GET /api/v1/openapi.json`** — hand-authored OpenAPI **3.1.1**, unauthenticated, + covering the *promised* surface only (§7's "integration surface" block). Rendered for humans at the + frontend's public `/api-docs`. ADR-068. *(Corrected 2026-08-17: this row previously read "The OpenAPI + document (`workwell.swagger.enabled=true`) advertises version `v1`" — a springdoc property from the + Java/Spring backend retired in #109 PR4. No document was served at all; probes against production and + staging returned `501 not_implemented`. A routed-path test now fails if a documented path stops being + served, so this row cannot go stale the same way again.)* ## 10) Inert-seam inventory diff --git a/docs/CDS_HOOKS.md b/docs/CDS_HOOKS.md new file mode 100644 index 00000000..d66edbb0 --- /dev/null +++ b/docs/CDS_HOOKS.md @@ -0,0 +1,197 @@ +# WorkWell CDS Hooks service — `2.0.1` + +*This patient is in front of a clinician right now. What is outstanding?* + +WorkWell implements the [CDS Hooks](https://cds-hooks.hl7.org/2.0/) contract so a quality gap can arrive +inside someone else's workflow instead of on a dashboard they would have to visit. We serve the standard's +shapes; we do not redefine them. + +--- + +## Request + +``` +GET /cds-services → discovery (public) +POST /cds-services/workwell-compliance-patient-view → cards +POST /cds-services/workwell-compliance-patient-view/feedback → 200 +``` + +| | | | +|---|---|---| +| `serviceId` | path, required | An `id` from discovery. Today there is exactly one. An unknown id is a **404** that lists what exists. | +| `hook` | body, required | Must be `patient-view`. A hook this service does not serve is a **400**, not a guess. | +| `hookInstance` | body, required | A UUID for this invocation, per the specification. | +| `context.patientId` | body, required | The FHIR `Patient.id`. A bare WebChart id is also tried as `wc|` — see *Subject resolution*. | +| `context.userId` | body, optional | `Practitioner/abc` or `PractitionerRole/123`. Recorded, not used for gating. | +| `fhirServer`, `fhirAuthorization`, `prefetch` | body, optional | **Accepted and not evaluated.** See *Limits, stated*. | + +**Authentication:** discovery is public — it returns service metadata and no patient data. Invoke and +feedback require the standard bearer token with `ROLE_MCP_CLIENT`, `ROLE_CASE_MANAGER` or `ROLE_ADMIN`, the +same authority as `/sse` and `/mcp/**`. An anonymous invoke is **401**. + +> **This is not the CDS Hooks JWT profile, and that is a stated gap.** The specification defines its own +> scheme — a JWT the *client* signs (RS384/ES384), verified against a JWKS, with `aud` equal to the invoked +> endpoint URL and an allowlist of trusted `iss`/`jku` values — and it **SHALL NOT** be signed with a +> symmetric algorithm. WorkWell's token is HS256 and self-issued, so it can never be a conformant CDS Hooks +> JWT. The profile is deliberately not implemented: `jku` fetching is an SSRF surface by design, and a +> verifier whose allowlist nobody has populated is a control that reads as present and cannot fire. If a real +> CDS client appears, the two things needed from it are its `iss` and its JWKS URL. + +## Response + +A successful invoke is `200` with a `cards` array, per the specification. + +```jsonc +{ + "cards": [ + { + "summary": "Annual Audiogram Completed — overdue", // ≤ 140 chars, per the spec + "indicator": "warning", // info | warning — never `critical` + "source": { + "label": "WorkWell Measure Studio", + "url": "https://studio.example.org/measures/audiogram" + }, + "detail": "**Overdue — last 2025-03-10 (55d over).** Escalate audiogram follow-up immediately.\n\n- Last completed: 2025-03-10\n- Days overdue: 55\n- Compliance window: 365 days\n\n_Computed by CQL and evaluated 2026-06-12T03:00:11.482Z (WorkWell run 7f3a…)._", + "uuid": "3c1d…", // derived, stable — cite it in feedback + "links": [ + { "label": "Open in WorkWell", "url": "https://studio.example.org/compliance?subjectId=emp-006", "type": "absolute" } + ], + "suggestions": [ // only for an APPROVED order code — see below + { + "label": "Order Comprehensive audiometry evaluation", + "uuid": "9b02…", + "actions": [ + { + "type": "create", + "description": "Draft order proposal for Comprehensive audiometry evaluation (Annual Audiogram Completed). Advisory — a clinician reviews and submits; WorkWell orders nothing.", + "resource": { "resourceType": "ServiceRequest", "intent": "proposal", "status": "draft" } + } + ] + } + ], + "selectionBehavior": "at-most-one" + } + ] +} +``` + +### An empty `cards` array means something specific + +| response | meaning | +|---|---| +| `{"cards": []}` | This patient **was** evaluated by a completed run and has no open gap. | +| one `info` card, *"No WorkWell evaluation on record…"* | Nothing has been evaluated for this patient — including the case where the id did not resolve. | + +> **These are deliberately not the same answer.** An empty array at the point of care reads as "no gaps", +> which is the confusion ADR-061's 404 exists to prevent. Because a hook's `patientId` is a bare EHR id while +> WorkWell persists live subjects as `wc|`, a namespace mismatch would otherwise be +> indistinguishable from a clean bill of health. `412 Precondition Failed` is also not used: it means the +> service could not retrieve FHIR data, and this is a truthful "nothing has been evaluated yet". + +### `indicator` never reaches `critical` + +The specification allows `info | warning | critical`, and `critical` means *the user must not proceed*. +WorkWell is supplementary to WebChart and is not entitled to say that about someone else's encounter, so +`OVERDUE` maps to `warning` and everything else to `info`. This is enforced by the card type, not by +convention. + +### Subject resolution + +`wc|` is tried first, then the bare id. So a WebChart client sending `4821` reaches the subject +WorkWell stored as `wc|4821`, and a synthetic-roster client sending `emp-006` reaches `emp-006`. + +### Suggestions are gated on an APPROVED terminology mapping + +A suggestion is a one-click order into an EHR, and `order-catalog.ts` describes its codes as +"representative (demo, not billing-certified)". So one is offered only where the order code carries an +**`APPROVED`** mapping in `terminology_mappings`, read from the store — approving a mapping unlocks a +suggestion with no code change. + +| measure | order code | offered? | +|---|---|---| +| `audiogram` | CPT 92557 | yes — APPROVED | +| `tb_surveillance` | CPT 86580 | yes — APPROVED | +| `flu_vaccine` | CVX 141 | yes — APPROVED | +| `hazwoper` | internal `hazwoper-exam` | no — mapping is `REVIEWED`, and the code is not a public standard | +| **`cms122`, `cms125`** | CPT 83036 / 77067 | **no — no mapping exists at all** | + +> **The consequence is intended, not an oversight.** The two officially-routed CMS measures carry +> information and a link rather than an order. Offering a demo-grade CPT for creation in a certified EHR is +> the harm the rule exists to prevent; giving them a suggestion is a terminology review, not a code change. + +Two measures sharing one order code (`diabetes_hba1c` and `cms122` both map to CPT 83036) collapse to a +single suggestion, because one order is the correct clinical action. + +## Feedback + +`POST /cds-services/{serviceId}/feedback` reports what a clinician did with a card. Optional in the +specification; implemented here because it is the one leg of guide S7's send/receive reconciliation WorkWell +can build alone. + +```jsonc +{ + "feedback": [ + { + "card": "3c1d…", // card.uuid from the invoke response + "outcome": "accepted", // accepted | overridden — there is no `declined` + "acceptedSuggestions": [{ "id": "9b02…" }], // REQUIRED when outcome is `accepted` + "outcomeTimestamp": "2026-06-12T10:05:31Z" + } + ] +} +``` + +Each entry writes one `CDS_HOOKS_FEEDBACK_RECEIVED` audit event. Nothing else is persisted, and nothing else +needed to be: `card.uuid` is derived from `(runId, subjectId, measureId)`, so correlating a uuid back to a +measure is a recomputation over that subject's outcomes rather than a lookup — which is why this endpoint +needs no schema change. The `CDS_HOOKS_INVOKED` event records the uuids it emitted, so the join runs from the +ledger. Deterministic ids also mean a client re-firing the hook for an unchanged run gets the same uuid, so +repeat feedback does not fragment across ids. + +## Errors + +| status | `error` | when | +|---|---|---| +| 400 | `invalid_request` | body is not a JSON object; missing `hook`/`hookInstance`/`context.patientId`; a hook this service does not serve; empty `feedback`; an `outcome` other than `accepted`/`overridden`; `accepted` without `acceptedSuggestions` | +| 401 | `unauthenticated` | no bearer token on invoke or feedback | +| 403 | `forbidden` | authenticated, but the role may not invoke a CDS service | +| 404 | `unknown_service` | unknown `serviceId`; the response lists the ids that exist | +| 405 | `method_not_allowed` | non-GET on discovery, non-POST on invoke or feedback | + +## What this promises + +**Stable:** the paths above, the CDS Hooks 2.0.1 response shape, the `indicator` ceiling of `warning`, the +distinction between an empty card list and an informational card, and the derivation of `card.uuid` from +`(runId, subjectId, measureId)`. + +**Not stable:** card `summary` and `detail` wording, which measures produce cards, and the set of order codes +eligible for a suggestion — the last of these moves when a terminology mapping is approved, by design. + +## Limits, stated + +- **`prefetch`, `fhirServer` and `fhirAuthorization` are accepted and NOT evaluated**, and the service says + so in its own `usageRequirements`. No prefetch template is declared, because declaring one would make a + client fetch and transmit data we ignore. Honouring it means evaluating a caller-supplied bundle per + request — a different capability, and the piece guide S7 still calls not built. +- **Cards are as fresh as the last completed run, not as fresh as this encounter.** They render persisted + outcomes of a FINALIZED run; a mid-run row is never served. +- **One hook.** `patient-view` is maturity 5 in the CDS Hooks Library IG; `encounter-start` is maturity 1 and + would return the same cards. +- **`systemActions` is never emitted.** Nothing WorkWell returns may change a chart without a human choosing + it (see `docs/AI_GUARDRAILS.md`). +- **No `overrideReasons`.** Offering a coded dismissal vocabulary we do not analyse would be decoration. +- **No tenant or site scoping** — a caller with a token may ask about any subject. A known posture, on the + production-readiness gap list (#269). +- **CORS is an exact-origin allowlist and is not relaxed.** A browser-based CDS client (including the public + sandbox at `sandbox.cds-hooks.org`) needs its origin added to `WORKWELL_CORS_ALLOWED_ORIGINS`. The + specification requires CORS support but explicitly declines to specify an allowlist rule. +- **Nothing in WebChart fires this hook today.** Whether WebChart acts as a CDS Hooks client is an open + question with MIE, and there is no public evidence either way. +- **Conformance is self-graded.** No external CDS Hooks conformance suite exists — the community validator is + JSON Schemas last touched in 2018, the sandbox is ungraded, and Inferno has no CDS Hooks kit. See + `docs/STANDARDS_CONFORMANCE.md`. +- **Compliance is computed by CQL and only by CQL** (ADR-008). A card is a rendering of a completed + evaluation, never a decision. + +*Implemented in `backend-ts/src/routes/cds-hooks.ts` and `backend-ts/src/cds/` · ADR-067 · guide +[S7](guide/10-scenarios.md) · proposal P1 (#458).* diff --git a/docs/COMPLIANCE_API.md b/docs/COMPLIANCE_API.md index 3229b1a1..aeb7029a 100644 --- a/docs/COMPLIANCE_API.md +++ b/docs/COMPLIANCE_API.md @@ -5,6 +5,11 @@ One subject, one measure, one stable answer. This is the contract an integrator builds against; everything else under `/api/` is internal and moves with the frontend. +> **Machine-readable and browsable.** This endpoint is described in the OpenAPI 3.1 document at +> `GET /api/v1/openapi.json`, rendered for humans at the frontend's public `/api-docs` (ADR-068). For the +> same answer delivered *into* a clinician's workflow rather than pulled per measure, see +> [`CDS_HOOKS.md`](CDS_HOOKS.md) (ADR-067). + --- ## Request diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index c7d820ac..9fc4f33f 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -18,6 +18,133 @@ > > **Sequence note:** ADR-033 does not exist — verified absent, and the number must not be reused. +## ADR-068: the OpenAPI document covers the PROMISED surface only, and a routed-path test is what makes hand-authoring defensible + +**Status:** Accepted (2026-08-17). Closes the tracked TODO at the top of `docs/JOURNAL.md`. + +**Context.** Doug asked whether WorkWell has a Swagger API. It did not: authenticated probes against +production and staging returned `501 not_implemented` from `/api/openapi.json`, `/api/swagger`, +`/swagger-ui` and `/api/docs`, while `ARCHITECTURE.md` §9 asserted *"The OpenAPI document +(`workwell.swagger.enabled=true`) advertises version `v1`"* — a springdoc property belonging to the Java +backend retired in #109 PR4. §7 did not mention `/api/v1/compliance` at all. So the repository was +simultaneously claiming a document it did not serve and omitting the one contract it does. + +**Decision.** + +1. **Serve a hand-authored OpenAPI 3.1.1 document at one canonical path**, `GET /api/v1/openapi.json`, + built by `backend-ts/src/openapi/spec.ts`. PERMIT: reading a contract should not require credentials, and + the document carries shapes and role names, not patient data. +2. **Scope is the PROMISED surface** — `/api/v1/compliance`, the three `/cds-services` operations, health + and version — and the document *says so*. The ~40 internal `/api/**` routes are excluded because + `COMPLIANCE_API.md` already draws that line ("everything else under `/api/` is internal and moves with + the frontend"), and documenting them would advertise stability over paths that carry none. +3. **Hand-authored, guarded by a contract test.** The alternatives each cost a dependency or a rewrite: zod + earns its keep only as a *runtime* validator, `@hono/zod-openapi` presupposes Hono and a router we do not + have, and TypeSpec would add a second hand-maintained source of truth with no coupling to a hand-rolled + dispatcher. The recognised risk of hand-authoring is drift and the recognised answer is a contract test, + so the test is treated as the other half of this decision rather than as optional garnish. +4. **The guard is two-way coverage.** Every `(path, method, status)` the document declares is produced by a + real request through the real worker, and every response the tests observe is declared. A documented path + that is not routed fails with `documented but NOT ROUTED`; an undocumented status fails the other + direction. Mutation-checked three ways — deleting the route, removing a produced status, and requiring an + absent property each fail the intended assertion. +5. **Redocly lints the document in CI**, pinned exactly, telemetry off, with **no ignore file**, because it + catches a class the contract test cannot. It did so immediately: five uses of `nullable`, which OpenAPI + 3.1 removed in favour of type unions. The five remaining warnings are explained in `spec.ts` rather than + silenced. +6. **3.1.1, not 3.2.** Renderer support for 3.2 is worse than absent, it is *silent* — Redoc 2.5.3 accepts a + 3.2 document by aliasing it to 3.1, so 3.2-only constructs are ignored rather than flagged, and Spectral + caps at 3.1. 3.1's Schema Objects are literal JSON Schema 2020-12, which is what makes a zero-dependency + response check tractable. +7. **The reference page is hand-rolled and public** (`frontend/app/api-docs`). Not only on the + no-new-dependency rule: `swagger-ui-react` peers on `react@">=16.8 <19"` and this app is on React 19, so + the React integration does not exist for us; Swagger UI's dark mode is a hard-coded `html.dark-mode` class + that would contest ownership of ``; and Scalar and Redoc both default to a CDN script the CSP and + the offline demo rule out. The trade is no try-it-out console — a copyable `curl` instead. + +**Consequences.** A 405 is **not representable** under a `get` operation, so the four unauthenticated GETs +keep an `operation-4xx-response` warning rather than mis-modelling their path-level 405 — the coverage test +refused the mis-modelling on the first attempt, which is the guard working. Adding a route to the promised +surface now means adding it to the document, because CI fails otherwise. Writing the document also exposed +a second stale ARCHITECTURE claim (§9 said `/api/version` returns `uptime`; it does not), corrected here. + +--- + +## ADR-067: CDS Hooks cards render a completed evaluation and never trigger one — and the outcome-to-card mapping is ours, which is stated rather than implied + +**Status:** Accepted (2026-08-17). Implements the 2026-08-14 decision that CDS Hooks is adopted as a +*specification* (ADR-008 stands; `cqf-fhir-cr` does not enter the runtime). Partially delivers proposal P1 +(#458). + +**Context.** WorkWell did not alert providers at all: verified by search, there was no CDS Hooks +implementation, no `PlanDefinition`, no `$apply`, and no hook fired anywhere — the alerts existed only on +WorkWell's own screens, which is the gap guide S7 describes. CDS Hooks is a JSON request/response contract +over HTTPS, so serving it requires no Java: two routes on the worker that already exists. Nicole's concern +was reinventing wheels, and adopting the community standard is the direct answer; adopting its *reference +implementation* at runtime would have replaced a working engine with a second one. + +**Decision.** + +1. **One service, `patient-view`.** In the separately versioned CDS Hooks Library IG (v1.0.1, 2025-03-12) + it carries maturity 5; `encounter-start`, the other hook that fits an in-encounter check, is at maturity + 1 and would return the same cards from the same outcomes. A second hook is a discovery entry and a + validation branch when a client asks for one. +2. **Cards render persisted outcomes of a FINALIZED run.** Never a preview, never a synthetic bundle. This + is why no `501` twin of ADR-061's `preview_unavailable` is needed — the answer is truthful on any stack — + and it answers P1's latency question by not incurring it. The cost, stated: a card is as fresh as the + last run, not as fresh as this encounter. +3. **No `prefetch` is declared, because none is evaluated**, and `usageRequirements` — the spec's own field + for telling a caller what it must know — says so in the machine-readable contract. `fhirServer`, + `fhirAuthorization` and `prefetch` are accepted and ignored. Declaring a template we would ignore would + make a client fetch and transmit data for nothing; *honouring* it means evaluating a caller-supplied + bundle per request, which is a different capability and is where S7's step 2 would land. +4. **An absence is a CARD, not an empty list.** A patient with no finalized outcome — including one whose id + did not resolve — gets one `info` card saying so. `{"cards": []}` at the point of care reads as "no + gaps", the confusion ADR-061's 404 exists to prevent, and because a hook's `context.patientId` is a bare + EHR id while WorkWell persists live subjects as `wc|`, a namespace mismatch would otherwise be + indistinguishable from a clean bill of health. Silence is reserved for a subject we *did* evaluate and + found compliant. Not `412`, which means a failure to retrieve FHIR data. +5. **`critical` is never emitted, and is unrepresentable in the card type.** In CDS Hooks it means the user + must not proceed; WorkWell is supplementary to WebChart (locked decision 1) and is not entitled to say + that about someone else's encounter. `systemActions` is likewise never emitted — nothing WorkWell returns + may change a chart without a human choosing it. +6. **A suggestion is offered only where the order code carries an APPROVED terminology mapping**, read from + the store rather than the seed array so that approving a mapping unlocks it without a code change. + `order-catalog.ts` describes its codes as "representative (demo, not billing-certified)", and a CDS + suggestion is a one-click order into a certified EHR. **The consequence is deliberate: `cms122` and + `cms125` get no suggestion**, because their CPT codes have no mapping at all — so the two + officially-routed CMS measures carry information and a link. Getting them a suggestion is a terminology + review, not a code change. +7. **The measure-outcome-to-card mapping is OURS.** HL7's blessed route is `PlanDefinition/$apply` → + `RequestOrchestration` → cards; DEQM's `$care-gaps` stops at a `DetectedIssue`, and **no published mapping + carries a care gap into a card**. We map `outcomes.evidence_json` directly, reusing the readers the roster + and case detail already use, and `STANDARDS_CONFORMANCE.md` records that the gap-to-card leg is local. +8. **Discovery is public; invoke and feedback are not.** `/cds-services` matches no `/api/**` rule and + `authorize` ends in permitAll for non-`/api` paths, so the two rules are mandatory rather than a + refinement — asserted both as a unit call and end-to-end through the worker, and both assertions fail when + either rule is removed. Invoke reuses the `/sse` and `/mcp/**` authority (`ROLE_MCP_CLIENT` / + `CASE_MANAGER` / `ADMIN`) rather than inventing a role, since the user directory stays hardcoded. +9. **Authentication is WorkWell's bearer token, and the spec's JWT profile is a NAMED GAP.** CDS Hooks + defines its own scheme — a JWT the *client* signs, verified against a JWKS, with `aud` equal to the + invoked endpoint and an `iss`/`jku` allowlist — and it **SHALL NOT** be signed with a symmetric algorithm, + so our HS256 token can never be a conformant CDS Hooks JWT. The profile is not built: `jku` fetching is + SSRF-by-design, and a verifier whose allowlist nobody has populated is a control that reads as present + and cannot fire. This becomes a precise question for MIE — *does WebChart act as a CDS Hooks client, and + if so what are its `iss` and JWKS URL?* +10. **The feedback endpoint is built, with no schema change.** `card.uuid` and `suggestion.uuid` derive from + `(runId, subjectId, measureId)`, so correlating feedback is a recomputation over the subject's own + outcomes rather than a lookup in a table — and schema is the owner's alone. Deterministic ids also mean a + client re-firing the hook for an unchanged run gets the same uuid, so repeat feedback does not fragment. + The handler records the uuid verbatim and asserts nothing about what it referred to. + +**Consequences.** WorkWell can now be pointed at by any conformant CDS client, which changes the joint-call +question from "should we do this?" to "does WebChart speak it?". CORS is **not** relaxed: production keeps its +exact-origin allowlist, so a browser-based client's origin must be added deliberately — the spec requires CORS +support but explicitly declines to specify an allowlist rule. Nothing here is justified by certification: ONC's +(b)(11) DSI criterion does not name CDS Hooks. + +--- + ## ADR-066: the documentation splits into a maintained guide and a dated archive — because a doc that explains and a doc that records rot at different speeds **Status:** Accepted (2026-08-10). Implements the owner directive to trim the documentation and diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index 793faf1c..6db845be 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -873,7 +873,8 @@ shows all services `Up`). | `WORKWELL_AUTH_JWT_SECRET` | Backend | Required when auth is enabled; use a strong secret | | `WORKWELL_AUTH_COOKIE_SAME_SITE` | Backend | Refresh-cookie SameSite. **Must be `None` in production** (split frontend/API origins). Default `Lax` for local same-origin dev. | | `WORKWELL_AUTH_COOKIE_SECURE` | Backend | Refresh-cookie Secure flag. **Must be `true` in production** (required for SameSite=None). Default `false` for local HTTP dev. | -| `NEXT_PUBLIC_API_BASE_URL` | Frontend | Backend URL for fetch calls (origin-only, no `/api` suffix, no trailing whitespace) | +| `WORKWELL_CORS_ALLOWED_ORIGINS` | Backend | Comma-list of **exact** origins. Production refuses a wildcard, a blank entry, a non-URL and `localhost` (`config/startup-safety.ts`), and an unsafe value makes every route answer **503 `unsafe_configuration`** rather than degrading. Two consumers beyond the SPA: the **first origin is used as the Studio link target in CDS Hooks cards** (ADR-067) — definitionally the frontend, so no new variable — and a **browser-based CDS Hooks client must have its origin added here** before it can invoke the service. The CDS Hooks specification requires CORS support but explicitly declines to specify an allowlist rule, so this stays a deliberate, reviewed addition per client; `sandbox.cds-hooks.org` is not pre-allowed. A server-side CDS client needs nothing here. | +| `NEXT_PUBLIC_API_BASE_URL` | Frontend | Backend URL for fetch calls (origin-only, no `/api` suffix, no trailing whitespace). Also what the public `/api-docs` page fetches `/api/v1/openapi.json` from, so an unset value leaves the API reference showing its unreachable-backend state. | | `NEXT_PUBLIC_APP_NAME` | Frontend | App display name | | `NEXT_PUBLIC_DEMO_MODE` | Frontend | Prefill login form for local/demo builds only; `true` **fails the production frontend build** | | `WORKWELL_EMAIL_PROVIDER` | Backend | Outreach email provider. **Stays `simulated` on the demo stack (default + CLAUDE.md hard rule).** | diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 7a16d28d..82cae72b 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -1,15 +1,112 @@ # Journal +## 2026-08-17 — the integration surface becomes real: a CDS Hooks service, and the OpenAPI document that was claimed for a year + +Both of the 2026-08-14 asks are now built, on one branch, because they are the same theme: a CDS Hooks +discovery endpoint is itself an API that wants documenting alongside `/api/v1/compliance`. **ADR-067** +(CDS Hooks) and **ADR-068** (OpenAPI). The TODO that stood at the top of this file is discharged. + +**What ships.** `GET /cds-services` (public), `POST /cds-services/{id}` and `POST .../{id}/feedback` +(bearer-gated) — a CDS Hooks **2.0.1** service for the `patient-view` hook, returning cards built from the +most recent **finalized** run. And `GET /api/v1/openapi.json`, a hand-authored OpenAPI **3.1.1** document +for the promised surface only, rendered for humans at the frontend's public `/api-docs`. Suite 1964 → 1969 +plus 6 frontend tests; 0 fail; `redocly lint` 0 errors. + +**Research corrected me three times before any code was written, and each correction changed the design.** +(1) **The auth model is not RFC 7523.** CDS Hooks defines a bespoke signed-JWT bearer scheme (RFC +7515/7517/7518): no `sub`, no token endpoint, `aud` equal to the invoked endpoint URL, an `iss`/`jku` +allowlist — and it **SHALL NOT** be signed with a symmetric algorithm. Our HS256 token therefore can never be +a conformant CDS Hooks JWT, so the profile is a *named gap* rather than something to claim. It is +deliberately not built: `jku` fetching is SSRF-by-design, and a verifier whose allowlist nobody has +populated is a control that reads as present and cannot fire. (2) **No graded conformance ruler exists** — +`cds-hooks/cds-validator` is JSON Schemas last pushed 2018-02-05, the sandbox emits no score, and Inferno has +no CDS Hooks kit (only the CRD-scoped one). So the claim is ADR-065's shape: conforms structurally, +self-graded, unverified by any external suite. (3) **The measure-outcome→card mapping is ours.** HL7's +blessed route is `PlanDefinition/$apply` → `RequestOrchestration` → cards, and CQF Ruler and AHRQ's CQL +Services both do CQL→cards that way — but **DEQM's `$care-gaps` stops at a `DetectedIssue` and nobody +publishes the gap→card bridge.** Recorded as a local mapping in `STANDARDS_CONFORMANCE.md` rather than cited +to an IG. + +**Three refusals are the substance of the CDS work, and each is mutation-checked.** `critical` is never +emitted and is *unrepresentable in the card type* — it means "the user must not proceed", which WorkWell is +not entitled to say about someone else's encounter (locked decision 1); `systemActions` likewise. A +suggestion is offered only where the order code carries an **APPROVED** terminology mapping read from the +store, so **cms122 and cms125 get no suggestion** — their CPT codes have no mapping at all, and +`order-catalog.ts` calls its codes "representative (demo, not billing-certified)". That consequence is +stated up front in three places rather than discovered at a demo; unlocking it is a terminology review, not a +code change. And **an absence is a CARD, not an empty list**: a patient with no finalized outcome gets one +informational card, because `{"cards":[]}` at the point of care reads as "no gaps" — the confusion ADR-061's +404 exists to prevent, and the one that would have hidden the `wc|` namespace trap +`PROPOSALS_2026-08.md` §P1 names. + +**The feedback endpoint exists with no schema change**, which is why it was worth building at all: `card.uuid` +derives from `(runId, subjectId, measureId)`, so correlating a uuid is a recomputation over the subject's own +outcomes rather than a lookup in a table only the owner may create. Deterministic ids also mean a client +re-firing the hook for an unchanged run gets the same uuid, so repeat feedback does not fragment. + +**The security-relevant part is small and was nearly invisible.** `/cds-services` is outside `/api/`, and +`authorize` ends in `return { ok: true }` for non-`/api` paths — permitAll, mirroring Spring's +`anyRequest().permitAll()`. Without an explicit rule the invoke endpoint would have served per-patient +clinical status to anonymous callers. Two rules added, order load-bearing (`/cds-services/**` also matches the +bare path), asserted both as a unit call and end-to-end through the worker; deleting either rule fails both +assertions. + +**On the OpenAPI side the guard found things immediately, which is the point.** The document is hand-authored +— zod earns its keep only as a runtime validator, `@hono/zod-openapi` presupposes a router we do not have, and +TypeSpec would add a second hand-maintained source of truth with no coupling to a hand-rolled dispatcher — so +the contract test is treated as the other half of the decision, not garnish. It asserts **two-way coverage**: +every documented `(path, method, status)` produced by a real request through the real worker, and every +observed response documented. Mutation-checked three ways. Then `redocly lint`, which catches a class the +contract test cannot, rejected **five uses of `nullable`** — a 3.0 keyword OpenAPI 3.1 removed in favour of +type unions. Two different guards, neither implying the other: the same lesson as the CVU+ XSD/Schematron +episode, where a check's scope was narrower than the claim it was cited for. + +**And the coverage test refused a mis-modelling of mine.** I documented a `405` under the `get` operations of +the GET-only paths to silence a Redocly `operation-4xx-response` warning. A 405 belongs to the *path*, not to +the GET, so the document would have been asserting that a GET returns 405 — false. The test failed, the 405s +came out, and four warnings remain with a written reason instead. No ignore file. + +**Two stale ARCHITECTURE claims are corrected, one of which I did not go looking for.** §9's +"The OpenAPI document (`workwell.swagger.enabled=true`) advertises version `v1`" was a springdoc property +belonging to a backend retired in #109 PR4 — and §7 did not mention `/api/v1/compliance` at all, so the file +was simultaneously claiming a document it did not serve and omitting the one contract it does. Writing the +document also surfaced that §9 promised `/api/version` returns `uptime`; it does not, and never did here. + +**S7 is rewritten** and its per-part table now splits along the line that matters: *delivering a finding into +a workflow*, *offering follow-up as an order*, and *did anyone act on it* move to **built**, while +**evaluating data supplied on the request** stays not built — that is step 2, and `prefetch` is exactly where +it would go. The service declares none *because* it evaluates none, which is the same refusal as ADR-061's +`mode=preview` 501, in a different place. The sequence diagram now draws that missing leg as a dashed arrow. + +**Also: `docs/transcripts/` is now gitignored.** Only basename globs (`**/*transcript*.md`) matched before, so +`docs/transcripts/2026-08-16 call.md` was committable by accident. Verified with `git check-ignore`. + +**Owner steps this creates.** Two, both cheap and both for other people. **MIE's CHPL entry could not be +read** — chpl.healthit.gov is an SPA and its API is key-gated — so whether WebChart holds §170.315(b)(11) is +a manual lookup; note that (b)(11) and HTI-1 **do not name CDS Hooks**, so nothing here is justified by +certification. And the joint call now has a precise question instead of an open one: **does WebChart act as a +CDS Hooks client, and if so what are its `iss` and JWKS URL?** There is no public evidence either way — zero +hits for CDS Hooks across both MIE docs sites, and their FHIR API is documented as new with essentially no +adoption. + +**Not done, deliberately.** No `encounter-start` service (maturity 1 in the CDS Hooks Library IG, and it would +return the same cards); no `prefetch` evaluation; no OpenAPI path aliases beyond the canonical one; no +try-it-out console on `/api-docs`, since `swagger-ui-react` peers on `react@">=16.8 <19"` and this app is on +React 19 — a copyable `curl` instead. + +--- + ## 2026-08-14 — M-E1 leads with immunization, and CDS Hooks turns out to be a specification rather than a dependency Two owner decisions, taken off the back of the questions in the entry below. -**TODO — publish the WorkWell API contract as OpenAPI and serve Swagger UI.** The TypeScript backend is -live, but authenticated probes against both production and staging confirm that `/api/openapi.json`, -`/api/swagger`, `/swagger-ui`, and `/api/docs` all return `501 not_implemented`. The Swagger UI at -`manager.os.mieweb.org/api` documents MIE Create-a-Container, not WorkWell. Add a versioned OpenAPI -document for the WorkWell integration surface (beginning with `/api/v1/compliance`), expose a Swagger UI, -test both routes in CI/deployment smoke checks, and correct the stale OpenAPI claim in `ARCHITECTURE.md`. +**TODO — publish the WorkWell API contract as OpenAPI and serve Swagger UI.** *(DONE 2026-08-17 — ADR-068; +see the entry above. Kept here because the probe result it records is the reason the work happened.)* The +TypeScript backend is live, but authenticated probes against both production and staging confirm that +`/api/openapi.json`, `/api/swagger`, `/swagger-ui`, and `/api/docs` all return `501 not_implemented`. The +Swagger UI at `manager.os.mieweb.org/api` documents MIE Create-a-Container, not WorkWell. Add a versioned +OpenAPI document for the WorkWell integration surface (beginning with `/api/v1/compliance`), expose a Swagger +UI, test both routes in CI/deployment smoke checks, and correct the stale OpenAPI claim in `ARCHITECTURE.md`. **Decision 1 — M-E1's first content pack is immunization, not OSHA.** Three reasons, in order of weight. **A written specification already exists**: WebChart's immunization surveillance system diff --git a/docs/STANDARDS_CONFORMANCE.md b/docs/STANDARDS_CONFORMANCE.md index 6d0c51db..691c5201 100644 --- a/docs/STANDARDS_CONFORMANCE.md +++ b/docs/STANDARDS_CONFORMANCE.md @@ -22,6 +22,8 @@ What WorkWell emits across the eCQM toolchain, and the conformance level of each | MeasureReport structure (base) | FHIR R4 `MeasureReport` | `GET /api/runs/{id}/measure-report` (summary + individual + Bundle) | **VALIDATOR-VERIFIED: 0 errors** against the HL7 FHIR validator, base R4 (2026-08-04) | Measured by `backend-ts/scripts/deqm-validate.ts` over **four** report shapes built by the REAL production builders — official cms125, official cms122 (inverse), authored audiogram, and an individual. Validator jar sha256 `fc663ae5…`, FHIR 4.0.1, Java 21. **This retires the "structural (not HL7-validator)" caveat that stood on the row above**, which has been corrected in place. Scope limit, stated: four hand-constructed reports, **not** a sweep of live endpoint responses — the same limit `qrda-schematron-check.py` states about itself. One base-R4 **warning** is outside the 0: `measureScore.value` is emitted at full float precision (`0.019417475728155338`) where `qrda3-export.ts` formats the same quantity `.toFixed(4)`, so the two exporters describing one run agree in value and not in representation. The floor is enforced by the script's exit code and the guard is mutation-proved (clean → exit 0; `--inject-invalid` → 2 errors, exit 1). Evidence: `docs/evidence/DEQM_VALIDATION_2026-08-04.md` | | MeasureReport conformance to DEQM | Da Vinci **DEQM STU5** (`hl7.fhir.us.davinci-deqm#5.0.0`) Individual + Summary MeasureReport | the same reports, validated against the DEQM profiles **explicitly** via `-profile` | **NOT CONFORMANT — gap MEASURED at exactly 3 errors per report, identical on all four shapes** | **We deliberately do NOT stamp `meta.profile` with a DEQM canonical**, so this is a GAP MEASUREMENT, not a claim — claiming a profile we do not meet is the misdeclaration ADR-050 corrected for QRDA's `…24.1.3`/`…27.1.2`. The three: **`deqm-0`** the canonical SHALL carry a version and ours does not (we already hold it in `evidence.official.version`, which ADR-046 threads to the QRDA III identity, so it is an omission at one call site rather than missing data); **`reporter`** cannot satisfy `qicore-organization` (QI-Core's constraint reaching us through DEQM — our contained Organization carries only `name`); **`deqm-3`** measure scoring is required on the root **or** every group and not both, and we emit none. **None is provenance-dependent** — identical on the official and authored paths — so they are properties of how every report is built, not of the ADR-046 identity split. All four findings are pinned in `src/fhir/measure-report.test.ts` citing their constraint keys; when one is fixed, INVERT the test and re-run the script so the drop is measured. **Adding `meta.profile` stays owner-reviewed and gated on this reaching 0.** Also recorded from the tool rather than from research: the DEQM package resolves `hl7.fhir.us.qicore#6.0.0` + `hl7.fhir.us.core#6.1.0`, confirming the published stack binds **QI-Core 6, not STU7**. Evidence: `docs/evidence/DEQM_VALIDATION_2026-08-04.md` | | Calculation vs a SECOND independent engine | `cqf-fhir-cr` (HAPI Clinical Reasoning 8.10.0) over the same official artifacts + the same MADiE cases | `backend-ts/scripts/cross-engine-check.ts` runs `Measure/$evaluate-measure` per subject and diffs the population vector against the measure developer's expected MeasureReport | **255/278 agree across SIX measures; CMS68 19/19, CMS951 55/55, CMS138 47/47 at 100%** | **The first time WorkWell's artifacts have been executed by anything that is not us**, and the reason it matters: `fqm-testify` and `deqm-test-server` both WRAP `fqm-execution`, so neither is independent of our engine — the Java stack is. CMS122 49/55, CMS125 56/66, CMS2 29/36; **IPP and DENOM agree on all 278**. Each measure's disagreements share one shape: CMS125 `DENEX 1→0`; CMS122 `DENEX 1→0` **and** `NUMER 0→1` (same root — inverse measure, `fqm` zeroes NUMER when an exclusion is true); CMS2 `NUMER 1→0`, a different failure. **Read the limits.** (1) The CMS125/CMS122 cause is **PROVEN by construction** (ADR-055 standard): three single-variable mutations isolate the failing conjunct to `"Has Dementia Medications in Year Before or During Measurement Period"` — injecting an Advanced Illness `Condition` bypasses only the medication path and flips DENEX to 1, which proves the age and frailty conjuncts ARE credited. `medicationRequestPeriod()` derives from `dosageInstruction`, which the MADiE cases omit, and the two engines disagree on the result. **CMS2's shape (`NUMER 1→0`) is a different, undiagnosed cause.** Enabling all of this: **`cqf-fhir-cr` retrieval is QI-Core `meta.profile`-sensitive** — an unstamped hand-PUT resource is stored, searchable, and silently never retrieved. (2) This does **NOT** show ours is right and theirs wrong — one stock HAPI configuration, one server version, no alternative CR settings explored. (3) `$evaluate-measure` **CACHES**, silently; every changed input needs a fresh container, and one conclusion in the evidence doc had to be re-proved cold after this was found. (4) CMS130/CMS165 have no test cases checked out locally and are **unmeasured**. (5) Synthetic MADiE patients, not real patient data. Context: the CMS7-FQR connectathon's own Java-vs-JS run found 98.16% pass with 3 of 74 measures disputed, and classifying such discrepancies is what that track asks participants to do. Evidence: `docs/evidence/CROSS_ENGINE_2026-08-04.md` | +| Decision support delivery | **HL7 CDS Hooks 2.0.1** (balloted STU2, `hl7.fhir.uv.cds-hooks#2.0.1`) | `GET /cds-services` discovery + `POST /cds-services/{id}` + `POST .../feedback` for the `patient-view` hook (`backend-ts/src/cds/`) | **Structurally conformant — SELF-GRADED. No external grader exists, so nothing here is validated by a third party** (2026-08-17) | **Read the grader question first, because it is the whole caveat.** There is **no** graded conformance suite for a CDS Hooks service: no HL7 test kit, no Touchstone suite, and **Inferno has no CDS Hooks test kit** (only the CRD-scoped `davinci-crd` kit, whose CDS-Hooks-layer checks are bound to CRD hooks and profiled cards, which we are not). The community `cds-hooks/cds-validator` is a set of JSON Schemas **last pushed 2018-02-05**, and `sandbox.cds-hooks.org` is an interactive simulator that emits **no score**. So this row is the ADR-065 situation again: the claim is "conforms structurally, unverified by any external suite", and it is graded by **our own tests** against the specification's field requirements — 22 assertions across `src/cds/cards.test.ts` and `src/routes/cds-hooks.test.ts`, plus the response shapes described in the OpenAPI document and checked against real responses (ADR-068). Pointing the public sandbox at a deployment would be a **demonstration**, not a verification, and must be recorded as such. **What we deliberately do NOT claim.** (1) **The CDS Hooks JWT profile is NOT implemented** — the spec defines a client-signed RS384/ES384 JWT verified against a JWKS with an `iss`/`jku` allowlist and **SHALL NOT** permit a symmetric algorithm, so WorkWell's HS256 bearer token is *not* that profile; invoke is gated by our own token and the gap is named rather than papered over (ADR-067 d9). (2) **The outcome→card MAPPING is ours and is unprecedented at one leg.** HL7's blessed route is `PlanDefinition/$apply` → `RequestOrchestration` → cards ("CDS Hooks is a wrapper around `PlanDefinition/$apply`", FHIR Clinical Reasoning), and CQF Ruler and AHRQ's CQL Services both do CQL→cards that way; but **DEQM `$care-gaps` stops at a `DetectedIssue` and no published mapping carries a care gap into a card**, so the gap→card leg is a LOCAL mapping. It is precedented at the CDS-Hooks-mechanics layer and nowhere at that leg, and it must not be described as following an IG. The draft HL7 IG "Using FHIR Clinical Reasoning with CDS Hooks" exists but is a CI build with ~8 commits and no authorized publication — a direction, not a ruler. (3) **No `PlanDefinition`, no `$apply`, no CQF Ruler** anywhere in the runtime — ADR-008 stands, and `cqf-fhir-cr` keeps only its B7 cross-check role. (4) **`critical` and `systemActions` are never emitted** (locked decision 1: WorkWell is supplementary to WebChart and may not tell a clinician not to proceed). (5) **Nothing here is justified by certification** — ONC's §170.315**(b)(11)** DSI criterion and the HTI-1 rule concern source attributes, evidence-based vs Predictive DSIs and configurability, and **do not name CDS Hooks**; whether MIE's WebChart holds (b)(11) is an unread CHPL lookup, and whether WebChart acts as a CDS Hooks *client* has **no public evidence either way** (zero hits across both MIE docs sites). Scope limit: cards render persisted outcomes of a FINALIZED run over the synthetic roster and any live tenant alike — **not** an evaluation of data supplied on the request, which is why no `prefetch` is declared. Contract: `docs/CDS_HOOKS.md` (ADR-067) | +| Integration API description | OpenAPI **3.1.1** | `GET /api/v1/openapi.json` (`backend-ts/src/openapi/spec.ts`), rendered at the frontend's public `/api-docs` | **VALID: 0 errors from `redocly lint` 2.46.1, in CI on every PR** (2026-08-17); 5 warnings, explained not silenced | Hand-authored and **scoped to the promised surface only** — `/api/v1/compliance`, the three `/cds-services` operations, health and version. The ~40 internal `/api/**` routes are excluded on purpose: documenting them would advertise stability over paths that carry none. **Two different guards, and neither implies the other** — the lesson from the CVU+ XSD/Schematron episode, where a check's scope was narrower than the claim it was cited for. `redocly lint` says the document is valid OpenAPI (it immediately caught five uses of `nullable`, which 3.1 removed in favour of type unions); it says **nothing** about whether the running worker matches. `src/routes/openapi.test.ts` says the two agree, via **two-way coverage**: every documented `(path, method, status)` is produced by a real request through the real worker, and every observed response is documented. It fails with `documented but NOT ROUTED` on the first — which is exactly how `ARCHITECTURE.md` came to assert a springdoc OpenAPI document for a year after the JVM was retired. Mutation-checked three ways. **3.1.1 not 3.2**, because 3.2 renderer support is silent rather than absent: Redoc 2.5.3 accepts a 3.2 document by aliasing it to 3.1, so 3.2-only constructs are ignored without a warning, and Spectral caps at 3.1. **No conformance certificate exists for an OpenAPI document**, and none is claimed — this is a validity result from one linter, not a graded standard. (ADR-068) | | Measure definition export | MAT (Measure/Library/ValueSet) | `GET /api/measures/{id}/versions/{vid}/export/mat` (FHIR R4 XML) | MAT-compatible | Hand-built FHIR R4 bundle | | Patient-level report | HL7 QRDA Category I | `GET /api/runs/{id}/qrda1` (one CDA per subject) (M-B) | **CVU+-VALIDATED: 0 findings against the HL7 base IG — CDA schema *and* Schematron — on 10 documents over the synthetic corpus (2026-08-02).** Not Calculation Check, not real patient data, official measures only | **The bar is the HL7 QRDA I R1 STU 5.3 US Realm IG** — the §170.205(h)(2) standard that §170.315**(c)(1)** "record and export" and **(c)(2)** "import and calculate" both reference, and the one Cypress validates Category I against. It is **not** the CMS QRDA I IG, which is titled "for Hospital Quality Reporting" (IQR/PI/OQR); CMS122/CMS125 are Eligible Clinician measures whose CMS *submission* format is Category III. Only §170.315(c)(3) "report" splits by setting. **Measured** with `backend-ts/scripts/qrda-schematron-check.py`, which runs the published CMS RY2026 Schematron and partitions failures by conformance number (`CONF:1198/3343/4509/1098/81/67-*` = base HL7, our bar; `CONF:CMS-*` = hospital-only, not our bar — **except** CMS_0105–0113 datatype and CMS_0115–0120 NPI/TIN rules, which carry CMS numbers but bind any conformant CDA and are counted as ours): **one document** with patient data has **0 base-HL7 errors** (+4 CMS-hospital-only findings, expected — we deliberately do not claim the CMS document template `…24.1.3`) and **without** one it has exactly **1** (the missing entry). Evidence, including a negative control that the partition catches: `docs/evidence/QRDA1_SCHEMATRON_2026-07-31.md`. **Scope: one document per state from a hand-built bundle, not a sweep of an endpoint response.** **It reports NO population membership** — Category I has no place for it; measured, no CMS RY2026 sample file contains an `IPOP`/`DENOM`/`NUMER`/`MSRAGG`, because the receiver *recalculates*. Membership stays in MeasureReport + Category III. The Patient Data section carries real QDM entries (Encounter/Diagnosis/Lab/Diagnostic Study/Procedure Performed) translated from the evaluated FHIR bundle — supplied only where the stack can genuinely re-read it (a WebChart-configured seam), **as of export time, not as of the run**; elsewhere the document is emitted, flagged `conformant: false`, counted in the response's `nonConformant`, and says in prose that it cannot be recalculated from. QRDA I **import** now exists — `POST /api/runs/{id}/evaluate` accepts `{measureId, qrda1}` and evaluates the imported bundle through the UNCHANGED engine (§170.315(c)(2) "import and calculate", ADR-051); an unreadable document is a 400 naming the reason, never a silent empty bundle, and every QDM template the mapper does not know is NAMED in the response (the CMS RY2026 sample carries 47). **A round trip found that a QRDA can only carry REAL terminology**: WorkWell's authored measures bind synthetic `urn:workwell:vs:*` value sets with no CDA code system OID, so their data cannot be exported at all — the export now says exactly that instead of emitting an empty document. **CYPRESS CVU+ HAS NOW RUN (2026-08-02, #380/#381) and Category I passes the HL7 base ruler with 0 findings.** 22 submissions of 12 generated documents to a local Cypress **v7.5.1** (application image digest matching the recorded pin), 10 Category I documents covering the five ADR-038 corpus targets × CMS122/CMS125, all HTTP 201. Against `organization=hl7` for reporting year 2026 the result is **0** from `CqmValidators::CDA` (the XSD schema) **and 0** from `Cat1R53` (the base-HL7 Schematron). **Read the Schematron-only row above with this correction:** its "0 base-HL7 errors" was CONFIRMED exactly by CVU+ — and was *narrower than it read*, because `qrda-schematron-check.py` validates Schematron and has **no XSD in it**, where every Category I document was failing 6–10 times. Three defects accounted for all 76: `@root` carrying a URN where CDA's `uid` admits only an OID or UUID (56), the eCQM version STRING in a CDA `INT` (10), and a `` misplaced after `setId`/`versionNumber` (10). All fixed; **76 → 0 re-measured by regenerating and re-uploading**, not re-derived. **What this does NOT license:** it is the externally-supplied-document validation route, **not Cypress Calculation Check** — nothing here says our *calculations* are right; the corpus is **synthetic**, not real patient data; and the 4-per-document CMS-ruler findings that remain are the CMS **Hospital** templateIds we deliberately do not claim, unchanged. The **authored** path still emits `urn:workwell:measure`, which CDA's `uid` rejects — non-conformant **by design** (ADR-046 decision 3 forbids inventing a published eMeasure identity; ADR-051 concluded the authored catalogue is not QRDA-representable at all) and pinned by a test as the only invalid root remaining. **That LOOP bar is RETIRED (2026-08-04, ADR-058)** — it was locked decision #2, "import → evaluate → export → CVU+ green," and it was never met. The loop itself was built and runs (ADR-055/056), but Cypress can only grade **QDM-lineage** documents and the QI-Core artifacts we execute carry no identity it can read, so the green was not obtainable in our lineage. **This row's own result stands unchanged and is not conditional on that bar:** 0 findings against the HL7 base ruler, XSD and Schematron. QRDA I is now scoped as an **interoperability bridge**, and the verification bar is the FHIR-column set in `docs/ROADMAP_2026-08-04.md` §4. Evidence: `docs/evidence/CVU_VALIDATION_RUN_2026-08-02.md`. (ADR-050/051, superseding ADR-049) | | Aggregate report | HL7 QRDA Category III | `GET /api/runs/{id}/qrda` (CDA XML) (E3.3) | **CVU+-VALIDATED: 0 findings against the HL7 base IG — CDA schema *and* Cat III Schematron (2026-08-02).** No longer a stub | It **was** a stub, and CVU+ measured the gap at **48 findings** (24 per document) before quantifying it closed the same day. Three kinds of wrong, none visible to a well-formedness check. **(a) The whole CDA header was missing** — no `recordTarget` (CONF:4484-17212), `author`/`time` (CONF:4484-18156/18158) or `custodian` (CONF:4484-17213), all SHALL. For an aggregate report `recordTarget` carries ``: CDA requires a patient identifier and this document is about a population, so it is nulled rather than invented. **(b) The population templates were INVERTED** — `…27.3.3` *is* the Aggregate Count template and sat on the OUTER assertion observation with `…27.3.24` inside, so the validator applied Aggregate Count's rules to the outer element (missing `MSRAGG` CONF:77-19508, `methodCode` CONF:77-19509, `INT` value CONF:77-17567 — three findings per population) while the inner element that satisfied all three was validated as nothing at all. Correct nesting is Measure Data `…27.3.5` wrapping Aggregate Count `…27.3.3`. **(c) TemplateId version drift** — `2017-06-01` where R2.1 wants `2020-12-01`, `2016-09-01` on `…27.3.5`; and the performance rate was `…27.3.4` + `code="REASON"` where it is `…27.3.14`/`…27.3.30` with LOINC `72510-1` and a `reference` to the numerator it rates. **Also dropped: `…27.1.2`**, claimed here with extension `2017-06-01`. It is "QRDA Category III Report — **CMS** (V4)" (extension `2022-12-01`), the same misdeclaration ADR-050 corrected for Category I's `…24.1.3`; the HL7 ruler never flagged it because the extension was wrong too, so it matched no rule. **The reference for all of it is Cypress's own conformant fixture**, read out of the running container — derived, not guessed — with the 2026 Schematron consulted directly where the 2018 fixture predates it (each Measure Data observation now carries the `reference`/`externalObservation`/`id` CONF:3259-18239 requires, naming the population criterion: the published `Measure.group.population.id` for an official measure, the population code otherwise). Like Category I, deliberately **no `legalAuthenticator`** — it would need an `assignedPerson` no real person stands behind, and the HL7 ruler does not require it. Evidence: `docs/evidence/CVU_VALIDATION_RUN_2026-08-02.md` §5.4, §10 | diff --git a/docs/guide/10-scenarios.md b/docs/guide/10-scenarios.md index 200f8216..b67c179d 100644 --- a/docs/guide/10-scenarios.md +++ b/docs/guide/10-scenarios.md @@ -404,13 +404,16 @@ authorizes. ## S7 — Quality inside the encounter (target state) -> **This one is not built.** Every scenario above documents shipped behaviour; this is the target -> architecture for the WebChart integration, and today's implementation is close to its opposite — -> WorkWell *pulls* from WebChart on a schedule rather than receiving pushes, there is no endpoint -> that takes a bundle and returns findings, and nothing writes back into the EHR. The table at the -> end of this section names exactly which parts exist. Mechanisms it would build on: +> **This flow is not built, but its delivery half now is.** Every scenario above documents shipped +> behaviour. This one is the target architecture for the WebChart integration, and it has moved: since +> 2026-08-17 WorkWell serves a **CDS Hooks** service — the community standard for exactly this shape of +> alert — so a clinician's system can ask "what is outstanding for this patient?" and get structured +> findings back. What is still absent is the half that makes it *live*: WorkWell answers from the last +> completed run rather than from data supplied on the request, nothing in WebChart fires the hook today, +> and nothing writes back into the EHR. The table at the end of this section names each part. Mechanisms: > [chapter 5](05-fhir.md) (FHIR), [chapter 4](04-engine-and-routing.md) (evaluation), -> [`docs/COMPLIANCE_API.md`](../COMPLIANCE_API.md) (the contract it extends). +> [`docs/CDS_HOOKS.md`](../CDS_HOOKS.md) (the built contract), +> [`docs/COMPLIANCE_API.md`](../COMPLIANCE_API.md) (the per-measure one it complements). The idea in one sentence: **quality stops being a report somebody visits and becomes an answer that arrives while the clinician can still act on it.** @@ -424,18 +427,20 @@ sequenceDiagram actor MGR as Quality manager NP->>WC: open an encounter, enter patient data loop as the visit progresses - WC->>WW: submit what the chart holds so far - WW-->>WC: quality findings for this patient + WC->>WW: fire the patient-view hook + WW-->>WC: cards — gaps, reasons, a draft order WC-->>NP: alerts, in the chart already open + NP->>WC: accept or dismiss a card + WC->>WW: feedback — accepted or overridden end - Note over WC,WW: weekly — the same exchange, run as a batch - WC->>WW: everything since the last exchange - WW-->>WC: findings across the population - WW->>WC: follow-up tasks and documents - WC-->>NP: work lands in the queue + Note over WC,WW: still missing — the chart's own data, sent for evaluation + WC-->>WW: submit what the chart holds so far MGR->>WW: population dashboards, already accumulated ``` +Solid arrows are built; the dashed one at the bottom is step 2, the piece that would make a card +reflect *this* visit rather than the last completed run. + What happens, in order: 1. **An encounter is an event, not a document.** The practitioner opens one and starts entering @@ -446,15 +451,23 @@ What happens, in order: thirty patients a day, so pulling a month of encounters for a ten-physician practice means fetching, assembling and evaluating tens of thousands of records before anybody sees a single answer. Pushing as it happens spreads that same work across the day and turns the population - view into a read of something already computed. -3. **Findings come back in-band, during the visit.** The exchange repeats as the visit progresses - — at check-in, after the nurse's intake, after the clinician's assessment, before close — so - each round of alerts reflects what is in the chart at that moment. An alert after the encounter - closes is a letter; an alert during it is a decision. -4. **The practitioner never leaves WebChart.** WorkWell returns structured findings and WebChart - renders them in the chart that is already open. This is the "embed quality in WebChart" half of - the idea, and doing it at the data level rather than as an embedded panel is deliberate: a panel - is still somewhere you have to go and look, which is the problem it was meant to solve. + view into a read of something already computed. **This is the part still missing.** CDS Hooks has + a place for it — `prefetch`, where a client ships FHIR resources alongside the invocation — and + WorkWell deliberately declares no prefetch template, because it does not evaluate data supplied + on the request and advertising otherwise would make a client fetch and transmit for nothing. +3. **Findings come back in-band, during the visit.** *Built, over yesterday's evaluation.* The + exchange is a CDS Hooks invocation: the client fires the `patient-view` hook with a patient id and + gets back **cards** — a one-line summary, a plain-English reason, the run and date it was computed + from, and a link into WorkWell. An alert after the encounter closes is a letter; an alert during it + is a decision. The honest limit is freshness, not delivery: cards reflect the last completed run, + so data entered five minutes ago is not yet in them. +4. **The practitioner never leaves WebChart.** *Built at the data level; not built as rendering.* + Cards are structured for the client to draw in its own UI, which is what makes this an + integration rather than an embedded panel — a panel is still somewhere you have to go and look. + How WebChart would render them is WebChart's to decide, and nothing renders them today. + Follow-up is a card **suggestion**: a `ServiceRequest` with `intent=proposal`, `status=draft`, + which the clinician accepts or ignores. That is write-back **without WorkWell holding write + credentials into a certified EHR** — the standard carries the proposal, the EHR performs the write. 5. **Weekly, the same exchange runs as a batch.** Same endpoint, same evaluation, wider window — it catches whatever the real-time path missed and gives both sides a natural reconciliation point. @@ -466,7 +479,10 @@ What happens, in order: different on purpose rather than by omission. 8. **Periodically, both sides reconcile what was sent against what was received.** Encounters WorkWell never got, and encounters it got that WebChart has no record of sending, are both - findings — a sink that silently drops messages looks exactly like a quiet week. + findings — a sink that silently drops messages looks exactly like a quiet week. *Half built:* the + CDS Hooks **feedback** endpoint records whether each card was accepted or overridden, so "did + anyone act on this?" is now answerable from the audit ledger. Reconciling *encounters* still needs + step 2. ### Why this is not just another quality dashboard @@ -486,15 +502,35 @@ answer is **traceable to a published measure** run on a reference engine, which | FHIR ingest from a live WebChart tenant | **Built, but pulling** — SMART Backend Services, read-only, on a schedule (S2) | | A compliance answer per subject and measure | **Built** — versioned, read-only `GET` ([`COMPLIANCE_API.md`](../COMPLIANCE_API.md)) | | Population dashboards for a quality manager | **Built** — the manager surface is the one part of this diagram that is real | -| WebChart pushing an encounter as it happens | Not built | -| Submit a bundle, get findings back synchronously | Not built — the closest thing is the QRDA import route (S5), which evaluates a supplied document but is not shaped for a live encounter | -| Quality rendered inside WebChart's own UI | Not built | -| Tasks and documents written back into WebChart | Not built — the integration is read-only in both directions today | -| Send/receive reconciliation between the two systems | Not built | +| **A standards-shaped way to deliver findings into a workflow** | **Built** — a CDS Hooks 2.0.1 service for the `patient-view` hook ([`CDS_HOOKS.md`](../CDS_HOOKS.md)); cards over the most recent **completed run** | +| **Follow-up offered as an order the clinician accepts** | **Built** — a card `suggestion` carrying a draft `ServiceRequest`, so nothing is written by WorkWell. Only for order codes with an APPROVED terminology mapping, which today excludes cms122/cms125 | +| **Did anyone act on the finding** | **Built** — the CDS Hooks feedback endpoint, audited | +| Evaluating data supplied on the request | Not built — this is step 2, and `prefetch` is where it would go. WorkWell declares none, because it evaluates none | +| WebChart pushing an encounter as it happens | Not built — and no public evidence either way that WebChart acts as a CDS Hooks client | +| Quality rendered inside WebChart's own UI | Not built — cards are structured for a client to draw; nothing draws them today | +| Tasks and documents written back into WebChart | Not built — a suggestion proposes an order; there is no task or document write path | +| Send/receive reconciliation of encounters | Not built — card feedback answers "was it acted on", not "did every encounter arrive" | One connection worth drawing, because it turns an existing refusal into a feature. The compliance API's `mode=preview` deliberately returns **501 on a WebChart-configured stack** (ADR-061): preview composes a *synthetic* bundle, and reporting demo playback as an evaluation of a live tenant would -be a lie. An endpoint where WebChart submits a **real** bundle and gets findings back is exactly -what makes that answer honest — the caller supplies the data, so nothing is being simulated. The -501-shaped hole in today's API is the shape of step 2 above. +be a lie. A path where the caller submits a **real** bundle and gets findings back is exactly what +makes that answer honest — the caller supplies the data, so nothing is being simulated. The +501-shaped hole in today's API is the shape of step 2 above, and in CDS Hooks it has a name: +`prefetch`. Serving it is the same refusal in a different place — the service declares no prefetch +template *because* it evaluates none, rather than accepting resources and quietly ignoring them. + +### Why CDS Hooks rather than an endpoint of our own + +The earlier sketch of this scenario described a bespoke submit-a-bundle endpoint. Building one would +have meant asking MIE to write a client against a contract only WorkWell speaks. CDS Hooks is the +published standard for this exact shape — discovery plus one invocation per service, returning cards +— and it is a JSON contract over HTTPS, so serving it needed no new dependency and no JVM: two routes +on the worker that already existed. Its `suggestion` mechanism also solves the hardest part of step 6 +for free, since a proposed order travels as data the EHR performs rather than as a write WorkWell +would need credentials for. + +What the standard does **not** settle: whether WebChart acts as a CDS Hooks client, and how it would +authenticate. CDS Hooks defines its own signed-JWT profile which forbids symmetric algorithms, so +WorkWell's bearer token is not it — the gap is named in [`CDS_HOOKS.md`](../CDS_HOOKS.md) rather than +papered over, and it reduces to two things to ask for: an issuer and a JWKS URL. diff --git a/docs/guide/README.md b/docs/guide/README.md index bc603fd5..d7ce36be 100644 --- a/docs/guide/README.md +++ b/docs/guide/README.md @@ -150,8 +150,8 @@ For the same flows drawn as *sequences* — who calls what, in what order — se The guide explains; these specify. [`ARCHITECTURE.md`](../ARCHITECTURE.md) (module-level detail), [`DATA_MODEL.md`](../DATA_MODEL.md) and [`DATA_MODEL_CONTRACTS.md`](../DATA_MODEL_CONTRACTS.md) -(schemas and contracts), [`COMPLIANCE_API.md`](../COMPLIANCE_API.md) and -[`PACKAGES.md`](../PACKAGES.md) (the two integrator contracts), +(schemas and contracts), [`COMPLIANCE_API.md`](../COMPLIANCE_API.md), [`CDS_HOOKS.md`](../CDS_HOOKS.md) and +[`PACKAGES.md`](../PACKAGES.md) (the integrator contracts — one answer, one workflow surface, one library), [`MEASURES.md`](../MEASURES.md) (the measure catalog in plain English), [`STANDARDS_CONFORMANCE.md`](../STANDARDS_CONFORMANCE.md) (what we claim and refuse to claim), [`ROADMAP_2026-08-04.md`](../ROADMAP_2026-08-04.md) (the approved plan), and From 3af2d83f20d621907adb53e151aac5e74ae88c47 Mon Sep 17 00:00:00 2001 From: Taleef Date: Mon, 17 Aug 2026 13:33:08 -0400 Subject: [PATCH 5/7] =?UTF-8?q?fix(cds):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?a=20failed=20audit=20no=20longer=20reports=20success,=20and=20a?= =?UTF-8?q?n=20engine=20failure=20is=20not=20a=20clinical=20claim?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the code review and Codex's PR comments. Seven substantive fixes, four vacuous guards replaced, and the doc claims corrected where they overstated. Correctness and safety: - Feedback returns 503, not 200, when its audit write fails. The audit event IS the entire persistence for that endpoint, and the spec gives feedback no response body to signal partial success, so a swallowed failure told the client never to retry and lost the accepted order silently. Invoke stays best-effort, since its cards are correct regardless. Both reviewers raised this independently; CLAUDE.md's "every state change writes audit_event -- no exceptions" decides it. - A FAILED evaluation is reported as ours. PARTIAL_FAILURE rows are served, and a subject whose evaluation threw persists as MISSING_DATA + evaluationError -- which deriveCell rendered as "No record on file" and nextActionFor turned into "Collect the missing documentation", asserting a fact about the patient when our engine threw. Now a "could not be evaluated" card, info, no suggested order. - A suggested ServiceRequest references the id the CLIENT sent. On a live tenant it carried Patient/wc|4821, which names nothing the client can resolve and is not a legal FHIR id, so the suggestion could not be applied (Codex). Re-pointed in the CDS layer only; /api/orders/proposals is unchanged. - Feedback is bounded: 100 entries per request, userComment truncated at 8000 chars. Each entry is an append to the append-only ledger by a machine credential, and userComment is the first unstructured clinical prose in audit_events -- now noted in the PHI posture. - card.uuid is a conformant UUID (version 8 per RFC 9562, variant 10xx). CDS Hooks types it as a UUID and a validating client would reject a bare hash. - refRunId was dead: no caller ever set detail.runId. The invoke event now records the run and maps each emitted uuid to its measure, which makes the documented correlation claim true -- it previously overstated by a step, since inverting a uuid meant a search over every run in the subject's history. - Removed the authorize rule for /api/v1/openapi.json: handleOpenApi runs before the auth gate, so the rule could never be consulted while reading as the control. Guards that could not fail, now replaced: - The alias assertion was `assert.notEqual(authorize(...).ok && alias === PATH, true)` -- the && is always false, so it passed for any implementation. Now asserts the aliases return the worker's 501 not_implemented, with a token, so it proves "not served" rather than "not reachable". - The `critical` loop asserted a value against a type that permits only those two values. Replaced with the priorityFor mapping, which is failable. - assert.ok(Array.isArray(cardUuids)) pinned a field name and nothing else, and `.find()` returned the OLDEST event despite being named `latest`. Now deep-equals the uuids the client received. - The openapi fixture had no APPROVED mapping, so the suggestion subschemas -- the deepest in the document -- were never schema-validated. Added. Also: the reachability walk starts from the paths rather than every component (a dead schema referencing another marked its target live) and follows $ref into its target; parameter schemas are walked; endpointsOf skips non-operation path-item keys; and a test pins dispositionFor's OPEN set equal to order-proposal's AT_RISK set, an undocumented coupling the suggestion dedupe silently depends on. One finding NOT acted on: Codex's claim that acceptedSuggestions is an array of UUID strings. The spec defines it as "an array of json objects identifying one or more of the user's AcceptedSuggestions", each with a REQUIRED `id` string, and every example is [{"id": "..."}]. The schema was already right. And a self-inflicted one worth recording: a sed -i over this UTF-8 source replaced a space with a literal NUL byte, twice, and it reached a commit -- it typechecked, every test passed, and the only symptom was grep reporting the file as binary. Fixed, with a test that would have caught both. backend 1976 tests / 0 fail; frontend 187 / lint clean / build clean; redocly 0 errors. Co-Authored-By: Claude Opus 5 (1M context) --- backend-ts/src/auth/authorize.ts | 7 +- backend-ts/src/cds/cards.test.ts | 111 ++++++++++++++++-- backend-ts/src/cds/cards.ts | Bin 11394 -> 15601 bytes backend-ts/src/routes/cds-hooks.test.ts | 67 ++++++++++- backend-ts/src/routes/cds-hooks.ts | 107 +++++++++++++++-- backend-ts/src/routes/openapi.test.ts | 64 ++++++++-- docs/CDS_HOOKS.md | 40 +++++-- docs/DECISIONS.md | 49 ++++++-- docs/PRODUCTION_READINESS_2026-07.md | 13 ++ .../api-docs/api-reference.test.tsx | 47 +++++++- frontend/components/api-docs/types.ts | 10 ++ 11 files changed, 454 insertions(+), 61 deletions(-) diff --git a/backend-ts/src/auth/authorize.ts b/backend-ts/src/auth/authorize.ts index 7f2ea21b..9087f570 100644 --- a/backend-ts/src/auth/authorize.ts +++ b/backend-ts/src/auth/authorize.ts @@ -59,9 +59,10 @@ const RULES: Rule[] = [ { pattern: rx("/api/health"), access: "PERMIT" }, { pattern: rx("/api/version"), access: "PERMIT" }, { pattern: rx("/health"), access: "PERMIT" }, - // The OpenAPI document (ADR-068) — shapes and role names, no patient data. Public because reading the - // contract without credentials is most of its value to an integrator. Must precede the `/api/**` tails. - { pattern: rx("/api/v1/openapi.json"), access: "PERMIT" }, + // NOTE: `/api/v1/openapi.json` deliberately has NO rule. Like health and version, `handleOpenApi` runs in + // the worker BEFORE the auth gate, so a rule here would never be consulted — a control that reads as + // load-bearing and cannot fire (review). Its public-ness is asserted by an actual unauthenticated request + // in `openapi.test.ts`, which is the only thing that proves it. { pattern: rx("/sse"), access: [A, CM, MCP] }, { pattern: rx("/mcp/**"), access: [A, CM, MCP] }, diff --git a/backend-ts/src/cds/cards.test.ts b/backend-ts/src/cds/cards.test.ts index 869103c9..b198928c 100644 --- a/backend-ts/src/cds/cards.test.ts +++ b/backend-ts/src/cds/cards.test.ts @@ -9,8 +9,11 @@ */ import { test } from "node:test"; import assert from "node:assert/strict"; +import { readdirSync, readFileSync } from "node:fs"; import { buildComplianceCards, cardUuid, noEvaluationCard, suggestionUuid, type CardInput } from "./cards.ts"; import { ORDER_CATALOG } from "../order/order-catalog.ts"; +import { dispositionFor } from "../case/case-logic.ts"; +import { proposeOrders } from "../order/order-proposal.ts"; import type { StandingOrderProvider } from "../order/standing-order-provider.ts"; /** No standing orders, so suppression never confounds a suggestion assertion. */ @@ -41,6 +44,7 @@ function row(measureId: string, status: string, extra: Partial = {}): const opts = (approved: ReadonlySet = APPROVED) => ({ subjectId: "emp-006", + patientId: "emp-006", approvedOrderCodes: approved, standingOrders: NO_STANDING_ORDERS, studioBaseUrl: "https://studio.example.org", @@ -63,23 +67,48 @@ test("an open gap becomes one card carrying the answer, its provenance and a lin ]); }); -test("`critical` is never emitted, for any open status", async () => { +test("`critical` is never emitted: OVERDUE is the ceiling, and it is `warning`", async () => { // In CDS Hooks `critical` means the user must not proceed. WorkWell is SUPPLEMENTARY to WebChart - // (locked decision 1) and is not entitled to say that about someone else's encounter. `CdsCard` makes - // it a type error; this asserts the runtime consequence over every status that produces a card. - for (const status of ["OVERDUE", "DUE_SOON", "MISSING_DATA"]) { + // (locked decision 1) and is not entitled to say that about someone else's encounter. + // + // **The type is the enforcement, not this test.** `CdsCard.indicator` is `"info" | "warning"`, so an + // assertion that the value is one of those two cannot fail for any implementation — a first version of + // this test did exactly that and read as coverage while being unfailable (review). What IS failable, and + // what actually pins the ceiling, is the `priorityFor` mapping below: flip either branch and this fails. + const byStatus: Array<[string, string]> = [ + ["OVERDUE", "warning"], + ["DUE_SOON", "info"], + ["MISSING_DATA", "info"], + ]; + for (const [status, expected] of byStatus) { const cards = await buildComplianceCards([row("audiogram", status)], opts()); assert.equal(cards.length, 1, `${status} must produce a card`); - assert.ok( - cards[0]!.indicator === "info" || cards[0]!.indicator === "warning", - `${status} produced indicator ${cards[0]!.indicator}`, - ); + assert.equal(cards[0]!.indicator, expected, `${status} must map to ${expected}`); } - // OVERDUE is the most urgent thing we say, and it is `warning` — the ceiling. - const overdue = await buildComplianceCards([row("audiogram", "OVERDUE")], opts()); - assert.equal(overdue[0]!.indicator, "warning"); - const dueSoon = await buildComplianceCards([row("audiogram", "DUE_SOON")], opts()); - assert.equal(dueSoon[0]!.indicator, "info"); + // And read through a widened type, so this keeps working — and keeps failing — if `CdsCard` ever admits + // `critical`. Without the widening the compiler makes the comparison unreachable. + const emitted = (await buildComplianceCards([row("audiogram", "OVERDUE")], opts()))[0]!; + assert.notEqual((emitted.indicator as string), "critical"); +}); + +test("a FAILED evaluation is reported as ours, not as a missing record in the chart", async () => { + // PARTIAL_FAILURE is terminal, so its rows are served, and a subject whose evaluation threw is persisted + // MISSING_DATA + evidence.evaluationError (DATA_MODEL_CONTRACTS §5). Without a branch for it, `deriveCell` + // falls through to "No record on file" and `nextActionFor` says "Collect the missing documentation" — + // asserting a fact about the PATIENT when our engine threw (review). + const failed = row("audiogram", "MISSING_DATA", { + evidence: { evaluationError: "CQL engine failure", message: "boom" }, + }); + const cards = await buildComplianceCards([failed], opts()); + assert.equal(cards.length, 1); + const card = cards[0]!; + assert.match(card.summary, /could not be evaluated/); + assert.equal(card.indicator, "info", "our failure is never a warning about the patient"); + assert.match(card.detail!, /no compliance gap is being asserted either way/); + assert.doesNotMatch(card.detail!, /Collect the missing/); + assert.doesNotMatch(card.summary, /missing data/i); + // And it must never carry an order derived from a status the engine did not compute. + assert.equal(card.suggestions, undefined); }); test("a suggestion is offered ONLY for an APPROVED order code", async () => { @@ -156,6 +185,22 @@ test("no studio base URL means no links, never a broken one", async () => { assert.equal(cards[0]!.source.url, undefined); }); +test("a suggested order references the id the CLIENT sent, not WorkWell's internal subject id", async () => { + // On a live tenant these differ: WorkWell persists `wc|4821`, the hook supplies `4821`. A ServiceRequest + // referencing `Patient/wc|4821` names nothing the client can resolve, and `|` is not a legal FHIR id — so + // the suggestion could not be applied (Codex review). + const cards = await buildComplianceCards([row("audiogram", "OVERDUE")], { + ...opts(), + subjectId: "wc|4821", + patientId: "4821", + }); + const resource = cards[0]!.suggestions![0]!.actions[0]!.resource as { subject: { reference: string } }; + assert.equal(resource.subject.reference, "Patient/4821"); + assert.doesNotMatch(resource.subject.reference, /\|/, "a FHIR id may not contain a pipe"); + // The internal id still keys card identity, so feedback correlation is unaffected. + assert.equal(cards[0]!.uuid, await cardUuid(RUN, "wc|4821", "audiogram")); +}); + test("card and suggestion uuids are deterministic, UUID-shaped, and distinct per measure", async () => { // Determinism is what lets the feedback endpoint exist with no schema change: the id is recomputable // from (runId, subjectId, measureId) rather than stored. @@ -163,6 +208,11 @@ test("card and suggestion uuids are deterministic, UUID-shaped, and distinct per const again = await cardUuid(RUN, "emp-006", "audiogram"); assert.equal(a, again, "the same card must always get the same uuid"); assert.match(a, /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/); + // A CONFORMANT uuid, not merely a hash in uuid shape: CDS Hooks types `card.uuid` as a UUID, so a client + // validating the version nibble or storing it in a native `uuid` column would reject a bare hash (review). + // Version 8 = RFC 9562 "custom", which is what a deterministic application-defined id is; variant 10xx. + assert.equal(a[14], "8", "version nibble must be 8"); + assert.ok("89ab".includes(a[19]!), `variant nibble must be 8/9/a/b, got ${a[19]}`); assert.notEqual(a, await cardUuid(RUN, "emp-006", "cms125"), "different measure → different uuid"); assert.notEqual(a, await cardUuid(RUN, "emp-007", "audiogram"), "different subject → different uuid"); assert.notEqual(a, await cardUuid("other-run", "emp-006", "audiogram"), "different run → different uuid"); @@ -174,6 +224,41 @@ test("card and suggestion uuids are deterministic, UUID-shaped, and distinct per assert.equal(cards[0]!.suggestions![0]!.uuid, await suggestionUuid(RUN, "emp-006", "audiogram")); }); +test("the carded statuses and the order-proposal statuses are the SAME set — the coupling the dedupe rests on", () => { + // Cards are selected by `dispositionFor(status) === "OPEN"`; proposals by `AT_RISK` in order-proposal.ts. + // Today those sets are identical, which is why the first card to claim a collapsed proposal is always the + // row that created it. If they diverge — someone adds a status to one and not the other — the claiming + // card and the ServiceRequest's `reasonCode` would name DIFFERENT measures, and the genuinely at-risk + // measure would silently lose its suggestion to a non-at-risk one (review flagged this as load-bearing and + // unpinned). This test is the pin; it has no other purpose. + const ALL = ["COMPLIANT", "DUE_SOON", "OVERDUE", "MISSING_DATA", "EXCLUDED", "DECLINED", "IN_PROGRESS"]; + const carded = ALL.filter((s) => dispositionFor(s) === "OPEN").sort(); + // Mirrors AT_RISK's keys. Kept as a literal so a change to either side shows up as a diff here. + const atRisk = ["OVERDUE", "DUE_SOON", "MISSING_DATA"].sort(); + assert.deepEqual(carded, atRisk); + // And prove the consequence rather than just the sets: an at-risk status yields a proposal for a mapped + // measure, so a carded row can always claim one. + for (const status of carded) { + const { proposed } = proposeOrders([{ subjectId: "emp-006", measureId: "audiogram", status }], NO_STANDING_ORDERS); + assert.equal(proposed.length, 1, `${status} is carded, so it must also propose`); + } +}); + +test("the CDS sources contain no invisible control characters", () => { + // Twice in one session a `sed -i` over this UTF-8 source replaced a space with a literal NUL byte, which + // reached a commit: it typechecked, every test passed, and the only symptom was `grep` reporting the file + // as binary. An invisible byte in a string literal that feeds a hash is exactly the kind of thing that is + // correct-by-accident until it is not. Cheap to assert, and it would have caught both occurrences. + const dir = new URL(".", import.meta.url); + for (const name of readdirSync(dir)) { + if (!name.endsWith(".ts")) continue; + const text = readFileSync(new URL(name, dir), "utf8"); + // Tab, LF and CR are legitimate; nothing else below 0x20 is, and neither is a NUL. + const bad = [...text].filter((ch) => ch < " " && ch !== "\n" && ch !== "\r" && ch !== "\t"); + assert.deepEqual(bad, [], `${name} contains ${bad.length} control character(s)`); + } +}); + test("an absence of data is a CARD, and it does not claim compliance", async () => { const card = noEvaluationCard("wc|4821"); assert.equal(card.indicator, "info"); diff --git a/backend-ts/src/cds/cards.ts b/backend-ts/src/cds/cards.ts index 46a1317373c60b18c5d6d55a0e700bb007ce3e84..adb246ef793b91c294de096676848db0d3d93066 100644 GIT binary patch delta 4034 zcmZ`+&vPS36|UU?#fnOR5E5W3$y?RdVhLqr2X-;q>;}a@c11b1OSVZ-o5jylBK5~NpgQ^t&0T;OPz3v&=-c71pwx#KQ{rY>~_q{j2 z|J8qHpVe+e`$I`lNGblTt?$v=R%c^-pNve>22F=iFr>)Xrz(kOq!O%7DG&*XAjLM-nUSV|N5crzlCKPDIQZAIX^^m8F3iNs6&A8#gGFVK$Z|k`OuS5bJP} zD4pG>dOaORxvAIf9R>>F5lvy~j5Z&4x}>F1nGU48%M`hAl%5EV8q|}4$Z$7*SELal zHW#TUVuVPELhxjOi1ro%vQN?<;(LBN7JVgQca*=fe0=PC zmWPi~Q1EUxW*TrQy)27DpLru}-VHJ+9yzEH6M-ipjzW=Q2UDrYlTS*bUN4pnQksw+ z3e6lwtpo~`f}%P3E_$HiY?L5z3$vEYRjlgq{fEK~FGFn++ZDQi=b4HU89LhAl9$&{ zSTm}RFgi;>NF?q=z$8XSBPk#Y9Dt9gPAZ}Vz~FMY1ScO#9ZirX>H_X$pj0m!WGXWr zwG)y;gKQe?okX^Fm-C1t$OY8eKG>q(ECpl8HxejvCPM*vB&k+mmIpHAqk1!_IlG za%s6iEB9*mLH#gd-8{MXlpfI1;=?VvL(9j@{TiL(@9OfUm8+Lmu0C}ue|%*nfG{a? zI!4SVp69u+PrZ>if9son_*$Jo+=b*R;}6r-MiZPqK2Jftyj^4Ht@?ZXdu);G2e z*LJq|yY038J#U0^|MQc(fVrR@pEeyTAb%wzEm0rCflOpl#Ab*7u(P+<*?x4$kzvzO zIwT)NgQ*7c@R+X_qz6l@&_GtsKnKq@)om&oI^ZKH20?K2m_Re=^!T~l$) zI%ax~B4gM}LltBrc(koY)!}H2)W~oEsQTR8L=WA??%Hkd+pai8P`(dGN_7S_fLJ=r zaXYjQlF0xbfG-%!g(=ex8%+rJ=qYYVw)og5aHkmZWLCq;wK|SQHl6!UW+Mr%^Jy|R z61{{~2-m`o^EZRu!V>LgxfQ;jsEq~1)^L(f@)($7wW2Zn0oUbfG%NkAD8qAg+vzWr z#cjc8juJIZYAsl%W>iio$>Nw!ps}b=OKi<>HQtaj9q87B2UJzIy;te<)IPBf9^e6Q zt)Y5prW5;t1?XfYwPvZ7Co}K5w#x6hgBxWk4{J}V&t+VF%5m}{ZkZAXsHxW*TT%aY$xkdb6CmCny{2$+aH2>u7H*Z=xUg`A3wPiSOzkc^xW&Xd~ zt@$5czxk4X`(*w>{ipd^12Dk*B)Ui3vi}qLA1QtG5hMkm%c-FZ1QvouXg#*`pj*2y ziqvfaO!5o3K|CYlz6C>a4^tQ>5-68`RH2*?N1Ik7%x!|nFc4$rX-`^cELN}-2?K#q zO~tHU_uvXxfgc`+;7-t@WJx-KToEX*vIn_yN)cOUF_BtHSv&sG4`> zJ7ctXP8C`m!#}xu!?(zeG`4+nF2w-N;&D1HG40J*eJp4{ReMrT(6_qsHyOGWtW~2` zjaZ3GP6QIbh65&^{2ELnA92NHdjJ3c delta 135 zcmexZ*%Y}!lXJ5M_hgpI`}h^4GV>HdLp@y-iZc=mQWZ2TbWOm}&`5LgH-62@Cxsdr z876xR2TlGbEXAu=oRgWHs$rm`U}&bfSwf_nW3#_ e.eventType === "CDS_HOOKS_INVOKED")!; - const payload = latest.payload as Record; - assert.equal(payload["sensitivityLabel"], "restricted"); - assert.ok(Array.isArray(payload["cardUuids"]), "the emitted uuids must be recorded for correlation"); + // `listAuditEvents` orders occurred_at ASC, so `.find` returns the OLDEST — a first version of this named + // the variable `latest` and inspected the wrong row (review). Take the last, and assert CONTENT: that the + // recorded uuids are exactly the ones the response carried. `Array.isArray(...)` alone cannot fail for any + // card content, which made it pin the field name and nothing else. + const invoked = (await stores.events.listAuditEvents(1000)).filter((e) => e.eventType === "CDS_HOOKS_INVOKED"); + const newest = invoked[invoked.length - 1]!; + assert.equal((newest.payload as Record)["sensitivityLabel"], "restricted"); + + const res = (await post(INVOKE, hookBody("emp-006")))!; + const returned = ((await res.json()) as { cards: Array<{ uuid?: string }> }).cards.map((c) => c.uuid); + const after = (await stores.events.listAuditEvents(1000)).filter((e) => e.eventType === "CDS_HOOKS_INVOKED"); + const forThatCall = after[after.length - 1]!.payload as Record; + const recorded = (forThatCall["cards"] as Array<{ uuid: string; measureId: string | null }>).map((c) => c.uuid); + assert.deepEqual(recorded, returned, "the ledger must record exactly the uuids the client received"); + // And the run, so recovering a measure from a feedback uuid is one recomputation per measure rather than a + // search over every run in the subject's history. + assert.equal(forThatCall["runId"], completedRunId); + assert.equal(newest.refRunId ?? forThatCall["runId"], completedRunId, "ref_run_id must be populated"); // Discovery must NOT write: it carries no patient data, and a public endpoint writing per request is a // denial-of-service amplifier against our own ledger. @@ -258,6 +272,51 @@ test("feedback validates the spec's conditional fields and records the outcome", assert.equal((overridden.payload as Record)["overrideReasonCode"], "not-now"); }); +test("feedback FAILS LOUDLY when the audit write fails — the event is the only record", async () => { + // For invoke, best-effort auditing is right: the cards are still correct. For feedback the audit event IS + // the persistence (ADR-067 d10), so swallowing a failure would make the endpoint a silent no-op that told + // the client never to retry — and the spec gives feedback no response body to signal otherwise. Both this + // review and Codex flagged it independently; CLAUDE.md's "every state change writes audit_event — no + // exceptions" is what decides it. + const broken = { ...env, DB: { ...(env["DB"] as object), prepare: () => { throw new Error("ledger down"); } } }; + const res = (await handleCdsHooks( + new Request(`http://x${INVOKE}/feedback`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ feedback: [{ card: "c", outcome: "overridden", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }), + }), + broken as never, + "tester@workwell.dev", + ))!; + assert.equal(res.status, 503, "a lost feedback event must not be reported as success"); + const body = (await res.json()) as { error: string; recorded: number; of: number }; + assert.equal(body.error, "audit_write_failed"); + assert.equal(body.recorded, 0); + assert.equal(body.of, 1); +}); + +test("feedback is bounded: entry count and free-text comment length", async () => { + const path = `${INVOKE}/feedback`; + const entry = { card: "c", outcome: "overridden", outcomeTimestamp: "2026-06-12T00:00:00Z" }; + // 100k entries would be 100k appends to the append-only ledger from one request by a machine credential — + // the amplification the discovery endpoint's no-audit decision avoided one route over (review). + const tooMany = (await post(path, { feedback: Array.from({ length: 101 }, () => entry) }))!; + assert.equal(tooMany.status, 400); + assert.match(((await tooMany.json()) as { message: string }).message, /at most 100 entries/); + assert.equal((await post(path, { feedback: Array.from({ length: 100 }, () => entry) }))!.status, 200); + + // A clinician's free text is capped and truncation-marked, matching AI_GUARDRAILS §2.2's bound. + const stores = await getStores(env as never); + await post(path, { + feedback: [{ ...entry, overrideReason: { reason: { code: "x", system: "urn:x" }, userComment: "z".repeat(9000) } }], + }); + const rows = (await stores.events.listAuditEvents(2000)).filter((e) => e.eventType === "CDS_HOOKS_FEEDBACK_RECEIVED"); + const withComment = rows.reverse().find((r) => (r.payload as Record)["userComment"])!; + const comment = String((withComment.payload as Record)["userComment"]); + assert.ok(comment.length < 9000, `comment was ${comment.length} chars`); + assert.match(comment, /\[truncated\]$/); +}); + test("the auth matrix: discovery is public, invoke and feedback are not", () => { // A pure `authorize` call, not a handler call — the gate runs in the worker before any handler, so // asserting it here is asserting the thing that actually protects the route. diff --git a/backend-ts/src/routes/cds-hooks.ts b/backend-ts/src/routes/cds-hooks.ts index 7ca41e64..46b4d75e 100644 --- a/backend-ts/src/routes/cds-hooks.ts +++ b/backend-ts/src/routes/cds-hooks.ts @@ -87,12 +87,22 @@ function studioBaseUrl(env: CdsHooksEnv): string | undefined { return parseAllowedOrigins(env.WORKWELL_CORS_ALLOWED_ORIGINS)[0]; } +/** + * Write one audit event. Returns whether it succeeded — and the two callers treat that differently, on + * purpose (review). + * + * For **invoke** a failure is best-effort: the cards returned are still correct, and turning a correct + * clinical answer into a 500 because the ledger hiccuped would be the worse outcome. For **feedback** the + * audit event IS the entire persistence (ADR-067 d10 — nothing else is stored, which is why the endpoint + * needed no schema change), so swallowing a failure would make it a silent no-op that told the client not to + * retry. CLAUDE.md's "every state change writes `audit_event` — no exceptions" is the rule that decides it. + */ async function auditCds( env: CdsHooksEnv, actor: string, eventType: "CDS_HOOKS_INVOKED" | "CDS_HOOKS_FEEDBACK_RECEIVED", detail: Record, -): Promise { +): Promise { try { const stores = await getStores(env); await stores.events.appendAudit({ @@ -100,22 +110,29 @@ async function auditCds( entityType: "cds_hooks", entityId: crypto.randomUUID(), actor, - refRunId: (detail["runId"] as string | undefined) ?? null, + // The run the cards came from, so the ledger row is joinable on `ref_run_id` rather than only through + // the payload. Earlier this read `detail["runId"]`, which no caller ever set — a line that looked like + // wiring and could not fire (review). + refRunId: (detail["runId"] as string | null | undefined) ?? null, refCaseId: null, refMeasureVersionId: null, payload: { sensitivityLabel: "restricted", timestamp: new Date().toISOString(), ...detail }, }); + return true; } catch (err) { - // Best-effort at the response boundary: an audit failure must not turn a correct answer into a 500. - console.error(`WORKWELL_ALERT cds-hooks audit write failed: ${String(err)}`); + console.error(`WORKWELL_ALERT cds-hooks audit write failed (${eventType}): ${String(err)}`); + return false; } } /** * The newest FINALIZED outcome per measure for the first subject id that resolves to any row at all. * - * Returns the subject id it used, so the caller can tell "this patient is unknown to WorkWell" from "this - * patient has only mid-run rows" — two absences that must not be reported identically. + * Returns the subject id it used. To the CLIENT both absences render as the same informational card, which + * is deliberate — a clinician does not need our run bookkeeping — but the audit trail distinguishes them + * (`subjectId: null` for an id that resolved to nothing at all, versus a value for a subject whose only rows + * belong to an unfinished run). An earlier version of this comment claimed the two were reported + * differently to the caller; they are not (review). */ async function latestFinalizedByMeasure( env: CdsHooksEnv, @@ -243,11 +260,17 @@ async function invoke( ? [noEvaluationCard(patientId, base)] : await buildComplianceCards(found.rows, { subjectId: found.subjectId, + // The id the CLIENT sent, which is the only one it can act on. See `CardOptions.patientId`. + patientId, approvedOrderCodes: await approvedOrderCodes(env), standingOrders: resolveStandingOrderProvider(env), studioBaseUrl: base, }); + // The runs these cards came from. One value is the overwhelmingly common case (a nightly ALL_PROGRAMS run + // covers every measure), so `refRunId` is set when it is unambiguous and left null when it is not, rather + // than picking one arbitrarily. + const runIds = [...new Set((found?.rows ?? []).map((r) => r.runId))]; await auditCds(env, actor, "CDS_HOOKS_INVOKED", { serviceId, hook: body.hook, @@ -255,14 +278,31 @@ async function invoke( patientId, subjectId: found?.subjectId ?? null, cardCount: cards.length, - // The uuids this invocation emitted. This is what makes a later feedback event correlatable without - // persisting a card table: feedback cites a uuid, this event maps that uuid to a patient and subject, - // and `cardUuid` recomputes the measure from the subject's own outcomes. See `feedback` below. - cardUuids: cards.map((c) => c.uuid).filter((u): u is string => !!u), + runId: runIds.length === 1 ? runIds[0]! : null, + // What makes a later feedback event correlatable without persisting a card table: feedback cites a uuid, + // and this event maps that uuid to the subject AND the run it came from — so recovering the measure is + // one `cardUuid` recomputation per measure, not a search over every run in the subject's history + // (review: without `runId` the documented claim overstated by a step). + cards: cards + .filter((c): c is typeof c & { uuid: string } => !!c.uuid) + .map((c) => ({ uuid: c.uuid, measureId: measureIdOfCard(c, found?.rows ?? []) })), }); return json({ cards }); } +/** + * Which measure a built card describes, for the audit payload only. + * + * Matched on the `source.url` the card already carries (`.../measures/`), falling back to a `summary` + * prefix match on the catalog name. Descriptive: it never changes what the client receives, and a card we + * cannot attribute records `null` rather than a guess. + */ +function measureIdOfCard(card: { source: { url?: string }; summary: string }, rows: readonly CardInput[]): string | null { + const fromUrl = /\/measures\/([^/?#]+)$/.exec(card.source.url ?? "")?.[1]; + if (fromUrl && rows.some((r) => r.measureId === fromUrl)) return fromUrl; + return null; +} + /** * Feedback — the only leg of the send/receive reconciliation in guide S7 that WorkWell can build alone. * @@ -288,6 +328,17 @@ async function feedback( if (!Array.isArray(entries) || entries.length === 0) { return json({ error: "invalid_request", message: "feedback must be a non-empty array" }, 400); } + // Bounded, because each entry is an append to the audit ledger and the caller is a machine credential. + // The spec permits batching "for multiple hook instances or multiple cards at the same time"; it does not + // ask a service to accept an unbounded batch, and one request driving 100k inserts is amplification + // against our own append-only ledger — the hazard the discovery endpoint's no-audit decision avoided one + // route over (review). + if (entries.length > MAX_FEEDBACK_ENTRIES) { + return json( + { error: "invalid_request", message: `feedback accepts at most ${MAX_FEEDBACK_ENTRIES} entries per request` }, + 400, + ); + } for (const e of entries) { if (!e.card || !e.outcomeTimestamp) { return json({ error: "invalid_request", message: "each feedback entry needs card and outcomeTimestamp" }, 400); @@ -307,16 +358,48 @@ async function feedback( } } + let written = 0; for (const e of entries) { - await auditCds(env, actor, "CDS_HOOKS_FEEDBACK_RECEIVED", { + const ok = await auditCds(env, actor, "CDS_HOOKS_FEEDBACK_RECEIVED", { serviceId, card: e.card, outcome: e.outcome, outcomeTimestamp: e.outcomeTimestamp, + acceptedSuggestions: (e.acceptedSuggestions ?? []).map((s) => s.id ?? null), ...(e.overrideReason?.reason?.code ? { overrideReasonCode: e.overrideReason.reason.code } : {}), - ...(e.overrideReason?.userComment ? { userComment: e.overrideReason.userComment } : {}), + // Clinician free text about an encounter. Capped and truncation-marked, matching the 8000-char bound + // AI_GUARDRAILS §2.2 already sets for interpolated untrusted text — and noted in the PHI posture, + // because this is the first path putting unstructured clinical prose into `audit_events` (review). + ...(e.overrideReason?.userComment + ? { userComment: truncateComment(e.overrideReason.userComment) } + : {}), }); + if (!ok) { + // The audit event is the ONLY record of this action, so a failed write must not report success: the + // spec gives feedback no response body, so a 200 tells the client never to retry and the accepted + // order is lost silently. `written` says how much of the batch did land, so a retry is informed. + return json( + { + error: "audit_write_failed", + message: + "feedback could not be recorded — the audit event is the only record of this action, so this " + + "request is reported as failed rather than silently dropped. Retry is safe: feedback is " + + "idempotent by (card, outcome, outcomeTimestamp).", + recorded: written, + of: entries.length, + }, + 503, + ); + } + written++; } // 200 with no body: the spec defines no response payload for feedback. return new Response(null, { status: 200 }); } + +const MAX_FEEDBACK_ENTRIES = 100; +const MAX_USER_COMMENT = 8000; + +function truncateComment(s: string): string { + return s.length <= MAX_USER_COMMENT ? s : `${s.slice(0, MAX_USER_COMMENT)}…[truncated]`; +} diff --git a/backend-ts/src/routes/openapi.test.ts b/backend-ts/src/routes/openapi.test.ts index 8738e8dd..fd3959be 100644 --- a/backend-ts/src/routes/openapi.test.ts +++ b/backend-ts/src/routes/openapi.test.ts @@ -25,6 +25,7 @@ import { createSqliteD1 } from "@mieweb/cloud-local"; import { RUN_STORE_FLOOR_DDL } from "../stores/sqlite/schema.ts"; import { SqliteRunStore } from "../stores/sqlite/run-store-sqlite.ts"; import { SqliteOutcomeStore } from "../stores/sqlite/outcome-store-sqlite.ts"; +import { SqliteValueSetStore } from "../stores/sqlite/value-set-store-sqlite.ts"; import { openApiDocument, type OpenApiSchema } from "../openapi/spec.ts"; import { OPENAPI_PATH } from "./openapi.ts"; import { PATIENT_VIEW_SERVICE_ID } from "../cds/discovery.ts"; @@ -112,6 +113,18 @@ before(async () => { WORKWELL_WEBCHART_API_KEY: "k", } as unknown as Env; + // An APPROVED mapping, so the card this fixture produces carries a `suggestion`. Without it + // `approvedOrderCodes` is empty and the deepest, most drift-prone subschemas in the document — + // `suggestions[].actions[].resource`, and `selectionBehavior` — were never validated against a real + // response, while the test that DOES produce a suggestion does no schema validation (review). + const valueSets = new SqliteValueSetStore(db); + await valueSets.createTerminologyMapping({ + id: crypto.randomUUID(), + localCode: "LOCAL-AUD-002", localDisplay: "Annual audiogram", localSystem: "urn:workwell:demo", + standardCode: "92557", standardDisplay: "Comprehensive audiometry evaluation", + standardSystem: "http://www.ama-assn.org/go/cpt", mappingStatus: "APPROVED", mappingConfidence: 0.98, notes: null, + }); + const runs = new SqliteRunStore(db); const outcomes = new SqliteOutcomeStore(db); const run = await runs.createRun({ @@ -149,17 +162,28 @@ test("the document is a well-formed OpenAPI 3.1 description with no dangling ref const operationIds = new Set(); const tagNames = new Set(doc.tags.map((t) => t.name)); + /** + * Mark everything reachable from the PATHS outward, following `$ref` into its target. + * + * Two corrections from review. (1) Reachability must start at the paths, not at every component: the + * first version walked all component schemas, so a dead schema referencing another marked its target + * "referenced" and a pair of mutually-referencing dead schemas both survived. (2) It must follow a `$ref` + * into the referenced schema, or a schema reachable only through another (CdsService, via + * CdsDiscoveryResponse) reads as dead. + */ const walk = (s: OpenApiSchema): void => { if (s.$ref) { const name = s.$ref.replace("#/components/schemas/", ""); assert.ok(schemaNames.has(name), `dangling $ref: ${s.$ref}`); - referenced.add(name); + if (!referenced.has(name)) { + referenced.add(name); + walk(doc.components.schemas[name]!); // follow it; the guard above makes this cycle-safe + } } Object.values(s.properties ?? {}).forEach(walk); if (s.items) walk(s.items); if (typeof s.additionalProperties === "object") walk(s.additionalProperties); }; - Object.values(doc.components.schemas).forEach(walk); for (const [path, item] of Object.entries(doc.paths)) { // Every `{param}` in the path must be declared, and every declared path param must be in the path. @@ -175,6 +199,9 @@ test("the document is a well-formed OpenAPI 3.1 description with no dangling ref Object.values(r.content ?? {}).forEach((c) => walk(c.schema)), ); Object.values(op.requestBody?.content ?? {}).forEach((c) => walk(c.schema)); + // Parameter schemas too: the first version skipped them, so a dangling `$ref` in a query or path + // parameter passed silently (review). + (op.parameters ?? []).forEach((p) => walk(p.schema)); } } @@ -189,12 +216,35 @@ test("the document is served, publicly, at one canonical path", async () => { const served = (await res.json()) as { openapi: string; paths: Record }; assert.equal(served.openapi, "3.1.1"); assert.deepEqual(Object.keys(served.paths).sort(), Object.keys(doc.paths).sort()); - // Public: readable with no token, because reading a contract should not need credentials. - assert.deepEqual(authorize("GET", OPENAPI_PATH, null), { ok: true }); - // The aliases the journal probed are deliberately NOT served — one canonical URL. - for (const alias of ["/api/openapi.json", "/api/swagger", "/swagger-ui", "/api/docs"]) { - assert.notEqual(authorize("GET", alias, null).ok && alias === OPENAPI_PATH, true); + // Public: readable with no token, because reading a contract should not need credentials. Proved by the + // `probe` above — an unauthenticated request through the real worker — NOT by an `authorize` assertion: + // `handleOpenApi` runs before the auth gate, so no rule participates in the decision (review). + assert.equal(res.status, 200); + + // The aliases the journal probed are deliberately NOT served — one canonical URL. This must assert that + // they are UNROUTED, which means the worker's 501 catch-all. A previous version wrote + // `assert.notEqual(authorize(...).ok && alias === OPENAPI_PATH, true)`, where the `&&` is always false, so + // it passed for every possible implementation — the worst of the new guards, and not one I had + // mutation-checked (review). + // WITH a token, so this proves "not served" rather than merely "not reachable" — the three `/api/*` + // aliases match the AUTHENTICATED `/api/**` tail, so unauthenticated they are 401 and would pass a + // sloppier version of this assertion for the wrong reason. + const cm = await login("cm@workwell.dev"); + for (const alias of ["/api/openapi.json", "/api/swagger", "/api/docs"]) { + const aliasRes = await worker.fetch( + new Request(`http://x${alias}`, { headers: { authorization: `Bearer ${cm}` } }), + env, + ctx, + ); + const body = (await aliasRes.json()) as { error?: string }; + assert.equal(aliasRes.status, 501, `${alias} must not be served`); + assert.equal(body.error, "not_implemented", `${alias} must fall through to the catch-all`); } + // `/swagger-ui` is outside `/api/`, where `authorize` permits by default, so it reaches the catch-all + // even anonymously — asserted separately so that difference is deliberate rather than assumed. + const swaggerUi = await worker.fetch(new Request("http://x/swagger-ui"), env, ctx); + assert.equal(swaggerUi.status, 501); + assert.equal(((await swaggerUi.json()) as { error?: string }).error, "not_implemented"); }); test("every documented status is produced by a real request through the real worker", async () => { diff --git a/docs/CDS_HOOKS.md b/docs/CDS_HOOKS.md index d66edbb0..d6714229 100644 --- a/docs/CDS_HOOKS.md +++ b/docs/CDS_HOOKS.md @@ -21,7 +21,7 @@ POST /cds-services/workwell-compliance-patient-view/feedback → 200 | `serviceId` | path, required | An `id` from discovery. Today there is exactly one. An unknown id is a **404** that lists what exists. | | `hook` | body, required | Must be `patient-view`. A hook this service does not serve is a **400**, not a guess. | | `hookInstance` | body, required | A UUID for this invocation, per the specification. | -| `context.patientId` | body, required | The FHIR `Patient.id`. A bare WebChart id is also tried as `wc|` — see *Subject resolution*. | +| `context.patientId` | body, required | The FHIR `Patient.id`. A bare WebChart id is also tried in the `wc` namespace — see *Subject resolution*. | | `context.userId` | body, optional | `Practitioner/abc` or `PractitionerRole/123`. Recorded, not used for gating. | | `fhirServer`, `fhirAuthorization`, `prefetch` | body, optional | **Accepted and not evaluated.** See *Limits, stated*. | @@ -122,6 +122,18 @@ suggestion with no code change. Two measures sharing one order code (`diabetes_hba1c` and `cms122` both map to CPT 83036) collapse to a single suggestion, because one order is the correct clinical action. +> **The `ServiceRequest` references the id YOU sent, not WorkWell's internal subject id.** On a live tenant +> those differ — we persist `wc|4821`, the hook supplies `4821` — and a reference into the `wc` namespace +> would name nothing the client can resolve (`|` is not a legal FHIR id either). Card identity still uses the +> internal id, so feedback correlation is unaffected. + +### A failed evaluation is reported as ours + +A run can finish `PARTIAL_FAILURE`, and a subject whose evaluation threw is persisted `MISSING_DATA` with an +`evaluationError`. Such a subject gets a card that says the measure **could not be evaluated**, with `info` +and no suggestion — never "no record on file / collect the missing documentation", which would assert a fact +about the patient when the truth is that our engine failed. + ## Feedback `POST /cds-services/{serviceId}/feedback` reports what a clinician did with a card. Optional in the @@ -143,20 +155,34 @@ can build alone. Each entry writes one `CDS_HOOKS_FEEDBACK_RECEIVED` audit event. Nothing else is persisted, and nothing else needed to be: `card.uuid` is derived from `(runId, subjectId, measureId)`, so correlating a uuid back to a -measure is a recomputation over that subject's outcomes rather than a lookup — which is why this endpoint -needs no schema change. The `CDS_HOOKS_INVOKED` event records the uuids it emitted, so the join runs from the -ledger. Deterministic ids also mean a client re-firing the hook for an unchanged run gets the same uuid, so -repeat feedback does not fragment across ids. +measure is a recomputation rather than a lookup — which is why this endpoint needs no schema change. The +`CDS_HOOKS_INVOKED` event records each emitted uuid **with its measure and run**, so the join runs from the +ledger in one step. Deterministic ids also mean a client re-firing the hook for an unchanged run gets the same +uuid, so repeat feedback does not fragment across ids. + +> **A failed write is a `503`, not a silent `200`.** Because the audit event is the *only* record of the +> action, and the specification gives feedback no response body to signal partial success, reporting success +> on a lost write would tell the client never to retry. `recorded` / `of` say how much of the batch landed. +> Retry is safe — feedback is idempotent by `(card, outcome, outcomeTimestamp)`. + +**Bounds.** At most **100** entries per request, and `overrideReason.userComment` is truncated at 8000 +characters (truncation-marked). Each entry is an append to an append-only ledger and the caller is a machine +credential, so an unbounded batch would be amplification against our own audit trail. ## Errors | status | `error` | when | |---|---|---| -| 400 | `invalid_request` | body is not a JSON object; missing `hook`/`hookInstance`/`context.patientId`; a hook this service does not serve; empty `feedback`; an `outcome` other than `accepted`/`overridden`; `accepted` without `acceptedSuggestions` | +| 400 | `invalid_request` | body is not a JSON object; missing `hook`/`hookInstance`/`context.patientId`; a hook this service does not serve; empty `feedback`; more than 100 feedback entries; an `outcome` other than `accepted`/`overridden`; `accepted` without `acceptedSuggestions` | | 401 | `unauthenticated` | no bearer token on invoke or feedback | | 403 | `forbidden` | authenticated, but the role may not invoke a CDS service | | 404 | `unknown_service` | unknown `serviceId`; the response lists the ids that exist | -| 405 | `method_not_allowed` | non-GET on discovery, non-POST on invoke or feedback | +| 405 | `method_not_allowed` | non-POST on invoke or feedback; a non-GET on discovery **when authenticated** — see below | +| 503 | `audit_write_failed` | feedback could not be recorded. The audit event is the only record of the action, so this is reported as failed rather than dropped silently. Carries `recorded` / `of` so a retry is informed; retry is safe. | + +> **Method vs auth precedence on discovery.** `GET /cds-services` is public, but the *path* is gated for every +> other method, so an anonymous `POST /cds-services` is **401**, not 405 — the auth gate runs before the +> handler. An authenticated `POST` there is 405. Stated because the order is not guessable from the table. ## What this promises diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index 9fc4f33f..0e6834a1 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -34,8 +34,9 @@ simultaneously claiming a document it did not serve and omitting the one contrac 1. **Serve a hand-authored OpenAPI 3.1.1 document at one canonical path**, `GET /api/v1/openapi.json`, built by `backend-ts/src/openapi/spec.ts`. PERMIT: reading a contract should not require credentials, and the document carries shapes and role names, not patient data. -2. **Scope is the PROMISED surface** — `/api/v1/compliance`, the three `/cds-services` operations, health - and version — and the document *says so*. The ~40 internal `/api/**` routes are excluded because +2. **Scope is the PROMISED surface** — `/api/v1/compliance`, the three `/cds-services` operations, this + document itself, health and version: **seven operations** — and the document *says so*. The ~40 internal + `/api/**` routes are excluded because `COMPLIANCE_API.md` already draws that line ("everything else under `/api/` is internal and moves with the frontend"), and documenting them would advertise stability over paths that carry none. 3. **Hand-authored, guarded by a contract test.** The alternatives each cost a dependency or a rewrite: zod @@ -43,15 +44,22 @@ simultaneously claiming a document it did not serve and omitting the one contrac have, and TypeSpec would add a second hand-maintained source of truth with no coupling to a hand-rolled dispatcher. The recognised risk of hand-authoring is drift and the recognised answer is a contract test, so the test is treated as the other half of this decision rather than as optional garnish. -4. **The guard is two-way coverage.** Every `(path, method, status)` the document declares is produced by a - real request through the real worker, and every response the tests observe is declared. A documented path - that is not routed fails with `documented but NOT ROUTED`; an undocumented status fails the other - direction. Mutation-checked three ways — deleting the route, removing a produced status, and requiring an - absent property each fail the intended assertion. +4. **The guard is two-way coverage, and the second direction is bounded.** Every `(path, method, status)` the + document declares is produced by a real request through the real worker — that direction is complete, and a + documented path that is not routed fails with `documented but NOT ROUTED`. The reverse direction sees only + statuses some probe produced, so a real status no test exercises is neither documented nor caught: a + path-level `405`, a `500` on a DB outage, a `503` from `startupGuard`. Stating the bound rather than + implying completeness (review). Mutation-checked three ways — deleting the route, removing a produced + status, and requiring an absent property each fail the intended assertion. 5. **Redocly lints the document in CI**, pinned exactly, telemetry off, with **no ignore file**, because it catches a class the contract test cannot. It did so immediately: five uses of `nullable`, which OpenAPI 3.1 removed in favour of type unions. The five remaining warnings are explained in `spec.ts` rather than - silenced. + silenced. **Stated precisely, because "pinned exactly" overstates it:** the *top-level* version is pinned, + but `npx --yes` resolves that package's transitive tree fresh on each run with no lockfile, in a job that + holds repository credentials. Elsewhere this repo pins by SHA-256 and gates on byte-reproducibility. The + trade taken here is a non-reproducible dev tree in exchange for not adding a `package.json` dependency; + if that becomes unacceptable, the alternative is a committed devDependency, not a different linter + (review). 6. **3.1.1, not 3.2.** Renderer support for 3.2 is worse than absent, it is *silent* — Redoc 2.5.3 accepts a 3.2 document by aliasing it to 3.1, so 3.2-only constructs are ignored rather than flagged, and Spectral caps at 3.1. 3.1's Schema Objects are literal JSON Schema 2020-12, which is what makes a zero-dependency @@ -137,8 +145,31 @@ implementation* at runtime would have replaced a working engine with a second on client re-firing the hook for an unchanged run gets the same uuid, so repeat feedback does not fragment. The handler records the uuid verbatim and asserts nothing about what it referred to. +11. **A FAILED evaluation is reported as ours.** `PARTIAL_FAILURE` is terminal, so its rows are served, and a + subject whose evaluation threw is persisted `MISSING_DATA` with an `evaluationError` + (`DATA_MODEL_CONTRACTS.md` §5). `deriveCell` has no branch for that and falls through to "No record on + file", after which `nextActionFor` says "Collect the missing documentation" — asserting a fact about the + **patient** when the truth is that our engine threw. Tolerable on a dashboard; in someone else's chart it + is the same confusion decision 4 exists to prevent. Such a row now gets a "could not be evaluated" card, + `info`, with no suggested order (review). +12. **A suggested resource references the id the CLIENT sent.** `toServiceRequest` writes + `Patient/`, which is right for `GET /api/orders/proposals` and wrong the moment the + resource crosses into an EHR: on a live tenant that is `Patient/wc|4821`, which names nothing the client + can resolve and is not a legal FHIR id. Re-pointed at the hook's `patientId` in the CDS layer only, so + the existing orders surface is unchanged. Card identity still uses the internal id (Codex review). +13. **Feedback fails loudly, and is bounded.** Because the audit event is the only record, a failed write + returns **503** with `recorded`/`of` rather than a `200` that tells the client never to retry — invoke + stays best-effort, since its cards are correct regardless. And a request carries at most 100 entries with + `userComment` capped at 8000 characters, because each entry is an append to the append-only ledger by a + machine credential (review; both reviewers raised the first independently). + **Consequences.** WorkWell can now be pointed at by any conformant CDS client, which changes the joint-call -question from "should we do this?" to "does WebChart speak it?". CORS is **not** relaxed: production keeps its +question from "should we do this?" to "does WebChart speak it?". The card-selection predicate +(`dispositionFor(...) === "OPEN"`) and the order-proposal predicate (`AT_RISK`) must stay the same set, or a +carded measure could claim a proposal created for a different one; a test now pins that equality, since +nothing in either file mentioned the other. `userComment` is the first path putting unstructured clinical +prose into `audit_events`, which reaches `GET /api/audit-events/export` — noted in +`PRODUCTION_READINESS_2026-07.md`. CORS is **not** relaxed: production keeps its exact-origin allowlist, so a browser-based client's origin must be added deliberately — the spec requires CORS support but explicitly declines to specify an allowlist rule. Nothing here is justified by certification: ONC's (b)(11) DSI criterion does not name CDS Hooks. diff --git a/docs/PRODUCTION_READINESS_2026-07.md b/docs/PRODUCTION_READINESS_2026-07.md index f1f8b9df..4f4aa40a 100644 --- a/docs/PRODUCTION_READINESS_2026-07.md +++ b/docs/PRODUCTION_READINESS_2026-07.md @@ -33,6 +33,19 @@ write every record in the database. There is no per-customer isolation, no encry beyond whatever Neon's default tier provides, and no retention policy. That is fine for synthetic data and disqualifying for real patient data. +### New since 2026-08-17: the first unstructured clinical prose in `audit_events` (ADR-067) + +The CDS Hooks feedback endpoint records `overrideReason.userComment` — **free text a clinician typed about a +specific patient encounter** — into `audit_events.payload`, which is reachable through +`GET /api/audit-events/export?format=csv`. Every other payload in that ledger is structured, bounded and +derived (ids, statuses, counts), so this is a category change rather than more of the same, and it is worth +naming here even though the demo stack cannot receive PHI: on a PHI-capable deployment it would be PHI in the +audit trail, and the audit trail is append-only by design. + +Bounded, not solved: the comment is truncated at 8000 characters and the batch at 100 entries. The open +questions for a PHI environment are retention (the ledger has none) and whether the CSV export should redact +it. Neither is a demo-stack concern; both belong in the gap list below. + ### Required environment split A PHI-capable deployment cannot be "the demo stack with real data typed in." It needs to be a **separate diff --git a/frontend/components/api-docs/api-reference.test.tsx b/frontend/components/api-docs/api-reference.test.tsx index 7b07f22e..d81b6102 100644 --- a/frontend/components/api-docs/api-reference.test.tsx +++ b/frontend/components/api-docs/api-reference.test.tsx @@ -36,6 +36,20 @@ const doc: OpenApiDoc = { // No tag at all — this operation must still appear. get: { operationId: "cdsDiscovery", summary: "Discover services", security: [], responses: { "200": { description: "Catalog." } } }, }, + "/cds-services/{serviceId}": { + // A path-level `parameters` key, which OpenAPI allows and which is NOT an operation. Iterating every + // key of a path item would render it as a ghost card with an undefined method (review). + parameters: [{ name: "serviceId", in: "path", required: true, schema: { type: "string" } }], + post: { + operationId: "cdsInvoke", + summary: "Invoke a service", + tags: ["compliance"], + security: [{ bearerAuth: [] }], + parameters: [{ name: "serviceId", in: "path", required: true, schema: { type: "string", example: "svc-1" } }], + requestBody: { required: true, content: { "application/json": { schema: { $ref: "#/components/schemas/Invoke" } } } }, + responses: { "200": { description: "Cards." } }, + }, + } as never, }, components: { schemas: { @@ -47,29 +61,50 @@ const doc: OpenApiDoc = { period: { type: "object", properties: { start: { type: ["string", "null"], description: "ISO-8601 or null." } } }, }, }, + Invoke: { + type: "object", + required: ["hook"], + properties: { hook: { type: "string", description: "The hook name.", example: "patient-view" } }, + }, }, }, }; describe("ApiReference", () => { - it("renders every operation in the document, including untagged ones", () => { + it("renders every operation in the document, including untagged ones, and no ghost operations", () => { render(); - // Two operations exist; both must be on the page. - expect(endpointsOf(doc)).toHaveLength(2); + // Three operations exist across three paths — and the fourth path item key, a path-level `parameters`, + // is NOT one of them. + expect(endpointsOf(doc)).toHaveLength(3); + expect(endpointsOf(doc).map((e) => e.method).sort()).toEqual(["GET", "GET", "POST"]); expect(screen.getByText("Is this subject compliant?")).toBeInTheDocument(); expect(screen.getByText("Discover services")).toBeInTheDocument(); + expect(screen.getByText("Invoke a service")).toBeInTheDocument(); // The untagged one lands under "Other" rather than vanishing. expect(screen.getByText("Other")).toBeInTheDocument(); expect(screen.getByText("compliance")).toBeInTheDocument(); }); + it("renders a request body, following its $ref, and puts it in the curl", () => { + // The real document's two POSTs both carry a requestBody; the original fixture had none, so this + // rendering path was untested (review). + render(); + expect(screen.getByText("Request body")).toBeInTheDocument(); + expect(screen.getByText("The hook name.")).toBeInTheDocument(); + const curl = screen.getByText(/^curl -sS -X POST/); + expect(curl.textContent).toContain(`-d '{"hook":"patient-view"}'`); + expect(curl.textContent).toContain("/cds-services/svc-1"); + }); + it("shows the method, path, auth posture, parameters and status codes", () => { render(); expect(screen.getByText("/api/v1/compliance/{subjectId}/{measureId}")).toBeInTheDocument(); expect(screen.getAllByText("GET").length).toBe(2); - // A bearer-gated operation and a public one must be distinguishable at a glance. - expect(screen.getByText("bearer token")).toBeInTheDocument(); - expect(screen.getByText("public")).toBeInTheDocument(); + expect(screen.getAllByText("POST").length).toBe(1); + // A bearer-gated operation and a public one must be distinguishable at a glance. Two of the three + // operations are gated, so this counts rather than asserting uniqueness. + expect(screen.getAllByText("bearer token")).toHaveLength(2); + expect(screen.getAllByText("public")).toHaveLength(1); expect(screen.getByText("subjectId")).toBeInTheDocument(); expect(screen.getByText("latest | preview")).toBeInTheDocument(); expect(screen.getByText("No finalized outcome.")).toBeInTheDocument(); diff --git a/frontend/components/api-docs/types.ts b/frontend/components/api-docs/types.ts index 50d9243f..8438f179 100644 --- a/frontend/components/api-docs/types.ts +++ b/frontend/components/api-docs/types.ts @@ -55,10 +55,20 @@ export interface Endpoint { op: Operation; } +/** + * The HTTP methods OpenAPI allows in a Path Item. Everything else a path item may carry — + * `parameters`, `summary`, `description`, `servers`, `$ref` — is NOT an operation, and iterating every key + * would render those as ghost operations with an undefined method and summary (review). The real document + * carries none today; a renderer that degrades rather than crashes should not depend on that. + */ +const HTTP_METHODS = new Set(["get", "put", "post", "delete", "options", "head", "patch", "trace"]); + export function endpointsOf(doc: OpenApiDoc): Endpoint[] { const out: Endpoint[] = []; for (const [path, item] of Object.entries(doc.paths ?? {})) { for (const [method, op] of Object.entries(item)) { + if (!HTTP_METHODS.has(method.toLowerCase())) continue; + if (typeof op !== "object" || op === null || typeof op.operationId !== "string") continue; out.push({ path, method: method.toUpperCase(), op }); } } From 9dd808d2834198052bf79dd374bb3d473b4aec55 Mon Sep 17 00:00:00 2001 From: Taleef Date: Mon, 17 Aug 2026 13:43:12 -0400 Subject: [PATCH 6/7] fix(cds): require AcceptedSuggestion.id, as the spec does The feedback check required `acceptedSuggestions` to be present but not that its entries identify anything, so `[{}]` passed and would have been recorded as an accepted suggestion nobody can resolve. The spec makes AcceptedSuggestion.id REQUIRED -- it is the card.suggestion.uuid we emitted -- so this is now enforced, with `[{}]` and `[{"id":""}]` pinned as 400s. Surfaced while replying to a Codex comment that argued acceptedSuggestions should be an array of strings rather than objects. That part is not right -- the spec defines "an array of json objects identifying one or more of the user's AcceptedSuggestions", each with a required `id`, and every example is the object form -- but checking it properly exposed that our runtime was more permissive than the schema we publish. Suite 1976, 0 fail. Co-Authored-By: Claude Opus 5 (1M context) --- backend-ts/src/routes/cds-hooks.test.ts | 4 ++++ backend-ts/src/routes/cds-hooks.ts | 23 +++++++++++++++++------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/backend-ts/src/routes/cds-hooks.test.ts b/backend-ts/src/routes/cds-hooks.test.ts index c1b90f1e..d69e5fbd 100644 --- a/backend-ts/src/routes/cds-hooks.test.ts +++ b/backend-ts/src/routes/cds-hooks.test.ts @@ -250,6 +250,10 @@ test("feedback validates the spec's conditional fields and records the outcome", { feedback: [{ card: uuid, outcome: "declined", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, { feedback: [{ card: uuid, outcome: "accepted", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, { feedback: [{ outcome: "overridden", outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, + // `[{}]` satisfies "the array is present" and identifies nothing. The spec makes AcceptedSuggestion.id + // REQUIRED, so accepting this would record an accepted suggestion nobody can resolve. + { feedback: [{ card: uuid, outcome: "accepted", acceptedSuggestions: [{}], outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, + { feedback: [{ card: uuid, outcome: "accepted", acceptedSuggestions: [{ id: "" }], outcomeTimestamp: "2026-06-12T00:00:00Z" }] }, ]; for (const body of bad) { assert.equal((await post(path, body))!.status, 400, JSON.stringify(body)); diff --git a/backend-ts/src/routes/cds-hooks.ts b/backend-ts/src/routes/cds-hooks.ts index 46b4d75e..07a6ab5d 100644 --- a/backend-ts/src/routes/cds-hooks.ts +++ b/backend-ts/src/routes/cds-hooks.ts @@ -349,12 +349,23 @@ async function feedback( 400, ); } - if (e.outcome === "accepted" && (!Array.isArray(e.acceptedSuggestions) || e.acceptedSuggestions.length === 0)) { - // CONDITIONAL in the spec: acceptedSuggestions is REQUIRED for an `accepted` outcome. - return json( - { error: "invalid_request", message: "acceptedSuggestions is required when outcome is 'accepted'" }, - 400, - ); + if (e.outcome === "accepted") { + // CONDITIONAL in the spec: `acceptedSuggestions` is REQUIRED for an `accepted` outcome, and each + // **AcceptedSuggestion** has a REQUIRED `id` — the `card.suggestion.uuid` we emitted. Checking only + // that the array exists accepted `[{}]`, which carries no information and would be recorded as an + // accepted suggestion nobody can identify (Codex review surfaced this while arguing a different point). + if (!Array.isArray(e.acceptedSuggestions) || e.acceptedSuggestions.length === 0) { + return json( + { error: "invalid_request", message: "acceptedSuggestions is required when outcome is 'accepted'" }, + 400, + ); + } + if (!e.acceptedSuggestions.every((s) => typeof s?.id === "string" && s.id.length > 0)) { + return json( + { error: "invalid_request", message: "each acceptedSuggestions entry requires a non-empty `id`" }, + 400, + ); + } } } From 00fddced51afa9f1919cfe49084cef455c016cbd Mon Sep 17 00:00:00 2001 From: Taleef Date: Mon, 17 Aug 2026 17:21:43 -0400 Subject: [PATCH 7/7] docs: state the outcome-scan latency as a limit, and record the review round CDS_HOOKS.md "Limits, stated" now says an invocation is a bounded but unindexed read rather than a constant-time one, and points at #470 -- an integrator reading the contract should not have to discover that. Names the four other callers that share the scan, so it reads as a pre-existing property rather than a property of this endpoint. JOURNAL records what the review changed: the two findings both reviewers reached independently, the four guards of mine that could not fail, the one Codex comment I declined with the spec text, and the NUL byte a sed run put in source twice. Co-Authored-By: Claude Opus 5 (1M context) --- docs/CDS_HOOKS.md | 8 ++++++++ docs/JOURNAL.md | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/docs/CDS_HOOKS.md b/docs/CDS_HOOKS.md index d6714229..a2cd2fd6 100644 --- a/docs/CDS_HOOKS.md +++ b/docs/CDS_HOOKS.md @@ -201,6 +201,14 @@ eligible for a suggestion — the last of these moves when a terminology mapping request — a different capability, and the piece guide S7 still calls not built. - **Cards are as fresh as the last completed run, not as fresh as this encounter.** They render persisted outcomes of a FINALIZED run; a mid-run row is never served. +- **An invocation is a bounded but unindexed read, not a constant-time one.** Resolving a patient scans that + subject's outcome history (up to 100,000 rows) and parses each row's evidence, then keeps the newest + finalized row per measure. The window has to be that large or an older valid outcome would read as "never + evaluated" — the confusion this whole design exists to prevent — so the fix is a per-measure query in the + store rather than a smaller window. Today a subject accrues roughly one row per measure per nightly run. + Tracked as [#470](https://github.com/Taleef7/workwell/issues/470); the same scan backs + `/api/v1/compliance`, MCP's `check_compliance` and the employee profile, so it is not specific to this + endpoint — but this is the only one of them on an interactive, point-of-care path. - **One hook.** `patient-view` is maturity 5 in the CDS Hooks Library IG; `encounter-start` is maturity 1 and would return the same cards. - **`systemActions` is never emitted.** Nothing WorkWell returns may change a chart without a human choosing diff --git a/docs/JOURNAL.md b/docs/JOURNAL.md index 82cae72b..26324246 100644 --- a/docs/JOURNAL.md +++ b/docs/JOURNAL.md @@ -94,6 +94,53 @@ return the same cards); no `prefetch` evaluation; no OpenAPI path aliases beyond try-it-out console on `/api-docs`, since `swagger-ui-react` peers on `react@">=16.8 <19"` and this app is on React 19 — a copyable `curl` instead. +**The review round changed real things, and two of them were mine to be embarrassed about (#469).** A +code-review pass plus Codex's PR comments produced seven substantive fixes. + +The one both reviewers found independently: **feedback returned `200` when its audit write failed.** +Best-effort auditing is right for *invoke* — the cards are still correct — but for feedback the audit event +IS the persistence, which is the whole reason the endpoint needed no schema change, so a swallowed failure +was a silent no-op that told the client never to retry. Now `503` with `recorded`/`of`. It also collided +verbatim with CLAUDE.md's "every state change writes `audit_event` — no exceptions". + +The one that mattered clinically: **a failed CQL evaluation rendered as a fact about the patient.** +`PARTIAL_FAILURE` is terminal so its rows are served, and a subject whose evaluation threw persists as +`MISSING_DATA` with an `evaluationError` — which `deriveCell` turned into "No record on file" and +`nextActionFor` into "Collect the missing documentation". On a dashboard that is an approximation; in +someone else's chart it is the same confusion `noEvaluationCard` exists to prevent, one layer in. Such a row +now gets a "could not be evaluated" card with no suggested order. + +Codex's best catch: **the suggested `ServiceRequest` referenced `Patient/wc|4821`** — WorkWell's internal +subject id, which names nothing in the client's namespace and is not a legal FHIR id, so the suggestion +could not be applied. Only bites on the deployment the feature exists for. Fixed in the CDS layer alone, +because `GET /api/orders/proposals` shares `toServiceRequest` and the internal id is correct there. + +**Four of my own guards could not fail**, which is worth recording in a change whose selling point is guard +rigour. The worst: `assert.notEqual(authorize(...).ok && alias === OPENAPI_PATH, true)` — the `&&` is always +false, so it passed for every possible implementation, and it was not one I had mutation-checked. Also a +`critical` assertion against a type that permits only two values, an `Array.isArray` that pinned a field name +and nothing else, and a `.find()` that returned the oldest audit event while being named `latest`. + +**One Codex comment I did not act on, and said so on the thread.** It argued `acceptedSuggestions` should be +an array of UUID strings; the spec defines "an array of json objects identifying one or more of the user's +**AcceptedSuggestion**s", each with a REQUIRED `id`, and every example is `[{"id": …}]`. Complying would have +made the published contract non-conformant. But checking it properly found something real underneath — our +runtime accepted `[{}]` — so the `id` is now required, and I had to push a second commit because I had +already *claimed* that in the reply before it was true. + +**And a self-inflicted one worth carrying: `sed -i` over UTF-8 source replaced a space with a literal NUL +byte, twice, and it reached a commit.** It typechecked, all 1,976 tests passed, and the only symptom was +`grep` reporting the file as binary — a signal I saw and dismissed as a bad measurement. It sat in the +separator feeding the card-uuid hash. Fixed, with a six-line test over `src/cds/` that would have caught +both. The rule going forward is simply: do not `sed` UTF-8 source. + +**Deferred with a reason, not forgotten: [#470](https://github.com/Taleef7/workwell/issues/470).** Resolving +a patient scans that subject's entire outcome history and parses each row's evidence. Real, but **not this +change's defect** — five callers share the constant, and the CDS route copied it deliberately; what is new +is a *latency budget* for it. The fix is a per-measure store query with a `store-contract.ts` case, which is +a different review surface, and nothing fires the hook today so it is unobserved. Stated in +`CDS_HOOKS.md` → *Limits, stated* rather than left for an integrator to discover. + --- ## 2026-08-14 — M-E1 leads with immunization, and CDS Hooks turns out to be a specification rather than a dependency