fix(agui): share resume coordination state across replicas - #2890
Open
ordinary-s wants to merge 3 commits into
Open
fix(agui): share resume coordination state across replicas#2890ordinary-s wants to merge 3 commits into
ordinary-s wants to merge 3 commits into
Conversation
Contributor
Author
|
The CI failure appears to be caused by the known flaky @tempdir cleanup race in agentscope-harness (DirectoryNotEmptyException, tracked in #2251). This PR does not modify the agentscope-harness module, and the failure occurred before the AG-UI/Redis modules were reached in the reactor build. Could someone please re-run the failed job when convenient? |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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
Make AG-UI resume coordination shareable across replicas so a resume request can be validated and claimed even when it reaches a different application instance.
Root cause
AguiResumeCoordinatorkept pending interrupt metadata and active-run ownership in process-local maps. In a multi-replica deployment, the replica receiving a later resume request could not see the state recorded by the original replica. Concurrent requests routed to different replicas could also both acquire ownership because each process made the decision independently.Implementation
AguiResumeStateStoreSPI for reading pending interrupts, atomically claiming/releasing a thread run, and owner-checked pending-state replacement.InMemoryAguiResumeStateStore.RedisAguiResumeStateStore, using one Redis hash per thread and Lua scripts for atomic ownership and pending-state transitions.AguiRequestProcessor.Builderand automatically consume an optional store bean in the MVC and WebFlux Spring Boot adapters.RUN_FINISHEDevents, Redis command/script behavior, and Spring bean wiring.Testing
mvn -pl :agentscope-extensions-agui -am -Dtest=AguiResumeCoordinatorTest,AguiRequestProcessorTest -Dsurefire.failIfNoSpecifiedTests=false test— 38 tests passed, 0 failures, 0 errors, 0 skipped.mvn -pl :agentscope-extensions-redis -am -Dtest=RedisAguiResumeStateStoreTest -Dsurefire.failIfNoSpecifiedTests=false test— 5 tests passed, 0 failures, 0 errors, 0 skipped.mvn -pl :agentscope-agui-spring-boot-starter -am -Dtest=AguiAdapterConfigAutoConfigurationTest -Dsurefire.failIfNoSpecifiedTests=false test— 19 tests passed, 0 failures, 0 errors, 0 skipped.mvn -pl :agentscope-extensions-agui -am -Dtest=Agui*Test,AgentResolverTest,SubagentEventConverterTest,AgentLifecycleEventConverterTest,ExternalAgentEventConverterTest -Dsurefire.failIfNoSpecifiedTests=false test— 500 tests passed, 0 failures, 0 errors, 0 skipped.mvn spotless:check— passed across all 89 reactor modules.git diff --check— passed.Known limitation
A hard process crash while Redis ownership is held may leave a stale claim. This implementation intentionally avoids a fixed TTL because a valid long-running request could outlive it and permit dual ownership. Lease renewal with fencing tokens should be addressed in a follow-up.
Fixes #2773