mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 08:22:20 +02:00
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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
313fb265db
commit
2e5bf12411
1 file changed
+57
-5
@@ -3292,8 +3292,10 @@ describe("sandbox callback bridge", () => {
|
||||
async function startHttp2GatewayForTest(options: {
|
||||
bridgeToken: string;
|
||||
maxBodyBytes?: number;
|
||||
holdProbeOpen?: () => boolean;
|
||||
deferStderrUntilExit?: boolean;
|
||||
forwardRequest: (request: Http2BridgeForwardRequest) => Promise<Http2BridgeForwardResult>;
|
||||
}): Promise<{ baseUrl: string; stop: () => Promise<void> }> {
|
||||
}, attempt = 0): Promise<{ baseUrl: string; stop: () => Promise<void> }> {
|
||||
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<void>((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<Record<string, string>> = [];
|
||||
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,
|
||||
|
||||
Reference in new issue
Block a user