Skip to content

test: verify multi-handler ordering with reverted ExternalActionPlugin#839

Draft
shahabdsh wants to merge 2 commits intoplayer-ui:mainfrom
shahabdsh:test/external-action-multi-handler-ordering-pre-refactor
Draft

test: verify multi-handler ordering with reverted ExternalActionPlugin#839
shahabdsh wants to merge 2 commits intoplayer-ui:mainfrom
shahabdsh:test/external-action-multi-handler-ordering-pre-refactor

Conversation

@shahabdsh
Copy link
Copy Markdown
Contributor

Summary

Companion to #838. This PR reverts #832 (ExternalActionPlugin refactor) and adds the same multi-handler ordering tests to demonstrate that all 3 tests pass with the pre-refactor implementation.

What this PR contains

  1. Revert of Refactor external action plugin #832 — restores the transition hook + setTimeout(0) implementation
  2. Same test file as test: multi-handler ordering regression in ExternalActionPlugin #838 — 3 tests for multi-handler ordering behavior

Why the tests pass here but fail on main

With the original setTimeout(0) approach:

  • Each handler runs in its own deferred microtask
  • If the first handler resolves synchronously, it transitions within its setTimeout callback
  • The second handler's setTimeout callback finds the player has already moved away from the external state → second handler is never invoked

With the afterTransition approach (#832):

  • Both handlers fire synchronously on the same hook call
  • Both capture currentState at the same moment, before either transitions
  • Both handlers are always invoked, even if the first one resolves synchronously

This behavioral difference affects plugin architectures where multiple ExternalActionPlugin instances are registered — the second handler's side effects now fire even when the first handler has already resolved the external state.

Test results

PR Test 1 (sync) Test 2 (async race) Test 3 (delegate)
#838 (main) FAIL PASS PASS
This PR (reverted) PASS PASS PASS

🤖 Generated with Claude Code

shahabdsh and others added 2 commits April 3, 2026 22:18
When multiple ExternalActionPlugin instances are registered (e.g. in a
plugin architecture where a host and embedded component each register
their own handler), the interaction between handlers matters.

With the pre-refactor implementation (transition hook + setTimeout), if
the first handler resolves synchronously, the second handler is never
invoked because the player has already transitioned away from the
external state. This is the correct behavior — a resolved external state
should not trigger additional handler side effects.

These tests document and verify this behavior. All 3 tests pass with the
reverted implementation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@shahabdsh shahabdsh requested a review from a team as a code owner April 4, 2026 02:19
@shahabdsh shahabdsh marked this pull request as draft April 4, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant