Skip to content

feat(http): stream HTTP bodies end-to-end - #5516

Open
NathanFlurry wants to merge 1 commit into
mainfrom
stack/feat-http-stream-http-bodies-end-to-end-npntkwqx
Open

feat(http): stream HTTP bodies end-to-end#5516
NathanFlurry wants to merge 1 commit into
mainfrom
stack/feat-http-stream-http-bodies-end-to-end-npntkwqx

Conversation

@NathanFlurry

@NathanFlurry NathanFlurry commented Jul 30, 2026

Copy link
Copy Markdown
Member
  • Restore end-to-end streaming for HTTP request and response bodies.
  • Add Envoy protocol v7 compatibility and dependent Flume changes.

@NathanFlurry

NathanFlurry commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Stack for rivet-dev/rivet

Get stack: forklift get 5516
Push local edits: forklift submit
Merge when ready: forklift merge 5516

change npntkwqx

@railway-app

railway-app Bot commented Jul 30, 2026

Copy link
Copy Markdown

🚅 Deployed to the rivet-pr-5516 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink 😴 Sleeping (View Logs) Web Jul 30, 2026 at 9:21 pm
frontend-inspector 😴 Sleeping (View Logs) Web Jul 30, 2026 at 9:19 pm
frontend-cloud 😴 Sleeping (View Logs) Web Jul 30, 2026 at 9:19 pm
website 😴 Sleeping (View Logs) Web Jul 30, 2026 at 9:17 pm
ladle ✅ Success (View Logs) Web Jul 30, 2026 at 9:11 pm
mcp-hub ✅ Success (View Logs) Web Jul 30, 2026 at 9:11 pm

@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Overview

This PR restores end-to-end streaming for HTTP request/response bodies through the full stack (guard-core -> pegboard-envoy -> envoy wire protocol -> pegboard-gateway2 -> rivetkit-core/napi/TS), and introduces envoy protocol v7 (with v6/v7 converters) to carry the new streaming message types. It also updates the flue-runtime integration to consume the new streaming behavior. Reviewed in five slices (gateway/guard-core, envoy protocol/envoy-client, rivetkit-core/rust/napi/wasm, rivetkit TypeScript client/registry, flue-runtime + misc). Overall the layering is respected well (core owns lifecycle/dispatch, napi stays pure bindings, TS client owns retry), the backpressure design (bounded response-body channel/budget) is sound, and protocol versioning follows the required "add v7, migrate converters" pattern rather than mutating v6. The issues below are concrete and worth addressing before merge, roughly in priority order.

