Skip to content

fix(amd): start SIP detection timeout after answer - #2226

Open
rosetta-livekit-bot[bot] wants to merge 2 commits into
mainfrom
routs-faking-banks
Open

fix(amd): start SIP detection timeout after answer#2226
rosetta-livekit-bot[bot] wants to merge 2 commits into
mainfrom
routs-faking-banks

Conversation

@rosetta-livekit-bot

@rosetta-livekit-bot rosetta-livekit-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Ports livekit/agents#6580 to the JS AMD implementation.

Summary

  • start the AMD detection budget only when listening begins, after SIP answer gating
  • settle as uncertain with participant_missing when participant audio never arrives or the participant disappears
  • update the telephony example to run AMD before dialing and bound the answered SIP request
Source diff coverage
  • examples/telephony/amd.py: adapted to examples/src/telephony_amd.ts. Registers room cleanup before dialing, starts AMD before creating the SIP participant, uses the JS SDK timeout: 45 option, handles call failure, and resolves the answered participant directly from room.remoteParticipants.
  • livekit-agents/livekit/agents/utils/participant.py: ported to agents/src/utils.ts. waitForTrackPublication rejects when the requested participant disconnects and removes the listener during cleanup.
  • livekit-agents/livekit/agents/voice/amd/classifier.py: adapted into agents/src/voice/amd.ts because JS combines classifier and detector. The fallback settlement path is generalized and supports immediate settlement before listening.
  • livekit-agents/livekit/agents/voice/amd/detector.py: adapted into agents/src/voice/amd.ts. Detection timing starts in startListening; track publication is bounded to 5 seconds; track timeout, participant disappearance, and SIP answer failure settle participant_missing.
  • tests/test_amd_classifier.py: ported to agents/src/voice/amd.test.ts. Source-equivalent regressions cover timer arming, pre-track disconnect, publication timeout, post-subscription disappearance, SIP answer failure, and pre-listening settlement.

No source files were omitted and no target infrastructure gap remains.

Validation

  • pnpm test agents/src/voice/amd.test.ts (36 passed)
  • pnpm build (40/40 workspace tasks passed)
  • pnpm lint (passed; existing warnings only)
  • Prettier check on all changed files (passed)
  • cue-cli voice-mode run emitted amd_prediction with AMD_HUMAN for a live-person utterance

Full touched-package suites were also run. They reproduce failures already present on main and unrelated to this port:

  • pnpm test agents: 1582 passed, 5 skipped, 2 failed in remote_session.test.ts; main forwards agent_false_interruption but its expected event set omits it.
  • pnpm test examples: 95 passed, 2 skipped, 2 failed in existing task/judge flows, plus 3 unrelated leaked async errors.

Ported from livekit/agents#6580

Original PR description

Carrier early media can publish audio before answer, exhausting AMD’s detection budget during ringback and misclassifying late answers.

Start the budget only when listening begins, and settle immediately if the participant disappears before audio arrives. This builds on #6386 by @dexhunter, credited as a commit co-author.

Fixes livekit/agents#6187. Closes livekit/agents#6187.

@rosetta-livekit-bot
rosetta-livekit-bot Bot requested a review from a team as a code owner August 5, 2026 14:18
@changeset-bot

changeset-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 5df65ae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 39 packages
Name Type
@livekit/agents Patch
@livekit/agents-plugin-anam Patch
@livekit/agents-plugin-anthropic Patch
@livekit/agents-plugin-assemblyai Patch
@livekit/agents-plugin-azure Patch
@livekit/agents-plugin-baseten Patch
@livekit/agents-plugin-bey Patch
@livekit/agents-plugin-cartesia Patch
@livekit/agents-plugin-cerebras Patch
@livekit/agents-plugin-deepgram Patch
@livekit/agents-plugin-did Patch
@livekit/agents-plugin-elevenlabs Patch
@livekit/agents-plugin-fishaudio Patch
@livekit/agents-plugin-google Patch
@livekit/agents-plugin-hedra Patch
@livekit/agents-plugin-hume Patch
@livekit/agents-plugin-inworld Patch
@livekit/agents-plugin-krisp Patch
@livekit/agents-plugin-lemonslice Patch
@livekit/agents-plugin-liveavatar Patch
@livekit/agents-plugin-livekit Patch
@livekit/agents-plugin-minimax Patch
@livekit/agents-plugin-mistral Patch
@livekit/agents-plugin-mistralai Patch
@livekit/agents-plugin-neuphonic Patch
@livekit/agents-plugin-openai Patch
@livekit/agents-plugin-perplexity Patch
@livekit/agents-plugin-phonic Patch
@livekit/agents-plugin-protoface Patch
@livekit/agents-plugin-resemble Patch
@livekit/agents-plugin-rime Patch
@livekit/agents-plugin-runway Patch
@livekit/agents-plugin-sarvam Patch
@livekit/agents-plugin-silero Patch
@livekit/agents-plugin-soniox Patch
@livekit/agents-plugin-tavus Patch
@livekit/agents-plugins-test Patch
@livekit/agents-plugin-trugen Patch
@livekit/agents-plugin-xai Patch

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 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 found 2 potential issues.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread agents/src/voice/amd.ts
Comment on lines +651 to +656
const trackGateAbort = this.trackGateAbort;
const publicationTimeout = setTimeout(() => {
if (this.trackGateAbort !== trackGateAbort || this.settled) return;
this.settleParticipantMissing('timed out waiting for participant audio track');
trackGateAbort.abort();
}, TRACK_PUBLICATION_TIMEOUT_MS);

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.

