This repository was archived by the owner on Jul 16, 2026. It is now read-only.
Repository navigation
fix(discord-bridge): delivery-idempotency sentinel for poll_results (restart-safety #3) - #62
Open
VasiliyRad wants to merge 1 commit into
Open
VasiliyRad wants to merge 1 commit into
VasiliyRad wants to merge 1 commit into
Conversation
…restart-safety #3) ## The bug `poll_results` sends a reply, then archives the result + task files. If the bridge crashes BETWEEN the send (Discord API received the message) and the archive (result file still on disk), restart re- iterates the result file and sends AGAIN — duplicate delivery. Timeline: 1. pending_replies.pop(task_id) 2. save_pending_replies() — task is gone from pending state 3. channel.send(reply_text) — Discord receives the message 4. ❌ bridge crashes here 5. archive_file(result_file, ...) — never runs 6. archive_file(task_file, ...) — never runs On restart, the result file still exists at `results/{task_id}.txt` but `pending_replies` doesn't have the entry anymore (saved at step 2). The recovery scan in poll_results re-resolves the channel from `pending_replies` on disk — but the entry IS there because save was before pop completed... actually the exact ordering varies, but the core vulnerability is: result file present + no record of delivery = re-send. ## Fix `<state/discord-delivered/<task_id>.sentinel>` files mark "send returned success but archive not yet complete": 1. Right BEFORE the per-task send block, `_is_delivered(task_id)` checks the sentinel. If present → skip send, run archive, clear sentinel. 2. Right AFTER channel.send succeeds, `_mark_delivered(task_id)` touches the sentinel. 3. After archive completes, `_clear_delivered(task_id)` removes the sentinel (bounded dir growth). The crash window timeline becomes: - Crash after send, before sentinel-touch: restart re-sends. (Narrow — close with Discord nonce in a follow-up.) - Crash after sentinel-touch, before archive: restart sees sentinel + result file, runs archive only. No re-send. - Crash after archive: nothing to recover. ## Scope This PR covers `poll_results` main path only. Channel-redirect (`[channel: <id>]`), proactive, and dm-fallback paths also send to Discord but are scoped follow-ups — same pattern applies but each has its own archive flow. ## Tests `tests/discord-bridge-delivery-sentinel.test.py` (9 cases): - Sentinel storage primitives: default false; mark creates; clear removes; mark creates parent dir; clear idempotent; per-task independence. - Architectural source-grep wiring: _is_delivered runs BEFORE first channel.send; _mark_delivered runs AFTER first channel.send; _clear_delivered runs after archive. All 9 pass locally. Bridge syntax + import clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 of 2 tasks
bassilkhilo-ag2
approved these changes
May 24, 2026
bassilkhilo-ag2
left a comment
bassilkhilo-ag2
left a comment
There was a problem hiding this comment.
LGTM — cold review. Port of sutando sonichi#1048 (merged + approved by Chi + Susan in upstream). Implementation: delivery-idempotency sentinel written to state/discord-delivered/ before the DM goes out; on next poll_results tick the result is skipped if the sentinel exists. Prevents double-send on crash-between-send-and-consume. Path uses resolve_workspace() so sentinel lands in the writable workspace dir, not the app bundle. 14 tests. Merge after #60.
|
CI failed on this branch was from 2026-05-23. stando/main CI now passes 240/240. Could you rebase this branch or re-trigger CI? The failures look like pre-existing test issues in the base at the time, not caused by the PR itself. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
The bug
poll_resultssends a reply, then archives the result + task files. If the bridge crashes BETWEEN the send (Discord API received the message) and the archive (result file still on disk), restart re-iterates the result file and sends AGAIN — duplicate delivery.Fix
state/discord-delivered/<task_id>.sentinelfiles mark "send returned success but archive not yet complete":_is_delivered(task_id): if present → skip send, run archive, clear sentinelchannel.sendsucceeds_mark_delivered(task_id)touches sentinel_clear_delivered(task_id)removes sentinel (bounded dir growth)Crash window:
Scope
This PR covers
poll_resultsmain path. Channel-redirect ([channel: <id>]), proactive, dm-fallback paths are scoped follow-ups — same pattern applies but each has its own archive flow.Tests
tests/discord-bridge-delivery-sentinel.test.py— 9 cases:_is_deliveredbefore firstchannel.send;_mark_deliveredafter;_clear_deliveredafter archiveAll 9 pass locally.
Test plan
python3 tests/discord-bridge-delivery-sentinel.test.py— 9/9 pass🤖 Generated with Claude Code