fix(#5570): skip clear sentinel backups during recovery

[rodboev]

Thinking Path

• /api/session/clear can still crash after persisting an empty sidecar and before removing the pre-clear .json.bak.
• Recovery is the shared decision point for manual recovery and startup recovery, but clear intent has to come from the clear operation, not from an empty sidecar shape that other flows can produce.
• The fix now marks real clear operations with persisted provenance and skips only backups from before that clear generation.

What Changed

• api/models.py: persists clear_generation as session metadata so post-clear saves keep the clear provenance.
• api/routes.py: writes a new clear generation during /api/session/clear only when the clear actually shrinks a sidecar with messages, and verifies that marker before treating the stale backup cleanup as safe.
• api/session_recovery.py: skips restoring a larger .json.bak only when the live sidecar has clear provenance and the backup does not carry the same generation.
• tests/test_issue5570_clear_backup_recovery.py: covers stale pre-clear skip, same-generation backup restore, clear-shaped files without provenance, malformed fail-open recovery, and compression recovery preservation.
• tests/test_issue5532_session_clear_state_db_replay.py: restores the later-backup recovery contract and verifies no-op clears of already-empty sessions keep existing recoverable backups.

Why It Matters

Clearing a conversation stays durable across the crash window that leaves a stale pre-clear backup behind. Legitimate post-clear backup recovery still works because recovery checks provenance rather than content shape alone.

Verification

python -m pytest tests/test_issue5570_clear_backup_recovery.py tests/test_issue5532_session_clear_state_db_replay.py -v --timeout=60
python -m pytest tests/test_issue5532_clear_truncation_watermark.py -v --timeout=60

Full-suite CI context, not a required local check unless requested: python -m pytest tests/ -v --timeout=60.

Upstream

Closes #5570.

Model Used

GPT 5.5 via Codex CLI

[greptile-apps]

Greptile Summary

This PR fixes a crash-window bug (#5570) where /api/session/clear persists an empty sidecar and exits the lock but crashes before removing the pre-clear .json.bak; on next startup, recovery incorrectly treats the larger backup as data-loss evidence and restores the cleared history.

• Adds a clear_generation UUID field to Session — written only for clears that actually shrink messages, propagated through save(), and verified before the pre-clear backup is deleted in routes.py.
• Adds _session_records_clear_sentinel() in session_recovery.py to detect a provenanced clear in the live sidecar, blocking restore only when the backup lacks the same generation (same-generation backups, representing post-clear recovery scenarios, still restore normally).
• Backs the fix with a new test file covering stale-skip, same-generation restore, shape-without-marker fail-open, malformed fail-open, and compression recovery preservation, plus two contract additions to the existing /api/session/clear does not delete messages from state.db — history survives clear+refresh #5532 suite.

Confidence Score: 5/5

Safe to merge — the fix is narrowly scoped to the crash window between clear-persist and backup-unlink, and fail-open semantics throughout the recovery path mean any parse or shape mismatch reverts to ordinary backup restoration.

The change threads a UUID generation token through three layers (model, route, recovery) with consistent None-guarding at each. Same-generation backups still restore normally, shape-without-marker files still restore, and malformed sidecars still restore — all confirmed by the new test suite. uuid was already imported in routes.py. No field is missing from save()'s METADATA_FIELDS list.

No files require special attention.

Important Files Changed

Filename	Overview
api/models.py	Adds clear_generation to the Session constructor, instance assignment, and METADATA_FIELDS/save() — minimal, correctly placed, backwards-compatible (defaults to None).
api/routes.py	Sets clear_generation only when had_sidecar_messages is True; extends the post-clear persistence verification to include the generation marker; backup deletion is gated on both conditions. Logic is consistent and handles the empty-session no-op case correctly.
api/session_recovery.py	New _session_records_clear_sentinel() checks generation presence, same-generation early-exit (allowing post-clear backup restore), and full expected-shape validation — fails open on any read/parse/shape error. Integrated cleanly at the top of the bak_count > live_count branch.
tests/test_issue5570_clear_backup_recovery.py	New test file with comprehensive coverage: stale-skip, same-generation restore, shape-without-marker fail-open, watermark mismatch fail-open, active/pending field fail-open, missing-field fail-open, malformed JSON fail-open, and compression recovery preservation.
tests/test_issue5532_session_clear_state_db_replay.py	Extends existing suite with clear_generation assertions, restores later-backup contract fields, and adds a no-op clear test verifying that clearing an already-empty session keeps existing recoverable backups and leaves clear_generation as None.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant ClearRoute as /api/session/clear
    participant Session as Session.save()
    participant Disk as Disk (.json / .bak)
    participant Recovery as session_recovery.py

    Client->>ClearRoute: POST /api/session/clear
    ClearRoute->>ClearRoute: "had_sidecar_messages = bool(s.messages)"
    ClearRoute->>ClearRoute: truncate_session_at_keep(s, 0)
    ClearRoute->>ClearRoute: "s.clear_generation = uuid.hex (if had_messages)"
    ClearRoute->>Session: s.save()
    Session->>Disk: copy existing to .json.bak (pre-clear, no clear_generation)
    Session->>Disk: "write new sidecar (0 msgs, clear_generation=abc)"
    Note over ClearRoute,Disk: Crash window — server dies here
    ClearRoute->>Disk: "verify persisted.clear_generation == s.clear_generation"
    ClearRoute->>Disk: unlink .json.bak (if had_messages and persisted_clear)

    Note over Recovery: On startup / manual recover
    Recovery->>Disk: "read .json (0 msgs, clear_generation=abc)"
    Recovery->>Disk: read .json.bak (N msgs, no clear_generation)
    Recovery->>Recovery: _session_records_clear_sentinel()
    Recovery->>Recovery: clear_generation present, bak lacks same gen, shape matches
    Recovery-->>Client: "recommend=no_action (stale backup skipped)"

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client
    participant ClearRoute as /api/session/clear
    participant Session as Session.save()
    participant Disk as Disk (.json / .bak)
    participant Recovery as session_recovery.py

    Client->>ClearRoute: POST /api/session/clear
    ClearRoute->>ClearRoute: "had_sidecar_messages = bool(s.messages)"
    ClearRoute->>ClearRoute: truncate_session_at_keep(s, 0)
    ClearRoute->>ClearRoute: "s.clear_generation = uuid.hex (if had_messages)"
    ClearRoute->>Session: s.save()
    Session->>Disk: copy existing to .json.bak (pre-clear, no clear_generation)
    Session->>Disk: "write new sidecar (0 msgs, clear_generation=abc)"
    Note over ClearRoute,Disk: Crash window — server dies here
    ClearRoute->>Disk: "verify persisted.clear_generation == s.clear_generation"
    ClearRoute->>Disk: unlink .json.bak (if had_messages and persisted_clear)

    Note over Recovery: On startup / manual recover
    Recovery->>Disk: "read .json (0 msgs, clear_generation=abc)"
    Recovery->>Disk: read .json.bak (N msgs, no clear_generation)
    Recovery->>Recovery: _session_records_clear_sentinel()
    Recovery->>Recovery: clear_generation present, bak lacks same gen, shape matches
    Recovery-->>Client: "recommend=no_action (stale backup skipped)"

Loading

_{Reviews (3): Last reviewed commit: "fix(#5570): require clear provenance for..." | Re-trigger Greptile}

[nesquena-hermes]

🔬 Gate certification — RED ⛔ (sentinel-skip is content-shape-based → suppresses legitimate backup recovery + reverses the #5532 recovery contract — CORE)

Certified head: sha:e1897a1b (rebased onto current master, git apply clean) · PR: #5584 · rodboev, fix(#5570): skip clear sentinel backups during recovery
Verdict: The #5570 intent (don't restore a stale larger backup over an intentionally-cleared session) is right, but the fix decides "intentional clear" from the live sidecar's CONTENT SHAPE (empty messages + 0.0 watermarks + null pending) — and that shape is NOT unique to a clear. A legitimate session that transiently matches it gets its real backup recovery silently suppressed (no_action), losing messages and reversing the existing #5532 recovery contract. Provenance, not shape, must drive the skip.

What I ran (rebased worktree /tmp/wt-rebase-5584)

Gate	Result
Rebase onto current master	✅ git apply clean
Codex (reproduce)	SHIP-WITH-FIXES — 1 CORE (content-shape false-positive suppresses recovery); Codex verified against the impl + contract test
Full pytest suite	✅ rc=0 green — but note the PR MODIFIED test_issue5532_session_clear_state_db_replay.py (+23), i.e. it changed the very test encoding the recovery contract it reverses (Codex flags :185)
PR's own test	✅ 16/16 (test_issue5570_clear_backup_recovery.py)

Findings

⛔ CORE (I + Codex CONFIRMED) — content-shape sentinel suppresses legitimate recovery (api/session_recovery.py:264): the branch returns no_action SOLELY on _session_records_clear_sentinel(session_path), which matches the sidecar's content shape (messages==[], context_messages==[], truncation_watermark/boundary==0.0, active_stream_id/pending_* None/[]). But that shape isn't unique to /api/session/clear: _advance_truncation_watermark_after_commit() intentionally leaves 0.0 (streaming.py:4956) and normal stream completion clears active/pending (streaming.py:8738-8742), so a session with REAL post-clear activity can carry the same fields. If a legitimate later shrink-to-empty then creates a .json.bak (models.py:1228-1246), recovery is suppressed purely because the live sidecar "looks like" a clear — silently refusing to restore legitimate messages, reversing the origin/master recovery contract (test_issue5532:185, which this PR edits). Fix (Codex-exact): don't infer intent from content shape — write explicit clear PROVENANCE from /api/session/clear (a marker / clear-generation stamp) and skip recovery ONLY when the backup predates that specific clear operation; otherwise keep restoring larger same-generation backups. Mark the clear operation itself, not "every later empty/0.0 sidecar."

✅ #5570 case is real (keep the goal): a crash right after clear CAN leave a stale larger backup that resurrects cleared content — worth fixing. The mechanism (provenance-marked clear + predates-check) is the safe version; the content-shape proxy is the flaw.

Recommendation to the next agent / author

RED — gate-fail/changes-requested (1 CORE): drive the backup-skip from explicit clear PROVENANCE (a marker/generation written by /api/session/clear), not from the live sidecar's content shape — skip only backups that predate that clear; keep restoring legitimate larger same-generation backups. The #5570 fix is worth having, but a content-shape sentinel is ambiguous with legitimately-empty sessions and reverses the #5532 recovery contract. Also revert/justify the test_issue5532:185 modification — changing the contract test to accommodate the regression is a flag. concept 4/5 (real #5570 fix; wrong provenance signal). Author @rodboev (T1). crit=3, data-integrity. (Gate value + recurring lesson: infer intent from PROVENANCE, not from CONTENT SHAPE — an empty/0.0 sidecar is not proof of an intentional clear; the same class as #5471 "identity+TTL can't tell new-value-same-key" and #5556. And watch a PR that MODIFIES the very test encoding a contract it changes — that's a signal the change reverses an intended invariant.)

---

_Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; confirmed recovery skip is content-shape-only (session_recovery.py:264 returns no_action solely on session_records_clear_sentinel), the shape isn't clear-unique (0.0 watermark not advanced streaming.py:4956, pending cleared on normal completion 8738-8742), so a legit transiently-empty session's backup recovery is suppressed + the #5532 contract (test:185, which this PR edits) reversed. Fix: clear-provenance marker + predates-check, not content shape. Suite green (runtime provenance-ambiguity, not a suite fail). Cert valid for sha:e1897a1b.

[rodboev]

Good catch. I reworked the recovery skip so it no longer treats the empty sidecar shape as proof of an intentional clear.

1. /api/session/clear now writes a clear_generation marker only when it actually shrinks a sidecar that had messages. That marker is persisted as normal session metadata, so later post-clear saves carry the same generation.

2. inspect_session_recovery_status() now skips a larger .json.bak only when the live file has a clear marker and the backup does not carry that same marker. If the backup carries the same generation, recovery restores it as a legitimate post-clear backup.

3. I restored the /api/session/clear does not delete messages from state.db — history survives clear+refresh #5532 contract test direction and added coverage for the exact false positive: an empty, 0.0 sidecar without clear provenance now restores its larger backup instead of returning no_action.

This keeps the #5570 crash-window fix for stale pre-clear backups, but it moves the decision from content shape to explicit clear provenance.

[nesquena-hermes]

🔬 Gate certification — GREEN ✅ · CONVERGED (round-1 content-shape CORE fixed → clear-provenance gate)

Certified head: sha:6792e58d (clean rebase, branch gate-rebase/5584-clear-provenance-recovery) · PR: #5584 · rodboev, fix(#5570): skip clear sentinel backups during recovery
Verdict: Full gate GREEN after round 2. My round-1 CORE (skip driven by ambiguous CONTENT SHAPE → suppressed legit recovery + reversed the #5532 contract) is fixed with explicit PROVENANCE: /api/session/clear stamps a unique clear_generation, and recovery skips a backup only when the live sidecar carries that marker AND the backup predates it. Codex SAFE, suite fully green (incl. the #5532 contract test).

What I ran (rebased worktree /tmp/wt-rebase-5584b)

Gate	Result
Rebase onto current master	✅ git apply clean
Codex (reproduce)	SAFE TO SHIP — 0 findings; round-1 CORE confirmed fixed
Full pytest suite	✅ 12039 passed, 0 failed
PR + #5532 contract tests	✅ 17/17 (incl. the previously-reversed test_issue5532 — now honored, not loosened)

Findings — round-1 CORE CLOSED

✅ Provenance-gated (no more content-shape false-positive): /api/session/clear sets s.clear_generation = uuid4().hex (only when had_sidecar_messages) at routes.py:13767 — a unique per-clear provenance stamp (server-set, a real Session field models.py:1056/1108). The recovery skip _session_records_clear_sentinel(session_path, bak_path) (session_recovery.py:228-231) now returns False UNLESS the live sidecar carries a non-empty clear_generation AND the backup does NOT carry the same generation — so:

• a legit transiently-empty session (no clear_generation) is NEVER falsely skipped → recovery runs (round-1 CORE closed);
• a genuinely cleared session's PRE-clear larger backup IS still skipped (Recovery: exempt the truncate-to-empty sentinel from .bak restore (clear crash-window) #5570 preserved);
• a same-generation post-clear backup STILL restores (not over-suppressed).
  Codex confirmed clear_generation is server-set + unique (uuid4, no spoof/collision), None when no messages cleared (never triggers skip), and the /api/session/clear does not delete messages from state.db — history survives clear+refresh #5532 recovery contract is honored (test passes, not loosened). 17 targeted + full suite green (0 failures).

Recommendation to the next agent

Ready to merge — use branch gate-rebase/5584-clear-provenance-recovery (sha:6792e58d), NOT the PR's stale head fe0d6836. The #5570 fix is now provenance-based (not the ambiguous content-shape sentinel) — a cleared session's stale pre-clear backup is skipped, but legitimate transiently-empty sessions and same-generation backups still recover, and the #5532 contract is honored. Codex SAFE + 17 targeted + full suite green (0 failures), backend/data-integrity. concept 4/5 (real #5570 fix, converged to the correct provenance model). Author @rodboev (T1). crit=3, data-integrity.

---

Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Rebased onto current master; round-1 content-shape CORE fixed via provenance — /api/session/clear stamps unique clear_generation (routes.py:13767, models.py:1056/1108), recovery skips only when live sidecar has the marker AND backup lacks the same generation (session_recovery.py:228-231); legit-empty not falsely skipped, #5570 preserved, same-generation restores, #5532 contract honored (test passes). Codex SAFE + 17 targeted + full suite green (0 failed). Cert valid for sha:6792e58d.

[nesquena-hermes]

Shipped in v0.51.875 — thanks @rodboev! 🎉

Clearing a conversation can no longer be undone by crash recovery. /api/session/clear writes a one-time .json.bak before wiping; if the app crashed/restarted, recovery saw the backup as "larger" and could resurrect the cleared transcript — including on top of a message sent after clearing. Clear now stamps a unique per-clear marker; recovery treats a backup predating that marker as stale, so a cleared conversation stays cleared. Genuine crash-loss recovery still restores.

Gate (2 rounds): Codex round-1 caught a real CORE gap — after clear + one post-clear message with a surviving stale .bak, recovery restored the pre-clear transcript on top of the new message. Root-fixed (_live_supersedes_backup_by_clear_generation, scoped to non-empty live, fail-open on partial reads) + 2 regression tests. Round-2 Codex SAFE TO SHIP; full suite 12067 passed. Merged via #5610. Hardens the #5532 clear data-loss fix.