Harden offline repair for oversized session replays

[ruizanthony]

Thinking Path

The runtime must not parse and copy replay-bloated sidecars inside an HTTP request, but an offline repair is safe only if it stays bounded in memory, coordinates with normal saves, preserves an exact rollback path, and fails closed on malformed or concurrently changed input. Review of the first implementation found three additional recovery hazards: large index rebuilds trusted a 64 KiB key-order prefix, the streaming validator accepted non-RFC JSON whitespace, and source publication was not durably ordered after the manifest rename.

What Changed

• above 128 MiB, defer inline repair only when an allocation-free scan finds an actual adjacent partial replay; valid large sessions remain writable;
• compact only top-level messages and context_messages in separate analysis/write passes;
• track exact replay keys in temporary SQLite with a bounded 4 MiB page cache instead of RAM-growing Python sets;
• serialize normal Session.save() and offline compact/restore publication through the same per-SID lock while advancing _sidecar_generation_v1 monotonically;
• serialize save, workspace recovery, compaction, restore, delete, and cleanup through the common SID authority; bind publication to generation + lifetime epoch, retain durable delete tombstones, and make both tombstone record/clear RMW fail closed when the fence is unreadable;
• preserve a recoverable backup when /clear is a no-op on an already-empty live session, while retiring pre-clear artifacts after a real truncate; restore malformed live JSON only through a tagged raw-digest CAS that is revalidated before publication;
• stream fixed index metadata regardless of top-level key order, skip large transcript/scene bodies, count messages when no persisted count exists, and reject a missing/invalid session_id instead of generating a phantom row;
• validate RFC 8259's exact whitespace set and reject non-standard constants or malformed copied JSON before any backup, manifest, or source publication;
• fsync the manifest directory entry before installing the compacted source, then fsync the source rename; remove and fsync an unpublished manifest after a non-crash source-install failure;
• preserve source mode on output, backup, manifest, and restore files; create manifest temporaries exclusively with the final private mode before writing content;
• exclude hidden maintenance manifests from session/index and startup-divergence discovery;
• reject --dry-run --restore rather than performing a destructive restore;
• provide fail-closed restore that refuses rollback after later sidecar changes.

Why It Matters

Very large replay-bloated sessions can now be repaired without request-path memory spikes, silent concurrent-write loss, omitted/phantom sidebar rows, malformed JSON republication, or a crash window where the compacted source is durable but its rollback manifest is not.

Operator Contract

Always stop or drain every WebUI process before running this strictly offline tool. The WebUI lifecycle paths hardened by this PR now share the SID authority, generation/epoch checks, and durable deletion fence; older runtimes and direct non-WebUI writers may not participate. The command is POSIX/WSL-only; use WSL rather than native Python on Windows.

python scripts/compact_session_replays.py --dry-run ~/.hermes/webui/sessions/<session-id>.json
python scripts/compact_session_replays.py ~/.hermes/webui/sessions/<session-id>.json
python scripts/compact_session_replays.py --restore <manifest-path>

RAM is bounded, but the temporary SQLite index needs scratch-disk headroom proportional to unique replay identities. One decoded JSON row is capped at 64 MiB. Strict stale-alias CAS after the maintenance lock is released remains the separate #7036 scope.

Contract Routing

State layers: session sidecar JSON, sidebar _index.json, per-SID maintenance lock, durable generation, content-addressed backup, and hidden rollback manifest.

Relevant public docs:

• AGENTS.md
• docs/CONTRACTS.md
• docs/GUIDELINES.md
• ARCHITECTURE.md

Scope

The cumulative reliability branch changes 15 files across the offline compactor, sidecar/recovery lifecycle, bounded metadata scanner, shared SID authority, and regression coverage. It remains one logical durability boundary: compact, restore, save, recover, delete, cleanup, index, and State DB materialization must agree on the same session lifetime.

This PR supersedes the offline-compaction portion of #6600. Please keep #6600 open as design/review history until the narrower replacements land.

Verification

