Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 66 additions & 13 deletions src/lib/__tests__/caller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 })) {
Expand All @@ -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([]);
Expand Down
18 changes: 14 additions & 4 deletions src/lib/__tests__/digest-arrival.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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> = {}): IssueCase {
return {
id: "PW-2291",
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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]);
Expand Down
13 changes: 10 additions & 3 deletions src/lib/__tests__/digest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -61,15 +62,21 @@ function caseOf(overrides: Partial<IssueCase> = {}): 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> = {}): 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> = {}): 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. */
Expand All @@ -81,7 +88,7 @@ function unreadableCase(overrides: Partial<IssueCase> = {}): 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,
});
}
Expand Down
9 changes: 8 additions & 1 deletion src/lib/__tests__/webhook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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> = {}): IssueCase {
return {
id: "PW-1",
Expand Down Expand Up @@ -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,
Expand Down
Loading