From e6ec781a1abcdcddb98e712bcb00b201303e44de Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:54:27 -0700 Subject: [PATCH] fix(e2e): verify review outcomes and preserve report measurements (#17556) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip's Product E2E journeys verify user outcomes in the real app and GitHub. > - The review journey must show the score and failed check for its exact reviewed commit, then the visible task status before and after reload. > - GitHub includes a hidden “Re-run failed checks” menu item before the visible check result. > - The journey selected that hidden item and failed even after native work and publication completed. A subsequent attempt exposed the same first-text-match problem with Done in the task UI. > - This change reads the selected check's status header and binds its URL, score, and commit. > - Browser fixtures retain this failure and reject plausible wrong projections. ## Linked Issues or Issue Description Refs: #17457, #17513, #17551. **What happened?** The bot completed its native run and task, then published its summary, inline precision finding, and failed 2/5 check. The browser assertion selected a hidden “Re-run failed checks” menu item before the visible result, so the journey still failed. After that was corrected, the next live attempt passed the check projection but selected hidden Done text before the visible task status. **Expected behavior** Verify the actual selected GitHub check. A hidden menu item or a word in the model's narrative cannot prove its conclusion. **Steps to reproduce** 1. Render a hidden “Re-run failed checks” item before a visible failed check. 2. Use the old first text match for “failed”. 3. Observe the false visibility failure. **Paperclip version or commit** The base includes the journey changes from #17457 and #17513. ## What Changed - Validate the check URL against the configured repository and select its exact check-run ID. - Require the expected score and full reviewed commit inside that check. - Read failure status only from that check's header. - Give each projection step its own phase so failures identify the correct boundary. - Stage the verified persisted screenshot as the standard report final-state artifact, rejecting a passing report if it is absent. - Preserve measured billing summaries during regeneration when raw run usage is absent; newer raw usage takes precedence. - Extend the catalog-wide history fixture to include the new journey evidence contract. - Read task completion from the visible Status property picker before and after reload, with separate phase labels. - Add eighteen isolated browser fixtures to the existing browser-support suite and document both boundaries. ## Verification - Reproduced the old selector failure in the live GitHub page and a credential-free browser fixture before fixing it. - All 60 browser-support cases pass, including eighteen focused projection cases. They reject the wrong check status, score, commit, page, and repository reference, plus hidden or unrelated Done text. - All 163 focused billing, history, accounting, report, and journey tests pass, as does runner E2E typechecking. - A regression reproduced discarded saved token totals. Retained totals now survive per-cell and campaign aggregation while missing dollar prices stay unknown; fresh raw usage still overrides stale summaries. - Diff and credential-value checks pass. - The earlier failed attempts remain failed. A fresh complete QA journey passed all 18 checkpoints and cleanup with a successful native run, a below-threshold review and inline precision finding, the correctly failed check, visible Done status, and reload persistence. The later reference-validation commit changes only browser fixtures; the live driver bytes are unchanged. - A report regression reproduced the missing standard final-state artifact. The packager now aliases only the hash-verified persisted screenshot for passing results; missing final-state proof is rejected. Presentation regeneration preserves the original live grade and dispatches no provider work. ## Risks This depends on GitHub's check-run header structure. A markup change fails the journey instead of accepting narrative text as a result. The product runtime and behavioral oracle stay unchanged. Usage-price coverage remains a separate evidence dimension. A saved billing summary is reused only when raw usage is absent, preserving the recorded coverage and unknown prices. The separate advisory Paperclip Review remains queued after its prior assigned attempt timed out. Its normal Retry control rejected the obsolete failed chat request as unsafe to retry. Required repository gates `ci / verify` and `ci / e2e` pass, all 55 completed CI checks are green, and final-head Greptile is 5/5 with no unresolved findings. The advisory queue is not represented as a completed assessment. ## Model Used OpenAI Codex, based on GPT-6. The exact runtime model ID and context-window size are not exposed in this session. Used for code, browser fixtures, and live evidence 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 | 13 ++- tests/runner-e2e/billing.test.ts | 19 ++++ tests/runner-e2e/billing.ts | 5 +- tests/runner-e2e/github-review-browser.ts | 29 ++++++ tests/runner-e2e/github-review-check.spec.ts | 94 +++++++++++++++++++ tests/runner-e2e/github-review-flow.ts | 13 ++- tests/runner-e2e/github-review-report.test.ts | 28 ++++++ tests/runner-e2e/github-review-report.ts | 6 ++ tests/runner-e2e/history.test.ts | 7 ++ tests/runner-e2e/playwright-support.config.ts | 2 +- 10 files changed, 208 insertions(+), 8 deletions(-) create mode 100644 tests/runner-e2e/github-review-check.spec.ts 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,