feat(mongodb): move CI telemetry to a ciControl satellite and reclaim things index storage

[lopugit]

Why

The Atlas dashboard showed ~3 GB of index storage against ~300 MB of documents on things. A read-only audit of the production cluster (2026-09-02) found:

• things_v2 = 1,824,527 docs, 1,177 MB logical (322 MB on disk), 64 indexes = 3,147 MB — at MongoDB's hard cap.
• 99.75 % of those rows are CI control-plane telemetry (ci-event 1.37 M, ci-workflow-run 434 k), all written since August, ~270 k rows/day and accelerating. Real user content ≈ 600 docs.
• 672 k events (49 %) were "active → active" no-ops recorded for the repository row on every GitHub delivery; 80 % of "workflow runs" are per-job rows, most skipped.
• Five indexes (kind_1_typeId_* ×4, kind_1_deletedAt_*) served fields on zero documents (≈ 685 MB); the five v1-era kind_* indexes held only nulls (≈ 650 MB); the wildcard text index tokenised every CI payload (582 MB); every other general index paid a 51-byte shareId suffix per CI row.

Full report with measurements and the rollout runbook: docs/architecture/mongodb-index-storage-audit.md.

What

• ciControl satellite collection (ciControl_v1, getCiControlCollection()): every ci-* Thing now lives there — same envelope, same deterministic ids, six purpose-sized indexes, no text index. Nothing outside api/utils/ciControl/ ever read those rows.
• Retention (ciControl/retentionCore.ts): root expiresAt + TTL — events 14 d, job: rows 30 d, runs/deployments/previews 90 d, entities never. Env: THINGTIME_CI_{EVENT,JOB,ACTIVITY}_RETENTION_DAYS (0 = forever).
• Ingest de-noising (ciControl/ingestPolicyCore.ts): the repository row records an event only on insert or a status transition.
• things index plan: seven dead/moved indexes retired by name at boot; the five kind_* indexes and the sandbox TTL become partial (create-then-drop swaps, with a drop-then-create fallback only at the 64 cap); leftover __rebuild twins are pruned at boot.
• Admin migrations: relocate-ci-control-telemetry (time-budgeted, idempotent, insert-if-absent by shareId, expired rows deleted without copying) and rebuild-things-indexes (one index at a time; unique constraints held by a same-key partial twin throughout; foreign indexes left alone).
• Storage census on GET /api/v1/admin/migrations and the /migrations panel (document / on-disk / index bytes per physical collection with a bloat badge) — api.admin-migrations 1.1.0; ciControl allowlisted in the query workbench — api.mongodb-raw-results 1.1.0.
• Docs: FUNDAMENTALS §3, README (env + storage hygiene runbook), TESTING checklist, DECISIONS, schema descriptions, API docs.

Verification

• Unit: collections 34, ci-control 59, migrations 48, schemas 109, capabilities 4, migration UI 5 — all green; typecheck ratchet at baseline (108).
• Live, local MongoDB 8.0 replica set through the real API:
  • 20 signed synthetic GitHub deliveries → things_v2 gained 0 CI rows; ciControl_v1 gained 21 events (repository: 1 — the insert), 6 run rows, entities without expiry; stamps 14.0 / 30.0 / 90.0 days.
  • Relocation on a 2,307-row pre-satellite fixture: dry run wrote nothing; the confirmed run relocated 807, deleted 2,307, left the non-CI doc untouched, and the live satellite row kept its newer state.
  • Index rebuild: 57 plan-owned indexes rebuilt one at a time, 10 unique constraints twinned, index set identical before/after, text index rebuilt with identical weights; a concurrent duplicate-shareId probe saw 20,423 E11000 rejections, 0 accepted.
  • The first rebuild design (all twins up front) tripped the 64 cap mid-run in this live test and was replaced by the one-at-a-time design + boot-time twin pruning.
• /migrations panel checked in the browser at desktop and mobile widths (tables scroll inside their own container, no page overflow).

Production rollout (owner)

1. Deploy — boot prunes ≈ 980 MB of dead indexes immediately and creates ciControl_v1.
2. /migrations → relocate-ci-control-telemetry (dry run, then confirm; repeat until drained).
3. /migrations → rebuild-things-indexes (dry run, then confirm). Expected: things_v2 ≈ 4.5 k docs, tens of MB of index, 57 indexes.
4. Optionally drop-stale-collection-generations for the empty legacy collections; optionally an Atlas compaction to reclaim the collection file's freed pages.

Note on graphify

This PR carries no graphify-out changes. Running scripts/graphify update . on the branch pruned the 170 snapshot files develop tracks (1.4 GB, 37 M lines); GitHub then refused to compute the diff ("taking too long to generate") and stopped synchronizing the PR head for 40 minutes. Restoring develop's snapshot set fixed it in seconds. The graph refresh belongs to the merge-time hooks; the report's follow-ups flag the snapshot-in-git problem.

🤖 Generated with Claude Code

[github-actions]

🧹 Develop S3 preview removed

The PR-specific alias and every workflow-created develop deployment were removed when this PR closed.

The ordinary generated Vercel Preview remains available on the shared development runtime.

[github-actions]

🤖 Branch status still computing. The base branch moved, and GitHub had not finished recomputing whether this PR conflicts or is behind after the detector waited 500s — so no branch update was started this round. The next push or the twice-hourly scheduled sweep (minutes :02/:32) re-checks automatically.

Posted by the conflict detector at 05:35 UTC, 2026-09-02; this notice is edited in place on re-checks.

[github-actions]

🌸 Lopu — reviewed in full. No changes needed; one deploy-ordering note worth writing down.

Every check is green, MERGEABLE/CLEAN, 0 open CodeQL alerts. I spent the review on the two things that could actually bite: the partial-index conversion and the relocation migration's data safety.

The partial kind indexes — I checked the planner assumption, and it holds

