diff --git a/docs/superpowers/notes/2026-08-08-mislabeled-commit-25b79ce4.md b/docs/superpowers/notes/2026-08-08-mislabeled-commit-25b79ce4.md deleted file mode 100644 index baa61472..00000000 --- a/docs/superpowers/notes/2026-08-08-mislabeled-commit-25b79ce4.md +++ /dev/null @@ -1,55 +0,0 @@ -# Correction: commit `25b79ce4` carries the wrong message - -**Date:** 2026-08-08 -**Branch:** `fix/streaming-frame-decoder` - -## What is wrong - -Commit `25b79ce4` has the message: - -> feat(client): add MergeSpecs and SourceKind - -That message is wrong. It describes work in `internal/client/` — the specification -merge primitive — and `25b79ce4` contains none of that. Its actual contents are: - -``` -extensions/streaming/frame_test.go -extensions/streaming/testdata/constants_fixture.go -internal/router/eventlog_id.go -internal/router/eventlog_id_test.go -``` - -This is streaming frame-decoder and router event-log work, belonging to a different -effort. Its original commit message is unrecoverable: the pre-amend commit is off -the branch, and the reflog records only that an amend occurred, not the message it -replaced. - -The `MergeSpecs` / `SourceKind` work that the message describes is in **`3d5bbd71`**, -which is correct and complete. - -## How it happened - -Two sessions were working in the same clone at the same time — the same working -directory, and therefore the same git index. `git add` and `git commit` are separate -operations, so one session's `add` can stage another's files, and one session's -`commit` can consume the other's staged index. Both occurred. A `git commit --amend` -issued as recovery then landed on the other session's commit rather than its own, -replacing that commit's message. - -No file content was lost. Every commit on the branch has the correct tree; only this -one commit's message describes the wrong change. - -## Why it was not repaired - -`25b79ce4` sits several commits deep, so rewording it means rewriting every commit -above it on a branch another session was actively committing to. That could strand -or corrupt in-flight work. Leaving an honest note was chosen over a history rewrite -under a live session. - -## What changed as a result - -Subsequent commits in the unified-streams-hooks-generation plan use -`git commit --only `, which builds a commit from exactly the named paths and -ignores the rest of the index, rather than `git add` followed by `git commit`. -Implementers are also instructed never to attempt `--amend`, `reset`, or `rebase` -recovery on a shared branch. diff --git a/docs/superpowers/notes/2026-08-08-two-replay-mechanisms.md b/docs/superpowers/notes/2026-08-08-two-replay-mechanisms.md deleted file mode 100644 index d89a3f75..00000000 --- a/docs/superpowers/notes/2026-08-08-two-replay-mechanisms.md +++ /dev/null @@ -1,52 +0,0 @@ -# Two SSE replay mechanisms now exist - -**Date:** 2026-08-08 -**Branch:** `fix/streaming-frame-decoder` - -## The two mechanisms - -**The router's event log** (`internal/router/eventlog*.go`, -`internal/router/streaming_sse_replay.go`). Positions are scalar, -`-`. Storage is a bounded in-memory ring per channel, per process. -Entries are written either by the connections on the route (`WithEventLog`) or by -the application's own producer (`WithProducerEventLog`). - -**The streaming extension's cursor replay** (`extensions/streaming/replay.go`). -A position is a vector — room ID to last delivered sequence — carried on the wire -as a base64url-encoded JSON token. Backlog comes from the `MessageStore`, and the -cursor is written by the producer, on every sequenced room message. - -## Only one can own the `id:` field - -An SSE event has one `id:`. When both mechanisms are active on a route, -`ErrEventIDAssignedByLog` settles it in the router's favour: `loggedStream` -refuses `SendWithID` / `SendJSONWithID`, the extension falls back to sending -without an ID, and that send routes through the router's `Send`, which emits the -router's scalar position instead. - -Messages keep flowing. What does not reach the wire is the cursor. - -As of this branch the refusal logs a warning once per stream, so the condition is -visible rather than silent. `WithEventLog`'s doc comment states the constraint. - -## Why the vector matters - -One SSE stream can carry many rooms, each advancing at its own rate. A scalar -position cannot encode where the client is in each of them, so a resume on a -multi-room stream cannot reconstruct the per-room set the client actually missed — -it replays the wrong set rather than failing. - -## What the router provides that the extension does not - -The router's replay has a wire contract the extension has no equivalent of: -`forge.resumed` (position resumed from, count delivered) and `forge.gap` (the gap -could not be filled), plus the client-side deferred recovery in -`packages/client-core` that waits for one of the two. That pair is how a client -learns whether its gap was actually filled, rather than assuming it was. - -## Open question - -Whether the extension's cursor should ride the router's id primitive and adopt the -`forge.resumed` / `forge.gap` contract — or whether the two should stay separate -and routes be required to pick one — is a question for whoever owns -`extensions/streaming`. Nothing here decides it. diff --git a/docs/superpowers/plans/2026-08-08-sse-event-replay.md b/docs/superpowers/plans/2026-08-08-sse-event-replay.md deleted file mode 100644 index 8b99c70d..00000000 --- a/docs/superpowers/plans/2026-08-08-sse-event-replay.md +++ /dev/null @@ -1,1513 +0,0 @@ -# SSE Event Replay Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make SSE streams resumable — a reconnecting client is handed the events it missed, or is honestly told the gap cannot be filled so it falls back to a full resync. - -**Architecture:** A pluggable `EventLog` records events per channel and assigns each an `-` ID. An opt-in route option wraps the SSE stream in a decorator that appends-then-sends, so the log and the wire cannot disagree about positions. On reconnect the wiring reads `Last-Event-ID`, replays what the log still holds, and emits a `forge.resumed` or `forge.gap` control event. The TypeScript client defers its existing recovery until one of those arrives or a grace window expires. - -**Tech Stack:** Go 1.26 (root module `github.com/xraph/forge`), testify for Go tests, TypeScript + vitest for `packages/client-core`. - -**Spec:** [docs/superpowers/specs/2026-08-08-sse-event-replay-design.md](../specs/2026-08-08-sse-event-replay-design.md) - -## Global Constraints - -- Root module only for Tasks 1–4; `packages/client-core` for Task 5; `extensions/streaming` for Task 6. -- Streams that do not opt in must be **byte-identical** on the wire to today's output. -- Any failure at any layer degrades to today's full-resync behavior, never to stale data. -- Every ID must pass the existing `validSSEFieldValue` check (no `\r` or `\n`). -- Comments explain **why**, matching the register of `generateConnectionID` and `validSSEFieldValue` in `internal/router/`. -- No `Co-Authored-By` trailers in any commit. -- `extensions/streaming` is a separate module owned by a parallel workstream and currently does not compile. Do not touch it before Task 6. -- Verification for root work: `go build ./... && go vet ./... && go test ./internal/router/...` - -## Two refinements discovered during planning - -Both deviate slightly from the approved spec. Flagged rather than silently applied. - -1. **`forge.gap` carries one reason, not four.** The spec listed `expired`, `epoch`, `malformed`, `unknown`, but `EventLog.Since` returns only a bool, so the wiring cannot tell which case occurred without widening the interface. Emitting a specific reason it cannot verify would be a lie in three cases out of four. The payload carries `{"reason":"unresumable"}`. The spec already states the client treats all reasons identically, so nothing downstream changes. - -2. **Frame decoders must pass `forge.*` frames through.** Control events are intercepted in `StreamBinder.accept` *after* `this.decode`, because the decoder is what knows the wire format. A decoder that swallows unrecognized frames will swallow control events, and recovery then falls back to the grace-window path — correct, just not optimal. Documented in Task 5. - -3. **`resumeGrace` lives on `StreamBinderOptions`, not `SubscriptionManagerOptions`.** The spec put it on the manager, but `recover` is the binder's method and the binder is what assigns `onReconnect`. The manager has a `sleep` and the binder does not, so the binder gains both `resumeGrace` and `sleep`. Putting the option on the manager would mean the manager holding a timer on behalf of a collaborator that owns the decision. - ---- - -### Task 1: Event ID codec - -The `-` format, parsed and formatted in one place. Pure functions, no state, so this task is entirely testable on its own. - -**Files:** -- Create: `internal/router/eventlog_id.go` -- Test: `internal/router/eventlog_id_test.go` - -**Interfaces:** -- Consumes: nothing -- Produces: `type eventID struct { Epoch string; Seq uint64 }`, `func formatEventID(epoch string, seq uint64) string`, `func parseEventID(s string) (eventID, bool)` - -- [ ] **Step 1: Write the failing test** - -Create `internal/router/eventlog_id_test.go`: - -```go -package router - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestEventID_RoundTrip(t *testing.T) { - s := formatEventID("7f3a9c1e", 42) - assert.Equal(t, "7f3a9c1e-42", s) - - id, ok := parseEventID(s) - require.True(t, ok) - assert.Equal(t, "7f3a9c1e", id.Epoch) - assert.Equal(t, uint64(42), id.Seq) -} - -// Epochs are UUIDs, which contain dashes, so the seq must be split off the -// right-hand end rather than the first dash found. -func TestEventID_EpochContainingDashes(t *testing.T) { - epoch := "3f2504e0-4f89-11d3-9a0c-0305e82c3301" - - id, ok := parseEventID(formatEventID(epoch, 7)) - require.True(t, ok) - assert.Equal(t, epoch, id.Epoch) - assert.Equal(t, uint64(7), id.Seq) -} - -// A malformed id must never parse into a plausible-looking position: every one -// of these resolves to "cannot resume" rather than to seq 0. -func TestEventID_Malformed(t *testing.T) { - for _, s := range []string{ - "", - "noseparator", - "epoch-", - "-42", - "epoch-notanumber", - "epoch--1", - "epoch-99999999999999999999999", - } { - t.Run(s, func(t *testing.T) { - _, ok := parseEventID(s) - assert.False(t, ok) - }) - } -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/router/ -run TestEventID -v` -Expected: FAIL — `undefined: formatEventID`, `undefined: parseEventID` - -- [ ] **Step 3: Write minimal implementation** - -Create `internal/router/eventlog_id.go`: - -```go -package router - -import ( - "strconv" - "strings" -) - -// eventID is a position in an event log, as it appears on the wire. -// -// The epoch exists because a sequence number alone is unsafe across a restart. -// A fresh process restarts its counters, so a client resuming from seq 41 would -// be handed events 42... that are entirely different events reusing the same -// numbers. Comparing epochs turns that silent mis-replay into an honest refusal -// to resume. -type eventID struct { - Epoch string - Seq uint64 -} - -// formatEventID renders a position for the wire. Both halves are text with no -// newline, so the result passes validSSEFieldValue without a special case. -func formatEventID(epoch string, seq uint64) string { - return epoch + "-" + strconv.FormatUint(seq, 10) -} - -// parseEventID parses a wire position. The bool reports whether s was -// well-formed; a false means the position cannot be honoured and the caller -// must treat the gap as unfillable. -// -// Split on the LAST separator: epochs are UUIDs and contain dashes of their -// own, so splitting on the first would read "3f2504e0" as the whole epoch and -// fail to parse the remainder as a number. -func parseEventID(s string) (eventID, bool) { - i := strings.LastIndexByte(s, '-') - if i <= 0 || i == len(s)-1 { - return eventID{}, false - } - - seq, err := strconv.ParseUint(s[i+1:], 10, 64) - if err != nil { - return eventID{}, false - } - - return eventID{Epoch: s[:i], Seq: seq}, true -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `go test ./internal/router/ -run TestEventID -v` -Expected: PASS (all subtests) - -- [ ] **Step 5: Verify and commit** - -```bash -go build ./... && go vet ./... && gofmt -l internal/router/eventlog_id.go internal/router/eventlog_id_test.go -git add internal/router/eventlog_id.go internal/router/eventlog_id_test.go -git commit -m "feat(router): event log position codec" -``` - ---- - -### Task 2: EventLog interface and in-memory implementation - -**Files:** -- Create: `internal/router/eventlog.go` (interface + `LoggedEvent`) -- Create: `internal/router/eventlog_memory.go` (bounded ring buffer) -- Test: `internal/router/eventlog_memory_test.go` -- Modify: `streaming.go` (root package re-exports, after the existing `Stream` alias) - -**Interfaces:** -- Consumes: `formatEventID`, `parseEventID`, `eventID` from Task 1 -- Produces: - - `type LoggedEvent struct { ID string; Event string; Data []byte }` - - `type EventLog interface { Append(ctx context.Context, channel, event string, data []byte) (string, error); Since(ctx context.Context, channel, id string) ([]LoggedEvent, bool, error) }` - - `type MemoryEventLogOptions struct { MaxPerChannel int; MaxAge time.Duration; Now func() time.Time }` - - `func NewMemoryEventLog(opts MemoryEventLogOptions) *MemoryEventLog` - - Constants `DefaultEventLogMaxPerChannel = 1024`, `DefaultEventLogMaxAge = 5 * time.Minute` - -- [ ] **Step 1: Write the failing test** - -Create `internal/router/eventlog_memory_test.go`: - -```go -package router - -import ( - "context" - "sync" - "testing" - "time" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func testLog(t *testing.T, opts MemoryEventLogOptions) *MemoryEventLog { - t.Helper() - - return NewMemoryEventLog(opts) -} - -func TestMemoryEventLog_AppendReturnsOrderedIDs(t *testing.T) { - log := testLog(t, MemoryEventLogOptions{}) - ctx := context.Background() - - first, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - second, err := log.Append(ctx, "orders", "created", []byte("b")) - require.NoError(t, err) - - a, ok := parseEventID(first) - require.True(t, ok) - - b, ok := parseEventID(second) - require.True(t, ok) - - assert.Equal(t, a.Epoch, b.Epoch, "one log, one epoch") - assert.Greater(t, b.Seq, a.Seq, "sequence must advance") -} - -func TestMemoryEventLog_SinceReplaysInOrder(t *testing.T) { - log := testLog(t, MemoryEventLogOptions{}) - ctx := context.Background() - - first, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - _, err = log.Append(ctx, "orders", "created", []byte("b")) - require.NoError(t, err) - _, err = log.Append(ctx, "orders", "updated", []byte("c")) - require.NoError(t, err) - - events, resumable, err := log.Since(ctx, "orders", first) - require.NoError(t, err) - require.True(t, resumable) - require.Len(t, events, 2) - - assert.Equal(t, "created", events[0].Event) - assert.Equal(t, []byte("b"), events[0].Data) - assert.Equal(t, "updated", events[1].Event) - assert.Equal(t, []byte("c"), events[1].Data) -} - -// The distinction the whole design rests on: "you missed nothing" and "I cannot -// tell you what you missed" must not look alike to the caller. -func TestMemoryEventLog_AtHeadIsResumableAndEmpty(t *testing.T) { - log := testLog(t, MemoryEventLogOptions{}) - ctx := context.Background() - - id, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - events, resumable, err := log.Since(ctx, "orders", id) - require.NoError(t, err) - assert.True(t, resumable, "at head: nothing was missed") - assert.Empty(t, events) -} - -func TestMemoryEventLog_EvictedByCountIsNotResumable(t *testing.T) { - log := testLog(t, MemoryEventLogOptions{MaxPerChannel: 2}) - ctx := context.Background() - - first, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - for _, payload := range []string{"b", "c", "d"} { - _, err = log.Append(ctx, "orders", "created", []byte(payload)) - require.NoError(t, err) - } - - events, resumable, err := log.Since(ctx, "orders", first) - require.NoError(t, err) - assert.False(t, resumable, "the events after first were evicted") - assert.Empty(t, events, "no events may be offered alongside a false") -} - -func TestMemoryEventLog_EvictedByAgeIsNotResumable(t *testing.T) { - now := time.Unix(1000, 0) - log := testLog(t, MemoryEventLogOptions{ - MaxAge: time.Minute, - Now: func() time.Time { return now }, - }) - ctx := context.Background() - - first, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - _, err = log.Append(ctx, "orders", "created", []byte("b")) - require.NoError(t, err) - - now = now.Add(2 * time.Minute) - - events, resumable, err := log.Since(ctx, "orders", first) - require.NoError(t, err) - assert.False(t, resumable) - assert.Empty(t, events) -} - -func TestMemoryEventLog_UnresumablePositions(t *testing.T) { - log := testLog(t, MemoryEventLogOptions{}) - ctx := context.Background() - - id, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - parsed, ok := parseEventID(id) - require.True(t, ok) - - tests := []struct { - name string - channel string - id string - }{ - {name: "malformed", channel: "orders", id: "not-an-id-at-all"}, - {name: "wrong epoch", channel: "orders", id: formatEventID("someotherepoch", parsed.Seq)}, - {name: "ahead of head", channel: "orders", id: formatEventID(parsed.Epoch, parsed.Seq+5)}, - {name: "unknown channel", channel: "invoices", id: id}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - events, resumable, err := log.Since(ctx, tt.channel, tt.id) - require.NoError(t, err) - assert.False(t, resumable) - assert.Empty(t, events) - }) - } -} - -// The caller may reuse its buffer after Append returns, so the log must hold a -// copy. Without one, a reused marshalling buffer rewrites history. -func TestMemoryEventLog_CopiesData(t *testing.T) { - log := testLog(t, MemoryEventLogOptions{}) - ctx := context.Background() - - first, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - payload := []byte("original") - - _, err = log.Append(ctx, "orders", "created", payload) - require.NoError(t, err) - - copy(payload, []byte("mutated!")) - - events, resumable, err := log.Since(ctx, "orders", first) - require.NoError(t, err) - require.True(t, resumable) - require.Len(t, events, 1) - assert.Equal(t, []byte("original"), events[0].Data) -} - -func TestMemoryEventLog_ConcurrentAppend(t *testing.T) { - log := testLog(t, MemoryEventLogOptions{}) - ctx := context.Background() - - var wg sync.WaitGroup - - for i := 0; i < 50; i++ { - wg.Add(1) - - go func() { - defer wg.Done() - - _, err := log.Append(ctx, "orders", "created", []byte("x")) - assert.NoError(t, err) - }() - } - - wg.Wait() -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/router/ -run TestMemoryEventLog -v` -Expected: FAIL — `undefined: MemoryEventLogOptions`, `undefined: NewMemoryEventLog` - -- [ ] **Step 3: Write the interface** - -Create `internal/router/eventlog.go`: - -```go -package router - -import "context" - -// LoggedEvent is one recorded event, as it will be replayed. -type LoggedEvent struct { - ID string - Event string - Data []byte -} - -// EventLog stores recent events so a reconnecting client can be handed the ones -// it missed instead of resynchronising from scratch. -type EventLog interface { - // Append records an event on a channel and returns the ID assigned to it. - Append(ctx context.Context, channel, event string, data []byte) (string, error) - - // Since returns the events recorded after id, in order. - // - // The bool reports whether id was still resolvable. False means the gap - // cannot be filled and the caller must fall back to a full resync; events is - // empty in that case and must NOT be read as "nothing was missed". - // - // Returning the two separately is the point of the signature. Folding them - // into an empty slice would make the case that silently serves stale data - // indistinguishable from the case that is safe. - Since(ctx context.Context, channel, id string) ([]LoggedEvent, bool, error) -} -``` - -- [ ] **Step 4: Write the in-memory implementation** - -Create `internal/router/eventlog_memory.go`: - -```go -package router - -import ( - "context" - "sync" - "time" - - "github.com/google/uuid" -) - -// Retention defaults. Bounded by count and age together: a count bound alone -// lets a quiet channel hold events long past their usefulness, and an age bound -// alone lets a busy channel grow without limit. The pair is what makes the -// footprint predictable. -const ( - DefaultEventLogMaxPerChannel = 1024 - DefaultEventLogMaxAge = 5 * time.Minute -) - -// MemoryEventLogOptions configures a MemoryEventLog. The zero value selects the -// defaults for every field. -type MemoryEventLogOptions struct { - MaxPerChannel int - MaxAge time.Duration - - // Now is the clock, injectable so age eviction is testable without sleeping. - Now func() time.Time -} - -// MemoryEventLog is a per-process, per-channel ring buffer. -// -// Each process gets its own epoch, so a client reconnecting to a different -// instance resolves to "not resumable" and resyncs. That is the honest answer -// rather than a wrong replay, and it is exactly the behaviour such a deployment -// has today. A shared log (Redis, NATS) with one epoch across instances is the -// supported upgrade and needs no transport changes. -type MemoryEventLog struct { - mu sync.Mutex - epoch string - maxPerChannel int - maxAge time.Duration - now func() time.Time - channels map[string]*channelLog -} - -type channelLog struct { - // nextSeq is the sequence the next append will take, so the newest retained - // position is nextSeq-1. Starts at 1 so that seq 0 means "before anything", - // which is what a client that has seen nothing reports. - nextSeq uint64 - entries []logEntry -} - -type logEntry struct { - seq uint64 - event string - data []byte - at time.Time -} - -// NewMemoryEventLog creates a bounded in-memory log. -func NewMemoryEventLog(opts MemoryEventLogOptions) *MemoryEventLog { - if opts.MaxPerChannel <= 0 { - opts.MaxPerChannel = DefaultEventLogMaxPerChannel - } - - if opts.MaxAge <= 0 { - opts.MaxAge = DefaultEventLogMaxAge - } - - if opts.Now == nil { - opts.Now = time.Now - } - - return &MemoryEventLog{ - epoch: uuid.NewString(), - maxPerChannel: opts.MaxPerChannel, - maxAge: opts.MaxAge, - now: opts.Now, - channels: map[string]*channelLog{}, - } -} - -// Append records an event and returns its wire position. -func (l *MemoryEventLog) Append(_ context.Context, channel, event string, data []byte) (string, error) { - l.mu.Lock() - defer l.mu.Unlock() - - ch := l.channels[channel] - if ch == nil { - ch = &channelLog{nextSeq: 1} - l.channels[channel] = ch - } - - seq := ch.nextSeq - ch.nextSeq++ - - // Copy: the caller may reuse its buffer as soon as this returns, and a - // shared backing array would let a later write rewrite recorded history. - stored := make([]byte, len(data)) - copy(stored, data) - - ch.entries = append(ch.entries, logEntry{seq: seq, event: event, data: stored, at: l.now()}) - - l.evict(ch) - - return formatEventID(l.epoch, seq), nil -} - -// Since returns the events after id. See EventLog.Since for the contract. -func (l *MemoryEventLog) Since(_ context.Context, channel, id string) ([]LoggedEvent, bool, error) { - parsed, ok := parseEventID(id) - if !ok { - return nil, false, nil - } - - l.mu.Lock() - defer l.mu.Unlock() - - if parsed.Epoch != l.epoch { - return nil, false, nil - } - - ch := l.channels[channel] - if ch == nil { - return nil, false, nil - } - - // Age eviction runs on read too. Without it a channel that stopped receiving - // events would keep reporting stale positions as resumable indefinitely. - l.evict(ch) - - // oldestRetained is the first position still held. With nothing retained it - // is nextSeq, which makes the check below accept only a client already at - // the head. - oldestRetained := ch.nextSeq - if len(ch.entries) > 0 { - oldestRetained = ch.entries[0].seq - } - - // Resumable when the client sits at or after the last position we can still - // prove continuity from, and not ahead of our head. A client ahead of the - // head is talking about events this log never issued, so it cannot be served - // correctly and is not served at all. - if parsed.Seq >= ch.nextSeq || parsed.Seq+1 < oldestRetained { - return nil, false, nil - } - - var events []LoggedEvent - - for _, entry := range ch.entries { - if entry.seq <= parsed.Seq { - continue - } - - data := make([]byte, len(entry.data)) - copy(data, entry.data) - - events = append(events, LoggedEvent{ - ID: formatEventID(l.epoch, entry.seq), - Event: entry.event, - Data: data, - }) - } - - return events, true, nil -} - -// evict drops entries past either bound. Caller holds l.mu. -func (l *MemoryEventLog) evict(ch *channelLog) { - if len(ch.entries) > l.maxPerChannel { - ch.entries = ch.entries[len(ch.entries)-l.maxPerChannel:] - } - - cutoff := l.now().Add(-l.maxAge) - - drop := 0 - - for drop < len(ch.entries) && ch.entries[drop].at.Before(cutoff) { - drop++ - } - - ch.entries = ch.entries[drop:] -} -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `go test ./internal/router/ -run TestMemoryEventLog -race -v` -Expected: PASS (all subtests) - -- [ ] **Step 6: Add root package re-exports** - -In `streaming.go` (repo root), after the `Stream` alias: - -```go -// EventLog stores recent events so a reconnecting SSE client can be handed the -// ones it missed. See WithEventLog. -type EventLog = router.EventLog - -// LoggedEvent is one recorded event, as it will be replayed. -type LoggedEvent = router.LoggedEvent - -// MemoryEventLogOptions configures NewMemoryEventLog. -type MemoryEventLogOptions = router.MemoryEventLogOptions - -// NewMemoryEventLog creates a bounded in-memory event log. -var NewMemoryEventLog = router.NewMemoryEventLog -``` - -- [ ] **Step 7: Verify and commit** - -```bash -go build ./... && go vet ./... && go test ./internal/router/... -race -git add internal/router/eventlog.go internal/router/eventlog_memory.go internal/router/eventlog_memory_test.go streaming.go -git commit -m "feat(router): bounded in-memory event log with honest gap reporting" -``` - ---- - -### Task 3: WithEventLog route option - -**Files:** -- Modify: `internal/router/router.go:108-135` (add two `RouteConfig` fields) -- Create: `internal/router/eventlog_option.go` -- Test: `internal/router/eventlog_option_test.go` -- Modify: `streaming.go` (root re-export of `WithEventLog`) - -**Interfaces:** -- Consumes: `EventLog` from Task 2, `RouteOption`/`RouteConfig` from `internal/router/router.go` -- Produces: `func WithEventLog(log EventLog, channel func(Context) string) RouteOption`, and `RouteConfig.EventLog` / `RouteConfig.EventLogChannel` - -- [ ] **Step 1: Write the failing test** - -Create `internal/router/eventlog_option_test.go`: - -```go -package router - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestWithEventLog_AppliesToConfig(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{}) - config := &RouteConfig{} - - WithEventLog(log, func(Context) string { return "orders" }).Apply(config) - - require.NotNil(t, config.EventLog) - require.NotNil(t, config.EventLogChannel) - assert.Equal(t, "orders", config.EventLogChannel(nil)) -} - -// A route with no option applied must be indistinguishable from today's, which -// is what keeps replay opt-in. -func TestRouteConfig_EventLogUnsetByDefault(t *testing.T) { - config := &RouteConfig{} - - assert.Nil(t, config.EventLog) - assert.Nil(t, config.EventLogChannel) -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/router/ -run 'TestWithEventLog|TestRouteConfig_EventLog' -v` -Expected: FAIL — `config.EventLog undefined`, `undefined: WithEventLog` - -- [ ] **Step 3: Add the RouteConfig fields** - -In `internal/router/router.go`, inside `type RouteConfig struct`, after the `MaxBodySize` field: - -```go - // EventLog makes an SSE route resumable. When set, events sent by the - // handler are recorded and a reconnecting client is replayed the ones it - // missed. Nil leaves the route behaving exactly as it did before. - EventLog EventLog - - // EventLogChannel derives the log partition from the request, so one route - // serving per-tenant or per-resource streams does not replay one client's - // events to another. Required whenever EventLog is set. - EventLogChannel func(Context) string -``` - -- [ ] **Step 4: Write the option** - -Create `internal/router/eventlog_option.go`: - -```go -package router - -// eventLogOpt carries the log and its channel resolver onto a route. -type eventLogOpt struct { - log EventLog - channel func(Context) string -} - -func (o *eventLogOpt) Apply(config *RouteConfig) { - config.EventLog = o.log - config.EventLogChannel = o.channel -} - -// WithEventLog makes an SSE route resumable. -// -// Events the handler sends are recorded in log, and a client reconnecting with -// a Last-Event-ID is replayed what it missed — or told the gap cannot be filled, -// so it can resync rather than silently continue with stale data. -// -// channel partitions the log by request. A route serving one global stream -// returns a constant; a route serving per-tenant streams returns the tenant, so -// one client's events are never replayed to another's reconnect. -func WithEventLog(log EventLog, channel func(Context) string) RouteOption { - return &eventLogOpt{log: log, channel: channel} -} -``` - -- [ ] **Step 5: Run tests to verify they pass** - -Run: `go test ./internal/router/ -run 'TestWithEventLog|TestRouteConfig_EventLog' -v` -Expected: PASS - -- [ ] **Step 6: Add the root re-export** - -In `streaming.go` (repo root), below the `NewMemoryEventLog` alias from Task 2: - -```go -// WithEventLog makes an SSE route resumable. See router.WithEventLog. -var WithEventLog = router.WithEventLog -``` - -- [ ] **Step 7: Verify and commit** - -```bash -go build ./... && go vet ./... && go test ./internal/router/... -race -git add internal/router/router.go internal/router/eventlog_option.go internal/router/eventlog_option_test.go streaming.go -git commit -m "feat(router): WithEventLog route option" -``` - ---- - -### Task 4: SSE replay wiring - -The stream decorator that appends-then-sends, the replay-on-connect logic, and the two control events. - -**Files:** -- Create: `internal/router/streaming_sse_replay.go` -- Test: `internal/router/streaming_sse_replay_test.go` -- Modify: `internal/router/router_streaming.go:63-110` (`EventStream`) - -**Interfaces:** -- Consumes: `EventLog`, `LoggedEvent` (Task 2); `RouteConfig.EventLog`, `RouteConfig.EventLogChannel` (Task 3); `Stream.SendWithID`, `Stream.LastEventID` (already shipped in `095a3887`) -- Produces: `const EventResumed = "forge.resumed"`, `const EventGap = "forge.gap"`, `type ResumedPayload struct { From string; Count int }`, `type GapPayload struct { Reason string }`, `func replayInto(stream Stream, log EventLog, channel string) error`, `type loggedStream struct` - -- [ ] **Step 1: Write the failing test** - -Create `internal/router/streaming_sse_replay_test.go`: - -```go -package router - -import ( - "context" - "net/http" - "net/http/httptest" - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func replayTestStream(t *testing.T, lastEventID string) (*sseStream, *httptest.ResponseRecorder) { - t.Helper() - - w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/events", nil) - - if lastEventID != "" { - req.Header.Set("Last-Event-ID", lastEventID) - } - - stream, err := newSSEStream(w, req, 0) - require.NoError(t, err) - - return stream, w -} - -// A fresh client has no position, so there is nothing to say to it. Emitting a -// control event here would make every first connection look like a recovery. -func TestReplayInto_FreshClientGetsNoControlEvent(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{}) - stream, w := replayTestStream(t, "") - - require.NoError(t, replayInto(stream, log, "orders")) - - assert.Empty(t, w.Body.String()) -} - -func TestReplayInto_ResumableReplaysThenMarksResumed(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{}) - ctx := context.Background() - - first, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - _, err = log.Append(ctx, "orders", "created", []byte("b")) - require.NoError(t, err) - _, err = log.Append(ctx, "orders", "updated", []byte("c")) - require.NoError(t, err) - - stream, w := replayTestStream(t, first) - - require.NoError(t, replayInto(stream, log, "orders")) - - body := w.Body.String() - assert.Contains(t, body, "data: b") - assert.Contains(t, body, "data: c") - assert.Contains(t, body, "event: "+EventResumed) - assert.Contains(t, body, `"count":2`) - - // The marker ends the replay, so it must follow the events it closes. - assert.Greater(t, strings.Index(body, EventResumed), strings.Index(body, "data: c")) -} - -func TestReplayInto_UnresumableEmitsGapAndNoEvents(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{MaxPerChannel: 1}) - ctx := context.Background() - - first, err := log.Append(ctx, "orders", "created", []byte("a")) - require.NoError(t, err) - - _, err = log.Append(ctx, "orders", "created", []byte("b")) - require.NoError(t, err) - _, err = log.Append(ctx, "orders", "created", []byte("c")) - require.NoError(t, err) - - stream, w := replayTestStream(t, first) - - require.NoError(t, replayInto(stream, log, "orders")) - - body := w.Body.String() - assert.Contains(t, body, "event: "+EventGap) - assert.NotContains(t, body, "data: b") - assert.NotContains(t, body, "data: c") -} - -func TestReplayInto_AtHeadResumesWithNoEvents(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{}) - - id, err := log.Append(context.Background(), "orders", "created", []byte("a")) - require.NoError(t, err) - - stream, w := replayTestStream(t, id) - - require.NoError(t, replayInto(stream, log, "orders")) - - body := w.Body.String() - assert.Contains(t, body, "event: "+EventResumed) - assert.Contains(t, body, `"count":0`) - assert.NotContains(t, body, "data: a") -} - -// The handler's events must reach both the log and the wire with the same ID, -// which is what makes a later resume land on the right position. -func TestLoggedStream_AppendsAndSendsWithSameID(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{}) - stream, w := replayTestStream(t, "") - - logged := &loggedStream{Stream: stream, log: log, channel: "orders"} - - require.NoError(t, logged.Send("created", []byte("a"))) - - body := w.Body.String() - require.Contains(t, body, "data: a") - - // The ID on the wire must be the one the log assigned. - events, resumable, err := log.Since(context.Background(), "orders", formatEventID(log.epoch, 0)) - require.NoError(t, err) - require.True(t, resumable) - require.Len(t, events, 1) - assert.Contains(t, body, "id: "+events[0].ID) -} - -func TestLoggedStream_SendJSONIsLogged(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{}) - stream, w := replayTestStream(t, "") - - logged := &loggedStream{Stream: stream, log: log, channel: "orders"} - - require.NoError(t, logged.SendJSON("created", map[string]string{"a": "b"})) - - assert.Contains(t, w.Body.String(), `data: {"a":"b"}`) - - events, resumable, err := log.Since(context.Background(), "orders", formatEventID(log.epoch, 0)) - require.NoError(t, err) - require.True(t, resumable) - require.Len(t, events, 1) -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `go test ./internal/router/ -run 'TestReplayInto|TestLoggedStream' -v` -Expected: FAIL — `undefined: replayInto`, `undefined: loggedStream`, `undefined: EventResumed` - -- [ ] **Step 3: Write the implementation** - -Create `internal/router/streaming_sse_replay.go`: - -```go -package router - -import ( - "context" - "encoding/json" -) - -// Control event names, reserved for the replay wiring. -// -// Namespaced so an application event can never collide with one: a forged -// "resumed" marker would convince a client that a gap was filled when it was -// not, which is the one failure this whole mechanism exists to prevent. -const ( - EventResumed = "forge.resumed" - EventGap = "forge.gap" -) - -// ResumedPayload closes a replay: the position resumed from and how many events -// were delivered. -type ResumedPayload struct { - From string `json:"from"` - Count int `json:"count"` -} - -// GapPayload tells the client the gap could not be filled. -// -// One reason value, not several. The log reports resumability as a bool, so the -// wiring cannot distinguish an expired position from a stale epoch without -// widening that interface, and naming a specific cause it has not established -// would be a guess dressed as a diagnosis. -type GapPayload struct { - Reason string `json:"reason"` -} - -// loggedStream records every event before sending it, and sends it under the ID -// the log assigned. -// -// Appending and sending in one place is what keeps the two consistent. If the -// handler sent directly and the log were written elsewhere, the wire and the log -// could disagree about a position, and a resume would then replay from the wrong -// point — silently, since neither side can detect the disagreement. -type loggedStream struct { - Stream - - log EventLog - channel string -} - -// Send records the event, then emits it with the recorded ID. -func (s *loggedStream) Send(event string, data []byte) error { - id, err := s.log.Append(s.Context(), s.channel, event, data) - if err != nil { - return err - } - - return s.Stream.SendWithID(id, event, data) -} - -// SendJSON marshals, then follows Send so the logged bytes are the sent bytes. -func (s *loggedStream) SendJSON(event string, v any) error { - data, err := json.Marshal(v) - if err != nil { - return err - } - - return s.Send(event, data) -} - -// replayInto brings a reconnecting client up to date, or tells it that it -// cannot be. -// -// Control events go to the underlying stream rather than a loggedStream: they -// describe the log and must not become entries in it, or every reconnect would -// append a marker that the next reconnect then replays. -func replayInto(stream Stream, log EventLog, channel string) error { - last := stream.LastEventID() - if last == "" { - // A first connection, not a resumption. Nothing was missed and there is - // nothing to report. - return nil - } - - events, resumable, err := log.Since(stream.Context(), channel, last) - if err != nil { - return err - } - - if !resumable { - return stream.SendJSON(EventGap, GapPayload{Reason: "unresumable"}) - } - - for _, event := range events { - if err := stream.SendWithID(event.ID, event.Event, event.Data); err != nil { - return err - } - } - - // Sent last, so receiving it means both "the gap was filled" and "the fill is - // complete". A marker sent first could not carry the second claim. - return stream.SendJSON(EventResumed, ResumedPayload{From: last, Count: len(events)}) -} - -// resumable wraps a stream for a route configured with an event log, replaying -// the client's gap first. Returns the stream the handler should use. -func resumable(stream Stream, log EventLog, channel string) (Stream, error) { - if err := replayInto(stream, log, channel); err != nil { - return nil, err - } - - return &loggedStream{Stream: stream, log: log, channel: channel}, nil -} -``` - -The `context` import is not needed in this file — `replayInto` reads the context off the stream, which is the one that gets cancelled when the client disconnects. - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `go test ./internal/router/ -run 'TestReplayInto|TestLoggedStream' -race -v` -Expected: PASS - -- [ ] **Step 5: Wire it into EventStream** - -In `internal/router/router_streaming.go`, inside the `EventStream` httpHandler, replace the `// Call handler` block with: - -```go - // A route with a log configured replays the client's gap and then hands - // the handler a stream that records what it sends. Without one, the - // handler gets the raw stream and the route behaves exactly as before. - handlerStream := Stream(stream) - - if routeConfig.EventLog != nil && routeConfig.EventLogChannel != nil { - channel := routeConfig.EventLogChannel(ctx) - - handlerStream, err = resumable(stream, routeConfig.EventLog, channel) - if err != nil { - if r.logger != nil { - r.logger.Error("SSE replay failed") - } - - return - } - } - - // Call handler - if err := handler(ctx, handlerStream); err != nil { - if r.logger != nil { - r.logger.Error("SSE handler error") - } - } -``` - -- [ ] **Step 6: Write the end-to-end route test** - -Append to `internal/router/streaming_sse_replay_test.go`: - -```go -// The opt-in guarantee: a route with no log configured must produce exactly -// what it produced before this feature existed. -func TestEventStream_WithoutEventLogIsUnchanged(t *testing.T) { - r := NewRouter() - - require.NoError(t, r.EventStream("/events", func(_ Context, s Stream) error { - return s.Send("created", []byte("a")) - })) - - w := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/events", nil) - req.Header.Set("Last-Event-ID", "someepoch-1") - - r.ServeHTTP(w, req) - - body := w.Body.String() - assert.Contains(t, body, "data: a") - assert.NotContains(t, body, "id:", "no log means no ids") - assert.NotContains(t, body, "forge.") -} - -func TestEventStream_WithEventLogReplaysOnReconnect(t *testing.T) { - log := NewMemoryEventLog(MemoryEventLogOptions{}) - r := NewRouter() - - require.NoError(t, r.EventStream("/events", func(_ Context, s Stream) error { - return s.Send("created", []byte("a")) - }, WithEventLog(log, func(Context) string { return "orders" }))) - - // First client: records one event and learns its id. - first := httptest.NewRecorder() - r.ServeHTTP(first, httptest.NewRequest(http.MethodGet, "/events", nil)) - - body := first.Body.String() - require.Contains(t, body, "id: ") - assert.NotContains(t, body, "forge.", "a fresh client gets no control event") - - // Second client resumes from before that event and is replayed it. - events, resumable, err := log.Since(context.Background(), "orders", formatEventID(log.epoch, 0)) - require.NoError(t, err) - require.True(t, resumable) - require.Len(t, events, 1) - - second := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodGet, "/events", nil) - req.Header.Set("Last-Event-ID", formatEventID(log.epoch, 0)) - - r.ServeHTTP(second, req) - - assert.Contains(t, second.Body.String(), "event: "+EventResumed) -} -``` - -If `NewRouter()` requires arguments in this codebase, match the construction used in `internal/router/router_test.go` rather than inventing one. - -- [ ] **Step 7: Run the full suite** - -Run: `go build ./... && go vet ./... && go test ./internal/router/... -race` -Expected: PASS - -- [ ] **Step 8: Commit** - -```bash -git add internal/router/streaming_sse_replay.go internal/router/streaming_sse_replay_test.go internal/router/router_streaming.go -git commit -m "feat(router): replay missed SSE events on reconnect" -``` - ---- - -### Task 5: Client-side conditional recovery - -**Files:** -- Modify: `packages/client-core/src/live.ts:265-284` (`StreamBinderOptions`), `live.ts:368` (`onReconnect` wiring), `live.ts:589` (`recover`), `live.ts:633` (`accept`) -- Modify: `packages/client-core/__tests__/harness.ts:54` (`harness` gains a binder-options passthrough) -- Test: `packages/client-core/__tests__/live.test.ts` - -**Interfaces:** -- Consumes: the wire contract from Task 4 — event names `forge.resumed` and `forge.gap`, arriving as frames shaped `{ type: 'forge.resumed', payload: {...} }` -- Produces: `resumeGrace?: number` and `sleep?: Sleep` on `StreamBinderOptions`; `StreamBinder` recovery deferral - -Existing fixtures this task builds on, all already in the repo — do not invent new ones: -- `harness(handler, bindings?, observe?)` in `__tests__/live.test.ts:54`, returning `{ cache, manager, binder, transport, sockets, batches, frames, release, clock, unknown }` -- `sockets.last().drop()` to sever a connection, `sockets.last().deliver(frame)` to push a message -- `clock.advance(ms)` — the manager's backoff `baseDelay` is 1000, so a reconnect completes at `advance(1000)` -- The existing `describe('gap recovery')` block at `live.test.ts:417`, whose tests assert on `transport.calls.length` - -- [ ] **Step 1: Give `harness` a binder-options passthrough** - -In `__tests__/live.test.ts`, extend the `harness` signature with a fourth parameter and spread it into the `StreamBinder` construction. Additive, so every existing call site is unaffected: - -```ts -function harness( - handler: Parameters[0], - bindings = streams, - observe?: (flush: () => void) => void, - binderOptions: Partial = {}, -) { -``` - -and in the `new StreamBinder({...})` call, add `sleep: clock.sleep,` and `...binderOptions,` as the final entries so a test can override either. - -Import `StreamBinderOptions` from `../src/live` alongside the existing `StreamBinder` import. - -- [ ] **Step 2: Write the failing tests** - -Append to the existing `describe('gap recovery')` block in `__tests__/live.test.ts`. These assert on refetches — the same observable the neighbouring tests use — rather than on a stubbed `cache.invalidate`: - -```ts - it('does not refetch when the server reports a completed replay', async () => { - const { cache, binder, sockets, transport, clock, batches } = harness((_request, call) => - call === 0 ? [{ id: 7, total: 99 }] : [{ id: 7, total: 4242 }], - ); - - cache.subscribe(orderList, undefined, () => undefined); - binder.subscribe(orderList); - await settleMicrotasks(); - - expect(transport.calls).toHaveLength(1); - - sockets.last().drop(); - await clock.advance(1000); - - // The server replayed the gap and said so. - sockets.last().deliver({ type: 'forge.resumed', payload: { from: 'e-1', count: 2 } }); - - // Well past the grace window: the deferred recovery must have been cancelled, - // not merely postponed. - await clock.advance(5000); - batches.flush(); - await settleMicrotasks(); - - expect(transport.calls).toHaveLength(1); - }); - - it('refetches immediately when the server reports an unfillable gap', async () => { - const { cache, binder, sockets, transport, clock, batches } = harness((_request, call) => - call === 0 ? [{ id: 7, total: 99 }] : [{ id: 7, total: 4242 }], - ); - - cache.subscribe(orderList, undefined, () => undefined); - binder.subscribe(orderList); - await settleMicrotasks(); - - sockets.last().drop(); - await clock.advance(1000); - - sockets.last().deliver({ type: 'forge.gap', payload: { reason: 'unresumable' } }); - batches.flush(); - await settleMicrotasks(); - - // Recovered without waiting out the grace window. - expect(transport.calls).toHaveLength(2); - }); - - // The fail-safe. A server that knows nothing about replay says nothing, and - // must land on exactly the behaviour that predates this deferral. - it('refetches when no control event arrives', async () => { - const { cache, binder, sockets, transport, clock, batches } = harness((_request, call) => - call === 0 ? [{ id: 7, total: 99 }] : [{ id: 7, total: 4242 }], - ); - - cache.subscribe(orderList, undefined, () => undefined); - binder.subscribe(orderList); - await settleMicrotasks(); - - sockets.last().drop(); - await clock.advance(1000); - - // Nothing yet: the grace window is still open. - expect(transport.calls).toHaveLength(1); - - await clock.advance(1000); - batches.flush(); - await settleMicrotasks(); - - expect(transport.calls).toHaveLength(2); - }); - - it('refetches without deferral when resumeGrace is 0', async () => { - const { cache, binder, sockets, transport, clock, batches } = harness( - (_request, call) => (call === 0 ? [{ id: 7, total: 99 }] : [{ id: 7, total: 4242 }]), - undefined, - undefined, - { resumeGrace: 0 }, - ); - - cache.subscribe(orderList, undefined, () => undefined); - binder.subscribe(orderList); - await settleMicrotasks(); - - sockets.last().drop(); - await clock.advance(1000); - - batches.flush(); - await settleMicrotasks(); - - expect(transport.calls).toHaveLength(2); - }); -``` - -- [ ] **Step 3: Run tests to verify they fail** - -Run: `cd packages/client-core && npx vitest run __tests__/live.test.ts -t "gap recovery"` -Expected: the two new deferral tests FAIL (recovery still fires immediately); the `resumeGrace: 0` and no-control-event tests may already pass, since today's behavior matches them - -- [ ] **Step 4: Add the binder options** - -In `packages/client-core/src/live.ts`, inside `StreamBinderOptions` after `onError`: - -```ts - /** - * How long to wait after a reconnect for the server to say whether it filled - * the gap, before recovering as if it had not. Defaults to 1000ms. - * - * A server that implements replay answers within a frame or two, so the full - * window is only ever paid by one that does not. 0 disables deferral and - * restores the unconditional recovery this option was added to soften. - */ - readonly resumeGrace?: number; - - /** Defaults to a real timer. Tests pass `manualClock().sleep`. */ - readonly sleep?: Sleep; -``` - -Import `Sleep` from `./transport` (it is exported at `transport.ts:120`). - -Assign both in the constructor alongside the existing options, defaulting `resumeGrace` to `1000` and `sleep` to `realSleep`. - -- [ ] **Step 5: Defer recovery in the binder** - -In `packages/client-core/src/live.ts`, replace the `onReconnect` assignment at line 368: - -```ts - // The gap-recovery trigger, wired here rather than by the caller so it - // cannot be left unwired -- which is a client that looks correct and is not. - // - // Deferred rather than conditional: this fires when the socket opens, which - // is before any control event can have arrived, so there is nothing to test - // yet. `settleRecovery` resolves it either way. - this.manager.onReconnect = (endpoint, channels) => { - if (this.resumeGrace === 0) { - this.recover(channels); - - return; - } - - this.pendingRecovery = channels; - - void this.sleep(this.resumeGrace).then(() => { - // Nothing said the gap was filled, so assume it was not. This is the - // path a server with no replay support always takes, and it must land - // on exactly the behaviour that predates this deferral. - this.settleRecovery(false); - }); - }; -``` - -Add to the class body, near `recover`: - -```ts - /** Channels awaiting a resume verdict, or undefined when none is pending. */ - private pendingRecovery: readonly string[] | undefined; - - /** - * Resolve a deferred recovery. - * - * `filled` true means the server replayed the gap and recovery is unnecessary. - * Every other caller passes false, so any doubt -- a gap report, a malformed - * payload, an expired window -- recovers. - */ - private settleRecovery(filled: boolean): void { - const channels = this.pendingRecovery; - - if (channels === undefined) return; - - this.pendingRecovery = undefined; - - if (!filled) this.recover(channels); - } -``` - -- [ ] **Step 6: Intercept control events in `accept`** - -In `live.ts`, immediately after the `if (decoded === undefined) return;` line at 633: - -```ts - // Control frames describe the stream rather than the data, so they are - // handled here and never reach the binding lookup -- which would report - // them as unknown messages and warn on every reconnect. - // - // Intercepted after decoding because the decoder is what knows the wire - // format. A decoder that drops frames it does not recognise will drop these - // too, and recovery then falls back to the grace window: later than ideal, - // still correct. - if (decoded.message === 'forge.resumed') { - this.settleRecovery(true); - - return; - } - - if (decoded.message === 'forge.gap') { - this.settleRecovery(false); - - return; - } -``` - -- [ ] **Step 7: Run tests to verify they pass** - -Run: `cd packages/client-core && npx vitest run __tests__/live.test.ts && npm run typecheck` -Expected: PASS, no type errors - -- [ ] **Step 8: Run the whole client suite for regressions** - -Run: `cd packages/client-core && npm test` -Expected: PASS. The pre-existing `gap recovery` tests at `live.test.ts:417` are the ones to watch: they drop a socket and `advance(1000)`, which now lands inside the grace window rather than after recovery. If they fail on a refetch count, add a second `await clock.advance(1000)` to carry them past the window — the behavior under test is unchanged, only its timing. - -- [ ] **Step 9: Commit** - -```bash -git add packages/client-core/src/live.ts packages/client-core/__tests__/live.test.ts -git commit -m "feat(client-core): skip gap recovery when the server replayed it" -``` - ---- - -### Task 6: Broker integration (gated) - -**Do not start this task until `cd extensions/streaming && GOWORK=off go build ./...` succeeds.** That module is owned by a parallel workstream and currently fails on `MessageTypeError` and `MessageTypeSystem` being undefined in `extension.go`. If it is still red, stop and report — Tasks 1–5 ship without it. - -Coordinate the `SessionSnapshot` change with whoever is editing `session_store.go` before writing it. - -**Files:** -- Modify: `extensions/streaming/session_store.go:10-17` (`SessionSnapshot`) -- Test: `extensions/streaming/session_store_test.go` - -**Interfaces:** -- Consumes: `forge.EventLog`, `forge.LoggedEvent` (Task 2 re-exports) -- Produces: `SessionSnapshot.LastEventIDs map[string]string` - -**Scope note.** This task adds the resume position to the session snapshot and nothing else. The spec's Layer 3 also calls for the publish path to append to the log and broadcast with the returned ID, which touches `manager.go` and `sse_connection.go` — files the parallel workstream is actively rewriting. Specifying edits against code that is changing under us would produce line references that are wrong by the time anyone reads them. That wiring gets its own task, planned once the module compiles and its publish path has settled. - -- [ ] **Step 1: Confirm the module builds** - -Run: `cd extensions/streaming && GOWORK=off go build ./...` -Expected: no output. **If this fails, stop and report — do not proceed.** - -- [ ] **Step 2: Write the failing test** - -Add to `extensions/streaming/session_store_test.go`: - -```go -// A snapshot records where each channel got to, so a resumption can resume -// rather than merely reconnect. Channels and DisconnectedAt alone say a session -// existed, not what it had seen. -func TestSessionSnapshot_CarriesLastEventIDs(t *testing.T) { - snapshot := &SessionSnapshot{ - SessionID: "s1", - Channels: []string{"orders"}, - LastEventIDs: map[string]string{"orders": "epoch-42"}, - } - - clone := snapshot.clone() - clone.LastEventIDs["orders"] = "epoch-99" - - assert.Equal(t, "epoch-42", snapshot.LastEventIDs["orders"], - "clone must not share the map with its original") -} -``` - -- [ ] **Step 3: Run test to verify it fails** - -Run: `cd extensions/streaming && GOWORK=off go test ./ -run TestSessionSnapshot_CarriesLastEventIDs -v` -Expected: FAIL — `unknown field LastEventIDs` - -- [ ] **Step 4: Add the field and extend `clone`** - -In `extensions/streaming/session_store.go`, add to `SessionSnapshot`: - -```go - // LastEventIDs is the position each channel had reached when the session - // dropped, so a resumption can ask for the gap instead of resynchronising. - LastEventIDs map[string]string `json:"last_event_ids,omitempty"` -``` - -Extend the existing `clone` method to deep-copy the new map, matching how it already copies `Rooms`, `Channels` and `Metadata` — two concurrent resumptions of one session must not observe each other's positions. - -- [ ] **Step 5: Run test to verify it passes** - -Run: `cd extensions/streaming && GOWORK=off go test ./ -run TestSessionSnapshot_CarriesLastEventIDs -v` -Expected: PASS - -- [ ] **Step 6: Verify and commit** - -```bash -cd extensions/streaming && GOWORK=off go build ./... && GOWORK=off go test ./... -cd ../.. && git add extensions/streaming/session_store.go extensions/streaming/session_store_test.go -git commit -m "feat(streaming): record per-channel resume positions on session snapshots" -``` - ---- - -## Final verification - -```bash -go build ./... && go vet ./... && go test ./internal/router/... -race -cd packages/client-core && npm test && npm run typecheck -cd ../../extensions/streaming && GOWORK=off go build ./... -``` - -The last line is expected to fail until the parallel workstream's refactor lands. That is not a regression from this work — confirm the failures name only `MessageTypeError` / `MessageTypeSystem` or other symbols this plan never touched. diff --git a/docs/superpowers/plans/2026-08-08-streaming-frame-decoder-followups.md b/docs/superpowers/plans/2026-08-08-streaming-frame-decoder-followups.md deleted file mode 100644 index 35ff1779..00000000 --- a/docs/superpowers/plans/2026-08-08-streaming-frame-decoder-followups.md +++ /dev/null @@ -1,783 +0,0 @@ -# Streaming Frame Decoder Follow-ups Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Close the gaps left by `cb773c34` — make the default decoder read the streaming envelope, and make the cross-language contract test actually enforce what its comments claim. - -**Architecture:** Two independent threads. The first reorders one expression in `decodeFrame` so the shipped default stops discarding streaming frames, which removes the requirement that every application opt in. The second replaces two hand-written literal lists in `frame_test.go` with values derived from the sources they claim to mirror — the Go constants by parsing the AST, the TypeScript set by reading the file — so drift on either side fails a test instead of passing one. - -**Tech Stack:** TypeScript (vitest) in `packages/client-core`; Go 1.26 (stdlib `go/ast`, `go/parser`, `regexp`) in `extensions/streaming`. - -## Global Constraints - -- **Never add `Co-Authored-By` trailers** to any commit. No co-author trailers of any kind. -- All Go commands in `extensions/streaming` require `GOWORK=off`: `cd extensions/streaming && GOWORK=off go build ./...` -- `extensions/streaming` is its **own Go module** (`extensions/streaming/go.mod`, `go 1.26.0`). It can be consumed standalone, so a test may read a file outside the module only if it skips cleanly when that file is absent. -- `extensions/streaming/internal/streaming.go`, `manager.go` and `extension.go` are shared with a parallel workstream. Task 2 **reads** `internal/streaming.go` and must not modify it. -- The streaming package's test binary is intermittently broken by that parallel workstream's in-flight test files. If `go test ./` fails to compile in a file you did not touch, that is not your change — verify with `GOWORK=off go build ./...` (library only) and note it. -- Comments in this codebase document *why* a choice was made and what the alternative cost. Match that. Do not write comments that describe what the next line does. - -## Preflight (not a task) - -`npm run typecheck` in `packages/client-core` fails with `TS2688: Cannot find type definition file for 'node'` because `@types/node` is not installed. This predates all of this work. Fix once, before starting: - -```bash -cd packages/client-core && npm install -``` - -Verify: `npm run typecheck` exits 0. If it still fails, the remaining tasks are unaffected — `npx tsc --noEmit -p tsconfig.json` covers `src/` and passes today. - -## Decision this plan assumes - -**Task 1 changes the behaviour of the shipped `decodeFrame`.** It is the highest-value item here and the only one a reviewer might reject outright, so it is Task 1 and nothing else depends on it — rejecting it leaves Tasks 2–5 intact. - -The change is reordering `type ?? event ?? name` to `event ?? type ?? name`. Rationale: - -- It is correct for **all three** documented envelope shapes rather than two. The plain Forge WebSocket shape (`{type: 'order.created', payload}`) carries no `event`, so `type` still wins. The SSE/AsyncAPI shape (`{event: 'order.created', data}`) already wanted `event` first. The streaming extension shape gets the domain name instead of the transport kind. -- The only server it breaks is one sending **both** `type` as a message name and `event` as something else. No test in the repo does this; the sole frame carrying both fields is `streaming.test.ts:199`, where both are empty strings. -- It does not make `forgeStreamingDecoder` redundant. That decoder still filters the extension's transport frames out of `onUnknown` and still owns `channelOf`. The reorder only stops the default from being catastrophically wrong. - -**If you reject Task 1**, the alternative is to leave `decodeFrame` alone and have every application pass `decode: forgeStreamingDecoder()`, which the docs committed in `5463788c` already instruct. That is a valid end state; it just means the failure mode stays one forgotten line away. - ---- - -## File Structure - -| File | Responsibility | Task | -|---|---|---| -| `packages/client-core/src/live.ts` | `decodeFrame` name resolution order + its doc comment | 1 | -| `packages/client-core/__tests__/streaming.test.ts` | Streaming envelope behaviour; one test currently pins the *old* default and must be rewritten | 1, 4 | -| `packages/client-core/src/streaming.ts` | `forgeStreamingDecoder` channel resolution | 4 | -| `extensions/streaming/frame_test.go` | The cross-language contract assertions | 2, 3 | -| `extensions/streaming/frame.go` | `NewEventMessage` godoc | 5 | - ---- - -### Task 1: Make the default decoder read `event` first - -**Files:** -- Modify: `packages/client-core/src/live.ts:217-240` (the `decodeFrame` doc comment and its `name` expression) -- Test: `packages/client-core/__tests__/streaming.test.ts:110-121` (rewrite the test that pins the old behaviour), plus one new case - -**Interfaces:** -- Consumes: nothing from other tasks. -- Produces: `decodeFrame` keeps its exported signature `FrameDecoder = (message: unknown) => DecodedFrame | undefined`. No call site changes. - -- [ ] **Step 1: Rewrite the test that pins the old behaviour** - -The existing test asserts the defect. Replace it — open `packages/client-core/__tests__/streaming.test.ts` and swap the whole `it('is unreadable by the default decoder, and drops the whole channel', ...)` block for these two: - -```ts - // What the reorder bought. `type` is the transport kind, so the old - // `type ?? event` order named every frame on every channel `message`, no - // manifest row is keyed on `message`, and the whole channel was discarded. - // Reading `event` first is correct for this envelope and still correct for - // the two shapes that carry no `event` at all. - it('is now readable by the default decoder', async () => { - const { cache, sockets, frames, unknown } = await connect(decodeFrame); - - sockets.last().deliver(frame('order.created', { id: 9, total: 5 })); - frames.flush(); - - expect(unknown).toEqual([]); - expect(cache.store.getRecord('Order:9')?.data).toEqual({ id: 9, total: 5 }); - }); - - // Why forgeStreamingDecoder still exists after the reorder. The default has - // no notion of a reserved transport kind, so a presence frame reaches it as - // the name `presence` and is reported -- once per (channel, message) in - // development -- for a frame that is working exactly as designed. - it('still reports the extension’s transport frames, which the streaming decoder does not', async () => { - const { sockets, frames, unknown } = await connect(decodeFrame); - - sockets.last().deliver({ id: 'm', type: 'presence', user_id: 'u-1', data: null }); - frames.flush(); - - expect(unknown).toEqual([{ message: 'presence', channel: '/ws/orders' }]); - }); -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -```bash -cd packages/client-core && npx vitest run __tests__/streaming.test.ts -``` - -Expected: FAIL. `is now readable by the default decoder` fails with `unknown` containing `{message: 'message', channel: '/ws/orders'}` and `Order:9` undefined. The second new test passes already (`presence` is reported under both orders). - -- [ ] **Step 3: Reorder the expression** - -In `packages/client-core/src/live.ts`, change the one line inside `decodeFrame`: - -```ts - const name = envelope['event'] ?? envelope['type'] ?? envelope['name']; -``` - -- [ ] **Step 4: Rewrite the doc comment above it** - -The existing comment explains the old order and is now wrong. Replace the paragraph beginning "The default envelope reader, over the three shapes in circulation." with: - -```ts -/** - * The default envelope reader, over the three shapes in circulation. - * - * `event`/`data` is what an SSE adapter naturally produces, since `EventSource` - * dispatches by event name, and what `extensions/streaming` sends; `type`/`payload` - * is what a plain Forge WebSocket handler emits; `name` is the AsyncAPI spelling. - * A message with a name and no payload field is its own payload, which is what a - * server that sends the entity flat with a `type` discriminator produces. - * - * `event` is read *first*, and the order is the whole of the fix for a defect - * that discarded entire channels. In the streaming extension `type` is not the - * message name at all -- it is the transport kind, one of seven reserved strings - * -- and the domain name lives in `event`. Under the previous `type ?? event` - * order every frame from that extension decoded as `message`, nothing in any - * generated manifest is keyed on `message`, and the channel was reported through - * `onUnknown` while its socket sat open and healthy. Reading `event` first costs - * the two older shapes nothing, because neither carries an `event` field; the - * only server this order is wrong for is one sending `type` as a message name - * *and* `event` as something else, which no shape in circulation does. - * - * This does not make `forgeStreamingDecoder` redundant. That decoder still knows - * which names are reserved transport kinds -- presence, typing, join -- and drops - * them silently instead of reporting them, and it owns the `channel_id` mapping. - * This one only stops the default from being wrong about the name. - */ -``` - -- [ ] **Step 5: Run the full client suite** - -```bash -cd packages/client-core && npm test -``` - -Expected: PASS, all files. Pay attention to `live.test.ts` and `envelope.test.ts` — they exercise the `{type, payload}` shape and must be unaffected. - -- [ ] **Step 6: Typecheck the source** - -```bash -cd packages/client-core && npx tsc --noEmit -p tsconfig.json -``` - -Expected: exit 0, no output. - -- [ ] **Step 7: Commit** - -```bash -git add packages/client-core/src/live.ts packages/client-core/__tests__/streaming.test.ts -git commit -m "fix(client-core): read event before type in the default frame decoder - -The streaming extension puts the domain name in event and the transport -kind in type, so the type-first order named every frame message and -discarded whole channels through onUnknown. Reading event first is -correct for that envelope and costs the two older shapes nothing, since -neither carries an event field. - -forgeStreamingDecoder is still the right choice for a streaming channel: -it drops the extension's reserved transport frames rather than reporting -them, and it owns the channel_id mapping. This only fixes the name." -``` - -- [ ] **Step 8: Update the docs that describe the old default** - -`5463788c` documented the default decoder as reading `type`. Three places now overstate the problem — they say a streaming channel is *discarded* without `forgeStreamingDecoder`, which after Task 1 is only true of its transport frames. - -In `packages/client-core/README.md`, in the "Which envelope the frames arrive in" section, replace the blockquote beginning `> **Getting this wrong is silent and total.**` with: - -```markdown -> **The default reads `event` first**, so a streaming channel's domain frames -> bind without any configuration. What `forgeStreamingDecoder` adds is the rest -> of the envelope: it knows `presence`, `typing` and `join` are transport kinds -> rather than message names, and drops them instead of reporting each one -> through `onUnknown` once per channel. -``` - -In `docs/content/docs/web-client/invalidation.mdx`, in the `` under "Which field carries the name", replace the first paragraph with: - -```markdown -**A channel served by `extensions/streaming` should pass `forgeStreamingDecoder`** as the binder's `decode`. The default decoder reads `event` first, so domain frames bind without it — but it has no notion of a reserved transport kind, so every `presence`, `typing` and `join` frame is reported as an unknown message on a channel that is working exactly as designed. -``` - -In `docs/content/docs/web-client/adapters.mdx`, in the live-queries callout, replace the paragraph beginning `And they need the binder to be reading the envelope` with: - -```markdown -A channel served by `extensions/streaming` should also pass `decode: forgeStreamingDecoder()`. Its domain frames bind under the default decoder, but its transport frames — presence, typing, join — are reported as unknown messages without it. See [Which field carries the name](/docs/web-client/invalidation#which-field-carries-the-name). -``` - -- [ ] **Step 9: Verify the docs still compile and commit** - -```bash -cd docs && npx fumadocs-mdx -``` - -Expected: `[MDX] generated files in ms`, exit 0. - -```bash -git add packages/client-core/README.md docs/content/docs/web-client/invalidation.mdx docs/content/docs/web-client/adapters.mdx -git commit -m "docs(web-client): the default decoder now reads event first - -Narrows the warning to what is still true after the reorder: a streaming -channel's domain frames bind out of the box, and forgeStreamingDecoder -earns its place by knowing which names are transport kinds." -``` - ---- - -### Task 2: Derive the Go constant list instead of copying it - -**Files:** -- Modify: `extensions/streaming/frame_test.go:74-111` (`TestTransportKindsMirrorTheConstants`) -- Create: `extensions/streaming/testdata/constants_fixture.go` -- Read only: `extensions/streaming/internal/streaming.go` (parsed, never modified — a parallel workstream owns it) - -**Interfaces:** -- Consumes: `streaming.TransportKinds() []string` from `frame.go`, already committed. -- Produces: two unexported test helpers used only within `frame_test.go` — - `messageTypesIn(t *testing.T, path string) []string` (parses one file) and - `declaredMessageTypes(t *testing.T) []string` (calls it on `internal/streaming.go` - and fails when the parse finds nothing). - -**Why:** the test's comment claims that adding a `MessageType*` constant without adding it to `TransportKinds()` fails here. It does not. `declared` is a hand-written literal that changes only when someone edits the test, so the assertion compares two copies of the same list and a new constant slips past both. The fix is to read the constants from the source that declares them. - -- [ ] **Step 1: Write the failing test** - -Add this helper and rewrite the first assertion in `extensions/streaming/frame_test.go`. The imports needed are `go/ast`, `go/parser`, `go/token`, `path/filepath`, `strconv`, `strings`. - -```go -// declaredMessageTypes reads every MessageType* constant out of the file that -// declares them, rather than restating them here. -// -// A hand-written copy was the first version, and it asserted nothing: it changed -// only when somebody edited this test, so the comparison was between two copies -// of the same list and a newly declared kind passed both. Parsing the source is -// the only spelling in which "a constant was added and TransportKinds was not" -// is a detectable event. -func declaredMessageTypes(t *testing.T) []string { - t.Helper() - - path := filepath.Join("internal", "streaming.go") - - file, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) - if err != nil { - t.Fatalf("parse %s: %v", path, err) - } - - var declared []string - - for _, decl := range file.Decls { - gen, ok := decl.(*ast.GenDecl) - if !ok || gen.Tok != token.CONST { - continue - } - - for _, spec := range gen.Specs { - value, ok := spec.(*ast.ValueSpec) - if !ok { - continue - } - - for i, name := range value.Names { - if !strings.HasPrefix(name.Name, "MessageType") || i >= len(value.Values) { - continue - } - - lit, ok := value.Values[i].(*ast.BasicLit) - if !ok || lit.Kind != token.STRING { - continue - } - - unquoted, err := strconv.Unquote(lit.Value) - if err != nil { - t.Fatalf("unquote %s: %v", name.Name, err) - } - - declared = append(declared, unquoted) - } - } - } - - if len(declared) == 0 { - t.Fatalf("no MessageType* constants found in %s; the parse found nothing to check", path) - } - - return declared -} -``` - -Then replace the `declared := []string{...}` literal in `TestTransportKindsMirrorTheConstants` with: - -```go - declared := declaredMessageTypes(t) -``` - -and change the function's doc comment, which currently overstates what it caught: - -```go -// TestTransportKindsMirrorTheConstants fails when a MessageType* constant is -// declared and not added to TransportKinds. -// -// The failure is the point. An unmirrored kind reaches the client as a frame -// name no binding claims and is reported as an unknown message on every channel -// that emits it -- a quiet, permanent warning for something working exactly as -// designed. The constants are parsed out of internal/streaming.go rather than -// copied here, because a copy is not a check: it agrees with whatever it was -// last edited to agree with. -``` - -- [ ] **Step 2: Split the path out so the parse itself is testable** - -The proof that "a new constant would be caught" must not require editing -`internal/streaming.go` — a parallel workstream owns that file and a temporary -edit there risks colliding with its writes, or being left behind if this task -errors out partway. Take the path as a parameter and point a second test at a -fixture instead. - -Restructure the helper from Step 1 into two functions: - -```go -// messageTypesIn reads every MessageType* constant declared in one file. -func messageTypesIn(t *testing.T, path string) []string { - t.Helper() - - file, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) - if err != nil { - t.Fatalf("parse %s: %v", path, err) - } - - var declared []string - - for _, decl := range file.Decls { - gen, ok := decl.(*ast.GenDecl) - if !ok || gen.Tok != token.CONST { - continue - } - - for _, spec := range gen.Specs { - value, ok := spec.(*ast.ValueSpec) - if !ok { - continue - } - - for i, name := range value.Names { - if !strings.HasPrefix(name.Name, "MessageType") || i >= len(value.Values) { - continue - } - - lit, ok := value.Values[i].(*ast.BasicLit) - if !ok || lit.Kind != token.STRING { - continue - } - - unquoted, err := strconv.Unquote(lit.Value) - if err != nil { - t.Fatalf("unquote %s: %v", name.Name, err) - } - - declared = append(declared, unquoted) - } - } - } - - return declared -} - -// declaredMessageTypes reads the constants out of the file that declares them, -// rather than restating them here. -// -// A hand-written copy was the first version, and it asserted nothing: it changed -// only when somebody edited this test, so the comparison was between two copies -// of the same list and a newly declared kind passed both. Parsing the source is -// the only spelling in which "a constant was added and TransportKinds was not" -// is a detectable event. -func declaredMessageTypes(t *testing.T) []string { - t.Helper() - - path := filepath.Join("internal", "streaming.go") - declared := messageTypesIn(t, path) - - if len(declared) == 0 { - t.Fatalf("no MessageType* constants found in %s; the parse found nothing to check", path) - } - - return declared -} -``` - -- [ ] **Step 3: Write the fixture** - -Create `extensions/streaming/testdata/constants_fixture.go`. The `testdata` -directory is ignored by the go tool, so a `.go` file inside it is never compiled -or vetted — it exists only to be parsed. - -```go -package fixture - -// A stand-in for the constant block in internal/streaming.go, with one kind the -// real file does not declare. If messageTypesIn stops noticing an added -// constant, the ack below stops appearing and the test that reads this fails -- -// which is the proof the real assertion cannot give without editing a file this -// module shares with another workstream. - -// Message types. -const ( - MessageTypeMessage = "message" - MessageTypeAck = "ack" -) - -// Deliberately not a message type: the prefix filter must skip it. -const NotAMessageType = "ignored" -``` - -- [ ] **Step 4: Write the test that proves the parse catches an added constant** - -Add to `extensions/streaming/frame_test.go`: - -```go -// TestMessageTypesInFindsEveryDeclaredConstant is the proof that -// declaredMessageTypes would notice a newly declared kind. -// -// Asserted against a fixture rather than by temporarily editing -// internal/streaming.go: that file is shared with another workstream, and a -// proof that requires mutating somebody else's file is a proof that will one -// day be left half-applied. -func TestMessageTypesInFindsEveryDeclaredConstant(t *testing.T) { - got := messageTypesIn(t, filepath.Join("testdata", "constants_fixture.go")) - - want := []string{"message", "ack"} - - if !slices.Equal(got, want) { - t.Errorf("messageTypesIn(fixture) = %v, want %v", got, want) - } -} -``` - -- [ ] **Step 5: Run both tests** - -```bash -cd extensions/streaming && GOWORK=off go test -run 'TestTransportKindsMirrorTheConstants|TestMessageTypesInFindsEveryDeclaredConstant' -v ./ -``` - -Expected: both PASS. `TestMessageTypesInFindsEveryDeclaredConstant` proves the -parse picks up `ack` — a constant `TransportKinds()` does not contain — so the -same helper pointed at the real file will notice a real addition. - -- [ ] **Step 6: Confirm you changed nothing you do not own** - -```bash -git status --short extensions/streaming/internal/ -``` - -Expected: no output attributable to this task. The parallel workstream may have -its own edits there; none of them should be yours. - -- [ ] **Step 7: Format, vet and commit** - -```bash -cd extensions/streaming && gofmt -l frame_test.go && GOWORK=off go vet ./ -``` - -Expected: no output from either. - -```bash -git add extensions/streaming/frame_test.go extensions/streaming/testdata/constants_fixture.go -git commit -m "test(streaming): parse the message-type constants instead of copying them - -The list was hand-written, so it agreed with whatever it was last edited -to agree with: a newly declared MessageType* constant that never reached -TransportKinds passed the assertion that exists to catch exactly that. -Reading them out of internal/streaming.go makes the omission detectable." -``` - ---- - -### Task 3: Read the TypeScript mirror from the TypeScript file - -**Files:** -- Modify: `extensions/streaming/frame_test.go` (the second assertion in `TestTransportKindsMirrorTheConstants`) -- Read only: `packages/client-core/src/streaming.ts` - -**Interfaces:** -- Consumes: `declaredMessageTypes` from Task 2 is *not* required — this task is independent and touches a different assertion in the same function. If Task 2 has not been done, the `declared` literal is still there and untouched by this task. -- Produces: an unexported test helper `mirroredTransportKinds(t *testing.T) ([]string, bool)`. - -**Why:** `mirrored` is a hardcoded snapshot of the TypeScript set inside a Go file. Editing `TRANSPORT_KINDS` in `streaming.ts` fails no test anywhere — the Go test only notices Go drifting away from a frozen copy, which is the less likely direction. Reading the actual file makes the mirror bidirectional. - -**Module boundary:** `extensions/streaming` is its own Go module and can be consumed without the rest of the repo. The helper returns `false` when the file is absent and the test skips, rather than failing for a consumer who has no `packages/` directory. - -- [ ] **Step 1: Write the helper** - -Add to `extensions/streaming/frame_test.go`. Imports needed: `os`, `regexp`, and `path/filepath` — the last is already present if Task 2 was done first, and must be added if this task is done alone. - -```go -// transportKindsLiteral matches the TRANSPORT_KINDS declaration in -// packages/client-core/src/streaming.ts and captures the body of its Set. -// -// A regexp rather than a TypeScript parse, and the narrowness is deliberate: it -// matches one declaration whose exact text is a few lines away in a file this -// repository owns. If that declaration is ever rewritten into a form this does -// not match, the helper reports no kinds and the test fails loudly rather than -// passing on an empty comparison -- see the length check below. -var transportKindsLiteral = regexp.MustCompile(`(?s)TRANSPORT_KINDS[^=]*=\s*new Set\(\[(.*?)\]\)`) - -var quotedKind = regexp.MustCompile(`'([^']*)'`) - -// mirroredTransportKinds reads the set the TypeScript decoder actually holds. -// -// Returns false when the client package is not present. This module is -// publishable on its own, and a consumer who fetched it without the repository -// around it has no packages/ directory -- skipping there is correct, whereas -// failing would make the module untestable outside its own tree. -func mirroredTransportKinds(t *testing.T) ([]string, bool) { - t.Helper() - - path := filepath.Join("..", "..", "packages", "client-core", "src", "streaming.ts") - - source, err := os.ReadFile(path) - if err != nil { - if os.IsNotExist(err) { - return nil, false - } - - t.Fatalf("read %s: %v", path, err) - } - - block := transportKindsLiteral.FindSubmatch(source) - if block == nil { - t.Fatalf("no TRANSPORT_KINDS set found in %s; the decoder's reserved kinds could not be read", path) - } - - var kinds []string - - for _, match := range quotedKind.FindAllSubmatch(block[1], -1) { - kinds = append(kinds, string(match[1])) - } - - if len(kinds) == 0 { - t.Fatalf("TRANSPORT_KINDS in %s parsed to nothing", path) - } - - return kinds, true -} -``` - -- [ ] **Step 2: Replace the hardcoded mirror** - -In `TestTransportKindsMirrorTheConstants`, swap the `mirrored := []string{...}` literal and the comment above it for: - -```go - // The set the TypeScript decoder actually holds, read from the file that - // holds it. A copy pinned here would only catch this side drifting; the - // direction that matters as much is the client's set gaining a kind that - // Go never reserved. - mirrored, present := mirroredTransportKinds(t) - if !present { - t.Skip("packages/client-core is not present; nothing to mirror against") - } -``` - -- [ ] **Step 3: Run it** - -```bash -cd extensions/streaming && GOWORK=off go test -run TestTransportKindsMirrorTheConstants -v ./ -``` - -Expected: PASS. - -- [ ] **Step 4: Prove it catches TypeScript-side drift** - -Temporarily add `'ack',` to the `TRANSPORT_KINDS` set in `packages/client-core/src/streaming.ts`, then: - -```bash -cd extensions/streaming && GOWORK=off go test -run TestTransportKindsMirrorTheConstants ./ -``` - -Expected: FAIL — `TransportKinds() = [...], but packages/client-core/src/streaming.ts holds [... ack]`. - -**Then revert the TypeScript edit** and re-run to confirm PASS. - -- [ ] **Step 5: Prove the skip works** - -```bash -cd extensions/streaming && GOWORK=off go test -run TestTransportKindsMirrorTheConstants -v ./ 2>&1 | grep -i skip -``` - -To exercise it, temporarily rename the file (`git stash` is not appropriate here — it would take the parallel workstream's changes too): - -```bash -mv packages/client-core/src/streaming.ts /tmp/streaming.ts.bak -cd extensions/streaming && GOWORK=off go test -run TestTransportKindsMirrorTheConstants -v ./ -mv /tmp/streaming.ts.bak packages/client-core/src/streaming.ts -``` - -Expected: `--- SKIP` with the reason, not a failure. Confirm the file is restored with `git status --short packages/client-core/src/streaming.ts` showing nothing. - -- [ ] **Step 6: Format, vet and commit** - -```bash -cd extensions/streaming && gofmt -l frame_test.go && GOWORK=off go vet ./ -``` - -```bash -git add extensions/streaming/frame_test.go -git commit -m "test(streaming): read the client's reserved kinds from its source - -The mirror was a snapshot of the TypeScript set pinned inside a Go file, -so it only ever caught Go drifting away from it. Editing TRANSPORT_KINDS -in streaming.ts broke no test at all. Reading the file makes the check -bidirectional, and skips when the client package is absent -- this module -is publishable without the repository around it." -``` - ---- - -### Task 4: Let an empty `channel_id` fall through to `channel` - -**Files:** -- Modify: `packages/client-core/src/streaming.ts:131` -- Test: `packages/client-core/__tests__/streaming.test.ts` (add to the `channel resolution` describe block) - -**Interfaces:** -- Consumes: `forgeStreamingDecoder(options?)` as committed. -- Produces: no signature change. - -**Why:** `envelope['channel_id'] ?? envelope['channel']` coalesces on null and undefined, not on the empty string. A frame spelling `{"channel_id": "", "channel": "orders"}` takes the empty `channel_id`, fails the non-empty check, and loses the `channel` it did carry. Unreachable from the Go extension, whose `ChannelID` is `omitempty`, and reachable from any hand-rolled server that emits the field unconditionally. Low severity, one line. - -- [ ] **Step 1: Write the failing test** - -Add inside the `describe('channel resolution', ...)` block in `packages/client-core/__tests__/streaming.test.ts`: - -```ts - // `??` coalesces on null and undefined, not on the empty string, so an - // envelope spelling channel_id unconditionally used to swallow the `channel` - // it did carry. Go's ChannelID is omitempty and never produces this; a - // hand-rolled server that always emits the field does. - it('falls through an empty channel_id to channel', () => { - const decode = forgeStreamingDecoder({ - channelOf: (id) => (id === 'orders' ? '/ws/orders' : undefined), - }); - - const decoded = decode({ type: 'message', event: 'order.created', channel_id: '', channel: 'orders', data: { id: 9 } }); - - expect(decoded?.channel).toBe('/ws/orders'); - }); -``` - -- [ ] **Step 2: Run it and watch it fail** - -```bash -cd packages/client-core && npx vitest run __tests__/streaming.test.ts -t 'falls through an empty channel_id' -``` - -Expected: FAIL — `expected undefined to be '/ws/orders'`. - -- [ ] **Step 3: Fix the coalescing** - -In `packages/client-core/src/streaming.ts`, replace the `channelID` line: - -```ts - const named = envelope['channel_id']; - const channelID = typeof named === 'string' && named !== '' ? named : envelope['channel']; -``` - -- [ ] **Step 4: Run the suite** - -```bash -cd packages/client-core && npm test -``` - -Expected: PASS, all files. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/src/streaming.ts packages/client-core/__tests__/streaming.test.ts -git commit -m "fix(client-core): fall through an empty channel_id to channel - -?? coalesces on null and undefined, not on the empty string, so a server -emitting channel_id unconditionally lost the channel it did carry. Go's -ChannelID is omitempty and never produces this shape; a hand-rolled -server does." -``` - ---- - -### Task 5 (optional): Say in `NewEventMessage` that a reserved name is unbindable - -**Files:** -- Modify: `extensions/streaming/frame.go` (the `NewEventMessage` doc comment only) -- Test: `extensions/streaming/frame_test.go` - -**Interfaces:** -- Consumes: `streaming.IsTransportKind(kind string) bool` as committed. -- Produces: no signature change. - -**Why, and why it is optional:** `NewEventMessage("presence", data)` builds a frame whose `event` collides with a reserved transport kind. `IsTransportKind` exists to detect this and nothing calls it. The severity is lower than it first looks: because `event` is non-empty, the client takes the `event` branch and the frame is *reported* through `onUnknown` rather than silently dropped — so the mistake is visible in development. That is why this is a doc change and a test rather than a signature change to a constructor with no consumers yet. **Skip this task if you disagree that it earns its keep.** - -- [ ] **Step 1: Write the test** - -Add to `extensions/streaming/frame_test.go`: - -```go -// TestNewEventMessageAcceptsAReservedName pins the deliberate absence of a -// guard. A domain name colliding with a transport kind is a producer mistake, -// but it is a visible one -- the client takes the event branch, finds no -// binding, and reports it -- so the constructor documents the collision and -// leaves IsTransportKind to the caller who wants to check. -func TestNewEventMessageAcceptsAReservedName(t *testing.T) { - msg := streaming.NewEventMessage(streaming.MessageTypePresence, nil) - - if msg.Event != streaming.MessageTypePresence { - t.Errorf("Event = %q, want the name it was given", msg.Event) - } - - if !streaming.IsTransportKind(msg.Event) { - t.Error("IsTransportKind is the check a producer runs to catch this") - } -} -``` - -- [ ] **Step 2: Run it** - -```bash -cd extensions/streaming && GOWORK=off go test -run TestNewEventMessageAcceptsAReservedName ./ -``` - -Expected: PASS immediately — this pins existing behaviour rather than driving a change. - -- [ ] **Step 3: Add the paragraph to the godoc** - -In `extensions/streaming/frame.go`, append to the `NewEventMessage` doc comment, before the closing line: - -```go -// An event whose name collides with a reserved transport kind is accepted and -// is a mistake: the client will look for a binding named "presence" and find -// none. It is not rejected here because the failure is visible -- an event name -// always takes the client's event branch, so the frame is reported rather than -// dropped -- and because a constructor that can fail is a worse trade than a -// caller running IsTransportKind when the name is not a literal. -``` - -- [ ] **Step 4: Format, vet, commit** - -```bash -cd extensions/streaming && gofmt -l frame.go frame_test.go && GOWORK=off go vet ./ -``` - -```bash -git add extensions/streaming/frame.go extensions/streaming/frame_test.go -git commit -m "docs(streaming): say that a reserved event name is unbindable - -NewEventMessage accepts one, and the collision is a producer mistake that -IsTransportKind exists to catch. Documented rather than rejected: the -failure is visible on the client, and a constructor that can fail is the -worse trade." -``` - ---- - -## Not in this plan - -- **`@types/node`** — environmental, handled in Preflight, not a code change. -- **The parallel workstream's test files** (`hooks_delivery_pool_test.go`, `manager_test.go`) breaking the streaming test binary. Not ours; verify with `GOWORK=off go build ./...` and wait. -- **Generator-emitted binder wiring.** No generator emits any `StreamBinder` or `SubscriptionManager` construction today — this was checked against `internal/client/generators/`. Making the generator produce stream wiring is new surface area and a separate design question, not a follow-up to this fix. Task 1 removes the reason it felt urgent. -- **The branch mixing** on `fix/streaming-frame-decoder`. Cherry-pick `cb773c34` and `5463788c` onto a fresh branch off `95e55b70` once the parallel session is idle, if you want them separated. diff --git a/docs/superpowers/plans/2026-08-08-unified-streams-hooks-generation.md b/docs/superpowers/plans/2026-08-08-unified-streams-hooks-generation.md deleted file mode 100644 index c9144174..00000000 --- a/docs/superpowers/plans/2026-08-08-unified-streams-hooks-generation.md +++ /dev/null @@ -1,1428 +0,0 @@ -# Unified Streams + Hooks Generation Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make `forge client generate` accept several specification documents and emit one package containing REST operations, hooks, and stream clients with a populated `streams` manifest. - -**Architecture:** Parse each source into an `APISpec` with entity resolution deferred, merge the results with OpenAPI authoritative on collisions, then resolve entity fields once over the merged whole. The IR already carries both halves and the TypeScript generator already gates emission per section on the data, so no generator changes are needed. - -**Tech Stack:** Go 1.26, stdlib `testing` (no testify anywhere in `internal/client`), `gopkg.in/yaml.v3`, `fsnotify`. - -## Global Constraints - -- Test package for `internal/client` tests is `package client_test`, using stdlib `testing` with `t.Fatalf`/`t.Errorf`. **No testify** — all 14 existing test files in that package are stdlib-only. -- `MergeSpecs` output must be deterministic: identical sources produce byte-identical results, and reordering sources **of differing document kinds** does not change the output. Precedence among sources **of the same kind** follows argument order — a user who lists two OpenAPI files has expressed an order, and inventing a tiebreaker from their titles would override that with something less predictable. This is a promise, not an accident: it must be pinned by test. `internal/client/generators/typescript/determinism_test.go` exists and must keep passing. -- Single-source behaviour must not change. A lone OpenAPI source still produces `ops.ts`/`hooks.ts`/`rest.ts`; a lone AsyncAPI source still produces `websocket.ts`/`events.ts` with `isAsyncAPIOnly` true. -- Existing scalar `path:` / `url:` keys in `.forge-client.yml` must keep working. -- Warnings go through the existing `spec.Warnings` field. Do not add a second warning channel. -- Run `GOWORK=off` is **not** needed here — `internal/client` is in the root module, which `go.work` includes. -- **Commit with `git commit --only -m "..."`. Never `git add` at all, and never `git add -A` / `git add .` / `git commit -a`.** - - This work happens on `fix/streaming-frame-decoder`, and **another live session is committing to the same branch and the same working directory concurrently.** A shared working directory means a shared index. `git add` and `git commit` are separate operations, so between your `add` and your `commit` the other session's `add` can stage their files into the same index — and their `commit` can consume yours. Both of those actually happened during Task 1, producing a commit that carries one session's message over the other's content. - - `git commit --only ` builds the commit from exactly the named paths, ignoring whatever else is in the index. That is the mitigation. Every commit step below lists its paths; pass exactly those to `--only`. - - If a commit fails or produces an unexpected result, **stop and report it — do not attempt recovery with `git commit --amend`, `git reset`, or `git rebase`.** An amend during Task 1 landed on the other session's commit and destroyed its message irrecoverably. Recovery on a shared branch is the coordinator's job, not yours. -- Touch nothing under `extensions/streaming/`. No task in this plan has any business there. - -## File Structure - -| File | Responsibility | -|---|---| -| `internal/client/merge.go` | **New.** `SourceKind`, `MergeSpecs`, collision policy. The only genuinely new logic. | -| `internal/client/merge_test.go` | **New.** Unit tests for merge semantics. | -| `internal/client/ir.go` | Add `Kind SourceKind` to `APISpec`. | -| `internal/client/spec_parser.go` | Split `ParseFile` into `parseDocument` (no resolution) + `ParseFile` (parse + resolve). Set `Kind`. | -| `internal/client/introspector.go` | Set `Kind = SourceIntrospection`. | -| `cmd/forge/plugins/client_config.go` | `SourceConfig.Sources []SourceEntry`, with scalar back-compat. | -| `cmd/forge/plugins/client.go` | `generationPlan` carries source lists; generate merges. | -| `cmd/forge/plugins/client_watch.go` | Watch every file source. | - ---- - -### Task 1: `SourceKind` and `MergeSpecs` unions - -**Files:** -- Create: `internal/client/merge.go` -- Create: `internal/client/merge_test.go` -- Modify: `internal/client/ir.go:6-53` (add one field to `APISpec`) - -**Interfaces:** -- Consumes: `APISpec`, `Server`, `Tag`, `SecurityScheme`, `Schema`, `EntityRef` from `internal/client/ir.go`. -- Produces: `type SourceKind int`; constants `SourceUnknown`, `SourceOpenAPI`, `SourceAsyncAPI`, `SourceIntrospection`; `func MergeSpecs(specs ...*APISpec) *APISpec`; field `APISpec.Kind SourceKind`. - -- [ ] **Step 1: Add the `Kind` field to `APISpec`** - -In `internal/client/ir.go`, inside the `APISpec` struct, after the `Streaming *StreamingSpec` field: - -```go - // Kind records which document family this spec was parsed from. MergeSpecs - // orders sources by this rather than by argument order, so that - // `--from-spec a.json --from-spec b.json` and the reverse produce identical - // output. A spec built by Introspector carries SourceIntrospection and - // ranks with OpenAPI, because it is authoritative for REST the same way. - Kind SourceKind -``` - -- [ ] **Step 2: Write the failing test** - -Create `internal/client/merge_test.go`: - -```go -package client_test - -import ( - "testing" - - "github.com/xraph/forge/internal/client" -) - -func restSpec() *client.APISpec { - return &client.APISpec{ - Kind: client.SourceOpenAPI, - Info: client.APIInfo{Title: "Orders", Version: "1.0.0"}, - Servers: []client.Server{{URL: "https://api.example.com"}}, - Endpoints: []client.Endpoint{{OperationID: "listOrders", Path: "/orders", Method: "GET"}}, - Schemas: map[string]*client.Schema{"Order": {Type: "object"}}, - Entities: map[string]*client.EntityRef{"Order": {Type: "Order", IDField: "id"}}, - Tags: []client.Tag{{Name: "orders"}}, - } -} - -func streamSpec() *client.APISpec { - return &client.APISpec{ - Kind: client.SourceAsyncAPI, - Info: client.APIInfo{Title: "Orders Streams", Version: "2.0.0"}, - Servers: []client.Server{{URL: "wss://api.example.com"}}, - WebSockets: []client.WebSocketEndpoint{{Path: "/ws/orders"}}, - Schemas: map[string]*client.Schema{"OrderEvent": {Type: "object"}}, - Tags: []client.Tag{{Name: "orders"}}, - } -} - -func TestMergeSpecsNilAndEmpty(t *testing.T) { - if got := client.MergeSpecs(); got != nil { - t.Fatalf("MergeSpecs() with no specs = %v, want nil", got) - } - if got := client.MergeSpecs(nil, nil); got != nil { - t.Fatalf("MergeSpecs(nil, nil) = %v, want nil", got) - } -} - -func TestMergeSpecsSingleSpecIsIdentity(t *testing.T) { - in := restSpec() - got := client.MergeSpecs(in) - if got != in { - t.Fatalf("MergeSpecs(one) must return that same spec unchanged") - } -} - -func TestMergeSpecsUnionsEndpointsAndStreams(t *testing.T) { - got := client.MergeSpecs(restSpec(), streamSpec()) - - if len(got.Endpoints) != 1 || got.Endpoints[0].OperationID != "listOrders" { - t.Errorf("Endpoints = %v, want the one REST endpoint", got.Endpoints) - } - if len(got.WebSockets) != 1 || got.WebSockets[0].Path != "/ws/orders" { - t.Errorf("WebSockets = %v, want the one stream endpoint", got.WebSockets) - } - if len(got.Schemas) != 2 { - t.Errorf("Schemas has %d entries, want 2 (Order, OrderEvent)", len(got.Schemas)) - } - if len(got.Servers) != 2 { - t.Errorf("Servers has %d entries, want 2 distinct URLs", len(got.Servers)) - } - if len(got.Tags) != 1 { - t.Errorf("Tags has %d entries, want 1 after dedup by name", len(got.Tags)) - } - if got.Info.Title != "Orders" { - t.Errorf("Info.Title = %q, want the OpenAPI document's title", got.Info.Title) - } - if got.RoutingTypes != nil { - t.Errorf("RoutingTypes must be nil after merge; resolveEntityFields rebuilds it") - } -} - -func TestMergeSpecsOrdersByDocumentKindNotArgumentOrder(t *testing.T) { - forward := client.MergeSpecs(restSpec(), streamSpec()) - reverse := client.MergeSpecs(streamSpec(), restSpec()) - - if forward.Info.Title != reverse.Info.Title { - t.Errorf("Info.Title differs by argument order: %q vs %q", forward.Info.Title, reverse.Info.Title) - } - if len(forward.Endpoints) != len(reverse.Endpoints) { - t.Errorf("Endpoints count differs by argument order") - } - if forward.Servers[0].URL != reverse.Servers[0].URL { - t.Errorf("Servers order differs by argument order: %q vs %q", - forward.Servers[0].URL, reverse.Servers[0].URL) - } -} -``` - -- [ ] **Step 3: Run the test to verify it fails** - -Run: `go test ./internal/client/ -run TestMergeSpecs -v` -Expected: FAIL to build — `undefined: client.MergeSpecs`, `undefined: client.SourceOpenAPI`, and `unknown field Kind`. - -- [ ] **Step 4: Write the implementation** - -Create `internal/client/merge.go`: - -```go -package client - -import "sort" - -// SourceKind records which document family a specification was parsed from. -type SourceKind int - -const ( - // SourceUnknown is a spec built by something that did not say. It ranks - // last, so it can never silently outrank a real REST document. - SourceUnknown SourceKind = iota - SourceOpenAPI - SourceAsyncAPI - SourceIntrospection -) - -// mergeRank orders sources for a merge. OpenAPI and introspection are -// authoritative for shared types because they carry full request and response -// schemas; AsyncAPI fills only what is absent. -func mergeRank(k SourceKind) int { - switch k { - case SourceOpenAPI, SourceIntrospection: - return 0 - case SourceAsyncAPI: - return 1 - default: - return 2 - } -} - -// MergeSpecs combines parsed specifications into one. -// -// Sources are ordered by document kind rather than by the order they were -// passed, so that `--from-spec async.json --from-spec openapi.json` and its -// reverse produce identical output. Precedence is a property of what a document -// is, not of what order somebody typed. -// -// The result's RoutingTypes is left nil: resolveEntityFields is its only writer -// and rebuilds it from scratch, and merging two pre-built maps would break the -// invariant that RoutingTypes and Entities are disjoint. The caller must run -// resolveEntityFields on the result. -// -// Merging a single spec returns that spec unchanged, so the single-source path -// costs nothing and cannot drift from the multi-source one. -func MergeSpecs(specs ...*APISpec) *APISpec { - ordered := make([]*APISpec, 0, len(specs)) - for _, s := range specs { - if s != nil { - ordered = append(ordered, s) - } - } - - switch len(ordered) { - case 0: - return nil - case 1: - return ordered[0] - } - - sort.SliceStable(ordered, func(i, j int) bool { - return mergeRank(ordered[i].Kind) < mergeRank(ordered[j].Kind) - }) - - out := &APISpec{ - Info: ordered[0].Info, - Kind: ordered[0].Kind, - Schemas: make(map[string]*Schema), - Entities: make(map[string]*EntityRef), - } - - seenServer := make(map[string]bool) - seenTag := make(map[string]bool) - seenScheme := make(map[string]bool) - - for _, s := range ordered { - out.Endpoints = append(out.Endpoints, s.Endpoints...) - out.WebSockets = append(out.WebSockets, s.WebSockets...) - out.SSEs = append(out.SSEs, s.SSEs...) - out.WebTransports = append(out.WebTransports, s.WebTransports...) - out.Warnings = append(out.Warnings, s.Warnings...) - - for _, srv := range s.Servers { - if !seenServer[srv.URL] { - seenServer[srv.URL] = true - out.Servers = append(out.Servers, srv) - } - } - for _, tag := range s.Tags { - if !seenTag[tag.Name] { - seenTag[tag.Name] = true - out.Tags = append(out.Tags, tag) - } - } - for _, sec := range s.Security { - if !seenScheme[sec.Name] { - seenScheme[sec.Name] = true - out.Security = append(out.Security, sec) - } - } - - for name, schema := range s.Schemas { - if _, taken := out.Schemas[name]; !taken { - out.Schemas[name] = schema - } - } - for name, ent := range s.Entities { - if _, taken := out.Entities[name]; !taken { - out.Entities[name] = ent - } - } - - if out.Streaming == nil { - out.Streaming = s.Streaming - } - } - - return out -} -``` - -- [ ] **Step 5: Run the test to verify it passes** - -Run: `go test ./internal/client/ -run TestMergeSpecs -v` -Expected: PASS, all four tests. - -- [ ] **Step 6: Verify nothing else broke** - -Run: `go build ./... && go test ./internal/client/...` -Expected: PASS. - -- [ ] **Step 7: Commit** - -```bash -git add internal/client/merge.go internal/client/merge_test.go internal/client/ir.go -git commit -m "feat(client): add MergeSpecs and SourceKind - -Combines parsed specifications into one, ordered by document kind rather -than argument order so precedence is a property of the document. Leaves -RoutingTypes nil for resolveEntityFields to rebuild." -``` - ---- - -### Task 2: Collision policy and warnings - -**Files:** -- Modify: `internal/client/merge.go` (the two map loops from Task 1) -- Modify: `internal/client/merge_test.go` (add cases) - -**Interfaces:** -- Consumes: `MergeSpecs` from Task 1. -- Produces: no new exported names. `MergeSpecs` now appends collision warnings to `out.Warnings`. - -- [ ] **Step 1: Write the failing test** - -Append to `internal/client/merge_test.go`: - -```go -func hasWarningContaining(warnings []string, substr string) bool { - for _, w := range warnings { - if strings.Contains(w, substr) { - return true - } - } - return false -} - -func TestMergeSpecsIdenticalRedeclarationIsSilent(t *testing.T) { - a := restSpec() - b := streamSpec() - // Same name, structurally identical: the normal case, not a conflict. - b.Schemas["Order"] = &client.Schema{Type: "object"} - - got := client.MergeSpecs(a, b) - - if hasWarningContaining(got.Warnings, "Order") { - t.Errorf("identical redeclaration must not warn, got %v", got.Warnings) - } -} - -func TestMergeSpecsWarnsOnDifferingSchemaShape(t *testing.T) { - a := restSpec() - b := streamSpec() - b.Schemas["Order"] = &client.Schema{Type: "string"} // genuinely different - - got := client.MergeSpecs(a, b) - - if got.Schemas["Order"].Type != "object" { - t.Errorf("Schemas[Order].Type = %q, want the OpenAPI shape %q", - got.Schemas["Order"].Type, "object") - } - if !hasWarningContaining(got.Warnings, "Order") { - t.Errorf("differing schema shape must warn, got %v", got.Warnings) - } -} - -func TestMergeSpecsWarnsOnDifferingEntityIDField(t *testing.T) { - a := restSpec() - b := streamSpec() - b.Entities = map[string]*client.EntityRef{ - "Order": {Type: "Order", IDField: "orderId"}, - } - - got := client.MergeSpecs(a, b) - - if got.Entities["Order"].IDField != "id" { - t.Errorf("Entities[Order].IDField = %q, want the OpenAPI value %q", - got.Entities["Order"].IDField, "id") - } - if !hasWarningContaining(got.Warnings, "orderId") { - t.Errorf("differing IDField must warn naming both values, got %v", got.Warnings) - } -} - -func TestMergeSpecsWarnsOnDuplicateRoute(t *testing.T) { - a := restSpec() - b := restSpec() - b.Info.Title = "Second" - - got := client.MergeSpecs(a, b) - - if !hasWarningContaining(got.Warnings, "GET /orders") { - t.Errorf("duplicate path+method must warn, got %v", got.Warnings) - } -} -``` - -Add `"strings"` to that file's imports. - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `go test ./internal/client/ -run TestMergeSpecs -v` -Expected: FAIL — the three warning assertions report empty `Warnings`. - -- [ ] **Step 3: Add schema equivalence and collision reporting** - -In `internal/client/merge.go`, add to the imports `"fmt"`, then replace the two map loops and add the route check. The loop body inside `for _, s := range ordered` becomes: - -```go - for name, schema := range s.Schemas { - existing, taken := out.Schemas[name] - if !taken { - out.Schemas[name] = schema - continue - } - if !sameSchemaShape(existing, schema) { - out.Warnings = append(out.Warnings, fmt.Sprintf( - "schema %q is declared differently in two sources; keeping the %s definition (type %q) and ignoring the %s one (type %q)", - name, kindName(out.Kind), schemaType(existing), kindName(s.Kind), schemaType(schema))) - } - } - for name, ent := range s.Entities { - existing, taken := out.Entities[name] - if !taken { - out.Entities[name] = ent - continue - } - if existing.IDField != ent.IDField { - out.Warnings = append(out.Warnings, fmt.Sprintf( - "entity %q has id field %q in the %s source and %q in the %s source; keeping %q", - name, existing.IDField, kindName(out.Kind), - ent.IDField, kindName(s.Kind), existing.IDField)) - } - } -``` - -And after the source loop, before `return out`: - -```go - seenRoute := make(map[string]bool) - for _, ep := range out.Endpoints { - key := ep.Method + " " + ep.Path - if seenRoute[key] { - out.Warnings = append(out.Warnings, fmt.Sprintf( - "route %q is declared in more than one source; the first declaration wins", key)) - continue - } - seenRoute[key] = true - } -``` - -Then add these helpers at the bottom of the file: - -```go -// sameSchemaShape reports whether two schemas describe the same thing closely -// enough that declaring both is not a conflict. It compares the structural -// fields only: descriptions and examples differ freely between a REST document -// and a stream document describing one type, and warning about those would -// train the reader to ignore the warning that matters. -func sameSchemaShape(a, b *Schema) bool { - if a == nil || b == nil { - return a == b - } - if a.Type != b.Type || a.Format != b.Format || a.Nullable != b.Nullable { - return false - } - if len(a.Properties) != len(b.Properties) || len(a.Required) != len(b.Required) { - return false - } - for name, av := range a.Properties { - bv, ok := b.Properties[name] - if !ok || !sameSchemaShape(av, bv) { - return false - } - } - required := make(map[string]bool, len(a.Required)) - for _, r := range a.Required { - required[r] = true - } - for _, r := range b.Required { - if !required[r] { - return false - } - } - return sameSchemaShape(a.Items, b.Items) -} - -func schemaType(s *Schema) string { - if s == nil { - return "" - } - return s.Type -} - -func kindName(k SourceKind) string { - switch k { - case SourceOpenAPI: - return "OpenAPI" - case SourceAsyncAPI: - return "AsyncAPI" - case SourceIntrospection: - return "introspected" - default: - return "unknown" - } -} -``` - -- [ ] **Step 4: Run the test to verify it passes** - -Run: `go test ./internal/client/ -run TestMergeSpecs -v` -Expected: PASS, all eight tests. - -- [ ] **Step 5: Commit** - -```bash -git add internal/client/merge.go internal/client/merge_test.go -git commit -m "feat(client): report merge collisions through spec.Warnings - -OpenAPI wins on a schema or entity declared in two sources. Structurally -identical redeclaration is silent -- it is the normal case -- so the -warning only fires on a genuine disagreement." -``` - ---- - -### Task 3: Defer entity resolution - -**Files:** -- Modify: `internal/client/spec_parser.go:28-75` -- Modify: `internal/client/introspector.go:64` -- Create: `internal/client/merge_resolve_test.go` - -**Interfaces:** -- Consumes: `MergeSpecs` from Task 1, `resolveEntityFields(spec *APISpec)` from `internal/client/entity_fields.go:62`. -- Produces: `func (p *SpecParser) ParseFileUnresolved(ctx context.Context, filePath string) (*APISpec, error)`. `ParseFile` keeps its existing signature and behaviour. - -**Why this task exists.** An earlier draft of this plan claimed the reason was warnings — that resolving a half-populated spec reports entities living in the other document as unresolvable stream bindings. **That claim was false and has been removed.** `resolveEntityFields` never writes to `spec.Warnings` (`grep -n Warnings internal/client/entity_fields.go` returns nothing). Those warnings come from `registerStreamBindingEntities`, called at `spec_parser.go:798` and `:821` *during parsing*, so when resolution runs cannot affect them. - -The honest reason is narrower. `resolveEntityFields` is idempotent — it replaces each entity's `Fields` and rebuilds `RoutingTypes` from scratch — so parsing-with-resolution and then re-resolving after the merge is equally correct. Deferring resolution avoids doing that work once per document and then discarding it, and it puts resolution at the point where the full set of schemas is actually known, which is where a reader expects it. This is a clarity and wasted-work improvement, not a correctness requirement. - -**Do not write a test asserting that deferral suppresses warnings.** It does not, and such a test can only pass vacuously — which is exactly what happened: the original assertion searched for the substring `"no schema describes"`, which appears nowhere in this codebase outside doc comments. The real message is `"...which has no matching schema component; this binding will not normalize"`. Test the property the split actually has: entity field edges spanning two documents resolve correctly over the merged spec. - -- [ ] **Step 1: Write the failing test** - -Create `internal/client/merge_resolve_test.go`: - -```go -package client_test - -import ( - "context" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/xraph/forge/internal/client" -) - -// writeSpec writes content to a temp file with the given name and returns its path. -func writeSpec(t *testing.T, name, content string) string { - t.Helper() - path := filepath.Join(t.TempDir(), name) - if err := os.WriteFile(path, []byte(content), 0o644); err != nil { - t.Fatalf("write %s: %v", name, err) - } - return path -} - -const restDoc = ` -openapi: 3.1.0 -info: - title: Orders - version: 1.0.0 -paths: - /orders: - get: - operationId: listOrders - responses: - '200': - description: ok - content: - application/json: - schema: - $ref: '#/components/schemas/Order' -components: - schemas: - Order: - type: object - x-forge-entity: - idField: id - properties: - id: - type: string -` - -const streamDoc = ` -asyncapi: 3.0.0 -info: - title: Orders Streams - version: 1.0.0 -channels: - orders: - address: /ws/orders - messages: - orderUpdated: - payload: - $ref: '#/components/schemas/OrderEvent' -components: - schemas: - OrderEvent: - type: object - properties: - id: - type: string -` - -func TestParseFileUnresolvedLeavesRoutingTypesUnbuilt(t *testing.T) { - p := client.NewSpecParser() - path := writeSpec(t, "openapi.yaml", restDoc) - - spec, err := p.ParseFileUnresolved(context.Background(), path) - if err != nil { - t.Fatalf("ParseFileUnresolved: %v", err) - } - if spec.RoutingTypes != nil { - t.Errorf("RoutingTypes = %v, want nil before resolution", spec.RoutingTypes) - } - if spec.Kind != client.SourceOpenAPI { - t.Errorf("Kind = %v, want SourceOpenAPI", spec.Kind) - } -} - -func TestParseFileStillResolves(t *testing.T) { - p := client.NewSpecParser() - path := writeSpec(t, "openapi.yaml", restDoc) - - spec, err := p.ParseFile(context.Background(), path) - if err != nil { - t.Fatalf("ParseFile: %v", err) - } - if spec.Entities["Order"] == nil { - t.Fatalf("Entities[Order] missing after ParseFile") - } - if spec.Kind != client.SourceOpenAPI { - t.Errorf("Kind = %v, want SourceOpenAPI", spec.Kind) - } -} - -func TestUnresolvedParseThenMergeCarriesNoSpuriousWarnings(t *testing.T) { - p := client.NewSpecParser() - restPath := writeSpec(t, "openapi.yaml", restDoc) - streamPath := writeSpec(t, "asyncapi.yaml", streamDoc) - - rest, err := p.ParseFileUnresolved(context.Background(), restPath) - if err != nil { - t.Fatalf("parse rest: %v", err) - } - stream, err := p.ParseFileUnresolved(context.Background(), streamPath) - if err != nil { - t.Fatalf("parse stream: %v", err) - } - - merged := client.MergeSpecs(rest, stream) - client.ResolveEntityFieldsForTest(merged) - - for _, w := range merged.Warnings { - if strings.Contains(w, "no schema describes") { - t.Errorf("merged spec carries a spurious unresolved-entity warning: %q", w) - } - } - if len(merged.Endpoints) == 0 { - t.Errorf("merged spec lost its REST endpoints") - } - if len(merged.WebSockets) == 0 { - t.Errorf("merged spec lost its stream endpoints") - } -} -``` - -- [ ] **Step 2: Add the test-only resolution export** - -`resolveEntityFields` is unexported and the tests are in `package client_test`. Create `internal/client/export_test.go`: - -```go -package client - -// ResolveEntityFieldsForTest exposes resolveEntityFields to the external test -// package. Test-only: this file is not compiled into the package binary. -func ResolveEntityFieldsForTest(spec *APISpec) { resolveEntityFields(spec) } -``` - -- [ ] **Step 3: Run the test to verify it fails** - -Run: `go test ./internal/client/ -run "TestParseFile|TestUnresolvedParse" -v` -Expected: FAIL to build — `p.ParseFileUnresolved undefined`. - -- [ ] **Step 4: Split the parser** - -In `internal/client/spec_parser.go`, replace the body of `ParseFile` (lines 28-75) with: - -```go -// ParseFile parses a specification file and resolves entity field edges. -// This is the single-source path and its behaviour is unchanged. -func (p *SpecParser) ParseFile(ctx context.Context, filePath string) (*APISpec, error) { - spec, err := p.ParseFileUnresolved(ctx, filePath) - if err != nil { - return nil, err - } - resolveEntityFields(spec) - return spec, nil -} - -// ParseFileUnresolved parses a specification file without resolving entity -// field edges. -// -// A merge of several documents must resolve once over the merged whole, not -// once per document. Resolving a half-populated spec does not merely waste -// work: it reports every entity that lives in the *other* document as a stream -// binding naming a type no schema describes, and those warnings would survive -// into the merged result and describe a correct pair of documents as broken. -// -// The caller is responsible for calling resolveEntityFields, directly or via -// ParseFile. -func (p *SpecParser) ParseFileUnresolved(ctx context.Context, filePath string) (*APISpec, error) { - data, err := os.ReadFile(filePath) - if err != nil { - return nil, fmt.Errorf("read spec file: %w", err) - } - - ext := strings.ToLower(filepath.Ext(filePath)) - isYAML := ext == ".yaml" || ext == ".yml" - - specType, err := p.detectSpecType(data, isYAML) - if err != nil { - return nil, fmt.Errorf("detect spec type: %w", err) - } - - var spec *APISpec - - switch specType { - case "openapi": - spec, err = p.parseOpenAPI(data, isYAML) - if spec != nil { - spec.Kind = SourceOpenAPI - } - case "asyncapi": - spec, err = p.parseAsyncAPI(data, isYAML) - if spec != nil { - spec.Kind = SourceAsyncAPI - } - default: - return nil, fmt.Errorf("unknown spec type: %s", specType) - } - - if err != nil { - return nil, err - } - - return spec, nil -} -``` - -Keep the existing `ctx` parameter unused exactly as it is today — do not rename it, and do not add a `_ = ctx`. - -- [ ] **Step 5: Set `Kind` on the introspector path** - -In `internal/client/introspector.go`, at line 64 where `resolveEntityFields(spec)` is called, add immediately before it: - -```go - spec.Kind = SourceIntrospection -``` - -- [ ] **Step 6: Run the tests to verify they pass** - -Run: `go test ./internal/client/ -run "TestParseFile|TestUnresolvedParse" -v` -Expected: PASS, all three tests. - -- [ ] **Step 7: Run the full package to catch regressions** - -Run: `go test ./internal/client/...` -Expected: PASS. The existing parser tests exercise `ParseFile`, which still resolves. - -- [ ] **Step 8: Commit** - -```bash -git add internal/client/spec_parser.go internal/client/introspector.go internal/client/export_test.go internal/client/merge_resolve_test.go -git commit -m "feat(client): add ParseFileUnresolved for multi-source merging - -Resolving a half-populated spec reports every entity living in the other -document as an unresolvable stream binding, and those warnings survive -the merge. ParseFile is unchanged, now composed from the two steps." -``` - ---- - -### Task 4: Multi-source configuration and CLI flags - -**Files:** -- Modify: `cmd/forge/plugins/client_config.go:50-63` -- Modify: `cmd/forge/plugins/client.go:93-94,114-115` -- Create: `cmd/forge/plugins/client_sources_test.go` - -**Interfaces:** -- Consumes: nothing from earlier tasks. -- Produces: `type SourceEntry struct { Type string; Path string; URL string }`; field `SourceConfig.Sources []SourceEntry`; method `func (s SourceConfig) Entries() []SourceEntry`. - -- [ ] **Step 1: Write the failing test** - -Create `cmd/forge/plugins/client_sources_test.go`: - -```go -package plugins - -import "testing" - -func TestSourceEntriesFromScalarPath(t *testing.T) { - s := SourceConfig{Type: "file", Path: "openapi.json"} - - got := s.Entries() - - if len(got) != 1 { - t.Fatalf("Entries() returned %d entries, want 1", len(got)) - } - if got[0].Path != "openapi.json" || got[0].Type != "file" { - t.Errorf("Entries()[0] = %+v, want the scalar path as one file entry", got[0]) - } -} - -func TestSourceEntriesFromScalarURL(t *testing.T) { - s := SourceConfig{Type: "url", URL: "https://example.com/openapi.json"} - - got := s.Entries() - - if len(got) != 1 || got[0].URL != "https://example.com/openapi.json" { - t.Fatalf("Entries() = %+v, want the scalar URL as one entry", got) - } -} - -func TestSourceEntriesPrefersExplicitList(t *testing.T) { - s := SourceConfig{ - Type: "file", - Path: "ignored.json", - Sources: []SourceEntry{ - {Type: "file", Path: "openapi.json"}, - {Type: "file", Path: "asyncapi.json"}, - }, - } - - got := s.Entries() - - if len(got) != 2 { - t.Fatalf("Entries() returned %d entries, want 2", len(got)) - } - if got[0].Path != "openapi.json" || got[1].Path != "asyncapi.json" { - t.Errorf("Entries() = %+v, want list order preserved", got) - } -} - -func TestSourceEntriesEmptyWhenNothingConfigured(t *testing.T) { - if got := (SourceConfig{}).Entries(); len(got) != 0 { - t.Errorf("Entries() = %+v, want empty", got) - } -} -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `go test ./cmd/forge/plugins/ -run TestSourceEntries -v` -Expected: FAIL to build — `undefined: SourceEntry`, `s.Sources undefined`, `s.Entries undefined`. - -- [ ] **Step 3: Add `SourceEntry` and `Entries()`** - -In `cmd/forge/plugins/client_config.go`, replace the `SourceConfig` struct (lines 50-63) with: - -```go -// SourceEntry is one specification document to read. -type SourceEntry struct { - // Type: "file" or "url". - Type string `yaml:"type"` - - Path string `yaml:"path,omitempty"` - URL string `yaml:"url,omitempty"` -} - -// SourceConfig defines where to get the API specification. -// -// Sources is one ordered list rather than parallel path and url arrays, -// because merge precedence depends on order and parallel arrays leave the -// relative order of a file source and a URL source undefined. -type SourceConfig struct { - // Type: "file", "url", "auto" - Type string `yaml:"type"` - - // Path to spec file (when type=file). Read as a one-element Sources list - // when Sources is empty, so an existing .forge-client.yml keeps working. - Path string `yaml:"path,omitempty"` - - // URL to fetch spec (when type=url). Same one-element handling as Path. - URL string `yaml:"url,omitempty"` - - // Sources lists several documents to parse and merge. When set it wins - // over the scalar Path and URL keys above. - Sources []SourceEntry `yaml:"sources,omitempty"` - - // Auto-discovery paths (when type=auto) - AutoDiscoverPaths []string `yaml:"auto_discover_paths,omitempty"` -} - -// Entries normalises this configuration into the list of documents to read. -func (s SourceConfig) Entries() []SourceEntry { - if len(s.Sources) > 0 { - return s.Sources - } - switch { - case s.Path != "": - return []SourceEntry{{Type: "file", Path: s.Path}} - case s.URL != "": - return []SourceEntry{{Type: "url", URL: s.URL}} - default: - return nil - } -} -``` - -- [ ] **Step 4: Run the test to verify it passes** - -Run: `go test ./cmd/forge/plugins/ -run TestSourceEntries -v` -Expected: PASS, all four tests. - -- [ ] **Step 5: Make the CLI flags repeatable** - -In `cmd/forge/plugins/client.go`, at lines 93-94 and again at 114-115, replace each `cli.NewStringFlag` for `from-spec` and `from-url` with its slice equivalent: - -```go - cli.WithFlag(cli.NewStringSliceFlag("from-spec", "s", "Path to an OpenAPI/AsyncAPI spec file (repeatable)", nil)), - cli.WithFlag(cli.NewStringSliceFlag("from-url", "u", "URL to fetch an OpenAPI/AsyncAPI spec (repeatable)", nil)), -``` - -`cli.NewStringSliceFlag` exists and is already used at `cmd/forge/plugins/generate.go:83`. Read the values back with `ctx.StringSlice("from-spec")` and `ctx.StringSlice("from-url")`, the same way `generate.go:878` does. - -- [ ] **Step 6: Verify the build** - -Run: `go build ./... && go test ./cmd/forge/plugins/` -Expected: PASS. - -- [ ] **Step 7: Commit** - -```bash -git add cmd/forge/plugins/client_config.go cmd/forge/plugins/client.go cmd/forge/plugins/client_sources_test.go -git commit -m "feat(cli): accept several spec sources - -SourceConfig gains an ordered Sources list and Entries() normalises the -scalar path/url keys into it, so an existing .forge-client.yml keeps -working. --from-spec and --from-url become repeatable." -``` - ---- - -### Task 5: Merge sources in the generate path - -**Files:** -- Modify: `cmd/forge/plugins/client.go:177-188` (`generationPlan`), and its `resolveGenerationPlan` and `generateClient` -- Create: `internal/client/generators/typescript/e2e_merged_sources_test.go` - -**Interfaces:** -- Consumes: `SourceConfig.Entries()` (Task 4), `SpecParser.ParseFileUnresolved` (Task 3), `client.MergeSpecs` (Task 1). -- Produces: field `generationPlan.specPaths []string` replacing `specPath`; field `generationPlan.specURLs []string` replacing `specURL`. - -- [ ] **Step 1: Write the failing E2E test** - -`internal/client/generators/typescript/e2e_specfile_test.go` already has two helpers in `package typescript_test`, both usable from a new file in the same package: - -- `writeSpecFile(t *testing.T, name, content string) string` (line 94) -- `generateFromSpecFile(t *testing.T, path string) map[string]string` (line 109) - -`generateFromSpecFile` takes **one** path, so this task adds a plural sibling. Build it by reading the singular one and changing only the parse-and-merge portion — everything after `MergeSpecs` (generator construction, config, collecting output files) must be identical, or the E2E test stops testing the real generation path. - -Create `internal/client/generators/typescript/e2e_merged_sources_test.go`: - -```go -package typescript_test - -import ( - "context" - "strings" - "testing" - - "github.com/xraph/forge/internal/client" -) - -const mergedRestDoc = ` -openapi: 3.1.0 -info: - title: Orders - version: 1.0.0 -paths: - /orders: - get: - operationId: listOrders - responses: - '200': - description: ok - content: - application/json: - schema: - $ref: '#/components/schemas/Order' -components: - schemas: - Order: - type: object - x-forge-entity: - idField: id - properties: - id: - type: string -` - -const mergedStreamDoc = ` -asyncapi: 3.0.0 -info: - title: Orders Streams - version: 1.0.0 -channels: - orders: - address: /ws/orders - messages: - orderUpdated: - payload: - $ref: '#/components/schemas/Order' -components: - schemas: - Order: - type: object - x-forge-entity: - idField: id - properties: - id: - type: string -` - -// generateFromSpecFiles is the plural form of generateFromSpecFile: it parses -// each document without resolving, merges, resolves once, then drives the -// generator exactly as the singular helper does. -func generateFromSpecFiles(t *testing.T, paths ...string) map[string]string { - t.Helper() - - parser := client.NewSpecParser() - specs := make([]*client.APISpec, 0, len(paths)) - for _, p := range paths { - spec, err := parser.ParseFileUnresolved(context.Background(), p) - if err != nil { - t.Fatalf("parse %s: %v", p, err) - } - specs = append(specs, spec) - } - - merged := client.MergeSpecs(specs...) - if merged == nil { - t.Fatal("MergeSpecs returned nil") - } - client.ResolveEntityFields(merged) - - // Everything below must mirror generateFromSpecFile after its parse step. - return generateFromMergedSpec(t, merged) -} - -func TestMergedSourcesProduceOnePackageWithBoth(t *testing.T) { - rest := writeSpecFile(t, "openapi.yaml", mergedRestDoc) - stream := writeSpecFile(t, "asyncapi.yaml", mergedStreamDoc) - - files := generateFromSpecFiles(t, rest, stream) - - for _, want := range []string{"ops.ts", "hooks.ts", "rest.ts", "websocket.ts"} { - if _, ok := files[want]; !ok { - t.Errorf("%s was not generated from a merged pair of documents", want) - } - } -} - -// TestMergedSourcesPopulateStreamsManifest is the assertion that makes this -// feature real. A package containing both file sets while `streams` stays -// empty is exactly the bug being fixed: every file is present and -// `{ live: true }` still does nothing. -func TestMergedSourcesPopulateStreamsManifest(t *testing.T) { - rest := writeSpecFile(t, "openapi.yaml", mergedRestDoc) - stream := writeSpecFile(t, "asyncapi.yaml", mergedStreamDoc) - - files := generateFromSpecFiles(t, rest, stream) - - ops, ok := files["ops.ts"] - if !ok { - t.Fatal("ops.ts was not generated from a merged spec") - } - if strings.Contains(ops, "streams: {}") { - t.Errorf("ops.ts carries an empty streams table; { live: true } would do nothing") - } - if !strings.Contains(ops, "/ws/orders") { - t.Errorf("ops.ts streams table does not mention the channel from the AsyncAPI document") - } -} - -func TestSingleSpecFileStillGeneratesIdentically(t *testing.T) { - path := writeSpecFile(t, "openapi.yaml", mergedRestDoc) - - singular := generateFromSpecFile(t, path) - plural := generateFromSpecFiles(t, path) - - for name, want := range singular { - if plural[name] != want { - t.Errorf("%s differs between the single-source and merge paths", name) - } - } -} -``` - -**`generateFromMergedSpec` does not exist yet.** Extract it from `generateFromSpecFile` (line 109) as a first refactor step: split that function at its parse boundary into `generateFromSpecFile` (parse, then delegate) and `generateFromMergedSpec(t *testing.T, spec *client.APISpec) map[string]string` (everything after). `TestSingleSpecFileStillGeneratesIdentically` above is what proves the extraction was faithful. - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `go test ./internal/client/generators/typescript/ -run TestMergedSources -v` -Expected: FAIL — either on the missing helper, or on `streams: {}` if the generator is fed a spec that never carried both. - -- [ ] **Step 3: Carry source lists on the plan** - -In `cmd/forge/plugins/client.go`, replace the `generationPlan` struct (lines 177-188) with: - -```go -type generationPlan struct { - // specPaths are the local files to parse, in configuration order. A - // downloaded spec appears here as a temp file. - specPaths []string - // specURLs is set for each spec that came over HTTP; the matching entry in - // specPaths is then a temp file holding the fetched bytes. generate and - // check never look at these -- they only need the files -- but `watch` - // cannot tell a downloaded spec from a local one by its path, and polling a - // temp file that nothing ever writes to again would be a watch that can - // never fire. - specURLs []string - outputDir string - config client.GeneratorConfig - cleanup func() -} -``` - -Update `resolveGenerationPlan` to build these lists from `SourceConfig.Entries()` and from the repeatable flags, appending flag sources in argument order after config sources. Keep the existing single-source resolution logic per entry — fetching a URL to a temp file, resolving a relative path — and accumulate `cleanup` into one closure that runs every per-entry cleanup. - -- [ ] **Step 4: Merge in the generate path** - -Replace the single `parser.ParseFile` call at `cmd/forge/plugins/client.go:815` with: - -```go - parser := client.NewSpecParser() - - specs := make([]*client.APISpec, 0, len(plan.specPaths)) - for _, path := range plan.specPaths { - // Unresolved: entity edges are resolved once over the merged whole. - // Resolving per document reports every entity defined in another - // document as unresolvable. - spec, err := parser.ParseFileUnresolved(context.Background(), path) - if err != nil { - // A source that will not parse aborts the run. Skipping it would - // emit a package with a silently empty streams table, which is the - // exact failure this path exists to remove. - return nil, fmt.Errorf("parse %s: %w", path, err) - } - specs = append(specs, spec) - } - - spec := client.MergeSpecs(specs...) - if spec == nil { - return nil, errors.New("no specification sources resolved") - } - client.ResolveEntityFields(spec) - - if len(spec.Endpoints) == 0 && len(spec.WebSockets) == 0 && - len(spec.SSEs) == 0 && len(spec.WebTransports) == 0 { - return nil, errors.New("merged specification describes no endpoints and no streams") - } -``` - -- [ ] **Step 5: Export `ResolveEntityFields`** - -The generate path lives in `package plugins` and needs resolution after merging, so it cannot use the test-only export from Task 3. In `internal/client/entity_fields.go`, add above the unexported function: - -```go -// ResolveEntityFields resolves entity field edges over a specification. Call it -// once, after merging every source: see MergeSpecs, which deliberately leaves -// RoutingTypes nil for this function to rebuild. -func ResolveEntityFields(spec *APISpec) { resolveEntityFields(spec) } -``` - -Then delete `internal/client/export_test.go` created in Task 3, and change the one call in `merge_resolve_test.go` from `client.ResolveEntityFieldsForTest(merged)` to `client.ResolveEntityFields(merged)`. - -- [ ] **Step 6: Run the tests** - -Run: `go test ./internal/client/... ./cmd/forge/plugins/` -Expected: PASS, including `TestMergedSourcesPopulateStreamsManifest`. - -- [ ] **Step 7: Verify single-source output is unchanged** - -Run: `go test ./internal/client/generators/typescript/ -run "TestDeterminism|TestE2E" -v` -Expected: PASS with no golden-file changes. If a golden changed, stop — single-source output must be byte-identical. - -- [ ] **Step 8: Commit** - -**Stage explicit paths only. Never `git add -A` or `git add .`** — this branch carries unrelated in-flight work from another effort, and a blanket stage would commit someone else's files under your message. - -```bash -git rm --cached -f internal/client/export_test.go 2>/dev/null; rm -f internal/client/export_test.go -git add cmd/forge/plugins/client.go \ - internal/client/entity_fields.go \ - internal/client/merge_resolve_test.go \ - internal/client/generators/typescript/e2e_specfile_test.go \ - internal/client/generators/typescript/e2e_merged_sources_test.go -git commit -m "feat(cli): merge several spec sources into one package - -generate parses each source unresolved, merges, then resolves once, so a -REST document and a stream document produce one package with a populated -streams table. A source that fails to parse aborts rather than degrading -to a half-empty package." -``` - ---- - -### Task 6: Watch every source - -**Files:** -- Modify: `cmd/forge/plugins/client_watch.go:168-260` -- Modify: `cmd/forge/plugins/client_watch_test.go` - -**Interfaces:** -- Consumes: `generationPlan.specPaths`, `generationPlan.specURLs` (Task 5). -- Produces: `func resolveWatchSources(plan *generationPlan) ([]watchSource, error)` replacing `resolveWatchSource`. - -- [ ] **Step 1: Write the failing test** - -Append to `cmd/forge/plugins/client_watch_test.go`: - -```go -func TestResolveWatchSourcesCoversEveryFileSource(t *testing.T) { - dir := t.TempDir() - openapi := filepath.Join(dir, "openapi.json") - asyncapi := filepath.Join(dir, "asyncapi.json") - for _, p := range []string{openapi, asyncapi} { - if err := os.WriteFile(p, []byte("{}"), 0o644); err != nil { - t.Fatalf("write %s: %v", p, err) - } - } - - plan := &generationPlan{specPaths: []string{openapi, asyncapi}} - - got, err := resolveWatchSources(plan) - if err != nil { - t.Fatalf("resolveWatchSources: %v", err) - } - if len(got) != 2 { - t.Fatalf("resolveWatchSources returned %d sources, want 2", len(got)) - } -} - -func TestResolveWatchSourcesErrorsWithNoSources(t *testing.T) { - if _, err := resolveWatchSources(&generationPlan{}); err == nil { - t.Fatal("resolveWatchSources with no sources must return an error") - } -} -``` - -Ensure `os` and `path/filepath` are imported in that test file. - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `go test ./cmd/forge/plugins/ -run TestResolveWatchSources -v` -Expected: FAIL to build — `undefined: resolveWatchSources`. - -- [ ] **Step 3: Implement the plural resolver** - -In `cmd/forge/plugins/client_watch.go`, replace `resolveWatchSource` with: - -```go -// resolveWatchSources decides what the plan's spec sources mean for a watcher. -// Every source is watched: a client generated from two documents is stale when -// either changes, and watching only the first would rebuild on a REST edit and -// sit still on a stream edit. -func resolveWatchSources(plan *generationPlan) ([]watchSource, error) { - sources := make([]watchSource, 0, len(plan.specPaths)) - - for i, path := range plan.specPaths { - if i < len(plan.specURLs) && plan.specURLs[i] != "" { - sources = append(sources, watchSource{url: plan.specURLs[i]}) - continue - } - resolved, err := filepath.Abs(path) - if err != nil { - return nil, cli.WrapError(err, "resolve spec path", cli.ExitUsageError) - } - sources = append(sources, watchSource{path: resolved}) - } - - if len(sources) == 0 { - return nil, cli.NewError("no spec source to watch", cli.ExitUsageError) - } - return sources, nil -} -``` - -Preserve every other check the original `resolveWatchSource` performed on a single path — read the function before replacing it and carry each guard into the loop body. Update the caller at line 94 to range over the returned slice, registering each with the watcher, and make `matches` be consulted for each source. - -- [ ] **Step 4: Run the tests** - -Run: `go test ./cmd/forge/plugins/ -v` -Expected: PASS, including the pre-existing watch tests. - -- [ ] **Step 5: Commit** - -```bash -git add cmd/forge/plugins/client_watch.go cmd/forge/plugins/client_watch_test.go -git commit -m "feat(cli): watch every spec source - -A client generated from two documents is stale when either changes. -Watching only the first would rebuild on a REST edit and sit still on a -stream edit." -``` - ---- - -### Task 7: Introspector as an optional source - -**Files:** -- Modify: `cmd/forge/plugins/client.go` (`resolveGenerationPlan`) -- Create: `internal/client/introspector_kind_test.go` - -**Interfaces:** -- Consumes: `client.NewIntrospector(r router.Router)` and `(*Introspector).Introspect(ctx) (*APISpec, error)` from `internal/client/introspector.go:21,26`; `MergeSpecs` (Task 1). -- Produces: no new exported names. - -- [ ] **Step 1: Write the failing test** - -Create `internal/client/introspector_kind_test.go`: - -```go -package client_test - -import ( - "testing" - - "github.com/xraph/forge/internal/client" -) - -// An introspected spec must rank with OpenAPI, not below AsyncAPI: it is -// authoritative for REST in the same way, so a merge that put it second would -// let a stream document's schema definition win over the live router's. -func TestIntrospectionRanksWithOpenAPI(t *testing.T) { - introspected := &client.APISpec{ - Kind: client.SourceIntrospection, - Info: client.APIInfo{Title: "Live"}, - Endpoints: []client.Endpoint{{OperationID: "listOrders", Path: "/orders", Method: "GET"}}, - Schemas: map[string]*client.Schema{"Order": {Type: "object"}}, - } - stream := &client.APISpec{ - Kind: client.SourceAsyncAPI, - Info: client.APIInfo{Title: "Streams"}, - WebSockets: []client.WebSocketEndpoint{{Path: "/ws/orders"}}, - Schemas: map[string]*client.Schema{"Order": {Type: "string"}}, - } - - got := client.MergeSpecs(stream, introspected) - - if got.Info.Title != "Live" { - t.Errorf("Info.Title = %q, want the introspected title", got.Info.Title) - } - if got.Schemas["Order"].Type != "object" { - t.Errorf("Schemas[Order].Type = %q, want the introspected shape", got.Schemas["Order"].Type) - } -} -``` - -- [ ] **Step 2: Run the test to verify it fails or passes** - -Run: `go test ./internal/client/ -run TestIntrospectionRanksWithOpenAPI -v` -Expected: PASS if Task 1's `mergeRank` is correct. This test pins that behaviour so a later edit to `mergeRank` cannot quietly demote introspection. If it FAILS, fix `mergeRank` — introspection must return rank 0. - -- [ ] **Step 3: Stop here — introspection is not reachable from the CLI** - -Verified: `grep -rn "router.Router" cmd/forge/` returns nothing. No CLI path holds a `router.Router`, so `Introspector.Introspect` cannot be called from `forge client generate` without first giving the CLI a way to obtain a live router — booting the app, or an in-process API. That is a separate feature with its own design, not a step in this plan. - -This task therefore delivers Steps 1-2 only: the ranking is pinned by test so that whoever wires introspection later inherits correct merge precedence rather than discovering it. Do **not** add an introspection flag or a stub router here. - -Record this in the commit message so the omission is deliberate and searchable. - -- [ ] **Step 4: Run the full suite** - -Run: `go build ./... && go test ./internal/client/... ./cmd/forge/plugins/` -Expected: PASS. - -- [ ] **Step 5: Commit** - -**Stage explicit paths only. Never `git add -A` or `git add .`** — see Task 5. - -```bash -git add internal/client/introspector_kind_test.go -git commit -m "feat(client): rank introspected specs with OpenAPI - -An introspected spec is authoritative for REST the same way an OpenAPI -document is, so it must not lose a schema collision to a stream document." -``` - ---- - -## Self-Review - -**Spec coverage.** Every section of the design maps to a task: architecture and the deferral to Task 3; `merge.go` to Tasks 1-2; the merge semantics table to Tasks 1-2; multi-source `SourceConfig` and CLI flags to Task 4; `generationPlan` lists and the generate path to Task 5; `resolveWatchSource` to Task 6; the introspector to Task 7. Error handling is covered in Task 5 Step 4 (hard errors) and Task 2 (warnings). Testing is distributed: unit in Tasks 1-2, determinism in Task 1 Step 4 and Task 5 Step 7, goldens in Task 5 Step 7, E2E in Task 5, cross-document resolution in Task 3. - -**Deviations from the spec, deliberate.** -1. The spec did not anticipate that `APISpec` records no document family. Task 1 adds `Kind SourceKind`, without which ordering by document type is not expressible. -2. The spec justified deferring resolution by cross-document edges. Task 3 records the stronger reason found in the code: `resolveEntityFields` is documented as safe to call twice, so edges alone would not require deferral — but resolving a half-populated spec emits spurious unresolvable-entity warnings that survive into the merged result. -3. Task 5 Step 5 exports `ResolveEntityFields`, replacing the test-only export added in Task 3, because `package plugins` needs it too. - -**Unknowns, resolved before the plan was finalised.** Three points would otherwise have had the plan inventing interfaces it had not verified. All three were checked against the tree: - -1. `cli.NewStringSliceFlag` **exists** (`cmd/forge/plugins/generate.go:83`), read back with `ctx.StringSlice` (`generate.go:878`). Task 4 Step 5 names both concretely. -2. The E2E helper is **`generateFromSpecFile(t, path) map[string]string`** (`e2e_specfile_test.go:109`) and takes a *file path*, not a spec. The first draft of Task 5 assumed a spec-based `generatePackage(t, spec)`, which would have generated from an in-memory spec and quietly bypassed the parse path the feature changes. Task 5 now extracts `generateFromMergedSpec` from the existing helper and pins the extraction with `TestSingleSpecFileStillGeneratesIdentically`. -3. **No CLI path holds a `router.Router`** — `grep -rn "router.Router" cmd/forge/` returns nothing. Introspection is therefore not reachable from `forge client generate` at all, so Task 7 is reduced to pinning merge precedence by test, with an explicit instruction not to build a stub router. - -**Scale.** Seven tasks, of which Task 7 is two steps. Tasks 1-3 are self-contained in `internal/client` and carry the whole merge semantic; Tasks 4-6 are CLI wiring. A reviewer can reject any one without rejecting its neighbours. diff --git a/docs/superpowers/plans/2026-08-09-ssr-dehydrate-hydrate.md b/docs/superpowers/plans/2026-08-09-ssr-dehydrate-hydrate.md deleted file mode 100644 index 213fc099..00000000 --- a/docs/superpowers/plans/2026-08-09-ssr-dehydrate-hydrate.md +++ /dev/null @@ -1,2375 +0,0 @@ -# SSR `dehydrate` / `hydrate` Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Ship `dehydrate`/`hydrate` for the Forge web client runtime, so a server render emits real markup and a hydrating client starts warm instead of empty. - -**Architecture:** A new `wire.ts` owns the JSON encoding — escaping data keys that collide with the `__ref` marker, detecting cycles, and reviving references through the existing `makeRef`/`markRewritten` primitives so `ref.ts`'s identity model is untouched. A new `ssr.ts` owns the policy — a reachability closure that emits only entities the exported queries reference, a required principal assertion on both sides, and two payload modes. `client-react` gains a hydration boundary that hydrates during render, and `getServerSnapshot` starts returning real state through a new non-opening `QueryCache.peek`. - -**Tech Stack:** TypeScript (ES2020, `strict`), Vitest, fast-check, size-limit. React 18/19 for the adapter. No new runtime dependencies. - -**Spec:** `docs/superpowers/specs/2026-08-08-ssr-dehydrate-hydrate-design.md` - -## Global Constraints - -- **Errors are plain `Error` with a `[forge] ` prefix.** This package has no custom error classes and gains none. See `src/client.ts:33`, `src/cache.ts:1093`. -- **`src/ref.ts` is not modified.** The revive pass uses the already-exported `makeRef` and `markRewritten`. -- **No new runtime dependencies** in `client-core` or `client-react`. -- **`client-core` ships no ambient Node types** (`tsconfig.json` sets `"types": []`). Nothing in `src/` may reference `process`, `Buffer`, or `require`. -- **Comments explain *why*, at the density of the surrounding files.** This codebase documents rejected alternatives inline; match it. Do not add comments that restate the code. -- **`client-react` resolves `@forge-go/client-core` from `file:../client-core`, i.e. from `dist/`.** Run `npm run build` in `client-core` before running `client-react` tests, every time core changes. -- **Commit after every task.** No `Co-Authored-By` trailers. -- Work from the package directory: `cd packages/client-core` or `cd packages/client-react`. - -## File Structure - -| File | Responsibility | -| --- | --- | -| `packages/client-core/src/wire.ts` | **New.** The JSON encoding alone: escape/unescape, cycle detection, reference revival. Knows nothing about `QueryCache`. | -| `packages/client-core/src/ssr.ts` | **New.** `dehydrate`/`hydrate`, the payload types, the reachability closure, the principal assertions. Knows nothing about escaping details. | -| `packages/client-core/src/cache.ts` | Add `peek`, `restore`, `settledQueries`; export `operationName`. | -| `packages/client-core/src/registry.ts` | `SettleResult.tags`. | -| `packages/client-core/src/client.ts` | `getServerState` on `QueryHandle`. | -| `packages/client-core/src/index.ts` | Exports. | -| `packages/client-core/package.json` | size-limit entries. | -| `packages/client-react/src/hydration.ts` | **New.** `ForgeHydrationBoundary`. | -| `packages/client-react/src/useQuery.ts` | `getServerSnapshot` via the handle. | -| `packages/client-react/src/index.ts` | Exports. | - ---- - -### Task 1: The wire encoding - -**Files:** -- Create: `packages/client-core/src/wire.ts` -- Test: `packages/client-core/__tests__/wire.test.ts` - -**Interfaces:** -- Consumes: `isRef`, `makeRef`, `markRewritten` from `./ref`; `EntityKey`, `Ref` from `./types`. -- Produces: - - `interface EncodeContext { readonly query: string; readonly entity?: string }` - - `interface EncodeResult { readonly value: unknown; readonly refs: readonly EntityKey[] }` - - `function encode(node: unknown, context: EncodeContext): EncodeResult` - - `function assertAcyclic(node: unknown, context: EncodeContext): void` - - `function revive(node: unknown): unknown` - -- [ ] **Step 1: Write the failing test** - -Create `packages/client-core/__tests__/wire.test.ts`: - -```ts -import { describe, expect, it } from 'vitest'; - -import { makeRef, isRef, isRewritten } from '../src/ref'; -import { assertAcyclic, encode, revive } from '../src/wire'; - -const where = { query: 'GET /orders({})' }; - -describe('encode', () => { - it('emits a reference as a plain marker object and reports its key', () => { - const { value, refs } = encode({ order: makeRef('Order:7') }, where); - - expect(value).toEqual({ order: { __ref: 'Order:7' } }); - expect(refs).toEqual(['Order:7']); - }); - - it('escapes response data that is shaped exactly like a reference', () => { - const { value, refs } = encode({ meta: { __ref: 'not a reference' } }, where); - - expect(value).toEqual({ meta: { ___ref: 'not a reference' } }); - expect(refs).toEqual([]); - }); - - it('escapes an already-escape-shaped key, so the scheme nests', () => { - expect(encode({ ___ref: 1, ____ref: 2 }, where).value).toEqual({ ____ref: 1, _____ref: 2 }); - }); - - it('leaves every other key alone', () => { - expect(encode({ __refs: 1, ref: 2, _ref: 3 }, where).value).toEqual({ - __refs: 1, - ref: 2, - _ref: 3, - }); - }); - - it('reports references found at any depth, deduplication left to the caller', () => { - const { refs } = encode( - { rows: [{ o: makeRef('Order:1') }, { o: makeRef('Order:2') }, makeRef('Order:1')] }, - where, - ); - - expect(refs).toEqual(['Order:1', 'Order:2', 'Order:1']); - }); - - it('allows the same object twice through different branches -- a DAG is not a cycle', () => { - const shared = { n: 1 }; - - expect(encode({ a: shared, b: shared }, where).value).toEqual({ a: { n: 1 }, b: { n: 1 } }); - }); - - it('throws on a cycle, naming the query and the path', () => { - const node: Record = { id: 7 }; - node.self = node; - - expect(() => encode(node, where)).toThrow(/cyclic value/); - expect(() => encode(node, where)).toThrow(/skeleton\.self/); - }); - - it('names the record when one is being encoded', () => { - const node: Record = {}; - node.meta = { self: node }; - - expect(() => encode(node, { query: 'GET /orders({})', entity: 'Order:7' })).toThrow( - /entity {2}Order:7/, - ); - expect(() => encode(node, { query: 'GET /orders({})', entity: 'Order:7' })).toThrow( - /data\.meta\.self/, - ); - }); - - it('reports an array index in the path', () => { - const row: Record = {}; - row.rows = [row]; - - expect(() => encode(row, where)).toThrow(/skeleton\.rows\[0\]/); - }); -}); - -describe('assertAcyclic', () => { - it('accepts an acyclic value', () => { - expect(() => assertAcyclic({ a: [1, { b: 2 }] }, where)).not.toThrow(); - }); - - it('throws on a cycle', () => { - const node: Record = {}; - node.self = node; - - expect(() => assertAcyclic(node, where)).toThrow(/cyclic value/); - }); -}); - -describe('revive', () => { - it('mints a genuine reference the runtime recognises', () => { - const revived = revive({ order: { __ref: 'Order:7' } }) as { order: unknown }; - - expect(isRef(revived.order)).toBe(true); - }); - - it('unescapes data that was shaped like a reference, and does not mint one', () => { - const revived = revive({ meta: { ___ref: 'not a reference' } }) as { meta: unknown }; - - expect(revived.meta).toEqual({ __ref: 'not a reference' }); - expect(isRef(revived.meta)).toBe(false); - }); - - it('marks a container that has a reference beneath it', () => { - const revived = revive({ rows: [{ __ref: 'Order:7' }] }) as { rows: object }; - - expect(isRewritten(revived.rows)).toBe(true); - expect(isRewritten(revived)).toBe(true); - }); - - it('leaves a container with no reference beneath it unmarked and by identity', () => { - const input = { totals: { open: 3 } }; - const revived = revive(input) as { totals: object }; - - expect(revived).toBe(input); - expect(isRewritten(revived.totals)).toBe(false); - }); - - it('does not mark a container that only needed unescaping', () => { - const revived = revive({ meta: { ___ref: 'x' } }) as object; - - expect(isRewritten(revived)).toBe(false); - }); - - it('ignores a marker-shaped object carrying anything but a lone string', () => { - expect(isRef(revive({ __ref: 7 }))).toBe(false); - expect(isRef(revive({ __ref: 'Order:7', extra: 1 }))).toBe(false); - }); - - it('round-trips through JSON', () => { - const encoded = encode({ rows: [makeRef('Order:7'), { __ref: 'data' }] }, where); - const revived = revive(JSON.parse(JSON.stringify(encoded.value))) as { rows: unknown[] }; - - expect(isRef(revived.rows[0])).toBe(true); - expect(revived.rows[1]).toEqual({ __ref: 'data' }); - expect(isRef(revived.rows[1])).toBe(false); - }); -}); -``` - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd packages/client-core && npx vitest run __tests__/wire.test.ts` -Expected: FAIL — `Failed to resolve import "../src/wire"`. - -- [ ] **Step 3: Write the implementation** - -Create `packages/client-core/src/wire.ts`: - -```ts -import { isRef, makeRef, markRewritten } from './ref'; -import type { EntityKey, Ref } from './types'; - -/** - * The JSON encoding a dehydrated payload uses, and its inverse. - * - * `__ref` is the wire form of a reference and always has been -- see `Ref` -- - * but it cannot be the *recognition* rule, because a response may legitimately - * contain an object of exactly that shape. `normalize` leaves such an object - * inline, so it reaches the store as ordinary record data; a revive pass that - * treated every `{__ref: string}` as a reference would mint one from it and - * `denormalize` would resolve it to `undefined`. That is precisely the lossy - * round-trip `ref.ts` refuses, reintroduced from the other direction. - * - * So the encoder escapes on the way out and the reviver unescapes on the way - * in. Both directions are one walk, and the encoder's walk is needed anyway -- - * `dehydrate` has to collect the references it finds to build its reachability - * closure, and has to notice a cycle, and both fall out of visiting every node. - */ - -/** A key that would be read back as the marker, or as an escape of one. */ -const COLLIDES = /^_*__ref$/; - -/** A key this encoder escaped on the way out. */ -const ESCAPED = /^_+__ref$/; - -export interface EncodeContext { - /** The query whose payload this is, for the cycle error. */ - readonly query: string; - /** The record being encoded, when this is a record rather than a skeleton. */ - readonly entity?: string; -} - -export interface EncodeResult { - readonly value: unknown; - /** - * Every reference found, in encounter order and **not** deduplicated. - * - * The caller is walking a closure and already holds the set of keys it has - * seen, so deduplicating here would build a second Set per node to answer a - * question the caller has to answer again anyway. - */ - readonly refs: readonly EntityKey[]; -} - -/** Copy `node` into a JSON-safe form, escaping keys and lifting references. */ -export function encode(node: unknown, context: EncodeContext): EncodeResult { - const refs: EntityKey[] = []; - const route = new Set(); - - function walk(value: unknown, path: string): unknown { - if (value === null || typeof value !== 'object') return value; - - if (isRef(value)) { - const key = (value as Ref).__ref; - refs.push(key); - - // A fresh literal rather than the frozen `Ref` itself. Emitting the - // reference would put an object registered in `ref.ts`'s WeakSet into the - // payload, which is harmless on the wire and confusing in a test. - return { __ref: key }; - } - - if (route.has(value)) throw cyclic(context, path); - - route.add(value); - - const out = Array.isArray(value) - ? value.map((element, index) => walk(element, `${path}[${index}]`)) - : walkObject(value as Record, path); - - route.delete(value); - - return out; - } - - function walkObject(value: Record, path: string): Record { - const out: Record = {}; - - for (const key of Object.keys(value)) { - out[COLLIDES.test(key) ? `_${key}` : key] = walk(value[key], `${path}.${key}`); - } - - return out; - } - - return { value: walk(node, rootPath(context)), refs }; -} - -/** - * Throw if `node` is cyclic, without copying it. - * - * The denormalized payload mode ships a rehydrated value straight to - * `JSON.stringify`, so it needs the cycle check and none of the escaping: it - * contains no genuine references, and escaping a value nothing will unescape - * would corrupt it. - */ -export function assertAcyclic(node: unknown, context: EncodeContext): void { - const route = new Set(); - - function walk(value: unknown, path: string): void { - if (value === null || typeof value !== 'object') return; - - if (route.has(value)) throw cyclic(context, path); - - route.add(value); - - if (Array.isArray(value)) { - value.forEach((element, index) => walk(element, `${path}[${index}]`)); - } else { - for (const key of Object.keys(value)) { - walk((value as Record)[key], `${path}.${key}`); - } - } - - route.delete(value); - } - - walk(node, rootPath(context)); -} - -/** - * Turn a decoded payload back into a skeleton the runtime recognises. - * - * `markRewritten` is applied to a container **only** where a reference occurs - * beneath it. A container without one is deliberately left unmarked and, where - * nothing about it changed, returned by identity -- which is what keeps - * `EntityStore`'s "not rewritten means no walk" fast path intact for a hydrated - * skeleton exactly as it is for a normalized one. Marking everything would be - * correct and would void structural sharing for the whole response. - */ -export function revive(node: unknown): unknown { - return reviveNode(node).value; -} - -interface Revived { - readonly value: unknown; - /** Whether a reference was minted here or anywhere beneath. */ - readonly refs: boolean; -} - -function reviveNode(node: unknown): Revived { - if (node === null || typeof node !== 'object') return { value: node, refs: false }; - - if (isMarker(node)) { - return { value: makeRef((node as Record).__ref as EntityKey), refs: true }; - } - - return Array.isArray(node) - ? reviveArray(node) - : reviveObject(node as Record); -} - -function reviveArray(node: unknown[]): Revived { - const out = new Array(node.length); - let refs = false; - let changed = false; - - for (let i = 0; i < node.length; i++) { - const child = reviveNode(node[i]); - - out[i] = child.value; - if (child.refs) refs = true; - if (child.value !== node[i]) changed = true; - } - - // Nothing moved, so nothing was minted either: a reference always differs - // from the marker object it replaced. - if (!changed) return { value: node, refs: false }; - - return { value: refs ? markRewritten(out) : out, refs }; -} - -function reviveObject(node: Record): Revived { - const out: Record = {}; - let refs = false; - let changed = false; - - for (const key of Object.keys(node)) { - const name = ESCAPED.test(key) ? key.slice(1) : key; - const child = reviveNode(node[key]); - - out[name] = child.value; - if (child.refs) refs = true; - if (name !== key || child.value !== node[key]) changed = true; - } - - if (!changed) return { value: node, refs: false }; - - return { value: refs ? markRewritten(out) : out, refs }; -} - -/** - * Whether a decoded object is the marker `encode` emits. - * - * Exactly one own key, named `__ref`, holding a string -- the shape - * `makeRef` produces and the only shape the encoder ever emits unescaped. - * Anything looser would claim objects the encoder escaped for a reason. - */ -function isMarker(node: object): boolean { - if (Array.isArray(node)) return false; - - const keys = Object.keys(node); - - return ( - keys.length === 1 && - keys[0] === '__ref' && - typeof (node as Record).__ref === 'string' - ); -} - -function rootPath(context: EncodeContext): string { - return context.entity === undefined ? 'skeleton' : 'data'; -} - -function cyclic(context: EncodeContext, path: string): Error { - const headline = - context.entity === undefined - ? 'cannot serialize a cyclic value' - : 'cannot serialize a cycle within one record'; - const entity = context.entity === undefined ? '' : `\n entity ${context.entity}`; - - return new Error( - `[forge] dehydrate: ${headline}\n query ${context.query}${entity}\n path ${path}`, - ); -} -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cd packages/client-core && npx vitest run __tests__/wire.test.ts && npm run typecheck` -Expected: all PASS, no type errors. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/src/wire.ts packages/client-core/__tests__/wire.test.ts -git commit -m "feat(client): add the SSR wire encoding, with reference-shaped data escaped" -``` - ---- - -### Task 2: `SettleResult.tags` - -**Files:** -- Modify: `packages/client-core/src/registry.ts:44-60` (the `SettleResult` interface) and `:306-328` (`settle`) -- Test: `packages/client-core/__tests__/registry.test.ts` - -**Interfaces:** -- Produces: `SettleResult.tags?: Iterable` — when present, `settle` uses these instead of resolving `provides` against a response. - -- [ ] **Step 1: Write the failing test** - -Append to `packages/client-core/__tests__/registry.test.ts`: - -```ts -describe('settling with tags supplied', () => { - it('uses the supplied tags instead of resolving provides against a response', () => { - const registry = new QueryRegistry(); - - registry.mount({ operation: 'orderList', args: {}, provides: ['Order:{res.id}'] })(); - registry.settle('orderList()', { tags: ['Order:7'], deps: ['Order:7'] }); - - expect(registry.queriesFor('Order:7').map((entry) => entry.key)).toEqual(['orderList()']); - }); - - it('still unions the supplied tags with the entity dependencies', () => { - const registry = new QueryRegistry(); - - registry.mount({ operation: 'orderList', args: {}, provides: [] })(); - registry.settle('orderList()', { tags: ['Order[]'], deps: ['Order:1'] }); - - const entry = registry.get('orderList()'); - - expect([...(entry?.tags ?? [])].sort()).toEqual(['Order:1', 'Order[]']); - }); - - it('reports no unresolved template when tags are supplied', () => { - const unresolved: string[] = []; - const registry = new QueryRegistry({ onUnresolved: (template) => unresolved.push(template) }); - - registry.mount({ operation: 'orderList', args: {}, provides: ['Order:{res.id}'] })(); - registry.settle('orderList()', { tags: ['Order:7'] }); - - expect(unresolved).toEqual([]); - }); -}); -``` - -Adjust the import block at the top of the file only if `QueryRegistry` is not already imported; it is. - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd packages/client-core && npx vitest run __tests__/registry.test.ts -t "settling with tags supplied"` -Expected: FAIL — the query is not found under `Order:7`, because `Order:{res.id}` resolved to nothing. - -- [ ] **Step 3: Write the implementation** - -In `packages/client-core/src/registry.ts`, add to `SettleResult` after the `response` field: - -```ts - /** - * The already-resolved tag set, bypassing `provides` resolution entirely. - * - * For `hydrate` in its normalized mode, which holds no response: `provides` - * templates naming `{res.x}` cannot be resolved without one, and resolving - * them to nothing would silently drop the tag, so a mutation would stop - * reaching a query that displays what it changed. The tags were resolved on - * the server, where the response existed, and are carried across instead. - * - * `response` is ignored when this is present. No caller supplies both. - */ - readonly tags?: Iterable; -``` - -Then replace the first statement of `settle` (`registry.ts:311-313`): - -```ts - const supplied = result.tags; - const resolved = - supplied === undefined - ? resolveTags(entry.provides, { ...entry.args, response: result.response }) - : { tags: new Set(supplied), unresolved: [] as string[] }; - - for (const template of resolved.unresolved) this.onUnresolved?.(template, entry); -``` - -The `retag` call two lines below already reads `resolved.tags`, so it needs no change. - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cd packages/client-core && npx vitest run __tests__/registry.test.ts && npm run typecheck` -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/src/registry.ts packages/client-core/__tests__/registry.test.ts -git commit -m "feat(client): let a settle supply resolved tags instead of a response" -``` - ---- - -### Task 3: `QueryCache.peek`, `settledQueries` and `restore` - -**Files:** -- Modify: `packages/client-core/src/cache.ts` — add three methods after `getState` (`cache.ts:271`), and export `operationName` (`cache.ts:1056`) -- Test: `packages/client-core/__tests__/cache.test.ts` - -**Interfaces:** -- Consumes: `SettleResult.tags` from Task 2. -- Produces: - - `interface CachedQuery { readonly key: string; readonly meta: OperationMeta; readonly args: TagContext; readonly skeleton: unknown }` - - `QueryCache.peek(meta: OperationMeta, args?: TagContext): QueryState | undefined` - - `QueryCache.settledQueries(): CachedQuery[]` - - `QueryCache.restore(meta: OperationMeta, args: TagContext | undefined, input: RestoreInput): void` - - `interface RestoreInput { readonly skeleton: unknown; readonly tags?: Iterable; readonly response?: unknown; readonly stale?: boolean }` - - `export function operationName(meta: OperationMeta): string` (package-internal; not re-exported from `index.ts`) - -- [ ] **Step 1: Write the failing test** - -Append to `packages/client-core/__tests__/cache.test.ts`: - -```ts -describe('peek', () => { - it('returns undefined for a query the cache has never opened, and opens nothing', () => { - const { cache } = cache_(() => [{ id: 7, total: 99 }]); - - expect(cache.peek(orderList)).toBeUndefined(); - expect(cache.size).toBe(0); - }); - - it('returns the same state object as getState once a record exists', async () => { - const { cache } = cache_(() => [{ id: 7, total: 99 }]); - - await cache.fetch(orderList); - - expect(cache.peek(orderList)).toBe(cache.getState(orderList)); - }); - - it('is referentially stable across calls while nothing changes', async () => { - const { cache } = cache_(() => [{ id: 7, total: 99 }]); - - await cache.fetch(orderList); - - expect(cache.peek(orderList)).toBe(cache.peek(orderList)); - }); -}); - -describe('settledQueries', () => { - it('lists only the queries that settled successfully', async () => { - const { cache } = cache_((request) => { - if (request.meta === customerList) throw new HttpFailure(500); - - return [{ id: 7, total: 99 }]; - }); - - await cache.fetch(orderList); - await cache.fetch(customerList).catch(() => undefined); - cache.getState(orderGet, { path: { id: 1 } }); - - expect(cache.settledQueries().map((query) => query.key)).toEqual([cache.key(orderList)]); - }); - - it('reports the skeleton the store holds, not the response', async () => { - const { cache } = cache_(() => [{ id: 7, total: 99 }]); - - await cache.fetch(orderList); - - expect(cache.settledQueries()[0]?.skeleton).toEqual([{ __ref: 'Order:7' }]); - }); -}); - -describe('restore', () => { - it('settles a query from a skeleton with no request', () => { - const { cache, transport } = cache_(() => [{ id: 7, total: 99 }]); - - cache.store.put('Order:7', { id: 7, total: 99 }); - cache.restore(orderList, undefined, { skeleton: [makeRef('Order:7')], tags: ['Order[]'] }); - - expect(cache.getState(orderList).status).toBe('success'); - expect(cache.getState(orderList).data).toEqual([{ id: 7, total: 99 }]); - expect(transport.calls).toHaveLength(0); - }); - - it('records the skeleton dependencies, so a write to the entity is seen', () => { - const { cache } = cache_(() => []); - - cache.store.put('Order:7', { id: 7, total: 99 }); - cache.restore(orderList, undefined, { skeleton: [makeRef('Order:7')], tags: ['Order[]'] }); - - expect([...(cache.registry.get(cache.key(orderList))?.deps ?? [])]).toEqual(['Order:7']); - }); - - it('leaves the entry fresh by default and stale when asked', () => { - const { cache } = cache_(() => []); - - cache.store.put('Order:7', { id: 7, total: 99 }); - cache.restore(orderList, undefined, { skeleton: [makeRef('Order:7')], tags: ['Order[]'] }); - expect(cache.registry.get(cache.key(orderList))?.stale).toBe(false); - - cache.restore(orderGet, { path: { id: 7 } }, { - skeleton: makeRef('Order:7'), - tags: ['Order:7'], - stale: true, - }); - expect(cache.registry.get(cache.key(orderGet, { path: { id: 7 } }))?.stale).toBe(true); - }); - - it('notifies the subscribers of a query it settles', () => { - const { cache } = cache_(() => []); - let notified = 0; - - cache.store.put('Order:7', { id: 7, total: 99 }); - cache.subscribe(orderList, undefined, () => { - notified++; - }); - - const before = notified; - cache.restore(orderList, undefined, { skeleton: [makeRef('Order:7')], tags: ['Order[]'] }); - - expect(notified).toBeGreaterThan(before); - }); -}); -``` - -Add to the imports at the top of `cache.test.ts`: - -```ts -import { makeRef } from '../src/ref'; -``` - -and rename the local helper `cache` to `cache_` throughout the file **only if** the new tests shadow it. The file already declares `function cache(handler)` at module scope; the new `describe` blocks call it as `cache_`, so add an alias next to the existing helper rather than renaming every call site: - -```ts -const cache_ = cache; -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `cd packages/client-core && npx vitest run __tests__/cache.test.ts -t "peek"` -Expected: FAIL — `cache.peek is not a function`. - -- [ ] **Step 3: Write the implementation** - -In `packages/client-core/src/cache.ts`, add after the `getState` method (`cache.ts:271`): - -```ts - /** - * This query's state **without opening a record for it.** - * - * `getState` routes through `open`, which creates the record if it is new -- - * correct for a subscriber, wrong for two callers that must not have side - * effects. A server render asks about every query on the page, including ones - * this request never fetched, and on a server the cache may be shared; and a - * dehydrated payload is read, not fetched. `undefined` means "nothing is - * cached", which is a different answer from `idle`. - * - * Deliberately does not move the record's LRU position either: a peek is not - * a use, and letting a server render reorder the eviction queue would make - * which query gets evicted depend on render order. - */ - peek(meta: OperationMeta, args?: TagContext): QueryState | undefined { - const record = this.records.get(this.key(meta, args)); - - if (record === undefined) return undefined; - - return this.snapshot(record) as QueryState; - } - - /** - * Every query that settled successfully, as `dehydrate` reads them. - * - * Pending and failed queries are absent by construction. A pending query has - * no skeleton to serialize, and a failed one would hydrate a client into a - * failure the server observed and the client cannot meaningfully retry -- - * both are better left for the client to fetch normally. - */ - settledQueries(): CachedQuery[] { - const out: CachedQuery[] = []; - - for (const record of this.records.values()) { - if (!record.settled || record.status !== 'success') continue; - - out.push({ - key: record.key, - meta: record.meta, - args: record.args, - skeleton: record.skeleton, - }); - } - - return out; - } - - /** - * Settle a query from a skeleton, with no request behind it. - * - * The seam `hydrate` writes through. Everything a settle normally does apart - * from the request: install the skeleton, mark the record successful, record - * the dependencies, retag the registry entry and notify. - * - * `deps` are recomputed from the skeleton against the live store rather than - * carried in the payload -- `dependencies` is exact, costs one memoized walk, - * and does not have to trust what arrived over the wire. - * - * Merges rather than replaces. A query the cache already holds is re-settled - * against the hydrated skeleton, and the records behind it went through `put`, - * which keeps the previous object for identical data. Hydrating the same - * payload twice therefore moves no version and changes no identity. - */ - restore(meta: OperationMeta, args: TagContext | undefined, input: RestoreInput): void { - const record = this.open(meta, args); - - record.skeleton = input.skeleton; - record.settled = true; - record.status = 'success'; - record.error = undefined; - record.fetching = false; - - const value = this.read(record); - - this.registry.settle(record.key, { - value, - deps: this.store.dependencies(input.skeleton), - ...(input.tags === undefined ? {} : { tags: input.tags }), - ...(input.response === undefined ? {} : { response: input.response }), - }); - - const entry = this.registry.get(record.key); - - if (input.stale === true && entry !== undefined) this.registry.markStale(entry); - - this.notify(record); - } -``` - -Add the two interfaces above the `QueryCache` class, next to `LiveBinding` (`cache.ts:84`): - -```ts -/** One settled query, as `dehydrate` reads it out of the cache. */ -export interface CachedQuery { - readonly key: string; - readonly meta: OperationMeta; - readonly args: TagContext; - readonly skeleton: unknown; -} - -/** What `QueryCache.restore` installs. See that method. */ -export interface RestoreInput { - readonly skeleton: unknown; - /** Resolved tags, for a payload that carries no response. */ - readonly tags?: Iterable; - /** The response, for a payload that does. Ignored when `tags` is present. */ - readonly response?: unknown; - /** Settle behind the server, so a mount refetches. */ - readonly stale?: boolean; -} -``` - -Finally, export the existing `operationName` helper by changing `cache.ts:1056` from `function operationName(` to `export function operationName(`. Add one line to its doc comment: - -``` - * Exported within the package so `ssr.ts` names an operation on the wire - * exactly as the cache keys it. It is not part of the public API. -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cd packages/client-core && npx vitest run && npm run typecheck` -Expected: all PASS, including every pre-existing test. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/src/cache.ts packages/client-core/__tests__/cache.test.ts -git commit -m "feat(client): add peek, settledQueries and restore to the query cache" -``` - ---- - -### Task 4: `dehydrate` - -**Files:** -- Create: `packages/client-core/src/ssr.ts` -- Test: `packages/client-core/__tests__/ssr.test.ts` - -**Interfaces:** -- Consumes: `encode`, `assertAcyclic` from `./wire`; `operationName`, `CachedQuery`, `QueryCache` from `./cache`. -- Produces: - - `type DehydratedState = NormalizedState | DenormalizedState` - - `interface NormalizedState { readonly v: 1; readonly mode: 'normalized'; readonly principal?: string | number | null; readonly records: Readonly>; readonly queries: readonly NormalizedQuery[] }` - - `interface DenormalizedState { readonly v: 1; readonly mode: 'denormalized'; readonly principal?: string | number | null; readonly queries: readonly DenormalizedQuery[] }` - - `interface NormalizedQuery { readonly operation: string; readonly args: TagContext; readonly skeleton: unknown; readonly tags: readonly string[] }` - - `interface DenormalizedQuery { readonly operation: string; readonly args: TagContext; readonly value: unknown }` - - `interface DehydrateOptions { readonly principal: string | number | null | undefined; readonly mode?: 'normalized' | 'denormalized'; readonly include?: readonly string[] }` - - `function dehydrate(cache: QueryCache, options: DehydrateOptions): DehydratedState` - -- [ ] **Step 1: Write the failing test** - -Create `packages/client-core/__tests__/ssr.test.ts`: - -```ts -import { describe, expect, it } from 'vitest'; - -import { QueryCache } from '../src/cache'; -import { manualScheduler } from '../src/invalidate'; -import { dehydrate } from '../src/ssr'; -import type { NormalizedState, DenormalizedState } from '../src/ssr'; -import type { OperationMeta, TransportRequest } from '../src/transport'; -import { fakeTransport } from './harness'; -import { schema } from './schema'; - -const orderList: OperationMeta = { - method: 'GET', - path: '/orders', - entity: 'Order', - provides: ['Order[]'], - invalidates: [], -}; - -const customerList: OperationMeta = { - method: 'GET', - path: '/customers', - entity: 'Customer', - provides: ['Customer[]'], - invalidates: [], -}; - -function cache(handler: (request: TransportRequest, call: number) => unknown): QueryCache { - const scheduler = manualScheduler(); - - return new QueryCache({ - transport: fakeTransport(handler), - entities: schema, - scheduler: scheduler.schedule, - }); -} - -describe('dehydrate, normalized', () => { - it('emits the skeleton, the reachable records and the resolved tags', async () => { - const client = cache(() => [{ id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }]); - - await client.fetch(orderList); - - const state = dehydrate(client, { principal: 'u-1' }) as NormalizedState; - - expect(state.v).toBe(1); - expect(state.mode).toBe('normalized'); - expect(state.principal).toBe('u-1'); - expect(state.queries).toEqual([ - { - operation: 'GET /orders', - args: {}, - skeleton: [{ __ref: 'Order:7' }], - tags: expect.arrayContaining(['Order[]', 'Order:7', 'Customer:c-3']), - }, - ]); - expect(state.records['Order:7']).toEqual({ - id: 7, - total: 99, - customer: { __ref: 'Customer:c-3' }, - }); - expect(state.records['Customer:c-3']).toEqual({ id: 'c-3', name: 'Ada' }); - }); - - it('survives JSON', async () => { - const client = cache(() => [{ id: 7, total: 99 }]); - - await client.fetch(orderList); - - expect(() => JSON.stringify(dehydrate(client, { principal: 'u-1' }))).not.toThrow(); - }); - - it('emits an entity cycle between records without difficulty', async () => { - const client = cache(() => [ - { id: 7, total: 99, customer: { id: 'c-3', orders: [{ id: 7 }] } }, - ]); - - await client.fetch(orderList); - - const state = dehydrate(client, { principal: 'u-1' }) as NormalizedState; - - expect(state.records['Customer:c-3']).toEqual({ - id: 'c-3', - orders: [{ __ref: 'Order:7' }], - }); - }); -}); - -describe('dehydrate, the reachability closure', () => { - it('omits an entity no exported query references', async () => { - const client = cache((request) => - request.meta === orderList ? [{ id: 7, total: 99 }] : [{ id: 'c-9', name: 'Grace' }], - ); - - await client.fetch(orderList); - await client.fetch(customerList); - - const state = dehydrate(client, { - principal: 'u-1', - include: [client.key(orderList)], - }) as NormalizedState; - - expect(Object.keys(state.records)).toEqual(['Order:7']); - expect(state.queries).toHaveLength(1); - }); - - it('never reads the store wholesale: an orphaned record is not emitted', async () => { - const client = cache(() => [{ id: 7, total: 99 }]); - - await client.fetch(orderList); - client.store.put('Order:999', { id: 999, secret: 'another request' }); - - const state = dehydrate(client, { principal: 'u-1' }) as NormalizedState; - - expect(Object.keys(state.records)).toEqual(['Order:7']); - }); - - it('throws for an include naming a key the cache does not hold', async () => { - const client = cache(() => [{ id: 7, total: 99 }]); - - await client.fetch(orderList); - - expect(() => dehydrate(client, { principal: 'u-1', include: ['GET /nope()'] })).toThrow( - /\[forge\] dehydrate: no settled query for GET \/nope\(\)/, - ); - }); - - it('omits a query that failed', async () => { - const client = cache((request) => { - if (request.meta === customerList) throw new Error('boom'); - - return [{ id: 7, total: 99 }]; - }); - - await client.fetch(orderList); - await client.fetch(customerList).catch(() => undefined); - - expect(dehydrate(client, { principal: 'u-1' }).queries).toHaveLength(1); - }); -}); - -describe('dehydrate, the principal', () => { - it('throws when it does not match the cache owner', async () => { - const client = cache(() => [{ id: 7, total: 99 }]); - - client.setPrincipal('u-1'); - await client.fetch(orderList); - - expect(() => dehydrate(client, { principal: 'u-2' })).toThrow( - /\[forge\] dehydrate: principal does not match the cache owner/, - ); - }); - - it('accepts an unset principal on both sides', async () => { - const client = cache(() => [{ id: 7, total: 99 }]); - - await client.fetch(orderList); - - expect(dehydrate(client, { principal: undefined }).principal).toBeUndefined(); - }); - - it('refuses a principal that cannot survive JSON', async () => { - const client = cache(() => [{ id: 7, total: 99 }]); - const owner = { id: 'u-1' }; - - client.setPrincipal(owner); - await client.fetch(orderList); - - expect(() => dehydrate(client, { principal: owner as never })).toThrow( - /\[forge\] dehydrate: principal must be a string, number, null or undefined/, - ); - }); -}); - -describe('dehydrate, denormalized', () => { - it('emits the rehydrated value and no records', async () => { - const client = cache(() => [{ id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }]); - - await client.fetch(orderList); - - const state = dehydrate(client, { - principal: 'u-1', - mode: 'denormalized', - }) as DenormalizedState; - - expect(state.mode).toBe('denormalized'); - expect(state.queries).toEqual([ - { - operation: 'GET /orders', - args: {}, - value: [{ id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }], - }, - ]); - expect(state).not.toHaveProperty('records'); - }); - - it('passes reference-shaped response data through untouched', async () => { - const client = cache(() => [{ id: 7, meta: { __ref: 'not a reference' } }]); - - await client.fetch(orderList); - - const state = dehydrate(client, { - principal: 'u-1', - mode: 'denormalized', - }) as DenormalizedState; - - expect(state.queries[0]?.value).toEqual([{ id: 7, meta: { __ref: 'not a reference' } }]); - }); - - it('throws on an entity cycle, which normalized mode serializes fine', async () => { - const client = cache(() => [ - { id: 7, total: 99, customer: { id: 'c-3', orders: [{ id: 7 }] } }, - ]); - - await client.fetch(orderList); - - expect(() => dehydrate(client, { principal: 'u-1', mode: 'denormalized' })).toThrow( - /cannot serialize a cyclic value/, - ); - expect(() => dehydrate(client, { principal: 'u-1' })).not.toThrow(); - }); -}); -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `cd packages/client-core && npx vitest run __tests__/ssr.test.ts` -Expected: FAIL — `Failed to resolve import "../src/ssr"`. - -- [ ] **Step 3: Write the implementation** - -Create `packages/client-core/src/ssr.ts`: - -```ts -import { operationName } from './cache'; -import type { CachedQuery, QueryCache } from './cache'; -import type { TagContext } from './tags'; -import type { EntityKey } from './types'; -import { assertAcyclic, encode } from './wire'; - -/** - * Serializing a cache for a server render, and reading it back. - * - * The payload is data embedded in an HTML response, so what it may contain is - * a property of this module rather than a caution in the documentation. - * `dehydrate` never reads the store wholesale: the record set is *built* by a - * reachability walk from the exported queries, so an entity no exported query - * references cannot appear in the payload -- not because a rule forbids it, but - * because nothing ever put it there. That is what makes a module-level cache - * shared across concurrent server requests survivable rather than a leak. - * - * Both sides also assert the principal. `dehydrate` refuses to serialize for - * anyone but the cache's current owner, and `hydrate` refuses a payload that - * belongs to someone else -- which is what a payload cached at a CDN and served - * to the wrong session runs into. - */ - -/** One query in a normalized payload. */ -export interface NormalizedQuery { - readonly operation: string; - readonly args: TagContext; - readonly skeleton: unknown; - /** - * The resolved tag set. - * - * Carried because this mode holds no response, and `provides` templates - * naming `{res.x}` cannot be resolved without one. See `SettleResult.tags`. - */ - readonly tags: readonly string[]; -} - -/** One query in a denormalized payload. */ -export interface DenormalizedQuery { - readonly operation: string; - readonly args: TagContext; - readonly value: unknown; -} - -export interface NormalizedState { - readonly v: 1; - readonly mode: 'normalized'; - readonly principal?: string | number | null; - readonly records: Readonly>; - readonly queries: readonly NormalizedQuery[]; -} - -export interface DenormalizedState { - readonly v: 1; - readonly mode: 'denormalized'; - readonly principal?: string | number | null; - readonly queries: readonly DenormalizedQuery[]; -} - -export type DehydratedState = NormalizedState | DenormalizedState; - -export interface DehydrateOptions { - /** - * Who this payload's data belongs to. **Required**, and asserted against the - * cache's owner. - * - * Constrained to a scalar. That is not arbitrary: `setPrincipal` compares - * with `===`, so an object principal already re-clears the cache on every - * call that mints a fresh one -- the store's working contract is a scalar, - * and this states it. `undefined` is encoded as the key's absence, which is - * what `JSON.stringify` does with it anyway. - */ - readonly principal: string | number | null | undefined; - /** - * `normalized` (the default) dedupes an entity several queries share and is - * the smallest wire form. `denormalized` ships each query's rehydrated value - * and needs no revive pass, at the cost of duplicating shared entities -- and - * it cannot express a query whose value contains an entity cycle, because - * `denormalize` rebuilds such a graph as a real cycle and no JSON encoding of - * one exists. - */ - readonly mode?: 'normalized' | 'denormalized'; - /** Cache keys to export. Every settled query, when absent. */ - readonly include?: readonly string[]; -} - -export function dehydrate(cache: QueryCache, options: DehydrateOptions): DehydratedState { - const { principal } = options; - - if (!scalar(principal)) { - throw new Error( - '[forge] dehydrate: principal must be a string, number, null or undefined, ' + - 'so that it survives JSON and compares by value', - ); - } - - if (!Object.is(principal, cache.owner)) { - throw new Error( - '[forge] dehydrate: principal does not match the cache owner -- ' + - 'this cache holds another identity’s data', - ); - } - - const exported = select(cache, options.include); - - return options.mode === 'denormalized' - ? denormalized(cache, exported, principal) - : normalized(cache, exported, principal); -} - -/** - * The queries to export: those named, or every settled one. - * - * A named key the cache does not hold throws rather than exporting nothing. A - * typo that silently ships an empty payload is the defect found in production, - * where it presents as SSR having quietly stopped working. - */ -function select(cache: QueryCache, include: readonly string[] | undefined): CachedQuery[] { - const settled = cache.settledQueries(); - - if (include === undefined) return settled; - - const byKey = new Map(settled.map((query) => [query.key, query])); - - return include.map((key) => { - const query = byKey.get(key); - - if (query === undefined) throw new Error(`[forge] dehydrate: no settled query for ${key}`); - - return query; - }); -} - -function normalized( - cache: QueryCache, - exported: readonly CachedQuery[], - principal: string | number | null | undefined, -): NormalizedState { - const queries: NormalizedQuery[] = []; - const records: Record = {}; - const seen = new Set(); - // Each pending key remembers the query that reached it, so a cycle inside a - // record can name the query whose payload would have carried it. - const pending: { key: EntityKey; from: string }[] = []; - - const enqueue = (keys: readonly EntityKey[], from: string): void => { - for (const key of keys) { - if (seen.has(key)) continue; - - seen.add(key); - pending.push({ key, from }); - } - }; - - for (const query of exported) { - const encoded = encode(query.skeleton, { query: query.key }); - - enqueue(encoded.refs, query.key); - - queries.push({ - operation: operationName(query.meta), - args: query.args, - skeleton: encoded.value, - tags: [...(cache.registry.get(query.key)?.tags ?? [])], - }); - } - - while (pending.length > 0) { - const { key, from } = pending.pop() as { key: EntityKey; from: string }; - const record = cache.store.getRecord(key); - - // A reference the store no longer holds -- evicted between the fetch and - // this call. It rehydrates to nothing on the client exactly as it does - // here, which is the behaviour `denormalize` already specifies for a hole. - if (record === undefined) continue; - - const encoded = encode(record.data, { query: from, entity: key }); - - records[key] = encoded.value; - enqueue(encoded.refs, from); - } - - return { - v: 1, - mode: 'normalized', - ...(principal === undefined ? {} : { principal }), - records, - queries, - }; -} - -function denormalized( - cache: QueryCache, - exported: readonly CachedQuery[], - principal: string | number | null | undefined, -): DenormalizedState { - const queries = exported.map((query) => { - // The cache retains no raw responses -- `settle` reads one to resolve tags - // and does not keep it -- so this is the response as the store now holds - // it, merges included. `store.write` re-normalizes it into the same records. - const value = cache.store.read(query.skeleton); - - assertAcyclic(value, { query: query.key }); - - return { operation: operationName(query.meta), args: query.args, value }; - }); - - return { - v: 1, - mode: 'denormalized', - ...(principal === undefined ? {} : { principal }), - queries, - }; -} - -function scalar(value: unknown): value is string | number | null | undefined { - return ( - value === undefined || value === null || typeof value === 'string' || typeof value === 'number' - ); -} -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cd packages/client-core && npx vitest run __tests__/ssr.test.ts && npm run typecheck` -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/src/ssr.ts packages/client-core/__tests__/ssr.test.ts -git commit -m "feat(client): add dehydrate, emitting only entities the exported queries reach" -``` - ---- - -### Task 5: `hydrate` - -**Files:** -- Modify: `packages/client-core/src/ssr.ts` -- Test: `packages/client-core/__tests__/ssr.test.ts` - -**Interfaces:** -- Consumes: `revive` from `./wire`; `QueryCache.restore` from Task 3. -- Produces: - - `interface HydrateOptions { readonly ops: Readonly>; readonly stale?: boolean }` - - `function hydrate(cache: QueryCache, state: DehydratedState, options: HydrateOptions): void` - -- [ ] **Step 1: Write the failing test** - -Append to `packages/client-core/__tests__/ssr.test.ts`: - -```ts -import { hydrate } from '../src/ssr'; -import { isRef } from '../src/ref'; - -/** The generated `ops.ts` table, keyed as the generator keys it. */ -const ops = { orderList, customerList }; - -/** Serialize and read back, exactly as an HTML round trip would. */ -function transfer(state: ReturnType): ReturnType { - return JSON.parse(JSON.stringify(state)) as ReturnType; -} - -describe('hydrate', () => { - it('serves the hydrated value with no request, in normalized mode', async () => { - const server = cache(() => [{ id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }]); - - await server.fetch(orderList); - - const client = cache(() => { - throw new Error('the client must not fetch'); - }); - - hydrate(client, transfer(dehydrate(server, { principal: undefined })), { ops }); - - expect(client.getState(orderList).status).toBe('success'); - expect(client.getState(orderList).data).toEqual([ - { id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }, - ]); - }); - - it('serves the hydrated value with no request, in denormalized mode', async () => { - const server = cache(() => [{ id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }]); - - await server.fetch(orderList); - - const client = cache(() => { - throw new Error('the client must not fetch'); - }); - - hydrate( - client, - transfer(dehydrate(server, { principal: undefined, mode: 'denormalized' })), - { ops }, - ); - - expect(client.getState(orderList).data).toEqual([ - { id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }, - ]); - }); - - it('produces a store the entity graph is genuinely normalized into', async () => { - const server = cache(() => [{ id: 7, total: 99, customer: { id: 'c-3', name: 'Ada' } }]); - - await server.fetch(orderList); - - const client = cache(() => []); - - hydrate(client, transfer(dehydrate(server, { principal: undefined })), { ops }); - - expect(client.store.has('Order:7')).toBe(true); - expect(client.store.has('Customer:c-3')).toBe(true); - expect(isRef((client.store.getRecord('Order:7')?.data as Record).customer)) - .toBe(true); - }); - - it('keeps reference-shaped response data as data', async () => { - const server = cache(() => [{ id: 7, meta: { __ref: 'not a reference' } }]); - - await server.fetch(orderList); - - const client = cache(() => []); - - hydrate(client, transfer(dehydrate(server, { principal: undefined })), { ops }); - - expect(client.getState(orderList).data).toEqual([ - { id: 7, meta: { __ref: 'not a reference' } }, - ]); - }); - - it('rebuilds an entity cycle as a cycle', async () => { - const server = cache(() => [ - { id: 7, total: 99, customer: { id: 'c-3', orders: [{ id: 7 }] } }, - ]); - - await server.fetch(orderList); - - const client = cache(() => []); - - hydrate(client, transfer(dehydrate(server, { principal: undefined })), { ops }); - - const rows = client.getState(orderList).data as { customer: { orders: unknown[] } }[]; - - expect(rows[0]?.customer.orders[0]).toBe(rows[0]); - }); - - it('carries a response-templated provides tag across, so a mutation still reaches it', async () => { - const listWithResponseTag: OperationMeta = { - ...orderList, - provides: ['Order[]', 'Batch:{res.0.id}'], - }; - const server = cache(() => [{ id: 7, total: 99 }]); - - await server.fetch(listWithResponseTag); - - const client = cache(() => []); - - hydrate(client, transfer(dehydrate(server, { principal: undefined })), { - ops: { orderList: listWithResponseTag }, - }); - - expect( - client.registry.queriesFor('Batch:7').map((entry) => entry.key), - ).toEqual([client.key(listWithResponseTag)]); - }); - - it('settles fresh by default and stale when asked', async () => { - const server = cache(() => [{ id: 7, total: 99 }]); - - await server.fetch(orderList); - - const state = transfer(dehydrate(server, { principal: undefined })); - - const fresh = cache(() => []); - hydrate(fresh, state, { ops }); - expect(fresh.registry.get(fresh.key(orderList))?.stale).toBe(false); - - const verifying = cache(() => []); - hydrate(verifying, state, { ops, stale: true }); - expect(verifying.registry.get(verifying.key(orderList))?.stale).toBe(true); - }); - - it('is idempotent: hydrating twice keeps the identity of what did not move', async () => { - const server = cache(() => [{ id: 7, total: 99 }]); - - await server.fetch(orderList); - - const state = transfer(dehydrate(server, { principal: undefined })); - const client = cache(() => []); - - hydrate(client, state, { ops }); - const first = client.getState(orderList).data; - - hydrate(client, transfer(dehydrate(server, { principal: undefined })), { ops }); - - expect(client.getState(orderList).data).toEqual(first); - expect(client.store.getRecord('Order:7')?.version).toBe(1); - }); - - it('refuses a payload belonging to another principal', async () => { - const server = cache(() => [{ id: 7, total: 99 }]); - - server.setPrincipal('u-1'); - await server.fetch(orderList); - - const state = transfer(dehydrate(server, { principal: 'u-1' })); - const client = cache(() => []); - - client.setPrincipal('u-2'); - - expect(() => hydrate(client, state, { ops })).toThrow( - /\[forge\] hydrate: this payload belongs to a different principal/, - ); - }); - - it('refuses an unrecognised payload version', () => { - const client = cache(() => []); - - expect(() => - hydrate(client, { v: 2, mode: 'normalized', records: {}, queries: [] } as never, { ops }), - ).toThrow(/\[forge\] hydrate: unsupported payload version 2/); - }); - - it('refuses an operation the ops table does not name', async () => { - const server = cache(() => [{ id: 7, total: 99 }]); - - await server.fetch(orderList); - - const client = cache(() => []); - - expect(() => - hydrate(client, transfer(dehydrate(server, { principal: undefined })), { - ops: { customerList }, - }), - ).toThrow(/\[forge\] hydrate: no operation named GET \/orders/); - }); -}); -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `cd packages/client-core && npx vitest run __tests__/ssr.test.ts -t hydrate` -Expected: FAIL — `hydrate is not exported by ../src/ssr`. - -- [ ] **Step 3: Write the implementation** - -In `packages/client-core/src/ssr.ts`, extend the imports: - -```ts -import { assertAcyclic, encode, revive } from './wire'; -import type { OperationMeta } from './transport'; -``` - -and append: - -```ts -export interface HydrateOptions { - /** - * The generated `ops.ts` table, passed verbatim. - * - * A cache record holds an `OperationMeta` and needs it to refetch, to - * `watchLive` and to drive the transport -- and that is route metadata living - * in the generated manifest, not in the store, so it cannot be reconstructed - * from a payload. Serializing it instead would make this argument unnecessary - * at the cost of putting the route table into every HTML response, to - * duplicate what the client bundle already ships. - * - * Keyed however the generator keys it; the values are what matter, and they - * are re-indexed below by the same `method path` the cache keys operations by. - */ - readonly ops: Readonly>; - /** - * Settle every hydrated query behind the server, so a mount refetches. - * - * Off by default, which is right for a dynamically rendered page: the server - * fetched the data milliseconds earlier. A statically generated or ISR page - * wants it on -- instant paint, then a verifying refetch. - */ - readonly stale?: boolean; -} - -export function hydrate( - cache: QueryCache, - state: DehydratedState, - options: HydrateOptions, -): void { - if (state.v !== 1) { - throw new Error(`[forge] hydrate: unsupported payload version ${String(state.v)}`); - } - - if (!Object.is(state.principal, cache.owner)) { - throw new Error( - '[forge] hydrate: this payload belongs to a different principal -- ' + - 'set the principal before hydrating, and never hydrate a payload built for someone else', - ); - } - - const index = new Map(); - - for (const meta of Object.values(options.ops)) index.set(operationName(meta), meta); - - const metaFor = (operation: string): OperationMeta => { - const meta = index.get(operation); - - if (meta === undefined) throw new Error(`[forge] hydrate: no operation named ${operation}`); - - return meta; - }; - - const stale = options.stale === true ? { stale: true } : {}; - - if (state.mode === 'normalized') { - // Records first. A skeleton restored before the entity it references would - // read as a hole, and `restore` reads its value as it settles. - for (const [key, data] of Object.entries(state.records)) { - cache.store.put(key, revive(data) as Record); - } - - for (const query of state.queries) { - cache.restore(metaFor(query.operation), query.args, { - skeleton: revive(query.skeleton), - tags: query.tags, - ...stale, - }); - } - - return; - } - - if (state.mode === 'denormalized') { - for (const query of state.queries) { - const meta = metaFor(query.operation); - const { skeleton } = cache.store.write( - query.value, - cache.entities, - meta.rootType ?? meta.entity, - ); - - cache.restore(meta, query.args, { skeleton, response: query.value, ...stale }); - } - - return; - } - - throw new Error( - `[forge] hydrate: unrecognised payload mode ${String((state as { mode: unknown }).mode)}`, - ); -} -``` - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cd packages/client-core && npx vitest run && npm run typecheck` -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/src/ssr.ts packages/client-core/__tests__/ssr.test.ts -git commit -m "feat(client): add hydrate, reviving a payload into a warm cache" -``` - ---- - -### Task 6: The round-trip property, and the generator that can actually hit the collision - -**Files:** -- Modify: `packages/client-core/__tests__/roundtrip.property.test.ts` - -**Interfaces:** -- Consumes: `dehydrate`, `hydrate` from Task 4 and Task 5. - -- [ ] **Step 1: Write the failing test** - -In `packages/client-core/__tests__/roundtrip.property.test.ts`, replace the `propertyName` arbitrary (currently a filtered `fc.string`) with: - -```ts -/** - * `__proto__` is not a data property: assigning it walks a setter instead of - * creating a key, so neither the runtime nor this file's reference - * implementation can round-trip it. It is out of scope, not a defect. - * - * `__ref` and its escape forms are drawn deliberately rather than left to - * chance. They are the names that collide with the SSR wire encoding, and an - * arbitrary string generator will never produce one -- which is exactly how a - * response legitimately containing `{__ref: ...}` stayed a comment in `ref.ts` - * instead of becoming a test. - */ -const propertyName = fc - .oneof( - { weight: 3, arbitrary: fc.string({ minLength: 1 }) }, - { weight: 1, arbitrary: fc.constantFrom('__ref', '___ref', '____ref', '__refs', '_ref') }, - ) - .filter((key) => key !== '__proto__'); -``` - -Then append a new `describe` block at the end of the file: - -```ts -describe('the SSR round trip', () => { - it('hydrates to the value the server rendered, through JSON', () => { - fc.assert( - fc.property(response, (value) => { - const server = ssrCache(); - - server.store.write(value, schema, 'Order'); - server.restore(orderList, undefined, { - skeleton: server.store.stage(value, schema, 'Order').skeleton, - tags: [], - }); - - const expected = server.getState(orderList).data; - const wire = JSON.parse(JSON.stringify(dehydrate(server, { principal: undefined }))); - - const client = ssrCache(); - hydrate(client, wire, { ops: { orderList } }); - - expect(client.getState(orderList).data).toEqual(expected); - }), - { numRuns: 200 }, - ); - }); - - it('recomputes the same dependency set it started with', () => { - fc.assert( - fc.property(response, (value) => { - const server = ssrCache(); - const staged = server.store.stage(value, schema, 'Order'); - - server.store.write(value, schema, 'Order'); - server.restore(orderList, undefined, { skeleton: staged.skeleton, tags: [] }); - - const before = [...(server.registry.get(server.key(orderList))?.deps ?? [])].sort(); - const wire = JSON.parse(JSON.stringify(dehydrate(server, { principal: undefined }))); - - const client = ssrCache(); - hydrate(client, wire, { ops: { orderList } }); - - expect([...(client.registry.get(client.key(orderList))?.deps ?? [])].sort()).toEqual( - before, - ); - }), - { numRuns: 200 }, - ); - }); -}); -``` - -Add to the top of the file: - -```ts -import { QueryCache } from '../src/cache'; -import { manualScheduler } from '../src/invalidate'; -import { dehydrate, hydrate } from '../src/ssr'; -import type { OperationMeta } from '../src/transport'; - -const orderList: OperationMeta = { - method: 'GET', - path: '/orders', - entity: 'Order', - rootType: 'Order', - provides: [], - invalidates: [], -}; - -/** A cache with a transport that must never be reached: these tests fetch nothing. */ -function ssrCache(): QueryCache { - return new QueryCache({ - transport: { - execute: () => { - throw new Error('the SSR property tests issue no requests'); - }, - }, - entities: schema, - scheduler: manualScheduler().schedule, - }); -} -``` - -If the existing arbitrary that generates whole response trees is not named `response`, use whatever the file calls it; do not rename it. - -- [ ] **Step 2: Run the tests to verify they fail** - -Run: `cd packages/client-core && npx vitest run __tests__/roundtrip.property.test.ts` -Expected: FAIL initially only if something is genuinely broken. If they pass immediately, that is the correct outcome for this task — the property is a regression net over Tasks 1–5, and the *generator* change is the new coverage. Confirm the generator change is live by temporarily reverting the `COLLIDES` regex in `wire.ts` to `/^__ref$/`, re-running, and observing a failure; then restore it. - -- [ ] **Step 3: Verify the collision is genuinely covered** - -Run: `cd packages/client-core && npx vitest run __tests__/roundtrip.property.test.ts` -Expected: PASS with the correct `wire.ts`, FAIL with the sabotaged one. Restore `wire.ts` before continuing. - -- [ ] **Step 4: Run the whole suite** - -Run: `cd packages/client-core && npx vitest run && npm run typecheck` -Expected: all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/__tests__/roundtrip.property.test.ts -git commit -m "test(client): assert the SSR round trip, with reference-shaped keys generated" -``` - ---- - -### Task 7: `getServerState` on the query handle, and the package exports - -**Files:** -- Modify: `packages/client-core/src/client.ts:72-90` (`QueryHandle`) and `:104-120` (`query`) -- Modify: `packages/client-core/src/index.ts` -- Modify: `packages/client-core/package.json` -- Test: `packages/client-core/__tests__/client.test.ts` - -**Interfaces:** -- Consumes: `QueryCache.peek` from Task 3. -- Produces: `QueryHandle.getServerState(): QueryState`. - -- [ ] **Step 1: Write the failing test** - -Append to `packages/client-core/__tests__/client.test.ts`: - -```ts -describe('the server snapshot', () => { - it('is idle for a query the cache has nothing for, and opens no record', () => { - const client = cache(() => [{ id: 7, total: 99 }]); - const handle = query(orderList)(undefined, { client }); - - expect(handle.getServerState()).toEqual({ - status: 'idle', - data: undefined, - error: undefined, - isFetching: false, - }); - expect(client.size).toBe(0); - }); - - it('is the same object every call, so useSyncExternalStore does not tear', () => { - const client = cache(() => [{ id: 7, total: 99 }]); - const handle = query(orderList)(undefined, { client }); - - expect(handle.getServerState()).toBe(handle.getServerState()); - }); - - it('returns what the cache holds once it holds something', async () => { - const client = cache(() => [{ id: 7, total: 99 }]); - const handle = query(orderList)(undefined, { client }); - - await client.fetch(orderList); - - expect(handle.getServerState().data).toEqual([{ id: 7, total: 99 }]); - }); -}); -``` - -Match the local helper names already used in `client.test.ts`; if it builds its cache differently, use its existing helper rather than introducing one. - -- [ ] **Step 2: Run the test to verify it fails** - -Run: `cd packages/client-core && npx vitest run __tests__/client.test.ts -t "the server snapshot"` -Expected: FAIL — `handle.getServerState is not a function`. - -- [ ] **Step 3: Write the implementation** - -In `packages/client-core/src/client.ts`, add a frozen constant above `QueryHandle`: - -```ts -/** - * The server snapshot for a query the cache holds nothing for. - * - * A module-level frozen constant because `getServerSnapshot` must be - * referentially stable under a harder condition than `getSnapshot`: it is asked - * about queries no record exists for, so there is no per-record memo to lean on. - */ -const IDLE: QueryState = Object.freeze({ - status: 'idle' as const, - data: undefined, - error: undefined, - isFetching: false, -}); -``` - -Add to the `QueryHandle` interface, after `getState`: - -```ts - /** - * The snapshot a server render sees, and the one a hydrating client's first - * pass must match. - * - * `peek` rather than `getState`: this is called for queries the cache has - * never opened, and opening a record as a side effect of a *render* is wrong - * twice over -- on a server the cache may be shared between concurrent - * requests, and a discarded render would leave an entry behind. `undefined` - * from `peek` means nothing is cached, which is `idle`. - * - * Real data here is only correct because hydration exists: React compares - * this against the client's first pass and treats a difference as a mismatch, - * so a hydration boundary must have run above the component. With one, both - * sides read the same warm cache and the server emits real markup. - */ - getServerState(): QueryState; -``` - -And in `query`'s returned handle, after `getState`: - -```ts - getServerState: () => cache.peek(meta, args) ?? (IDLE as QueryState), -``` - -In `packages/client-core/src/index.ts`, add after the `QueryCache` export block: - -```ts -export { dehydrate, hydrate } from './ssr'; -export type { - DehydratedState, - DehydrateOptions, - DenormalizedQuery, - DenormalizedState, - HydrateOptions, - NormalizedQuery, - NormalizedState, -} from './ssr'; -``` - -and extend the existing `export type { ... } from './cache'` list with `CachedQuery` and `RestoreInput`. - -Update the module doc comment at the top of `index.ts` by adding a paragraph before the closing one: - -``` - * **Server rendering**: `dehydrate` serializes a cache for an HTML response and - * `hydrate` reads it back. What may cross that boundary is a property of the - * API rather than a caution in the docs -- the payload holds only the entities - * the exported queries actually reference, and both sides assert the principal. -``` - -In `packages/client-core/package.json`, add a size-limit entry after the `stream binding` one: - -```json - { - "name": "ssr", - "path": "dist/index.js", - "import": "{ dehydrate, hydrate }", - "limit": "1.5 kB", - "gzip": true - }, -``` - -and raise the `core with streams` limit from `"14 kB"` to `"15 kB"`. - -- [ ] **Step 4: Run the tests to verify they pass** - -Run: `cd packages/client-core && npx vitest run && npm run typecheck && npm run size` -Expected: all tests PASS; every size-limit entry within budget. If `core, REST only` (9 kB) has moved at all, stop — `dehydrate`/`hydrate` must tree-shake out of that import set, and a regression there means something in `cache.ts` grew more than intended. - -- [ ] **Step 5: Commit** - -```bash -git add packages/client-core/src/client.ts packages/client-core/src/index.ts \ - packages/client-core/package.json packages/client-core/__tests__/client.test.ts -git commit -m "feat(client): give a query handle a real server snapshot, and export the SSR surface" -``` - ---- - -### Task 8: The React hydration boundary and the server snapshot - -**Files:** -- Create: `packages/client-react/src/hydration.ts` -- Modify: `packages/client-react/src/useQuery.ts:33-81` and `:173` -- Modify: `packages/client-react/src/index.ts` -- Modify: `packages/client-react/package.json` (size-limit) -- Test: `packages/client-react/__tests__/ssr.test.tsx` - -**Interfaces:** -- Consumes: `dehydrate`, `hydrate`, `DehydratedState`, `HydrateOptions` from `@forge-go/client-core`; `QueryHandle.getServerState` from Task 7. -- Produces: - - `interface ForgeHydrationBoundaryProps { readonly state: DehydratedState | undefined; readonly ops: Readonly>; readonly client?: QueryCache; readonly stale?: boolean; readonly children?: ReactNode }` - - `function ForgeHydrationBoundary(props: ForgeHydrationBoundaryProps): ReactNode` - -- [ ] **Step 1: Build the core so the adapter resolves it** - -Run: `cd packages/client-core && npm run build` -Expected: `dist/ssr.js`, `dist/wire.js` and updated `dist/index.d.ts` exist. - -- [ ] **Step 2: Write the failing test** - -Create `packages/client-react/__tests__/ssr.test.tsx`: - -```tsx -import { StrictMode } from 'react'; -import { renderToString } from 'react-dom/server'; -import { act, render, screen } from '@testing-library/react'; -import { describe, expect, it } from 'vitest'; -import { dehydrate } from '@forge-go/client-core'; -import type { DehydratedState } from '@forge-go/client-core'; -import { ForgeHydrationBoundary, ForgeProvider, useQuery } from '../src'; -import { harness, orderList, useOrderList } from './harness'; -import type { Order } from './harness'; - -const ops = { orderList }; - -function Orders(): JSX.Element { - const { status, data } = useQuery(useOrderList); - - return ( -
    - {(data ?? []).map((order) => ( -
  • {order.total}
    • - ))} -
