Skip to content

fix(gateway): preserve update notification retry state - #42191

Closed
mths-mcfs wants to merge 1 commit into
NousResearch:mainfrom
mths-mcfs:fix/update-sendresult-retry
Closed

mths-mcfs wants to merge 1 commit into
NousResearch:mainfrom
mths-mcfs:fix/update-sendresult-retry

Conversation

@mths-mcfs

Copy link
Copy Markdown

Summary

  • Treat SendResult(success=False) as a failed /update completion notification delivery.
  • Preserve .update_pending*, .update_output.txt, and .update_exit_code so the gateway can retry instead of dropping the final update result.
  • Add regression coverage for both the streaming watcher and the post-restart fallback notification path.

Background

PR #39091 fixed the missing/offline adapter case for /update completion notifications. This PR covers the adjacent soft-failure case where an adapter exists and adapter.send(...) returns SendResult(success=False) without raising.

Sensitive data audit

  • The PR contains only changes to gateway/run.py and two gateway update tests.
  • Test chat/user IDs are dummy values (111 and 222).
  • A staged-diff scan checked for common token/key/secret patterns, private key blocks, JWTs, Telegram bot tokens, absolute local home paths, likely real Telegram chat IDs, and e-mail addresses.
  • Commit metadata uses a GitHub noreply address.

Test Plan

  • uv run --extra dev pytest tests/gateway/test_update_streaming.py tests/gateway/test_update_command.py -o 'addopts=' -q
  • uv run --extra dev python -m py_compile gateway/run.py tests/gateway/test_update_streaming.py tests/gateway/test_update_command.py
  • uv run --extra dev ruff check gateway/run.py tests/gateway/test_update_streaming.py tests/gateway/test_update_command.py
  • uv run --extra dev pytest tests/gateway/test_platform_reconnect.py -o 'addopts=' -q

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists labels Jun 8, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for covering the SendResult(success=False) path. Current main still discards the return value of the final streaming send at gateway/run.py:14602-14612 and of the post-restart send at gateway/run.py:14806-14810, followed by marker cleanup at gateway/run.py:14617-14623 and gateway/run.py:14819-14824. gateway/platforms/base.py:1878-1892 defines SendResult.success as the delivery outcome, so the proposed preservation behavior addresses a real current-main bug in both delivery paths.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/install-update Installer, updater, packaging, wheels, doctor labels Jul 14, 2026
@mths-mcfs mths-mcfs closed this by deleting the head repository Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/install-update Installer, updater, packaging, wheels, doctor 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-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.

3 participants