initial hack working playback in JS

[kkroo]

No description provided.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: fa92fc7

Scope note: this is an all-additions PR (0 deletions), so the files at this head are the diff. The GitHub diff API refuses it (>20k lines), so each finding below was verified by reading the file at this exact commit. gateway.go and conn.go are the only pre-existing files touched, +2 lines each (a build tag).

Critical Issues (2)

• [pr-review-toolkit + gstack/review + native-codex — all three lenses independently] gateway_wasm.go:241 — Unchecked type assertion on network-controlled bytes panics the whole WASM module; the nil guard on line 242 is unreachable dead code.

  • The length guard at line 231 is len(data) < m.DataMsgHdrLen, so a datagram of exactly header length (empty payload), a truncated one, or an IPv6-encapsulated one reaches gopacket.NewPacket. That yields a DecodeFailure with no network layer, so p.NetworkLayer() returns a nil interface and nil.(*layers.IPv4) panics. Nothing recovers — there is no recover() anywhere in the Go sources, and MulticastConnWASM.ReadFrom (conn_wasm.go:69) calls this in an unbounded for loop, so a Go panic under GOARCH=wasm aborts the module and kills the gateway for every stream. The socket binds 0.0.0.0 (sw-socket-manager.js:34), so this is a one-packet remote DoS with no auth or on-path position required.
  • Line 257 in the same function already uses the correct comma-ok form for the UDP layer, so line 241 is a local inconsistency, not a missing convention.
  • Fix: ipHdr, ok := p.NetworkLayer().(*layers.IPv4); if !ok || ipHdr == nil { return nil, nil, fmt.Errorf(...) }, and consider rejecting data messages shorter than DataMsgHdrLen + 20 + 8 up front.
  • Context, not a finding against this PR: the same bare-assertion shape already exists in the pre-existing conn.go:175, which this PR leaves unchanged. Worth fixing there too, but it is not new here.

• [native-codex + pr-review-toolkit] iwa-amt-gateway/packet-buffer.js:329 — appendBuffer(dataToAppend.buffer) passes the backing ArrayBuffer instead of the view, silently re-appending the exact bytes the keyframe-sync just discarded.

  • sanitizeTSStream returns data.subarray(startOffset) on every path (ts-sync.js:208/224/231/236) — a view into combined (allocated exactly-sized at line 263). .buffer ignores byteOffset/byteLength, so after logging ✓ Synced to keyframe, discarded N bytes (line 297) the code appends those N bytes anyway. The decoder is initialized mid-GOP on unaligned data — precisely the MEDIA_ERR_DECODE / stuck-playback condition the recovery machinery elsewhere in this file exists to paper over. The alignment guard at line 312 cannot catch it: isTSAligned(dataToAppend) inspects the view while a different byte range is what gets appended.
  • This is the PR's headline feature ("working playback"), and the hazard is already documented one layer up — see the CRITICAL FIX: Pass Uint8Array directly, NOT .buffer comment at app.js:247. The fix was applied there but missed at this sink.
  • Fix: this.sourceBuffer.appendBuffer(dataToAppend) — a Uint8Array is a valid BufferSource and honors the offset.

Important Issues (3)

• [gstack/review] iwa-amt-gateway/output-servers/udp-server.js:186 — broadcastPacket gates on this.dataSocket, which is set to null in the constructor and assigned nowhere in the tree, so the UDP output server forwards zero packets while reporting itself healthy.

  • dataSocket has exactly two occurrences: the declaration (line 10) and this guard. start() assigns controlSocket, dataWriter and dataPort but never dataSocket. A client sends SUBSCRIBE, receives a valid ACK carrying dataPort, and getStatus() reports enabled: true with a live subscription count — but every broadcastPacket returns 0 before the send loop, with no error to diagnose it.
  • Fix: gate on !this.dataWriter (what sendData actually uses at line 245), or delete the vestigial field.

