From 8da49d6ea7210a5742142dceea42505a762ffbcd Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 28 Aug 2026 13:01:59 -0500 Subject: [PATCH] fix(ci): make runner image tests deterministic (#12452) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Pull request checks protect the runtime and authorization boundaries > - The same checks must produce the same result on GitHub and RunsOn Ubuntu images > - One runtime test assumed that a shell PID always owns the listening socket > - One watchdog test denied issue reads while it tried to test assignment denial > - These assumptions caused image-sensitive failures during the AWS runner canary > - This pull request tests the production contracts directly > - The benefit is a reliable CI result across both runner images ## Linked Issues or Issue Description Refs #12350 ## What Changed - Verify stale service ownership through the existing process-group ownership helper. - Allow normal issue reads in the watchdog reassignment fixture. - Assert that the watchdog reassignment reaches and denies the `tasks:assign` guard. ## Verification - `pnpm --filter @paperclipai/server typecheck` - `pnpm exec vitest run server/src/__tests__/workspace-runtime.test.ts -t "does not reuse a stopped auto-port service port while another process owns it"` - `pnpm exec vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts -t "still enforces normal assignment guards for watchdog reassignment"` - Ran the complete issue agent mutation ownership suite six times. All 522 test executions passed. - Ran the runtime regression case eight times. All eight test executions passed. ## Risks - Low risk. This pull request changes test fixtures and assertions only. - The process-group assertion matches the ownership rule that the runtime already uses. - The watchdog fixture still denies `issue:mutate` and `tasks:assign`. ## Model Used - OpenAI Codex with GPT-5.6 (`gpt-5.6-sol`). The model used reasoning, tool use, and code execution. ## 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, or Refs OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal or instance-local Paperclip issues or links - [x] My branch name describes the change and contains no internal Paperclip ticket id - [x] I have run relevant tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes. No documentation change is required for this test-only fix. - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- .../issue-agent-mutation-ownership-routes.test.ts | 10 ++++++++-- server/src/__tests__/workspace-runtime.test.ts | 7 ++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 4103989346..1db628e3f9 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -2369,9 +2369,12 @@ describe("agent issue mutation checkout ownership", () => { // Base boundary denied AND tasks:assign denied: the watchdog grant lets the // mutation past the ownership boundary, but the assignment guard must still bite. mockAccessService.decide.mockImplementation(async (input: { action: string }) => ({ - allowed: input.action === "company_scope:read", + allowed: input.action === "company_scope:read" || input.action === "issue:read", action: input.action, - reason: input.action === "company_scope:read" ? "allow_explicit_grant" : "deny_policy_restricted", + reason: + input.action === "company_scope:read" || input.action === "issue:read" + ? "allow_explicit_grant" + : "deny_policy_restricted", explanation: input.action === "tasks:assign" ? "Target agent requires approval before task assignment." @@ -2385,6 +2388,9 @@ describe("agent issue mutation checkout ownership", () => { expect(res.status, JSON.stringify(res.body)).toBe(403); expect(res.body.error).toContain("requires approval"); + expect(mockAccessService.decide).toHaveBeenCalledWith( + expect.objectContaining({ action: "tasks:assign" }), + ); expect(mockIssueService.update).not.toHaveBeenCalled(); }); diff --git a/server/src/__tests__/workspace-runtime.test.ts b/server/src/__tests__/workspace-runtime.test.ts index 577f0365d0..3693fccf95 100644 --- a/server/src/__tests__/workspace-runtime.test.ts +++ b/server/src/__tests__/workspace-runtime.test.ts @@ -8238,7 +8238,12 @@ describeEmbeddedPostgres("workspace runtime startup reconciliation", () => { expect(services[0]?.url).not.toBe(rootUrl); await expect(fetch(services[0]!.url!)).resolves.toMatchObject({ ok: true }); await expect(fetch(healthUrl)).resolves.toMatchObject({ ok: false, status: 503 }); - expect(await readLocalServicePortOwner(stalePort!)).toBe(staleProcess.pid); + const stalePortOwnerPid = await readLocalServicePortOwner(stalePort!); + expect(stalePortOwnerPid).not.toBeNull(); + expect(staleProcess.pid).toBeTypeOf("number"); + await expect( + isLocalServiceProcessOwnedBy(stalePortOwnerPid!, staleProcess.pid!), + ).resolves.toBe(true); } finally { leasedRunIds.delete(runId); await releaseRuntimeServicesForRun(runId);