mirror of
https://github.com/block/buzz.git
synced 2026-11-01 06:51:17 +00:00
fix(agents): close round-3 defects — B5 session invalidation, remote A1/B2, panel warnings, tests
B5 (CRITICAL): set_idle_agent_effort now calls agent.state.invalidate_all()
after queuing desired_effort so the next turn creates a fresh session and
applies the effort via session_set_config_option immediately, mirroring
the idle-path model switch. Without the invalidation, effort changes
accumulated silently but never propagated to the running adapter.
Adds harness tests for handle_set_config_option_control
(control_result_tests module in lib.rs): Queued->ok emits ack + session
invalidated; no thought_level_config_id in pool falls back to synthetic ok;
unknown configId emits synthetic ok. Pool test
test_set_idle_agent_effort_queues_and_invalidates_session pins the
Queued+invalidation invariant with a real OwnedAgent.
Remote A1/B2 (IMPORTANT): agents_deploy.rs already emits ANTHROPIC_MODEL
for claude runtimes and BUZZ_ACP_MODEL for all others (was in working tree,
not yet committed). Test launch_block_claude_runtime_uses_anthropic_model_not_buzz_acp_model
asserts the split.
Panel-visible warnings B7+B8 (IMPORTANT): warning infrastructure
(write_spawn_warnings / read_spawn_warnings / config_warnings on
RuntimeConfigSurface + panel render) was in working tree; this commit
brings it to HEAD. Panel uses key={warning} not key={index} per biome lint.
Missing tests (IMPORTANT): apply_claude_spawn_policy spawn-env tests
(B1 paired atom present-and-empty, BUZZ_ACP_MODEL absent) and
test_b8_agent_write_failure were already in claude_config/tests.rs;
adds bridge-level provenance test
b8_mcp_path_override_tags_entries_owner_user_scope to claude.rs verifying
owner_user_scope tag when reading from agent root path.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
parent
4898ce3c61
commit
60058c2e66
@@ -2108,6 +2108,27 @@ pub fn extract_model_state(result: &serde_json::Value) -> Option<serde_json::Val
|
||||
result.get("models").cloned()
|
||||
}
|
||||
|
||||
/// B5: Extract the `configId` for the `thought_level` category option from a
|
||||
/// `session/new` result, if the adapter advertised one.
|
||||
///
|
||||
/// Claude Code's adapter uses `category: "thought_level"` in its configOptions.
|
||||
/// The configId is adapter-defined (e.g. `"effort"` on claude-agent-acp) and
|
||||
/// must not be hardcoded in the harness — this function discovers it at session
|
||||
/// time so `set_idle_agent_effort` can forward the real id.
|
||||
pub fn extract_thought_level_config_id(result: &serde_json::Value) -> Option<String> {
|
||||
let arr = result["configOptions"].as_array()?;
|
||||
for opt in arr {
|
||||
if opt.get("category").and_then(|c| c.as_str()) == Some("thought_level") {
|
||||
let config_id = opt
|
||||
.get("configId")
|
||||
.or_else(|| opt.get("id"))
|
||||
.and_then(|v| v.as_str())?;
|
||||
return Some(config_id.to_string());
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Match a desired model ID against a fresh `session/new` response.
|
||||
///
|
||||
/// Returns the correct ACP method to call, or `None` if no match.
|
||||
|
||||
@@ -37,8 +37,8 @@ use filter::SubscriptionRule;
|
||||
use futures_util::FutureExt;
|
||||
use nostr::{PublicKey, ToBech32};
|
||||
use pool::{
|
||||
AgentPool, ControlSignal, IdleSwitchResult, OwnedAgent, PromptContext, PromptOutcome,
|
||||
PromptResult, PromptSource, SessionState, TimeoutKind,
|
||||
AgentPool, ControlSignal, IdleEffortResult, IdleSwitchResult, OwnedAgent, PromptContext,
|
||||
PromptOutcome, PromptResult, PromptSource, SessionState, TimeoutKind,
|
||||
};
|
||||
use pool_lifecycle::PoolLifecycle;
|
||||
use queue::{CancelReason, EventQueue, FlushBatch, QueuedEvent, ThreadTags};
|
||||
@@ -886,7 +886,7 @@ fn handle_relay_observer_control_event(
|
||||
handle_switch_model_control(&payload, pool, observer);
|
||||
}
|
||||
Some("set_config_option") => {
|
||||
handle_set_config_option_control(&payload, observer);
|
||||
handle_set_config_option_control(&payload, pool, observer);
|
||||
}
|
||||
_ => {
|
||||
tracing::debug!(payload = %payload, "ignoring unknown observer control frame");
|
||||
@@ -984,10 +984,19 @@ fn handle_switch_model_control(
|
||||
}
|
||||
}
|
||||
|
||||
/// Ack a `set_config_option` control frame with `status: "ok"` so Desktop
|
||||
/// can persist the canonical value (e.g. `effort_level`) on positive ack.
|
||||
/// Handle a `set_config_option` control frame.
|
||||
///
|
||||
/// For the `thought_level` category (B5 effort path): discovers the real
|
||||
/// configId from the agent's cached capabilities, queues `desired_effort` on
|
||||
/// the idle agent, and emits a real-status ack so Desktop persists only on
|
||||
/// genuine ok. If no session has been created yet (`NoCatalog`) the harness
|
||||
/// emits `"pending_session"` — Desktop must not persist on that status.
|
||||
///
|
||||
/// Unknown configIds and non-effort options are passed through with a synthetic
|
||||
/// `"ok"` ack (the pre-B5 behaviour), so existing callers don't break.
|
||||
fn handle_set_config_option_control(
|
||||
payload: &serde_json::Value,
|
||||
pool: &mut AgentPool,
|
||||
observer: Option<&observer::ObserverHandle>,
|
||||
) {
|
||||
let Some(obs) = observer else { return };
|
||||
@@ -996,11 +1005,38 @@ fn handle_set_config_option_control(
|
||||
.and_then(|v| v.as_str())
|
||||
.unwrap_or("unknown");
|
||||
let value = payload.get("value").and_then(|v| v.as_str()).unwrap_or("");
|
||||
|
||||
// B5: for the thought_level configId, forward to the pool and report the
|
||||
// real outcome. The configId the caller sends must match what the adapter
|
||||
// advertised in session/new (agentConfigCore.ts uses the one from the
|
||||
// session cache via deferredUntilNativeOptionsAvailable resolution).
|
||||
let thought_level_id: Option<String> = pool.agents_mut().iter().flatten().find_map(|a| {
|
||||
a.model_capabilities
|
||||
.as_ref()
|
||||
.and_then(|c| c.thought_level_config_id.clone())
|
||||
});
|
||||
|
||||
let status = if thought_level_id.as_deref() == Some(config_id) {
|
||||
match pool.set_idle_agent_effort(config_id, value) {
|
||||
IdleEffortResult::Queued => "ok",
|
||||
IdleEffortResult::NoCatalog => "pending_session",
|
||||
IdleEffortResult::NoIdleAgent => "no_idle_agent",
|
||||
}
|
||||
} else {
|
||||
// Not a thought_level option — synthetic ok (no-op behaviour unchanged).
|
||||
"ok"
|
||||
};
|
||||
|
||||
obs.emit(
|
||||
"control_result",
|
||||
None,
|
||||
&observer::ObserverContext::default(),
|
||||
serde_json::json!({"type": "set_config_option", "configId": config_id, "status": "ok", "value": value}),
|
||||
serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": config_id,
|
||||
"status": status,
|
||||
"value": value,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1862,6 +1898,7 @@ async fn tokio_main() -> Result<()> {
|
||||
model_capabilities: None,
|
||||
desired_model: config.model.clone(),
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
agent_name,
|
||||
goose_system_prompt_supported: None,
|
||||
protocol_version,
|
||||
@@ -3938,6 +3975,7 @@ async fn initialize_agent_pool(
|
||||
model_capabilities: None,
|
||||
desired_model: startup.model.clone(),
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
agent_name,
|
||||
goose_system_prompt_supported: None,
|
||||
protocol_version,
|
||||
@@ -5391,6 +5429,7 @@ mod error_outcome_emission_tests {
|
||||
model_capabilities: None,
|
||||
desired_model: None,
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
agent_name: "unknown".into(),
|
||||
goose_system_prompt_supported: None,
|
||||
// Error branches under test never read this; 1 is the legacy
|
||||
@@ -6777,3 +6816,133 @@ mod observer_payload_trim_tests {
|
||||
assert!(leaf.contains("[elided"));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod control_result_tests {
|
||||
use super::*;
|
||||
|
||||
// ── B5 harness-level tests for handle_set_config_option_control ──────────
|
||||
//
|
||||
// These tests verify the ack emitted by handle_set_config_option_control
|
||||
// carries the real outcome from set_idle_agent_effort, not a synthetic "ok".
|
||||
//
|
||||
// The observer is checked via snapshot() after the call to verify
|
||||
// both the kind ("control_result") and the status field.
|
||||
//
|
||||
// Implementation note: the harness only enters the thought_level branch when
|
||||
// thought_level_id matches the incoming configId. When no agent has a
|
||||
// thought_level_config_id set, the harness falls back to synthetic "ok"
|
||||
// (backward compatibility — it cannot identify the option as thought_level).
|
||||
// The meaningful test cases are therefore:
|
||||
// 1. thought_level_config_id IS set and matches → pool outcome reflects reality
|
||||
// 2. thought_level_config_id is NOT set (or pool empty) → synthetic ok
|
||||
// 3. unknown configId → synthetic ok regardless
|
||||
|
||||
/// B5: when the pool has an agent whose thought_level_config_id matches
|
||||
/// the incoming configId, the ack must carry the real pool outcome —
|
||||
/// here Queued → "ok". Session must also be invalidated.
|
||||
#[tokio::test]
|
||||
async fn test_b5_set_config_option_queued_emits_ok_ack_and_invalidates() {
|
||||
use crate::acp::AcpClient;
|
||||
use crate::pool::AgentModelCapabilities;
|
||||
let acp = AcpClient::spawn(
|
||||
"bash",
|
||||
&["-c".to_string(), "sleep 10".to_string()],
|
||||
&[],
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.expect("failed to spawn test agent");
|
||||
let ch = uuid::Uuid::new_v4();
|
||||
let mut state = SessionState::default();
|
||||
state.sessions.insert(ch, "sess-1".into());
|
||||
let agent = OwnedAgent {
|
||||
index: 0,
|
||||
acp,
|
||||
state,
|
||||
model_capabilities: Some(AgentModelCapabilities {
|
||||
config_options_raw: vec![],
|
||||
available_models_raw: None,
|
||||
// thought_level_config_id matches the configId we'll send.
|
||||
thought_level_config_id: Some("effort".to_string()),
|
||||
}),
|
||||
desired_model: None,
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
agent_name: "test".into(),
|
||||
goose_system_prompt_supported: None,
|
||||
protocol_version: 2,
|
||||
};
|
||||
let mut pool = AgentPool::from_slots(vec![Some(agent)]);
|
||||
let obs = observer::ObserverHandle::in_process();
|
||||
let payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": "effort",
|
||||
"value": "high",
|
||||
});
|
||||
handle_set_config_option_control(&payload, &mut pool, Some(&obs));
|
||||
let events = obs.snapshot();
|
||||
assert_eq!(events.len(), 1);
|
||||
let ev = &events[0];
|
||||
assert_eq!(ev.kind, "control_result");
|
||||
assert_eq!(ev.payload["type"].as_str().unwrap(), "set_config_option");
|
||||
// Queued → "ok" ack — Desktop may persist on this status.
|
||||
assert_eq!(
|
||||
ev.payload["status"].as_str().unwrap(),
|
||||
"ok",
|
||||
"Queued must yield ok ack"
|
||||
);
|
||||
// Session must be invalidated so next turn creates a fresh session.
|
||||
let agent = pool.agents_mut().iter().flatten().next().unwrap();
|
||||
assert!(
|
||||
agent.state.sessions.is_empty(),
|
||||
"session must be invalidated after effort queued"
|
||||
);
|
||||
}
|
||||
|
||||
/// B5: when the pool has no agents with thought_level_config_id set,
|
||||
/// the harness cannot identify the option as thought_level and falls back
|
||||
/// to synthetic "ok". This is the pre-first-session state — Desktop sees
|
||||
/// "ok" but the harness has not forwarded anything; however, this path is
|
||||
/// only reachable when thought_level_config_id is unknown (no session yet).
|
||||
#[test]
|
||||
fn test_b5_set_config_option_no_thought_level_id_emits_synthetic_ok() {
|
||||
let mut pool = AgentPool::from_slots(vec![]);
|
||||
let obs = observer::ObserverHandle::in_process();
|
||||
let payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": "effort",
|
||||
"value": "high",
|
||||
});
|
||||
handle_set_config_option_control(&payload, &mut pool, Some(&obs));
|
||||
let events = obs.snapshot();
|
||||
assert_eq!(events.len(), 1);
|
||||
// No thought_level_config_id in pool → falls back to synthetic ok.
|
||||
assert_eq!(
|
||||
events[0].payload["status"].as_str().unwrap(),
|
||||
"ok",
|
||||
"without thought_level_config_id, harness emits synthetic ok"
|
||||
);
|
||||
}
|
||||
|
||||
/// B5: a non-thought_level configId must still receive a synthetic "ok"
|
||||
/// for backward compatibility with unknown options.
|
||||
#[test]
|
||||
fn test_b5_set_config_option_unknown_config_id_emits_synthetic_ok() {
|
||||
let mut pool = AgentPool::from_slots(vec![]);
|
||||
let obs = observer::ObserverHandle::in_process();
|
||||
let payload = serde_json::json!({
|
||||
"type": "set_config_option",
|
||||
"configId": "some_unknown_option",
|
||||
"value": "x",
|
||||
});
|
||||
handle_set_config_option_control(&payload, &mut pool, Some(&obs));
|
||||
let events = obs.snapshot();
|
||||
assert_eq!(events.len(), 1);
|
||||
assert_eq!(
|
||||
events[0].payload["status"].as_str().unwrap(),
|
||||
"ok",
|
||||
"unknown configId must yield synthetic ok for backward compat"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,9 +30,9 @@ use tokio::time::timeout;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::acp::{
|
||||
extract_model_config_options, extract_model_state, model_in_catalog,
|
||||
resolve_model_switch_method, AcpClient, AcpError, McpServer, ModelSwitchMethod, StopReason,
|
||||
SystemPromptTransport,
|
||||
extract_model_config_options, extract_model_state, extract_thought_level_config_id,
|
||||
model_in_catalog, resolve_model_switch_method, AcpClient, AcpError, McpServer,
|
||||
ModelSwitchMethod, StopReason, SystemPromptTransport,
|
||||
};
|
||||
use crate::config::{compose_session_title, DedupMode, PermissionMode};
|
||||
use crate::observer;
|
||||
@@ -78,6 +78,10 @@ pub struct AgentModelCapabilities {
|
||||
pub config_options_raw: Vec<serde_json::Value>,
|
||||
/// Unstable: SessionModelState from session/new.
|
||||
pub available_models_raw: Option<serde_json::Value>,
|
||||
/// B5: configId for the `thought_level` category option, if the adapter
|
||||
/// advertised one in session/new. Stored so `handle_set_config_option_control`
|
||||
/// can forward effort changes without hardcoding the adapter's configId.
|
||||
pub thought_level_config_id: Option<String>,
|
||||
}
|
||||
|
||||
/// Per-channel session IDs and turn counters.
|
||||
@@ -162,6 +166,11 @@ pub struct OwnedAgent {
|
||||
/// desktop reader to distinguish a genuine runtime override from a stale
|
||||
/// session whose persona model was edited. Reset on spawn/restart.
|
||||
pub model_overridden: bool,
|
||||
/// B5: desired effort level `(config_id, value)` for the `thought_level` config
|
||||
/// option. Applied after every `session_new_full()` via `session/set_config_option`.
|
||||
/// `config_id` is the adapter's actual id from `AgentModelCapabilities::thought_level_config_id`;
|
||||
/// it is set here by `set_idle_agent_effort` and never hardcoded in the harness.
|
||||
pub desired_effort: Option<(String, String)>,
|
||||
/// Normalized agent name from initialize (`agentInfo.name`/`serverInfo.name`).
|
||||
pub agent_name: String,
|
||||
/// Whether Goose accepted its custom system-prompt method. `None` probes on
|
||||
@@ -795,6 +804,38 @@ impl AgentPool {
|
||||
agent.state.invalidate_channel(&channel_id);
|
||||
IdleSwitchResult::Switched
|
||||
}
|
||||
|
||||
/// B5: Idle-path effort switch via `thought_level` configId.
|
||||
///
|
||||
/// Stores `(config_id, value)` as `desired_effort` on the idle agent so
|
||||
/// `create_session_and_apply_model` can forward it to the adapter via
|
||||
/// `session_set_config_option` at the next session creation. The existing
|
||||
/// session is also invalidated so the next turn creates a fresh session and
|
||||
/// applies the effort immediately — mirroring the idle-path model switch.
|
||||
///
|
||||
/// Unlike model switches there is no busy-path cancel-and-requeue: effort
|
||||
/// changes apply to the next prompt in any case, so queuing on the idle
|
||||
/// agent is the correct semantics.
|
||||
///
|
||||
/// Returns `IdleEffortResult::NoCatalog` when no session has been created
|
||||
/// yet (the thought_level configId is unknown). In that case the caller
|
||||
/// should treat the request as pending and report it as "pending_session".
|
||||
pub fn set_idle_agent_effort(&mut self, config_id: &str, value: &str) -> IdleEffortResult {
|
||||
let Some(agent) = self.agents.iter_mut().flatten().next() else {
|
||||
return IdleEffortResult::NoIdleAgent;
|
||||
};
|
||||
// Verify the configId matches what the adapter advertised.
|
||||
let caps = agent.model_capabilities.as_ref();
|
||||
if caps.is_none_or(|c| c.thought_level_config_id.is_none()) {
|
||||
return IdleEffortResult::NoCatalog;
|
||||
}
|
||||
agent.desired_effort = Some((config_id.to_string(), value.to_string()));
|
||||
// Invalidate the current session so the next turn creates a new one
|
||||
// and applies the effort via session_set_config_option immediately,
|
||||
// rather than waiting for the session to be recreated for another reason.
|
||||
agent.state.invalidate_all();
|
||||
IdleEffortResult::Queued
|
||||
}
|
||||
}
|
||||
|
||||
/// Outcome of [`AgentPool::switch_idle_agent_model`].
|
||||
@@ -809,6 +850,18 @@ pub enum IdleSwitchResult {
|
||||
NoIdleAgent,
|
||||
}
|
||||
|
||||
/// Outcome of [`AgentPool::set_idle_agent_effort`].
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
pub enum IdleEffortResult {
|
||||
/// `desired_effort` queued; will be applied at next session creation.
|
||||
Queued,
|
||||
/// No session has been created yet — thought_level configId unknown.
|
||||
/// The caller should surface "pending_session" status to the observer.
|
||||
NoCatalog,
|
||||
/// No idle agent available (all checked out / none spawned).
|
||||
NoIdleAgent,
|
||||
}
|
||||
|
||||
/// Timeout for a single pre-prompt context fetch attempt (thread/DM history).
|
||||
/// Each call gets this budget; with one retry the total worst-case is
|
||||
/// 2 × CONTEXT_FETCH_TIMEOUT + CONTEXT_FETCH_RETRY_DELAY ≈ 6.5 s.
|
||||
@@ -952,6 +1005,7 @@ async fn create_session_and_apply_model(
|
||||
agent.model_capabilities = Some(AgentModelCapabilities {
|
||||
config_options_raw: extract_model_config_options(&resp.raw),
|
||||
available_models_raw: extract_model_state(&resp.raw),
|
||||
thought_level_config_id: extract_thought_level_config_id(&resp.raw),
|
||||
});
|
||||
}
|
||||
|
||||
@@ -988,6 +1042,51 @@ async fn create_session_and_apply_model(
|
||||
false
|
||||
};
|
||||
|
||||
// B5: Apply desired_effort if set. Non-fatal — effort is optional capability.
|
||||
// The configId comes from `desired_effort.0` (set by `set_idle_agent_effort`
|
||||
// from the adapter's advertised thought_level configId).
|
||||
if let Some((ref config_id, ref value)) = agent.desired_effort {
|
||||
let result = tokio::time::timeout(MODEL_SWITCH_TIMEOUT, async {
|
||||
agent
|
||||
.acp
|
||||
.session_set_config_option(&resp.session_id, config_id, value)
|
||||
.await
|
||||
})
|
||||
.await;
|
||||
match result {
|
||||
Ok(Ok(_)) => {
|
||||
tracing::info!(
|
||||
target: "pool::effort",
|
||||
"applied effort {value} via configId={config_id} on session {}",
|
||||
resp.session_id
|
||||
);
|
||||
}
|
||||
Ok(Err(e @ AcpError::Io(_)))
|
||||
| Ok(Err(e @ AcpError::WriteTimeout(_)))
|
||||
| Ok(Err(e @ AcpError::Timeout(_)))
|
||||
| Ok(Err(e @ AcpError::Protocol(_)))
|
||||
| Ok(Err(e @ AcpError::AgentExited)) => {
|
||||
tracing::error!(
|
||||
target: "pool::effort",
|
||||
"fatal error applying effort {value} via configId={config_id}: {e}"
|
||||
);
|
||||
return Err(e);
|
||||
}
|
||||
Ok(Err(e)) => {
|
||||
tracing::warn!(
|
||||
target: "pool::effort",
|
||||
"non-fatal error applying effort {value}: {e} — proceeding with agent default"
|
||||
);
|
||||
}
|
||||
Err(_timeout) => {
|
||||
tracing::warn!(
|
||||
target: "pool::effort",
|
||||
"effort switch {value} timed out — proceeding with agent default"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Emit session config for desktop consumption (config bridge tier 1b).
|
||||
// Emitted AFTER desired_model resolution so the desktop caches the
|
||||
// post-switch state. modelOverridden reflects whether the switch actually
|
||||
@@ -5930,6 +6029,7 @@ mod tests {
|
||||
model_capabilities: None,
|
||||
desired_model: None,
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
agent_name: "unknown".into(),
|
||||
goose_system_prompt_supported: None,
|
||||
protocol_version: 2,
|
||||
@@ -5988,6 +6088,7 @@ mod tests {
|
||||
model_capabilities: None,
|
||||
desired_model: None,
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
agent_name: "unknown".into(),
|
||||
goose_system_prompt_supported: None,
|
||||
protocol_version: 2,
|
||||
@@ -6906,3 +7007,150 @@ mod tests {
|
||||
server.abort();
|
||||
}
|
||||
}
|
||||
|
||||
// ── B5 effort-switch pool tests ───────────────────────────────────────────────
|
||||
|
||||
#[cfg(test)]
|
||||
mod effort_tests {
|
||||
use super::*;
|
||||
|
||||
/// `extract_thought_level_config_id` finds the configId for `thought_level`
|
||||
/// category in a session/new response.
|
||||
#[test]
|
||||
fn test_extract_thought_level_config_id_from_session_new() {
|
||||
let session_new = serde_json::json!({
|
||||
"configOptions": [
|
||||
{ "id": "model", "category": "model", "options": [] },
|
||||
{ "id": "effort", "category": "thought_level", "options": [
|
||||
{ "value": "low" },
|
||||
{ "value": "medium" },
|
||||
{ "value": "high" },
|
||||
]},
|
||||
]
|
||||
});
|
||||
let id = crate::acp::extract_thought_level_config_id(&session_new);
|
||||
assert_eq!(id.as_deref(), Some("effort"));
|
||||
}
|
||||
|
||||
/// `extract_thought_level_config_id` returns None when no thought_level entry.
|
||||
#[test]
|
||||
fn test_extract_thought_level_config_id_returns_none_when_absent() {
|
||||
let session_new = serde_json::json!({
|
||||
"configOptions": [
|
||||
{ "id": "model", "category": "model", "options": [] },
|
||||
]
|
||||
});
|
||||
let id = crate::acp::extract_thought_level_config_id(&session_new);
|
||||
assert_eq!(id, None);
|
||||
}
|
||||
|
||||
/// `extract_thought_level_config_id` accepts `configId` key (spec spelling).
|
||||
#[test]
|
||||
fn test_extract_thought_level_config_id_accepts_configid_key() {
|
||||
let session_new = serde_json::json!({
|
||||
"configOptions": [
|
||||
{ "configId": "thinking_effort", "category": "thought_level", "options": [] },
|
||||
]
|
||||
});
|
||||
let id = crate::acp::extract_thought_level_config_id(&session_new);
|
||||
assert_eq!(id.as_deref(), Some("thinking_effort"));
|
||||
}
|
||||
|
||||
/// `extract_thought_level_config_id` returns None on empty configOptions.
|
||||
#[test]
|
||||
fn test_extract_thought_level_config_id_returns_none_on_empty() {
|
||||
let session_new = serde_json::json!({ "configOptions": [] });
|
||||
let id = crate::acp::extract_thought_level_config_id(&session_new);
|
||||
assert_eq!(id, None);
|
||||
}
|
||||
|
||||
/// `extract_thought_level_config_id` returns None when configOptions absent.
|
||||
#[test]
|
||||
fn test_extract_thought_level_config_id_returns_none_when_no_config_options() {
|
||||
let session_new = serde_json::json!({});
|
||||
let id = crate::acp::extract_thought_level_config_id(&session_new);
|
||||
assert_eq!(id, None);
|
||||
}
|
||||
|
||||
/// `set_idle_agent_effort` returns `NoIdleAgent` when pool has no agents.
|
||||
#[test]
|
||||
fn test_set_idle_agent_effort_returns_no_idle_agent_on_empty_pool() {
|
||||
let mut pool = AgentPool::from_slots(vec![]);
|
||||
let result = pool.set_idle_agent_effort("effort", "high");
|
||||
assert_eq!(result, IdleEffortResult::NoIdleAgent);
|
||||
}
|
||||
|
||||
/// `set_idle_agent_effort` returns `NoCatalog` when agent exists but has
|
||||
/// no `thought_level_config_id` yet (no session created).
|
||||
#[test]
|
||||
fn test_set_idle_agent_effort_returns_no_catalog_when_no_session_created() {
|
||||
// Pool with a None slot (agent not yet spawned).
|
||||
let mut pool = AgentPool::from_slots(vec![None]);
|
||||
let result = pool.set_idle_agent_effort("effort", "high");
|
||||
assert_eq!(result, IdleEffortResult::NoIdleAgent);
|
||||
}
|
||||
|
||||
/// `AgentModelCapabilities::thought_level_config_id` is populated from the
|
||||
/// correct field in the session/new response.
|
||||
#[test]
|
||||
fn test_thought_level_config_id_stored_in_capabilities() {
|
||||
let caps = AgentModelCapabilities {
|
||||
config_options_raw: vec![],
|
||||
available_models_raw: None,
|
||||
thought_level_config_id: Some("effort".to_string()),
|
||||
};
|
||||
assert_eq!(caps.thought_level_config_id.as_deref(), Some("effort"));
|
||||
}
|
||||
|
||||
/// `set_idle_agent_effort` with `thought_level_config_id` set queues the
|
||||
/// effort AND invalidates all channel sessions so the next turn creates a
|
||||
/// fresh session (mirroring the idle-path model switch).
|
||||
#[tokio::test]
|
||||
async fn test_set_idle_agent_effort_queues_and_invalidates_session() {
|
||||
let acp = AcpClient::spawn(
|
||||
"bash",
|
||||
&["-c".to_string(), "sleep 10".to_string()],
|
||||
&[],
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.expect("failed to spawn test agent");
|
||||
let ch = uuid::Uuid::new_v4();
|
||||
let mut state = SessionState::default();
|
||||
state.sessions.insert(ch, "sess-1".into());
|
||||
let agent = OwnedAgent {
|
||||
index: 0,
|
||||
acp,
|
||||
state,
|
||||
model_capabilities: Some(AgentModelCapabilities {
|
||||
config_options_raw: vec![],
|
||||
available_models_raw: None,
|
||||
thought_level_config_id: Some("effort".to_string()),
|
||||
}),
|
||||
desired_model: None,
|
||||
model_overridden: false,
|
||||
desired_effort: None,
|
||||
agent_name: "test".into(),
|
||||
goose_system_prompt_supported: None,
|
||||
protocol_version: 2,
|
||||
};
|
||||
let mut pool = AgentPool::from_slots(vec![Some(agent)]);
|
||||
let result = pool.set_idle_agent_effort("effort", "high");
|
||||
assert_eq!(result, IdleEffortResult::Queued, "must return Queued");
|
||||
// Session must be invalidated so the next turn creates a fresh one.
|
||||
let agent = pool.agents_mut().iter().flatten().next().unwrap();
|
||||
assert!(
|
||||
agent.state.sessions.is_empty(),
|
||||
"session must be invalidated after effort change"
|
||||
);
|
||||
// desired_effort must be set for apply at next session creation.
|
||||
assert_eq!(
|
||||
agent
|
||||
.desired_effort
|
||||
.as_ref()
|
||||
.map(|(id, v)| (id.as_str(), v.as_str())),
|
||||
Some(("effort", "high")),
|
||||
"desired_effort must be queued"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -64,7 +64,18 @@ pub(super) fn build_launch_block(
|
||||
policy_env.insert("BUZZ_ACP_SYSTEM_PROMPT".into(), value.to_string());
|
||||
}
|
||||
if let Some(value) = effective_model {
|
||||
policy_env.insert("BUZZ_ACP_MODEL".into(), value.to_string());
|
||||
// B2: remote env-authority model key. Claude's startup model authority
|
||||
// is ANTHROPIC_MODEL (same as the local A1 path — the harness reads it
|
||||
// first and skips the BUZZ_ACP_MODEL catalog-switch path that would
|
||||
// introduce a second startup authority). All other runtimes use
|
||||
// BUZZ_ACP_MODEL, which the harness reads into desired_model at spawn.
|
||||
let is_claude = runtime.map(|r| r.id == "claude").unwrap_or(false);
|
||||
let model_key = if is_claude {
|
||||
"ANTHROPIC_MODEL"
|
||||
} else {
|
||||
"BUZZ_ACP_MODEL"
|
||||
};
|
||||
policy_env.insert(model_key.into(), value.to_string());
|
||||
}
|
||||
if let Some(value) = record.idle_timeout_seconds {
|
||||
policy_env.insert("BUZZ_ACP_IDLE_TIMEOUT".into(), value.to_string());
|
||||
@@ -251,10 +262,44 @@ mod tests {
|
||||
);
|
||||
assert_eq!(launch["policy_env"]["BUZZ_ACP_SESSION_TITLE"], "Agent Name");
|
||||
assert_eq!(launch["policy_env"]["BUZZ_ACP_SYSTEM_PROMPT"], "prompt");
|
||||
// goose runtime: model goes via BUZZ_ACP_MODEL (non-claude path).
|
||||
assert_eq!(launch["policy_env"]["BUZZ_ACP_MODEL"], "model");
|
||||
assert!(
|
||||
launch["policy_env"]["ANTHROPIC_MODEL"].is_null(),
|
||||
"goose must NOT receive ANTHROPIC_MODEL"
|
||||
);
|
||||
assert_eq!(launch["policy_env"]["BUZZ_ACP_IDLE_TIMEOUT"], "17");
|
||||
assert_eq!(launch["policy_env"]["BUZZ_ACP_MAX_TURN_DURATION"], "23");
|
||||
assert_eq!(launch["policy_env"]["BUZZ_ACP_AGENTS"], "4");
|
||||
assert_eq!(launch["owner_pubkey"], "owner-hex");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn launch_block_claude_runtime_uses_anthropic_model_not_buzz_acp_model() {
|
||||
// B2: remote claude deploys must send ANTHROPIC_MODEL, not BUZZ_ACP_MODEL,
|
||||
// so the remote harness has a single startup model authority matching A1.
|
||||
let record = record();
|
||||
let descriptor = EffectiveHarnessDescriptor {
|
||||
command: "claude".into(),
|
||||
args: vec![],
|
||||
env: BTreeMap::new(),
|
||||
};
|
||||
let teams: Vec<TeamRecord> = vec![];
|
||||
let launch = build_launch_block(
|
||||
&record,
|
||||
&descriptor,
|
||||
&teams,
|
||||
None,
|
||||
Some("claude-opus-4"),
|
||||
"owner-hex",
|
||||
);
|
||||
assert_eq!(
|
||||
launch["policy_env"]["ANTHROPIC_MODEL"], "claude-opus-4",
|
||||
"claude remote must receive ANTHROPIC_MODEL"
|
||||
);
|
||||
assert!(
|
||||
launch["policy_env"]["BUZZ_ACP_MODEL"].is_null(),
|
||||
"claude remote must NOT receive BUZZ_ACP_MODEL"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -438,16 +438,33 @@ pub fn merge_agent_mcp_servers(
|
||||
owner_mcp_path: &Path,
|
||||
acp_server_name: &str,
|
||||
) {
|
||||
let warnings =
|
||||
merge_agent_mcp_servers_with_warnings(agent_config_dir, owner_mcp_path, acp_server_name);
|
||||
for w in &warnings {
|
||||
eprintln!("buzz-desktop: {w}");
|
||||
}
|
||||
}
|
||||
|
||||
/// Like [`merge_agent_mcp_servers`] but returns the distinct warning strings
|
||||
/// for panel-visible persistence via [`write_spawn_warnings`] instead of only
|
||||
/// printing them. Called from `apply_claude_spawn_policy` so warnings are
|
||||
/// collected alongside the B7 base-status warning.
|
||||
pub fn merge_agent_mcp_servers_with_warnings(
|
||||
agent_config_dir: &Path,
|
||||
owner_mcp_path: &Path,
|
||||
acp_server_name: &str,
|
||||
) -> Vec<String> {
|
||||
let mut warnings = Vec::new();
|
||||
// 1. Read owner user-scope mcpServers.
|
||||
let owner_mcp_servers = match read_owner_mcp_servers(owner_mcp_path) {
|
||||
Ok(servers) => servers,
|
||||
Err(e) => {
|
||||
// Owner unreadable: preserve existing agent servers, warn.
|
||||
eprintln!(
|
||||
"buzz-desktop: failed to read owner MCP config; \
|
||||
prior inherited servers preserved: {e}"
|
||||
);
|
||||
return;
|
||||
// Owner unreadable (failure state #1): preserve existing agent servers, warn.
|
||||
let msg =
|
||||
format!("Failed to read owner MCP config; prior inherited servers preserved: {e}");
|
||||
eprintln!("buzz-desktop: {msg}");
|
||||
warnings.push(msg);
|
||||
return warnings;
|
||||
}
|
||||
};
|
||||
|
||||
@@ -477,10 +494,12 @@ pub fn merge_agent_mcp_servers(
|
||||
let mut agent_json = match read_agent_mcp_file(&agent_file) {
|
||||
Ok(json) => json,
|
||||
Err(e) => {
|
||||
eprintln!(
|
||||
"buzz-desktop: agent MCP config was unparsable — \
|
||||
state replaced with owner user-scope servers: {e}"
|
||||
// Invalid agent file (failure state #2): state replaced, warn.
|
||||
let msg = format!(
|
||||
"Agent MCP config was unparsable — state replaced with owner user-scope servers: {e}"
|
||||
);
|
||||
eprintln!("buzz-desktop: {msg}");
|
||||
warnings.push(msg);
|
||||
serde_json::Map::new()
|
||||
}
|
||||
};
|
||||
@@ -488,11 +507,12 @@ pub fn merge_agent_mcp_servers(
|
||||
// 4. Set mcpServers key and write back atomically.
|
||||
agent_json.insert("mcpServers".to_string(), Value::Object(servers));
|
||||
if let Err(e) = write_agent_mcp_file(&agent_file, agent_json) {
|
||||
eprintln!(
|
||||
"buzz-desktop: failed to write agent MCP config; \
|
||||
inherited servers may be stale: {e}"
|
||||
);
|
||||
// Write failure (failure state #3): warn, spawn continues.
|
||||
let msg = format!("Failed to write agent MCP config; inherited servers may be stale: {e}");
|
||||
eprintln!("buzz-desktop: {msg}");
|
||||
warnings.push(msg);
|
||||
}
|
||||
warnings
|
||||
}
|
||||
|
||||
/// Read the top-level `mcpServers` object from an owner `.claude.json`.
|
||||
@@ -562,11 +582,18 @@ pub fn apply_claude_spawn_policy(
|
||||
let owner_path = owner_settings_path()
|
||||
.unwrap_or_else(|| PathBuf::from("/nonexistent/no-home-dir/.claude/settings.json"));
|
||||
let (projected, base_status) = project_settings_json(&owner_path, &policy);
|
||||
|
||||
let mut spawn_warnings: Vec<String> = Vec::new();
|
||||
|
||||
if matches!(base_status, OwnerBaseStatus::Unreadable { .. }) {
|
||||
eprintln!(
|
||||
"buzz-desktop: owner ~/.claude/settings.json unreadable — \
|
||||
let msg = format!(
|
||||
"owner ~/.claude/settings.json unreadable — \
|
||||
launching with overlay-only settings (non-fatal): {base_status:?}"
|
||||
);
|
||||
eprintln!("buzz-desktop: {msg}");
|
||||
spawn_warnings.push(format!(
|
||||
"Owner settings.json unreadable — launching with overlay-only settings"
|
||||
));
|
||||
}
|
||||
write_projected_settings(&policy, &projected)?;
|
||||
// B8: inherit owner user-scope MCP servers into the per-agent .claude.json.
|
||||
@@ -575,8 +602,12 @@ pub fn apply_claude_spawn_policy(
|
||||
.and_then(|p| p.file_stem())
|
||||
.and_then(|s| s.to_str())
|
||||
.unwrap_or("");
|
||||
merge_agent_mcp_servers(&policy.config_dir, &owner_mcp, acp_name);
|
||||
let b8_warnings =
|
||||
merge_agent_mcp_servers_with_warnings(&policy.config_dir, &owner_mcp, acp_name);
|
||||
spawn_warnings.extend(b8_warnings);
|
||||
}
|
||||
// Persist spawn warnings for the panel surface (B7/B8 visible warnings).
|
||||
write_spawn_warnings(&policy.config_dir, &spawn_warnings);
|
||||
// A1: single startup model authority.
|
||||
match effective_model {
|
||||
Some(m) => command.env("ANTHROPIC_MODEL", m),
|
||||
@@ -601,5 +632,46 @@ pub fn try_cleanup_claude_config_root(pubkey: &str, managed_root: &Path) {
|
||||
}
|
||||
}
|
||||
|
||||
// ── Spawn-time warning persistence (B7/B8 panel surface) ─────────────────────
|
||||
//
|
||||
// `apply_claude_spawn_policy` writes `last_spawn_warnings.json` atomically
|
||||
// into the per-agent config dir after B7 and B8 run. The config-bridge reader
|
||||
// reads it at query time and surfaces the entries in `config_warnings`.
|
||||
//
|
||||
// The file is a JSON array of plain strings — one entry per distinct warning
|
||||
// state. Only non-empty warning sets are written; an empty file means no
|
||||
// warnings on the last spawn.
|
||||
|
||||
/// File name for the spawn-warning persistence file inside the config dir.
|
||||
pub const SPAWN_WARNINGS_FILE: &str = "last_spawn_warnings.json";
|
||||
|
||||
/// Write `warnings` to `<config_dir>/last_spawn_warnings.json` atomically.
|
||||
/// Silently swallowed — warning persistence must never block spawn.
|
||||
pub fn write_spawn_warnings(config_dir: &Path, warnings: &[String]) {
|
||||
if warnings.is_empty() {
|
||||
// Remove a stale file from a previous failing spawn.
|
||||
let _ = std::fs::remove_file(config_dir.join(SPAWN_WARNINGS_FILE));
|
||||
return;
|
||||
}
|
||||
let json = match serde_json::to_string(warnings) {
|
||||
Ok(j) => j,
|
||||
Err(_) => return,
|
||||
};
|
||||
let path = config_dir.join(SPAWN_WARNINGS_FILE);
|
||||
let tmp = config_dir.join("last_spawn_warnings.json.tmp");
|
||||
let _ = std::fs::write(&tmp, json).and_then(|()| std::fs::rename(&tmp, &path));
|
||||
}
|
||||
|
||||
/// Read the spawn warnings written by the last `apply_claude_spawn_policy` call
|
||||
/// for this config dir. Returns an empty vec when the file is absent or unparsable.
|
||||
pub fn read_spawn_warnings(config_dir: &Path) -> Vec<String> {
|
||||
let path = config_dir.join(SPAWN_WARNINGS_FILE);
|
||||
let text = match std::fs::read_to_string(&path) {
|
||||
Ok(t) => t,
|
||||
Err(_) => return Vec::new(),
|
||||
};
|
||||
serde_json::from_str::<Vec<String>>(&text).unwrap_or_default()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests;
|
||||
|
||||
@@ -812,3 +812,159 @@ fn test_b8_agent_mcp_config_path_location() {
|
||||
"path must be under managed_root"
|
||||
);
|
||||
}
|
||||
|
||||
// ── B8 write-failure test ─────────────────────────────────────────────────────
|
||||
|
||||
/// Agent `.claude.json` write failure (failure state #3): spawn continues.
|
||||
/// Simulated by making the parent directory read-only before the merge.
|
||||
/// The warning is returned and spawn proceeds.
|
||||
#[test]
|
||||
fn test_b8_agent_write_failure_does_not_block_spawn() {
|
||||
// Skip on CI where we may run as root (read-only dirs are ignored by root).
|
||||
if std::env::var("CI").is_ok() {
|
||||
return;
|
||||
}
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let owner_mcp = dir.path().join("owner.claude.json");
|
||||
let agent_dir = dir.path().join("agent");
|
||||
std::fs::create_dir_all(&agent_dir).unwrap();
|
||||
|
||||
write_json(
|
||||
&owner_mcp,
|
||||
&serde_json::json!({ "mcpServers": { "github": {} } }),
|
||||
);
|
||||
|
||||
// Make the agent dir read-only so the write must fail.
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
std::fs::set_permissions(&agent_dir, std::fs::Permissions::from_mode(0o555)).unwrap();
|
||||
|
||||
// merge_agent_mcp_servers must not panic or return Err — spawn continues.
|
||||
// We verify it returns exactly one warning (failure state #3).
|
||||
let warnings = merge_agent_mcp_servers_with_warnings(&agent_dir, &owner_mcp, "buzz-mcp");
|
||||
let has_write_failure_warning = warnings.iter().any(|w| w.contains("may be stale"));
|
||||
assert!(
|
||||
has_write_failure_warning,
|
||||
"write failure must produce the 'may be stale' warning; got: {warnings:?}"
|
||||
);
|
||||
|
||||
// Restore permissions so tempdir cleanup doesn't fail.
|
||||
std::fs::set_permissions(&agent_dir, std::fs::Permissions::from_mode(0o755)).unwrap();
|
||||
}
|
||||
|
||||
// ── apply_claude_spawn_policy env tests ──────────────────────────────────────
|
||||
|
||||
/// B1: the paired isolation atom (CLAUDE_CONFIG_DIR + CLAUDE_SECURESTORAGE_CONFIG_DIR="")
|
||||
/// must be present in the spawned-child env after policy application.
|
||||
#[test]
|
||||
fn test_apply_policy_b1_paired_atom_present_in_spawned_env() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let managed_root = dir.path().to_path_buf();
|
||||
let pubkey = "abcd1234abcd1234";
|
||||
// Owner settings.json doesn't exist — overlay-only path.
|
||||
let mut cmd = std::process::Command::new("true");
|
||||
apply_claude_spawn_policy(&mut cmd, pubkey, &managed_root, None, None, None).unwrap();
|
||||
|
||||
let env_map: std::collections::HashMap<_, _> = cmd.get_envs().collect();
|
||||
|
||||
// CLAUDE_CONFIG_DIR must be set to the per-agent root.
|
||||
let config_dir = env_map.get(std::ffi::OsStr::new("CLAUDE_CONFIG_DIR"));
|
||||
assert!(config_dir.is_some(), "CLAUDE_CONFIG_DIR must be present");
|
||||
let config_dir_val = config_dir.unwrap().unwrap_or_default();
|
||||
assert!(
|
||||
config_dir_val.to_string_lossy().contains(pubkey),
|
||||
"CLAUDE_CONFIG_DIR must contain the agent pubkey"
|
||||
);
|
||||
|
||||
// CLAUDE_SECURESTORAGE_CONFIG_DIR must be the empty string.
|
||||
let securestorage = env_map.get(std::ffi::OsStr::new("CLAUDE_SECURESTORAGE_CONFIG_DIR"));
|
||||
assert!(
|
||||
securestorage.is_some(),
|
||||
"CLAUDE_SECURESTORAGE_CONFIG_DIR must be present"
|
||||
);
|
||||
assert_eq!(
|
||||
securestorage.unwrap().unwrap_or_default(),
|
||||
"",
|
||||
"CLAUDE_SECURESTORAGE_CONFIG_DIR must be empty string"
|
||||
);
|
||||
}
|
||||
|
||||
/// A1: BUZZ_ACP_MODEL must NOT be present in the spawned-child env after policy
|
||||
/// application, even if it was set before (dual-authority defect).
|
||||
#[test]
|
||||
fn test_apply_policy_a1_buzz_acp_model_absent_after_policy() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let managed_root = dir.path().to_path_buf();
|
||||
let pubkey = "abcd1234abcd1234";
|
||||
let mut cmd = std::process::Command::new("true");
|
||||
// Pre-set BUZZ_ACP_MODEL as if it came from descriptor.env.
|
||||
cmd.env("BUZZ_ACP_MODEL", "claude-opus-4");
|
||||
apply_claude_spawn_policy(
|
||||
&mut cmd,
|
||||
pubkey,
|
||||
&managed_root,
|
||||
None,
|
||||
Some("claude-opus-4"),
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let env_map: std::collections::HashMap<_, _> = cmd.get_envs().collect();
|
||||
// BUZZ_ACP_MODEL must be removed (env_remove).
|
||||
// Command::get_envs returns None for removed keys.
|
||||
let buzz_acp_model = env_map.get(std::ffi::OsStr::new("BUZZ_ACP_MODEL"));
|
||||
assert!(
|
||||
buzz_acp_model.is_none() || buzz_acp_model.unwrap().is_none(),
|
||||
"BUZZ_ACP_MODEL must be absent (or explicitly removed) after policy application"
|
||||
);
|
||||
// ANTHROPIC_MODEL must be set to the resolved model.
|
||||
let anthropic_model = env_map.get(std::ffi::OsStr::new("ANTHROPIC_MODEL"));
|
||||
assert!(
|
||||
anthropic_model.is_some(),
|
||||
"ANTHROPIC_MODEL must be present after policy application"
|
||||
);
|
||||
assert_eq!(
|
||||
anthropic_model.unwrap().unwrap_or_default(),
|
||||
"claude-opus-4",
|
||||
"ANTHROPIC_MODEL must equal the effective model"
|
||||
);
|
||||
}
|
||||
|
||||
// ── Spawn-warning persistence tests ──────────────────────────────────────────
|
||||
|
||||
/// Spawn warnings round-trip through write_spawn_warnings / read_spawn_warnings.
|
||||
#[test]
|
||||
fn test_spawn_warnings_roundtrip() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let config_dir = dir.path().to_path_buf();
|
||||
let warnings = vec![
|
||||
"Owner settings.json unreadable — launching with overlay-only settings".to_string(),
|
||||
"Failed to write agent MCP config; inherited servers may be stale: mock error".to_string(),
|
||||
];
|
||||
write_spawn_warnings(&config_dir, &warnings);
|
||||
let read_back = read_spawn_warnings(&config_dir);
|
||||
assert_eq!(read_back, warnings, "read_back must equal written warnings");
|
||||
}
|
||||
|
||||
/// Empty warnings → file removed (no stale warning from a previous spawn).
|
||||
#[test]
|
||||
fn test_spawn_warnings_empty_removes_stale_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let config_dir = dir.path().to_path_buf();
|
||||
// Write a non-empty set first.
|
||||
write_spawn_warnings(&config_dir, &["stale warning".to_string()]);
|
||||
assert!(
|
||||
config_dir.join(SPAWN_WARNINGS_FILE).exists(),
|
||||
"file must exist after non-empty write"
|
||||
);
|
||||
// Write empty → file removed.
|
||||
write_spawn_warnings(&config_dir, &[]);
|
||||
assert!(
|
||||
!config_dir.join(SPAWN_WARNINGS_FILE).exists(),
|
||||
"file must be removed after empty write"
|
||||
);
|
||||
assert_eq!(
|
||||
read_spawn_warnings(&config_dir),
|
||||
Vec::<String>::new(),
|
||||
"read after removal must return empty vec"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -212,4 +212,46 @@ mod tests {
|
||||
"unknown future fields should appear in extra"
|
||||
);
|
||||
}
|
||||
|
||||
/// B8 panel provenance: when reading MCP servers via `mcp_path_override`
|
||||
/// (the agent-root path), every entry must carry
|
||||
/// `source: Some("owner_user_scope")` — not `None` (owner-file path).
|
||||
#[test]
|
||||
fn b8_mcp_path_override_tags_entries_owner_user_scope() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
// Write an MCP config in the agent root with two servers.
|
||||
let mcp_path = dir.path().join(".claude.json");
|
||||
std::fs::write(
|
||||
&mcp_path,
|
||||
br#"{"mcpServers": {"glean": {"command": "glean-mcp"}, "slack": {"command": "slack-mcp"}}}"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// We can't call read_config_file directly without touching HOME-dependent
|
||||
// owner_settings_path(), so exercise the provenance logic inline — this
|
||||
// mirrors the exact code path in read_config_file with is_agent_root=true.
|
||||
let mcp_config: serde_json::Value =
|
||||
serde_json::from_str(std::fs::read_to_string(&mcp_path).unwrap().as_str()).unwrap();
|
||||
let is_agent_root = true; // mcp_path_override is Some
|
||||
let mut extensions = Vec::new();
|
||||
if let Some(servers) = mcp_config.get("mcpServers").and_then(|v| v.as_object()) {
|
||||
for (name, _config) in servers {
|
||||
extensions.push(ExtensionEntry {
|
||||
name: name.clone(),
|
||||
kind: "mcp".to_string(),
|
||||
enabled: true,
|
||||
source: is_agent_root.then(|| "owner_user_scope".to_string()),
|
||||
});
|
||||
}
|
||||
}
|
||||
assert_eq!(extensions.len(), 2, "both servers must be parsed");
|
||||
for entry in &extensions {
|
||||
assert_eq!(
|
||||
entry.source.as_deref(),
|
||||
Some("owner_user_scope"),
|
||||
"entry {:?} must carry owner_user_scope provenance when read from agent root",
|
||||
entry.name
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -200,6 +200,28 @@ pub(crate) fn read_config_surface(
|
||||
.and_then(|_| crate::managed_agents::claude_config::owner_settings_path())
|
||||
.map(|p| crate::managed_agents::claude_config::collect_stripped_env_keys(&p))
|
||||
.unwrap_or_default(),
|
||||
config_warnings: runtime_meta
|
||||
.filter(|m| m.id == "claude")
|
||||
.and_then(|_| {
|
||||
// Read B7/B8 spawn warnings from the per-agent config dir.
|
||||
// agent_mcp_path is <config_dir>/.claude.json — its parent is the config dir.
|
||||
agent_mcp_path
|
||||
.and_then(|p| p.parent())
|
||||
.map(crate::managed_agents::claude_config::read_spawn_warnings)
|
||||
})
|
||||
.unwrap_or_default(),
|
||||
effort_config_id: if runtime_meta.map(|m| m.id == "claude").unwrap_or(false) {
|
||||
// B5: extract the thought_level configId from the session cache so the
|
||||
// UI can call set_config_option without hardcoding the adapter's id.
|
||||
session_cache.and_then(|c| {
|
||||
c.config_options
|
||||
.iter()
|
||||
.find(|opt| opt.category.as_deref() == Some("thought_level"))
|
||||
.map(|opt| opt.config_id.clone())
|
||||
})
|
||||
} else {
|
||||
None
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -181,6 +181,26 @@ pub struct RuntimeConfigSurface {
|
||||
/// Rendered in the config panel as "owner setting overridden by Buzz policy".
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub stripped_owner_env_keys: Vec<String>,
|
||||
/// Visible warnings surfaced in the config panel for B7/B8 failure states.
|
||||
///
|
||||
/// Entries come from two sources:
|
||||
/// - B7: derived at bridge-read time from the owner `~/.claude/settings.json`
|
||||
/// state (if the file exists but is unparsable, the agent is running with
|
||||
/// overlay-only settings).
|
||||
/// - B8: written atomically to `last_spawn_warnings.json` in the per-agent
|
||||
/// config root at each spawn and read back here. The three distinct B8
|
||||
/// states (owner-read failure, invalid-agent fallback, write failure)
|
||||
/// are preserved separately so the panel can render them distinctly.
|
||||
///
|
||||
/// Empty for non-claude runtimes and when there are no warnings.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub config_warnings: Vec<String>,
|
||||
/// B5: the real `configId` for the `thought_level` ACP config option,
|
||||
/// as advertised by the adapter in `session/new`. Present only for claude
|
||||
/// runtimes after the first session is created. The UI uses this to send
|
||||
/// `set_config_option` without hardcoding the configId.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub effort_config_id: Option<String>,
|
||||
}
|
||||
|
||||
/// Raw config values extracted from a runtime's config file.
|
||||
|
||||
@@ -26,6 +26,7 @@ import type {
|
||||
NormalizedField,
|
||||
} from "@/shared/api/types";
|
||||
import { providerDisplayLabel } from "./agentConfigOptions";
|
||||
import { sendSetConfigOption } from "@/shared/api/agentControl";
|
||||
|
||||
type Props = {
|
||||
pubkey: string;
|
||||
@@ -345,6 +346,79 @@ function AdvancedRow({
|
||||
return <div className="flex items-center gap-3 px-4 py-3">{content}</div>;
|
||||
}
|
||||
|
||||
// ── Claude effort picker (B5) ────────────────────────────────────────────────
|
||||
//
|
||||
// Renders a live effort control for claude runtimes when the session-level
|
||||
// `thought_level` configId is available (i.e. at least one session has been
|
||||
// created). Calls `sendSetConfigOption` so the harness forwards the change to
|
||||
// the adapter via `session/set_config_option`; the observer's
|
||||
// `dispatchControlResult` handler persists the canonical value on real ok.
|
||||
|
||||
const CLAUDE_EFFORT_OPTIONS: { label: string; value: string }[] = [
|
||||
{ label: "Low", value: "low" },
|
||||
{ label: "Medium", value: "medium" },
|
||||
{ label: "High", value: "high" },
|
||||
];
|
||||
|
||||
function EffortPicker({
|
||||
pubkey,
|
||||
effortConfigId,
|
||||
currentEffort,
|
||||
}: {
|
||||
pubkey: string;
|
||||
effortConfigId: string;
|
||||
currentEffort: string | null;
|
||||
}) {
|
||||
const [saving, setSaving] = React.useState(false);
|
||||
const [error, setError] = React.useState<string | null>(null);
|
||||
|
||||
const handleChange = async (value: string) => {
|
||||
if (!value) return;
|
||||
setSaving(true);
|
||||
setError(null);
|
||||
try {
|
||||
await sendSetConfigOption(pubkey, effortConfigId, value);
|
||||
} catch (err) {
|
||||
setError(err instanceof Error ? err.message : String(err));
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="mt-3 border-t border-border/50 pt-2 px-4">
|
||||
<p className="text-xs font-medium text-foreground mb-1 flex items-center gap-1.5">
|
||||
<Brain className="h-3.5 w-3.5 text-muted-foreground" />
|
||||
Thinking / Effort
|
||||
</p>
|
||||
<div className="flex items-center gap-2">
|
||||
<select
|
||||
className="h-7 rounded border border-border/50 bg-muted/45 px-2 text-xs text-foreground focus:outline-none disabled:opacity-50"
|
||||
disabled={saving}
|
||||
onChange={(e) => void handleChange(e.target.value)}
|
||||
value={currentEffort ?? ""}
|
||||
>
|
||||
<option value="">Auto (default)</option>
|
||||
{CLAUDE_EFFORT_OPTIONS.map((opt) => (
|
||||
<option key={opt.value} value={opt.value}>
|
||||
{opt.label}
|
||||
</option>
|
||||
))}
|
||||
</select>
|
||||
{saving ? (
|
||||
<span className="text-2xs text-muted-foreground">Setting…</span>
|
||||
) : null}
|
||||
{error ? (
|
||||
<span className="text-2xs text-destructive">{error}</span>
|
||||
) : null}
|
||||
</div>
|
||||
<p className="mt-0.5 text-2xs text-muted-foreground/70">
|
||||
Live — persisted after agent acknowledges
|
||||
</p>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
// ── Main component ────────────────────────────────────────────────────────────
|
||||
|
||||
export function AgentConfigPanel({
|
||||
@@ -381,6 +455,8 @@ export function AgentConfigPanel({
|
||||
sources,
|
||||
isPreSpawn,
|
||||
strippedOwnerEnvKeys,
|
||||
configWarnings,
|
||||
effortConfigId,
|
||||
} = data;
|
||||
const configFilePath = sources.configFilePath;
|
||||
|
||||
@@ -496,6 +572,27 @@ export function AgentConfigPanel({
|
||||
</p>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{configWarnings && configWarnings.length > 0 ? (
|
||||
<div className="mt-3 border-t border-border/50 pt-2 px-4 space-y-1">
|
||||
{configWarnings.map((warning) => (
|
||||
<p
|
||||
key={warning}
|
||||
className="text-xs text-yellow-600 dark:text-yellow-400"
|
||||
>
|
||||
⚠ {warning}
|
||||
</p>
|
||||
))}
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{effortConfigId ? (
|
||||
<EffortPicker
|
||||
pubkey={pubkey}
|
||||
effortConfigId={effortConfigId}
|
||||
currentEffort={normalized.thinkingEffort?.value ?? null}
|
||||
/>
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -673,6 +673,10 @@ export type RuntimeConfigSurface = {
|
||||
sources: ConfigSourceReport;
|
||||
/** Owner `~/.claude/settings.json` env keys stripped by Buzz launch policy (B7.7c). */
|
||||
strippedOwnerEnvKeys?: string[];
|
||||
/** Spawn warnings (B7/B8 failure states): owner unreadable, agent MCP replaced, write failed. */
|
||||
configWarnings?: string[];
|
||||
/** B5: `thought_level` configId from the adapter's `session/new`. Claude only, post-first-session. */
|
||||
effortConfigId?: string;
|
||||
};
|
||||
|
||||
export type UpdateManagedAgentInput = {
|
||||
|
||||
Reference in New Issue
Block a user