Merge pull request #2696 from QwenLM/feat/hooks-refactor-ui-event

refactor(ui): improve hook event handling with dedicated history items

packages/cli/src/ui/components/HistoryItemDisplay.tsx

@@ -6,7 +6,10 @@
 
 import type React from 'react';
 import { useMemo } from 'react';
-import { escapeAnsiCtrlCodes } from '../utils/textUtils.js';
+import {
+  escapeAnsiCtrlCodes,
+  sanitizeSensitiveText,
+} from '../utils/textUtils.js';
 import type { HistoryItem } from '../types.js';
 import {
   UserMessage,
@@ -26,7 +29,9 @@ import {
   RetryCountdownMessage,
   SuccessMessage,
 } from './messages/StatusMessages.js';
-import { Box } from 'ink';
+import { Box, Text } from 'ink';
+import { theme } from '../semantic-colors.js';
+import { MarkdownDisplay } from '../utils/MarkdownDisplay.js';
 import { AboutBox } from './AboutBox.js';
 import { StatsDisplay } from './StatsDisplay.js';
 import { ModelStatsDisplay } from './ModelStatsDisplay.js';
@@ -183,7 +188,9 @@ const HistoryItemDisplayComponent: React.FC<HistoryItemDisplayProps> = ({
       {itemForDisplay.type === 'compression' && (
         <CompressionMessage compression={itemForDisplay.compression} />
       )}
-      {item.type === 'summary' && <SummaryMessage summary={item.summary} />}
+      {itemForDisplay.type === 'summary' && (
+        <SummaryMessage summary={itemForDisplay.summary} />
+      )}
       {itemForDisplay.type === 'extensions_list' && <ExtensionsList />}
       {itemForDisplay.type === 'tools_list' && (
         <ToolsList
@@ -230,6 +237,30 @@ const HistoryItemDisplayComponent: React.FC<HistoryItemDisplayProps> = ({
       {itemForDisplay.type === 'btw' && itemForDisplay.btw && (
         <BtwMessage btw={itemForDisplay.btw} />
       )}
+      {itemForDisplay.type === 'user_prompt_submit_blocked' && (
+        <Box flexDirection="column">
+          <Text color={theme.status.warning}>
+            {`✕ UserPromptSubmit operation blocked by hook:\n${itemForDisplay.reason}\n\nOriginal prompt: ${sanitizeSensitiveText(itemForDisplay.originalPrompt)}`}
+          </Text>
+        </Box>
+      )}
+      {itemForDisplay.type === 'stop_hook_loop' && (
+        <InfoMessage
+          text={`Ran ${itemForDisplay.stopHookCount} stop hooks\n  ⎿  Stop hook error: ${itemForDisplay.reasons[itemForDisplay.reasons.length - 1]}`}
+        />
+      )}
+      {itemForDisplay.type === 'stop_hook_system_message' && (
+        <Box flexDirection="column">
+          <Text color={theme.text.primary}> ⎿ Stop says:</Text>
+          <Box marginLeft={4} flexDirection="column">
+            <MarkdownDisplay
+              text={itemForDisplay.message}
+              isPending={false}
+              contentWidth={contentWidth - 4}
+            />
+          </Box>
+        </Box>
+      )}
     </Box>
   );
 };

packages/cli/src/ui/hooks/useGeminiStream.test.tsx

@@ -3226,4 +3226,287 @@ describe('useGeminiStream', () => {
       });
     });
   });
+
+  describe('UserPromptSubmitBlocked Event', () => {
+    it('should handle UserPromptSubmitBlocked event and add blocked history item', async () => {
+      mockSendMessageStream.mockReturnValue(
+        (async function* () {
+          yield {
+            type: ServerGeminiEventType.UserPromptSubmitBlocked,
+            value: {
+              reason: 'Hook blocked due to security policy',
+              originalPrompt: 'This is the original user prompt',
+            },
+          };
+        })(),
+      );
+
+      const { result } = renderTestHook();
+
+      await act(async () => {
+        await result.current.submitQuery('This is the original user prompt');
+      });
+
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'user_prompt_submit_blocked',
+            reason: 'Hook blocked due to security policy',
+            originalPrompt: 'This is the original user prompt',
+          }),
+          expect.any(Number),
+        );
+      });
+
+      // Verify streaming state transitions correctly
+      expect(result.current.streamingState).toBe(StreamingState.Idle);
+    });
+
+    it('should move pending history item before adding UserPromptSubmitBlocked event', async () => {
+      mockSendMessageStream.mockReturnValue(
+        (async function* () {
+          yield {
+            type: ServerGeminiEventType.Content,
+            value: 'Partial response before block',
+          };
+          yield {
+            type: ServerGeminiEventType.UserPromptSubmitBlocked,
+            value: {
+              reason: 'Security violation detected',
+              originalPrompt: 'Execute system command',
+            },
+          };
+        })(),
+      );
+
+      const { result } = renderTestHook();
+
+      await act(async () => {
+        await result.current.submitQuery('Execute system command');
+      });
+
+      // Verify content was added first
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'gemini',
+            text: 'Partial response before block',
+          }),
+          expect.any(Number),
+        );
+      });
+
+      // Then verify blocked event was added
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'user_prompt_submit_blocked',
+            reason: 'Security violation detected',
+            originalPrompt: 'Execute system command',
+          }),
+          expect.any(Number),
+        );
+      });
+    });
+  });
+
+  describe('StopHookLoop Event', () => {
+    it('should handle StopHookLoop event and add stop hook loop history item', async () => {
+      mockSendMessageStream.mockReturnValue(
+        (async function* () {
+          yield {
+            type: ServerGeminiEventType.StopHookLoop,
+            value: {
+              iterationCount: 3,
+              reasons: [
+                'Reason 1: Continue analysis',
+                'Reason 2: More details needed',
+                'Reason 3: Incomplete response',
+              ],
+              stopHookCount: 3,
+            },
+          };
+        })(),
+      );
+
+      const { result } = renderTestHook();
+
+      await act(async () => {
+        await result.current.submitQuery('test query with stop hooks');
+      });
+
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'stop_hook_loop',
+            iterationCount: 3,
+            reasons: [
+              'Reason 1: Continue analysis',
+              'Reason 2: More details needed',
+              'Reason 3: Incomplete response',
+            ],
+          }),
+          expect.any(Number),
+        );
+      });
+
+      // Verify streaming state transitions correctly
+      expect(result.current.streamingState).toBe(StreamingState.Idle);
+    });
+
+    it('should move pending history item before adding StopHookLoop event', async () => {
+      mockSendMessageStream.mockReturnValue(
+        (async function* () {
+          yield {
+            type: ServerGeminiEventType.Content,
+            value: 'Initial response before loop',
+          };
+          yield {
+            type: ServerGeminiEventType.StopHookLoop,
+            value: {
+              iterationCount: 5,
+              reasons: ['Hook reason 1', 'Hook reason 2'],
+              stopHookCount: 2,
+            },
+          };
+        })(),
+      );
+
+      const { result } = renderTestHook();
+
+      await act(async () => {
+        await result.current.submitQuery('query triggering stop hooks');
+      });
+
+      // Verify content was added first
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'gemini',
+            text: 'Initial response before loop',
+          }),
+          expect.any(Number),
+        );
+      });
+
+      // Then verify stop hook loop event was added
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'stop_hook_loop',
+            iterationCount: 5,
+            reasons: ['Hook reason 1', 'Hook reason 2'],
+          }),
+          expect.any(Number),
+        );
+      });
+    });
+
+    it('should handle single iteration StopHookLoop event', async () => {
+      mockSendMessageStream.mockReturnValue(
+        (async function* () {
+          yield {
+            type: ServerGeminiEventType.StopHookLoop,
+            value: {
+              iterationCount: 1,
+              reasons: ['Single hook execution'],
+            },
+          };
+        })(),
+      );
+
+      const { result } = renderTestHook();
+
+      await act(async () => {
+        await result.current.submitQuery('single iteration query');
+      });
+
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'stop_hook_loop',
+            iterationCount: 1,
+            reasons: ['Single hook execution'],
+          }),
+          expect.any(Number),
+        );
+      });
+    });
+  });
+
+  describe('HookSystemMessage Event', () => {
+    it('should handle HookSystemMessage event and add stop_hook_system_message history item', async () => {
+      mockSendMessageStream.mockReturnValue(
+        (async function* () {
+          yield {
+            type: ServerGeminiEventType.HookSystemMessage,
+            value: '🔄 Ralph iteration 3 | No completion promise set',
+          };
+        })(),
+      );
+
+      const { result } = renderTestHook();
+
+      await act(async () => {
+        await result.current.submitQuery('test query with hook system message');
+      });
+
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'stop_hook_system_message',
+            message: '🔄 Ralph iteration 3 | No completion promise set',
+          }),
+          expect.any(Number),
+        );
+      });
+
+      expect(result.current.streamingState).toBe(StreamingState.Idle);
+    });
+
+    it('should display HookSystemMessage after content', async () => {
+      mockSendMessageStream.mockReturnValue(
+        (async function* () {
+          yield {
+            type: ServerGeminiEventType.Content,
+            value: 'Here is the response',
+          };
+          yield {
+            type: ServerGeminiEventType.HookSystemMessage,
+            value: 'Stop hook feedback message',
+          };
+        })(),
+      );
+
+      const { result } = renderTestHook();
+
+      await act(async () => {
+        await result.current.submitQuery(
+          'query with response and hook message',
+        );
+      });
+
+      // Verify content was added
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'gemini',
+            text: 'Here is the response',
+          }),
+          expect.any(Number),
+        );
+      });
+
+      // Verify hook system message was added
+      await waitFor(() => {
+        expect(mockAddItem).toHaveBeenCalledWith(
+          expect.objectContaining({
+            type: 'stop_hook_system_message',
+            message: 'Stop hook feedback message',
+          }),
+          expect.any(Number),
+        );
+      });
+    });
+  });
 });

