Verdict: Real — High (confidence: high) — the complete F2 fix (fencing)
The original review's mechanism is wrong, but the lost-update conclusion is real via a different path. This issue adds the fencing token that closes it for good — and the same primitive closes #136 (F7) and #139 (F9b)'s collision class.
Corrected mechanism (important)
- The review claimed
appendFile reads in tx1 / writes in tx2 and that the advisory lock doesn't serialize the composite. False in the exec path: #withTx and #withBareTx both route to the same #scriptTx, so getBlob (sql-fs.ts:806) and writeFileComposite (:819) run in one advisory-locked transaction. The two-tx interleave cannot occur, and the review's fix-(3) ("make appendFile single-transaction") is a no-op against a real bug.
- The real lost update:
appendFile captures its base from the in-memory pathCache (existing.contentSha256, sql-fs.ts:805) before any lock, and getBlob is content-addressed (reads by hash, postgres.ts:616). A zombie writer A (lease lapsed) reads its stale base even after a live writer B (other replica) committed a newer dirent, then replaces the dirent with new-inode(stale-base + append), silently erasing B's commit. The advisory lock can't prevent it because the base was captured in memory and the read is by content hash.
- Reachability (narrow): A's Redis writer flag must expire (PX
leaseMs, default 60 s) before A opens its script-tx (advisory lock acquired lazily on first write via #openScriptTx→setSandboxContextWithLock, postgres.ts:265). Window = A holds the distributed lock → runs an event-loop-blocking CPU/WASM prefix (heartbeat setTimeout can't fire, lease silently expires) → has not yet issued its first write.
Fix to ship (Layer 2 — the only complete fix)
- Add
version BIGINT NOT NULL DEFAULT 0 to the sandboxes table (new migration, per dialect).
- Pin the expected epoch at script-scope open (derive from the version read under the lease).
- Add
UPDATE sandboxes SET version = version + 1 WHERE id = $s AND version = $expected inside the advisory-locked ctx CTE of the composite writes. Zero rows updated ⇒ abort the tx ⇒ the zombie's COMMIT fails in Postgres, which doesn't share the zombie's delusions.
- Must respect
set_config('app.sandbox_id')/RLS and be wrapped in runTrustedDbAsync (defense-in-depth).
Acceptance criteria
- Integration test: two writers, force lease expiry on A before its first write, B commits, A's COMMIT must fail (epoch mismatch); B's append survives.
- RLS / SET LOCAL / transaction-pooling unaffected; defense-in-depth wrapping preserved.
- Migration applies cleanly on boot (
src/api/migrations.ts) for all configured dialects.
Parallelization
✅ Start now — no hard blocker (pairs with #130 L1). Design the sandboxes.version epoch so it is reused by #136 (tombstone/sandbox-gone) and #139 (collision-class closure) — see "Cluster C" in the verification doc.
References: verified analysis thoughts/shared/research/2026-06-13_distributed-flaws-verification.md · original review thoughts/shared/research/2026-06-10_distributed-design-review.md
Verdict: Real — High (confidence: high) — the complete F2 fix (fencing)
The original review's mechanism is wrong, but the lost-update conclusion is real via a different path. This issue adds the fencing token that closes it for good — and the same primitive closes #136 (F7) and #139 (F9b)'s collision class.
Corrected mechanism (important)
appendFilereads in tx1 / writes in tx2 and that the advisory lock doesn't serialize the composite. False in the exec path:#withTxand#withBareTxboth route to the same#scriptTx, sogetBlob(sql-fs.ts:806) andwriteFileComposite(:819) run in one advisory-locked transaction. The two-tx interleave cannot occur, and the review's fix-(3) ("make appendFile single-transaction") is a no-op against a real bug.appendFilecaptures its base from the in-memory pathCache (existing.contentSha256,sql-fs.ts:805) before any lock, andgetBlobis content-addressed (reads by hash,postgres.ts:616). A zombie writer A (lease lapsed) reads its stale base even after a live writer B (other replica) committed a newer dirent, then replaces the dirent withnew-inode(stale-base + append), silently erasing B's commit. The advisory lock can't prevent it because the base was captured in memory and the read is by content hash.leaseMs, default 60 s) before A opens its script-tx (advisory lock acquired lazily on first write via#openScriptTx→setSandboxContextWithLock,postgres.ts:265). Window = A holds the distributed lock → runs an event-loop-blocking CPU/WASM prefix (heartbeatsetTimeoutcan't fire, lease silently expires) → has not yet issued its first write.Fix to ship (Layer 2 — the only complete fix)
version BIGINT NOT NULL DEFAULT 0to thesandboxestable (new migration, per dialect).UPDATE sandboxes SET version = version + 1 WHERE id = $s AND version = $expectedinside the advisory-lockedctxCTE of the composite writes. Zero rows updated ⇒ abort the tx ⇒ the zombie's COMMIT fails in Postgres, which doesn't share the zombie's delusions.set_config('app.sandbox_id')/RLS and be wrapped inrunTrustedDbAsync(defense-in-depth).Acceptance criteria
src/api/migrations.ts) for all configured dialects.Parallelization
✅ Start now — no hard blocker (pairs with #130 L1). Design the
sandboxes.versionepoch so it is reused by #136 (tombstone/sandbox-gone) and #139 (collision-class closure) — see "Cluster C" in the verification doc.References: verified analysis
thoughts/shared/research/2026-06-13_distributed-flaws-verification.md· original reviewthoughts/shared/research/2026-06-10_distributed-design-review.md