Repository navigation
feat(client)!: deliver resource state events from one stream connector - #53
Conversation
Add proposal 0009, which makes the browser connectors deliver protocol events to a callback, adds a read-only ResourceStore for lists and viewers, and keeps LiveResourceStore as the optional editing layer. It opens with an overview of why live watches and three-way merge belong together. Point proposal 0006 at it and split 0006's remaining work into editor and stream tracks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tighten proposal 0009's connector contract (state events, lifecycle through state and subscribe, consumer exceptions, protocol header diagnostics) and its three-change delivery order. Add proposal 0010 for the separate Go API cleanup, and point proposal 0006 at both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Implement change 1 of proposal 0009. connectResourceStream is now the
only public connector: the managed fetch connector, taking a synchronous
callback for ResourceStateEvent instead of a LiveResourceStore. The
single-connection fetch loop is internal, and transport no longer
imports the editor store.
- The connector detects reset and synced from protocol events itself,
applies synced through the callback before publishing live, and
reports lifecycle only through state and subscribe (plus onError).
- A retrying state caused by a sequence gap carries
gap: { expected, received }; the next attempt clears it.
- A callback exception, including one thrown after close(), stops the
stream without retrying, publishes closed and rejects closed with the
original exception once reader, timers and listeners are released.
- A present X-KRM-Stream-Protocol header naming another version is a
terminal INTERNAL error before any event is applied; an absent header
is accepted. HTTP refusals keep their classification.
- applyStreamEvent and StreamChange move to the editor store module and
take ResourceStateEvent.
- Tests, the Go-wire e2e, examples, guides and spec §7 / consumer rule 9
use the one connector. The browser suite keeps raw EventSource checks
of the gateway: framing, cookies, a fresh snapshot on native reconnect
and test-owned terminal shutdown.
BREAKING CHANGE: connectManagedResourceStream is renamed connectResourceStream(url, consume, options), and the old single-connection connectResourceStream and connectWithEventSource are removed without aliases. ManagedStreamOptions/StreamOptions become ResourceStreamOptions and ManagedStreamHandle/StreamHandle become ResourceStreamHandle; onOpen, onSynced, onGap, onStateChange and onChange are removed in favour of state, subscribe and the callback. applyStreamEvent takes a ResourceStateEvent, without seq or error events. closed now rejects with a callback's exception, and a mismatched X-KRM-Stream-Protocol header is terminal.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe client package replaces managed and native EventSource connectors with a fetch-based connector that delivers resource state events to a consumer. Store application, connection lifecycle handling, browser examples, tests, and documentation are updated. A separate proposal describes potential Go gateway API cleanup. ChangesResource stream client
Gateway API cleanup proposal
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant Client
participant connectResourceStream
participant streamOnce
participant Consumer
participant LiveResourceStore
Client->>connectResourceStream: open URL with event consumer
connectResourceStream->>streamOnce: start fetch attempt
streamOnce->>Consumer: deliver recognized resource state event
Consumer->>LiveResourceStore: applyStreamEvent
LiveResourceStore-->>Consumer: return StreamChange
streamOnce-->>connectResourceStream: report attempt outcome
connectResourceStream-->>Client: publish connection state
Merge Risk: ⚪ Minimal · up to No merge-blocking issue is identified in the reviewed change; it is ready for normal merge checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The reviewed browser caller preserves its existing same-origin credential behavior, and the connector contains sequence gaps and consumer failures. No introduced security vulnerability was established. Gateway authentication and identity enforcement remain unverified, so the assessment is not minimal. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
| { | ||
| maxRetries: connections - 1, | ||
| retryDelayMs: 0, | ||
| fetch: (_input, init) => fetch(url(f.id, conn++), init), |
|
|
||
| try { | ||
| if (controller.signal.aborted) return undefined; | ||
| const res = await (opts.fetch ?? globalThis.fetch)(url, { |
Adds proposals 0009 (separating the browser stream from the editor) and 0010 (Go gateway API cleanup), and implements change 1 of proposal 0009: one public fetch connector that delivers protocol state events to a callback, so transport no longer depends on the editor store.
What changed
One connector.
connectResourceStream(url, consume, options)is the former managed connector under its final name. The single-connection fetch loop is now internal (streamOnceinsse.ts).connectManagedResourceStreamandconnectWithEventSourceare removed with no aliases.sse.tsandconnection.tsimport nothing from the store.State events in. A new
ResourceStateEventunion (reset,added/modified,deleted,synced; noseq, no transport error fields) is what the callback receives: synchronously, exactly once, in stream order. Unknown wire types still count in the sequence and are then ignored. Errors go only toonError. An upsert withoutobjector a delete withoutidentity.uidis dropped, as before.Lifecycle through
state+subscribe.onOpen,onSynced,onGap,onStateChangeandonChangeare removed;onErrorstays. The connector observesresetitself (it enterssyncingand clears the health timer before the callback applies the event). It publishesliveonly after the callback has appliedsynced, and does so after every completed snapshot. Aretryingstate caused by a sequence gap carriesgap: { expected, received }, which the next attempt clears.Consumer exceptions. The exception is captured where it is thrown, including after
close(). The reader, health timer and signal listener are released,closedis published, andhandle.closedrejects with the original exception. It is not retried and not reported as a stream error.Protocol header. If
X-KRM-Stream-Protocolis present on a successful response and names a different version, the stream fails with a terminal localINTERNALerror: the body is cancelled, no event is applied, and nothing is retried. A missing header is accepted. HTTP refusals keep their classification.Editor layer kept usable for now.
applyStreamEvent(store, event)andStreamChangemove intostore.tsand take aResourceStateEvent. The store's methods are otherwise unchanged; consolidating them is change 2.Migrated: package tests, the Go-wire e2e, the Vue and conditional-save examples, the vanilla browser demo, both READMEs, and the adoption, auth, state-model and why-a-gateway guides. Spec §7 and consumer rule 9 are reworded where removing the EventSource connector made them false. The wording changes are implementation-neutral; no wire fields or error codes change.
Raw EventSource coverage kept. The browser suite reads the real replay gateway with a raw
EventSourceand test-owned handling. It checks framing, that the session cookie is sent on both connections, that a native reconnect gets a freshseq: 1snapshot, and that closing on a terminal error stops reconnects.Breaking changes
connectManagedResourceStream(url, store, opts)→connectResourceStream(url, event => applyStreamEvent(store, event), opts). The old single-connectionconnectResourceStreamandconnectWithEventSourceare gone.ManagedStreamOptions/StreamOptions→ResourceStreamOptions;ManagedStreamHandle/StreamHandle→ResourceStreamHandle.onOpen,onSynced,onGap,onStateChange,onChangeare removed. Usestate/subscribe, the callback (render fromapplyStreamEvent's return value), andstate.gap.applyStreamEventtakes aResourceStateEvent: noseq, noerrorevents.StreamChange.typeno longer includes"error".closedrejects when the callback throws. A mismatched protocol header is terminal.Verification
Run on
42c745c:task fixtures-check(aftertask --force fixtures)task linttask testtask e2e-wiretask e2e-browsertask pack-clientconnectResourceStream/applyStreamEvent, and neither removed nameThe new connection tests were checked by breaking the implementation on purpose. Each of these made a test fail: ignoring a callback exception after
close(), publishinglivebeforesyncedis applied, dropping gap context, skipping the protocol check, leaking the health timer or the signal listener, observingresetafter the callback, and delivering events after a close.Not run:
task release-rehearsal(part oftask verify; it covers the Go adapter release and this PR doesn't touch it), and the cluster tasks (not a gate for browser separation).Remaining (proposal 0009 changes 2–3)
applyStreamEventmethod on the stores, withbeginSnapshot/endSnapshot/applyServerEvent/removeResourcemade private. Remove the standalone helper,ApplyResultandApplyOptions. Extract the sharedSnapshotTracker, addResourceStore, removeadoptSaved,takeTheirs,status()andreadOnlyPolicy, and simplifyReconciliationOptions. Add the/streamand/editorentry points, trim exports (including the internalSSEDecoder,StreamSequence, path/deep helpers andPROTOCOL_VERSION), and assert the emitted module graph.BREAKING CHANGE: connectManagedResourceStream is renamed connectResourceStream(url, consume, options), and the old single-connection connectResourceStream and connectWithEventSource are removed without aliases. ManagedStreamOptions/StreamOptions become ResourceStreamOptions and ManagedStreamHandle/StreamHandle become ResourceStreamHandle; onOpen, onSynced, onGap, onStateChange and onChange are removed in favour of state, subscribe and the callback. applyStreamEvent takes a ResourceStateEvent, without seq or error events. closed now rejects with a callback's exception, and a mismatched X-KRM-Stream-Protocol header is terminal.
🤖 Generated with Claude Code
Summary by CodeRabbit
connectResourceStream, which delivers resource events to a callback. Apply events to your store explicitly.EventSourceconnector is no longer available. Browser examples use fetch-based streaming.