packages/cli/src/ui/hooks/useGeminiStream.ts

@@ -972,6 +972,53 @@ export const useGeminiStream = (
     });
   }, [handleLoopDetectionConfirmation]);
 
+  const handleUserPromptSubmitBlockedEvent = useCallback(
+    (
+      value: { reason: string; originalPrompt: string },
+      userMessageTimestamp: number,
+    ) => {
+      if (pendingHistoryItemRef.current) {
+        addItem(pendingHistoryItemRef.current, userMessageTimestamp);
+        setPendingHistoryItem(null);
+      }
+      addItem(
+        {
+          type: 'user_prompt_submit_blocked',
+          reason: value.reason,
+          originalPrompt: value.originalPrompt,
+        } as HistoryItemWithoutId,
+        userMessageTimestamp,
+      );
+    },
+    [addItem, pendingHistoryItemRef, setPendingHistoryItem],
+  );
+
+  const handleStopHookLoopEvent = useCallback(
+    (
+      value: {
+        iterationCount: number;
+        reasons: string[];
+        stopHookCount: number;
+      },
+      userMessageTimestamp: number,
+    ) => {
+      if (pendingHistoryItemRef.current) {
+        addItem(pendingHistoryItemRef.current, userMessageTimestamp);
+        setPendingHistoryItem(null);
+      }
+      addItem(
+        {
+          type: 'stop_hook_loop',
+          iterationCount: value.iterationCount,
+          reasons: value.reasons,
+          stopHookCount: value.stopHookCount,
+        } as HistoryItemWithoutId,
+        userMessageTimestamp,
+      );
+    },
+    [addItem, pendingHistoryItemRef, setPendingHistoryItem],
+  );
+
   const processGeminiStreamEvents = useCallback(
     async (
       stream: AsyncIterable<GeminiEvent>,
@@ -1053,14 +1100,24 @@ export const useGeminiStream = (
             }
             break;
           case ServerGeminiEventType.HookSystemMessage:
-            // Display system message from hooks (e.g., Ralph Loop iteration info)
-            // This is handled as a content event to show in the UI
-            geminiMessageBuffer = handleContentEvent(
-              event.value + '\n',
-              geminiMessageBuffer,
+            // Display system message from Stop hooks with "Stop says:" prefix
+            addItem(
+              {
+                type: 'stop_hook_system_message',
+                message: event.value,
+              } as HistoryItemWithoutId,
               userMessageTimestamp,
             );
             break;
+          case ServerGeminiEventType.UserPromptSubmitBlocked:
+            handleUserPromptSubmitBlockedEvent(
+              event.value,
+              userMessageTimestamp,
+            );
+            break;
+          case ServerGeminiEventType.StopHookLoop:
+            handleStopHookLoopEvent(event.value, userMessageTimestamp);
+            break;
           default: {
             // enforces exhaustive switch-case
             const unreachable: never = event;
@@ -1089,6 +1146,9 @@ export const useGeminiStream = (
       setThought,
       pendingHistoryItemRef,
       setPendingHistoryItem,
+      handleUserPromptSubmitBlockedEvent,
+      handleStopHookLoopEvent,
+      addItem,
     ],
   );
 

packages/cli/src/ui/types.ts

@@ -361,6 +361,36 @@ export type HistoryItemBtw = HistoryItemBase & {
   btw: BtwProps;
 };
 
+/**
+ * UserPromptSubmit hook blocked event.
+ * Displayed when a UserPromptSubmit hook blocks the user's prompt.
+ */
+export type HistoryItemUserPromptSubmitBlocked = HistoryItemBase & {
+  type: 'user_prompt_submit_blocked';
+  reason: string;
+  originalPrompt: string;
+};
+
+/**
+ * Stop hook loop event.
+ * Displayed when Stop hooks create a loop, forcing the agent to continue.
+ */
+export type HistoryItemStopHookLoop = HistoryItemBase & {
+  type: 'stop_hook_loop';
+  iterationCount: number;
+  reasons: string[];
+  stopHookCount: number;
+};
+
+/**
+ * Stop hook system message.
+ * Displayed when Stop hooks return a systemMessage to show to the user.
+ */
+export type HistoryItemStopHookSystemMessage = HistoryItemBase & {
+  type: 'stop_hook_system_message';
+  message: string;
+};
+
 // Using Omit<HistoryItem, 'id'> seems to have some issues with typescript's
 // type inference e.g. historyItem.type === 'tool_group' isn't auto-inferring that
 // 'tools' in historyItem.
@@ -395,7 +425,10 @@ export type HistoryItemWithoutId =
   | HistoryItemArenaAgentComplete
   | HistoryItemArenaSessionComplete
   | HistoryItemInsightProgress
-  | HistoryItemBtw;
+  | HistoryItemBtw
+  | HistoryItemUserPromptSubmitBlocked
+  | HistoryItemStopHookLoop
+  | HistoryItemStopHookSystemMessage;
 
 export type HistoryItem = HistoryItemWithoutId & { id: number };
 

packages/cli/src/ui/utils/textUtils.test.ts

@@ -9,7 +9,7 @@ import type {
   ToolCallConfirmationDetails,
   ToolEditConfirmationDetails,
 } from '@qwen-code/qwen-code-core';
-import { escapeAnsiCtrlCodes } from './textUtils.js';
+import { escapeAnsiCtrlCodes, sanitizeSensitiveText } from './textUtils.js';
 
 describe('textUtils', () => {
   describe('escapeAnsiCtrlCodes', () => {
@@ -167,4 +167,71 @@ describe('textUtils', () => {
       });
     });
   });
+
+  describe('sanitizeSensitiveText', () => {
+    it('should return text unchanged if no sensitive patterns', () => {
+      const text = 'Hello, this is a normal prompt';
+      expect(sanitizeSensitiveText(text)).toBe(text);
+    });
+
+    it('should redact OpenAI-style API keys', () => {
+      const text = 'Use API key sk-1234567890abcdefghijklmnopqrstuv for access';
+      expect(sanitizeSensitiveText(text)).toBe(
+        'Use API key sk-***REDACTED*** for access',
+      );
+    });
+
+    it('should redact api_key assignments', () => {
+      const text = 'api_key=supersecretkey123456789012';
+      expect(sanitizeSensitiveText(text)).toBe('api_key=***REDACTED***');
+    });
+
+    it('should redact Bearer tokens', () => {
+      const text = 'Authorization: Bearer abc123token456xyz';
+      expect(sanitizeSensitiveText(text)).toBe(
+        'Authorization: Bearer ***REDACTED***',
+      );
+    });
+
+    it('should redact password assignments', () => {
+      const text = 'password=mysecretpassword123';
+      expect(sanitizeSensitiveText(text)).toBe('password=***REDACTED***');
+    });
+
+    it('should redact AWS access keys', () => {
+      const text = 'AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE';
+      expect(sanitizeSensitiveText(text)).toBe(
+        'AWS_ACCESS_KEY_ID=***REDACTED***',
+      );
+    });
+
+    it('should truncate long text', () => {
+      const text = 'a'.repeat(300);
+      const result = sanitizeSensitiveText(text, 200);
+      expect(result.length).toBe(200);
+      expect(result.endsWith('...')).toBe(true);
+    });
+
+    it('should handle custom max length', () => {
+      const text =
+        'This is a test prompt with sk-1234567890abcdefghijklmnopqrstuv';
+      const result = sanitizeSensitiveText(text, 20);
+      expect(result.length).toBe(20);
+      expect(result).toBe('This is a test pr...');
+    });
+
+    it('should handle empty string', () => {
+      expect(sanitizeSensitiveText('')).toBe('');
+    });
+
+    it('should redact multiple sensitive patterns', () => {
+      const text =
+        'api_key=secretkey12345678901234 and password=mypass123 and sk-test123456789012345678901';
+      const result = sanitizeSensitiveText(text);
+      expect(result).toContain('***REDACTED***');
+      expect(result).not.toContain('secretkey12345678901234');
+      expect(result).not.toContain('mypass123');
+      expect(result).not.toContain('sk-test123456789012345678901');
+    });
+  });
 });

packages/cli/src/ui/utils/textUtils.ts

@@ -214,3 +214,77 @@ export function escapeAnsiCtrlCodes<T>(obj: T): T {
 
   return newObj !== null ? newObj : obj;
 }
+
+/**
+ * Patterns that may indicate sensitive information like API keys, tokens, passwords.
+ */
+const SENSITIVE_PATTERNS: Array<{ pattern: RegExp; replacement: string }> = [
+  // API keys with common prefixes
+  {
+    pattern: /(sk-[a-zA-Z0-9]{20,})/g,
+    replacement: 'sk-***REDACTED***',
+  },
+  {
+    pattern: /(api[_-]?key[_-]?[=:]\s*)[a-zA-Z0-9_-]{20,}/gi,
+    replacement: '$1***REDACTED***',
+  },
+  // Bearer tokens
+  {
+    pattern: /(Bearer\s+)[a-zA-Z0-9._-]+/gi,
+    replacement: '$1***REDACTED***',
+  },
+  // Generic tokens
+  {
+    pattern: /(token[_-]?[=:]\s*)[a-zA-Z0-9._-]{10,}/gi,
+    replacement: '$1***REDACTED***',
+  },
+  // Passwords in connection strings or assignments
+  {
+    pattern: /(password[_-]?[=:]\s*)[^\s]+/gi,
+    replacement: '$1***REDACTED***',
+  },
+  {
+    pattern: /(pwd[_-]?[=:]\s*)[^\s]+/gi,
+    replacement: '$1***REDACTED***',
+  },
+  // AWS keys
+  {
+    pattern: /(AKIA[A-Z0-9]{16})/g,
+    replacement: '***REDACTED***',
+  },
+  // Generic secret patterns
+  {
+    pattern: /(secret[_-]?[=:]\s*)[a-zA-Z0-9._-]{10,}/gi,
+    replacement: '$1***REDACTED***',
+  },
+];
+
+/**
+ * Sanitizes text by redacting potentially sensitive information like API keys,
+ * tokens, and passwords. Also truncates long text to a maximum length.
+ *
+ * @param text The text to sanitize
+ * @param maxLength Maximum length of the output text (default: 200)
+ * @returns Sanitized and truncated text
+ */
+export function sanitizeSensitiveText(
+  text: string,
+  maxLength: number = 200,
+): string {
+  let result = text;
+
+  // Apply each sensitive pattern replacement
+  for (const { pattern, replacement } of SENSITIVE_PATTERNS) {
+    result = result.replace(pattern, replacement);
+  }
+
+  // Truncate if too long
+  if (result.length > maxLength) {
+    if (maxLength <= 3) {
+      return result.slice(0, maxLength);
+    }
+    return result.slice(0, maxLength - 3) + '...';
+  }
+
+  return result;
+}