Exclusive record locks: table.lock(id) serialized across worker threads (Phase 0 of #483)

[kriszyp]

Adds process-wide, per-record RocksDB locks that serialize table.lock(id) across worker threads on one node while leaving ordinary writes ungated. This review-feedback update exports the public lock types and makes surviving hold versions monotonic only after a real commit—including static same-key writes—without adding a record-lock scan or key lookup to ordinary transactions that own no locks.

The update also closes transaction-integrity edges found during review: acquiring a scoped lock no longer aborts a native handle containing earlier writes, scoped-to-hold upgrade detaches the correct staged write through an intervening same-key operation, synchronous sweep failures abort and release sibling locks, failed/skipped writes do not advance the holder floor, and lock acquisition no longer mints a stale ImmediateTransaction timestamp. One unresolved concern needs human judgment: acquisition-time record versions are also transaction-log timestamps, so a delayed holder mutation may fall behind a replication cursor that has already passed that time.

For the human reviewer

1. Advisory scope: this phase serializes only lock() participants inside one RocksDB process; ordinary writes and other nodes remain ungated. Gating ordinary writes was rejected because it added a native lock round-trip to every write and introduced deadlock/livelock shapes. Reversing this after release would be a breaking behavior change.
2. Holder precedence and replication ordering: holder writes use the acquisition-time version so a later ordinary write wins under LWW. The alternative is commit-time precedence. Please explicitly rule on the consequence that a holder mutation committed later can carry a transaction-log timestamp older than a replica's cursor; a “no” requires separating replication ordering from record conflict ordering before merge.
3. Immediate scoped lifetime: a scoped lock outside transaction() survives sequential saves until unlock() or lease expiry. Requiring an explicit transaction or releasing after one save would reduce lifecycle states but break the tested sequential-save behavior.
4. Hold-mode API: { hold: true } returns a resource-attached handle that outlives its acquiring transaction and is explicitly released with unlock(). Restricting the API to transaction scope would be simpler but removes the intended long-lived use case and is easiest to decide before shipping.
5. Platform scope: Phase 0 is in-process and RocksDB-only; LMDB returns 501 and no protocol authorization surface is added. Engine parity, cluster coordination, and protocol policy are deferred extensions rather than hidden fallbacks.
6. Known lifecycle boundaries: overlapping un-awaited saves on one handle, scoped locks sharing an ImmediateTransaction with unrelated instance writes, and sourced/caching-table reload semantics remain unproved. The documented and tested use is awaited saves on local RocksDB tables; expanding those guarantees should be a deliberate follow-up rather than inferred from this phase.

Verification

• npm run build; npm run typecheck; npm run test:types; and npm run lint:required all pass. Full npm run lint reaches only 13 pre-existing warnings in untouched files.
• npx mocha unitTests/resources/recordLock.test.js: 71 passing, 1 pending after the transaction/version fixes. Targeted reruns also pass the different-key staged-write preservation, static same-key floor, and owning-link timestamp isolation cases. The static same-key regression fails on the pre-fix parent as expected.
• npm run test:unit:resources: 2068 passing, 29 pending. HARPER_STORAGE_ENGINE=lmdb npx mocha unitTests/resources/recordLock.test.js: 2 passing, 65 pending, confirming the LMDB 501 boundary.
• npm run test:integration -- "integrationTests/resources/record-lock-concurrency.test.ts": 3 passing across four workers. The full integration run completed with only six tests requiring an unavailable live Ollama service failing; Harper's local/in-process suites passed.
• npm run test:unit:main: 5286 passing, 196 pending, with five unrelated environment-sensitive failures (application-spawn cleanup, deploy socket observation, harness Git environment, missing RSA fixture, and the long-worktree Unix socket warning).
• Independent review ran five rounds on the fix sequence with Claude, Gemini, Cursor/Grok, and Harper storage/data adjudication. It closed the staged-write abort blocker, tail-only upgrade detachment, failed-floor advancement, default-path bookkeeping, and timestamp-minting findings; the replication-ordering decision above remains intentionally open for human review.

Complexity: complicated

_{Review-Coverage: authored=codex; ran=claude,gemini; declined=cursor-grok,cursor-composer,domain; rounds=5 @ 553cfb6}

_{Human-Review-Need: 3 @ 553cfb6}

[gemini-code-assist]

Code Review

This pull request implements Phase 0 of exclusive per-record locks across worker threads on a single node. It introduces a conditional LOCK write on the record itself as the single authority of ownership, accompanied by advisory wait/wake doorbell machinery, transaction-scoped lock management, lease expiration, and crash-recovery replay. The feedback suggests renaming a local variable in the integration tests to avoid shadowing the imported after hook from node:test.

[kriszyp]

Strong Phase 0 — the single-durable-authority model, the stage-nothing gate, and the conditional UNLOCK replay are exactly the right shape. One concern worth resolving before this merges, because it's cheapest to fix at the choke point this PR already owns:

The lock descriptor leaves the node on every data write to a locked record

The mixed-version story here covers the LOCK/UNLOCK control entries (LOCAL_ONLY, peer never sees action 9/10 — good). But holder writes and every preserved-bit rewrite are ordinary PUT/PATCH entries whose encoded record carries LOCKED (0x4) + lockVersion + lockExpiresAt in the metadata prefix, and DESIGN.md makes that deliberate: "the record itself replicates as usual." Replication and base copy forward encoded entries — the send path tests metadataFlags precisely so it never re-encodes. Two consequences:

1. Silent value corruption on pre-Phase-0 peers. DESIGN.md already states the mechanism — "an older Harper decodes a LOCKED record's value from the wrong offset" — but scopes it to local downgrade. The same bytes travel the wire: during a rolling upgrade (or to a v5→v4 bridge route, or a clone target), any replicated write to a locked record hands an older peer a layout it decodes 16 bytes short. That's the silent-corruption class we've spent months eliminating, and "don't downgrade this node" doesn't cover it.

2. Phantom locks on same-version peers. A peer applies a holder's write with the bit and live lockExpiresAt baked in, and its own local writers then hit the gateOnLock path — but the origin's UNLOCK never arrives (LOCAL_ONLY), so the peer stays write-gated for the full residual lease after the origin released. Base-copy targets inherit the same phantom state. That contradicts the PR's own scope statement ("one node, every worker thread") in the deployment mode that matters most.

Suggested fix: strip the descriptor from the audit/outbound encoding

Dual-encode locked-record data writes at the recordUpdater choke point: the store copy keeps LOCKED + descriptor; the audit-log copy (which is what replication and copy read) drops the bit and both fields. Costs one extra encode only on writes to locked records — same cost class as the lock transitions themselves.

Notably, this is consistent with the crash-recovery stance this PR already takes: "A LOCK is not replayed: its holder died with the process." Replay restoring the stripped (unlocked) form is the same semantics, lease-bounded, and the conditional _writeUnlockReplay doesn't depend on the descriptor being present in data entries. Worth double-checking the resequencing/out-of-order history walk paths that reload entries from the audit log, since store and log encodings would no longer be byte-identical for locked records.

Phase 1 then replaces the strip with capability-gated propagation (the recordLocks capability in the harper-pro protocol-capability registry, harper-pro#440), rather than introducing lock state to the wire accidentally now and having to define its semantics retroactively.

If the dual encoding is judged too invasive for this PR, the minimum bar is widening the documented limitation from "no downgrade" to "no replication/clone of lock-bearing tables in a mixed-version cluster, and expect residual-lease write-gating on same-version peers" plus a release-note warning — but the strip closes both holes for real and keeps the PR's central invariant ("what makes a caller the holder is that its conditional write committed — nothing else, and no one else sees it") true on the wire, not just on the node.

— Claude (Fable 5), reviewing on behalf of Kris

[claude]

Reviewed; no blockers found.