From 5ddef23d949b0ebb5ed90d6ae4904798d1619bca Mon Sep 17 00:00:00 2001 From: adabarbulescu <94562950+adabarbulescu@users.noreply.github.com> Date: Fri, 7 Aug 2026 20:12:21 +0300 Subject: [PATCH 1/8] fix(welcome): rotate startup tips (#5871) --- static/index.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/static/index.html b/static/index.html index 8257660fe..0136f2316 100644 --- a/static/index.html +++ b/static/index.html @@ -1005,7 +1005,7 @@ var tips = mobile ? phone : desktop; var el = document.getElementById('welcome-tip'); if (el) { - el.textContent = 'Pick a model if you want, or just type.'; + el.textContent = tips[Math.floor(Math.random() * tips.length)]; } fetch('/api/version').then(function(r){return r.json()}).then(function(d){ if (d.version) window._appVersion = d.version; From 36d409842177e18017dac2fa4bbc5266bb451ac7 Mon Sep 17 00:00:00 2001 From: Jakub Grula Date: Fri, 7 Aug 2026 19:15:50 +0200 Subject: [PATCH 2/8] fix: Edit box formatting was removing triple tick boxes (#5737) --- static/js/chat.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/static/js/chat.js b/static/js/chat.js index ea2d8c1bb..ca583c5fc 100644 --- a/static/js/chat.js +++ b/static/js/chat.js @@ -4787,7 +4787,8 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr if (msgIndex < 0) return; const bodyEl = userMsgElement.querySelector('.body'); - const currentText = bodyEl ? bodyEl.textContent.trim().replace(/\s*\[\d+ attachment\(s\)\]$/, '') : ''; + let currentText = (userMsgElement.dataset.raw || (bodyEl ? bodyEl.textContent : '') || '').trim(); + currentText = currentText.replace(/\s*\[\d+ attachment\(s\)\]$/, ''); // Replace body with an editable textarea const editor = document.createElement('textarea'); From f1e96d102e5692fca3a91b1a40f46e22028e7ecb Mon Sep 17 00:00:00 2001 From: Husam Date: Fri, 7 Aug 2026 20:33:14 +0300 Subject: [PATCH 3/8] fix(tool_parsing): require a pipe on the Qwen bare end marker (#5829) The `end` branch of _QWEN_BARE_MARKER_RE had both pipes optional (`\|?end\|?`), so it also matched a bare `end` between whitespace and replaced it with a space. Messages containing Ruby, Lua or shell code that closes a block with a lone `end` had those lines deleted, and ordinary prose lost the word too. Require at least one pipe so only real turn markers match; `|end`, `end|`, `|end|` and `/|end|` strip exactly as before. Applied to the duplicated pattern in static/js/chatRenderer.js as well. Fixes #5547 --- src/tool_parsing.py | 6 +- static/js/chatRenderer.js | 5 +- tests/test_tool_parsing_bare_end_marker.py | 96 ++++++++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 tests/test_tool_parsing_bare_end_marker.py diff --git a/src/tool_parsing.py b/src/tool_parsing.py index 2885cc00f..98dc1b5f6 100644 --- a/src/tool_parsing.py +++ b/src/tool_parsing.py @@ -187,8 +187,12 @@ _FUNCTION_MODEL_NAME_RE = re.compile( _FUNCTION_MODEL_PARAMS_OPEN_RE = re.compile(r"\s*", re.IGNORECASE) _FUNCTION_MODEL_PARAMS_CLOSE_RE = re.compile(r"", re.IGNORECASE) _QWEN_ROLE_MARKER_RE = re.compile(r"?|?", re.IGNORECASE) +# At least one pipe is required around `end`. Both pipes used to be optional +# (`\|?end\|?`), which also matched a bare `end` on its own line and deleted it +# from ordinary prose and from Ruby/Lua/shell snippets that close blocks with +# one; see #5547. `|end`, `end|`, `|end|` and `/|end|` still strip as before. _QWEN_BARE_MARKER_RE = re.compile( - r"(?:^|[\t\r\n ])(?:\|?end\|?|/?\|end\|)(?=[\t\r\n ]|$)|" + r"(?:^|[\t\r\n ])(?:/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|" r"(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)", re.IGNORECASE, ) diff --git a/static/js/chatRenderer.js b/static/js/chatRenderer.js index 10709679d..1d6e2e4a9 100644 --- a/static/js/chatRenderer.js +++ b/static/js/chatRenderer.js @@ -478,7 +478,10 @@ const DSML_STRAY_RE = /<\s*\/?\s*[｜|]+\s*DSML\s*[｜|]+[^>]*>/gi; const DSML_INVOKE_RE = /<\s*[｜|]+\s*DSML\s*[｜|]+\s*invoke\b[^>]*>[\s\S]*?(?:<\s*\/\s*[｜|]+\s*DSML\s*[｜|]+\s*invoke\s*>|$)/gi; const RAW_OPENAI_TOOL_JSON_RE = /(?:\[\s*)?\{\s*"function"\s*:\s*\{[\s\S]*?\}\s*,\s*"id"\s*:\s*"[^"]*"\s*,\s*"type"\s*:\s*"function"\s*\}\s*\]?/gi; const QWEN_ROLE_MARKER_RE = /<\/?\|(?:assistant|assistan|user|system|tool)\|>?|<\/\|end\|>?/gi; -const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\|?end\|?|\/?\|end\|)(?=[\t\r\n ]|$)|(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)/gi; +// Keep in sync with _QWEN_BARE_MARKER_RE in src/tool_parsing.py. At least one +// pipe is required around `end`: with both optional (`\|?end\|?`) this also ate +// a bare `end` on its own line, breaking Ruby/Lua/shell snippets (#5547). +const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)/gi; // Self-narration about tool results (model echoing stdout/exit_code) const TOOL_NARRATION_RE = /(?:The (?:result|output) shows?:?\s*)?-?\s*(?:stdout|stderr|exit_code):\s*.+/gi; diff --git a/tests/test_tool_parsing_bare_end_marker.py b/tests/test_tool_parsing_bare_end_marker.py new file mode 100644 index 000000000..6167c8dde --- /dev/null +++ b/tests/test_tool_parsing_bare_end_marker.py @@ -0,0 +1,96 @@ +"""Regression: the Qwen bare-marker scrub must not eat a lone `end` (#5547). + +`_QWEN_BARE_MARKER_RE` cleans Qwen turn markers that leak into content. Its +`end` branch was `\\|?end\\|?` — both pipes optional — so it also matched a bare +`end` surrounded by whitespace and replaced it with a space. Any message +containing Ruby, Lua or shell code that closes a block with a lone `end` had +those lines silently deleted, in the stored text and in the rendered message. + +Requiring at least one pipe keeps every real marker (`|end`, `end|`, `|end|`, +`/|end|`) stripping as before. The same pattern is duplicated in +static/js/chatRenderer.js, so the JS copy is checked here too — the two must +not drift. +""" +import json +import re +import shutil +import subprocess +from pathlib import Path + +import pytest + +import src.agent_tools # noqa: F401 (break agent_tools<->tool_parsing import cycle) +from src.tool_parsing import strip_tool_blocks + +_REPO = Path(__file__).resolve().parent.parent +_CHAT_RENDERER = _REPO / "static" / "js" / "chatRenderer.js" + +# Inputs that must survive untouched, and the substring that proves they did. +KEPT = [ + ("loop do\n puts \"yo\"\nend\n", "\nend"), # the reported Ruby case + ("if x then\nend", "\nend"), + ("function f()\nend\n", "\nend"), + ("a end b", "a end b"), + ("append end", "append end"), + ("END", "END"), + ("\nEnd\n", "End"), +] + +# Real markers — at least one pipe, plus the role word — with the exact output +# they must still produce. Asserted as equality rather than "marker not in out" +# so narrowing the pattern can't pass by deleting more than it should. +STRIPPED = [ + ("a |end| b", "a b"), + ("a /|end| b", "a b"), + ("a |end b", "a b"), + ("a end| b", "a b"), + ("x assistant y", "x y"), +] + + +@pytest.mark.parametrize("text,kept", KEPT) +def test_bare_end_survives_stripping(text, kept): + assert kept in strip_tool_blocks(text) + + +@pytest.mark.parametrize("text,expected", STRIPPED) +def test_piped_end_markers_are_still_stripped(text, expected): + assert strip_tool_blocks(text) == expected + + +def test_bare_end_inside_a_fenced_block_survives(): + """The scrub runs over the whole message, fenced regions included.""" + out = strip_tool_blocks("Here:\n```ruby\nloop do\n puts 1\nend\n```\nDone.") + assert "\nend\n" in out + + +def _js_bare_marker_regex_source(): + src = _CHAT_RENDERER.read_text(encoding="utf-8") + m = re.search(r"^const QWEN_BARE_MARKER_RE = (/.*/[gimsuy]*);$", src, re.MULTILINE) + assert m, "QWEN_BARE_MARKER_RE literal not found in chatRenderer.js" + return m.group(1) + + +def test_js_copy_of_the_pattern_matches_the_python_one(): + """Guard the duplication: the JS branch must require a pipe too.""" + if shutil.which("node") is None: + pytest.skip("node binary not on PATH") + + cases = [text for text, _ in KEPT] + [text for text, _ in STRIPPED] + script = ( + "const RE = %s;\n" + "const cases = JSON.parse(process.argv[1]);\n" + "console.log(JSON.stringify(cases.map(c => c.replace(RE, ' '))));" + % _js_bare_marker_regex_source() + ) + result = subprocess.run( + ["node", "--input-type=module", "-e", script, json.dumps(cases)], + cwd=_REPO, capture_output=True, timeout=15, text=True, + ) + assert result.returncode == 0, f"node failed:\n{result.stderr}" + got = json.loads(result.stdout.splitlines()[-1]) + + for (text, kept), out in zip(KEPT, got): + assert kept in out, f"JS regex dropped {kept!r} from {text!r}" + for (text, expected), out in zip(STRIPPED, got[len(KEPT):]): + assert out == expected, f"JS regex: {text!r} -> {out!r}, expected {expected!r}" From 99566d28b53cdd53efaecff05a61ef3b7b5daa06 Mon Sep 17 00:00:00 2001 From: Husam Date: Fri, 7 Aug 2026 20:34:50 +0300 Subject: [PATCH 4/8] fix(chat): stop ArrowUp from eating an unsent multi-line prompt (#5875) 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" + ) From f06a0a30a80e6739a7f2d7f9b23ed38a8ffb21fd Mon Sep 17 00:00:00 2001 From: Samy <12219635+touzenesmy@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:04:53 -0400 Subject: [PATCH 5/8] fix(session): restore session URL hash writes (removed in cf4e240a) (#5872) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix: restore session URL hash writes (removed in cf4e240a) Restores history.replaceState() calls in selectSession() and materializePendingSession() that were dropped during the July 23 merge. Without these, chat URLs never update the address bar hash, making sessions unshareable and causing bare-URL reloads to land on the welcome screen instead of restoring the last active chat. Root cause: selectSession() had its hash-write deliberately removed; materializePendingSession() lost its during a larger refactor that added the stale-response and incognito guards. Fixes #5870 (upstream) * fix: session URL hash lost when sending message mid-stream Two independent bugs caused the session hash to disappear from the URL: Bug 1 — ReferenceError in catch block silently killed error recovery In handleChatSubmit, two const variables (streamingTTS at line 1922 and abortCtrl at line 1741) were declared inside the try block but referenced in the catch block. Since const is block-scoped in JavaScript, they were undefined in catch, causing a ReferenceError that silently aborted the error handler. This prevented materializePendingSession() from ever being called, so no hash was written to the URL. Fix: Hoisted both as let declarations before the try { block. Bug 2 — Dual sessions.js ES module instances with mismatched state app.js imported sessions.js with a version query string (?v=20260722ctxheader4) while every other module imported ./sessions.js without one. The browser treated them as different URLs, creating two separate module instances with independent _pendingChat and currentSessionId state. createDirectChat() set pending on one instance while handleChatSubmit() checked hasPendingChat() on the other — so the pending session never materialized. Fix: Removed the version query string from the sessions.js import in app.js and from the modulepreload + script tags in index.html. All modules now share a single sessions.js instance. Bonus guard: _adoptOpenedSessionBeforeAutoCreate() now checks hasPendingChat() before adopting a stale DOM-active session, preventing the send path from landing in the wrong session when a New Chat is pending. --------- Co-authored-by: samy --- static/app.js | 4 ++-- static/index.html | 8 ++++---- static/js/chat.js | 9 +++++++-- static/js/sessions.js | 5 +++++ 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/static/app.js b/static/app.js index c9e3a567f..9d05d8991 100644 --- a/static/app.js +++ b/static/app.js @@ -10,14 +10,14 @@ import modelsModule from './js/models.js?v=20260715startupcalm2'; import ragModule from './js/rag.js'; import presetsModule from './js/presets.js'; import searchModule from './js/search.js'; -import chatModule from './js/chat.js?v=20260722ctxheader4'; +import chatModule from './js/chat.js?v=20260801fix1'; import compareModule from './js/compare/index.js?v=20260723compareicon2'; import documentModule from './js/document.js?v=20260722emailfastindex1'; import searchChatModule from './js/search-chat.js'; import { makeWindowDraggable } from './js/windowDrag.js'; import markdownModule from './js/markdown.js'; import chatRenderer from './js/chatRenderer.js?v=20260722emailfastindex1'; -import sessionModule from './js/sessions.js?v=20260722ctxheader4'; +import sessionModule from './js/sessions.js'; import memoryModule from './js/memory.js?v=20260722memoryloading1'; import voiceRecorderModule from './js/voiceRecorder.js'; import censorModule from './js/censor.js'; diff --git a/static/index.html b/static/index.html index 0136f2316..0fea836ba 100644 --- a/static/index.html +++ b/static/index.html @@ -250,9 +250,9 @@ - + - + @@ -2504,7 +2504,7 @@ - + @@ -2522,7 +2522,7 @@ - + diff --git a/static/js/chat.js b/static/js/chat.js index ca583c5fc..3c8bbe850 100644 --- a/static/js/chat.js +++ b/static/js/chat.js @@ -349,6 +349,9 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr async function _adoptOpenedSessionBeforeAutoCreate() { if (!sessionModule || !sessionModule.getCurrentSessionId || sessionModule.getCurrentSessionId()) return true; + // Don't adopt a stale session when the user explicitly started a New Chat + // (pending state set) — the send path must materialize the pending session. + if (sessionModule.hasPendingChat && sessionModule.hasPendingChat()) return false; const activeRowId = document.querySelector('.list-item.active-session[data-session-id], .session-item.active[data-session-id]')?.dataset?.sessionId || ''; const hashId = _hashSessionCandidate(); const lastSelectedId = String(window.__odysseusLastSelectedSessionId || '').trim(); @@ -1403,6 +1406,8 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr currentAccumulated = ''; currentHolder = null; + let abortCtrl = null; + let streamingTTS = false; try { // Re-enable auto-scroll when user sends a message uiModule.setAutoScroll(true); @@ -1716,7 +1721,7 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr } - const abortCtrl = new AbortController(); + abortCtrl = new AbortController(); abortCtrl._reason = ''; currentAbort = abortCtrl; @@ -1897,7 +1902,7 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr let isThinking = false; let thinkingStartTime = null; // Streaming TTS: synthesize sentence-by-sentence during streaming - const streamingTTS = !!(window.aiTTSManager && window.aiTTSManager.autoPlay && window.aiTTSManager.available); + streamingTTS = !!(window.aiTTSManager && window.aiTTSManager.autoPlay && window.aiTTSManager.available); if (streamingTTS) window.aiTTSManager.streamingStart(); // Multi-bubble agent tracking let roundHolder = holder; // Current AI text bubble (changes per round) diff --git a/static/js/sessions.js b/static/js/sessions.js index cf59d478c..edf83c8a4 100644 --- a/static/js/sessions.js +++ b/static/js/sessions.js @@ -1847,6 +1847,10 @@ export async function selectSession(id, { keepSidebar = false, showLoading = tru const _isTransientChat = !!_meta && (_meta.folder === 'Assistant' || _meta.folder === 'Tasks'); if (!_isTransientChat) { Storage.set('lastSessionId', id); + // Update URL hash without triggering hashchange handler + if (window.location.hash !== '#' + id) { + history.replaceState(null, '', '#' + id); + } } // Restore character preset for persistent chats try { @@ -2313,6 +2317,7 @@ export async function materializePendingSession() { currentSessionId = payload.id; if (!isIncognito) { Storage.set('lastSessionId', payload.id); + history.replaceState(null, '', '#' + payload.id); } // Reload the sidebar in the background. Awaiting this used to block the first From 378518f6dfb994481a8ec5fbb0032d4a8c23c4e1 Mon Sep 17 00:00:00 2001 From: Samy <12219635+touzenesmy@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:06:17 -0400 Subject: [PATCH 6/8] Fix #5870: stale skills panel data on tab reopen (#5876) Remove early-return guard in loadSkills() that skipped both API re-fetch and renderSkillsList() when the Skills tab was reopened after first load. The cascade entrance animation is already handled inside renderSkillsList() via _cascadeNext, so the guard was unnecessary and caused deleted/edited skills to remain visible until a full page reload. Co-authored-by: samy --- static/js/skills.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/static/js/skills.js b/static/js/skills.js index 84974d446..b45403570 100644 --- a/static/js/skills.js +++ b/static/js/skills.js @@ -83,11 +83,9 @@ export async function loadSkills(cascade = false) { // Play the domino-in entrance on this load (set when the tab is opened, // not for the silent re-loads after an edit/delete). if (cascade) _cascadeNext = true; - if (cascade && loaded && !_loadPromise && _playSkillsCascade()) { - _cascadeNext = false; - updateCount(); - return; - } + // Always re-fetch when the tab is explicitly opened — the cascade + // animation is handled inside renderSkillsList() via _cascadeNext. + // Skipping the fetch here caused stale data on panel close/reopen (#5870). if (_loadPromise) return _loadPromise; _loadPromise = (async () => { try { From e4fa4ae5dd1d709ce4168397bd1d200fec1b2494 Mon Sep 17 00:00:00 2001 From: Wes Huber Date: Fri, 7 Aug 2026 13:07:07 -0700 Subject: [PATCH 7/8] fix(brain): give the Add Memory form a submit button and reliable Enter handling (#5830) The Brain > Add tab rendered only a text input and category select with no submit control, and Enter submission relied on a deprecated keypress listener that is not guaranteed to fire, so the form could not be submitted at all (#5828). Add a labelled submit button styled like the neighbouring Skill Import button (theme-io-btn, inline SVG icon), switch the Enter handler to keydown with preventDefault, ignore IME composition, and pin both submit paths with a source-level regression test. Fixes #5828 Co-authored-by: Claude Fable 5 --- static/app.js | 12 ++++- static/index.html | 1 + tests/test_memory_add_submit_regression.py | 54 ++++++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 tests/test_memory_add_submit_regression.py diff --git a/static/app.js b/static/app.js index 9d05d8991..2f1e8d4bf 100644 --- a/static/app.js +++ b/static/app.js @@ -1689,12 +1689,20 @@ function initializeEventListeners() { const newMemoryInput = el('new-memory-input'); if (newMemoryInput) { - newMemoryInput.addEventListener('keypress', (e) => { - if (e.key === 'Enter') { + // keydown, not the deprecated keypress: keypress is not guaranteed to + // fire for Enter everywhere, which left the Add Memory form with no + // working submit path (#5828). + newMemoryInput.addEventListener('keydown', (e) => { + if (e.key === 'Enter' && !e.isComposing) { + e.preventDefault(); memoryModule.addNewMemory(); } }); } + const newMemoryAddBtn = el('new-memory-add-btn'); + if (newMemoryAddBtn) { + newMemoryAddBtn.addEventListener('click', () => memoryModule.addNewMemory()); + } // Voice recording is handled by the dual-purpose send/mic button (see below) diff --git a/static/index.html b/static/index.html index 0fea836ba..fea4e20ac 100644 --- a/static/index.html +++ b/static/index.html @@ -365,6 +365,7 @@ Add a memory — e.g. 'I prefer concise replies' +
diff --git a/tests/test_memory_add_submit_regression.py b/tests/test_memory_add_submit_regression.py new file mode 100644 index 000000000..450d63003 --- /dev/null +++ b/tests/test_memory_add_submit_regression.py @@ -0,0 +1,54 @@ +"""The Brain > Add Memory form must be submittable (#5828). + +The form previously had no submit button and relied on a deprecated +``keypress`` listener for Enter, which is not guaranteed to fire on all +platforms — leaving the form with no working submit path. Pins: + +- a visible, keyboard-accessible submit button next to the category select; +- the button wired to ``memoryModule.addNewMemory()``; +- Enter handled via ``keydown`` with ``preventDefault()`` (and no lingering + ``keypress`` handler on the input). +""" +from pathlib import Path + +APP_JS = Path("static/app.js") +INDEX_HTML = Path("static/index.html") + + +def _add_memory_row(html): + start = html.index('id="new-memory-input"') + end = html.index("
", html.index('id="new-memory-add-btn"', start)) + return html[start:end] + + +def test_add_memory_form_renders_a_submit_button(): + html = INDEX_HTML.read_text() + row = _add_memory_row(html) + + assert 'id="new-memory-category"' in row, "button must sit in the same row as the form fields" + btn_start = row.index('id="new-memory-add-btn"') + btn_tag = row[row.rindex("", btn_start)] + assert 'type="button"' in btn_tag, "must not rely on implicit submit semantics" + + +def _new_memory_wiring_block(source): + start = source.index("const newMemoryInput = el('new-memory-input');") + end = source.index("// Voice recording", start) + return source[start:end] + + +def test_submit_button_is_wired_to_add_new_memory(): + block = _new_memory_wiring_block(APP_JS.read_text()) + + assert "el('new-memory-add-btn')" in block + assert "addEventListener('click', () => memoryModule.addNewMemory())" in block + + +def test_enter_uses_keydown_with_prevent_default(): + block = _new_memory_wiring_block(APP_JS.read_text()) + + assert "addEventListener('keydown'" in block + assert "addEventListener('keypress'" not in block, "keypress is deprecated and unreliable for Enter" + assert "e.preventDefault();" in block + assert "!e.isComposing" in block, "IME composition must not submit the form" + assert "memoryModule.addNewMemory();" in block From 42da399b4d9d814400b8e19365dffedad5b66ab3 Mon Sep 17 00:00:00 2001 From: Matyas Gosztonyi Date: Sat, 8 Aug 2026 23:06:41 +0200 Subject: [PATCH 8/8] fix(email): route summaries through shared LLM adapter (#5841) * fix(email): route summaries through shared llm adapter * chore(ci): refresh PR checks * fix(email): preserve scheduled summary safeguards --------- Co-authored-by: Matyas Fenyves <16389204+uhhgoat@users.noreply.github.com> --- routes/email_helpers.py | 120 +++++++ routes/email_pollers.py | 32 +- routes/email_routes.py | 94 +++--- static/js/emailLibrary.js | 7 +- static/js/emailLibrary/utils.js | 19 ++ tests/test_email_summary_error_ui_js.py | 52 +++ tests/test_email_summary_llm.py | 406 ++++++++++++++++++++++++ 7 files changed, 670 insertions(+), 60 deletions(-) create mode 100644 tests/test_email_summary_error_ui_js.py create mode 100644 tests/test_email_summary_llm.py diff --git a/routes/email_helpers.py b/routes/email_helpers.py index c8639e1c7..257f5f921 100644 --- a/routes/email_helpers.py +++ b/routes/email_helpers.py @@ -247,6 +247,7 @@ import re as _re_reply _REPLY_OPEN_RE = _re_reply.compile(r"<<<\s*(?:REPLY|SUMMARY|OUTPUT)\s*>>+", _re_reply.I) _REPLY_CLOSE_RE = _re_reply.compile(r"<<<\s*END\s*>>+", _re_reply.I) _REPLY_ROLE_MARKER_RE = _re_reply.compile(r"?|?", _re_reply.I) +_SUMMARY_BULLET_RE = _re_reply.compile(r"^(?:[-*\u2022]\s+|\d+[.)]\s+)") def _extract_reply(text: str) -> str: @@ -277,6 +278,125 @@ def _extract_reply(text: str) -> str: return _strip_think(t).strip() +def _build_email_summary_messages(sender: str, subject: str, body_for_llm: str) -> list[dict[str, str]]: + return [ + { + "role": "system", + "content": ( + "You are an email summarizer. Format: 1-3 short bullet points " + "(use '- '). Cover: main point, action items, deadlines. If the " + "email has attachments (marked '--- ATTACHMENTS ---'), USE THEIR " + "CONTENTS - pull invoice totals, deadlines, key clauses, concrete " + "numbers/dates from PDFs/docs into the bullets. Be terse.\n\n" + "OUTPUT FORMAT: Put ONLY the bullet points between these exact " + "markers, each on its own line:\n" + "<<>>\n" + "- ...\n" + "<<>>\n" + "Any reasoning must come BEFORE <<>> (ideally inside " + "...). Only the text between the markers is kept." + ), + }, + { + "role": "user", + "content": ( + f"From: {sender}\nSubject: {subject}\n\n{body_for_llm[:12000]}" + "\n\n---\n\nSummarize the email. Output the bullets between " + "<<>> and <<>>." + ), + }, + ] + + +async def _generate_email_summary( + url: str, + model: str, + sender: str, + subject: str, + body_for_llm: str, + *, + headers: dict | None = None, + max_tokens: int = 8192, + timeout: int = 180, +) -> str: + """Generate an interactive email summary through the shared LLM adapter.""" + from src.llm_core import llm_call_async + + raw = await llm_call_async( + url=url, + model=model, + messages=_build_email_summary_messages(sender, subject, body_for_llm), + temperature=0.3, + max_tokens=max_tokens, + headers=headers, + timeout=timeout, + workload="foreground", + ) + return _normalize_email_summary(raw) + + +async def _generate_scheduled_email_summary( + url: str, + model: str, + sender: str, + subject: str, + body_for_llm: str, + *, + headers: dict | None = None, + owner: str | None = None, + max_tokens: int = 8192, + timeout: int = 180, +) -> str: + """Generate a scheduled summary through the background task candidate chain.""" + from src.task_endpoint import task_llm_call_async + + raw = await task_llm_call_async( + messages=_build_email_summary_messages(sender, subject, body_for_llm), + fallback_url=url, + fallback_model=model, + fallback_headers=headers, + owner=owner, + temperature=0.3, + max_tokens=max_tokens, + timeout=timeout, + ) + return _normalize_email_summary(raw) + + +def _normalize_email_summary(raw) -> str: + """Extract a stable cache/UI summary from provider output.""" + raw_text = raw or "" + if _REPLY_OPEN_RE.search(raw_text): + summary = _extract_reply(raw_text) + if summary: + return summary + + cleaned = _strip_think(raw_text).strip() + bullets = [ + line.strip() + for line in cleaned.splitlines() + if _SUMMARY_BULLET_RE.match(line.strip()) + ] + if bullets: + return "\n".join(bullets) + return cleaned.strip() + + +EMAIL_SUMMARY_ERROR_CODE = "email_summary_unavailable" +EMAIL_SUMMARY_ERROR_MESSAGE = "Failed to summarize" + + +def _email_summary_failure_log_detail(exc: BaseException) -> str: + """Return useful provider-failure metadata without echoing exception text.""" + detail = f"type={type(exc).__name__}" + status = getattr(exc, "status_code", None) + if status is None: + status = getattr(getattr(exc, "response", None), "status_code", None) + if isinstance(status, int): + detail += f" status={status}" + return detail + + def _apply_email_style_mechanics(text: str) -> str: """Enforce deterministic writing-style mechanics that models often miss.""" if not text: diff --git a/routes/email_pollers.py b/routes/email_pollers.py index 5d96bd0f9..a2507989d 100644 --- a/routes/email_pollers.py +++ b/routes/email_pollers.py @@ -40,6 +40,7 @@ from routes.email_helpers import ( _pre_retrieve_context, _attach_compose_uploads, _cleanup_compose_uploads, _q, SCHEDULED_DB, _EMAIL_REPLY_SYS_PROMPT_BASE, _email_cache_owner_clause, + _generate_scheduled_email_summary, _email_summary_failure_log_detail, ) logger = logging.getLogger(__name__) @@ -653,6 +654,7 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None no_msgid = 0 examined = 0 _summaries_created = 0 + _summary_failed = 0 _events_created = 0 _replies_drafted = 0 _reply_failed = 0 @@ -785,16 +787,17 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None if need_sum: try: - summary = await task_llm_call_async( - messages=[ - {"role": "system", "content": "You are an email summarizer. Format: 1-3 short bullet points (use '- '). Cover: main point, action items, deadlines. If the email has attachments (marked '--- ATTACHMENTS ---'), USE THEIR CONTENTS — pull out invoice totals, deadlines, key clauses, any concrete numbers/dates in PDFs/docs, and reflect them in the bullets. Be terse.\n\nOUTPUT FORMAT: Put ONLY the bullet points between these exact markers, each on its own line:\n<<>>\n- ...\n<<>>\nAny reasoning or planning must come BEFORE <<>> (ideally inside ...). Only the text between the markers is kept."}, - {"role": "user", "content": f"From: {sender}\nSubject: {subject}\n\n{body_for_llm[:12000]}\n\n---\n\nSummarize the email. Output the bullets between <<>> and <<>>."}, - ], - fallback_url=url, fallback_model=model, fallback_headers=headers, + summary = await _generate_scheduled_email_summary( + url=url, + model=model, + sender=sender, + subject=subject, + body_for_llm=body_for_llm, + headers=req_headers, owner=account_owner or None, - temperature=0.3, max_tokens=16384, timeout=240, + max_tokens=16384, + timeout=240, ) - summary = _extract_reply((summary or "").strip()) if summary: _c = _sql3.connect(SCHEDULED_DB) _c.execute(""" @@ -808,10 +811,19 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None _summaries_created += 1 _uid_text = uid.decode() if isinstance(uid, bytes) else str(uid) _detail_lines.append(f"summary · {_folder}#{_uid_text} · {subject or '(no subject)'} — {sender or '(unknown sender)'}") + else: + _summary_failed += 1 + _uid_text = uid.decode() if isinstance(uid, bytes) else str(uid) + _detail_lines.append(f"summary empty · {_folder}#{_uid_text} · {subject or '(no subject)'} — {sender or '(unknown sender)'}") except Exception as e: + _summary_failed += 1 _uid_text = uid.decode() if isinstance(uid, bytes) else str(uid) _detail_lines.append(f"summary failed · {_folder}#{_uid_text} · {subject or '(no subject)'} — {sender or '(unknown sender)'}") - logger.warning(f"Auto-summary {uid} failed: {e}") + logger.warning( + "Auto-summary uid=%s failed %s", + _uid_text, + _email_summary_failure_log_detail(e), + ) if need_reply: await _emit_progress(progress_cb, f"Drafting reply {processed + 1}/{_max_process} · checked {examined}/{len(uid_list)}") @@ -1320,6 +1332,8 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None parts.append(f"processed {processed} new") if auto_sum: parts.append(f"summarized {_summaries_created}") + if _summary_failed: + parts.append(f"{_summary_failed} summary failed") if auto_reply_draft: parts.append(f"drafted {_replies_drafted} repl" + ("y" if _replies_drafted == 1 else "ies")) if _reply_failed: diff --git a/routes/email_routes.py b/routes/email_routes.py index 3c8e407bd..76a744ce1 100644 --- a/routes/email_routes.py +++ b/routes/email_routes.py @@ -57,7 +57,8 @@ from routes.email_helpers import ( _extract_attachment_to_disk, _extract_html, _extract_text, _fetch_sender_thread_context, _pre_retrieve_context, _EMAIL_REPLY_SYS_PROMPT_BASE, _POOL_HOOKS, - _friendly_email_auth_error, + _friendly_email_auth_error, _email_summary_failure_log_detail, + _generate_email_summary, EMAIL_SUMMARY_ERROR_CODE, EMAIL_SUMMARY_ERROR_MESSAGE, SendEmailRequest, ExtractStyleRequest, ATTACHMENTS_DIR, COMPOSE_UPLOADS_DIR, SCHEDULED_DB, attachment_extract_dir, _email_cache_owner_clause, email_translation_body_hash, @@ -4766,8 +4767,6 @@ def setup_email_routes(): """Generate a quick AI summary of an email body.""" try: from src.endpoint_resolver import resolve_endpoint - from src.llm_core import _uses_max_completion_tokens, _restricts_temperature - import requests as _req body = data.get("body", "") subject = data.get("subject", "") @@ -4778,7 +4777,11 @@ def setup_email_routes(): if account_id: _assert_owns_account(account_id, owner) if not body: - return {"success": False, "error": "No body provided"} + return { + "success": False, + "error": "No body provided", + "error_code": "email_summary_missing_body", + } # If we know which UID this is, fetch the raw message and pull # attachment text so the summary can reference invoice totals, @@ -4807,53 +4810,43 @@ def setup_email_routes(): if not url: url, model, headers = resolve_endpoint("default", owner=owner) if not url or not model: - return {"success": False, "error": "No LLM endpoint configured"} + return { + "success": False, + "error": "No model configured for email summaries", + "error_code": "email_summary_not_configured", + } req_headers = {"Content-Type": "application/json"} if headers: req_headers.update(headers) - tok_key = "max_completion_tokens" if _uses_max_completion_tokens(model) else "max_tokens" - payload = { - "model": model, - "messages": [ - {"role": "system", "content": "You are an email summarizer. Format: 1-3 short bullet points (use '- '). Cover: main point, action items, deadlines. If the email has attachments (marked '--- ATTACHMENTS ---'), USE THEIR CONTENTS — pull invoice totals, deadlines, key clauses, concrete numbers/dates from PDFs/docs into the bullets. Be terse.\n\nOUTPUT FORMAT: Put ONLY the bullet points between these exact markers, each on its own line:\n<<>>\n- ...\n<<>>\nAny reasoning must come BEFORE <<>> (ideally inside ...). Only the text between the markers is kept."}, - {"role": "user", "content": f"From: {sender}\nSubject: {subject}\n\n{body_for_llm[:12000]}\n\n---\n\nSummarize the email. Output the bullets between <<>> and <<>>."}, - ], - tok_key: 8192, - "temperature": 0.3, - "stream": False, - } - # Reasoning models (o1/o3/o4/gpt-5) reject an explicit temperature. - if _restricts_temperature(model): - payload.pop("temperature", None) - resp = await asyncio.to_thread( - _req.post, url, json=payload, headers=req_headers, timeout=180 - ) - if not resp.ok: - return {"success": False, "error": f"LLM HTTP {resp.status_code}"} - rdata = resp.json() - msg = (rdata.get("choices") or [{}])[0].get("message", {}) - content = (msg.get("content") or "").strip() - content = _extract_reply(content) + try: + content = await _generate_email_summary( + url=url, + model=model, + sender=sender, + subject=subject, + body_for_llm=body_for_llm, + headers=req_headers, + max_tokens=8192, + timeout=180, + ) + except Exception as e: + logger.warning( + "Email summary LLM call failed %s", + _email_summary_failure_log_detail(e), + ) + return { + "success": False, + "error": EMAIL_SUMMARY_ERROR_MESSAGE, + "error_code": EMAIL_SUMMARY_ERROR_CODE, + } if not content: - # Model put everything in reasoning_content — extract bullet points - rc = (msg.get("reasoning_content") or "").strip() - # Find bullet-point style output (lines starting with -, •, *, or numbered) - bullet_lines = [] - for line in rc.split("\n"): - stripped = line.strip() - if re.match(r"^[-•*]\s+|^\d+[.)]\s+", stripped): - bullet_lines.append(stripped) - if bullet_lines: - content = "\n".join(bullet_lines) - else: - # Last resort: take the last paragraph - paragraphs = [p.strip() for p in rc.split("\n\n") if p.strip()] - content = paragraphs[-1] if paragraphs else rc[:500] - - if not content: - return {"success": False, "error": "Empty response from model"} + return { + "success": False, + "error": "The model returned an empty summary", + "error_code": "email_summary_empty", + } # Cache the summary if we have a message_id mid = data.get("message_id", "") @@ -4876,8 +4869,15 @@ def setup_email_routes(): return {"success": True, "summary": content, "model_used": model} except Exception as e: - logger.error(f"Failed to summarize: {e}") - return {"success": False, "error": "Mail operation failed"} + logger.error( + "Email summary route failed %s", + _email_summary_failure_log_detail(e), + ) + return { + "success": False, + "error": EMAIL_SUMMARY_ERROR_MESSAGE, + "error_code": EMAIL_SUMMARY_ERROR_CODE, + } @router.post("/translate") async def translate_email(data: dict, owner: str = Depends(require_owner)): diff --git a/static/js/emailLibrary.js b/static/js/emailLibrary.js index 6a0d3e294..32b906ddc 100644 --- a/static/js/emailLibrary.js +++ b/static/js/emailLibrary.js @@ -13,7 +13,7 @@ import { makeWindowDraggable } from './windowDrag.js'; import { _esc, _escLinkify, _extractName, _parseTurnMeta, _formatBubbleDate, _formatRecipients, _senderColor, _initials, - _sanitizeHtml, + _sanitizeHtml, _renderEmailSummaryError, _TALON_WROTE, _TALON_FROM, _TALON_SENT, _TALON_SUBJ, _TALON_TO, _TALON_ORIG_RE, _SIG_BLOAT_MIN_CHARS, } from './emailLibrary/utils.js'; @@ -7259,12 +7259,11 @@ async function _generateSummary(reader, data, btn) { if (label) label.textContent = 'Summary'; } } else { - content.innerHTML = `${_esc(result.error || 'Failed to summarize')}`; - panel.remove(); + _renderEmailSummaryError(content, result); } } catch (e) { sp.destroy(); - panel.remove(); + _renderEmailSummaryError(content, null); if (uiModule) uiModule.showError?.('Failed to summarize'); } finally { if (btn) btn.disabled = false; diff --git a/static/js/emailLibrary/utils.js b/static/js/emailLibrary/utils.js index 82a5c86ec..f634c9949 100644 --- a/static/js/emailLibrary/utils.js +++ b/static/js/emailLibrary/utils.js @@ -30,6 +30,25 @@ export function _esc(text) { return div.innerHTML; } +const _EMAIL_SUMMARY_ERROR_MESSAGES = Object.freeze({ + email_summary_missing_body: 'No email body to summarize', + email_summary_not_configured: 'No model configured for email summaries', + email_summary_empty: 'The model returned an empty summary', + email_summary_unavailable: 'Failed to summarize', +}); + +export function _emailSummaryErrorMessage(result) { + const code = String(result?.error_code || ''); + return _EMAIL_SUMMARY_ERROR_MESSAGES[code] || 'Failed to summarize'; +} + +export function _renderEmailSummaryError(container, result) { + const message = container.ownerDocument.createElement('span'); + message.style.color = 'var(--red)'; + message.textContent = _emailSummaryErrorMessage(result); + container.replaceChildren(message); +} + function _attrEsc(text) { return String(text ?? '') .replace(/"/g, '"') diff --git a/tests/test_email_summary_error_ui_js.py b/tests/test_email_summary_error_ui_js.py new file mode 100644 index 000000000..1afc3bec9 --- /dev/null +++ b/tests/test_email_summary_error_ui_js.py @@ -0,0 +1,52 @@ +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + + +_REPO = Path(__file__).resolve().parent.parent +_UTILS = (_REPO / "static" / "js" / "emailLibrary" / "utils.js").as_posix() +_HAS_NODE = shutil.which("node") is not None + +pytestmark = pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH") + + +def test_email_summary_renderer_ignores_untrusted_provider_error_text(): + secret = ( + "endpoint=https://private.example.internal/v1 provider=ollama " + "model=private-model response_body=private-response " + "Authorization: Bearer token-secret-value" + ) + script = f""" + import {{ _renderEmailSummaryError }} from '{_UTILS}'; + const host = {{ + ownerDocument: {{ + createElement() {{ return {{ style: {{}}, textContent: '' }}; }}, + }}, + replaceChildren(node) {{ this.child = node; }}, + }}; + _renderEmailSummaryError(host, {{ + error_code: 'email_summary_unavailable', + error: {json.dumps(secret)}, + }}); + console.log(JSON.stringify({{ + text: host.child.textContent, + color: host.child.style.color, + }})); + """ + + proc = subprocess.run( + ["node", "--input-type=module"], + input=script, + capture_output=True, + text=True, + cwd=str(_REPO), + timeout=30, + ) + + assert proc.returncode == 0, proc.stderr + rendered = json.loads(proc.stdout) + assert rendered == {"text": "Failed to summarize", "color": "var(--red)"} + assert secret not in proc.stdout diff --git a/tests/test_email_summary_llm.py b/tests/test_email_summary_llm.py new file mode 100644 index 000000000..b0ab7b3be --- /dev/null +++ b/tests/test_email_summary_llm.py @@ -0,0 +1,406 @@ +import asyncio +import json +import logging +import os +import sqlite3 +import sys +import tempfile +from pathlib import Path + +import pytest + + +_TMP_DATA = Path(tempfile.mkdtemp(prefix="odysseus-email-summary-")) +os.environ.setdefault("DATA_DIR", str(_TMP_DATA)) +os.environ.setdefault("DATABASE_URL", f"sqlite:///{_TMP_DATA / 'app.db'}") + +PROJECT_ROOT = Path(__file__).resolve().parent.parent +if str(PROJECT_ROOT) not in sys.path: + sys.path.insert(0, str(PROJECT_ROOT)) + + +def _route_endpoint(router, path: str, method: str): + method = method.upper() + for route in router.routes: + if route.path == path and method in getattr(route, "methods", set()): + return route.endpoint + raise AssertionError(f"route not found: {method} {path}") + + +@pytest.mark.asyncio +async def test_generate_email_summary_uses_shared_llm_adapter(monkeypatch): + import routes.email_helpers as email_helpers + import src.llm_core as llm_core + + calls = {} + + async def fake_llm_call_async(url, model, messages, **kwargs): + calls["url"] = url + calls["model"] = model + calls["messages"] = messages + calls["kwargs"] = kwargs + return "thinking before marker\n<<>>\n- Pay the invoice by Friday.\n<<>>" + + monkeypatch.setattr(llm_core, "llm_call_async", fake_llm_call_async) + + summary = await email_helpers._generate_email_summary( + url="https://chatgpt.com/backend-api/codex/responses", + model="gpt-5.5", + sender="Billing ", + subject="Invoice due", + body_for_llm="Please pay invoice 123 by Friday.", + headers={"Authorization": "Bearer test"}, + max_tokens=1234, + timeout=45, + ) + + assert summary == "- Pay the invoice by Friday." + assert calls["url"] == "https://chatgpt.com/backend-api/codex/responses" + assert calls["model"] == "gpt-5.5" + assert calls["kwargs"]["headers"] == {"Authorization": "Bearer test"} + assert calls["kwargs"]["temperature"] == 0.3 + assert calls["kwargs"]["max_tokens"] == 1234 + assert calls["kwargs"]["timeout"] == 45 + assert calls["kwargs"]["workload"] == "foreground" + assert calls["messages"][0]["role"] == "system" + assert calls["messages"][1]["role"] == "user" + + +@pytest.mark.asyncio +async def test_scheduled_email_summary_uses_background_fallback_chain(monkeypatch): + import routes.email_helpers as email_helpers + import src.llm_core as llm_core + import src.task_endpoint as task_endpoint + + candidates = [ + ("http://primary.invalid/v1", "primary-model", {"X-Candidate": "primary"}), + ("http://fallback.invalid/v1", "fallback-model", {"X-Candidate": "fallback"}), + ] + resolve_calls = [] + wait_calls = [] + llm_calls = [] + + def fake_resolve_task_candidates(**kwargs): + resolve_calls.append(kwargs) + return candidates + + async def fake_wait_for_interactive_quiet(label): + wait_calls.append(label) + return False + + async def fake_llm_call_async(url, model, messages, **kwargs): + llm_calls.append((url, model, messages, kwargs)) + if model == "primary-model": + raise RuntimeError("primary unavailable") + return "<<>>\n- Used the fallback model.\n<<>>" + + monkeypatch.setattr(task_endpoint, "resolve_task_candidates", fake_resolve_task_candidates) + monkeypatch.setattr(task_endpoint, "wait_for_interactive_quiet", fake_wait_for_interactive_quiet) + monkeypatch.setattr(llm_core, "llm_call_async", fake_llm_call_async) + + summary = await email_helpers._generate_scheduled_email_summary( + url="http://caller-fallback.invalid/v1", + model="caller-fallback-model", + sender="Sender ", + subject="Scheduled subject", + body_for_llm="Please summarize this scheduled email.", + headers={"Authorization": "Bearer test"}, + owner="alice", + max_tokens=321, + timeout=54, + ) + + assert summary == "- Used the fallback model." + assert resolve_calls == [{ + "fallback_url": "http://caller-fallback.invalid/v1", + "fallback_model": "caller-fallback-model", + "fallback_headers": {"Authorization": "Bearer test"}, + "owner": "alice", + }] + assert wait_calls == ["background task LLM"] + assert [call[1] for call in llm_calls] == ["primary-model", "fallback-model"] + assert all(call[3]["workload"] == "background" for call in llm_calls) + assert all(call[3]["max_tokens"] == 321 for call in llm_calls) + assert all(call[3]["timeout"] == 54 for call in llm_calls) + + +@pytest.mark.asyncio +async def test_scheduled_local_summary_is_preempted_by_foreground_call(monkeypatch): + import routes.email_helpers as email_helpers + import src.llm_core as llm_core + import src.task_endpoint as task_endpoint + + local_url = "http://127.0.0.1:11434/v1/chat/completions" + background_started = asyncio.Event() + never_release = asyncio.Event() + observed_workloads = [] + + monkeypatch.setenv("ODYSSEUS_LOCAL_MODEL_GATE", "true") + monkeypatch.setenv("BACKGROUND_TASK_FOREGROUND_GATE", "false") + monkeypatch.setattr(llm_core, "_LOCAL_MODEL_LOCK", asyncio.Lock()) + monkeypatch.setattr(llm_core, "_LOCAL_MODEL_CURRENT", {}) + monkeypatch.setattr(llm_core, "_LOCAL_MODEL_WAITING_FOREGROUND", 0) + monkeypatch.setattr( + task_endpoint, + "resolve_task_candidates", + lambda **_kwargs: [(local_url, "scheduled-model", {})], + ) + + async def fake_wait_for_interactive_quiet(_label): + return False + + async def gated_llm_call(url, model, messages, **kwargs): + assert messages + workload = kwargs.get("workload") + observed_workloads.append(workload) + async with llm_core._local_model_slot(url, model, workload=workload): + background_started.set() + await never_release.wait() + return "unreachable" + + monkeypatch.setattr(task_endpoint, "wait_for_interactive_quiet", fake_wait_for_interactive_quiet) + monkeypatch.setattr(llm_core, "llm_call_async", gated_llm_call) + + background_task = asyncio.create_task(email_helpers._generate_scheduled_email_summary( + url=local_url, + model="scheduled-model", + sender="Sender", + subject="Scheduled", + body_for_llm="Scheduled body", + owner="alice", + )) + foreground_task = None + try: + await asyncio.wait_for(background_started.wait(), timeout=1) + + async def run_foreground(): + async with llm_core._local_model_slot( + local_url, + "interactive-model", + workload="foreground", + ): + return True + + foreground_task = asyncio.create_task(run_foreground()) + with pytest.raises(asyncio.CancelledError): + await asyncio.wait_for(background_task, timeout=1) + assert await asyncio.wait_for(foreground_task, timeout=1) is True + assert observed_workloads == ["background"] + finally: + for task in (background_task, foreground_task): + if task is not None and not task.done(): + task.cancel() + + +@pytest.mark.asyncio +async def test_manual_email_summary_uses_shared_helper_and_caches(tmp_path, monkeypatch): + import routes.email_helpers as email_helpers + import routes.email_routes as email_routes + import src.endpoint_resolver as endpoint_resolver + + db_path = tmp_path / "scheduled_emails.db" + monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path) + monkeypatch.setattr(email_routes, "SCHEDULED_DB", db_path) + email_helpers._init_scheduled_db() + + resolve_calls = [] + + def fake_resolve_endpoint(kind, owner=None): + resolve_calls.append((kind, owner)) + assert kind == "utility" + assert owner == "alice" + return ( + "https://chatgpt.com/backend-api/codex/responses", + "gpt-5.5", + {"Authorization": "Bearer test"}, + ) + + helper_calls = {} + + async def fake_generate_email_summary(**kwargs): + helper_calls.update(kwargs) + return "- Manual summary" + + monkeypatch.setattr(endpoint_resolver, "resolve_endpoint", fake_resolve_endpoint) + monkeypatch.setattr(email_routes, "_generate_email_summary", fake_generate_email_summary) + + router = email_routes.setup_email_routes() + summarize = _route_endpoint(router, "/api/email/summarize", "POST") + + result = await summarize( + { + "body": "This is a long enough email body for manual summary.", + "subject": "Manual subject", + "from": "Sender ", + "message_id": "", + "folder": "INBOX", + }, + owner="alice", + ) + + assert result == { + "success": True, + "summary": "- Manual summary", + "model_used": "gpt-5.5", + } + assert resolve_calls == [("utility", "alice")] + assert helper_calls["url"] == "https://chatgpt.com/backend-api/codex/responses" + assert helper_calls["model"] == "gpt-5.5" + assert helper_calls["headers"]["Authorization"] == "Bearer test" + assert helper_calls["headers"]["Content-Type"] == "application/json" + + conn = sqlite3.connect(db_path) + try: + row = conn.execute( + "SELECT owner, summary, model_used FROM email_summaries WHERE message_id=?", + ("",), + ).fetchone() + finally: + conn.close() + assert row == ("alice", "- Manual summary", "gpt-5.5") + + +@pytest.mark.asyncio +@pytest.mark.parametrize("exception_kind", ["http", "runtime"]) +async def test_manual_email_summary_never_exposes_provider_exception( + monkeypatch, + caplog, + exception_kind, +): + from fastapi import HTTPException + import routes.email_routes as email_routes + import src.endpoint_resolver as endpoint_resolver + + secret_detail = ( + "endpoint=https://private.example.internal/v1 provider=ollama " + "model=private-model response_body=private-response " + "Authorization: Bearer token-secret-value" + ) + + def fake_resolve_endpoint(kind, owner=None): + assert kind == "utility" + assert owner == "alice" + return ( + "https://private.example.internal/v1", + "private-model", + {"Authorization": "Bearer token-secret-value"}, + ) + + async def fail_summary(**_kwargs): + if exception_kind == "http": + raise HTTPException(status_code=502, detail=secret_detail) + raise RuntimeError(secret_detail) + + monkeypatch.setattr(endpoint_resolver, "resolve_endpoint", fake_resolve_endpoint) + monkeypatch.setattr(email_routes, "_generate_email_summary", fail_summary) + caplog.set_level(logging.WARNING, logger=email_routes.__name__) + + router = email_routes.setup_email_routes() + summarize = _route_endpoint(router, "/api/email/summarize", "POST") + result = await summarize( + { + "body": "This email body is long enough to summarize.", + "subject": "Sensitive provider failure", + "from": "Sender ", + }, + owner="alice", + ) + + assert result == { + "success": False, + "error": "Failed to summarize", + "error_code": "email_summary_unavailable", + } + exposed = json.dumps(result) + caplog.text + for marker in ( + "private.example.internal", + "ollama", + "private-model", + "private-response", + "token-secret-value", + ): + assert marker not in exposed + assert f"type={'HTTPException' if exception_kind == 'http' else 'RuntimeError'}" in caplog.text + + +@pytest.mark.asyncio +async def test_scheduled_email_summary_uses_shared_helper_and_caches(tmp_path, monkeypatch): + import routes.email_helpers as email_helpers + import routes.email_pollers as email_pollers + + db_path = tmp_path / "scheduled_emails.db" + monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path) + monkeypatch.setattr(email_pollers, "SCHEDULED_DB", db_path) + email_helpers._init_scheduled_db() + + raw_email = ( + b"From: Sender \r\n" + b"To: Alice \r\n" + b"Subject: Scheduled subject\r\n" + b"Message-ID: \r\n" + b"Date: Tue, 01 Jan 2026 12:00:00 +0000\r\n" + b"Content-Type: text/plain; charset=utf-8\r\n" + b"\r\n" + + (b"Please review this scheduled summary email. " * 8) + ) + + class FakeImap: + def __init__(self): + self.logout_calls = 0 + + def select(self, _folder, readonly=True): + return "OK", [] + + def uid(self, command, *args): + if command == "SEARCH": + return "OK", [b"1"] + if command == "FETCH": + return "OK", [(b"1 (RFC822)", raw_email)] + raise AssertionError(f"unexpected uid command: {command!r} {args!r}") + + def logout(self): + self.logout_calls += 1 + + fake_conn = FakeImap() + + def fake_resolve_task_candidates(owner=None): + assert owner == "alice" + return [( + "https://chatgpt.com/backend-api/codex/responses", + "gpt-5.5", + {"Authorization": "Bearer test"}, + )] + + helper_calls = {} + + async def fake_generate_email_summary(**kwargs): + helper_calls.update(kwargs) + return "- Scheduled summary" + + monkeypatch.setattr(email_pollers, "_load_settings", lambda: {"email_auto_summarize": True}) + monkeypatch.setattr(email_pollers, "_owner_for_email_account", lambda _account_id: "alice") + monkeypatch.setattr(email_pollers, "_imap_connect", lambda account_id=None, owner="": fake_conn) + monkeypatch.setattr(email_pollers, "_get_email_config", lambda account_id=None, owner="": {"from_address": "alice@example.com"}) + monkeypatch.setattr(email_pollers, "resolve_task_candidates", fake_resolve_task_candidates) + monkeypatch.setattr(email_pollers, "_generate_scheduled_email_summary", fake_generate_email_summary) + + result = await email_pollers._auto_summarize_pass_single(account_id="acct-alice") + + assert "summarized 1" in result + assert "summary failed" not in result + assert helper_calls["url"] == "https://chatgpt.com/backend-api/codex/responses" + assert helper_calls["model"] == "gpt-5.5" + assert helper_calls["headers"]["Authorization"] == "Bearer test" + assert helper_calls["headers"]["Content-Type"] == "application/json" + assert helper_calls["owner"] == "alice" + assert fake_conn.logout_calls == 1 + + conn = sqlite3.connect(db_path) + try: + row = conn.execute( + "SELECT owner, summary, model_used FROM email_summaries WHERE message_id=?", + ("",), + ).fetchone() + finally: + conn.close() + assert row == ("alice", "- Scheduled summary", "gpt-5.5")