[NumberWan]

Summary

Follow-up correctness fix from the Unified Full-duplex Framework review backlog in #7636. This PR covers Issue 9 (P2).

This is a small Serving transport correctness bug, not AURA-specific. No scheduler or plugin changes.

Issue 9 — Session kept alive for an impossible resume when session.created is not delivered

Where: vllm_omni/entrypoints/duplex/serving.py (_send_event)

Bug: Any pump send failure was treated as a disconnect of the current attachment: _detach_current_attachment keeps the engine session alive for resume (journal + disconnect grace). That is correct for mid-session failures (e.g. an audio delta after handshake), where the client already has session_id / resume_token.

When session.created itself never reaches the client, nobody can call session.resume. Two ways that happens:

1. send_json raises while sending session.created.
2. The reader detaches first. send_event then finds no attachment and, because session.created is not journaled, returns without raising. The close path never ran, so the admission slot stayed held until expiry.

Fix:

• Close via _close_undeliverable_session (handle.close(reason="session_created_undelivered")) unless the handshake was actually written to the socket.
• Delivery is the unjournaled on_accepted callback after a successful transport send. A journal record, or a detached no-op return, is not delivery. An in-flight send that then raises still closes; one that completes leaves the session resumable.
• Later send failures keep the detach path.
• The pump finally still drops _pumps and the attachment registry entry.

Test plan

• pytest tests/entrypoints/duplex/test_duplex_serving.py -o addopts='--strict-markers --strict-config' (20 passed)
• Raised send: test_session_created_send_failure_closes_instead_of_detaching
• Detach-before-send: test_disconnect_before_session_created_closes_instead_of_staying_detached
• Mid-session audio send failure still detaches: test_transport_send_failure_detaches_the_session_instead_of_closing_it
• Manager admission: test_undeliverable_handshake_close_releases_admission_after_detach (max_sessions=1; detach still rejects the next open; close(reason="session_created_undelivered") admits it). Both serving triggers call this close. FakeOmni does not model admission.

Files

Path	Change
vllm_omni/entrypoints/duplex/serving.py	Close when session.created is not transport-delivered
tests/entrypoints/duplex/test_duplex_serving.py	Raised-send and detach-before-send regressions
tests/engine/duplex/test_session_manager.py	max_sessions=1 slot release after that close

[vllm-omni-review-bot]

This PR appears to belong to: docs/design/module/profiling.md, docs/design/module/entrypoints.md, docs/design/module/observability.md.

Module owners: @alex-jw-brooks @linyueqian @NickCao

Routing: @alex-jw-brooks via module of the changed files, module named in the PR description, semantic router, CODEOWNERS; @linyueqian via module of the changed files, module named in the PR description, semantic router, CODEOWNERS; @NickCao via module of the changed files, module named in the PR description, semantic router, CODEOWNERS

@NumberWan, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

[vllm-omni-review-bot]

Omni ReviewBot triage note

Resolved as of 5b8264547645: the high-priority or low-quality signal noted on an earlier commit no longer applies.

[hsliuustc0106]

Ready for full review when [WIP] is removed from the title.

[NumberWan]

This PR appears to belong to: docs/design/module/profiling.md, docs/design/module/entrypoints.md, docs/design/module/observability.md.

Module owners: @alex-jw-brooks @linyueqian @NickCao

Routing: @alex-jw-brooks via module of the changed files, module named in the PR description, semantic router, CODEOWNERS; @linyueqian via module of the changed files, module named in the PR description, semantic router, CODEOWNERS; @NickCao via module of the changed files, module named in the PR description, semantic router, CODEOWNERS

@NumberWan, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer.

Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment.

Self-review: Added a function "_close_undeliverable_session" in serving.py. Before this PR, when a session pump a event failed. It will treat it as a " _detach_current_attachment"(offline and maybe resume) even the event having fully created.