🔴 Outbound calls can be abandoned as "no participant" five seconds after dialing starts

The five-second budget for the caller's audio to arrive is started when detection begins (setTimeout(...) at agents/src/voice/amd.ts:652-656) instead of after the call is answered, so a call that is still ringing is given up on and reported as having no participant.
Impact: Outbound calls that ring for more than a few seconds are classified as uncertain/participant_missing and the agent stops listening even though the callee later answers.

Ordering of track-publication bound vs. SIP answer gate

The PR's stated design is "start AMD before creating a SIP participant so no audio is missed" and "the detection timeout begins only after listening starts; SIP settings bound the pre-answer phase". However gateListening() (agents/src/voice/amd.ts:631-716) arms TRACK_PUBLICATION_TIMEOUT_MS = 5_000 immediately at execute() time and only waits for the SIP sip.callStatus === active attribute after the publication resolves. So the 5s bound is applied to the pre-answer phase, which is exactly what the fix intends to leave unbounded (Python bounds track publication only after the call is answered).

In examples/src/telephony_amd.ts:105 detector.execute() is invoked before sip.createSipParticipant(..., { waitUntilAnswered: true, timeout: 45 }) (examples/src/telephony_amd.ts:131-136). If the SIP participant does not join and get its audio track subscribed within 5s (trunk setup latency, no early media until answer, ring window up to 45s), settleParticipantMissing('timed out waiting for participant audio track') fires and the whole AMD run resolves as uncertain/participant_missing while the call is still ringing. The abort also tears down the track listener, so late audio is never observed.

Note the previous code armed detectionTimer in execute() as a global backstop; that backstop was removed here (startDetectionTimer() now only runs from startListening()), so the 5s publication timer is the only thing bounding the pre-answer phase.

Prompt for agents
In agents/src/voice/amd.ts gateListening(), the 5-second TRACK_PUBLICATION_TIMEOUT_MS timer is started as soon as execute() runs, i.e. before the SIP call is placed/answered. Since AMD is now documented (and used in examples/src/telephony_amd.ts) to be started before createSipParticipant, this bound elapses while the call is still ringing and settles the run as uncertain/participant_missing. The upstream Python fix bounds track publication only after the answer gate. Restructure the gate so the pre-answer phase is unbounded (relying on participant disconnect / SIP dial timeout to settle participant_missing), and only apply the 5s publication budget once the target participant exists and, for SIP participants, once sip.callStatus is active — e.g. wait for the participant to join, then wait for the SIP answer attribute, then wait (bounded) for the subscribed audio publication before startListening().
Open in Devin Review

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

const detector = new voice.AMD(session, {
participantIdentity,
});
const detection = detector.execute();

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.

🟡 Example agent can crash with an unhandled error while a call is being dialed

The detection run is kicked off and left without any error handler (detector.execute() at examples/src/telephony_amd.ts:105) while the code awaits the outbound dial, so a failure during that window becomes an unhandled error that can terminate the process.
Impact: If detection fails (for example the session closes or the classifier errors) while the phone is still ringing, the example agent process can crash instead of logging the failure.

Unhandled rejection window between execute() and the awaits

const detection = detector.execute(); at examples/src/telephony_amd.ts:105 has no attached handler until await detection at line 167 or the finally at line 195. The intervening await sip.createSipParticipant(...) can take up to 45 seconds. AMD.execute() rejects on aclose() (AMD closed) and on LLM classification errors (scheduleLLMClassificationrejectRun). A rejection occurring during that window has no handler at the time Node checks the microtask queue, producing an unhandledRejection (fatal by default in modern Node).

Attaching a no-op catch (or a logging catch) immediately after creating the promise avoids this while still allowing await detection later.

Suggested change
const detection = detector.execute();
const detection = detector.execute();
// Keep a handler attached while we dial so an early failure isn't an unhandled rejection.
detection.catch(() => {});
Open in Devin Review

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

Copy link
Copy Markdown
Member

Port verification: livekit/agents#6580 → this PR

Compared this PR's diff against the merged Python sources (voice/amd/detector.py, voice/amd/classifier.py, utils/participant.py, examples/telephony/amd.py, tests/test_amd_classifier.py) line-by-line, plus the full pre-existing agents/src/voice/amd.ts for context the diff alone doesn't show.

