restAdapter — the reference SyncAdapter speaks wire protocol v1 (DAN-780) - #24
Conversation
…col v1 (DAN-780) The client half of the HTTP contract: §7 bodies to the wire-protocol path constants, every status decision routed through classifyWireStatus, 409 throwing the coordinator's own SchemaVersionError so D12 suspension fires on the exact detection seam, loud validation of required response fields with §3 pass-through of unknown ones, and no subscribe/comparator/predicate surface by design (rev c C2-1, DAN-676). Proven, not definitionally, conformant: runSyncAdapterContract against a minimally §13-conformant stub server with every hook supplied (zero skipped blocks), wire-mapping tests for the layer the kit cannot see, and an end-to-end enableSync ↔ restAdapter ↔ stub integration — the first time Stage 3 speaks HTTP. Seven mechanisms watched to fail. Suite 566 → 619. Unexported (ADR-022 lines 1-2); protocol §14 updated — v1 stays DRAFT, the freeze trigger is the first export/publish of the sync surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…fresh-context review A set change's data is now required to be an entity object (null/absent data would pass the coordinator's falsy write-skip while the version still got stamped — permanent silent divergence on the re-delivered payload); confirmedMutations values must be numbers (a null seq passes '!== undefined' and pins the overlay forever — the silent half of D1); a reset's cursor must be string-or-null (the coordinator would adopt anything via '?? null'); and caller headers are lowercased before merging so a canonical Content-Type overrides the default instead of duplicating it into a joined header value. All six new tests watched red against the unfixed adapter first. Also tightened the 'zero skipped blocks' claim: 12 of 13 obligations exercised, the live-channel one satisfied by the deliberate absence of subscribe(). Suite 619 → 625. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d 1's pull-side fixes (landing gauntlet)
toPushResult now validates what the coordinator trusts: a verdict's version
must be string|number when present (version:null would be stamped into the
version map, where every later HLC-string token classifies lexicographically
older than 'null' — server truth silently dropped forever); transform data
must be an entity object (a truthy string reaches store.replace and poisons
the reject re-base shadow); remappedId must be a non-empty string ('' is
non-nullish so it becomes the target id, yet falsy so the remap block skips —
the correction lands under id ""). Each corruption path was executed through
the real coordinator by the gauntlet reviewer before fixing.
Also: a non-JSON 200 body now throws the adapter's loud malformed error
instead of a bare SyntaxError, and the protocol doc's updated: date is
corrected to today. All five new tests watched red first. Suite 625 → 630.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…col client half exercised (ADR-023) Land-step-7 decision-loop closure: the field is what makes 'grep Implementation:' measure the conceived-to-shipped ratio. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
What
restAdapter(options)— the first realSyncAdapter: the client half ofdocs/protocol/sync-wire-protocol-v1.md.POSTs the §7 bodies to thewire-protocol.tspath constants (cursor in the body, never the URL — §2/C5), routes every status decision throughclassifyWireStatus(§8), throws the coordinator's ownSchemaVersionErroron 409 so D12 outbox suspension fires on the exact seam the coordinator detects, validates required response fields loudly on both channels, and passes unknown fields through untouched (§3, both halves). Deliberately absent:subscribe()(v1 defines no live channel), a comparator (server-authoritative reference), and any predicate surface (rev c C2-1 / DAN-676 — held closed at compile time by@ts-expect-errorprobes that failtypecheckthe day awhere/filter/queryslot appears).Unexported from
index.ts(ADR-022 lines 1-2). Protocol §14 updated: v1 stays DRAFT — the freeze trigger is the first export/publish of the sync surface, not this in-tree landing.Discretion calls (as granted by the packet)
Responseobjects with genuine JSON bodies and real status codes — chosen overnode:httpfor determinism; nothing under test lives belowfetch, and serialization + status classification are fully exercised.{ baseUrl, headers? (static record or async callback — supports token refresh; auth never enters the protocol per §4), fetch?, schemaVersion? }. Caller header keys are lowercased before merging so a canonicalContent-Typeoverrides the default instead of duplicating into a joined header value.Gate evidence
CI=true pnpm -r test: 630 passed, 2 skipped at root (pre-existing sqlite-wasm pair) + 29 inpackages/mcp. Suite 566 → 630.pnpm -r typecheck/build/lint: green.rest-adapter.spec.ts: 64 named passing tests, zero skips — every kit hook supplied (12 of 13 adapter obligations exercised; the live-channel one is satisfied by the deliberate absence ofsubscribe()).enableSync ↔ restAdapter ↔ stub v1 server: local write pushes over HTTP, pull'sconfirmedMutationsdrops the overlay, server-seeded change applies undersync-pull, nothing echoes.Watched to fail (red-then-green, per house law)
Errorinstead ofSchemaVersionError: D12-seam test red — only this file's test caught it; the kit's D12 block accepts any throw.permanent: five transient-status tests red.confirmedMutations: integration overlay-drop red — the D1 behaviour proven end-to-end.where?: stringadded to the options type:pnpm typecheckfails via the now-unused expect-error probes.Plus 11 review-round tests (6 in round 1, 5 in round 2), each watched red against the unfixed adapter before its fix.
Review verdict
Two adversarial fresh-context rounds, both with findings, all fixed in-branch:
c51758f).version: nullstamped into the version map (silent permanent divergence), truthy non-object transformdatacorrupting the store + reject shadow,remappedId: ""landing corrections under the empty id (f152b05).Flagged, not fixed (out of scope): §7's reset example sends
"cursor": nullwhile the frozen type sayscursor?: string— reconcile before v1 freezes; and the kit's live-channel test early-returns inside itsit(pre-existing, DO-NOT-TOUCH here).Linear: DAN-780
🤖 Generated with Claude Code