Skip to Content

← All archived runs

Run: tenant-integrations-read-index

run.md

Run: tenant-integrations-read-index

  • branch: claude/tenant-integrations-read-index-20emgh
  • pr: #852

02_define/output/spec.md

Spec: A tenant read index for tenant_integrations

  • slug: tenant-integrations-read-index
  • personas: Admin
  • touches: packages/services/src/db/models/tenant-integration.ts, packages/services/src/db/migrations/, packages/services/src/db/audit/index.test.ts
  • complexity: trivial

Problem

The admin integrations list scans its collection. listIntegrations runs find({ tenantId }).sort({ createdAt: -1 }) (packages/services/src/db/services/tenant-integration/index.ts:39-46) and tenant_integrations has no index that can serve it. The one secondary index on the collection is the unique { tenantId, catalogueKey }, partial on { isDeleted: false } (db/models/tenant-integration.ts) — and softDeletePlugin injects $or: [{ isDeleted: false }, { isDeleted: { $exists: false } }] into every query (db/plugins/soft-delete.ts:24-27). A partial index is usable only when the query predicate provably implies its filter, and the $exists: false branch breaks that guarantee, so the planner cannot touch it. tenantPlugin adds no fallback either: it only declares tenantId with index: true when the schema hasn't declared the path, and this model has. Result: a collection scan plus an in-memory sort on every load of the admin integrations page and the admin dashboard tile (apps/web/app/(app)/admin/integrations/page.tsx:23, apps/web/lib/admin-dashboard-data.ts:195).

