CI

Lint fails (formatter): two trivial dprint formatting issues in ext/websocket/01_websocket.js and the new ext/node/polyfills/internal/deps/undici/undici.js. tools/format.js will auto-fix. Per the contributing guide it should be run before pushing. Build/test jobs pending behind the lint failure.

Scope creep — ext/fetch change is unrelated

// ext/fetch/lib.rs:827-...
pub struct CreateHttpClientArgs {
   ca_certs: Vec<String>,
   ...
+  unsafely_ignore_certificate_errors: Option<Vec<String>>,
   ...

// in op_fetch_custom_client:
-      unsafely_ignore_certificate_errors: options
+      unsafely_ignore_certificate_errors: args
         .unsafely_ignore_certificate_errors
-        .clone(),
+        .or_else(|| options.unsafely_ignore_certificate_errors.clone()),

This is genuinely a fix — Deno.createHttpClient({ unsafelyIgnoreCertificateErrors: [...] }) was previously silently dropped at the Rust deserialization layer. Now the per-call arg wins, falling back to runtime. But this has nothing to do with the WebSocket/undici dispatcher work the PR title and summary describe.

It's a security-relevant behavior change to a public Deno API (Deno.createHttpClient) — it deserves its own PR with:

• Its own dedicated tests (the new behavior isn't covered by test-http2-allow-http1-upgrade-ws since that test only exercises the websocket path)
• Its own commit message so the change shows up in git log / release notes
• Standalone bisectability if a regression is found later

Either split it out, or add Deno.createHttpClient-level tests verifying both that the per-call setting is now honored, and that an empty [] allowlist disables verification globally for that client (which is what connect.rejectUnauthorized: false lowers to in the new EnvHttpProxyAgent).

EnvHttpProxyAgent is a misleading name

The class as written is not an env-driven proxy agent — it ignores HTTP_PROXY / HTTPS_PROXY / NO_PROXY entirely, which is the entire purpose of the upstream undici class of the same name:

class EnvHttpProxyAgent {
  #client;
  constructor(options = { __proto__: null }) {
    const connect = options.connect ?? { __proto__: null };
    const clientOptions = { __proto__: null };
    if (connect.rejectUnauthorized === false) {
      clientOptions.unsafelyIgnoreCertificateErrors = [];
    }
    if (connect.ca !== undefined) {
      clientOptions.caCerts = Array.isArray(connect.ca) ? connect.ca : [connect.ca];
    }
    this.#client = createHttpClient(clientOptions);
  }
  ...
}

What it actually does: wraps a Deno HttpClient configured with TLS options. The functionality is closer to undici's Agent than EnvHttpProxyAgent. Two concerns:

1. Future user code that does new undici.EnvHttpProxyAgent() expecting proxy behavior (this is what the upstream class is for) will silently miss proxy resolution — there's no warning, no error.
2. Almost every Node test that touches undici uses Agent, not EnvHttpProxyAgent — new undici.Agent({ connect: { rejectUnauthorized: false } }) is the canonical pattern. Without an Agent export, those tests still won't pass.

Suggested fixes:

• Either rename to Agent (matches the canonical undici API) and export EnvHttpProxyAgent only as an alias once real proxy handling is implemented.
• Or add a TODO and a console.warn when proxy env vars are set and ignored, so users get a signal.

globalDispatcher is set but never read

let globalDispatcher;
function getGlobalDispatcher() { return globalDispatcher; }
function setGlobalDispatcher(dispatcher) { globalDispatcher = dispatcher; }

// ext/websocket/01_websocket.js
if (clientRid === null && initOrProtocols.dispatcher !== undefined) {
  clientRid = initOrProtocols.dispatcher?.client?.[internalRidSymbol] ?? null;
}

The WebSocket code only consults initOrProtocols.dispatcher (per-call). It never falls back to getGlobalDispatcher(). So setGlobalDispatcher(new EnvHttpProxyAgent({...})) followed by new WebSocket(url) (without dispatcher) silently ignores the global setting — different from Node's undici, where setGlobalDispatcher is the dominant pattern in tests.

If this PR's only goal is to enroll one specific test that passes dispatcher inline, fine — but the asymmetry should be a comment, and setGlobalDispatcher/getGlobalDispatcher should be either removed (since they are inert) or wired up.

Minor

connect.ca is assumed to be string-shaped

clientOptions.caCerts = Array.isArray(connect.ca) ? connect.ca : [connect.ca];

Real undici accepts string | Buffer | Array<string | Buffer>. Deno's caCerts field is a Vec<String>. If a user follows the standard Node pattern and passes a Buffer (Uint8Array) of PEM bytes, this will hand a Uint8Array to a code path expecting a string. Worth either decoding (new TextDecoder().decode(ca)) or erroring loudly.

Doc / TODO comment update

-    // TODO(bartlomieju): depends on 'internal/deps/undici/undici' internal module
+    // TODO(bartlomieju): depends on `setGlobalDispatcher()` support in fetch
     // "parallel/test-inspector-network-fetch.js": {},

This rewords the TODO but doesn't actually unblock that test (which depends on setGlobalDispatcher for fetch, not just for WebSocket — and the polyfill here doesn't wire it through to fetch). Worth either implementing the fetch wire-up in this PR, or dropping the TODO update entirely.

Verdict

REQUEST_CHANGES primarily on (1) lint failing and (2) the unrelated ext/fetch change being bundled in. The architectural concerns (EnvHttpProxyAgent naming, globalDispatcher being inert) are non-blocking but worth a response.

cc @bartlomieju on the user-facing Deno.createHttpClient semantic change — even though it's a clear fix, it deserves a maintainer's eye on its own merits separate from this enrollment PR.

Big improvement — the scope creep is reverted, the design now lives where it should, and setGlobalDispatcher is no longer inert.

Resolved from prior review

Scope creep reverted ✓

ext/fetch/lib.rs is back to main:

-      unsafely_ignore_certificate_errors: args
+      unsafely_ignore_certificate_errors: options
         .unsafely_ignore_certificate_errors
-        .or_else(|| options.unsafely_ignore_certificate_errors.clone()),
+        .clone(),

Deno.createHttpClient's public-API behavior is unchanged. ✓

TLS overrides scoped to WebSocket ✓

The new design lives in ext/websocket/lib.rs: op_ws_create gets two new parameters (ca_certs, unsafely_ignore_certificate_errors), and the new create_client_from_websocket_options builds a fresh client with the overrides only when the caller asks for them via dispatcher options. Clean separation — fetch's policy is untouched, WS gets a one-shot client per connection when the user opts in via dispatcher.

Agent is now the canonical name ✓

class Agent exported, with EnvHttpProxyAgent = Agent aliased for naming-compat. The bulk of usage (Node tests, undici docs) uses Agent, so this is the right primary name. The EnvHttpProxyAgent alias is still semantically misleading (no proxy env handling) but at least it's no longer the only export.

globalDispatcher no longer inert ✓

01_websocket.js:

let dispatcher = initOrProtocols.dispatcher;
if (
  dispatcher === undefined &&
  internals[kNodeUndiciGlobalDispatcher] !== undefined
) {
  dispatcher = internals[kNodeUndiciGlobalDispatcher];
}

setGlobalDispatcher(new Agent({...})) followed by new WebSocket(url) (without dispatcher) now picks up the global. Matches Node's undici semantics. ✓

connect.ca Buffer/ArrayBuffer support ✓

normalizeCaCerts now accepts string, ArrayBufferView (Uint8Array etc.), and AnyArrayBuffer. Throws TypeError on anything else. Matches what real undici accepts.

One blocker — lint failing

Three CI shards (linux/macos/windows) fail on prefer-primordials:

error[prefer-primordials]: Don't use the global intrinsic
  --> ext/node/polyfills/internal/deps/undici/undici.js:24:10

The line is:

return certs.map((cert) => {

.map(...) is the prototype-method form, which the lint rule rejects in polyfills. Replace with ArrayPrototypeMap(certs, (cert) => {...}) and pull ArrayPrototypeMap into the destructured primordials at the top of the file. Same fix that the other primordials-safe helpers in this file already use.

Smaller things still worth a look

[ca] array literal in normalizeCaCerts

const certs = ArrayIsArray(ca) ? ca : [ca];

Same primordial-strict rule may flag this in stricter linters — a single-element array constructor would be [ca] in plain JS, but if the lint rule has a "no array literal" mode, it'd want new SafeArrayIterator([ca]) or similar. Looking at the failure listed (line 24:10 — the .map), [ca] is line 23 and lint passed it, so probably fine. Just flagging for symmetry.

EnvHttpProxyAgent = Agent alias

It's better than the original (where EnvHttpProxyAgent was the only name and it didn't match what the class did), but it's still false advertising — EnvHttpProxyAgent in real undici reads HTTP_PROXY / HTTPS_PROXY / NO_PROXY. Anyone copy-pasting Node code that relies on those env vars will get silent no-op behavior.

Two options:

• Remove the alias. Force users to import Agent directly. Code that explicitly wanted EnvHttpProxyAgent will fail loudly with a clear undefined import error.
• Keep the alias and add a one-time console.warn on construction if any of HTTP_PROXY / HTTPS_PROXY / NO_PROXY are set (since that's when the false advertising actually matters).

Non-blocking — the existing aliasing isn't strictly worse than today's main.

internals[kNodeUndiciGlobalDispatcher] realm scoping

Each Deno realm has its own internals object, so workers get independent global dispatchers. ✓ Confirmed by the internals import path. Just noting since the prior review flagged this concern — now resolved.

CI

3 lint fail (the .map issue), 128 pass, 2 pending, 2 skipping. Lint is the only blocker — once that one line changes to ArrayPrototypeMap, the rest of the suite should be in good shape.

Holding to REQUEST_CHANGES on the lint failure. Substance is otherwise clean and the design refactor is a real improvement over the v1 approach.

Substance walk:

• New internal/deps/undici/undici.js — minimal Agent/EnvHttpProxyAgent/get-set-globalDispatcher polyfill that stores per-Agent TLS config on a SymbolFor("Deno.internal.node.undici.dispatcherOptions")-keyed property and the global dispatcher on internals[kNodeUndiciGlobalDispatcher]. normalizeCaCerts correctly handles string / ArrayBufferView / ArrayBuffer ca inputs, matching undici's connect.ca shape. ✓
• WebSocket constructor at 01_websocket.js:217–268 accepts a new dispatcher field in the init dictionary, falls back to internals[kNodeUndiciGlobalDispatcher] when not specified, then extracts caCerts + unsafelyIgnoreCertificateErrors from the dispatcher's options bag. The fall-through order (per-call dispatcher → setGlobalDispatcher value) matches Node/undici semantics.
• op_ws_create extended with ca_certs: Option<Vec<String>> + unsafely_ignore_certificate_errors: bool. When either is set and client_rid isn't, the op builds a per-call HTTP client via create_client_from_websocket_options rather than reusing the cached one. The HTTP client construction copies proxy/dns_resolver/client_cert_chain_and_key/etc. from the existing FetchOptions so dispatcher-specific overrides don't lose other config. ✓
• Test enable parallel/test-http2-allow-http1-upgrade-ws.js exercises this new path. CI 133/136 success.

Security: per-dispatcher unsafelyIgnoreCertificateErrors

The one thing worth surfacing for a maintainer's eye before this lands: the new path lets runtime JS code turn off TLS certificate validation for WebSocket connections by setting setGlobalDispatcher(new Agent({ connect: { rejectUnauthorized: false } })). That's the existing Node/undici contract, so this is Node-parity, not a new policy choice — but in Deno's permission model, --unsafely-ignore-certificate-errors is a CLI-time opt-in, and a malicious or misconfigured library that imports node:undici can now silently bypass it for WebSocket calls without the user's CLI consent.

The same pattern already exists for Deno.createHttpClient({ unsafelyIgnoreCertificateErrors: true }) (which similarly bypasses the CLI gate at runtime), so this isn't a regression — it's extending an existing permissive pattern to one more surface. Worth being explicit about that in the changelog so users/security auditors aren't surprised. cc @bartlomieju on whether the runtime dispatcher path warrants any additional gating, since the WebSocket vector is more commonly long-lived than one-shot fetch and a MITM there has more impact.

Smaller observations

• new TextDecoder() per cert in normalizeCaCerts: cached at module scope (or pulled from primordials) would be slightly tidier. Not a perf bottleneck since cert lists are tiny.
• internals[kNodeUndiciGlobalDispatcher] = dispatcher in setGlobalDispatcher overwrites without an unset path; passing undefined to clear isn't supported. Probably matches Node where you'd construct a fresh Agent rather than clearing.
• EnvHttpProxyAgent = Agent is a stub alias — the env-proxy logic isn't actually implemented. Worth a notImplemented or at least a comment so users who specifically need env-proxy support don't silently get plain Agent behavior.

LGTM