feat(agents): shared alias resolver + spawn effort bridge + deploy test (Phase 3)

- apply_effort_bridge() in config_bridge/mod.rs: applied after merged user env
  in resolve_effective_agent_env_with_def. Strips all foreign known effort keys
  (runtime-scoped invariant), then resolves canonical effort via tier-first
  precedence (record native → record legacy → persona native → persona legacy
  → global native → definition native; global legacy excluded end-to-end).
  Removes raw native key (possibly alias-form) from env before inserting
  canonical winner — skip-as-absent applies to the output map, not just winner
  selection.
- effort_tier_alias() with global_tier=true: honours native key only at global
  tier; legacy alias excluded there (plan v3 Delta 2).
- readiness_effort_bridge_tests.rs: 12 spawn tests covering bridge activation,
  tier-first precedence (record beats persona, record legacy beats persona
  native, global legacy excluded), alias normalization (none→off, xhigh→max),
  invalid-skip-as-absent (minimal skipped, persona native wins), and the
  bidirectional global coexist invariant (Goose and buzz-agent from same global
  config each receive only their own native key; GOOSE_THINKING_EFFORT stripped
  from buzz-agent descriptor, BUZZ_AGENT_THINKING_EFFORT stripped from Goose).
- agents_deploy.rs: deploy parity test pins that a legacy-only Goose record
  produces GOOSE_THINKING_EFFORT in launch.env while top-level env_vars retains
  the legacy key as compatibility bookkeeping (Delta-4 contract boundary).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
