Dispatch board, request idempotency, and the batch 2-5 programme - #296
Merged
important-new merged 113 commits intoAug 5, 2026
Merged
Conversation
…y per report An order sold three services and delivered one document, because nothing created a second report: the `reports` entity could model a radon report but no code path produced one. Selling the services now produces the deliverables, and each one publishes and announces itself on its own schedule. Generation runs at the point the work is scheduled to begin, never at booking — a report that materialises weeks early clutters the order and freezes a template the tenant may still be editing. A cron sweep picks up orders whose `scheduled_start_ms` has arrived; `inspections.reports_generated_at` is its latch, so finished orders are not re-scanned for ever. Reconciliation itself is idempotent per LINE rather than latched, because a client who adds a sewer scope at the door should get a sewer report. Title and template are snapshotted when the report is created, so a catalogue default configured tomorrow rewrites nothing. The placeholder primary every order is born with is ADOPTED by the first line rather than left orphaned beside it — but only while it is still empty; a primary somebody has written into keeps its title and its template. Two notification faults fall out of publishing per report, and each is invisible on its own: - The `report.published` dedup key named only the inspection, so the radon report's first publish read as a retry of the standard report's and was dropped for ever, not for a window. The key is per report now. - `resolvePublishTrigger` asked "has anything on this inspection been published" and so called the radon report's FIRST publish an amendment, sending the client "your report was updated" about a document they had never seen. It asks per report now. Fixing those makes two documents finished in one sitting cost the client two emails, so publishes coalesce inside a one-hour window: the second report is still published, versioned and signed, only the second announcement is suppressed. An amendment is never coalesced. `reports.published_at` / `notified_at` record which document shipped and which publish did the telling — `inspections.report_status` stays the order-wide roll-up every existing reader consumes and cannot answer either question. Migration 0030: four ADD COLUMNs, no table rebuild. Verified with db:check. Not in this change: the reports LIST surface and its delete-with-confirmation modal. There is no per-report UI in the tree at all yet, so a confirmation modal would have nothing to confirm; publishing a named report is reachable through the API (`POST /inspections/:id/publish` now takes `reportId`) and that is the seam the UI will use. Two behaviour-preserving extractions to keep the publish service inside the file-size gate: auto-sign-on-publish and the per-report publish bookkeeping both move to lib/inspection. The remaining +7/+2/+4 lines on three already- grandfathered files are baselined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FGxHcE92k4t3d79y6DQfi3
NULL means no stated preference, not English. Only a stored value is evidence a client wanted another language, which is the signal this column exists to produce. resolveContactLocale owns the fall-through order in one place — contact, then the account a contact is bound to, then the company default, then the request's Accept-Language, then English — because a notification is rendered for a recipient whose language is not the request's, and in a queue or cron context there is no request at all. Each level falls through when it names a language the catalogue has no messages for, so a stored fr-FR gets the company's language rather than untranslated keys. The supported set is restated in server/ (the BFF boundary forbids importing the paraglide runtime) and the equality with project.inlang/settings.json is asserted by the spec rather than asked of a comment: resolving to a locale with no messages behind it degrades to English silently. The workers batch spec's hand-written contacts DDL gains the column too — drizzle's multi-row INSERT binds every schema column, so a missing one fails the whole batch (verified: it goes red without it).
The contact locale column landed with nowhere to fill it from. This asks the person booking, on every public surface, and stores the answer on the contact the booking creates. Nothing is selected by default, and that is the point of the change. A pre-selected English would make every booking look like a stated preference, and the reason to collect this at all is to find out how many clients actually ask for another language — which only works if silence stays distinguishable from a choice. So the payload omits the key when unanswered, and the column stays NULL. - LanguageChoice wraps the shared-ui RadioGroup and takes its option labels from the same table the Workspace and Profile pickers use, so there is one vocabulary for one choice. Labels are never translated: someone who cannot read English cannot find an option labelled in English, which would defeat the control. - Both public forms carry it — the wizard and the embed. An embed is what a company puts on its own website, so leaving it out there would have collected nothing from the higher-traffic path. - It is NOT shown when an agent books for someone else. The agent would be guessing at their client's language, and a guess recorded as a stated preference corrupts the one number this field exists to produce. - The submitted tag is reduced through the resolver's own normalizer, so es-MX lands on es-419 and a language we have no messages for is stored as NULL rather than as a promise we would break at send time. The API takes a free BCP-47 tag rather than an enum for the same reason a cosmetic field must never reject a booking. - Unlike name and phone, a locale does not fill forward: a returning client who picks a different language gets the newer answer, because being written to in English after asking for Spanish is the failure. An omitted choice still never clears a stored one. RadioGroup gains a legendClassName so one component can wear the wizard's and the embed's field-label idioms; the legend element always renders, since it is the group's accessible name. booking.service.ts is over the file-size cap already; the baseline moves 966 -> 972 rather than splitting it, which this change does not justify.
Collection so far only happens where the client speaks for themselves, on the public booking form. That leaves no way to act on the commonest correction of all — the client said it on the phone, or picked wrong — and no way to undo a mistake once it is stored. The contact form is where a record gets fixed, so this is where the field belongs. Three states, same as the booking form: not set / English / Espanol. "Not set" is an option here rather than merely the initial state, because a correction path that cannot get back to "no stated preference" is not a correction path; it is first, and nothing is pre-selected, because a pre-selected English would turn every contact anyone ever edits into a stated preference and a stated preference is the only thing the column is evidence of. The control is the shape the profile locale picker already has — a select whose first option is the empty one — over the option labels the booking form and the settings pickers share, and over the same list of tags the server accepts. One vocabulary for one choice. Server side, the thing to get right is what an update MEANS by silence: - UpdateContactSchema is CreateContactSchema.partial(), and zod's .partial() KEEPS a .default(). A defaulted field therefore arrives on every request whether the caller sent it or not, and the handler writes it. `locale` carries no default for exactly that reason, and the spec asserts the ABSENCE OF THE KEY in what reaches the service rather than its value — a default of null would pass a value check and still be a silent overwrite of a real choice. - An explicit null must still clear it, so the handler tests `'locale' in raw` rather than `!== undefined`, as it already does for every other nullable field. - The service reduces whatever it is handed through the resolver's own normalizer before writing, so es-MX lands on es-419 and a language we have no messages for is stored as NULL. Every stored value is one resolveContactLocale would hand back; anything else is a promise broken at send time. The API keeps taking a free BCP-47 tag rather than an enum, matching the booking payload.
`contacts.locale` exists to decide whether the rest of the multilingual work is worth building, so the deciding number needs a definition someone can look up — and one that cannot be quoted without what it cannot see. docs/developers/multilingual-demand-signal.md fixes the decision rule BEFORE there is data (a threshold chosen afterwards is not a threshold) and pins the denominator, which the rule alone left open. It gives four queries rather than one: the composition of stated languages, the answer rate that qualifies it, the size of the blind spot, and a seed-data check — identical per-tenant counts and a single created_at day are fixtures, not demand. The blind spot is structural. The question is only asked where a client speaks for themselves; the agent-on-behalf booking path carries no locale field, and that is deliberate, because an agent's guess recorded as a client's stated preference is exactly what would poison this measurement. Every such client therefore sits in '(not stated)' whatever they speak, so the signal undercounts by however much of a deployment's volume is agent-booked. Query C measures the floor of that gap, and the caveat is written inside every SQL block so a copy-paste cannot leave it behind. tests/unit/contacts/demand-signal-queries.spec.ts executes each block in the doc against a database built from the real migrations and asserts the counts on an adversarial fixture: an archived Spanish speaker, an agent contact with a locale, a stated `en` that is a choice and a NULL that is an absence. A renamed column now breaks a test instead of the monthly report, and a query that loses its undercount comment fails the suite.
qbo/invoice-sync.ts already computes the partial-payment branch and passes the balance, and all three adapters dropped it as _balance because markPartial had nowhere to put it. The 'partial' status is derived, rendered in the client hub and the report-lock notice, and until now could not say what was still owed. Stores the amount PAID rather than QuickBooks' remaining Balance: our amountCents is the authoritative total, so remaining is derived against it and does not drift when either side edits the invoice. markPaid and markRefunded both clear it, so the amount can never contradict the derived status. The dollar-to-cent conversion happens once, in applyInvoiceStatusFromQBO where the QBO shape is still in view, so MarkPartialFn now carries cents already paid and no adapter repeats the arithmetic. Each side is rounded to its own exact cent before subtracting -- truncating the float difference loses a cent on ordinary amounts (a $100 invoice with $18.15 owed yields 8184.999999999999). Migration 0032 is a plain ADD COLUMN; invoices is FK-referenced, so the column goes at the end of the table definition to keep drizzle from rebuilding it. Refs InspectorHub#273.
The balance column landed with the previous commit and nothing read it. A 'partial' invoice could state that a payment had happened and not how much, so neither the inspector nor the client could see the outstanding figure. Remaining is derived, never stored: `amountCents - amountPaidCents` against our own authoritative total, clamped at zero. An external system can report more received than we billed after a divergent edit, and a negative balance shown to a client reads as a refund nobody promised. Two states, deliberately distinct. With a recorded amount the hub card and the invoices table say "$200.00 remaining", in the invoice's own snapshot currency rather than the viewer's default. With no recorded amount — rows written before the column existed — the card says the amount was not recorded and shows no figure at all. "$0.00 remaining" would be a false statement about money, and a client would read it as nothing owed. A viewer without the `financial` capability gets neither line: the balance is not unknown to the business, only to that reader, so `redactMoney` drops the field and the card stays silent. Three payload boundaries had to carry the new column, and one was dropping it silently: the public pay endpoint parses through `PublicInvoiceBodySchema` (IA-86), so an undeclared field is stripped one line before the payer's page — the column was written, the row carried it, and nothing failed. The checkout endpoint projects columns explicitly, and `InvoiceResponseSchema` describes the invoices list. All three now declare it, each pinned by a test. The OpenAPI snapshot reduces request schemas only, so no bump was needed. The public pay page's balance and Stripe pay panel are deliberately unchanged: the payment intent is minted from `amountCents`, and showing a reduced balance beside a charge that is not reduced would state a price the payer would not be charged. Charging the remainder is a payment-collection change, not a display one. File-size baseline bumped for four files already far over the cap (1202/510/769/674) whose growth here is 1-7 lines of return type and column projection — the return type cannot live anywhere but with its method. The invoices route stayed under the cap by extracting the amount cell instead.
One equivalent per canonical product term, decided once. Without it the same
noun gets four translations across 29 module files and the result reads as
machine output. Register: formal usted, sentence case for buttons.
Usted is chosen for reach, not politeness: es-419 spans voseo countries where
the tu imperative is audibly foreign, so usted is the only second person that
is correct across the whole region.
The glossary is machine-read rather than advisory. lint:i18n-glossary parses
its own tables and holds messages/es-419 to them:
- banned terms — only context-free wrong words are listed, so a hit is real
- consistency — character-identical English must have identical Spanish,
which is what stops Satisfactory reading two ways across
the five modules that use it
- placeholders — {name} tokens must survive translation; a dropped or
renamed one compiles to a different function signature
It also fails when it cannot trust its own inputs: markers gone, tables shrunk
below a floor, or a divergence entry naming a key that does not exist. A gate
that silently reads nothing passes everything.
The 15 login-pilot keys predate the glossary and were written in tu; they are
re-registered to usted here so the gate is green at rest rather than carrying a
standing exception.
First module end to end, to surface the mechanics at a scale where a mistake is
cheap. Every one of these 18 keys is reused across the app -- 203 call sites,
65 of them common_cancel -- so they set the vocabulary the other 28 modules are
held to.
Register is usted, per docs/developers/i18n-glossary.md. Terms come from the
glossary tables; the four words not in a table (Close, Copied, Done, Undo/Redo)
are single-word UI verbs with no regional split.
Proved the gates actually bite before trusting them. Deliberately broke each
check and watched it fail: an empty value ("key": "") -- the shape a translator
leaves behind when they skip a hard string -- fails lint:i18n-catalog naming the
key; a banned term, a tu-register possessive, a dropped {placeholder} and the
same English translated two ways across modules each fail lint:i18n-glossary.
Verified rendered, not just compiled: with PARAGLIDE_LOCALE=es-419 the server
renders lang="es-419" and the Spanish strings with JS off, so the locale is
resolved in the paraglide ALS scope and not patched in at hydration. Light and
dark, 1440px and 390px, no console errors. Spanish is ~31% wider than English
across these 18 strings (Add -> Agregar is +96%), but nothing clipped: the
surfaces checked size to their widest label, not to the English one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FGxHcE92k4t3d79y6DQfi3
Carries the severity scale the rest of the catalogue inherits: Satisfactory / Monitor / Defect become Satisfactorio / Vigilar / Defecto, per the glossary's rating table. Settles the one decision this module could not avoid. A status word attaches to a feminine noun in one module and a masculine one in the next -- inspeccion, informe, factura, acuerdo -- while the consistency check forces exactly one Spanish string per English string. 'Published' alone labels an inspections tab and two report states in this file. Status labels are therefore masculine singular, agreeing with the implicit estado, which is the only form that needs no divergence declaration anywhere in the remaining catalogue. Prose hints still agree normally: 'Cancelled inspections' is a sentence, not a chip. The glossary said Published -> Publicado and Scheduled -> programada in adjacent rows; that contradiction is resolved here rather than left for the next module to guess at. Coverage 33 -> 132, exactly the 99 keys this module holds.
The severity trio repeats here (repair_items_severity_*) and matches labels.json exactly, which is the point of doing these two together. Two collisions worth naming. 'Tag' and 'Label' are both etiqueta: they never share a surface, and inventing rotulo for one of them would spend a word nobody uses on a clash nobody sees. 'Est. min'/'Est. max' stay abbreviated -- the field is a narrow numeric input, est. abbreviates estimado in Spanish exactly as in English, and the glossary's ban on estimado is about the noun for a priced offer, not this abbreviation. Both are now glossary rows. Marketplace is left in English: it is a feature name, it is the word the region's software uses, and the MP badge that abbreviates it has no Spanish equivalent. Coverage 132 -> 323, exactly the 191 keys this module holds.
Third file carrying the severity trio (templates_mapping_row2_*), and it agrees with labels.json and library.json. The Spectora mapping panel keeps Spectora's vocabulary on the left-hand side: 'Orders' is translated as Ordenes because that is what the competitor calls the thing being mapped away. The glossary's ban is on 'orden de trabajo' as a name for our own Inspection, which this is not. Coverage 323 -> 437, exactly the 114 keys this module holds.
Counsel advised against embedding InterNACHI's governing-language clause as platform contractual language: it allocates risk between the tenant and their client, and we are not a party, author none of the agreement text, and control none of its terms. This states a fact and decides nothing. A test forbids govern/prevail/controls/ binding/shall, because "strengthening the wording" is the edit that would undo it. Containment is structural, not advisory. The copy is a <section role="note">, and the agreement body's write-time sanitizer allows neither -- so the disclosure cannot pass through the agreement pipeline and come out intact. Two guards hold the line: the spec asserts the agreement sanitizer destroys it, and a source scan asserts nothing on the agreement-body path imports the module. DOMPurify is deliberately not exercised here. Under happy-dom it drops the outermost element and applies no allow-list at all, so a round-trip assertion would have been a green that proved nothing about a browser. The spec reads the two real allow-lists from source instead.
…not in it Three surfaces show an agreement to a signer: the standalone signing page, the same section inside the client Hub, and the checkout sign card. A fourth — the archived copy rendered for the signed PDF — is the one a dispute produces. All four now carry the platform language disclosure, and none of them puts it in the agreement body. That distinction is the whole point. "Append it at render" and "append it to the body" produce identical screens and completely different legal positions: the body is a contract between the tenant and their client, which we are not a party to and write no word of. So the tests assert placement, not presence alone — the disclosure must sit outside the body container, and after a render the pinned snapshot and its content_hash must be byte-identical. They are: the hash is taken over the stored string, and the disclosure never enters it, so no existing signature is invalidated. The copy gained a plain-text heading, "Not part of this agreement", held to the same no-contractual-assertion test as the sentence. Position is what counsel asked for and a reader does not infer position from a border. Two guard changes fall out of this: - The renderer sanitizes with DISCLOSURE_SANITIZER_PROFILE, never <SanitizedHtml>. That component's allow-list is the tenant rich-text one, which permits neither <section> nor role, and would deliver the sentence as a loose paragraph among the terms — the exact reading this exists to prevent. Asserted against source, because DOMPurify under happy-dom applies no allow-list at all and both components emit identical markup on the first pass. - The containment scan previously listed agreements-render.ts as an agreement-body host, which would have kept the disclosure out of the archived copy entirely. It now names only the modules that compose the stored string; the renderer is held to a stronger pair of tests instead.
Full lint at the batch boundary found the one thing the per-task hooks cannot: knip went from zero to one finding. The constant is used only by the resolver in its own module, so it stops being an export rather than gaining a consumer -- the same call the same task made for normalizeLocale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FGxHcE92k4t3d79y6DQfi3
The disclosure is on four screens as of the last commit, and nowhere in the database. That gap only matters later: bump the copy, and every signature ever collected silently becomes a signature against text nobody can identify. agreement_signers gains language_disclosure_version, nullable. Nullable is the feature, not a concession — NULL means "the platform did not draw this screen and cannot say what was on it", and two real cases produce it. Signatures older than this commit are one. The other is POST /api/inspections/:id/sign: that endpoint hands the agreement text to a caller through GET /:id/agreement and takes back a signature, so what the signer read is the caller's business, not ours. It records null. The public sign route records the live version, because the two surfaces it serves both render the disclosure component and a test says so. markSignedBySigner takes the version as a required argument and never defaults it. The service knows nothing about screens; only a caller does. An optional field would let the next sign surface record silence indistinguishable from a pre-feature signature. What the record now supports decides what the evidence surfaces may print: - The archived copy shows the disclosure only when EVERY signature on the envelope recorded the version live today. Superseded copy is archived nowhere, so against an older signature the choice is between printing nothing and printing words that signer demonstrably never read. It prints nothing. - /verify follows the same rule — Task 2 deferred it here for exactly this reason — and gets the answer as a server-decided boolean. A public page is not where that rule gets re-derived. - The certificate of completion is a different document: it states facts ABOUT the signing event rather than reproducing what was signed, so it names whatever version was recorded, superseded included, and stays silent when nothing was. Erasure lint checked rather than assumed: its PII heuristic matches none of language_disclosure_version, so no ERASURE_OUT_OF_SCOPE entry is needed until the heuristic widens. The migration is a plain ALTER TABLE ADD (column at the table end, no rebuild); db:check green at hand=87 / generated=87. Every guard here was run with its fix removed first; eleven went red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…e, in the module Counsel's recommendation was already reflected in the disclosure's shape. What was missing is everything around it: the date, the three facts that make the recommendation land, and — the part that costs something if it is lost — that California Civil Code §1632 is still an open question, and what that question gates. The reader this header is now written for is the one about to translate the agreement body. That work is blocked on §1632, it must not be started on an assumption about the answer, and translating the disclosure is not a route around it. Nothing in the module said so; a person could have read the whole file, seen a disclosure about language, and concluded translation was the obvious next step. The three supplied facts are recorded with their edges intact rather than summarised into comfort. The material one is that we hold NO record of the language a booking was negotiated in — negotiation is typically a phone call outside the software — and that contacts.locale is a stated reading preference that must not be offered as evidence of it. That distinction is the one most likely to be lost by someone looking for a field to point at. One question is deliberately unasked and now says so where it will be found: if a tenant offers a courtesy Spanish report, does that report's limitations notice suffice or does the agreement need a companion sentence? It depends on wording that does not exist yet, so asking now buys an answer to the wrong question. Whoever schedules that work owns asking it. Four assertions hold the record in place — the date, the §1632 gate and what it blocks, the parked question, and that the cited counsel document EXISTS, so a rename fails in CI rather than in a dispute. A tidy-up that prunes "background" comments is the realistic way this gets deleted. All four were run against a stripped module first and went red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
Enums, not a pattern string: three real answers do not justify a parser and a validation surface. Defaults reproduce today's output exactly (us + 12h), so nothing changes for anyone until they choose. The workers specs build tenant_configs and users from literal CREATE TABLE strings, and the cmd-apply upsert binds every schema column, so both DDLs grow the columns too. Only the tenant_configs copy is guarded by a sync test; the users copy is duplicated inline in two specs with no guard.
Per-field fallback, not per-object: a user who set only the clock keeps the tenant's date order. Same contract as useDisplayTimeZone -- which returns its default outside a data router rather than throwing, so the chrome still renders on routes that are not under auth-layout. resolveDisplayPrefs also rejects a stored value outside the enum. The drizzle enum is type-layer only and D1 has no CHECK constraint, so a bad row would otherwise reach Intl as an unknown option key. useTenantFormatPrefs is the second half of the design rule: anything a second party also reads anchors to the tenant, and that needs a hook rather than a convention every caller re-implements.
…ed formatter
format-date.ts pinned locale:'en-US' at two call sites with a comment saying
Phase A would thread the viewer's locale through; it never did, so a tenant on
es-419 read `Aug 3 · 7:58 AM EDT` -- English month, English meridiem, on a
Spanish page. The pin was three files from anything a reviewer was looking at.
`locale` is now a REQUIRED fourth argument, the same remedy already applied to
`timeZone` in this file for the same class of defect: an optional locale with an
'en-US' default would have left all fourteen call sites rendering the bug. All
fourteen now name a locale.
The date is assembled part-by-part rather than handed to Intl as an option bag,
because an option bag cannot express the requirement: Intl derives the ORDER
from the locale, so es-419 with {month:'short',day:'numeric'} gives `11 sept`,
not the American order a US company asked for. The month WORD comes from the
locale, the ORDER from the enum. Only the clock goes to Intl whole, via
hourCycle.
Language follows the viewer, shape follows the tenant. Translating a month name
cannot be misread; reordering one can, and the inspector, the client and the
agent discuss one inspection out loud.
Public surfaces have no session, so the tenant brand grows defaultLocale +
dateFormat + timeFormat. The report payload already carried the brand.
Defaults render byte-identically to before.
… the shared formatter
All five read the BROWSER's locale and the BROWSER's zone. That is not a
cosmetic inconsistency: on this machine the settings tooltip rendered
"2023/11/15 06:13:20" for an instant every other surface calls Nov 14 — a
different DAY, from the same millisecond, on the same page.
format-date.ts gains formatShapedDate / formatShapedDateTime.
formatInspectionDateTime does not fit these callers: it takes a `now` and elides
the year, which is right for a dashboard row and wrong for an audit stamp. The
new pair takes no `now`, always carries the year, and always attaches the short
zone name.
Which resolution each site uses is a decision, not a detail. The settings panels
and the Stripe delivery log are CHROME — one admin reading their own page — so
the personal override applies. Version history is TENANT-anchored: collaborators
point at a row and say "restore that one", and two dates for one snapshot is the
failure the design forbids.
Plan Risk 3 fired. v.$token.tsx had no brand and nothing in the payload
identified the tenant, so GET /api/public/verify/report/:token now returns
tenantSlug and the loader resolves the brand through resolveTenantBrand — the
one resolver every other public surface already uses. The slug is already a
company's public identifier and the reader is holding that company's report link.
The guard goes in lint:i18n, not lint:tz as the plan said. check-tz-safety is
scoped to three calendar paths and its patterns are only sound there; check-i18n
already scans app/ + server/, already excludes the formatter modules, and
already carried a test asserting the OPPOSITE of this rule ("bare toLocale is
already viewer-responsive"). Two gates demanding different things of one line is
how a fix gets reverted by the other gate. That test is inverted, and the
inversion is the proof.
useDisplayDateFormat / useDisplayTimeFormat are deleted: nothing ever wanted one
axis without the other, and lint:deadcode flagged them. useChromeDateTimeFormat
replaces them as the viewer-side counterpart to useInspectionDateTimeFormat.
The file-size baseline moves for three files, not one: the gate reads the
working tree, so it cannot be satisfied one commit at a time. VersionHistoryPanel
is this commit's; the two settings routes belong to the Settings UI that follows.
…rsonal override
Both pickers render the SAME component, and both label every option with a
worked example instead of the enum's name — "2026-09-11", "11 Sep 2026",
"14:30". "ISO" tells a reader nothing they can check, and an unverifiable label
is why somebody changes this setting twice. The sample instant is fixed rather
than `Date.now()`: a live clock renders one value on the server and another in
the browser, and the current minute teaches nobody anything.
The write path did not exist and the plan did not list it: UpdateBrandingSchema,
PatchProfileSchema, the profile GET projection and PATCH assignment, and the
branding read-back and no-config-row default all had to be taught the two
columns. Both schemas use `.optional()` with NO `.default()`, because a default
turns an omitted key into an explicitly-sent one and rewrites a preference the
caller never mentioned. tests/unit/session/format-prefs-write-path.spec.ts
asserts the ABSENCE of the key — asserting its value would pass against exactly
the broken schema it exists to catch.
Chrome caught a defect nothing else could have. Conform's parseWithZod maps an
empty string to `undefined`, so "Use workspace default" dropped the key out of
the PATCH body and the stale override survived the save. The server was right
the whole time — a hand-rolled PATCH {dateFormat:''} cleared it — and every type
and unit test passed. `overrideFieldFromForm` reads the raw FormData, where ''
still means clear and an absent key still means leave-alone. `timezone` and
`locale` shipped with the same defect and are fixed in the same loop: clearing
either override through the profile UI has never worked.
The profile hint says what the setting does NOT reach, because the answer is
counter-intuitive and every support ticket that re-opens the decision starts
with someone assuming the opposite. Verified live: with the tenant on iso/24h
and the user on eu/12h, the inspections list read 2026-08-03 · 07:58 EDT while
the settings tooltip read 3 Aug 2026 · 10:32 PM EDT. Both sides of the rule, on
one screen, at the same time.
`uq_results_report` replaced `uq_results_inspection`, but it is a unique index on a NULLABLE column: SQLite accepts any number of NULLs. Both creation paths wrote the results row without a `report_id`, so nothing errored and every per-report read matched no document or a sibling's. `createPrimaryReport` now returns its id, and both paths create the report BEFORE the results row that has to name it.
…names what it destroys One order delivers several documents, but the page showed a single report status pill derived from the inspection row — so an order carrying three deliverables looked exactly like an order carrying one. The list rides the existing hub payload rather than a second round trip. Deletion is the one irreversible action here: a report owns its findings document, its Yjs state and its version chain, and with no foreign keys nothing notices the orphans. The confirmation names the report and says that the content already filled into it is destroyed. Two refusals, decided by ONE server function the endpoint enforces and the payload reports, so the card cannot offer what the API refuses: - the primary report (the collab route fails closed without one, so deleting it makes the whole order uneditable); - a published report (delivered, and its signed versions are what let a client verify the document they hold). Also: ConfirmDialog's "Cancel"/"Delete" were bare literals, shipping untranslated chrome from all ten of its call sites. Fixed at source. And the hub facade's return type is now DERIVED from its delegate — the hand-copy had rotted past `services`, `communication` and `unlockedAt`.
Also narrows one glossary ban that this module proves false-fires: the Repair Items row ruled out the word *recomendaciones* outright, but ASTM E2018 has a "1.5 Recommendations" section (here and in pca-report.json) and public.json says "grouped under Recommendations". The prohibition is real but cannot be a machine ban, and a gate with false positives gets bypassed — so it moves to the Why column as a rule. The Estimate row gets the same treatment in prose before someone machine-bans *estimado* and breaks "Costo estimado". Adds the editor-family and ASTM PCA term tables (29 rows) so the seven remaining waves inherit the decisions rather than re-arguing them, plus a rule that a format literal a parser matches on stays English — the units CSV hint shows `label,floor` and parseUnitCsv compares against exactly that.
Adds POST /api/invoices/{id}/payments — the smallest real thing the payment
ledger makes possible: an inspector takes $200 cash at the door and says so.
One appended ledger row, no provider charge, and the paired
GET /api/invoices/{id}/payments so a surface can show the rows rather than a
single total.
occurred_at is the date the money MOVED and is REQUIRED on the wire. Tuesday's
cash gets recorded on Thursday; defaulting it to now() would leave every
reporting period quietly wrong with nothing to notice. A future date is refused,
with a five-minute tolerance for client clock skew only.
Overpayment is measured against what is still OUTSTANDING and refused unless
the caller confirms it: real overpayments happen, but the same input is far more
often a decimal-point typo. card is not an accepted method here — those arrive
from the provider with a reference, and a hand-entered one would have no
reconcilable counterpart.
Gated on the financial capability, the same gate the rest of the billing surface
wears, asserted over HTTP rather than by a unit call. recorded_by comes from the
session, never the body.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
Adds POST /api/invoices/{id}/payments/{paymentId}/corrections. The original row
survives untouched and the correction is a second row, because an append-only
ledger is only reconcilable if nothing in it is ever rewritten. Without this
shipping alongside the recording endpoint, the first typo becomes a manual
database edit.
The correcting row is a refund-kind row carrying refunds_id, NOT a signed
adjustment — the choice a future reader will want to reverse, so the reasoning
sits at the code: kind carries direction in this table and adjustment is
additive in the recompute, so a downward correction as an adjustment would have
to smuggle a negative into amount_cents, which is the one thing the schema
forbids.
It inherits the ORIGINAL row's occurred_at. The money never moved on the day
the typo was spotted, so the correction belongs to the period the mistake landed
in. Correcting upward is refused: more money than was recorded is another
payment, and recording it as one keeps both facts true.
The request body is strict. A correction is exactly the shape where a forgiving
parser does real damage, so a key the endpoint does not accept is a 400 rather
than a silent no-op on a money edit. Lowering a payment can take an invoice back
out of paid, so the report's payment gate is re-synced.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
The staff invoice list gains a Payments surface per row. Deliberately the STAFF list: recording money is capability-gated on financial and attributed to the acting user, and the client portal and checkout have no such actor. Those two also deliberately quote the full invoice total, which is a settled payment-collection decision this change does not touch. A form, not a modal chain — amount, method, date and note visible at once. The date is pre-filled with today and stays VISIBLE and EDITABLE, and the browser converts the chosen calendar day into an absolute instant because only the browser knows which zone that day belongs to. Nothing on this path defaults to now(). The rows are listed, not just a total: amount, method, the date the money moved, and who wrote it down — which is what makes a disputed payment answerable. A correction appears directly under the payment it corrects, negative, with the original still standing. Remaining balance is the prominent figure, derived from the rows against the invoice total and formatted in the invoice's own currency snapshot. Walked through in Chrome in both themes: two payments and a correction recorded end to end, an overpayment refused and then confirmed. DOM sweep found no overflow inside the modal in either theme and no horizontal body scroll; every foreground/background pair measures at least 4.76:1, after moving the balance block's labels off ih-fg-3, which measured 4.34:1 on the muted panel in light mode. 32 strings added to both catalogues. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…n internal schema
The capability scan matched requireCapability('X') inside comments, so a comment
EXPLAINING a route's gating counted as a mount and was attributed to whichever
route began above it -- calendar-items.ts reported line 34 for a fact about the
route at line 76. Comments are now blanked with equal-length spaces before
matching, which keeps the line arithmetic valid. Falsified: removing the real
declaration makes it name line 76 correctly.
A gate that fires on prose is one people learn to bypass, so this was worth
fixing at the scanner rather than by rewording the comment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
One column per schedulable person on a shared 07:00-19:00 axis, the unassigned lane pinned left, and company closures greyed across the whole board rather than repeated per column. Placement is arithmetic over the wall-clock HH:MM strings the server already resolved in the TENANT timezone - no Date math in the view, so two dispatchers in different zones see one card in one place. Out-of-axis work is clamped into view and says so rather than vanishing: a 06:00 job is exactly what a dispatcher needs to see. Cards already carry the identifiers a drop handler needs, so drag-drop lands as behavior rather than as a re-layout. Measured in Chrome, both themes: the hour gutter and the lane heading moved a token darker (3.07:1 and 4.34:1 respectively). app/components/dispatch and the route join the lint:tz scope - it is the same surface the gate exists for.
A card dragged onto a column moves in time AND in ownership in one write (PATCH /api/inspections/:id/schedule): a dispatcher's gesture moves both, and two calls would leave a window where the board shows a job at a time nobody owns. Dropping onto the unassigned lane takes the person off and leaves the time alone. Vertical position snaps to the tenant's booking_slot_interval_min, not to a prettier number - a dragged job has to land on a time the booking engine would also have offered a customer. The lattice and the day's tenant-local midnight ride along in the board payload (slotIntervalMin, dayStartMs), so a dropped pixel becomes an instant without the browser ever guessing a timezone. Conflicts stay two distinct outcomes: advisory overlaps toast and the write stands; a 409 from a blocking tenant opens a modal naming the colliding jobs and nothing was written. There is deliberately no "do it anyway" - an override would make the setting a suggestion. Drag is HTML5 DnD, not sortablejs (plan corrected at source, superproject 48a36f35): sortablejs reorders DOM children and every card here is absolutely positioned, so its drop model cannot express which pixel the drag ended at. No dependency was added either way.
The wizard's date picker asks "when do you want it"; this asks "when could it
actually happen", and answers with the part the public booking surface
deliberately withholds - WHICH inspector is free.
New authenticated route GET /api/schedule/day-slots, composed onto the existing
/api/schedule router rather than mounted second at the same path: two
.route('/api/schedule', ...) calls work at runtime but the RPC TYPE keeps only
the first, so the BFF client silently loses every route on the second one.
Gated on requireCapability('scheduleOthers') like the rest of this plan, and a
caller-supplied userIds list NARROWS the qualified set instead of replacing it -
an id from a query string must not become bookable by being named.
Duration is resolved client-side rather than pushed into getTenantSlots: the
service reports slot STARTS, and whether a two-hour job fits at 10:30 is a
question about consecutive starts that the response already answers. Changing
that signature would have reached into the public booking path to compute
something the caller can derive. startsFittingDuration requires every slot the
duration spans to exist, follow on at exactly the tenant interval, and be free -
the contiguity check is what stops a lunch-break gap reading as an opening.
Slots arrive through a resource-route loader (BFF), and a failed lookup is
reported as a failure, not as an empty day.
New route, so the OpenAPI snapshot was regenerated (npm run mcp:snapshot).
Dispatch reaches the sidebar, gated on the scheduleOthers CAPABILITY rather than on a role tier - an inspector granted the override may dispatch and a manager whose override was revoked may not, and the shipped server guards key on exactly that. Getting there needed three pieces, because "the session profile already carries it" was not true: GET /api/session/context now resolves the viewer's capabilities with the same getCapabilities the API guards use and ships the ANSWER (not the raw overrides, which would make every consumer re-implement an authorization rule); SessionContext gained the field plus useCapability / useCapabilities; and BOTH nav surfaces filter through one visibleNavItems - filtering only the desktop sidebar would leave the mobile drawer offering a link that redirects. Filtering fails CLOSED: no context means no entry. The server refuses either way; this only decides whether to offer the door. The calendar's "Dispatch view" button is gated on the same resolved capability. The existing canManageTeam = isAdminRole uses are left alone - reconciling /api/calendar/items with the capability is a separate, already-recorded gap. Mobile: the board stays desktop-first and scrolls its columns sideways rather than stacking, because a stacked board is a list and a list cannot show two people's 10:00 at once. The page says so where the gesture is needed.
…t state Found in Chrome, not in the unit tests: `dragstart` set `draggingId` state and the drop handler read it out of a closure, so a drop that landed before React re-rendered saw `null` and silently did nothing. The unit tests missed it because testing-library wraps every fireEvent in act(), which flushes state between the two events - a real browser only usually gives you those frames. The id now travels through `dataTransfer`, which is what it is for and which needs no render in between; state stays the fallback and still drives the hover indicator, where a frame of lag is invisible. The lane's dragover preventDefault stopped being conditional on the same state for the same reason. The regression test fires a drop with NO preceding dragstart - that is the race made deterministic - and the test helper now carries a real dataTransfer rather than a spy, so the drop reads back what dragstart wrote. Verified end to end against local D1: an unassigned card dropped on an inspector's 11:00 lands in that column at 11:00-13:00 (duration preserved) and leaves the lane.
# Conflicts: # scripts/tenant-scoping-baseline.json # server/services/auth.service.ts
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…ly on success The in-flight guard is a ref, not fetcher.state. Measured here: with a state-only guard, two clicks dispatched inside one act() BOTH reach the action — submit() does not flip state before the handler returns, so both halves of a double click run inside a single render. The key holds across a failure (the retry must be the same request) and rotates on success (otherwise the next thing the user creates replays the first one's stored response and they are told it worked while nothing was written — a worse consistency violation than the duplicate this prevents). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…it (portal InspectorHub#105) Production, 2026-08-05: one tenant created three byte-identical inspections seconds apart. Create was a bare `fetcher.submit` — nothing checked whether a submit was already running — behind a button whose only disabled condition was the step's validation. Every impatient click was another inspection. The wizard now submits through useGuardedSubmit, so a second call in the same tick is dropped and every create carries an idempotency key for the server to dedupe on. The button also goes dead and spins while the submit is in flight: the guard alone makes the page look broken, which is what invites the second click in the first place. Spinner borders use border-current, so it is the button's own text colour in either colour scheme. The regression test dispatches both clicks inside one act() — React renders nothing between them, which is what a real double click looks like, and why a `fetcher.state` check cannot see the second one. Against the old code it records two create submits. file-size baseline: NewInspectionWizard.tsx 530 → 539 for the guard's comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
Mount the guard after jwtAuthMiddleware (tenant-scoped keys) and give the email send path the same claim, so a replayed send reaches neither the provider nor the meter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…te (portal InspectorHub#105) The middleware keys off the Idempotency-Key header; useGuardedSubmit can only put the key in the form body (fetcher.submit takes no headers, and the wizard posts to the React Router action, not to /api). Nothing carried it across, so both halves were live and neither guarded anything. The create action now lifts IDEMPOTENCY_FIELD off the submitted form data onto the header. buildCreateInspectionJson is an explicit whitelist, so the field is dropped from the JSON rather than persisted onto the inspection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…actually log in No seeded account could log in, and in fact no seeded row existed at all — global-setup catches seedFixtures' throw and downgrades it to a warning, so the first failing statement disappeared into the log. Four independent defects: 1. d1() addressed a database named 'openinspection-standalone-db', which is in no config in this repo, and passed no -c, so wrangler auto-discovered only wrangler.jsonc. Now addressed by BINDING (DB) against the resolved config, the way global-setup.ts already does it. Multi-line SQL is flattened because execSync goes through cmd.exe on Windows, where a newline ends the command. 2. The password hash was base64 with an iterations segment and no 'pbkdf2:' prefix, so verifyPassword took the legacy SHA-256 branch and could never match. Replaced with the format hashPassword() emits. 3. Users carried role 'admin', which is not in ROLES — requireRole never matches it and getCapabilities has no entry for it. 4. Tenant A was '…0aaa'. Standalone login resolves the user under SINGLE_TENANT_ID || all-zeros and never from the email, so a user in any other tenant is unloggable whatever its hash says. Tenant A is now the standalone tenant; tenant B stays separate for the multi-tenant fixtures. Inspection rows also named price / payment_required / agreement_required (now price_cents / is_payment_required / is_agreement_required) and used 'draft' and 'delivered' as order statuses, which are not in INSPECTION_STATUS. Verified locally: POST /api/auth/login as admin-seed@seed.test returns 200 with a session cookie, and GET /inspections renders the dashboard for 'Seed Admin' with the seeded '1 Empty St' inspection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…ase C Task 7) Two specs, plus the first HTML5 drag-and-drop helper in the suite. Playwright's mouse-based drag emits pointer events only; dragstart/dragover/drop are browser-native and never fire, so a board that reads the card id off dataTransfer sits perfectly still. helpers/html5-drag.ts builds one real DataTransfer, stashes it for the length of the gesture, and dispatches each DragEvent against it in a separate round trip so React can render between them — fired in one block, the hover indicator and the onDragOver guard are both dead. The spec registers its own playwright project; without one a new file in tests/e2e/ runs zero tests and reports green. The first spec caught a real defect, fixed here. listCalendarItems compared the raw inspections.date TEXT against the window bounds, but that column holds either YYYY-MM-DD or a datetime — the schedule endpoint deliberately keeps a time suffix because the busy checks read HH:MM back out of it. A string <= then drops every timed row on the window's LAST day. The month calendar over-fetches ±a month so its last day is empty, but the dispatch board asks for a single day, where that day is the only day: an inspection dropped onto a column vanished from the board it had just been dropped on. Both bounds now compare the date part. Both assertions were verified to fail on purpose: with the policy left advisory the board never claims to block, and with the second drop moved off the occupied slot under block policy no modal appears. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
createdAt is millisecond precision and the two creates routinely land in the same millisecond, at which point the service's `|| a.id.localeCompare(b.id)` tiebreak decides — and the ids are random nanoids. The spec failed roughly half the time in a loaded full run and passed every time in isolation, which reads as flake and is really a fixture that never established the difference it asserts on. Ages the Spanish row explicitly. Proven load-bearing: inverting the offset so that row is NEWER fails on all three of three runs, where before the fix the outcome was random. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
The release skill's Dependabot-zero gate runs up front, so these would have stopped the release rather than surfacing during it. Upstream had 9 open alerts, 6 distinct: hono, brace-expansion, ip-address, undici. hono ^4.12.25 -> ^4.12.34 (resolves 4.13.0, a minor bump, so the full type-check and the whole API suite were run against it: clean, 669 files / 4582 tests). brace-expansion and undici were already pinned through overrides and only needed the version raised; ip-address is a new override entry. Note for whoever checks this next: alerts are DISABLED on the fork, so querying important-new/OpenInspection returns a clean zero that means nothing. Query InspectorHub/OpenInspection. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
knip flagged BOARD_END_HOUR and DEFAULT_CARD_MINUTES as new dead exports. Both are read only inside dispatch-helpers.ts, so the export keyword was the dead part, not the constants. Removed rather than baselined — a dead-code allow-list that only grows stops being a ratchet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
UI-16 scans authenticated pages for enterprise-speak and listed
"Dispatch" as jargon for "send". Phase C then shipped a dispatch board —
the multi-column day view Jobber, Housecall Pro and ServiceTitan all call
Dispatch — and the nav item made the scan fail.
Checked every occurrence in messages/en/{nav,calendar}.json before
removing the entry: all of them are that board, none means "send".
Keeping it would have forced a worse word on the feature to satisfy a
rule written before the feature existed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
…st-navigation failure The readiness poll replaces nothing today but is the pattern the rest of the suite should use: /status is a plain JSON handler with no SSR and no assets, so a 200 means the worker serves. A fixed sleep is either too short on a cold machine or wasted on a warm one. The beforeEach navigation now passes `domcontentloaded` explicitly, which every test body below already did — the hook was the odd one out. Neither fixes `contacts @ ipad-portrait`, which fails deterministically when this project runs alone and passes in the full suite at workers: 3. Eight runs; the header comment records what was ruled out by experiment rather than by reasoning, so the next person does not repeat it. Not marked fixme: skipping the first test in the matrix only promotes the next one into the same position. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
CI caught this and local runs could not: the guard resolved REPO_ROOT/../../docs/legal, walking out of the submodule into the private superproject. That path exists when OpenInspection is checked out inside inspectorhub and does not exist when this repo is checked out alone — which is how it is published, and how CI builds it. The failure was the useful part. A guard that only runs from the superproject cannot guard the code that ships, and what it was guarding was wrong anyway: it asserted that counsel's preliminary position, the platform's legal posture, a Civil Code analysis, and a citation to a private document all STAYED in an open-source file. So the guard is inverted. The module keeps the engineering instruction — this states a fact and makes no contractual assertion; translating the agreement body is a different feature and must not grow out of this file; deployments with a language-specific statutory obligation get their own advice. The test now asserts the module cites no path outside this repository and carries no counsel record or jurisdiction analysis, which is checkable from any checkout. Nothing is lost: the removed analysis lives in the superproject's docs/legal/, where it always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ
important-new
added a commit
that referenced
this pull request
Aug 5, 2026
…lding (#297) * chore(deps): pin fast-uri to the patched 3.x A new advisory surfaced after #296 merged — host confusion via a backslash authority introducer, high, runtime scope. fast-uri arrives transitively through @modelcontextprotocol/sdk -> ajv, so it needs an override rather than a direct bump. Pinned `^3.1.5`, not `>=3.1.5`: the open range resolved to 4.1.2, and forcing a major on a transitive that ajv 8.20 expects at 3.x is a way to break schema validation while fixing a URL parser. The MCP suite (85 tests) passes on 3.1.5. Lockfile updated in place; linux entries still 346. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ * fix(gates): drive the dead-code baseline to zero The knip baseline held exactly two entries, and they were the same thing twice: `ERASURE_OUT_OF_SCOPE` and its `ErasureOutOfScopeEntry` type. Neither is dead. `scripts/check-erasure-manifest.mjs` is a plain .mjs gate against a TypeScript manifest, so it reads the declaration out of the SOURCE TEXT (`arrayBody(src, "ERASURE_OUT_OF_SCOPE")`) instead of importing it — a consumption no module-graph analyzer can see. A baseline entry says "this is dead and we tolerate it". Only "a tool consumes this" was true, so it is now said that way: the two exports carry a `@gateConsumed` JSDoc tag, wired through knip `tags: ["-gateConsumed"]`, with the reason written at the declaration. Chose the tag over adding the file to `entry`: `entry` would have exempted the whole file, so a future dead export in the manifest would go unreported. Verified by canary — a throwaway unused export added to that same file still fails the gate (exit 1), and removing it returns exit 0. scripts/knip-baseline.json is now `[]`, and the gate docstring says it must stay that way, with the three legitimate ways to declare a new finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ * fix(gates): replace the server-layer status literals with their enums All 8 server-side entries in the status-literal baseline are gone; the baseline drops 13 -> 5 and the 5 that remain are the app/ render branches, left alone deliberately. They were three different state machines, not one: - REPORT axis (server/api/inspections/publish.ts) — the three transition responses now return REPORT_STATUS.SUBMITTED / .IN_PROGRESS. - INSPECTION axis (server/services/concierge.service.ts) — the confirm-by- client write now sets INSPECTION_STATUS.CONFIRMED. The file already imported the constant and used it four lines earlier. - EVENT axis (server/services/event.service.ts, server/lib/google-calendar.ts) — a visit, not the order: a radon drop-off is `completed` while its inspection is still `confirmed`, and `results_received` has no counterpart on the order at all. These derive from EVENT_STATUS, NOT from INSPECTION_STATUS, even where the word is identical. event.service.ts also carried a hand-written `export type EventStatus` union duplicating the canonical one in lib/status/event-status.ts — deleted, nothing imported it. server/portal/outbox.service.ts was the fourth axis and the interesting one: its `published` means "handed to the queue", while a report's `published` means "delivered to the client". Collapsing them into REPORT_STATUS would have been worse than the literal. The column already declared its enum inline, so that enum is now named — lib/status/sync-outbox-status.ts, the same shape as the three existing axes — the schema derives from it (type-layer only; db:check confirms zero DDL drift) and the service's pending/published/failed literals all reference it. Also fixed one bare literal the gate CANNOT see, found while working: server/services/automation/conditions.ts compared inspection.status against 'cancelled' and 'completed' in a `||` chain. The gate's union-type guard (`/^\s*\|/`, meant for `status: 'a' | 'b'` type declarations) matches the `|` of a `||` too, so every comparison in an or-chain except the last is silently dropped. Reported separately — not fixed here, since tightening the guard surfaces new hits and this commit is about shrinking the baseline. test:unit 669 files / 4581 tests passed (1 file, 1 test skipped). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ * test(e2e): fail loudly when an explicitly requested SEED_E2E=1 seed fails globalSetup wrapped seedFixtures() in a try/catch that only console.warn'd. SEED_E2E=1 means the caller ASKED for the seed and every spec downstream depends on the rows it writes, so swallowing the failure did not make the run robust — it made it lie: the specs then failed at a login or a missing inspection id, which reads as a broken feature rather than a broken fixture. Five separate defects in tests/seed-fixtures.ts survived months behind that warning (wrong database name, missing -c, embedded newlines cmd.exe rejects, a password-hash format verifyPassword can never match, and a tenant id standalone login cannot resolve). The seed now throws, and a seedRequested flag carries the throw past the outer catch — which exists to tolerate a missing local D1 and would otherwise re-swallow it as the same soft warning. The un-requested (SEED_E2E unset) path still warns exactly as before. Verified both directions by running globalSetup directly with a temporary throw planted in seedFixtures: SEED_E2E=1 threw, SEED_E2E unset returned normally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ * test(e2e): delete the RR-migration skip leftovers instead of carrying them Six skipped tests targeted Alpine-era source the React Router migration deleted, so they could never be unskipped — they only inflated the suite's skip count with tests that can never run. - standalone-browser: UI-11 / UI-NOTIFY / UI-WIDGET. Empty bodies (async () => {}) keyed to #agreementsList, #notifyUnreadBadge and [data-widget-embed]. Placeholders, not tests — nothing is lost. - standalone-mobile: M-05, same empty-body shape (Alpine message FAB). - booking-date-input.spec.ts: the whole file was one describe.skip driving the Alpine booking form. File deleted. - sprint2-regression.spec.ts: both describe.skip blocks read src/templates/pages/rating-systems.tsx, src/templates/layouts/ main-layout.tsx and public/js/auth.js off disk — none of which exist. File deleted. Deleting the last two files empties their projects, so the sprint2-regression and booking-date-input entries go from playwright.config.ts as well: a project whose testMatch resolves to nothing is a new way to report green over zero tests. Verified the survivors: browser + mobile projects, 33 passed / 0 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ * test(e2e): correct the subsystem-C blocker — the seed harness is not what is missing The spec said it was "skipped pending the dual-server harness" and pointed at tests/global-setup.ts as the gap. That gap has been closed since the multi-user seed landed: SEED_E2E=1 + tests/seed-fixtures.ts exists and works, and subsystem-D/E now run on it. Anyone reading the old note would go looking for something that is already there. The real blocker is a run environment this repo cannot supply: the portal worker on 8787 AND this worker on 8789 AND `stripe listen` forwarding real signed events, all at once. Playwright's webServer starts one server and globalSetup seeds one D1. Stays skipped, now with a TODO that names what would unblock it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ * test(e2e): unskip subsystem D and E, re-pointed at the surfaces that ship today Nine tests that had been skipped since the Alpine build now run and pass, on a config that gives them a real home instead of an env var nobody sets. SEED (tests/seed-fixtures.ts) - inspector-half@seed.test, exported through SEED_EMAILS. The name says "half" of the INSPECTION, not of a seat quota: the only spec that logs in as it opens seed-half-done-inspection and asserts the publish pre-flight gates fail, and nothing anywhere touches seats. It now owns that inspection. - seed-empty-inspection and seed-republished-inspection are seeded propertyType 'commercial'. This is load-bearing, not cosmetic: the editor renders the units surface only when propertyType === 'commercial' (showUnitsSurface), so without it the unit flows have nothing to drive. - one inspector_credentials row, so a published report has a badge to render. - SEED_INSPECTIONS / SEED_TENANT_SLUG exported so specs stop hardcoding ids. WHAT CHANGED IN THE ASSERTIONS (all: the old contract no longer exists) - D P1+P2: no Alpine [title="Add building"] + window.prompt. Units are the UnitsManager drawer; scope selection is the BreadcrumbDropdown, and only in per-unit mode. "selectedUnitId mirrors the click" was a trailing comment and is now an assertion. - D P7+P9: the republish summary prompt is NOT in the editor's PublishModal — that modal has no summary field and never had one. It is the hub's PublishReportModal (/inspections/:id), shown when the next publish would be an amendment. "Send All" exists nowhere in this repo; the submit is "Publish report". - D P8: the page is /version-diff/:id?n=&from= — /inspections/:id/versions/:n/diff is the API path and was never a page. Headings are "Version N Changes" + Field/Before/After, not "v1 -> v2" / "Items changed". - E P1: there is no publish modal with a disabled "Send All" and a gate checklist; [data-test=publish-send-all] appears nowhere. The five-gate aggregator shipped as GET /:id/preflight with NO frontend consumer, so the test asserts it there. - E P2: neither tab id existed. Live keys are all|active|requested|to_review| awaiting_payment|published|cancelled — no "drafts", and snake_case. The active tab is the DS token class text-ih-primary, not .bg-indigo-600. - E P6: /settings/integrations (no -grid), owner-only (inspector gets AccessDenied), and the six cards are QBO/GCal/Places/Resend/Zapier/Gemini — Stripe Connect is its own panel now, not a card. - E P7: charts are div bars, not svg polyline, and the findings surface is "Findings by Section". The date range has to be stated or the card honestly renders "No data in this date range" and the test asserts a heading over nothing. - E P8: /report-view/:tenant/:id, and it is NOT anonymously readable (the public report API 404s without a recipient token or owner session). "Inspected by" is now "Inspector: {name}"; there is no hardcoded NACHI badge — the credential comes from the inspector's own row. Also: P2-P8 of subsystem E had no login at all and were driving ANONYMOUS pages, because Playwright gives each test a fresh context. Each logs in now, at the role its surface requires. RUN HOME (playwright.seeded.config.ts + npm run test:e2e:seeded + a CI step) The seed writes users into TENANT_A, which IS the standalone workspace, and the default run's api project asserts POST /api/auth/setup returns a fresh 200 — which 409s the moment any user exists. The two are mutually exclusive in one D1, so they are two runs sharing one worker, not two projects. The alternative, skipping these nine unless SEED_E2E is set, was rejected: it puts them straight back in the permanently-skipped column this work exists to empty. Verified: 9 passed. Then falsified — reverting the commercial propertyType and the credential row reds exactly P1+P2 and E-P8 and nothing else; dropping the inspector-half row aborts the whole run through Task 1's new fatal seed path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ * ci: make verify actually depend on e2e `verify` is the job branch protection keys off, and e2e was missing from its needs list. A red E2E run therefore left verify green — the suite ran, reported, and could not block a merge. Nobody decided E2E should be advisory; it just was. Costs verify the wait for e2e, which is the longest job at roughly five minutes. That is the price of the job meaning what its name says. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0125yvQL7kgEym1oR6hiqUAQ --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Batch 2-5 of the remaining programme: 112 commits across scheduling, idempotency, invoicing, agreements, events and the i18n rollout.
Correctness
Request idempotency, end to end. A duplicate-submission defect was observed in production on 2026-08-05: one company created three byte-identical inspections seconds apart. Both layers now exist and, importantly, meet.
idempotency_keys(composite PK(tenant_id, key)) plus a canonical request fingerprint, so a replayed key with a different body is refused rather than silently answered with the first response.jwtAuthMiddleware, not aftercontextBootstrap. This is a security boundary, not a preference: a bare key is a global namespace, and two tenants minting the same key would replay each other's stored response. The ordering is pinned by an assertion intests/unit/platform/middleware-order.spec.tsrather than by a line number.useGuardedSubmiton the client — one key per attempt, held across failure, rotated only on success. A key that never rotates turns a deliberate second action into a silent no-op, which is a worse consistency violation than the duplicate it prevents.Idempotency-Keyheader.fetcher.submit()cannot send headers and this app is a BFF, so the key travels in the body and the route action lifts it onto the header. Both halves had passing tests while the contract between them was dead; a test now crosses the boundary.Standalone login is scoped to the resolved tenant and fails closed on ambiguity, replacing a filter that excluded global agents by role. Matching on
tenant_idis strictly stronger: a NULL tenant can never match, and an ambiguous result refuses rather than picking a row.A calendar range query compared two date formats.
inspections.dateholds eitherYYYY-MM-DDor a datetime, and a string comparison dropped every timed row on the window's last day. The month calendar over-fetches, so it never noticed; the dispatch board asks for a single day, where the last day is the only day — an inspection dropped onto a column vanished from the board it had just been dropped onto.Features
scheduleOtherscapability rather than a role tier, enforced on both the read and the write.PATCH /api/inspections/:id/schedule, conflict policy (advisory/block), Find a Time.Housekeeping
hono,undici,brace-expansion,ip-address.honomoves a minor version, so the full type-check and the whole API suite were run against it.A local-only E2E failure — CI is the authority, and CI is green
Two specs failed on the author's Windows machine and both pass on CI (
e2e, 4m3s, this PR). Recorded because the shape is worth recognising, not because anything here is unresolved.The failures were
workspace-pages-responsive › contacts @ ipad-portraitandpeople-role-profiles › Step 1-2. Same signature both times: the first test of its project, timing out inbeforeEachat the/loginnavigation, while every later test issued the identical navigation and passed. Locally it reproduced across eight runs; slowness (a 90s budget times out the same way),waitUntilmode, worker readiness, retrying, and pre-warming the navigation were each ruled out by experiment.The cause is contention on a constrained local machine at
workers: 3— a hazard this repo has hit before when raising E2E concurrency. On CI the same suite runs clean. The spec header carries the ruled-out list so the next person on a slow machine does not repeat the search.🤖 Generated with Claude Code