Capability seam, unified marketplace, DSAR fulfilment, and enforced AI allowances - #309
Merged
Merged
Conversation
The gate skips a hit whose literal is immediately followed by `|`, because `status: 'a' | 'b'` is a type declaration, not a status write. The test for that was `/^\s*\|/`, which a logical `||` satisfies just as well — so in `status === 'completed' || status === 'cancelled'` the FIRST branch was discarded as if it were a union member, and only the last comparison in any `||` chain was ever scanned. `[^|]` is what makes the guard mean "a single pipe". Proven by adding the two cases first and watching them fail against the old regex: the `||` chain reported 1 hit instead of 2, and the object-property-with-`||`-default reported 0 instead of 1. A third case pins the narrowing does not go too far — an unspaced union (`status:'completed'|'cancelled'`) is still skipped. The hole was latent, not occupied: the gate still prints `OK (5 baselined, 0 new violations)`, unchanged. That was the expected result and is the reason to fix it now rather than after the next such write, which would have been invisible. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…hat it scanned `git ls-files` reports what the INDEX tracks. A file deleted in the working tree but not yet staged is still in that list, so `readFileSync` threw ENOENT and took the whole gate down with a stack trace — meaning the gate could not run at all during exactly the commit that deletes a file. Found by deleting portal's /setup; both repos' copies had the identical bug, so both are fixed. A file with no content on disk has no size to measure, so it is skipped. The skip count is printed on every run: a skip nobody can see is indistinguishable from a file that passed. Also prints the scanned count beside the grandfathered count, and fails closed when the file list comes back empty. A gate that scanned nothing was reporting the same cheerful green as a gate that scanned everything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…nant routing
The ledger recorded two symptoms and said the audit of what the e2e projects
share had not been done. It has now been done, and it moved one of the two
attributions.
## The 30s waitForURL — real, and it is capacity
`playwright.config.ts` defines 44 projects over ONE `wrangler dev` worker and
ONE local D1, at `workers: 3`. Ten projects declare `dependencies:
['editor-seed']`, so Playwright releases them together the moment the seed
finishes. Among that cohort, `workspace-pages-responsive` opens by POSTing 25
long-string contacts, while `people-role-profiles` runs three separate UI
logins — each a PBKDF2 hash on that same single isolate — inside a `beforeEach`
that spends the TEST's 30s budget. `workspace-pages-responsive` had already
noticed this from the other side: it allows itself 60s purely for the worker to
answer `/status`.
Three fixes, none structural: an explicit 60s on the login `waitForURL` (it is
measuring the worker's queue depth, not the page), a 90s describe timeout, and
a scheduling-only `dependencies: ['people-role-profiles']` on
`workspace-pages-responsive` so the 25-row burst no longer lands on top of the
logins.
## The 503 — the recorded cause cannot happen
The ledger attributed it to tenant-routing's "Tenant not found or system not
initialized". That branch is unreachable in standalone.
`tenant-routing/index.ts:51` enters only when `profile.fixedTenantId` is
truthy, and `resolveByFixedTenant` sets `tenantId` to that same value as its
first statement — so the `!c.get('tenantId')` guard at :60 that emits the 503
can never be true. Its unit test reaches it only by `vi.mock`-ing the resolver
into a no-op, commented "simulates resolver finding no tenant row"; the real
resolver finding no row still sets tenantId. The test's premise is false
against the code it protects.
The attribution was possible because the assertion interpolated the fixture
NAME and not the response body, so the failure printed a bare 503 and the
message had to be found by grepping. That assertion now prints the body. The
next occurrence will name its own cause instead of being matched to a string.
Two findings recorded rather than fixed, both deliberately out of scope here:
`reuseExistingServer: true` skips the build AND the four `--var` bindings, so a
leftover worker can serve the whole suite; and the unreachable 503 guard should
either go or become real, which is a fail-open→fail-closed behaviour change and
not a flake fix. Also corrected two config comments that described behaviour
that no longer exists — the file is read as documentation of what is shared,
and stale entries there are how the previous audit missed a cause.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…orHub#308/InspectorHub#293 round Two caps moved up by a hair and one moved down: app/routes/settings-communication.tsx 777 -> 778 (+1) server/api/admin/admin-settings.ts 723 -> 726 (+3) server/api/calendar.ts 472 -> 453 (-19) All three are long-grandfathered, well over the 400-line limit before any of this work. The two increases are the managed-SMS guard becoming a capability read and the calendar admin surface losing its Google assumption; splitting either file is a refactor neither change justifies, which is the sanctioned reason to bump rather than the sanctioned reason to ignore. The decrease is the part worth noting: calendar.ts genuinely shrank when the OAuth-specific connect branch became a provider-chosen flow, so the ratchet tightens there and cannot drift back to 472. Committed alone: a baseline is gate configuration, and mixing it into a feature commit escalates that commit's type-check tier for no benefit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…nspectorHub#308) Folds all seven tasks of the mode-branching plan into one commit; per-task commits were not worth a tsc run each. The seam already existed and three callers used it correctly; ten sites went around it. The keystone was the parameter: `getDeploymentProfile` read three optional fields and demanded the whole `AppEnv`, so every caller holding a narrower env — RR loaders with `WorkerEnv`, the cron with `ScheduledEnv`, the email assembler with `EmailServiceEnv` — either paid a cast or wrote its own `env.APP_MODE === 'saas'`. Narrowing it to a `ProfileEnv` of exactly what it reads made all of them structurally compatible, and the nine remaining sites became one-line substitutions. `mcpEnabled(env: { MCP_ENABLED?: string })` was the same family's precedent. Four capabilities cover the sites with nothing to substitute: `mcpApiRoute`, `videoBackendManaged`, `hasManagedCompliance`, and `hasContentMarketplace` (the last consumed by the marketplace route in the following commit). The gate is the part that keeps it true. It matches a property READ (`.APP_MODE`) rather than the bare word, so the seven env-interface declarations need no allowlist, and its `PORTAL_API_URL` pathspec now covers `app` — previously it greps only server and workers, which is exactly where the four loginRedirectBase copies could be written. `workers/env.ts` declines to spell that name even in a comment because the gate reads it there; `app/routes` wrote it five times because the gate never looked. Same repo, opposite outcomes, decided by a pathspec. Both gates now print scanned-count beside matched-count and fail on a zero scan. Two real bugs fell out, neither of them the point of the plan: - The video-provider decision was computed twice and did not agree. The `session-context.ts` copy reported `paid ? 'stream' : 'r2'` with NO check on the STREAM binding or STREAM_CUSTOMER_SUBDOMAIN, while `resolveVideoBackend` 503s in that state — so a paid SaaS tenant on a deploy missing either one was shown the Stream UI against an API refusing every call. One extraction now serves both, and `videoStreamServiceable` requires both bindings. - `metering.service.ts` took an `APP_MODE` parameter it never read. Dropping it turned two object-literal call sites in the spec into excess-property errors — invisible to vitest, fatal in CI. That spec is included here; the plan's own file list omitted it. Gate end state: `.APP_MODE — 3 non-test files matched, 0 stray, 0 awaiting migration`; `PORTAL_API_URL — 2 non-test files matched, 0 stray`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…ships The seeder was anchored to /setup, which refuses forever once a tenant user exists, so content only ever arrived on day one — an upgrade carried schema and code and no content at all. This calls the SAME canonical seeder both provisioning paths already use, from a tenant-scoped owner-gated route, so it works in either deployment mode and never touches /api/integration/*. It ADDS what is new and never refreshes what exists: the skip check is by name, so a renamed row comes back as a second copy. The UI says "install what's new" for that reason and states the rename consequence above the button, not in a result message after the fact. The marketplace becomes SaaS-only. In standalone the catalogue is empty with no path by which anything reaches it, so the browse route 404s rather than showing the "Marketplace is empty" screen this work exists because of. The API handlers stay ungated — harmless once nothing links to them, and InspectorHub#293 reuses them. Three things worth recording: - The Command Palette was a SECOND door. `CommandPalette.tsx` links to /library/marketplace, so hiding only the hub tile would have left a palette entry that 404s — the exact outcome the gate was meant to prevent. Both read the same flag now. - The mode check goes through the capability seam, not `env.APP_MODE`. This route and the InspectorHub#308 gate landed in the same round; the first draft branched on the mode directly and became that gate's tenth violation. It reads `profile.hasContentMarketplace` instead — file-disjoint plans can still collide through a shared invariant, which is not something a file-overlap matrix can predict. - The route needed a replay spec. The seeder is find-or-create by name, so duplicate library rows were never the exposure; what repeats is the `data.import` audit entry and a full re-scan of the bundled fixture. The spec was verified by removing the guard and watching all three assertions fail — including the subtle one, where a re-run honestly reports all-zero and the operator reads "nothing to add" for a click that added three rows. The route lives beside admin-data.ts rather than inside it: that file is 395 lines and this takes it past the 400 ceiling, the same reason and the same shape as the existing admin-data-import.ts. The external path is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…under it Tasks 1-10 of the CalDAV plan. Tasks 11-15 remain — see the reachability note at the bottom, which is load-bearing. The calendar surface had a provider abstraction in name only: paths defaulted `provider = 'google'`, the sweep filtered `eq(provider, 'google')`, credentials were a Google OAuth triple threaded by hand, and connect was hard-coded to an OAuth popup. Adding a second provider meant removing those assumptions first, so that is what most of this is. The instrument came before the refactor. `google-wire-parity.spec.ts` pins every Google request on the wire — full `toEqual` on method, url, headers and body per operation — and was proven to bite by dropping `singleEvents: 'true'` and watching it fail with an exact URL diff. Across all four refactor tasks NOT ONE of its `expect` lines changed; only the mock wiring did. That is the evidence that this is a refactor and not a rewrite. What changed shape: - Credentials are one opaque provider-owned handle. `resolveAuth` replaces callers reaching into a Google-shaped object, and a foreign handle throws rather than being coerced. - Every path reads the connection's provider instead of assuming Google. `getCalendarProvider(provider = 'google')` lost its default — a default that silently means Google is the same defect as a literal, and every caller already passed one. - Connect is a provider-chosen flow (`startConnect` / `completeConnect`), not an OAuth popup with a CalDAV special case bolted on. - CalDAV transport, discovery, calendar list and event-derived busy read, with Apple registered as a provider. One real bug, found by Task 9's failing test rather than by review: `availability_overrides.source` was compared against the literal `'google'` to decide timed-vs-manual busy, so an `'apple'` row blanked the entire day. The comparison is now synced-or-not (`!= null`), which is what the column means. Verified no migration is owed: the `source` enum going `['google']` -> `['google','apple']` is type-layer only per the Schema Rules, and `db:check` reports no drift (hand=96 generated=96). Two pieces of debt this does NOT clear, both recorded rather than hidden: `server/api/calendar-events.ts` still holds its own Google client and refresh because it needs event titles that `listBusy` deliberately does not carry — it is narrowed so a CalDAV connection contributes no events instead of sending an undefined refresh token; and the sweep's not-decryptable message still says "Reconnect Google Calendar" for any provider, left alone because changing it would alter a pre-existing assertion.⚠️ THIS DOES NOT SHIP APPLE SUPPORT. The provider works — registry, resolveAuth, completeConnect, listCalendars, listBusy — but there is no connect endpoint and no UI, so no user can reach it. That is the built-but-unwired pattern this repo has hit six times before; Tasks 12 and 13 are what close it, and nothing should describe Apple as supported until they land. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…r file uses The dead-code gate caught these the moment the full suite ran, and it was right: the CalDAV work landed Tasks 1-10 of 15, so the connect endpoint and UI that would have consumed this interface surface do not exist yet. All seven are used only inside their own file — as union members (CalendarOAuthCredentials / CalendarCalDavCredentials inside the exported CalendarCredentialPayload), as field types on already-exported interfaces (CalendarAuthType, CalendarConnectFlow, CalendarConnectSubmission), or as plain internal helpers (escapeXml, appleMaterialOf). Consumers read all of it structurally through the exported parents; `api/calendar.ts` branches on `connectFlow.kind` without ever naming the type. Un-exporting is knip's preferred fix and it keeps the baseline honest: it stays at 2 entries rather than growing to 9 with a note promising someone will come back. Whichever of these Task 12 needs to name, it re-exports in the commit that needs it — a one-word change made where the consumer is visible. `appleMaterialOf` also now matches its Google sibling, which the same refactor deliberately kept module-private. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
… that was too small
Both surfaced in the full suite, neither in pre-commit.
`resolve.spec.ts` — 13 failures, and a real one. `resolveVideoProvider` now reads
the deployment capability (`c.var.profile.videoBackendManaged`) rather than
`env.APP_MODE`, but the spec's `mockCtx` built a context carrying only
`{ tenantId }` in vars and no `var` accessor at all — so every case died on
`Cannot read properties of undefined (reading 'profile')`. The mock satisfied
the old read and nothing else.
The fix derives the profile from the same `mode` the helper already takes, and
exposes both `c.var` and `c.get`, because Hono has both and a mock offering one
silently breaks whichever caller reaches for the other. This is the failure the
mode-branching plan itself names in `login-saas-bounce.test.ts`: a mock that
does not match production is not a test. It went unnoticed here because the
spec kept passing against a resolver it no longer resembled.
`event-results-received.spec.ts` — timed out at the 5s default under full-suite
load while passing alone, which reads like a flake and is not one. Measured in
isolation: 2769ms for the FIRST test, ~200ms for the other two. The first pays
one-time warm-up — better-sqlite3's native addon and the drizzle module graph —
and the per-test budget is charged for it. One fork per spec file across 8
cores roughly doubles that under contention, so it crosses 5s and which file
loses the race is chance. The budget is the wrong size, not the test; raised to
30s with the measurement recorded next to it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
Found by clicking the button in a browser, which is the only rung that could
have found it: the string is grammatically wrong exactly when count is 1, and
nothing in the unit suite or the gates reads copy.
Both locales had it. English said "Added {count} new items."; Spanish said
"Se agregaron {count} elementos nuevos.", where the verb, noun and adjective
all inflect. Rather than invent a mechanism, both now use the caller-supplied
{plural} placeholder this repo already uses in eight other messages
("{count} defecto{plural}", "{count} item{plural} selected"). Spanish takes it
on all three inflected words, which is grammatical in both numbers and needs no
second parameter:
en Added {count} new item{plural}.
es-419 {count} elemento{plural} nuevo{plural} agregado{plural}.
Verified in Chrome against a real workspace: the install path returns a success
Banner, a second install returns the "Nothing to add" info Banner rather than
sitting inert, /library shows seven tiles with no Marketplace, /library/marketplace
404s for an authenticated user, and the command palette returns nothing for
"market". All correct in both light and dark.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…contractor-type round Three plans landed in one round and each grew a file it already owned. Bumped rather than split because none of the growth is a new responsibility — the services gained methods their own headers already describe. Committed alone: a baseline is gate configuration, and mixing it into a feature commit escalates that commit's type-check tier for nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
… reach the UI Two halves of one gap. The delete stays permitted — the schema calls this soft reference acceptable and published reports store the resolved label, not the id. What was missing is that the tenant was never told how many comments a delete would orphan. A new preflight route answers that, and one test pins the delete as STILL SUCCEEDING so a later reader does not "fix" the disclosure into a 409. trade_slug shipped invisible: the column existed but was absent from ContractorTypeSchema, which is the response schema for every list/create/update route, so nothing could render it. It is read-only on the API — the slug is the seeder's to assign, and a tenant-settable one would let two rows claim a canonical trade and surface as a constraint error rather than a validation one. `create` now writes it explicitly as null; the previous `row as ContractorType` cast was serving a shape the schema says cannot exist. Two things the plan got wrong, both about the tree rather than the design: `messages/en.json` has not existed since the catalogue was split per-module, and the ICU plural it specified would ship as a literal string — Paraglide does not support it and there is not one ICU plural in the catalogue. The house convention (paired _one/_many keys chosen at the call site) is used instead, and es-419 is translated because an untranslated key fails lint:i18n-catalog, which the plan never mentions. The two data migrations this work needs are numbered with the rest of the round's chain in the following commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
… and the migration chain Three plans that turned out to share files (marketplace.ts, template.service.ts) and one migration chain, so they land together rather than in a broken sequence. ## One catalogue (InspectorHub#293, Tasks 1-4) marketplace_libraries gains three browse axes because the legacy `category` was free text spanning a property type, a jurisdiction's form standard and an inspection kind — production confirms it: 7 `residential`, 2 `new_construction`, 1 each of `condo`/`commercial`/`trec`. One column could only ever describe one of the three. The 12 rows move over with ids intact, then the legacy pair is dropped: `tenant_marketplace_imports` was 0 rows, so there was no import history to preserve, and dropping it also removed the only FK pointing INTO `templates`. `importCatalogEntry` replaces importTemplate + importLibrary and branches on kind. Tasks 5, 8 and 9 are deliberately NOT built: Task 9 would have folded the catalogue into /library/templates as a tab, which is not mode-gated, silently turning the SaaS-only 404 that landed hours earlier back into a 200. ## Template mutations are gated (InspectorHub#307, Tasks 1-6, 8-9) TOGGLEABLE goes 5 -> 9. Nine mutation routes now carry a capability — the plan said eight; `POST /libraries/{id}/import` also mints templates and its enumerating grep missed it because that code writes `insert(templates as any)`. The delete guard's original justification died with the legacy tables, so it rests on a better one: `importCatalogEntry` is idempotent on the marker, so deleting an imported template makes the pack permanently un-reinstallable. ## The chain, 0057-0062 Numbered here because three plans each owed migrations and the numbering is not theirs to choose.⚠️ ORDER IS LOAD-BEARING: 0057 adds the columns, 0058 moves the rows, 0059 drops the legacy tables — collapsing them into one generated migration would leave nowhere for the row move. 0062 backfills the four production inspections that name a template but carry no snapshot, and it MUST precede the fallback retirement in this same commit: `computePublishReadiness` calls `requireTemplateSnapshot`, and `getInspectionHub` composes it, so one such row 500s an entire hub page rather than one report. The design assumed the true structure was recoverable from each row's signed report_versions; 3 of the 4 have no report_versions row at all, so the migration copies today's schema and records in its own comment what that trades away. Two things found only by applying the migrations rather than reading them: `db:generate` emitted the DROPs parent-before-child, the unsafe order, so the hand-written child-first bodies were kept; and the canonical-set migration failed on the real D1 path with "too many terms in compound SELECT" after passing a better-sqlite3 harness — rewritten as twelve guarded INSERTs, since a multi-row VALUES is implemented AS a compound SELECT and would hit the same ceiling. Verified after applying all six locally: browse axes present, legacy tables gone, 22 contractor types at 20 distinct slugs with 0 duplicate (tenant,slug) pairs, and the backfill proven idempotent and non-clobbering against every row shape. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…dividually
The tenant-scoping gate flagged five new unscoped by-id queries in
marketplace.service.ts. This is the cross-tenant leakage gate, so each was
checked against the schema rather than waved through as a batch:
4× marketplace_libraries.id — that table has NO tenant_id column. It is the
global catalogue, shared by every tenant by design, so there is nothing to
scope by. This is the gate's "truly global table" case.
1× tenantLibraryImports.id — that table IS tenant-scoped, so it needed a real
answer. `existing` is read at updateTemplateImport's own prior query, which
filters on eq(tenantLibraryImports.tenantId, this.tenantId). The id is
therefore a pk from a prior scoped fetch, which is the gate's first
sanctioned exception.
Committed alone so the reasoning is not buried in a feature diff, and because a
security baseline should be reviewable on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
The dead-code gate caught both in the full run, after all three feature commits passed their hooks cleanly. `templateBlockedError` is called once, at the delete guard in its own file, so it loses the `export` rather than the baseline gaining an entry. `MarketplaceBrowseQuery` was used nowhere at all — not even inside the file that declared it. It is a z.infer alias over a schema that callers already consume directly, so it is deleted rather than un-exported. If a later task needs to name that type, one line brings it back where the consumer is visible. Baseline stays at 2 entries either way, which is the point: this is the second round where stopping to check each finding cost less than the note-to-self a baseline bump would have left behind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…d placeholder
My own Round 1 fix, caught by the first full lint run since it landed.
Fixing "Added 1 new items." I gave English one {plural} and Spanish three, since
Spanish inflects the verb, the noun and the adjective. That is valid at runtime —
the same value substitutes three times — but lint:i18n-glossary compares
placeholder MULTISETS between source and translation, so [count, plural] against
[count, plural, plural, plural] reads as a translation that changed the contract.
The gate is right to be strict: it cannot tell a deliberate repeat from a
copy-paste that duplicated the wrong token.
Both languages now use the repo's actual plural convention — paired _one/_other
keys chosen at the call site, as in booking_unit_inspection_one/_other — which
carries no placeholder in the singular and one {count} in the plural, identically
on both sides. Spanish gets a properly inflected pair rather than a mechanical
one:
Added 1 new item. Se agregó 1 elemento nuevo.
Added {count} new items. Se agregaron {count} elementos nuevos.
Worth recording WHY this escaped Round 1's suite: the pluralisation was part of
the Chrome follow-up, which landed AFTER that round's full run. Any fix made
after the round's gate is ungated until the next one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
Ninth-inning fixture debt from retiring the template-snapshot fallback (InspectorHub#307). getRepairList reaches getReportData, which now REQUIRES the per-inspection snapshot instead of falling back to the live `templates` row — so a fixture naming a template without one throws where it used to silently resolve against today's schema. Nine tests, one cause. The plan's own task updated four such specs; this is the fifth and it was found by the full run rather than by the sweep, because the sweep looked for specs that read reports and this one reads a repair list that happens to compose one. Swept for the same shape afterwards: five other specs name a template with no snapshot (booking-people, inspection-patch-settings, inspection-request-people, inspection-request.service, plan-quota-guarded-services). All pass, because none of them reaches the report path. Left alone deliberately — adding a snapshot a spec never reads is noise, and the suite is the arbiter of which ones need one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…rectory (InspectorHub#106) A gate for two things the codebase could not previously see, and one worked directory as the reference conversion. **Call shape.** The literal `fetcher.submit(` finds barely half the sites: of 154 call sites in 83 files, 74 name the variable `fetcher` and the other 80 wear 53 distinct identifiers (`coverFetcher`, `deleteFetcher`, `credFetcher`, …). The rule matches `\w*[Ff]etcher\.submit(` instead. **busy.** `useGuardedSubmit` hands back a `busy` flag that a consumer can simply not thread, leaving a double-submittable button that looks guarded. The rule reads consumers and fails when `busy` is never used. Four things the plan had wrong, each proven rather than assumed: - **157 sites / 84 files is 154 / 83** — three of the "sites" are the string inside a comment or the hook's own docblock. - **Line-based keys collide.** Keying on the matched line collapsed 154 sites to 149, because most calls wrap their arguments and the matched line is a bare `fetcher.submit(`. Keys now balance forward to the closing paren, with an ordinal for two byte-identical duplicates, and the gate fails closed if the raw-site count ever diverges from the key count. - **Two files consume the hook, not one.** `settings-data.tsx` uses a non-destructured `install.busy` shape the specified rule could not see; without extending it, the rule passed that file vacuously. - **The plan's reconciliation identity is unsatisfiable** until the conversion is finished. Resolved by printing the debt instead of hiding it: every run now reports `51 exempt, 94 awaiting conversion` rather than one number that cannot distinguish them. The exempt count is 51, far above the spec's estimated 17, and the reason is structural: 22 sites carry FormData/JSON payloads that `useGuardedSubmit`'s `Record<string,string>` cannot express, and ~12 more share a fetcher with one that does. Directory 1 (`app/routes/settings-*`) is converted as the reference: 9 sites in 5 files. Two real defects fell out — `ContractorTypeRow` short-circuited inside `move()` but never disabled its chevrons, and the QBO pause/disconnect buttons had no disabled state at all.⚠️ 94 sites across ~60 files remain. The baseline names every one with the mutation it performs, so the next pass has a worklist rather than a grep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…spectorHub#107) The old gate proved coverage by grepping spec files for a route path as a string literal. That answers "did someone write this path down", not "is this route covered" — and it made 302 routes permanently `pending`, which is a burn-down list nobody can burn down. Coverage now comes from a table-driven suite generated off the same walker the gate uses, so the two cannot disagree about what the route surface is. Result: 344 mutating routes resolved, 328 verified by the table, 16 by design, 0 pending. Three premises in the plan did not survive contact: - **Its verification mock is broken as written.** `drizzle: vi.fn()` returns undefined, the guard passes that as `claimKey`'s first argument, and `expect.anything()` rejects undefined — so all 344 rows failed on the db handle rather than on anything the test meant to check. - **Its expected red for `{id}` vs `:id` normalisation never happened** — that worked first time, and the anti-vacuity cross-reference against `app.routes` passed with zero missing rows. - **Deleting a by-design entry no longer fails the gate.** Under the new definition it reclassifies the route as *verified*, so the by-design list lost all enforcement teeth. Replaced with direct policing — stale entries and reason-less entries both fail, for `uncoveredByDesign` and `tableExclusions` alike — and proven red with a fabricated key. The 16 by-design reasons are rewritten to answer the new definition's questions explicitly. Three of them are really "the write is a no-op", which is a third answer the two-question definition does not have; that is said in each reason rather than forced into the nearest box.⚠️ Task 8's spec-pruning step is NOT done, deliberately. Its rule — a replay spec survives only if its route is in `uncoveredByDesign` — matches none of the 15 existing replay specs, because they cover invoices, pay-splits, SMS, agreements and service lines. Applying it deletes all 15 (~2,600 lines) and leaves zero end-to-end proof that a replay returns the stored response, contradicting the plan's own header that "the end-to-end replay specs remain the proof". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
Config change, committed alone: staging package.json escalates the pre-commit type-check to the full tier, so mixing it into a feature commit costs the widest run for nothing. Adds the script, appends it to `lint` and `lint:gates-full`, and registers it in SCRIPT_GATES so pre-commit runs it — verified via `node scripts/run-gates.mjs --only submitguard`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…spectorHub#98 items 1-3) A new SaaS tenant cannot spend the platform's sending reputation in its first 24 hours. The refusal is typed and named, so it reads as a deliberate skip rather than a delivery failure, and the UI says when it lifts and how to opt out by bringing your own provider. Scope is deliberately narrow: only platform-funded outbound. A tenant on its own Resend key is unaffected, which is the escape hatch the banner points at. SMS gets no equivalent rule, and that reason now sits next to the SMS gate rather than being an absence someone has to rediscover.⚠️ The plan's central premise for the session-context change was FALSE, and Round 1 is what made it so. It says widening the projection "costs nothing — the row is already being fetched"; `session-context.ts` no longer selects from `tenants` at all, because routing that file through the capability seam moved the read inside `resolveVideoProvider`, which fetches tier/status only on the managed branch. There is no row in scope to widen. A dedicated primary-key read, guarded on saas + tenantId and failing open, is used instead — widening a video resolver's return type to carry an email anchor would couple two unrelated things for one query. The plan's own docblock text would also have failed the isolation gate: it says "never off env.APP_MODE", and that gate matches `.APP_MODE` in PROSE. Reworded. That trap has now caught three different authors, which is worth more than the fix itself. Four tree corrections beyond that: two imports the plan's snippets omitted; a missing `// @vitest-environment happy-dom` pragma without which all four component tests die on `document is not defined`; the `getDeploymentProfile` cast the plan specifies is no longer needed since the parameter was narrowed; and a `SessionContext` test fixture in `connected-apps.test.ts` that the new field broke in three places. Two things this commit fixes that the plan did not ask for, both found by gates rather than review: - `outbound-cooling-send-gate.spec.ts` typed its subject as "success OR error", so every `err.details` assertion would have read `undefined` — a green test about a gate that had silently opened. It now rejects the resolved path explicitly. - `base.ts` crossed the 400-line ceiling. Rather than bump a baseline for a file that had never been over it, three pure helpers moved to `./html-helpers.ts`. `escapeHtml` already had a consumer outside this directory, so keeping it in a class file made that borrowing look like a reach into a service.⚠️ NOT verified in a browser, and it cannot be locally: the banner renders only when `mode === 'saas'`, and SaaS login bounces to the portal, so a standalone dev server can never show it. What IS verified: lint:contrast clears the flagged `text-ih-primary-text` on `bg-ih-info-bg` pairing at 4.5:1 across all three themes, the component has its own test, and so does the session-context resolver. Whether it looks right on a real SaaS tenant needs a deployed check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…still resolves
Two names, deliberately not one. `legalName` is the contracting entity —
agreements, invoices, TCPA filings. `companyName` is the trading brand — reports
and UI. The product already collected the first one in the SMS compliance wizard
and threw it away.
The fallback (legalName → companyName) lives ONLY in `getBrand`, so no call site
can forget it, and whitespace counts as unset. Clearing the field stores NULL.
## What was actually broken
- **The standalone signing page painted the platform's brand.** A client signing
a binding agreement saw "OpenInspection", while the SAME envelope reached
through checkout showed the tenant's name.
- **Both TCPA-adjacent SMS surfaces read `tenants.name`** — written once at
provisioning and never updated, so it diverges permanently from the name on
every other document.
- **`{{company_name}}` resolved to the platform name at SEVEN call sites.** The
plan said six; `event.service.ts`'s `results_received` trigger was the seventh.
Unset now yields `''`: a blank sign-off is a visible gap, a platform sign-off
is a false statement about who wrote the email.
- **Renaming a company retroactively rewrote which entity every past agreement
was signed with**, and the Certificate of Completion named no entity at all.
Two columns freeze it at signing — deliberately NOT inside `contentSnapshot`,
because `contentHash` is taken over the stored string and folding a name in
would invalidate every signature ever collected. Proven by trying it and
watching the hash-invariance guard go red. NULL renders as "not recorded";
backfilling would assert something untrue about what was signed.
- **A public link carrying an old slug 404'd.** Core records the previous slug on
the tenant sync it already receives, and resolution falls through to it only
after a live-slug miss, so history can never shadow a real tenant. That path is
deliberately UNCACHED: `tenant:<slug>` is keyed on the REQUESTED slug, so
warming it from a history hit would serve the previous owner to whoever later
claims that slug, invisibly, for a full TTL.
## Two premises that did not survive
**Slug-history Task 6 was already done.** Its premise — that the provider only
clears `tenant:<old-slug>` when an existing tenant renames — is false; the delete
is unconditional on both branches. The test was written anyway and PASSED BEFORE
ANY CODE CHANGE, so it is a pin on existing behaviour, not a fix.
**Legal-name Task 7's two surfaces are one.** The public invoice printed no
issuing party at all, so this is an addition rather than a swap. The "agreement
body template's company token" does not exist — there is no token substitution
anywhere in the agreement render path.
Three files crossed the 400-line ceiling for the first time and were SPLIT, not
baselined: `schema/tenant/core.ts` (email templates out), `inspection/shared.ts`
(fireAutomation out, re-exported so existing `vi.mock('./shared')` still works,
ratchet TIGHTENED 518→499), and `api/sms.ts` was already at its cap so the new
resolver went to its own file. Only long-grandfathered files took line bumps.
Four defects the type-check caught that the commit tier would not have: a
`logger.warn` called with `logger.error`'s three-argument signature, an inert
config literal missing the new field, a spec handing a wrapped
`DrizzleD1Database` to a handler that takes a raw `D1Database` and wraps it
itself, and four `handleTenantUpdate` calls missing the required `status`.
⚠️ One decision is still owed and is NOT taken here: core has nine readers of
`tenants.name` that could move to `tenant_configs.companyName`. Moving them is
nine unplanned edits with their own test surface; declaring the column frozen is
a product policy. The census is verified — three of the paths the plan gives are
wrong and it misses a ninth in `portal/integration.routes.ts` — but the choice
belongs to a human.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
The communication GET now reads `legalName` through `services.branding.getBrand`, because that is the ONLY place the legalName → companyName fallback lives — putting it anywhere else would let a call site forget it. This spec's service stub supplies `getBranding` but not `getBrand`, so the route 500'd with `getBrand is not a function` and two tests read the failure as a bad status and unparseable JSON. The default goes in `buildApp` rather than in each test: five call sites pass different stubs and only two exercise the GET, so per-test additions would leave the next GET test to rediscover this. Spread last, so a test that cares can still override it. Found by the full suite; the commit-tier check cannot see it because the stub is structurally typed through `as unknown as`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…plan predicate Two plans land together because their halves only work as a pair: 2026-08-01 managed-ai-provider-tier Task 6 Part B, and 2026-08-07 managed-ai-paid-tier Tasks 1 and 3. `checkAiQuota` was written, tested, and read a delivered cap while having ZERO production callers — so a configured allowance was inert and nothing anywhere reported that it was. It is now called at the same chokepoint as the meter: check before the send, meter after success, so a failed model call never spends an allowance and an over-cap tenant never reaches the provider. Entitlement stops being the hardcoded `managedEntitled: false` and comes off the tenant's plan through ONE literal. Four surfaces read it and one is portal-facing: `GET /api/integration/ai-provisioning` changes what the operator console reports from a one-line change in this repository, with no portal deploy and no portal commit. Worth saying out loud in the PR. `video/resolve.ts` carried its own copy of "is this tenant paying"; both callers now use `isPaidPlan`, because two copies is the shape that ends with one of them believing a trialling workspace pays. Behaviour is unchanged today. The capability gate still refuses every managed output class, so no managed call reaches a provider. Flipping it is 08-07 Task 6 and is blocked on the privacy-policy notice window, not merely on Task 4 having run. tests/unit/ai/build-ai-service.spec.ts is new, and covers the link nothing did. Replacing the pre-flight injection with `undefined` — unwiring enforcement completely — left the existing AI specs at 39 passed, because every one of them builds an AIService by hand and none goes through the builder production uses. That is the same defect Part B exists to fix, one layer out. Positive control with the spec in place: 2 red. docs/compliance/ai-data-flow.md states from the code, not from the plan, what leaves the process and under which Google terms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…ally typed
Nine display reads took the company name off `tenants.name` — the container
name portal writes at provisioning — rather than `tenant_configs.company_name`,
which is what the tenant types into core's own settings. The two are allowed to
diverge and do: portal's rename sync writes company_name only when it is EMPTY
("initialize-only, never overwrite"), so the moment a tenant edits it in core
the two part company permanently and by design.
Measured against production on 2026-08-11: of 16 tenants, 11 match, 1 has
diverged, and FOUR have no company_name at all. That last number is why this is
a COALESCE and not a swap — reading company_name plainly would have rendered
four companies with a blank name in the agent directory, invite emails and
public profiles, which is a worse bug than the stale name being fixed. The one
diverged tenant has a container name that is an email local-part.
The rule lives in one place (`server/lib/tenant-display-name.ts`) and every
consumer LEFT JOINs tenant_configs — LEFT, because a tenant with no config row
must still show its container name rather than vanish from the result set.
Two corrections to the original census. `integration.routes.ts:276` is NOT a
display read: it reports the container name back to an M2M caller for seeding,
and deliberately stays on `tenants.name`. And the agent directory's `orderBy`
needed the same treatment as its `select` — sorting on the raw column would file
"important.new" under I while the row on screen reads "OpenInspection".
Positive control: mutating the rule to a bare company_name read turns 4 of the
6 new tests red.
The file-size baseline moves referral.ts 564 -> 566. It is already grandfathered
well past the 400-line cap and this change adds a join and an import; splitting
it is a refactor a display-name fix does not justify.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
… cover The core half of Privacy P3 (portal InspectorHub#86). Portal could raise a DSAR for a non-account data subject and had nothing on the other end: `cmd.subject.export` and `cmd.subject.erase` had no consumer, so every one parked as unknown-type-or-version while portal's request sat at `fulfilling`. Export assembles subject-scoped rows across 20 collections and streams the ZIP into EXPORTS_BUCKET through a writer now shared with the tenant export (data-export.service.ts 245 -> 178 lines). Erase runs the existing orchestrator and replies with a COVERAGE DISCLOSURE, because "completed" on its own is an unbacked claim that everything was erased. Three limits are stated rather than papered over. `executedTables` cannot distinguish "matched nothing" from "not reached" — the orchestrator records a decision only when a step changed rows or threw. `manifestRuleCount` describes a document, not the run, and rides with `catalogueIsAdvisory: true` because 10 of 56 rules are catalogued but unenforced. And erasure matches on email only, as portal already states; `ERASURE_SUBJECT_AXIS` names the eight email-keyed predicates and the command rejects a phone at the boundary. There is no field for a partial run, so a failed step emits NO reply and throws: the command retries (erasure is idempotent) and the request stays visibly at `fulfilling` next to a failed command. A reply would have been readable only as success. The refusal now logs WHY each table failed, not just which — a DSAR stopping here has a statutory clock running and this line is the only trace. DSAR commands are EXEMPT from the per-tenant stale guard, and this is the part portal should know about. The guard answers "has this tenant-field STATE been superseded?", which is right for slug/status/tier/quota. A DSAR is not state; it is an operation on behalf of a natural person, and it shares `tenants.cmd_seq` with every unrelated tenant command only because portal has one sequence per tenant. Left guarded, a quota sync that merely OVERTOOK the erasure in the queue would drop it silently, with no reply, until the Art. 12(3) month ran out. Nothing about an erasure is superseded by a seat-count change. Safe because both subject commands are idempotent and order-independent, and the seq advance is `lt`-guarded so an exempted command can never make a stale tenant update look fresh. It relies on max_concurrency 1, which wrangler.saas.jsonc already sets and already calls a correctness requirement. The workers spec's two last cases failed on their first real workerd run: the partial-run case replaced the shared `decisions` array wholesale and the reset restored only `.status`, while the refusal reads the DECISIONS. Each passed alone; two failed together. The reset now rebuilds both. No migration. Everything reuses processed_cmd_events, sync_outbox, erasure_log, tenant_configs and EXPORTS_BUCKET. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…napshot Same cause as adeee03, found in the rung that commit's sweep could not reach. Retiring the live-`templates` fallback (InspectorHub#307) made the per-inspection snapshot REQUIRED on the report path, so a fixture naming a template without one throws where it used to resolve silently against today's schema. Three tests, one cause: "This inspection has no template snapshot, so its report structure cannot be resolved." adeee03 swept for this shape afterwards and found five more specs — all under tests/unit/. It could not have found this one, because `test:workers` is not in the pre-push three-suite run and CI is where it would first have gone red. A hard requirement shipped past a fixture that violates it, gated by suites that never execute that fixture. Both seed sites now build the snapshot from the same object the template's `schema` is built from, so the two cannot drift into testing a mismatch no production inspection has. Swept tests/workers/ for the same shape: word-export-consumer is the only spec that seeds a `templateId`, and the three others that touch templates already carry snapshots. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
`portal-isolation.spec.ts` greps `server/` for `portal/integration.routes` as a CONTENT string, so it cannot tell a file that imports that route from one that merely mentions it in a comment. tenant-display-name.ts spelled the path out while explaining which call site deliberately does NOT use the new rule, and the gate read that sentence as a stray import. The information is worth keeping — the next reader should know the M2M seeding route stays on `tenants.name` on purpose — so it is named by role now, with the reason recorded in place so nobody helpfully restores the path. Found by the full run, not by the commit hook: this gate lives only in `test:unit`. Sixth time in these repos that a prose mention has tripped a gate that greps for code, and the first one that was mine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
CI's two e2e jobs failed on this PR and the cause was one line of setup, not
the code under test.
`globalSetup` ran `execSync('npm run db:migrate', { stdio: 'pipe' })` inside the
same try/catch as the optional D1 wipe. `execSync`'s default `maxBuffer` is 1 MB;
this release adds seven migrations at once, so applying the chain on CI's fresh
database printed more than that, the child died with **ENOBUFS**, and the catch
turned it into `WARNING: Could not reset local D1`.
The suite then ran against a half-migrated database. `POST /api/auth/setup`
returned 500 because drizzle's INSERT named `tenant_configs.legal_name`, a column
migration 0063 creates and that database never got. In the other job the same
missing schema starved the seeded fixtures and twelve UI specs timed out waiting
for content that was never written.
The numbers are the point: **39 specs "passed", 1 failed, and 151 never ran** —
against a database that was known-broken 15 seconds earlier, in a log line
formatted as a warning.
Two changes. `maxBuffer` is now 64 MB, because migration output grows with the
chain and this failure stays invisible until it is expensive. And the migrate
moves OUT of the soft-failure block: a database that did not migrate makes every
assertion after it meaningless, so it now throws. The wipe and the KV clear stay
soft — those really are best-effort.
This repo is fail-closed everywhere else (`check-migration-lag.mjs` treats an
unreadable database as an error, never as "no lag"). This was the one place that
was not.
Proven: with the migrate command pointed at a script that does not exist, the
run now aborts with `[globalSetup] db:migrate FAILED` and exit 1. Before, the
identical failure printed a warning and 24 specs went on to "pass".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…comments `seedDefaultComments` built its eight rows as eight `UNION ALL` arms of a derived table. On real D1 that fails every time with `too many terms in compound SELECT` — D1's `SQLITE_MAX_COMPOUND_SELECT` sits far below SQLite's stock 500 — and the failure was caught and logged as a WARN so first-run setup would not break. So it never broke, and it never worked either: every workspace created through `/api/auth/setup` has been getting an empty comment library, with the only trace a log line in a stream nobody reads. Not introduced by this release — `git log` on this file shows the last change to it added comments only. Found while reading an e2e run's output for an unrelated failure, which is the whole argument for reading the output rather than the summary line. Fixed the way the marketplace seeder was fixed when the same ceiling bit it: one guarded INSERT per row in a single `batch`, keeping the NOT EXISTS guard so re-running stays safe. A multi-row `VALUES` would hit the same limit. Verified by the absence of the warning: the run before this change emitted `seedDefaultComments.failed D1_ERROR: too many terms in compound SELECT`, the run after it emits nothing and the same 24 specs pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
The dead-code gate caught this, correctly. `ts-morph` was in devDependencies for `backfill-route-metadata.ts` and `backfill-zod-descriptions.ts` — two codemods removed earlier in this branch because they pointed at a `src/` directory this repo does not have and could never have run. Nothing else imports it: a repo-wide grep outside package-lock.json returns the package.json line alone. Removed with `npm uninstall`, which edits the lockfile in place — the multi-platform entries survive (346 linux references), so the ubuntu CI runner still resolves its binaries. This gate lives only in the full lint run, so pre-commit could not have seen it; CI is the rung that does. That is the ladder working, not a gap in it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
ensureSeeds now inserts each seed's message_templates row(s) directly and carries only their ids on the automations row, instead of writing subject_template/body_template/sms_body and relying on the separate backfillAutomationTemplates pass to migrate them afterward. create()'s empty-string tombstone for the dead body columns is gone too. subject_template/body_template were still NOT NULL with no default, so "stop writing them" required relaxing that constraint (migration 0067) — they were already marked DEAD and unread elsewhere. backfillAutomationTemplates (staying for now, a later task removes it) gained a fallback to the seed's own smsBody when the dead sms_body column is empty, since ensureSeeds no longer populates it — a tenant that enables the SMS channel on a freshly-seeded rule still gets its TCPA copy backfilled correctly (automation-seeds-sms.spec.ts). The standalone (/setup) raw-SQL seeder also stopped writing the three columns, and instead inserts its own message_templates rows up front so a self-hosted tenant's lifecycle emails keep their real copy instead of going out blank. Extracted into standalone-seed-automations.ts (plus a shared standalone-uuid.ts) to keep standalone.ts under the file-size gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
… drop
migrations/0067_thin_the_fury.sql was drizzle's generated twelve-step
rebuild (CREATE __new_automations -> INSERT SELECT -> DROP TABLE ->
RENAME), which the plan's global constraints forbid outright: D1 has no
PRAGMA foreign_keys=OFF outside a transaction, and DROP TABLE on an
FK-referenced table loses rows. automations happens not to be
FK-referenced, so this file would likely have survived, but the shape
must never enter the chain.
It also had no reason to exist: it relaxed subject_template/body_template
off NOT NULL so ensureSeeds could stop writing them, but this replacement
drops all three columns (subject_template, body_template, sms_body)
outright now that every pre-existing tenant has been drained into
message_templates. Relaxing a constraint on a column about to disappear
is a rebuild against production for no durable value.
Replaced with a hand-written native ALTER TABLE ... DROP COLUMN (same
number, descriptive name) - none of the three columns carries an index,
the one thing SQLite refuses a native drop for. Regenerated the meta
snapshot via db:generate and kept the hand-written SQL; db:generate
happened to emit the identical native drop, confirming the current
drizzle-kit no longer needs the rebuild shape for a plain column removal.
message-template-backfill.ts read the two email columns directly
(a.subjectTemplate / a.bodyTemplate) and read a.smsBody as a fallback
ahead of the matching seed's smsBody - both now-impossible once the
columns are gone. Gave the email branch the same seed-only fallback the
SMS branch already had (matched on name+trigger, the same key
ensureSeeds uses), rather than leaving it silently reading undefined.
backfillAllTenants/runAutomationTemplateBackfillOnce (the cron sweep)
are themselves now candidates for deletion per their own doc comments
("delete once the drained columns are dropped") but are left alone here
as a separate, larger change.
Updated the schema (dropped the two DEAD comment blocks describing
columns that no longer exist) and every test fixture that inserted
subject_template/body_template/sms_body directly into automations rows -
most needed no more than deleting the dead fields, but a handful relied
on backfillAutomationTemplates' old row-embedded-content fallback to
populate an email/SMS template and now create that message_templates row
directly, the same way ensureSeeds does for a freshly-seeded rule.
Verified: db:migrate applied cleanly against local D1, and the printed
automations DDL has no subject_template/body_template/sms_body column at
all (checked by reading the real CREATE TABLE, not by string-matching for
backticked column names). db:check reports no drift.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PZiNbozQRKbrmoyggQuNmh
…d missing coverage
Fix wave from a review of the automations-copy-columns removal:
- I-3: standalone-seed-automations.ts wrote automations.created_at in
SECONDS (unixepoch('now')) into a timestamp_ms column, reading back as
1970. Reuses the same millisecond expression as the message_templates
inserts a few lines away.
- I-4: replaced the up-front SELECT + unrelated inserts with a WHERE NOT
EXISTS guard on all three statements (both message_templates inserts and
the automations insert), batched per row via db.batch() so a mid-sequence
failure can't leave a template orphaned and two concurrent callers can't
both pass a stale check and double-seed. The analogous window in
ensureSeeds (core.ts) is recorded as a follow-up, not closed here.
- I-2: standalone-seed-automations.ts had zero coverage in any suite (the
unit-test D1 stub lacks .first()/.batch(), and the only specs touching
StandaloneProvider.handleTenantUpdate construct it with a stub DB that
swallows every row as a caught, logged warning). Added a real-workerd spec
exercising the file against real D1 with replayed migrations, covering
correctness, sequential idempotency, and concurrent-caller idempotency.
- I-5: check-seed-sql.mjs never scanned server/, where this raw SQL lives.
Added it; baselined one unrelated pre-existing violation it surfaced in
marketplace.service.ts.
- I-1: verified (not fixed — already correct) that the prior agent's new
discriminating test case in seed-writes-templates.spec.ts actually fails
when the direct-write path is removed, rather than passing vacuously.
- M-1..M-5: corrected a false claim about which columns SQLite refuses a
native drop for, an elided FK/transaction mechanism, and four comments
left describing dropped columns as still-present dead columns.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…wrong and nobody read data_version was frozen (schema comment: stop writes once the DO is the authority) and that condition now holds — the collab DO owns inspection_results and never bumps this counter, so it has been silently wrong since the DO became authoritative. Remove its one remaining writer (the results-batch path), drop the unused projection in the public report download route, and correct two comments that described caching behavior in terms of a counter that neither is bumped nor was ever read. The schema column itself is untouched here; a later task drops it.
comments.rating_bucket and automations.sms_body are DEAD (frozen, no
reads/writes) with severity and sms_template_id as their live successors.
Rewrite the three specs that still exercised the dead columns so the
coverage follows the behavior instead of vanishing with it:
- sms-schema.spec.ts: add the missing assertion for smsTemplateId (the
smsBody assertion was already dropped in an earlier task; this schema-
surface spec never picked up its replacement).
- comments.spec.ts: all round-trip/backward-compat/filter/tenant-isolation
cases now use severity ('good'|'marginal'|'significant'|'minor') instead
of ratingBucket, matching what /api/admin/comments actually filters and
sorts on today.
- comment-list-sort.spec.ts: the ORDER BY fragment for the default/relevance
sort now reads 'severity, created_at DESC', matching the live query.
…ould not go
The eight columns the plan set out to retire are down to five: an earlier task
in this chain already dropped the three automations copy columns, and a second
removal of a column that is not there fails and takes the whole migration with
it. The live table text was read before writing a line of SQL, not the plan's
list -- and read in full, because the backtick-anchored probe that would have
answered "is this column gone" inverts silently on any column that arrived via
a bare column addition and therefore carries no backticks. `automations` has
one of those sitting in it.
What goes: comments.rating_bucket (superseded by severity),
inspection_events.gcal_event_id (superseded by calendar_external_links), and
inspections.lead_inspector_id / helper_inspector_ids / data_version (superseded
by inspection_inspectors and by the collab document's own state vector). Every
reader and writer was retired by the tasks ahead of this one.
The index goes first. SQLite refuses to remove a column an index names, and
idx_comments_rating_bucket names one of these -- so the index goes in the same
migration, ahead of the column, and out of the Drizzle schema in the same
commit. Leaving the declaration would have made the drift gate report drift
against a migration that had just dropped it.
The SQL is hand-written and stays that way. Drizzle happened to emit the same
native statements this time; that is a fact about this diff, not a licence to
trust the generator, whose alternative is a table rebuild that D1 cannot run
safely outside a transaction. The generated snapshot is kept for the chain
link, the generated SQL is not.
Prose that asserted these columns exist and are frozen is corrected where it
would now mislead: the type in admin-comments.ts that omitted a key which no
longer exists, and three comments naming a FROZEN column. Narration of the past
("it used to read", "already held that mapping") is left alone -- it is true,
and it is why the replacement exists.
One correction is worth its own note. library-replace.schema.ts said
user-modified rows are detected via the comments category or rating_bucket
field, and that the service refuses replace without acknowledgement. Neither is
implemented: replace deletes every row carrying the library_id whatever the
flag says, and confirmLossOfEdits is only recorded into import history. So
dropping rating_bucket removed no dimension from any check -- there is no
check. The comment now says what the code does and why the missing guard is
missing (a comment carries no edit marker, so an edited import row cannot be
told from a fresh one without a per-row edit timestamp).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
Four Minor review findings, all "text describing a world that no longer
exists, or never did." Applied after the frozen-column drop
(comments.rating_bucket, inspections.lead_inspector_id,
inspections.helper_inspector_ids among the columns retired):
- library-replace.spec.ts: retitle the "without confirmLossOfEdits" test —
it never asserted a "when no user-modified rows exist" condition, because
no such guard exists. MarketplaceService.updateLibraryImport's replace
mode issues one unconditional delete; confirmLossOfEdits is recorded into
import-history metadata and nothing branches on it. Keep the coverage
(replace succeeds without the flag), just say what it actually proves.
- report-version.service.ts, version-diff.ts: leadInspectorId and
helperInspectorIds are gone as columns but survive as request-payload
field names (schedule.schema.ts, wizard.schema.ts) that sync into
inspection_inspectors on write. Reword both comments from "is the query
face over X + Y" (storage that no longer exists) to "is where the roster
lives; X and Y survive only as DTO fields on the way in".
- can-access.spec.ts: the historical block ("It used to read...") had one
present-tense clause left in it — "are NULL and '[]' on every production
row" — describing columns that no longer exist to be queried. Past tense
now, matching the rest of the paragraph.
- collab-route-auth.spec.ts: the fixture built fake inspection rows typed
with leadInspectorId/helperInspectorIds and derived the default roster by
JSON.parse-ing them — a row shape the database can no longer produce.
authorizeCollab() never reads either field off the row (who-may-edit comes
from the separately-mocked getInspectionRoster); getInspection is called
only for the tenant/404 guard. Repointed the fixture at inspectorId/
tenantId only, same pattern as 32008c1's comments-spec repoint. Coverage
unchanged: same 15 cases, same assertions: the one case needing a helper
on the roster now passes `roster: { lead, helpers }` directly instead of
smuggling it through a fake JSON column.
Verified: npx vitest run tests/unit/comments tests/unit/collab --config
vitest.api.config.ts — 235 passed, 32 files.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
Sixty-nine migration files collapse into a single 0000_baseline.sql regenerated from server/lib/db/schema, with one meta/0000_snapshot.json and a one-entry _journal.json. Equivalence was proven before anything was replaced: an empty D1 replayed through the old 69-file chain and an empty D1 built from the new baseline both yield 370 sqlite_master rows, 98 tables and 272 indexes, and every one of those 370 rows is identical once whitespace and identifier quote style are normalised. The raw dumps differ (1570 vs 1652 lines) only because SQLite stores an ALTER TABLE ADD COLUMN verbatim into the table's CREATE text and rewrites the table name with double quotes when a table is rebuilt; no column, type, default, constraint, foreign key, index or column order differs. Production's ledger was realigned to match: the rows naming the superseded migrations were deleted, leaving only 0000_baseline.sql, which wrangler already considers applied. A remote migrate is now a no-op and no DDL re-runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
scripts/seed-pca-demo.mjs was 705 lines nothing invoked -- not package.json, not CI, not a hook -- and twelve of its column names had drifted away from the schema, so its first INSERT failed. Eight of those twelve were one mistake: snake_casing the drizzle property and losing the `is_` prefix that boolean columns carry, so `is_requested` was written as `requested`. Renaming twelve columns would have fixed nothing that matters. The script would have stayed uninvoked and rotted again -- no fixture, no test, nobody notices, still no fixture. So the data shape moves into tests/seed-fixtures.ts behind SEED_PCA=1, where globalSetup runs it on every seeded e2e and treats a failed seed as fatal, and where lint:seed-sql checks every column name against the schema before it can be committed. Proved before trusting it: renaming is_requested back to requested makes that gate fail and name the line. The script is deleted rather than wrapped -- every shape moved -- and the two places that cited it as the motivating example now cite this history instead. Baselined seed-SQL violations drop 21 -> 3. Seeded and verified against a real local D1, which the old script never got: dri=4 psq=1 cost=3 signoff=2, timestamps 13 digits (milliseconds, not the seconds it wrote), booleans landing in the four states the checklist keeps apart. Two new specs cover what those tables exist FOR rather than what columns they have. The ASTM E2018 8.6 rule -- a document asked for and never received is a limitation, and a limitation must be stated -- lived in a module-private function inside the Word-export queue consumer, so the one mapping in that file a reader of the finished report can see had no test. Extracted to server/lib/pca-document-review.ts and pinned there: a never-requested document is still a line, N/A reads as a decision with its reason, received-but-not- reviewed never claims Reviewed, and the seeded checklist is the whole catalog (asserted against its length, not against zero). The PSQ spec pins that the questionnaire's fate is always on the record: a declined PSQ is a ROW, since the refusal is the exhibit the 11.4.3 disclosure points at, and the same inspection id under a second tenant is a different questionnaire -- which is what the tenant_id half of uq_psq_inspection buys. Both specs were proved to fail first, by mutating the real service: filtering the narrative rows, clearing responses on decline, and dropping the tenant filter from getCompliance each turn the expected tests red. seed-test-user.mjs is left alone with its reason written into the file: it honours TEST_EMAIL/TEST_PASSWORD/SINGLE_TENANT_ID and seeds a different tenant, so folding it in would silently change which tenant a `npm run dev` login lands in. seed-local-e2e.mjs untouched. file-size baseline tightened: the extraction shrank report-export-consumer.ts.
…-gate threshold The 0000_baseline.sql squash (258c2d9) removed both migration files that license-backfill.spec.ts and repair-action-tag-backfill.spec.ts located by name and executed as real SQL. Both specs threw at collection time. license-backfill.spec.ts and repair-action-tag-backfill.spec.ts are deleted, not repointed at the baseline: both tested one-time backfills that transform data already sitting in a database, and a squashed baseline builds every fresh database from empty — there is nothing to backfill, and the baseline is generated from the current schema, not from migration history, so neither file's statements exist there to run. Repointing them would either fail or have to be rewritten into something vacuous. What each spec pinned that is still true was kept, elsewhere: - users.license_number stays gone. Moved to a new assertion in tests/unit/credentials/schema.spec.ts (parallel to the file's existing `not.toContain('expires_at')` check) — the one fact from the pair that is a live schema property rather than a property of the retired SQL. The label text, sort-order convention, and dedup behavior the spec also covered were already independently tested against the live CredentialService in tests/unit/credentials/service.spec.ts. - The repair-action-tag backfill's "credit implies fund" coupling has no live successor to move: server/services/repair-request.service.ts passes requestedCreditCents and repairActionTag through independently on both create and patch, with no code path inferring one from the other today. That coupling was strictly a one-time correction of pre-InspectorHub#275 data. migration-lag-gate.spec.ts's guard ("has real migrations to compare") is fixed, not deleted: its intent — don't let the fixtures below compare against an empty directory — still holds with one migration file. Threshold changes from `> 10` (sized for the old 57-file chain) to `> 0`. Reading the fixtures it protects turned up one dependent on the old count: the ".slice(-6)" fixture reconstructing the 2026-08-09 incident (a database missing four named features) degenerates with a single file — still non-vacuous, but no longer resembles "behind by 6" despite its own "(the 2026-08-09 shape)" label. Rewritten to drop that claim and assert the same code's fundamental boundary directly: a database that has applied nothing is missing everything, still built from the real migrations/ directory per the file's own rule against invented names. npx vitest run tests/unit/credentials tests/unit/repair tests/unit/tooling \ --config vitest.api.config.ts 33 files passed, 437 tests passed (was 3 files failed / 1 test failed). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
The baseline rebuild landed as `chore(migrations):`, a type release-please's node preset does not emit into the CHANGELOG at all. The one change on this release that an operator must act on by hand would not have appeared in the release notes the upgrade guide tells them to read. This commit carries the footer that puts it there, and writes the procedure down. `docs/self-host/upgrade.md` gains "Upgrading across a rebuilt baseline", and four of its existing statements are corrected rather than appended to: the forward-only framing, the claim that applied migrations are never hand-edited, the description of `db:migrate:remote` as applying what has not run yet, and the deploy pipeline, which omitted the `db:lag` gate that is what will actually stop the deploy. The same forward-only framing is corrected in CONTRIBUTING.md, docs/README.md and docs/develop/architecture.md. Three populations, one of which nothing detects. A database that carries only `0000_baseline.sql` — every operator who upgrades tag to tag — matches the new baseline BY NAME, so `db:migrate:remote` prints "No migrations to apply!" and `db:lag` reports "in sync". Both are counting names, both are right, and the schema never moves. The guide names that case first and tells the reader to treat it as a symptom. `scripts/migration-lag-baseline.json` is created. `check-migration-lag.mjs` documented it as the place to declare pre-rebuild migration names and `loadBaseline` returns an empty list when it is absent, so the documented escape hatch did not exist to open. Every list in it is empty on purpose: this deployment reconciled by rewriting its ledger, so no name needs forgiving, and the file says so and says what would belong there instead. The gate's "database is AHEAD" message named two causes, both of which blame the operator, and neither of which is true here. It now names the third and points at the guide. The retirement DDL is in the guide because it can be nowhere else: production dropped those columns through forward migrations that ran before the squash, and those files are gone. Nine columns, not eight — `tenants.name` was dropped in the same window and is one of the three that are NOT NULL with no default, so a database that keeps it fails `/setup` outright. The backfill that has to precede that drop is included too; for a workspace that never set a company name, that column is the only name it has anywhere. BREAKING CHANGE: `migrations/0000_baseline.sql` was regenerated and the forward files it now covers were deleted. Because `wrangler d1 migrations apply` matches by filename, an existing database applies nothing on upgrade and may report success while doing it. Before deploying: bring the schema current on the pre-rebuild chain, then `DELETE FROM d1_migrations WHERE name != '0000_baseline.sql'`, then apply the nine column retirements by hand (with the company-name backfill before `tenants.name`). Back up first — `d1 export --remote` and a Time Travel bookmark. Full procedure, including how to tell which case your database is in, is in "Upgrading across a rebuilt baseline" in docs/self-host/upgrade.md.
…omments The cron-driven sweep that drained automations.subject_template/body_template/ sms_body into message_templates has finished its job — production is drained and those columns are dropped — so per its own doc comments it is deleted: runAutomationTemplateBackfillOnce, backfillAllTenants (both in server/services/message-template-backfill.ts), the block 8 call site in server/scheduled.ts, and the two specs that only exercised them. backfillAutomationTemplates is kept: AutomationCore.ensureSeeds calls it unconditionally on every run (not just for freshly-seeded tenants), and it is the only writer of inAppTemplateId anywhere in the codebase. Its doc comment is rewritten to describe what it now is — a standing repair path, not a one-time migration aid — and its "gets an empty one instead of nothing, same as before" claim is corrected: with the copy columns gone there is no fallback copy left, so an unmatched rule now gets no template. Also cleans up five stale artifacts a final review flagged: - scripts/seed-sql-baseline.json: drop the entry for a spec file already deleted earlier in this branch (3 -> 2 baselined entries). - tests/unit/tooling/migration-lag-gate.spec.ts: replace fixture filenames that named real migrations deleted by the chain squash (0052_repair_action_tag.sql, 0055_dazzling_namor.sql, 0056_fluffy_fenris.sql) with synthetic names, drop the now-unnecessary migrefs-allow escapes, and update the header's narration of the 2026-08-09 incident to match the post-squash single-file baseline. - scripts/verify-migration-equivalence.mjs: the baseline-rebuild date citation was one generation stale (2026-06-21 -> 2026-08-12). - tests/unit/automations/automation-seeds*.spec.ts: the columns they call "DEAD" are dropped, not frozen — automation.schema.ts carries no DEAD comment for them. - docs/reference/database.md: the baseline schema is 95 tables, not "50+".
…clients twice The /setup seeder kept its own copy of the rules under a comment asking the reader to keep it "semantically in sync with AUTOMATION_SEEDS". It had already drifted: six rules were spelled differently on each side. ensureSeeds dedupes on (name, trigger), so the first time it ran on a standalone tenant it seeded all six AGAIN under the other name — two active report.published -> client rules, two report.amended -> client rules, and so on. Every client on that tenant got the mail twice, and since this release each duplicate also minted its own message_templates row, so the operator saw it in their library too. The seeder now derives its rows from AUTOMATION_SEEDS. That is close to free: the seed list is a plain const with no drizzle involvement, and this file already imported from message-template-backfill. What the standalone path genuinely needs and ensureSeeds cannot give it is the raw-SQL WRITER — the write-time NOT EXISTS guard, the per-row batch, the role-profile subquery — and that stays. Only the data moved. A drift test over two lists would have kept two places to edit and only reported the problem afterwards. Two rules existed ONLY in the standalone list (a buyer's-agent cancellation notice and a client payment receipt). Deleting them would have quietly removed working behaviour from self-hosters, so they are promoted into AUTOMATION_SEEDS with notification classes of their own; SaaS gains them through the normal top-up path. Fixes two silent divergences found while verifying: delay_minutes was hardcoded to 0 here, so Post-inspection follow-up and Review request fired immediately on a self-hosted tenant instead of 1 and 3 days later; and several same-named rules carried different body copy on each side, with whichever path ran first winning. SMS: the two suites asserted opposite things and the standalone one was wrong. ensureSeeds gates SMS template creation on channels including 'sms'; this seeder gated only on the seed carrying an smsBody. No seed declares an sms channel, so it was creating templates nothing could send — the outcome message-template-backfill.ts documents itself as avoiding for the email and in-app cases. The implementation is fixed and the workers spec's assertion flipped to null rather than relaxed. Two gates on two rungs replace the comment. standalone-seed-parity.spec.ts runs the real seeder against a recording D1 stub and compares what it would write, field by field, against the seed list; the workers spec reproduces the real sequence (/setup, then ensureSeeds) and asserts the rule list equals AUTOMATION_SEEDS before and after, by sorted-array equality so a duplicate survives neither. Both were proven to fail first: skipping a seed and dropping the channel gate reds the parity spec with the offenders named, and reintroducing the Report Ready -> Report Ready (Client) rename reds the workers gate with the exact diff. The seed list crosses the 400-line file-size ratchet at 423. Baselined rather than split, and the reason is this commit's own thesis: the gate's preferred fix is to break the file into focused units, which would put the seed rules back in two files immediately after consolidating them into one. The next person grepping for a rule name has to land somewhere unambiguous, and it is a flat data table, not the branching complexity the ratchet aims at. One entry added by hand rather than regenerating the snapshot, so no other file's cap moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…d a slow spec - marketplace.service.ts insertLibraryComments: bind Date.now() (ms) instead of seconds into comments.created_at (timestamp_ms), and write the section value into the `section` column instead of `category`. category is a real, independently-read column elsewhere (RecommendationService's repair-item vocabulary, the admin comment CRUD, the inspection-edit picker's relevance scoring) and was never meant to receive import data. Forward-only: existing corrupted rows are reversible (multiply created_at by 1000; move category into section) but not backfilled here — see .superpowers/sdd/marketplace-fixes-report.md for why. - pca-compliance.service.ts setPsqStatus: include sentAt in the onConflictDoUpdate set so marking an EXISTING psq_responses row 'sent' records when, matching the insert branch. - price-capability-gate.spec.ts: give the describe block an explicit 30s timeout (sibling precedent: magic-login.spec.ts's 60s) — it spawns a repo-scanning gate per test and blew the 5000ms default under concurrent workers, though the file passes in ~1s isolated. - seed-sql-baseline.json: drop the now-fixed marketplace.service.ts seconds-into-ms suppression (--update). - file-size-baseline.json: bump marketplace.service.ts's cap by 15 lines (592 -> 607), all explanatory comments on the two-bug fix above. The file is already grandfathered well over the 400-line ratchet; this growth is documentation, not new surface, and doesn't justify a split (--update). Strengthened tests/unit/marketplace/unified-catalogue.spec.ts and tests/unit/reports/psq-responses.spec.ts to actually assert on the fixed behavior — both were previously too weak to catch either bug. Confirmed red-before/green-after against the pre-fix code for both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…ot, and clear stale docs residue
- check-migration-refs / check-capability-declarations / check-test-layout /
check-agent-routes now print what they walked (file/route counts) and fail
loudly on a zero scan instead of passing vacuously.
- check-naming.mjs also flags `text('is_foo')`/`text('has_foo')` — a
predicate name stored as text instead of a real boolean — and its comment
now states the half it still cannot see (a boolean under a non-predicate
name), so the gate's own scope reads honestly.
- Move the three spike write-ups out of scripts/ (an executables directory)
into docs/develop/spikes/, fix the paths they cite that no longer resolve
(some genuinely deleted, some just untracked node_modules internals, one
section superseded by a later approach), and write down why this repo does
not number-prefix its docs.
- Delete docs/superpowers/, a fourth home for the boolean-naming rule that
cited a squashed migration and a rename map whose old names no longer
exist; the gate, the CI chain, and CLAUDE.md already enforce the rule.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…row it shipped
Re-importing a comment library in replace mode deleted every row carrying that
library_id, unconditionally. Inspectors rewrite those sentences into their own
voice, citing their state's code and their local climate; that text goes out on
reports a client paid for, and a publisher's v2 destroyed it.
`confirmLossOfEdits` existed and was recorded but never acted on, and the reason
was honest rather than neglectful: a comment carried no edit marker, so an
edited import row was indistinguishable from a fresh one and a "refuse on edits"
check would have passed on every row. The missing piece was the marker.
comments gains two columns, appended at the end of the table:
edited_at epoch ms, display only — "edited 12 March"
import_hash the text the row arrived with
The decision rests on the hash, not the timestamp, and that is the whole design.
A row edited and then changed back hashes to its import and is correctly not a
conflict. A write path that forgets to stamp a timestamp cannot hide an edit,
because the evidence is the content itself — the new spec proves this by
rewriting a row with a bare UPDATE and setting no marker at all. And an entry
the new pack still ships verbatim identifies a row the publisher never touched,
so it is never raised.
confirmLossOfEdits now switches between keeping every rewrite (the default) and
deleting them; keeping also skips new-pack entries identical to a preserved
row's imported text, so nobody is handed back the sentence they rewrote.
The page is built around the sentences rather than the choice: the rewrites are
its body, each beside the publisher's version, and the two options sit beneath
them. Choosing to replace everything strikes those comments through and turns
them ih-bad on the spot — no confirmation dialog, because a dialog collects a
signature afterwards and this shows the bill beforehand. All three library
mutation endpoints previously had no frontend caller at all; the marketplace
card for an imported pack with a newer release now offers "Review update".
Where pairing a rewrite to its v2 successor is genuinely unknowable — pack
entries carry no stable ids — the code uses word overlap for DISPLAY only and
says "Not in 2.0.0" rather than inventing a match. What gets deleted rests
entirely on hash equality.
Templates were checked and do not share the hazard, structurally rather than by
luck: a template is a 1:1 kind whose import marker can be re-pointed, so
"update" mints a second local copy and leaves the edited one alone, and the
destructive path refuses any kind other than comments outright.
Migration is hand-written (two native ADD COLUMNs, no rebuild). Its first draft
failed its own rebuild-signature grep by naming the tokens while explaining why
a rebuild was avoided; the prose was rewritten, because a gate its own subject
trips is one people learn to wave through.
marketplace.service.ts is split into services/marketplace/{library-pack,
library-replace}.ts to stay under the file-size ratchet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…eader be whoever gets asked `tenant_destruction_records`, `ai_call_provenance` and `ai_content_reviews` each had exactly one insert and zero SELECTs. Three write-only ledgers is not three bugs, it is one: nobody decided who would ever ask. An audit record that cannot be produced differs from no record only in that it offers false comfort. So the retrieval rules are decided once, in `server/lib/compliance/assurance-records.ts`: read-only, explicit column projection (never `select()` -- a column added to `ai_call_provenance` later is a compliance decision, and `SELECT *` would publish it the day it lands), epoch milliseconds out, newest first, capped and pageable backwards. The HTTP layers above are thin; they pick a reader and a guard and add nothing to the shape. WHO THE READER IS The AI ledgers go to the workspace's own owner/manager, next to the erasure log they are shaped after. Both tables are tenant-scoped and therefore destroyed with the workspace by TenantPurgeService, so unlike the destruction record there is no later reader to design for -- if the workspace cannot read them while it exists, no one ever can. The liability is theirs too: `lib/inspection/reports.ts` says the narrative carries professional liability, and that is the inspector's, not the platform's. And nothing here is unsafe to show them: provenance stores no prompt by construction, reviews name a staff user of that same workspace. The destruction record goes to the platform operator over the portal M2M HMAC, and that is forced rather than chosen. The row describes a tenant that has been deleted, so no session can exist for its subject and any session-derived filter would make the proof permanently unreachable. `scoped-tables.ts` excludes the table from the purge sweep for the mirror-image reason on the write side. This repo has no sysadmin role to hang it on -- ROLES is closed and tenant-scoped -- but the operator already has an identity here, and the read is now the sibling of the `POST /tenants/:slug/purge` that wrote the row. THE RETRIEVAL UNIT IS THE CALL, NOT THE REVIEW Listing reviews answers "what did we confirm", which nobody asks. Listing calls with their reviews nested answers "what did a model write, and did anyone look at it" -- so an unreviewed call shows up as a row with an empty list rather than as an absence, and the nesting direction matches the schema's. TWO THINGS THE READ REFUSES TO LOSE Reviews are filtered on their own `tenant_id` as well as on the call-id set, even though that set is already tenant-derived; relying on the other table's filter would make isolation a property of a join. The spec was confirmed to go red with the filter removed. `POST /api/ai/reviews` takes `aiCallId` from the body without checking the call belongs to the caller, so a review citing a foreign call is constructible today. Both halves being tenant-filtered means it can never surface another workspace's call -- it just resolves to nothing. Rather than let it vanish, it is counted and surfaced, because a compliance view that quietly loses evidence rows is the failure this change exists to fix. The file-size baseline moves for `server/portal/integration.routes.ts` (449 -> 456). The handler was already extracted into its own module following that file's own pattern (`usage-report.ts`, `ai-provisioning.ts`); the residual seven lines are the irreducible import-and-mount cost on a file grandfathered far above the 400 cap, and splitting an unrelated handler to pay for them is a refactor this change does not justify. Verified in Chrome, both themes: the table overflowed its card by 65px until the capability folded into the model cell, and the paged empty state claimed nothing had been drafted with model assistance while standing on page two, which is false in exactly the situation a reader is most likely to be in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…cs get a real db type
Two failures from the same full-suite run.
The agreement containment scan reads every .ts/.tsx under server/ and app/ —
about 2700 files — to prove no module that composes an agreement body imports
the platform disclosure. ~5.1s idle, already past vitest's 5000ms default, so
whether it passed was decided by machine load rather than by what it found. It
failed in the full run and passed solo, which is the shape that gets filed as
flake and closed. Ninth spec here to need an explicit budget; the common thread
is that each takes THE WHOLE REPOSITORY as its subject.
The assurance specs handed a better-sqlite3 handle straight to parameters typed
`DrizzleD1Database<...> | { [k: string]: unknown }`. A class never gains an
implicit index signature, so that union's escape hatch accepts nothing a driver
actually produces — the specs did not type-check at all. `asAnyDb()` in
tests/unit/helpers/test-db.ts is what the five other specs on this seam use.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…cs least
Three tiers decide how much tsc a commit pays for. The narrow one — staged TS
confined to server/ and tests/ — runs `type-check:tests`, which compiles the
api program and then `tsconfig.tests.json`. The broad one, reached the moment a
commit also touches app/ or a ts-config, reimplemented `npm run type-check`
inline as `type-check:app && type-check:api` and dropped its final step. So the
test program went unchecked precisely when a commit was large enough to be
risky, and a spec staged next to a route change was compiled by nothing.
Found by a spec that shipped green and could never have type-checked: it passed
a better-sqlite3 handle to a parameter typed as a union whose escape hatch is
`{ [k: string]: unknown }`, which no class satisfies. Its commit touched app/.
The tests pass is now appended to both broad branches, and only when a spec is
actually staged, so an app-only commit pays nothing new. Both failure hints now
name `npm run type-check` — the script this tier was always standing in for,
and the one CI runs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
CI's verify runs on the upstream PR, not on pushes to this fork, so the local run I skipped was the only rung that could have caught these before the push. 1. web shard — the compliance settings loader reads THREE endpoints; its BFF test stubbed two. Each call is wrapped in `.catch(() => null)` so a failing endpoint degrades gracefully, but that catch covers a promise that exists, and a missing mock branch throws while reading `.$get` off undefined — synchronously, before any promise. One omitted stub failed all four loader tests, not the one that cared about it. 2. API shard — a doc comment naming `server/portal/<the integration route>` was read as an import by the portal-isolation gate, which greps `server/` with no AST. Prose that names a path is indistinguishable from code that imports it. The comment now says which route it means without spelling the path, and says why. 3. lint gates — the tenant-scoping baseline entry moved: the same unscoped catalog lookup now lives in the extracted `library-replace.ts` instead of `marketplace.service.ts`. Not a new violation; the count is still 70. The query is correct — `marketplace_libraries` is the published catalog and has no `tenant_id` at all, while the import row read beside it IS scoped. Said so at the call site, so the next reader does not "fix" it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…found out knip flagged nine unused exports across the last two batches. Read as "delete this", eight of the nine are wrong. Read as "nothing in this repository, tests included, ever names these", all nine are right — and knip counts specs as entry points, so an export no test imports is an export nothing verifies. The load-bearing case: `hashCommentText` and `normalizeCommentText` are what "keep my edits" rests on. They decide which of an inspector's rewritten comments a marketplace re-import may delete — the defect that motivated the feature (#348) — and they shipped with zero tests. tests/unit/marketplace/ library-edit-marker.spec.ts is 19 specs over both halves the module draws for itself: deletion is hash-exact and asserted exactly, pairing is a guess and is asserted as one. It pins the behaviours the header claims and nothing checked, including that a row edited and then changed BACK is not a conflict, that reflowed whitespace is not a rewrite, and that a row predating the marker has nothing claimed about it. Sabotaging the normalizer turns three of them red. The four assurance types and the panel's review row are now named by the specs that already covered that code, which is worth more than the gate: a field added to either shape now fails at a spec instead of rendering as a blank cell. Two were genuinely over-exported and are now module-private. Also: this gate's own header said the baseline "is `[]` and MUST stay that way". It has held two entries since InspectorHub#297, through two PRs. I read that line and drew a conclusion about the rules from it. A gate's description is read as its rules. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…der a key nothing reads
Its schema carried `entries: []`. `parseLibraryComments` reads `comments`. Both
halves were wrong, so the catalogue advertised a featured library — described as
covering six sections with three severities — that imported as ZERO ROWS, and
`itemCount` rendered 0 on the browse card with nobody asking what that meant.
The content it was supposed to carry already existed: CANNED_COMMENTS, the same
254 entries the starter-content service seeds directly into a new trial tenant.
The pack now derives from it — rows, section list and item count all computed,
not restated. Two hand-kept copies of one library is how they drift, and the
prose here proves it: while the array was empty nobody could see that it named a
severity vocabulary the product does not use ("satisfactory / monitor / defect"
against the canonical good / marginal / significant), claimed six categories
where the data has thirteen, and called 254 entries "small".
Two things the fix uncovered:
- The import dropped severity. A pack classifies every comment and
`comments.severity` is that exact vocabulary, but the insert never bound it,
so the marketplace copy of a library was strictly poorer than the copy seeded
directly — same content, two paths, one lossy. Now bound; CHUNK drops 20 to 18
to keep eight columns under the same placeholder ceiling its comment has
tracked since the row was six columns wide.
- The catalogue listing returned `schema` — the entire pack — by spreading the
row. Free while the pack was empty; ~50KB per library once it is not, at
pageSize 1000, on a browse page. No client reads it.
The insert moved to `marketplace/library-insert.ts`, beside library-pack and
library-replace, for the reason those are free functions: every path that writes
a comment row has to stamp the same marker on it, and one function is how that
stays true. That took the service from 586 lines to 527, and the ratchet down
with it rather than leaving the old ceiling in place.
The new test drives the SHIPPED fixture rather than a hand-written pack, which
is the whole lesson: eight existing tests passed because none of them used the
thing we actually ship. Restoring the `entries` key turns it red with
"expected +0 to be 254".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…space `recordContentReview` took `tenantId` from the verified session and `aiCallId` from the request body, and wrote the pair without asking whether they belonged together. Any authenticated user could file a review naming another tenant's call: a row asserting provenance it did not have, and one workspace's identifier landing in another's audit ledger. `readAiAssurance` counts such rows as orphans, which is the right treatment for rows that already exist and the wrong thing to lean on for rows still being written. The check goes in the function, not the route, because that is where every caller meets it. It answers with the same 404 for "no such call" and "not yours" — a different message would confirm the existence of an id the caller is not entitled to know about. It is a read before an insert, which the note on this function warns against. That warning is about deciding whether a DUPLICATE review exists, and that decision is still atomic: the insert and its ON CONFLICT DO NOTHING are untouched, and a spec now pins that a repeated review of an owned call is still a no-op. This read asks a different question, about a row the request cannot create and no concurrent retry can change. Three existing narrative specs began failing the moment the check landed, which is how you know it is live — they had been recording reviews of a call that was never seeded. They seed one now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
Fallout from the ownership check, and the same shape as the three narrative specs: it exercised POST /api/ai/reviews against two call ids that existed nowhere, which the route now refuses. Retry safety is still its subject; both calls are seeded under its tenant, and ownership is asserted next door. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…ion, not after
A tenant purge destroys D1 rows, R2 objects and KV keys, then filed the record
proving it happened. That record is the deliverable of an SCC Clause 8.5
certification and of GDPR Art. 5(2) accountability, and it was lost by precisely
the failures worth recording: a crash between the cascade and the insert left a
workspace permanently destroyed with nothing on file saying so, and the only
trace was a `logger.error` on a platform whose logs are kept for days against an
audit window kept for years.
Letting that final write throw does not fix it. By then the data is gone, so
failing there undoes nothing; it returns an error to a caller who may retry, and
the retry deletes nothing and files a record reading `rowsDeleted: 0` — a FALSE
certificate, worse than a missing one.
So the row is opened before anything is destroyed and closed after every step:
- The opening insert is deliberately NOT guarded. Throwing there is correct
and free — nothing is destroyed yet, so refusing to begin a destruction we
cannot evidence leaves the workspace exactly as it was.
- The closing update IS best-effort, for the opposite reason. A row stuck at
'started' after a successful purge understates what happened, which is the
safe direction: it asks a human to look rather than certifying something
false.
The reader surfaces `status` and `completedAt`, because an abandoned purge and a
completed one were otherwise the same row, and the abandoned one is the case
worth finding. An unrecognised status value reads as UNFINISHED — this module
does not certify what it cannot verify.
Retention is 3 years, decided and written down in
docs/compliance/destruction-evidence.md along with which instrument requires
what. No sweep enforces it yet and the doc says so.
Two things found on the way in:
- The status-literal gate reported ONE of the two literals this change
introduced. `'completed'` was caught because it collides with an inspection
status; `'started'` belongs to no registered axis and sailed through, same
file, same commit. The gate guarded two axes and fired on a third by word
coincidence. The destruction axis now has its own constant module and is
registered, an axis that parses to zero members now fails closed, and the fix
hint names all three. Registering it immediately surfaced the `'started'` in
the reader that nothing could see before.
- The `unresolvedReviewCount` doc still said the review route does not verify
call ownership. It does now — fixed two commits ago. A non-zero count means
historical rows, not an open hole.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
… enforcing it The period was decided and written down; nothing applied it. A retention period that only exists in a document is the same as none, and worse for saying so. It is a manifest entry plus an executor rather than a new sweep, because that structure already exists and already carries the guarantee that matters: a spec asserts RETENTION_MANIFEST and the executor map agree in BOTH directions, so a period with no executor (a promise nothing keeps) and an executor with no period (a delete statement on a number nobody wrote down) each fail. Only COMPLETED records expire, and that is a rule, not an optimisation — the same shape as excluding a `pending` row from the outbox sweep, and for a stronger reason. A record still reading `started` is a workspace that was destroyed and never certified: an open anomaly, and the only artifact that says so. Expiring it on age would close the question by deleting the evidence of it, which is precisely what opening the record before the destruction was meant to prevent. It ages out of nothing and waits for a person. Measured from `destroyed_at`, the initiation timestamp — the only one an unfinished row has, and seconds from `completed_at` on the rows that do expire. Removing the status predicate turns one spec red; deleting the executor turns the binding guard red naming the orphaned rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
…th a predicate
The retention gate demanded a decision only for tables whose NAME matched
`_log(s)`, `processed_*`, `parked_*` or `_events`. Sixteen of ninety-five tables
had a decision and the gate only ever asked about nine of them — the other seven
were volunteered by whoever wrote them. `tenant_destruction_records` was the
proof: a compliance ledger growing per purge, named `_records`, invisible to
that line, with no retention decision while the gate reported green.
`ai_call_provenance` and `ai_content_reviews` were in the same position.
All seven now have a window, all at three years. Five of them also carry a
predicate, because expiring a row purely on age would have broken something that
outlives it:
ai_call_provenance a call a surviving review still cites is kept. Equal
windows are why this is needed, not why it is not: a
review is written AFTER its call, so on equal clocks
the call goes first and leaves the review pointing at
nothing — the exact shape `readAiAssurance` reports as
`unresolvedReviewCount`. A sweep that manufactured
them would turn that alarm into noise. The two tables
share ONE constant, the documented exception to this
file's one-constant-per-number rule, because they are
one governance record split by normalization and must
never diverge.
report_versions superseded versions only. The highest version_number
is what the report IS and carries the signature a
verifier reads.
sms_disclosure_versions a version no surviving consent row cites, and never
the current one. `sms_consent_log` is kept
INDEFINITELY by an explicit exemption — the record is
the tenant's defence against a consent challenge — and
every consent row stamps the version it was shown.
Deleting a cited version leaves permanent evidence
pointing at text that no longer exists, gutting that
exemption from the other side. In practice this reaps
only versions published and never used, and that is the
correct outcome rather than a misconfiguration.
tenant_legal_versions superseded per (tenant, doc). The newest is the live
policy the hosted legal pages render; a naive "is
anything newer" predicate would let a newer TERMS
version expire the current PRIVACY one.
tenant_slug_history a row whose retirement block has not passed is kept.
Deleting one releases the slug early and another
tenant could inherit every stale link to the old owner.
Each predicate has a spec that goes red when it is removed.
The gate's pattern is widened afterwards rather than first, so it costs nothing
today — every newly in-scope table is already registered — and starts earning on
the next `_records` or `_versions` table. Six tables remain registered
voluntarily, outside even the widened pattern.
`retention-manifest.ts` crossed the size gate on the way, so the windows and
their reasoning moved to `retention-windows.ts` along a real seam: how long and
why there, which tables and what action here, how in retention-logs.ts. The gate
reads the manifest's arrays as source text and only ever parses `window.unit`,
never the value, so it cannot see the move.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SjSCw6F7yJCCdggf6Bcj5y
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A milestone batch of 79 commits. Each area below is independent; they ship together because
they were developed against one tree.
The migration chain is THREE files, not the ten this branch produced: they were squashed into
a single
0000_baseline.sqlonce the last of the frozen columns was gone. Wrangler tracksapplied migrations by NAME, so a database already holding a
0000_baseline.sqlrow skips thenew one silently — correct here only because production's schema was verified, column by
column, to already match what the new baseline builds.
Deployment-mode branching goes through the capability seam (#308)
Ten sites branched on the deployment mode directly instead of asking the
capability surface. They now read
deployment-profile.ts, which is the singleplace that answers what a given deployment can do. A gate keeps new branches
from going around it.
Closes #308
One marketplace catalogue, and template permissions with a real delete guard (#293, #307)
The template catalogue and the library catalogue were separate tables with
overlapping meaning. They are now one catalogue keyed by
kind, with the legacytable retired after its rows moved across (ids carry over, so every existing
reference still resolves).
Template mutation routes are now capability-gated per VERB rather than per
concern — create, edit, delete and import are four separate toggles. A single
toggle cannot express "may fix a typo, may not delete the template", and the
four verbs are not equally recoverable: an edit has an audit trail, a delete is
a rebuild-from-scratch.
Closes #293
Closes #307
Data-subject access requests can be fulfilled end to end
Export assembles subject-scoped rows across 20 collections and streams a ZIP
through a writer now shared with the tenant export. Erasure replies with a
COVERAGE DISCLOSURE rather than a bare "completed", because "completed" on its
own is an unbacked claim that everything was erased.
Three limits are stated in the code rather than papered over: the disclosure
cannot distinguish "matched nothing" from "not reached"; the manifest rule count
describes a document rather than the run; and erasure matches on email only.
A partial run emits NO reply and throws, so the command retries and the request
stays visibly unfinished. There is no field for "partly erased", and a reply
would have been readable only as success.
Managed AI allowances are enforced, not merely stored
checkAiQuotawas written, tested, and read a delivered cap while having zeroproduction callers — a configured allowance was inert with nothing reporting it.
It now runs at the same chokepoint as the meter: check before the send, meter
after success, so a failed model call never spends an allowance.
docs/compliance/ai-data-flow.mdstates, from the code, which fields leave theprocess and under which provider terms.
Behaviour is unchanged for existing deployments: the capability gate still
refuses every managed output class.
Calendar providers behind a real interface, with CalDAV under it
The provider "interface" was a shape nothing implemented. It is now real, with
CalDAV as a second implementation beside the existing one.
Starter content survives an upgrade
An owner can install the starter content a release ships, idempotently, instead
of the content being frozen at whatever the tenant was provisioned with.
Which company name a person sees
Two columns hold a company name and are allowed to diverge. Nine display reads
took the wrong one. The rule now lives in one place and every consumer LEFT
JOINs, so a tenant that never set a name still shows one rather than rendering
blank.
tenants.nameis gone; one company name, in one placeTwo columns held a company name and were allowed to diverge, so nine display
reads took the wrong one. Rather than keep both,
tenants.nameis removed andtenant_configs.company_nameis the single source, resolved through oneexpression that every consumer LEFT JOINs. A tenant that never set a name falls
back to its slug instead of rendering blank.
Two migrations, in this order and not the other:
0064backfills the configrows,
0065drops the column. The drop is a hand-written nativeALTER TABLE ... DROP COLUMN— drizzle's twelve-step table rebuild cannot runon a table other tables reference, because D1 has no
PRAGMA foreign_keys=OFFoutside a transaction.
Removing the column found five different syntactic shapes hiding a reference to
it, and the compiler could only see two of them. The other three were an
as nevercast that silently swallowed the field, a string literal in a testassertion, and raw SQL inside template strings. The last of those is now covered
by a gate (below); the others are why the diff touches more test fixtures than a
column removal suggests.
A rename actually reaches the engine
The portal could rename a company and the engine would never hear about it —
the endpoint returned 200 and changed nothing a user could see. A rename is now
its own command (
cmd.tenant.rename), applied unconditionally rather thanfalling back to a column that no longer exists.
Automations: a refusal that undoes itself
The outbound cooling window declines platform-funded client email for a
company's first 24 hours. Every other refusal the send boundary raises will
still be true on the next tick; this one carries the instant it stops being
true. Spending a terminal status on it meant a company's first report — signed
up and published the same afternoon, which is the ordinary first day — was never
delivered and nothing retried it. The row now stays pending with
send_atmovedto the unlock instant, and the Publish button says so.
Gate: raw SQL that names a column the schema does not have
Seeders talk to D1 in template strings, so nothing type-checks them; they fail
only when a real database rejects the statement. For the e2e fixtures that means
CI, minutes after the push. For
seed-pca-demoand the CLI self-host setupscript it means never — both are run by hand, months apart, so they rot until a
human hits the error.
Ported from the private portal repo, which grew the same gate after a column was
dropped out from under both of its seeders. It immediately found that the CLI
setup script stamps
created_atin seconds into a millisecond column, so thefirst tenant of every self-hosted install is dated January 1970, and that the
same script wrote a
subdomaincolumn that has not existed for far longer —the first command a self-hoster runs had been failing on its own.
Twenty further violations predate the gate and are baselined, with the count
printed on every run.
Gates
New: client submit-guard call-shape and busy rules; idempotency coverage driven
from a table rather than from spec prose; extension-collision detection for
files that are invisible to
tsc. Fixed: the status-literal union guard couldnot tell
|from||, and the file-size gate died on any deleted file whilenever printing what it had scanned.
Eight frozen columns are gone, and the rule that manufactured them is rewritten
tenants.name, three automation copy columns and five more had been FROZEN — read by nothing,dropped by nothing — because the schema rule said D1 could not drop a column an FK references.
It can: native
ALTER TABLE ... DROP COLUMNworks even there. What fails is drizzle'sgenerated twelve-step REBUILD, which needs
PRAGMA foreign_keys=OFFoutside a transaction.The rule now names SQLite's six genuine refusals instead, drop migrations are hand-written, and
grepping for the rebuild's four-string signature is part of accepting one.
A re-import can tell a rewrite from a row that arrived as-is (#348)
Replace mode deleted an inspector's rewritten comments along with untouched ones, because
nothing could separate them. There is now an edit marker: the hash of the text AT IMPORT TIME
plus an
edited_atstamp. A hash rather than a write flag, so a row edited and changed back isnot a conflict, and a path that forgets the timestamp still cannot hide an edit. The import
screen asks before it acts and defaults to keeping the edits.
Three write-only compliance ledgers get a reader
tenant_destruction_records,ai_call_provenanceandai_content_reviewswere each writtenby one call site and read by nothing. That is one defect, not three: nobody decided who would
ever ask. One read-only module now answers for all three, with explicit column projection so a
column added later is published deliberately.
Destruction evidence is written BEFORE the destruction
Filed last, the proof a workspace was purged was lost by exactly the failures worth recording:
a crash between the cascade and the insert left it destroyed with nothing on file. Letting that
write throw does not help — the data is already gone, so a retry deletes nothing and files a
rowsDeleted: 0record, which is a false certificate.The row is opened before anything is destroyed and closed after. The opening insert is
deliberately unguarded; the closing update is best-effort. A row left at
startedis anunfinished destruction that ages out of nothing and waits for a person. Retention is three
years, with the instruments behind it in
docs/compliance/destruction-evidence.md.Eight ledgers get a retention window, five with a predicate
The retention gate demanded a decision only for tables NAMED
_log(s),processed_*,parked_*or_events— sixteen of ninety-five tables had one and the gate had only everasked about nine. Eight ledgers now have a three-year window; five carry a predicate because
expiring on age alone would break something that outlives the row: a call a surviving review
cites, the current version of a report, an SMS disclosure a permanently-retained consent row
points at, the live legal version behind the hosted pages, a slug still inside its retirement
block. Each predicate has a spec that goes red without it. The gate's pattern is widened
afterwards, so it costs nothing today.
Two defects the release audit found
Starter Comment Pack— a FEATURED library — imported as zero rows: its entries sat under akey the parser does not read, and the array was empty besides. It now derives from the 254
comments the starter-content service already seeds, and the import stopped discarding the
severity a pack carries.
POST /api/ai/reviewstookaiCallIdfrom the request body without checking it belonged tothe caller's workspace, so a review could assert provenance it did not have.
Gates that were not looking
it reimplemented
npm run type-checkinline and dropped the tests pass.only the one COLLIDING with an inspection status was reported.
🤖 Generated with Claude Code