From dd8c76110f9b898b9152ea5fc181142c1cda84c1 Mon Sep 17 00:00:00 2001 From: Marcus Widing Date: Mon, 2 Mar 2026 09:40:22 +0100 Subject: [PATCH] fix: remove isFirst guard from NO_PROXY restore, add reverse-exit test Fix Greptile review: when call A exits before call B, the isFirst flag on B is false, so the restore condition (refCount===0 && isFirst) was never true and NO_PROXY leaked permanently. Remove '&& isFirst' so any last exiter (refCount===0) restores the original env vars. Added explicit reverse-exit-order regression test. --- src/browser/cdp-proxy-bypass.test.ts | 45 ++++++++++++++++++++++++++++ src/browser/cdp-proxy-bypass.ts | 2 +- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/browser/cdp-proxy-bypass.test.ts b/src/browser/cdp-proxy-bypass.test.ts index b8267acf112..578f0f39630 100644 --- a/src/browser/cdp-proxy-bypass.test.ts +++ b/src/browser/cdp-proxy-bypass.test.ts @@ -203,3 +203,48 @@ describe("withNoProxyForLocalhost concurrency", () => { } }); }); + +describe("withNoProxyForLocalhost reverse exit order", () => { + it("restores NO_PROXY when first caller exits before second", async () => { + const origNoProxy = process.env.NO_PROXY; + const origNoProxyLower = process.env.no_proxy; + delete process.env.NO_PROXY; + delete process.env.no_proxy; + process.env.HTTP_PROXY = "http://proxy:8080"; + + try { + const { withNoProxyForLocalhost } = await import("./cdp-proxy-bypass.js"); + + const delay = (ms: number) => new Promise((r) => setTimeout(r, ms)); + + // Call A enters first, exits first (short task) + // Call B enters second, exits last (long task) + const callA = withNoProxyForLocalhost(async () => { + await delay(10); + return "a"; + }); + const callB = withNoProxyForLocalhost(async () => { + await delay(60); + return "b"; + }); + + await Promise.all([callA, callB]); + + // After both complete, NO_PROXY must be cleaned up + expect(process.env.NO_PROXY).toBeUndefined(); + expect(process.env.no_proxy).toBeUndefined(); + } finally { + delete process.env.HTTP_PROXY; + if (origNoProxy !== undefined) { + process.env.NO_PROXY = origNoProxy; + } else { + delete process.env.NO_PROXY; + } + if (origNoProxyLower !== undefined) { + process.env.no_proxy = origNoProxyLower; + } else { + delete process.env.no_proxy; + } + } + }); +}); diff --git a/src/browser/cdp-proxy-bypass.ts b/src/browser/cdp-proxy-bypass.ts index 7331a0e0704..c41ac2becdc 100644 --- a/src/browser/cdp-proxy-bypass.ts +++ b/src/browser/cdp-proxy-bypass.ts @@ -92,7 +92,7 @@ export async function withNoProxyForLocalhost(fn: () => Promise): Promise< return await fn(); } finally { noProxyRefCount--; - if (noProxyRefCount === 0 && isFirst) { + if (noProxyRefCount === 0) { if (savedNoProxy !== undefined) { process.env.NO_PROXY = savedNoProxy; } else {