[yenuo26]

Contributor intent (required human answers)

1. What outcome do you want to achieve?

Fix the READY CI failure in build #17710: eliminate the flaky failure of test_master_port_env_takes_precedence_over_kwarg (AssertionError: assert 34510 == 34473) so READY CI builds stop failing on this pre-existing race. (Contributor's request: fix READY CI build #17710; the failure is pre-existing — the same contention signature appears in passing main builds and the failing test is outside PR #9046's diff.)

2. What is the highest-leverage variable for that outcome?

Test determinism — whether the assertion depends on unreserved OS port state. The test's sensitivity to concurrent port steals directly decides whether this flake recurs. (Contributor's explicit answer: 测试确定性 / test determinism.)

3. How will this work move that variable?

The flaky tests obtained ports via get_open_port() without reserving them, so a concurrent process could steal a port between selection and assertion; OmniDiffusionConfig.settle_port then stepped +37 by design (PR #3803, issue #3794) and the equality assertion failed. This PR removes that dependency:

• The four precedence/stability tests now use fixed ports with port-availability probing frozen via monkeypatch, making them fully deterministic while still exercising the env → kwarg → default resolution-precedence logic.
• The default-port availability test retries the resolve-and-bind sequence (bounded, 10 attempts) to ride out transient steals.
• The settle_port stepping test keeps its real reserved socket and is unchanged.

4. Whose customer or user needs does this address, and what are those needs?

Both (contributor's explicit answer: 两者都是):

• Contributors and maintainers need PRs not to be blocked or confused by a pre-existing flake, and need trustworthy READY CI signals for merge decisions.
• The CI triage team needs reliable READY CI results to attribute failures (PR-introduced vs pre-existing vs infra) in each inspection window without re-triaging a known flaky signature.

Purpose

Before: test_master_port_env_takes_precedence_over_kwarg (and the other get_open_port()-based tests in this file) could fail whenever a concurrent process bound the selected ephemeral port between get_open_port() and config construction; settle_port steps such a port by +37 by design, so assert config.master_port == env_port broke (build #17710: 34473 stolen → stepped to 34510).

After: those assertions are deterministic and independent of concurrent port state; the default-port test tolerates transient steals via bounded retry; the stepping test is unchanged.

Test-only change: vllm_omni/diffusion/data.py resolution behavior is untouched. No public configuration, defaults, or supported behavior changes, so no design contract or user-guide update is needed.

Test Plan

vLLM Version: 0.32.0 (CI pin: commit 5df63a7203cdb9328d824a27b682e77c7b9928be; local validation used the v0.32.0 release wheel)

vLLM-Omni Commit: main 95b1d97f2 + this PR

Test Result

Mechanism reproduced on main code: with MASTER_PORT=39001 and port 39001 held by a concurrent process, OmniDiffusionConfig(model="test", master_port=39002) logs Port 39001 was unavailable, using port 39038 instead — the exact +37 stepping that produced assert 34510 == 34473 in build #17710 (34473 + 37 = 34510).

Validation of this PR's test file on a checkout of main 95b1d97f2 (vllm 0.32.0, CPU, HF_HUB_OFFLINE unset to match CI):

• pytest tests/diffusion/test_omni_diffusion_config_master_port.py -v — 6/6 passed
• Same command with ports 39001 and 39002 deliberately held by a concurrent process (the #17710 contention class) — 6/6 passed
• 5 consecutive repeats — 6/6 passed each

Checks not run locally: the full pre-commit hook set (mypy et al.); ruff check, ruff format --check, and py_compile pass. The complete READY CI suite runs on this PR.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	⚠️ Failed 2026-10-21T10:17:29.753552Z We couldn’t complete this review. Comment @codex review to try again.	7075023	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[vllm-omni-review-bot]

This PR matches CODEOWNERS paths: /tests/.

Code owners: @NickCao

Routing: @NickCao via CODEOWNERS

@yenuo26, 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.

[yenuo26]

Self-review per the assignment request:

• Scope: single test file (tests/diffusion/test_omni_diffusion_config_master_port.py); no production code touched — OmniDiffusionConfig port-resolution behavior is unchanged.
• Root cause reproduced before fixing: with MASTER_PORT=39001 held by a concurrent process, settle_port steps +37 by design (Port 39001 was unavailable, using port 39038 instead) — the mechanism behind READY CI #17710's assert 34510 == 34473 (34473 + 37 = 34510).
• Determinism: the four precedence/stability tests use fixed ports with availability frozen via monkeypatch of vllm_omni.diffusion.data.is_port_available. Validated on main 95b1d97f2 (vllm 0.32.0, CPU): 6/6 pass, including with ports 39001/39002 deliberately held by another process, and across repeated runs.
• CodeQL finding (binding all interfaces): fixed by binding 127.0.0.1. While re-validating I found the first version of that fix had a real hole — a raw loopback bind used for the helper's candidate check can succeed while the port is held on a non-loopback interface (verified on a multi-homed host: loopback bind succeeds, is_port_available wildcard probe fails), which would make settle_port step again and break test_settle_port_steps_when_port_is_busy on hosts with interface-specific listeners. The candidate check now calls is_port_available itself. Re-validated with holders on both loopback and a non-loopback interface: 6/6 across repeated runs.
• Not run locally: the full pre-commit hook set (mypy et al.); the PR's pre-commit job covers it.