• 266 passed — every test file modified against origin/master, on exact head f6b67279, with the Hermes Agent runtime provided and zero skips;
• 156 passed — historical recovery/sidecar regression portfolio, zero skips;
• 9/9 PASS — exact-SHA adversarial probes for authority ordering, delete/cleanup fail-closed behavior, epoch ABA, materializers, full-payload backup/restore, partial signatures, complete private cleanup, and bounded scanning;
• predecessor d04170d4 failed nine GitHub matrix jobs across all three Python versions, reproducing four historical contracts: no-op clear backup preservation, malformed-live recovery, canonical ephemeral cleanup, and complete delete artifact cleanup; all four are GREEN locally on f6b67279, including a new mutation-between-CAS-reads rejection probe;
• exact archive SHA-256 4e1691c0d15780e37b6a60a35468496a93cedfe5312354540c302051c3e6a070 remained stable before/after execution; Ruff diff-scoped, py_compile, compileall, and git diff --check pass;
• GitHub CI and independent exact-SHA review are running on f6b67279 and are not claimed complete here.

Risks / Follow-ups

• The compactor remains operationally offline because older runtimes and non-WebUI writers may not share this branch's authority protocol.
• Temporary-disk use grows with unique replay identities even though RAM remains bounded.
• Native Windows execution is unsupported; use WSL.
• Merge remains blocked until exact-head CI and independent review both pass.

Model Used

OpenAI Codex / gpt-5.6-sol, with Hermes read-only delegated adversarial reviews and local deterministic probes.

Related

• fix(sessions): prevent incomplete-message amplification #6600
• fix(session): fence sidecar writes by durable revision #7036

[greptile-apps]

Confidence Score: 5/5

The reviewed changes appear safe to merge, with both previous findings resolved and no actionable new failure established.

Summary

This PR hardens oversized-session replay repair and the surrounding sidecar lifecycle.

• Adds bounded streaming analysis and offline replay compaction with durable backup and restore semantics.
• Coordinates save, repair, restore, deletion, recovery, and cleanup through shared per-session authority and revision fences.
• Improves bounded index metadata extraction and excludes hidden maintenance artifacts from discovery.
• Adds regression coverage for malformed input, concurrent mutation, cleanup failures, rollback safety, and large-session behavior.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Oversized session sidecar] --> B[Bounded metadata and replay scan]
    B -->|No adjacent partial replay| C[Normal session remains writable]
    B -->|Repair required| D[Defer request-path repair]
    D --> E[Offline compactor acquires SID authority]
    E --> F[Validate source revision and JSON]
    F --> G[Create and fsync byte-exact backup]
    G --> H[Publish and fsync rollback manifest]
    H --> I[Install and fsync compacted sidecar]
    I --> J[Advance generation under same lifetime epoch]
    J --> K[Optional restore validates manifest, backup, and current revision]

Loading

_{Reviews (19) · Last reviewed commit: "fix(background): treat a False hidden-cl..."}

[ruizanthony]

CI follow-up on be1e0db6:

• isolated models.SESSIONS in both index-rebuild probes so shard order cannot inject unrelated cached sessions;
• replaced the obsolete same-SID replace barrier with an event-gated serialization probe that observes the second authority attempt before releasing the first save;
• replayed all three former CI failures locally (3 passed), with diff-scoped Ruff, compilation, and git diff --check clean.

The commit is test-only; production remains the reviewed e1881e68 remediation.

[ruizanthony]

Rebased without conflicts onto current origin/master (108841ba); upstream changed only the composer reconnect UI and did not overlap this PR. Final head: dd74c5de. The three former CI failures pass on the rebased tree, with diff-scoped Ruff, compilation, and git diff --check clean.

[ruizanthony]

Remediated the three recovery blockers on exact head cb8c0b3b: large index rebuilds now stream fixed metadata across arbitrary key order and require a persisted valid session ID; the compactor accepts only RFC 8259 whitespace; and manifest publication is directory-fsynced before source installation with cleanup+fsync on non-crash install failure. Also closed the related dry-run restore, hidden-manifest diagnostic, temporary mode, and native-Windows contract residuals. The adversarial probes were RED on the predecessor and GREEN on this head. Final post-rebase gates: 774 session tests and 117 compact/recovery/index tests; Ruff/compile/diff-check clean. An actual 134,217,812-byte messages-first sidecar loaded at 58,912 KiB high-water RSS. Please re-review exact head cb8c0b3b.

[ruizanthony]

