From a317aa2eeaaa0c9014779ade5ca4de2c68f27d4d Mon Sep 17 00:00:00 2001 From: Anton Niklasson Date: Fri, 13 Nov 2026 13:42:33 +0100 Subject: [PATCH] Verify status board polling through the real sync engine in Chromium --- .github/workflows/feedback-loop.yml | 28 +++++++ .gitignore | 2 + AGENTS.md | 8 +- README.md | 4 + package.json | 3 +- packages/harness/README.md | 8 ++ packages/harness/package.json | 4 +- packages/harness/playwright.config.ts | 17 ++++ packages/harness/src/board.e2e.ts | 89 +++++++++++++++++++++ packages/web/src/components/PrStateIcon.tsx | 16 +++- pnpm-lock.yaml | 28 +++++++ 11 files changed, 202 insertions(+), 5 deletions(-) create mode 100644 .github/workflows/feedback-loop.yml create mode 100644 packages/harness/playwright.config.ts create mode 100644 packages/harness/src/board.e2e.ts diff --git a/.github/workflows/feedback-loop.yml b/.github/workflows/feedback-loop.yml new file mode 100644 index 0000000..84dfb38 --- /dev/null +++ b/.github/workflows/feedback-loop.yml @@ -0,0 +1,28 @@ +name: Browser feedback loop + +on: + push: + branches: [main] + pull_request: + +jobs: + scenario: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v4 + with: + node-version: 22 + cache: pnpm + - run: pnpm install --frozen-lockfile + - run: pnpm --filter harness exec playwright install --with-deps chromium + - run: pnpm test:e2e + - uses: actions/upload-artifact@v4 + if: failure() + with: + name: browser-feedback-loop + path: | + packages/harness/playwright-report/ + packages/harness/test-results/ + retention-days: 7 diff --git a/.gitignore b/.gitignore index 960088e..254ea23 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ pnpm-debug.log* .claude/settings.local.json .playwright-mcp/ packages/desktop/release/ +packages/harness/test-results/ +packages/harness/playwright-report/ diff --git a/AGENTS.md b/AGENTS.md index 0fe8d11..9a49d95 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,9 +10,11 @@ Close the loop before declaring done. Available tools: - `pnpm test` — vitest in both packages - `pnpm lint` — oxlint - `pnpm fmt:check` — oxfmt +- `pnpm test:e2e` — browser scenario through the built web app, real API, sync engine, and temporary SQLite cache. Install Chromium once with `pnpm --filter harness exec playwright install chromium`. Failure traces, screenshots, and diagnostics go to `packages/harness/test-results/` and `playwright-report/`. +- `pnpm scenario` — interactive isolated board; enter `queued`, `queue-failed`, `offline`, or `in-flight` to drive upstream transitions. See `packages/harness/README.md`. - `pnpm dev:web` — runs server + web concurrently (long-running; start in background). For agent work prefer this over `pnpm dev`, which also spawns an Electron window. Stdout/stderr is tee'd to `.logs/server.log` and `.logs/web.log` — tail or read these to inspect output. Logs reset on each start; accumulate across hot reloads within a session. -For UI/UX changes, type-checks aren't enough — drive the app via the **Playwright MCP** (`.mcp.json`): +For UI/UX changes, type-checks aren't enough — run `pnpm test:e2e` for repeatable scenarios. For interactive exploration, drive the app via the **Playwright MCP** (`.mcp.json`): - `browser_navigate` to `http://localhost:7200` - `browser_snapshot` for the accessibility tree (preferred over screenshots for assertions) @@ -30,3 +32,7 @@ A config file with valid tokens already exists at `~/.config/github-dashboard/co Things `oxfmt`/`oxlint` won't catch but we still care about: - **Blank lines around React hook calls.** Separate `useEffect`, `useMemo`, `useCallback`, etc. from surrounding code and from each other with a blank line above and below. Multi-statement hook bodies are easier to scan when they aren't visually fused to neighboring lines. + +## Review size + +Prefer small, focused commits and stacked PRs over large implementations. Keep verification infrastructure and product behavior changes in separate layers, each independently verifiable. diff --git a/README.md b/README.md index fade332..8aa8fa6 100644 --- a/README.md +++ b/README.md @@ -61,3 +61,7 @@ TUI state lives in XDG dirs and is safe to delete: ### Desktop/web sync engine scope The desktop and web clients use `packages/sync` for all GitHub access, including authentication, list synchronization, comments and file diffs. Supported writes are approval, draft/ready toggling, auto-merge toggling and notification dismissal. Direct merge/close, title editing, CI reruns, posting conversation comments, closed-PR search, colleague browsing, and detailed commit/check lists are currently unavailable; use “Open in browser” for those operations. The local Hono HTTP API remains the renderer transport, but owns no GitHub client or live cache. + +### Repeatable feedback loop + +Run `pnpm scenario` to develop against a controllable local GitHub simulator, or `pnpm test:e2e` to check queue transitions in Chromium through the real sync engine and SQLite cache. Both use disposable configuration and fake credentials. See [the harness guide](packages/harness/README.md) for setup, controls, and failure artifacts. diff --git a/package.json b/package.json index eab134b..9e56ab5 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,8 @@ "prepare": "husky", "cli": "node packages/cli/dist/index.js", "dev:cli": "pnpm --filter cli dev", - "scenario": "pnpm --filter web build && pnpm --filter harness scenario" + "scenario": "pnpm --filter web build && pnpm --filter harness scenario", + "test:e2e": "pnpm --filter sync build && pnpm --filter web build && pnpm --filter harness test:e2e" }, "devDependencies": { "@typescript/native-preview": "7.0.0-dev.20260420.1", diff --git a/packages/harness/README.md b/packages/harness/README.md index 920fbde..df0216d 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -7,3 +7,11 @@ Enter `queued`, then `queue-failed`, to move the fixture PR into and out of the The current board changes its queue icon; the dedicated merge-queue section in #84 is a subsequent product change. This harness supplies the repeatable state transitions for developing it. `pnpm --filter harness test` checks the simulator through the real engine: transitions, outage, recovery, and persisted snapshots across restart. Unsupported GitHub calls fail with 501 so new engine behavior requires an explicit fixture update. + +## Browser feedback loop + +Install Chromium once with `pnpm --filter harness exec playwright install chromium`, then run `pnpm test:e2e` from the root. No MCP connection or real GitHub access is required. + +The browser check runs the built renderer and real API/engine on disposable ports, then changes the upstream fixture. It verifies SQLite snapshots, normal GUI polling (without reloading), queue entry/exit, failure preservation, recovery, and absence of duplicate PRs. Browser console errors, runtime exceptions, unexpected external requests and unsuccessful API responses fail the check. + +Failures retain a Playwright trace and screenshot in `packages/harness/test-results/`. The HTML report includes API responses, upstream requests, and server logs; open it with `pnpm --filter harness exec playwright show-report`. The same check runs in CI and uploads its report on failure. diff --git a/packages/harness/package.json b/packages/harness/package.json index 86fb4ff..2179a6d 100644 --- a/packages/harness/package.json +++ b/packages/harness/package.json @@ -5,12 +5,14 @@ "scripts": { "typecheck": "tsgo --noEmit", "test": "vitest run", - "scenario": "tsx --conditions=development src/run.ts" + "scenario": "tsx --conditions=development src/run.ts", + "test:e2e": "playwright test" }, "dependencies": { "sync": "workspace:*" }, "devDependencies": { + "@playwright/test": "^1.65.0", "@types/node": "^25.5.0", "tsx": "^4.19.3", "vitest": "^4.1.2" diff --git a/packages/harness/playwright.config.ts b/packages/harness/playwright.config.ts new file mode 100644 index 0000000..2aed8b3 --- /dev/null +++ b/packages/harness/playwright.config.ts @@ -0,0 +1,17 @@ +import { defineConfig } from "@playwright/test"; + +export default defineConfig({ + testDir: "./src", + testMatch: "**/*.e2e.ts", + workers: 1, + retries: 0, + timeout: 90_000, + expect: { timeout: 20_000 }, + reporter: [["list"], ["html", { open: "never" }]], + use: { + browserName: "chromium", + viewport: { width: 1440, height: 900 }, + trace: "retain-on-failure", + screenshot: "only-on-failure", + }, +}); diff --git a/packages/harness/src/board.e2e.ts b/packages/harness/src/board.e2e.ts new file mode 100644 index 0000000..8e6acd9 --- /dev/null +++ b/packages/harness/src/board.e2e.ts @@ -0,0 +1,89 @@ +import { test, expect } from "@playwright/test"; +import { startScenario } from "./runtime.js"; +import { SCENARIO_TITLE } from "./github.js"; + +test("polls queue entry, queue failure, and recovery through the real sync engine", async ({ + page, +}, testInfo) => { + const scenario = await startScenario(); + const errors: string[] = []; + const apiResponses: { url: string; status: number }[] = []; + page.on("pageerror", (error) => errors.push(error.message)); + page.on("console", (message) => { + if (message.type() === "error") errors.push(message.text()); + }); + page.on("response", (response) => { + if (response.url().includes("/api/")) { + apiResponses.push({ url: response.url(), status: response.status() }); + } + }); + await page.route("**/*", (route) => { + if (new URL(route.request().url()).origin !== scenario.url) { + errors.push(`Unexpected external request: ${route.request().url()}`); + return route.abort(); + } + return route.continue(); + }); + try { + await scenario.syncPrs(); + await page.goto(scenario.url); + const card = page.getByRole("listitem").filter({ hasText: SCENARIO_TITLE }); + await expect(card).toHaveCount(1); + await expect(card.getByText("passing", { exact: true })).toBeVisible(); + await expect( + card.getByRole("img", { name: "Ready for review", exact: true }), + ).toBeVisible(); + + scenario.setPhase("queued"); + await scenario.syncPrs(); + expect(await scenario.persistedPrs()).toMatchObject([ + { number: 101, inMergeQueue: true, ciStatus: "pending" }, + ]); + // No page reload or response mocking: the renderer's normal poll must + // discover the engine's newly persisted snapshot. + await expect( + card.getByRole("img", { name: "In merge queue", exact: true }), + ).toBeVisible(); + await expect(card.getByText("pending", { exact: true })).toBeVisible(); + await expect(card).toHaveCount(1); + + scenario.setPhase("queue-failed"); + await scenario.syncPrs(); + expect(await scenario.persistedPrs()).toMatchObject([ + { number: 101, inMergeQueue: false, ciStatus: "failure" }, + ]); + await expect( + card.getByRole("img", { name: "In merge queue", exact: true }), + ).toHaveCount(0); + await expect(card.getByText("failing", { exact: true })).toBeVisible(); + await expect(card).toHaveCount(1); + + scenario.setPhase("offline"); + await expect(scenario.syncPrs()).rejects.toThrow("Scenario sync failed"); + expect(await scenario.persistedPrs()).toMatchObject([ + { number: 101, ciStatus: "failure" }, + ]); + await expect(card.getByText("failing", { exact: true })).toBeVisible(); + + scenario.setPhase("in-flight"); + await scenario.syncPrs(); + await expect(card.getByText("passing", { exact: true })).toBeVisible(); + await expect(card).toHaveCount(1); + expect(await scenario.persistedPrs()).toHaveLength(1); + expect(errors).toEqual([]); + expect(apiResponses.length).toBeGreaterThan(3); + expect(apiResponses.every((response) => response.status === 200)).toBe( + true, + ); + } finally { + await testInfo.attach("scenario-diagnostics", { + body: JSON.stringify( + { ...scenario.diagnostics(), errors, apiResponses }, + null, + 2, + ), + contentType: "application/json", + }); + await scenario.close(); + } +}); diff --git a/packages/web/src/components/PrStateIcon.tsx b/packages/web/src/components/PrStateIcon.tsx index 5ae90c5..f3c7285 100644 --- a/packages/web/src/components/PrStateIcon.tsx +++ b/packages/web/src/components/PrStateIcon.tsx @@ -34,7 +34,13 @@ export function PrStateIcon({ switch (status) { case "merge-queue": - return ; + return ( + + ); case "merged": return ; case "closed": @@ -50,6 +56,12 @@ export function PrStateIcon({ ); case "ready": - return ; + return ( + + ); } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index eb7290b..5fb5c4c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -98,6 +98,9 @@ importers: specifier: workspace:* version: link:../sync devDependencies: + '@playwright/test': + specifier: ^1.65.0 + version: 1.65.0 '@types/node': specifier: ^25.5.0 version: 25.5.0 @@ -1165,6 +1168,11 @@ packages: resolution: {integrity: sha512-+1VkjdD0QBLPodGrJUeqarH8VAIvQODIbwh9XpP5Syisf7YoQgsJKPNFoqqLQlu+VQ/tVSshMR6loPMn8U+dPg==} engines: {node: '>=14'} + '@playwright/test@1.65.0': + resolution: {integrity: sha512-iGURLGuKDLakSEl7Pgfv8h7f+XLT+V861Wfhz0zlEl5nyFuKUopdkFBPKqTllTUp06aOwPuH8iuUEb3yp3MpbQ==} + engines: {node: '>=20'} + hasBin: true + '@radix-ui/primitive@1.1.3': resolution: {integrity: sha512-JTF99U/6XIjCBo0wqkU5sK10glYe27MRRsfwoiq5zzOEZLHU3A3KCMa5X/azekYRCJ0HlwI0crAXS/5dEHTzDg==} @@ -3965,6 +3973,16 @@ packages: resolution: {integrity: sha512-wQ0b/W4Fr01qtpHlqSqspcj3EhBvimsdh0KlHhH8HRZnMsEa0ea2fTULOXOS9ccQr3om+GcGRk4e+isrZWV8qQ==} engines: {node: '>=16.20.0'} + playwright-core@1.65.0: + resolution: {integrity: sha512-vNSKgH6YqtN7cdT/L334zo0a+YwkrpicyrHJlWWPIm/Q+Nn2SWInil01oT0mbu96U4GgoRtCYQD9CzJQgju+lA==} + engines: {node: '>=20'} + hasBin: true + + playwright@1.65.0: + resolution: {integrity: sha512-F3AMWcR44qMBt4kq+6Kb6mX7bjGXshktKFOPULd7reED6ZJWevjbNRqysP7q+AATE9kdPEZBibex1BYsvpVoeA==} + engines: {node: '>=20'} + hasBin: true + plist@3.1.1: resolution: {integrity: sha512-ZIfcLJC+7E7FBFnDxm9MPmt7D+DidyQ26lewieO75AdhA2ayMtsJSES0iWzqJQbcVRSrTufQoy0DR94xHue0oA==} engines: {node: '>=10.4.0'} @@ -5865,6 +5883,10 @@ snapshots: '@pkgjs/parseargs@0.11.0': optional: true + '@playwright/test@1.65.0': + dependencies: + playwright: 1.65.0 + '@radix-ui/primitive@1.1.3': {} '@radix-ui/react-compose-refs@1.1.2(@types/react@19.2.14)(react@19.2.4)': @@ -8972,6 +8994,12 @@ snapshots: pkce-challenge@5.0.1: {} + playwright-core@1.65.0: {} + + playwright@1.65.0: + dependencies: + playwright-core: 1.65.0 + plist@3.1.1: dependencies: '@xmldom/xmldom': 0.9.10