fix: preserve clarify drafts on timeout

This commit is contained in:
Andy
2026-04-28 15:57:58 +09:00
committed by Hermes Agent
parent 4dbc9ac3e1
commit 9fabd12e41
6 changed files with 153 additions and 25 deletions
+14
View File
@@ -7,9 +7,11 @@ clarification string instead of an approval decision.
from __future__ import annotations
import threading
import time
from typing import Optional
DEFAULT_TIMEOUT_SECONDS = 120
_lock = threading.Lock()
_pending: dict[str, dict] = {}
_gateway_queues: dict[str, list] = {}
@@ -57,8 +59,20 @@ def clear_pending(session_key: str) -> int:
return len(entries)
def _with_timeout_metadata(data: dict) -> dict:
item = dict(data or {})
requested_at = float(item.get("requested_at") or time.time())
timeout_seconds = int(item.get("timeout_seconds") or DEFAULT_TIMEOUT_SECONDS)
expires_at = float(item.get("expires_at") or requested_at + timeout_seconds)
item["requested_at"] = requested_at
item["timeout_seconds"] = timeout_seconds
item["expires_at"] = expires_at
return item
def submit_pending(session_key: str, data: dict) -> _ClarifyEntry:
"""Queue a pending clarify request and notify the UI callback if registered."""
data = _with_timeout_metadata(data)
with _lock:
queue = _gateway_queues.setdefault(session_key, [])
# De-duplicate while unresolved: if the most recent pending clarify is
+1
View File
@@ -297,6 +297,7 @@
<div class="clarify-header">
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"><path d="M12 17h.01"/><path d="M9.09 9a3 3 0 1 1 5.82 1c0 2-3 2-3 4"/><circle cx="12" cy="12" r="10"/></svg>
<span id="clarifyHeading" data-i18n="clarify_heading">Clarification needed</span>
<span class="clarify-countdown" id="clarifyCountdown"></span>
</div>
<div class="clarify-question" id="clarifyQuestion"></div>
<div class="clarify-choices" id="clarifyChoices"></div>
+89 -13
View File
@@ -230,7 +230,7 @@ async function send(){
stopClarifyPolling();
// Only hide approval card if it belongs to the session that just finished
if(!_approvalSessionId || _approvalSessionId===activeSid) hideApprovalCard(true);removeThinking();
if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true);
if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
S.messages.push({role:'assistant',content:`**Error:** ${errMsg}`});
_queueDrainSid=activeSid;renderMessages();setBusy(false);setComposerStatus(`Error: ${errMsg}`);
return;
@@ -785,7 +785,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
stopApprovalPolling();
stopClarifyPolling();
if(!_approvalSessionId || _approvalSessionId===activeSid) hideApprovalCard(true);
if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true);
if(!_clarifySessionId || _clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
if(isActiveSession){
S.activeStreamId=null;
}
@@ -932,7 +932,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
source.close();
delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling();stopClarifyPolling();
if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
if(S.session&&S.session.session_id===activeSid){
S.activeStreamId=null;
clearLiveToolCards();if(!assistantText)removeThinking();
@@ -1010,7 +1010,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
source.close();
delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling();stopClarifyPolling();
if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
if(S.session&&S.session.session_id===activeSid){
S.activeStreamId=null;
}
@@ -1050,7 +1050,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling();stopClarifyPolling();
_closeSource();
if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
const isSessionViewed=_isSessionActivelyViewed(activeSid);
const completedSid=session.session_id||activeSid;
if(!isSessionViewed && typeof _markSessionCompletionUnread==='function'){
@@ -1091,7 +1091,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
delete INFLIGHT[activeSid];clearInflight();clearInflightState(activeSid);stopApprovalPolling();stopClarifyPolling();
_closeSource();
if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
if(S.session&&S.session.session_id===activeSid){
S.activeStreamId=null;
clearLiveToolCards();if(!assistantText)removeThinking();
@@ -1119,7 +1119,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
stopApprovalPolling();
stopClarifyPolling();
if(!_approvalSessionId||_approvalSessionId===activeSid) hideApprovalCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true);
if(!_clarifySessionId||_clarifySessionId===activeSid) hideClarifyCard(true, 'terminal');
if(S.session&&S.session.session_id===activeSid){
S.activeStreamId=null;
clearLiveToolCards();
@@ -1331,6 +1331,8 @@ let _clarifyVisibleSince = 0;
let _clarifySignature = '';
let _clarifySessionId = null;
let _clarifyMissingEndpointWarned = false;
let _clarifyCountdownTimer = null;
let _clarifyExpiresAt = 0;
const CLARIFY_MIN_VISIBLE_MS = 30000;
function _ensureClarifyCardDom() {
@@ -1349,6 +1351,7 @@ function _ensureClarifyCardDom() {
<div class="clarify-header">
<svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"><path d="M12 17h.01"/><path d="M9.09 9a3 3 0 1 1 5.82 1c0 2-3 2-3 4"/><circle cx="12" cy="12" r="10"/></svg>
<span id="clarifyHeading" data-i18n="clarify_heading">Clarification needed</span>
<span class="clarify-countdown" id="clarifyCountdown"></span>
</div>
<div class="clarify-question" id="clarifyQuestion"></div>
<div class="clarify-choices" id="clarifyChoices"></div>
@@ -1373,13 +1376,84 @@ function _clearClarifyHideTimer() {
}
}
function _clearClarifyCountdownTimer() {
if (_clarifyCountdownTimer) {
clearInterval(_clarifyCountdownTimer);
_clarifyCountdownTimer = null;
}
_clarifyExpiresAt = 0;
const countdown = $("clarifyCountdown");
if (countdown) {
countdown.textContent = "";
countdown.classList.remove("urgent");
}
}
function _clarifyExpiryMs(pending) {
const expiresAt = Number(pending && pending.expires_at);
if (Number.isFinite(expiresAt) && expiresAt > 0) return expiresAt * 1000;
const requestedAt = Number(pending && pending.requested_at);
const timeoutSeconds = Number(pending && pending.timeout_seconds);
if (Number.isFinite(requestedAt) && Number.isFinite(timeoutSeconds)) {
return (requestedAt + timeoutSeconds) * 1000;
}
return 0;
}
function _updateClarifyCountdown() {
const countdown = $("clarifyCountdown");
if (!countdown || !_clarifyExpiresAt) return;
const remaining = Math.max(0, Math.ceil((_clarifyExpiresAt - Date.now()) / 1000));
countdown.textContent = `${remaining}s`;
countdown.classList.toggle("urgent", remaining <= 10);
}
function _startClarifyCountdown(pending) {
_clearClarifyCountdownTimer();
_clarifyExpiresAt = _clarifyExpiryMs(pending);
if (!_clarifyExpiresAt) return;
_updateClarifyCountdown();
_clarifyCountdownTimer = setInterval(_updateClarifyCountdown, 1000);
}
function _stashClarifyDraft(reason) {
if (reason !== "expired" && reason !== "terminal") return false;
const input = $("clarifyInput");
const draft = String((input && input.value) || "").trim();
if (!draft) return false;
const sid = _clarifySessionId || (S.session && S.session.session_id) || "unknown";
const key = `hermes-clarify-draft-${sid}-${_clarifySignature || "unknown"}`;
try {
sessionStorage.setItem(key, JSON.stringify({
draft,
reason,
saved_at: Date.now(),
}));
} catch (_) {}
const composer = $('msg');
if (composer) {
const current = String(composer.value || "");
composer.value = current.trim() ? `${current.replace(/\s+$/, "")}\n\n${draft}` : draft;
if (typeof autoResize === "function") autoResize();
if (typeof updateSendBtn === "function") updateSendBtn();
}
const notice = reason === "expired"
? "Clarification timed out. Your draft was kept in the composer."
: "Clarification closed. Your draft was kept in the composer.";
if (typeof setComposerStatus === "function") setComposerStatus(notice);
else if (typeof setStatus === "function") setStatus(notice);
if (typeof showToast === "function") showToast(notice, 5000);
return true;
}
function _resetClarifyCardState() {
_clearClarifyHideTimer();
_clearClarifyCountdownTimer();
_clarifyVisibleSince = 0;
_clarifySignature = '';
}
function hideClarifyCard(force=false) {
function hideClarifyCard(force=false, reason="dismissed") {
const card = $("clarifyCard");
if (!card) {
_clarifySessionId = null;
@@ -1387,7 +1461,7 @@ function hideClarifyCard(force=false) {
if (typeof unlockComposerForClarify === "function") unlockComposerForClarify();
return;
}
if (!force && _clarifyVisibleSince) {
if (!force && reason !== "expired" && _clarifyVisibleSince) {
const remaining = CLARIFY_MIN_VISIBLE_MS - (Date.now() - _clarifyVisibleSince);
if (remaining > 0) {
const scheduledSignature = _clarifySignature;
@@ -1395,11 +1469,12 @@ function hideClarifyCard(force=false) {
_clarifyHideTimer = setTimeout(() => {
_clarifyHideTimer = null;
if (_clarifySignature !== scheduledSignature) return;
hideClarifyCard(true);
hideClarifyCard(true, reason);
}, remaining);
return;
}
}
_stashClarifyDraft(reason);
_clarifySessionId = null;
_resetClarifyCardState();
card.classList.remove("visible");
@@ -1450,6 +1525,7 @@ function showClarifyCard(pending) {
const sameClarify = card.classList.contains("visible") && _clarifySignature === sig;
_clarifySessionId = pending._session_id || (S.session && S.session.session_id) || null;
_clarifySignature = sig;
_startClarifyCountdown(pending);
if (!sameClarify) {
_clarifyVisibleSince = Date.now();
_clearClarifyHideTimer();
@@ -1529,7 +1605,7 @@ async function respondClarify(response) {
}
_clarifySessionId = null;
_clarifySetControlsDisabled(true, true);
hideClarifyCard(true);
hideClarifyCard(true, 'sent');
try {
await api("/api/clarify/respond", {
method: "POST",
@@ -1543,12 +1619,12 @@ function startClarifyPolling(sid) {
_clarifyMissingEndpointWarned = false;
_clarifyPollTimer = setInterval(async () => {
if (!S.session || S.session.session_id !== sid) {
stopClarifyPolling(); hideClarifyCard(true); return;
stopClarifyPolling(); hideClarifyCard(true, 'session'); 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(); }
else { hideClarifyCard(false, 'expired'); }
} catch(e) {
const msg = String((e && e.message) || "");
if (!_clarifyMissingEndpointWarned && /(^|\b)(404|not found)(\b|$)/i.test(msg)) {
+2
View File
@@ -472,6 +472,8 @@
.clarify-card.visible .clarify-inner{transform:translateY(0);opacity:1;}
.clarify-inner{background:var(--surface);backdrop-filter:blur(8px);border:1px solid var(--accent-bg-strong);border-radius:12px;padding:12px 14px 36px;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-countdown{margin-left:auto;min-width:42px;text-align:right;color:var(--muted);font-variant-numeric:tabular-nums;font-weight:700;}
.clarify-countdown.urgent{color:var(--error);}
.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 var(--accent-bg-strong);background:var(--accent-bg);color:var(--accent-text);cursor:pointer;transition:all .15s;white-space:normal;text-align:left;box-shadow:0 1px 0 rgba(255,255,255,.03) inset;}
+18 -4
View File
@@ -1,7 +1,6 @@
"""Tests for clarify prompt unblocking and HTTP endpoints."""
import json
import threading
import uuid
import urllib.request
import urllib.error
@@ -9,6 +8,8 @@ import urllib.parse
import pytest
from tests._pytest_port import BASE
try:
from api.clarify import (
register_gateway_notify,
@@ -30,8 +31,6 @@ pytestmark = pytest.mark.skipif(
reason="api.clarify not available in this environment",
)
from tests._pytest_port import BASE
def get(path):
url = BASE + path
@@ -95,12 +94,27 @@ class TestClarifyUnblocking:
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
assert entry.data["question"] == data["question"]
assert entry.data["choices_offered"] == data["choices_offered"]
assert entry.data["session_id"] == data["session_id"]
with _lock:
assert sid in _gateway_queues
clear_pending(sid)
def test_submit_pending_adds_timeout_metadata(self):
sid = f"unit-timeout-{uuid.uuid4().hex[:8]}"
entry = submit_pending(sid, {"question": "Wait", "choices_offered": []})
assert isinstance(entry.data["requested_at"], (int, float))
assert entry.data["timeout_seconds"] == 120
assert entry.data["expires_at"] == pytest.approx(
entry.data["requested_at"] + 120,
abs=0.1,
)
clear_pending(sid)
class TestClarifyModuleExports:
def test_register_gateway_notify_exported(self):
+29 -8
View File
@@ -12,13 +12,12 @@ Tests for:
"""
import json
import pathlib
import re
import urllib.request
import urllib.error
import urllib.parse
import pytest
from tests._pytest_port import BASE
@@ -44,8 +43,6 @@ def read(path):
with open(path, encoding="utf-8") as f:
return f.read()
import pathlib
REPO = pathlib.Path(__file__).parent.parent
@@ -518,6 +515,9 @@ class TestClarifyCardTimerLogic:
def _get_js(self):
return pathlib.Path(__file__).parent.parent / 'static' / 'messages.js'
def _get_html(self):
return pathlib.Path(__file__).parent.parent / 'static' / 'index.html'
def test_clarify_min_visible_ms_constant_present(self):
src = self._get_js().read_text()
assert 'CLARIFY_MIN_VISIBLE_MS' in src
@@ -529,6 +529,7 @@ class TestClarifyCardTimerLogic:
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, reason=' in src or \
'hideClarifyCard(force = false)' in src, \
'hideClarifyCard must have force=false default parameter'
@@ -548,6 +549,25 @@ class TestClarifyCardTimerLogic:
src = self._get_js().read_text()
assert '_clarifySignature' in src
def test_clarify_countdown_element_present(self):
html = self._get_html().read_text()
assert 'id="clarifyCountdown"' in html, \
'clarify card must include a countdown element so users see timeout risk'
def test_clarify_countdown_uses_pending_expiry(self):
src = self._get_js().read_text()
assert '_clarifyCountdownTimer' in src
assert 'function _startClarifyCountdown' in src
assert 'expires_at' in src, \
'clarify countdown must use expires_at from the pending payload'
def test_hide_clarify_card_can_preserve_draft(self):
src = self._get_js().read_text()
assert 'function _stashClarifyDraft' in src
assert 'sessionStorage.setItem' in src
assert "$('msg')" in src, \
'clarify timeout should keep the typed draft visible in the composer'
def test_respond_clarify_calls_hide_with_force(self):
src = self._get_js().read_text()
import re
@@ -555,14 +575,15 @@ class TestClarifyCardTimerLogic:
src, re.DOTALL)
assert m, 'respondClarify function not found'
body = m.group(0)
assert 'hideClarifyCard(true)' in body, \
assert 'hideClarifyCard(true' in body, \
'respondClarify must call hideClarifyCard(true) so card hides immediately after user clicks'
assert "'sent'" in body, \
'respondClarify must mark user-submitted hides so drafts are not re-stashed'
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, \
assert "else { hideClarifyCard(false, 'expired'); }" in src or \
"else {hideClarifyCard(false,'expired');}" in src, \
'Clarify poll loop should hide without force=true'
def test_show_clarify_card_signature_dedup(self):