From 396d0d0abd5c25ac7d1de8a73f240abb68c7f200 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 7 Jun 2026 23:18:51 -0700 Subject: [PATCH] =?UTF-8?q?Release=20v0.51.326=20=E2=80=94=20Release=20KP?= =?UTF-8?q?=20(#3618=20+=20#3802=20+=20#3762=20+=20#3810)=20(#3816)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Batch: mic STT capability probe+fallback (#3618, live-drive verified), journal cleanup on delete (#3802), minimal-schema SQL guard (#3762), Help hover readability (#3810). Full suite 8265, Codex SAFE, Opus SHIP, CI 11/11. Co-authored-by: nesquena-hermes Co-authored-by: dso2ng --- CHANGELOG.md | 8 ++ api/agent_sessions.py | 72 +++++++--- api/routes.py | 28 +++- api/run_journal.py | 26 ++++ api/turn_journal.py | 35 +++++ api/upload.py | 119 ++++++++++++++++ static/boot.js | 95 +++++++++++-- static/style.css | 8 +- tests/test_issue3227_help_tab.py | 25 ++++ ..._issue3762_importable_rows_schema_guard.py | 132 ++++++++++++++++++ .../test_issue3802_delete_session_journals.py | 113 +++++++++++++++ tests/test_sprint20.py | 54 ++++++- tests/test_voice_transcribe_endpoint.py | 77 +++++++++- 13 files changed, 755 insertions(+), 37 deletions(-) create mode 100644 tests/test_issue3762_importable_rows_schema_guard.py create mode 100644 tests/test_issue3802_delete_session_journals.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 5340aded..8ff26a54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ ## [Unreleased] +## [v0.51.326] — 2026-06-08 — Release KP (mic STT probe + journal cleanup + schema guard + help hover) + +### Fixed +- **Mic input prefers configured server-side STT, with a safe fallback.** A page-load capability probe defaults to the Hermes STT provider when one is configured; installs without server STT keep browser dictation, and a failed transcribe reverts to browser SpeechRecognition. (#3618, @dso2ng) +- **Deleting a conversation now also removes its turn/run journals** (previously the user's messages and full request/response payloads stayed on disk in plaintext after delete). (#3802) +- **Imported/CLI/agent sessions no longer vanish from the sidebar on minimal `state.db` schemas.** The session-list query now degrades gracefully when the `messages` table is absent or missing columns instead of raising and hiding every row. (#3762) +- **Help tab buttons stay readable on hover in every theme** (the hover fill no longer renders same-color text). (#3810) + ## [v0.51.325] — 2026-06-08 — Release KO (in-app Help tab) ### Added diff --git a/api/agent_sessions.py b/api/agent_sessions.py index bc39649f..488aa8af 100644 --- a/api/agent_sessions.py +++ b/api/agent_sessions.py @@ -447,11 +447,51 @@ def read_importable_agent_session_rows( origin_chat_id_expr = _optional_col('origin_chat_id', session_cols) origin_user_id_expr = _optional_col('origin_user_id', session_cols) platform_expr = _optional_col('platform', session_cols) - user_message_count_expr = ( - "COUNT(CASE WHEN LOWER(m.role) = 'user' THEN 1 END)" - if 'role' in message_cols - else "COUNT(m.id)" - ) + # Older/minimal state.db schemas can have NO ``messages`` table at all, + # or a ``messages`` table without a ``session_id`` / ``timestamp`` column. + # The projection SQL below joins ``messages`` and aggregates + # ``MAX(m.timestamp)`` unconditionally, so on those schemas the query + # raised ``sqlite3.OperationalError`` — which the caller + # (``get_cli_sessions``) swallows into an empty list, silently hiding + # ALL imported/CLI/agent sessions from the sidebar. Detect the columns + # and degrade gracefully (mirrors ``read_session_lineage_metadata``): + # only join/aggregate ``messages`` when it's actually usable, otherwise + # fall back to the per-session ``s.message_count`` / ``s.started_at``. (#3762) + messages_has_session_id = 'session_id' in message_cols + messages_has_timestamp = 'timestamp' in message_cols + use_messages_join = messages_has_session_id + count_col = 'id' if 'id' in message_cols else 'session_id' + + if use_messages_join: + actual_count_expr = f"COUNT(m.{count_col})" + if 'role' in message_cols: + user_message_count_expr = "COUNT(CASE WHEN LOWER(m.role) = 'user' THEN 1 END)" + else: + user_message_count_expr = f"COUNT(m.{count_col})" + last_activity_expr = "MAX(m.timestamp)" if messages_has_timestamp else "NULL" + join_clause = "LEFT JOIN messages m ON m.session_id = s.id" + group_by_clause = "GROUP BY s.id" + else: + # No usable messages table: use the denormalized per-session counts + # and ``started_at`` so the rows still surface in the sidebar. + actual_count_expr = "s.message_count" + user_message_count_expr = "s.message_count" + last_activity_expr = "NULL" + join_clause = "" + group_by_clause = "" + + if use_messages_join and messages_has_timestamp: + order_by_clause = "ORDER BY COALESCE(MAX(m.timestamp), s.started_at) DESC" + candidate_order_clause = ( + "ORDER BY COALESCE(\n" + " (SELECT MAX(mx.timestamp) FROM messages mx WHERE mx.session_id = s.id),\n" + " s.started_at\n" + " ) DESC,\n" + " s.started_at DESC" + ) + else: + order_by_clause = "ORDER BY s.started_at DESC" + candidate_order_clause = "ORDER BY s.started_at DESC" where_clauses = ["s.source IS NOT NULL"] params: list[object] = [] @@ -477,9 +517,9 @@ def read_importable_agent_session_rows( {parent_expr}, {ended_expr}, {end_reason_expr}, - COUNT(m.id) AS actual_message_count, + {actual_count_expr} AS actual_message_count, {user_message_count_expr} AS actual_user_message_count, - MAX(m.timestamp) AS last_activity + {last_activity_expr} AS last_activity """ if limit is not None: result_limit = max(0, int(limit)) @@ -500,19 +540,15 @@ def read_importable_agent_session_rows( SELECT s.id FROM sessions s WHERE {' AND '.join(where_clauses)} - ORDER BY COALESCE( - (SELECT MAX(mx.timestamp) FROM messages mx WHERE mx.session_id = s.id), - s.started_at - ) DESC, - s.started_at DESC + {candidate_order_clause} LIMIT ? ) {select_sql} FROM sessions s JOIN candidates c ON c.id = s.id - LEFT JOIN messages m ON m.session_id = s.id - GROUP BY s.id - ORDER BY COALESCE(MAX(m.timestamp), s.started_at) DESC + {join_clause} + {group_by_clause} + {order_by_clause} """, [*params, candidate_limit], ) @@ -521,10 +557,10 @@ def read_importable_agent_session_rows( f""" {select_sql} FROM sessions s - LEFT JOIN messages m ON m.session_id = s.id + {join_clause} WHERE {' AND '.join(where_clauses)} - GROUP BY s.id - ORDER BY COALESCE(MAX(m.timestamp), s.started_at) DESC + {group_by_clause} + {order_by_clause} """, params, ) diff --git a/api/routes.py b/api/routes.py index 0e2b5c00..eb54c0a2 100644 --- a/api/routes.py +++ b/api/routes.py @@ -3375,7 +3375,13 @@ from api.workspace import ( _is_remote_terminal_backend, _workspace_blocked_roots, ) -from api.upload import handle_upload, handle_upload_extract, handle_transcribe, handle_workspace_upload +from api.upload import ( + handle_upload, + handle_upload_extract, + handle_transcribe, + handle_transcribe_capability, + handle_workspace_upload, +) from api.streaming import ( _sse, _run_agent_streaming, @@ -5304,6 +5310,9 @@ def handle_get(handler, parsed) -> bool: pass return j(handler, settings) + if parsed.path == "/api/transcribe/capability": + return handle_transcribe_capability(handler) + if parsed.path == "/api/reasoning": # Current reasoning config (shared source of truth with the CLI — # reads display.show_reasoning and agent.reasoning_effort from @@ -7252,6 +7261,23 @@ def handle_post(handler, parsed) -> bool: shutil.rmtree(_session_attachment_dir(sid), ignore_errors=True) except Exception: logger.debug("Failed to clean attachment dir for deleted session %s", sid) + # Remove the turn-journal shards and the run-journal directory so a + # deleted conversation is not recoverable from disk. The session JSON + + # state.db rows are cleared above, but these journals retain the user's + # messages (turn journal) and the full request/response payloads (run + # journal) in plaintext. (#3802) + try: + from api.turn_journal import delete_turn_journal + + delete_turn_journal(sid) + except Exception: + logger.debug("Failed to delete turn journal for deleted session %s", sid) + try: + from api.run_journal import delete_run_journal + + delete_run_journal(sid) + except Exception: + logger.debug("Failed to delete run journal for deleted session %s", sid) # Prune the per-session agent lock so deleted sessions don't leak # Lock entries in SESSION_AGENT_LOCKS forever. with SESSION_AGENT_LOCKS_LOCK: diff --git a/api/run_journal.py b/api/run_journal.py index b7526797..c52e6b77 100644 --- a/api/run_journal.py +++ b/api/run_journal.py @@ -256,6 +256,32 @@ def find_run_summary(run_id: str, *, session_dir: Path | None = None) -> dict | return None +def delete_run_journal(session_id: str, *, session_dir: Path | None = None) -> bool: + """Remove the entire per-session run-journal directory (``_run_journal/{sid}/``). + + The run journal stores one directory per session containing a ``{rid}.jsonl`` + file per run, so removing the session's directory clears every run's full + request/response payloads. Invalid/empty ids and a missing directory are a + no-op so callers can invoke this unconditionally on delete. Returns ``True`` + if a directory was removed, ``False`` otherwise. + """ + import shutil + + sid = str(session_id or "").strip() + # Reject path-traversal ids: the regex below permits dots, so a bare "." or + # ".." would resolve `root / RUN_JOURNAL_DIR_NAME / sid` to the journal ROOT + # (or its parent) and rmtree the wrong directory. The route call site only + # passes real sids, but this is a public helper — guard it directly. + if sid in (".", "..") or not sid or "/" in sid or "\\" in sid or not _SAFE_ID_RE.fullmatch(sid): + return False + root = Path(session_dir) if session_dir is not None else _default_session_dir() + session_journal_dir = root / RUN_JOURNAL_DIR_NAME / sid + if not session_journal_dir.exists(): + return False + shutil.rmtree(session_journal_dir, ignore_errors=True) + return not session_journal_dir.exists() + + def stale_interrupted_event(session_id: str, run_id: str, *, after_seq: int | None = None) -> dict | None: summary = latest_run_summary(session_id, run_id) if summary.get("terminal") or not summary.get("event_count"): diff --git a/api/turn_journal.py b/api/turn_journal.py index e05363ca..b42bc2e4 100644 --- a/api/turn_journal.py +++ b/api/turn_journal.py @@ -242,5 +242,40 @@ def iter_turn_journal_session_ids(session_dir: Path) -> list[str]: return sorted(session_ids) +def delete_turn_journal(session_id: str, *, session_dir: Path | None = None) -> int: + """Remove every turn-journal shard for ``session_id``. + + Deletes both the pid-scoped shards (``{sid}~{pid}.jsonl``) written by + :func:`append_turn_journal_event` and the legacy single-file form + (``{sid}.jsonl``) that :func:`read_turn_journal` still merges. Returns the + number of files removed. Invalid/empty ids and a missing journal directory + are treated as a no-op so callers can invoke this unconditionally on delete. + """ + sid = str(session_id or "").strip() + # Reject "."/".." for parity with delete_run_journal — the regex permits + # dots, and a traversal id has no legitimate use here. + if sid in (".", "..") or not sid or "/" in sid or "\\" in sid or not _SESSION_ID_RE.fullmatch(sid): + return 0 + root = Path(session_dir) if session_dir is not None else _default_session_dir() + journal_dir = root / TURN_JOURNAL_DIR_NAME + if not journal_dir.exists(): + return 0 + removed = 0 + shards = list(journal_dir.glob(f"{sid}~*.jsonl")) + legacy = journal_dir / f"{sid}.jsonl" + if legacy.exists(): + shards.append(legacy) + for shard in shards: + try: + shard.unlink() + removed += 1 + except FileNotFoundError: + pass + except OSError: + # Best-effort cleanup; the caller logs the overall delete outcome. + pass + return removed + + def is_terminal_turn_event(event: dict) -> bool: return str((event or {}).get("event") or "") in _TERMINAL_EVENTS diff --git a/api/upload.py b/api/upload.py index aac45fd3..3125cb7e 100644 --- a/api/upload.py +++ b/api/upload.py @@ -398,6 +398,125 @@ def handle_transcribe(handler): pass +def _stt_provider_capability_from_module(stt): + """Return (available, provider) for a loaded transcription_tools module.""" + try: + load_cfg = getattr(stt, "_load_stt_config", None) + stt_config = load_cfg() if callable(load_cfg) else {} + cfg_dict = stt_config if isinstance(stt_config, dict) else {} + is_enabled = getattr(stt, "is_stt_enabled", None) + if callable(is_enabled) and not is_enabled(stt_config): + return False, "none" + + # Some tests and future agent releases expose the provider decision as a + # single helper. Use it when the lower-level capability flags are not + # available. The current agent module exposes the flags below, so the + # normal path mirrors _get_provider() without triggering its lazy local + # STT install side effect during a passive web page probe. + has_internal_flags = any( + hasattr(stt, name) + for name in ("_HAS_FASTER_WHISPER", "_HAS_OPENAI", "_HAS_MISTRAL") + ) + get_provider = getattr(stt, "_get_provider", None) + if callable(get_provider) and not has_internal_flags: + provider = str(get_provider(stt_config) or "none") + return provider not in ("", "none"), provider or "none" + + def env(name): + getter = getattr(stt, "get_env_value", None) + try: + if callable(getter): + return str(getter(name) or "").strip() + except Exception: + return "" + return os.getenv(name, "").strip() + + def has_local_command(): + helper = getattr(stt, "_has_local_command", None) + try: + return bool(helper()) if callable(helper) else False + except Exception: + return False + + def has_browser_audio_converter(): + helper = getattr(stt, "_find_ffmpeg_binary", None) + try: + return bool(helper()) if callable(helper) else False + except Exception: + return False + + def has_openai_audio(): + helper = getattr(stt, "_has_openai_audio_backend", None) + try: + return bool(helper()) if callable(helper) else False + except Exception: + return False + + def local_command_available(): + # The browser sends WebM/Ogg blobs; the local-command path converts + # non-WAV input through ffmpeg before invoking the command. + return has_local_command() and has_browser_audio_converter() + + def resolve_provider(provider): + if provider == "local": + if bool(getattr(stt, "_HAS_FASTER_WHISPER", False)): + return "local" + if local_command_available(): + return "local_command" + return "none" + if provider == "local_command": + if local_command_available(): + return "local_command" + if bool(getattr(stt, "_HAS_FASTER_WHISPER", False)): + return "local" + return "none" + if provider == "groq": + return "groq" if bool(getattr(stt, "_HAS_OPENAI", False)) and bool(env("GROQ_API_KEY")) else "none" + if provider == "openai": + return "openai" if bool(getattr(stt, "_HAS_OPENAI", False)) and has_openai_audio() else "none" + if provider == "mistral": + return "mistral" if bool(getattr(stt, "_HAS_MISTRAL", False)) and bool(env("MISTRAL_API_KEY")) else "none" + if provider == "xai": + try: + from tools.xai_http import resolve_xai_http_credentials + + return "xai" if resolve_xai_http_credentials().get("api_key") else "none" + except Exception: + return "none" + if provider == "elevenlabs": + return "elevenlabs" if bool(env("ELEVENLABS_API_KEY")) else "none" + return "none" + + explicit = "provider" in cfg_dict + if explicit: + configured = str(cfg_dict.get("provider") or "local") + provider = resolve_provider(configured) + return provider != "none", provider if provider != "none" else configured + + for candidate in ("local", "local_command", "groq", "openai", "mistral", "xai", "elevenlabs"): + provider = resolve_provider(candidate) + if provider != "none": + return True, provider + return False, "none" + except Exception: + return False, "none" + + + +def _stt_provider_capability(): + """Return (available, provider) for a cheap server-side STT capability probe.""" + try: + import tools.transcription_tools as stt + except ImportError: + return False, "none" + return _stt_provider_capability_from_module(stt) + + +def handle_transcribe_capability(handler): + available, provider = _stt_provider_capability() + return j(handler, {"ok": True, "available": bool(available), "provider": provider}) + + def handle_workspace_upload(handler): """Upload a file into a session's workspace directory. diff --git a/static/boot.js b/static/boot.js index 527270e9..34e8f8b8 100644 --- a/static/boot.js +++ b/static/boot.js @@ -470,7 +470,13 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu // Persist SR failure across reloads (e.g. Tailscale/network error) const _micForceMediaRecorderKey='mic_force_mediarecorder'; - let _forceMediaRecorder=!SpeechRecognition||localStorage.getItem(_micForceMediaRecorderKey)==='1'; + const _micForceMediaRecorderStored=localStorage.getItem(_micForceMediaRecorderKey); + // Prefer Hermes server-side STT (MediaRecorder -> /api/transcribe) only + // after the server confirms an STT provider is available. No stored key must + // keep browser SpeechRecognition as the first-click default until then; that + // avoids dropping the first dictation on installs without server STT. + let _serverSttAvailable=false; + let _forceMediaRecorder=!SpeechRecognition||(_micForceMediaRecorderStored===null?(_serverSttAvailable&&_canRecordAudio):_micForceMediaRecorderStored==='1'); // Raw audio mode preference: send audio file instead of transcribing let _rawAudioMode = localStorage.getItem('hermes-raw-audio-mode') === 'true'; @@ -485,7 +491,7 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu const statusText=status?status.querySelector('.status-text'):null; btn.style.display=''; // Show button — browser supports speech recognition or recording fallback - let recognition=(!_forceMediaRecorder&&SpeechRecognition)?new SpeechRecognition():null; + let recognition=null; let mediaRecorder=null; let mediaStream=null; let audioChunks=[]; @@ -556,6 +562,18 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu } } + function _isServerSttUnavailable(err){ + const status=err&&err.status; + if(status===404||status===503||status>=500) return true; + if(!status) return true; + const msg=String((err&&err.message)||'').toLowerCase(); + return msg.includes('unavailable')||msg.includes('not configured'); + } + + function _allowBrowserSttFallback(){ + return !!(SpeechRecognition&&localStorage.getItem(_micForceMediaRecorderKey)!=='1'); + } + async function _transcribeBlob(blob){ const ext=(blob.type&&blob.type.includes('ogg'))?'ogg':'webm'; const form=new FormData(); @@ -564,9 +582,21 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu try{ const res=await fetch('api/transcribe',{method:'POST',body:form}); const data=await res.json().catch(()=>({})); - if(!res.ok) throw new Error(data.error||'Transcription failed'); + if(!res.ok){ + const err=new Error(data.error||'Transcription failed'); + err.status=res.status; + throw err; + } _commitTranscript(data.transcript||''); }catch(err){ + if(_isServerSttUnavailable(err)&&_allowBrowserSttFallback()){ + window._micPendingSend=false; + localStorage.setItem(_micForceMediaRecorderKey,'0'); + _forceMediaRecorder=false; + recognition=_ensureSpeechRecognition(); + showToast(err.message||t('mic_network')); + return; + } window._micPendingSend=false; showToast(err.message||t('mic_network')); }finally{ @@ -600,14 +630,16 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu } window._stopMic=_stopMic; // expose for send-guard above - if(recognition && !_forceMediaRecorder){ - recognition.continuous=false; - recognition.interimResults=true; - recognition.lang=(typeof _locale!=='undefined'&&_locale._speech)||'en-US'; + function _ensureSpeechRecognition(){ + if(!SpeechRecognition) return null; + const sr=recognition||new SpeechRecognition(); + sr.continuous=false; + sr.interimResults=true; + sr.lang=(typeof _locale!=='undefined'&&_locale._speech)||'en-US'; - recognition.onstart=()=>{ _finalText=''; }; + sr.onstart=()=>{ _finalText=''; }; - recognition.onresult=(event)=>{ + sr.onresult=(event)=>{ let interim=''; let final=_finalText; for(let i=event.resultIndex;i{if(e&&e.preventDefault)e.preventDefault();$('fileInpu autoResize(); }; - recognition.onend=()=>{ + sr.onend=()=>{ const committed=_finalText ? (_prefix&&!_prefix.endsWith(' ')&&!_prefix.endsWith('\n') ? _prefix+' '+_finalText.trimStart() @@ -632,9 +664,10 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu window._micPendingSend=false; send(); } + _applyDeferredServerSttFlip(); }; - recognition.onerror=(event)=>{ + sr.onerror=(event)=>{ _setRecording(false); window._micPendingSend=false; _isRecording=false; @@ -651,8 +684,47 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu }; showToast(msgs[event.error]||t('mic_error')+event.error); }; + + return sr; } + if(!_forceMediaRecorder){ + recognition=_ensureSpeechRecognition(); + } + + async function _probeServerSttCapability(){ + if(!_canRecordAudio||_micForceMediaRecorderStored!==null) return; + try{ + const res=await fetch('api/transcribe/capability',{cache:'no-store'}); + const data=await res.json().catch(()=>({})); + if(res.ok&&data&&data.available){ + _serverSttAvailable=true; + if(!window._micActive){ + _forceMediaRecorder=true; + recognition=null; + } + } + }catch(_err){ + // Keep browser SpeechRecognition as the safe first-click default when the + // passive capability probe fails. + } + } + + // If the capability probe resolved WHILE a session was active, the flip to + // server STT was deferred to protect that in-flight session. Apply it once the + // session ends so subsequent clicks use the configured server STT as intended. + // Reads LIVE localStorage (not the init-time const) so a fallback that just + // persisted '0' is respected and not re-flipped. + function _applyDeferredServerSttFlip(){ + if(_serverSttAvailable&&!_forceMediaRecorder&&!window._micActive + &&localStorage.getItem(_micForceMediaRecorderKey)===null){ + _forceMediaRecorder=true; + recognition=null; + } + } + + _probeServerSttCapability(); + btn.onclick=async()=>{ // Race-condition guard: ignore rapid double-clicks if(_isRecording){ @@ -707,6 +779,7 @@ $('btnAttach').onclick=e=>{if(e&&e.preventDefault)e.preventDefault();$('fileInpu else if(window._micPendingSend){ window._micPendingSend=false; } + _applyDeferredServerSttFlip(); }; _activeCaptureMode=_rawAudioMode?'media-raw':'media-transcribe'; mediaRecorder.start(); diff --git a/static/style.css b/static/style.css index fc6f14c8..cd62b66d 100644 --- a/static/style.css +++ b/static/style.css @@ -1566,7 +1566,13 @@ .help-card-title{font-size:13.5px;font-weight:650;color:var(--text);margin-bottom:3px;} .help-card-desc{font-size:12px;color:var(--muted);line-height:1.45;margin-bottom:11px;} .help-card-link{display:inline-flex;align-items:center;gap:6px;text-decoration:none;padding:7px 13px;border-radius:7px;font-size:12px;font-weight:600;background:var(--input-bg);border:1px solid var(--border);color:var(--text);transition:all .15s;} - .help-card-link:hover{background:var(--accent);border-color:var(--accent);color:var(--accent-text,#1a1a1a);} + /* On hover the button fills with --accent; text must contrast against that fill. + --accent-text is an accent-on-background link color (often == --accent), so using + it here produces same-color text on the accent fill (invisible — e.g. slate/gold/ + ares/mono dark). --bg is the page background, which --accent is designed to contrast + against, so background-colored text on an accent fill stays readable in every theme. */ + .help-card-link:hover{background:var(--accent);border-color:var(--accent);color:var(--bg);} + .help-card-link:hover svg{opacity:1;} .help-card-link svg{flex:0 0 auto;opacity:.8;} .main{flex:1;display:flex;flex-direction:column;overflow:hidden;min-width:0;min-height:0;background:var(--main-bg);} .topbar{padding:12px 20px;border-bottom:1px solid var(--border);background:var(--topbar-bg);backdrop-filter:blur(12px);display:flex;align-items:center;justify-content:space-between;flex-shrink:0;position:relative;z-index:10;} diff --git a/tests/test_issue3227_help_tab.py b/tests/test_issue3227_help_tab.py index 5f4a6039..9e2f0e41 100644 --- a/tests/test_issue3227_help_tab.py +++ b/tests/test_issue3227_help_tab.py @@ -4,6 +4,7 @@ ROOT = Path(__file__).resolve().parents[1] INDEX_HTML = (ROOT / "static" / "index.html").read_text(encoding="utf-8") PANELS_JS = (ROOT / "static" / "panels.js").read_text(encoding="utf-8") I18N_JS = (ROOT / "static" / "i18n.js").read_text(encoding="utf-8") +STYLE_CSS = (ROOT / "static" / "style.css").read_text(encoding="utf-8") LOCALE_COUNT = 13 # en, it, ja, ru, es, de, zh, zh-TW, pt, ko, fr, tr, pl @@ -46,3 +47,27 @@ def test_i18n_help_keys_present_in_all_locales(): assert I18N_JS.count("settings_help_issue_label") == LOCALE_COUNT assert I18N_JS.count("settings_help_docs_link") == LOCALE_COUNT assert I18N_JS.count("settings_help_issue_link") == LOCALE_COUNT + + +def test_help_card_link_hover_is_contrast_safe(): + """The hover fill is var(--accent); the text color must NOT be var(--accent-text). + + In most themes --accent-text equals (or nearly equals) --accent — it is an + accent-on-background link color, not a contrasting color for text sitting ON + an accent fill. Using it on the accent-filled hover state produced same-color + text on the fill (invisible) across nearly every theme (slate/gold/ares/mono/ + sisyphus/catppuccin dark all had accent == accent-text). The fix uses + var(--bg) — the page background — which --accent is designed to contrast + against, so the text stays readable in every theme. + """ + import re + m = re.search(r"\.help-card-link:hover\s*\{([^}]*)\}", STYLE_CSS) + assert m, "expected a .help-card-link:hover rule in style.css" + rule = m.group(1) + assert "background:var(--accent)" in rule.replace(" ", "") + # The bug: accent-text on accent fill. Must not reappear. + assert "--accent-text" not in rule, ( + "hover text uses var(--accent-text) on an accent fill — invisible in most " + "themes; use var(--bg) for theme-agnostic contrast" + ) + assert "color:var(--bg)" in rule.replace(" ", "") diff --git a/tests/test_issue3762_importable_rows_schema_guard.py b/tests/test_issue3762_importable_rows_schema_guard.py new file mode 100644 index 00000000..61b40a3b --- /dev/null +++ b/tests/test_issue3762_importable_rows_schema_guard.py @@ -0,0 +1,132 @@ +"""Regression tests for #3762 — minimal state.db schemas must not hide all sessions. + +`read_importable_agent_session_rows()` built its projection SQL with +unconditional references to the `messages` table and `messages.timestamp` +(`MAX(m.timestamp)`, `COUNT(m.id)`, the `LEFT JOIN messages`, and the +`ORDER BY MAX(m.timestamp)`). On a `state.db` that has no `messages` table — or +a `messages` table without a `timestamp` column — that query raised +`sqlite3.OperationalError`, which `get_cli_sessions()` catches and turns into an +empty list, so EVERY imported/CLI/agent session silently vanished from the +sidebar. These tests build minimal schemas directly and assert the rows still +come back instead of the query exploding. +""" +import sqlite3 + +import pytest + +from api.agent_sessions import read_importable_agent_session_rows + + +def _make_db(path, *, with_messages_table, with_timestamp, with_session_id=True, + sessions=None, messages=None): + """Build a minimal state.db. + + sessions: list of (id, title, model, message_count, started_at, source, session_source) + messages: list of (session_id, role[, timestamp]) — inserted only when the + messages table is created with the matching columns. + """ + conn = sqlite3.connect(str(path)) + conn.execute( + """ + CREATE TABLE sessions ( + id TEXT PRIMARY KEY, title TEXT, model TEXT, message_count INTEGER, + started_at REAL, source TEXT, session_source TEXT + ) + """ + ) + for r in (sessions or []): + conn.execute( + "INSERT INTO sessions (id, title, model, message_count, started_at, source, session_source) " + "VALUES (?,?,?,?,?,?,?)", + r, + ) + if with_messages_table: + cols = ["id INTEGER PRIMARY KEY"] + if with_session_id: + cols.append("session_id TEXT") + cols.append("role TEXT") + if with_timestamp: + cols.append("timestamp REAL") + conn.execute(f"CREATE TABLE messages ({', '.join(cols)})") + for m in (messages or []): + if with_session_id and with_timestamp: + conn.execute("INSERT INTO messages (session_id, role, timestamp) VALUES (?,?,?)", m) + elif with_session_id: + conn.execute("INSERT INTO messages (session_id, role) VALUES (?,?)", m[:2]) + conn.commit() + conn.close() + + +def test_full_schema_returns_rows_baseline(tmp_path): + db = tmp_path / "state.db" + _make_db( + db, with_messages_table=True, with_timestamp=True, + sessions=[("cli-1", "Hello", "gpt", 3, 1000.0, "cli", "cli")], + messages=[("cli-1", "user", 1001.0), ("cli-1", "assistant", 1002.0)], + ) + out = read_importable_agent_session_rows(db, exclude_sources=None) + assert "cli-1" in {r["id"] for r in out} + + +def test_messages_table_without_timestamp_does_not_hide_sessions(tmp_path): + """The exact #3762 repro: messages table exists but has no timestamp column. + + On master this raised sqlite3.OperationalError on MAX(m.timestamp) and the + whole list collapsed to empty. The COUNT join still works, so the session + must surface with its real message count and a NULL last_activity. + """ + db = tmp_path / "state.db" + _make_db( + db, with_messages_table=True, with_timestamp=False, + sessions=[("cli-1", "Hello", "gpt", 2, 1000.0, "cli", "cli")], + messages=[("cli-1", "user"), ("cli-1", "assistant")], + ) + out = read_importable_agent_session_rows(db, exclude_sources=None) + assert {r["id"] for r in out} == {"cli-1"} + assert out[0].get("last_activity") is None + + +def test_no_messages_table_does_not_hide_sessions(tmp_path): + """state.db with a sessions table but NO messages table at all. + + With no join possible we fall back to the denormalized s.message_count, so + non-empty sessions still surface. + """ + db = tmp_path / "state.db" + _make_db( + db, with_messages_table=False, with_timestamp=False, + sessions=[ + ("cli-1", "First", "gpt", 2, 1000.0, "cli", "cli"), + ("cli-2", "Second", "gpt", 7, 2000.0, "cli", "cli"), + ], + ) + out = read_importable_agent_session_rows(db, exclude_sources=None) + assert {r["id"] for r in out} == {"cli-1", "cli-2"} + + +def test_messages_table_without_session_id_does_not_hide_sessions(tmp_path): + """A messages table that can't be joined (no session_id) degrades to s.message_count.""" + db = tmp_path / "state.db" + _make_db( + db, with_messages_table=True, with_timestamp=True, with_session_id=False, + sessions=[("cli-1", "Hello", "gpt", 4, 1000.0, "cli", "cli")], + ) + out = read_importable_agent_session_rows(db, exclude_sources=None) + assert {r["id"] for r in out} == {"cli-1"} + + +@pytest.mark.parametrize("with_timestamp", [True, False]) +def test_limit_path_also_guarded(tmp_path, with_timestamp): + """The bounded (limit set) candidate-CTE branch must be column-aware too.""" + db = tmp_path / "state.db" + sessions = [(f"cli-{i}", f"T{i}", "gpt", 2, 1000.0 + i, "cli", "cli") for i in range(5)] + messages = [] + for i in range(5): + if with_timestamp: + messages += [(f"cli-{i}", "user", 1000.0 + i), (f"cli-{i}", "assistant", 1000.5 + i)] + else: + messages += [(f"cli-{i}", "user"), (f"cli-{i}", "assistant")] + _make_db(db, with_messages_table=True, with_timestamp=with_timestamp, + sessions=sessions, messages=messages) + out = read_importable_agent_session_rows(db, limit=3, exclude_sources=None) + assert len(out) == 3 diff --git a/tests/test_issue3802_delete_session_journals.py b/tests/test_issue3802_delete_session_journals.py new file mode 100644 index 00000000..6e26cd81 --- /dev/null +++ b/tests/test_issue3802_delete_session_journals.py @@ -0,0 +1,113 @@ +"""Regression tests for #3802 — deleting a session must remove its journal files. + +Deleting a conversation from the WebUI removed the session JSON + state.db rows +but left the turn journal (`_turn_journal/{sid}*.jsonl`, user messages in +plaintext) and the run journal (`_run_journal/{sid}/`, full request/response +payloads) on disk, so the conversation stayed recoverable. These tests pin the +two cleanup helpers: every shard/dir for the deleted session is removed, and an +unrelated session's journals are left untouched. +""" +import os + +from api.run_journal import RunJournalWriter, delete_run_journal, read_run_events +from api.turn_journal import ( + append_turn_journal_event, + delete_turn_journal, + read_turn_journal, +) + + +def _submit(sid, content, session_dir): + return append_turn_journal_event( + sid, + {"event": "submitted", "turn_id": "t1", "stream_id": "s1", "role": "user", "content": content}, + session_dir=session_dir, + ) + + +def test_delete_turn_journal_removes_pid_shard_and_legacy(tmp_path): + # pid-scoped shard (written by append) + a legacy single-file shard. + _submit("sid-del", "secret message", session_dir=tmp_path) + journal_dir = tmp_path / "_turn_journal" + legacy = journal_dir / "sid-del.jsonl" + legacy.write_text('{"event":"submitted","session_id":"sid-del","content":"old"}\n', encoding="utf-8") + + pid_shard = journal_dir / f"sid-del~{os.getpid()}.jsonl" + assert pid_shard.exists() + assert legacy.exists() + + removed = delete_turn_journal("sid-del", session_dir=tmp_path) + + assert removed == 2 + assert not pid_shard.exists() + assert not legacy.exists() + # read_turn_journal now finds nothing for the deleted session. + assert read_turn_journal("sid-del", session_dir=tmp_path)["events"] == [] + + +def test_delete_turn_journal_leaves_other_sessions_intact(tmp_path): + _submit("sid-keep", "keep me", session_dir=tmp_path) + _submit("sid-del", "delete me", session_dir=tmp_path) + + delete_turn_journal("sid-del", session_dir=tmp_path) + + keep_shard = tmp_path / "_turn_journal" / f"sid-keep~{os.getpid()}.jsonl" + del_shard = tmp_path / "_turn_journal" / f"sid-del~{os.getpid()}.jsonl" + assert keep_shard.exists(), "unrelated session's journal must survive" + assert not del_shard.exists() + assert read_turn_journal("sid-keep", session_dir=tmp_path)["events"] + + +def test_delete_turn_journal_noop_on_missing_or_invalid(tmp_path): + # Missing directory: no error, zero removed. + assert delete_turn_journal("nope", session_dir=tmp_path) == 0 + # Invalid id: no error, zero removed (and never touches the filesystem). + assert delete_turn_journal("../etc/passwd", session_dir=tmp_path) == 0 + assert delete_turn_journal("", session_dir=tmp_path) == 0 + + +def test_delete_run_journal_removes_session_directory(tmp_path): + writer = RunJournalWriter("sid-del", "run-1", session_dir=tmp_path) + writer.append_sse_event("token", {"text": "hello"}) + writer.append_sse_event("done", {"session": {"session_id": "sid-del"}}) + run_dir = tmp_path / "_run_journal" / "sid-del" + assert run_dir.exists() + + assert delete_run_journal("sid-del", session_dir=tmp_path) is True + assert not run_dir.exists() + # No events recoverable after delete. + assert read_run_events("sid-del", "run-1", session_dir=tmp_path)["events"] == [] + + +def test_delete_run_journal_leaves_other_sessions_intact(tmp_path): + RunJournalWriter("sid-keep", "run-k", session_dir=tmp_path).append_sse_event("token", {"text": "k"}) + RunJournalWriter("sid-del", "run-d", session_dir=tmp_path).append_sse_event("token", {"text": "d"}) + + delete_run_journal("sid-del", session_dir=tmp_path) + + assert (tmp_path / "_run_journal" / "sid-keep").exists() + assert not (tmp_path / "_run_journal" / "sid-del").exists() + + +def test_delete_run_journal_noop_on_missing_or_invalid(tmp_path): + assert delete_run_journal("nope", session_dir=tmp_path) is False + assert delete_run_journal("../escape", session_dir=tmp_path) is False + assert delete_run_journal("", session_dir=tmp_path) is False + + +def test_delete_journals_reject_dot_traversal_ids(tmp_path): + """A bare '.'/'..' passes the dot-permitting id regex but must NOT resolve to + the journal root/parent and delete the wrong directory (no '/' to catch it). + """ + # Seed a real run + turn journal so we'd notice an over-broad delete. + writer = RunJournalWriter("keep", "run-1", session_dir=tmp_path) + writer.append_sse_event("token", {"text": "hello"}) + _submit("keep", "hi", tmp_path) + run_dir = tmp_path / "_run_journal" / "keep" + assert run_dir.exists() + for bad in (".", ".."): + assert delete_run_journal(bad, session_dir=tmp_path) is False + assert delete_turn_journal(bad, session_dir=tmp_path) == 0 + # The legitimate journals must still be present. + assert run_dir.exists() + assert read_turn_journal("keep", session_dir=tmp_path) diff --git a/tests/test_sprint20.py b/tests/test_sprint20.py index b6053570..5a44c303 100644 --- a/tests/test_sprint20.py +++ b/tests/test_sprint20.py @@ -338,12 +338,54 @@ def test_boot_js_media_recorder_fallback_posts_to_transcribe_api(): assert 'fetch(' in js +def test_boot_js_prefers_server_side_stt_by_default(): + """Default mic capture should prefer server STT only after the server confirms support.""" + js, _ = get_text("/static/boot.js") + assert "const _micForceMediaRecorderStored=localStorage.getItem(_micForceMediaRecorderKey);" in js + assert "let _serverSttAvailable=false" in js + assert "_micForceMediaRecorderStored===null?(_serverSttAvailable&&_canRecordAudio):" in js + assert "fetch('api/transcribe/capability'" in js + + +def test_boot_js_no_server_stt_first_click_uses_browser_speech_recognition(): + """No-server-STT installs must not lose the first mic click before fallback runs.""" + js, _ = get_text("/static/boot.js") + default_idx = js.find("_micForceMediaRecorderStored===null?") + assert default_idx != -1 + default_expr = js[default_idx:default_idx + 140] + assert "_serverSttAvailable" in default_expr + assert "_canRecordAudio" in default_expr + assert "_canRecordAudio:" not in default_expr + + +def test_boot_js_falls_back_to_browser_stt_when_server_transcribe_unavailable(): + """If server-side STT is unavailable, browser SpeechRecognition should be re-enabled.""" + js, _ = get_text("/static/boot.js") + assert "function _isServerSttUnavailable(err)" in js + assert "function _allowBrowserSttFallback()" in js + assert "localStorage.setItem(_micForceMediaRecorderKey,'0')" in js + assert "_forceMediaRecorder=false" in js + assert "recognition=_ensureSpeechRecognition()" in js + + +def test_boot_js_keeps_explicit_server_stt_preference_on_transcribe_failure(): + """An explicit mic_force_mediarecorder='1' preference must not be silently overwritten.""" + js, _ = get_text("/static/boot.js") + assert "localStorage.getItem(_micForceMediaRecorderKey)!=='1'" in js + + def test_routes_define_transcribe_endpoint(): """Server routes must expose /api/transcribe for MediaRecorder fallback uploads.""" routes = pathlib.Path(__file__).parent.parent.joinpath("api/routes.py").read_text(encoding="utf-8") assert '"/api/transcribe"' in routes +def test_routes_define_transcribe_capability_endpoint(): + """Server routes must expose a cheap STT capability probe before defaulting to MediaRecorder.""" + routes = pathlib.Path(__file__).parent.parent.joinpath("api/routes.py").read_text(encoding="utf-8") + assert '"/api/transcribe/capability"' in routes + + def test_boot_js_shows_mic_button_when_any_voice_path_is_supported(): """boot.js must reveal btnMic when speech recognition or recorder fallback is available.""" js, _ = get_text("/static/boot.js") @@ -384,7 +426,9 @@ def test_boot_js_prefix_captured_on_start(): def test_boot_js_onresult_prepends_prefix(): """onresult must include _prefix when writing to textarea (append, not replace).""" js, _ = get_text("/static/boot.js") - onresult_idx = js.find("recognition.onresult") + onresult_idx = js.find("sr.onresult") + if onresult_idx == -1: + onresult_idx = js.find("recognition.onresult") onresult_end = js.find("};", onresult_idx) onresult_body = js[onresult_idx:onresult_end] # ta.value must be set to _prefix + something, not just the transcript alone @@ -394,7 +438,9 @@ def test_boot_js_onresult_prepends_prefix(): def test_boot_js_onend_commits_with_prefix(): """onend must commit _prefix + _finalText so appended text survives after recognition ends.""" js, _ = get_text("/static/boot.js") - onend_idx = js.find("recognition.onend") + onend_idx = js.find("sr.onend") + if onend_idx == -1: + onend_idx = js.find("recognition.onend") onend_end = js.find("};", onend_idx) onend_body = js[onend_idx:onend_end] assert "_prefix" in onend_body @@ -413,7 +459,9 @@ def test_boot_js_prefix_reset_on_stop(): def test_boot_js_auto_space_between_prefix_and_transcript(): """onend must insert a space between existing text and new transcript when needed.""" js, _ = get_text("/static/boot.js") - onend_idx = js.find("recognition.onend") + onend_idx = js.find("sr.onend") + if onend_idx == -1: + onend_idx = js.find("recognition.onend") onend_end = js.find("};", onend_idx) onend_body = js[onend_idx:onend_end] # Should handle spacing — look for trimStart or endsWith(' ') check diff --git a/tests/test_voice_transcribe_endpoint.py b/tests/test_voice_transcribe_endpoint.py index 87ea19a4..1374ebda 100644 --- a/tests/test_voice_transcribe_endpoint.py +++ b/tests/test_voice_transcribe_endpoint.py @@ -3,7 +3,12 @@ import json import sys import types -from api.upload import handle_transcribe +import api.upload as upload +from api.upload import ( + _stt_provider_capability_from_module, + handle_transcribe, + handle_transcribe_capability, +) def _multipart_body(fields=None, files=None, boundary=b"voiceboundary"): @@ -49,6 +54,13 @@ class _FakeHandler: return json.loads(self.wfile.getvalue().decode("utf-8")) +def _install_fake_transcription_tools(monkeypatch, fake_mod): + monkeypatch.setitem(sys.modules, "tools.transcription_tools", fake_mod) + tools_pkg = sys.modules.get("tools") + if tools_pkg is not None: + monkeypatch.setattr(tools_pkg, "transcription_tools", fake_mod, raising=False) + + def test_handle_transcribe_requires_file_field(): body, content_type = _multipart_body(fields={"note": "missing file"}) handler = _FakeHandler(body, content_type) @@ -60,7 +72,7 @@ def test_handle_transcribe_requires_file_field(): def test_handle_transcribe_returns_transcript(monkeypatch): fake_mod = types.ModuleType("tools.transcription_tools") fake_mod.transcribe_audio = lambda path: {"success": True, "transcript": "hello from audio"} - monkeypatch.setitem(sys.modules, "tools.transcription_tools", fake_mod) + _install_fake_transcription_tools(monkeypatch, fake_mod) body, content_type = _multipart_body( files={"file": ("voice.webm", b"RIFFfakeaudio", "audio/webm")} @@ -75,7 +87,7 @@ def test_handle_transcribe_returns_transcript(monkeypatch): def test_handle_transcribe_surfaces_provider_error(monkeypatch): fake_mod = types.ModuleType("tools.transcription_tools") fake_mod.transcribe_audio = lambda path: {"success": False, "error": "STT not configured"} - monkeypatch.setitem(sys.modules, "tools.transcription_tools", fake_mod) + _install_fake_transcription_tools(monkeypatch, fake_mod) body, content_type = _multipart_body( files={"file": ("voice.webm", b"RIFFfakeaudio", "audio/webm")} @@ -85,3 +97,62 @@ def test_handle_transcribe_surfaces_provider_error(monkeypatch): assert handler.status == 503 assert handler.payload()["error"] == "STT not configured" + + +def test_handle_transcribe_capability_reports_unavailable_without_provider(monkeypatch): + monkeypatch.setattr(upload, "_stt_provider_capability", lambda: (False, "none")) + + handler = _FakeHandler(b"", "") + handle_transcribe_capability(handler) + + assert handler.status == 200 + assert handler.payload()["ok"] is True + assert handler.payload()["available"] is False + +def test_handle_transcribe_capability_reports_available_provider(monkeypatch): + monkeypatch.setattr(upload, "_stt_provider_capability", lambda: (True, "openai")) + + handler = _FakeHandler(b"", "") + handle_transcribe_capability(handler) + + assert handler.status == 200 + assert handler.payload()["ok"] is True + assert handler.payload()["available"] is True + assert handler.payload()["provider"] == "openai" + +def test_handle_transcribe_capability_requires_ffmpeg_for_local_command_webm(monkeypatch): + fake_mod = types.ModuleType("tools.transcription_tools") + fake_mod.__dict__.update( + { + "_load_stt_config": lambda: {"provider": "local_command"}, + "is_stt_enabled": lambda cfg=None: True, + "_HAS_FASTER_WHISPER": False, + "_HAS_OPENAI": False, + "_HAS_MISTRAL": False, + "_has_local_command": lambda: True, + "_find_ffmpeg_binary": lambda: None, + } + ) + available, provider = _stt_provider_capability_from_module(fake_mod) + + assert available is False + assert provider == "local_command" + + +def test_handle_transcribe_capability_reports_actual_local_command_fallback(monkeypatch): + fake_mod = types.ModuleType("tools.transcription_tools") + fake_mod.__dict__.update( + { + "_load_stt_config": lambda: {"provider": "local"}, + "is_stt_enabled": lambda cfg=None: True, + "_HAS_FASTER_WHISPER": False, + "_HAS_OPENAI": False, + "_HAS_MISTRAL": False, + "_has_local_command": lambda: True, + "_find_ffmpeg_binary": lambda: "/usr/bin/ffmpeg", + } + ) + available, provider = _stt_provider_capability_from_module(fake_mod) + + assert available is True + assert provider == "local_command"