fix(#5532): prevent clear from replaying state.db history - #5556
1 commit merged into
Conversation
4372ad2 to
bad2c80
Compare
|
| Filename | Overview |
|---|---|
| api/routes.py | Clear route extended to zero context_messages and all pending fields, bypassing the #1558 guard; backup removal logic is correctly gated on had_sidecar_messages AND verified persisted state. |
| tests/test_issue5532_session_clear_state_db_replay.py | New regression suite covering the happy path, negative-space (watermark=None allows replay), already-empty-sidecar preservation, and post-clear backup recovery. |
| docs/rfcs/session-sse-contract-v1.md | Line-number anchors updated to reflect the 30-line insertion in routes.py; no functional changes. |
Reviews (2): Last reviewed commit: "fix(#5532): prevent clear from replaying..." | Re-trigger Greptile
bad2c80 to
ea2dc31
Compare
🔬 Gate certification — RED ⛔ (direct-history clear is thorough, but a compressed continuation still replays PARENT history via lineage — 1 SILENT)Certified head: What I ran (rebased worktree
|
| Gate | Result |
|---|---|
| Rebase onto current master | ✅ git apply clean |
| Codex (reproduce) | SHIP-WITH-FIXES — 1 SILENT (parent-lineage replay); Codex verified with an isolated temp-session runtime check; I confirmed the code path |
| Full pytest suite | ✅ 11989 passed, 0 failed (the RFC's line-pin re-bump kept test_issue4812 green — confirming those pins are the #5513 collateral, root-cause in #5542) |
| PR's own test | ✅ 4/4 (test_issue5532_session_clear_state_db_replay.py) |
Findings
⛔ SILENT (I + Codex CONFIRMED) — clearing a compressed continuation replays the PARENT's history (api/routes.py /api/session/clear ~13718): the new persisted_clear verifies the sidecar's own fields are empty (messages, context_messages, truncation watermarks, active_stream_id, pending_*) — thorough — BUT it does NOT clear or verify parent_session_id. GET /api/session builds display via _webui_sidecar_lineage_messages_for_display() / _merged_webui_lineage_messages_for_display() (routes.py:8021/8106/11693), which reintroduce messages from parent_session_id. Codex verified empirically: an empty child with truncation_watermark=0.0 and a pre-compression parent still returned the parent prompt on reload. So a user who clears a compressed continuation still sees the old history come back — the exact #5532 symptom, one path over. Fix (Codex-exact): in /api/session/clear set s.parent_session_id = None before s.save(), add persisted.get("parent_session_id") is None to the persisted_clear predicate, and add a regression test for clearing a child whose parent sidecar is pre_compression_snapshot=True.
✅ Direct-history clear is sound (keep): the fully-empty persisted_clear verification + fails-closed backup removal (had_sidecar_messages and persisted_clear) is the right shape; 4/4 tests. ℹ️ RFC-doc change: just re-bumps the brittle api/routes.py:NNNN line-pins (16177→16207, etc.) to match this PR's routes.py shifts — a stopgap that kept test_issue4812 green (root-cause de-brittle tracked in #5542); no contract-semantics change.
Recommendation to the next agent / author
RED — gate-fail/changes-requested (1 SILENT): extend the clear to the lineage path — s.parent_session_id = None before save + add persisted.get("parent_session_id") is None to persisted_clear + a compressed-continuation regression test. The direct-history fix is thorough and the suite is green; this one gap (parent-lineage replay) is the same #5532 bug via a different path — a "clear" must sever the parent lineage too, or a compressed continuation replays. concept 4/5 (real data-integrity fix; one lineage path missed). Author @rodboev (T1). crit=3, data-integrity. (Gate value: same class as #5504 — a data-clear/tombstone must cover EVERY path that can reintroduce the data; here the sidecar-own fields were verified but the parent-lineage display path wasn't. When clearing/hiding history, enumerate all readers that reconstruct it — lineage/parent, backup, state.db, cache.)
_Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; direct-history clear verified thorough (fully-empty persisted_clear, fails closed) BUT clear route touches ZERO parent_session_id (grep empty) → compressed-continuation replays parent history via _*lineage_messages_for_display (Codex empirical temp-session check); suite fully green (RFC line-bump kept #4812 green, #5513 collateral per #5542). Cert valid for sha:ea2dc31c.
4dfeb78
|
Shipped in v0.51.860 🎉 — merged via release Thanks @rodboev — this closes the #5532 P0 (clearing a conversation didn't stick: history resurrected from state.db after a refresh, and continued chats carried the full pre-clear context to the model). Your fix (persisted-clear verification + stale-backup removal + the truncate-to-empty watermark) shipped as the primary, combined with the sibling maintainer PR #5553's improvements per the reliability review:
Gate: Codex SAFE + Opus SHIP + full suite 11997 passed. Non-blocking follow-ups filed: #5570, #5571, #5572. |
…uilt on de-brittled base) Rebuilt the combined nesquena#5556+nesquena#5553 fix on v0.51.859 (now has the flake fix + the nesquena#5542 RFC de-brittle, so no anchor-test collateral). Nathan's call: rodboev's nesquena#5556 primary + fold self-built nesquena#5553 improvements, credit both. Clear handler now: - routes through shared truncate_session_at_keep(s,0) (single source of truth, sets watermark=_truncation_watermark_for([])==0.0, the nesquena#2914 sentinel that blocks state.db append-merge replay) - detaches compression lineage ONLY when the parent is a pre_compression_snapshot (Codex-caught: preserve genuine fork parent links for nesting + "Forked from") - rodboev's persisted-clear read-back verification + stale .bak removal RFC conflict resolved in favor of the de-brittled (symbol-anchor) master version. Ships both PRs' test files (state_db_replay + clear_truncation_watermark incl the fork-preservation regression). Opus (on the amended tree): both resurrection paths closed, detach lineage-safe, verification sound — SHIP. 3 non-blocking follow-ups filed (nesquena#5570 .bak crash-window, nesquena#5571 fork-stitch corner, nesquena#5572 messaging clear). Co-authored-by: rodboev <rodboev@users.noreply.github.com>
Thinking Path
/api/session/clearshould be a destructive reset, but it currently clears only the WebUI sidecar arrays and leaves the persistent merge unmarked.context_messages, because provider-facing history can otherwise keep pre-clear turns even when the visible transcript is empty.What Changed
api/routes.py: clearcontext_messages, staleactive_stream_id, and pending user-message fields alongsidemessagesandtool_calls, persist the existing truncate-to-empty sentinel, and remove the route-created pre-clear.json.bakonly after the fully cleared state is verified on disk.tests/test_issue5532_session_clear_state_db_replay.py: cover clear plus state.db replay suppression,context_messagesreset, stale active/pending state reset, route-created backup removal after verified persistence, existing backup preservation for already-empty sidecars, later backup recovery, and normal un-cleared state.db recovery.docs/rfcs/session-sse-contract-v1.md: refresh route source anchors shifted by theapi/routes.pyinsertion.Why It Matters
Clear Conversation now stays cleared after refresh, restart recovery, and the next prompt. Users no longer get old transcript rows or hidden model context after asking WebUI to reset a session.
Verification
python -m pytest tests/test_issue5532_session_clear_state_db_replay.py tests/test_3230_preserve_manual_session_title.py::test_clear_route_uses_rename_helper_not_bare_title_assignment tests/test_memory_session_lifecycle_generation.py::test_clear_session_evicts_outside_session_lock tests/test_session_truncate_keep_count_validation.py::test_truncate_zero_keep_count_clears_transcript tests/test_issue4836_manual_compression_recovery.py tests/test_metadata_save_wipe_1558.py -v --timeout=60python -m pytest tests/test_issue4812_session_sse_contract_rfc.py::TestEndpointDistinction::test_rfc_cites_current_global_endpoint_source tests/test_issue4812_session_sse_contract_rfc.py::TestEndpointDistinction::test_rfc_run_journal_anchors_land_on_real_source -v --timeout=60Full-suite CI context, not a required local check:
python -m pytest tests/ -v --timeout=60.Upstream
Closes #5532.
The maintainer confirmation at #5532 (comment) shaped the sentinel approach and the
context_messagesreset.Related: #5553 covers the route sentinel and context reset path. This branch also guards the clear-created
.json.bakrecovery edge so restart recovery cannot undo the clear, while preserving unrelated backup recovery.Model Used
GPT 5.5 via Codex CLI