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;