From 389da5a0dca7ee66f0190a04ea9b0e5fff9bab88 Mon Sep 17 00:00:00 2001 From: liuxiaocs7 Date: Tue, 25 Aug 2026 04:15:50 +0800 Subject: [PATCH] test(desktop): scope slash menu refresh observer Limit the regression watcher to the open slash popover and the three projection refresh calls. Wait on each session Skill projection request before asserting that the original listbox and Skills group remain connected. Fixes #3727 Generated-by: OpenAI Codex --- apps/desktop/e2e/slash-command-menu.spec.ts | 139 +++++++++++++------- apps/desktop/src/preload/preload.ts | 33 ++++- 2 files changed, 119 insertions(+), 53 deletions(-) diff --git a/apps/desktop/e2e/slash-command-menu.spec.ts b/apps/desktop/e2e/slash-command-menu.spec.ts index e66adc28b5..61f7489c13 100644 --- a/apps/desktop/e2e/slash-command-menu.spec.ts +++ b/apps/desktop/e2e/slash-command-menu.spec.ts @@ -170,70 +170,109 @@ test('an open menu keeps its container and skills group across projection refres await composer.fill('seed session'); await composer.press('Enter'); await expect(page.getByText('Fake backend received: seed session')).toBeVisible(); + await expect(page.getByRole('button', { name: '停止' })).toHaveCount(0); + + // The completed turn publishes its own projection refresh. Wait on the + // composer's public loading state so that work cannot spill into the window + // this test is about. + await page.getByRole('button', { name: '添加上下文' }).click(); + const contextMenu = page.getByRole('menu', { name: '添加上下文' }); + await expect(contextMenu.getByRole('menuitem', { name: /选择技能/ })).not.toHaveAttribute( + 'aria-busy', + 'true', + ); + await page.keyboard.press('Escape'); + await expect(contextMenu).toHaveCount(0); await composer.click(); await composer.pressSequentially('/'); const menu = page.getByRole('listbox', { name: '命令和技能' }); await expect(menu.getByRole('group', { name: 'Skills' })).toBeVisible(); - // Armed before the refresh: the flicker was the skills group (and with it - // the listbox geometry) being torn down and re-created when the projection - // cleared and repopulated, so any removal during the refresh is the - // regression (#2667). - await page.evaluate(() => { - const state = { removals: 0 }; - (globalThis as unknown as { __slashMenuWatch?: unknown }).__slashMenuWatch = state; - const observer = new MutationObserver((mutations) => { - for (const mutation of mutations) { - for (const node of mutation.removedNodes) { - if (!(node instanceof HTMLElement)) continue; - if ( - node.matches('[role="listbox"], [role="group"]') || - node.querySelector('[role="listbox"], [role="group"]') !== null - ) { - state.removals += 1; - } - } - } - }); - observer.observe(document.body, { childList: true, subtree: true }); - }); - // A thinking-level change publishes the session's 'updated' event and // reloads the Skill projection without changing what the menu shows: the // exact same-content refresh that used to alternate the popup (#2667). - const sessionId = await page.evaluate(async () => { + const observation = await menu.evaluate(async (menuElement) => { const sessions = await ( window as unknown as { maka: { sessions: { list(): Promise> } }; } ).maka.sessions.list(); - return sessions[0]?.id; + const sessionId = sessions[0]?.id; + if (!sessionId) throw new Error('Session missing before projection refresh'); + + // Arm immediately before the refreshes and only on this popover. The + // document body also contains unrelated overlays whose teardown says + // nothing about this menu's identity. + const menuContainer = menuElement.parentElement; + const state = { menuRemovals: 0, skillsGroupRemovals: 0 }; + const skillsGroup = menuElement.querySelector('[role="group"][aria-label="Skills"]'); + if (!menuContainer) throw new Error('Slash menu container missing before projection refresh'); + if (!skillsGroup) throw new Error('Skills group missing before projection refresh'); + const recordRemoval = ( + mutations: MutationRecord[], + watchedNode: Node, + key: 'menuRemovals' | 'skillsGroupRemovals', + ) => { + for (const mutation of mutations) { + for (const node of mutation.removedNodes) { + if (node === watchedNode) state[key] += 1; + } + } + }; + const menuObserver = new MutationObserver((mutations) => { + recordRemoval(mutations, menuElement, 'menuRemovals'); + }); + const skillsGroupObserver = new MutationObserver((mutations) => { + recordRemoval(mutations, skillsGroup, 'skillsGroupRemovals'); + }); + menuObserver.observe(menuContainer, { childList: true }); + skillsGroupObserver.observe(menuElement, { childList: true }); + const maka = ( + window as unknown as { + maka: { + sessions: { + setThinkingLevel(id: string, level?: null): Promise; + }; + }; + } + ).maka; + const e2eControls = ( + window as unknown as { + makaE2eLatch?: { + waitForInvocableSkillsCall(sessionId: string): Promise; + }; + } + ).makaE2eLatch; + if (!e2eControls) throw new Error('E2E bridge controls missing before projection refresh'); + try { + for (let round = 0; round < 3; round += 1) { + const projectionSettled = e2eControls.waitForInvocableSkillsCall(sessionId); + await maka.sessions.setThinkingLevel(sessionId, null); + await projectionSettled; + await new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); + } + } finally { + // Drain the final queued batch before closing the exact refresh window; + // polling a monotonic counter cannot turn a failure into success. + recordRemoval(menuObserver.takeRecords(), menuElement, 'menuRemovals'); + recordRemoval(skillsGroupObserver.takeRecords(), skillsGroup, 'skillsGroupRemovals'); + menuObserver.disconnect(); + skillsGroupObserver.disconnect(); + } + return { + ...state, + menuConnected: menuElement.isConnected, + skillsGroupConnected: skillsGroup.isConnected, + }; + }); + expect(observation).toEqual({ + menuRemovals: 0, + skillsGroupRemovals: 0, + menuConnected: true, + skillsGroupConnected: true, }); - for (let round = 0; round < 3; round += 1) { - await page.evaluate( - (id) => - ( - window as unknown as { - maka: { sessions: { setThinkingLevel(id: string, level?: null): Promise } }; - } - ).maka.sessions.setThinkingLevel(id!, null), - sessionId, - ); - } - // The refresh round trip is IPC-fast; the poll below gives it room while - // asserting the menu never lost its skills group. - await expect(menu.getByRole('group', { name: 'Skills' })).toBeVisible(); - await expect - .poll( - () => - page.evaluate( - () => - (globalThis as unknown as { __slashMenuWatch: { removals: number } }).__slashMenuWatch - .removals, - ), - { timeout: 3_000 }, - ) - .toBe(0); await expect(menu.getByRole('group', { name: 'Skills' })).toBeVisible(); }); diff --git a/apps/desktop/src/preload/preload.ts b/apps/desktop/src/preload/preload.ts index 5382ec5843..3a090779f1 100644 --- a/apps/desktop/src/preload/preload.ts +++ b/apps/desktop/src/preload/preload.ts @@ -3144,18 +3144,20 @@ const makaBridge = { }, } satisfies MakaBridge; -// E2E-only async latches. Real users never get these: the preload mirrors the +// E2E-only async controls. Real users never get these: the preload mirrors the // main process's isolated-E2E gate (startup-context.ts) — MAKA_E2E alone is // not enough without the throwaway profile dir. An armed latch holds the next // bridge call or an explicitly gated renderer boundary until the test releases -// it, so Playwright gets a deterministic in-flight window instead of racing -// near-instant work. The wrappers must be installed BEFORE +// it, while a settled-call waiter exposes a deterministic completion boundary +// for work whose visible result may intentionally keep the same DOM identity. +// The wrappers must be installed BEFORE // exposeInMainWorld: the bridge is cloned into the main world at expose time, // and the exposed clone is sealed against later patching. if (process.env.MAKA_E2E === '1' && process.env.MAKA_E2E_USER_DATA_DIR) { type LatchKey = 'newTasks.listInvocableSkills' | 'sessions.list' | 'settings.chunk'; const gates = new Map; oneShot: boolean }>(); const releases = new Map void; reject: (error: Error) => void }>(); + const invocableSkillsWaiters = new Map void>>(); const waitForLatch = async (key: LatchKey): Promise => { const gate = gates.get(key); if (!gate) return; @@ -3177,6 +3179,24 @@ if (process.env.MAKA_E2E === '1' && process.env.MAKA_E2E_USER_DATA_DIR) { makaBridge.sessions.list.bind(makaBridge.sessions), 'sessions.list', ); + const listInvocableSkills = makaBridge.skills.listInvocable.bind(makaBridge.skills); + makaBridge.skills.listInvocable = async (...args) => { + try { + return await listInvocableSkills(...args); + } finally { + const sessionId = args[0]; + if (sessionId) { + const waiters = invocableSkillsWaiters.get(sessionId); + const resolve = waiters?.shift(); + if (waiters?.length === 0) invocableSkillsWaiters.delete(sessionId); + if (resolve) { + // Let consumers of the bridge promise run their state updates before + // the test continues from the observed completion. + setTimeout(resolve, 0); + } + } + } + }; contextBridge.exposeInMainWorld('makaE2eLatch', { arm(key: LatchKey, options?: { oneShot?: boolean }) { let resolve: () => void = () => {}; @@ -3191,6 +3211,13 @@ if (process.env.MAKA_E2E === '1' && process.env.MAKA_E2E_USER_DATA_DIR) { wait(key: 'settings.chunk') { return waitForLatch(key); }, + waitForInvocableSkillsCall(sessionId: string) { + return new Promise((resolve) => { + const waiters = invocableSkillsWaiters.get(sessionId) ?? []; + waiters.push(resolve); + invocableSkillsWaiters.set(sessionId, waiters); + }); + }, release(key: LatchKey) { releases.get(key)?.resolve(); releases.delete(key);