Repository navigation
fix(discord-bridge): #1854 follow-up — DM-fallback source gate fails CLOSED on missing source - #1913
Merged
Merged
Conversation
…CLOSED on missing source Post-merge codex audit flagged the residual leak: the gate skipped only when a source WAS readable and non-allowlisted, so a result whose task file was swept/unreadable DMed regardless of origin. Factor the decision into _dm_fallback_eligible() (missing/unreadable source -> NOT eligible) and wire poll_dm_fallback through it. The existing regression test now execs the REAL production function (was a replica) and the missing-source case asserts fail-closed. 9/9 tests pass.
…ale lives in the PR body)
Contributor
|
@cla-assistant check |
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.
Why this must change — the residual leak path in #1854:
#1854 gates DM delivery by reading the task file's source: field (only DM-origin tasks may DM). But when the task file is GONE, the source is unreadable, and the old code treated unknown-source as DM-eligible (fail-open). The task file goes missing in real, common timings: (a) the bridge restarts and re-scans results/ after the task file was archived; (b) a long-running task's result lands after the task file was swept. In exactly those messy windows, a web-chat (or any non-DM) result would leak into the owner's Discord DM again — the very bug #1854 fixed, re-entering through the back door.
The fix is one decision flip: unreadable/missing source -> NOT DM-eligible (fail-closed), factored into _dm_fallback_eligible() and wired through poll_dm_fallback. The cost asymmetry justifies closed-by-default: a wrongly-skipped result merely doesn't ping (it still surfaces via the retention sweep and stays in results/); a wrongly-sent one actively pushes wrong content at the owner. Every current task writer sets source:, so the closed default changes nothing on the happy path.
Test updated per the new test standard (pr-triage skill 3.3): the regression test now extracts and EXECUTES the real production function (previously it asserted against an inline replica — a production regression would not have failed it), and the missing-source case asserts fail-closed. 9/9 pass.
Post-merge codex audit finding on #1854; tracked in #1906.