From 8280715b64c6ab066a4f2a8c7f9684ebb38bfff6 Mon Sep 17 00:00:00 2001 From: lc <30007232+LouisClt@users.noreply.github.com> Date: Thu, 24 Sep 2026 22:55:06 +0200 Subject: [PATCH 1/6] fix(code-index): route Bedrock embedder through the system proxy The proxy fix for the chat provider never reached the code-index embedder, which built its own BedrockRuntimeClient without a request handler. Behind a corporate proxy Node resolves DNS locally and indexing fails with ENOTFOUND. Co-Authored-By: Claude Opus 5 --- .../embedders/__tests__/bedrock.spec.ts | 52 +++++++++++++++++++ src/services/code-index/embedders/bedrock.ts | 15 ++++++ 2 files changed, 67 insertions(+) diff --git a/src/services/code-index/embedders/__tests__/bedrock.spec.ts b/src/services/code-index/embedders/__tests__/bedrock.spec.ts index dfa9544715..de09fc3d0f 100644 --- a/src/services/code-index/embedders/__tests__/bedrock.spec.ts +++ b/src/services/code-index/embedders/__tests__/bedrock.spec.ts @@ -1,8 +1,13 @@ import type { MockedFunction } from "vitest" import { BedrockRuntimeClient, InvokeModelCommand } from "@aws-sdk/client-bedrock-runtime" +import { NodeHttpHandler } from "@smithy/node-http-handler" +import { HttpProxyAgent } from "http-proxy-agent" +import { HttpsProxyAgent } from "https-proxy-agent" + import { BedrockEmbedder } from "../bedrock" import { MAX_ITEM_TOKENS, INITIAL_RETRY_DELAY_MS } from "../../constants" +import { getSystemProxyUrl } from "../../../../utils/networkProxy" import { clearAllMocks } from "../../../../test-utils/reset" @@ -26,6 +31,17 @@ vitest.mock("@aws-sdk/credential-providers", () => ({ fromIni: vitest.fn().mockReturnValue(Promise.resolve({})), })) +vitest.mock("../../../../utils/networkProxy", () => ({ + getSystemProxyUrl: vitest.fn().mockReturnValue(undefined), +})) +vitest.mock("@smithy/node-http-handler", () => ({ + NodeHttpHandler: vitest.fn().mockImplementation(function (options: unknown) { + return { options } + }), +})) +vitest.mock("http-proxy-agent", () => ({ HttpProxyAgent: vitest.fn() })) +vitest.mock("https-proxy-agent", () => ({ HttpsProxyAgent: vitest.fn() })) + // Mock TelemetryService vitest.mock("@roo-code/telemetry", () => ({ TelemetryService: { @@ -114,6 +130,42 @@ describe("BedrockEmbedder", () => { }), ) }) + + it("should tunnel through the system proxy when one is configured", () => { + // The embedder built in beforeEach already consumed these mocks. + vitest.mocked(getSystemProxyUrl).mockReturnValue("http://proxy.corp.local:3128") + vitest.mocked(NodeHttpHandler).mockClear() + + new BedrockEmbedder("us-east-1", "test-profile", "amazon.titan-embed-text-v2:0") + + expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128") + expect(HttpsProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128") + + // Both agents must reach the handler: a client that tunnels only https still + // resolves http DNS locally, which is the ENOTFOUND this fix is about. + expect(NodeHttpHandler).toHaveBeenCalledOnce() + // The constructor accepts options or a provider function; only the object form is used here. + const handlerArg = vitest.mocked(NodeHttpHandler).mock.calls[0][0] + const handlerOptions = typeof handlerArg === "function" ? undefined : handlerArg + expect(handlerOptions?.httpAgent).toBe(vitest.mocked(HttpProxyAgent).mock.instances[0]) + expect(handlerOptions?.httpsAgent).toBe(vitest.mocked(HttpsProxyAgent).mock.instances[0]) + + // ...and the handler must reach the client. + const clientConfig = vitest.mocked(BedrockRuntimeClient).mock.calls.at(-1)?.[0] + expect(clientConfig?.requestHandler).toBe(vitest.mocked(NodeHttpHandler).mock.instances[0]) + }) + + it("should not install a request handler when no proxy is configured", () => { + vitest.mocked(getSystemProxyUrl).mockReturnValue(undefined) + vitest.mocked(NodeHttpHandler).mockClear() + + new BedrockEmbedder("us-east-1", "test-profile", "amazon.titan-embed-text-v2:0") + + expect(NodeHttpHandler).not.toHaveBeenCalled() + expect(BedrockRuntimeClient).toHaveBeenLastCalledWith( + expect.not.objectContaining({ requestHandler: expect.anything() }), + ) + }) }) describe("createEmbeddings", () => { diff --git a/src/services/code-index/embedders/bedrock.ts b/src/services/code-index/embedders/bedrock.ts index 833d4bf8f8..ee590f091a 100644 --- a/src/services/code-index/embedders/bedrock.ts +++ b/src/services/code-index/embedders/bedrock.ts @@ -1,5 +1,9 @@ import { BedrockRuntimeClient, InvokeModelCommand, InvokeModelCommandInput } from "@aws-sdk/client-bedrock-runtime" import { fromIni, fromNodeProviderChain } from "@aws-sdk/credential-providers" +import { NodeHttpHandler } from "@smithy/node-http-handler" +import { HttpProxyAgent } from "http-proxy-agent" +import { HttpsProxyAgent } from "https-proxy-agent" +import { getSystemProxyUrl } from "../../../utils/networkProxy" import { IEmbedder, EmbeddingResponse, EmbedderInfo } from "../interfaces" import { MAX_BATCH_TOKENS, @@ -40,10 +44,21 @@ export class BedrockEmbedder implements IEmbedder { // If profile is specified, use it; otherwise use default credential chain const credentials = this.profile ? fromIni({ profile: this.profile }) : fromNodeProviderChain() + // Behind a corporate proxy, Node resolves DNS locally before tunneling and Bedrock + // endpoints fail with ENOTFOUND. The proxy agents use CONNECT so the proxy resolves + // the hostname instead, matching the chat provider in src/api/providers/bedrock.ts. + const proxyUrl = getSystemProxyUrl() + this.bedrockClient = new BedrockRuntimeClient({ userAgentAppId: `ZooCode#${Package.version}`, region: this.region, credentials, + ...(proxyUrl && { + requestHandler: new NodeHttpHandler({ + httpAgent: new HttpProxyAgent(proxyUrl), + httpsAgent: new HttpsProxyAgent(proxyUrl), + }), + }), }) this.defaultModelId = modelId || getDefaultModelId("bedrock") From a8528d98ee6463668079256eedabc623c4d9d0a4 Mon Sep 17 00:00:00 2001 From: lc <30007232+LouisClt@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:22:18 +0200 Subject: [PATCH 2/6] fix(code-index): honor NO_PROXY and reuse the proxy tunnel Rebuild the Bedrock runtime host so NO_PROXY can exclude a directly-reachable endpoint, and keep the tunnel open between the one-request-per-text embedding calls, matching the handler's own default agents. Co-Authored-By: Claude Opus 5 --- .../code-index/embedders/__tests__/bedrock.spec.ts | 14 ++++++++++++-- src/services/code-index/embedders/bedrock.ts | 13 ++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/services/code-index/embedders/__tests__/bedrock.spec.ts b/src/services/code-index/embedders/__tests__/bedrock.spec.ts index de09fc3d0f..2e88493e46 100644 --- a/src/services/code-index/embedders/__tests__/bedrock.spec.ts +++ b/src/services/code-index/embedders/__tests__/bedrock.spec.ts @@ -138,8 +138,12 @@ describe("BedrockEmbedder", () => { new BedrockEmbedder("us-east-1", "test-profile", "amazon.titan-embed-text-v2:0") - expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128") - expect(HttpsProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128") + // The runtime host is passed so NO_PROXY can exclude a directly-reachable endpoint. + expect(getSystemProxyUrl).toHaveBeenLastCalledWith("https://bedrock-runtime.us-east-1.amazonaws.com") + + // keepAlive reuses the tunnel across the one-request-per-text embedding calls. + expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128", { keepAlive: true }) + expect(HttpsProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128", { keepAlive: true }) // Both agents must reach the handler: a client that tunnels only https still // resolves http DNS locally, which is the ENOTFOUND this fix is about. @@ -155,6 +159,12 @@ describe("BedrockEmbedder", () => { expect(clientConfig?.requestHandler).toBe(vitest.mocked(NodeHttpHandler).mock.instances[0]) }) + it("should resolve the China partition host for cn- regions", () => { + new BedrockEmbedder("cn-north-1", "test-profile", "amazon.titan-embed-text-v2:0") + + expect(getSystemProxyUrl).toHaveBeenLastCalledWith("https://bedrock-runtime.cn-north-1.amazonaws.com.cn") + }) + it("should not install a request handler when no proxy is configured", () => { vitest.mocked(getSystemProxyUrl).mockReturnValue(undefined) vitest.mocked(NodeHttpHandler).mockClear() diff --git a/src/services/code-index/embedders/bedrock.ts b/src/services/code-index/embedders/bedrock.ts index ee590f091a..58deffc710 100644 --- a/src/services/code-index/embedders/bedrock.ts +++ b/src/services/code-index/embedders/bedrock.ts @@ -47,7 +47,14 @@ export class BedrockEmbedder implements IEmbedder { // Behind a corporate proxy, Node resolves DNS locally before tunneling and Bedrock // endpoints fail with ENOTFOUND. The proxy agents use CONNECT so the proxy resolves // the hostname instead, matching the chat provider in src/api/providers/bedrock.ts. - const proxyUrl = getSystemProxyUrl() + // + // The SDK resolves the runtime host internally, so it is rebuilt here to let NO_PROXY + // exclude a directly-reachable endpoint. `cn-*` regions live in the China partition. + const endpointSuffix = this.region.startsWith("cn-") ? "amazonaws.com.cn" : "amazonaws.com" + const proxyUrl = getSystemProxyUrl(`https://bedrock-runtime.${this.region}.${endpointSuffix}`) + + // Embeddings are sent one request per text, so keep the tunnel open between them. + const agentOptions = { keepAlive: true } this.bedrockClient = new BedrockRuntimeClient({ userAgentAppId: `ZooCode#${Package.version}`, @@ -55,8 +62,8 @@ export class BedrockEmbedder implements IEmbedder { credentials, ...(proxyUrl && { requestHandler: new NodeHttpHandler({ - httpAgent: new HttpProxyAgent(proxyUrl), - httpsAgent: new HttpsProxyAgent(proxyUrl), + httpAgent: new HttpProxyAgent(proxyUrl, agentOptions), + httpsAgent: new HttpsProxyAgent(proxyUrl, agentOptions), }), }), }) From bdd38dda4352b4fa9452593c34bf60f346ad33f4 Mon Sep 17 00:00:00 2001 From: lc <30007232+LouisClt@users.noreply.github.com> Date: Fri, 25 Sep 2026 00:07:22 +0200 Subject: [PATCH 3/6] fix(code-index): stop reconstructing the Bedrock host for the proxy check The NO_PROXY destination added in the previous commit guessed bedrock-runtime..amazonaws.com (.com.cn for cn-*). The SDK does not resolve the host that way: its ruleset has four templates (fips and dualstack variants), only two of the eight partitions use amazonaws.com, and AWS_ENDPOINT_URL* or endpoint_url in the shared config can override the host outright. Matching that would mean reimplementing the ruleset. Pass no destination instead, so the proxy applies whenever one is configured. This is the trade src/api/providers/bedrock.ts already documents for the default managed endpoint, so the two Bedrock clients now behave the same. Co-Authored-By: Claude Opus 5 --- .../embedders/__tests__/bedrock.spec.ts | 16 +++++++++------- src/services/code-index/embedders/bedrock.ts | 8 ++++---- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/services/code-index/embedders/__tests__/bedrock.spec.ts b/src/services/code-index/embedders/__tests__/bedrock.spec.ts index 2e88493e46..de7125bc31 100644 --- a/src/services/code-index/embedders/__tests__/bedrock.spec.ts +++ b/src/services/code-index/embedders/__tests__/bedrock.spec.ts @@ -108,6 +108,11 @@ describe("BedrockEmbedder", () => { }) describe("constructor", () => { + afterEach(() => { + // clearAllMocks() keeps implementations, so a proxy stub would leak into later tests. + vitest.mocked(getSystemProxyUrl).mockReturnValue(undefined) + }) + it("should initialize with provided region, profile and model", () => { expect(embedder.embedderInfo.name).toBe("bedrock") }) @@ -138,8 +143,8 @@ describe("BedrockEmbedder", () => { new BedrockEmbedder("us-east-1", "test-profile", "amazon.titan-embed-text-v2:0") - // The runtime host is passed so NO_PROXY can exclude a directly-reachable endpoint. - expect(getSystemProxyUrl).toHaveBeenLastCalledWith("https://bedrock-runtime.us-east-1.amazonaws.com") + // No destination is passed: the SDK resolves the host, so it cannot be guessed here. + expect(getSystemProxyUrl).toHaveBeenLastCalledWith() // keepAlive reuses the tunnel across the one-request-per-text embedding calls. expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128", { keepAlive: true }) @@ -157,12 +162,9 @@ describe("BedrockEmbedder", () => { // ...and the handler must reach the client. const clientConfig = vitest.mocked(BedrockRuntimeClient).mock.calls.at(-1)?.[0] expect(clientConfig?.requestHandler).toBe(vitest.mocked(NodeHttpHandler).mock.instances[0]) - }) - - it("should resolve the China partition host for cn- regions", () => { - new BedrockEmbedder("cn-north-1", "test-profile", "amazon.titan-embed-text-v2:0") - expect(getSystemProxyUrl).toHaveBeenLastCalledWith("https://bedrock-runtime.cn-north-1.amazonaws.com.cn") + // Pinning an endpoint here would break FIPS, dualstack and non-default partitions. + expect(clientConfig).not.toHaveProperty("endpoint") }) it("should not install a request handler when no proxy is configured", () => { diff --git a/src/services/code-index/embedders/bedrock.ts b/src/services/code-index/embedders/bedrock.ts index 58deffc710..1c91afb163 100644 --- a/src/services/code-index/embedders/bedrock.ts +++ b/src/services/code-index/embedders/bedrock.ts @@ -48,10 +48,10 @@ export class BedrockEmbedder implements IEmbedder { // endpoints fail with ENOTFOUND. The proxy agents use CONNECT so the proxy resolves // the hostname instead, matching the chat provider in src/api/providers/bedrock.ts. // - // The SDK resolves the runtime host internally, so it is rebuilt here to let NO_PROXY - // exclude a directly-reachable endpoint. `cn-*` regions live in the China partition. - const endpointSuffix = this.region.startsWith("cn-") ? "amazonaws.com.cn" : "amazonaws.com" - const proxyUrl = getSystemProxyUrl(`https://bedrock-runtime.${this.region}.${endpointSuffix}`) + // No destination is passed for the NO_PROXY check: the SDK resolves the host itself + // from the partition, FIPS/dualstack flags and endpoint overrides, so it cannot be + // reconstructed here. As in the chat provider, the proxy applies whenever one is set. + const proxyUrl = getSystemProxyUrl() // Embeddings are sent one request per text, so keep the tunnel open between them. const agentOptions = { keepAlive: true } From cdf199362735ab1b315ac79fe4051e70046d4a02 Mon Sep 17 00:00:00 2001 From: lc <30007232+LouisClt@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:39:56 +0200 Subject: [PATCH 4/6] fix(code-index): pick the proxy per request so NO_PROXY is honored The embedder either guessed the Bedrock host to test NO_PROXY against, or skipped the test entirely. Neither works: the SDK derives the host from the partition, the FIPS/dualstack flags and any endpoint override, so it is only known once a request has been built. Add createProxyRoutingRequestHandler to networkProxy.ts. It returns undefined when no proxy is configured, and otherwise a handler that routes each request through the proxy unless NO_PROXY covers its resolved host. This matters beyond connectivity: the payload is indexed file content, so a NO_PROXY entry excluding Bedrock has to keep that content off the proxy. The helper lives in networkProxy.ts rather than in the embedder so the chat provider, which has the same gap for its default managed endpoint, can adopt it without another implementation. Co-Authored-By: Claude Opus 5 --- .../embedders/__tests__/bedrock.spec.ts | 60 +++----- src/services/code-index/embedders/bedrock.ts | 27 +--- src/utils/__tests__/networkProxy.spec.ts | 136 +++++++++++++++++- src/utils/networkProxy.ts | 78 ++++++++++ 4 files changed, 236 insertions(+), 65 deletions(-) diff --git a/src/services/code-index/embedders/__tests__/bedrock.spec.ts b/src/services/code-index/embedders/__tests__/bedrock.spec.ts index de7125bc31..f089fece39 100644 --- a/src/services/code-index/embedders/__tests__/bedrock.spec.ts +++ b/src/services/code-index/embedders/__tests__/bedrock.spec.ts @@ -1,13 +1,9 @@ import type { MockedFunction } from "vitest" import { BedrockRuntimeClient, InvokeModelCommand } from "@aws-sdk/client-bedrock-runtime" -import { NodeHttpHandler } from "@smithy/node-http-handler" -import { HttpProxyAgent } from "http-proxy-agent" -import { HttpsProxyAgent } from "https-proxy-agent" - import { BedrockEmbedder } from "../bedrock" import { MAX_ITEM_TOKENS, INITIAL_RETRY_DELAY_MS } from "../../constants" -import { getSystemProxyUrl } from "../../../../utils/networkProxy" +import { createProxyRoutingRequestHandler } from "../../../../utils/networkProxy" import { clearAllMocks } from "../../../../test-utils/reset" @@ -32,15 +28,8 @@ vitest.mock("@aws-sdk/credential-providers", () => ({ })) vitest.mock("../../../../utils/networkProxy", () => ({ - getSystemProxyUrl: vitest.fn().mockReturnValue(undefined), -})) -vitest.mock("@smithy/node-http-handler", () => ({ - NodeHttpHandler: vitest.fn().mockImplementation(function (options: unknown) { - return { options } - }), + createProxyRoutingRequestHandler: vitest.fn().mockReturnValue(undefined), })) -vitest.mock("http-proxy-agent", () => ({ HttpProxyAgent: vitest.fn() })) -vitest.mock("https-proxy-agent", () => ({ HttpsProxyAgent: vitest.fn() })) // Mock TelemetryService vitest.mock("@roo-code/telemetry", () => ({ @@ -109,8 +98,8 @@ describe("BedrockEmbedder", () => { describe("constructor", () => { afterEach(() => { - // clearAllMocks() keeps implementations, so a proxy stub would leak into later tests. - vitest.mocked(getSystemProxyUrl).mockReturnValue(undefined) + // clearAllMocks() keeps implementations, so a stub would leak into later tests. + vitest.mocked(createProxyRoutingRequestHandler).mockReturnValue(undefined) }) it("should initialize with provided region, profile and model", () => { @@ -136,44 +125,29 @@ describe("BedrockEmbedder", () => { ) }) - it("should tunnel through the system proxy when one is configured", () => { - // The embedder built in beforeEach already consumed these mocks. - vitest.mocked(getSystemProxyUrl).mockReturnValue("http://proxy.corp.local:3128") - vitest.mocked(NodeHttpHandler).mockClear() + it("should route requests through the proxy-aware handler when one is built", () => { + const handler = { + handle: vitest.fn(), + updateHttpClientConfig: vitest.fn(), + httpHandlerConfigs: vitest.fn(), + destroy: vitest.fn(), + } + vitest.mocked(createProxyRoutingRequestHandler).mockReturnValue(handler) new BedrockEmbedder("us-east-1", "test-profile", "amazon.titan-embed-text-v2:0") - // No destination is passed: the SDK resolves the host, so it cannot be guessed here. - expect(getSystemProxyUrl).toHaveBeenLastCalledWith() - - // keepAlive reuses the tunnel across the one-request-per-text embedding calls. - expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128", { keepAlive: true }) - expect(HttpsProxyAgent).toHaveBeenCalledWith("http://proxy.corp.local:3128", { keepAlive: true }) - - // Both agents must reach the handler: a client that tunnels only https still - // resolves http DNS locally, which is the ENOTFOUND this fix is about. - expect(NodeHttpHandler).toHaveBeenCalledOnce() - // The constructor accepts options or a provider function; only the object form is used here. - const handlerArg = vitest.mocked(NodeHttpHandler).mock.calls[0][0] - const handlerOptions = typeof handlerArg === "function" ? undefined : handlerArg - expect(handlerOptions?.httpAgent).toBe(vitest.mocked(HttpProxyAgent).mock.instances[0]) - expect(handlerOptions?.httpsAgent).toBe(vitest.mocked(HttpsProxyAgent).mock.instances[0]) - - // ...and the handler must reach the client. const clientConfig = vitest.mocked(BedrockRuntimeClient).mock.calls.at(-1)?.[0] - expect(clientConfig?.requestHandler).toBe(vitest.mocked(NodeHttpHandler).mock.instances[0]) - - // Pinning an endpoint here would break FIPS, dualstack and non-default partitions. + expect(clientConfig?.requestHandler).toBe(handler) + // Pinning an endpoint here would break FIPS, dualstack and non-default partitions, + // and would take the routing decision away from the handler. expect(clientConfig).not.toHaveProperty("endpoint") }) - it("should not install a request handler when no proxy is configured", () => { - vitest.mocked(getSystemProxyUrl).mockReturnValue(undefined) - vitest.mocked(NodeHttpHandler).mockClear() + it("should keep the client default handler when no proxy is configured", () => { + vitest.mocked(createProxyRoutingRequestHandler).mockReturnValue(undefined) new BedrockEmbedder("us-east-1", "test-profile", "amazon.titan-embed-text-v2:0") - expect(NodeHttpHandler).not.toHaveBeenCalled() expect(BedrockRuntimeClient).toHaveBeenLastCalledWith( expect.not.objectContaining({ requestHandler: expect.anything() }), ) diff --git a/src/services/code-index/embedders/bedrock.ts b/src/services/code-index/embedders/bedrock.ts index 1c91afb163..2473346562 100644 --- a/src/services/code-index/embedders/bedrock.ts +++ b/src/services/code-index/embedders/bedrock.ts @@ -1,9 +1,6 @@ import { BedrockRuntimeClient, InvokeModelCommand, InvokeModelCommandInput } from "@aws-sdk/client-bedrock-runtime" import { fromIni, fromNodeProviderChain } from "@aws-sdk/credential-providers" -import { NodeHttpHandler } from "@smithy/node-http-handler" -import { HttpProxyAgent } from "http-proxy-agent" -import { HttpsProxyAgent } from "https-proxy-agent" -import { getSystemProxyUrl } from "../../../utils/networkProxy" +import { createProxyRoutingRequestHandler } from "../../../utils/networkProxy" import { IEmbedder, EmbeddingResponse, EmbedderInfo } from "../interfaces" import { MAX_BATCH_TOKENS, @@ -44,28 +41,16 @@ export class BedrockEmbedder implements IEmbedder { // If profile is specified, use it; otherwise use default credential chain const credentials = this.profile ? fromIni({ profile: this.profile }) : fromNodeProviderChain() - // Behind a corporate proxy, Node resolves DNS locally before tunneling and Bedrock - // endpoints fail with ENOTFOUND. The proxy agents use CONNECT so the proxy resolves - // the hostname instead, matching the chat provider in src/api/providers/bedrock.ts. - // - // No destination is passed for the NO_PROXY check: the SDK resolves the host itself - // from the partition, FIPS/dualstack flags and endpoint overrides, so it cannot be - // reconstructed here. As in the chat provider, the proxy applies whenever one is set. - const proxyUrl = getSystemProxyUrl() - - // Embeddings are sent one request per text, so keep the tunnel open between them. - const agentOptions = { keepAlive: true } + // Behind a corporate proxy, Node resolves DNS locally and Bedrock endpoints fail with + // ENOTFOUND. The handler tunnels through the proxy with CONNECT so the proxy resolves + // the hostname, and connects directly to the destinations NO_PROXY excludes. + const requestHandler = createProxyRoutingRequestHandler() this.bedrockClient = new BedrockRuntimeClient({ userAgentAppId: `ZooCode#${Package.version}`, region: this.region, credentials, - ...(proxyUrl && { - requestHandler: new NodeHttpHandler({ - httpAgent: new HttpProxyAgent(proxyUrl, agentOptions), - httpsAgent: new HttpsProxyAgent(proxyUrl, agentOptions), - }), - }), + ...(requestHandler && { requestHandler }), }) this.defaultModelId = modelId || getDefaultModelId("bedrock") diff --git a/src/utils/__tests__/networkProxy.spec.ts b/src/utils/__tests__/networkProxy.spec.ts index 94e91cd990..a6c7b23200 100644 --- a/src/utils/__tests__/networkProxy.spec.ts +++ b/src/utils/__tests__/networkProxy.spec.ts @@ -1,11 +1,35 @@ import * as vscode from "vscode" -import { initializeNetworkProxy, getProxyConfig, isProxyEnabled, isDebugMode, getSystemProxyUrl } from "../networkProxy" +import { NodeHttpHandler } from "@smithy/node-http-handler" +import { HttpProxyAgent } from "http-proxy-agent" +import { HttpsProxyAgent } from "https-proxy-agent" +import { + initializeNetworkProxy, + getProxyConfig, + isProxyEnabled, + isDebugMode, + getSystemProxyUrl, + createProxyRoutingRequestHandler, +} from "../networkProxy" // Mock global-agent vi.mock("global-agent", () => ({ bootstrap: vi.fn(), })) +vi.mock("@smithy/node-http-handler", () => ({ + NodeHttpHandler: vi.fn().mockImplementation(function (options?: unknown) { + return { + options, + handle: vi.fn(), + updateHttpClientConfig: vi.fn(), + httpHandlerConfigs: vi.fn().mockReturnValue({}), + destroy: vi.fn(), + } + }), +})) +vi.mock("http-proxy-agent", () => ({ HttpProxyAgent: vi.fn() })) +vi.mock("https-proxy-agent", () => ({ HttpsProxyAgent: vi.fn() })) + // Mock vscode vi.mock("vscode", () => ({ workspace: { @@ -469,4 +493,114 @@ describe("networkProxy", () => { }) }) }) + + describe("createProxyRoutingRequestHandler", () => { + type Handler = NonNullable> + + // The handler reads only these fields off the request. Building a real HttpRequest would + // pull in @smithy/protocol-http, which is not a direct dependency, so a literal stands in. + const requestTo = (hostname: string, port?: number) => + ({ protocol: "https:", hostname, port }) as unknown as Parameters[0] + + // The two inner handlers are told apart by how they were built: only the proxied one + // receives agents. + const innerHandlers = () => { + const built = vi.mocked(NodeHttpHandler).mock.results.map((r) => r.value) + return { + direct: built.find((h) => h.options === undefined), + proxied: built.find((h) => h.options?.httpsAgent), + } + } + + beforeEach(() => { + vi.clearAllMocks() + delete process.env.HTTPS_PROXY + delete process.env.HTTP_PROXY + delete process.env.NO_PROXY + mockConfig.get.mockReturnValue(undefined) + }) + + it("should return undefined when no proxy is configured", () => { + expect(createProxyRoutingRequestHandler()).toBeUndefined() + expect(NodeHttpHandler).not.toHaveBeenCalled() + }) + + it("should build both proxy agents with a reusable tunnel", () => { + process.env.HTTPS_PROXY = "http://proxy.corp:3128" + + expect(createProxyRoutingRequestHandler()).toBeDefined() + + expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp:3128", { keepAlive: true }) + expect(HttpsProxyAgent).toHaveBeenCalledWith("http://proxy.corp:3128", { keepAlive: true }) + const { proxied } = innerHandlers() + expect(proxied?.options?.httpAgent).toBe(vi.mocked(HttpProxyAgent).mock.instances[0]) + expect(proxied?.options?.httpsAgent).toBe(vi.mocked(HttpsProxyAgent).mock.instances[0]) + }) + + it("should send a request through the proxy when NO_PROXY does not cover it", () => { + process.env.HTTPS_PROXY = "http://proxy.corp:3128" + process.env.NO_PROXY = "example.com" + + const handler = createProxyRoutingRequestHandler() + const request = requestTo("bedrock-runtime.us-east-1.amazonaws.com") + handler?.handle(request) + + const { direct, proxied } = innerHandlers() + expect(proxied?.handle).toHaveBeenCalledWith(request) + expect(direct?.handle).not.toHaveBeenCalled() + }) + + it("should send a request directly when NO_PROXY covers its host", () => { + process.env.HTTPS_PROXY = "http://proxy.corp:3128" + process.env.NO_PROXY = "amazonaws.com" + + const handler = createProxyRoutingRequestHandler() + // The host is only known per request, which is the point of deciding here: indexed + // file contents must not reach a proxy the user excluded. + const request = requestTo("bedrock-runtime.eu-west-1.amazonaws.com") + handler?.handle(request) + + const { direct, proxied } = innerHandlers() + expect(direct?.handle).toHaveBeenCalledWith(request) + expect(proxied?.handle).not.toHaveBeenCalled() + }) + + it("should send every request directly when NO_PROXY is '*'", () => { + process.env.HTTPS_PROXY = "http://proxy.corp:3128" + process.env.NO_PROXY = "*" + + const handler = createProxyRoutingRequestHandler() + handler?.handle(requestTo("bedrock-runtime.us-east-1.amazonaws.com")) + + const { direct, proxied } = innerHandlers() + expect(direct?.handle).toHaveBeenCalledOnce() + expect(proxied?.handle).not.toHaveBeenCalled() + }) + + it("should match NO_PROXY against the host when the request carries a port", () => { + process.env.HTTPS_PROXY = "http://proxy.corp:3128" + process.env.NO_PROXY = "amazonaws.com" + + const handler = createProxyRoutingRequestHandler() + handler?.handle(requestTo("bedrock-runtime.us-east-1.amazonaws.com", 8443)) + + const { direct, proxied } = innerHandlers() + expect(direct?.handle).toHaveBeenCalledOnce() + expect(proxied?.handle).not.toHaveBeenCalled() + }) + + it("should forward configuration updates and teardown to both routes", () => { + process.env.HTTPS_PROXY = "http://proxy.corp:3128" + + const handler = createProxyRoutingRequestHandler() + handler?.updateHttpClientConfig("requestTimeout", 1234) + handler?.destroy() + + const { direct, proxied } = innerHandlers() + expect(direct?.updateHttpClientConfig).toHaveBeenCalledWith("requestTimeout", 1234) + expect(proxied?.updateHttpClientConfig).toHaveBeenCalledWith("requestTimeout", 1234) + expect(direct?.destroy).toHaveBeenCalledOnce() + expect(proxied?.destroy).toHaveBeenCalledOnce() + }) + }) }) diff --git a/src/utils/networkProxy.ts b/src/utils/networkProxy.ts index 835887334e..7fb379fbac 100644 --- a/src/utils/networkProxy.ts +++ b/src/utils/networkProxy.ts @@ -11,6 +11,9 @@ */ import * as vscode from "vscode" +import { NodeHttpHandler } from "@smithy/node-http-handler" +import { HttpProxyAgent } from "http-proxy-agent" +import { HttpsProxyAgent } from "https-proxy-agent" import { Package } from "../shared/package" /** @@ -419,6 +422,81 @@ export function getSystemProxyUrl(targetUrl?: string): string | undefined { return undefined } +// Derived from the handler's own signatures so this file needs no @smithy/types dependency. +type HandleParameters = Parameters +type UpdateClientConfigParameters = Parameters + +/** + * The subset of the AWS SDK request-handler contract implemented below. Exported so callers and + * their tests can name the type without reaching for @smithy/types. + */ +export interface ProxyRoutingHandler { + handle(...args: HandleParameters): ReturnType + updateHttpClientConfig(...args: UpdateClientConfigParameters): void + httpHandlerConfigs(): ReturnType + destroy(): void +} + +/** + * AWS SDK request handler that chooses, per request, between the system proxy and a direct + * connection. + * + * The SDK resolves its own endpoint — region, partition, FIPS/dualstack flags, and any endpoint + * override — so the destination is only known once a request has been built. Choosing here is + * what makes NO_PROXY apply to the host actually called, rather than one guessed up front. + * + * Both routes are HTTP/1.1. A client whose default handler is NodeHttp2Handler therefore drops + * to 1.1 once a proxy is configured, including on the direct route; the Bedrock calls involved + * are unary requests, and the chat provider already tunnels over 1.1. + */ +class ProxyRoutingRequestHandler implements ProxyRoutingHandler { + private readonly direct = new NodeHttpHandler() + private readonly proxied: NodeHttpHandler + + constructor(proxyUrl: string) { + // Callers such as the code-index embedder send one request per item, so keep the tunnel. + const agentOptions = { keepAlive: true } + this.proxied = new NodeHttpHandler({ + httpAgent: new HttpProxyAgent(proxyUrl, agentOptions), + httpsAgent: new HttpsProxyAgent(proxyUrl, agentOptions), + }) + } + + private handlerFor(request: HandleParameters[0]): NodeHttpHandler { + // NO_PROXY entries match on host alone, so the port is left out. + return isNoProxyHost(`${request.protocol}//${request.hostname}`) ? this.direct : this.proxied + } + + handle(...args: HandleParameters) { + return this.handlerFor(args[0]).handle(...args) + } + + updateHttpClientConfig(...args: UpdateClientConfigParameters) { + this.direct.updateHttpClientConfig(...args) + this.proxied.updateHttpClientConfig(...args) + } + + httpHandlerConfigs() { + return this.proxied.httpHandlerConfigs() + } + + destroy() { + this.direct.destroy() + this.proxied.destroy() + } +} + +/** + * Build a request handler routing AWS SDK traffic through the system proxy, except for the + * destinations NO_PROXY excludes. + * + * Returns undefined when no proxy is configured, so the client keeps its own default handler. + */ +export function createProxyRoutingRequestHandler(): ProxyRoutingHandler | undefined { + const proxyUrl = getSystemProxyUrl() + return proxyUrl ? new ProxyRoutingRequestHandler(proxyUrl) : undefined +} + /** * Log a message to the output channel if available. */ From 82b3fee36f0bc59b610fa8a4541cdb4d4c8d62fc Mon Sep 17 00:00:00 2001 From: lc <30007232+LouisClt@users.noreply.github.com> Date: Sat, 26 Sep 2026 09:46:58 +0200 Subject: [PATCH 5/6] test(network-proxy): clear the lowercase proxy variables too createProxyRoutingRequestHandler() calls getSystemProxyUrl() with no target, so it reads https_proxy and http_proxy as well. The setup deleted only the uppercase names, matching neither getSystemProxyUrl's own test block nor the Linux runners, where process.env is case-sensitive. Co-Authored-By: Claude Opus 5 --- src/utils/__tests__/networkProxy.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utils/__tests__/networkProxy.spec.ts b/src/utils/__tests__/networkProxy.spec.ts index a6c7b23200..b6dab0b4e7 100644 --- a/src/utils/__tests__/networkProxy.spec.ts +++ b/src/utils/__tests__/networkProxy.spec.ts @@ -515,8 +515,11 @@ describe("networkProxy", () => { beforeEach(() => { vi.clearAllMocks() delete process.env.HTTPS_PROXY + delete process.env.https_proxy delete process.env.HTTP_PROXY + delete process.env.http_proxy delete process.env.NO_PROXY + delete process.env.no_proxy mockConfig.get.mockReturnValue(undefined) }) From 5919f915def89529b6ddee914770a186affe924b Mon Sep 17 00:00:00 2001 From: lc <30007232+LouisClt@users.noreply.github.com> Date: Sat, 26 Sep 2026 10:16:49 +0200 Subject: [PATCH 6/6] fix(network-proxy): let proxied connections die with their request The proxy agents were built with keepAlive, which retains an idle connection after each request. The client's default handler is NodeHttp2Handler with disableConcurrentStreams: true, so before this handler existed every request got an isolated HTTP/2 session that was destroyed on stream close, and nothing outlived it. IEmbedder has no disposal contract to reclaim a pooled connection, so drop the option rather than add a cleanup path with no call site. This also matches src/api/providers/bedrock.ts, which passes no agent options. Co-Authored-By: Claude Opus 5 --- src/utils/__tests__/networkProxy.spec.ts | 7 ++++--- src/utils/networkProxy.ts | 12 +++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/utils/__tests__/networkProxy.spec.ts b/src/utils/__tests__/networkProxy.spec.ts index b6dab0b4e7..41c85a10e9 100644 --- a/src/utils/__tests__/networkProxy.spec.ts +++ b/src/utils/__tests__/networkProxy.spec.ts @@ -528,13 +528,14 @@ describe("networkProxy", () => { expect(NodeHttpHandler).not.toHaveBeenCalled() }) - it("should build both proxy agents with a reusable tunnel", () => { + it("should build both proxy agents without keeping connections alive", () => { process.env.HTTPS_PROXY = "http://proxy.corp:3128" expect(createProxyRoutingRequestHandler()).toBeDefined() - expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp:3128", { keepAlive: true }) - expect(HttpsProxyAgent).toHaveBeenCalledWith("http://proxy.corp:3128", { keepAlive: true }) + // No agent options: an idle connection would outlive the request that opened it. + expect(HttpProxyAgent).toHaveBeenCalledWith("http://proxy.corp:3128") + expect(HttpsProxyAgent).toHaveBeenCalledWith("http://proxy.corp:3128") const { proxied } = innerHandlers() expect(proxied?.options?.httpAgent).toBe(vi.mocked(HttpProxyAgent).mock.instances[0]) expect(proxied?.options?.httpsAgent).toBe(vi.mocked(HttpsProxyAgent).mock.instances[0]) diff --git a/src/utils/networkProxy.ts b/src/utils/networkProxy.ts index 7fb379fbac..e5e75db1f6 100644 --- a/src/utils/networkProxy.ts +++ b/src/utils/networkProxy.ts @@ -445,20 +445,18 @@ export interface ProxyRoutingHandler { * override — so the destination is only known once a request has been built. Choosing here is * what makes NO_PROXY apply to the host actually called, rather than one guessed up front. * - * Both routes are HTTP/1.1. A client whose default handler is NodeHttp2Handler therefore drops - * to 1.1 once a proxy is configured, including on the direct route; the Bedrock calls involved - * are unary requests, and the chat provider already tunnels over 1.1. + * The agents take no options, so no connection outlives its request. Both routes are HTTP/1.1: + * a client defaulting to NodeHttp2Handler drops to 1.1 once a proxy is configured, which is what + * the chat provider already does. */ class ProxyRoutingRequestHandler implements ProxyRoutingHandler { private readonly direct = new NodeHttpHandler() private readonly proxied: NodeHttpHandler constructor(proxyUrl: string) { - // Callers such as the code-index embedder send one request per item, so keep the tunnel. - const agentOptions = { keepAlive: true } this.proxied = new NodeHttpHandler({ - httpAgent: new HttpProxyAgent(proxyUrl, agentOptions), - httpsAgent: new HttpsProxyAgent(proxyUrl, agentOptions), + httpAgent: new HttpProxyAgent(proxyUrl), + httpsAgent: new HttpsProxyAgent(proxyUrl), }) }