fix(session-persistence): prevent stale Artifact session saves

[ewen-poch]

Problem

Renderer store autosaves used a local queue, while Artifact finalization called the Session IPC save path directly. An older queued snapshot could therefore arrive after the latest Artifact-bound Session, replace the durable JSON, and make provenance capture fail with Artifact-owning Message is outside its bound Branch.

Proposed change

• Route store saves, manifest saves, and Artifact finalization saves through one ordered renderer persistence seam.
• Validate every finalized Artifact Message binding before replacing durable Session JSON, including Versions that already have an immutable Message Snapshot.
• Add regression coverage for renderer ordering, the Artifact event path, pre-write rejection, and snapshot-before/snapshot-after binding validation.

Scope and non-goals

• No schema migration, UI change, or Artifact format change.
• This does not introduce a general Session revision protocol; the fail-closed main-process guard is scoped to finalized Artifact Message bindings.
• This does not rewrite historical Session files that are already corrupt.

Acceptance criteria and validation

All listed checks ran after the last material edit.

• Store autosaves cannot overtake the latest explicit Artifact save -> npm test -- src/renderer/src/lib/session-persistence/session-persistence.test.ts -> passed in the 17-test file.
• Artifact finalization shares the ordered save path -> npm test -- src/renderer/src/lib/acp/workspace-events.test.ts -> passed in the 58-test file.
• A stale finalized binding cannot overwrite durable Session JSON -> npm test -- src/main/session-persistence/coordinator.test.ts -> passed in the 57-test file.
• Stale graphs are rejected before and after Message Snapshot creation -> npm test -- src/main/artifacts/provenance-message-snapshot.test.ts -> passed in the 8-test file.
• Combined targeted suite -> 4 files, 140 tests passed.
• npm run typecheck -> passed.
• npm run lint -> passed with 0 errors; 24 existing warnings are outside the changed files.
• npm test -> 544 files and 8140 tests passed; 11 files and 139 tests skipped by project configuration.

One full-suite run hit unrelated timing failures in ProjectFilesView and notebook force-stop tests. Both files then passed independently, 209/209, and the subsequent full-suite rerun passed.

Uncovered risk: there is no real Electron end-to-end stress test for multi-window writers, and the main guard adds a bounded finalized-Version lookup to Session saves.

Review focus

• Whether all renderer Session write paths share the intended queue.
• Whether finalized Artifact binding validation is correctly placed before JSON replacement.
• The query scope and fail-closed behavior for Sessions with long Artifact histories.

[github-actions]

Codex Review

Verdict: needs changes

[P1] Stale session saves are rejected without any conflict recovery path

src/main/session-persistence/coordinator.ts:384

Impact: When another lifecycle client updates the same session first (for example by finalizing an artifact or changing branches), this new validation rejects the lagging client's next save before any durable write. The renderer persistence bridge only logs save failures and then accepts the incoming sessionUpdated snapshot as authoritative, so local edits like a rename/pin made from the stale client are silently discarded and disappear on reload.

Recommendation: Return a typed conflict for this validation failure and handle it in the renderer by preserving dirty state and rebasing/retrying on top of the latest durable session, or merge safe non-graph fields against the current durable session before rejecting the save.

Summary: The new stale-graph guard closes the overwrite race, but it also introduces a blocking conflict path that the renderer does not recover from, so supported multi-client/session-update workflows can now lose unrelated user edits silently.

[ewen-poch]

Addressed the stale-save recovery concern in 2282cf5.

• The renderer now identifies only the safe non-graph fields actually changed by each autosave (title, pin state, and session preference fields) and includes that explicit intent with the save.
• Finalized Artifact binding mismatches now use a typed conflict. On that conflict, main reloads the latest durable Session, rebases only those explicitly changed fields onto the authoritative graph, revalidates the result, and then saves it. Generic validation failures and conflicts without safe-field intent still fail closed.
• Added renderer, coordinator, and IPC coverage for the rebase path. Type checks and targeted tests pass; the full suite passes 8,481 tests with one unrelated Prisma runtime-DDL baseline failure that reproduces on current main.

[github-actions]

Codex Review

Verdict: needs changes

[P1] Discarded rebased session graph leaves renderer stale

src/renderer/src/lib/session-persistence/session-persistence.ts:271

Impact: When the coordinator rebases a stale graph onto the authoritative session, applyDurableSessionProjection only detects upload-identity changes. With no upload delta it leaves the stale graph in memory, so later artifact saves can submit the same invalid binding and fail again, while the UI diverges from durable state.

Recommendation: Make durable projection distinguish a full conflict rebase from upload-only upgrades and replace the session graph and persisted fields when the saved source is still current; retain the existing merge-only behavior when newer renderer state exists.

Summary: Static inspection found one blocking renderer/main integration defect in the new conflict-rebase flow.

[ewen-poch]

Addressed the renderer graph refresh concern in fc22b10.

Autosaves that carry safe-field rebase intent now apply the returned durable Session as a full persisted projection only when the submitted source object is still current. This refreshes the renderer to the authoritative graph after a conflict rebase. If a newer renderer mutation has already replaced that source, the existing upload-identity-only merge remains in effect so the older response cannot roll back newer live state. Added regression coverage for both the authoritative replacement and newer-state fallback paths.

[github-actions]

Codex Review

Verdict: needs changes

[P1] Pre-save provenance lookup blocks durable session writes on database errors

src/main/session-persistence/coordinator.ts:463

Impact: Every session save now requires a successful artifact-database query before writing session JSON. A transient database failure prevents ordinary conversation changes from becoming durable, unlike the existing post-save provenance/indexing flow.

