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.
This commit is contained in:
@@ -196,7 +196,10 @@ _EXPIRED_CONFIRMATION_SENTINEL = (
|
|||||||
|
|
||||||
def is_dangerous_confirmation(content: Any) -> bool:
|
def is_dangerous_confirmation(content: Any) -> bool:
|
||||||
"""True if user-message text contains a known dangerous confirmation phrase."""
|
"""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(
|
def strip_stale_dangerous_confirmations(
|
||||||
|
|||||||
@@ -510,7 +510,8 @@ def _reset_per_turn_agent_state(agent: Any) -> None:
|
|||||||
_reset_consol()
|
_reset_consol()
|
||||||
# Expiry clock for build_api_messages: admission time (not the input's platform-event
|
# 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
|
# 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()
|
agent._current_turn_timestamp = time.time()
|
||||||
|
|
||||||
# Pre-turn connection health check: clean up dead TCP connections.
|
# 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
|
# 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.
|
# 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
|
# Without an anchor (compaction found no surviving user row) there is no provable
|
||||||
# persisted prefix, so nothing is canonicalized.
|
# persisted prefix, so nothing is canonicalized. The clock is stamped once per turn in
|
||||||
turn_now = getattr(agent, "_current_turn_timestamp", None) or time.time()
|
# _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
|
split = current_turn_user_idx if has_current else 0
|
||||||
canonical_messages = canonicalize_replay_history(messages[:split], now=turn_now) + messages[split:]
|
canonical_messages = canonicalize_replay_history(messages[:split], now=turn_now) + messages[split:]
|
||||||
|
|
||||||
|
|||||||
@@ -125,10 +125,10 @@ def _wire(messages):
|
|||||||
return json.dumps(ChatCompletionsTransport().convert_messages(list(messages)), sort_keys=True)
|
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(
|
request, _ = build_api_messages(
|
||||||
agent, history, current_turn_user_idx=len(history) - 1, ext_prefetch_cache="",
|
agent, history, current_turn_user_idx=len(history) - 1 if idx is None else idx,
|
||||||
plugin_user_context="", moa_config=None, active_system_prompt="",
|
ext_prefetch_cache="", plugin_user_context="", moa_config=None, active_system_prompt="",
|
||||||
)
|
)
|
||||||
return request
|
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": "{}"}}]},
|
{"id": "c4", "type": "function", "function": {"name": "search_files", "arguments": "{}"}}]},
|
||||||
{"role": "tool", "tool_call_id": "c4", "content": "[Command interrupted]"},
|
{"role": "tool", "tool_call_id": "c4", "content": "[Command interrupted]"},
|
||||||
]
|
]
|
||||||
request2 = build_api_messages(
|
request2 = _send(_SendAgent(), live, idx=len(persisted))
|
||||||
_SendAgent(), live, current_turn_user_idx=len(persisted), ext_prefetch_cache="",
|
|
||||||
plugin_user_context="", moa_config=None, active_system_prompt="",
|
|
||||||
)[0]
|
|
||||||
assert _wire(request2[: len(request)]) == _wire(request)
|
assert _wire(request2[: len(request)]) == _wire(request)
|
||||||
assert request2[-1]["content"] == "[Command interrupted]"
|
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
|
"""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
|
from agent.turn_context import _reset_per_turn_agent_state
|
||||||
|
|
||||||
agent = _SendAgent()
|
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": "{}"}}]},
|
{"id": "c1", "type": "function", "function": {"name": "read_file", "arguments": "{}"}}]},
|
||||||
{"role": "tool", "tool_call_id": "c1", "content": "ready"}]
|
{"role": "tool", "tool_call_id": "c1", "content": "ready"}]
|
||||||
monkeypatch.setattr("agent.turn_context.time.time", lambda: 10_500.0) # ...however long the tools take
|
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="",
|
assert _send(agent, history, idx=2)[0]["content"] == "confirm reboot"
|
||||||
plugin_user_context="", moa_config=None, active_system_prompt="")[0]
|
|
||||||
assert late[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"}]
|
row = [{"role": "user", "content": "confirm reboot", "timestamp": untrusted, "api_content": "confirm reboot"}]
|
||||||
out = canonicalize_replay_history(row, now=10_000.0)
|
out = canonicalize_replay_history(row, now=10_000.0)
|
||||||
assert "EXPIRED" in out[0]["content"] and "api_content" not in out[0], untrusted
|
assert "EXPIRED" in out[0]["content"] and "api_content" not in out[0], untrusted
|
||||||
|
|||||||
Reference in New Issue
Block a user