fix(aws): hold on to the user-text and deferred-recycle tasks

[Rehansanjay]

The bug

RealtimeSession already tracks and cancels four background tasks in aclose() — _session_recycle_task, _response_task, _audio_input_task and _main_atask. Two others are created and discarded. The event loop keeps only a weak reference to a bare task, so each can be garbage collected before it finishes, and neither is cancelled when the session closes.

_send_user_text

async def _send_user_text(text=text, fut=fut) -> None:
    await self._stream_ready.wait()
    await self._send_text_message(text, interactive=True)
    ...

asyncio.create_task(_send_user_text())

self._pending_generation_fut = fut is set just above, then the task is dropped. If it is collected while waiting on _stream_ready, the message is never sent and the future is never resolved — a generate_reply(user_input=...) waits forever.

Surviving past close is no better: nothing cancels it, so it can wake after aclose() and send on a stream that is gone.

_deferred_tool_recycle

asyncio.create_task(self._deferred_tool_recycle())

It sleeps 0.15s, then recycles the session so the new tool set is sent in the next prompt block. Collected during that sleep, the recycle never happens and the new tools are silently never applied — update_tools() returned normally, and the logs even say the recycle was scheduled.

It also had no replace-in-place guard, so two quick update_tools() calls race two recycles against each other. _start_session_recycle_timer already avoids exactly this:

if self._session_recycle_task and not self._session_recycle_task.done():
    self._session_recycle_task.cancel()

The fix

• _send_user_text tasks go into a set that discards on completion, and are cancelled in aclose().
• _deferred_tool_recycle is stored and follows _start_session_recycle_timer's replace-in-place pattern, and is cancelled in aclose().

Both cancellations sit next to the recycle-timer cancellation already in aclose(), and append to the same tasks list that is gathered at the end.

No behaviour change on the success path.

Checks

ruff check and ruff format --check pass on the changed file.

Note: this does not touch _session_recycle_task or _start_session_recycle_timer, so it should not conflict with #6281.

---

Found with a small AST pass looking for create_task calls whose result is discarded — same sweep as #7050 and #7051.

[devin-ai-integration]

Note

This report is out of date. Scroll down for Devin Review's latest report on this PR.

Devin Review found 2 potential issues.

[devin-ai-integration]

🟨 Provider failures expose sensitive data

A failed deferred recycle sends logger.exception the provider traceback. It can expose customer content or credentials through exception messages and causes.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[tinalenguyen]

hi, thanks for the PR! could you address the devin comments if applicable?

[Rehansanjay]

Thanks for looking — both addressed in c71252e.

Rapid tool updates strand sessions. This one was real and I introduced it. _deferred_tool_recycle debounces for 150ms and then tears the session down and brings it back, so only the debounce is safe to cancel; a second update_tools landing inside _graceful_session_recycle cancelled after _is_sess_active was cleared but before the streams restarted, and the replacement task then woke, saw an inactive session and returned, leaving it stopped. I had mirrored _start_session_recycle_timer, which is wrong here — that one is a bare sleep with no critical section.

Cancellation is now limited to the debounce. Past that point an incoming update sets a pending flag and the running task loops once more, so the newest tool set still reaches the session rather than racing the teardown.

Deferred failures expose provider details. Nothing awaited the task, so a failure surfaced through asyncio's unretrieved-exception handler, outside the plugin's PII-tagged logging. It is now caught and logged through the plugin logger, with CancelledError re-raised so cancellation still behaves normally.

Worth noting utils.log_exceptions was not enough on its own here — it logs but re-raises, so the exception would still have gone unretrieved.

[Rehansanjay]

Two more from the follow-up review, both fixed in 01f6f2b.

Shutdown can restart AWS sessions — this one is real and worth spelling out. aclose() treated a clear _is_sess_active as "already inactive" and returned. But _graceful_session_recycle clears that event and only sets it again after initialize_streams(is_restart=True), so for the whole length of a recycle the session reads as inactive while still owning live tasks. aclose() landing in that window cancelled nothing, and the recycle then opened a fresh session after the caller had closed it.

There is now a _closing flag that is never cleared, unlike _is_sess_active. aclose() sets it first and cancels/awaits the deferred recycle and user-text tasks before the inactive check rather than after, and _deferred_tool_recycle checks it before entering its critical section and before looping.

Tool identifiers bypass log redaction — tool_use_id now travels in extra under an lk.pii. key instead of being interpolated into the message body, matching the other customer-correlated values in this file.

[Rehansanjay]

Both fixed in 55be0d8, and both were fallout from my previous commit — worth stating plainly.

Shutdown strands recycled session resources. The real mistake was using _is_sess_active as the early-return guard at all. That event says nothing about whether the session was closed: _graceful_session_recycle clears it for the length of its teardown and sets it again afterwards. So cancelling the recycle could land inside that window and aclose() would return before closing the streams, the response task, the audio input task or the main task.

The guard is now _closing, which is never cleared, so close is properly idempotent. Only the prompt-end block stays conditional on the session being live; every resource teardown after it runs either way, because an interrupted recycle leaves the active event clear while its streams are still open.

Closed text replies never finish. _pending_generation_fut is now settled unconditionally, before anything can return. Cancelling a queued user-text send raises CancelledError, which _send_user_text's except Exception does not catch, so a caller of generate_reply(user_input=...) would have waited on a future nothing would resolve.

@tinalenguyen — this has grown past the two-line fix it started as. Every finding has been real, but if you'd rather this PR stay narrow, I am happy to reduce it to just holding the two task references and open the aclose lifecycle work separately. The aclose/recycle race is not something this PR introduced; the session recycle timer has the same window on main today.

[Rehansanjay]

Fixed in bdd0483 — and this one was a regression I introduced, so worth being explicit.

The _closing guard I added treated starting a shutdown as having finished one. If the first aclose() raised or was cancelled partway, every later call returned immediately and the remaining tasks and streams were never released. Before this PR there was no guard at all, so a retry did run — I made it worse.

Shutdown now runs as a single shared task. _closed is set only after cleanup actually completes, so a failed or cancelled attempt leaves it False and the next aclose() starts a fresh one. Concurrent callers await the same task instead of racing two teardowns, and asyncio.shield stops one caller's cancellation from aborting cleanup for everyone else. _closing is still set on entry, before any of that, so an in-flight recycle stops even if the cleanup task has not been scheduled yet.

@tinalenguyen my offer from the previous comment stands and I think it is worth taking seriously now: this started as six lines holding two task references and has become a rework of the session shutdown path. Every finding has been genuine, but the last three were all consequences of my own previous fix rather than of the original bug, and the aclose/recycle race exists on main today independently of this PR.

Happy to reduce this to just the two task references and open the shutdown lifecycle work as its own PR, where it can be reviewed on its own terms. Your call — I will do whichever you prefer.

[Rehansanjay]

On the remaining open thread — the exc_info one — I do not think it is actionable, and I would rather say so than quietly churn.

The earlier review on this series flagged the opposite problem: the prewarm failure interpolated the exception into the message body, where redaction cannot reach it. I moved it to exc_info, which is what the send and recv loops in this same file already do. The new finding says exc_info is itself the problem.

Taken literally that leaves nowhere to put the exception, and it would put this call site out of step with the rest of the codebase: exc_info= appears at 99 call sites across livekit-agents and livekit-plugins today. Dropping the exception entirely would mean a prewarm or a recycle can fail and leave an operator with a log line and no cause.

If the project does want a stricter rule for provider exceptions — a redacting formatter, or an lk.pii. structured field rather than exc_info — I am happy to follow it here and it would probably be worth applying repo-wide rather than to these two lines. Left as-is for now pending your view.

Separately, the shielded-shutdown finding is fixed in 1f48ba7: asyncio.shield kept the shared close task alive when its caller was cancelled but left it with no observer, so a late cleanup failure was discarded silently — the same unretrieved-task-exception problem this PR is about. A done callback now retrieves and logs it, and _closed still only flips after cleanup completes so the retry path holds.

[devin-ai-integration]

Devin Review found 1 new potential issue.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

[devin-ai-integration]

🟨 Shutdown failures expose customer data

When shutdown fails, _on_close_task_done logs the original exception and traceback. Provider errors can include customer payloads that enter unredactable logs.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[Rehansanjay]

Duplicate shutdown logging fixed in baff2d8 — correct catch. aclose carries @utils.log_exceptions, so a failure reaching an awaiting caller was already recorded there and my done callback logged it again, doubling error records and any alerting on them. The callback still retrieves the exception unconditionally so it is never reported as never-retrieved, but it only logs when no caller is left waiting, which is the case it exists for.

I am going to stop making changes here pending your view on scope. Where this stands:

Landed and confirmed resolved — the recycle cancellation during teardown, task cleanup bypassed by the inactive early-return, shutdown reopening the session, retries blocked after a failed close, the shielded close swallowing its own failure, the duplicate log, and the PII-tagged tool_use_id.

Still open, and I do not intend to change it without your say-so: the exc_info finding, for the reason in my previous comment — exc_info= is used at 99 call sites across livekit-agents and livekit-plugins, and the earlier review on this same series asked me to move to it from message-body interpolation.

The wider point I raised before still stands. This began as six lines holding two task references, and the last four commits have all been fixing consequences of my own previous fix rather than the original bug. The aclose/recycle race predates this PR — the session recycle timer has the same window on main today. I would rather you decide the shape than have it keep growing: happy to reduce this to just the two task references and open the shutdown lifecycle work separately, or leave it as it stands. Either is fine by me.