Skip to content

Non-atomic snapshot+mutate can permanently brick a record (orphan version row) #112

Description

@cuibonobo

Problem

Stack orchestrates every versioned mutation as two separate, non-transactional adapter calls: saveVersion(existing) then the mutation (patchContent/associate/setPermissions/deleteRecord/commitMigration). E.g. Stack.update() at stack.ts:887-889:

await this.saveVersion(existing);                    // writes snapshot for version N
return this.adapter.patchContent(id, content, ...);  // bumps N → N+1

saveVersion INSERTs into versions with a UNIQUE (record_id, version) PK, and a collision throws StackConflictError loudly and deliberately — correct, load-bearing behavior for the concurrent-writer case (record-logic.ts:430-457, testing.ts:244-253).

But if the process is interrupted between the two calls (crash, kill, unhandled rejection, power loss on a local adapter), the snapshot for version N is committed while the record is still at version N. Now every future mutation first snapshots version N → collides → StackConflictError forever. The record is permanently frozen: readable, but un-updatable, un-deletable (soft), un-restorable — escapable only by hard delete, which destroys it. There is no API to inspect or clear an orphan versions row.

migrateAll() interrupted between its saveVersion and commitMigration has the same failure shape. This is primarily a local-adapter problem: over the API adapter saveVersion is a no-op and the server snapshots atomically inside the mutation endpoint — which is exactly the shape the fix should bring to local adapters.

Fix — the sound one is architectural, not a patch

Move snapshotting into the adapter's mutation transaction, so prior-state snapshot and mutation commit as one unit — aligning local adapters with what the server/API contract already does ("the server snapshots automatically on every mutating endpoint"). The SQLite layer already has the transaction idiom (deleteUnreferencedAttachmentRecords, record-logic.ts:361-394). Concretely: each mutating adapter method snapshots the pre-image itself within its own BEGIN/COMMIT, and Stack stops issuing a separate saveVersion. This is the "#61's atomic snapshot+mutate land in the adapter" shape the original design-review keystone note anticipated but that implementation didn't land.

Do NOT take the tempting cheap mitigation ("on a (record_id, version) collision, if the existing snapshot is deep-equal to the one being written, treat it as success"). It is unsound: two concurrent last-writer-wins writers read the same version N and snapshot the identical prior state — the collision is precisely how the losing writer is rejected before its mutation partially applies. Swallowing an equal collision would let both mutations proceed, reintroducing the corruption the loud collision exists to prevent. The atomic-in-adapter fix is the real one.

Scope / coordination

  • Touches the StackRecordAdapter contract (snapshot responsibility moves from Stack into the adapter) and all three record adapters + MemoryAdapter. Best sequenced with any other adapter-contract work (cf. B1's snapshot normalization).
  • The API adapter needs no change (already server-atomic); this is about making local adapters match that guarantee.

Tests

  • Simulate interruption: write an orphan versions row at the record's current version, then assert a subsequent mutation still succeeds (no permanent brick)
  • Concurrent last-writer-wins race still rejects the losing writer with no partial apply (regression guard against the unsound mitigation)
  • migrateAll interrupted mid-record leaves the record mutable

Refs

#61 (full-state snapshots), #48 (optimistic concurrency / the loud collision), #46 (adapter/transaction work this belongs with). From docs/design-assessment-2026-07.md §B2 (PR #105).

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions