From e4a729a3cea75b78c1e387ffcc4766e0da725fbc Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Mon, 3 Aug 2026 19:59:52 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20CI=20round=204=20=E2=80=94=20tsc=20Easin?= =?UTF-8?q?g=20type,=20#[cfg(unix)]=20write-failure=20test,=20B5=20persist?= =?UTF-8?q?ence=20category=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix three CI failures introduced in round 3: 1. AgentDefinitionDialog.tsx + AgentInstanceEditDialog.tsx: inline number[] literal (from ratchet fix commit a64110ece) is not assignable to Easing (TS2322). Revert both to use ADVANCED_FIELDS_MOTION_TRANSITION constant as on main. 2. test_b8_agent_write_failure_does_not_block_spawn uses std::os::unix + Permissions::from_mode, which do not exist on windows-msvc. Add #[cfg(unix)] to gate the test to unix targets. 3. B5 persistence gate had two holes: (a) gated on literal configId "effort" — breaks if adapter renames the thought_level configId; (b) synthetic ok from the fallback branch reachable from the UI when the pool has no thought_level_config_id — Desktop would persist a value nothing forwarded. Fix: harness emits category: "thought_level" ONLY on the real-forward branch (when thought_level_id matches). Observer gates persistence on category === "thought_level" instead of a literal configId. Synthetic acks carry no category so Desktop cannot persist them. Add tests: test_b5_real_forward_ack_includes_thought_level_category (renamed configId still persists), test_b5_synthetic_ok_ack_has_no_ category (pool empty -> no category). Update existing B5 tests to assert category presence/absence. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- crates/buzz-acp/src/lib.rs | 110 ++++++++++++++++-- .../src/managed_agents/claude_config/tests.rs | 1 + .../src/features/agents/observerRelayStore.ts | 10 +- .../agents/ui/AgentDefinitionDialog.tsx | 2 +- .../agents/ui/AgentInstanceEditDialog.tsx | 2 +- desktop/src/shared/api/types.ts | 2 +- 6 files changed, 114 insertions(+), 13 deletions(-) diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 068914b79..d41fdbc53 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1016,7 +1016,8 @@ fn handle_set_config_option_control( .and_then(|c| c.thought_level_config_id.clone()) }); - let status = if thought_level_id.as_deref() == Some(config_id) { + let is_thought_level = thought_level_id.as_deref() == Some(config_id); + let status = if is_thought_level { match pool.set_idle_agent_effort(config_id, value) { IdleEffortResult::Queued => "ok", IdleEffortResult::NoCatalog => "pending_session", @@ -1027,16 +1028,24 @@ fn handle_set_config_option_control( "ok" }; + // B5: include "category": "thought_level" ONLY on the real-forward branch. + // Synthetic acks carry no category so the Desktop observer cannot persist + // them as if they were confirmed thought_level changes. + let mut ack = serde_json::json!({ + "type": "set_config_option", + "configId": config_id, + "status": status, + "value": value, + }); + if is_thought_level { + ack["category"] = serde_json::json!("thought_level"); + } + obs.emit( "control_result", None, &observer::ObserverContext::default(), - serde_json::json!({ - "type": "set_config_option", - "configId": config_id, - "status": status, - "value": value, - }), + ack, ); } @@ -6892,6 +6901,12 @@ mod control_result_tests { "ok", "Queued must yield ok ack" ); + // Real-forward ack must carry category so Desktop knows to persist. + assert_eq!( + ev.payload["category"].as_str().unwrap(), + "thought_level", + "real thought_level forward must include category field" + ); // Session must be invalidated so next turn creates a fresh session. let agent = pool.agents_mut().iter().flatten().next().unwrap(); assert!( @@ -6923,6 +6938,11 @@ mod control_result_tests { "ok", "without thought_level_config_id, harness emits synthetic ok" ); + // Synthetic ok must NOT carry category — Desktop must not persist it. + assert!( + events[0].payload.get("category").is_none() || events[0].payload["category"].is_null(), + "synthetic ok must not carry category field" + ); } /// B5: a non-thought_level configId must still receive a synthetic "ok" @@ -6945,4 +6965,80 @@ mod control_result_tests { "unknown configId must yield synthetic ok for backward compat" ); } + + /// B5 persistence gate — real-forward ack carries `"category": "thought_level"`. + /// The Desktop observer gates persistence on this field; renaming the adapter's + /// configId does not break persistence as long as the category is present. + #[tokio::test] + async fn test_b5_real_forward_ack_includes_thought_level_category() { + 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"); + // Use a renamed configId ("think_level_v2") to prove category-gating + // does not depend on a hardcoded "effort" literal. + let thought_level_id = "think_level_v2".to_string(); + let agent = OwnedAgent { + index: 0, + acp, + state: SessionState::default(), + model_capabilities: Some(AgentModelCapabilities { + config_options_raw: vec![], + available_models_raw: None, + thought_level_config_id: Some(thought_level_id.clone()), + }), + 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": thought_level_id, + "value": "high", + }); + 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"); + // Real-forward ack must carry category so Desktop persists. + assert_eq!( + events[0].payload["category"].as_str().unwrap(), + "thought_level", + "real thought_level forward must include category field" + ); + } + + /// B5 persistence gate — synthetic ack (no thought_level_config_id in pool) + /// must NOT carry `"category"`. The Desktop observer gates persistence on the + /// category field; absent category means no persist. + #[test] + fn test_b5_synthetic_ok_ack_has_no_category() { + 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); + assert_eq!(events[0].payload["status"].as_str().unwrap(), "ok"); + // Synthetic ack must NOT carry category — Desktop must not persist it. + assert!( + events[0].payload.get("category").is_none() || events[0].payload["category"].is_null(), + "synthetic ok ack must not carry category field" + ); + } } diff --git a/desktop/src-tauri/src/managed_agents/claude_config/tests.rs b/desktop/src-tauri/src/managed_agents/claude_config/tests.rs index f8cae7d81..e96278031 100644 --- a/desktop/src-tauri/src/managed_agents/claude_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/claude_config/tests.rs @@ -818,6 +818,7 @@ fn test_b8_agent_mcp_config_path_location() { /// 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. +#[cfg(unix)] #[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). diff --git a/desktop/src/features/agents/observerRelayStore.ts b/desktop/src/features/agents/observerRelayStore.ts index 5b475c5b5..b0dc5eb4f 100644 --- a/desktop/src/features/agents/observerRelayStore.ts +++ b/desktop/src/features/agents/observerRelayStore.ts @@ -502,11 +502,15 @@ function dispatchControlResult(agentPubkey: string, payload: unknown) { if (!isControlResultFrame(payload)) { return; } - // B5: on a positive set_config_option ack for effort, persist the canonical - // value to the agent record so it seeds settings.json on next spawn (B7). + // B5: on a positive set_config_option ack for a confirmed thought_level + // option, persist the canonical value to the agent record so it seeds + // settings.json on next spawn (B7). + // Gate on `category === "thought_level"` (present only on real-forward acks) + // rather than a literal configId — if the adapter renames the configId, + // persistence still works; synthetic acks (no category) never persist. if ( payload.type === "set_config_option" && - payload.configId === "effort" && + payload.category === "thought_level" && payload.status === "ok" ) { void persistAgentEffortLevel(agentPubkey, payload.value || null).catch( diff --git a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx index 8f60c97c5..1916b5706 100644 --- a/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx +++ b/desktop/src/features/agents/ui/AgentDefinitionDialog.tsx @@ -624,7 +624,7 @@ export function AgentDefinitionDialog({ ) : null; const advancedFieldsTransition = shouldReduceMotion ? { duration: 0 } - : { duration: 0.18, ease: [0.23, 1, 0.32, 1] }; + : ADVANCED_FIELDS_MOTION_TRANSITION; React.useEffect(() => { if ( diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 45d92b6b2..d717d773e 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -843,7 +843,7 @@ export function AgentInstanceEditDialog({ const previewAvatarUrl = avatarUrl.trim() || null; const advancedFieldsTransition = shouldReduceMotion ? { duration: 0 } - : { duration: 0.18, ease: [0.23, 1, 0.32, 1] }; + : ADVANCED_FIELDS_MOTION_TRANSITION; return ( diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index 2dd6bf8d3..241694e66 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -467,6 +467,7 @@ export type ControlResultFrame = status: "ok" | string; configId: string; value: string; + category?: "thought_level"; }; export type GitBashPrerequisite = { @@ -675,7 +676,6 @@ export type RuntimeConfigSurface = { 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; };