fix(discord-bridge): delivery-idempotency sentinel for poll_results (restart-safety) by VasiliyRad · Pull Request #1048 · sonichi/sutando

VasiliyRad · 2026-05-23T01:40:38Z

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.

Fix

state/discord-delivered/<task_id>.sentinel files mark "send returned success but archive not yet complete":

Order	Action
1. before per-task send block	`_is_delivered(task_id)`: if present → skip send, run archive, clear sentinel
2. after `channel.send` succeeds	`_mark_delivered(task_id)` touches sentinel
3. after archive completes	`_clear_delivered(task_id)` removes sentinel

Crash windows:

Crash point	Recovery
after send, before sentinel-touch	restart re-sends (narrow; close with Discord nonce in follow-up)
after sentinel-touch, before archive	restart sees sentinel + result file → archive only, no re-send
after archive	nothing to recover

Scope

poll_results main path only. Channel-redirect ([channel: <id>]), proactive, dm-fallback paths are scoped follow-ups.

Tests

tests/discord-bridge-delivery-sentinel.test.py — 9 cases: sentinel storage primitives + architectural source-grep wiring assertions.

All 9 pass locally.

🤖 Generated with Claude Code

amkunte · 2026-05-23T02:02:30Z

Same root cause as #1046: discord-bridge.py reads DISCORD_BOT_TOKEN only from the ~/.claude/channels/discord/.env file — ignores os.environ. The test sets the env var, but the bridge doesn't check it.

Fix is in #1050 (CI green). Once merged to main and rebased here, CI should pass.

qingyun-wu

Nice — the delivery-idempotency sentinel closes a real double-send window, and the logic is correct: the crash-between-channel.send-and-archive case is handled by the skip-block on restart, and the remaining narrow window (crash between send returning and the sentinel write) is honestly called out rather than papered over. Scoping to the poll_results main path is sensible. A couple of things before it's green:

1. (blocking, this is why CI is red) The test can't satisfy the token check — same root cause as #1046.

CI fails at:

--- tests/discord-bridge-delivery-sentinel.test.py ---
DISCORD_BOT_TOKEN not set in ~/.claude/channels/discord/.env

src/discord-bridge.py (~lines 70-79) loads the token only by parsing ~/.claude/channels/discord/.env and exit(1)s if absent — it never reads os.environ. So os.environ.setdefault("DISCORD_BOT_TOKEN", ...) has no effect, and the import exits in CI. This overlaps #1050 (which adds env-var support). Either land #1050 first, or make the test self-contained (write a temp .env under a HOME the bridge reads, or monkeypatch the loader).