• [gstack/review + pr-review-toolkit] iwa-amt-gateway/output-servers/websocket-server.js:110 — The WebSocket output server is built on chrome.sockets.tcp.*, a Chrome Apps API not available in an Isolated Web App service worker, and its startup failure is swallowed into a "servers enabled" status.

  • The sibling tcp-server.js and udp-server.js both correctly use Direct Sockets (TCPServerSocket/UDPSocket); only this file uses chrome.sockets (8 references, 0 Direct Sockets). service-worker.js:41 even gates startup on typeof TCPServerSocket !== 'undefined' and then registers this chrome.sockets-based server at line 50 — while the correctly-implemented tcp-server.js is never registered at all. server-manager.js:114 sets this.enabled = true unconditionally after Promise.allSettled, so a total startup failure still reports serversEnabled: true and any consumer pointed at the WS port hangs with no diagnostic.
  • Two further defects in this file that will surface once the API is corrected: setupClientReceiveHandler (lines 152/158) adds a global onReceive/onReceiveError pair per accepted client and nothing ever removes them (there is no removeListener anywhere in the PR), so listeners grow without bound and a recycled socket ID gets double-dispatched — the same bytes appended to client.buffer twice, desyncing the frame reader; and the payloadLength === 127 branch (lines 322-326) advances offset to 10 without ever reading the 64-bit length, leaving payloadLength at 127.
  • Fix: port to TCPServerSocket or stop registering it in service-worker.js; separately, set serverManager.enabled only when at least one server actually started.

• [native-codex] iwa-amt-gateway/sw-socket-manager.js:150 — _receiveLoop calls the async onPacket callback without awaiting it and immediately re-enters reader.read(), so there is no backpressure between the multicast source and the slowest consumer.

  • The registered callback (service-worker.js:132) is async and awaits serverManager.handleIncomingPacket, which serially awaits broadcastPacket per server, which serially awaits one send round-trip per client. One client on a congested link stalls that chain while the read loop keeps pulling datagrams at line rate, so pending promises accumulate without bound — each retaining a full packet plus, at service-worker.js:138, an Array.from(packet) plain-number array per connected client (roughly 8x the byte size). At ~1000 pkt/s the SW heap climbs until the browser terminates it, closing the UDP socket and killing every stream, not just the slow consumer.
  • Fix: await onPacket(...) (kernel-side UDP drop is the correct loss behavior for live video), or give each client a bounded drop-oldest queue; and post the Uint8Array as a transferable rather than Array.from.

Suggestions (3)

• [native-codex] gateway.go:1, conn.go:1 — Build tags are asymmetric: //go:build !wasm here vs //go:build js && wasm on the _wasm.go files. A GOOS=wasip1 GOARCH=wasm build matches neither set, leaving package amt with no implementation. //go:build !(js && wasm) closes the gap. There are no CI workflows at this head, so nothing currently catches it.
• [gstack/review] iwa-amt-gateway/packet-buffer-optimized.js:1 — 302 lines imported by nothing in the tree. Two near-identical buffer implementations will drift; delete it or make it the one in use.
• [gstack/review] iwa-amt-gateway/iwa-bundle/.well-known/manifest.webmanifest:1 — byte-identical to iwa-bundle/manifest.webmanifest apart from a trailing newline, and both are hand-maintained. Generate one from the other in the build to prevent divergence.

Strengths

• The bundle CSP is genuinely tight for a prototype: default-src 'self', connect-src 'self', require-trusted-types-for 'script' with a named trusted-types allowlist, and the permission set is exactly one entry (direct-sockets).
• No hardcoded credentials, keys, or tokens anywhere in the tree; the stream config points only at public TreeDN test sources.
• Install scripts scope rm -rf to a dedicated, properly quoted $HOME/.chrome-iwa-persistent profile dir, and no script passes --disable-web-security, --no-sandbox, or --ignore-certificate-errors.
• udp_socket_wasm.go:37 bounds its read/write channels at 100 with drop-on-overflow — the right instinct, and the model the JS receive path (Important #3) should follow.
• The correct comma-ok pattern at gateway_wasm.go:257 and the .buffer warning at app.js:247 show both Critical fixes are already understood in-tree; they just need applying at the two sites that were missed.

Recommended Action

1. Fix the two Critical issues before merge — gateway_wasm.go:241 is a one-packet remote crash, and packet-buffer.js:329 defeats the keyframe sync this PR is built to deliver. Both are one-line changes.
2. Address the Important issues this cycle: the UDP and WebSocket output servers cannot currently deliver bytes to an external consumer, and server-manager.js:114 masks that behind a healthy status.
3. Consider the Suggestions opportunistically.

Given the "initial hack" framing I've deliberately skipped style, docs, and test-coverage commentary; everything above is a concrete defect with a verified line at this commit.