vrr/goose
2
0
Fork 0
mirror of https://github.com/block/goose.git synced 2026-07-09 16:09:22 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 22

fix: don't bleed provider-specific request_params into delegate() sessions (#9906)

Browse source 
Co-authored-by: Douwe M Osinga <douwe@sidewalklabs.com>
This commit is contained in:
Kyle E DeFreitas 2026-07-03 12:23:26 -04:00 • committed by GitHub
parent 6d8c42cfa0
commit d873c91234
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 110 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

55
crates/goose-provider-types/src/model.rs
View file

@ -8,6 +8,19 @@ use utoipa::ToSchema;
pub const DEFAULT_CONTEXT_LIMIT: usize = 128_000;
/// Request param keys that describe model-family-agnostic reasoning behavior and
/// are therefore safe to carry across a model switch or subagent delegation.
/// Provider-specific keys (e.g. `anthropic_beta`) are deliberately excluded so
/// they can't bleed into a request targeting a different model family.
const INHERITED_SESSION_PARAM_KEYS: &[&str] = &[
"thinking_effort",
"thinking_budget",
"budget_tokens",
"enable_thinking",
"preserve_thinking_context",
"preserve_unsigned_thinking",
];
#[derive(Debug, Clone, Serialize, ToSchema)]
pub struct ModelConfig {
pub model_name: String,
@ -187,22 +200,13 @@ impl ModelConfig {
previous: Option<&ModelConfig>,
request_params: Option<HashMap<String, Value>>,
) -> Self {
if let Some(previous) = previous {
let has_thinking_effort = self
.request_params
.as_ref()
.and_then(|params| params.get("thinking_effort"))
.is_some();
if !has_thinking_effort {
if let Some(thinking_effort) = previous
.request_params
.as_ref()
.and_then(|params| params.get("thinking_effort"))
.cloned()
{
let params = self.request_params.get_or_insert_with(HashMap::new);
params.insert("thinking_effort".to_string(), thinking_effort);
if let Some(previous_params) = previous.and_then(|p| p.request_params.as_ref()) {
for key in INHERITED_SESSION_PARAM_KEYS {
if let Some(value) = previous_params.get(*key) {
self.request_params
.get_or_insert_with(HashMap::new)
.entry(key.to_string())
.or_insert_with(|| value.clone());
}
}
}
@ -365,15 +369,28 @@ mod tests {
}
#[test]
fn does_not_preserve_unrelated_request_params() {
fn inherits_reasoning_controls_but_not_provider_specific_params() {
let previous = config_with_params(
"previous",
HashMap::from([("provider_specific".to_string(), serde_json::json!("old"))]),
HashMap::from([
("budget_tokens".to_string(), serde_json::json!(8192)),
(
"preserve_thinking_context".to_string(),
serde_json::json!(true),
),
("anthropic_beta".to_string(), serde_json::json!("beta")),
]),
);
let config = ModelConfig::new("next")
.with_inherited_session_settings_from(Some(&previous), None);
assert!(config.request_params.is_none());
let params = config.request_params.expect("reasoning controls inherited");
assert_eq!(params.get("budget_tokens"), Some(&serde_json::json!(8192)));
assert_eq!(
params.get("preserve_thinking_context"),
Some(&serde_json::json!(true))
);
assert_eq!(params.get("anthropic_beta"), None);
}
#[test]

88
crates/goose/src/agents/platform_extensions/summon.rs
View file

@ -1609,23 +1609,29 @@ impl SummonClient {
if let Some(model) = override_model {
if model != model_config.model_name {
// Build the new config from scratch so canonical fields
// (context_limit, max_tokens, reasoning) and env-derived
// overrides (GOOSE_CONTEXT_LIMIT, GOOSE_MAX_TOKENS) match the
// overridden model, then preserve session-level state that is
// not model-specific from the parent.
// Build the overridden config through the canonical session-settings
// path. This materializes model-specific fields (context_limit,
// max_tokens, reasoning) and env overrides for the *new* model, and
// inherits only model-family-agnostic session state from the parent:
// reasoning controls like `thinking_effort` and `budget_tokens` carry
// over (with the child > parent > global-default precedence the helper
// applies), while provider-specific request_params such as
// `anthropic_beta` are dropped so they can't bleed into a child
// targeting a different model family and trigger a 400 INVALID_ARGUMENT.
let parent = model_config;
let mut cfg =
crate::model_config::model_config_from_user_config(provider_name, &model)?;
crate::model_config::model_config_from_user_config_with_session_settings(
provider_name,
&model,
Some(&parent),
None,
None,
)?;
// Remaining model-agnostic session settings the helper doesn't
// touch, copied from the parent explicitly.
cfg.toolshim = parent.toolshim;
cfg.toolshim_model = parent.toolshim_model;
cfg.temperature = cfg.temperature.or(parent.temperature);
if let Some(parent_params) = parent.request_params {
let merged = cfg.request_params.get_or_insert_with(Default::default);
for (k, v) in parent_params {
merged.insert(k, v);
}
}
model_config = cfg;
}
}
@ -2616,13 +2622,17 @@ You review code."#;
#[tokio::test]
#[serial]
async fn test_resolve_model_config_preserves_parent_request_params_on_override() {
async fn test_resolve_model_config_does_not_inherit_provider_specific_request_params() {
let _env = env_lock::lock_env([
("GOOSE_CONTEXT_LIMIT", None::<&str>),
("GOOSE_MAX_TOKENS", None::<&str>),
("GOOSE_SUBAGENT_MODEL", None::<&str>),
]);
// Parent session is a Claude model with anthropic_beta in request_params.
// When delegate() overrides to a different model (e.g. Gemini), provider-
// specific params like anthropic_beta must not bleed through — they would
// cause a 400 INVALID_ARGUMENT from the target API.
let mut parent = parent_config();
parent.request_params = Some(HashMap::from([(
"anthropic_beta".to_string(),
@ -2636,7 +2646,57 @@ You review code."#;
.request_params
.as_ref()
.and_then(|p| p.get("anthropic_beta")),
Some(&serde_json::json!("custom-beta-header")),
None,
"anthropic_beta must not be inherited by a child session with a different model"
);
}
#[tokio::test]
#[serial]
async fn test_resolve_model_config_inherits_thinking_effort_on_override() {
let _env = env_lock::lock_env([
("GOOSE_CONTEXT_LIMIT", None::<&str>),
("GOOSE_MAX_TOKENS", None::<&str>),
("GOOSE_SUBAGENT_MODEL", None::<&str>),
]);
// Reasoning controls are model-family-agnostic and should be inherited,
// while provider-specific params like anthropic_beta must not.
let mut parent = parent_config();
parent.request_params = Some(HashMap::from([
("thinking_effort".to_string(), serde_json::json!("high")),
("budget_tokens".to_string(), serde_json::json!(8192)),
(
"anthropic_beta".to_string(),
serde_json::json!("custom-beta-header"),
),
]));
let resolved = resolve_with_override(Some(OVERRIDE_MODEL), parent);
assert_eq!(
resolved
.request_params
.as_ref()
.and_then(|p| p.get("thinking_effort")),
Some(&serde_json::json!("high")),
"thinking_effort should be inherited across model families"
);
assert_eq!(
resolved
.request_params
.as_ref()
.and_then(|p| p.get("budget_tokens")),
Some(&serde_json::json!(8192)),
"budget_tokens should be inherited across model families"
);
assert_eq!(
resolved
.request_params
.as_ref()
.and_then(|p| p.get("anthropic_beta")),
None,
"anthropic_beta must not be inherited alongside reasoning controls"
);
}