Repository navigation
feat(voice): relay agent as a bodhi subagent — work calls return their own results - #5310
Conversation
Coverage Gate✅ No Python changes — nothing to gate (bar: 95% on changed lines). |
d7885a3 to
747652e
Compare
3435d14 to
00d5228
Compare
747652e to
4c2a9af
Compare
cf7ebbd to
4f18837
Compare
4c2a9af to
dc04722
Compare
sonichi
left a comment
There was a problem hiding this comment.
Reviewed the design — it's sound. RelayAgent's invoke/offerResult/abort handling is race-safe: an aborted work call correctly falls through to the ordinary delivery path (startResultWatcher's callback calls deliverResult when offerResult returns false), and reconcileVoiceTasks's isInFlight correctly checks relay.isWaiting() instead of a separate queue, so a task still held by an active work call isn't double-delivered by the reconcile pass. The "confirmed delivery never downgraded" rule in the store, the cancel/replay/not-picked bookkeeping, and the bodhi PersistentSubagentInstance wiring (relayAgentSubagentConfig, lifetime: 'persistent_session') all look correct and match the PR body's description.
Two things before this can merge, though:
-
Real CI failure, not a flake:
eslint over first-party TS + JSfails with 1 error (the 249no-explicit-anywarnings are pre-existing noise, not the problem) —src/voice-agent.ts:33:36 'readdirSync' is defined but never used. This is a genuine leftover from removing the old tasks/-directory-reading code this PR replaces with voiceTaskRows(). Easy fix (drop the import), but it's a real failing required check right now, not something to wave through. -
Live evidence is explicitly marked Pending in the PR body ("to be added from the test Mac after a restart"). This is a delivery-loop change on the voice live path — per this repo's own standard (the same one Kewei/Rui have held #5285/#5300/#5278 to today), harness-green isn't a witness here; it needs the real post-restart round trip (a work task's result spoken through its call, a result whose call was cancelled, "how many tasks", cancel) before merge, not after.
#5309 is clean and I've approved it. Will re-review #5310 once the lint fix lands and the live evidence is posted.
…r own results The relay agent is rebuilt as the bodhi PersistentSubagentInstance behind the work tool, in actor mode: - work is a background tool (pending message: the old "working on it" text); RelayAgent.invoke submits the task through the tool's own submission and resolves with that task's core result, which bodhi hands to the model when it is idle (and holds through a meeting). A queue position or an offline core is said at submission. - A result whose call is gone (cancelled, session closed) takes the ordinary delivery path; a voice task that cannot be delivered leaves one DM copy. - Kept from #5290: the voice task table, planReconcile (an unheard result is handed over again at most twice; a task not picked up after 3 min is reported once), and the status answers (get_task_status, the cancel list, the count ahead) read from the table. - Not kept: #5290's own result queue and pacing; bodhi's delivery does that. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… agents through a lost upstream 0.4.5 (randombet/bodhi_realtime_agent#52) fixes the #48 review items the relay agent depends on: under upstreamLossPolicy 'hold' a park or provider close is not a session end in actor mode (no agent stop, one redialer), upstreamRecovery requires the recovery primitives, and active silence hears recognized input. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
dc04722 to
56ca908
Compare
0393c02 to
62efa07
Compare
yixuan-ag2
left a comment
There was a problem hiding this comment.
Reviewed at head 62efa078 (base revert/5290-relay-agent = #5309). Code tree is identical to 0393c02a apart from main's #5266 test lists. Verdict: REQUEST_CHANGES.
Checks I ran:
tsc --noEmitclean.npm run test:ts: 1630 tests, 0 failures.- relay, cancel, meeting and task-bridge suites: 60/60 pass.
- Provenance of bodhi 0.4.5 (below).
bodhi 0.4.5 provenance: OK
- The lockfile
integrity(sha512-s7H3Jea…JrwVw==) equals the npmdist.integrityvalue. - npm
gitHead1cc152abis tagv0.4.5, identical to bodhimain. v0.4.4..v0.4.5contains exactly #52 (8ff7b635, merged, approved by yixuan-ag2 and sonichi at8ff7b635) plus #53.git diff 8ff7b635 v0.4.5is only thepackage.jsonversion bump, so the missing review on #53 does not matter.
Blockers
-
CI red, real: eslint reports
src/voice-agent.ts:33:36 'readdirSync' is defined but never used. Reproduced locally. tsc does not catch it. -
The design drops the result queue I asked to keep inside the instance.
invokeholds until the result arrives (relay-agent.ts:151-169). Bodhi then publishes it through its background-completionpublishSystemNotification("Background task work completed… Result: …"). That call is void, and NotificationActor flushes one notification per turn with no pacing. Status of each #5290 guarantee:- Every task ends in an outcome the user heard: weakened.
offerResultmarks'injected'the moment it hands the result to bodhi (relay-agent.ts:176). Nothing ever writes'spoken'(grep: no writer).planReconciletreatsinjectedas heard (relay-agent.ts:121), so a result bodhi did not voice is never replayed. - Reconcile replays at most twice: kept (
relay-agent.ts:110,118-127,task-bridge.ts:599). It is blind to the case above. - The "not picked up after 3 min" notice: kept (
relay-agent.ts:112,124). - One batch per turn, waiting for a pause, including after a reconnect: partly dropped. NotificationActor delivers immediately when the model is not speaking, with no user-speech gate. After a hold is released,
drainNotificationsWhenIdlesends one notification at once.conversation-pacing.tsis gone. - Re-hand after the user cuts the answer off: dropped. An interrupted turn does not re-deliver, and the row already says
injected. - Each item names its request: dropped. The notification carries only the framed result, no request text. This is exactly the three-PR attribution bug that #5290 fixed.
- DM fallback after 60 s, single copy: dropped for work-call results.
live-agent-runtime.ts:197routes them toofferResultand skipsdeliverWithRetry'sonExhaustedDM path (:157-161). With the page closed, bodhi parks the upstream (parkUpstreamLost→ hold), so the result waits in memory with no DM. A voice restart loses it while the row saysinjected. The single-copy guards for the non-work path are untested: withif (_hasLiveOfflineCopy(id)) continueremoved (task-bridge.ts:604), or with thedelivery === 'dm'guard removed (live-agent-runtime.ts:159), all 41 relay/cancel/meeting tests still pass. - Meeting hold with paced release (#5288): dropped. Bodhi's
DictationController.flushPendingContentTurnsdrains everything on exit, and Sutando's own hold releases withfor (… of heldForMeeting.splice(0))(live-agent-runtime.ts:113). The #5288 test ("spoken in turn, not over each other") is removed and not replaced. - voiceTasksAhead, the cancel list and get_core_status answering from the table: kept (
task-bridge.ts:565,629;inline-tools.ts:683,859;voice-agent.ts:490).
Fix: keep #5290's queue inside
RelayAgent.invokereturns a quick ack (nopendingMessage). Batch, wait for a pause, allow one batch per turn, and re-hand after a cut-off. Publish withtryPublishSystemNotificationand requeue or DM when it returns false. Take turn-end and interruption signals from the session's event bus (turn.end/turn.interrupted, which #5290 already subscribed to). Then bring back the dropped tests: batching, re-hand after cut-off,spokenon turn end, per-item DM fallback, the #5288 paced burst, and the e2e three PRs → DM → replay. - Every task ends in an outcome the user heard: weakened.
-
Cancel does not reach the instance.
noteVoiceTaskCancelleddeletes the task file, but the waitinginvokefor a queued, cancelled task never settles: the core writes no result for that id, so the bodhi workflow and waiter stay open until the session closes. It needs acancel(taskId)into the instance. Separately, bodhi'sbackgroundExecutorcatch publishes "Background task work failed… aborted. Tell the user the exact error" on any abort, while the task keeps running. Please check whether that path is reachable.
Mutation checks (relay, cancel and meeting suites; the control run passes 41/41):
offerResultnever matches: 3 failures. Caught.MAX_REPLAYSchanged from<to<=: 1 failure. Caught.offerResultstops recordinginjected: 2 failures. Caught.- Offline-copy guard removed: 0 failures. Uncaught.
dmdouble-speak guard removed: 0 failures. Uncaught.
There is no one-batch-per-turn or re-hand code left to mutate.
Live evidence: none posted (the body says "Pending"). This is a live-path change (voice-agent.ts, task-bridge.ts, live-agent-runtime.ts). At the combined head, after a restart, I need:
- three PRs asked back to back, each spoken and attributed;
- a DM fallback followed by a replay after an outage;
- a clean burst when a meeting ends;
- cancel, both queued and already done;
- the page closed for more than 60 s (park → redial) with the result still delivered.
Merge order: #5309 and this PR land together, after this PR is approvable (see my comment on #5309). license/cla passes. The eslint failure is the only real red check. Some jobs were still pending when I posted.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…n, DM when undeliverable, no pacing The previous head marked a result heard the moment it was handed to bodhi and had no fallback when the session could not take it (a result for a closed page waited in memory, lost on restart). The relay agent now owns #5290's result queue again, minus pacing: - invoke returns the submission status at once (queued, the queue position, a duplicate, a rejection) as the tool result; no pendingMessage. - RelayAgent.attachDelivery binds the queue to the session. Results are handed over through session.tryPublishSystemNotification; a refusal is retried, then the result goes to the DM, one copy per result. - A result is recorded spoken only when the turn ends, handed over again once if the user cuts it off, names the request it answers, and is held through a meeting. Phone-call results go through the same queue. - Not kept: conversation pacing and the 2 s gather window. bodhi delivers when the model is idle; the pacer delayed a result 49 s on room noise. - Tests: the queue, refusal -> DM, the subagent, and the two single-copy guards (an offline copy still in results/, a result already sent to the DM), each failing when its guard is removed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…eduped request is never left unheard Live: three PR status requests; the core answered all three in the first task's result and wrote [deduped: <first>] for the other two. The table did not know that, so the first result was handed over naming only its own request (the model spoke only that part) and the other two rows, holding a skip marker, were owed nothing. - A row records answeredBy when its result is [deduped: <task>] (local and relay-mode result paths; dedupTarget in skip_marker_ownership). - A result is handed over naming every request it answers (its own and each unheard row answered by it) and records all of their deliveries together. - Reconcile judges an answeredBy row by its answering task's result: if that was handed over without it (the dedup landed later), it is handed over again, named for this row's request. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
sonichi
left a comment
There was a problem hiding this comment.
Re-reviewed at 6110487. Both prior blockers resolved:
- eslint is clean now (confirmed CI green, no errors).
- Live evidence is present and substantive — covers all three queue positions, the batch delivery naming multiple requests, and the cut-off/re-hand-over path. The PR body is also honest that the noise-barge-in reconnects are bodhi's client VAD treating room noise as speech, out of this PR's scope, rather than hiding it.
Traced the new answeredBy/dedup mechanism specifically, since the live evidence's own description of finding that bug ("an earlier run... only 5308 was spoken") is exactly the kind of thing worth verifying rather than taking on faith:
startResultWatcher's skip-marker branch extractsdedupTarget(result)and callsvoiceTaskStore.setAnsweredBy(taskId, answeredIn)— the suppressed task records which task's result actually answers it.answering(taskId)(in live-agent-runtime.ts) looks upvoiceTaskStore.answeredBy(taskId)— every task whoseansweredBypoints at this one — filters to only the unheard ones, and feeds their original request text intorequestsplus their ids intoalsoFor.frameBatchthen tells the model to cover every named request, andrecord()marks all of them (not just the primary taskId) delivered once spoken.- This is symmetric and correct, and
dedupTarget's own unit tests (including the D7-header-peeled and "no match" cases) are solid.
The simplified invoke() (submit via submitWorkTask, return status immediately — no longer awaiting the result inside the tool call) matches the live evidence's ~64ms call durations, and decoupling delivery into attachDelivery's queue (the same battle-tested createResultQueue/frameBatch pattern from #5290, minus pacing) is a clean design.
No blockers. Approved — ready to merge with #5309 per the stack.
…task table covers what the call cannot Delivery goes back to the work call itself, now declared NON_BLOCKING: invoke waits for its task's result and returns it as the function response, so Gemini decides when to say it and which call it answers. The relay's own result queue is removed. The task table covers what a pending call cannot carry: - a fresh provider connection (session.reconnectBoundary): responses to calls made on the old one would be fenced off, so those calls end quietly and their results go out as a system notice naming the request; - a cancelled call or a restart: same notice path; one DM copy when the session cannot speak; held through a meeting; - a result answering several tasks (answeredBy) ends their waiting calls and records them all; a late dedup is offered to the task's own waiting call by reconcile first. A queue position or an offline core is said at submission. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e while the call waits for the result Live at 5b046c3: with work declared NON_BLOCKING and no pending message the model said nothing after calling it (an empty turn) until the user spoke again; it waited for the function response, which comes only when the core finishes. The pending message is back: bodhi answers the call at once with "still in progress" and hands the result over as a system notification when invoke returns, which a fresh provider connection does not drop, so the reconnect-boundary detach is gone too. invoke still waits for its task's result; the task table and reconcile cover a call that is gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
yixuan-ag2
left a comment
There was a problem hiding this comment.
Re-review at head 2d6e921d (verified). Base is #5309's head 56ca9084. Delta from 62efa078 is five additive commits with no rebase: 9532664b c429d009 61104872 5b046c36 2d6e921d. Verdict: REQUEST_CHANGES. Blocker 1 is fixed and attribution is back. The other blockers come from one decision: a result counts as heard the moment it is handed to the work call.
What I ran at 2d6e921d
tsc --noEmitis clean.- eslint on the 9 touched files: 0 errors (15 pre-existing
anywarnings). - relay, e2e, meeting-hold, cancel and task-bridge suites: 202/202 pass.
- CI runs 38074426009 and 38074425994 are both at
2d6e921dand green.license/clapasses.
Blockers from 62efa078
-
eslint: Fixed.
9532664bdropsreaddirSync(voice-agent.ts:33). eslint is 0 errors locally and in CI. -
The relay's result queue: Partly fixed.
c429d009restored the queue, and5b046c36removed it again. Results now return through the waitinginvoke(relay-agent.ts:192-229). bodhi publishes them as "Background task work completed… Result: …" (bodhivoice-session.ts:2262-2268, void, in memory). Status of each guarantee:- Per-item attribution: Fixed.
frameResultputs "This answers the user's request: """ (or "N of the user's requests: …; cover each") ahead of each result (relay-agent.ts:154-163).answering()fills it from the table (live-agent-runtime.ts:105-113). With the request names removed, 5 tests fail (M1 below). - Re-hand after a cut-off: Not fixed.
offerResultwritesinjectedat hand-off (relay-agent.ts:219). NotificationActor does not re-deliver an interrupted turn:turn_completeafterinterruptedonly skips the next flush (notification-actor.ts:145-157). Nothing re-hands the result. - One batch per turn / pause gating: Dropped, by design. When idle,
handlePublishdelivers each publish at once (notification-actor.ts,if (!this.audioReceived) this.deliver(n)). Three results landing together become three back-to-back synthetic user turns. Nothing gates on user speech. - 60 s DM fallback for work results: Not fixed.
deliver()sends a result toofferResultfirst (live-agent-runtime.ts:180-187). ThedeliverWithRetry→onExhaustedDM path (:136-178) runs only when no call is waiting. Example: page closed, call still waiting. The result is handed to bodhi and sits in its in-memory queue. There is no DM, and a voice restart loses it. The row saysinjected, so reconcile never replays it (relay-agent.ts:133). The fallback works only for results with no waiting call; the e2e test covers that case, not this one. - Paced post-meeting release (#5288): Partly. The hold works. Your 19:02 run shows a result held through a meeting and spoken after exit. Release is not paced: bodhi flushes on exit, and Sutando's own hold is
for (… of heldForMeeting.splice(0)) deliverElsewhere(held)(live-agent-runtime.ts:131).
- Per-item attribution: Fixed.
-
spokenis never recorded: Not fixed. Nothing insrc/writes'spoken'. The only writes are'injected'(relay-agent.ts:219,live-agent-runtime.ts:140) and'dm'.planReconciletreatsinjectedas heard (relay-agent.ts:133), and so docancel_taskand the status answers (inline-tools.ts:716,825,861,voice-agent.ts:495). A result bodhi never voiced is never replayed. (c429d009had "spoken only when the turn ends";5b046c36removed it.) -
Cancel: Not fixed.
noteVoiceTaskCancelled(task-bridge.ts:643-650) still does not touch the relay agent. The core answers theCANCEL_INSTRUCTIONtask, not the cancelled id, so the waitinginvokefor a queued, cancelled task never settles. The waiter and the bodhi workflow leak untildispose. There is nocancel(taskId)and no test for it. Separately, the abort path is live. Atransport.tool_call_cancelledfor aworkcall becomessubagent.cancel_requested(bodhitool-router-actor.ts:61-69). The relay then rejects withaborted(relay-agent.ts:203-208). Becauseworkhas apendingMessage, bodhi's catch publishes "Background task "work" failed. Exact error details: aborted. Tell the user the exact error details first" (voice-session.ts:2280-2286) while the task keeps running. The user hears a false failure, and later the real result. -
Live evidence: Not on the PR. The body's evidence is at
61104872, and it describes theattachDeliveryqueue, which no longer exists at the head (git grep attachDeliveryis empty). No comment carries the 18:15 run (three PRs) or the 18:59 run (meeting hold) at2d6e921d. I have only a relayed summary of them, not the log. Please post both as a PR comment: head sha, restart line, and transcript lines. On the 18:18:40 run, where three results were "read out together": the mechanism above sends three turns back to back with no pacing. Nothing cut the answer only if the transcript shows each PR named once, with noInterrupted/notification.interruptedbetween 18:18:40 and the end of the answer. If one was cut, it is lost and still markedinjected(see the re-hand bullet). I can't judge that from the summary.
Mutation checks (relay, e2e and meeting suites; the control run passes 41/41):
frameResultwithout request names: 5 failures. Caught.- The result watcher stops recording
answeredBy: 1 failure (e2e "three requests… all three named"). Caught. - Reconcile ignores
answeredBy(source = id): 2 failures. Caught. offerResultdoes not endalsoForwaiters:relay-agent.test.tshangs (killed after 559 s). It is caught, but as a hang, not as an assertion.
So the dedup / answeredBy path is properly pinned by unit and e2e tests, and I accept it without a live run.
What would make this approvable. Keep the work-call delivery and the pending message: the empty-turn finding at 17:59 is real. Add:
- (a) record
spokenonly onturn.endfor the turn that carried the result, and re-hand once onturn.interrupted; - (b) a waiting call whose session cannot speak (client gone) gets the same 60 s → one DM copy as
deliverElsewhere, or reconcile treats aninjectedrow older than N seconds with nospokenas owed; - (c)
RelayAgent.cancel(taskId), called fromnoteVoiceTaskCancelled, which resolves the waiter with a cancelled status (not a rejection, so bodhi does not say "failed"); - (d) a test for each, plus the live comment at the final head.
Not blocking: the PR body is stale. It describes the c429d009 queue design and the 61104872 evidence. Please rewrite it for the head.
Merge order is unchanged: #5309 and this PR land together, back to back. #5309 stays un-approved by me until this PR is approvable.
…ts result goes to the DM - Cancel: the core never answers a cancelled task, so its waiting call hung until the session ended. noteVoiceTaskCancelled now tells the relay agent, which ends the call quietly. - Abort: the relay rejected with "aborted", and because work has a pending message bodhi published "Background task failed… aborted. Tell the user" while the task kept running. The call now ends normally with "still running; the result comes separately", and the result goes the usual way. - No client connected: the result watcher sends a voice task's result to the DM (it never offers it to the relay), so the waiting call also hung. It now ends that call and the calls of the tasks answered in the same result. No call ends with a rejection: bodhi reads a rejection out as a failure. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Live evidence at 2d6e921 (test Mac)Both runs were on the test Mac with this PR's 2d6e921 plus unrelated PRs merged on top for testing:
Three requests back to back: each call answered at once, each result returned through its callThe answer was cut at 18:18:50 by a barge-in (background noise taken as the user). The rows stay A result held through a meeting, said after itBefore 2d6e921 (5b046c3: NON_BLOCKING, no pending message): silent after the callNot live-verified
🤖 Generated with Claude Code |
… whose reply contradicts the table
Live on the test Mac: the user cancelled a task still in the queue. cancel_task
deleted its file and also wrote a CANCEL_INSTRUCTION, then told the model to
wait for the core. The core found no file and replied "Nothing to cancel —
task was never queued"; that reply was read out as "the cancellation couldn't
be processed".
A queued task whose file is deleted never runs, so it is cancelled: the user is
told so at once and no instruction is written. If the core took the task in
that instant, its result already comes with CANCELLED_BUT_FINISHED_NOTE. The
cancel_requested state ("waiting for the core to confirm") is gone with it.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
yixuan-ag2
left a comment
There was a problem hiding this comment.
Re-review at head 19ecda86 (verified right before posting). The delta from my last review (2d6e921d) is af3ebdf1 and 19ecda86, with no rebase. Verdict: COMMENT. The three code blockers in scope are fixed and pinned by tests. What still stands between this PR and my approval is evidence and the PR body, not code. My earlier CHANGES_REQUESTED stays until the live run below is posted; after that I will approve this PR and #5309 together.
What I ran at 19ecda86 (in a throwaway clone)
tsc --noEmitis clean.- eslint on the 8 touched files: 0 errors (15 pre-existing warnings).
- relay, e2e, meeting-input-hold, voice-task-cancel and
task-bridge-*suites: 204/204 pass. - CI: 33/33 pass.
Blockers in scope
| Status | Evidence | |
|---|---|---|
a. Page closed, work call waiting → DM |
Fixed | The result watcher's !clientConnected branch now forwards to the DM and calls _onVoiceTaskEnded(id,'delivered_separately') for the task and each answeredBy row (task-bridge.ts:1756-1758). voice-agent.ts:964 wires that to relayAgent.endCall. The result is never offerResult-ed into a session nobody hears. New e2e test: "no client connected: … waiting call ends". Mutation: dropping the two lines makes that test fail (8 s timeout). |
| b. Cancel reaches the relay | Fixed | noteVoiceTaskCancelled calls _onVoiceTaskEnded(taskId,'cancelled') first (task-bridge.ts:643). endCall resolves, not rejects. A queued voice task is now cancelled outright, with no CANCEL_INSTRUCTION whose reply could contradict the table. Mutations: removing that call fails "a cancelled task ends its waiting call quietly"; removing the setVoiceTaskEndedListener wiring in voice-agent fails the source pin. |
| c. Abort → no "failed: aborted" | Fixed | The abort listener now calls endCall(taskId,'continuing'), which resolves with a framed "still running; result comes separately" (relay-agent.ts:209-212). bodhi's catch, which publishes "Background task … failed", is no longer reached (bodhi dist/index.js:14150-14162). The late result then sees offerResult refuse it and takes deliverElsewhere. Mutation: restoring reject(new Error('aborted')) fails the abort test. |
| d. PR body + live evidence | Body fixed; live evidence not at head | The body now describes the subagent design and lists the call endings. The only live comment (#6102273904) is at 2d6e921d, run with #5319/#5324/main merged on top, and it says af3ebdf is "not live-verified". |
Approval conditions (#5309 reverts #5290, so the out-of-scope items are regressions against main)
- Known limitations in the body: partly met. "Not in this PR, by the PR owner's decision" lists both items. Please title it Known limitations and say plainly that each one is a regression against main once #5309 lands.
- A follow-up issue for each: not met. I found none (searched issues since 10-09). Please file (i) re-hand of a cut-off result / record
spokenat turn end, and (ii) pacing of results that land together / paced post-meeting release. Link both from the body. - A live #5288 run at
19ecda86or a later head: not met. Please post a comment with the head sha, the restart line, and transcript lines showing 2+ results held during a meeting, each spoken whole after exit, and noInterrupted by user/notification.interruptedwhile they are read. Note that the 18:18:50 run in the existing comment shows exactly the failure mode (anInterrupted by usermid-answer, with the rows leftinjected). Ideally the same run also shows one af3ebdf ending live (a cancel of a queued task, or page-closed → DM).
Nits (non-blocking)
- The already-aborted early return (
relay-agent.ts:206,if (signal?.aborted) return …continuing) has no test. Turning it back intothrowleaves all 49 tests passing. - bodhi wraps every resolved value in "completed successfully. Result: … Please inform the user now." So
delivered_separately("Say nothing about it now") andcancelled("Do not mention it again") reach the model next to an instruction to inform the user. For the page-closed case nobody hears it. For the cancel case, expect a possible second "it's cancelled". Worth watching in the live run. - Case (a) covers
clientConnected === false. When the page is connected but the upstream is reconnecting, the result is still handed to the waiting call. I'm accepting that on the strength of bodhi 0.4.5 keeping agents through a lost upstream under hold, but I did not verify it live.
Live evidence at 19ecda8 (test Mac)Build cc3bd285 = this PR's 19ecda8 + #5319 + #5324, restarted 21:33 UTC from a GUI terminal. Cancel a queued task: decided by the task table, no instruction to the core, the call endsBefore 19ecda8 (af3ebdf), the cancel also wrote a CANCEL_INSTRUCTION; the core found no file and its reply was read out as a failure: At 19ecda8: No Page closed while the call waits: result to the DM, the call endsNot live-verified
🤖 Generated with Claude Code |
Live #5288 run at 19ecda8 (test Mac)Build cc3bd285 = this PR's 19ecda8 + #5319 + #5324, restarted again at 21:56 UTC (the cancel / page-closed run above was on the 21:33 restart of the same build). Two results land during a meeting, are held, and are said whole after the exit: Nothing was said during the meeting; after the exit both results (and the notes-saved line) were said in one answer. No Abort (af3ebdf) was not reproduced live: by voice, talking on after a request makes the model wait and decide not to call 🤖 Generated with Claude Code |
yixuan-ag2
left a comment
There was a problem hiding this comment.
Approving at 19ecda8. This replaces my CHANGES_REQUESTED at 2d6e921 and my COMMENT at this head.
Code: blockers a/b/c are fixed and pinned by tests that fail when each fix is reverted (details in #5310 (review)):
- a work call waiting with no page connected ends when its result goes to the DM;
- cancelling a queued task ends its call quietly, with no CANCEL_INSTRUCTION;
- an aborted call ends without bodhi's "failed" notice.
Live, at this head (build cc3bd285 = 19ecda8 + #5319 + #5324):
- #5310 (comment). Cancel of a queued task: no instruction written, call ended,
tasks/empty afterwards. Page closed mid-call: the result was forwarded to the DM after about 104 s and the call ended. - #5310 (comment). The #5288 meeting-exit repro, after a restart at 21:56. Two results were held during the meeting, with silence throughout. After the exit both were said in one answer, with no
Interrupted by user. - Abort was not reproducible by voice (the reason is given in the comment). I'm accepting
tests/relay-agent.test.tsfor it, since it pins the change and only alters how a call settles.
Scope: the PR body now lists both regressions against main under "Known limitations", with follow-ups #5327 (a cut-off result is not said again) and #5328 (no pacing). These were left out by the PR owner's decision.
Checks: CI 33/33 green at 19ecda8. Mergeable.
Nits, not blocking:
- After the meeting exit the model summarised both results instead of reading them out ("PR 5167 目前有被卡住的状态" drops "changes requested"). That is model wording, not delivery. Worth watching under #5328.
- Cancelling is announced twice: once by the model, once from bodhi's "Please inform the user now".
- The early return for an already-aborted call (
relay-agent.ts:206) has no test.
#5309 must merge together with this PR. I'm approving it alongside.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@cla-assistant check |
Why
Review asked for the relay agent to be a bodhi subagent in actor mode, not plain code in the voice process (#5290). #5308 put voice in actor mode; #5309 reverts #5290; this rebuilds the relay agent as the
PersistentSubagentInstancebehind theworktool.Stack: #5308 (merged) → #5309 → this. Merge #5309 and this together.
What
RelayAgent(src/relay-agent.ts), a bodhiPersistentSubagentInstance, registered assubagentConfigs.work(lifetime: 'persistent_session').workis a background tool with apendingMessage:invokesubmits the task through the tool's own submission (dedup, spoken turns, header and origin unchanged) and waits for the task's result. bodhi hands it to the model as a system notification when the model is idle.frameResult): "This answers the user's request: …" or "This answers N of the user's requests: …; cover each".state/voice-tasks.json, survives a restart): each task's text, submit time, cancel requested, how its result reached the user (injected/dm), andansweredBy, the task whose result answered it (from the core's[deduped: X]).answeredByrow is judged by its answering result; a task not picked up after 3 min is reported once. Status answers (get_task_status, the cancel list, the count ahead) come from the table.Known limitations
These are regressions against main once #5309 (the revert of #5290) lands:
spokenat turn end and handed a cut-off result over once; here a result counts as delivered (injected) when it is handed to itsworkcall. Left out on purpose: a cut-off answer is the voice layer's barge-in, not the task table. Follow-up: voice: a task result cut off mid-answer is never said again #5327.Tests
tests/relay-agent.test.ts(34): the subagent (status at once for a duplicate/rejection, queue position notice, result returned through the call and naming its request, several tasks answered in one result, abort ends without a failure, cancel ends the call quietly), the table, the reconcile rule and pass, status answers from the table, and pins that voice mapsworkto the relay agent with a pending message and ends cancelled/forwarded calls.tests/relay-agent-e2e.test.ts(4, own process): a result returns through its waiting call; three requests answered in one result are all named and all recorded heard; a session that cannot take a result leaves one DM copy; with no client connected the result goes to the DM and its waiting call ends.tests/meeting-input-hold.test.ts: results held through a meeting are handed over after it.npm run test:ts: no failures beyond the environment-only set that also fails onorigin/mainhere.tscclean, eslint 0 errors, src-map up to date.Live evidence
Test Mac, 2026-10-10, at 2d6e921 (details and log lines: #5310 (comment)):
workcall answered at once ("still checking", "right after the one I'm on", "2 in line"); all three results returned through their calls and were read out;workuntil the user spoke again; that is why the pending message is kept.At 19ecda8 (#5310 (comment)): cancel of a queued task decided by the table with no instruction, and a page closed while the call waited (result to the DM, call ended).
A #5288 run at 19ecda8 (#5310 (comment)): two results held through a meeting, said whole after the exit, no interruption.
Not live-verified: an aborted call (not reproducible by voice), and
answeredBydedup (the core answered each task separately in every run); both are covered by the unit and e2e tests.🤖 Generated with Claude Code