Skip to content

Commit 70cf363

Browse files
wenshaoyiliang114
andauthored
feat(web-shell): Improve split-view session navigation (#11250)
* feat(web-shell): Improve split-view session navigation * fix(ci): Run Web Shell browser checks on hosted runners * fix(web-shell): Keep split navigation outside approval shortcuts * fix(web-shell): Address split-view review regressions * fix(web-shell): Stabilize split approval reports * fix(web-shell): Preserve history anchors during slow rendering --------- Co-authored-by: 易良 <[email protected]>
1 parent b2e4db8 commit 70cf363

23 files changed

Lines changed: 1319 additions & 88 deletions

‎.github/workflows/ci.yml‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,23 +1304,18 @@ jobs:
13041304
needs.classify_pr.outputs.skip_ci != 'true' &&
13051305
needs.test.outputs.ci_profile == 'full'
13061306
}}
1307-
runs-on: '${{ fromJSON(needs.classify_pr.outputs.ubuntu_runner || ''["ubuntu-latest"]'') }}'
1308-
# Routed like test's and lint's, so it needs their allowance too — it was
1309-
# the last job on the pool still priced flat, and one commit lost it twice:
1310-
# on hk3-9 `npm ci` alone ran 19m46s of the 20 and never finished, and on
1311-
# hk4-23 install took 10m33s against 4m53s on hk5-2, leaving the browser
1312-
# smoke 8m15s before the job died against 2m29s warm. Both phases 2-3x is
1313-
# the pool, not the diff: the same job passed on the previous commit, which
1314-
# touches no web-shell file. Hosted runners keep 20 for the reason lint's
1315-
# copy gives — a genuine hang there should not burn the ECS allowance.
1316-
timeout-minutes: '${{ fromJSON(contains(needs.classify_pr.outputs.ubuntu_runner || ''["ubuntu-latest"]'', ''ecs-qwen'') && ''40'' || ''20'') }}'
1307+
# The same commit repeatedly lost browser resources or exceeded the
1308+
# document performance budget on ECS, while both gates passed on hosted.
1309+
# Keep this browser job hosted without relaxing its test assertions.
1310+
runs-on: 'ubuntu-latest'
1311+
timeout-minutes: 20
13171312
permissions:
13181313
contents: 'read'
13191314
steps:
1320-
# Self-hosted runners reuse the workspace; a prior containerised job can
1321-
# leave root-owned, read-only files anywhere in it. Restore ownership and
1322-
# write permission unconditionally before checkout — see the test job's
1323-
# 'Restore workspace ownership' step for why probing first is unsafe.
1315+
# Retain the shared-pool recovery steps so rerouting needs only a runner
1316+
# change. Hosted workspaces are fresh; on a reused workspace a prior
1317+
# containerised job can leave root-owned, read-only files. Restore them
1318+
# unconditionally before checkout, as in the test job.
13241319
- name: 'Restore workspace ownership'
13251320
run: |-
13261321
set -uo pipefail
@@ -1331,11 +1326,9 @@ jobs:
13311326
fi
13321327
chmod -R u+rwX "$GITHUB_WORKSPACE" 2>/dev/null || sudo -n chmod -R u+rwX "$GITHUB_WORKSPACE" || echo "::warning::could not restore workspace write permissions; checkout may fail on leftover read-only files"
13331328
1334-
# Same pre-checkout recovery as the test job: this job lands on the
1335-
# same reused pool, so leftover review worktrees and branches from an
1336-
# interrupted review would break this checkout too. The
1337-
# `.qwen.root-orig` name's provenance (an external recovery tool) is
1338-
# documented on the test job's copy.
1329+
# Keep this retained sweep pinned to the test job's hardened recovery
1330+
# recipe, even while hosted. The `.qwen.root-orig` name's provenance
1331+
# (an external recovery tool) is documented on the test job's copy.
13391332
- name: 'Clean stale .qwen before checkout'
13401333
run: |-
13411334
set -uo pipefail
@@ -1461,9 +1454,8 @@ jobs:
14611454
npm config set fetch-retries 5
14621455
npm config set fetch-timeout 300000
14631456
1464-
# Same pre-install admission check as the test job (#10035): this job
1465-
# lands on the same self-hosted pool and installs before the gate would
1466-
# otherwise have freed space for it.
1457+
# Retained for a future shared-pool reroute: apply the test job's
1458+
# pre-install admission check (#10035) before dependency installation.
14671459
- name: 'Disk floor gate (self-hosted)'
14681460
if: "${{ runner.environment == 'self-hosted' }}"
14691461
run: 'bash .github/scripts/check-disk-floor.sh "${GITHUB_WORKSPACE}" "${RUNNER_TEMP:-/tmp}"'
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# Web Shell split-view usability
2+
3+
The split view is intended for large displays and multiple monitors. Keep its
4+
equal-width horizontal row, full-height transcripts, independent session
5+
providers, six-pane limit, add/close controls, back navigation, maximize/restore,
6+
and existing per-tab session persistence. Additional layout modes, width dragging,
7+
and moving sessions between windows are outside this change.
8+
9+
Three small improvements make the existing view easier to use without adding a
10+
toolbar row or reducing transcript height:
11+
12+
- Reuse the sidebar's session-details popover on the pane title. Keep its delayed
13+
pointer hover, copy control, workspace, branch, PR/issue links, and status. Open
14+
below the title and constrain it to its pane, inside the Web Shell root. Honor
15+
the host's session-details action allowlist. Header action buttons
16+
remain outside the hover target. Hovering must not focus or activate a pane.
17+
- Mark the last clicked or keyboard-focused pane with an inset line in its
18+
existing header. Adding, closing, and maximizing panes keep this selection
19+
valid, choosing a neighbour when the selected pane closes. Expose the same
20+
current location to assistive technology.
21+
- Show a pending-session count in the existing toolbar only while a pane awaits
22+
tool approval or a user answer. Clicking cycles through waiting panes in row
23+
order, reveals a hidden maximized sibling when needed, scrolls the row to the
24+
target, and focuses the labelled pane wrapper outside all approval keyboard
25+
handlers. Enter, Escape, and digits immediately after navigation cannot submit
26+
a response; Escape can restore the row. Tab or a deliberate click enters the
27+
pane's controls. Announce pending counts without moving focus, and return focus
28+
to Back if the focused pending button disappears. The outer session's notice
29+
appears until a pane actually reports that approval, so failed or
30+
still-attaching panes cannot hide the only available notice.
31+
32+
`ChatPane` already derives its pending tool/question request from its own
33+
transcript. Report that boolean to `SplitView` with a stable callback, including
34+
cleanup when the session changes or unmounts. The parent counts only currently
35+
open panes, including hidden siblings. Do not infer waiting state from the
36+
session-list running flag or introduce another daemon subscription.
37+
38+
`SplitView` passes session-list metadata to the pane; its live running state
39+
comes from the pane's existing active-prompt bridge. Until metadata is available,
40+
retain the native title fallback. Extend
41+
`SessionDetailsTooltip` with bottom placement while preserving the sidebar's
42+
right placement and scoped portal behavior. Keep the shared details markup and
43+
styling, with no duplicate tooltip implementation.
44+
45+
Affected areas are `SplitView`, `ChatPane`, the shared sidebar details popover,
46+
their CSS and focused tests, English/Chinese labels, and the split-view browser
47+
regression suite. No daemon protocol, persistence format, package API, or layout
48+
breakpoint changes are required. There are no open design questions.
49+
50+
Validation covers delayed hover without focus theft, action-button exclusion,
51+
active-pane pointer/keyboard selection, pending tool and question transitions,
52+
hidden-pane navigation, draft retention, and existing add/close/maximize/reload
53+
behavior. The browser regressions are committed in
54+
`packages/web-shell/client/e2e/web-shell.split-persist.spec.ts` and tagged
55+
`@smoke` for PR CI. The baseline and verification evidence are published in
56+
[PR #11250](https://github.com/QwenLM/qwen-code/pull/11250).
57+
58+
The PR's browser CI job runs on GitHub hosted Ubuntu. The unchanged feature
59+
passed the document gate and all 50 smoke cases there, while three ECS runs
60+
failed during browser resource loading or at the document performance budget.
61+
Keep the existing 60-second document budget, smoke assertions, triggers, and
62+
hosted 20-minute job limit. Other CI jobs retain their current runner routing.
63+
This uses hosted capacity for the browser job and may incur hosted queue time;
64+
it does not diagnose the underlying ECS network or performance bottleneck.

‎packages/web-shell/client/App.test.tsx‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
type GoalSnapshotV2,
2828
} from '@qwen-code/sdk/daemon';
2929
import type { WebShellApi } from './App';
30+
import { DEFAULT_SESSION_ACTION_ITEMS } from './components/sidebar/WebShellSidebar';
3031
import type { Message } from './adapters/types';
3132
import type {
3233
VoiceStatusRevision,
@@ -653,6 +654,8 @@ const {
653654
settings: DaemonSettingDescriptor[];
654655
} | null,
655656
latestSplitViewProps: null as {
657+
onPendingPanesChange?: (ids: string[]) => void;
658+
showSessionDetails?: boolean;
656659
includeOtherWorkspaces?: boolean;
657660
workspaceCwd?: string;
658661
sessionWorkflowEnabled?: boolean;
@@ -1315,9 +1318,14 @@ vi.mock('./components/dialogs/DialogShell', async () => {
13151318
};
13161319
});
13171320

1318-
vi.mock('./components/sidebar/WebShellSidebar', async () => {
1321+
vi.mock('./components/sidebar/WebShellSidebar', async (importOriginal) => {
13191322
const React = await import('react');
1323+
const actual =
1324+
await importOriginal<
1325+
typeof import('./components/sidebar/WebShellSidebar')
1326+
>();
13201327
return {
1328+
DEFAULT_SESSION_ACTION_ITEMS: actual.DEFAULT_SESSION_ACTION_ITEMS,
13211329
WebShellSidebar: (props: {
13221330
collapsed?: boolean;
13231331
onOpenSettings?: () => void;
@@ -1747,6 +1755,7 @@ vi.doMock('./components/SplitView', async () => {
17471755
onExit?: () => void;
17481756
sessionIds?: string[];
17491757
onPanesChange?: (ids: string[]) => void;
1758+
onPendingPanesChange?: (ids: string[]) => void;
17501759
includeOtherWorkspaces?: boolean;
17511760
workspaceCwd?: string;
17521761
sessionWorkflowEnabled?: boolean;
@@ -10515,6 +10524,12 @@ describe('App session workflow', () => {
1051510524
await Promise.resolve();
1051610525
});
1051710526

10527+
await act(async () => {
10528+
container
10529+
.querySelector<HTMLButtonElement>('[data-testid="split-report-panes"]')
10530+
?.click();
10531+
});
10532+
1051810533
testState.settings = [sessionWorkflowSetting()];
1051910534
rerender();
1052010535
await flush();
@@ -28888,6 +28903,91 @@ describe('App session callbacks', () => {
2888828903
).toBeNull();
2888928904
});
2889028905

28906+
it.each<undefined | Array<'details'>>([undefined, [], ['details']])(
28907+
'applies the session-details allowlist to split panes: %j',
28908+
async (items) => {
28909+
const { container } = renderApp({
28910+
sidebar: { sessionActions: { items } },
28911+
});
28912+
await flush();
28913+
await act(async () => {
28914+
container
28915+
.querySelector<HTMLButtonElement>('[data-testid="open-split-view"]')
28916+
?.click();
28917+
});
28918+
expect(testState.latestSplitViewProps?.showSessionDetails).toBe(
28919+
(items ?? DEFAULT_SESSION_ACTION_ITEMS).includes('details'),
28920+
);
28921+
},
28922+
);
28923+
28924+
it.each([false, true])(
28925+
'does not rerender App for other split sessions (outer pending: %s)',
28926+
async (outerPending) => {
28927+
const { container, rerender } = renderApp();
28928+
await flush();
28929+
await act(async () => {
28930+
container
28931+
.querySelector<HTMLButtonElement>('[data-testid="open-split-view"]')
28932+
?.click();
28933+
});
28934+
await flush();
28935+
const report = testState.latestSplitViewProps!.onPendingPanesChange!;
28936+
const ownerIds = outerPending ? [mockConnection.sessionId!] : [];
28937+
await act(async () => report(ownerIds));
28938+
rerender();
28939+
expect(testState.latestSplitViewProps!.onPendingPanesChange).toBe(report);
28940+
expect(mockUseDaemonActivePromptBridge).toHaveBeenCalled();
28941+
mockUseDaemonActivePromptBridge.mockClear();
28942+
for (const ids of [['foreign-session'], ['another-session'], []]) {
28943+
await act(async () => report([...ownerIds, ...ids]));
28944+
expect(mockUseDaemonActivePromptBridge).not.toHaveBeenCalled();
28945+
}
28946+
},
28947+
);
28948+
28949+
it('keeps the outer approval notice until its current session is reported', async () => {
28950+
const { container, rerender } = renderApp();
28951+
await flush();
28952+
await act(async () => {
28953+
container
28954+
.querySelector<HTMLButtonElement>('[data-testid="open-split-view"]')
28955+
?.click();
28956+
});
28957+
await act(async () => {
28958+
testState.blocks = [makePendingPermissionBlock()];
28959+
rerender();
28960+
});
28961+
expect(
28962+
container.querySelector('[data-testid="split-initial"]')?.textContent,
28963+
).toContain(mockConnection.sessionId);
28964+
const notice = () =>
28965+
container.querySelector('[data-testid="split-approval-notice"]');
28966+
expect(notice()).not.toBeNull();
28967+
await act(async () => {
28968+
testState.latestSplitViewProps?.onPendingPanesChange?.([
28969+
mockConnection.sessionId!,
28970+
]);
28971+
});
28972+
expect(notice()).toBeNull();
28973+
const previous = testState.latestSplitViewProps!.onPendingPanesChange!;
28974+
const previousSessionId = mockConnection.sessionId!;
28975+
await act(async () => {
28976+
mockConnection.sessionId = 'outer-session-2';
28977+
rerender();
28978+
});
28979+
const next = testState.latestSplitViewProps!.onPendingPanesChange!;
28980+
expect(next).not.toBe(previous);
28981+
await act(async () => next([previousSessionId]));
28982+
expect(notice()).not.toBeNull();
28983+
await act(async () => next(['outer-session-2']));
28984+
expect(notice()).toBeNull();
28985+
await act(async () => {
28986+
testState.latestSplitViewProps?.onPendingPanesChange?.([]);
28987+
});
28988+
expect(notice()).not.toBeNull();
28989+
});
28990+
2889128991
it('surfaces the outer approval as a split notice and returns to chat when clicked', async () => {
2889228992
// The overlay is suppressed under the split, so the outer approval would be
2889328993
// invisible; a notice banner (with a way back) is the only signal.
@@ -28900,6 +29000,12 @@ describe('App session callbacks', () => {
2890029000
?.click();
2890129001
await Promise.resolve();
2890229002
});
29003+
await act(async () => {
29004+
container
29005+
.querySelector<HTMLButtonElement>('[data-testid="split-report-panes"]')
29006+
?.click();
29007+
});
29008+
2890329009
await act(async () => {
2890429010
testState.blocks = [makePendingPermissionBlock()];
2890529011
rerender();

‎packages/web-shell/client/App.tsx‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ import {
242242
} from './shadowDom';
243243
import {
244244
WebShellSidebar,
245+
DEFAULT_SESSION_ACTION_ITEMS,
245246
type WebShellSidebarBranding,
246247
type WebShellSidebarFooterOptions,
247248
type WebShellSidebarWorkspaceOverviewOptions,
@@ -8060,6 +8061,12 @@ export function App({
80608061
}, [artifactPanelOpen, useFloatingArtifactPanel]);
80618062
// Sessions to seed the split view with (e.g. the selection from the overview).
80628063
const [splitSessionIds, setSplitSessionIds] = useState<string[]>([]);
8064+
const [outerSplitPanePending, setOuterSplitPanePending] = useState(false);
8065+
const handleSplitPendingPanesChange = useCallback(
8066+
(ids: string[]) =>
8067+
setOuterSplitPanePending(ids.includes(connection.sessionId ?? '')),
8068+
[connection.sessionId],
8069+
);
80638070
// False until the split bootstrap has decided whether a split view is
80648071
// coming (URL deep link, per-tab sessionStorage, or controlled prop). The
80658072
// pane-tab reclaim below must wait for it: at restore-commit time
@@ -17634,9 +17641,10 @@ export function App({
1763417641
<div className={styles.fullPage} data-testid="split-view-page">
1763517642
{/* The outer session's approval overlay is suppressed under the
1763617643
split (it would own ghost keyboard shortcuts). If that
17637-
session isn't one of the panes, the approval would be
17638-
invisible — surface a notice with a way back to it. */}
17639-
{approvalOverlayActive && (
17644+
session's pane hasn't surfaced its approval (including
17645+
failed or still-attaching panes), show a way back to it. */}
17646+
{approvalOverlayActive &&
17647+
!outerSplitPanePending && (
1764017648
<div
1764117649
className={styles.splitApprovalNotice}
1764217650
role="status"
@@ -17655,11 +17663,16 @@ export function App({
1765517663
<WebShellCustomizationProvider value={customization}>
1765617664
<SplitView
1765717665
sessionIds={splitSessionIds}
17666+
showSessionDetails={
17667+
(sidebarOptions.sessionActions?.items ??
17668+
DEFAULT_SESSION_ACTION_ITEMS).includes('details')
17669+
}
1765817670
// Mirror live pane add/remove back up so switching away
1765917671
// and re-entering restores the same panes. Keep this
1766017672
// callback stable to avoid looping SplitView's reporting
1766117673
// effect.
1766217674
onPanesChange={handleSplitPanesChange}
17675+
onPendingPanesChange={handleSplitPendingPanesChange}
1766317676
includeOtherWorkspaces={!lockedWorkspaceCwd}
1766417677
workspaceCwd={lockedWorkspaceCwd}
1766517678
// Back returns to the Session Overview (the hub the split

‎packages/web-shell/client/components/ChatPane.module.css‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
border-radius: 0;
1616
}
1717

18+
.pane[data-pane-active] .header {
19+
box-shadow: inset 0 2px var(--primary);
20+
}
21+
1822
.paneEmbedded .footer {
1923
padding-bottom: 0;
2024
}

0 commit comments

Comments
 (0)