This is the last collection carrying a missing-tenant-index finding from the data-layer audit. hot-path-tenant-indexes (#809) covered leads, quotes, invoices and lead_expert_matches but not this one, and fix-broken-partial-uniques (#810) made the unique index build without changing the read path. It advances Refine the bridge / Q2-2026 Objective 1 — Establish Product-Market Fit with Vendor Partners: tenant admin surfaces stay fast as the vendor tenant count grows, and the audit board reaches a state where every remaining finding is real.

One premise from the stub is stale and this spec corrects it. The stub warns that #810 would silence the finding, because the auditor's tenant check is purely structural and would accept the partial unique. It no longer is. #809 also landed isGeneralTenantIndex (db/audit/index.ts:66-68), which requires partialFilterExpression === undefined, with unit tests covering exactly the partial-tenantId-leading case (db/audit/index.test.ts). So tenant_integrations still reports missing-tenant-index, correctly, and lead's partial { tenantId, requestId } no longer masks its collection either. Nothing is silenced; the finding is simply still open. The index below is what closes it.

Proposed change

Add a non-partial { tenantId: 1, createdAt: -1 } index to the tenant_integrations model with a matching migration in lockstep, following the hot-path-tenant-indexes house pattern (an IndexSpec table, tolerated-absence drops, a symmetric down). Equality field leads, sort field last, so the index both bounds the read to the tenant and supplies the sort; the $or on isDeleted stays a residual filter, which is all it ever needed to be.

Correct the stale comment block in tenant-integration.ts at the same time — it currently tells the next reader that the audit finding "will now go green" and should be treated as a false negative, which is no longer true — and lock the collection's own shape into the auditor's test file so the tenant_integrations case cannot silently regress the way the stub feared.

Acceptance criteria

  • tenant_integrations declares a non-partial { tenantId: 1, createdAt: -1 } index in db/models/tenant-integration.ts, and a new migration builds the same index with the same key and options — model and migration in lockstep, no drift.
  • The migration's down drops exactly what up created, tolerates an already-absent index or a never-written collection (the ABSENT pattern from 1786665600000-hot-path-tenant-indexes.ts), and re-running up is a no-op.
  • The migration id is unique across db/migrations/ — it does not reuse an existing timestamp prefix.
  • listIntegrations' query shape is servable by the new index: tenantId equality bounds the scan and createdAt: -1 supplies the sort, with the soft-delete $or left as a residual filter. Confirmed at Verify by an explain() on the preview database showing an IXSCAN on tenantId_1_createdAt_-1 rather than a COLLSCAN with a blocking SORT.
  • db/audit/index.test.ts covers the tenant_integrations shape: the partial unique { tenantId, catalogueKey } alone does not satisfy the tenant check, and the pair (partial unique + the new read index) does.
  • The comment block in db/models/tenant-integration.ts no longer claims the audit finding goes green or is a false negative, and points at the read index as the thing that serves reads.

Out of scope

  • The auditor's partial-index blind spot — the stub's third criterion. Already delivered by #809 (isGeneralTenantIndex + its tests); nothing left to build. Evidenced above.
  • The unique partial { tenantId, catalogueKey } index — correct for uniqueness, stays exactly as it is. Not re-litigated, not widened, not dropped.
  • The shared migration id 1786665600000 (three applied migrations). Jamie ruled on 2026-08-17: leave as-is — applied migrations keep their ids, only future migrations get unique prefixes. This spec's new migration honours the second half of that ruling by taking an unused id.
  • The "unused index suspects" list — still deferred pending a longer $indexStats window (D1).
  • Any change to listIntegrations itself, to softDeletePlugin's $or, or to the pagination / shape of the admin integrations page. The read is correct; only its access path is wrong.

Open questions

  • none

03_build/output/notes.md

Build notes: tenant-integrations-read-index

  • commits: feat: tenant-integrations-read-index — tenant-leading read index + migration

What changed

  • packages/services/src/db/models/tenant-integration.ts: added TenantIntegrationSchema.index({ tenantId: 1, createdAt: -1 }) — non-partial, ESR (equality first, sort last), cross-referencing the migration by filename per the house pattern. Also rewrote the stale half of the comment block above the unique index: it told the next reader the audit finding would "go green" and should be treated as a false negative, which stopped being true when #809 landed isGeneralTenantIndex. It now says only what remains true — the unique index enforces uniqueness and does not serve reads — and points at the read index below it.
  • packages/services/src/db/migrations/1787788800000-tenant-integrations-read-index.ts: new migration building the same index with the same key and no options. Raw driver (connection.collection("tenant_integrations")), never the model. down drops tenantId_1_createdAt_-1 and tolerates absence via the same ABSENT regex 1786665600000-hot-path-tenant-indexes.ts uses, so a partial re-run converges in both directions. Id 1787788800000 is one day past the highest existing prefix and is unused.
  • packages/services/src/db/audit/index.test.ts: two cases written from the spec's criteria — a partial unique index leading with tenantId does not satisfy the tenant check, and tenant_integrations counts as covered only once the non-partial read index is present. The existing fixtures covered leads' partial-on-requestId shape; these cover partial-on-a- soft-delete-flag, which is the shape this collection actually has.

Acceptance criteria status

  • Non-partial { tenantId: 1, createdAt: -1 } declared in the model and built by a migration — same key, same (empty) options, model comment naming the migration and migration comment naming the model. No drift.
  • Symmetric down — drops exactly what up created, tolerates an already-absent index or a never-written collection, and createIndex is idempotent for an identical key so re-running up is a no-op.
  • Migration id unique — 1787788800000 appears once in db/migrations/. The three migrations already sharing 1786665600000 are left alone, per Jamie's 2026-08-17 ruling.
  • explain() on the preview database showing IXSCAN on tenantId_1_createdAt_-1 rather than a COLLSCAN with a blocking SORT. Not verifiable here — the agent sandbox can't reach MongoDB (TCP 27017 blocked, per the db-migration skill), and the index only exists on a database once the migration has run. This is a Verify-stage check against the preview, and the criterion says so. The static half is met: the query is { tenantId, $or: [{isDeleted:false},{isDeleted:{$exists:false}}] } sorted createdAt: -1, the $or touches neither index key, so tenantId bounds the scan, createdAt supplies the sort, and the $or is left as a residual filter.
  • db/audit/index.test.ts covers the tenant_integrations shape — both directions of the pair. Written, not run: pnpm test is the factory's, and the Quality workflow's Run tests step reports it.
  • The model comment no longer claims the finding goes green or is a false negative.

Notes for Verify

  • The spec corrects a stale premise in the stub, and that correction is load-bearing. The stub was written expecting #810 to silence the missing-tenant-index finding on tenant_integrations via a purely structural auditor check. #809 had already fixed that check — isGeneralTenantIndex (db/audit/index.ts:66-68) requires partialFilterExpression === undefined and ships with tests. So the stub's third acceptance criterion needed no code, and the spec moved it to Out of scope with that evidence rather than re-fixing a fixed thing. Worth a second pair of eyes on that call.
  • Run the explain() on the preview once db-migrate.yaml has applied the migration — that's the one criterion this stage couldn't close. Query shape: db.tenant_integrations.find({tenantId: ObjectId(...), $or: [{isDeleted:false},{isDeleted:{$exists:false}}]}).sort({createdAt:-1}).explain("executionStats").
  • The migration has not run anywhere yet, so unlike 1786665600000-hot-path-tenant-indexes it is still safe to edit if review wants the key changed.
  • No app code changed — listIntegrations, the admin page and the dashboard tile are untouched. The read was already correct; only its access path was.
  • Context budget: within the Inputs table.