From 764d5b8601b2cdde5e56d5e81b3f97a0687fce60 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Wed, 24 Jun 2026 09:04:21 -0400 Subject: [PATCH] fix(#4749): show steer failure reason and offer retry/dismiss recovery --- static/commands.js | 42 ++- static/i18n.js | 10 + static/style.css | 5 + ...est_issue4749_steer_reason_and_recovery.py | 275 ++++++++++++++++++ 4 files changed, 327 insertions(+), 5 deletions(-) create mode 100644 tests/test_issue4749_steer_reason_and_recovery.py diff --git a/static/commands.js b/static/commands.js index 63bf57c2..b0d1aff4 100644 --- a/static/commands.js +++ b/static/commands.js @@ -1230,6 +1230,12 @@ async function cmdSteer(args){ * @returns {Promise} true when the steer was delivered, false when the * draft was restored and the active stream was left untouched. */ +function _steerFailureMessageKey(fallback) { + const key = 'steer_fail_' + (fallback || 'unknown'); + return (typeof LOCALES !== 'undefined' && LOCALES.en && LOCALES.en[key]) + ? key : 'steer_fail_unknown'; +} + function _showSteerIndicator(text){ const inner=document.getElementById('msgInner'); if(!inner) return; @@ -1250,6 +1256,34 @@ function _showSteerIndicator(text){ if(typeof scrollToBottom==='function') scrollToBottom(); } +function _showSteerRecovery(msg, explicitSteer, fallback) { + const inner = document.getElementById('msgInner'); + if (!inner) return; + const old = inner.querySelector('.steer-recovery'); + if (old) old.remove(); + const el = document.createElement('div'); + el.className = 'steer-recovery'; + const label = document.createElement('span'); + label.className = 'steer-recovery-label'; + label.textContent = t(_steerFailureMessageKey(fallback)); + el.appendChild(label); + const retryBtn = document.createElement('button'); + retryBtn.className = 'steer-recovery-retry'; + retryBtn.textContent = t('steer_recovery_retry'); + retryBtn.addEventListener('click', () => { + el.remove(); + _trySteer(msg, explicitSteer); + }); + el.appendChild(retryBtn); + const dismissBtn = document.createElement('button'); + dismissBtn.className = 'steer-recovery-dismiss'; + dismissBtn.textContent = t('steer_recovery_dismiss'); + dismissBtn.addEventListener('click', () => el.remove()); + el.appendChild(dismissBtn); + inner.appendChild(el); + if (typeof scrollToBottom === 'function') scrollToBottom(); +} + async function _trySteer(msg, explicitSteer){ let result=null; try{ @@ -1279,11 +1313,9 @@ async function _trySteer(msg, explicitSteer){ if(typeof autoResize==='function')autoResize(); } if(typeof renderTray==='function')renderTray(); - if(explicitSteer){ - showToast(t('cmd_steer_fallback'),3500); - } else { - showToast(t('busy_steer_fallback'),3500); - } + const failbackCode = result && result.fallback; + showToast(t(_steerFailureMessageKey(failbackCode)), 3500); + _showSteerRecovery(msg, explicitSteer, failbackCode); return false; } diff --git a/static/i18n.js b/static/i18n.js index cc450645..6da14a0d 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -298,6 +298,16 @@ const LOCALES = { cmd_steer_delivered: 'Steer delivered — agent will see it on its next tool result', steer_leftover_queued: 'Steer queued for next turn', busy_steer_fallback: 'Steer unavailable — draft restored', + steer_fail_no_cached_agent: 'No agent cached for this session — try refreshing', + steer_fail_agent_lacks_steer: 'This agent version does not support steer', + steer_fail_session_not_found: 'Session not found — it may have expired', + steer_fail_not_running: 'Agent is not currently running', + steer_fail_stream_dead: 'The agent stream has ended', + steer_fail_steer_error: 'Steer delivery failed — the agent may have finished', + steer_fail_network_error: 'Network error — check your connection', + steer_fail_unknown: 'Steer unavailable', + steer_recovery_retry: 'Retry', + steer_recovery_dismiss: 'Dismiss', busy_interrupt_confirm: 'Interrupted — sending new message', settings_label_busy_input_mode: 'Busy input mode', settings_desc_busy_input_mode: 'Controls what happens when you send a message while the agent is running. Queue waits; Interrupt cancels and starts fresh; Steer injects a correction mid-turn without interrupting. If Steer is unavailable, the draft is restored so you can choose the next action.', diff --git a/static/style.css b/static/style.css index dbd6e194..2c42b228 100644 --- a/static/style.css +++ b/static/style.css @@ -1735,6 +1735,11 @@ .steer-indicator{display:flex;align-items:baseline;gap:8px;padding:10px 0;opacity:.65;font-style:italic;color:var(--accent-text);} .steer-indicator .steer-body{white-space:pre-wrap;word-break:break-word;} .steer-badge{display:inline-block;font-size:10px;font-weight:600;letter-spacing:.04em;text-transform:uppercase;color:var(--accent-text);background:var(--accent-bg);border:1px solid var(--accent-bg-strong);border-radius:4px;padding:1px 6px;vertical-align:middle;line-height:1.6;font-style:normal;flex-shrink:0;} + /* Steer recovery bar: dismissible after failure, same lifecycle as steer-indicator */ + .steer-recovery{display:flex;align-items:center;gap:0.5rem;padding:0.5rem 0.75rem;margin:0.25rem 0;border-radius:var(--border-radius,0.5rem);background:var(--color-surface-secondary,#f5f5f5);font-size:0.85rem} + .steer-recovery-label{flex:1;min-width:0} + .steer-recovery button{padding:0.25rem 0.75rem;border-radius:var(--border-radius,0.375rem);border:1px solid var(--color-border,#ddd);background:var(--color-surface,#fff);cursor:pointer;font-size:0.8rem} + .steer-recovery button:hover{background:var(--color-surface-hover,#eee)} .msg-role{font-size:12px;font-weight:500;letter-spacing:.01em;margin-bottom:8px;display:flex;align-items:center;gap:8px;} .msg-role.user{color:var(--accent);} .msg-role.assistant{color:var(--accent-text);opacity:.6;} diff --git a/tests/test_issue4749_steer_reason_and_recovery.py b/tests/test_issue4749_steer_reason_and_recovery.py new file mode 100644 index 00000000..7a946b58 --- /dev/null +++ b/tests/test_issue4749_steer_reason_and_recovery.py @@ -0,0 +1,275 @@ +"""Tests for issue #4749: steer failure reason display and recovery bar. + +Covers: + 1. i18n contract — all expected keys exist in the en locale block + 2. Reason map contract — _steerFailureMessageKey returns correct keys + 3. Backend parity — frontend reason map covers all backend fallback codes + 4. Recovery DOM — _showSteerRecovery creates correct structure; dismiss removes it +""" +import re +import subprocess +import sys +import textwrap +from pathlib import Path + +REPO = Path(__file__).parent.parent +I18N_JS = REPO / "static" / "i18n.js" +COMMANDS_JS = REPO / "static" / "commands.js" +STREAMING_PY = REPO / "api" / "streaming.py" + +EXPECTED_I18N_KEYS = [ + "steer_fail_no_cached_agent", + "steer_fail_agent_lacks_steer", + "steer_fail_session_not_found", + "steer_fail_not_running", + "steer_fail_stream_dead", + "steer_fail_steer_error", + "steer_fail_network_error", + "steer_fail_unknown", + "steer_recovery_retry", + "steer_recovery_dismiss", +] + +BACKEND_CODES = { + "no_cached_agent", + "agent_lacks_steer", + "session_not_found", + "not_running", + "stream_dead", + "steer_error", +} + +FRONTEND_NETWORK_CODE = "network_error" + + +def test_i18n_steer_failure_keys_exist(): + """All 10 expected i18n keys are present in the en locale block.""" + text = I18N_JS.read_text(encoding="utf-8") + # Find the en locale block: from 'en:' or 'en =' up to the next top-level locale key + en_match = re.search(r"\ben\s*:\s*\{", text) + assert en_match, "Could not locate 'en:' locale block in i18n.js" + # Grab a large slice starting at the en block; the block is thousands of lines + en_block = text[en_match.start():] + for key in EXPECTED_I18N_KEYS: + pattern = rf"^\s+{re.escape(key)}\s*:" + assert re.search(pattern, en_block, re.MULTILINE), ( + f"Expected i18n key '{key}' not found in the en locale block" + ) + + +def test_reason_map_contract(): + """_steerFailureMessageKey maps each known code to the correct key.""" + node = _find_node() + script = textwrap.dedent(""" + const LOCALES = { en: { + steer_fail_no_cached_agent: 'x', + steer_fail_agent_lacks_steer: 'x', + steer_fail_session_not_found: 'x', + steer_fail_not_running: 'x', + steer_fail_stream_dead: 'x', + steer_fail_steer_error: 'x', + steer_fail_network_error: 'x', + steer_fail_unknown: 'x', + }}; + + function _steerFailureMessageKey(fallback) { + const key = 'steer_fail_' + (fallback || 'unknown'); + return (typeof LOCALES !== 'undefined' && LOCALES.en && LOCALES.en[key]) + ? key : 'steer_fail_unknown'; + } + + const codes = [ + 'no_cached_agent', 'agent_lacks_steer', 'session_not_found', + 'not_running', 'stream_dead', 'steer_error', 'network_error', + ]; + let ok = true; + for (const c of codes) { + const got = _steerFailureMessageKey(c); + const want = 'steer_fail_' + c; + if (got !== want) { + console.error('FAIL code=' + c + ' got=' + got + ' want=' + want); + ok = false; + } + } + // unknown code + const unk = _steerFailureMessageKey('something_unknown_xyz'); + if (unk !== 'steer_fail_unknown') { + console.error('FAIL unknown: got=' + unk); + ok = false; + } + // null / undefined + const n = _steerFailureMessageKey(null); + if (n !== 'steer_fail_unknown') { + console.error('FAIL null: got=' + n); + ok = false; + } + const u = _steerFailureMessageKey(undefined); + if (u !== 'steer_fail_unknown') { + console.error('FAIL undefined: got=' + u); + ok = false; + } + process.exit(ok ? 0 : 1); + """) + result = subprocess.run([node, "-e", script], capture_output=True, text=True) + assert result.returncode == 0, ( + f"_steerFailureMessageKey contract failed:\n{result.stdout}\n{result.stderr}" + ) + + +def test_backend_parity(): + """Frontend reason map covers all backend fallback codes from _handle_chat_steer.""" + streaming_text = STREAMING_PY.read_text(encoding="utf-8") + # Extract fallback codes from _handle_chat_steer function only + fn_match = re.search(r"def _handle_chat_steer\b.*?(?=\ndef |\Z)", streaming_text, re.DOTALL) + assert fn_match, "Could not find _handle_chat_steer in streaming.py" + fn_body = fn_match.group(0) + # Exclude placeholder strings like "" that appear in docstrings + found_codes = set( + c for c in re.findall(r'"fallback":\s*"([^"<>]+)"', fn_body) + if c # non-empty after filtering + ) + assert found_codes, "No fallback codes found in _handle_chat_steer" + assert found_codes == BACKEND_CODES, ( + f"Backend fallback codes mismatch.\n" + f" Found: {sorted(found_codes)}\n" + f" Expected: {sorted(BACKEND_CODES)}" + ) + # Also confirm frontend adds network_error + commands_text = COMMANDS_JS.read_text(encoding="utf-8") + assert FRONTEND_NETWORK_CODE in commands_text, ( + "network_error not found in commands.js" + ) + # Confirm the dynamic key builder and recovery bar are present in commands.js + assert "_steerFailureMessageKey" in commands_text, ( + "_steerFailureMessageKey not found in commands.js" + ) + assert "_showSteerRecovery" in commands_text, ( + "_showSteerRecovery not found in commands.js" + ) + # The builder uses the 'steer_fail_' prefix pattern + assert "steer_fail_" in commands_text, ( + "'steer_fail_' prefix not found in commands.js" + ) + + +def test_recovery_dom_structure(): + """_showSteerRecovery creates a div with label, retry, dismiss; dismiss removes it.""" + node = _find_node() + script = textwrap.dedent(""" + // Minimal DOM stubs + const elements = {}; + function createElement(tag) { + const el = { + tag, className: '', textContent: '', children: [], + listeners: {}, + appendChild(c) { this.children.push(c); }, + addEventListener(ev, fn) { this.listeners[ev] = fn; }, + remove() { el._removed = true; }, + querySelector(sel) { + // only handle .steer-recovery for old-removal check + return null; + }, + }; + elements[tag + '_' + Math.random()] = el; + return el; + } + const inner = createElement('div'); + inner.querySelector = (sel) => null; // no existing recovery bar + const document = { + getElementById(id) { return id === 'msgInner' ? inner : null; }, + createElement, + }; + function t(key) { return key; } + function _steerFailureMessageKey(fallback) { + const key = 'steer_fail_' + (fallback || 'unknown'); + const LOCALES = { en: { + steer_fail_not_running: 'Agent is not currently running', + steer_fail_unknown: 'Steer unavailable', + steer_recovery_retry: 'Retry', + steer_recovery_dismiss: 'Dismiss', + }}; + return (LOCALES.en && LOCALES.en[key]) ? key : 'steer_fail_unknown'; + } + function _trySteer() {} // stub for retry handler + + function _showSteerRecovery(msg, explicitSteer, fallback) { + const inner = document.getElementById('msgInner'); + if (!inner) return; + const old = inner.querySelector('.steer-recovery'); + if (old) old.remove(); + const el = document.createElement('div'); + el.className = 'steer-recovery'; + const label = document.createElement('span'); + label.className = 'steer-recovery-label'; + label.textContent = t(_steerFailureMessageKey(fallback)); + el.appendChild(label); + const retryBtn = document.createElement('button'); + retryBtn.className = 'steer-recovery-retry'; + retryBtn.textContent = t('steer_recovery_retry'); + retryBtn.addEventListener('click', () => { + el.remove(); + _trySteer(msg, explicitSteer); + }); + el.appendChild(retryBtn); + const dismissBtn = document.createElement('button'); + dismissBtn.className = 'steer-recovery-dismiss'; + dismissBtn.textContent = t('steer_recovery_dismiss'); + dismissBtn.addEventListener('click', () => el.remove()); + el.appendChild(dismissBtn); + inner.appendChild(el); + } + + _showSteerRecovery('hello', false, 'not_running'); + + const bar = inner.children[inner.children.length - 1]; + let ok = true; + + if (bar.className !== 'steer-recovery') { + console.error('FAIL: bar className=' + bar.className); + ok = false; + } + const [lbl, retry, dismiss] = bar.children; + if (!lbl || lbl.className !== 'steer-recovery-label') { + console.error('FAIL: label missing or wrong class'); + ok = false; + } + if (!retry || retry.className !== 'steer-recovery-retry') { + console.error('FAIL: retry btn missing or wrong class'); + ok = false; + } + if (!dismiss || dismiss.className !== 'steer-recovery-dismiss') { + console.error('FAIL: dismiss btn missing or wrong class'); + ok = false; + } + // Simulate dismiss click + dismiss.listeners['click'](); + if (!bar._removed) { + console.error('FAIL: bar not removed after dismiss'); + ok = false; + } + process.exit(ok ? 0 : 1); + """) + result = subprocess.run([node, "-e", script], capture_output=True, text=True) + assert result.returncode == 0, ( + f"Recovery DOM structure test failed:\n{result.stdout}\n{result.stderr}" + ) + + +def _find_node(): + """Return path to node.exe, skipping wrapper scripts.""" + import shutil + candidates = ["node", "node.exe"] + for c in candidates: + path = shutil.which(c) + if path: + # Verify it's actually node, not a wrapper + try: + r = subprocess.run([path, "--version"], capture_output=True, text=True, timeout=5) + if r.returncode == 0 and r.stdout.strip().startswith("v"): + return path + except Exception: + continue + pytest_skip = getattr(sys.modules.get("pytest"), "skip", None) + if pytest_skip: + pytest_skip("node.js not found — skipping node-executed tests") + raise RuntimeError("node.js not found")