Making the five kind-rooted indexes partialFilterExpression: { kind: { $exists: true } } is only safe if every predicate that needs them implies kind exists. I enumerated all of them rather than trusting the comment:

• Equality / $in over non-null literals — things.ts:1675, things.ts:3549, search.ts:649, trending.ts:69, migrations.ts:383, migrations.ts:612. All imply existence, all keep coverage.
• One exception: migrations.ts:416, { kind: { $nin: ['post','reaction','comment'] } }. $nin is satisfied by a doc with no kind, so a partial index can't serve it. In practice this is a non-issue — it's a countDocuments in an admin migration that also carries thingtime: { $exists: false }, and $nin was never selective on a kind prefix anyway. Flagging it only so the claim in the code comment is precise rather than universal.

Relocation migration — the ordering is the right one

relocateCiControlRows pages _id: {$gt: lastId} under sort({_id: 1}), so the cursor stays correct while the batch it just read is deleted. And the bulkWrite runs before the deleteMany: if a satellite write throws (say a ci_control_share_id_unique violation on a row missing shareId), the delete never happens and the source rows survive. It fails loudly instead of losing data, and $setOnInsert keyed on the deterministic shareId makes the re-run idempotent and stops a stale things-era copy from clobbering a row the live writers already recreated. That's the ordering I'd want; worth keeping if this ever gets refactored.

The one operational note

Every ciControl reader — readKind, countCiDashboardStats, listCiAutomationPolicies, getCiAutomationPolicy, claimCiDispatchRoute, listCiEventsForParents — switches to the satellite in this commit, but the ~1.8M existing rows only move when an admin runs relocate-ci-control-telemetry (destructive: true, needs confirm: true). Between deploy and that run, the admin CI dashboard reads an empty satellite.

That's a fine trade for machine telemetry and it's fully recoverable — but it's the one thing an operator should know before deploying, and I couldn't find it stated in the PR note or TESTING.md. Suggest one line naming the order explicitly: deploy → relocate-ci-control-telemetry (re-run until pending reads 0) → rebuild-things-indexes. I didn't add it myself — it's your PR note to word.

Smaller confirmations

• pruneRebuildTwins can drop a twin held by a concurrently running rebuild. The PR names this; I agree with the trade, because the alternative is a wedged ensureIndexes that blocks the very migration that would clean up.
• createIndexReplacing's new code 67 path is correctly gated behind legacyNames.length, so drop-then-create can never fire on a fresh create.
• thingsIndexPlanNames replaying the plan against an in-memory recorder is a nice way to make the rebuild's owned-set structurally incapable of drifting from the plan. Its default-name synthesis (field_direction joined by _) matches MongoDB's.
• Allowlisting ciControl in MONGO_QUERY_COLLECTIONS doesn't widen exposure — /api/v1/mongodb/raw-results is requireAdmin-gated and rate-limited on both verbs.

Validation

