From 1c2ebc064e3f499dc1063a1a58b3568f497a8109 Mon Sep 17 00:00:00 2001 From: husamemad Date: Sun, 2 Aug 2026 12:20:48 +0300 Subject: [PATCH] fix(chat): stop ArrowUp from eating an unsent multi-line prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit static/app.js carried a near-verbatim copy of the prompt-recall logic in static/js/composerArrowUpRecall.js, wired as a second capture-phase keydown listener on the same #message textarea. The copy omitted the draft guard the module has: it called preventDefault() and stopImmediatePropagation() unconditionally, then recalled history[0] over whatever the user had typed. Because it stopped immediate propagation, the copy won regardless of registration order — if it ran first the module never saw the event, and if it ran second the module had already declined to stop propagation on an unmatched draft. The guard at composerArrowUpRecall.js:109 was unreachable on the real page, so ArrowUp on a multi-line draft replaced it with the last sent prompt instead of moving the caret up a line. Delete the duplicate. The module keeps ownership of ArrowUp/ArrowDown recall, which is the behavior MODULE_SUMMARY.md documents ("on an empty composer") and the behavior tests/test_composer_arrow_up_recall_js.py already pins via test_non_empty_composer_does_not_recall and test_multiline_caret_navigation_preserved. Also correct a stale comment in the module that described the deleted behavior and contradicted the guard 35 lines above it, and add a regression test asserting app.js does not reintroduce a second handler. Fixes #5862 --- static/app.js | 83 ++--------------------- static/js/composerArrowUpRecall.js | 6 +- tests/test_composer_arrow_up_recall_js.py | 21 ++++++ 3 files changed, 28 insertions(+), 82 deletions(-) diff --git a/static/app.js b/static/app.js index 97f0ae77e..c9e3a567f 100644 --- a/static/app.js +++ b/static/app.js @@ -3908,85 +3908,10 @@ function startOdysseusApp() { const messageInput = el('message'); const modelPickerWrap = document.getElementById('model-picker-wrap'); - function _readComposerPromptHistory() { - const chatBox = document.getElementById('chat-history'); - if (!chatBox) return []; - return Array.from(chatBox.querySelectorAll('.msg-user')) - .reverse() - .map(msg => { - const body = msg.querySelector('.body'); - return msg.dataset?.raw || (body ? body.textContent : '') || ''; - }) - .filter(Boolean); - } - - if (messageInput && !messageInput._odysseusPromptRecallCapture) { - messageInput._odysseusPromptRecallCapture = true; - let recallHistory = []; - let recallIndex = -1; - let lastRecalled = ''; - const norm = (v) => String(v || '').replace(/\r\n/g, '\n').trimEnd(); - messageInput.addEventListener('input', () => { - if (norm(messageInput.value) === norm(lastRecalled)) return; - recallHistory = []; - recallIndex = -1; - lastRecalled = ''; - try { delete messageInput.dataset.odysseusRecallIndex; } catch {} - }, true); - messageInput.addEventListener('keydown', (e) => { - if (e.key !== 'ArrowUp' && e.key !== 'ArrowDown') return; - if (e.shiftKey || e.altKey || e.ctrlKey || e.metaKey || e.isComposing) return; - if (window._ghostAutocomplete?.isActive?.()) return; - const fresh = _readComposerPromptHistory(); - const history = fresh.length ? fresh : recallHistory; - if (!history.length) return; - const current = norm(messageInput.value); - let currentIndex = current ? history.findIndex(item => norm(item) === current) : -1; - if (current && currentIndex < 0 && current === norm(lastRecalled)) currentIndex = recallIndex; - if (current && currentIndex < 0) { - const markedIndex = Number(messageInput.dataset.odysseusRecallIndex); - if (Number.isInteger(markedIndex) && markedIndex >= 0 && markedIndex < history.length) { - currentIndex = markedIndex; - } - } - e.preventDefault(); - e.stopPropagation(); - e.stopImmediatePropagation(); - if (e.key === 'ArrowDown') { - if (currentIndex < 0) return; - const nextIndex = currentIndex - 1; - if (nextIndex < 0) { - recallHistory = history; - recallIndex = -1; - lastRecalled = ''; - try { delete messageInput.dataset.odysseusRecallIndex; } catch {} - messageInput.value = ''; - try { messageInput.selectionStart = messageInput.selectionEnd = 0; } catch {} - try { uiModule.autoResize(messageInput); } catch {} - return; - } - const recalled = history[nextIndex]; - recallHistory = history; - recallIndex = nextIndex; - lastRecalled = recalled; - try { messageInput.dataset.odysseusRecallIndex = String(nextIndex); } catch {} - messageInput.value = recalled; - try { messageInput.selectionStart = messageInput.selectionEnd = recalled.length; } catch {} - try { uiModule.autoResize(messageInput); } catch {} - return; - } - const nextIndex = currentIndex >= 0 ? Math.min(currentIndex + 1, history.length - 1) : 0; - const recalled = history[nextIndex]; - if (!recalled) return; - recallHistory = history; - recallIndex = nextIndex; - lastRecalled = recalled; - try { messageInput.dataset.odysseusRecallIndex = String(nextIndex); } catch {} - messageInput.value = recalled; - try { messageInput.selectionStart = messageInput.selectionEnd = recalled.length; } catch {} - try { uiModule.autoResize(messageInput); } catch {} - }, true); - } + // ArrowUp/ArrowDown prompt recall on #message lives in + // static/js/composerArrowUpRecall.js (wired from chat.js). Do not re-add a + // copy here: two capture-phase listeners on the same textarea meant the one + // without the draft guard won and ate unsent multi-line prompts (#5862). const _sendIcon = ''; const _micIcon = ''; diff --git a/static/js/composerArrowUpRecall.js b/static/js/composerArrowUpRecall.js index e0b20d6b4..83141bfe9 100644 --- a/static/js/composerArrowUpRecall.js +++ b/static/js/composerArrowUpRecall.js @@ -143,9 +143,9 @@ export function wireArrowUpRecall(composer, getUserMessages, options = {}) { return; } - // ArrowUp owns prompt history in the chat composer. If the current text - // is not already a recalled prompt, start from newest instead of letting - // the browser move the caret inside the textarea. + // ArrowUp walks older prompts. An unmatched draft already returned above, + // so reaching here means the composer is empty or holds a recalled prompt + // — the caret-navigation case is never hijacked. const nextIndex = currentIndex >= 0 ? Math.min(currentIndex + 1, history.length - 1) : 0; const recalled = history[nextIndex]; if (!recalled) { diff --git a/tests/test_composer_arrow_up_recall_js.py b/tests/test_composer_arrow_up_recall_js.py index eadc3bc94..022fcbc02 100644 --- a/tests/test_composer_arrow_up_recall_js.py +++ b/tests/test_composer_arrow_up_recall_js.py @@ -306,3 +306,24 @@ def test_integration_recalls_from_chat_history_dom(): ) assert proc.returncode == 0, proc.stderr assert json.loads(proc.stdout.strip()) == {"value": "stored prompt", "prevented": True} + + +def test_prompt_recall_is_not_duplicated_in_app_js(): + """Only composerArrowUpRecall.js may own ArrowUp on #message (issue #5862). + + static/app.js once carried a near-verbatim copy of this recall logic, wired + as a second capture-phase listener on the same textarea. That copy lacked + the draft guard here, and because it called stopImmediatePropagation it won + regardless of registration order — so a typed multi-line prompt was replaced + by the last sent one instead of the caret moving up a line. + """ + app_js = (_REPO / "static" / "app.js").read_text(encoding="utf-8") + for marker in ( + "_odysseusPromptRecallCapture", + "_readComposerPromptHistory", + "odysseusRecallIndex", + ): + assert marker not in app_js, ( + f"static/app.js reintroduces prompt recall ({marker!r}); " + "it belongs to static/js/composerArrowUpRecall.js alone" + )