mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-29 20:20:57 +00:00
feat(cli): queue input editing — pop queued messages for editing via ↑/ESC (#2871)
* feat(cli): add queue input editing via Up arrow key Allow users to edit queued messages by pressing the Up arrow key when the cursor is at the top of the input. All queued messages are popped into the input field for revision before resubmission, reducing wasted turns from incorrect queued instructions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add missing mocks for InputPrompt tests and attachment mode guard - Add popAllQueuedMessages mock and messageQueue to UIState/UIActions mocks in InputPrompt.test.tsx to fix 25 test failures - Add !isAttachmentMode guard to prevent queue pop from conflicting with attachment navigation - Add single-message popAllMessages test case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address Copilot review - restrict to Up arrow, add tests, update docs - Only trigger queue pop on NAVIGATION_UP (arrow key), not HISTORY_UP (Ctrl+P), preserving existing Ctrl+P history navigation behavior - Update AsyncMessageQueue class docs to describe popLast() LIFO semantics - Add InputPrompt tests: Up arrow pops queue, Up arrow falls back to history when queue empty, Ctrl+P not intercepted by queue pop Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update fileoverview docs and make popAllMessages atomic via ref - Update @fileoverview to describe FIFO+LIFO capability instead of "Simple FIFO queue" - Use queueRef to make popAllMessages atomic, preventing duplicate pops from key auto-repeat before React re-renders Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: sync queueRef in addMessage/clearQueue and fall through on null pop - Update queueRef inside addMessage setter and clearQueue to keep ref in sync between renders, preventing stale reads after clearQueue - When popAllQueuedMessages returns null (queue already cleared), fall through to normal history navigation instead of consuming the key Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: remove dead popLast() and align popAllMessages separator to \n\n - Remove unused AsyncMessageQueue.popLast() (no production callers) - Change popAllMessages join separator from \n to \n\n for consistency with getQueuedMessagesText and auto-submit behavior Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: use hook's drainQueue for mid-turn drain to prevent double-consumption race The midTurnDrainRef previously used a separate messageQueueRef (synced from React state), while popAllMessages uses the hook's internal queueRef. If a tool completed between popAllMessages clearing queueRef and React re-rendering, midTurnDrainRef would read stale data and consume the same messages a second time. Switching to the hook's drainQueue makes both paths read from the same synchronous ref, eliminating the window for double consumption. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add missing popAllMessages mock and prepend branch test Add popAllMessages to useMessageQueue mock in AppContainer tests. Add test for prepending queued messages before existing input text. * feat: add ESC trigger, cursor preservation, and progressive hint - ESC pops queued messages before double-ESC clear logic - Cursor stays at user's editing position after pop via moveToOffset - Extract popQueueIntoInput helper to share logic between Up and ESC - QueuedMessageDisplay hint hides after 3 empty→non-empty transitions * test: add null-pop fallthrough test for queue race condition Verify that when React state shows non-empty queue but the ref is already drained (popAllQueuedMessages returns null), Up arrow falls through to normal history navigation instead of getting stuck. --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
72b7887194
commit
61ad9db9c1
15 changed files with 355 additions and 3 deletions
|
|
@ -27,6 +27,8 @@ import * as clipboardUtils from '../utils/clipboardUtils.js';
|
|||
import { createMockCommandContext } from '../../test-utils/mockCommandContext.js';
|
||||
import stripAnsi from 'strip-ansi';
|
||||
import chalk from 'chalk';
|
||||
import { useUIState } from '../contexts/UIStateContext.js';
|
||||
import { useUIActions } from '../contexts/UIActionsContext.js';
|
||||
|
||||
vi.mock('../hooks/useShellHistory.js');
|
||||
vi.mock('../hooks/useCommandCompletion.js');
|
||||
|
|
@ -34,12 +36,13 @@ vi.mock('../hooks/useInputHistory.js');
|
|||
vi.mock('../hooks/useReverseSearchCompletion.js');
|
||||
vi.mock('../utils/clipboardUtils.js');
|
||||
vi.mock('../contexts/UIStateContext.js', () => ({
|
||||
useUIState: vi.fn(() => ({ isFeedbackDialogOpen: false })),
|
||||
useUIState: vi.fn(() => ({ isFeedbackDialogOpen: false, messageQueue: [] })),
|
||||
}));
|
||||
vi.mock('../contexts/UIActionsContext.js', () => ({
|
||||
useUIActions: vi.fn(() => ({
|
||||
handleRetryLastPrompt: vi.fn(),
|
||||
temporaryCloseFeedbackDialog: vi.fn(),
|
||||
popAllQueuedMessages: vi.fn(() => null),
|
||||
})),
|
||||
}));
|
||||
|
||||
|
|
@ -2530,12 +2533,14 @@ describe('InputPrompt', () => {
|
|||
let mockUIActions: {
|
||||
handleRetryLastPrompt: ReturnType<typeof vi.fn>;
|
||||
temporaryCloseFeedbackDialog: ReturnType<typeof vi.fn>;
|
||||
popAllQueuedMessages: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
mockUIActions = {
|
||||
handleRetryLastPrompt: vi.fn(),
|
||||
temporaryCloseFeedbackDialog: vi.fn(),
|
||||
popAllQueuedMessages: vi.fn(() => null),
|
||||
};
|
||||
|
||||
// Override the mock for useUIActions
|
||||
|
|
@ -2645,6 +2650,165 @@ describe('InputPrompt', () => {
|
|||
unmount();
|
||||
});
|
||||
});
|
||||
|
||||
describe('queue input editing', () => {
|
||||
afterEach(() => {
|
||||
// Restore default mocks
|
||||
vi.mocked(useUIState).mockReturnValue({
|
||||
isFeedbackDialogOpen: false,
|
||||
messageQueue: [],
|
||||
} as ReturnType<typeof useUIState>);
|
||||
vi.mocked(useUIActions).mockReturnValue({
|
||||
handleRetryLastPrompt: vi.fn(),
|
||||
temporaryCloseFeedbackDialog: vi.fn(),
|
||||
popAllQueuedMessages: vi.fn(() => null),
|
||||
} as unknown as ReturnType<typeof useUIActions>);
|
||||
});
|
||||
|
||||
it('should pop queued messages into input on Up arrow when queue is non-empty', async () => {
|
||||
const mockPopAll = vi.fn(() => 'queued msg 1\n\nqueued msg 2');
|
||||
vi.mocked(useUIState).mockReturnValue({
|
||||
isFeedbackDialogOpen: false,
|
||||
messageQueue: ['queued msg 1', 'queued msg 2'],
|
||||
} as ReturnType<typeof useUIState>);
|
||||
vi.mocked(useUIActions).mockReturnValue({
|
||||
handleRetryLastPrompt: vi.fn(),
|
||||
temporaryCloseFeedbackDialog: vi.fn(),
|
||||
popAllQueuedMessages: mockPopAll,
|
||||
} as unknown as ReturnType<typeof useUIActions>);
|
||||
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} />,
|
||||
);
|
||||
await wait();
|
||||
|
||||
stdin.write('\u001B[A'); // Up arrow
|
||||
await wait();
|
||||
|
||||
expect(mockPopAll).toHaveBeenCalled();
|
||||
expect(props.buffer.setText).toHaveBeenCalledWith(
|
||||
'queued msg 1\n\nqueued msg 2',
|
||||
);
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should prepend queued messages before existing input text', async () => {
|
||||
const mockPopAll = vi.fn(() => 'queued msg');
|
||||
vi.mocked(useUIState).mockReturnValue({
|
||||
isFeedbackDialogOpen: false,
|
||||
messageQueue: ['queued msg'],
|
||||
} as ReturnType<typeof useUIState>);
|
||||
vi.mocked(useUIActions).mockReturnValue({
|
||||
handleRetryLastPrompt: vi.fn(),
|
||||
temporaryCloseFeedbackDialog: vi.fn(),
|
||||
popAllQueuedMessages: mockPopAll,
|
||||
} as unknown as ReturnType<typeof useUIActions>);
|
||||
|
||||
// Set existing text in buffer
|
||||
props.buffer.text = 'existing input';
|
||||
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} />,
|
||||
);
|
||||
await wait();
|
||||
|
||||
stdin.write('\u001B[A'); // Up arrow
|
||||
await wait();
|
||||
|
||||
expect(props.buffer.setText).toHaveBeenCalledWith(
|
||||
'queued msg\nexisting input',
|
||||
);
|
||||
// Cursor should be positioned at start of existing text
|
||||
expect(props.buffer.moveToOffset).toHaveBeenCalledWith(
|
||||
'queued msg'.length + 1, // popped length + newline
|
||||
);
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should pop queued messages on ESC when queue is non-empty', async () => {
|
||||
const mockPopAll = vi.fn(() => 'queued msg');
|
||||
vi.mocked(useUIState).mockReturnValue({
|
||||
isFeedbackDialogOpen: false,
|
||||
messageQueue: ['queued msg'],
|
||||
} as ReturnType<typeof useUIState>);
|
||||
vi.mocked(useUIActions).mockReturnValue({
|
||||
handleRetryLastPrompt: vi.fn(),
|
||||
temporaryCloseFeedbackDialog: vi.fn(),
|
||||
popAllQueuedMessages: mockPopAll,
|
||||
} as unknown as ReturnType<typeof useUIActions>);
|
||||
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} />,
|
||||
);
|
||||
await wait();
|
||||
|
||||
stdin.write('\u001B'); // ESC
|
||||
await wait();
|
||||
|
||||
expect(mockPopAll).toHaveBeenCalled();
|
||||
expect(props.buffer.setText).toHaveBeenCalledWith('queued msg');
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should fall through to history when pop returns null (race condition)', async () => {
|
||||
// Simulate: React state says queue is non-empty, but queueRef was
|
||||
// already drained by another pop/drain — popAllQueuedMessages returns null.
|
||||
const mockPopAll = vi.fn(() => null);
|
||||
vi.mocked(useUIState).mockReturnValue({
|
||||
isFeedbackDialogOpen: false,
|
||||
messageQueue: ['stale msg'],
|
||||
} as ReturnType<typeof useUIState>);
|
||||
vi.mocked(useUIActions).mockReturnValue({
|
||||
handleRetryLastPrompt: vi.fn(),
|
||||
temporaryCloseFeedbackDialog: vi.fn(),
|
||||
popAllQueuedMessages: mockPopAll,
|
||||
} as unknown as ReturnType<typeof useUIActions>);
|
||||
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} />,
|
||||
);
|
||||
await wait();
|
||||
|
||||
stdin.write('\u001B[A'); // Up arrow
|
||||
await wait();
|
||||
|
||||
expect(mockPopAll).toHaveBeenCalled();
|
||||
expect(props.buffer.setText).not.toHaveBeenCalled();
|
||||
expect(mockInputHistory.navigateUp).toHaveBeenCalled();
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should navigate history on Up arrow when queue is empty', async () => {
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} />,
|
||||
);
|
||||
await wait();
|
||||
|
||||
stdin.write('\u001B[A'); // Up arrow
|
||||
await wait();
|
||||
|
||||
expect(mockInputHistory.navigateUp).toHaveBeenCalled();
|
||||
unmount();
|
||||
});
|
||||
|
||||
it('should not intercept Ctrl+P when queue is non-empty', async () => {
|
||||
vi.mocked(useUIState).mockReturnValue({
|
||||
isFeedbackDialogOpen: false,
|
||||
messageQueue: ['queued msg'],
|
||||
} as ReturnType<typeof useUIState>);
|
||||
|
||||
const { stdin, unmount } = renderWithProviders(
|
||||
<InputPrompt {...props} />,
|
||||
);
|
||||
await wait();
|
||||
|
||||
stdin.write('\u0010'); // Ctrl+P
|
||||
await wait();
|
||||
|
||||
expect(mockInputHistory.navigateUp).toHaveBeenCalled();
|
||||
unmount();
|
||||
});
|
||||
});
|
||||
});
|
||||
function clean(str: string | undefined): string {
|
||||
if (!str) return '';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue