fix(storage): add idx_traces_ts to unblock event loop on newest-first trace reads

[kiwipaulrob]

Summary

Fixes a production daemon freeze/restart-storm caused by an unindexed newest-first trace read that blocked the Node event loop. Adds one index migration plus a migrator heal for release-train version collisions.

Problem

On an install whose traces table reached ~30k rows (~235 MB with embedding blobs + tool-call JSON), the memos-local-plugin daemon:

• never answered HTTP requests after boot (TCP accepted into the kernel backlog, zero bytes served)
• spent 40–60s per boot at ~100% CPU inside a single synchronous SQLite call
• was restarted every ~3 minutes by our liveness watchdog — 321 restarts in one day, self-sustaining because each doomed generation re-ran the scan at boot and was killed mid-scan

Root cause (CPU-profiled)

A --cpu-prof capture showed 28% of all samples in one statement: all() @ connection.js under two call paths:

1. /api/v1/health → latestTraceTs() → traces.list({ limit: 1 }) (three calls per health request)
2. bootstrap recent-events replay (createPipeline → traces.list({ limit: 30 }))

Both issue SELECT ... FROM traces ORDER BY ts DESC, id DESC LIMIT n. Every existing traces index leads with owner_agent_kind, owner_profile_id, share_scope, session_id, or episode_id, so the unfiltered newest-first read degenerates to:

EXPLAIN QUERY PLAN SELECT * FROM traces ORDER BY ts DESC, id DESC LIMIT 1;
-- SCAN traces
-- USE TEMP B-TREE FOR ORDER BY

Because better-sqlite3 executes statements synchronously on the JS event loop, the scan blocks all request handling while it runs.

Fix

1. 013-traces-ts-index.sql — CREATE INDEX IF NOT EXISTS idx_traces_ts ON traces(ts DESC, id DESC).
2. migrator.ts — same tableExists("traces") guard pattern as 012 (partial test schemas must not fail), plus a release-train collision heal: databases migrated by the other train carry schema_migrations rows whose version numbers collide under different names (observed live: (13,'skill-repair-origin'), (14,'episode-outcome')). Version-only bookkeeping silently skipped any same-numbered migration from this build — including additive repair migrations like this one. Guarded/additive migrations now still apply under such collisions and repair their bookkeeping row via upsert; non-guarded migrations keep the conservative skip behaviour.
3. Regression tests for both behaviours in migrator.test.ts.

Validation

Check	Before	After
EXPLAIN of newest-first read	SCAN traces + temp B-tree	SCAN ... USING INDEX idx_traces_ts
Newest-trace lookup (warm)	~700 ms	~0.7 ms (>1000×)
Sandbox boot→serving 200 OK (prod-sized DB copy, previously never served once in 150 s)	never	<6 s
Production cutover: boot to pipeline.ready	60 s+ stuck	<1 s
Production health probe	timeout	200 OK @ ~205 ms
Restarts/day (watchdog loop)	321	0 since fix

• tests/unit/storage/: 82/82 pass
• tsc --noEmit: no errors in storage/* (remaining errors are pre-existing adapters/deepseek-harness/* optional-dep issues)
• Index build cost measured at ~1 s per 100k rows; migration is additive and idempotent

Test plan

• npx vitest run tests/unit/storage/migrator.test.ts (8/8)
• npx vitest run tests/unit/storage/ (82/82)
• CPU-profiled sandbox reproduction before/after
• Live production cutover on a storm-affected install

[Memtensor-AI]

🤖 Open Code Review

Target: PR #2284
Task: 4269caffd382f99e
Base: main
Head: fix/traces-ts-index-health-freeze

🔍 OpenCodeReview found 2 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.

---

1. apps/memos-local-plugin/core/storage/migrations/013-traces-ts-index.sql (L26)

REPAIRABLE_UNDER_NAME_COLLISION is a manually maintained parallel data structure: every guarded applyMigration case must also appear here, and the comment warns about it. Missing an entry causes silent loss of repair behaviour with no compile-time or runtime signal. Consider co-locating the repairable flag with the per-version guard (e.g. a GUARDED_MIGRATIONS map keyed by version) so the two stay in sync structurally rather than by convention.

---

2. apps/memos-local-plugin/core/storage/migrator.ts (L218-L220)

The comment correctly flags the manual sync requirement between applyMigration() guards and REPAIRABLE_UNDER_NAME_COLLISION, but there is no compile-time or runtime enforcement. A future contributor who adds a guarded branch for a new version and forgets to add its number to the set will silently lose collision-repair behaviour — isPending() will return false (the set check fails), the migration will be skipped, and no error or log line is emitted.

Recommendation: add a unit test that cross-checks the two structures — assert every version number with a named guard in applyMigration() is present in REPAIRABLE_UNDER_NAME_COLLISION. Alternatively, make the coupling structural by replacing the parallel set + if-chain with a single Map<number, (db, file) => void> and deriving the set from its keys.

💡 Suggested Change

Before:

  // Keep REPAIRABLE_UNDER_NAME_COLLISION (bottom of file) in sync when
  // adding guarded cases here — a guarded case missing from that set
  // silently loses repair-under-name-collision (the file is just skipped).

After:

  // Coupling is enforced by the unit test in migrator.test.ts:
  // "every guarded applyMigration version is in REPAIRABLE_UNDER_NAME_COLLISION"
  // Add a test assertion for any new guarded branch added here.

---

🧹 Filtered 4 low-confidence OCR finding(s) before posting/fix-loop (duplicate: 3, existing_code_mismatch: 1).

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (8/8 executed). memos_local_plugin/unit: 8/8. Duration: 3s [advisory, non-gating] AI-generated tests on branch test/auto-gen-7588bca7b16bfc30-20260826123140: 27/34 passed, 7 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/traces-ts-index-health-freeze

[kiwipaulrob]

Thanks @OPEN Code Review — both findings accepted, both addressed in d6ab77f (comment-only, no behavior change; tests/unit/storage still 82/82).

On (1) — version 11: agreed the omission deserved an inline explanation, so one is now there. One correction to the suggested rationale: 011's SQL is actually fully idempotent — every statement is CREATE TABLE/INDEX ... IF NOT EXISTS. Version 11 is excluded deliberately because it has no guarded apply path in applyMigration(), and hub-sharing is opt-in team-sharing runtime that a collision heal must never materialize implicitly. The behaviour is pinned by the existing "keeps skipping a version recorded under a foreign name when that migration is not repairable" test in migrator.test.ts. The new comment states this true rationale and warns against adding 11 without first giving 011 a guarded path.

On (2) — allowlist/guard coupling: agreed; added the cross-reference comment above the guarded case chain. If maintainers would prefer structural enforcement over comments, happy to follow up with a small variant that derives the allowlist from the guarded-case table itself so the two cannot drift.

[Memtensor-AI]

✅ Automated Test Results: PASSED

All tests passed (8/8 executed). memos_local_plugin/unit: 8/8. Duration: 3s [advisory, non-gating] AI-generated tests on branch test/auto-gen-4269caffd382f99e-20260826181419: 26/36 passed, 10 failed — these do NOT affect the PR verdict; review the branch manually.

Branch: fix/traces-ts-index-health-freeze