From 56809d7069639e84eb7b9fc769054335f4b0c9b8 Mon Sep 17 00:00:00 2001 From: Sandy Tao Date: Mon, 4 May 2026 14:54:13 -0700 Subject: [PATCH] fix(cli): make SkillInboxDialog fit and scroll in alternate buffer (#26455) --- .../src/ui/components/InboxDialog.test.tsx | 332 +++++ .../cli/src/ui/components/InboxDialog.tsx | 1121 +++++++++++------ 2 files changed, 1050 insertions(+), 403 deletions(-) diff --git a/packages/cli/src/ui/components/InboxDialog.test.tsx b/packages/cli/src/ui/components/InboxDialog.test.tsx index 08dab23e3c..969b7e9ff4 100644 --- a/packages/cli/src/ui/components/InboxDialog.test.tsx +++ b/packages/cli/src/ui/components/InboxDialog.test.tsx @@ -26,8 +26,13 @@ import { } from '@google/gemini-cli-core'; import { waitFor } from '../../test-utils/async.js'; import { renderWithProviders } from '../../test-utils/render.js'; +import { createMockSettings } from '../../test-utils/settings.js'; import { InboxDialog } from './InboxDialog.js'; +const altBufferSettings = createMockSettings({ + ui: { useAlternateBuffer: true }, +}); + vi.mock('@google/gemini-cli-core', async (importOriginal) => { const original = await importOriginal(); @@ -835,5 +840,332 @@ describe('InboxDialog', () => { consoleErrorSpy.mockRestore(); unmount(); }); + + const tallPatch: InboxPatch = { + fileName: 'tall.patch', + name: 'tall-patch', + entries: [ + { + targetPath: '/repo/.gemini/skills/docs-writer/SKILL.md', + diffContent: [ + '--- /repo/.gemini/skills/docs-writer/SKILL.md', + '+++ /repo/.gemini/skills/docs-writer/SKILL.md', + '@@ -1,4 +1,8 @@', + ' line1', + ' line2', + '+added-1', + '+added-2', + '+added-3', + '+added-4', + ' line3', + ' line4', + ].join('\n'), + }, + ], + }; + + it('alt-buffer: renders a bounded ScrollableList viewport for tall patches', async () => { + // Alt-buffer mode has no terminal scrollback, so the dialog must + // scroll inside itself. ScrollableList renders a `█` thumb when + // content exceeds viewport height — the regression signal that the + // diff is bounded and off-screen content is reachable via PgUp/PgDn. + mockListInboxSkills.mockResolvedValue([]); + mockListInboxPatches.mockResolvedValue([tallPatch]); + mockListInboxMemoryPatches.mockResolvedValue([]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + storage: { + getProjectSkillsDir: vi.fn().mockReturnValue('/repo/.gemini/skills'), + }, + } as unknown as Config; + + const { lastFrame, stdin, unmount, waitUntilReady } = await act( + async () => + renderWithProviders( + , + { + settings: altBufferSettings, + uiState: { terminalHeight: 18 }, + }, + ), + ); + + await waitFor(() => { + expect(lastFrame()).toContain('tall-patch'); + }); + + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + const frame = lastFrame() ?? ''; + expect(frame).toContain('Apply'); + expect(frame).toContain('Dismiss'); + expect(frame).toContain('█'); + }); + + unmount(); + }); + + it('alt-buffer: surfaces PgUp/PgDn in the patch-preview footer', async () => { + mockListInboxSkills.mockResolvedValue([]); + mockListInboxPatches.mockResolvedValue([inboxPatch]); + mockListInboxMemoryPatches.mockResolvedValue([]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + storage: { + getProjectSkillsDir: vi.fn().mockReturnValue('/repo/.gemini/skills'), + }, + } as unknown as Config; + + const { lastFrame, stdin, unmount, waitUntilReady } = await act( + async () => + renderWithProviders( + , + { settings: altBufferSettings }, + ), + ); + + await waitFor(() => { + expect(lastFrame()).toContain('update-docs'); + }); + + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + expect(lastFrame()).toContain('PgUp/PgDn to scroll'); + }); + + unmount(); + }); + + it('non-alt-buffer: clips the diff via DiffRenderer with a "lines hidden" hint', async () => { + // Non-alt-buffer mode uses the codebase's standard bounded + // DiffRenderer + ShowMoreLines + Ctrl+O pattern (matches + // FolderTrustDialog/ThemeDialog). MaxSizedBox emits a + // "... first/last N line(s) hidden ..." hint when it clips, which + // is the regression signal that the diff is bounded. + mockListInboxSkills.mockResolvedValue([]); + mockListInboxPatches.mockResolvedValue([tallPatch]); + mockListInboxMemoryPatches.mockResolvedValue([]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + storage: { + getProjectSkillsDir: vi.fn().mockReturnValue('/repo/.gemini/skills'), + }, + } as unknown as Config; + + const { lastFrame, stdin, unmount, waitUntilReady } = await act( + async () => + renderWithProviders( + , + { uiState: { terminalHeight: 18, constrainHeight: true } }, + ), + ); + + await waitFor(() => { + expect(lastFrame()).toContain('tall-patch'); + }); + + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + expect(lastFrame() ?? '').toMatch(/lines? hidden/); + }); + + unmount(); + }); + + it('non-alt-buffer: surfaces Ctrl+O inline (not in the footer) when the diff overflows', async () => { + // In non-alt-buffer mode the Ctrl+O affordance is rendered inline + // by ShowMoreLines above the footer when the diff is clipped. The + // footer itself stays clean (no PgUp/PgDn or Ctrl+O text) since + // duplicating the hint there would be noisy. + mockListInboxSkills.mockResolvedValue([]); + mockListInboxPatches.mockResolvedValue([tallPatch]); + mockListInboxMemoryPatches.mockResolvedValue([]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + storage: { + getProjectSkillsDir: vi.fn().mockReturnValue('/repo/.gemini/skills'), + }, + } as unknown as Config; + + const { lastFrame, stdin, unmount, waitUntilReady } = await act( + async () => + renderWithProviders( + , + { uiState: { terminalHeight: 18, constrainHeight: true } }, + ), + ); + + await waitFor(() => { + expect(lastFrame()).toContain('tall-patch'); + }); + + await act(async () => { + stdin.write('\r'); + await waitUntilReady(); + }); + + await waitFor(() => { + const frame = lastFrame() ?? ''; + expect(frame).toContain('Ctrl+O'); + expect(frame).not.toContain('PgUp/PgDn to scroll'); + }); + + unmount(); + }); + }); + + it('renders each list row as exactly two lines even with long descriptions', async () => { + // Reproduces the production bug: with the previous renderer, long + // descriptions wrapped onto multiple lines (and the date sibling was + // interleaved into the wrap), making each item 3-5 rows tall and + // breaking the listMaxItemsToShow budget. The fix uses height={2} + // and wrap="truncate-end" on every list row. + const longDescription = + 'This is an extremely long description that would absolutely wrap to ' + + 'multiple lines if rendered without truncation, which used to push the ' + + 'list-phase footer off the bottom of the alternate buffer in production.'; + mockListInboxSkills.mockResolvedValue([ + { + dirName: 'long-skill', + name: 'long-skill', + description: longDescription, + content: '---\nname: x\ndescription: y\n---\n', + }, + ]); + mockListInboxPatches.mockResolvedValue([]); + mockListInboxMemoryPatches.mockResolvedValue([]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + } as unknown as Config; + + const { lastFrame, unmount } = await act(async () => + renderWithProviders( + , + ), + ); + + await waitFor(() => { + expect(lastFrame()).toContain('long-skill'); + }); + + const frame = lastFrame() ?? ''; + expect(frame).not.toContain('production'); + expect(frame).toContain('extremely long description'); + + unmount(); + }); + + it('keeps the list-phase footer on screen with many long-description skills', async () => { + const longDesc = + 'A very long description that would wrap across multiple lines if not ' + + 'truncated, which was causing the dialog body to overflow the bottom ' + + 'of the alternate buffer'; + const manySkills: InboxSkill[] = Array.from({ length: 8 }, (_, i) => ({ + dirName: `skill-${i}`, + name: `skill-${i}`, + description: `${longDesc} (#${i})`, + content: '---\nname: x\ndescription: y\n---\n', + })); + mockListInboxSkills.mockResolvedValue(manySkills); + mockListInboxPatches.mockResolvedValue([]); + mockListInboxMemoryPatches.mockResolvedValue([]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + } as unknown as Config; + + const { lastFrame, unmount } = await act(async () => + renderWithProviders( + , + { uiState: { terminalHeight: 28 } }, + ), + ); + + await waitFor(() => { + const frame = lastFrame() ?? ''; + expect(frame).toContain('Memory Inbox'); + expect(frame).toContain('Esc to close'); + }); + + unmount(); + }); + + it('keeps the list-phase footer on screen on short terminals', async () => { + const manySkills: InboxSkill[] = Array.from({ length: 12 }, (_, i) => ({ + dirName: `skill-${i}`, + name: `Skill ${i}`, + description: `Description ${i}`, + content: '---\nname: Skill\ndescription: Skill\n---\n', + })); + mockListInboxSkills.mockResolvedValue(manySkills); + mockListInboxPatches.mockResolvedValue([inboxPatch]); + mockListInboxMemoryPatches.mockResolvedValue([]); + + const config = { + isTrustedFolder: vi.fn().mockReturnValue(true), + storage: { + getProjectSkillsDir: vi.fn().mockReturnValue('/repo/.gemini/skills'), + }, + } as unknown as Config; + + const { lastFrame, unmount } = await act(async () => + renderWithProviders( + , + { uiState: { terminalHeight: 18 } }, + ), + ); + + await waitFor(() => { + const frame = lastFrame() ?? ''; + expect(frame).toContain('Memory Inbox'); + expect(frame).toContain('Esc to close'); + }); + + unmount(); }); }); diff --git a/packages/cli/src/ui/components/InboxDialog.tsx b/packages/cli/src/ui/components/InboxDialog.tsx index c7471f2567..3da004266c 100644 --- a/packages/cli/src/ui/components/InboxDialog.tsx +++ b/packages/cli/src/ui/components/InboxDialog.tsx @@ -6,16 +6,26 @@ import * as path from 'node:path'; import type React from 'react'; -import { useState, useMemo, useCallback, useEffect } from 'react'; -import { Box, Text, useStdout } from 'ink'; +import { Fragment, useState, useMemo, useCallback, useEffect } from 'react'; +import { Box, Text } from 'ink'; import { theme } from '../semantic-colors.js'; +import { useUIState } from '../contexts/UIStateContext.js'; import { useKeypress } from '../hooks/useKeypress.js'; import { Command } from '../key/keyMatchers.js'; import { useKeyMatchers } from '../hooks/useKeyMatchers.js'; import { BaseSelectionList } from './shared/BaseSelectionList.js'; import type { SelectionListItem } from '../hooks/useSelectionList.js'; import { DialogFooter } from './shared/DialogFooter.js'; -import { DiffRenderer } from './messages/DiffRenderer.js'; +import { + DiffRenderer, + parseDiffWithLineNumbers, + renderDiffLines, + type DiffLine, +} from './messages/DiffRenderer.js'; +import { ScrollableList } from './shared/ScrollableList.js'; +import { ShowMoreLines } from './ShowMoreLines.js'; +import { useAlternateBuffer } from '../hooks/useAlternateBuffer.js'; +import { OverflowProvider } from '../contexts/OverflowContext.js'; import { type Config, type InboxSkill, @@ -215,6 +225,102 @@ function formatDate(isoString: string): string { } } +interface DiffSection { + /** Stable identifier for the section (e.g. patch entry path + index). */ + key: string; + /** Header rendered above the diff body, e.g. file path or "SKILL.md". */ + header: string; + /** Raw unified-diff string. Parsed via parseDiffWithLineNumbers. */ + diffContent: string; +} + +interface DiffViewportItem { + key: string; + /** Pre-rendered React node for this row. */ + element: React.ReactElement; +} + +/** + * A fixed-height, scrollable diff viewer used by the skill, patch, and + * memory-patch preview phases. It flattens one or more DiffSections into + * individual line items so ScrollableList can virtualize and so + * PgUp/PgDn/Shift+arrows move the viewport over arbitrarily long diffs + * without overflowing the alternate buffer. + * + * The visual styling matches DiffRenderer's renderDiffLines path; we share + * that helper instead of nesting DiffRenderer (whose own MaxSizedBox + * wrapping would interfere with virtualization). + */ +const ScrollableDiffViewport: React.FC<{ + sections: DiffSection[]; + width: number; + height: number; + hasFocus: boolean; +}> = ({ sections, width, height, hasFocus }) => { + const items = useMemo(() => { + const result: DiffViewportItem[] = []; + sections.forEach((section, sectionIndex) => { + // Header (with a blank spacer row above for separation between + // sections — skipped above the first section). + if (sectionIndex > 0) { + result.push({ + key: `${section.key}:spacer`, + element: , + }); + } + result.push({ + key: `${section.key}:header`, + element: ( + + {section.header} + + ), + }); + + const parsed: DiffLine[] = parseDiffWithLineNumbers(section.diffContent); + const rendered = renderDiffLines({ + parsedLines: parsed, + filename: section.header, + terminalWidth: width, + }); + rendered.forEach((node, index) => { + result.push({ + key: `${section.key}:line:${index}`, + // renderDiffLines emits ReactNodes with their own keys; wrap each + // in a Fragment so ScrollableList sees a single ReactElement per + // row regardless of node shape. + element: {node}, + }); + }); + }); + return result; + }, [sections, width]); + + const renderItem = useCallback( + ({ item }: { item: DiffViewportItem }) => item.element, + [], + ); + const keyExtractor = useCallback((item: DiffViewportItem) => item.key, []); + // Most diff rows are exactly one line tall; long lines wrap so this is a + // lower bound. ScrollableList re-measures via ResizeObserver, so the + // estimate only matters for initial sizing. + const estimatedItemHeight = useCallback(() => 1, []); + + return ( + + + data={items} + renderItem={renderItem} + keyExtractor={keyExtractor} + estimatedItemHeight={estimatedItemHeight} + hasFocus={hasFocus} + initialScrollIndex={0} + scrollbar={true} + /> + + ); +}; + interface InboxDialogProps { config: Config; onClose: () => void; @@ -229,8 +335,8 @@ export const InboxDialog: React.FC = ({ onReloadMemory, }) => { const keyMatchers = useKeyMatchers(); - const { stdout } = useStdout(); - const terminalWidth = stdout?.columns ?? 80; + const { terminalWidth, terminalHeight, constrainHeight } = useUIState(); + const isAlternateBuffer = useAlternateBuffer(); const isTrustedFolder = config.isTrustedFolder(); const [phase, setPhase] = useState('list'); const [items, setItems] = useState([]); @@ -676,6 +782,117 @@ export const InboxDialog: React.FC = ({ { isActive: true, priority: true }, ); + // Hoist the per-phase preview data so the array literals passed to + // ScrollableDiffViewport don't change identity on every parent render. + // ScrollableDiffViewport memoizes its expensive `parseDiffWithLineNumbers` + // + `renderDiffLines` on `sections`, so a new array literal every render + // would defeat that and re-colorize the diff each time. Keying on + // `selectedItem` captures every input that affects the rendered diffs. + // Must live above the early returns below so React sees a consistent + // hook order. + const previewData = useMemo(() => { + if (!selectedItem) { + return { + skillSections: undefined as DiffSection[] | undefined, + patchSections: undefined as DiffSection[] | undefined, + memoryGroups: undefined as + | Array<[string, { isNewFile: boolean; diffs: string[] }]> + | undefined, + memorySections: undefined as DiffSection[] | undefined, + }; + } + + if (selectedItem.type === 'skill') { + const skill = selectedItem.skill; + if (!skill.content) { + return { + skillSections: undefined, + patchSections: undefined, + memoryGroups: undefined, + memorySections: undefined, + }; + } + return { + skillSections: [ + { + key: `skill:${skill.dirName}`, + header: 'SKILL.md', + diffContent: newFileDiff('SKILL.md', skill.content), + }, + ], + patchSections: undefined, + memoryGroups: undefined, + memorySections: undefined, + }; + } + + if (selectedItem.type === 'patch') { + const patch = selectedItem.patch; + return { + skillSections: undefined, + patchSections: patch.entries.map((entry, index) => ({ + key: `${patch.fileName}:${entry.targetPath}:${index}`, + header: entry.targetPath, + diffContent: entry.diffContent, + })), + memoryGroups: undefined, + memorySections: undefined, + }; + } + + if (selectedItem.type === 'memory-patch') { + // Group hunks by target file. Multiple source patches may touch the + // same file (e.g. several patches all updating MEMORY.md); showing + // the file path once with all its hunks beneath is less noisy than + // repeating the path for every hunk. + const groups = new Map(); + for (const entry of selectedItem.memoryPatch.entries) { + const existing = groups.get(entry.targetPath); + if (existing) { + existing.diffs.push(entry.diffContent); + if (entry.isNewFile) existing.isNewFile = true; + } else { + groups.set(entry.targetPath, { + isNewFile: entry.isNewFile, + diffs: [entry.diffContent], + }); + } + } + const memoryGroups = Array.from(groups.entries()); + + const memorySections: DiffSection[] = []; + memoryGroups.forEach(([targetPath, { isNewFile, diffs }], groupIndex) => { + const headerAnnotation = `${isNewFile ? ' (new file)' : ''}${ + diffs.length > 1 + ? ` · ${diffs.length} changes from different patches` + : '' + }`; + diffs.forEach((diff, hunkIndex) => { + memorySections.push({ + key: `${targetPath}:${groupIndex}:${hunkIndex}`, + header: + hunkIndex === 0 ? `${targetPath}${headerAnnotation}` : targetPath, + diffContent: diff, + }); + }); + }); + + return { + skillSections: undefined, + patchSections: undefined, + memoryGroups, + memorySections, + }; + } + + return { + skillSections: undefined, + patchSections: undefined, + memoryGroups: undefined, + memorySections: undefined, + }; + }, [selectedItem]); + if (loading) { return ( = ({ // Border + paddingX account for 6 chars of width const contentWidth = terminalWidth - 6; + // Diff-rendering budgets. Two strategies, picked by `isAlternateBuffer`: + // + // - Alt-buffer: a fixed-height ScrollableList viewport. There is no + // terminal scrollback, so we must scroll inside the dialog itself + // via PgUp/PgDn/Shift+arrows. + // + // - Non-alt-buffer: the codebase's standard pattern of a bounded + // DiffRenderer + ShowMoreLines + Ctrl+O (see FolderTrustDialog, + // ThemeDialog). Clipped content lands in terminal scrollback when + // the user expands via Ctrl+O. + // + // Chrome accounts for the dialog's borders, padding, title + subtitle, + // action list (two `minHeight={2}` rows), the section's `marginTop`, + // the dialog footer, and a couple of safety rows. Bumped when inline + // feedback is showing. + const DIALOG_CHROME_HEIGHT = 16; + const feedbackHeight = feedback ? 2 : 0; + const diffViewportHeight = Math.max( + 3, + terminalHeight - DIALOG_CHROME_HEIGHT - feedbackHeight, + ); + + // For the non-alt-buffer DiffRenderer path, mirror MainContent / + // DialogManager and drop the clamp when the user has pressed Ctrl+O. + const availableContentHeight = constrainHeight + ? diffViewportHeight + : undefined; + const PATCH_ENTRY_OVERHEAD = 2; // target-path label + marginBottom + const patchEntryCount = + selectedItem?.type === 'patch' + ? selectedItem.patch.entries.length + : selectedItem?.type === 'memory-patch' + ? selectedItem.memoryPatch.entries.length + : 1; + const availablePatchEntryHeight = + availableContentHeight === undefined + ? undefined + : Math.max( + 3, + Math.floor( + (availableContentHeight - patchEntryCount * PATCH_ENTRY_OVERHEAD) / + Math.max(1, patchEntryCount), + ), + ); + + const previewNavigationHint = isAlternateBuffer + ? 'PgUp/PgDn to scroll' + : undefined; + + // Budget the list phase so the dialog footer never clips on shorter + // terminals. Every visible row — skill items, patch items, memory-patch + // items, and the section headers — renders at exactly 2 rows tall + // (enforced by `height={2}` on item renders and `marginTop={1}` + 1 + // text line for headers), so the windowed-slot count maps directly to + // terminal rows. + // + // Chrome rows accounted for: + // - round border (2) + // - paddingY (2) + // - DefaultAppLayout's alt-buffer paddingBottom (1) + // - title + subtitle (2) + // - marginTop above the list (1) + // - dialog footer marginTop + text (2) + // - BaseSelectionList ▲ + ▼ scroll arrows (2) — always shown when + // items > maxItemsToShow, which is precisely when this budget + // matters + const LIST_PHASE_CHROME_HEIGHT = 12; + const LIST_ROW_HEIGHT = 2; + const listMaxItemsToShow = Math.max( + 1, + Math.min( + 8, + Math.floor( + (terminalHeight - LIST_PHASE_CHROME_HEIGHT - feedbackHeight) / + LIST_ROW_HEIGHT, + ), + ), + ); + return ( - - {phase === 'list' && ( - <> - - Memory Inbox ({items.length} item{items.length !== 1 ? 's' : ''}) - - - Extracted from past sessions. Select one to review. - + + + {phase === 'list' && ( + <> + + Memory Inbox ({items.length} item{items.length !== 1 ? 's' : ''}) + + + Extracted from past sessions. Select one to review. + - - - items={listItems} - initialIndex={Math.max( - 0, - Math.min(lastListIndex, listItems.length - 1), - )} - onSelect={handleSelectItem} - isFocused={true} - showNumbers={false} - showScrollArrows={true} - maxItemsToShow={8} - renderItem={(item, { titleColor }) => { - if (item.value.type === 'header') { - return ( - - - {item.value.label} - - - ); - } - if (item.value.type === 'skill') { - const skill = item.value.skill; - return ( - - - {skill.name} - - - - {skill.description} - - {skill.extractedAt && ( - - {' · '} - {formatDate(skill.extractedAt)} - - )} - - - ); - } - if (item.value.type === 'memory-patch') { - const memoryPatch = item.value.memoryPatch; - return ( - - - {memoryPatch.name} - - - - {formatMemoryPatchSummary(memoryPatch)} - - {memoryPatch.extractedAt && ( - - {' · '} - {formatDate(memoryPatch.extractedAt)} - - )} - - - ); - } - const patch = item.value.patch; - const fileNames = patch.entries.map((e) => - getPathBasename(e.targetPath), - ); - const origin = getSkillOriginTag( - patch.entries[0]?.targetPath ?? '', - ); - return ( - - - - {patch.name} - - {origin && ( - - {` [${origin}]`} - - )} - - - - {fileNames.join(', ')} - - {patch.extractedAt && ( - - {' · '} - {formatDate(patch.extractedAt)} - - )} - - - ); - }} - /> - - - {feedback && ( - - - {feedback.isError ? '✗ ' : '✓ '} - {feedback.text} - - - )} - - - - )} - - {phase === 'skill-preview' && selectedItem?.type === 'skill' && ( - <> - {selectedItem.skill.name} - - Review new skill before installing. - - - {selectedItem.skill.content && ( - - SKILL.md - - + items={listItems} + initialIndex={Math.max( + 0, + Math.min(lastListIndex, listItems.length - 1), )} - filename="SKILL.md" - terminalWidth={contentWidth} + onSelect={handleSelectItem} + isFocused={true} + showNumbers={false} + showScrollArrows={true} + maxItemsToShow={listMaxItemsToShow} + renderItem={(item, { titleColor }) => { + if (item.value.type === 'header') { + return ( + + + {item.value.label} + + + ); + } + if (item.value.type === 'skill') { + const skill = item.value.skill; + const subtitle = skill.extractedAt + ? `${skill.description} · ${formatDate(skill.extractedAt)}` + : skill.description; + return ( + + + {skill.name} + + + {subtitle} + + + ); + } + if (item.value.type === 'memory-patch') { + const memoryPatch = item.value.memoryPatch; + const summary = formatMemoryPatchSummary(memoryPatch); + const subtitle = memoryPatch.extractedAt + ? `${summary} · ${formatDate(memoryPatch.extractedAt)}` + : summary; + return ( + + + {memoryPatch.name} + + + {subtitle} + + + ); + } + const patch = item.value.patch; + const fileNames = patch.entries.map((e) => + getPathBasename(e.targetPath), + ); + const origin = getSkillOriginTag( + patch.entries[0]?.targetPath ?? '', + ); + const titleLine = origin + ? `${patch.name} [${origin}]` + : patch.name; + const subtitle = patch.extractedAt + ? `${fileNames.join(', ')} · ${formatDate(patch.extractedAt)}` + : fileNames.join(', '); + return ( + + + {titleLine} + + + {subtitle} + + + ); + }} /> - )} - - - items={skillPreviewItems} - onSelect={handleSkillPreviewAction} - isFocused={true} - showNumbers={true} - renderItem={(item, { titleColor }) => ( - - - {item.value.label} - - - {item.value.description} - - - )} - /> - - - {feedback && ( - - - {feedback.isError ? '✗ ' : '✓ '} - {feedback.text} - - - )} - - - - )} - - {phase === 'skill-action' && selectedItem?.type === 'skill' && ( - <> - Move "{selectedItem.skill.name}" - - Choose where to install this skill. - - - - - items={destinationItems} - onSelect={handleSelectDestination} - isFocused={true} - showNumbers={true} - renderItem={(item, { titleColor }) => ( - - - {item.value.label} - - - {item.value.description} - - - )} - /> - - - {feedback && ( - - - {feedback.isError ? '✗ ' : '✓ '} - {feedback.text} - - - )} - - - - )} - - {phase === 'patch-preview' && selectedItem?.type === 'patch' && ( - <> - {selectedItem.patch.name} - - - Review changes before applying. - - {(() => { - const origin = getSkillOriginTag( - selectedItem.patch.entries[0]?.targetPath ?? '', - ); - return origin ? ( - {` [${origin}]`} - ) : null; - })()} - - - - {selectedItem.patch.entries.map((entry, index) => ( - - - {entry.targetPath} + {feedback && ( + + + {feedback.isError ? '✗ ' : '✓ '} + {feedback.text} - + )} + + + + )} + + {phase === 'skill-preview' && selectedItem?.type === 'skill' && ( + <> + {selectedItem.skill.name} + + Review new skill before installing. + + + {selectedItem.skill.content && + (isAlternateBuffer ? ( + + + + ) : ( + + + SKILL.md + + + + ))} + + + + items={skillPreviewItems} + onSelect={handleSkillPreviewAction} + isFocused={true} + showNumbers={true} + renderItem={(item, { titleColor }) => ( + + + {item.value.label} + + + {item.value.description} + + + )} + /> + + + {feedback && ( + + + {feedback.isError ? '✗ ' : '✓ '} + {feedback.text} + + + )} + + {!isAlternateBuffer && ( + + )} + + + + )} + + {phase === 'skill-action' && selectedItem?.type === 'skill' && ( + <> + Move "{selectedItem.skill.name}" + + Choose where to install this skill. + + + + + items={destinationItems} + onSelect={handleSelectDestination} + isFocused={true} + showNumbers={true} + renderItem={(item, { titleColor }) => ( + + + {item.value.label} + + + {item.value.description} + + + )} + /> + + + {feedback && ( + + + {feedback.isError ? '✗ ' : '✓ '} + {feedback.text} + + + )} + + + + )} + + {phase === 'patch-preview' && selectedItem?.type === 'patch' && ( + <> + {selectedItem.patch.name} + + + Review changes before applying. + + {(() => { + const origin = getSkillOriginTag( + selectedItem.patch.entries[0]?.targetPath ?? '', + ); + return origin ? ( + {` [${origin}]`} + ) : null; + })()} + + + + {isAlternateBuffer ? ( + + ) : ( + selectedItem.patch.entries.map((entry, index) => ( + + + {entry.targetPath} + + + + )) + )} + + + + + items={patchActionItems} + onSelect={handleSelectPatchAction} + isFocused={true} + showNumbers={true} + renderItem={(item, { titleColor }) => ( + + + {item.value.label} + + + {item.value.description} + + + )} + /> + + + {feedback && ( + + + {feedback.isError ? '✗ ' : '✓ '} + {feedback.text} + + + )} + + {!isAlternateBuffer && ( + + )} + + + + )} + + {phase === 'memory-preview' && + selectedItem?.type === 'memory-patch' && ( + <> + {selectedItem.memoryPatch.name} + + Review {formatMemoryPatchSummary(selectedItem.memoryPatch)}{' '} + before applying. Apply runs each source patch atomically; + Dismiss removes them all. + + + {(() => { + // Grouping + section flattening were hoisted into the + // `previewData` useMemo so the array identities passed into + // ScrollableDiffViewport stay stable across re-renders. + const groupEntries = previewData.memoryGroups ?? []; + + if (isAlternateBuffer) { + return ( + + + + ); + } + + return groupEntries.map( + ([targetPath, { isNewFile, diffs }]) => ( + + + {targetPath} + {isNewFile ? ' (new file)' : ''} + {diffs.length > 1 + ? ` · ${diffs.length} changes from different patches` + : ''} + + {diffs.map((diff, hunkIndex) => ( + + ))} + + ), + ); + })()} + + + + items={memoryPatchActionItems} + onSelect={handleSelectMemoryPatchAction} + isFocused={true} + showNumbers={true} + renderItem={(item, { titleColor }) => ( + + + {item.value.label} + + + {item.value.description} + + + )} /> - ))} - - - - items={patchActionItems} - onSelect={handleSelectPatchAction} - isFocused={true} - showNumbers={true} - renderItem={(item, { titleColor }) => ( - - - {item.value.label} - - - {item.value.description} + {feedback && ( + + + {feedback.isError ? '✗ ' : '✓ '} + {feedback.text} )} - /> - - {feedback && ( - - - {feedback.isError ? '✗ ' : '✓ '} - {feedback.text} - - - )} - - - - )} - - {phase === 'memory-preview' && selectedItem?.type === 'memory-patch' && ( - <> - {selectedItem.memoryPatch.name} - - Review {formatMemoryPatchSummary(selectedItem.memoryPatch)} before - applying. Apply runs each source patch atomically; Dismiss removes - them all. - - - {(() => { - // Group hunks by target file. Multiple source patches may touch - // the same file (e.g. several patches all updating MEMORY.md); - // showing the file path once with all its hunks beneath is much - // less visually noisy than repeating the path for every hunk. - const groups = new Map< - string, - { isNewFile: boolean; diffs: string[] } - >(); - for (const entry of selectedItem.memoryPatch.entries) { - const existing = groups.get(entry.targetPath); - if (existing) { - existing.diffs.push(entry.diffContent); - // If any hunk for this target was a creation, treat the - // group as a creation overall. - if (entry.isNewFile) existing.isNewFile = true; - } else { - groups.set(entry.targetPath, { - isNewFile: entry.isNewFile, - diffs: [entry.diffContent], - }); - } - } - - return Array.from(groups.entries()).map( - ([targetPath, { isNewFile, diffs }]) => ( - - - {targetPath} - {isNewFile ? ' (new file)' : ''} - {diffs.length > 1 - ? ` · ${diffs.length} changes from different patches` - : ''} - - {diffs.map((diff, hunkIndex) => ( - - ))} - - ), - ); - })()} - - - - items={memoryPatchActionItems} - onSelect={handleSelectMemoryPatchAction} - isFocused={true} - showNumbers={true} - renderItem={(item, { titleColor }) => ( - - - {item.value.label} - - - {item.value.description} - - + {!isAlternateBuffer && ( + )} - /> - - {feedback && ( - - - {feedback.isError ? '✗ ' : '✓ '} - {feedback.text} - - + + )} - - - - )} - + + ); };