From e535dd0dfea35eaa0fe9d76489c3085515c12548 Mon Sep 17 00:00:00 2001 From: Yiheng Tao Date: Sat, 13 Jun 2026 21:50:23 -0700 Subject: [PATCH 1/3] feat(coc): hide Thread/Agents toggle until the chat has sub-agents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Agents canvas is only meaningful once a chat has actually spawned sub-agents. ChatDetail now derives `hasSubAgents` from the agent-run tree and: - hides the Thread/Agents toggle when there are none, and - renders from `effectiveView` (= `view` only when sub-agents exist, otherwise `thread`) so a stale `?view=agents` deep-link can't strand the user on an empty canvas with no toggle to escape — it instead "waits", revealing the canvas the moment the first sub-agent appears. The thread-only flow cards (Ralph start, Implement-plan) also key off `effectiveView`, so they still show on the thread fallback. Adds a static-analysis test for the gating wiring (mirroring the other ChatDetail source tests in that folder) and updates the coc-knowledge dashboard-spa reference. The data contract this relies on (no `Task` calls ⇒ no children) is already covered in agent-canvas-data.test.ts. Co-Authored-By: Claude Opus 4.8 --- .../coc-knowledge/references/dashboard-spa.md | 9 +++- .../client/react/features/chat/ChatDetail.tsx | 16 +++++-- .../repos/ChatDetail-agents-toggle.test.ts | 48 +++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 packages/coc/test/server/spa/client/repos/ChatDetail-agents-toggle.test.ts diff --git a/.github/skills/coc-knowledge/references/dashboard-spa.md b/.github/skills/coc-knowledge/references/dashboard-spa.md index f7bdbdb93..ea12a93f1 100644 --- a/.github/skills/coc-knowledge/references/dashboard-spa.md +++ b/.github/skills/coc-knowledge/references/dashboard-spa.md @@ -303,8 +303,13 @@ under `features/chat/agent-canvas/`) via its `viewToggle` slot. `ChatDetail` owns the `view` state and, in `agents` mode, swaps the `ConversationArea` inner row for `AgentCanvas` — a pannable/zoomable spatial tree of the chat's recursive sub-agent runs — while keeping the composer/scratchpad and hiding the -thread-only flow cards (Ralph start, Implement-plan). The toggle is hidden in -the `floating` variant and while loading/pending. In the main inline context +thread-only flow cards (Ralph start, Implement-plan). The toggle is hidden when +the chat has no sub-agents (`hasSubAgents = agentRoot.children.length > 0`), in +the `floating` variant, and while loading/pending. Rendering keys off +`effectiveView` (= `view` only when sub-agents exist, otherwise `thread`), so a +stale `?view=agents` deep-link can't strand the user on an empty canvas — it +"waits", revealing the canvas the moment the first sub-agent appears. In the +main inline context the view is deep-linked: a `?view=agents` query param on the chat hash (`#repos///?view=agents`) is read on mount (so a shared/bookmarked URL reopens straight into the canvas) and written via diff --git a/packages/coc/src/server/spa/client/react/features/chat/ChatDetail.tsx b/packages/coc/src/server/spa/client/react/features/chat/ChatDetail.tsx index 41c5f480c..6d2122b11 100644 --- a/packages/coc/src/server/spa/client/react/features/chat/ChatDetail.tsx +++ b/packages/coc/src/server/spa/client/react/features/chat/ChatDetail.tsx @@ -318,6 +318,14 @@ export function ChatDetail({ taskId, onBack, workspaceId, isPopOut = false, vari status: effectiveStatus, }), [turns, task?.customTitle, task?.title, task?.displayName, title, effectiveStatus]); + // The Agents view only makes sense once this chat has actually spawned + // sub-agents. With none, hide the Thread/Agents toggle and pin the thread, + // so a stale `?view=agents` deep-link can't strand the user on an empty + // canvas with no toggle to escape. Keeping `view` state untouched means a + // deep-link "waits": the canvas appears the moment the first sub-agent does. + const hasSubAgents = agentRoot.children.length > 0; + const effectiveView: ChatView = hasSubAgents ? view : 'thread'; + // The inspector's "Open in thread" action: switch to the thread and scroll // to the run's turn. const openAgentInThread = useCallback((node: AgentRunNode) => { @@ -1670,7 +1678,7 @@ export function ChatDetail({ taskId, onBack, workspaceId, isPopOut = false, vari onRenameTitle={processId ? () => setRenameOpen(true) : undefined} onStartFreshSameContext={onStartFreshSameContext} startingFreshSameContext={startingFreshSameContext} - viewToggle={!loading && !isPending && variant !== 'floating' + viewToggle={hasSubAgents && !loading && !isPending && variant !== 'floating' ? : undefined} /> @@ -1701,7 +1709,7 @@ export function ChatDetail({ taskId, onBack, workspaceId, isPopOut = false, vari > {/* Inner row: ConversationArea + MiniMap, or the Agents canvas */}
- {view === 'agents' ? ( + {effectiveView === 'agents' ? ( ) : ( <> @@ -1761,7 +1769,7 @@ export function ChatDetail({ taskId, onBack, workspaceId, isPopOut = false, vari )}
{/* Ralph grilling complete — show Start Ralph panel (thread view only) */} - {view === 'thread' && (() => { + {effectiveView === 'thread' && (() => { const ralphCtx = getRalphContext(task); const goalPath = detectedGoalFile || (task?.metadata?.goalFilePath as string | undefined) || ''; // Path 1: traditional grilling-phase → start @@ -1804,7 +1812,7 @@ export function ChatDetail({ taskId, onBack, workspaceId, isPopOut = false, vari return null; })()} {/* Plan file complete — offer one-click handoff to autopilot (thread view only) */} - {view === 'thread' && isTerminal && !planChatBusy && resolveLoadedTaskMode(task) === 'ask' && effectivePlanPath && ( + {effectiveView === 'thread' && isTerminal && !planChatBusy && resolveLoadedTaskMode(task) === 'ask' && effectivePlanPath && ( { + it('derives hasSubAgents from the agent tree children', () => { + expect(source).toMatch(/const\s+hasSubAgents\s*=\s*agentRoot\.children\.length\s*>\s*0/); + }); + + it('pins the rendered view to thread when there are no sub-agents', () => { + expect(source).toMatch(/const\s+effectiveView\s*:\s*ChatView\s*=\s*hasSubAgents\s*\?\s*view\s*:\s*'thread'/); + }); + + it('gates the Thread/Agents toggle on hasSubAgents', () => { + // The toggle must be hidden when no sub-agents exist; `hasSubAgents` + // is the first guard in the viewToggle expression. + expect(source).toMatch(/viewToggle=\{hasSubAgents\s*&&/); + }); + + it('renders the canvas from effectiveView, not the raw view state', () => { + expect(source).toMatch(/effectiveView\s*===\s*'agents'\s*\?/); + expect(source).not.toMatch(/\{view\s*===\s*'agents'\s*\?/); + }); + + it('gates the thread-only side panels on effectiveView', () => { + // Ralph start + ImplementPlan cards should show when we fall back to + // the thread, so they must key off effectiveView rather than `view`. + expect(source).not.toMatch(/\{view\s*===\s*'thread'\s*&&/); + const threadGuards = source.match(/effectiveView\s*===\s*'thread'\s*&&/g) ?? []; + expect(threadGuards.length).toBeGreaterThanOrEqual(2); + }); +}); From 303523a603fe64599365f7ed29011bc5c0250193 Mon Sep 17 00:00:00 2001 From: Yiheng Tao Date: Sat, 13 Jun 2026 22:09:11 -0700 Subject: [PATCH 2/3] fix(coc): don't zoom the canvas when scrolling inside an overlay MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agents-canvas wheel-to-zoom handler in the shared `useZoomPan` hook ran for any wheel event bubbling to the canvas container, including ones from inside the open AgentInspector panel — so scrolling the panel zoomed the canvas instead of scrolling the panel's content. The wheel handler now skips events whose target is inside a `[data-no-drag]` region — the same opt-out the pan-drag handler already honors. The inspector (and the toolbar/legend, and the DAG charts' zoom controls) already carry `data-no-drag`, so their content scrolls natively while the canvas surface still zooms as before. Adds real-DOM wheel tests to useZoomPan.test.ts (the existing tests poke a mock ref and never attach the native wheel listener) covering both the zoom-over-surface and no-zoom-inside-overlay cases, and notes the opt-out in the coc-knowledge dashboard-spa reference. Co-Authored-By: Claude Opus 4.8 --- .../coc-knowledge/references/dashboard-spa.md | 3 ++ .../spa/client/react/hooks/ui/useZoomPan.ts | 6 +++ .../test/spa/react/hooks/useZoomPan.test.ts | 46 ++++++++++++++++++- 3 files changed, 54 insertions(+), 1 deletion(-) diff --git a/.github/skills/coc-knowledge/references/dashboard-spa.md b/.github/skills/coc-knowledge/references/dashboard-spa.md index ea12a93f1..578ed21fe 100644 --- a/.github/skills/coc-knowledge/references/dashboard-spa.md +++ b/.github/skills/coc-knowledge/references/dashboard-spa.md @@ -335,6 +335,9 @@ centered (`centerContent`), re-centering on mount/growth/resize until the user takes over. The toolbar's % is a dropdown of preset levels (25/50/75/100/150/200% + Fit) backed by `useZoomPan`'s `zoomTo(scale)` (zooms about the viewport center); the Fit button zooms to fit the whole tree. +`useZoomPan`'s wheel-zoom and pan-drag both skip events originating inside a +`[data-no-drag]` overlay — the toolbar, legend, and the open inspector — so +those scroll/click natively instead of zooming/panning the canvas behind them. It renders curved SVG edges + node cards (role glyph, name, live elapsed, spawn-count pill, status dot, progress bar) and a live 1s clock for running nodes. Clicking a sub-agent node opens `AgentInspector` — a right-side panel diff --git a/packages/coc/src/server/spa/client/react/hooks/ui/useZoomPan.ts b/packages/coc/src/server/spa/client/react/hooks/ui/useZoomPan.ts index 7446fd6a1..fe2ca01fe 100644 --- a/packages/coc/src/server/spa/client/react/hooks/ui/useZoomPan.ts +++ b/packages/coc/src/server/spa/client/react/hooks/ui/useZoomPan.ts @@ -83,6 +83,12 @@ export function useZoomPan(options: UseZoomPanOptions): UseZoomPanReturn { const onWheel = (e: WheelEvent) => { if (requireModifierKey && !e.ctrlKey && !e.metaKey) return; + // Don't hijack the wheel over interactive overlays (toolbars, an + // open inspector panel, etc.). They opt out with [data-no-drag] — + // the same marker the pan-drag handler honors — so their own + // content can scroll natively instead of zooming the canvas behind. + const target = e.target as HTMLElement | null; + if (target?.closest('[data-no-drag]')) return; e.preventDefault(); e.stopPropagation(); diff --git a/packages/coc/test/spa/react/hooks/useZoomPan.test.ts b/packages/coc/test/spa/react/hooks/useZoomPan.test.ts index 0a7b00617..dcf9a3f31 100644 --- a/packages/coc/test/spa/react/hooks/useZoomPan.test.ts +++ b/packages/coc/test/spa/react/hooks/useZoomPan.test.ts @@ -1,9 +1,32 @@ import { describe, it, expect } from 'vitest'; -import { renderHook, act } from '@testing-library/react'; +import { renderHook, act, render, fireEvent } from '@testing-library/react'; +import { createElement, type ReactElement } from 'react'; import { useZoomPan } from '../../../../src/server/spa/client/react/hooks/ui/useZoomPan'; const defaultOptions = { contentWidth: 400, contentHeight: 200 }; +/** + * A real-DOM harness: the wheel listener is attached in an effect to + * `containerRef.current`, so it only exists when the ref points at a live node + * (the `renderHook` tests above poke a mock ref and never exercise the wheel). + * The container holds a bare canvas surface and a `[data-no-drag]` overlay + * (mirroring the inspector/toolbar) so we can dispatch wheel events at each. + */ +function ZoomPanHarness(): ReactElement { + const { containerRef, zoomLabel } = useZoomPan(defaultOptions); + return createElement( + 'div', + { ref: containerRef, 'data-testid': 'container' }, + createElement('div', { 'data-testid': 'surface' }), + createElement( + 'div', + { 'data-no-drag': true }, + createElement('div', { 'data-testid': 'overlay-content' }), + ), + createElement('span', { 'data-testid': 'label' }, zoomLabel), + ); +} + describe('useZoomPan', () => { it('initial state is scale=1, translate=(0,0), isDragging=false', () => { const { result } = renderHook(() => useZoomPan(defaultOptions)); @@ -181,3 +204,24 @@ describe('useZoomPan', () => { expect(result.current.state.translateX).toBe(0); }); }); + +describe('useZoomPan wheel handling', () => { + it('zooms when the wheel fires over the canvas surface', () => { + const { getByTestId } = render(createElement(ZoomPanHarness)); + // deltaY < 0 → zoom in by one ZOOM_STEP (0.15) → 115%. + const prevented = fireEvent.wheel(getByTestId('surface'), { deltaY: -100, clientX: 10, clientY: 10 }); + expect(getByTestId('label').textContent).toBe('115%'); + // The handler called preventDefault, so the page/canvas doesn't scroll. + expect(prevented).toBe(false); + }); + + it('does NOT zoom when the wheel fires inside a [data-no-drag] overlay', () => { + // Regression: scrolling inside the inspector panel must scroll the panel, + // not zoom the canvas underneath it. + const { getByTestId } = render(createElement(ZoomPanHarness)); + const notPrevented = fireEvent.wheel(getByTestId('overlay-content'), { deltaY: -100, clientX: 10, clientY: 10 }); + expect(getByTestId('label').textContent).toBe('100%'); + // Not prevented → the browser performs its native scroll of the overlay. + expect(notPrevented).toBe(true); + }); +}); From 5bf7c1583b70bd0482ffcc10be9743e7bee72223 Mon Sep 17 00:00:00 2001 From: Yiheng Tao Date: Sat, 13 Jun 2026 22:57:27 -0700 Subject: [PATCH 3/3] test(coc): de-flake WorkItemDetail chat-lens unsaved-edits assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-save `data-unsaved === 'true'` check read the attribute synchronously right after the title edit, while the post-save check already used `waitFor`. `hasUnsavedChanges` reaches the (mocked) chat lens a render-tick after the edit re-renders the panel, so under CI load the bare read caught a stale 'false' (observed on the macos-latest shard 1 of coc-test). Poll for it with `waitFor`, matching the post-save assertion. Test-only; unrelated to the agents-canvas changes in this PR — the chat stack is fully mocked here, so it surfaced purely as CI flake on this PR's run. Co-Authored-By: Claude Opus 4.8 --- .../test/spa/react/repos/WorkItemDetail.chat-lens.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/coc/test/spa/react/repos/WorkItemDetail.chat-lens.test.tsx b/packages/coc/test/spa/react/repos/WorkItemDetail.chat-lens.test.tsx index 03648df0b..ffc21d2a9 100644 --- a/packages/coc/test/spa/react/repos/WorkItemDetail.chat-lens.test.tsx +++ b/packages/coc/test/spa/react/repos/WorkItemDetail.chat-lens.test.tsx @@ -226,7 +226,12 @@ describe('WorkItemDetail Work Item chat lens', () => { expect(localStorage.getItem(getReviewChatOpenStorageKey(target))).toBe('true'); expect(screen.getByTestId('work-item-chat-lens').getAttribute('data-work-item-id')).toBe('wi-1'); expect(screen.getByTestId('work-item-chat-lens').getAttribute('data-title')).toBe('Saved title one'); - expect(screen.getByTestId('work-item-chat-lens').getAttribute('data-unsaved')).toBe('true'); + // `hasUnsavedChanges` reaches the lens a render-tick after the title edit, + // so poll for it rather than reading synchronously (mirrors the post-save + // assertion below). A bare read flakes under CI load (stale 'false'). + await waitFor(() => + expect(screen.getByTestId('work-item-chat-lens').getAttribute('data-unsaved')).toBe('true'), + ); fireEvent.click(screen.getByTestId('wi-save-btn'));