From a0e5166e974a7f6ffabe2a43f816a83449fdef57 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:39:21 -0700 Subject: [PATCH] Fix GitHub journey registration diagnostics and native retry cleanup (#17513) ## Thinking Path > - Paperclip manages work performed by AI agents. > - The Product E2E harness verifies a GitHub review-bot journey. > - GitHub can return a registration page without the expected creation control. > - The journey could then wait for the full login allowance without useful evidence. > - A failed native run can also retain a retry after its terminal status is visible. > - This change records a safe registration boundary and uses the normal Stop route for owned retries. > - Operators can diagnose the setup failure and confirm cleanup without changing the behavioral oracle. ## Linked Issues or Issue Description Refs: #17457. **What happened?** The GitHub review journey waited at App registration with no useful handoff evidence. Its cleanup also skipped failed native runs that still owned a durable retry. **Expected behavior** The journey should report a bounded registration failure or reauthentication boundary. Cleanup should Stop an owned native retry and require its audited acknowledgement. **Steps to reproduce** 1. Run the configured GitHub review journey. 2. Return a GitHub registration page without the expected creation button, or fail a native review run while its coordinator can retry. 3. Inspect the result and cleanup evidence. **Paperclip version or commit** Source built from current master. This extends the driver merged in #17457. ## What Changed - Bound the registration render wait to 30 seconds. Keep the separate attended confirmation allowance. - Retain GitHub origin, path, and control-presence flags. Retain headings, button labels, and alerts only on known registration error pages without authentication fields. - Exclude query parameters, entered values, page bodies, grant payloads, and authentication content from that diagnostic. - Add an optional private authentication hook for ordinary GitHub access reconfirmation. Detect passkey-first sudo at the manifest URL without retaining its authentication content. - Stop owned failed native retries through the existing run-only cancellation route. Verify company, agent, task, and audit acknowledgement. - Use native Stop correlation only for native runs. Queued legacy or unresolved runs use the ordinary cancel request. Stop scheduled retries and wait for them to leave the live state set. - Keep pending accounting debt intact. Add regression coverage and journey documentation. ## Verification - Focused journey and report suite: 58 tests passed. - Runner E2E TypeScript check passed. - `git diff --check` passed. Credential-value scan passed. - Live fixture reproduced the missing creation control at the manifest handoff. The new diagnostic retained its path and control-presence flags. No model work started. Draft cleanup passed. - All 58 current-head checks are terminal and green. Greptile scored the exact head 5/5, with no unresolved review threads. ## Risks GitHub pages that take more than 30 seconds to render the creation control now fail early. Native cleanup fails closed if the server does not provide the expected Stop acknowledgement. The live setup-to-Done behavioral journey is still unqualified. These changes do not repair a missing provider usage receipt or weaken its accounting gate. ## Model Used OpenAI Codex, based on GPT-6. Exact runtime model identifier and context-window size are not exposed in this session. Used for code execution, browser tooling, and regression analysis. ## 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 - [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 --------- Co-authored-by: Paperclip Co-authored-by: Paperclip --- tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md | 9 ++ tests/runner-e2e/github-review-browser.ts | 48 +++++++++ tests/runner-e2e/github-review-flow.ts | 8 +- .../runner-e2e/github-review-journey.test.ts | 101 +++++++++++++++++- tests/runner-e2e/github-review-launch.ts | 19 ++-- tests/runner-e2e/github-review-ownership.ts | 31 ++++++ 6 files changed, 203 insertions(+), 13 deletions(-) diff --git a/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md b/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md index ceeb56a95e0..12cddb6c387 100644 --- a/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md +++ b/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md @@ -89,6 +89,15 @@ connections, and completed task evidence. An unconfirmed cleanup fails the attempt; retain the resource receipt for recovery. Read the receipt before any manual cleanup. Never delete by a guessed name or broad account search. +Registration render waits are bounded separately from attended confirmation. +The driver retains only the GitHub origin/path and control-presence flags when +the creation control is missing. On a known registration error page without auth +fields, it also retains headings, button labels, and alerts. It excludes query +parameters and authentication content. Reauthentication is a setup boundary, +not evidence that an App exists. +Cleanup also stops failed native runs that still own a retry, and requires the +normal run-only Stop acknowledgement. It never clears pending accounting debt. + The configured deadline and observed run limit bound the attempt. They are not a hard provider-dollar cap or proof that no request outlives cancellation. Use the existing budgeted Cloud operations adapter before automated paid campaigns; diff --git a/tests/runner-e2e/github-review-browser.ts b/tests/runner-e2e/github-review-browser.ts index dffb516bcdd..b064d0ad384 100644 --- a/tests/runner-e2e/github-review-browser.ts +++ b/tests/runner-e2e/github-review-browser.ts @@ -10,6 +10,54 @@ export async function enterGitHubJourneyCompany(page: Page, origin: string, comp await page.waitForURL(url => url.origin === origin && url.pathname === pathname); } +export interface GitHubRegistrationBoundary { + origin: string; pathname: string; passwordRequired: boolean; oneTimeCodeRequired: boolean; + reauthenticationRequired: boolean; createControlVisible: boolean; + controls?: { headings: string[]; buttons: string[]; alerts: string[] }; +} + +/** Keep only navigation and control-presence evidence. Authentication fields, + * page content, query parameters, and grant payloads must stay out of reports. */ +export async function waitForGitHubRegistrationControl(page: Page, owner: string, timeoutMs: number, + record: (boundary: GitHubRegistrationBoundary) => Promise = async () => {}, reauthenticate?: () => Promise) { + const control = page.getByRole("button", { name: new RegExp(`^Create GitHub App(?: for ${owner})?$`) }); + const observe = async (includeErrorControls = false) => { + const url = new URL(page.url()); + if (url.origin !== "https://github.com") throw new ConnectionFailure("github_registration_origin_mismatch"); + const boundary: GitHubRegistrationBoundary = { origin: url.origin, pathname: url.pathname, + passwordRequired: await page.locator('input[type="password"]:visible').count() > 0, + oneTimeCodeRequired: await page.locator('input[autocomplete="one-time-code"]:visible').count() > 0, + reauthenticationRequired: await page.getByRole("heading", { name: /^Confirm access$/i }).isVisible() + || await page.getByRole("button", { name: /^Use (?:your password|passkey)$/i }).isVisible(), + createControlVisible: await control.isVisible() }; + boundary.reauthenticationRequired ||= boundary.passwordRequired || boundary.oneTimeCodeRequired + || /\/(?:login|sessions)(?:\/|$)/.test(boundary.pathname); + if (includeErrorControls && !boundary.reauthenticationRequired + && ["/settings/apps/new", "/settings/apps/manifest"].includes(url.pathname)) { + boundary.controls = { headings: await page.getByRole("heading").allTextContents(), + buttons: await page.getByRole("button").allTextContents(), alerts: await page.getByRole("alert").allTextContents() }; + } + await record(boundary); + return boundary; + }; + await observe(); + let authenticationAttempted = false; + for (;;) { + try { await control.waitFor({ timeout: Math.min(timeoutMs, 30_000) }); break; } + catch (error) { + if (page.isClosed()) throw error; + const boundary = await observe(true); + if (boundary.reauthenticationRequired) { + if (reauthenticate && !authenticationAttempted) { authenticationAttempted = true; await reauthenticate(); continue; } + throw new ConnectionBlock("awaiting_user", "github_registration_reauthentication_required"); + } + throw new ConnectionFailure("github_registration_create_control_missing"); + } + } + await observe(); + return control; +} + /** Capture measured pages only, excluding authentication and unrelated GitHub data. */ export function isGitHubJourneyEvidencePage(value: string, targetOrigin: string, repository: string, appSlug?: string) { const url = new URL(value); diff --git a/tests/runner-e2e/github-review-flow.ts b/tests/runner-e2e/github-review-flow.ts index c580ed767fa..5547adf21a2 100644 --- a/tests/runner-e2e/github-review-flow.ts +++ b/tests/runner-e2e/github-review-flow.ts @@ -3,7 +3,7 @@ import type { GitHubTaskReview } from "../../packages/shared/src/types/chat-gith import type { GitHubRepositoryPage } from "../../packages/shared/src/types/chat-channels.js"; import { ConnectionBlock, ConnectionFailure } from "./connection-config.js"; import { ConnectionApi } from "./connection-target.js"; -import { awaitJourneyCondition, createGitHubJourneyPullRequest, enterGitHubJourneyCompany, observeGitHubHead, selectGitHubJourneyRepository } from "./github-review-browser.js"; +import { awaitJourneyCondition, createGitHubJourneyPullRequest, enterGitHubJourneyCompany, observeGitHubHead, selectGitHubJourneyRepository, waitForGitHubRegistrationControl, type GitHubRegistrationBoundary } from "./github-review-browser.js"; import { githubCartFixture, hasOnlyOpenedAutomaticReview, matchesGitHubJourneyEnvironment, retainGitHubReview, verifyGitHubReview, verifyGitHubReviewRun, type GitHubReviewJourneyEvidence } from "./github-review-evidence.js"; import type { GitHubReviewJourneyConfig } from "./github-review-config.js"; import { captureOwnedGitHubApp, readOwnedGitHubReviewRuns, type GitHubJourneyResources } from "./github-review-ownership.js"; @@ -15,6 +15,8 @@ export async function runGitHubReviewFlow(input: { checkpoint(id: keyof GitHubReviewJourneyEvidence["checkpoints"], page: Page): Promise; phase(id: string): Promise; persistResources(): Promise; + recordRegistrationBoundary?(boundary: GitHubRegistrationBoundary): Promise; + authenticateRegistration?(page: Page): Promise; }) { const { page, github, api, config, resources, evidence } = input; const capabilities = await api.get('/api/instance/settings/experimental'); @@ -69,8 +71,8 @@ export async function runGitHubReviewFlow(input: { // Manifest flow navigates the same tab. External App grants remain attended. evidence.assisted = true; await awaitJourneyCondition(async () => new URL(page.url()).hostname === "github.com", 30_000, "github_manifest_handoff_missing", input.signal); - const createApp = page.getByRole("button", { name: new RegExp(`^Create GitHub App(?: for ${config.github.appOwner})?$`) }); - await createApp.waitFor({ timeout: config.browser.loginTimeoutMs }); + const createApp = await waitForGitHubRegistrationControl(page, config.github.appOwner, config.browser.loginTimeoutMs, + input.recordRegistrationBoundary, input.authenticateRegistration ? () => input.authenticateRegistration!(page) : undefined); await page.bringToFront(); await input.checkpoint("app_creation_consent", page); console.log(`[github-review-journey] Authorize creation of only ${resources.appName} in the dedicated QA browser. Install it only on ${config.github.repository}.`); diff --git a/tests/runner-e2e/github-review-journey.test.ts b/tests/runner-e2e/github-review-journey.test.ts index 8489b4596b0..09ce3e869dd 100644 --- a/tests/runner-e2e/github-review-journey.test.ts +++ b/tests/runner-e2e/github-review-journey.test.ts @@ -8,9 +8,9 @@ import type { GitHubTaskReview } from "../../packages/shared/src/types/chat-gith import { verifyConnectionTarget } from "./connection-target.js"; import { parseConnectionConfig } from "./connection-config.js"; import type { APIRequestContext } from "@playwright/test"; -import { captureOwnedGitHubApp, readOwnedGitHubReviewRuns, type GitHubJourneyResources } from "./github-review-ownership.js"; +import { captureOwnedGitHubApp, readOwnedGitHubReviewRuns, stopOwnedGitHubReviewRun, type GitHubJourneyResources } from "./github-review-ownership.js"; import type { ChatEndpoint } from "../../packages/shared/src/types/chat-channels.js"; -import { enterGitHubJourneyCompany, isGitHubJourneyEvidencePage } from "./github-review-browser.js"; +import { enterGitHubJourneyCompany, isGitHubJourneyEvidencePage, waitForGitHubRegistrationControl } from "./github-review-browser.js"; import { attendGitHubJourneyAction } from "./github-review-attendance.js"; import { PassThrough } from "node:stream"; import type { Locator, Page } from "@playwright/test"; @@ -19,7 +19,104 @@ import { runGitHubReviewCampaign } from "./github-review-launch.js"; import type { ConnectionApi } from "./connection-target.js"; import { runGitHubReviewFlow } from "./github-review-flow.js"; +describe("GitHub registration handoff", () => { + const page = (pathname: string, password: boolean, available: boolean) => ({ + url: () => `https://github.com${pathname}?return_to=private-grant&token=never-retain`, isClosed: () => false, + locator: (selector: string) => ({ count: async () => selector.includes('password') && password ? 1 : 0 }), + getByRole: (_role?: string, options?: { name?: string | RegExp }) => ({ isVisible: async () => String(options?.name).includes('Create GitHub App') && available, allTextContents: async () => ['Registration error'], + waitFor: vi.fn(async () => { if (!available) throw new Error("control_missing"); }) }), + }); + it("classifies a reauthentication boundary and retains no query or auth content", async () => { + const record = vi.fn(); + await expect(waitForGitHubRegistrationControl(page('/sessions/sudo', true, false) as unknown as Page, 'fixture', 600000, record)) + .rejects.toThrow('github_registration_reauthentication_required'); + expect(record.mock.calls.at(-1)?.[0]).toEqual({ origin: 'https://github.com', pathname: '/sessions/sudo', passwordRequired: true, oneTimeCodeRequired: false, reauthenticationRequired: true, createControlVisible: false }); + expect(JSON.stringify(record.mock.calls)).not.toMatch(/private-grant|never-retain/); + }); + it("fails a missing creation control on an ordinary registration page", async () => { + const record = vi.fn(); + await expect(waitForGitHubRegistrationControl(page('/settings/apps/manifest', false, false) as unknown as Page, 'fixture', 600000, record)) + .rejects.toThrow('github_registration_create_control_missing'); + expect(record.mock.calls.at(-1)?.[0].controls.alerts).toEqual(['Registration error']); + }); + it("admits the ordinary creation control with a bounded render wait", async () => { + const target = page('/settings/apps/new', false, true); + const control = target.getByRole('button', { name: /^Create GitHub App/ }); + const get = target.getByRole; + target.getByRole = (role, options) => String(options?.name).includes('Create GitHub App') ? control : get(role, options); + await expect(waitForGitHubRegistrationControl(target as unknown as Page, 'fixture', 600000)).resolves.toBe(control); + expect(control.waitFor).toHaveBeenCalledWith({ timeout: 30000 }); + }); + it("keeps passkey-first sudo at the manifest path out of authentication content diagnostics", async () => { + const target = page('/settings/apps/manifest', false, false), get = target.getByRole; + target.getByRole = (role, options) => String(options?.name).includes('Confirm access') + ? { ...get(role, options), isVisible: async () => true } : get(role, options); + const record = vi.fn(); + await expect(waitForGitHubRegistrationControl(target as unknown as Page, 'fixture', 600000, record)) + .rejects.toThrow('github_registration_reauthentication_required'); + expect(record.mock.calls.at(-1)?.[0].reauthenticationRequired).toBe(true); + expect(record.mock.calls.at(-1)?.[0].controls).toBeUndefined(); + }); + it("uses a supplied ordinary reauthentication adapter once before retrying creation readiness", async () => { + const target = page('/settings/apps/manifest', true, false), get = target.getByRole; + let authenticated = false; + const control = { ...get(), isVisible: async () => authenticated, + waitFor: vi.fn(async () => { if (!authenticated) throw new Error('sudo_required'); }) }; + target.getByRole = (role, options) => String(options?.name).includes('Create GitHub App') ? control : get(role, options); + const authenticate = vi.fn(async () => { authenticated = true; }); + await expect(waitForGitHubRegistrationControl(target as unknown as Page, 'fixture', 600000, async () => {}, authenticate)).resolves.toBe(control); + expect(authenticate).toHaveBeenCalledOnce(); + }); +}); + describe("reused target ownership", () => { + it("stops a scheduled legacy retry and waits until its cancellation finishes", async () => { + const scope = { companyId: 'company', agentId: 'agent', issueId: 'issue' }; + const scheduled = { ...scope, id: 'run', status: 'scheduled_retry', runtimeMode: 'legacy' }; + const get = vi.fn().mockResolvedValueOnce(scheduled).mockResolvedValueOnce(scheduled) + .mockResolvedValue({ ...scheduled, status: 'cancelled' }); + const post = vi.fn().mockResolvedValue({}); + await stopOwnedGitHubReviewRun({ get, post } as unknown as ConnectionApi, 'run', scope, new AbortController().signal); + expect(post).toHaveBeenCalledExactlyOnceWith('/api/heartbeat-runs/run/cancel', {}); + expect(get).toHaveBeenCalledTimes(3); + }); + it.each(['legacy', undefined])("stops a queued %s run without a native-only cancellation request", async runtimeMode => { + const scope = { companyId: 'company', agentId: 'agent', issueId: 'issue' }; + const queued = { ...scope, id: 'run', status: 'queued', runtimeMode }; + const get = vi.fn().mockResolvedValueOnce(queued).mockResolvedValue({ ...queued, status: 'cancelled' }); + const post = vi.fn(async (_route: string, body: Record) => { + if (body.cancellationRequestId) throw new Error('Correlated Stop requires a native run'); + return {}; + }); + await stopOwnedGitHubReviewRun({ get, post } as unknown as ConnectionApi, 'run', scope, new AbortController().signal); + expect(post).toHaveBeenCalledExactlyOnceWith('/api/heartbeat-runs/run/cancel', {}); + expect(get).toHaveBeenCalledTimes(2); + }); + it.each([true, false])("stops a failed native retry and verifies its audited acknowledgement (dispatched=%s)", async dispatched => { + const scope = { companyId: 'company', agentId: 'agent', issueId: 'issue' }; + const failed = { ...scope, id: 'run', status: 'failed', runtimeMode: 'native', nativePhase: 'retryable_failure' }; + const get = vi.fn().mockResolvedValueOnce(failed).mockResolvedValue({ ...failed, status: 'cancelled', costAccountingPending: true, + resultJson: { nativeCancellation: { schema: 'paperclip.native-cancellation.v1', runId: 'run', companyId: 'company', issueId: 'issue', scope: 'run', reasonCode: 'cancellation_run_only', dispatched, dispatchState: 'acknowledged', intentAuditId: 'intent', acknowledgementAuditId: 'ack' } } }); + const post = vi.fn().mockResolvedValue({}); + await stopOwnedGitHubReviewRun({ get, post } as unknown as ConnectionApi, 'run', scope, new AbortController().signal); + expect(post).toHaveBeenCalledExactlyOnceWith('/api/heartbeat-runs/run/cancel', { cancellationRequestId: expect.any(String) }); + expect(get).toHaveBeenCalledTimes(2); + }); + it("refuses to stop a related run belonging to a different task", async () => { + const post = vi.fn(); + const api = { get: async () => ({ id: 'run', companyId: 'company', agentId: 'agent', issueId: 'other', status: 'running' }), post }; + await expect(stopOwnedGitHubReviewRun(api as unknown as ConnectionApi, 'run', { companyId: 'company', agentId: 'agent', issueId: 'issue' }, new AbortController().signal)) + .rejects.toThrow('cleanup_run_owner_mismatch'); + expect(post).not.toHaveBeenCalled(); + }); + it("does not accept cancelled status as native Stop acknowledgement", async () => { + const scope = { companyId: 'company', agentId: 'agent', issueId: 'issue' }; + const get = vi.fn().mockResolvedValueOnce({ ...scope, id: 'run', status: 'running', runtimeMode: 'native' }) + .mockResolvedValue({ ...scope, id: 'run', status: 'cancelled', runtimeMode: 'native' }); + await expect(stopOwnedGitHubReviewRun({ get, post: async () => ({}) } as unknown as ConnectionApi, 'run', scope, new AbortController().signal, 1)) + .rejects.toThrow('owned_run_cancellation_unconfirmed'); + expect(get.mock.calls.length).toBeGreaterThan(1); + }); it("admits an inherited environment before setup and still leaves actual-run verification mandatory", async () => { const setup = parseGitHubReviewJourneyConfig(config()); const company = { id: setup.target.companyId, issuePrefix: "QA" }; diff --git a/tests/runner-e2e/github-review-launch.ts b/tests/runner-e2e/github-review-launch.ts index a78365f6add..83f4667d90c 100644 --- a/tests/runner-e2e/github-review-launch.ts +++ b/tests/runner-e2e/github-review-launch.ts @@ -12,7 +12,7 @@ import { createPrivateJourneyRepository, ensureGitHubJourneyLogin, closeOwnedGit import { loadGitHubReviewJourneyConfig } from "./github-review-config.js"; import { gradeGitHubReviewJourney, type GitHubReviewJourneyEvidence } from "./github-review-evidence.js"; import { runGitHubReviewFlow } from "./github-review-flow.js"; -import { captureOwnedGitHubApp, readOwnedGitHubReviewRuns, type GitHubJourneyResources } from "./github-review-ownership.js"; +import { captureOwnedGitHubApp, readOwnedGitHubReviewRuns, stopOwnedGitHubReviewRun, type GitHubJourneyResources } from "./github-review-ownership.js"; import { JourneyDependencyError, resolveJourneyCredential } from "./journey-dependencies.js"; import { sanitizeJson, assertSecretFree } from "./redaction.js"; import { writeGitHubReviewReport } from "./github-review-report.js"; @@ -101,7 +101,14 @@ export async function runGitHubReviewCampaign(input: { executions: readonly Matr const identity = await verifyConnectionTarget(config.target.baseURL, common, browser.context.request); evidence.targetCommit = identity.commit; evidence.checkpoints.target = true; await runGitHubReviewFlow({ page, github, githubUsername: config.github.account, api, config, resources, evidence, signal, - phase: checkpoint, persistResources: () => save("owned-resources.json", resources), checkpoint: async (id, tab) => { + phase: checkpoint, persistResources: () => save("owned-resources.json", resources), + ...(typeof authentication?.authenticateGitHubJourneyRegistration === "function" ? { + authenticateRegistration: (tab: Page) => authentication.authenticateGitHubJourneyRegistration({ page: tab, + configFile: config!.authentication!.configFile, expectedAccount: config!.github.account, signal, + deadlineAt: Date.now() + Math.min(config!.browser.loginTimeoutMs, 120_000), + registerSecret: (value: string) => secrets.push(value), checkpoint: (id: string) => checkpoint(id) }), + } : {}), + recordRegistrationBoundary: boundary => save("github-registration-boundary.json", boundary), checkpoint: async (id, tab) => { evidence.checkpoints[id] = true; // Authentication pages and forms are deliberately never captured. const consentUrl = new URL(tab.url()); @@ -180,12 +187,8 @@ export async function runGitHubReviewCampaign(input: { executions: readonly Matr await save("owned-resources.json", resources); await api.post(`/api/chat-endpoints/${resources.endpointId}/setup`, { action: "remove" }); for (const runId of resources.runIds) { - const run = await api.get(`/api/heartbeat-runs/${runId}`); - if (run.companyId !== config!.target.companyId || run.agentId !== config!.agent.id) throw new ConnectionFailure("cleanup_run_owner_mismatch"); - if (["queued", "running"].includes(run.status)) { - await api.post(`/api/heartbeat-runs/${runId}/cancel`); - await awaitJourneyCondition(async () => !["queued", "running"].includes((await api!.get(`/api/heartbeat-runs/${runId}`)).status), 30_000, "owned_run_cancellation_unconfirmed", cleanupSignal, "failed"); - } + if (!resources.issueId) throw new ConnectionFailure("cleanup_run_issue_missing"); + await stopOwnedGitHubReviewRun(api, runId, { companyId: config!.target.companyId, agentId: config!.agent.id, issueId: resources.issueId }, cleanupSignal); } if (resources.issueId && result.status !== "passed") { const issue = await api.get(`/api/issues/${resources.issueId}`); diff --git a/tests/runner-e2e/github-review-ownership.ts b/tests/runner-e2e/github-review-ownership.ts index f1d15ee4855..867ff16e84c 100644 --- a/tests/runner-e2e/github-review-ownership.ts +++ b/tests/runner-e2e/github-review-ownership.ts @@ -2,6 +2,10 @@ import type { ChatEndpoint } from "../../packages/shared/src/types/chat-channels import { ConnectionFailure } from "./connection-config.js"; import type { GitHubReviewJourneyConfig } from "./github-review-config.js"; import type { ConnectionApi } from "./connection-target.js"; +import { randomUUID } from "node:crypto"; +import { awaitJourneyCondition } from "./github-review-browser.js"; + +const liveRunStatuses = new Set(["queued", "running", "scheduled_retry"]); /** The issue run source includes history beyond company list windows. Related * activity can name a run for another task, so hydrate and check direct ownership. */ @@ -13,6 +17,33 @@ export async function readOwnedGitHubReviewRuns(api: Pick, return runs.filter(run => run.issueId === scope.issueId && run.agentId === scope.agentId); } +/** A failed native run can still own a durable retry. Stop it through the + * normal audited route and require acknowledgement, not just terminal status. */ +export async function stopOwnedGitHubReviewRun(api: Pick, runId: string, + scope: { companyId: string; agentId: string; issueId: string }, signal: AbortSignal, timeoutMs = 30_000) { + const read = async () => { + const run = await api.get>(`/api/heartbeat-runs/${runId}`); + if (run.id !== runId || run.companyId !== scope.companyId || run.agentId !== scope.agentId || run.issueId !== scope.issueId) { + throw new ConnectionFailure("cleanup_run_owner_mismatch"); + } + return run; + }; + const run = await read(); + const retry = run.runtimeMode === "native" && run.status === "failed" && run.nativePhase === "retryable_failure"; + if (!liveRunStatuses.has(run.status) && !retry) return; + await api.post(`/api/heartbeat-runs/${runId}/cancel`, run.runtimeMode === "native" ? { cancellationRequestId: randomUUID() } : {}); + await awaitJourneyCondition(async () => { + const stopped = await read(); + if (run.runtimeMode !== "native") return !liveRunStatuses.has(stopped.status); + const intent = stopped.resultJson?.nativeCancellation; + return stopped.status === "cancelled" && intent?.schema === "paperclip.native-cancellation.v1" + && intent.runId === runId && intent.companyId === scope.companyId && intent.issueId === scope.issueId + && intent.scope === "run" && intent.reasonCode === "cancellation_run_only" && typeof intent.dispatched === "boolean" + && intent.dispatchState === "acknowledged" && typeof intent.intentAuditId === "string" && intent.intentAuditId.length > 0 + && typeof intent.acknowledgementAuditId === "string" && intent.acknowledgementAuditId.length > 0; + }, timeoutMs, "owned_run_cancellation_unconfirmed", signal, "failed"); +} + export interface GitHubJourneyResources { endpointId?: string; pullNumber?: number; branch: string; appName: string; appId?: string; appSlug?: string; appSettingsUrl?: string; installationSettingsUrl?: string;