Skip to content

fix(gateway): expire stale dangerous-confirmation text on replay (#59607) - #60110

Merged
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/59607-stale-confirmation
Jul 7, 2026
Merged

fix(gateway): expire stale dangerous-confirmation text on replay (#59607)#60110
kshitijk4poor merged 3 commits into
NousResearch:mainfrom
kshitijk4poor:salvage/59607-stale-confirmation

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

A stale high-risk confirmation phrase ("confirm forced restart" / 確認強制重開機) left in the transcript tail by a host reboot can no longer trigger the destructive action a second time when the user later sends an unrelated message (#59607, second failure mode).

Root cause: the host reboot kills the gateway before the tool result is persisted, so the transcript tail keeps the user's confirmation text. On resume, the model reads it as a live confirmation and re-executes (observed: second shutdown.exe reboot 50 minutes later, triggered by "are you there?").

Changes

  • agent/replay_cleanup.py: is_dangerous_confirmation() + strip_stale_dangerous_confirmations() — user messages matching known dangerous-confirmation patterns (incl. the i18n variants from the incident) whose timestamp is older than 60s are expired on gateway history replay — @knoal's fix, cherry-picked from fix(gateway): strip stale dangerous-confirmation text in user messages (#59607) #59640
  • gateway/run.py: wire the stripper into _build_gateway_agent_history() after the existing tail strippers; _build_replay_entry() preserves timestamp on user messages so the stripper can read it (transports already drop timestamp before the API — Strict chat-completions providers reject leaked messages[].timestamp metadata #47868) — @knoal
  • tests/gateway/test_stale_confirmation_expiry.py: 6 tests incl. a bug-detection test that fails on unfixed code — @knoal
  • Follow-up: expired confirmations are now REDACTED IN PLACE (content replaced with an explicit "confirmation EXPIRED — re-confirm before any destructive action" sentinel) instead of deleting the message. Deletion broke strict user/assistant role alternation on the exact incident tail (user(confirm) → assistant("OK, restarting") → two consecutive assistant messages). Redaction neutralizes the trigger, keeps the sequence valid, and gives the model an affirmative do-not-act instruction. + alternation-preservation regression test.

Note: the pattern list is a bounded mitigation for the observed incident class, not a general solution — a tracked-approval-with-TTL mechanism at the approval layer would cover arbitrary phrasing. Filing that as a follow-up design issue.

Validation

Before After
stale (>60s) confirmation on resume replayed verbatim → re-execution risk redacted to EXPIRED sentinel
fresh (<60s) confirmation replayed replayed (unchanged)
role alternation after expiry broken by deletion (2× assistant) preserved (u/a/u/a/u verified)
tests 7 expiry tests + 12 auto_continue pass
E2E (exact #59607 incident shape, EN + 中文) redaction + alternation + fresh-preserve all pass

Credit

Salvaged from #59640 by @knoal (commit cherry-picked; re-authored to their GitHub noreply identity since the original commit carried an anonymized agent email). Closes #59640. Refs #59607.

knoal and others added 3 commits July 7, 2026 11:23
NousResearch#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 conversation
transcript. If the host restart killed the gateway process before the
assistant's tool result was written, the transcript tail ends on the
assistant's text response - and the dangerous confirmation text remains
in the user role.

On the next inbound message - possibly a casual 'are you there?' from
the user minutes later - the LLM sees the stale confirmation and may
interpret the new turn as a fresh re-confirmation, re-executing the
destructive action. This is the failure mode reported in NousResearch#59607.

Fix:
- Add strip_stale_dangerous_confirmations() in agent/replay_cleanup.py
  that removes user messages whose content matches a known dangerous
  confirmation pattern AND whose timestamp is older than 60 seconds.
- Add is_dangerous_confirmation() helper with the matched patterns
  (i18n-aware: covers 確認強制重開機 from the original incident).
- Wire the stripper into _build_gateway_agent_history() right after the
  existing 75ed07a strippers, so the strip chain is:
  strip_interrupted_tool_tails -> strip_dangling_tool_call_tail ->
  strip_stale_dangerous_confirmations.
- Update _build_replay_entry() to preserve the timestamp on user
  messages (it was previously dropped), since the new stripper needs it.

Complements 75ed07a (which strips the assistant side of the broken
tail) by handling the user side: a stale plain-text confirmation that
the assistant has not yet responded to in a way the resume logic
recognises.

Failing-test-first discipline: the bug-detection test
test_stale_confirmation_text_is_stripped_on_resume fails on unfixed
code (proves the test catches the bug) and passes after the fix.
Five additional safety tests confirm no regression on:
- fresh confirmations (within expiry) are preserved
- non-confirmation text is preserved
- non-matching histories are untouched
- dangerous-pattern detection works in all cases (case, i18n, None)
- direct unit test of the strip helper

Refs: NousResearch#59607
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 NousResearch#59640 by @knoal.
Gateway user replay entries now carry a timestamp (read by the
stale-confirmation expiry check). The transports already sanitize it
(NousResearch#47868), but handle_max_iterations hand-builds api_messages and calls
chat.completions.create() directly, bypassing the transport — a strict
provider would 400 on the foreign key. Mirror the transport's pop here,
alongside the existing tool_name/codex_* sanitization.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) July 7, 2026 09:07
@kshitijk4poor
kshitijk4poor merged commit 6d3d9d0 into NousResearch:main Jul 7, 2026
29 checks passed
@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P1 High — major feature broken, no workaround labels Jul 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: salvage/cherry-pick of #59640 (@knoal, authorship preserved) with a redaction-in-place follow-up that keeps user/assistant alternation valid on the #59607 incident tail. This is the active superseding version. Also fixes #59607 (second, user-side failure mode) and relates to #49201 (distinct assistant-tail replay mechanism). Maintainer to pick between #59640 and this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/gateway Gateway runner, session dispatch, delivery P1 High — major feature broken, no workaround sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants