[Bugfix]: strip @rank suffix in DP intra-node GET proxy URLs

[wuhang2014]

Fix #220: intra-node DP GET proxies fail when worker URLs contain @rank.

When --intra-node-data-parallel-size > 1, worker registry URLs carry an @rank suffix (e.g. http://127.0.0.1:18100@0). proxy_get_request forwarded these URLs to reqwest without stripping @rank, so reqwest parsed @ as RFC 3986 userinfo and sent the request to the rank digit as host instead of the worker (e.g. GET /v1/models became a request to http://0.0.0.0/v1/models and returned 500). Chat POST already stripped @rank; GET did not.

Purpose

Fix #220 (worker URLs with @rank are misinterpreted as userinfo in intra-node DP GET proxies).

• proxy_get_request now parses the worker URL with dp_utils::parse_worker_url and sets X-data-parallel-rank via dp_utils::add_dp_rank_header, matching the chat POST path. DP=1 is unchanged (no @rank, no extra header).
• Affected endpoints fixed: GET /v1/models, GET /health_generate, GET /get_server_info, GET /get_model_info.
• GET /health was already fine (send_health_check strips @rank).
• Added test_regular_router_dp2_get_v1_models in tests/test_dp_routing.rs: asserts GET /v1/models through a DP=2 regular router returns 200 with the worker's model list. Verified the test fails on the previous implementation (500 from the userinfo-corrupted URL) and passes with the fix.

Test Plan

• cargo test --test test_dp_routing test_regular_router_dp2_get_v1_models
• cargo test --test test_dp_routing
• cargo test --test test_transparent_proxy_routing --test api_endpoints_test
• cargo test --lib --bins
• cargo fmt --check
• cargo clippy --all-targets --all-features -- -D warnings

Test Result

running 1 test
test dp_e2e_tests::test_regular_router_dp2_get_v1_models ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 16 filtered out; finished in 0.69s

test result: ok. 17 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 1.39s

test result: ok. 11 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

test result: ok. 484 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 75.19s

Before the fix, the regression test fails (worker never receives the request):

thread 'dp_e2e_tests::test_regular_router_dp2_get_v1_models' panicked:
GET /v1/models must succeed when DP > 1 (got 500). @rank in the worker URL was misinterpreted as userinfo.

---

Essential Elements of an Effective PR Description Checklist

• The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
• The test plan, such as providing test command.
• The test results, such as pasting the results comparison before and after, or e2e results

[Copilot]

Pull request overview

Fixes intra-node data-parallel (DP) GET proxying when worker registry URLs include an @rank suffix by ensuring GET requests use the base worker URL (without @rank) and carry the DP rank via header, aligning behavior with the existing chat POST routing.

Changes:

• Update proxy_get_request to parse DP-aware worker URLs and add X-data-parallel-rank when applicable.
• Add an e2e regression test ensuring GET /v1/models succeeds through a DP=2 regular router and returns the worker’s model list.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/routers/http/router.rs	Fixes GET proxy URL construction for DP-aware worker URLs and adds DP rank header.
tests/test_dp_routing.rs	Adds regression coverage for GET /v1/models under DP=2.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[fake0fan]

Thanks for the fix! Parsing the worker URL before constructing the GET request looks correct and addresses #220. Preserving the selected worker’s DP rank in the header also makes sense, even though endpoints such as /v1/models do not consume it. The regression test checks both the status and the worker’s model list, which provides good coverage of the original failure.

I have two suggestions before merging:

1. Verify the forwarded DP rank. Could we extend the mock worker’s /v1/models handler to capture request headers and assert that X-data-parallel-rank matches the selected worker’s rank? Currently, removing add_dp_rank_header() would still leave this test passing.

2. Handle a conflicting client-provided rank header. The router adds its selected rank before copying incoming headers, and that loop does not exclude X-data-parallel-rank. This could introduce duplicate values when the client supplies its own rank. When the router selects a DP rank, could we filter out the incoming value and then inject the selected rank?

It would be useful to cover the second case with a conflicting client header and assert that the worker receives exactly one rank value matching the router’s selection. That assertion should inspect all values, since the existing capture helper stores headers in a HashMap, which cannot preserve duplicates.

[wuhang2014]

Thanks @fake0fan! Both suggestions are addressed in 9480f7e:

1. Verify the forwarded DP rank: the mock worker's /v1/models handler now captures request headers, and test_regular_router_dp2_get_v1_models asserts the request carried X-data-parallel-rank with a value matching one of the deployment's ranks (0 or 1 — both DP ranks share the mock worker, and select_first_worker order over the registry is not deterministic, so the assertion checks membership rather than a fixed rank). Removing add_dp_rank_header() now fails the test.

2. Conflicting client-provided rank header: proxy_get_request now skips the incoming x-data-parallel-rank when the router selected a DP rank, so the worker receives exactly one value (the router's). DP=1 passthrough is unchanged since the router injects nothing in that case. To make the assertion duplicate-proof, CapturedRequest now stores all header values (HashMap<String, Vec<String>>) instead of collapsing them. New test test_regular_router_dp2_get_v1_models_overrides_client_dp_rank sends X-data-parallel-rank: 99 and asserts the worker received exactly one value equal to the router's selection; on the pre-fix code it fails with worker received ["1", "99"].

All gates pass: cargo test --test test_dp_routing (18), otel suites, --lib --bins (484), cargo fmt --check, clippy -D warnings.

[fake0fan]

Thanks for addressing both points! The header filtering and duplicate-preserving capture cover the concerns I raised. The updated tests look sufficient for this fix. LGTM.