fix(shell): improve shell output presentation and usability (#8837)

packages/cli/src/ui/components/messages/ToolMessage.tsx

@@ -62,11 +62,42 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
     ptyId === activeShellPtyId &&
     embeddedShellFocused;
 
+  const [lastUpdateTime, setLastUpdateTime] = React.useState<Date | null>(null);
+  const [userHasFocused, setUserHasFocused] = React.useState(false);
+  const [showFocusHint, setShowFocusHint] = React.useState(false);
+
+  React.useEffect(() => {
+    if (resultDisplay) {
+      setLastUpdateTime(new Date());
+    }
+  }, [resultDisplay]);
+
+  React.useEffect(() => {
+    if (!lastUpdateTime) {
+      return;
+    }
+
+    const timer = setTimeout(() => {
+      setShowFocusHint(true);
+    }, 5000);
+
+    return () => clearTimeout(timer);
+  }, [lastUpdateTime]);
+
+  React.useEffect(() => {
+    if (isThisShellFocused) {
+      setUserHasFocused(true);
+    }
+  }, [isThisShellFocused]);
+
   const isThisShellFocusable =
     (name === SHELL_COMMAND_NAME || name === 'Shell') &&
     status === ToolCallStatus.Executing &&
     config?.getShouldUseNodePtyShell();
 
+  const shouldShowFocusHint =
+    isThisShellFocusable && (showFocusHint || userHasFocused);
+
   const availableHeight = availableTerminalHeight
     ? Math.max(
         availableTerminalHeight - STATIC_HEIGHT - RESERVED_LINE_COUNT,
@@ -99,7 +130,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
           description={description}
           emphasis={emphasis}
         />
-        {isThisShellFocusable && (
+        {shouldShowFocusHint && (
           <Box marginLeft={1} flexShrink={0}>
             <Text color={theme.text.accent}>
               {isThisShellFocused ? '(Focused)' : '(ctrl+f to focus)'}

packages/core/src/services/shellExecutionService.test.ts

@@ -64,6 +64,7 @@ const shellExecutionConfig = {
   terminalHeight: 24,
   pager: 'cat',
   showColor: false,
+  disableDynamicLineTrimming: true,
 };
 
 const createExpectedAnsiOutput = (text: string | string[]): AnsiOutput => {
@@ -441,6 +442,7 @@ describe('ShellExecutionService', () => {
         showColor: true,
         defaultFg: '#ffffff',
         defaultBg: '#000000',
+        disableDynamicLineTrimming: true,
       };
       const mockAnsiOutput = [
         [{ text: 'hello', fg: '#ffffff', bg: '#000000' }],
@@ -458,7 +460,6 @@ describe('ShellExecutionService', () => {
 
       expect(mockSerializeTerminalToObject).toHaveBeenCalledWith(
         expect.anything(), // The terminal object
-        { defaultFg: '#ffffff', defaultBg: '#000000' },
       );
 
       expect(onOutputEventMock).toHaveBeenCalledWith(
@@ -476,7 +477,11 @@ describe('ShellExecutionService', () => {
           pty.onData.mock.calls[0][0]('a\u001b[31mred\u001b[0mword');
           pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null });
         },
-        { ...shellExecutionConfig, showColor: false },
+        {
+          ...shellExecutionConfig,
+          showColor: false,
+          disableDynamicLineTrimming: true,
+        },
       );
 
       const expected = createExpectedAnsiOutput('aredword');
@@ -498,7 +503,11 @@ describe('ShellExecutionService', () => {
           );
           pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null });
         },
-        { ...shellExecutionConfig, showColor: false },
+        {
+          ...shellExecutionConfig,
+          showColor: false,
+          disableDynamicLineTrimming: true,
+        },
       );
 
       const expected = createExpectedAnsiOutput(['line 1', 'line 2', 'line 3']);

packages/core/src/services/shellExecutionService.ts

@@ -57,6 +57,8 @@ export interface ShellExecutionConfig {
   showColor?: boolean;
   defaultFg?: string;
   defaultBg?: string;
+  // Used for testing
+  disableDynamicLineTrimming?: boolean;
 }
 
 /**
@@ -383,6 +385,7 @@ export class ShellExecutionService {
         const MAX_SNIFF_SIZE = 4096;
         let sniffedBytes = 0;
         let isWriting = false;
+        let hasStartedOutput = false;
         let renderTimeout: NodeJS.Timeout | null = null;
 
         const render = (finalRender = false) => {
@@ -394,12 +397,20 @@ export class ShellExecutionService {
             if (!isStreamingRawContent) {
               return;
             }
+
+            if (!shellExecutionConfig.disableDynamicLineTrimming) {
+              if (!hasStartedOutput) {
+                const bufferText = getFullBufferText(headlessTerminal);
+                if (bufferText.trim().length === 0) {
+                  return;
+                }
+                hasStartedOutput = true;
+              }
+            }
+
             let newOutput: AnsiOutput;
             if (shellExecutionConfig.showColor) {
-              newOutput = serializeTerminalToObject(headlessTerminal, {
-                defaultFg: shellExecutionConfig.defaultFg,
-                defaultBg: shellExecutionConfig.defaultBg,
-              });
+              newOutput = serializeTerminalToObject(headlessTerminal);
             } else {
               const buffer = headlessTerminal.buffer.active;
               const lines: AnsiOutput = [];
@@ -422,12 +433,32 @@ export class ShellExecutionService {
               newOutput = lines;
             }
 
+            let lastNonEmptyLine = -1;
+            for (let i = newOutput.length - 1; i >= 0; i--) {
+              const line = newOutput[i];
+              if (
+                line
+                  .map((segment) => segment.text)
+                  .join('')
+                  .trim().length > 0
+              ) {
+                lastNonEmptyLine = i;
+                break;
+              }
+            }
+
+            const trimmedOutput = newOutput.slice(0, lastNonEmptyLine + 1);
+
+            const finalOutput = shellExecutionConfig.disableDynamicLineTrimming
+              ? newOutput
+              : trimmedOutput;
+
             // Using stringify for a quick deep comparison.
-            if (JSON.stringify(output) !== JSON.stringify(newOutput)) {
-              output = newOutput;
+            if (JSON.stringify(output) !== JSON.stringify(finalOutput)) {
+              output = finalOutput;
               onOutputEvent({
                 type: 'data',
-                chunk: newOutput,
+                chunk: finalOutput,
               });
             }
           };
@@ -569,12 +600,26 @@ export class ShellExecutionService {
    * @param input The string to write to the terminal.
    */
   static writeToPty(pid: number, input: string): void {
+    if (!this.isPtyActive(pid)) {
+      return;
+    }
+
     const activePty = this.activePtys.get(pid);
     if (activePty) {
       activePty.ptyProcess.write(input);
     }
   }
 
+  static isPtyActive(pid: number): boolean {
+    try {
+      // process.kill with signal 0 is a way to check for the existence of a process.
+      // It doesn't actually send a signal.
+      return process.kill(pid, 0);
+    } catch (_) {
+      return false;
+    }
+  }
+
   /**
    * Resizes the pseudo-terminal (PTY) of a running process.
    *
@@ -583,6 +628,10 @@ export class ShellExecutionService {
    * @param rows The new number of rows.
    */
   static resizePty(pid: number, cols: number, rows: number): void {
+    if (!this.isPtyActive(pid)) {
+      return;
+    }
+
     const activePty = this.activePtys.get(pid);
     if (activePty) {
       try {
@@ -607,6 +656,10 @@ export class ShellExecutionService {
    * @param lines The number of lines to scroll.
    */
   static scrollPty(pid: number, lines: number): void {
+    if (!this.isPtyActive(pid)) {
+      return;
+    }
+
     const activePty = this.activePtys.get(pid);
     if (activePty) {
       try {

packages/core/src/utils/terminalSerializer.ts

@@ -131,15 +131,12 @@ class Cell {
   }
 }
 
-export function serializeTerminalToObject(
-  terminal: Terminal,
-  options?: { defaultFg?: string; defaultBg?: string },
-): AnsiOutput {
+export function serializeTerminalToObject(terminal: Terminal): AnsiOutput {
   const buffer = terminal.buffer.active;
   const cursorX = buffer.cursorX;
   const cursorY = buffer.cursorY;
-  const defaultFg = options?.defaultFg ?? '#ffffff';
-  const defaultBg = options?.defaultBg ?? '#000000';
+  const defaultFg = '';
+  const defaultBg = '';
 
   const result: AnsiOutput = [];