fix(#6008): keep retained approvals visible in session attention
This commit is contained in:
1 parent
5826d76da0
commit
b481dfea13
3 files changed
+138
-41
No files matched your search
+1
-29
@@ -9747,40 +9747,12 @@ except ImportError:
|
||||
|
||||
def _session_attention_summary(session_id: str) -> dict | None:
|
||||
"""Return sidebar attention metadata for pending approval/clarify work."""
|
||||
active_run_id = ""
|
||||
try:
|
||||
from api.gateway_chat import _STREAM_RUN_IDS
|
||||
|
||||
session = get_session(session_id)
|
||||
active_stream_id = getattr(session, "active_stream_id", None) if session is not None else None
|
||||
if active_stream_id:
|
||||
active_run_id = str(_STREAM_RUN_IDS.get(active_stream_id) or "").strip()
|
||||
except Exception:
|
||||
active_run_id = ""
|
||||
|
||||
approval_count = 0
|
||||
with _lock:
|
||||
reconcile_gateway_pending_mirror_locked(session_id)
|
||||
queue_list = _pending.get(session_id)
|
||||
if isinstance(queue_list, list):
|
||||
gateway_queue = _gateway_queues.get(session_id) or []
|
||||
live_gateway_run_ids = {
|
||||
str((getattr(entry, "data", None) or {}).get("run_id") or "").strip()
|
||||
for entry in gateway_queue
|
||||
if str((getattr(entry, "data", None) or {}).get("run_id") or "").strip()
|
||||
}
|
||||
approval_count = sum(
|
||||
1
|
||||
for entry in queue_list
|
||||
if not (
|
||||
isinstance(entry, dict)
|
||||
and entry.get(_GATEWAY_MIRROR_FLAG)
|
||||
and str(entry.get("run_id") or "").strip()
|
||||
and not str(entry.get(_GATEWAY_MIRROR_TOKEN) or "").strip()
|
||||
and str(entry.get("run_id") or "").strip() not in live_gateway_run_ids
|
||||
and str(entry.get("run_id") or "").strip() != active_run_id
|
||||
)
|
||||
)
|
||||
approval_count = len(queue_list)
|
||||
elif queue_list:
|
||||
approval_count = 1
|
||||
if approval_count > 0:
|
||||
|
||||
@@ -421,6 +421,88 @@ def test_gateway_runs_api_streaming_parses_real_run_events():
|
||||
assert approvals.gateway_pending_mirror("sess1", run_id="run-abc") is None
|
||||
|
||||
|
||||
def test_gateway_runs_api_streaming_same_run_fifo_emits_head_and_promotes_successor():
|
||||
"""Runs API approval events publish the reconciled FIFO head and count."""
|
||||
from api.config import STREAM_PARTIAL_TEXT, STREAM_REASONING_TEXT
|
||||
from api.gateway_chat import _STREAM_RUN_IDS, _run_gateway_runs_api_streaming
|
||||
from api import routes
|
||||
import api.route_approvals as approvals
|
||||
|
||||
sid = "sess-same-run-producer"
|
||||
stream_id = "stream-same-run-producer"
|
||||
events = []
|
||||
STREAM_PARTIAL_TEXT[stream_id] = ""
|
||||
STREAM_REASONING_TEXT[stream_id] = ""
|
||||
|
||||
class _JsonResponse:
|
||||
def read(self, _limit=None):
|
||||
return b'{"run_id":"run-fifo"}'
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *args):
|
||||
return None
|
||||
|
||||
class _SseResponse:
|
||||
def __iter__(self):
|
||||
return iter([
|
||||
b'data: {"event":"approval.request","command":"first","description":"First","run_id":"run-fifo","approval_id":"approval-first"}\n',
|
||||
b'\n',
|
||||
b'data: {"event":"approval.request","command":"second","description":"Second","run_id":"run-fifo","approval_id":"approval-second"}\n',
|
||||
b'\n',
|
||||
b'data: [DONE]\n',
|
||||
b'\n',
|
||||
])
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *args):
|
||||
return None
|
||||
|
||||
def fake_urlopen(req, *, timeout=None):
|
||||
return _JsonResponse() if req.full_url.endswith("/v1/runs") else _SseResponse()
|
||||
|
||||
try:
|
||||
with patch("urllib.request.urlopen", side_effect=fake_urlopen):
|
||||
_run_gateway_runs_api_streaming(
|
||||
sid, "hi", "test", "/tmp", stream_id, "http://gw:8642", "", [], {},
|
||||
put_gateway_event=lambda event, data: events.append((event, data)),
|
||||
cancel_event=threading.Event(),
|
||||
)
|
||||
|
||||
approval_events = [data for event, data in events if event == "approval"]
|
||||
assert [(data["approval_id"], data["pending_count"]) for data in approval_events] == [
|
||||
("approval-first", 1),
|
||||
("approval-first", 2),
|
||||
]
|
||||
|
||||
handler = MagicMock()
|
||||
handler.wfile = io.BytesIO()
|
||||
with patch("api.routes.get_session", return_value=SimpleNamespace(active_stream_id=stream_id)), \
|
||||
patch("api.runner_client.HttpRunnerClient.respond_approval") as respond_approval:
|
||||
routes._handle_approval_respond(handler, {
|
||||
"session_id": sid,
|
||||
"choice": "deny",
|
||||
"approval_id": "approval-first",
|
||||
})
|
||||
|
||||
handler.send_response.assert_called_with(200)
|
||||
respond_approval.assert_called_once_with("run-fifo", "approval-first", "deny")
|
||||
promoted = approvals.gateway_pending_mirror(sid, run_id="run-fifo")
|
||||
assert promoted is not None
|
||||
assert promoted["approval_id"] == "approval-second"
|
||||
with approvals._lock:
|
||||
assert len(approvals._pending[sid]) == 1
|
||||
finally:
|
||||
STREAM_PARTIAL_TEXT.pop(stream_id, None)
|
||||
STREAM_REASONING_TEXT.pop(stream_id, None)
|
||||
_STREAM_RUN_IDS.pop(stream_id, None)
|
||||
approvals._pending.pop(sid, None)
|
||||
approvals._gateway_queues.pop(sid, None)
|
||||
|
||||
|
||||
def test_empty_id_runs_approval_reaches_real_response_lifecycle():
|
||||
"""Runs API empty IDs are carried and resolved through the live mirror."""
|
||||
from api.gateway_chat import _run_gateway_runs_api_streaming
|
||||
|
||||
@@ -115,22 +115,65 @@ def test_attention_summary_prefers_pending_approvals_over_clarify_questions():
|
||||
_clear_attention_state(sid)
|
||||
|
||||
|
||||
def test_attention_summary_ignores_stale_gateway_mirror():
|
||||
sid = "attention-stale-gateway-session"
|
||||
def test_attention_summary_keeps_direct_runs_mirror_after_stream_pointer_clears():
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import api.route_approvals as approvals
|
||||
|
||||
sid = "attention-retained-gateway-session"
|
||||
_clear_attention_state(sid)
|
||||
try:
|
||||
with routes._lock:
|
||||
routes._pending[sid] = [{
|
||||
"_gateway_mirror": True,
|
||||
"approval_id": "appr-stale-gateway",
|
||||
"run_id": "run-stale-gateway",
|
||||
"command": "rm -rf /tmp/stale",
|
||||
"description": "Stale approval",
|
||||
}]
|
||||
routes._gateway_queues[sid] = []
|
||||
approvals.submit_gateway_pending_mirror(sid, {
|
||||
"_gateway_mirror": True,
|
||||
"approval_id": "approval-retained",
|
||||
"run_id": "run-retained",
|
||||
"command": "rm -rf /tmp/retained",
|
||||
"description": "Retained approval",
|
||||
})
|
||||
handler = MagicMock()
|
||||
handler.wfile = io.BytesIO()
|
||||
|
||||
assert routes._session_attention_summary(sid) is None
|
||||
with patch("api.routes.get_session", return_value=SimpleNamespace(active_stream_id=None)), \
|
||||
patch("api.runner_client.HttpRunnerClient.respond_approval") as respond_approval:
|
||||
assert routes._session_attention_summary(sid) == {
|
||||
"kind": "approval",
|
||||
"count": 1,
|
||||
"severity": "critical",
|
||||
}
|
||||
routes._handle_approval_respond(handler, {
|
||||
"session_id": sid,
|
||||
"choice": "deny",
|
||||
"approval_id": "approval-retained",
|
||||
})
|
||||
|
||||
handler.send_response.assert_called_with(200)
|
||||
respond_approval.assert_called_once_with("run-retained", "approval-retained", "deny")
|
||||
assert json.loads(handler.wfile.getvalue().decode("utf-8"))["ok"] is True
|
||||
finally:
|
||||
approvals._pending.pop(sid, None)
|
||||
_clear_attention_state(sid)
|
||||
|
||||
|
||||
def test_attention_summary_hides_retired_direct_runs_mirror():
|
||||
import api.route_approvals as approvals
|
||||
|
||||
sid = "attention-retired-gateway-session"
|
||||
_clear_attention_state(sid)
|
||||
try:
|
||||
approvals.submit_gateway_pending_mirror(sid, {
|
||||
"_gateway_mirror": True,
|
||||
"approval_id": "approval-retired",
|
||||
"run_id": "run-retired",
|
||||
"command": "rm -rf /tmp/retired",
|
||||
"description": "Retired approval",
|
||||
})
|
||||
|
||||
assert approvals.retire_gateway_pending_mirror(sid, run_id="run-retired") is True
|
||||
assert routes._session_attention_summary(sid) is None
|
||||
assert approvals.gateway_pending_mirror(sid, run_id="run-retired") is None
|
||||
finally:
|
||||
approvals._pending.pop(sid, None)
|
||||
_clear_attention_state(sid)
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user