mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
test(restart-chaos): save a screenshot and banner HTML when the Retry button wait fails (#20643)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The restart-chaos harness drives the board UI and clicks Retry the way a person would > - When no visible Retry appears, the harness grades `ui_retry_button_missing` and keeps only the banner text > - That is not enough to tell a product bug (no button, a hidden button, a wrong name) from a harness locator bug after the wave has ended > - This pull request saves a full-page screenshot and the banner and Retry-button DOM when the wait fails > - The benefit is that every `ui_retry_button_missing` can be diagnosed from the wave folder ## Linked Issues or Issue Description Refs #20616 **What happened?** Wave `w-20261110T174930-s9205` t01 graded `ui_retry_button_missing`, but the captured banner text contained "Retry". The harness saved no screenshot or DOM, so nobody could prove whether the page had a hidden duplicate or no actionable button. **Expected behavior** A failed Retry-button wait leaves enough evidence to diagnose it later. **Steps to reproduce** 1. Run a restart-chaos wave with the board UI. 2. Get a task whose Retry wait times out. 3. Observe that the wave folder holds only the banner text. ## What Changed - `tests/restart-chaos/board-ui.ts`: add `saveRetryMissingDiagnostics`. On a failed Retry wait it saves `ui-retry-missing-<issue>-<time>.png` (full page) and a `.json` file with the page URL, each `[role=status]`/`[role=alert]` banner's `outerHTML`, and every Retry-named button's visibility, bounding box, banner membership and HTML. Diagnostics never change the click result. - `tests/restart-chaos/main.ts`: the wave's board helper writes to `<wave>/ui-retry-diagnostics`. - `tests/restart-chaos/observer.ts`, `types.ts`: the task's retry attempt records the saved file paths. - `tests/restart-chaos/board-ui.test.ts`: a test for the saved files. ## Verification - `npx vitest run --config tests/restart-chaos/vitest.config.ts tests/restart-chaos/board-ui.test.ts`: 9 passed (new: "saves a screenshot and the banner's HTML when no visible Retry appears"). - `npx tsc --noEmit -p tests/restart-chaos/tsconfig.json` passes. ## Risks - Harness only; no product code changes. A failed screenshot or file write is ignored and does not change the grade. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
1 parent
ed5e7493bb
commit
6892ed3a98
5 files changed
+74
-6
No files matched your search
@@ -1,4 +1,7 @@
|
||||
import { createServer, type Server } from "node:http";
|
||||
import { mkdtemp, readFile, readdir, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { once } from "node:events";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { BoardUi, UI_RETRY_BUTTON_MISSING, UI_RETRY_NO_REQUEST, isRetryRequestFor } from "./board-ui.js";
|
||||
@@ -7,6 +10,7 @@ const retry = (issue: string) => `fetch('/api/issues/${issue}/retry',{method:'PO
|
||||
const pages: Record<string, string> = {
|
||||
"/issues/CHA-1": `<div role="status">Couldn’t finish this attempt. <button onclick="${retry("i1")}">Retry</button></div>`,
|
||||
"/issues/CHA-2": `<div role="status">Saving files from the previous attempt</div>`,
|
||||
"/issues/CHA-7": `<div role="status">Couldn’t finish this attempt. <button style="display:none">Retry</button></div>`,
|
||||
"/issues/CHA-3": `<div role="status">Automatic recovery stopped. <button>Retry</button></div>`,
|
||||
"/issues/CHA-4": `<section><p>Couldn’t load the subtasks.</p><button onclick="window.reloaded=true">Retry</button></section>
|
||||
<div role="status">Couldn’t finish this attempt. <button onclick="${retry("i4")}">Retry</button></div>`,
|
||||
@@ -54,6 +58,37 @@ describe("BoardUi.clickRetry", () => {
|
||||
});
|
||||
}, 60_000);
|
||||
|
||||
it("saves a screenshot and the banner's HTML when no visible Retry appears", async () => {
|
||||
const directory = await mkdtemp(path.join(tmpdir(), "chaos-retry-diagnostics-"));
|
||||
const board = new BoardUi(baseUrl, undefined, directory);
|
||||
try {
|
||||
const result = await board.clickRetry("CHA-2", "i2", 1_500);
|
||||
expect(result).toMatchObject({ clicked: false, error: UI_RETRY_BUTTON_MISSING,
|
||||
diagnostics: { screenshot: expect.stringMatching(/\.png$/), dom: expect.stringMatching(/\.json$/) } });
|
||||
expect((await readdir(directory)).sort()).toHaveLength(2);
|
||||
const dom = JSON.parse(await readFile(result.diagnostics!.dom!, "utf8"));
|
||||
expect(dom).toMatchObject({ url: expect.stringContaining("/issues/CHA-2"), retryButtons: [],
|
||||
banners: [expect.stringContaining("Saving files from the previous attempt")] });
|
||||
} finally {
|
||||
await board.close();
|
||||
await rm(directory, { recursive: true, force: true });
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it("records a hidden Retry with its visibility and box", async () => {
|
||||
const directory = await mkdtemp(path.join(tmpdir(), "chaos-retry-hidden-"));
|
||||
const board = new BoardUi(baseUrl, undefined, directory);
|
||||
try {
|
||||
const result = await board.clickRetry("CHA-7", "i7", 1_500);
|
||||
expect(result).toMatchObject({ clicked: false, error: UI_RETRY_BUTTON_MISSING });
|
||||
const dom = JSON.parse(await readFile(result.diagnostics!.dom!, "utf8"));
|
||||
expect(dom.retryButtons).toEqual([expect.objectContaining({ visible: false, box: null, inBanner: true })]);
|
||||
} finally {
|
||||
await board.close();
|
||||
await rm(directory, { recursive: true, force: true });
|
||||
}
|
||||
}, 60_000);
|
||||
|
||||
it("reports a Retry button whose click sends no retry request", async () => {
|
||||
await withBoard(async board => {
|
||||
expect(await board.clickRetry("CHA-3", "i3", 1_500, 500)).toMatchObject({ clicked: true, httpStatus: null, error: UI_RETRY_NO_REQUEST });
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { mkdir, writeFile } from "node:fs/promises";
|
||||
import path from "node:path";
|
||||
import { chromium, type Browser, type ElementHandle, type Page } from "@playwright/test";
|
||||
import { UI_RETRY_BUTTON_MISSING, UI_RETRY_NO_REQUEST } from "./types.js";
|
||||
|
||||
@@ -9,6 +11,29 @@ export interface UiRetryClick {
|
||||
accepted: boolean | null;
|
||||
error: string | null;
|
||||
bannerText: string | null;
|
||||
diagnostics?: { screenshot: string | null; dom: string | null } | null;
|
||||
}
|
||||
|
||||
export async function saveRetryMissingDiagnostics(page: Page, directory: string, label: string) {
|
||||
const stem = path.join(directory, `ui-retry-missing-${label.replace(/[^A-Za-z0-9_-]+/g, "_")}-${Date.now()}`);
|
||||
await mkdir(directory, { recursive: true });
|
||||
const screenshot = await page.screenshot({ path: `${stem}.png`, fullPage: true, timeout: 10_000 })
|
||||
.then(() => `${stem}.png`).catch(() => null);
|
||||
const banners = await page.locator("[role=status], [role=alert]")
|
||||
.evaluateAll(nodes => nodes.map(node => (node as Element).outerHTML.slice(0, 20_000))).catch(() => []);
|
||||
const buttons = [];
|
||||
for (const button of await page.getByRole("button", { name: "Retry", exact: true, includeHidden: true })
|
||||
.elementHandles().catch(() => [])) {
|
||||
buttons.push({
|
||||
visible: await button.isVisible().catch(() => null),
|
||||
box: await button.boundingBox().catch(() => null),
|
||||
inBanner: await button.evaluate(node => Boolean((node as Element).closest("[role=status], [role=alert]"))).catch(() => null),
|
||||
html: await button.evaluate(node => (node as Element).outerHTML.slice(0, 2_000)).catch(() => null),
|
||||
});
|
||||
}
|
||||
const dom = await writeFile(`${stem}.json`, JSON.stringify({ url: page.url(), savedAt: new Date().toISOString(),
|
||||
banners, retryButtons: buttons }, null, 2)).then(() => `${stem}.json`).catch(() => null);
|
||||
return { screenshot, dom };
|
||||
}
|
||||
|
||||
const PER_BUTTON_REQUEST_MS = 5_000;
|
||||
@@ -30,7 +55,7 @@ async function retryButtonsBannerFirst(page: Page): Promise<ElementHandle[]> {
|
||||
}
|
||||
|
||||
export async function clickRetryOnPage(page: Page, url: string, issueRefs: readonly string[], timeoutMs: number,
|
||||
perButtonMs = PER_BUTTON_REQUEST_MS): Promise<UiRetryClick> {
|
||||
perButtonMs = PER_BUTTON_REQUEST_MS, diagnosticsDirectory: string | null = null): Promise<UiRetryClick> {
|
||||
await page.goto(url, { waitUntil: "domcontentloaded", timeout: timeoutMs });
|
||||
const banner = async () => {
|
||||
const texts = await page.locator("[role=status], [role=alert]").allInnerTexts().catch(() => []);
|
||||
@@ -40,7 +65,11 @@ export async function clickRetryOnPage(page: Page, url: string, issueRefs: reado
|
||||
await page.getByRole("button", { name: "Retry", exact: true }).filter({ visible: true }).first()
|
||||
.waitFor({ state: "visible", timeout: timeoutMs });
|
||||
} catch {
|
||||
return { clicked: false, httpStatus: null, accepted: null, error: UI_RETRY_BUTTON_MISSING, bannerText: await banner() };
|
||||
const bannerText = await banner();
|
||||
const diagnostics = diagnosticsDirectory
|
||||
? await saveRetryMissingDiagnostics(page, diagnosticsDirectory, issueRefs[0] ?? "issue").catch(() => null) : undefined;
|
||||
return { clicked: false, httpStatus: null, accepted: null, error: UI_RETRY_BUTTON_MISSING, bannerText,
|
||||
...(diagnostics !== undefined ? { diagnostics } : {}) };
|
||||
}
|
||||
const bannerText = await banner();
|
||||
for (const button of await retryButtonsBannerFirst(page)) {
|
||||
@@ -59,7 +88,8 @@ export async function clickRetryOnPage(page: Page, url: string, issueRefs: reado
|
||||
export class BoardUi {
|
||||
private browser: Promise<Browser> | null = null;
|
||||
|
||||
constructor(private readonly baseUrl: string, private readonly launch: () => Promise<Browser> = () => chromium.launch({ headless: true })) {}
|
||||
constructor(private readonly baseUrl: string, private readonly launch: () => Promise<Browser> = () => chromium.launch({ headless: true }),
|
||||
private readonly diagnosticsDirectory: string | null = null) {}
|
||||
|
||||
async clickRetry(identifier: string, issueId: string = identifier, timeoutMs = 60_000, perButtonMs = PER_BUTTON_REQUEST_MS): Promise<UiRetryClick> {
|
||||
let context: Awaited<ReturnType<Browser["newContext"]>> | null = null;
|
||||
@@ -67,7 +97,7 @@ export class BoardUi {
|
||||
this.browser ??= this.launch();
|
||||
context = await (await this.browser).newContext({ viewport: { width: 1440, height: 1000 } });
|
||||
return await clickRetryOnPage(await context.newPage(), `${this.baseUrl}/issues/${encodeURIComponent(identifier)}`,
|
||||
[identifier, issueId], timeoutMs, perButtonMs);
|
||||
[identifier, issueId], timeoutMs, perButtonMs, this.diagnosticsDirectory);
|
||||
} catch (error) {
|
||||
return { clicked: false, httpStatus: null, accepted: null, error: error instanceof Error ? error.message : String(error), bannerText: null };
|
||||
} finally {
|
||||
|
||||
@@ -348,7 +348,8 @@ async function main(): Promise<number> {
|
||||
}
|
||||
// The instance root holds the secrets master key and the database: oracle code may not read it.
|
||||
const protectedRoots = [os.homedir(), supervisor.paths.temporaryRoot, resultsRoot];
|
||||
board = serverEnvironment.SERVE_UI === "false" ? null : new BoardUi(supervisor.baseUrl);
|
||||
board = serverEnvironment.SERVE_UI === "false" ? null
|
||||
: new BoardUi(supervisor.baseUrl, undefined, path.join(waveDir, "ui-retry-diagnostics"));
|
||||
const observers = plan.tasks.map(task => new TaskObserver(task, api, lab, timeline, supervisor, protectedRoots, board));
|
||||
const pending = [...observers];
|
||||
const launched: TaskObserver[] = [];
|
||||
|
||||
@@ -717,7 +717,8 @@ export class TaskObserver {
|
||||
const attempt: RetryAttempt = { at: now, observedRunId, httpStatus: null, accepted: null, error: null };
|
||||
if (this.board) {
|
||||
const click = await this.board.clickRetry(issueRef, issueId);
|
||||
Object.assign(attempt, { via: "ui", httpStatus: click.httpStatus, accepted: click.accepted, error: click.error, bannerText: click.bannerText });
|
||||
Object.assign(attempt, { via: "ui", httpStatus: click.httpStatus, accepted: click.accepted, error: click.error, bannerText: click.bannerText,
|
||||
...(click.diagnostics !== undefined ? { diagnostics: click.diagnostics } : {}) });
|
||||
if (click.accepted) attempt.acceptedAt = Math.max(now, Date.now());
|
||||
return attempt;
|
||||
}
|
||||
|
||||
@@ -252,6 +252,7 @@ export interface RetryAttempt {
|
||||
error: string | null;
|
||||
via?: "ui" | "api";
|
||||
bannerText?: string | null;
|
||||
diagnostics?: { screenshot: string | null; dom: string | null } | null;
|
||||
}
|
||||
|
||||
export const UI_RETRY_BUTTON_MISSING = "ui_retry_button_missing";
|
||||
|
||||
Reference in new issue
Block a user