refactor(wasm)!: implement the poll traits natively by kixelated · Pull Request #369 · moq-dev/web-transport

kixelated · 2026-08-11T21:04:30Z

Why

moq#2736 made moq-net poll-only. Every backend that had no native poll implementation got a transitional adapter, and web-transport-wasm got one inside moq-wasm's own transport::Session newtype: a boxed local future per operation, wrapping the async methods this crate already had. Its own follow-up list says that newtype should shrink to nothing once this crate grows a real poll implementation.

This is that implementation. Not an adapter over our async methods -- the state machine underneath them.

What

Every operation is now a poll_* method, with the async methods as thin poll_fn wrappers, and the crate implements both halves of web_transport_trait (poll::{Session, SendStream, RecvStream} and the async ones) plus Error. A compile-time conformance test in traits.rs pins the signatures so drift fails here rather than downstream.

The core is Op in the new js.rs: a JsFuture retained across polls. A JsFuture subscribes to its promise when it is built and owns the slot the result lands in, so dropping one between polls throws that result away -- for a read() that is an accepted stream or a chunk of stream data, gone with no way to ask for it again. Retaining it means no boxed async blocks and, more importantly, nothing that has to own a stream.

That last part is what the adapter could not do. Its closed() future moved the stream in, which deadlocked any later read or write, so both stream closed-watches had to be emulated from reads (moq#2736 found this via goaway_gates_new_subscribes_moq_lite_04). Nothing here owns the stream: poll_closed polls the browser's own closed promise. That is the whole contract with no emulation -- including a peer STOP_SENDING arriving before finish(), which the emulation structurally could not see.

Bugs the conversion caught

stop and reset never transmitted a code. The browser derives STOP_SENDING and RESET_STREAM from a WebTransportError reason and reports anything else as 0, so the string reason we passed -- and web-transport's &code.to_string() feeding it -- always meant 0. Both now take a u32 and build the error the browser actually reads the code from.
web-sys's WebTransportError constructor throws. It binds new WebTransportError(message, options); browsers take a single init dictionary and reject that form with a TypeError. So the error was never built, the thrown TypeError went to cancel/abort as the reason, and a reason that is not a WebTransportError carries no code -- the peer saw 0. Now built through Reflect. This also broke session close reporting, which synthesized an error the same way: closed() yielded Unknown(TypeError) and session_error() answered None for every close. A synthesized error cannot carry a session source either (the dictionary's source member is ignored), so Error::Session now carries { code, reason } itself.
Code widths. A stream code is a clamped byte, as web-sys had it; a session close code is a genuine u32 off WebTransportCloseInfo. Codes stay u32 through the API because that is what the wire and the trait carry, and the clamp is the browser's to apply. This removes the reason web-transport truncated its own closed codes to u8 -- the comments there cite the WASM signature as the cause -- so both platforms now report the full u32.
Cloning a session could not accept. Each call built its own Reader, and a browser stream can only be locked once, so a second clone threw. The locks now live in an Rc shared by every clone while each clone issues its own read(); the streams spec queues concurrent reads and fulfills them in order, so one clone's pending read is never handed to another. That is what makes cloning the way to run operations concurrently, as the poll contract assumes.

Breaking changes

SendStream::reset and RecvStream::stop take a u32 code instead of a &str reason (see above).
web-streams is dropped -- its Reader and Writer are async-only, so they cannot be polled -- and Error::Streams with it.
Error::Closed is new: a write to a stream already finished or reset.
SendStream::closed and RecvStream::closed take &mut self, and return Option<u32> rather than Option<u8> -- as does Error::code and both platforms of web-transport.
poll_write reports bytes accepted, taking its backpressure from waiting on the previous chunk, which is what keeps a Pending from ever consuming the caller's buffer. A write's own failure now surfaces on the next write or on closed(), the same as quinn -- rather than from the write().await that started it.

web-transport's wasm router is updated for the two signature changes; its public API is otherwise unchanged.

Smaller things

max_datagram_size comes from the browser instead of moq's conservative 1200.
Error's Display prints the DOMException message -- the reason the peer sent -- instead of a JS handle dump, and that message is what session_error() returns.
poll_recv_datagram reports the session error when the datagram stream ends, instead of an empty Bytes.
try_send_datagram sheds rather than queues when a write is still outstanding or the browser reports no room, so a stalled connection cannot grow an unbounded backlog that later goes out stale. The poll path still waits; draining the previous write is what bounds it.

