Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion desktop/src-tauri/src/managed_agents/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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;
Expand Down
15 changes: 8 additions & 7 deletions desktop/src-tauri/src/managed_agents/persona_events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -498,16 +498,17 @@ 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
/// anyway. Idempotent: applying it to an already-current record is a no-op,
/// 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,
Expand Down
4 changes: 2 additions & 2 deletions desktop/src-tauri/src/managed_agents/process_lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<super::AcpAvailabilityStatus>,
start_nonce: String,
Expand All @@ -149,7 +149,7 @@ pub fn finish_spawn(
super::ManagedAgentProcess {
child,
log_path,
spawn_config_hash,
spawn_config,
setup_mode,
adapter_availability,
start_nonce,
Expand Down
2 changes: 1 addition & 1 deletion desktop/src-tauri/src/managed_agents/readiness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
10 changes: 7 additions & 3 deletions desktop/src-tauri/src/managed_agents/restore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ManagedAgentProcess>),
Skipped,
Failed(String),
}
Expand Down Expand Up @@ -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),
}
}
Expand Down Expand Up @@ -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) => {
Expand Down
143 changes: 69 additions & 74 deletions desktop/src-tauri/src/managed_agents/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,49 +226,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).
Expand Down Expand Up @@ -321,6 +322,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(),
Expand All @@ -341,19 +343,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,
Expand Down Expand Up @@ -474,7 +463,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
Expand All @@ -491,8 +480,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 =
Expand Down Expand Up @@ -736,16 +726,16 @@ 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 {
command.env_remove("BUZZ_ACP_TEAM_INSTRUCTIONS");
}

// 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.
//
Expand All @@ -771,8 +761,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 {
Expand Down Expand Up @@ -887,6 +878,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)]
Expand All @@ -912,18 +919,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).
Expand All @@ -946,7 +941,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,
Expand All @@ -956,7 +951,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,
Expand Down
2 changes: 1 addition & 1 deletion desktop/src-tauri/src/managed_agents/runtime/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
42 changes: 7 additions & 35 deletions desktop/src-tauri/src/managed_agents/runtime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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));
}
Loading
Loading