feat(desktop): deliver background-agent completions into live voice sessions#10257
feat(desktop): deliver background-agent completions into live voice sessions#10257skanderkaroui wants to merge 12 commits into
Conversation
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for wiring this path up — the overall shape is promising, and I like that the completion delta is treated as untrusted context rather than forcing a synthetic voice response.
I do need to block this head on the delivery/acknowledgement contract: AgentCompletionVoiceDelivery acknowledges the kernel checkpoint whenever sendBackgroundAgentContext returns true, but RealtimeHubSession.sendTextInput also returns true when it merely buffers text because the socket is not open or Gemini has no activity window. That means a completion can be marked seen before it is actually delivered; if the session closes before the buffered text flushes, close() clears pendingTextInputs and the completion is lost permanently. This contradicts the PR invariant that the checkpoint advances only after confirmed delivery.
Please make the background-completion path distinguish “accepted into an in-memory buffer” from “actually sent to the provider conversation,” or otherwise delay acknowledgeCompletedAgentDelta until the buffered context is flushed successfully. A regression test should cover the socket/activity-window buffered path plus close-before-flush so this exactly-once guarantee does not regress.
Also, formal approval is intentionally withheld because this is a nontrivial desktop voice-agent behavior change and the Desktop Swift checks are still pending; it will still need maintainer/product review after the delivery-loss bug is fixed.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
Git-on-my-level
added
needs-tests
a259fb0 to
4d41f40
Compare
|
Thanks — good catch, that was a real exactly-once break and I've fixed it in What changed: Bound (documented, not a loss): a completion arriving while a live Gemini session has no open activity window is deferred until the next window/turn or reconnect — never acknowledged until actually sent. Regression tests (
(new Separately, I also fixed the pending Desktop Swift CI failure it was blocked on: the |
9befcb8 to
45bdcc3
Compare
…essions A voice-spawned background agent's completion was invisible to the realtime voice model: the kernel journaled the run and the floating pill turned green, but no observer converted "done" into conversation input for the live provider session, so the model could not mention the result or answer "what did it find". The pull API built for exactly this (peekCompletedAgentDelta, with per-surface seen-id + high-water checkpoints) had zero callers. AgentCompletionVoiceDelivery wires that orphaned pipeline to the realtime hub: - Watches AgentRuntimeStatusStore projections for transitions into a terminal state on background surfaces only (floating_bar/service/workstream), so ordinary chat/voice answers never trigger kernel delta reads. - Reads the canonical completion delta from the kernel under the realtime_voice checkpoint namespace and injects the existing delta prompt (already fenced as untrusted, already says "do not read raw ids aloud") into the live provider session as silent context via the same buffered text-input path PTT uses. No synthetic response is requested, so the voice turn coordinator's output-lease authority and the one-start/one-terminal turn analytics contract are untouched; the model mentions the completion at the next natural turn boundary. - Acknowledges the checkpoint only after the session confirms the send, so an undelivered completion is retried (next transition or next session connect) instead of lost; hubDidConnect drains completions that finished while no session was live. Verification: - xcrun swift build -c debug --package-path Desktop: clean - New behavioral suite AgentCompletionVoiceDeliveryTests (9 tests) passes: delivery+ack ordering, checkpoint-unadvanced on failed inject, no-op without a live session, primary-surface transitions never trigger, repeated terminal projections fire once, cleared-and-recreated surface is a fresh transition, connect drains pending, in-flight coalescing, empty delta acks nothing. - ./scripts/agent-logic-harness.sh: all four gates green (focused Swift lifecycle/state, agent runtime, pi-mono-extension, self-check). - python3 scripts/check_desktop_test_quality.py: at/below baseline. - Not exercised live: a real voice session observing a completion on a named bundle (requires provider audio session); noted in PR for follow-up.
…ompiles AgentSyncRecoveryTests drives AgentSyncService's startForTesting / syncOnceForTesting seam, which only exists under #if DEBUG (AgentSyncService.swift:194). The debug test build compiles, but the release-mode notification regression step (run-swift-ci.sh --release-notification-regression) compiles the test bundle in release and fails on every PR since the suite landed. Wrap the suite in #if DEBUG with the established omi-release-compile marker, matching APIClientAuthRetryTests and the other DEBUG-seam suites. Verification: xcrun swift build --build-tests (debug) clean locally; full suite + release regression step run on the qualification machine (evidence in PR thread).
… compiles Same class of break as the previous commit, surfaced once AgentSyncRecoveryTests stopped masking it: VoiceTurnCoordinatorTests calls DesktopDiagnosticsManager.resetForTests(), which only exists under #if DEBUG (DesktopDiagnosticsManager.swift:866). Eight of the nine test files using that seam were already #if DEBUG-gated; this one was the recent exception, breaking the release-mode notification regression compile on every PR. Verification: - xcrun swift test -c release --filter UserNotificationCallbackBridgeTests/ (the exact CI step) now compiles the full test bundle and passes (2 tests). - Both gated suites still execute in debug: 37 tests, 0 failures (VoiceTurnCoordinatorTests 28, AgentSyncRecoveryTests 9).
Review by David (Git-on-my-level) caught a real exactly-once break: AgentCompletionVoiceDelivery advanced the kernel checkpoint whenever sendBackgroundAgentContext returned true, but that path delegated to RealtimeHubSession.sendTextInput, which also returns true when it merely BUFFERS the text (socket not open, or Gemini has no activity window). A completion could be marked seen while only sitting in pendingTextInputs — and stop()/close() clears that buffer, losing the completion permanently while the checkpoint stayed advanced. sendBackgroundAgentContext no longer buffers. It writes to the live provider only when the session can accept it now (isOpen, and for Gemini an open activity window) and returns true; otherwise it returns false without buffering, so the caller leaves the checkpoint unadvanced and retries on the next terminal transition or session connect. This restores the "checkpoint advances only after confirmed delivery" invariant with nothing to lose on close. Bound (not a loss): a completion arriving while a live Gemini session has no open activity window is deferred, not delivered, until the next window/turn or reconnect. It is never acknowledged until actually sent. Tests (RealtimeHubSessionInputLifecycleTests, DEBUG-seam suite): background context before transport ready / open OpenAI / Gemini-without-window each assert the return value and that nothing was buffered (new pendingTextInputCount field on the DEBUG input-lifecycle snapshot). Failure-Class: none
The Desktop Swift Test Suites job hung for 6h (killed at the CI cap) once the release-compile gate was fixed and the full suite could finally run. Root cause: the hubDidConnect hook calls AgentCompletionVoiceDelivery.shared .voiceSessionDidConnect(), and the shared instance's default (production) seams spawn a Task into DesktopCoordinatorService.shared -> AgentRuntimeProcess.shared, which tries to launch and waitForInit the real Node agent runtime. RealtimeHub unit tests drive hubDidConnect, so those tests reached into the runtime; with no app bundle/dist present that launch could block indefinitely. The hang was nondeterministic (the first CI run passed step 7, the rebased run hung; the mini's different sandbox never hit it). The service now stays fully inert — no status subscription, no delivery, no runtime reach — until start() runs, which only OmiApp calls at launch. Tests never start it, so voiceSessionDidConnect() is a guaranteed no-op there. Test: voiceSessionDidConnect before start schedules no work and reaches no seam. Failure-Class: none
45bdcc3 to
d63fb3d
Compare
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the follow-up fixes — the original ack-before-delivery issue is much improved now that sendBackgroundAgentContext refuses instead of buffering, and the new unit coverage around closed sockets/Gemini idle windows is useful.
I still need to request changes on one remaining delivery hole: when a Gemini session is already connected but idle, sendBackgroundAgentContext correctly returns false because there is no open activity window, so AgentCompletionVoiceDelivery leaves the checkpoint unadvanced. But nothing schedules another delivery when the next activity window opens. The only retry triggers I see are another background-agent terminal transition and voiceSessionDidConnect(). beginInputTurn() opens the Gemini activity window and flushes ordinary buffered text inputs, but this background-completion path deliberately does not buffer and AgentCompletionVoiceDelivery is not notified there. So a completion that lands while Gemini is warm-idle can still fail to reach the very next natural voice turn — exactly the follow-up moment this feature is meant to support — until an unrelated terminal transition or reconnect happens.
Please add a retry trigger at the point the realtime session becomes able to accept background context again (for example when a Gemini activity window opens / input turn begins, while keeping the no-buffer/ack-after-send invariant), and cover that end-to-end in tests: completion delivery fails while Gemini is connected idle, the next user turn/window opens, delivery is retried, and the checkpoint advances only after the send succeeds.
Formal approval remains withheld because this is a nontrivial desktop voice-agent behavior change with outstanding live named-bundle verification/product sign-off.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
…e checkpoint A rebase reintroduced the ack-before-delivery bug: sendBackgroundAgentContext delegated to the buffering sendTextInput, which returns true after *buffering* when the session is warm-idle or the socket is closed. The delivery service acks the exactly-once kernel checkpoint on that true, but stopOnQueue/abandonInputTurn clear pendingTextInputs — so a completion could be acked and then dropped (acked-but-lost). Give background context its own non-buffering path: refuse (return false, leave the checkpoint unadvanced) when the session can't accept context now, and when it can, resume the continuation from the provider send's completion so true means a *confirmed* send, not a fire-and-forget enqueue. Also emit hubDidOpenInputWindow when a warm Gemini activity window opens, so a refused completion can be retried (consumed in the next commit). - canAcceptInjectedContext capability check; shared textInputWire builder - tests: refuse-while-warm-idle -> false; confirmed-send failure -> false (DEBUG seam)
…reopens sendBackgroundAgentContext now refuses (leaves the checkpoint unadvanced) while a warm Gemini session is idle. The only delivery triggers were a terminal transition and session connect, so a completion landing while connected-idle was never retried when the next natural turn opened the activity window. Route the new hubDidOpenInputWindow capability signal to the delivery service so it retries then — the checkpoint still advances only after a confirmed send. - RealtimeHubController forwards hubDidOpenInputWindow -> voiceSessionDidOpenInputWindow - test: connected-idle -> window opens -> retried -> acked exactly once
Resolve conflicts from main's RealtimeHubSession type split (RealtimeHubSessionTypes.swift) and the hubDidError -> RealtimeHubTransportFailure refactor: - hubDidOpenInputWindow moves onto the relocated RealtimeHubSessionDelegate protocol (+ default no-op) in RealtimeHubSessionTypes.swift; the confirmed-send sendBackgroundAgentContext and beginInputTurn signal stay in RealtimeHubSession.swift. - Line-count baseline: RealtimeHubSession.swift 1584 -> 1639 (main's Ticket 14 body plus the confirmed-send/hubDidOpenInputWindow additions).
…start voiceSessionDidOpenInputWindow() reached scheduleDelivery() without the hasStarted guard that voiceSessionDidConnect() has. beginInputTurn emits hubDidOpenInputWindow, which RealtimeHub unit tests exercise, so before start() the shared delivery singleton drove the live agent runtime mid-test and the Desktop Swift Test Suites job hung (reintroducing the regression 9befcb8 fixed for voiceSessionDidConnect). Guard it the same way and add an inert-before-start regression test.
Only the desktop-swift-floatingcontrolbar line-count baseline conflicted: main dropped the now-1497-line RealtimeHubController.swift entry (below the 1500 threshold) and this branch raised RealtimeHubSession.swift to 1639 for the confirmed-send/hubDidOpenInputWindow additions. Keep main's structure plus the 1639 raise.
…ency RealtimeVoiceContextSingleFlightTests constructed a nonisolated Gate class inside @mainactor methods and sent it into the nonisolated single-flight closures, which fails the Swift 6 sending/data-race check on the current toolchain (Desktop Swift Test Suites could not compile the test target). Making Gate an actor makes it Sendable and serializes its state, also closing the latent startCount/continuation race the class form had. Verified on macOS: test target builds clean and the suite passes 3/3.
Git-on-my-level
left a comment
Git-on-my-level
left a comment
There was a problem hiding this comment.
Thanks for the quick follow-up. I re-reviewed the current head (7348c68) against the two prior automation blockers:
- The original ack-before-delivery issue now looks addressed:
sendBackgroundAgentContextrefuses instead of buffering when the provider cannot accept context, and acknowledgement stays after a successful provider send. - The Gemini warm-idle retry hole now looks addressed:
beginInputTurn()opens the activity window and notifiesAgentCompletionVoiceDelivery, with coverage for the failed-while-idle → retry-on-window-open → ack-after-send path.
I’m leaving this as a positive signal rather than formal approval because this is still a nontrivial desktop voice behavior change, the current Desktop Swift CI is not green (Desktop Swift Test Suites was cancelled and the aggregate build/test gate is failing), and the PR body still calls out that live named-bundle/provider verification has not been exercised. Please keep needs-maintainer-review/needs-tests until CI and the live voice-session validation are clean.
by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.
Current head 7348c68 addresses this automation review blocker; replacing with the latest review status.
Take main's RealtimeVoiceContextSingleFlightTests rewrite (event-driven Gate that stores the continuation before bumping startCount) — it fixes the startCount/continuation race whose missed wakeup hung the suite forever and timed out the 1h Desktop Swift Test Suites job; it supersedes this branch's actor-based fix for the same file. Baselines: OmiApp 1596->1599 (start() wiring on top of main's onboarding-journal extraction), RealtimeHubSession 1584->1639.
2 participants
Problem
A voice-spawned background agent's completion never reaches the live voice conversation. The kernel journals the run and the floating pill turns green, but nothing converts "done" into conversation input for the realtime model — so it cannot mention the result or answer "what did it find?". The pull API built for exactly this (
DesktopCoordinatorService.peekCompletedAgentDelta, with per-surface seen-id + high-water checkpoints and a prompt that already says "Do not read raw ids aloud") had zero callers.Change
AgentCompletionVoiceDelivery(new,Chat/) wires that orphaned pipeline to the realtime hub:AgentRuntimeStatusStoreprojections for transitions into a terminal state on background surfaces only (floating_bar/service/workstream) — ordinary main-chat/voice answers never trigger kernel delta reads.realtime_voicecheckpoint namespace and injects the existing delta prompt into the live provider session as silent context via the same buffered text-input path PTT uses (sendBackgroundAgentContext, new thin wrapper besidesendTestTextInput). No synthetic response is requested — the voice turn coordinator's output-lease authority and the one-start/one-terminal turn analytics contract are untouched; the model mentions the completion at its next natural turn.hubDidConnect(which also drains completions that finished while no session was live).Hooks are minimal: 1 line in
hubDidConnect, 1 startup call inOmiApp, a 2-member controller extension in its own file.Product invariants affected
RealtimeHub*); no session-death or auth behavior changes. The new code performs no auth calls; it forwards text into an already-authenticated session.Verification
xcrun swift build -c debug --package-path Desktopclean.AgentCompletionVoiceDeliveryTests(9 tests, injected seams, no wall-clock waits): delivery+ack ordering; checkpoint unadvanced on failed inject; no-op without a live session; primary-surface terminals never trigger; repeated terminal projections fire once; cleared-and-recreated surface is a fresh transition; connect drains pending; in-flight coalescing; empty delta acks nothing../scripts/agent-logic-harness.sh— all four gates green.python3 scripts/check_desktop_test_quality.py— at/below baseline.make preflightgreen at HEAD.Not exercised live: a real provider voice session observing a completion on a named bundle (needs live OpenAI/Gemini audio session). Follow-up before merge or as merge-gate evidence: spawn an agent by voice on a named bundle, wait for completion, confirm the next model turn mentions it.
Deliberate scope cut
Immediate spoken announcement (model proactively speaking mid-idle) is out of scope:
hubDidReceiveAudio/hubDidEmitTextdrop events without an accepted turn identity, so proactive speech requires aVoiceTurnCoordinator-owned announcement turn — tracked as follow-up work, not smuggled around the output authority here.Ratchet notes
RealtimeHubSession.swift(+7) andOmiApp.swift(+3) baselines raised with one-line justifications; the controller addition lives in a new extension file instead of growingRealtimeHubController.swiftpast its threshold.Failure class (fixes)
Failure-Class: none
The two follow-up
fix:commits are specific instance fixes (an ack-before-delivery bug and a test-triggered runtime reach), each with a regression test. No existing failure class matches and neither warrants a new reusable guard class.