fix(client): always create cache buster on 409 and fix parked frame leak in #start

Two fixes:

1. Both 409 handlers (#requestShape and #fetchSnapshotWithRetry) now
   unconditionally create a cache buster instead of only doing so
   conditionally when the handle is missing or recycled. This eliminates
   the same-handle 409 infinite loop (where identical retry URLs would
   hit CDN cache forever) and removes two conditional branches, making
   the behavior safer and easier to verify exhaustively.

2. Changed `await this.#start(); return` to `return this.#start()` in
   the onError retry path. The old pattern parked the outer #start frame
   on the call stack for the entire lifetime of the replacement stream,
   accumulating one frame per error recovery. The new pattern resolves
   the outer frame immediately.

Also adds model-based test commands for 409-no-handle and 409-same-handle
scenarios, plus a targeted regression test verifying consecutive same-handle
409s produce unique retry URLs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: KyleAMathews

packages/typescript-client/src/client.ts

@@ -772,8 +772,7 @@ export class ShapeStream<T extends Row<unknown> = Row>
 
           // Restart from current offset
           this.#started = false
-          await this.#start()
-          return
+          return this.#start()
         }
         // onError returned void, meaning it doesn't want to retry
         // This is an unrecoverable error, notify subscribers
@@ -911,15 +910,14 @@ export class ShapeStream<T extends Row<unknown> = Row>
         }
 
         const newShapeHandle = e.headers[SHAPE_HANDLE_HEADER]
-        let nextRequestShapeCacheBuster: string | undefined
         if (!newShapeHandle) {
           console.warn(
             `[Electric] Received 409 response without a shape handle header. ` +
               `This likely indicates a proxy or CDN stripping required headers.`,
             new Error(`stack trace`)
           )
-          nextRequestShapeCacheBuster = createCacheBuster()
         }
+        const nextRequestShapeCacheBuster = createCacheBuster()
         this.#reset(newShapeHandle)
 
         // must refetch control message might be in a list or not depending
@@ -1935,22 +1933,16 @@ export class ShapeStream<T extends Row<unknown> = Row>
         // For snapshot 409s, only update the handle — don't reset offset/schema/etc.
         // The main stream is paused and should not be disturbed.
         const nextHandle = e.headers[SHAPE_HANDLE_HEADER]
-        let nextCacheBuster: string | undefined
         if (nextHandle) {
           this.#syncState = this.#syncState.withHandle(nextHandle)
-          // If 409 returned the same handle, the URL won't change —
-          // pass a cache buster to the next retry to force a unique URL.
-          if (nextHandle === usedHandle) {
-            nextCacheBuster = createCacheBuster()
-          }
         } else {
           console.warn(
             `[Electric] Received 409 response without a shape handle header. ` +
               `This likely indicates a proxy or CDN stripping required headers.`,
             new Error(`stack trace`)
           )
-          nextCacheBuster = createCacheBuster()
         }
+        const nextCacheBuster = createCacheBuster()
 
         return this.#fetchSnapshotWithRetry(
           opts,

packages/typescript-client/test/model-based.test.ts

@@ -102,6 +102,22 @@ function make409(newHandle: string): Response {
   )
 }
 
+/**
+ * 409 Conflict without a handle header. Simulates a proxy stripping
+ * the header. Must always produce a unique retry URL via cache buster.
+ */
+function make409NoHandle(): Response {
+  return new Response(
+    JSON.stringify([{ headers: { control: `must-refetch` } }]),
+    {
+      status: 409,
+      headers: {
+        'content-type': `application/json`,
+      },
+    }
+  )
+}
+
 /** Valid headers but non-array body. Throws FetchError → onError. */
 function makeMalformed200(handle: string): Response {
   nextSeq()
@@ -484,6 +500,82 @@ class RespondMissingHeadersCmd
   }
 }
 
+/**
+ * 409 Conflict without a handle header (proxy stripped it).
+ * Handled by creating a random cache buster. Does NOT affect
+ * the retry counter — same as normal 409.
+ */
+class Respond409NoHandleCmd
+  implements fc.AsyncCommand<StreamModel, StreamReal>
+{
+  check(m: Readonly<StreamModel>): boolean {
+    return !m.terminated
+  }
+  async run(_m: StreamModel, r: StreamReal): Promise<void> {
+    const prevUrl = r.gate.lastUrl
+    await r.respond(make409NoHandle())
+    r.currentHandle = `` // handle cleared after 409 with no handle
+    expect(r.subscriberError).toBeNull()
+    assertGlobalInvariants(r)
+    if (prevUrl) {
+      expect(r.gate.lastUrl).not.toBe(prevUrl)
+    }
+  }
+  toString(): string {
+    return `Respond409NoHandle`
+  }
+}
+
+/**
+ * 409 with the same handle as the current one.
+ */
+class Respond409SameHandleCmd
+  implements fc.AsyncCommand<StreamModel, StreamReal>
+{
+  check(m: Readonly<StreamModel>): boolean {
+    return !m.terminated
+  }
+  async run(_m: StreamModel, r: StreamReal): Promise<void> {
+    const prevUrl = r.gate.lastUrl
+    // 409 with the SAME handle — tests cache buster for handle recycling
+    await r.respond(make409(r.currentHandle))
+    // Don't update r.currentHandle — it's the same
+    expect(r.subscriberError).toBeNull()
+    assertGlobalInvariants(r)
+    if (prevUrl) {
+      expect(r.gate.lastUrl).not.toBe(prevUrl)
+    }
+  }
+  toString(): string {
+    return `Respond409SameHandle`
+  }
+}
+
+// ─── Scenario Tests ────────────────────────────────────────────────
+
+describe(`ShapeStream targeted scenario tests`, () => {
+  it(`consecutive 409s with the same handle produce unique retry URLs`, async () => {
+    const real = await createStreamReal()
+    try {
+      // Advance to a non-initial offset so the first 409 reset is visible
+      await real.respond(make200WithData(real.currentHandle))
+
+      // First 409 with same handle — URL changes because offset resets to -1
+      const urlBefore = real.gate.lastUrl
+      await real.respond(make409(real.currentHandle))
+      expect(real.gate.lastUrl).not.toBe(urlBefore)
+
+      // Second 409 with same handle — offset is already -1, handle unchanged.
+      // Without a cache buster, the retry URL would be identical.
+      const urlAfterFirstRetry = real.gate.lastUrl
+      await real.respond(make409(real.currentHandle))
+      expect(real.gate.lastUrl).not.toBe(urlAfterFirstRetry)
+    } finally {
+      real.cleanup()
+    }
+  })
+})
+
 // ─── Property Tests ─────────────────────────────────────────────────
 
 describe(`ShapeStream model-based property tests`, () => {
@@ -498,6 +590,8 @@ describe(`ShapeStream model-based property tests`, () => {
             fc.constant(new Respond204Cmd()),
             fc.constant(new Respond400Cmd()),
             fc.constant(new Respond409Cmd()),
+            fc.constant(new Respond409SameHandleCmd()),
+            fc.constant(new Respond409NoHandleCmd()),
             fc.constant(new RespondMalformed200Cmd()),
             fc.constant(new RespondMissingHeadersCmd()),
           ],