Pre-submission checklist
Bug Description
Summary
core/storage/repos/skills.ts upserts skills via SQLite INSERT OR REPLACE (onConflict: "replace"). When PRAGMA recursive_triggers is 0 (a common non-default SQLite setting some hosts run), the implicit delete performed internally by OR REPLACE does not fire delete triggers — including the skills_fts cleanup trigger defined in the FTS migration. Every upsert-on-conflict therefore leaves the old skills_fts row behind (orphaned) while inserting a new one, so skills_fts accumulates duplicate/orphaned rows over time relative to the base skills table.
Root cause
// core/storage/repos/skills.ts
const upsert = db.prepare(
buildInsert({ table: "skills", columns: COLUMNS, onConflict: "replace" }),
);SQLite's own documentation is explicit about this: "The REPLACE conflict resolution algorithm deletes pre-existing rows that are causing the constraint violation... this algorithm does not invoke the row-delete ON DELETE trigger unless PRAGMA recursive_triggers is set to true." (This is unrelated to ON DELETE foreign-key triggers — it also applies to plain AFTER DELETE triggers used for FTS sync, which is the case here.)
This is host-config-dependent, not universal — hosts running with recursive_triggers at its default (or explicitly ON) won't see it. But recursive_triggers=0 is common enough (SQLCipher and several ORMs set it) that this is a real footgun for anyone deploying with that setting.
Impact
skills_fts grows stale/duplicate rows over the lifetime of an install with recursive_triggers=0 — never self-heals.searchByText-style retrieval ranks and LIMITs over the polluted FTS table before joining back to the base table, so this degrades retrieval quality (wrong ranking, phantom/stale hits), not just table bloat.
Suggested Fix
Either:
- Don't rely on implicit-trigger-driven
REPLACE for tables with FTS sync — do an explicit DELETE + INSERT in the upsert path (guaranteed to fire triggers regardless of recursive_triggers), or
- Have MemOS set
PRAGMA recursive_triggers = ON itself at connection open, so FTS-trigger correctness doesn't depend on the host's SQLite defaults.
Happy to open a PR for whichever direction the maintainers prefer.
Pre-submission checklist
Bug Description
Summary
core/storage/repos/skills.tsupserts skills via SQLiteINSERT OR REPLACE(onConflict: "replace"). WhenPRAGMA recursive_triggersis0(a common non-default SQLite setting some hosts run), the implicit delete performed internally byOR REPLACEdoes not fire delete triggers — including theskills_ftscleanup trigger defined in the FTS migration. Every upsert-on-conflict therefore leaves the oldskills_ftsrow behind (orphaned) while inserting a new one, soskills_ftsaccumulates duplicate/orphaned rows over time relative to the baseskillstable.Root cause
SQLite's own documentation is explicit about this: "The REPLACE conflict resolution algorithm deletes pre-existing rows that are causing the constraint violation... this algorithm does not invoke the row-delete
ON DELETEtrigger unlessPRAGMA recursive_triggersis set to true." (This is unrelated toON DELETEforeign-key triggers — it also applies to plainAFTER DELETEtriggers used for FTS sync, which is the case here.)This is host-config-dependent, not universal — hosts running with
recursive_triggersat its default (or explicitlyON) won't see it. Butrecursive_triggers=0is common enough (SQLCipher and several ORMs set it) that this is a real footgun for anyone deploying with that setting.Impact
skills_ftsgrows stale/duplicate rows over the lifetime of an install withrecursive_triggers=0— never self-heals.searchByText-style retrieval ranks and LIMITs over the polluted FTS table before joining back to the base table, so this degrades retrieval quality (wrong ranking, phantom/stale hits), not just table bloat.Suggested Fix
Either:
REPLACEfor tables with FTS sync — do an explicitDELETE+INSERTin the upsert path (guaranteed to fire triggers regardless ofrecursive_triggers), orPRAGMA recursive_triggers = ONitself at connection open, so FTS-trigger correctness doesn't depend on the host's SQLite defaults.Happy to open a PR for whichever direction the maintainers prefer.