diff --git a/src/cli/output/icons.ts b/src/cli/output/icons.ts index 8c09d567..8b9a1ff8 100644 --- a/src/cli/output/icons.ts +++ b/src/cli/output/icons.ts @@ -11,3 +11,9 @@ export const ICON_SKIPPED = '↓'; // U+2193 DOWNWARDS ARROW /** Braille spinner frames for loading animation */ export const SPINNER_FRAMES = ['\u280B', '\u2819', '\u2839', '\u2838', '\u283C', '\u2834', '\u2826', '\u2827', '\u2807', '\u280F']; + +/** Circle for pending states */ +export const ICON_PENDING = '\u25CB'; // ○ WHITE CIRCLE + +/** X mark for error states */ +export const ICON_ERROR = '\u2717'; // ✗ BALLOT X diff --git a/src/cli/output/ink-runner.tsx b/src/cli/output/ink-runner.tsx index 9e5e49b3..5587101a 100644 --- a/src/cli/output/ink-runner.tsx +++ b/src/cli/output/ink-runner.tsx @@ -1,5 +1,25 @@ /** * Ink-based skill runner with real-time progress display. + * + * ## Ink Rendering Constraints + * + * This file uses Ink (React for CLIs) which has specific constraints that, + * if violated, cause duplicate output lines or corrupted display: + * + * 1. **Single Static component**: Ink's Static uses `position: 'absolute'`. + * Multiple Static components cause layout conflicts. We print the header + * before Ink starts to avoid needing a second Static. + * + * 2. **Stable item references**: Static tracks items by reference equality. + * Never wrap items in new objects (e.g., `{ type: 'skill', skill }`) on + * each render. Pass the original objects directly. + * + * 3. **Batched updates**: Rapid consecutive rerender() calls cause duplicate + * output. The updateUI() function batches updates using setImmediate(). + * + * 4. **No direct writes to stderr**: Writing to process.stderr while Ink is + * running corrupts cursor tracking. The onLargePrompt/onPromptSize callbacks + * are exceptions that may cause minor display glitches in edge cases. */ import React, { useState, useEffect } from 'react'; @@ -15,11 +35,9 @@ import { } from './tasks.js'; import { formatDuration, truncate, countBySeverity, formatSeverityDot } from './formatters.js'; import { Verbosity } from './verbosity.js'; -import { ICON_CHECK, ICON_SKIPPED, SPINNER_FRAMES } from './icons.js'; +import { ICON_CHECK, ICON_SKIPPED, ICON_PENDING, ICON_ERROR, SPINNER_FRAMES } from './icons.js'; import figures from 'figures'; -type StaticItem = { type: 'header' } | { type: 'skill'; skill: SkillState }; - interface SkillRunnerProps { skills: SkillState[]; completedItems: SkillState[]; @@ -91,7 +109,7 @@ function CompletedSkill({ skill }: { skill: SkillState }): React.ReactElement { return ( - {'\u2717'} + {ICON_ERROR} {skill.displayName} {duration && [{duration}]} @@ -127,46 +145,42 @@ function RunningSkill({ skill }: { skill: SkillState }): React.ReactElement { ); } +/** + * Renders the skill execution UI. + * + * IMPORTANT: Ink's Static component tracks items by reference equality. + * Items passed to Static must have stable references across renders. + * Creating new wrapper objects causes Static to mishandle its internal + * state, resulting in duplicate output lines. + * + * We use a SINGLE Static component to avoid layout conflicts from multiple + * absolutely-positioned Static containers. + */ function SkillRunner({ skills, completedItems }: SkillRunnerProps): React.ReactElement { const running = skills.filter((s) => s.status === 'running'); const pending = skills.filter((s) => s.status === 'pending'); - // Build static items: header first, then completed skills - const staticItems: StaticItem[] = [ - { type: 'header' }, - ...completedItems.map((skill) => ({ type: 'skill' as const, skill })), - ]; - return ( - <> - {/* Static content: header + completed skills */} - - {(item) => { - switch (item.type) { - case 'header': - return ( - - SKILLS - - ); - case 'skill': - return ; - } - }} + + {/* + * Completed skills - passed directly to Static WITHOUT wrapper objects. + * completedItems elements have stable references (same objects from parent), + * so Ink can correctly track which items are new vs already rendered. + */} + + {(skill) => } - {/* Dynamic content: running + pending */} - - {running.map((skill) => ( - - ))} - {pending.map((skill) => ( - - {'\u25CB'} {skill.displayName} - - ))} - - + {/* Dynamic content - updates in place */} + {running.map((skill) => ( + + ))} + {pending.map((skill) => ( + + {ICON_PENDING} {skill.displayName} + + ))} + ); } @@ -206,14 +220,28 @@ export async function runSkillTasksWithInk( const completedItems: SkillState[] = []; const completedNames = new Set(); + // Print header before Ink starts - this avoids multiple Static components + // which can cause layout conflicts due to absolute positioning + process.stderr.write('\x1b[1mSKILLS\x1b[0m\n'); + // Create Ink instance const { rerender, unmount } = render( , { stdout: process.stderr } ); + // Batch UI updates to prevent rapid consecutive rerenders that cause duplicate lines. + // Without batching, multiple callbacks firing in quick succession (e.g., 5 files + // starting simultaneously) trigger 5 immediate rerenders, which Ink cannot + // process correctly, resulting in the same line appearing multiple times. + let updatePending = false; const updateUI = () => { - rerender(); + if (updatePending) return; + updatePending = true; + setImmediate(() => { + updatePending = false; + rerender(); + }); }; // Callbacks to update state @@ -297,13 +325,14 @@ export async function runSkillTasksWithInk( updateUI(); }, - // Warn about large prompts - write directly to stderr to avoid Ink interference + // CAUTION: Direct stderr writes while Ink is running can cause display glitches. + // These callbacks are rare (large prompts, debug mode) so the tradeoff is acceptable. + // If these cause issues, consider queueing messages and printing after unmount(). onLargePrompt: (_skillName, filename, lineRange, chars, estimatedTokens) => { const location = `${filename}:${lineRange}`; const size = `${Math.round(chars / 1000)}k chars (~${Math.round(estimatedTokens / 1000)}k tokens)`; process.stderr.write(`\x1b[33m${figures.warning}\x1b[0m Large prompt for ${location}: ${size}\n`); }, - // Debug mode: show prompt sizes onPromptSize: verbosity >= Verbosity.Debug ? (_skillName, filename, lineRange, systemChars, userChars, totalChars, estimatedTokens) => { const location = `${filename}:${lineRange}`;