fix(state): honor boolean operators in trigram-off LIKE fallback + fail-closed drops

hermes_state.py

@@ -1169,27 +1169,74 @@ def _drop_trigram_schema(cursor: sqlite3.Cursor) -> bool:
         return them to the OS), ``False`` when there was nothing to drop — so
         callers can vacuum only when there is real space to reclaim rather
         than on every open with trigram disabled.
+
+        The trigger + table drops fail closed. A concurrent writer can hold
+        the DB lock during startup (gateway + cron), and swallowing that lock
+        on a trigger or table DROP could leave a stale trigram trigger pointing
+        at a missing table (or report a drop that did not happen). Acquire a
+        write transaction up front so the sequence commits as a unit; suppress
+        only known FTS-unavailable errors and propagate lock/unrelated failures.
         """
-        table_existed = bool(
-            cursor.execute(
-                "SELECT 1 FROM sqlite_master "
-                "WHERE type = 'table' AND name = 'messages_fts_trigram'"
-            ).fetchone()
-        )
-        for trigger in (
-            "messages_fts_trigram_insert",
-            "messages_fts_trigram_delete",
-            "messages_fts_trigram_update",
-        ):
-            try:
-                cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}")
-            except sqlite3.OperationalError:
-                pass
+        started_transaction = False
         try:
+            cursor.execute("BEGIN IMMEDIATE")
+            started_transaction = True
+            for trigger in (
+                "messages_fts_trigram_insert",
+                "messages_fts_trigram_delete",
+                "messages_fts_trigram_update",
+            ):
+                cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}")
+            table_existed = bool(
+                cursor.execute(
+                    "SELECT 1 FROM sqlite_master "
+                    "WHERE type = 'table' AND name = 'messages_fts_trigram'"
+                ).fetchone()
+            )
             cursor.execute("DROP TABLE IF EXISTS messages_fts_trigram")
-        except sqlite3.OperationalError:
-            return False
-        return table_existed
+            cursor.execute("COMMIT")
+            started_transaction = False
+            return table_existed
+        except sqlite3.OperationalError as exc:
+            if started_transaction:
+                try:
+                    cursor.execute("ROLLBACK")
+                except sqlite3.OperationalError:
+                    pass
+            if SessionDB._is_fts5_unavailable_error(exc):
+                # The DROP TABLE needs to load the vtable module/tokenizer, so a
+                # build missing the trigram tokenizer raises here and the
+                # rollback above also undid the (tokenizer-free) trigger drops.
+                # Leaving the triggers in place means every message INSERT fires
+                # messages_fts_trigram_insert against the unusable vtable and
+                # crashes. DROP TRIGGER does not need the tokenizer, so drop them
+                # in a standalone committed step and degrade to the base
+                # FTS/LIKE path. The orphaned trigram table is harmless without
+                # triggers.
+                try:
+                    cursor.execute("BEGIN IMMEDIATE")
+                    for trigger in (
+                        "messages_fts_trigram_insert",
+                        "messages_fts_trigram_delete",
+                        "messages_fts_trigram_update",
+                    ):
+                        cursor.execute(f"DROP TRIGGER IF EXISTS {trigger}")
+                    cursor.execute("COMMIT")
+                except sqlite3.OperationalError as cleanup_exc:
+                    # Roll back the partial trigger drop, then FAIL CLOSED: if the
+                    # standalone cleanup was itself blocked (e.g. a concurrent
+                    # gateway/cron holds the write lock), swallowing it would
+                    # leave a stale messages_fts_trigram_* trigger firing against
+                    # the unusable vtable on the next INSERT. Propagate so the
+                    # caller aborts and a later uncontended startup retries the
+                    # whole drop, exactly like the main lock path below.
+                    try:
+                        cursor.execute("ROLLBACK")
+                    except sqlite3.OperationalError:
+                        pass
+                    raise cleanup_exc
+                return False
+            raise
 
     @staticmethod
     def _fts_trigger_count(
@@ -5145,22 +5192,66 @@ def search_messages(
             if not _trigram_succeeded:
                 # Short / mixed CJK query, trigram unavailable, or trigram
                 # <3 CJK chars. Fall back to LIKE substring search.
-                # For multi-token OR queries (e.g. "广西 OR 桂林 OR 漓江"),
-                # build one LIKE condition per non-operator token so each term
-                # is matched independently (#20494).
-                non_op_tokens = [
-                    t for t in raw_query.split()
-                    if t.upper() not in {"AND", "OR", "NOT"}
-                ] or [raw_query]
-                token_clauses = []
+                # Preserve the query's boolean structure (#20494): walk the
+                # tokens left-to-right, honoring AND / OR / NOT operators so
+                # `A NOT B` excludes B and `A AND B` requires both. The old
+                # flat OR-everything shape silently over-matched once this
+                # fallback became the only path for disabled-trigram installs.
                 like_params: list = []
-                for tok in non_op_tokens:
+
+                def _term_clause(tok: str) -> str:
                     esc = tok.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
-                    token_clauses.append(
-                        "(m.content LIKE ? ESCAPE '\\' OR m.tool_name LIKE ? ESCAPE '\\' OR m.tool_calls LIKE ? ESCAPE '\\')"
+                    like_params.extend([f"%{esc}%", f"%{esc}%", f"%{esc}%"])
+                    # COALESCE to '' so a NULL column yields FALSE (not NULL)
+                    # under LIKE — critical for the `AND NOT (...)` branch,
+                    # where `NOT (FALSE OR NULL)` would otherwise be NULL and
+                    # silently drop every row (tool_name/tool_calls are NULL on
+                    # plain user/assistant messages).
+                    return (
+                        "(COALESCE(m.content, '') LIKE ? ESCAPE '\\' "
+                        "OR COALESCE(m.tool_name, '') LIKE ? ESCAPE '\\' "
+                        "OR COALESCE(m.tool_calls, '') LIKE ? ESCAPE '\\')"
                     )
-                    like_params += [f"%{esc}%", f"%{esc}%", f"%{esc}%"]
-                like_where = [f"({' OR '.join(token_clauses)})"]
+
+                tokens = raw_query.split()
+                non_op_tokens = [
+                    t for t in tokens if t.upper() not in {"AND", "OR", "NOT"}
+                ] or [raw_query]
+                # Build the boolean expression. Adjacent terms without an
+                # explicit operator default to AND (FTS5 semantics). NOT
+                # attaches to the following term as `AND NOT (...)`; a preceding
+                # OR/AND is kept as the connector, so `A OR NOT B` stays
+                # `A OR NOT B` rather than collapsing to `A AND NOT B`.
+                expr_parts: list = []
+                pending_conn = None  # None | "AND" | "OR"  (connector)
+                pending_negate = False  # a NOT applies to the next term
+                for tok in tokens:
+                    upper = tok.upper()
+                    if upper in {"AND", "OR"}:
+                        pending_conn = upper
+                        continue
+                    if upper == "NOT":
+                        pending_negate = True
+                        continue
+                    clause = _term_clause(tok)
+                    if not expr_parts:
+                        # First term: a leading NOT still excludes.
+                        expr_parts.append(f"NOT {clause}" if pending_negate else clause)
+                    else:
+                        # Default connector between adjacent terms is AND
+                        # (FTS5 semantics); an explicit OR/AND overrides it.
+                        conn = pending_conn or "AND"
+                        if pending_negate:
+                            expr_parts.append(f"{conn} NOT {clause}")
+                        else:
+                            expr_parts.append(f"{conn} {clause}")
+                    pending_conn = None
+                    pending_negate = False
+                if not expr_parts:
+                    # No word tokens survived (pure operators or empty) — match
+                    # the raw query as a single substring.
+                    expr_parts.append(_term_clause(raw_query))
+                like_where = [f"({' '.join(expr_parts)})"]
                 if source_filter is not None:
                     like_where.append(f"s.source IN ({','.join('?' for _ in source_filter)})")
                     like_params.extend(source_filter)

tests/test_hermes_state.py

@@ -869,6 +869,182 @@ def test_trigram_gate_off_drops_table_and_falls_back_to_like(
         finally:
             db.close()
 
+    def test_trigram_off_like_fallback_honors_boolean_operators(
+        self, tmp_path, monkeypatch
+    ):
+        """With trigram gated off, the LIKE fallback must honor AND/NOT.
+
+        Regression: the old fallback OR-joined every non-operator token, so
+        `A NOT B` returned rows containing B and `A AND B` returned rows
+        matching only A. Once trigram is disabled this fallback is the only
+        CJK/substring path, so the boolean structure has to be preserved.
+        """
+        monkeypatch.setattr(
+            SessionDB, "_read_fts_trigram_config", lambda self: False
+        )
+        db_path = tmp_path / "state.db"
+        db = SessionDB(db_path=db_path)
+        try:
+            db.create_session(session_id="a", source="cli")
+            db.append_message("a", role="user", content="大别山项目 概述")
+            db.create_session(session_id="ab", source="cli")
+            db.append_message("ab", role="user", content="大别山项目 桂林项目 联合")
+            db.create_session(session_id="ac", source="cli")
+            db.append_message("ac", role="user", content="大别山项目 武汉分部")
+
+            # NOT excludes the second term.
+            not_results = db.search_messages("大别山项目 NOT 桂林项目", limit=10)
+            assert {r["session_id"] for r in not_results} == {"a", "ac"}
+
+            # AND requires both terms.
+            and_results = db.search_messages("大别山项目 AND 桂林项目", limit=10)
+            assert {r["session_id"] for r in and_results} == {"ab"}
+
+            # OR still unions.
+            or_results = db.search_messages("武汉分部 OR 桂林项目", limit=10)
+            assert {r["session_id"] for r in or_results} == {"ab", "ac"}
+
+            # OR NOT keeps the OR connector (must not collapse to AND NOT):
+            # "桂林项目 OR NOT 武汉分部" = rows with 桂林项目, OR rows without
+            # 武汉分部. Session "a" has neither 桂林项目 nor 武汉分部, so the
+            # NOT arm includes it; "ab" matches the OR arm; "ac" has 武汉分部
+            # and not 桂林项目, so it is excluded.
+            or_not_results = db.search_messages("桂林项目 OR NOT 武汉分部", limit=10)
+            assert {r["session_id"] for r in or_not_results} == {"a", "ab"}
+        finally:
+            db.close()
+
+    def test_drop_trigram_schema_drops_triggers_when_tokenizer_missing(
+        self, tmp_path, monkeypatch
+    ):
+        """When the SQLite build lacks the trigram tokenizer, the DROP TABLE
+        raises 'no such tokenizer: trigram' and the transactional rollback
+        undoes the (tokenizer-free) trigger drops. Leaving the triggers behind
+        makes every later message INSERT fire messages_fts_trigram_insert
+        against the unusable vtable and crash. The drop must still remove the
+        triggers so writes degrade to the base FTS/LIKE path.
+        """
+        monkeypatch.setattr(SessionDB, "_read_fts_trigram_config", lambda self: True)
+        db_path = tmp_path / "state.db"
+        db = SessionDB(db_path=db_path)
+        try:
+            db.create_session(session_id="s1", source="cli")
+            db.append_message("s1", role="user", content="the quick brown fox")
+            assert db._conn.execute(
+                "SELECT count(*) FROM sqlite_master WHERE name='messages_fts_trigram'"
+            ).fetchone()[0] == 1
+
+            real_conn = db._conn
+
+            class _NoTokenizerCursor:
+                def execute(self, sql, *args, **kwargs):
+                    normalized = sql.upper()
+                    if "DROP TABLE" in normalized and "MESSAGES_FTS_TRIGRAM" in normalized:
+                        raise sqlite3.OperationalError("no such tokenizer: trigram")
+                    return real_conn.execute(sql, *args, **kwargs)
+
+            # Tokenizer-missing is a known FTS-unavailable error, so the drop
+            # reports False (nothing reclaimable) but must NOT leave stale
+            # triggers pointing at the unusable vtable.
+            assert db._drop_trigram_schema(_NoTokenizerCursor()) is False
+
+            triggers = {
+                row[0]
+                for row in db._conn.execute(
+                    "SELECT name FROM sqlite_master WHERE type='trigger' "
+                    "AND name LIKE 'messages_fts_trigram_%'"
+                ).fetchall()
+            }
+            assert triggers == set()
+
+            # With the triggers gone, a new message INSERT no longer crashes on
+            # the unusable trigram vtable.
+            db.append_message("s1", role="user", content="second message after drop")
+        finally:
+            db.close()
+
+    def test_drop_trigram_schema_fails_closed_if_tokenizer_fallback_locked(
+        self, tmp_path, monkeypatch
+    ):
+        """Tokenizer-missing recovery must itself fail closed on a lock.
+
+        When DROP TABLE raises 'no such tokenizer: trigram', the fallback
+        re-drops the triggers in a standalone transaction. If THAT is blocked
+        (a concurrent gateway/cron holds the write lock), swallowing it would
+        leave a stale trigger firing against the unusable vtable on the next
+        INSERT. The lock must propagate so a later uncontended startup retries.
+        """
+        monkeypatch.setattr(SessionDB, "_read_fts_trigram_config", lambda self: True)
+        db_path = tmp_path / "state.db"
+        db = SessionDB(db_path=db_path)
+        try:
+            db.create_session(session_id="s1", source="cli")
+            db.append_message("s1", role="user", content="the quick brown fox")
+            real_conn = db._conn
+
+            class _TokenizerThenLockCursor:
+                def __init__(self):
+                    self._table_drop_seen = False
+
+                def execute(self, sql, *args, **kwargs):
+                    normalized = sql.upper()
+                    if "DROP TABLE" in normalized and "MESSAGES_FTS_TRIGRAM" in normalized:
+                        self._table_drop_seen = True
+                        raise sqlite3.OperationalError("no such tokenizer: trigram")
+                    # After the tokenizer failure, the standalone trigger-drop
+                    # BEGIN IMMEDIATE is blocked by a concurrent writer.
+                    if self._table_drop_seen and "BEGIN IMMEDIATE" in normalized:
+                        raise sqlite3.OperationalError("database is locked")
+                    return real_conn.execute(sql, *args, **kwargs)
+
+            with pytest.raises(sqlite3.OperationalError, match="database is locked"):
+                db._drop_trigram_schema(_TokenizerThenLockCursor())
+        finally:
+            db.close()
+
+    def test_drop_trigram_schema_propagates_locked_drop(
+        self, tmp_path, monkeypatch
+    ):
+        """A locked trigram DROP must fail closed, not be swallowed.
+
+        The drop is transactional, so a lock on the table DROP rolls the
+        trigger drops back too — table + triggers stay together for the next
+        uncontended startup instead of leaving a stale trigger pointing at a
+        missing table.
+        """
+        monkeypatch.setattr(SessionDB, "_read_fts_trigram_config", lambda self: True)
+        db_path = tmp_path / "state.db"
+        db = SessionDB(db_path=db_path)
+        try:
+            db.create_session(session_id="s1", source="cli")
+            db.append_message("s1", role="user", content="the quick brown fox")
+            real_conn = db._conn
+
+            class _BlockTableDropCursor:
+                def execute(self, sql, *args, **kwargs):
+                    normalized = sql.upper()
+                    if "DROP TABLE" in normalized and "MESSAGES_FTS_TRIGRAM" in normalized:
+                        raise sqlite3.OperationalError("database is locked")
+                    return real_conn.execute(sql, *args, **kwargs)
+
+            with pytest.raises(sqlite3.OperationalError, match="database is locked"):
+                db._drop_trigram_schema(_BlockTableDropCursor())
+
+            # Failed closed: table and trigram triggers stay together.
+            assert db._conn.execute(
+                "SELECT count(*) FROM sqlite_master WHERE name='messages_fts_trigram'"
+            ).fetchone()[0] == 1
+            triggers = {
+                row[0]
+                for row in db._conn.execute(
+                    "SELECT name FROM sqlite_master WHERE type='trigger' "
+                    "AND name LIKE 'messages_fts_trigram_%'"
+                ).fetchall()
+            }
+            assert triggers == set(hermes_state._TRIGRAM_FTS_TRIGGERS)
+        finally:
+            db.close()
+
     def test_trigram_gate_on_keeps_table(self, tmp_path, monkeypatch):
         monkeypatch.setattr(
             SessionDB, "_read_fts_trigram_config", lambda self: True