Verdict: Faithful, careful port of a timing/concurrency-sensitive fix. No confirmed correctness regressions found across two passes. Labeled effort:high — the change rewires several interlocking async paths (track-publication gate, SIP-answer wait, detection/no-speech timers) across amd.ts, utils.ts, and the telephony example, and is worth a human reviewer's own trace through the race conditions rather than relying solely on this review.

Full analysis

utils.ts (waitForTrackPublication) vs participant.py (wait_for_track_publication)

  • The new onParticipantDisconnected listener, registered only when identity !== undefined (↔ Python's if identity is not None), rejects the pending future with the same message shape (Participant ${identity} disconnected while waiting for track publicationparticipant {identity!r} disconnected while waiting for track publication). Listener cleanup in finally mirrors Python's room.off(...) teardown.
  • The docstring update matches the behavior change 1:1.

amd.ts (gateListening) vs detector.py (_setup)

  • The 5s track-publication timeout (TRACK_PUBLICATION_TIMEOUT_MS_TRACK_PUBLICATION_TIMEOUT) is implemented via a manual setTimeout + AbortController in JS, standing in for Python's asyncio.wait_for(...). Traced the abort path: firing the timeout calls settleParticipantMissing then trackGateAbort.abort(), which synchronously rejects the waitForTrackPublication future via its abort listener; the .catch() handler checks trackGateAbort.signal.aborted and returns without double-settling. settle()'s own if (this.settled) return guard makes this idempotent either way.
  • Publisher resolution after the track resolves (lookup by identity, or by scanning trackPublications for the resolved sid) matches Python's if self._participant_identity: ... else: next(...) branch structure. (Aside, out of scope for this PR: JS's targetIdentity also falls back to roomIO?.linkedParticipant?.identity when participantIdentity is unset, a pre-existing JS-only fallback with no Python equivalent — untouched by this diff.)
  • SIP vs. non-SIP branching, and the _wait_for_sip_answer/waitForParticipantAttribute failure path, both settle participant_missing and both skip settling when torn down mid-wait (Python via CancelledError falling through the except RuntimeError, JS via the explicit trackGateAbort.signal.aborted check) — different mechanisms, same effect.
  • startListening() combining startDetectionTimer() with arming the no-speech timer matches detector.py's _start_listening() wrapping classifier.start_detection_timer() + classifier.start_listening() into one call.
  • Removing the eager this.startDetectionTimer() call from execute()'s promise executor (so the budget only starts once actual listening begins) matches Python: the classifier stays dormant until _start_listening fires it, which now only happens after the track/SIP gate clears.

settle/onTimeout rename

Cosmetic rename to match Python's already-settle-named method; the behavior (fallback verdict commit + gated emission, waitUntilFinished interaction) is unchanged from the pre-PR JS implementation and still matches classifier.py's settle.

Test coverage

All 5 new Python scenarios (arm-timer-only-at-listening, disconnect-before-track, publication-timeout, disappear-after-track, SIP-answer-failure) plus the standalone pre-listening-settle test are reproduced in amd.test.ts. One coverage-style gap: Python's version uses a _RecordingClassifier mock to assert start_detection_timer is called exactly once and only after the track resolves; JS's "arms detection timer only when listening starts" test instead asserts end-to-end timing behavior (a detection_timeout reason after a delayed TrackSubscribed). This is a reasonable consequence of JS merging classifier+detector into one class (no seam to mock at that granularity) rather than a missed port, but it is a slightly coarser assertion than the Python original.

examples/src/telephony_amd.ts vs examples/telephony/amd.py

  • Moving ctx.addShutdownCallback to immediately after session.start() (before AMD/SIP setup) matches Python's callback placement and fixes the stated bug (early call failures previously weren't cleaned up).
  • Calling detector.execute() immediately after construction and awaiting the stored promise later reproduces Python's async with AMD(...) as detector: semantics, where __aenter__ (not execute()) is what starts the internal setup/gating — JS's AMD has no context-manager equivalent, so starting execute() early and draining it in finally (await detection.catch(() => {})) is the correct adaptation to preserve the "start listening before dialing" guarantee.
  • The catch around sip.createSipParticipant is broader in JS (catches any error) than Python's except (api.SipCallError, asyncio.TimeoutError). In example code this is low-severity, but it does mean an unrelated bug (e.g. a JS TypeError from misconfiguration) would be silently treated as "call not answered" rather than surfacing. Worth a maintainer's call on whether to narrow it.

Not independently re-verified

Did not re-run the test suites; relied on the PR's own reported results (pnpm test agents/src/voice/amd.test.ts: 36 passed; full pnpm test agents: 1582 passed / 5 skipped / 2 failures in remote_session.test.ts that the PR attributes to a pre-existing, unrelated main issue). Did not attempt to enumerate every possible interleaving of track-subscribe/SIP-answer/timeout/abort beyond the ported test scenarios and the hand-traces above.


Generated by Claude Code

@chenghao-mou chenghao-mou self-assigned this Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AMD: detection_timeout still armed at track subscription poisons long-ring outbound SIP calls (follow-up to #5848)

1 participant