fix(memory): don't crash search on FTS5 query syntax in user/LLM text

[ly-wang19]

What & why

MemoryStore.search_facts() passes the raw search string straight to FTS5:

... JOIN facts_fts fts ... WHERE facts_fts MATCH ?

But FTS5 has its own query grammar, so any search text containing FTS5 metasyntax raises an unhandled sqlite3.OperationalError and crashes the memory tool. The memory search action is exposed to the LLM, which routinely produces such queries. Reproduced (on main):

s = MemoryStore(":memory:")
s.add_fact("Rust is memory safe", category="tech")
s.search_facts("memory:safe")   # OperationalError: no such column: memory
s.search_facts('say "hi')       # OperationalError: unterminated string
s.search_facts("foo AND bar")   # OperationalError: fts5: syntax error near ...
s.search_facts("(python")       # OperationalError: fts5: syntax error

Stray double-quotes, key:value colons, bare AND/OR/NEAR, and parens all blow up.

Fix

Try the raw query first (so valid FTS5 — including prefix term* — is unchanged); on OperationalError, retry with each whitespace token quoted as a literal phrase via _fts5_safe_query(), which neutralizes the metasyntax:

try:
    rows = self._conn.execute(sql, params).fetchall()
except sqlite3.OperationalError:
    safe = _fts5_safe_query(query)        # 'memory:safe' -> '"memory:safe"'
    if not safe:
        return []
    params[0] = safe
    rows = self._conn.execute(sql, params).fetchall()

So memory:safe degrades to the phrase "memory safe" and still finds the Rust fact, instead of crashing.

How to test

scripts/run_tests.sh tests/plugins/memory/test_holographic_store_search_fts5.py   # 14 passed

Covers the crashing-input set (quote/colon/AND/OR/NEAR/paren — all return a list, never raise), the colon-query fallback still matching, normal and prefix (rust*) queries unchanged, quotes-only → [], and the _fts5_safe_query sanitizer.

Platforms

Pure SQLite plugin logic; verified via the CI-parity runner.

[liuhao1024]

Positive verification: FTS5 crash fix

Reviewed this PR in detail — the fix is clean and well-structured:

1. _fts5_safe_query() helper correctly wraps each whitespace-delimited token in double-quotes and escapes embedded quotes by doubling them (" → ""), which is the FTS5 phrase escaping convention.
2. Try/catch fallback catches sqlite3.OperationalError from MATCH on malformed FTS5 syntax, then retries with the safe literal query. This preserves the ability to use valid FTS5 advanced syntax (e.g. rust* prefix queries) on the first attempt while degrading gracefully.
3. Edge case: empty-after-sanitization — if not safe: return [] prevents passing "" to MATCH, which would also raise OperationalError.
4. Test coverage is thorough — parametrized tests cover unterminated quotes, colons, bare AND/OR/NEAR, parentheses, C++ operators, and the helper function itself. Tests verify both crash-prevention AND that normal/valid-advanced queries still work.

The _fts5_safe_query function only appears on the left-hand side of assignments (definition + test usage), which is expected for a pure helper. No dead code.

No issues found. This is a good defensive fix for a crash path exposed to arbitrary LLM/user input.

[liuhao1024]

Verification: FTS5 crash guard — looks clean.

The _fts5_safe_query helper wraps each whitespace token in double-quotes (with embedded-quote doubling), degrading gracefully from FTS5 metasyntax to literal phrase matching. The try/except on sqlite3.OperationalError catches all known FTS5 parser failures (stray quotes, colons, bare AND/OR/NEAR, unmatched parens) without masking unrelated errors.

Test coverage is thorough — parametrized across 9 metasyntax variants plus explicit regression tests for colon queries finding matches through the safe path, prefix queries still working (raw path succeeds first), and empty-after-sanitization returning [].

[austinpickett]

✅ Approved

The bug: search_facts in holographic/store.py passed the raw LLM search query directly to FTS5 MATCH. LLM queries routinely contain FTS5 reserved syntax (bare AND/OR/NEAR, colons, unmatched quotes, parens), raising sqlite3.OperationalError and crashing the memory tool.

The fix: Retry path via _fts5_safe_query wraps each whitespace token in double quotes (escaping embedded double-quotes via doubling). The try/except → safe retry pattern is correct and the function is properly unit-tested. Gracefully returns [] when the sanitized query is empty.

Note: PR #43490 applies the same pattern to retrieval.py's _fts_candidates — different file, no conflict, both are wanted.

Reviewed by Hermes Agent

[ly-wang19]

Small, self-contained fix for an unhandled sqlite3.OperationalError: an LLM-issued memory search whose text contains FTS5 metasyntax (a stray ", a key:value colon, a bare AND/OR/NEAR, or a () was passed straight to MATCH and crashed the memory tool. The fix tries the raw query first (so valid FTS5 like term* keeps working) and, on OperationalError, retries with each token quoted as a literal phrase. Regression tests fail without the change.

It's one file + tests, still merges cleanly on current main, and the automated review above already verified it. CI hasn't been able to run yet (first-time contributor) — whenever a maintainer has a moment, could the workflows be approved? Happy to rebase or address any feedback. Thanks!

[teknium1]

Thanks for the focused regression coverage. The direct-store crash is still present on current main, but the implementation needs a small adaptation to the newer query path.

Problems

• Current main now derives match_query through FactRetriever._sanitize_fts_query before executing MATCH (plugins/memory/holographic/store.py:254-277, commit 638d2e7bfcad1be6e779c0d1af95481a6e92d811). The original raw-query hunk therefore no longer applies directly.
• The remaining defect is narrower but real: the sanitizer returns raw input when no tokens survive (plugins/memory/holographic/retrieval.py:616-618), and search_facts executes it directly. Inputs such as AND, a OR, and quote-only text can therefore still raise from FTS5.

Suggested changes

• Preserve the current sanitizer and adapt the store-side fallback/guard around the direct MATCH call.
• Retain the proposed direct-store regressions, particularly the all-token-discarded inputs above.

Automated hermes-sweeper review.

[teknium1]

Current main now builds match_query through FactRetriever._sanitize_fts_query before this execution path (638d2e7). Please preserve that sanitizer when salvaging this guard: its raw fallback for all-discarded tokens is the remaining crash path.

[ly-wang19]

Rebased on current main and applied @teknium1's test-fixture feedback from #43404 here too: swapped MemoryStore(":memory:") for a tmp_path-backed fixture that closes the store (plus the _clean_shared_registry autouse fixture from test_holographic_store.py), since MemoryStore resolves db_path before connecting and keys its shared-connection registry on it — so ":memory:" was a shared on-disk file, not SQLite's in-memory sentinel.

That surfaced something worth flagging, so I've narrowed this PR's claim: search_facts now pre-sanitizes via FactRetriever._sanitize_fts_query, which already neutralises most metasyntax (colons, stray quotes, parens, NEAR(). Measured on current main, only bare boolean operators still reach FTS5 and raise:

search_facts("AND")   -> sqlite3.OperationalError: fts5: syntax error near "AND"
search_facts("a OR")  -> sqlite3.OperationalError: fts5: syntax error near ""

So this is no longer "FTS5 metasyntax crashes the memory tool" broadly — it's the residual bare-operator case the sanitizer doesn't cover. I dropped the now-obsolete memory:safe assertion (the sanitizer rewrites it to "memorysafe", so returning no rows is correct behaviour, not a bug — my old test only passed because the shared ":memory:" DB carried rows from other tests).

Still worth landing IMO since an LLM-emitted query of just AND/OR is realistic, but happy to close it if you'd rather fold the guard into the sanitizer instead.