Final rebase-only refresh: exact head is now 58d90099, based on current master 71584418. The upstream delta touched only CHANGELOG.md, Dockerfile, and tests/test_sqlite_wal_reset_upgrade.py; none overlaps this PR. Post-rebase targeted gate: 117 passed; Ruff/compile/diff-check remain clean. The three-P1 remediation is unchanged from the reviewed cb8c0b3b tree, but please attach the final gate to exact head 58d90099.

[ruizanthony]

CI follow-up on exact head cb6e1873: the three Python shard failures were the same two source-contract oracles in tests/test_pr1341_context_window_persistence.py. The bounded reader introduced the first explicit def __init__ before Session, and the shared metadata tuple no longer matched the historical METADATA_FIELDS = [...] source assertion. Both failures were reproduced locally, then fixed without runtime behavior changes by using an explicit reader initializer and retaining the shared metadata field collection as a list. The complete #1341 + offline-compactor gate is 37 passed; Ruff/compile/diff-check are clean. Fresh CI is running on cb6e1873.

[ruizanthony]

Final rebase-only refresh: exact head is now 5283e1f0 on upstream dc3bf44e. The upstream delta did not overlap the PR files. Post-rebase local gates: 123/123 targeted tests plus Ruff, py_compile, and diff-check. Please review this exact head; prior exact-SHA reviews are superseded.

[ruizanthony]

Exact final candidate: e3f0307fd882251757143c4e0312e7d4bf9c7ae6 on current master dc3bf44e. The three additional commits close archive restore/publication, manifest/retention, permission, CAS, and bounded metadata semantic gaps without weakening replay identity. Official diff-derived gate: 126 passed; changed-line Ruff, Python compilation, and git diff --check are clean. @nesquena-hermes please review this exact SHA; the favorable 5283e1f0 review is superseded by these hardening commits.

[ruizanthony]

Final exact candidate after global-suite remediation: ac2cf0b54a70f4d0d639cbe477b2de475a5477f8 on unchanged master dc3bf44e. Sidecar CAS is now scoped to the SID actually observed: fresh standalone replacements retain compatibility, loaded stale writers remain fenced, rotations start a new epoch, and authoritative State DB refresh transfers the matching revision.

Evidence: 128 modified-portfolio tests passed; Ruff diff, py_compile, and git diff --check are clean; GitHub CI is 24/24 green. Please re-review and merge this exact SHA.

[ruizanthony]

HOLD MERGE on current head ac2cf0b54a70f4d0d639cbe477b2de475a5477f8: an independent exact-SHA integration review reproduced a P1 adapter mismatch where the runtime partial-message signature is bytes but the offline compactor accepts only tuples, causing an exact duplicate partial to remain unchanged. Remediation and regression test are in progress. I will post a replacement exact SHA and lift this hold only after the full compactor portfolio and integration review are green.

[ruizanthony]

Replacement candidate published: bd003e939c508185491e63ce0b8cd251c9a0b06b. The tuple/bytes adapter P1 is covered by RED→GREEN probes: exact duplicate bytes signatures compact; durable provenance differences remain separate. Compactor suite: 51 passed. Composed three-PR portfolio: 689 passed, 2 expected skips. Lint/compile/diff-check clean. HOLD remains until exact-SHA re-review and CI complete.

[ruizanthony]

HOLD on current head bd003e939c508185491e63ce0b8cd251c9a0b06b: independent exact-head adversarial review has reproduced a P1 publication/delete race. After DELETE returns ok=True with a durable tombstone, an already-running compactor can republish its pre-delete epoch and resurrect the sidecar when replay-artifact cleanup is degraded. Final review is still running and may report additional findings. Please do not merge this head despite green CI.

[ruizanthony]

Final independent exact-head verdict for bd003e939c508185491e63ce0b8cd251c9a0b06b: BLOCK (2×P0, 3×P1, 1×P2). P0s are (1) delete/compactor lock-domain mismatch allowing resurrection of a tombstoned session with plaintext replay artifacts, and (2) raw workspace-binding writer overwriting a concurrent canonical save and regressing generation. Additional blockers: cleanup artifact retention, fail-open unlink, tombstone-cap eviction, and dependence on unpublished stacked fixes. A single local TDD remediation is in progress. Please do not merge this head.