Review fixes (`673a708`)

Four defects came out of review, all cases where a browser primitive does not behave the way the first cut assumed:

A dropped session clone swallowed a stream. A browser read() cannot be cancelled -- once issued, the browser hands the next value to that request -- so dropping the handle that made it stranded whatever arrived. Reads orphaned by a dropped handle now go to a shared queue the next poll adopts, oldest first.
RecvStream::poll_closed could hang. It deferred while bytes sat buffered, on the theory the caller would drain them and wake it -- but it borrows &mut self, so a caller waiting there is a caller who cannot read. stop() then closed() wedged forever, as did a peer reset behind buffered bytes. The deferral is gone; buffered bytes are still delivered by later reads, matching quinn.
try_send_datagram queued without bound, and codes above 255 were corrupted -- both described above.

Verification

cargo check and cargo clippy --all-targets -- -D warnings for web-transport-wasm and web-transport, on wasm32-unknown-unknown and on the host: green. cargo fmt, cargo sort --check, cargo shear: clean. CI green on 673a708.

Measured against a real browser (Chromium 148) driving the echo-server example over QUIC, watching the code that reached the peer:

reason passed to `abort()`	on the wire
`WebTransportError{streamErrorCode: 7}`	`RESET_STREAM: 7`
`WebTransportError{streamErrorCode: 200}`	`RESET_STREAM: 200`
`WebTransportError{streamErrorCode: 42069}`	`RESET_STREAM: 255` (browser clamp)
`"42069"` -- the reason before this branch	`RESET_STREAM: 0`
the `TypeError` -- this branch before `e147246`	`RESET_STREAM: 0`

The last two rows are the control: the code was silently dropped both before this branch and partway through it.

A browser harness now covers the poll state machine (just harness, added in 258056a). Seven checks against a real browser and a real QUIC peer -- 7 passed, 0 failed in Chromium 148:

check
echo round trip	pass
`accept_uni` delivers every stream	pass
a dropped clone does not swallow a stream	pass
`closed()` resolves with bytes still buffered	pass
peer reset code reaches the receiver	pass
datagram round trip	pass
session close code and reason survive	pass

The two regression checks were confirmed to fail without their fix, not merely pass with it: reverting the orphan handoff fails the dropped-clone check with accept_uni never resolved, and restoring the buffered deferral fails the closed-watch check on its timeout. The other five stay green through both, so neither is passing by accident.

Still untested: Firefox and Safari, and backpressure under real flow-control pressure. The harness runs by hand, not in CI -- it needs a browser and a live QUIC peer.

Note on the branch

The branch carries a pre-existing Bump -proto commit that was already on it and is not on main. It is unrelated to this change.

Follow-ups

Once this lands, moq-wasm's transport.rs can drop its adapter and become a plain newtype.

(written by Opus 5)

The browser API is a set of promises, so moq-wasm wrapped it in a `transport::Session` newtype that boxed a local future per operation to fake the poll interface moq-net requires. Build that surface here instead, as a state machine rather than an adapter over our own async methods. The core is `Op`: a `JsFuture` retained across polls. A `JsFuture` subscribes to its promise when it is built and owns the slot the result lands in, so dropping one between polls throws that result away -- for a `read()` that is an accepted stream or a chunk of stream data, gone with no way to ask for it again. Retaining it means no boxed async blocks and nothing that has to own a stream. That last part is what the adapter could not do. Its `closed()` future moved the stream in, which deadlocked any later read or write, so both stream closed-watches had to be emulated from reads. Nothing here owns the stream: `poll_closed` polls the browser's own `closed` promise, which is the whole contract -- including a peer STOP_SENDING arriving before `finish()`, which the emulation could not see. Two bugs fell out of the conversion: - `stop` and `reset` never transmitted a code. The browser derives STOP_SENDING and RESET_STREAM from a `WebTransportError` reason and reports anything else as 0, so the string reason we passed -- and the router's `&code.to_string()` feeding it -- always meant 0. Both now take a `u32` and build the error the browser reads it from. - Cloning a session could not accept. Each call built its own reader and a browser stream can only be locked once, so the second clone threw. The locks now live in an `Rc` while each clone issues its own `read()`; the streams spec queues concurrent reads and fulfills them in order, which is what makes cloning the way to run operations concurrently. Breaking, beyond the two signatures above: `web-streams` is gone, since its `Reader` and `Writer` are async-only and cannot be polled, and `Error::Streams` with it. `Error::Closed` is new, reported by a write to a stream that was finished or reset. `SendStream::closed` and `RecvStream::closed` take `&mut self`. `poll_write` reports bytes accepted rather than delivered, taking its backpressure from waiting on the *previous* chunk -- which is what keeps a `Pending` from ever consuming the caller's buffer. A write's own failure now surfaces on the next write or on `closed()`, as it does for quinn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

