feat(llm): add Kilo Gateway provider support#189
Closed
skulldogged wants to merge 7 commits into
Closed
Conversation
![tembo[bot]](https://avatars.githubusercontent.com/in/1223475?s=60&v=4){kind=small}
Contributor
WalkthroughAdds Kilo Gateway support across docs, configuration, runtime, routing, and UI: new ApiType variant and kilo_key wiring, provider detection/status propagation, OpenAI-compatible call path extended with extra_headers and Kilo-specific headers, UI icon/labels, and routing defaults for "kilo". Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 (2)
interface/src/components/ModelSelect.tsx (1)
149-152:⚠️ Potential issue | 🟡 Minor
?? 99fallback is dead code — unknown providers sort to the top, not bottom.
Array.prototype.indexOfreturns-1(notnull/undefined) for missing items, so the nullish coalescing?? 99never fires. Any provider not inproviderOrdergets index-1, causing it to sort before all listed providers rather than after them. The fix is to use|| 99(which catches -1) or an explicit sentinel:🐛 Proposed fix
const sortedProviders = Object.keys(grouped).sort( (a, b) => - (providerOrder.indexOf(a) ?? 99) - (providerOrder.indexOf(b) ?? 99), + (providerOrder.indexOf(a) === -1 ? 99 : providerOrder.indexOf(a)) - + (providerOrder.indexOf(b) === -1 ? 99 : providerOrder.indexOf(b)), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@interface/src/components/ModelSelect.tsx` around lines 149 - 152, The comparator for sortedProviders uses (providerOrder.indexOf(a) ?? 99) which never fires because indexOf returns -1 for missing values, so unknown providers sort to the top; update the comparator to treat -1 as a sentinel (e.g. compute const ia = providerOrder.indexOf(a); const ib = providerOrder.indexOf(b); return (ia === -1 ? 99 : ia) - (ib === -1 ? 99 : ib)) so unknown providers sort after known ones; change the logic around the providerOrder.indexOf calls in the sortedProviders sort comparator to use this -1 check instead of nullish coalescing.src/config.rs (1)
2289-2296:⚠️ Potential issue | 🟡 MinorAll API key environment variables in
load_from_env()silently dropVarError::NotUnicodeinstead of explicit handling.The
.ok()pattern here and in the fallback at lines 2668–2675 swallows non-UTF8 env values without surfacing them. This contradicts the coding guideline: "Don't silently discard errors; no let _ = on Results. Handle, log, or propagate errors."While rare, misconfigured env vars become undiagnosable. Note that
load_from_toml()already usesresolve_env_value()for the primary lookup—apply consistent error handling toload_from_env()as well, covering all API keys (anthropic, openai, openrouter, kilo, zhipu, groq, together, fireworks, deepseek, xai, mistral, gemini, ollama, ollama_base_url).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 2289 - 2296, In load_from_env(), do not call .ok() on std::env::var for the various LLM API keys (the LlmConfig fields: anthropic_key, openai_key, openrouter_key, kilo_key, zhipu_key and the other keys mentioned) because that silently drops VarError::NotUnicode; instead, perform explicit handling: use std::env::var and match its Result, convert or surface VarError::NotUnicode via the existing resolve_env_value() helper (or log/propagate a clear error) so non-UTF8 env values are not swallowed; apply this change consistently for all API key lookups (including groq, together, fireworks, deepseek, xai, mistral, gemini, ollama, ollama_base_url and the anthropic fallback logic) so errors are logged or returned rather than ignored.
🧹 Nitpick comments (3)
src/api/models.rs (1)
104-119:direct_provider_mapping("kilo")is dead code — kilo models need to be inextra_models()instead.Kilo is a gateway (not a base model provider), so models.dev almost certainly doesn't list a
"kilo"provider. Thismatcharm will never fire. More critically, sinceextra_models()has no kilo entries,GET /api/models?provider=kiloreturns an empty list, leaving the model picker in the Kilo config dialog blank — users must type the model ID manually, unlike every other provider.Known Kilo gateway model IDs follow the routing convention already used elsewhere in this PR (
kilo/anthropic/claude-sonnet-4.5,kilo/anthropic/claude-haiku-4.5, etc.). These should be seeded inextra_models().♻️ Proposed fix
- "kilo" => Some("kilo"),And in
extra_models():+ // Kilo Gateway + ModelInfo { + id: "kilo/anthropic/claude-sonnet-4.5".into(), + name: "Claude Sonnet 4.5".into(), + provider: "kilo".into(), + context_window: Some(200_000), + tool_call: true, + reasoning: false, + input_audio: false, + }, + ModelInfo { + id: "kilo/anthropic/claude-haiku-4.5".into(), + name: "Claude Haiku 4.5".into(), + provider: "kilo".into(), + context_window: Some(200_000), + tool_call: true, + reasoning: false, + input_audio: false, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/models.rs` around lines 104 - 119, The "kilo" arm in direct_provider_mapping is dead and prevents Kilo gateway models from being exposed; remove the "kilo" => Some("kilo") match arm from direct_provider_mapping and instead add the known Kilo gateway model IDs to the extra_models() seed list (e.g., entries like "kilo/anthropic/claude-sonnet-4.5", "kilo/anthropic/claude-haiku-4.5" or other Kilo gateway IDs used elsewhere in the repo) so GET /api/models?provider=kilo returns those models and the Kilo config picker is populated; update extra_models() to include these new strings and ensure provider filtering logic recognizes "kilo" as a provider for those IDs.src/llm/model.rs (1)
746-757: UnreachableApiType::KiloGatewayarm incall_openai_compatible.
attempt_completiondispatchesApiType::KiloGatewayexclusively tocall_openai_compatible_with_optional_auth, so theKiloGatewaybranch in this function'sendpoint_pathmatch can never execute. Remove it to keep the match consistent with actual routing.🧹 Proposed cleanup
- ApiType::OpenAiChatCompletions | ApiType::Gemini | ApiType::KiloGateway => { + ApiType::OpenAiChatCompletions | ApiType::Gemini => { "/chat/completions" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm/model.rs` around lines 746 - 757, The match in call_openai_compatible that sets endpoint_path includes an unreachable ApiType::KiloGateway arm; remove KiloGateway from that match so the arms reflect only the ApiType variants actually routed here (e.g., OpenAiCompletions, OpenAiResponses, OpenAiChatCompletions, Gemini, Anthropic handling remains as-is). Update the match to no longer list KiloGateway and ensure any logic that depends on provider_config.api_type in call_openai_compatible remains consistent with call_openai_compatible_with_optional_auth dispatching KiloGateway elsewhere.README.md (1)
187-187:api_typecomment omitsgeminiandkilo_gatewayvalues.The inline comment lists four of the six valid
api_typevalues. Sinceconfig.mdxnow documents all six, this example comment could point readers to the full set or at least includekilo_gatewayfor consistency with what this PR introduces.📝 Suggested update
-api_type = "openai_completions" # or "openai_chat_completions", "openai_responses", "anthropic" +api_type = "openai_completions" # or "openai_chat_completions", "openai_responses", "anthropic", "gemini", "kilo_gateway"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 187, The inline comment for the api_type example is missing the "gemini" and "kilo_gateway" options; update the comment attached to the api_type example so it lists all valid values (e.g., "openai_completions", "openai_chat_completions", "openai_responses", "anthropic", "gemini", "kilo_gateway") or change it to point readers to the full set in config.mdx; locate the api_type example in README.md and modify its trailing comment accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/content/docs/`(configuration)/config.mdx:
- Line 177: Update the sentence that lists implicit env fallbacks for LLM keys
to include all 12 environment variables referenced by the [llm] configuration
block: ANTHROPIC_API_KEY, OPENAI_API_KEY, OPENROUTER_API_KEY, KILO_API_KEY,
ZHIPU_API_KEY, GROQ_API_KEY, TOGETHER_API_KEY, FIREWORKS_API_KEY,
DEEPSEEK_API_KEY, XAI_API_KEY, MISTRAL_API_KEY, and OPENCODE_ZEN_API_KEY so the
docs match the implicit fallback behavior implemented in the [llm] config
parsing (see the [llm] configuration block and its environment lookups).
---
Outside diff comments:
In `@interface/src/components/ModelSelect.tsx`:
- Around line 149-152: The comparator for sortedProviders uses
(providerOrder.indexOf(a) ?? 99) which never fires because indexOf returns -1
for missing values, so unknown providers sort to the top; update the comparator
to treat -1 as a sentinel (e.g. compute const ia = providerOrder.indexOf(a);
const ib = providerOrder.indexOf(b); return (ia === -1 ? 99 : ia) - (ib === -1 ?
99 : ib)) so unknown providers sort after known ones; change the logic around
the providerOrder.indexOf calls in the sortedProviders sort comparator to use
this -1 check instead of nullish coalescing.
In `@src/config.rs`:
- Around line 2289-2296: In load_from_env(), do not call .ok() on std::env::var
for the various LLM API keys (the LlmConfig fields: anthropic_key, openai_key,
openrouter_key, kilo_key, zhipu_key and the other keys mentioned) because that
silently drops VarError::NotUnicode; instead, perform explicit handling: use
std::env::var and match its Result, convert or surface VarError::NotUnicode via
the existing resolve_env_value() helper (or log/propagate a clear error) so
non-UTF8 env values are not swallowed; apply this change consistently for all
API key lookups (including groq, together, fireworks, deepseek, xai, mistral,
gemini, ollama, ollama_base_url and the anthropic fallback logic) so errors are
logged or returned rather than ignored.
---
Nitpick comments:
In `@README.md`:
- Line 187: The inline comment for the api_type example is missing the "gemini"
and "kilo_gateway" options; update the comment attached to the api_type example
so it lists all valid values (e.g., "openai_completions",
"openai_chat_completions", "openai_responses", "anthropic", "gemini",
"kilo_gateway") or change it to point readers to the full set in config.mdx;
locate the api_type example in README.md and modify its trailing comment
accordingly.
In `@src/api/models.rs`:
- Around line 104-119: The "kilo" arm in direct_provider_mapping is dead and
prevents Kilo gateway models from being exposed; remove the "kilo" =>
Some("kilo") match arm from direct_provider_mapping and instead add the known
Kilo gateway model IDs to the extra_models() seed list (e.g., entries like
"kilo/anthropic/claude-sonnet-4.5", "kilo/anthropic/claude-haiku-4.5" or other
Kilo gateway IDs used elsewhere in the repo) so GET /api/models?provider=kilo
returns those models and the Kilo config picker is populated; update
extra_models() to include these new strings and ensure provider filtering logic
recognizes "kilo" as a provider for those IDs.
In `@src/llm/model.rs`:
- Around line 746-757: The match in call_openai_compatible that sets
endpoint_path includes an unreachable ApiType::KiloGateway arm; remove
KiloGateway from that match so the arms reflect only the ApiType variants
actually routed here (e.g., OpenAiCompletions, OpenAiResponses,
OpenAiChatCompletions, Gemini, Anthropic handling remains as-is). Update the
match to no longer list KiloGateway and ensure any logic that depends on
provider_config.api_type in call_openai_compatible remains consistent with
call_openai_compatible_with_optional_auth dispatching KiloGateway elsewhere.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
README.mddocs/content/docs/(configuration)/config.mdxdocs/content/docs/(deployment)/roadmap.mdxdocs/content/docs/(getting-started)/quickstart.mdxinterface/src/api/client.tsinterface/src/components/ModelSelect.tsxinterface/src/lib/providerIcons.tsxinterface/src/routes/Settings.tsxsrc/api/models.rssrc/api/providers.rssrc/config.rssrc/llm/model.rssrc/llm/providers.rssrc/llm/routing.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.rs (1)
4298-4315:⚠️ Potential issue | 🔴 CriticalOnboarding provider index mapping is out of sync (Kilo selection resolves incorrectly).
The menu has 16 items (
0..15), but the matcher still uses aGeminislot at10and assigns Kilo to16. In this state, Kilo cannot be selected correctly from onboarding.🛠️ Minimal mapping fix for current menu order
- 10 => ("Google Gemini API key", "gemini_key", "gemini"), - 11 => ("Ollama base URL (optional)", "ollama_base_url", "ollama"), - 12 => ("OpenCode Zen API key", "opencode_zen_key", "opencode-zen"), - 13 => ("MiniMax API key", "minimax_key", "minimax"), - 14 => ("Moonshot API key", "moonshot_key", "moonshot"), - 15 => ( + 10 => ("Ollama base URL (optional)", "ollama_base_url", "ollama"), + 11 => ("OpenCode Zen API key", "opencode_zen_key", "opencode-zen"), + 12 => ("MiniMax API key", "minimax_key", "minimax"), + 13 => ("Moonshot API key", "moonshot_key", "moonshot"), + 14 => ( "Z.AI Coding Plan API key", "zai_coding_plan_key", "zai-coding-plan", ), - 16 => ("Kilo Gateway API key", "kilo_key", "kilo"), + 15 => ("Kilo Gateway API key", "kilo_key", "kilo"),A safer follow-up is to replace index-based matching with a single
ProviderChoicetable used for both display and metadata.Also applies to: 4357-4380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 4298 - 4315, The onboarding provider index mapping is out of sync with the providers array; update the index-based matcher(s) that reference Gemini at 10 and Kilo at 16 so they match the providers array (which is 16 items, indices 0..15, with "Kilo Gateway" at index 15), i.e. change any hardcoded case/constant that sets Kilo to 16 to 15 and remove the incorrect Gemini=10 slot, and similarly fix the block around the second mapping mentioned (lines ~4357-4380) so all indices align; as a safer follow-up replace these parallel index maps with a single ProviderChoice table used for both display and onboarding metadata to avoid future drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/api/providers.rs`:
- Around line 195-200: The match arm for "kilo" constructs a ProviderConfig but
omits the required use_bearer_auth field, causing a compile error; update the
"kilo" ProviderConfig (the match arm that sets ApiType::KiloGateway) to include
use_bearer_auth with the appropriate boolean (e.g., true if Kilo expects Bearer
token) alongside api_type, base_url, api_key, and name so the struct
initialization is complete.
In `@src/config.rs`:
- Around line 2361-2369: The ProviderConfig initializers for the Kilo provider
(triggered when llm.kilo_key is Some) are missing the required boolean field
use_bearer_auth, causing a compile error; update both Kilo registration sites
where ProviderConfig is constructed (the blocks creating ProviderConfig with
api_type ApiType::KiloGateway and base_url KILO_PROVIDER_BASE_URL) to include
use_bearer_auth: true or false as appropriate (match other providers or expected
behavior), so each ProviderConfig includes use_bearer_auth alongside api_type,
base_url, api_key, and name.
---
Outside diff comments:
In `@src/config.rs`:
- Around line 4298-4315: The onboarding provider index mapping is out of sync
with the providers array; update the index-based matcher(s) that reference
Gemini at 10 and Kilo at 16 so they match the providers array (which is 16
items, indices 0..15, with "Kilo Gateway" at index 15), i.e. change any
hardcoded case/constant that sets Kilo to 16 to 15 and remove the incorrect
Gemini=10 slot, and similarly fix the block around the second mapping mentioned
(lines ~4357-4380) so all indices align; as a safer follow-up replace these
parallel index maps with a single ProviderChoice table used for both display and
onboarding metadata to avoid future drift.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/content/docs/(getting-started)/quickstart.mdxinterface/src/api/client.tsinterface/src/routes/Settings.tsxsrc/api/providers.rssrc/config.rssrc/llm/model.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/content/docs/(getting-started)/quickstart.mdx
- interface/src/api/client.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config.rs (1)
4300-4317:⚠️ Potential issue | 🟠 MajorFix onboarding provider index mismatch (wrong provider gets configured).
The provider display list and
provider_idxmatch table are out of sync.providerscurrently has no Gemini option, but Line 4370 assumes index10is Gemini. This shifts mappings from Ollama onward; e.g., selecting Kilo Gateway (Line 4316) won’t reach Line 4380 and will be treated as another provider.🛠️ Proposed fix
let providers = &[ "Anthropic", "OpenRouter", "OpenAI", "Z.ai (GLM)", "Groq", "Together AI", "Fireworks AI", "DeepSeek", "xAI (Grok)", "Mistral AI", + "Google Gemini", "Ollama", "OpenCode Zen", "MiniMax", "Moonshot AI (Kimi)", "Z.AI Coding Plan", "Kilo Gateway", ];Also applies to: 4359-4381
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 4300 - 4317, The providers array and the provider_idx match table are out of sync causing the wrong provider to be configured; update the providers list (the providers variable) to include "Gemini" at the correct position (so its index aligns with the provider_idx mapping that expects Gemini at index 10) or reorder/adjust the provider_idx mapping to reflect the current providers array order; ensure the selection logic that uses provider_idx and the providers array (the onboarding provider selection code) are consistent so selecting items like "Kilo Gateway" maps to the intended provider.
🧹 Nitpick comments (2)
src/config.rs (1)
2361-2394: Consider extracting shorthand provider registration into a shared helper.
load_from_envandfrom_tomlduplicate provider registration blocks (including Kilo/Z.AI variants). Centralizing this mapping would reduce drift and prevent future mismatches between the two paths.Also applies to: 2886-2919
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 2361 - 2394, The provider-registration blocks for Kilo/Z.AI are duplicated between load_from_env and from_toml; extract that logic into a shared helper (e.g., a function like register_shorthand_providers or add_shorthand_provider) that takes &mut LlmConfig (or the llm variable), key name, ProviderConfig fields (api_type, base_url constant, api_key, name, use_bearer_auth) and performs llm.providers.entry(...).or_insert_with(...); then replace the inline blocks in load_from_env and from_toml to call this helper for "kilo", "zhipu", "zai-coding-plan" using the existing constants KILO_PROVIDER_BASE_URL, ZHIPU_PROVIDER_BASE_URL, ZAI_CODING_PLAN_BASE_URL and the respective kilo_key, zhipu_key, zai_coding_plan_key variables.src/api/providers.rs (1)
169-300: Prefer shared provider metadata sources instead of duplicated literals.
build_test_llm_config()hardcodes base URLs/names that are also defined elsewhere (e.g., config defaults). Reusing a shared source (constants/helper) would lower drift risk when providers evolve.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/providers.rs` around lines 169 - 300, build_test_llm_config currently duplicates provider base_url and name literals (creating drift); update it to reference the canonical provider metadata/constants instead of hardcoding values: replace literal strings inside ProviderConfig constructions in build_test_llm_config with the shared constants or helper functions exported from crate::config (e.g., use existing GEMINI_PROVIDER_BASE_URL and any provider-specific BASE_URL/_NAME constants or a provider metadata map), keep ApiType and api_key assignment the same, and remove duplicated literals so provider info is sourced from the single shared location.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/config.rs`:
- Around line 4300-4317: The providers array and the provider_idx match table
are out of sync causing the wrong provider to be configured; update the
providers list (the providers variable) to include "Gemini" at the correct
position (so its index aligns with the provider_idx mapping that expects Gemini
at index 10) or reorder/adjust the provider_idx mapping to reflect the current
providers array order; ensure the selection logic that uses provider_idx and the
providers array (the onboarding provider selection code) are consistent so
selecting items like "Kilo Gateway" maps to the intended provider.
---
Nitpick comments:
In `@src/api/providers.rs`:
- Around line 169-300: build_test_llm_config currently duplicates provider
base_url and name literals (creating drift); update it to reference the
canonical provider metadata/constants instead of hardcoding values: replace
literal strings inside ProviderConfig constructions in build_test_llm_config
with the shared constants or helper functions exported from crate::config (e.g.,
use existing GEMINI_PROVIDER_BASE_URL and any provider-specific BASE_URL/_NAME
constants or a provider metadata map), keep ApiType and api_key assignment the
same, and remove duplicated literals so provider info is sourced from the single
shared location.
In `@src/config.rs`:
- Around line 2361-2394: The provider-registration blocks for Kilo/Z.AI are
duplicated between load_from_env and from_toml; extract that logic into a shared
helper (e.g., a function like register_shorthand_providers or
add_shorthand_provider) that takes &mut LlmConfig (or the llm variable), key
name, ProviderConfig fields (api_type, base_url constant, api_key, name,
use_bearer_auth) and performs llm.providers.entry(...).or_insert_with(...); then
replace the inline blocks in load_from_env and from_toml to call this helper for
"kilo", "zhipu", "zai-coding-plan" using the existing constants
KILO_PROVIDER_BASE_URL, ZHIPU_PROVIDER_BASE_URL, ZAI_CODING_PLAN_BASE_URL and
the respective kilo_key, zhipu_key, zai_coding_plan_key variables.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/config.rs (2)
4739-4763:⚠️ Potential issue | 🟡 MinorTest env isolation is incomplete for
MINIMAX_CN_API_KEY.
EnvGuarddoes not snapshot/restoreMINIMAX_CN_API_KEY, so tests can leak env state when that variable is present.🧪 Fix for deterministic tests
- const KEYS: [&str; 23] = [ + const KEYS: [&str; 24] = [ "SPACEBOT_DIR", "SPACEBOT_DEPLOYMENT", "SPACEBOT_CRON_TIMEZONE", "ANTHROPIC_API_KEY", "ANTHROPIC_OAUTH_TOKEN", "OPENAI_API_KEY", "OPENROUTER_API_KEY", "KILO_API_KEY", "ZHIPU_API_KEY", "GROQ_API_KEY", "TOGETHER_API_KEY", "FIREWORKS_API_KEY", "DEEPSEEK_API_KEY", "XAI_API_KEY", "MISTRAL_API_KEY", "GEMINI_API_KEY", "NVIDIA_API_KEY", "OLLAMA_API_KEY", "OLLAMA_BASE_URL", "OPENCODE_ZEN_API_KEY", "MINIMAX_API_KEY", + "MINIMAX_CN_API_KEY", "MOONSHOT_API_KEY", "ZAI_CODING_PLAN_API_KEY", ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 4739 - 4763, The test environment snapshot in src/config.rs is missing MINIMAX_CN_API_KEY from the KEYS list used by EnvGuard, causing env leaks; update the KEYS constant (the array named KEYS) to include "MINIMAX_CN_API_KEY" so that EnvGuard will snapshot and restore that variable (ensure the same identifier KEYS used by EnvGuard is modified so snapshot/restore covers MINIMAX_CN_API_KEY).
2383-2400:⚠️ Potential issue | 🟡 Minor
needs_onboarding()still missesMINIMAX_CN_API_KEY.If only
MINIMAX_CN_API_KEYis set, onboarding can still trigger even though Line 2477 and Line 2958 support that provider path.🛠️ Minimal fix
let has_legacy_keys = std::env::var("ANTHROPIC_API_KEY").is_ok() || std::env::var("OPENAI_API_KEY").is_ok() || std::env::var("OPENROUTER_API_KEY").is_ok() || std::env::var("KILO_API_KEY").is_ok() || std::env::var("ZHIPU_API_KEY").is_ok() || std::env::var("GROQ_API_KEY").is_ok() || std::env::var("TOGETHER_API_KEY").is_ok() || std::env::var("FIREWORKS_API_KEY").is_ok() || std::env::var("DEEPSEEK_API_KEY").is_ok() || std::env::var("XAI_API_KEY").is_ok() || std::env::var("MISTRAL_API_KEY").is_ok() || std::env::var("NVIDIA_API_KEY").is_ok() || std::env::var("OLLAMA_API_KEY").is_ok() || std::env::var("OLLAMA_BASE_URL").is_ok() || std::env::var("OPENCODE_ZEN_API_KEY").is_ok() || std::env::var("MINIMAX_API_KEY").is_ok() + || std::env::var("MINIMAX_CN_API_KEY").is_ok() || std::env::var("MOONSHOT_API_KEY").is_ok() || std::env::var("ZAI_CODING_PLAN_API_KEY").is_ok();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 2383 - 2400, The has_legacy_keys check inside needs_onboarding() omits the MINIMAX_CN_API_KEY env var so onboarding can incorrectly run when only that key is set; update the boolean expression that sets has_legacy_keys (the std::env::var(...) chain) to also check std::env::var("MINIMAX_CN_API_KEY").is_ok() so that needs_onboarding() recognizes the Minimax CN provider path.
🧹 Nitpick comments (1)
src/config.rs (1)
342-501: Centralize provider registration on one source of truth.
default_provider_configis a good start, but provider metadata is still duplicated across this helper and theload_from_env/from_tomlinsertion paths. This will drift again over time.♻️ Suggested direction
-fn add_shorthand_provider( +fn add_shorthand_provider( providers: &mut std::collections::HashMap<String, ProviderConfig>, provider_id: &str, key: Option<String>, - api_type: ApiType, - base_url: &str, - name: Option<&str>, - use_bearer_auth: bool, ) { if let Some(api_key) = key { - providers - .entry(provider_id.to_string()) - .or_insert_with(|| ProviderConfig { - api_type, - base_url: base_url.to_string(), - api_key, - name: name.map(str::to_string), - use_bearer_auth, - }); + if let Some(default_config) = default_provider_config(provider_id, api_key) { + providers + .entry(provider_id.to_string()) + .or_insert(default_config); + } } }Then call this with only
(providers, provider_id, key)where applicable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config.rs` around lines 342 - 501, The provider metadata is duplicated between default_provider_config and the insertion paths (load_from_env / from_toml); consolidate metadata into a single source (e.g., a static/map or helper function mapping provider_id to (ApiType, base_url, name, use_bearer_auth)) and have default_provider_config and add_shorthand_provider use that centralized mapping to construct ProviderConfig instances; update add_shorthand_provider to call that lookup (or default_provider_config) when key is Some, and update load_from_env/from_toml to call add_shorthand_provider with only (providers, provider_id, key) so all metadata comes from the single source (referencing default_provider_config, add_shorthand_provider, ProviderConfig, ApiType, load_from_env, from_toml).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/config.rs`:
- Around line 4739-4763: The test environment snapshot in src/config.rs is
missing MINIMAX_CN_API_KEY from the KEYS list used by EnvGuard, causing env
leaks; update the KEYS constant (the array named KEYS) to include
"MINIMAX_CN_API_KEY" so that EnvGuard will snapshot and restore that variable
(ensure the same identifier KEYS used by EnvGuard is modified so
snapshot/restore covers MINIMAX_CN_API_KEY).
- Around line 2383-2400: The has_legacy_keys check inside needs_onboarding()
omits the MINIMAX_CN_API_KEY env var so onboarding can incorrectly run when only
that key is set; update the boolean expression that sets has_legacy_keys (the
std::env::var(...) chain) to also check
std::env::var("MINIMAX_CN_API_KEY").is_ok() so that needs_onboarding()
recognizes the Minimax CN provider path.
---
Nitpick comments:
In `@src/config.rs`:
- Around line 342-501: The provider metadata is duplicated between
default_provider_config and the insertion paths (load_from_env / from_toml);
consolidate metadata into a single source (e.g., a static/map or helper function
mapping provider_id to (ApiType, base_url, name, use_bearer_auth)) and have
default_provider_config and add_shorthand_provider use that centralized mapping
to construct ProviderConfig instances; update add_shorthand_provider to call
that lookup (or default_provider_config) when key is Some, and update
load_from_env/from_toml to call add_shorthand_provider with only (providers,
provider_id, key) so all metadata comes from the single source (referencing
default_provider_config, add_shorthand_provider, ProviderConfig, ApiType,
load_from_env, from_toml).
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/api/providers.rssrc/config.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/api/providers.rs
Contributor
Author
|
Superseded by #225 |
aspotton
added a commit
to aspotton/spacebot
that referenced
this pull request
Apr 7, 2026
…bounce Add parent_task_number to DelegationConfig and SendAgentMessageTool so that when a delegated worker creates sub-tasks, they carry a reference to their parent task. When the cortex completes a child task, it automatically marks the parent task as done. Changes: - DelegationConfig: added parent_task_number field - SendAgentMessageTool: added with_parent_task_number setter and includes parent_task_number in task metadata - worker.rs: extracts task_number from task_metadata and passes it as parent_task_number when creating sub-tasks - cortex.rs: auto-completes parent task when child task completes This eliminates the bounce pattern where Planning Lead workers created follow-up tasks (spacedriveapp#189, spacedriveapp#191, spacedriveapp#192) to check on delegated task spacedriveapp#188.
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
2 participants
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.
Summary
This PR adds first-class support for Kilo Gateway across config, routing, API provider management, model catalog mapping, and the Settings UI.
Changes
kilo_keyandKILO_API_KEYsupport in config, with automatic provider registration askilo(https://api.kilo.ai/api/gateway)channel/branch:kilo/anthropic/claude-sonnet-4.5worker/compactor/cortex:kilo/anthropic/claude-haiku-4.5kilo/...) in provider routing helpers/chat/completions(instead of/v1/chat/completions)HTTP-Referer: https://github.com/spacedriveapp/spacebotX-Title: spacebot/api/providers,/api/models)Why
Kilo is OpenAI-compatible but uses a different chat completions path shape than providers that use
/v1/chat/completions. This makes Kilo work out of the box in Spacebot with proper routing and setup support.Note
Comprehensive integration of Kilo Gateway support with 14 files modified. Changes span config handling, provider routing (with Claude Sonnet 4.5 for channels and Claude Haiku 4.5 for workers), OpenAI-compatible API support with custom headers, model discovery endpoints, and UI components. Documentation updated across quickstart, config guide, and roadmap to reflect Kilo among supported providers (now 12 total).
Written by Tembo for commit 064b922. This will update automatically on new commits.