Repository navigation
[Bug] skills upsert (INSERT OR REPLACE) skips FTS cleanup trigger under recursive_triggers=0, orphaning skills_fts rows #2363
Description
Pre-submission checklist
- I have searched existing issues and this hasn't been mentioned before
- I have read the project documentation and confirmed this issue doesn't already exist
- This issue is specific to MemOS and not a general software issue
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_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:
- Don't rely on implicit-trigger-driven
REPLACEfor tables with FTS sync — do an explicitDELETE+INSERTin the upsert path (guaranteed to fire triggers regardless ofrecursive_triggers), or - Have MemOS set
PRAGMA 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.
Activity
Memtensor-AI commentedon Sep 14, 2026
🤖 AutoDev has picked up this issue and started working on it.
Task ID:
261a199963ed3881Working branch:
bugfix/autodev-2363-20260914203915788Target branch:
latest dev* branchWorkflow: opsp (analysis → coding → testing → PR)
I will post the PR link here once done. If I need more information, I will ask in the comments.
fix(storage): set PRAGMA recursive_triggers=ON so upsert cleans FTS
Memtensor-AI commentedon Sep 14, 2026
✅ AutoDev task
261a199963ed3881completed.Summary: Fixes #2363. Root cause is exactly as reported:
apps/memos-local-plugin/core/storage/repos/skills.tsupserts viaINSERT OR REPLACE, and per SQLite's documented REPLACE semantics the internal DELETE only firesAFTER DELETEtriggers whenPRAGMA recursive_triggersis ON. SQLite's default is OFF, and several hosts (SQLCipher and some ORM stacks) leave it OFF, so theskills_fts_adcleanup trigger is skipped on every upsert-on-conflict whileskills_fts_aistill runs — leavingskills_ftswith duplicate/orphan rows relative toskillsand pollutingsearchByTextrankings. The same footgun applies uniformly to everyonConflict: "replace"upsert paired with an FTS mirror (traces/policies/world_model).Chose option 2 from the issue: set
PRAGMA recursive_triggers = ONat connection open inapps/memos-local-plugin/core/storage/connection.ts(both read-write and read-only branches, for symmetry). One line, zero API surface change, no migration, covers all four FTS-backed tables at once, and future-proofs any additionalAFTER DELETEtrigger we may add. Rejected option 1 (rewriting each upsert as explicit DELETE+INSERT) because it duplicates the fix across four repos, changes the atomicity contract callers rely on, and doesn't help the next FTS-mirrored table someone adds. Foreign-key cascades are unaffected — those are FK actions, not triggers, and always run.Regression coverage lives in
apps/memos-local-plugin/tests/unit/storage/skills-fts-upsert.test.ts(3 cases: base row-count invariantcount(skills_fts) == count(skills)after upsert-on-conflict, stale-token search behavior through the fullsearchByTextpath, and a directPRAGMA recursive_triggersassertion so a future contributor silently dropping the pragma is caught immediately). All three cases fail on the pre-fix code and pass on the fix.Verified: plugin unit suite 180/180 files, 1553/1553 tests pass (1 skipped, pre-existing); integration suite 3/3 files, 5/5 tests pass;
npm run lint(tsc --noEmit) is clean. opsp artifacts (proposal / spec / design / verification-report / task) are inopenspec/changes/2026-09-14-2363-.../and synced+pushed tospecs/2026-09-14-2363-.../. Working branch pushed to origin at commit20c13665.Base branch:
mainBranch:
bugfix/autodev-2363-20260914203915788Commit:
20c1366PR: #2368
Assigned to: @wustzdy
Reviewers: @wustzdy
fix(storage): drop no-op recursive_triggers pragma from read-only branch
github-actions commentedon Oct 18, 2026
This issue has been automatically marked as stale due to inactivity.
github-actions commentedon Oct 27, 2026
This issue has been automatically closed due to inactivity.