chatgpt-codex-connector · 2026-08-11T21:04:36Z

You have reached your Codex usage limits for security reviews. Please try again later.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3cde4a4565

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

coderabbitai · 2026-08-11T21:11:28Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b674536d-cda1-456e-b4aa-a14718d53064

📥 Commits

Reviewing files that changed from the base of the PR and between 2c91a10 and 8db02ab.

📒 Files selected for processing (3)

justfile
rs/web-transport-wasm/examples/harness.rs
rs/web-transport-wasm/src/send.rs

🚧 Files skipped from review as they are similar to previous changes (3)

rs/web-transport-wasm/examples/harness.rs
justfile
rs/web-transport-wasm/src/send.rs

Walkthrough

The WASM crate now uses native browser readable and writable streams with retained poll-based operations. Session clones share browser stream locks and datagram state while maintaining independent operation state. RecvStream and SendStream expose polling APIs and numeric stream-control codes. Async methods delegate to polling methods. Trait adapters provide asynchronous and poll-based interfaces. Error handling, crate wiring, dependencies, documentation, and the protocol crate version were updated.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description clearly explains the native polling implementation and related WebTransport WASM changes.
Title check	✅ Passed	The title clearly identifies the main change: native implementation of polling traits in the WASM backend.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

✨ Simplify code

