From 5880d4dc09ae906836577dd8f0ff5313fcfd7565 Mon Sep 17 00:00:00 2001 From: Mariano Blua Date: Sat, 17 Oct 2026 09:17:47 -0300 Subject: [PATCH 1/6] fix(3051): derive identity hash for each agent profile pair --- src-tauri/src/config/settings.rs | 434 +++++++++++++++++++++++++++++++ 1 file changed, 434 insertions(+) diff --git a/src-tauri/src/config/settings.rs b/src-tauri/src/config/settings.rs index ca81e6b35..43cab6350 100644 --- a/src-tauri/src/config/settings.rs +++ b/src-tauri/src/config/settings.rs @@ -4994,6 +4994,78 @@ pub fn refresh_agent_id_hashes(agents: &mut [AgentConfig]) { } } +/// Informational identity of one base/profile pair, using the existing scalar rules. +fn pair_identity(command: &str, envs: &[CodingAgentEnv], profile: &ProfileCellConfig) -> String { + identity_digest( + "cid1:", + &identity_preimage([ + configuration_identity_preimage(command, envs), + profile_identity_preimage(profile), + ]), + ) +} + +fn refresh_pair_id_hashes(agents: &[AgentConfig], profiles: &mut CodingAgentProfilesConfig) { + for (id, cells) in &mut profiles.profiles_by_agent { + // Historical duplicate IDs use the same first-match rule as spawn. + let agent = agents.iter().find(|agent| agent.id == *id); + for cell in cells.values_mut() { + cell.id_hash = agent.map(|agent| pair_identity(&agent.command, &agent.envs, cell)); + } + } +} + +/// Project only identity inputs from the first physical row. Protected groups +/// are best effort here; their existing full decoders still own save errors. +fn physical_pair_base(agents: Option<&Value>, id: &str) -> Option<(String, Vec)> { + let row = agents? + .as_array()? + .iter() + .find(|row| row.get("id").and_then(Value::as_str) == Some(id))? + .as_object()?; + let command = raw_configuration_command(row, "baseCommand") + .ok()? + .unwrap_or_default(); + let envs = match row.get("envs") { + Some(value) => serde_json::from_value(value.clone()).ok()?, + None => Vec::new(), + }; + Some((command.to_owned(), envs)) +} + +fn refresh_physical_pair_id_hashes(agents: Option<&Value>, profiles: &mut Value) { + let Some(rows) = profiles + .get_mut("profilesByAgent") + .and_then(Value::as_object_mut) + else { + return; + }; + for (id, cells) in rows { + let base = physical_pair_base(agents, id); + for raw in cells + .as_object_mut() + .into_iter() + .flat_map(|cells| cells.values_mut()) + { + let hash = base.as_ref().and_then(|(command, envs)| { + // Decode the cell's existing aliases/env contract without rewriting it. + let cell: ProfileCellConfig = serde_json::from_value(raw.clone()).ok()?; + Some(pair_identity(command, envs, &cell)) + }); + if let Some(object) = raw.as_object_mut() { + match hash { + Some(hash) => { + object.insert("id_hash".into(), Value::String(hash)); + } + None => { + object.remove("id_hash"); + } + } + } + } + } +} + /// A configured empty cell has an identity; an absent cell has no identity. /// The base configuration ID and decorative profile letter are not inputs. pub fn profile_identity(profile: &ProfileCellConfig) -> String { @@ -8368,6 +8440,17 @@ fn save_agents_instance_file( } } } + if editable_profiles { + // A present empty physical agents array is authoritative. Never substitute + // the effective (possibly overlay-owned) base for physical metadata. + let agents = target + .get("agents") + .or_else(|| before.get("agents")) + .cloned(); + if let Some(profiles) = target.get_mut("codingAgentProfiles") { + refresh_physical_pair_id_hashes(agents.as_ref(), profiles); + } + } if editable_agents || editable_profiles { let mut after = before.clone(); after.extend(target.clone()); @@ -8637,6 +8720,7 @@ fn parse_settings_with_startup( // disk write or a backup rotation. finalize_agent_order(&mut settings.agents); refresh_agent_id_hashes(&mut settings.agents); + refresh_pair_id_hashes(&settings.agents, &mut settings.coding_agent_profiles); report_overlay_diagnostics(source, &overlay); settings.local_overlay_state = Arc::new(overlay); if settings.local_overlay_state.owns_top_level("agents") { @@ -8746,6 +8830,7 @@ fn default_settings_with_overlay_captured( // the defaults-plus-overlay view; still no write. finalize_agent_order(&mut settings.agents); refresh_agent_id_hashes(&mut settings.agents); + refresh_pair_id_hashes(&settings.agents, &mut settings.coding_agent_profiles); settings.local_overlay_state = Arc::new(overlay); repair_loaded_response_close_settings(&mut settings, source); settings @@ -9646,6 +9731,7 @@ pub fn validate_and_repair_settings(settings: &mut AppSettings) -> Result<(), St repair_coding_agent_profiles_config(&mut settings.coding_agent_profiles, &settings.agents); validate_agent_commands(settings)?; refresh_agent_id_hashes(&mut settings.agents); + refresh_pair_id_hashes(&settings.agents, &mut settings.coding_agent_profiles); validate_screenshot_hotkey(&settings.screenshot_capture_hotkey)?; validate_sidebar_compact_hotkey(&settings.sidebar_compact_hotkey)?; validate_room_number_mask(&settings.room_number_mask)?; @@ -13658,6 +13744,7 @@ fn save_settings_value_locked_with_startup( ) })?; refresh_agent_id_hashes(&mut written.agents); + refresh_pair_id_hashes(&written.agents, &mut written.coding_agent_profiles); written.project_path_state = Arc::new(fresh_state); written.coding_agent_identity_physical = Some(Arc::new(identity_physical)); written.local_overlay_state = settings.local_overlay_state.clone(); @@ -14058,6 +14145,7 @@ fn decode_disk_settings_for_terminal_snapshot_cas( // base `agents` array before writing the file. finalize_agent_order(&mut settings.agents); refresh_agent_id_hashes(&mut settings.agents); + refresh_pair_id_hashes(&settings.agents, &mut settings.coding_agent_profiles); repair_loaded_response_close_settings(&mut settings, &settings_path.display().to_string()); Ok(settings) } @@ -28566,6 +28654,352 @@ mod tests { .collect() } + #[test] + fn issue_3047_pair_table_and_independent_preimages() { + use sha2::{Digest, Sha256}; + // Independent oracle: concatenate the actual fields, then normalize once. + let expected = |parts: &[&str]| { + let mut chars: Vec = parts + .concat() + .chars() + .flat_map(char::to_lowercase) + .filter(|c| !c.is_whitespace()) + .collect(); + chars.sort_unstable(); + format!("cid1:{:x}", Sha256::digest(chars.iter().collect::())) + }; + let mut agents: Vec = serde_json::from_value(serde_json::json!([ + {"id":"a","label":"Alpha","color":"red","baseCommand":"Codex", + "envs":[{"key":"K","value":"V","enabled":false}]}, + {"id":"b","label":"Beta","color":"blue","baseCommand":"Claude"} + ])).unwrap(); + let mut profiles: super::CodingAgentProfilesConfig = serde_json::from_value( + serde_json::json!({"profilesByAgent": { + "a":{"A":{"additionalArguments":"--model x","env":{"P":"Q"}}, + "B":{"additionalArguments":"--model y","enabled":false}}, + "b":{"A":{"additionalArguments":"--effort low"}, + "B":{"additionalArguments":"--effort high"}} + }}), + ).unwrap(); + super::refresh_agent_id_hashes(&mut agents); + let base = agents[0].id_hash.clone(); + super::refresh_pair_id_hashes(&agents, &mut profiles); + let original = profiles.clone(); + assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Codex", "K", "V", "--model x", "P", "Q"]))); + assert_eq!(profiles.profiles_by_agent["a"]["B"].id_hash, + Some(expected(&["Codex", "K", "V", "--model y"]))); + assert_eq!(profiles.profiles_by_agent["b"]["A"].id_hash, + Some(expected(&["Claude", "--effort low"]))); + assert_eq!(profiles.profiles_by_agent["b"]["B"].id_hash, + Some(expected(&["Claude", "--effort high"]))); + assert_ne!(profiles.profiles_by_agent["b"]["A"].id_hash, + profiles.profiles_by_agent["b"]["B"].id_hash); + profiles.profiles_by_agent.get_mut("a").unwrap().get_mut("A").unwrap().command = "--model z".into(); + super::refresh_pair_id_hashes(&agents, &mut profiles); + assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Codex", "K", "V", "--model z", "P", "Q"]))); + assert_eq!(profiles.profiles_by_agent["a"]["B"], original.profiles_by_agent["a"]["B"]); + assert_eq!(profiles.profiles_by_agent["b"], original.profiles_by_agent["b"]); + assert_eq!(agents[0].id_hash, base); + agents[0].command = "Other".into(); + agents[0].envs[0].value = "W".into(); + super::refresh_pair_id_hashes(&agents, &mut profiles); + assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Other", "K", "W", "--model z", "P", "Q"]))); + assert_eq!(profiles.profiles_by_agent["a"]["B"].id_hash, + Some(expected(&["Other", "K", "W", "--model y"]))); + assert_eq!(profiles.profiles_by_agent["b"], original.profiles_by_agent["b"]); + profiles.profiles_by_agent.get_mut("a").unwrap().get_mut("A").unwrap() + .env.insert("P".into(), "R".into()); + super::refresh_pair_id_hashes(&agents, &mut profiles); + assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Other", "K", "W", "--model z", "P", "R"]))); + assert_eq!(agents.iter().map(|a| a.id.as_str()).collect::>(), ["a", "b"]); + } + + #[test] + fn issue_3047_pair_equivalence_exclusions_empty_orphan_and_first_match() { + let mut agents: Vec = serde_json::from_value(serde_json::json!([ + {"id":"a","label":"First","color":"red","command":"İ A\u{2003}b"}, + {"id":"a","label":"Second","color":"blue","command":"wrong"} + ])).unwrap(); + let mut profiles: super::CodingAgentProfilesConfig = serde_json::from_value( + serde_json::json!({"profilesByAgent": { + "a":{"A":{"command":" XY"},"B":{}}, + "orphan":{"A":{"id_hash":"forged"}} + }}), + ).unwrap(); + super::refresh_pair_id_hashes(&agents, &mut profiles); + let original = profiles.profiles_by_agent["a"]["A"].clone(); + assert_eq!(original.id_hash, Some(super::configuration_identity("i\u{307}abxy", &[]))); + assert_eq!(profiles.profiles_by_agent["a"]["B"].id_hash, + Some(super::configuration_identity(&agents[0].command, &[]))); + assert!(!profiles.profiles_by_agent["a"].contains_key("C")); + assert_eq!(profiles.profiles_by_agent["orphan"]["A"].id_hash, None); + agents[0].label = "Renamed".into(); + agents[0].command = "b aİ".into(); + profiles.default_profile_by_agent.insert("a".into(), "B".into()); + profiles.profile_labels_by_agent.insert("a".into(), + std::collections::BTreeMap::from([("A".into(), "Renamed profile".into())])); + let cell = profiles.profiles_by_agent.get_mut("a").unwrap().get_mut("A").unwrap(); + cell.command = "y\u{a0}x".into(); + cell.enabled = false; + cell.notes = "ignored".into(); + cell.preflight = Some("ignored too".into()); + super::refresh_pair_id_hashes(&agents, &mut profiles); + assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, original.id_hash); + let cell = &profiles.profiles_by_agent["a"]["A"]; + let pid = super::profile_identity(cell); + let mut forged = cell.clone(); + forged.id_hash = Some("client".into()); + assert_eq!(super::profile_identity(&forged), pid); + assert_ne!(super::pair_identity("ab", &[], cell), super::pair_identity("aab", &[], cell)); + } + + #[test] + fn issue_3047_pair_load_save_fixed_point_and_untrusted_metadata() { + for sidecar in [false, true] { + for supplied in [serde_json::json!(null), serde_json::json!(false), serde_json::json!(42), + serde_json::json!(1.5), serde_json::json!([]), serde_json::json!({}), + serde_json::json!(""), serde_json::json!("forged")] { + let (temp, path) = hash_activation_seed(sidecar); + let physical = if sidecar { b3_sidecar_path(temp.path()) } else { path.clone() }; + let mut raw = serde_json::Value::Object(b3_object(&physical)); + raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ + "alpha":{"A":{"additionalArguments":"--model x","id_hash":supplied, + "sentinel":{"keep":true}}} + }); + b3_write(&physical, &raw); + let before = hash_activation_files(temp.path()); + for (loader, loaded) in b3_all_loaders(&path) { + let cell = &loaded.coding_agent_profiles.profiles_by_agent["alpha"]["A"]; + assert_eq!(cell.id_hash, Some(super::configuration_identity("claude--modelx", &[])), "{loader}"); + assert_eq!(hash_activation_files(temp.path()), before, "{loader}"); + } + let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + let written = super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + let groups = b3_object(&b3_sidecar_path(temp.path())); + assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + written.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash.clone().unwrap()); + let stable = hash_activation_files(temp.path()); + let reloaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + super::save_settings_to_path_preserving_project_paths(&reloaded, &path).unwrap(); + assert_eq!(hash_activation_files(temp.path()), stable); + let mut stale = reloaded.clone(); + stale.coding_agent_profiles.profiles_by_agent.get_mut("alpha").unwrap().get_mut("A").unwrap().id_hash = Some("stale".into()); + let cas = super::compare_and_set_terminal_snapshots_enabled_at_path( + &stale, &path, stale.terminal_snapshots_enabled, stale.terminal_snapshots_enabled, + ).unwrap(); + assert_eq!(cas.coding_agent_profiles, reloaded.coding_agent_profiles); + } + } + } + + #[test] + fn issue_3047_pair_protected_base_projection_and_early_rejections() { + for sidecar in [false, true] { + for invalid in [false, true] { + let (temp, path) = hash_activation_seed(sidecar); + let physical = if sidecar { b3_sidecar_path(temp.path()) } else { path.clone() }; + let mut raw = serde_json::Value::Object(b3_object(&physical)); + raw["agents"][0]["sentinel"] = serde_json::json!({"keep":19}); + raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ + "alpha":{"A":{"additionalArguments":"--model x","sentinel":"cell"}} + }); + b3_write(&physical, &raw); + let mut overlay_agent = b3_agent("alpha", "Overlay"); + overlay_agent["command"] = serde_json::json!("codex --overlay"); + b3_write(&b3_overlay_path(temp.path()), &serde_json::json!({"agents":[overlay_agent]})); + let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + assert!(loaded.local_overlay_state.owns_top_level("agents")); + if invalid { + raw["agents"][0]["label"] = serde_json::json!(17); + b3_write(&physical, &raw); + } + if !sidecar { + std::fs::remove_file(b3_sidecar_path(temp.path())).unwrap(); + } + let before = std::fs::read(&physical).unwrap(); + let mut out = serde_json::to_value(&loaded).unwrap(); + out["codingAgentProfiles"] = raw["codingAgentProfiles"].clone(); + let result = super::save_agents_instance_file( + &loaded, &path, &super::ProjectWriteMode::Preserve, None, + out.as_object().unwrap(), super::SettingsSaveStage::PreserveDiskGate, None, + ); + if invalid { + let error = result.unwrap_err(); + assert_eq!(error.stage, super::SettingsSaveStage::PreserveDiskGate); + if sidecar { + assert_eq!(error.target_path.as_ref(), &b3_sidecar_path(temp.path())); + assert!(matches!(&error.legacy_outward, + super::SettingsSaveLegacyOutward::DiskRead(text) + if text.contains("Refusing to overwrite") && text.contains("undecodable agents array"))); + } else { + assert_eq!(error.target_path.as_ref(), &path); + assert!(matches!(error.cause, super::SettingsSaveCause::Semantic( + super::SettingsSaveReason::DiskSettingsValidationRejected))); + assert!(matches!(&error.legacy_outward, + super::SettingsSaveLegacyOutward::DiskValidation(text) + if text.contains("non-project fields are invalid"))); + assert!(!b3_sidecar_path(temp.path()).exists()); + } + assert_eq!(std::fs::read(&physical).unwrap(), before); + } else { + let groups = result.unwrap(); + assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + super::configuration_identity("claude--modelx", &[])); + assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["sentinel"], "cell"); + if sidecar { + assert_eq!(groups["agents"], raw["agents"]); + } else { + assert_eq!(b3_object(&path)["agents"], raw["agents"]); + assert!(!groups.contains_key("agents")); + } + } + } + } + } + + #[test] + fn issue_3047_pair_unresolvable_first_row_and_empty_physical_base() { + for first in [serde_json::json!({"id":"a","baseCommand":17}), + serde_json::json!({"id":"a","baseCommand":"codex","envs":"bad"})] { + let agents = serde_json::json!([first, {"id":"a","baseCommand":"valid"}]); + let before = agents.clone(); + let mut profiles = serde_json::json!({"profilesByAgent":{ + "a":{"A":{"command":"--model x","id_hash":"stale","unknown":1}} + }}); + super::refresh_physical_pair_id_hashes(Some(&agents), &mut profiles); + assert!(profiles["profilesByAgent"]["a"]["A"].get("id_hash").is_none()); + assert_eq!(profiles["profilesByAgent"]["a"]["A"]["unknown"], 1); + assert_eq!(agents, before); + let (temp, path) = hash_activation_seed(true); + let mut groups = serde_json::Value::Object(b3_object(&b3_sidecar_path(temp.path()))); + groups["agents"] = agents; + b3_write(&b3_sidecar_path(temp.path()), &groups); + let bytes = std::fs::read(b3_sidecar_path(temp.path())).unwrap(); + assert!(super::save_agents_instance_file( + &super::AppSettings::default(), &path, &super::ProjectWriteMode::Preserve, + None, &serde_json::Map::new(), super::SettingsSaveStage::PreserveDiskGate, None, + ).is_err()); + assert_eq!(std::fs::read(b3_sidecar_path(temp.path())).unwrap(), bytes); + } + for agents in [None, Some(serde_json::json!([]))] { + let mut profiles = serde_json::json!({"profilesByAgent":{ + "a":{"A":{"id_hash":"stale"}} + }}); + super::refresh_physical_pair_id_hashes(agents.as_ref(), &mut profiles); + assert!(profiles["profilesByAgent"]["a"]["A"].get("id_hash").is_none()); + } + let agents = serde_json::json!([{"id":"a","command":"codex","label":17, + "envs":[{"key":"K","value":"V","enabled":false}]}]); + let mut profiles = serde_json::json!({"profilesByAgent":{ + "a":{"A":{"command":"--model x","env":{"P":"Q"},"unknown":1}} + }}); + let before = profiles.clone(); + super::refresh_physical_pair_id_hashes(Some(&agents), &mut profiles); + assert_eq!(profiles["profilesByAgent"]["a"]["A"]["id_hash"], + super::configuration_identity("codexKV--modelxPQ", &[])); + profiles["profilesByAgent"]["a"]["A"].as_object_mut().unwrap().remove("id_hash"); + assert_eq!(profiles, before); + } + + #[test] + fn issue_3047_pair_ownership_matrix_and_base_or_profile_only_edits() { + for sidecar in [false, true] { + for protected in ["agents", "codingAgentProfiles", "neither"] { + let (temp, path) = hash_activation_seed(sidecar); + let physical = if sidecar { b3_sidecar_path(temp.path()) } else { path.clone() }; + let mut raw = serde_json::Value::Object(b3_object(&physical)); + raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ + "alpha":{"A":{"command":"--model x","id_hash":"physical"}} + }); + b3_write(&physical, &raw); + if protected != "neither" { + let mut overlay = serde_json::Map::new(); + let mut value = raw[protected].clone(); + if protected == "agents" { + value[0].as_object_mut().unwrap().remove("baseCommand"); + value[0]["command"] = serde_json::json!("codex"); + } else { + value["profilesByAgent"]["alpha"]["A"]["command"] = serde_json::json!("--effort high"); + } + overlay.insert(protected.into(), value); + b3_write(&b3_overlay_path(temp.path()), &serde_json::Value::Object(overlay)); + } + let mut loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + if protected == "agents" { + loaded.coding_agent_profiles.profiles_by_agent.get_mut("alpha").unwrap().get_mut("A").unwrap().command = "--model y".into(); + } else { + loaded.agents[0].command = "other-base".into(); + } + let written = super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + let cell = &written.coding_agent_profiles.profiles_by_agent["alpha"]["A"]; + assert_eq!(cell.id_hash, Some(super::pair_identity(&written.agents[0].command, &written.agents[0].envs, cell))); + let groups = b3_object(&b3_sidecar_path(temp.path())); + if protected == "codingAgentProfiles" { + let retained = groups.get(protected).cloned() + .unwrap_or_else(|| b3_object(&path)[protected].clone()); + assert_eq!(retained, raw[protected]); + } else { + let expected_base = if protected == "agents" { "claude" } else { "other-base" }; + let expected_args = if protected == "agents" { "--model y" } else { "--model x" }; + assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + super::configuration_identity(&format!("{expected_base}{expected_args}"), &[])); + } + let reloaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + assert_eq!(reloaded.coding_agent_profiles, written.coding_agent_profiles); + } + } + } + + #[test] + fn issue_3047_pair_effective_base_without_physical_base_and_rejected_overlay() { + let (temp, path) = hash_activation_seed(true); + let sidecar = b3_sidecar_path(temp.path()); + let mut groups = serde_json::Value::Object(b3_object(&sidecar)); + groups.as_object_mut().unwrap().remove("agents"); + groups["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ + "alpha":{"A":{"additionalArguments":"--model x","id_hash":"stale"}} + }); + b3_write(&sidecar, &groups); + b3_write(&b3_overlay_path(temp.path()), &serde_json::json!({ + "agents":[b3_agent("alpha", "Effective only")] + })); + let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + let written = super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + assert_eq!(written.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash, + Some(super::configuration_identity("claude--modelx", &[]))); + let raw = b3_object(&sidecar); + assert!(!raw.contains_key("agents")); + assert!(raw["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"].get("id_hash").is_none()); + + // Rejected overlays still derive the final accepted physical view. + let (temp, path) = hash_activation_seed(true); + let sidecar = b3_sidecar_path(temp.path()); + let mut groups = serde_json::Value::Object(b3_object(&sidecar)); + groups["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ + "alpha":{"A":{"additionalArguments":"--model x","id_hash":"stale"}} + }); + b3_write(&sidecar, &groups); + b3_write(&b3_overlay_path(temp.path()), &serde_json::json!({"defaultShellArgs":"bad"})); + let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + assert!(loaded.local_overlay_state.rejection().is_some()); + assert_eq!(loaded.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash, + Some(super::configuration_identity("claude--modelx", &[]))); + let previous = std::fs::read(&path).unwrap(); + super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| { + *slot.borrow_mut() = Some((path.clone(), super::SettingsSaveStage::AtomicReplace)) + }); + let result = super::save_settings_to_path_preserving_project_paths(&loaded, &path); + super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| *slot.borrow_mut() = None); + assert!(result.is_err()); + assert_eq!(std::fs::read(&path).unwrap(), previous); + assert_eq!(b3_object(&sidecar)["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + super::configuration_identity("claude--modelx", &[])); + } + fn hash_activation_assert(settings: &super::AppSettings) { for agent in &settings.agents { assert_eq!( From c5b5ee1dd6b788292e56d30675f3bdf1685ff336 Mon Sep 17 00:00:00 2001 From: Mariano Blua Date: Sat, 17 Oct 2026 11:30:16 -0300 Subject: [PATCH 2/6] fix(3051): preserve protected profile raw state and final pair hashes --- src-tauri/src/commands/config.rs | 151 ++++++- src-tauri/src/config/settings.rs | 686 ++++++++++++++++++++++++++----- 2 files changed, 719 insertions(+), 118 deletions(-) diff --git a/src-tauri/src/commands/config.rs b/src-tauri/src/commands/config.rs index 1f88e20df..a446fd230 100644 --- a/src-tauri/src/commands/config.rs +++ b/src-tauri/src/commands/config.rs @@ -1039,6 +1039,16 @@ fn settings_draft_update_events( } } + let mut before_profiles = before.coding_agent_profiles.clone(); + let mut after_profiles = after.coding_agent_profiles.clone(); + for profiles in [&mut before_profiles, &mut after_profiles] { + for cells in profiles.profiles_by_agent.values_mut() { + for cell in cells.values_mut() { + cell.id_hash = None; + } + } + } + SettingsDraftUpdateEvents { // #592/#597 - the drift content-hash fingerprints the EFFECTIVE command // (agent base command + cell command, via compose_effective_command), so a @@ -1048,7 +1058,7 @@ fn settings_draft_update_events( // here the SettingsModal save path emits NO refresh event and the outdated // badge never updates. coding_agent_profiles_updated drives // refreshProfileOutdated in the sidebar, so route base-command edits through it. - profiles_changed: before.coding_agent_profiles != after.coding_agent_profiles + profiles_changed: before_profiles != after_profiles || agent_commands_by_id(before) != agent_commands_by_id(after), env_agent_ids, } @@ -4980,9 +4990,38 @@ mod tests { serde_json::to_value(&written.agents).unwrap() ); assert_ne!(written.agents[0].id_hash, original.agents[0].id_hash); + let mut expected_profiles = original.coding_agent_profiles.profiles_by_agent.clone(); + for (letter, cell) in expected_profiles.get_mut("agent-0").unwrap() { + // Independent preimage from the final command, restored live env and + // the original payload. Disabled env rows still participate. + let mut preimage = written.agents[0].command.clone(); + for env in &original.agents[0].envs { + preimage.push_str(&env.key); + preimage.push_str(&env.value); + } + preimage.push_str(&cell.command); + for (key, value) in &cell.env { + preimage.push_str(key); + preimage.push_str(value); + } + let mut chars: Vec<_> = preimage + .chars() + .flat_map(char::to_lowercase) + .filter(|c| !c.is_whitespace()) + .collect(); + chars.sort_unstable(); + cell.id_hash = Some(format!( + "cid1:{:x}", + Sha256::digest(chars.iter().collect::()) + )); + assert_eq!( + written.coding_agent_profiles.profiles_by_agent["agent-0"][letter].id_hash, + cell.id_hash + ); + } assert_eq!( written.coding_agent_profiles.profiles_by_agent, - original.coding_agent_profiles.profiles_by_agent + expected_profiles ); let before = serde_json::to_value(&original).unwrap(); let after = serde_json::to_value(&written).unwrap(); @@ -6970,6 +7009,101 @@ mod tests { assert!(events.env_agent_ids.is_empty(), "no env changed"); } + #[test] + fn id_hash_activation_events_ignore_only_cell_metadata() { + let mut before = settings_with_single_agent(); + crate::config::settings::validate_and_repair_settings(&mut before).unwrap(); + let mut metadata = before.clone(); + metadata + .coding_agent_profiles + .profiles_by_agent + .get_mut("agent-0") + .unwrap() + .get_mut("A") + .unwrap() + .id_hash = Some("client-metadata".into()); + let events = super::settings_draft_update_events(&before, &metadata); + assert!(!events.profiles_changed); + assert!(events.env_agent_ids.is_empty()); + assert_ne!(before.coding_agent_profiles, metadata.coding_agent_profiles); + for change in [ + "command", + "env", + "enabled", + "notes", + "preflight", + "timeout", + "cell", + "agent", + "default", + "label", + "slots", + "schema", + "identity", + "home", + ] { + let mut after = metadata.clone(); + let profiles = &mut after.coding_agent_profiles; + let cell = profiles + .profiles_by_agent + .get_mut("agent-0") + .unwrap() + .get_mut("A") + .unwrap(); + match change { + "command" => cell.command.push_str("--real"), + "env" => { + cell.env.insert("K".into(), "V".into()); + } + "enabled" => cell.enabled = !cell.enabled, + "notes" => cell.notes.push_str("real"), + "preflight" => cell.preflight = Some("real".into()), + "timeout" => cell.preflight_timeout_seconds = Some(99), + "cell" => { + profiles + .profiles_by_agent + .get_mut("agent-0") + .unwrap() + .remove("A"); + } + "agent" => { + profiles + .profiles_by_agent + .insert("new".into(), BTreeMap::new()); + } + "default" => { + profiles + .default_profile_by_agent + .insert("agent-0".into(), "B".into()); + } + "label" => { + profiles.profile_labels_by_agent.insert( + "agent-0".into(), + BTreeMap::from([("A".into(), "real".into())]), + ); + } + "slots" => { + profiles.profile_slots.clear(); + } + "schema" => profiles.schema_version += 1, + "identity" => { + profiles + .identity_by_agent + .insert("agent-0".into(), "real".into()); + } + "home" => { + profiles + .codex_home_key_by_agent + .insert("agent-0".into(), "real".into()); + } + _ => unreachable!(), + } + let events = super::settings_draft_update_events(&before, &after); + assert!(events.profiles_changed, "{change}"); + assert!(events.env_agent_ids.is_empty(), "{change}"); + } + } + #[test] fn settings_draft_events_quiet_for_non_drift_field_edit() { // A non-drift field (label) changing must NOT emit a profiles refresh, so the @@ -6987,8 +7121,9 @@ mod tests { fn settings_draft_events_flag_agent_env_edit() { // Regression guard: the pre-existing env path still flags the agent id (the // base env is also a drift-hash input, surfaced via the env event). - let before = settings_with_single_agent(); - let mut after = settings_with_single_agent(); + let mut before = settings_with_single_agent(); + crate::config::settings::validate_and_repair_settings(&mut before).unwrap(); + let mut after = before.clone(); after.agents[0].envs = vec![CodingAgentEnv { key: "FOO".to_string(), value: "bar".to_string(), @@ -6996,6 +7131,14 @@ mod tests { enabled: true, }]; + after + .coding_agent_profiles + .profiles_by_agent + .get_mut("agent-0") + .unwrap() + .get_mut("A") + .unwrap() + .id_hash = Some("derived-after-env-edit".into()); let events = super::settings_draft_update_events(&before, &after); assert_eq!(events.env_agent_ids, vec!["agent-0".to_string()]); assert!( diff --git a/src-tauri/src/config/settings.rs b/src-tauri/src/config/settings.rs index 43cab6350..5e720e3db 100644 --- a/src-tauri/src/config/settings.rs +++ b/src-tauri/src/config/settings.rs @@ -10180,6 +10180,7 @@ fn load_settings_from_path_with_blocking_menus_publish( settings.coding_agent_identity_physical = capture_project_settings(path, false) .ok() .and_then(|(captured, _, _)| captured.coding_agent_identity_physical); + refresh_pair_id_hashes(&settings.agents, &mut settings.coding_agent_profiles); settings } @@ -10503,6 +10504,8 @@ fn load_settings_for_cli_from_path(path: &Path) -> AppSettings { adopt_orphaned_profiles(&mut settings); repair_coding_agent_profiles_config(&mut settings.coding_agent_profiles, &settings.agents); + refresh_pair_id_hashes(&settings.agents, &mut settings.coding_agent_profiles); + // NO root_token auto-gen, NO save_settings call. settings } @@ -10566,6 +10569,7 @@ fn load_settings_for_cli_strict_from_path(path: Option<&Path>) -> Result { + out.insert(key.to_owned(), raw.clone()); + } + None => { + out.remove(key); + } + } + // Only legacy MAIN identities retain the existing #2450 writer output. + // All other fields of a protected profiles group remain raw disk values. + if key == "codingAgentProfiles" { + match &identity_rows { + Some(rows) => { + let profiles = out + .entry(key.to_owned()) + .or_insert_with(|| Value::Object(Map::new())); + profiles + .as_object_mut() + .expect("validated profiles object") + .insert("identityByAgent".to_owned(), rows.clone()); + } + None => { + if let Some(profiles) = out.get_mut(key).and_then(Value::as_object_mut) { + profiles.remove("identityByAgent"); + } + } + } + } + } let value = Value::Object(out); let written_bytes = write_value_atomic(&value, path)?; if let Some(previous) = previous_contents { @@ -23823,10 +23866,11 @@ mod tests { let object = disk_object(&crate::config::settings::agents_instance_path(&path)); let profiles = &object["codingAgentProfiles"]; - assert_eq!( - profiles["profilesByAgent"]["agent_2_bbbbbb"]["A"], - cell_value("--special") - ); + assert_eq!(profiles["profilesByAgent"]["agent_2_bbbbbb"]["A"], { + let mut expected = cell_value("--special"); + expected["id_hash"] = json!(configuration_identity("claude--special", &[])); + expected + }); assert_eq!( profiles["profileLabelsByAgent"]["agent_2_bbbbbb"], json!({"A": "Mine"}) @@ -23865,7 +23909,13 @@ mod tests { validate_and_repair_settings(&mut settings).unwrap(); let cells = &settings.coding_agent_profiles.profiles_by_agent["n"]; assert_eq!(cells["B"].command, "--b"); - assert_eq!(cells.get("A"), Some(&empty_profile_cell())); + let mut expected_a = empty_profile_cell(); + expected_a.id_hash = Some(configuration_identity("claude", &[])); + assert_eq!(cells.get("A"), Some(&expected_a)); + assert_eq!( + cells["B"].id_hash, + Some(configuration_identity("claude--b", &[])) + ); } // 7 @@ -24118,10 +24168,11 @@ mod tests { assert_eq!(a_command(&settings, "n").as_deref(), Some("--special")); let profiles = &disk_object(&crate::config::settings::agents_instance_path(&path)) ["codingAgentProfiles"]; - assert_eq!( - profiles["profilesByAgent"]["n"]["A"], - cell_value("--special") - ); + assert_eq!(profiles["profilesByAgent"]["n"]["A"], { + let mut expected = cell_value("--special"); + expected["id_hash"] = json!(configuration_identity("claude--special", &[])); + expected + }); assert_eq!( profiles["profileLabelsByAgent"]["n"], json!({"A": "Special"}) @@ -24157,8 +24208,15 @@ mod tests { validate_and_repair_settings(&mut settings).unwrap(); let cells = &settings.coding_agent_profiles.profiles_by_agent; assert_eq!(cells["n"]["B"].command, "--b"); - assert_eq!(cells["n"]["A"], empty_profile_cell()); - assert_eq!(cells["k"]["A"], empty_profile_cell()); + let mut expected_a = empty_profile_cell(); + expected_a.id_hash = Some(configuration_identity("claude", &[])); + assert_eq!(cells["n"]["A"], expected_a); + expected_a.id_hash = Some(configuration_identity("codex", &[])); + assert_eq!(cells["k"]["A"], expected_a); + assert_eq!( + cells["n"]["B"].id_hash, + Some(configuration_identity("claude--b", &[])) + ); } } @@ -28666,56 +28724,102 @@ mod tests { .filter(|c| !c.is_whitespace()) .collect(); chars.sort_unstable(); - format!("cid1:{:x}", Sha256::digest(chars.iter().collect::())) + format!( + "cid1:{:x}", + Sha256::digest(chars.iter().collect::()) + ) }; let mut agents: Vec = serde_json::from_value(serde_json::json!([ {"id":"a","label":"Alpha","color":"red","baseCommand":"Codex", "envs":[{"key":"K","value":"V","enabled":false}]}, {"id":"b","label":"Beta","color":"blue","baseCommand":"Claude"} - ])).unwrap(); - let mut profiles: super::CodingAgentProfilesConfig = serde_json::from_value( - serde_json::json!({"profilesByAgent": { + ])) + .unwrap(); + let mut profiles: super::CodingAgentProfilesConfig = + serde_json::from_value(serde_json::json!({"profilesByAgent": { "a":{"A":{"additionalArguments":"--model x","env":{"P":"Q"}}, "B":{"additionalArguments":"--model y","enabled":false}}, "b":{"A":{"additionalArguments":"--effort low"}, "B":{"additionalArguments":"--effort high"}} - }}), - ).unwrap(); + }})) + .unwrap(); super::refresh_agent_id_hashes(&mut agents); let base = agents[0].id_hash.clone(); super::refresh_pair_id_hashes(&agents, &mut profiles); let original = profiles.clone(); - assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, - Some(expected(&["Codex", "K", "V", "--model x", "P", "Q"]))); - assert_eq!(profiles.profiles_by_agent["a"]["B"].id_hash, - Some(expected(&["Codex", "K", "V", "--model y"]))); - assert_eq!(profiles.profiles_by_agent["b"]["A"].id_hash, - Some(expected(&["Claude", "--effort low"]))); - assert_eq!(profiles.profiles_by_agent["b"]["B"].id_hash, - Some(expected(&["Claude", "--effort high"]))); - assert_ne!(profiles.profiles_by_agent["b"]["A"].id_hash, - profiles.profiles_by_agent["b"]["B"].id_hash); - profiles.profiles_by_agent.get_mut("a").unwrap().get_mut("A").unwrap().command = "--model z".into(); + assert_eq!( + profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Codex", "K", "V", "--model x", "P", "Q"])) + ); + assert_eq!( + profiles.profiles_by_agent["a"]["B"].id_hash, + Some(expected(&["Codex", "K", "V", "--model y"])) + ); + assert_eq!( + profiles.profiles_by_agent["b"]["A"].id_hash, + Some(expected(&["Claude", "--effort low"])) + ); + assert_eq!( + profiles.profiles_by_agent["b"]["B"].id_hash, + Some(expected(&["Claude", "--effort high"])) + ); + assert_ne!( + profiles.profiles_by_agent["b"]["A"].id_hash, + profiles.profiles_by_agent["b"]["B"].id_hash + ); + profiles + .profiles_by_agent + .get_mut("a") + .unwrap() + .get_mut("A") + .unwrap() + .command = "--model z".into(); super::refresh_pair_id_hashes(&agents, &mut profiles); - assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, - Some(expected(&["Codex", "K", "V", "--model z", "P", "Q"]))); - assert_eq!(profiles.profiles_by_agent["a"]["B"], original.profiles_by_agent["a"]["B"]); - assert_eq!(profiles.profiles_by_agent["b"], original.profiles_by_agent["b"]); + assert_eq!( + profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Codex", "K", "V", "--model z", "P", "Q"])) + ); + assert_eq!( + profiles.profiles_by_agent["a"]["B"], + original.profiles_by_agent["a"]["B"] + ); + assert_eq!( + profiles.profiles_by_agent["b"], + original.profiles_by_agent["b"] + ); assert_eq!(agents[0].id_hash, base); agents[0].command = "Other".into(); agents[0].envs[0].value = "W".into(); super::refresh_pair_id_hashes(&agents, &mut profiles); - assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, - Some(expected(&["Other", "K", "W", "--model z", "P", "Q"]))); - assert_eq!(profiles.profiles_by_agent["a"]["B"].id_hash, - Some(expected(&["Other", "K", "W", "--model y"]))); - assert_eq!(profiles.profiles_by_agent["b"], original.profiles_by_agent["b"]); - profiles.profiles_by_agent.get_mut("a").unwrap().get_mut("A").unwrap() - .env.insert("P".into(), "R".into()); + assert_eq!( + profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Other", "K", "W", "--model z", "P", "Q"])) + ); + assert_eq!( + profiles.profiles_by_agent["a"]["B"].id_hash, + Some(expected(&["Other", "K", "W", "--model y"])) + ); + assert_eq!( + profiles.profiles_by_agent["b"], + original.profiles_by_agent["b"] + ); + profiles + .profiles_by_agent + .get_mut("a") + .unwrap() + .get_mut("A") + .unwrap() + .env + .insert("P".into(), "R".into()); super::refresh_pair_id_hashes(&agents, &mut profiles); - assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, - Some(expected(&["Other", "K", "W", "--model z", "P", "R"]))); - assert_eq!(agents.iter().map(|a| a.id.as_str()).collect::>(), ["a", "b"]); + assert_eq!( + profiles.profiles_by_agent["a"]["A"].id_hash, + Some(expected(&["Other", "K", "W", "--model z", "P", "R"])) + ); + assert_eq!( + agents.iter().map(|a| a.id.as_str()).collect::>(), + ["a", "b"] + ); } #[test] @@ -28723,48 +28827,80 @@ mod tests { let mut agents: Vec = serde_json::from_value(serde_json::json!([ {"id":"a","label":"First","color":"red","command":"İ A\u{2003}b"}, {"id":"a","label":"Second","color":"blue","command":"wrong"} - ])).unwrap(); - let mut profiles: super::CodingAgentProfilesConfig = serde_json::from_value( - serde_json::json!({"profilesByAgent": { + ])) + .unwrap(); + let mut profiles: super::CodingAgentProfilesConfig = + serde_json::from_value(serde_json::json!({"profilesByAgent": { "a":{"A":{"command":" XY"},"B":{}}, "orphan":{"A":{"id_hash":"forged"}} - }}), - ).unwrap(); + }})) + .unwrap(); super::refresh_pair_id_hashes(&agents, &mut profiles); let original = profiles.profiles_by_agent["a"]["A"].clone(); - assert_eq!(original.id_hash, Some(super::configuration_identity("i\u{307}abxy", &[]))); - assert_eq!(profiles.profiles_by_agent["a"]["B"].id_hash, - Some(super::configuration_identity(&agents[0].command, &[]))); + assert_eq!( + original.id_hash, + Some(super::configuration_identity("i\u{307}abxy", &[])) + ); + assert_eq!( + profiles.profiles_by_agent["a"]["B"].id_hash, + Some(super::configuration_identity(&agents[0].command, &[])) + ); assert!(!profiles.profiles_by_agent["a"].contains_key("C")); assert_eq!(profiles.profiles_by_agent["orphan"]["A"].id_hash, None); agents[0].label = "Renamed".into(); agents[0].command = "b aİ".into(); - profiles.default_profile_by_agent.insert("a".into(), "B".into()); - profiles.profile_labels_by_agent.insert("a".into(), - std::collections::BTreeMap::from([("A".into(), "Renamed profile".into())])); - let cell = profiles.profiles_by_agent.get_mut("a").unwrap().get_mut("A").unwrap(); + profiles + .default_profile_by_agent + .insert("a".into(), "B".into()); + profiles.profile_labels_by_agent.insert( + "a".into(), + std::collections::BTreeMap::from([("A".into(), "Renamed profile".into())]), + ); + let cell = profiles + .profiles_by_agent + .get_mut("a") + .unwrap() + .get_mut("A") + .unwrap(); cell.command = "y\u{a0}x".into(); cell.enabled = false; cell.notes = "ignored".into(); cell.preflight = Some("ignored too".into()); super::refresh_pair_id_hashes(&agents, &mut profiles); - assert_eq!(profiles.profiles_by_agent["a"]["A"].id_hash, original.id_hash); + assert_eq!( + profiles.profiles_by_agent["a"]["A"].id_hash, + original.id_hash + ); let cell = &profiles.profiles_by_agent["a"]["A"]; let pid = super::profile_identity(cell); let mut forged = cell.clone(); forged.id_hash = Some("client".into()); assert_eq!(super::profile_identity(&forged), pid); - assert_ne!(super::pair_identity("ab", &[], cell), super::pair_identity("aab", &[], cell)); + assert_ne!( + super::pair_identity("ab", &[], cell), + super::pair_identity("aab", &[], cell) + ); } #[test] fn issue_3047_pair_load_save_fixed_point_and_untrusted_metadata() { for sidecar in [false, true] { - for supplied in [serde_json::json!(null), serde_json::json!(false), serde_json::json!(42), - serde_json::json!(1.5), serde_json::json!([]), serde_json::json!({}), - serde_json::json!(""), serde_json::json!("forged")] { + for supplied in [ + serde_json::json!(null), + serde_json::json!(false), + serde_json::json!(42), + serde_json::json!(1.5), + serde_json::json!([]), + serde_json::json!({}), + serde_json::json!(""), + serde_json::json!("forged"), + ] { let (temp, path) = hash_activation_seed(sidecar); - let physical = if sidecar { b3_sidecar_path(temp.path()) } else { path.clone() }; + let physical = if sidecar { + b3_sidecar_path(temp.path()) + } else { + path.clone() + }; let mut raw = serde_json::Value::Object(b3_object(&physical)); raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ "alpha":{"A":{"additionalArguments":"--model x","id_hash":supplied, @@ -28774,26 +28910,150 @@ mod tests { let before = hash_activation_files(temp.path()); for (loader, loaded) in b3_all_loaders(&path) { let cell = &loaded.coding_agent_profiles.profiles_by_agent["alpha"]["A"]; - assert_eq!(cell.id_hash, Some(super::configuration_identity("claude--modelx", &[])), "{loader}"); + assert_eq!( + cell.id_hash, + Some(super::configuration_identity("claude--modelx", &[])), + "{loader}" + ); assert_eq!(hash_activation_files(temp.path()), before, "{loader}"); } let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); - let written = super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + let written = + super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); let groups = b3_object(&b3_sidecar_path(temp.path())); - assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], - written.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash.clone().unwrap()); + assert_eq!( + groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + written.coding_agent_profiles.profiles_by_agent["alpha"]["A"] + .id_hash + .clone() + .unwrap() + ); let stable = hash_activation_files(temp.path()); let reloaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); super::save_settings_to_path_preserving_project_paths(&reloaded, &path).unwrap(); assert_eq!(hash_activation_files(temp.path()), stable); let mut stale = reloaded.clone(); - stale.coding_agent_profiles.profiles_by_agent.get_mut("alpha").unwrap().get_mut("A").unwrap().id_hash = Some("stale".into()); + stale + .coding_agent_profiles + .profiles_by_agent + .get_mut("alpha") + .unwrap() + .get_mut("A") + .unwrap() + .id_hash = Some("stale".into()); let cas = super::compare_and_set_terminal_snapshots_enabled_at_path( - &stale, &path, stale.terminal_snapshots_enabled, stale.terminal_snapshots_enabled, - ).unwrap(); + &stale, + &path, + stale.terminal_snapshots_enabled, + stale.terminal_snapshots_enabled, + ) + .unwrap(); assert_eq!(cas.coding_agent_profiles, reloaded.coding_agent_profiles); } } + // Missing-A repair and matching orphan adoption happen after parse. + // Both CLI loaders must return final hashes without publishing anything. + for sidecar in [false, true] { + for orphan in ["missing", "A", "B", "unmatched"] { + for fail_publish in [false, true] { + let (temp, path) = hash_activation_seed(sidecar); + let physical = if sidecar { + b3_sidecar_path(temp.path()) + } else { + path.clone() + }; + let mut raw = serde_json::Value::Object(b3_object(&physical)); + let agent: super::AgentConfig = + serde_json::from_value(raw["agents"][0].clone()).unwrap(); + raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({}); + raw["codingAgentProfiles"]["identityByAgent"] = serde_json::json!({}); + raw["codingAgentProfiles"]["profileLabelsByAgent"] = serde_json::json!({}); + if orphan != "missing" { + let letter = if orphan == "B" { "B" } else { "A" }; + raw["codingAgentProfiles"]["profilesByAgent"]["o"] = serde_json::json!({ + letter:{"additionalArguments":"--special","notes":"payload"} + }); + raw["codingAgentProfiles"]["identityByAgent"]["o"] = + serde_json::json!(if orphan == "unmatched" { + "1234567890abcdef".to_string() + } else { + super::agent_profile_identity(&agent) + }); + } + b3_write(&physical, &raw); + let before = hash_activation_files(temp.path()); + let cli = super::load_settings_for_cli_from_path(&path); + let strict = + super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + for settings in [&cli, &strict] { + let cells = &settings.coding_agent_profiles.profiles_by_agent["alpha"]; + let mut expected_a = super::empty_profile_cell(); + expected_a.id_hash = Some(super::configuration_identity("claude", &[])); + if orphan == "A" { + expected_a.command = "--special".into(); + expected_a.notes = "payload".into(); + expected_a.id_hash = + Some(super::configuration_identity("claude--special", &[])); + } + assert_eq!(cells["A"], expected_a, "{sidecar}/{orphan}"); + if orphan == "B" { + assert_eq!(cells["B"].command, "--special"); + assert_eq!(cells["B"].notes, "payload"); + assert_eq!( + cells["B"].id_hash, + Some(super::configuration_identity("claude--special", &[])) + ); + } + if orphan == "unmatched" { + assert_eq!( + settings.coding_agent_profiles.profiles_by_agent["o"]["A"].id_hash, + None + ); + } else { + assert!(!settings + .coding_agent_profiles + .profiles_by_agent + .contains_key("o")); + } + } + assert_eq!(hash_activation_files(temp.path()), before); + let main_before = std::fs::read(&path).unwrap(); + if fail_publish { + super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| { + *slot.borrow_mut() = + Some((path.clone(), super::SettingsSaveStage::AtomicReplace)) + }); + } + let gui = super::load_settings_from_path(&path); + // Inspect the same existing failure boundary's diagnostic; + // the loader reports it and retains its repaired in-memory view. + if fail_publish { + let error = + super::save_settings_to_path_preserving_project_paths(&gui, &path) + .unwrap_err(); + assert_eq!(error.stage, super::SettingsSaveStage::AtomicReplace); + assert_eq!(error.target_path.as_ref(), &path); + } + super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| *slot.borrow_mut() = None); + assert_eq!( + gui.coding_agent_profiles.profiles_by_agent, + strict.coding_agent_profiles.profiles_by_agent + ); + if fail_publish { + assert_eq!(std::fs::read(&path).unwrap(), main_before); + assert!( + b3_object(&b3_sidecar_path(temp.path()))["codingAgentProfiles"] + ["profilesByAgent"]["alpha"]["A"]["id_hash"] + .is_string() + ); + } else { + let reload = + super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + assert_eq!(gui.coding_agent_profiles, reload.coding_agent_profiles); + } + } + } + } } #[test] @@ -28801,7 +29061,11 @@ mod tests { for sidecar in [false, true] { for invalid in [false, true] { let (temp, path) = hash_activation_seed(sidecar); - let physical = if sidecar { b3_sidecar_path(temp.path()) } else { path.clone() }; + let physical = if sidecar { + b3_sidecar_path(temp.path()) + } else { + path.clone() + }; let mut raw = serde_json::Value::Object(b3_object(&physical)); raw["agents"][0]["sentinel"] = serde_json::json!({"keep":19}); raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ @@ -28810,7 +29074,10 @@ mod tests { b3_write(&physical, &raw); let mut overlay_agent = b3_agent("alpha", "Overlay"); overlay_agent["command"] = serde_json::json!("codex --overlay"); - b3_write(&b3_overlay_path(temp.path()), &serde_json::json!({"agents":[overlay_agent]})); + b3_write( + &b3_overlay_path(temp.path()), + &serde_json::json!({"agents":[overlay_agent]}), + ); let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); assert!(loaded.local_overlay_state.owns_top_level("agents")); if invalid { @@ -28824,8 +29091,13 @@ mod tests { let mut out = serde_json::to_value(&loaded).unwrap(); out["codingAgentProfiles"] = raw["codingAgentProfiles"].clone(); let result = super::save_agents_instance_file( - &loaded, &path, &super::ProjectWriteMode::Preserve, None, - out.as_object().unwrap(), super::SettingsSaveStage::PreserveDiskGate, None, + &loaded, + &path, + &super::ProjectWriteMode::Preserve, + None, + out.as_object().unwrap(), + super::SettingsSaveStage::PreserveDiskGate, + None, ); if invalid { let error = result.unwrap_err(); @@ -28837,8 +29109,12 @@ mod tests { if text.contains("Refusing to overwrite") && text.contains("undecodable agents array"))); } else { assert_eq!(error.target_path.as_ref(), &path); - assert!(matches!(error.cause, super::SettingsSaveCause::Semantic( - super::SettingsSaveReason::DiskSettingsValidationRejected))); + assert!(matches!( + error.cause, + super::SettingsSaveCause::Semantic( + super::SettingsSaveReason::DiskSettingsValidationRejected + ) + )); assert!(matches!(&error.legacy_outward, super::SettingsSaveLegacyOutward::DiskValidation(text) if text.contains("non-project fields are invalid"))); @@ -28847,9 +29123,14 @@ mod tests { assert_eq!(std::fs::read(&physical).unwrap(), before); } else { let groups = result.unwrap(); - assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], - super::configuration_identity("claude--modelx", &[])); - assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["sentinel"], "cell"); + assert_eq!( + groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + super::configuration_identity("claude--modelx", &[]) + ); + assert_eq!( + groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["sentinel"], + "cell" + ); if sidecar { assert_eq!(groups["agents"], raw["agents"]); } else { @@ -28863,15 +29144,19 @@ mod tests { #[test] fn issue_3047_pair_unresolvable_first_row_and_empty_physical_base() { - for first in [serde_json::json!({"id":"a","baseCommand":17}), - serde_json::json!({"id":"a","baseCommand":"codex","envs":"bad"})] { + for first in [ + serde_json::json!({"id":"a","baseCommand":17}), + serde_json::json!({"id":"a","baseCommand":"codex","envs":"bad"}), + ] { let agents = serde_json::json!([first, {"id":"a","baseCommand":"valid"}]); let before = agents.clone(); let mut profiles = serde_json::json!({"profilesByAgent":{ "a":{"A":{"command":"--model x","id_hash":"stale","unknown":1}} }}); super::refresh_physical_pair_id_hashes(Some(&agents), &mut profiles); - assert!(profiles["profilesByAgent"]["a"]["A"].get("id_hash").is_none()); + assert!(profiles["profilesByAgent"]["a"]["A"] + .get("id_hash") + .is_none()); assert_eq!(profiles["profilesByAgent"]["a"]["A"]["unknown"], 1); assert_eq!(agents, before); let (temp, path) = hash_activation_seed(true); @@ -28880,9 +29165,15 @@ mod tests { b3_write(&b3_sidecar_path(temp.path()), &groups); let bytes = std::fs::read(b3_sidecar_path(temp.path())).unwrap(); assert!(super::save_agents_instance_file( - &super::AppSettings::default(), &path, &super::ProjectWriteMode::Preserve, - None, &serde_json::Map::new(), super::SettingsSaveStage::PreserveDiskGate, None, - ).is_err()); + &super::AppSettings::default(), + &path, + &super::ProjectWriteMode::Preserve, + None, + &serde_json::Map::new(), + super::SettingsSaveStage::PreserveDiskGate, + None, + ) + .is_err()); assert_eq!(std::fs::read(b3_sidecar_path(temp.path())).unwrap(), bytes); } for agents in [None, Some(serde_json::json!([]))] { @@ -28890,7 +29181,9 @@ mod tests { "a":{"A":{"id_hash":"stale"}} }}); super::refresh_physical_pair_id_hashes(agents.as_ref(), &mut profiles); - assert!(profiles["profilesByAgent"]["a"]["A"].get("id_hash").is_none()); + assert!(profiles["profilesByAgent"]["a"]["A"] + .get("id_hash") + .is_none()); } let agents = serde_json::json!([{"id":"a","command":"codex","label":17, "envs":[{"key":"K","value":"V","enabled":false}]}]); @@ -28899,9 +29192,14 @@ mod tests { }}); let before = profiles.clone(); super::refresh_physical_pair_id_hashes(Some(&agents), &mut profiles); - assert_eq!(profiles["profilesByAgent"]["a"]["A"]["id_hash"], - super::configuration_identity("codexKV--modelxPQ", &[])); - profiles["profilesByAgent"]["a"]["A"].as_object_mut().unwrap().remove("id_hash"); + assert_eq!( + profiles["profilesByAgent"]["a"]["A"]["id_hash"], + super::configuration_identity("codexKV--modelxPQ", &[]) + ); + profiles["profilesByAgent"]["a"]["A"] + .as_object_mut() + .unwrap() + .remove("id_hash"); assert_eq!(profiles, before); } @@ -28910,10 +29208,17 @@ mod tests { for sidecar in [false, true] { for protected in ["agents", "codingAgentProfiles", "neither"] { let (temp, path) = hash_activation_seed(sidecar); - let physical = if sidecar { b3_sidecar_path(temp.path()) } else { path.clone() }; + let physical = if sidecar { + b3_sidecar_path(temp.path()) + } else { + path.clone() + }; let mut raw = serde_json::Value::Object(b3_object(&physical)); - raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({ - "alpha":{"A":{"command":"--model x","id_hash":"physical"}} + raw["agents"][0]["sentinel"] = serde_json::json!({"agent": 19}); + raw["codingAgentProfiles"] = serde_json::json!({ + "schemaVersion": 2, + "sentinel": {"sibling": 23}, + "profilesByAgent": {"alpha":{"A":{"command":"--model x","id_hash":"physical", "sentinel":17}}} }); b3_write(&physical, &raw); if protected != "neither" { @@ -28923,33 +29228,165 @@ mod tests { value[0].as_object_mut().unwrap().remove("baseCommand"); value[0]["command"] = serde_json::json!("codex"); } else { - value["profilesByAgent"]["alpha"]["A"]["command"] = serde_json::json!("--effort high"); + value["profilesByAgent"]["alpha"]["A"]["command"] = + serde_json::json!("--effort high"); } overlay.insert(protected.into(), value); - b3_write(&b3_overlay_path(temp.path()), &serde_json::Value::Object(overlay)); + b3_write( + &b3_overlay_path(temp.path()), + &serde_json::Value::Object(overlay), + ); } - let mut loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + let mut loaded = + super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); if protected == "agents" { - loaded.coding_agent_profiles.profiles_by_agent.get_mut("alpha").unwrap().get_mut("A").unwrap().command = "--model y".into(); + loaded + .coding_agent_profiles + .profiles_by_agent + .get_mut("alpha") + .unwrap() + .get_mut("A") + .unwrap() + .command = "--model y".into(); } else { loaded.agents[0].command = "other-base".into(); } - let written = super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + let written = + super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); let cell = &written.coding_agent_profiles.profiles_by_agent["alpha"]["A"]; - assert_eq!(cell.id_hash, Some(super::pair_identity(&written.agents[0].command, &written.agents[0].envs, cell))); + let effective_base = if protected == "agents" { + "codex" + } else { + "other-base" + }; + let effective_args = if protected == "codingAgentProfiles" { + "--effort high" + } else if protected == "agents" { + "--model y" + } else { + "--model x" + }; + assert_eq!( + cell.id_hash, + Some(super::configuration_identity( + &format!("{effective_base}{effective_args}"), + &[] + )) + ); let groups = b3_object(&b3_sidecar_path(temp.path())); if protected == "codingAgentProfiles" { - let retained = groups.get(protected).cloned() + let retained = groups + .get(protected) + .cloned() .unwrap_or_else(|| b3_object(&path)[protected].clone()); - assert_eq!(retained, raw[protected]); + let mut expected = raw[protected].clone(); + if !sidecar { + expected["identityByAgent"] = serde_json::json!({ + "alpha": super::agent_profile_identity(&written.agents[0]) + }); + } + assert_eq!(retained, expected); } else { - let expected_base = if protected == "agents" { "claude" } else { "other-base" }; - let expected_args = if protected == "agents" { "--model y" } else { "--model x" }; - assert_eq!(groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], - super::configuration_identity(&format!("{expected_base}{expected_args}"), &[])); + let expected_base = if protected == "agents" { + "claude" + } else { + "other-base" + }; + let expected_args = if protected == "agents" { + "--model y" + } else { + "--model x" + }; + assert_eq!( + groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + super::configuration_identity( + &format!("{expected_base}{expected_args}"), + &[] + ) + ); + } + if protected == "agents" { + let retained = groups + .get("agents") + .cloned() + .unwrap_or_else(|| b3_object(&path)["agents"].clone()); + assert_eq!(retained, raw["agents"]); } let reloaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); - assert_eq!(reloaded.coding_agent_profiles, written.coding_agent_profiles); + assert_eq!( + reloaded.coding_agent_profiles, + written.coding_agent_profiles + ); + } + } + // Presence is independent of emptiness. Exercise the real writer with + // sparse legacy groups, including D13's partially protected group. + for case in [ + "present-absent", + "absent-present", + "absent-absent", + "empty-present", + "sidecar-empty", + ] { + let (temp, path) = hash_activation_seed(false); + let mut raw = serde_json::Value::Object(b3_object(&path)); + raw["agents"] = serde_json::json!([]); + let sparse = + serde_json::json!({"schemaVersion":2,"sentinel":{"raw":29},"profilesByAgent":{}}); + if case.starts_with("absent-") { + raw.as_object_mut().unwrap().remove("codingAgentProfiles"); + } else { + raw["codingAgentProfiles"] = sparse.clone(); + } + let overlay = match case { + "present-absent" => { + serde_json::json!({"codingAgentProfiles":{"identityByAgent":{"o":"1234567890abcdef"}}}) + } + "absent-present" => { + serde_json::json!({"agents":[b3_agent("alpha", "Effective only")]}) + } + _ => { + serde_json::json!({"codingAgentProfiles":{"profilesByAgent":{"o":{"A":{"command":"--model x"}}}}}) + } + }; + if case == "sidecar-empty" { + b3_write( + &b3_sidecar_path(temp.path()), + &serde_json::json!({"agents":[],"codingAgentProfiles":{}}), + ); + } + b3_write(&path, &raw); + b3_write(&b3_overlay_path(temp.path()), &overlay); + let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + assert!( + loaded + .local_overlay_state + .owns_top_level("codingAgentProfiles"), + "{case}" + ); + super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + let main = b3_object(&path); + let groups = b3_object(&b3_sidecar_path(temp.path())); + match case { + "present-absent" => assert_eq!(main["codingAgentProfiles"], sparse), + "absent-present" => assert_eq!( + main["codingAgentProfiles"], + serde_json::json!({"identityByAgent":{}}) + ), + "absent-absent" => assert!(!main.contains_key("codingAgentProfiles")), + "empty-present" => { + let mut expected = sparse.clone(); + expected["identityByAgent"] = serde_json::json!({}); + assert_eq!(main["codingAgentProfiles"], expected); + } + "sidecar-empty" => { + assert!(!main.contains_key("codingAgentProfiles")); + assert_eq!(groups["codingAgentProfiles"], serde_json::json!({})); + } + _ => unreachable!(), + } + if case != "sidecar-empty" { + assert!(!groups.contains_key("codingAgentProfiles"), "{case}"); } } } @@ -28964,16 +29401,30 @@ mod tests { "alpha":{"A":{"additionalArguments":"--model x","id_hash":"stale"}} }); b3_write(&sidecar, &groups); - b3_write(&b3_overlay_path(temp.path()), &serde_json::json!({ - "agents":[b3_agent("alpha", "Effective only")] - })); + b3_write( + &b3_overlay_path(temp.path()), + &serde_json::json!({ + "agents":[b3_agent("alpha", "Effective only")] + }), + ); let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); - let written = super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); - assert_eq!(written.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash, - Some(super::configuration_identity("claude--modelx", &[]))); + let written = + super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + assert_eq!( + written.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash, + Some(super::configuration_identity("claude--modelx", &[])) + ); let raw = b3_object(&sidecar); assert!(!raw.contains_key("agents")); - assert!(raw["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"].get("id_hash").is_none()); + assert!(loaded + .local_overlay_state + .owns_top_level("codingAgentProfiles")); + assert!(!groups.as_object().unwrap().contains_key("agents")); + assert_eq!(raw["codingAgentProfiles"], groups["codingAgentProfiles"]); + assert_eq!( + raw["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + "stale" + ); // Rejected overlays still derive the final accepted physical view. let (temp, path) = hash_activation_seed(true); @@ -28983,11 +29434,16 @@ mod tests { "alpha":{"A":{"additionalArguments":"--model x","id_hash":"stale"}} }); b3_write(&sidecar, &groups); - b3_write(&b3_overlay_path(temp.path()), &serde_json::json!({"defaultShellArgs":"bad"})); + b3_write( + &b3_overlay_path(temp.path()), + &serde_json::json!({"defaultShellArgs":"bad"}), + ); let loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); assert!(loaded.local_overlay_state.rejection().is_some()); - assert_eq!(loaded.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash, - Some(super::configuration_identity("claude--modelx", &[]))); + assert_eq!( + loaded.coding_agent_profiles.profiles_by_agent["alpha"]["A"].id_hash, + Some(super::configuration_identity("claude--modelx", &[])) + ); let previous = std::fs::read(&path).unwrap(); super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| { *slot.borrow_mut() = Some((path.clone(), super::SettingsSaveStage::AtomicReplace)) @@ -28996,8 +29452,10 @@ mod tests { super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| *slot.borrow_mut() = None); assert!(result.is_err()); assert_eq!(std::fs::read(&path).unwrap(), previous); - assert_eq!(b3_object(&sidecar)["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], - super::configuration_identity("claude--modelx", &[])); + assert_eq!( + b3_object(&sidecar)["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + super::configuration_identity("claude--modelx", &[]) + ); } fn hash_activation_assert(settings: &super::AppSettings) { From 14cf9c3a721c0a9cf5a8c4ba7edd58b1d7b803fe Mon Sep 17 00:00:00 2001 From: Mariano Blua Date: Sat, 17 Oct 2026 11:31:55 -0300 Subject: [PATCH 3/6] test(3051): qualify existing digest trait in pair oracle --- src-tauri/src/commands/config.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src-tauri/src/commands/config.rs b/src-tauri/src/commands/config.rs index a446fd230..9196bb914 100644 --- a/src-tauri/src/commands/config.rs +++ b/src-tauri/src/commands/config.rs @@ -5012,7 +5012,7 @@ mod tests { chars.sort_unstable(); cell.id_hash = Some(format!( "cid1:{:x}", - Sha256::digest(chars.iter().collect::()) + ::digest(chars.iter().collect::()) )); assert_eq!( written.coding_agent_profiles.profiles_by_agent["agent-0"][letter].id_hash, From 670cbe28f329276ebcb6b4a777df4494d0c5661d Mon Sep 17 00:00:00 2001 From: Mariano Blua Date: Sat, 17 Oct 2026 11:46:16 -0300 Subject: [PATCH 4/6] test(3051): use typed save error and capture GUI diagnostic --- src-tauri/src/config/settings.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src-tauri/src/config/settings.rs b/src-tauri/src/config/settings.rs index 5e720e3db..bf0b10982 100644 --- a/src-tauri/src/config/settings.rs +++ b/src-tauri/src/config/settings.rs @@ -29024,13 +29024,20 @@ mod tests { Some((path.clone(), super::SettingsSaveStage::AtomicReplace)) }); } + let (records, _capture_guard) = capture_settings_save_diagnostics(); let gui = super::load_settings_from_path(&path); - // Inspect the same existing failure boundary's diagnostic; - // the loader reports it and retains its repaired in-memory view. + // Inspect the GUI diagnostic and the same typed failure boundary. if fail_publish { - let error = - super::save_settings_to_path_preserving_project_paths(&gui, &path) - .unwrap_err(); + let captured = records.borrow(); + assert_eq!(captured.len(), 1); + let fields = parse_settings_save_diagnostic(&captured[0]); + assert_eq!(fields["stage"], "atomic_replace"); + assert_eq!(fields["target_path"], super::render_diagnostic_path(&path)); + drop(captured); + let error = super::save_settings_to_path_preserving_project_paths_typed( + &gui, &path, + ) + .unwrap_err(); assert_eq!(error.stage, super::SettingsSaveStage::AtomicReplace); assert_eq!(error.target_path.as_ref(), &path); } From d8ba354dcdda612505ff7f21a153e82ecae0d136 Mon Sep 17 00:00:00 2001 From: Mariano Blua Date: Sat, 17 Oct 2026 12:22:02 -0300 Subject: [PATCH 5/6] refactor(3051): split pair persistence blocks for cognitive gate --- src-tauri/src/config/settings.rs | 569 ++++++++++++++++--------------- 1 file changed, 301 insertions(+), 268 deletions(-) diff --git a/src-tauri/src/config/settings.rs b/src-tauri/src/config/settings.rs index bf0b10982..de12adee8 100644 --- a/src-tauri/src/config/settings.rs +++ b/src-tauri/src/config/settings.rs @@ -13479,6 +13479,51 @@ fn save_settings_value_locked( ) } +fn restore_protected_agents_instance_groups( + out: &mut Map, + disk: Option<&Map>, + overlay: &LocalSettingsOverlay, + agents_groups: &Map, +) { + let identity_rows = out + .get("codingAgentProfiles") + .and_then(|profiles| profiles.get("identityByAgent")) + .cloned(); + for key in AGENTS_INSTANCE_KEYS { + if !overlay.owns_top_level(key) || agents_groups.contains_key(key) { + continue; + } + match disk.and_then(|disk| disk.get(key)) { + Some(raw) => { + out.insert(key.to_owned(), raw.clone()); + } + None => { + out.remove(key); + } + } + // Only legacy MAIN identities retain the existing #2450 writer output. + // All other fields of a protected profiles group remain raw disk values. + if key == "codingAgentProfiles" { + match &identity_rows { + Some(rows) => { + let profiles = out + .entry(key.to_owned()) + .or_insert_with(|| Value::Object(Map::new())); + profiles + .as_object_mut() + .expect("validated profiles object") + .insert("identityByAgent".to_owned(), rows.clone()); + } + None => { + if let Some(profiles) = out.get_mut(key).and_then(Value::as_object_mut) { + profiles.remove("identityByAgent"); + } + } + } + } + } +} + fn save_settings_value_locked_with_startup( settings: &AppSettings, path: &Path, @@ -13708,43 +13753,12 @@ fn save_settings_value_locked_with_startup( // written verbatim from the live settings/disk, matching pre-#1077 behavior. // Restore complete protected groups after every retention/repair stage. // Sidecar key presence, including empty values, takes precedence over main. - let identity_rows = out - .get("codingAgentProfiles") - .and_then(|profiles| profiles.get("identityByAgent")) - .cloned(); - for key in AGENTS_INSTANCE_KEYS { - if !settings.local_overlay_state.owns_top_level(key) || agents_groups.contains_key(key) { - continue; - } - match disk.as_ref().and_then(|disk| disk.get(key)) { - Some(raw) => { - out.insert(key.to_owned(), raw.clone()); - } - None => { - out.remove(key); - } - } - // Only legacy MAIN identities retain the existing #2450 writer output. - // All other fields of a protected profiles group remain raw disk values. - if key == "codingAgentProfiles" { - match &identity_rows { - Some(rows) => { - let profiles = out - .entry(key.to_owned()) - .or_insert_with(|| Value::Object(Map::new())); - profiles - .as_object_mut() - .expect("validated profiles object") - .insert("identityByAgent".to_owned(), rows.clone()); - } - None => { - if let Some(profiles) = out.get_mut(key).and_then(Value::as_object_mut) { - profiles.remove("identityByAgent"); - } - } - } - } - } + restore_protected_agents_instance_groups( + &mut out, + disk.as_ref(), + &settings.local_overlay_state, + &agents_groups, + ); let value = Value::Object(out); let written_bytes = write_value_atomic(&value, path)?; if let Some(previous) = previous_contents { @@ -28882,6 +28896,112 @@ mod tests { ); } + fn assert_issue_3047_pair_adoption_view( + settings: &super::AppSettings, + orphan: &str, + sidecar: bool, + ) { + let cells = &settings.coding_agent_profiles.profiles_by_agent["alpha"]; + let mut expected_a = super::empty_profile_cell(); + expected_a.id_hash = Some(super::configuration_identity("claude", &[])); + if orphan == "A" { + expected_a.command = "--special".into(); + expected_a.notes = "payload".into(); + expected_a.id_hash = Some(super::configuration_identity("claude--special", &[])); + } + assert_eq!(cells["A"], expected_a, "{sidecar}/{orphan}"); + if orphan == "B" { + assert_eq!(cells["B"].command, "--special"); + assert_eq!(cells["B"].notes, "payload"); + assert_eq!( + cells["B"].id_hash, + Some(super::configuration_identity("claude--special", &[])) + ); + } + if orphan == "unmatched" { + assert_eq!( + settings.coding_agent_profiles.profiles_by_agent["o"]["A"].id_hash, + None + ); + } else { + assert!(!settings + .coding_agent_profiles + .profiles_by_agent + .contains_key("o")); + } + } + + fn check_issue_3047_pair_adoption_case(sidecar: bool, orphan: &str, fail_publish: bool) { + let (temp, path) = hash_activation_seed(sidecar); + let physical = if sidecar { + b3_sidecar_path(temp.path()) + } else { + path.clone() + }; + let mut raw = serde_json::Value::Object(b3_object(&physical)); + let agent: super::AgentConfig = serde_json::from_value(raw["agents"][0].clone()).unwrap(); + raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({}); + raw["codingAgentProfiles"]["identityByAgent"] = serde_json::json!({}); + raw["codingAgentProfiles"]["profileLabelsByAgent"] = serde_json::json!({}); + if orphan != "missing" { + let letter = if orphan == "B" { "B" } else { "A" }; + raw["codingAgentProfiles"]["profilesByAgent"]["o"] = serde_json::json!({ + letter:{"additionalArguments":"--special","notes":"payload"} + }); + raw["codingAgentProfiles"]["identityByAgent"]["o"] = + serde_json::json!(if orphan == "unmatched" { + "1234567890abcdef".to_string() + } else { + super::agent_profile_identity(&agent) + }); + } + b3_write(&physical, &raw); + let before = hash_activation_files(temp.path()); + let cli = super::load_settings_for_cli_from_path(&path); + let strict = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + for settings in [&cli, &strict] { + assert_issue_3047_pair_adoption_view(settings, orphan, sidecar); + } + assert_eq!(hash_activation_files(temp.path()), before); + let main_before = std::fs::read(&path).unwrap(); + if fail_publish { + super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| { + *slot.borrow_mut() = Some((path.clone(), super::SettingsSaveStage::AtomicReplace)) + }); + } + let (records, _capture_guard) = capture_settings_save_diagnostics(); + let gui = super::load_settings_from_path(&path); + // Inspect the GUI diagnostic and the same typed failure boundary. + if fail_publish { + let captured = records.borrow(); + assert_eq!(captured.len(), 1); + let fields = parse_settings_save_diagnostic(&captured[0]); + assert_eq!(fields["stage"], "atomic_replace"); + assert_eq!(fields["target_path"], super::render_diagnostic_path(&path)); + drop(captured); + let error = super::save_settings_to_path_preserving_project_paths_typed(&gui, &path) + .unwrap_err(); + assert_eq!(error.stage, super::SettingsSaveStage::AtomicReplace); + assert_eq!(error.target_path.as_ref(), &path); + } + super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| *slot.borrow_mut() = None); + assert_eq!( + gui.coding_agent_profiles.profiles_by_agent, + strict.coding_agent_profiles.profiles_by_agent + ); + if fail_publish { + assert_eq!(std::fs::read(&path).unwrap(), main_before); + assert!( + b3_object(&b3_sidecar_path(temp.path()))["codingAgentProfiles"]["profilesByAgent"] + ["alpha"]["A"]["id_hash"] + .is_string() + ); + } else { + let reload = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + assert_eq!(gui.coding_agent_profiles, reload.coding_agent_profiles); + } + } + #[test] fn issue_3047_pair_load_save_fixed_point_and_untrusted_metadata() { for sidecar in [false, true] { @@ -28956,113 +29076,39 @@ mod tests { for sidecar in [false, true] { for orphan in ["missing", "A", "B", "unmatched"] { for fail_publish in [false, true] { - let (temp, path) = hash_activation_seed(sidecar); - let physical = if sidecar { - b3_sidecar_path(temp.path()) - } else { - path.clone() - }; - let mut raw = serde_json::Value::Object(b3_object(&physical)); - let agent: super::AgentConfig = - serde_json::from_value(raw["agents"][0].clone()).unwrap(); - raw["codingAgentProfiles"]["profilesByAgent"] = serde_json::json!({}); - raw["codingAgentProfiles"]["identityByAgent"] = serde_json::json!({}); - raw["codingAgentProfiles"]["profileLabelsByAgent"] = serde_json::json!({}); - if orphan != "missing" { - let letter = if orphan == "B" { "B" } else { "A" }; - raw["codingAgentProfiles"]["profilesByAgent"]["o"] = serde_json::json!({ - letter:{"additionalArguments":"--special","notes":"payload"} - }); - raw["codingAgentProfiles"]["identityByAgent"]["o"] = - serde_json::json!(if orphan == "unmatched" { - "1234567890abcdef".to_string() - } else { - super::agent_profile_identity(&agent) - }); - } - b3_write(&physical, &raw); - let before = hash_activation_files(temp.path()); - let cli = super::load_settings_for_cli_from_path(&path); - let strict = - super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); - for settings in [&cli, &strict] { - let cells = &settings.coding_agent_profiles.profiles_by_agent["alpha"]; - let mut expected_a = super::empty_profile_cell(); - expected_a.id_hash = Some(super::configuration_identity("claude", &[])); - if orphan == "A" { - expected_a.command = "--special".into(); - expected_a.notes = "payload".into(); - expected_a.id_hash = - Some(super::configuration_identity("claude--special", &[])); - } - assert_eq!(cells["A"], expected_a, "{sidecar}/{orphan}"); - if orphan == "B" { - assert_eq!(cells["B"].command, "--special"); - assert_eq!(cells["B"].notes, "payload"); - assert_eq!( - cells["B"].id_hash, - Some(super::configuration_identity("claude--special", &[])) - ); - } - if orphan == "unmatched" { - assert_eq!( - settings.coding_agent_profiles.profiles_by_agent["o"]["A"].id_hash, - None - ); - } else { - assert!(!settings - .coding_agent_profiles - .profiles_by_agent - .contains_key("o")); - } - } - assert_eq!(hash_activation_files(temp.path()), before); - let main_before = std::fs::read(&path).unwrap(); - if fail_publish { - super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| { - *slot.borrow_mut() = - Some((path.clone(), super::SettingsSaveStage::AtomicReplace)) - }); - } - let (records, _capture_guard) = capture_settings_save_diagnostics(); - let gui = super::load_settings_from_path(&path); - // Inspect the GUI diagnostic and the same typed failure boundary. - if fail_publish { - let captured = records.borrow(); - assert_eq!(captured.len(), 1); - let fields = parse_settings_save_diagnostic(&captured[0]); - assert_eq!(fields["stage"], "atomic_replace"); - assert_eq!(fields["target_path"], super::render_diagnostic_path(&path)); - drop(captured); - let error = super::save_settings_to_path_preserving_project_paths_typed( - &gui, &path, - ) - .unwrap_err(); - assert_eq!(error.stage, super::SettingsSaveStage::AtomicReplace); - assert_eq!(error.target_path.as_ref(), &path); - } - super::PROJECT_MIGRATION_ATOMIC_FAULT.with(|slot| *slot.borrow_mut() = None); - assert_eq!( - gui.coding_agent_profiles.profiles_by_agent, - strict.coding_agent_profiles.profiles_by_agent - ); - if fail_publish { - assert_eq!(std::fs::read(&path).unwrap(), main_before); - assert!( - b3_object(&b3_sidecar_path(temp.path()))["codingAgentProfiles"] - ["profilesByAgent"]["alpha"]["A"]["id_hash"] - .is_string() - ); - } else { - let reload = - super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); - assert_eq!(gui.coding_agent_profiles, reload.coding_agent_profiles); - } + check_issue_3047_pair_adoption_case(sidecar, orphan, fail_publish); } } } } + fn assert_issue_3047_pair_projection_rejection( + error: super::SettingsSaveError, + sidecar: bool, + path: &std::path::Path, + sidecar_path: &std::path::Path, + ) { + assert_eq!(error.stage, super::SettingsSaveStage::PreserveDiskGate); + if sidecar { + assert_eq!(error.target_path.as_path(), sidecar_path); + assert!(matches!(&error.legacy_outward, + super::SettingsSaveLegacyOutward::DiskRead(text) + if text.contains("Refusing to overwrite") && text.contains("undecodable agents array"))); + } else { + assert_eq!(error.target_path.as_path(), path); + assert!(matches!( + error.cause, + super::SettingsSaveCause::Semantic( + super::SettingsSaveReason::DiskSettingsValidationRejected + ) + )); + assert!(matches!(&error.legacy_outward, + super::SettingsSaveLegacyOutward::DiskValidation(text) + if text.contains("non-project fields are invalid"))); + assert!(!sidecar_path.exists()); + } + } + #[test] fn issue_3047_pair_protected_base_projection_and_early_rejections() { for sidecar in [false, true] { @@ -29108,25 +29154,12 @@ mod tests { ); if invalid { let error = result.unwrap_err(); - assert_eq!(error.stage, super::SettingsSaveStage::PreserveDiskGate); - if sidecar { - assert_eq!(error.target_path.as_ref(), &b3_sidecar_path(temp.path())); - assert!(matches!(&error.legacy_outward, - super::SettingsSaveLegacyOutward::DiskRead(text) - if text.contains("Refusing to overwrite") && text.contains("undecodable agents array"))); - } else { - assert_eq!(error.target_path.as_ref(), &path); - assert!(matches!( - error.cause, - super::SettingsSaveCause::Semantic( - super::SettingsSaveReason::DiskSettingsValidationRejected - ) - )); - assert!(matches!(&error.legacy_outward, - super::SettingsSaveLegacyOutward::DiskValidation(text) - if text.contains("non-project fields are invalid"))); - assert!(!b3_sidecar_path(temp.path()).exists()); - } + assert_issue_3047_pair_projection_rejection( + error, + sidecar, + &path, + &b3_sidecar_path(temp.path()), + ); assert_eq!(std::fs::read(&physical).unwrap(), before); } else { let groups = result.unwrap(); @@ -29210,120 +29243,120 @@ mod tests { assert_eq!(profiles, before); } + fn check_issue_3047_pair_ownership_case(sidecar: bool, protected: &str) { + let (temp, path) = hash_activation_seed(sidecar); + let physical = if sidecar { + b3_sidecar_path(temp.path()) + } else { + path.clone() + }; + let mut raw = serde_json::Value::Object(b3_object(&physical)); + raw["agents"][0]["sentinel"] = serde_json::json!({"agent": 19}); + raw["codingAgentProfiles"] = serde_json::json!({ + "schemaVersion": 2, + "sentinel": {"sibling": 23}, + "profilesByAgent": {"alpha":{"A":{"command":"--model x","id_hash":"physical", "sentinel":17}}} + }); + b3_write(&physical, &raw); + if protected != "neither" { + let mut overlay = serde_json::Map::new(); + let mut value = raw[protected].clone(); + if protected == "agents" { + value[0].as_object_mut().unwrap().remove("baseCommand"); + value[0]["command"] = serde_json::json!("codex"); + } else { + value["profilesByAgent"]["alpha"]["A"]["command"] = + serde_json::json!("--effort high"); + } + overlay.insert(protected.into(), value); + b3_write( + &b3_overlay_path(temp.path()), + &serde_json::Value::Object(overlay), + ); + } + let mut loaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + if protected == "agents" { + loaded + .coding_agent_profiles + .profiles_by_agent + .get_mut("alpha") + .unwrap() + .get_mut("A") + .unwrap() + .command = "--model y".into(); + } else { + loaded.agents[0].command = "other-base".into(); + } + let written = + super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); + let cell = &written.coding_agent_profiles.profiles_by_agent["alpha"]["A"]; + let effective_base = if protected == "agents" { + "codex" + } else { + "other-base" + }; + let effective_args = if protected == "codingAgentProfiles" { + "--effort high" + } else if protected == "agents" { + "--model y" + } else { + "--model x" + }; + assert_eq!( + cell.id_hash, + Some(super::configuration_identity( + &format!("{effective_base}{effective_args}"), + &[] + )) + ); + let groups = b3_object(&b3_sidecar_path(temp.path())); + if protected == "codingAgentProfiles" { + let retained = groups + .get(protected) + .cloned() + .unwrap_or_else(|| b3_object(&path)[protected].clone()); + let mut expected = raw[protected].clone(); + if !sidecar { + expected["identityByAgent"] = serde_json::json!({ + "alpha": super::agent_profile_identity(&written.agents[0]) + }); + } + assert_eq!(retained, expected); + } else { + let expected_base = if protected == "agents" { + "claude" + } else { + "other-base" + }; + let expected_args = if protected == "agents" { + "--model y" + } else { + "--model x" + }; + assert_eq!( + groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], + super::configuration_identity(&format!("{expected_base}{expected_args}"), &[]) + ); + } + if protected == "agents" { + let retained = groups + .get("agents") + .cloned() + .unwrap_or_else(|| b3_object(&path)["agents"].clone()); + assert_eq!(retained, raw["agents"]); + } + let reloaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); + assert_eq!( + reloaded.coding_agent_profiles, + written.coding_agent_profiles + ); + } + #[test] fn issue_3047_pair_ownership_matrix_and_base_or_profile_only_edits() { for sidecar in [false, true] { for protected in ["agents", "codingAgentProfiles", "neither"] { - let (temp, path) = hash_activation_seed(sidecar); - let physical = if sidecar { - b3_sidecar_path(temp.path()) - } else { - path.clone() - }; - let mut raw = serde_json::Value::Object(b3_object(&physical)); - raw["agents"][0]["sentinel"] = serde_json::json!({"agent": 19}); - raw["codingAgentProfiles"] = serde_json::json!({ - "schemaVersion": 2, - "sentinel": {"sibling": 23}, - "profilesByAgent": {"alpha":{"A":{"command":"--model x","id_hash":"physical", "sentinel":17}}} - }); - b3_write(&physical, &raw); - if protected != "neither" { - let mut overlay = serde_json::Map::new(); - let mut value = raw[protected].clone(); - if protected == "agents" { - value[0].as_object_mut().unwrap().remove("baseCommand"); - value[0]["command"] = serde_json::json!("codex"); - } else { - value["profilesByAgent"]["alpha"]["A"]["command"] = - serde_json::json!("--effort high"); - } - overlay.insert(protected.into(), value); - b3_write( - &b3_overlay_path(temp.path()), - &serde_json::Value::Object(overlay), - ); - } - let mut loaded = - super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); - if protected == "agents" { - loaded - .coding_agent_profiles - .profiles_by_agent - .get_mut("alpha") - .unwrap() - .get_mut("A") - .unwrap() - .command = "--model y".into(); - } else { - loaded.agents[0].command = "other-base".into(); - } - let written = - super::save_settings_to_path_preserving_project_paths(&loaded, &path).unwrap(); - let cell = &written.coding_agent_profiles.profiles_by_agent["alpha"]["A"]; - let effective_base = if protected == "agents" { - "codex" - } else { - "other-base" - }; - let effective_args = if protected == "codingAgentProfiles" { - "--effort high" - } else if protected == "agents" { - "--model y" - } else { - "--model x" - }; - assert_eq!( - cell.id_hash, - Some(super::configuration_identity( - &format!("{effective_base}{effective_args}"), - &[] - )) - ); - let groups = b3_object(&b3_sidecar_path(temp.path())); - if protected == "codingAgentProfiles" { - let retained = groups - .get(protected) - .cloned() - .unwrap_or_else(|| b3_object(&path)[protected].clone()); - let mut expected = raw[protected].clone(); - if !sidecar { - expected["identityByAgent"] = serde_json::json!({ - "alpha": super::agent_profile_identity(&written.agents[0]) - }); - } - assert_eq!(retained, expected); - } else { - let expected_base = if protected == "agents" { - "claude" - } else { - "other-base" - }; - let expected_args = if protected == "agents" { - "--model y" - } else { - "--model x" - }; - assert_eq!( - groups["codingAgentProfiles"]["profilesByAgent"]["alpha"]["A"]["id_hash"], - super::configuration_identity( - &format!("{expected_base}{expected_args}"), - &[] - ) - ); - } - if protected == "agents" { - let retained = groups - .get("agents") - .cloned() - .unwrap_or_else(|| b3_object(&path)["agents"].clone()); - assert_eq!(retained, raw["agents"]); - } - let reloaded = super::load_settings_for_cli_strict_from_path(Some(&path)).unwrap(); - assert_eq!( - reloaded.coding_agent_profiles, - written.coding_agent_profiles - ); + check_issue_3047_pair_ownership_case(sidecar, protected); } } // Presence is independent of emptiness. Exercise the real writer with From b482edc61b0d43ed64f90d63a2369f096cdb03f1 Mon Sep 17 00:00:00 2001 From: Mariano Blua Date: Sat, 17 Oct 2026 14:01:32 -0300 Subject: [PATCH 6/6] test(3051): reconcile persistence controls with protected groups and pair hashes --- src-tauri/src/commands/config.rs | 392 +++++++++++++++++++++---------- src-tauri/src/config/settings.rs | 80 ++++++- 2 files changed, 340 insertions(+), 132 deletions(-) diff --git a/src-tauri/src/commands/config.rs b/src-tauri/src/commands/config.rs index 9196bb914..5cc28847b 100644 --- a/src-tauri/src/commands/config.rs +++ b/src-tauri/src/commands/config.rs @@ -9088,142 +9088,290 @@ mod tests { // P5 #[tokio::test] async fn a_profile_edit_persists_except_for_the_overlay_introduced_agent() { - let temp = tempfile::tempdir().unwrap(); - let path = seed(temp.path(), &scratch_agent_overlay()); - let loaded = load_settings_from_path(&path); - assert!(loaded - .coding_agent_profiles - .profiles_by_agent - .contains_key("scratch-agent")); - let state = state_for(loaded.clone()); + for protected in [true, false] { + let temp = tempfile::tempdir().unwrap(); + let overlay = if protected { + scratch_agent_overlay() + } else { + json!({}) + }; + let path = seed(temp.path(), &overlay); + let mut expected_raw = disk(&path)["codingAgentProfiles"].clone(); + // Independent #2450 preimages for these two fixed seed rows. + expected_raw["identityByAgent"] = json!({ + "codex":"460361e30eb7c7a5", "claude":"57bf1d23533f9c40" + }); + let loaded = load_settings_from_path(&path); + assert_eq!( + loaded + .local_overlay_state + .owns_top_level("codingAgentProfiles"), + protected + ); + assert_eq!( + loaded + .coding_agent_profiles + .profiles_by_agent + .contains_key("scratch-agent"), + protected + ); - let mut draft = renderer_round_trip(&loaded); - draft - .coding_agent_profiles - .profiles_by_agent - .get_mut("codex") - .unwrap() - .get_mut("A") - .unwrap() - .command = "codex --edited".to_string(); - draft.coding_agent_profiles.profile_slots.insert( - "B".to_string(), - crate::config::settings::ProfileSlotConfig { - label: "Second".to_string(), - }, - ); - draft - .coding_agent_profiles - .default_profile_by_agent - .insert("codex".to_string(), "A".to_string()); + let state = state_for(loaded.clone()); - let path2 = path.clone(); - persist_settings_draft_update_with_saver(&state, draft, move |candidate| { - save_settings_to_path_preserving_project_paths(candidate, &path2) - }) - .await - .unwrap(); + let mut draft = renderer_round_trip(&loaded); + draft + .coding_agent_profiles + .profiles_by_agent + .get_mut("codex") + .unwrap() + .get_mut("A") + .unwrap() + .command = "codex --edited".to_string(); + draft.coding_agent_profiles.profile_slots.insert( + "B".to_string(), + crate::config::settings::ProfileSlotConfig { + label: "Second".to_string(), + }, + ); + draft + .coding_agent_profiles + .default_profile_by_agent + .insert("codex".to_string(), "A".to_string()); + + let path2 = path.clone(); + let (saved, _events) = + persist_settings_draft_update_with_saver(&state, draft, move |candidate| { + save_settings_to_path_preserving_project_paths(candidate, &path2) + }) + .await + .unwrap(); - let profiles = disk(&path)["codingAgentProfiles"].clone(); - assert_eq!( - profiles["profilesByAgent"]["codex"]["A"]["additionalArguments"], - json!("codex --edited") - ); - assert_eq!( - profiles["profilesByAgent"]["codex"]["A"]["command"], - json!("codex") - ); - assert_eq!(profiles["profileSlots"]["B"]["label"], json!("Second")); - assert_eq!(profiles["defaultProfileByAgent"]["codex"], json!("A")); - assert!( - profiles["profilesByAgent"].get("scratch-agent").is_none(), - "only the overlay-introduced agent's cells are frozen: {profiles}" - ); + let physical_path = if protected { + path.clone() + } else { + path.with_file_name("agents.30.instance.no-git.json") + }; + let profiles = disk(&physical_path)["codingAgentProfiles"].clone(); + if protected { + assert_eq!( + profiles, expected_raw, + "whole protected group, only existing identity output excepted" + ); + } else { + assert_eq!( + profiles["profilesByAgent"]["codex"]["A"]["additionalArguments"], + json!("codex --edited") + ); + assert_eq!( + profiles["profilesByAgent"]["codex"]["A"]["command"], + json!("codex") + ); + assert_eq!(profiles["profileSlots"]["B"]["label"], json!("Second")); + assert_eq!(profiles["defaultProfileByAgent"]["codex"], json!("A")); + } + assert!(profiles["profilesByAgent"].get("scratch-agent").is_none()); + let expected_command = if protected { "codex" } else { "codex --edited" }; + let expected_hash = if protected { + "cid1:439b584f39ee418c1c6e59bb9b06e7f04b83c2b338d48580f1fb8b7a1750703d" + } else { + "cid1:2d09a350fb33981f1d78f968766bba7ba46ae78531a2ee157653926809a03114" + }; + assert_eq!( + saved.coding_agent_profiles.profiles_by_agent["codex"]["A"].command, + expected_command + ); + assert_eq!( + saved.coding_agent_profiles.profiles_by_agent["codex"]["A"] + .id_hash + .as_deref(), + Some(expected_hash) + ); + let reloaded = load_settings_from_path(&path); + assert_eq!(saved.coding_agent_profiles, reloaded.coding_agent_profiles); + assert_eq!( + state.read().await.coding_agent_profiles, + saved.coding_agent_profiles + ); + } } // P6 #[test] fn the_dedicated_profiles_command_shape_has_the_same_outcome() { - let temp = tempfile::tempdir().unwrap(); - let path = seed(temp.path(), &scratch_agent_overlay()); - let loaded = load_settings_from_path(&path); - - // `persist_coding_agent_profiles_update` clones the live guard, assigns - // the payload and saves; `save_settings` delegates to the path-taking - // preserving writer for a resolved path. Reproduced here because that - // helper takes no injectable saver and redirecting `save_settings` - // would need a process-global env mutation. - let mut candidate = loaded.clone(); - candidate - .coding_agent_profiles - .profiles_by_agent - .get_mut("codex") - .unwrap() - .get_mut("A") - .unwrap() - .command = "codex --edited".to_string(); - candidate.coding_agent_profiles.profile_slots.insert( - "B".to_string(), - crate::config::settings::ProfileSlotConfig { - label: "Second".to_string(), - }, - ); - candidate - .coding_agent_profiles - .default_profile_by_agent - .insert("codex".to_string(), "A".to_string()); - validate_and_repair_settings(&mut candidate).unwrap(); - save_settings_to_path_preserving_project_paths(&candidate, &path).unwrap(); + for protected in [true, false] { + let temp = tempfile::tempdir().unwrap(); + let overlay = if protected { + scratch_agent_overlay() + } else { + json!({}) + }; + let path = seed(temp.path(), &overlay); + let mut expected_raw = disk(&path)["codingAgentProfiles"].clone(); + // Independent #2450 preimages for these two fixed seed rows. + expected_raw["identityByAgent"] = json!({ + "codex":"460361e30eb7c7a5", "claude":"57bf1d23533f9c40" + }); + let loaded = load_settings_from_path(&path); + assert_eq!( + loaded + .local_overlay_state + .owns_top_level("codingAgentProfiles"), + protected + ); + assert_eq!( + loaded + .coding_agent_profiles + .profiles_by_agent + .contains_key("scratch-agent"), + protected + ); - let profiles = disk(&path)["codingAgentProfiles"].clone(); - assert_eq!( - profiles["profilesByAgent"]["codex"]["A"]["additionalArguments"], - json!("codex --edited") - ); - assert_eq!( - profiles["profilesByAgent"]["codex"]["A"]["command"], - json!("codex") - ); - assert_eq!(profiles["profileSlots"]["B"]["label"], json!("Second")); - assert_eq!(profiles["defaultProfileByAgent"]["codex"], json!("A")); - assert!(profiles["profilesByAgent"].get("scratch-agent").is_none()); + // `persist_coding_agent_profiles_update` clones the live guard, assigns + // the payload and saves; `save_settings` delegates to the path-taking + // preserving writer for a resolved path. Reproduced here because that + // helper takes no injectable saver and redirecting `save_settings` + // would need a process-global env mutation. + let mut candidate = loaded.clone(); + candidate + .coding_agent_profiles + .profiles_by_agent + .get_mut("codex") + .unwrap() + .get_mut("A") + .unwrap() + .command = "codex --edited".to_string(); + candidate.coding_agent_profiles.profile_slots.insert( + "B".to_string(), + crate::config::settings::ProfileSlotConfig { + label: "Second".to_string(), + }, + ); + candidate + .coding_agent_profiles + .default_profile_by_agent + .insert("codex".to_string(), "A".to_string()); + validate_and_repair_settings(&mut candidate).unwrap(); + let saved = + save_settings_to_path_preserving_project_paths(&candidate, &path).unwrap(); + + let physical_path = if protected { + path.clone() + } else { + path.with_file_name("agents.30.instance.no-git.json") + }; + let profiles = disk(&physical_path)["codingAgentProfiles"].clone(); + if protected { + assert_eq!( + profiles, expected_raw, + "whole protected group, only existing identity output excepted" + ); + } else { + assert_eq!( + profiles["profilesByAgent"]["codex"]["A"]["additionalArguments"], + json!("codex --edited") + ); + assert_eq!( + profiles["profilesByAgent"]["codex"]["A"]["command"], + json!("codex") + ); + assert_eq!(profiles["profileSlots"]["B"]["label"], json!("Second")); + assert_eq!(profiles["defaultProfileByAgent"]["codex"], json!("A")); + } + assert!(profiles["profilesByAgent"].get("scratch-agent").is_none()); + let expected_command = if protected { "codex" } else { "codex --edited" }; + let expected_hash = if protected { + "cid1:439b584f39ee418c1c6e59bb9b06e7f04b83c2b338d48580f1fb8b7a1750703d" + } else { + "cid1:2d09a350fb33981f1d78f968766bba7ba46ae78531a2ee157653926809a03114" + }; + assert_eq!( + saved.coding_agent_profiles.profiles_by_agent["codex"]["A"].command, + expected_command + ); + assert_eq!( + saved.coding_agent_profiles.profiles_by_agent["codex"]["A"] + .id_hash + .as_deref(), + Some(expected_hash) + ); + let reloaded = load_settings_from_path(&path); + assert_eq!(saved.coding_agent_profiles, reloaded.coding_agent_profiles); + } } // P7 #[test] fn an_agent_delete_removes_its_default_profile_entry_from_the_base_file() { - let temp = tempfile::tempdir().unwrap(); - let path = seed(temp.path(), &scratch_agent_overlay()); - let mut seeded = load_settings_from_path(&path); - seeded - .coding_agent_profiles - .default_profile_by_agent - .insert("claude".to_string(), "A".to_string()); - save_settings_to_path_preserving_project_paths(&seeded, &path).unwrap(); - assert_eq!( - disk(&path)["codingAgentProfiles"]["defaultProfileByAgent"]["claude"], - json!("A") - ); - - // The agent-delete shape: remove the entry from a clone of the live - // guard and save. - let loaded = load_settings_from_path(&path); - let mut candidate = loaded.clone(); - candidate - .coding_agent_profiles - .default_profile_by_agent - .remove("claude"); - save_settings_to_path_preserving_project_paths(&candidate, &path).unwrap(); - - assert!( - disk(&path)["codingAgentProfiles"]["defaultProfileByAgent"] - .get("claude") - .is_none(), - "the removal must persist" - ); - assert!(disk(&path)["codingAgentProfiles"]["profilesByAgent"] - .get("scratch-agent") - .is_none()); + for protected in [true, false] { + let temp = tempfile::tempdir().unwrap(); + let overlay = if protected { + scratch_agent_overlay() + } else { + json!({}) + }; + let path = seed(temp.path(), &overlay); + // Seed the physical default before protection. The negative + // must prove an existing A survives, not that absent stays absent. + let mut raw = disk(&path); + raw["codingAgentProfiles"]["defaultProfileByAgent"]["claude"] = json!("A"); + let mut expected_raw = raw["codingAgentProfiles"].clone(); + expected_raw["identityByAgent"] = json!({ + "codex":"460361e30eb7c7a5", "claude":"57bf1d23533f9c40" + }); + std::fs::write(&path, serde_json::to_vec_pretty(&raw).unwrap()).unwrap(); + let loaded = load_settings_from_path(&path); + assert_eq!( + loaded + .local_overlay_state + .owns_top_level("codingAgentProfiles"), + protected + ); + let physical_path = if protected { + path.clone() + } else { + path.with_file_name("agents.30.instance.no-git.json") + }; + assert_eq!( + disk(&physical_path)["codingAgentProfiles"]["defaultProfileByAgent"]["claude"], + json!("A") + ); + let mut candidate = loaded.clone(); + assert_eq!( + candidate + .coding_agent_profiles + .default_profile_by_agent + .remove("claude"), + Some("A".to_string()) + ); + let saved = + save_settings_to_path_preserving_project_paths(&candidate, &path).unwrap(); + let profiles = disk(&physical_path)["codingAgentProfiles"].clone(); + if protected { + assert_eq!(profiles, expected_raw); + assert_eq!(profiles["defaultProfileByAgent"]["claude"], json!("A")); + assert_eq!( + saved + .coding_agent_profiles + .default_profile_by_agent + .get("claude"), + Some(&"A".to_string()) + ); + } else { + assert!( + profiles["defaultProfileByAgent"].get("claude").is_none(), + "the editable removal must persist" + ); + assert!(!saved + .coding_agent_profiles + .default_profile_by_agent + .contains_key("claude")); + } + assert!(profiles["profilesByAgent"].get("scratch-agent").is_none()); + assert_eq!( + saved.coding_agent_profiles, + load_settings_from_path(&path).coding_agent_profiles + ); + } } } diff --git a/src-tauri/src/config/settings.rs b/src-tauri/src/config/settings.rs index c583bbf45..1486a3a83 100644 --- a/src-tauri/src/config/settings.rs +++ b/src-tauri/src/config/settings.rs @@ -24788,14 +24788,27 @@ mod tests { } // S6 never seeded profile cells. Their generated defaults are fresh // canonical rows and must not acquire invented historical fields. - for cells in object["codingAgentProfiles"]["profilesByAgent"] + // Independent ASCII preimages: empty cell/env means the base text. + let pair_hashes = BTreeMap::from([ + ( + "claude", + "cid1:15a3fc2b7f142361454b99d62429610d12f5f7aedb12b8500a2bfa32d379837d", + ), + ( + "codex", + "cid1:2cf586be28b9573af75bfbdadfe0bd8760e3382b547cfe2c527eea368920a8e5", + ), + ]); + for (id, cells) in object["codingAgentProfiles"]["profilesByAgent"] .as_object_mut() .unwrap() - .values_mut() + .iter_mut() { for cell in cells.as_object_mut().unwrap().values_mut() { let command = cell.as_object_mut().unwrap().remove("command").unwrap(); cell["additionalArguments"] = command; + assert!(cell.get("id_hash").is_none()); + cell["id_hash"] = json!(pair_hashes[id.as_str()]); } } let sorted: BTreeMap = @@ -27557,15 +27570,15 @@ mod tests { let settings = super::load_settings_from_path(&path); assert_eq!(order_2306_ids(&settings.agents), ["only-b", "only-a"]); assert_eq!(order_2306_positions(&settings.agents), [Some(0), Some(1)]); - // The fresh-install save writes a base file, but the base never owned - // `agents`, so the save restores the serialized default empty array: no - // overlay record or overlay ordinal may leak into it. + // Whole-group protection preserves physical absence. The effective + // overlay order is independent of materializing a base agents array. let disk = disk_object(&path); - assert_eq!( - disk.get("agents"), - Some(&serde_json::json!([])), - "the absent base restores its default empty agents array: {disk:?}" + assert!( + !disk.contains_key("agents"), + "the absent base agents key must stay absent: {disk:?}" ); + let sidecar = super::agents_instance_path(&path); + assert!(!sidecar.exists() || !disk_object(&sidecar).contains_key("agents")); assert_eq!(std::fs::read(&local_path).unwrap(), overlay_before); } @@ -30501,6 +30514,7 @@ mod tests { fn b3_assert_split_matches_seed(path: &std::path::Path, seeded: &[u8], skip: &[&str]) { let seed: serde_json::Value = serde_json::from_slice(seeded).unwrap(); let mut seed = seed.as_object().unwrap().clone(); + let physical_agents = seed.get("agents").cloned(); let mut disk = b3_object(path); let sidecar = b3_object(&super::agents_instance_path(path)); for key in super::AGENTS_INSTANCE_KEYS { @@ -30550,10 +30564,56 @@ mod tests { if field == "profilesByAgent" { // Compare the complete original cells, including old // command values, plus only the new physical leaf. - for cells in expected.as_object_mut().unwrap().values_mut() { + for (id, cells) in expected.as_object_mut().unwrap().iter_mut() { + // First raw ID wins before decoding, including an + // unresolvable first row. Never use effective/held. + let base = physical_agents + .as_ref() + .and_then(serde_json::Value::as_array) + .and_then(|rows| { + rows.iter().find(|row| { + row.get("id").and_then(serde_json::Value::as_str) + == Some(id.as_str()) + }) + }) + .and_then(|row| { + serde_json::from_value::(row.clone()) + .ok() + }); for cell in cells.as_object_mut().unwrap().values_mut() { assert!(cell.get("additionalArguments").is_none()); cell["additionalArguments"] = cell["command"].clone(); + let profile: super::ProfileCellConfig = + serde_json::from_value(cell.clone()).unwrap(); + let hash = base.as_ref().map(|base| { + // Independent raw-field preimage, every base + // env row (disabled included), then cell env. + let parts = std::iter::once(base.command.as_str()) + .chain(base.envs.iter().flat_map(|env| { + [env.key.as_str(), env.value.as_str()] + })) + .chain(std::iter::once(profile.command.as_str())) + .chain(profile.env.iter().flat_map(|(key, value)| { + [key.as_str(), value.as_str()] + })); + let mut chars: Vec = parts + .flat_map(str::chars) + .flat_map(char::to_lowercase) + .filter(|c| !c.is_whitespace()) + .collect(); + chars.sort_unstable(); + serde_json::Value::String(format!( + "cid1:{:x}", + ::digest( + chars.iter().collect::() + ) + )) + }); + // Replace only expected derived metadata. None + // asserts concrete absence; observed stays raw. + let object = cell.as_object_mut().unwrap(); + object.remove("id_hash"); + object.extend(hash.map(|hash| ("id_hash".to_string(), hash))); } } }