Merge tauri-plugin-web-transport into this repo #100
Conversation
WalkthroughThis change adds a new Tauri-based WebTransport plugin and polyfill (web-transport-tauri) with Rust command handlers, desktop implementation, mobile scaffolding, TypeScript RPC client, permissions schema/configuration, and TS polyfill. The repository is reconfigured to use Nix flakes/direnv for development and CI, adds gtk3 to the dev shell, and updates the GitHub Actions workflow to run via Nix. A PNPM workspace and shared TypeScript config/biome settings are introduced. Cargo workspace gains the new member. Justfile tasks are updated. Minor formatting and non-functional edits appear across existing JS/TS examples and modules. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Tip 🧪 Early access (models): enabledWe are currently testing Sonnet 4.5 code review models, which should lead to better review quality. However, this model may result in higher noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 28
🧹 Nitpick comments (35)
.gitignore (1)
4-9: Add common Node/Parcel logs to ignore listHelps keep the repo clean during local builds.
# Node.js node_modules/ dist/ +.parcel-cache/ + +# Logs +pnpm-debug.log* +npm-debug.log* +yarn-error.log* # Direnv .direnv/web-transport-tauri/.rustfmt.toml (1)
1-4: Tweak comment tone; keep settingsMinor wording polish; config itself looks good and matches .editorconfig.
-# i die on this hill +# Prefer tabs in this crate to align with .editorconfig and project style hard_tabs = true max_width = 120scripts/package.ts (1)
5-5: Make path rewrites more robust (optional)Consider handling bare
src/...(not just./src/...) to avoid stale paths in dist/package.json.-import { copyFileSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { copyFileSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; // ... -function rewritePath(p: string): string { - return p.replace(/^\.\/src/, "."); -} +function rewritePath(p: string): string { + // strip optional leading ./ and src/ once + return p.replace(/^(\.\/)?src\//, ""); +}web-transport-tauri/.editorconfig (1)
3-15: Extend overrides for .yaml and MarkdownPrevents YAML tab issues and preserves Markdown hard line breaks.
[*] charset = utf-8 end_of_line = lf trim_trailing_whitespace = true insert_final_newline = true indent_style = tab indent_size = 4 max_line_length = 120 # YAML doesn't support hard tabs 🙃 [**.yml] indent_style = space indent_size = 2 + +[**.yaml] +indent_style = space +indent_size = 2 + +[*.md] +# keep two-space line breaks intact +trim_trailing_whitespace = falseweb-transport-ws/src/stream.ts (1)
36-48: Optional: add table-driven tests for canRecv/canSendA small matrix covering {Bi,Uni} × {server,client} would lock behavior.
web-transport-ws/src/varint.ts (2)
52-59: Use byteLength for clarity with typed arrays.
buffer.lengthworks for Uint8Array butbyteLengthis idiomatic and avoids confusion.- if (2 > buffer.length) { + if (buffer.byteLength < 2) { @@ - if (4 > buffer.length) { + if (buffer.byteLength < 4) { @@ - if (8 > buffer.length) { + if (buffer.byteLength < 8) {Also applies to: 70-75, 77-82, 84-89
26-36: Clarify/guard “append” semantics.
encodetreatsdst.byteLengthas “used” and writes past it within the same ArrayBuffer. This is uncommon; most APIs take an explicit offset/cursor. Consider accepting an offset and returning nextOffset, or document this contract.package.json (2)
8-14: Add a lint script to surface Biome checks in CI.You already depend on Biome; wire it into scripts.
"scripts": { "build": "pnpm -r build", "dev": "pnpm -r dev", "check": "pnpm -r check", "clean": "pnpm -r clean && rimraf dist node_modules/.cache", - "install-all": "pnpm install --frozen-lockfile" + "install-all": "pnpm install --frozen-lockfile", + "lint": "biome check ." },
1-7: Pin the workspace package manager for reproducibility.Consider adding “packageManager” to align local/CI tooling.
{ "name": "web-transport-workspace", "private": true, "type": "module", + "packageManager": "pnpm@9", "engines": { "node": ">=18" },Also applies to: 15-21
web-transport-tauri/.gitignore (1)
6-10: Ignore common npm/pnpm debug logs.Minor QoL improvement.
debug.log +npm-debug.log* +npm-error.log* +pnpm-debug.log* package-lock.jsontsconfig.json (1)
2-13: Tighten TS settings for safer ESM builds.These help avoid downleveled syntax and accidental type erasure.
"compilerOptions": { @@ "declaration": true, "isolatedModules": true, + "verbatimModuleSyntax": true, + "exactOptionalPropertyTypes": true, "strict": true, "skipLibCheck": true },web-transport-ws/examples/client.ts (1)
81-90: Ensure cleanup on error paths.Wrap the main body in try/finally to always close the transport.
-async function main() { +async function main() { + let transport: WebTransport | undefined; // ... - const transport = new WebTransport(url); + transport = new WebTransport(url); // ... -// Run the client -main().catch(console.error); +// Run the client +main().catch(async (err) => { + console.error(err); +});(And inside main, in a finally block:
transport?.close({ closeCode: 0, reason: "Error/cleanup" });)Also applies to: 93-95
web-transport-tauri/Cargo.toml (2)
9-10: Remove unnecessarylinksfield in web-transport-tauri/Cargo.toml
Thelinks = "web-transport-tauri"entry isn’t needed—build.rs does not emit anycargo:rustc-link-liborcargo:rustc-link-searchdirectives.
7-7: rust-version supports three-component SemVer; patch-level “1.77.2” is valid but you may simplify to “1.77” for brevity.biome.jsonc (1)
20-29: Lint rules look fine; consider enabling import sorting later.Optional quality-of-life rule via Biome’s organize imports.
"linter": { "rules": { "style": { "useNodejsImportProtocol": "off" }, + "complexity": { + "noUselessFragment": "warn" + }, "suspicious": { // Some runtimes need ts-ignore "noTsIgnore": "off" } } },web-transport-quinn/web/client.js (1)
22-73: Add basic error handling and await closure for clearer UX.Wrap
run()in try/catch, logtransport.closed, andawait transport.closedbefore final log. Also consider looping reads for streams to handle partial reads.Example:
async function run() { try { // ... existing code ... const closed = transport.closed.catch(e => log(`closed with error: ${e}`)); transport.close(); await closed; log("closed"); } catch (e) { log(`error: ${e}`); } }For stream reads:
if (!datagram) { let chunks = []; while (true) { const { value, done } = await reader.read(); if (done) break; chunks.push(...value); } const recv = new TextDecoder().decode(new Uint8Array(chunks)); log(`recv: ${recv}`); }justfile (2)
41-45: Permit fixing in dirty worktrees to reduce friction.
cargo clippy --fixoften runs on uncommitted changes; add--allow-dirtyalongside--allow-staged.Apply this diff:
-cargo clippy --fix --allow-staged --all-targets --all-features +cargo clippy --fix --allow-dirty --allow-staged --all-targets --all-features-cargo clippy --fix --allow-staged --all-targets --all-features --target wasm32-unknown-unknown -p web-transport +cargo clippy --fix --allow-dirty --allow-staged --all-targets --all-features --target wasm32-unknown-unknown -p web-transport
55-57: Avoid mutating the lockfile during “fix.”Use a read-only install to prevent accidental dependency updates in a fix pass.
Apply this diff:
-pnpm install +pnpm install --frozen-lockfile --prefer-offlineweb-transport-tauri/src/error.rs (2)
25-26: Preserve underlying hex error details in the message.Including the source error helps debugging without leaking internals.
Apply this diff:
- #[error("invalid hex")] - InvalidHex(#[from] hex::FromHexError), + #[error("invalid hex: {0}")] + InvalidHex(#[from] hex::FromHexError),
1-1: Serialize as{ code, message }for programmatic handling on the JS side.A structured error keeps human-readable context and enables client-side branching on
code.Apply this diff:
-use serde::{ser::Serializer, Serialize}; +use serde::{ + ser::{SerializeStruct, Serializer}, + Serialize, +};-impl Serialize for Error { - fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> - where - S: Serializer, - { - serializer.serialize_str(self.to_string().as_ref()) - } -} +impl Serialize for Error { + fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error> + where + S: Serializer, + { + let mut s = serializer.serialize_struct("Error", 2)?; + s.serialize_field("code", self.code())?; + s.serialize_field("message", &self.to_string())?; + s.end() + } +}Add this method (outside the ranges above):
impl Error { pub fn code(&self) -> &'static str { match self { Self::Io(_) => "io", #[cfg(any(target_os = "ios", target_os = "android"))] Self::PluginInvoke(_) => "plugin_invoke", Self::WebTransport(_) => "web_transport", Self::DuplicateSession => "duplicate_session", Self::NoSession => "no_session", Self::NoStream => "no_stream", Self::InvalidHex(_) => "invalid_hex", } } }Also applies to: 29-36
web-transport-tauri/src/rpc.rs (2)
13-20: Validate server_certificate_hashes format (hex sha256).Enforce 64-hex chars early to fail-fast and reduce round-trips.
Options:
- Validate in command handler.
- Introduce a newtype with a regex/length check and
serde(validate_with=...).
1-3: Ensure correct Bytes serialization over the Tauri bridge
・Cargo.toml already enablesbytes = { version = "1", features = ["serde"] }, soByteswill serialize as a JSONnumber[]by default—verify your TS layer wraps these arrays into/fromUint8Arrayon send/receive.
・Noserde_bytesusage detected; consider adding and usingserde_bytes(e.g.ByteBuf) for more compact binary encoding.pnpm-workspace.yaml (1)
1-6: Future-proof package matching (optional).If more packages are added, consider a glob to avoid manual edits.
Example:
-packages: - - web-transport-ws - - web-transport-tauri +packages: + - "web-transport-*".envrc (1)
1-1: Direnv + flakes setup is correct.Tiny nit: add a newline and optional hint comment for contributors.
-use flake +use flake +# run: direnv allowREADME.md (1)
33-35: Consistency: hyphenate JSON-RPC and clarify “client”.Small wording tweak for accuracy.
-- [web-transport-tauri](web-transport-tauri) polyfills the WebTransport API using Tauri. The web client communicates with the Rust client via a JSON RPC. +- [web-transport-tauri](web-transport-tauri) polyfills the WebTransport API using Tauri. The web client communicates with the Rust client via JSON‑RPC.web-transport-tauri/src/index.ts (2)
2-2: Avoid importing with .ts extension to prevent ESM runtime issuesUsing a .ts extension can break consumers unless allowImportingTsExtensions/rewrite transforms are configured. Prefer extensionless import.
Apply:
-import WebTransportTauri from "./polyfill.ts"; +import WebTransportTauri from "./polyfill";
1-12: Make Tauri detection resilient to non-Tauri builds (optional)A static import of @tauri-apps/api can bloat or break web builds. Consider lazy detection without requiring the package at import time.
Option A (non-breaking, keep install() sync; heuristic on globals):
function isTauriRuntime(): boolean { // v1/v2-compatible heuristics; safe in browsers/Node // biome-ignore lint/suspicious/noExplicitAny: runtime probe const w = (globalThis as any); return !!(w && (w.__TAURI_INTERNALS__ || w.__TAURI__)); } export function install(): boolean { if ("WebTransport" in globalThis) return false; if (!isTauriRuntime()) return false; (globalThis as any).WebTransport = WebTransportTauri; return true; }Option B (async, precise via dynamic import of @tauri-apps/api):
export async function installAsync(): Promise<boolean> { if ("WebTransport" in globalThis) return false; try { const { isTauri } = await import("@tauri-apps/api/core"); if (!isTauri()) return false; (globalThis as any).WebTransport = WebTransportTauri; return true; } catch { return false; } }web-transport-tauri/permissions/autogenerated/reference.md (2)
5-5: Fix heading level and trailing colon (markdownlint).Adjust h4 → h3 and drop the colon.
-#### This default permission set includes the following: +### This default permission set includes the following
16-466: Docs UX: huge HTML table; consider a Markdown table + ToC and accessibility tweaks.Switch to Markdown tables and add a generated ToC; if staying with HTML, add a caption and header scope attributes for screen readers.
web-transport-tauri/src/lib.rs (1)
49-53: Minor: collapse cfg-initialization to a single let.Avoid duplicate let bindings under different cfgs.
- #[cfg(mobile)] - let web_transport = mobile::init(app, api)?; - #[cfg(desktop)] - let web_transport = desktop::init(app, api)?; + let web_transport = { + #[cfg(mobile)] + { mobile::init(app, api)? } + #[cfg(desktop)] + { desktop::init(app, api)? } + };web-transport-tauri/src/commands.rs (1)
42-45: reset doesn’t await; make it sync for consistency.Saves an unnecessary async wrapper.
-#[command] -pub(crate) async fn reset<R: Runtime>(app: AppHandle<R>, payload: ResetRequest) -> Result<ResetResponse> { - app.web_transport().reset(payload) -} +#[command] +pub(crate) fn reset<R: Runtime>(app: AppHandle<R>, payload: ResetRequest) -> Result<ResetResponse> { + app.web_transport().reset(payload) +}web-transport-tauri/src/desktop.rs (1)
124-125: Address the TODO comment for reset during readThe TODO comment indicates that resetting a stream during an active read operation is not supported. This could lead to undefined behavior if a reset is attempted while a read is in progress.
Would you like me to help implement proper handling for resetting streams during active read operations or create an issue to track this task?
.github/workflows/pr.yml (3)
17-21: Nix flakes robustness + binary cacheExplicitly enable flakes and hook up Cachix to greatly speed CI.
Apply:
- - uses: cachix/install-nix-action@v27 - with: - github_access_token: ${{ secrets.GITHUB_TOKEN }} + - uses: cachix/install-nix-action@v27 + with: + github_access_token: ${{ secrets.GITHUB_TOKEN }} + extra_nix_config: | + experimental-features = nix-command flakes + + - name: Enable Cachix binary cache + uses: cachix/cachix-action@v15 + with: + name: web-transport + authToken: ${{ secrets.CACHIX_AUTH_TOKEN }}If you don’t plan to push to a private cache, omit authToken and keep just
name.
22-23: Remove no-op bootstrap stepThis echo-only “nix develop” adds latency without value.
Apply:
- # Use our flake to get all development tools (Rust, pnpm, just, cargo tools, etc.) - - run: nix develop --command echo "Development environment ready"
29-29: Minor: unify invocation style and make checks reproducible
-cis the idiomatic form and avoids extra argv parsing.Apply:
- - run: nix develop --command just check + - run: nix develop -c just checkIf
just checkmay hit the network (e.g., fetching registries), ensure the flake pins those tools or add--accept-flake-configif you rely onflake.nixconfig.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
flake.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (59)
.envrc(1 hunks).github/workflows/pr.yml(2 hunks).gitignore(1 hunks)Cargo.toml(1 hunks)README.md(1 hunks)biome.jsonc(1 hunks)flake.nix(1 hunks)justfile(3 hunks)package.json(1 hunks)pnpm-workspace.yaml(1 hunks)scripts/package.ts(2 hunks)tsconfig.json(1 hunks)web-transport-quinn/web/client.js(1 hunks)web-transport-quinn/web/package.json(1 hunks)web-transport-tauri/.editorconfig(1 hunks)web-transport-tauri/.gitignore(1 hunks)web-transport-tauri/.rustfmt.toml(1 hunks)web-transport-tauri/Cargo.toml(1 hunks)web-transport-tauri/README.md(1 hunks)web-transport-tauri/build.rs(1 hunks)web-transport-tauri/package.json(1 hunks)web-transport-tauri/permissions/autogenerated/commands/accept-bidirectional.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/accept-unidirectional.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/accept.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/close-stream.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/close.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/closed.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/connect.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/create-bidirectional.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/create-unidirectional.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/open.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/ping.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/read-stream.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/read.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/reset-stream.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/reset.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/write-stream.toml(1 hunks)web-transport-tauri/permissions/autogenerated/commands/write.toml(1 hunks)web-transport-tauri/permissions/autogenerated/reference.md(1 hunks)web-transport-tauri/permissions/default.toml(1 hunks)web-transport-tauri/permissions/schemas/schema.json(1 hunks)web-transport-tauri/src/commands.rs(1 hunks)web-transport-tauri/src/desktop.rs(1 hunks)web-transport-tauri/src/error.rs(1 hunks)web-transport-tauri/src/index.ts(1 hunks)web-transport-tauri/src/lib.rs(1 hunks)web-transport-tauri/src/mobile.rs(1 hunks)web-transport-tauri/src/polyfill.ts(1 hunks)web-transport-tauri/src/rpc.rs(1 hunks)web-transport-tauri/src/rpc.ts(1 hunks)web-transport-tauri/tsconfig.json(1 hunks)web-transport-ws/examples/client.ts(1 hunks)web-transport-ws/package.json(1 hunks)web-transport-ws/src/frame.ts(8 hunks)web-transport-ws/src/index.ts(1 hunks)web-transport-ws/src/session.ts(16 hunks)web-transport-ws/src/stream.ts(1 hunks)web-transport-ws/src/varint.ts(1 hunks)web-transport-ws/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
web-transport-tauri/src/index.ts (2)
web-transport-ws/src/index.ts (1)
install(4-9)web-transport-tauri/src/polyfill.ts (1)
WebTransportTauri(4-150)
web-transport-ws/src/stream.ts (2)
web-transport-ws/src/varint.ts (1)
VarInt(1-97)web-transport-ws/src/stream.rs (1)
dir(26-32)
web-transport-tauri/src/polyfill.ts (3)
web-transport-tauri/src/desktop.rs (2)
hashes(43-43)accept(90-98)web-transport-tauri/src/rpc.ts (1)
accept(43-45)web-transport-tauri/src/commands.rs (1)
accept(23-25)
web-transport-tauri/src/mobile.rs (2)
web-transport-tauri/src/desktop.rs (1)
init(15-23)web-transport-tauri/src/lib.rs (1)
init(36-57)
web-transport-ws/src/index.ts (1)
web-transport-ws/src/session.ts (1)
WebTransportWs(5-430)
web-transport-tauri/src/rpc.rs (1)
web-transport-tauri/src/rpc.ts (16)
ConnectRequest(3-9)ConnectResponse(11-13)CloseRequest(25-29)CloseResponse(31-31)ClosedRequest(37-39)ClosedResponse(41-41)AcceptRequest(47-50)AcceptResponse(52-54)OpenRequest(60-64)OpenResponse(66-68)ReadRequest(74-77)ReadResponse(79-81)WriteRequest(89-93)WriteResponse(95-95)ResetRequest(103-107)ResetResponse(109-109)
web-transport-tauri/src/desktop.rs (4)
web-transport-tauri/src/lib.rs (3)
init(36-57)web_transport(26-26)web_transport(30-32)web-transport-tauri/src/rpc.ts (24)
connect(15-19)ConnectRequest(3-9)ConnectResponse(11-13)close(21-23)CloseRequest(25-29)CloseResponse(31-31)closed(33-35)ClosedRequest(37-39)ClosedResponse(41-41)accept(43-45)AcceptRequest(47-50)AcceptResponse(52-54)open(56-58)OpenRequest(60-64)OpenResponse(66-68)read(70-72)ReadRequest(74-77)ReadResponse(79-81)write(83-87)WriteRequest(89-93)WriteResponse(95-95)reset(97-101)ResetRequest(103-107)ResetResponse(109-109)web-transport-tauri/src/commands.rs (8)
connect(8-10)close(13-15)closed(18-20)accept(23-25)open(28-30)read(33-35)write(38-40)reset(43-45)web-transport-tauri/src/polyfill.ts (3)
close(118-126)stream(75-97)stream(99-116)
web-transport-tauri/src/lib.rs (5)
web-transport-tauri/src/desktop.rs (11)
init(15-23)new(139-146)connect(32-58)close(69-81)closed(83-88)accept(90-98)open(100-108)read(110-114)write(116-120)reset(122-127)reset(210-220)web-transport-tauri/src/mobile.rs (1)
init(13-22)web-transport-tauri/src/rpc.ts (8)
connect(15-19)close(21-23)closed(33-35)accept(43-45)open(56-58)read(70-72)write(83-87)reset(97-101)web-transport-tauri/src/commands.rs (8)
connect(8-10)close(13-15)closed(18-20)accept(23-25)open(28-30)read(33-35)write(38-40)reset(43-45)web-transport-tauri/src/polyfill.ts (1)
close(118-126)
web-transport-tauri/src/commands.rs (3)
web-transport-tauri/src/rpc.ts (24)
connect(15-19)ConnectRequest(3-9)ConnectResponse(11-13)close(21-23)CloseRequest(25-29)CloseResponse(31-31)closed(33-35)ClosedRequest(37-39)ClosedResponse(41-41)accept(43-45)AcceptRequest(47-50)AcceptResponse(52-54)open(56-58)OpenRequest(60-64)OpenResponse(66-68)read(70-72)ReadRequest(74-77)ReadResponse(79-81)write(83-87)WriteRequest(89-93)WriteResponse(95-95)reset(97-101)ResetRequest(103-107)ResetResponse(109-109)web-transport-tauri/src/desktop.rs (9)
connect(32-58)close(69-81)closed(83-88)accept(90-98)open(100-108)read(110-114)write(116-120)reset(122-127)reset(210-220)web-transport-tauri/src/polyfill.ts (1)
close(118-126)
web-transport-tauri/src/rpc.ts (3)
web-transport-tauri/src/commands.rs (8)
connect(8-10)close(13-15)closed(18-20)accept(23-25)open(28-30)read(33-35)write(38-40)reset(43-45)web-transport-tauri/src/desktop.rs (9)
connect(32-58)close(69-81)closed(83-88)accept(90-98)open(100-108)read(110-114)write(116-120)reset(122-127)reset(210-220)web-transport-tauri/src/polyfill.ts (1)
close(118-126)
web-transport-ws/src/session.ts (2)
web-transport-ws/src/varint.ts (1)
VarInt(1-97)web-transport-ws/src/frame.ts (2)
StopSending(26-30)Any(47-47)
web-transport-ws/src/frame.ts (2)
web-transport-ws/src/stream.ts (1)
Id(10-49)web-transport-ws/src/varint.ts (3)
VarInt(1-97)encode(27-50)decode(52-96)
🪛 markdownlint-cli2 (0.17.2)
web-transport-tauri/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (49)
web-transport-quinn/web/package.json (1)
2-5: Whitespace-only reformat looks fineNo functional changes; consistent with tabs elsewhere.
Cargo.toml (1)
6-6: Add workspace member: LGTMMember inclusion is correct.
scripts/package.ts (1)
63-65: Creating dist/ pre-write: LGTMPrevents ENOENT on first run.
web-transport-ws/src/stream.ts (1)
29-34: Bitmask logic matches QUIC stream ID layoutdir/serverInitiated calculations are correct with BigInt masks.
web-transport-ws/package.json (1)
16-22: No changes needed: tsconfig outDir aligns with “dist”.CompilerOptions.outDir in web-transport-ws/tsconfig.json is set to “./dist”, matching the build and release scripts.
web-transport-tauri/.gitignore (1)
13-17: Keep Cargo.lock ignored: This is a library crate (src/lib.rs present), so ignoring Cargo.lock is standard.tsconfig.json (1)
7-9: Confirm runtime handling of rewritten “.js” importsWorkspace tsconfigs (in web-transport-tauri and web-transport-ws) extend the root tsconfig, so all
*.tsimports you found (e.g. insrc/index.ts,polyfill.ts,session.ts, etc.) will be emitted and rewritten to*.js. Manually verify that your bundler or runtime loader supports these.jsextensions and that no imports will break at runtime.web-transport-ws/examples/client.ts (1)
13-21: LGTM: example wiring is clear and idiomatic.Global polyfill install and URL handling read cleanly.
web-transport-tauri/Cargo.toml (1)
15-16: tauri and tauri-plugin versions exist
crates.io lists tauri 2.5.0 and tauri-plugin 2.2.0. cite12biome.jsonc (1)
17-19: No change required: Biome uses “files.includes”, not “files.include”.Likely an incorrect or invalid review comment.
justfile (1)
34-37: JS/TS checks integration looks good.
pnpm install --frozen-lockfile, recursivecheck, and Biome linting are aligned with the workspace tooling.web-transport-tauri/src/error.rs (1)
9-11: Confirm the cfg gate for mobile builds.
#[cfg(mobile)]may not be set by default; consider gating by target OS.Apply this diff:
- #[cfg(mobile)] + #[cfg(any(target_os = "ios", target_os = "android"))]web-transport-tauri/src/rpc.rs (3)
5-11: Enum casing matches TS. LGTM."kebab-case" mapping aligns with TS union literals.
107-115: FinishRequest exists here but no matching command in build.rs.Either wire a "finish" command or remove these types if writes handle FIN implicitly.
Would you like me to add the command to build.rs and draft the handler?
46-50: ClosedResponse shape diverges from TS.TS defines an empty object; Rust has a TODO field placeholder. This breaks RPC shape checks.
Make it empty (or add the field to TS). Suggested:
- pub struct ClosedResponse { - // TODO error code - } + pub struct ClosedResponse {}Likely an incorrect or invalid review comment.
pnpm-workspace.yaml (1)
1-6: Workspace entries look good.Packages are correctly listed; onlyBuiltDependencies includes esbuild for native binary builds.
web-transport-tauri/tsconfig.json (1)
1-7: No action required: declarations are enabled via base tsconfig
The base tsconfig.json already sets"declaration": true(line 9), so.d.tsfiles will be emitted intodistperoutDir.web-transport-ws/src/index.ts (1)
5-8: Minor style change only. LGTM.No behavioral changes; install() remains idempotent.
web-transport-tauri/build.rs (1)
1-8: Additional search yielded no references to “finish” or “FinishRequest” handlers in the codebase—no evidence of a finish command implementation. Manual verification recommended to confirm whether a “finish” RPC exists elsewhere and should be added toCOMMANDS.web-transport-tauri/package.json (2)
22-24: No changes needed: pinned @tauri-apps/api version is correct
@tauri-apps/api v2.8.0 is the latest stable release and aligns with Tauri core v2.8.x (e.g. core v2.8.5), so the existing pin is valid.
17-20: Packaging script correctly includes compiled outputs and metadata:tscemits compiled.jsfiles todist(perweb-transport-tauri/tsconfig.json), andscripts/package.tsgeneratesdist/package.jsonand copies static assets (README.md,LICENSE-MIT,LICENSE-APACHE) intodist. No additional copying is required.web-transport-tauri/permissions/autogenerated/commands/read.toml (1)
6-13: Allow-read command entry aligns with TS usage; ensure trailing newline at EOFweb-transport-tauri/permissions/autogenerated/commands/write.toml (1)
5-13: LGTM – consistent with the permissions schema patternMatches the allow/deny convention and schema reference; no issues spotted.
web-transport-tauri/permissions/autogenerated/commands/read-stream.toml (1)
6-13: Remove or align unusedread-streampermission manifest
Manifest defines a separateread-streamcommand, but the polyfill only invokesrpc.read(norpc.read_stream); verify that the backend actually exposesread-streamor remove/renamepermissions/autogenerated/commands/read-stream.toml.web-transport-tauri/permissions/autogenerated/commands/create-unidirectional.toml (1)
6-13: Double-check command surface: "create-unidirectional" vs using "open" with bidirectional=falseThe TS polyfill uses rpc.open({ bidirectional: false }) to create a unidirectional stream. If there is no separate "create-unidirectional" command in the backend, this permission entry might be redundant or stale.
Use the same script from the read-stream comment to verify.
web-transport-tauri/permissions/autogenerated/commands/reset.toml (1)
5-13: LGTM – matches referenced RPC ("reset") and follows the allow/deny templateNo issues noted.
web-transport-ws/tsconfig.json (1)
2-6: Base tsconfig already enforcesstrict,moduleResolution, andtarget;dist/is correctly git-ignoredweb-transport-tauri/src/index.ts (1)
1-12: Verify isTauri() export in @tauri-apps/api v2.8.0
Detected @tauri-apps/[email protected] in package.json; ensureisTauri()is still exported from@tauri-apps/api/coreas a function returning a boolean to satisfy its use inweb-transport-tauri/src/index.ts.web-transport-ws/src/session.ts (5)
22-25: LGTM: Clean ReadableStream property formatting.The formatting changes improve readability while maintaining the same functional behavior.
31-37: LGTM: Proper error handling for WebTransport options.The validation logic correctly throws errors for unsupported
requireUnreliableoption and provides appropriate warnings forserverCertificateHashes.
57-67: LGTM: Clean ReadableStream initialization.The formatting changes improve code readability while maintaining identical functionality for stream initialization.
104-104: LGTM: Consistent error message formatting.The error messages are now consistently formatted as single-line strings, improving readability without changing functionality.
Also applies to: 110-111, 122-122
141-142: LGTM: Consistent formatting improvements throughout.All these changes are formatting-only improvements that enhance code readability without altering the functional behavior:
- Consistent indentation and spacing
- Stream ID creation consolidated to single line expressions
- Error handling and cleanup logic remains functionally identical
- Method calls and property assignments properly aligned
The changes maintain the same stream management, error handling, and WebSocket communication logic.
Also applies to: 149-149, 159-160, 181-181, 188-188, 201-201, 205-208, 211-213, 217-217, 226-227, 235-236, 238-243, 252-252, 257-257, 267-267, 285-285, 334-334, 354-354, 381-425
web-transport-tauri/permissions/default.toml (1)
1-12: LGTM: Comprehensive default permissions for WebTransport operations.The permission set includes all essential WebTransport operations: connection management (
allow-connect,allow-close,allow-closed), stream handling (allow-accept,allow-open,allow-read,allow-write), and error handling (allow-reset). This provides a sensible default baseline for typical WebTransport usage scenarios.web-transport-tauri/permissions/autogenerated/commands/close.toml (1)
1-13: LGTM: Standard autogenerated permission structure.The file follows the established pattern for Tauri command permissions with proper schema reference and both allow/deny permissions for the
closecommand. The autogenerated header appropriately warns against manual editing.web-transport-tauri/permissions/autogenerated/commands/ping.toml (1)
1-13: LGTM: Consistent permission manifest structure.The file maintains the same structure as other autogenerated command permissions, properly defining both
allow-pinganddeny-pingpermissions with appropriate descriptions and schema references.web-transport-ws/src/frame.ts (3)
15-17: LGTM: Improved comment formatting.The comments are now consistently formatted as single-line entries, improving readability without changing their meaning or the code functionality.
Also applies to: 23-24, 35-36
47-47: LGTM: Cleaner type union definition.The single-line union type is more concise and easier to read than a multi-line format.
53-54: LGTM: Enhanced frame encoding and decoding logic.The changes improve the frame handling implementation:
- Encoding improvements: Better buffer size calculations and cleaner data handling for stream, reset_stream, stop_sending, and connection_close frames
- Decoding improvements: Added proper validation with empty buffer check and enhanced variable handling
- Code field support: Both ResetStream and StopSending frames now properly encode/decode the code field as a VarInt
The logic correctly maintains protocol compatibility while improving code structure and error handling.
Also applies to: 58-60, 65-65, 74-74, 84-84, 89-91, 98-100, 102-106, 108-113, 121-127, 136-138, 148-151
flake.nix (1)
1-46: LGTM: Well-structured Nix development environment.The flake provides a comprehensive development setup with:
- Rust tooling: Complete set including rustc, cargo, formatting, linting, and utility tools (cargo-shear, cargo-sort, cargo-edit)
- JavaScript tooling: Modern Node.js 24, pnpm 10, and Biome for formatting/linting
- Build tooling: Just for task automation
- Multi-system support: Uses flake-utils for cross-platform compatibility
- Stable foundation: Uses nixpkgs-unstable for latest packages
The structure is clean, well-organized, and provides all necessary tools for the WebTransport project's multi-language development needs.
web-transport-tauri/permissions/autogenerated/commands/reset-stream.toml (1)
1-13: LGTM: Consistent autogenerated permission structure.The file follows the established pattern for command permissions with proper schema reference and both allow/deny permissions for the
reset-streamcommand. The structure aligns with other autogenerated permission files in the project.web-transport-tauri/permissions/autogenerated/commands/close-stream.toml (1)
3-3: Schema path looks correct. LGTM.
../../schemas/schema.jsonresolves as expected fromautogenerated/commands/.web-transport-tauri/permissions/autogenerated/commands/closed.toml (1)
3-3: LGTM on structure and schema reference.Matches the pattern used across command manifests.
web-transport-tauri/permissions/autogenerated/commands/write-stream.toml (1)
5-13: LGTM; mirrors the established pattern.No issues spotted.
web-transport-tauri/permissions/autogenerated/commands/accept-unidirectional.toml (1)
5-13: LGTM; consistent with other accept variants.Identifiers and command strings align with naming.
web-transport-tauri/permissions/autogenerated/commands/create-bidirectional.toml (1)
5-13: LGTM; consistent pattern.No structural issues.
web-transport-tauri/permissions/autogenerated/commands/accept.toml (1)
5-13: LGTM.Matches schema and other manifests.
web-transport-tauri/src/lib.rs (1)
29-33: Trait accessor looks good.State indirection via Manager::state().inner() is fine given setup manages WebTransport.
web-transport-tauri/src/rpc.ts (1)
1-110: Well-structured TypeScript RPC interfaceThe TypeScript RPC interface is well-designed with proper typing, consistent patterns, and clear separation between request and response types. The use of
Record<string, never>for empty responses is a good TypeScript practice.
| @@ -1,4 +1,4 @@ | |||
| name: Rust | |||
| name: CI | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden workflow + cancel redundant runs
Add least-privilege permissions and PR-run concurrency; also bump checkout to v4.
Apply:
name: CI
+permissions:
+ contents: read
+
on:
pull_request:
branches: ["main"]
@@
- - uses: actions/checkout@v3
+ - uses: actions/checkout@v4Optionally cap job runtime:
jobs:
build:
runs-on: ubuntu-latest
+ timeout-minutes: 20📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: CI | |
| name: CI | |
| permissions: | |
| contents: read | |
| on: | |
| pull_request: | |
| branches: ["main"] | |
| jobs: | |
| build: | |
| runs-on: ubuntu-latest | |
| timeout-minutes: 20 | |
| steps: | |
| - uses: actions/checkout@v4 | |
| # … other steps … |
🤖 Prompt for AI Agents
.github/workflows/pr.yml around line 1: the workflow needs hardening and run
deduplication; update the top-level workflow to (1) add a permissions block
granting only required least-privilege scopes for the jobs (e.g., read:
contents, pull-requests; write only what is needed), (2) add a concurrency key
that cancels redundant PR runs (e.g., concurrency: { group: 'pr-${{ github.ref
}}', cancel-in-progress: true }), (3) bump actions/checkout from v3 to v4 in the
checkout step, and (4) optionally add timeout-minutes on long-running jobs to
cap runtime; make these edits directly in .github/workflows/pr.yml.
justfile
Outdated
| # A separate entrypoint for CI. | ||
| setup-tools: | ||
| cargo binstall -y cargo-shear cargo-sort cargo-upgrades cargo-edit | ||
| cargo install -y cargo-shear cargo-sort cargo-upgrades cargo-edit |
There was a problem hiding this comment.
cargo install -y is invalid; remove -y and optionally gate installs.
cargo install doesn’t support -y. This will fail the setup step.
Apply this diff:
- cargo install -y cargo-shear cargo-sort cargo-upgrades cargo-edit
+ # Install only missing tools; keep reproducible with --locked
+ set -eu
+ for crate in cargo-shear cargo-sort cargo-upgrades cargo-edit; do
+ if ! command -v "$crate" >/dev/null 2>&1; then
+ cargo install --locked "$crate"
+ fi
+ done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cargo install -y cargo-shear cargo-sort cargo-upgrades cargo-edit | |
| # Install only missing tools; keep reproducible with --locked | |
| set -eu | |
| for crate in cargo-shear cargo-sort cargo-upgrades cargo-edit; do | |
| if ! command -v "$crate" >/dev/null 2>&1; then | |
| cargo install --locked "$crate" | |
| fi | |
| done |
🤖 Prompt for AI Agents
In justfile around line 14, the command uses an unsupported flag "-y" with
"cargo install"; remove the "-y" so it reads "cargo install cargo-shear
cargo-sort cargo-upgrades cargo-edit". Optionally wrap the installs behind a
guard (check if each binary is already installed or gate with an environment
variable) to avoid reinstalling during CI or local runs.
| const fingerprint = []; | ||
| for (let c = 0; c < fingerprintHex.length - 1; c += 2) { | ||
| fingerprint.push(parseInt(fingerprintHex.substring(c, c + 2), 16)); | ||
| fingerprint.push(parseInt(fingerprintHex.substring(c, c + 2), 16)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden fingerprint hex parsing and validate expected SHA-256 length.
Trims whitespace, ignores non-hex, validates even length, and warns if not 32 bytes.
Apply this diff:
-const fingerprint = [];
-for (let c = 0; c < fingerprintHex.length - 1; c += 2) {
- fingerprint.push(parseInt(fingerprintHex.substring(c, c + 2), 16));
-}
+const fingerprint = (() => {
+ const hex = fingerprintHex.trim().replace(/[^0-9a-f]/gi, "");
+ if (hex.length % 2 !== 0) throw new Error("Invalid fingerprint hex length");
+ const out = new Array(hex.length / 2);
+ for (let i = 0; i < hex.length; i += 2) out[i / 2] = parseInt(hex.slice(i, i + 2), 16);
+ if (out.length !== 32) console.warn(`Expected 32-byte SHA-256 fingerprint, got ${out.length} bytes`);
+ return out;
+})();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const fingerprint = []; | |
| for (let c = 0; c < fingerprintHex.length - 1; c += 2) { | |
| fingerprint.push(parseInt(fingerprintHex.substring(c, c + 2), 16)); | |
| fingerprint.push(parseInt(fingerprintHex.substring(c, c + 2), 16)); | |
| } | |
| // Replace the old fingerprint parsing with: | |
| const fingerprint = (() => { | |
| const hex = fingerprintHex.trim().replace(/[^0-9a-f]/gi, ""); | |
| if (hex.length % 2 !== 0) throw new Error("Invalid fingerprint hex length"); | |
| const out = new Array(hex.length / 2); | |
| for (let i = 0; i < hex.length; i += 2) out[i / 2] = parseInt(hex.slice(i, i + 2), 16); | |
| if (out.length !== 32) console.warn(`Expected 32-byte SHA-256 fingerprint, got ${out.length} bytes`); | |
| return out; | |
| })(); |
🤖 Prompt for AI Agents
In web-transport-quinn/web/client.js around lines 5 to 8, the fingerprint hex
parsing is brittle; update it to first trim whitespace from fingerprintHex,
strip out any non-hex characters, check that the resulting string has even
length and that its byte length equals 32 (i.e. 64 hex chars) and issue a
console.warn if not, then iterate in 2-char steps over the sanitized hex string
and parse each pair into a byte with parseInt(...,16) pushing into fingerprint;
if length is odd, throw or early-return with a warning to avoid misaligned
parsing.
| const url = params.get("url") || "https://localhost:4443" | ||
| const datagram = params.get("datagram") || false | ||
| const url = params.get("url") || "https://localhost:4443"; | ||
| const datagram = params.get("datagram") || false; |
There was a problem hiding this comment.
Fix boolean parsing of ?datagram= to avoid enabling datagrams when set to "false".
URLSearchParams.get() returns a string; any non-empty value is truthy, so ?datagram=false currently enables datagrams.
Apply this diff:
-const datagram = params.get("datagram") || false;
+const datagramParam = params.get("datagram");
+const datagram =
+ datagramParam === "1" ||
+ (datagramParam && datagramParam.toLowerCase() === "true");Also applies to: 39-52
🤖 Prompt for AI Agents
In web-transport-quinn/web/client.js around line 13 (and similarly for the block
around lines 39-52), the code uses params.get("datagram") directly which returns
a string and treats any non-empty value (including "false") as truthy; change
the parsing to explicitly convert the param to a boolean by checking for the
string "true" (e.g., const datagram = params.get("datagram") === "true") so that
?datagram=false yields false and only ?datagram=true (or absence-handling as
desired) enables datagrams.
| this.closed = this.#session.then((session) => | ||
| rpc.closed({ | ||
| session, | ||
| }), | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align closed assignment with updated type or return info.
Completes the stopgap above.
- this.closed = this.#session.then((session) =>
- rpc.closed({
- session,
- }),
- );
+ this.closed = this.#session.then(async (session) => {
+ await rpc.closed({ session });
+});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.closed = this.#session.then((session) => | |
| rpc.closed({ | |
| session, | |
| }), | |
| ); | |
| this.closed = this.#session.then(async (session) => { | |
| await rpc.closed({ session }); | |
| }); |
🤖 Prompt for AI Agents
In web-transport-tauri/src/polyfill.ts around lines 42–47, the assignment
this.closed = this.#session.then((session) => rpc.closed({ session })) no longer
matches the updated return/type contract for closed; either convert the
rpc.closed result to the expected closed value (e.g., map its resolved value to
the boolean/void shape your closed property expects) or update the closed
property's type to match rpc.closed's return type—adjust the .then chain to
transform the rpc.closed result into the correct shape (or change the
property/type) so the assignment's return type aligns with the declared type.
| #[derive(Debug, Clone, Default, Deserialize, Serialize)] | ||
| #[serde(rename_all = "camelCase")] | ||
| pub struct ConnectResponse { | ||
| pub session: usize, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid usize in RPC payloads crossing the JS boundary.
JS numbers are IEEE-754 doubles; values > 2^53-1 lose precision. Use u64 with an enforced upper bound (or u32), or encode as strings.
Minimal change:
- pub session: usize,
+ pub session: u64,(and same for stream). If you keep u64 as JSON numbers, enforce < Number.MAX_SAFE_INTEGER at allocation sites. Alternatively, serialize as strings via a newtype.
Run to locate all sites:
Also applies to: 31-31, 43-43, 55-55, 62-62, 68-71, 76-76, 82-83, 96-98, 108-109, 119-121
🏁 Script executed:
#!/bin/bash
rg -nP 'pub\s+(session|stream):\s*usize' web-transport-tauri/srcLength of output: 822
Replace all usize RPC fields with a fixed-size integer
Avoid using usize in structs sent over the JS boundary—JS only safely represents integers up to 2⁵³ − 1. Change every
pub session: usize,
pub stream: usize,to
- pub session: usize,
+ pub session: u64,
- pub stream: usize,
+ pub stream: u64,(or u32 if your IDs never exceed 2³² − 1). If you keep them as JSON numbers, enforce < Number.MAX_SAFE_INTEGER at allocation sites; otherwise serialize IDs as strings via a newtype.
Occurrences found in web-transport-tauri/src/rpc.rs at lines: 25, 31, 43, 55, 62, 68, 76, 82–83, 96–97, 108–109, 119–120.
🤖 Prompt for AI Agents
In web-transport-tauri/src/rpc.rs around lines 25, 31, 43, 55, 62, 68, 76,
82-83, 96-97, 108-109, 119-120 replace all fields currently typed as usize
(e.g., session, stream, etc.) with a fixed-size integer type (prefer u64; use
u32 only if you guarantee IDs < 2^32) or switch to a string-serialized newtype
if you must preserve larger ranges; update the struct definitions and their
serde (de)serialization attributes accordingly, then audit and update all
allocation/creation sites that set these IDs to either validate they are <
Number.MAX_SAFE_INTEGER when emitting JSON numbers or to produce the
string-serialized form, and update any associated tests and function signatures
that assume usize to use the new fixed-size type.
| pub struct WriteRequest { | ||
| pub session: usize, | ||
| pub stream: usize, | ||
| pub data: Bytes, | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align WriteRequest nullability with TS (Uint8Array | null vs. Bytes).
TS allows null; Rust requires data. This will panic/deser-fail when TS sends null. Decide on one:
- Make Rust optional (preferred if null is meaningful), or
- Make TS non-null.
Apply one of:
- pub struct WriteRequest {
+ pub struct WriteRequest {
pub session: usize,
pub stream: usize,
- pub data: Bytes,
+ pub data: Option<Bytes>,
}or change TS to data: Uint8Array and keep Rust as-is.
🤖 Prompt for AI Agents
In web-transport-tauri/src/rpc.rs around lines 95 to 99, the WriteRequest struct
declares data: Bytes while the TS side allows data to be null; this mismatch
will cause deserialization failures or panics when TS sends null. Make Rust
accept null by changing the field to an Option<Bytes> and update serde
attributes if needed so null maps to None; then audit all usages of
WriteRequest.data and handle the None case (return an appropriate error or treat
as empty payload). Alternatively, if you choose to keep Rust non-optional,
update the TS type to data: Uint8Array (non-null) and ensure callers never send
null.
| "exports": { | ||
| ".": "./src/index.ts" | ||
| }, | ||
| "types": "./src/index.ts", | ||
| "files": [ | ||
| "./src" | ||
| ], |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Publishing risk: exports/types/files point to TS sources; dist isn’t included.
As written, consumers will resolve “./src/index.ts”. Node can’t execute .ts, and “files” excludes “dist”, so a compiled build wouldn’t be shipped. Recommend exporting built JS + d.ts and including “dist” in files.
Apply:
"exports": {
- ".": "./src/index.ts"
+ ".": {
+ "types": "./dist/index.d.ts",
+ "import": "./dist/index.js"
+ }
},
- "types": "./src/index.ts",
+ "types": "./dist/index.d.ts",
"files": [
- "./src"
+ "dist"
],Also applies to: 12-15, 16-22
🏁 Script executed:
#!/bin/bash
# Verify ws package resolves to dist and includes d.ts
jq '.exports, .types, .files' web-transport-ws/package.json
# If you keep source publishing, ensure tsconfig emits to dist:
fd tsconfig.json | xargs -I{} jq '.compilerOptions.outDir? // null' {}
# Dry-run packing to inspect contents (no publish):
( cd web-transport-ws && npm pack --dry-run 2>/dev/null ) || trueLength of output: 118
Ensure package.json ships compiled dist outputs
Exports, types, and files currently point at ./src/*.ts, so npm pack only includes source .ts files and omits the compiled dist output—consumers cannot load .ts at runtime. Update package.json as follows:
"exports": {
- ".": "./src/index.ts"
+ ".": {
+ "import": "./dist/index.js",
+ "types": "./dist/index.d.ts"
+ }
},
- "types": "./src/index.ts",
+ "types": "./dist/index.d.ts",
"files": [
- "./src"
+ "dist"
],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "exports": { | |
| ".": "./src/index.ts" | |
| }, | |
| "types": "./src/index.ts", | |
| "files": [ | |
| "./src" | |
| ], | |
| "exports": { | |
| ".": { | |
| "import": "./dist/index.js", | |
| "types": "./dist/index.d.ts" | |
| } | |
| }, | |
| "types": "./dist/index.d.ts", | |
| "files": [ | |
| "dist" | |
| ], |
🤖 Prompt for AI Agents
In web-transport-ws/package.json around lines 9 to 15, exports, types, and files
point to ./src/*.ts which causes npm to ship only TypeScript sources; update
them to reference the compiled outputs instead (e.g., set "exports" entry to
"./dist/index.js" or "./dist/index.cjs" / "./dist/index.mjs" as appropriate, set
"types" to "./dist/index.d.ts", and include "./dist" in the "files" array), and
confirm any "main"/"module"/"browser" fields match the dist paths so consumers
load the compiled JavaScript and type declarations at runtime.
| encode<T extends ArrayBuffer>(dst: Uint8Array<T>): Uint8Array<T> { | ||
| const x = this.value | ||
| const x = this.value; | ||
|
|
||
| const size = this.size() | ||
| if (dst.buffer.byteLength < dst.byteLength + size) { | ||
| throw new Error("destination buffer too small") | ||
| } | ||
| const size = this.size(); | ||
| if (dst.buffer.byteLength < dst.byteLength + size) { |
There was a problem hiding this comment.
Type error: Uint8Array isn’t generic; signature won’t type-check.
Uint8Array<T> isn’t valid TS. Use non-generic Uint8Array.
- encode<T extends ArrayBuffer>(dst: Uint8Array<T>): Uint8Array<T> {
+ encode(dst: Uint8Array): Uint8Array {
const x = this.value;
@@
- return new Uint8Array(dst.buffer, dst.byteOffset, dst.byteLength + size);
+ return new Uint8Array(dst.buffer, dst.byteOffset, dst.byteLength + size);Also applies to: 49-50
🤖 Prompt for AI Agents
In web-transport-ws/src/varint.ts around lines 27-31 (and also at lines 49-50)
the code incorrectly uses a generic Uint8Array<T>; TypeScript's Uint8Array is
not generic. Remove the generic parameter and change the method signature and
return types to use plain Uint8Array (e.g., encode(dst: Uint8Array): Uint8Array)
and update any matching variable/parameter typings on lines 49-50 to Uint8Array
as well so the signatures type-check.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
web-transport-tauri/permissions/autogenerated/reference.md (1)
5-5: Fix heading level increment for markdown linter compliance.The markdown linter flags a heading level jump from h2 (## Default Permission) to h4 (#### This default permission set includes...). Consider using h3 (###) instead to maintain proper heading hierarchy.
If this file is autogenerated, update the generator template to use:
-#### This default permission set includes the following: +### This default permission set includes the following:justfile (1)
14-14: Consider making cargo install idempotent.The
cargo installcommand will reinstall tools every timejust setupruns, even if they're already present. While the command syntax is correct (no invalid-yflag), this can be slow during repeated local development.Consider adding a guard to check if tools are already installed:
- cargo install cargo-shear cargo-sort cargo-upgrades cargo-edit + #!/usr/bin/env bash + for tool in cargo-shear cargo-sort cargo-upgrades cargo-edit; do + if ! command -v "$tool" &> /dev/null; then + cargo install --locked "$tool" + fi + doneThe
--lockedflag ensures reproducible builds by using exact dependency versions from the tool's lock file..github/workflows/pr.yml (1)
17-25: Update Nix action to latest version and consider explicit caching.The Nix-based development environment setup is well-structured. However, the workflow uses
cachix/install-nix-action@v27, while the latest release is v31.4.1. Consider updating to the latest version to benefit from recent improvements and bug fixes.Additionally, while
github_access_tokenenables basic Nix caching, adding explicit Cachix configuration could further speed up CI runs if the flake has many dependencies.web-transport-tauri/src/desktop.rs (1)
126-131: Address the TODO comment.The TODO indicates that resetting a stream mid-read is not yet supported. Ensure this limitation is documented and tracked.
Would you like me to open an issue to track implementing reset support during active reads?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/pr.yml(2 hunks)flake.nix(1 hunks)justfile(3 hunks)web-transport-tauri/Cargo.toml(1 hunks)web-transport-tauri/permissions/autogenerated/reference.md(1 hunks)web-transport-tauri/permissions/schemas/schema.json(1 hunks)web-transport-tauri/src/desktop.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web-transport-tauri/Cargo.toml
- flake.nix
🧰 Additional context used
🧬 Code graph analysis (1)
web-transport-tauri/src/desktop.rs (4)
web-transport-tauri/src/lib.rs (3)
init(36-57)web_transport(26-26)web_transport(30-32)web-transport-tauri/src/rpc.ts (24)
connect(15-19)ConnectRequest(3-9)ConnectResponse(11-13)close(21-23)CloseRequest(25-29)CloseResponse(31-31)closed(33-35)ClosedRequest(37-39)ClosedResponse(41-41)accept(43-45)AcceptRequest(47-50)AcceptResponse(52-54)open(56-58)OpenRequest(60-64)OpenResponse(66-68)read(70-72)ReadRequest(74-77)ReadResponse(79-81)write(83-87)WriteRequest(89-93)WriteResponse(95-95)reset(97-101)ResetRequest(103-107)ResetResponse(109-109)web-transport-tauri/src/commands.rs (8)
connect(8-10)close(13-15)closed(18-20)accept(23-25)open(28-30)read(33-35)write(38-40)reset(43-45)web-transport-tauri/src/polyfill.ts (3)
close(118-126)stream(75-97)stream(99-116)
🪛 markdownlint-cli2 (0.18.1)
web-transport-tauri/permissions/autogenerated/reference.md
5-5: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (33)
web-transport-tauri/permissions/schemas/schema.json (5)
1-33: LGTM: Root schema structure is well-designed.The root schema correctly uses JSON Schema draft-07 and provides a flexible structure with optional
default,set, andpermissionproperties that align with Tauri's permission model.
34-92: LGTM: Permission definitions are well-structured.The
DefaultPermissionandPermissionSetdefinitions are logically structured with appropriate required fields and constraints. The documentation convention for<h4>headings aligns with Tauri's documentation generation requirements.
133-133: Fix the typo in the scope description.Line 133 contains a typo: "scoped" should be "scopes".
Apply this diff to fix the typo:
- "description": "Allowed or denied scoped when using this permission.", + "description": "Allowed or denied scopes when using this permission.",
152-293: LGTM: ACL definitions are comprehensive and well-documented.The
Commands,Scopes,Value,Number, andTargetdefinitions provide a robust foundation for fine-grained permission control. The recursiveValuetype enables complex ACL structures, and the documentation clearly explains the deny-takes-priority behavior for commands.
294-400: LGTM: PermissionKind enum is comprehensive and well-structured.The
PermissionKinddefinition covers all command permissions with allow/deny variants and includes a documented default permission set. The structure correctly usesoneOfwithconstvalues and provides bothdescriptionandmarkdownDescriptionfields for tooling support.web-transport-tauri/permissions/autogenerated/reference.md (3)
1-14: Past security concern about permissive defaults.The default permission set allowing all commands without scopes has been flagged previously as overly permissive. This is documented as a duplicate concern.
24-232: Past concern about identifier namespacing.A previous review raised questions about whether permission identifiers should use the plugin crate name prefix (web-transport-tauri: vs web-transport:). This has been documented as a duplicate concern.
18-232: LGTM: Permission table is well-structured and comprehensive.The HTML table clearly documents all 16 permission pairs (allow/deny for each command) with consistent formatting and clear descriptions. The identifier naming is consistent throughout.
.github/workflows/pr.yml (2)
1-3: Permissions added, but consider concurrency control and timeouts.The
contents: readpermission is a good security practice. However, the previous review suggested additional hardening:
- Add a
concurrencyblock to cancel redundant PR runs (e.g., when new commits are pushed)- Add
timeout-minutesto cap job runtimeThese additions improve resource usage and workflow reliability.
Consider adding:
name: CI permissions: contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true on:And under the
buildjob:jobs: build: runs-on: ubuntu-latest + timeout-minutes: 20
31-31: LGTM! Just command properly wrapped in Nix environment.The
just checkcommand is correctly invoked throughnix develop --command, ensuring all tools and dependencies are available from the Nix flake.justfile (2)
55-57: JS/TS fixes properly integrated.The JavaScript/TypeScript fix workflow mirrors the check workflow appropriately:
- Uses
pnpm install(without--frozen-lockfilesince fixes may update lock files)- Applies
biome check --fixto automatically fix issuesThis aligns well with the Rust fix workflow and provides a consistent developer experience.
34-37: JS/TS checks properly integrated.The JavaScript/TypeScript checks are well-structured:
--frozen-lockfileensures reproducible installs in CIpnpm -r run checkexecutes checks across all workspace packagesbiome checkprovides linting and formatting validationBoth
pnpm(version 10) andbiomeare correctly declared inflake.nix(lines 32-33) within thejsToolsarray and included in the default development shell, ensuring they're available when runningnix develop.web-transport-tauri/src/desktop.rs (21)
1-13: LGTM!The imports are appropriate for managing concurrent sessions, streams, and integrating with the Tauri plugin system.
15-23: LGTM!The initialization function correctly creates a WebTransport instance with default state. The unused parameters are appropriately prefixed.
25-29: LGTM!The struct design appropriately uses Arc and Mutex for shared concurrent access to sessions, and an atomic counter for session ID generation.
32-40: LGTM!The connect method correctly handles congestion control configuration, establishes the transport connection, and safely inserts the new session with duplicate detection.
Also applies to: 48-58
60-67: LGTM!The helper method cleanly retrieves and clones a session, leveraging the Arc-based Session design for efficient shared access.
69-85: LGTM!The close method now correctly cleans up all stream resources before removing the session, addressing the previous memory leak concern.
87-92: LGTM!The closed method correctly awaits the underlying session's closed future.
94-102: LGTM!The accept method correctly dispatches between bidirectional and unidirectional stream acceptance.
104-112: LGTM!The open method correctly dispatches between bidirectional and unidirectional stream creation with optional send order.
114-118: LGTM!The read method correctly delegates to the session's recv method.
120-124: LGTM!The write method correctly delegates to the session's send method.
134-140: LGTM!The Session struct appropriately uses Arc for shared stream maps and Clone is correctly derived for the Arc-based design.
142-150: LGTM!The Session constructor cleanly initializes all fields with appropriate defaults.
152-158: LGTM!The accept_bi method correctly accepts a bidirectional stream, generates a unique ID, and registers both directions.
160-165: LGTM!The accept_uni method correctly accepts a unidirectional receive stream and registers it.
167-176: LGTM!The create_bi method correctly opens a bidirectional stream with optional send priority and registers both directions.
178-186: LGTM!The create_uni method correctly opens a unidirectional send stream with optional priority and registers it.
214-224: LGTM!The reset method correctly removes and stops both send and receive streams as appropriate without concurrency issues.
41-47: Validate certificate hash length.The code decodes hex-encoded SHA-256 hashes without verifying they are exactly 32 bytes (64 hex characters). Invalid hash lengths should be rejected before passing to the builder.
Consider adding validation:
let client = match payload.server_certificate_hashes { Some(hashes) => { - let hashes = hashes.iter().map(hex::decode).collect::<Result<Vec<_>, _>>()?; + let hashes = hashes + .iter() + .map(|h| { + if h.len() != 64 { + return Err(hex::FromHexError::InvalidStringLength); + } + hex::decode(h) + }) + .collect::<Result<Vec<_>, _>>()?; builder.with_server_certificate_hashes(hashes) } None => builder.with_system_roots(), }?;
188-197: Race condition in concurrent stream read operations.The recv method removes the stream from the map, performs an async read, then re-inserts it. This creates a window where concurrent reads on the same stream can fail with NoStream or cause state corruption.
Consider restructuring to avoid remove-operate-reinsert. Store
Arc<tokio::sync::Mutex<RecvStream>>in the map instead:
- Lock the map briefly to clone the Arc for the stream ID (return NoStream if missing).
- Drop the map lock.
- Acquire the per-stream mutex and perform the read across await points.
- If the stream is closed (returns None), acquire the map lock again and remove the entry.
This avoids holding the global lock across awaits and prevents concurrent access conflicts.
199-212: Race condition in concurrent stream write operations.The send method removes the stream from the map, performs async writes, then re-inserts it. This creates the same race condition as the recv method where concurrent operations can fail or corrupt state.
Apply the same solution as for recv: store
Arc<tokio::sync::Mutex<SendStream>>in the map:
- Lock the map briefly to clone the Arc for the stream ID (return NoStream if missing).
- Drop the map lock.
- Acquire the per-stream mutex and perform writes/finish across await points.
- If finishing the stream (empty data), acquire the map lock again and remove the entry.
This prevents holding the global lock across awaits and eliminates race conditions.
More eyes, more automation.
Summary by CodeRabbit
New Features
Chores
Documentation
Style