From 00e6d953c2c51d7f99e65040854f6e8a925bc35e Mon Sep 17 00:00:00 2001 From: ai-ag2026 <261867348+ai-ag2026@users.noreply.github.com> Date: Sat, 30 May 2026 18:10:14 +0200 Subject: [PATCH] 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> --- api/clarify.py | 14 ++++ api/routes.py | 6 +- static/messages.js | 41 +++++++++- static/sessions.js | 39 +++++++++ tests/test_1694_terminal_cleanup_ownership.py | 25 ++++++ tests/test_attention_session_events.py | 68 ++++++++++++++++ tests/test_session_attention_sound.py | 81 +++++++++++++++++++ 7 files changed, 271 insertions(+), 3 deletions(-) create mode 100644 tests/test_attention_session_events.py create mode 100644 tests/test_session_attention_sound.py diff --git a/api/clarify.py b/api/clarify.py index f6d16bbb..f43d40dd 100644 --- a/api/clarify.py +++ b/api/clarify.py @@ -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 diff --git a/api/routes.py b/api/routes.py index fd219647..66d237f7 100644 --- a/api/routes.py +++ b/api/routes.py @@ -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): diff --git a/static/messages.js b/static/messages.js index d9209cce..01f7d9ef 100644 --- a/static/messages.js +++ b/static/messages.js @@ -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; diff --git a/static/sessions.js b/static/sessions.js index e4a63e42..a2aaf017 100644 --- a/static/sessions.js +++ b/static/sessions.js @@ -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); diff --git a/tests/test_1694_terminal_cleanup_ownership.py b/tests/test_1694_terminal_cleanup_ownership.py index 1390d9fc..2993c473 100644 --- a/tests/test_1694_terminal_cleanup_ownership.py +++ b/tests/test_1694_terminal_cleanup_ownership.py @@ -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. diff --git a/tests/test_attention_session_events.py b/tests/test_attention_session_events.py new file mode 100644 index 00000000..66dfed4f --- /dev/null +++ b/tests/test_attention_session_events.py @@ -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) diff --git a/tests/test_session_attention_sound.py b/tests/test_session_attention_sound.py new file mode 100644 index 00000000..9f595520 --- /dev/null +++ b/tests/test_session_attention_sound.py @@ -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