fix: CI round 4 — tsc Easing type, #[cfg(unix)] write-failure test, B5 persistence category gate

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 <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
2026-08-03 19:59:52 -04:00
co-authored by Will Pfleger
parent 60058c2e66
commit e4a729a3ce
6 changed files with 114 additions and 13 deletions
+103 -7
View File
@@ -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"
);
}
}
@@ -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).
@@ -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(
@@ -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 (
@@ -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 (
<Dialog onOpenChange={handleOpenChange} open={open}>
+1 -1
View File
@@ -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;
};