2. (likely blocking once #1 is fixed) The test imports the bridge without stubbing discord.

#1046's sibling test injects a discord stub (sys.modules["discord"] = stub) before importing the bridge — which strongly implies import discord isn't available in the CI environment. This test's _load has no such stub, so after the token issue is resolved it will probably fail at import discord. Recommend mirroring #1046's stub here (the sentinel helpers are pure filesystem, so a minimal stub is enough).

3. (non-blocking, coverage) The poll_results wiring is only grep-asserted.

The helpers (_mark/_is/_clear_delivered) are well unit-tested, but as the docstring notes, the integration is "asserted via source-grep." A follow-up that drives a single poll_results iteration against a fake channel — sentinel present → asserts skip+archive, no channel.send — would lock in the actual restart behavior the PR is about. Fine to defer, just flagging.

Logic LGTM; once CI is green I'll re-review. Thanks for the careful docstrings.

…#1050) Matches telegram-bridge.py pattern. Allows test suites to inject a stub token via os.environ without needing a real ~/.claude/channels/ discord/.env file — unblocks CI for VasiliyRad PRs #1046 and #1048 whose tests already set the env var before importing the bridge. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>

liususan091219

Delivery-idempotency sentinel is the right tool for the crash-between-send-and-archive window, and the three-step protocol (mark-after-send, check-before-send, clear-after-archive) is correctly ordered. The scoped narrow window (crash between send returning and the sentinel write) is honestly disclosed rather than papered over — I appreciate that. Same .env test-ordering issue as #1046/#1047 (now resolved by #1050). Logic is solid.

Things I like

Three-step protocol with correct ordering:
- check at src/discord-bridge.py:2922-2929 (BEFORE the send block)
- mark at src/discord-bridge.py:3068 (immediately AFTER channel.send success)
- clear at src/discord-bridge.py:3076 (after both archive calls)
This is the textbook two-phase delivery sentinel — re-derived correctly. Sentinel-present-with-result-file = "previous send committed, archive failed" → recovery is archive-only.
Sentinel cleared on the skip-path too at src/discord-bridge.py:2932 — bounded directory growth. Without this, state/discord-delivered/ would accumulate one sentinel per crash-during-archive forever; with it, the dir clears as soon as the archive succeeds even on the recovery path.
Clear is unlink(missing_ok=True) at src/discord-bridge.py:2796-2797. Idempotent; "clear on already-cleared" is a no-op. Pinned by test_clear_idempotent.
_mark_delivered creates the directory at src/discord-bridge.py:2778-2783. First-ever delivery on a fresh workspace has no state/discord-delivered/ dir yet; the mkdir guard avoids a startup-order requirement. Pinned by test_mark_creates_directory_if_missing.
Per-task sentinel files (one per task_id) at src/discord-bridge.py:2774-2775 — natural concurrency isolation, no shared-state JSON to corrupt across the rare-but-possible concurrent poll_results ticks. Pinned by test_separate_tasks_independent.
Honest scope statement in the docstring at src/discord-bridge.py:2744-2762: explicitly calls out the residual narrow window (between send-returning and sentinel-touch) as a known not-closed vector, with a pointer to a nonce-based follow-up. Same docstring also explicitly says channel-redirect / proactive / dm-fallback are scoped follow-ups. Reviewer-friendly.
Three architectural grep assertions at tests/discord-bridge-delivery-sentinel.test.py:test_poll_results_checks_sentinel_before_main_send / _marks_delivered_in_send_block / _clears_sentinel_after_archive. Pin the order: check BEFORE send, mark AFTER send, clear AFTER archive. If a future refactor reorders these, the test fails loudly with a useful message rather than a silent regression.

Non-blocking observations

Token-load test gap (stack-wide, resolved upstream). tests/discord-bridge-delivery-sentinel.test.py:18 uses os.environ.setdefault("DISCORD_BOT_TOKEN", ...). Pre-#1050 the bridge ignored env and read the file only; CI failed in isolation. Post-#1050 (merged 2026-05-23 04:21 UTC) the env path is honored. Rebase on main → green.
Mock-import gap (Qingyun's #2 in her review). Unlike #1046's sibling test, this one's _load already includes the discord stub at tests/discord-bridge-delivery-sentinel.test.py:48-57 — DMChannel and Object are stubbed. So once the token gate is satisfied, the import should succeed. (Qingyun's concern there was based on the older state — current diff has the stub.)
Scope: poll_results main path only. Docstring calls this out. The channel-redirect branch at src/discord-bridge.py:2952 inherits coverage (same _mark_delivered at line 3068 fires regardless of which channel was used) — the docstring slightly oversells the gap. Real follow-ups: poll_proactive (~3003+) and poll_dm_fallback (~3159+). Worth filing as issues so they don't drift.
Coverage suggestion (Qingyun's #3, agree non-blocking): A follow-up unit test driving one poll_results iteration against a fake channel — sentinel present → asserts channel.send NOT called + archive ran — would lock the end-to-end behavior, not just source text.
No bound on sentinel-dir growth in pathological case (consistent archive failure). Edge case; flagging only.

Cross-PR linkages

#1046 (orphan .sending recovery) — complementary primitives at different layers: #1046 fixes "claimed-but-undelivered proactive files"; this PR fixes "delivered-but-unarchived result files." No textual conflict.
#1047 (REST catch-up) — both touch poll_results / on_ready at src/discord-bridge.py:2023+. Trivial merge conflict whichever lands second.
#1049 (attempts counter) — interlocks cleanly: #1049 = inbound idempotency signal, this PR = outbound idempotency enforcement.
#1050 (merged) — unblocks CI for this PR.
#1036 (belt-suspenders task-bridge guard) — this PR is its outbound twin. #1033 (per-channel pull namespace) — those results bypass poll_results entirely so no sentinel coverage there (correct call, different idempotency model on that path).

Tests

9 cases — sentinel touch / check / clear / mkdir-on-first-use / clear-idempotency / per-task isolation + 3 architectural source-grep assertions. Good coverage of the pure primitives.
The grep-assertions in test_poll_results_* are particularly nice — they pin the SEQUENCE (check before send, mark after send, clear after archive) by textual position rather than just presence. Catches the class of regression where someone moves a call to "clean up" the structure and breaks the protocol.
Missing (acknowledged): integration test of poll_results end-to-end. Fine to defer.

Ready to merge?

Rebase on main (picks up #1050), confirm CI green. Logic is correct; honest about scope; tests are well-shaped. Once green, ready.

— Lucy (Mac Studio bot)

sonichi

LGTM. Delivery-idempotency sentinel is the right tool for the crash-between-send-and-archive window. Three-step protocol (mark-after-send / check-before-send / clear-after-archive) ordered correctly. Builds on #1049's attempts: counter (just landed) — the agent now has both signal and the dedup primitive.

Honest about the residual narrow window (crash between send returning and the sentinel write) rather than papering over it. Per Lucy.

sonichi · 2026-05-23T13:17:33Z

@VasiliyRad — this PR now needs a rebase. After #1047 (your REST catch-up) merged, the discord-bridge.py functions in this PR conflict with the DM-checkpoint block in #1047. Both features are sound and the conflict is mechanical (both add functions in the same area). Quick fix on your end:

git fetch origin
git checkout fix/discord-bridge-delivery-idempotency-sentinel
git rebase origin/main
# resolve discord-bridge.py — keep both your delivery-sentinel block AND
# the DM-checkpoint block from main (they don't interact)
git push --force-with-lease

Approved on the design side (LGTM already posted). Ready to merge once you push.

…restart-safety) `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. `<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-between-send-and-sentinel-touch window remains a narrow double-send vector — Discord nonce-based dedup would close it tighter; deferred to follow-up. `poll_results` main path only. Channel-redirect (`[channel: <id>]`), proactive, and dm-fallback paths are scoped follow-ups. `tests/discord-bridge-delivery-sentinel.test.py` (9 cases): sentinel storage primitives + architectural source-grep wiring assertions (`_is_delivered` before first `channel.send`, `_mark_delivered` after send, `_clear_delivered` after archive). All 9 pass locally. Co-Authored-By: Vasiliy's Sutando <noreply@anthropic.com>

… + mtime-immutable + .sending contract + residual-hole) 5 corrections from qingyun-sutando's "approve with changes" review on #1074: 1. Fixed `<id>` path-template ambiguity. The task header is `id: task-<epoch-ms>` (already includes the `task-` prefix). Original wording used `results/task-<id>.txt` which would resolve to `results/task-task-<epoch>.txt` — double-prefixed. Real bug: every completed-but-unarchived task misses its DONE marker → misclassified as ORPHAN → spurious "needs review" recovery note sent to the original sender for work that already finished. That inverts the skill's purpose. Now uses `<id>` consistently across step 2/3 with an explicit note. 2. Age now comes from the IMMUTABLE arrival time (header `timestamp:` ISO field preferred, `task-<epoch-ms>` id fallback), NOT file mtime. Mtime gets reset by `sync-workspace.sh`, `git checkout`, any `touch`, etc. — making a genuinely old orphan look FRESH (<5 min) → left for the watcher → re-emitted → side effect re-fires. That's the exact bug this skill exists to prevent. 3. Added step 2b clarifying the `.sending` contract: `<id>.txt` (no suffix) = DONE (task processed + result body written), `<id>.txt.sending` = bridge mid-delivery (#1046/#1048 lifecycle). Treat .sending as DONE for orphan-check purposes — the discord-bridge owns its own startup `.sending` sweep, so we don't second-guess. 4. Named the known <5min residual hole explicitly under its own section, with a prioritized list of side-effect surfaces that still need markers (voice, phone, Telegram delivery, generic API/shell mutations). Was buried in "What it MIGHT need in the future" prose; now front-and-center as a known-residual-risk. 5. Noted the "promote to scripts/orphan-check.py" trigger — when the marker set grows past "marker-or-not + age-vs-5min", flip to deterministic script for testability. All 5 are in qingyun-sutando's review comment on #1074. Items 1 and 2 are real correctness bugs (would have produced wrong outcomes); items 3-5 are clarity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…ve to #1049/#1066) Per Chi 2026-05-23 Discord redesign exchange. Replaces the attempts-counter approach (#1049 + #1066's followup) with a startup-time classification pass that uses existing side-effect markers (PR #1048's `.sending` files for Discord delivery, result files in `results/`, archive presence) to decide what to do with each live task in `<workspace>/tasks/`. ## The bug being addressed If the agent crashes mid-task with non-idempotent side effects already executed (Discord message sent, file written, API call made) but the archive of result + task files never ran, on restart the task file is still in `tasks/`. The watcher re-emits. The agent re-processes. The side effect fires again. ## What #1049 tried Inline attempts-counter inside the task file. Watcher subscribes to Created+Renamed; bumper uses tmp+rename → fires Renamed → watcher re-emits → bumper bumps again. Infinite loop. ## What #1066 tried Switch tmp+rename to in-place `open(file, 'w')`. But on macOS, `O_WRONLY|O_CREAT|O_TRUNC` STILL fires the Created event because it flips the ItemCreated bit. Verified empirically 3x. Doesn't close the loop. ## What this PR does Move the dedup logic out of the watcher's event surface entirely. On startup (invoked as step 2 of /startup from PR #1072), the agent does ONE classification pass: - For each tasks/task-<id>.txt: - If results/task-<id>.txt OR results/archive/task-<id>.txt OR results/proactive-<id>.txt[.sending] exists → DONE, archive - Else if task arrived <5min ago → FRESH, leave for watcher - Else → ORPHAN, write recovery-sentinel result + archive task No counter inside the task file, no in-band writes, no bumper, no self-trigger loop. The watcher runs unchanged; just sees a smaller tasks/ dir. ## What this skill ships - skills/task-orphan-check/SKILL.md (~170 lines, mostly procedure prose) No bash/python script — the classification rules are LLM-judgment territory (cross-reference markers, age-relative-to-now, 3-outcome decision). A bash mirror would be a duplicate code path. Can promote later if workload grows. ## Followup (separate PRs) - Revert #1049: drop task_bump_attempts.py + the watcher's bump-on-emit hook + the `attempts:` field from task-file format (back-compat: agents ignore the field if present in older files). - Close #1066 as "superseded by /task-orphan-check." - Migration of operator-side launchd plists from /schedule-crons → /startup (#1072 follow-up; not blocking on this PR). ## Test plan - [ ] Fresh workspace (no tasks/) → orphan-check emits "no tasks/ dir, nothing to recover" - [ ] tasks/ with no live tasks → "no live tasks, nothing to recover" - [ ] Live task <5min old → classified FRESH, untouched - [ ] Task with results/task-<id>.txt present → classified DONE, archived - [ ] Task >5min old with no completion markers → classified ORPHAN, sentinel result written - [ ] Discord-delivered task: `.sending` lifecycle from #1048 read correctly Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

… + mtime-immutable + .sending contract + residual-hole) 5 corrections from qingyun-sutando's "approve with changes" review on #1074: 1. Fixed `<id>` path-template ambiguity. The task header is `id: task-<epoch-ms>` (already includes the `task-` prefix). Original wording used `results/task-<id>.txt` which would resolve to `results/task-task-<epoch>.txt` — double-prefixed. Real bug: every completed-but-unarchived task misses its DONE marker → misclassified as ORPHAN → spurious "needs review" recovery note sent to the original sender for work that already finished. That inverts the skill's purpose. Now uses `<id>` consistently across step 2/3 with an explicit note. 2. Age now comes from the IMMUTABLE arrival time (header `timestamp:` ISO field preferred, `task-<epoch-ms>` id fallback), NOT file mtime. Mtime gets reset by `sync-workspace.sh`, `git checkout`, any `touch`, etc. — making a genuinely old orphan look FRESH (<5 min) → left for the watcher → re-emitted → side effect re-fires. That's the exact bug this skill exists to prevent. 3. Added step 2b clarifying the `.sending` contract: `<id>.txt` (no suffix) = DONE (task processed + result body written), `<id>.txt.sending` = bridge mid-delivery (#1046/#1048 lifecycle). Treat .sending as DONE for orphan-check purposes — the discord-bridge owns its own startup `.sending` sweep, so we don't second-guess. 4. Named the known <5min residual hole explicitly under its own section, with a prioritized list of side-effect surfaces that still need markers (voice, phone, Telegram delivery, generic API/shell mutations). Was buried in "What it MIGHT need in the future" prose; now front-and-center as a known-residual-risk. 5. Noted the "promote to scripts/orphan-check.py" trigger — when the marker set grows past "marker-or-not + age-vs-5min", flip to deterministic script for testability. All 5 are in qingyun-sutando's review comment on #1074. Items 1 and 2 are real correctness bugs (would have produced wrong outcomes); items 3-5 are clarity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…1049 attempts-counter) (#1074) * feat(task-orphan-check): startup-time orphan-task recovery (alternative to #1049/#1066) Per Chi 2026-05-23 Discord redesign exchange. Replaces the attempts-counter approach (#1049 + #1066's followup) with a startup-time classification pass that uses existing side-effect markers (PR #1048's `.sending` files for Discord delivery, result files in `results/`, archive presence) to decide what to do with each live task in `<workspace>/tasks/`. ## The bug being addressed If the agent crashes mid-task with non-idempotent side effects already executed (Discord message sent, file written, API call made) but the archive of result + task files never ran, on restart the task file is still in `tasks/`. The watcher re-emits. The agent re-processes. The side effect fires again. ## What #1049 tried Inline attempts-counter inside the task file. Watcher subscribes to Created+Renamed; bumper uses tmp+rename → fires Renamed → watcher re-emits → bumper bumps again. Infinite loop. ## What #1066 tried Switch tmp+rename to in-place `open(file, 'w')`. But on macOS, `O_WRONLY|O_CREAT|O_TRUNC` STILL fires the Created event because it flips the ItemCreated bit. Verified empirically 3x. Doesn't close the loop. ## What this PR does Move the dedup logic out of the watcher's event surface entirely. On startup (invoked as step 2 of /startup from PR #1072), the agent does ONE classification pass: - For each tasks/task-<id>.txt: - If results/task-<id>.txt OR results/archive/task-<id>.txt OR results/proactive-<id>.txt[.sending] exists → DONE, archive - Else if task arrived <5min ago → FRESH, leave for watcher - Else → ORPHAN, write recovery-sentinel result + archive task No counter inside the task file, no in-band writes, no bumper, no self-trigger loop. The watcher runs unchanged; just sees a smaller tasks/ dir. ## What this skill ships - skills/task-orphan-check/SKILL.md (~170 lines, mostly procedure prose) No bash/python script — the classification rules are LLM-judgment territory (cross-reference markers, age-relative-to-now, 3-outcome decision). A bash mirror would be a duplicate code path. Can promote later if workload grows. ## Followup (separate PRs) - Revert #1049: drop task_bump_attempts.py + the watcher's bump-on-emit hook + the `attempts:` field from task-file format (back-compat: agents ignore the field if present in older files). - Close #1066 as "superseded by /task-orphan-check." - Migration of operator-side launchd plists from /schedule-crons → /startup (#1072 follow-up; not blocking on this PR). ## Test plan - [ ] Fresh workspace (no tasks/) → orphan-check emits "no tasks/ dir, nothing to recover" - [ ] tasks/ with no live tasks → "no live tasks, nothing to recover" - [ ] Live task <5min old → classified FRESH, untouched - [ ] Task with results/task-<id>.txt present → classified DONE, archived - [ ] Task >5min old with no completion markers → classified ORPHAN, sentinel result written - [ ] Discord-delivered task: `.sending` lifecycle from #1048 read correctly Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(task-orphan-check): qingyun-sutando review pass (id-double-prefix + mtime-immutable + .sending contract + residual-hole) 5 corrections from qingyun-sutando's "approve with changes" review on #1074: 1. Fixed `<id>` path-template ambiguity. The task header is `id: task-<epoch-ms>` (already includes the `task-` prefix). Original wording used `results/task-<id>.txt` which would resolve to `results/task-task-<epoch>.txt` — double-prefixed. Real bug: every completed-but-unarchived task misses its DONE marker → misclassified as ORPHAN → spurious "needs review" recovery note sent to the original sender for work that already finished. That inverts the skill's purpose. Now uses `<id>` consistently across step 2/3 with an explicit note. 2. Age now comes from the IMMUTABLE arrival time (header `timestamp:` ISO field preferred, `task-<epoch-ms>` id fallback), NOT file mtime. Mtime gets reset by `sync-workspace.sh`, `git checkout`, any `touch`, etc. — making a genuinely old orphan look FRESH (<5 min) → left for the watcher → re-emitted → side effect re-fires. That's the exact bug this skill exists to prevent. 3. Added step 2b clarifying the `.sending` contract: `<id>.txt` (no suffix) = DONE (task processed + result body written), `<id>.txt.sending` = bridge mid-delivery (#1046/#1048 lifecycle). Treat .sending as DONE for orphan-check purposes — the discord-bridge owns its own startup `.sending` sweep, so we don't second-guess. 4. Named the known <5min residual hole explicitly under its own section, with a prioritized list of side-effect surfaces that still need markers (voice, phone, Telegram delivery, generic API/shell mutations). Was buried in "What it MIGHT need in the future" prose; now front-and-center as a known-residual-risk. 5. Noted the "promote to scripts/orphan-check.py" trigger — when the marker set grows past "marker-or-not + age-vs-5min", flip to deterministic script for testability. All 5 are in qingyun-sutando's review comment on #1074. Items 1 and 2 are real correctness bugs (would have produced wrong outcomes); items 3-5 are clarity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

liususan091219

Cold review (MacBook node). Read the diff (+320/0), the existing send-then-archive flow at src/discord-bridge.py:2975-3195, and the marker-skip block at L2977-2983 to confirm the new sentinel doesn't collide with [no-send]/[REPLIED]/[deduped:].

Solid fix to a real correctness bug. The ordering is right:

Marker-skip continues at L2983 (untouched, sentinel never created for these).
New _is_delivered skip-block at L3028 — present sentinel → archive both files, clear sentinel, continue. Correct recovery.
_mark_delivered after all channel.send calls complete (including file attachments), before archive_file. Correct placement.
_clear_delivered after archive completes. Bounded growth.

DELIVERED_DIR = REPO / "state" / "discord-delivered" resolves to <workspace>/state/discord-delivered/ per resolve_workspace() — matches the workspace contract.

BLOCKING

None.

NON-BLOCKING

Sentinel orphans on archive failure. If archive_file raises (rare — permission flip, full disk), the sentinel stays forever for that task_id, and on a future restart it triggers the skip-path archive — which may still fail the same way. Bounded but messy. Suggest a startup-time sweep: any sentinel whose corresponding results/<task_id>.txt no longer exists is stale → unlink. Cheap, one-shot, prevents long-term dir bloat.
The narrow crash window between channel.send returning and _mark_delivered is honestly documented as a follow-up (Discord nonce). Fine for this PR. Worth filing the follow-up issue explicitly so it doesn't get lost — the PR body mentions it but a tracked issue is more durable than a comment line.
Docstring on DELIVERED_DIR could note that the marker-skip block ([no-send]/[REPLIED]/[deduped:]) at L2977 deliberately bypasses sentinel creation — readers tracing the file might wonder why those paths don't _mark_delivered. One-line comment, not blocking.
Scope confidence. "poll_results main path only — channel-redirect, proactive, dm-fallback are scoped follow-ups" is explicit and correct. Those paths each need their own analysis (channel-redirect in particular has its own send call). Mention in a follow-up issue list, otherwise they'll drift.

Approving — the design is right, the placement is correct, tests are mocked properly, and the remaining gaps are appropriately scoped.

— Maddy

…inel

…-idempotency (sonichi#1048) and shared allowlist (sonichi#1029) mkdir exist_ok, fail-open on exception, call order check (is_delivered < mark_delivered < clear_delivered), log message, _mark_delivered after channel.send in poll_results. SEND_ALLOWED_PREFIXES (/tmp/sutando-), is_path_sendable(), realpath traversal defense, isfile check, os.sep suffix check, discord-bridge imports from send_allowlist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…sonichi#1050) Matches telegram-bridge.py pattern. Allows test suites to inject a stub token via os.environ without needing a real ~/.claude/channels/ discord/.env file — unblocks CI for VasiliyRad PRs sonichi#1046 and sonichi#1048 whose tests already set the env var before importing the bridge. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Qingyun Wu <qingyun0327@gmail.com>

…onichi#1049 attempts-counter) (sonichi#1074) * feat(task-orphan-check): startup-time orphan-task recovery (alternative to sonichi#1049/sonichi#1066) Per Chi 2026-05-23 Discord redesign exchange. Replaces the attempts-counter approach (sonichi#1049 + sonichi#1066's followup) with a startup-time classification pass that uses existing side-effect markers (PR sonichi#1048's `.sending` files for Discord delivery, result files in `results/`, archive presence) to decide what to do with each live task in `<workspace>/tasks/`. ## The bug being addressed If the agent crashes mid-task with non-idempotent side effects already executed (Discord message sent, file written, API call made) but the archive of result + task files never ran, on restart the task file is still in `tasks/`. The watcher re-emits. The agent re-processes. The side effect fires again. ## What sonichi#1049 tried Inline attempts-counter inside the task file. Watcher subscribes to Created+Renamed; bumper uses tmp+rename → fires Renamed → watcher re-emits → bumper bumps again. Infinite loop. ## What sonichi#1066 tried Switch tmp+rename to in-place `open(file, 'w')`. But on macOS, `O_WRONLY|O_CREAT|O_TRUNC` STILL fires the Created event because it flips the ItemCreated bit. Verified empirically 3x. Doesn't close the loop. ## What this PR does Move the dedup logic out of the watcher's event surface entirely. On startup (invoked as step 2 of /startup from PR sonichi#1072), the agent does ONE classification pass: - For each tasks/task-<id>.txt: - If results/task-<id>.txt OR results/archive/task-<id>.txt OR results/proactive-<id>.txt[.sending] exists → DONE, archive - Else if task arrived <5min ago → FRESH, leave for watcher - Else → ORPHAN, write recovery-sentinel result + archive task No counter inside the task file, no in-band writes, no bumper, no self-trigger loop. The watcher runs unchanged; just sees a smaller tasks/ dir. ## What this skill ships - skills/task-orphan-check/SKILL.md (~170 lines, mostly procedure prose) No bash/python script — the classification rules are LLM-judgment territory (cross-reference markers, age-relative-to-now, 3-outcome decision). A bash mirror would be a duplicate code path. Can promote later if workload grows. ## Followup (separate PRs) - Revert sonichi#1049: drop task_bump_attempts.py + the watcher's bump-on-emit hook + the `attempts:` field from task-file format (back-compat: agents ignore the field if present in older files). - Close sonichi#1066 as "superseded by /task-orphan-check." - Migration of operator-side launchd plists from /schedule-crons → /startup (sonichi#1072 follow-up; not blocking on this PR). ## Test plan - [ ] Fresh workspace (no tasks/) → orphan-check emits "no tasks/ dir, nothing to recover" - [ ] tasks/ with no live tasks → "no live tasks, nothing to recover" - [ ] Live task <5min old → classified FRESH, untouched - [ ] Task with results/task-<id>.txt present → classified DONE, archived - [ ] Task >5min old with no completion markers → classified ORPHAN, sentinel result written - [ ] Discord-delivered task: `.sending` lifecycle from sonichi#1048 read correctly Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(task-orphan-check): qingyun-sutando review pass (id-double-prefix + mtime-immutable + .sending contract + residual-hole) 5 corrections from qingyun-sutando's "approve with changes" review on sonichi#1074: 1. Fixed `<id>` path-template ambiguity. The task header is `id: task-<epoch-ms>` (already includes the `task-` prefix). Original wording used `results/task-<id>.txt` which would resolve to `results/task-task-<epoch>.txt` — double-prefixed. Real bug: every completed-but-unarchived task misses its DONE marker → misclassified as ORPHAN → spurious "needs review" recovery note sent to the original sender for work that already finished. That inverts the skill's purpose. Now uses `<id>` consistently across step 2/3 with an explicit note. 2. Age now comes from the IMMUTABLE arrival time (header `timestamp:` ISO field preferred, `task-<epoch-ms>` id fallback), NOT file mtime. Mtime gets reset by `sync-workspace.sh`, `git checkout`, any `touch`, etc. — making a genuinely old orphan look FRESH (<5 min) → left for the watcher → re-emitted → side effect re-fires. That's the exact bug this skill exists to prevent. 3. Added step 2b clarifying the `.sending` contract: `<id>.txt` (no suffix) = DONE (task processed + result body written), `<id>.txt.sending` = bridge mid-delivery (sonichi#1046/sonichi#1048 lifecycle). Treat .sending as DONE for orphan-check purposes — the discord-bridge owns its own startup `.sending` sweep, so we don't second-guess. 4. Named the known <5min residual hole explicitly under its own section, with a prioritized list of side-effect surfaces that still need markers (voice, phone, Telegram delivery, generic API/shell mutations). Was buried in "What it MIGHT need in the future" prose; now front-and-center as a known-residual-risk. 5. Noted the "promote to scripts/orphan-check.py" trigger — when the marker set grows past "marker-or-not + age-vs-5min", flip to deterministic script for testability. All 5 are in qingyun-sutando's review comment on sonichi#1074. Items 1 and 2 are real correctness bugs (would have produced wrong outcomes); items 3-5 are clarity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…restart-safety) (sonichi#1048) `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. `<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-between-send-and-sentinel-touch window remains a narrow double-send vector — Discord nonce-based dedup would close it tighter; deferred to follow-up. `poll_results` main path only. Channel-redirect (`[channel: <id>]`), proactive, and dm-fallback paths are scoped follow-ups. `tests/discord-bridge-delivery-sentinel.test.py` (9 cases): sentinel storage primitives + architectural source-grep wiring assertions (`_is_delivered` before first `channel.send`, `_mark_delivered` after send, `_clear_delivered` after archive). All 9 pass locally. Co-authored-by: Vasiliy's Sutando <noreply@anthropic.com> Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>

…ask-metadata, and outbox labels Covers: - 22 tests for delivery-idempotency sentinels + send_allowlist (sonichi#1048/sonichi#1029) - 19 tests for task-file channel/guild metadata + inline-tools workspace scan (sonichi#1077/sonichi#1081) - 15 tests for outbox_log recipient_label (sonichi#1078) and submodule/worktree detection (sonichi#1076) All 56 tests pass against current main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…-idempotency (sonichi#1048) and shared allowlist (sonichi#1029) mkdir exist_ok, fail-open on exception, call order check (is_delivered < mark_delivered < clear_delivered), log message, _mark_delivered after channel.send in poll_results. SEND_ALLOWED_PREFIXES (/tmp/sutando-), is_path_sendable(), realpath traversal defense, isfile check, os.sep suffix check, discord-bridge imports from send_allowlist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ask-metadata, and outbox labels Covers: - 22 tests for delivery-idempotency sentinels + send_allowlist (sonichi#1048/sonichi#1029) - 19 tests for task-file channel/guild metadata + inline-tools workspace scan (sonichi#1077/sonichi#1081) - 15 tests for outbox_log recipient_label (sonichi#1078) and submodule/worktree detection (sonichi#1076) All 56 tests pass against current main. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…-idempotency (sonichi#1048) and shared allowlist (sonichi#1029) mkdir exist_ok, fail-open on exception, call order check (is_delivered < mark_delivered < clear_delivered), log message, _mark_delivered after channel.send in poll_results. SEND_ALLOWED_PREFIXES (/tmp/sutando-), is_path_sendable(), realpath traversal defense, isfile check, os.sep suffix check, discord-bridge imports from send_allowlist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

amkunte mentioned this pull request May 23, 2026

fix(discord-bridge): check DISCORD_BOT_TOKEN env var before .env file #1050

Merged

qingyun-wu reviewed May 23, 2026

View reviewed changes

qingyun-wu mentioned this pull request May 23, 2026

fix(discord-bridge): REST catch-up missed DMs on reconnect (restart-safety) #1047

Merged

1 of 2 tasks

liususan091219 mentioned this pull request May 23, 2026

fix(bridges): recover orphan .sending files on startup (restart-safety) #1046

Merged

1 of 2 tasks

liususan091219 reviewed May 23, 2026

View reviewed changes

liususan091219 mentioned this pull request May 23, 2026

feat(task-bridge): attempts counter on task files (restart-safety) #1049

Merged

1 of 2 tasks

sonichi approved these changes May 23, 2026

View reviewed changes

VasiliyRad force-pushed the fix/discord-bridge-delivery-idempotency-sentinel branch from 437b308 to ef9f1fa Compare May 23, 2026 16:46

sonichi mentioned this pull request May 23, 2026

feat(task-orphan-check): startup-time orphan-task recovery (replaces #1049 attempts-counter) #1074

Merged

6 tasks

liususan091219 approved these changes May 24, 2026

View reviewed changes

bassilkhilo-ag2 mentioned this pull request May 24, 2026

fix(discord-bridge): delivery-idempotency sentinel for poll_results (restart-safety #3) AG2Platform/stando#62

Open

1 of 2 tasks

Merge branch 'main' into fix/discord-bridge-delivery-idempotency-sent…

5a1247e

…inel

sonichi merged commit 33789fc into sonichi:main May 25, 2026
2 checks passed

qingyun-wu mentioned this pull request Jul 26, 2026

feat(gateway): drain results/proactive-*.txt to a configured room (REMOTE_PROACTIVE_ROOM) #2324

Merged

keweichen mentioned this pull request Aug 19, 2026

bug(discord): recreated task results are sent again after confirmed delivery #3172

Open

Navigation Menu

Repository navigation

fix(discord-bridge): delivery-idempotency sentinel for poll_results (restart-safety) - #1048

fix(discord-bridge): delivery-idempotency sentinel for poll_results (restart-safety)#1048
sonichi merged 2 commits into
sonichi:mainfrom
VasiliyRad:fix/discord-bridge-delivery-idempotency-sentinel

VasiliyRad commented May 23, 2026

Uh oh!

amkunte commented May 23, 2026

Uh oh!

qingyun-wu left a comment

Uh oh!

liususan091219 left a comment

Uh oh!

sonichi left a comment

Uh oh!

sonichi commented May 23, 2026

Uh oh!

liususan091219 left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

VasiliyRad commented May 23, 2026

The bug

Fix

Scope

Tests

Uh oh!

amkunte commented May 23, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

liususan091219 left a comment

Choose a reason for hiding this comment

Uh oh!

sonichi left a comment

Choose a reason for hiding this comment

Uh oh!

sonichi commented May 23, 2026

Uh oh!

liususan091219 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants