diff --git a/apps/android/app/src/main/java/com/codedeck/plus/ui/screens/ProvidersScreen.kt b/apps/android/app/src/main/java/com/codedeck/plus/ui/screens/ProvidersScreen.kt index 1e0b2e1e..331e2372 100644 --- a/apps/android/app/src/main/java/com/codedeck/plus/ui/screens/ProvidersScreen.kt +++ b/apps/android/app/src/main/java/com/codedeck/plus/ui/screens/ProvidersScreen.kt @@ -1,8 +1,10 @@ package com.codedeck.plus.ui.screens import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.padding import androidx.compose.material.icons.Icons import androidx.compose.material.icons.outlined.Close import androidx.compose.material3.Checkbox @@ -17,10 +19,13 @@ import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.input.PasswordVisualTransformation +import androidx.compose.ui.text.style.TextOverflow import com.codedeck.plus.core.CoreHost import com.codedeck.plus.ui.components.Chip import com.codedeck.plus.ui.components.ErrorNote +import com.codedeck.plus.ui.components.ExpandableRow import com.codedeck.plus.ui.components.Field import com.codedeck.plus.ui.components.Group import com.codedeck.plus.ui.components.GroupBody @@ -114,6 +119,8 @@ fun ProvidersContent( onBack: () -> Unit, /** Opens straight on the editor (for a snapshot). */ startEditing: ProviderEditor? = null, + /** The provider shown opened (for a snapshot). */ + openProfile: String? = null, /** The core's base URL rule — a native call, so a snapshot, which cannot * load the core, passes its own. */ validBaseUrl: (String) -> Boolean = ::isValidProviderBaseUrl, @@ -187,8 +194,7 @@ fun ProvidersContent( Group(footer = agent?.let { providerUse(it) + " Tokens stay on the machine; this phone never stores them." }) { own.forEachIndexed { i, p -> if (i > 0) Divider() - GroupBody { - ProfileSummary(p) + ProfileRow(p, startOpen = p.id == openProfile) { if (confirmDelete == p.id) { Text( if (agent?.supportsProviderModels == true) { @@ -229,8 +235,7 @@ fun ProvidersContent( ) { unassigned.forEachIndexed { i, p -> if (i > 0) Divider() - GroupBody { - ProfileSummary(p) + ProfileRow(p, startOpen = p.id == openProfile) { Row(horizontalArrangement = Arrangement.spacedBy(Tokens.Space1)) { QuietButton("Use for $agentName", onClick = { write(p.id, rewrite(p, agentId)) }, enabled = !busy && p.hasToken) QuietButton("Delete", onClick = { write(p.id, null) }, danger = true, enabled = !busy) @@ -244,24 +249,92 @@ fun ProvidersContent( } } +/** "3 models", "1 model". */ +private fun modelCount(n: Int) = "$n ${if (n == 1) "model" else "models"}" + +/** + * One provider: its name and endpoint, with a chip for what matters most — + * why its agent does not offer it, a missing token, or how many models it + * has. Opened, it lists its models and then [actions]. + */ @Composable -private fun ProfileSummary(p: UniffiProviderProfileInfo) { - Row(Modifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(Tokens.Space2)) { - Text(p.label, color = Tokens.Text, fontSize = Tokens.TextMd, modifier = Modifier.weight(1f)) - Chip( - if (p.hasToken) "token set" else "no token", - color = if (p.hasToken) Tokens.Success else Tokens.TextDim, - border = if (p.hasToken) Tokens.Success.copy(alpha = 0.4f) else Tokens.Border, - ) +private fun ProfileRow(p: UniffiProviderProfileInfo, startOpen: Boolean, actions: @Composable () -> Unit) { + var open by remember(p.id) { mutableStateOf(startOpen) } + ExpandableRow( + p.label, + p.baseUrl, + enabled = p.error == null, + open = open, + onOpenChange = { open = it }, + subtitleMono = true, + openSubtitleLines = 3, + trailing = { + when { + p.error != null -> Chip("not offered", color = Tokens.Danger, border = Tokens.Danger.copy(alpha = 0.4f)) + !p.hasToken -> Chip("no token", color = Tokens.Warn, border = Tokens.Warn.copy(alpha = 0.4f)) + else -> Chip(modelCount(p.models.size)) + } + }, + ) { + Column(Modifier.padding(top = Tokens.Space2), verticalArrangement = Arrangement.spacedBy(Tokens.Space3)) { + // Why the agent does not offer its models (its name is taken, say). + p.error?.let { Text(it, color = Tokens.Danger, fontSize = Tokens.TextSm) } + Text( + (if (p.modelsFromProvider) "Models read from the provider at the last save" else "Models typed in by hand") + + if (p.hasToken) " · token set" else " · no token, so nothing runs on it", + color = Tokens.TextMuted, + fontSize = Tokens.TextSm, + ) + ProfileModels(p) + actions() + } + } +} + +/** + * A profile's models, under the provider a gateway routes each to when the + * endpoint names one (the part of the model's group after the profile's + * name); its default marked. + */ +@Composable +private fun ProfileModels(p: UniffiProviderProfileInfo) { + // Those under no other provider first, so none reads as part of a group. + val byUpstream = p.models + .groupBy { m -> m.provider?.removePrefix(p.label)?.removePrefix(" · ")?.takeIf { it.isNotEmpty() } } + .entries + .sortedBy { it.key != null } + val default = p.defaultModel ?: p.models.firstOrNull()?.id + Column(verticalArrangement = Arrangement.spacedBy(Tokens.Space2)) { + byUpstream.forEach { (upstream, models) -> + if (upstream != null) Text(upstream, color = Tokens.TextMuted, fontSize = Tokens.TextXs, fontWeight = FontWeight.Medium) + models.forEach { m -> + Row(Modifier.fillMaxWidth(), verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(Tokens.Space2)) { + Text( + m.label ?: m.id, + color = Tokens.Text, + fontSize = Tokens.TextMd, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.weight(1f), + ) + if (m.id == default) Chip("default") + m.contextWindow?.let { Text(contextSize(it), color = Tokens.TextDim, fontSize = Tokens.TextXs, fontFamily = Tokens.FontMono) } + } + } + } + } +} + +/** A context window as people write it: 1M, 200K, 128K (2^17 tokens). */ +internal fun contextSize(tokens: UInt): String { + val n = tokens.toLong() + fun scaled(unit: Long, binary: Long, suffix: String): String = + "${if (n % binary == 0L) n / binary else Math.round(n.toDouble() / unit)}$suffix" + return when { + n >= 1_000_000L -> scaled(1_000_000L, 1L shl 20, "M") + n >= 1_000L -> scaled(1_000L, 1L shl 10, "K") + else -> n.toString() } - Text(p.baseUrl, color = Tokens.TextMuted, fontSize = Tokens.TextSm, fontFamily = Tokens.FontMono) - Text( - "${p.models.size} ${if (p.models.size == 1) "model" else "models"}" + - (if (p.modelsFromProvider) " from the provider" else "") + - (p.defaultModel?.let { ", default $it" } ?: ""), - color = Tokens.TextMuted, - fontSize = Tokens.TextSm, - ) } /** The last save's outcome, in words. */ diff --git a/apps/android/app/src/main/java/uniffi/client_ffi/client_ffi.kt b/apps/android/app/src/main/java/uniffi/client_ffi/client_ffi.kt index 7897d0b9..a5ac35d5 100644 --- a/apps/android/app/src/main/java/uniffi/client_ffi/client_ffi.kt +++ b/apps/android/app/src/main/java/uniffi/client_ffi/client_ffi.kt @@ -6114,6 +6114,11 @@ data class UniffiModelEntry ( * Who serves the model, when the agent says. */ var `provider`: kotlin.String? + , + /** + * How many tokens the model takes in, when known. + */ + var `contextWindow`: kotlin.UInt? ){ @@ -6133,19 +6138,22 @@ public object FfiConverterTypeUniffiModelEntry: FfiConverterRustBuffer Unit> = linkedMapOf( }, "plugins_opencode" to { PluginsContent(workstation, "opencode", dispatch = {}, onBack = {}) }, "providers_opencode" to { ProvidersContent(workstation, "opencode", status = null, dispatch = {}, onBack = {}) }, + "providers_opencode_open" to { ProvidersContent(workstation, "opencode", status = null, dispatch = {}, onBack = {}, openProfile = "home-gateway") }, "providers_claude_empty" to { ProvidersContent(workstation, "claude-code", status = null, dispatch = {}, onBack = {}) }, "providers_edit" to { ProvidersContent(workstation, "opencode", status = null, dispatch = {}, onBack = {}, startEditing = ProviderEditor.Existing("home-gateway"), validBaseUrl = { true }) @@ -290,6 +291,7 @@ class DesignSnapshotTest { @Test fun mcp_add() = paparazzi.page("mcp_add") @Test fun mcp_import() = paparazzi.page("mcp_import") @Test fun providers_opencode() = paparazzi.page("providers_opencode") + @Test fun providers_opencode_open() = paparazzi.page("providers_opencode_open") @Test fun providers_claude_empty() = paparazzi.page("providers_claude_empty") @Test fun providers_edit() = paparazzi.page("providers_edit") @Test fun providers_add() = paparazzi.page("providers_add") diff --git a/apps/android/app/src/test/java/com/codedeck/plus/ui/components/ModelPickerOptionsTest.kt b/apps/android/app/src/test/java/com/codedeck/plus/ui/components/ModelPickerOptionsTest.kt index 576941ae..63bb178f 100644 --- a/apps/android/app/src/test/java/com/codedeck/plus/ui/components/ModelPickerOptionsTest.kt +++ b/apps/android/app/src/test/java/com/codedeck/plus/ui/components/ModelPickerOptionsTest.kt @@ -9,9 +9,9 @@ class ModelPickerOptionsTest { fun `models are grouped by provider, in the order providers first appear`() { val options = modelPickerOptions( listOf( - UniffiModelEntry("OpenCode Go/glm-5.3-flash", "glm-5.3-flash", "OpenCode Go"), - UniffiModelEntry("Z.ai/glm-5.3-flash", "glm-5.3-flash", "Z.ai"), - UniffiModelEntry("OpenCode Go/minimax-m3", null, "OpenCode Go"), + UniffiModelEntry("OpenCode Go/glm-5.3-flash", "glm-5.3-flash", "OpenCode Go", null), + UniffiModelEntry("Z.ai/glm-5.3-flash", "glm-5.3-flash", "Z.ai", null), + UniffiModelEntry("OpenCode Go/minimax-m3", null, "OpenCode Go", null), ), ) assertEquals( diff --git a/apps/android/app/src/test/java/com/codedeck/plus/ui/screens/ProvidersTest.kt b/apps/android/app/src/test/java/com/codedeck/plus/ui/screens/ProvidersTest.kt new file mode 100644 index 00000000..633644eb --- /dev/null +++ b/apps/android/app/src/test/java/com/codedeck/plus/ui/screens/ProvidersTest.kt @@ -0,0 +1,16 @@ +package com.codedeck.plus.ui.screens + +import org.junit.Assert.assertEquals +import org.junit.Test + +class ProvidersTest { + @Test + fun a_context_window_reads_as_people_write_it() { + assertEquals("1M", contextSize(1_000_000u)) + assertEquals("1M", contextSize(1_048_576u)) + assertEquals("200K", contextSize(200_000u)) + assertEquals("128K", contextSize(131_072u)) + assertEquals("33K", contextSize(32_768u + 1u)) + assertEquals("512", contextSize(512u)) + } +} diff --git a/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_claude_empty.png b/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_claude_empty.png index 86036af5..ad77ccfe 100644 Binary files a/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_claude_empty.png and b/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_claude_empty.png differ diff --git a/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_opencode.png b/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_opencode.png index b1c5ff50..ad68dd19 100644 Binary files a/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_opencode.png and b/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_opencode.png differ diff --git a/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_opencode_open.png b/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_opencode_open.png new file mode 100644 index 00000000..c0ca4b13 Binary files /dev/null and b/apps/android/app/src/test/snapshots/images/com.codedeck.plus.ui_DesignSnapshotTest_providers_opencode_open.png differ diff --git a/crates/agent-protocol/src/codec.rs b/crates/agent-protocol/src/codec.rs index cb043e4b..f970703f 100644 --- a/crates/agent-protocol/src/codec.rs +++ b/crates/agent-protocol/src/codec.rs @@ -96,7 +96,8 @@ impl HostMessage { | Self::McpServers { .. } | Self::SessionMcp { .. } | Self::CredentialChecked { .. } - | Self::ProviderModels { .. } => true, + | Self::ProviderModels { .. } + | Self::ProvidersSet { .. } => true, Self::SessionEvent { .. } | Self::RequestPermission(_) | Self::AskQuestion(_) diff --git a/crates/agent-protocol/src/messages.rs b/crates/agent-protocol/src/messages.rs index e2db4d1c..6f26d665 100644 --- a/crates/agent-protocol/src/messages.rs +++ b/crates/agent-protocol/src/messages.rs @@ -130,6 +130,14 @@ pub struct ProviderBinding { pub default_model: Option, } +/// A provider profile an agent left out of `set-providers`. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, specta::Type)] +pub struct RefusedProvider { + /// The profile's id. + pub id: String, + pub reason: String, +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, specta::Type)] #[serde(rename_all = "camelCase")] pub struct StartSession { @@ -415,9 +423,11 @@ pub enum BridgeMessage { auth_token: Secret, }, /// The provider profiles of an agent whose catalog entry `supports` - /// `providerModels`, all of them: sent after `initialize` and whenever - /// one changes. The agent offers their models beside its own. Reply: - /// `ack`. + /// `providerModels`, all of them, oldest saved first: sent after + /// `initialize` and whenever one changes. The agent offers their models + /// beside its own. One it cannot add (its name is taken by one of the + /// agent's own providers, or by an earlier profile) is left out. Reply: + /// `providers-set`. SetProviders { agent: String, providers: Vec, @@ -494,6 +504,9 @@ pub enum HostMessage { }, /// Reply to `list-provider-models`: never empty. ProviderModels { models: Vec }, + /// Reply to `set-providers`: the profiles left out, with the reason in + /// words for a person; every other one is offered. + ProvidersSet { refused: Vec }, /// A notification: no frame id, no reply. SessionEvent { session_id: String, diff --git a/crates/bridge-core/src/engine/host.rs b/crates/bridge-core/src/engine/host.rs index 79b57e9d..3e39d5d7 100644 --- a/crates/bridge-core/src/engine/host.rs +++ b/crates/bridge-core/src/engine/host.rs @@ -59,7 +59,8 @@ pub(crate) enum HostCall { CheckProvider { ticket: u64 }, /// A provider's model list, for the save waiting on it. ListProviderModels { ticket: u64 }, - SetProviders { agent: String }, + /// An agent's profiles, for the save waiting on them when there is one. + SetProviders { agent: String, save: Option }, DeleteConversation(ConversationDelete), } @@ -254,10 +255,13 @@ impl Engine { }; self.on_provider_models_fetched(ticket, models); } - HostCall::SetProviders { agent } => { - if let Err(err) = result { - log::warn!("[Engine] {agent} did not take its provider profiles: {err}"); - } + HostCall::SetProviders { agent, save } => { + let refused = match result { + Ok(HostMessage::ProvidersSet { refused }) => Ok(refused), + Ok(_) => Err("the agent host answered with the wrong reply".to_string()), + Err(err) => Err(err), + }; + self.on_providers_set(&agent, save, refused); } HostCall::DeleteConversation(delete) => match result { Ok(_) => log::info!("[Engine] Conversation {} of {} deleted", delete.conversation_id, delete.session_id), diff --git a/crates/bridge-core/src/engine/mod.rs b/crates/bridge-core/src/engine/mod.rs index a63a6dbe..9269dd0b 100644 --- a/crates/bridge-core/src/engine/mod.rs +++ b/crates/bridge-core/src/engine/mod.rs @@ -151,6 +151,12 @@ pub struct Engine { /// set-provider-profile waiting on the provider's model list: ticket → /// (phone, the profile to store once its models are in). model_fetches: BTreeMap, + /// set-provider-profile waiting on its agent taking the new list: + /// ticket → the save, undone if the agent leaves the profile out. + profile_saves: BTreeMap, + /// Why an agent leaves a profile out of its models, by profile id, from + /// its last answer to `set-providers`. Not persisted. + profile_refusals: BTreeMap, next_ticket: u64, sync: SyncServer, pairing: Option, @@ -195,6 +201,8 @@ impl Engine { credential_acks: BTreeMap::new(), profile_acks: BTreeMap::new(), model_fetches: BTreeMap::new(), + profile_saves: BTreeMap::new(), + profile_refusals: BTreeMap::new(), next_ticket: 0, sync, pairing: None, diff --git a/crates/bridge-core/src/engine/settings.rs b/crates/bridge-core/src/engine/settings.rs index d9bcae1b..d0349b81 100644 --- a/crates/bridge-core/src/engine/settings.rs +++ b/crates/bridge-core/src/engine/settings.rs @@ -4,7 +4,7 @@ //! starts, never logged and never sent back — a phone learns only whether a //! value is set and whether its provider accepted it. -use agent_protocol::{AgentInfo, BridgeMessage, ProviderBinding, Secret}; +use agent_protocol::{AgentInfo, BridgeMessage, ProviderBinding, RefusedProvider, Secret}; use protocol::commands::{SetCredentialsMsg, SetProviderProfileMsg}; use protocol::common::{is_valid_provider_base_url, CredentialStatus, ProviderModel, PROVIDER_BASE_URL_ERROR}; use protocol::events::{BridgeToPhone, CredentialsAckMsg, ProviderProfileAckMsg, ProviderProfilesMsg}; @@ -14,6 +14,14 @@ use super::{Engine, HostCall}; use crate::io::store_keys; use crate::settings::{ProviderProfile, StoredProfiles, GITHUB_PAT, GITHUB_PAT_LABEL}; +/// A saved provider profile waiting on its agent to take it. +pub(crate) struct ProfileSave { + phone: String, + profile: ProviderProfile, + /// What the profile was before, restored if the agent leaves it out. + previous: Option, +} + /// A `credentials-ack` waiting on credential checks. pub(crate) struct CredentialAck { phone: String, @@ -207,26 +215,25 @@ impl Engine { pub(super) fn provider_profiles_msg(&self) -> BridgeToPhone { BridgeToPhone::ProviderProfiles(ProviderProfilesMsg { machine: self.config.machine.clone(), - profiles: self.profiles.values().map(ProviderProfile::redacted).collect(), + profiles: self.profiles.values().map(|p| p.redacted(self.profile_refusals.get(&p.id).map(String::as_str))).collect(), }) } /// Hand an agent whose catalog entry `supports.provider_models` its /// profiles — every usable one, so a deleted or emptied profile goes - /// away too. - pub(super) fn push_providers(&mut self, agent: &str) { + /// away too — oldest saved first, so of two that clash the older stays. + /// `save` is the save waiting on the answer. Whether it was sent. + pub(super) fn push_providers(&mut self, agent: &str, save: Option) -> bool { if !self.host.initialized || !self.catalog.get(agent).is_some_and(|a| a.supports.provider_models) { - return; + return false; } - let providers: Vec = self - .profiles - .values() - .filter(|p| p.agent == agent && is_valid_provider_base_url(&p.base_url)) - .filter_map(ProviderProfile::binding) - .collect(); + let mut profiles: Vec<&ProviderProfile> = + self.profiles.values().filter(|p| p.agent == agent && is_valid_provider_base_url(&p.base_url)).collect(); + profiles.sort_by(|a, b| (&a.updated_at, &a.id).cmp(&(&b.updated_at, &b.id))); + let providers: Vec = profiles.into_iter().filter_map(ProviderProfile::binding).collect(); log::info!("[Engine] {agent} gets {} provider profile(s)", providers.len()); let message = BridgeMessage::SetProviders { agent: agent.to_string(), providers }; - self.call(HostCall::SetProviders { agent: agent.to_string() }, message); + self.call(HostCall::SetProviders { agent: agent.to_string(), save }, message).is_some() } /// Every agent's profiles, once the host is (back) up. @@ -234,8 +241,59 @@ impl Engine { let agents: Vec = self.catalog.all().iter().filter(|a| a.supports.provider_models).map(|a| a.id.clone()).collect(); for agent in agents { - self.push_providers(&agent); + self.push_providers(&agent, None); + } + } + + /// `agent` answered `set-providers`: it offers every profile but the + /// `refused` ones. A save waiting on the answer whose profile was left + /// out is undone and refused with the agent's reason; any other goes on + /// to its token check. + pub(super) fn on_providers_set(&mut self, agent: &str, save: Option, refused: Result, String>) { + let save = save.and_then(|ticket| self.profile_saves.remove(&ticket)); + let refused = match refused { + Ok(refused) => refused, + Err(err) => { + log::warn!("[Engine] {agent} did not take its provider profiles: {err}"); + if let Some(save) = save { + self.check_saved_profile(&save.phone, &save.profile); + } + return; + } + }; + let before = self.profile_refusals.clone(); + let profiles = &self.profiles; + self.profile_refusals.retain(|id, _| profiles.get(id).is_some_and(|p| p.agent != agent)); + for r in &refused { + log::info!("[Engine] {agent} leaves provider profile '{}' out: {}", r.id, r.reason); + self.profile_refusals.insert(r.id.clone(), r.reason.clone()); + } + let Some(save) = save else { + if self.profile_refusals != before { + let list = self.provider_profiles_msg(); + self.publish_all(list); + } + return; + }; + let id = save.profile.id.clone(); + let Some(reason) = refused.into_iter().find(|r| r.id == id).map(|r| r.reason) else { + return self.check_saved_profile(&save.phone, &save.profile); + }; + let moved_from = save.previous.as_ref().map(|p| p.agent.clone()).filter(|a| a != agent); + let mut next = self.profiles.clone(); + match save.previous { + Some(previous) => next.insert(id.clone(), previous), + None => next.remove(&id), + }; + self.profile_refusals.remove(&id); + if let Err(err) = self.store_profiles(next) { + log::error!("[Engine] Could not undo the save of provider profile '{id}': {err}"); + } + self.push_providers(agent, None); + if let Some(moved_from) = moved_from { + self.push_providers(&moved_from, None); } + self.send_profile_ack(&save.phone, &id, Err(reason)); } /// Create, update or delete one profile. An upsert whose models come @@ -246,8 +304,9 @@ impl Engine { let id = m.profile_id; log::info!("[Engine] set-provider-profile '{id}' from {}...", phone.get(..8).unwrap_or(phone)); // The newest write of a profile is the one that stands: a model list - // still on its way for an older one is dropped. + // or an agent's answer still on its way for an older one is dropped. self.model_fetches.retain(|_, (_, pending)| pending.id != id); + self.profile_saves.retain(|_, pending| pending.profile.id != id); let Some(write) = m.profile else { // Deleting is never refused, whatever the stored URL: a profile // that needs fixing must stay removable. Sessions bound to it @@ -259,9 +318,10 @@ impl Engine { return self.send_profile_ack(phone, &id, Err(err)); } log::info!("[Engine] Provider profile '{id}' deleted (existed={})", removed.is_some()); + self.profile_refusals.remove(&id); self.send_profile_ack(phone, &id, Ok(None)); if let Some(removed) = removed { - self.push_providers(&removed.agent); + self.push_providers(&removed.agent, None); } return; }; @@ -341,8 +401,9 @@ impl Engine { } } - /// Store an upserted profile, hand it to its agent (and take it from the - /// agent it was for before), then have its token checked before acking. + /// Store an upserted profile and hand it to its agent (taking it from + /// the agent it was for before); once that agent has taken it, have its + /// token checked before acking. fn save_profile(&mut self, phone: &str, profile: ProviderProfile) { let id = profile.id.clone(); let mut next = self.profiles.clone(); @@ -359,10 +420,22 @@ impl Engine { if profile.models_from_provider { " from the provider" } else { "" }, profile.auth_token.is_some() ); - self.push_providers(&profile.agent); - if let Some(previous) = previous.filter(|p| p.agent != profile.agent) { - self.push_providers(&previous.agent); + if let Some(moved) = previous.as_ref().filter(|p| p.agent != profile.agent) { + let agent = moved.agent.clone(); + self.push_providers(&agent, None); } + self.next_ticket += 1; + let ticket = self.next_ticket; + if self.push_providers(&profile.agent, Some(ticket)) { + self.profile_saves.insert(ticket, ProfileSave { phone: phone.to_string(), profile, previous }); + } else { + self.check_saved_profile(phone, &profile); + } + } + + /// Have a saved profile's token checked by its agent, then ack. + fn check_saved_profile(&mut self, phone: &str, profile: &ProviderProfile) { + let id = profile.id.clone(); let check = profile.binding().zip(profile.fallback_model().map(str::to_string)); let Some((provider, model)) = check else { return self.send_profile_ack(phone, &id, Ok(None)); diff --git a/crates/bridge-core/src/settings.rs b/crates/bridge-core/src/settings.rs index 7fe36262..1a6b21c0 100644 --- a/crates/bridge-core/src/settings.rs +++ b/crates/bridge-core/src/settings.rs @@ -96,8 +96,9 @@ impl ProviderProfile { }) } - /// The phone's view: whether a token is set, never the token. - pub fn redacted(&self) -> ProviderProfileInfo { + /// The phone's view: whether a token is set, never the token; and why + /// its agent does not offer it, when it does not. + pub fn redacted(&self, error: Option<&str>) -> ProviderProfileInfo { ProviderProfileInfo { id: self.id.clone(), agent: self.agent.clone(), @@ -107,6 +108,7 @@ impl ProviderProfile { models_from_provider: self.models_from_provider, default_model: self.default_model.clone(), has_token: self.auth_token.is_some(), + error: error.map(str::to_string), } } } @@ -161,12 +163,12 @@ mod tests { label: "P".into(), base_url: "https://x".into(), auth_token: Some(Secret::new("t")), - models: vec![ProviderModel { id: "m1".into(), label: None }], + models: vec![ProviderModel { id: "m1".into(), ..Default::default() }], models_from_provider: false, default_model: None, updated_at: None, }; - let info = profile.redacted(); + let info = profile.redacted(None); assert!(info.has_token); assert!(!serde_json::to_string(&info).unwrap().contains("\"t\"")); assert_eq!(profile.fallback_model(), Some("m1")); diff --git a/crates/bridge-core/tests/settings.rs b/crates/bridge-core/tests/settings.rs index 10c95ae1..4dfad186 100644 --- a/crates/bridge-core/tests/settings.rs +++ b/crates/bridge-core/tests/settings.rs @@ -3,7 +3,7 @@ mod support; -use agent_protocol::{BridgeMessage, HostMessage, SessionEvent}; +use agent_protocol::{BridgeMessage, HostMessage, RefusedProvider, SessionEvent}; use bridge_core::ports::memory::{MemoryStore, MemoryTranscripts}; use bridge_core::ports::Transcripts as _; use bridge_core::{Effect, Input, PairingCloseReason, Via}; @@ -321,7 +321,7 @@ fn take_model_fetch(rig: &mut Rig) -> Option<(String, String, String)> { } fn listed(ids: &[&str]) -> HostMessage { - HostMessage::ProviderModels { models: ids.iter().map(|id| ProviderModel { id: (*id).into(), label: None }).collect() } + HostMessage::ProviderModels { models: ids.iter().map(|id| ProviderModel { id: (*id).into(), ..Default::default() }).collect() } } fn stored_profile(rig: &mut Rig) -> Option { @@ -433,6 +433,82 @@ fn an_agent_that_adds_profile_models_gets_its_profiles_whenever_they_change() { assert!(!rig.has_host_request(|m| matches!(m, BridgeMessage::SetProviders { agent, .. } if agent == "alpha"))); } +/// Answer the pending `set-providers` to delta, leaving out `refused` +/// (profile id, reason). +fn delta_takes(rig: &mut Rig, refused: &[(&str, &str)]) { + let (id, _) = rig.host_request(|m| matches!(m, BridgeMessage::SetProviders { agent, .. } if agent == "delta")); + let refused = refused.iter().map(|(id, reason)| RefusedProvider { id: (*id).into(), reason: (*reason).into() }).collect(); + rig.host_reply(&id, HostMessage::ProvidersSet { refused }); +} + +fn delta_kimi(label: &str) -> serde_json::Value { + json!({"agent":"delta","label":label,"baseUrl":"https://k.test","authToken":"t","models":[{"id":"m"}]}) +} + +#[test] +fn a_save_the_agent_leaves_out_is_undone_and_refused_with_its_reason() { + let mut rig = rig_with_providers(); + set_profile(&mut rig, delta_kimi("Home")); + assert!(!answer_token_check(&mut rig, None), "the token waits on the agent taking the profile"); + delta_takes(&mut rig, &[]); + assert!(answer_token_check(&mut rig, Some(true))); + assert_eq!(stored_profile(&mut rig).map(|p| p.label), Some("Home".into())); + + // Renamed to a name the agent already has: the save is undone. + set_profile(&mut rig, delta_kimi("DeepSeek")); + let reason = "OpenCode already has a provider called 'deepseek'. Give this profile another name."; + delta_takes(&mut rig, &[("kimi", reason)]); + assert!(!answer_token_check(&mut rig, None)); + assert_eq!(ack_error(&mut rig).as_deref(), Some(reason)); + assert!(rig.store.snapshot()["providerProfiles"].contains("\"Home\""), "the stored profile is the one before"); + let (_, again) = rig.host_request(|m| matches!(m, BridgeMessage::SetProviders { agent, .. } if agent == "delta")); + match again { + BridgeMessage::SetProviders { providers, .. } => assert_eq!(providers[0].label, "Home", "and the agent gets it back"), + _ => unreachable!(), + } + + // A new profile the agent leaves out is not stored at all. + rig.take(); + rig.send(json!({"type":"set-provider-profile","profileId":"other","profile":delta_kimi("DeepSeek")})); + delta_takes(&mut rig, &[("other", reason)]); + assert_eq!(ack_error(&mut rig).as_deref(), Some(reason)); + assert!(!rig.store.snapshot()["providerProfiles"].contains("\"other\"")); +} + +#[test] +fn a_profile_the_agent_leaves_out_later_shows_why() { + let mut rig = rig_with_providers(); + set_profile(&mut rig, delta_kimi("Home")); + delta_takes(&mut rig, &[]); + answer_token_check(&mut rig, Some(true)); + rig.take(); + // The host restarts and the agent no longer takes it. + rig.input(Input::HostDown { reason: "test".into() }); + rig.host_up_with(vec![alpha(), beta(), delta()]); + delta_takes(&mut rig, &[("kimi", "taken")]); + assert_eq!(stored_profile(&mut rig).and_then(|p| p.error).as_deref(), Some("taken")); + // Taken again: the reason goes. + set_profile(&mut rig, delta_kimi("Home 2")); + delta_takes(&mut rig, &[]); + answer_token_check(&mut rig, None); + assert_eq!(stored_profile(&mut rig).map(|p| p.error), Some(None)); +} + +#[test] +fn profiles_reach_an_agent_oldest_saved_first() { + let mut rig = rig_with_providers(); + for id in ["zeta", "alpha-1"] { + rig.send(json!({"type":"set-provider-profile","profileId":id,"profile":delta_kimi(id)})); + delta_takes(&mut rig, &[]); + answer_token_check(&mut rig, None); + rig.advance(1_000); + } + rig.take(); + rig.input(Input::HostDown { reason: "test".into() }); + rig.host_up_with(vec![alpha(), beta(), delta()]); + assert_eq!(pushed_to_delta(&mut rig), Some(vec!["zeta".to_string(), "alpha-1".to_string()])); +} + fn with_kimi() -> Rig { let mut rig = rig_with_providers(); set_profile(&mut rig, kimi(Some("tok"))); diff --git a/crates/client-core/src/stores/fetches.rs b/crates/client-core/src/stores/fetches.rs index cc91cb75..e19a2eea 100644 --- a/crates/client-core/src/stores/fetches.rs +++ b/crates/client-core/src/stores/fetches.rs @@ -1,4 +1,4 @@ -//! `fetches` — which on-request answers from a bridge are still fresh, so a +//! `fetches` — which on-request answers from a bridge are still current, so a //! screen that asks again sends nothing. //! //! A bridge answers model lists and provider profiles only when asked; the @@ -6,16 +6,16 @@ //! for depends on how it can change: //! - provider profiles: the bridge pushes the new list to every phone //! whenever one changes, so an answer holds for the whole connection; -//! - a model list can change without a push (the agent learns of a new -//! model), so it holds for [`MODELS_FRESH_FOR_MS`] — and not at all after -//! the machine's credentials change ([`Fetches::forget_models`]). +//! - a model list can change without a push (a provider profile was added, +//! the agent learned of a new model), so it is asked for every time a +//! screen needs it — only a request still in flight is not repeated. //! //! A reconnect may have missed a push, so it forgets everything //! ([`Fetches::forget_all`]). In memory only: a fresh process asks again. use std::collections::BTreeMap; -/// A request whose answer is worth keeping for a while. +/// A request whose answer, or whose pending answer, is worth remembering. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub enum Fetch { ProviderProfiles, @@ -23,9 +23,6 @@ pub enum Fetch { Models(String), } -/// How long an agent's model list counts as current. -pub const MODELS_FRESH_FOR_MS: u64 = 5 * 60_000; - /// A request unanswered for this long may have been lost: asking again is /// allowed. pub const FETCH_RETRY_AFTER_MS: u64 = 15_000; @@ -34,8 +31,8 @@ pub const FETCH_RETRY_AFTER_MS: u64 = 15_000; enum State { /// Sent at this time (ms), no answer yet. InFlight(u64), - /// Answered at this time (ms). - Answered(u64), + /// Answered, and every change since will be pushed. + Current, } #[derive(Debug, Default, Clone, PartialEq)] @@ -45,16 +42,12 @@ pub struct Fetches { impl Fetches { /// Whether to send `fetch` to `machine` now; when it says yes it counts - /// the request as sent. No while a fresh answer is held or a request is + /// the request as sent. No while a pushed answer is held or a request is /// under [`FETCH_RETRY_AFTER_MS`] old. pub fn should_request(&mut self, machine: &str, fetch: Fetch, now: u64) -> bool { - let fresh_for = match fetch { - Fetch::ProviderProfiles => u64::MAX, - Fetch::Models(_) => MODELS_FRESH_FOR_MS, - }; let key = (machine.to_string(), fetch); let skip = match self.entries.get(&key) { - Some(State::Answered(at)) => now.saturating_sub(*at) < fresh_for, + Some(State::Current) => true, Some(State::InFlight(at)) => now.saturating_sub(*at) < FETCH_RETRY_AFTER_MS, None => false, }; @@ -64,19 +57,18 @@ impl Fetches { !skip } - /// `machine` answered `fetch` at `now`. - pub fn answered(&mut self, machine: &str, fetch: Fetch, now: u64) { - self.entries.insert((machine.to_string(), fetch), State::Answered(now)); - } - - /// Ask again next time (an answer that was an error, say). - pub fn forget(&mut self, machine: &str, fetch: Fetch) { - self.entries.remove(&(machine.to_string(), fetch)); - } - - /// `machine`'s model lists may have changed (its credentials did). - pub fn forget_models(&mut self, machine: &str) { - self.entries.retain(|(m, f), _| !(m == machine && matches!(f, Fetch::Models(_)))); + /// `machine` answered `fetch`. Only an answer the bridge keeps current + /// by pushing is held; any other is asked for again next time. + pub fn answered(&mut self, machine: &str, fetch: Fetch) { + let key = (machine.to_string(), fetch); + match key.1 { + Fetch::ProviderProfiles => { + self.entries.insert(key, State::Current); + } + Fetch::Models(_) => { + self.entries.remove(&key); + } + } } /// A new connection: pushes may have been missed while away. @@ -95,8 +87,8 @@ mod tests { assert!(f.should_request("m", Fetch::ProviderProfiles, 0)); assert!(!f.should_request("m", Fetch::ProviderProfiles, 1_000), "in flight"); assert!(f.should_request("m", Fetch::ProviderProfiles, FETCH_RETRY_AFTER_MS), "presumed lost"); - f.answered("m", Fetch::ProviderProfiles, FETCH_RETRY_AFTER_MS); - assert!(!f.should_request("m", Fetch::ProviderProfiles, 100 * MODELS_FRESH_FOR_MS), "pushed on change"); + f.answered("m", Fetch::ProviderProfiles); + assert!(!f.should_request("m", Fetch::ProviderProfiles, u64::MAX), "pushed on change"); assert!(f.should_request("other", Fetch::ProviderProfiles, 0), "per machine"); f.forget_all(); @@ -104,18 +96,13 @@ mod tests { } #[test] - fn a_model_list_goes_stale_and_new_credentials_retire_it() { + fn a_model_list_is_asked_for_again_once_answered() { let mut f = Fetches::default(); let claude = || Fetch::Models("claude".into()); - f.answered("m", claude(), 0); - f.answered("m", Fetch::ProviderProfiles, 0); - assert!(!f.should_request("m", claude(), MODELS_FRESH_FOR_MS - 1)); - assert!(f.should_request("m", claude(), MODELS_FRESH_FOR_MS), "stale"); - assert!(f.should_request("m", Fetch::Models("opencode".into()), 0), "per agent"); - - f.answered("m", claude(), 0); - f.forget_models("m"); assert!(f.should_request("m", claude(), 0)); - assert!(!f.should_request("m", Fetch::ProviderProfiles, 0), "profiles are kept"); + assert!(!f.should_request("m", claude(), 1_000), "in flight"); + assert!(f.should_request("m", Fetch::Models("opencode".into()), 1_000), "per agent"); + f.answered("m", claude()); + assert!(f.should_request("m", claude(), 1_001), "answered: the next screen asks again"); } } diff --git a/crates/client-core/src/stores/machines.rs b/crates/client-core/src/stores/machines.rs index cfdb1ac6..8a45bedc 100644 --- a/crates/client-core/src/stores/machines.rs +++ b/crates/client-core/src/stores/machines.rs @@ -1854,6 +1854,7 @@ mod tests { models_from_provider: false, default_model: None, has_token: true, + error: None, }], }, ); diff --git a/crates/client-ffi/src/intent.rs b/crates/client-ffi/src/intent.rs index 41740b30..043c6345 100644 --- a/crates/client-ffi/src/intent.rs +++ b/crates/client-ffi/src/intent.rs @@ -527,7 +527,7 @@ impl TryFrom for Intent { label: p.label, base_url: p.base_url, auth_token: p.auth_token.into(), - models: p.models.into_iter().map(|m| ProviderModel { id: m.id, label: m.label }).collect(), + models: p.models.into_iter().map(|m| ProviderModel { id: m.id, label: m.label, ..Default::default() }).collect(), models_from_provider: p.models_from_provider, default_model: p.default_model, }), @@ -770,7 +770,7 @@ mod tests { label: "Kimi K3".into(), base_url: "https://api.moonshot.ai/anthropic".into(), auth_token: Tristate::Set("sk-1".into()), - models: vec![ProviderModel { id: "kimi-k3".into(), label: Some("Kimi K3".into()) }], + models: vec![ProviderModel { id: "kimi-k3".into(), label: Some("Kimi K3".into()), ..Default::default() }], models_from_provider: true, default_model: Some("kimi-k3".into()), }), diff --git a/crates/client-ffi/src/views.rs b/crates/client-ffi/src/views.rs index 3ca5c05c..255c5f03 100644 --- a/crates/client-ffi/src/views.rs +++ b/crates/client-ffi/src/views.rs @@ -328,10 +328,35 @@ pub struct UniffiModelEntry { pub label: Option, /// Who serves the model, when the agent says. pub provider: Option, + /// How many tokens the model takes in, when known. + pub context_window: Option, } fn to_uniffi_model_entries(models: &[protocol::events::ModelEntry]) -> Vec { - models.iter().map(|m| UniffiModelEntry { id: m.id.clone(), label: m.label.clone(), provider: m.provider.clone() }).collect() + models + .iter() + .map(|m| UniffiModelEntry { id: m.id.clone(), label: m.label.clone(), provider: m.provider.clone(), context_window: None }) + .collect() +} + +/// A provider profile's model, grouped under the profile — and under the +/// provider a gateway routes it to, when the endpoint names one, the same +/// group the agent's own list shows it under. Unnamed, it goes by its id +/// without that provider, which the group already shows. +fn to_uniffi_profile_model(profile: &str, m: &protocol::common::ProviderModel) -> UniffiModelEntry { + let upstream = m.provider.as_deref().filter(|p| !p.is_empty()); + let label = m.label.clone().or_else(|| { + upstream.and_then(|p| m.id.strip_prefix(p)).and_then(|rest| rest.strip_prefix('/')).map(str::to_string) + }); + UniffiModelEntry { + id: m.id.clone(), + label, + provider: Some(match upstream { + Some(p) => format!("{profile} · {p}"), + None => profile.to_string(), + }), + context_window: m.context_window, + } } /// A custom AI provider profile the bridge has stored — gated on the @@ -349,6 +374,8 @@ pub struct UniffiProviderProfileInfo { pub models_from_provider: bool, pub default_model: Option, pub has_token: bool, + /// Why the agent does not offer this profile's models, when it does not. + pub error: Option, } /// One selectable value of an agent option (a mode, an effort level, a plan @@ -758,14 +785,11 @@ pub fn build_uniffi_machines_view(v: &MachinesView) -> UniffiMachinesView { agent: p.agent.clone(), label: p.label.clone(), base_url: p.base_url.clone(), - models: p - .models - .iter() - .map(|m| UniffiModelEntry { id: m.id.clone(), label: m.label.clone(), provider: Some(p.label.clone()) }) - .collect(), + models: p.models.iter().map(|m| to_uniffi_profile_model(&p.label, m)).collect(), models_from_provider: p.models_from_provider, default_model: p.default_model.clone(), has_token: p.has_token, + error: p.error.clone(), }) .collect(), plugins: m.plugins.iter().map(|(agent, p)| to_uniffi_agent_plugins(agent, p)).collect(), @@ -1377,6 +1401,24 @@ mod tests { cache.as_ref().map(|c| c.entries().iter().map(|e| e.seq).collect()).unwrap_or_default() } + #[test] + fn a_profile_model_is_grouped_under_the_profile_and_its_upstream() { + let routed = protocol::common::ProviderModel { + id: "OpenCode Go/deepseek-v4.1-flash".into(), + provider: Some("OpenCode Go".into()), + context_window: Some(1_000_000), + ..Default::default() + }; + let m = to_uniffi_profile_model("CCR", &routed); + assert_eq!(m.provider.as_deref(), Some("CCR · OpenCode Go")); + assert_eq!(m.label.as_deref(), Some("deepseek-v4.1-flash"), "named without the group's provider"); + assert_eq!(m.context_window, Some(1_000_000)); + + let own = protocol::common::ProviderModel { id: "kimi-k3".into(), label: Some("Kimi K3".into()), ..Default::default() }; + let m = to_uniffi_profile_model("Moonshot", &own); + assert_eq!((m.provider.as_deref(), m.label.as_deref()), (Some("Moonshot"), Some("Kimi K3"))); + } + #[test] fn the_cache_takes_in_only_what_was_stored_since() { let mut cache = None; diff --git a/crates/client-runtime/src/dispatch.rs b/crates/client-runtime/src/dispatch.rs index 28cfa631..a88508d4 100644 --- a/crates/client-runtime/src/dispatch.rs +++ b/crates/client-runtime/src/dispatch.rs @@ -339,13 +339,10 @@ impl<'a> Router<'a> { // --- slice C: machines-slice updates + fire-and-answer acks --- BridgeToPhone::Models(m) => { self.stores.machines.apply_models(machine, m); - // An empty list comes with the reason: ask again next time. - let fetch = Fetch::Models(m.agent.clone()); - if m.models.is_empty() { - self.stores.machines.fetches.forget(machine, fetch); - } else { - self.stores.machines.fetches.answered(machine, fetch, self.now); - } + self.stores + .machines + .fetches + .answered(machine, Fetch::Models(m.agent.clone())); r.persist(StoreId::Machines); } BridgeToPhone::Usage(m) => { @@ -407,8 +404,11 @@ impl<'a> Router<'a> { } BridgeToPhone::ProviderProfiles(m) => { self.stores.machines.apply_provider_profiles(machine, m); - self.stores.machines.fetches.answered(machine, Fetch::ProviderProfiles, self.now); - // CDX-062: provider profiles are never persisted. + self.stores.machines.fetches.answered(machine, Fetch::ProviderProfiles); + // CDX-062: provider profiles are never written to disk — the + // machines slice strips them when it serializes — but this + // persist is what tells the views the slice changed. + r.persist(StoreId::Machines); } BridgeToPhone::CredentialsAck(m) => { self.stores.ui.apply_credentials_ack( @@ -427,8 +427,6 @@ impl<'a> Router<'a> { m.agent.as_deref(), &m.credentials, ); - // New credentials can change what the agents list. - self.stores.machines.fetches.forget_models(machine); r.persist(StoreId::Machines); } } @@ -1326,6 +1324,37 @@ mod tests { ); } + /// A new profile list must reach the open providers page at once: the + /// machines persist is the views' refresh signal, even though the + /// profiles themselves are never written. + #[tokio::test] + async fn provider_profiles_refresh_the_machines_view() { + let (mut s, ts, kp) = stores().await; + s.machines.register_machine(MACHINE, "laptop", None, None, &[]); + let mut r = Router::new(&mut s, &ts, &kp, 1_000); + let out = r + .route( + MACHINE, + &BridgeToPhone::ProviderProfiles(protocol::events::ProviderProfilesMsg { + machine: MACHINE.into(), + profiles: vec![protocol::common::ProviderProfileInfo { + id: "home".into(), + agent: "opencode".into(), + label: "Home".into(), + base_url: "http://192.168.1.2:3456".into(), + models: vec![], + models_from_provider: false, + default_model: None, + has_token: true, + error: None, + }], + }), + ) + .await; + assert_eq!(out.persist, vec![StoreId::Machines]); + assert_eq!(s.machines.machine(MACHINE).unwrap().provider_profiles.as_ref().unwrap()[0].id, "home"); + } + #[tokio::test] async fn option_confirmed_writes_through_to_the_session_info() { let (mut s, ts, kp) = stores().await; diff --git a/crates/client-runtime/src/intent.rs b/crates/client-runtime/src/intent.rs index 533c9302..8165bf66 100644 --- a/crates/client-runtime/src/intent.rs +++ b/crates/client-runtime/src/intent.rs @@ -1465,9 +1465,10 @@ mod tests { )); } - /// Screens ask each time they open; only what is not fresh goes out. + /// Screens ask each time they open; the profiles, which the bridge + /// pushes on every change, are not asked for twice. #[tokio::test] - async fn screens_reopening_send_only_the_requests_whose_answers_are_not_fresh() { + async fn screens_reopening_ask_again_for_everything_not_pushed() { let (mut s, kp) = stores().await; let mut sent = 0; let mut open_screen = |s: &mut CoreStores, now: u64| { @@ -1481,13 +1482,11 @@ mod tests { }; assert_eq!(open_screen(&mut s, 0), 2, "first open asks for both"); assert_eq!(open_screen(&mut s, 1_000), 0, "both still in flight"); - s.machines.fetches.answered("m", Fetch::ProviderProfiles, 2_000); - s.machines.fetches.answered("m", Fetch::Models("claude".into()), 2_000); - assert_eq!(open_screen(&mut s, 60_000), 0, "both answered and fresh"); - let later = 2_000 + client_core::stores::fetches::MODELS_FRESH_FOR_MS; - assert_eq!(open_screen(&mut s, later), 1, "the model list went stale; profiles are pushed"); + s.machines.fetches.answered("m", Fetch::ProviderProfiles); + s.machines.fetches.answered("m", Fetch::Models("claude".into())); + assert_eq!(open_screen(&mut s, 3_000), 1, "the model list is asked for again; profiles are pushed"); s.machines.fetches.forget_all(); - assert_eq!(open_screen(&mut s, later + 1), 2, "a reconnect asks again"); + assert_eq!(open_screen(&mut s, 4_000), 2, "a reconnect asks again"); } #[tokio::test] diff --git a/crates/protocol/fixtures/corpus.json b/crates/protocol/fixtures/corpus.json index 2ffe748c..5438f6eb 100644 --- a/crates/protocol/fixtures/corpus.json +++ b/crates/protocol/fixtures/corpus.json @@ -148,7 +148,7 @@ { "type": "credentials-ack", "machine": "m", "success": false, "credentials": [], "error": "not writable" }, { "type": "pair-ack", "machine": "m", "ok": true }, { "type": "pair-ack", "machine": "m", "ok": false, "reason": "bad-token", "relays": ["wss://r"], "host": "cli" }, - { "type": "provider-profiles", "machine": "m", "profiles": [{ "id": "p", "agent": "claude-code", "label": "L", "baseUrl": "https://x", "models": [{ "id": "m" }], "hasToken": true }, { "id": "q", "agent": "opencode", "label": "Q", "baseUrl": "https://y", "models": [{ "id": "a/b", "label": "B" }], "modelsFromProvider": true, "hasToken": true }] }, + { "type": "provider-profiles", "machine": "m", "profiles": [{ "id": "p", "agent": "claude-code", "label": "L", "baseUrl": "https://x", "models": [{ "id": "m" }], "hasToken": true }, { "id": "q", "agent": "opencode", "label": "Q", "baseUrl": "https://y", "models": [{ "id": "a/b", "label": "B" }, { "id": "OpenCode Go/deepseek-v4.1-flash", "label": "deepseek-v4.1-flash", "provider": "OpenCode Go", "contextWindow": 1000000 }], "modelsFromProvider": true, "hasToken": true }, { "id": "r", "agent": "opencode", "label": "DeepSeek", "baseUrl": "https://z", "models": [{ "id": "c" }], "hasToken": true, "error": "OpenCode already has a provider called 'deepseek'. Give this profile another name." }] }, { "type": "provider-profile-ack", "machine": "m", "profileId": "p", "success": true, "tokenValid": false } ], "rejected": [ diff --git a/crates/protocol/src/common.rs b/crates/protocol/src/common.rs index f7fe68d6..ccb9663a 100644 --- a/crates/protocol/src/common.rs +++ b/crates/protocol/src/common.rs @@ -540,11 +540,20 @@ fn is_private_network_address(host: &str) -> bool { false } -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, specta::Type)] +/// A model a provider profile offers. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, specta::Type)] +#[serde(rename_all = "camelCase")] pub struct ProviderModel { pub id: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub label: Option, + /// The provider a gateway routes the model to (`OpenCode Go` for a + /// router's `OpenCode Go/deepseek-v4.1-flash`), when the endpoint says. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub provider: Option, + /// How many tokens the model takes in, when the endpoint says. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub context_window: Option, } /// The REDACTED wire shape of a stored provider profile (`hasToken` only — the @@ -565,6 +574,10 @@ pub struct ProviderProfileInfo { #[serde(default, skip_serializing_if = "Option::is_none")] pub default_model: Option, pub has_token: bool, + /// Why the agent does not offer this profile's models, when it does not + /// (one of its own providers has the profile's name, say). + #[serde(default, skip_serializing_if = "Option::is_none")] + pub error: Option, } // --- transcript entries --- diff --git a/docs/AGENT-CANDIDATES.md b/docs/AGENT-CANDIDATES.md index 10d31d66..232529a2 100644 --- a/docs/AGENT-CANDIDATES.md +++ b/docs/AGENT-CANDIDATES.md @@ -105,7 +105,10 @@ offer no tool that only works on its vendor's API. OpenCode meets all three: its web search is offered only on its own provider (checked on 1.18.32), its web fetch is local, and a provider profile adds the endpoint's models to its list beside OpenCode Zen's free ones — read from -the endpoint, never typed by hand. Codex is out (its client speaks only the +the endpoint, never typed by hand. The server the bridge starts sets +`OPENCODE_ENABLE_EXA`, so that web search (Exa's keyless endpoint, behind +OpenCode's `websearch` permission) is offered to every model, not only to +OpenCode's own providers'. Codex is out (its client speaks only the Responses API); a Pi-based entry stays an option once the Pi driver exists. ## Installing agents on demand diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 642a7f38..b3615802 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -239,9 +239,18 @@ Messages: redirect is followed, and the save is refused when the list cannot be read or is empty. A `defaultModel` the list does not name is dropped. The stored profile reports the flag back, so a later save (with the token kept) reads - the list again. + the list again. A model read so keeps, when the endpoint says, the + `provider` a gateway routes it to (the part of the id before its first + `/`, which stays in the id) and its `contextWindow`; phones group such a + model under "profile · provider". - The token is checked by the profile's agent, with the smallest request on the API it speaks (`tokenValid` in the ack). +- For an agent with `supports.providerModels`, a save waits on the agent + taking the new list: a profile it leaves out (its own provider, or an + older profile, has the name) is not saved — the earlier version stands — + and the ack carries the agent's reason. A stored profile the agent leaves + out later (after an operator adds a provider of that name) carries the + reason as `error` in `provider-profiles`. - A profile stored before profiles named their agent has an empty `agent`: no agent uses it until a save names one. - `provider-profiles-request` → `provider-profiles {profiles[]}` to the asking @@ -515,8 +524,8 @@ The bridge's ids are `b1, b2, …`; the host's are `h1, h2, …`. | `session-mcp-toggle {sessionId, name, enabled}` | `session-mcp {…}` once done, or `error` | | `check-credential {agent, credential, value}` | `credential-checked {valid?}` | | `check-provider {agent, provider, model}` | `credential-checked {valid?}`: a provider profile's token, checked on the API `agent` speaks | -| `list-provider-models {agent, baseUrl, authToken}` | `provider-models {models}` (never empty), read the way `agent` signs in; or `error` with the reason there is none | -| `set-providers {agent, providers}` | `ack` once an agent with `supports.providerModels` offers these profiles' models (all of its profiles, sent after `initialize` and on every change), or `error` | +| `list-provider-models {agent, baseUrl, authToken}` | `provider-models {models}` (never empty; each `{id, label?, provider?, contextWindow?}`), read the way `agent` signs in; or `error` with the reason there is none | +| `set-providers {agent, providers}` | `providers-set {refused: [{id, reason}]}` once an agent with `supports.providerModels` offers these profiles' models (all of its profiles, oldest saved first, sent after `initialize` and on every change) — all but the refused ones, which it cannot add (a name one of its own providers, or an earlier profile, has); or `error` | `AgentInfo` is the catalog entry minus credential status (the bridge adds that), plus `credentials[].envVar` and `unavailableReason`. diff --git a/packages/agent-host/src/drivers/opencode/__tests__/opencodeDriver.test.ts b/packages/agent-host/src/drivers/opencode/__tests__/opencodeDriver.test.ts index e00aab6c..cddf7cc2 100644 --- a/packages/agent-host/src/drivers/opencode/__tests__/opencodeDriver.test.ts +++ b/packages/agent-host/src/drivers/opencode/__tests__/opencodeDriver.test.ts @@ -196,6 +196,26 @@ describe('OpenCode permission asks', () => { expect(client.permission.reply).toHaveBeenCalledWith({ requestID: 'per_1', directory: '/tmp', reply: 'once' }); }); + it("offers OpenCode's always when it has one, and replies with it", async () => { + const offered = { ...ask, properties: { ...ask.properties, always: ['/etc/*'] } }; + const client = clientWith([pendingRead, offered]); + const ctx = start(client, {}, { permission: () => ({ outcome: 'selected', optionId: 'allow_always' }) }); + await ctx.ended(); + expect(ctx.permissions[0]!.options.map((o) => [o.id, o.label])).toEqual([ + ['allow', 'Allow'], + ['allow_always', 'Always allow in this project'], + ['deny', 'Deny'], + ]); + expect(client.permission.reply).toHaveBeenCalledWith({ requestID: 'per_1', directory: '/tmp', reply: 'always' }); + }); + + it('an ask with nothing to always allow refuses an always answer', async () => { + const client = clientWith([pendingRead, ask]); + const ctx = start(client, {}, { permission: () => ({ outcome: 'selected', optionId: 'allow_always' }) }); + await ctx.ended(); + expect(client.permission.reply).toHaveBeenCalledWith({ requestID: 'per_1', directory: '/tmp', reply: 'reject' }); + }); + it('the auto-approve mode allows without asking', async () => { const client = clientWith([pendingRead, ask]); const ctx = start(client, { mode: 'default' }); @@ -361,8 +381,9 @@ describe('OpenCode options', () => { const ctx = recordingContext(); const session = OpenCodeDriver.withClient(clientWith([])).startSession({ sessionId: 's1', agent: 'opencode', cwd: '/tmp' }, ctx); await session.setOption('mode', 'default'); + await session.setOption('mode', 'plan'); await session.setOption('model', 'anthropic/claude-sonnet-5'); - await expect(session.setOption('mode', 'plan')).rejects.toThrow(/no mode/); + await expect(session.setOption('mode', 'acceptEdits')).rejects.toThrow(/no mode/); await expect(session.setOption('model', 'sonnet')).rejects.toThrow(/provider\/model/); await expect(session.setOption('effort', 'high')).rejects.toThrow(/no effort/); await session.end(); @@ -423,6 +444,25 @@ describe('OpenCode default model', () => { }); }); +describe('OpenCode plan mode', () => { + it("runs OpenCode's plan agent, and its default agent again once left", async () => { + const client = clientWith([]); + const promptAsync = vi.fn().mockResolvedValue({ data: undefined, error: undefined }); + (client.session as unknown as { promptAsync: unknown }).promptAsync = promptAsync; + const ctx = recordingContext(); + const session = OpenCodeDriver.withClient(client).startSession({ sessionId: 's1', agent: 'opencode', cwd: '/tmp', mode: 'plan' }, ctx); + await ctx.waitFor((e) => e.type === 'ready'); + session.prompt('how would you split this module?'); + await expect.poll(() => promptAsync.mock.calls.length).toBe(1); + expect(promptAsync.mock.calls[0]![0].agent).toBe('plan'); + await session.setOption('mode', 'ask'); + session.prompt('do it'); + await expect.poll(() => promptAsync.mock.calls.length).toBe(2); + expect(promptAsync.mock.calls[1]![0]).not.toHaveProperty('agent'); + await session.end(); + }); +}); + describe('OpenCode context usage', () => { const withLimits = (client: FakeClient) => Object.assign(client, { diff --git a/packages/agent-host/src/drivers/opencode/__tests__/opencodeProviders.test.ts b/packages/agent-host/src/drivers/opencode/__tests__/opencodeProviders.test.ts index 37a11fa2..eb66505e 100644 --- a/packages/agent-host/src/drivers/opencode/__tests__/opencodeProviders.test.ts +++ b/packages/agent-host/src/drivers/opencode/__tests__/opencodeProviders.test.ts @@ -5,7 +5,7 @@ import { describe, expect, it, vi } from 'vitest'; import type { OpencodeClient } from '@opencode-ai/sdk/v2/client'; import { OpenCodeDriver } from '../driver'; -import { providersConfig, serverSetup } from '../providers'; +import { admitProfiles, profileModelGroup, profileProviderId, providersConfig, serverSetup } from '../providers'; import type { StartOpenCodeServerOptions } from '../server'; import type { ProviderBinding } from '../../../sdk/types'; @@ -26,7 +26,7 @@ describe('providersConfig', () => { theme: 'dark', provider: { mine: { npm: 'x' }, - 'codedeck-router': { + 'home-router': { npm: '@ai-sdk/openai-compatible', name: 'Home router', options: { baseURL: 'http://192.168.1.2:3458/v1', apiKey: '{env:CODEDECK_PROVIDER_KEY_0}' }, @@ -37,6 +37,35 @@ describe('providersConfig', () => { expect(JSON.stringify(config)).not.toContain('tok-secret'); }); + it('names a routed model without its upstream, which its group shows, and gives OpenCode a known context window', () => { + const routed = router({ models: [{ id: 'OpenCode Go/deepseek-v4.1-flash', provider: 'OpenCode Go', contextWindow: 1_000_000 }] }); + const provider = (providersConfig([routed]).provider as Record }>)['home-router']!; + expect(provider.models).toEqual({ + 'OpenCode Go/deepseek-v4.1-flash': { name: 'deepseek-v4.1-flash', limit: { context: 1_000_000, output: 0 } }, + }); + expect(profileModelGroup('CCR', routed.models[0])).toBe('CCR · OpenCode Go'); + expect(profileModelGroup('CCR', { id: 'kimi-k3' })).toBe('CCR'); + }); + + it('names each provider as the user named the profile', () => { + expect(profileProviderId(router({ label: 'CCR' }))).toBe('ccr'); + expect(profileProviderId(router({ label: 'OpenCode Go (OpenAI)' }))).toBe('opencode-go-openai'); + expect(profileProviderId(router({ label: 'Café / LAN' }))).toBe('cafe-lan'); + expect(profileProviderId(router({ label: '★', id: 'p-7' }))).toBe('p-7'); + }); + + it("leaves out a profile whose name one of OpenCode's providers, or an earlier profile, already has", () => { + const ccr = router({ id: 'a', label: 'CCR' }); + const again = router({ id: 'b', label: 'ccr' }); + const deepseek = router({ id: 'c', label: 'DeepSeek' }); + const { admitted, refused } = admitProfiles([ccr, again, deepseek], new Set(['deepseek', 'opencode'])); + expect(admitted.map((p) => p.id)).toEqual(['a']); + expect(refused).toEqual([ + { id: 'b', reason: "The provider profile 'CCR' already goes by 'ccr' in OpenCode. Give this one another name." }, + { id: 'c', reason: "OpenCode already has a provider called 'deepseek'. Give this profile another name." }, + ]); + }); + it('does not restrict the providers OpenCode already has', () => { const config = providersConfig([router()]); expect(config).not.toHaveProperty('enabled_providers'); @@ -47,21 +76,23 @@ describe('providersConfig', () => { describe('serverSetup', () => { it('passes the tokens in the environment and guards the server with a password of its own', () => { - const a = serverSetup([router(), router({ id: 'or', authToken: 'tok-2' })], {}); + const a = serverSetup([router(), router({ id: 'or', label: 'My OpenRouter', authToken: 'tok-2' })], {}); expect(a.env.CODEDECK_PROVIDER_KEY_0).toBe('tok-secret'); expect(a.env.CODEDECK_PROVIDER_KEY_1).toBe('tok-2'); - expect(JSON.parse(a.env.OPENCODE_CONFIG_CONTENT!).provider).toHaveProperty('codedeck-or'); + expect(JSON.parse(a.env.OPENCODE_CONFIG_CONTENT!).provider).toHaveProperty('my-openrouter'); const password = a.env.OPENCODE_SERVER_PASSWORD!; expect(password).toMatch(/^[0-9a-f]{48}$/); expect(a.headers.authorization).toBe(`Basic ${Buffer.from(`opencode:${password}`).toString('base64')}`); expect(serverSetup([], {}).env.OPENCODE_SERVER_PASSWORD).not.toBe(password); + // Web search for every model, not only OpenCode's own providers'. + expect(serverSetup([], {}).env.OPENCODE_ENABLE_EXA).toBe('1'); // No profiles: the operator's config stands as it is. expect(serverSetup([], { OPENCODE_CONFIG_CONTENT: '{"theme":"x"}' }).env).not.toHaveProperty('OPENCODE_CONFIG_CONTENT'); }); it("keeps the operator's own environment config underneath", () => { const { env } = serverSetup([router()], { OPENCODE_CONFIG_CONTENT: '{"provider":{"mine":{"npm":"x"}}}' }); - expect(Object.keys(JSON.parse(env.OPENCODE_CONFIG_CONTENT!).provider)).toEqual(['mine', 'codedeck-router']); + expect(Object.keys(JSON.parse(env.OPENCODE_CONFIG_CONTENT!).provider)).toEqual(['mine', 'home-router']); }); }); @@ -116,6 +147,64 @@ describe('an OpenCode driver given provider profiles', () => { expect(closed).toEqual([1, 2, 3]); }); + it('has a model list asked for during the restart wait for the new server', async () => { + let release!: () => void; + const closing = new Promise((resolve) => (release = resolve)); + let starts = 0; + const client = (n: number) => + ({ + config: { + providers: async () => ({ data: { providers: [{ id: 'zen', name: 'Zen', models: { [`m${n}`]: { id: `m${n}`, name: `M${n}` } } }], default: {} } }), + get: async () => ({ data: {} }), + }, + }) as unknown as OpencodeClient; + const driver = await OpenCodeDriver.create({ + autoStart: true, + binaryPath: process.execPath, + log: () => {}, + startServer: async () => { + const n = ++starts; + return { url: `http://127.0.0.1:${4100 + n}`, pid: n, exited: new Promise(() => {}), close: () => closing }; + }, + connect: () => client(starts), + }); + const restart = driver.setProviders([router()]); + await new Promise((resolve) => setTimeout(resolve, 0)); + const listed = driver.listModels(); + release(); + await restart; + expect((await listed).models.map((m) => m.id)).toEqual(['zen/m2']); + expect(starts).toBe(2); + }); + + it("leaves out a profile named as one of OpenCode's providers, but never one it added itself", async () => { + const starts: StartOpenCodeServerOptions[] = []; + const ids = ['opencode', 'deepseek']; + const driver = await OpenCodeDriver.create({ + autoStart: true, + binaryPath: process.execPath, + log: () => {}, + startServer: async (options) => { + starts.push(options); + return { url: `http://127.0.0.1:${4100 + starts.length}`, pid: starts.length, exited: new Promise(() => {}), close: async () => {} }; + }, + // The running server lists what it has, the profiles it was given too. + connect: () => ({ provider: { list: async () => ({ data: { all: ids.map((id) => ({ id })), default: {}, connected: [] } }) } }) as unknown as OpencodeClient, + }); + expect(await driver.setProviders([router({ label: 'DeepSeek' })])).toEqual([ + { id: 'router', reason: "OpenCode already has a provider called 'deepseek'. Give this profile another name." }, + ]); + expect(starts).toHaveLength(1); + + expect(await driver.setProviders([router({ label: 'CCR' })])).toEqual([]); + expect(starts).toHaveLength(2); + ids.push('ccr'); + // Saved again, it keeps the name it already has. + expect(await driver.setProviders([router({ label: 'CCR', models: [{ id: 'other' }] })])).toEqual([]); + expect(starts).toHaveLength(3); + await driver.shutdown(); + }); + it('refuses them for a server it does not start', async () => { const driver = await OpenCodeDriver.create({ serverUrl: 'http://127.0.0.1:4096', log: () => {}, connect: () => ({}) as OpencodeClient }); expect(driver.info().supports?.providerModels).toBe(false); diff --git a/packages/agent-host/src/drivers/opencode/driver.ts b/packages/agent-host/src/drivers/opencode/driver.ts index bc8c3a7b..6be47498 100644 --- a/packages/agent-host/src/drivers/opencode/driver.ts +++ b/packages/agent-host/src/drivers/opencode/driver.ts @@ -37,13 +37,15 @@ import type { } from '@opencode-ai/sdk/v2/client'; import { parseSlashCommand, slashCommand } from '../../sdk/commands'; import type { Driver, DriverSession, McpManager, PluginManager, SessionContext, SessionMcpState } from '../../sdk/driver'; -import { PERMISSION_ALLOW, PERMISSION_DENY, toolKindOf, toolLocations, toolTitle } from '../../sdk/tools'; +import { PERMISSION_ALLOW, PERMISSION_ALLOW_ALWAYS, PERMISSION_DENY, toolKindOf, toolLocations, toolTitle } from '../../sdk/tools'; import { newTranslateContext } from '../../sdk/transcript'; import type { AgentInfo, ModelEntry, + PermissionOption, ProviderBinding, QuestionSpec, + RefusedProvider, SessionOption, SlashCommand, StartSession, @@ -53,7 +55,7 @@ import type { import { opencodeEventToEntries, toolCallDiffs, type OpenCodeEvent } from './adapter'; import { OpenCodeMcp, openCodeSessionMcp, toggleOpenCodeMcp } from './mcp'; import { OpenCodePlugins } from './plugins'; -import { providersFingerprint, serverSetup } from './providers'; +import { admitProfiles, profileModelGroup, profileProviderId, providersFingerprint, serverSetup } from './providers'; import { resolveOpenCodePath, startOpenCodeServer, @@ -68,13 +70,23 @@ import type { EndpointModel } from '../../sdk/providerModels'; export const OPENCODE_AGENT_ID = 'opencode'; /** `ask` sends every permission OpenCode asks for to the phone; `default` - * (the auto-approve mode every agent shares) allows them all. */ + * (the auto-approve mode every agent shares) allows them all; `plan` runs + * OpenCode's own plan agent, which may not edit, asking like `ask`. */ const AUTO_APPROVE_MODE = 'default'; const DEFAULT_MODE = 'ask'; +const PLAN_MODE = 'plan'; const OPENCODE_MODES = [ { id: DEFAULT_MODE, label: 'Ask', description: 'Ask before each tool call' }, + { id: PLAN_MODE, label: 'Plan', description: 'Explore and plan without editing; switch to Ask or YOLO to build it' }, { id: AUTO_APPROVE_MODE, label: 'YOLO', description: 'Run every tool without asking' }, ]; +/** The OpenCode agent the plan mode runs. In any other mode a prompt names + * none, so OpenCode uses its default agent (`build`, or the operator's). */ +const PLAN_AGENT = 'plan'; + +/** OpenCode's "always": the rule stands for every session of the project + * until its server restarts — it is not saved. */ +const PERMISSION_ALLOW_IN_PROJECT: PermissionOption = { ...PERMISSION_ALLOW_ALWAYS, label: 'Always allow in this project' }; type ToolPart = Extract; type PermissionAsk = EventPermissionAsked['properties']; @@ -107,7 +119,9 @@ interface NormalizedPermission { toolUseID: string; title: string; description?: string; - reply: (response: 'once' | 'reject') => Promise; + /** OpenCode offers to allow such calls from now on. */ + always: boolean; + reply: (response: 'once' | 'always' | 'reject') => Promise; } /** A sub-agent's child session, as its `task` call described it. */ @@ -708,7 +722,7 @@ export class OpenCodeSession implements DriverSession { private handlePermission(permission: NormalizedPermission, child?: ChildSession): void { if (this.answeredAsks.has(permission.id)) return; this.answeredAsks.add(permission.id); - const reply = (response: 'once' | 'reject'): void => { + const reply = (response: 'once' | 'always' | 'reject'): void => { // Best effort — the ask then stays pending server-side; there is no // live connection left to retry over. permission.reply(response).catch(() => {}); @@ -727,10 +741,13 @@ export class OpenCodeSession implements DriverSession { description: permission.description ?? permission.title, locations: toolLocations(permission.input), rawInput: permission.input, - options: [PERMISSION_ALLOW, PERMISSION_DENY], + options: permission.always ? [PERMISSION_ALLOW, PERMISSION_ALLOW_IN_PROJECT, PERMISSION_DENY] : [PERMISSION_ALLOW, PERMISSION_DENY], ...(child ? { subagent: subagentOf(child) } : {}), }) - .then((outcome) => reply(outcome.outcome === 'selected' && outcome.optionId === PERMISSION_ALLOW.id ? 'once' : 'reject')) + .then((outcome) => { + const chosen = outcome.outcome === 'selected' ? outcome.optionId : undefined; + reply(chosen === PERMISSION_ALLOW.id ? 'once' : chosen === PERMISSION_ALLOW_IN_PROJECT.id && permission.always ? 'always' : 'reject'); + }) .catch(() => reply('reject')); } @@ -750,6 +767,7 @@ export class OpenCodeSession implements DriverSession { toolUseID: ask.tool?.callID ?? ask.id, title: description, description, + always: ask.always.length > 0, reply: async (response) => { const { error } = await client.permission.reply({ requestID: ask.id, @@ -773,6 +791,7 @@ export class OpenCodeSession implements DriverSession { input: permission.metadata ?? {}, toolUseID: permission.callID ?? permission.id, title: permission.title, + always: false, reply: async (response) => { await client.permission.respond({ sessionID: permission.sessionID, @@ -831,6 +850,7 @@ export class OpenCodeSession implements DriverSession { directory: this.cwd, parts: [{ type: 'text', text }], ...(this.model ? { model: this.model } : {}), + ...this.agentField(), }); if (error) this.deliver({ type: 'error', content: `OpenCode prompt failed: ${JSON.stringify(error)}` }); }) @@ -839,6 +859,12 @@ export class OpenCodeSession implements DriverSession { }); } + /** The OpenCode agent a prompt names: the plan agent in the plan mode, + * none (OpenCode's default) otherwise. */ + private agentField(): { agent?: string } { + return this.mode === PLAN_MODE ? { agent: PLAN_AGENT } : {}; + } + /** `session.command` answers only once the whole turn is over, so it is * not awaited: the turn itself arrives on the event stream. A refusal * becomes an error entry; a connection given up on while the turn runs @@ -846,7 +872,7 @@ export class OpenCodeSession implements DriverSession { private runCommand(client: OpencodeClient, sessionID: string, name: string, args: string): void { const model = this.model ? `${this.model.providerID}/${this.model.modelID}` : undefined; client.session - .command({ sessionID, directory: this.cwd, command: name, arguments: args, ...(model ? { model } : {}) }) + .command({ sessionID, directory: this.cwd, command: name, arguments: args, ...(model ? { model } : {}), ...this.agentField() }) .then(({ error }) => { if (error) this.deliver({ type: 'error', content: `OpenCode /${name} failed: ${JSON.stringify(error)}` }); }) @@ -990,6 +1016,8 @@ export class OpenCodeDriver implements Driver { * one was started with. */ private providers: ProviderBinding[] = []; private served = providersFingerprint([]); + /** The provider ids the running server has from profiles. */ + private servedIds = new Set(); /** Provider changes are applied one at a time, in order. */ private applying: Promise = Promise.resolve(); readonly plugins: PluginManager = new OpenCodePlugins(() => this.client()); @@ -1070,15 +1098,18 @@ export class OpenCodeDriver implements Driver { } this.server = server; this.served = providersFingerprint(profiles); + this.servedIds = new Set(profiles.map(profileProviderId)); const added = profiles.length > 0 ? `, with ${profiles.length} provider profile(s)` : ''; this.options.log(`[opencode] started ${server.url} (pid ${server.pid ?? '?'})${added}`); return this.connect({ baseUrl: server.url, headers: setup.headers }); } /** Find (or install) `opencode`, start its server and connect — sessions - * started meanwhile wait on the same promise. */ - private launch(): void { + * and model lists asked for meanwhile wait on the same promise. `first` + * runs before the start (closing the server being replaced). */ + private launch(first?: () => Promise): void { const attempt = (async (): Promise => { + await first?.(); this.bin ??= await this.options.installOpenCode!(); return this.startServer(this.bin); })(); @@ -1094,33 +1125,54 @@ export class OpenCodeDriver implements Driver { ); } - /** The provider profiles to add to OpenCode's own providers. The server - * reads its config only when it starts, so a changed list restarts it: + /** The provider profiles to add to OpenCode's own providers; those whose + * name one of its providers already has are left out. The server reads + * its config only when it starts, so a changed list restarts it: * sessions on the old one end with an error and the bridge resumes them * on the new one. */ - async setProviders(providers: ProviderBinding[]): Promise { + async setProviders(providers: ProviderBinding[]): Promise { if (!this.manages) { throw new Error('OpenCode runs on a server this bridge does not start (CODEDECK_OPENCODE_SERVER_URL), so it cannot add provider profiles to it'); } - // A profile the bridge would not let a session use is not added either. - this.providers = providers.filter((p) => isValidProviderBaseUrl(p.baseUrl) && p.authToken !== '' && p.models.length > 0); - const apply = this.applying.then(() => this.applyProviders()); - this.applying = apply.catch(() => {}); - await apply; + const apply = this.applying.then(() => this.applyProviders(providers)); + this.applying = apply.then( + () => {}, + () => {}, + ); + return apply; } - private async applyProviders(): Promise { + private async applyProviders(providers: ProviderBinding[]): Promise { // A server still being installed or started is waited for: it may have - // read an older list. + // read an older list, and it says which names are taken. await this.clientPromise?.catch(() => {}); - if (this.stopped || !this.server || !this.bin || providersFingerprint(this.providers) === this.served) return; + // A profile the bridge would not let a session use is not added either. + const usable = providers.filter((p) => isValidProviderBaseUrl(p.baseUrl) && p.authToken !== '' && p.models.length > 0); + const { admitted, refused } = admitProfiles(usable, await this.ownProviderIds()); + this.providers = admitted; + if (this.stopped || !this.server || !this.bin || providersFingerprint(this.providers) === this.served) return refused; this.options.log('[opencode] restarting the server: its provider profiles changed'); const old = this.server; this.server = null; - this.clientPromise = null; - await old.close(); - this.launch(); + this.launch(() => old.close()); await this.clientPromise; + return refused; + } + + /** The provider ids OpenCode has of its own — built in, configured by + * the operator, signed in to — without the profiles this driver added. + * Empty when the server cannot say (nothing is refused then). */ + private async ownProviderIds(): Promise> { + try { + const client = await this.clientPromise; + if (!client) return new Set(); + const { data, error } = await client.provider.list(); + if (error || !data) throw new Error(JSON.stringify(error ?? 'no provider list')); + return new Set(data.all.map((p) => p.id).filter((id) => !this.servedIds.has(id))); + } catch (err) { + this.options.log(`[opencode] could not list its providers, so profile names are not checked: ${err instanceof Error ? err.message : String(err)}`); + return new Set(); + } } info(): AgentInfo { @@ -1185,10 +1237,14 @@ export class OpenCodeDriver implements Driver { if (error || !data) return { models: [] }; const models: ModelEntry[] = []; const contextLimits: Record = {}; + const profiles = new Map(this.providers.map((p) => [profileProviderId(p), p])); for (const provider of data.providers) { + const profile = profiles.get(provider.id); for (const model of Object.values(provider.models)) { const id = `${provider.id}/${model.id}`; - models.push({ id, label: model.name, provider: provider.name || provider.id }); + const name = provider.name || provider.id; + const group = profile ? profileModelGroup(name, profile.models.find((m) => m.id === model.id)) : name; + models.push({ id, label: model.name, provider: group }); const window = model.limit?.context ?? 0; if (window > 0) contextLimits[id] = window; } diff --git a/packages/agent-host/src/drivers/opencode/providers.ts b/packages/agent-host/src/drivers/opencode/providers.ts index e970a2a7..82a3214a 100644 --- a/packages/agent-host/src/drivers/opencode/providers.ts +++ b/packages/agent-host/src/drivers/opencode/providers.ts @@ -8,9 +8,10 @@ * handed over in its environment, on top of the operator's files (which are * never written): * - * - each profile is the provider `codedeck-`, named after the - * profile, so its models are `codedeck-/` and the phone shows - * them under the profile's name; + * - each profile is a provider named as the user named the profile, its + * id that name in lower case (`CCR` is `ccr`, its models `ccr/`); + * a profile whose id one of OpenCode's own providers already has, or an + * earlier profile, is left out with the reason rather than shadow it; * - each token sits in the server's environment only, referenced from the * config, never in a file; * - the server answers only with a password made for it, since its API @@ -22,17 +23,51 @@ */ import { createHash, randomBytes } from 'node:crypto'; import { providerApiRoot } from '../../sdk/providerModels'; -import type { ProviderBinding } from '../../sdk/types'; +import type { ProviderBinding, ProviderModel, RefusedProvider } from '../../sdk/types'; -/** What a profile's provider id starts with: no OpenCode provider (models.dev - * catalog or the operator's) is named so. */ -export const PROFILE_PROVIDER_PREFIX = 'codedeck-'; /** The user name OpenCode's server expects with its password. */ const SERVER_USERNAME = 'opencode'; -/** The OpenCode provider id of a profile. */ +/** `name` as an OpenCode provider id: lower case, each run of anything but + * letters and digits one `-` (a `/` would split its model ids). */ +function providerIdOf(name: string): string { + return name + .normalize('NFKD') + .replace(/[\u0300-\u036f]/g, '') + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, ''); +} + +/** The OpenCode provider id of a profile: its name's, or its id's when the + * name has no letter or digit. */ export function profileProviderId(profile: ProviderBinding): string { - return `${PROFILE_PROVIDER_PREFIX}${profile.id}`; + return providerIdOf(profile.label) || providerIdOf(profile.id) || 'provider'; +} + +/** Which of `profiles` OpenCode can take, in order: one whose provider id + * is `taken` by a provider OpenCode already has, or by an earlier profile, + * is refused with the reason. */ +export function admitProfiles( + profiles: ProviderBinding[], + taken: ReadonlySet, +): { admitted: ProviderBinding[]; refused: RefusedProvider[] } { + const admitted: ProviderBinding[] = []; + const refused: RefusedProvider[] = []; + const used = new Map(); + for (const profile of profiles) { + const id = profileProviderId(profile); + const earlier = used.get(id); + if (taken.has(id)) { + refused.push({ id: profile.id, reason: `OpenCode already has a provider called '${id}'. Give this profile another name.` }); + } else if (earlier) { + refused.push({ id: profile.id, reason: `The provider profile '${earlier.label}' already goes by '${id}' in OpenCode. Give this one another name.` }); + } else { + used.set(id, profile); + admitted.push(profile); + } + } + return { admitted, refused }; } /** The variable profile `index`'s token reaches the server in. */ @@ -51,13 +86,31 @@ export function providersConfig(profiles: ProviderBinding[], base: Record [m.id, m.label ? { name: m.label } : {}])), + models: Object.fromEntries(profile.models.map((m) => [m.id, modelConfig(m)])), }, ]), ); return { ...base, provider: { ...own, ...added } }; } +/** A profile model as OpenCode's config holds it: named without the + * upstream provider its group already shows. A known context window + * lets OpenCode compact before the model overflows; the output size is + * never listed by endpoints, so it stays 0, OpenCode's own "unknown". */ +function modelConfig(model: ProviderModel): Record { + const name = model.label ?? (model.provider && model.id.startsWith(`${model.provider}/`) ? model.id.slice(model.provider.length + 1) : undefined); + return { + ...(name ? { name } : {}), + ...(model.contextWindow ? { limit: { context: model.contextWindow, output: 0 } } : {}), + }; +} + +/** The group a profile model is listed under: the profile, and the + * provider a gateway routes the model to when the endpoint names one. */ +export function profileModelGroup(profile: string, model: ProviderModel | undefined): string { + return model?.provider ? `${profile} · ${model.provider}` : profile; +} + /** What the server is started with to serve `profiles`, and how its client * signs in. */ export interface ServerSetup { @@ -65,13 +118,17 @@ export interface ServerSetup { headers: Record; } -/** The environment and credentials of a server for `profiles`. `baseEnv` - * is the host's own environment. */ +/** The environment and credentials of a server for `profiles` — and, for + * every model, web search. `baseEnv` is the host's own environment. */ export function serverSetup(profiles: ProviderBinding[], baseEnv: NodeJS.ProcessEnv): ServerSetup { const password = randomBytes(24).toString('hex'); const env: Record = { OPENCODE_SERVER_USERNAME: SERVER_USERNAME, OPENCODE_SERVER_PASSWORD: password, + // OpenCode offers its web search only on its own providers unless this + // is set; with it, every model can search (Exa's keyless endpoint, + // still behind the `websearch` permission). + OPENCODE_ENABLE_EXA: '1', }; if (profiles.length > 0) { env.OPENCODE_CONFIG_CONTENT = JSON.stringify(providersConfig(profiles, operatorConfig(baseEnv))); diff --git a/packages/agent-host/src/generated/protocol.ts b/packages/agent-host/src/generated/protocol.ts index c4d2f054..65517ca0 100644 --- a/packages/agent-host/src/generated/protocol.ts +++ b/packages/agent-host/src/generated/protocol.ts @@ -272,9 +272,11 @@ export type BridgeMessage_Deserialize = } } | /** * The provider profiles of an agent whose catalog entry `supports` - * `providerModels`, all of them: sent after `initialize` and whenever - * one changes. The agent offers their models beside its own. Reply: - * `ack`. + * `providerModels`, all of them, oldest saved first: sent after + * `initialize` and whenever one changes. The agent offers their models + * beside its own. One it cannot add (its name is taken by one of the + * agent's own providers, or by an earlier profile) is left out. Reply: + * `providers-set`. */ { kind: "set-providers"; payload: { agent: string, @@ -434,9 +436,11 @@ export type BridgeMessage_Serialize = } } | /** * The provider profiles of an agent whose catalog entry `supports` - * `providerModels`, all of them: sent after `initialize` and whenever - * one changes. The agent offers their models beside its own. Reply: - * `ack`. + * `providerModels`, all of them, oldest saved first: sent after + * `initialize` and whenever one changes. The agent offers their models + * beside its own. One it cannot add (its name is taken by one of the + * agent's own providers, or by an earlier profile) is left out. Reply: + * `providers-set`. */ { kind: "set-providers"; payload: { agent: string, @@ -763,6 +767,13 @@ export type HostMessage_Deserialize = { kind: "provider-models"; payload: { models: ProviderModel_Deserialize[], } } | +/** + * Reply to `set-providers`: the profiles left out, with the reason in + * words for a person; every other one is offered. + */ +{ kind: "providers-set"; payload: { + refused: RefusedProvider[], +} } | /** A notification: no frame id, no reply. */ { kind: "session-event"; payload: { sessionId: string, @@ -842,6 +853,13 @@ export type HostMessage_Serialize = { kind: "provider-models"; payload: { models: ProviderModel_Serialize[], } } | +/** + * Reply to `set-providers`: the profiles left out, with the reason in + * words for a person; every other one is offered. + */ +{ kind: "providers-set"; payload: { + refused: RefusedProvider[], +} } | /** A notification: no frame id, no reply. */ { kind: "session-event"; payload: { sessionId: string, @@ -1192,16 +1210,33 @@ export type ProviderBinding_Serialize = { defaultModel?: string | null, }; +/** A model a provider profile offers. */ export type ProviderModel = ProviderModel_Serialize | ProviderModel_Deserialize; +/** A model a provider profile offers. */ export type ProviderModel_Deserialize = { id: string, label?: string | null, + /** + * The provider a gateway routes the model to (`OpenCode Go` for a + * router's `OpenCode Go/deepseek-v4.1-flash`), when the endpoint says. + */ + provider?: string | null, + /** How many tokens the model takes in, when the endpoint says. */ + contextWindow?: number | null, }; +/** A model a provider profile offers. */ export type ProviderModel_Serialize = { id: string, label?: string | null, + /** + * The provider a gateway routes the model to (`OpenCode Go` for a + * router's `OpenCode Go/deepseek-v4.1-flash`), when the endpoint says. + */ + provider?: string | null, + /** How many tokens the model takes in, when the endpoint says. */ + contextWindow?: number | null, }; export type QuestionOption = QuestionOption_Serialize | QuestionOption_Deserialize; @@ -1253,6 +1288,13 @@ export type QuestionSpec_Serialize = { multiSelect?: boolean, }; +/** A provider profile an agent left out of `set-providers`. */ +export type RefusedProvider = { + /** The profile's id. */ + id: string, + reason: string, +}; + /** Who wrote a text entry. */ export type Role = "user" | "agent"; diff --git a/packages/agent-host/src/host/host.ts b/packages/agent-host/src/host/host.ts index b359c0a5..12668aa1 100644 --- a/packages/agent-host/src/host/host.ts +++ b/packages/agent-host/src/host/host.ts @@ -238,13 +238,23 @@ export class AgentHost { const driver = this.driver(agent); if (!driver.listProviderModels) throw new Error(`${driver.info().displayName} cannot read a provider's models`); const models = await driver.listProviderModels(baseUrl, authToken); - return { kind: 'provider-models', payload: { models: models.map((m) => ({ id: m.id, ...(m.label ? { label: m.label } : {}) })) } }; + return { + kind: 'provider-models', + payload: { + models: models.map((m) => ({ + id: m.id, + ...(m.label ? { label: m.label } : {}), + ...(m.provider ? { provider: m.provider } : {}), + ...(m.contextWindow !== undefined ? { contextWindow: m.contextWindow } : {}), + })), + }, + }; } case 'set-providers': { const driver = this.driver(message.payload.agent); if (!driver.setProviders) throw new Error(`${driver.info().displayName} does not add provider profiles to its models`); - await driver.setProviders(message.payload.providers); - return ack(); + const refused = await driver.setProviders(message.payload.providers); + return { kind: 'providers-set', payload: { refused } }; } default: throw new Error(`unsupported request ${(message as { kind: string }).kind}`); diff --git a/packages/agent-host/src/sdk/__tests__/tools.test.ts b/packages/agent-host/src/sdk/__tests__/tools.test.ts new file mode 100644 index 00000000..241aa59d --- /dev/null +++ b/packages/agent-host/src/sdk/__tests__/tools.test.ts @@ -0,0 +1,16 @@ +/** + * Tool kinds: each agent's tool names, as the phone groups and labels them. + */ +import { describe, expect, it } from 'vitest'; +import { toolKindOf } from '../tools'; + +describe('toolKindOf', () => { + it("reads every agent's spelling of a tool", () => { + expect(toolKindOf('Edit')).toBe('edit'); + expect(toolKindOf('apply_patch')).toBe('edit'); + expect(toolKindOf('patch')).toBe('edit'); + expect(toolKindOf('lsp')).toBe('read'); + expect(toolKindOf('websearch')).toBe('fetch'); + expect(toolKindOf('something-new')).toBe('other'); + }); +}); diff --git a/packages/agent-host/src/sdk/driver.ts b/packages/agent-host/src/sdk/driver.ts index 38b2396a..7f785c9d 100644 --- a/packages/agent-host/src/sdk/driver.ts +++ b/packages/agent-host/src/sdk/driver.ts @@ -27,6 +27,7 @@ import type { ProviderBinding, QuestionOutcome, QuestionSpec, + RefusedProvider, SelectOutcome, SessionEvent, SessionMcpServer, @@ -152,9 +153,12 @@ export interface Driver { * there is no list. */ listProviderModels?(baseUrl: string, token: string): Promise; /** For an agent whose catalog entry `supports.providerModels`: its - * provider profiles, all of them, whenever one changes. Their models are - * offered beside the agent's own; nothing the agent already has goes. */ - setProviders?(providers: ProviderBinding[]): Promise; + * provider profiles, all of them, oldest saved first, whenever one + * changes. Their models are offered beside the agent's own; nothing the + * agent already has goes. Resolves to the profiles left out (a name one + * of the agent's providers, or an earlier profile, already has), each + * with the reason in words for a person. */ + setProviders?(providers: ProviderBinding[]): Promise; /** * Delete the agent's own record of a conversation (an `info` * `nativeSessionId` it reported, run in `cwd`): its transcript files, or diff --git a/packages/agent-host/src/sdk/tools.ts b/packages/agent-host/src/sdk/tools.ts index 6e4050cb..3cb13202 100644 --- a/packages/agent-host/src/sdk/tools.ts +++ b/packages/agent-host/src/sdk/tools.ts @@ -15,6 +15,7 @@ const TOOL_KINDS: Record = { multiedit: 'edit', notebookedit: 'edit', patch: 'edit', + apply_patch: 'edit', bash: 'execute', bashoutput: 'execute', killshell: 'execute', @@ -30,6 +31,8 @@ const TOOL_KINDS: Record = { enterplanmode: 'switch_mode', task: 'agent', agent: 'agent', + // OpenCode's code-intelligence queries (diagnostics, definitions). + lsp: 'read', // The DeepSeek Harness's own tools. pwsh: 'execute', str_replace_editor: 'edit', diff --git a/packages/agent-host/src/sdk/types.ts b/packages/agent-host/src/sdk/types.ts index 97c3022d..f79a2666 100644 --- a/packages/agent-host/src/sdk/types.ts +++ b/packages/agent-host/src/sdk/types.ts @@ -12,6 +12,8 @@ export type BridgeFrame = G.Frame_Serialize; export type BridgeMessage = G.BridgeMessage_Serialize; export type StartSession = G.StartSession_Serialize; export type ProviderBinding = G.ProviderBinding_Serialize; +export type ProviderModel = G.ProviderModel_Serialize; +export type RefusedProvider = G.RefusedProvider; export type SelectOutcome = G.SelectOutcome; export type PlanOutcome = G.PlanOutcome_Serialize; export type QuestionOutcome = G.QuestionOutcome;