feature: provider & model config#8515
Conversation
alexhancock
requested review from
DOsinga,
jamadeo,
jh-block,
lifeizhou-ap and
michaelneale
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9dd5d352a
ℹ️ 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".
| if (!selectedProvider) return; | ||
| setPendingConfigValues(values); | ||
| setPhase("select_model"); | ||
| }, |
There was a problem hiding this comment.
This callback advances to select_model immediately after collecting credentials, but the values are only kept in pendingConfigValues and are not persisted yet. ModelSelector then calls _goose/providers/models, and the server currently rejects providers that are not already configured from stored config (on_get_provider_models checks required keys before fetching models). For providers that start unconfigured, model loading fails even after valid keys are entered, which breaks the configure-new-provider flow.
Useful? React with 👍 / 👎.
| if (error) { | ||
| return ( | ||
| <Box flexDirection="column" justifyContent="center" alignItems="center" width={columns} height={height}> |
There was a problem hiding this comment.
This early error return prevents the manual-entry UI from ever rendering, because manualEntry is checked later in the component. As a result, when model loading fails or times out, pressing m (as instructed by the on-screen hint) cannot open manual model input, so users cannot proceed without backing out.
Useful? React with 👍 / 👎.
alexhancock
force-pushed
the
alexhancock/tui-onboarding
branch
from
a879727 to
a0706f0
Compare
alexhancock
force-pushed
the
alexhancock/provider-and-model-selection
branch
2 times, most recently
from
a920e5b to
db06163
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db06163709
ℹ️ 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".
| await client.goose.GooseConfigUpsert({ key: "GOOSE_PROVIDER", value: provider.name }); | ||
| await client.goose.GooseConfigUpsert({ key: "GOOSE_MODEL", value: model }); | ||
| await client.goose.GooseSessionProviderUpdate({ |
There was a problem hiding this comment.
applyProviderModel persists GOOSE_PROVIDER/GOOSE_MODEL before calling GooseSessionProviderUpdate, so a failed live switch (for example from an unsupported manual model entry or provider initialization error) leaves global config pointing to the new selection while the current session remains on the old provider/model. That mismatch is user-visible on the next session start and is hard to recover from after canceling out of the error state; write defaults only after a successful session update (or roll them back on failure).
Useful? React with 👍 / 👎.
| <Box flexDirection="column" alignItems="center" width={maxWidth}> | ||
| <Text color={GOLD}>⚠ Failed to load models</Text> | ||
| <Box marginTop={1} width={maxWidth}> | ||
| <Text color={TEXT_DIM} wrap="wrap">{error}</Text> |
There was a problem hiding this comment.
In ModelSelector's error state, the message is rendered with wrap="wrap" inside a fixed-height full-screen layout; Ink does not clip overflowing content, so long backend error strings can spill into neighboring rows and break the screen layout. This should be line-capped/truncated (for example wrap="truncate" plus pre-truncation) before rendering.
Useful? React with 👍 / 👎.
alexhancock
force-pushed
the
alexhancock/provider-and-model-selection
branch
from
db06163 to
0265549
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0265549c1f
ℹ️ 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".
| setModels(resp.models); | ||
| const defaultIdx = resp.models.findIndex((m) => m === provider.defaultModel); | ||
| setSelectedIdx(defaultIdx >= 0 ? defaultIdx : 0); | ||
| setLoading(false); | ||
| clearTimeout(timeoutId); |
There was a problem hiding this comment.
When the 30s timeout fires, error is set, but if the provider responds later the success path only updates models/selectedIdx/loading and never clears error. Because render checks if (error) before the normal selector UI, slow-but-successful responses remain stuck on the failure screen instead of recovering. This affects providers that return after the timeout threshold.
Useful? React with 👍 / 👎.
| </Text> | ||
| )} | ||
| <Text color={TEXT_DIM}>^C exit</Text> | ||
| <Text color={TEXT_DIM}>^G configure · ^C exit</Text> |
There was a problem hiding this comment.
The new ^G configure · ^C exit label is longer than the right header column at small widths (the current split leaves only ~15 chars there), so it wraps to a second line in narrow terminals. App still budgets header height as two rows total (headerH = 2), so this wrap causes viewport math to be off and can visually overlap/crop content below the header.
Useful? React with 👍 / 👎.
michaelneale
left a comment
michaelneale
left a comment
There was a problem hiding this comment.
looks ok to me. I guess a new GUI using acp server would want models and ctx limits too, nice one.
alexhancock
force-pushed
the
alexhancock/provider-and-model-selection
branch
from
0265549 to
a96d97f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a96d97f05f
ℹ️ 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".
| const keys = provider.configKeys.filter( | ||
| (k) => k.required && !k.oauthFlow && !k.deviceCodeFlow, | ||
| ); | ||
| setSelectedProvider(provider); | ||
| if (keys.length > 0 && !provider.isConfigured) { |
There was a problem hiding this comment.
This branch filters out oauthFlow/deviceCodeFlow keys when deciding whether to show ProviderConfigurator, so providers whose only required credential is OAuth (e.g. GitHub Copilot, ChatGPT Codex, Gemini OAuth in crates/goose/src/providers/*) are treated as if they need no setup and jump straight to model selection. The subsequent _goose/providers/models call then fails because on_get_provider_models requires required keys to already exist in config, which blocks configuring these providers from the new Ctrl+G flow.
Useful? React with 👍 / 👎.
…rning * origin/main: move agent skills to repo root for tool discoverability (#8535) port goose2 chat attachments into goose (#8534) chore(release): bump version to 1.31.0 (minor) (#8527) feature: provider & model config (#8515) Move goose2 (#8516) feat: onboarding UX for the TUI (#8513) Set MACOSX_DEPLOYMENT_TARGET=12.0 in build-cli.yml (#8525) Improve local inference settings and model downloader (#8467) Add prompt injection mitigation logging back (#8518) feat(providers): add llama-swap declarative provider (#8462) fix: Unable to Run `goose update` on Linux (#8465) Add vision/image support for local inference models (#8442)
2 participants
Summary
Ctrl + Gwill launch the Provider & Model selection grid UI where you can select or configure a provider and a model withinI don't have any attachment to that keyboard shortcut. Any feedback?
Testing
Manual usage
Related Issues
Phase 2 of #6642
Screenshots/Demos (for UX changes)