(8/9) Attach named visual states to browser reports

.github/workflows/feedback-loop.yml

@@ -19,7 +19,7 @@ jobs:
       - run: pnpm --filter harness exec playwright install --with-deps chromium
       - run: pnpm test:e2e
       - uses: actions/upload-artifact@v4
-        if: failure()
+        if: always()
         with:
           name: browser-feedback-loop
           path: |

packages/harness/README.md

@@ -21,3 +21,7 @@ Additional phases: `conflicts`, `needs-approval`, `changes-requested`, `empty`,
 ## GitHub access boundary
 
 `pnpm check:boundaries` parses production TypeScript and checks client dependencies. GitHub/network clients and direct server/CLI requests must stay in `packages/sync`; renderer requests are centralized in `api.ts` and use relative `/api/` URLs. Consumers may import only the public `sync` entry point. Electron's loopback readiness probe and port allocation are explicit exceptions; its updater remains separate from dashboard data access. Tests and the simulator may implement an HTTP server. This is a regression guard, not a security sandbox. It runs as part of lint and CI.
+
+## Visual review
+
+Every browser run captures named states (in-flight, queued, queue-failed, outage, recovered, conflicts, approval/review changes, empty lists, partial outage, and rate-limit pause). View PNG attachments in the HTML report, or download the `browser-feedback-loop` Actions artifact, uploaded on successful and failed runs. Screenshots use a pinned viewport, light theme, locale and timezone with animations disabled. These are review artifacts, not pixel-diff assertions; text/structure/state assertions determine correctness.

packages/harness/playwright.config.ts

@@ -10,6 +10,10 @@ export default defineConfig({
   reporter: [["list"], ["html", { open: "never" }]],
   use: {
     browserName: "chromium",
+    colorScheme: "light",
+    locale: "en-GB",
+    timezoneId: "UTC",
+    reducedMotion: "reduce",
     viewport: { width: 1440, height: 900 },
     trace: "retain-on-failure",
     screenshot: "only-on-failure",

packages/harness/src/artifacts.ts

@@ -0,0 +1,19 @@
+import type { Page, TestInfo } from "@playwright/test";
+
+/** Review evidence, not pixel-diff baselines: viewport/theme are pinned in config. */
+export async function captureState(
+  page: Page,
+  testInfo: TestInfo,
+  name: string,
+) {
+  await page.evaluate(() => document.fonts.ready);
+  const path = testInfo.outputPath(`${name}.png`);
+  await page.screenshot({
+    path,
+    fullPage: true,
+    animations: "disabled",
+    caret: "hide",
+    scale: "css",
+  });
+  await testInfo.attach(name, { path, contentType: "image/png" });
+}

packages/harness/src/board.e2e.ts

@@ -1,5 +1,6 @@
 import { test, expect } from "@playwright/test";
 import { startScenario } from "./runtime.js";
+import { captureState } from "./artifacts.js";
 import { SCENARIO_TITLE } from "./github.js";
 
 test("polls queue entry, queue failure, and recovery through the real sync engine", async ({
@@ -34,6 +35,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin
       card.getByRole("img", { name: "Ready for review", exact: true }),
     ).toBeVisible();
 
+    await captureState(page, testInfo, "in-flight");
     scenario.setPhase("queued");
     await scenario.syncAll();
     expect(await scenario.persistedPrs()).toMatchObject([
@@ -47,6 +49,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin
     await expect(card.getByText("pending", { exact: true })).toBeVisible();
     await expect(card).toHaveCount(1);
 
+    await captureState(page, testInfo, "queued");
     scenario.setPhase("queue-failed");
     await scenario.syncAll();
     expect(await scenario.persistedPrs()).toMatchObject([
@@ -58,6 +61,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin
     await expect(card.getByText("failing", { exact: true })).toBeVisible();
     await expect(card).toHaveCount(1);
 
+    await captureState(page, testInfo, "queue-failed");
     scenario.setPhase("offline");
     await expect(scenario.syncAll()).rejects.toThrow("Scenario sync failed");
     expect(await scenario.persistedPrs()).toMatchObject([
@@ -70,6 +74,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin
       }),
     ).toBeVisible();
 
+    await captureState(page, testInfo, "upstream-outage");
     scenario.setPhase("in-flight");
     await scenario.syncAll();
     await expect(card.getByText("passing", { exact: true })).toBeVisible();
@@ -78,6 +83,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin
     await expect(
       page.getByLabel("Sync status: Up to date", { exact: true }),
     ).toBeVisible();
+    await captureState(page, testInfo, "recovered");
     expect(errors).toEqual([]);
     expect(apiResponses.length).toBeGreaterThan(3);
     expect(apiResponses.every((response) => response.status === 200)).toBe(

packages/harness/src/states.e2e.ts

@@ -1,10 +1,11 @@
 import { test, expect } from "@playwright/test";
 import { startScenario } from "./runtime.js";
+import { captureState } from "./artifacts.js";
 import { SCENARIO_TITLE } from "./github.js";
 
 test("renders conflict, review-request membership, approval and empty states", async ({
   page,
-}) => {
+}, testInfo) => {
   const scenario = await startScenario();
   try {
     for (const phase of [
@@ -42,13 +43,16 @@ test("renders conflict, review-request membership, approval and empty states", a
           .getByRole("listitem")
           .filter({ hasText: "Review the storage change" }),
       ).toHaveCount(phase === "review-requested" ? 1 : 0);
+      await captureState(page, testInfo, phase);
     }
   } finally {
     await scenario.close();
   }
 });
 
-test("keeps two instances separate when one fails", async ({ page }) => {
+test("keeps two instances separate when one fails", async ({
+  page,
+}, testInfo) => {
   const scenario = await startScenario({ instances: 2 });
   const errors: string[] = [];
   page.on("console", (message) => {
@@ -73,6 +77,7 @@ test("keeps two instances separate when one fails", async ({ page }) => {
         exact: true,
       }),
     ).toBeVisible();
+    await captureState(page, testInfo, "partial-outage");
     expect(errors).toEqual([]);
   } finally {
     await scenario.close();
@@ -81,7 +86,7 @@ test("keeps two instances separate when one fails", async ({ page }) => {
 
 test("shows a rate-limit pause and clears it after the budget resets", async ({
   page,
-}) => {
+}, testInfo) => {
   const scenario = await startScenario();
   try {
     await scenario.syncPrs();
@@ -96,6 +101,7 @@ test("shows a rate-limit pause and clears it after the budget resets", async ({
     await expect(paused).toBeVisible();
     await paused.click();
     await expect(page.getByText(/Retry after/).first()).toBeVisible();
+    await captureState(page, testInfo, "rate-limit-pause");
     scenario.setPhase("in-flight");
     await expect
       .poll(