From ccba2f5c018917e4cecae0c1aee78a85509274fe Mon Sep 17 00:00:00 2001 From: Frank Song Date: Wed, 15 Apr 2026 13:10:50 +0800 Subject: [PATCH 1/2] feat: harden clarify dialog flow and refresh recovery --- api/clarify.py | 128 +++++++++++++++ api/routes.py | 87 ++++++++++ api/streaming.py | 88 ++++++++++ static/i18n.js | 32 +++- static/index.html | 15 ++ static/messages.js | 297 +++++++++++++++++++++++++++++++++- static/sessions.js | 4 + static/style.css | 32 ++++ static/ui.js | 37 ++++- tests/test_clarify_unblock.py | 165 +++++++++++++++++++ tests/test_sprint30.py | 190 ++++++++++++++++++++++ 11 files changed, 1066 insertions(+), 9 deletions(-) create mode 100644 api/clarify.py create mode 100644 tests/test_clarify_unblock.py diff --git a/api/clarify.py b/api/clarify.py new file mode 100644 index 0000000..4fbbfc3 --- /dev/null +++ b/api/clarify.py @@ -0,0 +1,128 @@ +"""Clarify prompt state for the WebUI. + +This mirrors the approval flow structure, but the response is a free-form +clarification string instead of an approval decision. +""" + +from __future__ import annotations + +import threading +from typing import Optional + + +_lock = threading.Lock() +_pending: dict[str, dict] = {} +_gateway_queues: dict[str, list] = {} +_gateway_notify_cbs: dict[str, object] = {} + + +class _ClarifyEntry: + """One pending clarify request inside a session.""" + + __slots__ = ("event", "data", "result") + + def __init__(self, data: dict): + self.event = threading.Event() + self.data = data + self.result: Optional[str] = None + + +def register_gateway_notify(session_key: str, cb) -> None: + """Register a per-session callback for sending clarify requests to the UI.""" + with _lock: + _gateway_notify_cbs[session_key] = cb + + +def _clear_queue_locked(session_key: str) -> list[_ClarifyEntry]: + entries = _gateway_queues.pop(session_key, []) + _pending.pop(session_key, None) + return entries + + +def unregister_gateway_notify(session_key: str) -> None: + """Unregister the per-session callback and unblock any waiting clarify prompt.""" + with _lock: + _gateway_notify_cbs.pop(session_key, None) + entries = _clear_queue_locked(session_key) + for entry in entries: + entry.event.set() + + +def clear_pending(session_key: str) -> int: + """Clear any pending clarify prompts for the session without removing the callback.""" + with _lock: + entries = _clear_queue_locked(session_key) + for entry in entries: + entry.event.set() + return len(entries) + + +def submit_pending(session_key: str, data: dict) -> _ClarifyEntry: + """Queue a pending clarify request and notify the UI callback if registered.""" + with _lock: + queue = _gateway_queues.setdefault(session_key, []) + # De-duplicate while unresolved: if the most recent pending clarify is + # semantically identical, reuse it instead of stacking duplicates. + if queue: + last = queue[-1] + if ( + str(last.data.get("question", "")) == str(data.get("question", "")) + and list(last.data.get("choices_offered") or []) + == list(data.get("choices_offered") or []) + ): + entry = last + cb = _gateway_notify_cbs.get(session_key) + # Keep _pending aligned to the oldest unresolved entry. + _pending[session_key] = queue[0].data + if cb: + try: + cb(dict(entry.data)) + except Exception: + pass + return entry + + entry = _ClarifyEntry(data) + queue.append(entry) + _pending[session_key] = queue[0].data + cb = _gateway_notify_cbs.get(session_key) + if cb: + try: + cb(data) + except Exception: + pass + return entry + + +def get_pending(session_key: str) -> dict | None: + """Return the oldest pending clarify request for this session, if any.""" + with _lock: + queue = _gateway_queues.get(session_key) or [] + if queue: + return dict(queue[0].data) + pending = _pending.get(session_key) + return dict(pending) if pending else None + + +def has_pending(session_key: str) -> bool: + with _lock: + return bool(_gateway_queues.get(session_key)) + + +def resolve_clarify(session_key: str, response: str, resolve_all: bool = False) -> int: + """Resolve the oldest pending clarify request for a session.""" + with _lock: + queue = _gateway_queues.get(session_key) + if not queue: + _pending.pop(session_key, None) + return 0 + entries = list(queue) if resolve_all else [queue.pop(0)] + if queue: + _pending[session_key] = queue[0].data + else: + _clear_queue_locked(session_key) + count = 0 + for entry in entries: + entry.result = response + entry.event.set() + count += 1 + return count diff --git a/api/routes.py b/api/routes.py index 260866a..ab1173d 100644 --- a/api/routes.py +++ b/api/routes.py @@ -214,6 +214,18 @@ except ImportError: _lock = threading.Lock() _permanent_approved = set() +# Clarify prompts (optional -- graceful fallback if agent not available) +try: + from api.clarify import ( + submit_pending as submit_clarify_pending, + get_pending as get_clarify_pending, + resolve_clarify, + ) +except ImportError: + submit_clarify_pending = lambda *a, **k: None + get_clarify_pending = lambda *a, **k: None + resolve_clarify = lambda *a, **k: 0 + # ── Login page locale strings ───────────────────────────────────────────────── # Add entries here to support more languages on the login page. @@ -603,6 +615,15 @@ def handle_get(handler, parsed) -> bool: return j(handler, {"error": "not found"}, status=404) return _handle_approval_inject(handler, parsed) + if parsed.path == "/api/clarify/pending": + return _handle_clarify_pending(handler, parsed) + + if parsed.path == "/api/clarify/inject_test": + # Loopback-only: used by automated tests; blocked from any remote client + if handler.client_address[0] != "127.0.0.1": + return j(handler, {"error": "not found"}, status=404) + return _handle_clarify_inject(handler, parsed) + # ── Cron API (GET) ── if parsed.path == "/api/crons": from cron.jobs import list_jobs @@ -911,6 +932,10 @@ def handle_post(handler, parsed) -> bool: if parsed.path == "/api/approval/respond": return _handle_approval_respond(handler, body) + # ── Clarify (POST) ── + if parsed.path == "/api/clarify/respond": + return _handle_clarify_respond(handler, body) + # ── Skills (POST) ── if parsed.path == "/api/skills/save": return _handle_skill_save(handler, body) @@ -1672,6 +1697,34 @@ def _handle_approval_inject(handler, parsed): return j(handler, {"error": "session_id required"}, status=400) +def _handle_clarify_pending(handler, parsed): + sid = parse_qs(parsed.query).get("session_id", [""])[0] + pending = get_clarify_pending(sid) + if pending: + return j(handler, {"pending": pending}) + return j(handler, {"pending": None}) + + +def _handle_clarify_inject(handler, parsed): + """Inject a fake pending clarify prompt -- loopback-only, used by automated tests.""" + qs = parse_qs(parsed.query) + sid = qs.get("session_id", [""])[0] + question = qs.get("question", ["Which option?"])[0] + choices = qs.get("choices", []) + if sid: + submit_clarify_pending( + sid, + { + "question": question, + "choices_offered": choices, + "session_id": sid, + "kind": "clarify", + }, + ) + return j(handler, {"ok": True, "session_id": sid}) + return j(handler, {"error": "session_id required"}, status=400) + + def _handle_live_models(handler, parsed): """Return the live model list for a provider. @@ -1892,6 +1945,24 @@ def _handle_chat_start(handler, body): except ValueError as e: return bad(handler, str(e)) model = body.get("model") or s.model + # Prevent duplicate runs in the same session while a stream is still active. + # This commonly happens after page refresh/reconnect races and can produce + # duplicated clarify cards for what appears to be a single user request. + current_stream_id = getattr(s, "active_stream_id", None) + if current_stream_id: + with STREAMS_LOCK: + current_active = current_stream_id in STREAMS + if current_active: + return j( + handler, + { + "error": "session already has an active stream", + "active_stream_id": current_stream_id, + }, + status=409, + ) + # Stale stream id from a previous run; clear and continue. + s.active_stream_id = None stream_id = uuid.uuid4().hex s.workspace = workspace s.model = model @@ -2303,6 +2374,22 @@ def _handle_approval_respond(handler, body): return j(handler, {"ok": True, "choice": choice}) +def _handle_clarify_respond(handler, body): + sid = body.get("session_id", "") + if not sid: + return bad(handler, "session_id is required") + response = body.get("response") + if response is None: + response = body.get("answer") + if response is None: + response = body.get("choice") + response = str(response or "").strip() + if not response: + return bad(handler, "response is required") + resolve_clarify(sid, response, resolve_all=False) + return j(handler, {"ok": True, "response": response}) + + def _handle_skill_save(handler, body): try: require(body, "name", "content") diff --git a/api/streaming.py b/api/streaming.py index 6ad6a47..8aa14f4 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -88,6 +88,16 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta if q is None: return + # ── MCP Server Discovery (lazy import, idempotent) ── + # discover_mcp_tools() is called here (rather than at server startup) so that + # the hermes-agent package is fully initialized before we try to connect. + # It is safe to call multiple times — already-connected servers are skipped. + try: + from tools.mcp_tool import discover_mcp_tools + discover_mcp_tools() + except Exception: + pass # MCP not available or not configured — non-fatal + # Sprint 10: create a cancel event for this stream cancel_event = threading.Event() with STREAMS_LOCK: @@ -162,6 +172,65 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta except ImportError: logger.debug("Approval module not available, falling back to polling") + _clarify_registered = False + _unreg_clarify_notify = None + try: + from api.clarify import ( + register_gateway_notify as _reg_clarify_notify, + unregister_gateway_notify as _unreg_clarify_notify, + ) + + def _clarify_notify_cb(clarify_data): + put('clarify', clarify_data) + + _reg_clarify_notify(session_id, _clarify_notify_cb) + _clarify_registered = True + except ImportError: + logger.debug("Clarify module not available, falling back to polling") + + def _clarify_callback_impl(question, choices, sid, cancel_evt, put_event): + """Bridge Hermes clarify prompts to the WebUI.""" + timeout = 120 + choices_list = [str(choice) for choice in (choices or [])] + data = { + 'question': str(question or ''), + 'choices_offered': choices_list, + 'session_id': sid, + 'kind': 'clarify', + 'requested_at': time.time(), + } + try: + from api.clarify import submit_pending as _submit_clarify_pending, clear_pending as _clear_clarify_pending + except ImportError: + return ( + "The user did not provide a response within the time limit. " + "Use your best judgement to make the choice and proceed." + ) + + entry = _submit_clarify_pending(sid, data) + deadline = time.monotonic() + timeout + while True: + if cancel_evt.is_set(): + _clear_clarify_pending(sid) + return ( + "The user did not provide a response within the time limit. " + "Use your best judgement to make the choice and proceed." + ) + remaining = deadline - time.monotonic() + if remaining <= 0: + _clear_clarify_pending(sid) + return ( + "The user did not provide a response within the time limit. " + "Use your best judgement to make the choice and proceed." + ) + if entry.event.wait(timeout=min(1.0, remaining)): + response = str(entry.result or "").strip() + return ( + response + or "The user did not provide a response within the time limit. " + "Use your best judgement to make the choice and proceed." + ) + try: _token_sent = False # tracks whether any streamed tokens were sent _reasoning_text = '' # accumulates reasoning/thinking trace for persistence @@ -304,6 +373,11 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta stream_delta_callback=on_token, reasoning_callback=on_reasoning, tool_progress_callback=on_tool, + clarify_callback=( + lambda question, choices: _clarify_callback_impl( + question, choices, session_id, cancel_event, put + ) + ), ) # Store agent instance for cancel/interrupt propagation @@ -565,6 +639,11 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta _unreg_notify(session_id) except Exception: logger.debug("Failed to unregister approval callback") + if _clarify_registered and _unreg_clarify_notify is not None: + try: + _unreg_clarify_notify(session_id) + except Exception: + logger.debug("Failed to unregister clarify callback") with _ENV_LOCK: if old_cwd is None: os.environ.pop('TERMINAL_CWD', None) else: os.environ['TERMINAL_CWD'] = old_cwd @@ -660,6 +739,15 @@ def cancel_stream(stream_id: str) -> bool: f"cancel_event flag set, will be checked on agent startup" ) + # Clear any pending clarify prompt so the blocked tool call can unwind. + try: + from api.clarify import clear_pending as _clear_clarify_pending + + if agent and getattr(agent, "session_id", None): + _clear_clarify_pending(agent.session_id) + except Exception: + logger.debug("Failed to clear clarify prompt during cancel") + # Put a cancel sentinel into the queue so the SSE handler wakes up q = STREAMS.get(stream_id) if q: diff --git a/static/i18n.js b/static/i18n.js index 155e7e7..bca1e54 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -44,6 +44,12 @@ const LOCALES = { approval_btn_deny: 'Deny', approval_btn_deny_title: 'Deny — do not run this command', approval_responding: 'Responding\u2026', + clarify_heading: 'Clarification needed', + clarify_hint: 'Pick a choice, or type your own answer below.', + clarify_other: 'Other', + clarify_send: 'Send', + clarify_input_placeholder: 'Type your response…', + clarify_responding: 'Responding\u2026', untitled: 'Untitled', n_messages: (n) => `${n} messages`, model_unavailable: ' (unavailable)', @@ -452,6 +458,12 @@ const LOCALES = { approval_btn_deny: 'Denegar', approval_btn_deny_title: 'Denegar — no ejecutar este comando', approval_responding: 'Respondiendo…', + clarify_heading: 'Se necesita aclaración', + clarify_hint: 'Elige una opción o escribe tu propia respuesta abajo.', + clarify_other: 'Otra', + clarify_send: 'Enviar', + clarify_input_placeholder: 'Escribe tu respuesta…', + clarify_responding: 'Respondiendo…', untitled: 'Sin título', n_messages: (n) => `${n} mensajes`, model_unavailable: ' (no disponible)', @@ -852,6 +864,12 @@ const LOCALES = { approval_btn_deny: 'Ablehnen', approval_btn_deny_title: 'Ablehnen \u2014 diesen Befehl nicht ausführen', approval_responding: 'Antwortet\u2026', + clarify_heading: 'Klärung erforderlich', + clarify_hint: 'Wähle eine Option oder schreibe deine eigene Antwort unten.', + clarify_other: 'Andere', + clarify_send: 'Senden', + clarify_input_placeholder: 'Gib deine Antwort ein…', + clarify_responding: 'Antwortet\u2026', untitled: 'Unbenannt', n_messages: (n) => `${n} Nachrichten`, model_unavailable: ' (nicht verfügbar)', @@ -1056,6 +1074,12 @@ const LOCALES = { approval_btn_deny: '拒绝', approval_btn_deny_title: '拒绝 — 不执行此命令', approval_responding: '处理中…', + clarify_heading: '需要澄清', + clarify_hint: '请选择一个选项，或在下方输入你自己的回答。', + clarify_other: '其他', + clarify_send: '发送', + clarify_input_placeholder: '请输入你的回答…', + clarify_responding: '处理中…', untitled: '\u672a\u547d\u540d', n_messages: (n) => `${n} \u6761\u6d88\u606f`, model_unavailable: '\uff08\u4e0d\u53ef\u7528\uff09', @@ -1369,7 +1393,7 @@ const LOCALES = { workspace_paths_validated_hint: '保存前会校验路径是否为已存在目录。', workspace_added: '工作区已添加', workspace_remove_confirm_title: '移除工作区', - workspace_remove_confirm_message: (path) => `要移除“${path}”吗？`, + workspace_remove_confirm_message: (path) => `要移除"${path}"吗？`, workspace_removed: '工作区已移除', workspace_switch_prompt_title: '切换工作区', workspace_switch_prompt_message: '输入绝对路径以添加并切换当前会话的工作区。', @@ -1455,6 +1479,12 @@ const LOCALES = { approval_btn_deny: '\u62d2\u7edd', approval_btn_deny_title: '\u62d2\u7edd — \u4e0d\u57f7\u884c\u6b64\u547d\u4ee4', approval_responding: '\u8655\u7406\u4e2d\u2026', + clarify_heading: '\u9700\u8981\u91cb\u6e05', + clarify_hint: '\u8acb\u9078\u64c7\u4e00\u500b\u9078\u9805\uff0c\u6216\u5728\u4e0b\u65b9\u8f38\u5165\u4f60\u81ea\u5df1\u7684\u56de\u7b54\u3002', + clarify_other: '\u5176\u4ed6', + clarify_send: '\u9001\u51fa', + clarify_input_placeholder: '\u8f38\u5165\u4f60\u7684\u56de\u7b54\u2026', + clarify_responding: '\u8655\u7406\u4e2d\u2026', untitled: '\u672a\u547d\u540d', n_messages: (n) => `${n} \u689d\u8a0a\u606f`, model_unavailable: '\uff08\u4e0d\u53ef\u7528\uff09', diff --git a/static/index.html b/static/index.html index aa38b42..0297783 100644 --- a/static/index.html +++ b/static/index.html @@ -246,6 +246,21 @@ +
diff --git a/static/messages.js b/static/messages.js index 6537afb..20e7564 100644 --- a/static/messages.js +++ b/static/messages.js @@ -44,6 +44,7 @@ async function send(){ saveInflightState(activeSid,{streamId:null,messages:INFLIGHT[activeSid].messages,uploaded,toolCalls:[]}); } startApprovalPolling(activeSid); + startClarifyPolling(activeSid); S.activeStreamId = null; // will be set after stream starts // Set provisional title from user message immediately so session appears @@ -79,12 +80,34 @@ async function send(){ const cancelBtn=$('btnCancel'); if(cancelBtn) cancelBtn.style.display='inline-flex'; }catch(e){ + const errMsg=String((e&&e.message)||''); + const conflictActiveStream=/session already has an active stream/i.test(errMsg); + if(conflictActiveStream){ + delete INFLIGHT[activeSid]; + if(typeof clearInflightState==='function') clearInflightState(activeSid); + stopApprovalPolling(); + stopClarifyPolling(); + // Keep the user's attempted turn by queueing it for after the current run. + queueSessionMessage(activeSid,{text:msgText,files:[]}); + updateQueueBadge(activeSid); + showToast('Current session is still running. Reconnected and queued your message.',2600); + try{ + await loadSession(activeSid); + setComposerStatus(''); + return; + }catch(_){ + // Fall through to standard error handling if session reload fails. + } + } + delete INFLIGHT[activeSid]; stopApprovalPolling(); + stopClarifyPolling(); // Only hide approval card if it belongs to the session that just finished if(!_approvalSessionId || _approvalSessionId===activeSid) hideApprovalCard(true);removeThinking(); - S.messages.push({role:'assistant',content:`**Error:** ${e.message}`}); - renderMessages();setBusy(false);setComposerStatus(`Error: ${e.message}`); + if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true); + S.messages.push({role:'assistant',content:`**Error:** ${errMsg}`}); + renderMessages();setBusy(false);setComposerStatus(`Error: ${errMsg}`); return; } @@ -290,6 +313,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ source.addEventListener('tool',e=>{ const d=JSON.parse(e.data); + if(d.name==='clarify') return; const tc={name:d.name, preview:d.preview||'', args:d.args||{}, snippet:'', done:false, tid:d.tid||`live-${Date.now()}-${Math.random().toString(36).slice(2,8)}`}; if(!Array.isArray(INFLIGHT[activeSid].toolCalls)) INFLIGHT[activeSid].toolCalls=[]; INFLIGHT[activeSid].toolCalls.push(tc); @@ -305,6 +329,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ source.addEventListener('tool_complete',e=>{ const d=JSON.parse(e.data); + if(d.name==='clarify') return; const inflight=INFLIGHT[activeSid]; if(!inflight) return; if(!Array.isArray(inflight.toolCalls)) inflight.toolCalls=[]; @@ -340,13 +365,23 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ sendBrowserNotification('Approval required',d.description||'Tool approval needed'); }); + source.addEventListener('clarify',e=>{ + const d=JSON.parse(e.data); + d._session_id=activeSid; + showClarifyCard(d); + playNotificationSound(); + sendBrowserNotification('Clarification needed',d.question||'Tool clarification needed'); + }); + source.addEventListener('done',e=>{ source.close(); const d=JSON.parse(e.data); delete INFLIGHT[activeSid]; clearInflight();clearInflightState(activeSid); stopApprovalPolling(); + stopClarifyPolling(); if(!_approvalSessionId || _approvalSessionId===activeSid) hideApprovalCard(true); + if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true); if(S.session&&S.session.session_id===activeSid){ S.activeStreamId=null; const _cb=$('btnCancel');if(_cb)_cb.style.display='none'; @@ -396,8 +431,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ // Application-level error sent explicitly by the server (rate limit, crash, etc.) // This is distinct from the SSE network 'error' event below. source.close(); - delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling(); + delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling();stopClarifyPolling(); if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true); + if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true); if(S.session&&S.session.session_id===activeSid){ S.activeStreamId=null;const _cbe=$('btnCancel');if(_cbe)_cbe.style.display='none'; clearLiveToolCards();if(!assistantText)removeThinking(); @@ -457,8 +493,9 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ source.addEventListener('cancel',e=>{ source.close(); - delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling(); + delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling();stopClarifyPolling(); if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true); + if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true); if(S.session&&S.session.session_id===activeSid){ S.activeStreamId=null;const _cbc=$('btnCancel');if(_cbc)_cbc.style.display='none'; } @@ -472,9 +509,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ } function _handleStreamError(){ - delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling(); + delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling();stopClarifyPolling(); _closeSource(); if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true); + if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true); if(S.session&&S.session.session_id===activeSid){ S.activeStreamId=null;const _cbe=$('btnCancel');if(_cbe)_cbe.style.display='none'; clearLiveToolCards();if(!assistantText)removeThinking(); @@ -622,6 +660,255 @@ function stopApprovalPolling() { if (_approvalPollTimer) { clearInterval(_approvalPollTimer); _approvalPollTimer = null; } } +// ── Clarify polling ── +let _clarifyPollTimer = null; +let _clarifyHideTimer = null; +let _clarifyVisibleSince = 0; +let _clarifySignature = ''; +let _clarifySessionId = null; +let _clarifyMissingEndpointWarned = false; +const CLARIFY_MIN_VISIBLE_MS = 30000; + +function _ensureClarifyCardDom() { + let card = $("clarifyCard"); + if (card) return card; + const host = $("msgInner") || $("messages"); + if (!host) return null; + card = document.createElement("div"); + card.className = "clarify-card"; + card.id = "clarifyCard"; + card.setAttribute("role", "dialog"); + card.setAttribute("aria-labelledby", "clarifyHeading"); + card.setAttribute("aria-describedby", "clarifyQuestion clarifyHint"); + card.innerHTML = ` +
+
+ + Clarification needed +
+
+
+
+ + +
+
Please choose one option, or type your own response below.
+
+ `; + host.appendChild(card); + const submit = $("clarifySubmit"); + if (submit) submit.onclick = () => respondClarify(); + if (typeof applyLocaleToDOM === "function") applyLocaleToDOM(); + return card; +} + +function _clearClarifyHideTimer() { + if (_clarifyHideTimer) { + clearTimeout(_clarifyHideTimer); + _clarifyHideTimer = null; + } +} + +function _resetClarifyCardState() { + _clearClarifyHideTimer(); + _clarifyVisibleSince = 0; + _clarifySignature = ''; +} + +function hideClarifyCard(force=false) { + const card = $("clarifyCard"); + if (!card) { + _clarifySessionId = null; + _resetClarifyCardState(); + if (typeof unlockComposerForClarify === "function") unlockComposerForClarify(); + return; + } + if (!force && _clarifyVisibleSince) { + const remaining = CLARIFY_MIN_VISIBLE_MS - (Date.now() - _clarifyVisibleSince); + if (remaining > 0) { + const scheduledSignature = _clarifySignature; + _clearClarifyHideTimer(); + _clarifyHideTimer = setTimeout(() => { + _clarifyHideTimer = null; + if (_clarifySignature !== scheduledSignature) return; + hideClarifyCard(true); + }, remaining); + return; + } + } + _clarifySessionId = null; + _resetClarifyCardState(); + card.classList.remove("visible"); + if (typeof unlockComposerForClarify === "function") unlockComposerForClarify(); + $("clarifyQuestion").textContent = ""; + $("clarifyChoices").innerHTML = ""; + $("clarifyInput").value = ""; + $("clarifyInput").disabled = false; + $("clarifyInput").onkeydown = null; + const submit = $("clarifySubmit"); + if (submit) { submit.disabled = false; submit.classList.remove("loading"); } +} + +function _clarifySetControlsDisabled(disabled, loading=false) { + const input = $("clarifyInput"); + const submit = $("clarifySubmit"); + if (input) input.disabled = disabled; + if (submit) { + submit.disabled = disabled; + submit.classList.toggle("loading", !!loading); + } + const choices = $("clarifyChoices"); + if (choices) { + choices.querySelectorAll("button").forEach(btn => { + btn.disabled = disabled; + if (loading && btn.dataset && btn.dataset.choice === "other") { + btn.classList.toggle("loading", false); + } + }); + } +} + +function showClarifyCard(pending) { + const question = pending.question || pending.description || ''; + const choices = Array.isArray(pending.choices_offered) + ? pending.choices_offered + : (Array.isArray(pending.choices) ? pending.choices : []); + const sig = JSON.stringify({ + question, + choices, + sid: pending._session_id || (S.session && S.session.session_id) || null, + }); + const card = _ensureClarifyCardDom(); + if (!card) return; + const questionEl = $("clarifyQuestion"); + const choicesEl = $("clarifyChoices"); + const input = $("clarifyInput"); + const sameClarify = card.classList.contains("visible") && _clarifySignature === sig; + _clarifySessionId = pending._session_id || (S.session && S.session.session_id) || null; + _clarifySignature = sig; + if (!sameClarify) { + _clarifyVisibleSince = Date.now(); + _clearClarifyHideTimer(); + } + if (questionEl) questionEl.textContent = question; + if (choicesEl) { + choicesEl.innerHTML = ''; + choicesEl.style.display = choices.length ? '' : 'none'; + if (choices.length) { + choices.forEach((choice, idx) => { + const btn = document.createElement('button'); + btn.type = 'button'; + btn.className = 'clarify-choice'; + btn.dataset.choice = choice; + btn.onclick = () => respondClarify(choice); + const badge = document.createElement('span'); + badge.className = 'clarify-choice-badge'; + badge.textContent = String(idx + 1); + const text = document.createElement('span'); + text.className = 'clarify-choice-text'; + text.textContent = choice; + btn.appendChild(badge); + btn.appendChild(text); + choicesEl.appendChild(btn); + }); + const other = document.createElement('button'); + other.type = 'button'; + other.className = 'clarify-choice other'; + other.dataset.choice = 'other'; + other.setAttribute('data-i18n', 'clarify_other'); + const otherBadge = document.createElement('span'); + otherBadge.className = 'clarify-choice-badge other'; + otherBadge.textContent = '•'; + const otherText = document.createElement('span'); + otherText.className = 'clarify-choice-text'; + otherText.textContent = t('clarify_other') || 'Other'; + other.appendChild(otherBadge); + other.appendChild(otherText); + other.onclick = () => { + const el = $("clarifyInput"); + if (el) { + el.focus(); + if (typeof el.select === 'function') el.select(); + } + }; + choicesEl.appendChild(other); + } + } + if (input) { + if (!sameClarify) input.value = ''; + input.disabled = false; + input.onkeydown = (e) => { + if (e.key === 'Enter') { + e.preventDefault(); + respondClarify(); + } + }; + } + if (typeof lockComposerForClarify === "function") { + lockComposerForClarify(question ? `Clarification needed: ${question}` : "Clarification needed"); + } + _clarifySetControlsDisabled(false, false); + const msgInner = $("msgInner"); + if (msgInner && card.parentElement !== msgInner) { + msgInner.appendChild(card); + } + card.classList.add("visible"); + if (!sameClarify) card.scrollIntoView({block:"nearest", behavior:"smooth"}); + if (typeof applyLocaleToDOM === "function") applyLocaleToDOM(); + if (input && !sameClarify) setTimeout(() => input.focus(), 50); +} + +async function respondClarify(response) { + const sid = _clarifySessionId || (S.session && S.session.session_id); + if (!sid) return; + const input = $("clarifyInput"); + let value = typeof response === 'string' ? response : (input ? input.value : ''); + value = String(value || '').trim(); + if (!value) { + if (input) input.focus(); + return; + } + _clarifySessionId = null; + _clarifySetControlsDisabled(true, true); + hideClarifyCard(true); + try { + await api("/api/clarify/respond", { + method: "POST", + body: JSON.stringify({ session_id: sid, response: value }) + }); + } catch(e) { setStatus(t("clarify_responding") + " " + e.message); } +} + +function startClarifyPolling(sid) { + stopClarifyPolling(); + _clarifyMissingEndpointWarned = false; + _clarifyPollTimer = setInterval(async () => { + if (!S.session || S.session.session_id !== sid) { + stopClarifyPolling(); hideClarifyCard(true); return; + } + try { + const data = await api("/api/clarify/pending?session_id=" + encodeURIComponent(sid)); + if (data.pending) { data.pending._session_id=sid; showClarifyCard(data.pending); } + else { hideClarifyCard(); } + } catch(e) { + const msg = String((e && e.message) || ""); + if (!_clarifyMissingEndpointWarned && /(^|\b)(404|not found)(\b|$)/i.test(msg)) { + _clarifyMissingEndpointWarned = true; + setComposerStatus("Clarify unavailable on current server build. Restart server."); + if (typeof showToast === "function") { + showToast("Clarify endpoint unavailable. Please restart server.", 5000); + } + stopClarifyPolling(); + } + // Ignore transient poll errors; SSE clarify event still provides a fast path. + } + }, 1500); +} + +function stopClarifyPolling() { + if (_clarifyPollTimer) { clearInterval(_clarifyPollTimer); _clarifyPollTimer = null; } +} + // ── Notifications and Sound ────────────────────────────────────────────────── function playNotificationSound(){ diff --git a/static/sessions.js b/static/sessions.js index 11dba4f..f24ce8d 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -38,6 +38,8 @@ async function newSession(flash){ async function loadSession(sid){ stopApprovalPolling();hideApprovalCard(); + if(typeof stopClarifyPolling==='function') stopClarifyPolling(); + if(typeof hideClarifyCard==='function') hideClarifyCard(); const data=await api(`/api/session?session_id=${encodeURIComponent(sid)}`); S.session=data.session; S.lastUsage={...(data.session.last_usage||{})}; @@ -95,6 +97,7 @@ async function loadSession(sid){ } setBusy(true);setComposerStatus(''); startApprovalPolling(sid); + if(typeof startClarifyPolling==='function') startClarifyPolling(sid); S.activeStreamId=activeStreamId; const _cb=$('btnCancel');if(_cb&&activeStreamId)_cb.style.display='inline-flex'; if(INFLIGHT[sid].reattach&&activeStreamId&&typeof attachLiveStream==='function'){ @@ -122,6 +125,7 @@ async function loadSession(sid){ syncTopbar();renderMessages();appendThinking();loadDir('.'); updateQueueBadge(sid); startApprovalPolling(sid); + if(typeof startClarifyPolling==='function') startClarifyPolling(sid); if(typeof attachLiveStream==='function') attachLiveStream(sid, activeStreamId, data.session.pending_attachments||[], {reconnecting:true}); else if(typeof watchInflightSession==='function') watchInflightSession(sid, activeStreamId); }else{ diff --git a/static/style.css b/static/style.css index 5063dc3..483707e 100644 --- a/static/style.css +++ b/static/style.css @@ -304,6 +304,30 @@ .approval-btn.deny{border-color:rgba(233,69,96,0.5);color:var(--accent);} .approval-btn.deny:hover{background:rgba(233,69,96,0.12);border-color:rgba(233,69,96,0.7);} .approval-btn.loading{opacity:.7;cursor:wait;} + /* ── Clarify card ── */ + .clarify-card{display:none;max-width:680px;margin:4px 0 2px 40px;padding:0;} + .clarify-card.visible{display:block;} + .clarify-inner{background:rgba(255,255,255,.03);backdrop-filter:blur(8px);border:1px solid rgba(124,185,255,0.16);border-radius:12px;padding:12px 14px 13px;box-shadow:0 1px 0 rgba(255,255,255,.02) inset;} + .clarify-header{display:flex;align-items:center;gap:8px;margin-bottom:10px;font-size:12px;font-weight:700;color:var(--blue);letter-spacing:.01em;} + .clarify-question{font-size:14px;color:var(--text);line-height:1.7;white-space:pre-wrap;margin-bottom:12px;} + .clarify-choices{display:flex;flex-direction:column;gap:8px;margin-bottom:12px;} + .clarify-choice{display:flex;align-items:flex-start;gap:10px;width:100%;padding:11px 14px;border-radius:12px;font-size:13px;font-weight:600;border:1px solid rgba(124,185,255,0.3);background:rgba(124,185,255,0.08);color:var(--blue);cursor:pointer;transition:all .15s;white-space:normal;text-align:left;box-shadow:0 1px 0 rgba(255,255,255,.03) inset;} + .clarify-choice:hover{background:rgba(124,185,255,0.16);transform:translateY(-1px);box-shadow:0 4px 12px rgba(0,0,0,0.18);} + .clarify-choice:focus-visible{outline:2px solid rgba(124,185,255,.75);outline-offset:2px;} + .clarify-choice-badge{display:inline-flex;align-items:center;justify-content:center;min-width:24px;height:24px;border-radius:999px;background:rgba(124,185,255,0.16);border:1px solid rgba(124,185,255,0.3);color:var(--blue);font-size:11px;font-weight:800;flex-shrink:0;line-height:1;} + .clarify-choice-badge.other{background:rgba(201,168,76,0.12);border-color:rgba(201,168,76,0.32);color:var(--gold);} + .clarify-choice-text{flex:1;line-height:1.45;min-width:0;} + .clarify-choice.other{border-color:rgba(201,168,76,0.35);color:var(--gold);background:rgba(201,168,76,0.08);} + .clarify-choice.other:hover{background:rgba(201,168,76,0.14);border-color:rgba(201,168,76,0.55);} + .clarify-response{display:flex;gap:8px;align-items:center;flex-wrap:wrap;} + .clarify-input{flex:1;min-width:220px;padding:10px 12px;border-radius:8px;border:1px solid var(--border2);background:var(--input-bg);color:var(--text);font:inherit;outline:none;transition:all .15s;} + .clarify-input:focus{border-color:rgba(124,185,255,.5);box-shadow:0 0 0 3px rgba(124,185,255,.08);background:var(--hover-bg);} + .clarify-submit{display:inline-flex;align-items:center;justify-content:center;min-width:92px;padding:10px 14px;border-radius:8px;border:1px solid rgba(124,185,255,0.35);background:rgba(124,185,255,0.14);color:var(--blue);font-size:12px;font-weight:700;cursor:pointer;transition:all .15s;white-space:nowrap;} + .clarify-submit:hover{background:rgba(124,185,255,0.22);transform:translateY(-1px);} + .clarify-submit:disabled{opacity:.6;cursor:not-allowed;transform:none;} + .clarify-submit.loading{opacity:.75;cursor:wait;} + .clarify-hint{margin-top:8px;font-size:11px;line-height:1.45;color:var(--muted);} + .clarify-card.visible .clarify-question{padding-left:1px;} /* Sidebar navigation tabs */ .sidebar-nav{display:flex;border-bottom:1px solid var(--border);flex-shrink:0;padding:6px 8px 0;gap:2px;} .nav-tab{flex:1;padding:10px 4px 8px;font-size:20px;text-align:center;cursor:pointer;color:var(--muted);border:none;background:none;transition:color .15s;border-bottom:2px solid transparent;white-space:nowrap;overflow:hidden;position:relative;} @@ -691,6 +715,13 @@ .approval-btns{gap:6px;} .approval-btn{padding:8px 12px;font-size:12px;min-height:44px;} .approval-kbd{display:none;} + /* Clarify card */ + .clarify-card{margin:6px 0 4px 0;max-width:100%;} + .clarify-inner{padding:12px 12px 13px;} + .clarify-response{flex-direction:column;align-items:stretch;} + .clarify-input,.clarify-submit{width:100%;min-height:44px;} + .clarify-choice{min-height:44px;} + .clarify-choice-badge{min-width:22px;height:22px;} .app-dialog-overlay{padding:12px;} .app-dialog{width:100%;padding:16px 16px 14px;border-radius:16px;} .app-dialog-actions{flex-direction:column-reverse;align-items:stretch;} @@ -825,6 +856,7 @@ /* Approval buttons: tab stops */ .approval-btn:focus{outline:2px solid var(--blue);outline-offset:2px;} +.clarify-choice:focus,.clarify-submit:focus,.clarify-input:focus{outline:2px solid var(--blue);outline-offset:2px;} /* Message role: breathing room between icon and name */ .msg-role > span{line-height:1;} diff --git a/static/ui.js b/static/ui.js index 3eb479c..3fb2601 100644 --- a/static/ui.js +++ b/static/ui.js @@ -611,11 +611,43 @@ function setComposerStatus(t){ el.style.display=''; } +let _composerLockState=null; + +function lockComposerForClarify(placeholderText){ + const input=$('msg'); + if(!input) return; + if(!_composerLockState){ + _composerLockState={ + disabled: input.disabled, + placeholder: input.placeholder, + }; + } + input.disabled=true; + if(placeholderText) input.placeholder=placeholderText; + updateSendBtn(); +} + +function unlockComposerForClarify(){ + const input=$('msg'); + if(!input) return; + if(_composerLockState){ + input.disabled=!!_composerLockState.disabled; + if(typeof _composerLockState.placeholder==='string'){ + input.placeholder=_composerLockState.placeholder; + } + _composerLockState=null; + }else{ + input.disabled=false; + } + updateSendBtn(); +} + function updateSendBtn(){ const btn=$('btnSend'); if(!btn) return; - const hasContent=$('msg').value.trim().length>0||S.pendingFiles.length>0; - const canSend=hasContent&&!S.busy; + const msg=$('msg'); + const hasContent=msg&&msg.value.trim().length>0||S.pendingFiles.length>0; + const canSend=hasContent&&!S.busy&&!(msg&&msg.disabled); // Hide while busy (cancel button takes its place); show otherwise btn.style.display=S.busy?'none':''; btn.disabled=!canSend; @@ -1830,4 +1862,3 @@ async function uploadPendingFiles(){ if(failures===total&&total>0)throw new Error(t('all_uploads_failed',total)); return names; } - diff --git a/tests/test_clarify_unblock.py b/tests/test_clarify_unblock.py new file mode 100644 index 0000000..89fb7d2 --- /dev/null +++ b/tests/test_clarify_unblock.py @@ -0,0 +1,165 @@ +"""Tests for clarify prompt unblocking and HTTP endpoints.""" + +import json +import threading +import uuid +import urllib.request +import urllib.error +import urllib.parse + +import pytest + +try: + from api.clarify import ( + register_gateway_notify, + unregister_gateway_notify, + resolve_clarify, + clear_pending, + _gateway_queues, + _gateway_notify_cbs, + _lock, + _ClarifyEntry, + submit_pending, + ) + CLARIFY_AVAILABLE = True +except ImportError: + CLARIFY_AVAILABLE = False + +pytestmark = pytest.mark.skipif( + not CLARIFY_AVAILABLE, + reason="api.clarify not available in this environment", +) + +from tests._pytest_port import BASE + + +def get(path): + url = BASE + path + with urllib.request.urlopen(url, timeout=10) as r: + return json.loads(r.read()) + + +def post(path, body=None): + url = BASE + path + data = json.dumps(body or {}).encode() + req = urllib.request.Request(url, data=data, headers={"Content-Type": "application/json"}) + try: + with urllib.request.urlopen(req, timeout=10) as r: + return json.loads(r.read()), r.status + except urllib.error.HTTPError as e: + return json.loads(e.read()), e.code + + +class TestClarifyUnblocking: + """Unit tests for clarify queue resolution.""" + + def test_resolve_clarify_sets_event(self): + sid = f"unit-clarify-{uuid.uuid4().hex[:8]}" + entry = _ClarifyEntry({"question": "Pick one", "choices_offered": ["a", "b"]}) + with _lock: + _gateway_queues.setdefault(sid, []).append(entry) + + resolved = resolve_clarify(sid, "a", resolve_all=False) + assert resolved == 1 + assert entry.event.is_set() + assert entry.result == "a" + + def test_register_and_fire_notify_cb(self): + sid = f"unit-notify-{uuid.uuid4().hex[:8]}" + fired = [] + register_gateway_notify(sid, lambda d: fired.append(d)) + + with _lock: + cb = _gateway_notify_cbs.get(sid) + assert cb is not None + + data = {"question": "What now?", "choices_offered": ["x", "y"]} + cb(data) + assert fired == [data] + + unregister_gateway_notify(sid) + + def test_clear_pending_unblocks_waiters(self): + sid = f"unit-clear-{uuid.uuid4().hex[:8]}" + entry = _ClarifyEntry({"question": "Wait", "choices_offered": []}) + with _lock: + _gateway_queues.setdefault(sid, []).append(entry) + + cleared = clear_pending(sid) + assert cleared == 1 + assert entry.event.is_set() + with _lock: + assert sid not in _gateway_queues + + def test_submit_pending_registers_entry(self): + sid = f"unit-submit-{uuid.uuid4().hex[:8]}" + data = {"question": "Pick", "choices_offered": ["one", "two"], "session_id": sid} + entry = submit_pending(sid, data) + assert entry.data == data + with _lock: + assert sid in _gateway_queues + + clear_pending(sid) + + +class TestClarifyModuleExports: + def test_register_gateway_notify_exported(self): + import api.clarify as ap + assert hasattr(ap, "register_gateway_notify") + + def test_unregister_gateway_notify_exported(self): + import api.clarify as ap + assert hasattr(ap, "unregister_gateway_notify") + + def test_resolve_clarify_exported(self): + import api.clarify as ap + assert hasattr(ap, "resolve_clarify") + + def test_clarify_entry_exported(self): + import api.clarify as ap + assert hasattr(ap, "_ClarifyEntry") + + +class TestClarifyHTTPEndpoints: + """Regression tests for /api/clarify/respond against the live test server.""" + + def test_respond_returns_ok_no_pending(self): + sid = f"http-no-pending-{uuid.uuid4().hex[:8]}" + result, status = post("/api/clarify/respond", { + "session_id": sid, + "response": "Use option A", + }) + assert status == 200 + assert result["ok"] is True + + def test_respond_requires_session_id(self): + result, status = post("/api/clarify/respond", {"response": "Hello"}) + assert status == 400 + + def test_respond_requires_response(self): + sid = f"http-no-response-{uuid.uuid4().hex[:8]}" + result, status = post("/api/clarify/respond", {"session_id": sid}) + assert status == 400 + + def test_respond_clears_injected_pending(self): + sid = f"http-clear-{uuid.uuid4().hex[:8]}" + question = urllib.parse.quote("Pick the better option") + choices = urllib.parse.quote("A") + inject = get( + f"/api/clarify/inject_test?session_id={urllib.parse.quote(sid)}" + f"&question={question}&choices={choices}" + ) + assert inject["ok"] is True + + data = get(f"/api/clarify/pending?session_id={urllib.parse.quote(sid)}") + assert data["pending"] is not None + + result, status = post("/api/clarify/respond", { + "session_id": sid, + "response": "B", + }) + assert status == 200 + assert result["ok"] is True + + data2 = get(f"/api/clarify/pending?session_id={urllib.parse.quote(sid)}") + assert data2["pending"] is None diff --git a/tests/test_sprint30.py b/tests/test_sprint30.py index 93dc823..328d670 100644 --- a/tests/test_sprint30.py +++ b/tests/test_sprint30.py @@ -91,6 +91,31 @@ class TestApprovalCardHTML: "approval card missing aria-labelledby" +class TestClarifyCardHTML: + + def test_clarify_card_markup_present(self): + html = read(REPO / "static/index.html") + assert 'id="clarifyCard"' in html, "clarify card missing from index.html" + assert 'id="clarifyHeading"' in html, "clarify heading missing" + assert 'id="clarifyQuestion"' in html, "clarify question text missing" + assert 'id="clarifyChoices"' in html, "clarify choices container missing" + assert 'id="clarifyInput"' in html, "clarify input missing" + assert 'id="clarifySubmit"' in html, "clarify submit button missing" + + def test_clarify_card_has_data_i18n(self): + html = read(REPO / "static/index.html") + assert 'data-i18n="clarify_heading"' in html + assert 'data-i18n="clarify_send"' in html + assert 'data-i18n-placeholder="clarify_input_placeholder"' in html + + def test_clarify_card_has_aria_roles(self): + html = read(REPO / "static/index.html") + assert 'role="dialog"' in html, \ + "clarify card missing role=dialog for accessibility" + assert 'aria-labelledby="clarifyHeading"' in html, \ + "clarify card missing aria-labelledby" + + # ── CSS ────────────────────────────────────────────────────────────────────── class TestApprovalCardCSS: @@ -130,6 +155,37 @@ class TestApprovalCardCSS: assert cls in css, f"CSS class '{cls}' missing" +class TestClarifyCardCSS: + + def test_clarify_styles_present(self): + css = read(REPO / "static/style.css") + for cls in ( + ".clarify-card", + ".clarify-card.visible", + ".clarify-inner", + ".clarify-header", + ".clarify-question", + ".clarify-choices", + ".clarify-choice", + ".clarify-response", + ".clarify-input", + ".clarify-submit", + ".clarify-hint", + ): + assert cls in css, f"CSS class '{cls}' missing" + + def test_clarify_mobile_styles_present(self): + css = read(REPO / "static/style.css") + assert ".clarify-card{padding:0 10px 8px;}" in css or \ + ".clarify-card { padding:0 10px 8px; }" in css or \ + "clarify-card" in css, "clarify mobile styles missing" + + def test_clarify_focus_styles_present(self): + css = read(REPO / "static/style.css") + assert ".clarify-choice:focus" in css and ".clarify-submit:focus" in css, \ + "clarify focus styles missing" + + # ── i18n keys ──────────────────────────────────────────────────────────────── class TestApprovalI18nKeys: @@ -178,6 +234,38 @@ class TestApprovalI18nKeys: "English approval_btn_deny value incorrect" +class TestClarifyI18nKeys: + + REQUIRED_KEYS = [ + "clarify_heading", + "clarify_hint", + "clarify_other", + "clarify_send", + "clarify_input_placeholder", + "clarify_responding", + ] + + def test_english_locale_has_all_clarify_keys(self): + src = read(REPO / "static/i18n.js") + en_block_end = src.find("\n};") + en_block = src[:en_block_end] + for key in self.REQUIRED_KEYS: + assert f"{key}:" in en_block, f"English locale missing i18n key: {key}" + + def test_chinese_locale_has_all_clarify_keys(self): + src = read(REPO / "static/i18n.js") + zh_start = src.find("\n zh: {") + assert zh_start != -1, "zh locale block not found in i18n.js" + zh_block = src[zh_start:] + for key in self.REQUIRED_KEYS: + assert f"{key}:" in zh_block, f"Chinese locale missing i18n key: {key}" + + def test_clarify_heading_english_value(self): + src = read(REPO / "static/i18n.js") + assert "clarify_heading: 'Clarification needed'" in src, \ + "English clarify_heading value incorrect" + + # ── messages.js behaviour ──────────────────────────────────────────────────── class TestApprovalMessagesJS: @@ -209,6 +297,30 @@ class TestApprovalMessagesJS: "showApprovalCard should focus the Allow once button" +class TestClarifyMessagesJS: + + def test_clarify_event_listener_present(self): + src = read(REPO / "static/messages.js") + assert "addEventListener('clarify'" in src, \ + "clarify SSE listener missing from messages.js" + + def test_show_clarify_card_present(self): + src = read(REPO / "static/messages.js") + assert "function showClarifyCard" in src, "showClarifyCard missing" + assert "clarifyChoices" in src and "clarifyInput" in src, \ + "showClarifyCard should manage clarify DOM elements" + + def test_respond_clarify_uses_api_endpoint(self): + src = read(REPO / "static/messages.js") + assert '/api/clarify/respond' in src, \ + "respondClarify should POST to /api/clarify/respond" + + def test_clarify_polling_helpers_present(self): + src = read(REPO / "static/messages.js") + for token in ("startClarifyPolling", "stopClarifyPolling", "hideClarifyCard", "_clarifySessionId"): + assert token in src, f"{token} missing from messages.js" + + # ── boot.js keyboard shortcut ──────────────────────────────────────────────── class TestApprovalKeyboardShortcut: @@ -248,6 +360,21 @@ class TestStreamingApprovalScoping: assert "_approval_registered = False" in src, \ "_approval_registered flag must be initialised to False" + def test_clarify_registered_flag_present(self): + src = read(REPO / "api/streaming.py") + assert "_clarify_registered = False" in src, \ + "_clarify_registered flag must be initialised to False" + + def test_clarify_unreg_notify_initialised_to_none(self): + src = read(REPO / "api/streaming.py") + assert "_unreg_clarify_notify = None" in src, \ + "_unreg_clarify_notify must be initialised to None before the try block" + + def test_finally_checks_clarify_unreg_notify_not_none(self): + src = read(REPO / "api/streaming.py") + assert "_unreg_clarify_notify is not None" in src, \ + "finally block must check '_unreg_clarify_notify is not None' before calling it" + # ── HTTP regression: approval respond ──────────────────────────────────────── @@ -384,3 +511,66 @@ class TestApprovalCardTimerLogic: src = self._get_js().read_text() assert '_clearApprovalHideTimer' in src, \ '_clearApprovalHideTimer helper must exist to cancel deferred setTimeout' + + +class TestClarifyCardTimerLogic: + + def _get_js(self): + return pathlib.Path(__file__).parent.parent / 'static' / 'messages.js' + + def test_clarify_min_visible_ms_constant_present(self): + src = self._get_js().read_text() + assert 'CLARIFY_MIN_VISIBLE_MS' in src + import re + m = re.search(r'CLARIFY_MIN_VISIBLE_MS\s*=\s*(\d+)', src) + assert m is not None, 'CLARIFY_MIN_VISIBLE_MS not assigned' + assert int(m.group(1)) == 30000, f'Expected 30000, got {m.group(1)}' + + def test_hide_clarify_card_has_force_parameter(self): + src = self._get_js().read_text() + assert 'hideClarifyCard(force=false)' in src or \ + 'hideClarifyCard(force = false)' in src, \ + 'hideClarifyCard must have force=false default parameter' + + def test_hide_clarify_card_checks_force_flag(self): + src = self._get_js().read_text() + assert '!force' in src, 'hideClarifyCard must check !force before deferred hide' + + def test_clarify_hide_timer_variable_present(self): + src = self._get_js().read_text() + assert '_clarifyHideTimer' in src + + def test_clarify_visible_since_variable_present(self): + src = self._get_js().read_text() + assert '_clarifyVisibleSince' in src + + def test_clarify_signature_variable_present(self): + src = self._get_js().read_text() + assert '_clarifySignature' in src + + def test_respond_clarify_calls_hide_with_force(self): + src = self._get_js().read_text() + import re + m = re.search(r'async function respondClarify.*?(?=\nasync function|\nfunction |\Z)', + src, re.DOTALL) + assert m, 'respondClarify function not found' + body = m.group(0) + assert 'hideClarifyCard(true)' in body, \ + 'respondClarify must call hideClarifyCard(true) so card hides immediately after user clicks' + + def test_clarify_poll_loop_uses_no_force(self): + src = self._get_js().read_text() + assert 'else { hideClarifyCard(); }' in src or \ + 'else {hideClarifyCard();}' in src or \ + 'else { hideClarifyCard() }' in src, \ + 'Clarify poll loop should hide without force=true' + + def test_show_clarify_card_signature_dedup(self): + src = self._get_js().read_text() + import re + m = re.search(r'function showClarifyCard.*?(?=\nfunction |\nasync function |\Z)', + src, re.DOTALL) + assert m, 'showClarifyCard function not found' + body = m.group(0) + assert 'JSON.stringify' in body, 'showClarifyCard must compute a signature via JSON.stringify' + assert '_clarifySignature' in body, 'showClarifyCard must check/set _clarifySignature' From 1bd0341243cee2747a5b0cf7ec8d15b44825a44f Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Wed, 15 Apr 2026 07:24:53 +0000 Subject: [PATCH 2/2] fix: expand test_sprint36 search window for setBusy after stopClarifyPolling additions --- tests/test_sprint36.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_sprint36.py b/tests/test_sprint36.py index c6b265b..1475d51 100644 --- a/tests/test_sprint36.py +++ b/tests/test_sprint36.py @@ -154,7 +154,7 @@ def test_sse_cancel_handler_calls_set_busy(): if idx == -1: idx = src.find('addEventListener("cancel"') assert idx != -1 - block = src[idx:idx + 800] + block = src[idx:idx + 1000] assert "setBusy(false)" in block, ( "SSE cancel handler no longer calls setBusy(false)" )