From 779f190c1b5ea28b6f6f305e9ba13c8044c7b9b4 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Wed, 2 Sep 2026 23:42:09 +0800 Subject: [PATCH] fix(cli): flag mis-mapped lost_and_found salvage as not verified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lost_and_found salvage lane maps cells positionally onto the destination template's declared column order, but a source upgraded via ALTER TABLE has its columns in the order they were added, which differs from SCHEMA_SQL whenever a column was inserted mid-definition (#101409). Every row still inserts, so integrity/FK/FTS/count checks stay green and the report ends up verified: true — while all 1,875 sessions in the reporter's DB got started_at = 0.0 with counters and URLs shifted into the wrong columns. Add a semantic plausibility gate to the salvage lane: when every salvaged sessions.started_at or messages.timestamp is NULL or before 2001-09, the cells were mapped onto the wrong columns and the recovery is reported with errors and healthy: false, so verified no longer claims a mis-mapped output. A partially damaged column (torn cells on some rows) does not trip the gate — only a systematic violation does. Fixing the positional mapping itself needs a historical-layouts table derived from schema-version history; that design decision is left to maintainers (suggested fix 2 in the issue). --- hermes_cli/session_recovery.py | 75 ++++++++ .../test_session_recovery_lost_and_found.py | 167 ++++++++++++++++++ 2 files changed, 242 insertions(+) diff --git a/hermes_cli/session_recovery.py b/hermes_cli/session_recovery.py index 9a376550ad..20255ffe58 100644 --- a/hermes_cli/session_recovery.py +++ b/hermes_cli/session_recovery.py @@ -1431,6 +1431,66 @@ def _finalize_derived_metadata(destination: sqlite3.Connection) -> dict[str, Any return result +# Floor for a plausible unix-epoch timestamp (2001-09-09). Salvage rows +# uniformly below it were mis-mapped, not merely unlucky (#101409). +_PLAUSIBLE_TIMESTAMP_FLOOR = 1_000_000_000.0 + + +def _lost_and_found_plausibility_errors( + conn: sqlite3.Connection, +) -> list[str]: + """Flag systematic timestamp mis-mapping in a salvaged database. + + Structural checks (integrity, FK, FTS, row counts) pass on mis-mapped + salvage because every row still inserts. Only semantics give it away: + the physical column order of a source upgraded via ALTER TABLE differs + from the destination template's declared order, so positional cell + mapping lands counters/strings where ``started_at``/``timestamp`` + belong — and the NOT NULL substitutes turn gaps into 0.0. When every + row violates the epoch floor, the mapping was wrong. + """ + + errors: list[str] = [] + + (session_total,) = conn.execute( + "SELECT COUNT(*) FROM sessions" + ).fetchone() + if session_total: + (implausible,) = conn.execute( + "SELECT COUNT(*) FROM sessions " + "WHERE started_at IS NULL OR started_at < ?", + (_PLAUSIBLE_TIMESTAMP_FLOOR,), + ).fetchone() + if implausible == session_total: + errors.append( + f"sessions.started_at is implausible in all " + f"{session_total} salvaged row(s) (NULL or before 2001-09): " + "the source's physical column order did not match the " + "destination template, so cells were mapped onto the " + "wrong columns" + ) + + (message_total,) = conn.execute( + "SELECT COUNT(*) FROM messages" + ).fetchone() + if message_total: + (implausible,) = conn.execute( + "SELECT COUNT(*) FROM messages " + "WHERE timestamp IS NULL OR timestamp < ?", + (_PLAUSIBLE_TIMESTAMP_FLOOR,), + ).fetchone() + if implausible == message_total: + errors.append( + f"messages.timestamp is implausible in all " + f"{message_total} salvaged row(s) (NULL or before 2001-09): " + "the source's physical column order did not match the " + "destination template, so cells were mapped onto the " + "wrong columns" + ) + + return errors + + def _recover_via_lost_and_found( *, source: Path, @@ -1530,6 +1590,21 @@ def _recover_via_lost_and_found( ) verification["complete"] = False + # Structural checks cannot see a positional mis-mapping (#101409): + # every row still inserts, so integrity/FK/FTS stay green. A + # systematic timestamp violation is the semantic tell — surface it + # so a mis-mapped salvage is never reported as verified. + plausibility_conn = sqlite3.connect(str(output), isolation_level=None) + try: + plausibility_errors = _lost_and_found_plausibility_errors( + plausibility_conn + ) + finally: + plausibility_conn.close() + if plausibility_errors: + verification["errors"].extend(plausibility_errors) + verification["healthy"] = False + source_unchanged = ( _source_fingerprint(source) == inspection["source_fingerprint"] ) diff --git a/tests/hermes_cli/test_session_recovery_lost_and_found.py b/tests/hermes_cli/test_session_recovery_lost_and_found.py index d93bd69b6f..79bb6810a7 100644 --- a/tests/hermes_cli/test_session_recovery_lost_and_found.py +++ b/tests/hermes_cli/test_session_recovery_lost_and_found.py @@ -569,3 +569,170 @@ def test_fingerprint_error_enumerates_parent_cli_session( assert "CLI session" in message assert "fresh shell" in message assert "snapshot" in message + + +# ── issue #101409: mis-mapped salvage must not be reported verified ───────── + + +def _map_salvage_rows( + tmp_path: Path, + *, + blank_session_started_at: bool, + blank_message_timestamp: bool, +) -> sqlite3.Connection: + """Map synthetic lost_and_found cells into a fresh template DB. + + With either ``blank_*`` flag the cells mimic an upgraded source's + *physical* column order (#101409): whatever lands on the declared + ``started_at``/``timestamp`` position is not an epoch timestamp, so + the NOT NULL substitute turns it into 0.0 on every row. + """ + + schema_ref = tmp_path / "schema-ref.db" + SessionDB(db_path=schema_ref).close() + schema = sqlite3.connect(str(schema_ref)) + try: + sessions_columns = [ + str(row[1]) for row in schema.execute("PRAGMA table_info(sessions)") + ] + messages_columns = [ + str(row[1]) for row in schema.execute("PRAGMA table_info(messages)") + ] + finally: + schema.close() + current_width = len(sessions_columns) + + lf_path = tmp_path / "lost_and_found.db" + lf_conn = sqlite3.connect(str(lf_path), isolation_level=None) + try: + lf_cells = ", ".join(f"c{i}" for i in range(current_width)) + lf_conn.execute( + "CREATE TABLE lost_and_found (rootpgno INTEGER, pgno INTEGER, " + "nfield INTEGER, id INTEGER, " + lf_cells + ")" + ) + + def insert(nfield: int, rowid: int, values: list) -> None: + padded = list(values) + [None] * (current_width - len(values)) + placeholders = ", ".join("?" for _ in range(4 + current_width)) + lf_conn.execute( + "INSERT INTO lost_and_found VALUES (" + placeholders + ")", + [2, 5, nfield, rowid, *padded], + ) + + def session_row(session_id: str) -> list: + # title is UNIQUE (idx_sessions_title_unique) — keep it distinct + # per row so the probe isolates timestamp mis-mapping. + row = { + "id": session_id, + "source": "telegram", + "started_at": None + if blank_session_started_at + else 1_754_000_000.0, + "message_count": 2, + "title": f"mis-mapped probe {session_id}", + } + return [row.get(column) for column in sessions_columns] + + for index in range(3): + insert( + current_width, + index + 1, + session_row(f"20260101_01010{index}_aaa00{index}"), + ) + + for index in range(2): + message = { + "id": None, + "session_id": "20260101_010100_aaa000", + "role": "user", + "content": "payload", + "timestamp": None + if blank_message_timestamp + else 1_754_000_100.0 + index, + } + insert( + 23, + 100 + index, + [message.get(column) for column in messages_columns[:23]], + ) + finally: + lf_conn.close() + + output = tmp_path / "mapped.db" + SessionDB(db_path=output).close() + lf_conn = sqlite3.connect(str(lf_path), isolation_level=None) + dest = sqlite3.connect(str(output), isolation_level=None) + try: + dest.execute("PRAGMA foreign_keys=OFF") + map_lost_and_found_rows(lf_conn, dest) + finally: + lf_conn.close() + dest.close() + return sqlite3.connect(str(output), isolation_level=None) + + +def test_plausibility_gate_flags_positional_mis_mapping( + tmp_path: Path, +) -> None: + """A salvage whose timestamps all landed below the epoch floor was + mapped onto the wrong columns and must be flagged, not verified + (#101409).""" + + conn = _map_salvage_rows( + tmp_path, + blank_session_started_at=True, + blank_message_timestamp=False, + ) + try: + # The mapper happily inserted every row; structural checks pass. + assert conn.execute("SELECT COUNT(*) FROM sessions").fetchone()[0] == 3 + assert conn.execute( + "SELECT COUNT(*) FROM sessions WHERE started_at = 0.0" + ).fetchone()[0] == 3 + + errors = session_recovery._lost_and_found_plausibility_errors(conn) + assert len(errors) == 1 + assert "sessions.started_at" in errors[0] + finally: + conn.close() + + +def test_plausibility_gate_flags_mis_mapped_message_timestamps( + tmp_path: Path, +) -> None: + conn = _map_salvage_rows( + tmp_path, + blank_session_started_at=False, + blank_message_timestamp=True, + ) + try: + errors = session_recovery._lost_and_found_plausibility_errors(conn) + assert len(errors) == 1 + assert "messages.timestamp" in errors[0] + finally: + conn.close() + + +def test_plausibility_gate_passes_correctly_mapped_salvage( + tmp_path: Path, +) -> None: + """Well-mapped rows — and partially damaged ones (a torn cell on some + rows is expected salvage noise) — must not trip the gate: it fires + only on a *systematic* violation.""" + + conn = _map_salvage_rows( + tmp_path, + blank_session_started_at=False, + blank_message_timestamp=False, + ) + try: + # Damage one of three sessions the way a torn cell would. + conn.execute( + "UPDATE sessions SET started_at = 0.0 WHERE id = ?", + ("20260101_010101_aaa001",), + ) + conn.commit() + + assert session_recovery._lost_and_found_plausibility_errors(conn) == [] + finally: + conn.close()