feat(server): add LibsqlKv, the DenoKvLike coordination store over libSQL - #430
Conversation
…bSQL Implements scale-out phase 1 (#428, spec/scale-out.md §8): a standalone module presenting @dwk/deno-host's DenoKvLike seam over one dedicated libSQL database, so the lease/alarm/queue machinery can later span replicas in the proposed central storage mode. Order-preserving tuple key encoding (numeric parts sort numerically under BLOB memcmp), CAS via a store-wide monotonic versionstamp (never reissued after a sweep, unlike per-key counters), checks evaluated into a scratch row before any mutation inside one batch("write") transaction, lazy TTL with an explicit sweepExpired for poll ticks. Colocated tests cover codec round-trips and ordering properties, CAS interleavings, expiry semantics, and drive the real @dwk/deno-host acquireLease/setAlarm/QueueBroker code against LibsqlKv, including the two-replica claim race. Nothing composes it into the host yet — no behavior change for existing users. Closes #428 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgeyeFnkNceEwUxBKgCTrC
davidwkeith
left a comment
There was a problem hiding this comment.
Overview
LibsqlKv: @dwk/deno-host's DenoKvLike seam implemented over one libSQL table, for the future scale-out central coordination store (#428). Order-preserving tuple key codec (encodeKvKey/decodeKvKey), store-wide monotonic versionstamp, and pre-mutation-state CAS via a scratch-column guard evaluated before any mutation in one batch(..., "write") transaction. Standalone — nothing composes it into the host yet.
Blocking: CI is red (build-test job fails)
This is the one that matters most — the PR's own checklist claims the local CI gate ("pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm test") ran clean, but the actual build-test GitHub Actions job fails at the typecheck step:
packages/server typecheck: src/libsql-kv.ts(49,8): error TS2307: Cannot find module '@dwk/deno-host' or its corresponding type declarations.
packages/server typecheck: src/libsql-kv.test.ts(18,8): error TS2307: Cannot find module '@dwk/deno-host' or its corresponding type declarations.
...5 more implicit-any errors cascading from that unresolved import...
packages/server typecheck: src/libsql-kv.ts(258,5): error TS2322: Type 'Promise<void> | undefined' is not assignable to type 'Promise<void>'.
Two distinct, real bugs:
packages/server/tsconfig.json'spathsmap was never updated for the new@dwk/deno-hostdependency. That file (not touched by this PR at all, per the diff) exists specifically sopnpm typecheckworks beforepnpm buildruns (see its own header comment) — every other workspace dependency@dwk/serverhas is listed there, this one is the sole omission:(alphabetically between"@dwk/deno-host": ["../deno-host/src/index.ts"],
@dwk/cf-shimsand@dwk/dpop). This is almost certainly why the local run looked clean: if@dwk/deno-host'sdist/already existed on disk locally (e.g. from a priorpnpm build), module resolution would fall through to the built output and mask the missing source-path mapping — but CI'stypecheckstep runs beforebuild, so it hits the gap. Couldn't leave this inline since the file isn't part of this diff.libsql-kv.ts:258— a real, separate type error in#ready(). Left inline; it's the classic TS limitation wherethis.#schema ??= someCall()doesn't narrow the field for the very nextreturn this.#schema, because the RHS contains a call.
Both need fixing before this can merge — CONTRIBUTING.md's CI gate (lint → format → typecheck → build → test) is a hard requirement, not just a local nice-to-have.
Code quality / correctness (reviewed independent of the CI failure)
The design itself looks sound once it compiles:
- Key codec (
encodeKvKey/decodeKvKey): FDB-style element-tagged encoding with proper0x00→0x00 0xFFescaping, correct IEEE-754 sign-flip transform for total numeric order (verified the forward/reverse bit logic by hand), and theprefix ‖ 0xFFupper bound forlist()range scans is a valid strict bound since every tag byte is ≤0x05.NaNand out-of-range bigints correctly rejected. - CAS commit path (
_commit): checks are evaluated into thekv_meta.okscratch column before any mutation statement runs in the samebatch(..., "write")transaction, so a failed check genuinely makes every mutation a no-op — matches the documented pre-mutation-state semantics. The store-wide (not per-key) versionstamp is the right call for the reason given in the doc comment (a per-key counter really could let a stale CAS wrongly succeed after a delete+recreate). - All SQL is parameterized; the only string-built SQL fragments (
guard,conditions.join(...)) are static clause shapes, not user data — no injection surface. - No issues found in
sweepExpired,get,set,delete, or theLibsqlKvAtomicbuilder.
CONTRIBUTING.md conformance
- ✅ PR title
feat(server): add LibsqlKv, the DenoKvLike coordination store over libSQL— correct Conventional Commits scope/format. - ✅ PR body keeps
Summary/Packages affected/Checklistheadings verbatim. - ✅ Both unchecked checklist items (changeset, catalog/conformance) carry valid one-line reasons rather than being deleted —
@dwk/serveris genuinely private/unpublished and this adds no new worker. - ✅ Spec updated in the same PR:
spec/scale-out.md§8 gets an "Update (issue #428): implemented" callout describing the two documented divergences from the sketch. - ✅ Colocated tests present and substantive (
libsql-kv.test.ts): codec round-trip/rejection, an ordering property test across all five key-part types, CAS interleavings, expiry, and the real@dwk/deno-hostlease/alarm/queue-broker code driven againstLibsqlKvincluding a two-replica exactly-once delivery case. - ❌ CI gate is not actually green — see above. Checklist item is checked but the claim doesn't hold.
No other conformance issues found. Once the two typecheck errors are fixed, this looks like a solid, well-tested increment.
Generated by Claude Code
Adds the missing @dwk/deno-host paths entry to packages/server/ tsconfig.json — CI runs typecheck before build, so without the source mapping the import only resolved when a stale local dist/ happened to exist. Also restructures LibsqlKv's #ready() to return the ??= expression directly, sidestepping the field-narrowing limitation the unresolved import surfaced as TS2322. Verified by deleting deno-host's dist and re-running typecheck. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DgeyeFnkNceEwUxBKgCTrC
|
Both typecheck failures are fixed in aa76d84: the missing Your diagnosis of why the local gate looked clean was exactly right: Generated by Claude Code |
Summary
Implements scale-out phase 1 (#428, spec/scale-out.md §8):
LibsqlKv, a standalone@dwk/servermodule presenting@dwk/deno-host'sDenoKvLikeseam over one dedicated libSQL database — the coordination store the proposedcentralstorage mode's lease/alarm/queue machinery will run on. Nothing composes it into the host yet; no behavior change for existing users.Design highlights (per the spec, with two refinements now noted in an update callout in §8):
encodeKvKey/decodeKvKey, exported): element-tagged FDB-style encoding with escaped terminators and sign-flipped IEEE-754 numerics, so BLOB-memcmpprimary-key order equals key-array order — numeric due-index parts sort numerically, prefixes match per element (["a"]never matches["ab"]), andlist'sprefix/start/end(exclusive) ranges are plain indexed SQL range scans.kv_meta.seq) rather than the spec sketch's per-key counter — a per-key counter could reissue a stamp after a sweep deletes and a later write recreates the key, letting a stale lease release wrongly succeed; store-wide can't. Allsets in one atomic commit share one stamp (equality-only CAS makes both refinements invisible to the seam's consumers; documented in the module doc).atomic().check(...).set/delete(...).commit()becomes onebatch(..., "write")transaction that stores the check verdict in a scratch column first and guards every mutation on it — pre-mutation-state semantics exactly matching the seam's reference implementation.expireIn→expires_at; expired rows are immediately invisible toget/list/checks and physically removed bysweepExpired(), which the future host poll ticks will call.LibsqlClientLike— the module never constructs a connection or reads the environment, per the composition contract.Testing (spec §14 item 1, plus the first slice of item 2's multi-replica posture): codec round-trips and rejection cases; an ordering property test comparing
listscan order against a model comparator across all five key-part types; CAS interleavings (absence checks, stale stamps, racing claims, all-or-nothing multi-op); expiry (invisible-before-sweep, stamp-never-reissued, physical sweep); and the real@dwk/deno-hostacquireLease/releaseLease,setAlarm/getAlarm/deleteAlarm, andQueueBrokercode driven againstLibsqlKv— including release-after-expiry keeping the new holder, single-slot due-index replacement, ack/backoff/maxAttemptssemantics, and two brokers sharing the store delivering a message exactly once. Unit tests run against anode:sqlite-backedLibsqlClientLikefake (same posture as@dwk/deno-host's unpublished harness); no live service required.Also:
@dwk/deno-hostadded to@dwk/server's dependencies (type-only in production code today; the runtime machinery arrives with thecentralmode), exports added tosrc/index.ts, the §8 implementation-update callout inspec/scale-out.md, and the serverCLAUDE.mdfile-layout/deps notes.Closes #428.
Packages affected
@dwk/server(private, never published;@dwk/deno-hostadded as a dependency, its code unchanged)Checklist
spec/packages/and updated them ifbehaviour changed
src/*.test.ts)pnpm lint && pnpm format:check && pnpm typecheck && pnpm build && pnpm testpnpm changeset) — not applicable,@dwk/serveris private and never publishedcatalog.json/conformance/status.json— not applicable, no new workerGenerated by Claude Code