Payments, scheduling and retention (#299) * test(quota): pin the D1 semantics the inspection cap depends on Probes the primitive the cap's concurrency guarantee rests on, before any code relies on it: a conditional upsert whose WHERE holds a correlated subquery over another table. Settles three things: - `excluded.tenant_id` IS in scope inside `DO UPDATE ... WHERE`, and resolves to the calling tenant (a sibling tenant's rows do not cap it). No bound-parameter fallback is needed. - `changes === 0` is reported when that subquery is false. - The plain `INSERT ... VALUES` form leaves the INSERT branch UNGATED: a conflict-free insert never reaches DO UPDATE, so a tenant already over the cap but with no counter row yet gets a free pass. The `INSERT ... SELECT ... WHERE` form gates both branches with the same predicate and is what the guard will use. * fix(quota): count inspections that exist, not creates that happened The free-tier cap read a monotonic counter, so a tenant who deleted an inspection never got the allowance back. Found in production 2026-08-05: one tenant had 1 inspection and 4 of 5 consumed; another was capped at 5 with 3 rows. Five counters were corrected by hand that day — this makes that correction unnecessary rather than repeatable. The gate now counts inspection rows. `usage_counters.value` for this metric degrades from source-of-truth to a self-healing display cache. Still one statement, so `meta.changes === 0` stays the authoritative "at cap" answer with no read-then-write window inside the guard. Two things the probe (previous commit) forced: - `INSERT ... SELECT ... WHERE` rather than VALUES, so the INSERT branch is gated too. A conflict-free INSERT never reaches DO UPDATE, so the VALUES form waves through a tenant who has rows but no counter row. - `consumeInspection(tenantId, count)`. Because the gate counts rows the caller inserts only afterwards, N looped calls all read the same count and all pass — a 3-sub inspection request took a tenant from 3 inspections to 6, deterministically. InspectionRequestService.create now consumes the batch as one unit; it is admitted whole or not at all. What counting rows gives up, unavoidably: two creates that overlap before either row lands both pass. The cap becomes a steady-state invariant rather than a serialized claim — the overshoot is bounded by in-flight concurrency and self-corrects, since the next create counts what exists. That is the better trade: the old design bought serialization with a counter that could only climb, which is what cost real tenants their allowance permanently. sms/email are untouched — consumed events with no rows to count. Converting them to a row count would silently uncap them. * test(quota): verify the inspection cap on real D1 under genuine concurrency The unit suite runs the guard against better-sqlite3, which is synchronous — Promise.all there overlaps nothing, so it can only assert the statement's logic. Two things are testable only under workerd: - That D1 accepts the statement at all. `INSERT ... SELECT ... WHERE` with an `ON CONFLICT ... DO UPDATE ... WHERE` whose predicate is a correlated subquery over another table, plus `excluded.` inside that predicate, is not a shape D1 code usually reaches for, and `meta.changes === 0` on the no-rows-selected path is the signal the whole gate reads. Confirmed. - What concurrent callers actually observe. At the cap, six genuinely overlapping consumes ALL fail — no phantom pass. Below the cap, two that overlap before either row lands both pass, which pins the bound recorded in guard.ts as real-engine behaviour rather than a test-driver artefact. * docs(quota)+fix(usage): the counter is a cache, and /api/usage says so Retires the 2026-08-05 manual production correction rather than leaving it to be repeated. guard.ts header now records that `usage_counters.value` for `inspections` is a cache and not the gate, so the next person who finds a value higher than the row count does not "fix" it. Deleting the row is worse than leaving it, and the header says why. sms/email are called out as the opposite case: consumed events whose counters ARE the source of truth. /api/usage decision (plan Task 4 Step 2), taken rather than left open: a tenant whose inspections are capped now gets the live row count — the number shown against a cap has to be the number the cap is enforced against, or a tenant who deletes three inspections is told "5 of 5 used" while creating works, which is the visible half of the defect. Uncapped tenants keep the cumulative lifetime counter: with `caps: null` it is measured against nothing, and silently redefining it from "ever created" to "currently have" would change an analytics figure nobody asked to change. * fix(usage): route the inspection count through getDrizzle lint:provider-helpers hard-fails on `drizzle(c.env.DB)` in an API route; route handlers go through the helper. * fix(qbo): book payments on the date the money moved, in the tenant zone recordPayment stamped TxnDate with the push date (UTC today), discarding the ledger row's occurred_at — an inspector recording Tuesday's cash on Thursday booked it into the wrong accounting period. TxnDate is a bare calendar date, so it is now derived from occurred_at in the TENANT's timezone (epochMsToWallClockYmd + resolveTenantTimeZone): a 6pm Pacific payment is the next day in UTC, a real one-day period error at month end. All three push sites (mark-paid, offline recording, Stripe webhook) pass the appended ledger row's occurredAt. The check-tz-safety header no longer names QBO payment TxnDate as a legitimate UTC-today use. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * fix(services): refuse to remove a service line that a report delivers removeInspectionService soft-deleted unconditionally. The deferral written at the function came due when the reports table landed: with no FKs by design, nothing else surfaces a report left pointing at a line that is no longer on the invoice. Removal now returns 409 Conflict naming the blocking report (in_progress and published both block — both are work the line paid for); reports on OTHER lines do not block. The pay-split clause stays deferred to the pay-splits plan as its own explicit step, since inspection_service_pay_splits still does not exist. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * feat(ai): expose GET /api/integration/ai-provisioning for portal's tier console Per-tier tenant counts bucketed managed/byo/unconfigured by the SAME resolveAi call the runtime meters with — resolveRuntimeAiSource is extracted from buildAiMeter so the still-false entitlement literal lives in exactly one place and the console can never disagree with the resolver. Wire contract pinned by portal's narrowAiProvisioning: a tier with no tenants is absent, never zeroed; all three counts finite. The /usage handler moves verbatim to server/portal/usage-report.ts — the file-size ratchet asked for an extracted unit, not a baseline bump. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * gate(idempotency): ledger every mutating route's retry safety A mutating route with no idempotency story looks identical to a guarded one at the call site — the duplicate row only appears when a customer's network retries a POST. This freezes today's surface as a burn-down ledger and fails on any new mutating route that arrives without one. Ported from portal's equivalent gate rather than written fresh, keeping its four-category ledger (verified / pending / uncoveredByDesign / knownUnreachable), its fail-closed behaviour when zero routes are discovered, and its stale-entry check — a ratchet that only ever grows lies about progress. Discovery is rewritten for OI's layout: the walk starts at server/index.ts's .route() mounts and follows sub-routers, alias chains, ident statements and router-taking helpers recursively. Evidence files diverge from portal's: only `*-replay.spec.ts` under tests/unit/idempotency/ and `*-idempotency.test.ts` under app/ count. The mechanism's own unit specs live in that directory and quote real route paths as sample input — scanning them would mark routes verified on the strength of a hash test that never calls them. Seen red both ways before wiring: a throwaway `testHooks.post('/x-throwaway')` was named and exited 1, and a hand-added `POST /api/gone-away` pending entry failed as stale. 302 of 312 declared mutating routes resolve; 287 pending, 14 by design, 1 verified. * chore(gates): wire lint:idempotency into both lint chains Config-only, in its own commit: package.json in a diff escalates the pre-commit type-check to the full tier, so it does not ride along with code. Registered in `lint` AND `lint:gates-full` — the two chains are hand-duplicated, and a gate added to one drifts silently out of the other. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * feat(ai): receive delivered AI allowances, without a migration Portal's tier console owns the numbers and records who set them; core needed somewhere to put them and a command case to receive them. Storage is the existing tenant_configs.integration_config blob under a reserved key, so this lands with no schema change. Two properties of that column make sharing it safe, and both are load-bearing, so both are asserted rather than described: the tenant-facing writer MERGES over the stored object (a Settings save cannot drop a key it does not know about), and the tenant-facing route validates against a closed Zod object that STRIPS unknown keys (a tenant cannot write their own allowance). No cap constant appears anywhere. An absent key, an absent tier, an absent metric and an unparseable blob all mean the same thing — unconfigured, so unenforced. Caps resolve through a loader rather than at guard construction, so the per-request site pays no read and no tenant's caps can bind to another tenant's check. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * feat(ai): resolve delivered AI caps per check, at all seven guard sites The guard could already read an allowance; nothing gave it one. All seven production construction sites now pass tenantAiCapsLoader, and checkAiQuota resolves it at the moment of the check. A loader rather than a value, because two of those seven sites make an eager read wrong rather than merely wasteful: the DI middleware constructs a guard on every authenticated request, and the cron tick reuses one guard across every tenant — where a single resolved value would bind the first tenant's caps to everybody else's check. Tests keep passing an object, which is what makes a configured-cap control cheap to write. managedEntitled is still a literal false, so no cap is reachable in production yet; this is the path being ready, not switched on. No cap constant is introduced anywhere: an absent loader, an absent tier and an absent metric all mean unenforced, and FREE_TIER_CAPS is asserted to carry no AI entry so that "no managed allowance" cannot silently inherit one. Seen RED at "promise resolved undefined instead of rejecting" with the resolution removed — while the fourteen object-shaped cases stayed green, which is precisely why the loader case had to be added. Baseline +1 on sms.ts and di.ts: one import line each into files already grandfathered at 844 and 433. Splitting either is a refactor this change does not justify. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * test(idempotency): verify POST /api/invoices replay writes one row Tier 1 of the burn-down starts where a duplicate costs money. Creating an invoice twice does not just duplicate a record — it doubles what the tenant believes is owed, and pushes the duplicate into their QuickBooks where a human has to unpick it. The route is tenant-authenticated, so the global mount already sits in front of it. What the mount does not tell you is whether the endpoint's side effects all happen INSIDE the guard's span: the QBO upsert is scheduled through executionCtx.waitUntil from the handler, and anything scheduled outside the guarded window would repeat on a replay while the invoice row correctly did not. So the spec drives the real router behind the real middleware over a real in-memory D1 and counts rows and provider calls. Seen RED with the guard removed from the harness, on all five containment cases — "expected [ {...}, {...} ] to have a length of 1 but got 2", "expected vi.fn() to be called 1 times, but got 2 times", and "expected 201 to be 422" for the changed-payload case. Baseline: 287 -> 286 pending, 2 verified by replay spec. * test(idempotency): verify payment-capture replay does not book cash twice Covers POST /api/invoices/{id}/payments and POST /api/invoices/{id}/mark-paid. Both fan out past the invoice row — the report's payment gate is opened and the movement is pushed to QuickBooks — so the 201 is not the thing to assert on. The offline ledger is append-only BY DESIGN (a correction is a new row, never an edit), so a retried POST conflicts with nothing and looks like an error from no angle. It just books the same cash twice. Two findings the red run forced into the spec: - Written around a FULL payment the duplicate is invisible: the overpayment refusal fires first, so the second row never lands and the guard cannot be seen to do anything. The duplicate proof therefore uses a partial deposit, which nothing refuses. The settling case is kept as its own test for what the guard actually changes there — the operator gets the original receipt back instead of a balance error about a cheque they have already banked. - mark-paid is append-once on its own (markPaid returns nothing when the ledger already covers the balance, and the QBO push is conditional on that return), so those two assertions pass unguarded. They are kept, labelled CHARACTERIZATION, so nobody later reads them as evidence. The route still needed the guard: the payment-gate call is unconditional. Six of eight cases seen RED with the guard removed from the harness — "expected [ {...}, {...} ] to have a length of 1 but got 2", "expected vi.fn() to be called 1 times, but got 2 times" (QBO push, and the payment gate on mark-paid), "expected 422 to be 201" (settling retry refused instead of replayed), and "expected 201 to be 422" (changed payload under one key). The two survivors are the fresh-key control and the characterization test. Baseline: 286 -> 284 pending, 4 verified by replay spec. * test(idempotency): verify agreement send does not re-mail on replay POST /api/admin/agreements/send. An email that has left cannot be recalled, and this one carries a per-signer signing link — a retried send puts a second "please sign" in the client's inbox with a link that also works, and nobody can tell afterwards which one they used. The envelope is NOT the exposure: AgreementService.findOrCreate is find-or-create, so an unguarded duplicate returns the same requestId and leaves one envelope. That is kept as a labelled CHARACTERIZATION test so the requestId assertion is never mistaken for the containment proof. What actually repeats is the tail — one outbound email per signer, and the request.sent entry in the tamper-evident audit chain, which is supposed to be the record of how many times the envelope was mailed. The changed-payload case turned out to be worse than a wasted email: unguarded, a retry carrying a different signer list is MERGED into the live envelope ("findOrCreate merged signers", added: 1), adding a party to an agreement already out for signature. The spec now asserts the signer rows, not just that no mail went out. Seen RED with the guard removed: "expected vi.fn() to be called 2 times, but got 4 times", "expected [ [...], [...] ] to have a length of 1 but got 2" (audit chain), "expected null to be 'true'" (no replay flag), and "expected 200 to be 422" (changed signer list accepted under the used key). Baseline: 284 -> 283 pending, 5 verified by replay spec. * test(idempotency): verify manual SMS send does not text twice on replay POST /api/inspections/{id}/send-sms. The ledger's claim was checked before writing anything, and it holds: Task 4's title said "email and SMS" but its Files list only ever named the email builder, so there is NO service-level dedupe on the SMS path — nothing resembling buildEmailDedupe exists under server/lib/sms/ or in send-one-sms.ts. Reading the handler end to end confirms it: every call mints a fresh automation_logs id, inserts a pending row, and hands it to sendOneSms without consulting whether this message already went out. The claim needs one refinement, because the two halves are easy to conflate. The HTTP route IS tenant-authenticated, so the global mount does span it — the guard contains a retried REQUEST. What the missing service-level dedupe means is that anything reaching sendOneSms by another path (the automation flush, an Outbox resend) is still unprotected. This commit closes the first, not the second. A duplicate here costs twice: the carrier charges per segment and the send meters against the tenant's quota. Unlike email it also lands on a phone. Seen RED with the guard removed: "expected vi.fn() to be called 1 times, but got 2 times" (the Twilio seam), "expected [ {...}, {...} ] to have a length of 1 but got 2" (the SMS ledger the Outbox and metering read), "expected null to be 'true'", and "expected 200 to be 422" for a changed recipient under a used key. The survivor is the fresh-key control. Baseline: 283 -> 282 pending, 6 verified by replay spec. * test(idempotency): verify workspace agreement send does not re-mail on replay POST /api/inspections/{id}/agreement-requests — the sibling of the admin send, and the reason it gets its own commit rather than a footnote: the inspection workspace's "Send agreement" button posts HERE, not to /api/admin, and the two share emailSignersTheirLinks precisely because they had already drifted into mailing different links once (IA-65). A shared helper is not shared containment. Verifying one route and calling the behaviour covered would have left the button most inspectors actually press outside the ledger, with the gate reporting progress for it. Seen RED with the guard removed: "expected [ 'jane@example.com', ...(1) ] to deeply equal [ 'jane@example.com' ]", "expected null to be 'true'", and "expected 200 to be 422" — the last one again logging "findOrCreate merged signers, added: 1", i.e. a mis-keyed retry adding a party to an agreement already out for signature. Baseline: 282 -> 281 pending, 7 verified by replay spec. * test(idempotency): verify request-payment replay does not re-bill the client POST /api/invoices/request-payment — the hub's "Request payment" button. The endpoint resolves or CREATES the inspection's invoice, marks it sent, mints the recipient's portal token, and emails a working link to the public payment page. Two of those are self-limiting and are asserted as CHARACTERIZATION rather than as evidence: the invoice is reused when one exists, and issueToken is deliberately idempotent so older copies of the email keep working. The email is not self-limiting — a retry sends the client a second "please pay for your inspection", the message in this system most likely to be read as a second bill. The red run turned up one effect that was not obvious from reading the handler: unguarded, the replay reruns markSent and the returned sentAt MOVES. A retry nobody made rewrites the invoice's own record of when it was sent, so that assertion is now explicit rather than incidental to a deep-equal. Seen RED with the guard removed: "expected vi.fn() to be called 1 times, but got 2 times", the sentAt drift above ("2026-08-06T00:52:52.722Z" vs "...52.753Z"), and "expected 200 to be 422" — that last one meaning a key reused against a DIFFERENT inspection raised an invoice on the wrong job. Baseline: 281 -> 280 pending, 8 verified by replay spec. * test(idempotency): order the in-flight overlap instead of racing for it The concurrency case fired its second request immediately after the first and assumed the first had already claimed the key. It usually had. Under load it sometimes had not — and then BOTH requests claim, both enter the handler, and both park on a gate that is only released after the second returns. The deadlock surfaces as a 5s timeout, which reads like a slow test rather than the ordering bug it is. Observed for real: adding six replay specs to this directory raised the parallel load enough to trip it ("Test timed out in 5000ms" on tests/unit/idempotency/middleware.spec.ts:113), while the file on its own passed every time. Found by running the directory, not the file. The fix waits for the handler to be ENTERED, which is strictly after the claim, so the overlap is ordered rather than hoped for. It does not weaken the test: `ran` becoming 2 still fails, and a missing in-flight branch still parks the second request and times out. * feat(pay-splits): pay rules and split records (#278) Splits are RECORDS, not a calculation: a tenant rule populates a row once, then only a deliberate edit moves it. A derived split would rewrite what someone was already paid whenever the rule changed. Grain is the billing line (`inspection_services.id`), the same key `reports.inspection_service_id` uses, so "what did this line earn and what did it produce" stays joinable. Two partial unique indexes rather than the obvious plain ones: - `service_pay_rules` — SQLite treats NULLs as distinct, so a single unique over (tenant, service, user) silently accepts TWO default rules for one service and the populate step would then pick one arbitrarily. - `inspection_service_pay_splits` — one PRIMARY split per (line, user), but correction rows against a locked split must be insertable, and an unconditional unique makes that path impossible to write. Both tables are payroll records about staff, so they are declared in ERASURE_OUT_OF_SCOPE with a reason: a client's erasure request does not reach them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * feat(pay-splits): populate splits from rules, once (#278) Rules populate a split row; after that only an explicit edit moves it. `populateSplits` is additive — an existing (line, user) pair is skipped whatever the rules now say — and `refreshSplits` is the single, explicit path that re-derives, behind `previewRefresh` so nobody's pay moves without someone deciding it should. Three rules that are easy to get wrong and are pinned by tests: - The computed amount is DIVIDED by the number of inspectors eligible for that line. Without it, attaching a second inspector pays out 120% of the service. - Reads filter `inspection_services.is_active`. A line declined at the door survives because a report or a split may point at it; paying against it anyway is what that column exists to prevent. A split whose line is deactivated later surfaces as an orphan, the same treatment a removed inspector's split gets. - `percent_after_deduction` takes the deduction off the top BEFORE the percentage, so it is not restatable as a smaller percentage. The roster is read through `getInspectionRoster` only, and assignment writes now go through `syncAssignmentsAndSplits` rather than `syncInspectionAssignments` directly — the roster write and the pay reconciliation move together so a new assignment path cannot record who worked the job while leaving the money on whoever worked it before. The reconciliation is deliberately quiet: a misconfigured pay rule must not make saving an assignment fail. Once payroll exports a split it locks, and a later adjustment is a new row carrying the delta. An in-place edit after money has moved desynchronises the books from what was actually paid with nothing surfacing the divergence. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * fix(pay-splits): stop exporting a helper nothing outside the module calls lint:deadcode exits 1 on it, and knip-baseline.json is empty on purpose — an export with no consumer is a failure, not an allow-list entry. Its only caller is refreshSplits in the same file; Task 3 gives it a route and can export it then. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * feat(pay-splits): the API surface, and visibility as query scoping (#278) An inspector may see their own pay and nobody else's. That is a THIRD state — `financial: false` AND `subject = self` — and no boolean permission expresses it, which is exactly why Housecall Pro and Jobber do not offer it: their pay permissions are binary. So it is implemented as query scoping, not a capability and not a redactor exemption. financial: true -> every split on the inspection, editable financial: false -> only rows where user_id = the caller, read-only `server/lib/auth/money-redaction.ts` is deliberately untouched. That redactor stops an endpoint leaking the COMPANY's money; an endpoint that structurally returns only the caller's own row is not leaking, so there is nothing to exempt, and the single shared security-sensitive file stays as it was. No `payroll` capability was added. The line is the existing `financial`. Mounted under the inspections aggregator rather than server/index.ts, which sits at 703 of a baselined 704 — a top-level mount costs two lines and hard- fails lint:filesize. The payroll export is company-level, so it went to the team router (staff administration) instead of inventing a home for it. The route definitions are NAMED CONSTS, and that is not style. `check-idempotency-coverage.mjs` discovers routes by resolving `.openapi(IDENT)` to a `const X = createRoute(...)`; a route written inline as `.openapi(createRoute(...), handler)` is INVISIBLE to it and is never asked for a retry story. Written inline first, all four money routes passed the gate by not existing to it. That is also why `POST /api/inspections/{id}/services` (inline, shipped earlier) is absent from the baseline while its named-const siblings are there — a pre-existing hole, reported not fixed here. All four mutating routes are VERIFIED by a replay spec, not parked: - POST .../corrections writes a NEW row carrying a delta, so an unguarded retry pays the delta twice and the ledger stays internally consistent while doing it. - POST /api/team/payroll-export LOCKS what it returns, so a replayed handler hands the operator an EMPTY run and the money reads as unowed. - PATCH and refresh are contained on their own; asserted as characterization and labelled, so nobody reads them as evidence for the guard. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * fix(qbo): require owner or manager on the QuickBooks router Authentication is not authorization, and this router had only the first. It verified the JWT and ran the handler — no role, no capability — so any signed-in inspector could call POST /disconnect, which revokes the Intuit refresh token and deletes the tenant's whole qbo_entity_map. That table is the OI-invoice to QBO-invoice correspondence; reconnecting does not restore it, so the next push writes duplicate invoices against the same DocNumbers. Listing a Stripe webhook log, in the router next door, already required owner or manager. Neither authorization gate could have caught this. check-capability-declarations scans for createRoute(withMcpMetadata( windows and this file is a hand-rolled Hono router with none, so it passed vacuously; the authorization-surface spec is the same inversion against the live registry. Both ask whether declaration and enforcement agree — a route that declares nothing and mounts nothing looks correct to both. So the assertion is an explicit HTTP-level spec instead. Seen RED at "expected 200 to be 403": an inspector was getting 200 on disconnect. The owner/manager control passed before the fix too, which is why it is there — without it the four refusal cases would also pass against a router that refuses everyone. Applied router-wide rather than per route, since /status exposes the connected realm, company name and sync errors, and a uniform surface gives a future edit no per-route reasoning to get wrong. It also refuses a caller with no role at all, which is what an agent JWT is: it satisfies the verifier and carries no tenant by design. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * feat(metrics): pay, attributed revenue, and a turnaround that says its basis (#278) Two thirds of this already shipped, so this is a rewrite of `byInspector` rather than an introduction. Every field of the old row is gone, and each for its own reason: count -> ledCount + assistedCount revenue -> payCents + attributedRevenueCents avgTurnaroundDays -> medianTurnaroundDays + turnaroundBasis The lead-only grouping the old query used was a deliberate anti-double-count — its own comment said so. Widening it to the whole roster is right for COUNTS (a helper was there) and wrong for the company's revenue, so the count widens and that figure does not follow. What follows instead is `attributedRevenueCents`, which is an attribution and is NOT additive down the column: two inspectors on one job are each credited with the work they were on. It is eligible-line scoped by the SAME rule pay splits use, so the two columns sitting side by side are comparable. Pay is not attributed revenue. Housecall Pro names the worker's money `Commission Cost`, which is safe there because only an owner opens that report. Our inspector opens this page, so it is Pay, and there is no column called "revenue" holding both. Median, not mean: one delayed report on a complex property drags a mean and misrepresents the person it is attached to. Turnaround anchors, both corrected: - END is `reports.published_at` — per deliverable, nullable — NOT `report_versions.published_at`, which is NOT NULL and fires per version AND per amendment. The old query also joined report_versions on version_number = 1 scoped by inspection_id alone, so an order delivering several reports scored an arbitrary one. - START is MAX(inspection_events.completed_at), which has no frontend writer yet. So the metric reports `turnaroundBasis: 'no_data'` and renders "no field-completion times have been recorded" rather than substituting a booking-confirmation clock, which measures how fast the office confirms bookings under this metric's name. Referrals: `referred_by_contact_id is not null` was silently dropping every row whose only attribution is free text — and for a one-person firm those are usually the only rows there are. They come back as a second bucket tagged `kind: 'source'`, listed after the contact-keyed rows and never merged into them; they are different kinds of answer. The route widens to `inspector` with scoping instead of a capability, matching the pay-split surface: without `financial` the caller gets their own row and the company figures as NULL, not zero. Zero is a claim about the business. Verified in Chrome, light and dark: the By Inspector table renders Led / Assisted / Pay / Attributed revenue / Median turnaround with the basis line under the title, legible in both themes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * fix(qbo): make the OAuth pair reachable, and authorize the callback by state Two independent defects, either of which alone kept the QuickBooks connect flow from ever running. Neither is visible from the settings page, which is why the integration has never completed a handshake. First, unreachable. workers/app.ts forwards an explicit prefix allow-list to the Hono app; /settings/** is not on it, so the connect and callback routes went to React Router, which has no such route. The webhook next door lives under /api/* and works — that asymmetry was the tell. /connect and /callback move to /api/integrations/qbo, beside the webhook. The siblings (/status, /pause, /sync, /errors/:id/retry, /contacts/:contactId/link) stay where they are: the settings page reaches those through the in-process API_WORKER binding, and they work today. Second, the session cookie cannot arrive. __Host-inspector_token is SameSite=Strict, and Intuit returns the user by a cross-site top-level navigation — the exact case Strict withholds a cookie on, as the portal cookie right below it in auth-helpers.ts already documents. Making the route reachable alone would have turned a 404 into a 401. So /callback is unauthenticated and authorized by state instead: the value stored under qbo_oauth_state:${state} is now the tenantId rather than the placeholder, and the callback resolves the tenant from it. The state is a server-generated UUID, single-use, 600s TTL, and only ever issued to a caller who has just passed the owner/manager guard on /connect — which /connect keeps. Declared public in jwt-auth.ts alongside the QBO webhook, for the same reason: neither caller can hold a session. Also found while tracing, not in the brief: the client id and secret can be a per-tenant secret, and integrationSecretsMiddleware only merges those once a tenant is known — which on this request it is not, in saas mode. The callback loads them for the tenant its state names, using the same helper and the same precedence rule. redirectUri was built from the same literal in two places. Both now call qboRedirectUri(), and the mount plus the jwt-auth entry come from the same constants, so the path cannot move without the URI moving with it. Intuit compares that string byte-for-byte against the registered value. The load-bearing test sends what Intuit sends — a bare GET with no Cookie header — and asserts it completes against the right tenant. Proven red first: "expected 401 to be 302". Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * feat(qbo): choose the Intuit API host by QBO_ENV, with no default The accounting host was compiled in — quickbooks.api.intuit.com, in services/qbo/api-base.ts and again in the OAuth callback's companyinfo lookup. Sandbox lives at sandbox-quickbooks.api.intuit.com, and the two are not reachable with the same credentials: Intuit Development keys authenticate only against sandbox companies, Production keys only against real ones. A build that can only address production therefore cannot be exercised against a sandbox at all, which is a large part of why this integration has never run end to end. QBO_ENV names the host. It fails closed — unset or unrecognised raises before any request leaves the worker, and the OAuth callback refuses to store a connection rather than saving a working token against an API the worker will then decline to call. No fallback constant, because either default is wrong half the time and both failure modes read as credential problems: pointed at production with Development keys you get an auth error, and pointed at sandbox with Production keys a paying customer's books quietly receive nothing. Resolved lazily off the constructor rather than eagerly, since the service is built for every request that touches an invoice and a deployment with no QuickBooks connection should not fail on a setting it never uses. The token and revoke endpoints are shared by both environments and are unchanged. Asserted through apiCall — what matters is the URL that actually leaves the worker, not a helper in isolation. Proven red first: "expected 'https://quickbooks.api.intuit.com/...' to be 'https://sandbox-quickbooks.api.intuit.com/...'", and the two fail-closed cases resolved instead of rejecting. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * fix(qbo): stop disconnecting a customer over a transient refresh failure refreshToken treated any non-2xx from Intuit's token endpoint as "reauthorize required" and DELETEd the connection row. Intuit rotates the refresh token every 24-26 hours, so that path runs on every connected tenant every day, forever — which means one Intuit 5xx, or one rate limit, permanently severed a paying customer's integration and sent an owner back through the whole OAuth flow. Their books stop syncing in the meantime, with nothing recorded that says why. Only an explicit refusal of the grant is terminal now. 400 (invalid_grant) and 401 are the answers that actually say the token is dead; 429, 5xx and network errors say nothing about its validity, so the row is left intact and the next attempt uses the same token. The two cases also raise distinct messages, so a log line no longer has to be guessed at. Intuit has begun returning a field on this response that dates the refresh token's hard expiry. It is deliberately not read: the exact key could not be verified from the published documentation, and keying a destructive delete off a guessed field name would reintroduce exactly the failure this removes. The status code already separates the two cases; the field would only allow disconnecting EARLIER, which is not the useful direction. Proven red first: at 500 and at 429, "expected undefined to be truthy" — the connection row was gone. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * fix(tests): let the demand-signal doc parser survive a Windows checkout The spec reads its SQL out of docs/developers/multilingual-demand-signal.md by matching a fence followed immediately by \n. A Windows checkout has CRLF there, so the pattern matched nothing, every block was silently missed, and the failure surfaced as "doc has no -- Query D block" — pointing at the document rather than at the parser that had read none of it. CI is Linux, so this passed there while being unrunnable on the machine the document is edited on. Also two glossary violations in the new metrics translations, which only the full lint chain can see. "tus" is ruled out for second-person possessive. And English "Pay" now has a declared divergence: it is the verb on the client checkout (Pagar) and a column header naming what an inspector earned (Pago). Pagar as a header reads as an instruction to pay someone. The header is deliberately "Pay" rather than "Cost" because the inspector reads this column, and calling their earnings a cost states the company's view of them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014Bc5q8eunHDbkqDHtYvEkv * fix(gate): the idempotency gate could not see inline route registrations `scripts/check-idempotency-coverage.mjs` resolved registrations with `.openapi(\s*(\w+))` and looked the captured name up among `const X = createRoute(...)` declarations. For the INLINE form — `.openapi(createRoute({ method, path, ... }))` — that captured the literal word `createRoute`, found no declaration, and dropped the route in silence. The blindness was measurable the whole time and the gate printed it every run: the baseline recorded `declaredMutating: 316` against `resolvedMutating: 306`, and that difference of 10 was the exact number of mutating routes the parser could not place. Two numbers that should be equal sat side by side and the difference was never read as a defect. Four money-moving pay-split routes were written inline and the gate reported the same resolved count as before they existed. Three changes: - The registration pass tells a NAMED const from a CALL by the trailing `(`, not by the callee name, and reads method+path out of an inline route object exactly as it does from a named const's body. Matching on the name `createRoute` would go blind again the day someone wraps it. - A router chain now runs to the next TOP-LEVEL STATEMENT, not to the next column-zero line. Several modules close a registration with a column-zero `}, { scopes: [...] })), async (c) => {`, and the old rule cut those chains off after their first route — five routes in marketplace.ts, two in inspection-sync.ts. `const X = withMcpMetadata(createRoute(...))` is also recognised now (the inspection-prefs.ts PATCH was invisible for it). - THE STRUCTURAL FIX: `declaredMutating` and `resolvedMutating` are now keyed by declaration site (`:`) rather than being two incomparable tallies, they must be EQUAL, and the gate FAILS when they are not — naming each unresolved site with its source line. A route form the parser does not understand now turns the gate red instead of quietly shrinking coverage. The declared side stays a raw, parser-independent scan; the resolved count had to rise to meet it, not the other way. 316 declared / 306 resolved -> 316 / 316, gap 0. 333 distinct mutating paths, up from 306. 27 routes became visible; the three money-moving `/api/inspections/{id}/services` verbs are VERIFIED by a new replay spec (the DELETE has a real hazard: unguarded, the retry turns a successful removal into a 404 the operator reads as "the line is still there"), `POST /api/services/discount/validate` is by-design (it writes nothing — no uses_count increment, no reservation), and the remaining 23 enter the pending ratchet. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * feat(pay-splits): give service pay rules a write face (#278) Tasks 1-4 shipped the schema, populateSplits, the split API and the metrics, but nothing could create a `service_pay_rules` row — no route, no service writer, no UI — so populateSplits read an empty rule set and produced nothing. The feature was a machine with no switch. This is the switch, server side. THE UNIT CONTRACT. `service_pay_rules.value` is basis points for a percentage and integer cents for `fixed`: one column, two units, decided by a sibling field. That is defensible in the schema and indefensible on the wire, where 60 meaning 0.6% when the caller meant 60% is a hundredfold money error no type catches. So `value` never appears on the wire, in either direction. Each variant names its own unit — percentBps, amountCents — and the request objects are STRICT, so a payload written against the column is a 400 naming the unexpected key rather than a row stored a hundred times too small. Converting a human percent server-side was the alternative and is worse: it puts two representations of the same number in the system and moves the off-by-100 from the wire, where a schema catches it, into arithmetic, where nothing does. A discriminated union on `type`, not one loose object, because deductionCents is meaningful only for percent_after_deduction — the deduction comes off the top BEFORE the percentage, which is why that type exists at all. A `percent` rule carrying one is ambiguous and is refused; a loose object would have dropped it in silence, and the spec proving that is red without `.strict()`. The two partial unique indexes surface as a 409 with an actionable message. Unguarded the same request is a 500 reading `UNIQUE constraint failed: service_pay_rules.tenant_id, …`, which is what the spec sees when the pre-check and its race backstop are removed. percentBps is capped at 10000. That is not the split-ceiling check the populate path already owns: above 100% the gross exceeds the line price for EVERY roster size, because the per-inspector divisor cancels, so no such rule could ever pay out. A 100% rule with two inspectors pays 50% each and stays legal. Routes sit at /api/services/{id}/pay-rules, mirroring the /{id}/inspectors pair — same arity, same param name, registered before the bare /{id} routes. Writes are owner/manager like every other write on this router: deciding what a person is paid is company configuration. No new capability; `financial` stays the only line. All three mutating routes land VERIFIED in the idempotency gate rather than pending. The hazard is not a duplicate row (the unique index stops that) but what the retry SEES: unguarded, the second POST is a 409 indistinguishable from a colleague having added a rule in another tab, and the retried DELETE 404s on the caller's own success. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * fix(pay-splits): the pay-rule union blew tsc's heap — same contract, one object The previous commit shipped the pay-rule body as a `z.discriminatedUnion` on `type`, which is the shape the rule genuinely has. It also broke `npm run type-check:app`, and not with an error: tsc runs for five minutes and dies with "Ineffective mark-compacts near heap limit — JavaScript heap out of memory" at its 8 GB cap. Bisected rather than guessed. Same tree with these three server files reverted to their previous versions and pay-rules.ts moved aside, type-check:app exits 0; put back, OOM. The cause is hono/client, which expands every route's request type into the app-wide RPC type that `createApi` returns — three strict union members plus the three `.omit()` members of the update union is enough to take the whole program past the limit. The previous commit's own pre-commit hook could not see this: staging only `server/**` selects the api tier, and the api tsconfig does not build the client type. That is a real hole in the ladder for any route-shape change, not a one-off. The fix keeps the contract and changes only its encoding. One strict object carrying every rate field as optional, plus `exactlyTheFieldsFor` — a table of which fields each type REQUIRES and, just as load-bearing, which it FORBIDS, applied as a superRefine. Identical rejections: `percent` with a `deductionCents` is still a 400, an unknown key is still a 400, a missing `percentBps` is still a 400. The proof nothing moved is that not one line of either spec changed and all 24 still pass — the wire shape, the field names and the statuses are byte-identical, so no caller can tell the two encodings apart. `toColumns` gains a `need()` guard where the union used to narrow for it. Deliberately a throw and not a `!`: the refinement that guarantees the field lives in another file, and the failure being guarded is the money column silently taking undefined. The path back is written next to the schema. If hono's RPC inference ever gets cheaper, the union is the better model and that is where it goes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * feat(pay-splits): a tenant can now switch pay splits on from Settings (#278) The server half landed the write face; this is the surface a person actually uses. Without it the feature stays off for every tenant that does not write curl by hand, which is every tenant. The widget sits on the service row directly under the qualification line, and is deliberately that widget's twin — same disclosure, same inline panel, same tokens. The two answer adjacent questions about one service ("who MAY run this" / "what they EARN running it"), and a second visual idiom for the second question would read as a different kind of setting. Reusing the pattern reuses its answers too: read-only when there are no members to choose from, save and cancel in the same place. The unit boundary is one function. `toHundredths` is the only ×100 in the path and it is called beside the "%" and "$" a person can see; the API is already strict about receiving basis points and cents under names that say so, so a second conversion site would be the thing that pays someone 0.6%. It rounds rather than floors: 8.2 × 100 is 819.9999999999999 in IEEE-754, and flooring pays 8.19% forever while the number on screen still reads 8.2. That assertion goes red if anyone simplifies it — checked by deleting the fix, not by reading it. Two 409s are prevented rather than reported. The "Applies to" picker drops inspectors who already have a rule and drops the default option once the default exists — the DB refuses those writes, and offering a choice that is always refused is worse than not offering it. The API's 409 is still surfaced verbatim when a race gets through, because that message is written for this screen. The divisor is stated in the panel. A 60% rule with two inspectors pays 30% each, which is correct and is not what someone typing 60 expects; it halves people's pay when it surprises them, so it is said where the number is typed rather than discovered in payroll. `failureMessage` takes `{ json(): Promise }` rather than `Response`: hono/client returns a `ClientResponse<…, 409, "json">` whose body type differs per status, and widening to `Response` at the call site would discard the typing that makes the client worth having. Copy in en + es-419. The glossary gate caught `Sin configurar` for "Not set", which is already `Sin definir` on four other keys — aligned. The noun sense of "Pay" is `Pago` per the declared divergence. Chrome, light and dark: create, edit, switch type (the deduction field and its hint appear only on percent_after_deduction), remove, and the collapsed summary flipping between "Not set" and "1 pay rule". Re-walked after the schema was re-encoded — 62.5 typed in the box comes back as 62.5, which is the readable proof it was stored as 6250 basis points. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * refactor(invoices): the payment ledger becomes its own sub-router `server/api/invoices.ts` sat at 534 lines against a 535 cap, so the next change to it — the partial-refund writer #19 Task 3 needs — could not add even an import line. This is a pure move: no route path, operationId, schema, status code or handler body changes, and the OpenAPI snapshot is byte-identical (tests/unit/mcp all green without regeneration). The seam is the sub-resource, not the line count. Everything under `/{id}/payments` is one thing: append-only rows describing money that moved, gated on the `financial` capability rather than role alone, with its own correction mechanism (a reversing row, never an edit) and its own rule about what reaches QuickBooks. What stays behind is the invoice ROW — create, list, mark sent, mark paid, void, and the request-payment orchestration. `grep -c orderPayments` now answers which file you want. Composed with `.route('/', …)`, the shape every other split router here uses (calendar, inspections, admin, auth). That preserves the hono/client RPC path exactly, which is the thing a reader should doubt: the page still calls `api.invoices[":id"].payments.$post` and `type-check:app` proves it — the api tsconfig alone would not have. `INVOICE_ID` moved to `invoice.schema.ts` because two route modules now need it and importing it back from the parent would be a cycle. It also belongs there under the repo's own rule that schemas live in `lib/validations`, and it carries its comment with it: `invoices.id` is opaque TEXT and a route demanding a UUID 400s a real invoice. The baseline ENTRY is removed rather than tightened. At 339 lines the file is governed by the ordinary 400-line rule again; re-baselining at 339 would have recreated the same wall a little further out. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * refactor(invoices): split the ledger off InvoiceService, and export the gate `invoice.service.ts` sat at 512 lines against a 513 cap. Pure move: not one behavioural line changed, and the 612 tests across invoices, idempotency, billing, qbo, client-portal and contacts pass untouched. TWO MODULES, NOT ONE, and the second is the point. `syncInspectionPaymentGate` was a PRIVATE method called by `markRefunded`, `voidInvoice` and `correctPayment`. It is the thing that stops a report staying publicly unlocked after the payment behind it is reversed, and being private meant a new money writer either moved into this class or silently skipped it — invisible to every test that does not read `inspections`. The #19 Task 3 audit found exactly that omission in the partial-refund plan. So it is now `server/services/invoice-payment-gate.ts`, exported, with the invariant written above it and a spec that already carries its name (`tests/unit/invoices/invoice-payment-gate.spec.ts`). Task 3 can call it; it cannot forget it exists. `invoice-payments.service.ts` takes the six ledger methods — markPaid, markPartial, recordOfflinePayment, correctPayment, listPayments, markRefunded — as free functions over the drizzle handle, the shape `payment-ledger.service.ts` already uses and which these all orchestrate. The boundary is mechanical rather than tasteful: InvoiceService owns the invoice ROW, this module owns anything that reads or appends `order_payments`, and `payment-ledger.service` now has exactly one value importer. A money writer that finds itself importing it in the other file is in the wrong file. `InvoiceService` keeps a one-line method per function. Not decoration: `c.var.services.invoice.x()` and the QBO reconciler's bound callbacks are the established call shape, and rewriting eight production call sites and six specs would have made this a refactor with a blast radius instead of a move. Parameter ORDER is preserved exactly as each method declared it, including the inconsistent `(id, tenantId)` / `(tenantId, id)` pair — both are `string`, so tidying that up mid-move is how the two get swapped without a type error. Baseline ENTRY removed, not tightened. 234 lines is under the ordinary 400 rule, which is the durable outcome; re-baselining at 234 would have rebuilt the wall. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * refactor(invoices): the row's status and its verbs become components `app/routes/invoices.tsx` sat at 509 lines against a 510 cap. Pure move: the rendered markup is byte-identical, every class name and message key is the one that was there, and all 17 co-located tests — which drive the real DOM through `createRoutesStub` and click the buttons — pass unchanged. Two cells, two components, into `app/components/invoices/` beside the three that already live there. `InvoiceRowActions` is the one that earns it. That cell holds a small state machine — a row is either showing its verbs or showing the payment-method picker, and which verbs exist depends on whether the invoice is paid and whether an inspection stands behind it. The page keeps the state (one picker open at a time across the table) and passes it in; the component owns what each branch renders. `getPayMethods` went with it, because the picker is its only caller and the thunk exists for a reason worth keeping next to its use: Paraglide labels must resolve inside the per-request locale scope, not freeze at import. `InvoiceStatusCell` takes the pill, `STATUS_TONE` and `methodLabel`. Small, but it is the whole of one column and it was the only remaining reason the route imported `Pill` and `PillTone`. The IA-122 rationale moved to the control it explains. The client column carried twenty-six lines describing, among other things, why the Action column looks the way it does — with the Action column now in another file that comment would have been the first thing to drift. What is left in the route says what the client cell decides (the name is text, the row has no click handler) and points at the component for the rest. Both components declare their own structural prop type, the convention `PaymentsModal` and `InvoiceAmountCell` already set: "only the fields this surface reads". Baseline ENTRY removed, not tightened — 367 lines is back under the ordinary 400 rule. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * refactor(inspections): one snapshot writer for inspection_services, not two `inspection-core.service.ts:556` was the only place that wrote the tier-2 money-authority rows, and it wrote them inline inside a 250-line `createInspection`. That mattered beyond tidiness: the public booking path never writes these rows at all, so a booking-created inspection has no tier-2 authority and `getEffectivePriceCents` falls through to `inspections.price` — the cache the schema rules forbid treating as authority — or to `price: 0` on the no-service branch. Pay splits attach to `inspection_services` lines, so the feature #278 just shipped is inert for exactly the orders the public form creates. The fix for that is booking-deposit (#20) Decision 3, and its first instruction is: do not write a second writer. So the writer moves out first, whole, to `server/lib/inspection/service-snapshot.ts`, beside `reports.ts` and `roster.ts`. Pure move — the SQL, the precedence rule (serviceSelections over the legacy flat serviceIds), the override map and the two early exits are the ones that were there. The only addition is a return value: the rows written, which #20 needs to resolve a deposit against the summed selected-service price. NOT WIRED INTO BOOKING IN THIS COMMIT, deliberately. Two reasons, both recorded in the module doc so the next reader finds them at the call site rather than in a commit log. First, the call site is a real design choice and not mine to make here: `fulfillBooking`'s direct-insert branch and `InspectionRequestService.create` each own half the bookings, and wiring both double-writes a multi-service order's lines. Second, turning tier 2 on for those orders CHANGES INVOICE TOTALS — from zero to real money — for every booking-created order in existence. That is a behaviour change owed a commit and a test of its own, not a line slipped in under a refactor. The doc also records why `attachHoldServices` (`server/services/concierge/hold-inputs.ts:90`) is NOT folded in despite writing the same table: it throws on an unknown service id, because a hold that silently drops a service under-quotes the client, while the wizard skips unknown ids. Two contracts, two functions. The parameter object spells `| undefined` on both keys rather than relying on `?`: exactOptionalPropertyTypes is on, so an absent key and a present-but-undefined one are different types, and every caller reads these off a wider input object where they are the latter. `services` leaves the schema import — the table is no longer read here. tests/unit/{inspections,idempotency,pay-splits} service-line and effective-price specs, 63 tests, pass untouched. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * refactor(booking): four things were living in booking.service.ts; now four files `booking.service.ts` sat at 971 lines against a 972 cap, and three separate plans — booking-deposit (#20), scheduling-phase-b (#18), scheduling-phase-d (#27) — all need to edit it and none of them could add a line. Pure move: no query, message, status code, log key or ordering changed, and the 133 tests across tests/unit/{bookings,scheduling,calendar,usage} plus the workers db-batch-atomicity spec pass untouched. `fulfillBooking` alone was 513 of those lines, and it was four things. `booking/booking-admission.ts` — EVERYTHING THAT MUST HOLD BEFORE A ROW EXISTS: bot protection, widget origin allowlist, inspector-belongs-to-tenant, booking-open, holiday block, and the slot claim. The seam is the absence of writes. Every refusal here leaves the database exactly as it was; the moment one row exists the failure mode changes from "refuse" to "compensate" (see `arbitrateSlotRace`). Grouping them is also what makes it checkable that quota is consumed after all of them — the property `plan-quota-guarded-services.spec.ts` asserts. `booking/booking-people.ts` — WHO the booking is for. Task 13 dropped the legacy contact columns from `inspections`, so `inspection_people` is the only persistence of who, and on this path it is written from two ends: the referring agent resolved from `?ref=` before anything exists, the client upserted after the rows commit. Eighty lines apart in the old file, which is how one of them gets forgotten. Every write in the module is non-fatal by design and the doc says so once, at the top, instead of five times inline. `booking/booking-confirmation.ts` — what the world hears afterwards: the Google Calendar push and the customer's confirmation email with its ICS invite, inspector signature, credentials footer and SMS double-opt-in link. The seam is "after the answer is sent" — the caller hands it to `waitUntil`, so nothing in it can change the booker's response. `windowLabel` went with it; the email is its only reader. `booking/fulfill-booking.ts` keeps what remains, which is exactly the part that writes and the part that cleans up after itself: the two row-creation branches, the post-insert TOCTOU arbitration that may revoke what was just written, and the scheduled-instant stamp over the survivors. Those three cannot be separated. `AvailabilityService` — a second exported class that shared nothing with the first but the file — becomes `services/availability.service.ts`. It is the WRITE side of availability (the weekly grid and its dated exceptions); BookingService is the read side that turns those rows into bookable slots. They never shared a query. Its three importers (`di.ts`, `types/hono.ts`, `booking-delete-override-scope.spec.ts`) are repointed rather than given a re-export. TWO D1 HANDLES, AND THEY ARE NOT INTERCHANGEABLE. The original mixed `drizzle(c.env.DB)` with `this.db` (`resolvePublicHolidayEffect`, `new PeopleService({ DB: this.db })`), and unit tests construct the service with a handle that is not `c.env.DB`. Both are explicit parameters now, with the reason written down, because collapsing them to one is the obvious tidy that would silently break those specs. `originHeader` stays `string | undefined` rather than being normalised to null on the way through the claim object: it is forwarded verbatim into widget telemetry, where `undefined` omits the key and `null` writes one. fulfillBooking stays a method — six specs and every caller reach it as `c.var.services.booking.fulfillBooking(...)`. Baseline ENTRY removed, not tightened. At 380 lines the file is governed by the ordinary 400-line rule again; re-baselining at 380 would have rebuilt the wall one line further out. `type-check:app` run explicitly, not just `:api` — `types/hono.ts` is on the hono/client RPC path and the api tsconfig does not build it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01QFkgupms2h36Mmf11ojFan * refactor(requests): reading a request and writing one stop sharing a file `inspection-request.service.ts` sat at 500 lines against a 501 cap and is on booking-deposit (#20)'s path. Pure move plus one deduplication, described below; the 138 tests across tests/unit/{inspections,usage,bookings} pass untouched. The seam is read vs write, and the two share nothing but table names. Writing is quota, template-ownership validation, inserts and the `inspection_people` mirror. Reading is one projection — parent plus children plus template names — and its correctness rests on an invariant that has nothing to do with writing: CLIENT NAME COMES FROM `inspection_people`, NEVER FROM `inspections.client_name`. That column survives GDPR erasure as a stale denormalized cache, so reading it leaks an erased subject's name. The Task 9c comment saying so was written twice, above two verbatim copies of a four-table LEFT JOIN — once in `list`, once in `get`. In `inspection-request/request-read.ts` it is `selectSubInspections`, said once, narrowed by the caller's condition. THAT IS THE ONE NON-MOVE IN THIS COMMIT: same joins, same order (role filter first, so the join does not fan out over every role), same projection, same `where` shape with the caller's predicate in the position the caller's predicate was in. `ListFilter`, `SubInspectionRow` and `RequestRow` go with it; only `ListFilter` comes back, as a type impor…

Uh oh!

Per-inspector performance metrics #278

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions