fix(memory): harden memory followups

docs/issues/pr1794-memory-hardening/plan.md

@@ -0,0 +1,39 @@
+# Plan
+
+## Core memory runtime
+
+1. Change extraction parsing to return a discriminated result so parse errors can propagate as `ok:false` from `extractAndStore()`.
+2. Filter memory extraction spans to user-visible text only; exclude assistant reasoning fields.
+3. Add a current-embedding fingerprint guard and/or queue wait before reindex reset to prevent stale drains from writing old vectors.
+4. Gate `restoreMemory()` with `canWriteAgentMemory()` and delete vectors when rows are archived/forgotten.
+5. Update memory FTS query construction to tokenize multi-word queries while retaining safe quoting.
+
+## Database and FTS
+
+1. Extend database security copy to preserve non-shadow indexes and triggers, or run a schema repair/rebuild pass after copy.
+2. Add FTS meta/version/tokenizer tracking to agent memory FTS and rebuild when capability differs.
+3. Make tape search FTS replacement/deletion stable by using a deterministic rowid or external-content style rebuild/delete path.
+4. Make clear/reset paths tolerate missing FTS/meta tables.
+5. Seed schema version for newly created databases or otherwise avoid running historical migrations against freshly-created latest schemas.
+
+## Renderer UI
+
+1. Add top-level MemorySettings error/finally handling.
+2. Add request-id guards to MemoryManagerPanel refresh.
+3. Surface search errors distinctly from empty results.
+4. Rename default destructive memory operations to archive/restore semantics; keep permanent delete as an explicit dangerous action if still exposed.
+5. Move/label advanced retrieval tuning to reduce accidental misuse.
+6. Localize all newly added memory management and advanced settings copy in every supported locale, preserving interpolation placeholders.
+
+## Tests
+
+- Main tests for extraction parse failure cursor behavior and reasoning exclusion.
+- Main tests for reindex/drain stale guard and restore disabled behavior.
+- SQLite tests for FTS tokenizer rebuild and stale token replacement.
+- Database security migration tests for indexes/triggers if existing harness supports it.
+- Renderer tests for MemorySettings error state and stale refresh/search behavior.
+
+## Compatibility
+
+- Existing rows remain valid. FTS rebuilds are derived from canonical SQLite rows/projection.
+- Archived rows remain available for restore; sidecar vectors are treated as cache and can be regenerated.

docs/issues/pr1794-memory-hardening/spec.md

@@ -0,0 +1,41 @@
+# PR1794 Memory Hardening
+
+## User need
+
+PR #1794 adds a broad long-term memory system. Before merge, the implementation must avoid silent memory loss, memory contamination, data/index corruption, confusing destructive UI behavior, and obvious reliability regressions.
+
+## Goal
+
+Fix the must-fix and medium-high priority review findings for PR #1794 while preserving the overall tape-native memory design.
+
+## Acceptance criteria
+
+- Memory extraction does not advance session memory cursors when the LLM extraction output cannot be parsed as a valid result.
+- Assistant reasoning/internal thinking fields are excluded from memory extraction spans.
+- Embedding reindex/reset cannot be raced by an older in-flight embedding drain writing stale vectors.
+- Disabling memory consistently prevents restore/write paths from scheduling new embeddings, unless explicitly documented as read-only management.
+- Archive/forget removes stale vectors from the sidecar or otherwise compacts them so archived rows do not bloat vector recall.
+- Memory keyword search supports tokenized multi-word queries, not only exact phrase queries.
+- SQLCipher/encryption copy preserves or rebuilds indexes/triggers required by memory/tape tables.
+- FTS indexes have a version/tokenizer rebuild path so runtime capability changes do not leave stale FTS schemas.
+- Tape search FTS replacement/deletion does not leave stale tokens for replaced entries.
+- Memory settings UI recovers from load errors, avoids stale refresh overwrites, and makes archive vs permanent delete semantics clear.
+- Relevant unit tests cover the fixed behaviors.
+
+## Constraints
+
+- Keep changes focused on PR #1794 memory hardening; avoid unrelated refactors.
+- Preserve backward compatibility with existing local SQLite databases.
+- Do not introduce new runtime dependencies.
+- Main-process DB operations remain synchronous where existing SQLite presenter patterns require it.
+- UI strings must use i18n keys and newly added translations must be localized for each supported locale rather than copied from English.
+
+## Non-goals
+
+- Redesigning the entire memory architecture.
+- Changing the public PR feature scope beyond hardening and safety fixes.
+- Shipping a complete vector compaction scheduler if immediate vector deletion on archive is sufficient.
+
+## Open questions
+
+None.

docs/issues/pr1794-memory-hardening/tasks.md

@@ -0,0 +1,16 @@
+# Tasks
+
+- [x] Core: make extraction parse failures retryable.
+- [x] Core: exclude assistant reasoning from extraction spans.
+- [x] Core: prevent stale embedding drains during reindex/reset.
+- [x] Core: gate restore and clean vector sidecar on archive/forget.
+- [x] Core: improve memory FTS tokenized search.
+- [x] DB: preserve/rebuild indexes and triggers during encrypted copy.
+- [x] DB: version/rebuild agent memory FTS tokenizer.
+- [x] DB: stabilize tape search FTS replacement/deletion.
+- [x] DB: harden clear/reset and new DB schema version initialization.
+- [x] UI: add MemorySettings error handling.
+- [x] UI: guard MemoryManagerPanel refresh and search errors.
+- [x] UI: localize new memory management and advanced settings strings for all supported locales.
+- [x] Tests: update memory extraction tests for `parseMemoryCandidates` union return.
+- [ ] Validation: `pnpm run format`, `pnpm run i18n`, `pnpm run lint`, and `pnpm run typecheck` passed under Node v26 warning; `pnpm test` was stopped per user instruction after memoryPresenter failures surfaced.

src/main/presenter/agentRuntimePresenter/index.ts

@@ -1992,13 +1992,8 @@ export class AgentRuntimePresenter implements IAgentImplementation {
             const b = block as {
               type?: string
               content?: unknown
-              reasoning_content?: unknown
-              text?: unknown
             }
             if (b?.type === 'content' && typeof b.content === 'string') return b.content
-            if (b?.type === 'reasoning_content' && typeof b.content === 'string') return b.content
-            if (typeof b?.reasoning_content === 'string') return b.reasoning_content
-            if (b?.type === 'reasoning' && typeof b.text === 'string') return b.text
             return ''
           })
           .filter(Boolean)

src/main/presenter/databaseSecurityPresenter/index.ts

@@ -63,6 +63,22 @@ type SqliteSchemaRow = {
 const quoteIdentifier = (value: string): string => `"${value.replace(/"/g, '""')}"`
 const getMigrationLockPath = (dbPath: string): string => path.resolve(dbPath)
 
+const FTS_SCHEMA_OBJECT_NAME_PATTERN = /(^|_)fts($|_)/i
+const FTS_TRIGGER_NAME_PATTERN = /_(ai|ad|au)$/i
+
+function isFtsMaintenanceSchemaObject(row: SqliteSchemaRow & { tbl_name?: string }): boolean {
+  const sql = row.sql.toLowerCase()
+  return (
+    FTS_SCHEMA_OBJECT_NAME_PATTERN.test(row.name) ||
+    (row.tbl_name ? FTS_SCHEMA_OBJECT_NAME_PATTERN.test(row.tbl_name) : false) ||
+    /\b[a-z0-9_]+_fts\b/i.test(row.sql) ||
+    /\busing\s+fts[345]?\b/i.test(row.sql) ||
+    (row.type === 'trigger' &&
+      FTS_TRIGGER_NAME_PATTERN.test(row.name) &&
+      sql.includes('insert into'))
+  )
+}
+
 export class DatabaseSecurityPresenter {
   private readonly store: ElectronStore<{ metadata: DatabaseSecurityMetadata }>
   private readonly dbPath: string
@@ -361,6 +377,7 @@ export class DatabaseSecurityPresenter {
         )
       }
 
+      this.copySchemaObjects(db)
       this.copySqliteSequence(db)
       db.exec('COMMIT')
     } catch (error) {
@@ -369,6 +386,54 @@ export class DatabaseSecurityPresenter {
     }
   }
 
