Type coverage for the test suites, gates that can fail, and the defects they found - #306
Merged
Merged
Conversation
…d route to a model
The capability gate keyed on `AiUsageKind` — the two-member COST split
('translate' | 'assist'). That could not express the posture it was asked to
hold: `assist` alone spans rewording one finding, summarising a whole report,
and offering upkeep advice, which are different statements about a property. A
single answer attached to `assist` gave all three the same treatment.
Adds `output-classification.ts`: six classes for what the output IS, and a
posture table over (class, credential source) carrying allowed / requiresReview
/ conditions. The table is a `Record` over both unions, so an unstated posture
does not compile rather than resolving to "allowed" — the previous
`checkAiCapability` ended in a terminal `return { allowed: true }`.
`legal_text` and `repair_pricing` are refused as `capability_prohibited` on both
sources, a new denial reason distinct from `capability_not_released`. The
difference is readable to the person refused: one is a "not yet" they might
clear by confirming a key, the other has no version of itself that ships.
Classification lives on the PROMPT and is REQUIRED by `VersionedPrompt`, with
`AI_PROMPTS` carrying `satisfies Record<string, VersionedPrompt<never>>`.
Verified by removing one: TS2741 at the definition and TS2345 at the chokepoint,
two independent tripwires. The gate asks the prompt rather than `kind`, because
`kind` defaults to 'assist' and a new capability that forgot to pass one would
be judged as generic assistance.
The new gate covers what a type cannot see: a SECOND way to reach a model.
Classification is only enforced where `callGemini` demands a `VersionedPrompt`,
so code that sends a prompt another way is a capability nothing classifies,
meters or records. Rules: a completion endpoint outside the provider adapters,
and adapter construction outside the two files whose job that is.
Keyed on the ENDPOINT, not the host. `secrets.ts` and `integrations.ts` both
call generativelanguage.googleapis.com for the "Test connection" diagnostic —
`/v1/models?pageSize=1`, which asks which models exist and sends no prompt. A
host-based rule would have flagged both on day one.
Each rule proven to go red before being kept: all three fire, and the gate fails
when it cannot find the prompt table or scans too few files. Fixed a defect
found that way — the unclassified-prompt finding reported line 19 instead of
107, because the offset of the table was not carried through.
`requiresReview` has no enforcement yet: there is no review surface and nowhere
to record that a review happened. It is asserted in tests so the rule cannot be
silently flipped in the meantime.
Refs InspectorHub#60
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…n full lint Three registration points, all hand-duplicated in this repo: `lint`, `lint:gates-full`, and `SCRIPT_GATES` in run-gates.mjs. Pre-commit rather than CI-only, on the criterion already stated for the two gates beside it: what it catches is a CAPABILITY arriving. By the time CI sees a new AI call site it is written and argued for, and "no capability reaches a model outside the chokepoint" is much easier to hold than "please reroute the feature you just built". Measured 0.4s against 0.31s for the tracking gate it sits next to. Its own commit because staging package.json escalates the pre-commit type-check from the api tier to the full one. Refs InspectorHub#60 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…t depends on #19 Task 4. The audit said this task was three UI surfaces rather than three prose steps, and that held: there was no cancellation settings panel, no agreement editor, and Task 3's cancellation-quote endpoint had zero consumers. This lands the settings side. The clause lives in this panel rather than on the agreements page because a fee is only collectable if the signed agreement says so — one decision, and splitting it across two screens produces exactly the state the server refuses: a configured fee whose clause nobody confirmed. POST /api/admin/branding applies the attestation BEFORE the policy, so confirming and enabling is one save. cancellationClause {current, everAttested, agreementId} is now computed on the SERVER and shipped in the branding payload. `current` is getCancellationAttestation() !== null — the attested-vs-current version comparison. Deriving it in the loader from the raw columns plus the agreement list would put a second copy of that invalidation rule somewhere that has to remember to run it, and the copy the panel shows would eventually disagree with the one the fee gate reads. `everAttested` is deliberately the RAW timestamp: an attestation invalidated by an edit is still one that was made, and "you confirmed this, then the agreement changed" needs different words from "you have never confirmed this". Two extractions, both because a file was at its size cap and both defensible on their own terms. The deposit save moved to booking-policy-actions.ts beside the new cancellation intent — they share the "an absent key must not read as a clear" rule, which is load-bearing because neither field has a Zod .default() and null is a real instruction that CLEARS the policy. The fee row became its own component because both rungs are the same three answers, and two copies would be two places for the percent/cents rule to drift. TWO DEFECTS FOUND IN CHROME THAT THE UNIT TESTS COULD NOT SEE: The clause requirement was gated on the PARSED policy, so picking "Percent of the price" with an empty box removed the "you charge nothing" line and put nothing in its place — no requirement, no guidance, no error. The clause is a precondition of the INTENT, so it now appears with the intent. Regression test added and proven to fail without the fix. And the no-agreements state said "create one" without saying where, which is unactionable from a settings panel. It now links to /agreements. Tests proven red before being kept: removing the clause guard fails exactly one case, and making attestCancellationClause unconditional fails exactly one other. The second is the trap — the key is transient and its null WITHDRAWS an attestation, so a helpful default would revoke the confirmation on any unrelated fee edit. The price-capability gate caught the new money control, as designed. Inventoried as the tenant's own fee, the same category as the deposit — not a repair cost and not a negotiation amount. Verified in Chrome, light and dark: contrast on the segmented control, the percent input, the amber requirement, the new link and the save button. Refs #19 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…a trade_slug #26 Task 2, Steps 1/2/4. Step 3 (backfilling existing tenants) is NOT in this commit — see below. THE BLOCKER IS RESOLVED, AND THE PLAN'S ORIGINAL SHAPE WON. The audit marked this task blocked on a contradiction: the plan describes a soft link to an immutable DEFECT_TRADES, while a later note recorded a decision to DELETE DEFECT_TRADES and make contractor_types the single authority. That decision was conditional on the constant having no effect, and it does have effect: - server/services/inspection/shared.ts sanitizes at runtime with isDefectTrade - server/types/inspection-item-state.ts uses DefectTrade as a compile-time type - tests/unit/inspections/defect-fields.spec.ts pins its length and membership And there is a SECOND, hand-copied vocabulary in app/lib/defect-fields.ts feeding three editor components — so deleting the server constant is both an eight-file break AND it strands the client copy, which would keep validating against a list the server no longer has. That is worse than either design. The two copies existing at all is a real defect, but it is a different change. WHAT trade_slug BUYS. `name` is free text a tenant may rename at will, and the mapping to the canonical trade dies the moment they do. The slug survives it. NULL is valid and PERMANENT, not a backfill gap: a tenant-created type has no counterpart, and a tenant who renamed a seeded type before this column existed cannot be matched by name. Both correctly stay NULL. THE SEED IS NOW DERIVED, NOT COPIED. It was ten hand-written names under a comment saying they "MUST stay in sync with the contractor-type backfill in 0000_baseline.sql" — a backfill that no longer exists, so the comment pointed at nothing and nothing enforced it. Per the Comment Rules a sync note becomes executable or it becomes wrong; the fixture computes 20 canonical rows from DEFECT_TRADES plus the 2 tenant-only extras, and a test asserts the derivation. It went in its own module because starter-content.service.ts had ~29 lines of headroom and the fixtures/ directory is where this repo already keeps seed data. Seeding now matches canonical rows on SLUG rather than name. There is no unique index on this table, so a name-keyed seed would insert a second row for a trade a workspace already has under a different name, and nothing downstream would object. Covered by a test that renames a seeded row and re-seeds. Assertions split by row kind, which is the mutual unsatisfiability the audit found: one test asserting the seeded slug set EQUALS DEFECT_TRADES cannot coexist with a tenant-only type carrying NULL. The extras win — they are real data — so the canonical assertion filters NULLs. Two existing assertions updated rather than deleted: the ten-name literal and the count-of-10 both became derivations, so the trade vocabulary can grow without editing a magic number. Title-casing leaves a word alone when it already contains a capital, so "HVAC technician" does not become "Hvac Technician" — wrong in a way that looks deliberate in a diff. Asserted. NOT DONE, deliberately: Step 3 backfills 11 existing tenants and is a production data migration. It needs a dry-run count per group and a duplicate guard, because this table has no unique index and re-running would insert duplicates. Refs #26
Follow-up to 1f4effc. `CancellationClauseStateSchema` was exported and then used only by `BrandingResponseSchema` twenty lines below it, so knip counted it as a new unused export and `lint:deadcode` went red — a gate that lives ONLY in the full lint, not at pre-commit, which is why the commit that introduced it passed. Found by a subagent running the full gate set on unrelated work. Worth noting as the mechanism rather than the typo: `knip-baseline.json` is `[]` and must stay so, and the rung that enforces that is the pre-push full run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
… data path InspectorHub#275, Tasks 1/2/3/5-read/6. The tag control itself is deliberately NOT here; see below. Q1 = REJECT, scoped to the field. A non-null tag from `creator.kind === 'inspector'` returns 403, placed AFTER `runAssertCanEdit` in both item handlers.⚠️ The field's ABSENCE stays legal, and that is the whole point: an inspector on owner-preview creates lists and adds items today, so "the service refuses an inspector-authored write" — the obvious implementation — would break a working flow. Two tests hold that line: the tagged write is refused, the untagged one still succeeds. Q2 = REJECT'S SIBLING DECISION, Q2b = BACKFILL. The `UPDATE` rides in the GENERATED column migration rather than a hand-written one of its own: a standalone data migration ships with no drizzle snapshot, which severs the meta chain so `db:generate` refuses to author the next migration — and `db:check` cannot see the break, because the resulting tables are still correct. Only `lint:migchain` catches it. Guarded `AND repair_action_tag IS NULL`, so it is re-runnable and never overwrites a chosen tag. The backfill is recorded as a decision, not a convenience: it IS the platform stating an intent on the buyer's behalf, on a negotiation document they already submitted. It was chosen over asserting nothing because gating the amount input on `fund` would otherwise make every existing credit vanish from the builder — the buyer's own stated ask, silently absent, with nothing looking wrong. It must not be extended to inferring any other tag value. TWO LIVE BUGS FIXED IN PASSING, both the same shape the plan warned about: `app/routes/agent/repair-items.tsx` posts an explicit field list and was already dropping `trade` — and it was missing from the `SourceDefect` interface too, not just the JSON body. And `crud-routes.ts` carried a second stale route description, corrected at source BEFORE regenerating the MCP snapshot. The share page renders the tag in the NOTE cell, not the Finding cell. Everything in the Finding cell is inspector-authored, so a seller reading "Replace" there reads it as the inspector's recommendation. A test asserts the tag is not inside the trade cell's parent. Tests proved red before being kept, one per seam: the 403 on POST and PATCH, the untagged-inspector control, the backfill COUNT (breaking it printed `before = 7; after = 0`), zero-credit-is-a-credit, the insert, the idempotent re-add, both form seams, the agent forward, four share-page assertions, and the erasure blind-spot key.⚠️ ONE TEST DID NOT GO RED, and its name and comment now say so. Narrowing `updateItem`'s allow-list back to credit+note leaves the runtime spec green, because drizzle's `.set()` forwards whatever the object holds — that allow-list is enforced by `type-check:api`, not at runtime. Claiming the spec covered it would have been an assertion that tests nothing. The erasure out-of-scope entry is registered AND added to `HEURISTIC_BLIND_SPOTS`, which is what makes it enforced rather than merely disciplined: the manifest gate matches a fixed PII name pattern and `repair_action_tag` matches nothing. 🛑 Task 3a (the agent-side tag) is NOT done, and it is larger than the plan assumed. `AgentRepairRow` comes from `agent-recommendations.ts`, a different flattener over `templateSnapshot` + `resultsData`, while the builder goes through `getRepairList()`. There is no join key: `findingKey` carries a `#N` collision ordinal that depends on `getRepairList`'s row order, and the agent flattener computes neither `source` nor `recommendationId`. Deriving it there would be a second copy of a keying scheme whose ordinals depend on another module. The documented alternative — drop the agent tag explicitly — lands with the UI task. NOT DONE, deliberately: no visible control, and `RepairBuilderSection.tsx` untouched at 527/527. Resolving that zero headroom (extract a `useRepairItemDrafts` hook) is part of the UI pass, which also needs a real-browser walkthrough. Refs InspectorHub#275 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
… gates InspectorHub#275 Task 4. Completes the data path landed in b32b65b — that commit deliberately shipped no visible control, and a data path with nothing bound to it is the unwired-primitive shape this programme keeps logging.⚠️ NOT YET VERIFIED IN A REAL BROWSER. Stated up front because this repo's UI gate is not a formality: Chrome caught two defects on the cancellation panel today that eight passing unit tests could not see. The local database has no published report with client portal access, the seed does not create one, and no e2e spec visits this page — so reaching it needs a publish → deliver → client-link flow. The specific risk a browser would answer: a native `<select>`'s dropdown arrow and option list are UA-rendered, which is exactly the part design tokens do not control, so dark mode can produce unreadable options even with correct classes. Do the walkthrough before the upstream PR. RepairBuilderSection was at 527/527 with zero headroom. Draft state and the item mutations moved to `useRepairItemDrafts`, which is a better home on the merits: no rendering, no fetcher, the offline queue injected as `enqueueOp`. The queue stays in the parent because it needs the item-id map derived from the same `existingItems` — one direction only, so there is never a second source for it. 527 → 450, and the ratchet was TIGHTENED to 450 rather than bumped. A native select, not the SegmentedControl the settings panels use: five choices do not fit this row's width without wrapping, and this page is used on a phone at a property. The option words are the SAME ones the shared list renders — an action that changes name between choosing it and reading it reads as two things. "No preference" is an option rather than a blank, because having no preference is a different statement from not having reached the question. It reports null, not "", so the route's `parseRepairActionTag` clears the stored tag instead of the draft showing cleared while the server stays set. `updateTag` enqueues on a null, unlike `updateCredit`. Clearing a tag is a real instruction — the buyer withdrawing a stated intent — while an empty money box is the state a half-typed amount passes through. The tag also rides on the add-item op, because a defect can be tagged, deselected and reselected, and the second add would otherwise resurrect the item without it. 🔴 A TEST I BROKE WITHOUT BREAKING IT — the reason this commit is worth reading. `RepairDefectRow.test.tsx` guards "the row prints no supplied estimate" with an explicit anti-vacuity check: assert the credit input is on screen, so the absence assertions cannot pass against a collapsed row. Adding the action select kept that guard GREEN while the thing it guards stopped rendering — because the new select's aria-label is "Requested action for Shingles" and the guard matched `/Shingles/i`. A different control answered to the same matcher. The guard now matches `/credit request for Shingles/i`, and the fixture carries `actionTag="fund"`; removing that tag makes it fail, which it did not before. `actionTag` and `onUpdateTag` are REQUIRED props, not optional like `phrases`. `AgentRepairInspectionBlock` renders `RepairDefectRowView` directly and never reaches this component's expanded region, so no caller legitimately omits them — and optional would let a select render with no handler behind it. The cross-portal guard is NOT extended to compare the tag: it renders `isSelected={false}`, so a tag absent from both sides would compare equal and pass for the wrong reason. Every new assertion proven red first: removing `=== "fund"` fails exactly the four hiding cases and nothing else; the three non-money answers are asserted individually, because a gate written `!== 'repair'` would satisfy a test that only tried one of them. Refs InspectorHub#275 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…client link that lets anyone look at it Three things, one cause: the InspectorHub#275 control could not be opened in a browser locally, so it shipped unverified. It has now been walked through, and doing that required seeding the path. **The control uses the DS `Select`, not a bare `<select>`.** The raw element was wrong for a reason already solved once in this repo: its chevron and option list are UA-rendered, which is the one part design tokens do not reach. `packages/shared-ui/src/Select.tsx` sets `appearance-none` and draws a tokenized chevron over `.ih-input` precisely because of that. `bare`, because this row supplies its own micro-label to match the credit field beside it, and `!h-8` because `.ih-input` is 36px while this row's controls are 32px — matching the neighbour beats matching the default when the two share a grid row. **Seeded the publish → deliver → client-link path**, which no fixture covered. `runBuilderGate` turned out to need four things, not the three I had established: published report status, `is_customer_repair_export_enabled` (not `enable_customer_repair_export`, as I had it), a live `inspection_access_tokens` row — and a `contact_role_profiles` row, because `resolveBuilderAccess` maps the token's role KEY to a role-profile KIND and returns null for anything else. With no role profile a perfectly valid token 401s, which is invisible from the token row.⚠️ The token fixture writes BOTH `token_hash` and plaintext `token`. The resolver reads the hash first and lazily UPGRADES a plaintext hit by rewriting the row, so hash-only avoids a mutation on first use, while keeping the plaintext lets a re-issue path return the same token instead of throwing. `token_enc` cannot be seeded — it needs the worker's HKDF KEK. **An e2e spec that actually opens the page**: list renders, the action control gates the credit, the choice survives a reload (the whole write path — form parse → service → column — which every unit test stubs), and the same URL without a token does not render the list. ## Found in the browser, fixed here `public/sw.js`'s offline response sent `text/plain` with NO charset, so its em-dash — two UTF-8 bytes — was decoded with the browser's legacy default and rendered as "Offline 钦� please reconnect to continue." Nothing in the source looks wrong; only a browser on a CJK-locale machine shows it. ## Verified in Chrome, both themes `color-scheme` resolves to dark/light on html, body AND the select, which is the mechanism that makes the native option list follow the theme — the specific risk that made a bare `<select>` unsafe. Select and money input both measure 32px with identical top offsets, so the grid pair aligns. Contrast checked in both. ##⚠️ A pre-existing bug this exposed, NOT fixed here Choosing an action while the add-item request is still in flight loses it silently: the row keeps showing the choice, no error appears, and the column stays NULL. `useRepairOpQueue.ts:65-73` drops `update-item`/`remove-item` ops whose server item id is not yet known, with no retry. The comment justifies one case ("added and removed before its add resolved") while the code covers every case — including the one where the add is about to succeed. Not InspectorHub#275-specific; a note or credit typed in the same window has the same exposure. Its own commit follows. Also incidental: `tests/e2e/core.integration.spec.ts:336` has a TS1005 syntax error that has gone unnoticed because `tests/**` is only in `tsconfig.playwright.json`, which no script and no CI step runs. Refs InspectorHub#275 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…is in flight A client who ticked a defect and immediately chose a requested action — or typed a note, or entered a credit — lost it. The row kept showing the choice, no error appeared, and the column stayed NULL. Nothing looked wrong to anyone.⚠️ THE BUG WAS A COMMENT THAT JUSTIFIED LESS THAN THE CODE DID. The branch dropped every `update-item`/`remove-item` op whose server item id was not yet known, with this reasoning: "Item not on the server (e.g. added+removed before its add resolved, or never persisted) — nothing to do". That case is real and the drop is right for it. But the code also caught the case where the add is STILL IN FLIGHT and about to succeed, which is the ordinary path, not an edge one. Reachable because `drainQueue` guards on a `mutationFetcher.state` value CAPTURED at render, while a click handler can hold a closure from before the add was submitted — so the guard reads "idle" while a request is outstanding. `inFlightAddKeyRef` is a ref and reads the truth at call time, which is why the new check uses it rather than the fetcher state. Now: if an add for that findingKey is in flight OR still queued, the op goes back to the FRONT of the queue and draining stops; the settle effect drains again once the id is known. Order is preserved, so a later update cannot overtake an earlier one. If no add is pending the op is still dropped — which is also what bounds the deferral, because a FAILED add clears the ref and the next drain reaches the drop instead of requeueing forever. Not InspectorHub#275-specific. The tag is simply the field most likely to be set in the second after ticking the box, which is why extending the builder surfaced it. Found by a subagent running the seeded E2E suite at 3 workers; the same spec passes alone, which is why the obvious spec shape is green on a quiet machine. The regression test here drives the hook through a fake fetcher whose state it controls rather than hoping a real one lands in the window.⚠️ And one of those new tests was VACUOUS when first written. `useRepairOpQueue` declares `createFetcher` BEFORE `mutationFetcher`, so the fake at index 0 is the create fetcher — and "drops an update for an item that will never exist" asserts ZERO submissions, which an unused fetcher satisfies for free. It passed against the wrong object. Fixed, and the index is now stated in a comment at the mock. Disabling the deferral fails exactly the one case that covers it. Refs InspectorHub#275 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…ases duplicating the action Two findings from the InspectorHub#275 browser walkthrough, both left open in the previous commits. ## The e2e type program was never run because it COULD NOT run `tsconfig.playwright.json` appears in no npm script and no CI step, and a real TS1005 syntax error had been sitting behind it in `tests/e2e/core.integration.spec.ts` — a `test(...)` block missing its closing `});`, so the next test was nested inside it.⚠️ But the wiring was not the root cause. It `extends` the app program and adds `tests/**/*` with NO `references`, so the ~2045 `../../../server/…` imports in the test tree were compiled AS SOURCE: one non-composite pass holding the whole server tree, the app tree and every spec. Measured: 2 minutes to a heap OOM at the default ~4 GB. **A program that dies is indistinguishable from one nobody wired up**, and that is why the syntax error survived. Referencing the api project — the same fix that took `tsconfig.tests-typecheck.json` from 238s to 7s — brings it to **16.7s**. Added as `type-check:e2e` and appended to the `type-check` composite, so CI's existing typecheck job picks it up.⚠️ SCOPED TO `tests/e2e/**`, which is a decision and not just the name. Across the whole test tree it reports HUNDREDS of pre-existing errors in `tests/unit/**` — missing Cloudflare globals, a better-sqlite3 handle passed where a D1 one is typed, drizzle insert shapes. Nothing checks any of that today (vitest strips types with esbuild), so it is real debt, but turning it all on at once yields either a red gate nobody can land against or a suppression list — and a suppressed gate is the state this config was already in. Recorded rather than papered over. Two genuine errors fixed to get to green: `tests/e2e/helpers/html5-drag.ts` declared `const STASH = '…'` without `as const`, so the computed key in `Win` produced no literal property and `w[stash]` was an untyped index. Proven: a deliberate type error in an e2e spec now fails the gate, and green returns when removed. ## The note quick-phrases said what the structured field now says The seeded defaults were literally "Repair requested" / "Replacement requested" — the same two answers the new REQUESTED ACTION control offers. A buyer could state the same request twice, in two places, with nothing reconciling them. These are OUR defaults, not tenant data, so they are ours to fix; I previously described them as tenant-configured, which is true of the list but not of what ships in it. The note's remaining job is the RATIONALE — conditions on how the work is done and what evidence is wanted, which no structured field carries. Now "Licensed contractor required" / "Please provide the invoice", still two, so the "[] means the tenant switched them off" distinction keeps a non-empty default to be distinct from. Copy decision, overridable per tenant. Key names changed with the values: `_repair`/`_replace` encoded the old meaning and would have misled the next reader. es-419 translated in the same commit — `lint:i18n-catalog` holds parity at 4635/4635. Refs InspectorHub#275 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…d the call id it cites InspectorHub#61 data path. The review CONTROL is a separate commit; this is the evidence store and the plumbing that lets a row point at the call it reviewed. The join points AT `ai_call_provenance`, never the reverse. That table's schema comment forbids adding an identifier of the inspection INTO it — that pulls it into erasure scope — so `ai_content_reviews.ai_call_id` points outward and adds nothing to it. `model` and `prompt_version` are deliberately NOT columns here: they already live on the provenance row, and duplicating them creates two numbers that must agree and eventually will not. `record()` now returns the id it inserted, hoisted above the insert and still awaited, so a caller never holds an id for a row that does not exist. That threads out through `callGemini` and the four public methods into the four response schemas. ✅ No MCP snapshot step: the snapshot records `inputSchema` only, verified earlier by regenerating after a response-schema change and getting a zero diff.⚠️ `aiCallId` is NULLABLE on three of four methods, and the invariant is written down rather than left to be inferred: it is present exactly when the payload carries model-generated text. Three arms return prose no model wrote — the standalone dev mocks, the "No significant defects observed" literal, and the empty list a runtime failure degrades to. An id on any of those would be evidence of a human reviewing model output that was never generated, which is worse than no id. `SuggestCommentResponseSchema.data` changes from a bare `string[]` to `{ suggestions, aiCallId }` — an array cannot carry the field. The only breaking response change in the set, and nothing in `app/` consumes any AI endpoint. ## artifact_type has ONE member, and finding out why was the useful part The agent's draft had `['inspection_result', 'report_version']` and flagged the second as a guess. Checked, and it is worse than a guess: 🔴 **`report_versions.summary` is not a report summary — it is the per-publish AMENDMENT REASON.** `inspection-report.service.ts` says so outright ("Reason reuses report_versions.summary"), it surfaces as `reason` in the amendment trail, its value comes from the publish body, and `automation/delivery.ts` reads it into CLIENT DELIVERY. AI summary text written there would reach the client as "why this version was re-published". There is no report-narrative column anywhere. So the member named a home that does not exist, and it is removed — a reserved slot must not double as an unlocked door. Filed as its own task with the shape to copy (`inspections.pca_narrative`, which already solved this on the commercial side) and the warning that the residential field must not be called `summary`: that word already means three different things here, none of them prose. Removing the member needed no migration change — drizzle `{ enum }` is type-layer only and the DDL is a bare `text NOT NULL`. `db:check` clean at 96=96. ## Erasure: out-of-scope register, not a manifest rule All seven columns declared, following the `ai_call_provenance` precedent of listing every column. The row holds a staff identity, two opaque ids, an enum and a timestamp — and, by the same constraint as the provenance ledger, no part of the reviewed text. `reviewed_by` is a STAFF user; the register already answers exactly this shape for `report_signoff.person_id` and `erasure_log.requested_by`. `artifact_id` follows `reports.inspection_id` — the professional record carries the rules, annotations on it follow.⚠️ The declaration alone would be discipline only: `check-erasure-manifest.mjs`'s PII heuristic matches nothing in these names. `reviewed_by` and `artifact_id` are pinned in `HEURISTIC_BLIND_SPOTS`, which is what makes it enforced — demonstrated by deleting both declarations and watching the coverage spec go red WHILE `lint:erasure` stayed green. Five breakages proven red-then-green, including the absence-trap guard: making `suggestComment` return early before reaching `callGemini` fails `expect(record).toHaveBeenCalledTimes(1)`, so "no id" cannot pass for free when the method never ran. Refs InspectorHub#61 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
InspectorHub#61, server side. The control itself is next; this is the endpoint it posts to. The reviewer is WHOEVER IS AUTHENTICATED, never a field in the body. A client-supplied reviewer id would let one account file a review in another person's name — and naming the person IS the claim this row exists to make. ## Idempotent by an index, and proven rather than declared The gate offered two ways out: a replay spec, or an `uncoveredByDesign` entry with a one-line reason. "Naturally idempotent" is a claim about behaviour, so it gets a spec — declaring it would have been the one place in this change where I asserted something instead of showing it. Unique index on (tenant, artifact type, artifact id, ai call, reviewer) plus `ON CONFLICT DO NOTHING`.⚠️ Not read-then-insert: two concurrent retries would both see no row, both insert, and the index would reject the second with a 500 — turning a harmless replay into an error the client has to interpret. The conflict clause makes the second write a no-op atomically.⚠️ `reviewed_by` is IN the key. Two people reviewing the same output is two facts — exactly what a four-eyes policy wants — so the key must not collapse them. Both dedup cases in the spec are paired with a CONTROL that must produce a second row, because "one row after two calls" is satisfied for free by an implementation that never inserts anything. Removing `ON CONFLICT` fails exactly the two dedup cases and leaves both controls green. ## Two gate details worth writing down The coverage gate wants the route path as a QUOTED STRING LITERAL (`specText.includes("'" + path + "'")`), so a path merely embedded in a full URL string does not count. That strictness is right — it wants the route declared, not incidentally contained — and the spec now names it as its own constant. ✅ And this time `mcp:snapshot` DID drift (+18 lines), which confirms the earlier finding was precise rather than lucky: the snapshot records `inputSchema` only, so a response-schema change produces a zero diff and a new route with a request body does not. Refs InspectorHub#61 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
… need InspectorHub#61 was scoped as "add review controls to the AI assist surface in the editor". There is no AI assist surface. Four generation endpoints (comment-assist, auto-summary, comment/edit, suggest-comment) have shipped for a long time; nothing under app/ has ever called one. The typed client is even constructed on every request (`api-client.server.ts`: `ai: mk<AiApi>`) and never dereferenced. So the whole feature has been reachable by an API client and by nothing an inspector can click — which is also why the review table could land before any control that writes to it. This wires ONE capability end to end, with the review attached: results GET now returns `resultId` — the `inspection_results` primary key an `ai_content_reviews` row cites. The map never carried it, so the editor had no artifact to name. Nullable for legacy inspections with no results row; the affordance is then absent rather than recording against a guessed id. `/resources/ai-assist` — BFF with two intents. A client fetch of /api/ai would arrive unauthenticated (the JWT is a server-held cookie), and the aiCallId is only ever the one the assist response returned, so a caller can cite only calls it actually caused. The reviewer stays server-derived. `AiAssistPanel` — the draft appears BELOW the note, never in it. The inspector's own words and the model's stay visible together until they act.⚠️ FAIL CLOSED ON THE REVIEW WRITE: if the review cannot be recorded, the note is not changed. Text-in / evidence-maybe is the exact state InspectorHub#61 exists to end and it fails silently. Both that and the no-artifact refusal were proven red first — the fail-open reordering and the removed guard each turn exactly one spec red. The control says REVIEW, never ACCEPT. "The user clicked confirm, therefore the platform is absolved" is not a position this product takes, so the checkbox states only what the inspector is in a position to state. Chrome (light + dark, real D1) found two things the specs could not: `border-l-ih-warn` compiled to nothing. This design system has no `warn` colour — the family is `ih-watch` — so the accent rule that carries the whole "not yet decided" signal was silently absent. A wrong token name is not a failure, it is no rule at all. The banner rendered "Internal server error Your note was not changed." The server does not promise its message ends in a sentence. Two lines now. Chrome also confirmed the write: one `ai_content_reviews` row naming the person, the artifact `seed-delivered-results`, and the call. file-size baseline bumped for three already-grandfathered files (+24/+15/+2); splitting any of them is a refactor this change does not justify. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
… duplicated vocabulary
## lint:i18n-glossary had three false greens, all reproduced before fixing
The glossary's own header claims the gate "fails the build when
messages/es-419/** contradicts" its tables. It does not:
The APPROVED-TRANSLATION COLUMN IS ENFORCED BY NOTHING. `row.approved` is
parsed and then used only inside an error message. Of 231 rows, ~200 have an
em dash in the Never column and are skipped outright; 31 strings are checked.
Corrupting `| Report | informe |` to `| Report | XXNONSENSEXX |` printed
"OK - 231 term row(s)" and exited 0.
THE FLOOR WAS CALIBRATED ONE BELOW TOTAL COLLAPSE. `MIN_BANNED_TERMS = 15`,
and the Register enforcement table supplies 16 of the 31 - the entire
usted/tu vocabulary. Deleting that whole table leaves 15, `15 < 15` is false,
and the gate prints OK.
DO-NOT-TRANSLATE WAS PROSE. The document calls a translated STOP "a
compliance failure, not a typo" and nothing checked it. Translating STOP in
an SMS-keyword key: green.
Every parser change now biases toward FAILING: columns bind by header name
rather than position, a table must sit directly under its marker (the old scan
latched onto any table downstream), marker and table counts must agree, row
width must match the header, prose in the Never column is an error, and a
banned term equal to some row's approved value fails at parse time. The two
magic floors became a per-section baseline. A `gate:literal` table now
enforces 35 literals byte-identical.
Also: the old script contained a literal NUL byte, so `file` called the gate
binary and grep skipped it - you could not search your own gate.
## The symbol resolver renamed baseline keys when a signature wrapped
`enclosingSymbol` only recognised a method whose whole signature fits one
line. Widening a return type until it wraps therefore moved the key of every
hit inside - and a moved key reads as a NEW violation. That is how
lint:tenant-scope came to fail on `ai.service.ts::generateProfessionalComment`,
a method containing no query: the unscoped select belongs to
`generateInspectionSummary`, whose signature grew
`Promise<{ summary: string; aiCallId: string | null }>` and wrapped.
AND IT WAS WORSE THAN A WRONG NAME. Four query sites in
marketplace.service.ts resolved to two symbols, so two pairs produced
IDENTICAL keys and the baseline - a Set - held one entry per pair. Baselining
one silently baselined the other. On a gate whose subject is cross-tenant data
leaks, that is a hole. The baseline goes 70 to 72 because the fix un-collapses
them; the 13 changed entries were each verified to be a rename of an existing
(file, signature) pair, with zero new query sites.
Two attempts were needed. A line-shape regex alone matched `and(` and `or(` -
wrapped drizzle predicates - and renamed a dozen keys to `::and::`. Candidates
are now confirmed by reading forward to the matching `)` and requiring `{`.
The obvious return-type pattern for that test rejected
`): Promise<{ ... }> {` and fell through to naming the `constructor`, so the
test is simply "ends with `{`".
## One vocabulary, not two
`app/lib/defect-fields.ts` held its own copy of the trade / deadline /
timeframe lists. They were identical by luck: nothing compared them, and the
server list is the one that decides what survives a write - `shared.ts` nulls
any value it does not recognise, so a drifted client would offer options the
server silently discards. The client module now imports the server list. Drift
is not detected, it is impossible, which is why no gate was added.
`server/types/defect-fields.ts` is now in the CLIENT bundle. It has zero
imports today and its docblock says to keep it that way.
## Recorded, not changed: the repair_action_tag axis
The plan that introduced the column cited "the union of HIP and ISN" for its
four values. ISN's help centre describes those four as what the reviewing
AGENT picks - verbatim, "as a response to the request item" - and Home
Inspector Pro's page carries no role label at all. The column stays on the
REQUEST axis as a product choice; the schema now says which of those two
things is evidence and which is a decision, and that a response axis must
never reuse this column or its words.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…s asks to read Two features that arrived at the same gap from opposite ends: a report the inspector cannot speak in, and a document the buyer writes that the inspection company cannot see. ## InspectorHub#71 - reports.inspector_narrative The residential report had NO inspector-authored narrative field anywhere, which is why `POST /api/ai/auto-summary` has generated a report-level summary for a long time with nowhere to put it. It is not called `summary`, and that is the point. `report_versions.summary` is the per-publish AMENDMENT REASON - surfaced as `reason` in the amendment trail and read into client delivery - so the obvious name means something else one join away. And it is not an item: `ItemType` has nine members and none is narrative, while `inspection-analytics.service.ts` counts an item as captured on `rating != null || value != null`, so prose parked in a `textarea` item would be tallied as an inspection data point. `inspector_` names the author because the author is the constraint: the narrative carries professional liability, so a model may draft it and may never be it. The AI review evidence follows it. `aiContentReviews.artifactType` becomes `['inspection_result', 'report']` - the member its own schema comment said to add "in the commit that adds the field it points at". It is `report` and not `report_version`: a version row is an immutable publish snapshot, so a review attached to one would be orphaned by the next re-publish. THE ENUM CHANGE EXPOSED A FALSE GREEN IN ITS OWN TESTS. Removing the new member left all 13 specs passing, because drizzle `{ enum }` is type-layer only - no CHECK constraint reaches D1, so SQLite stored 'report' against a one-member enum without complaint. Insert-and-read-back proves nothing about a declaration. Two assertions now read the declaration itself. GDPR: `PII_HEURISTIC` matches nothing in `inspector_narrative`, so free prose about the data subject would have shipped with no rule and no signal. An `anonymize` / `art_17_3_e` rule is wired into the existing `reports` erasure step and pinned in `HEURISTIC_BLIND_SPOTS`. Clearing it is safe for the integrity chain ONLY because the narrative is not in `report_versions.snapshot_json`; putting it there reopens that decision. NOT YET ON THE DELIVERED REPORT. `getReportData` never reads `reports`, and the publish snapshot does not include it, so the narrative is editor-only and outside `content_hash` today. A deliberate stopping point, not an oversight. ## InspectorHub#69 - the Repair Request Log Nothing on the server or app side listed submitted repair requests to staff. `listMine` filters by creator, and a staff JWT resolves to `{kind:'inspector'}`, so "mine" for staff meant only the lists an inspector built in owner-preview - never the buyer's. Every repair route also lives on `/api/public`, which the JWT gate skips, so no staff-reachable read could exist there. Shape taken from the industry, not invented: read-only, on the order record rather than in the editor, many lists rather than one, published-only, and no submission notification - all four sourced from Spectora's and ISN's own help centres. Research in docs/superpowers/research/competitors/2026-08-08-inspector-side-repair-request-visibility.md. `GET /api/inspections/:id/repair-requests` sits under the JWT gate with `requireRole`. On an unpublished order it returns `lists: []` WITHOUT querying - and the test asserts the service was not called, because `lists: []` alone passes with the gate deleted. Fields are projected explicitly, never spread: `repair_requests` carries a live `share_token`, and a spread would put a bearer credential into a staff page the day someone adds a column. The read-only surface renders `RepairDefectRowView`, the inner presentational component - `RepairDefectRow` now requires `onUpdateTag` and would put a tag control on a document the company does not author. `cross-portal-reuse.test.tsx` grew a third portal. Spectora reaches its log from a three-dot menu on the inspection detail page; there is no kebab menu anywhere on this hub and `BlockHeading` documents that headers are labels and never controls, so the faithful translation is a bottom-of-card action on the deliverables card. ## One type fix that is not cosmetic `listForInspection` needs an early `return []` (an `inArray` over an empty list renders `in ()`, which SQLite rejects), which made its inferred return type `never[] | RepairRequestWithItems[]`. A caller's `.map(rr => ...)` then resolved `rr` as an implicit any - and what sits under that map is the explicit projection keeping `share_token` out of the response. The return type is now stated, and the shapes moved to `repair-request.types.ts` to stay under the file-size ceiling rather than bumping a baseline for a file that was never grandfathered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
Chrome found what the component tests could not. The link shipped on `ReportsCard` - the PLURAL deliverables card - and rendered directly beneath that card's empty state, "No reports on this order yet", on an order whose report was published and whose SINGULAR report card said Published ten centimetres below. Two cards, near-identical names, different subjects. The plural card lists the deliverables generated from services sold; the singular card owns the publication lifecycle. The log is about a published report, so it belongs with Create re-inspection / Send report / Unpublish, inside the branch already gated on `reportShipped` - which is the log's own precondition, not a second one bolted on. The two tests that passed on the wrong placement asserted only that the card renders a link when handed a href. That is true of any card. They are replaced by a source-level assertion that the link sits inside the published branch and is absent from `ReportsCard` entirely - unusual for this repo, and deliberate: the question is structural, the hub route is 1,200 lines and expensive to render, and "which of two similar cards" is exactly what a component test cannot ask. Proven by relocating the link into the unpublished arm and watching two of the three go red. It does not guard safety. If the link ever leaked onto an unpublished order the page still refuses, and that gate is tested on the route where it lives. This guards reachability and coherence - the failure this codebase keeps producing is a capability that exists and cannot be found. file-size baseline +10 lines on the already-grandfathered hub route. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…mounted it
`requireRole` was declared `(c: Context, next: Next)`. Hono's bare `Context`
defaults its Env generic to `any`, and `@hono/zod-openapi` derives an
`.openapi()` handler's Env FROM the route's `middleware:` tuple
(`RouteMiddlewareParams<R>['env'] & E`). `any & HonoConfig` is `any`, so on the
332 routes carrying `requireRole` and the 22 carrying `requireCapability`, `c`
was `Context<any>`: `c.var.services.*`, `c.get(...)` and `c.env.*` were all
unchecked. `void c.var.services.ai.noSuchMethodAtAll()` compiled clean.
Two annotations fix it. A minimal repro isolated the cause: a plain `app.get`
handler and an `.openapi()` handler with no middleware were both already
checked; only `.openapi()` plus an `as const` middleware tuple whose entry has
an `any` Env collapses. `csrf.ts` had always used `MiddlewareHandler<HonoConfig>`,
which is why only these two guards were affected.
Turning the compiler back on surfaced 36 errors in 17 files. They are fixed
here, not suppressed — no `any`, no assertions, no `@ts-expect-error`. What
they turned out to be:
- `services.ts` declared `data?: { success: boolean }` for two discount-code
routes that return a list and an object. The schema is the published
OpenAPI contract and types `packages/api-types`, so the frontend had been
typed against a response the server never sends; `settings-services.tsx`
was already working around it with a cast.
- `automations.ts` create declared `channels?: ('email' | 'sms')[]`, a stale
hand-written pair that excludes `in_app` — a first-class channel the column
and the parser both support.
- `automation_logs.automation_id` is nullable and the null is load-bearing:
it is the manual-send marker `getCommunicationDeliveries` branches on. The
response schema declared it `z.string()`.
- `integrations.ts` reads four email-provider keys that were missing from
`AppEnv`. They are real: `secrets-catalog.ts` lists them and
`integration-secrets.ts` merges them into `c.env` in place.
- `availability.ts` re-derived the owner/manager test four times as
`![ROLE.MANAGER, ROLE.OWNER].includes(userRole)`. `isAdminRole` already
calls itself THE one definition and lists seven copies it absorbed; this
was an eighth, and it survived precisely because `.includes(any)` typechecks.
- `createReinspection` was declared `Promise<Inspection>` — a nine-field
response projection — held together by `as unknown as Inspection`, so
`created.reinspectionRound ?? 1` was a fallback nobody could check. The
round was correct at runtime; the type was one "fix" away from silently
reporting round 1 forever. It now returns the row it actually built.
- `/dashboard` fed `c.json` Date objects where the schema says ISO strings,
and four conditionally-spread keys. An optional property cannot satisfy
passthrough's `JSONValue` index signature — `JSONValue` cannot spell "may
be absent" — so those keys are now always present and null when they do not
apply, and declared in the schema instead of arriving undocumented.
- the rest are `exactOptionalPropertyTypes`: Zod `.optional()` yields
`T | undefined`, which a `k?: T` parameter refuses. Fixed on whichever side
was actually wrong, callee or call site.
`tests/unit/platform/route-middleware-env.spec-d.ts` stops it coming back. The
failure produced ZERO signal — no error, no lint warning, no red test; the
compiler simply stopped asking questions. Nothing in the gate ladder can notice
a check that quietly stopped happening, so the guard has to assert the type
itself. Restoring the old annotation turns exactly its three real assertions
red, and it carries a deliberately-broken control so it cannot pass vacuously.
`file-size-baseline.json`: `inspection-analytics.service.ts` 740 -> 745, five
lines of comment on an already-grandfathered file. `ItemEditor.tsx` tightens
661 -> 651.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…nspectorHub#73) "Recommended" was `absolute right-2 top-2` inside the Notes textarea's positioned wrapper. A textarea cannot indent only its first line, so the moment line one reached the right edge the control covered the inspector's own words — a browser screenshot has it printed over "at the eave. Granule". That was occasional until the AI "Improve wording" action started returning full paragraphs instead of short phrases; then it was every note. No CSS fixes this, because the collision is not a CSS problem. A control that inserts text into a field was being rendered as an ornament on the text. It now sits in the field's own header row, at the same rank as the NOTES label: [NOTES] [Recommended comments ▾] ················ [180 chars] Below the field was not free either — that space belongs to `AiAssistPanel`, whose draft slab needs the full width so the model's paragraph can be read against the inspector's own. Also here: - the trigger is hidden when the item has no canned comments. It used to render regardless and open an empty list — a dead control, and a more conspicuous one now that it looks like a real button. - the label gains `htmlFor`, which it never had, and the trigger carries `aria-haspopup="listbox"` / `aria-expanded`. - "Recommended ▾" becomes "Recommended comments", with the caret as a separate aria-hidden span, matching CloneLastButton. Reuses the editor's existing dropdown-trigger vocabulary (`Button` secondary/sm), no new primitive. Placement is pinned structurally by `item-editor-notes-toolbar.test.tsx`: restoring the absolute positioning turns 4 of its 8 specs red. Verified in Chrome, light and dark. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…rHub#67, part 1) `GET /api/inspections/:id/cancellation-quote` and `POST /:id/cancel` had ZERO callers in all of `app/`. The cancellation-policy panel configured a ladder that could not be climbed: the billing logic behind it was built, tested, and unreachable. The flow, on the hub's Lifecycle card — which already owns "Mark fieldwork complete" and already renders the cancelled terminal state, so cancelling is the other end of the same axis: 1. pick a reason (7 values from the server's own enum, imported so the list cannot drift). The reason decides the outcome, so it comes first. 2. the quote appears BEFORE the cancel is confirmable: fee kept, refund, collected so far, and a plain sentence when no policy is configured. 3. a confirmation that names what happens, including the fee, and says the inspection cannot be un-cancelled from that page. Showing the quote first is not presentation. The server refuses with 409 `CANCELLATION_FEE_NEEDS_CONFIRM` unless `acknowledgedFeeCents` equals the current fee, so an absent value means "no fee was shown to anybody". Quote on a loader, cancel on an action — deliberately not two POST intents, because behind a POST every reason change would be a mutation revalidating the whole hub. Reuses Modal / Button / Select / Textarea / ConfirmDialog (which gains an optional `cancelLabel`) and `formatCents`; no new primitives. Three defects the work surfaced, all fixed: - `Number(null) === 0` was forwarding a MISSING acknowledged fee as "the caller was shown a fee of zero" — inventing a claim about what a human read out of an empty field, against the exact value the 409 gate compares. - the 409's re-priced quote was being rendered as a second source of truth; it goes stale the instant the reason changes. The panel now has one origin. - a `setConfirming(false)` on failure re-ran on every parent revalidation and could shut the confirmation under the user. Verified in Chrome end to end on a scheduled inspection, both themes: quote, confirm, and the hub reflecting the cancelled state afterwards. NOT closed by this commit — see the follow-up tasks. The premise "no UI can cancel an inspection" turned out to be wrong in a way that matters: the inspections list has a per-row status dropdown offering "Cancelled", which PATCHes the status directly and skips the fee entirely. So the policy now has a front door AND a side door that does not charge. The agreement template editor (part 2) is also still absent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…torHub#65 step 1) `contractor_types` gains a PARTIAL unique index on `(tenant_id, trade_slug) WHERE trade_slug IS NOT NULL`. It has to come before the trade_slug backfill, and the reason is written in the service itself: `starter-content.service.ts` dedupes by slug in APPLICATION code, against a snapshot read, and its own comment says "There is no unique index on this table, so nothing downstream would object." A backfill that stamps a slug onto a legacy row for a trade the workspace already has produces two rows for one trade, and `defectTrade -> contractorType` then resolves to whichever the query returned first. The constraint makes that a failure instead of a silent duplicate. Partial, because NULL is the permanent and correct state for a tenant-created type with no canonical counterpart. `tests/unit/db/contractor-types-trade-slug-index.spec.ts` pins it, and the first draft of that spec was a false green worth recording. It asserted the `WHERE` predicate by inserting two NULL slugs and expecting them to be allowed. Deleting the predicate left all four cases GREEN: SQLite already treats NULLs as distinct in a unique index, so the predicate changes nothing about NULL behaviour — it keeps un-mapped rows out of the index and states the intent. The only way to pin it is to read the DDL back out of `sqlite_master`, which the spec now does. All four ways to get the index wrong — drop the predicate, drop `tenant_id`, drop UNIQUE, drop the index — each turn it red. The production backfill (step 2) is NOT in this commit and is not approved to run: it needs a duplicate count against prod first, and prod D1 needs an authenticated wrangler session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…lly fail (InspectorHub#58) Counsel's position is that eight categories inside the report and agreement are not content but a legal instrument, English-authoritative: reliance clause, limitation of liability, arbitration, warranty disclaimer, governing law, contract terms, signature, acknowledgements. This registry is a precondition for #23 (report courtesy translation) — translation is not released, so there is no runtime effect today; the point is that #23 cannot ship without it. Three pieces, following the erasure/retention shape already in the repo: a manifest (18 entries, every one carrying a non-empty reason, covering 8/8 categories), an out-of-scope list (5), and `scripts/check-non-translatable.mjs` wired into `lint`. The out-of-scope list records the boundary that is easy to get wrong: both platform notices are NOT part of this registry. They are neutral platform disclosures, counsel placed them outside §1632, and they are already live. The difference between the two modules is version integrity, not translatability. Sixteen break-and-restore probes, each proving a specific rule red before it was kept. Two of them came back GREEN on the first attempt, and both were real defects in the gate: - `arrayBody` located the declaration with `indexOf('export const NAME')`, which also matches the prefix of `NAME_V2`. Renaming the registry away left the gate happily parsing the renamed copy and reporting OK — the coarsest possible sabotage, invisible. - the locator check used `text.includes()`, so renaming `userReliance` to `userRelianceText` still matched. A third surfaced while writing the renamed fixture: the unanchored match landed inside a doc comment that quotes the declaration, so the gate parsed from mid-sentence and reported "zero entries" rather than "array missing".⚠️ `check-erasure-manifest.mjs` and `check-retention-manifest.mjs` were copied from the same shape and inherit the first and third of those. They are not touched here — each needs its own red proof — and the note in `arrayBody` names them. Neither zero-headroom file was touched: `inspection-report.service.ts` (957/957) and `public-report.ts` (566/566) are referenced BY PATH from the manifest, not imported. One import line would have broken `lint:filesize`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…sumed `lint:deadcode` flagged `non-translatable-manifest.ts` and `non-translatable-out-of-scope.ts` as unused files, and it is right: nothing imports them. That is the difference from their siblings — `erasure-manifest.ts` is live code that the gate ALSO parses, whereas this registry is parsed only as source text and has no runtime consumer until #23 (report courtesy translation) exists. Baselined rather than exempted in `knip.json`. A `knip.json` entry would be silent forever; a baseline entry that stops hitting prints a "stale baseline entry" notice, so when #23 gives the manifest a real consumer the exception announces that it is no longer needed. Found late because the previous full `npm run lint` short-circuited: it is an `&&` chain, the file-size gate failed partway through, and everything after it — deadcode, tenant-scope, i18n-glossary, naming — never ran at all. "The rest was green in the same run" was not true; the rest had not run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…w away (InspectorHub#77) `TOGGLEABLE` declares five staff capabilities. `InviteMemberSchema` and `UpdateMemberSchema` hand-listed four. Zod strips unknown keys, so `viewCommunication` was not rejected with a 400 — it was discarded without a sound: capability TOGGLEABLE Invite Update financial true true true manageContacts true true true publish true true true scheduleOthers true true true viewCommunication true false false The user-visible half is worse than "the capability is ungrantable". The checkbox ALREADY EXISTED — `InviteSeatDrawer` and `EditMemberDrawer` both render `TOGGLEABLE.map(...)`, so they were derived and correct. An owner could tick "View sent messages & notices", press Save, get a success toast, and nothing happened. The capability gates the per-inspection Outbox, recipient addresses included. No second write path: the only two request-bearing routes are `team.ts:245` and `:272`, both behind those schemas. `TeamService` already iterated `TOGGLEABLE`, so the service was never the blocker. `role-profiles.ts` is a different axis (contact role profiles), not staff bits. Why every test missed it: `update-member.spec.ts` calls `TeamService.updateMember` DIRECTLY, bypassing the schema, and `capabilities.spec.ts` tests `getCapabilities`. Both sides of the strip were covered. The strip was not. Fixed by deriving the shape from `TOGGLEABLE` via a `capabilityToggleMap()` factory — a hand-written list is exactly how the fifth went missing, and a sixth would go the same way. It now backs all THREE lists, including `TeamMembersResponseSchema`, which happened to be complete but carried the same hazard. `capability-schema-parity.spec.ts` asserts the accepted key set EQUALS `TOGGLEABLE`, written as parse RESULTS rather than shape introspection — stripping is what harmed users, and a schema that mentions a key it then drops would pass an introspection test. Replacing the derivation with `TOGGLEABLE.filter(c => c !== 'viewCommunication')` turns three of the four red; the fourth ("still strips a capability the server never declared") correctly stays green because it asserts the opposite direction. Verified in the browser: untick, save, hard reload, re-open — the box stays unticked. Before this it came back ticked. Left alone: `server/api/auth/profile.ts:116-122` still hand-lists the five in the `/me` response schema, each with a bespoke description. It is complete today and zod-openapi does not validate responses, so a sixth capability would be a documentation gap rather than a functional one; deriving it would cost those descriptions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…o longer existed (InspectorHub#75) `check-erasure-manifest.mjs` and `check-retention-manifest.mjs` located their registry with `text.indexOf('export const NAME')`, which also matches the prefix of `NAME_V2`. Both defects were reproduced on each file before being fixed — observing the bug is the evidence it is really present here and not only in the sibling gate it was first found in. 1. Rename `ERASURE_MANIFEST` to `ERASURE_MANIFEST_V2`: `OK (56 rules, 113 out-of-scope declarations)`, exit 0. A clean bill of health for an array that no longer exists under the name it is checked by. Retention: `OK (6 rules, 7 out-of-scope, 2 open; …)`, exit 0. 2. Leave the array intact and add a doc comment QUOTING the declaration: `parsed ZERO rules — parser drift or empty manifest`, exit 1. Fifty-six intact rules accused of being empty. Not a green, but a wrong answer with a misleading message — which is why the spec asserts `not.toContain('parsed ZERO rules')` and not merely a non-zero exit. On the combined tree (renamed AND quoted), each parser variant lands here: raw indexOf -> inside the comment lookahead only -> inside the comment ^ + lookahead -> not found -> "could not locate" so both halves of the fix are load-bearing; a lookahead alone turns case 2 from `OK` into `parsed ZERO rules`. Hence separate `renamed` and `renamed-quoted` fixtures — the two defects mask each other. Zero-entry guards were already present on the load-bearing array in both gates (proved live during the cycles above). The secondary arrays have none, and deliberately keep none: emptying each was checked rather than assumed, and all three go red indirectly through the coverage scan, because an entry only exists there because the scan flagged that column. For `RETENTION_OPEN` an empty array is the GOAL state — every table decided — so a hard zero-guard would fail the repo for succeeding. `manifest-gate-parsing.spec.ts` drives both gates through one `describe.each` battery as child processes, so the exit code is the contract. Every negative is paired with a control: the clean probe must be accepted with its exact counts, and the intact-but-quoted probe must stay GREEN, so a parser "hardened" by refusing any file that mentions its own array name cannot pass. A final block asserts all three sibling parsers carry the anchored form and none carries the bare `indexOf`. Reverting `arrayBody` in both gates turns 10 of the 20 red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…that can see it (InspectorHub#79) Six helper lines in shared-ui rendered at `text-[11px] text-ih-fg-4`. Measured against `--ih-bg-card`: token light dark field ih-fg-4 #94a3b8 2.56:1 ✗ #64748b 3.07:1 ✗ 6.96:1 ✓ ih-fg-3 #64748b 4.76:1 ✓ #94a3b8 5.71:1 ✓ 12.02:1 ✓ `FileDropzone.tsx:213` already used `ih-fg-3` — one of six right — and `RadioCardGroup` renders its option descriptions at `fg-3` eleven lines above its own `fg-4` hint. This was copy-paste, not a design position. Fixed in `Input`, `Select`, `Textarea`, `RadioGroup`, `RadioCardGroup`, and the file-size line in `FileDropzone` (three lines above the hint that was already correct). Errors and labels were checked and are clean (6.29 / 5.29 and 7.58 / 9.85). Two things the third token block revealed. `data-color-scheme="field"` shifts BOTH tokens a tier brighter, so field was never broken and does not regress. But field's promise of high-contrast large type never reached these lines at all: `text-[11px]` is an absolute value and does not scale with field's 18px root. `scripts/check-contrast.mjs` / `lint:contrast`, wired into `run-gates.mjs` (what the pre-commit hook actually invokes) as well as `lint`. It does the arithmetic rather than matching names: parses the `@theme` alias block, resolves each token through all three cascade blocks, and requires 4.5:1 in every theme for any class string pairing a <=14px size with an unprefixed `text-ih-*`. It exits 1 if it scanned zero colours, throws if the reference surface stops resolving, and fails on `KNOWN_DEBT` entries that no longer match live code. `lint:ds` stays green on the live defect — it validates token NAMES and cannot see contrast at all, which is why a second gate rather than an extra rule. The red run also caught a bug in the scanner itself: an apostrophe inside a prose comment ("the card's overflow") opened a phantom string literal and made it report the wrong line. That could have swallowed a real class string and never checked it. `stripComments()` now blanks comments while preserving line count. Reverting one file to `ih-fg-4` produces the failure with both ratios printed, `run-gates --only contrast` exit 1, and 2 of the 17 specs red — while `lint:ds` still exits 0. Verified in Chrome afterwards on the cancellation modal's Select hint: 2.56 -> 4.76 light, 3.07 -> 5.71 dark, same element. Scoped out, on purpose and with numbers rather than a shrug: `Table.tsx:33` (every table in the app, recorded in `KNOWN_DEBT` with a staleness guard), and `app/**`, where the same scan finds 579 of 2231 but 212 are false positives of the single-reference-surface assumption — honest coverage there needs per-element background inference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…t charge (InspectorHub#78) The `/inspections` list rendered a per-row status dropdown offering `cancelled`, which PATCHed the status directly. So the fee ladder shipped one commit ago had a front door AND a free entrance — worse than before, because the UI now looked like it had a gate. What each path actually did, which is the part nobody remembered: PATCH {status:'cancelled'} POST /cancel fee ladder / quote no yes acknowledgedFeeCents 409 no yes refund to ledger no yes QuickBooks credit memo no yes cancel_reason / cancel_notes no yes fireAutomation('inspection.cancelled') no yes Google Calendar entry removed YES no audit row YES no Those last two rows are why "just close the door" would have been a regression: it would have traded a billing hole for a cancelled inspection still sitting on the inspector's Monday, with no record of who cancelled it. Both now live on `/cancel`. A THIRD door existed: `PATCH /api/inspections/bulk` with `{action:'updateStatus', status:'cancelled'}` — same bypass, and an MCP `extended`/`write` tool, so an agent could reach it. The cancel route's own comment had predicted this ("stops the next surface — a bulk action, an MCP tool, a mobile client"); two of the three were real. Closed too. Deliberately still open: `POST /api/admin/data/import` inserts historical rows with `status:'cancelled'`. That is a create of past records, not a lifecycle transition, and `CreateInspectionSchema` has no `status` field so the normal create endpoints cannot produce one. Recovery survives — refusing only `cancelled` leaves `cancelled -> scheduled` working — but it was quietly incomplete: the PATCH never cleared `cancel_reason`/`cancel_notes`. Harmless while the dropdown was ALSO the canceller (it never wrote a reason); once `/cancel` is the only door, every un-cancel would have inherited a stale "no_show". `clearCancellationOnRecovery` now clears both. The dropdown keeps a DISABLED `cancelled` option on an already-cancelled row, without which the select matches nothing and a cancelled inspection renders as "Requested". Fixture migration was one file: only `tests/e2e/inspection-lifecycle.spec.ts` cancelled through the PATCH; the other 14 cancelled-inspection fixtures write the row directly and are unaffected. `cancel-single-write-path.spec.ts`: against the permissive handler, 4 of its 7 fail — both refusals, the whole-patch case, and `expected 'no_show' to be null` — while the three positive controls (a legitimate transition, a bulk move to `confirmed`, an unrelated patch not clearing the reason) pass throughout. `DashboardInspectionRow.test.tsx` +3, proved red by restoring the old option. The refusal is 400 `USE_CANCEL_ENDPOINT`, not 409: the file-size ratchet left one line of headroom in `core.ts`, so the rule folded into the already-called `findPatchRefusal`, whose caller returns 400 — which is also what both routes already return for a body they will not apply. Baseline: `core.ts` 544 -> 543, a tightening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…ad no front door (InspectorHub#67 part 2) `POST`, `PUT` and `DELETE /api/admin/agreements` all had ZERO callers in `app/`. Three complete, role-gated, sanitising write endpoints, and no way to reach any of them: `agreements.tsx:55` was a bare `<Button variant="primary">` with no onClick, and the per-row "Edit" had none either. A workspace could read the agreement it was seeded with and could not change a word of it. WHAT WAS NOT BUILT, and why. There are no merge fields. The starter template's `[INSPECTOR_NAME]`, `[PROPERTY_ADDRESS]` and friends appear in exactly one file — the fixture — and nothing substitutes them; they are literal prose the tenant is meant to overwrite. (`interpolate.ts`'s `{{...}}` is the automation/email pipeline and never touches agreement bodies.) A toolbar offering placeholders nothing fills would be its own kind of lie. NO DEPENDENCY ADDED. There was no rich-text editor in the repo to reuse — the grep for Quill/TipTap/ProseMirror/Slate/Lexical is empty, and `CommentEditor`, `TemplateEditorModal` and `RatingSystemEditor` are all plain textareas. Both sanitisers' comments claim their allow-list "mirrors the Quill editor toolbar"; there has never been a Quill here. So `AgreementRichText` is a contenteditable region whose toolbar is exactly the eight formats the allow-list keeps. THE SECURITY DESIGN IS A FIXED POINT, NOT A BLOCKLIST. `agreement-markup.ts` converges input onto the intersection of the write-time and render-time allow-lists, and the spec pins `sanitizeAgreementHtml(normalize(x)) === normalize(x)` for every case. No attribute survives at all — not even `class` — so "no href, no src, no on*" is a property of the serialiser rather than a list somebody has to keep complete. Both allow-lists are read out of the other two files' source so a copy cannot drift. Untrusted markup is parsed into an inert `<template>` fragment, never into a live element. That last detail was not theoretical: the first implementation used a live document, and happy-dom does not emulate `createHTMLDocument` inertness, so the test suite issued a REAL NETWORK REQUEST to the hostile fixture's host. Confirmed fixed by the requests disappearing. Red-then-green, per guard: neutering `normalizeAgreementHtml` turns 9 red; the route's session/id/empty-body checks, 7; paste normalisation, 2; `emit()` normalisation, 1. Removing BOTH normalisation passes turns 5 red including the hostile-markup and fixed-point assertions — and that is itself the finding: either pass alone left the emitted value green, so the suite asserts hostile markup dead in the DOM the author sees AND in the value handed to the caller. The hostile fixture covers `<a href>`, `<img onerror>`, `<svg onload>`, `<p onclick>`, `<script>`, `<iframe>`, `<span style>` and a comment-wrapped script, asserts every emitted tag matches `^<[a-z0-9]+>$`, and requires the result to pass the real server sanitiser byte-identical. 74 tests across 6 spec files. Two rendering defects found on the way, both on pages a client signs: - `prose prose-sm` matched NO CSS rule. `@tailwindcss/typography` is not installed, so under preflight the agreement's `<h2>`/`<h3>` rendered at body size and `<ul>`/`<ol>` had no marker and no indent — on the signing page and on `/verify`, which shows the exact signed snapshot. Both sanitisers were carefully permitting tags the page could not display. One shared `.ih-agreement-prose` now backs the editor and all three render surfaces. - every seeded agreement is plain text rendered through `innerHTML`, and the sanitiser returns anything without a `<` verbatim, so the starter template collapsed into one wall of text with its `##` and `**` showing literally. The editor paragraphs it on load. No markdown parser was added. Also recorded, not fixed: `agreements` has no `updated_at` column, yet the list route and `TemplateRow` both read `updatedAt` — "Last updated" has always shown the created date. Documented at the read site. 🔴 FOLLOW-UP, TRACKED SEPARATELY AND NOT IN THIS COMMIT: this editor makes a dormant path live. `getCancellationAttestation` invalidates the cancellation-fee attestation whenever `agreements.version` changes, and until now nothing in the product could bump that version. From here on, a tenant who edits their agreement silently loses the ability to charge cancellation fees. The mechanism is right; the missing piece is telling the author. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…irmation (InspectorHub#83) The editor that landed in 66a62f7 made a dormant path live. `getCancellationAttestation` revokes the cancellation-fee attestation whenever `agreements.version` changes, and `updateBranding` then refuses a fee-charging policy. Nothing in the product could bump that version before, so it had never fired. From that commit on, a tenant who edited a word of their agreement lost fee-charging with no notice anywhere. Measured rather than reasoned — a throwaway probe drove the real services and printed both numbers on one line: after attest attestedVersion=1 currentVersion=1 LIVE after IDENTICAL-content re-save attestedVersion=1 currentVersion=2 null fee-charging policy save -> REFUSED Sharper than the report: re-saving byte-identical name and content still revokes, because `updateAgreement` compares nothing and bumps unconditionally (`template.ts:57`). That bump is the correct behaviour and is NOT changed here — the attestation means "somebody confirmed THIS text", so a save has to invalidate it. What was missing is telling the author. Two facts made a scoped warning possible instead of a blanket one. The attestation names a SPECIFIC template (`tenant_configs .cancellation_clause_agreement_id` + `_version`, re-read filtered by that id), so the banner appears only when the attested template is the one being edited — and the "editing a different template must not warn" control is asserted, and passed both before and after, which is the point of having it. And the re-confirmation destination already exists: Settings → Online Booking → Cancellation policy has a `drifted` state built for exactly this, with the agreement picker and "Confirm again". That is now pinned by a test proved non-vacuous — deleting the picker turns it red — so the banner cannot end up naming a destination that has quietly gone away. `clauseAttested` comes from the server-computed field the fee gate itself reads, never recomputed in the loader. A failed branding read **fails the load closed** rather than opening the editor with `clauseAttested: false`, which would be the silent revocation all over again. The banner informs and does not block: no extra tick-box, and the save path is byte-identical to any other template's. This is a review surface, not a consent one. Two gaps left open on purpose, tracked separately: - `PUT /agreements/{id}` is also MCP tool `updateTenantAgreement` (`admin`/`extended`), which revokes with no banner anywhere. The honest fix is server-side — the response saying what it revoked — not a second UI. - Deleting the attested template revokes too, and the delete confirmation does not say so. Same loader flag would feed it; left out to keep this scoped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…nspectorHub#63 phase 0) 753 files / ~101k lines were compiled by exactly zero tsc programs. `tsconfig.json` excludes `app/**/*.test.*` and never mentioned `tests/`; `tsconfig.api.json`'s include is `server/**`; the playwright config is scoped to `tests/e2e`; `tsconfig.tests-typecheck.json` takes only the four `*.spec-d.ts`. vitest strips types with esbuild and never checks them, so a spec could assert against a shape the code stopped having months ago and stay green. `tsconfig.tests.json` REFERENCES `tsconfig.api.json` rather than joining it. Joining is the obvious move and the expensive one: api is composite and a project reference of the app program, so `tsc -b tsconfig.json` builds it FIRST — 753 extra root files would then be paid for by every pure-frontend commit and by CI's typecheck-app. Referencing costs the app program nothing. EXCLUDE-RATCHET, NOT INCLUDE-RATCHET. `include` covers the whole tree from day one; the not-yet-clean files sit in `exclude`. An include whitelist would let a new spec in a new domain silently escape coverage — the exact bug being fixed. 198 excluded of 785; **587 files type-checked from today**. The exclude list is measured, not guessed. Rather than estimate by reading, the whole tree was swept in eight bounded 100-file tranches (7-9s each), then the list was validated by the first single-process run: 587 files, 15.7s, **ZERO errors** — the tranche union converged exactly.⚠️ Two things that cost a round each and are now written down where they bite: - **`exclude` is not suppression.** An excluded file still enters the program if an INCLUDED file imports it. Two dirty helpers under `tests/unit/helpers/` were unaffected by exclusion; the four otherwise-clean specs importing them had to go too. - **`references` is not inherited through `extends`.** A probe that extended this config without re-declaring it silently lost the redirect: 790 server SOURCE files entered the program and it OOM'd at 2048 MB. `tsconfig.playwright.json`'s header already said so. Also fixed, and not guessable by reading: `@cloudflare/vitest-pool-workers` must be listed as the `/types` SUBPATH. The bare name — which `tsconfig.api.json` and `tsconfig.tests-typecheck.json` both use — resolves to `defineWorkersConfig` and leaves all 18 `cloudflare:test` imports as TS2307. Deliberately NOT mirroring three of `vitest.api.config.ts`'s four aliases: zero files under `tests/` import `cloudflare:workers`, `@cloudflare/workers-oauth-provider` or `agents/mcp`. Only the transitive `server/index.ts` graph does, and it arrives as pre-built `.d.ts` already checked against the real types. Aliasing would redirect those onto 22-line runtime shims and produce errors in generated files with no source to fix. `check-tests-tsconfig.mjs` / `lint:tests-tsconfig` keeps the list shrinking only. Four controls, each observed red before being kept: a list that grew, a shrink not recorded in the baseline, a stale entry matching no file on disk, and the spec-level pair going out of sync. Each detection case is paired with a negative control, so a gate that always cried violation would fail its own suite. HEAP: 1024 MB, measured by bisection — 512 OOMs, 768 passes. Tight on purpose, for the `references` reason above: at 1024 that mistake dies in seconds; at 4096 it might merely get slower and pass. CI runs it inside the EXISTING typecheck-app job, where `tsc -b` has already built api. Corrections to the audit's numbers, for whoever plans the batches: OI has 712 `as any` across 219 files (not 746/218), 753 files in `tests/unit` (not 728) and 329 co-located (not 319). Still uncovered and deliberately out of phase 0: the 329 co-located `app/**/*.test.tsx` and 19 shared-ui specs. They belong to the APP program — the fix is deleting one exclude line — and that line sits on the hot pre-commit and CI path, so it needs a measured cold run first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…phase 0) Separate commit because staging `package.json` escalates the pre-commit type-check from the api tier (~11s) to the FULL tier, and `vitest --changed` degrades to the whole suite when it appears in the diff — one line turning a 3s run into 3975 tests. Splitting removes both at once. Adds `type-check:tests` and `lint:tests-tsconfig`, into `type-check`, `lint` and `lint:gates-full`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…lution (InspectorHub#88) Reproducing command, red 2/2 before and green 2/2 after: npx vitest run app/routes --config vitest.config.ts --maxWorkers=16 Pre-fix: `5018ms` then `5544ms`, both `Error: Test timed out in 5000ms.` No seed and no `--sequence.*` flag is involved — the variable is transform contention, not order. MY HYPOTHESIS WAS WRONG AND THE MECHANISM RULES IT OUT. `vitest.config.ts` uses the default `pool: forks, isolate: true`, so module state cannot cross files; cross-test pollution was never possible here. What actually happens: 1. the spec reaches its subject via `await import(...)` INSIDE the first `it()`, so the test body pays for that module's whole import graph; 2. the graph reaches `~/paraglide/messages`, whose generated `_index.js` is 3.67 MB (re-exporting en 1.27 MB + es-419 1.30 MB) and which 441 source files import — transformed on the single main thread every worker shares, so the cost is queueing, not compute; 3. that wait is billed against the 5000 ms default `testTimeout`. Solo it took 3640 ms; under 59 files 4628 ms; at 16 workers 5018 ms — over the line; 4. `afterEach(vi.resetModules)` made all five tests re-walk the graph. Which is exactly why it looked intermittent: whichever spec demands the transform first while workers are starting draws the short straw, and across runs the victim MOVED — this file, `settings-automations.test.ts`, and `agent/settings-profile.test.tsx` in turn. Fix is confined to this spec: module-scoped mocks, both route modules imported once in `beforeAll` where loading a fixture belongs, mocks re-armed per test, `afterEach`/`resetModules` gone, and a stub for `~/paraglide/messages` — no action path here reads a message. First test 3640 ms → 6 ms solo, 5018 ms (timeout) → 13 ms under the reproducing command; whole file 4.96 s → 1.45 s. Positive control: flipping the two expected values to `CONTROL_WRONG` turns exactly those two tests red, so the rewritten mocks are wired and the actions really execute rather than the file merely getting faster. Ruled out rather than assumed: the two `app/routes/public/**` sweeps earlier today are irrelevant — a className rename cannot change a `toHaveBeenCalledWith` on a request body, and this spec renders no JSX. Unrestored global `fetch` is excluded because `tests/setup-web.ts`'s hermetic guard throws a distinct message, not a timeout; singletons and happy-dom bleed are excluded by `isolate: true` plus the victim being the FIRST test in its file. Five other specs share the shape (heavy import inside a timed `it()`) and one of them was watched timing out at default worker count; they are tracked separately rather than fixed blind. A class-level `deps.optimizer.web` fix was tried, appeared to cut transform time 20.8 s → 5.5 s, turned out to be cold-cache noise with no bundle ever produced, and was reverted — `vitest.config.ts` is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…st that proved nothing (InspectorHub#63 phase 1, batch 2) Exclude list 160 → 105. Ten whole domains: agents, bookings, calendar, concierge, idempotency, notifications, people, qbo, reports, usage. `tests/unit/inspections/**` was left alone — another agent was moving a route through two of its specs. Suppression counts, all three falling, measured over `tests/unit` before and after: as any 703 → 702 @ts-expect-error 0 → 0 as unknown as 583 → 582 (My brief said 584 for the last one; 583 was what measurement showed at the start, so the delta here is against what was actually there.) Unlike batch 1, no spec was excluded merely because a shared helper it imports was dirty: all 132 non-`inspections` entries were opened in one program first and every one errored on its own, so domains could be picked freely. 🔴 THE ANTI-LEAK TEST IN `agents/agent-service-listings.spec.ts` WAS VACUOUS. It seeded `clientName: 'LEAKED-PII-SHOULD-NOT-APPEAR'` and asserted the read did not fall back to it. `inspections.clientName` was DROPPED ("superseded by inspection_people — do not reintroduce"), drizzle silently discards unknown keys, so the sentinel could never appear and `.not.toBe('LEAKED-PII…')` was asserting against a value that had no way of existing. A test with PII in its name, proving the empty set. The dead seed and dead assertion are gone and the title now states what still holds — post-erasure, a buyer_agent row with no client row must surface a NULL client, which the surviving `expect(erased?.clientName).toBeNull()` actually checks. Five more specs where the TEST was wrong, each changed only where the intent was unambiguous and the evidence is recorded: - six specs seeded the same four dropped columns — dead writes, keys removed, every one already seeded the real `inspection_people` rows alongside; - `concierge-schema` used `status: 'pending'`, not in `INSPECTION_STATUSES`; the row is FK scaffolding nothing reads → `'requested'`, the default; - `booking-deposit-intent-replay` used `'in_progress'`, a REPORT status, on the inspection axis. `deposit-intent.ts:85` admits `REQUESTED` only, so the branch under test is "already past requested" → `'confirmed'`, retitled; - `booking-toctou` defaulted to `status: 'draft' as any` — `'draft'` never existed and the cast was hiding it. `fulfill-booking.ts:151` writes `REQUESTED`; the param is now typed `InspectionStatus`. That is the one `as any` removed; - three `qbo` specs had eight `protected override`s narrowing members that are public on the base — widened, behaviour-identical. Two new shared helpers rather than casts: `helpers/email-provider.ts`, whose webhook members THROW instead of returning a plausible `[]` so a spec that grows an inbound dependency fails loudly, and `helpers/report-photos.ts`. Left alone deliberately: the ten flagged entries (roles that cannot exist, ratings outside the enum, config fields that do not exist) stay excluded; `report-photos.ts` under-declares its return type and `ScopedDB.getById` is `any` on both sides — both are `server/` defects worked around test-side with a note, not edited to make a test compile. Heap re-bisected cold with the larger program (680 spec files): 1024 still OOMs, 1280 passes, 1536 passes in 14.5s — the packaged cap is still right, so `package.json` is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…torHub#89, first of five) `settings-automations.test.ts` was the file InspectorHub#88 named as the highest residual risk and could not make fail on demand in four attempts, so it was left alone under the "no reproduction, no fix" rule. Running InspectorHub#88's own reproducing command after its fix landed produced it immediately: npx vitest run app/routes --config vitest.config.ts --maxWorkers=16 FAIL settings-automations.test.ts > every recipient kind … has a label Error: Test timed out in 5000ms. Same mechanism, second independent victim: `await import('./settings-automations')` sat inside the first `it()`, so that test body paid for an import graph reaching `~/paraglide/messages` (~3.7 MB, transformed on the one main thread all workers share) against the 5000 ms `testTimeout`. Note the full `npm run test:web` was GREEN on the same tree — the suite passing is not evidence the exposure is gone, which is exactly why the amplifying command is worth keeping written down. Hoisted into `beforeAll`, which has no timeout budget and is where loading a fixture belongs. Both tests are now synchronous. First test 2774 ms → 5 ms. Verified with the reproducing command 2/2 green (59 files / 394 tests), plus `app/components` under the same flags (178 / 1011). Positive control: changing the expected value to `kind + '_CONTROL_WRONG'` turns exactly that test red, so the module really is loaded and compared rather than the file merely having got faster. Four files with the same shape remain, tracked in InspectorHub#89. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…roblem (InspectorHub#89, InspectorHub#90) InspectorHub#88 listed five specs sharing the "heavy `await import()` inside a timed `it()`" shape. That list was matched on SHAPE, and shape is not the thing that costs. Measured, with a throwaway probe timing a bare dynamic import from INSIDE the amplifier run: await import('./repair-builder.$tenant.$id') 1877 ms, then 5000 ms timeout on 3 of 3 runs await import('../RepairBuilderSection') 3702 ms await import('…/report-card-stack?raw') 47 ms await import('…/ReportView?raw') 42 ms await import('./useRepairOpQueue') 63 ms `?raw` returns the file's TEXT — Vite never walks the import graph, so there is nothing for the main-thread transform queue to bill. Same shape, no cost. Three of the four remaining files are `?raw` or load one small module, and are left untouched. The one real case, `repair-builder-trade-seam.test.tsx`, is hoisted into `beforeAll`. Its numbers looked mild because it was SHIELDED BY ACCIDENT — its static `import { RepairBuilderSection }` already drags the paraglide graph in during collection, so the dynamic import paid only the residual. That shield is exactly one test deep and would vanish the moment the render test above it moved. Hoisted anyway, to make it structural rather than incidental. Messages are NOT stubbed here — test 1 renders a component whose visible copy IS messages. Positive control: both expected trades flipped to `CONTROL-NOT-A-TRADE` turn both tests red, each printing the real observed value. Correcting my own brief: none of the four had `afterEach(vi.resetModules)`. InspectorHub#90 — `vitest.config.ts`'s `deps.optimizer.web` block is deleted along with its claim. Dead twice over, both source-backed: `web` is not a key Vitest 4 reads (the environments are `client` / `ssr`; `web` was the ≤3 name, and the type's `({} & string)` member means it type-checks and is then read by nobody), and an empty `include` disables the optimizer regardless via `noDiscovery && !include?.length`. Naming a real module under the CORRECT key does produce an artifact — and it is still never loaded. Prepending `throw new Error("POISON…")` to the emitted bundle left 12 tests passing with the file still poisoned: `vite:resolve-dev` substitutes optimized deps only for BARE specifiers, and `~/paraglide/messages` is alias-rewritten to an absolute path first. The optimizer cannot reach it. For testing-library — what the deleted comment actually named — the block's own `exclude: ['react','react-dom']` makes it hard-fail on `react-dom/test-utils`. The timings are the argument for the replacement comment: 3.35 / 12.23 / 7.78 / 2.45 s across variants differing only in dead config, and the fastest run was the one with the poisoned, unloadable bundle. The new comment tells the next reader to prove any future entry with an artifact under `node_modules/.vite/vitest/<hash>/deps*`, not with a stopwatch. Verified: `app/routes` 59 files / 394 tests and `app/components` 178 / 1011, both under `--maxWorkers=16`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…ests that could not fail (InspectorHub#63 phase 1, batch 3) 105 → 3. With batches 1 and 2 that is 198 → 3, and 782 spec files now type-check. The up-front check first, because it decides whether domains can be picked freely: the ratchet was emptied to its structural four and the whole tree compiled in one program. **Exactly 105 files errored, and they were exactly the 105 entries** — nothing was listed while clean, nothing unlisted errored. 473 diagnostics. Suppression counts, measured the same way batch 2 measured them: as any 702 → 697 @ts-expect-error 0 → 0 as unknown as 582 → 582 Down five, up none. The new `helpers/fetch-mock.ts` clears ~110 diagnostics by DECLARING the mock's parameters so `vi.fn` infers `[string, RequestInit]`, rather than asserting the shape. 🔴 TWO MORE ANTI-PII-LEAK ASSERTIONS THAT COULD NOT FAIL — batch 2 found the first, this makes three: - `email/transactional-message-notification.spec.ts` seeded `STALE-LEGACY-NAME` and `leaked-pii@example.com` into DROPPED `inspections` columns across four cases and asserted they never reached a recipient. Drizzle discards unknown `.values()` keys, so the sentinels never existed and `.not.toContain` was asserting against the empty set. The positive halves are kept — recipient and `fromName` come from `inspection_people`, and after erasure no client email is addressed at all — and the header now says what is actually provable. - `inspections/inspector-portal.spec.ts` seeded divergent decoy values plus two decoy contact rows and made six `.not.toBe(sentinel)` assertions on them. All vacuous; the `toMatchObject` above them was doing the work. 86 dead seed keys across 17 specs wrote to columns `schema/inspection/core.ts:26-29,37` documents as dropped. **The vitest requirement earned itself.** `client-portal/portal-access.spec.ts` compiled fine, and running it revealed a real gap: giving the fixture the `id` its type demands exposed a FOURTH key, `accessTokenId` (`server/lib/public-access.ts:66`). The case titled "returns {tenantId, role, recipientEmail}" had been checking three of four — the fourth read `undefined` because the fixture had no `id`, and `toEqual` skips undefined-valued keys. Now asserted. CORRECTING AN EARLIER FINDING I PROPAGATED TWICE: `holidayInternalPolicy` and `holidayPublicPolicy` BOTH exist on `TenantHolidayConfig`. Batch 1 reported neither did and I repeated it into two briefs. The real shape is that each function takes a `Pick` of only the one it reads, so the other was an excess property the callee could never consult — a different defect with a different fix. Roles, severities and stale fixtures fixed with the source line that pins each intent: `'admin'`/`'member'` are not in `ROLES` (`auth/roles.ts:7`); `'defect'` and `'monitor'` are not in the severity vocabulary (`schema/inspection/comments.ts:29`) and the product's own catalogue maps both (`data/recommendation-seeds.ts:44,50`); `AdminService`'s second parameter is `IntegrationProvider` and `getMembers` never reads it; `QBOServiceBase.apiCall` is public, which is what made five `spyOn(svc as never, …)` casts necessary. THREE ENTRIES REMAIN EXCLUDED, all because the SOURCE type is wrong, each documented inline in the exclude block rather than papered over: - `createInspection`'s parameter is the PARSED-REQUEST shape, where `templateId` and `clientName` are required, while the body only does `if (data.templateId)` — production's own internal caller routes around it with `as unknown as CreateInspectionData`. Two specs legitimately create inspections with neither; a `templateId: ''` fixture would be a lie that compiles. - `updatePropertyFacts`'s facade type omits `unit`, `county` and `metadata`, which five cases exercise and which pass at runtime because the facade spreads the object through. Cold heap re-bisected with the program at 782 files (was 680): 1024 OOMs, 1280 passes, 1536 passes. **The floor did not move** — `package.json` unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…pectorHub#86) Tenants set a brand colour, and `brandTokens()` re-points `--ih-primary` at it on every public surface. `contrastForeground()` then chose the text sitting ON that colour using YIQ perceived brightness with a threshold of 150 — an NTSC luma formula, which answers a different question than AA asks. Across 636,056 sampled sRGB colours: text ON the brand button fails AA 28.5% contrastForeground, wrong formula brand colour AS link text fails AA 63.6% nothing guarded it at all `#00ff00` has a YIQ of 149.685 — 0.3 under the threshold — so it took white text at 1.37:1. THE SPLIT. `--ih-primary` stays the tenant's exact colour and remains the FILL, so buttons are unchanged. `--ih-primary-text` is new: the same hue and saturation with only LIGHTNESS moved, binary-searched for the value closest to the original that still clears 4.5:1 against `--ih-bg-card`, then stepped on the 8-bit grid because the continuous answer can round back under. Direction comes from the surface's luminance — on white you can only gain by going darker, on `#1e293b` only by going lighter — and an endpoint always exists, so it always terminates on a passing colour. A colour that already clears is returned untouched, which is why the platform default has `--ih-primary-text == --ih-primary`. Hue and saturation surviving exactly is asserted, because that is what a reader recognises as the brand; nobody's brand is the hex of a hyperlink. Delivered as `light-dark(lightVal, darkVal)` in the inline style: an inline style cannot carry a media query, but `light-dark()` resolves against the element's computed `color-scheme`, which the stylesheet already sets per theme — one declaration covers light, dark, field and a mid-session switch, with no call-site changes. `#00ff00` becomes `light-dark(#008a00, #00ff00)`. `contrastForeground()` now compares real ratios. That takes the on-fill failures from 28.5% to **6.74%**, and the property test asserts the remainder is exactly the unwinnable band — colours where BOTH white and the dark token fall short (worst 4.21 at `#9f66ae`). Those need the FILL moved, which this design deliberately does not do; the honest place to raise it is the colour picker (tracked separately). Defaults fixed: light `--ih-primary` `#6366f1` → `#6265f0` (4.4669 → 4.5268, 1/255 per channel, imperceptible), and light `--ih-status-bad` `#ef4444` → `#dc2626` (white on it 3.7631 → 4.8294).⚠️ "Change light only" was my instruction and it was wrong. `--ih-status-bad` is declared once in `:root` and INHERITED by both dark themes, where `--ih-fg-inverse` is near-black rather than white — so darkening it regressed dark and field from 4.74 to 3.70. It is now pinned back to `#ef4444` in the dark block. The gate caught that on its first run after the edit, not reasoning. 251 occurrences across 153 files migrate from `text-ih-primary` to `text-ih-primary-text`. The rule: it migrates when it colours glyphs a reader reads on a surface the brand does not paint; it stays when the brand is the fill of the thing being coloured. In this tree that second case is exactly 27 native checkbox/radio accents. The "icon inside a filled button" case does not exist here — nothing pairs `bg-ih-primary` with `text-ih-primary`, which would be invisible. Red-then-green: the property test run against the CURRENT YIQ implementation reproduced 28.47% / worst 1.372 @ `#00ff00` before anything changed — that is what makes the passing run afterwards mean something. Neutralising the derivation reproduced 63.62%. Reverting `--ih-primary-text` to `#6366f1` trips the new token invariant; deleting its `@theme` alias trips `lint:ds` rule 9 with 256 violations. `PALETTE_DEBT` is now empty. Also here: the contrast gate's own POSITIVE CONTROL had to move. It used `--ih-primary` at 4.47:1 as its known-failing fixture — the thing proving the exemption machinery is not a blindfold — and this change paid that debt, so the control read zero while still passing. It now builds a synthetic record over `--ih-fg-inverse` on `--ih-status-watch` (2.15:1 light, clears elsewhere: the same light-only shape), so the test is about the MACHINERY rather than about whatever the real debt list happens to hold. A control that survives its own subject disappearing is not a control. WHAT REMAINS UNCOVERED, stated in the gate's header rather than left implied: a tenant's brand colour lives in D1 and arrives as an inline style at request time. It never appears in `tailwind.css`, so no static gate sees it. The 63.6% and the 6.7% are held only by `brand.ts` under the sRGB-cube property test. And the derivation targets `--ih-bg-card`, so a brand link directly on `--ih-bg-app` measures ~4.33 in the worst case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…adable text (InspectorHub#91) The token split in `b76b1771` made brand-coloured TEXT safe by derivation, and moved on-fill failures from 28.5% to 6.74% by choosing the foreground on measured ratio instead of YIQ. What it could not fix is the remaining band: mid-tone colours where BOTH white and the near-black token fall short of 4.5:1 (worst 4.21 at `#9f66ae`). The fill is the tenant's brand and the design deliberately does not move it — so the only honest place to raise this is the moment they pick it. WARN, DO NOT SUBSTITUTE, DO NOT REFUSE THE SAVE. Silently nudging their colour would be changing someone's brand behind their back; blocking the save would be deciding for them. The control reviews; it does not accept on their behalf. The message carries the number the button will actually achieve and the number AA asks for, because a vague "low contrast" gets dismissed and a measured one gets considered. It also says what is NOT damaged — brand-coloured text and links are derived and stay readable; it is the filled controls that suffer — and that the colour is saved exactly as picked. Overstating the blast radius gets a warning ignored as fast as understating it. Both numbers come from `fillContrast`, the same function `contrastForeground` uses to choose the button's real text colour. A second implementation could disagree with the first and neither side would know. Live AND persistent, and the reason is not implementation convenience: the component is a pure function of the colour in the picker, which the page seeds from the stored value, so it is there while choosing and still there on every later visit. The person who has to answer "why are our buttons hard to read" is usually not the person who picked the colour, and rarely on the day they picked it. Red-then-green, with the two mutations kept separate because they discriminate: - always return null → the two "warns, with the measurement" tests go red and the silence test stays green; - render for every colour → only the silence test goes red. That second control is the one that matters. A component that always renders satisfies "it warns" perfectly while being decoration people learn to scroll past. The spec also pins that the two fixtures really are one failing and one passing colour, so the pair cannot quietly collapse into the same case if the palette maths moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…orHub#93) Its comment claimed pre-bundling was "cached in node_modules/.vite keyed on the dependency set — so the cost is paid once, not once per file per run." Nothing was ever cached. The cache dir, located from source rather than assumed (`VitestCache.resolveCacheDir`, label unset so `sha1("")`), held only `results.json` — no `deps_ssr/`, ever. But this DIVERGES from its sibling in `vitest.config.ts`, which was deleted in d85313c, and the divergence is the finding. There, the key was `web` — a name Vitest 4 never reads. Here the key is `ssr`, which is genuinely read: `resolveOptimizerConfig` keys off the Vite environment name and `environment: 'node'` IS the ssr environment. So only the second defect applied — and it is a hard one. `resolveOptimizerConfig` sets `noDiscovery: true` on every optimizer environment UNCONDITIONALLY, and Vite disables optimization when `noDiscovery && !include?.length`. `enabled: true` can therefore never bootstrap itself; an explicit `include` is the only way in. With `include: ['drizzle-orm', 'drizzle-orm/sqlite-core']` an artifact appears — and unlike the sibling it is actually LOADED. Proved by poisoning it: Error: POISON-93-deps_ssr-artifact-was-loaded ❯ node_modules/.vite/vitest/da39a3ee…/deps_ssr/drizzle-orm.js:1:7 ❯ server/lib/db/schema/tenant/core.ts:2:1 The reason the two configs differ: there, the reachable candidates were app sources behind an alias, which `vite:resolve-dev` will not substitute because it only swaps BARE specifiers. Here they are real bare node_modules imports — `drizzle-orm` alone has 303 import sites in `server/`. Two selection criteria, both load-bearing and both now in the comment: - NEVER MOCKED. `vi.mock('drizzle-orm/d1')` appears in 361 specs. A pre-bundled copy of something a spec replaces is a trap, so those specifiers stay out — which is also why `drizzle-orm/d1` and `/better-sqlite3` are absent while the parent package is present. - DUPLICATE-SAFE. A pre-bundled entry is a second copy of the module. drizzle tolerates that by design: its brands are `Symbol.for('drizzle:*')` from the global registry and `is()` compares `entityKind` STRINGS, not identity. `@hono/zod-openapi` was tried and rejected despite being the second-largest candidate at 214 sites: it would inline its own `zod` and `hono` while `server/` imports both raw (14 and 73 sites). `extendZodWithOpenApi` patches one prototype, and the raw-zod schemas in `role-profile.schema.ts`, `video.schema.ts` and `api/services.ts` would never see it. 31 files ran green, but a dual-instance hazard cannot be cleared without a full-suite run.⚠️ The timings say nothing and the comment says so. Same config, warm both ways, machine shared with other agents: 15.6 s / 26.2 s with the include, 20.5 s / 19.8 s without — ±70% variance on identical work, ranges fully overlapping. The gain is argued from mechanism (one module fetch per fork instead of dozens, across 743 forks) and the comment tells the next reader that verification is the poison test, never the clock. Handback for `vitest.config.ts`'s owner: its replacement comment says the optimizer has "no useful target here". The `web`-key half is right; the stronger half is questionable, because that config is also `environment: 'node'` and an `ssr` include of a real bare dependency would work there too. What actually protects it is that `resolveOptimizerConfig` hardcodes an exclude list — `['vitest', 'react', 'vue', …]` — and note that is exact-name matching, so `react-dom` is NOT on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…udget, not vitest's (InspectorHub#92) Reported as `render`/`waitFor` timing out under contention. Two things about that framing were wrong, and both changed the fix. FIRST, THE REPORTED COMMAND DOES NOT REPRODUCE IT. `npx vitest run app/routes --maxWorkers=16` passed 2/2 on an idle machine. The failure needs ambient CPU load, which is not a variable you can assert on — so it was turned into one: N bounded burner processes, recorded and killed BY PID, each with a self-exit deadline so none can outlive the run. Eight logical cores; 28 burners is ~3.5x oversubscription on top of vitest. The dial being monotonic is the proof, not the single red run: 0 burners 259 ms pass 10 burners 875 ms pass (at the wall) 16 burners 1211 ms FAIL 28 burners 1401-2071 ms FAIL 3/3, both reported tests every time SECOND, THE DEADLINE IS NOT VITEST'S. The error is `TestingLibraryElementError: Unable to find an element with the display value: jane` — `@testing-library/dom`'s `asyncUtilTimeout`, which defaults to 1000 ms. `testTimeout` at 5000 ms was never reached, and nothing in the repo overrides the testing-library value. `createRoutesStub` starts in a loading state, so `render()` returns before the page exists and the whole first paint is billed to the following `findBy*`. Of the two opposite causes, this is "real work on mount against a deadline that is too tight", not "the condition is never satisfied". The evidence separates them: the timezone card is a `<Select>` over `Intl.supportedValuesOf('timeZone')` — 418 `<option>` nodes, rebuilt on each of the file's 7 renders — and elapsed time tracks contention linearly while every assertion passes 3/3 when idle. An unsatisfiable condition would fail at ~1000 ms REGARDLESS of load and never pass, so raising a timeout would have been hiding a bug rather than fixing one. `configure({ asyncUtilTimeout: 4000 })` file-scoped, plus `vi.setConfig({ testTimeout: 20_000 })` — the vitest backstop raised only so it cannot fire FIRST: testing-library's error names the element and dumps the DOM, vitest's says only that 5000 ms elapsed. Nothing is hidden; an unsatisfiable condition still fails, four seconds later. Red 3/3 then green 3/3 at 28 burners, with `BURNERS_ALIVE_AT_END: 28/28` each time so the load is known to have been present. Slowest observed 3117 ms. THE POPULATION IS MEASURED, NOT GUESSED. 67 co-located specs use `createRoutesStub` and NONE sets `asyncUtilTimeout`, so all 67 run against the 1000 ms default. Timing every one of them on an idle machine and sorting by per-test duration puts `settings-profile` at 5 of the 6 slowest tests in `app/routes` (432-687 ms — 43-69% of the budget), which is why it was the one that fell over. The next tier is `NewInspectionWizard` 614 ms, `ViewerTimeZoneNotice` 517 ms (which renders the same 418-option list, and so corroborates the mechanism), `AddressAutocomplete` 514 ms, `booking-deposit` 506 ms, `booking` 469 ms, `comments-delete` 428 ms, `agent/repair-items` 415 ms. The systemic one-liner — `configure({ asyncUtilTimeout })` in `tests/setup-web.ts` — would cover all 67 at once but is a shared surface affecting 305 specs. Tracked separately rather than folded in here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…pectorHub#90 follow-up) d85313c's replacement comment said the optimizer has "no useful target here". The `web`-key half of that is right. The stronger half is not, and InspectorHub#93 proved it on the sibling config: with the `ssr` key and an explicit `include`, a real bare dependency IS pre-bundled and IS loaded — verified by poisoning the emitted bundle and watching the import throw. This config is also `environment: 'node'`, so the same would work here. What is genuinely out of reach is only the module that started the investigation: `~/paraglide/messages` is alias-rewritten to a project path before the optimizer's resolver sees it, so listing it produces a bundle nothing imports. Also corrected: the claim that `resolve.dedupe` is what protects react and react-dom from being double-bundled. `resolveOptimizerConfig` hardcodes its own exclude list — `['vitest', 'react', 'vue', …]` — matched by EXACT name, so `react` is covered there and `react-dom` is not; dedupe is what keeps that one single. A comment that overclaims is the same defect as a gate that reports a summary hiding its own comparison: the next reader designs around something that was never established. The corrected version says adding an entry is a real option and names the two tests it has to pass — is the specifier ever `vi.mock`ed, and does a second copy break identity for anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…tion (InspectorHub#93) `7630a602` added `include: ['drizzle-orm', 'drizzle-orm/sqlite-core']` after proving the mechanism works: the artifact appeared in `deps_ssr/`, and poisoning it with a `throw` proved it was actually LOADED, not merely built. Both of those were true and neither was enough. The full `npm run test:unit` — which the investigation could not run — failed two files at COLLECTION: Cannot find module '/node_modules/drizzle-orm/d1/index.js&v=b448f4c1' tests/unit/calendar/google-export.spec.ts tests/unit/calendar/sync-sweep.spec.ts Optimizing any entry point of a package version-stamps how the WHOLE package resolves. `drizzle-orm/d1` is `vi.mock`ed by 361 specs and was deliberately kept out of `include` for exactly that reason — but out of `include` is not out of reach. Adding it to `exclude` was tried and does not help: the `&v=` query is still attached and the mock's resolution still fails. So the bar is higher than the two criteria that let this through. It is not "is the specifier mocked" but "is ANY subpath of the package mocked or aliased anywhere", and the only instrument that answers it is a full suite run. A 31-file sample said yes and was wrong. Reverted to an empty `include`, which switches the optimizer off again — the same state as before `7630a602`, but no longer for the same reason. The comment now records what was learned rather than resetting to the earlier claim: the `ssr` key IS read (unlike the sibling config's `web`), `noDiscovery` is set unconditionally so `enabled: true` can never bootstrap itself, the poison test is how you check an entry does anything, and — new — the poison test is necessary and not sufficient, because it says nothing about what the package's other subpaths now resolve to. Nothing was pushed. The gate ladder worked as designed: pre-commit passed because it should have (the config is valid and the targeted specs were green), and the pre-push full suite is the rung that owns cross-file interaction. This is the case it exists for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…lback (InspectorHub#95) `await import()` of a module whose graph reaches `~/paraglide/messages` costs 1.9-3.7 s of main-thread transform, and inside a test body that is billed against `testTimeout`. Four specs were fixed by hand across this session; InspectorHub#94 rejected a disk cache as the alternative partly because a warm cache would make this class stop reproducing locally and start failing only in CI. So: a gate. TWO PRIOR FINDINGS CONTRADICTED EACH OTHER AND ONE WAS WRONG. `report-card-stack.buttons.test.ts` has 8 dynamic imports: seven `?raw` and one `../../../../messages/en/reports.json`. The measurement that called it clean was right; the report that named it was almost certainly reading that JSON line — an 18 KB data module that by construction imports nothing. That is precisely the false positive a shape-only matcher produces, so `.json` is a named exclusion with its own printed reason rather than being folded in with `?raw`.⚠️ AND `beforeAll` DOES NOT HAVE "NO BUDGET" — a correction to what earlier commit messages in this session said, including mine. Hoisting `sidebar.test.ts` into `beforeAll` failed with `Hook timed out in 10000ms`. The budget is SEPARATE, not ABSENT: it is `hookTimeout`. Only COLLECTION is unbudgeted. Hence a three-rung ladder, now in the gate's failure message and both file headers: static import first (nothing to mock means nothing has to be dynamic); `beforeAll` only when a `vi.doMock` must precede resolution, and bounded by `hookTimeout`; a raised timeout of either kind, never. EXPLICIT TIMEOUTS ARE NOT A SANCTIONED EXCEPTION, and the numbers settle it rather than taste. `sidebar.test.ts`'s first test: 2472 ms solo, **19206 ms** under `--maxWorkers=16`, against its own `20000` ceiling — 794 ms of headroom, on a laptop, on a growing suite, not on CI. The timeout never removed the cost; the other fifteen workers queued behind the same transform regardless. It moved the cliff, and the cliff had nearly arrived. It is also self-granting: anyone the gate stops can silence it with a third argument. The gate parses with the TypeScript compiler rather than a regex, and that is not fastidiousness. `app/components/editor/batch-action-bar.test.ts` contains `/role="radio"/g`; a hand-lexer that skips string literals reads that `"` as an opening quote, desyncs, and reports THE FILE AS HAVING NO FINDINGS — silent blindness, not an error. Same class as the apostrophe that blinded the contrast scanner earlier today. `typeof import(...)` (8 specs) is a different node kind and is free. It follows one level of helper indirection, because `async function load() { return import(x) }` was otherwise a one-line bypass and two specs already use that shape. Non-literal specifiers go to a non-fatal `unresolved` bucket, pinned by file in the spec. Total accounting on every run, so "clean" can never mean "matched nothing": examined 333 spec file(s) under app/ — 324 with no dynamic import (passing), 9 with at least one; 35 dynamic import site(s) in total. Those 324 are passes, not skips, and an unparsable file is a FAILURE rather than a clean one. Red-then-green: the pre-fix `settings-automations` shape is reported; its hoisted form reports 0 violations but `sites: 1`, so it cleared on PLACEMENT rather than by not being seen. A `?raw` control is not reported while `cheap[0].why` proves it was examined. Mutating `TIMED_ROOTS` from `it` to `itt` turns 10 of 22 red — and notably NOT the "real tree loads no module graph" test, which is exactly why the accounting and unresolved-pin assertions exist; both of those did go red. Dropping the `?` exclusion reddens exactly 5, including the control. Two files fixed. `sidebar.test.ts` goes to static imports with all four explicit timeouts removed; `useRepairOpQueue.test.tsx` to `beforeAll`, which is correct there because its `vi.mock("react-router")` must precede resolution. Per-file positive controls: falsifying one expected value in each reddens exactly that test and nothing else. `app/components` under `--maxWorkers=16` is 179 files / 1016 tests green, and sidebar's first test is 19206 ms -> 7 ms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…b#95) Separate commit — staging `package.json` escalates the pre-commit type-check from the api tier to the full tier, and makes `vitest --changed` degrade to the whole suite. Into `lint` and `lint:gates-full`, deliberately NOT into `run-gates.mjs` and so not into pre-commit. It costs ~5.5 s, of which 2.1 s is loading `typescript` — it refuses to prefilter by regex for the reason in its header (a regex lexer desyncs on `/role="radio"/g` and silently reports the file as clean). What it prevents is a slow CI suite, not a broken commit, so the CI-tier chain is where it belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…nspectorHub#63 phase 0.5) `tsconfig.json` excluded `app/**/*.test.ts(x)`, so 329 co-located specs plus 19 under `packages/shared-ui` were compiled by no tsc program. That exclusion is gone; the exclude list is now just `["node_modules","build","dist"]` and 93 errors across 38 files are down to zero. DESIGN (A) OVER (B), DECIDED ON NUMBERS. Making `tsconfig.json` composite so a separate program could reference it works and is cheap in time — but costs 34 errors only app SOURCE can fix: 30 × TS6307 (a composite project must enumerate every input), plus TS2742 on three hooks whose inferred types "cannot be named without a reference to react-router/…/route-data". That is a permanent annotation tax on every future hook returning an RR internal type, to save four seconds.⚠️ THE HEAP FIGURE IS UNCHANGED, AND THE INVESTIGATION'S RECOMMENDATION TO DROP IT TO 2048 IS NOT APPLIED, BECAUSE THERE ARE TWO DIFFERENT COLDS. .types/ PRESENT (local pre-commit) 1280 MB passes .types/ ABSENT (npm ci, i.e. CI) 6144 OOMs, 8192 passes (219 s) `tsc -b tsconfig.json` builds the api project first when its outputs are missing. Locally `.types/` usually survives, so deleting only `.tsbuildinfo` measures the app program re-checking against existing `.d.ts` — genuinely ~1.3 GB. CI starts from a fresh checkout where `.types/` has never existed, so it pays the api source build too. Dropping the cap to 2048 would have passed every local measurement and broken CI on the first run. This also sharpens an earlier finding of mine: InspectorHub#62 attributed the 8 GB requirement to `tsc -b --force`. The number was right and the attribution was not — `--force` merely guarantees what a fresh checkout gets for free. The variable is whether `.types/` exists. Re-measured after this change: 8192 still passes, so 352 more files did not move the ceiling. Warm incremental after a spec edit is 6.0 s; after a source edit, unchanged. 🔴 ROOT CAUSE OF 21 OF THE 93 ERRORS, AND IT IS NOT IN THE SPECS. `worker-configuration.d.ts:1523` and `@cloudflare/workers-types:1789` both declare HTMLRewriter's `interface Element` with a chaining `remove(): Element`. TypeScript MERGES that with lib.dom's `Element.remove(): void`, and the resulting TS2717 is swallowed by `skipLibCheck: true`. Because `HTMLSelectElement` is the one lib.dom subtype that re-declares `remove`, it stops being assignable to `HTMLElement` — so `el as HTMLSelectElement` is TS2352, the reverse is TS2345, and `getByRole<HTMLSelectElement>()` fails its own `T extends HTMLElement` constraint. This is latent in app source today (nothing casts to a `<select>` there); the specs were simply the first files to touch it. Fixed without a suppression: `tests/helpers/dom.ts#asSelect` narrows through a user-defined type guard — a RUNTIME assertion, strictly stronger than the 21 casts it replaces. Suppressions, side by side over the 352 specs plus three new helpers: as any 39 → 32 @ts-expect-error 0 → 0 as unknown as 91 → 91 The helpers add zero — the one `as unknown as` a grep finds in `helpers/dom.ts` is inside a comment explaining why it was NOT used. `types` gained exactly two entries, each verified by removal: `vitest/globals` (201 specs use bare `describe`/`it`) and `@testing-library/jest-dom/vitest` (19 × TS2339 without it; the subpath matters). `@types/node` LOOKS required by the four specs importing `node:fs` — it is not, because `worker-configuration.d.ts` is generated with `nodejs_compat` and already declares them. Adding it would have put `NodeJS.Timeout` into ~685 browser files' `setTimeout`. NINE SPECS WERE ASSERTING THINGS THAT COULD NOT HOLD, and the casts were what hid them: `IconButton` given an `icon` prop it does not have (spread onto the DOM `<button>`); an `AddressSelection` fixture using `description:` where the field is `formatted:`; two render helpers omitting a REQUIRED `onReorder`, so a drag-reorder would have thrown; `referrals?: typeof REFERRAL_I1[]` making the first fixture its own contract and describing IA-35 policy cases the route cannot receive; a double `as never` that erased all prop checking and hid a `brand` fixture missing 8 of `TenantBrand`'s fields; an action mock whose `mock.calls[0][0]` indexed a zero-length tuple; a per-service `depositPolicy` inferred as `null`, so the "service opted out" case described a shape that cannot exist. Plus 13 route-spec call sites passing 3 of the 5 arguments RR v8 hands a server loader. Anti-leak audit, per the pattern that produced four vacuous assertions in Phase 1: every security/privacy/permission spec touched here has a working negative control. The sms-consent case got stronger — its `expect(res.error)` was conditional on `!res.ok`, and now also asserts the action dispatched to the intended intent, because one branch has no `ok` at all. The ratchet was EXTENDED, not duplicated: a `PROGRAMS` array covers both configs with their own structural lists, every existing export and verdict is unchanged, and the gate's own spec passes untouched. Proved red by adding a file to `tsconfig.json`'s exclude. A missing baseline key reads as `[]`, never as "adopt whatever the tsconfig currently says". Reported and not fixed: `reportViewProps` defaults every field it reads but its parameter type demands the full success payload; `packages/shared-ui/tsconfig.json` is confirmed orphaned (nothing references it, no script runs it); `eslint.config.js`'s ignore for `app/**/*.test.*` was justified by "excluded from tsconfig.json, can't be placed in any TS project" — that premise is now false and only the comment says so, because un-ignoring 352 files is a change of its own size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…nothing
eslint's ignore list justified skipping `app/**/*.test.{ts,tsx}` with "excluded
from tsconfig.json, so the type-aware parser can't place them in any TS
project". InspectorHub#63 Phase 0.5 retired that exclusion months of commits ago; the
comment outlived the fact. Measured before acting, in both gate shapes, which
agree exactly because every error-level rule here is syntactic:
ESLINT_FAST=1 (pre-commit) 33 errors, 19 warnings
type-aware (CI) 33 errors, 332 warnings
20 no-explicit-any · 10 no-unused-vars · 2 no-console · 1 no-require-imports
across 15 of 352 files.
Now 2 -> 0. CI pays rule evaluation only, not program construction: tsconfig.json
already contained these files whether or not eslint read them.
`tests/**` stays ignored, and the rationale in the config is rewritten to say why
for the CURRENT reason rather than the dead one: 1581 errors across 364 of 856
files, 970 of which are the fixture case the override block already claims to
exempt and never listed. It also cannot go type-aware until tsconfig.tests.json's
exclude ratchet reaches zero.
What the un-ignore found, none of which was a lint problem:
* `portal-auth.test.tsx` was type-checked by NO program. It sat beside
`portal-auth.test.ts`, and a tsconfig `include` glob keeps one extension per
base path (.ts outranks .tsx), so the twin was dropped silently. vitest
collected and ran it, so it looked covered. Demonstrated with one deliberate
`const x: number = "string"` in the same file under two names: as
`.redeem-destination.test.ts` tsc reports TS2322 and exits 1; as
`portal-auth.test.tsx` it exits 0. Renamed after what it covers.
* `connected-apps.test.ts` claimed "never calls window.confirm (ConfirmDialog
is used instead)" and asserted only the first half. The unused `html` binding
sat under a comment saying a closed ConfirmDialog still renders in static
markup. It does not -- shared-ui Modal returns null when !open -- so the
second assertion was never written and would have failed if it had been.
* `report-view.test.ts` passed `brand: { name: 'Acme' }`. TenantBrand has no
`name`; it has `companyName`. A dead fixture an `as any` was hiding.
* `automation-editor-logic-only.test.ts`'s unused `src` was a coverage fossil:
when the form moved into the modal the must-not-bind guard moved with it,
leaving the route free to grow a body input back unwatched.
Partial fixtures are now `Partial<T>` asserted to `T` rather than `as any`, which
still checks every key and value. Three mutants confirmed red then reverted:
`name: 'Acme'` -> TS2353, `isPublishedd` -> TS2561, a typo'd severity bucket ->
TS2345. Both rewritten assertions were driven red with their intended messages
before being restored.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
A tsconfig `include` glob names no extensions. TypeScript expands the directory and keeps ONE file per base path in a fixed priority (.ts > .tsx > .d.ts), so two files differing only by extension mean one of them is silently absent from every program -- with no diagnostic, because from tsc's side nothing is wrong. Every other signal says the file is fine. Git tracks it, vitest globs by filename and collects it, its tests run and pass, coverage counts it. The only thing that never happens is type-checking, and nothing announces an error that was never produced. `portal-auth.test.tsx` sat like that from the day it was written; only eslint's type-aware parser ever said so, and only once the co-located specs stopped being ignored. `check-tests-tsconfig.mjs` structurally cannot catch this class: it compares a DECLARED `exclude` array, and a collided file is excluded by nothing -- it loses a tiebreak. Proven red on the real case before being kept: recreating `portal-auth.test.tsx` alongside its `.ts` twin makes the gate exit 1 and name the file; removing it returns 0 across 2765 files. The scanned count prints on every run beside the collision count, and a scan that reaches 0 files fails instead of reporting OK -- a gate that cannot see what it checks has no business being green. At pre-commit rather than CI because a collision is created at exactly one moment, when a file is added or renamed, and that is the rung that sees it. It is also the rung where the fix is free: renaming a file nobody has pulled costs nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
Own commit because a staged package.json escalates the pre-commit type-check to the full tier and degrades `vitest --changed` to the whole suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
`backups/` is the gitignored dump directory (D1 exports, plus whatever scratch a measurement leaves behind). Flat config does not read .gitignore, so one stray `.ts` landing there is a fatal parse error in `npm run lint` — and only locally, because CI checks out a tree in which the directory does not exist. That divergence is the actual defect, not the unlinted file: the pre-push full run exists to predict CI, and a gate that is red in exactly the place CI is green teaches people to push through it. Found by the pre-push run itself, after a timezone measurement left a probe harness in there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
`TIMEZONE_SELECT_OPTIONS` computed each zone's offset in the map that feeds the sort, destructured it away, and then recomputed it inside `timeZoneLabel`. That is 838 `Intl.DateTimeFormat` constructions plus `formatToParts` calls for 419 zones, at module scope — paid once per isolate on the server, and again in the browser during hydration of any route whose chunk graph reaches here. `timeZoneLabel` now takes the offset optionally; every other caller is unchanged. Measured in Chromium via CDP CPU throttling (tests/e2e/public-timezone- hydration-cost.spec.ts, InspectorHub#99). Cold first pass of the table: 50ms at 1x, ~1.25-1.5s at 6x. Running the two variants in BOTH orders, the duplicate work accounts for 26-60% of that — the spread is wide because whichever variant runs second inherits a warm ICU cache, and this machine could not pin it tighter. An earlier single-order run reported 57.95%, which was arithmetically impossible (it claimed to save 35ms from a 25.4ms total) and is why the harness now runs both orders. What this does NOT claim: page-level improvement. On /verify at 6x the LONGEST blocking task did drop consistently — all four samples after (422/1042/1052/1128 ms) sit below both samples before (1372/1800 ms) — but TOTAL blocking time did not move (1678 before; 1615/1670/1780 after). The work appears to redistribute across tasks rather than disappear. n is 2 vs 4; that is not a result. The change stands on the part that is not in doubt: identical output, half the work. `every label matches what recomputing the offset would produce` asserts that across all 419 zones rather than a sample, and was driven red first (a deliberate `offset + 60` reported `Pacific/Midway: expected '(UTC-10:00)' to be '(UTC-11:00)'`) — a mismatch would be one zone reading wrong in the picker, exactly what three spot-checks miss. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…ble (InspectorHub#99) A measurement instrument, not a gate. It asserts only that its own instrument works — no ms thresholds, because a wall-clock budget on a shared runner is a coin flip and a flaky perf gate gets disabled, which is worse than not having one. Env-gated behind TZ_PERF with the same `.never.ts` shape as the `cloud` project, so it cannot silently collect nothing while looking like a passing suite. It separates two costs with different populations, which the original framing of InspectorHub#99 had merged: A. module evaluation — paid by EVERYONE who loads /verify, including a visitor whose envelope id is bogus. verify.tsx returns early on !result and the notice is further gated on signers.some(s => s.signedAt), so the control never renders — but the module is a static import, so hydration builds the table anyway. B. 419 <option> DOM nodes — only when the notice really renders. Measured split at 6x: table build ~1250-1500ms cold, option mount ~68-93ms. The DOM is roughly 5% of it. That is the finding that matters for choosing a fix: virtualizing or deferring the <option> elements — the usual advice, and what most timezone pickers do — attacks the small share here. Two methodological guards, both added after the harness lied: * Both orderings are run and both printed. A fixed order lets whichever variant runs second inherit a warm ICU cache and a warmed JIT. The first run reported saving 35ms out of a 25.4ms total, which is impossible, and that arithmetic is what exposed it rather than review. * COLD first pass is reported separately from the warm median. Hydration pays the cold one; the warm numbers understate it by roughly half. Test timeout scales with the throttle rate. At 6x the four build sequences are ~20 throttled passes, which blew the 30s default and took the /verify measurement down with it. Trimming samples would have produced a fast, quiet, wrong answer instead of a visible failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…d for
A visitor opening /verify with an invalid link saw "Verification failed" and
nothing else — and their browser still downloaded and built the full IANA zone
table during hydration. ~450ms of it at a 6x CPU throttle, for a control that
page never renders.
Three things had to change together; any one alone does nothing.
1. SPLIT THE MODULE. Importing any binding evaluates the whole module, and
`viewer-timezone.tsx` imported two cheap helpers out of the same file the
419-entry table lived in. Making the component lazy would have changed
nothing while that import stood. `timezones.ts` now holds only cheap
primitives; `timezone-options.ts` (full, Settings) and
`timezone-options-public.ts` (curated, public) are separate leaves — separate
because putting both tables behind one import would recreate the same defect
under a new filename.
2. CURATE THE PUBLIC LIST. ~90 zones with names a reader recognises ("Central
Time", not "America/Indiana/Tell_City"), which is what mainstream pickers
ship. Settings keeps the complete list: cal.com shortened theirs and had to
reopen it as a high-priority issue to put the missing cities back, and a
tenant configuring their company zone needs their actual zone. A viewer
correcting a browser guess does not — and when their zone genuinely is absent
it gets added, because a <select> whose value matches no <option> displays the
FIRST one instead, which would tell them their times are on a clock they never
chose, silently.
3. GATE THE IMPORT ON DATA THE SERVER ALREADY HAS. Whether the control appears
is decided by the loader (`signers.some(s => s.signedAt)`, `inspection.date`).
`LazyViewerTimeZoneNotice` dynamic-imports the rest only when it will show,
with a skeleton for the gap. Deliberately not `lazy` + `Suspense`: that
suspends during SSR and would put a skeleton in the server HTML, where a
no-JS reader and the PDF renderer would keep it forever. It renders nothing
until mounted, which is exactly what this page emitted before.
Measured at 6x: /verify with a bogus id went from 1678-2244ms of long tasks to
603ms, and the longest single blocking task from 1372-1800ms to 251ms. But the
assertion kept is not a timing one — timing on a shared box said the longest task
shrank while total blocking did not, which is not a result. `a visitor who cannot
see the control never downloads it` asserts the chunks are not REQUESTED, which
is a fact, and it carries a positive control (the cheap module MUST load) so it
cannot pass by watching a page that never touches timezones.
Two guards, both driven red before being kept:
* `timezone-module-boundaries.test.ts` reads the imports, because crossing this
boundary produces no behaviour to assert on: one static import in the cheap
path puts 419 Intl constructions back into every public hydration, renders
identically, and leaves every other test green. Verified by re-adding the
import (and a static ViewerTimeZoneNotice import in verify.tsx) and watching
both fail by name.
* `publicTimezoneOptions` inserts an uncurated viewer's own zone. The fixture
asserts it is genuinely uncurated first, or the three tests below it would
pass by exercising the curated path.
RENAMED zones are handled rather than assumed. `Intl.supportedValuesOf` omits
aliases and which spelling is canonical depends on the runtime's ICU age — this
repo's Node answers `Asia/Calcutta` where browsers answer `Asia/Kolkata`. The
mismatch is silent (DateTimeFormat accepts both, so offsets and labels stay
correct) but shows the viewer two entries for the one zone they are in. Entries
resolve to whichever spelling the runtime lists, and the count that resolved to
neither is asserted to be zero so the next rename fails a test instead of
quietly dropping zones.
Verified in Chrome, light and dark, against a synthetic signed agreement in local
D1 (removed afterwards): 88 curated options, the browser zone selected and
labelled "(UTC+08:00) Beijing, Shanghai", readable in both themes. With the
viewer forced to America/Indiana/Tell_City: 89 options, that zone selected (not
the first one), sorted between Panama and Puerto Rico, and the rendered signature
timestamp switched to CDT. On the invalid-link page: no select, and no stray
skeleton.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
Two additions to the InspectorHub#99 harness. LIST-SIZE SWEEP. The mainstream React component ships ~86 hand-picked zones rather than ~419 raw IANA ids, so the question was what shrinking buys. A curated list is modelled by SIZE alone: the per-zone work is one Intl construction either way and a friendlier label is data, not computation. Each size gets a fresh page so its first pass is cold, and the sweep runs both directions because whatever runs later inherits a warm ICU cache. Measured at 6x, cold: 419 zones 477/428ms, 200 zones 278/240ms, 86 zones 231/157ms, 40 zones 139/146ms. Fitted: a ~110ms fixed floor plus ~0.82ms per zone. The floor is ICU's one-time initialisation and curation does not touch it, confirmed from the warm column where the same fit gives a fixed term near zero — so the intuitive "86/419, therefore 20%" is wrong and the answer is about 43%. The 86-zone row is the least trustworthy point in the table; its two directions disagree by 47% where every other size agrees closely. Also recorded: react-timezone-select's offset+DST de-duplication runs over its curated file, not the full set, because de-duplicating 419 zones means resolving all 419 first — the very cost being avoided. Curation has to be static data. CHUNK ASSERTION. Timing said the longest blocking task shrank while total blocking did not, across too few samples to conclude from. Whether the browser REQUESTS the chunk is a fact and does not flake, so that is what is asserted now. It carries a positive control — the cheap primitives module MUST load — because otherwise "no zone chunks were fetched" would also pass on a page that never touches timezones at all. Its first run failed usefully: the filter matched `timezones-*.js`, which after the split is the 1KB cheap module and is supposed to load, and `LazyViewerTimeZoneNotice-*.js`, the ~1.6KB wrapper the route imports statically so that it can decide NOT to fetch the rest. Both are now excluded by name, and which chunk actually holds the table was read out of build/client rather than guessed from filenames. Test timeout scales with the throttle rate: at 6x the four build sequences are ~20 throttled passes, which blew the 30s default and took the /verify measurement down with it. Trimming samples would have produced a fast, quiet, wrong answer instead of a visible failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…able
Found by the pre-push suite, which failed the rotation test under load while it
passed alone. The tempting reading is "flake"; it was not.
The settle effect released the in-flight guard and THEN queued the new key:
inFlight.current = false;
if (!failed(fetcher.data)) setIdempotencyKey(mintKey());
`submit` read the key out of the closed-over state, so between the effect
running and React committing that render, both guards were open while the key
was still the spent one. A click landing there sent a duplicate request bearing
the used key, which the server treats as a replay of the first: the caller is
told it worked and nothing is written — the worse of the two failures this
hook's own docblock describes.
The key now lives in a ref updated synchronously at rotation, and the guard is
released after it. The state variable stays, because the rendered value is what
tests and any UI read; the ref is what the request carries. `submit` no longer
depends on the key, so its identity stops churning too.
I could not build a deterministic reproduction: the window sits between the
effect and the render commit, and `act()` flushes both. The evidence is the
loaded-suite failure with exactly this signature, the code path, and the fact
that the fix removes the window by construction rather than by timing.
The test is deliberately NOT changed to wait for the rendered key. `settled()`
waits on `busy`, which comes from `fetcher.state` and goes idle BEFORE the settle
effect — so a submit fired right after it lands in precisely the moment worth
testing. My first attempt at this commit "fixed" the test that way instead, and
it would have made the rotation test pass no matter what the hook did. The
docblock now says so.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged `agreementHtmlIsEmpty` on this PR: js/incomplete-multi-character-sanitization, high. It is a real alert on code this PR introduced (InspectorHub#67 part 2), not a stale attribution. It is NOT exploitable here. Both callers use only the boolean; the stripped string is never rendered, and rendering goes through SanitizedHtml + DOMPurify. Nor is the loop a behaviour fix, and it is not claimed as one. For this exact regex a single pass is already complete: `[^>]*` cannot cross a `>`, so a match beginning at `<` consumes everything up to the first following `>`, including any `<` between — a surviving `<` therefore has no `>` after it and cannot form a tag. Checked against eleven nested and malformed constructions; looping changes nothing in any of them. What the loop buys is that the argument above stops being load-bearing. It depends entirely on the current character class: narrow it, or add an alternation, and one pass silently stops being enough. The loop keeps by construction what is presently only guaranteed by hand — and clears the alert on a file in the agreement-HTML family, where a standing suppression is the wrong thing to leave behind. Adds the coverage the function never had, including the malformed cases, so a future narrowing of the regex fails here rather than quietly leaving markup. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
…t its regex The previous commit's test inlined `replace(/<[^>]*>/g, "")` to demonstrate the strip property. Two things wrong with that. It tested the pattern rather than the behaviour — the function could have been deleted and the test would still pass. And it tripped the same CodeQL rule on the test file, which is fair: the rule does not know a file is a test, and a repository that suppresses it there teaches itself to suppress it anywhere. Rewritten to go through `agreementHtmlIsEmpty` and to assert the direction that reaches a user: markup they did not write cleanly must not make their words disappear, because "empty" here means the save is refused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0185QebqzFLviQKtnEjLkC3H
This was referenced Aug 10, 2026
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
2 participants
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.
62 commits. The connecting thread is not a feature — it is a class of defect: things that were green because they could not see what they claimed to check. Most of what follows was found by making an existing check able to fail, and several of the fixes are new checks rather than new behaviour.
Type coverage for the test suites (#63)
The specs were excluded from every TypeScript program, so a fixture could name a field that did not exist and nothing would say so.
tsconfig.tests.json(phase 0) bringstests/unitandtests/workersinto a program, with an exclude ratchet so the remaining files can only decrease.app/**/*.test.ts(x)exclusion — 352 co-located specs now type-check.as anyfixtures.What that surfaced, in order of how quietly it had been failing:
Contextdefaults its Env toany, andany & HonoConfigisany, so an unannotated middleware collapsedctoContext<any>across 332 routes. Annotating it produced 36 real errors that had been invisible.tests/unit/platform/route-middleware-env.spec-d.tsholds the line, with a control that asserts the broken shape really isany.portal-auth.test.tsxwas type-checked by no program at all. A tsconfigincludeglob keeps one extension per base path (.tsbeats.tsx), and aportal-auth.test.tssat beside it. Git tracked it, vitest ran it, coverage counted it; only type-checking never happened. Demonstrated with the same deliberate error under both names, and now gated bycheck-extension-collisions.mjs.namefieldTenantBranddoes not have.Conformance gates
bg-ih-*class names resolved to no CSS (feat: P2 advanced features — Subtypes + SpeedMode + ConflictResolver (Gap 11-16) #85). Tailwind does not error on an unknown token, andlint:dswas blind to it.app/(feat: Foundation layer — composite finding keys + property type system #79, feat: editor 4-column layout restructure (Gap 2) #80), with a gate that reads the computed pairs rather than a list of known-bad spots./role="radio"/gand reported the file clean.Product surfaces
cancelledwithout charging./uncancelmoves into the cancellation module; status writes now refuse withUSE_CANCEL_ENDPOINT.viewCommunicationwas stripped by a zod schema; the checkbox saved, the bit did not.ai_content_reviews, the write path, the review control, and a gate that classifies what each AI capability writes and blocks a second route to a model.Design system
--ih-primarykeeps the tenant's exact hex for fills; brand-coloured text is derived by moving HSL lightness until it clears AA.contrastForegroundnow computes a real WCAG ratio instead of YIQ.Performance
/verifylink downloaded and built the full IANA table during hydration for a control that page never renders. Fixed by splitting the module (importing one cheap helper was evaluating the whole table), curating the public list to ~90 recognisable names, and gating the import on loader data the server already has. Measured at a 6x CPU throttle: long tasks 1678–2244 ms → 603 ms. The assertion kept is not a timing one — it asserts the chunks are never requested, with a positive control so it cannot pass by watching a page that never touches timezones.Found by the pre-push suite itself
The rotation test in
useGuardedSubmitfailed under load while passing alone. Itwas not a flake. The settle effect released the in-flight guard and then queued
the new idempotency key, and
submitread the key from closed-over state — sobetween the effect and the render commit, both guards were open while the key was
still the spent one. A click there sends a duplicate bearing the used key, which
the server treats as a replay: the caller is told it worked and nothing is
written, the worse of the two failures that hook's own docblock describes. The
key now lives in a ref updated synchronously at rotation, with the guard released
after it.
The test was deliberately not relaxed.
settled()waits onbusy, which comesfrom
fetcher.stateand goes idle before the settle effect — so a submit firedright after it lands in exactly the moment worth testing. Waiting for the
rendered key instead would have made the test pass regardless of the hook.
Testing
npm run lint,npm run test:unit,npm run test:weball green locally before push. New guards were each driven red before being kept; where a test could only pass vacuously, it carries an explicit control.🤖 Generated with Claude Code