Skip to content

Commit 7137cf2

Browse files
authored
fix(desktop): land prompt-rail jumps at the top and fence the e2e sends (#4577)
Desktop e2e sends failed intermittently with `element(s) not found` right after Enter. The specs pressed Enter on an intermediate signal — a mounted composer, a cleared draft, an updated model label — none of which means the shell will accept a submission; moving Desktop e2e to 4 workers on one runner cut each worker's CPU share and removed the margin that had been hiding it. One exported `awaitSendReady` now waits on the single signal that covers the whole admission chain (发送 enabled), and the assertions that observe the resulting Turn carry an explicit 20s. Fencing those sends left three failures that were not sends, all chased to their cause here rather than deferred. The product one: clicking a prompt-rail tick for an unloaded prompt aims that turn at the top of the scrollport, while the shell answers the same click's load request by publishing the search reveal, which centres the turn with the app's scroll motion. Two writers, two answers, and the animated one walked the prompt 68px back off the top. The reveal is not redundant — it also records the reading position a session switch restores from — so only its alignment conflicts. `ChatView`, which both aims the turn and consumes the reveal, reconciles them: a reveal for the turn the rail is holding is instant and top-aligned, every other one stays centred and animated. The rail's claim belongs to the one navigation its click asked for, so it binds to the first target that arrives for that turn and is spent on anything else — a later search for the same turn keeps its own contract. The other two are test-side. `promptRailWorker` and `promptRailMotionWindow` named no locale, so their renderers took the host's: Chinese on a developer's desktop, English on the CI runner, and every label-addressed control a coin flip. And the skip-link walk re-parks and retries, because the composer's one-shot draft-caret restore uses `getSelection().addRange(...)`, which focuses a `contenteditable` and so takes focus once per cold start with no `focus()` call to fence on. That focus theft is a real accessibility defect on main; it is fixed separately, and that change removes this retry as part of its own diff. No e2e test is added — the diff under `apps/desktop/e2e` adds zero `test(` blocks and only changes what existing tests wait on. An earlier revision also made the shell block sends until the readiness probe resolved. CI disproved it: `pending` reaches the composer through `sendBlocked`, and `sendCurrent` discards a submission on `sendBlocked` rather than waiting, so the gate silently dropped an Enter pressed during the probe window instead of letting it through. Giving a dropped submission feedback comes first (#4573, item 3), then the button can tighten. No migration or compatibility impact. Closes #4573 Generated-by: Claude Code
1 parent 8ea3c4f commit 7137cf2

8 files changed

Lines changed: 238 additions & 26 deletions

File tree

.github/workflows/ci.yml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,18 @@ jobs:
368368
npm exec -w @maka/desktop -- playwright test \
369369
--config e2e/playwright.config.ts --workers="$worker_count"
370370
371+
# Playwright keeps a trace, a video and a screenshot for every failed
372+
# test. Without this they die with the runner, and an e2e flake can only
373+
# be diagnosed by reproducing it.
374+
- name: Upload Desktop e2e results
375+
if: failure() && steps.plan.outputs.e2e == 'true'
376+
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
377+
with:
378+
name: desktop-e2e-results
379+
path: apps/desktop/e2e/test-results/
380+
if-no-files-found: ignore
381+
retention-days: 7
382+
371383
- name: Browser WebContentsView semantic smoke
372384
if: steps.plan.outputs.e2e == 'true'
373385
# Hosted Linux runners cannot configure Electron's SUID helper. This

apps/desktop/e2e/accessibility-coverage.spec.ts

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
import { FAKE_HOLD_OPEN_PROMPT } from '@maka/runtime/test-only/fake-backend';
2121
import type { CDPSession, Locator, Page } from '@playwright/test';
22-
import { expect, test, COMPOSER_INPUT } from './fixtures';
22+
import { awaitSendReady, expect, test, COMPOSER_INPUT } from './fixtures';
2323
import { auditAxTree } from '../../../scripts/ax-tree-audit.mjs';
2424
import { groupedNav } from '../src/renderer/settings/settings-nav';
2525

@@ -52,13 +52,27 @@ async function tabTo(page: Page, target: Locator, label: string, limit = 30): Pr
5252
).toBe(true);
5353
}
5454

