Sunshine 9600414002
fix(channels): retain strong reference to Discord ack-reaction tasks (#5049)
* fix(channels): retain strong reference to Discord ack-reaction tasks

The event loop keeps only weak references to scheduled tasks, so the bare
asyncio.create_task(self._add_reaction(message)) at both message-acceptance
sites could be garbage-collected mid-flight and silently drop the 
acknowledgment. The module-level retention set holds each task until
completion and logs failures, mirroring the deferred subagent cleanup fix
in #4928 and the stream bridge cleanup fix in #4931.

* fix(channels): drain in-flight ack reactions on Discord channel shutdown

Adversarial follow-up to the retention fix: stop() cancelled typing tasks
but left in-flight ack-reaction tasks in the module retention set when the
channel stopped mid-HTTP-call, pinning the channel instance and the discord
Message object graph across restart cycles. _cancel_ephemeral_tasks /
_discard_ephemeral_tasks now cover both families at every stop() cleanup
point and the _run_client finally. Also tests the done-callback error-log
branch and documents the GC test as a contract pin.

* test(channels): pin Discord ack-reaction drain wiring and sharpen wording

Second adversarial pass found the drain test cancelled a never-started
task (no yield between schedule and cancel), nothing exercised the real
stop() cross-loop wiring, and the cleanup log messages still said
"typing-task" while now covering both families. The drain test now yields
so the task genuinely suspends first, a new test drives stop() through a
real background loop and fails if the wiring reverts to typing-only
cleanup, wording covers ephemeral tasks, and the discard path snapshots
the set before iterating.

* refactor(channels): scope Discord ack-reaction retention to the channel instance

The retention set was module-level, so one DiscordChannel's stop() would
cancel every other instance's in-flight ack reactions. Instance-level
matches the typing-task bookkeeping and keeps shutdown scoped; pinned by
a two-channel isolation test.

* test(channels): drive two-channel ack isolation through full stop()

Independent review of the instance-scoping refactor confirmed the P2 fix
but suggested matching the review's letter exactly: the isolation test now
shuts the first channel down via stop() instead of the drain helper, and
one stale docstring reference to the former module-level set is corrected.
2026-08-27 17:56:56 +08:00
..