fix(voice): propagate job context into tool execute on Node 24#1592
Open
gidiupgidi wants to merge 1 commit into
Open
fix(voice): propagate job context into tool execute on Node 24#1592gidiupgidi wants to merge 1 commit into
gidiupgidi wants to merge 1 commit into
Conversation
Re-establish jobContextStorage inside the per-tool Task.from() body in performToolExecutions so getJobContext() works from a tool's execute() function on Node 24. Node 24's default AsyncContextFrame AsyncLocalStorage implementation does not propagate the job context across the Task.from() boundary the way the legacy async_hooks implementation did, which previously caused getJobContext() to throw "no job context found" inside tools (see livekit#1255).
🦋 Changeset detectedLatest commit: da12cbf The changes in this PR will be included in the next version bump. This PR includes changesets to release 33 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
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
2 participants
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.
Fixes #1255.
Problem
On Node 24,
getJobContext()throwsno job context foundwhen called inside a tool'sexecute()function. The workaroundNODE_OPTIONS=--no-async-context-framereverts Node 24 to the legacyasync_hooks-basedAsyncLocalStorage, which propagates the job context through implicitly.Root cause
Tool execution constructs a
TaskviaTask.from(async () => …)that wraps threeAsyncLocalStoragecontexts inside the task body:agentActivityStorage— wrapped inagent_activity.tsspeechHandleStorage— wrapped inagent_activity.tsfunctionCallStorage— wrapped ingeneration.tsjobContextStorageis not wrapped anywhere along this path. With the oldasync_hooks-basedAsyncLocalStorage(Node 22 /--no-async-context-frame), the job context propagated implicitly across theTask.from()boundary. With Node 24's defaultAsyncContextFrameimplementation, it does not, sogetJobContext()returnsundefinedinside tool execute functions. See nodejs/node#58204 for background on theAsyncContextFramechange.Fix
In
performToolExecutions(agents/src/voice/generation.ts):JobContextsynchronously at the top ofexecuteToolsTask, where it is still in scope.Task.from(...)body inrunWithJobContextAsync(currentJobContext, ...)so the context is re-established inside the new async scope, alongside the existingfunctionCallStorage.run(...)wrap.If no job context is in scope (e.g. unit tests calling
performToolExecutionsdirectly), the wrap is skipped so behaviour is unchanged.Test plan
pnpm -F @livekit/agents buildsucceedspnpm -w lint:fixclean (no new warnings)pnpm -w format:checkandpnpm throws:checkcleanpnpm test agents— full agents suite (60 files / 840 tests) passes on Node 24.10.0--no-async-context-frame:getJobContext()inside a tool'sexecute()returns the room name as expected. (Reporter — please verify with your repro.)Note on CI coverage
The
BuildandTestworkflows pinactions/setup-nodeto Node 20, where this bug does not manifest in the first place (Node 20 still uses theasync_hooks-basedAsyncLocalStoragethat propagates job context implicitly). The fix is a safe no-op on Node 20 — if no job context is in scope at the synchronous capture point, the extrarunWithJobContextAsync(...)wrap is skipped. So CI going green confirms no regression on Node 20 but cannot confirm the fix is effective on Node 24; that has to come from the reporter's repro.I tried adding a focused unit-level regression test that wraps
performToolExecutionsinrunWithJobContextAsyncand callsgetJobContext()from a tool, but the bug does not reproduce in that minimal harness on Node 24 — presumably because the lost-propagation only manifests through the deeperTask.from()chain established by the real agent lifecycle. Happy to add a heavier integration-level test if the maintainers can point me at the right harness.