Skip to content

[Bob] Fix gateway restart final-delivery continuity - #68902

Open
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/slack-continuity-transaction-e2e
Open

[Bob] Fix gateway restart final-delivery continuity#68902
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/slack-continuity-transaction-e2e

Conversation

@trac3r00

Copy link
Copy Markdown
Contributor

[Bob]

Summary

  • keep resume_pending durable until the messaging platform ACKs the final response
  • commit normal final-delivery recovery from the adapter after SendResult.success while preserving streaming and intentional-silence behavior
  • count restart-loop breaker state once per gateway boot so a late Slack/platform reconnect can resume pending work
  • add a real-path Slack continuity E2E with temp HERMES_HOME, real SessionDB, deterministic compression rollback/retry, turn-lease release, restart-boundary delivery ledger recovery, thread identity, and duplicate/output-leak assertions

Root cause

The gateway cleared resume_pending as soon as the model turn returned, before the base adapter had recorded a delivery obligation or received a platform ACK. A restart in that gap left neither a resumable turn nor a durable final response. The reconnect auto-resume pass also recorded the restart-loop breaker twice in one boot (startup plus reconnect), suppressing recovery.

Verification

  • RED before fix: resume_pending_during_send == [False]
  • scripts/run_tests.sh focused continuity/regression set: 149 passed, 0 failed
  • gateway sweep: 9,531 passed; 7 unrelated macOS/environment failures (/tmp alias expectation, unsupported systemd abstract socket, file-descriptor exhaustion, and existing platform isolation failures)
  • git diff --check: clean

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 21, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: merged #67181 introduced the durable delivery ledger and open #67078 covers broader crash-safe in-flight recovery. This PR is the narrower post-ACK resume_pending lifecycle follow-up.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tracing the pre-ACK recovery gap and adding a real restart-boundary scenario.

Problems

  • The new confirmation path in gateway/platforms/base.py only runs after the normal final text send. A successful short Telegram auto-TTS reply ACKs its final text via play_tts() (gateway/platforms/base.py:5842-5850) and intentionally skips that text-send branch (gateway/platforms/base.py:5868). The proposed gateway/run.py condition only confirms already_sent or intentional silence there, so this successful final-delivery path would retain resume_pending.
  • The new Slack test exercises ordinary text delivery, but not the caption-only path already covered by tests/gateway/test_base_topic_sessions.py:285-304.

Suggested changes

  • Centralize confirmation on every successful final user-visible payload, including TTS captions and attachment-only responses, and add the caption-only resume-pending regression.

Automated hermes-sweeper review.

Comment thread gateway/platforms/base.py
# The model turn is not a completed restart recovery until
# the platform has ACKed its final response. GatewayRunner
# deliberately keeps ``resume_pending`` set while the text
# is only a local return value; clear it here, after the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This confirmation only follows the normal text _send_with_retry route. A successful short Telegram auto-TTS reply ACKs its final caption through play_tts() and skips the text-send block, so the proposed runner-side already_sent/silence fallback will leave resume_pending set. Please route every successful final payload, including caption-only TTS and attachment-only responses, through the same confirmation boundary.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

3 participants