fix: reject future terminal turn origins
This commit is contained in:
1 parent
de96c1d384
commit
9bde2cf3af
3 files changed
+131
-2
No files matched your search
+4
-1
@@ -6512,7 +6512,10 @@ def _terminal_turn_duration(session, *, now: float | None = None) -> float | Non
|
||||
return None
|
||||
if not math.isfinite(started) or not math.isfinite(ended) or started <= 0:
|
||||
return None
|
||||
return round(max(0.0, ended - started), 3)
|
||||
elapsed = ended - started
|
||||
if elapsed < 0:
|
||||
return None
|
||||
return round(elapsed, 3)
|
||||
|
||||
|
||||
def _build_partial_message(content_text, reasoning_text, tool_calls) -> dict | None:
|
||||
|
||||
@@ -15,7 +15,7 @@ import api.streaming as streaming
|
||||
from api.models import Session
|
||||
|
||||
|
||||
@pytest.mark.parametrize("started_at", [None, 0, -1, "invalid", float("nan"), float("inf")])
|
||||
@pytest.mark.parametrize("started_at", [None, 0, -1, "invalid", float("nan"), float("inf"), 101.0])
|
||||
def test_terminal_turn_duration_omits_invalid_origin(started_at):
|
||||
session = types.SimpleNamespace(pending_started_at=started_at)
|
||||
|
||||
@@ -179,6 +179,57 @@ def test_silent_failure_preserves_partials(tmp_path):
|
||||
assert err_msg.get("_turnDuration", 0) >= 12
|
||||
|
||||
|
||||
@pytest.mark.parametrize("started_at", [None, "future"])
|
||||
def test_result_error_omits_turn_duration_for_missing_or_future_origin(tmp_path, started_at):
|
||||
"""Result-error writeback must not manufacture durations from invalid timer origins."""
|
||||
label = "future" if started_at == "future" else "missing"
|
||||
fake_session = Session(session_id=f"test_sess_silent_{label}", title="Test Session")
|
||||
fake_session.pending_user_message = "What is python?"
|
||||
fake_session.pending_started_at = time.time() + 30 if started_at == "future" else started_at
|
||||
fake_session.active_stream_id = f"test_stream_silent_{label}"
|
||||
fake_session.save()
|
||||
models.SESSIONS[fake_session.session_id] = fake_session
|
||||
|
||||
class SilentFailureAgent(MockAgent):
|
||||
def run_conversation(self, **kwargs):
|
||||
if self.stream_delta_callback:
|
||||
self.stream_delta_callback("Python is a programming language.")
|
||||
return {
|
||||
"status": "error",
|
||||
"error": "Silent failure details",
|
||||
"messages": kwargs.get("conversation_history") or [],
|
||||
}
|
||||
|
||||
fake_queue = queue.Queue()
|
||||
streaming.STREAMS[fake_session.active_stream_id] = fake_queue
|
||||
config.STREAM_PARTIAL_TEXT[fake_session.active_stream_id] = ""
|
||||
|
||||
with mock.patch.object(streaming, "get_session", return_value=fake_session), \
|
||||
mock.patch.object(streaming, "_get_ai_agent", return_value=SilentFailureAgent), \
|
||||
mock.patch.object(streaming, "resolve_model_provider", return_value=("test-model", "test-provider", None)), \
|
||||
mock.patch("api.config.get_config", return_value={}), \
|
||||
mock.patch("api.config._resolve_cli_toolsets", return_value=[]):
|
||||
|
||||
streaming._run_agent_streaming(
|
||||
session_id=fake_session.session_id,
|
||||
msg_text="What is python?",
|
||||
model="test-model",
|
||||
workspace=str(tmp_path),
|
||||
stream_id=fake_session.active_stream_id,
|
||||
)
|
||||
|
||||
saved = Session.load(fake_session.session_id)
|
||||
assert saved is not None
|
||||
assert saved.messages[-1].get("_error") is True
|
||||
assert "_turnDuration" not in saved.messages[-1]
|
||||
|
||||
apperrors = [item[1] for item in list(fake_queue.queue) if item[0] == "apperror"]
|
||||
assert apperrors
|
||||
payload_error = apperrors[-1]["session"]["messages"][-1]
|
||||
assert payload_error.get("_error") is True
|
||||
assert "_turnDuration" not in payload_error
|
||||
|
||||
|
||||
def test_exception_preserves_partials(tmp_path):
|
||||
"""Test that an unhandled exception preserves partial streamed text."""
|
||||
fake_session = Session(session_id="test_sess_exc", title="Test Session")
|
||||
@@ -225,6 +276,47 @@ def test_exception_preserves_partials(tmp_path):
|
||||
assert err_msg.get("_turnDuration", 0) >= 9
|
||||
|
||||
|
||||
@pytest.mark.parametrize("started_at", [None, "future"])
|
||||
def test_exception_error_omits_turn_duration_for_missing_or_future_origin(tmp_path, started_at):
|
||||
"""Exception writeback must not persist durations from invalid timer origins."""
|
||||
label = "future" if started_at == "future" else "missing"
|
||||
fake_session = Session(session_id=f"test_sess_exc_{label}", title="Test Session")
|
||||
fake_session.pending_user_message = "Exception test"
|
||||
fake_session.pending_started_at = time.time() + 30 if started_at == "future" else started_at
|
||||
fake_session.active_stream_id = f"test_stream_exc_{label}"
|
||||
fake_session.save()
|
||||
models.SESSIONS[fake_session.session_id] = fake_session
|
||||
|
||||
class ExceptionAgent(MockAgent):
|
||||
def run_conversation(self, **kwargs):
|
||||
if self.stream_delta_callback:
|
||||
self.stream_delta_callback("Stream before crash.")
|
||||
raise RuntimeError("Fake provider crash!")
|
||||
|
||||
fake_queue = queue.Queue()
|
||||
streaming.STREAMS[fake_session.active_stream_id] = fake_queue
|
||||
config.STREAM_PARTIAL_TEXT[fake_session.active_stream_id] = ""
|
||||
|
||||
with mock.patch.object(streaming, "get_session", return_value=fake_session), \
|
||||
mock.patch.object(streaming, "_get_ai_agent", return_value=ExceptionAgent), \
|
||||
mock.patch.object(streaming, "resolve_model_provider", return_value=("test-model", "test-provider", None)), \
|
||||
mock.patch("api.config.get_config", return_value={}), \
|
||||
mock.patch("api.config._resolve_cli_toolsets", return_value=[]):
|
||||
|
||||
streaming._run_agent_streaming(
|
||||
session_id=fake_session.session_id,
|
||||
msg_text="Exception test",
|
||||
model="test-model",
|
||||
workspace=str(tmp_path),
|
||||
stream_id=fake_session.active_stream_id,
|
||||
)
|
||||
|
||||
saved = Session.load(fake_session.session_id)
|
||||
assert saved is not None
|
||||
assert saved.messages[-1].get("_error") is True
|
||||
assert "_turnDuration" not in saved.messages[-1]
|
||||
|
||||
|
||||
def test_empty_partials_do_not_create_spurious_messages(tmp_path):
|
||||
"""Test that if no content has been streamed, no _partial message is created on error."""
|
||||
fake_session = Session(session_id="test_sess_empty", title="Test Session")
|
||||
|
||||
@@ -4,6 +4,7 @@ from email.message import Message
|
||||
import json
|
||||
from pathlib import Path
|
||||
import re
|
||||
import time
|
||||
import urllib.error
|
||||
|
||||
import api.gateway_chat as gateway_chat
|
||||
@@ -519,6 +520,38 @@ def test_gateway_chat_worker_classifies_terminal_provider_error_without_text(tmp
|
||||
unknown_errors = [item[1] for item in unknown_events if item[0] == "apperror"]
|
||||
assert unknown_errors[-1]["type"] == "error"
|
||||
assert "Gateway provider failed" in unknown_errors[-1]["message"]
|
||||
unknown_payload_error = unknown_errors[-1]["session"]["messages"][-1]
|
||||
assert unknown_payload_error.get("_error") is True
|
||||
assert "_turnDuration" not in unknown_payload_error
|
||||
|
||||
response_error[0] = error_text
|
||||
future_stream_id = "stream-gateway-future-duration-terminal-error-test"
|
||||
s = new_session()
|
||||
s.active_stream_id = future_stream_id
|
||||
s.pending_user_message = "Say hello"
|
||||
s.pending_started_at = time.time() + 30
|
||||
s.pending_attachments = []
|
||||
s.save()
|
||||
future_events = []
|
||||
future_channel = MagicMock()
|
||||
future_channel.put_nowait = lambda item: future_events.append(item)
|
||||
STREAMS[future_stream_id] = future_channel
|
||||
gateway_chat._run_gateway_chat_streaming(
|
||||
s.session_id,
|
||||
"Say hello",
|
||||
"test-model",
|
||||
str(tmp_path),
|
||||
future_stream_id,
|
||||
[],
|
||||
)
|
||||
future_errors = [item[1] for item in future_events if item[0] == "apperror"]
|
||||
assert future_errors[-1]["type"] in {"model_not_found", "auth_mismatch"}
|
||||
saved = models.get_session(s.session_id)
|
||||
assert saved.messages[-1].get("_error") is True
|
||||
assert "_turnDuration" not in saved.messages[-1]
|
||||
future_payload_error = future_errors[-1]["session"]["messages"][-1]
|
||||
assert future_payload_error.get("_error") is True
|
||||
assert "_turnDuration" not in future_payload_error
|
||||
|
||||
response_error[0] = "partial"
|
||||
partial_stream_id = "stream-gateway-partial-terminal-error-test"
|
||||
@@ -553,6 +586,7 @@ def test_gateway_chat_worker_classifies_terminal_provider_error_without_text(tmp
|
||||
assert payload_messages[-2]["_partial"] is True
|
||||
assert payload_messages[-2]["content"] == "partial"
|
||||
assert payload_messages[-1]["_error"] is True
|
||||
assert "_turnDuration" not in payload_messages[-1]
|
||||
|
||||
|
||||
def test_gateway_chat_worker_persists_reasoning_and_tool_state_on_terminal_error(tmp_path, monkeypatch):
|
||||
|
||||
Reference in new issue
Block a user