Add browser support for per-stream flow control - #4

Thanks @Brumbelow -- and thanks for keeping this in lockstep with the CLI side. Merging
bitbang-cli#15 and finding the matching browser commit already pushed made that review a
lot easier, and the constants line up exactly (1 MiB window, 256 frames)

Two questions before I merge, both about coverage rather than the code.

What did the Playwright run exercise?

The CLI e2e suite defaults to BITBANG_TEST_SERVER=test.bitba.ng, so run unmodified the
browser would fetch the deployed bootstrap.js -- currently v3 -- and the session would
negotiate down. That result is worth having, since it proves the downgrade path works from a
v4 connector, but it wouldn't touch a line of this PR.

Did you serve this branch's assets and point the suite at them? "Coordinated" reads like you
did, I'd just like it stated in the PR so the next person doesn't have to guess. If you did,
please say how you did it-- a local server plus the env var, or something else -- because that recipe
is genuinely useful and nothing in either repo documents it today.

If you didn't, no biggee: it's the one gap I'd want closed before merge, since bootstrap.js
is where most of the change lives.

bootstrap.js is the biggest change and the least covered

flow-control.js and sw-upload.js both got real unit tests, which is great--
extracting them made them testable. But bootstrap.js is +565/-151, holds the send-credit
and receive-accounting integration, and its only listed check is node --check, which
confirms it parses.

I'm not asking you to retrofit a test harness onto bootstrap.js. But if the Playwright run
did cover it end to end, that closes the gap and the answer above is all I need.

No CI in this repo

bitbang-server has no .github/workflows at all, so node --test web_test/ runs only when
someone remembers. That was tolerable when the browser code had no tests; now that it has
some, and now that the two implementations of v4 have to agree on window size, update
threshold, credit-exempt frames, and queue bounds, a silent drift between them is the most
likely future failure and nothing would catch it.

Would you be willing to add a small workflow here running node --test web_test/ and
node --check on the browser scripts? If you'd rather keep this one focused I'll open it as a follow-up and do
it myself -- your call, and it won't hold up the merge either way.

No — that run was unmodified, so it proved the downgrade path from a v4 connector and none of this branch's browser code ran.

Now closed: the same suite against this branch served locally passes 10/10, with the browser reporting negotiatedVersion: 4 (checked via window.__bitbangConnection) — bootstrap.js end to end on the paths the suite drives. Recipe is in the description under Testing.

Added the workflow here rather than leaving it to a follow-up: node --check on web/*.js and node --test on web_test/, on push and PR.

Pushed — node --check over web/*.js plus node --test web_test/*.test.js, on PRs and pushes to main. The glob instead of the bare directory is deliberate: node --test web_test/ errors on node 22 trying to resolve the directory as a module. Smoke-ran it on my fork first: 6 files checked, 18/18 tests green.

Hi Brumbelow, my apologies for the wait on this. I'd been focusing on the CLI repo and more or less forgot about this repo, even though it's arguably more important! Everything I asked for is here:
the local Playwright run with negotiatedVersion: 4, and the workflow I'd offered to
do myself. The note about node --test web_test/ failing on node 22 and
using an explicit glob instead is the kind of detail that saves the next
person an afternoon.

One thing blocks, one is a follow-up.

Uploads get a wall-clock deadline they deliberately did not have

sw.js today exempts requests with a body from the 30-second timeout, and
the guard is explicit:

if (!hasBody) {
    timeout = setTimeout(() => { ... }, 30000);
}

An upload takes as long as it takes. The new AckGate reintroduces the
deadline on exactly that path:

const uploadAcks = new SWSPUpload.AckGate(30000);
...
const ack = uploadAcks.wait(seq);
channel.port1.postMessage({ type: 'bodyChunk', data: chunk, seq }, [chunk.buffer]);
await ack;

The ack is not local. bootstrap.js sends bodyAck only after the slice has
cleared stream credit and the aggregate data-channel buffer -- so it waits
on the remote end actually consuming. With MAX_SLICE_BYTES at 1 MiB, that
means every slice needs roughly 280 kbit/s sustained or the upload fails
with upload backpressure timeout. A relayed connection, a phone on a bad
network, or the campus-style link where TURN is in play can sit under that,
and a 10 MB upload needs ten consecutive slices to clear it.

On main that upload completes, slowly. That is what makes this a
regression rather than a limitation, and it is the only reason I am holding
the merge.

The fix I would suggest is not a bigger number -- it is that the timer
should detect a stall, not slowness. Reset it whenever any credit is
granted for the stream, rather than giving each 1 MiB slice its own
deadline. Then a slow-but-moving upload runs as long as it needs and a
genuinely dead peer still fails in 30 seconds, which is what the timeout is
for. If that is more surgery than you want here, dropping the deadline
entirely is also fine by me: the data channel closing already surfaces a
dead peer, which is how main gets away with having no upload timeout at
all.

The workflow could take the Go tests too

Not a blocker, and adding CI at all was already more than I asked for. But
this repo's Go tests -- handler, identity, metrics, pairing, releases -- still
only run when someone remembers, and the moment the file is being created is
the cheap moment to add the job. Happy to do it as a follow-up if you'd
rather keep this focused; I'd just rather it not wait another six weeks
because I forgot again.

Checked and clean

For the record, since some of this was the thing I was worried about:

• Constants match the CLI exactly -- 1 MiB window, half-window update
  threshold, 256 frames, 32 KB payload, version 4. That was my stated drift
  risk and there is no drift.
• receive() sets receiveEnded before validating, which looked like it
  could strand a stream. It cannot: every call site resets the stream on a
  false return, so the state is discarded either way.
• The event.source !== iframe.contentWindow guard survives, so the ack
  tokens are not reachable from another frame.
• static.go adds both new assets to the whitelist; missing one would have
  been a silent 404 at page load.
• Go builds and every Go test passes on the branch; node --test is 18/18
  on node 22.

One deployment note, for me rather than you

bootstrap.js is served to every browser by the signaling server, so unlike
the CLI there is no gradual rollout -- merging and deploying puts this in
front of every session at once. That is an argument for test.bitba.ng
first, and it is my problem, not a change request.

Pushed.

Dropped the deadline rather than moving it. The CLI's reserveSend has none either, and with credit arriving per half-window a stall timer keyed on it would still fail anything under ~140 kbit/s. bootstrap.js's own 30 s request timer had the same per-slice reset, so it now runs only once the whole request is out: for a GET as before, and for a POST with nothing in it. The 30 s reap after an early response now waits for the body as well, and an answer that arrives before the upload is done is delivered rather than lost to the stream reset that follows it (the listener drops the body pipe once it has responded; that 'read/write on closed pipe' reset is a CLI-side race I'll follow up on). A dead peer still surfaces through the channel closing.

Go tests: ci.yml from #2 already runs go vet and go test ./... on every PR and push, so I left tests.yml to the web side.

Rebased onto main while in there: the bench stream opens and consumes credit like the others, the Firefox buffered body goes through the same slice/ack loop, and the two new scripts are in stampInputs. Checked with the CLI suite against this branch's assets plus a 3 MiB upload into a 16 KiB/s sink, credit-bound from the second slice on: it died at the 30 s mark before the change and completes after it.