+  private copySchemaObjects(db: Database.Database): void {
+    for (const object of this.listMigratableSchemaObjects(db)) {
+      db.exec(this.qualifyCreateSchemaObjectSql(object))
+    }
+  }
+
+  private listMigratableSchemaObjects(db: Database.Database): SqliteSchemaRow[] {
+    const virtualTableNames = new Set(
+      (
+        db
+          .prepare(
+            `SELECT name, sql FROM sqlite_master
+             WHERE type = 'table'
+               AND sql IS NOT NULL
+               AND name NOT LIKE 'sqlite_%'`
+          )
+          .all() as SqliteSchemaRow[]
+      )
+        .filter((row) => /^CREATE\s+VIRTUAL\s+TABLE\s+/i.test(row.sql))
+        .map((row) => row.name)
+    )
+    const rows = db
+      .prepare(
+        `SELECT type, name, tbl_name, sql FROM sqlite_master
+         WHERE type IN ('index', 'trigger', 'view')
+           AND sql IS NOT NULL
+           AND name NOT LIKE 'sqlite_%'
+         ORDER BY CASE type WHEN 'index' THEN 0 WHEN 'trigger' THEN 1 ELSE 2 END, name ASC`
+      )
+      .all() as Array<SqliteSchemaRow & { tbl_name?: string }>
+    return rows.filter((row) => {
+      if (shouldExcludeFromSqliteCopy(row.name)) return false
+      if (row.tbl_name && shouldExcludeFromSqliteCopy(row.tbl_name)) return false
+      if (isFtsMaintenanceSchemaObject(row)) return false
+      for (const virtualTableName of virtualTableNames) {
+        if (
+          row.name === virtualTableName ||
+          row.name.startsWith(`${virtualTableName}_`) ||
+          row.tbl_name === virtualTableName ||
+          row.sql.includes(virtualTableName)
+        ) {
+          return false
+        }
+      }
+      return true
+    })
+  }
+
   private listMigratableTables(db: Database.Database): SqliteSchemaRow[] {
     const rows = db
       .prepare(
@@ -404,6 +469,28 @@ export class DatabaseSecurityPresenter {
     )
   }
 
+  private qualifyCreateSchemaObjectSql(row: SqliteSchemaRow): string {
+    if (row.type === 'index') {
+      return row.sql.replace(
+        /^CREATE\s+(UNIQUE\s+)?INDEX\s+(IF\s+NOT\s+EXISTS\s+)?/i,
+        (_match, unique: string | undefined, ifNotExists: string | undefined) =>
+          `CREATE ${unique ?? ''}INDEX ${ifNotExists ?? ''}${MIGRATION_TARGET_SCHEMA}.`
+      )
+    }
+    if (row.type === 'trigger') {
+      return row.sql.replace(
+        /^CREATE\s+(TEMP\s+|TEMPORARY\s+)?TRIGGER\s+(IF\s+NOT\s+EXISTS\s+)?/i,
+        (_match, temp: string | undefined, ifNotExists: string | undefined) =>
+          `CREATE ${temp ?? ''}TRIGGER ${ifNotExists ?? ''}${MIGRATION_TARGET_SCHEMA}.`
+      )
+    }
+    return row.sql.replace(
+      /^CREATE\s+(TEMP\s+|TEMPORARY\s+)?VIEW\s+(IF\s+NOT\s+EXISTS\s+)?/i,
+      (_match, temp: string | undefined, ifNotExists: string | undefined) =>
+        `CREATE ${temp ?? ''}VIEW ${ifNotExists ?? ''}${MIGRATION_TARGET_SCHEMA}.`
+    )
+  }
+
   private copySqliteSequence(db: Database.Database): void {
     const sourceSequence = db
       .prepare("SELECT 1 FROM main.sqlite_master WHERE type = 'table' AND name = 'sqlite_sequence'")

src/main/presenter/memoryPresenter/extraction.ts

@@ -53,19 +53,27 @@ export function buildExtractionPrompt(spanText: string): string {
   ].join('\n')
 }
 
-// Tolerant parse: code fences, surrounding noise, and missing fields all degrade to [].
-export function parseMemoryCandidates(raw: string): MemoryCandidate[] {
-  if (!raw) return []
+export type MemoryCandidateParseResult =
+  | { ok: true; candidates: MemoryCandidate[] }
+  | {
+      ok: false
+      reason: 'empty-response' | 'missing-json-array' | 'invalid-json' | 'non-array'
+    }
+
+// Tolerant per-entry parse: surrounding noise and malformed entries are ignored, but malformed
+// top-level model output is reported so callers can retry instead of advancing durable cursors.
+export function parseMemoryCandidates(raw: string): MemoryCandidateParseResult {
+  if (typeof raw !== 'string' || !raw.trim()) return { ok: false, reason: 'empty-response' }
   const jsonText = extractJsonArray(raw)
-  if (!jsonText) return []
+  if (!jsonText) return { ok: false, reason: 'missing-json-array' }
 
   let parsed: unknown
   try {
     parsed = JSON.parse(jsonText)
   } catch {
-    return []
+    return { ok: false, reason: 'invalid-json' }
   }
-  if (!Array.isArray(parsed)) return []
+  if (!Array.isArray(parsed)) return { ok: false, reason: 'non-array' }
 
   const candidates: MemoryCandidate[] = []
   for (const entry of parsed) {
@@ -78,7 +86,7 @@ export function parseMemoryCandidates(raw: string): MemoryCandidate[] {
     candidates.push({ kind, content, importance })
     if (candidates.length >= MAX_CANDIDATES) break
   }
-  return candidates
+  return { ok: true, candidates }
 }
 
 function clampImportance(value: unknown): number {

src/main/presenter/memoryPresenter/index.ts

@@ -477,6 +477,15 @@ export class MemoryPresenter implements MemoryRuntimePort {
           this.isPendingEmbeddableRow(agentId, this.deps.repository.getById(record.memoryId))
         )
         if (!live.length) return { written: new Set<string>(), usable: true }
+        const currentEmbedding = this.deps.resolveAgentConfig(agentId)?.memoryEmbedding
+        if (
+          !currentEmbedding?.providerId ||
+          !currentEmbedding?.modelId ||
+          embeddingFingerprint(currentEmbedding.providerId, currentEmbedding.modelId) !==
+            embeddingFingerprint(embedding.providerId, embedding.modelId)
+        ) {
+          return { written: new Set<string>(), usable: true }
+        }
         const store = await this.openVectorStoreLocked(
           agentId,
           { providerId: embedding.providerId, modelId: embedding.modelId },
@@ -492,6 +501,17 @@ export class MemoryPresenter implements MemoryRuntimePort {
 
       if (!this.canContinueAgentMemoryTask(agentId)) return
       const fingerprint = embeddingFingerprint(embedding.providerId, embedding.modelId)
+      const currentEmbedding = this.deps.resolveAgentConfig(agentId)?.memoryEmbedding
+      const currentFingerprint =
+        currentEmbedding?.providerId && currentEmbedding?.modelId
+          ? embeddingFingerprint(currentEmbedding.providerId, currentEmbedding.modelId)
+          : null
+      if (currentFingerprint !== fingerprint) {
+        logger.info(
+          `[Memory] embedding config changed during drain for ${agentId}; discarding stale vectors`
+        )
+        return
+      }
       for (const record of records) {
         if (outcome.written.has(record.memoryId)) {
           this.deps.repository.updatePendingEmbeddingStatus(agentId, record.memoryId, 'embedded', {
@@ -541,6 +561,11 @@ export class MemoryPresenter implements MemoryRuntimePort {
     // `force` rebuilds an unusable on-disk store even with nothing to re-queue (the foreign file is
     // itself what blocks recovery); otherwise skip the reset when there is no stale work.
     if (!requeued && !force) return
+    // Wait for a drain that captured the previous embedding config before resetting the sidecar,
+    // otherwise stale vectors can be written into the freshly reset store.
+    const inFlightDrain = this.embeddingDrains.get(agentId)
+    if (inFlightDrain) await inFlightDrain.catch(() => undefined)
+    if (!this.canContinueAgentMemoryTask(agentId)) return
     // Drop the stale-dimension store under the per-agent lock; the next embed rebuilds it.
     await this.runExclusiveForAgent(agentId, async () => {
       if (!this.canContinueAgentMemoryTask(agentId)) return
@@ -627,7 +652,12 @@ export class MemoryPresenter implements MemoryRuntimePort {
       )
       // Teardown may have begun during the extraction await; stop before any candidate processing.
       if (!this.canWriteAgentMemory(input.agentId)) return { ok: true, createdIds: [] }
-      const candidates = parseMemoryCandidates(response)
+      const parsed = parseMemoryCandidates(response)
+      if (!parsed.ok) {
+        logger.warn(`[Memory] extraction parse failed: ${parsed.reason}`)
+        return { ok: false }
+      }
+      const candidates = parsed.candidates
       const options: WriteMemoriesOptions = {
         agentId: input.agentId,
         sourceSession: input.sourceSession ?? null,
@@ -1136,7 +1166,7 @@ export class MemoryPresenter implements MemoryRuntimePort {
   restoreMemory(agentId: string, memoryId: string): boolean {
     if (this.disposed) return false
     this.assertSafeAgentId(agentId)
-    if (!this.isManagedAgent(agentId)) return false
+    if (!this.canWriteAgentMemory(agentId)) return false
     const row = this.deps.repository.getById(memoryId)
     if (!row || row.agent_id !== agentId || row.status !== 'archived') return false
     this.deps.repository.updateStatus(memoryId, 'pending_embedding')
@@ -1151,11 +1181,13 @@ export class MemoryPresenter implements MemoryRuntimePort {
   async forgetMemory(agentId: string, memoryId: string): Promise<boolean> {
     if (this.disposed) return false
     this.assertSafeAgentId(agentId)
-    if (!this.isManagedAgent(agentId)) return false
+    if (!this.canWriteAgentMemory(agentId)) return false
     const row = this.deps.repository.getById(memoryId)
     if (!row || row.agent_id !== agentId) return false
     if (row.status === 'archived') return true
     this.deps.repository.archive(row.id, Date.now())
+    await this.deleteVectorsForMemoryIds(agentId, [memoryId])
+    if (this.disposed) return true
     this.syncWorkingMemoryAfterMutation(agentId)
     this.emitChanged(agentId, 'extract')
     return true
@@ -2051,18 +2083,7 @@ export class MemoryPresenter implements MemoryRuntimePort {
     if (row.kind !== 'working') {
       this.syncWorkingMemoryAfterMutation(agentId)
     }
-    // Run the vector delete under the per-agent lock so dispose() awaits it (via vectorStoreLocks)
-    // before closing the sidecar — otherwise a teardown landing mid-DELETE could close the DuckDB
-    // connection while the statement runs. If teardown already began, skip it: the authoritative row
-    // is gone and an orphaned vector is harmless (recall skips matches whose SQLite row is missing).
-    await this.runExclusiveForAgent(agentId, async () => {
-      if (this.disposed) return
-      const store = await this.vectorStoreForAgent(agentId)
-      if (!store) return
-      await store.deleteByMemoryIds([memoryId]).catch((error) => {
-        logger.warn(`[Memory] vector delete failed: ${String(error)}`)
-      })
-    })
+    await this.deleteVectorsForMemoryIds(agentId, [memoryId])
     if (this.disposed) return true
     this.emitChanged(agentId, 'delete')
     return true
@@ -2119,6 +2140,21 @@ export class MemoryPresenter implements MemoryRuntimePort {
     if (resetError) throw resetError
   }
 
+  private async deleteVectorsForMemoryIds(agentId: string, memoryIds: string[]): Promise<void> {
+    if (!memoryIds.length) return
+    // Run vector deletes under the per-agent lock so dispose() awaits them (via vectorStoreLocks)
+    // before closing the sidecar. If teardown already began, skip it: SQLite status is authoritative
+    // and recall ignores rows that are archived/deleted.
+    await this.runExclusiveForAgent(agentId, async () => {
+      if (this.disposed) return
+      const store = await this.vectorStoreForAgent(agentId)
+      if (!store) return
+      await store.deleteByMemoryIds(memoryIds).catch((error) => {
+        logger.warn(`[Memory] vector delete failed: ${String(error)}`)
+      })
+    })
+  }
+
   private async settleDeletedAgentInFlight(agentId: string): Promise<void> {
     const reindexing = this.reindexing.get(agentId)
     const backfilling = this.backfilling.get(agentId)