diff --git a/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md b/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md index 12cddb6c387..bc765f21390 100644 --- a/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md +++ b/tests/runner-e2e/GITHUB-REVIEW-JOURNEY.md @@ -79,6 +79,14 @@ completed state alone is also insufficient: the actual run must succeed and the associated task must reach Done. A native failure after publication fails the journey and retains the publication evidence already observed. +The rendered check must match its exact GitHub run URL, score, and reviewed +commit. Its failure status comes from that check's header. Hidden re-run menu +items, a failure on another check, and words in the model's narrative do not +prove the conclusion. `github-review-check.spec.ts` covers this boundary in the +credential-free `test:e2e:runner:browser-support` suite. Task completion reads +the visible Status property's picker, before and after reload. Hidden controls, +SVG titles, and narrative text saying Done cannot satisfy that UI checkpoint. + Each attempt records endpoint, App, PR/branch, task, and run ownership. Cleanup removes the owned endpoint, closes the owned PR/deletes its branch, and asks for the owned App's removal. App identity must match the created endpoint, company, @@ -122,7 +130,10 @@ cleanup. Missing usage remains unknown. Authentication screens are excluded. `github-review-report.ts` uses the existing evidence sanitizer and manifest, stages only declared screenshots, and verifies screenshot hashes and attempt path ownership. The standard report inputs are `evidence/result.json`, -`evidence/evidence-manifest.json`, and `evidence/snapshots/`. Raw private operator +`evidence/evidence-manifest.json`, and `evidence/snapshots/`. A passing result +also stages the hash-verified persisted checkpoint as `evidence/final-state.png` +for the standard report's final-state requirement. A missing persisted screenshot +rejects that passing report. Raw private operator receipts are outside the manifest. Existing report tooling imports these files; no second runner or report service is required. A presentation repair must use a new directory, preserve the original result/hash and failure, and dispatch no diff --git a/tests/runner-e2e/billing.test.ts b/tests/runner-e2e/billing.test.ts index 48352815c0e..037ccae91ec 100644 --- a/tests/runner-e2e/billing.test.ts +++ b/tests/runner-e2e/billing.test.ts @@ -31,6 +31,25 @@ function result(overrides: Partial = {}): RunnerE2EResult { } describe("runner E2E billing summaries", () => { + it("preserves captured token totals when a retained result has only a billing summary", () => { + const billing = summarizeExecutionBilling(result({ usage: { + inputTokens: 59_989, outputTokens: 4_974, cachedInputTokens: 488_798, costStatus: "unpriced", + } })); + const retained = result({ billing }); + expect(summarizeExecutionBilling(retained)).toEqual(billing); + expect(aggregateCampaignBilling([retained])).toMatchObject({ llm: { + inputTokens: 59_989, outputTokens: 4_974, cachedInputTokens: 488_798, + runsWithTokenUsage: 1, runsWithReportedCost: 0, + }, testsWithCompleteBilling: 0 }); + }); + + it("uses newly captured usage instead of a stale retained billing summary", () => { + const billing = summarizeExecutionBilling(result({ usage: { inputTokens: 100, outputTokens: 10, costStatus: "unpriced" } })); + const fresh = summarizeExecutionBilling(result({ billing, usage: { inputTokens: 200, outputTokens: 20, costStatus: "unpriced" } })); + expect(fresh.llm).toMatchObject({ inputTokens: 200, outputTokens: 20, runsWithReportedCost: 0 }); + expect(fresh.complete).toBe(false); + }); + it("keeps missing agent time separate from attempt duration and measured zero", () => { const interrupted = result({ status: "failed", durationMs: 108_850 }); expect(summarizeExecutionBilling(interrupted).runtime.agentRunDurationMs).toBeNull(); diff --git a/tests/runner-e2e/billing.ts b/tests/runner-e2e/billing.ts index 35f70e83569..766167c0bfb 100644 --- a/tests/runner-e2e/billing.ts +++ b/tests/runner-e2e/billing.ts @@ -278,8 +278,11 @@ export function summarizeExecutionBilling( result: Pick< RunnerE2EResult, "runIds" | "usage" | "environmentId" | "durationMs" | "runtimeUsage" | "firstTaskQuality" | "completionQuality" | "publicMcp" | "productJourney" - > & Partial>, + > & Partial>, ): RunnerE2EBillingSummary { + // Some retained journeys store their measured summary without raw run usage. + // Regeneration must preserve those measurements, including unknown prices. + if (result.usage === undefined && result.billing) return structuredClone(result.billing); const browserOnly = Boolean(result.productJourney && result.runtimeMode === "none" && !result.runIds?.length); const requestedRunCount = browserOnly ? 0 : Math.max(result.runIds?.length ?? 0, 1); const entries = browserOnly ? [] : usageEntries(result.usage, requestedRunCount); diff --git a/tests/runner-e2e/github-review-browser.ts b/tests/runner-e2e/github-review-browser.ts index b064d0ad384..f4749931dc2 100644 --- a/tests/runner-e2e/github-review-browser.ts +++ b/tests/runner-e2e/github-review-browser.ts @@ -10,6 +10,35 @@ export async function enterGitHubJourneyCompany(page: Page, origin: string, comp await page.waitForURL(url => url.origin === origin && url.pathname === pathname); } +export async function assertGitHubJourneyCheck(page: Page, expected: { repository: string; url: string; headSha: string; score: number }, timeout = 30_000) { + let target: URL; + try { target = new URL(expected.url); } + catch { throw new ConnectionFailure("github_check_reference_mismatch"); } + const prefix = `/${expected.repository}/runs/`; + const id = target.pathname.startsWith(prefix) ? target.pathname.slice(prefix.length) : ""; + if (target.origin !== "https://github.com" || target.username || target.password || target.search || target.hash + || !/^[1-9][0-9]*$/.test(id) || !/^[a-f0-9]{40,64}$/.test(expected.headSha) + || !Number.isInteger(expected.score) || expected.score < 1 || expected.score > 5) { + throw new ConnectionFailure("github_check_reference_mismatch"); + } + await expect(page).toHaveURL(target.href, { timeout }); + const check = page.locator(`#check_run_${id}`); + await expect(check.getByRole("heading", { name: `${expected.score}/5`, exact: true })).toBeVisible({ timeout }); + // GitHub keeps a hidden "Re-run failed checks" menu item before the visible + // result. Only the selected check's status header can prove its conclusion. + await expect(check.locator(":scope > .border-bottom").getByText(/failed/i)).toBeVisible({ timeout }); + await expect(check.getByText(expected.headSha, { exact: true })).toBeVisible({ timeout }); +} + +export async function assertGitHubJourneyTaskDone(page: Page, timeout = 30_000) { + const status = page.locator('[data-property-row="true"]:visible') + .filter({ has: page.locator('[data-property-label="Status"]') }) + .locator('[data-property-value="true"]'); + // Status glyphs contain SVG titles, and responsive layouts can retain hidden + // controls. Read the visible status picker rather than the first Done text. + await expect(status.getByRole("button", { name: "Change status (current: Done)", exact: true })).toBeVisible({ timeout }); +} + export interface GitHubRegistrationBoundary { origin: string; pathname: string; passwordRequired: boolean; oneTimeCodeRequired: boolean; reauthenticationRequired: boolean; createControlVisible: boolean; diff --git a/tests/runner-e2e/github-review-check.spec.ts b/tests/runner-e2e/github-review-check.spec.ts new file mode 100644 index 00000000000..705bc5246a9 --- /dev/null +++ b/tests/runner-e2e/github-review-check.spec.ts @@ -0,0 +1,94 @@ +import { expect, test, type Page } from "@playwright/test"; +import { assertGitHubJourneyCheck, assertGitHubJourneyTaskDone } from "./github-review-browser.js"; + +const expected = { repository: "fixture/repo", url: "https://github.com/fixture/repo/runs/123", headSha: "a".repeat(40), score: 2 }; + +async function openCheck(page: Page, input: { status?: string; score?: number; headSha?: string; unrelatedFailure?: boolean; url?: string } = {}) { + const body = ` +
+
Fixture bot / Paperclip Review ${input.status ?? "failed"}
+

${input.score ?? expected.score}/5

+

Reviewed commit: ${input.headSha ?? expected.headSha}

+

Review rationale: failed tests are discussed in the summary.

+
+ ${input.unrelatedFailure ? '
Another check failed

2/5

' : ""}`; + await page.route("https://github.com/**", route => route.fulfill({ contentType: "text/html", body })); + await page.goto(input.url ?? expected.url); +} + +test("accepts the visible failed check despite an earlier hidden re-run menu item", async ({ page }) => { + await openCheck(page); + await expect(page.getByText(/failed/i).first()).toBeHidden(); + await assertGitHubJourneyCheck(page, expected, 150); +}); + +test("rejects a successful check even when its narrative and another check say failed", async ({ page }) => { + await openCheck(page, { status: "succeeded", unrelatedFailure: true }); + await expect(assertGitHubJourneyCheck(page, expected, 150)).rejects.toThrow(); +}); + +test("rejects the wrong reviewed commit", async ({ page }) => { + await openCheck(page, { headSha: "b".repeat(40) }); + await expect(assertGitHubJourneyCheck(page, expected, 150)).rejects.toThrow(); +}); + +test("rejects a different score", async ({ page }) => { + await openCheck(page, { score: 3 }); + await expect(assertGitHubJourneyCheck(page, expected, 150)).rejects.toThrow(); +}); + +test("rejects a page for another repository", async ({ page }) => { + await openCheck(page, { url: "https://github.com/foreign/repo/runs/123" }); + await expect(assertGitHubJourneyCheck(page, expected, 150)).rejects.toThrow(); +}); + +test("rejects a check reference outside the configured repository", async ({ page }) => { + const url = "https://github.com/foreign/repo/runs/123"; + await openCheck(page, { url }); + await expect(assertGitHubJourneyCheck(page, { ...expected, url }, 150)).rejects.toThrow(); +}); + +for (const [name, override] of [ + ["foreign origin", { url: "https://example.com/fixture/repo/runs/123" }], + ["embedded credentials", { url: "https://fixture-user:fixture-password@github.com/fixture/repo/runs/123" }], + ["query suffix", { url: `${expected.url}?x=1` }], + ["hash suffix", { url: `${expected.url}#fragment` }], + ["non-numeric run ID", { url: "https://github.com/fixture/repo/runs/abc" }], + ["short commit", { headSha: "a".repeat(7) }], + ["score below range", { score: 0 }], + ["score above range", { score: 6 }], +] as const) { + test(`rejects a malformed check reference: ${name}`, async ({ page }) => { + // These must reject before navigation or DOM inspection. + await expect(assertGitHubJourneyCheck(page, { ...expected, ...override }, 150)) + .rejects.toThrow("github_check_reference_mismatch"); + }); +} + +function taskStatus(status: string, hidden = false) { + return `
+ Status
+ +
`; +} + +test("accepts visible Done status despite hidden controls and SVG title text", async ({ page }) => { + await page.setContent(`${taskStatus("Done", true)}${taskStatus("Done")}`); + await expect(page.getByText("Done", { exact: true }).first()).toBeHidden(); + await assertGitHubJourneyTaskDone(page, 150); +}); + +test("rejects Done in the narrative when the task is still in progress", async ({ page }) => { + await page.setContent(`

Done

${taskStatus("In Progress")}`); + await expect(assertGitHubJourneyTaskDone(page, 150)).rejects.toThrow(); +}); + +test("rejects hidden Done status when the visible status is in progress", async ({ page }) => { + await page.setContent(`${taskStatus("Done", true)}${taskStatus("In Progress")}`); + await expect(assertGitHubJourneyTaskDone(page, 150)).rejects.toThrow(); +}); + +test("rejects a Done picker in an unrelated property", async ({ page }) => { + await page.setContent(taskStatus("Done").replace('data-property-label="Status">Status', 'data-property-label="Assignee">Assignee')); + await expect(assertGitHubJourneyTaskDone(page, 150)).rejects.toThrow(); +}); diff --git a/tests/runner-e2e/github-review-flow.ts b/tests/runner-e2e/github-review-flow.ts index 5547adf21a2..d43e919ddd8 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, waitForGitHubRegistrationControl, type GitHubRegistrationBoundary } from "./github-review-browser.js"; +import { assertGitHubJourneyCheck, assertGitHubJourneyTaskDone, 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"; @@ -222,17 +222,20 @@ export async function runGitHubReviewFlow(input: { text: await inline.locator(".review-comment .comment-body").filter({ hasText: /precision|decimal|floating|cents/i }).first().innerText() }; await inline.scrollIntoViewIfNeeded(); await input.checkpoint("github_inline_delivery", github); + await input.phase("github_check_projection"); + if (!review!.checkUrl) throw new ConnectionFailure("github_check_reference_missing"); const checkPopup = page.context().waitForEvent("page"); await page.getByRole("link", { name: "Check", exact: true }).click(); const check = await checkPopup; - await expect(check.getByText(`${evidence.score}/5`, { exact: false }).first()).toBeVisible(); - await expect(check.getByText(/failed/i).first()).toBeVisible(); + await assertGitHubJourneyCheck(check, { repository: config.github.repository, url: review!.checkUrl, headSha, score: evidence.score }); await input.checkpoint("check_projection", check); + await input.phase("task_done"); await page.getByRole("link", { name: "Paperclip task", exact: true }).click(); - await expect(page.getByText("Done", { exact: true }).first()).toBeVisible(); + await assertGitHubJourneyTaskDone(page); await input.checkpoint("task_done", page); + await input.phase("persisted"); await page.reload(); await github.reload(); - await expect(page.getByText("Done", { exact: true }).first()).toBeVisible(); + await assertGitHubJourneyTaskDone(page); await expect(github.getByText(`${evidence.score}/5`, { exact: false }).first()).toBeVisible(); await input.checkpoint("persisted", page); } diff --git a/tests/runner-e2e/github-review-report.test.ts b/tests/runner-e2e/github-review-report.test.ts index 8840fcf171d..9ad23acbcd5 100644 --- a/tests/runner-e2e/github-review-report.test.ts +++ b/tests/runner-e2e/github-review-report.test.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { afterEach, expect, it } from "vitest"; import { runnerMatrix } from "./catalog.js"; import { writeGitHubReviewReport } from "./github-review-report.js"; +import { githubJourneyCheckpoints } from "./github-review-evidence.js"; import { validateRetainedRunnerResult } from "./result-validation.js"; import type { RunnerE2EResult } from "./types.js"; @@ -49,3 +50,30 @@ it("refuses an altered screenshot rather than retaining its claimed digest", asy await expect(writeGitHubReviewReport({ output, campaignId: "altered", result: fixture(), resources: { branch: "owned", appName: "owned", runIds: [] }, catalog: runnerMatrix, secrets: [] })) .rejects.toThrow("journey_screenshot_digest_mismatch"); }); + +it("stages the verified persisted checkpoint as the standard report's final state", async () => { + const output = await mkdtemp(path.join(os.tmpdir(), "github-report-test-")); temporary.push(output); + const input = fixture(); + input.status = "passed"; delete input.error; + input.githubReviewJourney!.outcome = "passed"; + input.githubReviewJourney!.checkpoints = Object.fromEntries(githubJourneyCheckpoints.map(id => [id, true])); + input.screenshots = [{ id: "persisted", label: "Persisted completion", file: "persisted.png", sha256: createHash("sha256").update(pixel).digest("hex") }]; + await writeFile(path.join(output, "persisted.png"), pixel); + const result = await writeGitHubReviewReport({ output, campaignId: "passing-journey", result: input, resources: { branch: "owned", appName: "owned", runIds: ["run"] }, catalog: runnerMatrix, secrets: [] }); + expect(result.status).toBe("passed"); + const manifest = JSON.parse(await readFile(path.join(output, "evidence/evidence-manifest.json"), "utf8")); + expect(manifest.files).toContain("final-state.png"); + expect(await readFile(path.join(output, "evidence/final-state.png"))).toEqual(pixel); + expect(await readFile(path.join(output, "evidence/snapshots/persisted.png"))).toEqual(pixel); +}); + +it("refuses a passing report without its persisted screenshot", async () => { + const output = await mkdtemp(path.join(os.tmpdir(), "github-report-test-")); temporary.push(output); + const input = fixture(); + input.status = "passed"; delete input.error; + input.githubReviewJourney!.outcome = "passed"; + input.githubReviewJourney!.checkpoints = Object.fromEntries(githubJourneyCheckpoints.map(id => [id, true])); + await writeFile(path.join(output, "failure.png"), pixel); + await expect(writeGitHubReviewReport({ output, campaignId: "missing-final-state", result: input, resources: { branch: "owned", appName: "owned", runIds: ["run"] }, catalog: runnerMatrix, secrets: [] })) + .rejects.toThrow("journey_final_state_missing"); +}); diff --git a/tests/runner-e2e/github-review-report.ts b/tests/runner-e2e/github-review-report.ts index 6ad2cc05149..7d0e3332619 100644 --- a/tests/runner-e2e/github-review-report.ts +++ b/tests/runner-e2e/github-review-report.ts @@ -29,6 +29,7 @@ export async function writeGitHubReviewReport(input: { try { await mkdir(path.join(stage, "snapshots"), { mode: 0o700 }); const result = structuredClone(input.result); + let finalStateStaged = false; for (const shot of result.screenshots ?? []) { const source = path.resolve(output, shot.file); if (!source.startsWith(`${output}${path.sep}`) || !/^[a-z0-9_-]+$/.test(shot.id) || path.extname(source) !== ".png") throw new Error("unsafe_journey_screenshot_reference"); @@ -37,8 +38,13 @@ export async function writeGitHubReviewReport(input: { if (shot.sha256 && createHash("sha256").update(bytes).digest("hex") !== shot.sha256) throw new Error("journey_screenshot_digest_mismatch"); shot.file = `snapshots/${shot.id}.png`; await writeFile(path.join(stage, shot.file), bytes, { mode: 0o600 }); + if (result.status === "passed" && shot.id === "persisted" && result.githubReviewJourney?.checkpoints.persisted === true) { + await writeFile(path.join(stage, "final-state.png"), bytes, { mode: 0o600 }); + finalStateStaged = true; + } } validateRetainedRunnerResult(result); + if (result.status === "passed" && !finalStateStaged) throw new Error("journey_final_state_missing"); await save(stage, "result.json", result); await save(stage, "snapshots/journey-resources.json", input.resources); await save(stage, "snapshots/journey-evidence.json", result.githubReviewJourney); diff --git a/tests/runner-e2e/history.test.ts b/tests/runner-e2e/history.test.ts index c717441e321..b6216af3745 100644 --- a/tests/runner-e2e/history.test.ts +++ b/tests/runner-e2e/history.test.ts @@ -32,6 +32,7 @@ import { renderRunnerHistoryIndex } from "./history-index.js"; import { renderPublicCampaignSummary } from "./public-summary-image.js"; import { isPublicRunnerScreenshotRoute } from "./screenshot-policy.js"; import { connectionCheckpoints, type ConnectionEvidence } from "./connection-evidence.js"; +import { githubJourneyCheckpoints, type GitHubReviewJourneyEvidence } from "./github-review-evidence.js"; import type { MatrixExecution, RunnerE2EResult } from "./types.js"; const temporaryDirectories: string[] = []; @@ -65,6 +66,12 @@ function result(execution: MatrixExecution, status: "passed" | "failed") { durationMs: 1_000, cleanup: "passed", ...(execution.task.flow === "provider_connection" ? { providerConnection: { version: 1, outcome: status, phase: "finished", assisted: false, authFreshness: "signed-in", target: { mode: "attach", origin: "http://127.0.0.1:3100", commit: "a".repeat(40), deploymentMode: "local_trusted" }, entry: "agent", method: "api-key", checkpoints: Object.fromEntries(connectionCheckpoints.map(key => [key, true])), waits: [] } satisfies ConnectionEvidence } : {}), + ...(execution.task.flow === "github_review_journey" ? { githubReviewJourney: { + version: 1, outcome: status, phase: "finished", setupId: "fixture", hosting: "self-hosted", + targetCommit: "a".repeat(40), assisted: false, dependencies: [], + checkpoints: Object.fromEntries(githubJourneyCheckpoints.map(key => [key, true])), + cleanup: { endpoint: true, pullRequest: true, branch: true, app: true }, + } satisfies GitHubReviewJourneyEvidence } : {}), } satisfies RunnerE2EResult; } diff --git a/tests/runner-e2e/playwright-support.config.ts b/tests/runner-e2e/playwright-support.config.ts index bdaefb27292..0256f9ef884 100644 --- a/tests/runner-e2e/playwright-support.config.ts +++ b/tests/runner-e2e/playwright-support.config.ts @@ -3,7 +3,7 @@ import { defineConfig } from "@playwright/test"; /** Browser-only harness regressions: no Paperclip instance or provider credentials. */ export default defineConfig({ testDir: ".", - testMatch: ["screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts"], + testMatch: ["screenshot-readiness.spec.ts", "service-worker-reload.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts", "browser-bootstrap-diagnostics.spec.ts", "service-worker-module-loading.spec.ts", "browser-bootstrap-recovery.spec.ts", "blocker-input.spec.ts", "native-blocker-visible.spec.ts", "copilot-permission-card.spec.ts", "github-review-check.spec.ts"], workers: 1, retries: 0, timeout: 10_000,