Notification classes & preferences, credentials as first-class records, and a worker that fits - #290
Merged
important-new merged 48 commits intoAug 1, 2026
Conversation
…ould wrongly disable `renderer.ts` reads `descriptor.required` to decide whether a tenant may switch a template off for everybody. Only 2 of 20 templates carried it, so today an operator can disable: password-reset every user loses account recovery workspace-invitation an invited colleague can never join agent-invite same, for agents agent-login-link an agent is locked out with no way back agreement-request the client never gets the link to sign payment-request we do not tell someone they owe money report-ready(-pdf) the report delivery itself All eight are spec §2.0/§2.1 NEVER rows. Verified against production: zero tenants have disabled any template, so this closes the hole without changing anyone's live behavior. The deeper problem was two surfaces asking the same question in isolation — the OPERATOR's kill switch (renderer.ts) and the RECIPIENT's (the preferences screen this unblocks). They are one question: a notification that must reach someone for legal or operational reasons must not be suppressible by EITHER party. So there is one flag and both read it; keeping them separate would let a tenant disable mail the recipient is told is "always sent". server/lib/notifications/classes.ts is that vocabulary, and classes.spec.ts makes spec §2 executable rather than aspirational: - every registry trigger must have a class - registry.required must EQUAL class.required, so the two axes cannot diverge - every class must appear in exactly one of NEVER_OFF / RECIPIENT_MAY_MUTE, so a newly added notification fails the build until someone decides - isSuppressible() fails closed on an unknown id Proved the gate is real before relying on it: run against the old registry it named exactly those eight and nothing else. Two existing specs pinned the old answer and were updated, not weakened: email-registry's hardcoded ['agreement-signed','evidence-pack'] was a snapshot of a wrong answer and its coverage moved (strictly stronger) into classes.spec; email-override-render used report-ready as its "non-required" example and now uses booking-confirmation, which is a genuine operator choice. Spec: docs/superpowers/specs/2026-07-31-notification-preferences-design.md §3.1 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
`sendEmail(to, subject, html)` carried an address and a rendered string — enough to deliver, not enough to decide. "An email to jane@x.com" cannot be matched against "Jane muted review requests", so a recipient-preference check placed at the boundary would have had nothing to check. This adds the field that makes the check possible; enforcement lands with the preference table. The design question was not "how do we pass a class id" but "how do we make it impossible to pass the WRONG one". ~20 mixin call sites each render a trigger and then send. A `classId` argument would have been a second chance to be wrong: a site could render booking-confirmation and declare report-ready, and nothing would catch it. So the trigger rides INSIDE `RenderResult`, stamped by whatever rendered it, and `sendRendered(rendered, to, …)` reads it from there. There is no argument through which a caller can name a template it did not render. 20 of 22 mixin sites convert mechanically; the two that did not are the interesting ones: - booking-confirmation appends the SMS opt-in block to the body. It spreads the render result rather than rebuilding it — rebuilding would drop the trigger and silently turn a classified send unclassified. Pinned by a test. - transactional.ts:126 is a THIRD raw call site, and §5.0's audit does not list it. That census swept ROUTES; this one is hand-built HTML inside the email service itself, where a route sweep cannot see it. Same lesson as repair-builder, one layer deeper: a census only finds what it thinks to look at. It is the free-tier quota warning, now classified `usage-quota-warning` (required — muting it means hitting the wall with no warning, the same harm as hiding money owed). Moving it onto a template is P3. The class gate proved itself on that new class before I trusted it: adding `usage-quota-warning` turned classes.spec red, named it, and refused to pass until someone decided whether it could be muted. That is the behavior the gate exists for, observed rather than assumed. An unclassified send stays SENDABLE and un-mutable — a boundary that dropped unclassified mail would turn a missing annotation into lost notifications. Spec: docs/superpowers/specs/2026-07-31-notification-preferences-design.md §5.0 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
Three sends built their own HTML: the repair-request share, the client
portal sign-in link, and the free-tier quota notice. Each shipped a
hardcoded slate button that ignored the company's colour and logo, could
not be edited or translated, and reached the send boundary with no
notification class. Being a template fixes all four at once.
Two of them lived in ROUTES, which is why they were invisible: a route
that builds an email is a route doing the email service's job, and no
sweep of the email service can see it. The routes now own the LINK and
nothing else.
Three judgements are worth stating.
`repair-request-share` is `required` — not because it is important, but
because the recipient is an address someone typed into a box. No account,
no relationship, nowhere for a preference to live. A preference is a
standing choice about a stream, and one share is not a stream; the only
thing "suppressible" could mean there is the operator switch, which would
make a send button report success and do nothing. classes.ts now names
this as the third case that earns `required`.
The quota notice becomes TWO templates, not one with a variable. "One
left" and "none left" are different messages, and a recipient reading a
list of what we send should see both. Both are `editable: false` +
`brand: 'platform'`: our message about our billing, on the same footing
as password-reset. The admin editor lists only editable templates, so
they correctly never appear there.
Converting them exposed two layout defects, both fixed with the tests
that found them. An optional block rendered an empty `<p>` and its
margin, so "optional" could only be expressed by the caller assembling
the block list — no template could declare it. And every `multiline`
block invites newlines that HTML then collapsed; they now survive as
`<br />`, inserted after escaping, so no author-supplied markup goes live.
The second one was already wrong for every editable template, not just
the new ones.
A new gate asserts every declared variable has a preview example. It
found two pre-existing holes: `agent-login-link.loginUrl` and
`concierge-cancelled-agent.reason` fell back to the literal `{loginUrl}`,
so the preview an admin used to check their copy rendered the CTA with a
junk href. Nothing failed; it just looked wrong to whoever opened it.
The class gate proved itself again before being trusted: with the four
new descriptors added and no classes, it went red naming exactly the
unclassified ones.
The registry outgrew the file-size cap, so it splits by audience into
`catalog/{system,client,agent,concierge}.ts`. Splitting rather than
bumping the baseline is safe here because the scannable "everything we
send" list is now NOTIFICATION_CLASSES; the registry is the copy store.
Two count assertions were stale and are now honest: `email-registry`'s
uniqueness check compared against a literal 20 instead of REGISTRY.length,
and the route's own OpenAPI prose still claimed 17 editable templates when
there were 19.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
Three paths carried their own copy of the SMS gate chain: the real send, the template test-send, and the settings test-connection. They did not BYPASS the gates — that would be the obvious bug and it is not the one that was present. Each had a copy, and a copy only has the gates someone remembered to add to it. That is not theoretical, and it is why this is worth doing. When the STOP-revocation check was added it landed in exactly one of the three. Nobody skipped a step; the other two were not there to receive it. Both test paths would send to a number that had texted STOP, and report success. Two failing tests proved that before the fix, one per path. The chain now lives in `lib/sms/send-gate.ts` and a caller declares a `purpose`. `test` names exactly one exemption — express consent, because there is no contact to hold any, so requiring it would mean no test send could ever succeed. It is NOT exempt from revocation: honoring STOP does not depend on the basis the first message was sent under, and it does not care that this one is a test. Revocation for a test send matches the NUMBER, since there is no contact. That match is normalized exactly the way the inbound STOP webhook normalizes on read — if the two disagreed, a revocation could be recorded against a contact this check would then fail to find, and the revocation would exist while doing nothing. A test pins it by seeding the contact as `(555) 999-1234` and texting `+15559991234`; breaking the normalization makes only that test go red. This also closes a hole in the real path. Revocation used to be checked only when the log carried a contact id; a log without one skipped the check entirely. It now falls through to the number match, so an implied-basis recipient whose number texted STOP is refused rather than texted. All 187 automation tests pass unchanged, so the paths that did have a contact id behave exactly as before. `rawDb` leaves `SendOneSmsArgs`: it existed only to hand SmsConsentService a raw binding, and the gate reads consent through drizzle. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
…ld miss `lint:provider-helpers` already guards the TRANSPORT. Transport was never the problem — everything above it was, and both defects were found by hand-auditing call sites. The second audit found what the first structurally could not: it swept ROUTES, and the miss was inside a service. A third audit would miss the next one. So `lint:notification-dispatch`, four HARD rules with NO baseline: route-builds-html a route may not build notification HTML unclassified-send a send must name what it is sms-send-without-gate an SMS send must consult smsSendGate second-gate-chain managedSendAllowed is callable from one file Every rule was proved to fail before being trusted: a probe file containing one violation of each produced four findings on the right lines and exit 1, and adding a classId cleared only the second — which matters, because "never call sendEmail" would be a different and wrong rule. No baseline is the deliberate part. Every rule is at zero TODAY, and that is only true because P3 and P4 made it true. A baseline is how the next violation gets admitted as pre-existing. Two sends were classified to reach zero. The per-role report delivery now carries the same class as the two branches beside it — it differs only in WORDING, chosen by the recipient's role, and a different template is not a different thing to have a preference about. The admin test send gets `admin-test-send` in a new `diagnostic` category: it only ever reaches whoever pressed the button, so no recipient can hold a preference about it, but the boundary still has to be able to say what it is sending. `diagnostic` is what keeps it off the preferences screen. Two sends are allowlisted rather than classified, and the distinction is the point. The automation RULES layer sends tenant-authored templates, so there is no fixed class id, and what class those carry is a real open question V2 has to answer. A wrong class is worse than a stated absence, so the gate names them instead of pretending. report-delivery.ts is 3 lines over its ratchet (733 → 736) for the annotation and its reason; splitting a 736-line route file is a refactor this change does not justify. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
…nothing The gate ladder says: do not run a gate the next rung will run. That is a rule someone has to remember, and it drifted inside the session that wrote it — lint:filesize four times in one task, type-check:api six, and a `vitest --changed` that quietly became all 3975 tests because package.json was in the diff. The whole argument of the last four commits is that a coupling held together by prose drifts, and the fix is to make the wrong thing cost nothing rather than ask anyone to avoid it. That applies here too. KEYED ON WHAT THE GATE READ, NOT ON THE INDEX. `git diff --cached` is the obvious key and it is wrong: these gates scan the WORKING TREE, so an unstaged edit introducing a violation would hash identically and the gate would print "cached" over a real failure. A false green is worse than no cache. The key is the gate's own source plus (path, mtime, size) of every file it scans. Three properties, because a cache is exactly how a gate silently stops working, and all three were proved rather than assumed: - a hit PRINTS that it was a hit - a FAILING run is never cached — verified by failing twice in a row - any flag (--update) bypasses it; a side effect is not cacheable Also verified: touching a scanned file re-runs, and a violation introduced after a passing run fails rather than reporting cached. Wired into exactly two gates. lint:deadcode is the only one whose cost is real work — 8.0s → 0.2s. The conformance gates are ~0.6s each and almost all of that is node startup the cache cannot remove, so wiring them would save under 0.3s apiece while adding seventeen chances to under-specify an input set, which is the one way this produces a false green. Not worth it, deliberately. Wiring knip up immediately earned its keep: it caught two dead exports this branch introduced (NotificationCategory, SmsPurpose), both only reachable from the full-lint rung the inner loop never runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
…that has to hold One recipient's answer to "send me this or don't", per class per channel. ONE SUBJECT COLUMN, NOT TWO. The obvious design is a nullable user_id and a nullable contact_id with a rule that exactly one is set. It does not work: SQLite treats NULLs as DISTINCT in a unique index, so (t1, NULL, 'c1', 'email') does not conflict with itself. The constraint meant to guarantee one answer per (who, what, how) would silently admit duplicates — and a duplicate here is two contradictory answers with no rule for which wins. subject_kind + subject_id are both NOT NULL, so the index actually holds and the two-columns-one-truth state cannot be written. A spec asserts the UNIQUE rejection rather than describing it. subject_kind is part of the key, not a label: users.id and contacts.id are independent id spaces that can collide. ABSENCE IS NOT "OFF". No row means the class default applies, which is "send". Only an explicit enabled=false suppresses, and only for a class isSuppressible() allows — which fails closed on ids it has never heard of. So a preference row can never silence something the recipient is told is always sent. Erasure deletes these with their subject, and the reason is not tidiness: a contact id is REUSED after an erasure, so a surviving row hands the next person at that id the erased subject's mute settings — invisibly, and in the direction that withholds mail nobody asked to withhold. Scoped to subject_kind='contact'; staff preferences are not a consumer data subject's. The manifest-coverage drift guard binds the new rule to the orchestrator step that realizes it. The erasure test was proved twice. The first red was my own bad fixture id (the InspectorHub#88 describe block seeds contact-88, not contact-subject), which proves nothing about the code, so the delete was neutered afterwards to confirm the test actually catches its absence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
The email boundary now drops a recipient who switched this notification class off. Two halves, split deliberately: a PORT decides whether a class may be withheld from an address, the BOUNDARY decides what to do about it. THE REQUIRED CHECK RUNS FIRST, before any lookup, and that is what makes the screen trustworthy. A class the recipient is told is always sent must stay unmutable even if a row says otherwise — a stale row, a class whose required flag changed, a hand-written INSERT. isSuppressible() also fails closed on ids it has never heard of, so a newly added notification is never withheld before someone has decided it may be. The screen's promise and the send path's behaviour cannot diverge. An address resolves to BOTH id spaces. An agent with an account who is also a contact on an inspection is one human, and asking them to switch the same thing off twice is the kind of half-working control that is worse than none. FAIL-OPEN, like the suppression gate beside it. A failed query must never be the reason someone did not hear from us: nobody reports mail that never arrived. An UNCLASSIFIED send never consults the port at all — a preference that cannot be named must not be applied by guesswork. Absence is not "off": no row means the class default, which is "send". Two proofs rather than one, because the interesting failure here is a gate wired to nothing — which looks exactly like a gate that passes, and is how check-ts-range.mjs spent months reporting "skipped". So there is a test that assembles the service production assembles and asserts a real row stops a real provider call, and it was verified to go red when the port is unwired from the constructor. One test bug worth recording: the boundary probe first recorded its own argument instead of what reached the provider, so it passed while the filtered list never shrank. An assertion has to sit on the far side of the thing under test. SMS is deliberately untouched — consent, not preference, is the authority there, and it already has one gate chain. The automation RULES layer is still unclassified (the gate allowlists it): its trigger enum is a fixed 19-value vocabulary and is the right class, but that is its own step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
…acing transactional / operational / marketing is a COMPLIANCE taxonomy — the vocabulary CAN-SPAM and GDPR reason in — and it says what the CONTENT is. `diagnostic` was added to it for the admin test send, and that was wrong: "only ever reaches whoever pressed the button" is a fact about the AUDIENCE. One column answering two different questions is the defect this work keeps finding elsewhere, so it should not have been introduced here. The audience fact moves to `recipientFacing: false`, which says what it means and leaves the taxonomy alone. The class still exists, and the send boundary can still name what it is sending — that part was never in question. A test now pins the vocabulary at three values, and was verified to fail by reintroducing the fourth. That test is the thing that would have caught this when it was written rather than two commits later. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
All scheduled and automatic sends are automation rules, so until now the entire rules layer reached the send boundary unnamed — and a preference cannot apply to something that cannot be named. The gate's allowlist entry for it is gone, and breaking the new call site was verified to make the gate fail. THE CLASS IS THE SEED, NOT THE TRIGGER. §2 already refuted the trigger: report.published alone carries five seeds, three to the same client saying different things, and §5.3 settles those outright — "report-ready is required and post-inspection follow-up / review request are not". One trigger-keyed class could not hold both answers, and `required` is the field the spec calls load-bearing. Seeds whose notification ALREADY has a class reuse it — Booking Confirmation, Report Ready, the invoice, both agreement ones, and the buyer's-agent report-ready that services/email/agent.ts also sends. The manual path and the automatic path are one notification arriving; two switches for one notification is how a control comes to half-work. Nine office alerts are nine classes. §2.5 lists them as one row for brevity; they are nine distinct events, and collapsing them would be the same mistake as keying on the trigger. Staff and inspector classes are `required` because §2.5 marks them Operator, not You. The operator's control is the rule's active flag, which is why one flag still suffices. Tenant-WRITTEN rules resolve to undefined and stay unclassified: they still send, they just cannot be muted by a recipient, and the operator can disable any rule. Inventing a per-rule class would put tenant data into a vocabulary the boundary fails closed on. Two gates earned themselves immediately. The seed-coverage one found three seeds a manual read had missed — their names contain an apostrophe, so they are double-quoted in the source and a single-quote regex skipped them silently; 29 seeds, not the 26 I had counted. And a channel gate caught `inspection-cancelled` declaring SMS: I transcribed that from §2.2, but the Cancellation Notice seed has no smsBody, so the screen would have rendered a switch for a message that can never be sent. §2 lists the channels the product INTENDS; the class must list the ones it has content for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
Both deviations were confirmed as intentional, which means the thing to protect is no longer the decision but its consequence — the part a later change would narrow without noticing it was narrowing anything. D3: the boundary resolves an address in BOTH id spaces, so a mute crosses identities. The existing test covered the user→contact direction; this adds contact→user, and the case that actually bites — a tenant who also keeps a staff address in `contacts`, where one person's mute governs both identities. That is the intent (one human, one inbox), and narrowing the lookup to a single space now fails here instead of quietly halving the control. A second test pins that the crossing stops at the tenant boundary: the same address in another tenant is a different relationship. D5: two contacts sharing a number means one person's STOP withholds the other's message. Not introduced by the shared gate — the inbound STOP webhook already records revocation against EVERY contact matching the number, so the ledger was always number-shaped, and reading it any other way would honour a revocation for one row while ignoring it for its twin. Both were verified to go red under exactly the narrowing they warn about (consult only the addressed contact / only one id space), and green again when restored. A test that has never failed is a test that has proved nothing. One fixture bug found on the way: `users.tenant_id` carries a legacy FK, so the cross-tenant case needs the other tenant seeded first — the test was failing on its own setup, not on a leak. The in-app half of v2 is NOT included and is not a small addition: nine insert sites across three files need a class first, which is the same shape of work P2 did for email. Enforcing preferences only where a class happens to be available would build the half-working control this whole change exists to avoid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
The notice header IS the in-app delivery, so withholding one means not
writing the row. `insertNoticeHeader` now returns null when the recipient
switched that class off, and the fan-out marks those logs skipped with a
reason rather than leaving a pending row that never resolves.
ONE DECISION, TWO CHANNELS. The subject-keyed core is extracted and
shared rather than restated — the required check above all, because that
is the thing keeping the screen's promise and the send path's behaviour
in agreement. A second copy of it is how the two would drift into
disagreeing about what "always sent" means.
In-app needs none of the address resolution email does: a header is
`user_id XOR contact_id` by construction, so the subject is already in
hand. That is why the shared piece is the decision, not the port.
The class comes from the RULE, threaded like the wording already was —
two rules on one event are two different things to have a preference
about, so a per-firing class would be wrong for the same reason a
per-firing title was.
Unclassified headers are always written, matching the email boundary: a
notice that cannot say what it is must never be silenced by guesswork.
Required classes are always written too — §2.5, an individual cannot mute
their own dispatch.
Verified by unwiring the check and watching the enforcement test go red.
THE GATE FOUND A DEFECT IN ITSELF. `lint:notification-dispatch` required
a literal `classId:`, but the idiom under exactOptionalPropertyTypes is a
conditional spread — `classId ? { classId } : {}` — which is shorthand
and has no colon. It reported a correctly classified send as
unclassified. Worth noting how it survived a commit: the gate lives only
in the full lint run, so nothing ran it between the type fix that
introduced the shorthand and the commit after it.
Two fixture bugs on the way, both the same shape: `users.tenant_id` and
`notifications.user_id` carry legacy FKs, so a test row has to exist
before another row can point at it. Both failed on their own setup, not
on the behaviour under test.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
§2 has two columns the code has to honour. P1 made "Off?" executable as `required`; this is the other one. The screen needs it — a client shown "Office alert — new booking" is being asked about mail they can never receive, which answers neither of the two questions §4 says the page exists to answer. All 48 classes now declare an audience, transcribed from §2, and a gate checks each seed-backed one against the rule that actually sends it: recipientKind staff/inspector → staff, buyer_agent/listing_agent → agent, otherwise client. Verified by flipping an office alert to 'client' and watching it fail. Without that, a class and the rule that sends it can disagree, and the reader is the last to find out. One class has an EMPTY audience. A repair-request share goes to an address someone typed, so there is no account to render it on — the same fact that makes it required. The model itself is one function because three surfaces render it. The filtering rules would otherwise be decided three times, and a class added later would show up on two screens out of three with nothing to say which was right. Two distinctions from §4 are encoded rather than described: `unavailable` is not `off`. A review request has no in-app form, and an off-switch for a channel that does not exist is a lie about what exists — a reader who turned it on would be right to expect something to happen. Absence is not "off" either, so the model takes the set of explicit MUTES rather than a full preference set. Storing a row that merely restates the default makes the table grow with the user base instead of with the decisions (§3.2). `alwaysSent` carries no per-channel state at all, so there is nothing for a stale row to flip — the required guarantee holds at the screen for the same structural reason it holds at the send boundary. The UI is NOT in this commit. Per the gate ladder, a screen needs the frontend-design skill and a Chrome walkthrough in both themes before it can be committed, and "the model is green in vitest" says nothing about whether the page is usable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
…ackages The screen §4 describes, as one component for all three audiences — CLAUDE.md's Cross-Portal Reuse rule: one entity, one component, the differences as props. A parallel implementation would drift and only one of the three would get the next fix. Three choices are deliberate, and each is a place where the obvious implementation lies to the reader: ALWAYS SENT is a section with a reason, not a row of disabled switches. A greyed-out toggle invites the reader to try, then refuses. The count is the loudest thing on the page because §4 says why: "7 notifications you cannot switch off" is a number a reader can hold, and "we may send you service messages" is not. An em dash is not an off switch. It means the notification has no form on that channel, and an unchecked box there would invite someone to turn on something that can never happen. Text is not a third identical toggle. Consent is the authority there and a preference can only narrow it (§3.3), so this leaves the seam for the v4 ledger block rather than rendering a switch that would lie. It carries ARIA table semantics because notification x channel IS tabular data. That came from the test being awkward to write — a test that has to walk .closest().parentElement is telling you the markup threw away its structure — and it gives a screen-reader user row and column context the div grid did not. TWO PACKAGES ADDED, and one of my reasons for the second was wrong. jest-dom earns its place on failure MESSAGES: `expect(el.disabled) .toBe(true)` fails with "expected false to be true" and names neither the element nor the reason. user-event I justified as catching controls a real user could not reach — verified, and it only holds for INLINE styles. Vitest loads no Tailwind, so a `pointer-events-none` class has nothing behind it and the click goes straight through. Real unreachability here comes from classes and overlays and is invisible at this level. user-event still earns its keep on the focus/pointer sequence and keyboard interaction; it is not a reachability gate, and the test file now says so, because I nearly gave myself the impression it was. The Chrome walkthrough is the rung that answers reachability and both themes, and it is not in this commit — no route renders this yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
GET returns the two sections §4 describes; PUT records one choice. THE SUBJECT COMES FROM THE SESSION, NEVER THE BODY. A preference is a statement about one person, so a subject id in the request would let anyone silence anyone. A test sends one anyway and asserts the row lands against the signed-in reader. PUT refuses three things, and the third came from a test failing for a reason I had not considered. A required class is refused; a channel the class never uses is refused; and now a class this reader is never addressed by. That last one surfaced when a staff-role test wrote a mute for an agent-only notification and succeeded — a row nobody could ever see or clear, because no screen renders it. The argument is the same as the other two: accepting a change that can never take effect is dishonest. The send boundary is what makes the guarantee TRUE; this is what makes the screen HONEST. Switching something back ON deletes the row rather than storing `enabled = true` (§3.2): a row that restates the default makes the table grow with the user base instead of with the decisions. Two more gates earned their place. `primary-tier route count ≤ 45` caught me tiering a settings surface as primary — that budget is the MCP tool surface, and a reader's own preferences are not a tool. The input-description gate caught two undescribed fields. server/index.ts is 2 lines over its ratchet. It is the route registry; it grows by one line per route by construction, and splitting it is not a refactor that adding a route justifies. Worth stating because it will otherwise read as a bug: STAFF have almost nothing in "you choose". §2.5 makes work notifications the operator's call rather than the individual's, so of the staff-facing classes only the concierge review is theirs to mute. That is the design, and the screen's empty state has to carry it rather than look broken. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019PhkhCUaL5BSTVTvhWYFNq
…t and client
Retires the three per-event booleans on `users` and gives every audience a
screen backed by `notification_preferences`. The old shape was one column per
event read by one send method each, so the ~45 notifications nobody added a
column for simply had no off switch.
Storage rule: store only what DIFFERS from the class default; matching it
deletes the row. Stated that way rather than "delete on enable" because
`agent-invoice-paid` defaults to OFF -- its column defaulted to false, and
`defaultEnabled` moved that default across with the data, so the migration's
third backfill statement is inverted. Without it a naive migration had only bad
answers: a mute row per user, or agents silently starting to receive invoice
mail.
Three routes, not one, because the subject differs:
- staff -- `users` row, tenant from the JWT (Settings > Profile)
- agent -- PER COMPANY, keyed on each company's `contacts` row. An agent
account is global (`users.tenant_id IS NULL`) and its JWT carries no
tenant, so there is no session tenant to scope a row to.
`scope: 'all'` applies one change to every linked company.
- client -- portal session cookie; one email can be several contacts in a
tenant, so a choice is written to all of them and a mute on any
one of them counts.
Refusals live in one place (`preference-write.ts`): unknown class, always-sent
class, a channel the class never uses, a class this reader is not addressed by.
The send boundary is what makes a preference TRUE; these keep the screen HONEST.
Also fixes, found in Chrome and invisible to every unit test:
- a failed read rendered as "0 notifications you cannot switch off" -- a
confident false answer, and the count is the loudest thing on the card. A
failure is now distinct from emptiness on all three surfaces.
- auto-save had no reply, so a reader could not tell a persisted change from a
box that merely looked ticked. Added a saving/saved indicator that never
claims "Saved" when the write failed.
- `notification-preferences` was never registered in the per-module hono
client, so `api["notification-preferences"]` type-checked against nothing.
The staff helper typed its client as `any`, which is what hid it.
Chrome verification is PARTIAL. The staff surface was driven end to end in both
themes: 17 always-sent / 1 choosable, a click writes the row, a second click
deletes it, and the bell's settings link works. The agent and client surfaces
are covered by unit tests only -- establishing a non-staff session in the
browser failed, and `GET /api/agent/profile` (untouched code) fails the same
way, so the blocker is session plumbing rather than this change.
File-size gate: `settings-profile.tsx` (+1) and `portal-inspection.tsx` are
bumped in the baseline. Two real extractions came first --
`settings-notifications.server.ts` and `portal-notification-preferences.ts` --
the residue is route wiring that has to live in the route.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
…on SMS
Builds on the per-recipient preference screens with the grid-shaped bulk
controls (`applyBulk`) and closes a hole the screen itself created.
THE SMS SWITCH WAS A LIE. Two client classes declare an SMS channel
(`booking-confirmation`, `inspection-reminder`), so the screen rendered a Text
switch for them — and `smsSendGate` never consulted preferences, so ticking it
off stored a row nothing read and the text went out anyway. That is exactly the
defect this program exists to remove. The gate now takes a `classId` and checks
the recipient's preference AFTER consent and BEFORE quota: after consent
because a preference NARROWS what consent allows and must never widen it
(§3.3), before quota because a text nobody wanted must not spend the tenant's
allowance. Unclassified sends (an admin test send) stay unmutable, which is
`isSuppressible` failing closed.
Bulk actions are scoped the way the grid is: a row (every channel of one
notification), a column (one channel across every notification), or the corner.
Loose buttons above the table would have made the reader work out which cells
each one touched. `reset` is a separate verb from `enable` and the difference
is load-bearing: reset DELETES rows so each class returns to its own default,
and `agent-invoice-paid` defaults to OFF.
Two things Chrome caught that no unit test could:
- an all-unavailable column rendered a bulk checkbox that looked like "all off"
and did nothing when clicked — the em dash's own lie, reintroduced one level
up. A scope with no selectable cell now renders no control.
- `?section=notifications` fell through to the overview: `HubSection` had the
member, the hand-maintained `HUB_SECTIONS` array did not. Replaced with a
`Record<HubSection, true>` so the compiler keeps them in sync (CLAUDE.md:
make a "must stay in sync" coupling executable, not a comment).
Feedback moved to the existing ToastPortal. The inline red line it replaces sat
inside a card the reader may well have scrolled past — a message about mail they
will not receive, placed where they cannot see it. Only the in-flight state
stays inline, next to the switch that was touched. The em dash now carries a
one-line legend: a symbol a reader has to ask about has been left to guess, and
the natural guess here ("it's off") is the wrong one.
Chrome walkthrough, both themes: staff (17 always-sent / 1 choosable, write and
delete round trip), agent (per-company isolation verified across two companies,
column action wrote exactly one row because every other class already defaults
to on). The client Hub section is reachable after the section fix; its grid is
the same shared component the other two exercise.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
The grid no longer narrows itself to what a class declares today, and it never reads the tenant's automation rules or templates. Every notification a reader is addressed by shows all three channels, and `unavailable` (the em dash) is gone from the model entirely. A preference is a statement of INTENT. "Do not text me about bookings" is a true and useful sentence before anyone has written that text, and storing it now means the answer is honoured the moment the content and the rule are completed — rather than being unaskable until then and silently lost in between. The asymmetry is what makes this safe: the switch's meaningful direction is OFF, and OFF always works. A channel left ON that nothing sends yet is quiet, not broken. The alternative — deriving the cells from what the tenant can send today — would have made the screen change shape underneath the reader every time an operator toggled a rule or added a template body, which is the confusion this avoids. Consequences, all deliberate: - `assertChoosable` loses its channel refusal. Storing a preference for a channel nothing sends on yet is now the point, not an error. - `applyBulk` covers all three channels rather than the class's own list. - `classes.ts`'s `channels` is unchanged and still the truth about what the CODE can send; it still gates the send path. It just no longer decides what the screen offers, and `automation-classes.spec.ts` still holds it honest against the seeds. Every spec that pinned the old answer was rewritten rather than deleted, and one was inverted on purpose: "never writes a channel the notification does not use" is now "DOES write a channel the notification does not send on yet", because narrowing the write is exactly how the answer would be dropped. Verified in Chrome across all three surfaces. On the client Hub, turning the Text column off wrote 10 rows — one per choosable class, including those with no SMS form in the code today — while the corner and every row control moved to indeterminate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
…nel switch means V4's first half. The grid answers *which notifications*; this answers *this channel at all*, and the two are different questions the send gate already reads in that order — consent first, then preference. Someone can consent to texts and still not want booking confirmations. Spec §4 was rewritten where it had gone stale or was never written: - the mock still showed em dashes, which the previous commit retired - "the SMS block shows the LEDGER, not a switch" predated per-notification SMS preferences; §4.2 now states how the ledger and the Text column coexist - "deployment-aware rendering" was a roadmap phrase with no definition. It has one now, and the answer is that deployment mode changes NOTHING — hiding the block when a tenant has no SMS provider would make the screen read tenant configuration, which §4 choice 3 exists to avoid. THE TWO CHANNELS ARE NOT SYMMETRICAL, and §4.2 says so in a table rather than letting the code imply otherwise. SMS has a legal consent artifact (`sms_consent_log`); email has only deliverability suppression, which is a different fact. So switching SMS off writes a `revoked` row AND cascades the Text column; switching email off cascades only — and email's "off" can never mean "no email", because required classes still send. That last point is said out loud in the UI rather than left to be discovered. There is deliberately NO "turn back on" control. Granting consent means recording a disclosure version, capture method, ip and user agent — evidence only `/sms-optin/:token` can honestly produce, so the block offers Stop and sends the reader out to grant. Revocation delegates to `SmsConsentService` rather than inserting directly, because that is what stamps the current disclosure version; a hand-rolled insert would drift from the version the opt-in page and the STOP webhook both use. Who sees the block, and why staff do not: consent attaches to a `contacts` row and a staff member is a `users` row, and no user-facing class is both staff-addressed and SMS. There is nothing to revoke. Inventing a staff consent row so the screen looks uniform would be a control over nothing — the same mistake as an off-switch on a channel that does not exist. An agent's revocation is recorded AS an agent's, because the ledger column exists to say which basis the person was reachable under. Chrome caught one thing the tests could not: the ledger date rendered as 2026年6月13日 inside an otherwise-English page, because `toLocaleDateString(undefined, …)` reads navigator.language. The obvious fix, `useDisplayLocale()`, needs route loader data the token-authenticated client portal does not have — so the locale is a prop, which works on all three surfaces and keeps the component renderable on its own. Still open in V4: the not-signed-in landing page and the legal-document links (§4.1). Legal copy changes need a version bump plus `terms:publish`, which is outward-facing and stays a human decision. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
… Text column The consent block was a dead end after a stop: no Stop button (correct, already stopped) and no way back (not). Caught by looking at it, not by a test. It now sits ABOVE the grid, because consent is the gate and the grid is what happens behind it, and it can turn the channel back on. Turning it back on is an inline grant, which is only honest under one condition: the disclosure has to be ON SCREEN and its VERSION has to travel with the acknowledgement. Both hold — the text renders in the block, the version comes back with the click, and the route refuses a version that is not the current one, because a stale version means the reader agreed to text they are no longer shown. `captured_via` gains `settings_page`; the enums are type-layer only in drizzle (the DDL is plain text), so widening cost no migration. A revoked consent now LOCKS the Text column rather than merely unchecking it. No text can arrive whatever a row says, so leaving the switches live would let someone tick "yes, text me about bookings" while consent says we may not text them at all — a screen disagreeing with the send gate. The column's bulk control disappears with it, for the same reason an all-unavailable column had none. Two things only the browser could have found: - the ledger recorded NULL ip and user agent. The BFF calls the API in-process over the `API_WORKER` binding, so `cf-connecting-ip` and `user-agent` never reach the handler on their own. They are forwarded explicitly now — those two fields are what make a consent row defensible in a carrier audit, and nothing in the types or the tests would have said a word. - absent headers are OMITTED rather than sent empty. An empty string is stored as one, and a row claiming "we recorded an ip and it was blank" is worse evidence than one that plainly has none. Verified end to end in the browser: stop → column locks and the block offers a way back → acknowledge → grant lands as `granted / settings_page / version 1` with a real user agent. Still open in V4: staff SMS (needs `subject_kind`/`subject_id` on the consent ledger — a reversal of the 2026-07-30 ISV decision, so that spec and docs/sms-compliance.md change with it), the not-signed-in landing page, and the legal-document links. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
…a contact The ISV strategy (2026-07-30) promised staff a "separate track — employment / account terms + STOP" and the schema could not keep the second half: `sms_consent_log.contact_id` was NOT NULL and a staff member is a `users` row with no contact, so a staff STOP had nowhere to land. The ledger now carries `subject_kind` / `subject_id`, mirroring `notification_preferences` — one shape for "a person, of either kind" rather than a second XOR of nullable columns. `recipient_type` gains `staff`, and the send gate's latest-event lookup keys on the subject, so a staff revocation is honoured by the same query that has always honoured a contact's. WHAT DELIBERATELY DID NOT CHANGE is the half a carrier asks about: only consumers ever produce a `granted` row. `grantSms` returns early for any non-client audience, so agents and staff stay implied and never enter the ledger as grants. "Show us your opt-in proof" keeps pointing at consumers alone, while nobody who says stop keeps getting texts. Both compliance documents were amended rather than left to drift — the strategy spec gains a §2.1 amendment saying what moved and what did not, and docs/sms-compliance.md now states the asymmetry out loud. The generated migration was BROKEN and is hand-edited. drizzle-kit emitted a table rebuild whose INSERT selected `subject_kind` and `subject_id` FROM the old table, where neither exists — "no such column", with DROP TABLE as the next statement. On a table holding consent evidence that is not an acceptable failure mode. The copy now supplies them as literals and backfills the subject from the contact every existing row already has. Applied locally; remote stays behind the D1 SOP backup. Two defects found on the way, both pre-existing: - `requiresExpressSmsConsent` THREW on an unrecognised role kind — indexing the basis map with a value not in it and reading `.basis` of undefined. In a compliance gate that is worse than either answer: not a refusal, not a send, but a 500 whose meaning depends on the caller. It now fails CLOSED. - two consent events recorded in the same millisecond resolved arbitrarily, because `ORDER BY created_at DESC LIMIT 1` had no tiebreak. A STOP and a START a millisecond apart could pick either. Insertion order now breaks the tie — for a consent ledger, "which one is latest" must not be a coin toss. Surfaced when the new index changed which plan SQLite chose. Not eyeballed: the staff screen's rendering. Verifying it needed a staff session I had just logged out of, and logging back in would have meant typing a saved password. Its server path is covered by `channel-consent.spec.ts`, including the staff subject specifically. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
… contract Two gaps, both found by looking at the screen rather than at a test. THE ONE-WAY DOOR. Last commit gave staff a STOP and no way back: `grantSms` returned early for any non-client audience, on the reasoning that a staff `granted` row pollutes consumer evidence. That reasoning was too blunt. Staff and agents never granted anything — they are reachable under an existing relationship — so their "on" is a RESUME that withdraws the earlier stop, not a consent capture. It is recorded under `recipient_type: 'staff'` / `'agent'`, with no disclosure and nothing to acknowledge, because there was never anything to agree to. The invariant that actually protects the ISV filing is not "no staff rows" but "NO NON-CONSUMER ROW IS EVER LABELLED `client`" — a filing counting opt-in evidence filters on that column, which is the whole reason it is not a boolean. `channel-consent.spec.ts` now pins that directly. Both compliance documents were corrected rather than left saying the stricter thing the code no longer does. THE TRIPLICATION. Staff, agent and client each carried their own fetcher, status, toast, save/bulk handlers and — worst — their own copy of the rule that a revoked consent locks the Text column. Three copies of a rule is three chances for one surface to quietly stop enforcing it, and that one is what keeps the screen agreeing with the send gate. `<NotificationSettings>` now owns all of it; the three wrappers are ~50 lines of chrome and an intent name. Falling out of that: - express-vs-implied comes from the SERVER as `smsConsent.mode`. Three call sites hand-setting it was three chances to ask a staff member to acknowledge a consumer disclosure they never needed. - all three routes expose the same `PUT …/sms-consent`, so no surface is the one that cannot resume. - `<SmsDisclosure>` is shared with the public `/sms-optin` page. Both stamp the same `disclosure_version` into the same ledger, so two copies of that markup could drift while the row still claimed the reader saw version N. It also fixed a real gap: the inline grant was missing the privacy and terms links the opt-in page has shown all along. Verified in the browser: staff STOP writes `subject_kind=user, contact_id=null, recipient_type=staff, revoked`, the Text column locks, and the resume writes `granted / settings_page` with a real user agent — the append-only ledger keeping both halves of the history. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
…ture Two defects on Settings -> Profile, both found by looking at the page. ADDING A CREDENTIAL SHOWED NOTHING TO FILL IN. `onAdd` creates a blank row and the `<details>` was collapsed, so a new credential rendered as an upload box and the word "Details" with nothing saying what it was — the two fields hidden at exactly the moment they are needed. It opens by default now, and stays collapsible for someone who has already filled several in. The uploader column also went w-24 -> w-36: its own caption wrapped to three lines and read as a broken layout. CREDENTIALS NEVER REACHED THE EMAIL SIGNATURE. `inspectorSignature()` has accepted a `credentials` argument since Spec B and renders badges from it — and no caller ever passed one. The feature was wired and dead: the settings copy promises "shown on your reports, emails, and booking page" while the signature showed only the legacy `license_number` line. The preview now supplies the inspector's active credentials, so what a reader sees is what a recipient gets. Both are one half of a migration Spec B started and did not finish; the other half (retiring `users.license_number`, which still renders in the signature and the PDF footer) is written up rather than done, because deleting the field today would silently drop those two surfaces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RgBRZQhGELdkaWGorWwWKV
The agent portal's "Log out" pointed at `/logout`, and the teardown behind it
ends with an unconditional `redirect("/login")`. `/login` is the STAFF front
door — an agent has no account there — and under `APP_MODE=saas` that page 302s
again to `${PORTAL_API_URL}/login`, out of this product entirely and onto a
portal sign-in an agent cannot use at all. So logging out of the agent portal
did not land on a login page; it landed on a dead end.
The same wrong door was on the EXPIRY path, which is the one that fires without
anybody clicking anything: `requireToken` throws `/login` on a missing token and
routes an expired one through the same teardown, and it is `agent-layout`'s
loader that calls it. An agent whose token aged out was dumped on the staff
login too.
`loginPathFor(request)` now derives the door from the path rather than taking it
from each caller, because the callers are the two functions every agent loader
and the logout route already go through — a caller-supplied argument is a thing
a new agent surface can forget, and this one is only ever wrong in a direction
nobody tests. `agent-logout` exists for the same reason: it is the same module
as `/logout`, and the entire difference is that the path carries the signal.
The prefix is the whole rule, which the spec pins in both directions:
`/contacts` and `/inspections/agent-notes` are staff pages ABOUT agents and stay
on the staff door.
Also `/agent-signup`'s "already have an account?" link, which pointed at
`/login` — the one page that cannot accept the account it was offering.
Verified live on all four paths (302 Location): /agent-logout -> /agent-login,
/logout -> /login, /agent-dashboard -> /agent-login, /inspections -> /login.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
…d in Spec §4.1 asks the privacy policy and the terms to link to the notification control. There was nowhere to link. The client's copy of that surface hangs off the Hub bell, so its URL names an INSPECTION — and the people who follow a link out of a legal document are typically not standing on one, and often are not signed in at all. A link into the Hub would have been a link into an inspection they may not have open. So `/portal/:tenant/notifications` takes no inspection and assumes no session. Signed in, it renders the same `PortalNotificationSection` the Hub renders and its action listens for the same three intents through the same three helpers — one surface, one contract, because a divergence here is a switch that works on one entrance and not the other. SIGNED OUT, IT MUST NOT SAY WHETHER THE ADDRESS IS KNOWN. That property is already the API's (`request-link` is payload- AND timing-identical either way, the send deferred to waitUntil); this page's part is to never ask a question whose answer could differ, and to render the same conditional-voice panel every time — "If an account matches that address, a link is on its way." Verified with a known and an unknown address: identical body, 0.249s vs 0.256s. THE LINK HAS TO COME BACK HERE, and that is where this could have gone wrong. The obvious shape is a `next` path echoed into an outbound email — which is an open redirect with a delivery mechanism attached. `destination` is therefore an ENUM of two names, and `?to=` is matched against one literal rather than used as a path, so there is nothing for a crafted link to point at. The spec pins that `//evil.example`, `https://evil.example` and `/agent-dashboard` are all 400 at the schema, before any link is built. `redeemDestination` is a pure function and not a ternary in the loader because one of its four answers is a security property, not a routing preference: an agent-resolved redeem holds `__Host-inspector_token` and no `__Host-portal_session`, so it must never be handed a `/portal/` path. Its spec asserts the negative — an agent stays on `/agent-` in BOTH arms — which keeps holding if someone later "unifies" the two branches. An agent who asked for notifications lands on their own settings rather than the dashboard, so the link does not stop one page short of what it promised. server/api/portal.ts crossed its file-size cap by the 10 lines of the enum and its comment; baseline bumped rather than split, since splitting a route module is a refactor this change does not justify. Verified in Chrome, light and dark: signed-in surface, signed-out form, and the sent panel for an address that does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Design §6A: the platform's own documents live in a repository, so a version registry there can hash the text and let git hold it. A tenant's `tenant_configs.privacy_body` is a mutable TEXT column with nothing behind it — so the same table shape ported across would prove that the text CHANGED while being unable to produce the text that changed, failing at exactly the moment somebody needs it. `tenant_legal_versions` therefore stores the BODY, copying the shape this codebase already uses for the same problem (`agreement_requests.content_snapshot` / `.content_hash`) rather than the platform's. That also sidesteps lifting private-repo source into public OSS. WHAT A ROW MEANS. One publish of one document. `version` is a date string where the inspection Agreement's is an auto-increment integer — the formats differ so a reader cannot mistake one object for the other, and they share no table, no counter and no acceptance flow. The two failure modes are opposite and both invisible from the settings page, so both are pinned: recording NOTHING (the handler never calls the historian), and recording on every PATCH (a tenant changes their booking hours and mints a new revision of their privacy policy). The comparison is on the content hash, so an unchanged body is a no-op. Removing the guard turns three specs red. The route spec found the first one for real: the harness's service stub had no `legalVersion`, the PATCH handler swallowed the failure — deliberately, since a version row is evidence ABOUT a save and must never cost the tenant the save itself — and the version table stayed empty. That is exactly the risk of a non-fatal write, and the spec is the compensating control. It now runs the real service over the test DB rather than a stub that could silently do nothing. DATES ARE THE TENANT'S. The service resolves the timezone itself instead of trusting a caller: 2026-08-01 in UTC is still July 31 across the Americas for most of the day, and a "last updated" one day ahead of the company's own calendar only ever surfaces as a complaint. Same-day republishes collapse onto the text that ENDED the day, which is the one anything downstream could have relied on. "Last updated" replaces a HARDCODED literal — `public_legal_effective` read "Effective: July 30, 2026" and was shown on every tenant's page whatever their document said, stale from the release that shipped it. The replacement is string arithmetic with no `Date` anywhere, because the value is already a civil date and parsing it as a UTC instant is how it would render as the previous day. Null until a tenant publishes, and the line is omitted rather than invented. Also answers an open question in §6A.5 while passing through it: the hosted page renders the custom body in `whitespace-pre-wrap`, i.e. as PLAIN TEXT. A Markdown quick-insert affordance would ship as literal characters, so that feature stays out until the page renders Markdown. The table is declared in the erasure manifest as out of scope with reasons rather than left silent — the PII heuristic flags nothing here, and silence is not a decision. Migration 0021 is a bare CREATE TABLE: no rebuilds, so it does not touch `sms_consent_log`. `db:check` clean. Read path verified end to end against local D1: row -> API `lastUpdated: "2026-08-01"` -> page "Last updated August 1, 2026". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
…eview `inspectorSignature()` has rendered credential badges since Spec B and NO CALLER EVER SUPPLIED ANY. The feature was wired and dead in both directions: the renderer accepted a field nothing set, and `SenderSignature` — the type that carries a signature to the send path — had nowhere to put them. So every outbound email showed the legacy `license_number` line while Settings → Profile promised badges "shown on your reports, emails, and booking page". The preview was fixed in 44bfaa4. This is the other half: both resolvers (`resolveSignatureInspector` for an inspection's inspector, `lookupSenderSignature` for the acting user) now populate credentials, and every call site already passes the object through whole, so they reach the renderer unmodified. THE OPTIONAL FIELD IS WHY THIS WENT UNNOTICED, so the fix is structural rather than a promise to remember. `SignatureUser.credentials` has to stay optional — the renderer accepts callers that predate Spec B — but the resolvers now return `ResolvedSignature`, where it is required. The next omission is a compile error instead of a silently emptier email. THE ASSERTION THAT MATTERS IS THE ABSOLUTE URL. A spec checking only "credentials were passed" would pass while every recipient saw a broken image: the stored `imageUrl` is root-relative and a relative `src` inside an email resolves against the recipient's mail client, which is nowhere. The spec pins `src="https://<host>/api/public/brand-asset…"` AND the absence of any `src="/api/public…"`, plus the text fallback — mail clients block remote images by default, so a credential that exists only as an `<img>` is one most recipients never see. `CredentialService.listRenderable` replaces three hand-written copies of the same six-line mapping (booking's footer, the Profile preview, and this), which is how the badge URL form comes to differ between the email a client receives and the page they land on. Its spec covers the parts a re-implementation gets wrong: url-encoding a key containing a space, the inspector's own sort order, dropping inactive rows, and dropping a row that is neither badge nor label — a credential row is created BLANK and filled in, so an abandoned one would otherwise render as an empty chip. The report cover strip and report signature block are still fed nothing; that is the snapshot work, which is a larger piece. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Settings → Profile had six sections and ONE save affordance, floating over all
of them and owning one. The other five saved on upload, on toggle, on sign, on
blur, on click. So the sticky "Save Profile" bar taught the wrong rule in both
directions: a reader who edited a credential saw it and assumed nothing had
been saved yet (it had), and a reader who edited their name watched it follow
them down the page with no sign of what it belonged to.
The button now lives INSIDE the profile card, and the form contains only that
card. Every other section saves itself — which was already true of four of
them, and is now true of the email-signature toggle as well. It was a checkbox
inside the profile form, saved by the page's button along with name and phone:
a control that looked self-contained and was not.
MOVING IT OUT SET A TRAP, which is the part worth reading. The old code did
`fd.getAll("signatureEnabled").at(-1) === "true"`. On a form that no longer
carries the field that evaluates `undefined === "true"` and writes FALSE — so
saving an unrelated profile field would quietly switch an inspector's email
signature off, with nothing on screen to say so; they would find out from a
recipient. `signatureEnabledFromForm` makes absence distinct from false, and
its spec turns red the moment the guard is removed.
The saves with no button now have to be confirmable, or "no button means it
saved" is a claim the page cannot back:
- credentials save on BLUR, the most invisible save here — nothing moved when
it worked and nothing moved when it did not;
- a FAILED photo upload said nothing at all (success reloads the page, and
failure left the old photo sitting there looking untouched);
- and all four credential handlers `await`ed the call and returned
`success: true` whatever came back, so a rejected write reported as a save.
Survivable while nothing rendered the result; not survivable now.
The two signature cards moved into their own module, because holding this rule
in the route meant the route also held two fetchers, a toast, the pad's state
and their markup on top of the one form it actually submits. Each card owning
its own is what makes the rule legible rather than asserted. Their specs pin
that neither grew a submit control.
Verified in the browser before the extraction: one submit button, inside
`#profile-details`; no sticky bar; the form contains only that section; the
toggle persists (`is_signature_enabled` 1 -> 0) and reports "Saved"; and a
profile save afterwards leaves the flag alone. Both themes checked by computed
style — card, divider and button tokens all resolve per theme. Chrome's
screenshot transport failed partway through (a CDP parameter error), so the
post-extraction pass is render specs and type-check rather than a picture.
File-size baseline: +15 after extracting 90 lines out.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Spec B §1 asks report surfaces to snapshot credentials and the resolved layout
at publish while live surfaces read current state. The snapshot captured
`{ inspection, data, units }` and nothing else, and the report resolved
credentials LIVE on every read — so an inspector who left an association
silently rewrote the cover of every report they had ever delivered, including
ones a client downloaded months earlier and may be relying on.
TWO FINDINGS THAT CHANGED THE SCOPE, both the opposite of what the plan assumed.
First, the plan's audit said the report cover and signature block were "fed
nothing" (`grep inspectorCredentials server/` → zero hits). That is not true and
has not been since InspectorHub#260: the payload carries them, the response schema declares
them, the loader reads them. What was actually missing was not the wiring — it
was that the wiring resolved LIVE.
Second, and this is the one worth keeping: growing the snapshot was assumed to
need a dual-basis verifier, since every existing version was hashed without
these fields. It does not. `content_hash` is the SHA-256 of the stored
`snapshot_json` STRING and `verifyByToken` recomputes it from that same stored
column, so a row written under the old shape keeps hashing to exactly what it
hashed to. No versioned hashing basis, no migration of signed rows. A spec now
pins that directly by re-signing a row in the old shape and verifying it —
because if someone later "simplifies" the verifier into re-serialising a parsed
object, every report issued before today starts reporting as TAMPERED on its own
verification page, and nothing else would catch it.
`schemaVersion` still earns its place, for readers rather than hashes: a v1 row
has no `inspectors` and a v2 row may have an empty one. Identical as JSON,
opposite as a claim on a cover page — "this predates the capture, live is all
there is" versus "this inspector held none". `pinnedLeadCredentials` returns
null for the first and `[]` for the second, and its spec pins that they are not
the same answer, because `?? live` silently does the wrong thing on one of them.
`inspectors` is a LIST from day one though only the lead's badges render
(option A, matching the report's single name and single signer). Whether to
credit helpers is a product call; what this fixes is that making it later must
not mean migrating every stored snapshot. The role travels with the credentials
because a badge is a claim about a PERSON on a document about an INSPECTION —
an unattributed pool would state something neither of them said.
THE PER-VERSION PDF WAS NEVER FROZEN. `verify.ts` built a render URL naming no
version, so the "immutable" artifact was generated from the LIVE page the first
time anyone downloaded it — which may be long after publication. The freezing
was an R2 cache key, not a property of the document. The version now travels
into the render, INSIDE the signed token rather than as a query param: a link
holder who could append `&v=1` would be asking the renderer for a version they
were never sent. A spec re-encodes the token body with a different version and
asserts it fails to verify.
Scope, stated plainly: pinned reads take credentials and the resolved style
preset from the snapshot. Sections, ratings and photos are still derived live
from the template, so a template edited after publication still moves them. That
is the remainder of option A and it is a larger piece — this commit is the part
that closes the credential requirement, not the whole read path.
Three grandfathered files bumped (+29/+5/+2); the snapshot loader was extracted
to `lib/report-snapshot.ts` rather than left in a 920-line assembly service,
which took the largest of those from +60 down to +29.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
`users.license_number` predates `inspector_credentials` and is still the only source of the license line on two surfaces — the email signature footer and the PDF footer. Retiring the column is the next step; this one makes the data exist in the new shape FIRST, so those surfaces can move over without a window in which an inspector's license silently vanishes from a document. `sort_order = -1`, not 0. The state license is the one credential with legal weight, and letting it land wherever insertion order puts it among voluntary association logos is the wrong answer even though it looks cosmetic. IDEMPOTENT, and the guard keys on the NUMBER rather than the label — an inspector who already typed their license in under their own wording must not end up with two of them. Removing the guard turns two specs red. The spec runs the REAL migration SQL rather than a reimplementation, because the thing that can be wrong here is the SQL: a guard that does not guard, a filter that misses soft-deleted users, whitespace that counts as a license. A hand-written equivalent would only test the equivalent. It locates the file by NAME rather than by sequence number, so a squash that renumbers migrations cannot break it for a reason unrelated to the backfill — the same reason `lint:migrefs` forbids those numbers in comments. The column is deliberately NOT dropped here. Backfill, ship, verify the surfaces render, drop later — and per the Schema Rules a retired column is frozen with a comment rather than dropped anyway, since D1 cannot drop a column on an FK-referenced table. Verified against the real local D1: one row after running the file twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
…censes & affiliations Steps 4 and 5 of the credentials plan, together, because the rename only stops being a lie once the field it collides with is gone. THE FIELD HAD SIX READERS, not the two the plan listed. Alongside the email signature and the PDF footer: the agreement sign-effects block, the booking confirmation footer, the report payload's `inspectorLicense`, and the publish gate. All six now read `CredentialService.primaryLicenseNumber`, which is defined as "first active credential carrying a member number, in the inspector's own order" — and that rule works precisely because the backfill seeded the licence at `sort_order = -1`. That sort order was chosen for this, not for looks. `inspector-signature.ts` no longer renders its hard-coded "Licensed home inspector · <n>" line at all. The licence renders with the other credentials, under the label the backfill gave it, because two sources for one line is how a recipient ends up reading their licence twice. A spec pins the stronger claim that a caller still passing the frozen field gets NOTHING extra — that is the one that matters while any caller lags. `users.license_number` is FROZEN, not dropped: D1 cannot drop a column on an FK-referenced table, and per the Schema Rules a retired column keeps its name forever with a DEAD comment. `RENDER_VERSION` r10 -> r11 so cached PDFs are re-rendered rather than serving a footer built from a column nothing reads. THE RENAME. Both words are industry-standard and neither is ours: Spectora calls its licence box "Credentials", so "Credentials & badges" sitting NEXT TO a "License #" field read as two different things to exactly the users we import from — when the intent was that they are one thing. Inspectors' own sites say "Affiliations". The section heading, the empty state, the add button and the two placeholders all move; the table, the API path and the code identifiers do not, because `inspector_credentials` is accurate and renaming a shipped table buys nothing. The subtitle promised "shown on your reports, emails, and booking page" while two of the three were empty. It is true now — reports since the snapshot work, emails since the send-path fix — so the copy is corrected rather than softened.⚠️ DEPLOY ORDER IS LOAD-BEARING. Nothing reads `users.license_number` after this, so `db:migrate:remote` (the backfill) MUST run BEFORE the worker deploys. Deploying first leaves every licence line blank — PDF footers, email signatures, agreement blocks — until the migration lands. And per the D1 SOP, that remote migration needs a `d1 export --remote` first, because 0020 on this branch is a hand-edited full-table rebuild. Full API suite green (4134); the one unrelated pre-existing failure is the MCP openapi-snapshot drift check, which this commit regenerates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
`npm run lint` is the only rung that runs knip, so these accumulated across the branch without any commit gate seeing them. Six dead exports, all the same shape: a type used inside its own module and exported as though something outside wanted it. `loginPathFor` is mine, from the agent-logout fix. Its two callers are both in `session.server.ts` and the specs reach it through `requireToken`, which is the surface that actually exists — so the export was aspiration, not API. `ChannelId` was a pure pass-through re-export from `NotificationSettings`, and removing it orphaned the import that fed it — which eslint caught at the commit gate, since an unused import is an error there while the dead export was invisible to it. The other four (`ChannelState` x2, `SmsConsentState`, `ScreenRow`) are internal to their modules. Nothing changes at runtime; the point is that the next real dead export is visible instead of being the seventh line of a list nobody reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
The last of the knip findings. Zero imports across app/, tests/ and server/; the component specs drive interactions through `fireEvent`. Its own commit because package.json is one of the paths that escalates the pre-commit type-check from the api tier to the full one AND makes `vitest --changed` degrade to the whole suite — so mixing it into a code commit costs several minutes for a one-line deletion. Lockfile edited by `npm uninstall` rather than regenerated: 346 linux entries still present, so the cross-platform optional deps are intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
The naming rule says boolean columns carry an `is_`/`has_` prefix, and `lint:naming` enforces it — but that gate runs only in the full `npm run lint`, never at the commit gate. So the column shipped bare and stayed that way for a dozen commits on this branch, which is the whole argument for running the full suite before pushing rather than trusting the hook. RENAME, not drop-and-add: D1 rebuilds a table to drop a column, and a rebuild is the operation this branch has already been bitten by once. `ALTER TABLE ... RENAME COLUMN` touches nothing else. The drizzle PROPERTY stays `enabled`, so no call site, no Zod field and no API response moves — the change is entirely at the DB boundary, where the rule applies. `db:generate` could not produce this (its rename-vs-drop prompt is interactive), so the SQL, the meta snapshot and the journal entry are hand-written; `db:check` confirms migrations and schema still agree at 86 tables. Full lint now passes every gate: 0 eslint errors, and DS, SVG, erasure, migration-refs, tenant-scope, status-literals, capability-decl, provider-helpers, notification-dispatch, deadcode, timestamps, tz, i18n, i18n-catalog and naming all green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
A defect in 20e1ae8, found by checking a scope claim I had made rather than by anything failing. Pinning the badge strip while leaving the two fields BESIDE it to resolve live meant a pinned per-version read produced three answers about one person from three places: inspectorCredentials frozen, from the snapshot inspectorLicense live, via primaryLicenseNumber on current rows inspectorName live An inspector who renews their licence therefore gets a document showing the old number in the cover strip and the new one on the signature block. Same page, two numbers, both ours. That is the "two sources for one line" failure I wrote a commit message about two commits earlier, reintroduced in the same document. `pinnedLead` replaces `pinnedLeadCredentials` and returns the WHOLE person, because the fix is not "pin one more field" — it is that name, licence and badges are three facts about one person and have to come from one place or the report contradicts itself. The null-vs-`[]` distinction survives intact: null means the snapshot cannot answer (no version pinned, a v1 row, an empty list) and live applies; a lead with `credentials: []` is a real answer and must NOT fall through, or live state resurrects badges the delivered document never carried. `primaryLicenseOf` is now a free function over a list rather than a method that queries. That is what made the bug possible: the rule lived inside the DB read, so the pinned path could not reach it and called the live one instead. One rule, two sources. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
…s on `loginPathFor` decides which sign-in page a session ends on by reading the request path. That removed a per-caller argument a new route could forget — but it only MOVED the forgetting. A page mounted inside `agent-layout` without the prefix silently gets the STAFF login, which has no account for an agent and, under `APP_MODE=saas`, bounces on to the portal's sign-in. Nothing would fail: the session specs pin the routes that exist today, and a wrong redirect is a semantically wrong string, not a type error. So the convention now has an enforcer. One rule, one file, no exemptions list. The specs deliberately cover how a gate like this fails SILENTLY rather than just the happy path, because that is the failure this repository keeps hitting: it fails loudly when the layout block is missing, and when the block contains no routes at all. The obvious implementation — slice to the first `]` — would stop inside a nested layout and skip everything after it while printing OK; there is a spec for that too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Found by opening the pages, which is the gate no hook can run. All three were invisible to type-check, to the unit suites and to every conformance gate. THE SIGNATURE PREVIEW WAS DARK-ON-DARK. `inspectorSignature()` bakes literal colours into its HTML (`#0f172a` text, an `#e2e8f0` rule) and has to — a mail client has none of our tokens, and the footer must read correctly in an inbox. Dropped straight onto a themed card, that HTML was near-black on near-black in dark mode: the one surface whose entire job is showing what the recipient sees, showing nothing. The swatch now carries the MEDIUM's background rather than the app's, which is also more honest about what it is previewing. THE CREDENTIAL UPLOADER WAS THE WRONG SIZE FOR ITS COLUMN. `LogoUploader` is the Media Studio company-logo control: a wide row, 112px preview plus a text column plus 20px padding — more than the ~144px credential cell has before the caption gets a pixel. So the preview collapsed to a vertical sliver with the button and caption floating off-centre beside it. An earlier pass had widened the cell w-24 -> w-36 because the caption wrapped to three lines; that treated the symptom. It takes a `size="compact"` now: stacked, 64px preview, quieter caption. One component with two sizes rather than two components that drift. "1 YOU CHOOSE" READ AS A TALLY OF CHOICES. It counts NOTIFICATIONS — the same thing "17 ALWAYS SENT" counts — but "you choose" is a verb whose object is missing, so after unchecking every channel on the only row the reader is looking at "1 you choose" beside three empty boxes. Now "1 YOU CAN SWITCH OFF": same number, named as a capability, and a direct contrast with the section above it that says these cannot be. Verified in a real browser, both themes. Chrome MCP's screenshot transport is still returning a CDP parameter error, so this pass ran through Playwright with a locally-seeded password. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Nothing crops or compresses a credential badge on the way in, deliberately: the
uploader keeps the original format so a transparent PNG or an SVG survives
intact, and the server writes the bytes straight to R2 behind a 2 MB cap and a
mime allowlist. Every surface then scales the result to between 28 and 40 CSS
pixels.
So the allowlist permits JPEG, somebody uploads a photograph, and it is
delivered whole to draw a chip the height of a line of text. Email is where that
bites: mail clients have no `srcset`, so every recipient downloads the full
object on every open.
Measured against the photo already in the local bucket:
original 60,662 image/jpeg
v=email 5,752 image/png -91%
v=reportCover 1,418 image/webp -98%
v=reportSignature 940 image/webp -98%
SERVE-time, not upload-time, and that is the whole reason to do it this way: an
upload rule only ever helps the NEXT upload, while the oversized badge already
sitting in R2 is the one costing every recipient of every send. The transform
path already existed for photos (`serve-photo.ts`); brand-asset simply never
used it.
EMAIL GETS PNG. Outlook on Windows draws with Word's engine and shows a
broken-image box for WebP, and PNG keeps the transparency that makes a badge a
badge rather than a white rectangle. A badge that fails to render in an inbox is
worse than one that is 30% larger.
FAILS OPEN IN EVERY DIRECTION — no variant, no IMAGES binding, an SVG, an
unrecognised name, or a transform that throws all serve the original. `v` is a
string rather than a zod enum for exactly this: an enum 400s on an unknown
value, so during a rolling deploy a client holding older or newer JS would ask
for a variant this worker does not know and get a BROKEN IMAGE. Bigger than
necessary is a cost; absent is a defect.
SVG is left alone — already resolution-independent, and rasterising it would
discard the property that makes it the format the uploader recommends.
The spec caught a real one on the way: `BADGE_VARIANTS['__proto__']` resolves to
`Object.prototype`, which is truthy, so a bare index took the transform branch
with `{ width: undefined, format: undefined }`. `Object.hasOwn` now guards it,
and with `v` no longer enum-validated at the route that guard is load-bearing
rather than theoretical.
Verified end to end against the real R2 object, including both degrade paths.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Four things, all from driving the page rather than reading it.
THE UPLOAD CONTROLS OPENED NOTHING. Both the credential badge picker and (now)
the signature one were a `<button>` calling `inputRef.click()` on a
`display:none` input. That is the fragile half of the pattern: a non-rendered
input is not reliably clickable, and when the browser declines there is nothing
on screen to say so — the control simply does not respond, which is exactly how
it was reported. They are `<label>`s wrapping the input now, so the picker opens
NATIVELY with no JavaScript in the path. The input is `sr-only` rather than
hidden, so it stays a rendered, focusable control.
The same `hidden` + `.click()` pattern is inside shared-ui's `FileDropzone`, so
that is fixed at the source rather than per-caller — every consumer benefits,
including the CSV import.
A REFUSED BADGE UPLOAD SAID NOTHING. The API answers
`{ success: false, error: { message } }` and several call sites read
`err.message` off the TOP level, which always misses — so a 3 MB file hitting
the 2 MB cap collapsed to "Save failed", and a rejected upload was
indistinguishable from a dead button. `apiErrorMessage` reads the nested field,
and the reason now appears on the ROW it belongs to: a toast cannot say which of
three uploaders refused.
THE SIGNATURE WAS NEVER SHOWN. The card said "Signature saved." and displayed
nothing, so the one thing worth checking — that the mark captured is the one
you meant — was the one thing the page would not tell you. It renders now, and
it can be UPLOADED as well as drawn: an inspector with a scanned signature has
no reason to redraw it with a mouse, and one without a scanner cannot upload, so
the two are siblings rather than a primary and a fallback. Raster uploads are
downscaled through a canvas before they become a data URI, because the column is
TEXT and is read on every report render.
The card is now "Signature" rather than "Saved Signature" — the latter names a
state, not a thing — and the swatch is a signing LINE rather than an image
frame: a white field with a hairline baseline, the way a printed form presents
the space you sign. Empty, it is the same line, an invitation rather than a grey
box. The rule is a literal slate hairline because it sits on a fixed-white
field, where a theme-aware border is invisible in one of the two themes, which
is what it was.
AND THE SUCCESS BANNER IS A TOAST NOW. A full-width green bar between the
heading and the mark pushed the card open and said "Signature saved." directly
above a signature the reader could already see — a receipt for something
visible. The mark changing is the confirmation. A file WE refused stays inline,
next to the control they will use again: no request was made, and the fault is
in their hand.
Verified in a real browser on the HMR server, both themes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Settings -> Profile takes three images and did something different with each. The photo went through a cropper; the signature went through a hand-rolled canvas downscale no other surface knew about; the badge went to the server byte-for-byte, up to 2 MB. So the two images that get composited onto a published report were the two with no way to straighten a photographed scan or trim the paper around the mark. They share PhotoCropper now, and it grew the thing all three needed: - ROTATION, as two quarter-turn buttons. Every source that arrives sideways arrives sideways by a quarter turn. The bake rebuilds the rotated space the crop rect was measured in before indexing into it; getting that wrong does not throw, it silently returns the wrong region. rotatedBounds is pure and tested. The zero-rotation path is the previous single drawImage, untouched, because every photo crop in the inspection editor takes it and the rotated path costs a second full-size canvas. - PNG OUTPUT where transparency has to survive. A signature and a badge are cut out against transparency and JPEG has no alpha channel, so as a JPEG either lands on the report cover as a white rectangle. Verified end to end: the stored badge is PNG colour-type 6 with corner alpha 0. - cropShape, so AvatarCropper became a thin wrapper rather than a second copy of the cropper. That copy is why rotation would otherwise have reached inspection photos and not profile photos. Three bugs found by actually driving it in a browser: - boundedSourceUrl appended ?w=4096 to blob: URLs. That does not weaken the request, it unresolves the URL — and every image picked off disk here is a blob URL. The cropper renders the raw URL so the file looked fine right up to Save doing nothing at all. A bake that throws now says so instead of leaving the button looking inert. - The email-signature preview absolutized badge URLs against the in-process API request's host, which is not the host the browser is on. Preview badges now render relative and resolve against whatever origin is serving the page; email keeps absolute, since a mail client resolves nothing for us. - users.photo_url stored that same wrong absolute origin. Every consumer is a browser surface, so it stores a relative path now — also no longer a hostage to the deploy origin never changing. ProfilePhotoCard is extracted for the same reason the signature cards were: it owns its own save, and the route was carrying its fetcher, toast, state and cropper on top of the one form it submits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Ticking "Auto-sign this report on publish" and publishing did not sign the report. The checkbox fired its own fetcher; the publish button fired another; and the publish handler decides whether to sign by RE-READING the inspection row. When the toggle lost that race the handler read the old value and the report went out unsigned — while the flag still landed, so the next publish signed. That is the worst possible shape for this bug. Nothing on screen says the report was published unsigned, and the second attempt working teaches the reader they mis-clicked the first time. What actually happened is that a client received a report the inspector had explicitly asked to sign. Found by publishing a real report locally, not by reading the code: first publish left `_inspector_signature` absent in inspection_results.data, second one injected it with the flag already persisted — which is what identified a race rather than a rendering fault. The fix follows the `markComplete` precedent already in this handler: the publish request carries the value, and the server writes it and awaits before publishing. The checkbox now only moves local state, so there is no second request left to race with — and a choice the reader can still cancel out of is no longer persisted behind their back. The now-unreachable `toggle-auto-sign` intent goes with it. The specs assert ORDER, not just that both calls happened — the racing version called both too. All three fail against the old shape. Verified end to end: with the flag reset to 0 and no signature present, ticking the box and publishing with no delay between the two now signs on the first publish, and the report renders the signature image. file-size baseline bumped for two files already far over the 400-line cap (2489→2496, 496→501); the growth is comments, and splitting either is a refactor this fix does not justify. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
The everyday web read resolved credentials from live tables. Only the
render-token path — the verify page's frozen PDF — passed a version, so
an inspector who renewed a licence or left an association silently
retro-edited every report they had ever issued, on documents carrying a
signature and an integrity hash asserting they had not changed. What was
shipped was option B; the decision on 2026-07-31 was A.
The recipient track now resolves the latest published version and renders
its snapshot. The version is server-derived, like the render token's, so a
link holder still cannot ask for one they were never sent.
Two exclusions, and they are NOT the same shape as the access gate — which
is why `shouldPinLatestPublished` is its own pure function beside it rather
than a second inline `renderMode || ownerPreview`:
- render token keeps the version it names; the verify page materialises
ONE version, and "the latest" would be a different document than the
hash being checked.
- owner preview stays LIVE. The owner is the author and preview exists to
show work in progress; pinning it would hide every edit since the last
publish. This is the plan's own split — publish → snapshot, live/draft
preview → read current state.
A draft has no version row and falls through to live.
Two existing specs asserted the OPPOSITE, with a comment saying a version
number here "would mean an ordinary client read was being served a frozen
snapshot" — the option-B intent, written down as a guard. Left alone they
would have blocked this change. Inverted, with the reasoning replaced.
Verified by the assertion the plan named and said nobody writes: publish,
change the credential, re-read. Recipient (seeded token, no session) gets
the frozen TX-INSP-9001; the owner's browser, same inspection, same moment,
gets the live value. Both halves, in one pass.
Also corrected a comment claiming the payload advertises
`snapshotSchemaVersion` — nothing emits that field. It now states the
overlay's real scope instead: inspector identity + credentials and the
style profile are pinned; everything else still resolves live (structure is
frozen separately, at creation, by template_snapshot and
rating_system_snapshot). Anything added to the snapshot later must be
overlaid here too or it keeps tracking live state.
file-size baseline bumped for two files already far over the cap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
The header dropped "Preview" at 2xl and "Preview PDF" at xl while Publish never dropped at all, so the entire 768-1279px band — iPad landscape, the stated field target, included — could publish a report it had no way to look at first. Sign, version history and the theme control vanished in the same band with nowhere to go: the mobile drawer only exists below 768. Rather than widen the breakpoints, spend one control where two were spent. Web report and PDF are one intent at two fidelities, never two decisions, so they merge into a single Preview menu that never hides. What the width does drop now folds into a "More" overflow sitting immediately left of Publish, and that menu hides itself at xl where everything it holds is inline again. Membership is decided by one table both sides import (header-visibility.ts) rather than by matching class strings written twice — the two lists must be exact complements, and a control in both is duplicated while a control in neither is unreachable. That is the failure this commit is fixing, so it gets a mechanism rather than a comment. Also: the mobile drawer's per-item Preview tab was labelled with the header's full-report Preview string — two different destinations, one word. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
Exported for the first draft of the visibility spec, which keyed its assertions off it; the rewritten spec reads the table's own entries instead and left the type behind. `lint:deadcode` runs only in the full lint, so the commit that introduced it passed pre-commit clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
…sh on a phone
Four asks in one change, joined by one thing: a rule nobody could state was a
rule nobody could satisfy.
DROP users.license_number, do not freeze it. The backfill already seeded a
credential row per licensed user and nothing reads the column. Hand-written
rather than generated because `users` is the FK target of six tables and the
hazard here is drizzle's 12-step rebuild — CREATE __new_users / INSERT / DROP
TABLE users / RENAME — whose DROP TABLE is what loses rows on remote D1.
SQLite's native DROP COLUMN keeps the table's identity; verified on real local
D1 that every row and the FK survive. (Note for whoever needs `db:generate`
next: the drizzle meta chain is already broken — no 0022 snapshot, and 0023's
prevId skips 0021. Pre-existing; `db:check` is a custom script and passes.)
BADGES CANNOT BE JPEG. A badge composites over the report cover and beside the
signature, so a format with no alpha renders the logo inside a white rectangle —
in dark mode, the loudest thing on the page.
THE SIGNATURE BADGE HAS A NAME NOW. It was an inline `.find()` in JSX, so which
of several badges appeared was an accident of sort order that no inspector could
aim at. `primaryBadgeOf` states it — first badge in the inspector's own order —
and Licenses & affiliations gains the reorder control that makes that order
settable. Ordering IS the choice; no second "primary" flag to drift out of step
with the list it describes. Both rules moved to lib/credentials/primary so the
renderer can import them without pulling drizzle into the client bundle.
A PHONE CAN FINISH A JOB. Below 768px the editor renders its own tree, and the
four dialogs that end an inspection lived only in the desktop one, below the
early return — no Publish, no Sign, no Preview, and the app bar's own "More
actions" button was a comment reading `future: open more menu`. They are now a
fragment both trees render, reached from that button.
Found by driving the browser, not by a test: reordering WIPED every label.
`UpdateCredentialSchema` was `CreateCredentialSchema.partial()`, and `.partial()`
does not remove a `.default()` — it wraps it. So `{sortOrder: 0}` validated into
`{label: '', sortOrder: 0}` and the service writes every key that is not
undefined. Pre-existing: editing a member number alone has always blanked that
credential's label. The OpenAPI snapshot had published the defaulting as
contract. Fixed by spelling the patch schema out; the spec asserts ABSENT keys,
because asserting values would have passed the whole time.
Verified in Chrome and Playwright: reorder round-trips the signature badge in
both directions with labels intact; Publish / Sign / Report settings all open at
390x844 in dark and light; the desktop tree is unaffected.
File-size baseline bumped for three grandfathered files (+6 comment, +22 an
action branch beside its siblings, +34 the shared fragment); the mobile drawer
was extracted rather than inlined.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
`db:generate` had been unusable: drizzle-kit walks meta/NNNN_snapshot.json by prevId, and two links were broken. The licence backfill is a DATA migration, so it shipped with no snapshot — it changes no DDL, which is exactly why nobody noticed one was missing — and the next generated migration then linked past it to its grandparent. The column drop added a third gap. Repaired by giving the data migration the link it still needs (its predecessor's schema under a fresh id), pointing the rename at it, and authoring the drop's snapshot as the rename's minus the dropped column. Proof it is right rather than merely well-formed: `db:generate` now reports "No schema changes, nothing to migrate", which it can only do if the reconstructed chain diffs to exactly the current schema. `db:check` stays green (86 tables both sides). The gate is the part that matters. This rotted silently across two migrations because nothing was watching, and `db:check` cannot watch it: that script APPLIES the SQL and compares the resulting tables, so a chain drizzle-kit refuses to walk still builds a correct database and still passes. The two answer different questions — "do the migrations build the schema" versus "can drizzle author the next one" — and only the first had a gate. `lint:migchain` is the second, wired into `lint` and `lint:gates-full`. Verified by re-breaking it both ways: a missing snapshot and a mislinked prevId each fail with a message naming the file and the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
The worker was being uploaded as unminified source: 177,809 lines averaging 37
bytes each. Vite turns minification OFF for SSR builds, on the reasonable
assumption that server output runs in Node where bytes are free — but here the
SSR build IS the deployed artifact, and Workers Free caps the script at 3 MiB
gzipped.
3096 KiB / 100.8% -> 1779 KiB / 57.9%
That is the whole of the bundle problem, and it explains why the diet backlog
was so hard: every previous attempt went looking for a dependency fat enough to
matter. The two big levers already spent — messaging SDKs to REST, konva out of
the server graph — bought ~134 KiB between them. A build flag nobody had set was
worth 1317 KiB. pdf-lib, the next-fattest dependency at 329 KiB and the one the
backlog earmarked next, no longer needs touching.
The other two candidate flags were measured first and are already exhausted:
the SSR target emits ZERO downlevel helpers (`__spreadValues` / `__objRest` /
`__async` all absent), and legal comments total 335 bytes.
MINIFICATION MANGLES NAMES, and `wrangler.jsonc` binds Durable Objects BY NAME,
so that was verified rather than assumed. The built entry still exports
InspectionDocDO, InspectionPresenceDO, TenantPresenceDO, InspectorMcp and
SignCompletionWorkflow under their real names; only internals are mangled.
Then exercised in real workerd (`npm run dev`, production shape): public routes,
the authenticated editor, and the report view with its badge strip and signature
block all render. `/api/…/pdf` returns 500 — and that is NOT this change: the
same request fails identically with `minify: false`, because the failure is
`The RPC receiver does not implement the method "quickAction"`, i.e. the local
Browser Rendering binding, not a mangled symbol.
Note for the record: this branch was ALREADY over the limit before this
session's work — b948a46 measures 3090 KiB / 100.6%. It had simply not been
pushed, so the pre-push gate had not yet been asked the question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
… the old door
This branch had 47 unpushed commits, so CI had never run on any of them. The
first push surfaced all of it at once. Three distinct causes, none of which the
suites I had been running could reach.
MIGRATION REPLAY (3 workers suites, one root cause). `D1Database.exec()` treats
each newline as a statement boundary, so the harness flattens a statement onto
one line. It dropped only lines that START with `--`, so a TRAILING comment
survived the filter and then commented out the rest of the flattened statement:
'Licensed home inspector', -- the string the old renderer hard-coded
became `… SELECT 'Licensed home inspector', -- the string …` and D1 answered
`incomplete input`. The migration was valid SQL; the replay was not.
Three specs each carried their own copy of that harness and all three broke the
same day — duplication that had not drifted, which is the harder kind to spot.
Now one `migration-replay.ts`, with comment stripping that respects string
literals (`'has -- inside'` is data).
MY OPTION-A REGRESSION. `reinspections.spec.ts` builds its own two-service
container, and the recipient read now resolves the latest published version, so
`c.var.services.reportVersion` was undefined → 500. The real DI registers it
(`di.ts`), so production was never affected. Registered in the harness rather
than guarded with `?.` in the route: a missing service is a DI misconfiguration,
and optional-chaining it would turn a loud 500 into a report silently served
from live tables. Same call I made for two unit harnesses earlier — I simply had
not run `test:workers`.
A STALE SECURITY ASSERTION. The agent E2E asserted that an unauthenticated hit
on `/agent-dashboard` lands on a URL containing `/login`. `loginPathFor` now
sends anything under `/agent-` to `/agent-login` — a session ends at the door it
was opened at — and `/agent-login` does not contain `/login`. The security
property the test names still holds; only the door changed. Third time this
session a test encoded a contract the code had deliberately moved on from.
Full workers suite: 20 files, 91 tests, green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HcJgdRoQzrRJ8RvEcpUDR5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
47 commits. Three threads, plus one build fix that turned out to matter more than any of them.
Notification classes and per-recipient preferences
A class vocabulary the send boundary actually knows about, a preferences table shaped by the constraint that has to hold, and enforcement at the point of send rather than at the point of display. Staff, agent and client each get their own preference screen; the screen offers every channel always, and SMS is gated on consent with an inline grant and a locked Text column until it is given.
Consent is keyed on a subject, not a contact, so staff and agents can STOP and turn texts back on through one surface with one contract.
Credentials become first-class records
The state licence moves out of a
userscolumn and becomes aninspector_credentialsrow, seeded ahead of voluntary badges.users.license_numberis then dropped outright rather than frozen.Two single-credential rules — the licence line and the badge beside the signature — are named (
primaryLicenseOf/primaryBadgeOf) and live in one module both the live and pinned-snapshot paths reach. Ordering the list in Licenses & affiliations IS how an inspector chooses which leads; there is no second "primary" flag to drift out of step.Reports render what they froze
A delivered report resolves the latest published version and renders its snapshot. Render tokens keep the version they name; owner preview stays live, because preview exists to show work in progress.
Editor reachability
Preview was gated at
2xland Preview PDF atxlwhile Publish never hid at all — so the whole 768–1279px band, iPad landscape included, could publish a report it had no way to look at first. The two previews merge into one control that never hides; what the width does drop folds into a "More" overflow. Below 768px the editor renders its own tree that had no Publish, no Sign and no Preview at all, and whose "More actions" button was a comment readingfuture: open more menu.The build fix
The worker was being uploaded as unminified source — 177,809 lines averaging 37 bytes. Vite turns minification off for SSR builds, on the reasonable assumption that server output runs in Node where bytes are free; here the SSR build IS the deployed artifact and Workers Free caps the script at 3 MiB gzipped.
This branch was already over the limit before this work started (
3090 KiB / 100.6%); it had simply not been pushed, so the pre-push gate had not been asked. Verified that the named exports Durable Object bindings depend on survive mangling, and exercised the minified worker in real workerd.Gates added
lint:migchain(the drizzle meta chain had silently rotted, makingdb:generateunusable —db:checkcannot see it, because it compares the tables the SQL builds, not whether drizzle can walk the chain), plus gates for agent-route prefixes and a cached gate runner.🤖 Generated with Claude Code