From 03d69b4a82878a8f889d84c568c7a0c471fb95bd Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Fri, 4 Sep 2026 00:42:46 +0900 Subject: [PATCH 01/26] fix(web-shell): keep the auto-sized dialog's gutter on a phone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `auto` size floors the panel at `min-w-[min(100%,560px)]` so a small plan graph does not collapse to a narrow column. tailwind-merge keeps that class alongside the base ceiling `max-w-[calc(100%-2rem)]` — different modifier groups — and below the `sm:` breakpoint min-width wins over max-width. On a 390px viewport the Plan & tasks dialog therefore rendered its rounded, ringed panel flush to both screen edges, while every fixed size (sm/md/lg/xl) kept the 1rem gutter at that same width. Floor the panel at the gutter the ceiling already reserves. --- .../client/components/dialogs/DialogShell.test.tsx | 8 +++++++- .../web-shell/client/components/dialogs/DialogShell.tsx | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/web-shell/client/components/dialogs/DialogShell.test.tsx b/packages/web-shell/client/components/dialogs/DialogShell.test.tsx index 26425e6b1d5..36a507df0f4 100644 --- a/packages/web-shell/client/components/dialogs/DialogShell.test.tsx +++ b/packages/web-shell/client/components/dialogs/DialogShell.test.tsx @@ -187,7 +187,13 @@ describe('DialogShell', () => { // stays full-width and never tracks the content. expect(panel.className).toContain('w-max'); expect(panel.className).not.toContain('w-full'); - expect(panel.className).toContain('min-w-[min(100%,560px)]'); + // The floor has to subtract the same gutter the surviving base ceiling + // (`max-w-[calc(100%-2rem)]`) reserves. twMerge keeps both classes, and + // below `sm:` a bare `min(100%,560px)` floor outranks that ceiling, so the + // panel would render flush to both screen edges on a phone. + expect(panel.className).toContain('min-w-[min(calc(100%-2rem),560px)]'); + expect(panel.className).not.toContain('min-w-[min(100%,560px)]'); + expect(panel.className).toContain('max-w-[calc(100%-2rem)]'); expect(panel.className).toContain( 'sm:max-w-[min(calc(100vw-2rem),1120px)]', ); diff --git a/packages/web-shell/client/components/dialogs/DialogShell.tsx b/packages/web-shell/client/components/dialogs/DialogShell.tsx index 260aab228e0..341a0e2083c 100644 --- a/packages/web-shell/client/components/dialogs/DialogShell.tsx +++ b/packages/web-shell/client/components/dialogs/DialogShell.tsx @@ -43,7 +43,12 @@ const sizeClass: Record = { // room. `w-max` wins over DialogContent's base `w-full` through // tailwind-merge. The floor keeps small graphs from collapsing to a narrow // panel; the ceiling keeps large ones from spanning a wide monitor. - auto: 'w-max min-w-[min(100%,560px)] sm:max-w-[min(calc(100vw-2rem),1120px)]', + // The floor uses the same 2rem gutter the base ceiling + // (`max-w-[calc(100%-2rem)]`) reserves: twMerge keeps both classes, and + // below `sm:` a bare `min(100%,560px)` floor outranks that ceiling, so the + // panel rendered flush to both screen edges on a phone while every fixed + // size kept its gutter. + auto: 'w-max min-w-[min(calc(100%-2rem),560px)] sm:max-w-[min(calc(100vw-2rem),1120px)]', }; const FOCUSABLE_SELECTOR = [ From 099d37d44838febebc5f116c00a1cb7861d87664 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Fri, 4 Sep 2026 00:42:46 +0900 Subject: [PATCH 02/26] fix(web-shell): bind the DAG lane reservation to the constant that places lanes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lane spacing was stated twice in two languages: `EDGE_LANE_HEIGHT` in TS, which places each layer-spanning return lane at `lane * EDGE_LANE_HEIGHT`, and a literal `9px` in `.dagCanvas`'s bottom padding, which reserves room for them. The two are coupled only through the `--plan-edge-lanes` inline style, so raising the constant alone leaves the reservation short; with enough spanning edges the clamp pins the outer lanes together and layer-spanning edges overlap again — the exact defect the lanes exist to fix. Nothing would have caught it: the lane test mocks `offsetHeight` directly. TS now publishes the pitch as `--plan-edge-lane-height` beside the lane count, and the stylesheet reads it. Also give the graph a phone: three 240px lanes plus two 64px gutters are ~850px, so the canvas scrolled in both axes at once and the horizontal scroll hid the layer the vertical scroll was looking for. Below 720px the lane and gutter narrow. Layer semantics and the rect-measured edge routing are untouched, so this changes what fits, not what is drawn. The new stylesheet guard pins both, in the shape the other `.css.test.ts` files use, since jsdom does not compute the cascade. One test-only addition on the same component: the spanning-edge test fixed the edge's arrival column (`H 876`) but not its arrival row, and nothing else in the file asserted a spanning edge's `endY`. Substituting the return lane's `routeY` for `endY` in the path tail draws the edge ~170px below the target node while every existing assertion still passes. Pinned corner-agnostically. Refs #10866. --- .../messages/PlanExecutionView.css.test.ts | 50 +++++++++++++++++++ .../messages/PlanExecutionView.module.css | 22 +++++++- .../messages/PlanExecutionView.test.tsx | 8 +++ .../components/messages/PlanExecutionView.tsx | 7 ++- 4 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 packages/web-shell/client/components/messages/PlanExecutionView.css.test.ts diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.css.test.ts b/packages/web-shell/client/components/messages/PlanExecutionView.css.test.ts new file mode 100644 index 00000000000..36d6234807b --- /dev/null +++ b/packages/web-shell/client/components/messages/PlanExecutionView.css.test.ts @@ -0,0 +1,50 @@ +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +// jsdom does not compute the CSS cascade, so pin the stylesheet's source +// shape instead. The DAG reserves bottom padding for its layer-spanning +// return lanes, and that reservation has to use the same pitch that places +// the lanes: TS owns `EDGE_LANE_HEIGHT` and the canvas placed lanes at +// `lane * EDGE_LANE_HEIGHT`, while the stylesheet re-stated the pitch as a +// literal `9px`. Raising the constant alone left the reservation short, the +// clamp pinned the outer lanes together, and layer-spanning edges overlapped +// again — the exact defect the lanes exist to fix — with no test signal, +// because the lane test mocks `offsetHeight` directly. Strip comments so the +// guards match declarations only, not prose about them. +const planCss = readFileSync( + fileURLToPath(new URL('./PlanExecutionView.module.css', import.meta.url)), + 'utf8', +).replace(/\/\*[\s\S]*?\*\//g, ''); +const planSource = readFileSync( + fileURLToPath(new URL('./PlanExecutionView.tsx', import.meta.url)), + 'utf8', +); + +describe('PlanExecutionView stylesheet', () => { + it('takes the edge-lane pitch from TS instead of restating it', () => { + const canvas = planCss.match(/\.dagCanvas\s*\{[^}]*\}/)?.[0]; + expect(canvas).toBeTruthy(); + expect(canvas).toMatch(/var\(--plan-edge-lane-height/); + // The literal that used to stand in for the constant must not return + // anywhere in the reservation. + expect(canvas).not.toMatch(/--plan-edge-lanes\)\s*\*\s*9px/); + // TS has to actually publish it, or the fallback silently takes over. + expect(planSource).toMatch( + /'--plan-edge-lane-height':\s*`\$\{EDGE_LANE_HEIGHT\}px`/, + ); + }); + + it('keeps the DAG on one scroll axis at phone widths', () => { + // Three 240px lanes plus two 64px gutters exceed a phone viewport, so the + // canvas scrolled in both axes at once and the horizontal scroll hid the + // layer the vertical scroll was looking for. + expect(planCss).toMatch( + /\.dagCanvas\s+\.layer\s*\{[^}]*flex:\s*0\s+0\s+var\(--plan-layer-width/, + ); + const narrow = planCss.match( + /@media\s*\(max-width:\s*720px\)\s*\{[\s\S]*?\n\}/, + )?.[0]; + expect(narrow).toMatch(/--plan-layer-width:/); + }); +}); diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.module.css b/packages/web-shell/client/components/messages/PlanExecutionView.module.css index fa499953916..9153b372678 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.module.css +++ b/packages/web-shell/client/components/messages/PlanExecutionView.module.css @@ -137,6 +137,7 @@ .dagCanvas { --plan-edge-lanes: 0; + --plan-edge-lane-height: 9px; position: relative; display: flex; align-items: center; @@ -144,7 +145,11 @@ gap: 64px; width: max-content; min-width: 100%; - padding: 8px 10px calc(28px + var(--plan-edge-lanes) * 9px); + /* Both operands come from TS (PlanExecutionView sets --plan-edge-lanes and + --plan-edge-lane-height from EDGE_LANE_HEIGHT) so the reservation cannot + drift from the lane placement it is reserving for. */ + padding: 8px 10px + calc(28px + var(--plan-edge-lanes) * var(--plan-edge-lane-height, 9px)); } .dagEdges { @@ -218,10 +223,23 @@ .dagCanvas .layer { position: relative; z-index: 1; - flex: 0 0 240px; + flex: 0 0 var(--plan-layer-width, 240px); min-width: 0; } +/* Three 240px lanes plus two 64px gutters are ~850px, so on a phone the graph + became a canvas that scrolls in both axes at once — the horizontal scroll + hiding the layer the vertical scroll is looking for. Narrowing the lane and + the gutter keeps the whole topology on the horizontal axis the graph + actually needs, and leaves the layer semantics (and the measured edge + routing, which reads real rects) untouched. */ +@media (max-width: 720px) { + .dagCanvas { + --plan-layer-width: 168px; + gap: 32px; + } +} + /* Status drives one accent, and the accent drives every edge treatment on the node: the border, and the selection ring. Before this, status set `border-color` while selection stacked an always-blue ring on top, so a diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx index be0b4eea8d0..3e0f098b3a7 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx @@ -888,6 +888,14 @@ describe('PlanExecutionView', () => { (match) => Number(match[1]), ); expect(Math.max(...routedYs)).toBeGreaterThan(200); + // ...and arrives at the target's own row, not merely at its column. The + // `release` node sits at y 10 with height 80 inside a viewport whose + // origin is (100, 50), so its vertical centre is 50. Without this, a tail + // drawn from the return lane's `routeY` instead of the edge's `endY` + // lands ~170px below the node while the column and clearance assertions + // above both still pass. + expect(spanningEdge).toMatch(/ 50 H 876$/); + expect(routedYs).toContain(50); act(() => root.unmount()); container.remove(); diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.tsx index f2a1c735a99..0c8ead95987 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.tsx @@ -1158,7 +1158,12 @@ export function PlanExecutionView({ ref={hasDependencies ? graphRef : undefined} style={ hasDependencies - ? ({ '--plan-edge-lanes': graph.lanes } as CSSProperties) + ? ({ + '--plan-edge-lanes': graph.lanes, + // Publish the lane pitch so .dagCanvas reserves bottom + // padding from the same constant that places the lanes. + '--plan-edge-lane-height': `${EDGE_LANE_HEIGHT}px`, + } as CSSProperties) : undefined } > From 3863bb683bfc9fc57103678c873ce6bb175ee116 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Fri, 4 Sep 2026 00:43:06 +0900 Subject: [PATCH 03/26] test(sdk): pin the skills entry in the transcript compactor keep-set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `compactTaskExecutionOutput` keeps an allowlist of `task_execution` fields when it drops the bulky ones, and every neighbouring entry — `executionSummary`, `tokenCount` — is already pinned by the reducer tests. `skills` was not: removing it from the keep-set left the whole suite green, and a session restored from a persisted transcript would silently lose the skill list the live run recorded. Mutation-checked: deleting `'skills'` from the keep-set fails exactly this assertion (1 failed | 327 passed); restored, 328/328. --- packages/sdk-typescript/test/unit/daemonUi.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/sdk-typescript/test/unit/daemonUi.test.ts b/packages/sdk-typescript/test/unit/daemonUi.test.ts index 4f535e8849e..120b59e0e0e 100644 --- a/packages/sdk-typescript/test/unit/daemonUi.test.ts +++ b/packages/sdk-typescript/test/unit/daemonUi.test.ts @@ -8531,6 +8531,7 @@ describe('parallel subAgent text interleaving fix', () => { result: 'large result', taskPrompt: 'large prompt', toolCalls: [{ callId: 'child-tool' }], + skills: ['repo-ops'], executionSummary: { inputTokens: 100, outputTokens: 20, @@ -8548,6 +8549,13 @@ describe('parallel subAgent text interleaving fix', () => { taskPrompt: expect.anything(), toolCalls: expect.anything(), }); + // The keep-set is what survives compaction, and every other entry in it is + // pinned here. Without this, dropping 'skills' from that list ships green + // and a session restored from a persisted transcript silently loses the + // skill list the live run recorded. + expect( + (state.blocks[1] as { rawOutput?: Record }).rawOutput, + ).toMatchObject({ skills: ['repo-ops'] }); expect(state.blocks[1]).not.toHaveProperty('content'); }); From f2c818f9409cd3df16e5e1ca1c21099e8f585357 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Fri, 4 Sep 2026 00:43:06 +0900 Subject: [PATCH 04/26] feat(web-shell): make workflow dependencies navigable, and let the surface state each thing once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related passes over the Session Workflow inspector and cockpit. ## Dependencies become navigation A step's dependencies rendered as `todo.blockedBy!.join(', ')` — a run of raw Todo ids, in both the graph node and the inspector detail. An id is an address, not a label, so following an edge meant reading `step-3, step-7` and then finding those rows by eye, in a panel whose entire subject is which step waits on which. Each reference is now the step it points at — the step number the list and the graph both already show, plus its title — as a control that selects that step. The dependency list becomes the graph's navigation. Recent activity was sliced to 6 while the count beside it reported the true total, so the panel contradicted itself and the remainder was unreachable. The cap stays as a preview and gains a control that opens the rest. ## The surface states each thing once The panel had accumulated chrome that competed with its content: - Section labels were tracked-out uppercase at 10-11px, so every fixed label shouted over the step title and status it introduced. They are labels, not headlines: sentence case at a muted weight. - The cockpit spent the accent colour on a fixed "Dependency graph" eyebrow above the session title — making the one blue thing on the page the one thing that never changes, while the graph below uses that same accent to mean "this node is selected". The label moves to the subtitle line and the accent is left to mean selection. - Completed-over-total sat in a tinted pill directly above the progress bar that draws the same ratio, beside the status word that names the same state — three surfaces for one fact. The count keeps the tone as ink and drops the pill. - Runtime, tool calls, tokens and artifact kind/status were glued into single strings with middle dots, which wrapped as a unit and read out as one run of text. They are separate facts, so they are separate elements. The identity line keeps the mono face only on the short session id, which is an address the user copies or matches against a daemon log. No token or theme values change; this is the existing system applied more sparingly. Both behaviour changes are mutation-checked: reverting the dependency list to the joined id string, and restoring the silent cap, each fail exactly their new test. Refs #10866. --- .../SessionWorkflowCockpit.module.css | 34 ++++-- .../workflow/SessionWorkflowCockpit.tsx | 13 +- .../SessionWorkflowInspector.module.css | 103 ++++++++++++++-- .../SessionWorkflowInspector.test.tsx | 115 ++++++++++++++++++ .../workflow/SessionWorkflowInspector.tsx | 91 ++++++++++++-- packages/web-shell/client/i18n.tsx | 2 + 6 files changed, 326 insertions(+), 32 deletions(-) diff --git a/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.module.css b/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.module.css index a48bf34c6b7..953a487da93 100644 --- a/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.module.css +++ b/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.module.css @@ -47,12 +47,31 @@ min-width: 0; } -.identity span { - color: var(--agent-blue-500); - font-size: 10px; - font-weight: 800; - letter-spacing: 0.1em; - text-transform: uppercase; +/* The accent is the selection colour the graph below uses to say "this node". + Spending it on a fixed label above the title made the one blue thing on the + page the one thing that never changes, so the label drops to the subtitle + line and the accent is left to mean selection. */ +.identity small { + display: flex; + flex-wrap: wrap; + align-items: baseline; + gap: 2px 10px; +} + +.identity small > span { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.workspaceName { + max-width: 28ch; +} + +/* The short session id is an address the user copies or matches against a + daemon log, so it keeps the mono face the rest of the line does not need. */ +.sessionRef { + font-family: var(--font-mono, monospace); } /* preflight.css resets headings to `font: inherit`, so size and weight both @@ -60,7 +79,7 @@ `min-width: 0` already caps the width; a fixed max-width only stranded space on wide viewports. */ .identity h1 { - margin: 4px 0 3px; + margin: 0 0 3px; overflow: hidden; font-size: clamp(20px, 2.2vw, 28px); font-weight: 600; @@ -71,7 +90,6 @@ .identity small { color: var(--muted-foreground); - font-family: var(--font-mono, monospace); font-size: 11px; } diff --git a/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.tsx b/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.tsx index 15ed8ef087e..df7dd7ae931 100644 --- a/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.tsx +++ b/packages/web-shell/client/components/workflow/SessionWorkflowCockpit.tsx @@ -88,14 +88,19 @@ export function SessionWorkflowCockpit({ {t('workflow.chatTitle')}
- {t('workflow.inspector.graphCanvas')}

