diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index e561e502c7..231691118f 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -138,6 +138,7 @@ export default defineConfig({ "**/where-to-run-config.spec.ts", "**/huddle-transcription.spec.ts", "**/agent-numeric-tuning.spec.ts", + "**/needs-restart-screenshots.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 12e6983eea..2dc0ba0d69 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -12,10 +12,10 @@ use crate::{ RuntimeConfigSurface, SessionConfigCache, }, }, - current_instance_id, is_reserved_env_key, is_well_formed_env_key, known_acp_runtime, - load_managed_agents, load_personas, save_managed_agents, sync_managed_agent_processes, - AgentDefinition, GlobalAgentConfig, KnownAcpRuntime, ManagedAgentRecord, - ManagedAgentRuntimeKey, MAX_ENV_VALUE_BYTES, + current_instance_id, is_reserved_env_key, is_safe_to_reveal, is_well_formed_env_key, + known_acp_runtime, load_managed_agents, load_personas, save_managed_agents, + sync_managed_agent_processes, AgentDefinition, GlobalAgentConfig, KnownAcpRuntime, + ManagedAgentRecord, ManagedAgentRuntimeKey, MAX_ENV_VALUE_BYTES, }, }; @@ -209,27 +209,6 @@ pub struct BakedEnvEntry { pub masked: bool, } -/// Returns `true` when a baked-env key is safe to display unmasked in the UI. -/// -/// This uses an explicit allowlist of keys that are known safe (non-secret). -/// Any key NOT in this set is masked — default-deny for a security surface. -/// -/// Allowlist (case-insensitive): -/// - `BUZZ_AGENT_PROVIDER`, `BUZZ_AGENT_MODEL` — agent runtime selection -/// - `BUZZ_AGENT_THINKING_EFFORT` — non-secret enum (none/minimal/low/medium/high/xhigh/max) -/// - `DATABRICKS_HOST`, `DATABRICKS_MODEL` — Block non-secret defaults -fn is_safe_to_reveal(key: &str) -> bool { - const SAFE_KEYS: &[&str] = &[ - "BUZZ_AGENT_PROVIDER", - "BUZZ_AGENT_MODEL", - "BUZZ_AGENT_THINKING_EFFORT", - "DATABRICKS_HOST", - "DATABRICKS_MODEL", - ]; - let upper = key.to_ascii_uppercase(); - SAFE_KEYS.iter().any(|safe| upper == *safe) -} - /// Expose the baked build env to the frontend with values shown, but any /// key not in the safe-to-reveal allowlist has its value replaced by `••••••`. /// diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index 9bb0f6230d..e06f176216 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -76,7 +76,7 @@ pub(super) fn build_launch_block( policy_env.insert(SESSION_TITLE_ENV_VAR.into(), value); } if let Some(value) = - crate::managed_agents::spawn_hash::effective_team_instructions(record, teams) + crate::managed_agents::spawn_snapshot::effective_team_instructions(record, teams) { policy_env.insert("BUZZ_ACP_TEAM_INSTRUCTIONS".into(), value); } diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 1653371e7f..07705ee998 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -224,6 +224,28 @@ pub fn validate_user_env_keys(env_vars: &BTreeMap) -> Result<(), Ok(()) } +/// Returns `true` when `key` is safe to show verbatim — not a credential. +/// +/// Default-deny: every key NOT in this explicit allowlist is masked. Callers +/// that display env values (baked-env UI, spawn-diff tooltip) share this +/// single authority — no second list. +/// +/// Allowlist (case-insensitive): +/// - `BUZZ_AGENT_PROVIDER`, `BUZZ_AGENT_MODEL` — agent runtime selection +/// - `BUZZ_AGENT_THINKING_EFFORT` — non-secret enum (none/minimal/low/medium/high/xhigh/max) +/// - `DATABRICKS_HOST`, `DATABRICKS_MODEL` — Block non-secret defaults +pub(crate) fn is_safe_to_reveal(key: &str) -> bool { + const SAFE_KEYS: &[&str] = &[ + "BUZZ_AGENT_PROVIDER", + "BUZZ_AGENT_MODEL", + "BUZZ_AGENT_THINKING_EFFORT", + "DATABRICKS_HOST", + "DATABRICKS_MODEL", + ]; + let upper = key.to_ascii_uppercase(); + SAFE_KEYS.iter().any(|safe| upper == *safe) +} + /// Per-value byte cap for env values. 32 KiB is generous for credentials, /// JWT-ish tokens, certs etc., but small enough that a malformed IPC /// caller can't blow up the persona/agent JSON file. Tune up if real diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 772d707f27..a848b6f02f 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -31,7 +31,7 @@ mod runtime; mod runtime_commands; mod runtime_types; pub(crate) mod snapshot_avatar; -pub(crate) mod spawn_hash; +pub(crate) mod spawn_snapshot; pub(crate) mod storage; pub(crate) mod team_events; mod team_repair; diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index 6afc18a501..17b7416b38 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -450,12 +450,12 @@ pub fn persona_snapshot(persona: &AgentDefinition) -> PersonaSnapshot { /// This is the single apply used by every snapshot-apply site: the spawn /// re-pin (`start_local_agent_with_preflight`), the launch backfill and /// restore re-snapshot (`restore.rs`), and the prospective re-snapshot inside -/// `spawn_config_hash` — so a future `PersonaSnapshot` field addition -/// propagates to all of them at once. +/// `prospective_spawn_config_snapshot` — so a future `PersonaSnapshot` field +/// addition propagates to all of them at once. /// /// Deliberately does NOT touch `updated_at`: persistence stamps are the -/// caller's concern, and `spawn_config_hash` (which applies this to a clone) -/// must stay pure. +/// caller's concern, and the prospective snapshot (which applies this to a +/// clone) must stay pure. pub fn apply_persona_snapshot(record: &mut ManagedAgentRecord, persona: &AgentDefinition) { let snapshot = persona_snapshot(persona); if let Some(prompt) = snapshot.system_prompt { @@ -498,8 +498,9 @@ pub fn apply_persona_snapshot(record: &mut ManagedAgentRecord, persona: &AgentDe /// paths re-pin it to its linked persona, without mutating `record` itself. /// /// Every decision made ahead of the real re-pin — the relay-mesh preflight in -/// `start_local_agent_with_preflight`, the restart-badge hash in -/// `spawn_config_hash` — needs to reason about spawn-time state, not +/// `start_local_agent_with_preflight`, the restart-badge snapshot in +/// `prospective_spawn_config_snapshot` — needs to reason about spawn-time +/// state, not /// pre-snapshot bytes, so a persona edit that flips a field (e.g. `provider` /// to/from relay-mesh) between saves is reflected in the decision instead of /// the stale value the real [`apply_persona_snapshot`] is about to overwrite @@ -507,7 +508,7 @@ pub fn apply_persona_snapshot(record: &mut ManagedAgentRecord, persona: &AgentDe /// so the spawn-time stamp and later recomputes agree when nothing changed. /// /// Orphaned records (persona deleted) pass through unchanged: the caller's -/// own orphan handling — refusing to spawn, hashing as `(None, None, None)` +/// own orphan handling — refusing to spawn, snapshotting as `(None, None, None)` /// — runs on the real record downstream, not on this preview. pub fn preview_prospective_persona_snapshot( record: &ManagedAgentRecord, diff --git a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs index 8dddf9f715..479d6ec913 100644 --- a/desktop/src-tauri/src/managed_agents/process_lifecycle.rs +++ b/desktop/src-tauri/src/managed_agents/process_lifecycle.rs @@ -133,7 +133,7 @@ pub fn taskkill_tree(pid: u32) -> Result<(), String> { pub fn finish_spawn( child: std::process::Child, log_path: std::path::PathBuf, - spawn_config_hash: u64, + spawn_config: super::spawn_snapshot::SpawnConfigSnapshot, setup_mode: bool, adapter_availability: Option, start_nonce: String, @@ -149,7 +149,7 @@ pub fn finish_spawn( super::ManagedAgentProcess { child, log_path, - spawn_config_hash, + spawn_config, setup_mode, adapter_availability, start_nonce, diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index 26902ae8de..c072448ff1 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -82,7 +82,7 @@ pub(crate) struct EffectiveAgentEnv { // // A single owned type that fully describes what a spawn would run. Produced // by `resolve_effective_harness_descriptor` and consumed by spawn_agent_child, -// spawn_config_hash, build_managed_agent_summary, get_agent_models, and +// spawn_snapshot, build_managed_agent_summary, get_agent_models, and // agent_readiness — so the harness-definition lookup and arg/env resolution // happen exactly once, in one place. diff --git a/desktop/src-tauri/src/managed_agents/restore.rs b/desktop/src-tauri/src/managed_agents/restore.rs index 1910620159..25dadbeec6 100644 --- a/desktop/src-tauri/src/managed_agents/restore.rs +++ b/desktop/src-tauri/src/managed_agents/restore.rs @@ -18,7 +18,9 @@ use tauri::Manager; /// restore would kill reconcile's lazy child by its receipt and replace it with /// an eager one, flipping the pair's laziness on a startup race. enum SpawnOutcome { - Spawned(super::ManagedAgentRuntimeKey, ManagedAgentProcess), + /// Boxed: the spawned process carries its full spawn-config snapshot, so an + /// inline variant would make every `Skipped`/`Failed` outcome pay for it. + Spawned(super::ManagedAgentRuntimeKey, Box), Skipped, Failed(String), } @@ -338,7 +340,9 @@ pub async fn restore_managed_agents_on_launch( owner_hex_ref, ) }) { - Ok(process) => SpawnOutcome::Spawned(key, process), + Ok(process) => { + SpawnOutcome::Spawned(key, Box::new(process)) + } Err(error) => SpawnOutcome::Failed(error), } } @@ -400,7 +404,7 @@ pub async fn restore_managed_agents_on_launch( record.last_stopped_at = None; record.last_exit_code = None; record.last_error = None; - runtimes.insert(key, super::ManagedAgentPairRuntime::starting(process)); + runtimes.insert(key, super::ManagedAgentPairRuntime::starting(*process)); successfully_spawned.push(pubkey); } SpawnOutcome::Failed(error) => { diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 3173126b90..4041a4fd94 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -225,49 +225,50 @@ pub fn build_managed_agent_summary( } }; - // Restart badge: the running process stamped its effective spawn config - // at launch; recompute from current disk state and flag drift. Only the - // tracked live pair for THIS workspace can drift — stopped agents spawn - // fresh, adopted (runtime_pid-only) processes have no stamped hash to - // compare, and pairs running for other communities are judged in their - // own community (hashing them against this workspace's relay would flag - // a spurious restart on every community switch). + // Restart badge: the running process stamped the effective spawn config + // it was launched with; recompute a prospective one from current disk + // state and report every differing field. Only the tracked live pair for + // THIS workspace can drift — stopped agents spawn fresh, adopted + // (runtime_pid-only) processes have no stamp to compare, and pairs running + // for other communities are judged in their own community (comparing them + // against this workspace's relay would flag a spurious restart on every + // community switch). // - // Additionally, for runtimes with an adapter version gate (codex only), - // check whether the cached adapter availability has drifted from the value - // stamped at spawn. This catches out-of-band adapter changes (manual - // npm install/downgrade) that Phase-1 auto-restart doesn't cover. The - // cache is read-only here — no subprocess is spawned. + // Adapter-availability drift (codex only) contributes its own synthetic + // entry, so an out-of-band adapter change (manual npm install/downgrade) + // that Phase-1 auto-restart doesn't cover still shows the user what moved. + // The cache is read-only here — no subprocess is spawned. // - // Global config drives both the restart-drift hash and descriptor env - // layering below — the caller loads it once and passes it in, so + // Global config drives both the prospective snapshot and the descriptor + // env layering below — the caller loads it once and passes it in, so // list-style callers pay one disk read per call rather than one per record. - let needs_restart = pair_key - .as_ref() - .and_then(|key| runtimes.get(key).map(|runtime| (key, runtime))) - .is_some_and(|(key, runtime)| { - let teams_for_hash = crate::managed_agents::load_teams(app).unwrap_or_default(); - let hash_drift = runtime.spawn_config_hash - != crate::managed_agents::spawn_hash::spawn_config_hash( - record, - personas, - &teams_for_hash, - &key.relay_url, - global_config, - ); - let availability_drift = super::availability_drift( - runtime.adapter_availability.as_ref(), - super::adapter_availability_cached(), - ); - // An orphan can never be restarted successfully — - // `spawn_agent_child` refuses it before any process side effect — - // so `needs_restart` must never fire for one regardless of hash or - // availability drift. Surfacing "Restart required" here would offer - // an action guaranteed to fail; the UI shows `persona_orphaned` - // instead (see `ManagedAgentSummary::persona_orphaned`). - restart_eligible(persona_orphaned, hash_drift, availability_drift) - }); + // The prospective side is computed only for a tracked pair: it costs a + // teams-store read, and an unstamped agent has nothing to compare against. + let tracked_spawn = pair_key.as_ref().zip(pair_runtime).map(|(key, runtime)| { + let teams = crate::managed_agents::load_teams(app).unwrap_or_default(); + let current = crate::managed_agents::spawn_snapshot::prospective_spawn_config_snapshot( + record, + personas, + &teams, + &key.relay_url, + global_config, + ); + (runtime, current) + }); + let restart_diff = crate::managed_agents::spawn_snapshot::eligible_restart_diff( + persona_orphaned, + tracked_spawn.as_ref().map(|(runtime, current)| { + crate::managed_agents::spawn_snapshot::TrackedSpawnState { + stamped: &runtime.spawn_config, + current, + stamped_availability: runtime.adapter_availability.as_ref(), + current_availability: super::adapter_availability_cached(), + } + }), + ); + // One vector is the whole truth: badge on ⟺ there is a diff to show. + let needs_restart = !restart_diff.is_empty(); // Resolve the effective harness via the single typed descriptor — same resolver // as spawn, so the UI reflects the persona's current harness (or explicit pin). @@ -320,6 +321,7 @@ pub fn build_managed_agent_summary( persona_out_of_date, persona_orphaned, needs_restart, + restart_diff, env_vars: record.env_vars.clone(), backend: record.backend.clone(), backend_agent_id: record.backend_agent_id.clone(), @@ -340,19 +342,6 @@ pub fn build_managed_agent_summary( }) } -/// Pure predicate: should the "Restart required" badge fire? -/// -/// An orphaned linked instance (its persona/definition no longer exists) -/// can never be restarted successfully — `spawn_agent_child` refuses to -/// spawn it before any process side effect. Surfacing "Restart required" -/// for one would offer an action guaranteed to fail, so this always -/// returns `false` for an orphan regardless of drift. Extracted for unit -/// testing without `AppHandle`/global state, following the -/// `availability_drift` pattern in `discovery.rs`. -fn restart_eligible(persona_orphaned: bool, hash_drift: bool, availability_drift: bool) -> bool { - !persona_orphaned && (hash_drift || availability_drift) -} - pub fn find_managed_agent_mut<'a>( records: &'a mut [ManagedAgentRecord], pubkey: &str, @@ -473,7 +462,7 @@ pub fn spawn_agent_child( let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default(); // Resolve model/provider/prompt ONCE, here, at the shared spawn boundary — - // the single source both the env writes below and `spawn_config_hash` + // the single source both the env writes below and the spawn-config snapshot // read from. Previously prompt was read from the record's own (possibly // stale, Phase-A-snapshot) bytes while model/provider were resolved live // from `personas`; a definition edit landing between a caller's snapshot @@ -490,8 +479,9 @@ pub fn spawn_agent_child( // Single typed resolver: validates runtime id (dangling harness → Err), resolves // command, args (instance wins over definition default), and the full env layer stack. - // This is the sole path for harness-definition lookup — spawn, hash, summary, and - // model probes all consume this descriptor rather than assembling values inline. + // This is the sole path for harness-definition lookup — spawn, snapshot, + // summary, and model probes all consume this descriptor rather than + // assembling values inline. // Like the orphan refusal above, this runs before any side effect so a refused // spawn leaves no trace. let descriptor = @@ -735,7 +725,7 @@ pub fn spawn_agent_child( } } } - let team_instructions = super::spawn_hash::effective_team_instructions(record, &teams); + let team_instructions = super::spawn_snapshot::effective_team_instructions(record, &teams); if let Some(instructions) = &team_instructions { command.env("BUZZ_ACP_TEAM_INSTRUCTIONS", instructions); } else { @@ -743,8 +733,8 @@ pub fn spawn_agent_child( } // Prompt, model, and provider all come from the single `effective_cfg` - // resolved at the top of this function — the SAME resolve `spawn_config_hash` - // performs below, so env write and restart badge cannot disagree. Linked + // resolved at the top of this function — the SAME resolve the spawn-config + // snapshot reads, so env write and restart badge cannot disagree. Linked // instances never consult the record's own model/provider/prompt bytes; // definition-less instances fall back to their own fields, then global. // @@ -770,8 +760,9 @@ pub fn spawn_agent_child( } // Session title for the harness to pass out-of-band on `session/new`. The // adapter names the session after it; it never reaches the prompt, so this - // is display metadata only. `spawn_config_hash` hashes the same resolve, so - // a rename raises the restart badge instead of leaving the process stale. + // is display metadata only. The spawn-config snapshot records the same + // resolve, so a rename raises the restart badge instead of leaving the + // process stale. if let Some(title) = resolve_session_title(record.display_name.as_deref(), &record.name) { command.env(SESSION_TITLE_ENV_VAR, title); } else { @@ -881,6 +872,22 @@ pub fn spawn_agent_child( .env("BUZZ_MANAGED_AGENT", current_instance_id(app)) .env("BUZZ_MANAGED_AGENT_START_NONCE", &start_nonce); + // Stamp the effective spawn config from the values that populated the + // `Command` above, BEFORE spawning. Re-resolving after `spawn()` would let + // a persona/harness/global edit landing in between stamp the NEW config + // onto a child running the OLD one, silently suppressing the badge. + let spawn_config = super::spawn_snapshot::SpawnConfigSnapshot::from_inputs( + super::spawn_snapshot::SpawnConfigInputs { + record, + descriptor: &descriptor, + relay_url: &effective_relay_url, + team_instructions: team_instructions.as_deref(), + system_prompt: effective_prompt.as_deref(), + model: effective_model.as_deref(), + provider: effective_provider.as_deref(), + }, + ); + // Spawn the harness in its own process group so we can kill the entire // tree (harness + MCP servers + agent subprocesses) on shutdown. #[cfg(unix)] @@ -906,18 +913,6 @@ pub fn spawn_agent_child( ) })?; - // Stamp the effective spawn config so the summary builder can flag - // needs_restart when disk state drifts from what this process runs. - // `effective_relay_url` is already resolved, and resolution is idempotent, - // so it serves as the workspace-relay input here. - let spawn_config_hash = super::spawn_hash::spawn_config_hash( - record, - &personas, - &teams, - &effective_relay_url, - &global, - ); - // Stamp the adapter availability for runtimes with a version gate (codex // only). The summary builder compares this against the current cached value // to detect out-of-band adapter changes after spawn (Phase-2 badge fallback). @@ -940,7 +935,7 @@ pub fn spawn_agent_child( return Ok(super::process_lifecycle::finish_spawn( child, log_path, - spawn_config_hash, + spawn_config, spawned_setup_mode, spawned_adapter_availability, start_nonce, @@ -950,7 +945,7 @@ pub fn spawn_agent_child( Ok(crate::managed_agents::ManagedAgentProcess { child, log_path, - spawn_config_hash, + spawn_config, setup_mode: spawned_setup_mode, adapter_availability: spawned_adapter_availability, start_nonce, diff --git a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs index 96ac73e347..26d210e5c6 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs @@ -25,7 +25,7 @@ pub(crate) fn runtime_metadata_env_vars<'a>( } /// Env var carrying the session title to the harness. Shared with -/// `spawn_hash` so the restart badge hashes the same key the spawn writes. +/// `spawn_snapshot` so the restart badge records the same key the spawn writes. pub(crate) const SESSION_TITLE_ENV_VAR: &str = "BUZZ_ACP_SESSION_TITLE"; /// Resolve the session title for an agent: its `display_name` when it has one, diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 3f6ee996f6..bea4b1c3e3 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -1271,7 +1271,13 @@ fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRun let process = crate::managed_agents::ManagedAgentProcess { child, log_path: std::path::PathBuf::new(), - spawn_config_hash: 0, + spawn_config: crate::managed_agents::spawn_snapshot::prospective_spawn_config_snapshot( + &minimal_record(&"cc".repeat(32)), + &[], + &[], + "wss://relay.example", + &Default::default(), + ), setup_mode: false, adapter_availability: None, start_nonce: "test-nonce".to_string(), @@ -1280,37 +1286,3 @@ fn make_pair_runtime_placeholder() -> crate::managed_agents::ManagedAgentPairRun }; crate::managed_agents::ManagedAgentPairRuntime::starting(process) } - -// ── restart_eligible tests ────────────────────────────────────────────── - -#[test] -fn restart_eligible_true_when_non_orphan_has_hash_drift() { - assert!(super::restart_eligible(false, true, false)); -} - -#[test] -fn restart_eligible_true_when_non_orphan_has_availability_drift() { - assert!(super::restart_eligible(false, false, true)); -} - -#[test] -fn restart_eligible_false_when_orphan_has_hash_drift() { - // An orphan can never be restarted successfully — spawn refuses it — - // so hash drift alone must not surface "Restart required". - assert!(!super::restart_eligible(true, true, false)); -} - -#[test] -fn restart_eligible_false_when_orphan_has_availability_drift() { - assert!(!super::restart_eligible(true, false, true)); -} - -#[test] -fn restart_eligible_false_when_orphan_has_no_drift() { - assert!(!super::restart_eligible(true, false, false)); -} - -#[test] -fn restart_eligible_false_when_non_orphan_has_no_drift() { - assert!(!super::restart_eligible(false, false, false)); -} diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs deleted file mode 100644 index 648cc62bbe..0000000000 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ /dev/null @@ -1,160 +0,0 @@ -//! Spawn-time config hash for the restart-required badge. -//! -//! [`spawn_config_hash`] digests the *effective spawned values* — what a -//! process launch of `record` would actually receive — so the UI can compare -//! a running process's hash (stamped on [`super::ManagedAgentProcess`] at -//! spawn) against a recomputation from current disk state and show a -//! "restart required" badge only when a restart would change what runs. -//! -//! Scope rules (decided in #centralize-personas-and-agents, revised in PR -//! #1602 review): -//! - Inputs mirror what a start would actually run: the start/restore paths -//! re-snapshot the linked persona's prompt/model/provider/env onto the -//! record immediately before spawning (`start_local_agent_with_preflight`, -//! `restore_managed_agents_on_launch`), so persona edits to those fields DO -//! apply on a plain restart and are hashed via the same prospective -//! re-snapshot. Harness command, args/mcp, env layering, and the record -//! fields the spawn env writes read are hashed as spawn resolves them. -//! - The relay URL is hashed in resolved form (`effective_agent_relay_url`): -//! every record spawns against the active workspace relay (legacy per-record -//! pins are ignored), so a workspace relay change means a restart would -//! change what runs. -//! - Channel membership is not an input: agents pick up channel changes live -//! (#1468), never via restart. -//! -//! The hash never crosses a process or persistence boundary, so -//! `DefaultHasher` (not stable across Rust releases) is sufficient. - -use std::hash::{DefaultHasher, Hash, Hasher}; - -use super::{ - effective_config::{resolve_effective_config, EffectiveConfigResult}, - known_acp_runtime, normalize_agent_args, - persona_events::preview_prospective_persona_snapshot, - runtime::{resolve_session_title, SESSION_TITLE_ENV_VAR}, - types::{AgentDefinition, ManagedAgentRecord, TeamRecord}, - GlobalAgentConfig, -}; - -/// Resolve the current instructions for this instance's deployment-time team binding. -/// A deleted team deliberately degrades to no team section. -pub(crate) fn effective_team_instructions( - record: &ManagedAgentRecord, - teams: &[TeamRecord], -) -> Option { - teams - .iter() - .find(|team| Some(team.id.as_str()) == record.team_id.as_deref()) - .and_then(|team| team.instructions.as_deref()) - .map(str::trim) - .filter(|instructions| !instructions.is_empty()) - .map(str::to_string) -} - -/// Digest the effective spawn configuration of `record` under the current -/// `personas`, resolving a blank record relay against `workspace_relay`. -/// Pure — no `AppHandle`, no disk, no keyring. -pub(crate) fn spawn_config_hash( - record: &ManagedAgentRecord, - personas: &[AgentDefinition], - teams: &[TeamRecord], - workspace_relay: &str, - global: &GlobalAgentConfig, -) -> u64 { - // Prospective re-snapshot: apply the same `apply_persona_snapshot` the - // start/restore paths run right before spawning, so the hash covers what a - // restart would actually run. Idempotent, so the spawn-time stamp - // (post-snapshot record) and later recomputes (persisted record) agree - // when nothing changed. The persona env itself reaches the hash through - // the descriptor's layered env below; `persona_source_version` is set on - // the clone but is not a hash input. - let record = preview_prospective_persona_snapshot(record, personas); - let record = &record; - - // Resolve command, args, and env via the single typed descriptor — same path - // as spawn_agent_child. Dangling harness id falls back to the infallible - // record_agent_command (no-op: a dangling harness can't be spawned, so the - // hash never matters for that agent). - let descriptor = - crate::managed_agents::resolve_effective_harness_descriptor(record, personas, global) - .unwrap_or_else(|_| { - let cmd = crate::managed_agents::record_agent_command(record, personas); - let args = normalize_agent_args(&cmd, record.agent_args.clone()); - crate::managed_agents::readiness::EffectiveHarnessDescriptor { - command: cmd, - args, - env: Default::default(), - } - }); - let runtime_meta = known_acp_runtime(&descriptor.command); - - let mut hasher = DefaultHasher::new(); - - // Harness identity and derivations (live-persona-resolved, like spawn). - record.acp_command.hash(&mut hasher); - descriptor.command.hash(&mut hasher); - descriptor.args.hash(&mut hasher); - runtime_meta - .and_then(|r| r.mcp_command) - .unwrap_or("") - .hash(&mut hasher); - - // Effective env layering (baked floor → runtime metadata → definition env - // → global → persona → agent). BTreeMap iteration is ordered, deterministic. - descriptor.env.hash(&mut hasher); - - // Record fields the spawn env writes read directly. The relay is hashed - // resolved: every record spawns on the workspace relay (legacy pins - // ignored), so a workspace relay change must trip the badge. - crate::relay::effective_agent_relay_url(&record.relay_url, workspace_relay).hash(&mut hasher); - // Team instructions use the same resolver as spawn. - effective_team_instructions(record, teams).hash(&mut hasher); - // Prompt, model, and provider all come from ONE `resolve_effective_config` - // call — the SAME resolve `spawn_agent_child` performs for the env write, - // so env write and this badge cannot disagree. An orphaned link (missing - // definition) hashes as if all three were absent: `spawn_agent_child` - // refuses to spawn an orphan regardless, so this is a display-only - // convenience, not the spawn gate. - let (resolved_prompt, resolved_model, resolved_provider) = - match resolve_effective_config(record, personas, global) { - EffectiveConfigResult::Resolved(cfg) => { - (cfg.system_prompt.value, cfg.model.value, cfg.provider.value) - } - EffectiveConfigResult::OrphanedInstance { .. } => (None, None, None), - }; - resolved_prompt.hash(&mut hasher); - resolved_model.hash(&mut hasher); - resolved_provider.hash(&mut hasher); - // Session title: the same resolve `spawn_agent_child` performs for its env - // write, so a rename raises the restart badge. Skipped when a user env - // override shadows it — spawn writes the title BEFORE the user env layer, - // so the override is what actually runs, and it already reaches this hash - // through `descriptor.env` above. Hashing the record-derived value under an - // override would badge a rename that changes nothing. - let effective_session_title = (!descriptor.env.contains_key(SESSION_TITLE_ENV_VAR)) - .then(|| resolve_session_title(record.display_name.as_deref(), &record.name)) - .flatten(); - effective_session_title.hash(&mut hasher); - record.auth_tag.hash(&mut hasher); - record.respond_to.as_str().hash(&mut hasher); - // The allowlist is hashed as the env receives it: spawn sets - // BUZZ_ACP_RESPOND_TO_ALLOWLIST only in allowlist mode, and normalized - // (trim/lowercase/dedup via `validate_respond_to_allowlist`) — so edits - // that don't survive normalization, or edits while another mode is - // active, must not badge. A list spawn would reject hashes raw: the - // stamped hash comes from a successful spawn, so any invalid edit - // correctly compares unequal. - if record.respond_to == super::types::RespondTo::Allowlist { - super::types::validate_respond_to_allowlist(&record.respond_to_allowlist) - .unwrap_or_else(|_| record.respond_to_allowlist.clone()) - .hash(&mut hasher); - } - record.idle_timeout_seconds.hash(&mut hasher); - record.max_turn_duration_seconds.hash(&mut hasher); - record.parallelism.hash(&mut hasher); - - hasher.finish() -} - -#[cfg(test)] -mod tests; diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs new file mode 100644 index 0000000000..73a006e70f --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs @@ -0,0 +1,263 @@ +//! Spawn-time config snapshot for the restart-required badge. +//! +//! [`SpawnConfigSnapshot`] captures the *effective spawned values* — what a +//! process launch of a record would actually receive. The running process +//! stamps one on [`super::ManagedAgentProcess`] at spawn; the summary builder +//! recomputes a prospective one from current disk state and compares. Drift +//! means a restart would change what runs, and the field-by-field difference +//! is what the UI shows (see [`diff`]). +//! +//! Scope rules (decided in #centralize-personas-and-agents, revised in PR +//! #1602 review): +//! - Inputs mirror what a start would actually run: the start/restore paths +//! re-snapshot the linked persona's prompt/model/provider/env onto the +//! record immediately before spawning (`start_local_agent_with_preflight`, +//! `restore_managed_agents_on_launch`), so persona edits to those fields DO +//! apply on a plain restart and reach the prospective snapshot via the same +//! re-snapshot. Harness command, args/mcp, env layering, and the record +//! fields the spawn env writes read are captured as spawn resolves them. +//! - The relay URL is captured in resolved form (`effective_agent_relay_url`): +//! every record spawns against the active workspace relay (legacy per-record +//! pins are ignored), so a workspace relay change means a restart would +//! change what runs. +//! - Channel membership is not an input: agents pick up channel changes live +//! (#1468), never via restart. +//! +//! The snapshot never crosses a process or persistence boundary — it is +//! runtime state only, held on the running `ManagedAgentProcess`. + +use std::collections::BTreeMap; + +use serde::Serialize; + +use super::{ + effective_config::{resolve_effective_config, EffectiveConfigResult}, + known_acp_runtime, normalize_agent_args, + persona_events::preview_prospective_persona_snapshot, + readiness::EffectiveHarnessDescriptor, + runtime::{resolve_session_title, SESSION_TITLE_ENV_VAR}, + types::{AgentDefinition, ManagedAgentRecord, TeamRecord}, + GlobalAgentConfig, +}; + +pub(crate) mod diff; +pub(crate) use diff::{eligible_restart_diff, RestartDiffEntry, TrackedSpawnState}; + +/// Resolve the current instructions for this instance's deployment-time team binding. +/// A deleted team deliberately degrades to no team section. +pub(crate) fn effective_team_instructions( + record: &ManagedAgentRecord, + teams: &[TeamRecord], +) -> Option { + teams + .iter() + .find(|team| Some(team.id.as_str()) == record.team_id.as_deref()) + .and_then(|team| team.instructions.as_deref()) + .map(str::trim) + .filter(|instructions| !instructions.is_empty()) + .map(str::to_string) +} + +/// The already-resolved values a spawn feeds into its `Command`. +/// +/// Taking them rather than re-resolving is what makes the stamp describe the +/// process that was actually launched: a persona/harness/global edit landing +/// between spawn's resolution and the stamp can no longer suppress the badge. +pub(crate) struct SpawnConfigInputs<'a> { + pub record: &'a ManagedAgentRecord, + pub descriptor: &'a EffectiveHarnessDescriptor, + /// Resolved workspace/pair relay — never the record's legacy pin. + pub relay_url: &'a str, + pub team_instructions: Option<&'a str>, + pub system_prompt: Option<&'a str>, + pub model: Option<&'a str>, + pub provider: Option<&'a str>, +} + +/// The effective spawn configuration of one managed-agent process. +/// +/// Serialization invariants (load-bearing — the drift comparison and the diff +/// walk both read `canonical()`): +/// - plain derived `Serialize`: no `flatten`, no `skip_serializing_if`, no +/// custom or fallible field serializers, no colliding serialized names, so +/// every field is always present on both sides of a comparison; +/// - `Option::None` serializes as JSON `null`; a *missing* key is reserved for +/// dynamic-map membership (`env.` added/removed); +/// - arrays are atomic leaves — `args` and `respond_to_allowlist` compare and +/// render whole, never element-wise. +/// +/// `Debug` is implemented by hand: [`ManagedAgentProcess`] derives `Debug`, so +/// a derived impl here would print env values, auth tags, and CLI arguments. +/// +/// [`ManagedAgentProcess`]: super::ManagedAgentProcess +#[derive(Clone, Serialize)] +pub(crate) struct SpawnConfigSnapshot { + /// The ACP harness binary the desktop launches (`buzz-acp`). + pub acp_command: String, + /// The effective agent command the harness drives. + pub command: String, + pub args: Vec, + /// Catalog-derived from `command`; `""` when the runtime has none. + pub mcp_command: String, + /// Fully layered process env: baked floor -> runtime metadata -> + /// definition -> global -> persona -> agent. + pub env: BTreeMap, + pub relay_url: String, + pub team_instructions: Option, + pub system_prompt: Option, + pub model: Option, + pub provider: Option, + /// `None` when a user env override shadows `BUZZ_ACP_SESSION_TITLE`: spawn + /// writes the title BEFORE the user env layer, so the override is what + /// actually runs and it already reaches this snapshot through `env`. + /// Capturing the record-derived value under an override would badge a + /// rename that changes nothing. + pub session_title: Option, + pub auth_tag: Option, + pub respond_to: String, + /// `None` outside allowlist mode — spawn sets + /// `BUZZ_ACP_RESPOND_TO_ALLOWLIST` only there, so edits to a dormant list + /// must not badge. Normalized (trim/lowercase/dedup) as the env receives + /// it, so edits that don't survive normalization must not badge either. + pub respond_to_allowlist: Option>, + pub idle_timeout_seconds: Option, + pub max_turn_duration_seconds: Option, + pub parallelism: u32, +} + +impl SpawnConfigSnapshot { + /// Assemble the snapshot from values a spawn has already resolved. + pub(crate) fn from_inputs(inputs: SpawnConfigInputs<'_>) -> Self { + let SpawnConfigInputs { + record, + descriptor, + relay_url, + team_instructions, + system_prompt, + model, + provider, + } = inputs; + Self { + acp_command: record.acp_command.clone(), + command: descriptor.command.clone(), + args: descriptor.args.clone(), + mcp_command: known_acp_runtime(&descriptor.command) + .and_then(|runtime| runtime.mcp_command) + .unwrap_or("") + .to_string(), + env: descriptor.env.clone(), + relay_url: relay_url.to_string(), + team_instructions: team_instructions.map(str::to_string), + system_prompt: system_prompt.map(str::to_string), + model: model.map(str::to_string), + provider: provider.map(str::to_string), + session_title: (!descriptor.env.contains_key(SESSION_TITLE_ENV_VAR)) + .then(|| resolve_session_title(record.display_name.as_deref(), &record.name)) + .flatten(), + auth_tag: record.auth_tag.clone(), + respond_to: record.respond_to.as_str().to_string(), + respond_to_allowlist: (record.respond_to == super::types::RespondTo::Allowlist).then( + || { + // A list spawn would reject is captured raw: the stamped + // snapshot comes from a successful spawn, so any invalid + // edit correctly compares unequal. + super::types::validate_respond_to_allowlist(&record.respond_to_allowlist) + .unwrap_or_else(|_| record.respond_to_allowlist.clone()) + }, + ), + idle_timeout_seconds: record.idle_timeout_seconds, + max_turn_duration_seconds: record.max_turn_duration_seconds, + parallelism: record.parallelism, + } + } + + /// Canonical JSON projection — the single representation both the drift + /// comparison and the diff walk read, so a lit badge always has a + /// non-empty diff and vice versa. + /// + /// Infallible by the serialization invariants documented on the struct + /// (plain derive over strings, scalars, string maps, and string vectors); + /// a failure here is a broken invariant, never a runtime condition, so it + /// must not degrade into an empty diff. + pub(crate) fn canonical(&self) -> serde_json::Value { + serde_json::to_value(self).expect("SpawnConfigSnapshot serializes infallibly") + } +} + +impl std::fmt::Debug for SpawnConfigSnapshot { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "SpawnConfigSnapshot({})", + diff::redacted_canonical(&self.canonical()) + ) + } +} + +/// Snapshot the effective spawn configuration `record` would get if it were +/// started right now under the current `personas`/`teams`/`global`, resolving +/// a blank record relay against `workspace_relay`. +/// +/// Pure — no `AppHandle`, no disk, no keyring. This is the *prospective* side +/// of the comparison; the stamped side is built at spawn from the values that +/// actually fed the child's `Command`. +pub(crate) fn prospective_spawn_config_snapshot( + record: &ManagedAgentRecord, + personas: &[AgentDefinition], + teams: &[TeamRecord], + workspace_relay: &str, + global: &GlobalAgentConfig, +) -> SpawnConfigSnapshot { + // Prospective re-snapshot: apply the same `apply_persona_snapshot` the + // start/restore paths run right before spawning, so this describes what a + // restart would actually run. Idempotent, so a spawn-time stamp taken + // after those paths saved the record compares equal when nothing changed. + // The persona env itself arrives through the descriptor's layered env + // below; `persona_source_version` is set on the clone but is not an input. + let record = preview_prospective_persona_snapshot(record, personas); + let record = &record; + + // Resolve command, args, and env via the single typed descriptor — same + // path as spawn_agent_child. Dangling harness id falls back to the + // infallible record_agent_command (no-op: a dangling harness can't be + // spawned, so the snapshot never matters for that agent). + let descriptor = + crate::managed_agents::resolve_effective_harness_descriptor(record, personas, global) + .unwrap_or_else(|_| { + let command = crate::managed_agents::record_agent_command(record, personas); + let args = normalize_agent_args(&command, record.agent_args.clone()); + EffectiveHarnessDescriptor { + command, + args, + env: Default::default(), + } + }); + + // Prompt, model, and provider all come from ONE `resolve_effective_config` + // call — the SAME resolve `spawn_agent_child` performs for the env write, + // so env write and this badge cannot disagree. An orphaned link (missing + // definition) resolves as if all three were absent: `spawn_agent_child` + // refuses to spawn an orphan regardless, and `eligible_restart_diff` + // suppresses the badge for one. + let (prompt, model, provider) = match resolve_effective_config(record, personas, global) { + EffectiveConfigResult::Resolved(cfg) => { + (cfg.system_prompt.value, cfg.model.value, cfg.provider.value) + } + EffectiveConfigResult::OrphanedInstance { .. } => (None, None, None), + }; + + SpawnConfigSnapshot::from_inputs(SpawnConfigInputs { + record, + descriptor: &descriptor, + // Resolved, not stored: every record spawns on the workspace relay + // (legacy pins ignored), so a workspace relay change must badge. + relay_url: &crate::relay::effective_agent_relay_url(&record.relay_url, workspace_relay), + team_instructions: effective_team_instructions(record, teams).as_deref(), + system_prompt: prompt.as_deref(), + model: model.as_deref(), + provider: provider.as_deref(), + }) +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff.rs new file mode 100644 index 0000000000..a61eb92e2e --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff.rs @@ -0,0 +1,307 @@ +//! Redacted field-by-field diff of two [`SpawnConfigSnapshot`]s. +//! +//! The walk is generic over the snapshot's canonical JSON: it compares leaves +//! by path and emits one entry per inequality. Adding a field to +//! [`SpawnConfigSnapshot`] therefore reaches the UI with no change here — the +//! only per-path knowledge in this module is [`policy_for`], which decides how +//! a leaf may be *shown*, never which leaves are compared. +//! +//! Raw values drive comparison; redaction happens strictly afterwards, when +//! the serializable entry is built. Comparing masked forms would let two +//! secrets with colliding suffixes read as "no drift". + +use serde::Serialize; +use serde_json::{Map, Value}; + +use super::SpawnConfigSnapshot; +use crate::managed_agents::AcpAvailabilityStatus; + +/// Synthetic field id for adapter-availability drift, which lives outside the +/// snapshot: it describes the environment around the process, not the config +/// the process was spawned with. +const ADAPTER_AVAILABILITY_FIELD: &str = "adapter_availability"; + +const MASK: &str = "••••"; + +/// One changed field. `field` is a dotted path built from serde field names, +/// with dynamic map keys appended verbatim (`env.OPENAI_API_KEY`). The UI +/// humanizes it generically and must never switch on its value. +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct RestartDiffEntry { + pub field: String, + pub change: RestartChange, +} + +/// How a changed field is presented. The UI switches on `kind` — a closed set +/// — and renders any `field` path. +#[derive(Debug, Clone, PartialEq, Serialize)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum RestartChange { + /// Safe scalar or array shown verbatim. `null` means absent. + Value { before: Value, after: Value }, + /// Large text shown as character counts only. `null` means absent. + Text { + before_chars: Option, + after_chars: Option, + }, + /// Secret-bearing leaf. `null` means absent. + Masked { + before: Option, + after: Option, + }, + /// Dynamic-map key present only on the new side. No payload — the value + /// would be secret-bearing and the key name alone is the useful signal. + Added, + /// Dynamic-map key present only on the old side. + Removed, +} + +/// How a leaf at `path` may be displayed. +#[derive(Clone, Copy, PartialEq)] +enum MaskPolicy { + /// Shown verbatim. + Plain, + /// Character counts only. + Text, + /// `••••` plus the last four characters when longer than eight. + MaskedSuffix, + /// `••••` and nothing else. + MaskedBare, +} + +/// The single redaction authority: the wire diff and the snapshot's `Debug` +/// both route every leaf through this. +/// +/// A new snapshot field needs an arm here only if it can carry a credential or +/// is too large to render; everything else falls through to `Plain`. +fn policy_for(path: &str) -> MaskPolicy { + match path { + // Arbitrary user text — a rendered before/after would be unbounded as + // well as unreadable. + "system_prompt" | "team_instructions" => MaskPolicy::Text, + // Arbitrary CLI arguments: `--token=...` is legal, so no part of the + // value may be disclosed. Same for the relay URL — `normalize_relay_url` + // rejects userinfo but deliberately preserves query strings, so + // `wss://relay.example/ws?token=...` is a valid value. + "args" | "relay_url" => MaskPolicy::MaskedBare, + // NIP-OA auth tag: a credential, but a suffix tells the user which tag + // they are looking at. + "auth_tag" => MaskPolicy::MaskedSuffix, + // Env values: consult the shared allowlist. Allowlisted keys (e.g. + // `BUZZ_AGENT_THINKING_EFFORT`) render plain so the user sees the + // actual enum values; every other env key stays masked. + _ if path.starts_with("env.") => { + let key = &path[4..]; + if crate::managed_agents::is_safe_to_reveal(key) { + MaskPolicy::Plain + } else { + MaskPolicy::MaskedSuffix + } + } + // Plain arm. Every path reaching it is already rendered verbatim in + // the runtime UI today: + // acp_command / command / mcp_command — resolved binary names + // session_title — display chrome + // model / provider — catalog ids + // respond_to / respond_to_allowlist — gate mode + pubkeys + // idle_timeout_seconds / max_turn_duration_seconds / parallelism + // — numeric limits + // adapter_availability — an enum variant name + _ => MaskPolicy::Plain, + } +} + +/// `••••` plus the last four characters, or a bare `••••` when the value is +/// short enough that a suffix would disclose too much of it. +/// +/// Character-based throughout: byte slicing can panic on a multi-byte value or +/// disclose the wrong suffix. +fn mask(value: &str) -> String { + let chars: Vec = value.chars().collect(); + match chars.len() { + len if len > 8 => format!("{MASK}{}", chars[len - 4..].iter().collect::()), + _ => MASK.to_string(), + } +} + +/// Character count of a text leaf; `None` when the leaf is absent. +fn char_count(value: &Value) -> Option { + match value { + Value::Null => None, + Value::String(text) => Some(text.chars().count()), + // Fail closed on an unexpected shape: count it, never show it. + other => Some(other.to_string().chars().count()), + } +} + +/// Masked rendering of a leaf; `None` when the leaf is absent. +fn masked(policy: MaskPolicy, value: &Value) -> Option { + match (policy, value) { + (_, Value::Null) => None, + (MaskPolicy::MaskedSuffix, Value::String(text)) => Some(mask(text)), + // Fail closed: an unexpected shape under a redacting policy still + // redacts rather than disclosing the raw value. + _ => Some(MASK.to_string()), + } +} + +fn change_for(policy: MaskPolicy, before: &Value, after: &Value) -> RestartChange { + match policy { + MaskPolicy::Plain => RestartChange::Value { + before: before.clone(), + after: after.clone(), + }, + MaskPolicy::Text => RestartChange::Text { + before_chars: char_count(before), + after_chars: char_count(after), + }, + MaskPolicy::MaskedSuffix | MaskPolicy::MaskedBare => RestartChange::Masked { + before: masked(policy, before), + after: masked(policy, after), + }, + } +} + +/// Lexicographically sorted union of both maps' keys, so entry order — and +/// therefore the UI's "first N plus and-N-more" truncation — is stable. +fn key_union<'a>(before: &'a Map, after: &'a Map) -> Vec<&'a str> { + let mut keys: Vec<&str> = before + .keys() + .chain(after.keys()) + .map(String::as_str) + .collect(); + keys.sort_unstable(); + keys.dedup(); + keys +} + +fn child_path(parent: &str, key: &str) -> String { + if parent.is_empty() { + key.to_string() + } else { + format!("{parent}.{key}") + } +} + +fn walk( + path: &str, + before: Option<&Value>, + after: Option<&Value>, + out: &mut Vec, +) { + match (before, after) { + (before, after) if before == after => {} + // Present on one side only. Struct fields are always present (`None` + // serializes as `null`), so this is dynamic-map membership. + (None, Some(_)) => out.push(RestartDiffEntry { + field: path.to_string(), + change: RestartChange::Added, + }), + (Some(_), None) => out.push(RestartDiffEntry { + field: path.to_string(), + change: RestartChange::Removed, + }), + (Some(Value::Object(before)), Some(Value::Object(after))) => { + for key in key_union(before, after) { + walk(&child_path(path, key), before.get(key), after.get(key), out); + } + } + // Everything else is a leaf: scalars, and arrays (atomic — `args` + // changed as a whole, never `args.0`). + (before, after) => out.push(RestartDiffEntry { + field: path.to_string(), + change: change_for( + policy_for(path), + before.unwrap_or(&Value::Null), + after.unwrap_or(&Value::Null), + ), + }), + } +} + +/// The redacted diff of two snapshots, in stable path order. +fn diff(before: &SpawnConfigSnapshot, after: &SpawnConfigSnapshot) -> Vec { + let mut entries = Vec::new(); + walk( + "", + Some(&before.canonical()), + Some(&after.canonical()), + &mut entries, + ); + entries +} + +fn availability_value(status: Option<&AcpAvailabilityStatus>) -> Value { + status + .and_then(|status| serde_json::to_value(status).ok()) + .unwrap_or(Value::Null) +} + +/// What a tracked runtime was launched with, paired with what a launch would +/// use now. Absent (`None` at the call site) for every agent this workspace +/// tracks no live pair for — stopped, or `runtime_pid`-adopted across an app +/// restart, whose spawn config was never stamped and so can never be shown to +/// have drifted. +pub(crate) struct TrackedSpawnState<'a> { + pub stamped: &'a SpawnConfigSnapshot, + pub current: &'a SpawnConfigSnapshot, + pub stamped_availability: Option<&'a AcpAvailabilityStatus>, + pub current_availability: Option, +} + +/// The final restart-diff for one agent — the single source of both the wire +/// field and the badge, which is `!result.is_empty()`. +/// +/// Empty for an un-stamped agent (see [`TrackedSpawnState`]) and for an +/// orphaned instance: `spawn_agent_child` refuses to spawn an orphan before +/// any side effect, so "Restart required" would offer an action guaranteed to +/// fail. The UI surfaces `persona_orphaned` instead. +pub(crate) fn eligible_restart_diff( + persona_orphaned: bool, + tracked: Option>, +) -> Vec { + let Some(tracked) = tracked.filter(|_| !persona_orphaned) else { + return Vec::new(); + }; + let mut entries = diff(tracked.stamped, tracked.current); + if crate::managed_agents::availability_drift( + tracked.stamped_availability, + tracked.current_availability.clone(), + ) { + entries.push(RestartDiffEntry { + field: ADAPTER_AVAILABILITY_FIELD.to_string(), + change: RestartChange::Value { + before: availability_value(tracked.stamped_availability), + after: availability_value(tracked.current_availability.as_ref()), + }, + }); + } + entries +} + +/// The canonical snapshot with every leaf passed through [`policy_for`], +/// rendered as JSON text. Backs `SpawnConfigSnapshot`'s manual `Debug` so a +/// log line can never disclose what the wire diff redacts. +pub(crate) fn redacted_canonical(value: &Value) -> String { + fn redact(path: &str, value: &Value) -> Value { + match value { + Value::Object(fields) => Value::Object( + fields + .iter() + .map(|(key, child)| (key.clone(), redact(&child_path(path, key), child))) + .collect(), + ), + leaf => match policy_for(path) { + MaskPolicy::Plain => leaf.clone(), + MaskPolicy::Text => char_count(leaf).map_or(Value::Null, |count| { + Value::String(format!("<{count} chars>")) + }), + policy => masked(policy, leaf).map_or(Value::Null, Value::String), + }, + } + } + redact("", value).to_string() +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs new file mode 100644 index 0000000000..a7a8cab93e --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/diff/tests.rs @@ -0,0 +1,572 @@ +use super::*; +use std::collections::{BTreeMap, BTreeSet}; + +const SECRET: &str = "sk-live-SENTINEL-0000"; +const RELAY_WITH_TOKEN: &str = "wss://relay.example/ws?token=SENTINEL"; + +/// Every field populated, so mutating one to `None` is a real change and the +/// coverage guard below sees the full serialized key set. +fn base() -> SpawnConfigSnapshot { + SpawnConfigSnapshot { + acp_command: "buzz-acp".into(), + command: "goose".into(), + args: vec!["--mode".into(), "acp".into()], + mcp_command: "goose-mcp".into(), + env: BTreeMap::from([ + ("OPENAI_API_KEY".to_string(), SECRET.to_string()), + ("BUZZ_LOG".to_string(), "info".to_string()), + ]), + relay_url: "wss://relay.example".into(), + team_instructions: Some("Team says hello.".into()), + system_prompt: Some("You are a test agent.".into()), + model: Some("gpt-5".into()), + provider: Some("openai".into()), + session_title: Some("Fizz".into()), + auth_tag: Some("tag-abcdefgh".into()), + respond_to: "owner-only".into(), + respond_to_allowlist: Some(vec!["a".repeat(64)]), + idle_timeout_seconds: Some(600), + max_turn_duration_seconds: Some(7200), + parallelism: 1, + } +} + +fn fields(entries: &[RestartDiffEntry]) -> Vec<&str> { + entries.iter().map(|entry| entry.field.as_str()).collect() +} + +fn change_at<'a>(entries: &'a [RestartDiffEntry], field: &str) -> &'a RestartChange { + &entries + .iter() + .find(|entry| entry.field == field) + .unwrap_or_else(|| panic!("no entry for {field}; got {:?}", fields(entries))) + .change +} + +/// One mutation per snapshot field, keyed by the diff path it must produce. +type Mutation = (&'static str, fn(&mut SpawnConfigSnapshot)); + +fn mutations() -> Vec { + vec![ + ("acp_command", |s| s.acp_command = "other-acp".into()), + ("command", |s| s.command = "claude".into()), + ("args", |s| s.args = vec!["--other".into()]), + ("mcp_command", |s| s.mcp_command = String::new()), + ("env.OPENAI_API_KEY", |s| { + s.env + .insert("OPENAI_API_KEY".into(), "sk-live-rotated-9999".into()); + }), + ("relay_url", |s| s.relay_url = "wss://other.example".into()), + ("team_instructions", |s| s.team_instructions = None), + ("system_prompt", |s| s.system_prompt = None), + ("model", |s| s.model = None), + ("provider", |s| s.provider = None), + ("session_title", |s| s.session_title = None), + ("auth_tag", |s| s.auth_tag = None), + ("respond_to", |s| s.respond_to = "anyone".into()), + ("respond_to_allowlist", |s| s.respond_to_allowlist = None), + ("idle_timeout_seconds", |s| s.idle_timeout_seconds = None), + ("max_turn_duration_seconds", |s| { + s.max_turn_duration_seconds = None + }), + ("parallelism", |s| s.parallelism = 8), + ] +} + +#[test] +fn every_field_mutation_drifts_the_canonical_value_and_names_that_field() { + for (field, mutate) in mutations() { + let before = base(); + let mut after = base(); + mutate(&mut after); + + assert_ne!( + before.canonical(), + after.canonical(), + "{field}: mutation must move the canonical value the badge compares" + ); + assert_eq!( + fields(&diff(&before, &after)), + vec![field], + "{field}: mutation must produce exactly that field's entry" + ); + // Both directions: `None -> Some` must be as visible as `Some -> None`. + assert_eq!( + fields(&diff(&after, &before)), + vec![field], + "{field}: reverse mutation must be equally visible" + ); + } +} + +#[test] +fn mutation_table_covers_every_serialized_field() { + let covered: BTreeSet<&str> = mutations() + .iter() + .map(|(field, _)| field.split('.').next().expect("non-empty path")) + .collect(); + let canonical = base().canonical(); + let serialized: BTreeSet<&str> = canonical + .as_object() + .expect("snapshot serializes as an object") + .keys() + .map(String::as_str) + .collect(); + assert_eq!( + covered, serialized, + "add a mutation row for every new snapshot field" + ); +} + +#[test] +fn identical_snapshots_produce_no_entries() { + assert!(diff(&base(), &base()).is_empty()); +} + +#[test] +fn env_map_insertion_order_is_not_drift() { + let mut reordered = base(); + reordered.env = base().env.into_iter().rev().collect(); + assert!(diff(&base(), &reordered).is_empty()); +} + +#[test] +fn entries_are_ordered_lexicographically_by_path() { + let mut after = base(); + after.parallelism = 4; + after.command = "claude".into(); + after.env.insert("ZZZ".into(), "1".into()); + after.env.insert("AAA".into(), "1".into()); + assert_eq!( + fields(&diff(&base(), &after)), + vec!["command", "env.AAA", "env.ZZZ", "parallelism"] + ); +} + +// ── map membership vs. nullable struct fields ──────────────────────────── + +#[test] +fn env_key_insertion_is_added_without_a_payload() { + let mut after = base(); + after.env.insert("NEW_KEY".into(), SECRET.into()); + assert_eq!( + change_at(&diff(&base(), &after), "env.NEW_KEY"), + &RestartChange::Added + ); +} + +#[test] +fn env_key_removal_is_removed_without_a_payload() { + let mut after = base(); + after.env.remove("BUZZ_LOG"); + assert_eq!( + change_at(&diff(&base(), &after), "env.BUZZ_LOG"), + &RestartChange::Removed + ); +} + +#[test] +fn cleared_nullable_field_stays_a_value_change_not_a_removal() { + let mut after = base(); + after.model = None; + assert_eq!( + change_at(&diff(&base(), &after), "model"), + &RestartChange::Value { + before: Value::String("gpt-5".into()), + after: Value::Null, + } + ); +} + +#[test] +fn array_field_changes_as_one_atomic_leaf() { + let mut after = base(); + after.respond_to_allowlist = Some(vec!["b".repeat(64)]); + let entries = diff(&base(), &after); + assert_eq!(fields(&entries), vec!["respond_to_allowlist"]); + assert!(matches!( + change_at(&entries, "respond_to_allowlist"), + RestartChange::Value { .. } + )); +} + +#[test] +fn allowlisted_env_key_shows_plain_value() { + // BUZZ_AGENT_THINKING_EFFORT is on the safe-to-reveal allowlist — the user + // must be able to see actual enum values like "medium → high". + let mut before = base(); + before + .env + .insert("BUZZ_AGENT_THINKING_EFFORT".into(), "medium".into()); + let mut after = before.clone(); + after + .env + .insert("BUZZ_AGENT_THINKING_EFFORT".into(), "high".into()); + assert_eq!( + change_at(&diff(&before, &after), "env.BUZZ_AGENT_THINKING_EFFORT"), + &RestartChange::Value { + before: Value::String("medium".into()), + after: Value::String("high".into()), + }, + "allowlisted env key must render plain before/after values" + ); +} + +#[test] +fn allowlisted_env_key_is_case_insensitive() { + // The allowlist comparison is case-insensitive; lowercase path must also + // render plain. + let mut before = base(); + before + .env + .insert("buzz_agent_provider".into(), "anthropic".into()); + let mut after = before.clone(); + after + .env + .insert("buzz_agent_provider".into(), "openai".into()); + assert_eq!( + change_at(&diff(&before, &after), "env.buzz_agent_provider"), + &RestartChange::Value { + before: Value::String("anthropic".into()), + after: Value::String("openai".into()), + }, + "allowlist match must be case-insensitive" + ); +} + +#[test] +fn non_allowlisted_env_key_stays_masked() { + // A key not in the allowlist must remain masked regardless of its name. + let mut after = base(); + after + .env + .insert("SOME_API_KEY".into(), "sk-live-rotated-9999".into()); + // SOME_API_KEY is a new key — starts as Added, not a value change. + // Use an existing env key (OPENAI_API_KEY is in base()) to test masking. + let mut before = base(); + before + .env + .insert("OPENAI_API_KEY".into(), "sk-live-SENTINEL-0000".into()); + let mut after2 = before.clone(); + after2 + .env + .insert("OPENAI_API_KEY".into(), "sk-live-rotated-9999".into()); + assert!( + matches!( + change_at(&diff(&before, &after2), "env.OPENAI_API_KEY"), + RestartChange::Masked { .. } + ), + "non-allowlisted env key must stay masked" + ); +} + +// ── masking policy ─────────────────────────────────────────────────────── + +#[test] +fn env_value_longer_than_eight_chars_shows_a_four_char_suffix() { + let mut after = base(); + after + .env + .insert("OPENAI_API_KEY".into(), "abcdefghi".into()); + assert_eq!( + change_at(&diff(&base(), &after), "env.OPENAI_API_KEY"), + &RestartChange::Masked { + before: Some("••••0000".into()), + after: Some("••••fghi".into()), + } + ); +} + +#[test] +fn env_value_of_exactly_eight_chars_shows_no_suffix() { + let mut before = base(); + before + .env + .insert("OPENAI_API_KEY".into(), "abcdefgh".into()); + let mut after = before.clone(); + after.env.insert("OPENAI_API_KEY".into(), "12345678".into()); + assert_eq!( + change_at(&diff(&before, &after), "env.OPENAI_API_KEY"), + &RestartChange::Masked { + before: Some("••••".into()), + after: Some("••••".into()), + } + ); +} + +#[test] +fn masking_counts_characters_not_bytes() { + // Nine two-byte characters: a byte-based length test would call this + // short, and byte slicing the last four would split a code point. + let mut before = base(); + before.env.insert("K".into(), "áéíóúàèìò".into()); + let mut after = before.clone(); + after.env.insert("K".into(), "áéíóúàèìá".into()); + assert_eq!( + change_at(&diff(&before, &after), "env.K"), + &RestartChange::Masked { + before: Some("••••àèìò".into()), + after: Some("••••àèìá".into()), + } + ); +} + +#[test] +fn args_are_masked_without_any_suffix() { + let mut after = base(); + after.args = vec![format!("--token={SECRET}")]; + assert_eq!( + change_at(&diff(&base(), &after), "args"), + &RestartChange::Masked { + before: Some("••••".into()), + after: Some("••••".into()), + } + ); +} + +#[test] +fn relay_url_is_masked_without_any_suffix() { + let mut after = base(); + after.relay_url = RELAY_WITH_TOKEN.into(); + assert_eq!( + change_at(&diff(&base(), &after), "relay_url"), + &RestartChange::Masked { + before: Some("••••".into()), + after: Some("••••".into()), + } + ); +} + +#[test] +fn auth_tag_is_masked_with_a_suffix() { + let mut after = base(); + after.auth_tag = Some("tag-ijklmnop".into()); + assert_eq!( + change_at(&diff(&base(), &after), "auth_tag"), + &RestartChange::Masked { + before: Some("••••efgh".into()), + after: Some("••••mnop".into()), + } + ); +} + +#[test] +fn large_text_fields_report_character_counts_only() { + let mut after = base(); + after.system_prompt = Some("Longer replacement prompt.".into()); + after.team_instructions = None; + let entries = diff(&base(), &after); + assert_eq!( + change_at(&entries, "system_prompt"), + &RestartChange::Text { + before_chars: Some("You are a test agent.".chars().count()), + after_chars: Some("Longer replacement prompt.".chars().count()), + } + ); + assert_eq!( + change_at(&entries, "team_instructions"), + &RestartChange::Text { + before_chars: Some("Team says hello.".chars().count()), + after_chars: None, + } + ); +} + +// ── secrecy sentinels ──────────────────────────────────────────────────── + +/// A snapshot whose every secret-bearing leaf carries a sentinel. +fn seeded_with_sentinels() -> SpawnConfigSnapshot { + let mut snapshot = base(); + snapshot.relay_url = RELAY_WITH_TOKEN.into(); + snapshot.args = vec![format!("--token={SECRET}")]; + snapshot.auth_tag = Some(SECRET.into()); + snapshot.env.insert("OPENAI_API_KEY".into(), SECRET.into()); + snapshot +} + +/// Every sentinel-bearing leaf changed, plus an added key, so each masking +/// arm has to redact a real value. +fn rotated_sentinels() -> SpawnConfigSnapshot { + let mut snapshot = seeded_with_sentinels(); + snapshot.relay_url = format!("{RELAY_WITH_TOKEN}2"); + snapshot.args = vec![format!("--token={SECRET}2")]; + snapshot.auth_tag = Some(format!("{SECRET}2")); + snapshot + .env + .insert("OPENAI_API_KEY".into(), format!("{SECRET}2")); + snapshot.env.insert("ADDED".into(), SECRET.into()); + snapshot +} + +#[test] +fn no_sentinel_reaches_the_serialized_diff() { + let entries = diff(&seeded_with_sentinels(), &rotated_sentinels()); + assert!(!entries.is_empty(), "fixture must actually drift"); + let wire = serde_json::to_string(&entries).expect("diff serializes"); + assert!(!wire.contains("SENTINEL"), "diff leaked a secret: {wire}"); + assert!( + !wire.contains("token="), + "diff leaked a query token: {wire}" + ); +} + +#[test] +fn no_sentinel_reaches_snapshot_debug_output() { + let rendered = format!("{:?}", seeded_with_sentinels()); + assert!(!rendered.contains("SENTINEL"), "Debug leaked: {rendered}"); + assert!(!rendered.contains("token="), "Debug leaked: {rendered}"); + // Large text is summarized rather than dumped. + assert!(!rendered.contains("You are a test agent.")); + // Non-secret leaves stay legible, or the log line is useless. + assert!(rendered.contains("goose")); +} + +#[test] +fn no_sentinel_reaches_the_owning_process_debug_output() { + // `ManagedAgentProcess` derives `Debug` and delegates to the snapshot's + // manual impl — this pins that the derive can never become the leak path. + #[cfg(unix)] + let program = "/usr/bin/true"; + #[cfg(windows)] + let program = "true"; + let child = std::process::Command::new(program) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn placeholder child"); + let process = crate::managed_agents::ManagedAgentProcess { + child, + log_path: std::path::PathBuf::new(), + spawn_config: seeded_with_sentinels(), + setup_mode: false, + adapter_availability: None, + start_nonce: "test-nonce".to_string(), + #[cfg(windows)] + job: None, + }; + let rendered = format!("{process:?}"); + assert!( + !rendered.contains("SENTINEL"), + "process Debug leaked a secret" + ); + assert!( + !rendered.contains("token="), + "process Debug leaked a query token" + ); +} + +// ── B1: the eligible vector is the single source of the badge ──────────── + +fn eligible( + orphaned: bool, + stamped: &SpawnConfigSnapshot, + current: &SpawnConfigSnapshot, + stamped_availability: Option, + current_availability: Option, +) -> (bool, Vec) { + let entries = eligible_restart_diff( + orphaned, + Some(TrackedSpawnState { + stamped, + current, + stamped_availability: stamped_availability.as_ref(), + current_availability, + }), + ); + (!entries.is_empty(), entries) +} + +#[test] +fn no_drift_yields_no_badge_and_no_entries() { + let (needs_restart, entries) = eligible(false, &base(), &base(), None, None); + assert!(!needs_restart); + assert!(entries.is_empty()); +} + +#[test] +fn snapshot_drift_yields_a_badge_and_that_entry() { + let mut current = base(); + current.model = Some("claude-4".into()); + let (needs_restart, entries) = eligible(false, &base(), ¤t, None, None); + assert!(needs_restart); + assert_eq!(fields(&entries), vec!["model"]); +} + +#[test] +fn availability_drift_alone_yields_a_badge_and_its_synthetic_entry() { + let (needs_restart, entries) = eligible( + false, + &base(), + &base(), + Some(AcpAvailabilityStatus::Available), + Some(AcpAvailabilityStatus::AdapterOutdated), + ); + assert!(needs_restart); + assert_eq!(fields(&entries), vec!["adapter_availability"]); + assert_eq!( + change_at(&entries, "adapter_availability"), + &RestartChange::Value { + before: Value::String("available".into()), + after: Value::String("adapter_outdated".into()), + } + ); +} + +#[test] +fn orphan_with_snapshot_drift_yields_no_badge_and_no_entries() { + let mut current = base(); + current.model = Some("claude-4".into()); + let (needs_restart, entries) = eligible(true, &base(), ¤t, None, None); + assert!(!needs_restart); + assert!(entries.is_empty()); +} + +#[test] +fn orphan_with_availability_drift_yields_no_badge_and_no_entries() { + let (needs_restart, entries) = eligible( + true, + &base(), + &base(), + Some(AcpAvailabilityStatus::Available), + Some(AcpAvailabilityStatus::AdapterOutdated), + ); + assert!(!needs_restart); + assert!(entries.is_empty()); +} + +#[test] +fn unstamped_availability_is_not_drift() { + // A runtime without a version gate stamps no availability; comparing that + // absence against a freshly cached value must not invent a badge. + let (needs_restart, entries) = eligible( + false, + &base(), + &base(), + None, + Some(AcpAvailabilityStatus::AdapterOutdated), + ); + assert!(!needs_restart); + assert!(entries.is_empty()); +} + +#[test] +fn unstamped_agent_yields_no_badge_and_no_entries() { + // A `runtime_pid`-adopted process — and any agent this workspace tracks no + // live pair for — has no `ManagedAgentProcess`, so no spawn config was ever + // stamped. With nothing to compare against there is no drift to report, and + // the badge derives from that emptiness. Distinct from the case above, + // where a real pair IS tracked and only its availability stamp is absent. + for orphaned in [false, true] { + let entries = eligible_restart_diff(orphaned, None); + let needs_restart = !entries.is_empty(); + assert!( + entries.is_empty(), + "unstamped agent (orphaned={orphaned}) must report no changed fields" + ); + assert!( + !needs_restart, + "unstamped agent (orphaned={orphaned}) must not light the badge" + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs similarity index 68% rename from desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs rename to desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs index f4ad404814..d76605ecff 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs @@ -2,6 +2,19 @@ use super::*; use crate::managed_agents::types::RespondTo; use std::collections::BTreeMap; +/// Canonical projection of a prospective snapshot — the exact value the drift +/// comparison reads, so these tests assert on drift itself rather than on a +/// proxy for it. +fn snapshot( + record: &ManagedAgentRecord, + personas: &[AgentDefinition], + teams: &[TeamRecord], + workspace_relay: &str, + global: &GlobalAgentConfig, +) -> serde_json::Value { + prospective_spawn_config_snapshot(record, personas, teams, workspace_relay, global).canonical() +} + fn record() -> ManagedAgentRecord { ManagedAgentRecord { pubkey: "p".repeat(64), @@ -86,22 +99,22 @@ fn persona(id: &str, runtime: Option<&str>, prompt: &str) -> AgentDefinition { } #[test] -fn hash_is_deterministic() { +fn snapshot_is_deterministic() { let rec = record(); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn materializing_runtime_keeps_hash_stable() { +fn materializing_runtime_keeps_snapshot_stable() { // Migration cutover invariant (Phase 1A): materializing the linked - // persona's runtime onto the record must NOT change the spawn hash — + // persona's runtime onto the record must NOT change the spawn snapshot — // otherwise every running persona-linked agent would show a spurious // restart badge right after migration. Pre-migration the command resolves // through the persona fallback; post-migration through record.runtime. - // Same persona, same runtime, same command → same hash. + // Same persona, same runtime, same command → equal snapshots. let personas = vec![persona("p1", Some("goose"), "Persona prompt.")]; let mut pre = record(); @@ -111,14 +124,14 @@ fn materializing_runtime_keeps_hash_stable() { post.runtime = Some("goose".into()); assert_eq!( - spawn_config_hash( + snapshot( &pre, &personas, &[], "wss://ws.example", &Default::default() ), - spawn_config_hash( + snapshot( &post, &personas, &[], @@ -129,31 +142,31 @@ fn materializing_runtime_keeps_hash_stable() { } #[test] -fn record_env_var_edit_changes_hash() { +fn record_env_var_edit_changes_snapshot() { let rec = record(); let mut edited = record(); edited .env_vars .insert("SOME_KEY".into(), "some-value".into()); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn record_prompt_edit_changes_hash() { +fn record_prompt_edit_changes_snapshot() { let rec = record(); let mut edited = record(); edited.system_prompt = Some("Edited prompt.".into()); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn persona_runtime_edit_changes_hash() { +fn persona_runtime_edit_changes_snapshot() { // The harness command resolves live personas at spawn, so a persona // runtime change means a restart WOULD change what runs → badge trips. let mut rec = record(); @@ -161,13 +174,13 @@ fn persona_runtime_edit_changes_hash() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &before, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &after, &[], "wss://ws.example", &Default::default()) ); } #[test] -fn persona_prompt_edit_changes_hash() { +fn persona_prompt_edit_changes_snapshot() { // Start/restore re-snapshot the persona prompt onto the record right // before spawning, so a persona prompt edit DOES apply on a plain // restart → the badge must trip. @@ -176,13 +189,13 @@ fn persona_prompt_edit_changes_hash() { let before = [persona("pers", Some("goose"), "old prompt")]; let after = [persona("pers", Some("goose"), "new prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &before, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &after, &[], "wss://ws.example", &Default::default()) ); } #[test] -fn workspace_relay_change_trips_hash_even_for_stored_record_relay() { +fn workspace_relay_change_trips_snapshot_even_for_stored_record_relay() { // The legacy per-record relay pin is ignored (#2122): every record spawns // against the active workspace relay, so a workspace relay change means a // restart would change what runs — pinned records included. @@ -192,13 +205,13 @@ fn workspace_relay_change_trips_hash_even_for_stored_record_relay() { "fixture should carry a legacy pin" ); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://relay-a.example", &Default::default()), - spawn_config_hash(&rec, &[], &[], "wss://relay-b.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://relay-a.example", &Default::default()), + snapshot(&rec, &[], &[], "wss://relay-b.example", &Default::default()) ); } #[test] -fn stored_record_relay_does_not_affect_hash() { +fn stored_record_relay_does_not_affect_snapshot() { // Editing the (ignored) stored pin must not badge a restart: what a // restart would run is identical either way. let mut a = record(); @@ -206,20 +219,20 @@ fn stored_record_relay_does_not_affect_hash() { a.relay_url = String::new(); b.relay_url = "wss://legacy-pin.example".into(); assert_eq!( - spawn_config_hash(&a, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&b, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&a, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&b, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn respond_to_allowlist_edit_changes_hash() { +fn respond_to_allowlist_edit_changes_snapshot() { let rec = record(); let mut edited = record(); edited.respond_to = RespondTo::Allowlist; edited.respond_to_allowlist = vec!["a".repeat(64)]; assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } @@ -231,13 +244,13 @@ fn allowlist_ignored_when_mode_is_not_allowlist() { let mut edited = record(); edited.respond_to_allowlist = vec!["a".repeat(64)]; assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn allowlist_normalization_equivalent_edits_do_not_change_hash() { +fn allowlist_normalization_equivalent_edits_do_not_change_snapshot() { // The env receives the normalized list (trim/lowercase/dedup), so edits // that normalize to the same value must not badge. let mut rec = record(); @@ -249,48 +262,48 @@ fn allowlist_normalization_equivalent_edits_do_not_change_hash() { "a".repeat(64), // duplicate ]; assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn allowlist_content_edit_still_changes_hash() { +fn allowlist_content_edit_still_changes_snapshot() { let mut rec = record(); rec.respond_to = RespondTo::Allowlist; rec.respond_to_allowlist = vec!["a".repeat(64)]; let mut edited = rec.clone(); edited.respond_to_allowlist = vec!["b".repeat(64)]; assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn explicit_max_turn_duration_changes_hash_from_none() { +fn explicit_max_turn_duration_changes_snapshot_from_none() { let rec = record(); let mut edited = record(); edited.max_turn_duration_seconds = Some(7200); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn non_default_max_turn_duration_changes_hash() { +fn non_default_max_turn_duration_changes_snapshot() { let rec = record(); let mut edited = record(); edited.max_turn_duration_seconds = Some(42); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] -fn non_spawn_bookkeeping_fields_do_not_change_hash() { +fn non_spawn_bookkeeping_fields_do_not_change_snapshot() { // updated_at / runtime_pid / last_* are lifecycle bookkeeping, not spawn // inputs — routine record saves must not trip the badge. let rec = record(); @@ -300,17 +313,17 @@ fn non_spawn_bookkeeping_fields_do_not_change_hash() { edited.last_started_at = Some("later".into()); edited.last_exit_code = Some(0); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()) + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()) ); } #[test] fn resnapshot_does_not_clobber_record_quad_with_definition_absent_quad() { - // B5 hash row 3: the prospective re-snapshot copies ONLY + // B5 drift row 3: the prospective re-snapshot copies ONLY // prompt/model/provider/env from the linked definition. An instance // whose owner hand-set respond_to/allowlist/parallelism must - // hash identically whether or not its definition carries a quad — + // snapshot identically whether or not its definition carries a quad — // activation of the definition-level defaults must never reach through // spawn and overwrite instance state. let quadless_definition = vec![persona("p1", Some("goose"), "Persona prompt.")]; @@ -326,44 +339,44 @@ fn resnapshot_does_not_clobber_record_quad_with_definition_absent_quad() { definition_with_quad[0].parallelism = Some(8); assert_eq!( - spawn_config_hash( + snapshot( &rec, &quadless_definition, &[], "wss://ws.example", &Default::default() ), - spawn_config_hash( + snapshot( &rec, &definition_with_quad, &[], "wss://ws.example", &Default::default() ), - "definition quad must not leak into the spawn hash of an existing instance" + "definition quad must not leak into the spawn snapshot of an existing instance" ); } #[test] -fn empty_prompt_hashes_like_absent_prompt() { - // B5 hash row 2 foundation: Some("") and None spawn identically (env var - // absent either way), so they must hash equal — a backfilled prompt-less +fn empty_prompt_snapshots_like_absent_prompt() { + // B5 drift row 2 foundation: Some("") and None spawn identically (env var + // absent either way), so they must snapshot equal — a backfilled prompt-less // record re-snapshots to Some("") and must not trip the badge. let mut absent = record(); absent.system_prompt = None; let mut empty = record(); empty.system_prompt = Some(String::new()); assert_eq!( - spawn_config_hash(&absent, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&empty, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&absent, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&empty, &[], &[], "wss://ws.example", &Default::default()), ); } -/// (a) A definition-runtime edit must change spawn_config_hash for a +/// (a) A definition-runtime edit must change the snapshot for a /// materialized, override-free record — the prospective re-snapshot now -/// copies the persona's runtime onto the record before hashing. +/// copies the persona's runtime onto the record before snapshotting. #[test] -fn definition_runtime_edit_changes_hash_for_materialized_record() { +fn definition_runtime_edit_changes_snapshot_for_materialized_record() { let mut rec = record(); rec.persona_id = Some("pers".into()); rec.runtime = Some("goose".into()); // materialized runtime on instance @@ -371,8 +384,8 @@ fn definition_runtime_edit_changes_hash_for_materialized_record() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &before, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &after, &[], "wss://ws.example", &Default::default()), "definition runtime edit must badge a materialized, override-free instance" ); } @@ -389,8 +402,8 @@ fn known_runtime_pin_yields_to_definition_runtime_change() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &before, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &after, &[], "wss://ws.example", &Default::default()), "stale known-runtime pin must not shadow a definition runtime edit" ); } @@ -407,16 +420,16 @@ fn custom_command_override_beats_definition_runtime_change() { let before = [persona("pers", Some("goose"), "prompt")]; let after = [persona("pers", Some("claude"), "prompt")]; assert_eq!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &before, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &after, &[], "wss://ws.example", &Default::default()), "custom command override must win regardless of definition runtime change" ); } /// (d) When the linked definition is absent the prospective re-snapshot is -/// skipped entirely: the materialized runtime must still affect the hash. +/// skipped entirely: the materialized runtime must still reach the snapshot. #[test] -fn missing_definition_leaves_materialized_runtime_in_hash() { +fn missing_definition_leaves_materialized_runtime_in_snapshot() { let mut rec = record(); rec.persona_id = Some("missing".into()); rec.runtime = Some("goose".into()); // materialized runtime @@ -427,28 +440,28 @@ fn missing_definition_leaves_materialized_runtime_in_hash() { no_runtime.runtime = None; assert_ne!( - spawn_config_hash( + snapshot( &rec, no_personas, &[], "wss://ws.example", &Default::default() ), - spawn_config_hash( + snapshot( &no_runtime, no_personas, &[], "wss://ws.example", &Default::default() ), - "materialized runtime must still affect hash when definition is absent" + "materialized runtime must still reach the snapshot when definition is absent" ); } -// ── Global default trips hash for linked inherited agents ───────────────── +// ── Global default trips drift for linked inherited agents ─────────────── #[test] -fn global_model_change_trips_hash_for_linked_inherited_agent() { +fn global_model_change_trips_snapshot_for_linked_inherited_agent() { let mut rec = record(); rec.persona_id = Some("p1".into()); rec.model = Some("stale-record-model".into()); @@ -466,17 +479,17 @@ fn global_model_change_trips_hash_for_linked_inherited_agent() { ..Default::default() }; - let hash_a = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_a); - let hash_b = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_b); + let snapshot_a = snapshot(&rec, &personas, &[], "wss://ws.example", &global_a); + let snapshot_b = snapshot(&rec, &personas, &[], "wss://ws.example", &global_b); assert_ne!( - hash_a, hash_b, - "changing the global default must trip the hash for a linked inherited agent" + snapshot_a, snapshot_b, + "changing the global default must drift a linked inherited agent" ); } #[test] -fn global_model_change_trips_hash_without_model_env_var() { +fn global_model_change_trips_snapshot_without_model_env_var() { let mut rec = record(); rec.persona_id = Some("p1".into()); rec.agent_command = "some-harness-without-model-env".into(); @@ -497,26 +510,26 @@ fn global_model_change_trips_hash_without_model_env_var() { ..Default::default() }; - let hash_a = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_a); - let hash_b = spawn_config_hash(&rec, &personas, &[], "wss://ws.example", &global_b); + let snapshot_a = snapshot(&rec, &personas, &[], "wss://ws.example", &global_a); + let snapshot_b = snapshot(&rec, &personas, &[], "wss://ws.example", &global_b); assert_ne!( - hash_a, hash_b, - "global model change must trip hash even without a model_env_var runtime" + snapshot_a, snapshot_b, + "global model change must drift even without a model_env_var runtime" ); } #[test] -fn linked_instance_stale_prompt_bytes_are_inert_at_hash_time() { +fn linked_instance_stale_prompt_bytes_are_inert_at_snapshot_time() { // Regression for the split-resolve defect: prompt used to be read from // the record's own (possibly Phase-A-snapshot-stale) bytes while // model/provider were resolved live from the definition. A definition // edit landing between a caller's snapshot apply and spawn could hand a - // fresh model/provider to a stale prompt, and the hash (which already + // fresh model/provider to a stale prompt, and the drift check (which already // resolved model/provider live) would silently agree with a spawn that // wrote the stale prompt. Now both come from one `resolve_effective_config` // call, so a record whose own `system_prompt` bytes disagree with the - // live definition must hash exactly as if the record carried the + // live definition must snapshot exactly as if the record carried the // definition's prompt verbatim — the record's prompt bytes are inert for // a linked instance. let mut rec = record(); @@ -529,26 +542,26 @@ fn linked_instance_stale_prompt_bytes_are_inert_at_hash_time() { let personas = [persona("p1", Some("goose"), "live prompt")]; assert_eq!( - spawn_config_hash( + snapshot( &rec, &personas, &[], "wss://ws.example", &Default::default() ), - spawn_config_hash( + snapshot( &matching_bytes, &personas, &[], "wss://ws.example", &Default::default() ), - "record's own system_prompt bytes must not affect the hash of a linked instance" + "record's own system_prompt bytes must not affect the snapshot of a linked instance" ); } #[test] -fn display_name_edit_changes_hash() { +fn display_name_edit_changes_snapshot() { // The spawn writes BUZZ_ACP_SESSION_TITLE from display_name-or-name, so a // rename must trip the badge: the running process keeps the old title // until it restarts, and the operator has to be told that. @@ -556,32 +569,32 @@ fn display_name_edit_changes_hash() { let mut renamed = record(); renamed.display_name = Some("Fizz".into()); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&renamed, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&renamed, &[], &[], "wss://ws.example", &Default::default()), "a display-name rename changes the spawned session title and must badge" ); } #[test] -fn name_edit_changes_hash_when_display_name_is_absent() { +fn name_edit_changes_snapshot_when_display_name_is_absent() { // With no display_name the title falls back to the unique handle, so the - // handle is what the env write carries and what must be hashed. + // handle is what the env write carries and what must be snapshotted. let rec = record(); let mut renamed = record(); renamed.name = "agent-2".into(); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&renamed, &[], &[], "wss://ws.example", &Default::default()), - "the fallback title source must reach the hash too" + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&renamed, &[], &[], "wss://ws.example", &Default::default()), + "the fallback title source must reach the snapshot too" ); } #[test] -fn display_name_edit_does_not_change_hash_under_an_explicit_title_override() { +fn display_name_edit_does_not_change_snapshot_under_an_explicit_title_override() { // User env is written AFTER the Buzz-set title (last-wins), so an explicit // BUZZ_ACP_SESSION_TITLE is what the child actually runs with. Renaming the // record changes nothing about the spawned process, so badging it would be - // a false restart prompt. The override itself still reaches the hash + // a false restart prompt. The override itself still reaches the snapshot // through the effective env. let mut rec = record(); rec.env_vars @@ -589,14 +602,14 @@ fn display_name_edit_does_not_change_hash_under_an_explicit_title_override() { let mut renamed = rec.clone(); renamed.display_name = Some("Fizz".into()); assert_eq!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&renamed, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&renamed, &[], &[], "wss://ws.example", &Default::default()), "a rename shadowed by an explicit title override must not badge" ); } #[test] -fn title_override_edit_changes_hash() { +fn title_override_edit_changes_snapshot() { // Counterpart to the test above: the override is not inert — editing it // changes what the child runs with and must badge. let mut rec = record(); @@ -607,8 +620,8 @@ fn title_override_edit_changes_hash() { .env_vars .insert("BUZZ_ACP_SESSION_TITLE".into(), "Other Title".into()); assert_ne!( - spawn_config_hash(&rec, &[], &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&edited, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&edited, &[], &[], "wss://ws.example", &Default::default()), "editing an explicit title override must badge" ); } @@ -616,7 +629,7 @@ fn title_override_edit_changes_hash() { #[test] fn linked_instance_prompt_model_provider_resolve_from_one_call() { // The prompt for a linked instance must track the definition, exactly - // like model/provider — a definition prompt edit trips the hash even + // like model/provider — a definition prompt edit drifts the snapshot even // though the record's own (stale) system_prompt bytes are unchanged. let mut rec = record(); rec.persona_id = Some("p1".into()); @@ -626,25 +639,25 @@ fn linked_instance_prompt_model_provider_resolve_from_one_call() { let after = [persona("p1", Some("goose"), "new definition prompt")]; assert_ne!( - spawn_config_hash(&rec, &before, &[], "wss://ws.example", &Default::default()), - spawn_config_hash(&rec, &after, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &before, &[], "wss://ws.example", &Default::default()), + snapshot(&rec, &after, &[], "wss://ws.example", &Default::default()), "linked instance prompt must resolve from the live definition, not stale record bytes" ); } -// ── I2: definition args and env reach spawn_config_hash ────────────────────── +// ── I2: definition args and env reach the snapshot ─────────────────────────── // // These tests prove that editing a custom harness definition's args or env -// changes spawn_config_hash, which trips the "restart required" badge. -// They would fail if spawn_config_hash used only record.agent_args without +// change the snapshot, which trips the "restart required" badge. +// They would fail if the snapshot used only record.agent_args without // falling back to definition args, or if resolve_effective_agent_env did not // include definition env. /// When a record has no instance args but the definition has default args, -/// changing the definition args changes the spawn hash. This would fail if -/// spawn_config_hash used only record.agent_args. +/// changing the definition args changes the snapshot. This would fail if +/// the snapshot used only record.agent_args. #[test] -fn spawn_hash_changes_when_definition_default_args_change() { +fn spawn_snapshot_changes_when_definition_default_args_change() { use crate::managed_agents::custom_harnesses::{ registry_test_lock, warm_harness_registry_from_dir, }; @@ -652,8 +665,8 @@ fn spawn_hash_changes_when_definition_default_args_change() { use tempfile::tempdir; // The loaded-harness registry is process-global: a parallel test re-warming - // it between the two hash computations makes both resolve to no-definition - // and h1 == h2 (observed on Windows CI). + // it between the two snapshots makes both resolve to no-definition + // and s1 == s2 (observed on Windows CI). let _lock = registry_test_lock(); let dir = tempdir().unwrap(); @@ -669,7 +682,7 @@ fn spawn_hash_changes_when_definition_default_args_change() { r.runtime = Some("my-def".into()); r.agent_args = vec![]; // no instance args → definition args are used - let h1 = spawn_config_hash(&r, &[], &[], "ws://relay", &Default::default()); + let s1 = snapshot(&r, &[], &[], "ws://relay", &Default::default()); // Update to v2 args and re-warm (simulating save + transactional refresh). fs::write( @@ -679,18 +692,18 @@ fn spawn_hash_changes_when_definition_default_args_change() { .unwrap(); warm_harness_registry_from_dir(Some(dir.path())); - let h2 = spawn_config_hash(&r, &[], &[], "ws://relay", &Default::default()); + let s2 = snapshot(&r, &[], &[], "ws://relay", &Default::default()); assert_ne!( - h1, h2, - "changing definition default args must change the spawn hash" + s1, s2, + "changing definition default args must change the snapshot" ); } -/// When a definition has env vars, adding them changes the spawn hash. This +/// When a definition has env vars, adding them changes the snapshot. This /// proves resolve_effective_agent_env includes definition env in the layering. #[test] -fn spawn_hash_changes_when_definition_env_changes() { +fn spawn_snapshot_changes_when_definition_env_changes() { use crate::managed_agents::custom_harnesses::{ registry_test_lock, warm_harness_registry_from_dir, }; @@ -712,7 +725,7 @@ fn spawn_hash_changes_when_definition_env_changes() { let mut r = record(); r.runtime = Some("env-def".into()); - let h1 = spawn_config_hash(&r, &[], &[], "ws://relay", &Default::default()); + let s1 = snapshot(&r, &[], &[], "ws://relay", &Default::default()); // Update to include env and re-warm. fs::write( @@ -722,16 +735,16 @@ fn spawn_hash_changes_when_definition_env_changes() { .unwrap(); warm_harness_registry_from_dir(Some(dir.path())); - let h2 = spawn_config_hash(&r, &[], &[], "ws://relay", &Default::default()); + let s2 = snapshot(&r, &[], &[], "ws://relay", &Default::default()); - assert_ne!(h1, h2, "adding definition env must change the spawn hash"); + assert_ne!(s1, s2, "adding definition env must change the snapshot"); } /// Instance-level args win over definition default args (non-empty instance -/// args must NOT be overridden by the definition). The hash must match a record +/// args must NOT be overridden by the definition). The snapshot must match a record /// that has the same effective args from either source. #[test] -fn spawn_hash_instance_args_win_over_definition_args() { +fn spawn_snapshot_instance_args_win_over_definition_args() { use crate::managed_agents::custom_harnesses::{ registry_test_lock, warm_harness_registry_from_dir, }; @@ -756,12 +769,12 @@ fn spawn_hash_instance_args_win_over_definition_args() { r_no_instance.runtime = Some("arg-def".into()); r_no_instance.agent_args = vec![]; - let h_instance = spawn_config_hash(&r_instance, &[], &[], "ws://relay", &Default::default()); - let h_no_instance = - spawn_config_hash(&r_no_instance, &[], &[], "ws://relay", &Default::default()); + let snapshot_instance = snapshot(&r_instance, &[], &[], "ws://relay", &Default::default()); + let snapshot_no_instance = + snapshot(&r_no_instance, &[], &[], "ws://relay", &Default::default()); assert_ne!( - h_instance, h_no_instance, - "instance args and definition args must produce different hashes" + snapshot_instance, snapshot_no_instance, + "instance args and definition args must produce different snapshots" ); } diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 255c1aae32..c5bb6173d1 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -462,13 +462,12 @@ pub struct RelayMeshConfig { pub struct ManagedAgentProcess { pub child: Child, pub log_path: PathBuf, - /// Digest of the effective spawn config at launch (see - /// `spawn_hash::spawn_config_hash`). Runtime-only — never persisted. The - /// summary builder recomputes the hash from current disk state and flags - /// `needs_restart` on mismatch. Agents adopted via a persisted - /// `runtime_pid` have no `ManagedAgentProcess` entry, so their spawn - /// config is unknown and the badge stays off. - pub spawn_config_hash: u64, + /// The effective spawn config this process was launched with (see + /// `spawn_snapshot::SpawnConfigSnapshot`). Runtime-only — never persisted. + /// The summary builder recomputes a prospective snapshot and reports + /// differing fields via `ManagedAgentSummary::restart_diff`. Agents + /// adopted via `runtime_pid` have none; their config is unknown. + pub spawn_config: super::spawn_snapshot::SpawnConfigSnapshot, /// Whether this process was spawned in setup-listener mode (i.e. /// `BUZZ_ACP_SETUP_PAYLOAD` was set at launch because the agent was /// `NotReady`). Runtime-only — never persisted. Used by @@ -541,13 +540,14 @@ pub struct ManagedAgentSummary { /// `OrphanedInstance` arm via `require_resolved`) — so the UI /// should surface that it's stuck, not merely stale. pub persona_orphaned: bool, - /// `true` when the running process was spawned with a config that no - /// longer matches what a spawn would use today — a plain restart would - /// change what runs. Complements `persona_out_of_date`: the badge means - /// "a restart would change what runs"; out-of-date means "a respawn - /// would." Always `false` for stopped agents and for processes adopted - /// via a persisted `runtime_pid` (their spawn config is unknown). + /// `true` when the running process's spawn config no longer matches + /// what a spawn would use today. Derived from `restart_diff` — lit + /// exactly when there is something to show. Always `false` for stopped, + /// orphaned, or `runtime_pid`-adopted agents. pub needs_restart: bool, + /// Fields that drifted since launch, redacted for display. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub restart_diff: Vec, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub env_vars: BTreeMap, pub backend: BackendKind, diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 96ed556068..1db7b9b524 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -694,3 +694,93 @@ fn mint_rejects_out_of_range_input_parallelism() { "input-branch error must not blame the definition: {err}" ); } + +// ── Restart-diff wire shape ───────────────────────────────────────────────── + +fn summary_fixture( + restart_diff: Vec, +) -> super::ManagedAgentSummary { + super::ManagedAgentSummary { + pubkey: "aa".repeat(32), + name: "test".into(), + persona_id: None, + runtime: None, + team_id: None, + relay_url: String::new(), + acp_command: "buzz-acp".into(), + agent_command: "goose".into(), + agent_command_override: None, + agent_args: Vec::new(), + mcp_command: String::new(), + turn_timeout_seconds: 320, + idle_timeout_seconds: None, + max_turn_duration_seconds: None, + parallelism: 1, + system_prompt: None, + avatar_url: None, + model: None, + model_source: None, + provider: None, + persona_out_of_date: false, + persona_orphaned: false, + // Both fields derive from one vector in `build_managed_agent_summary`; + // the fixture reproduces that rule rather than letting them disagree. + needs_restart: !restart_diff.is_empty(), + restart_diff, + env_vars: Default::default(), + backend: super::BackendKind::Local, + backend_agent_id: None, + status: "running".into(), + pid: Some(4242), + created_at: "2026-01-01T00:00:00Z".into(), + updated_at: "2026-01-01T00:00:00Z".into(), + last_started_at: None, + last_stopped_at: None, + last_exit_code: None, + last_error: None, + last_error_code: None, + start_on_app_launch: false, + auto_restart_on_config_change: false, + log_path: String::new(), + respond_to: RespondTo::OwnerOnly, + respond_to_allowlist: Vec::new(), + } +} + +#[test] +fn summary_without_drift_omits_restart_diff_from_the_wire() { + // An adopted `runtime_pid`-only process is never stamped, so its summary + // carries an empty vector. `skip_serializing_if` must then drop the key + // entirely — the frontend normalizes omission to `[]`, and emitting an + // empty array on every stopped agent would bloat every list response. + let wire = serde_json::to_value(summary_fixture(Vec::new())).expect("summary serializes"); + assert_eq!(wire.get("needs_restart"), Some(&serde_json::json!(false))); + assert!( + wire.get("restart_diff").is_none(), + "empty restart_diff must be omitted, got: {wire}" + ); +} + +#[test] +fn summary_with_drift_serializes_restart_diff_entries() { + // The other side of the same rule: a present entry must reach the wire + // under its snake_case key with the tagged change payload intact. + let wire = serde_json::to_value(summary_fixture(vec![ + crate::managed_agents::spawn_snapshot::RestartDiffEntry { + field: "model".into(), + change: crate::managed_agents::spawn_snapshot::diff::RestartChange::Value { + before: serde_json::json!("gpt-5"), + after: serde_json::json!("claude-4"), + }, + }, + ])) + .expect("summary serializes"); + assert_eq!(wire.get("needs_restart"), Some(&serde_json::json!(true))); + assert_eq!( + wire.get("restart_diff"), + Some(&serde_json::json!([{ + "field": "model", + "change": { "kind": "value", "before": "gpt-5", "after": "claude-4" }, + }])) + ); +} diff --git a/desktop/src-tauri/src/migration/backfill.rs b/desktop/src-tauri/src/migration/backfill.rs index cd62f63bbb..74cef7ffe6 100644 --- a/desktop/src-tauri/src/migration/backfill.rs +++ b/desktop/src-tauri/src/migration/backfill.rs @@ -26,7 +26,7 @@ use crate::managed_agents::{ /// `unwrap_or_default`, env COPIED so later instances inherit a working /// config, quad copied to the definition defaults) and the record gains /// `persona_source_version` = the new definition's content hash, so -/// neither `spawn_config_hash` nor the drift badge moves. +/// neither the spawn-config snapshot nor the drift badge moves. /// /// The manufactured definition's slug is the agent's pubkey: 64-hex passes /// the NIP-AP slug grammar on both relay and desktop ends, and agent pubkeys diff --git a/desktop/src-tauri/src/migration/backfill_tests.rs b/desktop/src-tauri/src/migration/backfill_tests.rs index 5d52d56678..d277a2aa5f 100644 --- a/desktop/src-tauri/src/migration/backfill_tests.rs +++ b/desktop/src-tauri/src/migration/backfill_tests.rs @@ -1,5 +1,5 @@ use super::backfill_standalone_agents_in_dir; -use crate::managed_agents::spawn_hash::spawn_config_hash; +use crate::managed_agents::spawn_snapshot::prospective_spawn_config_snapshot; use crate::managed_agents::{AgentDefinition, ManagedAgentRecord}; use crate::migration::test_support::{read_agents_json, write_agents_json}; use std::path::Path; @@ -116,11 +116,11 @@ fn backfilled_definition_carries_prompt_present_even_if_empty() { } #[test] -fn backfill_of_promptless_record_keeps_spawn_hash_stable() { - // B5 hash row 2: pre-backfill the record hashes prompt None; post-backfill +fn backfill_of_promptless_record_keeps_spawn_snapshot_stable() { + // B5 drift row 2: pre-backfill the record snapshots prompt None; post-backfill // the prospective re-snapshot pulls Some("") from the manufactured // definition. The spawn layer treats an empty prompt as no prompt (env - // absent either way), so the hash must not move — otherwise every + // absent either way), so the snapshot must not move — otherwise every // prompt-less standalone agent lights the restart badge on upgrade. let dir = tempfile::tempdir().unwrap(); let pubkey = "c".repeat(64); @@ -131,7 +131,7 @@ fn backfill_of_promptless_record_keeps_spawn_hash_stable() { let pre_records = load_typed(dir.path()); let pre_instance = pre_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); - let hash_before = spawn_config_hash( + let before = prospective_spawn_config_snapshot( pre_instance, &[], &[], @@ -147,7 +147,7 @@ fn backfill_of_promptless_record_keeps_spawn_hash_stable() { .iter() .filter_map(|r| r.to_definition_view()) .collect(); - let hash_after = spawn_config_hash( + let after = prospective_spawn_config_snapshot( post_instance, &personas, &[], @@ -156,15 +156,16 @@ fn backfill_of_promptless_record_keeps_spawn_hash_stable() { ); assert_eq!( - hash_before, hash_after, + before.canonical(), + after.canonical(), "backfill must not flip the restart badge for prompt-less agents" ); } #[test] -fn backfill_of_prompted_record_keeps_spawn_hash_stable() { +fn backfill_of_prompted_record_keeps_spawn_snapshot_stable() { // The general no-behavior-change rail: a standalone agent WITH config - // must also hash identically across backfill (the definition snapshots + // must also snapshot identically across backfill (the definition snapshots // the record's own values, so the re-snapshot writes back what is // already there). let dir = tempfile::tempdir().unwrap(); @@ -180,7 +181,7 @@ fn backfill_of_prompted_record_keeps_spawn_hash_stable() { let pre_records = load_typed(dir.path()); let pre_instance = pre_records.iter().find(|r| !r.pubkey.is_empty()).unwrap(); - let hash_before = spawn_config_hash( + let before = prospective_spawn_config_snapshot( pre_instance, &[], &[], @@ -196,7 +197,7 @@ fn backfill_of_prompted_record_keeps_spawn_hash_stable() { .iter() .filter_map(|r| r.to_definition_view()) .collect(); - let hash_after = spawn_config_hash( + let after = prospective_spawn_config_snapshot( post_instance, &personas, &[], @@ -204,7 +205,7 @@ fn backfill_of_prompted_record_keeps_spawn_hash_stable() { &Default::default(), ); - assert_eq!(hash_before, hash_after); + assert_eq!(before.canonical(), after.canonical()); } #[test] diff --git a/desktop/src-tauri/src/migration/materialize.rs b/desktop/src-tauri/src/migration/materialize.rs index 5930920dd2..6ca23200e6 100644 --- a/desktop/src-tauri/src/migration/materialize.rs +++ b/desktop/src-tauri/src/migration/materialize.rs @@ -15,8 +15,8 @@ use super::{canonical_dev_data_dir, load_persona_runtimes, patch_json_records}; /// persona (unified agent model, Phase 1A). After this, spawn resolution reads /// the record's own runtime (`record_agent_command` step 2) instead of the /// live persona — same effective command by construction, so the spawn-config -/// hash is unchanged and no running agent shows a spurious restart badge (see -/// `spawn_hash::tests::materializing_runtime_keeps_hash_stable`). +/// snapshot is unchanged and no running agent shows a spurious restart badge +/// (see `spawn_snapshot::tests::materializing_runtime_keeps_snapshot_stable`). /// /// Idempotent: records that already carry `runtime` are untouched, as are /// records with no linked persona or a persona without a runtime (both keep diff --git a/desktop/src/features/agents/ui/AgentIdentityCard.tsx b/desktop/src/features/agents/ui/AgentIdentityCard.tsx index 6f13a84ae8..b0668616ae 100644 --- a/desktop/src/features/agents/ui/AgentIdentityCard.tsx +++ b/desktop/src/features/agents/ui/AgentIdentityCard.tsx @@ -77,7 +77,12 @@ export function AgentIdentityCard({ {modelLabel} ) : null} - {statusBadge} + {/* pointer-events-auto: the overlay button above has pointer-events-none + on this container, but the status badge itself (a sibling of the button + in z-order) needs hover so the restart diff tooltip can fire. */} + {statusBadge ? ( +
{statusBadge}
+ ) : null} ); diff --git a/desktop/src/features/agents/ui/ManagedAgentRow.tsx b/desktop/src/features/agents/ui/ManagedAgentRow.tsx index 606d2b7883..62a4169fc9 100644 --- a/desktop/src/features/agents/ui/ManagedAgentRow.tsx +++ b/desktop/src/features/agents/ui/ManagedAgentRow.tsx @@ -1,11 +1,6 @@ import * as React from "react"; -import { - AlertTriangle, - ChevronDown, - ChevronRight, - RefreshCw, -} from "lucide-react"; +import { AlertTriangle, ChevronDown, ChevronRight } from "lucide-react"; import { useAppNavigation } from "@/app/navigation/useAppNavigation"; import { PresenceDot } from "@/features/presence/ui/PresenceBadge"; @@ -27,6 +22,7 @@ import { friendlyAgentLastError } from "@/features/agents/lib/friendlyAgentLastE import { ManagedAgentLogPanel } from "./ManagedAgentLogPanel"; import { PubKey } from "@/shared/ui/PubKey"; import { SubsectionLabel } from "@/shared/ui/PageHeader"; +import { RestartDiffBadge } from "./RestartDiffBadge"; export function ManagedAgentRow({ agent, @@ -100,7 +96,7 @@ export function ManagedAgentRow({ "overflow-hidden transition-colors", isLogSelected ? "bg-primary/5" : "hover:bg-muted/20", )} - data-testid={`managed-agent-${agent.pubkey}`} + data-testid={`managed-agent-row-${agent.pubkey}`} >
{isLocal ? ( @@ -158,7 +154,16 @@ export function ManagedAgentRow({
)} + {/* B4: restart badge is a sibling of the expansion button — never + inside it. TooltipTrigger renders as a (non-interactive), + so no nested interactive elements are introduced here. */}
+ {agent.needsRestart ? ( + + ) : null}
) : null} diff --git a/desktop/src/shared/api/restartDiff.ts b/desktop/src/shared/api/restartDiff.ts new file mode 100644 index 0000000000..1ea46f488e --- /dev/null +++ b/desktop/src/shared/api/restartDiff.ts @@ -0,0 +1,34 @@ +/** + * Wire types for the restart-required config diff. + * + * These are camelCase / TS-idiomatic mirrors of the Rust `RestartDiffEntry` + * serialized shape. The `field` is a dotted path derived from serde field + * names (e.g. `"model"`, `"env.FOO"`); display labels are produced by + * humanising the path — there is no per-field label map. + */ + +/** A JSON-compatible value. Arrays are atomic leaves in the diff. */ +export type JsonValue = + | null + | boolean + | number + | string + | JsonValue[] + | { [key: string]: JsonValue }; + +/** + * One change in a restart diff. The `kind` discriminant is a closed set of + * five values; unknown `field` paths must render gracefully in the UI. + */ +export type RestartChange = + | { kind: "value"; before: JsonValue; after: JsonValue } + | { kind: "text"; before_chars: number | null; after_chars: number | null } + | { kind: "masked"; before: string | null; after: string | null } + | { kind: "added" } + | { kind: "removed" }; + +export type RestartDiffEntry = { + /** Dotted path derived from serde field names, e.g. "model", "env.FOO". */ + field: string; + change: RestartChange; +}; diff --git a/desktop/src/shared/api/tauri.ts b/desktop/src/shared/api/tauri.ts index 62a262e4f6..c44fd3b1c0 100644 --- a/desktop/src/shared/api/tauri.ts +++ b/desktop/src/shared/api/tauri.ts @@ -118,6 +118,8 @@ type RawRelayAgent = { respond_to?: RelayAgent["respondTo"]; respond_to_allowlist?: string[]; }; + +import type { RestartDiffEntry as RawRestartDiffEntry } from "./restartDiff"; export type RawManagedAgent = { pubkey: string; name: string; @@ -143,6 +145,7 @@ export type RawManagedAgent = { persona_out_of_date: boolean; persona_orphaned: boolean; needs_restart: boolean; + restart_diff?: RawRestartDiffEntry[]; env_vars?: Record; status: ManagedAgent["status"]; pid: number | null; @@ -158,8 +161,7 @@ export type RawManagedAgent = { auto_restart_on_config_change?: boolean; backend: ManagedAgentBackend; backend_agent_id: string | null; - // Optional: pre-feature mock fixtures may omit these. Mapped to - // `"owner-only"` / `[]` in `fromRawManagedAgent`. + // Pre-feature fixtures may omit these; mapped to "owner-only"/[] in fromRawManagedAgent. respond_to?: ManagedAgent["respondTo"]; respond_to_allowlist?: string[]; }; @@ -707,11 +709,11 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { avatarUrl: agent.avatar_url ?? null, model: agent.model, modelSource: agent.model_source ?? null, - // Fallbacks for pre-feature mocks/fixtures. Real records always carry them. provider: agent.provider ?? null, personaOutOfDate: agent.persona_out_of_date ?? false, personaOrphaned: agent.persona_orphaned ?? false, needsRestart: agent.needs_restart ?? false, + restartDiff: agent.restart_diff ?? [], envVars: agent.env_vars ?? {}, status: agent.status, pid: agent.pid, @@ -727,8 +729,6 @@ export function fromRawManagedAgent(agent: RawManagedAgent): ManagedAgent { autoRestartOnConfigChange: agent.auto_restart_on_config_change ?? true, backend: agent.backend, backendAgentId: agent.backend_agent_id, - // Fallbacks for pre-feature mocks/fixtures that don't carry these fields. - // Real agent records always include them (defaulted server-side). respondTo: agent.respond_to ?? "owner-only", respondToAllowlist: agent.respond_to_allowlist ?? [], }; diff --git a/desktop/src/shared/api/types.ts b/desktop/src/shared/api/types.ts index fd2c71bced..78f5d1aa3f 100644 --- a/desktop/src/shared/api/types.ts +++ b/desktop/src/shared/api/types.ts @@ -304,6 +304,8 @@ export type ManagedAgentBackend = | { type: "local" } | { type: "provider"; id: string; config: Record }; +import type { RestartDiffEntry } from "./restartDiff"; +export type { JsonValue, RestartChange, RestartDiffEntry } from "./restartDiff"; export type ManagedAgent = { pubkey: string; name: string; @@ -357,6 +359,8 @@ export type ManagedAgent = { * Always `false` for stopped agents. */ needsRestart: boolean; + /** Non-empty iff `needsRestart` is true. Empty when Rust omits the field. */ + restartDiff: RestartDiffEntry[]; /** Per-agent env vars. Layered on top of persona envVars. */ envVars: Record; status: "running" | "stopped" | "deployed" | "not_deployed"; @@ -382,11 +386,7 @@ export type ManagedAgent = { respondToAllowlist: string[]; }; -/** - * Inbound author gate mode. Mirrors `buzz-acp`'s `--respond-to` CLI flag. - * `"nobody"` is supported by the harness but not surfaced through this API — - * it's a heartbeat-only mode without a meaningful GUI use case. - */ +/** Inbound author gate mode. Mirrors buzz-acp's --respond-to CLI flag. */ export type RespondToMode = "owner-only" | "allowlist" | "anyone"; export type BackendProviderCandidate = { diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 33b1fb38f4..e430549d43 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -88,6 +88,7 @@ export type MockManagedAgentSeed = { lastError?: string | null; lastErrorCode?: number | null; needsRestart?: boolean; + restartDiff?: Array<{ field: string; change: unknown }>; autoRestartOnConfigChange?: boolean; respondTo?: RawManagedAgent["respond_to"]; respondToAllowlist?: string[]; @@ -826,6 +827,7 @@ type RawManagedAgent = { last_error: string | null; last_error_code: number | null; needs_restart?: boolean; + restart_diff?: Array<{ field: string; change: unknown }>; log_path: string; start_on_app_launch: boolean; auto_restart_on_config_change?: boolean; @@ -1627,6 +1629,7 @@ function cloneManagedAgent(agent: MockManagedAgent): RawManagedAgent { last_error: agent.last_error, last_error_code: agent.last_error_code, needs_restart: agent.needs_restart ?? false, + restart_diff: agent.restart_diff ? [...agent.restart_diff] : [], log_path: agent.log_path, start_on_app_launch: agent.start_on_app_launch, auto_restart_on_config_change: agent.auto_restart_on_config_change ?? true, @@ -2180,6 +2183,7 @@ function buildSeededManagedAgent(seed: MockManagedAgentSeed): MockManagedAgent { last_error: seed.lastError ?? null, last_error_code: seed.lastErrorCode ?? null, needs_restart: seed.needsRestart ?? false, + restart_diff: seed.restartDiff ?? [], log_path: `/tmp/mock-agent-${seed.pubkey}.log`, start_on_app_launch: true, auto_restart_on_config_change: seed.autoRestartOnConfigChange ?? true, diff --git a/desktop/tests/e2e/needs-restart-screenshots.spec.ts b/desktop/tests/e2e/needs-restart-screenshots.spec.ts index 7c836974f7..1a695844fb 100644 --- a/desktop/tests/e2e/needs-restart-screenshots.spec.ts +++ b/desktop/tests/e2e/needs-restart-screenshots.spec.ts @@ -1,11 +1,14 @@ /** - * Screenshot spec for the needsRestart badge and banner (PR #1853). + * Screenshot spec for the needsRestart badge and config diff (PR #1853 + diff + * overlay work). * - * Exercises two surfaces: - * - Agent grid card: warning badge ("Restart required") on standalone and - * persona-backed cards when `needsRestart: true`, absent when false. - * - Profile panel Runtime tab: amber banner with copy branched on the - * per-agent auto-restart toggle. + * Exercises: + * - Agent grid card and list-row badges at all three badge sites. + * - Hover tooltip with itemised before→after diff (capped at 6 + "and N more"). + * - Runtime-tab banner with full uncapped diff list. + * - Side-panel badge visible on the default (Info) tab — not only Runtime. + * - DOM validity: tooltip trigger has no