diff --git a/scripts/ensure_state_db_read_indexes.py b/scripts/ensure_state_db_read_indexes.py index 9d8fb197..9562338a 100644 --- a/scripts/ensure_state_db_read_indexes.py +++ b/scripts/ensure_state_db_read_indexes.py @@ -52,6 +52,14 @@ INDEXES = { ("source", "id", "message_count", "last_activity_at"))), } +# The only schema gap an older Agent is known to have: sessions.last_activity_at +# arrived later than the other keyed columns. An index whose missing columns are +# all listed here is reported "skipped"; any other missing column means this is +# not an Agent state.db (or a damaged one), and the tool fails closed. +_LEGACY_OPTIONAL_COLUMNS = { + "idx_sessions_webui_fingerprint": frozenset({"last_activity_at"}), +} + def _exclusive_lock(lock_file): if lock_file is None: @@ -91,20 +99,30 @@ def ensure_read_indexes(db_path, *, confirmed_drained=False, lock_file=None): with closing(sqlite3.connect(state_db_file_uri(db_path) + "?mode=rw", uri=True)) as db: db.execute("BEGIN IMMEDIATE") try: + # Validate the whole Agent schema before creating anything, so a + # wrong database fails closed without a partial index set. + missing_by_index = {} for name, (table, keys) in INDEXES.items(): - # An older Agent schema can lack a column one index keys on - # (sessions.last_activity_at is newer than - # messages.timestamp). Skip that index instead of letting its - # CREATE fail and roll back the ones this schema supports. present = {r[1] for r in db.execute(f"PRAGMA table_info({table})")} if not present: - # No such table: this is not an agent state.db (or a - # mistyped path to another database). Fail loud. raise RuntimeError(f"state.db has no {table!r} table; refusing to continue") - if any(col not in present for col, _ in keys): + missing = {col for col, _ in keys if col not in present} + if missing - _LEGACY_OPTIONAL_COLUMNS.get(name, frozenset()): + raise RuntimeError( + f"state.db {table!r} table lacks Agent column(s) " + f"{sorted(missing)}; not an Agent state.db, refusing to continue" + ) + missing_by_index[name] = missing + for name, (table, keys) in INDEXES.items(): + existing = db.execute("SELECT tbl_name FROM sqlite_master WHERE name=?", (name,)).fetchone() + if missing_by_index[name]: + # Known legacy gap. A same-named index here cannot be the + # shape we expect (it keys on the absent column), so it is + # incompatible, not something to skip past. + if existing: + raise RuntimeError(f"Incompatible index: {name}") statuses[name] = "skipped" continue - existing = db.execute("SELECT tbl_name FROM sqlite_master WHERE name=?", (name,)).fetchone() if existing: actual = tuple((r[2], r[4]) for r in db.execute(f"PRAGMA index_xinfo({name})") if r[5]) flags = next((r for r in db.execute(f"PRAGMA index_list({table})") if r[1] == name), None) diff --git a/tests/test_pr7445_readonly_review_followups.py b/tests/test_pr7445_readonly_review_followups.py index 45cdbbcc..5cb74a30 100644 --- a/tests/test_pr7445_readonly_review_followups.py +++ b/tests/test_pr7445_readonly_review_followups.py @@ -285,6 +285,40 @@ def test_maintenance_fails_loud_on_a_database_without_agent_tables(tmp_path, mai assert conn.execute("SELECT count(*) FROM sqlite_master WHERE type='index'").fetchone()[0] == 0 +def test_maintenance_fails_closed_on_same_named_tables_without_agent_columns(tmp_path, maintenance_without_fcntl): + """Tables named ``messages``/``sessions`` with foreign columns are not an Agent + state.db: raise, create nothing, never report every index ``skipped``.""" + module = maintenance_without_fcntl + path = tmp_path / "state.db" + with closing(sqlite3.connect(str(path))) as conn: + conn.executescript("CREATE TABLE messages(body TEXT); CREATE TABLE sessions(title TEXT);") + with pytest.raises(RuntimeError, match="not an Agent state.db"): + module.ensure_read_indexes(path, confirmed_drained=True) + with closing(sqlite3.connect(str(path))) as conn: + assert conn.execute("SELECT count(*) FROM sqlite_master WHERE type='index'").fetchone()[0] == 0 + + +def test_maintenance_rejects_a_same_named_index_on_the_legacy_gap(tmp_path, maintenance_without_fcntl): + """On the legacy schema a pre-existing ``idx_sessions_webui_fingerprint`` can't + be the expected shape (it would key on the absent column): report it as + incompatible instead of skipping past it, and roll back the message indexes.""" + module = maintenance_without_fcntl + path = tmp_path / "state.db" + with closing(sqlite3.connect(str(path))) as conn: + conn.executescript( + """ + CREATE TABLE sessions(source TEXT, id TEXT, message_count INTEGER); + CREATE TABLE messages(session_id TEXT, timestamp REAL, role TEXT); + CREATE INDEX idx_sessions_webui_fingerprint ON messages(role); + """ + ) + with pytest.raises(RuntimeError, match="Incompatible index: idx_sessions_webui_fingerprint"): + module.ensure_read_indexes(path, confirmed_drained=True) + with closing(sqlite3.connect(str(path))) as conn: + names = {r[0] for r in conn.execute("SELECT name FROM sqlite_master WHERE type='index'")} + assert names == {"idx_sessions_webui_fingerprint"} + + @pytest.fixture def maintenance_without_fcntl(monkeypatch): """Import the maintenance module as a platform without ``fcntl`` sees it."""