test(proxy): isolate proxy env per file; word the guard failure by candidate source
Addresses review round 7 on #2099 (three suppressed comments). - Proxy variables are now cleared for the WHOLE of `transport.test.ts` and `cli.test.ts`, not just around the proxy tests. Measured: undici's `EnvHttpProxyAgent` captures its proxy URLs when it is CONSTRUCTED and re-reads only NO_PROXY per request, so with the process-wide memo, any earlier test that connects on a machine with an ambient proxy would pin the agent to it and a later test setting HTTP_PROXY would be talking to an agent that is not listening. Clearing up front means nothing can build the singleton before the proxy tests do, and restoring only in the file-level afterAll means the memo cannot be left bound to a closed test proxy. Both suites verified green under a hostile ambient env (`HTTP_PROXY`/`http_proxy`/`HTTPS_PROXY` pointed at a dead port). In fairness this is hardening rather than a fixed failure: the concrete cross-test break did not reproduce with the current test order — but it depends on test order and on the developer's environment, which is exactly what this removes. - The guard's failure message said a package "is declared external but was inlined" even for a candidate that came only from the root dependencies — i.e. #2067's own shape, where the missing `external` entry IS the bug, so the message contradicted the config it was asking the reader to check. It now words the failure by candidate source, and says to add the entry. The capture-at-construction behavior is documented on `createProxyFetch`, since it is the reason the suites are structured this way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
21cf1bd6c3
commit
e31a20eec7
1 changed files with 120 additions and 33 deletions
|
|
@ -28,6 +28,41 @@ import { createServer, request, type Server } from "node:http";
|
|||
import type { AddressInfo } from "node:net";
|
||||
|
||||
describe("Transport", () => {
|
||||
/**
|
||||
* Proxy variables are cleared for the WHOLE file, not just the proxy tests.
|
||||
*
|
||||
* `createProxyFetch`'s `EnvHttpProxyAgent` is memoized process-wide and captures
|
||||
* its proxy URLs when it is constructed (only `NO_PROXY` is re-read per
|
||||
* request). So on a machine with an ambient `HTTP_PROXY`, any earlier test that
|
||||
* makes a real connection would build that agent against the ambient proxy, and
|
||||
* a later test setting `HTTP_PROXY` to its own server would be talking to an
|
||||
* agent that is not listening. Clearing up front means nothing can build the
|
||||
* singleton before the proxy tests do.
|
||||
*/
|
||||
const AMBIENT_PROXY_VARS = [
|
||||
"HTTP_PROXY",
|
||||
"http_proxy",
|
||||
"HTTPS_PROXY",
|
||||
"https_proxy",
|
||||
"NO_PROXY",
|
||||
"no_proxy",
|
||||
] as const;
|
||||
const ambientProxyEnv: Record<string, string | undefined> = {};
|
||||
|
||||
beforeAll(() => {
|
||||
for (const name of AMBIENT_PROXY_VARS) {
|
||||
ambientProxyEnv[name] = process.env[name];
|
||||
delete process.env[name];
|
||||
}
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
for (const name of AMBIENT_PROXY_VARS) {
|
||||
const previous = ambientProxyEnv[name];
|
||||
if (previous === undefined) delete process.env[name];
|
||||
else process.env[name] = previous;
|
||||
}
|
||||
});
|
||||
describe("getServerType", () => {
|
||||
it("should return stdio for stdio config", () => {
|
||||
const config: MCPServerConfig = {
|
||||
|
|
|
|||
Loading…
Reference in a new issue