feat: live in-flight run progress notification (A1) - #111
Merged
Conversation
onDestroy() cancelled the collector scope then immediately called notifier.cancelRunProgress(), but scope.cancel() is cooperative and does not preempt a collector iteration already executing synchronously. An in-flight iteration could still call notifier.postRunProgress(...) after the cancel, with no ordering guarantee, permanently stranding an ongoing notification. Hang the final cancel off the Job's actual completion (invokeOnCompletion) instead of off the cancel() call, so it fires only after every child coroutine has genuinely finished. Also collapse the two separate profiles.active.value reads in updateRunProgress into one, so a profile switch landing between them can no longer split a notification's title/route from its accent colour across two different tenants.
A run's tenant is now latched into RunProgress at message.start / session.info time instead of being re-read from ProfileManager.active at notification-post time. Opening a cross-profile session (SessionsViewModel.prepareOpen) switches the active profile without resetting run state, so the two could drift and post a notification with one tenant's accent/title while its route still pointed at another tenant's session. Also: a differing sessionId on a running session.info now starts a fresh run instead of silently keeping the stale one, and the second reserved notification id (1003, run-progress) is now guarded against id collisions the same way the first (1001) already was.
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
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.
Roadmap item A1 from
docs/ideas/2026-07-16-competitive-refresh.md(P0).Until now the app notified only when a run finished and showed nothing during the minutes a run was executing — the only in-flight signal was a spinner inside an open chat. This adds a live ongoing notification so an agent run is glanceable while the app is backgrounded.
What it does
acme · agent running/Calling tool: web_search.N/Mbar whenever the agent'stodotool reports counts (the same signal the desktop client uses for its Tasks chip).Notification.ProgressStyleso the system can promote it to a status-bar Live Update; API 26–35 falls back toNotificationCompat.setProgress.run_progresschannel atIMPORTANCE_LOW, separate from the MINservicechannel (which stays untouched) — MIN would have suppressed the status-bar presence this feature exists to provide.Scope note
The roadmap proposed a true
step 3/5. That is not achievable client-only: the only real iteration counter (api_call_count/max_iterations) is wired intogateway/run.pyand the ACP adapter, never intotui_gateway/server.py, which is the path this app's WebSocket uses. Thetodolist is the one client-reachable proxy. A generic determinate bar would need a gateway change — deliberately out of scope here.Architecture
Gateway events → a pure reducer (
RunProgress.reduce) → a pure mapper (toSpec) →HermesNotifier, driven fromGatewayConnectionService, which already owned the single event collector. Run state deliberately does not live inChatUiState— that is scoped to an open chat and dies when backgrounded, which is exactly when this must survive. Keeping the reducer and mapper pure means all decision logic is unit-testable without Android (this repo has no Robolectric).Also wires up
session.info.running, which the client never parsed before, as the authoritative busy/idle backstop —message.completealone misses interrupted and compacted turns, which would otherwise strand a permanent "running" notification.Review findings fixed along the way
message.start. A mid-runProfileManager.switchTo()— whichSessionsViewModel.prepareOpentriggers when opening a session from the cross-profile list — re-posted an acme run asglobex · agent runningwith globex's accent, still routed to acme's session. Tenant is now latched into the run itself, so name, accent, and route come from one snapshot.scope.cancel()is cooperative, so an in-flight collector iteration could re-post afteronDestroy()'s cancel, leaving an unswipeable ongoing notification. The cancel now hangs offjob.invokeOnCompletion, firing only once every child has terminated.session.info{running:true}with a differing sessionId now starts a fresh run instead of keeping the stale one; the reserved-id guard now covers 1003 as well as 1001.Verification
351 unit tests green (30 new across the reducer and mapper),
compileDebugKotlinandassembleBetagreen, gitleaks clean. On device: foreground service and therun_progresschannel confirmed on the emulator; the full end-to-end live-run observation was not performed (no gateway pairing credentials available) and is recorded as skipped rather than assumed.Known follow-ups (non-blocking)
CompositionLocaland the notifier has no composition, so honouring it needs a repository-backed read.START_STICKYrestart with no UI,ProfileManager.activeis null, so the title degrades to a generic "Agent running" (never a wrong tenant).