fix(chat): stop ArrowUp from eating an unsent multi-line prompt

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
This commit is contained in:
husamemad 2026-08-02 12:20:48 +03:00
parent 25c9e735ef
commit 1c2ebc064e
3 changed files with 28 additions and 82 deletions

View file

@ -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 = '<svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round"><path d="M12 19V5M5 12l7-7 7 7"/></svg>';
const _micIcon = '<svg width="16" height="16" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round" stroke-linejoin="round"><path d="M12 1a3 3 0 0 0-3 3v8a3 3 0 0 0 6 0V4a3 3 0 0 0-3-3z"/><path d="M19 10v2a7 7 0 0 1-14 0v-2"/><line x1="12" y1="19" x2="12" y2="23"/><line x1="8" y1="23" x2="16" y2="23"/></svg>';

View file

@ -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) {

View file

@ -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"
)