From ee926225b7e4138cd4f39e4b3783936c5acefc73 Mon Sep 17 00:00:00 2001 From: Bo Liu Date: Mon, 24 Aug 2026 17:55:16 +0800 Subject: [PATCH] fix(desktop): keep the slash picker stable across same-context refreshes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An open `/` menu alternated between its commands-only and commands-plus-skills geometries on every session or MCP event (#2667): each refresh cleared the invocable-Skill catalog fail-closed, so the popup lost and regained its Skills group for the length of one IPC round trip. Fail closed only when the context key actually changes. A same-context refresh keeps the Skills already on screen, and a settled refresh that returned an identical list keeps the previous array identity, so the composer's trigger memo and menu-replay effect stay quiet. A Skill withdrawn inside that stale window still fails safely, because selection resolves through the Runtime resolver that no longer knows it. The ＋ menu's separate fail-closed path is unchanged: a Plan toggle moves the context key, so it still clears and still reads `settled`. --- apps/desktop/e2e/slash-command-menu.spec.ts | 75 +++++++++++++++++++ .../src/renderer/use-composer-mentions.ts | 57 +++++++++++--- 2 files changed, 121 insertions(+), 11 deletions(-) diff --git a/apps/desktop/e2e/slash-command-menu.spec.ts b/apps/desktop/e2e/slash-command-menu.spec.ts index 49c705992c..e66adc28b5 100644 --- a/apps/desktop/e2e/slash-command-menu.spec.ts +++ b/apps/desktop/e2e/slash-command-menu.spec.ts @@ -162,3 +162,78 @@ test('dispatches /side instead of steering it into a running turn', async ({ await expect(page.locator('.maka-quote-workbar-panel')).toHaveCount(1); await page.getByRole('button', { name: '停止' }).click(); }); + +test('an open menu keeps its container and skills group across projection refreshes', async ({ + invocableSkillsWindow: page, +}) => { + const composer = page.locator(COMPOSER_INPUT); + await composer.fill('seed session'); + await composer.press('Enter'); + await expect(page.getByText('Fake backend received: seed session')).toBeVisible(); + + 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 sessions = await ( + window as unknown as { + maka: { sessions: { list(): Promise> } }; + } + ).maka.sessions.list(); + return sessions[0]?.id; + }); + 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/renderer/use-composer-mentions.ts b/apps/desktop/src/renderer/use-composer-mentions.ts index a4061b491d..2d31f9505c 100644 --- a/apps/desktop/src/renderer/use-composer-mentions.ts +++ b/apps/desktop/src/renderer/use-composer-mentions.ts @@ -26,6 +26,27 @@ import type { DesktopNewTaskTarget } from '../preload/bridge-contract.js'; /** One frozen identity, so a context-mismatch render does not churn props. */ const EMPTY_SKILLS: InvocableSkillEntry[] = []; +/** + * Whether a reloaded projection describes the same Skills as the one on + * screen, so an unchanged refresh can keep the array it already published. + */ +function invocableSkillListsEqual( + current: readonly InvocableSkillEntry[], + next: readonly InvocableSkillEntry[], +): boolean { + if (current.length !== next.length) return false; + return current.every((skill, index) => { + const other = next[index]; + return ( + other !== undefined && + skill.ref === other.ref && + skill.id === other.id && + skill.name === other.name && + skill.description === other.description + ); + }); +} + /** * Owns the composer mention popup wiring so app-shell.tsx keeps no inline * `window.maka` state (app-shell-composer-attachment-owner-contract). Derives @@ -105,14 +126,21 @@ export function useComposerMentions(options: { let requestVersion = 0; const refresh = () => { const version = ++requestVersion; - setCatalog((previous) => ({ - contextKey, - loading: true, - // A same-context refresh keeps its settled verdict; a context switch - // has nothing settled to hold. - settled: previous.contextKey === contextKey ? previous.settled : undefined, - skills: [], - })); + setCatalog((previous) => + previous.contextKey === contextKey + ? // A same-context refresh keeps both its settled verdict and the + // Skills already on screen. Clearing here is what made an open `/` + // menu alternate between its commands-only and commands-plus-skills + // geometries on every session or MCP event (#2667). The backend + // surface has not changed, so there is nothing to fail closed + // against; and a Skill withdrawn inside the one-IPC-round-trip + // stale window still fails safely, because selection resolves + // through the Runtime resolver that no longer knows it. + { ...previous, loading: true } + : // A context switch has nothing settled to hold, and its Skills + // belong to the surface being left behind. + { contextKey, loading: true, settled: undefined, skills: [] }, + ); const context = { ...(newSessionModel ?? {}), collaborationMode: newSessionCollaborationMode ?? 'agent', @@ -128,12 +156,19 @@ export function useComposerMentions(options: { void request.then( (next) => { if (cancelled || version !== requestVersion) return; - setCatalog({ + setCatalog((previous) => ({ contextKey, loading: false, settled: next.length === 0 ? 'empty' : 'populated', - skills: next, - }); + // A refresh that changed nothing keeps the previous array + // identity, so the composer's trigger memo and the menu-replay + // effect stay quiet instead of remounting the popup. + skills: + previous.contextKey === contextKey && + invocableSkillListsEqual(previous.skills, next) + ? previous.skills + : [...next], + })); }, () => { // Fail soft: an unavailable projection leaves `/` with no suggestions.