After this PR, when the failed event is "SessionCreated", will call the _close_undeliverable_session → handle.close(reason="session_created_undelivered"). Otherwise will use the old method " _detach_current_attachment"

[vllm-omni-review-bot]

Omni ReviewBot: no human activity for 7 days

@NumberWan this pull request has had no human commit, comment or review since 2026-09-23. Please confirm the current plan and next step. The author or a maintainer decides whether to change the PR state.

To keep it moving, any one of these is enough: push an update, reply to the open blocker, or post the current plan and timeline.

[NumberWan]

This PR will be scheduled to merge after 5/10/2026

[chickeyton]

LGTM

[vllm-omni-review-bot]

Omni ReviewBot routing record

Assigned Strict on zcode (GLM-5.3-Flash) under experiment fleet-strict-cursor-grok46-zcode-glm53flash-5050-c5-z10-20261002.

[NumberWan]

[buildkite/vllm-omni-amd-ci](https://buildkite.com/vllm/vllm-omni-amd-ci/builds/13420)
buildkite/vllm-omni-amd-ci — Build #13420 failed (40 minutes, 15 seconds)
[buildkite/vllm-omni-intel-ci](https://buildkite.com/vllm/vllm-omni-intel-ci/builds/9707)
buildkite/vllm-omni-intel-ci — Build #9707 failed (25 minutes, 19 seconds)

These two CI failures are not caused by this PR.

This PR only changes the duplex websocket send-failure path, and only when the event that failed to send is session.created.

No diffusion, sleep-mode, model-parallel, Qwen2.5-Omni, or XPU code is touched. The new branch runs only inside the realtime websocket pump after send_json raises.

1. Diffusion custom-pipeline failures (DiffusionWorker pid=181)
   FAILED tests/e2e/features/custom_pipeline/test_diffusion_pause_keep.py::test_sleep_right_after_keep_pause_keeps_the_running_batch[uni]
   RuntimeError: Orchestrator initialization failed: Sleep mode can only be used for one instance per process.
   FAILED tests/e2e/features/custom_pipeline/test_worker_extension.py::test_collective_rpc_test_extension_name
   RuntimeError: Orchestrator initialization failed: model parallel state must be empty before initialization; found dp, cfg, sp, pp, vllm_tp, vllm_dp, vllm_pp
   Both tests fail while constructing the diffusion orchestrator, before any request is served. The messages are process-global leftovers in that same DiffusionWorker: a sleep-mode instance is already registered, and parallel groups (dp, cfg, sp, pp, …) were not cleared. 11 other tests in that job passed. This PR never imports those tests and never touches sleep mode or model-parallel init, so it cannot leave that state behind.

2. Qwen2.5-Omni XPU errors (xpu_9ea9a6c…, this PR's commit)
   ERROR tests/e2e/offline_inference/test_qwen2_5_omni_expansion.py::test_mix_to_audio[omni_runner0-xpu]
   ERROR tests/e2e/offline_inference/test_qwen2_5_omni_expansion.py::test_text_to_text[omni_runner0-xpu]
   5 skipped, 8244 deselected, 23 warnings, 2 errors in 117.01s
   These are ERRORs (setup/runtime), not assertion failures, in the offline Qwen2.5-Omni expansion suite. 8244 tests were deselected; the job did not run the new duplex unit test. Nothing in this diff is on the offline-inference or omni_runner0-xpu path.

A failure caused by this PR would show up as test_session_created_send_failure_closes_instead_of_detaching (or another tests/entrypoints/duplex/ case). Neither log does.

[amy-why-3459]

Reviewed 9ea9a6c430feebc92d389935615f3a80641857d6, which is still the current head. Closing on a failed SessionCreated send is the right direction, but another pre-handshake disconnect path still retains an unresumable session.

[P2] Also close when the connection detaches before SessionCreated is sent

At vllm_omni/entrypoints/duplex/serving.py:405-414, the new close path requires send_event() to raise. Consider this ordering:

1. The engine session is opened, but the pump has not sent session.created yet.
2. The read loop receives a disconnect and _on_disconnect() detaches the resumable attachment.
3. The pump then processes SessionCreated.
4. DuplexSessionAttachmentRegistry.send_event() finds state.attachment is None and returns without sending or raising.
5. _close_undeliverable_session() is never called. The client has no session ID/resume credentials, but the session retains its admission slot until expiry.

An isolated harness executing the relevant methods extracted from this head, with stubbed surrounding objects, produced transport_sends=0, close_calls=0, detach_calls=1 for that sequence. This validates the control-flow gap, not a full engine/end-to-end reproduction.

Please track successful initial-handshake transport delivery and close a session that disconnects before that handshake completes. Distinguish journal acceptance from successful transport delivery, and handle the in-flight-send/disconnect race. Add a regression that delays the first event pump, disconnects the reader, then lets the pump process SessionCreated.

Please verify actual admission-slot release

The new test verifies that the fake handle is closed rather than detached. It does not verify that the production manager releases capacity. Please add a manager/integration regression with max_sessions=1 showing that a second session can open after the failed initial handshake is cleaned up. Cover both a raised send failure and detach-before-send, and retain the mid-session send-failure test demonstrating that a successfully established session remains resumable.

The PR description also still claims a FakeOmni.max_sessions capacity test and a successful second open; the current commit removed those assertions. Please update the description to match the actual coverage.

Validation status

The local command python -m pytest -q -rs tests/entrypoints/duplex/test_duplex_serving.py failed during import because the installed vLLM lacks vllm.v1.outputs.RoutedExpertsLists. No local pytest pass is claimed. At review time CUDA/NPU CI statuses were successful and AMD/Intel statuses were failing.

Please provide complete repository unit-test evidence tied to this head or the updated head after fixes, using the supported CI unit-test shards and applicable backend jobs, including the entrypoint/engine coverage defined in .buildkite/cuda/test-ready.yml. Attach exact commands, environment/dependency versions, passed/failed/skipped/deselected/collection-error counts where available, and CI or complete-log links. Existing complete evidence on the same SHA can be linked without a redundant rerun. Explain skipped or unavailable jobs, and provide a same-environment base/head comparison for failures believed to be pre-existing. If coverage cannot be completed, obtain an explicit maintainer disposition for the remaining gap; mixed CI statuses and the local import failure do not establish merge readiness.

[BeatSeat]

Very clean fix. Detaching on an initial session.created send failure was indeed dangerous because the client never obtained the session ID or resume token, leaving the admission slot trapped until idle expiry.

Just double-checking the cleanup boundary in _close_undeliverable_session:

async def _close_undeliverable_session(self, session_id: str) -> None:
    handle = self._omni.get_session(session_id)
    if handle is None or handle.closed:
        return
    try:
        await handle.close(reason="handshake_undeliverable")
    except Exception:
        logger.warning("Failed to close undeliverable duplex session %s", session_id, exc_info=True)

If handle.close() happens to raise here (e.g., if downstream workers or runners encountered an error), we want to be sure that the handler's internal mappings (_live_envelopes, _pumps) are still guaranteed to be cleared so no zombie references remain in serving.py. From looking at _pump_events, the outer finally block appears to handle this cleanup, which is great.

The added test test_session_created_send_failure_closes_instead_of_detaching covers the admission slot recovery scenario very clearly. LGTM!

[NumberWan]

Reviewed 9ea9a6c430feebc92d389935615f3a80641857d6, which is still the current head. Closing on a failed SessionCreated send is the right direction, but another pre-handshake disconnect path still retains an unresumable session.

[P2] Also close when the connection detaches before SessionCreated is sent

At vllm_omni/entrypoints/duplex/serving.py:405-414, the new close path requires send_event() to raise. Consider this ordering:

1. The engine session is opened, but the pump has not sent session.created yet.
2. The read loop receives a disconnect and _on_disconnect() detaches the resumable attachment.
3. The pump then processes SessionCreated.
4. DuplexSessionAttachmentRegistry.send_event() finds state.attachment is None and returns without sending or raising.
5. _close_undeliverable_session() is never called. The client has no session ID/resume credentials, but the session retains its admission slot until expiry.

An isolated harness executing the relevant methods extracted from this head, with stubbed surrounding objects, produced transport_sends=0, close_calls=0, detach_calls=1 for that sequence. This validates the control-flow gap, not a full engine/end-to-end reproduction.

Please track successful initial-handshake transport delivery and close a session that disconnects before that handshake completes. Distinguish journal acceptance from successful transport delivery, and handle the in-flight-send/disconnect race. Add a regression that delays the first event pump, disconnects the reader, then lets the pump process SessionCreated.

Please verify actual admission-slot release

The new test verifies that the fake handle is closed rather than detached. It does not verify that the production manager releases capacity. Please add a manager/integration regression with max_sessions=1 showing that a second session can open after the failed initial handshake is cleaned up. Cover both a raised send failure and detach-before-send, and retain the mid-session send-failure test demonstrating that a successfully established session remains resumable.

The PR description also still claims a FakeOmni.max_sessions capacity test and a successful second open; the current commit removed those assertions. Please update the description to match the actual coverage.

Validation status

The local command python -m pytest -q -rs tests/entrypoints/duplex/test_duplex_serving.py failed during import because the installed vLLM lacks vllm.v1.outputs.RoutedExpertsLists. No local pytest pass is claimed. At review time CUDA/NPU CI statuses were successful and AMD/Intel statuses were failing.

Please provide complete repository unit-test evidence tied to this head or the updated head after fixes, using the supported CI unit-test shards and applicable backend jobs, including the entrypoint/engine coverage defined in .buildkite/cuda/test-ready.yml. Attach exact commands, environment/dependency versions, passed/failed/skipped/deselected/collection-error counts where available, and CI or complete-log links. Existing complete evidence on the same SHA can be linked without a redundant rerun. Explain skipped or unavailable jobs, and provide a same-environment base/head comparison for failures believed to be pre-existing. If coverage cannot be completed, obtain an explicit maintainer disposition for the remaining gap; mixed CI statuses and the local import failure do not establish merge readiness.

Thanks for the review. Head 5b8264547 covers the pre-handshake detach path.

session.created is unjournaled, so send_event returning is not delivery. Delivery is the on_accepted callback after a successful socket send.

• If that send raises, we still close (session_created_undelivered) instead of detaching.
• If the reader detaches first, send_event returns without raising and we close on the missing transport callback. test_disconnect_before_session_created_closes_instead_of_staying_detached delays the pump, disconnects the reader, then lets the pump process SessionCreated.
• If an in-flight send completes, the callback has already run and the session stays resumable. A later audio send failure still detaches (test_transport_send_failure_detaches_the_session_instead_of_closing_it).

Admission: FakeOmni does not model capacity. test_undeliverable_handshake_close_releases_admission_after_detach uses DuplexSessionManager with max_sessions=1: after detach the next open is resource_exhausted; close(reason="session_created_undelivered") admits it. Both serving triggers call that same close. The raised-send case is test_session_created_send_failure_closes_instead_of_detaching.

Local: pytest tests/entrypoints/duplex/test_duplex_serving.py -o addopts='--strict-markers --strict-config' — 20 passed, plus the manager test above. The PR description now matches this coverage (no FakeOmni.max_sessions claim). CI on 5b8264547 is the remaining check.

[natureofnature]

LGTM. The updated handling covers both a failed initial session.created send and disconnect-before-send, while preserving resumability after a successful handshake. The manager regression also verifies that admission capacity is reclaimed.

Please resolve the outstanding CI checks before merging.