diff --git a/e2e/v137-sharing-managed-profiles.spec.ts b/e2e/v137-sharing-managed-profiles.spec.ts index 116fd643e..ac9373488 100644 --- a/e2e/v137-sharing-managed-profiles.spec.ts +++ b/e2e/v137-sharing-managed-profiles.spec.ts @@ -317,7 +317,7 @@ test.describe.serial("scoped sharing browser journeys", () => { } }); - test("keeps an adult READ grant read-only and outside Settings", async ({ + test("[J1-adult-levels-lifecycle] keeps an adult READ grant read-only and outside Settings", async ({ page, }) => { const record = E2E_LEVEL_RECORDS.find( @@ -344,7 +344,7 @@ test.describe.serial("scoped sharing browser journeys", () => { await leaveRecord(page); }); - test("lets an adult WRITE grant add only to the selected record", async ({ + test("[J1-adult-levels-lifecycle] lets an adult WRITE grant add only to the selected record", async ({ page, }) => { const record = E2E_LEVEL_RECORDS.find( @@ -400,7 +400,7 @@ test.describe.serial("scoped sharing browser journeys", () => { await leaveRecord(page); }); - test("opens MANAGE-only generated reads without opening adult Settings", async ({ + test("[J2-manage-edits-without-settings] opens MANAGE-only generated reads without opening adult Settings", async ({ page, }) => { const fenceHeaders = trackFenceAssertion(page); @@ -435,7 +435,7 @@ test.describe.serial("scoped sharing browser journeys", () => { await leaveRecord(page); }); - test("keeps managed Settings on the active record and integrations status-only", async ({ + test("[J2-manage-edits-without-settings] keeps managed Settings on the active record and integrations status-only", async ({ page, }) => { const record = E2E_LEVEL_RECORDS.find( @@ -492,7 +492,7 @@ test.describe.serial("scoped sharing browser journeys", () => { await expect(banner).toHaveCount(0); }); - test("refuses direct ungranted routes before coach reads start", async ({ + test("[J1-adult-levels-lifecycle] refuses direct ungranted routes before coach reads start", async ({ page, }) => { await openRecord(page, "e2e-scope-measurements"); diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 9701fa010..fa1aeaa28 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -5886,8 +5886,20 @@ model PushAttempt { /// while the database trigger requires it to equal `recipientUserId`. userId String @map("user_id") /// The health-record subject. This supplies calculation context and content. + /// + /// Optional HERE and `NOT NULL` in the database, deliberately, and the two do + /// not disagree: `push_attempt_attribution_guard` is a BEFORE INSERT trigger, + /// so a write that omits both principals has them filled from `userId` before + /// the constraint is checked. Declaring them required would force every + /// established self-delivery writer to name a pair it has no opinion about; + /// making the columns nullable in SQL would let a partial pair reach the + /// table, which the trigger exists to refuse. `prisma migrate diff` reports + /// the difference because it cannot see the trigger — that report is expected, + /// and `notification-attribution.test.ts` ("persistence maps an omitted pair + /// from the legacy recipient") is what proves the accommodation still works. recordUserId String? @map("record_user_id") /// The account whose channel, preferences, and locale receive the delivery. + /// Nullable for the same reason as `recordUserId` above. recipientUserId String? @map("recipient_user_id") channel String // "APNS" | "WEB_PUSH" | "TELEGRAM" | "NTFY" eventType String @map("event_type") // dispatcher event-type string diff --git a/scripts/verify-v137-adversarial.ts b/scripts/verify-v137-adversarial.ts new file mode 100644 index 000000000..fab06f39c --- /dev/null +++ b/scripts/verify-v137-adversarial.ts @@ -0,0 +1,305 @@ +/** + * v1.37.0 integration verifier — the inventories, and whether any of them is empty. + * + * ## What this is for + * + * The release is proved by inventories: a migration allocation, a set of + * admitted mutating handlers, a notification delivery matrix, a set of security + * principals, five accessibility states and eight journeys. Every one of them is + * consumed by a test that iterates it, and every one of those tests passes + * trivially when the list it iterates is empty. That is the failure this script + * exists to catch, and it is not hypothetical — three separate checks in this + * repository have been found green because their matcher matched nothing. + * + * So the rule here is that a count is asserted before it is used, and asserted + * against a number written down rather than against "more than zero" alone. + * A list that lost half its entries is not caught by a non-empty check. + * + * ## What this is NOT + * + * It does not run the suites and it cannot tell whether the tests that consume + * these inventories assert anything worth asserting. It answers one question — + * "is there something in each of these, and is it the amount there is supposed + * to be" — and leaves the rest to the tests themselves. + * + * ## Usage + * + * pnpm dlx tsx scripts/verify-v137-adversarial.ts # everything + * pnpm dlx tsx scripts/verify-v137-adversarial.ts --check-inventories # counts only + * pnpm dlx tsx scripts/verify-v137-adversarial.ts --check-migrations # allocation only + * + * Exits non-zero on the first failing assertion, and prints every check it ran + * so a run that checked nothing is visible as a run that printed nothing. + */ +import { readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { + ACCESSIBILITY_STATES, + SHARING_ACCESSIBILITY_STATES, + V137_E2E_JOURNEYS, +} from "../tests/fixtures/v137/e2e-journeys"; +import { NOTIFICATION_DELIVERY_MATRIX } from "../tests/fixtures/v137/notification-matrix"; +import { SECURITY_PRINCIPALS } from "../tests/fixtures/v137/security-principals"; +import { ADMITTED_MUTATING_HANDLERS } from "../tests/fixtures/v137/sharing-matrix"; + +const ROOT = process.cwd(); + +let failures = 0; +let checks = 0; + +function check(label: string, ok: boolean, detail: string): void { + checks += 1; + if (ok) { + console.log(` ok ${label} — ${detail}`); + return; + } + console.error(` FAIL ${label} — ${detail}`); + failures += 1; +} + +function expectCount(label: string, actual: number, expected: number): void { + check(label, actual === expected, `${actual} (expected ${expected})`); +} + +/* -------------------------------------------------------------------------- */ +/* Migration allocation */ +/* -------------------------------------------------------------------------- */ + +/** + * The release's allocation, corrected. The synthesis said 0296-0299 and called + * the range frozen; the release shipped seven. The list is here so a builder + * inventing an eighth number, or a merge quietly renumbering one of these, + * fails rather than being noticed at the release audit or not at all. + */ +const MIGRATION_ALLOCATION = [ + "0296_account_grant_scope_and_manage", + "0297_managed_profile", + "0298_notification_delivery_identity", + "0299_medication_intake_import_actor_attribution", + "0300_notification_event_created_at_index", + "0301_notification_egress_authorization", + "0302_session_record_epoch", +] as const; + +/** + * Triggers this release installs. Neither is modelled in the Prisma schema, so + * `migrate diff` has nothing to compare them against and cannot report one + * going missing — the drift check is structurally blind here. Their presence in + * the migration text is the cheapest honest signal; the behaviour is proved by + * the integration suites named beside each one. + */ +const MIGRATION_TRIGGERS: ReadonlyArray<{ + trigger: string; + migration: string; + provenBy: string; +}> = [ + { + trigger: "push_attempt_attribution_guard", + migration: "0298_notification_delivery_identity", + provenBy: "tests/integration/notification-attribution.test.ts", + }, + { + trigger: "sessions_record_epoch_bump", + migration: "0302_session_record_epoch", + provenBy: "tests/integration/record-session-epoch-trigger.test.ts", + }, +]; + +function checkMigrations(): void { + console.log("migration allocation"); + const dir = join(ROOT, "prisma/migrations"); + const present = readdirSync(dir, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name); + + check( + "the migrations directory really loaded", + present.length > 100, + `${present.length} migrations on disk`, + ); + + const release = present.filter((name) => /^(029[6-9]|030[0-2])_/.test(name)); + expectCount( + "the release allocates exactly seven", + release.length, + MIGRATION_ALLOCATION.length, + ); + + for (const expected of MIGRATION_ALLOCATION) { + check( + `allocated ${expected.slice(0, 4)}`, + present.includes(expected), + expected, + ); + } + + // Every number used once. A collision is the thing renumbering exists to + // prevent, and two directories sharing a prefix apply in an order nobody + // chose. + const releaseNumbers = release.map((name) => name.slice(0, 4)); + const duplicated = releaseNumbers.filter( + (n, i) => releaseNumbers.indexOf(n) !== i, + ); + check( + "no number is allocated twice in the release range", + duplicated.length === 0, + duplicated.length === 0 ? "none" : duplicated.join(", "), + ); + + for (const { trigger, migration, provenBy } of MIGRATION_TRIGGERS) { + const sql = readFileSync(join(dir, migration, "migration.sql"), "utf8"); + check( + `${migration} installs ${trigger}`, + sql.includes(trigger), + `behaviour proved by ${provenBy}`, + ); + } +} + +/* -------------------------------------------------------------------------- */ +/* Inventories */ +/* -------------------------------------------------------------------------- */ + +function checkInventories(): void { + console.log("inventories"); + + // Handlers. Every admitted mutation is meant to run level, owner, actor, + // effect and encrypted-field cases; an empty list runs none of them and the + // matrix test reports the same green either way. + check( + "the admitted mutating handlers are non-empty", + ADMITTED_MUTATING_HANDLERS.length > 0, + `${ADMITTED_MUTATING_HANDLERS.length} handlers`, + ); + // Keyed on route AND action, because one route legitimately appears more + // than once: an `[id]` route carries an update and a delete, and they are + // different contracts with different admitted levels. Keying on the route + // alone reported eleven false collisions on the first run of this script. + const handlerKeys = new Set( + ADMITTED_MUTATING_HANDLERS.map((h) => `${h.action} ${h.route}`), + ); + expectCount( + "no route and action pair is admitted twice", + handlerKeys.size, + ADMITTED_MUTATING_HANDLERS.length, + ); + check( + "every handler names a module that exists", + ADMITTED_MUTATING_HANDLERS.every((h) => { + try { + return ( + readFileSync(join(ROOT, "src", h.handlerModule), "utf8").length > 0 + ); + } catch { + return false; + } + }), + "each handlerModule resolves under src/", + ); + + // The notification matrix is two records by two Guardians by four channels. + // Written as a product, so a dropped channel shows up here as sixteen minus + // four rather than as a suite that simply ran fewer cases. + expectCount( + "the notification delivery matrix is 2 records x 2 Guardians x 4 channels", + NOTIFICATION_DELIVERY_MATRIX.length, + 16, + ); + check( + "no delivery in the matrix is interactive", + NOTIFICATION_DELIVERY_MATRIX.every((row) => row.interactive === false), + "a Guardian's notification offers no action buttons", + ); + + // Security principals. Each names an actor, a record and a recipient; a + // principal missing one of the three would let a test assert against + // undefined and pass. + const principals = Object.entries(SECURITY_PRINCIPALS); + check( + "the security principals are non-empty", + principals.length > 0, + `${principals.length} principals`, + ); + for (const [name, principal] of principals) { + const { actorUserId, recordUserId, recipientUserId } = principal.identities; + check( + `${name} names all three identities`, + Boolean(actorUserId && recordUserId && recipientUserId), + `${actorUserId} / ${recordUserId} / ${recipientUserId}`, + ); + } + + // Accessibility states and journeys. Both are enumerations the browser suite + // is read back against, and both are exact rather than non-empty: a ninth + // journey nobody counted and a fifth state quietly absorbed into a fourth are + // the two failures the enumeration exists to make visible. + expectCount( + "exactly five accessibility states", + ACCESSIBILITY_STATES.length, + 5, + ); + expectCount( + "every state carries a case", + SHARING_ACCESSIBILITY_STATES.length, + 5, + ); + expectCount("exactly eight journeys", V137_E2E_JOURNEYS.length, 8); + expectCount( + "every journey id is distinct", + new Set(V137_E2E_JOURNEYS.map((j) => j.id)).size, + 8, + ); + + // Each journey id must appear in a test title somewhere under e2e/ or tests/ + // or src/__tests__/. `sharing-accessibility-states.test.tsx` pins WHICH file + // carries each one; this leg is the cheaper cross-check that none of the + // eight has become a claim with no test anywhere at all. + const searched = [ + ...readdirSync(join(ROOT, "e2e")).map((f) => join("e2e", f)), + ...readdirSync(join(ROOT, "tests/integration")).map((f) => + join("tests/integration", f), + ), + ...readdirSync(join(ROOT, "src/__tests__")).map((f) => + join("src/__tests__", f), + ), + ].filter((f) => f.endsWith(".ts") || f.endsWith(".tsx")); + check( + "the test tree really loaded", + searched.length > 50, + `${searched.length} files scanned`, + ); + const corpus = searched + .map((f) => readFileSync(join(ROOT, f), "utf8")) + .join("\n"); + for (const journey of V137_E2E_JOURNEYS) { + check( + `${journey.id} is marked in a test title`, + corpus.includes(`[${journey.id}]`), + journey.claim.slice(0, 60) + "…", + ); + } +} + +/* -------------------------------------------------------------------------- */ + +const argv = process.argv.slice(2); +const only = argv.filter((a) => a.startsWith("--")); +const runMigrations = only.length === 0 || only.includes("--check-migrations"); +const runInventories = + only.length === 0 || only.includes("--check-inventories"); + +if (runMigrations) checkMigrations(); +if (runInventories) checkInventories(); + +// A run that asserted nothing is a run that proves nothing, and it would exit 0. +if (checks === 0) { + console.error("no checks ran — refusing to report success"); + process.exit(1); +} + +console.log( + `\n${checks} check(s), ${failures} failure(s)` + + (failures === 0 ? " — inventories are non-empty and exact" : ""), +); +process.exit(failures === 0 ? 0 : 1); diff --git a/src/__tests__/sharing-accessibility-states.test.tsx b/src/__tests__/sharing-accessibility-states.test.tsx index e68899ec6..bcf8dad58 100644 --- a/src/__tests__/sharing-accessibility-states.test.tsx +++ b/src/__tests__/sharing-accessibility-states.test.tsx @@ -53,21 +53,46 @@ const CONTRAST = "expectContentContrast"; /** * Split a spec into `title -> body`. * - * Bodies are delimited by the next `test(` rather than by brace matching: a + * Bodies are delimited by the next opener rather than by brace matching: a * body containing a string with a brace in it is common, a body containing the * literal `test(` is not. Exported and exercised against a synthetic spec * below, because a splitter that returned an empty map would make every leg * here pass by matching nothing. + * + * `it(` counts as an opener alongside `test(`. Both are the same call, and the + * repository splits by suite rather than by preference: the browser specs + * write `test(`, the Vitest files write `it(`. Five of the eight release + * journeys are proved by Vitest files, so a parser that saw only `test(` would + * read those files as having no tests at all — and every per-file leg below is + * over a matched set, so it would have passed on the empty one. + * + * ## What it does not see, said plainly + * + * A title written as a template literal, because the extractor takes only + * double-quoted strings — several parameterised cases in the sharing suites are + * written that way, and a marker placed in one would be invisible here. + * And a parameterised opener whose argument list contains its own `)` + * (`it.each([{ a: f() }])`), because the table is skipped by a non-nesting + * match. Both are limits of the matcher rather than statements about the + * suites, so a journey is wired to a plainly-quoted title on purpose. */ export function testBodies(source: string): Map { const bodies = new Map(); // `test.only(` and `test.skip(` are tests; `test.describe(` is not, and the // synthetic spec below is what caught the first version treating it as one. - const opener = /\btest(?:\.(?!describe)\w+)*\(\s*"([^"]+)"/g; + // `\b` keeps `it` from matching inside a word — `submit(`, `exit(`, `await(` + // — because neither side of the pair is a boundary there. + // + // Two shapes, because `it.each(table)("title")` puts the table between the + // opener and the title and the single-call form does not. The synthetic + // Vitest spec below is what caught the first version seeing only the second + // shape and reading a parameterised case as no case at all. + const opener = + /\b(?:test|it)(?:\.(?!describe)\w+)*\s*(?:\(\s*"([^"]+)"|\([^)]*\)\s*\(\s*"([^"]+)")/g; const starts: Array<{ title: string; from: number }> = []; let match: RegExpExecArray | null; while ((match = opener.exec(source)) !== null) { - starts.push({ title: match[1], from: match.index }); + starts.push({ title: match[1] ?? match[2], from: match.index }); } for (let i = 0; i < starts.length; i += 1) { const to = i + 1 < starts.length ? starts[i + 1].from : source.length; @@ -103,6 +128,40 @@ describe("the state parser sees what it looks for", () => { it("does not treat a describe block as a test", () => { expect(testBodies(SYNTHETIC).has("a group")).toBe(false); }); + + const SYNTHETIC_VITEST = ` + import { describe, expect, it } from "vitest"; + + describe("a suite", () => { + it("answers the first question", async () => { + const submitted = await submit({ a: 1 }); + expect(submitted).toBe(true); + }); + + it.each([1, 2])("answers the parameterised one", async () => { + expect(await exit()).toBe(0); + }); + }); + `; + + it("reads a Vitest file's it( openers too", () => { + // Five of the eight journeys are proved by Vitest files. Without this the + // wiring leg below would read every one of them as a file with no tests, + // and pass on the empty set. + const bodies = testBodies(SYNTHETIC_VITEST); + expect([...bodies.keys()]).toEqual([ + "answers the first question", + "answers the parameterised one", + ]); + }); + + it("does not find a test inside submit( or exit(", () => { + // The bodies above call both. If the word boundary were wrong, the map + // would carry entries the file never declared, and the wiring leg could be + // satisfied by a marker sitting in a helper call rather than in a title. + expect(testBodies(SYNTHETIC_VITEST).size).toBe(2); + expect(testBodies(SYNTHETIC_VITEST).has("a suite")).toBe(false); + }); }); describe("the five states are enumerated once each", () => { @@ -229,45 +288,174 @@ describe("the eight release journeys are enumerated", () => { }); /** - * v1.37.0 — the ids that are wired to a test title, and only those. + * v1.37.0 — every journey, and the files that carry it. * * The enumeration above says eight journeys exist. It says nothing about * whether any of them RAN, which is the same gap between "the suite is * green" and "the suite covers this" that the rest of this file exists to - * close. Wiring all eight is the integration plan's item; this list is the - * subset already claimed by a spec, and it grows as they land. + * close. All eight are wired now, so the list is no longer a growing subset: + * a journey missing from it fails below rather than reading as one nobody + * has got to yet. * - * An id on this list whose spec loses the marker fails here rather than - * disappearing quietly from a run nobody counted. + * ## Why a journey names more than one file + * + * A journey is a claim, not a test. J1 says an adult grant runs "through + * invite, accept, switch, expiry, revoke and direct URL" — the browser + * proves the switch, what each level opens and the direct-URL refusal; the + * lifecycle transitions either side of it are database facts and are proved + * against real Postgres. Forcing that onto one file would mean either a + * browser spec driving grant expiry through the clock, or an integration + * file asserting what a page renders. Both would be worse tests than the two + * that exist, so the wiring records both homes. + * + * ## Why the runner is written down + * + * Three journeys ride Playwright and run only in the acceptance stage; five + * are Vitest and run in the ordinary gate. That split is the same one + * `record-session-fence-acceptance-map.test.ts` records for the fence, and + * it is here for the same reason: a green run of THIS file means "all eight + * are claimed, and here is which ones have actually executed", never "all + * eight passed". The runner is checked against the path rather than trusted, + * so it cannot drift into a label that flatters the split. + * + * ## What this cannot do + * + * It proves a marker appears in a test title in a file that really has + * tests. It cannot prove the test behind the marker asserts anything worth + * asserting. Ten empty tests would satisfy it, which is why the journey + * claims stay written out above in the release's own words: the marker says + * where to look, and the claim says what to look for. */ - const WIRED_JOURNEYS: ReadonlyArray<{ id: string; spec: string }> = [ + const WIRED_JOURNEYS: ReadonlyArray<{ + id: string; + proofs: ReadonlyArray<{ file: string; runner: "playwright" | "vitest" }>; + }> = [ + { + id: "J1-adult-levels-lifecycle", + proofs: [ + { + file: "e2e/v137-sharing-managed-profiles.spec.ts", + runner: "playwright", + }, + { + file: "tests/integration/sharing-lifecycle.test.ts", + runner: "vitest", + }, + ], + }, + { + id: "J2-manage-edits-without-settings", + proofs: [ + { + file: "e2e/v137-sharing-managed-profiles.spec.ts", + runner: "playwright", + }, + { + file: "tests/integration/sharing-manage-handler-matrix.test.ts", + runner: "vitest", + }, + ], + }, { id: "J3-managed-profile-lifecycle", - spec: "e2e/v137-sharing-managed-profiles.spec.ts", + proofs: [ + { + file: "e2e/v137-sharing-managed-profiles.spec.ts", + runner: "playwright", + }, + ], + }, + { + id: "J4-guardian-notification-matrix", + proofs: [ + { + file: "tests/integration/guardian-notification-fanout.test.ts", + runner: "vitest", + }, + ], + }, + { + id: "J5-revoked-idempotent-replay", + proofs: [ + { + file: "tests/integration/sharing-idempotency-revocation.test.ts", + runner: "vitest", + }, + ], + }, + { + id: "J6-no-delegated-provider-egress", + proofs: [ + { + file: "tests/integration/sharing-provider-origin.test.ts", + runner: "vitest", + }, + ], + }, + { + id: "J7-durable-import-actor", + proofs: [ + { + file: "tests/integration/sharing-import-attribution.test.ts", + runner: "vitest", + }, + ], + }, + { + id: "J8-old-client-compatibility", + proofs: [ + { + file: "src/__tests__/sharing-legacy-client-contract.test.ts", + runner: "vitest", + }, + ], }, ]; - it("claims only ids the enumeration actually declares", () => { - // A wiring entry naming a journey nobody enumerated would be a claim about - // nothing, and would make the leg below pass against a marker that means - // no more than the string it is. - expect(WIRED_JOURNEYS.length).toBeGreaterThan(0); - const declared = new Set(V137_E2E_JOURNEYS.map((j) => j.id)); - for (const { id } of WIRED_JOURNEYS) { - expect(declared, id).toContain(id); + it("wires every enumerated journey, and claims no other", () => { + // Both directions. A wiring entry naming a journey nobody enumerated would + // be a claim about nothing; an enumerated journey missing from the wiring + // is the dropped one this whole file exists to surface, and before this + // release the list was allowed to be a subset, so a drop and a not-yet + // looked identical. + const declared = V137_E2E_JOURNEYS.map((j) => j.id).sort(); + const wired = WIRED_JOURNEYS.map((j) => j.id).sort(); + expect(wired).toEqual(declared); + expect(new Set(wired).size).toBe(wired.length); + }); + + it("splits the runners honestly, and by the path rather than the label", () => { + const all = WIRED_JOURNEYS.flatMap((j) => j.proofs); + // Non-zero on both sides: a split with nothing on one of them would let + // "which of these have actually executed in the gate" be answered by a + // list that never distinguished anything. + const browser = all.filter((p) => p.runner === "playwright"); + const gate = all.filter((p) => p.runner === "vitest"); + expect(browser.length).toBeGreaterThan(0); + expect(gate.length).toBeGreaterThan(0); + for (const proof of all) { + // A Playwright spec lives under e2e/ and nothing else does. Deriving the + // expected runner from the path is what stops the field from becoming a + // label somebody set to whichever value made the count read better. + const expected = proof.file.startsWith("e2e/") ? "playwright" : "vitest"; + expect(proof.runner, proof.file).toBe(expected); } }); - it.each(WIRED_JOURNEYS)("$id is carried by a test title", ({ id, spec }) => { - const source = readFileSync(join(ROOT, spec), "utf8"); - // Non-zero proof: the spec really loaded and really has tests, so a path + it.each( + WIRED_JOURNEYS.flatMap(({ id, proofs }) => + proofs.map((p) => ({ id, ...p })), + ), + )("$id is carried by a test title in $file", ({ id, file }) => { + const source = readFileSync(join(ROOT, file), "utf8"); + // Non-zero proof: the file really loaded and really has tests, so a path // that stopped existing fails here rather than matching nothing quietly. expect(source.length).toBeGreaterThan(1000); const titles = [...testBodies(source).keys()]; - expect(titles.length).toBeGreaterThan(1); + expect(titles.length, `${file} parsed to no tests`).toBeGreaterThan(1); expect( titles.filter((title) => title.includes(`[${id}]`)).length, - `no test title in ${spec} carries [${id}]`, + `no test title in ${file} carries [${id}]`, ).toBeGreaterThan(0); }); }); diff --git a/src/__tests__/sharing-legacy-client-contract.test.ts b/src/__tests__/sharing-legacy-client-contract.test.ts index ff3beb419..177565293 100644 --- a/src/__tests__/sharing-legacy-client-contract.test.ts +++ b/src/__tests__/sharing-legacy-client-contract.test.ts @@ -93,7 +93,7 @@ beforeEach(() => { }); describe("what an older client makes of the v1.37.0 account payload", () => { - it("covers every fixture case, and the fixtures describe the server", async () => { + it("[J8-old-client-compatibility] covers every fixture case, and the fixtures describe the server", async () => { // Non-zero discovery, and the assertion that keeps the fixture honest: it // is a description of what the resolver emits, not a parallel invention. expect(LEGACY_ACCOUNT_PAYLOADS.length).toBeGreaterThan(0); @@ -130,7 +130,7 @@ describe("what an older client makes of the v1.37.0 account payload", () => { }, ); - it("refuses a scoped grant rather than reading it as the whole record", async () => { + it("[J8-old-client-compatibility] refuses a scoped grant rather than reading it as the whole record", async () => { const scoped = LEGACY_ACCOUNT_PAYLOADS.find( (fixture) => fixture.sections !== null, ); @@ -171,7 +171,7 @@ describe("what the canonical readers make of the same payload", () => { }, ); - it("reads a scope the server could not parse as nothing, on both ends", async () => { + it("[J8-old-client-compatibility] reads a scope the server could not parse as nothing, on both ends", async () => { // The fail-closed reading: a stored scope this build cannot make sense of // resolves to an EMPTY section list rather than to the whole record. The // server refuses every section for it, and the client has to agree — an diff --git a/tests/integration/guardian-notification-fanout.test.ts b/tests/integration/guardian-notification-fanout.test.ts index efc49c218..c23f51583 100644 --- a/tests/integration/guardian-notification-fanout.test.ts +++ b/tests/integration/guardian-notification-fanout.test.ts @@ -212,7 +212,7 @@ beforeEach(async () => { }); describe("Guardian fan-out (real Postgres)", () => { - it("isolates recipient locale and global preference across two Guardians and two records", async () => { + it("[J4-guardian-notification-matrix] isolates recipient locale and global preference across two Guardians and two records", async () => { const [recordA, recordB, guardianEn, guardianDe] = await Promise.all([ createUser({ label: "record-a", locale: "en", managed: true }), createUser({ label: "record-b", locale: "de", managed: true }), @@ -319,7 +319,7 @@ describe("Guardian fan-out (real Postgres)", () => { ); }); - it("writes the complete two-record, two-Guardian, four-channel attribution matrix", async () => { + it("[J4-guardian-notification-matrix] writes the complete two-record, two-Guardian, four-channel attribution matrix", async () => { const [recordA, recordB, guardianOne, guardianTwo, ordinaryRecord] = await Promise.all([ createUser({ label: "record-a", locale: "en", managed: true }), diff --git a/tests/integration/medication-reminder-cross-midnight.test.ts b/tests/integration/medication-reminder-cross-midnight.test.ts index c67dc2e85..64a4072d7 100644 --- a/tests/integration/medication-reminder-cross-midnight.test.ts +++ b/tests/integration/medication-reminder-cross-midnight.test.ts @@ -120,17 +120,23 @@ describe("medication reminder cross-midnight persistence", () => { }), ); - const anchors = await prisma.pushAttempt.findMany({ + // The anchor is a record-scoped scheduler event, not a delivery attempt. + // It used to be a `push_attempts` row on a synthetic `DEDUP` channel; the + // separation moved it to `notification_events` and nothing writes that + // channel any more, so a query for it matched no rows and passed on an + // empty set whatever the tick did. Read the ledger the claim actually + // appends to, and pin the count, so a second dispatch shows up as a + // second row rather than as silence. + const anchors = await prisma.notificationEvent.findMany({ where: { - userId: USER_ID, - channel: "DEDUP", + recordUserId: USER_ID, eventType: "MEDICATION_REMINDER", }, - select: { reason: true }, + select: { dedupKey: true }, }); expect(anchors).toEqual([ { - reason: expect.stringContaining("23:45:YELLOW:2026-07-28"), + dedupKey: expect.stringContaining("23:45:YELLOW:2026-07-28"), }, ]); }); diff --git a/tests/integration/sharing-account-access.test.ts b/tests/integration/sharing-account-access.test.ts index bb0daef58..a14fb0f69 100644 --- a/tests/integration/sharing-account-access.test.ts +++ b/tests/integration/sharing-account-access.test.ts @@ -70,8 +70,14 @@ interface AccountAccessEntry { accountId: string; username: string; displayName: string | null; - access: "read" | "write" | "manage"; + /** + * The legacy name, and deliberately narrower than `level`: it carries only + * `read` and `write`, because that is the union v1.36.0 published and + * shipped clients still decode. A MANAGE grant is a `write` here. + */ + access: "read" | "write"; level: "read" | "write" | "manage"; + recordKind: "shared" | "managed"; sections: string[] | null; canWrite: boolean; } @@ -79,6 +85,7 @@ interface AccountAccessEntry { interface AccountAccessBlock { accounts: AccountAccessEntry[]; active: AccountAccessEntry | null; + recordKind: "self" | "shared" | "managed"; canSwitch: boolean; } @@ -146,6 +153,9 @@ describe("accountAccess — what the payload publishes", () => { expect(accountAccess).toEqual({ accounts: [], active: null, + // The record in view is this account's own, and the block says so + // rather than leaving the client to infer it from `active` being null. + recordKind: "self", canSwitch: false, }); }); @@ -165,6 +175,10 @@ describe("accountAccess — what the payload publishes", () => { displayName: owner.displayName, access: "read", level: "read", + // An adult who shared their own record. `managed` is reserved for a + // profile that has no login of its own, and the two are not + // interchangeable on any surface that renders a name. + recordKind: "shared", sections: null, canWrite: false, }, @@ -260,11 +274,21 @@ describe("accountAccess — what the payload publishes", () => { expect(accountAccess.accounts[0].sections).toEqual([]); }); - it("publishes a manage grant as manage, and as writable", async () => { + it("publishes a manage grant as manage on level, and as write on the legacy name", async () => { // The shipped expression was `access === "WRITE" ? "write" : "read"`, // which would have published a grant that can delete entries as read-only // — the payload understating access is the direction that reads as safe // and is not, because the server would have gone on admitting the writes. + // + // The correction is on `level`, and only there. `access` is the v1.36.0 + // name, and its union is `read | write`: a client built against that + // release decodes it as a closed set and fails on a third value it has + // never seen. So MANAGE publishes as `write` on the legacy name — an + // understatement a shipped client can act on, beside the exact answer on + // `level` for a client that knows to read it. The narrowing is enforced + // rather than incidental: `accountAccessEntrySchema` raises an issue + // unless a `manage` entry carries exactly legacy write access and whole + // scope (`src/lib/sharing/account-access-schema.ts`). const owner = await makeUser("owner"); const delegate = await makeUser("delegate"); await grantAccess(owner.id, delegate.id, { access: "MANAGE" }); @@ -273,9 +297,10 @@ describe("accountAccess — what the payload publishes", () => { const { accountAccess } = await readMe(); expect(accountAccess.accounts[0].level).toBe("manage"); - expect(accountAccess.accounts[0].access).toBe("manage"); + expect(accountAccess.accounts[0].access).toBe("write"); expect(accountAccess.accounts[0].canWrite).toBe(true); expect(accountAccess.accounts[0].sections).toBeNull(); + expect(accountAccess.accounts[0].recordKind).toBe("shared"); }); it("never lists a grant this account GAVE — the block is about what it may open", async () => { @@ -311,12 +336,17 @@ describe("accountAccess — the active record", () => { displayName: owner.displayName, access: "read", level: "read", + recordKind: "shared", sections: null, canWrite: false, }); // The active entry is one of the listed ones, by construction. A banner // that had to join the two could render unnamed when they disagreed. expect(accountAccess.accounts).toContainEqual(accountAccess.active); + // The block-level kind moves with the switch. It reads `self` in the + // empty case above, so a value stuck on one of the two would show up as + // one of these two cases disagreeing with the session. + expect(accountAccess.recordKind).toBe("shared"); }); it("reads as not-switched when the stamp outlives the grant", async () => { diff --git a/tests/integration/sharing-idempotency-revocation.test.ts b/tests/integration/sharing-idempotency-revocation.test.ts index 50cd3114f..b944ae5bc 100644 --- a/tests/integration/sharing-idempotency-revocation.test.ts +++ b/tests/integration/sharing-idempotency-revocation.test.ts @@ -146,7 +146,7 @@ beforeEach(async () => { }); describe("delegated idempotency replay", () => { - it("replays an active delegated retry without a second mutation", async () => { + it("[J5-revoked-idempotent-replay] replays an active delegated retry without a second mutation", async () => { const principal = SECURITY_PRINCIPALS.activeDelegation; await createUser(principal.identities.actorUserId); await createUser(principal.identities.recordUserId); @@ -177,7 +177,7 @@ describe("delegated idempotency replay", () => { ).toBe(1); }); - it("refuses a revoked delegated retry without returning the cached DTO", async () => { + it("[J5-revoked-idempotent-replay] refuses a revoked delegated retry without returning the cached DTO", async () => { const principal = SECURITY_PRINCIPALS.revokedDelegation; await createUser(principal.identities.actorUserId); await createUser(principal.identities.recordUserId); diff --git a/tests/integration/sharing-import-attribution.test.ts b/tests/integration/sharing-import-attribution.test.ts index ae4190304..d832ebf09 100644 --- a/tests/integration/sharing-import-attribution.test.ts +++ b/tests/integration/sharing-import-attribution.test.ts @@ -47,7 +47,7 @@ beforeEach(async () => { }); describe("shared intake import attribution", () => { - it("retains the record and manager through one durable completion audit", async () => { + it("[J7-durable-import-actor] retains the record and manager through one durable completion audit", async () => { const prisma = getPrismaClient(); await prisma.user.createMany({ data: [ @@ -110,7 +110,7 @@ describe("shared intake import attribution", () => { }); }); - it("lets the initiating manager poll the record-scoped job", async () => { + it("[J7-durable-import-actor] lets the initiating manager poll the record-scoped job", async () => { const prisma = getPrismaClient(); await prisma.user.createMany({ data: [ diff --git a/tests/integration/sharing-lifecycle.test.ts b/tests/integration/sharing-lifecycle.test.ts index beeec56d8..efb0909c1 100644 --- a/tests/integration/sharing-lifecycle.test.ts +++ b/tests/integration/sharing-lifecycle.test.ts @@ -241,7 +241,7 @@ beforeEach(async () => { }); describe("the handshake, end to end", () => { - it("carries a household from invitation to revocation and leaves the right rows", async () => { + it("[J1-adult-levels-lifecycle] carries a household from invitation to revocation and leaves the right rows", async () => { const owner = await makeUser("owner"); const delegate = await makeUser("delegate"); await seedWeight(owner.id, 81); @@ -741,7 +741,7 @@ describe("grant management cannot be reached from inside a switch", () => { }); describe("the switch endpoint", () => { - it("refuses a switch into a grant that is not active", async () => { + it("[J1-adult-levels-lifecycle] refuses a switch into a grant that is not active", async () => { const owner = await makeUser("owner"); const delegate = await makeUser("delegate"); await signIn(owner.id); diff --git a/tests/integration/sharing-manage-handler-matrix.test.ts b/tests/integration/sharing-manage-handler-matrix.test.ts index 05cfca8c1..e5b4f70b0 100644 --- a/tests/integration/sharing-manage-handler-matrix.test.ts +++ b/tests/integration/sharing-manage-handler-matrix.test.ts @@ -1912,7 +1912,7 @@ proveManageRoute("POST /api/export/health-record", { /* -------------------------------------------------------------------------- */ describe("what a destruction leaves behind", () => { - it("C3: a hard delete files what it destroyed, and never the encrypted text", async () => { + it("[J2-manage-edits-without-settings] C3: a hard delete files what it destroyed, and never the encrypted text", async () => { // The preventive-care reminder is the sharpest case in the tree: the row // is gone, no restore route reaches it, and the audit row used to say // `{ reminderId }` — a pointer at nothing. An owner reading their feed @@ -1983,7 +1983,7 @@ describe("what a destruction leaves behind", () => { expect(row.details ?? "").not.toContain("reaction"); }); - it("C4: an edit files the field family and the value that is gone", async () => { + it("[J2-manage-edits-without-settings] C4: an edit files the field family and the value that is gone", async () => { const owner = await makeUser("owner"); const manager = await makeUser("manager"); const measurement = await makeMeasurement(owner.id, 80); diff --git a/tests/integration/sharing-provider-origin.test.ts b/tests/integration/sharing-provider-origin.test.ts index 6f953c00c..798ce3893 100644 --- a/tests/integration/sharing-provider-origin.test.ts +++ b/tests/integration/sharing-provider-origin.test.ts @@ -154,7 +154,7 @@ beforeEach(async () => { }); describe("sharing provider origin", () => { - it("enqueues and dispatches an owner-origin mutation as the positive control", async () => { + it("[J6-no-delegated-provider-egress] enqueues and dispatches an owner-origin mutation as the positive control", async () => { await createUser("provider-owner"); headerJar.set( "authorization", @@ -197,7 +197,7 @@ describe("sharing provider origin", () => { }); }); - it("enqueues and dispatches no provider work for a delegate, including a mutation-to-red payload", async () => { + it("[J6-no-delegated-provider-egress] enqueues and dispatches no provider work for a delegate, including a mutation-to-red payload", async () => { await createUser("provider-record"); await createUser("provider-delegate"); await grantManage({