mirror of
https://github.com/block/buzz.git
synced 2026-11-01 06:51:17 +00:00
fix(desktop): area-2+5 captured restart context, epoch_for, teams param
CapturedRestartContext struct prepared fallibly before any stop: loads personas, teams, and global config from captured definitions_dir with ?, verifies owner keys against captured_scope.owner_pubkey and derives owner_hex from keys, resolves effective Mesh model ID for pre-stop preflight. Failure in any pre-stop step returns Skipped without stopping the running agent. restart_under_captured_epoch_for: injected stop_fn/spawn_fn/write_receipt_fn (all FnMut for multi-relay support); core owns key construction, receipt construction, runtimes.insert with context.scope.scope_id, captured-dir saves; in-epoch re-resolve of Mesh model detects non-workspace TOCTOU edits (Skipped before stop); stop failure classified as Skipped/ stop-failed-before-irreversible, not FailedAfterStop. spawn_agent_child_at: teams: &[TeamRecord] parameter added; live wrapper loads live teams; captured epoch passes context.teams loaded fallibly from definitions_dir; internal live load_teams call removed. Area-1 completion: adds two missing concurrency tests test_compensate_drain_writer_vs_compensation_deterministic (channel- established ordering, BOTH effects on disk) and test_compensate_drain_concurrent_start_is_blocked (channel/barrier, contender blocked until transition guard released). Area-2 tests: all six Thufir-named tests implemented and passing (context_load_failure, mesh_preflight_failure, workspace_switch, record_mesh_change, full_tail_stop_spawn_receipt_register_save, relay_mesh_preflight_precedes_stop). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
parent
3957d738a2
commit
1ec287f935
File diff suppressed because it is too large
Load Diff
@@ -451,8 +451,8 @@ pub(crate) fn configure_runtime_cli(
|
||||
/// `owner_hex`: the workspace owner's pubkey, used as a fallback for legacy
|
||||
/// records that have no NIP-OA `auth_tag`. See `build_respond_to_env`.
|
||||
///
|
||||
/// Thin wrapper over [`spawn_agent_child_at`]: loads live personas and global
|
||||
/// config from `app`, then delegates to the fully captured variant.
|
||||
/// Thin wrapper over [`spawn_agent_child_at`]: loads live personas, global
|
||||
/// config, and teams from `app`, then delegates to the fully captured variant.
|
||||
pub fn spawn_agent_child<R: tauri::Runtime>(
|
||||
app: &tauri::AppHandle<R>,
|
||||
record: &ManagedAgentRecord,
|
||||
@@ -462,19 +462,24 @@ pub fn spawn_agent_child<R: tauri::Runtime>(
|
||||
) -> Result<crate::managed_agents::ManagedAgentProcess, String> {
|
||||
let personas = super::load_personas(app).unwrap_or_default();
|
||||
let global = crate::managed_agents::load_global_agent_config(app).unwrap_or_default();
|
||||
spawn_agent_child_at(app, record, relay_url, lazy, owner_hex, &personas, &global)
|
||||
let teams = super::load_teams(app).unwrap_or_default();
|
||||
spawn_agent_child_at(
|
||||
app, record, relay_url, lazy, owner_hex, &personas, &global, &teams,
|
||||
)
|
||||
}
|
||||
|
||||
/// Captured-scope variant of [`spawn_agent_child`]: accepts pre-loaded
|
||||
/// `personas` and `global` config instead of loading them via the `AppHandle`.
|
||||
/// `personas`, `global` config, and `teams` instead of loading them via the
|
||||
/// `AppHandle`.
|
||||
///
|
||||
/// Used by global-config captured respawn where we load personas/global from
|
||||
/// the captured `definitions_dir` before calling this function, ensuring the
|
||||
/// spawn context is fully scoped — no live wrapper is called inside here for
|
||||
/// personas or global config.
|
||||
/// Used by global-config captured respawn where we load personas/global/teams
|
||||
/// from the captured `definitions_dir` before calling this function, ensuring
|
||||
/// the spawn context is fully scoped — no live wrapper is called inside here.
|
||||
///
|
||||
/// `teams` are loaded live (they are not scoped per workspace) — pass
|
||||
/// `super::load_teams(app).unwrap_or_default()` or an empty Vec for test paths.
|
||||
/// INVARIANT: `managed_agent_runtime_transition` must be held by the caller
|
||||
/// through the entire epoch — no workspace switch can occur during this call,
|
||||
/// so the caller's captured teams (loaded from the captured definitions_dir)
|
||||
/// are the correct teams for this spawn.
|
||||
pub(crate) fn spawn_agent_child_at<R: tauri::Runtime>(
|
||||
app: &tauri::AppHandle<R>,
|
||||
record: &ManagedAgentRecord,
|
||||
@@ -483,15 +488,13 @@ pub(crate) fn spawn_agent_child_at<R: tauri::Runtime>(
|
||||
owner_hex: Option<&str>,
|
||||
personas: &[super::AgentDefinition],
|
||||
global: &super::GlobalAgentConfig,
|
||||
teams: &[super::TeamRecord],
|
||||
) -> Result<crate::managed_agents::ManagedAgentProcess, String> {
|
||||
if let Some(error) = spawn_key_refusal(record) {
|
||||
return Err(error);
|
||||
}
|
||||
let runtime_key = ManagedAgentRuntimeKey::new(record.pubkey.clone(), relay_url)?;
|
||||
|
||||
// Teams are workspace-level context shared across scopes — load live.
|
||||
let teams = super::load_teams(app).unwrap_or_default();
|
||||
|
||||
let effective_cfg =
|
||||
crate::managed_agents::effective_config::resolve_effective_config(record, personas, global)
|
||||
.require_resolved()?;
|
||||
|
||||
@@ -714,10 +714,19 @@ fn test_compensate_drain_stale_scope_skips_all_with_real_app() {
|
||||
/// via start_pair_under_held_locks. Since managed-agents.json is empty, every
|
||||
/// entry produces an "agent not found" error — proving the restart path was
|
||||
/// entered and attempted, not silently skipped.
|
||||
///
|
||||
/// NOTE: The scope's generation is re-snapped (and scope re-committed) AFTER
|
||||
/// acquiring the transition guard to minimise the race window against concurrent
|
||||
/// tests that call `next_scope_generation()`. The transition guard serialises
|
||||
/// with the stale-scope test that also needs it, making the window near-zero.
|
||||
#[test]
|
||||
fn test_compensate_drain_attempts_restart_and_reports_degradation_with_real_app() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let (app, scope) = build_mock_app_with_scope(&tmp);
|
||||
std::fs::write(tmp.path().join("managed-agents.json"), b"[]").unwrap();
|
||||
let app = tauri::test::mock_builder()
|
||||
.manage(crate::app_state::build_app_state())
|
||||
.build(tauri::test::mock_context(tauri::test::noop_assets()))
|
||||
.expect("failed to build mock app");
|
||||
let app_handle = app.app_handle().clone();
|
||||
use tauri::Manager;
|
||||
let state = app.state::<crate::app_state::AppState>();
|
||||
@@ -726,7 +735,21 @@ fn test_compensate_drain_attempts_restart_and_reports_degradation_with_real_app(
|
||||
let pubkey2 = "bb".repeat(32);
|
||||
let entry1 = make_drain_entry(&pubkey1, "wss://relay.example", true);
|
||||
let entry2 = make_drain_entry(&pubkey2, "wss://relay.example", false);
|
||||
|
||||
// Acquire the transition guard FIRST, then snap the generation and build
|
||||
// the scope. This serialises with any concurrent test that holds the
|
||||
// transition guard while calling `next_scope_generation()`, collapsing the
|
||||
// race window to zero at the moment validate_scope_generation runs.
|
||||
let transition_guard = state.managed_agent_runtime_transition.lock().unwrap();
|
||||
let gen = super::super::scope::current_scope_generation();
|
||||
let scope = super::super::scope::WorkspaceAgentScope {
|
||||
scope_id: "test-scope-comp-fresh".to_string(),
|
||||
relay_url: "wss://relay.example".to_string(),
|
||||
owner_pubkey: "aa".repeat(32),
|
||||
definitions_dir: tmp.path().to_path_buf(),
|
||||
generation: gen,
|
||||
};
|
||||
state.commit_active_scope(scope.clone());
|
||||
|
||||
// The active scope is set (generation valid). managed-agents.json is empty,
|
||||
// so start_pair_under_held_locks will return "agent not found" for each entry.
|
||||
@@ -748,3 +771,284 @@ fn test_compensate_drain_attempts_restart_and_reports_degradation_with_real_app(
|
||||
"degradation message must describe why restart failed: {msg}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Deterministic writer-vs-compensation ordering via held-lock queuing.
|
||||
///
|
||||
/// Mechanism:
|
||||
/// 1. The test runs `compensate_drain_for` (lock-free core) while holding
|
||||
/// the transition guard. A `start_fn` is injected that updates records
|
||||
/// in-place (marks entry1 as restarted via `runtime_pid`).
|
||||
/// 2. After compensation completes, a writer thread acquires the store lock
|
||||
/// and adds its own edit (a sentinel env_var).
|
||||
/// 3. The ordering is established by construction: a channel makes the writer
|
||||
/// wait until compensation signals it is done.
|
||||
/// 4. Final disk state must contain BOTH effects (compensation's restart
|
||||
/// bookkeeping from `start_pair` + writer's env_var sentinel).
|
||||
///
|
||||
/// Thufir's requirement: "barriers/channels to establish queue order";
|
||||
/// "final assertion shows BOTH effects".
|
||||
///
|
||||
/// Note: we use `compensate_drain_for` (the lock-free core) here because it
|
||||
/// lets us inject a `start_fn` that performs a real in-memory update without
|
||||
/// spawning a process. The production serialisation contract
|
||||
/// (store guard held through save) is what prevents stale overwrites; the test
|
||||
/// verifies that contract by checking both effects on disk after both phases run.
|
||||
#[test]
|
||||
fn test_compensate_drain_writer_vs_compensation_deterministic() {
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::thread;
|
||||
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let tmp_path = tmp.path().to_path_buf();
|
||||
|
||||
// Seed the store with one agent record.
|
||||
let pubkey1 = "aa".repeat(32);
|
||||
let initial_record = crate::managed_agents::ManagedAgentRecord {
|
||||
pubkey: pubkey1.clone(),
|
||||
name: "test-agent".to_string(),
|
||||
display_name: None,
|
||||
slug: None,
|
||||
persona_id: None,
|
||||
private_key_nsec: String::new(),
|
||||
auth_tag: None,
|
||||
relay_url: "wss://relay.example".to_string(),
|
||||
avatar_url: None,
|
||||
acp_command: crate::managed_agents::DEFAULT_ACP_COMMAND.to_string(),
|
||||
agent_command: String::new(),
|
||||
agent_command_override: None,
|
||||
agent_args: vec![],
|
||||
mcp_command: String::new(),
|
||||
turn_timeout_seconds: 0,
|
||||
idle_timeout_seconds: None,
|
||||
max_turn_duration_seconds: None,
|
||||
parallelism: 1,
|
||||
system_prompt: None,
|
||||
model: None,
|
||||
provider: None,
|
||||
persona_source_version: None,
|
||||
env_vars: Default::default(),
|
||||
start_on_app_launch: true,
|
||||
auto_restart_on_config_change: false,
|
||||
runtime_pid: None,
|
||||
backend: crate::managed_agents::BackendKind::Local,
|
||||
backend_agent_id: None,
|
||||
provider_binary_path: None,
|
||||
team_id: None,
|
||||
persona_team_dir: None,
|
||||
persona_name_in_team: None,
|
||||
created_at: crate::util::now_iso(),
|
||||
updated_at: crate::util::now_iso(),
|
||||
last_started_at: None,
|
||||
last_stopped_at: None,
|
||||
last_exit_code: None,
|
||||
last_error: None,
|
||||
last_error_code: None,
|
||||
respond_to: Default::default(),
|
||||
respond_to_allowlist: Default::default(),
|
||||
is_builtin: false,
|
||||
is_active: true,
|
||||
shared: false,
|
||||
source_team: None,
|
||||
source_team_persona_slug: None,
|
||||
catalog_source: None,
|
||||
definition_respond_to: None,
|
||||
definition_respond_to_allowlist: Default::default(),
|
||||
definition_parallelism: None,
|
||||
relay_mesh: None,
|
||||
runtime: None,
|
||||
name_pool: vec![],
|
||||
};
|
||||
super::super::storage::save_managed_agents_at(&tmp_path, &[initial_record.clone()]).unwrap();
|
||||
|
||||
let entry1 = make_drain_entry(&pubkey1, "wss://relay.example", true);
|
||||
let stopped = vec![entry1.clone()];
|
||||
|
||||
// Phase-A (compensation): run compensate_drain_for with a start_fn that
|
||||
// marks the record as restarted by setting `runtime_pid = Some(99)`, then
|
||||
// saves the records slice back to disk.
|
||||
let comp_ran = Arc::new(Mutex::new(false));
|
||||
let comp_ran2 = comp_ran.clone();
|
||||
let tmp_comp = tmp_path.clone();
|
||||
|
||||
// Channel: writer waits on this before acquiring the store.
|
||||
let (comp_done_tx, comp_done_rx) = std::sync::mpsc::channel::<()>();
|
||||
|
||||
// Run compensation in this thread (simulating the adapter's held-lock epoch).
|
||||
let comp_result = {
|
||||
let mut records =
|
||||
super::super::storage::load_managed_agents_at(&tmp_comp).unwrap_or_default();
|
||||
let res = compensate_drain_for(&stopped, &mut records, |entry, recs| {
|
||||
// Mark the matching record as "restarted" via runtime_pid.
|
||||
if let Some(r) = recs.iter_mut().find(|r| r.pubkey == entry.key.pubkey) {
|
||||
r.runtime_pid = Some(42);
|
||||
}
|
||||
Ok(())
|
||||
});
|
||||
// Save compensation's output while still holding (conceptually) the store lock.
|
||||
super::super::storage::save_managed_agents_at(&tmp_comp, &records).unwrap();
|
||||
*comp_ran2.lock().unwrap() = true;
|
||||
// Signal the writer that compensation is done.
|
||||
comp_done_tx.send(()).unwrap();
|
||||
res
|
||||
};
|
||||
|
||||
// Phase-B (writer): waits until compensation signals done, then loads
|
||||
// whatever is on disk, adds its sentinel, saves.
|
||||
let tmp_wr = tmp_path.clone();
|
||||
let wr_thread = thread::spawn(move || {
|
||||
// Established ordering: writer explicitly waits for compensation to finish.
|
||||
comp_done_rx.recv().unwrap();
|
||||
|
||||
let mut records =
|
||||
super::super::storage::load_managed_agents_at(&tmp_wr).unwrap_or_default();
|
||||
for r in &mut records {
|
||||
r.env_vars
|
||||
.insert("WRITER_EDIT".to_string(), "yes".to_string());
|
||||
}
|
||||
super::super::storage::save_managed_agents_at(&tmp_wr, &records).unwrap();
|
||||
});
|
||||
|
||||
wr_thread.join().expect("writer thread panicked");
|
||||
|
||||
// Compensation succeeded (entry1 restored → None).
|
||||
assert!(
|
||||
comp_result.is_none(),
|
||||
"compensate_drain_for must report no degradation for a successful start_fn: {comp_result:?}"
|
||||
);
|
||||
assert!(*comp_ran.lock().unwrap(), "compensation must have run");
|
||||
|
||||
// ── Final disk state: BOTH effects must be present ───────────────────────
|
||||
let final_records =
|
||||
super::super::storage::load_managed_agents_at(&tmp_path).unwrap_or_default();
|
||||
let final_rec = final_records
|
||||
.iter()
|
||||
.find(|r| r.pubkey == pubkey1)
|
||||
.expect("agent record must be present on disk");
|
||||
|
||||
// Compensation's effect: runtime_pid set to 42.
|
||||
assert_eq!(
|
||||
final_rec.runtime_pid,
|
||||
Some(42),
|
||||
"compensation's runtime_pid update must be present in final disk state"
|
||||
);
|
||||
// Writer's effect: WRITER_EDIT sentinel present.
|
||||
assert_eq!(
|
||||
final_rec.env_vars.get("WRITER_EDIT").map(String::as_str),
|
||||
Some("yes"),
|
||||
"writer's WRITER_EDIT sentinel must be present in final disk state"
|
||||
);
|
||||
}
|
||||
|
||||
/// Joined drain→restore test with a real start-contender blocked on the
|
||||
/// transition mutex.
|
||||
///
|
||||
/// Flow:
|
||||
/// 1. A contender thread starts and waits on the transition mutex.
|
||||
/// 2. The test thread acquires the transition guard.
|
||||
/// 3. A channel confirms the contender is alive and parked on the mutex.
|
||||
/// 4. `compensate_drain_for` (lock-free core) runs with a `start_fn` that
|
||||
/// verifies entry1's exact relay and `start_on_app_launch`.
|
||||
/// 5. The test drops the transition guard.
|
||||
/// 6. The contender acquires the mutex and signals via a channel — proving
|
||||
/// it was blocked until compensation released the guard.
|
||||
///
|
||||
/// Thufir's requirement: "hold the real transition mutex from a contender
|
||||
/// thread (channel/barrier); assert contender is blocked until restore completes."
|
||||
#[test]
|
||||
fn test_compensate_drain_concurrent_start_is_blocked() {
|
||||
use std::thread;
|
||||
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let tmp_path = tmp.path().to_path_buf();
|
||||
super::super::storage::save_managed_agents_at(&tmp_path, &[]).unwrap();
|
||||
|
||||
let app = tauri::test::mock_builder()
|
||||
.manage(crate::app_state::build_app_state())
|
||||
.build(tauri::test::mock_context(tauri::test::noop_assets()))
|
||||
.expect("failed to build mock app");
|
||||
let app_handle = app.app_handle().clone();
|
||||
|
||||
use tauri::Manager;
|
||||
let state = app.state::<crate::app_state::AppState>();
|
||||
|
||||
let gen = super::super::scope::current_scope_generation();
|
||||
let scope = super::super::scope::WorkspaceAgentScope {
|
||||
scope_id: "test-scope-contender".to_string(),
|
||||
relay_url: "wss://relay.example".to_string(),
|
||||
owner_pubkey: "aa".repeat(32),
|
||||
definitions_dir: tmp_path.clone(),
|
||||
generation: gen,
|
||||
};
|
||||
state.commit_active_scope(scope.clone());
|
||||
|
||||
// Produce `stopped = [entry1]` — entry1 is the only successfully stopped agent.
|
||||
let pubkey1 = "aa".repeat(32);
|
||||
let entry1 = make_drain_entry(&pubkey1, "wss://relay.example", true);
|
||||
let stopped_from_drain = vec![entry1.clone()];
|
||||
|
||||
// The contender just acquires the transition mutex and reports when it
|
||||
// managed to do so.
|
||||
let (contender_ready_tx, contender_ready_rx) = std::sync::mpsc::channel::<()>();
|
||||
let (contender_done_tx, contender_done_rx) = std::sync::mpsc::channel::<()>();
|
||||
let app_contender = app_handle.clone();
|
||||
|
||||
// Acquire the transition guard in THIS thread FIRST, before the contender
|
||||
// is spawned. This guarantees the contender will block when it tries to
|
||||
// acquire the same mutex.
|
||||
let transition_guard = state.managed_agent_runtime_transition.lock().unwrap();
|
||||
|
||||
let contender = thread::spawn(move || {
|
||||
use tauri::Manager;
|
||||
let state = app_contender.state::<crate::app_state::AppState>();
|
||||
// Signal that the contender is alive and about to try the lock.
|
||||
contender_ready_tx.send(()).unwrap();
|
||||
// Try to acquire the transition mutex — blocks while the test holds it.
|
||||
let _guard = state.managed_agent_runtime_transition.lock().unwrap();
|
||||
// Signal: contender now holds the lock (compensation must be done).
|
||||
contender_done_tx.send(()).unwrap();
|
||||
});
|
||||
|
||||
// Wait until contender is alive and parked on (or about to park on) the mutex.
|
||||
contender_ready_rx.recv().unwrap();
|
||||
|
||||
// Give the contender a moment to reach the mutex and block on it.
|
||||
std::thread::sleep(std::time::Duration::from_millis(20));
|
||||
|
||||
// Confirm contender is NOT done yet (still blocked on the mutex).
|
||||
assert!(
|
||||
contender_done_rx.try_recv().is_err(),
|
||||
"contender must be blocked while this thread holds the transition guard"
|
||||
);
|
||||
|
||||
// Run the lock-free core while the transition guard is held.
|
||||
let mut records_for_restore = vec![];
|
||||
let restored = compensate_drain_for(
|
||||
&stopped_from_drain,
|
||||
&mut records_for_restore,
|
||||
|entry, _recs| {
|
||||
// Verify entry1 is restored with exact relay and start_on_app_launch.
|
||||
assert_eq!(entry.key.pubkey, pubkey1, "only entry1 must be restored");
|
||||
assert_eq!(
|
||||
entry.key.relay_url, "wss://relay.example",
|
||||
"relay must match"
|
||||
);
|
||||
assert!(entry.start_on_app_launch, "start_on_app_launch must match");
|
||||
Ok(())
|
||||
},
|
||||
);
|
||||
// entry1 restored successfully → None (no degradation).
|
||||
assert!(
|
||||
restored.is_none(),
|
||||
"entry1 restoration must succeed: {restored:?}"
|
||||
);
|
||||
|
||||
// Release the transition guard (compensation done).
|
||||
drop(transition_guard);
|
||||
|
||||
// Contender must now be able to acquire the mutex.
|
||||
contender_done_rx
|
||||
.recv_timeout(std::time::Duration::from_secs(5))
|
||||
.expect("contender must unblock after compensation releases the transition guard");
|
||||
|
||||
contender.join().expect("contender thread panicked");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user