test(rivetkit): cover bypass fetch header forwarding

[NathanFlurry]

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[NathanFlurry]

• fix(pegboard): align drain grace defaults with stop threshold #4958 : 2 dependent PRs (#4960 , #4966 )
• fix(rivetkit): preserve bridge error messages #4959
• fix(rivetkit): fire abort signal on shutdown finalize #4957
• fix(rivetkit): use startup kv preload #4956
• fix(client): flatten skipReadyWait option #4955
• ci(publish): add manual preview publish flow #4954
• fix(depot): add ltx decode diagnostics #4953
• fix(universaldb): preserve plain set values #4952
• fix(universaldb): align versionstamp operand encoding #4951
• fix(rivetkit): export database provider types #4950
• wip: kitchen sink agentic loop finish #4949
• chore(rivetkit): note duplicate admission logic #4948
• test(rivetkit): cover bypass fetch header forwarding #4947 👈 (View in Graphite)
• feat(kitchen-sink): add actor probe controls #4946
• fix(gateway): split actor response wait errors #4945
• fix(guard): handle bypass preflight before query parse #4944
• feat(kitchen-sink): add mock agent event log copy #4943
• fix(kitchen-sink): default mock agent to local endpoint #4942
• test(rivetkit): enable driver lifecycle regressions #4941
• test(rivetkit): verify cached websocket db refs exceed grace #4940
• fix(rivetkit-core): keep sleeping actors reachable during grace #4939
• test(rivetkit): cover async websocket close db access #4938
• fix(rivetkit): keep actors awake until keepAwake work finishes #4937
• fix(rivetkit): persist native onSleep state on errors #4936
• fix(rivetkit-core): allow dispatch during sleep grace #4935
• fix(rivetkit): make duplicate sleep requests idempotent #4934
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[claude]

Code Review: test(rivetkit): cover bypass fetch header forwarding

Overview

This PR adds two new test cases to remote-engine-client-public-token.test.ts that verify the bypassConnectable header is correctly forwarded through the request pipeline at two layers:

1. Driver layer — RemoteEngineControlClient.sendRequest() with { bypassConnectable: true } sets HEADER_RIVET_BYPASS_CONNECTABLE: "1" on the outbound HTTP request.
2. Client layer — ActorHandle.fetch() with { gateway: { bypassConnectable: true } } threads the option through rawHttpFetch → sendRequest and produces the same header.

---

Strengths

• Good layered coverage. Testing both sendRequest directly and the full handle.fetch() path catches regressions at different abstraction levels.
• Self-contained mocks. Each test installs its own vi.stubGlobal("fetch", ...) and restores it via afterEach(vi.unstubAllGlobals). No shared state risk.
• disableMetadataLookup: true is the right choice here — it eliminates the metadata fetch noise so each test only makes one HTTP call and the assertions are unambiguous.
• Consistent with existing patterns — the fetchCalls accumulator + normalizeRequest helper mirrors the pre-existing tests in the file.
• No vi.mock / jest.mock — uses vi.fn() for tracking only, which is allowed per project conventions.

---

Issues and Suggestions

[Minor] Misleading actor name in the second test

The call client.getForId("mockAgenticLoop", "actor-http-bypass") passes "mockAgenticLoop" as the actor type name (first arg to getForId(name, actorId)). That name reads like a real actor type from a different feature area and is unrelated to this test. Something like "test-actor" or "counter" would be less surprising to a future reader.

[Minor] No negative path (header absent when option is omitted)

Both new tests only cover bypassConnectable: true. There is no assertion that the header is absent when the option is omitted or false. The existing tests implicitly cover this, but an explicit expect(actorRequest?.headers.get(HEADER_RIVET_BYPASS_CONNECTABLE)).toBeNull() somewhere would make the contract explicit.

[Nit] Unnecessary optional chaining

After expect(fetchCalls).toHaveLength(1), fetchCalls[0] is guaranteed to exist. The ?. in actorRequest?.headers.get(...) is unnecessary. This matches the pre-existing tests, so it is not a blocker.

---

Summary

Clean, focused test PR with good coverage of the bypassConnectable forwarding path. All issues above are minor style/completeness nits — the logic is correct and the tests accurately exercise the feature. The misleading actor type name is the most worth fixing before merge.

Generated with Claude Code

[claude]

Code Review: test(rivetkit): cover bypass fetch header forwarding

Overview: This PR adds two new tests to remote-engine-client-public-token.test.ts that verify the bypassConnectable flag correctly propagates HEADER_RIVET_BYPASS_CONNECTABLE through both the low-level RemoteEngineControlClient.sendRequest path and the high-level ActorHandle.fetch path.

What is Good:

• Tests cover real behavior end-to-end. The second test goes from createClient through ActorHandle.fetch, validating gateway option threading in actor-handle.ts and actor-common.ts is wired correctly.
• No module-level mocking. Uses vi.stubGlobal for fetch, consistent with CLAUDE.md constraints against vi.mock and jest.mock.
• Consistent with existing test patterns. The normalizeRequest helper, fetchCalls array, and vi.stubGlobal setup mirror surrounding tests faithfully.

Issues and Suggestions:

1. URL assertion mismatch in the second test

The second test calls handle.fetch('/bypass', ...) which produces path /bypass, then asserts the outgoing URL is https://api.rivet.dev/request/bypass. Those paths do not match. The first test uses new Request('http://actor/request/bypass') which extracts /request/bypass as the path. The second test would produce https://api.rivet.dev/bypass, not /request/bypass. This assertion will fail as written.

2. fetchCalls length assertion may be incorrect in the second test

Confirm that ActorHandle.fetch with bypassConnectable + getForId does not emit any resolution preflights. disableMetadataLookup: true suppresses metadata fetches, but a resolution step might still occur and cause the toHaveLength(1) assertion to fail.

3. Missing negative assertion for HEADER_RIVET_TOKEN

Neither test asserts that HEADER_RIVET_TOKEN is absent. Since no token is provided, buildGuardHeaders should not set it. Adding a null assertion would document the intentional absence and guard against regression.

4. Slightly misleading test name

The second test is titled 'handle fetch forwards bypass connectable to browser request'. The word 'browser' is misleading in a Node/Vitest environment. Consider 'ActorHandle.fetch forwards bypassConnectable header'.

Summary: The intent and structure are correct and cover a real coverage gap. The primary concern is the URL path mismatch in the second test (point 1), which looks like a copy-paste oversight and will cause a test failure. Please verify the path before merging.