Correctness issues

  • Streamed requests unconditionally lose retry-on-transient-failure (engine/packages/guard-core/src/proxy_service.rs, engine/packages/pegboard-gateway2/src/lib.rs): PegboardGateway2::streams_request_body() is hardcoded to true for the whole gateway, so handle_http_request skips the existing retry loop (service_unavailable, actor_stopped_while_waiting, tunnel_request_aborted, etc.) entirely, even though the actual streaming decision (should_stream_http_request_body_hint) only streams unknown-size/>64KB bodies, and GET/HEAD are always buffered. Most requests are still fully buffered internally but now lose transparent retry purely because of the trait's class-wide granularity. Consider making the retry-vs-stream decision per-request rather than gating on a blanket trait flag.
  • Message-index race between response-start and overload-abort (engine/sdks/rust/envoy-client/src/actor/http.rs, handle_req_chunk's TrySendError::Full branch): on backpressure, task_abort_handle.abort() only cancels at the next await point, so the aborted task can still independently emit ToRivetResponseStart{index:0} concurrently with the abort path's ToRivetResponseAbort{index:0} for the same request, producing duplicate/racing indices on the wire.
  • u16 message-index wraparound with no bound check (same file, send_response_data_chunks): wrapping_add(1) silently wraps after roughly 65536 chunks with no log/error, causing chunk-index collisions on very large streamed responses.
  • wasm's new cancelToken can never fire (rivetkit-typescript/packages/rivetkit-wasm/src/lib.rs around line 816-823, root cause in rivetkit-rust/packages/rivetkit-core/src/registry/http.rs around line 126-140): cancellation is only armed for ActorHttpResponse::Stream; wasm's response path always produces Buffered, so the newly exposed cancelToken is dead wiring as written.
  • Retry-safety gap for non-Request streaming bodies (rivetkit-typescript/packages/rivetkit/src/client/actor-handle.ts line 644): #fetchWithResolvedActor only clones/guards the body when input is Request-shaped. A caller passing a path/URL with init.body as a raw ReadableStream reuses the same disturbed stream across retry attempts, which can silently send a truncated/empty body on retry. No test covers this path.

Convention / style violations (per CLAUDE.md)

  • Catch-all match arm on a protocol enum: engine/packages/pegboard-gateway2/src/http_stream/response.rs (drain_http_response_stream) uses other => { ... } over ToRivetTunnelMessageKind, equivalent to a banned _ => fallthrough. New variants would silently fold into a generic "unexpected message" path instead of forcing an explicit decision.
  • std::sync::Mutex in a new forced-sync context (engine/sdks/rust/envoy-client/src/callbacks.rs): ActorStopHandle wraps its oneshot::Sender in Arc<std::sync::Mutex<...>> with hand-rolled poison unwrapping. Per the async-locks policy this should be parking_lot::Mutex, which avoids the poisoning boilerplate entirely.
  • integrations/flue-runtime/CLAUDE.md gains a roughly 152-line prose section (numbered procedures, multi-line snippets) that violates the "keep CLAUDE.md entries concise, no paragraphs" rule; this content belongs in .claude/reference/ or docs-internal/. The directory also still has no AGENTS.md symlink despite this PR touching the file.
  • Integration duplicates RivetKit's local-Engine default resolution: integrations/flue-runtime/src/target.ts (around line 422-428) adds its own FLUE_MODE === 'local' / endpoint-presence check to decide registry.config.startEngine, which duplicates config-resolution logic CLAUDE.md says must be left to RivetKit itself.
  • Missing vi.waitFor justification comment: new await vi.waitFor(...) in rivetkit-typescript/packages/rivetkit/tests/registry-readiness.test.ts has no preceding // comment explaining why polling is needed, per the repo's enforced convention.
  • Orphaned SVG asset: website/src/content/posts/2026-07-23-flue-now-supports-agentos/container-vs-isolate.svg is committed to git but not referenced anywhere in the post's page.mdx, looks like leftover/forgotten content, and website media generally belongs in R2 rather than git.

Minor / lower priority

  • Non-atomic two-counter budget check in HttpResponseQueueBudget::try_reserve (pegboard-gateway2/src/http_stream/response_queue.rs) makes the cap approximate rather than hard under concurrent forwarders for one request. Self-corrects, not a real bug.
  • Unbounded outbound WS channel (engine/sdks/rust/envoy-client/src/connection/native.rs and wasm.rs, tracked via existing TODO fix(envoy-client): bound the shared WebSocket writer queue #5468) now carries streamed response chunks too, making its memory-bound gap more consequential given this PR's purpose. Worth confirming it's an active near-term follow-up.
  • Buffered-vs-streamed cancellation-arming asymmetry in rivetkit-core/src/registry/http.rs (around line 126-140) has no comment explaining the intent, and is currently only observable on wasm (see cancelToken finding above) since NAPI masks it with its own unconditional dispatch-cancel token.
  • Request::into_buffered() (rivetkit-core/src/actor/messages.rs around line 113-124) has redundant double-.take() shadowing. Not a bug, just simplifiable.
  • Fragile cross-file invariant in rivetkit-typescript/packages/rivetkit/src/registry/native.ts (around line 4752-4762): bodyCompletion.then(cleanupRequest).catch(logOnly) skips cleanup on rejection, currently safe only because pumpResponseBody in native-http.ts never rethrows.
  • tests/actor-http-client.test.ts uses vi.stubGlobal("fetch", vi.fn(...)) for a race test rather than a real local server like its sibling streaming tests. Not a banned API, but in tension with the "test against real infrastructure" preference.

Test coverage

Coverage is generally strong: new SSE/streaming contract harness and driver tests exercise cancellation, backpressure, partial reads, and shutdown-drain; envoy protocol v6/v7 converters have dedicated compat tests; new gateway2 http_stream support modules have payload-accounting tests. Gaps worth closing: no test for the non-Request streaming-body retry path noted above, and no regression test for the message-index wraparound / race conditions in envoy-client/src/actor/http.rs.

Security / trust boundaries

No violations found of the envoy/pegboard-envoy or client/engine untrusted-boundary rules; envoy-originated streaming data appears validated before reaching trusted internal systems in the reviewed files.

@NathanFlurry
NathanFlurry force-pushed the stack/feat-http-stream-http-bodies-end-to-end-npntkwqx branch from 1708458 to 3ed475a Compare July 30, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant