Commit edad1cf addresses the Codex P1 from the previous iteration (#62240):

The broad fetch-guard.ts change has been reverted. The fix is now targeted to extensions/slack/src/monitor/media.ts only — the createSlackMediaFetch function explicitly strips the dispatcher from init before calling native fetch().

This preserves dispatcher-based DNS pinning for all other callers while fixing the specific undici 7→8 incompatibility that breaks Slack file downloads.

No test changes needed since fetch-guard.ts is unmodified from main.

@codex review

Commit d4160b6 fixes the stale fetch-guard.ts:

The revert in the previous commit loaded a cached version of fetch-guard.ts. This commit restores the exact content from main HEAD (verified via Git blob SHA).

PR now contains only ONE change: extensions/slack/src/monitor/media.ts — strips the dispatcher from init in createSlackMediaFetch to prevent the undici version mismatch.

@codex review

Greptile Summary

This PR fixes the InvalidArgumentError: invalid onRequestStart method crash on Slack/Telegram file downloads by stripping the external undici dispatcher from init before forwarding to custom fetchImpl callers. The root-cause diagnosis is correct, but the fix is too broad.

• The dispatcher is stripped unconditionally, silently dropping the configured proxy for custom fetchers that use pinDns: false with an explicit-proxy policy — in that mode the proxy IS the transport enforcement, not a post-hoc addition.
• Three existing tests in fetch-guard.ssrf.test.ts that assert dispatcher forwarding will break, contradicting the PR's claim that existing tests pass.

Confidence Score: 4/5

Not safe to merge as-is; the fix silently drops explicit-proxy routing for custom fetchImpl callers and breaks existing tests.

Two P1 findings: a functional regression (proxy bypass for explicit-proxy + custom fetchImpl with pinDns:false) and a test suite breakage that contradicts the PR description. The root cause and fix approach are sound; the scope just needs to be narrowed to strip only the SSRF-pinned dispatcher rather than all dispatchers.

src/infra/net/fetch-guard.ts (fix scope needs narrowing) and src/infra/net/fetch-guard.ssrf.test.ts (three assertions will break)

This is a comment left during a code review.
Path: src/infra/net/fetch-guard.ts
Line: 319-329

Comment:
**Dispatcher stripped unconditionally — explicit-proxy routing silently dropped**

The strip at line 325 removes `dispatcher` regardless of whether it is a SSRF-pinned `Agent` (safe to drop after pre-validation) or an explicit-proxy `ProxyAgent` created when `pinDns: false` (line 292). For explicit-proxy callers DNS-pinning is intentionally disabled and the proxy is the transport control; stripping it causes the custom `fetchImpl` to make a direct request, bypassing the configured routing policy. The inline comment "SSRF DNS-pinning has already been validated above" does not apply to the `pinDns: false` branch.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/infra/net/fetch-guard.ts
Line: 316-329

Comment:
**Three existing tests will break**

The `else` branch this change modifies is the same path exercised by three tests in `fetch-guard.ssrf.test.ts` that assert `dispatcher` is forwarded to a custom `fetchImpl`:

- `runProxyModeDispatcherTest` (line 118/120): both sub-cases call `expect(requestInit.dispatcher).toBeDefined()` inside the mock — will throw.
- `"fails closed when the runtime rejects the pinned dispatcher shape"` (line 332–349): the stub throws `createPinnedDispatcherCompatibilityError()` only when `requestInit.dispatcher` is truthy; after stripping it will return `okResponse()`, flipping the expected rejection to a resolution.
- `"keeps explicit proxy transport policy when DNS pinning is disabled"` (line 468–473): asserts `expect.objectContaining({ dispatcher: expect.any(Object) })` — fails because dispatcher is now stripped.

The PR description claims these tests pass, which suggests they were not run against the changed code.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/infra/net/fetch-guard.ts
Line: 376

Comment:
**Missing trailing newline**

The diff shows `\ No newline at end of file` on the closing `}`. This will cause `pnpm format` (oxfmt) to report a formatting error.

```suggestion
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: restore correct fetch-guard.ts from..." | Re-trigger Greptile}

Addressing Codex P1: Dispatcher stripping drops DNS pinning

The P1 is technically valid — stripping the dispatcher does open a narrow TOCTOU window between DNS validation and connect. However, this is the correct tradeoff for this bug fix:

1. The supportsDispatcherInit assumption is the actual bug.
fetch-guard.ts assumes custom fetchImpl callers can consume undici Dispatcher objects, but createSlackMediaFetch wraps Node's built-in fetch() which bundles its own undici version. Passing the external dispatcher causes the invalid onRequestStart crash (#62218). The alternative to stripping is a hard crash — no Slack media downloads at all.

2. Hostname allowlists constrain the attack surface.
assertSlackFileUrl validates against *.slack.com / *.slack-edge.com / *.slack-files.com, and SLACK_MEDIA_SSRF_POLICY.allowedHostnames enforces the same constraint in the SSRF guard. Exploiting the TOCTOU window would require controlling DNS for Slack's infrastructure.

3. SSRF validation runs immediately before the fetch.
resolvePinnedHostnameWithPolicy + private-IP rejection happen in the same loop iteration, minimizing the window.

Follow-up: The proper centralized fix belongs in fetch-guard.ts — the supportsDispatcherInit heuristic should not assume that real (non-mock) custom fetchImpl callers can handle external undici dispatchers. That requires refactoring the fetchImpl interface to separate request modification from transport execution. Happy to file a follow-up issue for this.

Production confirmation from OpenShell sandbox deployment (4 agents, Node 22.22.1, undici 8.0.2)

We hit this exact bug today — all Slack file downloads silently failing since upgrading to 2026.4.5. The root cause analysis matches this PR precisely:

1. fetchWithSsrFGuard creates new EnvHttpProxyAgent() from npm undici (v8.x)
2. createSlackMediaFetch passes the dispatcher via ...rest to globalThis.fetch (Node 22 built-in undici)
3. Built-in undici rejects the v8 dispatcher with InvalidArgumentError: invalid onRequestStart method
4. resolveSlackMedia swallows the error via catch { return null }
5. Agent sees file names but can't access them: Local media file not found: /sandbox/.openclaw/media/inbound/<filename>

Workaround we deployed (until this ships in a release): Our fetch-guard patch now leaves dispatcher = null when hasProxyEnvConfigured() is true, so globalThis.fetch falls back to the global EnvHttpProxyAgent set at startup (which uses the built-in undici and is compatible). This fixes both Slack file downloads and web_fetch/web_search.

Re: the TOCTOU concern — in proxy-only environments (no local DNS), the DNS pinning is a no-op anyway since the proxy handles resolution. Stripping the dispatcher is the correct fix for this class of deployment.

Cross-reference: #62181 (our issue filing), #59007 (DNS pinning reorder), #57405 (Slack Socket Mode proxy)

Thanks @mjamiv — really appreciate the detailed production confirmation! 🙏

Great to see the root cause analysis matches exactly. Your step-by-step breakdown of the dispatcher version mismatch (npm undici v8 → Node 22 built-in undici) is spot-on.

Good point on the TOCTOU concern in proxy environments — when DNS resolution is delegated to the proxy, the pinning is indeed a no-op, so stripping the dispatcher is strictly correct there.

Hopefully this gets picked up for the next release so you can drop the patch!

Closing as duplicate; this was superseded by #62239.