From f65bf671f2612e546714f6a05d7b6a07b2c25f26 Mon Sep 17 00:00:00 2001 From: Cui Chen Date: Wed, 11 Nov 2026 20:58:30 +0800 Subject: [PATCH 1/7] Run every remaining browser check in CI A new Interface checks job runs each check script that only npm scripts reached, one step each, and the Node 22 job runs the website checks next to the blog check. CI passed requires the new job. --- .github/workflows/ci.yml | 151 ++++++++++++++++++++++++++++++++++++++- scripts/ci-passed.sh | 1 + tests/ci-passed.test.ts | 4 +- 3 files changed, 153 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 83b6e71e..f22ac9a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,6 +70,9 @@ jobs: run: | engine/node_modules/.bin/playwright install --with-deps 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 @@ -134,6 +137,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 --with-deps 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 +344,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 +352,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/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/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); From 68851532de36ebe33e01fc602985b2ee992f6500 Mon Sep 17 00:00:00 2001 From: Cui Chen Date: Wed, 11 Nov 2026 22:20:15 +0800 Subject: [PATCH 2/7] Fix the four browser checks that failed once they ran in CI - check-skills still expected the commit form of tasks that accept straight into the manuscript, removed in #128; it now checks the save card of a deferred task. - check-review-pdf-appearance expected the phone decision dock at the bottom edge; since the mobile redesign it floats just above the marking tools, which the check now verifies. - check-fast-decisions pressed keys as soon as the current change was shown, before the viewer's key listener was attached; it waits a frame. - On Linux fonts, Spanish "Desactivado" overlapped "Pocas" under the note-alternatives slider at 390 px. On phones the labels are now spaced out instead of placed under the stops. --- scripts/check-fast-decisions.mjs | 8 +++++++- scripts/check-review-pdf-appearance.mjs | 10 ++++++---- scripts/check-skills.mjs | 10 ++++++---- src/web/style.css | 5 +++-- 4 files changed, 22 insertions(+), 11 deletions(-) diff --git a/scripts/check-fast-decisions.mjs b/scripts/check-fast-decisions.mjs index c1f0aaec..2ba57075 100644 --- a/scripts/check-fast-decisions.mjs +++ b/scripts/check-fast-decisions.mjs @@ -66,7 +66,13 @@ async function open(page) { return route.fulfill({ json: view(extra) }); }); await page.goto(`${origin}/?task=flow`); + await ready(page); +} +// The viewer listens for keys from an effect that runs after it first +// renders the current change; a key pressed before then is not a decision. +async function ready(page) { await expect(page.locator('.change.current')).toHaveAttribute('data-stop', '0'); + await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => setTimeout(resolve)))); } const decides = () => posts.filter(post => post.route === 'decide'); const current = page => page.locator('.change.current'); @@ -288,5 +294,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-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/src/web/style.css b/src/web/style.css index c12b90c6..051fc6fa 100644 --- a/src/web/style.css +++ b/src/web/style.css @@ -883,8 +883,9 @@ body.resizing iframe{pointer-events:none} .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}} +/* On phones the stops are too close for long translated labels such as + Spanish "Desactivado": they are spaced out instead of placed under them. */ +@media(max-width:480px){.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)} From c6853dfaa7a949f8630ece32269651fc3709caa4 Mon Sep 17 00:00:00 2001 From: Cui Chen Date: Wed, 11 Nov 2026 23:36:20 +0800 Subject: [PATCH 3/7] Install browsers and desktop libraries without the Ubuntu mirror For over an hour the runners could not resolve Ubuntu's archive, so every Linux job failed while installing Chromium's system libraries (--with-deps) or the desktop smoke test's packages. The image already has the libraries Chromium needs, and most desktop packages: Chromium installs without --with-deps, and the desktop step asks the mirror only for packages the image lacks. --- .github/workflows/ci.yml | 11 +++++++---- .github/workflows/desktop.yml | 13 +++++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f22ac9a9..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,7 +71,7 @@ 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 @@ -101,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 @@ -161,7 +164,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 - name: ai-review run: node --import tsx scripts/check-ai-review.mjs diff --git a/.github/workflows/desktop.yml b/.github/workflows/desktop.yml index 216ba106..ff45e889 100644 --- a/.github/workflows/desktop.yml +++ b/.github/workflows/desktop.yml @@ -69,8 +69,17 @@ jobs: - name: Install Linux desktop dependencies 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 + # The image has most of these; only missing ones need the package + # mirror, which the runner sometimes cannot resolve. + 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}" + if [ ${#missing[@]} -gt 0 ]; then + sudo apt-get -o Acquire::Retries=3 update -qq + sudo apt-get -o Acquire::Retries=3 install -y "${missing[@]}" + fi - run: npm ci # Making the installers takes most of a full build. The smoke tests # launch the unpacked app, which --dir builds by itself. From 387d41768b2596ca3a8e0d81e4fb08a686e8c12e Mon Sep 17 00:00:00 2001 From: Cui Chen Date: Wed, 11 Nov 2026 23:52:43 +0800 Subject: [PATCH 4/7] Space the note-alternatives labels out at every width Placing each label under its stop overlapped Spanish "Desactivado" and "Pocas" on Linux fonts at 768 px too, not only on phones: the space depends on the settings card, not the window. The labels are now spaced out from end to end at every width. --- src/web/note-option-settings.tsx | 2 +- src/web/style.css | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) 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 051fc6fa..858129a4 100644 --- a/src/web/style.css +++ b/src/web/style.css @@ -878,14 +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} -/* On phones the stops are too close for long translated labels such as - Spanish "Desactivado": they are spaced out instead of placed under them. */ -@media(max-width:480px){.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)} From 6c7fd3faa512fd34ee997519a516963f75689502 Mon Sep 17 00:00:00 2001 From: Cui Chen Date: Wed, 11 Nov 2026 23:54:16 +0800 Subject: [PATCH 5/7] Cache the Linux desktop smoke test's packages per image The image lacks libnotify4, libfuse2 and openbox, so the desktop smoke test still needed Ubuntu's mirror while the runners could not resolve it. Their .deb files are now cached per image, as TeX's are: main saves them, and pull requests install from the cache without the mirror. --- .github/workflows/desktop.yml | 48 ++++++++++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/.github/workflows/desktop.yml b/.github/workflows/desktop.yml index ff45e889..25006817 100644 --- a/.github/workflows/desktop.yml +++ b/.github/workflows/desktop.yml @@ -66,20 +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: | - # The image has most of these; only missing ones need the package - # mirror, which the runner sometimes cannot resolve. 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}" - if [ ${#missing[@]} -gt 0 ]; then - sudo apt-get -o Acquire::Retries=3 update -qq - sudo apt-get -o Acquire::Retries=3 install -y "${missing[@]}" - fi + 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. From 666224f128a19295e11e0615a5059b6b3711d26d Mon Sep 17 00:00:00 2001 From: Cui Chen Date: Thu, 12 Nov 2026 00:02:24 +0800 Subject: [PATCH 6/7] Press review keys only after the viewer listens for them check-review-batching pressed Accept as soon as the current change was shown, before the viewer's key listener effect had run, and the key was lost now and then on CI. A shared afterRender wait in scripts/playwright.mjs now precedes the first key in both review keyboard checks. --- scripts/check-fast-decisions.mjs | 6 ++---- scripts/check-review-batching.mjs | 3 ++- scripts/playwright.mjs | 3 +++ 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/check-fast-decisions.mjs b/scripts/check-fast-decisions.mjs index 2ba57075..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 }); @@ -68,11 +68,9 @@ async function open(page) { await page.goto(`${origin}/?task=flow`); await ready(page); } -// The viewer listens for keys from an effect that runs after it first -// renders the current change; a key pressed before then is not a decision. async function ready(page) { await expect(page.locator('.change.current')).toHaveAttribute('data-stop', '0'); - await page.evaluate(() => new Promise(resolve => requestAnimationFrame(() => setTimeout(resolve)))); + await afterRender(page); } const decides = () => posts.filter(post => post.route === 'decide'); const current = page => page.locator('.change.current'); 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/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)))); From 84cfe0e38de6822c079503847fdc49796ce40a45 Mon Sep 17 00:00:00 2001 From: Cui Chen Date: Thu, 12 Nov 2026 00:14:24 +0800 Subject: [PATCH 7/7] Retry opening the composer in the viewer lock test On CI's Node 22 job, with every test file running at once, the first C could arrive before the viewer listened for keys; the composer never opened and the test waited out its 30 s. It now presses C until the composer shows (C does nothing once it is open) and allows 90 s. --- tests/review-state.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) 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);