fix(gateway): recover resume_pending sessions instead of sending a blank turn - #46997
Closed
abchiaravalle wants to merge 1 commit into
Closed
fix(gateway): recover resume_pending sessions instead of sending a blank turn#46997abchiaravalle wants to merge 1 commit into
abchiaravalle wants to merge 1 commit into
Conversation
…ank turn A session interrupted by a gateway restart is flagged resume_pending and auto-continued on startup via _schedule_resume_pending_sessions(), which dispatches an empty-text internal MessageEvent. The recovery system note that should fill that empty turn is gated, in _run_agent(), on _interruption_is_fresh — the age of the LAST PERSISTED TRANSCRIPT ROW. For an active thread returned to after >1h of silence, that transcript clock is stale even though the interruption (last_resume_marked_at) is seconds old. The gate evaluates False, the note is not prepended, and the model receives a genuinely blank user turn — replying with confused 'that message came through blank' noise. Fix (two parts, both default-on, behavior unchanged for healthy turns): 1. resume_pending freshness now also considers last_resume_marked_at (the restart watchdog's own stamp). The branch fires when EITHER the transcript clock OR the resume mark is fresh, so the startup scheduler's freshness decision and the per-turn injection agree. 2. Empty-turn safety net: if the user turn is still blank after all injections AND the session is resume_pending, backfill a recovery note so a blank turn can never reach the model. Scoped to resume_pending so ordinary empty turns (e.g. uncaptioned image) are untouched. Adds 3 regression tests; the two core ones fail on the pre-fix logic.
tonydwb
approved these changes
Jun 16, 2026
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Fixes gateway resume_pending sessions to recover properly instead of sending blank turns. Targeted fix (2 files, ~8.7KB diff).
Looks Good
- Improves session recovery reliability
- No security or performance concerns
Reviewed by Hermes Agent (cron batch, 2026-06-16)
Contributor
|
Merged via PR #56262 — your commit was cherry-picked onto current main with your authorship preserved (commit c2db3ed). Salvaged onto the latest main; per review, the empty-turn safety net now reuses the canonical reason-aware recovery wording from the resume_pending branch instead of a second differently-worded note. Thanks for the clean diagnosis and the regression tests (confirmed RED pre-fix). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a confusing post-restart symptom where the agent replies with a variant of "that message came through blank" in a thread after the gateway restarts mid-turn.
The restart-resume machinery itself works — sessions are correctly flagged
resume_pendingand auto-continued on startup. The bug is a freshness-signal mismatch in how the recovery note is injected.Problem
On startup,
_schedule_resume_pending_sessions()re-continues each interrupted session by dispatching an empty-text internalMessageEvent(text=""). The empty turn is intended to be filled by the reason-aware recovery system note in_run_agent().But that note is gated on
_interruption_is_fresh, which measures the age of the last persisted transcript row. Meanwhile the startup scheduler made its own freshness decision offlast_resume_marked_at(the restart watchdog's stamp, set at interrupt time).These two clocks disagree:
last_resume_marked_at— seconds old → schedules the resume._is_resume_pendingisFalse→ the note is not prepended.Result:
messagestays"", the model gets a genuinely blank user turn, and replies with confused "the message came through blank" noise.Observed in production logs as frequent
Scheduled auto-resume for N restart-interrupted session(s)lines immediately followed by blank-turn replies in the affected threads.Fix
Two parts, both default-on, behavior unchanged for healthy turns:
resume_pendingis now considered fresh when either the transcript clock orlast_resume_marked_atis within the window — so the startup scheduler's decision and the per-turn injection agree.resume_pending, backfill a recovery note. Scoped toresume_pendingonly, so a legitimately empty user turn (e.g. an uncaptioned image) on a normal session is untouched.Testing
tests/gateway/test_restart_resume_pending.py— 76 passed (3 new):test_fresh_resume_mark_fires_despite_stale_transcript— the exact bug; fails on pre-fix logic.test_empty_resume_turn_never_reaches_model_blank— safety net; fails on pre-fix logic.test_empty_turn_guard_only_applies_to_resume_pending— guards against over-firing on ordinary empty turns.Both core regression tests were confirmed RED on the pre-fix logic and GREEN after. Broader gateway suite (
test_restart_resume_pending.py+test_clean_shutdown_marker.py) — 83 passed.