Repository navigation
feat: narrow models to what the providers/set gateway lists - #627
EugeneTheDev merged 1 commit into
Conversation
| await this.enqueueProviderUpdate((client) => client.setProvider(params)); | ||
| await this.codexAcpClient.setProvider(params); | ||
| // Copies the routing with its model listing, so the restarted app-server does not list the models again. | ||
| await this.enqueueProviderUpdate((client) => client.adoptRoutingFrom(this.codexAcpClient)); |
There was a problem hiding this comment.
High: a providers/set can be lost during a provider restart. setProvider awaits this.codexAcpClient.setProvider(...), and that call now awaits the network listing. The receiver is bound before the await. If a restart queued by an earlier providers/set swaps this.codexAcpClient while this listing is in flight, the new routing is written to the old client, which has already been replaced. This queued update then runs adoptRoutingFrom(this.codexAcpClient) and copies the previous routing from the current client.
I reproduced it with a temporary test: set A (one session, replacement initialize held), then set B with a deferred fetch, release the restart, then resolve B. Both calls succeed, but the final client reports baseUrl A. Before this PR, the queued update re-applied params on the replacement, so B won.
Suggestion: take the listing first (it does not depend on the client), then apply the config synchronously to whatever client is current, and/or keep passing the built GatewayConfig into the queued update instead of copying from this.codexAcpClient.
| try { | ||
| gatewayConfig.listedModels = previous?.listedModels && sameGatewayRouting(previous, gatewayConfig) | ||
| ? previous.listedModels | ||
| : await listGatewayModels(gatewayConfig.config.base_url, gatewayConfig.config.http_headers); |
There was a problem hiding this comment.
Medium: a providers/disable (or a newer providers/set) sent while this listing is in flight gets overwritten. previous is read before the await, and gatewayConfig is assigned unconditionally after it. So set(A) → disable → listing resolves leaves A active, even though the client's last request was the disable and both requests succeeded. I confirmed this with a temporary test: after set then disable, listProviders() still reports A's baseUrl. Two overlapping sets with different configs behave the same way: the one whose listing finishes last wins, not the one sent last.
A generation counter (or serializing the providers/* calls) would let a stale listing drop its result instead of applying it.
| // The gateway narrowed the models, so a model it does not list must not be used. When it lists none, | ||
| // no model is available and the thread keeps its own; the gateway then refuses the requests. | ||
| const narrowedDefault = availableModels.find(m => m.isDefault) ?? availableModels[0]; | ||
| if (modelId && narrowedDefault && this.isNarrowedByGateway()) { |
There was a problem hiding this comment.
This replaces any thread model that is not in the narrowed Codex catalogue, even when the gateway itself lists it. Say config.toml sets model = "gpt-6.1-sol", the gateway lists gpt-6.1-sol, and Codex's bundled catalogue doesn't have it yet. The session silently switches to the narrowed default. That undoes the custom-provider behaviour described in the comment just below (keep an uncatalogued configured model). Should this branch run only when the gateway does not list modelId either? For example, check listed.has(normalizeModelId(modelId)), which would need a small helper exported from GatewayModelDiscovery.
| if (!availableModels.some(model => model.id === currentBaseModelId)) { | ||
| // An uncatalogued current model is offered beside the catalogue. With no model at all (a gateway that | ||
| // lists none of them), nothing is offered. | ||
| if (availableModels.length > 0 && !availableModels.some(model => model.id === currentBaseModelId)) { |
There was a problem hiding this comment.
Low: when the gateway lists no Codex models, the model select option is returned with options: [] but currentValue is still the thread model (gpt-5.5 in the test). A select whose currentValue is not among its options is inconsistent, and clients may reject it or render it oddly. Would it be better to omit the model option entirely in this case?
a0cecc8 to
0665800
Compare
An organization can restrict the models its JetBrains gateway serves, but
the picker offered Codex's whole catalogue. When providers/set routes Codex
through a gateway, the adapter now lists the gateway's models before it
responds (GET {baseUrl}/models with the headers Codex sends) and reports
only the Codex models the gateway lists:
- ids match without case and date suffix; exactly one narrowed model stays
the default
- a thread model the gateway does not list is replaced by the narrowed
default, also on turn/start; one it lists but Codex does not know is kept
- sessions open across providers/set or providers/disable take the models
of the restarted app-server, also when they are resumed later by recovery,
and the client gets a config_option_update
- a listing that finishes after a newer providers/set or providers/disable
was applied is dropped (a newer request that failed drops nothing), and
the routing goes to the client a restart installed
- a 4xx answer means the gateway has no listing: providers/set succeeds and
nothing is narrowed; any other failed listing fails providers/set with
internal_error and keeps the current routing; a base URL that is not
http(s) fails with invalid_params
- a listing that matches no model leaves the session none, and there is no
model config option
- the gateway auth method and providers from the Codex config are not
listed; the same configuration set again, and a provider restart, reuse
the listing
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0665800 to
cd62740
Compare
Summary
Addresses IJAI-1783.
An organization can restrict the models its JetBrains gateway serves, but a session routed through
providers/setoffered Codex's whole catalogue, so the picker showed models the gateway then refused.providers/setnow lists the gateway's models before it responds, and the adapter reports only the Codex models the gateway lists.GET {baseUrl}/modelswith the headers Codex sends to the gateway (X-Client-Feature-ID: codexplus theproviders/setheaders); OpenAI format,data[].id. The listing is stored with the gateway config, so a provider restart reuses it, and setting the same configuration again does not list again.fetchAvailableModelsnarrowsmodel/list, so new, resumed, loaded and forked sessions andunstable_setSessionModelall see the same list. Ids match without case and date suffix; exactly one narrowed model staysisDefault.turn/start; a model the gateway lists but Codex's catalogue lacks is kept. Sessions open acrossproviders/setorproviders/disabletake the models of the restarted app-server, and the client gets aconfig_option_update. A listing that matches no Codex model leaves the session with no model: there is no model config option at all.providers/setsucceeds and nothing is narrowed. Any other failed listing (network error, 5xx, malformed body, 10 s timeout) failsproviders/setwithinternal_error, and the current routing stays. A listing that finishes after a newerproviders/setorproviders/disablewas applied is dropped (a newer request that failed drops nothing), and the routing goes to the client a provider restart installed. AbaseUrlthat is not an http(s) URL, accepted on v1 before, now fails withinvalid_params; an invalid header or a base URL with credentials fails without repeating the value.gatewayauth method and providers from the Codex config (config.toml,CODEX_CONFIG) are not listed.readme-dev.mddescribes the behaviour.Not covered
Title generation (
gpt-5.6-luna), subagents,/reviewand compaction choose their own model and are not checked against the listing.Verification
gateway-model-discovery.test.tscovers the request and headers, narrowing and the default, the replaced thread model onturn/start, failingproviders/set(HTTP error, malformed body, network error) with the routing kept, the empty list, no listing withoutproviders/setor for the gateway auth method,providers/disable, reuse, and the provider restart. Tests that callproviders/setstub the listing (stubGatewayModelListing). Checked locally against the JetBrains gateway with an earlier build of this branch: the adapter's log shows/modelsansweringgpt-5.6-sol,gpt-6-sol,gpt-6.1-solin one session and an empty list with only Bedrock configured; the empty-list and failing-listing behaviour of the final build is covered by the tests only.🤖 Generated with Claude Code