test(runtime): prove the proxied HTTP forward path end-to-end

[ggbdpq]

Summary

This PR was reshaped after review. The original version pinned
ProxyAgent back to CONNECT-first tunneling for plain-HTTP targets
(proxyTunnel: true); that direction is withdrawn here and replaced by
a test-only hardening of the existing forwarding behavior. No product
code changes remain: proxy-dispatcher.ts is back to upstream/main
unchanged, and the CONNECT-asserting regression test is gone.

Why the CONNECT pinning was withdrawn

Two reviews pulled in opposite directions, so each claim was traced
back to falsifiable evidence:

Claim	Evidence	Verdict
hqhq1025's P2 in #5805: plain-HTTP targets hang for ~8s and lose cancellation	The hang does not reproduce on undici 8.11.2: local scoped-fetch-transport baseline is 42/42 green and recent main CI runs are green. Cancellation is already covered at the connect level: the forward request still goes through the dispatcher factory's connect wrapper (withSocketCancellation / buildAbortableConnector); onRequestUpgrade only fires for TLS tunnel upgrades, so the "cancellation contract broken" argument never applied to the forward path.	Hang not reproducible; cancellation already covered
Astro-Han's P2 in #6006: the eval fixture breaks	The eval fixture (packages/eval/src/__tests__/maka-initialization.test.ts) explicitly asserts the forward contract: it serves absolute-form URLs and answers CONNECT with 403 ("Unexpected CONNECT"). Forwarding has been the behavior on main since undici 8 (undici#1247), alive for ~3.5 months with green CI.	Forwarding is the established contract; pinning CONNECT turns CI red
Astro-Han's P3 in #6006: real-world proxies	Squid's default config rejects CONNECT to non-SSL ports, so http:// providers behind a LAN proxy (Ollama, vLLM, ...) would go from working to 403 under forced tunneling.	Forced tunneling would be a real-world regression

The one finding from the reviews that held up: the runtime success-path
test previously used a fake proxy answering a fixed 200 to anything, so
the proxied HTTP path passed without ever proving a relay
("accidentally green"). This PR closes exactly that gap.

What this PR does instead

startForwardProxy replaces the self-answering fake for the plain-HTTP
success case, and it behaves like a real forward proxy:

