fix(session): fence sidecar writes by durable revision

[ruizanthony]

Thinking Path

WebUI session sidecars are durable conversation state. A writer that publishes from a stale in-memory snapshot can erase newer turns, so every mutation must be authorized by the exact revision and digest it observed.

What Changed

• serialize compliant writers per session ID across threads and processes;
• reject stale in-memory sessions instead of allowing last-writer-wins transcript loss;
• hash the exact canonical LF bytes published on native Windows;
• make live-sidecar first publication atomically create-only via POSIX hard-link or native Windows rename, and fail closed when neither primitive exists;
• preserve shrinking-write backups by ordered semantic dominance, or stream malformed, non-object, foreign-SID, and incomparable generations into digest-verified temporary archives before atomic promotion without requiring hard-link support;
• apply the same authority to load-time repair, .bak recovery, state.db materialization, workspace metadata patching, and explicit session deletion;
• revalidate delete tombstones and state rows under the SID authority and reject backups whose embedded session identity does not match their path;
• never grant a stale workspace alias the new durable revision, reload/evict stale cached owners, and make chat-start callers reauthorize the returned owner and recompute its recovery state before model resolution or turn start;
• retire a backup only when both its exact receipt and the authorizing live generation still match.

Why It Matters

Concurrent save, recovery, clear, delete, or workspace reconciliation must never turn a stale snapshot into the winning transcript. Ambiguous or unverifiable generations fail closed, while intentional shrinking writes retain a recoverable predecessor.

Legacy sidecars without _sidecar_generation_v1 remain readable as generation 0; their first fenced mutation advances them to generation 1. Cache freshness uses the bounded metadata prefix for generated sidecars rather than hashing a large transcript on every read.

Scope Boundaries

This PR changes only the session sidecar durability layer and its regression coverage.

It does not include:

• strict replay-row reconciliation (separate PR fix(webui): make replay reduction payload-strict #7032);
• offline conversation squash/compaction orchestration (separate PR Harden offline repair for oversized session replays #7039);
• UI, SSE, or model-context behavior changes.

Current GitHub delta: 7 production/docs files (+1732 / -324) and 20 test files (+3107 / -144).

State layer and invariant

Layer: session JSON sidecar and .json.bak recovery snapshot. _index.json remains a derived projection.

Invariant: a writer may publish only while the exact generation and content digest it observed remain current. Missing-target publication is create-only. Any intentional history shrink must first preserve a recoverable generation; incomparable primary backups are archived before the latest snapshot is promoted. Recovery must still be authorized against durable delete/state evidence while holding the SID authority.

Contract Routing

• Contract family: session sidecar persistence, recovery, and exact-revision concurrency control.
• Authorities: docs/CONTRACTS.md, the repository run-state consistency contract, and the sidecar generation/digest invariants documented in this PR.
• This is a durability repair, not an intentional weakening of behavior: stale or unverifiable writers are rejected, and recoverable history remains authoritative.
• Evidence routes through the sidecar revision/fence, reconciliation, workspace recovery, compression recovery, and stale-owner regression suites.

Verification

• exact head ed2d93d1: all 24 GitHub checks are completed successfully, including the Python 3.11/3.12/3.13 matrix, browser smoke, live-to-final, lint, docs, and Greptile;
• exact local portfolio: 533 passed, 2 skipped; the skipped cases are the two agent-dependent tests in that isolated review harness;
• the worktree and tree remained unchanged across the local portfolio (1909336d3bf02c8c5e9b2c44e6775f06952fb35b);
• an independent exact-SHA review is running; the two historical CHANGES_REQUESTED reviews remain in force until that review proves their findings obsolete.

Review focus

• generation/digest comparison and legacy-generation upgrade;
• lock ordering between the per-session mutation lock and sidecar authority;
• create-only publication on first save, recovery, and materialization;
• ordered backup monotonicity, content-addressed incomparable archives, and dual-receipt cleanup;
• tombstone/state-row revalidation inside the authority;
• cache invalidation after out-of-band writers without per-read full-file hashing.

Rollback

Revert the PR commits. The added generation field is ignorable metadata for older code, so rollback does not require a data migration. Keep any .json.bak and .json.bak.archive-* files created by the fenced writer until the live sidecar has been inspected.

Model Used

OpenAI Codex / gpt-5.6-sol, with independent read-only adversarial review through Hermes delegation.

Release note

Session sidecar writes now reject stale writers and preserve recoverable history before shrinking, reducing conversation loss during concurrent save, repair, recovery, workspace reconciliation, clear, and delete operations.

[greptile-apps]

Greptile Summary

The PR introduces durable generation-and-digest fencing for session sidecar mutations and extends that authority across recovery, state database reconciliation, workspace repair, and deletion.

• Serializes sidecar writers per session across threads and processes.
• Adds create-only first publication and exact-revision checks for later mutations.
• Preserves shrinking-write backups and archives incomparable generations.
• Revalidates recovery, deletion, and state database materialization under the session authority.
• Updates chat-start and streaming cleanup paths to reject stale cached owners.
• Adds extensive concurrency, recovery, and stale-generation regression coverage.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains established in the available follow-up-review scope.

No blocking failure remains.

Important Files Changed

Filename	Overview
api/models.py	Adds the central sidecar revision, cross-process authority, backup preservation, state materialization, and deletion primitives.
api/session_recovery.py	Revalidates backup recovery and state database materialization while holding per-session authority.
api/session_discoverability.py	Makes discoverability repair create-only and rechecks authoritative state before publication.
api/routes.py	Reauthorizes stale session owners and routes explicit deletion through the fenced artifact-removal protocol.
api/streaming.py	Applies revision-aware ownership and guarded deletion to cancellation and hidden-session finalization paths.
tests/test_session_sidecar_revision_fence.py	Adds broad regression coverage for concurrent publication, stale writers, backups, tombstones, recovery, and cleanup.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant Owner as In-memory Session
  participant Lock as Per-SID Authority
  participant Sidecar as Session Sidecar
  participant Backup as Backup/Archive
  Caller->>Lock: Acquire SID authority
  Lock->>Sidecar: Read generation and exact digest
  Sidecar-->>Owner: Current durable revision
  Owner->>Owner: Compare observed revision
  alt Revision matches
    Owner->>Backup: Preserve shrinking/incomparable predecessor
    Owner->>Sidecar: Atomic publish with next generation
    Sidecar-->>Owner: New revision receipt
  else Revision differs
    Owner-->>Caller: Reject stale mutation and reload
  end
  Caller->>Lock: Release SID authority

Loading

_{Reviews (16): Last reviewed commit: "test(webui): target Session context fiel..." | Re-trigger Greptile}

[nesquena-hermes]

The regressions are resolved and the direction is right — but full-gate adversarial probes reproduced 5 new failure paths, including a Windows brick. SHIP ONLY WITH FIXES.

Strong recovery on the CI regressions — I re-ran all 243 tests across the nine files that were red on the first head and they all pass now, the full suite is green (14,570 passed), and I confirmed the "Align compression snapshot tests with CAS ownership" commit did not weaken its oracle: it still compares the exact persisted snapshot and continuation state. The fence direction (reject stale writers, preserve recoverable history before shrinking) is sound.

I then ran the full authoritative gate on this exact head (91746c1a5235) — full suite + adversarial production-path reproduction. The suite is green, but the adversarial pass reproduced five failures that the focused tests don't cover. Each was verified to pass at the base commit and fail at this head (or fail automatic recovery):

1. 🔴 BRICK — every native-Windows send fails before the provider runs

api/models.py:1916 (repeated in the workspace patch at :6036). Text-mode writes translate \n→\r\n, but the stored revision hashes the untranslated string. /api/chat/start saves at routes.py:21612, the worker immediately saves again at streaming.py:9955, and the second save raises StaleSessionGenerationError. A deterministic CRLF simulation failed at this head and succeeded at base. Fix: write both temp files with newline="\n", or hash the actual temp-file bytes before publication. Add a Windows second-save regression test.

2. 🟠 CORE — implicit workspace recovery rejects the user's turn

api/routes.py:23010 (callers at :22219, :22848). persist_recovered_workspace_binding() correctly returns a reloaded revision owner, but the helper discards it and the callers continue into chat-start with the invalidated alias → StaleSessionGenerationError in _prepare_chat_start_session_for_stream(). Fix: return and adopt the persisted Session at both callers before model resolution and _start_run().

3. 🟠 CORE — undo/truncate/clear permanently break on a malformed historical .json.bak

api/models.py:1847. The new code raises permanently instead of replacing an unusable backup. The same shrink succeeds at base and fails here. Fix: content-addressedly archive the raw malformed bytes, promote the valid live sidecar as the new primary backup, and fail only if preservation itself fails.

4. 🟠 CORE — first publication is non-atomic on filesystems without hard-link support

api/models.py:320. The O_EXCL fallback exposes the destination before copying bytes; a synchronized probe observed an existing zero-byte sidecar and a concurrent Session.load() raised JSONDecodeError. Fix: use an atomic no-replace rename primitive; if the platform provides none, fail closed rather than exposing a partial path.

5. 🟡 SILENT — a foreign-SID backup can authorize a shrink that recovery later refuses

api/models.py:1858. Dominance checks compare only messages, so a foreign backup containing the live rows is retained, but api/session_recovery.py:464 later rejects it — the probe saved the shrink and then failed automatic recovery (silent data loss). Fix: validate the embedded session_id before coverage checks; archive a foreign backup and replace it with the correctly-identified live generation, or fail closed.

Net

This is close and worth landing — the core CAS fence and the test alignment are correct. But #1 in particular means the fence would brick every Windows user's first turn, and #2/#3 add new turn-rejection / operation-break paths, so it can't ship as-is. Please address the five above (they're all edge-path hardening, not a redesign) and re-push — I'll re-run the full adversarial gate on the new head.

[ruizanthony]

Addressed all five adversarial findings on exact head bdb0925527461675b5ae5cfd1d8023511e33aff2:

1. Native-Windows text publication now pins newline="\n" for both revision-hashed writers; the simulated CRLF second-save probe passes.
2. Implicit workspace recovery carries the durable revision owner to both /api/chat/start and start_session_turn before model resolution/run start; both caller probes pass.
3. Malformed and non-object .json.bak bytes are archived content-addressedly before the valid live generation is promoted; preservation failure remains fail-closed.
4. The visible O_EXCL + copy fallback is removed. POSIX uses hard-link create-only, native Windows uses create-only rename, and unsupported POSIX filesystems fail closed without creating the destination.
5. Backup dominance now requires the embedded SID to match; foreign backups are archived and replaced with the correctly identified live snapshot, and automatic recovery succeeds.

Verification on this head: 848 passed; changed-file portfolio 420 passed, 2 skipped; zero Ruff diagnostics on changed lines; compile and git diff --check pass. Please rerun the full adversarial gate.

[ruizanthony]

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

[ruizanthony]

@nesquena-hermes please re-review final head 13686212. All five requested adversarial paths are remediated with RED/GREEN probes; the prior rebased head passed the full 24-check matrix, and this final rebase adds only upstream CHANGELOG.md outside the PR delta. The fresh matrix is running now. The REST review-request endpoint is not permitted from the fork author token, so this comment is the explicit re-review request.

[ruizanthony]

Remediated the three exact-SHA blockers on final head 0e22207c: (1) /api/chat/start now reauthorizes the workspace-recovered owner before model resolution or _start_run; (2) compression recovery is recomputed from that owner before clear/restore decisions; (3) content-addressed backup archives are streamed to a complete temporary file, digest-verified, fsynced, and atomically renamed without relying on hard links, while live sidecar first-publication remains strict create-only. Each adversarial probe was observed RED on the predecessor and GREEN on this head. Local final gates: 799 passed; changed-file plus adjacent recovery portfolio 439 passed, 2 skipped; Ruff/compile/diff-check clean. Please re-review exact head 0e22207c.

[ruizanthony]

@nesquena-hermes final exact head is now b042b465, rebased cleanly onto master 71584418. The upstream delta had no overlap with this PR. All 14 changed test files pass on the final tree (423 passed, 2 expected agent-dependent skips); Ruff reports no new diff findings, compilation and git diff --check are clean. The requested owner reauthorization, recovery recomputation, and POSIX-safe archival fixes are unchanged; please re-review b042b465.

[nesquena-hermes]

All 5 prior defects are verified FIXED — strong convergence. But the sidecar-CAS hardening exposed 2 NEW same-class stale-revision paths. SHIP ONLY WITH FIXES (R3).

Re-gated the full authoritative pass on head b042b465 (Codex adversarial + full suite). The recovery is real and I want to be clear about it first:

✅ All 5 findings from the last round are FIXED (verified head-fail→now-pass):

1. Windows CRLF brick — temp files now hash LF-consistent bytes; the chat-start→worker double-save no longer raises StaleSessionGenerationError.
2. Implicit workspace recovery — both callers now adopt the persisted revision owner before model-resolve / _start_run().
3. Malformed historical .json.bak — now archived (raw bytes preserved) + valid live sidecar promoted; the shrink succeeds.
4. First publication atomicity — now atomic no-replace (fail-closed where unsupported).
5. Foreign-SID backup — embedded session_id validated before coverage; archived/replaced or fail-closed.
   (48 sidecar/workspace + 33 recovery/streaming focused tests pass; full suite green.)

🔴 But two NEW defects appeared in the CAS hardening — both the same shape: a save-path advances the durable revision but hands back a cached owner still carrying the OLD revision, so the next save/send throws StaleSessionGenerationError (verified head-fail / base-pass):

A. api/models.py:~4357 — state.db lost-response recovery returns an unsaveable cached session

The locked.save() advances the durable revision, but the recovered fields (messages, context_messages, …) are copied onto the caller's cached session WITHOUT its new revision. Chat-start receives that alias and throws StaleSessionGenerationError on its next save.
Fix: when the cached alias owned the exact pre-save revision, transfer the saved owner's post-save revision onto it; otherwise replace/return the freshly-saved owner (don't hand back a stale alias).

B. api/session_discoverability.py:~450 (_clear_sidecar_cli_flag, --repair-safe --apply) — sidecar rewrite doesn't advance generation or invalidate the cache

The flag-repair rewrites the sidecar without bumping _sidecar_generation_v1 or invalidating the cached owner, so generation-based freshness then serves the stale cache and the next save/send throws StaleSessionGenerationError.
Fix: perform the repair under SID authority, increment _sidecar_generation_v1, publish canonical bytes atomically, and invalidate-or-adopt the cached owner.

Net

This is close and converging well — the core CAS fence and all five original edge-paths are correct now. Both remaining issues are the identical "post-save revision not propagated to the returned cached owner" pattern on two more paths (state.db self-heal + CLI flag-repair), so they're a focused fix, not a redesign. Address A and B and re-push — I'll re-run the full adversarial gate on the new head.

(Release-manager overnight re-gate. Data-safety write-path PR — the ship decision is the maintainer's regardless of gate outcome.)

[ruizanthony]

Exact final candidate: faf43df9e6cccbd3344d1368299571d6bb12a279 on current master dc3bf44e. The R3 post-save ownership defects are closed for both state.db lost-response self-heal and discoverability flag repair; the follow-up also serializes repair writers and hardens tombstone/delete/recovery durability and fail-closed publication. Official diff-derived gate: 497 passed, 2 expected agent-dependent skips; changed-line Ruff, Python compilation, and git diff --check are clean. @nesquena-hermes please re-run the adversarial gate on this exact SHA; prior CHANGES_REQUESTED reviews target obsolete heads.

[ruizanthony]

Final exact candidate after global-suite remediation: c333c5f1f9d211069522e5921a83ebcd1765e4ad on unchanged master dc3bf44e. It restores unsafe session_id validation wording and preserves the compression-recovery-required 409 contract when stale workspace recovery also fails.

Evidence: the exact RED tests and 51 adjacent recovery/validation tests pass; the 18-file modified portfolio passes (497 passed, 2 skipped); all 15 Python matrix shards and all required checks are green. The single failed terminal-error browser job is explicitly informational (continue-on-error: true) and passes locally end-to-end on this exact SHA; I am rerunning only that job. Please re-review and merge this exact SHA; the existing CHANGES_REQUESTED decision targets a superseded head.

[ruizanthony]

HOLD MERGE on current head c333c5f1f9d211069522e5921a83ebcd1765e4ad: independent integration review reproduced a P1 check→delete race in /api/sessions/cleanup*. Cleanup can classify an empty sidecar, a concurrent writer can publish a durable turn, then raw p.unlink() deletes that newer generation without a tombstone. A deterministic RED→GREEN fix is in progress; do not merge this head.

[ruizanthony]

Replacement candidate published: 02c48a4fc5714039fd8ec8cce09cb71902be9077. The cleanup check→delete P1 is fixed by reading a validated payload/revision and deleting under the same SID authority; revision is rechecked immediately before durable tombstone + artifact removal + fsync. Concurrent writers are blocked/fenced, partial failures are not counted or reclassified as index ghosts. Evidence: deterministic multiprocessing race probes, 117 passed + 1 skip focused, 500 passed + 2 skips modified portfolio, diff lint/compile/docs/diff-check clean. HOLD remains until exact-SHA integration re-review and CI complete.

[ruizanthony]

HOLD remains on 02c48a4fc5714039fd8ec8cce09cb71902be9077: final exact-SHA integration review found another P1 in hidden ephemeral cleanup. Two api/streaming.py paths still unlink only the primary sidecar outside the shared SID authority, leaving .bak/archives/replay-v10 artifacts and bypassing the tombstone fence. A RED→GREEN PR2 remediation is in progress; do not merge this head.

[ruizanthony]

Published candidate d3829448dc7e52ba14c2f48aecc68e1a222071ad (direct child of 02c48a4f). This closes the final exact-SHA integration-review blocker: hidden ephemeral /btw cancellation and normal-completion cleanup no longer unlink the primary sidecar directly. Both paths now use the SID authority and durable tombstone protocol, validate the canonical sidecar path, remove backup/archive/replay-v10 artifacts, invalidate cached aliases, and preserve the lock order agent lock -> SID authority. Added deterministic RED/GREEN probes for cancellation, normal completion, tombstone failure, noncanonical paths, artifacts, and no ERROR-log completion. Local verification: full changed-test portfolio 521 passed / 2 skipped (agent runtime unavailable), targeted owner/cancel/sidecar portfolios green, diff-scoped Ruff clean, compileall and diff-check clean. Please re-run the data-safety gate on this exact SHA; merge remains held until exact-SHA CI and review are green.

[ruizanthony]

@nesquena-hermes please re-review exact head d3829448dc7e52ba14c2f48aecc68e1a222071ad. The prior requested changes target superseded commits; all named stale-revision/repair findings plus the later cleanup races and hidden-ephemeral destructors are now covered by deterministic regression probes. Exact-head GitHub CI is fully green. The author account has READ permission on upstream and cannot dismiss the stale review or merge, so maintainer action is required after your gate.

[ruizanthony]

Additional HOLD on current head d3829448dc7e52ba14c2f48aecc68e1a222071ad: independent exact-head review reproduced a P1 where backup dominance based only on messages can discard unique durable non-transcript data (for example tool_calls) across shrink→grow→shrink. Remote CI is green but this head is not release-authorized. A full-snapshot dominance fix and regression probe are in progress.

[ruizanthony]

Status update on exact head ed2d93d1624724eb2a7563f3f64389fe8e57b3fd — requesting dismissal of the two outdated change requests and re-review.

1. The two outstanding CHANGES_REQUESTED reviews are confirmed obsolete.
They target 91746c1a and b042b465, which are not ancestors of the current head (20+ commits since). An independent exact-SHA re-review verified every finding from both reviews is fixed in ed2d93d1:

• Review on 91746c1a (5 defects): CRLF newline="\n" fence, revision-owner adoption by persist_recovered_workspace_binding callers, malformed .bak archiving with live promotion, atomic no-replace initial publication, embedded session_id validation before coverage checks — all present.
• Review on b042b465 (A/B): _sync_sidecar_from_state_db_if_newer transfers the post-save revision to the owner alias; _clear_sidecar_cli_flag operates under SID authority, increments _sidecar_generation_v1 and invalidates the cache — all present.

2. Independent exact-SHA review of ed2d93d1.
CI 24/24 SUCCESS on this SHA; 129 targeted tests passed on an exact git archive extraction. Three reproducible findings remain, and all three reproduce identically on base dc3bf44e — they are pre-existing defects, not regressions introduced by this PR:

• pre-compression snapshot can adopt a stale revision under a concurrent writer (data recoverable from .bak only);
• a torn _deleted_webui_sessions.json rewrite can drop prior tombstones (PR3 Harden offline repair for oversized session replays #7039 closes this on its side);
• multi-profile state.db fallback can silently read the active profile DB when the requested profile DB is absent.

The CAS/revision fence this PR adds is correct and does not worsen any of them.

Request: please dismiss the two outdated CHANGES_REQUESTED and re-review exact head ed2d93d1. If you prefer, I can address the three pre-existing defects in a dedicated follow-up PR so this one can ship.

[ruizanthony]

Rebased onto current master at exact head 28f6f794dee7dbe8524399e6e729bf38cc827601. The only conflict was the RFC (lifecycle-busy item kept + generation fence). Product code auto-merged; the sidecar CAS / revision fence is unchanged.

The outstanding CHANGES_REQUESTED reviews still target 91746c1a and b042b465, which are not ancestors of this head. The previously requested Windows CRLF, workspace-owner adoption, .bak archive, first-publication atomicity, foreign-SID backup, lost-response revision transfer, and repair-safe generation bump remain on this tree.

Local: 172 focused + 534 portfolio passed (2 agent-runtime skips); compileall and git diff --check clean. Please dismiss the stale reviews and re-gate this SHA.