Create PR with simplified code
Commit simplified code in branch claude/web-transport-wasm-poll-5c0060

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rs/web-transport-proto/Cargo.toml`:
- Line 8: Revert the manual version change in the package manifest by restoring
the existing version value for this crate. Leave release-plz to manage future
Rust crate version bumps and changelog updates.

In `@rs/web-transport-wasm/src/recv.rs`:
- Around line 145-148: Update RecvStream::poll_closed_raw so closure polling
does not return Poll::Pending merely because buffer contains unread bytes; poll
reader.closed() independently of buffer and preserve the underlying close
result.

In `@rs/web-transport-wasm/src/send.rs`:
- Around line 74-77: Preserve the full u32 range for WebTransport stream error
codes by replacing the web-sys Option<u8> getter/setter usage with suitable
bindings or JavaScript interop, then update the affected WASM and wrapper APIs
to use u32. Apply this consistently at rs/web-transport-wasm/src/send.rs:74-77
and 114-116, and rs/web-transport-wasm/src/recv.rs:125-128 and 180-182, ensuring
codes above 255 are neither converted to None nor sent as zero.

In `@rs/web-transport-wasm/src/traits.rs`:
- Around line 229-231: Update poll_closed_raw, used by the poll_closed trait
adapter, so a non-empty self.buffer cannot cause an unregistered Pending result.
Poll self.closed even while buffered data remains, or ensure draining the buffer
produces a wakeable transition, and preserve peer reset propagation through both
trait adapters.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ec906829-89ad-45c4-8812-a3023956dddd

📥 Commits

Reviewing files that changed from the base of the PR and between 79b9e52 and 3cde4a4.

📒 Files selected for processing (11)

rs/web-transport-proto/Cargo.toml
rs/web-transport-wasm/Cargo.toml
rs/web-transport-wasm/README.md
rs/web-transport-wasm/src/error.rs
rs/web-transport-wasm/src/js.rs
rs/web-transport-wasm/src/lib.rs
rs/web-transport-wasm/src/recv.rs
rs/web-transport-wasm/src/send.rs
rs/web-transport-wasm/src/session.rs
rs/web-transport-wasm/src/traits.rs
rs/web-transport/src/wasm.rs

Four defects found in review of the poll conversion, all of them cases where a browser primitive does not behave the way the first cut assumed. A dropped session clone swallowed a stream. A browser `read()` cannot be cancelled: once issued, the browser hands the next value to *that* request. Dropping the handle that made it stranded whatever arrived, so a surviving clone never learned the stream existed. Reads orphaned by a dropped handle now go to a shared queue that the next poll adopts before issuing a fresh read, oldest first, which also keeps the order the browser fulfills them in. `RecvStream::poll_closed` could hang. It deferred while bytes sat in our buffer, on the theory that the caller would drain them and wake us. But it borrows `&mut self`, so a caller waiting there is a caller who cannot read, and `stop()` followed by `closed()` wedged forever -- as did a peer reset arriving behind buffered bytes. The transport's closure is now reported when it happens; the buffered bytes are still delivered by later reads, which is also how quinn behaves. `try_send_datagram` queued without bound. It wrote on every call and replaced the previous unresolved write, and a browser write cannot be cancelled, so a media loop on a stalled connection would pile up a backlog and eventually send it stale. It now sheds when a write is still outstanding or `desiredSize` reports no room, and returns whether the browser took it. The poll path keeps waiting instead of shedding; draining the previous write is what bounds it. Stream and session error codes above 255 were corrupted. `web-sys` types `streamErrorCode` as a byte, from a WebIDL draft that has since widened it to `unsigned long`: its setter sent a larger code as 0 and its getter truncated one into a different code's meaning. Both directions now go around the binding via `Reflect`, and the codes stay `u32` out through `Error::code` and `closed`. That last change removes the reason `web-transport` truncated its own `closed` codes to `u8` -- the comments there cite the WASM signature as the cause -- so both platforms now report the full `u32`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

chatgpt-codex-connector · 2026-08-11T21:32:06Z

You have reached your Codex usage limits for security reviews. Please try again later.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 673a708e7d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Verified against Chromium 148 with the echo-server example, watching what reached the peer on the wire. `web-sys` binds the constructor as `new WebTransportError(message, options)`. Browsers take a single init dictionary and reject that form with a TypeError, so every call threw and we handed the thrown TypeError to `cancel`/`abort` as the reason. A reason that is not a `WebTransportError` carries no code, so the peer saw 0. Building the dictionary and constructing through `Reflect` fixes it. Measured, aborting a stream with each reason: WebTransportError{streamErrorCode: 7} -> RESET_STREAM: 7 WebTransportError{streamErrorCode: 200} -> RESET_STREAM: 200 WebTransportError{streamErrorCode: 42069} -> RESET_STREAM: 255 "42069" (the reason before this branch) -> RESET_STREAM: 0 the TypeError (this branch, before now) -> RESET_STREAM: 0 The same throw broke session close reporting, which used the same constructor to synthesize an error from the close info: `closed()` yielded `Unknown(TypeError)`, so `session_error()` answered `None` for every close and no caller could read a close code. It cannot be fixed by constructing either, because the `source` member of the dictionary is ignored -- a synthesized error is always a stream error, never a session one. So `Error::Session` now carries `{ code, reason }` itself and no JS object is built for it at all. That also settles the width question. A stream code really is a clamped byte, as `web-sys` had it and contrary to the review that prompted the widening: 255 stays 255, 256 and 42069 arrive as 255, -1 as 0. A session close code is a genuine u32, straight off `WebTransportCloseInfo`. Codes stay u32 through the API because that is what the wire and the trait carry, and the clamp is the browser's to apply. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rs/web-transport-wasm/src/error.rs`:
- Around line 47-50: Update the WebTransportErrorSource::Session branch in the
relevant error conversion function to return Error::Unknown(v) or another
distinct session-failure variant instead of Error::Session with code 0. Reserve
Error::Session exclusively for session_error(info) close handling, and preserve
the existing error message/details where applicable.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6fa8bc93-129c-4356-b5c6-2c30895636f9

📥 Commits

Reviewing files that changed from the base of the PR and between 673a708 and e147246.

📒 Files selected for processing (2)

rs/web-transport-wasm/src/error.rs
rs/web-transport-wasm/src/traits.rs

🚧 Files skipped from review as they are similar to previous changes (1)

rs/web-transport-wasm/src/traits.rs