2026-08-13 12:26:10 -04:00
committed by Duncan
co-authored by Will Pfleger
parent 071ed50d4a
commit 542baacbfc
4 changed files with 525 additions and 22 deletions
@@ -475,4 +475,69 @@ mod tests {
"legacy top-level parallelism must match launch.policy_env — both must be {cap}"
);
}
/// Deploy parity (plan v3 Delta 4): for a legacy-only Goose record, the bridged
/// descriptor feeds `launch.env` with the native key, while the separately-merged
/// top-level `env_vars` retains the legacy key untouched.
///
/// Contract: providers execute `launch`; top-level `env_vars` is compatibility
/// bookkeeping. This test pins that boundary.
#[test]
fn deploy_parity_launch_env_carries_native_goose_effort() {
use crate::managed_agents::known_acp_runtime_exact;
use crate::managed_agents::{
global_config::GlobalAgentConfig, resolve_effective_agent_env,
};
// Goose record with only legacy BUZZ_AGENT_THINKING_EFFORT.
let record: ManagedAgentRecord = serde_json::from_value(serde_json::json!({
"pubkey": "pk",
"name": "goose-agent",
"private_key_nsec": "",
"relay_url": "",
"acp_command": "goose-acp",
"agent_command": "goose",
"agent_args": [],
"mcp_command": "",
"turn_timeout_seconds": 320,
"parallelism": 1,
"respond_to": "owner-only",
"respond_to_allowlist": [],
"env_vars": { "BUZZ_AGENT_THINKING_EFFORT": "high" },
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}))
.unwrap();
let runtime = known_acp_runtime_exact("goose");
let global = GlobalAgentConfig::default();
// The descriptor (= what launch.env uses) comes from resolve_effective_agent_env.
let descriptor = resolve_effective_agent_env(&record, &[], runtime, &global);
// launch.env: legacy key translated → native key.
assert_eq!(
descriptor
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("high"),
"launch.env must carry native GOOSE_THINKING_EFFORT"
);
assert!(
!descriptor.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"),
"launch.env must not carry legacy key"
);
// top-level env_vars (unmodified raw input): legacy key is still there.
// (The deploy payload's `env_vars` field is merged_user_env of the raw record —
// the bridge only affects the descriptor/launch path.)
assert_eq!(
record
.env_vars
.get("BUZZ_AGENT_THINKING_EFFORT")
.map(String::as_str),
Some("high"),
"top-level env_vars retains legacy key as compatibility bookkeeping"
);
}
}
@@ -19,7 +19,6 @@ pub(crate) const LEGACY_THINKING_EFFORT_KEY: &str = "BUZZ_AGENT_THINKING_EFFORT"
/// The set of all known native thinking-effort env keys across all runtimes.
/// Used to strip foreign effort keys from a runtime's effective descriptor.
/// Must stay in sync with `KnownAcpRuntime::thinking_env_var` declarations.
#[allow(dead_code)] // used in Phase 3 (spawn foreign-key stripping in readiness.rs)
pub(crate) const ALL_KNOWN_EFFORT_KEYS: &[&str] = &[
LEGACY_THINKING_EFFORT_KEY, // buzz-agent native
"GOOSE_THINKING_EFFORT", // Goose native
@@ -73,3 +72,78 @@ pub(crate) fn effort_tier_alias(
pub(crate) fn read_goose_file_config() -> Option<RuntimeFileConfig> {
goose::read_config_file()
}
/// Apply the spawn-side legacy effort bridge to an already-merged effective env.
///
/// For runtimes with a static effort vocabulary (`effort_normalization` is `Some`):
/// 1. Walk per-tier sanitized maps in tier-first precedence order and resolve the
/// canonical effort value.
/// 2. Strip all foreign known effort keys from `env` (runtime-scoped invariant).
/// 3. Remove any raw (possibly invalid/alias-form) entry for the native key.
/// 4. Insert the canonical value under the native key (if any tier resolved one).
///
/// Tier order (spawn; ACP and file tiers absent):
/// record native → record legacy → persona native → persona legacy
/// → global native → definition native
///
/// Global legacy is excluded end-to-end (plan v3 Delta 2).
pub(crate) fn apply_effort_bridge(
env: &mut std::collections::BTreeMap<String, String>,
runtime: Option<&crate::managed_agents::discovery::KnownAcpRuntime>,
record_env: &std::collections::BTreeMap<String, String>,
personas: &[crate::managed_agents::types::AgentDefinition],
persona_id: Option<&str>,
global_env: &std::collections::BTreeMap<String, String>,
harness_def: Option<&crate::managed_agents::custom_harnesses::HarnessDefinition>,
) {
use std::collections::BTreeMap;
let rt = match runtime {
Some(rt) => rt,
None => return,
};
// Strip foreign known effort keys for any runtime that has a native effort key,
// regardless of whether it has an effort_normalization contract.
// This ensures GOOSE_THINKING_EFFORT is absent from buzz-agent descriptors and vice versa.
if let Some(native_key) = &rt.thinking_env_var {
for &key in ALL_KNOWN_EFFORT_KEYS {
if key != *native_key {
env.remove(key);
}
}
}
// Effort tier resolution and alias normalization require effort_normalization.
let (norm, native_key) = match (&rt.effort_normalization, &rt.thinking_env_var) {
(Some(n), Some(k)) => (n, k),
_ => return,
};
let norm_fn = |raw: &str| norm.normalize_str(raw);
let mue = crate::managed_agents::env_vars::merged_user_env;
let is_reserved = crate::managed_agents::env_vars::is_reserved_env_key;
let live_persona_env = crate::managed_agents::env_vars::live_persona_env;
let s_record = mue(&BTreeMap::new(), record_env);
let s_persona = mue(&BTreeMap::new(), &live_persona_env(personas, persona_id));
let s_global = mue(&BTreeMap::new(), global_env);
let s_def: BTreeMap<String, String> = harness_def
.map(|d| {
d.env
.iter()
.filter(|(k, _)| !is_reserved(k))
.map(|(k, v)| (k.clone(), v.clone()))
.collect()
})
.unwrap_or_default();
let canonical = None
.or_else(|| effort_tier_alias(&s_record, native_key, norm_fn, false))
.or_else(|| effort_tier_alias(&s_persona, native_key, norm_fn, false))
.or_else(|| effort_tier_alias(&s_global, native_key, norm_fn, true))
.or_else(|| effort_tier_alias(&s_def, native_key, norm_fn, false));
// Remove raw native key (may be alias-form or invalid); canonical re-inserted below.
env.remove(*native_key);
if let Some(value) = canonical {
env.insert(native_key.to_string(), value);
}
}
@@ -24,19 +24,8 @@
//!
//! ## Env-assembly precedence (mirrors `spawn_agent_child`)
//!
//! 1. Baked build defaults (`baked_build_env()`) — injected first so the
//! layers above can override them.
//! 2. Runtime metadata env vars (`runtime_metadata_env_vars`) — provider /
//! model env keys derived from the record's `model`/`provider` fields and
//! the runtime's `model_env_var`/`provider_env_var`.
//! 3. Merged user env (`merged_user_env`) — live persona env under the
//! record's `env_vars` overrides, after reserved-key and malformed-key
//! filtering. Last-wins on collision.
//!
//! The config-file tier (Goose `~/.config/goose/config.yaml`) is tracked
//! separately because it is not part of the process env — the harness reads
//! it at startup. We do not evaluate it here; it is exposed for future
//! UI display only.
//! Baked build defaults → runtime metadata env → merged user env.
//! Config-file tier (Goose `~/.config/goose/config.yaml`) tracked separately.
use serde::{Deserialize, Serialize};
use std::collections::BTreeMap;
@@ -250,17 +239,12 @@ fn resolve_effective_agent_env_with_def(
}
}
// Layer 3a: global env vars — the lowest user-settable layer.
// Injected before persona/agent so per-agent values win on collision.
// `merged_user_env` with an empty "lower" map applies reserved/malformed-key
// filtering to the global map for free.
// Layer 3a: global env vars (lowest user-settable layer).
// `merged_user_env` with empty lower map applies reserved/malformed-key filtering.
let global_env = merged_user_env(&BTreeMap::new(), &global.env_vars);
env.extend(global_env);
// Layer 3b: merged user env — live persona env under the record's own
// overrides (last-wins), after reserved/malformed-key filtering. Reading
// the persona live is what makes persona credential edits refresh on the
// next spawn instead of being frozen into the record.
// Layer 3b: merged user env — persona env under record overrides, filtered.
let user_env = merged_user_env(
&super::env_vars::live_persona_env(personas, record.persona_id.as_deref()),
&record.env_vars,
@@ -276,6 +260,17 @@ fn resolve_effective_agent_env_with_def(
effective_model.as_deref(),
);
// Phase 3: translate legacy effort key → native key, strip foreign effort keys.
crate::managed_agents::config_bridge::apply_effort_bridge(
&mut env,
runtime,
&record.env_vars,
personas,
record.persona_id.as_deref(),
&global.env_vars,
harness_def.as_deref(),
);
EffectiveAgentEnv {
env,
config_file_path: runtime.and_then(|r| r.config_file_path),
@@ -1739,3 +1734,8 @@ mod tests {
#[cfg(test)]
#[path = "readiness_goose_file_config_tests.rs"]
mod goose_file_config_tests;
// Phase 3 effort-bridge spawn tests live in a sibling file.
#[cfg(test)]
#[path = "readiness_effort_bridge_tests.rs"]
mod effort_bridge_tests;
@@ -0,0 +1,364 @@
//! Spawn-side legacy effort bridge tests (Phase 3).
//!
//! Tests for `resolve_effective_agent_env` when the runtime has `effort_normalization`.
//! Verifies: tier-first precedence, alias normalization, foreign-key stripping,
//! invalid-value skip-as-absent, and global-scope invariants.
//!
//! Included from `readiness.rs` via `#[path]`; `super::*` resolves against that module.
use std::collections::BTreeMap;
use super::*;
use crate::managed_agents::discovery::known_acp_runtime_exact;
fn goose_runtime() -> Option<&'static KnownAcpRuntime> {
known_acp_runtime_exact("goose")
}
fn empty_global() -> crate::managed_agents::global_config::GlobalAgentConfig {
Default::default()
}
fn make_record(
env_vars: BTreeMap<String, String>,
) -> crate::managed_agents::types::ManagedAgentRecord {
crate::managed_agents::types::ManagedAgentRecord {
pubkey: "test-pubkey".to_string(),
name: "test-agent".to_string(),
persona_id: None,
private_key_nsec: String::new(),
auth_tag: None,
relay_url: String::new(),
avatar_url: None,
acp_command: "goose-acp".to_string(),
agent_command: "goose".to_string(),
agent_command_override: None,
agent_args: vec![],
mcp_command: String::new(),
turn_timeout_seconds: 320,
idle_timeout_seconds: None,
max_turn_duration_seconds: None,
parallelism: 1,
system_prompt: None,
model: None,
provider: None,
persona_source_version: None,
env_vars,
start_on_app_launch: false,
auto_restart_on_config_change: true,
runtime_pid: None,
backend: Default::default(),
backend_agent_id: None,
provider_binary_path: None,
team_id: None,
persona_team_dir: None,
persona_name_in_team: None,
created_at: String::new(),
updated_at: String::new(),
last_started_at: None,
last_stopped_at: None,
last_exit_code: None,
last_error: None,
last_error_code: None,
respond_to: Default::default(),
respond_to_allowlist: vec![],
display_name: None,
slug: None,
runtime: None,
name_pool: Vec::new(),
is_builtin: false,
is_active: true,
shared: false,
source_team: None,
source_team_persona_slug: None,
catalog_source: None,
definition_respond_to: None,
definition_respond_to_allowlist: Vec::new(),
definition_parallelism: None,
relay_mesh: None,
}
}
fn make_persona(
id: &str,
env_vars: BTreeMap<String, String>,
) -> crate::managed_agents::types::AgentDefinition {
serde_json::from_value(serde_json::json!({
"id": id,
"display_name": "test-persona",
"system_prompt": "",
"env_vars": env_vars,
"created_at": "2026-01-01T00:00:00Z",
"updated_at": "2026-01-01T00:00:00Z"
}))
.unwrap()
}
fn env_with(pairs: &[(&str, &str)]) -> BTreeMap<String, String> {
pairs
.iter()
.map(|(k, v)| (k.to_string(), v.to_string()))
.collect()
}
// ── Bridge activation ───────────────────────────────────────────────────────
#[test]
fn goose_record_native_effort_is_translated_and_foreign_key_stripped() {
// Record has GOOSE_THINKING_EFFORT — bridge uses it. No legacy key present.
let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "high")]));
let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global());
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("high")
);
// No legacy key in output.
assert!(!effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"));
}
#[test]
fn goose_record_legacy_effort_migrated_to_native_key() {
// Record has only BUZZ_AGENT_THINKING_EFFORT — bridge translates it.
let record = make_record(env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "medium")]));
let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global());
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("medium"),
"legacy key must be translated to GOOSE_THINKING_EFFORT"
);
assert!(
!effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"),
"legacy key must be stripped from the descriptor"
);
}
// ── Tier-first precedence ────────────────────────────────────────────────────
#[test]
fn goose_record_native_beats_persona_native() {
// record GOOSE_THINKING_EFFORT=high, persona GOOSE_THINKING_EFFORT=low → high wins.
let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "high")]));
let persona = make_persona("p1", env_with(&[("GOOSE_THINKING_EFFORT", "low")]));
let mut record_with_persona = record;
record_with_persona.persona_id = Some("p1".to_string());
let effective = resolve_effective_agent_env(
&record_with_persona,
&[persona],
goose_runtime(),
&empty_global(),
);
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("high")
);
}
#[test]
fn goose_record_legacy_beats_persona_native_tier_first() {
// record BUZZ_AGENT_THINKING_EFFORT=high, persona GOOSE_THINKING_EFFORT=low
// → tier-first: record legacy wins over persona native.
let record = make_record(env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "high")]));
let persona = make_persona("p1", env_with(&[("GOOSE_THINKING_EFFORT", "low")]));
let mut r2 = record;
r2.persona_id = Some("p1".to_string());
let effective = resolve_effective_agent_env(&r2, &[persona], goose_runtime(), &empty_global());
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("high"),
"record legacy must beat persona native under tier-first rule"
);
}
#[test]
fn goose_global_native_excluded_from_legacy_fallback() {
// Only global has BUZZ_AGENT_THINKING_EFFORT (legacy at global tier).
// Global legacy is excluded end-to-end → no effort key in output.
let mut global = empty_global();
global
.env_vars
.insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "low".to_string());
let record = make_record(BTreeMap::new());
let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &global);
assert!(
!effective.env.contains_key("GOOSE_THINKING_EFFORT"),
"global legacy must not seed Goose effort (excluded end-to-end)"
);
assert!(
!effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"),
"global legacy must be stripped from Goose descriptor"
);
}
// ── Alias normalization ──────────────────────────────────────────────────────
#[test]
fn goose_alias_none_is_normalized_to_off() {
let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "none")]));
let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global());
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("off"),
"none→off alias normalization"
);
}
#[test]
fn goose_alias_xhigh_is_normalized_to_max() {
let record = make_record(env_with(&[("GOOSE_THINKING_EFFORT", "xhigh")]));
let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &empty_global());
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("max"),
"xhigh→max alias normalization"
);
}
#[test]
fn goose_invalid_value_skip_as_absent_allows_lower_tier_to_win() {
// Record has invalid "minimal" — skipped. Persona native "low" wins.
let record = make_record(env_with(&[("BUZZ_AGENT_THINKING_EFFORT", "minimal")]));
let persona = make_persona("p1", env_with(&[("GOOSE_THINKING_EFFORT", "low")]));
let mut r2 = record;
r2.persona_id = Some("p1".to_string());
let effective = resolve_effective_agent_env(&r2, &[persona], goose_runtime(), &empty_global());
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("low"),
"invalid record legacy must be skipped, persona native wins"
);
}
// ── Foreign-key stripping (global-scope coexist invariant) ─────────────────
#[test]
fn buzz_agent_global_effort_survives_when_goose_record_effort_migrated() {
// Global config has both BUZZ_AGENT_THINKING_EFFORT (buzz-agent's native key)
// and GOOSE_THINKING_EFFORT (Goose's native key). A Goose agent's effective
// descriptor must strip BUZZ_AGENT_THINKING_EFFORT (foreign to Goose) while
// honouring GOOSE_THINKING_EFFORT from the global tier.
//
// Note: global GOOSE_THINKING_EFFORT is treated as a global-native tier (not
// legacy), so it IS honoured. Global legacy (BUZZ_AGENT_THINKING_EFFORT for
// Goose) is excluded. Only the native key appears in the Goose descriptor.
let mut global = empty_global();
global
.env_vars
.insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string());
global
.env_vars
.insert("GOOSE_THINKING_EFFORT".to_string(), "low".to_string());
let record = make_record(BTreeMap::new());
let effective = resolve_effective_agent_env(&record, &[], goose_runtime(), &global);
// Global native (GOOSE_THINKING_EFFORT=low) wins.
assert_eq!(
effective
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("low"),
"global native GOOSE_THINKING_EFFORT must reach Goose descriptor"
);
// Foreign key must be stripped from Goose descriptor.
assert!(
!effective.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"),
"BUZZ_AGENT_THINKING_EFFORT must be stripped from Goose descriptor"
);
}
// ── Bidirectional global coexist: each runtime sees only its own key ────────
#[test]
fn buzz_agent_global_effort_descriptor_strips_goose_native_key() {
// Both BUZZ_AGENT_THINKING_EFFORT and GOOSE_THINKING_EFFORT are in global
// config. A buzz-agent descriptor must contain only BUZZ_AGENT_THINKING_EFFORT;
// GOOSE_THINKING_EFFORT (foreign to buzz-agent) must be stripped.
// Paired with buzz_agent_global_effort_survives_when_goose_record_effort_migrated
// to assert the global coexist invariant from both directions.
use crate::managed_agents::discovery::known_acp_runtime_exact;
let buzz_agent_runtime = known_acp_runtime_exact("buzz-agent");
let mut global = empty_global();
global
.env_vars
.insert("BUZZ_AGENT_THINKING_EFFORT".to_string(), "high".to_string());
global
.env_vars
.insert("GOOSE_THINKING_EFFORT".to_string(), "low".to_string());
let record = make_record(std::collections::BTreeMap::new());
let effective = resolve_effective_agent_env(&record, &[], buzz_agent_runtime, &global);
// buzz-agent's native key must be present.
assert_eq!(
effective
.env
.get("BUZZ_AGENT_THINKING_EFFORT")
.map(String::as_str),
Some("high"),
"buzz-agent native key must survive in global coexist scenario"
);
// Foreign key must be stripped.
assert!(
!effective.env.contains_key("GOOSE_THINKING_EFFORT"),
"GOOSE_THINKING_EFFORT must be stripped from buzz-agent descriptor"
);
}
#[test]
fn global_coexist_both_runtimes_retain_own_key_independently() {
// Using the same global config, build descriptors for both Goose and buzz-agent
// and verify each sees only its own key. Proves the invariant is symmetric.
use crate::managed_agents::discovery::known_acp_runtime_exact;
let goose = goose_runtime();
let buzz = known_acp_runtime_exact("buzz-agent");
let mut global = empty_global();
global.env_vars.insert(
"BUZZ_AGENT_THINKING_EFFORT".to_string(),
"medium".to_string(),
);
global
.env_vars
.insert("GOOSE_THINKING_EFFORT".to_string(), "high".to_string());
let record = make_record(std::collections::BTreeMap::new());
let goose_eff = resolve_effective_agent_env(&record, &[], goose, &global);
let buzz_eff = resolve_effective_agent_env(&record, &[], buzz, &global);
// Goose sees its own key.
assert_eq!(
goose_eff
.env
.get("GOOSE_THINKING_EFFORT")
.map(String::as_str),
Some("high")
);
assert!(!goose_eff.env.contains_key("BUZZ_AGENT_THINKING_EFFORT"));
// buzz-agent sees its own key.
assert_eq!(
buzz_eff
.env
.get("BUZZ_AGENT_THINKING_EFFORT")
.map(String::as_str),
Some("medium")
);
assert!(!buzz_eff.env.contains_key("GOOSE_THINKING_EFFORT"));
}