📋 Review Summary

This PR introduces a feature to allow users to edit queued messages by pressing the Up arrow key when the cursor is at the top of the input field. The implementation adds popLast() to AsyncMessageQueue, popAllMessages() to the useMessageQueue hook, and integrates this functionality into the InputPrompt component with appropriate UI hints in QueuedMessageDisplay. The code is well-structured with good test coverage, though there are a few areas that could be improved.

🔍 General Feedback

• The implementation follows existing patterns in the codebase (similar to how other queue operations are handled)
• Good separation of concerns: core queue logic in packages/core, UI hook in packages/cli, and component integration
• Comprehensive test coverage with 26 unit tests mentioned
• Proper i18n support across 6 languages (de, en, ja, pt, ru, zh)
• The feature addresses a real UX pain point (reducing wasted turns from incorrect queued instructions)
• Consistent with existing keyboard shortcut patterns (Esc for cancel, now Up for edit)

🎯 Specific Feedback

🟡 High

• File: packages/cli/src/ui/components/InputPrompt.tsx:839-857 - The condition for triggering the queue pop is complex and may have edge cases. The logic checks both Command.HISTORY_UP and Command.NAVIGATION_UP with cursor position checks, but the interaction with attachment mode (isAttachmentMode) isn't explicitly handled. Consider adding a guard to ensure this doesn't interfere with attachment navigation:

  // Add !isAttachmentMode guard to prevent conflict
  if (
    !isAttachmentMode &&  // Add this check
    uiState.messageQueue.length > 0 &&
    ...
  )

• File: packages/cli/src/ui/hooks/useMessageQueue.ts:58-62 - The popAllMessages function joins messages with single \n, but getQueuedMessagesText uses \n\n. This inconsistency could lead to confusing behavior when messages are re-submitted. Consider using the same separator (\n\n) for consistency with the auto-submit behavior:

  const allText = messageQueue.join('\n\n');  // Use double newline for consistency

🟢 Medium

• File: packages/core/src/utils/asyncMessageQueue.ts:50-57 - The popLast() method is specific to this feature but may have broader utility. Consider adding a doc comment explaining the intended use case and behavior, similar to other methods in the class:

  /**
   * Remove and return the last item from the queue, or null if empty.
   * Useful for retrieving the most recently queued message for editing.
   */
  popLast(): T | null { ... }

• File: packages/cli/src/ui/components/QueuedMessageDisplay.tsx:46-50 - The edit hint is always shown when there are messages in the queue, but it doesn't indicate when the feature is unavailable (e.g., during streaming). Consider adding a condition to only show when the feature is actually usable:

  // Only show hint when user can actually interact (e.g., not during streaming)
  {messageQueue.length > 0 && streamingState !== StreamingState.Responding && (
    <Box paddingLeft={2}>
      <Text dimColor italic>
        {t('Press ↑ to edit queued messages')}
      </Text>
    </Box>
  )}

• File: packages/cli/src/ui/components/InputPrompt.tsx:839-857 - When popping messages and prepending to current buffer text, there's no validation of the resulting text length. If the user has a long message already in the input and pops multiple queued messages, it could create an unexpectedly large input. Consider adding a safeguard or at least a comment about this behavior.

🔵 Low

• File: packages/cli/src/i18n/locales/de.js:1442 - The German translation "Drücken Sie ↑, um Nachrichten in der Warteschlange zu bearbeiten" is quite long compared to the English version. This might cause UI layout issues in the terminal. Consider shortening to "↑ drücken, um Nachrichten zu bearbeiten" (consistent with other German translations in the file that are more concise).

• File: packages/cli/src/ui/components/QueuedMessageDisplay.tsx - The component doesn't receive streamingState as a prop, which would be needed to conditionally show the edit hint. This would require updating the component's interface and all call sites. This is a minor suggestion for future enhancement.

• File: packages/cli/src/ui/hooks/useMessageQueue.test.ts:256 - The test "should pop all messages from queue" verifies the joined format, but doesn't test the edge case of a single message (which shouldn't have any separator). Consider adding a test case for a single message in the queue.

✅ Highlights

• Test coverage is excellent: The PR includes tests for both AsyncMessageQueue.popLast() and useMessageQueue.popAllMessages(), including edge cases (empty queue returns null)
• Clean API design: The popAllMessages() function returns string | null, making it easy for callers to handle the empty queue case without try-catch
• Thoughtful UX consideration: The feature falls back gracefully to history navigation when the queue is empty, maintaining existing behavior
• Proper TypeScript usage: All new functions have proper type annotations and return types
• Good i18n coverage: All major supported languages have been updated with the new translation string
• Non-breaking change: The feature is additive and doesn't modify existing behavior, only adds new functionality when the queue is non-empty

Thanks for the thorough review! Addressed the feedback in 9a419ca:

🟡 High

• !isAttachmentMode guard — ✅ Fixed. Added guard to prevent queue pop from conflicting with attachment navigation.
• Join separator (\n vs \n\n) — Keeping \n intentionally. This matches upstream Claude Code behavior (popAllEditable uses \n). The \n\n separator in getQueuedMessagesText is for auto-submission where messages are combined into a single prompt, while \n is better for editing in the input buffer.

🟢 Medium

• QueuedMessageDisplay hint during streaming — The hint is intentionally always shown when the queue has messages. Queued messages only appear during streaming (that's when users submit to the queue), so the hint is always actionable when visible. Adding streamingState as a prop would add unnecessary complexity.
• Text length validation — No practical limit needed; the input buffer already handles large text.

🟢 Low

• Single message test — ✅ Added test case for single message (no separator in output).
• InputPrompt test failures — ✅ Fixed. Added missing popAllQueuedMessages mock and messageQueue: [] to UIState mock in InputPrompt.test.tsx, fixing 25 test failures.