mirror of
https://github.com/block/buzz.git
synced 2026-10-29 05:21:31 +00:00
test(desktop): prove revocation at real prior-session teardown
Use the existing marker-observable test child with TERM readiness and successful revocation exit; reap the simulated previous-session child concurrently. Assert receipt ownership and revocation before cleanup, retaining local Start refusal and restore no-publish. Removing only the shared fresh admission reload makes this regression fail after actual revocation. No production credential authority changes. Repair native evidence gates: cover runtime_configuration in the snapshot mutation table, compare physical shell PWD on macOS, and isolate synchronous login-shell probe counts from parallel callers with a counter regression. Apply native rustfmt to integrated F6. All 13 no-default credential cases, full native workspace, clippy -D warnings and nested fmt pass. Signed-off-by: Logan Johnson <loganj@squareup.com>
This commit is contained in:
@@ -505,8 +505,9 @@ async fn restart_single_agent_after_install(
|
||||
managed_agents::{
|
||||
agent_readiness, current_instance_id, find_managed_agent_mut, known_acp_runtime,
|
||||
load_global_agent_config, load_managed_agents, load_personas, record_agent_command,
|
||||
resolve_effective_agent_env, storage::save_runtime_metadata_batch, stop_managed_agent_process,
|
||||
sync_managed_agent_processes, AgentReadiness, BackendKind,
|
||||
resolve_effective_agent_env, stop_managed_agent_process,
|
||||
storage::save_runtime_metadata_batch, sync_managed_agent_processes, AgentReadiness,
|
||||
BackendKind,
|
||||
},
|
||||
};
|
||||
use tauri::Manager;
|
||||
@@ -644,7 +645,9 @@ fn persist_last_error_on_install(
|
||||
) -> Result<(), String> {
|
||||
use crate::{
|
||||
app_state::AppState,
|
||||
managed_agents::{find_managed_agent_mut, load_managed_agents, storage::save_runtime_metadata_batch},
|
||||
managed_agents::{
|
||||
find_managed_agent_mut, load_managed_agents, storage::save_runtime_metadata_batch,
|
||||
},
|
||||
};
|
||||
use tauri::Manager;
|
||||
let state = app.state::<AppState>();
|
||||
|
||||
@@ -343,7 +343,8 @@ fn record_field_updates_persist_effort_to_disk() {
|
||||
"last_stopped_at": null, "last_exit_code": null, "last_error": null
|
||||
}))
|
||||
.unwrap();
|
||||
crate::managed_agents::storage::save_managed_agents_with_new_keys(app.handle(), &[seed]).unwrap();
|
||||
crate::managed_agents::storage::save_managed_agents_with_new_keys(app.handle(), &[seed])
|
||||
.unwrap();
|
||||
|
||||
// Drive the production seam: load → apply_record_field_updates →
|
||||
// stamp_record_updated_at → save. This is the exact sequence that
|
||||
|
||||
@@ -18,8 +18,8 @@ use crate::{
|
||||
managed_agents::{
|
||||
agent_readiness, current_instance_id, find_managed_agent_mut, known_acp_runtime,
|
||||
load_global_agent_config, load_managed_agents, load_personas, record_agent_command,
|
||||
resolve_effective_agent_env, save_global_agent_config, storage::save_runtime_metadata_batch,
|
||||
stop_managed_agent_process, sync_managed_agent_processes, validate_global_config,
|
||||
resolve_effective_agent_env, save_global_agent_config, stop_managed_agent_process,
|
||||
storage::save_runtime_metadata_batch, sync_managed_agent_processes, validate_global_config,
|
||||
AgentReadiness, BackendKind, GlobalAgentConfig,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -18,8 +18,9 @@ use crate::{
|
||||
decrypt_envelope, parse_chunk_payload, resolve_unlock_secret, ChunkPayload,
|
||||
LOCKED_CARD_REFUSAL,
|
||||
},
|
||||
load_managed_agents, load_personas, storage::save_managed_agents_with_new_keys, save_personas, AgentDefinition,
|
||||
ManagedAgentRecord, RespondTo,
|
||||
load_managed_agents, load_personas, save_personas,
|
||||
storage::save_managed_agents_with_new_keys,
|
||||
AgentDefinition, ManagedAgentRecord, RespondTo,
|
||||
},
|
||||
relay::{effective_agent_relay_url, relay_ws_url_with_override},
|
||||
util::now_iso,
|
||||
|
||||
@@ -17,8 +17,10 @@ use crate::{
|
||||
},
|
||||
managed_agents::{
|
||||
agent_snapshot::{build_snapshot, AgentSnapshot, AgentSnapshotMemoryEntry, MemoryLevel},
|
||||
load_managed_agents, load_personas, load_teams, load_teams_readonly, storage::save_managed_agents_with_new_keys,
|
||||
save_personas, save_teams, AgentDefinition, ManagedAgentRecord, TeamRecord,
|
||||
load_managed_agents, load_personas, load_teams, load_teams_readonly, save_personas,
|
||||
save_teams,
|
||||
storage::save_managed_agents_with_new_keys,
|
||||
AgentDefinition, ManagedAgentRecord, TeamRecord,
|
||||
},
|
||||
relay::{effective_agent_relay_url, relay_ws_url_with_override, sync_managed_agent_profile},
|
||||
util::now_iso,
|
||||
|
||||
@@ -1,21 +1,40 @@
|
||||
//! Test-only counter for login-shell spawn attempts.
|
||||
//!
|
||||
//! `run_in_login_shell` is the single subprocess-spawning step on the
|
||||
//! absent-command resolution path, so counting its calls proves whether a
|
||||
//! cheap discovery re-spawns after a negative resolution was cached.
|
||||
//! `run_in_login_shell` and the discovery calls under test are synchronous.
|
||||
//! Count the calling thread's attempts, not unrelated parallel tests' probes;
|
||||
//! the process-environment lock does not serialize every resolver caller.
|
||||
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::cell::Cell;
|
||||
|
||||
static COUNT: AtomicUsize = AtomicUsize::new(0);
|
||||
thread_local! {
|
||||
static COUNT: Cell<usize> = const { Cell::new(0) };
|
||||
}
|
||||
|
||||
pub(crate) fn record() {
|
||||
COUNT.fetch_add(1, Ordering::SeqCst);
|
||||
COUNT.with(|count| count.set(count.get() + 1));
|
||||
}
|
||||
|
||||
pub(crate) fn reset() {
|
||||
COUNT.store(0, Ordering::SeqCst);
|
||||
COUNT.with(|count| count.set(0));
|
||||
}
|
||||
|
||||
pub(crate) fn count() -> usize {
|
||||
COUNT.load(Ordering::SeqCst)
|
||||
COUNT.with(Cell::get)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn counts_this_synchronous_probe_not_parallel_callers() {
|
||||
reset();
|
||||
record();
|
||||
std::thread::spawn(|| {
|
||||
assert_eq!(count(), 0);
|
||||
record();
|
||||
record();
|
||||
assert_eq!(count(), 2);
|
||||
reset();
|
||||
})
|
||||
.join()
|
||||
.unwrap();
|
||||
assert_eq!(count(), 1);
|
||||
reset();
|
||||
}
|
||||
|
||||
@@ -117,6 +117,24 @@ fn marked_child_process_fixture() {
|
||||
}
|
||||
let ready_path = std::env::var_os(MARKED_CHILD_READY_ENV)
|
||||
.expect("marked child fixture requires a readiness path");
|
||||
// Optional lifecycle-fixture mode. Install TERM handling before readiness;
|
||||
// mutate only the explicitly supplied temporary store, then exit normally.
|
||||
// This runs in the dedicated child executable, never the parent test runner.
|
||||
if let Some(source) = std::env::var_os("BUZZ_TEST_MARKED_CHILD_REVOKED_STORE") {
|
||||
let destination = std::env::var_os("BUZZ_TEST_MARKED_CHILD_STORE")
|
||||
.expect("revoking child requires a destination store");
|
||||
let (send, receive) = std::sync::mpsc::channel();
|
||||
ctrlc::set_handler(move || {
|
||||
let _ = send.send(());
|
||||
})
|
||||
.expect("install marked child TERM handler");
|
||||
std::fs::write(ready_path, b"ready").expect("write marked child readiness handshake");
|
||||
receive
|
||||
.recv_timeout(std::time::Duration::from_secs(30))
|
||||
.expect("marked child must be stopped within fixture deadline");
|
||||
std::fs::copy(source, destination).expect("revoke temporary inline credential");
|
||||
return;
|
||||
}
|
||||
std::fs::write(ready_path, b"ready").expect("write marked child readiness handshake");
|
||||
loop {
|
||||
std::thread::park_timeout(std::time::Duration::from_secs(60));
|
||||
|
||||
@@ -5,10 +5,10 @@ use tauri::{AppHandle, Emitter, Manager};
|
||||
use super::{
|
||||
agent_readiness, current_instance_id, find_managed_agent_mut, load_global_agent_config,
|
||||
load_managed_agents, load_personas, managed_agent_runtime_log_path, process_is_running,
|
||||
record_agent_command, resolve_effective_agent_env, storage::save_runtime_metadata_batch, terminate_process,
|
||||
terminate_untracked_pair_runtime, write_agent_runtime_receipt, AgentReadiness, BackendKind,
|
||||
ManagedAgentPairRuntime, ManagedAgentRuntimeKey, ManagedAgentRuntimeLifecycle,
|
||||
ManagedAgentRuntimeReceipt, ManagedAgentRuntimeStatus,
|
||||
record_agent_command, resolve_effective_agent_env, storage::save_runtime_metadata_batch,
|
||||
terminate_process, terminate_untracked_pair_runtime, write_agent_runtime_receipt,
|
||||
AgentReadiness, BackendKind, ManagedAgentPairRuntime, ManagedAgentRuntimeKey,
|
||||
ManagedAgentRuntimeLifecycle, ManagedAgentRuntimeReceipt, ManagedAgentRuntimeStatus,
|
||||
};
|
||||
use crate::app_state::AppState;
|
||||
|
||||
@@ -1033,7 +1033,8 @@ mod stop_scope_tests {
|
||||
);
|
||||
record.pubkey = pubkey.clone();
|
||||
record.updated_at = "before".into();
|
||||
super::super::storage::save_managed_agents_with_new_keys(app.handle(), &[record.clone()]).unwrap();
|
||||
super::super::storage::save_managed_agents_with_new_keys(app.handle(), &[record.clone()])
|
||||
.unwrap();
|
||||
|
||||
let stored_key = super::ManagedAgentRuntimeKey::new(&pubkey, stored_relay).unwrap();
|
||||
let requested_key = super::ManagedAgentRuntimeKey::new(&pubkey, requested_relay).unwrap();
|
||||
|
||||
@@ -415,8 +415,10 @@ async fn shared_spawn_registers_exact_plan_and_rejects_edits_or_lost_identity()
|
||||
let mut runtimes = std::collections::HashMap::new();
|
||||
let path = agents::storage::managed_agents_store_path(app.handle()).unwrap();
|
||||
agents::storage::atomic_write_json_restricted(
|
||||
&path, &serde_json::to_vec(&[record.clone()]).unwrap(),
|
||||
).unwrap();
|
||||
&path,
|
||||
&serde_json::to_vec(&[record.clone()]).unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
agents::start_managed_agent_process_prepared(
|
||||
app.handle(),
|
||||
&mut record,
|
||||
@@ -439,7 +441,9 @@ async fn shared_spawn_registers_exact_plan_and_rejects_edits_or_lost_identity()
|
||||
std::fs::read_to_string(capture).unwrap(),
|
||||
format!(
|
||||
"fixture-model\nfixture-model\nopenai\nfixture-model\n{}\nprepared team instructions\nchannel\n{}\n\n",
|
||||
temp.path().display(),
|
||||
// The shell reports physical PWD (/private/var on macOS), not
|
||||
// the symlink spelling returned by the temporary directory API.
|
||||
temp.path().canonicalize().unwrap().display(),
|
||||
mcp_path.display()
|
||||
)
|
||||
);
|
||||
|
||||
@@ -143,20 +143,45 @@ fn inline_revocation_during_ordinary_stop_survives_same_host_start() {
|
||||
let mut revoked = raw(&fixture);
|
||||
revoked.private_key_nsec.clear();
|
||||
std::fs::write(&revoked_path, serde_json::to_vec(&[revoked]).unwrap()).unwrap();
|
||||
on_stop(
|
||||
&fixture,
|
||||
&format!(
|
||||
"/bin/cp \"{}\" \"{}\"; exit 0",
|
||||
revoked_path.display(),
|
||||
path.display()
|
||||
),
|
||||
);
|
||||
// System shell/sleep environments are not observable through the
|
||||
// macOS ownership reader. Exec our existing marked child seam so
|
||||
// the prior-session receipt is genuinely eligible for teardown.
|
||||
let ready = fixture.temp.path().join("revoking-child-ready");
|
||||
let script = std::fs::read_to_string(&fixture.record.acp_command).unwrap();
|
||||
let script = script.replace("exec /bin/sleep 20", &format!(
|
||||
"export BUZZ_TEST_MARKED_CHILD_FIXTURE=1\nexport BUZZ_TEST_MARKED_CHILD_READY='{}'\nexport BUZZ_TEST_MARKED_CHILD_REVOKED_STORE='{}'\nexport BUZZ_TEST_MARKED_CHILD_STORE='{}'\nexec '{}' --exact managed_agents::runtime::test_fixtures::marked_child_process_fixture --nocapture",
|
||||
ready.display(), revoked_path.display(), path.display(),
|
||||
std::env::current_exe().unwrap().display(),
|
||||
));
|
||||
std::fs::write(&fixture.record.acp_command, script).unwrap();
|
||||
assert_eq!(
|
||||
fixture.action(Action::Start, None).await.outcome,
|
||||
Outcome::Running
|
||||
);
|
||||
fixture.launched("fixture-model|fixture-model");
|
||||
let pid = fixture.running().unwrap().0;
|
||||
let _child_guard = agents::runtime::test_fixtures::MarkedProcessGuard::new(pid);
|
||||
for _ in 0..100 {
|
||||
if ready.is_file() {
|
||||
break;
|
||||
}
|
||||
std::thread::sleep(std::time::Duration::from_millis(20));
|
||||
}
|
||||
assert!(
|
||||
ready.is_file(),
|
||||
"TERM handler must be ready before teardown"
|
||||
);
|
||||
let receipts = agents::read_all_agent_runtime_receipts(fixture.app.handle());
|
||||
assert_eq!(receipts.len(), 1);
|
||||
assert_eq!(receipts[0].1.pid, pid);
|
||||
assert!(
|
||||
agents::valid_agent_runtime_receipt(
|
||||
&receipts[0].0,
|
||||
&receipts[0].1,
|
||||
&agents::current_instance_id(fixture.app.handle()),
|
||||
),
|
||||
"synthetic prior-session child must prove production ownership"
|
||||
);
|
||||
if launch == "signed-stop" {
|
||||
assert_eq!(fixture.stop().await, StopOutcome::Stopped);
|
||||
} else {
|
||||
@@ -172,8 +197,12 @@ fn inline_revocation_during_ordinary_stop_survives_same_host_start() {
|
||||
.unwrap()
|
||||
.remove(&key)
|
||||
.unwrap();
|
||||
if launch == "local-start" {
|
||||
assert!(crate::commands::start_local_agent_with_preflight_using(
|
||||
// A previous Desktop's child is reaped by its new parent. Keep
|
||||
// that behavior here: retaining an unreaped Child would turn a
|
||||
// successful TERM into a zombie and falsely fail teardown.
|
||||
let reaper = std::thread::spawn(move || old_runtime.child.wait().unwrap());
|
||||
let refused = if launch == "local-start" {
|
||||
crate::commands::start_local_agent_with_preflight_using(
|
||||
fixture.app.handle(),
|
||||
&state,
|
||||
&fixture.record.pubkey,
|
||||
@@ -185,7 +214,7 @@ fn inline_revocation_during_ordinary_stop_survives_same_host_start() {
|
||||
|_, _| async { Ok(()) },
|
||||
)
|
||||
.await
|
||||
.is_err());
|
||||
.is_err()
|
||||
} else {
|
||||
agents::restore_with(
|
||||
fixture.app.handle(),
|
||||
@@ -196,10 +225,17 @@ fn inline_revocation_during_ordinary_stop_survives_same_host_start() {
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
}
|
||||
// Reap the synthetic child even though Desktop no longer tracks it.
|
||||
let _ = agents::terminate_process(old_runtime.child.id());
|
||||
let _ = old_runtime.child.wait();
|
||||
true
|
||||
};
|
||||
// Observe the actual boundary BEFORE cleanup. Refusal due only
|
||||
// to unsuccessful teardown must not count as key revocation.
|
||||
let exited = !agents::process_is_running(pid);
|
||||
let revoked = raw(&fixture).private_key_nsec.is_empty();
|
||||
let _ = agents::terminate_process(pid);
|
||||
assert!(reaper.join().unwrap().success(), "TERM effect must finish");
|
||||
assert!(exited, "ordinary teardown must reap the prior child");
|
||||
assert!(revoked, "revocation must precede fresh admission");
|
||||
assert!(refused, "post-teardown revoked local Start must refuse");
|
||||
}
|
||||
assert!(!agents::process_is_running(pid));
|
||||
assert!(raw(&fixture).private_key_nsec.is_empty());
|
||||
|
||||
@@ -89,14 +89,20 @@ impl Fixture {
|
||||
fixture
|
||||
}
|
||||
fn persist(&self) {
|
||||
agents::storage::save_managed_agents_with_new_keys(self.app.handle(), &[self.record.clone()]).unwrap();
|
||||
agents::storage::save_managed_agents_with_new_keys(
|
||||
self.app.handle(),
|
||||
&[self.record.clone()],
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
// Simulate an independent destination credential writer, not a metadata edit.
|
||||
fn write_raw(&self, record: &ManagedAgentRecord) {
|
||||
let path = agents::storage::managed_agents_store_path(self.app.handle()).unwrap();
|
||||
agents::storage::atomic_write_json_restricted(
|
||||
&path, &serde_json::to_vec(&[record]).unwrap(),
|
||||
).unwrap();
|
||||
&path,
|
||||
&serde_json::to_vec(&[record]).unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
fn target(&self) -> StopTarget {
|
||||
StopTarget {
|
||||
|
||||
@@ -8,7 +8,12 @@ const RELAY_WITH_TOKEN: &str = "wss://relay.example/ws?token=SENTINEL";
|
||||
/// coverage guard below sees the full serialized key set.
|
||||
fn base() -> SpawnConfigSnapshot {
|
||||
SpawnConfigSnapshot {
|
||||
runtime_configuration: None,
|
||||
runtime_configuration: Some(
|
||||
crate::managed_agents::runtime_configurations::RuntimeConfigurationRef {
|
||||
id: "fixture-configuration".into(),
|
||||
revision: "fixture-revision".into(),
|
||||
},
|
||||
),
|
||||
acp_command: "buzz-acp".into(),
|
||||
command: "goose".into(),
|
||||
args: vec!["--mode".into(), "acp".into()],
|
||||
@@ -51,6 +56,7 @@ type Mutation = (&'static str, fn(&mut SpawnConfigSnapshot));
|
||||
|
||||
fn mutations() -> Vec<Mutation> {
|
||||
vec![
|
||||
("runtime_configuration", |s| s.runtime_configuration = None),
|
||||
("acp_command", |s| s.acp_command = "other-acp".into()),
|
||||
("command", |s| s.command = "claude".into()),
|
||||
("args", |s| s.args = vec!["--other".into()]),
|
||||
|
||||
Reference in New Issue
Block a user