ReviewOS

also looking at this

stacks/rpx

fix(test): stop a leaked spawn mock hanging the gateway CLI tests (#2267), pin forwarded headers (#2265)

#2268
Merged chris wants to merge fix/2265-2267-proxy-header-and-cli-coverage into main
4 files +57 -4
10 days ago

Test-only, two commits, one per issue.

#2267 — root cause found: a leaked spawn spy

Not a Linux bug, and not a bug in the CLI.

process-manager.test.ts installs spyOn(childProcess, 'spawn') returning a stub — pid: 12345, no-op on / once / stdout.on / stderr.on — and its afterEach said:

// No cleanup needed - Bun's spyOn handles cleanup automatically

It does not. The spy sits on the shared node:child_process module object, so it stayed installed for every file that ran after this one in the same bun test process. Anything spawning a subprocess afterwards got the stub back: no child ever started, so nothing was written to stdout or stderr and no exit event ever fired — exactly the reported symptom of a live child, zero bytes, no spawn error. The pid in the CI diagnostics was literally this file's 12345.

Why it looked platform-specific: bun test walks files in a deterministic but platform-dependent order. On the Linux runners process-manager.test.ts landed before gateway.test.ts with no mock.restore() in between; on macOS a file that does call mock.restore() happened to fall between the two.

Reproduce on any platform by forcing the order:

bun test packages/rpx/test/process-manager.test.ts packages/rpx/test/gateway.test.ts

45s timeout on the gateway CLI case before this change; passes after. Same result in a Linux container, forced order and full suite.

The fix is mock.restore() in that afterEach, so the describe.skipIf goes and both gateway CLI cases run on every platform again.

#2265 — forwarded headers

Not reproducible on main: all of x-forwarded-host / -for / -proto and x-real-ip reach the upstream on all three transports (pooled, fetch fallback, websocket), verified against a live echo upstream through createProxyFetchHandler.

Coverage had a real gap though — x-forwarded-proto was asserted nowhere, and nothing covered the handler end to end, only the pool serializer beneath it. Both transports are now pinned at the handler boundary, plus the missing proto assertion in the pool test. Confirmed as genuine guards by reintroducing the reported bug in serializeRequest: 8 cases fail, including the new one.

Verification

  • macOS full suite: 518 pass / 0 fail
  • Linux (oven/bun:1, bun 1.4.0): forced poisoning order 19/0; full suite — both gateway CLI cases pass, only the pre-existing sudo-dependent utils failures that CI's arm64 job already excludes
  • pickier clean, tsc --noEmit clean

Closes #2267, refs #2265

🤖 Generated with Claude Code

4 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.