From 760dae06e540e099c7f2ce4dcf724e29ad537ea2 Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Sat, 18 Jul 2026 15:41:53 +0100 Subject: [PATCH] feat(providers): Moonshot AI provider + extract shared OpenAI-compat helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add MoonshotProvider and MoonshotCodeProvider (OpenAI-compatible) - Extract `fetch_openai_models()` and `build_openai_llm()` shared functions, deduplicating model-listing and client construction across OpenAI, OpenRouter, DeepSeek, LM Studio, and Z.AI providers - Add `lists_models` field to `ProviderUiMeta` — drives frontend model picker dynamically instead of hardcoded `type_id` list - Port DeepSeek, LM Studio, OpenRouter model listing to `fetch_openai_models` - Set `lists_models: true` on Z.AI provider (missed in prior pass) feat(mcp): named key placeholders {SECRET:name}/{ENV:name} in connector URLs - `apply_key_placeholder` now accepts an `env` map alongside `api_key` - Named tokens resolve from the connector's described `env[]` fields, with `{SECRET:name}` falling back to `api_key` for backward compat - Replace `substitute_secret_tokens` with `substitute_named_tokens` - Unresolved tokens are left in place (visible misconfig) rather than silently producing a wrong URL fix(ui): connector detail hides generic API key when schema has secret fix(ui): model picker uses `lists_models` from provider types endpoint --- crates/core-api/src/provider.rs | 4 + crates/plugin-elevenlabs/src/lib.rs | 1 + .../skald-core/src/llm/providers/anthropic.rs | 1 + .../skald-core/src/llm/providers/deepseek.rs | 35 +--- .../skald-core/src/llm/providers/lm_studio.rs | 19 +- crates/skald-core/src/llm/providers/mod.rs | 60 +++++- .../skald-core/src/llm/providers/moonshot.rs | 197 ++++++++++++++++++ crates/skald-core/src/llm/providers/ollama.rs | 1 + crates/skald-core/src/llm/providers/openai.rs | 14 +- .../src/llm/providers/openrouter.rs | 32 +-- crates/skald-core/src/llm/providers/zai.rs | 18 +- crates/skald-core/src/mcp/mod.rs | 174 ++++++++++++---- crates/skald-core/src/skald/bundles.rs | 2 + web/components/connector-detail.js | 7 +- web/components/models-llm.js | 15 +- 15 files changed, 442 insertions(+), 138 deletions(-) create mode 100644 crates/skald-core/src/llm/providers/moonshot.rs diff --git a/crates/core-api/src/provider.rs b/crates/core-api/src/provider.rs index b011497..deb89c0 100644 --- a/crates/core-api/src/provider.rs +++ b/crates/core-api/src/provider.rs @@ -123,6 +123,10 @@ pub struct ProviderUiMeta { pub color: &'static str, pub icon: &'static str, pub fields: &'static [ProviderField], + /// Whether the provider can list its remote LLM catalog + /// (`list_llm_models` is implemented). Drives the frontend's model picker — + /// no provider type_id is ever hardcoded in the UI. + pub lists_models: bool, } #[derive(Debug, Clone, serde::Serialize)] diff --git a/crates/plugin-elevenlabs/src/lib.rs b/crates/plugin-elevenlabs/src/lib.rs index 1b7889b..f18126c 100644 --- a/crates/plugin-elevenlabs/src/lib.rs +++ b/crates/plugin-elevenlabs/src/lib.rs @@ -307,6 +307,7 @@ impl ApiProvider for ElevenLabsProvider { description: Some("Text-to-speech and transcription"), color: "#f59e0b", icon: "bi-waveform", + lists_models: false, fields: &[ ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, ], diff --git a/crates/skald-core/src/llm/providers/anthropic.rs b/crates/skald-core/src/llm/providers/anthropic.rs index ceb716c..5e23baf 100644 --- a/crates/skald-core/src/llm/providers/anthropic.rs +++ b/crates/skald-core/src/llm/providers/anthropic.rs @@ -111,6 +111,7 @@ impl ApiProvider for AnthropicProvider { description: None, color: "#d4a574", icon: "bi-chat-square-dots", + lists_models: false, fields: &[ ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, ], diff --git a/crates/skald-core/src/llm/providers/deepseek.rs b/crates/skald-core/src/llm/providers/deepseek.rs index 953a2f3..41439d0 100644 --- a/crates/skald-core/src/llm/providers/deepseek.rs +++ b/crates/skald-core/src/llm/providers/deepseek.rs @@ -1,10 +1,7 @@ -use std::sync::Arc; +use anyhow::{Result, anyhow}; -use anyhow::{Context, Result, anyhow}; - -use crate::chatbot::openai::OpenAiClient; use crate::llm::{LlmModelRecord, LlmProviderRecord}; -use crate::llm::providers::{RemoteLlmModelInfo, extra_with_reasoning}; +use crate::llm::providers::{RemoteLlmModelInfo, build_openai_llm, fetch_openai_models}; use crate::provider::{ApiProvider, BuiltLlmClient, ProviderField, ProviderUiMeta, ReasoningMode, ServiceType}; pub struct DeepSeekProvider { @@ -50,21 +47,8 @@ impl ApiProvider for DeepSeekProvider { let api_key = record.api_key.as_deref() .ok_or_else(|| anyhow!("provider '{}': api_key required for deepseek model listing", record.name))?; - let resp: serde_json::Value = self.http - .get("https://api.deepseek.com/models") - .bearer_auth(api_key) - .send() - .await - .map_err(|e| anyhow!("DeepSeek request failed: {e}"))? - .error_for_status() - .map_err(|e| anyhow!("DeepSeek error response: {e}"))? - .json() - .await - .map_err(|e| anyhow!("DeepSeek response parse failed: {e}"))?; - - let models = resp["data"] - .as_array() - .ok_or_else(|| anyhow!("unexpected DeepSeek response shape"))? + let raw = fetch_openai_models(&self.http, "https://api.deepseek.com", Some(api_key), "DeepSeek").await?; + let models = raw .iter() .filter_map(|m| { let id = m["id"].as_str()?.to_string(); @@ -120,15 +104,7 @@ impl ApiProvider for DeepSeekProvider { } fn build_llm(&self, record: &LlmProviderRecord, model: &LlmModelRecord) -> Option> { - Some((|| { - let key = record.api_key.as_deref() - .with_context(|| format!("provider '{}': api_key required for deepseek", record.name))?; - let extra = extra_with_reasoning(self, model); - Ok(BuiltLlmClient { - client: Arc::new(OpenAiClient::new("https://api.deepseek.com/v1", key, extra, false)), - prompt_cache: false, - }) - })()) + Some(build_openai_llm(self, "https://api.deepseek.com/v1", record, model, false)) } fn ui_meta(&self) -> ProviderUiMeta { @@ -138,6 +114,7 @@ impl ApiProvider for DeepSeekProvider { description: None, color: "#0ea5e9", icon: "bi-search", + lists_models: true, fields: &[ ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, ], diff --git a/crates/skald-core/src/llm/providers/lm_studio.rs b/crates/skald-core/src/llm/providers/lm_studio.rs index 9881d86..5290d6c 100644 --- a/crates/skald-core/src/llm/providers/lm_studio.rs +++ b/crates/skald-core/src/llm/providers/lm_studio.rs @@ -1,10 +1,10 @@ use std::sync::Arc; -use anyhow::{Result, anyhow}; +use anyhow::Result; use crate::chatbot::lm_studio::LmStudioClient; use crate::llm::{LlmModelRecord, LlmProviderRecord}; -use crate::llm::providers::RemoteLlmModelInfo; +use crate::llm::providers::{RemoteLlmModelInfo, fetch_openai_models}; use crate::provider::{ApiProvider, BuiltLlmClient, ProviderField, ProviderUiMeta, ServiceType}; pub struct LmStudioProvider { @@ -31,19 +31,9 @@ impl ApiProvider for LmStudioProvider { } async fn list_llm_models(&self, record: &LlmProviderRecord) -> Result>> { - let url = format!("{}/models", Self::base_url(record).trim_end_matches('/')); - let resp: serde_json::Value = self.http - .get(&url) - .send() - .await - .map_err(|e| anyhow!("LM Studio request failed: {e}"))? - .json() - .await - .map_err(|e| anyhow!("LM Studio response parse failed: {e}"))?; + let raw = fetch_openai_models(&self.http, &Self::base_url(record), None, "LM Studio").await?; - let models = resp["data"] - .as_array() - .ok_or_else(|| anyhow!("unexpected LM Studio response shape"))? + let models = raw .iter() .filter_map(|m| { let id = m["id"].as_str()?.to_string(); @@ -78,6 +68,7 @@ impl ApiProvider for LmStudioProvider { description: Some("Local models via LM Studio"), color: "#6b7280", icon: "bi-window-stack", + lists_models: true, fields: &[ ProviderField { key: "base_url", label: "Base URL", required: false, secret: false }, ], diff --git a/crates/skald-core/src/llm/providers/mod.rs b/crates/skald-core/src/llm/providers/mod.rs index 666fb29..edae18b 100644 --- a/crates/skald-core/src/llm/providers/mod.rs +++ b/crates/skald-core/src/llm/providers/mod.rs @@ -1,6 +1,7 @@ pub mod anthropic; pub mod deepseek; pub mod lm_studio; +pub mod moonshot; pub mod ollama; pub mod openai; pub mod openrouter; @@ -10,7 +11,13 @@ pub mod zai; pub use crate::provider::ServiceType; pub use core_api::provider::RemoteLlmModelInfo; -use core_api::provider::{ApiProvider, LlmModelRecord}; +use std::sync::Arc; + +use anyhow::{anyhow, Context, Result}; + +use core_api::provider::{ApiProvider, BuiltLlmClient, LlmModelRecord, LlmProviderRecord}; + +use crate::chatbot::openai::OpenAiClient; /// Computes the `extra_params` an OpenAI-compatible client should be built with, /// given a model's stored `extra_params` and its selected reasoning value. The @@ -37,3 +44,54 @@ pub(crate) fn extra_with_reasoning( } } } + +/// Fetches the OpenAI-style `GET {base_url}/models` catalog shared by most +/// OpenAI-compatible providers. Returns the raw per-model JSON objects from +/// the `data` envelope so each provider can map and enrich them with its own +/// heuristics. `api_key` is sent as a bearer token when present (local +/// providers pass `None`); `who` is the display name used in error messages. +pub(crate) async fn fetch_openai_models( + http: &reqwest::Client, + base_url: &str, + api_key: Option<&str>, + who: &str, +) -> Result> { + let url = format!("{}/models", base_url.trim_end_matches('/')); + let mut req = http.get(&url); + if let Some(key) = api_key { + req = req.bearer_auth(key); + } + let resp: serde_json::Value = req + .send() + .await + .map_err(|e| anyhow!("{who} request failed: {e}"))? + .error_for_status() + .map_err(|e| anyhow!("{who} error response: {e}"))? + .json() + .await + .map_err(|e| anyhow!("{who} response parse failed: {e}"))?; + + resp["data"] + .as_array() + .cloned() + .ok_or_else(|| anyhow!("unexpected {who} response shape")) +} + +/// Builds an `OpenAiClient` for an OpenAI-compatible provider: requires the +/// provider record's `api_key` and merges the model's stored `extra_params` +/// with the provider-translated reasoning fragment (see `extra_with_reasoning`). +pub(crate) fn build_openai_llm( + provider: &dyn ApiProvider, + base_url: &str, + record: &LlmProviderRecord, + model: &LlmModelRecord, + prompt_cache: bool, +) -> Result { + let key = record.api_key.as_deref() + .with_context(|| format!("provider '{}': api_key required for {}", record.name, provider.type_id()))?; + let extra = extra_with_reasoning(provider, model); + Ok(BuiltLlmClient { + client: Arc::new(OpenAiClient::new(base_url, key, extra, prompt_cache)), + prompt_cache, + }) +} diff --git a/crates/skald-core/src/llm/providers/moonshot.rs b/crates/skald-core/src/llm/providers/moonshot.rs new file mode 100644 index 0000000..be21e8e --- /dev/null +++ b/crates/skald-core/src/llm/providers/moonshot.rs @@ -0,0 +1,197 @@ +use anyhow::{Result, anyhow}; + +use crate::llm::{LlmModelRecord, LlmProviderRecord}; +use crate::llm::providers::{RemoteLlmModelInfo, build_openai_llm, fetch_openai_models}; +use crate::provider::{ApiProvider, BuiltLlmClient, ProviderField, ProviderUiMeta, ReasoningMode, ServiceType}; + +/// Moonshot AI — pay-as-you-go platform. +/// +/// Endpoint `https://api.moonshot.ai/v1/chat/completions` (OpenAI-compatible); +/// `OpenAiClient` appends `/chat/completions`, so the base URL is `.../v1`. +/// +/// `GET /models` returns the full catalog including `context_length`, +/// `supports_image_in` and `supports_reasoning`, so the model list is entirely +/// endpoint-driven. Thinking models (kimi-k2-thinking…) always reason — the +/// platform exposes no request-level knob, hence no `ReasoningMode`. +pub struct MoonshotProvider { + http: reqwest::Client, +} + +/// Moonshot AI — Kimi Code subscription. +/// +/// Endpoint `https://api.kimi.com/coding/v1/chat/completions` (OpenAI-compatible). +/// The model catalog comes from `GET /models` like the platform; only the +/// metadata the endpoint omits (context size, vision) is filled from the +/// published docs. K3 is the only model with a reasoning knob: a graded +/// `reasoning_effort` (`low`/`high`/`max`, default `max`); the kimi-for-coding +/// series (K2.7 Code) always thinks and has no toggle. +pub struct MoonshotCodeProvider { + http: reqwest::Client, +} + +impl MoonshotProvider { + pub fn new() -> Self { + Self { http: reqwest::Client::new() } + } + + const BASE_URL: &'static str = "https://api.moonshot.ai/v1"; +} + +impl MoonshotCodeProvider { + pub fn new() -> Self { + Self { http: reqwest::Client::new() } + } + + const BASE_URL: &'static str = "https://api.kimi.com/coding/v1"; + + /// Fills the metadata the Kimi Code `/models` endpoint may omit, from the + /// published docs: K3 → up to 1M context + native visual understanding; + /// the kimi-for-coding series → 256k. Values already present in the + /// endpoint response (e.g. a tier-specific context size) always win. + fn enrich(info: &mut RemoteLlmModelInfo) { + let id = info.id.to_lowercase(); + if info.context_length.is_none() { + if id.starts_with("k3") { + info.context_length = Some(1_048_576); + } else if id.starts_with("kimi-for-coding") { + info.context_length = Some(262_144); + } + } + if info.vision.is_none() && id.starts_with("k3") { + info.vision = Some(true); + } + } +} + +/// Shared model listing for both Moonshot APIs: same envelope, same per-model +/// fields (`context_length`, `supports_image_in`, `supports_reasoning` — all +/// optional, absent fields are left for the caller to enrich). +async fn list_models( + http: &reqwest::Client, + base_url: &str, + record: &LlmProviderRecord, + who: &str, +) -> Result> { + let api_key = record.api_key.as_deref() + .ok_or_else(|| anyhow!("provider '{}': api_key required for {who} model listing", record.name))?; + + let raw = fetch_openai_models(http, base_url, Some(api_key), who).await?; + let models = raw + .iter() + .filter_map(|m| { + let id = m["id"].as_str()?.to_string(); + let mut capabilities = vec!["function_calling".to_string()]; + if m["supports_reasoning"].as_bool().unwrap_or(false) { + capabilities.push("reasoning".to_string()); + } + if m["supports_image_in"].as_bool().unwrap_or(false) { + capabilities.push("vision".to_string()); + } + Some(RemoteLlmModelInfo { + id, + name: m["id"].as_str()?.to_string(), + context_length: m["context_length"].as_u64(), + max_completion_tokens: None, + knowledge_cutoff: None, + capabilities, + vision: m["supports_image_in"].as_bool(), + price_input_per_million: None, + price_output_per_million: None, + reasoning: None, + }) + }) + .collect(); + + Ok(models) +} + +#[async_trait::async_trait] +impl ApiProvider for MoonshotProvider { + fn type_id(&self) -> &'static str { "moonshot" } + fn display_name(&self) -> &'static str { "Moonshot AI pay-as-you-go" } + fn supported_types(&self) -> &'static [ServiceType] { + &[ServiceType::Llm] + } + + async fn list_llm_models(&self, record: &LlmProviderRecord) -> Result>> { + Ok(Some(list_models(&self.http, Self::BASE_URL, record, "Moonshot AI").await?)) + } + + fn build_llm(&self, record: &LlmProviderRecord, model: &LlmModelRecord) -> Option> { + Some(build_openai_llm(self, Self::BASE_URL, record, model, false)) + } + + fn ui_meta(&self) -> ProviderUiMeta { + ProviderUiMeta { + type_id: "moonshot", + display_name: "Moonshot AI pay-as-you-go", + description: Some("Kimi models on the Moonshot AI platform (OpenAI-compatible)"), + color: "#2563eb", + icon: "bi-moon-stars", + lists_models: true, + fields: &[ + ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, + ], + } + } +} + +#[async_trait::async_trait] +impl ApiProvider for MoonshotCodeProvider { + fn type_id(&self) -> &'static str { "moonshot_code" } + fn display_name(&self) -> &'static str { "Moonshot AI Kimi Code" } + fn supported_types(&self) -> &'static [ServiceType] { + &[ServiceType::Llm] + } + + async fn list_llm_models(&self, record: &LlmProviderRecord) -> Result>> { + let mut models = list_models(&self.http, Self::BASE_URL, record, "Kimi Code").await?; + for m in &mut models { + Self::enrich(m); + } + Ok(Some(models)) + } + + fn reasoning_mode(&self, model_id: &str, _capabilities: &[String]) -> Option { + let id = model_id.to_lowercase(); + // K3 exposes a graded `reasoning_effort` (low/high/max; default max). + // "disabled" turns thinking off — the API then routes to K2.6. + // kimi-for-coding (K2.7 Code) always thinks and has no knob. + if id.starts_with("k3") { + Some(ReasoningMode::ValueSet { + values: ["disabled", "low", "high", "max"] + .iter().map(|s| s.to_string()).collect(), + default: Some("max".to_string()), + }) + } else { + None + } + } + + fn reasoning_request(&self, value: &serde_json::Value) -> Option { + // The Kimi Code API accepts a flat `reasoning_effort`: "none" disables + // thinking, low/high/max select the effort (unknown values → HTTP 400). + match value.as_str()? { + "disabled" => Some(serde_json::json!({ "reasoning_effort": "none" })), + effort => Some(serde_json::json!({ "reasoning_effort": effort })), + } + } + + fn build_llm(&self, record: &LlmProviderRecord, model: &LlmModelRecord) -> Option> { + Some(build_openai_llm(self, Self::BASE_URL, record, model, false)) + } + + fn ui_meta(&self) -> ProviderUiMeta { + ProviderUiMeta { + type_id: "moonshot_code", + display_name: "Moonshot AI Kimi Code", + description: Some("Kimi Code subscription models — k3 / kimi-for-coding (OpenAI-compatible)"), + color: "#000000", + icon: "bi-code-slash", + lists_models: true, + fields: &[ + ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, + ], + } + } +} diff --git a/crates/skald-core/src/llm/providers/ollama.rs b/crates/skald-core/src/llm/providers/ollama.rs index ffdfdd1..a80f704 100644 --- a/crates/skald-core/src/llm/providers/ollama.rs +++ b/crates/skald-core/src/llm/providers/ollama.rs @@ -115,6 +115,7 @@ impl ApiProvider for OllamaProvider { description: Some("Local models via Ollama"), color: "#f97316", icon: "bi-terminal", + lists_models: true, fields: &[ ProviderField { key: "base_url", label: "Base URL", required: false, secret: false }, ], diff --git a/crates/skald-core/src/llm/providers/openai.rs b/crates/skald-core/src/llm/providers/openai.rs index 924a2f3..d060a87 100644 --- a/crates/skald-core/src/llm/providers/openai.rs +++ b/crates/skald-core/src/llm/providers/openai.rs @@ -2,9 +2,8 @@ use std::sync::Arc; use anyhow::{Context, Result}; -use crate::chatbot::openai::OpenAiClient; use crate::llm::{LlmModelRecord, LlmProviderRecord}; -use crate::llm::providers::{RemoteLlmModelInfo, extra_with_reasoning}; +use crate::llm::providers::{RemoteLlmModelInfo, build_openai_llm}; use crate::transcribe::TranscribeModelRecord; use crate::transcribe::openai_audio::OpenAiAudioTranscriber; use crate::tts::TtsModelRecord; @@ -47,15 +46,7 @@ impl ApiProvider for OpenAiProvider { } fn build_llm(&self, record: &LlmProviderRecord, model: &LlmModelRecord) -> Option> { - Some((|| { - let key = record.api_key.as_deref() - .with_context(|| format!("provider '{}': api_key required for open_ai", record.name))?; - let extra = extra_with_reasoning(self, model); - Ok(BuiltLlmClient { - client: Arc::new(OpenAiClient::new("https://api.openai.com/v1", key, extra, false)), - prompt_cache: false, - }) - })()) + Some(build_openai_llm(self, "https://api.openai.com/v1", record, model, false)) } fn build_tts(&self, record: &LlmProviderRecord, model: &TtsModelRecord) -> Option>> { @@ -90,6 +81,7 @@ impl ApiProvider for OpenAiProvider { description: None, color: "#10a37f", icon: "bi-lightning-charge", + lists_models: false, fields: &[ ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, ProviderField { key: "base_url", label: "Base URL (optional)", required: false, secret: false }, diff --git a/crates/skald-core/src/llm/providers/openrouter.rs b/crates/skald-core/src/llm/providers/openrouter.rs index b6a5349..a3aae5e 100644 --- a/crates/skald-core/src/llm/providers/openrouter.rs +++ b/crates/skald-core/src/llm/providers/openrouter.rs @@ -2,11 +2,10 @@ use std::sync::Arc; use anyhow::{Context, Result, anyhow}; -use crate::chatbot::openai::OpenAiClient; use crate::image_generate::ImageGenerateModelRecord; use crate::image_generate::openrouter_image::OpenRouterImageGenerator; use crate::llm::{LlmModelRecord, LlmProviderRecord}; -use crate::llm::providers::{RemoteLlmModelInfo, extra_with_reasoning}; +use crate::llm::providers::{RemoteLlmModelInfo, build_openai_llm, fetch_openai_models}; use crate::transcribe::TranscribeModelRecord; use crate::transcribe::openai_audio::OpenAiAudioTranscriber; use crate::tts::TtsModelRecord; @@ -48,19 +47,9 @@ impl OpenRouterProvider { } async fn fetch_catalog(&self, api_key: &str) -> Result> { - let resp: serde_json::Value = self.http - .get("https://openrouter.ai/api/v1/models") - .bearer_auth(api_key) - .send() - .await - .map_err(|e| anyhow!("OpenRouter request failed: {e}"))? - .json() - .await - .map_err(|e| anyhow!("OpenRouter response parse failed: {e}"))?; + let raw = fetch_openai_models(&self.http, "https://openrouter.ai/api/v1", Some(api_key), "OpenRouter").await?; - let models = resp["data"] - .as_array() - .ok_or_else(|| anyhow!("unexpected OpenRouter response shape"))? + let models = raw .iter() .filter_map(|m| { let id = m["id"].as_str()?.to_string(); @@ -150,17 +139,9 @@ impl ApiProvider for OpenRouterProvider { } fn build_llm(&self, record: &LlmProviderRecord, model: &LlmModelRecord) -> Option> { - Some((|| { - let key = record.api_key.as_deref() - .with_context(|| format!("provider '{}': api_key required for openrouter", record.name))?; - // Anthropic prompt-caching only works for models served by Anthropic on OpenRouter. - let prompt_cache = model.model_id.starts_with("anthropic/"); - let extra = extra_with_reasoning(self, model); - Ok(BuiltLlmClient { - client: Arc::new(OpenAiClient::new("https://openrouter.ai/api/v1", key, extra, prompt_cache)), - prompt_cache, - }) - })()) + // Anthropic prompt-caching only works for models served by Anthropic on OpenRouter. + let prompt_cache = model.model_id.starts_with("anthropic/"); + Some(build_openai_llm(self, "https://openrouter.ai/api/v1", record, model, prompt_cache)) } fn build_tts(&self, record: &LlmProviderRecord, model: &TtsModelRecord) -> Option>> { @@ -207,6 +188,7 @@ impl ApiProvider for OpenRouterProvider { description: None, color: "#8b5cf6", icon: "bi-hdd-stack", + lists_models: true, fields: &[ ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, ], diff --git a/crates/skald-core/src/llm/providers/zai.rs b/crates/skald-core/src/llm/providers/zai.rs index f67178f..db72acf 100644 --- a/crates/skald-core/src/llm/providers/zai.rs +++ b/crates/skald-core/src/llm/providers/zai.rs @@ -1,10 +1,7 @@ -use std::sync::Arc; +use anyhow::Result; -use anyhow::{Context, Result}; - -use crate::chatbot::openai::OpenAiClient; use crate::llm::{LlmModelRecord, LlmProviderRecord}; -use crate::llm::providers::{RemoteLlmModelInfo, extra_with_reasoning}; +use crate::llm::providers::{RemoteLlmModelInfo, build_openai_llm}; use crate::provider::{ApiProvider, BuiltLlmClient, ProviderField, ProviderUiMeta, ReasoningMode, ServiceType}; /// Z.AI (Zhipu AI) — OpenAI-compatible GLM API. @@ -119,15 +116,7 @@ impl ApiProvider for ZaiProvider { } fn build_llm(&self, record: &LlmProviderRecord, model: &LlmModelRecord) -> Option> { - Some((|| { - let key = record.api_key.as_deref() - .with_context(|| format!("provider '{}': api_key required for zai", record.name))?; - let extra = extra_with_reasoning(self, model); - Ok(BuiltLlmClient { - client: Arc::new(OpenAiClient::new(Self::BASE_URL, key, extra, false)), - prompt_cache: false, - }) - })()) + Some(build_openai_llm(self, Self::BASE_URL, record, model, false)) } fn ui_meta(&self) -> ProviderUiMeta { @@ -137,6 +126,7 @@ impl ApiProvider for ZaiProvider { description: Some("Zhipu AI GLM models (OpenAI-compatible)"), color: "#4f46e5", icon: "bi-stars", + lists_models: true, fields: &[ ProviderField { key: "api_key", label: "API Key", required: true, secret: true }, ], diff --git a/crates/skald-core/src/mcp/mod.rs b/crates/skald-core/src/mcp/mod.rs index f4b859d..e69f9fc 100644 --- a/crates/skald-core/src/mcp/mod.rs +++ b/crates/skald-core/src/mcp/mod.rs @@ -399,46 +399,89 @@ fn transport_of(s: &str) -> McpTransport { } } -/// Some remote MCP servers take their key as a **query parameter** rather than the -/// `Authorization: Bearer` header this client sends by default (Tavily wants -/// `?tavilyApiKey=…`). Those declare a `{key}` placeholder in their URL, which is -/// substituted here — at connect time, in memory. +/// Some remote MCP servers take their credentials as **query parameters** rather +/// than the `Authorization: Bearer` header this client sends by default (Tavily +/// wants `?tavilyApiKey=…`). Those declare placeholders in their URL, substituted +/// here — at connect time, in memory. /// -/// Doing it here rather than at write time keeps the key in its own column (where -/// it is redacted and, for a per-user connector, encrypted with the rest of -/// `{userid}.db`) instead of baking a live secret into a stored URL. Once -/// substituted, the key is cleared so it is not also sent as a bearer header the -/// server never asked for. +/// Three placeholder forms are understood: +/// - `{key}` — the legacy single-key form: replaced with `api_key`. +/// - `{SECRET:name}` / `{ENV:name}` — the **named** form: replaced with the +/// connector's own form field `env[name]`. This is what lets a remote connector's +/// URL carry more than one credential (`?key={SECRET:apiKey}®ion={ENV:region}`), +/// each supplied by a described `env[]` entry. +/// - `{SECRET:name}` with no matching `env[name]` — the pre-schema form (a row +/// written before the feed moved a connector's key into `env[]`): falls back to +/// `api_key`, so an old row still connects. +/// +/// Resolving here rather than at write time keeps a per-user secret in the encrypted +/// `{userid}.db` (`env_json` / the api_key column) instead of baking a live secret +/// into a stored URL. `api_key` is cleared once it has been spent on the URL so it is +/// not also sent as a bearer header the server never asked for. fn apply_key_placeholder( url: Option, api_key: Option, + env: &HashMap, ) -> (Option, Option) { - match (url, api_key) { - (Some(u), Some(k)) if u.contains("{key}") => (Some(u.replace("{key}", &k)), None), - // Unified {SECRET:} placeholder (e.g. Tavily's - // `?tavilyApiKey={SECRET:tavilyApiKey}`). Any SECRET token in a URL is - // the api_key for a remote connector — a URL never carries the user's - // other secrets — so we substitute every occurrence. - (Some(u), Some(k)) if u.contains("{SECRET:") => (Some(substitute_secret_tokens(&u, &k)), None), - (u, k) => (u, k), - } + let url = match url { + Some(u) => u, + None => return (None, api_key), + }; + + let mut api_key_spent = false; + + // Legacy single-key placeholder first (no name to resolve). + let url = if url.contains("{key}") { + match &api_key { + Some(k) => { api_key_spent = true; url.replace("{key}", k) } + None => url, + } + } else { + url + }; + + let url = substitute_named_tokens(&url, env, api_key.as_deref(), &mut api_key_spent); + let api_key = if api_key_spent { None } else { api_key }; + (Some(url), api_key) } -/// Replaces every `{SECRET:…}` token in `text` with `value`. Used for the -/// api-key-in-URL case; other placeholders are left untouched. -fn substitute_secret_tokens(text: &str, value: &str) -> String { +/// Replaces `{SECRET:name}` / `{ENV:name}` tokens in `text`, each looked up by name +/// in `env`. A `{SECRET:name}` with no matching `env` entry falls back to `api_key` +/// (setting `api_key_spent`) — the pre-schema single-key form. A token that resolves +/// to nothing is left in place, so a misconfiguration is a visible, debuggable URL +/// rather than a silently-wrong one. Non-`SECRET:`/`ENV:` braces (e.g. a stray +/// `{key}` when no api_key was set) are left untouched. +fn substitute_named_tokens( + text: &str, + env: &HashMap, + api_key: Option<&str>, + api_key_spent: &mut bool, +) -> String { let mut out = String::with_capacity(text.len()); let mut rest = text; - while let Some(open) = rest.find("{SECRET:") { + while let Some(open) = rest.find('{') { out.push_str(&rest[..open]); let after = &rest[open..]; - if let Some(close) = after.find('}') { - out.push_str(value); - rest = &after[close + 1..]; - } else { + let Some(close) = after.find('}') else { out.push_str(after); - break; + return out; + }; + let token = &after[1..close]; + let resolved = if let Some(name) = token.strip_prefix("SECRET:") { + match env.get(name) { + Some(v) => Some(v.clone()), + None => api_key.map(|k| { *api_key_spent = true; k.to_string() }), + } + } else if let Some(name) = token.strip_prefix("ENV:") { + env.get(name).cloned() + } else { + None + }; + match resolved { + Some(v) => out.push_str(&v), + None => out.push_str(&after[..=close]), // leave the `{…}` literally } + rest = &after[close + 1..]; } out.push_str(rest); out @@ -447,14 +490,15 @@ fn substitute_secret_tokens(text: &str, value: &str) -> String { /// Builds a spec for a globally-active connector — host transport (`launch_in` /// = None), so it runs in the Skald process, not in any container (§7). pub fn global_row_spec(row: &crate::db::mcp_global_servers::McpGlobalServerRow) -> McpServerSpec { - let (url, api_key) = apply_key_placeholder(row.url.clone(), row.api_key.clone()); + let env = row.env(); + let (url, api_key) = apply_key_placeholder(row.url.clone(), row.api_key.clone(), &env); McpServerSpec { config: McpServerConfig { name: row.name.clone(), transport: transport_of(&row.transport), command: row.command.clone(), args: Some(row.args()).filter(|v| !v.is_empty()), - env: Some(row.env()).filter(|m| !m.is_empty()), + env: Some(env).filter(|m| !m.is_empty()), url, api_key, launch_in: None, @@ -473,14 +517,15 @@ pub fn user_row_spec( ) -> McpServerSpec { let transport = transport_of(&row.transport); let launch_in = matches!(transport, McpTransport::Stdio).then(|| container.to_string()); - let (url, api_key) = apply_key_placeholder(row.url.clone(), row.api_key.clone()); + let env = row.env(); + let (url, api_key) = apply_key_placeholder(row.url.clone(), row.api_key.clone(), &env); McpServerSpec { config: McpServerConfig { name: row.name.clone(), transport, command: row.command.clone(), args: Some(row.args()).filter(|v| !v.is_empty()), - env: Some(row.env()).filter(|m| !m.is_empty()), + env: Some(env).filter(|m| !m.is_empty()), url, api_key, launch_in, @@ -591,25 +636,71 @@ pub fn content_type_for_ext(ext: &str) -> &'static str { mod tests { use super::*; + fn map(pairs: &[(&str, &str)]) -> HashMap { + pairs.iter().map(|(k, v)| (k.to_string(), v.to_string())).collect() + } + #[test] fn key_placeholder_legacy_is_substituted() { let (url, key) = apply_key_placeholder( Some("https://x/?k={key}".into()), Some("secret123".into()), + &HashMap::new(), ); assert_eq!(url.as_deref(), Some("https://x/?k=secret123")); assert!(key.is_none(), "api_key is consumed after substitution"); } #[test] - fn key_placeholder_secret_token_is_substituted() { - // Tavily's unified form: the URL carries {SECRET:tavilyApiKey}. + fn key_placeholder_secret_token_falls_back_to_api_key() { + // Pre-schema Tavily: the key sits in the api_key column and the URL names + // {SECRET:tavilyApiKey}, but env carries no such entry → fall back to api_key. let (url, key) = apply_key_placeholder( Some("https://mcp.tavily.com/mcp/?tavilyApiKey={SECRET:tavilyApiKey}".into()), Some("tvly-abc".into()), + &HashMap::new(), ); assert_eq!(url.as_deref(), Some("https://mcp.tavily.com/mcp/?tavilyApiKey=tvly-abc")); - assert!(key.is_none(), "api_key is consumed when the URL had a SECRET token"); + assert!(key.is_none(), "api_key is consumed when it was spent on the URL"); + } + + #[test] + fn secret_token_resolves_from_env_by_name() { + // New schema-driven Tavily: the key was typed into the described `env[]` + // field `tavilyApiKey`, so it lives in env and there is no api_key column. + let (url, key) = apply_key_placeholder( + Some("https://mcp.tavily.com/mcp/?tavilyApiKey={SECRET:tavilyApiKey}".into()), + None, + &map(&[("tavilyApiKey", "tvly-xyz")]), + ); + assert_eq!(url.as_deref(), Some("https://mcp.tavily.com/mcp/?tavilyApiKey=tvly-xyz")); + assert!(key.is_none()); + } + + #[test] + fn multiple_named_params_resolve_independently() { + // The point of the named form: more than one credential in one URL, each + // from its own form field (SECRET for secrets, ENV for the rest). + let (url, key) = apply_key_placeholder( + Some("https://api/mcp?key={SECRET:apiKey}®ion={ENV:region}".into()), + None, + &map(&[("apiKey", "sk-1"), ("region", "us-east-1")]), + ); + assert_eq!(url.as_deref(), Some("https://api/mcp?key=sk-1®ion=us-east-1")); + assert!(key.is_none()); + } + + #[test] + fn unresolved_token_is_left_in_place() { + // No env entry and no api_key to fall back to → the token stays, a visible + // misconfiguration rather than a silently-wrong URL. + let (url, key) = apply_key_placeholder( + Some("https://api/mcp?key={SECRET:missing}".into()), + None, + &HashMap::new(), + ); + assert_eq!(url.as_deref(), Some("https://api/mcp?key={SECRET:missing}")); + assert!(key.is_none()); } #[test] @@ -619,17 +710,24 @@ mod tests { let (url, key) = apply_key_placeholder( Some("https://x.example.com/mcp".into()), Some("bearer-key".into()), + &HashMap::new(), ); assert_eq!(url.as_deref(), Some("https://x.example.com/mcp")); assert_eq!(key.as_deref(), Some("bearer-key")); } #[test] - fn substitute_secret_tokens_replaces_every_occurrence() { - let s = substitute_secret_tokens( + fn substitute_named_tokens_prefers_env_over_api_key() { + // A SECRET token whose name IS in env resolves from env; the api_key is left + // untouched (it may still be needed as a bearer for the same connector). + let mut spent = false; + let s = substitute_named_tokens( "a={SECRET:K}&b={SECRET:K}&c={ENV:C}", - "VAL", + &map(&[("K", "VAL"), ("C", "CC")]), + Some("bearer"), + &mut spent, ); - assert_eq!(s, "a=VAL&b=VAL&c={ENV:C}"); + assert_eq!(s, "a=VAL&b=VAL&c=CC"); + assert!(!spent, "env satisfied the tokens, so api_key was not spent"); } } diff --git a/crates/skald-core/src/skald/bundles.rs b/crates/skald-core/src/skald/bundles.rs index 9975ed6..e76f8c7 100644 --- a/crates/skald-core/src/skald/bundles.rs +++ b/crates/skald-core/src/skald/bundles.rs @@ -69,6 +69,8 @@ impl Models { provider_registry.register_builtin(crate::llm::providers::lm_studio::LmStudioProvider::new()); provider_registry.register_builtin(crate::llm::providers::deepseek::DeepSeekProvider::new()); provider_registry.register_builtin(crate::llm::providers::zai::ZaiProvider::new()); + provider_registry.register_builtin(crate::llm::providers::moonshot::MoonshotProvider::new()); + provider_registry.register_builtin(crate::llm::providers::moonshot::MoonshotCodeProvider::new()); let provider_registry = Arc::new(provider_registry); info!("provider registry ready ({} built-in providers)", provider_registry.all().len()); diff --git a/web/components/connector-detail.js b/web/components/connector-detail.js index a20eb8d..242222a 100644 --- a/web/components/connector-detail.js +++ b/web/components/connector-detail.js @@ -422,6 +422,11 @@ export class ConnectorDetailPage extends LightElement { const canManage = this._isGlobal ? this._isAdmin : true; const hasVerify = !!e.verify_command; const oauth = e.auth_kind === 'oauth'; + // An api_key connector that declares its key as a described `env[]` field (secret) + // collects it there — the generic, label-less "API key" box would be a duplicate + // asking for the same value. Fall back to the generic box only when the schema + // names no secret of its own (a bare `requires:[API_KEY]` connector). + const schemaHasSecret = this._schema.some(f => f.secret); if (this._isGlobal && !this._isAdmin) return nothing; @@ -452,7 +457,7 @@ export class ConnectorDetailPage extends LightElement { : 'Already active. Re-submitting replaces the stored credentials.'} ` : nothing} - ${e.auth_kind === 'api_key' ? html` + ${e.auth_kind === 'api_key' && !schemaHasSecret ? html`
t.type_id === provider.type); - if (hasModelPicker) { + if (typeMeta?.lists_models) { this._orForm = emptyOrForm(); this._orSearch = ''; this._orModels = [];