fix(discord): surface result delivery failures

[john-the-dev]

What changed, and why?

Discord result delivery now handles API exceptions as a real failed-delivery outcome instead of a console-only log:

• writes a failed row to result-audit.log
• emits channel.discord.out with outcome=error and the task/error details
• sends the Result Router §9.3 failure notice to the owner DM
• preserves the original exception in the notice so failures such as HTTP 413 are actionable

This fixes the observed path where reply text was sent first, an oversized video upload then failed, and the bridge archived the task/result without telling the owner.

Review first

• src/discord-bridge.py: _report_delivery_failure() and the poll_results() exception hook
• tests/discord-bridge-delivery-failure-visible.test.py: end-to-end poller regression

User behavior proved

Before: Discord showed success-looking text but no attachment; only the bridge console contained Reply failed: 413 Payload Too Large.

After: the same simulated attachment rejection produces an explicit owner-visible failure notice naming the task and error, a failed audit row, and an error observability event. It never records the task as delivered.

Verification

Manual live reproduction on unpatched main:

• Task task-1784486069331 sent text, then the 17 MB MP4 upload failed.
• The live sutando-discord pane recorded: Reply failed: 413 Payload Too Large (error code: 40005): Request entity too large.
• The task/result were archived, while the owner received no failure notice.
• Re-sending through task-1784556712838 reproduced the same 413 and silent archive behavior.

Failing-before regression on origin/main:

$ python3 tests/discord-bridge-delivery-failure-visible.test.py
Reply failed: 413 Payload Too Large
AssertionError: attachment failure was silent — no owner-visible failure notice

Passing-after:

$ python3 tests/discord-bridge-delivery-failure-visible.test.py
Reply failed: 413 Payload Too Large
[delivery-failure] owner notified for task-1784556770535: 413 Payload Too Large
PASS — Discord attachment failures are visible, audited, and observable

Additional passing checks:

python3 tests/bridge-audit-wiring.test.py
python3 tests/discord-bridge-delivery-sentinel.test.py
python3 tests/discord-bridge-file-markers.test.py
python3 tests/result-router-fallback.test.py
git diff --check

Edge cases

• Owner DM failures reuse the originating DM, avoiding an unnecessary lookup.
• Channel and non-owner task failures resolve the canonical owner through the existing Discord owner-config chain.
• Failure-notice delivery is itself guarded so a second Discord/config error cannot terminate the bridge.
• Unknown tiers remain unknown; they are never silently upgraded to owner.

Migrations, permissions, rollback, and risks

• Migrations/config/permission changes: N/A.
• Rollback: revert the single commit.
• Risk: one additional owner DM is sent only when a result delivery has already failed. Successful delivery behavior is unchanged.

Known gaps

• This PR surfaces the API limit failure; it does not automatically transcode or resize oversized attachments.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 58%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/discord-bridge.py (100%)

Summary

• Total: 39 lines
• Missing: 0 lines
• Coverage: 100%

[john-the-dev]

Author follow-up from the two-week sweep: CI is blocked only by the Python diff-coverage gate (38.5%; uncovered delivery-failure fallback/owner-DM branches around src/discord-bridge.py:3664-3700). The full test suite and other gates pass. Add focused async tests for those branches before re-requesting review.

[qingyun-wu]

Approve. The bug is real (silent archive after a 413 attachment failure), the before/after is a genuine live reproduction on main, and the test exercises the real path rather than asserting through a mock.

I focused verification on the one thing that can turn a failure-notifier into a new failure mode: does the notice path itself stay contained? It does, and I checked each layer rather than trusting the docstring:

• The DM-send block (fetch_user → create_dm → send) is inside try/except Exception as notice_error, so an owner DM that also 413s is logged, not raised.
• _emit_channel is separately wrapped.
• result_audit.record() runs before those try blocks, which briefly worried me — but it's self-guarded (src/result_audit.py:40, "Never raises", full try/except pass). So the helper's "deliberately never raises" contract actually holds end-to-end, which matters because the call site is inside poll_results's own except — a raise there would propagate into the handler that invoked it.

The only lines in the helper outside a guard are the DeliveryFailure(...) dataclass construction and str(error). Both are effectively non-raising; not worth changing, noting only so the "never raises" claim is understood as "in practice" rather than "structurally enforced."

Test quality — the assert channel.file_attempts == 1 ("the attachment failure was not exercised") is the load-bearing guard: without it, a test that stopped triggering the 413 would still pass every other assertion and silently stop testing the bug. That's exactly the fixture-trap that's easy to ship, and it's guarded here. The ledger assertions check both the presence of failed and the absence of delivered, which is the right pair.

Consistent with the delivery-visibility theme across the current bridge PRs (#2216, #2226). Nice to have the 413 → owner-visible path closed on discord.

[sonichi]

Reviewed safe on the cold-review pass: surface discord delivery failures — deps confirmed on main.

Approval executed by Sutando-Pro (Stand: Echo Act IV Pro) at @sonichi's explicit direction on the cold-review shortlist.

[github-actions]

@cla-assistant check