fix: avoid AceEvent shared-frame taint for combat log registration (#22)#26
Conversation
Replace addon:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED", ...) with a
private, unnamed CreateFrame("Frame") + OnEvent script. The shared
AceEvent30Frame can be tainted by upstream addons via CallbackHandler's
OnUsed lazy-registration path, which then blocks any first-time event
registration from clean addons with ADDON_ACTION_FORBIDDEN. Using a
private unnamed frame for CLEU isolates DragonShout from this class of
cross-addon taint.
The one-shot PLAYER_LOGIN registration in Core/Lifecycle.lua remains on
AceEvent - it's rarely the first registrant and degrades gracefully.
See .deliverables/tech-lead/ADR-0001-dragonshout-aceevent-taint.md
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdd runtime CLEU capability detection, gate CLEU behavior on ns.capabilities.cleuRestricted, move CLEU registration to private frames, add UnitEventListener to translate UNIT_* events into CLEU-shaped calls for restricted clients, and update listeners/docs/lint to accept translated extras. ChangesCLEU gating and translation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@DragonShout/Listeners/CombatLogListener.lua`:
- Around line 68-70: The CombatLogListener currently calls
frame:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED") unconditionally in
ns.CombatLogListener.Initialize which causes ADDON_ACTION_FORBIDDEN on Retail
12.0+; modify Initialize to check a capability flag (ns.capabilities.combatLog)
before registering and ensure that ns.capabilities is defined/initialized
(detect API availability in Init.lua during OnEnable where LISTENER_MODULES are
iterated), and if ns.capabilities.combatLog is false/absent, do not register the
combat log event and instead wire up a fallback path (create or reuse a
UnitEventListener fallback and call its Initialize from the same module); ensure
the check references ns.CombatLogListener.Initialize and the LISTENER_MODULES
loop so the gating covers the current initialization flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 8fe0550c-628c-4a3e-b278-2f6fa482400b
📒 Files selected for processing (1)
DragonShout/Listeners/CombatLogListener.lua
| function ns.CombatLogListener.Initialize(_addon) | ||
| frame:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED") | ||
| ns.DebugPrint("CombatLogListener initialized") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate capability routing and listener initialization =="
rg -n --type=lua -C3 'ns\.capabilities\.combatLog|CombatLogListener\.Initialize|UnitEventListener\.Initialize'
echo
echo "== Locate all CLEU registrations =="
rg -n --type=lua -C2 'RegisterEvent\("COMBAT_LOG_EVENT_UNFILTERED"\)'
echo
echo "Expected:"
echo "1) A selection branch on ns.capabilities.combatLog."
echo "2) combatLog=false path initializes UnitEventListener (not CombatLogListener)."
echo "3) No unconditional call path that reaches CombatLogListener.Initialize when combatLog=false."Repository: Xerrion/DragonShout
Length of output: 1360
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find Initialize calls in main addon files =="
rg -n --type=lua 'CombatLogListener\.Initialize|UnitEventListener\.Initialize' --glob='!Listeners'
echo
echo "== Check for capability gating near Initialize calls =="
rg -n --type=lua -B5 -A5 '(CombatLogListener|UnitEventListener)\.Initialize'
echo
echo "== List all Listener files =="
fd -t f '\.lua$' DragonShout/Listeners/Repository: Xerrion/DragonShout
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== List top-level addon files =="
ls -la DragonShout/*.lua 2>/dev/null || echo "No top-level lua files"
echo
echo "== Find all Lua files and structure =="
fd -t f '\.lua$' | head -20
echo
echo "== Search for ADDON_LOADED or main initialization =="
rg -n --type=lua 'ADDON_LOADED|DragonShout|RegisterAddon' | head -20
echo
echo "== Check if UnitEventListener exists =="
fd -t f 'UnitEventListener'Repository: Xerrion/DragonShout
Length of output: 2540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check Core/Init.lua =="
cat -n DragonShout/Core/Init.lua
echo
echo "== Check Core/Lifecycle.lua =="
cat -n DragonShout/Core/Lifecycle.lua
echo
echo "== Search for ns.capabilities =="
rg -n --type=lua 'ns\.capabilities'Repository: Xerrion/DragonShout
Length of output: 7746
Capability gating is missing — COMBAT_LOG_EVENT_UNFILTERED unconditionally registers without version/capability checks.
CombatLogListener is initialized unconditionally via the LISTENER_MODULES loop in Init.lua:OnEnable() (lines 117-122), with no gating on ns.capabilities.combatLog. On Retail 12.0+, this triggers ADDON_ACTION_FORBIDDEN. The addon needs either:
- A capability flag in
ns.capabilities.combatLogthat gates initialization, or - A UnitEventListener fallback when the capability is unavailable.
Currently neither exists — ns.capabilities is not defined, and UnitEventListener does not exist in the codebase.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@DragonShout/Listeners/CombatLogListener.lua` around lines 68 - 70, The
CombatLogListener currently calls
frame:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED") unconditionally in
ns.CombatLogListener.Initialize which causes ADDON_ACTION_FORBIDDEN on Retail
12.0+; modify Initialize to check a capability flag (ns.capabilities.combatLog)
before registering and ensure that ns.capabilities is defined/initialized
(detect API availability in Init.lua during OnEnable where LISTENER_MODULES are
iterated), and if ns.capabilities.combatLog is false/absent, do not register the
combat log event and instead wire up a fallback path (create or reuse a
UnitEventListener fallback and call its Initialize from the same module); ensure
the check references ns.CombatLogListener.Initialize and the LISTENER_MODULES
loop so the gating covers the current initialization flow.
There was a problem hiding this comment.
@coderabbitai Thanks for the catch — the ns.capabilities.combatLog and UnitEventListener references you flagged are stale documentation, not a missing capability gate. They came from abandoned PR #24 (fix/22-cleu-midnight-migration), which misdiagnosed issue #22 as a Retail 12.0 HasRestrictions = true registration block. That PR was closed unmerged.
The actual root cause of #22 is unrelated to Midnight: AceEvent-3.0 shares a single AceEvent30Frame (one CallbackHandler dispatch frame across every addon that embeds AceEvent), so cross-addon taint accumulates on it and triggers ADDON_ACTION_FORBIDDEN when CLEU is registered through AceEvent:RegisterEvent. The fix in this PR — registering COMBAT_LOG_EVENT_UNFILTERED on a private unnamed CreateFrame("Frame") — sidesteps the shared frame entirely. No capability gate or unit-event fallback is needed; verified against current FrameXML annotations and the CLEU wiki history (no 11.x/12.x restriction entries; HasRestrictions = true is the longstanding 'no varargs, use CombatLogGetCurrentEventInfo()' flag, not a registration block).
I've just pushed a follow-up commit cleaning up the stale AGENTS.md prose so it matches the actual fix.
There was a problem hiding this comment.
@Xerrion, thanks for the detailed correction — you're right on all counts.
The cross-addon taint cascade via AceEvent30Frame is a well-documented real-world issue where ADDON_ACTION_FORBIDDEN is thrown against AceEvent30Frame:RegisterEvent() through the OnUsed path in CallbackHandler-1.0 — exactly as you described. The error can appear attributed to a completely innocent addon because the shared AceEvent30Frame was already tainted by a different addon's execution context. The private unnamed CreateFrame("Frame") pattern is also consistent with Wowpedia's own recommended example code for registering COMBAT_LOG_EVENT_UNFILTERED.
My concern about ns.capabilities.combatLog and UnitEventListener was based on artifacts from PR #24 that no longer exist in the codebase. The fix here is correct and sufficient for the stated root cause. I'm withdrawing that comment.
One minor heads-up worth tracking separately (not a blocker for this PR): the combat log system was fundamentally changed in 12.0.0 (Midnight) — COMBAT_LOG_EVENT_UNFILTERED is no longer available to addons, and CombatLogGetCurrentEventInfo() is unavailable to tainted code. The private frame won't help there since the event won't fire at all on Midnight. That's a separate migration concern for whenever you need to support 12.0+, not for this PR.
✏️ Learnings added
Learnt from: Xerrion
Repo: Xerrion/DragonShout PR: 26
File: DragonShout/Listeners/CombatLogListener.lua:68-70
Timestamp: 2026-05-12T07:50:03.252Z
Learning: In DragonShout (WoW addon, Lua), the ADDON_ACTION_FORBIDDEN taint error on COMBAT_LOG_EVENT_UNFILTERED registration is caused by AceEvent-3.0's shared AceEvent30Frame getting tainted by cross-addon execution context (via CallbackHandler-1.0's OnUsed lazy registration path), not by a Retail 12.0 HasRestrictions flag. The correct fix is to register CLEU on a private unnamed CreateFrame("Frame") owned by the addon instead of using addon:RegisterEvent() through AceEvent.
Learnt from: Xerrion
Repo: Xerrion/DragonShout PR: 26
File: DragonShout/Listeners/CombatLogListener.lua:68-70
Timestamp: 2026-05-12T07:50:03.252Z
Learning: In DragonShout/Listeners/CombatLogListener.lua, HasRestrictions=true on COMBAT_LOG_EVENT_UNFILTERED is the longstanding "no varargs, use CombatLogGetCurrentEventInfo()" flag — it is NOT a registration block in pre-12.0 clients. COMBAT_LOG_EVENT_UNFILTERED is genuinely removed for addon code only in WoW 12.0.0 (Midnight). Do not flag private unnamed frame registration of CLEU as needing a capability gate for current (pre-Midnight) retail/MoP Classic/TBC.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Cleanup leftover design notes from abandoned PR #24 that documented a ns.capabilities.combatLog gate and UnitEventListener module which were never merged. PR #26 fixes #22 via a private CreateFrame frame to escape shared AceEvent30Frame taint; the prior Midnight/HasRestrictions diagnosis was incorrect.
|
@coderabbitai — flagging this for the learnings table. Update: PR scope vs. user-visible symptom on retail MidnightAfter more digging into why issue #22 happens on retail 12.0.5 specifically, there's a second platform-level factor in play that this PR does not address. The PR is still correct and still mergeable as-is, but it's worth being explicit about what it does and doesn't fix. What this PR fixes (still valid)The Ace3 shared-bus taint problem is real:
What this PR does NOT fix (new finding)Retail Midnight (interface 120000+, pre-patch 2026-01-28, release 2026-03-02) removed Sources:
Real-world impact: Details! ships a Net effect for the reporter in #22: after this PR lands, the error stops spamming on login, but interrupt/CC/dispel announcements still won't function inside raids or Mythic+ on retail because CLEU won't fire there at all. Classic flavors are unaffected and benefit fully from this PR. Note re: CodeRabbit's withdrawn commentCodeRabbit's withdrawn comment on this PR had a buried claim that CLEU wouldn't fire on Midnight. The stated rationale ( Follow-upTracked in #28 — capability-gated migration to Recommend merging this PR as-is — it's a strict improvement on every flavor — and treating #28 as the separate retail-Midnight functional fix. |
|
Tip For best results, initiate chat on the files or code changes.
✏️ Learnings added
|
Retail patch 12.0 (Midnight, Interface 120000+) suppresses COMBAT_LOG_EVENT_UNFILTERED for addons - the event registers but never fires. Route interrupt and CC-on-player detection through UNIT_* events on affected clients while leaving Classic (MoP, TBC Anniversary) on the identical CLEU code path. Per ADR-0002: - New Core/Capabilities.lua exposes ns.capabilities.cleuRestricted, set at file scope from (C_RestrictedActions ~= nil) AND mainline build >= 120000. - New Listeners/UnitEventListener.lua: private unnamed CreateFrame (mirrors PR #26 anti-taint pattern), registers UNIT_SPELLCAST_INTERRUPTED and UNIT_AURA. Translates payloads into the CLEU-shaped positional tuple the existing handlers consume, plus a trailing 'extras' table for spell info (CombatLogGetCurrentEventInfo is not the source of truth on Midnight). - CombatLogListener.Initialize/Shutdown self-gate on cleuRestricted - no CLEU registration on retail Midnight. - InterruptListener.OnInterrupt and AuraListener.OnAuraApplied gain an optional trailing 'extras' parameter. When nil (Classic forwarding via CombatLogListener), behavior is unchanged. - Both dispatchers are always in LISTENER_MODULES; each self-gates on ns.capabilities inside its own Initialize. Dropped on retail Midnight (no clean attribution event): - dispels: DispelListener stays on disk but is never invoked. - ccApplied (CC the player lands on others): sourceUnit on AuraData is unreliable. Files added: Core/Capabilities.lua, Listeners/UnitEventListener.lua. Files modified: Core/Init.lua, DragonShout.toc, .luacheckrc, Listeners/{CombatLogListener,InterruptListener,AuraListener}.lua. Refs: #28, ADR-0002
… (#29) Retail patch 12.0 (Midnight, Interface 120000+) suppresses COMBAT_LOG_EVENT_UNFILTERED for addons - the event registers but never fires. Route interrupt and CC-on-player detection through UNIT_* events on affected clients while leaving Classic (MoP, TBC Anniversary) on the identical CLEU code path. Per ADR-0002: - New Core/Capabilities.lua exposes ns.capabilities.cleuRestricted, set at file scope from (C_RestrictedActions ~= nil) AND mainline build >= 120000. - New Listeners/UnitEventListener.lua: private unnamed CreateFrame (mirrors PR #26 anti-taint pattern), registers UNIT_SPELLCAST_INTERRUPTED and UNIT_AURA. Translates payloads into the CLEU-shaped positional tuple the existing handlers consume, plus a trailing 'extras' table for spell info (CombatLogGetCurrentEventInfo is not the source of truth on Midnight). - CombatLogListener.Initialize/Shutdown self-gate on cleuRestricted - no CLEU registration on retail Midnight. - InterruptListener.OnInterrupt and AuraListener.OnAuraApplied gain an optional trailing 'extras' parameter. When nil (Classic forwarding via CombatLogListener), behavior is unchanged. - Both dispatchers are always in LISTENER_MODULES; each self-gates on ns.capabilities inside its own Initialize. Dropped on retail Midnight (no clean attribution event): - dispels: DispelListener stays on disk but is never invoked. - ccApplied (CC the player lands on others): sourceUnit on AuraData is unreliable. Files added: Core/Capabilities.lua, Listeners/UnitEventListener.lua. Files modified: Core/Init.lua, DragonShout.toc, .luacheckrc, Listeners/{CombatLogListener,InterruptListener,AuraListener}.lua. Refs: #28, ADR-0002
1 participant
Summary
Fixes the
ADDON_ACTION_FORBIDDENtaint error fired against DragonShout when registering forCOMBAT_LOG_EVENT_UNFILTEREDvia AceEvent-3.0.Closes #22
The bug
Users hit:
…even though DragonShout itself does nothing protected. The
AceEvent30Frameis shared across every Ace3 addon in the session, andCallbackHandler-1.0's lazyOnUsedpath registers the underlying WoW event on that shared frame the first time any consumer subscribes. If an upstream addon taints that path before DragonShout's first registration, the next call - ours - blows up.This is a known, widespread issue affecting ~55 Ace3-based addons since January 2026 (CallbackHandler / shared AceEvent30Frame interaction).
Root cause
AceEvent30Frameis one process-wide frame shared by all Ace3 addons.CallbackHandler-1.0'sOnUsedcallback callsframe:RegisterEvent(event)lazily on first subscription.RegisterEventcalls from clean addons - including DragonShout's CLEU registration - are then blocked by Blizzard's secure-execution guard.The fix
Replace
addon:RegisterEvent("COMBAT_LOG_EVENT_UNFILTERED", handler)inDragonShout/Listeners/CombatLogListener.luawith a private, unnamedCreateFrame("Frame")plus anOnEventscript. The frame is owned exclusively by DragonShout's CLEU listener - no shared CallbackHandler path, no shared frame, no cross-addon taint vector.What is intentionally NOT changed
Core/Lifecycle.luastill uses AceEvent for the one-shotPLAYER_LOGINregistration. It runs early in the addon lifecycle, is rarely the first registrant on the shared frame, and degrades gracefully if it ever did fail. Keeping it on AceEvent preserves the standard Ace3 lifecycle conventions for everything that isn't the high-volume CLEU path.See
.deliverables/tech-lead/ADR-0001-dragonshout-aceevent-taint.mdfor the full design record (local working artifact, not committed).Verification
luacheck .- 0 warnings, 0 errorsNotes
Summary by CodeRabbit
Refactor
New Features
Behavior
Documentation