fix(kanban): add agent-only notification delivery - #84191
Conversation
alt-glitch
added
type/bug
|
Closing as superseded by #85487 (salvage of #37865), which landed per-subscription One piece of your PR survives as a real gap on main: for a push-adapter sub in wake-only mode, the wake is the sole delivery yet the cursor still advances BEFORE the best-effort wake — a failed wake permanently loses the event. Your delivery-before-advance ordering (with rewind/retry) is the right fix, and we're extracting it as a follow-up PR with credit to you for identifying it. Thanks — the ordering insight was the sharpest part of this PR. |
…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>
|
The residual ordering fix promised in the close comment above has landed: PR #85556 (commit f57116b) runs wake-only delivery BEFORE cursor advance on push adapters with rewind/retry, so a failed wake can no longer lose the event. You are credited via Co-authored-by on the commit. Thanks again for spotting the gap. |
3 participants
Summary
kanban.notification_delivery_modewith backwards-compatibletext_and_agentand newagent_onlyagent_only, without native Kanban text/artifactsWhy
Push adapters currently couple native Kanban terminal messages to the LLM wake. Disabling native Discord notifications therefore also removes the completion wake, so an asynchronously completed task can silently lose its user-facing result.
Tests
hermes_clisuite: 10,027 passed, 64 skipped, 0 failedpy_compile, andgit diff --check: passedCompatibility
The default remains
text_and_agent; invalid values fail safely to that existing behavior.