Skip to content

fix(gateway): retry restart completion delivery - #70859

Open
stepanov1975 wants to merge 2 commits into
NousResearch:mainfrom
stepanov1975:fix/gateway-restart-notification-delivery
Open

fix(gateway): retry restart completion delivery#70859
stepanov1975 wants to merge 2 commits into
NousResearch:mainfrom
stepanov1975:fix/gateway-restart-notification-delivery

Conversation

@stepanov1975

@stepanov1975 stepanov1975 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Problem

The gateway consumed .restart_notify.json on a one-shot startup send. If the platform adapter was not ready yet, the transport refused the send as retryable, or startup/shutdown raced the worker, the completion obligation could be lost and the requester never learned that the gateway returned.

Current main still fails the PR-owned regression tests when they are transplanted without the production change (11 failed, 36 passed).

Approach

  • claim the durable restart marker before adapters connect;
  • schedule delivery as a supervised background task so startup is not blocked;
  • preserve the marker across transport unavailability, explicit retryable pre-send refusal, cancellation, and gateway shutdown;
  • retry with bounded backoff and retry_after support;
  • enforce the total deadline inside the dispatch task, including providers that suppress cancellation;
  • serialize concurrent /restart handlers and finish any in-flight worker-thread marker write before propagating even repeated cancellation, so stale writes cannot reorder;
  • assign each marker a unique request ID, publish that generation before the worker-thread write, and revalidate ownership before every attempt/send/delete so an older worker cannot send for or consume a replacement marker;
  • preserve malformed, unreadable, or replacement marker bytes instead of deleting an obligation the worker cannot prove it owns.

The implementation reuses the current resolve_delivery_transport() path and keeps current-main async marker writes and startup/session lifecycle changes intact.

Relationship to overlapping work

Validation

Rebased onto NousResearch/hermes-agent@56526bc0d3 (2026-08-16).

  • scripts/run_tests.sh across 12 restart/startup/reconnect/session-finalization/update-streaming files: 154 passed, 2 skipped
  • the async-writer race regression fails on the rebased original implementation and passes after the current-main adaptation
  • ruff check on all touched Python files: passed
  • python3 -m py_compile on touched modules/tests: passed
  • python3 scripts/check-windows-footguns.py --all: passed (973 files scanned)
  • test-only transplant onto the exact base: 11 failed, 36 passed, confirming the residual behavior remains on main
  • mypy gateway/run.py gateway/slash_commands.py: blocked identically on the candidate and exact base by the same two pre-existing repository errors (mutagen.oggopus stubs missing and the existing tools/tts_tool.py syntax error)

Related #69370.

Related #66589.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 24, 2026
@stepanov1975
stepanov1975 force-pushed the fix/gateway-restart-notification-delivery branch from 63366dd to 233dc45 Compare July 24, 2026 21:41
@isak-ialogics

Copy link
Copy Markdown
Contributor

Current GitHub state is DIRTY, and a local git merge-tree --write-tree upstream/main 233dc454 reproduces a content conflict specifically in gateway/run.py (gateway/slash_commands.py auto-merges). Since the PR’s merge base (7cd48733d) main has added the loop-liveness watchdog and startup/reconnect fixes in the same runner, the next useful step is to refresh onto current main, resolve gateway/run.py, then rerun tests/gateway/test_restart_notification.py and tests/gateway/test_startup_restart_race.py to verify the supervised retry task still composes with the new startup/stop lifecycle.

@stepanov1975
stepanov1975 force-pushed the fix/gateway-restart-notification-delivery branch from 233dc45 to b72824e Compare July 25, 2026 08:13
@stepanov1975

Copy link
Copy Markdown
Contributor Author

Thanks for flagging the new lifecycle changes. I rechecked the premise against current main rather than only resolving the conflict.

Current main now includes the Telegram cold-start readiness behavior associated with #69240: a cold Telegram adapter must complete a generation-scoped getUpdates round trip before it reports connected. I have therefore reframed this PR so it no longer claims to implement that readiness fix or to Fixes #69370.

A separate gateway-owned durability gap still reproduces on exact current main: the restart-completion consumer runs once and removes .restart_notify.json when the target transport is still recovering, when delivery returns an explicit retryable pre-send refusal, or when a newer marker replaces the startup generation before the older worker first runs. Applying only this PR's regression tests to current main produced five expected failures covering transport appearance, retryable recovery, unique marker ownership, post-read replacement preservation, and pre-first-run replacement preservation.

The currentized candidate keeps the new resolve_delivery_transport() relay routing, Telegram readiness behavior, reconnect lifecycle, and loop-liveness guards. Its scope is now only durable chat-originated restart completion across transient startup/reconnect conditions.

Current local verification before publication:

  • 7 relevant files: 154 passed, 0 failed
  • Ruff: passed
  • Windows footgun scan: 808 files, no findings
  • git diff --check and synthetic merge-tree: passed
  • mypy and ty: no added normalized diagnostic shapes against the exact base
  • independent focused review of exact b72824eb47cdf3710a6c749f2d3828a8844dac67: PASS, with no Critical, Important, or Minor findings

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for narrowing this to the remaining restart-delivery durability gap. The premise is verified on current main: gateway/run.py:20093-20099 returns when no transport is live, while gateway/run.py:20151-20152 always removes the marker; gateway/run.py:20133-20140 likewise consumes it after any failed SendResult. The candidate’s bounded retry, marker-payload ownership check, and request ID target those paths without changing platform send gates.

