diff --git a/api/models.py b/api/models.py index a5a4be43..ffee1d76 100644 --- a/api/models.py +++ b/api/models.py @@ -5293,12 +5293,22 @@ def merge_session_messages_append_only( # post-edit content and must merge through (not be dropped as a replaced # tail). The conservative skip still applies for boundary is None and # boundary == watermark (not-advanced / legacy). + # + # CRITICAL: the boundary-advanced signal may only bypass the skip AFTER + # state replay has consumed the sidecar's visible checkpoint + # (state_replay_idx >= len(sidecar_visible_sequence)). A deleted suffix + # row with ts > watermark that appears in state.db BEFORE the edited + # checkpoint must still be skipped — otherwise the advanced signal would + # resurrect it. The sidecar-max-timestamp signal needs no such gate (a + # sidecar tail beyond the watermark is itself proof the checkpoint has + # advanced). + checkpoint_consumed = state_replay_idx >= len(sidecar_visible_sequence) sidecar_advanced_past_watermark = ( watermark_timestamp is not None and ( (max_sidecar_timestamp is not None and max_sidecar_timestamp > watermark_timestamp) - or watermark_advanced_by_boundary + or (watermark_advanced_by_boundary and checkpoint_consumed) ) ) if ( @@ -5408,9 +5418,15 @@ def merge_session_messages_append_only( # block would normally skip it ("sidecar already has this message"), # but the sidecar doesn't — it's a genuine state-only recovery row. # Let it through (CORE-B, #4767). + # + # Only AFTER the sidecar's visible checkpoint has been consumed + # (checkpoint_consumed) — a same-second row appearing in state.db + # BEFORE the edited user replay is a deleted/replaced row, not the + # post-edit reply, and must stay skipped. if ( watermark_timestamp is not None and timestamp == watermark_timestamp + and checkpoint_consumed and str(msg.get("role", "")).lower() != "user" and _session_message_content_key(msg) not in seen_content_keys ): diff --git a/tests/test_core_data_loss_cases.py b/tests/test_core_data_loss_cases.py index d8803f63..4bc487d5 100644 --- a/tests/test_core_data_loss_cases.py +++ b/tests/test_core_data_loss_cases.py @@ -193,6 +193,44 @@ def test_advanced_sidecar_at_watermark_keeps_state_only_post_edit_reply(): ) +def test_advanced_does_not_resurrect_stale_post_watermark_row_before_checkpoint(): + """The boundary watermark that appears in state.db BEFORE the edited + checkpoint row. + + Codex final-gate found that making `sidecar_advanced_past_watermark` + globally true (whenever boundary < watermark) let a stale ts>watermark row + bypass the skip before state replay reached the sidecar's edited checkpoint, + resurrecting deleted turns. The bypass is now gated on the checkpoint having + been consumed (state_replay_idx >= len(sidecar_visible_sequence)).""" + # sidecar ends at the edited user @200 (== watermark); state.db lists a + # deleted future suffix (@300/@301) BEFORE the appended edited turn. + sidecar = [ + _msg("user", "first", 50.0), + _msg("assistant", "first reply", 51.0), + _msg("user", "edited prompt", 200.0), + ] + state = [ + _msg("user", "first", 50.0), + _msg("assistant", "first reply", 51.0), + _msg("user", "deleted future prompt", 300.0), + _msg("assistant", "deleted future answer", 301.0), + _msg("user", "edited prompt", 200.0), + ] + + merged = models.merge_session_messages_append_only( + sidecar, state, truncation_watermark=200.0, truncation_boundary=51.0 + ) + + contents = [m["content"] for m in merged] + assert "deleted future prompt" not in contents, ( + f"Stale post-watermark row resurrected before checkpoint! {contents}" + ) + assert "deleted future answer" not in contents, ( + f"Stale post-watermark row resurrected before checkpoint! {contents}" + ) + + # ─── Same-second assistant reply dropped ──────────────────────────────────────