From 5c4c31cf4d88b084061a5a25ade1b47ab5b751e1 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sun, 13 Sep 2026 19:10:50 +0530 Subject: [PATCH] refactor(agent): fail loudly on a missing turn clock; single lowercase in is_dangerous_confirmation - build_api_messages reads agent._current_turn_timestamp directly: a caller that skipped the turn prologue now raises instead of silently falling back to per-request wall time, which would re-create the mid-turn drift the fix removes. Only production caller (assemble_api_request) runs after _reset_per_turn_agent_state; cross-reference to the tripwire _inflight_turn_started so the two clocks are not "unified" by mistake. - is_dangerous_confirmation lowercases once instead of once per pattern (now on the per-request path). - Tests: one _send(idx=) helper instead of three spellings of the builder call; the untrustworthy-stamp contract is its own test. --- agent/replay_cleanup.py | 5 ++++- agent/turn_context.py | 9 ++++++--- tests/agent/test_replay_cleanup.py | 25 ++++++++++++------------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/agent/replay_cleanup.py b/agent/replay_cleanup.py index ab00f4526a..67a1efd7ec 100644 --- a/agent/replay_cleanup.py +++ b/agent/replay_cleanup.py @@ -196,7 +196,10 @@ _EXPIRED_CONFIRMATION_SENTINEL = ( def is_dangerous_confirmation(content: Any) -> bool: """True if user-message text contains a known dangerous confirmation phrase.""" - return isinstance(content, str) and any(pattern in content.strip().lower() for pattern in _DANGEROUS_CONFIRMATION_PATTERNS) + if not isinstance(content, str): + return False + lowered = content.strip().lower() + return any(pattern in lowered for pattern in _DANGEROUS_CONFIRMATION_PATTERNS) def strip_stale_dangerous_confirmations( diff --git a/agent/turn_context.py b/agent/turn_context.py index 4365ed71b3..27fe703a9b 100644 --- a/agent/turn_context.py +++ b/agent/turn_context.py @@ -510,7 +510,8 @@ def _reset_per_turn_agent_state(agent: Any) -> None: _reset_consol() # Expiry clock for build_api_messages: admission time (not the input's platform-event # stamp, which can predate admission by minutes), frozen so every request this turn - # sends identical bytes. + # sends identical bytes. Distinct from note_turn_start's _inflight_turn_started, a + # tripwire slot cleared at persist. agent._current_turn_timestamp = time.time() # Pre-turn connection health check: clean up dead TCP connections. @@ -1062,8 +1063,10 @@ def build_api_messages( # calls/results) are live and must never be rewritten between iterations. The # expiry clock is the turn's admission time, frozen in _reset_per_turn_agent_state. # Without an anchor (compaction found no surviving user row) there is no provable - # persisted prefix, so nothing is canonicalized. - turn_now = getattr(agent, "_current_turn_timestamp", None) or time.time() + # persisted prefix, so nothing is canonicalized. The clock is stamped once per turn in + # _reset_per_turn_agent_state; a caller that skipped the prologue fails loudly here + # rather than silently un-freezing it. + turn_now = agent._current_turn_timestamp split = current_turn_user_idx if has_current else 0 canonical_messages = canonicalize_replay_history(messages[:split], now=turn_now) + messages[split:] diff --git a/tests/agent/test_replay_cleanup.py b/tests/agent/test_replay_cleanup.py index d14226730c..f97acaff9e 100644 --- a/tests/agent/test_replay_cleanup.py +++ b/tests/agent/test_replay_cleanup.py @@ -125,10 +125,10 @@ def _wire(messages): return json.dumps(ChatCompletionsTransport().convert_messages(list(messages)), sort_keys=True) -def _send(agent, history): +def _send(agent, history, idx=None): request, _ = build_api_messages( - agent, history, current_turn_user_idx=len(history) - 1, ext_prefetch_cache="", - plugin_user_context="", moa_config=None, active_system_prompt="", + agent, history, current_turn_user_idx=len(history) - 1 if idx is None else idx, + ext_prefetch_cache="", plugin_user_context="", moa_config=None, active_system_prompt="", ) return request @@ -177,17 +177,14 @@ def test_send_wire_matches_replay_wire_after_db_round_trip(tmp_path): {"id": "c4", "type": "function", "function": {"name": "search_files", "arguments": "{}"}}]}, {"role": "tool", "tool_call_id": "c4", "content": "[Command interrupted]"}, ] - request2 = build_api_messages( - _SendAgent(), live, current_turn_user_idx=len(persisted), ext_prefetch_cache="", - plugin_user_context="", moa_config=None, active_system_prompt="", - )[0] + request2 = _send(_SendAgent(), live, idx=len(persisted)) assert _wire(request2[: len(request)]) == _wire(request) assert request2[-1]["content"] == "[Command interrupted]" -def test_confirmation_expiry_uses_frozen_admission_clock_and_fails_closed(monkeypatch): +def test_confirmation_expiry_uses_frozen_admission_clock(monkeypatch): """Expiry is judged once per turn at admission (not the input's event stamp, not - per-request wall time); a present-but-corrupt stamp is treated as expired.""" + per-request wall time).""" from agent.turn_context import _reset_per_turn_agent_state agent = _SendAgent() @@ -211,11 +208,13 @@ def test_confirmation_expiry_uses_frozen_admission_clock_and_fails_closed(monkey {"id": "c1", "type": "function", "function": {"name": "read_file", "arguments": "{}"}}]}, {"role": "tool", "tool_call_id": "c1", "content": "ready"}] monkeypatch.setattr("agent.turn_context.time.time", lambda: 10_500.0) # ...however long the tools take - late = build_api_messages(agent, history, current_turn_user_idx=2, ext_prefetch_cache="", - plugin_user_context="", moa_config=None, active_system_prompt="")[0] - assert late[0]["content"] == "confirm reboot" + assert _send(agent, history, idx=2)[0]["content"] == "confirm reboot" - for untrusted in ("nan", 96_400.0, 4_000_000_000.0): # corrupt, or issued in the future + +def test_untrustworthy_confirmation_stamp_fails_closed(): + """A corrupt or future stamp on a dangerous confirmation cannot vouch for its age: the + text and its sidecar expire. A missing stamp (legacy row) is still left alone.""" + for untrusted in ("nan", 96_400.0, 4_000_000_000.0): row = [{"role": "user", "content": "confirm reboot", "timestamp": untrusted, "api_content": "confirm reboot"}] out = canonicalize_replay_history(row, now=10_000.0) assert "EXPIRED" in out[0]["content"] and "api_content" not in out[0], untrusted