From 96b1d8f639991e896bc8d31afe64d6309bf3ccd2 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Fri, 13 Mar 2026 12:21:50 +1000 Subject: [PATCH] fix(app): stabilize todo dock e2e with composer probe (#17267) --- packages/app/e2e/AGENTS.md | 19 ++ .../e2e/session/session-composer-dock.spec.ts | 236 +++++++++++++----- .../composer/session-composer-region.tsx | 9 +- .../composer/session-composer-state.ts | 55 +++- .../session/composer/session-todo-dock.tsx | 21 ++ packages/app/src/testing/session-composer.ts | 84 +++++++ 6 files changed, 350 insertions(+), 74 deletions(-) create mode 100644 packages/app/src/testing/session-composer.ts diff --git a/packages/app/e2e/AGENTS.md b/packages/app/e2e/AGENTS.md index cb8080fb2..4b62634f0 100644 --- a/packages/app/e2e/AGENTS.md +++ b/packages/app/e2e/AGENTS.md @@ -176,6 +176,25 @@ await page.keyboard.press(`${modKey}+Comma`) // Open settings - These helpers use the fixture-enabled test-only terminal driver and wait for output after the terminal writer settles. - Avoid `waitForTimeout` and custom DOM or `data-*` readiness checks. +### Wait on state + +- Never use wall-clock waits like `page.waitForTimeout(...)` to make a test pass +- Avoid race-prone flows that assume work is finished after an action +- Wait or poll on observable state with `expect(...)`, `expect.poll(...)`, or existing helpers +- Prefer locator assertions like `toBeVisible()`, `toHaveCount(0)`, and `toHaveAttribute(...)` for normal UI state, and reserve `expect.poll(...)` for probe, mock, or backend state + +### Add hooks + +- If required state is not observable from the UI, add a small test-only driver or probe in app code instead of sleeps or fragile DOM checks +- Keep these hooks minimal and purpose-built, following the style of `packages/app/src/testing/terminal.ts` +- Test-only hooks must be inert unless explicitly enabled; do not add normal-runtime listeners, reactive subscriptions, or per-update allocations for e2e ceremony +- When mocking routes or APIs, expose explicit mock state and wait on that before asserting post-action UI + +### Prefer helpers + +- Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise +- Use direct locators when the interaction is simple and a helper would not add clarity + ## Writing New Tests 1. Choose appropriate folder or create new one diff --git a/packages/app/e2e/session/session-composer-dock.spec.ts b/packages/app/e2e/session/session-composer-dock.spec.ts index 055e8eed2..ffaede92a 100644 --- a/packages/app/e2e/session/session-composer-dock.spec.ts +++ b/packages/app/e2e/session/session-composer-dock.spec.ts @@ -1,12 +1,11 @@ import { test, expect } from "../fixtures" -import { cleanupSession, clearSessionDockSeed, seedSessionQuestion, seedSessionTodos } from "../actions" +import { composerEvent, type ComposerDriverState, type ComposerProbeState, type ComposerWindow } from "../../src/testing/session-composer" +import { cleanupSession, clearSessionDockSeed, seedSessionQuestion } from "../actions" import { permissionDockSelector, promptSelector, questionDockSelector, sessionComposerDockSelector, - sessionTodoDockSelector, - sessionTodoListSelector, sessionTodoToggleButtonSelector, } from "../selectors" @@ -42,12 +41,8 @@ async function withDockSeed(sdk: Sdk, sessionID: string, fn: () => Promise async function clearPermissionDock(page: any, label: RegExp) { const dock = page.locator(permissionDockSelector) - for (let i = 0; i < 3; i++) { - const count = await dock.count() - if (count === 0) return - await dock.getByRole("button", { name: label }).click() - await page.waitForTimeout(150) - } + await expect(dock).toBeVisible() + await dock.getByRole("button", { name: label }).click() } async function setAutoAccept(page: any, enabled: boolean) { @@ -59,6 +54,120 @@ async function setAutoAccept(page: any, enabled: boolean) { await expect(button).toHaveAttribute("aria-pressed", enabled ? "true" : "false") } +async function expectQuestionBlocked(page: any) { + await expect(page.locator(questionDockSelector)).toBeVisible() + await expect(page.locator(promptSelector)).toHaveCount(0) +} + +async function expectQuestionOpen(page: any) { + await expect(page.locator(questionDockSelector)).toHaveCount(0) + await expect(page.locator(promptSelector)).toBeVisible() +} + +async function expectPermissionBlocked(page: any) { + await expect(page.locator(permissionDockSelector)).toBeVisible() + await expect(page.locator(promptSelector)).toHaveCount(0) +} + +async function expectPermissionOpen(page: any) { + await expect(page.locator(permissionDockSelector)).toHaveCount(0) + await expect(page.locator(promptSelector)).toBeVisible() +} + +async function todoDock(page: any, sessionID: string) { + await page.addInitScript(() => { + const win = window as ComposerWindow + win.__opencode_e2e = { + ...win.__opencode_e2e, + composer: { + enabled: true, + sessions: {}, + }, + } + }) + + const write = async (driver: ComposerDriverState | undefined) => { + await page.evaluate( + (input) => { + const win = window as ComposerWindow + const composer = win.__opencode_e2e?.composer + if (!composer?.enabled) throw new Error("Composer e2e driver is not enabled") + composer.sessions ??= {} + const prev = composer.sessions[input.sessionID] ?? {} + if (!input.driver) { + if (!prev.probe) { + delete composer.sessions[input.sessionID] + } else { + composer.sessions[input.sessionID] = { probe: prev.probe } + } + } else { + composer.sessions[input.sessionID] = { + ...prev, + driver: input.driver, + } + } + window.dispatchEvent(new CustomEvent(input.event, { detail: { sessionID: input.sessionID } })) + }, + { event: composerEvent, sessionID, driver }, + ) + } + + const read = () => + page.evaluate((sessionID) => { + const win = window as ComposerWindow + return win.__opencode_e2e?.composer?.sessions?.[sessionID]?.probe ?? null + }, sessionID) as Promise + + const api = { + async clear() { + await write(undefined) + return api + }, + async open(todos: NonNullable) { + await write({ live: true, todos }) + return api + }, + async finish(todos: NonNullable) { + await write({ live: false, todos }) + return api + }, + async expectOpen(states: ComposerProbeState["states"]) { + await expect.poll(read, { timeout: 10_000 }).toMatchObject({ + mounted: true, + collapsed: false, + hidden: false, + count: states.length, + states, + }) + return api + }, + async expectCollapsed(states: ComposerProbeState["states"]) { + await expect.poll(read, { timeout: 10_000 }).toMatchObject({ + mounted: true, + collapsed: true, + hidden: true, + count: states.length, + states, + }) + return api + }, + async expectClosed() { + await expect.poll(read, { timeout: 10_000 }).toMatchObject({ mounted: false }) + return api + }, + async collapse() { + await page.locator(sessionTodoToggleButtonSelector).click() + return api + }, + async expand() { + await page.locator(sessionTodoToggleButtonSelector).click() + return api + }, + } + + return api +} + async function withMockPermission( page: any, request: { @@ -70,7 +179,7 @@ async function withMockPermission( always?: string[] }, opts: { child?: any } | undefined, - fn: () => Promise, + fn: (state: { resolved: () => Promise }) => Promise, ) { let pending = [ { @@ -119,8 +228,14 @@ async function withMockPermission( if (sessionList) await page.route("**/session?*", sessionList) + const state = { + async resolved() { + await expect.poll(() => pending.length, { timeout: 10_000 }).toBe(0) + }, + } + try { - return await fn() + return await fn(state) } finally { await page.unroute("**/permission", list) await page.unroute("**/session/*/permissions/*", reply) @@ -173,14 +288,12 @@ test("blocked question flow unblocks after submit", async ({ page, sdk, gotoSess }) const dock = page.locator(questionDockSelector) - await expect.poll(() => dock.count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(promptSelector)).toHaveCount(0) + await expectQuestionBlocked(page) await dock.locator('[data-slot="question-option"]').first().click() await dock.getByRole("button", { name: /submit/i }).click() - await expect.poll(() => page.locator(questionDockSelector).count(), { timeout: 10_000 }).toBe(0) - await expect(page.locator(promptSelector)).toBeVisible() + await expectQuestionOpen(page) }) }) }) @@ -199,15 +312,14 @@ test("blocked permission flow supports allow once", async ({ page, sdk, gotoSess metadata: { description: "Need permission for command" }, }, undefined, - async () => { + async (state) => { await page.goto(page.url()) - await expect.poll(() => page.locator(permissionDockSelector).count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(promptSelector)).toHaveCount(0) + await expectPermissionBlocked(page) await clearPermissionDock(page, /allow once/i) + await state.resolved() await page.goto(page.url()) - await expect.poll(() => page.locator(permissionDockSelector).count(), { timeout: 10_000 }).toBe(0) - await expect(page.locator(promptSelector)).toBeVisible() + await expectPermissionOpen(page) }, ) }) @@ -226,15 +338,14 @@ test("blocked permission flow supports reject", async ({ page, sdk, gotoSession patterns: ["/tmp/opencode-e2e-perm-reject"], }, undefined, - async () => { + async (state) => { await page.goto(page.url()) - await expect.poll(() => page.locator(permissionDockSelector).count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(promptSelector)).toHaveCount(0) + await expectPermissionBlocked(page) await clearPermissionDock(page, /deny/i) + await state.resolved() await page.goto(page.url()) - await expect.poll(() => page.locator(permissionDockSelector).count(), { timeout: 10_000 }).toBe(0) - await expect(page.locator(promptSelector)).toBeVisible() + await expectPermissionOpen(page) }, ) }) @@ -254,15 +365,14 @@ test("blocked permission flow supports allow always", async ({ page, sdk, gotoSe metadata: { description: "Need permission for command" }, }, undefined, - async () => { + async (state) => { await page.goto(page.url()) - await expect.poll(() => page.locator(permissionDockSelector).count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(promptSelector)).toHaveCount(0) + await expectPermissionBlocked(page) await clearPermissionDock(page, /allow always/i) + await state.resolved() await page.goto(page.url()) - await expect.poll(() => page.locator(permissionDockSelector).count(), { timeout: 10_000 }).toBe(0) - await expect(page.locator(promptSelector)).toBeVisible() + await expectPermissionOpen(page) }, ) }) @@ -301,14 +411,12 @@ test("child session question request blocks parent dock and unblocks after submi }) const dock = page.locator(questionDockSelector) - await expect.poll(() => dock.count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(promptSelector)).toHaveCount(0) + await expectQuestionBlocked(page) await dock.locator('[data-slot="question-option"]').first().click() await dock.getByRole("button", { name: /submit/i }).click() - await expect.poll(() => page.locator(questionDockSelector).count(), { timeout: 10_000 }).toBe(0) - await expect(page.locator(promptSelector)).toBeVisible() + await expectQuestionOpen(page) }) } finally { await cleanupSession({ sdk, sessionID: child.id }) @@ -344,17 +452,15 @@ test("child session permission request blocks parent dock and supports allow onc metadata: { description: "Need child permission" }, }, { child }, - async () => { + async (state) => { await page.goto(page.url()) - const dock = page.locator(permissionDockSelector) - await expect.poll(() => dock.count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(promptSelector)).toHaveCount(0) + await expectPermissionBlocked(page) await clearPermissionDock(page, /allow once/i) + await state.resolved() await page.goto(page.url()) - await expect.poll(() => page.locator(permissionDockSelector).count(), { timeout: 10_000 }).toBe(0) - await expect(page.locator(promptSelector)).toBeVisible() + await expectPermissionOpen(page) }, ) } finally { @@ -365,36 +471,31 @@ test("child session permission request blocks parent dock and supports allow onc test("todo dock transitions and collapse behavior", async ({ page, sdk, gotoSession }) => { await withDockSession(sdk, "e2e composer dock todo", async (session) => { - await withDockSeed(sdk, session.id, async () => { - await gotoSession(session.id) + const dock = await todoDock(page, session.id) + await gotoSession(session.id) + await expect(page.locator(sessionComposerDockSelector)).toBeVisible() - await seedSessionTodos(sdk, { - sessionID: session.id, - todos: [ - { content: "first task", status: "pending", priority: "high" }, - { content: "second task", status: "in_progress", priority: "medium" }, - ], - }) + try { + await dock.open([ + { content: "first task", status: "pending", priority: "high" }, + { content: "second task", status: "in_progress", priority: "medium" }, + ]) + await dock.expectOpen(["pending", "in_progress"]) - await expect.poll(() => page.locator(sessionTodoDockSelector).count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(sessionTodoListSelector)).toBeVisible() + await dock.collapse() + await dock.expectCollapsed(["pending", "in_progress"]) - await page.locator(sessionTodoToggleButtonSelector).click() - await expect(page.locator(sessionTodoListSelector)).toBeHidden() + await dock.expand() + await dock.expectOpen(["pending", "in_progress"]) - await page.locator(sessionTodoToggleButtonSelector).click() - await expect(page.locator(sessionTodoListSelector)).toBeVisible() - - await seedSessionTodos(sdk, { - sessionID: session.id, - todos: [ - { content: "first task", status: "completed", priority: "high" }, - { content: "second task", status: "cancelled", priority: "medium" }, - ], - }) - - await expect.poll(() => page.locator(sessionTodoDockSelector).count(), { timeout: 10_000 }).toBe(0) - }) + await dock.finish([ + { content: "first task", status: "completed", priority: "high" }, + { content: "second task", status: "cancelled", priority: "medium" }, + ]) + await dock.expectClosed() + } finally { + await dock.clear() + } }) }) @@ -414,8 +515,7 @@ test("keyboard focus stays off prompt while blocked", async ({ page, sdk, gotoSe ], }) - await expect.poll(() => page.locator(questionDockSelector).count(), { timeout: 10_000 }).toBe(1) - await expect(page.locator(promptSelector)).toHaveCount(0) + await expectQuestionBlocked(page) await page.locator("main").click({ position: { x: 5, y: 5 } }) await page.keyboard.type("abc") diff --git a/packages/app/src/pages/session/composer/session-composer-region.tsx b/packages/app/src/pages/session/composer/session-composer-region.tsx index 2034fbead..84f77ea4a 100644 --- a/packages/app/src/pages/session/composer/session-composer-region.tsx +++ b/packages/app/src/pages/session/composer/session-composer-region.tsx @@ -44,9 +44,9 @@ export function SessionComposerRegion(props: { }) { const prompt = usePrompt() const language = useLanguage() - const { sessionKey } = useSessionKey() + const route = useSessionKey() - const handoffPrompt = createMemo(() => getSessionHandoff(sessionKey())?.prompt) + const handoffPrompt = createMemo(() => getSessionHandoff(route.sessionKey())?.prompt) const previewPrompt = () => prompt @@ -62,7 +62,7 @@ export function SessionComposerRegion(props: { createEffect(() => { if (!prompt.ready()) return - setSessionHandoff(sessionKey(), { prompt: previewPrompt() }) + setSessionHandoff(route.sessionKey(), { prompt: previewPrompt() }) }) const [store, setStore] = createStore({ @@ -85,7 +85,7 @@ export function SessionComposerRegion(props: { } createEffect(() => { - sessionKey() + route.sessionKey() const ready = props.ready const delay = 140 @@ -194,6 +194,7 @@ export function SessionComposerRegion(props: { >
setStore("body", el)}> return !!permissionRequest() || !!questionRequest() }) + const [test, setTest] = createStore({ + on: false, + live: undefined as boolean | undefined, + todos: undefined as Todo[] | undefined, + }) + + const pull = () => { + const id = params.id + if (!id) { + setTest({ on: false, live: undefined, todos: undefined }) + return + } + + const next = composerDriver(id) + if (!next) { + setTest({ on: false, live: undefined, todos: undefined }) + return + } + + setTest({ + on: true, + live: next.live, + todos: next.todos?.map((todo) => ({ ...todo })), + }) + } + + onMount(() => { + if (!composerEnabled()) return + + pull() + createEffect(on(() => params.id, pull, { defer: true })) + + const onEvent = (event: Event) => { + const detail = (event as CustomEvent<{ sessionID?: string }>).detail + if (detail?.sessionID !== params.id) return + pull() + } + + window.addEventListener(composerEvent, onEvent) + onCleanup(() => window.removeEventListener(composerEvent, onEvent)) + }) + const todos = createMemo((): Todo[] => { + if (test.on && test.todos !== undefined) return test.todos const id = params.id if (!id) return [] return globalSync.data.session_todo[id] ?? [] @@ -64,7 +108,10 @@ export function createSessionComposerState(options?: { closeMs?: number | (() => }) const busy = createMemo(() => status().type !== "idle") - const live = createMemo(() => busy() || blocked()) + const live = createMemo(() => { + if (test.on && test.live !== undefined) return test.live + return busy() || blocked() + }) const [store, setStore] = createStore({ responding: undefined as string | undefined, @@ -116,6 +163,10 @@ export function createSessionComposerState(options?: { closeMs?: number | (() => // Keep stale turn todos from reopening if the model never clears them. const clear = () => { + if (test.on && test.todos !== undefined) { + setTest("todos", []) + return + } const id = params.id if (!id) return globalSync.todo.set(id, []) diff --git a/packages/app/src/pages/session/composer/session-todo-dock.tsx b/packages/app/src/pages/session/composer/session-todo-dock.tsx index c7907bb54..5500de97a 100644 --- a/packages/app/src/pages/session/composer/session-todo-dock.tsx +++ b/packages/app/src/pages/session/composer/session-todo-dock.tsx @@ -8,6 +8,7 @@ import { TextReveal } from "@opencode-ai/ui/text-reveal" import { TextStrikethrough } from "@opencode-ai/ui/text-strikethrough" import { Index, createEffect, createMemo, on, onCleanup } from "solid-js" import { createStore } from "solid-js/store" +import { composerEnabled, composerProbe } from "@/testing/session-composer" function dot(status: Todo["status"]) { if (status !== "in_progress") return undefined @@ -35,6 +36,7 @@ function dot(status: Todo["status"]) { } export function SessionTodoDock(props: { + sessionID?: string todos: Todo[] title: string collapseLabel: string @@ -69,6 +71,8 @@ export function SessionTodoDock(props: { const off = createMemo(() => hide() > 0.98) const turn = createMemo(() => Math.max(0, Math.min(1, value()))) const full = createMemo(() => Math.max(78, store.height)) + const e2e = composerEnabled() + const probe = composerProbe(props.sessionID) let contentRef: HTMLDivElement | undefined createEffect(() => { @@ -83,6 +87,23 @@ export function SessionTodoDock(props: { onCleanup(() => observer.disconnect()) }) + createEffect(() => { + if (!e2e) return + + probe.set({ + mounted: true, + collapsed: store.collapsed, + hidden: store.collapsed || off(), + count: props.todos.length, + states: props.todos.map((todo) => todo.status), + }) + }) + + onCleanup(() => { + if (!e2e) return + probe.drop() + }) + return ( > +} + +export type ComposerProbeState = { + mounted: boolean + collapsed: boolean + hidden: boolean + count: number + states: Todo["status"][] +} + +type ComposerState = { + driver?: ComposerDriverState + probe?: ComposerProbeState +} + +export type ComposerWindow = Window & { + __opencode_e2e?: { + composer?: { + enabled?: boolean + sessions?: Record + } + } +} + +const clone = (driver: ComposerDriverState) => ({ + live: driver.live, + todos: driver.todos?.map((todo) => ({ ...todo })), +}) + +export const composerEnabled = () => { + if (typeof window === "undefined") return false + return (window as ComposerWindow).__opencode_e2e?.composer?.enabled === true +} + +const root = () => { + if (!composerEnabled()) return + const state = (window as ComposerWindow).__opencode_e2e?.composer + if (!state) return + state.sessions ??= {} + return state.sessions +} + +export const composerDriver = (sessionID?: string) => { + if (!sessionID) return + const state = root()?.[sessionID]?.driver + if (!state) return + return clone(state) +} + +export const composerProbe = (sessionID?: string) => { + const set = (next: ComposerProbeState) => { + if (!sessionID) return + const sessions = root() + if (!sessions) return + const prev = sessions[sessionID] ?? {} + sessions[sessionID] = { + ...prev, + probe: { + ...next, + states: [...next.states], + }, + } + } + + return { + set, + drop() { + set({ + mounted: false, + collapsed: false, + hidden: true, + count: 0, + states: [], + }) + }, + } +}