feat: add distinct attention request sound
Adds a distinct two-tone attention sound (880->660Hz) for approval and clarify prompts so they are not confused with the existing completion sound, plus sidebar attention badges + colored rails driven by `attention` metadata on /api/sessions. Includes lock-safety note: the in-lock publish_session_list_changed() calls in clarify.py are safe because publish() only takes the leaf _SESSION_EVENTS_LOCK and never re-acquires clarify._lock (verified by Opus advisor review). Co-authored-by: ai-ag2026 <261867348+ai-ag2026@users.noreply.github.com>
This commit is contained in:
parent
d2aa066862
commit
00e6d953c2
7 changed files with 271 additions and 3 deletions
|
|
@ -12,6 +12,8 @@ import time
|
|||
import uuid
|
||||
from typing import Optional
|
||||
|
||||
from api.session_events import publish_session_list_changed
|
||||
|
||||
|
||||
DEFAULT_TIMEOUT_SECONDS = 120
|
||||
_lock = threading.Lock()
|
||||
|
|
@ -52,6 +54,8 @@ def unregister_gateway_notify(session_key: str) -> None:
|
|||
with _lock:
|
||||
_gateway_notify_cbs.pop(session_key, None)
|
||||
entries = _clear_queue_locked(session_key)
|
||||
if entries:
|
||||
publish_session_list_changed("attention_cleared")
|
||||
for entry in entries:
|
||||
entry.event.set()
|
||||
|
||||
|
|
@ -60,6 +64,8 @@ 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)
|
||||
if entries:
|
||||
publish_session_list_changed("attention_cleared")
|
||||
for entry in entries:
|
||||
entry.event.set()
|
||||
return len(entries)
|
||||
|
|
@ -135,6 +141,9 @@ def submit_pending(session_key: str, data: dict) -> _ClarifyEntry:
|
|||
cb(dict(entry.data))
|
||||
except Exception:
|
||||
pass
|
||||
# Safe to call while holding _lock: publish() only takes the
|
||||
# leaf _SESSION_EVENTS_LOCK and never re-acquires this lock.
|
||||
publish_session_list_changed("attention_pending")
|
||||
return entry
|
||||
|
||||
entry = _ClarifyEntry(data)
|
||||
|
|
@ -145,6 +154,7 @@ def submit_pending(session_key: str, data: dict) -> _ClarifyEntry:
|
|||
cb = _gateway_notify_cbs.get(session_key)
|
||||
# Notify SSE subscribers from inside _lock for ordering guarantees.
|
||||
_clarify_sse_notify(session_key, dict(gw_queue[0].data), len(gw_queue))
|
||||
publish_session_list_changed("attention_pending")
|
||||
if cb:
|
||||
try:
|
||||
cb(data)
|
||||
|
|
@ -191,6 +201,7 @@ def resolve_clarify(session_key: str, response: str, resolve_all: bool = False)
|
|||
else:
|
||||
_clear_queue_locked(session_key)
|
||||
_clarify_sse_notify(session_key, None, 0)
|
||||
publish_session_list_changed("attention_resolved")
|
||||
count = 0
|
||||
for entry in entries:
|
||||
entry.result = response
|
||||
|
|
@ -218,6 +229,9 @@ def resolve_clarify_by_id(session_key: str, clarify_id: str, response: str) -> b
|
|||
else:
|
||||
_clear_queue_locked(session_key)
|
||||
_clarify_sse_notify(session_key, None, 0)
|
||||
# Safe to call while holding _lock: publish() only takes the
|
||||
# leaf _SESSION_EVENTS_LOCK and never re-acquires this lock.
|
||||
publish_session_list_changed("attention_resolved")
|
||||
entry.result = response
|
||||
entry.event.set()
|
||||
return True
|
||||
|
|
|
|||
|
|
@ -2924,6 +2924,7 @@ def submit_pending(session_key: str, approval: dict) -> None:
|
|||
# submit_pending calls can't deliver out-of-order (T2's later
|
||||
# notify arriving before T1's earlier notify with a stale count).
|
||||
_approval_sse_notify_locked(session_key, head, total)
|
||||
publish_session_list_changed("attention_pending")
|
||||
# NOTE: We do NOT call _submit_pending_raw here — that function overwrites
|
||||
# _pending[session_key] with a single dict, which would undo the list we just
|
||||
# built. The gateway blocking path uses _gateway_queues (a separate mechanism
|
||||
|
|
@ -11099,7 +11100,10 @@ def _resolve_approval_legacy(sid: str, approval_id: str, choice: str) -> bool:
|
|||
gateway_resolved = resolve_gateway_approval(sid, choice, resolve_all=False) or 0
|
||||
# Keep the historical no-id response path truthy for old clients/tests while
|
||||
# making stale explicit ids bounded as not-active for Slice 3b.
|
||||
return bool(pending) or bool(gateway_resolved) or not bool(approval_id)
|
||||
resolved = bool(pending) or bool(gateway_resolved) or not bool(approval_id)
|
||||
if resolved:
|
||||
publish_session_list_changed("attention_resolved")
|
||||
return resolved
|
||||
|
||||
|
||||
def _handle_approval_respond(handler, body):
|
||||
|
|
|
|||
|
|
@ -1706,14 +1706,14 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
|||
source.addEventListener('approval',e=>{
|
||||
const d=JSON.parse(e.data);
|
||||
showApprovalForSession(activeSid, d, 1);
|
||||
playNotificationSound();
|
||||
playAttentionSound(_attentionSoundKey(activeSid,'approval',1));
|
||||
sendBrowserNotification('Approval required',d.description||'Tool approval needed');
|
||||
});
|
||||
|
||||
source.addEventListener('clarify',e=>{
|
||||
const d=JSON.parse(e.data);
|
||||
showClarifyForSession(activeSid, d);
|
||||
playNotificationSound();
|
||||
playAttentionSound(_attentionSoundKey(activeSid,'clarify',1));
|
||||
sendBrowserNotification('Clarification needed',d.question||'Tool clarification needed');
|
||||
});
|
||||
|
||||
|
|
@ -3332,6 +3332,43 @@ function playNotificationSound(){
|
|||
}catch(e){console.warn('Notification sound failed:',e);}
|
||||
}
|
||||
|
||||
|
||||
function _attentionSoundKey(sid,kind,count){
|
||||
const safeSid=String(sid||'');
|
||||
const safeKind=String(kind||'attention');
|
||||
const safeCount=Math.max(1,Number(count)||1);
|
||||
return `${safeSid}:${safeKind}:${safeCount}`;
|
||||
}
|
||||
|
||||
function playAttentionSound(key){
|
||||
if(!window._soundEnabled) return;
|
||||
const nowMs=Date.now();
|
||||
if(window._lastAttentionSoundAt&&nowMs-window._lastAttentionSoundAt<900) return;
|
||||
const dedupeKey=key?String(key):'';
|
||||
if(dedupeKey){
|
||||
const seen=window._attentionSoundSeenKeys instanceof Map?window._attentionSoundSeenKeys:new Map();
|
||||
window._attentionSoundSeenKeys=seen;
|
||||
for(const [seenKey,seenAt] of seen){
|
||||
if(nowMs-Number(seenAt||0)>300000) seen.delete(seenKey);
|
||||
}
|
||||
if(seen.has(dedupeKey)) return;
|
||||
seen.set(dedupeKey,nowMs);
|
||||
}
|
||||
window._lastAttentionSoundAt=nowMs;
|
||||
try{
|
||||
const ctx=new (window.AudioContext||window.webkitAudioContext)();
|
||||
const osc=ctx.createOscillator();
|
||||
const gain=ctx.createGain();
|
||||
osc.connect(gain);gain.connect(ctx.destination);
|
||||
osc.type='sine';osc.frequency.setValueAtTime(880,ctx.currentTime);
|
||||
osc.frequency.setValueAtTime(660,ctx.currentTime+0.075);
|
||||
gain.gain.setValueAtTime(0.24,ctx.currentTime);
|
||||
gain.gain.exponentialRampToValueAtTime(0.01,ctx.currentTime+0.24);
|
||||
osc.start(ctx.currentTime);osc.stop(ctx.currentTime+0.24);
|
||||
osc.onended=()=>ctx.close();
|
||||
}catch(e){console.warn('Attention sound failed:',e);}
|
||||
}
|
||||
|
||||
function sendBrowserNotification(title,body){
|
||||
if(!window._notificationsEnabled||!document.hidden) return;
|
||||
if(!('Notification' in window)) return;
|
||||
|
|
|
|||
|
|
@ -1607,6 +1607,8 @@ async function _ensureAllMessagesLoaded() {
|
|||
}
|
||||
|
||||
let _allSessions = []; // cached for search filter
|
||||
let _sessionAttentionSoundPrimed = false;
|
||||
const _sessionAttentionSoundState = new Map();
|
||||
let _renamingSid = null; // session_id currently being renamed (blocks list re-renders)
|
||||
let _showArchived = false; // toggle to show archived sessions
|
||||
let _sessionSelectMode = false; // batch select mode
|
||||
|
|
@ -2361,6 +2363,42 @@ function _schedulePendingSessionListApply(){
|
|||
}, Math.max(120, SESSION_LIST_INTERACTION_IDLE_MS));
|
||||
}
|
||||
|
||||
|
||||
function _sessionAttentionSoundSignature(s){
|
||||
const attention=s&&s.attention&&typeof s.attention==='object'?s.attention:null;
|
||||
const count=Number(attention&&attention.count);
|
||||
if(!attention||!attention.kind||!Number.isFinite(count)||count<=0)return null;
|
||||
const kind=String(attention.kind)==='approval'?'approval':(String(attention.kind)==='clarify'?'clarify':'attention');
|
||||
return `${kind}:${Math.max(1,count||1)}`;
|
||||
}
|
||||
|
||||
function _syncSessionAttentionSoundState(sessions){
|
||||
const next=new Map();
|
||||
for(const s of Array.isArray(sessions)?sessions:[]){
|
||||
if(!s||!s.session_id)continue;
|
||||
const sig=_sessionAttentionSoundSignature(s);
|
||||
if(sig) next.set(s.session_id,sig);
|
||||
}
|
||||
if(!_sessionAttentionSoundPrimed){
|
||||
_sessionAttentionSoundPrimed=true;
|
||||
_sessionAttentionSoundState.clear();
|
||||
next.forEach((sig,sid)=>_sessionAttentionSoundState.set(sid,sig));
|
||||
return;
|
||||
}
|
||||
next.forEach((sig,sid)=>{
|
||||
const prev=_sessionAttentionSoundState.get(sid);
|
||||
if(prev!==sig){
|
||||
const [kind,countRaw]=String(sig).split(':');
|
||||
const count=Number(countRaw)||1;
|
||||
const s=(Array.isArray(sessions)?sessions:[]).find(item=>item&&item.session_id===sid)||{session_id:sid};
|
||||
const playKey=typeof _attentionSoundKey==='function'?_attentionSoundKey(s.session_id,kind,count):`${s.session_id}:${sig}`;
|
||||
if(playKey&&typeof playAttentionSound==='function') playAttentionSound(playKey);
|
||||
}
|
||||
});
|
||||
_sessionAttentionSoundState.clear();
|
||||
next.forEach((sig,sid)=>_sessionAttentionSoundState.set(sid,sig));
|
||||
}
|
||||
|
||||
function _applySessionListPayload(sessData, projData){
|
||||
// Server's other_profile_count tells us how many sessions exist outside the
|
||||
// active profile so the "Show N from other profiles" toggle can render
|
||||
|
|
@ -2381,6 +2419,7 @@ function _applySessionListPayload(sessData, projData){
|
|||
: (sessData.sessions||[]);
|
||||
_reconcileActiveSessionIdleStateFromList(serverSessions);
|
||||
_allSessions = _mergeOptimisticFirstTurnSessions(serverSessions);
|
||||
_syncSessionAttentionSoundState(_allSessions);
|
||||
_clearLineageReportCache();
|
||||
_allProjects = projData.projects||[];
|
||||
_markPollingCompletionUnreadTransitions(_allSessions);
|
||||
|
|
|
|||
|
|
@ -137,6 +137,31 @@ def test_done_handler_is_idempotent_for_replay_or_duplicate_done_events():
|
|||
)
|
||||
|
||||
|
||||
def test_attention_events_use_distinct_sound_from_completion():
|
||||
approval_body = _event_body("approval")
|
||||
clarify_body = _event_body("clarify")
|
||||
done_body = _event_body("done")
|
||||
attention_body = _function_body("playAttentionSound")
|
||||
|
||||
assert "playAttentionSound(_attentionSoundKey(activeSid,'approval',1));" in approval_body
|
||||
assert "playAttentionSound(_attentionSoundKey(activeSid,'clarify',1));" in clarify_body
|
||||
for body in (approval_body, clarify_body):
|
||||
assert "playNotificationSound();" not in body
|
||||
assert "playNotificationSound();" in done_body
|
||||
assert "playAttentionSound();" not in done_body
|
||||
assert "osc.type='sine'" in attention_body
|
||||
assert "window._lastAttentionSoundAt" in attention_body
|
||||
assert "nowMs-window._lastAttentionSoundAt<900" in attention_body
|
||||
assert "window._attentionSoundSeenKeys" in attention_body
|
||||
assert "seen.has(dedupeKey)" in attention_body
|
||||
assert "seen.set(dedupeKey,nowMs)" in attention_body
|
||||
assert "osc.frequency.setValueAtTime(880,ctx.currentTime);" in attention_body
|
||||
assert "osc.frequency.setValueAtTime(660,ctx.currentTime+0.075);" in attention_body
|
||||
assert "gain.gain.setValueAtTime(0.24,ctx.currentTime);" in attention_body
|
||||
assert "osc.stop(ctx.currentTime+0.24);" in attention_body
|
||||
assert "Notification sound failed" not in attention_body
|
||||
|
||||
|
||||
def test_attach_live_stream_registers_one_source_per_session_stream():
|
||||
"""Reconnect/compaction paths must not stack same-stream EventSources.
|
||||
|
||||
|
|
|
|||
68
tests/test_attention_session_events.py
Normal file
68
tests/test_attention_session_events.py
Normal file
|
|
@ -0,0 +1,68 @@
|
|||
"""Attention badges must refresh the session sidebar immediately.
|
||||
|
||||
Approval/clarify queues already notify the per-session card SSE stream. The
|
||||
sidebar, however, listens to the global session-events stream and then reloads
|
||||
``/api/sessions``. Without publishing on queue transitions, the badge and
|
||||
attention sound only appear after the user opens the affected thread.
|
||||
"""
|
||||
|
||||
|
||||
def test_clarify_pending_and_resolved_publish_session_list_changed(monkeypatch):
|
||||
import api.clarify as clarify
|
||||
|
||||
events = []
|
||||
monkeypatch.setattr(
|
||||
clarify,
|
||||
"publish_session_list_changed",
|
||||
lambda reason: events.append(reason),
|
||||
)
|
||||
sid = "test-attention-clarify"
|
||||
|
||||
with clarify._lock:
|
||||
clarify._pending.pop(sid, None)
|
||||
clarify._gateway_queues.pop(sid, None)
|
||||
clarify._gateway_notify_cbs.pop(sid, None)
|
||||
|
||||
try:
|
||||
entry = clarify.submit_pending(sid, {"question": "Need input?"})
|
||||
assert events == ["attention_pending"]
|
||||
pending = clarify.get_pending(sid)
|
||||
assert pending is not None
|
||||
assert pending["clarify_id"] == entry.clarify_id
|
||||
|
||||
assert clarify.resolve_clarify_by_id(sid, entry.clarify_id, "yes") is True
|
||||
assert events == ["attention_pending", "attention_resolved"]
|
||||
assert clarify.get_pending(sid) is None
|
||||
finally:
|
||||
with clarify._lock:
|
||||
clarify._pending.pop(sid, None)
|
||||
clarify._gateway_queues.pop(sid, None)
|
||||
clarify._gateway_notify_cbs.pop(sid, None)
|
||||
|
||||
|
||||
def test_approval_pending_and_resolved_publish_session_list_changed(monkeypatch):
|
||||
import api.routes as routes
|
||||
|
||||
events = []
|
||||
monkeypatch.setattr(
|
||||
routes,
|
||||
"publish_session_list_changed",
|
||||
lambda reason: events.append(reason),
|
||||
)
|
||||
sid = "test-attention-approval"
|
||||
|
||||
with routes._lock:
|
||||
routes._pending.pop(sid, None)
|
||||
routes._gateway_queues.pop(sid, None)
|
||||
|
||||
try:
|
||||
routes.submit_pending(sid, {"command": "echo ok", "pattern_key": "test-key"})
|
||||
assert events == ["attention_pending"]
|
||||
|
||||
ok = routes._resolve_approval_legacy(sid, "", "deny")
|
||||
assert ok is True
|
||||
assert events == ["attention_pending", "attention_resolved"]
|
||||
finally:
|
||||
with routes._lock:
|
||||
routes._pending.pop(sid, None)
|
||||
routes._gateway_queues.pop(sid, None)
|
||||
81
tests/test_session_attention_sound.py
Normal file
81
tests/test_session_attention_sound.py
Normal file
|
|
@ -0,0 +1,81 @@
|
|||
"""Regression tests for attention sounds on session attention state.
|
||||
|
||||
Approval/clarify prompts can surface through the sidebar session metadata rather
|
||||
than the active live SSE stream. The sidebar badge path must play the distinct
|
||||
attention sound when a session newly needs user input, without blasting sounds
|
||||
for already-existing badges on initial load.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
REPO = Path(__file__).parent.parent
|
||||
SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _body_from_brace(src: str, brace: int, label: str) -> str:
|
||||
assert brace >= 0, f"body opening brace not found for: {label}"
|
||||
depth = 1
|
||||
i = brace + 1
|
||||
while i < len(src) and depth:
|
||||
ch = src[i]
|
||||
if ch == "{":
|
||||
depth += 1
|
||||
elif ch == "}":
|
||||
depth -= 1
|
||||
i += 1
|
||||
assert depth == 0, f"body did not close for: {label}"
|
||||
return src[brace + 1 : i - 1]
|
||||
|
||||
|
||||
def _function_body(src: str, name: str) -> str:
|
||||
marker = f"function {name}("
|
||||
start = src.find(marker)
|
||||
assert start >= 0, f"function not found: {name}"
|
||||
signature_end = src.find("){", start)
|
||||
assert signature_end >= 0, f"function body not found: {name}"
|
||||
return _body_from_brace(src, signature_end + 1, name)
|
||||
|
||||
|
||||
def test_sidebar_attention_state_plays_distinct_sound_on_new_attention_only():
|
||||
sync_body = _function_body(SESSIONS_JS, "_syncSessionAttentionSoundState")
|
||||
apply_body = _function_body(SESSIONS_JS, "_applySessionListPayload")
|
||||
|
||||
assert "let _sessionAttentionSoundPrimed = false;" in SESSIONS_JS
|
||||
assert "const _sessionAttentionSoundState = new Map();" in SESSIONS_JS
|
||||
assert "_syncSessionAttentionSoundState(_allSessions);" in apply_body
|
||||
assert "if(!_sessionAttentionSoundPrimed)" in sync_body
|
||||
assert "_sessionAttentionSoundPrimed=true;" in sync_body
|
||||
assert "playKey=typeof _attentionSoundKey==='function'?_attentionSoundKey(s.session_id,kind,count):`${s.session_id}:${sig}`;" in sync_body
|
||||
assert "if(playKey&&typeof playAttentionSound==='function') playAttentionSound(playKey);" in sync_body
|
||||
assert "playNotificationSound" not in sync_body
|
||||
|
||||
|
||||
def test_attention_signature_tracks_kind_and_count_for_badge_changes():
|
||||
signature_body = _function_body(SESSIONS_JS, "_sessionAttentionSoundSignature")
|
||||
|
||||
assert "attention.kind" in signature_body
|
||||
assert "Number.isFinite(count)" in signature_body
|
||||
assert "count<=0" in signature_body
|
||||
assert "approval" in signature_body
|
||||
assert "clarify" in signature_body
|
||||
assert "return `${kind}:${Math.max(1,count||1)}`;" in signature_body
|
||||
|
||||
|
||||
def test_attention_sound_is_softer_short_reverse_of_completion_sound():
|
||||
attention_body = _function_body(MESSAGES_JS, "playAttentionSound")
|
||||
completion_body = _function_body(MESSAGES_JS, "playNotificationSound")
|
||||
|
||||
assert "osc.type='sine'" in attention_body
|
||||
assert "window._lastAttentionSoundAt" in attention_body
|
||||
assert "nowMs-window._lastAttentionSoundAt<900" in attention_body
|
||||
assert "window._attentionSoundSeenKeys" in attention_body
|
||||
assert "seen.has(dedupeKey)" in attention_body
|
||||
assert "seen.set(dedupeKey,nowMs)" in attention_body
|
||||
assert "300000" in attention_body
|
||||
assert "osc.frequency.setValueAtTime(880,ctx.currentTime);" in attention_body
|
||||
assert "osc.frequency.setValueAtTime(660,ctx.currentTime+0.075);" in attention_body
|
||||
assert "gain.gain.setValueAtTime(0.24,ctx.currentTime);" in attention_body
|
||||
assert "osc.stop(ctx.currentTime+0.24);" in attention_body
|
||||
assert "osc.frequency.setValueAtTime(660,ctx.currentTime);" in completion_body
|
||||
assert "osc.frequency.setValueAtTime(880,ctx.currentTime+0.1);" in completion_body
|
||||
assert "osc.stop(ctx.currentTime+0.3);" in completion_body
|
||||
Loading…
Reference in a new issue