Skip to content

fix(voice): keep the run open while a tool holds the floor - #6694

Merged
longcw merged 3 commits into
mainfrom
longc/foreground-run-guard
Aug 5, 2026
Merged

fix(voice): keep the run open while a tool holds the floor#6694
longcw merged 3 commits into
mainfrom
longc/foreground-run-guard

Conversation

@longcw

@longcw longcw commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Fixes #6658

The bug

Under session.run(), a tool that calls update_agent() inside ctx.foreground() records no handoff and nothing after it.

ctx.foreground() waits for the activity to become idle. The activity becomes idle only after the speech handle of the turn completes, and that same completion ends the RunResult. The run finishes while the tool still waits for the floor. update_agent() then finds a run that is already done, so _watch_handle does nothing.

The fix

  • _wait_for_idle_and_hold registers a guard on the active run before it waits for the floor, and releases it on exit. A run cannot complete while a tool waits for the floor or holds it.
  • AgentTask suspends that guard with the blocked tasks it already suspends. Without this step, await AgentTask() inside ctx.foreground() deadlocks. The run waits for the guard, the guard waits for the task, and the task waits for a user turn. That turn only arrives after run() returns.
  • _watch_handle and the methods next to it accept asyncio.Future. asyncio.Task is a subclass of Future, so every current caller stays valid.

Scope

A tool that returns early with ctx.update() and then works in the background never takes the floor. It gets no guard, so the run keeps its own timing.

A tool that returns early, waits, and then takes the floor much later is still not recorded. Nothing holds the floor in that window, and the later work belongs to a new turn.

ctx.foreground() waits for the activity to become idle, and that same
transition is what completes the RunResult. The run therefore finished
while the tool was still waiting for the floor, so a handoff made inside
the scope went unrecorded.

_wait_for_idle_and_hold now registers a guard on the active run before it
waits, and releases it on exit. AgentTask suspends the guard along with
the blocked tasks it already suspends, so a foregrounded task that needs
a future user turn still lets run() return.
@longcw
longcw requested a review from a team as a code owner August 4, 2026 07:23

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

The circular wait one level deeper than the existing case: the foreground
scope holds a generate_reply, and a tool of that reply awaits a task that
needs a future user turn.
devin-ai-integration[bot]

This comment was marked as resolved.

@biztex

biztex commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

I'd misdiagnosed this one on the issue — I reproduced without ctx.foreground() and blamed the pre-1.6.7 on_enter gap. Reaching idle being what completes the run is the part I missed. I've corrected that on #6658 so the reporter isn't left with my workaround.

Independently verified the branch, since I had the context loaded:

  • Full pytest --unit: 1708 passed. (One unrelated failure, test_ipc.py::test_proc_pool, is a local native-DLL memory error under full-suite load — passes in isolation.)
  • A/B'd the guard by stubbing only its registration in _wait_for_idle_and_hold and leaving everything else identical: 2 failed, 4 passed, failing on exactly the two handoff tests. So they're testing the guard rather than passing incidentally.
  • guard.set_result(None) in the finally looked like it could hit InvalidStateError if anything cancelled a watched handle, but nothing in RunResult ever cancels what it watches — the only mutations are add/discard — so it holds.
  • Registering before the wait_for_idle() await, with the wait moved inside the try, is what makes the ActivityClosedError path safe; test_guard_released_when_the_floor_never_arrives pins it.

One observation, not a request. In the AgentTask restore path the suspended handles are re-watched into session._global_run_state read after self.__fut, which may be a newer run than the one they were suspended from. That's pre-existing for speech handles and blocked tasks, and for a guard it seems right anyway — the tool still holds the floor, so the new run's turn genuinely isn't finished. Noting it only because guards now ride that path too, and the comment above it explains the unwatch but not the re-watch target.

The guard was taken from the session's active run at scope entry, with no
link to the turn of the caller. A tool that answered early with ctx.update()
and kept working could enter ctx.foreground() while a later session.run()
was in flight, and hold that unrelated run open for the whole scope.

RunContext now records the run its call belongs to, and a scope guards only
that run while it is still the active one.
@longcw
longcw merged commit 9e24530 into main Aug 5, 2026
25 checks passed
@longcw
longcw deleted the longc/foreground-run-guard branch August 5, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test framework issues when updating agent from tool

3 participants