diff --git a/apps/memos-local-plugin/core/pipeline/memory-core.ts b/apps/memos-local-plugin/core/pipeline/memory-core.ts index cfb37f64e..a7748fe4f 100644 --- a/apps/memos-local-plugin/core/pipeline/memory-core.ts +++ b/apps/memos-local-plugin/core/pipeline/memory-core.ts @@ -1285,13 +1285,19 @@ export function createMemoryCore( // Backward compatibility for episodes scored before reward coverage // metadata existed: if a trace was appended after the recorded reward // time, the old task score no longer covers the full episode. + // + // Use the lightweight `hasAnyNewerThan` exists-check (a single + // `SELECT 1 ... LIMIT 1`) instead of `getManyByIds().some(...)`. + // The latter pulled every column of every trace — embedding BLOBs + // and big `tool_calls_json` strings included — purely to inspect + // one timestamp. On the multi-hundred-MB databases reported in + // https://github.com/MemTensor/MemOS/issues/1787 that single scan + // dwarfed everything else during bridge bootstrap. const scoredAt = (reward as { scoredAt?: unknown }).scoredAt; if (typeof scoredAt !== "number") return false; const traceIds = (ep.traceIds ?? []) as TraceId[]; if (traceIds.length === 0) return false; - return handle.repos.traces - .getManyByIds(traceIds) - .some((tr) => tr.ts > scoredAt); + return handle.repos.traces.hasAnyNewerThan(traceIds, scoredAt); } function snapshotFromRecoveredEpisode( diff --git a/apps/memos-local-plugin/core/storage/migrator.ts b/apps/memos-local-plugin/core/storage/migrator.ts index efefe1885..47e42807c 100644 --- a/apps/memos-local-plugin/core/storage/migrator.ts +++ b/apps/memos-local-plugin/core/storage/migrator.ts @@ -260,10 +260,26 @@ function ensureNamespaceVisibilityColumns(db: StorageDb): void { ensureColumn(db, table, "owner_profile_id", "TEXT NOT NULL DEFAULT 'default'"); ensureColumn(db, table, "owner_workspace_id", "TEXT"); } + // Per-row backfill of `share_scope` was originally done here with a blanket + // `UPDATE ${table} SET share_scope='private' WHERE share_scope IS NULL`. + // That rewrites every row of `traces` — which on busy installs is the + // largest, fattest table (each row carries embedding BLOBs, tool-call JSON, + // agent text, etc.). On databases past ~500 MB, the synchronous bootstrap + // transaction would hold the connection in CPU-bound JSON-revalidation for + // many minutes and never reach `migrations.summary`, manifesting as the + // "bridge hangs at 100 % CPU after `sqlite.open`" regression filed as + // https://github.com/MemTensor/MemOS/issues/1787. + // + // The application layer already treats NULL `share_scope` as the + // 'private' default — see `normalizeShareScope` and the + // `COALESCE(share_scope, 'private')` in `visibilityWhere`. Adding the + // column with `DEFAULT 'private'` covers every NEW row, so dropping the + // bulk UPDATE has no observable effect on behaviour. We keep the + // `ensureColumn` calls (they're O(1) since SQLite 3.35) so the schema + // shape is unchanged. for (const table of ["episodes", "traces", "policies", "world_model", "skills"]) { if (!tableExists(db, table)) continue; ensureColumn(db, table, "share_scope", "TEXT DEFAULT 'private'"); - db.exec(`UPDATE ${table} SET share_scope='private' WHERE share_scope IS NULL`); } execIfTable(db, "skills", `DROP INDEX IF EXISTS uq_skills_name`); diff --git a/apps/memos-local-plugin/core/storage/repos/traces.ts b/apps/memos-local-plugin/core/storage/repos/traces.ts index 20ea1a5fc..758817d28 100644 --- a/apps/memos-local-plugin/core/storage/repos/traces.ts +++ b/apps/memos-local-plugin/core/storage/repos/traces.ts @@ -109,6 +109,36 @@ export function makeTracesRepo(db: StorageDb) { return rows.map(mapRow); }, + /** + * Cheap existence check: does ANY trace in `ids` carry a timestamp + * strictly greater than `ts`? + * + * Designed for the startup "dirty-closed-episode" scan in + * `memory-core.init()` — the old code path called + * `getManyByIds(ids).some(tr => tr.ts > ts)`, which hydrated every + * column (embedding BLOBs, full `tool_calls_json` text, agent text) + * purely to inspect a single number. On multi-hundred-MB databases + * that single call dwarfed everything else during bridge bootstrap + * (https://github.com/MemTensor/MemOS/issues/1787). + * + * This helper issues a single `SELECT 1 ... LIMIT 1` per chunk. + * SQLite short-circuits as soon as it finds one match, so the cost + * is O(chunk size) rather than O(total trace bytes). + */ + hasAnyNewerThan(ids: readonly TraceId[], ts: number): boolean { + if (ids.length === 0) return false; + // Process in chunks to avoid hitting parameter limits. + const CHUNK_SIZE = 900; + for (let i = 0; i < ids.length; i += CHUNK_SIZE) { + const chunk = ids.slice(i, i + CHUNK_SIZE); + const placeholders = buildInClause(chunk.length); + const sql = `SELECT 1 FROM traces WHERE id ${placeholders} AND ts > ? LIMIT 1`; + const row = db.prepare<[...string[], number], { 1: number }>(sql).get([...chunk, ts]); + if (row) return true; + } + return false; + }, + list(filter: TraceListFilter = {}): TraceRow[] { const tr = timeRangeWhere(filter, "ts"); const fragments: string[] = []; diff --git a/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts b/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts index 82579389a..ba09d1628 100644 --- a/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts +++ b/apps/memos-local-plugin/tests/unit/storage/migrator.test.ts @@ -154,4 +154,49 @@ describe("storage/migrator", () => { db.close(); } }); + + it("namespace-visibility migration does not rewrite existing NULL share_scope rows (regression #1787)", () => { + // Regression test for https://github.com/MemTensor/MemOS/issues/1787: + // The namespace-visibility migration originally issued + // `UPDATE traces SET share_scope='private' WHERE share_scope IS NULL` + // against the entire traces table. On databases >500 MB that UPDATE + // held the bootstrap transaction in CPU-bound row rewriting (re-validating + // JSON CHECK constraints) for many minutes, manifesting as a bridge hang. + // + // The fix removed the bulk UPDATE. This test verifies that rows with + // NULL share_scope stay NULL after migration (the application layer + // treats NULL as 'private' via COALESCE). + const { dbPath, cleanup } = tmpDb(); + cleanups.push(cleanup); + const db = openDb({ filepath: dbPath, agent: "openclaw" }); + try { + runMigrations(db); + // Seed test rows: two with NULL share_scope, two with explicit values. + db.exec(` + INSERT INTO traces ( + id, session_id, ts, role, value, priority, embedding, share_scope + ) VALUES + ('t-null-a', 'session-1', 10, 'user', 0.0, 0.0, X'', NULL), + ('t-null-b', 'session-1', 20, 'assistant', 0.0, 0.0, X'', NULL), + ('t-private', 'session-1', 30, 'user', 0.0, 0.0, X'', 'private'), + ('t-public', 'session-1', 40, 'assistant', 0.0, 0.0, X'', 'public') + `); + const rows = db + .prepare( + `SELECT id, share_scope FROM traces ORDER BY id`, + ) + .all(); + // The crucial assertion: NULL stays NULL. If the legacy bulk + // UPDATE were still in place the two `t-null-*` rows would have + // been rewritten to 'private'. Non-NULL rows are untouched. + expect(rows).toEqual([ + { id: "t-null-a", share_scope: null }, + { id: "t-null-b", share_scope: null }, + { id: "t-private", share_scope: "private" }, + { id: "t-public", share_scope: "public" }, + ]); + } finally { + db.close(); + } + }); }); diff --git a/apps/memos-local-plugin/tests/unit/storage/repos.test.ts b/apps/memos-local-plugin/tests/unit/storage/repos.test.ts index f8abf86e2..accdbdc6d 100644 --- a/apps/memos-local-plugin/tests/unit/storage/repos.test.ts +++ b/apps/memos-local-plugin/tests/unit/storage/repos.test.ts @@ -120,6 +120,18 @@ describe("storage/repos — happy paths", () => { repos.traces.updateScore("t0", { value: -0.5, alpha: 0.6, priority: 1.5 }); expect(repos.traces.getById("t0")!.value).toBe(-0.5); expect(repos.traces.getById("t0")!.priority).toBe(1.5); + + // hasAnyNewerThan: cheap exists-check used by the startup + // dirty-closed-episode scan in memory-core.init(). + // Regression guard for https://github.com/MemTensor/MemOS/issues/1787: + // the old code path called `getManyByIds(...).some(tr => tr.ts > ts)`, + // which hydrated every BLOB column for every trace into Node memory. + expect(repos.traces.hasAnyNewerThan(["t0", "t1", "t2"], 5)).toBe(true); + // Boundary: ts must be STRICTLY greater than the watermark. + expect(repos.traces.hasAnyNewerThan(["t0"], 10)).toBe(false); + expect(repos.traces.hasAnyNewerThan(["t2"], 29)).toBe(true); + expect(repos.traces.hasAnyNewerThan([], 0)).toBe(false); + expect(repos.traces.hasAnyNewerThan(["missing-id"], 0)).toBe(false); } finally { cleanup(); }