Repository navigation
Speed up thread loading over Connect - #909
Merged
Merged
Conversation
brsbl
marked this pull request as ready for review
July 30, 2026 21:23
SawyerHood
added a commit
that referenced
this pull request
Jul 30, 2026
This reverts commit dce86d9.
SawyerHood
added a commit
that referenced
this pull request
Jul 30, 2026
…915) Restores #909 (reverted in 17d652c) and fixes the response corruption that forced the revert. ## The corruption #909 lets the tunnel client forward the origin's compressed bytes verbatim. The gate then rebuilt those bytes into new `Response` objects under workerd's default `encodeBody: "automatic"`, which means *"this body is identity and I own the encoding"* — so workerd dropped `content-encoding` and shipped gzip bytes labelled `text/html`. Browsers rendered raw gzip. Two sites relayed pre-encoded bodies that way, and the correct fix is **asymmetric**: | Site | Body it wraps | Encoding | |---|---|---| | `tunnel-do.ts` resp-head | raw origin bytes from tunnel frames | `manual` — was corrupt | | `cache.ts` **hit** | Cache API stores bytes still compressed | `manual` — was corrupt | | `cache.ts` **miss** | workerd content-decodes a subrequest body as the gate reads it | **automatic** — already correct | Marking the miss path `manual` (the symmetric-looking fix) actively breaks it: it advertises gzip over plaintext and clients fail with `Z_DATA_ERROR`. The regression test catches that. ## Regression test `apps/connect/src/response-encoding.test.ts` drives the **real `TunnelDO` and real `serveWithCache`** inside workerd (miniflare), with a fake tunnel client relaying gzip frames over a real WebSocket. Node's `Response` ignores `encodeBody`, so a Node-only test would pass with or without this fix. Mutation-checked: reverting the `tunnel-do.ts` call site fails 4 tests, reverting the `cache.ts` hit path fails 1. Two control routes pin workerd's default behaviour so the tests document the bug rather than merely guarding it. ## Verification on real Cloudflare Deployed to staging (`bb-connect-staging`, version `0946eaab`, `sawyer.vibecodethis.site`). Because the tunnelled path needs a client + owner session, the end-to-end check ran against a throwaway workers.dev probe running this same `TunnelDO` + `cache.ts`, driven by the **real `TunnelSession` client** in front of the real built bb app (383 precompressed `.gz` assets, served as `apps/server` does). Probe deleted afterwards. - `index.html`: gzip-only client got 1509 raw bytes (`1f8b0800`) gunzipping to exactly 4255 bytes of real HTML; browser-style clients got it transcoded to zstd by the edge — which the edge can only do if it correctly understands the body as encoded. - 342 KB app bundle, miss → hit → hit: `x-bb-cache: miss`/`hit` plus `cf-cache-status: HIT`, every response gunzipping to sha256 `18cb8f3a…`, byte-identical to the origin file. - Legacy control at the edge reproduces the production symptom: `content-encoding` dropped, edge zstd-wraps the raw gzip, browser renders `^_M-^K^H…` as `text/html`. - `Accept-Encoding: identity`: correct 4255 plain bytes, no encoding header. - Identity bodies measured through both paths are byte-identical (19984 → 4882 gzip, zstd for browser-style), so `manual` does **not** suppress edge compression for uncompressed origins — port shares and pre-#909 clients keep it. ## Compatibility Old client → new gate is safe: a pre-#909 client strips `accept-encoding` and drops `content-encoding` from resp-head, so its bodies are identity with no encoding header and `manual` asserts nothing false. The broken direction is **new client → old gate**, which is unchanged by this PR: prod's gate must ship this before any released client carries the reapplied #909. Merging this triggers `deploy-connect.yml`, which deploys the prod gate — the correct order. `HOST_DAEMON_PROTOCOL_VERSION` not bumped (stays 68): worker-only change, no daemon command / session payload / WS message altered, tunnel frame format unchanged (`PROTOCOL_VERSION` stays 1). ## Tests - `@bb/connect` 88 ✅ + typecheck - `@bb/tunnel-client` 6 ✅ + typecheck - `bb-plugin-connect` 65 ✅ - `@bb/app` 28 focused (the four suites from #909) ✅ + typecheck 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
amadad
pushed a commit
to amadad/bb
that referenced
this pull request
Aug 14, 2026
## The problem When you open a thread through bb connect, your browser has to talk to your laptop through a long pipe that runs out to Cloudflare and back. Every single question your browser asks down that pipe takes about **95 milliseconds** to get an answer, no matter how small the answer is. That's just how long the trip takes. So if you ask two questions one after the other, you wait ~190 ms. If you ask them at the same time, you wait ~95 ms. The number of questions matters way more than how big the answers are. Opening a thread used to ask more questions than it needed to, and it asked some of them in a line when they could have been asked all at once. ## What this changes **1. Stop waiting in line.** Loading a thread needs two things: the thread itself, and its timeline. Neither one needs the other. But the code asked for the thread, waited for the whole answer to come back, and only then asked for the timeline. Now both go out at the same time. **2. Stop asking twice.** Right after loading the thread, the app immediately asked for the same thread again. Now it skips the second ask if the first one just answered (within 5 seconds). **3. Stop asking for things nobody looked at.** There's a "Parent" dropdown in the thread sidebar. The app fetched the list of possible parent threads on every thread open — even though most people never open that dropdown. Now it only fetches when you actually click it. You'll see a brief "Loading threads…" the first time, and a retry option if it fails. **4. Let answers be zipped.** The pipe had a rule that said "don't send me compressed data." That rule made your laptop unzip everything before pushing it through the pipe, so we shipped big uncompressed answers when small zipped ones would do. That rule is gone. ## Did it actually work? Yes for get-bb#1, barely for get-bb#4. I tested this against the real production Connect — real Cloudflare, real tunnel, not a simulation. **Asking both questions at once (change get-bb#1):** | | how long | |---|---:| | Before (one after the other) | **255 ms** | | After (both at once) | **141 ms** | That's **114 ms faster, about 45%**. This is the change that matters. **Zipping the answers (change get-bb#4):** barely moves the needle, ~0–10 ms. The reason is boring: the answers are already small. Earlier work capped how much timeline gets sent at once, so even the biggest threads on my machine only produce 15–61 KB. Zipping shrinks that by 3–4×, which sounds great, but the pipe moves data fast enough (~18 MB/s) that 40 KB was never the problem. The 95 ms trip was. Zipping still earns its place in two spots, though: - On a slow connection (say hotel wifi at 10 Mbps), that same 40 KB *is* worth ~35 ms per request. - It turns the app's precompressed asset files back on. Those had been silently disabled over Connect this whole time by the "don't send me compressed data" rule, and they're much bigger than any API response. **Bottom line: don't expect to feel the zipping on fast internet. Do expect thread opens to feel snappier.** ## Things that will look different - **A new "Parent: None" row.** If you have a project with only one thread, the Parent row used to be hidden — the app knew there was nothing to pick. Now it can't know that without asking, and asking is the thing we're trying to avoid. So the row is always there for root threads. - **The Parent dropdown can be tabbed to** in a couple of cases where it previously couldn't. - **Re-opening a thread you've seen before** now asks "what's new since X?" instead of "give me everything," and falls back to asking for everything if the server can't answer that. ## The part to be careful about To let zipped data through the pipe, I rewrote the piece of code that forwards requests, swapping `fetch` for Node's lower-level HTTP client. This touches **every** thing that goes through the tunnel — including any dev server you've shared with `bb connect expose <port>`, not just BB itself. It's the riskiest part of this PR. Things that could have broken, and why they didn't: - **Garbled responses.** Raw headers now get forwarded as-is, which is dangerous if a "this data is chunked" header rides along with data that's already been un-chunked. The relay already throws those headers away (`apps/connect/src/tunnel-do.ts:44`), so it can't happen. - **Wrong length.** A "this is 500 bytes" header on a body that isn't 500 bytes would break things. Checked both places that set it — they always match what actually gets sent. - **Redirects.** The old code told `fetch` not to follow them. Node's client doesn't follow them anyway. Same behavior. - **Reused memory.** The new code hands out views into Node's internal buffer. Safe here, because it's copied out synchronously before Node can reuse it. No daemon protocol version bump needed — the wire format didn't change, no worker code changed, and the already-deployed relay handles these headers fine. **One thing tests can't cover:** whether Cloudflare's runtime passes a zipped response through untouched. I confirmed it by hand (step 2 below). Running it locally uses the same runtime but skips Cloudflare's CDN layer, so local testing isn't a full substitute. ## How to check it yourself Node 22, pnpm 9, clean checkout: ```bash pnpm install --offline --frozen-lockfile pnpm run ensure-native-modules pnpm exec turbo run test typecheck --filter=@bb/tunnel-client --filter=bb-plugin-connect --force pnpm exec turbo run test --filter=@bb/app --force -- src/hooks/cache-owners/cache-owner-registry.test.ts src/hooks/queries/thread-queries.test.tsx src/components/secondary-panel/ThreadMetadataContent.test.tsx src/views/thread-detail/ThreadDetailSecondaryContent.test.tsx pnpm exec turbo run typecheck --filter=@bb/app --force ``` Then open DevTools → Network and load a thread through bb connect: 1. The thread and timeline requests start together, and the thread isn't requested twice. 2. The timeline response has `content-encoding: gzip` and a `server-timing: bb_connect_origin` header. 3. The Connect host log prints timing (`originTtfbMs`, `originBodyMs`, `responseBytes`, `contentEncoding`). 4. Nothing requests parent candidates until you click **Parent**. First click shows "Loading threads…", then the list. 5. A root thread in a project with no other threads shows the **Parent: None** row. ## Tests All green. Tunnel client 6/6, connect plugin 65/65, four focused app suites 28/28, plus typechecks. The full GitHub matrix passes — app, server, integration, packages, checks, and macOS/Linux smoke. The new tests are the interesting ones. They check that the timeline request really does leave before the thread request comes back, that a warm cache asks for a delta and merges the answer correctly, that the duplicate thread request is skipped when fresh but still happens when stale, and that zipped bytes cross the tunnel byte-for-byte identical.
amadad
pushed a commit
to amadad/bb
that referenced
this pull request
Aug 14, 2026
This reverts commit dce86d9.
amadad
pushed a commit
to amadad/bb
that referenced
this pull request
Aug 14, 2026
…et-bb#915) Restores get-bb#909 (reverted in 17d652c) and fixes the response corruption that forced the revert. ## The corruption get-bb#909 lets the tunnel client forward the origin's compressed bytes verbatim. The gate then rebuilt those bytes into new `Response` objects under workerd's default `encodeBody: "automatic"`, which means *"this body is identity and I own the encoding"* — so workerd dropped `content-encoding` and shipped gzip bytes labelled `text/html`. Browsers rendered raw gzip. Two sites relayed pre-encoded bodies that way, and the correct fix is **asymmetric**: | Site | Body it wraps | Encoding | |---|---|---| | `tunnel-do.ts` resp-head | raw origin bytes from tunnel frames | `manual` — was corrupt | | `cache.ts` **hit** | Cache API stores bytes still compressed | `manual` — was corrupt | | `cache.ts` **miss** | workerd content-decodes a subrequest body as the gate reads it | **automatic** — already correct | Marking the miss path `manual` (the symmetric-looking fix) actively breaks it: it advertises gzip over plaintext and clients fail with `Z_DATA_ERROR`. The regression test catches that. ## Regression test `apps/connect/src/response-encoding.test.ts` drives the **real `TunnelDO` and real `serveWithCache`** inside workerd (miniflare), with a fake tunnel client relaying gzip frames over a real WebSocket. Node's `Response` ignores `encodeBody`, so a Node-only test would pass with or without this fix. Mutation-checked: reverting the `tunnel-do.ts` call site fails 4 tests, reverting the `cache.ts` hit path fails 1. Two control routes pin workerd's default behaviour so the tests document the bug rather than merely guarding it. ## Verification on real Cloudflare Deployed to staging (`bb-connect-staging`, version `0946eaab`, `sawyer.vibecodethis.site`). Because the tunnelled path needs a client + owner session, the end-to-end check ran against a throwaway workers.dev probe running this same `TunnelDO` + `cache.ts`, driven by the **real `TunnelSession` client** in front of the real built bb app (383 precompressed `.gz` assets, served as `apps/server` does). Probe deleted afterwards. - `index.html`: gzip-only client got 1509 raw bytes (`1f8b0800`) gunzipping to exactly 4255 bytes of real HTML; browser-style clients got it transcoded to zstd by the edge — which the edge can only do if it correctly understands the body as encoded. - 342 KB app bundle, miss → hit → hit: `x-bb-cache: miss`/`hit` plus `cf-cache-status: HIT`, every response gunzipping to sha256 `18cb8f3a…`, byte-identical to the origin file. - Legacy control at the edge reproduces the production symptom: `content-encoding` dropped, edge zstd-wraps the raw gzip, browser renders `^_M-^K^H…` as `text/html`. - `Accept-Encoding: identity`: correct 4255 plain bytes, no encoding header. - Identity bodies measured through both paths are byte-identical (19984 → 4882 gzip, zstd for browser-style), so `manual` does **not** suppress edge compression for uncompressed origins — port shares and pre-get-bb#909 clients keep it. ## Compatibility Old client → new gate is safe: a pre-get-bb#909 client strips `accept-encoding` and drops `content-encoding` from resp-head, so its bodies are identity with no encoding header and `manual` asserts nothing false. The broken direction is **new client → old gate**, which is unchanged by this PR: prod's gate must ship this before any released client carries the reapplied get-bb#909. Merging this triggers `deploy-connect.yml`, which deploys the prod gate — the correct order. `HOST_DAEMON_PROTOCOL_VERSION` not bumped (stays 68): worker-only change, no daemon command / session payload / WS message altered, tunnel frame format unchanged (`PROTOCOL_VERSION` stays 1). ## Tests - `@bb/connect` 88 ✅ + typecheck - `@bb/tunnel-client` 6 ✅ + typecheck - `bb-plugin-connect` 65 ✅ - `@bb/app` 28 focused (the four suites from get-bb#909) ✅ + typecheck 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
When you open a thread through bb connect, your browser has to talk to your laptop through a long pipe that runs out to Cloudflare and back. Every single question your browser asks down that pipe takes about 95 milliseconds to get an answer, no matter how small the answer is. That's just how long the trip takes.
So if you ask two questions one after the other, you wait ~190 ms. If you ask them at the same time, you wait ~95 ms. The number of questions matters way more than how big the answers are.
Opening a thread used to ask more questions than it needed to, and it asked some of them in a line when they could have been asked all at once.
What this changes
1. Stop waiting in line. Loading a thread needs two things: the thread itself, and its timeline. Neither one needs the other. But the code asked for the thread, waited for the whole answer to come back, and only then asked for the timeline. Now both go out at the same time.
2. Stop asking twice. Right after loading the thread, the app immediately asked for the same thread again. Now it skips the second ask if the first one just answered (within 5 seconds).
3. Stop asking for things nobody looked at. There's a "Parent" dropdown in the thread sidebar. The app fetched the list of possible parent threads on every thread open — even though most people never open that dropdown. Now it only fetches when you actually click it. You'll see a brief "Loading threads…" the first time, and a retry option if it fails.
4. Let answers be zipped. The pipe had a rule that said "don't send me compressed data." That rule made your laptop unzip everything before pushing it through the pipe, so we shipped big uncompressed answers when small zipped ones would do. That rule is gone.
Did it actually work?
Yes for #1, barely for #4. I tested this against the real production Connect — real Cloudflare, real tunnel, not a simulation.
Asking both questions at once (change #1):
That's 114 ms faster, about 45%. This is the change that matters.
Zipping the answers (change #4): barely moves the needle, ~0–10 ms.
The reason is boring: the answers are already small. Earlier work capped how much timeline gets sent at once, so even the biggest threads on my machine only produce 15–61 KB. Zipping shrinks that by 3–4×, which sounds great, but the pipe moves data fast enough (~18 MB/s) that 40 KB was never the problem. The 95 ms trip was.
Zipping still earns its place in two spots, though:
Bottom line: don't expect to feel the zipping on fast internet. Do expect thread opens to feel snappier.
Things that will look different
The part to be careful about
To let zipped data through the pipe, I rewrote the piece of code that forwards requests, swapping
fetchfor Node's lower-level HTTP client. This touches every thing that goes through the tunnel — including any dev server you've shared withbb connect expose <port>, not just BB itself. It's the riskiest part of this PR.Things that could have broken, and why they didn't:
apps/connect/src/tunnel-do.ts:44), so it can't happen.fetchnot to follow them. Node's client doesn't follow them anyway. Same behavior.No daemon protocol version bump needed — the wire format didn't change, no worker code changed, and the already-deployed relay handles these headers fine.
One thing tests can't cover: whether Cloudflare's runtime passes a zipped response through untouched. I confirmed it by hand (step 2 below). Running it locally uses the same runtime but skips Cloudflare's CDN layer, so local testing isn't a full substitute.
How to check it yourself
Node 22, pnpm 9, clean checkout:
Then open DevTools → Network and load a thread through bb connect:
content-encoding: gzipand aserver-timing: bb_connect_originheader.originTtfbMs,originBodyMs,responseBytes,contentEncoding).Tests
All green. Tunnel client 6/6, connect plugin 65/65, four focused app suites 28/28, plus typechecks. The full GitHub matrix passes — app, server, integration, packages, checks, and macOS/Linux smoke.
The new tests are the interesting ones. They check that the timeline request really does leave before the thread request comes back, that a warm cache asks for a delta and merges the answer correctly, that the duplicate thread request is skipped when fresh but still happens when stale, and that zipped bytes cross the tunnel byte-for-byte identical.