fix(pegboard-gateway): forward request ray id to actors

[eersnington]

Standardizing where ray IDs originate. The client (caller) can provide one, otherwise the engine generates it. The same ray ID is forwarded to the actor and returned in the resp.

[eersnington]

Stack for rivet-dev/actors

• #5734 docs(rivetkit): add actor tracing docs
• #5733 feat(rivetkit): forward otel sdk warnings to the pino logger
• #5732 feat(rivetkit): trace actor to actor calls
• #5730 feat(rivetkit-core): trace queue sends and receives
• #5731 feat(rivetkit-core): trace raw http requests
• #5729 feat(rivetkit-core): trace scheduled invocations
• #5726 feat(rivetkit): send trace context from clients
• #5723 feat(rivetkit): pass trace context to js and logs
• #5727 feat(rivetkit): trace sqlite operations
• #5721 feat(rivetkit): trace actor invocations
• #5728 fix(pegboard-gateway): forward request ray id to actors 👈
• #5724 test(rivetkit): let driver runtimes take extra environment
• #5722 fix(rivetkit): disable services in driver test runtimes
• main

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

_{change ootsovmt}

[the-company-company]

🟠 1 medium-severity finding

_{Reviewed commit d69ed40.}

[claude]

Review: forward request ray id to actors (re-verified against ef31ccb; diff is unchanged from the previously reviewed commit)

Independently re-traced the flow (ProxyService::process header normalization -> RequestContext -> pegboard-gateway2/pegboard-gateway3 header forwarding -> response header reconciliation) rather than trusting the diff at face value, and confirmed the RivetKit-side claim by grepping rivetkit-typescript/ and rivetkit-rust/. My findings match the prior review on this PR.

Findings

1. Doc comment overstates a cross-repo guarantee (proxy_service.rs:47-50)

/// Returns `value` when it is a ray ID actors accept: 1 to 30 characters of
/// `[A-Za-z0-9_-]`. RivetKit enforces the same bound on its side. The engine
/// cannot depend on RivetKit crates, so the rule is repeated here.

Confirmed by dedicated search: rayId/ray_id is treated as an opaque, unbounded pass-through string everywhere in rivetkit-typescript/ and rivetkit-rust/ (actor/errors.ts only does a typeof === "string" check, client/utils.ts:170 and client/actor-handle.ts read it straight off the x-rivet-ray-id header, rivetkit-napi/src/actor_factory.rs deserializes it as an unconstrained Option<String>). No length or charset validation exists on the RivetKit side, and no BARE schema indirectly imposes 30 chars either. "RivetKit enforces the same bound" is inaccurate - the bound is unilateral. The 30-char/[A-Za-z0-9_-] choice itself is sound (it matches the internal rivet_util::Id base36 string length exactly, and the charset is safe to embed in a header value and in the "{group}.{code}#{ray_id}" WS close reason without injection risk), so this is a comment-accuracy nit, not a functional issue - just drop or reword the "RivetKit enforces..." sentence so it doesn't imply a synced contract that doesn't exist.

2. forward_ray() calls in gateway2/gateway3 look like no-ops today
ProxyService::process rewrites x-rivet-ray-id directly on req (proxy_service.rs:381-390) before req_ctx.headers is cloned from that same request a few lines later (req.headers().clone() at the RequestContext::new call). Since RequestContext is only constructed once, from that already-mutated request, pegboard-gateway2/pegboard-gateway3 building their header maps from req_ctx.headers() will already see the validated/generated value by the time req_ctx.forward_ray(&mut headers) runs (pegboard-gateway2/src/lib.rs:140,364, pegboard-gateway3/src/lib.rs:137, pegboard-gateway3/src/http_stream/handler.rs:85). These calls are harmless (idempotent overwrite of the same key/value) but read as load-bearing when they're actually defense-in-depth. Worth a one-line comment noting that, or removing them if the intent really was for RequestContext to be the single source of truth.

3. Minor test-coverage gaps

• No test for bounded_external_ray_id rejecting a same-length-or-shorter value with a disallowed byte (space, ., #, non-ASCII) - tests/proxy.rs only exercises the 31-char over-length case, so charset rejection is untested.
• No test asserts the header actually reaches the actor-facing request through pegboard-gateway2/pegboard-gateway3; coverage is entirely at the guard-core HTTP boundary. Given finding 2, a regression in forward_ray (e.g. if RequestContext ever stops being built from the mutated request) wouldn't be caught today.

4. Minor style nit
proxy_service.rs mixes direct field access (req_ctx.external_ray_id, e.g. line 477) and the accessor method (req_ctx.external_ray_id(), e.g. lines 635/1339). Not a bug, just inconsistent within the same file.

What's solid

• Charset is ASCII-only (1-30 bytes of [A-Za-z0-9_-]), so HeaderValue::from_str can't fail and there's no header/log-injection risk from client-controlled input - correctly validates untrusted client input at the engine edge per this repo's trust-boundary rules.
• The response-header mismatch check correctly special-cases api-builder's own response ray id (existing_ray_id_value != req_ctx.ray_id.to_string()) rather than being dead code - for API routes reached via CustomServe, the request extensions (including RequestIds) are preserved in-process, so api-builder's middleware reuses the same internal ray_id guard set, and this now correctly avoids a false-positive "downstream service set ray id to a different value" warning.
• Net effect is a real improvement: previously an arbitrary, unvalidated client-supplied x-rivet-ray-id could be echoed/forwarded as-is; now it's validated and bounded at the edge, consistently forwarded to the actor, and echoed back on the response.
• Structured tracing fields (no string-interpolated messages), lowercase log text, and comment style follow this repo's CLAUDE.md conventions.

Nothing blocking. Item 1 (fix/drop the inaccurate comment) is the only one worth doing before merge; 2-4 are optional polish/follow-up.

[the-company-company]

🟠 1 medium-severity finding

_{Reviewed commit 87d5134.}

[eersnington]

it states RivetKit enforces the same bound on its side, but I could not find a matching length or charset validation anywhere in rivetkit-typescript or rivetkit-core in this repo.

comes in later down the stack #5721 #5726

Silent no-op on HeaderValue::from_str failure

already validated as header safe, so this can't fail with the current inputs

does not exercise an invalid-character ray id (for example one containing a dot or space that is still short enough)

not necessary. one test covers a similar edge case

a one-line comment noting why it's needed (guards against the rare HeaderValue::from_str failure from point 2)

that isn't why it's there. it forwards the validated ray ID from the request context

[the-company-company]

🟠 1 medium-severity finding

_{Reviewed commit 7e3ab40.}

[the-company-company]

🟠 1 medium-severity finding

_{Reviewed commit 79a7a3f.}

[the-company-company]

🟠 1 medium-severity finding

_{Reviewed commit 9ded92b.}

[the-company-company]

✅ No issues found

_{Reviewed commit dc30cc2.}

[MasterPtato]

• is keeping external ray id a string instead of parsing as Id intentional?
• how do you find traces for an engine ray_id vs an external_ray_id?
• is it easy to find the external_ray_id associated with an engine ray_id and vice versa?
• the external ray id isnt passed down the line to consumers like api and actor workflows. see ctx.with_ray in create_routing_function. is this intentional?
• would the api be cleaner if the two were combined and external_ray_id was required to be a proper Id?

if keeping engine rays and client rays separate is part of the goal then most of these questions can be ignored