From 2e5bf12411afad5d9352abec821cfd7c1ef7e78e Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Mon, 19 Oct 2026 12:49:25 -0700 Subject: [PATCH] test(adapter-utils): retry HTTP/2 fixture port collisions (#16888) ## Thinking Path > - Paperclip manages work for AI agents. > - Sandbox agents use a callback bridge to reach the API. > - Tests start the generated HTTP/2 gateway on an assigned loopback port. > - The test probe releases that port before the gateway binds it. > - Another process can claim the port during that gap. > - This PR retries that specific fixture startup collision and tests its limits. ## Linked Issues or Issue Description **What happened?** The unchanged callback bridge test failed in #16605 because its selected port was already in use. [CI job 118251110752](https://github.com/paperclipai/paperclip/actions/runs/39308303837/job/118251110752) reports `EADDRINUSE` before the gateway READY frame. The same helper is present on master `4d92589799`; no merged or open fix for this exact failure was found. **Expected behavior** The fixture starts on a fresh port after a confirmed bind collision. It stops after three attempts and preserves other startup failures. **Steps to reproduce** The new held-probe case keeps the selected socket open before the real child starts. Before the correction, the original authorization test fails with `EADDRINUSE`. The unchanged no-collision case passes. **Additional context** This is the separate scoped correction for #16605's unrelated CI failure. It changes one test file. ## What Changed - Retry only pre-READY gateway exits with code 1 and the specific `EADDRINUSE` bind diagnostic. - Wait for child close and fully drained stderr before classifying startup. Kill failed attempts and preserve the original failure after three collisions. - Keep the real probe socket open to inject collisions without a timing race. - Verify successful recovery, bounded exhaustion, delayed stderr, and immediate failure for unrelated startup errors. Accept successful starts anywhere within the three-attempt limit. Preserve the original authorization and header-stripping assertions. ## Verification - Negative controls: the held-port authorization case fails with `EADDRINUSE`, and retry exhaustion observes only one attempt before the correction. The ordinary case passes. - `pnpm exec vitest run packages/adapter-utils/src/sandbox-callback-bridge.test.ts`: all 89 tests pass on the final head. - `pnpm --filter @paperclipai/adapter-utils typecheck`: passes. - Initial standard Greptile returned 4/5 for late stderr classification and exact success-count assertions. Both are corrected. The final real-pipe control reproduces missing diagnostics with the old exit listener and passes with close-based readiness. An initial pause-only control was not discriminating because Node resumes pipes on exit; the final control holds that resume until after exit. - Diff checks and the complete secrets/PII review pass. - Full local repository tests and build were not run for this one-file fixture change. Hosted CI and standard Greptile are pending. ## Risks The fixture can still fail after three consecutive collisions. Other startup failures remain failures. The generated gateway and its assigned-port protocol are unchanged. ## Model Used OpenAI Codex, GPT-6, with reasoning, repository inspection, code execution, and testing. The exact deployment model ID and context window are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../src/sandbox-callback-bridge.test.ts | 62 +++++++++++++++++-- 1 file changed, 57 insertions(+), 5 deletions(-) diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts index b2c8c11edbe..a636116e084 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts @@ -3292,8 +3292,10 @@ describe("sandbox callback bridge", () => { async function startHttp2GatewayForTest(options: { bridgeToken: string; maxBodyBytes?: number; + holdProbeOpen?: () => boolean; + deferStderrUntilExit?: boolean; forwardRequest: (request: Http2BridgeForwardRequest) => Promise; - }): Promise<{ baseUrl: string; stop: () => Promise }> { + }, attempt = 0): Promise<{ baseUrl: string; stop: () => Promise }> { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-http2-test-")); cleanupDirs.push(rootDir); const entrypoint = path.join(rootDir, "paperclip-bridge-server.mjs"); @@ -3308,7 +3310,12 @@ describe("sandbox callback bridge", () => { reject(new Error("Could not reserve a loopback port for the test.")); return; } - probe.close(() => resolve(address.port)); + if (options.holdProbeOpen?.()) { + cleanupFns.push(() => new Promise((resolve) => probe.close(() => resolve()))); + resolve(address.port); + } else { + probe.close(() => resolve(address.port)); + } }); }); @@ -3330,6 +3337,16 @@ describe("sandbox callback bridge", () => { stderr += chunk.toString("utf8"); }); + if (options.deferStderrUntilExit) { + const resume = child.stderr.resume.bind(child.stderr); + child.stderr.pause(); + child.stderr.resume = () => child.stderr; + child.once("exit", () => setImmediate(() => { + child.stderr.resume = resume; + resume(); + })); + } + const dataListeners: Array<(chunk: Uint8Array) => void> = []; const pending: Buffer[] = []; let dispatchStarted = false; @@ -3367,7 +3384,7 @@ describe("sandbox callback bridge", () => { } deliver(chunk); }); - child.once("exit", (code) => { + child.once("close", (code) => { clearTimeout(readyTimer); if (!sawReadyLine) { rejectReady(new Error("The http2 gateway exited early with code " + String(code) + ". stderr: " + stderr)); @@ -3392,7 +3409,16 @@ describe("sandbox callback bridge", () => { }, }; - await readyPromise; + try { + await readyPromise; + } catch (error) { + child.kill(); + if (attempt < 2 && child.exitCode === 1 + && stderr.includes("http2 gateway could not bind port") && stderr.includes("EADDRINUSE")) { + return startHttp2GatewayForTest(options, attempt + 1); + } + throw error; + } const handle = createHttp2BridgeServer({ bridgeToken: options.bridgeToken, @@ -3828,15 +3854,38 @@ describe("sandbox callback bridge", () => { expect(seenRequests[0]?.body).toBe(requestBodyText); }); - it("accepts the bridge token in X-API-Key on the HTTP/2 path and strips the header", async () => { + it("does not retry unrelated HTTP/2 fixture startup failures", async () => { + let attempts = 0; + await expect(startHttp2GatewayForTest({ + bridgeToken: "", + holdProbeOpen: () => { attempts++; return false; }, + forwardRequest: async () => { throw new Error("Unexpected request to an unstarted gateway"); }, + })).rejects.toThrow("PAPERCLIP_BRIDGE_TOKEN"); + expect(attempts).toBe(1); + }); + + it("limits HTTP/2 fixture retries when every assigned port is occupied", async () => { + let attempts = 0; + await expect(startHttp2GatewayForTest({ + bridgeToken: createSandboxCallbackBridgeToken(), + holdProbeOpen: () => { attempts++; return true; }, + forwardRequest: async () => { throw new Error("Unexpected request to an unstarted gateway"); }, + })).rejects.toThrow("EADDRINUSE"); + expect(attempts).toBe(3); + }, 15000); + + it.each([[false, false], [true, false], [true, true]])("accepts the bridge token in X-API-Key on the HTTP/2 path and strips the header (collision=%s, late stderr=%s)", async (portCollision, lateStderr) => { // Both generated gateways share one token reader, so the HTTP/2 transport // must accept "X-API-Key" exactly as the file-mode transport does. The // header allowlist must still strip the credential before the gateway // forwards the request to the host. const bridgeToken = createSandboxCallbackBridgeToken(); const seenHeaders: Array> = []; + let attempts = 0; const gateway = await startHttp2GatewayForTest({ bridgeToken, + holdProbeOpen: () => { attempts++; return portCollision && attempts === 1; }, + deferStderrUntilExit: lateStderr, forwardRequest: async (request) => { seenHeaders.push(request.headers); return { @@ -3847,6 +3896,9 @@ describe("sandbox callback bridge", () => { }, }); + expect(attempts).toBeGreaterThanOrEqual(portCollision ? 2 : 1); + expect(attempts).toBeLessThanOrEqual(3); + const response = await fetch(`${gateway.baseUrl}/api/agents/me`, { headers: { "x-api-key": bridgeToken,