diff --git a/src/lib/__tests__/caller.test.ts b/src/lib/__tests__/caller.test.ts index 3c4212a..52e7388 100644 --- a/src/lib/__tests__/caller.test.ts +++ b/src/lib/__tests__/caller.test.ts @@ -120,20 +120,73 @@ describe("attribution", () => { }); }); -/* ── The guard is on the class, and only on the class ───────────────────── */ +/* ── The guard is on the class, and only on the class ─────────────────── */ + +/** + * Every comparison against a permission set in a file, with the argument it was + * handed. + * + * This is a text scan, so it reads comments as well as code and cannot tell the + * difference. Nothing in this file may spell the pattern out in prose — doing so + * makes the guard report itself, which is a failure that looks exactly like a + * real one and wastes the next reader's afternoon. + */ +const COMPARISON = /\.actor\b\s*\.\s*includes\(([^)]*)\)/g; + +/** + * Whether one such comparison is testing something that is not a class. + * + * `transition.actor` is a PERMISSION SET — a list of classes. Two things may + * legitimately be tested against it, and they are both classes: + * + * - a caller's `kind`, which is the runtime check `applyAction` performs; + * - a class named outright, which is how a caller asks a question ABOUT the + * permission set rather than about a caller. `personActionsFor` does this: + * "which actions may a person fire" is precisely what a permission set is + * for, and the answer drives which buttons exist. + * + * What is forbidden is comparing an IDENTITY against it — a user id, an agent + * name, any string naming who rather than which class. That guard answers for + * the strings it lists and has to guess for the rest, so an unlisted caller + * either gets in or is locked out for having a new name, and a caller that picks + * a listed name gets in on the strength of the name alone. F4 exists because + * `actor` conflated the two; a check that cannot tell them apart either is the + * same defect one level up. + * + * The permitted literals are read off the registry rather than written here, so + * a third class added to `vocabulary.json` is permitted the moment it is + * declared, and an agent name — `"checkpoint"`, `"grouping"`, `"migration"` — + * is never one of them. + * + * A class held in a variable is still flagged. That is deliberate: it is rare, + * it is indistinguishable from an identity by reading, and the two call sites in + * this codebase both have a better form available. Erring tight on a check like + * this costs an author one line of justification; erring loose costs the guard. + */ +function comparesANonClass(argument: string): boolean { + const text = argument.trim(); + if (/\.kind\b/.test(text)) return false; + return !REGISTRY_CLASSES.some((className) => text === `"${className}"` || text === `'${className}'`); +} describe("no identity string is ever compared against a permission list", () => { + it("knows an identity from a class", () => { + // The guard was narrowed in R2, so what it still catches is asserted rather + // than assumed. A check that was loosened without this is a check nobody can + // tell from a deleted one. + expect(comparesANonClass("options.by.kind")).toBe(false); + expect(comparesANonClass("by.kind")).toBe(false); + for (const className of REGISTRY_CLASSES) { + expect(comparesANonClass(`"${className}"`), `${className} is a class the registry declares`).toBe(false); + } + // Identities, every one of which the old field could hold and none of which + // is a class. + for (const identity of ['"checkpoint"', '"grouping"', '"migration"', '"rae@webflow.com"', "rec.owner", "userId", "entry.by.userId"]) { + expect(comparesANonClass(identity), `${identity} is an identity, not a class`).toBe(true); + } + }); + it("is true of every file under src/", () => { - /** - * The shape this chunk exists to prevent: a guard that asks whether an - * identity string is in a list of permitted names. It answers for the - * strings it lists and has to guess for the rest, so an unlisted caller - * either gets in or is locked out for having a new name — and a caller that - * picks a listed name gets in on the strength of the name alone. - * - * `transition.actor` may only ever be tested against a caller's `kind`. - * This reads every source file and checks that. - */ const files: string[] = []; const walk = (dir: string) => { for (const entry of readdirSync(dir, { withFileTypes: true })) { @@ -147,8 +200,8 @@ describe("no identity string is ever compared against a permission list", () => const offenders: string[] = []; for (const file of files) { const source = readFileSync(file, "utf8"); - for (const match of source.matchAll(/\.actor\b\s*\.\s*includes\(([^)]*)\)/g)) { - if (!/\.kind\b/.test(match[1])) offenders.push(`${path.relative(srcDir, file)}: ${match[0]}`); + for (const match of source.matchAll(COMPARISON)) { + if (comparesANonClass(match[1])) offenders.push(`${path.relative(srcDir, file)}: ${match[0]}`); } } expect(offenders).toEqual([]); diff --git a/src/lib/__tests__/digest-arrival.test.ts b/src/lib/__tests__/digest-arrival.test.ts index c30c526..742bcca 100644 --- a/src/lib/__tests__/digest-arrival.test.ts +++ b/src/lib/__tests__/digest-arrival.test.ts @@ -21,6 +21,7 @@ import { type IssueCase, type IssueState, } from "../issue-case"; +import type { Caller } from "../caller"; import { recordCheckpointReading } from "../checkpoint-evaluation"; import { normalizePerformanceThresholds } from "../performanceThresholds"; import { ISSUE_TRANSITIONS, QUEUES, WORK_STATES, type IssueAction } from "../vocabulary"; @@ -43,6 +44,15 @@ const AT = "2026-08-25T06:00:00.000Z"; const DATE = "2026-08-25"; const APP = "https://watch.example.com/page-watch"; +/** + * Whoever walked the case down the lifecycle below. + * + * The address is the case's id and nothing else, so who moved it is exactly the + * kind of fact the link is not allowed to carry. This exists to satisfy the + * transition guard and is never asserted on. + */ +const PERSON: Caller = { kind: "person", userId: "rae@webflow.com" }; + function caseOf(overrides: Partial = {}): IssueCase { return { id: "PW-2291", @@ -129,9 +139,9 @@ describe("the case route", () => { */ const link = digestOf([caseOf()]).sections[0].lines[0].href; let issue = caseOf(); - issue = accept(issue, { actor: "person", at: AT }); - issue = start(issue, { actor: "person", at: AT }); - issue = markFixed(issue, { actor: "person", at: AT }); + issue = accept(issue, { by: PERSON, at: AT }); + issue = start(issue, { by: PERSON, at: AT }); + issue = markFixed(issue, { by: PERSON, at: AT }); issue = recordCheckpointReading(issue, { interval: "7d", outcome: "disagreed", at: AT }).issue; expect(issue.state).toBe("reopened"); expect(issue.id).toBe("PW-2291"); @@ -187,7 +197,7 @@ describe("the context banner", () => { it("repeats the line the message wrote, because both come from one writer", () => { const reopened = recordCheckpointReading( - markFixed(caseOf({ state: "in_progress" }), { actor: "person", at: AT }), + markFixed(caseOf({ state: "in_progress" }), { by: PERSON, at: AT }), { interval: "7d", outcome: "disagreed", at: AT }, ).issue; const digest = digestOf([reopened]); diff --git a/src/lib/__tests__/digest.test.ts b/src/lib/__tests__/digest.test.ts index 2326138..d1b08f5 100644 --- a/src/lib/__tests__/digest.test.ts +++ b/src/lib/__tests__/digest.test.ts @@ -14,6 +14,7 @@ import { digestLinks, renderDigestMessage } from "../digest-email"; import { DIGEST_CADENCE_LABEL } from "../digestCadence"; import { formatImpact } from "../impact-format"; import { markFixed, scheduleCheckpoints, type IssueCase } from "../issue-case"; +import type { Caller } from "../caller"; import { recordCheckpointReading } from "../checkpoint-evaluation"; import { normalizePerformanceThresholds } from "../performanceThresholds"; import { casePath } from "../paths"; @@ -61,15 +62,21 @@ function caseOf(overrides: Partial = {}): IssueCase { }; } +/** + * Whoever marked the fix. The digest never renders them — it is about cases, not + * about who moved them — so this exists only to satisfy the transition guard. + */ +const PERSON: Caller = { kind: "person", userId: "rae@webflow.com" }; + /** A case the system brought back, produced by the evaluator rather than posed. */ function cameBackCase(overrides: Partial = {}): IssueCase { - const fixed = markFixed(caseOf({ state: "in_progress", ...overrides }), { actor: "person", at: AT }); + const fixed = markFixed(caseOf({ state: "in_progress", ...overrides }), { by: PERSON, at: AT }); return recordCheckpointReading(fixed, { interval: "7d", outcome: "disagreed", at: AT }).issue; } /** A fixed case still waiting: three checkpoints scheduled, nothing read. */ function heldCase(overrides: Partial = {}): IssueCase { - return markFixed(caseOf({ state: "in_progress", ...overrides }), { actor: "person", at: AT }); + return markFixed(caseOf({ state: "in_progress", ...overrides }), { by: PERSON, at: AT }); } /** A fixed case whose three checks all failed to read — evaluation rule 4. */ @@ -81,7 +88,7 @@ function unreadableCase(overrides: Partial = {}): IssueCase { attempts: 2, result: "unavailable" as const, })), - history: [{ at: AT, from: "in_progress", to: "fixed", actor: "person" }], + history: [{ at: AT, from: "in_progress", to: "fixed", by: PERSON }], ...overrides, }); } diff --git a/src/lib/__tests__/webhook.test.ts b/src/lib/__tests__/webhook.test.ts index a3e3887..5a04545 100644 --- a/src/lib/__tests__/webhook.test.ts +++ b/src/lib/__tests__/webhook.test.ts @@ -7,6 +7,7 @@ import { import { buildDigest, type Digest } from "../digest"; import { renderDigestMessage } from "../digest-email"; import { markFixed, type IssueCase } from "../issue-case"; +import type { Caller } from "../caller"; import { recordCheckpointReading } from "../checkpoint-evaluation"; import { normalizePerformanceThresholds } from "../performanceThresholds"; import { pendingPage } from "../mutations"; @@ -18,6 +19,12 @@ afterEach(() => { const AT = "2026-08-04T06:00:00.000Z"; +/** + * Whoever marked the fix. The payload is the digest message and the digest is + * about cases, so this only ever satisfies the transition guard. + */ +const PERSON: Caller = { kind: "person", userId: "rae@webflow.com" }; + function caseOf(overrides: Partial = {}): IssueCase { return { id: "PW-1", @@ -71,7 +78,7 @@ describe("alert webhook", () => { * sections, same sentences. */ const digest = digestOf([ - recordCheckpointReading(markFixed(caseOf(), { actor: "person", at: AT }), { + recordCheckpointReading(markFixed(caseOf(), { by: PERSON, at: AT }), { interval: "7d", outcome: "disagreed", at: AT,