test: multi-handler ordering regression in ExternalActionPlugin#838
Draft
shahabdsh wants to merge 1 commit intoplayer-ui:mainfrom
Draft
test: multi-handler ordering regression in ExternalActionPlugin#838shahabdsh wants to merge 1 commit intoplayer-ui:mainfrom
shahabdsh wants to merge 1 commit intoplayer-ui:mainfrom
Conversation
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. Previously (transition hook + setTimeout), if the first handler resolved synchronously, the second handler was never invoked because the player had already transitioned away from the external state. After the refactor to afterTransition, both handlers are always invoked regardless, which can cause unexpected side effects in the second handler. These tests document the expected behavior: a synchronously-resolving first handler should prevent the second handler from executing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Summary
Adds tests demonstrating a behavioral regression introduced in #832 (refactor of ExternalActionPlugin from
transition+setTimeouttoafterTransition).The issue
When multiple
ExternalActionPlugininstances are registered on the same Player (common in plugin architectures where a host and embedded component each register their own handler):setTimeoutcallback ran.afterTransitionfires both synchronously in the same tick, and both capturecurrentStatebefore either has transitioned.This means handlers that have side effects (logging, analytics, network calls, state mutations) now execute even when another handler has already resolved the external state — which can cause unexpected behavior in plugin architectures.
Test cases
Suggested fix
The
afterTransitionapproach should check whether the external state has already been resolved before invoking the handler, or use a coordination mechanism (e.g., a flag on the flow instance) to prevent multiple handlers from racing.🤖 Generated with Claude Code