feat(models): discover the harness's own model list instead of hardcoding it - #213
Open
davidliuk wants to merge 1 commit into
Open
feat(models): discover the harness's own model list instead of hardcoding it#213davidliuk wants to merge 1 commit into
davidliuk wants to merge 1 commit into
Conversation
…ding it The model lists in shared/modelConstants.js are hand-maintained, so every time a CLI ships a new model somebody has to notice and send a patch — and until they do the picker offers models that no longer exist while hiding the ones that do. That drift is already real. codex-cli 0.145.0 serves gpt-5.6-sol, gpt-5.4-mini and gpt-5.3-codex-spark; the pinned list still offered gpt-5.6, o3 and o4-mini. OpenRouter's live catalogue is 342 models including claude-opus-5, against ~60 pinned. This asks the tool instead: - Codex via `codex app-server` line-delimited JSON-RPC (initialize, then model/list with nextCursor pagination) — the same catalogue the Codex CLI's own picker reads. - OpenRouter via its public /api/v1/models endpoint. - Everything else keeps its built-in list; those CLIs expose no equivalent. Discovery is strictly additive and can never make things worse: - Any failure — CLI missing, old, logged out, unresponsive — falls back to the built-in list. The endpoint never rejects, so the picker always renders. - 15s hard timeout per probe, with SIGTERM escalating to SIGKILL so a CLI that traps signals cannot outlive the probe holding its pipes. - Results cached 10 minutes; failures re-tried after 1 minute so the picker recovers on its own once a CLI is installed or logged in. - Concurrent callers collapse onto one probe. Probes carry a generation stamp so a slow superseded probe cannot land last and undo a refresh. - Models in the built-in list that the harness no longer serves stay in the picker, marked deprecated, so a saved preference is never stranded — but if the *selected* model is one of them, the client moves to the harness default rather than submitting a model the harness will reject. New: GET /api/models/:provider (?refresh=1), POST /api/models/:provider/refresh, GET /api/models/providers. The /model slash command now uses the same source. Verified against the real codex CLI end to end: 824ms cold, 3ms cached, correct live list and default. 26 tests drive the real code against a fake harness covering pagination, endless cursors, JSON-RPC errors, non-JSON banner output, silent hangs, multi-byte UTF-8 split across stdout chunks, cache/TTL behaviour, concurrency collapse, and stale-probe ordering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds server-side model discovery so the UI and /model command can prefer each harness’s live model catalogue (Codex via JSON-RPC; OpenRouter via HTTP) instead of relying solely on compiled-in lists, while keeping safe fallbacks, caching, and timeouts.
Changes:
- Introduces a discovery subsystem (
harnessModelDiscovery) with caching, timeouts, pagination bounding, and fallback-to-static behavior. - Adds protected
/api/models/*endpoints and updates the/modelslash command to read from the same source. - Updates the chat model picker to consume discovered model options via a new
useHarnessModelshook, and documents the behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/chat/view/subcomponents/ChatComposer.tsx | Uses useHarnessModels to prefer discovered options and rescues retired stored selections to a harness default. |
| src/components/chat/hooks/useHarnessModels.ts | New client hook to fetch /api/models/:provider with caching/refresh behavior. |
| server/utils/harnessModelDiscovery.js | New discovery implementation for Codex/OpenRouter with merge + caching + concurrency collapse. |
| server/utils/tests/harnessModelDiscovery.test.js | Unit/integration-style tests for discovery transport, pagination, timeouts, cache semantics, and edge cases. |
| server/routes/models.js | New Express routes for model discovery, refresh, and provider listing. |
| server/routes/commands.js | Updates /model command to use getModelsForProvider instead of constants. |
| server/index.js | Mounts the new /api/models routes under auth. |
| server/tests/models-route.test.mjs | HTTP-level tests for /api/models/* route behavior. |
| docs/configuration.md | Adds a “Model Discovery” section describing sources, caching/timeouts, and API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for (const option of discovered) { | ||
| if (!option?.value || seen.has(option.value)) continue; | ||
| seen.add(option.value); | ||
| merged.push({ value: option.value, label: option.label || option.value }); |
Comment on lines
+196
to
+200
| const [claude, cursor, codex] = await Promise.all([ | ||
| getModelsForProvider('claude'), | ||
| getModelsForProvider('cursor'), | ||
| getModelsForProvider('codex'), | ||
| ]); |
Comment on lines
+37
to
+61
| // Guards against a slow response for a provider the user has already switched | ||
| // away from overwriting the current one. | ||
| const requestedProviderRef = useRef<string | null>(null); | ||
|
|
||
| const refresh = useCallback(() => setRefreshToken((n) => n + 1), []); | ||
|
|
||
| useEffect(() => { | ||
| if (!provider) { | ||
| setOptions(null); | ||
| setSource(null); | ||
| setDefaultModel(null); | ||
| return; | ||
| } | ||
|
|
||
| let cancelled = false; | ||
| requestedProviderRef.current = provider; | ||
| setIsLoading(true); | ||
| // Clear immediately rather than on response: otherwise the picker keeps | ||
| // rendering the previous provider's models for the duration of the request. | ||
| setOptions(null); | ||
| setSource(null); | ||
| setDefaultModel(null); | ||
|
|
||
| const query = refreshToken > 0 ? '?refresh=1' : ''; | ||
| authenticatedFetch(`/api/models/${encodeURIComponent(provider)}${query}`) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The drift is already real
gpt-5.6,gpt-5.3-codex,gpt-5.2-codex,gpt-5.2,gpt-5.1-codex-max,o3,o4-minigpt-5.6-sol,gpt-5.6-terra,gpt-5.6-luna,gpt-5.5,gpt-5.4,gpt-5.4-mini,gpt-5.3-codex-sparkanthropic/claude-opus-5Note
gpt-5.6vsgpt-5.6-sol— the pinned default is a model the CLI no longer serves.How it discovers
codex app-serverline-delimited JSON-RPC →initialize, thenmodel/listwithnextCursorpagination. This is the same catalogue the Codex CLI's own picker reads. HonoursCODEX_CLI_PATH.GET https://openrouter.ai/api/v1/models(public, no key)/api/tags, unchangedAdding a harness later means adding one function to the
DISCOVERERSmap.It can never make things worse
This repo just spent a PR fixing hangs, so the failure modes got the most attention:
GET /api/models/:providernever rejects, so the picker always renders.SIGTERMescalates toSIGKILLafter 2s so a CLI that traps signals can't outlive the probe holding its pipes open. The kill timer isunref'd.API
GET /api/models/:providersourceisdiscoveredorstatic.?refresh=1bypasses the cache.POST /api/models/:provider/refreshGET /api/models/providersThe
/modelslash command now reads from the same source, so the two lists can't disagree.Testing
Against the real Codex CLI, end to end through the Express route:
26 tests drive the real discovery code against a fake harness that speaks the same JSON-RPC over stdio — no CLI install required in CI. Covered: multi-page pagination, endless-cursor bounding, JSON-RPC errors, non-JSON banner output, silent hangs, hidden-model filtering, multi-byte UTF-8 split across stdout chunks, TTL behaviour, concurrency collapse, forced refresh vs. in-flight stale probe, and unknown providers.
Full suite 133 passed;
npm run typecheckandnpm run buildclean.Review: Codex reviewed this independently and reported 7 issues. Six were real and are fixed here with tests: the discovered default being ignored by the UI,
SIGTERMnever escalating, forced refresh joining a stale in-flight probe, pagination stopping after two pages, the hook keeping the previous provider's options during a switch, and UTF-8 corruption from per-chunktoString(). The seventh — a claim that discovery could never work without aninitializednotification — is contradicted by the live run above, which succeeded without it; the notification is sent anyway, since the documented lifecycle expects it and other versions may enforce it.Docs
docs/configuration.mdgains a Model Discovery section covering sources, caching, timeouts, and the API.