Prevent deleted MCP sessions from being resurrected

[kylebernhardy]

Fixes #1368 by making MCP session deletion terminal at the storage layer. Post-creation saves use an internal conditional patch, and the commit path rechecks existence on every optimistic retry. A request that loaded a session before a concurrent DELETE can no longer recreate its row.

DELETE now uses bounded, isolated retries: it returns 204 only after observing the row absent, and fails instead of hanging or reporting success under sustained contention. Live subscriptions and pending server requests are torn down only after storage deletion succeeds.

This targets main only; a v5.2 backport is not required. The MCP session table is explicitly non-replicated, and unsafe replicated or source-backed conditional writes are rejected.

For the human reviewer

1. Physical deletion plus a replay-persisted conditional patch was chosen over tombstones. Reversing this after release would require a storage-format compatibility decision.
2. Conditional session saves use dedicated transactions so a long-lived SSE request cannot pin them; they are intentionally not atomic with an ambient caller transaction.
3. DELETE retries three times in fresh transactions and returns 500 if newer writes keep winning. The retry budget is intentionally bounded and easy to revise.
4. This PR makes persistent-row deletion terminal, but it does not make DELETE a cross-worker cancellation barrier for already-admitted POST or GET/SSE activity. Strengthening that promise requires separate ITC work.
5. Conditional writes require a local, source-free session table rather than defining convergent replicated deletion semantics. Changing that later would require coordinated harper-pro replication behavior.

Verification

• npm run build passed.
• Touched-file Oxlint, Prettier, and git diff --check passed. Repository-wide lint remains blocked by unrelated warnings outside this PR.
• Focused RocksDB storage, audit, replay, and MCP suites passed: 203 passing, 1 pending.
• Real-table MCP race tests cover a late save, a stale load held across DELETE, a delete that loses once and retries, and bounded failure under sustained contention.
• The conditional-patch suites also passed under LMDB: 14 passing.
• Crash replay coverage proves a conditional patch does not recreate a missing row and independently verifies LOCAL_ONLY metadata.
• Transport coverage proves successful DELETE clears live subscriptions while failed deletion preserves them.
• The full resource suite was attempted but entered a pre-existing repeated-unhandled-rejection failure in unitTests/resources/caching.test.js; CI is authoritative for the complete gate.

Comment generated by kAIle (GPT-5.6)

Complexity: complicated

_{Review-Coverage: authored=codex; ran=claude; adjudicated=domain; blocked=gemini(auth); declined=cursor-grok,cursor-composer; rounds=17 @ ba12c7c}

_{Human-Review-Need: 3 (decisions: node-local-session-storage, conditional-patch-layer, bounded-delete-retries, late-save-semantics) @ ba12c7c}

[github-actions]

Release cherry-pick v5.2: cancelled

Cherry-pick branch cherry-pick/v5.2/pr-2446 was deleted — this PR no longer targets v5.2 (milestone is now <none>).

[gemini-code-assist]

Code Review

This pull request implements durable terminal states for client session deletion using short-lived tombstones and field-level patches, preventing concurrent updates from resurrecting deleted sessions. The changes transition saveSession to use partial patches instead of full record replacements, update the session loading logic to reject terminated or incomplete rows, and add comprehensive integration and unit tests for patch conflict merging and tombstone behavior. The review feedback suggests avoiding in-place mutations of the session object (such as logLevel and subscriptions) to prevent potential TypeErrors when the session object is frozen.

[claude]

Reviewed; no blockers found.

[kriszyp]

This needs to be a 5.2 patch?
_{🤖 Reviewed with Claude}

[kylebernhardy]

@kriszyp Yes, this targets v5.2. I implemented your conditional-patch suggestion in this PR: MCP deletion is a real delete, late saves use a commit-time ifExists patch, and crash replay preserves the condition. Audited/unaudited retry coverage and the deterministic MCP deletion race are included. All of your threads are addressed and resolved. Would you re-review when you have a chance?

Comment generated by kAIle (GPT-5.6)

[kriszyp]

To be clear, the question wasn't seeking help in reading the Milestone field (I can read it!), it was about justification for the patch. I dunno if you need an upgrade to Astra to answer (sorry, just here to troll codex at this point).
_{🤖 Reviewed with Claude}

[kylebernhardy]

The v5.2 justification is that this fixes an MCP session-deletion correctness bug in the release line: an in-flight request could rewrite a deleted session row after DELETE. The final shape is limited to the non-replicated system.mcp_session table and leaves public Table.patch behavior unchanged. I also addressed the inline concerns and added coverage for replay, delete contention, and successful/failed teardown. Please take another look when CI is green.

Comment generated by kAIle (GPT-5.6)

[kriszyp]

Do you think there are users/customers impacted by this?

[kylebernhardy]

No specific impacted users or customers are known. I agree that does not justify a v5.2 backport, so I removed the milestone and updated the PR to target main only.

Comment generated by kAIle (GPT-5.6)