xiaoxue_admin
/
orca
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix: address review findings (#2807)

This commit is contained in:
Jinjing 2026-05-25 17:43:08 -07:00 committed by GitHub
parent bec6b81cb4
commit 36357dfcb7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 411 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

67
src/renderer/src/components/dashboard/DashboardAgentChildDisclosure.tsx Normal file
View File

@ -0,0 +1,67 @@
import React, { useCallback } from 'react'
import { ChevronRight } from 'lucide-react'
import { cn } from '@/lib/utils'
type Props = {
childAgentCount?: number
childAgentsExpanded: boolean
onToggleChildAgents?: () => void
reserveDisclosureGutter: boolean
}
export function DashboardAgentChildDisclosure({
childAgentCount,
childAgentsExpanded,
onToggleChildAgents,
reserveDisclosureGutter
}: Props) {
const hasChildDisclosure =
typeof childAgentCount === 'number' &&
childAgentCount > 0 &&
typeof onToggleChildAgents === 'function'
const handleToggleChildren = useCallback(
(e: React.MouseEvent<HTMLButtonElement>) => {
e.preventDefault()
e.stopPropagation()
onToggleChildAgents?.()
},
[onToggleChildAgents]
)
const stopMouseDown = useCallback((e: React.MouseEvent) => {
e.stopPropagation()
}, [])
const stopKeyDown = useCallback((e: React.KeyboardEvent) => {
if (e.key === 'Enter' || e.key === ' ') {
e.stopPropagation()
}
}, [])
if (!hasChildDisclosure) {
return reserveDisclosureGutter ? (
<span aria-hidden className="-ml-0.5 inline-block size-4 shrink-0" />
) : null
}
// Why: the chevron owns child disclosure; leaf spacers keep the leading
// state-dot column aligned across the card.
return (
<button
type="button"
onClick={handleToggleChildren}
onMouseDown={stopMouseDown}
onKeyDown={stopKeyDown}
className="-ml-0.5 inline-flex size-4 shrink-0 items-center justify-center rounded-sm text-muted-foreground hover:bg-sidebar-accent hover:text-foreground"
aria-label={`${childAgentsExpanded ? 'Hide' : 'Show'} ${childAgentCount} child ${
childAgentCount === 1 ? 'agent' : 'agents'
}`}
aria-expanded={childAgentsExpanded}
>
<ChevronRight
className={cn(
'size-3 transition-transform duration-150',
childAgentsExpanded && 'rotate-90'
)}
/>
</button>
)
}

42
src/renderer/src/components/dashboard/DashboardAgentRow.tsx
View File

@ -6,6 +6,7 @@ import { AgentIcon } from '@/lib/agent-catalog'
import { agentTypeToIconAgent, formatAgentTypeLabel } from '@/lib/agent-status'
import CommentMarkdown from '@/components/sidebar/CommentMarkdown'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { DashboardAgentChildDisclosure } from './DashboardAgentChildDisclosure'
import type { AgentStatusState } from '../../../../shared/agent-status-types'
import type { DashboardAgentRow as DashboardAgentRowData } from './useDashboardData'
@ -112,6 +113,14 @@ type Props = {
hideExpand?: boolean
/** Reuse the row's hover tint to show the focused terminal pane's agent. */
isFocusedPane?: boolean
// Why: inline-card orchestration rows fold children under a leading chevron.
childAgentCount?: number
childAgentsExpanded?: boolean
onToggleChildAgents?: () => void
// Why: leaf siblings reserve the chevron gutter so state dots align.
reserveDisclosureGutter?: boolean
// Why: chevron indentation replaces fixed-offset lineage connector art.
hideLineageConnectors?: boolean
}
const DashboardAgentRow = React.memo(function DashboardAgentRow({
@ -123,8 +132,17 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
stateDotSize = 'md',
hideIdentityIcon = false,
hideExpand = false,
isFocusedPane = false
isFocusedPane = false,
childAgentCount,
childAgentsExpanded = false,
onToggleChildAgents,
reserveDisclosureGutter = false,
hideLineageConnectors = false
}: Props) {
const hasChildDisclosure =
typeof childAgentCount === 'number' &&
childAgentCount > 0 &&
typeof onToggleChildAgents === 'function'
const [expanded, setExpanded] = useState(false)
// Why: stop propagation so clicking the X doesn't also fire the worktree
// card's click handler (which navigates away from the dashboard).
@ -243,14 +261,14 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
role={participatesInLineage ? 'treeitem' : undefined}
aria-level={participatesInLineage ? (lineage?.depth ?? 0) + 1 : undefined}
>
{lineageChildCount > 0 ? (
{lineageChildCount > 0 && !hideLineageConnectors ? (
<span
aria-hidden
data-agent-lineage-parent-connector
className="pointer-events-none absolute bottom-[-0.75rem] left-[13px] top-[1.05rem] border-l-[1.5px] border-muted-foreground/45 dark:border-muted-foreground/35"
/>
) : null}
{isLineageChild ? (
{isLineageChild && !hideLineageConnectors ? (
<span
aria-hidden
data-agent-lineage-connector={lineage?.isLastSibling === false ? 'branch' : 'last'}
@ -271,6 +289,12 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
</span>
) : null}
<div className="flex items-center gap-1.5">
<DashboardAgentChildDisclosure
childAgentCount={childAgentCount}
childAgentsExpanded={childAgentsExpanded}
onToggleChildAgents={onToggleChildAgents}
reserveDisclosureGutter={reserveDisclosureGutter}
/>
{/* Why: state indicator lives in the leading gutter so the user's
eye can sweep one column and know which rows are working,
waiting, or done at a glance — the list-view convention (Linear,
@ -330,6 +354,18 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
>
{displayLabel}
</span>
{/* Why: "+N" badge mirrors the leading chevron — without it the
parent row reads identical to a leaf row when collapsed, and the
child count is invisible. Hidden when expanded because the
children are visible directly below. */}
{hasChildDisclosure && !childAgentsExpanded && (
<span
className="shrink-0 text-[10px] font-normal leading-none text-muted-foreground/70 tabular-nums"
aria-hidden
>
+{childAgentCount}
</span>
)}
{/* Why: right cluster keeps passive time and dismiss affordance in one
place. State belongs in the leading gutter; repeating it here as
text makes interrupted rows look like the old badge treatment. */}

157
src/renderer/src/components/sidebar/WorktreeCardAgents.test.tsx
View File

@ -1,12 +1,15 @@
import { renderToStaticMarkup } from 'react-dom/server'
import type { ReactNode } from 'react'
import { beforeEach, describe, expect, it, vi } from 'vitest'
let mockAgents = [
let mockAgents: unknown[] = [
{
paneKey: 'tab-1:1',
tab: { id: 'tab-1' },
state: 'working',
entry: {
stateStartedAt: 1000
stateStartedAt: 1000,
orchestration: undefined
}
}
]
@ -31,9 +34,37 @@ vi.mock('@/components/dashboard/useNow', () => ({
}))
vi.mock('@/components/dashboard/DashboardAgentRow', () => ({
default: ({ agent, isFocusedPane }: { agent: { paneKey: string }; isFocusedPane?: boolean }) => (
<div data-testid="agent-row" data-focused={isFocusedPane ? 'true' : 'false'}>
default: ({
agent,
isFocusedPane,
childAgentCount,
childAgentsExpanded,
onToggleChildAgents
}: {
agent: { paneKey: string }
isFocusedPane?: boolean
childAgentCount?: number
childAgentsExpanded?: boolean
onToggleChildAgents?: () => void
}) => (
<div
data-testid="agent-row"
data-focused={isFocusedPane ? 'true' : 'false'}
data-pane-key={agent.paneKey}
>
{agent.paneKey}
{typeof childAgentCount === 'number' && childAgentCount > 0 ? (
<button
type="button"
aria-label={`${childAgentsExpanded ? 'Hide' : 'Show'} ${childAgentCount} child ${
childAgentCount === 1 ? 'agent' : 'agents'
}`}
aria-expanded={childAgentsExpanded ?? false}
onClick={onToggleChildAgents}
>
+{childAgentCount}
</button>
) : null}
</div>
)
}))
@ -42,6 +73,12 @@ vi.mock('./focused-agent-row-highlight', () => ({
useFocusedAgentPaneKey: vi.fn(() => mockFocusedAgentPaneKey)
}))
vi.mock('@/components/ui/tooltip', () => ({
Tooltip: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipContent: ({ children }: { children: ReactNode }) => <>{children}</>,
TooltipTrigger: ({ children }: { children: ReactNode }) => <>{children}</>
}))
describe('WorktreeCardAgents', () => {
beforeEach(() => {
vi.clearAllMocks()
@ -49,15 +86,17 @@ describe('WorktreeCardAgents', () => {
{
paneKey: 'tab-1:1',
tab: { id: 'tab-1' },
state: 'working',
entry: {
stateStartedAt: 1000
stateStartedAt: 1000,
orchestration: undefined
}
}
]
mockFocusedAgentPaneKey = null
})
it('renders rows in a labeled group without the removed per-card toggle header', async () => {
it('renders ordinary rows in a labeled group without a child disclosure', async () => {
const { default: WorktreeCardAgents } = await import('./WorktreeCardAgents')
const markup = renderToStaticMarkup(<WorktreeCardAgents worktreeId="wt-1" />)
@ -91,8 +130,110 @@ describe('WorktreeCardAgents', () => {
const markup = renderToStaticMarkup(<WorktreeCardAgents worktreeId="wt-1" />)
expect(markup).toContain('data-focused="false">tab-1:1')
expect(markup).toContain('data-focused="true">tab-1:2')
expect(markup).toContain('data-focused="false" data-pane-key="tab-1:1"')
expect(markup).toContain('data-focused="true" data-pane-key="tab-1:2"')
})
it('collapses orchestration child agent rows behind a parent disclosure by default', async () => {
mockAgents = [
{
paneKey: 'tab-parent:1',
tab: { id: 'tab-parent' },
state: 'working',
entry: {
stateStartedAt: 1000,
orchestration: undefined
},
lineage: {
depth: 0,
isFirstSibling: true,
isLastSibling: true,
childCount: 1
}
},
{
paneKey: 'tab-child:1',
tab: { id: 'tab-child' },
state: 'done',
entry: {
stateStartedAt: 1500,
orchestration: {
parentPaneKey: 'tab-parent:1'
}
},
lineage: {
depth: 1,
isFirstSibling: true,
isLastSibling: true,
childCount: 0
}
}
]
const { default: WorktreeCardAgents } = await import('./WorktreeCardAgents')
const markup = renderToStaticMarkup(<WorktreeCardAgents worktreeId="wt-1" />)
expect(markup).toContain('role="tree"')
expect(markup).toContain('data-pane-key="tab-parent:1"')
expect(markup).not.toContain('data-pane-key="tab-child:1"')
expect(markup).toContain('aria-label="Show 1 child agent"')
expect(markup).toContain('aria-expanded="false"')
})
it('keeps partially cyclic orchestration rows visible as flat roots', async () => {
mockAgents = [
{
paneKey: 'tab-root:1',
tab: { id: 'tab-root' },
state: 'working',
entry: {
stateStartedAt: 1000,
orchestration: undefined
}
},
{
paneKey: 'tab-cycle-a:1',
tab: { id: 'tab-cycle-a' },
state: 'working',
entry: {
stateStartedAt: 1200,
orchestration: {
parentPaneKey: 'tab-cycle-b:1'
}
},
lineage: {
depth: 0,
isFirstSibling: true,
isLastSibling: false,
childCount: 1
}
},
{
paneKey: 'tab-cycle-b:1',
tab: { id: 'tab-cycle-b' },
state: 'done',
entry: {
stateStartedAt: 1300,
orchestration: {
parentPaneKey: 'tab-cycle-a:1'
}
},
lineage: {
depth: 1,
isFirstSibling: false,
isLastSibling: true,
childCount: 1
}
}
]
const { default: WorktreeCardAgents } = await import('./WorktreeCardAgents')
const markup = renderToStaticMarkup(<WorktreeCardAgents worktreeId="wt-1" />)
expect(markup).toContain('data-pane-key="tab-root:1"')
expect(markup).toContain('data-pane-key="tab-cycle-a:1"')
expect(markup).toContain('data-pane-key="tab-cycle-b:1"')
expect(markup).not.toContain('aria-label="Show 1 child agent"')
})
it('does not render the labeled wrapper when there are no agent rows', async () => {

183
src/renderer/src/components/sidebar/WorktreeCardAgents.tsx
View File

@ -1,4 +1,4 @@
import React, { useCallback, useMemo } from 'react'
import React, { useCallback, useMemo, useState } from 'react'
import { useAppStore } from '@/store'
import { activateAndRevealWorktree } from '@/lib/worktree-activation'
import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane'
@ -47,6 +47,70 @@ type BodyProps = {
className?: string
}
type AgentLineageModel = {
rootAgents: DashboardAgentRowData[]
childrenByParentPaneKey: Map<string, DashboardAgentRowData[]>
}
function buildAgentLineageModel(agents: DashboardAgentRowData[]): AgentLineageModel {
const agentPaneKeys = new Set(agents.map((agent) => agent.paneKey))
const childrenByParentPaneKey = new Map<string, DashboardAgentRowData[]>()
const childPaneKeys = new Set<string>()
for (const agent of agents) {
const parentPaneKey = agent.entry.orchestration?.parentPaneKey
if (!parentPaneKey || !agentPaneKeys.has(parentPaneKey)) {
continue
}
childPaneKeys.add(agent.paneKey)
const siblings = childrenByParentPaneKey.get(parentPaneKey)
if (siblings) {
siblings.push(agent)
} else {
childrenByParentPaneKey.set(parentPaneKey, [agent])
}
}
const rootAgents = agents.filter((agent) => !childPaneKeys.has(agent.paneKey))
if (rootAgents.length === 0 && agents.length > 0) {
// Why: malformed orchestration metadata can theoretically form a cycle.
// Keep every row visible instead of recursing forever or hiding the list.
return { rootAgents: agents, childrenByParentPaneKey: new Map() }
}
const reachablePaneKeys = new Set<string>()
const markReachable = (
agent: DashboardAgentRowData,
ancestorPaneKeys: ReadonlySet<string> = new Set()
): void => {
if (reachablePaneKeys.has(agent.paneKey) || ancestorPaneKeys.has(agent.paneKey)) {
return
}
reachablePaneKeys.add(agent.paneKey)
const descendantAncestorPaneKeys = new Set(ancestorPaneKeys)
descendantAncestorPaneKeys.add(agent.paneKey)
for (const childAgent of childrenByParentPaneKey.get(agent.paneKey) ?? []) {
markReachable(childAgent, descendantAncestorPaneKeys)
}
}
for (const rootAgent of rootAgents) {
markReachable(rootAgent)
}
for (const agent of agents) {
if (reachablePaneKeys.has(agent.paneKey)) {
continue
}
// Why: a partial cycle alongside a valid root has no true root, so it
// would otherwise disappear. Render malformed participants as flat rows
// and drop their child edges, matching the dashboard lineage fallback.
rootAgents.push(agent)
childrenByParentPaneKey.delete(agent.paneKey)
}
return { rootAgents, childrenByParentPaneKey }
}
const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({
worktreeId,
agents,
@ -125,46 +189,111 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({
// don't pay any timer cost.
const now = useNow(30_000)
const hasLineage = agents.some((agent) => agent.lineage && agent.lineage.depth > 0)
const { rootAgents, childrenByParentPaneKey } = useMemo(
() => buildAgentLineageModel(agents),
[agents]
)
const [expandedLineageParents, setExpandedLineageParents] = useState<ReadonlySet<string>>(
() => new Set()
)
const toggleLineageParent = useCallback((paneKey: string) => {
setExpandedLineageParents((current) => {
const next = new Set(current)
if (next.has(paneKey)) {
next.delete(paneKey)
} else {
next.add(paneKey)
}
return next
})
}, [])
const stopBubble = useCallback((e: React.MouseEvent) => {
e.stopPropagation()
}, [])
// Why: when any root row has a disclosure chevron, leaf siblings reserve a
// matching leading spacer so the state-dot column stays aligned across the
// card. Without this, parent rows shift right by the chevron's width while
// leaf rows hug the gutter — visible misalignment when the user sweeps the
// leading column.
const anyRootHasChildren = rootAgents.some(
(agent) => (childrenByParentPaneKey.get(agent.paneKey) ?? []).length > 0
)
const renderAgentBranch = (
agent: DashboardAgentRowData,
ancestorPaneKeys: ReadonlySet<string> = new Set()
): React.ReactNode => {
if (ancestorPaneKeys.has(agent.paneKey)) {
// Why: orchestration metadata is external state and can be malformed.
// Bail out of repeated ancestors instead of recursing forever.
return null
}
const childAgents = childrenByParentPaneKey.get(agent.paneKey) ?? []
const hasChildAgents = childAgents.length > 0
const expanded = expandedLineageParents.has(agent.paneKey)
const descendantAncestorPaneKeys = new Set(ancestorPaneKeys)
descendantAncestorPaneKeys.add(agent.paneKey)
return (
<React.Fragment key={agent.paneKey}>
<DashboardAgentRow
agent={agent}
onDismiss={handleDismissAgent}
onActivate={handleActivateAgentTab}
now={now}
// Why: bold an agent row until the user has visited its tab.
// useAutoAckViewedAgent acks automatically when the user
// focuses the agent's tab, which mutes the row in lockstep.
isUnvisited={unvisitedByPaneKey[agent.paneKey] ?? false}
// Why: inline rows pack tighter than a full-panel layout;
// 'md' reads as a second ~12px glyph users confuse with the
// agent identity icon right next to it. 'sm' keeps the two
// distinguishable at a glance.
stateDotSize="sm"
// Why: in the per-card inline list clicking the row jumps
// directly to the agent, so the expand chevron is redundant.
// Keep the identity glyph (Claude/Gemini/…) so users can tell
// agents apart at a glance within a worktree.
hideExpand
// Why: fold orchestration children under the parent row's leading
// chevron so a parent reads as a tree node, not as a separate
// disclosure stripe below it. Variant B in the mockups.
childAgentCount={hasChildAgents ? childAgents.length : undefined}
childAgentsExpanded={expanded}
onToggleChildAgents={
hasChildAgents ? () => toggleLineageParent(agent.paneKey) : undefined
}
// Why: keep leaf rows aligned with parent rows in the same card —
// see anyRootHasChildren above.
reserveDisclosureGutter={anyRootHasChildren && !hasChildAgents}
isFocusedPane={agent.paneKey === focusedAgentPaneKey}
// Why: the disclosure variant uses chevron + indentation to show
// hierarchy. The legacy L-connector / vertical-trunk decorations
// are pinned to a fixed left offset that doesn't match the
// chevron-shifted column and read as floating fragments.
hideLineageConnectors
/>
{hasChildAgents && expanded
? childAgents.map((childAgent) =>
renderAgentBranch(childAgent, descendantAncestorPaneKeys)
)
: null}
</React.Fragment>
)
}
return (
// Why: swallow bubbling so clicks on the gutter around the agent rows
// don't reach WorktreeCard's activate / edit-meta handlers.
<div
className={cn('flex flex-col mt-1 mb-1 divide-y divide-border/30', className)}
className={cn('flex flex-col mt-1 mb-1', className)}
onClick={stopBubble}
onDoubleClick={stopBubble}
role={hasLineage ? 'tree' : 'group'}
aria-label="Agents"
>
{agents.map((agent) => (
<div key={agent.paneKey}>
<DashboardAgentRow
agent={agent}
onDismiss={handleDismissAgent}
onActivate={handleActivateAgentTab}
now={now}
// Why: bold an agent row until the user has visited its tab.
// useAutoAckViewedAgent acks automatically when the user
// focuses the agent's tab, which mutes the row in lockstep.
isUnvisited={unvisitedByPaneKey[agent.paneKey] ?? false}
// Why: inline rows pack tighter than a full-panel layout;
// 'md' reads as a second ~12px glyph users confuse with the
// agent identity icon right next to it. 'sm' keeps the two
// distinguishable at a glance.
stateDotSize="sm"
// Why: in the per-card inline list clicking the row jumps
// directly to the agent, so the expand chevron is redundant.
// Keep the identity glyph (Claude/Gemini/…) so users can tell
// agents apart at a glance within a worktree.
hideExpand
isFocusedPane={agent.paneKey === focusedAgentPaneKey}
/>
</div>
))}
{rootAgents.map((rootAgent) => renderAgentBranch(rootAgent))}
</div>
)
})