fix(anthropic): wire buildModelFetch into the github-copilot createClient branch#100550
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review: needs maintainer review before merge. Reviewed July 6, 2026, 11:34 PM ET / 03:34 UTC. Summary PR surface: Source +1, Tests +211. Total +212 across 3 files. Reproducibility: yes. Current main's GitHub Copilot Anthropic constructor lacks a custom fetch, and the PR includes a real-SDK loopback negative control showing the host fetch is not invoked without the new line. Review metrics: 1 noteworthy metric.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review detailsBest possible solution: Land this narrow wiring after maintainers accept guarded-fetch parity for GitHub Copilot Anthropic, or add an explicit compatibility/recovery path before making the guarded fetch default. Do we have a high-confidence way to reproduce the issue? Yes. Current main's GitHub Copilot Anthropic constructor lacks a custom fetch, and the PR includes a real-SDK loopback negative control showing the host fetch is not invoked without the new line. Is this the best way to solve the issue? Yes for the code shape: using the existing host AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 118e5bd76223. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1, Tests +211. Total +212 across 3 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (5 earlier review cycles)
|
bc774a7 to
b64c511
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
b64c511 to
bd47e66
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
bd47e66 to
cc6e918
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
…ient branch Reopens openclaw#98014 after packages/ai extraction (openclaw#99059) moved anthropic.ts out of src/llm/providers. Adds a single fetch line to the github-copilot createClient branch, parallel to the cloudflare branch wired by openclaw#98003 and the same-shape api-key/oauth/foundry siblings landing in this series. Bytes-cap verification is owned by buildModelFetch itself; this PR proves the constructor wiring via inline test through production streamAnthropic.
cc6e918 to
2e6e805
Compare
|
Landed the complete Anthropic SDK transport fix in #101357 ( Your contribution is preserved in the landed commit with |
What Problem This Solves
The
github-copilotcreateClient branch ofpackages/ai/src/providers/anthropic.ts(lines 1108-1127) currently constructsnew Anthropic({...})without afetch:option. The Anthropic SDK then falls back to the platform-defaultglobalThis.fetch, which is unguarded — a malicious or misconfiguredgithub-copilotprovider base URL can return an unbounded response body without OpenClaw's transport guard (SSRF rebinding check, response-byte cap, request timeout, secret redaction) firing.#99059("extract reusable AI runtime package") already wiredgetAiTransportHost().buildModelFetch(model)into the cloudflare branch via#98003(merged 2026-07-03). This PR closes the symmetric gap on the github-copilot branch so 4 of 5 Anthropic createClient branches share the same host transport policy.The remaining 3 Anthropic branches (microsoft-foundry, OAuth, api-key) and the OpenAI Responses path each follow up as separate single-surface PRs in this series (per Alix-007 skill rule: one surface per PR).
Changes
packages/ai/src/providers/anthropic.ts:1124— addfetch: getAiTransportHost().buildModelFetch(model),inside thenew Anthropic({...})call in the github-copilot branch. Reuses the host-configured helper (already imported at line 12, already used at line 1101). No new helper.packages/ai/src/providers/anthropic.test.ts:152— inlineit("wires host buildModelFetch into the github-copilot Anthropic client")that captures the constructor args viavi.mock("@anthropic-ai/sdk")and assertsconfig.fetch === hostFetch. Mirrors the existing cloudflare test (line 62).packages/ai/src/providers/anthropic-copilot-fetch-loopback.test.ts(new) — real-SDK loopback proof. Starts ahttp.createServeron127.0.0.1:random, configuresgetAiTransportHost().buildModelFetchto return a recording fetch that delegates toglobalThis.fetch, drivesstreamAnthropic, and asserts the SDK actually invoked the wired fetch on the real network path. The constructor-capture mock cannot satisfy this — it never lets the SDK call fetch.Real behavior proof
This PR provides two complementary proofs:
Proof A — Mock constructor-capture (lightweight, existing pattern)
anthropic.test.tsusesvi.mock("@anthropic-ai/sdk")to capture the constructor config. Without thefetch:line:With the
fetch:line:Proof B — Real-SDK loopback network path (this PR's new proof)
anthropic-copilot-fetch-loopback.test.tsdrives the real@anthropic-ai/sdkagainst anhttp.createServerloopback. The server records every incoming request. The host fetch records every invocation and delegates toglobalThis.fetch. The test asserts:recordingFetchCalls.length === 1— the wired fetch was actually invoked by the SDKrecordingFetchCalls[0].url === http://127.0.0.1:<port>/v1/messagesrecordingFetchCalls[0].init.method === "POST"POST /v1/messageswithAuthorization: Bearer copilot-test-tokenX-Initiator: userandOpenai-Intent: conversation-edits(provesbuildCopilotDynamicHeadersruns)model: claude-sonnet-4-6andstream: truestreamAnthropicparsed the SSE response into ≥ 5AssistantMessageEvents (start,text_start,text_delta,text_end,done)Negative control — Proof B is not a tautology
git checkout HEAD~1 -- packages/ai/src/providers/anthropic.tsremoves the newfetch:line, then re-runs the loopback test:Without the
fetch:line, the SDK falls back to defaultglobalThis.fetch(NOT our wired fetch), sorecordingFetchCallsstays empty (length 0). The assertion fires only when the source change is present. The test reverses pass/fail cleanly with the source line.Evidence
Evidence A — Loopback test terminal output (real-SDK network path)
The loopback test (
packages/ai/src/providers/anthropic-copilot-fetch-loopback.test.ts) drives the real@anthropic-ai/[email protected]throughstreamAnthropicagainst anhttp.createServerloopback. Running the test with verbose reporter prints the wire data captured below. The PR body shows the real wire headers, body, and parsed events from the real SDK call:The wire data proves:
recordingFetchexactly once (not defaultglobalThis.fetch)POST /v1/messageswithAuthorization: Bearer copilot-test-token(proves theauthToken: apiKeyline in anthropic.ts:1111 routes through the github-copilot branch correctly)X-Initiator: userandOpenai-Intent: conversation-editsare present (provesbuildCopilotDynamicHeaders()ran)model=claude-sonnet-4-6,stream=true, ephemeral cache control on user messagestreamAnthropicparsed the SSE response into 5 normalizedAssistantMessageEvents — full pipeline (real SDK → host-fetch → HTTP loopback → SSE-parser) works end-to-endEvidence B — vitest output (committed test pass/fail counts)
Evidence C — Negative control (test reverses pass/fail with source)
git checkout HEAD~1 -- packages/ai/src/providers/anthropic.tsremoves the newfetch:line, then re-runs the loopback test. Without the wired fetch, the SDK uses defaultglobalThis.fetchand bypasses ourrecordingFetch:Restoring the source line returns the test to passing. The test is not a tautology — it reverses cleanly with the source change.
Evidence D — Mock constructor-capture (lightweight, kept for completeness)
anthropic.test.ts:152adds the canonical mock-based test (matching the cloudflare branch test at line 62). Withoutfetch:line,config.fetchisundefined; with the line,config.fetch === hostFetch(function-identity, stronger thanexpect.any(Function)):buildModelFetchreturns the platform's policy-guarded fetch) apply to github-copilot requests. Evidence A proves this on a real network path with actual wire data; Evidence D covers the constructor argument shape.2b48e98148(latest openclaw/main after rebase)fetch:line, Evidence A shows the real SDK invokes our wiredrecordingFetchexactly once withPOST /v1/messages, the loopback server receivesAuthorization: Bearer copilot-test-token+ github-copilot dynamic headers, andstreamAnthropicparses the SSE response into 5 normalizedAssistantMessageEvents. Without the source line, Evidence C shows the assertion fires (recordingFetchCalls.length === 0) because the SDK uses defaultglobalThis.fetchand bypasses our wired helper.buildGuardedModelFetchitself; proven by its own tests insrc/agents/provider-transport-fetch.test.ts. This PR proves the wiring reaches the guard; the guard's own tests prove the guard's behavior.Compatibility risk
This PR intentionally changes transport behavior for existing custom
github-copilotAnthropic base URLs. Per ClawSweeper's review of #100550, the change may cause fail-closed behavior under OpenClaw's host fetch guard:github-copilotbase URL whose hostname fails the platform's SSRF rebinding check will now error instead of completing.model.headersthat match the host's secret patterns will be redacted before the wire request.This is intentional hardening consistent with the cloudflare branch (wired by #98003, merged 2026-07-03) and the managed Anthropic API branch (in main). It restores symmetry: every Anthropic createClient branch now applies the host transport guard. If any existing custom Copilot endpoint was relying on unguarded behavior, that endpoint needs to be migrated to comply with the guard policy (or the user opts out of the
github-copilotAnthropic surface).Maintainer decision needed (per ClawSweeper's routing to steipete): accept the fail-closed behavior on this branch as the desired hardening, OR pause this branch pending an explicit compatibility/migration plan for custom Copilot endpoints.
Risk checklist
fetchoption — Anthropic SDK contract unchanged. ExistingapiKey/authToken/defaultHeadersassertions still pass.new Anthropic({...})callnode scripts/run-tsgo.mjsclean onpackages/ai(pre-existingui/src/pages/...errors are unrelated to this PR)node scripts/run-oxlint-shards.mjs --threads=4clean on touched filesOut of scope (separate follow-ups)
buildModelFetchitself, not by this wiring PR