From e8b78cce5bb557b086bee9ca930eeaab1d09b244 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Wed, 28 Oct 2026 23:15:10 -0700 Subject: [PATCH] Isolate browser fixtures and reserve HTTPS test ports (#18128) https://entire.com/gh/paperclipai/-/trails/234/changes/paperclip/234 ## Thinking Path > - Paperclip manages agents and their tasks. > - CI verifies runtime setup and task execution in browsers. > - Shared fixtures and port collisions can fail unrelated changes. > - Copying configuration during navigation can observe incomplete state. > - This pull request isolates fixtures and waits for completed reloads. > - These changes make the existing checks more reliable. ## Linked Issues or Issue Description Extracted from #18050 so test stability can be reviewed on its own. Searches for signoff fixture and HTTPS backfill port fixes found no separate duplicate of these three edits. **What happened?** The signoff suite shares company and agent state across scenarios. The HTTPS backfill fixture probes only one listener and can select ephemeral client ports. The YAML reload test reads before navigation and reload readiness are complete. **Expected behavior** Each scenario has its own state. Both app and HMR ports are available. The YAML test reads after the canonical route and reload action are ready. **Steps to reproduce** Run the signoff scenarios in sequence, run the HTTPS backfill test with other local sockets, or run the YAML lost-response reload case during navigation. ## What Changed - Create and clean up signoff company and agent fixtures for each scenario. - Probe both app and HMR ports below the ephemeral client range. - Wait for the canonical agent route and enabled reload action before checking the saved document. ## Verification - These exact fixture changes passed the selected HTTPS test and all 12 YAML/signoff browser cases in the combined change. Those results apply to that earlier source. - Fresh local checks and CI for this isolated branch are pending. No full pass is claimed for this new head yet. - No production source or dependency change is included. ## Risks Test isolation creates more fixture companies. Each case cleans its fixtures. Product behavior and assertions stay the same. No user documentation change is needed. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository tools and code execution. The exact serving snapshot is not exposed. ## 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 - [ ] 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/__tests__/workspace-runtime.test.ts | 19 ++++--------------- tests/e2e/agent-configuration.spec.ts | 3 ++- tests/e2e/signoff-policy.spec.ts | 4 ++-- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index c3d17eae425..d870f42b295 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -7974,22 +7974,11 @@ describeEmbeddedPostgres("workspace runtime startup reconciliation", () => { const reservePort = async () => { for (let attempt = 0; attempt < 100; attempt += 1) { - const probe = net.createServer(); - // macOS can allocate an entire ephemeral range above 55535. Pick a - // bounded candidate so the test's HMR companion remains a valid port. - await new Promise((resolve) => { - probe.once("error", () => resolve()); - probe.listen(20_000 + Math.floor(Math.random() * 20_000), "127.0.0.1", resolve); - }); - if (!probe.listening) continue; - const address = probe.address(); - const port = typeof address === "object" && address ? address.port : null; - await new Promise((resolve, reject) => { - probe.close((error) => error ? reject(error) : resolve()); - }); - if (port && port <= 55_535 && (port < 42_000 || port > 42_999)) return port; + // Keep both listeners below ephemeral client ports and outside the broker range. + const port = 12_000 + Math.floor(Math.random() * 6_000); + if (await isLoopbackPortFree(port) && await isLoopbackPortFree(port + 10_000)) return port; } - throw new Error("Failed to reserve an HTTPS backfill test port outside the broker range"); + throw new Error("Failed to reserve app and HMR ports for the HTTPS backfill test"); }; const isLoopbackPortFree = async (port: number) => { const probe = net.createServer(); diff --git a/tests/e2e/agent-configuration.spec.ts b/tests/e2e/agent-configuration.spec.ts index 10a43c88b2f..f0ce7810e32 100644 --- a/tests/e2e/agent-configuration.spec.ts +++ b/tests/e2e/agent-configuration.spec.ts @@ -218,8 +218,9 @@ test("a lost creation response requires reload and cannot blindly replay command }, { capture: true, once: true })); expect((await document(page)).spec.apiKeys!.create).toHaveLength(2); await page.getByRole("button", { name: "Reload saved", exact: true }).click(); - await expect.poll(async () => (await document(page)).spec.apiKeys!.create).toEqual([]); await expect(page).toHaveURL(/\/agents\/partially-renamed-yaml-engineer\/advanced$/); + await expect(page.getByRole("button", { name: "Reload saved", exact: true })).toBeEnabled(); + expect((await document(page)).spec.apiKeys!.create).toEqual([]); await expect(page.getByRole("textbox", { name: "API key: First key token" })).toBeVisible(); await edit(page, value => { value.spec.title = "Recovered save"; }); await save(page); diff --git a/tests/e2e/signoff-policy.spec.ts b/tests/e2e/signoff-policy.spec.ts index 31276258904..a9483e31bb8 100644 --- a/tests/e2e/signoff-policy.spec.ts +++ b/tests/e2e/signoff-policy.spec.ts @@ -589,12 +589,12 @@ test("a deferred signoff wake ignores cancelled history from an earlier stage", test.describe("Signoff execution policy", () => { let ctx: TestContext; - test.beforeAll(async () => { + test.beforeEach(async () => { const boardRequest = await pwRequest.newContext({ baseURL: BASE_URL }); ctx = await setupCompany(boardRequest); }); - test.afterAll(async () => { + test.afterEach(async () => { if (!ctx) return; const board = ctx.boardRequest;