diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83b6e71e..8c98cf5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -45,10 +45,13 @@ jobs: if: matrix.node == 22 uses: ./.github/actions/setup-tex - run: npm run typecheck + # The runner image already has the libraries Chromium needs. Installing + # them (--with-deps) needs Ubuntu's package mirror, which the runner + # sometimes cannot resolve for an hour or more. - name: Install browser for PDF review tests if: matrix.node == 22 working-directory: engine - run: npx playwright install --with-deps chromium + run: npx playwright install chromium - run: npm test env: # The viewer's lock test drives the built review viewer in Chromium. @@ -68,8 +71,11 @@ jobs: # The static pages do not depend on the server's Node version. if: matrix.node == 22 run: | - engine/node_modules/.bin/playwright install --with-deps chromium + engine/node_modules/.bin/playwright install chromium node scripts/check-site-blog.mjs + node scripts/check-site.mjs + node scripts/check-site-analytics.mjs + node scripts/check-site-privacy.mjs node scripts/check-playground.mjs env: PAPILOT_BROWSER_CHANNEL: chromium @@ -98,7 +104,7 @@ jobs: uses: ./.github/actions/setup-tex - name: Install browser working-directory: engine - run: npx playwright install --with-deps chromium + run: npx playwright install chromium - run: npm run build # One step each, so one failure does not hide the others' results. - name: Review batching @@ -134,6 +140,151 @@ jobs: if-no-files-found: ignore retention-days: 7 + # Every other interface check, against the real built app with mocked or + # real services, real LaTeX where a check compiles. One step each, so one + # failure does not hide the others. + interface: + name: Interface checks + runs-on: ubuntu-latest + timeout-minutes: 45 + env: + PAPILOT_BROWSER_CHANNEL: chromium + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-node@v5 + with: + node-version: 22 + cache: npm + cache-dependency-path: | + package-lock.json + engine/package-lock.json + - run: npm ci + - run: npm run setup:review + - name: Install TeX + uses: ./.github/actions/setup-tex + - name: Install browser + working-directory: engine + run: npx playwright install chromium + - run: npm run build + - name: ai-review + run: node --import tsx scripts/check-ai-review.mjs + - name: appearance + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-appearance.mjs + - name: attachments + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-attachments.mjs + - name: build-problems + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-build-problems.mjs + - name: comparison-progress + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-comparison-progress.mjs + - name: compilation + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-compilation.mjs + - name: fast-decisions + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-fast-decisions.mjs + - name: git-panel + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-git-panel.mjs + - name: git-workspace + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-git-workspace.mjs + - name: imports + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-imports.mjs + - name: locales + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-locales.mjs + - name: manuscript-source + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-manuscript-source.mjs + - name: note-groups + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-note-groups.mjs + - name: note-new-pass + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-note-new-pass.mjs + - name: note-selection + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-note-selection.mjs + - name: papers + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-papers.mjs + - name: pdf-tools + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-pdf-tools.mjs + - name: pdf-words + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-pdf-words.mjs + - name: persistent-pdf + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-persistent-pdf.mjs + - name: places + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-places.mjs + - name: reply-duplicates + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-reply-duplicates.mjs + - name: review-notes + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-review-notes.mjs + - name: review-pdf-appearance + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-review-pdf-appearance.mjs + - name: review-tools + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-review-tools.mjs + - name: settings-layout + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-settings-layout.mjs + - name: skill-library + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-skill-library.mjs + - name: skills + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-skills.mjs + - name: source-edit + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-source-edit.mjs + - name: system-requirements + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-system-requirements.mjs + - name: task-model + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-task-model.mjs + - name: task-permissions + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-task-permissions.mjs + - name: task-preparation + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-task-preparation.mjs + - name: ui + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-ui.mjs + - name: unified-review + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-unified-review.mjs + - name: updates + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-updates.mjs + - name: workspace-switcher-notes + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-workspace-switcher-notes.mjs + - name: workspace-switcher + if: ${{ !cancelled() }} + run: node --import tsx scripts/check-workspace-switcher.mjs + - name: Keep screenshots of a failed check + if: failure() + uses: actions/upload-artifact@v4 + with: + name: interface-results + path: test-results/ + if-no-files-found: ignore + retention-days: 7 + # The same checks on a native Windows host: the CLI, server and review engine # run there too, and the tests that need POSIX signals skip themselves. windows: @@ -196,7 +347,7 @@ jobs: passed: name: CI passed if: always() - needs: [check, review, windows, changes, desktop] + needs: [check, review, interface, windows, changes, desktop] runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 @@ -204,6 +355,7 @@ jobs: env: CHECK_RESULT: ${{ needs.check.result }} REVIEW_RESULT: ${{ needs.review.result }} + INTERFACE_RESULT: ${{ needs.interface.result }} WINDOWS_RESULT: ${{ needs.windows.result }} CHANGES_RESULT: ${{ needs.changes.result }} DESKTOP_SCOPE: ${{ needs.changes.outputs.desktop }} diff --git a/.github/workflows/desktop.yml b/.github/workflows/desktop.yml index 216ba106..25006817 100644 --- a/.github/workflows/desktop.yml +++ b/.github/workflows/desktop.yml @@ -66,11 +66,54 @@ jobs: ~/AppData/Local/electron-builder/Cache key: electron-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('package-lock.json') }} restore-keys: electron-${{ runner.os }}-${{ runner.arch }}- - - name: Install Linux desktop dependencies + # The image has most of the packages the Linux smoke test needs. The rest + # come from a cache of their .deb files per image, like TeX's, so a run + # needs Ubuntu's mirror only on a new image: the runners sometimes cannot + # resolve it for an hour or more. + - name: Find missing Linux desktop packages + id: desktop-packages if: runner.os == 'Linux' run: | - sudo apt-get update -qq - sudo apt-get install -y libgtk-3-0 libnss3 libxss1 libxtst6 libasound2 libgbm1 libnotify4 libsecret-1-0 libfuse2 xvfb xauth openbox + missing=() + for package in libgtk-3-0 libnss3 libxss1 libxtst6 libasound2 libgbm1 libnotify4 libsecret-1-0 libfuse2 xvfb xauth openbox; do + dpkg -s "$package" >/dev/null 2>&1 || missing+=("$package") + done + echo "Missing: ${missing[*]:-none}" + echo "list=${missing[*]}" >> "$GITHUB_OUTPUT" + echo "dir=/tmp/papilot-desktop-debs" >> "$GITHUB_OUTPUT" + echo "key=desktop-debs-v1-$ImageOS-$ImageVersion-$(uname -m)-$(sha256sum <<< "${missing[*]}" | cut -c1-16)" >> "$GITHUB_OUTPUT" + - id: desktop-cache + if: runner.os == 'Linux' && steps.desktop-packages.outputs.list != '' + uses: actions/cache/restore@v5 + with: + path: ${{ steps.desktop-packages.outputs.dir }} + key: ${{ steps.desktop-packages.outputs.key }} + - name: Install Linux desktop packages from the cache + id: desktop-cached + if: steps.desktop-cache.outputs.cache-hit == 'true' + continue-on-error: true + env: + DIR: ${{ steps.desktop-packages.outputs.dir }} + run: sudo dpkg -i "$DIR"/*.deb + - name: Install Linux desktop packages from the mirror + id: desktop-download + if: steps.desktop-packages.outputs.list != '' && (steps.desktop-cache.outputs.cache-hit != 'true' || steps.desktop-cached.outcome == 'failure') + env: + DIR: ${{ steps.desktop-packages.outputs.dir }} + PACKAGES: ${{ steps.desktop-packages.outputs.list }} + run: | + apt=(-o Acquire::Retries=3 -o Acquire::http::Timeout=30 -o Dir::Cache::archives="$DIR" -o APT::Keep-Downloaded-Packages=true) + mkdir -p "$DIR/partial" && sudo chown _apt "$DIR/partial" + sudo apt-get "${apt[@]}" update -qq + sudo apt-get "${apt[@]}" install -y $PACKAGES + sudo rm -rf "$DIR/partial" "$DIR/lock" + sudo chown -R "$(id -u):$(id -g)" "$DIR" + # Only main saves, as for TeX: pull requests restore its cache. + - if: github.ref == 'refs/heads/main' && steps.desktop-download.outcome == 'success' + uses: actions/cache/save@v5 + with: + path: ${{ steps.desktop-packages.outputs.dir }} + key: ${{ steps.desktop-packages.outputs.key }} - run: npm ci # Making the installers takes most of a full build. The smoke tests # launch the unpacked app, which --dir builds by itself. diff --git a/scripts/check-fast-decisions.mjs b/scripts/check-fast-decisions.mjs index c1f0aaec..17f9fd6b 100644 --- a/scripts/check-fast-decisions.mjs +++ b/scripts/check-fast-decisions.mjs @@ -6,7 +6,7 @@ import fs from 'node:fs'; import path from 'node:path'; import Fastify from 'fastify'; import staticFiles from '@fastify/static'; -import { chromium, webkit, expect } from './playwright.mjs'; +import { afterRender, chromium, webkit, expect } from './playwright.mjs'; import { annotateReviewState } from '../src/server/review.ts'; fs.mkdirSync('test-results', { recursive: true }); @@ -66,7 +66,11 @@ async function open(page) { return route.fulfill({ json: view(extra) }); }); await page.goto(`${origin}/?task=flow`); + await ready(page); +} +async function ready(page) { await expect(page.locator('.change.current')).toHaveAttribute('data-stop', '0'); + await afterRender(page); } const decides = () => posts.filter(post => post.route === 'decide'); const current = page => page.locator('.change.current'); @@ -288,5 +292,5 @@ async function reloadFresh(page, status) { reset(status); await page.addInitScript(() => localStorage.clear()); await page.reload(); - await expect(current(page)).toHaveAttribute('data-stop', '0'); + await ready(page); } diff --git a/scripts/check-review-batching.mjs b/scripts/check-review-batching.mjs index 0ffe4e94..eae42dd5 100644 --- a/scripts/check-review-batching.mjs +++ b/scripts/check-review-batching.mjs @@ -4,7 +4,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import path from 'node:path'; import { EventEmitter } from 'node:events'; -import { chromium, expect } from './playwright.mjs'; +import { afterRender, chromium, expect } from './playwright.mjs'; import { Fixture, METHOD } from '../engine/tests/ts/helpers.ts'; import { createServer } from '../engine/src/server.ts'; import { Builder } from '../engine/src/core/build.ts'; @@ -53,6 +53,7 @@ try { page.on('pageerror', error => errors.push(error.message)); await page.goto(`http://127.0.0.1:${listener.port}/review/paper/?task=batch`); await expect(page.locator('.change.current')).toHaveAttribute('data-stop', '0'); + await afterRender(page); await page.keyboard.press('a'); await expect.poll(() => app.review.pageState(task.name).decisions.c001).toBe('accepted'); await page.keyboard.press(process.platform === 'darwin' ? 'Meta+z' : 'Control+z'); diff --git a/scripts/check-review-pdf-appearance.mjs b/scripts/check-review-pdf-appearance.mjs index 7b50c50c..207d7a8c 100644 --- a/scripts/check-review-pdf-appearance.mjs +++ b/scripts/check-review-pdf-appearance.mjs @@ -147,10 +147,12 @@ try { await page.reload(); await page.getByRole('button', { name: /^Review(?:,|$)/ }).click(); await expect(frame.locator('html')).toHaveAttribute('data-papilot-phone', ''); - await expect.poll(() => frame.locator('.change.current .dock').evaluate(dock => { - const box = dock.getBoundingClientRect(); - return box.bottom <= innerHeight && box.bottom >= innerHeight - 16 && box.left >= 0 && box.right <= innerWidth; - })).toBe(true); + // The decision dock floats just above the marking tools, fully on screen. + const dockPlacement = async () => { + const dock = await frame.locator('.change.current .dock').boundingBox(), tools = await page.locator('.review-marking-rail .pdf-rail').boundingBox(); + return Boolean(dock && tools) && dock.x >= 0 && dock.x + dock.width <= 390 && dock.y >= 0 && dock.y + dock.height <= tools.y && tools.y - (dock.y + dock.height) <= 24; + }; + await expect.poll(dockPlacement).toBe(true); assert.deepEqual(errors, []); console.log('PASS: all 12 UI/PDF/review appearance combinations, inline and standalone changes, preview filters, controls, LaTeX colors, per-edit action hover states, pending/decided toolbars at 320/500/700 px, localized two-digit numbers, pointer target sizes, and phone dock positioning.'); } catch (error) { diff --git a/scripts/check-skills.mjs b/scripts/check-skills.mjs index c2196e42..68f9c37f 100644 --- a/scripts/check-skills.mjs +++ b/scripts/check-skills.mjs @@ -21,7 +21,7 @@ const skills = [ { id: 'proofread', name: 'Proofread', description: 'Fix typos, grammar and punctuation without rewording', hint: 'Which part? Leave empty for the whole paper', source: 'builtin' }, { id: 'venue', name: 'Venue style', description: 'Follow the venue style guide', hint: '', icon: 'book', source: 'paper' }, ]; -const task = { id: 'skill-fixture-0001', projectId: 'paper', name: 'p-skill', title: 'Polish: the introduction', instructions: 'x', worktree: '/tmp/paper-tasks/p-skill', threadId: 't', turnId: null, model: null, status: 'reviewing', createdAt: 1, updatedAt: 1, feedbackCursor: 0, error: null, +const task = { id: 'skill-fixture-0001', projectId: 'paper', name: 'p-skill', title: 'Polish: the introduction', instructions: 'x', worktree: '/tmp/paper-tasks/p-skill', threadId: 't', turnId: null, model: null, status: 'reviewing', deferred: true, createdAt: 1, updatedAt: 1, feedbackCursor: 0, error: null, review: { state: 'done', round: 2, counts: { pending: 0, accepted: 2, rejected: 1, commented: 0 }, unsent: 3, comments: 0, files: ['sections/intro.tex'] } }; const snapshot = { version: 'test', csrf: 'fixture', projects: [{ id: 'paper', name: 'Paper', path: '/tmp/paper', createdAt: 1 }], tasks: [task], requests: [], runtime: { connected: true, authenticated: true, account: null, error: null, models: [] } }; const git = { remote: true, branch: 'main', upstream: 'origin/main', ahead: 0, behind: 0, head: { hash: 'abc1234', subject: 'Draft', date: 1 }, files: [{ path: 'sections/intro.tex', status: 'M', staged: false, lines: [3, 2] }] }; @@ -51,6 +51,8 @@ await page.route('**/api/**', async route => { } if (url.pathname.endsWith('/items')) return json({ items: taskItems.get(url.pathname.split('/')[3]) || items }); if (url.pathname.endsWith('/note-results')) return json({ notes: [] }); + // A decided task compiles what applying it writes, ahead of time. + if (url.pathname.endsWith('/predict')) return json({ ok: true }); if (url.pathname.endsWith('/git')) return json(git); // A finished review writes its commit message as soon as it opens. if (url.pathname.endsWith('/git/message') || url.pathname.endsWith('/commit-message')) return json({ message: 'Polish the introduction', agent: 'codex' }); @@ -107,10 +109,10 @@ try { assert.equal(messages[0].feedback, true); task.review = { ...task.review, unsent: 0, comments: 0 }; await sync(); await page.getByRole('button', { name: 'Chat', exact: true }).click(); - // The accepted change is in the manuscript; committing it is offered, not required. + // The accepted changes are saved when the author chooses: applied or committed. const commitCard = page.locator('.thread .commit-card'); - await expect(commitCard.getByRole('textbox', { name: 'Commit message', exact: true })).toHaveValue('Polish the introduction'); - await expect(commitCard.getByRole('button', { name: 'Commit', exact: true })).toBeEnabled(); + await expect(commitCard.getByRole('button', { name: 'Apply to workspace', exact: true })).toBeEnabled(); + await expect(commitCard.getByRole('textbox', { name: 'Commit message', exact: true })).toBeVisible(); await page.getByRole('button', { name: 'Split', exact: true }).click(); // "/" lists skills in the follow-up composer; the chosen one runs without text. diff --git a/scripts/ci-passed.sh b/scripts/ci-passed.sh index d34d9288..e37240ef 100644 --- a/scripts/ci-passed.sh +++ b/scripts/ci-passed.sh @@ -2,6 +2,7 @@ set -euo pipefail [[ "${CHECK_RESULT:-}" == success ]] || exit 1 [[ "${REVIEW_RESULT:-}" == success ]] || exit 1 +[[ "${INTERFACE_RESULT:-}" == success ]] || exit 1 [[ "${WINDOWS_RESULT:-}" == success ]] || exit 1 event=${GITHUB_EVENT_NAME:-} if [[ "$event" == push && "${GITHUB_REF:-}" != refs/heads/main ]]; then event=workflow_call; fi diff --git a/scripts/playwright.mjs b/scripts/playwright.mjs index c90d96c1..112e7fe1 100644 --- a/scripts/playwright.mjs +++ b/scripts/playwright.mjs @@ -5,3 +5,6 @@ import { expect as playwrightExpect } from '../engine/node_modules/@playwright/test/index.mjs'; export { chromium, webkit, _electron } from '../engine/node_modules/@playwright/test/index.mjs'; export const expect = playwrightExpect.configure({ timeout: 15_000 }); +// The review viewer listens for keys from an effect that runs after it shows +// the current change; a key pressed before then is not a decision. +export const afterRender = page => page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => setTimeout(resolve)))); diff --git a/src/web/note-option-settings.tsx b/src/web/note-option-settings.tsx index 5be751a0..6b0cf93f 100644 --- a/src/web/note-option-settings.tsx +++ b/src/web/note-option-settings.tsx @@ -27,7 +27,7 @@ export function NoteOptionSettings() { void (choice === 'off' ? saveNoteAlternatives(false) : saveNoteAlternatives(true, choice)); }}/> - + {help} ; diff --git a/src/web/style.css b/src/web/style.css index c12b90c6..858129a4 100644 --- a/src/web/style.css +++ b/src/web/style.css @@ -878,13 +878,11 @@ body.resizing iframe{pointer-events:none} .settings-note-count .effort-track input::-webkit-slider-thumb{width:16px;height:16px;margin-top:1px} .settings-note-count .effort-track input::-moz-range-thumb{width:16px;height:16px} .settings-note-count .note-count-scale{margin-top:3px} -.note-count-scale{position:relative;height:16px;font-size:11px} -.note-count-scale span{position:absolute;transform:translateX(-50%);white-space:nowrap} +/* The labels are spaced out from end to end rather than placed under each + stop, so long translations such as Spanish "Desactivado" never overlap. */ +.note-count-scale{font-size:11px} +.note-count-scale span{white-space:nowrap} .note-count-scale span.active{color:var(--text-2)} -.note-count-scale span:first-child{left:0;transform:none} -.note-count-scale span:last-child{right:0;transform:none} -/* Long translated Off labels need their own space on the smallest screens. */ -@media(max-width:360px){.note-count-scale span{position:static;transform:none}} @media(max-width:480px){.settings-note-count{align-self:flex-end}} .settings-name{font-size:14px;font-weight:500;color:var(--text)} .segmented{display:inline-flex;gap:2px;padding:3px;border-radius:var(--r-md);background:var(--bg-muted)} diff --git a/tests/ci-passed.test.ts b/tests/ci-passed.test.ts index dffc6b61..88f273d4 100644 --- a/tests/ci-passed.test.ts +++ b/tests/ci-passed.test.ts @@ -2,11 +2,11 @@ import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; import { test } from 'node:test'; -const run = (env: Record) => spawnSync('bash', ['scripts/ci-passed.sh'], { env: { ...process.env, GITHUB_EVENT_NAME: 'pull_request', GITHUB_REF: 'refs/heads/main', CHECK_RESULT: 'success', REVIEW_RESULT: 'success', WINDOWS_RESULT: 'success', CHANGES_RESULT: 'success', DESKTOP_SCOPE: 'quick', DESKTOP_RESULT: 'success', DESKTOP_VERIFIED: 'success', ...env } }).status; +const run = (env: Record) => spawnSync('bash', ['scripts/ci-passed.sh'], { env: { ...process.env, GITHUB_EVENT_NAME: 'pull_request', GITHUB_REF: 'refs/heads/main', CHECK_RESULT: 'success', REVIEW_RESULT: 'success', INTERFACE_RESULT: 'success', WINDOWS_RESULT: 'success', CHANGES_RESULT: 'success', DESKTOP_SCOPE: 'quick', DESKTOP_RESULT: 'success', DESKTOP_VERIFIED: 'success', ...env } }).status; test('CI requires explicit desktop success, including reusable workflow output', () => { assert.equal(run({}), 0); assert.equal(run({ GITHUB_EVENT_NAME: 'push', GITHUB_REF: 'refs/tags/v1.0.0', CHANGES_RESULT: 'skipped', DESKTOP_RESULT: 'skipped', DESKTOP_VERIFIED: '' }), 0); - for (const key of ['CHECK_RESULT', 'REVIEW_RESULT', 'WINDOWS_RESULT', 'CHANGES_RESULT', 'DESKTOP_RESULT', 'DESKTOP_VERIFIED']) + for (const key of ['CHECK_RESULT', 'REVIEW_RESULT', 'INTERFACE_RESULT', 'WINDOWS_RESULT', 'CHANGES_RESULT', 'DESKTOP_RESULT', 'DESKTOP_VERIFIED']) for (const value of ['', 'skipped', 'cancelled', 'failure']) assert.notEqual(run({ [key]: value }), 0, `${key}=${value}`); assert.notEqual(run({ DESKTOP_SCOPE: '' }), 0); assert.equal(run({ DESKTOP_SCOPE: 'none', DESKTOP_RESULT: 'skipped', DESKTOP_VERIFIED: '' }), 0); diff --git a/tests/review-state.test.ts b/tests/review-state.test.ts index 189f334b..bae95a28 100644 --- a/tests/review-state.test.ts +++ b/tests/review-state.test.ts @@ -87,7 +87,7 @@ test('state annotation preserves decision response extras and ignores non-state // Needs the built viewer and a browser; CI runs it on Node 22 with Chromium: // PAPILOT_REVIEW_BROWSER=1 npx tsx --test tests/review-state.test.ts test('viewer locks buttons and keyboard decisions, permits comments, and follows lock changes', { - skip: process.env.PAPILOT_REVIEW_BROWSER !== '1', timeout: 30000, + skip: process.env.PAPILOT_REVIEW_BROWSER !== '1', timeout: 90000, }, async () => { const [{ default: Fastify }, { default: staticFiles }] = await Promise.all([ import('fastify'), import('@fastify/static'), @@ -132,12 +132,18 @@ test('viewer locks buttons and keyboard decisions, permits comments, and follows await expect(page.locator('.change.current')).toBeVisible(); await expect(page.locator('#review-lock')).toContainText('Pause this task'); const decisions = page.locator('button.accept, button.reject, button.undo'); + // A key pressed before the viewer listens is lost; once the composer is + // open, C does nothing more, so pressing again is safe. + const openComposer = () => expect(async () => { + await page.keyboard.press('c'); + await expect(page.locator('#composer-text')).toBeVisible({ timeout: 1000 }); + }).toPass({ timeout: 15000 }); assert.ok(await decisions.count() >= 6, 'Includes the dock, individual units, Accept all, and Undo'); for (const button of await decisions.all()) await expect(button).toBeDisabled(); for (const button of await page.locator('button.comment').all()) await expect(button).toBeEnabled(); for (const key of ['a', 'r', 'u', process.platform === 'darwin' ? 'Meta+z' : 'Control+z']) await page.keyboard.press(key); assert.equal(writes.length, 0, 'Keyboard shortcuts do not bypass the lock'); - await page.keyboard.press('c'); + await openComposer(); await page.locator('#composer-text').fill('Comment while the task runs'); await page.getByRole('button', { name: 'Send', exact: true }).click(); await expect(page.locator('.composer')).toHaveCount(0); @@ -153,7 +159,7 @@ test('viewer locks buttons and keyboard decisions, permits comments, and follows assert.equal(writes[1].action, 'accept'); await expect(page.locator('.accept-all')).toBeEnabled(); - await page.keyboard.press('c'); + await openComposer(); await page.locator('#composer-text').fill('Keep this unsent draft'); tasks = []; await page.clock.fastForward(1600);