fix: address review findings (#2667)

This commit is contained in:
Jinjing
2026-05-22 23:42:40 -07:00
committed by GitHub
parent 79b18f6fed
commit de96ac5678
12 changed files with 395 additions and 12 deletions
+3 -1
View File
@@ -453,6 +453,7 @@ function openMainWindow(): BrowserWindow {
if (mainWindow?.isDestroyed()) {
return
}
const orchestration = runtime?.getAgentStatusOrchestrationContextForPaneKey(paneKey)
mainWindow?.webContents.send('agentStatus:set', {
...payload,
paneKey,
@@ -460,7 +461,8 @@ function openMainWindow(): BrowserWindow {
worktreeId,
connectionId,
receivedAt,
stateStartedAt
stateStartedAt,
...(orchestration ? { orchestration } : {})
})
recordCrashBreadcrumb('agent_state_changed', {
agentType: payload.agentType ?? 'unknown',
+62
View File
@@ -7,6 +7,7 @@ import {
isShellProcess
} from '../../shared/agent-detection'
import type { AgentStatus } from '../../shared/agent-detection'
import type { AgentStatusOrchestrationContext } from '../../shared/agent-status-types'
import { gitExecFileAsync, wslAwareSpawn } from '../git/runner'
import { isWslPath, parseWslPath, getWslHome } from '../wsl'
import { createHash, randomUUID } from 'crypto'
@@ -10183,6 +10184,67 @@ export class OrcaRuntimeService {
}
}
getAgentStatusOrchestrationContextForPaneKey(
paneKey: string
): AgentStatusOrchestrationContext | undefined {
const handle = this.getTerminalHandleForPaneKey(paneKey)
if (!handle) {
return undefined
}
const db = this.getOrchestrationDbIfAvailable()
const dispatch = db?.getActiveDispatchForTerminal(handle)
if (!dispatch) {
return undefined
}
const task = db?.getTask(dispatch.task_id)
const activeRun = db?.getActiveCoordinatorRun()
const parentTerminalHandle =
task?.created_by_terminal_handle ??
(activeRun?.coordinator_handle && activeRun.coordinator_handle !== handle
? activeRun.coordinator_handle
: undefined)
const parentPaneKey = parentTerminalHandle
? this.getPaneKeyForTerminalHandle(parentTerminalHandle)
: undefined
return {
taskId: dispatch.task_id,
dispatchId: dispatch.id,
...(parentTerminalHandle ? { parentTerminalHandle } : {}),
...(parentPaneKey ? { parentPaneKey } : {}),
...(activeRun?.coordinator_handle ? { coordinatorHandle: activeRun.coordinator_handle } : {}),
...(activeRun?.id ? { orchestrationRunId: activeRun.id } : {})
}
}
private getTerminalHandleForPaneKey(paneKey: string): string | null {
const parsed = parsePaneKey(paneKey)
if (parsed) {
const leaf = this.leaves.get(this.getLeafKey(parsed.tabId, parsed.leafId))
if (leaf?.ptyId) {
return this.issueHandle(leaf)
}
}
for (const pty of this.ptysById.values()) {
if (pty.paneKey === paneKey) {
return this.issuePtyHandle(pty)
}
}
return null
}
private getPaneKeyForTerminalHandle(handle: string): string | null {
const livePty = this.getLivePtyForHandle(handle)
if (livePty?.pty.paneKey) {
return livePty.pty.paneKey
}
const record = this.handles.get(handle)
if (!record || record.runtimeId !== this.runtimeId) {
return null
}
return makePaneKey(record.tabId, record.leafId)
}
// Why: OSC title detection via onPtyData is the tightest signal for agent
// presence, but the runtime may not see PTY data for daemon-hosted terminals
// (the daemon adapter stubs getForegroundProcess). This checks three signals
@@ -173,4 +173,54 @@ describe('DashboardAgentRow', () => {
expect(interruptedIndex).toBeGreaterThan(promptIndex)
expect(markup).not.toContain('lucide-circle-check')
})
it('renders orchestration child rows with a connector and tree level', () => {
const markup = renderRow(
makeAgent({
lineage: {
depth: 1,
isFirstSibling: true,
isLastSibling: true,
childCount: 0
}
})
)
expect(markup).toContain('data-agent-lineage-connector="last"')
expect(markup).toContain('role="treeitem"')
expect(markup).toContain('aria-level="2"')
expect(classTokens(markup)).toContain('pl-5')
expect(classTokens(markup)).toContain('left-[13px]')
expect(classTokens(markup)).toContain('top-[0.7rem]')
expect(classTokens(markup)).toContain('w-1.5')
expect(classTokens(markup)).toContain('border-l-[1.5px]')
expect(classTokens(markup)).toContain('border-t-[1.5px]')
expect(classTokens(markup)).toContain('border-muted-foreground/45')
})
it('annotates parent identity icon when it dispatched children', () => {
const markup = renderToStaticMarkup(
<TooltipProvider>
<DashboardAgentRow
agent={makeAgent({
lineage: {
depth: 0,
isFirstSibling: true,
isLastSibling: true,
childCount: 2
}
})}
onDismiss={vi.fn()}
onActivate={vi.fn()}
now={NOW}
hideExpand
/>
</TooltipProvider>
)
expect(markup).toContain('title="Codex - dispatched 2 agents"')
expect(markup).toContain('data-agent-lineage-parent-connector="true"')
expect(classTokens(markup)).toContain('left-[13px]')
expect(markup).toContain('aria-level="1"')
})
})
@@ -185,6 +185,16 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
const toolInput = isWorking ? (agent.entry.toolInput?.trim() ?? '') : ''
const lastAssistantMessage = agent.entry.lastAssistantMessage?.trim() ?? ''
const isInterrupted = agent.entry.interrupted === true
const lineage = agent.lineage
const isLineageChild = lineage?.depth === 1
const lineageChildCount = lineage?.childCount ?? 0
const participatesInLineage = isLineageChild || lineageChildCount > 0
const identityTitle =
lineageChildCount > 0
? `${formatAgentTypeLabel(agent.agentType)} - dispatched ${lineageChildCount} ${
lineageChildCount === 1 ? 'agent' : 'agents'
}`
: formatAgentTypeLabel(agent.agentType)
// Why: interrupted is a terminal outcome the user needs to scan in the
// leading state column; the secondary-line text below provides the
// explanation without competing with the prompt or timestamp.
@@ -218,7 +228,8 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
className={cn(
// Why: this row owns the timestamp/X hover boundary; anonymous
// ancestor groups from workspace cards must not reveal every row's X.
'group/agent-row relative flex flex-col -ml-2 px-2 py-1',
'group/agent-row relative flex flex-col -ml-2 py-1',
isLineageChild ? 'pl-5 pr-2' : 'px-2',
// Why: hover tints have to go in opposite directions per theme —
// dark mode adds light on dark (bg-accent/30), light mode needs to
// add *dark* on white. Alpha-on-accent in light mode collapses to
@@ -228,7 +239,36 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
'cursor-pointer rounded-sm hover:bg-black/[0.06] dark:hover:bg-accent/30'
)}
title={tsParts.length > 0 ? tsParts.join(' • ') : undefined}
role={participatesInLineage ? 'treeitem' : undefined}
aria-level={participatesInLineage ? (lineage?.depth ?? 0) + 1 : undefined}
>
{lineageChildCount > 0 ? (
<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 ? (
<span
aria-hidden
data-agent-lineage-connector={lineage?.isLastSibling === false ? 'branch' : 'last'}
className="pointer-events-none absolute bottom-[-1px] left-[13px] top-[-1px] w-3"
>
<span
className={cn(
'absolute left-0 border-l-[1.5px] border-muted-foreground/45 dark:border-muted-foreground/35',
lineage?.isFirstSibling ? 'top-[-0.9rem]' : 'top-[-1px]',
lineage?.isLastSibling
? lineage?.isFirstSibling
? 'h-[1.6rem]'
: 'h-[calc(0.7rem+1px)]'
: 'bottom-[-1px]'
)}
/>
<span className="absolute left-0 top-[0.7rem] w-1.5 border-t-[1.5px] border-muted-foreground/45 dark:border-muted-foreground/35" />
</span>
) : null}
<div className="flex items-center gap-1.5">
{/* Why: state indicator lives in the leading gutter so the user's
eye can sweep one column and know which rows are working,
@@ -257,7 +297,7 @@ const DashboardAgentRow = React.memo(function DashboardAgentRow({
them — keeping the icon only on the prompt row lets the sub-rows
indent under the prompt text cleanly. */}
{!hideIdentityIcon && (
<span className="inline-flex shrink-0" title={formatAgentTypeLabel(agent.agentType)}>
<span className="inline-flex shrink-0" title={identityTitle}>
<AgentIcon agent={agentTypeToIconAgent(agent.agentType)} size={14} />
</span>
)}
@@ -0,0 +1,89 @@
import type { DashboardAgentRow } from './useDashboardData'
export type AgentRowLineagePresentation = {
depth: 0 | 1
isFirstSibling: boolean
isLastSibling: boolean
childCount: number
}
export type DashboardAgentRowWithLineage = DashboardAgentRow & {
lineage: AgentRowLineagePresentation
}
const ROOT_LINEAGE: AgentRowLineagePresentation = {
depth: 0,
isFirstSibling: true,
isLastSibling: true,
childCount: 0
}
export function applyAgentRowLineage(rows: DashboardAgentRow[]): DashboardAgentRowWithLineage[] {
if (rows.length <= 1) {
return rows.map((row) => ({ ...row, lineage: ROOT_LINEAGE }))
}
const rowsByPaneKey = new Map(rows.map((row) => [row.paneKey, row]))
const childrenByParentPaneKey = new Map<string, DashboardAgentRow[]>()
const childPaneKeys = new Set<string>()
for (const row of rows) {
const parentPaneKey = row.entry.orchestration?.parentPaneKey
if (!parentPaneKey || !rowsByPaneKey.has(parentPaneKey)) {
continue
}
childPaneKeys.add(row.paneKey)
const siblings = childrenByParentPaneKey.get(parentPaneKey)
if (siblings) {
siblings.push(row)
} else {
childrenByParentPaneKey.set(parentPaneKey, [row])
}
}
if (childPaneKeys.size === 0) {
return rows.map((row) => ({ ...row, lineage: ROOT_LINEAGE }))
}
const ordered: DashboardAgentRowWithLineage[] = []
const emitted = new Set<string>()
const emitRow = (row: DashboardAgentRow, lineage: AgentRowLineagePresentation): boolean => {
if (emitted.has(row.paneKey)) {
return false
}
emitted.add(row.paneKey)
ordered.push({ ...row, lineage })
return true
}
const emitSubtree = (row: DashboardAgentRow, lineage: AgentRowLineagePresentation): void => {
const children = childrenByParentPaneKey.get(row.paneKey) ?? []
if (!emitRow(row, { ...lineage, childCount: children.length })) {
return
}
children.forEach((child, index) => {
emitSubtree(child, {
// Why: nested dispatches should still stay under their nearest
// visible parent, but visual depth remains capped so dense sidebar
// rows don't lose too much prompt width.
depth: 1,
isFirstSibling: index === 0,
isLastSibling: index === children.length - 1,
childCount: 0
})
})
}
for (const row of rows) {
if (childPaneKeys.has(row.paneKey)) {
continue
}
emitSubtree(row, ROOT_LINEAGE)
}
for (const row of rows) {
emitRow(row, ROOT_LINEAGE)
}
return ordered
}
@@ -24,6 +24,12 @@ export type DashboardAgentRow = {
* stateHistory entry, falling back to updatedAt when no history exists yet.
* Used to sort agents by when they started. */
startedAt: number
lineage?: {
depth: 0 | 1
isFirstSibling: boolean
isLastSibling: boolean
childCount: number
}
}
// Why: the shape here is deliberately minimal. The per-card rendering pipeline
@@ -122,6 +122,7 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({
// never mount this component (see WorktreeCardAgents), so idle worktrees
// don't pay any timer cost.
const now = useNow(30_000)
const hasLineage = agents.some((agent) => agent.lineage && agent.lineage.depth > 0)
const stopBubble = useCallback((e: React.MouseEvent) => {
e.stopPropagation()
@@ -134,7 +135,7 @@ const WorktreeCardAgentsBody = React.memo(function WorktreeCardAgentsBody({
className={cn('flex flex-col mt-1 mb-1 divide-y divide-border/30', className)}
onClick={stopBubble}
onDoubleClick={stopBubble}
role="group"
role={hasLineage ? 'tree' : 'group'}
aria-label="Agents"
>
{agents.map((agent) => (
@@ -12,11 +12,14 @@ import {
selectMigrationUnsupportedEntriesForWorktree,
selectRetainedAgentEntriesForWorktree
} from './useWorktreeAgentRows'
import { applyAgentRowLineage } from '@/components/dashboard/agent-row-lineage'
import { makePaneKey } from '../../../../shared/stable-pane-id'
const ORPHAN_PANE_KEY = makePaneKey('tab-orphan', '11111111-1111-4111-8111-111111111111')
const PANE_KEY_1 = makePaneKey('tab-1', '22222222-2222-4222-8222-222222222222')
const PANE_KEY_2 = makePaneKey('tab-2', '33333333-3333-4333-8333-333333333333')
const PANE_KEY_3 = makePaneKey('tab-3', '55555555-5555-4555-8555-555555555555')
const PANE_KEY_4 = makePaneKey('tab-4', '66666666-6666-4666-8666-666666666666')
function makeTab(id: string): TerminalTab {
return {
@@ -115,6 +118,110 @@ describe('buildWorktreeAgentRows', () => {
})
})
describe('applyAgentRowLineage', () => {
it('places orchestration children immediately after their parent', () => {
const parent = makeEntry(PANE_KEY_2, 2000, {
prompt: 'parent'
})
const firstChild = makeEntry(PANE_KEY_1, 1000, {
prompt: 'first child',
orchestration: {
taskId: 'task-1',
dispatchId: 'ctx-1',
parentTerminalHandle: 'term-parent',
parentPaneKey: PANE_KEY_2
}
})
const secondChild = makeEntry(PANE_KEY_3, 3000, {
prompt: 'second child',
orchestration: {
taskId: 'task-2',
dispatchId: 'ctx-2',
parentTerminalHandle: 'term-parent',
parentPaneKey: PANE_KEY_2
}
})
const rows = buildWorktreeAgentRows({
tabs: [makeTab('tab-1'), makeTab('tab-2'), makeTab('tab-3')],
entries: [firstChild, parent, secondChild],
retained: [],
now: 4000
})
const ordered = applyAgentRowLineage(rows)
expect(ordered.map((row) => row.paneKey)).toEqual([PANE_KEY_2, PANE_KEY_1, PANE_KEY_3])
expect(ordered[0].lineage).toMatchObject({ depth: 0, childCount: 2 })
expect(ordered[1].lineage).toMatchObject({
depth: 1,
isFirstSibling: true,
isLastSibling: false
})
expect(ordered[2].lineage).toMatchObject({
depth: 1,
isFirstSibling: false,
isLastSibling: true
})
})
it('leaves orphan orchestration rows flat when the parent pane is not visible', () => {
const child = makeEntry(PANE_KEY_1, 1000, {
orchestration: {
taskId: 'task-1',
dispatchId: 'ctx-1',
parentTerminalHandle: 'term-missing',
parentPaneKey: PANE_KEY_2
}
})
const rows = buildWorktreeAgentRows({
tabs: [makeTab('tab-1')],
entries: [child],
retained: [],
now: 2000
})
expect(applyAgentRowLineage(rows)[0].lineage).toMatchObject({ depth: 0, childCount: 0 })
})
it('keeps nested dispatches under their nearest visible parent', () => {
const parent = makeEntry(PANE_KEY_1, 1000, { prompt: 'parent' })
const child = makeEntry(PANE_KEY_2, 2000, {
prompt: 'child',
orchestration: {
taskId: 'task-child',
dispatchId: 'ctx-child',
parentPaneKey: PANE_KEY_1
}
})
const grandchild = makeEntry(PANE_KEY_3, 3000, {
prompt: 'grandchild',
orchestration: {
taskId: 'task-grandchild',
dispatchId: 'ctx-grandchild',
parentPaneKey: PANE_KEY_2
}
})
const sibling = makeEntry(PANE_KEY_4, 4000, { prompt: 'sibling root' })
const rows = buildWorktreeAgentRows({
tabs: [makeTab('tab-1'), makeTab('tab-2'), makeTab('tab-3'), makeTab('tab-4')],
entries: [parent, child, grandchild, sibling],
retained: [],
now: 5000
})
const ordered = applyAgentRowLineage(rows)
expect(ordered.map((row) => row.paneKey)).toEqual([
PANE_KEY_1,
PANE_KEY_2,
PANE_KEY_3,
PANE_KEY_4
])
expect(ordered[1].lineage).toMatchObject({ depth: 1, childCount: 1 })
expect(ordered[2].lineage).toMatchObject({ depth: 1, childCount: 0 })
})
})
describe('selectMigrationUnsupportedEntriesForWorktree', () => {
it('returns raw migration records so shallow selectors can cache snapshots', () => {
const unsupported: MigrationUnsupportedPtyEntry = {
@@ -13,6 +13,7 @@ import {
} from '../../../../shared/agent-status-types'
import { parsePaneKey } from '../../../../shared/stable-pane-id'
import { migrationUnsupportedToAgentStatusEntry } from '@/lib/migration-unsupported-agent-entry'
import { applyAgentRowLineage } from '@/components/dashboard/agent-row-lineage'
// Why: stable empty-array references so narrow selectors return the same
// reference when there's nothing for this worktree. Without stable empties,
@@ -324,12 +325,14 @@ export function useWorktreeAgentRows(worktreeId: string): DashboardAgentRow[] {
})
]
: liveEntries
return buildWorktreeAgentRows({
tabs: tabs ?? [],
entries,
retained,
now
})
return applyAgentRowLineage(
buildWorktreeAgentRows({
tabs: tabs ?? [],
entries,
retained,
now
})
)
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [tabs, liveEntries, migrationUnsupported, retained, agentStatusEpoch])
}
+4 -1
View File
@@ -1977,7 +1977,10 @@ export function useIpcEvents(): void {
) {
return 'dropped'
}
store.setAgentStatus(data.paneKey, payload, title, {
const statusPayload = data.orchestration
? { ...payload, orchestration: data.orchestration }
: payload
store.setAgentStatus(data.paneKey, statusPayload, title, {
updatedAt: data.receivedAt,
stateStartedAt: data.stateStartedAt
})
@@ -6,6 +6,7 @@ import {
AGENT_STATE_HISTORY_MAX,
type AgentStateHistoryEntry,
type AgentStatusEntry,
type AgentStatusOrchestrationContext,
type AgentType,
type MigrationUnsupportedPtyEntry,
type ParsedAgentStatusPayload
@@ -54,7 +55,7 @@ export type AgentStatusSlice = {
/** Update or insert an agent status entry from a status payload. */
setAgentStatus: (
paneKey: string,
payload: ParsedAgentStatusPayload,
payload: ParsedAgentStatusPayload & { orchestration?: AgentStatusOrchestrationContext },
terminalTitle?: string,
timing?: { updatedAt?: number; stateStartedAt?: number }
) => void
@@ -252,6 +253,10 @@ export const createAgentStatusSlice: StateCreator<AppState, [], [], AgentStatusS
toolName: payload.toolName,
toolInput: payload.toolInput,
lastAssistantMessage: payload.lastAssistantMessage,
// Why: orchestration dispatch metadata may disappear after the
// worker completes and the active dispatch closes. Preserve the last
// known parent-child link so done/retained rows stay grouped.
orchestration: payload.orchestration ?? existing?.orchestration,
// Why: interrupted lives on `done` only. parseAgentStatusPayload
// already clamps it to `undefined` for non-done states, so writing
// the field through directly preserves truth for done and resets
@@ -289,6 +294,7 @@ export const createAgentStatusSlice: StateCreator<AppState, [], [], AgentStatusS
entry.toolName !== existing.toolName ||
entry.toolInput !== existing.toolInput ||
entry.lastAssistantMessage !== existing.lastAssistantMessage ||
entry.orchestration !== existing.orchestration ||
entry.interrupted !== existing.interrupted)
const retentionRelevantChange = sortRelevantChange || doneRetentionFieldsChanged
// Why: a new status event means the agent is live again — lift any
+14
View File
@@ -52,6 +52,15 @@ export type AgentStateHistoryEntry = {
/** Maximum number of history entries kept per agent to bound memory. */
export const AGENT_STATE_HISTORY_MAX = 20
export type AgentStatusOrchestrationContext = {
taskId: string
dispatchId: string
parentTerminalHandle?: string
parentPaneKey?: string
coordinatorHandle?: string
orchestrationRunId?: string
}
export type AgentStatusEntry = {
state: AgentStatusState
/** The user's most recent prompt, when the hook payload carried one.
@@ -86,6 +95,10 @@ export type AgentStatusEntry = {
* cancelled it. Undefined while the agent is working or when no interrupt
* signal was available. */
interrupted?: boolean
/** Orchestration dispatch context for agent panes spawned by another agent.
* Why: parent/child agent hierarchy is pane-level state, not worktree
* lineage; workers often run in the same worktree as their coordinator. */
orchestration?: AgentStatusOrchestrationContext
}
export type MigrationUnsupportedPtyEntry = {
@@ -143,6 +156,7 @@ export type AgentStatusIpcPayload = ParsedAgentStatusPayload & {
receivedAt: number
/** Timestamp (ms) when the current state first appeared for this pane. */
stateStartedAt: number
orchestration?: AgentStatusOrchestrationContext
}
/** Maximum character length for the prompt field. Truncated on parse. */