-
Notifications
You must be signed in to change notification settings - Fork 197
feat: honor HTTP_PROXY/HTTPS_PROXY/NO_PROXY for all outbound traffic #487
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
bfee8b9
ed46887
e30eb89
6b4e094
72cf98e
54e97c0
6cf1d3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| "@moonshot-ai/agent-core": minor | ||
| "@moonshot-ai/kimi-code-sdk": minor | ||
| "@moonshot-ai/kimi-code": minor | ||
| --- | ||
|
|
||
| Honor the standard `HTTP_PROXY` / `HTTPS_PROXY` / `ALL_PROXY` / `NO_PROXY` environment variables, including SOCKS proxies, for all outbound traffic. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import { ErrorCodes, KimiError } from '#/errors'; | ||
| import type { McpServerStdioConfig } from '#/config/schema'; | ||
| import { proxyEnvForChild, reconcileChildNoProxy } from '#/utils/proxy'; | ||
| import { Client } from '@modelcontextprotocol/sdk/client/index.js'; | ||
| import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; | ||
|
|
||
|
|
@@ -216,13 +217,24 @@ class BoundedTail { | |
| } | ||
|
|
||
| // Inherit the parent's env so PATH/HOME/etc. survive — otherwise `npx`/`uvx` | ||
| // style stdio servers fail to launch even with a valid config. Explicit | ||
| // `config.env` entries still override on conflict. | ||
| function mergeStdioEnv(configEnv?: Record<string, string>): Record<string, string> { | ||
| // style stdio servers fail to launch even with a valid config. `config.env` | ||
| // overrides on conflict. A node child does not inherit our in-process undici | ||
| // dispatcher, so `proxyEnvForChild` adds `NODE_USE_ENV_PROXY` (and a | ||
| // loopback-protected `NO_PROXY`) to make it honor the proxy natively (on a Node | ||
| // version that supports the flag — ≥22.21 or ≥24.5). It is computed from the | ||
| // MERGED env so a proxy declared only in `config.env` is honored too. | ||
| // `reconcileChildNoProxy` then mirrors a single-casing `NO_PROXY` override onto | ||
| // both casings so it isn't shadowed by the injected value. | ||
| export function mergeStdioEnv( | ||
| configEnv?: Record<string, string>, | ||
| parentEnv: Readonly<Record<string, string | undefined>> = process.env, | ||
| ): Record<string, string> { | ||
| const merged: Record<string, string> = {}; | ||
| for (const [key, value] of Object.entries(process.env)) { | ||
| for (const [key, value] of Object.entries(parentEnv)) { | ||
| if (value !== undefined) merged[key] = value; | ||
| } | ||
| if (configEnv !== undefined) Object.assign(merged, configEnv); | ||
| Object.assign(merged, proxyEnvForChild(merged)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a stdio server config explicitly sets Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the parent env has a lowercase proxy (for example Useful? React with 👍 / 👎. |
||
| reconcileChildNoProxy(merged, configEnv); | ||
| return merged; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the packaged CLI, this new SDK import pulls in
packages/agent-core/src/utils/proxy.ts, which statically importsundiciandsocks. I checkedapps/kimi-code/tsdown.config.ts: only@moonshot-ai/*packages are forced into the bundle, so those third-party imports are left as runtime dependencies, butapps/kimi-code/package.jsondoes not declare either package. A fresh npm install of@moonshot-ai/kimi-codewould therefore fail to loaddist/main.mjsbefore startup whenever Node cannot resolve those packages; add them to the app package dependencies or force them into the CLI bundle.Useful? React with 👍 / 👎.