{sessionName || t('workflow.session.defaultTitle')}

- {sessionId.slice(0, 8)} ·{' '} - {workspaceCwd?.split('/').at(-1) || - t('workflow.session.workspace')} + {t('workflow.inspector.graphCanvas')} + + {workspaceCwd?.split('/').at(-1) || + t('workflow.session.workspace')} + + {sessionId.slice(0, 8)}
diff --git a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.module.css b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.module.css index 5c2d6dda1d7..40871e01ae0 100644 --- a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.module.css +++ b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.module.css @@ -44,35 +44,39 @@ gap: 3px; } +/* Section labels name what follows; they are not decoration. Tracked-out + uppercase made every one of them shout over the content it introduces, so + they read as chrome. Sentence case at a muted weight puts the emphasis back + on the step title and the status beside it. Kept scoped to the heading's + inner div so the count beside it keeps its own tone. */ .summaryHeading > div span, .sectionHeading > div > span { color: var(--muted-foreground); font-size: 11px; - font-weight: 700; - letter-spacing: 0.06em; - text-transform: uppercase; + font-weight: 500; } .summaryHeading strong { font-size: 16px; } +/* Completed-over-total is a reading of the progress bar directly beneath it, + and the status word sits to its left, so a third tinted surface said the + same thing a third time. The count keeps the tone as ink and drops the + pill. */ .summaryCount { - padding: 5px 8px; - border-radius: 999px; color: var(--status-running-fg); - background: var(--status-running-bg); - font-size: 12px; + font-size: 13px; + font-variant-numeric: tabular-nums; + font-weight: 600; } .summaryCount[data-status='attention'] { color: var(--status-attention-fg); - background: var(--status-attention-bg); } .summaryCount[data-status='completed'] { color: var(--status-done-fg); - background: var(--status-done-bg); } .progress { @@ -276,6 +280,71 @@ overflow-wrap: anywhere; } +/* A dependency is an edge in the graph beside it, so it is rendered as the + step it points at — number and title — and selects that step on click. The + previous comma-joined id list made the reader find the row by eye. */ +.dependencyList { + display: flex; + flex-wrap: wrap; + gap: 4px; + margin: 0; + padding: 0; + list-style: none; +} + +.dependencyLink { + display: flex; + max-width: 100%; + align-items: center; + gap: 6px; + padding: 3px 7px 3px 4px; + border: 1px solid var(--border); + border-radius: var(--radius-md); + cursor: pointer; + font-size: 11px; + text-align: left; +} + +.dependencyLink > span:last-child { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; +} + +.dependencyLink:hover { + border-color: color-mix(in srgb, var(--agent-blue-500) 45%, var(--border)); + background: var(--accent); +} + +.dependencyNumber { + display: grid; + width: 16px; + height: 16px; + flex: none; + place-items: center; + border-radius: var(--radius-sm, 4px); + color: var(--muted-foreground); + background: var(--muted); + font-size: 10px; + font-variant-numeric: tabular-nums; +} + +/* The summary beside the list reports the true total, so the preview cap + needs a way out of it rather than a silent truncation. */ +.showAllActivity { + padding: 7px 8px; + border-radius: var(--radius-md); + color: var(--muted-foreground); + cursor: pointer; + font-size: 11px; + text-align: left; +} + +.showAllActivity:hover { + color: var(--foreground); + background: var(--accent); +} + .linkedAgents { margin-top: 14px; padding-top: 12px; @@ -431,6 +500,22 @@ font-size: 11px; } +/* Runtime, tool calls and tokens are separate facts, so they are separate + elements with real spacing rather than one string glued with middle dots — + which also let them wrap as a unit and be read out as one run of text. */ +/* Specificity note: `.linkedAgents button small` (0,1,2) and + `.deliverables small` (0,1,1) both declare the single-line ellipsis shape + these rows use, so the wrapping metrics row has to out-specify them rather + than rely on source order. */ +.linkedAgents button small.metrics, +.deliverables button small.metrics { + display: flex; + overflow: visible; + flex-wrap: wrap; + gap: 2px 10px; + white-space: normal; +} + .stateLabel { color: var(--muted-foreground); font-size: 11px; diff --git a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx index 2f42b77fb50..a66f37596cf 100644 --- a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx +++ b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx @@ -147,4 +147,119 @@ describe('SessionWorkflowInspector', () => { act(() => root.unmount()); container.remove(); }); + + it('selects the referenced step from a dependency, by number and title', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + const onSelectedTodoIdChange = vi.fn(); + const todos: TodoItem[] = [ + { id: 'prepare', content: 'Prepare inputs', status: 'completed' }, + { + id: 'ship', + content: 'Ship result', + status: 'in_progress', + blockedBy: ['prepare'], + }, + ]; + + act(() => { + root.render( + + + , + ); + }); + + const upstream = container.querySelector( + '[data-testid="workflow-dependency-prepare"]', + ); + // The reference reads as the step it points at — the step number the list + // and graph both show, plus its title — not the bare `prepare` id. + expect(upstream).toBeTruthy(); + expect(upstream?.textContent).toContain('1'); + expect(upstream?.textContent).toContain('Prepare inputs'); + + onSelectedTodoIdChange.mockClear(); + act(() => upstream?.click()); + expect(onSelectedTodoIdChange).toHaveBeenCalledWith('prepare'); + + act(() => root.unmount()); + container.remove(); + }); + + it('keeps every activity row reachable past the preview cap', () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + const todos: TodoItem[] = [ + { id: 'build', content: 'Build', status: 'in_progress' }, + ]; + const tools: ACPToolCall[] = Array.from({ length: 8 }, (_, index) => ({ + callId: `agent-${index}`, + toolName: 'Agent', + title: `Agent ${index}`, + status: 'completed' as const, + args: { todo_id: 'build' }, + })); + const tasks = tools.map((tool, index) => ({ + kind: 'agent' as const, + id: `task-${index}`, + label: `Agent ${index}`, + description: `Ran step ${index}`, + status: 'completed' as const, + startTime: 1_000 + index, + endTime: 2_000 + index, + runtimeMs: 1_000, + isBackgrounded: true, + toolUseId: tool.callId, + })); + + act(() => { + root.render( + + + , + ); + }); + + const activityList = () => + Array.from(container.querySelectorAll('button')).filter((button) => + /^\d{1,2}:\d{2}/.test(button.textContent?.trim() ?? ''), + ); + const beforeExpand = activityList().length; + const showAll = container.querySelector( + '[data-testid="workflow-activity-show-all"]', + ); + // The count beside the list reports the true total, so the cap has to be + // an invitation rather than a silent truncation. + expect(beforeExpand).toBe(6); + expect(showAll?.textContent).toContain('8'); + + act(() => showAll?.click()); + expect(activityList().length).toBe(8); + expect( + container.querySelector('[data-testid="workflow-activity-show-all"]'), + ).toBeNull(); + + act(() => root.unmount()); + container.remove(); + }); }); diff --git a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.tsx b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.tsx index 4b7f484adb8..b71f6393953 100644 --- a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.tsx +++ b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo } from 'react'; +import { useEffect, useMemo, useState } from 'react'; import type { DaemonSessionArtifact, DaemonSessionTaskStatus, @@ -21,6 +21,12 @@ import { } from './session-workflow-model'; import styles from './SessionWorkflowInspector.module.css'; +/** + * Activity rows shown before the list asks to be expanded. The summary beside + * it reports the true total, so the remainder must stay reachable. + */ +const ACTIVITY_PREVIEW_COUNT = 6; + export interface SessionWorkflowInspectorProps { todos: readonly TodoItem[]; tools: readonly ACPToolCall[]; @@ -52,6 +58,15 @@ export function SessionWorkflowInspector({ [tasks, todos, tools], ); const defaultTodoId = getDefaultWorkflowTodoId(todos, projection); + // Dependencies are stated as Todo ids, which are addresses, not labels. The + // step number is what the list and the graph both show, so carry it into + // every dependency reference. + const stepNumberById = useMemo(() => { + const numbers = new Map(); + todos.forEach((todo, index) => numbers.set(todo.id, index + 1)); + return numbers; + }, [todos]); + const [showAllActivity, setShowAllActivity] = useState(false); const effectiveSelectedTodoId = projection.todosById.has(selectedTodoId ?? '') ? selectedTodoId : defaultTodoId; @@ -90,6 +105,29 @@ export function SessionWorkflowInspector({ ? (projection.dependentsByTodo.get(selectedTodo.id) ?? []) : []; + // A dependency reference selects the step it names: the list of ids was + // read-only text, so following an edge meant finding the row by eye. + const dependencyLink = (todoId: string) => { + const target = projection.todosById.get(todoId); + const number = stepNumberById.get(todoId); + return ( +
  • + +
    • + ); + }; + const detail = selectedTodo && selectedState && (
    @@ -106,17 +144,25 @@ export function SessionWorkflowInspector({
    {t('workflow.dependencies.upstream')}
    - {upstream?.length - ? upstream.join(', ') - : t('workflow.dependencies.none')} + {upstream?.length ? ( +
      + {upstream.map((id) => dependencyLink(id))} +
    + ) : ( + t('workflow.dependencies.none') + )}
    {t('workflow.dependencies.unblocks')}
    - {downstream.length - ? downstream.map((todo) => todo.id).join(', ') - : t('workflow.dependencies.noDownstream')} + {downstream.length ? ( +
      + {downstream.map((todo) => dependencyLink(todo.id))} +
    + ) : ( + t('workflow.dependencies.noDownstream') + )}
    @@ -156,7 +202,13 @@ export function SessionWorkflowInspector({ task.description} )} - {metrics.length > 0 && {metrics.join(' · ')}} + {metrics.length > 0 && ( + + {metrics.map((metric) => ( + {metric} + ))} + + )} {task && ( @@ -302,7 +354,10 @@ export function SessionWorkflowInspector({ {projection.activity.length}
    - {projection.activity.slice(0, 6).map((task) => { + {(showAllActivity + ? projection.activity + : projection.activity.slice(0, ACTIVITY_PREVIEW_COUNT) + ).map((task) => { const tool = projection.toolsByTaskId.get(task.id); const at = task.endTime ?? task.startTime; const content = ( @@ -340,6 +395,19 @@ export function SessionWorkflowInspector({ {projection.activity.length === 0 && (

    {t('workflow.activity.empty')}

    )} + {!showAllActivity && + projection.activity.length > ACTIVITY_PREVIEW_COUNT && ( + + )}
    @@ -358,8 +426,9 @@ export function SessionWorkflowInspector({ > {artifact.title} - - {artifact.kind} · {artifact.status} + + {artifact.kind} + {artifact.status}
    @@ -1421,7 +1423,9 @@ export function PlanExecutionView({ type="button" > {(stepNumberByTodo.get(id) ?? 0) || '?'} - {todosById.get(id)?.content ?? id} + + {todosById.get(id)?.content ?? id} + ))} @@ -1439,7 +1443,9 @@ export function PlanExecutionView({ type="button" > {(stepNumberByTodo.get(id) ?? 0) || '?'} - {todosById.get(id)?.content ?? id} + + {todosById.get(id)?.content ?? id} + ))} diff --git a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx index a66f37596cf..0b7d1f2f64e 100644 --- a/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx +++ b/packages/web-shell/client/components/workflow/SessionWorkflowInspector.test.tsx @@ -193,6 +193,34 @@ describe('SessionWorkflowInspector', () => { act(() => upstream?.click()); expect(onSelectedTodoIdChange).toHaveBeenCalledWith('prepare'); + // Downstream too: the two directions render through the same helper but + // read from different sources (`blockedBy` vs the projection's + // `dependentsByTodo`), so covering one does not cover the other. + act(() => { + root.render( + + + , + ); + }); + const downstream = container.querySelector( + '[data-testid="workflow-dependency-ship"]', + ); + expect(downstream?.textContent).toContain('2'); + expect(downstream?.textContent).toContain('Ship result'); + onSelectedTodoIdChange.mockClear(); + act(() => downstream?.click()); + expect(onSelectedTodoIdChange).toHaveBeenCalledWith('ship'); + act(() => root.unmount()); container.remove(); }); From e6d9c044099ef8a9d81ea1375d5cbfae0d3602b4 Mon Sep 17 00:00:00 2001 From: yiliang114 Date: Fri, 4 Sep 2026 11:06:30 +0000 Subject: [PATCH 11/26] fix(web-shell): drop ghost dependency controls and mark them interactive The step-details dependency buttons mapped raw selectedTodo.blockedBy, so an unresolvable step id rendered as a clickable "? " control; clicking it selected a ghost id, emptied selectedTodo, and hid the details panel mid-navigation. Render from the same filtered projection the edges draw from (dependencyIdsByTodo: unknown ids, duplicates, and self-references dropped) and hide the section when nothing resolvable remains. Also add the data-plan-interactive marker to both step-details dependency button groups. The host keyboard handlers in ToolApproval and TasksStatusMessage isolate plan controls through that marker, and these were the only interactive controls in the view without it. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtmourmvi4 --- .../messages/PlanExecutionView.test.tsx | 50 +++++++++++++++++++ .../components/messages/PlanExecutionView.tsx | 13 ++++- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx index 20cdf5e4dea..d8ced777699 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx @@ -539,10 +539,19 @@ describe('PlanExecutionView', () => { '[data-plan-dependency="research"]', ); expect(upstreamLink?.tagName).toBe('BUTTON'); + // The host keyboard handlers isolate plan controls through this marker + // (ToolApproval/TasksStatusMessage early-return on it), so these buttons + // must carry it like every other control in the view. + expect(upstreamLink?.hasAttribute('data-plan-interactive')).toBe(true); expect(upstreamLink?.textContent).toBe('1Research'); expect( details?.querySelector('[data-plan-dependency="verify"]')?.textContent, ).toBe('3Verify'); + expect( + details + ?.querySelector('[data-plan-dependency="verify"]') + ?.hasAttribute('data-plan-interactive'), + ).toBe(true); expect(details?.textContent).not.toContain('Depends on: research'); expect(details?.textContent).toContain('Subagents'); expect(details?.textContent).toContain( @@ -576,6 +585,47 @@ describe('PlanExecutionView', () => { container.remove(); }); + it('drops unresolvable dependency ids from the step-details controls', () => { + // blockedBy is model-authored todo_write output parsed without + // normalization, so it can name a step that does not exist. Such an id + // must not render as a control: clicking it would select a ghost id, + // empty selectedTodo, and hide the panel mid-navigation. + const ghostTodos = todos.map((todo) => + todo.id === 'build' + ? { ...todo, blockedBy: ['research', 'ghost-step'] } + : todo, + ); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + act(() => { + container + .querySelector('[data-plan-node-id="build"]') + ?.click(); + }); + const details = container.querySelector('[data-plan-step-details]'); + expect(details).not.toBeNull(); + // The resolvable dependency stays a working control… + expect( + details?.querySelector('[data-plan-dependency="research"]'), + ).not.toBeNull(); + // …while the ghost id renders none at all. + expect( + details?.querySelector('[data-plan-dependency="ghost-step"]'), + ).toBeNull(); + expect(details?.textContent).not.toContain('ghost-step'); + + act(() => root.unmount()); + container.remove(); + }); + // R7-2: a replayed transcript of an interrupted session carries Agent tool // calls still in_progress (or paused) with NO live daemon task. The node // badges render Running/Paused off executionStatus's transcript fallback; diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.tsx index ec1579e3072..c715e9e27e2 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.tsx @@ -566,6 +566,7 @@ export function PlanExecutionView({ activeAgentCount, attentionCount, topology, + dependencyIdsByTodo, topologyKey, dependencyCount, hasDependencies, @@ -936,6 +937,12 @@ export function PlanExecutionView({ const selectedDependents = selectedTodo ? (dependentsByTodo.get(selectedTodo.id) ?? []) : []; + // The same filtered projection the edges draw from: the topology builder + // drops ids that name no step (and self-references), so no control here + // can select a ghost id and hide this panel mid-navigation. + const selectedDependencies = selectedTodo + ? (dependencyIdsByTodo.get(selectedTodo.id) ?? []) + : []; const detailsId = `plan-step-details-${graphId}`; const overallProgressId = `plan-overall-progress-${graphId}`; @@ -1410,12 +1417,13 @@ export function PlanExecutionView({ chips on the node face these references can be controls: each one selects the step it names, which is what makes the dependency list the graph's navigation. */} - {(selectedTodo.blockedBy?.length ?? 0) > 0 && ( + {selectedDependencies.length > 0 && (
    {t('planExecution.dependsOn')} - {selectedTodo.blockedBy!.map((id) => ( + {selectedDependencies.map((id) => (
    - {/* Rendered whenever nothing else states the - dependency: drawn edges are aria-hidden, so they - state it only visually. Above - MAX_RENDERED_PLAN_EDGES no edges draw at all, and - with the details panel off (the cockpit) or - selection disabled (document mode) the panel never - states it either. */} - {(!drawsDependencyEdges || - !showStepDetails || - documentMode) && + {statesDependenciesVisibly && faceDependencies.length > 0 && (
    {t('planExecution.dependsOn')} @@ -1417,6 +1428,28 @@ export function PlanExecutionView({ ))}
    )} + {/* The interactive graph draws the dependency instead + of stating it, and drawn edges are aria-hidden — so + without this the node's accessible name stops + naming its blockers and a screen-reader user has to + activate every node to find out what blocks it. + Same sr-only channel as the status word, same + step-number-plus-title labels as the chips, and it + never brings the visible row back. */} + {!statesDependenciesVisibly && + faceDependencies.length > 0 && ( + + {t('planExecution.dependsOn')}{' '} + {faceDependencies + .map( + (id) => + `${(stepNumberByTodo.get(id) ?? 0) || '?'} ${ + todosById.get(id)?.content ?? id + }`, + ) + .join(', ')} + + )} {executions.length > 0 && (
    From 5f70a13866fd678a5fb3bd6ef81e6b0664496d55 Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sun, 13 Sep 2026 14:23:48 +0800 Subject: [PATCH 24/26] fix(web-shell): count node-face agents from the same sources the rows render R8-2: agentCount read only transcript subTools, while renderExecution also renders live child tasks from the task index, so a live child with no transcript entry rendered a row the tally never counted. Count the same deduped union the rows come from. Co-authored-by: Qwen-Coder --- .../messages/PlanExecutionView.test.tsx | 66 ++++++++++++++++++- .../components/messages/PlanExecutionView.tsx | 27 ++++++-- 2 files changed, 84 insertions(+), 9 deletions(-) diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx index ddf30aaa921..ff22cf4b24e 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx @@ -586,10 +586,14 @@ describe('PlanExecutionView', () => { // the dependency chips, and the agent count and elapsed carry the "is // this alive" signal onto the face. expect(buildNode?.textContent).toContain('Build'); - expect(buildNode?.textContent).toContain('1 agent'); + // Root agent plus the live child task below (parentAgentId set, no + // transcript entry): the node renders two agent rows, so the face tally + // must read 2 — counting only transcript subTools is the R8-2 divergence. + expect(buildNode?.textContent).toContain('2 agents'); expect(buildNode?.textContent).toContain('1m 5s'); - // The singular branch above and the plural branch here are separate - // formatter paths; `1 agents` would ship green with only one of them. + // The plural branch is pinned above; `1 agents` would ship green without + // the negative assertion here. The singular branch is pinned in + // 'the node-face agent tally matches the rows it renders'. const researchNode = container .querySelector('[data-plan-node-id="research"]') ?.closest('article'); @@ -1839,6 +1843,62 @@ describe('PlanExecutionView', () => { container.remove(); }); + it('the node-face agent tally matches the rows it renders', () => { + // R8-2 witness: a live child task whose toolUseId matches no subTools + // callId renders a row (via toolForNestedTask) while the transcript-only + // tally never saw it — the node read "1 agent" beside two agent rows. + const rootTool: ACPToolCall = { + ...agentTool('build'), + subTools: [ + { + callId: 'call-nested', + toolName: 'Agent', + title: 'Nested agent', + status: 'in_progress', + }, + ], + }; + const rootTask = task('running'); + const liveChild = task('running', { + id: 'agent-live-child', + label: 'Live child', + toolUseId: 'call-live-child', + parentAgentId: rootTask.id, + }); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + + const buildNode = container + .querySelector('[data-plan-node-id="build"]') + ?.closest('article'); + // Three rows render: root, the transcript nested agent, and the live + // child — and the tally counts the same three. + expect(buildNode?.textContent).toContain('3 agents'); + expect(buildNode?.textContent).toContain('Nested agent'); + expect(buildNode?.textContent).toContain('Live child'); + // The lone root on the research node keeps the singular path pinned. + const researchNode = container + .querySelector('[data-plan-node-id="research"]') + ?.closest('article'); + expect(researchNode?.textContent).toContain('1 agent'); + expect(researchNode?.textContent).not.toContain('1 agents'); + + act(() => root.unmount()); + container.remove(); + }); + it('renders no agent tally for a non-agent tool tagged to the step', () => { // Declaring todo_id does not make a tool an agent: the tally hides // rather than printing "0 agents" beside the tool's row. diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.tsx index 663bcf74fcc..dde97308673 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.tsx @@ -1293,14 +1293,29 @@ export function PlanExecutionView({ // Agents, not bare executions: a nested subagent counts too, // matching the rows this node renders and the inspector's // Subagents list for the same step. The runtime above stays - // on roots, so nested time is not summed twice. - const agentCount = executions.reduce( - (count, tool) => + // on roots, so nested time is not summed twice. Count from + // the same two sources renderExecution draws the rows from — + // live child tasks from the task index plus transcript + // subTools, deduped by toolUseId exactly as the rows are — + // otherwise a live child task with no transcript entry + // renders a row the count never sees. + const agentCount = executions.reduce((count, tool) => { + const liveNested = nestedTasksFromIndex(tool, taskIndex); + const liveCallIds = new Set( + liveNested.flatMap(({ task }) => + task.toolUseId ? [task.toolUseId] : [], + ), + ); + const transcriptOnly = nestedAgentToolsForTool(tool).filter( + ({ tool: nested }) => !liveCallIds.has(nested.callId), + ); + return ( count + (isSubAgentToolCall(tool) ? 1 : 0) + - nestedAgentToolsForTool(tool).length, - 0, - ); + liveNested.length + + transcriptOnly.length + ); + }, 0); // blockedBy is model-authored and can repeat an id — or name // the todo itself. Dedup and drop self-references like the // topology builder; ghost ids stay, because above the edge From d91a0f7e740c052dd5cff52d329e5c6ab73343a8 Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sun, 13 Sep 2026 16:49:51 +0800 Subject: [PATCH 25/26] fix(web-shell): keep layer-skipping plan lanes out of the step they pass (#10938) wenshao's round-2 verification names two merge conditions on 5f70a13. At <=480px the router gave every layer-skipping edge a fixed 24px shoulder. The 18px gutter leaves a 10px run, so the vertical segment landed inside the intervening lane: a dependency from step 3 to step 5 read as one into step 4, and the SVG paints under the nodes so the lane just disappeared into it. Derive each shoulder from the gutter measured on that side, the way the adjacent-layer edge already does, and halve the corner radius so the run into the arrowhead keeps a positive length. The 64px desktop tier has a 56px run, so min(24, 28) is still 24 and every desktop path stays byte-identical; the new witness pins the exact desktop path string. The node-face agent tally dedups by toolUseId exactly as the rows do, but nothing exercised the overlap. Add a fixture where a live child task and a transcript subTool share one toolUseId and assert it counts once (2 agents beside 2 rows); dropping the filter reads 3 agents. Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtzjtfna1y --- .../messages/PlanExecutionView.test.tsx | 170 ++++++++++++++++++ .../components/messages/PlanExecutionView.tsx | 72 ++++++-- 2 files changed, 232 insertions(+), 10 deletions(-) diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx index ff22cf4b24e..482a685ec71 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.test.tsx @@ -1511,6 +1511,121 @@ describe('PlanExecutionView', () => { } }); + it('keeps a layer-skipping lane out of the step it passes', () => { + // The other half of R6-3. The router gave every layer-skipping edge a + // fixed 24px shoulder, which is wider than the ≤480px gutter (18px, a + // 10px run). At 390/430px the rise sat 10px inside the intervening step, + // so a dependency from step 1 to step 3 read as one into step 2 — and the + // SVG paints under the nodes, so the lane simply disappeared into it. + // Restoring `startX + 24` / `endX - 24` turns the 18px tier red. + const skip: TodoItem[] = [ + { id: 'one', content: 'One', status: 'completed' }, + { id: 'two', content: 'Two', status: 'pending', blockedBy: ['one'] }, + { + id: 'three', + content: 'Three', + status: 'pending', + // The layer-skipping edge: `one` is layer 0, `three` is layer 2. + blockedBy: ['two', 'one'], + }, + ]; + // The lane width the 700px tier narrows to; only the gutter varies. + const lane = 168; + // gutter → 64px (≥721px), 32px (≤720px), 18px (≤480px). + for (const gap of [64, 32, 18]) { + const rect = (left: number, top: number, width: number, height: number) => + ({ + x: left, + y: top, + left, + top, + width, + height, + right: left + width, + bottom: top + height, + toJSON: () => ({}), + }) as DOMRect; + // One node per layer here, so the step the lane passes is all of layer 1. + const leftOf = (layer: number) => layer * (lane + gap); + const rectSpy = vi + .spyOn(HTMLElement.prototype, 'getBoundingClientRect') + .mockImplementation(function () { + if (this.parentElement?.hasAttribute('data-plan-workflow')) { + return rect(100, 50, 1100, 300); + } + if (this.tagName === 'ARTICLE') { + const id = this.querySelector('[data-plan-node-id]')!.getAttribute( + 'data-plan-node-id', + )!; + const layer = id === 'one' ? 0 : id === 'two' ? 1 : 2; + return rect(100 + leftOf(layer), 60, lane, 80); + } + return rect(0, 0, 0, 0); + }); + const widthSpy = vi + .spyOn(HTMLElement.prototype, 'offsetWidth', 'get') + .mockReturnValue(1100); + const heightSpy = vi + .spyOn(HTMLElement.prototype, 'offsetHeight', 'get') + .mockReturnValue(300); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + try { + act(() => { + root.render( + + + , + ); + }); + + const d = container + .querySelector('[data-from="one"][data-to="three"]') + ?.getAttribute('d'); + expect(d, `${gap}px gutter drew no lane`).toBeTruthy(); + // The route's four turns, as [control x, end x]: Q1/Q2 carry dropX, + // Q3/Q4 carry riseX. Those are the two vertical segments' columns. + const turns = [ + ...d!.matchAll(/Q ([-\d.]+) [-\d.]+ ([-\d.]+) [-\d.]+/g), + ].map((match) => [Number(match[1]), Number(match[2])]); + expect(turns).toHaveLength(4); + const dropX = turns[0][0]; + const riseX = turns[2][0]; + const startX = lane + 4; + const endX = leftOf(2) - 4; + // The lane may only turn inside its own gutter, never inside layer 1. + expect(dropX, `${gap}px drop column`).toBeGreaterThan(startX); + expect(dropX, `${gap}px drop column`).toBeLessThan(leftOf(1)); + expect(riseX, `${gap}px rise column`).toBeGreaterThan(leftOf(1) + lane); + expect(riseX, `${gap}px rise column`).toBeLessThan(endX); + // The corner is halved at 18px so the run into the arrowhead keeps a + // positive length; at zero the head flips back at its source. + expect(Number(/H ([-\d.]+)$/.exec(d!)![1])).toBe(endX); + expect( + endX - turns[3][1], + `${gap}px gutter end tangent x`, + ).toBeGreaterThan(0); + + if (gap === 64) { + // A 64px gutter affords the full 24px shoulder on both sides, so + // desktop geometry is untouched. Pinned as a golden path because + // the requirement is byte-identical, not merely non-crossing. + expect(d).toBe( + 'M 172 50 H 190 Q 196 50 196 56 V 98 Q 196 104 202 104 ' + + 'H 430 Q 436 104 436 98 V 56 Q 436 50 442 50 H 460', + ); + } + } finally { + act(() => root.unmount()); + container.remove(); + rectSpy.mockRestore(); + widthSpy.mockRestore(); + heightSpy.mockRestore(); + } + } + }); + it('does not synchronously remeasure unchanged topology on task polling', () => { const rectSpy = vi.spyOn(HTMLElement.prototype, 'getBoundingClientRect'); const container = document.createElement('div'); @@ -1899,6 +2014,61 @@ describe('PlanExecutionView', () => { container.remove(); }); + it('counts an agent seen as both a live task and a transcript sub-tool once', () => { + // The tally counts the same deduped union the rows render from, so an + // agent observed through BOTH a live child task and a persisted subTools + // entry is one agent, not two. Dropping the `!liveCallIds.has(…)` filter + // reads "3 agents" beside two rows and turns this red (mutant M20). + const rootTool: ACPToolCall = { + ...agentTool('build'), + subTools: [ + { + callId: 'call-nested', + toolName: 'Agent', + title: 'Nested agent', + status: 'in_progress', + }, + ], + }; + const rootTask = task('running'); + // The live child carries the same toolUseId as the sub-tool above: one + // agent, two observations of it. + const liveChild = task('running', { + id: 'agent-live-child', + label: 'Nested agent', + toolUseId: 'call-nested', + parentAgentId: rootTask.id, + }); + const container = document.createElement('div'); + document.body.appendChild(container); + const root = createRoot(container); + act(() => { + root.render( + + + , + ); + }); + + const buildNode = container + .querySelector('[data-plan-node-id="build"]') + ?.closest('article'); + expect(buildNode?.textContent).toContain('2 agents'); + // Root row plus one nested row: the tally and the rows agree because they + // dedup the same way. + expect(buildNode?.textContent).toContain('Nested agent'); + expect( + buildNode?.querySelectorAll(`.${styles.nestedExecution}`), + ).toHaveLength(1); + + act(() => root.unmount()); + container.remove(); + }); + it('renders no agent tally for a non-agent tool tagged to the step', () => { // Declaring todo_id does not make a tool an agent: the tally hides // rather than printing "0 agents" beside the tool's row. diff --git a/packages/web-shell/client/components/messages/PlanExecutionView.tsx b/packages/web-shell/client/components/messages/PlanExecutionView.tsx index dde97308673..7a2764ca6c5 100644 --- a/packages/web-shell/client/components/messages/PlanExecutionView.tsx +++ b/packages/web-shell/client/components/messages/PlanExecutionView.tsx @@ -62,6 +62,12 @@ const EMPTY_GRAPH_LAYOUT: PlanGraphLayout = { const EDGE_LANE_HEIGHT = 9; /** Corner radius where an orthogonal edge turns. */ const EDGE_CORNER = 6; +/** + * Widest horizontal shoulder a layer-spanning edge runs before turning down + * into its return lane. It is a ceiling, never a floor: a narrow gutter takes + * half of its own run instead. See the router for why. + */ +const EDGE_SHOULDER = 24; const MAX_RENDERED_PLAN_EDGES = 500; @@ -791,6 +797,9 @@ export function PlanExecutionView({ ? graphRect.height / graphElement.offsetHeight : 1; const measuredNodes = new Map(); + // Horizontal extent of every topological layer, so an edge that skips + // a layer can size its shoulders from the gutter it really crosses. + const layerBounds = new Map(); let maxNodeBottom = 0; for (const [todoId, node] of nodeRefs.current) { const rect = node.getBoundingClientRect(); @@ -805,6 +814,17 @@ export function PlanExecutionView({ } as DOMRect; measuredNodes.set(todoId, normalizedRect); maxNodeBottom = Math.max(maxNodeBottom, normalizedRect.bottom); + const layer = layerByTodoRef.current.get(todoId) ?? 0; + const bounds = layerBounds.get(layer); + if (bounds) { + bounds.left = Math.min(bounds.left, normalizedRect.left); + bounds.right = Math.max(bounds.right, normalizedRect.right); + } else { + layerBounds.set(layer, { + left: normalizedRect.left, + right: normalizedRect.right, + }); + } } const edges: PlanEdgePath[] = []; const spanning: Array<{ @@ -868,8 +888,40 @@ export function PlanExecutionView({ maxNodeBottom + 14 + lane * EDGE_LANE_HEIGHT, Math.max(graphHeight - 6, maxNodeBottom + 14), ); - const dropX = edge.startX + 24; - const riseX = edge.endX - 24; + // Shoulders derived from the gutter each side actually has, the same + // way the adjacent-layer edge already does it — not a fixed 24px. The + // ≤480px gutter is 18px, so a 24px shoulder left a 10px run and put + // the vertical segment inside the intervening lane: a dependency from + // step 3 to step 5 looked like it entered step 4, and the SVG paints + // under the nodes so the lane just vanished into it. Halving the run + // centres the segment in its own gutter. EDGE_SHOULDER still wins + // wherever the gutter can afford it (the 64px desktop tier has a 56px + // run), so wide lanes keep the exact curve they had. + const sourceLayer = layerByTodoRef.current.get(edge.from) ?? 0; + const targetLayer = layerByTodoRef.current.get(edge.to) ?? 0; + // An unmeasured neighbouring layer falls back to the full shoulder. + const nextLayerLeft = + layerBounds.get(sourceLayer + 1)?.left ?? Number.POSITIVE_INFINITY; + const prevLayerRight = + layerBounds.get(targetLayer - 1)?.right ?? Number.NEGATIVE_INFINITY; + const dropShoulder = Math.min( + EDGE_SHOULDER, + (nextLayerLeft - 4 - edge.startX) / 2, + ); + const riseShoulder = Math.min( + EDGE_SHOULDER, + (edge.endX - prevLayerRight - 4) / 2, + ); + // The corner has to fit inside both shoulders. At 18px the shoulder + // is 5, so the unhalved 6px radius left a zero-length final segment + // and the arrowhead lost its direction again. + const corner = Math.min( + EDGE_CORNER, + dropShoulder / 2, + riseShoulder / 2, + ); + const dropX = edge.startX + dropShoulder; + const riseX = edge.endX - riseShoulder; const down = routeY > edge.startY ? 1 : -1; const up = edge.endY > routeY ? 1 : -1; edges.push({ @@ -877,14 +929,14 @@ export function PlanExecutionView({ to: edge.to, d: `M ${edge.startX} ${edge.startY} ` + - `H ${dropX - EDGE_CORNER} ` + - `Q ${dropX} ${edge.startY} ${dropX} ${edge.startY + EDGE_CORNER * down} ` + - `V ${routeY - EDGE_CORNER * down} ` + - `Q ${dropX} ${routeY} ${dropX + EDGE_CORNER} ${routeY} ` + - `H ${riseX - EDGE_CORNER} ` + - `Q ${riseX} ${routeY} ${riseX} ${routeY + EDGE_CORNER * up} ` + - `V ${edge.endY - EDGE_CORNER * up} ` + - `Q ${riseX} ${edge.endY} ${riseX + EDGE_CORNER} ${edge.endY} ` + + `H ${dropX - corner} ` + + `Q ${dropX} ${edge.startY} ${dropX} ${edge.startY + corner * down} ` + + `V ${routeY - corner * down} ` + + `Q ${dropX} ${routeY} ${dropX + corner} ${routeY} ` + + `H ${riseX - corner} ` + + `Q ${riseX} ${routeY} ${riseX} ${routeY + corner * up} ` + + `V ${edge.endY - corner * up} ` + + `Q ${riseX} ${edge.endY} ${riseX + corner} ${edge.endY} ` + `H ${edge.endX}`, }); }); From 132f2b275e56e2175155a59f48ac95ebc6349e5b Mon Sep 17 00:00:00 2001 From: "jinjing.zzj" Date: Sun, 13 Sep 2026 19:43:27 +0800 Subject: [PATCH 26/26] ci(web-shell): re-fire the head CI lanes that never scheduled The Qwen Code CI, Web-shell Visuals and review-lane runs created for d91a0f7e at 2026-09-13T08:50:29Z never scheduled a single job: a repo-wide Actions queue jam in that window left 59 runs stuck at status=queued with zero jobs, and both cancel and rerun are refused on them (cancel reports 'completed', rerun reports 'already running'). Runs created after ~09:30Z schedule normally, so these are dead rather than slow. No content change. This empty commit only re-fires pull_request:synchronize so the lanes actually run on the exact tree wenshao validated in round 3 ('merge once CI has actually run green on this head'). Co-authored-by: Qwen-Coder Patrol-Run: qwen-pr-closeout/jmtzq8wyy28