- ); -} - -/** A server render for one request: its own cache, prefetched, then dehydrated. */ -async function serverRender(): Promise<{ html: string; state: DehydratedState }> { - const server = harness(() => [{ id: 7, total: 99 }]); - - await server.cache.fetch(orderList); - - const state = dehydrate(server.cache, { principal: undefined }); - const html = renderToString( - - - - - , - ); - - return { html, state: JSON.parse(JSON.stringify(state)) as DehydratedState }; -} - -describe('server rendering', () => { - it('emits the data rather than the loading branch', async () => { - const { html } = await serverRender(); - - expect(html).toContain('data-status="success"'); - expect(html).toContain('99'); - }); -}); - -describe('hydrating', () => { - it('renders the server data on the first pass and issues no request', async () => { - const { state } = await serverRender(); - const client = harness(() => { - throw new Error('a hydrated query must not fetch'); - }); - - await act(async () => { - render( - - - - - , - ); - }); - - expect(screen.getByTestId('orders').dataset.status).toBe('success'); - expect(screen.getByText('99')).toBeTruthy(); - expect(client.transport.calls).toHaveLength(0); - }); - - it('hydrates once under StrictMode, whose renders are double-invoked', async () => { - const { state } = await serverRender(); - const client = harness(() => { - throw new Error('a hydrated query must not fetch'); - }); - - await act(async () => { - render( - - - - - - - , - ); - }); - - expect(client.cache.store.getRecord('Order:7')?.version).toBe(1); - }); - - it('refetches on mount when hydrated stale', async () => { - const { state } = await serverRender(); - const client = harness(() => [{ id: 7, total: 120 }]); - - await act(async () => { - render( - - - - - , - ); - }); - - expect(client.transport.calls).toHaveLength(1); - }); - - it('renders children unchanged when there is nothing to hydrate', async () => { - const client = harness(() => [{ id: 7, total: 99 }]); - - await act(async () => { - render( - - - - - , - ); - }); - - expect(screen.getByTestId('orders')).toBeTruthy(); - }); -}); -``` - -- [ ] **Step 3: Run the test to verify it fails** - -Run: `cd packages/client-react && npx vitest run __tests__/ssr.test.tsx` -Expected: FAIL — `ForgeHydrationBoundary` is not exported. - -- [ ] **Step 4: Write the implementation** - -Create `packages/client-react/src/hydration.ts`: - -```ts -import type { ReactNode } from 'react'; -import { hydrate } from '@forge-go/client-core'; -import type { DehydratedState, OperationMeta, QueryCache } from '@forge-go/client-core'; -import { useForgeClient } from './context'; - -/** - * Which payloads have already been hydrated into which cache. - * - * Keyed on the cache first because the same payload legitimately hydrates two - * of them -- this component renders on the server as well, against the cache - * that produced the payload, and again on the client against a fresh one. - * - * An optimisation rather than a correctness requirement: `hydrate` merges, and - * a record written with identical data keeps its previous object and bumps no - * version. What this buys is that StrictMode's double-invoked render does not - * walk the payload twice. - */ -const hydrated = new WeakMap>(); - -export interface ForgeHydrationBoundaryProps { - /** The payload from `dehydrate`, after whatever transport carried it. */ - readonly state: DehydratedState | undefined; - /** The generated `ops.ts` table, passed verbatim. */ - readonly ops: Readonly>; - /** Use this cache rather than the provided or configured one. */ - readonly client?: QueryCache; - /** Settle the hydrated queries behind the server, so mounting refetches. */ - readonly stale?: boolean; - readonly children?: ReactNode; -} - -/** - * Hydrate a payload into the cache this subtree reads from. - * - * **It hydrates during render, not in an effect.** Children read `getSnapshot` - * during their own render, which happens after this one returns, so a - * render-phase hydrate is visible to them on the first pass. An effect runs - * after the tree commits: the first paint would be the loading branch and then - * flip, which is a visible flash and, on the hydration pass, exactly the - * mismatch this component exists to remove. - * - * Rendering no element of its own is deliberate. A wrapper would change the DOM - * the server and client compare, for a component whose entire job is to make - * those two agree. - */ -export function ForgeHydrationBoundary(props: ForgeHydrationBoundaryProps): ReactNode { - const client = useForgeClient(props.client); - const { state } = props; - - if (state !== undefined) { - let seen = hydrated.get(client); - - if (seen === undefined) { - seen = new WeakSet(); - hydrated.set(client, seen); - } - - if (!seen.has(state)) { - seen.add(state); - hydrate(client, state, { - ops: props.ops, - ...(props.stale === true ? { stale: true } : {}), - }); - } - } - - return props.children ?? null; -} -``` - -In `packages/client-react/src/useQuery.ts`, delete the `IDLE` constant and the `serverSnapshot` function together with their comment block (`:33-81`), and replace the `useSyncExternalStore` call at `:173` with: - -```ts - const state = useSyncExternalStore(handle.subscribe, handle.getState, handle.getServerState); -``` - -Replace the deleted comment block with a short one above the `useQuery` export: - -```ts -/** - * The server snapshot now comes from the handle, which reads it out of the - * cache with `peek` -- see `QueryHandle.getServerState`. This file used to hold - * a frozen `idle` constant instead, because no store serialisation existed and - * returning server-fetched data would have been a guaranteed hydration mismatch - * rather than an optimisation. With `ForgeHydrationBoundary` above the tree, - * both sides read the same warm cache and a server render emits real markup. - */ -``` - -In `packages/client-react/src/index.ts`, add: - -```ts -export { ForgeHydrationBoundary } from './hydration'; -export type { ForgeHydrationBoundaryProps } from './hydration'; -``` - -and replace the final line of the module doc comment (`Streaming (\`live: true\`), devtools and SSR hydration land in later chunks.`) with: - -``` - * SSR is here: `ForgeHydrationBoundary` hydrates a payload from `dehydrate` - * during render, and a server render emits real markup rather than a spinner. -``` - -In `packages/client-react/package.json`, raise the `adapter` size-limit from `"2 kB"` to `"2.5 kB"`. - -- [ ] **Step 5: Run the tests to verify they pass** - -Run: `cd packages/client-react && npx vitest run && npm run typecheck && npm run size` -Expected: all PASS, including every pre-existing test in `useQuery.test.tsx`, `useQuery.live.test.tsx`, `context.test.tsx` and `useMutation.test.tsx`. - -- [ ] **Step 6: Commit** - -```bash -git add packages/client-react/src/hydration.ts packages/client-react/src/useQuery.ts \ - packages/client-react/src/index.ts packages/client-react/package.json \ - packages/client-react/__tests__/ssr.test.tsx -git commit -m "feat(react): hydrate during render, and emit real markup from a server render" -``` - ---- - -### Task 9: Documentation - -**Files:** -- Create: `docs/content/docs/web-client/ssr.mdx` -- Modify: `docs/content/docs/web-client/meta.json` -- Modify: `docs/content/docs/web-client/not-yet-shipped.mdx` -- Modify: `packages/client-core/README.md` -- Modify: `packages/client-react/README.md` - -- [ ] **Step 1: Add the page to the sidebar** - -In `docs/content/docs/web-client/meta.json`, insert `"ssr"` into `pages` immediately after `"adapters"`: - -```json - "---Browser runtime---", - "runtime", - "adapters", - "ssr", - "devtools", -``` - -- [ ] **Step 2: Write the SSR page** - -Create `docs/content/docs/web-client/ssr.mdx`. It must cover, in this order: - -1. Frontmatter: `title: Server rendering`, `description: Prefetch on the server, hydrate on the client, and emit real markup`, `icon: Server`. -2. The Next.js App Router example from the spec's "Package placement" section, verbatim — a server component building a per-request cache, calling `setPrincipal`, prefetching, and passing `dehydrate(...)` to a client component that wraps its tree in ``. -3. **A `` on the principal**: `dehydrate` requires it, asserts it against the cache owner, and `hydrate` refuses a payload built for anyone else. State plainly that a payload is server state embedded in an HTML response, and that the payload holds only the entities the exported queries reference — an entity nothing exported points at cannot be in it. -4. `include`, for exporting a subset. -5. The two modes as a table: `normalized` (default; smallest; dedupes shared entities; serializes entity cycles) and `denormalized` (no revive pass; duplicates shared entities; **cannot** serialize a query whose value contains an entity cycle). -6. Freshness: `stale` on both `hydrate` and the boundary, with the SSR-versus-SSG framing from the spec. -7. A short "Why a per-request cache" section: the module-level cache is shared between concurrent server requests, and while the reachability closure means one request cannot export another's entities, a per-request cache is still the correct shape — pointing at `ForgeProvider`. -8. A closing note that Vue and Angular can call `dehydrate`/`hydrate` directly but ship no boundary component. - -- [ ] **Step 3: Correct `not-yet-shipped.mdx`** - -Delete the whole `## SSR `dehydrate` / `hydrate` is not built` section, including its claim about `packages/nextjs-plugin`. - -Add to the `## Smaller gaps in the runtime` list: - -```md -- **SSR ships for React only.** `dehydrate`/`hydrate` are framework-agnostic and the Vue and Angular adapters can call them directly, but neither ships a hydration boundary component or a server-snapshot path. There is also no streamed-payload injection helper: multiple boundaries work, but flushing a payload mid-stream is the application's job. -- **A denormalized payload cannot carry an entity cycle.** `dehydrate`'s default `normalized` mode serializes `Order → Customer → Orders[] → Order` without difficulty, because it closes through references. `mode: 'denormalized'` ships the rehydrated value, which *is* such a cycle, and throws. -``` - -Add to the `## What is shipped` list, after the React/Vue/Angular adapters line: - -```md -- SSR `dehydrate`/`hydrate`, with a reachability-closed payload and principal assertions on both sides -``` - -Then re-read the page's opening paragraph and closing line (`The gap is two designed features and a handful of runtime edges…`) and correct the count: one designed feature remains unbuilt (capability gating), not two. - -- [ ] **Step 4: Update `packages/client-core/README.md`** - -Delete the first bullet of `## Known gaps, deliberately left to later chunks` — the one beginning `SSR revival.` - -Add a `## Server rendering` section after `## Stream binding`, covering: -- the `dehydrate`/`hydrate` signatures; -- the reachability closure as the security property, in the README's voice — that the payload is *built* by a walk rather than read off the store, so an entity nothing exported references cannot be in it; -- the `__ref` collision and the escape scheme, because that is the non-obvious part and the README is where this codebase records non-obvious parts; -- the two modes and the entity-cycle limitation of the denormalized one; -- that `deps` are recomputed from the skeleton rather than trusted from the wire, and `version`/`frameAt` are not carried because the frame clock is per session. - -In the size-budget section, add a sentence recording that `core with streams` moved from 14 kB to 15 kB to admit `ssr.ts`, that `dehydrate`/`hydrate` tree-shake out of an application that never imports them, and that the two application-facing budgets — `core, REST only` at 9 kB and the per-surface entries — did not move. - -- [ ] **Step 5: Update `packages/client-react/README.md`** - -Add a `## Server rendering` section: the boundary, that it hydrates during render and why, the `ops` prop, `stale`, and that `getServerSnapshot` now returns real state through `peek`. Remove any line claiming SSR is a later chunk. - -- [ ] **Step 6: Verify the docs build** - -Run: `cd docs && npm run build` (or `pnpm build`, matching whatever the `docs` package declares) -Expected: the build succeeds and `web-client/ssr` appears in the generated sidebar. - -- [ ] **Step 7: Commit** - -```bash -git add docs/content/docs/web-client packages/client-core/README.md packages/client-react/README.md -git commit -m "docs(client): document SSR dehydrate/hydrate and retire the not-yet-shipped section" -``` - ---- - -### Task 10: Full verification and the website mirror - -**Files:** -- No source changes. Mirrors generated docs into `/Users/rexraphael/Work/xraph/website`. - -- [ ] **Step 1: Run everything, from a clean build** - -```bash -cd packages/client-core && npm run build && npx vitest run && npm run typecheck && npm run size -``` - -Expected: build clean, all tests PASS, no type errors, every size-limit entry within budget. - -- [ ] **Step 2: Run the adapter against the freshly built core** - -```bash -cd packages/client-react && npx vitest run && npm run typecheck && npm run size -``` - -Expected: all PASS. Also run the Vue and Angular adapters, which consume the same `dist` and must not have regressed: - -```bash -cd packages/client-vue && npx vitest run -cd packages/client-angular && npx vitest run -``` - -- [ ] **Step 3: Mirror the docs to the website** - -```bash -cd /Users/rexraphael/Work/xraph/website && pnpm docs:import /Users/rexraphael/Work/xraph/forge forge v1 -``` - -Expected: `content/docs/forge/v1/web-client/ssr.mdx` appears and `not-yet-shipped.mdx` is updated. **That tree is generated — never hand-edit it.** If the import reports a failure, fix the source under `docs/content/docs/` in the forge repo and re-run rather than touching the mirror. - -- [ ] **Step 4: Commit the mirror** - -```bash -cd /Users/rexraphael/Work/xraph/website -git add content/docs/forge/v1 -git commit -m "docs(forge): mirror v1 web-client SSR page" -``` - -- [ ] **Step 5: Report** - -State plainly: the test counts for each package, whether every size budget held, and which budget figure moved and why. - ---- - -## Self-Review - -**Spec coverage.** Every spec section maps to a task: the revive pass and escaping → Task 1; `SettleResult.tags` → Task 2; `peek`/`restore`/`settledQueries` → Task 3; the payload, the closure, the principal, the modes and the cycle errors → Tasks 4–5; the round-trip property and the generator fix → Task 6; `getServerState`, exports and budgets → Task 7; the boundary and `getServerSnapshot` → Task 8; every documentation change including the `nextjs-plugin` correction → Task 9; the website mirror → Task 10. - -**Type consistency.** `EncodeContext`/`EncodeResult` (Task 1) are consumed unchanged in Task 4. `RestoreInput` (Task 3) is what `hydrate` passes in Task 5. `CachedQuery` (Task 3) is what `select` returns in Task 4. `HydrateOptions` (Task 5) is what the boundary spreads in Task 8. `operationName` is exported in Task 3 and imported in Tasks 4 and 5. - -**Known judgement calls left to the implementer.** Task 6 says to match the existing arbitrary's name rather than assuming `response`, and Task 7 says to match `client.test.ts`'s existing cache helper. Both are named explicitly because guessing them from here would be inventing an API that already exists. diff --git a/docs/superpowers/specs/2026-08-08-sse-event-replay-design.md b/docs/superpowers/specs/2026-08-08-sse-event-replay-design.md deleted file mode 100644 index 3adaf04a..00000000 --- a/docs/superpowers/specs/2026-08-08-sse-event-replay-design.md +++ /dev/null @@ -1,310 +0,0 @@ -# SSE Event Replay: Resumable Streams and Honest Gap Recovery - -Date: 2026-08-08 -Status: Approved, pending implementation plan - -## Problem - -A client whose stream drops and reconnects has missed events. It has no way to learn -*which* ones, so the only correct recovery available to it is to assume it missed -everything: `StreamBinder.recover` (`packages/client-core/src/live.ts:589`) invalidates -every list tag on the channel and refetches every registered live query. - -That recovery is correct and deliberately blunt. Its own comment says so — a gap is every -event that did not arrive, so recovery has to be broader than any single event. The cost is -paid on every reconnect regardless of whether one event was missed or ten thousand: a laptop -lid closed for four seconds triggers the same full resync as an hour-long outage. - -The server holds the information that would make a precise gap-fill possible and throws it -away. Commit `095a3887` added the transport primitive — `SendWithID`, `SendJSONWithID` and -`LastEventID` on the `Stream` interface — but nothing produces IDs and nothing reads the -resume position. The capability exists and has no callers. - -### What the original finding got right, and what it got wrong - -The finding that motivated `095a3887` claimed the missing event IDs are *why* -`StreamBinder.recover` must refetch. The first half is right and the second half names the -wrong transport. - -Two independent facts, both established by reading the generated clients: - -- **The SSE client is already built and waiting.** `internal/client/generators/typescript/sse.go` - tracks `lastEventId` and sends it on reconnect as both a `Last-Event-ID` header and a - `lastEventId` query parameter (`sse.go:307-356`). It has been sending a resume position all - along to a server that ignored it. The SSE half of this work needs no client changes. - -- **`StreamBinder.recover` is not on SSE.** Channels and rooms generate a *WebSocket* client - (`channels.go`, `rooms.go`). WebSocket has no `Last-Event-ID`; that header is part of the - SSE spec and has no analogue in the WebSocket protocol. Server-side event IDs are necessary - for the live-query fix but not sufficient, because the live-query transport cannot carry the - resume position without an application-level handshake. - -So there are two paths needing two mechanisms, and only one of them is cheap today: - -| Path | Transport | Resume mechanism | Client work | -| --- | --- | --- | --- | -| `EventStream` routes | SSE | `Last-Event-ID` (native) | already built | -| Live queries, channels, rooms | WebSocket | app-level resume frame | new | - -**This spec covers the SSE path only.** WebSocket resume is deferred to a follow-up spec. -The consequence must be stated plainly: *the live-query refetch this work was originally -motivated by is not fixed by this spec.* What this spec delivers is the event log, the wire -contract, and the client-side recovery machinery — all three of which the WebSocket phase -reuses unchanged, and none of which can be designed well without a working transport to -prove them against. - -## Goals - -- A replayable event log with pluggable storage and a bounded in-memory default -- A resume that is *honest*: the server never implies it filled a gap it could not fill -- Fail-safe degradation — any failure, at any layer, reduces to today's full-resync behavior -- No behavior change for streams that do not opt in -- A wire contract the WebSocket phase can adopt without renegotiation - -## Non-goals - -- WebSocket resume (follow-up spec; the live-query path stays on full resync) -- Cross-instance replay in the default implementation (see Multi-instance, below) -- Exactly-once delivery. Replay is at-least-once and deduplicated by ID on the client -- Retrofitting IDs onto existing `Send`/`SendJSON` callers, which stay ID-free - -## Decisions - -Four decisions were taken before design, and the rest of this document follows from them: - -1. **Scope is end-to-end** — root module, `packages/client-core`, and `extensions/streaming`. -2. **Best-effort replay with an explicit gap signal.** The server replays what it holds; when - it cannot honor a resume position it says so, and the client falls back to full resync. -3. **Pluggable `EventLog` with a bounded in-memory default**, so single-instance deployments - work with no configuration and multi-instance deployments can substitute a shared log. -4. **SSE first; WebSocket as phase 2.** - -## Architecture - -### Layer 1 — the `EventLog` contract (root) - -Defined in `internal/router`, re-exported from the root `forge` package alongside -`Connection` and `Stream`, following the existing alias pattern in `streaming.go`. - -```go -// LoggedEvent is one recorded event, as it will be replayed. -type LoggedEvent struct { - ID string - Event string - Data []byte -} - -type EventLog interface { - // Append records an event on a channel and returns the ID assigned to it. - Append(ctx context.Context, channel, event string, data []byte) (string, error) - - // Since returns the events recorded after id, in order. - // - // resumable reports whether id was still resolvable in the log. False means - // the gap cannot be filled and the caller must fall back to a full resync; - // events is empty in that case and must not be treated as "nothing missed". - Since(ctx context.Context, channel, id string) (events []LoggedEvent, resumable bool, err error) -} -``` - -The `resumable` boolean is the whole design in one return value. The alternative — returning -an empty slice for both "nothing was missed" and "I cannot tell you what was missed" — makes -the dangerous case indistinguishable from the safe one at the call site, and the dangerous -case is the one that silently serves stale data. A separate return value cannot make ignoring -it a compile error, since `_` is always available; what it does is make ignoring it *visible* -in the source, which is the most a signature can do here. - -### ID scheme - -`-`, where `epoch` identifies the log generation and `seq` is a per-channel -monotonic counter rendered in decimal. Example: `7f3a9c1e-42`. - -Both components are decimal/hex text, so an ID is newline-free by construction and passes -`validSSEFieldValue` without a special case. - -The epoch exists because `seq` alone is unsafe across a restart. A fresh process restarts its -counters at zero, so a client resuming from `41` would be told that events `42…` are its -missed events when they are in fact entirely different events that happen to reuse the -numbers. Comparing epochs turns that silent mis-replay into an honest `resumable=false`. - -`Since` resolves `resumable` as follows, and every unresolvable case is false rather than a -guess: - -| Condition | `resumable` | -| --- | --- | -| `id` is malformed | false | -| `id` epoch ≠ current epoch | false | -| `seq` older than the oldest retained entry | false | -| `seq` newer than the newest entry | false | -| otherwise | true, with events at `seq > id.seq` | - -The "newer than newest" row covers a client that reconnects to an instance behind the one it -was talking to. It cannot be served correctly, so it is not served at all. - -### In-memory default - -A per-channel ring buffer bounded by both entry count and age, evicting on whichever binds -first. Defaults: 1024 entries per channel, 5 minutes. Both configurable. A count bound alone -lets a quiet channel retain events long past their usefulness; an age bound alone lets a busy -channel consume unbounded memory. The pairing is what makes the footprint predictable. - -Eviction is what produces `resumable=false` for the expired case, so retention is a tuning -knob on *how often clients fall back*, never on correctness. - -### Multi-instance - -The in-memory log is per-process, so each instance has a distinct epoch and a reconnect -landing on a different instance resolves to `resumable=false` and a full resync. That is the -correct outcome, arrived at honestly, and it is exactly today's behavior — a multi-instance -deployment that does not configure a shared log is no worse off than before. A Redis or -NATS-backed `EventLog` sharing one epoch across instances is the supported upgrade and needs -no transport changes. - -### Layer 2 — SSE replay wiring (root) - -Opt-in per route, via a route option carrying both the log and the channel the route's -events belong to: - -```go -router.EventStream("/orders/live", handler, forge.WithEventLog(log, channelFor)) -``` - -`channelFor` derives the log channel from the request (`func(Context) string`), so one route -serving per-tenant or per-resource streams keeps a separate log partition per client rather -than replaying one client's events to another. A route whose stream is global returns a -constant. Handlers on a logged route append through the stream — `SendWithID` is not called -directly by application code; the wiring assigns IDs so that the log and the wire cannot -disagree about them. - -A stream with no log configured behaves exactly as it does today. - -On connect, before the handler runs: - -1. Read `stream.LastEventID()`. Empty means a fresh client — no replay, no control event. -2. Call `log.Since(ctx, channel, id)`. -3. `resumable` with at least one event → replay each via `SendWithID`, then emit - `forge.resumed`. -4. `!resumable` → emit `forge.gap` immediately, with no replay. -5. `resumable` with zero events → `forge.resumed` only on a log registered with - `WithProducerEventLog`; otherwise `forge.gap`. A log written by connections alone records - nothing while nobody is connected, so an empty result there cannot be told apart from - nothing having been recorded, and the client that was the log's only writer is by - construction at the head when it returns. Only a producer-written log can say "you missed - nothing" and mean it. - -Replay reads the log up to its current head and then subscribes from that head. Events -arriving during replay may therefore be delivered twice. That is deliberate: at-least-once -plus client-side dedup by ID is far simpler to make correct than an exactly-once handoff, and -`extensions/streaming` already has the dedup (`dedup.go`, sharded, keyed on message ID) that -makes duplicates harmless. - -### Wire contract - -Two reserved event names in a `forge.` namespace. Applications must not emit them; the -replay wiring is their only producer. - -``` -event: forge.resumed -data: {"from":"7f3a9c1e-41","count":12} - -event: forge.gap -data: {"reason":"unresumable"} -``` - -`forge.resumed` is sent **after** the replayed batch, making it an end-of-replay marker: a -client that receives it knows both that the gap was filled and that the fill is complete. -`forge.gap` is sent immediately, with no replay. - -`reason` carries the single value `"unresumable"`. An earlier draft of this spec enumerated -`expired`, `epoch`, `malformed`, and `unknown`; the implementation deliberately does not, -because `EventLog.Since` reports resumability as a bool and the wiring therefore never -establishes which of the four applies. Naming one would be a guess presented as a diagnosis, -and a diagnosis is the one thing a log line is read as. The client treated all four -identically in any case, so the distinction bought nothing on the wire. A log that genuinely -knows the cause is free to widen the interface later; until it does, the honest answer is the -one value. - -### Layer 3 — broker integration (`extensions/streaming`) - -- Publishing appends to the log and broadcasts with the returned ID. -- SSE-backed connections send via `SendWithID`. -- `SessionSnapshot` (`session_store.go:10`) gains `LastEventIDs map[string]string`, a - per-channel resume position. The snapshot already records `Channels` and `DisconnectedAt` - but no position, which is precisely the field that makes a resumption able to resume. - -This module is mid-refactor and does not currently compile (`MessageTypeError` and -`MessageTypeSystem` undefined in `extension.go`), so this layer is sequenced last and gated -on it building. Nothing in Layers 1, 2 or 4 depends on it. - -### Layer 4 — conditional recovery (`packages/client-core`) - -`StreamBinder.recover` becomes conditional. The difficulty is one of ordering: `onReconnect` -fires when the socket opens, which is *before* any control event can have arrived, so -recovery cannot simply be skipped on a flag that does not exist yet. - -Recovery is therefore deferred rather than cancelled, resolving on the first of: - -| Signal | Outcome | -| --- | --- | -| `forge.resumed` arrives | recovery cancelled — the gap was filled | -| `forge.gap` arrives | recovery runs immediately | -| `resumeGrace` elapses (default 1000ms) | recovery runs | -| control event malformed | recovery runs | - -Only the first row is a behavior change. Every other path runs exactly today's -invalidate-and-refetch, so a server that does not implement replay, a dropped control event, -a malformed payload, and a transport that cannot carry the resume position all converge on -current behavior. The worst case is a refetch delayed by `resumeGrace`, never a refetch that -should have happened and did not. - -`resumeGrace` is configurable on `SubscriptionManagerOptions`. Setting it to 0 disables -deferral and restores today's unconditional behavior exactly, which is the escape hatch if -the deferral proves problematic in practice. - -## Testing - -**`EventLog` contract tests**, written against the interface and run against the in-memory -implementation, so a future Redis-backed log inherits them: append-then-since round trip, -eviction by count, eviction by age, epoch mismatch, malformed ID, seq-ahead-of-head, and an -empty-but-resumable result distinguished from an unresumable one. That last pair is the -distinction the whole design rests on and deserves an explicit test. - -**SSE wiring tests**: fresh connect emits no control event; resumable connect replays in -order then emits `forge.resumed` with a matching count; unresumable connect emits -`forge.gap` and no replayed events; a stream with no log configured is byte-identical to -today's output. That last one is the regression guard for the opt-in claim. - -**Client tests** (`live.ts`): recovery cancelled on `forge.resumed`; recovery runs on -`forge.gap`; recovery runs on grace-window expiry with no control event; recovery runs on a -malformed control payload; `resumeGrace: 0` reproduces current behavior. The existing tests -use a manual clock (`manualClock().sleep`), so the grace window is testable without real -timers. - -**Cross-module**: an end-to-end replay through the broker, gated on `extensions/streaming` -compiling. - -## Risks - -**`extensions/streaming` is a moving target.** It is red and actively being refactored by a -parallel workstream. Layer 3 is sequenced last and every other layer is independent of it, so -the work does not stall — but Layer 3's estimate is unreliable until that module builds, and -the `SessionSnapshot` change needs coordination with whoever is editing `session_store.go`. - -**The headline problem stays open.** Live queries ride WebSocket and keep refetching on -reconnect until phase 2. Anyone reading commit `095a3887` or this spec's title could -reasonably assume otherwise, which is why it is stated in the Problem section, in the -Non-goals, and here. - -**`resumeGrace` adds latency to genuine gap recovery.** A server that never sends control -events makes every reconnect refetch 1000ms later than it does today. Mitigated by the -`forge.gap` fast path for servers that do implement the contract, and by `resumeGrace: 0` -for those that do not. - -## Sequencing - -1. `EventLog` interface, in-memory implementation, contract tests — no dependencies -2. SSE replay wiring and the `forge.resumed` / `forge.gap` contract — depends on 1 -3. `client-core` conditional recovery — depends on the wire contract in 2, not its code -4. Broker integration in `extensions/streaming` — depends on 1, 2, and that module compiling - -Steps 1–3 are independently shippable and leave the system in a working state at each point. diff --git a/docs/superpowers/specs/2026-08-08-unified-streams-hooks-generation-design.md b/docs/superpowers/specs/2026-08-08-unified-streams-hooks-generation-design.md deleted file mode 100644 index 607f94fe..00000000 --- a/docs/superpowers/specs/2026-08-08-unified-streams-hooks-generation-design.md +++ /dev/null @@ -1,180 +0,0 @@ -# Unified streams + hooks generation - -**Date:** 2026-08-08 -**Status:** Approved, ready for implementation planning -**Scope:** `internal/client`, `cmd/forge/plugins` - -## Problem - -`forge client generate` reads exactly one specification document. Forge emits its -REST operations to OpenAPI and its stream bindings to AsyncAPI, so no single -invocation can produce a package containing both: - -| Source | You get | You do not get | -|---|---|---| -| `openapi.json` | `ops.ts`, `hooks.ts`, `rest.ts` | `websocket.ts`; `streams` is empty | -| `asyncapi.json` | `websocket.ts`, `events.ts` | `ops.ts`, `hooks.ts` | - -The consequence is that `{ live: true }` — documented, and with runtime support -built and tested in `@forge-go/client-core` — has a populated manifest only for a -source document carrying both, which Forge's own two documents do not produce -through the CLI. The feature is written and unreachable. - -## Current state - -Verified against the tree at `95e55b70`: - -- `APISpec` (`internal/client/ir.go:6`) already carries `Endpoints`, - `WebSockets`, `SSEs` and `WebTransports` in one struct. The IR is unified. -- `parseOpenAPI` (`spec_parser.go:104`) fills `Endpoints` and never touches - streams. `parseAsyncAPI` (`spec_parser.go:222`) fills `WebSockets`/`SSEs` at - lines 346 and 350. Each parse half-populates the same shape. -- The TypeScript generator is already written for the both-populated case. Its - emission decisions are per-section and data-driven: - `isAsyncAPIOnly := config.HasAnyStreamingFeature() && len(spec.Endpoints) == 0` - (`generators/typescript/generator.go:266`), with `ops`/`rest` gated on - `len(spec.Endpoints) > 0` (line 285) and `hooks`/manifest on - `config.HooksEnabled() && len(spec.Endpoints) > 0` (line 300). Feed it a spec - with both halves and every gate opens by itself. -- `resolveEntityFields` runs *inside* each parse (`spec_parser.go:63`) and once - in `Introspector.Introspect` (`introspector.go:64`). -- `Introspector` (`introspector.go:21`, taking a `router.Router`) already - produces a both-populated spec — `Endpoints` at lines 53 and 196, `WebSockets` - and `SSEs` at 271 and 275 — and has no non-test callers. (The unrelated - `infra.Introspector` in `cmd/forge/plugins` does app discovery.) -- `SourceConfig` (`cmd/forge/plugins/client_config.go:51`) holds a single `Path` - or `URL`; `--from-spec` and `--from-url` are singular - (`cmd/forge/plugins/client.go:93`). -- `spec.Warnings` already exists and is surfaced by the generators - (`internal/client/envelope.go`, `introspector.go:1162`). - -**Therefore this is an input-stage problem, not a codegen problem.** The -generator and the IR already support the destination. - -## Design - -### Architecture - -``` -sources[] → parse each (resolution deferred) → MergeSpecs → resolveEntityFields (once) → existing generator -``` - -Resolution must move after the merge. An `Order` defined in the REST document -and a stream binding referencing `Order` in the AsyncAPI document form a -cross-document edge; resolving per-document resolves it before the other half -exists. - -The single-source path keeps its current behaviour by composing the two steps. - -### Components - -| Component | Change | -|---|---| -| `internal/client/merge.go` | **New.** `MergeSpecs(specs ...*APISpec) *APISpec` and the collision policy. The only genuinely new logic. | -| `SpecParser` | Split `Parse` into `parseDocument` (detect + parse, no resolution) and `Parse` (`parseDocument` + resolve). Existing callers of `Parse` are unaffected. | -| `SourceConfig` | Gains one ordered `Sources []SourceEntry` (each entry a type plus a path or a URL), replacing the scalar `Path`/`URL`. A single list rather than parallel `paths`/`urls` arrays, so ordering across mixed file and URL sources is well defined. The scalar `path`/`url` keys keep working, read as a one-element list. CLI takes repeatable `--from-spec` / `--from-url`, appended in argument order. | -| `generationPlan` | Carries source lists rather than a single `specPath`/`specURL`. | -| `resolveWatchSource` (`client_watch.go:216`) | Watches every file source instead of one. | -| `Introspector` | Wired as an optional source. Returns a both-populated spec, so it enters the merge as one element with no special case. | -| TypeScript / Go generators | **Unchanged.** | - -### Merge semantics - -OpenAPI is authoritative for shared types, because it carries full -request/response schemas; AsyncAPI fills only what is absent. - -| Field | Rule | -|---|---| -| `Info` | First OpenAPI source wins; with no OpenAPI source, the first source in merge order. Others ignored | -| `Endpoints` | Union | -| `WebSockets`, `SSEs`, `WebTransports` | Union | -| `Schemas` | OpenAPI wins by name; differing shape emits a warning | -| `Entities` | OpenAPI wins by name; differing `IDField` emits a warning | -| `RoutingTypes` | Discarded before merge; rebuilt by `resolveEntityFields` | -| `Servers` | Union, deduped by URL | -| `Security` | Union, deduped by scheme name; OpenAPI wins | -| `Tags` | Union, deduped by name | -| `Warnings` | Concatenated, plus collision warnings | - -**Precedence follows document type, not argument order.** Sources are ordered -OpenAPI-first before merging, so `--from-spec async.json --from-spec openapi.json` -produces byte-identical output to the reverse. `determinism_test.go` exists; -making output depend on typing order would undercut it. - -**`RoutingTypes` is rebuilt, not merged.** Its doc comment -(`ir.go:18`) states the two maps "are disjoint by construction: -`resolveEntityFields` builds this one as the useful types MINUS the entities, and -it is the only writer." Merging two pre-built maps breaks that invariant — a type -that is routing-only in one document and a full entity in the other would land in -both — and `spec.Entities[name]` is read at several call sites as the question -"is this an entity". Dropping and rebuilding preserves the invariant. - -## Error handling - -**Warnings** (generation proceeds), via the existing `spec.Warnings`: - -- A schema or entity name declared in both documents with a differing shape or - `IDField`. Reports the name, both sources, and which won. -- Duplicate `path` + `method` across two OpenAPI sources. - -Identical redeclaration across documents is silent — it is the normal case, not -a conflict. - -**Hard errors** (generation stops): - -- No sources resolved. -- Any single source fails to parse. Partial generation is the exact failure this - feature removes: a package with a silently-empty `streams` table. A broken - AsyncAPI document must not degrade into a REST-only package. -- Merged spec has neither endpoints nor streams. - -**Merging a single spec is identity.** A lone AsyncAPI source still produces -`websocket.ts`/`events.ts` with no `ops.ts`, and `isAsyncAPIOnly` still evaluates -true. This is a regression risk, not a feature, and is pinned by tests. - -## Testing - -- **Unit** — `MergeSpecs` table tests per field rule: union, dedup, - OpenAPI-wins precedence, warning emitted on genuine shape disagreement and not - on identical redeclaration. -- **Determinism** — extend `determinism_test.go`: the same two sources in either - argument order produce byte-identical output. -- **Regression goldens** — single-source OpenAPI and single-source AsyncAPI - outputs unchanged. The merge path reroutes both, so this is non-negotiable. -- **E2E** — extend `e2e_specfile_test.go` with a two-document fixture asserting - one package containing `ops.ts`, `hooks.ts`, `rest.ts`, `websocket.ts` and - `events.ts`, **and a non-empty `streams` manifest**. The file list alone would - pass while `{ live: true }` still did nothing. -- **Cross-document entity resolution** — a stream binding in the AsyncAPI - document referencing an entity defined only in the OpenAPI document resolves - its field edges. This is the case per-document resolution gets wrong, so it is - the test that proves the deferral was necessary. - -## Scope - -**In:** `merge.go`; the `SpecParser` split; multi-source `SourceConfig` and CLI -flags; `generationPlan` carrying lists; `resolveWatchSource` watching every file -source; the introspector as an optional source. - -**Out, deliberately:** - -- `forge client diff` stays one document per side. Diff compares two *versions*; - making each side independently multi-source is a second feature, and - conflating them would make a cache-breaking-change report depend on merge - precedence. -- Capability gating, SSR `dehydrate`/`hydrate`, and optimistic overlays. -- The seven runtime edges listed in `packages/client-core/README.md`. -- Changing Forge's spec emission to a single combined document. That option - stays available later; this design does not foreclose it. - -## Follow-up - -The **field-renaming bug** sits on the same path and should be the next spec. -Under any non-`preserve` naming, hooks return wire-cased fields while `rest.ts` -returns renamed ones from the same package, contradicting the generated types. -Unified generation puts both in one package for the first time, which makes the -inconsistency far easier to hit. Per the runtime README, closing it needs the two -codec ids on `OperationMeta` **and** the `entities` table renamed in the same -change — `opsmanifest.go` emits `idField` and `fields` as verbatim wire names, so -renaming one without the other silently stops the normalizer finding ids, and a -type whose id field is absent is not an entity, so nothing reports it.