55+
/**
56+
* Walk to the skip link from the document start, taking the start back if a
57+
* cold start moves it.
58+
*
59+
* Parking focus on `body` is not a one-shot the renderer respects: the composer
60+
* restores its draft caret with `getSelection().addRange(...)`, and a range set
61+
* inside a `contenteditable` focuses it — so once per cold start, tens of
62+
* milliseconds after the park and with no `focus()` call to fence on, focus
63+
* lands in the composer. A walk that starts there has to run out the tab ring
64+
* and wrap around, which is over budget. The restore fires once, so re-park and
65+
* walk again rather than widening the budget — the budget is the assertion.
66+
*/
5567
async function enterMainFromSkipLink(page: Page): Promise<void> {
56-
await page.evaluate(() => {
57-
document.body.tabIndex = -1;
58-
document.body.focus();
59-
});
6068
const skipLink = page.getByRole('link', { name: '跳到主要内容' });
61-
await tabTo(page, skipLink, 'skip link', 10);
69+
await expect(async () => {
70+
await page.evaluate(() => {
71+
document.body.tabIndex = -1;
72+
document.body.focus();
73+
});
74+
await tabTo(page, skipLink, 'skip link', 10);
75+
}).toPass({ timeout: 30_000 });
6276
await page.keyboard.press('Enter');
6377
await expect(page.getByRole('main')).toBeFocused();
6478
await page.evaluate(() => document.body.removeAttribute('tabindex'));
@@ -183,6 +197,7 @@ test('data-backed conversation exposes ordered todos and keyboard access to tool
183197
await page.keyboard.insertText('/graph on');
184198
const send = page.getByRole('button', { name: '发送' });
185199
await tabTo(page, send, 'Send button', 20);
200+
await awaitSendReady(page);
186201
await page.keyboard.press('Enter');
187202
await expect(page.getByText('Graph Mode 已开启', { exact: true })).toBeVisible();
188203
await assertAxHealth(cdp, 'overlay/graph-mode-toast');
@@ -203,6 +218,7 @@ test('toast and error states expose healthy live regions', async ({ window: page
203218
const cdp = await page.context().newCDPSession(page);
204219
const composer = page.locator(COMPOSER_INPUT);
205220
await composer.fill('/graph history');
221+
await awaitSendReady(page);
206222
await composer.press('Enter');
207223
await expect(page.getByText('Graph 历史', { exact: true })).toBeVisible();
208224
await assertAxHealth(cdp, 'overlay/graph-history-toast');
@@ -224,10 +240,17 @@ test('a streaming answer exposes a healthy live conversation state', async ({ wi
224240
await tabTo(page, composer, 'streaming composer', 60);
225241
await page.keyboard.insertText(FAKE_HOLD_OPEN_PROMPT);
226242
const send = page.getByRole('button', { name: '发送' });
243+
// After the Tab walk, not before it: a tooltip-carrying Astryx Button is
244+
// disabled via `aria-disabled`, so it stays focusable and `tabTo` would
245+
// reach it either way.
227246
await tabTo(page, send, 'streaming Send button', 20);
247+
await awaitSendReady(page);
228248
await page.keyboard.press('Enter');
229249

230-
await expect(page.locator('.maka-bubble-streaming')).toContainText('Fake backend waiting');
250+
await expect(page.locator('.maka-bubble-streaming')).toContainText(
251+
'Fake backend waiting',
252+
{ timeout: 20_000 },
253+
);
231254
await expect(page.getByRole('button', { name: '停止' })).toBeEnabled();
232255
await assertAxHealth(cdp, 'conversation/streaming');
233256

@@ -254,8 +277,8 @@ test('composer and workbar entry points expose named actionable controls', async
254277
await tabTo(page, composer, 'new-task composer', 60);
255278
await page.keyboard.insertText(prompt);
256279
const send = page.getByRole('button', { name: '发送' });
257-
await expect(send).toBeEnabled();
258280
await tabTo(page, send, 'new-task Send button', 20);
281+
await awaitSendReady(page);
259282
await page.keyboard.press('Enter');
260283
await expect(page.getByText(`Fake backend received: ${prompt}`)).toBeVisible({
261284
timeout: 30_000,

apps/desktop/e2e/fixtures.ts

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,31 @@ export async function ensureSidebarExpanded(page: Page): Promise<void> {
7171
).toBeVisible();
7272
}
7373

74+
/**
75+
* Wait until the composer is in a state where Enter is a real submission: the
76+
* connections projection has produced at least one connection, the draft is
77+
* non-empty, no known blocker is showing and no earlier send is still in
78+
* flight. 发送 is disabled for all of that, so it is the one signal covering
79+
* it; intermediate signals (a cleared draft, an updated model label) resolve
80+
* earlier and mean nothing here.
81+
*
82+
* It does NOT cover the submission-readiness probe: an unresolved snapshot is
83+
* not a hard block, so the button is enabled while the probe is in flight, and
84+
* `send()` awaits the probe again on its own — a first send inside a barrier
85+
* that gives up at 30s. The post-send assertions wait 20s, which covers every
86+
* admission measured here but not that whole barrier. Widening them past it
87+
* buys nothing: the 60s test budget is the real cap, a send admitted at 25s
88+
* leaves the multi-send specs unable to finish anyway, and the only change
89+
* would be trading a named assertion failure for a bare test timeout. A probe
90+
* that comes back blocked still drops the send with no feedback, which is a
91+
* product gap, not something a test-side fence can close.
92+
*/
93+
export async function awaitSendReady(page: Page): Promise<void> {
94+
await expect(page.getByRole('button', { name: '发送' })).toBeEnabled({
95+
timeout: 20_000,
96+
});
97+
}
98+
7499
/**
75100
* Wait for the default Host's Coordination Session and the WorkHub projection
76101
* to agree that the surface is ready. A mounted WorkHub main is not sufficient:
@@ -683,9 +708,9 @@ export const test = base.extend<E2eTestFixtures, E2eWorkerFixtures>({
683708
use,
684709
);
685710
},
686-
// This scenario is read-only at the Host boundary. Keep its real Electron +
687-
// Host composition warm for the worker, while the test-scoped wrapper below
688-
// restores Host and renderer state between tests.
711+
// Keep this scenario's real Electron + Host composition warm for the worker,
712+
// while the test-scoped wrapper below restores Host and renderer state
713+
// between tests. Tests on it may run a Turn, so the reset is not read-only.
689714
promptRailWorker: [async ({}, use) => {
690715
await withE2eWindow({
691716
seed: false,
@@ -695,6 +720,10 @@ export const test = base.extend<E2eTestFixtures, E2eWorkerFixtures>({
695720
// assertion that names it.
696721
readinessSelector: '[data-turn-id]',
697722
e2eFixtureScenario: 'chat-prompt-rail',
723+
// Every other fixture window names its locale; without one the renderer
724+
// takes the host's, so any test that reaches a control by its label
725+
// passes on a Chinese desktop and cannot find it on an English CI runner.
726+
locale: 'zh',
698727
showWindow: true,
699728
}, async (page, { app }) => {
700729
const viewport = await page.evaluate(() => ({ width: innerWidth, height: innerHeight }));
@@ -730,8 +759,17 @@ export const test = base.extend<E2eTestFixtures, E2eWorkerFixtures>({
730759
promptRailMotionWindow: async ({}, use) => {
731760
await withE2eWindow({
732761
seed: false,
733-
readinessSelector: '[data-turn-id]',
762+
// The transcript and the fixture attributes arrive on two unordered
763+
// async paths: `runDeferredStartupRefreshes` fires `refreshSessions()`
764+
// and `applyE2eFixture()` side by side, and only the second one — after
765+
// its `e2eFixture.getState()` IPC resolves — writes
766+
// `data-maka-scroll-motion`. A turn can therefore paint while the
767+
// document still says nothing about scroll motion. Requiring both in one
768+
// selector is what makes "this window scrolls smoothly" true by the time
769+
// a test body reads it.
770+
readinessSelector: 'html[data-maka-scroll-motion="smooth"] [data-turn-id]',
734771
e2eFixtureScenario: 'chat-prompt-rail',
772+
locale: 'zh',
735773
showWindow: true,
736774
scrollMotion: 'smooth',
737775
}, use);

apps/desktop/e2e/streaming-remount.spec.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,13 @@ import {
2323
FAKE_WAIT_FOR_STEERING_LARGE_RESPONSE_PROMPT,
2424
} from '@maka/runtime/test-only/fake-backend';
2525
import type { Locator } from '@playwright/test';
26-
import { COMPOSER_INPUT, ensureSidebarExpanded, expect, test } from './fixtures';
26+
import {
27+
awaitSendReady,
28+
COMPOSER_INPUT,
29+
ensureSidebarExpanded,
30+
expect,
31+
test,
32+
} from './fixtures';
2733

2834
interface SessionObservationLatchWindow extends Window {
2935
/** E2E-only preload affordance; see the MAKA_E2E block in preload.ts. */
@@ -54,10 +60,12 @@ test('a failed first observation seed reconnects to the live Turn', async ({ win
5460

5561
const composer = page.locator(COMPOSER_INPUT);
5662
await composer.fill(FAKE_HOLD_OPEN_PROMPT);
63+
await awaitSendReady(page);
5764
await composer.press('Enter');
5865

5966
await expect(page.locator('.maka-bubble-streaming')).toContainText(
6067
'Fake backend waiting',
68+
{ timeout: 20_000 },
6169
);
6270
await page.getByRole('button', { name: '停止' }).click();
6371
await expect(page.getByRole('button', { name: '重新生成' })).toHaveCount(1, {
@@ -73,11 +81,12 @@ test('remounting a live surface leaves accumulated output settled', async ({
7381

7482
const composer = page.locator(COMPOSER_INPUT);
7583
await composer.fill(FAKE_HOLD_OPEN_REWRITE_PROMPT);
84+
await awaitSendReady(page);
7685
await composer.press('Enter');
7786

7887
const accumulatedOutput = 'prefix sk-123456789012345';
7988
const liveBubble = page.locator('.maka-bubble-streaming');
80-
await expect(liveBubble).toContainText(accumulatedOutput);
89+
await expect(liveBubble).toContainText(accumulatedOutput, { timeout: 20_000 });
8190

8291
const sidebar = page.getByRole('navigation', { name: '任务列表' });
8392
await ensureSidebarExpanded(page);
@@ -137,8 +146,12 @@ test('keeps a completed reply after an interrupted turn and conversation remount
137146
expect(await page.evaluate(() => matchMedia('(prefers-reduced-motion: reduce)').matches)).toBe(false);
138147
const composer = page.locator(COMPOSER_INPUT);
139148
await composer.fill('temporary conversation');
149+
await awaitSendReady(page);
140150
await composer.press('Enter');
141-
await expect(page.getByRole('log')).toContainText('Fake backend received: temporary conversation');
151+
await expect(page.getByRole('log')).toContainText(
152+
'Fake backend received: temporary conversation',
153+
{ timeout: 20_000 },
154+
);
142155
await expect(page.getByRole('button', { name: '重新生成' })).toHaveCount(1, {
143156
timeout: 20_000,
144157
});
@@ -157,9 +170,11 @@ test('keeps a completed reply after an interrupted turn and conversation remount
157170
await expect(composer).toHaveText('');
158171

159172
await composer.fill(FAKE_HOLD_OPEN_PROMPT);
173+
await awaitSendReady(page);
160174
await composer.press('Enter');
161175
await expect(page.locator('.maka-bubble-streaming')).toContainText(
162176
'Fake backend waiting',
177+
{ timeout: 20_000 },
163178
);
164179
const originalSessionId = await sidebar
165180
.locator('[data-session-id]:has([aria-current="page"])')
@@ -178,9 +193,7 @@ test('keeps a completed reply after an interrupted turn and conversation remount
178193
{ timeout: 20_000 },
179194
).toBe(0);
180195
await composer.fill(FAKE_WAIT_FOR_STEERING_LARGE_RESPONSE_PROMPT);
181-
await expect(page.getByRole('button', { name: '发送' })).toBeEnabled({
182-
timeout: 20_000,
183-
});
196+
await awaitSendReady(page);
184197
await composer.press('Enter');
185198
await expect(page.locator('.maka-user-message', {
186199
hasText: FAKE_WAIT_FOR_STEERING_LARGE_RESPONSE_PROMPT,
@@ -220,11 +233,12 @@ test('returning to a live conversation settles output accumulated while away', a
220233
expect(await page.evaluate(() => matchMedia('(prefers-reduced-motion: reduce)').matches)).toBe(false);
221234
const composer = page.locator(COMPOSER_INPUT);
222235
await composer.fill(FAKE_HOLD_OPEN_PROMPT);
236+
await awaitSendReady(page);
223237
await composer.press('Enter');
224238

225239
const accumulatedOutput = 'Fake backend waiting for the test to stop the Turn.';
226240
const liveBubble = page.locator('.maka-bubble-streaming');
227-
await expect(liveBubble).toContainText(accumulatedOutput);
241+
await expect(liveBubble).toContainText(accumulatedOutput, { timeout: 20_000 });
228242

229243
const sidebar = page.getByRole('navigation', { name: '任务列表' });
230244
await page.getByRole('button', { name: '展开侧边栏' }).click();
@@ -239,9 +253,11 @@ test('returning to a live conversation settles output accumulated while away', a
239253
await sidebar.getByRole('button', { name: '新任务', exact: true }).click();
240254
await expect(composer).toHaveText('');
241255
await composer.fill('temporary second conversation');
256+
await awaitSendReady(page);
242257
await composer.press('Enter');
243258
await expect(page.getByRole('log')).toContainText(
244259
'Fake backend received: temporary second conversation',
260+
{ timeout: 20_000 },
245261
);
246262
await expect(page.getByRole('button', { name: '重新生成' })).toHaveCount(1, {
247263
timeout: 20_000,

apps/desktop/e2e/transcript-scroll.spec.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@
1717
* under the License.
1818
*/
1919

20-
import { expect, test, COMPOSER_INPUT, ensureSidebarExpanded } from './fixtures';
20+
import {
21+
awaitSendReady,
22+
expect,
23+
test,
24+
COMPOSER_INPUT,
25+
ensureSidebarExpanded,
26+
} from './fixtures';
2127
import type { Page } from '@playwright/test';
2228

2329
/**
@@ -196,6 +202,8 @@ function measureTailLag(page: Page, frames: number): Promise<{
196202
async function sendPrompt(page: Page, text: string): Promise<void> {
197203
const composer = page.locator(COMPOSER_INPUT);
198204
await composer.fill(text);
205+
// Switching Session or model restarts asynchronous send admission.
206+
await awaitSendReady(page);
199207
await composer.press('Enter');
200208
}
201209

@@ -313,7 +321,9 @@ test('switching Sessions restores a Turn anchor while a tail Session follows bac
313321
.__makaBackgroundTailProbe = state;
314322
}, tailSessionId);
315323
await sendPrompt(page, LONG_PROMPT);
316-
await expect(page.locator('.maka-user-message', { hasText: '第 1 行' })).toBeVisible();
324+
await expect(page.locator('.maka-user-message', { hasText: '第 1 行' })).toBeVisible({
325+
timeout: 20_000,
326+
});
317327

318328
// The transcript collapses before each async replacement. This round trip
319329
// therefore exercises the production ordering that made a saved scrollTop
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
import assert from 'node:assert/strict';
21+
import { test } from 'node:test';
22+
import { resolveRailAlignedTarget } from '../chat-view.js';
23+
24+
test('a rail claim aims its own navigation and nothing after it', () => {
25+
// The click, before the shell has published anything.
26+
let claim = resolveRailAlignedTarget({ turnId: 'a' }, undefined).claim;
27+
assert.deepEqual(claim, { turnId: 'a' });
28+
29+
// The load the click asked for. The reveal has to agree with the rail.
30+
let resolved = resolveRailAlignedTarget(claim, { turnId: 'a', nonce: 1 });
31+
assert.equal(resolved.target?.align, 'start');
32+
claim = resolved.claim;
33+
34+
// Still the same command, re-rendered while the loaded range settles.
35+
resolved = resolveRailAlignedTarget(claim, { turnId: 'a', nonce: 1 });
36+
assert.equal(resolved.target?.align, 'start');
37+
claim = resolved.claim;
38+
39+
// A later search for the same Turn is a different command, and wants the
40+
// search contract back.
41+
resolved = resolveRailAlignedTarget(claim, { turnId: 'a', nonce: 2 });
42+
assert.equal(resolved.target?.align, 'center');
43+
assert.equal(resolved.claim, undefined);
44+
});
45+
46+
test('a search for another Turn spends an unconsumed rail claim', () => {
47+
const resolved = resolveRailAlignedTarget({ turnId: 'a' }, { turnId: 'b', nonce: 1 });
48+
assert.equal(resolved.target?.align, 'center');
49+
assert.equal(resolved.claim, undefined);
50+
});
51+
52+
test('a search with no rail claim behind it is centred', () => {
53+
const resolved = resolveRailAlignedTarget(undefined, { turnId: 'a', nonce: 1 });
54+
assert.equal(resolved.target?.align, 'center');
55+
assert.deepEqual(resolved.target, { turnId: 'a', nonce: 1, align: 'center' });
56+
});

0 commit comments

Comments
 (0)