fix(kanban): run wake-only delivery before cursor advance on push adapters - #85556
Merged
Conversation
…h rewind/retry For a push-adapter subscription with delivery_mode='wake' the visible text ping is intentionally skipped (the send_passive gate), so the wake injection IS the sole delivery — yet the event cursor advanced BEFORE the wake, which then ran best-effort with its failure swallowed. A single failed wake permanently lost the event. Apply the same ordering the non-push (api_server) self-post branch already uses: attempt the wake BEFORE advancing the cursor; on failure rewind the claim (_kanban_rewind) and bump the per-sub failure counter so the next tick retries; on success reset the counter; drop the subscription after MAX_SEND_FAILURES consecutive failures like text sends do. notify+wake mode is unchanged: the text ping is the delivery and the wake stays best-effort after the cursor advance. Extracts the residual delivery-ordering insight from closed PR #84191. Co-authored-by: MaximCrabbe <crabbemaxim@gmail.com>
Contributor
૮ >ﻌ< ა ci reviewran on 5d95ba0 — fix(gateway): run wake-only kanban delivery before cursor ad
|
alt-glitch
added
type/bug
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
2 participants
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.
Summary
Wake-only kanban subscriptions (
delivery_mode='wake') on push adapters no longer lose events when the wake fails — the wake now runs BEFORE cursor advance, with rewind/retry on failure. Extracts the residual insight from #84191; Co-authored-by @MaximCrabbe, who identified the ordering gap.The wake is the SOLE delivery in wake-only mode (the text ping is intentionally skipped), yet the cursor advanced first and the wake ran best-effort after — a failed wake silently lost the event forever. The non-push (api_server) branch already had correct ordering; this brings the push-adapter branch in line.
Changes
gateway/kanban_watchers.py(+112/−55): push-adapter wake logic extracted into a shared_push_wake()helper; wake-only mode runs wake → success? → advance (else_kanban_rewind+ failure counter, sub dropped atMAX_SEND_FAILURES);notify+wakesemantics unchanged (text is the delivery, wake stays best-effort); premature counter-reset at the send_passive skip gate removedtests/gateway/test_kanban_notifier_wake_only_ordering.py: 4 tests — success advance, failure rewind+redeliver+counter, notify+wake unchanged, failure-cap dropValidation
Infographic