Skip to content

fix(router): route unlisted models to wildcard external workers under IGW (#1871)#1896

Open
slin1237 wants to merge 1 commit into
mainfrom
fix/igw-external-wildcard-routing
Open

fix(router): route unlisted models to wildcard external workers under IGW (#1871)#1896
slin1237 wants to merge 1 commit into
mainfrom
fix/igw-external-wildcard-routing

Conversation

@slin1237

@slin1237 slin1237 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Description

Problem

In IGW mode, starting with --enable-igw --worker-urls <local-sglang> https://api.openai.com (no key) registers OpenAI as a wildcard external worker, but a gpt-* request returned 404 model_not_found — it was never dispatched to the OpenAI proxy router (discussion #1871).

Root cause: select_router_for_request resolves a request's candidate workers via the registry's per-model index (get_by_model), which only contains each worker's explicitly-listed models. A wildcard external worker has none, so for an unlisted model the lookup is empty → dispatch falls back to the default (local) router instead of the external→provider-router branch in select_router_for_workers.

Solution

When the per-model index is empty, fall back to external workers that support the model, so the request dispatches to the OpenAI/Anthropic/Gemini proxy router — which forwards the caller's bearer (BYOK) and strips SGLang-only fields. Indexed (explicit) workers still win, so a locally-served model never routes to a wildcard.

First of a few focused PRs from discussion #1871. Follow-ups: warn + auto-enable IGW when --worker-urls includes an external provider; stop --backend openai from classifying self-hosted workers as external.

Changes

  • model_gateway/.../routers/router_manager.rs: wildcard-external fallback in select_router_for_request, plus a unit test asserting dispatch by router identity (Arc::ptr_eq).
  • e2e_test/infra/gateway.py: allow --enable-igw together with --worker-urls.
  • e2e_test/router/test_worker_api.py: gated e2e for the mixed --enable-igw --worker-urls path.

Test Plan

Unit:

cargo test -p smg --lib -- routers::router_manager::tests::igw_dispatches_wildcard_external_model_to_provider_router
  • gpt-4o-mini (served only by the wildcard external worker) → OpenAI router; a locally-served model → regular router (never the wildcard).

e2eTestIGWWorkerUrlsMixedExternal (gated on OPENAI_API_KEY, @gpu(1) @engine(sglang)): starts --enable-igw --worker-urls <local-sglang> https://api.openai.com, asserts the OpenAI URL registers as external, and a gpt-4o-mini chat carrying the caller's Authorization: Bearer (BYOK) returns 200.

Checklist
  • cargo +nightly fmt passes
  • cargo clippy -- -D warnings clean on the changed crate
  • (Optional) Documentation updated
  • (Optional) Please join us on Slack #sig-smg to discuss, review, and merge PRs

Summary by CodeRabbit

  • Bug Fixes

    • Improved IGW startup when worker URLs are provided: IGW takes precedence, the gateway waits for the expected number of workers, and startup logs reflect the worker count.
    • Updated IGW model dispatch to correctly route models from unindexed “wildcard” external workers, while preserving locally indexed routing.
  • Tests

    • Added an end-to-end regression test for IGW setups mixing a local HTTP worker with an external OpenAI worker URL.
    • Added a unit test ensuring wildcard external models route to the provider (OpenAI) path.

@github-actions github-actions Bot added tests Test changes model-gateway Model gateway crate changes labels Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Router manager adds a fallback in select_router_for_request for models only covered by wildcard external workers. IGW gateway startup now handles mixed worker_urls with IGW mode, and a new e2e test verifies OpenAI routing in that mixed setup.

Changes

IGW Wildcard External Model Routing

Layer / File(s) Summary
Router manager fallback dispatch and unit test
model_gateway/src/routers/router_manager.rs
select_router_for_request falls back to RuntimeType::External workers that supports_model(model) when the per-model lookup is empty, with a unit test covering wildcard external vs local routing.
Gateway test infra mode handling and worker-urls startup
e2e_test/infra/gateway.py
Gateway.start() treats IGW mode as taking precedence over regular mode, and IGW startup now passes --worker-urls plus num_workers when worker URLs are provided.
Mixed local/external worker_urls e2e regression test
e2e_test/router/test_worker_api.py
A new IGW regression test starts a local worker alongside https://api.openai.com, checks the worker registry, and confirms chat completions succeed through OpenAI with a bearer token.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Test
  participant Gateway
  participant RouterManager
  participant WorkerRegistry
  participant OpenAIProxyRouter

  Test->>Gateway: start(worker_urls, igw_mode=True)
  Gateway->>RouterManager: initialize IGW routing
  Test->>RouterManager: select_router_for_request(model)
  RouterManager->>WorkerRegistry: get_by_model(model)
  WorkerRegistry-->>RouterManager: no indexed workers
  RouterManager->>WorkerRegistry: scan External workers supporting_model(model)
  WorkerRegistry-->>RouterManager: wildcard external worker
  RouterManager->>OpenAIProxyRouter: route request
Loading

Possibly related PRs

  • lightseekorg/smg#643: Both PRs modify Gateway.start() in e2e_test/infra/gateway.py to change how gateway mode selection/launch arguments are handled.
  • lightseekorg/smg#756: Both PRs affect how external workers are indexed or selected for model dispatch in the IGW path.
  • lightseekorg/smg#839: Both PRs involve IGW mixed local/external worker handling and external runtime classification.

Suggested labels: openai

Suggested reviewers: CatherineSue, key4ng, XinyueZhang369

Poem

A rabbit hopped through router trails,
To seek the model in the veils.
With wildcard ears and OpenAI light,
The request found home and took its flight.
🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: IGW routing now sends unlisted models to wildcard external workers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/igw-external-wildcard-routing

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request enables support for mixed self-hosted and external workers under the Inference Gateway (IGW), resolving issue #1871. When a requested model is not explicitly indexed, the gateway now falls back to wildcard external workers that support the model instead of defaulting to the local router. The changes include updates to the gateway test infrastructure, a new integration test, and a unit test in Rust. The review feedback suggests optimizing the worker lookup in RouterManager by using get_workers_filtered instead of get_all() to avoid cloning all registered workers on the hot path.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +334 to +341
self.worker_registry
.get_all()
.into_iter()
.filter(|w| {
matches!(w.metadata().spec.runtime_type, RuntimeType::External)
&& w.supports_model(model)
})
.collect()

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.

medium

Using get_all() on the worker registry clones the Arc<dyn Worker> for every single registered worker, which can be quite expensive on the hot path of request routing. Since we only care about external workers here, we can use get_workers_filtered to only retrieve and clone the external workers, significantly improving efficiency.

                self.worker_registry
                    .get_workers_filtered(None, None, None, Some(RuntimeType::External), false)
                    .into_iter()
                    .filter(|w| w.supports_model(model))
                    .collect()

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21bb672a3e

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +337 to +340
.filter(|w| {
matches!(w.metadata().spec.runtime_type, RuntimeType::External)
&& w.supports_model(model)
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Infer provider before wildcard external fallback

In an IGW BYOK setup with more than one keyless external provider (for example --worker-urls https://api.openai.com https://api.anthropic.com), this fallback collects every wildcard external worker because supports_model() is true for all of them. Those wildcard workers have no default provider until discovery, and the downstream provider filter treats all-None providers as a single-provider set, so a gpt-* request can be selected onto the Anthropic worker and forward the caller's OpenAI bearer there. Please preserve/infer the provider from the worker URL or filter the fallback candidates by inferred provider/model before returning them.

Useful? React with 👍 / 👎.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped fix. The fallback from per-model index to wildcard external workers is correct — indexed workers always win, wildcard externals only activate when the model is genuinely unlisted. Unit and e2e coverage look solid.

… IGW

In IGW mode, `select_router_for_request` resolves a request's candidate workers
via the registry's per-model index (`get_by_model`), which only holds workers'
explicitly-listed models. A wildcard external worker (e.g.
`--worker-urls https://api.openai.com` with no key) isn't indexed, so an
unlisted model like `gpt-4o-mini` matched no worker and fell back to the default
local router — returning `model_not_found` instead of dispatching to the OpenAI
proxy router (discussion #1871).

When the index lookup is empty, fall back to external workers that support the
model, so the request dispatches to the provider proxy router (which forwards
the caller's bearer / BYOK and strips SGLang-only fields). Indexed (explicit)
workers still win, so a local model never leaks to a wildcard.

Also lets the e2e harness start `--enable-igw` together with `--worker-urls`.

Refs #1871

Signed-off-by: Simo Lin <linsimo.mark@gmail.com>
@slin1237 slin1237 force-pushed the fix/igw-external-wildcard-routing branch from 21bb672 to f3b6068 Compare July 9, 2026 07:06

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f3b6068e08

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +338 to +339
matches!(w.metadata().spec.runtime_type, RuntimeType::External)
&& w.supports_model(model)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard wildcard fallback from local startup misses

When IGW is started with both local workers and a keyless external wildcard, startup only submits InitializeWorkersFromConfig in the background (server.rs) and the job queue registers workers asynchronously, so the external worker can be present before the local model is indexed. In that window, a request for the local model sees an empty model index and this predicate matches the wildcard because supports_model() is true for any model, sending the request through the external proxy with the caller's auth instead of treating the local worker as not ready. Please restrict the fallback by inferred provider/model family or otherwise avoid using wildcard externals for local-looking models while configured local workers are still registering.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
e2e_test/infra/gateway.py (1)

45-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Docstring doesn't mention the new mixed IGW + worker_urls startup path.

Worth a line noting start(igw_mode=True, worker_urls=[...]) is now supported for registering self-hosted + external workers at launch, since that's the mode this PR adds and tests.

📝 Suggested docstring update
     Four startup modes:
     - Regular: start(worker_urls=[...], model_path="...")
     - PD: start(prefill_workers=[...], decode_workers=[...])
-    - IGW: start(igw_mode=True), then add_worker(url) dynamically
+    - IGW: start(igw_mode=True), then add_worker(url) dynamically, or
+      start(igw_mode=True, worker_urls=[...]) to register workers at launch
     - Cloud: start(cloud_backend="openai"|"xai"|"anthropic")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e_test/infra/gateway.py` around lines 45 - 49, Update the startup mode
docstring in the gateway helper to document the new mixed IGW launch path. In
the `start(...)` docstring, add a line for `start(igw_mode=True,
worker_urls=[...])` to show that self-hosted and external workers can be
registered at launch, alongside the existing `start`, PD, IGW, and Cloud
examples.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@model_gateway/src/routers/router_manager.rs`:
- Around line 326-341: The fallback in `RouterManager::get_target_workers` (the
`indexed.is_empty()` branch) is doing a full `worker_registry.get_all()` scan
and clone on every unindexed-model request, which is too expensive on the hot
path. Replace this linear search with a small secondary cache/index for wildcard
`RuntimeType::External` workers, and update it when workers are registered or
deregistered so the request path can look up matching external workers without
scanning the full registry.

---

Outside diff comments:
In `@e2e_test/infra/gateway.py`:
- Around line 45-49: Update the startup mode docstring in the gateway helper to
document the new mixed IGW launch path. In the `start(...)` docstring, add a
line for `start(igw_mode=True, worker_urls=[...])` to show that self-hosted and
external workers can be registered at launch, alongside the existing `start`,
PD, IGW, and Cloud examples.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f04790be-938d-4292-9f87-2f4ebde8b1e4

📥 Commits

Reviewing files that changed from the base of the PR and between 21bb672 and f3b6068.

📒 Files selected for processing (3)
  • e2e_test/infra/gateway.py
  • e2e_test/router/test_worker_api.py
  • model_gateway/src/routers/router_manager.rs

Comment on lines +326 to +341
let indexed = self.worker_registry.get_by_model(model).to_vec();
if indexed.is_empty() {
// Wildcard external workers (e.g. `--worker-urls https://api.openai.com`
// with no key) accept any model but aren't in the per-model index, so
// `get_by_model` misses them. Fall back to external workers that support
// the model, so the request dispatches to the provider proxy router
// instead of the default local router. Indexed (explicit) workers win,
// so a local model never falls through to a wildcard. (#1871)
self.worker_registry
.get_all()
.into_iter()
.filter(|w| {
matches!(w.metadata().spec.runtime_type, RuntimeType::External)
&& w.supports_model(model)
})
.collect()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

Full-registry scan on every unindexed-model request.

When get_by_model misses, this scans and clones every worker in the registry (get_all()) on the hot request path for each unlisted model — this happens for every gpt-*-style call, not just at startup. At small worker counts this is fine, but it doesn't scale with registry size. Consider maintaining a small secondary index/cache of wildcard RuntimeType::External workers (updated on register/deregister) instead of a linear scan per request.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model_gateway/src/routers/router_manager.rs` around lines 326 - 341, The
fallback in `RouterManager::get_target_workers` (the `indexed.is_empty()`
branch) is doing a full `worker_registry.get_all()` scan and clone on every
unindexed-model request, which is too expensive on the hot path. Replace this
linear search with a small secondary cache/index for wildcard
`RuntimeType::External` workers, and update it when workers are registered or
deregistered so the request path can look up matching external workers without
scanning the full registry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

model-gateway Model gateway crate changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant