feat(acp): derive and forward thinking effort from the ACP harness (#10949)
Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jasper Hugo <jasper@jasperhugo.com>
This commit is contained in:
@@ -27,7 +27,7 @@ serde = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
strum = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
tokio = { workspace = true, features = ["time"] }
|
||||
tokio = { workspace = true, features = ["sync", "time"] }
|
||||
tracing = { workspace = true }
|
||||
unicode-normalization = { version = "0.1.22", default-features = false, features = ["std"] }
|
||||
uuid = { workspace = true, features = ["v4", "std"] }
|
||||
|
||||
@@ -5,6 +5,7 @@ use serde::{Deserialize, Serialize};
|
||||
use serde_json::Value;
|
||||
use std::collections::HashMap;
|
||||
use std::pin::Pin;
|
||||
use tokio::sync::watch;
|
||||
|
||||
use crate::{
|
||||
canonical::{catalog::ProviderSetupMetadata, map_to_canonical_model, CanonicalModelRegistry},
|
||||
@@ -17,6 +18,7 @@ use crate::{
|
||||
model::ModelConfig,
|
||||
permission::PermissionConfirmation,
|
||||
retry::RetryConfig,
|
||||
thinking::ThinkingEffortSupport,
|
||||
};
|
||||
|
||||
/// Metadata about a provider's configuration requirements and capabilities
|
||||
@@ -646,6 +648,40 @@ pub trait Provider: Send + Sync {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// How this provider participates in thinking-effort selection. Providers
|
||||
/// that manage reasoning through an external harness report the harness's
|
||||
/// advertised capability; the default keeps the model-name-based path.
|
||||
fn thinking_effort_support(&self) -> ThinkingEffortSupport {
|
||||
ThinkingEffortSupport::Unspecified
|
||||
}
|
||||
|
||||
/// Subscribe to provider-managed thinking-effort capability changes.
|
||||
/// Providers without an asynchronous capability source return `None`.
|
||||
fn subscribe_thinking_effort_support(&self) -> Option<watch::Receiver<ThinkingEffortSupport>> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Forward a thinking-effort selection to the provider. Returns `Ok(true)`
|
||||
/// when the provider applied the value itself (no provider recreation
|
||||
/// needed); `Ok(false)` when the caller should use the legacy path.
|
||||
async fn set_thinking_effort(
|
||||
&self,
|
||||
_session_id: &str,
|
||||
_value: &str,
|
||||
) -> Result<bool, ProviderError> {
|
||||
Ok(false)
|
||||
}
|
||||
|
||||
/// Apply a session's model selection after the provider is installed.
|
||||
/// Providers that manage their own model (e.g. ACP harnesses) override
|
||||
/// this to sync the selection before the first prompt.
|
||||
async fn apply_model_selection(
|
||||
&self,
|
||||
_model_config: &ModelConfig,
|
||||
) -> Result<(), ProviderError> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn permission_routing(&self) -> PermissionRouting {
|
||||
PermissionRouting::Noop
|
||||
}
|
||||
|
||||
@@ -30,6 +30,11 @@ pub enum ProviderError {
|
||||
#[error("Request failed: {0}")]
|
||||
RequestFailed(String),
|
||||
|
||||
/// Bad input rather than an operational failure: retrying is pointless, but
|
||||
/// a different value may succeed.
|
||||
#[error("Invalid value: {0}")]
|
||||
InvalidValue(String),
|
||||
|
||||
#[error("Execution error: {0}")]
|
||||
ExecutionError(String),
|
||||
|
||||
@@ -69,6 +74,7 @@ impl ProviderError {
|
||||
ProviderError::ServerError(_) => "server",
|
||||
ProviderError::NetworkError(_) => "network",
|
||||
ProviderError::RequestFailed(_) => "request",
|
||||
ProviderError::InvalidValue(_) => "invalid_value",
|
||||
ProviderError::ExecutionError(_) => "execution",
|
||||
ProviderError::UsageError(_) => "usage",
|
||||
ProviderError::NotImplemented(_) => "not_implemented",
|
||||
|
||||
@@ -213,7 +213,10 @@ impl ModelConfig {
|
||||
}
|
||||
|
||||
pub fn with_default_thinking_effort(mut self, effort: Option<ThinkingEffort>) -> Self {
|
||||
if self.thinking_effort().is_none() {
|
||||
// Guard on raw-param presence rather than parseability: a persisted
|
||||
// harness value like "default" doesn't parse into ThinkingEffort but
|
||||
// is still an explicit user pick that must not be overwritten.
|
||||
if self.request_param::<String>("thinking_effort").is_none() {
|
||||
if let Some(effort) = effort {
|
||||
self = self.with_thinking_effort(effort);
|
||||
}
|
||||
@@ -387,6 +390,31 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn with_default_thinking_effort_preserves_unparseable_raw_param() {
|
||||
let config = config_with_params(
|
||||
"test",
|
||||
HashMap::from([("thinking_effort".to_string(), serde_json::json!("default"))]),
|
||||
)
|
||||
.with_default_thinking_effort(Some(ThinkingEffort::High));
|
||||
|
||||
assert_eq!(
|
||||
config
|
||||
.request_params
|
||||
.as_ref()
|
||||
.and_then(|params| params.get("thinking_effort")),
|
||||
Some(&serde_json::json!("default"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn with_default_thinking_effort_applies_when_absent() {
|
||||
let config =
|
||||
ModelConfig::new("test").with_default_thinking_effort(Some(ThinkingEffort::High));
|
||||
|
||||
assert_eq!(config.thinking_effort(), Some(ThinkingEffort::High));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_explicit_thinking_effort() {
|
||||
let previous = config_with_params(
|
||||
|
||||
@@ -338,6 +338,34 @@ impl fmt::Display for ThinkingEffort {
|
||||
}
|
||||
}
|
||||
|
||||
/// A single selectable effort value advertised by a provider-managed harness.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ThinkingEffortOption {
|
||||
pub value: String,
|
||||
pub label: String,
|
||||
}
|
||||
|
||||
/// A harness-advertised effort config option mirrored verbatim into goose's
|
||||
/// `thinking_effort` session option.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct ThinkingEffortCapability {
|
||||
/// The harness's config option id, e.g. "effort".
|
||||
pub option_id: String,
|
||||
pub values: Vec<ThinkingEffortOption>,
|
||||
pub current: Option<String>,
|
||||
}
|
||||
|
||||
/// How a provider participates in thinking-effort selection.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub enum ThinkingEffortSupport {
|
||||
/// The provider doesn't manage effort; callers keep the model-name-based path.
|
||||
Unspecified,
|
||||
/// The provider manages reasoning itself but has no effort knob for the current model.
|
||||
Unsupported,
|
||||
/// The provider passes through a harness-advertised effort option.
|
||||
Options(ThinkingEffortCapability),
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,4 +1,4 @@
|
||||
use crate::agents::ExtensionLoadResult;
|
||||
use crate::agents::{Agent, ExtensionLoadResult};
|
||||
use crate::config::{Config, GooseMode};
|
||||
use crate::providers::inventory::{ProviderInventoryEntry, ProviderInventoryService};
|
||||
use crate::session::session_manager::SessionUsageTotals;
|
||||
@@ -11,10 +11,11 @@ use agent_client_protocol::schema::v1::{
|
||||
};
|
||||
use agent_client_protocol::{Client, ConnectionTo};
|
||||
use goose_providers::model::ModelConfig;
|
||||
use goose_providers::thinking::ThinkingEffort;
|
||||
use goose_providers::thinking::{ThinkingEffort, ThinkingEffortCapability, ThinkingEffortSupport};
|
||||
use serde::Serialize;
|
||||
use strum::{EnumMessage, VariantNames};
|
||||
|
||||
use super::provider::resolve_effort_value;
|
||||
use super::server::{build_usage_updates, DEFAULT_PROVIDER_ID, DEFAULT_PROVIDER_LABEL};
|
||||
|
||||
pub(super) fn session_provider_selection(session: &Session) -> &str {
|
||||
@@ -232,9 +233,19 @@ pub(super) fn build_mode_state(
|
||||
))
|
||||
}
|
||||
|
||||
/// The provider decides whether goose owns the effort menu; a session without a
|
||||
/// live provider keeps the model-name based path.
|
||||
pub(super) async fn agent_thinking_effort_support(agent: &Agent) -> ThinkingEffortSupport {
|
||||
match agent.provider().await {
|
||||
Ok(provider) => provider.thinking_effort_support(),
|
||||
Err(_) => ThinkingEffortSupport::Unspecified,
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) async fn build_session_setup_config(
|
||||
provider_inventory: &ProviderInventoryService,
|
||||
session: &Session,
|
||||
effort_support: &ThinkingEffortSupport,
|
||||
) -> Result<(SessionModeState, Option<Vec<SessionConfigOption>>), agent_client_protocol::Error> {
|
||||
let mode_state = build_mode_state(session.goose_mode)?;
|
||||
|
||||
@@ -259,6 +270,7 @@ pub(super) async fn build_session_setup_config(
|
||||
model_config,
|
||||
provider_selection,
|
||||
provider_options,
|
||||
effort_support,
|
||||
);
|
||||
Ok((mode_state, Some(config_options)))
|
||||
}
|
||||
@@ -269,6 +281,7 @@ pub(super) fn build_config_options(
|
||||
model_config: &ModelConfig,
|
||||
provider_selection: &str,
|
||||
provider_options: Vec<SessionConfigSelectOption>,
|
||||
effort_support: &ThinkingEffortSupport,
|
||||
) -> Vec<SessionConfigOption> {
|
||||
let mode_options: Vec<SessionConfigSelectOption> = mode_state
|
||||
.available_modes
|
||||
@@ -283,14 +296,8 @@ pub(super) fn build_config_options(
|
||||
.iter()
|
||||
.map(|m| SessionConfigSelectOption::new(m.id.clone(), m.name.clone()))
|
||||
.collect();
|
||||
let thinking_effort_options = thinking_effort_values(model_config)
|
||||
.iter()
|
||||
.map(|effort| {
|
||||
let effort = effort.to_string();
|
||||
SessionConfigSelectOption::new(effort.clone(), effort)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let current_thinking_effort = current_thinking_effort_value(model_config);
|
||||
let (thinking_effort_options, current_thinking_effort) =
|
||||
build_thinking_effort_choices(model_config, effort_support);
|
||||
vec![
|
||||
SessionConfigOption::select(
|
||||
"provider",
|
||||
@@ -323,6 +330,59 @@ pub(super) fn build_config_options(
|
||||
]
|
||||
}
|
||||
|
||||
/// A provider that manages its own reasoning decides the menu: it mirrors the
|
||||
/// agent's effort selector verbatim, or honestly offers nothing when the agent
|
||||
/// has no such knob for the current model. Only providers that leave effort to
|
||||
/// goose fall back to the model-name based values, which the ACP sentinel model
|
||||
/// name can never satisfy.
|
||||
fn build_thinking_effort_choices(
|
||||
model_config: &ModelConfig,
|
||||
effort_support: &ThinkingEffortSupport,
|
||||
) -> (Vec<SessionConfigSelectOption>, String) {
|
||||
match effort_support {
|
||||
ThinkingEffortSupport::Options(capability) => (
|
||||
capability
|
||||
.values
|
||||
.iter()
|
||||
.map(|option| {
|
||||
SessionConfigSelectOption::new(option.value.clone(), option.label.clone())
|
||||
})
|
||||
.collect(),
|
||||
capability_thinking_effort_value(capability, model_config),
|
||||
),
|
||||
ThinkingEffortSupport::Unsupported => {
|
||||
let off = ThinkingEffort::Off.to_string();
|
||||
(
|
||||
vec![SessionConfigSelectOption::new(off.clone(), off.clone())],
|
||||
off,
|
||||
)
|
||||
}
|
||||
ThinkingEffortSupport::Unspecified => (
|
||||
thinking_effort_values(model_config)
|
||||
.iter()
|
||||
.map(|effort| {
|
||||
let effort = effort.to_string();
|
||||
SessionConfigSelectOption::new(effort.clone(), effort)
|
||||
})
|
||||
.collect(),
|
||||
current_thinking_effort_value(model_config),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// The goose-side value that will actually be sent wins — `resolve_effort_value`
|
||||
/// is shared with the provider's send path — then whatever the agent currently
|
||||
/// has, which is what it keeps when goose sends nothing.
|
||||
fn capability_thinking_effort_value(
|
||||
capability: &ThinkingEffortCapability,
|
||||
model_config: &ModelConfig,
|
||||
) -> String {
|
||||
resolve_effort_value(capability, model_config)
|
||||
.or_else(|| capability.current.clone())
|
||||
.or_else(|| capability.values.first().map(|option| option.value.clone()))
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn thinking_effort_values(model_config: &ModelConfig) -> &'static [ThinkingEffort] {
|
||||
if model_config.is_reasoning_model() {
|
||||
&[
|
||||
@@ -423,8 +483,10 @@ pub(super) fn send_session_setup_notifications(
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::super::provider::THINKING_EFFORT_PARAM;
|
||||
use super::*;
|
||||
use agent_client_protocol::schema::v1::SessionConfigKind;
|
||||
use goose_providers::thinking::ThinkingEffortOption;
|
||||
use test_case::test_case;
|
||||
|
||||
fn model_selection(current: &str, models: &[&str]) -> ModelSelection {
|
||||
@@ -660,6 +722,7 @@ mod tests {
|
||||
&model_config,
|
||||
provider_name,
|
||||
provider_options,
|
||||
&ThinkingEffortSupport::Unspecified,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -680,6 +743,7 @@ mod tests {
|
||||
&model_config,
|
||||
"openai",
|
||||
vec![SessionConfigSelectOption::new("openai", "openai")],
|
||||
&ThinkingEffortSupport::Unspecified,
|
||||
);
|
||||
let option = options
|
||||
.iter()
|
||||
@@ -709,6 +773,7 @@ mod tests {
|
||||
&model_config,
|
||||
"openai",
|
||||
vec![SessionConfigSelectOption::new("openai", "openai")],
|
||||
&ThinkingEffortSupport::Unspecified,
|
||||
);
|
||||
let option = options
|
||||
.iter()
|
||||
@@ -727,4 +792,123 @@ mod tests {
|
||||
])
|
||||
);
|
||||
}
|
||||
|
||||
fn effort_capability(values: &[&str], current: &str) -> ThinkingEffortCapability {
|
||||
ThinkingEffortCapability {
|
||||
option_id: "effort".to_string(),
|
||||
values: values
|
||||
.iter()
|
||||
.map(|value| ThinkingEffortOption {
|
||||
value: value.to_string(),
|
||||
label: value.to_string(),
|
||||
})
|
||||
.collect(),
|
||||
current: Some(current.to_string()),
|
||||
}
|
||||
}
|
||||
|
||||
fn build_effort_option(
|
||||
model_name: &str,
|
||||
persisted_effort: Option<&str>,
|
||||
effort_support: &ThinkingEffortSupport,
|
||||
) -> (String, Vec<SessionConfigSelectOption>) {
|
||||
let mode_state = build_mode_state(GooseMode::Auto).unwrap();
|
||||
let model_state = model_selection(model_name, &[model_name]);
|
||||
let mut model_config = ModelConfig::new(model_name);
|
||||
if let Some(effort) = persisted_effort {
|
||||
model_config =
|
||||
model_config.with_merged_request_params(std::collections::HashMap::from([(
|
||||
THINKING_EFFORT_PARAM.to_string(),
|
||||
serde_json::json!(effort),
|
||||
)]));
|
||||
}
|
||||
|
||||
let options = build_config_options(
|
||||
&mode_state,
|
||||
&model_state,
|
||||
&model_config,
|
||||
"claude-acp",
|
||||
vec![SessionConfigSelectOption::new("claude-acp", "claude-acp")],
|
||||
effort_support,
|
||||
);
|
||||
let option = options
|
||||
.into_iter()
|
||||
.find(|option| option.id.0.as_ref() == "thinking_effort")
|
||||
.expect("thinking_effort option");
|
||||
let SessionConfigKind::Select(select) = option.kind else {
|
||||
panic!("thinking_effort should be a select option");
|
||||
};
|
||||
let values = match select.options {
|
||||
agent_client_protocol::schema::v1::SessionConfigSelectOptions::Ungrouped(values) => {
|
||||
values
|
||||
}
|
||||
grouped => panic!("unexpected thinking_effort options: {grouped:?}"),
|
||||
};
|
||||
(select.current_value.0.to_string(), values)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_config_options_mirrors_agent_effort_menu_on_the_model_sentinel() {
|
||||
let model_name = crate::acp::ACP_CURRENT_MODEL;
|
||||
assert!(
|
||||
!ModelConfig::new(model_name).is_reasoning_model(),
|
||||
"the sentinel is the model name that made the old sniffing path collapse the menu"
|
||||
);
|
||||
let support = ThinkingEffortSupport::Options(effort_capability(
|
||||
&["default", "high", "xhigh"],
|
||||
"high",
|
||||
));
|
||||
|
||||
let (current, values) = build_effort_option(model_name, Some("high"), &support);
|
||||
|
||||
assert_eq!(current, "high");
|
||||
assert_eq!(
|
||||
values,
|
||||
vec![
|
||||
SessionConfigSelectOption::new("default", "default"),
|
||||
SessionConfigSelectOption::new("high", "high"),
|
||||
SessionConfigSelectOption::new("xhigh", "xhigh"),
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test_case(Some("high") => "high".to_string() ; "persisted value")]
|
||||
#[test_case(Some("off") => "default".to_string() ; "persisted off maps onto the agent default")]
|
||||
#[test_case(Some("max") => "xhigh".to_string() ; "persisted max maps onto the agent's xhigh")]
|
||||
#[test_case(Some("unknown") => "low".to_string() ; "unmappable persisted value falls back to the agent")]
|
||||
#[test_case(None => "low".to_string() ; "no persisted value falls back to the agent")]
|
||||
fn test_build_config_options_effort_current_value(persisted: Option<&str>) -> String {
|
||||
// Unoffered by the capability below, so the global default never wins
|
||||
// and the test doesn't depend on the machine's configured value.
|
||||
let _guard = env_lock::lock_env([("GOOSE_THINKING_EFFORT", Some("medium"))]);
|
||||
let support = ThinkingEffortSupport::Options(effort_capability(
|
||||
&["default", "low", "high", "xhigh"],
|
||||
"low",
|
||||
));
|
||||
|
||||
build_effort_option(crate::acp::ACP_CURRENT_MODEL, persisted, &support).0
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_config_options_effort_falls_back_to_the_global_default() {
|
||||
let _guard = env_lock::lock_env([("GOOSE_THINKING_EFFORT", Some("high"))]);
|
||||
let support =
|
||||
ThinkingEffortSupport::Options(effort_capability(&["default", "high"], "default"));
|
||||
|
||||
let (current, _) = build_effort_option(crate::acp::ACP_CURRENT_MODEL, None, &support);
|
||||
|
||||
assert_eq!(current, "high");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_config_options_offers_no_effort_when_the_agent_has_none() {
|
||||
let (current, values) = build_effort_option(
|
||||
"claude-sonnet-4",
|
||||
Some("high"),
|
||||
&ThinkingEffortSupport::Unsupported,
|
||||
);
|
||||
|
||||
assert_eq!(current, "off");
|
||||
assert_eq!(values, vec![SessionConfigSelectOption::new("off", "off")]);
|
||||
}
|
||||
}
|
||||
|
||||
+332
-14
@@ -2,9 +2,10 @@ use crate::acp::custom_notifications::*;
|
||||
use crate::acp::custom_requests::*;
|
||||
use crate::acp::fs::AcpTools;
|
||||
pub(super) use crate::acp::response_builder::{
|
||||
build_config_options, build_mode_state, build_model_state, build_provider_options,
|
||||
build_session_info, build_session_setup_config, send_session_setup_notifications, session_meta,
|
||||
session_provider_selection, session_response_meta, should_refresh_inventory_for_session_init,
|
||||
agent_thinking_effort_support, build_config_options, build_mode_state, build_model_state,
|
||||
build_provider_options, build_session_info, build_session_setup_config,
|
||||
send_session_setup_notifications, session_meta, session_provider_selection,
|
||||
session_response_meta, should_refresh_inventory_for_session_init,
|
||||
};
|
||||
use crate::acp::tool_call_notifier::ToolCallNotifier;
|
||||
use crate::acp::{PermissionDecision, ACP_CURRENT_MODEL};
|
||||
@@ -23,6 +24,7 @@ use crate::conversation::message::{
|
||||
ActionRequiredData, Message, MessageContent, SystemNotificationContent, SystemNotificationType,
|
||||
ToolRequest, ToolResponse,
|
||||
};
|
||||
use crate::conversation::Conversation;
|
||||
use crate::execution::manager::{AgentManager, AgentManagerGetResult, RuntimeContext};
|
||||
use crate::permission::permission_confirmation::PrincipalType;
|
||||
use crate::permission::{Permission, PermissionConfirmation};
|
||||
@@ -63,6 +65,7 @@ use anyhow::Result;
|
||||
use fs_err as fs;
|
||||
use futures::future::{BoxFuture, FutureExt};
|
||||
use futures::stream::{self, StreamExt};
|
||||
use goose_providers::errors::ProviderError;
|
||||
use rmcp::model::{
|
||||
Annotations as RmcpAnnotations, ImageContent as RmcpImageContent, Role,
|
||||
TextContent as RmcpTextContent,
|
||||
@@ -72,7 +75,7 @@ use std::collections::{HashMap, HashSet};
|
||||
use std::panic::AssertUnwindSafe;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
use tokio::sync::{Mutex, OnceCell};
|
||||
use tokio::sync::{mpsc, Mutex, OnceCell};
|
||||
use tokio_util::compat::{TokioAsyncReadCompatExt as _, TokioAsyncWriteCompatExt as _};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
use tracing::{debug, error, info, warn};
|
||||
@@ -170,6 +173,41 @@ fn agent_creation_error(error: anyhow::Error, context: &str) -> agent_client_pro
|
||||
}
|
||||
}
|
||||
|
||||
/// Only a value the client could usefully change is `invalid_params`; everything
|
||||
/// else (a dead agent subprocess, a failed persist, a failed provider respawn) is
|
||||
/// an operational failure the client cannot fix by picking differently.
|
||||
fn thinking_effort_error(error: anyhow::Error) -> agent_client_protocol::Error {
|
||||
let base = match error.downcast_ref::<ProviderError>() {
|
||||
Some(ProviderError::InvalidValue(_)) => agent_client_protocol::Error::invalid_params(),
|
||||
_ => agent_client_protocol::Error::internal_error(),
|
||||
};
|
||||
// `{error:#}` rather than `{error}`: context layering hides the cause chain,
|
||||
// including the variant this mapping branched on.
|
||||
base.data(format!("Failed to update thinking effort: {error:#}"))
|
||||
}
|
||||
|
||||
async fn resume_saved_provider_session(
|
||||
provider: &Arc<dyn Provider>,
|
||||
conversation: Option<&Conversation>,
|
||||
) {
|
||||
let Some(conversation) = conversation else {
|
||||
return;
|
||||
};
|
||||
let provider_name = provider.get_name();
|
||||
let Some(session_id) =
|
||||
crate::agents::latest_provider_session_id(conversation.messages(), provider_name)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
if let Err(error) = provider.resume(session_id).await {
|
||||
warn!(
|
||||
provider = provider_name,
|
||||
%error,
|
||||
"Could not resume provider session during ACP session setup"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) const DEFAULT_PROVIDER_ID: &str = "goose";
|
||||
pub(super) const DEFAULT_PROVIDER_LABEL: &str = "Goose (Default)";
|
||||
const PROVIDER_CONFIG_STATUS_CHECK_CONCURRENCY: usize = 16;
|
||||
@@ -243,6 +281,8 @@ pub struct GooseAcpAgent {
|
||||
client_requests_tool_call_label_enrichment: OnceCell<bool>,
|
||||
use_login_shell_path: OnceCell<bool>,
|
||||
client_cx: OnceCell<ConnectionTo<Client>>,
|
||||
thinking_effort_update_tx: mpsc::UnboundedSender<String>,
|
||||
thinking_effort_update_rx: Mutex<Option<mpsc::UnboundedReceiver<String>>>,
|
||||
config_dir: std::path::PathBuf,
|
||||
session_manager: Arc<SessionManager>,
|
||||
permission_manager: Arc<PermissionManager>,
|
||||
@@ -779,6 +819,7 @@ impl GooseAcpAgent {
|
||||
options.goose_platform.clone(),
|
||||
);
|
||||
let agent_manager = Arc::new(AgentManager::new(agent_config, None).await?);
|
||||
let (thinking_effort_update_tx, thinking_effort_update_rx) = mpsc::unbounded_channel();
|
||||
|
||||
Ok(Self {
|
||||
sessions: Arc::new(Mutex::new(HashMap::new())),
|
||||
@@ -796,6 +837,8 @@ impl GooseAcpAgent {
|
||||
client_requests_tool_call_label_enrichment: OnceCell::new(),
|
||||
use_login_shell_path: OnceCell::new(),
|
||||
client_cx: OnceCell::new(),
|
||||
thinking_effort_update_tx,
|
||||
thinking_effort_update_rx: Mutex::new(Some(thinking_effort_update_rx)),
|
||||
config_dir: options.config_dir,
|
||||
session_manager,
|
||||
permission_manager,
|
||||
@@ -1038,8 +1081,70 @@ impl GooseAcpAgent {
|
||||
}
|
||||
|
||||
async fn register_acp_session(&self, session_id: String, agent: Arc<Agent>) {
|
||||
let acp_session = GooseAcpSession { agent };
|
||||
self.sessions.lock().await.insert(session_id, acp_session);
|
||||
let acp_session = GooseAcpSession {
|
||||
agent: agent.clone(),
|
||||
};
|
||||
self.sessions
|
||||
.lock()
|
||||
.await
|
||||
.insert(session_id.clone(), acp_session);
|
||||
self.subscribe_thinking_effort_updates(&session_id, &agent)
|
||||
.await;
|
||||
}
|
||||
|
||||
async fn subscribe_thinking_effort_updates(&self, session_id: &str, agent: &Arc<Agent>) {
|
||||
let Ok(provider) = agent.provider().await else {
|
||||
return;
|
||||
};
|
||||
let Some(mut updates) = provider.subscribe_thinking_effort_support() else {
|
||||
return;
|
||||
};
|
||||
let session_id = session_id.to_string();
|
||||
let tx = self.thinking_effort_update_tx.clone();
|
||||
tokio::spawn(async move {
|
||||
while updates.changed().await.is_ok() {
|
||||
if tx.send(session_id.clone()).is_err() {
|
||||
break;
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
async fn start_thinking_effort_update_forwarder(self: &Arc<Self>, cx: &ConnectionTo<Client>) {
|
||||
let Some(mut updates) = self.thinking_effort_update_rx.lock().await.take() else {
|
||||
return;
|
||||
};
|
||||
let agent = Arc::downgrade(self);
|
||||
let cx = cx.clone();
|
||||
tokio::spawn(async move {
|
||||
while let Some(session_id) = updates.recv().await {
|
||||
let Some(agent) = agent.upgrade() else {
|
||||
break;
|
||||
};
|
||||
if agent.closed_session_ids.lock().await.contains(&session_id) {
|
||||
continue;
|
||||
}
|
||||
let session_id = SessionId::new(session_id);
|
||||
match agent.build_config_update(&session_id).await {
|
||||
Ok((notification, _)) => {
|
||||
if let Err(error) = cx.send_notification(notification) {
|
||||
warn!(
|
||||
session_id = %session_id,
|
||||
%error,
|
||||
"Failed to forward thinking-effort config update"
|
||||
);
|
||||
}
|
||||
}
|
||||
Err(error) => {
|
||||
warn!(
|
||||
session_id = %session_id,
|
||||
?error,
|
||||
"Failed to build thinking-effort config update"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
async fn activate_acp_session(
|
||||
@@ -2128,6 +2233,8 @@ impl GooseAcpAgent {
|
||||
.recreate_provider_for_session(session_id, &provider_name, model_config)
|
||||
.await
|
||||
.internal_err_ctx("Failed to recreate provider")?;
|
||||
self.subscribe_thinking_effort_updates(session_id, &agent)
|
||||
.await;
|
||||
// model_config is already updated on the session by the agent's update_provider call.
|
||||
Ok(())
|
||||
}
|
||||
@@ -2171,6 +2278,7 @@ impl GooseAcpAgent {
|
||||
¤t_model_config,
|
||||
session_provider_selection(&session),
|
||||
provider_options,
|
||||
&provider.thinking_effort_support(),
|
||||
);
|
||||
let notification = SessionNotification::new(
|
||||
session_id.clone(),
|
||||
@@ -2205,17 +2313,11 @@ impl GooseAcpAgent {
|
||||
session_id: &str,
|
||||
effort_id: &str,
|
||||
) -> Result<(), agent_client_protocol::Error> {
|
||||
let effort = effort_id
|
||||
.parse::<goose_providers::thinking::ThinkingEffort>()
|
||||
.map_err(|_| {
|
||||
agent_client_protocol::Error::invalid_params()
|
||||
.data(format!("Invalid thinking effort: {}", effort_id))
|
||||
})?;
|
||||
let agent = self.get_session_agent(session_id).await?;
|
||||
agent
|
||||
.update_thinking_effort(session_id, effort)
|
||||
.update_thinking_effort(session_id, effort_id)
|
||||
.await
|
||||
.internal_err_ctx("Failed to update thinking effort")?;
|
||||
.map_err(thinking_effort_error)?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -2279,6 +2381,8 @@ impl GooseAcpAgent {
|
||||
.recreate_provider_for_session(session_id, &resolved_provider_name, model_config)
|
||||
.await
|
||||
.internal_err_ctx("Failed to recreate provider")?;
|
||||
self.subscribe_thinking_effort_updates(session_id, &agent)
|
||||
.await;
|
||||
|
||||
// provider_name is already updated on the session by the agent's update_provider call.
|
||||
Ok(())
|
||||
@@ -2415,11 +2519,71 @@ mod tests {
|
||||
SelectedPermissionOutcome, TextResourceContents,
|
||||
};
|
||||
use goose_providers::conversation::token_usage::Usage as TokenUsage;
|
||||
use goose_providers::thinking::{
|
||||
ThinkingEffortCapability, ThinkingEffortOption, ThinkingEffortSupport,
|
||||
};
|
||||
use std::io::Write;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::NamedTempFile;
|
||||
use test_case::test_case;
|
||||
|
||||
#[derive(Debug)]
|
||||
struct AsyncEffortProvider {
|
||||
updates: tokio::sync::watch::Sender<ThinkingEffortSupport>,
|
||||
}
|
||||
|
||||
impl AsyncEffortProvider {
|
||||
fn new() -> Self {
|
||||
let (updates, _) = tokio::sync::watch::channel(Self::support("low", &["low", "high"]));
|
||||
Self { updates }
|
||||
}
|
||||
|
||||
fn support(current: &str, values: &[&str]) -> ThinkingEffortSupport {
|
||||
ThinkingEffortSupport::Options(ThinkingEffortCapability {
|
||||
option_id: "effort".to_string(),
|
||||
values: values
|
||||
.iter()
|
||||
.map(|value| ThinkingEffortOption {
|
||||
value: value.to_string(),
|
||||
label: value.to_string(),
|
||||
})
|
||||
.collect(),
|
||||
current: Some(current.to_string()),
|
||||
})
|
||||
}
|
||||
|
||||
fn update(&self, current: &str, values: &[&str]) {
|
||||
self.updates.send_replace(Self::support(current, values));
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl Provider for AsyncEffortProvider {
|
||||
fn get_name(&self) -> &str {
|
||||
"openai"
|
||||
}
|
||||
|
||||
fn thinking_effort_support(&self) -> ThinkingEffortSupport {
|
||||
self.updates.borrow().clone()
|
||||
}
|
||||
|
||||
fn subscribe_thinking_effort_support(
|
||||
&self,
|
||||
) -> Option<tokio::sync::watch::Receiver<ThinkingEffortSupport>> {
|
||||
Some(self.updates.subscribe())
|
||||
}
|
||||
|
||||
async fn stream(
|
||||
&self,
|
||||
_model_config: &goose_providers::model::ModelConfig,
|
||||
_system: &str,
|
||||
_messages: &[Message],
|
||||
_tools: &[rmcp::model::Tool],
|
||||
) -> Result<crate::providers::base::MessageStream, ProviderError> {
|
||||
Ok(Box::pin(futures::stream::empty()))
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn agent_creation_auth_error_maps_to_auth_required() {
|
||||
let error = anyhow::Error::new(agent_client_protocol::Error::auth_required());
|
||||
@@ -3143,4 +3307,158 @@ print(\"hello, world\")
|
||||
.and_then(|goose| goose.tool_call_label_enrichment)
|
||||
.unwrap_or(false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thinking_effort_error_maps_a_rejected_value_to_invalid_params() {
|
||||
let error = thinking_effort_error(
|
||||
anyhow::Error::new(ProviderError::InvalidValue(
|
||||
"Agent offers no thinking effort 'medium'".to_string(),
|
||||
))
|
||||
.context("Provider rejected thinking effort update"),
|
||||
);
|
||||
|
||||
assert_eq!(error.code, agent_client_protocol::ErrorCode::InvalidParams);
|
||||
// The cause chain, not just the outermost context, reaches the client.
|
||||
let data = error.data.unwrap().to_string();
|
||||
assert!(data.contains("Provider rejected thinking effort update"));
|
||||
assert!(data.contains("Agent offers no thinking effort 'medium'"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thinking_effort_error_maps_an_operational_failure_to_internal_error() {
|
||||
let error = thinking_effort_error(
|
||||
anyhow::Error::new(ProviderError::RequestFailed(
|
||||
"Failed to set ACP effort option: agent is gone".to_string(),
|
||||
))
|
||||
.context("Provider rejected thinking effort update"),
|
||||
);
|
||||
|
||||
assert_eq!(error.code, agent_client_protocol::ErrorCode::InternalError);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thinking_effort_error_maps_an_untyped_failure_to_internal_error() {
|
||||
let error = thinking_effort_error(anyhow::anyhow!("Failed to persist thinking effort"));
|
||||
|
||||
assert_eq!(error.code, agent_client_protocol::ErrorCode::InternalError);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn asynchronous_provider_effort_update_is_forwarded_to_client() {
|
||||
let root = tempfile::tempdir().unwrap();
|
||||
let provider_factory: AcpProviderFactory = Arc::new(
|
||||
|_provider_name, _extensions, _working_dir, _use_default_model| {
|
||||
Box::pin(async { Err(anyhow::anyhow!("unused provider factory")) })
|
||||
},
|
||||
);
|
||||
let server = Arc::new(
|
||||
GooseAcpAgent::new(GooseAcpAgentOptions {
|
||||
provider_factory,
|
||||
builtin_selection: AcpBuiltinSelection::default(),
|
||||
data_dir: root.path().to_path_buf(),
|
||||
config_dir: root.path().to_path_buf(),
|
||||
disable_session_naming: true,
|
||||
goose_platform: GoosePlatform::GooseCli,
|
||||
additional_source_roots: Vec::new(),
|
||||
scheduler: None,
|
||||
})
|
||||
.await
|
||||
.unwrap(),
|
||||
);
|
||||
let session = server
|
||||
.session_manager
|
||||
.create_session(
|
||||
root.path().to_path_buf(),
|
||||
"Effort update test".to_string(),
|
||||
SessionType::Acp,
|
||||
GooseMode::Auto,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let session_agent = Arc::new(Agent::with_config(AgentConfig::new(
|
||||
server.session_manager.clone(),
|
||||
server.permission_manager.clone(),
|
||||
None,
|
||||
GooseMode::Auto,
|
||||
true,
|
||||
GoosePlatform::GooseCli,
|
||||
)));
|
||||
let provider = Arc::new(AsyncEffortProvider::new());
|
||||
session_agent
|
||||
.update_provider(
|
||||
provider.clone(),
|
||||
goose_providers::model::ModelConfig::new("gpt-4o").with_merged_request_params(
|
||||
HashMap::from([("thinking_effort".to_string(), serde_json::json!("xhigh"))]),
|
||||
),
|
||||
&session.id,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
server
|
||||
.register_acp_session(session.id.clone(), session_agent)
|
||||
.await;
|
||||
|
||||
let (client_read, server_write) = tokio::io::duplex(64 * 1024);
|
||||
let (server_read, client_write) = tokio::io::duplex(64 * 1024);
|
||||
let (notification_tx, mut notification_rx) =
|
||||
mpsc::unbounded_channel::<SessionNotification>();
|
||||
let client = tokio::spawn(async move {
|
||||
Client
|
||||
.builder()
|
||||
.on_receive_notification(
|
||||
async move |notification: SessionNotification, _cx| {
|
||||
let _ = notification_tx.send(notification);
|
||||
Ok(())
|
||||
},
|
||||
agent_client_protocol::on_receive_notification!(),
|
||||
)
|
||||
.connect_to(ByteStreams::new(
|
||||
client_write.compat_write(),
|
||||
client_read.compat(),
|
||||
))
|
||||
.await
|
||||
});
|
||||
|
||||
let session_id = SessionId::new(session.id);
|
||||
let server_for_connection = server.clone();
|
||||
SacpAgent
|
||||
.builder()
|
||||
.name("effort-update-test")
|
||||
.connect_with(
|
||||
ByteStreams::new(server_write.compat_write(), server_read.compat()),
|
||||
async move |cx: ConnectionTo<Client>| {
|
||||
server_for_connection
|
||||
.start_thinking_effort_update_forwarder(&cx)
|
||||
.await;
|
||||
provider.update("xhigh", &["default", "high", "xhigh"]);
|
||||
|
||||
let notification = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(1),
|
||||
notification_rx.recv(),
|
||||
)
|
||||
.await
|
||||
.expect("timed out waiting for effort update")
|
||||
.expect("client notification channel closed");
|
||||
assert_eq!(notification.session_id, session_id);
|
||||
let SessionUpdate::ConfigOptionUpdate(update) = notification.update else {
|
||||
panic!("expected config option update");
|
||||
};
|
||||
let option = update
|
||||
.config_options
|
||||
.iter()
|
||||
.find(|option| option.id.0.as_ref() == "thinking_effort")
|
||||
.expect("thinking_effort option");
|
||||
let agent_client_protocol::schema::v1::SessionConfigKind::Select(select) =
|
||||
&option.kind
|
||||
else {
|
||||
panic!("thinking_effort should be a select option");
|
||||
};
|
||||
assert_eq!(select.current_value.0.as_ref(), "xhigh");
|
||||
Ok(())
|
||||
},
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
client.await.unwrap().unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,6 +22,7 @@ impl HandleDispatchFrom<Client> for GooseAcpHandler {
|
||||
// new_session/load_session on this connection. Set-once per
|
||||
// connection; the result is ignored on later requests.
|
||||
let _ = agent.client_cx.set(cx.clone());
|
||||
agent.start_thinking_effort_update_forwarder(&cx).await;
|
||||
|
||||
// InitializeRequest runs inline: it sets connection-scoped state
|
||||
// (client fs/terminal capabilities) that later handlers read with
|
||||
|
||||
@@ -38,7 +38,7 @@ impl GooseAcpAgent {
|
||||
|
||||
let new_session = self
|
||||
.session_manager
|
||||
.get_session(&new_session_id, false)
|
||||
.get_session(&new_session_id, true)
|
||||
.await
|
||||
.internal_err()?;
|
||||
|
||||
@@ -47,14 +47,20 @@ impl GooseAcpAgent {
|
||||
new_session.clone(),
|
||||
args.cwd.clone(),
|
||||
args.mcp_servers,
|
||||
false,
|
||||
true,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let (agent, extension_results) = self.prepare_acp_session_agent(cx, &goose_session).await?;
|
||||
self.apply_session_recipe(&agent, &goose_session).await?;
|
||||
self.register_acp_session(goose_session.id.clone(), agent)
|
||||
self.register_acp_session(goose_session.id.clone(), agent.clone())
|
||||
.await;
|
||||
let provider = agent
|
||||
.provider()
|
||||
.await
|
||||
.internal_err_ctx("Failed to get provider while forking ACP session")?;
|
||||
resume_saved_provider_session(&provider, goose_session.conversation.as_ref()).await;
|
||||
let effort_support = agent_thinking_effort_support(&agent).await;
|
||||
|
||||
let acp_session_id = SessionId::new(new_session_id.clone());
|
||||
let mut meta = session_meta(&goose_session);
|
||||
@@ -63,7 +69,8 @@ impl GooseAcpAgent {
|
||||
}
|
||||
|
||||
let (mode_state, config_options) =
|
||||
build_session_setup_config(&self.provider_inventory, &goose_session).await?;
|
||||
build_session_setup_config(&self.provider_inventory, &goose_session, &effort_support)
|
||||
.await?;
|
||||
|
||||
let mut response = ForkSessionResponse::new(acp_session_id.clone())
|
||||
.modes(mode_state)
|
||||
|
||||
@@ -7,7 +7,6 @@ use super::tool_calls::conversion::{
|
||||
};
|
||||
use super::tool_calls::enrichment::tool_chain_summary;
|
||||
use super::*;
|
||||
use crate::conversation::Conversation;
|
||||
use agent_client_protocol::schema::v1::ToolCall;
|
||||
|
||||
fn replay_audience_annotations(audience: &[Role]) -> Annotations {
|
||||
@@ -299,6 +298,11 @@ impl GooseAcpAgent {
|
||||
self.apply_session_recipe(&agent, &session).await?;
|
||||
self.register_acp_session(session_id_str.clone(), agent.clone())
|
||||
.await;
|
||||
let provider = agent
|
||||
.provider()
|
||||
.await
|
||||
.internal_err_ctx("Failed to get provider while loading ACP session")?;
|
||||
resume_saved_provider_session(&provider, session.conversation.as_ref()).await;
|
||||
self.resend_pending_tool_permissions(cx, &agent, &session)?;
|
||||
|
||||
session = self
|
||||
@@ -312,8 +316,12 @@ impl GooseAcpAgent {
|
||||
.update_working_dir(&session.working_dir)
|
||||
.await;
|
||||
|
||||
let (mode_state, config_options) =
|
||||
build_session_setup_config(&self.provider_inventory, &session).await?;
|
||||
let (mode_state, config_options) = build_session_setup_config(
|
||||
&self.provider_inventory,
|
||||
&session,
|
||||
&agent_thinking_effort_support(&agent).await,
|
||||
)
|
||||
.await?;
|
||||
|
||||
self.notify_session_setup(cx, &session).await?;
|
||||
|
||||
@@ -332,7 +340,79 @@ impl GooseAcpAgent {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::conversation::message::InferenceMetadata;
|
||||
use goose_providers::thinking::{
|
||||
ThinkingEffortCapability, ThinkingEffortOption, ThinkingEffortSupport,
|
||||
};
|
||||
use rmcp::model::CallToolRequestParams;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
||||
#[derive(Debug)]
|
||||
struct ResumeEffortProvider {
|
||||
resumed: AtomicBool,
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl Provider for ResumeEffortProvider {
|
||||
fn get_name(&self) -> &str {
|
||||
"claude-acp"
|
||||
}
|
||||
|
||||
async fn resume(&self, session_id: &str) -> std::result::Result<(), ProviderError> {
|
||||
assert_eq!(session_id, "saved-inner-session");
|
||||
self.resumed.store(true, Ordering::Release);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn thinking_effort_support(&self) -> ThinkingEffortSupport {
|
||||
let value = if self.resumed.load(Ordering::Acquire) {
|
||||
"high"
|
||||
} else {
|
||||
"low"
|
||||
};
|
||||
ThinkingEffortSupport::Options(ThinkingEffortCapability {
|
||||
option_id: "effort".to_string(),
|
||||
values: vec![ThinkingEffortOption {
|
||||
value: value.to_string(),
|
||||
label: value.to_string(),
|
||||
}],
|
||||
current: Some(value.to_string()),
|
||||
})
|
||||
}
|
||||
|
||||
async fn stream(
|
||||
&self,
|
||||
_model_config: &goose_providers::model::ModelConfig,
|
||||
_system: &str,
|
||||
_messages: &[Message],
|
||||
_tools: &[rmcp::model::Tool],
|
||||
) -> std::result::Result<crate::providers::base::MessageStream, ProviderError> {
|
||||
Ok(Box::pin(futures::stream::empty()))
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn saved_provider_session_is_resumed_before_effort_snapshot() {
|
||||
let provider = Arc::new(ResumeEffortProvider {
|
||||
resumed: AtomicBool::new(false),
|
||||
});
|
||||
let conversation = Conversation::new_unvalidated([Message::assistant().with_inference(
|
||||
InferenceMetadata {
|
||||
provider: "claude-acp".to_string(),
|
||||
requested_model: "current".to_string(),
|
||||
resolved_model: None,
|
||||
provider_session_id: Some("saved-inner-session".to_string()),
|
||||
},
|
||||
)]);
|
||||
|
||||
let provider_dyn: Arc<dyn Provider> = provider.clone();
|
||||
resume_saved_provider_session(&provider_dyn, Some(&conversation)).await;
|
||||
|
||||
let ThinkingEffortSupport::Options(capability) = provider.thinking_effort_support() else {
|
||||
panic!("expected resumed effort capability");
|
||||
};
|
||||
assert_eq!(capability.current.as_deref(), Some("high"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn acp_replay_populates_only_empty_marked_assistant_messages() {
|
||||
|
||||
@@ -9,6 +9,7 @@ use super::GooseAcpAgent;
|
||||
use agent_client_protocol::schema::v1::{Meta, NewSessionRequest, NewSessionResponse, SessionId};
|
||||
use agent_client_protocol::{Client, ConnectionTo};
|
||||
use goose_providers::model::ModelConfig;
|
||||
use goose_providers::thinking::ThinkingEffortSupport;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
use tracing::warn;
|
||||
@@ -86,7 +87,11 @@ impl GooseAcpAgent {
|
||||
|
||||
let reloaded_session = self.reload_session(&session.id).await?;
|
||||
let response = self
|
||||
.build_new_session_response(&reloaded_session, &extension_results)
|
||||
.build_new_session_response(
|
||||
&reloaded_session,
|
||||
&extension_results,
|
||||
&super::agent_thinking_effort_support(&agent).await,
|
||||
)
|
||||
.await?;
|
||||
Ok(response)
|
||||
}
|
||||
@@ -243,9 +248,11 @@ impl GooseAcpAgent {
|
||||
&self,
|
||||
session: &Session,
|
||||
extension_results: &[ExtensionLoadResult],
|
||||
effort_support: &ThinkingEffortSupport,
|
||||
) -> Result<NewSessionResponse, agent_client_protocol::Error> {
|
||||
let (mode_state, config_options) =
|
||||
super::build_session_setup_config(&self.provider_inventory, session).await?;
|
||||
super::build_session_setup_config(&self.provider_inventory, session, effort_support)
|
||||
.await?;
|
||||
|
||||
let mut response =
|
||||
NewSessionResponse::new(SessionId::new(session.id.clone())).modes(mode_state);
|
||||
|
||||
@@ -69,7 +69,7 @@ use crate::tool_monitor::RepetitionInspector;
|
||||
use crate::utils::is_token_cancelled;
|
||||
use goose_providers::conversation::token_usage::{ProviderUsage, Usage};
|
||||
use goose_providers::errors::ProviderError;
|
||||
use goose_providers::thinking::ThinkingEffort;
|
||||
use goose_providers::thinking::{ThinkingEffort, ThinkingEffortSupport};
|
||||
use regex::Regex;
|
||||
use rmcp::model::{
|
||||
CallToolRequestParams, CallToolResult, ContentBlock, ElicitationAction, ErrorCode, ErrorData,
|
||||
@@ -93,6 +93,27 @@ fn provider_creation_error(error: anyhow::Error, context: impl fmt::Display) ->
|
||||
error.context(message)
|
||||
}
|
||||
|
||||
fn normalize_legacy_provider_thinking_effort(
|
||||
mut model_config: goose_providers::model::ModelConfig,
|
||||
effort_support: &ThinkingEffortSupport,
|
||||
) -> goose_providers::model::ModelConfig {
|
||||
let has_raw_effort = model_config
|
||||
.request_params
|
||||
.as_ref()
|
||||
.is_some_and(|params| params.contains_key("thinking_effort"));
|
||||
if !matches!(effort_support, ThinkingEffortSupport::Unspecified)
|
||||
|| !has_raw_effort
|
||||
|| model_config.thinking_effort().is_some()
|
||||
{
|
||||
return model_config;
|
||||
}
|
||||
|
||||
if let Some(params) = model_config.request_params.as_mut() {
|
||||
params.remove("thinking_effort");
|
||||
}
|
||||
model_config.with_default_thinking_effort(Config::global().get_goose_thinking_effort())
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum ToolCategory {
|
||||
Shell,
|
||||
@@ -3460,9 +3481,21 @@ impl Agent {
|
||||
.unwrap_or(model_config),
|
||||
Err(_) => model_config,
|
||||
};
|
||||
let effort_support = provider.thinking_effort_support();
|
||||
let model_config = normalize_legacy_provider_thinking_effort(model_config, &effort_support);
|
||||
|
||||
let mut current_provider = self.provider.lock().await;
|
||||
*current_provider = Some(provider);
|
||||
{
|
||||
let mut current_provider = self.provider.lock().await;
|
||||
*current_provider = Some(Arc::clone(&provider));
|
||||
}
|
||||
|
||||
// A freshly created provider that manages its own model starts on its
|
||||
// own default, so the session's selection has to be pushed to it before
|
||||
// the next config snapshot is built. Failures are not fatal here: the
|
||||
// selection is re-applied at stream time.
|
||||
if let Err(e) = provider.apply_model_selection(&model_config).await {
|
||||
warn!("Failed to apply model selection to provider: {e}");
|
||||
}
|
||||
|
||||
self.config
|
||||
.session_manager
|
||||
@@ -3530,20 +3563,51 @@ impl Agent {
|
||||
self.update_goose_mode(mode, session_id).await
|
||||
}
|
||||
|
||||
pub async fn update_thinking_effort(
|
||||
&self,
|
||||
session_id: &str,
|
||||
effort: ThinkingEffort,
|
||||
) -> Result<()> {
|
||||
/// Apply a thinking-effort selection. `effort` is the raw option value: a
|
||||
/// provider that manages effort through a harness has its own vocabulary,
|
||||
/// which is not always a `ThinkingEffort` member.
|
||||
pub async fn update_thinking_effort(&self, session_id: &str, effort: &str) -> Result<()> {
|
||||
let current_provider = self.provider().await?;
|
||||
let provider_name = current_provider.get_name().to_string();
|
||||
let model_config = self
|
||||
.model_config_for_session(session_id)
|
||||
.await?
|
||||
.with_thinking_effort(effort);
|
||||
|
||||
self.recreate_provider_for_session(session_id, &provider_name, model_config)
|
||||
// Context rather than a formatted string: the caller distinguishes a
|
||||
// value rejection from an operational failure by downcasting to
|
||||
// `ProviderError`, which stringifying would destroy.
|
||||
let provider_handled = current_provider
|
||||
.set_thinking_effort(session_id, effort)
|
||||
.await
|
||||
.context("Provider rejected thinking effort update")?;
|
||||
|
||||
let model_config = self.model_config_for_session(session_id).await?;
|
||||
|
||||
if provider_handled {
|
||||
// The provider applied the value live; recreating it would discard
|
||||
// the very session state we just configured.
|
||||
let model_config = model_config.with_merged_request_params(HashMap::from([(
|
||||
"thinking_effort".to_string(),
|
||||
Value::String(effort.to_string()),
|
||||
)]));
|
||||
return self
|
||||
.config
|
||||
.session_manager
|
||||
.clone()
|
||||
.update(session_id)
|
||||
.model_config(model_config)
|
||||
.apply()
|
||||
.await
|
||||
.context("Failed to persist thinking effort to session");
|
||||
}
|
||||
|
||||
let effort = effort.parse::<ThinkingEffort>().map_err(|_| {
|
||||
anyhow::Error::new(ProviderError::InvalidValue(format!(
|
||||
"Invalid thinking effort: {effort}"
|
||||
)))
|
||||
})?;
|
||||
let provider_name = current_provider.get_name().to_string();
|
||||
self.recreate_provider_for_session(
|
||||
session_id,
|
||||
&provider_name,
|
||||
model_config.with_thinking_effort(effort),
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Restore the provider from session data or fall back to global config
|
||||
@@ -4365,6 +4429,235 @@ mod tests {
|
||||
assert_eq!(conf.permission, crate::permission::Permission::AllowOnce);
|
||||
}
|
||||
|
||||
enum EffortOutcome {
|
||||
Applied,
|
||||
Unhandled,
|
||||
Rejected,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct EffortProvider {
|
||||
applies_effort: bool,
|
||||
rejects_effort: bool,
|
||||
effort_calls: std::sync::Mutex<Vec<String>>,
|
||||
model_selections: std::sync::Mutex<Vec<String>>,
|
||||
}
|
||||
|
||||
impl EffortProvider {
|
||||
fn new(outcome: EffortOutcome) -> Self {
|
||||
Self {
|
||||
applies_effort: matches!(outcome, EffortOutcome::Applied),
|
||||
rejects_effort: matches!(outcome, EffortOutcome::Rejected),
|
||||
effort_calls: std::sync::Mutex::new(Vec::new()),
|
||||
model_selections: std::sync::Mutex::new(Vec::new()),
|
||||
}
|
||||
}
|
||||
|
||||
fn effort_calls(&self) -> Vec<String> {
|
||||
self.effort_calls.lock().unwrap().clone()
|
||||
}
|
||||
|
||||
fn model_selections(&self) -> Vec<String> {
|
||||
self.model_selections.lock().unwrap().clone()
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl crate::providers::base::Provider for EffortProvider {
|
||||
fn get_name(&self) -> &str {
|
||||
"test-effort"
|
||||
}
|
||||
fn thinking_effort_support(&self) -> ThinkingEffortSupport {
|
||||
if self.applies_effort {
|
||||
ThinkingEffortSupport::Options(
|
||||
goose_providers::thinking::ThinkingEffortCapability {
|
||||
option_id: "effort".to_string(),
|
||||
values: vec![goose_providers::thinking::ThinkingEffortOption {
|
||||
value: "default".to_string(),
|
||||
label: "Default".to_string(),
|
||||
}],
|
||||
current: Some("default".to_string()),
|
||||
},
|
||||
)
|
||||
} else {
|
||||
ThinkingEffortSupport::Unspecified
|
||||
}
|
||||
}
|
||||
async fn stream(
|
||||
&self,
|
||||
_: &goose_providers::model::ModelConfig,
|
||||
_: &str,
|
||||
_: &[crate::conversation::message::Message],
|
||||
_: &[rmcp::model::Tool],
|
||||
) -> Result<crate::providers::base::MessageStream, ProviderError> {
|
||||
unimplemented!()
|
||||
}
|
||||
async fn set_thinking_effort(
|
||||
&self,
|
||||
_session_id: &str,
|
||||
value: &str,
|
||||
) -> Result<bool, ProviderError> {
|
||||
self.effort_calls.lock().unwrap().push(value.to_string());
|
||||
if self.rejects_effort {
|
||||
return Err(ProviderError::RequestFailed("no such effort".to_string()));
|
||||
}
|
||||
Ok(self.applies_effort)
|
||||
}
|
||||
async fn apply_model_selection(
|
||||
&self,
|
||||
model_config: &goose_providers::model::ModelConfig,
|
||||
) -> Result<(), ProviderError> {
|
||||
self.model_selections
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push(model_config.model_name.clone());
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
async fn effort_test_agent(
|
||||
outcome: EffortOutcome,
|
||||
) -> (Agent, String, Arc<EffortProvider>, TempDir) {
|
||||
let (agent, session, data_dir) = tracing_test_agent_and_session().await;
|
||||
let provider = Arc::new(EffortProvider::new(outcome));
|
||||
agent
|
||||
.update_provider(
|
||||
provider.clone(),
|
||||
goose_providers::model::ModelConfig::new("mock-model"),
|
||||
&session.id,
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
(agent, session.id, provider, data_dir)
|
||||
}
|
||||
|
||||
async fn persisted_thinking_effort(agent: &Agent, session_id: &str) -> Option<String> {
|
||||
agent
|
||||
.model_config_for_session(session_id)
|
||||
.await
|
||||
.unwrap()
|
||||
.request_param::<String>("thinking_effort")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_provider_applies_the_model_selection() {
|
||||
let (_agent, _session_id, provider, _data_dir) =
|
||||
effort_test_agent(EffortOutcome::Applied).await;
|
||||
|
||||
assert_eq!(provider.model_selections(), ["mock-model"]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_provider_replaces_harness_only_effort_for_legacy_provider() {
|
||||
let _guard = env_lock::lock_env([("GOOSE_THINKING_EFFORT", Some("high"))]);
|
||||
let (agent, session, _data_dir) = tracing_test_agent_and_session().await;
|
||||
let provider = Arc::new(EffortProvider::new(EffortOutcome::Unhandled));
|
||||
let model_config =
|
||||
goose_providers::model::ModelConfig::new("mock-model").with_merged_request_params(
|
||||
HashMap::from([("thinking_effort".to_string(), serde_json::json!("default"))]),
|
||||
);
|
||||
|
||||
agent
|
||||
.update_provider(provider, model_config, &session.id)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
persisted_thinking_effort(&agent, &session.id)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("high")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_provider_preserves_harness_only_effort_for_managed_provider() {
|
||||
let _guard = env_lock::lock_env([("GOOSE_THINKING_EFFORT", Some("high"))]);
|
||||
let (agent, session, _data_dir) = tracing_test_agent_and_session().await;
|
||||
let provider = Arc::new(EffortProvider::new(EffortOutcome::Applied));
|
||||
let model_config =
|
||||
goose_providers::model::ModelConfig::new("mock-model").with_merged_request_params(
|
||||
HashMap::from([("thinking_effort".to_string(), serde_json::json!("default"))]),
|
||||
);
|
||||
|
||||
agent
|
||||
.update_provider(provider, model_config, &session.id)
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
persisted_thinking_effort(&agent, &session.id)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("default")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thinking_effort_persists_the_raw_value_when_the_provider_applies_it() {
|
||||
let (agent, session_id, provider, _data_dir) =
|
||||
effort_test_agent(EffortOutcome::Applied).await;
|
||||
|
||||
// "xhigh" is a harness value, not a ThinkingEffort member spelling.
|
||||
agent
|
||||
.update_thinking_effort(&session_id, "xhigh")
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(provider.effort_calls(), ["xhigh"]);
|
||||
assert_eq!(
|
||||
persisted_thinking_effort(&agent, &session_id)
|
||||
.await
|
||||
.as_deref(),
|
||||
Some("xhigh")
|
||||
);
|
||||
// The unregistered test provider was not respawned.
|
||||
assert_eq!(agent.provider().await.unwrap().get_name(), "test-effort");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thinking_effort_rejects_an_unparseable_value_on_the_legacy_path() {
|
||||
let (agent, session_id, provider, _data_dir) =
|
||||
effort_test_agent(EffortOutcome::Unhandled).await;
|
||||
|
||||
let err = agent
|
||||
.update_thinking_effort(&session_id, "bogus")
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
err.downcast_ref::<ProviderError>(),
|
||||
Some(ProviderError::InvalidValue(_))
|
||||
));
|
||||
assert!(err.to_string().contains("Invalid thinking effort"));
|
||||
assert_eq!(provider.effort_calls(), ["bogus"]);
|
||||
assert!(persisted_thinking_effort(&agent, &session_id)
|
||||
.await
|
||||
.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_thinking_effort_surfaces_a_provider_rejection() {
|
||||
let (agent, session_id, _provider, _data_dir) =
|
||||
effort_test_agent(EffortOutcome::Rejected).await;
|
||||
|
||||
let err = agent
|
||||
.update_thinking_effort(&session_id, "high")
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
||||
assert!(err.to_string().contains("Provider rejected"));
|
||||
// The caller classifies the failure by variant, so the provider's typed
|
||||
// error has to survive the trip up.
|
||||
assert!(matches!(
|
||||
err.downcast_ref::<ProviderError>(),
|
||||
Some(ProviderError::RequestFailed(_))
|
||||
));
|
||||
assert!(persisted_thinking_effort(&agent, &session_id)
|
||||
.await
|
||||
.is_none());
|
||||
}
|
||||
|
||||
const ALWAYS_BLOCK_SCRIPT: &str = r#"#!/bin/sh
|
||||
echo blocked >> "$PLUGIN_ROOT/hook.log"
|
||||
echo "always block" >&2
|
||||
|
||||
@@ -39,7 +39,7 @@ pub use subagent_task_config::TaskConfig;
|
||||
pub use tool_execution::ToolCallContext;
|
||||
pub use types::{FrontendTool, RetryConfig, SessionConfig, SuccessCheck};
|
||||
|
||||
fn latest_provider_session_id<'a>(
|
||||
pub(crate) fn latest_provider_session_id<'a>(
|
||||
messages: &'a [crate::conversation::message::Message],
|
||||
provider: &str,
|
||||
) -> Option<&'a str> {
|
||||
|
||||
@@ -0,0 +1,329 @@
|
||||
use agent_client_protocol::schema::v1::{
|
||||
AgentCapabilities, ConfigOptionUpdate, InitializeRequest, InitializeResponse,
|
||||
LoadSessionRequest, LoadSessionResponse, NewSessionRequest, NewSessionResponse,
|
||||
SessionConfigOption, SessionConfigOptionCategory, SessionConfigSelectOption, SessionId,
|
||||
SessionNotification, SessionUpdate, SetSessionConfigOptionRequest,
|
||||
SetSessionConfigOptionResponse, UsageUpdate,
|
||||
};
|
||||
use agent_client_protocol::schema::ProtocolVersion;
|
||||
use agent_client_protocol::{on_receive_request, Agent as SacpAgent, ByteStreams};
|
||||
use goose::acp::{AcpProvider, AcpProviderConfig};
|
||||
use goose::config::GooseMode;
|
||||
use goose::providers::base::Provider;
|
||||
use goose_providers::thinking::ThinkingEffortSupport;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::{Arc, Mutex};
|
||||
use tokio::sync::Notify;
|
||||
use tokio::time::{timeout, Duration};
|
||||
use tokio_util::compat::{TokioAsyncReadCompatExt, TokioAsyncWriteCompatExt};
|
||||
|
||||
fn effort_option(current: &str, values: &[&str]) -> SessionConfigOption {
|
||||
SessionConfigOption::select(
|
||||
"effort",
|
||||
"Thinking",
|
||||
current.to_string(),
|
||||
values
|
||||
.iter()
|
||||
.map(|value| SessionConfigSelectOption::new(value.to_string(), *value))
|
||||
.collect::<Vec<_>>(),
|
||||
)
|
||||
.category(SessionConfigOptionCategory::ThoughtLevel)
|
||||
}
|
||||
|
||||
/// Agents that pin a model during session bootstrap rebuild their per-model
|
||||
/// effort levels in that response, so it supersedes the `session/new` snapshot
|
||||
/// the mirrored capability was first built from.
|
||||
#[tokio::test]
|
||||
async fn bootstrap_config_option_response_refreshes_the_effort_mirror() {
|
||||
let (client_read, agent_write) = tokio::io::duplex(64 * 1024);
|
||||
let (agent_read, client_write) = tokio::io::duplex(64 * 1024);
|
||||
|
||||
let bootstrap_picks: Arc<Mutex<Vec<(String, String)>>> = Arc::new(Mutex::new(Vec::new()));
|
||||
let recorded_picks = bootstrap_picks.clone();
|
||||
|
||||
let agent = tokio::spawn(async move {
|
||||
SacpAgent
|
||||
.builder()
|
||||
.name("scripted-agent")
|
||||
.on_receive_request(
|
||||
async |_req: InitializeRequest, responder, _cx| {
|
||||
responder.respond(InitializeResponse::new(ProtocolVersion::LATEST))
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.on_receive_request(
|
||||
async |_req: NewSessionRequest, responder, _cx| {
|
||||
responder.respond(
|
||||
NewSessionResponse::new(SessionId::new("scripted-session")).config_options(
|
||||
vec![effort_option("medium", &["low", "medium", "high"])],
|
||||
),
|
||||
)
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.on_receive_request(
|
||||
async |req: SetSessionConfigOptionRequest, responder, _cx| {
|
||||
let value = req
|
||||
.value
|
||||
.as_value_id()
|
||||
.expect("select option value")
|
||||
.0
|
||||
.to_string();
|
||||
recorded_picks
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push((req.config_id.0.to_string(), value));
|
||||
responder.respond(SetSessionConfigOptionResponse::new(vec![effort_option(
|
||||
"high",
|
||||
&["minimal", "high"],
|
||||
)]))
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.connect_to(ByteStreams::new(
|
||||
agent_write.compat_write(),
|
||||
agent_read.compat(),
|
||||
))
|
||||
.await
|
||||
});
|
||||
|
||||
let config = AcpProviderConfig {
|
||||
command: "unused".into(),
|
||||
args: vec![],
|
||||
env: vec![],
|
||||
env_remove: vec![],
|
||||
work_dir: std::env::temp_dir(),
|
||||
mcp_servers: vec![],
|
||||
session_mode_id: None,
|
||||
session_config_options: vec![("model".to_string(), "gpt-5".to_string())],
|
||||
model_config_option_id: Some("model".to_string()),
|
||||
mode_mapping: HashMap::new(),
|
||||
notification_callback: None,
|
||||
};
|
||||
|
||||
let provider = AcpProvider::connect_with_transport(
|
||||
"scripted-acp".to_string(),
|
||||
GooseMode::default(),
|
||||
config,
|
||||
ByteStreams::new(client_write.compat_write(), client_read.compat()),
|
||||
)
|
||||
.await
|
||||
.expect("provider should connect to the scripted agent");
|
||||
|
||||
assert_eq!(
|
||||
*bootstrap_picks.lock().unwrap(),
|
||||
vec![("model".to_string(), "gpt-5".to_string())]
|
||||
);
|
||||
|
||||
match provider.thinking_effort_support() {
|
||||
ThinkingEffortSupport::Options(capability) => {
|
||||
assert_eq!(capability.current.as_deref(), Some("high"));
|
||||
assert_eq!(
|
||||
capability
|
||||
.values
|
||||
.iter()
|
||||
.map(|option| option.value.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["minimal", "high"]
|
||||
);
|
||||
}
|
||||
other => panic!("expected the agent's rebuilt effort options, got {other:?}"),
|
||||
}
|
||||
|
||||
drop(provider);
|
||||
agent.abort();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn new_session_preserves_pre_response_effort_update() {
|
||||
let (client_read, agent_write) = tokio::io::duplex(64 * 1024);
|
||||
let (agent_read, client_write) = tokio::io::duplex(64 * 1024);
|
||||
|
||||
let agent = tokio::spawn(async move {
|
||||
SacpAgent
|
||||
.builder()
|
||||
.name("scripted-agent")
|
||||
.on_receive_request(
|
||||
async |_req: InitializeRequest, responder, _cx| {
|
||||
responder.respond(InitializeResponse::new(ProtocolVersion::LATEST))
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.on_receive_request(
|
||||
async |_req: NewSessionRequest, responder, cx| {
|
||||
cx.send_notification(SessionNotification::new(
|
||||
SessionId::new("scripted-session"),
|
||||
SessionUpdate::ConfigOptionUpdate(ConfigOptionUpdate::new(vec![
|
||||
effort_option("high", &["default", "high", "xhigh"]),
|
||||
])),
|
||||
))?;
|
||||
responder.respond(NewSessionResponse::new(SessionId::new("scripted-session")))
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.connect_to(ByteStreams::new(
|
||||
agent_write.compat_write(),
|
||||
agent_read.compat(),
|
||||
))
|
||||
.await
|
||||
});
|
||||
|
||||
let provider = AcpProvider::connect_with_transport(
|
||||
"scripted-acp".to_string(),
|
||||
GooseMode::default(),
|
||||
AcpProviderConfig {
|
||||
command: "unused".into(),
|
||||
args: vec![],
|
||||
env: vec![],
|
||||
env_remove: vec![],
|
||||
work_dir: std::env::temp_dir(),
|
||||
mcp_servers: vec![],
|
||||
session_mode_id: None,
|
||||
session_config_options: vec![],
|
||||
model_config_option_id: None,
|
||||
mode_mapping: HashMap::new(),
|
||||
notification_callback: None,
|
||||
},
|
||||
ByteStreams::new(client_write.compat_write(), client_read.compat()),
|
||||
)
|
||||
.await
|
||||
.expect("provider should preserve the pre-response update");
|
||||
|
||||
match provider.thinking_effort_support() {
|
||||
ThinkingEffortSupport::Options(capability) => {
|
||||
assert_eq!(capability.current.as_deref(), Some("high"));
|
||||
assert_eq!(
|
||||
capability
|
||||
.values
|
||||
.iter()
|
||||
.map(|option| option.value.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["default", "high", "xhigh"]
|
||||
);
|
||||
}
|
||||
other => panic!("expected the pre-response effort options, got {other:?}"),
|
||||
}
|
||||
|
||||
drop(provider);
|
||||
agent.abort();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loaded_session_refreshes_the_effort_mirror() {
|
||||
let (client_read, agent_write) = tokio::io::duplex(64 * 1024);
|
||||
let (agent_read, client_write) = tokio::io::duplex(64 * 1024);
|
||||
|
||||
let emit_late_update = Arc::new(Notify::new());
|
||||
let agent_emit_late_update = emit_late_update.clone();
|
||||
let active_update_received = Arc::new(Notify::new());
|
||||
let callback_active_update_received = active_update_received.clone();
|
||||
|
||||
let agent = tokio::spawn(async move {
|
||||
SacpAgent
|
||||
.builder()
|
||||
.name("scripted-agent")
|
||||
.on_receive_request(
|
||||
async |_req: InitializeRequest, responder, _cx| {
|
||||
responder.respond(
|
||||
InitializeResponse::new(ProtocolVersion::LATEST)
|
||||
.agent_capabilities(AgentCapabilities::new().load_session(true)),
|
||||
)
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.on_receive_request(
|
||||
async |_req: NewSessionRequest, responder, _cx| {
|
||||
responder.respond(
|
||||
NewSessionResponse::new(SessionId::new("temporary-session"))
|
||||
.config_options(vec![effort_option(
|
||||
"medium",
|
||||
&["low", "medium", "high"],
|
||||
)]),
|
||||
)
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.on_receive_request(
|
||||
async move |req: LoadSessionRequest, responder, cx| {
|
||||
assert_eq!(req.session_id.0.as_ref(), "saved-session");
|
||||
responder.respond(LoadSessionResponse::new().config_options(vec![
|
||||
effort_option("xhigh", &["default", "high", "xhigh"]),
|
||||
]))?;
|
||||
agent_emit_late_update.notified().await;
|
||||
cx.send_notification(SessionNotification::new(
|
||||
SessionId::new("temporary-session"),
|
||||
SessionUpdate::ConfigOptionUpdate(ConfigOptionUpdate::new(vec![
|
||||
effort_option("medium", &["low", "medium", "high"]),
|
||||
])),
|
||||
))?;
|
||||
cx.send_notification(SessionNotification::new(
|
||||
SessionId::new("saved-session"),
|
||||
SessionUpdate::UsageUpdate(UsageUpdate::new(1, 100)),
|
||||
))
|
||||
},
|
||||
on_receive_request!(),
|
||||
)
|
||||
.connect_to(ByteStreams::new(
|
||||
agent_write.compat_write(),
|
||||
agent_read.compat(),
|
||||
))
|
||||
.await
|
||||
});
|
||||
|
||||
let config = AcpProviderConfig {
|
||||
command: "unused".into(),
|
||||
args: vec![],
|
||||
env: vec![],
|
||||
env_remove: vec![],
|
||||
work_dir: std::env::temp_dir(),
|
||||
mcp_servers: vec![],
|
||||
session_mode_id: None,
|
||||
session_config_options: vec![],
|
||||
model_config_option_id: None,
|
||||
mode_mapping: HashMap::new(),
|
||||
notification_callback: Some(Arc::new(move |notification| {
|
||||
if notification.session_id.0.as_ref() == "saved-session"
|
||||
&& matches!(notification.update, SessionUpdate::UsageUpdate(_))
|
||||
{
|
||||
callback_active_update_received.notify_one();
|
||||
}
|
||||
})),
|
||||
};
|
||||
|
||||
let provider = AcpProvider::connect_with_transport(
|
||||
"scripted-acp".to_string(),
|
||||
GooseMode::default(),
|
||||
config,
|
||||
ByteStreams::new(client_write.compat_write(), client_read.compat()),
|
||||
)
|
||||
.await
|
||||
.expect("provider should connect to the scripted agent");
|
||||
|
||||
provider
|
||||
.resume("saved-session")
|
||||
.await
|
||||
.expect("provider should load the saved session");
|
||||
|
||||
emit_late_update.notify_one();
|
||||
timeout(Duration::from_secs(1), active_update_received.notified())
|
||||
.await
|
||||
.expect("active-session barrier notification should be received");
|
||||
|
||||
match provider.thinking_effort_support() {
|
||||
ThinkingEffortSupport::Options(capability) => {
|
||||
assert_eq!(capability.current.as_deref(), Some("xhigh"));
|
||||
assert_eq!(
|
||||
capability
|
||||
.values
|
||||
.iter()
|
||||
.map(|option| option.value.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
vec!["default", "high", "xhigh"]
|
||||
);
|
||||
}
|
||||
other => panic!("expected the loaded session's effort options, got {other:?}"),
|
||||
}
|
||||
|
||||
drop(provider);
|
||||
agent.abort();
|
||||
}
|
||||
@@ -25,6 +25,7 @@ use goose::providers::xai::XAI_DEFAULT_MODEL;
|
||||
use goose::session::{SessionManager, SessionType};
|
||||
use goose_providers::databricks::DATABRICKS_DEFAULT_MODEL;
|
||||
use goose_providers::errors::ProviderError;
|
||||
use goose_providers::thinking::ThinkingEffortSupport;
|
||||
use goose_test_support::{
|
||||
EnforceSessionId, ExpectedSessionId, IgnoreSessionId, McpFixture, FAKE_CODE,
|
||||
};
|
||||
@@ -123,6 +124,7 @@ struct ProviderTestConfig {
|
||||
test_mode_update: bool,
|
||||
test_mcp_tools: bool,
|
||||
test_context_length_exceeded: bool,
|
||||
test_thinking_effort: bool,
|
||||
expect_context_length_exceeded: bool,
|
||||
context_length_exceeded: usize,
|
||||
}
|
||||
@@ -147,6 +149,7 @@ impl ProviderTestConfig {
|
||||
test_mode_update: true,
|
||||
test_mcp_tools: true,
|
||||
test_context_length_exceeded: true,
|
||||
test_thinking_effort: false,
|
||||
expect_context_length_exceeded: true,
|
||||
context_length_exceeded: 600_000,
|
||||
}
|
||||
@@ -177,6 +180,11 @@ impl ProviderTestConfig {
|
||||
self
|
||||
}
|
||||
|
||||
fn test_thinking_effort(mut self, v: bool) -> Self {
|
||||
self.test_thinking_effort = v;
|
||||
self
|
||||
}
|
||||
|
||||
fn expect_context_length_exceeded(mut self, v: bool) -> Self {
|
||||
self.expect_context_length_exceeded = v;
|
||||
self
|
||||
@@ -497,6 +505,53 @@ impl ProviderFixture {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn test_thinking_effort(&self) -> Result<()> {
|
||||
let ThinkingEffortSupport::Options(capability) = self.provider.thinking_effort_support()
|
||||
else {
|
||||
anyhow::bail!("{} should mirror the agent's effort option", self.name);
|
||||
};
|
||||
assert!(!capability.values.is_empty());
|
||||
let target = capability.values.last().unwrap().value.clone();
|
||||
println!(
|
||||
"=== {}::thinking_effort ({} -> {}) === {:?}",
|
||||
self.name,
|
||||
capability.current.as_deref().unwrap_or("unset"),
|
||||
target,
|
||||
capability.values
|
||||
);
|
||||
|
||||
assert!(
|
||||
self.provider
|
||||
.set_thinking_effort(&self.session_id, &target)
|
||||
.await?
|
||||
);
|
||||
|
||||
let effort_config = self
|
||||
.model_config
|
||||
.clone()
|
||||
.with_merged_request_params(HashMap::from([(
|
||||
"thinking_effort".to_string(),
|
||||
serde_json::json!(target),
|
||||
)]));
|
||||
let message = Message::user().with_text("Just say hello!");
|
||||
let (response, _) = goose::session_context::with_session_id(
|
||||
Some(self.session_id.clone()),
|
||||
self.provider.complete(
|
||||
&effort_config,
|
||||
"You are a helpful assistant.",
|
||||
&[message],
|
||||
&[],
|
||||
),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert!(response
|
||||
.content
|
||||
.iter()
|
||||
.any(|c| matches!(c, MessageContent::Text(_))));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn test_model_listing(&self) -> Result<()> {
|
||||
let models = self.provider.fetch_supported_models().await?;
|
||||
|
||||
@@ -687,6 +742,12 @@ async fn test_provider(config: ProviderTestConfig) -> Result<()> {
|
||||
.test_context_length_exceeded_error()
|
||||
.await?;
|
||||
}
|
||||
if config.test_thinking_effort {
|
||||
run_test(GooseMode::Auto)
|
||||
.await?
|
||||
.test_thinking_effort()
|
||||
.await?;
|
||||
}
|
||||
if config.test_permissions {
|
||||
run_test(GooseMode::Approve)
|
||||
.await?
|
||||
@@ -895,6 +956,7 @@ async fn test_codex_provider() -> Result<()> {
|
||||
async fn test_claude_acp_provider() -> Result<()> {
|
||||
ProviderTestConfig::with_agentic_provider("claude-acp", ACP_CURRENT_MODEL, "claude-agent-acp")
|
||||
.model_switch_name("sonnet")
|
||||
.test_thinking_effort(true)
|
||||
.run()
|
||||
.await
|
||||
}
|
||||
|
||||
+20
-1
@@ -14,6 +14,7 @@ activities:
|
||||
- Test load tool for knowledge injection and discovery
|
||||
- Test delegate tool for task delegation (sync and async)
|
||||
- Test multi-turn thinking preservation for providers that reject replayed reasoning_content
|
||||
- Validate ACP thinking-effort discovery, forwarding, and provider switching
|
||||
- Test error boundaries including nested delegation prevention
|
||||
- Generate comprehensive test report
|
||||
|
||||
@@ -22,7 +23,7 @@ parameters:
|
||||
input_type: string
|
||||
requirement: optional
|
||||
default: "all"
|
||||
description: "Which test phases to run: all, basic, extensions, delegation, reasoning, advanced"
|
||||
description: "Which test phases to run: all, basic, extensions, delegation, reasoning, acp-effort, advanced"
|
||||
|
||||
- key: test_depth
|
||||
input_type: string
|
||||
@@ -391,6 +392,24 @@ prompt: |
|
||||
Log results to: {{ workspace_dir }}/phase3c_reasoning.md
|
||||
{% endif %}
|
||||
|
||||
{% if test_phases == "all" or "acp-effort" in test_phases %}
|
||||
## 🧠 PHASE 3D: ACP Thinking-Effort Testing
|
||||
|
||||
**Prerequisites**: Run this phase from a goose source checkout with the Rust toolchain
|
||||
available. Skip this phase and record it as SKIPPED otherwise.
|
||||
|
||||
### ACP Effort Integration Test
|
||||
1. Run `cargo test -p goose effort`.
|
||||
2. Verify ACP agents' advertised effort menus and current values are mirrored after new,
|
||||
loaded, and reconfigured sessions.
|
||||
3. Verify supported effort selections are forwarded to ACP agents and unsupported values
|
||||
are rejected without changing the session.
|
||||
4. Verify ACP-only effort values are removed when switching to a legacy provider while
|
||||
managed providers preserve values they support.
|
||||
|
||||
Log results to: {{ workspace_dir }}/phase3d_acp_effort.md
|
||||
{% endif %}
|
||||
|
||||
{% if test_phases == "all" or "advanced" in test_phases %}
|
||||
## 🔬 PHASE 4: Advanced Testing
|
||||
|
||||
|
||||
Reference in New Issue
Block a user