mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix: redact expired confirmations in place to preserve role alternation
Deleting the matched user message breaks the strict role-alternation
invariant on the exact incident tail this fix targets — user(confirm) →
assistant('OK, restarting') becomes two consecutive assistant messages,
which strict providers reject and which the alternation-repair passes
upstream don't cover. Replace the message content with an explicit
'confirmation EXPIRED, re-confirm before any destructive action'
sentinel instead: the trigger text is still neutralized, the model gets
an affirmative instruction not to act, and the message sequence stays
valid. Adds an alternation-preservation regression test.
Follow-up to the salvage of #59640 by @knoal.
This commit is contained in:
parent
33a529538d
commit
e7a6d676c8
2 changed files with 59 additions and 11 deletions
|
|
@ -168,6 +168,15 @@ _DANGEROUS_CONFIRMATION_PATTERNS: tuple = (
|
|||
"確認重啟",
|
||||
)
|
||||
|
||||
# Replacement text for an expired confirmation. Redacting in place (rather
|
||||
# than deleting the message) preserves strict user/assistant role
|
||||
# alternation in the replayed history.
|
||||
_EXPIRED_CONFIRMATION_SENTINEL = (
|
||||
"[A high-risk confirmation previously given here has EXPIRED and must "
|
||||
"not be acted on. Ask the user to re-confirm explicitly before "
|
||||
"performing any destructive action.]"
|
||||
)
|
||||
|
||||
|
||||
def is_dangerous_confirmation(content: Any) -> bool:
|
||||
"""Return True if a user-message text matches a known dangerous confirmation.
|
||||
|
|
@ -188,7 +197,7 @@ def strip_stale_dangerous_confirmations(
|
|||
now: float,
|
||||
expiry_seconds: float = _DANGEROUS_CONFIRMATION_EXPIRY_SECONDS,
|
||||
) -> List[Dict[str, Any]]:
|
||||
"""Strip stale dangerous-confirmation text in user messages (#59607).
|
||||
"""Expire stale dangerous-confirmation text in user messages (#59607).
|
||||
|
||||
When a high-risk side effect (e.g. host restart via ``shutdown.exe``)
|
||||
runs, the user's plain-text confirmation phrase is persisted in the
|
||||
|
|
@ -202,14 +211,19 @@ def strip_stale_dangerous_confirmations(
|
|||
interpret the new turn as a fresh re-confirmation, re-executing the
|
||||
destructive action. This is the failure mode reported in #59607.
|
||||
|
||||
This function strips user messages whose content matches a known
|
||||
dangerous confirmation pattern *and* whose timestamp is older than
|
||||
``expiry_seconds`` (default: 60s). Messages without a timestamp are
|
||||
left untouched (backward compatibility: legacy transcripts and
|
||||
in-memory test scaffolding have no timestamps). User messages that
|
||||
contain dangerous confirmation text but are within the expiry window
|
||||
are also left untouched — they represent a fresh confirmation that
|
||||
has not yet been acted on.
|
||||
Expired confirmations are REDACTED IN PLACE, not removed: deleting a
|
||||
user message from the incident tail (``user(confirm) →
|
||||
assistant("OK, restarting")``) would leave two consecutive assistant
|
||||
messages, violating the strict role-alternation invariant providers
|
||||
enforce. The message survives with its role intact; only the trigger
|
||||
text is replaced by a sentinel that tells the model the confirmation
|
||||
has expired.
|
||||
|
||||
Messages without a timestamp are left untouched (backward
|
||||
compatibility: legacy transcripts and in-memory test scaffolding have
|
||||
no timestamps). User messages that contain dangerous confirmation
|
||||
text but are within the expiry window are also left untouched — they
|
||||
represent a fresh confirmation that has not yet been acted on.
|
||||
|
||||
Complements 75ed07ace (which strips the *assistant* side of the
|
||||
broken tail) by handling the *user* side: a stale plain-text
|
||||
|
|
@ -229,12 +243,15 @@ def strip_stale_dangerous_confirmations(
|
|||
ts = msg.get("timestamp")
|
||||
if ts is not None and (now - float(ts)) > expiry_seconds:
|
||||
logger.debug(
|
||||
"Stripping stale dangerous-confirmation text from user "
|
||||
"Redacting stale dangerous-confirmation text in user "
|
||||
"message (age=%.1fs, expiry=%.1fs): %r",
|
||||
now - float(ts),
|
||||
expiry_seconds,
|
||||
(msg.get("content") or "")[:80],
|
||||
)
|
||||
redacted = dict(msg)
|
||||
redacted["content"] = _EXPIRED_CONFIRMATION_SENTINEL
|
||||
cleaned.append(redacted)
|
||||
continue
|
||||
cleaned.append(msg)
|
||||
return cleaned
|
||||
|
|
|
|||
|
|
@ -188,4 +188,35 @@ def test_strip_stale_dangerous_confirmations_directly():
|
|||
# The original user question and the assistant responses stay
|
||||
assert any("can you force a restart" in (m.get("content") or "") for m in cleaned)
|
||||
assert any("Rebooting the host is dangerous" in (m.get("content") or "") for m in cleaned)
|
||||
assert any("OK, restarting now" in (m.get("content") or "") for m in cleaned)
|
||||
assert any("OK, restarting now" in (m.get("content") or "") for m in cleaned)
|
||||
|
||||
def test_redaction_preserves_role_alternation():
|
||||
"""Expiry must redact in place, never delete the user message.
|
||||
|
||||
The incident tail is ``user(confirm) → assistant("OK, restarting")``.
|
||||
Deleting the user row would leave two consecutive assistant messages,
|
||||
violating the strict role-alternation invariant providers enforce.
|
||||
"""
|
||||
current_time = time.time()
|
||||
history = _make_history_with_confirmation(
|
||||
user_message_at=current_time - 1000,
|
||||
assistant_warning_at=current_time - 999,
|
||||
confirmation_message="confirm forced restart",
|
||||
confirmation_at=current_time - 300,
|
||||
assistant_action_at=current_time - 299,
|
||||
)
|
||||
|
||||
cleaned = _strip_stale_dangerous_confirmations(history, now=current_time)
|
||||
|
||||
# Same message count — nothing deleted.
|
||||
assert len(cleaned) == len(history)
|
||||
# The user slot survives with an expiry sentinel instead of the phrase.
|
||||
redacted = cleaned[2]
|
||||
assert redacted["role"] == "user"
|
||||
assert "confirm forced restart" not in redacted["content"]
|
||||
assert "EXPIRED" in redacted["content"]
|
||||
# No two consecutive same-role messages anywhere.
|
||||
roles = [m["role"] for m in cleaned]
|
||||
assert all(a != b for a, b in zip(roles, roles[1:])), roles
|
||||
# Original history object is not mutated.
|
||||
assert history[2]["content"] == "confirm forced restart"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue