From d6266e12496825371c42a32c7a84d85264c391d0 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 22:02:42 +0800 Subject: [PATCH 01/12] docs(desktop): correct the play-function convention in FIDELITY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The render smoke waits on Storybook's `storyFinished` channel event, so every `play` function executes and a failed assertion inside one fails the lane. FIDELITY said the opposite — that the smoke mounts with autoplay disabled — which is how #4766's assertion-bearing stories came to be read as review-only scaffolding. Say what the lane actually does, and keep the two limits that are real: geometry and theme matrices still belong elsewhere because CI mounts once at 1280 in light, and a probe that must be installed before mount belongs in a test that owns the global, not in a story that can only observe after it renders. Generated-by: Claude Code --- apps/desktop/stories/FIDELITY.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/desktop/stories/FIDELITY.md b/apps/desktop/stories/FIDELITY.md index 5890b82af5..17bc65f42b 100644 --- a/apps/desktop/stories/FIDELITY.md +++ b/apps/desktop/stories/FIDELITY.md @@ -45,7 +45,7 @@ A story earns its place by rendering pixels no other story renders. A second lev Two facts decide it, and both were guessed wrong once: - **Where a story renders.** CI mounts every story exactly once at 1280 wide in light. It does not maintain a viewport, theme or screenshot matrix. Responsive and theme behaviour belongs in a focused component contract or the real desktop E2E harness. -- **Whether `play` reaches the state.** Local Storybook can use `play` to drive a story into the state a reviewer needs to see. CI deliberately mounts stories with `embed=true`, so it does not execute those interactions or treat them as product tests. +- **Whether `play` reaches the state.** `play` drives a story into the state a reviewer needs to see, and CI runs it — so the state it lands on is the state the smoke reads, and a story that only differs by a `play` step is a second state, not a variant. Extra stories still cost: a reviewer scanning the sidebar cannot tell which entry is the page, and duplicates re-render the same pixels every run while claiming coverage they do not add. Where a state matters but renders nothing new, pin it somewhere that runs — a `packages/ui` test or an e2e journey. @@ -61,11 +61,15 @@ When a component has two hosts, one frame is not both. `capability-audit-strip.s If the runtime computes a field, ask the runtime for it. A story that hardcodes what a classifier would have returned is asserting a fact rather than showing one, and nothing fails when the classifier moves. -## A `play` function is a local review driver, not a CI test +## A `play` function runs in CI, and its assertions are real -The render smoke uses Storybook's embedded mode, which mounts the story but disables autoplay. That keeps the Storybook lane responsible for one thing: every production-backed story must render without runtime, console or page errors. It does not turn keyboard, pointer, focus, geometry or state-transition interactions into a second desktop E2E suite. +The render smoke waits for Storybook's `storyFinished` event before it reads the accessibility tree, so every `play` function executes and a failed assertion inside one fails the lane. This paragraph used to say the opposite — that the smoke mounts with autoplay disabled — and it was wrong: nothing passes `embed`, and #4766 landed 18 stories whose assertions are the coverage. -Put behavioural and computed-style contracts where they can name what they check — a `packages/ui` test or a real Electron E2E journey. Use `play` only when a local reviewer needs help navigating to a visual state; do not put product assertions in it. +That makes `play` the right home for a behavioural contract whose subject is the browser: a live Selection, a caret between text nodes, an undo transaction, a portal's identity across a re-render. None of those exist in a `packages/ui` DOM shim, and none of them need Electron. + +It is still not a place for geometry or theme matrices. CI mounts every story once, at 1280 wide, in light; a contract that depends on any other viewport or scheme belongs in a `packages/ui` test or the desktop E2E harness. And a rule that is pure state — which commands a Session offers, what a query parses to — belongs in a unit test, where it costs milliseconds instead of a browser. + +Write the assertion so it can only pass for the reason it names. A story that mounts the surface and then observes it cannot see anything that happened during the mount, so a probe that must be installed first (a constructor count, an event before the first paint) belongs in a test that owns the global. ## A story that renders nothing is not a story From 442b25cbd6eb9a8af7266ba154da3625f09f43d3 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 22:02:52 +0800 Subject: [PATCH 02/12] test(ui): pin the prompt rail observer against streaming deltas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transcript-wide IntersectionObserver must be built once, not once per delta. Two mechanisms hold that in series — ChatView memoizes the rail's turn list on persisted text, and PromptAnchorRail stabilizes `orderedTurnIdsRef` on the turn ids — and neither is observable from a story: the probe has to be installed before the rail's own observer, and a story can only observe after it has mounted. So this is a component test that owns the global, counting constructions across ten re-renders that grow only the streaming tail. Collapse the two spellings of the reading band onto `READING_BAND_TOP_PERCENT`, so asserting the `rootMargin` string is a contract about the band rather than a copy of the literal next to it. Generated-by: Claude Code --- .../prompt-rail-observer-identity.test.tsx | 241 ++++++++++++++++++ packages/ui/src/prompt-anchor-rail.tsx | 11 +- 2 files changed, 250 insertions(+), 2 deletions(-) create mode 100644 packages/ui/src/__tests__/prompt-rail-observer-identity.test.tsx diff --git a/packages/ui/src/__tests__/prompt-rail-observer-identity.test.tsx b/packages/ui/src/__tests__/prompt-rail-observer-identity.test.tsx new file mode 100644 index 0000000000..53a8f88d22 --- /dev/null +++ b/packages/ui/src/__tests__/prompt-rail-observer-identity.test.tsx @@ -0,0 +1,241 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * The prompt rail observes every mounted Turn in the transcript. What that + * observer is for is Turn identity and order, and a streaming answer delivers + * several deltas a second that change neither — so a delta must not tear the + * observer down and rebuild it over the whole conversation. + * + * Two things hold that in series: ChatView hands the rail the previous entry + * array back when no persisted prompt or answer text moved, and the rail keys + * the observer's lifetime on the Turn id list rather than on its props. Either + * one alone keeps the count at 1, which is why this asserts the composed + * outcome the way the deleted E2E case did rather than probing one of them. + * + * A probe, not a story: this counts constructions and reads the init a + * constructor was handed, and both are only observable from before the rail's + * own observer exists. Installing `IntersectionObserver` on the global here is + * what makes the positive control real — a story mounting the rail first can + * only watch the observer it already has, so the `rootMargin` assertion below + * would pass against any literal. + */ + +import assert from 'node:assert/strict'; +import { afterEach, test } from 'node:test'; +import { act, createElement, type ReactElement } from 'react'; +import { createRoot } from 'react-dom/client'; +import { parseHTML } from 'linkedom'; +import type { SessionSummary, StoredMessage } from '@maka/core/session'; +import { AstryxLocaleProvider } from '../astryx-i18n.js'; +import { ChatSurfaceLayout } from '../chat-surface-layout.js'; +import { ChatView } from '../chat-view.js'; +import { LocaleProvider } from '../locale-context.js'; +import { READING_BAND_TOP_PERCENT } from '../prompt-anchor-rail.js'; + +const originalGlobals = { + CSS: globalThis.CSS, + document: globalThis.document, + Element: globalThis.Element, + HTMLElement: globalThis.HTMLElement, + IntersectionObserver: globalThis.IntersectionObserver, + MutationObserver: globalThis.MutationObserver, + Node: globalThis.Node, + ResizeObserver: globalThis.ResizeObserver, + matchMedia: globalThis.matchMedia, + requestAnimationFrame: globalThis.requestAnimationFrame, + cancelAnimationFrame: globalThis.cancelAnimationFrame, + window: globalThis.window, +}; +const originalActEnvironment = (globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; +}).IS_REACT_ACT_ENVIRONMENT; + +let mountedRoot: ReturnType | undefined; + +afterEach(async () => { + if (mountedRoot) await act(() => mountedRoot?.unmount()); + mountedRoot = undefined; + Object.assign(globalThis, { + ...originalGlobals, + IS_REACT_ACT_ENVIRONMENT: originalActEnvironment, + }); +}); + +const TURN_COUNT = 6; + +const activeSession: SessionSummary = { + id: 'session-rail', + name: '提问导航', + isFlagged: false, + isArchived: false, + labels: [], + hasUnread: false, + status: 'active', + lastMessageAt: 0, + backend: 'ai-sdk', + llmConnectionId: 'connection-anthropic', + llmConnectionSlug: 'anthropic', + connectionLocked: false, + model: 'claude-sonnet-4-5', + permissionMode: 'ask', +}; + +function turnMessages(answerText: (index: number) => string): StoredMessage[] { + return Array.from({ length: TURN_COUNT }, (_, index): StoredMessage[] => [ + { + type: 'user', + id: `user-${index}`, + turnId: `turn-${index}`, + ts: index * 2, + text: `第 ${index} 个问题`, + }, + { + type: 'assistant', + id: `assistant-${index}`, + turnId: `turn-${index}`, + ts: index * 2 + 1, + text: answerText(index), + modelId: 'claude-sonnet-4-5', + }, + ]).flat(); +} + +interface ObservedInit { + root: unknown; + rootMargin?: string; + threshold?: number | number[]; +} + +function harness() { + const { document, window } = parseHTML('
'); + const inits: ObservedInit[] = []; + class CountingIntersectionObserver { + constructor(_callback: IntersectionObserverCallback, init?: IntersectionObserverInit) { + inits.push({ + root: init?.root, + rootMargin: init?.rootMargin, + threshold: init?.threshold as number | number[] | undefined, + }); + } + observe(): void {} + unobserve(): void {} + disconnect(): void {} + takeRecords(): IntersectionObserverEntry[] { + return []; + } + } + class InertResizeObserver { + observe(): void {} + unobserve(): void {} + disconnect(): void {} + } + // linkedom lays nothing out, so every box is zero-sized. The rail reads + // geometry only to pick which tick is current; the observer it builds to do + // that is what this test is about, and a constructor call does not need a + // layout to be counted. + const rect = { + bottom: 600, height: 600, left: 0, right: 800, top: 0, width: 800, x: 0, y: 0, + toJSON: () => ({}), + } satisfies DOMRect; + window.Element.prototype.getBoundingClientRect = () => rect; + Object.assign(globalThis, { + CSS: { supports: () => false }, + document, + Element: window.Element, + HTMLElement: window.HTMLElement, + IntersectionObserver: CountingIntersectionObserver, + MutationObserver: window.MutationObserver, + Node: window.Node, + ResizeObserver: InertResizeObserver, + matchMedia: () => ({ + matches: false, + addEventListener() {}, + removeEventListener() {}, + }), + requestAnimationFrame: (callback: FrameRequestCallback) => { + callback(0); + return 0; + }, + cancelAnimationFrame: () => {}, + window, + IS_REACT_ACT_ENVIRONMENT: true, + }); + const mount = document.querySelector('#mount'); + assert.ok(mount); + return { inits, mount }; +} + +function view(messages: StoredMessage[]): ReactElement { + const chat = createElement(ChatView, { messages, activeSession, onNew: () => {} } as never); + const layout = createElement(ChatSurfaceLayout, { + scrollOwner: 'host', + composer: null, + children: chat, + }); + const astryx = createElement(AstryxLocaleProvider, { children: layout }); + return createElement(LocaleProvider, { locale: 'zh-CN', children: astryx }); +} + +test('streaming deltas do not reconstruct the prompt rail observer', async () => { + const { inits, mount } = harness(); + const root = createRoot(mount); + mountedRoot = root; + + await act(() => { + root.render(view(turnMessages(() => '答案'))); + }); + assert.equal(inits.length, 1, 'the rail observes the transcript once on mount'); + + // Ten deltas on the tail answer. Every one of them hands ChatView a fresh + // message array and fresh turn records — which is exactly the shape that + // used to rebuild the observer over the whole transcript per frame. + for (let delta = 1; delta <= 10; delta += 1) { + await act(() => { + root.render( + view( + turnMessages((index) => + index === TURN_COUNT - 1 ? `答案${'。'.repeat(delta)}` : '答案', + ), + ), + ); + }); + } + assert.equal(inits.length, 1, `the observer was rebuilt ${inits.length - 1} times`); +}); + +test('the rail observes its reading band, not the whole scrollport', async () => { + const { inits, mount } = harness(); + const root = createRoot(mount); + mountedRoot = root; + await act(() => { + root.render(view(turnMessages(() => '答案'))); + }); + + const init = inits[0]; + assert.ok(init); + // The band is the top slice of the scrollport, so the bottom inset is its + // complement. Both spellings come from one constant; a rail that observed + // the whole scrollport would call every Turn on screen "being read". + assert.equal(init.rootMargin, `0px 0px -${100 - READING_BAND_TOP_PERCENT}% 0px`); + assert.ok(READING_BAND_TOP_PERCENT > 0 && READING_BAND_TOP_PERCENT < 100); + // Zero alone reports a boundary touch as an intersection; the second, + // positive threshold is what distinguishes real overlap from that. + assert.deepEqual(init.threshold, [0, 0.000_001]); +}); diff --git a/packages/ui/src/prompt-anchor-rail.tsx b/packages/ui/src/prompt-anchor-rail.tsx index a78de69d54..29e80605cc 100644 --- a/packages/ui/src/prompt-anchor-rail.tsx +++ b/packages/ui/src/prompt-anchor-rail.tsx @@ -47,6 +47,13 @@ const PREVIEW_DELAY_MS = 120; const MAX_PROMPT_RAIL_TICKS = 64; /** Distinguish a positive IO overlap from Chromium's zero-area edge contact. */ const POSITIVE_INTERSECTION_RATIO = 0.000_001; +/** + * The top slice of the scrollport a reader is taken to be reading. Whole + * percent, because the observer spells it as a `rootMargin` string and the + * geometry seed spells it as a fraction — one number, two spellings, and a + * decimal fraction would not survive the round trip exactly. + */ +export const READING_BAND_TOP_PERCENT = 34; /** Quiet frames at the destination that end a jump's hold. */ const JUMP_SETTLE_QUIET_FRAMES = 3; @@ -528,7 +535,7 @@ export const PromptAnchorRail = memo(function PromptAnchorRail({ turns, scrollRe readingBandTurnIds.clear(); for (const turnId of turnIdsIntersecting( rootBounds.top, - rootBounds.top + rootBounds.height * 0.34, + rootBounds.top + rootBounds.height * (READING_BAND_TOP_PERCENT / 100), )) { readingBandTurnIds.add(turnId); } @@ -565,7 +572,7 @@ export const PromptAnchorRail = memo(function PromptAnchorRail({ turns, scrollRe resolveActive(); }, { root, - rootMargin: '0px 0px -66% 0px', + rootMargin: `0px 0px -${100 - READING_BAND_TOP_PERCENT}% 0px`, // The positive threshold delivers a callback when an overlap becomes // a zero-area boundary touch, which the strict geometry rule excludes. threshold: [0, POSITIVE_INTERSECTION_RATIO], From fe58d4b5bbdbac4bf1374c3ff0097dbd1f2f4631 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 22:03:03 +0800 Subject: [PATCH 03/12] test(desktop): move the slash-command menu off Electron MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2948 and #3289 were the only two flake reports before #4523, and both were this file. Nothing it asserts needs a layout engine. Each of the six cases lands at the tier that owns what it guards: - Which commands a state offers is pure, and `desktop-slash-command.test.ts` already covers it; only the rendering — two groups, 命令 then Skills, each row carrying its `/id` token — needs a browser. - Whether a `/` addresses a command is `slashCommandQuery`, pure, so it becomes a unit test alongside `skillMentionQuery`. - Path separators (#3849) and block-break line boundaries live in the patched Astryx `useTriggerMenu` and need a live Selection across text nodes; the same-content projection refresh (#2667) needs a portal whose identity survives a re-render. Those become stories, mounting the real Composer inside the real ComposerMentionsProvider over a scoped bridge so `invocableSkillListsEqual` is genuinely exercised. `PathSeparatorsAreNotTriggers` opens the menu first, from the same editor with the same synthetic input, so a menu that stopped opening at all cannot pass it. The command list is assembled in the story from the same three authorities app-shell reads, rather than extracted into `desktop-slash-command.ts`: the architecture ratchet counts dependencies per file, so moving the catalog import into the smaller module books new debt there instead of retiring any — the reason #4762 left the catalog query where it was. Generated-by: Claude Code --- apps/desktop/e2e/slash-command-menu.spec.ts | 322 --------------- .../stories/composer-slash-menu.stories.tsx | 380 ++++++++++++++++++ .../src/__tests__/chat-input-behavior.test.ts | 34 ++ 3 files changed, 414 insertions(+), 322 deletions(-) delete mode 100644 apps/desktop/e2e/slash-command-menu.spec.ts create mode 100644 apps/desktop/stories/composer-slash-menu.stories.tsx diff --git a/apps/desktop/e2e/slash-command-menu.spec.ts b/apps/desktop/e2e/slash-command-menu.spec.ts deleted file mode 100644 index 4a1c4a026b..0000000000 --- a/apps/desktop/e2e/slash-command-menu.spec.ts +++ /dev/null @@ -1,322 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { expect, test, COMPOSER_INPUT } from './fixtures'; - -test('shows only slash commands executable in the current session state', async ({ - invocableSkillsWindow: page, -}) => { - const composer = page.locator(COMPOSER_INPUT); - await composer.click(); - await composer.pressSequentially('/'); - - const freshMenu = page.getByRole('listbox', { name: '命令和技能' }); - const freshCommands = freshMenu.getByRole('group', { name: '命令' }); - await expect(freshCommands.getByRole('option')).toHaveCount(2); - await expect(freshCommands.getByRole('option', { name: /使用 Graph.*\/graph/ })).toBeVisible(); - await expect(freshCommands.getByRole('option', { name: /使用 Swarm.*\/swarm/ })).toBeVisible(); - - await composer.press('Escape'); - 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: '命令和技能' }); - const groups = menu.getByRole('group'); - await expect(groups).toHaveCount(2); - await expect(groups.nth(0)).toHaveAttribute('aria-label', '命令'); - await expect(groups.nth(1)).toHaveAttribute('aria-label', 'Skills'); - - await expect(groups.nth(0).getByRole('option')).toHaveCount(4); -}); - -test('compacts the active session', 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(); - - const sessionId = await page.evaluate(async () => (await window.maka.sessions.list())[0]?.id); - expect(sessionId).toBeTruthy(); - await page.evaluate((activeSessionId) => { - const testWindow = window as typeof window & { - __makaObservedCompactCompletion?: boolean; - }; - testWindow.__makaObservedCompactCompletion = false; - window.maka.sessions.subscribeChanges((event) => { - if (event.reason === 'status-change' && event.sessionId === activeSessionId) { - testWindow.__makaObservedCompactCompletion = true; - } - }); - }, sessionId!); - - await composer.click(); - await composer.pressSequentially('/'); - - const menu = page.getByRole('listbox', { name: '命令和技能' }); - const compact = menu.getByRole('group', { name: '命令' }).getByRole('option', { - name: /压缩上下文.*\/compact/, - }); - await compact.click(); - - await expect.poll(() => composer.textContent()).toBe('/compact '); - await expect(menu).not.toBeVisible(); - await composer.press('Enter'); - - await expect - .poll(() => - page.evaluate( - () => - (window as typeof window & { __makaObservedCompactCompletion?: boolean }) - .__makaObservedCompactCompletion, - ), - ) - .toBe(true); - await expect.poll(() => composer.textContent()).toBe(''); - await expect(page.getByText('压缩失败')).toHaveCount(0); - - // After the compact completes the composer clears and can remount. `fill()` - // can land before the contentEditable is focused again, so the draft never - // populates and Enter submits nothing — the flake in issue #3289. Type - // through the focused element and require the draft to have settled before - // dispatching, mirroring the running-turn spec below. - await composer.click(); - await composer.pressSequentially('after compact'); - await expect.poll(() => composer.textContent()).toBe('after compact'); - await composer.press('Enter'); - await expect(page.getByText('Fake backend received: after compact')).toBeVisible(); - await expect(page.getByText('Fake backend received: /compact')).toHaveCount(0); -}); - -test('offers commands only for the first token and keeps explicit Skill queries separate', 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.fill('explain /'); - const inlineMenu = page.getByRole('listbox', { name: '命令和技能' }); - await expect(inlineMenu.getByRole('group', { name: '命令' })).toHaveCount(0); - await expect(inlineMenu.getByRole('group', { name: 'Skills' })).toBeVisible(); - - await composer.fill('/skill:compact'); - await expect(inlineMenu.getByRole('option', { name: /\/compact/ })).toHaveCount(0); - - await composer.fill('/side'); - // macOS maps Home to document scrolling rather than line-start movement in - // contentEditable. Put the caret at the same semantic position on every OS - // before checking that a slash with text after the caret is not a command. - await composer.press(process.platform === 'darwin' ? 'Meta+ArrowLeft' : 'Home'); - await composer.press('ArrowRight'); - await expect(inlineMenu.getByRole('group', { name: '命令' })).toHaveCount(0); -}); - -test('does not open the slash menu for path separators', async ({ - invocableSkillsWindow: page, -}) => { - const composer = page.locator(COMPOSER_INPUT); - const menu = page.getByRole('listbox', { name: '命令和技能' }); - - for (const prefix of ['帮我整理到/Users', 'path/to/file']) { - await composer.fill(prefix); - await composer.evaluate((editable) => { - // Chromium can put the next typed character in a new text node. Recreate - // that observed input shape without letting Playwright normalize the DOM. - const node = editable.appendChild(document.createTextNode('/')); - const range = document.createRange(); - range.setStart(node, 1); - range.collapse(true); - const selection = window.getSelection(); - selection?.removeAllRanges(); - selection?.addRange(range); - editable.dispatchEvent(new InputEvent('input', { - bubbles: true, - data: '/', - inputType: 'insertText', - })); - }); - - await expect(menu).toHaveCount(0); - await expect.poll(() => composer.textContent()).toBe(`${prefix}/`); - await expect - .poll(() => - composer.evaluate((editable) => { - const selection = window.getSelection(); - if (!selection?.focusNode || !editable.contains(selection.focusNode)) return -1; - const range = document.createRange(); - range.selectNodeContents(editable); - range.setEnd(selection.focusNode, selection.focusOffset); - return range.toString().length; - }), - ) - .toBe(`${prefix}/`.length); - } -}); - -test('opens the slash menu after a DOM block break', async ({ - invocableSkillsWindow: page, -}) => { - const composer = page.locator(COMPOSER_INPUT); - const menu = page.getByRole('listbox', { name: '命令和技能' }); - - await composer.fill('first line'); - await composer.evaluate((editable) => { - const block = document.createElement('div'); - const slash = block.appendChild(document.createTextNode('/')); - editable.appendChild(block); - - const range = document.createRange(); - range.setStart(slash, 1); - range.collapse(true); - const selection = window.getSelection(); - selection?.removeAllRanges(); - selection?.addRange(range); - editable.dispatchEvent(new InputEvent('input', { - bubbles: true, - data: '/', - inputType: 'insertText', - })); - }); - - await expect.poll(() => composer.evaluate((editable) => editable.innerText)).toBe( - 'first line\n/', - ); - await expect(menu).toBeVisible(); -}); - -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 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(); - - // 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 observation = await menu.evaluate(async (menuElement) => { - const sessions = await ( - window as unknown as { - maka: { sessions: { list(): Promise> } }; - } - ).maka.sessions.list(); - 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, - }); - await expect(menu.getByRole('group', { name: 'Skills' })).toBeVisible(); -}); diff --git a/apps/desktop/stories/composer-slash-menu.stories.tsx b/apps/desktop/stories/composer-slash-menu.stories.tsx new file mode 100644 index 0000000000..0f178f443c --- /dev/null +++ b/apps/desktop/stories/composer-slash-menu.stories.tsx @@ -0,0 +1,380 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * The composer's `/` menu, on the real Composer with the real Desktop command + * derivation behind it. + * + * Fidelity convention (#1433): app-shell.tsx renders this Composer with + * `slashCommands` derived from the catalog and `mentionSkills` from Runtime's + * invocable projection through `ComposerMentionsProvider`. The projection is + * the real one here, over a bridge that answers the way IPC does; the command + * list is assembled from the same authorities (see below). See FIDELITY.md. + * + * A browser, not a DOM shim: what these assert is where the caret is, which + * text node it sits in, and whether Astryx's patched `useTriggerMenu` reads a + * boundary in front of it (`patches/@astryxdesign+core+0.5.2.patch`). None of + * that exists without a real Selection. + */ + +import { useMemo } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, userEvent, waitFor, within } from 'storybook/test'; +import { slashCommandsForSurface } from '@maka/core/slash-command-catalog'; +import { Composer } from '@maka/ui'; +import { + ComposerMentionsProvider, + useComposerMentionsContext, +} from '../src/renderer/composer-mentions'; +import { desktopSlashCommandAvailability } from '../src/renderer/desktop-slash-command'; +import { getShellCopy } from '../src/renderer/locales/shell-copy'; +import { withScopedMakaBridge } from './maka-bridge'; + +const COMPOSER_INPUT = '.maka-composer-editor [contenteditable="true"]'; +const MENU_LABEL = '命令和技能'; +const SESSION_ID = 'session-slash-menu'; + +/** + * What app-shell.tsx builds for `slashCommands`, from the same three + * authorities: the catalog, the availability predicate, and the shell's copy. + * The keywords and the icon it also attaches are pure presentation and nothing + * below reads them. + * + * Assembled here rather than lifted into `desktop-slash-command.ts`, for the + * reason #4762 gave for leaving the catalog query in app-shell: the debt + * ratchet counts dependencies per file, so moving the catalog import into the + * smaller module books a new dependency there instead of retiring one. + */ +function slashCommandOptions(state: { hasSession: boolean; streaming: boolean }) { + const copy = getShellCopy('zh-CN').app.slashCommands; + return slashCommandsForSurface('desktop') + .filter(desktopSlashCommandAvailability(state)) + .map(({ id }) => ({ id, ...copy[id] })); +} + +/** What Runtime's invocable projection answers for this Session. */ +const invocableSkills = [ + { ref: 'project/project-only', id: 'project-only', name: 'Project Only', description: 'Project-scoped suggestion.' }, + { ref: 'workspace/workspace-only', id: 'workspace-only', name: 'Workspace Only', description: 'Maka workspace suggestion.' }, +]; + +/** Publish a Session 'updated' event, the way a thinking-level change does. */ +let publishSessionUpdate: (() => void) | undefined; +/** Projection loads served so far, so a story can wait for one to land. */ +let projectionLoads = 0; + +/** + * The bridge `useComposerMentions` reads. A fresh array per call on purpose: + * a real IPC round trip never hands back the object it handed back last time, + * so holding the projection's identity steady is work the renderer has to do. + */ +const loadProjection = async () => { + projectionLoads += 1; + return invocableSkills.map((skill) => ({ ...skill })); +}; + +const makaBridge = { + skills: { listInvocable: loadProjection }, + newTasks: { + listInvocableSkills: loadProjection, + searchFiles: async () => ({ ok: true, files: [] }), + subscribeChanges: () => () => {}, + }, + sessions: { + subscribeChanges(listener: (event: { sessionId: string; reason: string }) => void) { + publishSessionUpdate = () => listener({ sessionId: SESSION_ID, reason: 'updated' }); + return () => { + publishSessionUpdate = undefined; + }; + }, + }, + mcp: { subscribeChanges: () => () => {} }, + workspace: { searchFiles: async () => ({ ok: true, files: [] }) }, +}; + +function SlashMenuComposer({ + hasSession, + streaming, +}: { + hasSession: boolean; + streaming: boolean; +}): React.ReactElement { + const mentions = useComposerMentionsContext(); + // Memoized exactly as app-shell.tsx memoizes it: the composer rebuilds its + // trigger — and with it an open menu — whenever this array's identity moves. + const slashCommands = useMemo( + () => slashCommandOptions({ hasSession, streaming }), + [hasSession, streaming], + ); + return ( + {}} + onStop={() => {}} + /> + ); +} + +function SlashMenuHarness({ + hasSession = true, + streaming = false, +}: { + hasSession?: boolean; + streaming?: boolean; +}): React.ReactElement { + return ( +
+ + + +
+ ); +} + +const meta = { + title: 'Product/Composer Slash Menu', + component: SlashMenuHarness, + decorators: [withScopedMakaBridge(makaBridge)], + parameters: { layout: 'fullscreen' }, +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +function editor(canvasElement: HTMLElement): HTMLElement { + const element = canvasElement.querySelector(COMPOSER_INPUT); + if (!element) throw new Error('composer editor is missing'); + return element; +} + +/** The popup renders in a layer outside the story canvas. */ +function overlay(): ReturnType { + return within(document.body); +} + +async function openMenu(canvasElement: HTMLElement): Promise { + const composer = editor(canvasElement); + await userEvent.click(composer); + await userEvent.keyboard('/'); + return overlay().findByRole('listbox', { name: MENU_LABEL }); +} + +/** + * Type a `/` into a text node the harness placed itself. Chromium can put the + * next typed character in a text node of its own, and reproducing that shape is + * the whole point — `pressSequentially` would let the browser normalize it away. + */ +function typeSlashAtEnd(composer: HTMLElement): void { + const node = composer.appendChild(document.createTextNode('/')); + const range = document.createRange(); + range.setStart(node, 1); + range.collapse(true); + const selection = window.getSelection(); + selection?.removeAllRanges(); + selection?.addRange(range); + composer.dispatchEvent( + new InputEvent('input', { bubbles: true, data: '/', inputType: 'insertText' }), + ); +} + +/** Characters between the start of the editor and the caret. */ +function caretOffset(composer: HTMLElement): number { + const selection = window.getSelection(); + if (!selection?.focusNode || !composer.contains(selection.focusNode)) return -1; + const range = document.createRange(); + range.selectNodeContents(composer); + range.setEnd(selection.focusNode, selection.focusOffset); + return range.toString().length; +} + +// Real path: 主窗口的新任务 composer（还没有 Session）→ 在空草稿开头输入 `/`。 +// Only the commands that need no Session are offered; the Skill projection is +// still the second group. +export const BeforeAnySessionExists: Story = { + args: { hasSession: false }, + play: async ({ canvasElement }) => { + const menu = await openMenu(canvasElement); + const commands = within(menu).getByRole('group', { name: '命令' }); + const options = within(commands).getAllByRole('option'); + await expect(options).toHaveLength(2); + await expect(options.map((option) => option.getAttribute('aria-label') ?? option.textContent)) + .toEqual([expect.stringContaining('/graph'), expect.stringContaining('/swarm')]); + await expect(within(options[0]!).getByText('使用 Graph')).toBeVisible(); + }, +}; + +// Real path: 打开一个已有 Session → 在 composer 的空草稿开头输入 `/`。 +// Commands first, Skills second, and each command row carries the token a user +// would otherwise have to know how to type. +export const InAnActiveSession: Story = { + play: async ({ canvasElement }) => { + const menu = await openMenu(canvasElement); + const groups = within(menu).getAllByRole('group'); + await expect(groups).toHaveLength(2); + await expect(groups[0]).toHaveAttribute('aria-label', '命令'); + await expect(groups[1]).toHaveAttribute('aria-label', 'Skills'); + await expect(within(groups[0]!).getAllByRole('option')).toHaveLength(4); + await expect(within(groups[0]!).getByText('/compact')).toBeVisible(); + await expect(within(groups[1]!).getByText('Workspace Only')).toBeVisible(); + }, +}; + +// Real path: 同上，然后点菜单里的「压缩上下文」。 +// Picking a command writes its invocation into the draft and closes the menu — +// the user still presses Enter. Nothing here auto-sends. +export const PickingACommandWritesItsInvocation: Story = { + play: async ({ canvasElement }) => { + const menu = await openMenu(canvasElement); + await userEvent.click(within(menu).getByRole('option', { name: /压缩上下文.*\/compact/ })); + const composer = editor(canvasElement); + await waitFor(() => expect(composer.textContent).toBe('/compact ')); + await waitFor(() => + expect(overlay().queryByRole('listbox', { name: MENU_LABEL })).not.toBeInTheDocument(), + ); + }, +}; + +// Real path: 在 composer 里写一个路径，例如「帮我整理到/Users/」。 +// #3849: a `/` that separates path segments is text. The positive control runs +// first — the same editor, the same synthetic input shape, opening the menu +// from an empty draft — so a menu that stopped opening at all cannot pass this. +export const PathSeparatorsAreNotTriggers: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + await userEvent.click(composer); + + typeSlashAtEnd(composer); + await overlay().findByRole('listbox', { name: MENU_LABEL }); + await userEvent.keyboard('{Escape}'); + await waitFor(() => + expect(overlay().queryByRole('listbox', { name: MENU_LABEL })).not.toBeInTheDocument(), + ); + + for (const prefix of ['帮我整理到/Users', 'path/to/file']) { + await userEvent.clear(composer); + await userEvent.type(composer, prefix); + typeSlashAtEnd(composer); + + await waitFor(() => expect(composer.textContent).toBe(`${prefix}/`)); + await expect(overlay().queryByRole('listbox', { name: MENU_LABEL })).not.toBeInTheDocument(); + // The caret stays after the slash the user just typed: a controlled + // rewrite that reset it would put the next character at the front. + await expect(caretOffset(composer)).toBe(`${prefix}/`.length); + } + }, +}; + +// Real path: 在 composer 里换行，然后在新行开头输入 `/`。 +// #3849, the other half: a block element is a line boundary, so a `/` opening a +// new visual line is a trigger even though the character before it in the DOM +// belongs to a different node. +export const ABlockBreakIsALineBoundary: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + await userEvent.click(composer); + await userEvent.type(composer, 'first line'); + + const block = document.createElement('div'); + const slash = block.appendChild(document.createTextNode('/')); + composer.appendChild(block); + const range = document.createRange(); + range.setStart(slash, 1); + range.collapse(true); + const selection = window.getSelection(); + selection?.removeAllRanges(); + selection?.addRange(range); + composer.dispatchEvent( + new InputEvent('input', { bubbles: true, data: '/', inputType: 'insertText' }), + ); + + await waitFor(() => expect(composer.innerText).toBe('first line\n/')); + const menu = await overlay().findByRole('listbox', { name: MENU_LABEL }); + // Skills only: the slash does not open the draft, so it addresses no + // command — `slashCommandQuery` owns that rule and is tested on its own. + await expect(within(menu).getByRole('group', { name: 'Skills' })).toBeVisible(); + await expect(within(menu).queryByRole('group', { name: '命令' })).not.toBeInTheDocument(); + }, +}; + +// Real path: `/` 菜单开着时 Session 发出 'updated'（改 thinking level、MCP 变更）， +// Skill 投影随之重载。 +// #2667: republishing the projection with the same content used to alternate +// the popup. The menu element and its Skills group must survive as the same +// nodes — a menu torn down and rebuilt loses the highlighted item and the +// keyboard position under the user's hands. +export const SurvivesASameContentProjectionRefresh: Story = { + play: async ({ canvasElement }) => { + const menu = await openMenu(canvasElement); + const skillsGroup = within(menu).getByRole('group', { name: 'Skills' }); + const container = menu.parentElement; + if (!container) throw new Error('slash menu container is missing'); + + const removals = { menu: 0, skillsGroup: 0 }; + const record = (mutations: MutationRecord[], watched: Node, key: 'menu' | 'skillsGroup') => { + for (const mutation of mutations) { + for (const node of mutation.removedNodes) if (node === watched) removals[key] += 1; + } + }; + // Armed on this popover only, immediately before the refreshes: the + // document body carries unrelated overlays whose teardown says nothing + // about this menu's identity. + const menuObserver = new MutationObserver((mutations) => record(mutations, menu, 'menu')); + const groupObserver = new MutationObserver((mutations) => + record(mutations, skillsGroup, 'skillsGroup'), + ); + menuObserver.observe(container, { childList: true }); + groupObserver.observe(menu, { childList: true }); + try { + for (let round = 0; round < 3; round += 1) { + const before = projectionLoads; + publishSessionUpdate?.(); + await waitFor(() => expect(projectionLoads).toBeGreaterThan(before)); + await new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); + } + } finally { + // Drain the last queued batch before closing the window this story is + // about; polling a monotonic counter cannot turn a failure into a pass. + record(menuObserver.takeRecords(), menu, 'menu'); + record(groupObserver.takeRecords(), skillsGroup, 'skillsGroup'); + menuObserver.disconnect(); + groupObserver.disconnect(); + } + + await expect(removals).toEqual({ menu: 0, skillsGroup: 0 }); + await expect(menu.isConnected).toBe(true); + await expect(skillsGroup.isConnected).toBe(true); + }, +}; diff --git a/packages/ui/src/__tests__/chat-input-behavior.test.ts b/packages/ui/src/__tests__/chat-input-behavior.test.ts index 06f0e24bd1..de48b84baa 100644 --- a/packages/ui/src/__tests__/chat-input-behavior.test.ts +++ b/packages/ui/src/__tests__/chat-input-behavior.test.ts @@ -25,6 +25,8 @@ import { composerWireText, createTriggerSearchSource, isChatInputComposing, + skillMentionQuery, + slashCommandQuery, } from '../chat-input-behavior.js'; describe('shared chat input behavior', () => { @@ -112,6 +114,38 @@ describe('shared chat input behavior', () => { assert.deepEqual(states, ['drop', null]); }); + // The `/` trigger serves two catalogs from one menu. A Skill matches wherever + // the trigger is legal, a command only when the slash opens the draft's first + // token — otherwise `请看 /Users/me` and `修一下 /compact` would both read as + // an instruction to run something. + it('offers commands only for a slash that opens the draft', () => { + // Caret right after a leading `/`, then after four typed characters. + assert.equal(slashCommandQuery('/', '', ''), ''); + assert.equal(slashCommandQuery('/comp', '', 'comp'), 'comp'); + // Leading whitespace is still an empty first token. + assert.equal(slashCommandQuery(' /comp', '', 'comp'), 'comp'); + // A slash later in the draft is prose or a path, never a command. + assert.equal(slashCommandQuery('explain /', '', ''), null); + assert.equal(slashCommandQuery('first line\n/', '', ''), null); + // `/skill:` is the explicit Skill grammar and addresses no command. + assert.equal(slashCommandQuery('/skill:compact', '', 'skill:compact'), null); + assert.equal(slashCommandQuery('/SKILL:compact', '', 'SKILL:compact'), null); + // Text after the caret means the user is editing inside a word, not + // starting a command — `/side` with the caret between `/` and `side`. + assert.equal(slashCommandQuery('/', 'side', ''), null); + // A space after the caret is not text the command would swallow. + assert.equal(slashCommandQuery('/comp', ' tail', 'comp'), 'comp'); + // The query must actually sit against the trigger the menu reports. + assert.equal(slashCommandQuery('comp', '', 'comp'), null); + }); + + it('reads `/skill:` and a bare `/` as the same Skill search', () => { + assert.equal(skillMentionQuery('skill:comp'), 'comp'); + assert.equal(skillMentionQuery('SKILL:Comp'), 'Comp'); + assert.equal(skillMentionQuery('comp'), 'comp'); + assert.equal(skillMentionQuery('skill:'), ''); + }); + it('does not let late completion clear state after reset', async () => { const states: Array = []; const owner = createChatInputActionOwner((action) => states.push(action)); From 28e38631d56d394dc08c74a298a93d2b3374aa59 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 22:03:11 +0800 Subject: [PATCH 04/12] test(desktop): move composer paste and Skill drafts off Electron MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six more cases whose subject is the browser, not the app: an undo transaction, a paste that must not open a trigger menu, and a Skill token redrawn after a draft scope switch. All of them run in the smoke's real Chromium. Undo is driven with `document.execCommand('insertText' | 'undo')` rather than synthetic keys, because `userEvent` events are not trusted input and the browser's undo stack ignores them — the same reason #3787 made the paste path use `insertHTML` in the first place. The deleted spec counted `sessionStorage` writes to prove a paste synchronizes the draft once. The story spies on `draftPersistence.write` instead: the seam `chat-composer-region.tsx` actually calls, one level above the storage the counter was reading downstream of. Generated-by: Claude Code --- .../e2e/composer-skill-invocation.spec.ts | 74 ------- apps/desktop/e2e/composer-undo.spec.ts | 143 ------------- .../stories/composer-paste.stories.tsx | 202 ++++++++++++++++++ .../stories/composer-skill-draft.stories.tsx | 150 +++++++++++++ 4 files changed, 352 insertions(+), 217 deletions(-) delete mode 100644 apps/desktop/e2e/composer-skill-invocation.spec.ts delete mode 100644 apps/desktop/e2e/composer-undo.spec.ts create mode 100644 apps/desktop/stories/composer-paste.stories.tsx create mode 100644 apps/desktop/stories/composer-skill-draft.stories.tsx diff --git a/apps/desktop/e2e/composer-skill-invocation.spec.ts b/apps/desktop/e2e/composer-skill-invocation.spec.ts deleted file mode 100644 index 3f0fab53e5..0000000000 --- a/apps/desktop/e2e/composer-skill-invocation.spec.ts +++ /dev/null @@ -1,74 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { expect, test, COMPOSER_INPUT } from './fixtures'; - -test('staged Skills come back as chips after leaving and returning', async ({ - invocableSkillsWindow: page, -}) => { - const composer = page.locator(COMPOSER_INPUT); - const projectChip = page.locator('[data-astryx-token-value="/skill:project-only"]'); - const workspaceChip = page.locator('[data-astryx-token-value="/skill:workspace-only"]'); - const pick = async (query: string, name: RegExp) => { - await composer.click(); - await composer.pressSequentially(` /${query}`); - const option = page.getByRole('listbox', { name: /技能/ }).getByRole('option', { name }); - await expect(option).toBeVisible(); - // This journey owns draft restoration, while keyboard selection is covered - // separately. Select the exact option without coupling setup to the - // popover's transient highlighted-item state under concurrent workers. - await option.click(); - }; - - await composer.fill('alpha-marker'); - await composer.press('Enter'); - await expect(page.getByText(/Fake backend received: alpha-marker/)).toBeVisible(); - - await pick('project', /Project Only/); - await composer.pressSequentially('run it'); - await pick('workspace', /Workspace Only/); - await expect(projectChip).toContainText('Project Only'); - // Both chips must land before navigating away: leaving while the second - // token is still committing races the draft snapshot and loses the chip. - await expect(workspaceChip).toContainText('Workspace Only'); - - await page.getByRole('button', { name: '展开侧边栏' }).click(); - const sidebar = page.getByRole('navigation', { name: '任务列表' }); - await sidebar.getByRole('button', { name: '新任务', exact: true }).click(); - await expect(composer).toHaveText(''); - - await sidebar.locator('[data-session-id]').first().click(); - await expect(composer).toContainText('run it'); - await expect(projectChip).toContainText('Project Only'); - await expect(workspaceChip).toContainText('Workspace Only'); - // The token text itself is gone: a chip drawn beside the text it renders - // would mean the draft now carried the Skill twice. - await expect(composer).not.toContainText('/skill:'); - - await composer.click(); - await composer.press('Enter'); - await expect( - page.getByLabel('你发送的消息').last().locator('.astryx-badge'), - ).toHaveText(['Project Only', 'Workspace Only']); -}); - -// The starter-skill window, three phases in dependency order: the blocked -// invocation pins zero turns and zero sessions so it must run before any -// send; the chip-only send then re-enables the Skill and owns the first -// message; the ＋ entry phases send nothing and run last. diff --git a/apps/desktop/e2e/composer-undo.spec.ts b/apps/desktop/e2e/composer-undo.spec.ts deleted file mode 100644 index 80c7946d8e..0000000000 --- a/apps/desktop/e2e/composer-undo.spec.ts +++ /dev/null @@ -1,143 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import type { Locator } from '@playwright/test'; -import { expect, test, COMPOSER_INPUT } from './fixtures'; - -const TYPED = 'x'; -const PASTED = '中文 & "quoted"\r\nhttps://example.test/path?x=1&y=2\n第二行 <>&'; -const PASTED_AS_PLAIN_TEXT = PASTED.replace(/\r\n/g, '\n'); -const UNDO_SHORTCUT = process.platform === 'darwin' ? 'Meta+z' : 'Control+z'; - -async function expectComposerText( - composer: Locator, - expected: string, -): Promise { - await expect - .poll(() => composer.evaluate((element) => element.innerText.replace(/\r\n/g, '\n'))) - .toBe(expected); -} - -async function pastePlainText(composer: Locator, pasted: string): Promise { - await composer.evaluate((element, text) => { - const transfer = new DataTransfer(); - transfer.setData('text/plain', text); - element.dispatchEvent( - new ClipboardEvent('paste', { - bubbles: true, - cancelable: true, - clipboardData: transfer, - }), - ); - }, pasted); -} - -test('plain-text paste is undone separately from prior typing', async ({ window: page }) => { - const composer = page.locator(COMPOSER_INPUT); - - await composer.click(); - await page.keyboard.type(TYPED); - await expectComposerText(composer, TYPED); - - await pastePlainText(composer, PASTED); - await expectComposerText(composer, `${TYPED}${PASTED_AS_PLAIN_TEXT}`); - - await page.keyboard.press(UNDO_SHORTCUT); - await expectComposerText(composer, TYPED); - - await page.keyboard.press(UNDO_SHORTCUT); - await expectComposerText(composer, ''); - - await page.keyboard.press(UNDO_SHORTCUT); - await expectComposerText(composer, ''); -}); - -test('pasted absolute path sends on the first Enter without opening the slash menu', async ({ - window: page, -}) => { - const composer = page.locator(COMPOSER_INPUT); - const pasted = '/Users/me/notes.txt'; - - await expect(page.locator('button.maka-workspace-picker')).toBeEnabled(); - await composer.click(); - await pastePlainText(composer, pasted); - - await expect(composer).toHaveAttribute('aria-expanded', 'false'); - await composer.press('Enter'); - await expect(page.getByText(`Fake backend received: ${pasted}`)).toBeVisible(); -}); - -test('pasted mention-looking text does not open the file menu', async ({ window: page }) => { - const composer = page.locator(COMPOSER_INPUT); - - await expect(page.locator('button.maka-workspace-picker')).toBeEnabled(); - await composer.click(); - await pastePlainText(composer, 'review @name'); - - await expect(composer).toHaveAttribute('aria-expanded', 'false'); - await expect(page.getByRole('listbox')).toHaveCount(0); -}); - -test('plain-text paste closes an existing trigger menu before the first Enter', async ({ - window: page, -}) => { - const composer = page.locator(COMPOSER_INPUT); - const pasted = 'Users/me/notes.txt'; - - await expect(page.locator('button.maka-workspace-picker')).toBeEnabled(); - await composer.click(); - await composer.pressSequentially('/'); - await expect(composer).toHaveAttribute('aria-expanded', 'true'); - - await pastePlainText(composer, pasted); - - await expect(composer).toHaveAttribute('aria-expanded', 'false'); - await composer.press('Enter'); - await expect(page.getByText(`Fake backend received: /${pasted}`)).toBeVisible(); -}); - -test('plain-text paste synchronizes the controlled draft once', async ({ window: page }) => { - const composer = page.locator(COMPOSER_INPUT); - - await expect(page.locator('button.maka-workspace-picker')).toBeEnabled(); - await page.evaluate(() => { - const storageKey = 'maka-new-task-reload-intent-v1'; - sessionStorage.setItem(storageKey, JSON.stringify({ draft: '' })); - const originalSetItem = Storage.prototype.setItem; - let writes = 0; - Storage.prototype.setItem = function setItem(key: string, value: string): void { - if (key === storageKey) writes += 1; - originalSetItem.call(this, key, value); - }; - Object.defineProperty(globalThis, '__makaComposerDraftWrites', { - configurable: true, - get: () => writes, - }); - }); - - await composer.click(); - await pastePlainText(composer, 'one controlled change'); - - const writes = await page.evaluate( - () => - (globalThis as typeof globalThis & { __makaComposerDraftWrites?: number }) - .__makaComposerDraftWrites ?? 0, - ); - expect(writes).toBe(1); -}); diff --git a/apps/desktop/stories/composer-paste.stories.tsx b/apps/desktop/stories/composer-paste.stories.tsx new file mode 100644 index 0000000000..b6bd60beed --- /dev/null +++ b/apps/desktop/stories/composer-paste.stories.tsx @@ -0,0 +1,202 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * What a plain-text paste owes the composer (#3787). + * + * The composer inserts pasted text with `execCommand('insertHTML')` — the one + * primitive left that opens a browser undo transaction — instead of writing the + * controlled draft. So the browser's own undo stack is the subject here, and + * these stories drive it with the browser's own primitives: `insertText` for a + * keystroke's transaction, `undo` for the shortcut. A DOM shim has no undo + * stack at all, and a synthetic `Meta+z` is not trusted input, so neither the + * unit tier nor `userEvent` can reach this. + * + * Fidelity convention (#1433): chat-composer-region.tsx renders this Composer + * with a `draftPersistence` that writes the new-task reload draft; the spy + * below stands exactly where that writer stands. See FIDELITY.md. + */ + +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, fn, userEvent, waitFor, within } from 'storybook/test'; +import { slashCommandsForSurface } from '@maka/core/slash-command-catalog'; +import { Composer } from '@maka/ui'; +import { desktopSlashCommandAvailability } from '../src/renderer/desktop-slash-command'; +import { getShellCopy } from '../src/renderer/locales/shell-copy'; + +const COMPOSER_INPUT = '.maka-composer-editor [contenteditable="true"]'; +const TYPED = 'x'; +const PASTED = '中文 & "quoted"\r\nhttps://example.test/path?x=1&y=2\n第二行 <>&'; +const PASTED_AS_PLAIN_TEXT = PASTED.replace(/\r\n/g, '\n'); + +// The new-task composer's command list, from the catalog, the availability +// predicate and the shell's copy — the same three app-shell.tsx reads. +const slashCommands = slashCommandsForSurface('desktop') + .filter(desktopSlashCommandAvailability({ hasSession: false, streaming: false })) + .map(({ id }) => ({ id, ...getShellCopy('zh-CN').app.slashCommands[id] })); + +/** Records what the host would have persisted, one call per controlled change. */ +const draftWrites = fn(); +const sent = fn(); + +function PasteHarness(): React.ReactElement { + return ( +
+ undefined, write: (_key, value) => draftWrites(value) }} + slashCommands={slashCommands} + onSearchMentionFiles={async () => []} + onSend={(text) => { + sent(text); + }} + onStop={() => {}} + /> +
+ ); +} + +const meta = { + title: 'Product/Composer Paste', + component: PasteHarness, + parameters: { layout: 'fullscreen' }, + beforeEach: () => { + draftWrites.mockClear(); + sent.mockClear(); + }, +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +function editor(canvasElement: HTMLElement): HTMLElement { + const element = canvasElement.querySelector(COMPOSER_INPUT); + if (!element) throw new Error('composer editor is missing'); + return element; +} + +/** The `/` and `@` popups render in a layer outside the story canvas. */ +function overlay(): ReturnType { + return within(document.body); +} + +function pastePlainText(composer: HTMLElement, text: string): void { + const transfer = new DataTransfer(); + transfer.setData('text/plain', text); + composer.dispatchEvent( + new ClipboardEvent('paste', { bubbles: true, cancelable: true, clipboardData: transfer }), + ); +} + +function draftText(composer: HTMLElement): string { + return composer.innerText.replace(/\r\n/g, '\n'); +} + +// Real path: 在 composer 里打了字，再从别处粘贴一段文本，然后按 ⌘Z。 +// One paste is one undo. Before #3787 the paste wrote the controlled draft +// instead of inserting, which left the browser with no transaction to reverse: +// the first undo took the whole draft, typing included. +export const PasteIsItsOwnUndoStep: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + await userEvent.click(composer); + // The browser's own text insertion, so the keystroke opens the same kind + // of transaction a trusted key press would. + document.execCommand('insertText', false, TYPED); + await waitFor(() => expect(draftText(composer)).toBe(TYPED)); + + pastePlainText(composer, PASTED); + await waitFor(() => expect(draftText(composer)).toBe(`${TYPED}${PASTED_AS_PLAIN_TEXT}`)); + + document.execCommand('undo'); + await waitFor(() => expect(draftText(composer)).toBe(TYPED)); + document.execCommand('undo'); + await waitFor(() => expect(draftText(composer)).toBe('')); + // An empty stack stays empty rather than reaching past the composer. + document.execCommand('undo'); + await expect(draftText(composer)).toBe(''); + }, +}; + +// Real path: 从 Finder 或终端复制一个绝对路径，粘进 composer，直接回车。 +// A pasted absolute path is text, and Enter sends it. The slash it starts with +// must not arm the command menu, or the first Enter would be swallowed +// accepting a suggestion. +export const PastedPathSendsOnTheFirstEnter: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + const pasted = '/Users/me/notes.txt'; + await userEvent.click(composer); + pastePlainText(composer, pasted); + + await waitFor(() => expect(draftText(composer)).toBe(pasted)); + await expect(composer).toHaveAttribute('aria-expanded', 'false'); + await userEvent.keyboard('{Enter}'); + await waitFor(() => expect(sent).toHaveBeenCalledWith(pasted)); + }, +}; + +// Real path: 粘贴一段带 `@名字` 的文本（评论、聊天记录）。 +// The same rule for `@`: a pasted mention is text, not a file lookup. +export const PastedMentionDoesNotOpenTheFileMenu: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + await userEvent.click(composer); + pastePlainText(composer, 'review @name'); + + await waitFor(() => expect(draftText(composer)).toBe('review @name')); + await expect(composer).toHaveAttribute('aria-expanded', 'false'); + await expect(overlay().queryByRole('listbox')).not.toBeInTheDocument(); + }, +}; + +// Real path: `/` 菜单已经打开时粘贴，再回车。 +// A menu that was already open closes on paste. It is answering a query the +// paste has just invalidated, and leaving it open costs the next Enter. +export const PasteClosesAnOpenTriggerMenu: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + const pasted = 'Users/me/notes.txt'; + await userEvent.click(composer); + await userEvent.keyboard('/'); + await waitFor(() => expect(composer).toHaveAttribute('aria-expanded', 'true')); + + pastePlainText(composer, pasted); + + await waitFor(() => expect(composer).toHaveAttribute('aria-expanded', 'false')); + await userEvent.keyboard('{Enter}'); + await waitFor(() => expect(sent).toHaveBeenCalledWith(`/${pasted}`)); + }, +}; + +// Real path: 在新任务 composer 里粘贴一次；宿主随即写一次重载草稿。 +// One paste, one controlled change. The insertion and the sync used to both +// reach the host, which wrote the reload draft twice for a single edit. +export const PasteSynchronizesTheDraftOnce: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + await userEvent.click(composer); + pastePlainText(composer, 'one controlled change'); + + await waitFor(() => expect(draftText(composer)).toBe('one controlled change')); + await expect(draftWrites).toHaveBeenCalledTimes(1); + await expect(draftWrites).toHaveBeenCalledWith('one controlled change'); + }, +}; diff --git a/apps/desktop/stories/composer-skill-draft.stories.tsx b/apps/desktop/stories/composer-skill-draft.stories.tsx new file mode 100644 index 0000000000..123c10637a --- /dev/null +++ b/apps/desktop/stories/composer-skill-draft.stories.tsx @@ -0,0 +1,150 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * A staged Skill is text in the draft, drawn as a chip. + * + * That is the whole point of the design: there is one draft, `/skill:` is + * the invocation grammar Runtime already parses, and everything that carries a + * draft carries the Skill for free. The cost is that a controlled write flattens + * the chips back to their text — Astryx rebuilds the editor from the string — + * so `redrawSkillTokens` has to put them back. Leaving a Session and returning + * is the path that write is on. + * + * Fidelity convention (#1433): the composer's `draftKey` is what app-shell + * changes when the active Session changes; switching it here is the same event. + * See FIDELITY.md. + * + * A browser, not a DOM shim: the redraw drives `insertToken` through a live + * document Selection over the editor's text node offsets. + */ + +import { useEffect, useState } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, fn, userEvent, waitFor, within } from 'storybook/test'; +import { Composer } from '@maka/ui'; + +const COMPOSER_INPUT = '.maka-composer-editor [contenteditable="true"]'; +const SESSION_DRAFT_KEY = 'session:skill-draft'; +const NEW_TASK_DRAFT_KEY = 'new-task:story'; + +const skills: ReadonlyArray<{ id: string; name: string; description: string }> = [ + { id: 'project-only', name: 'Project Only', description: 'Project-scoped suggestion.' }, + { id: 'workspace-only', name: 'Workspace Only', description: 'Maka workspace suggestion.' }, +]; + +/** Move the composer to another draft scope, the way a Session switch does. */ +let selectDraftKey: ((key: string) => void) | undefined; +const sent = fn(); + +function SkillDraftHarness(): React.ReactElement { + const [draftKey, setDraftKey] = useState(SESSION_DRAFT_KEY); + useEffect(() => { + selectDraftKey = setDraftKey; + return () => { + selectDraftKey = undefined; + }; + }, []); + return ( +
+ { + sent(text); + }} + onStop={() => {}} + /> +
+ ); +} + +const meta = { + title: 'Product/Composer Skill Draft', + component: SkillDraftHarness, + parameters: { layout: 'fullscreen' }, + beforeEach: () => { + sent.mockClear(); + }, +} satisfies Meta; + +export default meta; + +type Story = StoryObj; + +function editor(canvasElement: HTMLElement): HTMLElement { + const element = canvasElement.querySelector(COMPOSER_INPUT); + if (!element) throw new Error('composer editor is missing'); + return element; +} + +function chip(id: string): HTMLElement | null { + return document.querySelector(`[data-astryx-token-value="/skill:${id}"]`); +} + +async function pickSkill(composer: HTMLElement, query: string, name: RegExp): Promise { + await userEvent.click(composer); + await userEvent.keyboard(` /${query}`); + const option = await within(document.body) + .findByRole('listbox', { name: /技能/ }) + .then((menu) => within(menu).findByRole('option', { name })); + // The exact option, not the popover's transient highlight: what this story + // owns is draft restoration, and keyboard selection is covered on its own. + await userEvent.click(option); +} + +// Real path: 在某个 Session 的 composer 里用 `/` 选两个 Skill 并夹一句话 → 侧边栏 +// 「新任务」→ 再点回原来那个 Session。 +// #2137: both chips come back after the draft has been away and returned, and +// the token text they were drawn from is gone — a chip standing beside the text +// it renders would mean the draft carries that Skill twice. +export const StagedSkillsSurviveADraftScopeSwitch: Story = { + play: async ({ canvasElement }) => { + const composer = editor(canvasElement); + + await pickSkill(composer, 'project', /Project Only/); + await userEvent.keyboard('run it'); + await pickSkill(composer, 'workspace', /Workspace Only/); + // Both tokens must have committed before the draft moves: leaving while the + // second is still landing races the snapshot and loses the chip. + await waitFor(() => expect(chip('project-only')).toHaveTextContent('Project Only')); + await waitFor(() => expect(chip('workspace-only')).toHaveTextContent('Workspace Only')); + + // Away: a new task is a different draft scope and starts empty. + selectDraftKey?.(NEW_TASK_DRAFT_KEY); + await waitFor(() => expect(composer.textContent).toBe('')); + await expect(chip('project-only')).toBeNull(); + + // Back: the draft returns as a flat string and the chips are redrawn from it. + selectDraftKey?.(SESSION_DRAFT_KEY); + await waitFor(() => expect(composer.textContent).toContain('run it')); + await waitFor(() => expect(chip('project-only')).toHaveTextContent('Project Only')); + await expect(chip('workspace-only')).toHaveTextContent('Workspace Only'); + await expect(composer.textContent).not.toContain('/skill:'); + + // The restored draft is still the invocation Runtime parses. + await userEvent.click(composer); + await userEvent.keyboard('{Enter}'); + await waitFor(() => expect(sent).toHaveBeenCalledTimes(1)); + const [wire] = sent.mock.calls[0] as [string]; + await expect(wire).toContain('/skill:project-only'); + await expect(wire).toContain('/skill:workspace-only'); + await expect(wire).toContain('run it'); + }, +}; From a637a7cabfe8518ae9166bff3466f6cb0cf99f25 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 22:03:16 +0800 Subject: [PATCH 05/12] test(desktop): take fixture-thread-search off the warm window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was the last CI test sharing the warm prompt-rail window, and sharing bought it nothing: search reads the Host through the bridge and renders nothing, so it needs neither that window's compositor nor its between-test reset. Moving it onto an ordinary per-test window retires the cross-test readiness bleed behind #4707. The warm fixture itself stays — `native-transcript-perf` still needs a window whose compositor has settled. Generated-by: Claude Code --- apps/desktop/e2e/fixture-thread-search.spec.ts | 2 +- apps/desktop/e2e/fixtures.ts | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/apps/desktop/e2e/fixture-thread-search.spec.ts b/apps/desktop/e2e/fixture-thread-search.spec.ts index e4b50b1bc1..43a675a2ab 100644 --- a/apps/desktop/e2e/fixture-thread-search.spec.ts +++ b/apps/desktop/e2e/fixture-thread-search.spec.ts @@ -25,7 +25,7 @@ import { import { expect, test } from './fixtures.js'; test('fixture-seeded transcripts return content hits with turn ids', async ({ - promptRailWindow: page, + threadSearchWindow: page, }) => { const outcome = await page.evaluate(async () => window.maka.search.thread({ diff --git a/apps/desktop/e2e/fixtures.ts b/apps/desktop/e2e/fixtures.ts index 45a3b91bb1..42ce777aca 100644 --- a/apps/desktop/e2e/fixtures.ts +++ b/apps/desktop/e2e/fixtures.ts @@ -571,6 +571,7 @@ type E2eTestFixtures = { parentRemovalWindow: Page; railRenderWindow: Page; promptRailWindow: Page; + threadSearchWindow: Page; partialHistoryWindow: Page; requestHeaderRowWindow: Page; permissionCenterWindow: Page; @@ -753,6 +754,18 @@ export const test = base.extend({ await setPromptRailWindowVisible(promptRailWorker, false); } }, + // The same seeded transcript, on a window of its own. Search reads the Host + // through the bridge and renders nothing, so it needs neither the warm + // window's compositor nor its between-test reset — and taking it off the + // reused window is what retires the readiness gate's cross-test bleed (#4707). + threadSearchWindow: async ({}, use) => { + await withE2eWindow({ + seed: false, + readinessSelector: '[data-turn-id]', + e2eFixtureScenario: 'chat-prompt-rail', + locale: 'zh-CN', + }, use); + }, // A transcript larger than the bounded Desktop range. Clicking an unloaded // prompt exercises the real load-around path and its partial-history UI. partialHistoryWindow: async ({}, use) => { From 07abb21297d6c2ec5502f6b72ab33d4ef49ddb62 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 22:35:43 +0800 Subject: [PATCH 06/12] test(desktop): fence the new-task-reload send MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It failed on CI with `element(s) not found` after Enter, and the failure snapshot carries the whole signature #4573 described: the draft still sitting in the composer, no Turn, and 发送 already enabled by the time the assertion gave up. A successful send clears the draft, so the keypress was dropped by `sendCurrent` while admission was still unresolved — not rendered slowly. #4577 fenced the three specs that had failed by then and left the rest to adopt `awaitSendReady` when they failed. This one just did. Generated-by: Claude Code --- apps/desktop/e2e/new-task-reload.spec.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/desktop/e2e/new-task-reload.spec.ts b/apps/desktop/e2e/new-task-reload.spec.ts index 0c2c30fd95..d6913a49f6 100644 --- a/apps/desktop/e2e/new-task-reload.spec.ts +++ b/apps/desktop/e2e/new-task-reload.spec.ts @@ -17,15 +17,18 @@ * under the License. */ -import { COMPOSER_INPUT, ensureSidebarExpanded, expect, test } from './fixtures'; +import { COMPOSER_INPUT, awaitSendReady, ensureSidebarExpanded, expect, test } from './fixtures'; test('an explicit new task survives a renderer reload without reopening history', async ({ window: page, }) => { const composer = page.locator(COMPOSER_INPUT); await composer.fill('create history'); + await awaitSendReady(page); await composer.press('Enter'); - await expect(page.getByText(/Fake backend received: create history/)).toBeVisible(); + await expect(page.getByText(/Fake backend received: create history/)).toBeVisible({ + timeout: 20_000, + }); await ensureSidebarExpanded(page); await page.getByRole('button', { name: '新任务', exact: true }).click(); From 23b6b1a5779d4e2e81851ba6317a83e8a3789268 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 23:12:54 +0800 Subject: [PATCH 07/12] fix(ui): believe a reader who scrolls while the answer grows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The scroll authority decided "this was content, not the reader" from the geometry having changed since the last event. During growth it always has, so a reader's own scroll arrived carrying a changed `scrollHeight` and was discarded along with it: the pin stayed set, and the next ResizeObserver delivery wrote the view straight back to the tail. Scroll up while an answer streams and the transcript pulls you back — the one moment a reader most needs to be believed. Geometry cannot decide it, because during growth both readings are true. What content can do to the offset is bounded: anchoring pushes it down by at most what was inserted above the reader, and a transcript that shrank clamps it up to the new end. A move outside that band has no explanation but the reader, so that is what decides now. The case the old rule existed for is unchanged and still covered: growth that outran this authority's own write moves the offset by nothing while the tail runs away, which is inside the band and so is not a reader. This is what made `ReaderScrolledUpIsNotPulledBack` and `DockAffordanceReturnsToTail` fail on CI and never locally — CI is slow enough for a resolution to land between the write and its event. Both are about the reader being left alone, so both were telling the truth. Generated-by: Claude Code --- .../transcript-scroll-authority.test.ts | 25 +++++++++++ .../ui/src/transcript-scroll-authority.tsx | 41 +++++++++++++------ 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/packages/ui/src/__tests__/transcript-scroll-authority.test.ts b/packages/ui/src/__tests__/transcript-scroll-authority.test.ts index 9901780cb0..3f1cc7056a 100644 --- a/packages/ui/src/__tests__/transcript-scroll-authority.test.ts +++ b/packages/ui/src/__tests__/transcript-scroll-authority.test.ts @@ -254,6 +254,31 @@ test('growth that outruns the write does not read as the reader scrolling up', ( }); }); +test('a reader who scrolls up while the answer grows is still the reader', () => { + withObservers((resize) => { + const root = fakeRoot(); + const authority = createTranscriptScrollAuthority(); + authority.attach(root as unknown as HTMLElement); + assert.equal(root.scrollTop, 2_400); + + // The same shape as the case above — a scroll event carrying a grown + // `scrollHeight` — and the opposite intent. Growth cannot move the offset + // backwards, so an offset that went up the transcript is the reader's, and + // during a streaming answer this is the only kind of event they produce. + root.grow(37); + root.scrollTop = 1_900; + root.emitScroll(); + assert.equal(authority.getSnapshot().pinned, false); + assert.equal(authority.getSnapshot().awayFromTail, true); + + // And the pin stays off: what arrives next is more of the same answer, and + // following it would take the transcript away from where they went. + root.grow(300); + resize(); + assert.equal(root.scrollTop, 1_900); + }); +}); + test('content landing above a released reader does not re-pin them', () => { withObservers((resize) => { const root = fakeRoot(); diff --git a/packages/ui/src/transcript-scroll-authority.tsx b/packages/ui/src/transcript-scroll-authority.tsx index e2a20e5b0a..f09f24c97a 100644 --- a/packages/ui/src/transcript-scroll-authority.tsx +++ b/packages/ui/src/transcript-scroll-authority.tsx @@ -110,6 +110,8 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { */ let lastScrollHeight = 0; let lastClientHeight = 0; + /** The offset the last event saw, to measure the next one's move against. */ + let lastScrollTop = 0; let snapshot: TranscriptScrollSnapshot = { pinned, awayFromTail }; const listeners = new Set<() => void>(); const readerListeners = new Set<() => void>(); @@ -131,6 +133,7 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { lastWrittenTop = root.scrollTop; lastScrollHeight = root.scrollHeight; lastClientHeight = root.clientHeight; + lastScrollTop = root.scrollTop; awayFromTail = false; publish(); }; @@ -150,24 +153,37 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { if (lastWrittenTop !== undefined && Math.abs(target.scrollTop - lastWrittenTop) < 1) { lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; + lastScrollTop = target.scrollTop; return; } - // An event that arrives with the scroll geometry changed is the content - // or the viewport moving under the reader, not the reader moving: - // anchoring holding them still as turns land above, growth that outran - // this authority's own write, or a resize the browser answered by - // clamping the offset. Their offset changed and their intent did not, - // so the pin — which is that intent — must not be re-derived from where - // they now are, and nobody may be told the reader asked for anything. - // The affordance still follows the new distance, because that is a fact - // about the viewport rather than about them. - const moved = - target.scrollHeight !== lastScrollHeight || target.clientHeight !== lastClientHeight; + // Content moves the offset too, and only within a band it can account + // for: anchoring pushes the offset down by at most what was inserted + // above the reader, and a transcript that shrank clamps it up to the + // new end. Inside that band their offset changed and their intent did + // not, so the pin must not be re-derived from where they now are, and + // nobody may be told the reader asked for anything. The affordance + // still follows the new distance, because that is a fact about the + // viewport rather than about them. + // + // Outside it, the move is the reader's, and this may not be decided + // from the geometry merely having changed. During growth it always + // has, so a reader who scrolled while an answer streamed arrived + // carrying a changed `scrollHeight` and was discarded along with it — + // the pin stayed, and the next growth wrote the view back to the tail. + // Scrolling away from a streaming answer is the one moment a reader + // most needs to be believed. + const maxScroll = target.scrollHeight - target.clientHeight; + const explainedLow = Math.min(0, maxScroll - lastScrollTop); + const explainedHigh = Math.max(0, maxScroll - (lastScrollHeight - lastClientHeight)); + const topDelta = target.scrollTop - lastScrollTop; + const readerMoved = + topDelta < explainedLow - PIN_THRESHOLD_PX || topDelta > explainedHigh + PIN_THRESHOLD_PX; lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; + lastScrollTop = target.scrollTop; const distance = distanceToTail(); awayFromTail = distance > BUTTON_THRESHOLD_PX; - if (moved) { + if (!readerMoved) { publish(); return; } @@ -177,6 +193,7 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { }; lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; + lastScrollTop = target.scrollTop; target.addEventListener('scroll', onScroll, { passive: true }); // Everything that moves the tail without the reader asking, watched in // one place: the scroller's own box, because the tail also moves when the From 2338ebd1ce4654133eda85c73d142a3033088dec Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 23:30:16 +0800 Subject: [PATCH 08/12] fix(ui): read the reader's move against what content can explain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the previous commit found the band right and the reading of it wrong, in two ways that cancelled part of the fix and cost some of what already worked. Content can pull the offset up as well as push it down: anchoring answers a removal above the reader — a tool block folding, a status line going — by moving the offset up by exactly what went. The lower bound covered only end-clamping, so a fold above a pinned reader read as a reader and quietly stopped the transcript following its own answer. And the slack was `PIN_THRESHOLD_PX`, which is a distance from the tail and says nothing about how far an offset may miss its explanation. Ten pixels of it, spent afresh on every event, swallowed every move smaller than that: a reader crossing the transcript on a trackpad was never heard at all, growth or no growth. What is left over is now kept between events and measured against the sub-pixel rounding it actually is, so a run of small moves adds up to the reader it plainly is. Three cases cover the corrections: a slow reader, a fold above the reader, and a scrollport that grew. Re-baselining every event turns the first red; dropping the removal term turns the second red. Generated-by: Claude Code --- .../transcript-scroll-authority.test.ts | 76 +++++++++++++++++++ .../ui/src/transcript-scroll-authority.tsx | 45 ++++++++--- 2 files changed, 110 insertions(+), 11 deletions(-) diff --git a/packages/ui/src/__tests__/transcript-scroll-authority.test.ts b/packages/ui/src/__tests__/transcript-scroll-authority.test.ts index 3f1cc7056a..4debc43eaf 100644 --- a/packages/ui/src/__tests__/transcript-scroll-authority.test.ts +++ b/packages/ui/src/__tests__/transcript-scroll-authority.test.ts @@ -279,6 +279,82 @@ test('a reader who scrolls up while the answer grows is still the reader', () => }); }); +test('a slow reader is a reader, however small each step is', () => { + withObservers((resize) => { + const root = fakeRoot(); + const authority = createTranscriptScrollAuthority(); + authority.attach(root as unknown as HTMLElement); + let readerMoves = 0; + authority.subscribeToReaderScroll(() => { + readerMoves += 1; + }); + + // A trackpad crossing the transcript unhurriedly. Judged one event at a + // time against the rounding this has to tolerate, every one of these is + // noise and the reader never moves at all; they only mean anything added + // up. Nothing grows here, so there is nothing else they could be. + for (let step = 0; step < 90; step += 1) { + root.scrollTop -= 2; + root.emitScroll(); + } + assert.equal(authority.getSnapshot().pinned, false); + assert.ok(readerMoves > 0, 'the reader moved 180px and was never heard'); + + root.grow(500); + resize(); + assert.equal(root.scrollTop, 2_220); + }); +}); + +test('content leaving from above the reader is not the reader either', () => { + withObservers(() => { + const root = fakeRoot(); + const authority = createTranscriptScrollAuthority(); + authority.attach(root as unknown as HTMLElement); + authority.releasePin(); + root.scrollTop = 1_500; + root.emitScroll(); + assert.equal(authority.getSnapshot().pinned, false); + let readerMoves = 0; + authority.subscribeToReaderScroll(() => { + readerMoves += 1; + }); + + // A tool block above them folds away. Anchoring answers a removal the same + // way it answers an arrival — by moving the offset exactly as far — so the + // reader is still looking at the same content and has asked for nothing. + root.grow(-60); + root.scrollTop = 1_440; + root.emitScroll(); + assert.equal(readerMoves, 0); + assert.equal(authority.getSnapshot().pinned, false); + }); +}); + +test('a viewport that grew does not move the reader, it only clamps them', () => { + withObservers(() => { + const root = fakeRoot(); + const authority = createTranscriptScrollAuthority(); + authority.attach(root as unknown as HTMLElement); + authority.releasePin(); + root.scrollTop = 2_350; + root.emitScroll(); + let readerMoves = 0; + authority.subscribeToReaderScroll(() => { + readerMoves += 1; + }); + + // The composer loses a line, so the scrollport gets taller and the end of + // the transcript moves up past where the reader was sitting. The browser + // clamps them to it; they did not ask to go. + root.shrinkViewport(-200); + root.scrollTop = 2_200; + root.emitScroll(); + assert.equal(readerMoves, 0); + assert.equal(authority.getSnapshot().pinned, false); + }); +}); + test('content landing above a released reader does not re-pin them', () => { withObservers((resize) => { const root = fakeRoot(); diff --git a/packages/ui/src/transcript-scroll-authority.tsx b/packages/ui/src/transcript-scroll-authority.tsx index f09f24c97a..9f2b6916d8 100644 --- a/packages/ui/src/transcript-scroll-authority.tsx +++ b/packages/ui/src/transcript-scroll-authority.tsx @@ -53,6 +53,14 @@ import { ChatLayoutScrollButton } from '@astryxdesign/core/Chat'; /** Astryx's own thresholds, so the affordance keeps the feel readers learnt. */ const PIN_THRESHOLD_PX = 10; const BUTTON_THRESHOLD_PX = 100; +/** + * How far the offset may miss what the content accounts for and still be the + * content. `scrollHeight` and `clientHeight` are integers while `scrollTop` is + * not, so the arithmetic below is exact to under a pixel — this is that + * rounding, not a gesture the reader might have made. The pin threshold is a + * distance from the tail and says nothing about it. + */ +const GEOMETRY_ROUNDING_PX = 2; export interface TranscriptScrollSnapshot { /** Following the tail: growth writes `scrollTop`. */ @@ -112,6 +120,8 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { let lastClientHeight = 0; /** The offset the last event saw, to measure the next one's move against. */ let lastScrollTop = 0; + /** Offset movement no content change accounts for, since the last reading. */ + let unexplained = 0; let snapshot: TranscriptScrollSnapshot = { pinned, awayFromTail }; const listeners = new Set<() => void>(); const readerListeners = new Set<() => void>(); @@ -134,6 +144,7 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { lastScrollHeight = root.scrollHeight; lastClientHeight = root.clientHeight; lastScrollTop = root.scrollTop; + unexplained = 0; awayFromTail = false; publish(); }; @@ -154,16 +165,19 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; lastScrollTop = target.scrollTop; + unexplained = 0; return; } // Content moves the offset too, and only within a band it can account - // for: anchoring pushes the offset down by at most what was inserted - // above the reader, and a transcript that shrank clamps it up to the - // new end. Inside that band their offset changed and their intent did - // not, so the pin must not be re-derived from where they now are, and - // nobody may be told the reader asked for anything. The affordance - // still follows the new distance, because that is a fact about the - // viewport rather than about them. + // for. Native anchoring answers content landing above the reader by + // pushing the offset down by exactly what was inserted, and content + // leaving from above by pulling it up by exactly what went; a + // transcript shorter than the offset clamps it to the new end. Inside + // that band their offset changed and their intent did not, so the pin + // must not be re-derived from where they now are, and nobody may be + // told the reader asked for anything. The affordance still follows the + // new distance, because that is a fact about the viewport rather than + // about them. // // Outside it, the move is the reader's, and this may not be decided // from the geometry merely having changed. During growth it always @@ -173,11 +187,18 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { // Scrolling away from a streaming answer is the one moment a reader // most needs to be believed. const maxScroll = target.scrollHeight - target.clientHeight; - const explainedLow = Math.min(0, maxScroll - lastScrollTop); - const explainedHigh = Math.max(0, maxScroll - (lastScrollHeight - lastClientHeight)); + const contentDelta = maxScroll - (lastScrollHeight - lastClientHeight); + const explainedLow = Math.min(0, contentDelta, maxScroll - lastScrollTop); + const explainedHigh = Math.max(0, contentDelta); const topDelta = target.scrollTop - lastScrollTop; - const readerMoved = - topDelta < explainedLow - PIN_THRESHOLD_PX || topDelta > explainedHigh + PIN_THRESHOLD_PX; + // What the content could not account for, kept across events rather + // than judged one at a time. A reader crossing the transcript slowly + // produces a run of moves each smaller than the rounding this has to + // tolerate, and re-baselining on every event would spend every one of + // them: they would never add up to a reader, and the answer would go + // on writing the view out from under a reader who was plainly moving. + unexplained += topDelta - Math.min(explainedHigh, Math.max(explainedLow, topDelta)); + const readerMoved = Math.abs(unexplained) > GEOMETRY_ROUNDING_PX; lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; lastScrollTop = target.scrollTop; @@ -187,6 +208,7 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { publish(); return; } + unexplained = 0; pinned = distance <= PIN_THRESHOLD_PX; publish(); for (const listener of [...readerListeners]) listener(); @@ -194,6 +216,7 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; lastScrollTop = target.scrollTop; + unexplained = 0; target.addEventListener('scroll', onScroll, { passive: true }); // Everything that moves the tail without the reader asking, watched in // one place: the scroller's own box, because the tail also moves when the From fc427477a3c679a427b0c16acb904bcbfee2ba5d Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 23:35:54 +0800 Subject: [PATCH 09/12] refactor(ui): drop the clamp term the content delta already covers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ablation: removing it fails nothing, and it cannot — the offset is never past the end it is read from, so `maxScroll - lastScrollTop` is always the looser of the two and the minimum was never taking it. What content did to the offset is one number, and the band is between nothing and that. Generated-by: Claude Code --- .../ui/src/transcript-scroll-authority.tsx | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/ui/src/transcript-scroll-authority.tsx b/packages/ui/src/transcript-scroll-authority.tsx index 9f2b6916d8..39f189a256 100644 --- a/packages/ui/src/transcript-scroll-authority.tsx +++ b/packages/ui/src/transcript-scroll-authority.tsx @@ -168,16 +168,17 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { unexplained = 0; return; } - // Content moves the offset too, and only within a band it can account - // for. Native anchoring answers content landing above the reader by - // pushing the offset down by exactly what was inserted, and content - // leaving from above by pulling it up by exactly what went; a - // transcript shorter than the offset clamps it to the new end. Inside - // that band their offset changed and their intent did not, so the pin - // must not be re-derived from where they now are, and nobody may be - // told the reader asked for anything. The affordance still follows the - // new distance, because that is a fact about the viewport rather than - // about them. + // Content moves the offset too, and only ever by how much the end of + // the transcript moved. Native anchoring answers content landing above + // the reader by pushing the offset down by exactly what was inserted, + // content leaving from above by pulling it up by exactly what went, + // and a transcript that ends before the offset by clamping it to the + // new end — every one of them somewhere between nothing and that whole + // amount. Inside that band their offset changed and their intent did + // not, so the pin must not be re-derived from where they now are, and + // nobody may be told the reader asked for anything. The affordance + // still follows the new distance, because that is a fact about the + // viewport rather than about them. // // Outside it, the move is the reader's, and this may not be decided // from the geometry merely having changed. During growth it always @@ -188,7 +189,7 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { // most needs to be believed. const maxScroll = target.scrollHeight - target.clientHeight; const contentDelta = maxScroll - (lastScrollHeight - lastClientHeight); - const explainedLow = Math.min(0, contentDelta, maxScroll - lastScrollTop); + const explainedLow = Math.min(0, contentDelta); const explainedHigh = Math.max(0, contentDelta); const topDelta = target.scrollTop - lastScrollTop; // What the content could not account for, kept across events rather From 94e86beac648c9c9f0eb56bfc25af928ea4a65f6 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Fri, 4 Sep 2026 23:42:37 +0800 Subject: [PATCH 10/12] test(desktop): keep the two slash claims Electron still owns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the move found the leaf assertions carried over and the wiring under them left behind, in two places, and it was right on both. The stories are handed `hasSession`, so `Boolean(activeId)` in app-shell is theirs to assume. Setting it to `false` in production leaves every slash story green while an active Session silently loses `/compact` and `/side`. And selecting `/compact` only writes an invocation into the draft. What that draft does when submitted — `sessions.compact()`, a status change, a composer that clears, and above all a command that never reaches the model — was covered by the deleted spec and by nothing after it. Making the branch unreachable leaves the story green and the whole 2157-test Desktop suite green; the deleted E2E failed on it. Neither has a home outside Electron today: the routing is inline in app-shell, `src/renderer` has no test directory, and opening a seam there is what the architecture ratchet exists to refuse. So they stay, in one test named for the two claims it holds. 11 of 13 moved, not 12. Both mutations confirmed red against it. Generated-by: Claude Code --- .../desktop/e2e/slash-command-compact.spec.ts | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 apps/desktop/e2e/slash-command-compact.spec.ts diff --git a/apps/desktop/e2e/slash-command-compact.spec.ts b/apps/desktop/e2e/slash-command-compact.spec.ts new file mode 100644 index 0000000000..fbd0cf1195 --- /dev/null +++ b/apps/desktop/e2e/slash-command-compact.spec.ts @@ -0,0 +1,108 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * What the slash menu's move to Storybook could not take with it. + * + * The stories cover the menu: which rows a state offers, what picking one + * writes, which slashes are triggers at all. All of that is the composer, and + * none of it needs Electron. Two claims underneath it do, and this is them. + * + * The first is the binding between a shell that has a Session and a menu that + * offers the commands needing one. The stories are handed `hasSession`, so + * `Boolean(activeId)` in app-shell is theirs to assume, not to check — and an + * active Session silently losing `/compact` and `/side` would pass every one + * of them. + * + * The second is that `/compact` compacts. Selecting it only writes an + * invocation into the draft; what happens when that draft is submitted is a + * Host round trip — `sessions.compact()`, a status change, a cleared composer + * — and the thing it must not do is reach the model as an ordinary message. + * That routing lives inline in `app-shell.tsx`, with no seam under it to hang + * a renderer test on, and opening one there is what the architecture ratchet + * exists to refuse. + */ + +import { awaitSendReady, COMPOSER_INPUT, expect, test } from './fixtures'; + +test('a session gets the commands that need one, and /compact compacts it', async ({ + invocableSkillsWindow: page, +}) => { + const composer = page.locator(COMPOSER_INPUT); + await composer.fill('seed session'); + await awaitSendReady(page); + await composer.press('Enter'); + await expect(page.getByText('Fake backend received: seed session')).toBeVisible({ + timeout: 20_000, + }); + + const sessionId = await page.evaluate(async () => (await window.maka.sessions.list())[0]?.id); + expect(sessionId).toBeTruthy(); + await page.evaluate((activeSessionId) => { + const testWindow = window as typeof window & { + __makaObservedCompactCompletion?: boolean; + }; + testWindow.__makaObservedCompactCompletion = false; + window.maka.sessions.subscribeChanges((event) => { + if (event.reason === 'status-change' && event.sessionId === activeSessionId) { + testWindow.__makaObservedCompactCompletion = true; + } + }); + }, sessionId!); + + await composer.click(); + await composer.pressSequentially('/'); + + const menu = page.getByRole('listbox', { name: '命令和技能' }); + const commands = menu.getByRole('group', { name: '命令' }); + // Four, not the two a shell without a Session offers: this is the binding a + // story cannot see, because a story is handed the answer. + await expect(commands.getByRole('option')).toHaveCount(4); + + await commands.getByRole('option', { name: /压缩上下文.*\/compact/ }).click(); + await expect.poll(() => composer.textContent()).toBe('/compact '); + await composer.press('Enter'); + + await expect + .poll(() => + page.evaluate( + () => + (window as typeof window & { __makaObservedCompactCompletion?: boolean }) + .__makaObservedCompactCompletion, + ), + ) + .toBe(true); + await expect.poll(() => composer.textContent()).toBe(''); + await expect(page.getByText('压缩失败')).toHaveCount(0); + + // After the compact completes the composer clears and can remount. `fill()` + // can land before the contentEditable is focused again, so the draft never + // populates and Enter submits nothing — the flake in issue #3289. Type + // through the focused element and require the draft to have settled before + // dispatching. + await composer.click(); + await composer.pressSequentially('after compact'); + await expect.poll(() => composer.textContent()).toBe('after compact'); + await awaitSendReady(page); + await composer.press('Enter'); + await expect(page.getByText('Fake backend received: after compact')).toBeVisible({ + timeout: 20_000, + }); + await expect(page.getByText('Fake backend received: /compact')).toHaveCount(0); +}); From c47e4159495dee7ee5d77c928773ce4acc4df1a7 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 5 Sep 2026 00:07:23 +0800 Subject: [PATCH 11/12] fix(ui): size the reader test from what a browser actually does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule that tells a reader's scroll from content moving under them carried a 2px tolerance and a running total of what the content could not explain. Neither was measured. `packages/ui` runs on a fake DOM of integers, so no test there could reach either one, and an ablation passed whatever the numbers were. Measured in a real layout engine, on fractional heights, through both paths that move the offset without the reader: native anchoring misses its own band by exactly one pixel on about a quarter of the frames it compensates, and clamping never misses. So the tolerance is one pixel rather than two, and the running total has to go — the error is bounded per event but one-directional across a stream, so accumulating it turns a pixel of arithmetic into a drift that crosses any threshold and hands a streaming reader back to the tail. The story holds the measurement rather than the conclusion: shrink the constant and the misses are reported as gestures, widen it and a two pixel gesture goes unheard. Generated-by: Claude Code --- .../ui/src/transcript-scroll-authority.tsx | 36 ++-- .../transcript-scroll-rounding.stories.tsx | 181 ++++++++++++++++++ 2 files changed, 198 insertions(+), 19 deletions(-) create mode 100644 packages/ui/stories/transcript-scroll-rounding.stories.tsx diff --git a/packages/ui/src/transcript-scroll-authority.tsx b/packages/ui/src/transcript-scroll-authority.tsx index 39f189a256..418c1f239f 100644 --- a/packages/ui/src/transcript-scroll-authority.tsx +++ b/packages/ui/src/transcript-scroll-authority.tsx @@ -54,13 +54,23 @@ import { ChatLayoutScrollButton } from '@astryxdesign/core/Chat'; const PIN_THRESHOLD_PX = 10; const BUTTON_THRESHOLD_PX = 100; /** - * How far the offset may miss what the content accounts for and still be the - * content. `scrollHeight` and `clientHeight` are integers while `scrollTop` is - * not, so the arithmetic below is exact to under a pixel — this is that - * rounding, not a gesture the reader might have made. The pin threshold is a - * distance from the tail and says nothing about it. + * How far one event's offset may miss what the content accounts for and still + * be the content. + * + * Native anchoring moves the offset by what it inserted, but it rounds the + * anchor's old and new positions separately, while the range it is measured + * against is `scrollHeight - clientHeight` — two more integers, each rounded on + * its own. Growth above the reader is measured landing a whole pixel outside + * the band on about a quarter of its frames, which is what this absorbs; a + * story in `packages/ui/stories` holds that measurement, so this is a number + * from a browser rather than a guess, and it goes red if the browser stops + * agreeing. + * + * It is spent per event and never carried. The error is bounded per event but + * one-directional across a stream, so keeping a running total would turn a + * pixel of arithmetic into a drift that eventually crosses any threshold. */ -const GEOMETRY_ROUNDING_PX = 2; +const GEOMETRY_ROUNDING_PX = 1; export interface TranscriptScrollSnapshot { /** Following the tail: growth writes `scrollTop`. */ @@ -120,8 +130,6 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { let lastClientHeight = 0; /** The offset the last event saw, to measure the next one's move against. */ let lastScrollTop = 0; - /** Offset movement no content change accounts for, since the last reading. */ - let unexplained = 0; let snapshot: TranscriptScrollSnapshot = { pinned, awayFromTail }; const listeners = new Set<() => void>(); const readerListeners = new Set<() => void>(); @@ -144,7 +152,6 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { lastScrollHeight = root.scrollHeight; lastClientHeight = root.clientHeight; lastScrollTop = root.scrollTop; - unexplained = 0; awayFromTail = false; publish(); }; @@ -165,7 +172,6 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; lastScrollTop = target.scrollTop; - unexplained = 0; return; } // Content moves the offset too, and only ever by how much the end of @@ -192,13 +198,7 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { const explainedLow = Math.min(0, contentDelta); const explainedHigh = Math.max(0, contentDelta); const topDelta = target.scrollTop - lastScrollTop; - // What the content could not account for, kept across events rather - // than judged one at a time. A reader crossing the transcript slowly - // produces a run of moves each smaller than the rounding this has to - // tolerate, and re-baselining on every event would spend every one of - // them: they would never add up to a reader, and the answer would go - // on writing the view out from under a reader who was plainly moving. - unexplained += topDelta - Math.min(explainedHigh, Math.max(explainedLow, topDelta)); + const unexplained = topDelta - Math.min(explainedHigh, Math.max(explainedLow, topDelta)); const readerMoved = Math.abs(unexplained) > GEOMETRY_ROUNDING_PX; lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; @@ -209,7 +209,6 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { publish(); return; } - unexplained = 0; pinned = distance <= PIN_THRESHOLD_PX; publish(); for (const listener of [...readerListeners]) listener(); @@ -217,7 +216,6 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; lastScrollTop = target.scrollTop; - unexplained = 0; target.addEventListener('scroll', onScroll, { passive: true }); // Everything that moves the tail without the reader asking, watched in // one place: the scroller's own box, because the tail also moves when the diff --git a/packages/ui/stories/transcript-scroll-rounding.stories.tsx b/packages/ui/stories/transcript-scroll-rounding.stories.tsx new file mode 100644 index 0000000000..bbbdc148de --- /dev/null +++ b/packages/ui/stories/transcript-scroll-rounding.stories.tsx @@ -0,0 +1,181 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * The one claim in `TranscriptScrollAuthority` a fake DOM cannot make. + * + * It decides the reader moved by measuring the offset against what the content + * accounts for, and it reads those from two different number systems: CSSOM + * gives `scrollTop` as a double and `scrollHeight` / `clientHeight` as longs. + * So the comparison holds an exact number against rounded ones, and the + * `packages/ui` suite — a fake DOM of integers — cannot put a fraction into it. + * + * It is measured here instead, in a real layout engine on fractional heights, + * through both paths that move the offset without the reader: native anchoring + * compensating content above them, and the browser clamping the offset when + * the transcript ends before it. Clamping lands exact. Anchoring misses by a + * whole pixel on about a quarter of its frames, which is where + * `GEOMETRY_ROUNDING_PX` comes from — it is that measurement, and this is the + * thing that holds it. Shrink the constant and the misses below are read as + * gestures; widen it and the deliberate one at the end goes unheard. + * + * It asks the authority directly rather than reading a scroll position. A + * misclassification while the reader is at the tail re-derives the same pin and + * moves nothing, so position is exactly the observable that cannot see this; + * `subscribeToReaderScroll` is the module's own answer to "was that the + * reader", which is the question. + */ + +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect } from 'storybook/test'; +import { createTranscriptScrollAuthority } from '../src/transcript-scroll-authority.js'; + +const SCROLLER_ID = 'rounding-probe-scroller'; + +function Scroller() { + return ( +
+
+
+ anchor +
+
+
+ ); +} + +const meta = { + title: 'Product/Transcript Scroll Rounding', + component: Scroller, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +function scroller(): HTMLElement { + const root = document.getElementById(SCROLLER_ID); + if (!root) throw new Error('the probe scroller is missing'); + return root; +} + +function settled(): Promise { + return new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); +} + +// Real path: none — this is a probe, and it says so. The surface it guards is +// every streaming transcript; what it needs from a browser is fractional box +// heights and CSSOM's two number systems, which no product state adds to it. +export const ContentThatOnlyRoundsIsNotTheReader: Story = { + play: async () => { + const root = scroller(); + const above = root.querySelector('[data-probe="above"]'); + const below = root.querySelector('[data-probe="below"]'); + if (!above || !below) throw new Error('the probe spacers are missing'); + + const authority = createTranscriptScrollAuthority(); + const detach = authority.attach(root); + try { + // Released, so the authority writes nothing and every offset change + // below is the browser's alone. Far enough off the tail that a misread + // cannot be laundered by re-deriving the same pinned answer, but close + // enough that content leaving from underneath reaches them in a few + // steps. + authority.releasePin(); + root.scrollTop = root.scrollHeight - root.clientHeight - 30; + await settled(); + + let readerMoves = 0; + authority.subscribeToReaderScroll(() => { + readerMoves += 1; + }); + + // Fractional content change, crossing a rounding boundary on every step, + // through both paths that move the offset without the reader. Growth + // above them is answered by native anchoring, which pushes the offset + // down by what it inserted. Content taken from under them eventually + // ends the transcript in front of the offset, and the browser pulls the + // offset back to that end — the end it holds itself, while the authority + // reads it as `scrollHeight - clientHeight`, two integers each rounded + // on its own. + const escapes: number[] = []; + const record = async (change: () => void): Promise => { + const beforeTop = root.scrollTop; + const beforeRange = root.scrollHeight - root.clientHeight; + change(); + await settled(); + const contentDelta = root.scrollHeight - root.clientHeight - beforeRange; + const topDelta = root.scrollTop - beforeTop; + const low = Math.min(0, contentDelta); + const high = Math.max(0, contentDelta); + escapes.push(topDelta - Math.min(high, Math.max(low, topDelta))); + }; + + let aboveHeight = 400.5; + for (let step = 0; step < 40; step += 1) { + await record(() => { + aboveHeight += 7.3; + above.style.height = `${aboveHeight}px`; + }); + } + let belowHeight = 900.5; + for (let step = 0; step < 40; step += 1) { + await record(() => { + belowHeight -= 7.3; + below.style.height = `${belowHeight}px`; + }); + } + + // What the arithmetic missed by, which is what the constant has to + // cover. Recorded rather than merely tolerated: if a browser starts + // missing by more, the number in the module is stale and this says so + // here, instead of the transcript quietly deciding a streaming answer's + // reader had reached for the scrollbar. + const worst = Math.max(...escapes.map(Math.abs)); + await expect( + worst, + `content moved the offset further outside its own band than the module allows; escapes ${escapes + .map((value) => value.toExponential(2)) + .join(' ')}`, + ).toBeLessThanOrEqual(1); + + // And none of it was read as the reader, who touched nothing at all. + await expect( + readerMoves, + `content the browser moved under the reader was read as a gesture; escapes ${escapes + .map((value) => value.toExponential(2)) + .join(' ')}`, + ).toBe(0); + + // And the run is only worth anything if this classifier still says yes + // to a reader. Two pixels: the smallest move that has to survive a + // tolerance sized to absorb one, so a constant that grew would be caught + // here rather than by a reader losing their place mid-answer. + root.scrollTop -= 2; + await settled(); + await expect(readerMoves, 'a real gesture went unheard').toBe(1); + } finally { + detach(); + } + }, +}; From 91da7980b31ab8d1699e727cf05c6822cf06ad27 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Sat, 5 Sep 2026 00:32:31 +0800 Subject: [PATCH 12/12] fix(ui): spend the rounding slack only where rounding happened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught the tolerance being cut to one pixel: a real transcript's content steps miss their own band by more than the probe scroller's do, so growth was read as the reader and the view stopped following the tail mid-answer. Two pixels is what the band actually needs, and the reason it looked too wide is that it was being spent everywhere. An event that finds the content unchanged has no rounded reading in it — the band is a point, and the offset either moved or it did not. So the slack belongs to events the content moved, and a reader inching down a settled transcript is heard exactly rather than swallowed by arithmetic that did not happen to them. Generated-by: Claude Code --- .../ui/src/transcript-scroll-authority.tsx | 33 ++++++++++--------- .../transcript-scroll-rounding.stories.tsx | 24 ++++++++------ 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/packages/ui/src/transcript-scroll-authority.tsx b/packages/ui/src/transcript-scroll-authority.tsx index 418c1f239f..f4de10b70c 100644 --- a/packages/ui/src/transcript-scroll-authority.tsx +++ b/packages/ui/src/transcript-scroll-authority.tsx @@ -54,23 +54,25 @@ import { ChatLayoutScrollButton } from '@astryxdesign/core/Chat'; const PIN_THRESHOLD_PX = 10; const BUTTON_THRESHOLD_PX = 100; /** - * How far one event's offset may miss what the content accounts for and still - * be the content. + * How far an offset may miss what the content accounts for and still be the + * content. * - * Native anchoring moves the offset by what it inserted, but it rounds the - * anchor's old and new positions separately, while the range it is measured - * against is `scrollHeight - clientHeight` — two more integers, each rounded on - * its own. Growth above the reader is measured landing a whole pixel outside - * the band on about a quarter of its frames, which is what this absorbs; a - * story in `packages/ui/stories` holds that measurement, so this is a number - * from a browser rather than a guess, and it goes red if the browser stops - * agreeing. + * The band below holds an exact `scrollTop` against a range built from two + * rounded integers, and native anchoring rounds the anchor's own positions + * separately again, so a step that is entirely the content still lands a pixel + * or two outside its own band. A story in `packages/ui/stories` measures that + * against a real layout engine and goes red if a browser starts missing by + * more. * - * It is spent per event and never carried. The error is bounded per event but - * one-directional across a stream, so keeping a running total would turn a - * pixel of arithmetic into a drift that eventually crosses any threshold. + * It is spent only where that arithmetic happened. An event that finds the + * content unchanged has nothing rounded in it: the band is a point, the offset + * either moved or did not, and a reader inching down a settled transcript is + * heard exactly. Spending it on those events instead is what would make a slow + * reader unhearable, and no accumulator can buy that back — the error is + * bounded per event but one-directional across a stream, so a running total + * turns a pixel of arithmetic into a drift that crosses any threshold. */ -const GEOMETRY_ROUNDING_PX = 1; +const GEOMETRY_ROUNDING_PX = 2; export interface TranscriptScrollSnapshot { /** Following the tail: growth writes `scrollTop`. */ @@ -199,7 +201,8 @@ export function createTranscriptScrollAuthority(): TranscriptScrollAuthority { const explainedHigh = Math.max(0, contentDelta); const topDelta = target.scrollTop - lastScrollTop; const unexplained = topDelta - Math.min(explainedHigh, Math.max(explainedLow, topDelta)); - const readerMoved = Math.abs(unexplained) > GEOMETRY_ROUNDING_PX; + const slack = contentDelta === 0 ? 0 : GEOMETRY_ROUNDING_PX; + const readerMoved = Math.abs(unexplained) > slack; lastScrollHeight = target.scrollHeight; lastClientHeight = target.clientHeight; lastScrollTop = target.scrollTop; diff --git a/packages/ui/stories/transcript-scroll-rounding.stories.tsx b/packages/ui/stories/transcript-scroll-rounding.stories.tsx index bbbdc148de..2192ff88df 100644 --- a/packages/ui/stories/transcript-scroll-rounding.stories.tsx +++ b/packages/ui/stories/transcript-scroll-rounding.stories.tsx @@ -29,11 +29,14 @@ * It is measured here instead, in a real layout engine on fractional heights, * through both paths that move the offset without the reader: native anchoring * compensating content above them, and the browser clamping the offset when - * the transcript ends before it. Clamping lands exact. Anchoring misses by a - * whole pixel on about a quarter of its frames, which is where - * `GEOMETRY_ROUNDING_PX` comes from — it is that measurement, and this is the - * thing that holds it. Shrink the constant and the misses below are read as - * gestures; widen it and the deliberate one at the end goes unheard. + * the transcript ends before it. Clamping lands exact. Anchoring misses, which + * is where `GEOMETRY_ROUNDING_PX` comes from — it is that measurement, and + * this is the thing that holds it. + * + * And it holds the other half of the rule, which is that the slack is spent + * only on events the content actually moved. A settled transcript rounds + * nothing, so a reader inching down one is heard exactly; that is the last + * phase, and it is what a slack applied unconditionally would swallow. * * It asks the authority directly rather than reading a scroll position. A * misclassification while the reader is at the tail re-derives the same pin and @@ -157,7 +160,7 @@ export const ContentThatOnlyRoundsIsNotTheReader: Story = { `content moved the offset further outside its own band than the module allows; escapes ${escapes .map((value) => value.toExponential(2)) .join(' ')}`, - ).toBeLessThanOrEqual(1); + ).toBeLessThanOrEqual(2); // And none of it was read as the reader, who touched nothing at all. await expect( @@ -168,12 +171,13 @@ export const ContentThatOnlyRoundsIsNotTheReader: Story = { ).toBe(0); // And the run is only worth anything if this classifier still says yes - // to a reader. Two pixels: the smallest move that has to survive a - // tolerance sized to absorb one, so a constant that grew would be caught - // here rather than by a reader losing their place mid-answer. + // to a reader. Two pixels, on content that just settled: no arithmetic + // happened, so nothing here is owed any slack, and a version that spent + // it anyway would lose this reader — the same reader, moving the same + // way, that a streaming transcript has to keep. root.scrollTop -= 2; await settled(); - await expect(readerMoves, 'a real gesture went unheard').toBe(1); + await expect(readerMoves, 'a reader on settled content went unheard').toBe(1); } finally { detach(); }