• it records the request line and headers arriving at the proxy and
  asserts the absolute-form request line
  (GET http://127.0.0.1:<port>/models HTTP/1.1);
• it relays the raw connection to a live target server over a TCP pipe
  and never answers on its own, so the client response can only have
  come from the target;
• it asserts proxy-authorization rides the forward request when
  credentials are configured (matching the eval fixture's expectation).

If maintainers' actual intent is tunnel semantics for plain-HTTP
targets, reverting this test's direction is the correct and cheap move;
this PR deliberately no longer forces that decision in product code.

Refs #6005

Verification

Check	Result
packages/runtime: tsc -p tsconfig.json	pass
node --test dist/network/__tests__/scoped-fetch-transport.test.js	42/42 pass
node --test "dist/network/__tests__/*.test.js" (adjacent suite)	54/54 pass
node --test "dist/bots/__tests__/*.test.js" (adjacent suite)	109/109 pass
npx biome check on the changed file	pass (formatted to satisfy the formatter)
npm run check:asf-headers	pass (4308 covered / 253 excluded)
Test self-validity: temporarily self-answer from the proxy (mimicking the old fixture)	suite fails 41/42 on "the proxy must relay the forward request to the real target"; pipe restored, 42/42
eval package	untouched and not run (no product change; CI verifies it)

AI use

Select exactly one:

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: GLM-5.3-Flash (ZCode) authored the analysis, the
test changes and this write-up; the human directed the adjudication
between the conflicting reviews. The commit carries the Generated-by
trailer.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Review of fac28309 (+78/-4, 2 files). The change sets proxyTunnel: true on the HTTP(S) ProxyAgent in buildProxyDispatcher so plain-HTTP targets are sent as CONNECT host:80 through the proxy instead of being forwarded in absolute form.

P2: this breaks an existing test contract, and CI test is red because of it. packages/eval/src/__tests__/maka-initialization.test.ts ("official Maka shim uses one Host for preflight and execution across retries and key rotation") runs a fixture proxy that expects plain-HTTP requests to http://provider.invalid to be forwarded with Proxy-Authorization. Its connect handler records Unexpected CONNECT and answers 403. On this head the run fails with Error: Unexpected CONNECT provider.invalid:80 (run 37790469660). Main's last five CI runs are green, so the PR causes this failure. Undici 8 has been on main since #1247, so forwarding is the current, tested behavior and not a recent regression. Changing it needs either that fixture updated along with a stated reason, or a narrower fix.

P3: CONNECT to port 80 is often refused by real proxies. Squid's stock config has http_access deny CONNECT !SSL_ports, and many corporate proxies do the same. With proxyTunnel: true, a user with an http:// provider base URL (local gateways, LAN Ollama/vLLM behind a proxy) gets a 403 where forwarding works today. If the goal is to keep the onRequestUpgrade socket cancellation, note that forwarded requests already go through factory, which wraps connect with withSocketCancellation/buildAbortableConnector. Abort coverage may already hold without forcing tunnels; a test showing that an abort during a forwarded request leaks would make the case for this change.

The new startTunnelProxy helper and the CONNECT assertion test look correct for what they check. There are no protocol files, so there is no epoch impact. The PR is MERGEABLE.

[ggbdpq]

@Astro-Han Thank you — both points are accepted, and the PR has been reshaped accordingly (new head 433b64dbe, test-only, proxy-dispatcher.ts restored to upstream/main).

This review and the earlier P2 in #5805 pulled in opposite directions, so each claim was traced back to falsifiable evidence before deciding:

1. The reported ~8s hang (P2 in ci(perf): retire the broken transcript data-plane benchmark #5805) does not reproduce on undici 8.11.2. Local baseline: scoped-fetch-transport 42/42 green, recent main CI runs green. That claim was the load-bearing justification for pinning CONNECT, so it falls.
2. Cancellation is already covered on the forward path — as this review notes, forwarded requests still go through the dispatcher factory's connect wrapper (withSocketCancellation / buildAbortableConnector); onRequestUpgrade only fires for TLS tunnel upgrades. No coverage was lost.
3. P2 here (eval fixture) is decisive: the eval fixture explicitly asserts the absolute-form forward and answers CONNECT with 403. Forwarding has been the tested contract on main since undici 8 (undici#1247), ~3.5 months green. Pinning CONNECT turned that suite red (run 37790469660).
4. P3 here (Squid http_access deny CONNECT !SSL_ports): forced tunneling would turn working http://-behind-proxy setups (LAN Ollama/vLLM) into 403s — a real-world regression.

The review's one durable finding — the old fake proxy answered a fixed 200 to anything, so the proxied HTTP success path was never actually proven — is exactly what the reshaped PR fixes. The new startForwardProxy records the absolute-form request line and proxy-authorization header, pipes the raw connection to a live target over TCP, and never self-answers; the success-path test now fails (41/42, "the proxy must relay the forward request to the real target") if the proxy self-answers instead of relaying. Details and the full verification table are in the PR description. If maintainers' intent is tunnel semantics for plain-HTTP targets, reverting this test's direction is the cheap, correct move — this PR no longer forces that decision in product code.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review: fac28309 -> 433b64db. Both heads sit on the same main base. The PR was reshaped, not rebased: the old commit is replaced by a single test-only commit (+83/-4, packages/runtime/src/network/__tests__/scoped-fetch-transport.test.ts).

Prior findings

• P2 (eval maka-initialization test red because proxyTunnel: true made plain-HTTP targets use CONNECT): fixed. proxy-dispatcher.ts is back to main and is no longer in the diff. Plain-HTTP targets keep the undici 8 forward behavior the eval fixture asserts. CI test is green on this head.
• P3 (Squid and similar proxies deny CONNECT to port 80): resolved by the same revert. Forced tunneling is gone.

New test. For the http case, startForwardProxy replaces the self-answering fake proxy. It records each request head and pipes raw bytes to a live target, so a response can only come from the target. It checks three things: the absolute-form request line, proxy-authorization built from the configured credentials, and targetHits === index + 1. If undici ever switched back to CONNECT, the target HTTP server would answer the piped CONNECT line with 400 and the test would fail. So this pins the forward contract from the runtime side as well as the eval side. The abort-listener check now runs against a real relayed connection rather than a fixed 200, which is what the test name claims. I found no defects.

Non-blocking nit: pending is declared per proxy, not per client connection. It works here because the 25 requests are sequential and each connection closes. Moving it inside the net.createServer callback would make the helper safe if it is ever reused with concurrent or keep-alive connections.

No protocol files are touched, so there is no epoch impact. The PR is MERGEABLE, and CI test passes on 433b64db.

No findings. This looks good to land.

[Astro-Han]

Approved at @Astro-Han's explicit request: a small, focused change with no blocking findings in our automated review of this exact head, and CI is green.