[ruizanthony]

Final independent exact-SHA review of f6b67279eccfecf6b3bee9993e7b709b8204b039: APPROVE — no reproducible P0/P1/P2, and the corrupt-tombstone P2 is closed.

Evidence on this exact head:

• HEAD/tree verified clean and byte-identical to the published blobs; PR head on GitHub matches this SHA exactly; mergeStateStatus CLEAN.
• CI: 24/24 checks SUCCESS on this SHA.
• Corrupt-tombstone closure confirmed by code review AND independent reproduction: tombstone RMW is fail-closed under cross-process authority (strict=True), atomic 0600 rewrite with file+dir fsync, post-save re-read; a torn fence raises before any write and Session.save() fails closed; deleted sessions cannot be resurrected by reconciliation, the compactor, or capacity trim.
• All delete/cleanup paths (route, zero-message, ephemeral, hidden) go through the SID authority with tombstone recorded BEFORE unlink; failure returns 500 without unlinking.
• Suites at the exact SHA: 145 passed (compactor, PR findings, Recovery: exempt the truncate-to-empty sentinel from .bak restore (clear crash-window) #5570, P0 DATA LOSS: metadata-only Session.save() wipes conversation history (v0.50.279+) #1558, [Bug]: Session messages lost when WebUI server restarts during task execution #765, cancelled-turn, design(worktree): define lifecycle, cleanup, and safety model #2057) + 173 passed / 1 skipped (regressions, session_index, file_manager, Orphan native-WebUI sessions (zero-message state.db rows) accumulate in sidebar and 404 on click — sibling of #4591 #4985, public_share). >128 MiB session preserved byte-for-byte.

Non-blocking doc note: the offline compactor preserves mode (0600) but not ownership when run as root on foreign-owned sidecars (recoverable via chown; no data loss).

This supersedes all prior HOLDs on bd003e93, ae5e349c and earlier candidates. Ready for maintainer re-review and merge.

[nesquena-hermes]

🔬 Gate certification — RED ⛔

Certified head: sha:d3420c6a18f5a34f99a398dda2772f7733048deb (contributor head; gated as its rebase onto current origin/master 842b2ac4, integration b69e2382)
Verdict: gate-fail — two reproduced data-durability / liveness defects in the new hidden-/btw ephemeral-cleanup path.

This cert is valid only while the head stays at sha:d3420c6a.

What I ran (own isolated worktrees, rebase-first)

• Rebase-first: applied the PR's diff onto current origin/master; conflict-free. git patch-id --stable of the contributor diff (e88a11bc…) is identical before and after rebase, so the findings below are the PR's own, not staleness.
• Codex (adversarial diff-vs-master reproduction) → SHIP ONLY WITH FIXES (two blockers, both reproduced with production-shaped probes).
• Opus (architecture/security) → independently flagged the same /btw cancel deadlock plus notes below.
• Full pytest suite (sandboxed, --basetemp isolated): 15,125 passed, 9 failed + 2 errors — every one of those 11 non-pass nodes reproduces identically on the exact current-master control in a matched guarded sandbox (zero candidate-only failures, pass-count delta 0). So the suite surfaces no PR-introduced test regression; they are pre-existing box/environment failures.
• Independent repros (both in the bubblewrap sandbox, never bare execution):
  • deadlock probe → cancelled /btw worker stays alive with its sidecar on disk (thread_alive=True sidecar_exists=True); the exact current-master control completes and removes it.
  • tombstone-eviction probe → after WEBUI_DELETED_SESSION_TOMBSTONE_CAP (1000) hidden cleanups, a genuinely-deleted session's fence is evicted and recover_missing_sidecars_from_state_db re-materializes it (target_still_fenced=False materialized=1 target_exists=True).

Findings (blockers)

1. [CORE / liveness] /btw cancellation self-deadlocks the worker and strands the hidden session.
api/streaming.py:2303 — _cleanup_ephemeral_session_sidecar_locked() acquires _get_session_agent_lock(sid), but every production caller already holds that same non-reentrant per-session threading.Lock. On the cancel path (_cleanup_ephemeral_cancelled_turn → line 2325) and the normal-completion path (line 10866) the nested re-acquisition deadlocks; the supposedly-ephemeral session then persists on disk / in the sidebar. Reproduced with a production-shaped probe (worker hangs, sidecar remains).
Fix-spec: make the cleanup helper require the caller-held agent lock (drop its own with _get_session_agent_lock(sid)), and acquire that lock at the normal-completion caller (line ~10866). Add a production-shaped cancellation regression that asserts the worker exits and the sidecar is gone.

2. [SILENT / data-loss] Hidden-cleanup churn evicts live deletion fences → deleted transcripts resurrect.
api/models.py:1150 — hidden /btw/background cleanup calls _delete_session_sidecar_artifacts_locked(..., record_tombstone=True), so it shares the single 1000-entry deletion tombstone log with real user deletions (_save_webui_deleted_session_tombstone truncates to the last 1000). After >1000 hidden cleanups, an older user-deletion fence is silently evicted; if that session's state.db row cleanup had previously failed, /api/session/recovery/repair-safe → recover_missing_sidecars_from_state_db re-materializes the deleted transcript (its tombstone check now returns false). Reproduced against the real cap + recovery function.
Fix-spec: don't let hidden ephemeral cleanups evict authoritative user-deletion fences. Either delete the ephemeral session's state.db rows instead of tombstoning it (so no fence is needed and no recovery source remains — matches the earlier maintainer note on this PR), or exempt hidden-session cleanups from the shared capped fence, or make the cap not evict a fence while any authoritative recovery source (state.db row) still exists.

Maintainer notes (non-blocking, from Opus)

• _handle_background replaces a successfully-computed answer with a cleanup-failed message when the hidden sidecar can't be removed — delivering the answer and flagging the cleanup failure is more proportionate.
• The compactor nulls compression_anchor_visible_idx when messages change while the runtime inline repair leaves it untouched (defensible, but the two paths disagree).
• restore_manifest trusts manifest source/backup paths without the symlink/identity checks compact_sidecar applies (operator-only tool, low risk).
• _read_sidecar_generation in api/models.py has zero callers (dead code).

Recommendation to the next agent

RED — do not merge. Fix findings #1 (lock-ordering rework) and #2 (fence/cap redesign), then re-warm/re-gate. Both need @ruizanthony's design intent (lock contract + delete-vs-tombstone policy for hidden sessions), so this is a contributor bounce, not a mechanical maintainer fix. The concept (bounded offline session-repair + hidden-/btw cleanup) remains in-scope health/operator hardening; the engineering blockers are the two reproduced durability/liveness defects above.

---

Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy/close — that's the release agent's call. This cert is valid only while the head stays at sha:d3420c6a.

[ruizanthony]

Rebased onto current master (c367ef3) and pushed 2752e30.

Greptile P1 (cleanup failure reported as success) — fixed. A False return from _delete_hidden_background_session_sidecar() now follows the same failure path as an exception (warning log, "(background task cleanup failed)" published instead of the answer). New regression tests in tests/test_pr7039_gate_red_regressions.py cover the False (red on the previous head, green now), exception and True cases. Thread resolved.

Verification on 2752e30: targeted suite (PR tests + background/session adjacent files) 926 passed / 1 skipped / 1 xfailed; ruff_lint.py (diff-scoped) clean; compileall OK; git diff --check OK.

CI note — Conversation lifecycle (informational) / live-to-final (terminal-error) on the previous head 5d0b393: this job is continue-on-error: true and the workflow concluded success. The failure (anchor scene was not persisted before reload … anchor scene requests: []) is a known intermittent one, not attributable to this PR: the same message appears on unrelated PR heads (e.g. runs 35615549744 and 35604060232 on 2026-09-21) while the normal and historical-transcript-hydration matrix legs passed here, and this PR does not touch static/ or tests/browser_conversation_lifecycle.py. No change made for it.

[ruizanthony]

Rebased onto current master 5e16dd860f3397a5f540f69e92c33eb396f63df7 and pushed 48fc6675694c2c689b3ef04d3a7c63d9d884d864 with an explicit lease. 155 focused durability/recovery tests passed; Ruff diff, compileall and diff check passed. Combined patch-id unchanged; exact-head CI and maintainer review pending. This is a source refresh, not a claim that the new head has passed GitHub CI or maintainer certification.