From 698526e0bc8754e1b0cb85d903676ac966c1e1bb Mon Sep 17 00:00:00 2001 From: wsp Date: Fri, 18 Sep 2026 16:01:34 +0800 Subject: [PATCH] fix(web-ui): unify exec process card previews and footer alignment - Apply output-driven expansion and the one-second minimum preview to WriteStdin and ExecControl, matching ExecCommand. - Remove tail-specific collapse behavior while preserving manual toggles. - Right-align footer metadata consistently across all three card types. - Extend shared regression coverage to all exec process cards. --- .../ExecProcessToolCardView.test.tsx | 34 ++++--- .../tool-cards/ExecProcessToolCardView.tsx | 90 +++++-------------- 2 files changed, 44 insertions(+), 80 deletions(-) diff --git a/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.test.tsx b/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.test.tsx index 80c88b109f..be0b0a608d 100644 --- a/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.test.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.test.tsx @@ -43,7 +43,7 @@ vi.mock('@/tools/terminal/components/LazyTerminalOutputRenderer', () => ({ }), })); -const model: ExecProcessCardModel = { +const baseModel: ExecProcessCardModel = { kind: 'command', actionLabel: 'Run command:', primaryText: 'npm test', @@ -54,7 +54,7 @@ const model: ExecProcessCardModel = { resultOutput: '', }; -function toolItem(status: FlowToolItem['status'], isParamsStreaming = false): FlowToolItem { +function makeToolItem(status: FlowToolItem['status'], isParamsStreaming = false): FlowToolItem { return { id: 'tool-exec-1', type: 'tool', @@ -69,7 +69,16 @@ function toolItem(status: FlowToolItem['status'], isParamsStreaming = false): Fl }; } -describe('ExecProcessToolCardView', () => { +describe.each([ + ['command', 'ExecCommand'], + ['stdin', 'WriteStdin'], + ['control', 'ExecControl'], +] as const)('ExecProcessToolCardView (%s)', (kind, toolName) => { + const model: ExecProcessCardModel = { ...baseModel, kind }; + const toolItem = (status: FlowToolItem['status'], isParamsStreaming = false): FlowToolItem => ({ + ...makeToolItem(status, isParamsStreaming), + toolName, + }); let dom: JSDOM; let container: HTMLDivElement; let root: Root; @@ -273,10 +282,10 @@ describe('ExecProcessToolCardView', () => { expect(container.querySelector('[data-openbitfun-component="command-tool-card"] [data-openbitfun-part="output"] pre')).toBeNull(); }); - it('retains a just-completed WriteStdin tail result during the grace period', () => { + it('retains a compact completed result when it stops being the tail', () => { + vi.useFakeTimers(); const resultModel: ExecProcessCardModel = { ...model, - kind: 'stdin', resultOutput: 'All tests passed', }; @@ -318,6 +327,8 @@ describe('ExecProcessToolCardView', () => { ); }); + expect(container.querySelector(expandedSurface)).not.toBeNull(); + act(() => { vi.advanceTimersByTime(1000); }); // Collapsed cards keep the prominent framework shell and animate height closed. expect(container.querySelector('[data-openbitfun-part="surface"][data-openbitfun-attention="prominent"]')).not.toBeNull(); expect(container.querySelector('[data-openbitfun-part="surface"][data-openbitfun-attention="prominent"][data-openbitfun-state~="expanded"]')).toBeNull(); @@ -372,10 +383,9 @@ describe('ExecProcessToolCardView', () => { expect(container.querySelector('[data-openbitfun-part="outputFrame"]')?.getAttribute('data-sizing')).toBe('content'); }); - it('pushes WriteStdin session and execution metadata to the footer end', () => { - const stdinModel: ExecProcessCardModel = { + it('pushes session and execution metadata to the footer end', () => { + const metadataModel: ExecProcessCardModel = { ...model, - kind: 'stdin', sessionId: 42, exitCode: 0, wallTimeSeconds: 1.25, @@ -384,8 +394,8 @@ describe('ExecProcessToolCardView', () => { act(() => { root.render( , ); }); @@ -401,9 +411,9 @@ describe('ExecProcessToolCardView', () => { expect(footerItems[0]?.getAttribute('data-push-to-end')).toBe('true'); expect(footerItems[0]?.textContent).toContain('#42'); expect(footerItems[1]?.getAttribute('data-push-to-end')).toBe('false'); - expect(footerItems[1]?.textContent).toContain('toolCards.execProcess.wallTime'); + expect(footerItems[1]?.textContent).toContain(kind === 'stdin' ? 'toolCards.execProcess.wallTime' : 'Exit code: 0'); expect(footerItems[2]?.getAttribute('data-push-to-end')).toBe('false'); - expect(footerItems[2]?.textContent).toContain('Exit code: 0'); + expect(footerItems[2]?.textContent).toContain(kind === 'stdin' ? 'Exit code: 0' : 'toolCards.execProcess.wallTime'); }); it('keeps the output frame and footer mounted while content changes', () => { diff --git a/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.tsx b/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.tsx index 47514503f3..a9b1b07340 100644 --- a/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.tsx +++ b/src/web-ui/src/flow_chat/tool-cards/ExecProcessToolCardView.tsx @@ -13,7 +13,6 @@ import { ToolCardCopyAction } from './ToolCardCopyAction'; import { ToolTimeoutIndicator } from './ToolTimeoutIndicator'; import { useCopyTextAction } from '../hooks/useCopyTextAction'; import { useToolCardHeightContract } from './useToolCardHeightContract'; -import { useToolCardCompletionGracePeriod } from './useToolCardCompletionGracePeriod'; import { formatSessionViewPreviewText } from '../utils/sessionViewPreview'; const EXEC_COLLAPSED_STATUSES = new Set(['completed', 'cancelled', 'error', 'rejected']); @@ -51,26 +50,8 @@ function isCollapsedStatus(status: string): boolean { return EXEC_COLLAPSED_STATUSES.has(status); } -function getInitialExpandedState(status: string, waitForOutput: boolean, hasLiveOutput: boolean): boolean { - return !isCollapsedStatus(status) && (!waitForOutput || hasLiveOutput || status === 'pending_confirmation'); -} - -function getAutoExpandedStateForStatus( - status: string, - isLastItem: boolean | undefined, - keepTailPreview: boolean, - hasLiveOutput: boolean, - waitForOutput: boolean, -): boolean | null { - if (isCollapsedStatus(status)) { - return isLastItem === true && keepTailPreview ? null : false; - } - - if (status === 'preparing' || status === 'streaming' || status === 'running' || status === 'receiving') { - return !waitForOutput || hasLiveOutput; - } - - return null; +function getInitialExpandedState(status: string, hasLiveOutput: boolean): boolean { + return !isCollapsedStatus(status) && (hasLiveOutput || status === 'pending_confirmation'); } function isCancelledStatus(status: string): boolean { @@ -113,7 +94,6 @@ export const ExecProcessToolCardView: React.FC = ( toolItem, model, onExpand, - isLastItem, }) => { const { t } = useTranslation('flow-chat'); const status = toolItem.status || 'pending'; @@ -132,10 +112,9 @@ export const ExecProcessToolCardView: React.FC = ( ? 'toolCards.terminal.rejected' : 'toolCards.terminal.cancelled'; const toolId = toolItem.id ?? toolItem.toolCall?.id; - const waitForOutput = model.kind === 'command'; const hasLiveOutput = liveOutput.length > 0; - const [isExpanded, setIsExpandedState] = useState(() => getInitialExpandedState(status, waitForOutput, hasLiveOutput)); + const [isExpanded, setIsExpandedState] = useState(() => getInitialExpandedState(status, hasLiveOutput)); const userToggledRef = useRef(false); const autoExpandedAtRef = useRef(null); const outputRendererRef = useRef(null); @@ -143,17 +122,6 @@ export const ExecProcessToolCardView: React.FC = ( toolId, toolName: toolItem.toolName, }); - const { - begin: beginCompletionPreview, - isActive: isCompletionPreviewActive, - } = useToolCardCompletionGracePeriod({ - eligible: - !waitForOutput && - isCollapsedStatus(status) && - isLastItem === true && - isExpanded && - !userToggledRef.current, - }); const applyExecExpandedState = useCallback((nextExpanded: boolean) => { if (nextExpanded === isExpanded) { @@ -173,50 +141,36 @@ export const ExecProcessToolCardView: React.FC = ( return; } - if (waitForOutput) { - if (isRunning && hasLiveOutput) { - // Start once, including when mounting with output already available. - autoExpandedAtRef.current ??= Date.now(); - applyExecExpandedState(true); - } else if (isCollapsedStatus(status)) { - const remainingMs = autoExpandedAtRef.current === null - ? 0 - : EXEC_MINIMUM_EXPANDED_MS - (Date.now() - autoExpandedAtRef.current); - if (isExpanded && remainingMs > 0) { - const timer = setTimeout(() => { - if (!userToggledRef.current) { - applyExecExpandedState(false); - } - }, remainingMs); - return () => clearTimeout(timer); - } - applyExecExpandedState(false); - } else if (isRunning && autoExpandedAtRef.current === null) { - applyExecExpandedState(false); + if (isRunning && hasLiveOutput) { + // Start once, including when mounting with output already available. + autoExpandedAtRef.current ??= Date.now(); + applyExecExpandedState(true); + } else if (isCollapsedStatus(status)) { + const remainingMs = autoExpandedAtRef.current === null + ? 0 + : EXEC_MINIMUM_EXPANDED_MS - (Date.now() - autoExpandedAtRef.current); + if (isExpanded && remainingMs > 0) { + const timer = setTimeout(() => { + if (!userToggledRef.current) { + applyExecExpandedState(false); + } + }, remainingMs); + return () => clearTimeout(timer); } - return; - } - - const keepTailPreview = isCollapsedStatus(status) && beginCompletionPreview(); - const nextExpanded = getAutoExpandedStateForStatus(status, isLastItem, keepTailPreview, hasLiveOutput, waitForOutput); - if (nextExpanded !== null) { - applyExecExpandedState(nextExpanded); + applyExecExpandedState(false); + } else if (isRunning && autoExpandedAtRef.current === null) { + applyExecExpandedState(false); } }, [ applyExecExpandedState, - beginCompletionPreview, hasLiveOutput, - isCompletionPreviewActive, isExpanded, isRunning, - isLastItem, status, - waitForOutput, ]); const compactSettledPreview = isExpanded && - (waitForOutput || isLastItem === true) && isCollapsedStatus(status) && !userToggledRef.current; // Keep auto-managed completed cards on the compact preview through the @@ -348,7 +302,7 @@ export const ExecProcessToolCardView: React.FC = ( } } footerItems.push(...footerMetadataItems.map((item, index) => ( - model.kind === 'stdin' && index === 0 + index === 0 ? { ...item, pushToEnd: true } : item )));