diff --git a/src/main/ipc/dashboard-payload-validation.test.ts b/src/main/ipc/dashboard-payload-validation.test.ts index 3ae3e85fd..afdf4b366 100644 --- a/src/main/ipc/dashboard-payload-validation.test.ts +++ b/src/main/ipc/dashboard-payload-validation.test.ts @@ -50,6 +50,56 @@ describe('dashboard payload validation', () => { ).toBe(false) }) + it('accepts repo icons a pop-out can safely render, and rejects the rest', () => { + expect( + isDashboardSnapshot({ + ...SNAPSHOT, + repoIconsByRepoId: { + 'repo-1': { type: 'lucide', name: 'Rocket' }, + 'repo-2': null, + 'repo-3': { + type: 'image', + src: 'https://github.com/anthropics.png?size=64', + source: 'github' + } + } + }) + ).toBe(true) + // Absent entirely: a pop-out on older code still gets its snapshot. + expect(isDashboardSnapshot({ ...SNAPSHOT, repoIconsByRepoId: undefined })).toBe(true) + + expect( + isDashboardSnapshot({ + ...SNAPSHOT, + repoIconsByRepoId: { + 'repo-1': { type: 'image', src: 'javascript:alert(1)', source: 'file' } + } + }) + ).toBe(false) + expect( + isDashboardSnapshot({ + ...SNAPSHOT, + repoIconsByRepoId: { 'repo-1': { type: 'nonsense' } } + }) + ).toBe(false) + expect(isDashboardSnapshot({ ...SNAPSHOT, repoIconsByRepoId: [] })).toBe(false) + }) + + it('bounds the conversation name', () => { + expect( + isDashboardSnapshot({ + ...SNAPSHOT, + cards: [{ ...SNAPSHOT.cards[0], conversationName: 'Sparse-checkout parser' }] + }) + ).toBe(true) + expect( + isDashboardSnapshot({ + ...SNAPSHOT, + cards: [{ ...SNAPSHOT.cards[0], conversationName: 'x'.repeat(1_025) }] + }) + ).toBe(false) + }) + it('requires complete bounded reveal routing', () => { expect( isDashboardRevealAgentArgs({ diff --git a/src/main/ipc/dashboard-payload-validation.ts b/src/main/ipc/dashboard-payload-validation.ts index 54168d190..69abed2c9 100644 --- a/src/main/ipc/dashboard-payload-validation.ts +++ b/src/main/ipc/dashboard-payload-validation.ts @@ -1,4 +1,5 @@ import type { DashboardRevealAgentArgs, DashboardSnapshot } from '../../shared/dashboard-snapshot' +import { sanitizeRepoIcon } from '../../shared/repo-icon' import { AGENT_STATUS_ASSISTANT_MESSAGE_MAX_LENGTH, AGENT_STATUS_INTERACTIVE_PROMPT_MAX_LENGTH, @@ -7,6 +8,7 @@ import { } from '../../shared/agent-status-types' const MAX_DASHBOARD_CARDS = 1_000 +const MAX_DASHBOARD_REPO_ICONS = 500 const MAX_ID_LENGTH = 4_096 const MAX_LABEL_LENGTH = 1_024 const DASHBOARD_BUCKETS = new Set(['attention', 'working', 'idle']) @@ -50,7 +52,28 @@ export function isDashboardSnapshot(value: unknown): value is DashboardSnapshot isFiniteNumber(snapshot.generatedAt) && Array.isArray(snapshot.cards) && snapshot.cards.length <= MAX_DASHBOARD_CARDS && - snapshot.cards.every(isDashboardCard) + snapshot.cards.every(isDashboardCard) && + isDashboardRepoIcons(snapshot.repoIconsByRepoId) + ) +} + +/** Repo icons reach the pop-out's ``, so each one must survive the + * same sanitizer the settings picker writes through. */ +function isDashboardRepoIcons(value: unknown): boolean { + if (value === undefined) { + return true + } + if (!value || typeof value !== 'object' || Array.isArray(value)) { + return false + } + const entries = Object.entries(value as Record) + return ( + entries.length <= MAX_DASHBOARD_REPO_ICONS && + entries.every( + ([repoId, icon]) => + isBoundedString(repoId, MAX_ID_LENGTH) && + (icon === null || sanitizeRepoIcon(icon) !== undefined) + ) ) } @@ -80,6 +103,7 @@ function isDashboardCard(value: unknown): boolean { (card.finishedAt === null || isFiniteNumber(card.finishedAt)) && isFiniteNumber(card.stateChangedAt) && typeof card.unseen === 'boolean' && - isOptionalBoundedString(card.askSummary, AGENT_STATUS_INTERACTIVE_PROMPT_MAX_LENGTH) + isOptionalBoundedString(card.askSummary, AGENT_STATUS_INTERACTIVE_PROMPT_MAX_LENGTH) && + isOptionalBoundedString(card.conversationName, MAX_LABEL_LENGTH) ) } diff --git a/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.test.tsx b/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.test.tsx index 9a5eed152..941d3cfec 100644 --- a/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.test.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.test.tsx @@ -4,6 +4,7 @@ import '@testing-library/jest-dom/vitest' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react' import type { DashboardCard, DashboardSnapshot } from '../../../../shared/dashboard-snapshot' +import type { RepoIcon } from '../../../../shared/repo-icon' import { AgentKanbanBoard } from './AgentKanbanBoard' // Stub the card and dialog so the board test stays free of xterm / Radix @@ -11,10 +12,12 @@ import { AgentKanbanBoard } from './AgentKanbanBoard' vi.mock('./AgentKanbanCard', () => ({ AgentKanbanCard: ({ card, + repoIcon, now, onOpenTerminal }: { card: DashboardCard + repoIcon?: RepoIcon | null now: number onOpenTerminal: (card: DashboardCard) => void }) => ( @@ -23,6 +26,7 @@ vi.mock('./AgentKanbanCard', () => ({ data-bucket={card.bucket} data-unseen={card.unseen} data-now={now} + data-repo-icon={repoIcon === null ? 'none' : JSON.stringify(repoIcon)} onClick={() => onOpenTerminal(card)} > {card.worktreeName} @@ -70,8 +74,11 @@ function card(overrides: Partial): DashboardCard { } } -function renderBoard(cards: DashboardCard[]): void { - const snapshot: DashboardSnapshot = { generatedAt: 1, cards } +function renderBoard( + cards: DashboardCard[], + repoIconsByRepoId?: Record +): void { + const snapshot: DashboardSnapshot = { generatedAt: 1, cards, repoIconsByRepoId } render() } @@ -108,6 +115,30 @@ describe('AgentKanbanBoard', () => { expect(screen.getByText('3 total')).toBeTruthy() }) + it('leaves every column border neutral now that cards carry the state color', () => { + renderBoard([card({ bucket: 'attention' })]) + for (const column of document.querySelectorAll('section')) { + expect(column.className).toContain('border-border/60') + expect(column.className).not.toContain('amber') + } + }) + + it('routes each card its own repo icon', () => { + renderBoard( + [ + card({ repoId: 'r1', worktreeName: 'from-r1' }), + card({ repoId: 'r2', worktreeName: 'from-r2' }), + card({ repoId: 'r3', worktreeName: 'from-r3' }) + ], + { r1: { type: 'lucide', name: 'Rocket' }, r2: null } + ) + + expect(screen.getByText('from-r1').dataset.repoIcon).toBe('{"type":"lucide","name":"Rocket"}') + expect(screen.getByText('from-r2').dataset.repoIcon).toBe('none') + // Unknown repo → the card's own default glyph, never another repo's icon. + expect(screen.getByText('from-r3').dataset.repoIcon).toBe('none') + }) + it('shows "None" for empty columns', () => { renderBoard([card({ bucket: 'working' })]) // attention and idle are empty → two "None" placeholders. diff --git a/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.tsx b/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.tsx index 01c46072c..b43fc971e 100644 --- a/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentKanbanBoard.tsx @@ -6,7 +6,9 @@ import { type DashboardCard, type DashboardSnapshot } from '../../../../shared/dashboard-snapshot' +import type { RepoIcon } from '../../../../shared/repo-icon' import { cn } from '@/lib/utils' +import { TooltipProvider } from '@/components/ui/tooltip' import { installWindowVisibilityInterval } from '@/lib/window-visibility-interval' import { AgentKanbanCard } from './AgentKanbanCard' import { AgentTerminalDialog, type AgentRevealArgs } from './AgentTerminalDialog' @@ -58,22 +60,20 @@ function groupByBucket(cards: DashboardCard[]): Record | undefined now: number onOpenTerminal: (card: DashboardCard) => void }): React.JSX.Element { - const highlight = bucket === 'attention' && cards.length > 0 return ( -
+ // Why: attention no longer tints the whole column — the cards inside carry + // their own state color, so a column border would double-signal it. +
{bucketLabel(bucket)} @@ -92,6 +92,7 @@ function KanbanColumn({ @@ -192,54 +193,60 @@ export function AgentKanbanBoard({ }, [dialogCard?.unseen, dialogCard?.paneKey, onAckAgent]) return ( -
-
-

- {translate('dashboardPopout.title', 'Agents')} -

- - {translate('dashboardPopout.total', '{{count}} total', { - count: snapshot.cards.length - })} - - {headerActions || onClose ? ( -
- {headerActions} - {onClose ? ( - - ) : null} -
- ) : null} -
-
- {/* Why: columns share the window width up to a readable cap; mx-auto + // Why: the pop-out is its own React root with no app-level provider, and the + // card's repo tooltip needs one in both hosts. Nesting inside the main + // window's provider is harmless. + +
+
+

+ {translate('dashboardPopout.title', 'Agents')} +

+ + {translate('dashboardPopout.total', '{{count}} total', { + count: snapshot.cards.length + })} + + {headerActions || onClose ? ( +
+ {headerActions} + {onClose ? ( + + ) : null} +
+ ) : null} +
+
+ {/* Why: columns share the window width up to a readable cap; mx-auto centers the capped board so leftover space splits evenly instead of pooling on the right. In overflow the auto margins collapse to 0, keeping the left edge reachable while scrolling. */} -
- {DASHBOARD_BUCKET_ORDER.map((bucket) => ( - - ))} +
+ {DASHBOARD_BUCKET_ORDER.map((bucket) => ( + + ))} +
+
- -
+ ) } diff --git a/src/renderer/src/components/dashboard-popout/AgentKanbanCard.test.tsx b/src/renderer/src/components/dashboard-popout/AgentKanbanCard.test.tsx index ebcfee3d2..16739e15d 100644 --- a/src/renderer/src/components/dashboard-popout/AgentKanbanCard.test.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentKanbanCard.test.tsx @@ -4,7 +4,9 @@ import '@testing-library/jest-dom/vitest' import { act, cleanup, render, screen } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { DashboardCard } from '../../../../shared/dashboard-snapshot' +import type { RepoIcon } from '../../../../shared/repo-icon' import { i18n } from '@/i18n/i18n' +import { TooltipProvider } from '@/components/ui/tooltip' import { AgentKanbanCard } from './AgentKanbanCard' const agentIconRender = vi.fn() @@ -42,6 +44,24 @@ function card(overrides: Partial = {}): DashboardCard { } } +function renderCard(props: { + card: DashboardCard + now: number + repoIcon?: RepoIcon | null + onOpenTerminal?: () => void +}): ReturnType { + return render( + + + + ) +} + describe('AgentKanbanCard', () => { beforeEach(async () => { await i18n.changeLanguage('en') @@ -53,9 +73,7 @@ describe('AgentKanbanCard', () => { }) it('does not render an invented age when the start time is unknown', () => { - render( - - ) + renderCard({ card: card({ startedAt: 0 }), now: 2_000_000_000 }) expect(screen.queryByText(/\d+d/)).not.toBeInTheDocument() }) @@ -66,47 +84,160 @@ describe('AgentKanbanCard', () => { dotState: 'waiting', askSummary: 'Approve deploy?' }) - const { container, rerender } = render( - - ) + const { container, rerender } = renderCard({ card: attentionCard, now: 2_000 }) expect(screen.queryByTestId('state-dot')).not.toBeInTheDocument() expect(container.querySelectorAll('.lucide-message-circle-question-mark')).toHaveLength(1) rerender( - + + + ) expect(screen.getByTestId('state-dot')).toBeInTheDocument() expect(container.querySelector('.lucide-message-circle-question-mark')).toBeNull() }) + it('tints attention amber and done green, leaving every other state neutral', () => { + const { container: attention } = renderCard({ + card: card({ bucket: 'attention', dotState: 'waiting' }), + now: 2_000 + }) + expect(attention.querySelector('button')?.className).toContain('border-amber-500/40') + + cleanup() + const { container: done } = renderCard({ + card: card({ bucket: 'idle', dotState: 'done' }), + now: 2_000 + }) + expect(done.querySelector('button')?.className).toContain('border-emerald-500/40') + + cleanup() + const { container: idle } = renderCard({ + card: card({ bucket: 'idle', dotState: 'idle' }), + now: 2_000 + }) + const idleClassName = idle.querySelector('button')?.className ?? '' + expect(idleClassName).toContain('border-border/60') + expect(idleClassName).not.toContain('emerald') + expect(idleClassName).not.toContain('amber') + }) + + it('heads the card with the conversation name and drops the worktree to the footer', () => { + const { container } = renderCard({ + card: card({ lastUserMessage: 'ship it', conversationName: 'Sparse-checkout parser' }), + now: 2_000 + }) + + const [header, footer] = [ + container.querySelector('button')!.firstElementChild!, + container.querySelector('button')!.lastElementChild! + ] + expect(header).toHaveTextContent('Sparse-checkout parser') + expect(header).not.toHaveTextContent('dashboard-review') + expect(footer).toHaveTextContent('dashboard-review') + // The message line is attributed to the user again — the name moved up. + expect(screen.getByText('You')).toBeInTheDocument() + }) + + it('heads the card with the worktree when no name resolves, without repeating it', () => { + const { container } = renderCard({ card: card({ lastUserMessage: 'ship it' }), now: 2_000 }) + + expect(screen.getAllByText('dashboard-review')).toHaveLength(1) + expect(container.querySelector('button')!.firstElementChild).toHaveTextContent( + 'dashboard-review' + ) + }) + + it('shows the repo as an icon labelled with its name instead of inline text', () => { + renderCard({ + card: card(), + now: 2_000, + repoIcon: { type: 'emoji', emoji: '🐳' } + }) + + expect(screen.getByLabelText('Orca')).toBeInTheDocument() + expect(screen.getByText('🐳')).toBeInTheDocument() + }) + it('skips structured-clone rerenders until visible card data or its age changes', () => { const onOpenTerminal = vi.fn() const initial = card({ startedAt: 1_000 }) + const repoIcon: RepoIcon = { type: 'lucide', name: 'Rocket' } const { rerender } = render( - + + + ) expect(agentIconRender).toHaveBeenCalledTimes(1) expect(screen.getByText('1m')).toBeInTheDocument() - rerender() + // A fresh structured clone of identical data — including the repo icon. + rerender( + + + + ) expect(agentIconRender).toHaveBeenCalledTimes(1) rerender( - + + + ) expect(agentIconRender).toHaveBeenCalledTimes(2) expect(screen.getByText('2m')).toBeInTheDocument() }) - it('updates the relative age when the UI language changes', async () => { - render( - + it('rerenders when the repo icon changes', () => { + const onOpenTerminal = vi.fn() + const initial = card({ startedAt: 1_000 }) + const { rerender } = render( + + + ) + expect(agentIconRender).toHaveBeenCalledTimes(1) + + rerender( + + + + ) + expect(agentIconRender).toHaveBeenCalledTimes(2) + }) + + it('updates the relative age when the UI language changes', async () => { + renderCard({ card: card({ startedAt: 1_000 }), now: 121_500 }) expect(screen.getByText('2m')).toBeInTheDocument() await act(async () => { diff --git a/src/renderer/src/components/dashboard-popout/AgentKanbanCard.tsx b/src/renderer/src/components/dashboard-popout/AgentKanbanCard.tsx index e8d08b525..9262077e1 100644 --- a/src/renderer/src/components/dashboard-popout/AgentKanbanCard.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentKanbanCard.tsx @@ -4,8 +4,11 @@ import { MessageCircleQuestion } from 'lucide-react' import { AgentIcon } from '@/lib/agent-catalog' import { agentTypeToIconAgent, formatAgentTypeLabel } from '@/lib/agent-status' import { AgentStateDot } from '@/components/AgentStateDot' +import { RepoIconGlyph } from '@/components/repo/repo-icon' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { cn } from '@/lib/utils' import type { DashboardCard } from '../../../../shared/dashboard-snapshot' +import type { RepoIcon } from '../../../../shared/repo-icon' import { translate } from '@/i18n/i18n' /** Compact "started N ago" (the card is glanceable — coarse units are fine). */ @@ -53,12 +56,34 @@ function sameCard(a: DashboardCard, b: DashboardCard): boolean { a.finishedAt === b.finishedAt && a.stateChangedAt === b.stateChangedAt && a.unseen === b.unseen && - a.askSummary === b.askSummary + a.askSummary === b.askSummary && + a.conversationName === b.conversationName ) } +/** Structural — the icon arrives inside a fresh structured clone each publish, + * so identity alone would re-render every card several times a second. */ +function sameRepoIcon(a: RepoIcon | null | undefined, b: RepoIcon | null | undefined): boolean { + if (a === b) { + return true + } + if (!a || !b || a.type !== b.type) { + return false + } + if (a.type === 'lucide') { + return a.name === (b as typeof a).name + } + if (a.type === 'emoji') { + return a.emoji === (b as typeof a).emoji + } + const image = b as typeof a + return a.src === image.src && a.source === image.source && a.label === image.label +} + type AgentKanbanCardProps = { card: DashboardCard + /** The card repo's icon. null renders the default folder glyph. */ + repoIcon?: RepoIcon | null now: number /** Opens the board-level terminal dialog. The dialog is NOT owned by the * card: bucket moves remount the card, and an embedded dialog would close @@ -68,8 +93,23 @@ type AgentKanbanCardProps = { /** One agent on the kanban board. Clicking opens the board's live terminal dialog. */ export const AgentKanbanCard = memo( - function AgentKanbanCard({ card, now, onOpenTerminal }: AgentKanbanCardProps): React.JSX.Element { + function AgentKanbanCard({ + card, + repoIcon = null, + now, + onOpenTerminal + }: AgentKanbanCardProps): React.JSX.Element { useTranslation() + // Why: the two outcomes worth scanning for get a tinted card — amber for + // "answer me", green for "finished, look at it". Everything else stays + // neutral so the tint keeps meaning something. + const needsYou = card.bucket === 'attention' + const isDone = card.dotState === 'done' + // Why: the session's own name heads the card. Without one the worktree is + // the best heading left — and then the footer drops it rather than say it + // twice. + const heading = card.conversationName ?? card.worktreeName + const worktreeInFooter = card.conversationName !== undefined return (