GitHub currently reports commit b72824eb47cdf3710a6c749f2d3828a8844dac67 as conflicting with current main, so salvage will require reconciling gateway/run.py with the newer startup lifecycle. No additional correctness defect was identified in the reviewed diff.

Automated hermes-sweeper review.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Eight PRs address or reference the Telegram restart-notification complex. The diffs split between readiness-only fixes, non-durable retries, durable marker ownership, and a now-separate cold/crash startup broadcast policy; current main addresses the original cold-start readiness race, while retryable transport recovery and marker retention remain the distinct concern in #70859.

Related pull requests

Duplicates

#65709, #69530, and #73483 substantially overlap on readiness-gated requester retry; #72009 duplicates that retry intent without durable ownership, while #68569 targets the same symptom but conflicts with the polling-health contract. #70859 and #66598 overlap on retryable requester-marker durability, but #70859 adds generic transport and marker-generation ownership while #66598 also covers planned-restart home-channel delivery; #69676 is now separate policy scope.

Suggested consolidation

Author action: rebase #70859 onto current main, reconcile gateway/run.py with the newer startup lifecycle, and rerun tests/gateway/test_restart_notification.py plus tests/gateway/test_startup_restart_race.py; this follows its keep_open verdict and preserves its Verify-selected generic transport/marker-ownership work. Keep #66598 open with the salvage path of validating its startup and multi-target marker semantics; #68569 and #72009 can be closed as weaker duplicates after their reviewed objections remain unresolved, while #69676 should stay isolated for broadcast-policy review and the already-closed #65709, #69530, and #73483 remain historical/superseded references.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I66589(["issue #66589 (open)"])
    I69370(["issue #69370 (closed)"])
    I72663(["issue #72663 (open)"])
    I73473(["issue #73473 (closed)"])
    P70859["PR #70859 (open)"]
    P70859 -.->|partial| I66589
    P70859 -->|best fix| I69370
    P70859 -->|best fix| I72663
    P70859 -->|best fix| I73473
    class I66589 open
    class I69370 closed
    class I72663 open
    class I73473 closed
    class P70859 open
    class P70859 best
    class P70859 best
    class P70859 best
    class P70859 target
    click I66589 "https://github.com/NousResearch/hermes-agent/issues/66589"
    click I69370 "https://github.com/NousResearch/hermes-agent/issues/69370"
    click I72663 "https://github.com/NousResearch/hermes-agent/issues/72663"
    click I73473 "https://github.com/NousResearch/hermes-agent/issues/73473"
    click P70859 "https://github.com/NousResearch/hermes-agent/pull/70859"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 8 pull requests and 5 issues in this complex. Each diff was read against this issue; Assessment working set: 116 kB of PR diffs, 49 kB of issue/PR text, 36 kB of discussion (40 comments), 59 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@stepanov1975
stepanov1975 force-pushed the fix/gateway-restart-notification-delivery branch from 53ae5c9 to e5d24a8 Compare August 3, 2026 21:41
@alt-glitch alt-glitch added platform/telegram Telegram bot adapter needs-decision Awaiting maintainer decision before any implementation and removed platform/telegram Telegram bot adapter labels Aug 3, 2026
@stepanov1975
stepanov1975 force-pushed the fix/gateway-restart-notification-delivery branch from e5d24a8 to 0b7d455 Compare August 6, 2026 17:29
@stepanov1975

Copy link
Copy Markdown
Contributor Author

Rebuilt this branch from current main (9d4ef04ed) and reduced the review surface from 8 files to one 4-file commit (0b7d45584). The unrelated Relay/WebSocket transport hardening is intentionally dropped; the retained state machine still covers marker ownership, provider-entry revalidation, bounded retry, safe cancellation, and newer-marker preservation.

Local verification: 47 focused tests, Ruff, Windows-footgun scan, diff check, and synthetic merge passed. An independent adversarial re-review found no Critical/Important/Minor issues.

Keep the durable restart marker while a transport is temporarily unavailable or after an explicit retryable pre-send refusal. Retry with bounded backoff without blocking gateway startup, and enforce the total deadline inside the provider-dispatch task without waiting for cancellation-resistant sends to settle.

Claim marker ownership before adapters connect without letting malformed marker bytes abort startup, preserve a captured obligation if its first worker read fails, revalidate ownership before every attempt, and add a unique request ID so an older worker cannot send for or consume a replacement restart marker.

Related NousResearch#69370.

Related NousResearch#66589.

(cherry picked from commit 53ae5c9d849dac713b825037c5470bbaa5b3324e)
@stepanov1975
stepanov1975 force-pushed the fix/gateway-restart-notification-delivery branch from 0b7d455 to 9c7da2e Compare August 16, 2026 11:14
@stepanov1975
stepanov1975 force-pushed the fix/gateway-restart-notification-delivery branch from 9c7da2e to 8c4ba82 Compare August 16, 2026 11:20
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 needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants