From d460614cd7ad9e047a2792139ea67e16caa82ea7 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Fri, 20 Mar 2026 14:12:06 +1000 Subject: [PATCH] fix: lots of desktop stability, better e2e error logging (#18300) --- .github/workflows/test.yml | 28 +- packages/app/e2e/actions.ts | 152 +++++++++- packages/app/e2e/fixtures.ts | 44 ++- .../app/e2e/projects/projects-switch.spec.ts | 53 +--- .../projects/workspace-new-session.spec.ts | 71 ++--- .../session/session-model-persistence.spec.ts | 38 +-- packages/app/src/context/global-sync.tsx | 1 + .../src/context/global-sync/child-store.ts | 10 + packages/app/src/pages/error.tsx | 10 +- packages/app/src/pages/layout.tsx | 47 +-- packages/app/src/pages/layout/helpers.ts | 4 +- .../app/src/pages/layout/sidebar-project.tsx | 7 +- .../src/pages/layout/sidebar-workspace.tsx | 4 +- packages/opencode/src/session/prompt.ts | 6 +- packages/opencode/src/session/summary.ts | 20 +- packages/ui/src/components/message-part.tsx | 277 +++++++++--------- 16 files changed, 458 insertions(+), 314 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c928e8223..9c58be30a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -50,20 +50,17 @@ jobs: e2e: name: e2e (${{ matrix.settings.name }}) - needs: unit strategy: fail-fast: false matrix: settings: - name: linux host: blacksmith-4vcpu-ubuntu-2404 - playwright: bunx playwright install --with-deps - name: windows host: blacksmith-4vcpu-windows-2025 - playwright: bunx playwright install runs-on: ${{ matrix.settings.host }} env: - PLAYWRIGHT_BROWSERS_PATH: 0 + PLAYWRIGHT_BROWSERS_PATH: ${{ github.workspace }}/.playwright-browsers defaults: run: shell: bash @@ -76,9 +73,28 @@ jobs: - name: Setup Bun uses: ./.github/actions/setup-bun - - name: Install Playwright browsers + - name: Read Playwright version + id: playwright-version + run: | + version=$(node -e 'console.log(require("./packages/app/package.json").devDependencies["@playwright/test"])') + echo "version=$version" >> "$GITHUB_OUTPUT" + + - name: Cache Playwright browsers + id: playwright-cache + uses: actions/cache@v4 + with: + path: ${{ github.workspace }}/.playwright-browsers + key: ${{ runner.os }}-${{ runner.arch }}-playwright-${{ steps.playwright-version.outputs.version }}-chromium + + - name: Install Playwright system dependencies + if: runner.os == 'Linux' working-directory: packages/app - run: ${{ matrix.settings.playwright }} + run: bunx playwright install-deps chromium + + - name: Install Playwright browsers + if: steps.playwright-cache.outputs.cache-hit != 'true' + working-directory: packages/app + run: bunx playwright install chromium - name: Run app e2e tests run: bun --cwd packages/app test:e2e:local diff --git a/packages/app/e2e/actions.ts b/packages/app/e2e/actions.ts index 88d71f94c..aced0756c 100644 --- a/packages/app/e2e/actions.ts +++ b/packages/app/e2e/actions.ts @@ -9,6 +9,7 @@ import { createSdk, modKey, resolveDirectory, serverUrl } from "./utils" import { dropdownMenuTriggerSelector, dropdownMenuContentSelector, + projectSwitchSelector, projectMenuTriggerSelector, projectCloseMenuSelector, projectWorkspacesToggleSelector, @@ -23,6 +24,16 @@ import { workspaceMenuTriggerSelector, } from "./selectors" +const phase = new WeakMap() + +export function setHealthPhase(page: Page, value: "test" | "cleanup") { + phase.set(page, value) +} + +export function healthPhase(page: Page) { + return phase.get(page) ?? "test" +} + export async function defocus(page: Page) { await page .evaluate(() => { @@ -196,11 +207,51 @@ export async function closeDialog(page: Page, dialog: Locator) { } export async function isSidebarClosed(page: Page) { - const button = page.getByRole("button", { name: /toggle sidebar/i }).first() - await expect(button).toBeVisible() + const button = await waitSidebarButton(page, "isSidebarClosed") return (await button.getAttribute("aria-expanded")) !== "true" } +async function errorBoundaryText(page: Page) { + const title = page.getByRole("heading", { name: /something went wrong/i }).first() + if (!(await title.isVisible().catch(() => false))) return + + const description = await page + .getByText(/an error occurred while loading the application\./i) + .first() + .textContent() + .catch(() => "") + const detail = await page + .getByRole("textbox", { name: /error details/i }) + .first() + .inputValue() + .catch(async () => + ( + (await page + .getByRole("textbox", { name: /error details/i }) + .first() + .textContent() + .catch(() => "")) ?? "" + ).trim(), + ) + + return [title ? "Error boundary" : "", description ?? "", detail ?? ""].filter(Boolean).join("\n") +} + +export async function assertHealthy(page: Page, context: string) { + const text = await errorBoundaryText(page) + if (!text) return + console.log(`[e2e:error-boundary][${context}]\n${text}`) + throw new Error(`Error boundary during ${context}\n${text}`) +} + +async function waitSidebarButton(page: Page, context: string) { + const button = page.getByRole("button", { name: /toggle sidebar/i }).first() + const boundary = page.getByRole("heading", { name: /something went wrong/i }).first() + await button.or(boundary).first().waitFor({ state: "visible", timeout: 10_000 }) + await assertHealthy(page, context) + return button +} + export async function toggleSidebar(page: Page) { await defocus(page) await page.keyboard.press(`${modKey}+B`) @@ -209,7 +260,7 @@ export async function toggleSidebar(page: Page) { export async function openSidebar(page: Page) { if (!(await isSidebarClosed(page))) return - const button = page.getByRole("button", { name: /toggle sidebar/i }).first() + const button = await waitSidebarButton(page, "openSidebar") await button.click() const opened = await expect(button) @@ -226,7 +277,7 @@ export async function openSidebar(page: Page) { export async function closeSidebar(page: Page) { if (await isSidebarClosed(page)) return - const button = page.getByRole("button", { name: /toggle sidebar/i }).first() + const button = await waitSidebarButton(page, "closeSidebar") await button.click() const closed = await expect(button) @@ -241,6 +292,7 @@ export async function closeSidebar(page: Page) { } export async function openSettings(page: Page) { + await assertHealthy(page, "openSettings") await defocus(page) const dialog = page.getByRole("dialog") @@ -253,6 +305,8 @@ export async function openSettings(page: Page) { if (opened) return dialog + await assertHealthy(page, "openSettings") + await page.getByRole("button", { name: "Settings" }).first().click() await expect(dialog).toBeVisible() return dialog @@ -314,10 +368,12 @@ export async function seedProjects(page: Page, input: { directory: string; extra export async function createTestProject() { const root = await fs.mkdtemp(path.join(os.tmpdir(), "opencode-e2e-project-")) + const id = `e2e-${path.basename(root)}` - await fs.writeFile(path.join(root, "README.md"), "# e2e\n") + await fs.writeFile(path.join(root, "README.md"), `# e2e\n\n${id}\n`) execSync("git init", { cwd: root, stdio: "ignore" }) + await fs.writeFile(path.join(root, ".git", "opencode"), id) execSync("git config core.fsmonitor false", { cwd: root, stdio: "ignore" }) execSync("git add -A", { cwd: root, stdio: "ignore" }) execSync('git -c user.name="e2e" -c user.email="e2e@example.com" commit -m "init" --allow-empty', { @@ -339,12 +395,24 @@ export function slugFromUrl(url: string) { return /\/([^/]+)\/session(?:[/?#]|$)/.exec(url)?.[1] ?? "" } +async function probeSession(page: Page) { + return page + .evaluate(() => { + const win = window as E2EWindow + const current = win.__opencode_e2e?.model?.current + if (!current) return null + return { dir: current.dir, sessionID: current.sessionID } + }) + .catch(() => null as { dir?: string; sessionID?: string } | null) +} + export async function waitSlug(page: Page, skip: string[] = []) { let prev = "" let next = "" await expect .poll( - () => { + async () => { + await assertHealthy(page, "waitSlug") const slug = slugFromUrl(page.url()) if (!slug) return "" if (skip.includes(slug)) return "" @@ -374,6 +442,7 @@ export async function waitDir(page: Page, directory: string) { await expect .poll( async () => { + await assertHealthy(page, "waitDir") const slug = slugFromUrl(page.url()) if (!slug) return "" return resolveSlug(slug) @@ -386,6 +455,69 @@ export async function waitDir(page: Page, directory: string) { return { directory: target, slug: base64Encode(target) } } +export async function waitSession(page: Page, input: { directory: string; sessionID?: string }) { + const target = await resolveDirectory(input.directory) + await expect + .poll( + async () => { + await assertHealthy(page, "waitSession") + const slug = slugFromUrl(page.url()) + if (!slug) return false + const resolved = await resolveSlug(slug).catch(() => undefined) + if (!resolved || resolved.directory !== target) return false + if (input.sessionID && sessionIDFromUrl(page.url()) !== input.sessionID) return false + + const state = await probeSession(page) + if (input.sessionID && (!state || state.sessionID !== input.sessionID)) return false + if (state?.dir) { + const dir = await resolveDirectory(state.dir).catch(() => state.dir ?? "") + if (dir !== target) return false + } + + return page + .locator(promptSelector) + .first() + .isVisible() + .catch(() => false) + }, + { timeout: 45_000 }, + ) + .toBe(true) + return { directory: target, slug: base64Encode(target) } +} + +export async function waitSessionSaved(directory: string, sessionID: string, timeout = 30_000) { + const sdk = createSdk(directory) + const target = await resolveDirectory(directory) + + await expect + .poll( + async () => { + const data = await sdk.session + .get({ sessionID }) + .then((x) => x.data) + .catch(() => undefined) + if (!data?.directory) return "" + return resolveDirectory(data.directory).catch(() => data.directory) + }, + { timeout }, + ) + .toBe(target) + + await expect + .poll( + async () => { + const items = await sdk.session + .messages({ sessionID, limit: 20 }) + .then((x) => x.data ?? []) + .catch(() => []) + return items.some((item) => item.info.role === "user") + }, + { timeout }, + ) + .toBe(true) +} + export function sessionIDFromUrl(url: string) { const match = /\/session\/([^/?#]+)/.exec(url) return match?.[1] @@ -797,8 +929,14 @@ export async function openStatusPopover(page: Page) { } export async function openProjectMenu(page: Page, projectSlug: string) { + await openSidebar(page) + const item = page.locator(projectSwitchSelector(projectSlug)).first() + await expect(item).toBeVisible() + await item.hover() + const trigger = page.locator(projectMenuTriggerSelector(projectSlug)).first() await expect(trigger).toHaveCount(1) + await expect(trigger).toBeVisible() const menu = page .locator(dropdownMenuContentSelector) @@ -807,7 +945,7 @@ export async function openProjectMenu(page: Page, projectSlug: string) { const close = menu.locator(projectCloseMenuSelector(projectSlug)).first() const clicked = await trigger - .click({ timeout: 1500 }) + .click({ force: true, timeout: 1500 }) .then(() => true) .catch(() => false) diff --git a/packages/app/e2e/fixtures.ts b/packages/app/e2e/fixtures.ts index 7bc994e50..7232df687 100644 --- a/packages/app/e2e/fixtures.ts +++ b/packages/app/e2e/fixtures.ts @@ -1,7 +1,16 @@ import { test as base, expect, type Page } from "@playwright/test" import type { E2EWindow } from "../src/testing/terminal" -import { cleanupSession, cleanupTestProject, createTestProject, seedProjects, sessionIDFromUrl } from "./actions" -import { promptSelector } from "./selectors" +import { + healthPhase, + cleanupSession, + cleanupTestProject, + createTestProject, + setHealthPhase, + seedProjects, + sessionIDFromUrl, + waitSlug, + waitSession, +} from "./actions" import { createSdk, dirSlug, getWorktree, sessionPath } from "./utils" export const settingsKey = "settings.v3" @@ -27,6 +36,29 @@ type WorkerFixtures = { } export const test = base.extend({ + page: async ({ page }, use) => { + let boundary: string | undefined + setHealthPhase(page, "test") + const consoleHandler = (msg: { text(): string }) => { + const text = msg.text() + if (!text.includes("[e2e:error-boundary]")) return + if (healthPhase(page) === "cleanup") { + console.warn(`[e2e:error-boundary][cleanup-warning]\n${text}`) + return + } + boundary ||= text + console.log(text) + } + const pageErrorHandler = (err: Error) => { + console.log(`[e2e:pageerror] ${err.stack || err.message}`) + } + page.on("console", consoleHandler) + page.on("pageerror", pageErrorHandler) + await use(page) + page.off("console", consoleHandler) + page.off("pageerror", pageErrorHandler) + if (boundary) throw new Error(boundary) + }, directory: [ async ({}, use) => { const directory = await getWorktree() @@ -48,21 +80,20 @@ export const test = base.extend({ const gotoSession = async (sessionID?: string) => { await page.goto(sessionPath(directory, sessionID)) - await expect(page.locator(promptSelector)).toBeVisible() + await waitSession(page, { directory, sessionID }) } await use(gotoSession) }, withProject: async ({ page }, use) => { await use(async (callback, options) => { const root = await createTestProject() - const slug = dirSlug(root) const sessions = new Map() const dirs = new Set() await seedStorage(page, { directory: root, extra: options?.extra }) const gotoSession = async (sessionID?: string) => { await page.goto(sessionPath(root, sessionID)) - await expect(page.locator(promptSelector)).toBeVisible() + await waitSession(page, { directory: root, sessionID }) const current = sessionIDFromUrl(page.url()) if (current) trackSession(current) } @@ -77,13 +108,16 @@ export const test = base.extend({ try { await gotoSession() + const slug = await waitSlug(page) return await callback({ directory: root, slug, gotoSession, trackSession, trackDirectory }) } finally { + setHealthPhase(page, "cleanup") await Promise.allSettled( Array.from(sessions, ([sessionID, directory]) => cleanupSession({ sessionID, directory })), ) await Promise.allSettled(Array.from(dirs, (directory) => cleanupTestProject(directory))) await cleanupTestProject(root) + setHealthPhase(page, "test") } }) }, diff --git a/packages/app/e2e/projects/projects-switch.spec.ts b/packages/app/e2e/projects/projects-switch.spec.ts index e9cbf868d..b46c1b407 100644 --- a/packages/app/e2e/projects/projects-switch.spec.ts +++ b/packages/app/e2e/projects/projects-switch.spec.ts @@ -1,5 +1,4 @@ import { base64Decode } from "@opencode-ai/util/encode" -import type { Page } from "@playwright/test" import { test, expect } from "../fixtures" import { defocus, @@ -7,43 +6,14 @@ import { cleanupTestProject, openSidebar, sessionIDFromUrl, - waitDir, + setWorkspacesEnabled, + waitSession, + waitSessionSaved, waitSlug, } from "../actions" import { projectSwitchSelector, promptSelector, workspaceItemSelector, workspaceNewSessionSelector } from "../selectors" import { dirSlug, resolveDirectory } from "../utils" -async function workspaces(page: Page, directory: string, enabled: boolean) { - await page.evaluate( - ({ directory, enabled }: { directory: string; enabled: boolean }) => { - const key = "opencode.global.dat:layout" - const raw = localStorage.getItem(key) - const data = raw ? JSON.parse(raw) : {} - const sidebar = data.sidebar && typeof data.sidebar === "object" ? data.sidebar : {} - const current = - sidebar.workspaces && typeof sidebar.workspaces === "object" && !Array.isArray(sidebar.workspaces) - ? sidebar.workspaces - : {} - const next = { ...current } - - if (enabled) next[directory] = true - if (!enabled) delete next[directory] - - localStorage.setItem( - key, - JSON.stringify({ - ...data, - sidebar: { - ...sidebar, - workspaces: next, - }, - }), - ) - }, - { directory, enabled }, - ) -} - test("can switch between projects from sidebar", async ({ page, withProject }) => { await page.setViewportSize({ width: 1400, height: 800 }) @@ -84,9 +54,7 @@ test("switching back to a project opens the latest workspace session", async ({ await withProject( async ({ directory, slug, trackSession, trackDirectory }) => { await defocus(page) - await workspaces(page, directory, true) - await page.reload() - await expect(page.locator(promptSelector)).toBeVisible() + await setWorkspacesEnabled(page, slug, true) await openSidebar(page) await expect(page.getByRole("button", { name: "New workspace" }).first()).toBeVisible() @@ -108,8 +76,7 @@ test("switching back to a project opens the latest workspace session", async ({ await expect(btn).toBeVisible() await btn.click({ force: true }) - await waitSlug(page) - await waitDir(page, space) + await waitSession(page, { directory: space }) // Create a session by sending a prompt const prompt = page.locator(promptSelector) @@ -123,6 +90,7 @@ test("switching back to a project opens the latest workspace session", async ({ const created = sessionIDFromUrl(page.url()) if (!created) throw new Error(`Failed to get session ID from url: ${page.url()}`) trackSession(created, space) + await waitSessionSaved(space, created) await expect(page).toHaveURL(new RegExp(`/${next}/session/${created}(?:[/?#]|$)`)) @@ -130,15 +98,14 @@ test("switching back to a project opens the latest workspace session", async ({ const otherButton = page.locator(projectSwitchSelector(otherSlug)).first() await expect(otherButton).toBeVisible() - await otherButton.click() - await expect(page).toHaveURL(new RegExp(`/${otherSlug}/session`)) + await otherButton.click({ force: true }) + await waitSession(page, { directory: other }) const rootButton = page.locator(projectSwitchSelector(slug)).first() await expect(rootButton).toBeVisible() - await rootButton.click() + await rootButton.click({ force: true }) - await waitDir(page, space) - await expect.poll(() => sessionIDFromUrl(page.url()) ?? "").toBe(created) + await waitSession(page, { directory: space, sessionID: created }) await expect(page).toHaveURL(new RegExp(`/session/${created}(?:[/?#]|$)`)) }, { extra: [other] }, diff --git a/packages/app/e2e/projects/workspace-new-session.spec.ts b/packages/app/e2e/projects/workspace-new-session.spec.ts index 0858f2627..3a7a6bbc2 100644 --- a/packages/app/e2e/projects/workspace-new-session.spec.ts +++ b/packages/app/e2e/projects/workspace-new-session.spec.ts @@ -1,6 +1,15 @@ import type { Page } from "@playwright/test" import { test, expect } from "../fixtures" -import { openSidebar, resolveSlug, sessionIDFromUrl, setWorkspacesEnabled, waitDir, waitSlug } from "../actions" +import { + openSidebar, + resolveSlug, + sessionIDFromUrl, + setWorkspacesEnabled, + waitDir, + waitSession, + waitSessionSaved, + waitSlug, +} from "../actions" import { promptSelector, workspaceItemSelector, workspaceNewSessionSelector } from "../selectors" import { createSdk } from "../utils" @@ -14,20 +23,7 @@ function button(space: { slug: string; raw: string }) { async function waitWorkspaceReady(page: Page, space: { slug: string; raw: string }) { await openSidebar(page) - await expect - .poll( - async () => { - const row = page.locator(item(space)).first() - try { - await row.hover({ timeout: 500 }) - return true - } catch { - return false - } - }, - { timeout: 60_000 }, - ) - .toBe(true) + await expect(page.locator(item(space)).first()).toBeVisible({ timeout: 60_000 }) } async function createWorkspace(page: Page, root: string, seen: string[]) { @@ -49,7 +45,8 @@ async function openWorkspaceNewSession(page: Page, space: { slug: string; raw: s await expect(next).toBeVisible() await next.click({ force: true }) - return waitDir(page, space.directory) + await waitSession(page, { directory: space.directory }) + await expect.poll(() => sessionIDFromUrl(page.url()) ?? "").toBe("") } async function createSessionFromWorkspace( @@ -57,39 +54,28 @@ async function createSessionFromWorkspace( space: { slug: string; raw: string; directory: string }, text: string, ) { - const next = await openWorkspaceNewSession(page, space) + await openWorkspaceNewSession(page, space) const prompt = page.locator(promptSelector) await expect(prompt).toBeVisible() - await expect(prompt).toBeEditable() - await prompt.click() - await expect(prompt).toBeFocused() await prompt.fill(text) - await expect.poll(async () => ((await prompt.textContent()) ?? "").trim()).toContain(text) - await prompt.press("Enter") - - await waitDir(page, next.directory) - await expect.poll(() => sessionIDFromUrl(page.url()) ?? "", { timeout: 30_000 }).not.toBe("") + await page.keyboard.press("Enter") + await expect.poll(() => sessionIDFromUrl(page.url()) ?? "", { timeout: 15_000 }).not.toBe("") const sessionID = sessionIDFromUrl(page.url()) if (!sessionID) throw new Error(`Failed to parse session id from url: ${page.url()}`) - await expect(page).toHaveURL(new RegExp(`/session/${sessionID}(?:[/?#]|$)`)) - return { sessionID, slug: next.slug } -} -async function sessionDirectory(directory: string, sessionID: string) { - const info = await createSdk(directory) - .session.get({ sessionID }) - .then((x) => x.data) + await waitSessionSaved(space.directory, sessionID) + await createSdk(space.directory) + .session.abort({ sessionID }) .catch(() => undefined) - if (!info) return "" - return info.directory + return sessionID } test("new sessions from sidebar workspace actions stay in selected workspace", async ({ page, withProject }) => { await page.setViewportSize({ width: 1400, height: 800 }) - await withProject(async ({ directory, slug: root, trackSession, trackDirectory }) => { + await withProject(async ({ slug: root, trackDirectory, trackSession }) => { await openSidebar(page) await setWorkspacesEnabled(page, root, true) @@ -101,17 +87,8 @@ test("new sessions from sidebar workspace actions stay in selected workspace", a trackDirectory(second.directory) await waitWorkspaceReady(page, second) - const firstSession = await createSessionFromWorkspace(page, first.slug, `workspace one ${Date.now()}`) - trackSession(firstSession.sessionID, first.directory) - - const secondSession = await createSessionFromWorkspace(page, second.slug, `workspace two ${Date.now()}`) - trackSession(secondSession.sessionID, second.directory) - - const thirdSession = await createSessionFromWorkspace(page, first.slug, `workspace one again ${Date.now()}`) - trackSession(thirdSession.sessionID, first.directory) - - await expect.poll(() => sessionDirectory(first.directory, firstSession.sessionID)).toBe(first.directory) - await expect.poll(() => sessionDirectory(second.directory, secondSession.sessionID)).toBe(second.directory) - await expect.poll(() => sessionDirectory(first.directory, thirdSession.sessionID)).toBe(first.directory) + trackSession(await createSessionFromWorkspace(page, first, `workspace one ${Date.now()}`), first.directory) + trackSession(await createSessionFromWorkspace(page, second, `workspace two ${Date.now()}`), second.directory) + trackSession(await createSessionFromWorkspace(page, first, `workspace one again ${Date.now()}`), first.directory) }) }) diff --git a/packages/app/e2e/session/session-model-persistence.spec.ts b/packages/app/e2e/session/session-model-persistence.spec.ts index 2c2e4e886..b758a3b3d 100644 --- a/packages/app/e2e/session/session-model-persistence.spec.ts +++ b/packages/app/e2e/session/session-model-persistence.spec.ts @@ -1,6 +1,14 @@ import type { Locator, Page } from "@playwright/test" import { test, expect } from "../fixtures" -import { openSidebar, resolveSlug, sessionIDFromUrl, setWorkspacesEnabled, waitSessionIdle, waitSlug } from "../actions" +import { + openSidebar, + resolveSlug, + sessionIDFromUrl, + setWorkspacesEnabled, + waitSession, + waitSessionIdle, + waitSlug, +} from "../actions" import { promptAgentSelector, promptModelSelector, @@ -29,8 +37,6 @@ const text = async (locator: Locator) => ((await locator.textContent()) ?? "").t const modelKey = (state: Probe | null) => (state?.model ? `${state.model.providerID}:${state.model.modelID}` : null) -const dirKey = (state: Probe | null) => state?.dir ?? "" - async function probe(page: Page): Promise { return page.evaluate(() => { const win = window as Window & { @@ -44,21 +50,6 @@ async function probe(page: Page): Promise { }) } -async function currentDir(page: Page) { - let hit = "" - await expect - .poll( - async () => { - const next = dirKey(await probe(page)) - if (next) hit = next - return next - }, - { timeout: 30_000 }, - ) - .not.toBe("") - return hit -} - async function read(page: Page): Promise