You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(tool): Handle tool approval timeout state - #755
This completed future cannot prove that its completion was awaited: the callback appends onToolFinishedAwaitable before returning, so the asserted order remains identical if production code invokes the callback and ignores its future. Return a controllable/instrumented incomplete future and assert that its get/completion occurs before onToolApprovalTimedOut to make this regression test exercise the synchronization guarantee it documents.
- Ensures that the tool approval awaitable is properly completed regardless of the exit path.
- Implements robust timeout handling within `SessionManager` to clear state on timeout.
- Exposes a dedicated callback in `ChatClient` for managing tool completion promises.
BREAKING CHANGE: The `sendStreamingMessageWithCallback` function now accepts `onToolFinishedAwaitable` to explicitly manage the completion of the tool's pending promise.
awaitableInvoked is released inside the callback before it returns, so observing callTask.isDone == false does not prove that the returned future was awaited; an implementation that ignores the future can still be between countDown() and task completion and pass once this test completes the future. Conversely, a delayed test thread can let the correct one-second get time out before this assertion. Use an instrumented CompletableFuture whose timed get signals a second latch, and assert that latch instead of task timing.
This cap now counts completed threads that are intentionally retained until the ViewModel consumes them. Because the stale-entry replacement happens only later, a retry can be rejected when the map has 20 entries even though one or more are complete (including the same session, which the previous code removed before this check). Count only incomplete threads here so stale bookkeeping entries do not consume streaming capacity.
Make awaiting-state assertion deterministic with a controlled future
This check still has an elapsed-time race: the implementation waits on future for only 1 second, so a CI pause after awaitableInvoked is released can let that wait time out and complete callTask before isDone is inspected. A test-controlled CompletableFuture whose timed get blocks on a release latch would make the “currently awaiting” assertion deterministic.
Test sibling completion notification when approval request times out
This setup does not exercise the scenario described by the test: only request A's beforeToolExecution runs, and A itself is the request that times out. Make A a non-approval tool, invoke both starts, and require approval only for B; then the assertion verifies that an already-started sibling receives its completion notification when B times out.
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
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.
haiphucnguyen
left a comment
Summary
onToolApprovalTimedOutcallback to tool streaming workflows.Related Issue(s)
Type of Change
What Changed & Why
How to Test
./gradlew testScreenshots / Recordings (if applicable)
Checklist
main./gradlew spotlessApplyand./gradlew detektlocally./gradlew testand all tests passgit commit -sAdditional Notes