CI only compiles this crate. The browser API it wraps exists nowhere else, so every poll path went to review with no automated coverage -- which is how a dropped clone swallowing a stream, and a closed-watch deadlocking behind buffered bytes, both got as far as they did. `just harness` builds the wasm client, starts the QUIC peer that drives it, and serves the page. Seven checks, a row each rather than a panic, so one failure does not hide the rest: echo round trip accept_uni delivers every stream a dropped clone does not swallow a stream closed() resolves with bytes still buffered peer reset code reaches the receiver datagram round trip session close code and reason survive The peer is a new example rather than the existing echo servers, which only ever answer: nothing in them opens a stream toward the client, resets one, or closes the session, so accept and the reset and close paths could not be reached at all. This one takes a command per stream, so the harness sets up each scenario deterministically instead of by timing. Two checks are regressions, and both were confirmed to fail without their fix rather than merely pass with it. Reverting the orphan handoff fails "a dropped clone does not swallow a stream" with `accept_uni never resolved`; restoring the buffered deferral fails "closed() resolves with bytes still buffered" on its timeout. The other five stayed green through both, so neither check is passing by accident. The orphan check polls a clone by hand with a noop waker instead of awaiting it, so its browser read is definitely outstanding before the peer opens anything -- awaiting would have resolved it and tested nothing. Verified in Chromium 148: 7 passed, 0 failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…fixes # Conflicts: # rs/web-transport-wasm/src/session.rs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3612f2b72

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…rness` The recipe assumed whichever wasm-bindgen was on PATH matched what we build with. The nix shell pins its own wasm-bindgen-cli -- currently 0.2.114 against the 0.2.126 in the lockfile -- and that is the shell the repository tells you to use, so the first thing `just harness` did there was fail. The failure is a schema error naming two versions without saying where either came from. Compare the two up front and say which to install instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1616f24b22

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kixelated · 2026-08-12T05:16:26Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: acbf53a548

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kixelated · 2026-08-12T05:47:54Z

@codex review

coderabbitai

Actionable comments posted: 3

🧹 Nitpick comments (3)

rs/web-transport-wasm/examples/harness.rs (1)

354-377: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

elapsed is unused.

The Rc<Cell<bool>> is created at line 355, cloned at line 358, and set at line 366, but no code reads it. select already reports which branch finished. Remove the cell.

♻️ Proposed cleanup

 async fn timeout<T>(future: impl std::future::Future<Output = T>, millis: i32) -> Result<T, ()> {
-    let elapsed = Rc::new(std::cell::Cell::new(false));
-
-    let sleep = {
-        let elapsed = elapsed.clone();
-        async move {
-            let promise = js_sys::Promise::new(&mut |resolve, _| {
-                let window = web_sys::window().expect("no window");
-                let _ =
-                    window.set_timeout_with_callback_and_timeout_and_arguments_0(&resolve, millis);
-            });
-            let _ = wasm_bindgen_futures::JsFuture::from(promise).await;
-            elapsed.set(true);
-        }
-    };
+    let sleep = async move {
+        let promise = js_sys::Promise::new(&mut |resolve, _| {
+            let window = web_sys::window().expect("no window");
+            let _ = window.set_timeout_with_callback_and_timeout_and_arguments_0(&resolve, millis);
+        });
+        let _ = wasm_bindgen_futures::JsFuture::from(promise).await;
+    };

Remove the now-unused rc::Rc import at line 18.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rs/web-transport-wasm/examples/harness.rs` around lines 354 - 377, Remove the
unused elapsed Rc<Cell<bool>> state from timeout, including its clone and
elapsed.set call in the sleep future, while preserving the existing
futures::future::select branch handling. Also remove the now-unused Rc import.

rs/web-transport-quinn/examples/harness-server.rs (1)

166-177: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The comment does not match the lifetime of streams.

streams is dropped when open_streams returns, not while the harness reads the earlier streams. The retention therefore lasts only for the loop body. If the intent is to keep the streams alive until the client has read them, the caller must hold them. If quinn already flushes finished streams after drop, correct the comment instead.
📝 Proposed comment fix
-    // Held until every stream is open, so none is dropped while the harness is
-    // still working through the earlier ones.
+    // Held until every stream is open, so an earlier stream is not dropped while
+    // later ones are still being opened.
     let mut streams = Vec::with_capacity(count);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rs/web-transport-quinn/examples/harness-server.rs` around lines 166 - 177,
Correct the lifetime claim in the comment above streams within open_streams: the
local Vec is dropped when the function returns, so it does not retain streams
while the harness reads them. Either move stream ownership to the caller if
retention is required, or revise the comment to describe only the loop-body
lifetime based on Quinn’s finished-stream behavior.

justfile (1)

105-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

The install hint is unreachable when wasm-bindgen is not installed.

Line 107 runs wasm-bindgen --version before the guarded call. With set -euo pipefail, a missing CLI aborts the recipe there, so the message at lines 114-117 never prints. Detect the missing CLI first.

🛠️ Proposed fix

 	want=$(cargo metadata --format-version 1 \
 		| python3 -c 'import json,sys; print(next(p["version"] for p in json.load(sys.stdin)["packages"] if p["name"]=="wasm-bindgen"))')
-	got=$(wasm-bindgen --version | awk '{print $2}')
+	if ! command -v wasm-bindgen >/dev/null; then
+		echo "wasm-bindgen CLI not found; the build uses $want." >&2
+		echo "    cargo binstall -y --force wasm-bindgen-cli@$want" >&2
+		exit 1
+	fi
+	got=$(wasm-bindgen --version | awk '{print $2}')

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@justfile` around lines 105 - 118, Update the harness recipe’s
version-detection flow around the `got` assignment and guarded `wasm-bindgen`
invocation so a missing CLI is handled before running `wasm-bindgen --version`.
Preserve the existing mismatch message and installation hint, ensuring they
print for both an absent CLI and a CLI whose version differs from `want`.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@justfile`:
- Around line 100-120: Update the setup guard before the harness build so it
runs ./dev/setup unless both dev/localhost.crt and dev/localhost.hex exist.
Preserve the existing setup command and subsequent copy behavior.

In `@rs/web-transport-wasm/examples/harness.rs`:
- Around line 338-348: Update decode_hex to process the trimmed input as bytes
rather than slicing the &str by byte indices, while retaining the even-length
validation and returning None for any invalid byte pair. Ensure multibyte or
non-ASCII input cannot panic.
- Line 340: Update the WASM crate configuration to declare Rust 1.87 as its
rust-version, covering the use of Waker::noop() and usize::is_multiple_of().

---

Nitpick comments:
In `@justfile`:
- Around line 105-118: Update the harness recipe’s version-detection flow around
the `got` assignment and guarded `wasm-bindgen` invocation so a missing CLI is
handled before running `wasm-bindgen --version`. Preserve the existing mismatch
message and installation hint, ensuring they print for both an absent CLI and a
CLI whose version differs from `want`.

In `@rs/web-transport-quinn/examples/harness-server.rs`:
- Around line 166-177: Correct the lifetime claim in the comment above streams
within open_streams: the local Vec is dropped when the function returns, so it
does not retain streams while the harness reads them. Either move stream
ownership to the caller if retention is required, or revise the comment to
describe only the loop-body lifetime based on Quinn’s finished-stream behavior.

In `@rs/web-transport-wasm/examples/harness.rs`:
- Around line 354-377: Remove the unused elapsed Rc<Cell<bool>> state from
timeout, including its clone and elapsed.set call in the sleep future, while
preserving the existing futures::future::select branch handling. Also remove the
now-unused Rc import.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a114e662-1421-4473-8c25-760ebadf3f6b

📥 Commits

Reviewing files that changed from the base of the PR and between e147246 and 2c91a10.

📒 Files selected for processing (9)

justfile
rs/web-transport-quinn/examples/harness-server.rs
rs/web-transport-wasm/Cargo.toml
rs/web-transport-wasm/README.md
rs/web-transport-wasm/examples/harness.html
rs/web-transport-wasm/examples/harness.rs
rs/web-transport-wasm/src/error.rs
rs/web-transport-wasm/src/js.rs
rs/web-transport-wasm/src/session.rs

🚧 Files skipped from review as they are similar to previous changes (3)

rs/web-transport-wasm/src/js.rs
rs/web-transport-wasm/src/error.rs
rs/web-transport-wasm/src/session.rs

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c91a103b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kixelated · 2026-08-12T06:20:01Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8db02abc5b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kixelated · 2026-08-12T06:41:38Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0150ab10b3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

kixelated · 2026-08-12T07:14:27Z

@codex review

chatgpt-codex-connector · 2026-08-12T07:19:58Z

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 32c9e899c9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

A review flagged `poll_send_datagram` for deadlocking the loser of a datagram capacity race: it would hold a `ready` promise that has already fulfilled, see the capacity gone, and fall through to `writer.closed()`, which never resolves on a healthy session. The stream semantics behind that are real -- in a browser the shared `ready` does fulfill, the winner does return `desiredSize` to zero, the writer does swap in a fresh `ready`, and the old one stays fulfilled -- but the branch is unreachable. `poll_send_datagram` opens by draining `send_datagram` with `poll_settled`, and that is the same slot the waits use, so a stale fulfilled `ready` is consumed at the top and the loop re-reads `writer.ready()` and gets the current promise. Instrumenting the fallback to flag itself confirmed it: never entered, across the whole harness including a hand-driven race. So this adds no fix, only the guard that was missing. It drives the race by hand -- one clone takes the writer's only slot, two more park on the same `ready`, capacity returns, one wins, and the loser is then awaited with a timeout -- and asserts the loser still sends. A volume loop was tried first and could not be made to fail; capacity recovers between `await`s often enough to miss the window. Worth keeping because the obvious fix would break it. Giving `ready` and `closed` separate slots, as the review recommended, removes the `poll_settled` drain that makes the branch unreachable, and the deadlock becomes real. This check fails if anyone does that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`Bump -proto` rode along on this branch from before the WASM work and is unrelated to it. release-plz owns crate versions here, so a hand-written bump either collides with the one it computes or silently replaces it. The workspace depends on `web-transport-proto` as "0.6", which both versions satisfy, so nothing here needed the bump. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Replaces the release-plz proposal in #363. Two of its minor bumps were driven only by `auto_trait_impl_removed`, which is not a break worth a major-equivalent version here: no name, signature, or behavior changed, and a caller that was relying on `UnwindSafe` for one of these types was relying on an accident. - qmux 0.5.0 -> 0.5.1. The only flagged break is `Session`, `Stream`, and `Upgraded` losing `UnwindSafe`/`RefUnwindSafe`. `Session::stats()` gained RTT and delivery rate (#380), which is additive. - web-transport-quinn 0.12.0 -> 0.12.1. #369 touched this crate only to add the `harness-server` example; the library is unchanged. - web-transport-quiche 0.6.0 -> 0.7.0. Not unwind: #367 and #372 reverse the meaning of `SendStream::set_priority` and `ez::SendStream::set_priority` to higher-first. Same signature, opposite behavior, so a patch would silently invert send order for anyone who upgrades. - web-transport-wasm 0.5.10 -> 0.6.0. Also not unwind: `Error::Streams` is gone, `Error::Session` is now a struct variant, `Error::Closed` is new, and both `closed()` methods take `&mut self`. - web-transport 0.11.0 -> 0.12.0, following wasm, whose items it re-exports wholesale on wasm32. web-transport-proto stays at the 0.6.1 already in its manifest but not yet on crates.io. web-transport-ffi and web-transport-node need no bump at all: their `web-transport-quinn = "0.12"` requirement still matches 0.12.1, so the cascade that gave them 0.1.3 and 0.0.7 in #363 disappears. Also corrects the qmux README install snippet, which still said 0.4. Claude-Session: https://claude.ai/code/session_015gokPRtDeLNb9vgWqdFqWh Co-authored-by: Claude <noreply@anthropic.com>

Semantic conflicts resolved beyond the textual ones: - js/net origin.ts: dev put the broadcast routing table there (moq-dev#2705), main moved the origin *id* module there from lite/ (moq-dev#2910). The table keeps origin.ts; the id module is now the internal js/net/src/hop.ts, shared by both wire protocols as main intended. - js/watch broadcast.ts: dev rejects a whole catalog when a rendition's broadcast reference escapes the root (moq-dev#2630); main hides renditions whose broadcast is not announced (moq-dev#2918). Both kept. The announcement gate moved into #relativeTarget so playback and rendition selection cannot disagree about what is reachable, and filterCatalog now covers text renditions. - js/net ietf publisher: main's options-object constructor plus dev's origin-backed broadcasts and main's cluster advert. - moq-ffi session: main's wasm32 browser client alongside dev's reconnecting moq_tokio::Connection, with Inner::Connection, MoqBackoff, and the moq_tokio::Status conversion gated to native. - moq-net ietf subscriber: main's Arrival parameter plus dev's GOAWAY drain cost. web-transport-wasm 0.6 implements the poll traits moq-net requires (moq-dev/web-transport#369), so the hand-written adapters in moq-wasm and moq-ffi are gone; both files are now just the dial. This is what unblocks moq-ffi's wasm32 build (moq-dev#2911) under dev's poll-only transport (moq-dev#2736). Two of main's additions were written against APIs dev had already changed, and merged cleanly because neither side touched the other's lines: - test/wasm harness published through Established.publish, removed by moq-dev#2705. It now publishes into an Origin and passes publish: origin.consume(). - test/wasm, rs/justfile, moq-bench's hd.toml, and the new iroh doc invoked --server-bind / --server-version / --client-connect, which moq-dev#2915 refuses. moq_net::model::resume::consecutive_updates_wake asserted an absolute wake count. main's kio Park now reuses a still-registered waiter (moq-dev#2905), so applying a subscription change notifies a list the poll is parked on and self-wakes once. That costs a redundant poll and nothing else, while a lost wakeup parks the task forever, so the test measures the delta instead. moq-mux's tdt_round_trips_as_latest_value fails here and on dev alike; it is not a merge regression. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017wFQ5wqKbvWwET3G5MJXY9

kixelated and others added 2 commits August 6, 2026 20:31

Bump -proto

cb976f7

chatgpt-codex-connector Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/session.rs Outdated

Comment thread rs/web-transport-wasm/src/recv.rs Outdated

coderabbitai Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread rs/web-transport-proto/Cargo.toml Outdated

Comment thread rs/web-transport-wasm/src/recv.rs Outdated

Comment thread rs/web-transport-wasm/src/send.rs

Comment thread rs/web-transport-wasm/src/traits.rs

chatgpt-codex-connector Bot reviewed Aug 11, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/session.rs Outdated

Comment thread rs/web-transport-wasm/src/session.rs Outdated

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/error.rs Outdated

coderabbitai Bot mentioned this pull request Aug 12, 2026

web-transport-wasm: dropping an open_uni/open_bi future can orphan a stream #371

Open

kixelated added 2 commits August 11, 2026 21:55

fix(wasm): preserve progress across cloned operations

b3612f2

Merge remote-tracking branch 'origin/main' into codex/pr-369-browser-…

b7c20f3

…fixes # Conflicts: # rs/web-transport-wasm/src/session.rs

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/error.rs Outdated

kixelated and others added 2 commits August 11, 2026 22:06

fix(wasm): avoid inventing session close codes

bd32f9f

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/session.rs

fix(wasm): allow compatible bindgen schemas

acbf53a

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread justfile Outdated

Comment thread rs/web-transport-wasm/src/session.rs

fix(wasm): wake orphan readers and share backpressure

2c91a10

coderabbitai Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread justfile Outdated

Comment thread rs/web-transport-wasm/examples/harness.rs

Comment thread rs/web-transport-wasm/examples/harness.rs Outdated

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/send.rs

fix wasm browser harness edge cases

8db02ab

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/session.rs Outdated

reap abandoned wasm stream opens

0150ab1

chatgpt-codex-connector Bot reviewed Aug 12, 2026

View reviewed changes

Comment thread rs/web-transport-wasm/src/session.rs

Comment thread rs/web-transport-wasm/src/send.rs Outdated

handle closed wasm writers

32c9e89

kixelated and others added 2 commits August 12, 2026 08:05

kixelated enabled auto-merge (squash) August 12, 2026 17:21

kixelated merged commit 2450296 into main Aug 12, 2026
1 check passed

kixelated deleted the claude/web-transport-wasm-poll-5c0060 branch August 12, 2026 17:40

moq-bot Bot mentioned this pull request Aug 12, 2026

chore: release #363

Open

This was referenced Aug 19, 2026

fix(wasm): report None when no subprotocol was negotiated #381

Merged

chore: bump crate versions by hand #384

Merged

kixelated mentioned this pull request Aug 20, 2026

chore: merge main into dev moq-dev/moq#2951

Merged

Uh oh!

Conversation

kixelated commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Bugs the conversion caught

Breaking changes

Smaller things

Review fixes (673a708)

Verification

Note on the branch

Follow-ups

Uh oh!

chatgpt-codex-connector Bot commented Aug 11, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Aug 11, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

kixelated commented Aug 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

kixelated commented Aug 12, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

kixelated commented Aug 12, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

kixelated commented Aug 11, 2026 •

edited

Loading

Review fixes (`673a708`)

coderabbitai Bot commented Aug 11, 2026 •

edited

Loading