feat(cli): add kimi provider subcommand for non-interactive provider management#313
Conversation
Add a non-interactive equivalent of the TUI `/provider` command: - `kimi provider add <url> --api-key <key>` imports every provider in a custom api.json registry, persisting `source` so the next TUI launch refreshes the model list automatically. - `kimi provider remove <id>` deletes a provider and its model aliases. - `kimi provider list [--json]` prints configured providers with model counts and source labels. - `kimi provider catalog list [providerId] [--filter] [--url] [--json]` browses the public models.dev catalog. - `kimi provider catalog add <providerId> --api-key <key> [--default-model]` imports a known provider straight from the catalog. All actions reuse `fetchCustomRegistry`, `applyCustomRegistryProvider`, `fetchCatalog`, and `applyCatalogProvider` from the existing oauth/SDK helpers.
🦋 Changeset detectedLatest commit: dfa496a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
- Use `Array#toSorted()` instead of `Array#sort()` to avoid mutating
arrays returned from `Object.keys()` / `Object.entries()`.
- Drop redundant boolean-literal comparisons on `model.capability.*`
fields (already typed as `boolean | undefined`).
- Remove the unnecessary `source as Record<string, unknown>` assertion
in `providerSourceLabel` — `ProviderConfig.source` is already typed
that way in the schema.
- Drop the empty-object fallback in `{ ...(config.models ?? {}) }`
inside the test harness.
![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: eace0a6b44
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const entry of entryList) { | ||
| if (config.providers[entry.id] !== undefined) { | ||
| config = await harness.removeProvider(entry.id); | ||
| } | ||
| applyCustomRegistryProvider(asManaged(config), entry, source); |
There was a problem hiding this comment.
When the registry contains more than one provider and any later entry already exists locally, this config = await harness.removeProvider(entry.id) reloads the config from disk, discarding providers that were already applied in memory earlier in the loop but not yet persisted. For example, importing a registry with new provider A followed by existing provider B will finish with only B in the final setConfig patch, so A is silently lost. Remove stale aliases without replacing the accumulated in-memory config, or persist/reload after each application.
Useful? React with 👍 / 👎.
| apiKey, | ||
| models, | ||
| selectedModelId: opts.defaultModel ?? '', | ||
| thinking: false, |
There was a problem hiding this comment.
When users pass --default-model, this hard-codes default_thinking = false for the imported model. In the runtime resolver, defaultThinking === false forces thinking off, so kimi provider catalog add anthropic --default-model <thinking-capable-model> silently disables thinking for future sessions even when the existing config would otherwise use the normal/global thinking default. Preserve the previous default, infer from the selected model, or expose a CLI option instead of always writing false.
Useful? React with 👍 / 👎.
| const previousDefaultModel = config.defaultModel; | ||
| const previousDefaultThinking = config.defaultThinking; |
There was a problem hiding this comment.
When re-importing an already-configured catalog provider without --default-model, these values are captured after removeProvider has already cleared defaultModel if it pointed at one of that provider's aliases. So a user who runs kimi provider catalog add anthropic --api-key ... to refresh or rotate the key loses an existing default_model = "anthropic/...", despite the command/comment/docs promising to preserve it when no new default is requested. Save the previous default before removal and restore it after the aliases are repopulated.
Useful? React with 👍 / 👎.
P1 — `provider add`: `harness.removeProvider` re-reads the config from disk (see `agent-core/src/rpc/core-impl.ts removeKimiProvider`), so calling it mid-loop discarded providers we had already applied in memory but not yet persisted. Importing a registry that added a new provider then replaced an existing one silently lost the new one. Drop every stale id up front in a single batch, then apply each entry against the resulting fresh config. P2 — `catalog add`: `applyCatalogProvider` always writes `defaultThinking`. Hardcoding `false` would silently disable thinking for thinking-capable models when the user had it on. Thread the prior `defaultThinking` through. P2 — `catalog add`: `removeProvider` clears `defaultModel` when it pointed at one of the provider's aliases, so capturing `previousDefaultModel` AFTER the removal yielded `undefined`. Capture both `defaultModel` and `defaultThinking` BEFORE the removal so re-importing a configured provider (e.g. to rotate the api key) preserves the user's chosen default. Tests: - `makeHarness` now models the on-disk semantics of `removeProvider` (clears `defaultModel` when an alias matches, returns fresh disk view), so behavior that depended on the buggy in-memory mock is exercised honestly. - Three new regression tests, each verified to fail against the pre-fix handler.
|
Thanks for the careful review — all three findings reproduced and have been fixed in P1 — accumulated providers lost across removals ( You're right: Fix: drop every stale id up front in a single batch before the apply loop, then apply each entry against the resulting fresh config without further removals. let config = await harness.getConfig();
const staleIds = entryList
.filter((entry) => config.providers[entry.id] !== undefined)
.map((entry) => entry.id);
for (const id of staleIds) {
config = await harness.removeProvider(id);
}
for (const entry of entryList) {
applyCustomRegistryProvider(asManaged(config), entry, source);
...
}P2 — Correct. P2 — Also correct. Test harness fidelity The original
Three new regression tests cover all three bugs, and I verified each one fails against the pre-fix handler. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f97ff2b6fe
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| apiKey, | ||
| models, | ||
| selectedModelId: opts.defaultModel ?? '', | ||
| thinking: previousDefaultThinking ?? false, |
There was a problem hiding this comment.
When --default-model is supplied and the config has no existing default_thinking, this falls back to false, and resolveThinkingLevel treats defaultThinking === false as an explicit off request before considering the normal thinking defaults. The fresh evidence is that the previous hard-coded value is now a nullish fallback, so a first-time kimi provider catalog add anthropic --default-model claude-opus-4-7 still persists default_thinking = false and disables thinking for future sessions even for thinking-capable models.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| if (opts.defaultModel === undefined) { | ||
| config.defaultModel = previousDefaultModel; |
There was a problem hiding this comment.
When re-importing a catalog provider without --default-model, this restores the old defaultModel even if the refreshed catalog no longer includes that model. In that case removeProvider has already deleted the old alias and applyCatalogProvider only repopulates the current catalog models, so future sessions using the preserved default_model fail with “Model ... is not configured”; restore it only if the alias exists after the import, otherwise clear it or choose a valid replacement.
Useful? React with 👍 / 👎.
Two more findings on `catalog add`:
P2 — `default_thinking` fallback to `false` was wrong even after the
previous fix. `resolveThinkingLevel` (agent-core/.../thinking.ts:23)
treats `defaultThinking === false` as an explicit "off" request and
silently disables thinking before per-model defaults kick in. A
first-time `kimi provider catalog add anthropic --default-model
claude-opus-4-7` was therefore still persisting `default_thinking =
false` for thinking-capable models. The handler now always restores
the previous `defaultThinking` (including `undefined`) — the only
way to let the runtime resolver pick the per-model default.
P2 — Restoring `default_model` was unconditional, even when the
refreshed catalog no longer ships that model. `applyCatalogProvider`
drops the old aliases and only populates the current catalog, so
restoring an alias the catalog no longer contains would point
`default_model` at a non-existent entry and break the next session.
The handler now checks whether the alias still resolves and clears
it otherwise.
Test harness:
- The fake `setConfig` now mirrors the real `mergeConfigPatch`
semantics (deep-merge with `undefined` keys skipped), so tests can
honestly assert that `setConfig({defaultModel: undefined})` does
NOT wipe a key from disk — only `removeProvider` can.
Two new regression tests, each verified to fail against the pre-fix
handler.
|
Thanks — both follow-up P2s landed. Fix in P2 — You're right; the previous Fix: always restore the previous // Always restore — including `undefined`. Anything else would override
// the resolver's per-model default with an explicit on/off request.
config.defaultThinking = previousDefaultThinking;To make this honestly testable, the fake P2 — Only restore catalog defaults that still resolve Also correct. Fix: gate the restore on whether the alias still resolves after the import. const stillResolves =
previousDefaultModel !== undefined &&
config.models?.[previousDefaultModel] !== undefined;
config.defaultModel = stillResolves ? previousDefaultModel : undefined;Cross-provider defaults are also correctly handled: an unrelated Two new regression tests, both verified to fail against the pre-fix handler. |
1 participant
Related Issue
No tracking issue — opened for discussion alongside this PR.
Problem
Today, importing providers into Kimi Code requires launching the TUI and walking through the
/providerdialog: an entry-source picker, a URL/key form for custom registries, and a model selector. That is great for first-time setup, but rules out a few practical workflows:~/.kimi-code/config.tomlto run an LLM-backed task non-interactively.The same applies to discovering providers from the public models.dev catalog — currently only reachable from
/provider→ "Known third-party provider".What changed
Adds a new
kimi providersubcommand group that mirrors the TUI flow non-interactively. All actions reuse existing helpers (fetchCustomRegistry,applyCustomRegistryProvider,fetchCatalog,applyCatalogProvider,KimiHarness.{getConfig,setConfig,removeProvider}); no new business logic is introduced in the SDK / oauth packages.Five actions, all designed for shell use:
kimi provider add <url> --api-key <key>api.jsonregistry. Persists the samesource = { kind = "apiJson", url, api_key }block the TUI writes, sorefreshAllProviderModelskeeps the model list current on next launch. Falls back toKIMI_REGISTRY_API_KEY.kimi provider remove <providerId>kimi provider list [--json]apiJson(...) / oauth / inline);--jsonfor piping.kimi provider catalog list [providerId] [--filter] [--url] [--json]providerId, drills into models with context and capabilities.kimi provider catalog add <providerId> --api-key <key> [--default-model]--default-modelis validated against the catalog's model list before writing.Design notes:
apps/kimi-code/src/cli/sub/export.tspattern (registerXxxCommand(parent, deps?)with dependency injection for tests).catalog addpreserves the existingdefault_modelunless--default-modelis explicitly supplied —applyCatalogProvideralways assigns one, so the handler restores the previous value when none is requested.catalog addvalidates--default-modelagainst the catalog before writing, so a typo can't produce a stale config.Docs are updated to cover both the user-facing reference (
docs/{en,zh}/reference/kimi-command.md) and a cross-link from the providers config page (docs/{en,zh}/configuration/providers.md). The changeset isminor(@moonshot-ai/kimi-code) pergen-changesets/SKILL.md— new backwards-compatible feature, no schema or behavior changes.Tests: 23 new unit tests in
apps/kimi-code/test/cli/provider.test.tscover happy paths, stale-provider replacement, env-var fallback, missing key, HTTP error surfacing, catalog filtering, drill-down rendering,--default-modelvalidation, default-model preservation, and the commander wiring. The existingoptions.test.tsregistered-subcommands assertion is updated to includeprovider.Verified manually against
https://free-tokens.msh.team/v1/models/api.json(3 providers, 14 models imported into~/.kimi-code/config.toml) andhttps://models.dev/api.json(catalog list anthropicreturns 24 models with correct capabilities).Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.