Recommendation: Preserve the authoritative JSON-first durability guarantee by separating conflict validation from transient provenance failures, or introduce an atomic authority mechanism that can validate bindings without blocking the session file commit.

[P2] Forced write retries discard conflict-rebase intent

src/renderer/src/lib/session-persistence/session-persistence.ts:262

Impact: After a save fails, the saver has already advanced previousSessions to the attempted snapshot. retryWrites then sees no changed safe fields and sends no conflictRebaseFields, so a stale-graph retry cannot rebase and can remain rejected until another preference mutation occurs.

Recommendation: Retain the calculated rebase fields with each failed write target, or compare retries against the last acknowledged durable snapshot rather than the last attempted snapshot.

Summary: Static inspection found two persistence regressions affecting durable session saves and conflict recovery.

[ewen-poch]

Addressed both persistence concerns in b87b0bf.

• Pre-save binding validation now distinguishes FinalizedArtifactBindingConflictError from transient provenance lookup failures. Confirmed binding conflicts still fail closed/rebase as before; transient database failures are logged and no longer prevent the authoritative Session JSON write, preserving the existing JSON-first durability path.
• Failed session targets now retain the safe-field rebase intent captured by the attempted snapshot. Explicit retries feed that intent back into the forced save instead of recalculating from the already-advanced attempted baseline. The retained intent is accumulated across consecutive failures and cleared only after success or target removal.

Added regression coverage for transient validation availability and forced retry intent.

[github-actions]

Codex Review

Verdict: needs changes

[P1] Avoid loading full artifact versions on every session save

src/main/artifacts/provenance-message-snapshot.ts:141

Impact: The renderer autosaves on frequent store updates, including streamed message chunks. Each save now performs an unbounded findMany that fetches every finalized artifact version, including large evidence fields, before the session can be written. Artifact-rich sessions can build a serialized persistence backlog and leave recent conversation state unwritten for extended periods.

Recommendation: Validate only when graph bindings could have changed, and use a narrow Prisma select containing only the binding fields required by resolveScopePath.

Summary: Static inspection found one performance and durability regression in the new provenance validation path.

[ewen-poch]

Addressed the binding-validation hot path in 026a0e1.

• The coordinator now fingerprints only the graph topology used to resolve finalized Artifact bindings. Streaming content/status updates and safe metadata changes reuse a confirmed fingerprint, while topology changes still validate before the Session JSON write.
• Artifact/session mutations and catalog reloads invalidate the fingerprint, so newly finalized bindings and externally refreshed authority are revalidated. Transient validation failures are not cached.
• Both validation and pending snapshot capture now select only rootFrameId, agentFrameId, messageBranchId, and messageId from Prisma instead of loading large evidence/content fields.

Added regression coverage for streaming saves, topology and Artifact-binding invalidation, transient lookup retries, and the narrow Prisma select.

[github-actions]

Codex Review

Verdict: needs changes

[P2] Rebase specialist binding writes during artifact conflicts

src/main/session-persistence/coordinator.ts:520

Impact: A concurrent renderer save or artifact finalization can cause the main-process specialist switch persistence to hit a finalized-binding conflict and fail because it supplies no rebase fields.

Recommendation: Pass conflictRebaseFields: ['specialistId'] from the specialist persistence caller, or add a coordinator helper that records this safe-field intent.

Summary: Static inspection found one integration defect. Branch and title prechecks are valid; no tests, lint, typecheck, builds, or project executables were run.

[ewen-poch]

Addressed the specialist binding conflict path in 60b5e66.

• Added a named saveSessionSpecialistBinding coordinator entry point that always delegates with conflictRebaseFields: ["specialistId"].
• Routed the main-process specialist persistence caller through that entry point, so a finalized Artifact binding conflict reloads the authoritative Session graph and rebases only the requested specialist UUID, including clearing it with undefined.
• Added regression coverage proving the specialist change survives while the authoritative message graph wins.

Focused tests, type checking, formatting, lint, and an independent Standards/Spec review passed. The full suite remains at one previously reproduced PermissionGrant runtime-DDL baseline failure, with 8,485 tests passing.

[github-actions]

Codex Review

Verdict: needs changes

[P2] Advance the revision when rebasing safe fields

src/main/session-persistence/coordinator.ts:186

Impact: If the authoritative session is newer than the submitted snapshot, rebasing a field keeps its existing updatedAt. Other renderer windows treat the lifecycle update as equal-revision and merge only upload identities, so rebased title, specialist, or preference changes remain stale until reload.

Recommendation: Ensure a rebased safe-field change receives a strictly newer revision, or broadcast an explicit field update that equal-revision consumers apply.

Summary: One cross-window persistence propagation defect blocks merging.

[ewen-poch]

Addressed cross-window safe-field propagation in 32f8807.

rebaseSafeSessionFields now assigns Math.max(authoritative.updatedAt, submitted.updatedAt) + 1, so every conflict-rebased title, specialist binding, pin, or preference update has a revision strictly newer than both competing snapshots. Other lifecycle clients therefore take the full newer-session path instead of the equal-revision upload-identity-only merge.

The regression test now asserts the rebased revision is greater than both inputs. Coordinator, renderer session-store, and lifecycle-sync tests pass, as do type checking, formatting, lint, and an independent Standards/Spec review. The full suite still has only the previously reproduced PermissionGrant runtime-DDL baseline failure, with 8,485 tests passing.

[github-actions]

Codex Review

Verdict: mergeable

No actionable findings.

Summary: Static inspection found no concrete merge-blocking defects introduced by this pull request. Branch and title prechecks are valid.