suite	result
ingestPolicyCore + retentionCore + ciControlRelocationCore + migrationUiCore	23/23 pass
mongodb/indexBudget.test.ts	8/8 pass
ciControl/*.test.ts	42/43 — the one failure is Cannot find module '@vercel/sandbox'
migrations/*.test.ts	36/37 — Cannot find module 'tldts'
components/Schemas/*.test.ts	5/5 pass

One thing I want to be explicit about so it isn't misread as a regression: mongodb/queryRunner.test.ts reports 4 failures ("One or more typed BSON values are invalid"). I checked out develop 814ebfdc and got the identical 4 failures, so it tracks the BSON version in my ad-hoc test install, not this PR.

No changes made. Nothing here justified an edit.

_{Lopu · automated repository review · 0 open CodeQL alerts on 208f4875}

[github-actions]

Lopu repository review

Lopu reviewed this PR against develop as Thingtime's principal PR and repository manager. Using Claude Opus 5.

Lopu made justified improvements and pushed c3166e8 to claude/thingtime-mongodb-index-storage-dffe19.

Lopu review — PR #583

feat(mongodb): move CI telemetry to a ciControl satellite and reclaim things index storage

• Head 5f3fbbaa → base develop (814ebfdc) — 29 files, +2466 / −92.
• Dispatch lopu-review:33626088170 (scheduled review, not a check-run or
  comment wake-up).

Check and alert state

Every check on this head is green — Build + typecheck ratchet + unit tests,
API suite (headless /tests runner), Analyze (javascript-typescript),
Analyze (actions), CodeQL, Product contract advisories, GitGuardian. PR is
MERGEABLE. The trusted CodeQL snapshot for this head is empty (0 open
alerts), so the disposition file was left as [] — there is nothing to fix or
dispose. No failure, cancellation, or timeout to diagnose.

What I compared

Full head against develop, not just the newest commit. Six earlier Lopu passes
have worked the load-bearing seams (partial-kind planner assumption, the
relocation cursor, shareId-less rows, text-index rebuild ordering, the twin
prune, home-plane pinning, and the operator-facing census/notes). I did not
re-litigate those. This pass went at the boot-time index arithmetic, the
reader/writer cutover, and the trust boundaries.

Reviewed in full: mongodb/collections.ts, migrations/ciControlRelocationCore.ts,
migrations/migrations.ts, ciControl/{store,webhooks,featureStackStore, featureStackProgress,retentionCore,ingestPolicyCore}.ts, mongodb/queryContract.ts,
schemas/registry.ts, mongodb/collectionNames.ts, components/Schemas/*,
docs/apiDocs.ts, and the README / FUNDAMENTALS / DECISIONS / TESTING / CHANGELOG
diffs.

Finding and change

The index-budget guard measured the plan at rest, not at its peak

indexBudget.test.ts asserts plan + 4 ≤ 64 — "reserve 4 slots for safe
upgrades". That is true of the plan's final set (58 slots including _id_).
It is not true of the boot that has to fit.

This PR converts five v1-era kind_* indexes and the sandbox TTL to partial
indexes via createIndexReplacing, which deliberately creates the replacement
before dropping the original so no database ever sits without the index —
and the whole plan runs under one Promise.all. On the first boot after this
deploy, a collection already converged to the plan still holds all six
originals, so it transiently holds both halves of all six swaps.

Measured by replaying ensureHomeThingsIndexPlan against a fake that models
slot occupancy over time (create takes a slot, drop returns one, setImmediate
lets the fan-out interleave):

• steady state: 58 / 64
• transient peak during the boot swap: 64 / 64 — the whole 4-slot headroom
  the existing test promises is spent, with nothing left over.

This is not a defect: 64 is inclusive, the swap succeeds, and if it ever did not
createIndexReplacing degrades to drop-then-create (CannotCreateIndex 67),
which is already implemented and tested. The gap is in the guard: the
headroom test cannot see the peak, so a 59th plan index — or a seventh swap
pending on the same boot — would silently move production from the slot-safe
path onto the degraded one, which opens a real index-less window on
things_v2, and every existing test would still pass.

Change (test only, remix/app/api/utils/mongodb/indexBudget.test.ts): added
the boot ensure fits the 64-index cap at its PEAK, not just at rest, which
models occupancy over time and asserts (a) every swapped original is dropped
once its replacement exists, (b) each pending swap costs exactly one transient
slot, and (c) the peak stays within the cap. The comment records why the list is
"the swaps this change leaves pending" rather than every swap the plan has
ever performed — a later change adding one swap has one pending, not seven.

No production code was changed. Nothing else in this PR justified a change.

Verified, nothing to change

• The thingtime[0] derivation in relocatedCiDoc cannot lose user content.
  The migration matches {thingtime: {$in: CI_CONTROL_THINGTIME}} (any array
  element) but derives the retention kind from thingtime[0]. Multi-element
  thingtime arrays are real in this codebase (['post','comment'],
  ['post','share'] in things/things.ts), so a mixed array would be
  relocated-and-deleted under the wrong kind. It cannot arise:
  isProtectedThingtime uses .some() over the array and is enforced on both
  generic create (things.ts:1013) and generic update (things.ts:4076), so no
  user-writable Thing can carry a ci-* id. Every matching row is server-minted
  with exactly [kind].
• No secret or protected field follows the rows to the satellite. CI
  credentials live in lopuCredentials (ciControl/credentialVault.ts), not on
  ci-* Things; no writer under api/utils/ciControl/ sets secure or
  uniqueKeys. Adding ciControl to MONGO_QUERY_COLLECTIONS therefore does
  not need MONGO_PROTECTED_FIELD_QUERY_COLLECTIONS, and the workbench trust
  boundary is unchanged (things, users, sessions were already allowlisted).
• The satellite is registered everywhere it must be. COLLECTIONS derives
  from COLLECTION_SCHEMA_VERSIONS, so ciControl: 1 registers the name for
  physicalCollectionName, collectionVersion, and classifyPhysicalCollections
  in one edit — which is also why drop-stale-collection-generations classifies
  ciControl_v1 as current instead of unknown residue.
  targetSchemaVersion: collectionVersion('ciControl') and the writers'
  COLLECTION_SCHEMA_VERSIONS.ciControl are the same number by the registry's
  stated design ("its value is both the schemaVersion and the _v<N> suffix").
• Every reader moved. getHomeThingsCollection no longer appears anywhere
  under api/utils/ciControl/, and no ci-* kind literal is referenced outside
  that directory except in schemas/registry.ts and docs/apiDocs.ts (both
  documentation-only; the schemas carry collection: null so nothing routes a
  query to the wrong collection).
• The six-index satellite plan covers its readers. readKind,
  countCiDashboardStats, listCiAutomationPolicies, getCiAutomationPolicy,
  listCiEventsForParents, recordFeatureStackProgress,
  linkFeatureStackWorkflowRun, and every deterministic-shareId upsert land on
  an index prefix. (ciDashboardFieldFilter(..., 'state', ...) — one of the four
  dashboard counts — filters crystal.state, so it uses the
  {thingtime, crystal.repository} prefix with a residual, not the third key;
  the comment claims index coverage only for the crystal.status counts, so it
  is accurate.)
• thingsIndexPlanNames() cannot drift from the plan. It replays the plan
  against an in-memory recorder, its default-name derivation matches MongoDB's,
  and things_device_ttl — the one index migrateDeviceIndexLayout creates
  outside the parallel ensure — is also in createThingsDataIndexes, so the
  rebuild owns the complete set rather than reporting part of it as skipped.
• rebuild-things-indexes pending() does fire for the case it exists for.
  Its threshold is per-index > max(8 × dataBytes, 64 MB). After relocation the
  ~4.5k remaining docs put dataBytes in the low tens of MB, and the wildcard
  text index alone (582 MB measured) clears the threshold, so the panel does not
  report "nothing pending" while 1.5 GB of index files are still held.
  runMigration does not gate on pending() in any case.
• Retention classes line up with the writers: webhooks.ts:339 is the only
  job: external id, and githubClient.ts:751 reconciles top-level runs with a
  bare run id (90-day activity class). ci-dispatch is permanent, so a claim
  cannot expire under a running workflow.

Observations (deliberately not changed)

• relocationShareId's ci-relocated-<_id> fallback protects against the
  shareId-less data-loss case an earlier pass reproduced. Its trade is that such
  a row lands on the satellite under a synthetic id, so a live writer would later
  create a second projection for the same entity under the deterministic id. That
  is the right trade (a visible duplicate over silent loss), and production is
  not expected to hold any shareId-less CI row.
• formatBytes (server, migrations.ts) and formatGenerationBytes (client,
  migrationUiCore.ts) are near-duplicates. Sharing them would mean importing a
  component-directory module into api/utils, which is the wrong direction; the
  duplication is the cheaper of the two.

Validation

Run in the PR worktree at 5f3fbbaa (deps via pnpm install --frozen-lockfile,
matching web-ci.yml):

Suite	Result
test:collections	37 pass / 0 fail (36 + the new peak guard)
test:migrations	52 pass / 0 fail
test:ci-control	59 pass / 0 fail
test:schemas	109 pass / 0 fail
test:api-capabilities	5 pass / 0 fail
test:client-errors	51 pass / 0 fail

• eslint clean on collections.ts, migrations.ts, ciControlRelocationCore.ts,
  store.ts, retentionCore.ts, ingestPolicyCore.ts, MigrationsPanel.tsx,
  migrationUiCore.ts, and indexBudget.test.ts.
• typecheck:ratchet reports 109 vs baseline 108 both with and without my
  change (verified by removing the file and re-running). The +1 is
  pre-existing on develop: the same { name: 'branch', … } field with no
  description sits at registry.ts:1794 on the base and :1802 on the head —
  this PR shifted its line number, it did not introduce it. Non-blocking, and
  the check is green.
• The new guard is a real guard, not a tautology: adding one index to
  createThingsDataIndexes and re-running gives
  not ok 13 — the boot ensure fits the 64-index cap at its PEAK, not just at rest / the boot ensure peaks at 65/64 index slots … while
  ok 12 — current Things index plan keeps four slots free below MongoDB hard limit still passes — which is precisely the gap it closes. Reverted, back
  to 37/0.

Disposition

Sound, and the best-evidenced change in this repository. Ready to merge. The
README rollout order is correct: deploy → drain relocate-ci-control-telemetry
until pending reads 0 → rebuild-things-indexes. Worth knowing for step 1:
the first boot spends its entire index headroom on the six concurrent swaps, so
an ad-hoc operator index or an orphan __rebuild twin on things_v2 at that
moment is what moves it onto the drop-then-create path.

View Lopu workflow run

[github-actions]

Lopu review — PR #583 (208f4875)

All checks are green and there is no CodeQL alert on this head, so this is a
code review, not a repair. The audit behind this PR is the most
carefully-evidenced change I've reviewed in this repo — so rather than restate
it, I stood up a MongoDB 8.0.29 replica set (the version named in the audit)
and re-derived the claims the change is load-bearing on.

Independently confirmed on 8.0.29

• Same-key indexes differing only by partialFilterExpression coexist → the
  kind_* swap really is slot-safe, never a no-index window.
• The partial index is used by {kind:'post'} and {kind:{$in:[…]}}, and the
  feed $or still plans SORT_MERGE with no blocking sort. The two shapes that
  would fall back to COLLSCAN ($in containing null, $ne) don't occur
  anywhere in the codebase — I checked every kind predicate.
• The rebuild holds uniqueness throughout: 79 concurrent duplicate-shareId
  inserts, 0 accepted; text index round-trips with identical weights and
  $text keeps working; index name set identical before/after, zero twins left.

Two defects found, both reproduced and fixed in this branch

1. relocate-ci-control-telemetry silently lost rows with no shareId.
   things indexes shareId unique sparse, so a legacy ci-* row can carry
   none — and this migration sweeps the whole history of the collection. Keyed on
   a missing shareId the upsert filter serializes to {shareId: null}: the first
   such row inserts a null-shareId doc and every later one matches it, is
   counted as copied, and is then deleted from things. Repro: 4 rows in (2
   without shareId) → report said copied: 4, deleted: 4, satellite held 3.
   Silent, unbounded, unrecoverable.
   Fix: relocationShareId() falls back to a deterministic ci-relocated-<_id>
   key — _id is immutable and the batch cursor already depends on it, so
   re-runs stay insert-if-absent, and it can't collide with a real ci- + 48-hex
   id. Repro now gives 4 in → 4 out with a truthful count.

2. The boot ensure could abort rebuild-things-indexes mid-run. This PR's own
   pruneRebuildTwins runs inside ensureIndexes, which registerUser reaches —
   so an ordinary signup on a fresh serverless instance can fire during the
   multi-minute step-3 rebuild, drop the live __rebuild twin, and make the
   rebuild's own dropIndex raise IndexNotFound (27). Repro: THREW code=27,
   run dies partway, skipping the remaining indexes and the closing
   ensurePlan().
   Fix: dropIndexIfPresent — every drop in the rebuild means "converge this name
   to absent", so already-gone is the state it wanted. Repro now completes 3 of 3
   while the pruner drops 2 twins underneath it.

Both are covered by new unit tests (ciControlRelocationCore.test.ts, 11 → 13).
Touched cores: 66/66 green.

Notes, no change made — your call:

• §3.1 calls the dashboard status counts "index-only", but the open-PR count uses
  crystal.state while ci_control_repository_status is on crystal.status.
  Unchanged from before, and PR cardinality is small.
• The satellite TTL {expiresAt: 1} is unfiltered, so permanent entity rows add
  null entries — the pattern this PR makes partial on things. Negligible volume
  here; consistency rather than cost.
• relocateCiControlTelemetry.pending() counts up to 1.8 M docs on every
  /migrations load until it drains — the admin page will feel slow during the
  rollout window itself.
• rebuildTwinOptions approximates a compound sparse index with
  {firstField: {$exists: true}}, which is narrower than sparse's "any indexed
  field exists". Unreachable today (both sparse unique indexes are single-field,
  where the two are exactly equivalent — verified), but worth a guard if a
  compound sparse unique index is ever added, since partialFilterExpression
  can't express $or.
• ci-dispatch is the one permanent class whose cardinality grows with
  activity rather than with repo entities. Correct as written — expiring it
  would break the idempotency it exists for — but the one to watch if the Vercel
  runner is enabled broadly.
• The adjacent Docs / Documents columns on the migrations panel are a count
  and a byte total; both right, the labels invite a misread.

Rollout order in §5 looks right to me, and step 2's "re-run until pending is 0"
is now safe to take literally.

— Lopu

[github-actions]

🤖 Lopu review — no defects found. One operational note.

The measurement in the code is what makes this reviewable, and I want to
credit it: ciControlRelocationCore.ts documents production state as of
2026-09-02 — things_v2 at 1,824,527 docs of which 1,820,014 are ci-* rows
(ci-event 1.37M, ci-workflow-run 434k), growing ~270k/day, with all 64
indexes paying an entry per row: 3.15 GB of index for ~4.5k content documents.
That's a well-evidenced case for a satellite, not a speculative refactor.

Architecture fit. This is consistent with FUNDAMENTALS.md §3 rather than a
departure from it — the everything-is-a-thing rule already carves out
single-purpose satellites (sessions, passwordResets, authOtps, email_*,
rosters), and high-volume machine telemetry with its own retention policy is
exactly that shape. FUNDAMENTALS.md, AI_ALL.md and docs/architecture/ are
updated in the same change, so the canonical list stays truthful — the part these
migrations usually get wrong.

Migration properties, all the ones I'd want on a 1.8M-doc relocation:

• Pure over injected collection handles, so the batch logic is genuinely
  unit-testable with fakes — that's the seam that makes the 395-line test honest.
• Resumable + idempotent: budgetMs (120s default) keeps a run inside the
  serverless limit, and drained distinguishes "source exhausted" from "budget
  stopped it early".
• Batched at 500 via bulkWrite; dryRun supported; assertLease() injectable
  so a long run can verify it still holds its lease instead of racing a second
  worker; per-kind accounting so a partial run is auditable.
• Retention applied during relocation, so expired rows are dropped rather
  than copied — it won't carry 1.8M docs of dead telemetry into the new satellite.
• baseTimeOf guards with instanceof Date + Number.isFinite, so a malformed
  timestamp can't produce an Invalid Date expiry.

💭 Operational note, not a code finding: the code is right; the risk here lives
in running it. I'd go dryRun first against production and watch the
byKind / drained report across a few budgeted runs before letting it drain.
The design clearly anticipates exactly that — which is why I'm noting it rather
than flagging it.

All checks green (API suite, typecheck ratchet, both CodeQL analyses); CodeQL
0 alerts. No changes requested.

[github-actions]

🌸 Lopu review — PR #583 (59f0892e) — one real defect found and fixed, plus a lot of independent re-verification.

All checks green, MERGEABLE/CLEAN, 0 open CodeQL alerts on this head. So instead of re-reading the diff a fourth time, I spent this pass reproducing the load-bearing claims against a real MongoDB 8.0 replica set, driving this branch's own source. Nearly everything held. One thing did not.

---

The migration promised something it did not do

rebuild-things-indexes tells the operator, in the text rendered on /migrations:

The wildcard text index is rebuilt last, so ranked text search errors for the seconds it takes to build.

It wasn't. rebuildPlanIndexes walked listIndexes order — which is creation order — and replaying the real plan against 8.0 put things_text_search 36th of 57.

That ordering carries more weight than it looks like it does. Every other plan index is degrade-only while it's missing: the query falls back to a scan of a now-small collection, exactly as the description says. The text index is the one exception — $text against a collection with no text index is a hard failure, not a slow path:

$text WITHOUT a text index: ERROR code 27 IndexNotFound - text index required for $text query

and things/search.ts:578 puts $text straight into ranked search. So the single hard-failure window in a multi-minute destructive migration was landing at an arbitrary point in the middle of the run, while the operator had been told it lands at the end.

Fixed in ciControlRelocationCore.ts: text indexes sort last (sort() is stable, so every other index keeps its creation order), and the {_fts,_ftsx} test is extracted into isTextIndexDefinition so the ordering and the recreate path can't drift apart about what a text index even is. things_text_search is now 57 of 57, and the dry run previews the same order the real run takes. One new unit test covers both.

I also renamed two adjacent columns in the /migrations storage table: Docs (a count) sat next to Documents (bytes). That table is where someone decides to run a destructive migration, so Doc bytes and Index bytes · count seemed worth one word.

---

What I re-verified live, and what it says about this PR

I want to be specific about this, because the result is genuinely unusual:

thingsIndexPlanNames() vs the names MongoDB actually assigns	57/57 exact — nothing in the collection outside the plan, nothing in the plan uncreated
Full 57-index rebuild on the real plan	name set and the entire listIndexes output byte-identical before/after; 10 unique constraints twinned; 0 leftover twins; $text working after
Wildcard text index round-trip	{_fts,_ftsx} key and all six weights identical
Unique twin, sparse original (shareId_1) and partial original (things_reaction_unique)	coexists, enforces E11000 while the original is dropped, original restored
Relocation cursor at scale	LIMIT ← FETCH ← IXSCAN{_id_}, no blocking sort, 500 docs examined per batch
Relocation drain, 60k rows	11.4 s, drained, non-CI doc untouched, expiresAt/schemaVersion stamped (≈350 s for production's 1.82 M — the 120 s budget + re-run design is sized right)
relocate-ci-control-telemetry pending() at 500k rows	353 ms, index-only, 0 docs examined

That 57/57 line is the one I'd have bet against. Deriving plan-owned index names by replaying the plan through an in-memory recorder is the kind of thing that is almost right — one $**, one unnamed compound, one 2dsphere, and the rebuild silently reports an index as skipped, leaves its file at full size, and reports success. It's exact. Same for the text-index recreate: reconstructing {'$**':'text', 'crystal.name':'text', …} from a weights map and having MongoDB accept it and produce a byte-identical index is not obviously going to work, and it does.

The plan-name derivation is the right design, too — the plan is only ever expressed as createIndex calls, so replaying it is the only definition that can't drift. Worth keeping that property in mind next time someone is tempted to hand-maintain the list.

Things I checked and deliberately did not change

• getCollection('things') in the relocation vs getHomeThingtimeDb() in the rebuild. This looked like a real cross-database hazard — read from a session's override endpoint, write to home. It isn't: server/routes/api/[...].ts:355 pins every v1/admin/ route to home explicitly, with a comment saying why. Correct as written.
• ciControl in the query-workbench allowlist — admin-gated, rate-limited, read-only, bounded.
• CI_CONTROL_THINGTIME is the single source for both the protected-kind list and the migration's kinds, so no kind can be stranded in things while its readers look at the satellite.

One thing for you, not for me to decide

0 retention doesn't un-expire rows that are already stamped. Entities self-heal — upsertCiEntity $unsets a stale expiresAt on the next accepted update — but ci-event rows are immutable and only ever get their stamp in $setOnInsert. So an operator who sets THINGTIME_CI_EVENT_RETENTION_DAYS=0 because they want to keep history will still watch the already-stamped events get reaped, and README/apiDocs both read as though 0 protects them.

I left it alone on purpose: the honest fix might be a sweep that $unsets the class rather than a caveat in the docs, and that's a product call about what 0 should mean, not a reviewer's call to quietly make.

---

Rollout order in the PR body is right and worth following as written: deploy (boot alone prunes the dead indexes), drain relocate-ci-control-telemetry until pending is 0, then rebuild-things-indexes. Running the rebuild first does nothing useful — and its pending() correctly reads 0 until the relocation has actually shrunk the documents, which is a nice touch.

No blocking concerns.

[github-actions]

🌸 Lopu review — PR #583 (426830d9) — two defects found and fixed, both reproduced first.

All checks green, MERGEABLE/CLEAN, 0 open CodeQL alerts on this head (trusted snapshot empty, dispositions left as []). So this is a code review, not a repair.

Four earlier passes covered the partial-kind planner assumption, the relocation cursor, shareId-less rows, and the text-index rebuild ordering. I didn't re-litigate those — I went at what they hadn't reached, which turned out to be the two seams around this change rather than inside it.

---

1. The twin prune could silently unprotect the key it promises to hold

rebuild-things-indexes tells the operator, in the text rendered on /migrations:

Unique indexes are protected by a same-key twin throughout, so no duplicate can slip in mid-rebuild.

Round 2 found that ensureIndexes → pruneRebuildTwins can drop the live __rebuild twin underneath a running rebuild, and fixed the resulting crash (dropIndexIfPresent). But surviving the drop isn't the same as being correct. The pruner dropped every twin unconditionally — including the one whose original is absent because the rebuild is between its dropIndex and createIndex. In that window the twin is the only thing holding the unique key.

That window isn't exotic: mongo-warmup fires ensureIndexes() on every serverless cold start, and the rebuild runs for minutes.

Replayed the exact sequence on a MongoDB 8.0 replica set — twin created → original dropped → prune runs → 20 duplicate-shareId inserts:

	twin after prune	accepted	rejected (E11000)
before	dropped	20	0
after	preserved	1	19

Twenty duplicates on a key the migration was reporting as twinned.

Fix: the prune now decides per twin. Original present → pure redundancy, dropped exactly as before — and that's the shape that actually parked the collection at the cap in round 2 (an aborted run that created all its twins up front), so it keeps doing the job it was added for. Orphan → left to the rebuild's own reconcileRebuildTwins, unless there's no free slot at all, where a stuck boot ensure is the worse failure and the plan recreates the original moments later. MONGODB_COLLECTION_INDEX_LIMIT is a named export now instead of a number repeated in prose; three tests cover the three branches.

2. relocate-ci-control-telemetry read the wrong database plane

Its source was getCollection('things') — the request's active endpoint — while its target getCiControlCollection() is home-pinned like every satellite.

The data-plane migrations in that file follow the active endpoint on purpose, and that's right for them. But ci-* rows are control plane: every writer used getHomeThingsCollection() before this change, and endpoint.ts says it outright — identity, protected kinds and "every satellite collection" stay on home. rebuild-things-indexes, twenty lines below, already pins itself with getHomeThingtimeDb().

Every API route runs inside runWithMongoEndpoint (server/routes/api/[...].ts:359), so an admin holding a tt_mongo override would have had this destructive migration read, copy and deleteMany against their own database into Thingtime's home ciControl_v1. And in the ordinary case — an override database with no ci-* rows — it reports drained: true and pending: 0 while production things_v2 still holds all 1.8M rows, after which step 3 rebuilds indexes on an uncleaned collection. Both pending() and run() are pinned to getHomeThingsCollection() now.

---

Re-verified, nothing to change

The one I most wanted to check was the wildcard text index round-trip, because indexCreateSpecFromDefinition reconstructs it from listIndexes — which reports {_fts,_ftsx}, not the declared keys — and a failure there would land on the last index of a destructive run and leave ranked search erroring until a cold start. Confirmed on 8.0: the derived key ($** plus every weighted path) is accepted, weights come back byte-identical, $text works after. Mixing $** with named text paths is legal.

Also clean: ciControl registers through COLLECTION_SCHEMA_VERSIONS so there's no second list to drift; upsertCiEntity's $set/$unset of expiresAt are mutually exclusive and clear a stale stamp when a row becomes permanent; and the twin's partial filter is faithful for every unique index actually in the plan — the only sparse uniques (shareId_1, uniqueKeys_1) are single-field, where {field: {$exists: true}} matches sparse semantics exactly.

Two notes I did not act on

• ci-dispatch is the one CI kind whose id isn't bounded by repository cardinality. Retention class permanent, but claimCiDispatchRoute's automatic:<workflow>:<deliveryKey> is per-delivery. Dormant today — all eight workflows are defaultProvider: 'github-actions', which returns before the claim — and changing its retention would also govern feature-stack run history, so that's your call, not mine.
• The relocation's bulkWrite can raise E11000 when a live writer inserts the same deterministic shareId in the gap between the upsert's read and insert. Fails safe (the batch's deleteMany never runs, a re-run matches the live row) but surfaces as a failed run rather than a skipped row.

Validation

test:collections 36 ✅ · test:migrations 51 ✅ · test:ci-control 59 ✅ · test:schemas 109 ✅ · test:api-capabilities 5 ✅. Typecheck ratchet reports 109 both with and without my change (verified by stashing), so the +1 over the recorded 108 baseline predates this round and no file I touched reports an error.

The audit behind this PR remains the best-evidenced change I've reviewed in this repo — both defects I found are in the machinery around the measurement, not in the measurement itself.

— Lopu, Thingtime's principal developer and repository steward. Using Claude Opus 5.

[github-actions]

🌸 Lopu review — PR #583 (59f7c5f8) — three defects found and fixed, all on the operator-facing surface rather than the data path.

All checks green, MERGEABLE/CLEAN, 0 open CodeQL alerts on this head (trusted snapshot empty, dispositions left []). So this is a code review, not a repair.

Six earlier passes have worked over the load-bearing seams — the partial-kind planner assumption, the relocation cursor, shareId-less rows, the text-index rebuild ordering, the twin prune, the home-plane pinning. I didn't re-litigate any of those. I went at the surface none of them had reached: the page the operator actually runs this rollout from.

---

1. The advisory storage census could 500 the endpoint that runs this PR's own rollout

collectionStorage() rethrew every $collStats failure except NamespaceNotFound (26). It's called once per physical collection from getMigrationStatus(), and again from rebuild-things-indexes's pending() — so one refused $collStats turns GET /api/v1/admin/migrations into a 500.

That endpoint backs /migrations, which is the only in-app way to run relocate-ci-control-telemetry and rebuild-things-indexes. $collStats isn't universally available: it's rejected outright on a view (CommandNotSupportedOnView 166 — and listCollections returns views), and withheld on some managed tiers. Trading the rollout path for a decorative byte count is the wrong way round.

What makes this clearly a defect rather than a judgement call: the code was already written to survive an absent census. docs falls back to estimatedDocumentCount(), pending() returns 0, and the panel's formatGenerationBytes renders —. The one state the server could never actually produce was the null one.

Fix: the census is now genuinely advisory — any $collStats failure degrades to "no census". I couldn't stand up a rejecting cluster in the runner, so I'm citing the view case and the design argument, not a live repro.

2. An unavailable census serialized as 0 — which reads as "empty"

CollectionGenerationStatus typed the four census fields required and getMigrationStatus filled them with ?? 0. But MigrationsPanel.tsx types them optional and carries this comment:

// storage census (older servers omit these — render as unknown, never 0)

The client was right; the server contradicted it. A generation whose census couldn't be taken rendered 0 B / 0 B · 0 — exactly what a genuinely empty collection looks like, on the page where an operator decides what's safe to drop and whether index bloat justifies a destructive rebuild. "Unknown" and "empty" are the two answers that must never collide there.

Fix: optional on the server type too, and omitted rather than zeroed, so the panel's existing "unknown" path is the one that fires. No other consumer reads them.

3. The dry run told the operator to do something impossible

On the 1.8M-row production collection the dry run hits the 120s budget every time. The note it printed:

Time budget reached (120s): more rows remain — run this migration again until pending reads 0

A dry run writes nothing, so pending cannot move. This is the first thing the operator sees at the start of a destructive rollout, and it's an instruction that can never be satisfied. Now dry-run aware, and says what's true: the counts are a sample of the rows scanned so far, and pending() is the authoritative total.

(Also merged a duplicate ./migrationUiCore import in MigrationsPanel.tsx and dropped a stray trailing line — both introduced by this PR.)

---

Re-verified, nothing to change

• The relocation's data safety holds. deleteMany runs only after a successful bulkWrite — a throw aborts before any delete and the re-run is idempotent — and the sort({_id:1}).limit(n) cursor uses a bounded top-K sort, so it can't trip the 32 MB blocking-sort ceiling on a 1.8M-row scan.
• No reader was left behind. The three files in api/utils/ciControl/ the diff doesn't touch (adminPreviewDeployments, vercelRunner, dashboardQueryCore) reach no collection directly, so they follow store.ts to the satellite for free. CI_CONTROL_THINGTIME covers all 13 written kinds including the feature-stack pair, and CI_THINGTIME aliases the same array so clearCiControlForTests can't drift.
• Retiring thingtime_1_parentId_1_createdAt_-1_shareId_1 is safe — grepping every parentId predicate paired with thingtime finds exactly two call sites, both inside api/utils/ciControl/. The comment's claim checks out.
• rebuildTwinOptions' sparse heuristic ({fields[0]: {$exists: true}}) is narrower than sparse semantics for a compound sparse unique index — but the only two sparse uniques in the plan are single-field, where they're exactly equivalent. Correct today; worth remembering if a compound one is ever added.

Validation

test:collections 36 ✅ · test:migrations 52 ✅ (51 + the new census test) · test:ci-control 59 ✅ · test:schemas 109 ✅ · test:api-capabilities 5 ✅ · test:client-errors 51 ✅ · eslint clean on all four files. Typecheck reports 109 both with and without my change (verified by stashing) and none in a file I touched.

The new test is a real guard: reverting collectionStorage to the pre-fix throw and re-running gives not ok 47 — the storage census reads $collStats and degrades to no census instead of failing the endpoint (51/1). Restored, back to 52/0.

---

The audit behind this PR remains the best-evidenced change in this repo, and the migration machinery itself I found sound. All three defects this round are in the reporting around it — which is a fair place for them to hide after six passes on the data path, and a bad place to leave them when the next thing that happens is someone dropping 1.8M rows from production.

Ready to merge. Rollout sequence in the README is right: deploy → drain relocate-ci-control-telemetry until pending reads 0 → rebuild-things-indexes.

— Lopu, Thingtime's principal developer and repository steward. Using Claude Opus 5.

[github-actions]

🌸 Lopu review — PR #583 (5f3fbbaa) — one finding, in the guard rather than the code. It comes with a number nobody had measured.

All checks green, MERGEABLE, 0 open CodeQL alerts on this head (trusted snapshot empty, dispositions left []). So this is a code review, not a repair.

Seven earlier passes have worked the data path and, last round, the operator-facing surface. I didn't re-litigate any of it. I went at the one thing none of them had put a number on: what this boot actually costs in index slots on the collection that is already at the cap.

---

The headroom test measures the plan at rest. The boot is not at rest.

indexBudget.test.ts asserts plan + 4 ≤ 64 — "reserve 4 slots for safe upgrades." True of the plan's final set. Not true of the boot that has to fit.

The five kind_* swaps and the sandbox TTL swap are create-then-drop on purpose — the replacement is built before the original is dropped, so no database ever sits without the index. And the whole plan runs under one Promise.all. So on the first boot after this deploy, a collection already converged to the plan still holds all six originals, and holds both halves of all six swaps at once.

I replayed ensureHomeThingsIndexPlan against a fake that models occupancy over time — create takes a slot, drop returns one, setImmediate lets the fan-out interleave the way the driver does:

	slots
steady state	58 / 64
transient peak during the boot swap	64 / 64

The entire 4-slot headroom the test promises is spent, with nothing left over.

This is not a defect. 64 is inclusive, the swap succeeds, and createIndexReplacing's CannotCreateIndex (67) fallback is already implemented and already tested for the case where it doesn't. The gap is that the guard cannot see the number it is guarding: a 59th plan index, or a seventh swap pending on the same boot, moves production off the slot-safe path and onto drop-then-create — a real index-less window on things_v2 — and every existing test still passes.

Fix (test only): a companion guard that models occupancy over time and asserts the peak fits, that each pending swap costs exactly one transient slot, and that every swapped original is dropped once its replacement exists. It is a real guard, not a tautology — adding one index to the plan and re-running gives:

ok 12 - current Things index plan keeps four slots free below MongoDB hard limit
not ok 13 - the boot ensure fits the 64-index cap at its PEAK, not just at rest
  error: 'the boot ensure peaks at 65/64 index slots; above the cap the swaps
          degrade to drop-then-create, leaving things_v2 without those indexes mid-boot'

The steady-state test passing while the peak test fails is precisely the gap. Reverted, back to 37/0.

Worth knowing for the rollout: step 1 spends the whole headroom. An ad-hoc operator index or an orphan __rebuild twin sitting on things_v2 at that moment is what tips the first boot onto the degraded path. It self-heals on the next boot — the replacements exist by then — but it's the kind of thing better known before than after.

---

Verified, nothing to change

• The thingtime[0] derivation cannot lose user content — and it was worth checking. The relocation matches {thingtime: {$in: CI_CONTROL_THINGTIME}} (any array element) but relocatedCiDoc derives the retention kind from thingtime[0]. Multi-element arrays are real here (['post','comment'], ['post','share'] in things.ts), so a mixed array would be relocated-and-deleted under the wrong kind by a destructive migration. It cannot arise: isProtectedThingtime uses .some() over the array and is enforced on both generic create (things.ts:1013) and generic update (things.ts:4076). Every matching row is server-minted with exactly [kind].
• No secret follows the rows to the satellite. CI credentials live in lopuCredentials, not on ci-* Things, and no writer under api/utils/ciControl/ sets secure or uniqueKeys — so ciControl correctly does not need MONGO_PROTECTED_FIELD_QUERY_COLLECTIONS, and adding it to the workbench allowlist moves no trust boundary.
• One registry edit registers the satellite everywhere. COLLECTIONS derives from COLLECTION_SCHEMA_VERSIONS, so ciControl: 1 is also what makes drop-stale-collection-generations classify ciControl_v1 as current rather than unknown residue.
• thingsIndexPlanNames() owns the complete set. things_device_ttl — the one index migrateDeviceIndexLayout creates outside the parallel ensure — is also in createThingsDataIndexes, so the rebuild doesn't quietly report part of the collection as skipped.
• rebuild-things-indexes pending() fires for the case it exists for. Per-index > max(8 × dataBytes, 64 MB): after relocation dataBytes is in the low tens of MB and the 582 MB text index alone clears it, so the panel won't read "nothing pending" while 1.5 GB of index files are still held. (runMigration doesn't gate on pending() anyway.)
• Retention classes match the writers: webhooks.ts:339 is the only job: external id, githubClient.ts:751 reconciles top-level runs into the 90-day class, and ci-dispatch is permanent — a claim can't expire under a running workflow.

Validation

test:collections 37 ✅ (36 + the new guard) · test:migrations 52 ✅ · test:ci-control 59 ✅ · test:schemas 109 ✅ · test:api-capabilities 5 ✅ · test:client-errors 51 ✅ · eslint clean on all nine files I read closely.

typecheck:ratchet reports 109 vs baseline 108 both with and without my change (verified by removing the file and re-running). The +1 is pre-existing on develop: the same { name: 'branch', … } field with no description sits at registry.ts:1794 on the base and :1802 on the head — this PR shifted its line number, it did not introduce it.

---

Ready to merge. Rollout order in the README is right: deploy → drain relocate-ci-control-telemetry until pending reads 0 → rebuild-things-indexes.

— Lopu, Thingtime's principal developer and repository steward. Using Claude Opus 5.