Repository navigation
Conversation
|
Independent verification on the PR head (26da902): tests/gateway/test_update_command.py passes locally, 20/20 on Linux (canonical runner). Bounded wait with a sensible default: a notice naming a never-configured platform stops re-logging forever after an hour. No findings. |
An /update run leaves a marker naming the chat to notify. When that platform's adapter is not connected as the update finishes, _send_update_notification defers and keeps the markers for a later retry — correct for the restart that /update itself triggers, where the adapter reconnects seconds later. Nothing bounds that wait. If the platform is not configured at all, no adapter will ever appear and the defer never resolves. The startup path reschedules the watcher whenever the markers are still on disk, so the marker outlives every restart: it re-logs "adapter not connected yet" each poll_interval (2s), in every process, indefinitely. A marker written months ago was still doing this on one install, filling gateway.log with ~19k duplicate lines. Bound the wait using the timestamp the marker already carries. Once the marker is older than the cap the notice is undeliverable by any retry, so log it once at WARNING, clear the markers, and return True — the definitive answer that stops the caller rescheduling. Markers with no timestamp (written before that field existed) keep the previous retry behavior rather than guess an age. Tests cover the three behaviors: a stale marker is dropped and clears every marker file, a recent marker is still held for the reconnecting adapter, and a marker without a timestamp still retries.
gkd2323c
force-pushed
the
fix/gateway-update-notification-stale-marker
branch
from
September 19, 2026 05:12
26da902 to
f4a904b
Compare
Collaborator
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.
Bug
_send_update_notificationdefers with no upper bound when the marker's platform has no adapter. That defer is right for the case it was written for:/updaterestarts the gateway, so the requesting platform's adapter is briefly absent when the new process checks, and dropping the markers there would silently lose the "update finished" notice.But when no adapter will ever appear — the marker names a platform this install does not run — the defer never resolves.
_send_update_notificationreturns False, sorun_startupsees the markers still on disk and calls_schedule_update_notification_watch(); the watcher polls every 2s and re-logsNothing breaks that cycle, because the markers can never be delivered and so are never cleared.
Concretely: one install had a marker written 2026-07-08 still doing this today, every 2 seconds, across every gateway restart — about 19,000 lines of
gateway.log. The marker named a platform with no configured credentials, so no adapter was ever going to appear.Root cause
gateway/run_notifications.py, in_send_update_notification:The comment reads the absence as temporary ("not reconnected yet"). Nothing checks that assumption, and
_deferrenamesclaimedback topendingeach time, so the markers stay on disk and the next boot re-arms the watcher.Fix
Bound the wait using the
timestampthe marker already carries.slash_commands.pystampsdatetime.now().isoformat()when it writes the marker, and that value survives thepending ↔ claimedrename — the file mtime does not, which rules the mtime out as an age source. Once the marker is older than the cap the notice is undeliverable by any retry, so it is logged once at WARNING, the markers are cleared, and the method returns True, the definitive answerrun_startupkeys off to stop rescheduling. Markers with no parseable timestamp (written before the field existed) keep the previous retry behavior rather than have the code guess an age.Changed in
gateway/run_notifications.py: theif chat_id and not adapter:branch, a new_marker_age_seconds()helper beside_marker_profile(), and a new_UPDATE_NOTIFY_MAX_ADAPTER_WAIT_SECONDSconstant.Tests
tests/gateway/test_update_command.pygains three cases inTestSendUpdateNotification:Reverting the source change alone makes the first case fail with
assert False is True, which is the reported behavior. The file's other 17 cases pass, as dotest_platform_reconnect.py,test_update_streaming.py,test_startup_restart_race.pyandtest_heartbeat_watch_restore.py(69 total).Risk
Low. The new branch is reachable only when the adapter is already missing (today's defer) and the marker is older than an hour, so no currently-deliverable notification changes path. The threshold is a plain module constant with no config surface, and the helper returns
Noneon anything it cannot parse.