Skip to content

fix(gateway): harden restart resume recovery - #30030

Open
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/restart-resume-reply-anchor
Open

fix(gateway): harden restart resume recovery#30030
Qwinty wants to merge 1 commit into
NousResearch:mainfrom
Qwinty:fix/restart-resume-reply-anchor

Conversation

@Qwinty

@Qwinty Qwinty commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Refreshes restart-resume recovery for the current async session architecture.

  • Enumerate restart-interrupted sessions through a public SessionStore.list_resume_pending() snapshot and the AsyncSessionStore facade; the gateway no longer takes the store lock or reads _entries directly on the event loop.
  • Load transcript freshness and reply anchors through AsyncSessionStore.load_transcript().
  • Resolve Telegram DM topic bindings and compression tips before selecting the transcript that recovery will consume.
  • Anchor the synthetic resume event to the latest persisted user message, not a stale long-lived routing origin.
  • Clear resume_pending without a new model turn when the transcript already ends in a completed assistant response.
  • Preserve the pre-claim sentinel so inbound messages cannot race a startup resume into a duplicate agent.
  • On platform reconnect, redeliver completed ledger obligations before scheduling interrupted turns, scoped to that platform.
  • Prefer the live cached source for shutdown/restart notifications so topic replies target the current inbound message.

Verification

Immutable current-main test archive:

  • restart/resume/reconnect/delivery suites: 102 passed
  • Ruff, py_compile, and git diff --check: passed

Fixes #10163.
Related: #23314, #29713, #46963, #46997.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery labels May 21, 2026
@Qwinty
Qwinty force-pushed the fix/restart-resume-reply-anchor branch 2 times, most recently from 387ec85 to a7fa549 Compare May 26, 2026 08:47
@Qwinty

Qwinty commented May 26, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on current upstream/main; PR is mergeable again.

Local verification:

  • python -m pytest -q -o addopts='' tests/gateway/test_goal_verdict_send.py tests/gateway/test_restart_resume_pending.py - 86 passed
  • python -m ruff check gateway/run.py tests/gateway/test_goal_verdict_send.py tests/gateway/test_restart_resume_pending.py - passed
  • git diff --check upstream/main...HEAD - passed

CI has been retriggered on a7fa549d3.

@xiaoyaner0201

Copy link
Copy Markdown

Cherry-picked onto our v0.15.2-based fork (Discord gateway) together with #37669. The two compose cleanly — only a small benign overlap in _schedule_resume_pending_sessions, where #37669's in-flight guard and this PR's freshness / empty-bound-topic skips sit next to each other (both kept, sequentially). Restart-resume regression suite is green. Thanks for hardening this path!

@Qwinty
Qwinty force-pushed the fix/restart-resume-reply-anchor branch from e5069a6 to a4e81bd Compare June 9, 2026 12:48
@Qwinty
Qwinty force-pushed the fix/restart-resume-reply-anchor branch from a4e81bd to 57598bf Compare June 17, 2026 07:39
@Qwinty
Qwinty force-pushed the fix/restart-resume-reply-anchor branch from 42e581a to a2bc864 Compare July 10, 2026 17:11

@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 the focused recovery hardening. The reported behavior is still present on current main: gateway/run.py:6603-6606 schedules from marker freshness without examining completed transcript state, gateway/run.py:10288-10306 invokes the goal hook for any non-empty result, and gateway/run.py:10878-10886 drops restart metadata when topic binding switches sessions.

Problems

  • The PR makes _schedule_resume_pending_sessions() async but calls synchronous SessionStore I/O directly at gateway/run.py:6720, :6736, :6760, and :6775 in PR head c05ab12. Current main commit 9d38a2309 established AsyncSessionStore as the gateway-loop boundary (gateway/session.py:969-983), so this needs a port rather than a direct cherry-pick.

Suggested changes

  • Use the current async SessionStore facade for transcript loads and resume-marker clears, with an async-safe candidate snapshot.
  • Add a call-site test for a non-empty failed/provider-error result to prove _post_turn_goal_continuation is not invoked.

Automated hermes-sweeper review.

Comment thread gateway/run.py Outdated
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@Qwinty
Qwinty force-pushed the fix/restart-resume-reply-anchor branch 2 times, most recently from b27d3da to 8c9f0dd Compare July 13, 2026 21:27
@Qwinty
Qwinty force-pushed the fix/restart-resume-reply-anchor branch from 8c9f0dd to 68b9c9c Compare July 19, 2026 13:30
@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current upstream/main.

Preserved main's delivery-ledger redelivery path (_redeliver_pending_obligations) and made _schedule_resume_pending_sessions async as in this PR, including:

  • await self._redeliver_pending_obligations()
  • await self._schedule_resume_pending_sessions()

Local verification:

python -m pytest -q -o 'addopts=' \
  tests/gateway/test_restart_resume_pending.py \
  tests/gateway/test_platform_reconnect.py \
  tests/gateway/test_goal_verdict_send.py
# 149 passed

@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Late findings follow-up

Pushed e27c41db8 for restart-resume recovery hardening:

  • sweep_recoverable(platforms=...) no longer claims ledger rows for adapters that are still offline
  • reconnect path redelivers completed obligations before scheduling another auto-resume turn
  • Telegram DM topic resume bindings follow get_compression_tip() so compressed children are used
  • added tests/gateway/test_restart_resume_late_regressions.py + reconnect ordering coverage

Local verification: restart-resume / delivery-ledger / reconnect suites green.

@Qwinty

Qwinty commented Jul 19, 2026

Copy link
Copy Markdown
Contributor Author

Final independent-review follow-up

Pushed 735ba2103:

  • updated the defensive adapter-race comment and removed parameter shadowing
  • added an end-to-end regression: offline row stays unclaimed, same-process reconnect reclaims it, and delivery succeeds

Local verification: 156 passed; ruff clean.

@Qwinty
Qwinty force-pushed the fix/restart-resume-reply-anchor branch from 735ba21 to 0b7a65b Compare August 9, 2026 10:01
@Qwinty

Qwinty commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Rebuilt the branch on current main as 0b7a65b980 after the async session-store refactor.

The resume scheduler now uses a public SessionStore.list_resume_pending() snapshot through AsyncSessionStore; transcript reads and marker clears are also async-facade calls, so the gateway no longer takes the store lock or reads _entries on the event loop.

The refreshed version also preserves the remaining recovery behavior: Telegram topic binding/compression-tip resolution, latest-user reply anchors, completed-assistant-tail suppression, startup pre-claim, live shutdown anchors, and ledger-before-resume ordering after reconnect.

Immutable verification: 102 tests passed across restart/resume/reconnect/delivery suites; Ruff, py_compile, and diff-check passed. CI is running on the new head.

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 sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

[Bug]: Telegram topic session can behave like /new after gateway restart/update despite persistent-session design

4 participants