fix(sessions): notify cloud turn completions in either log order#3682
Merged
Conversation
|
😎 Merged successfully - details. |
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
charlesvien
marked this pull request as ready for review
July 22, 2026 08:23
Contributor
|
Reviews (1): Last reviewed commit: "keep cloud turn armed until turn_complet..." | Re-trigger Greptile |
charlesvien
marked this pull request as draft
July 22, 2026 08:26
charlesvien
marked this pull request as ready for review
July 22, 2026 08:26
Contributor
|
Reviews (2): Last reviewed commit: "keep cloud turn armed until turn_complet..." | Re-trigger Greptile |
Member
|
meep |
Contributor
|
/trunk merge |
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.
Problem
Since #3572, desktop notifications for cloud task completions only fire when
_posthog/turn_completehappens to land before thesession/promptresponse in the log stream. The two writes race in the agent's log stream, so roughly half of all cloud completions never notify at all: no banner, no sound, not even later.The
completedActiveTurngate from #3572 treats a turn as already notified oncecurrentPromptIdis cleared, but the prompt response also clears it. When the response lands first, the turn is disarmed beforeturn_completearrives and the notification is suppressed. The old comment claiming "Cloud sessions never see that response" is wrong: the agent taps both directions of the ACP stream into the log, and a survey of 33 completed turns in real session logs found 14 with the response first and 19 withturn_completefirst.Note: this is one of two issues behind the "notifs don't fire until I refocus the app" report. The other (notifications bursting on refocus instead of arriving while the app is backgrounded) is a delivery stall around App Nap and the cloud watcher's permanent give-up, and needs a separate fix.
Changes
Cloud sessions no longer disarm the turn on the
session/promptresponse; only_posthog/turn_completedoes. The response still clearsisPromptPending, so spinner behavior is unchanged, and local sessions are untouched. Theturn_completehandler now reads the entry's realstopReasoninstead of hardcodingend_turn, so cancelled or failed turns no longer ring or speak "done". Keeping the turn start time intact untilturn_completealso fixes completion sound duration scaling for cloud tasks, which the response previously wiped.How did you test this?
cloudTaskUpdateNotifications.test.ts: response before and afterturn_complete, duplicateturn_completeafter a response-first turn, multi-turn with mixed orderings, a cancelled turn and a duration assertion.sessionService.ts(4 failures) and pass with the fix.@posthog/core: full suite (2490 tests), typecheck andbiome lintclean.@posthog/ui: sessions and notifications suites (542 tests) pass after updating one hydration test that encoded the old disarm behavior; workspace typecheck clean.Automatic notifications