diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc594e16ad..d65516aec6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -970,6 +970,13 @@ jobs: # Serial: windows_resolver_tests mutate process-global env # (BUZZ_SHELL/GIT_BASH/SystemRoot) that SharedState::new reads. run: cargo test -p buzz-dev-mcp --target $env:TARGET -- --test-threads=1 + - name: Test (buzz-acp ledger) + # The pending-turn ledger persists snapshots by writing `.tmp` and + # renaming it over the live file. Windows rename semantics differ from + # POSIX, so the overwrite path only gets real coverage if these tests run + # ON Windows. Scoped to `ledger::` — the full buzz-acp suite is already + # covered by the Linux jobs and is too slow to duplicate here. + run: "cargo test -p buzz-acp --target $env:TARGET ledger::" # Smoke-test the new host-prereq contract: Git for Windows (which provides # bash) is available on the runner, a shell command round-trips, and bash # does NOT resolve from System32 (so WSL's launcher is never picked up). diff --git a/Cargo.lock b/Cargo.lock index 5d9a2d7646..b28b342885 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -808,6 +808,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.11.0", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-tungstenite 0.29.0", diff --git a/crates/buzz-acp/Cargo.toml b/crates/buzz-acp/Cargo.toml index d047849806..2a85e68cef 100644 --- a/crates/buzz-acp/Cargo.toml +++ b/crates/buzz-acp/Cargo.toml @@ -79,3 +79,4 @@ nix = { version = "0.31", default-features = false, features = ["signal"] } [dev-dependencies] tokio = { workspace = true, features = ["test-util"] } httparse = "1" +tempfile = "3" diff --git a/crates/buzz-acp/src/config.rs b/crates/buzz-acp/src/config.rs index dab61be30a..12ddcfaf9b 100644 --- a/crates/buzz-acp/src/config.rs +++ b/crates/buzz-acp/src/config.rs @@ -418,6 +418,29 @@ pub struct CliArgs { )] pub base_prompt_file: Option, + /// Resume in-progress work on restart (kill -9, crash, manual restart, + /// or app update) by replaying a durable pending-turn ledger on boot. + /// On by default. `--resume-on-restart=false` disables. + #[arg( + long, + env = "BUZZ_ACP_RESUME_ON_RESTART", + default_value_t = true, + action = clap::ArgAction::Set, + num_args = 0..=1, + default_missing_value = "true", + )] + pub resume_on_restart: bool, + + /// TTL (seconds) for resume ledger entries, by original admission time. + /// 0 = no expiry (default) — the ledger only ever holds never-completed + /// turns, so even a long-idle relaunch should still resume them. + #[arg(long, env = "BUZZ_ACP_RESUME_TTL", default_value_t = 0)] + pub resume_ttl_secs: u64, + + /// Directory for the resume ledger file. Defaults to `/.buzz-acp/`. + #[arg(long, env = "BUZZ_ACP_STATE_DIR")] + pub state_dir: Option, + /// Desired LLM model ID. Applied to every new ACP session after creation. /// Use `buzz-acp models` to discover available model IDs. #[arg(long, env = "BUZZ_ACP_MODEL")] @@ -561,6 +584,14 @@ pub struct Config { /// `from_cli()`. `None` when using the compiled-in default or when /// `--no-base-prompt` is set. pub base_prompt_content: Option, + /// Whether to replay the durable pending-turn ledger on boot. On by + /// default; `--resume-on-restart=false` restores today's stateless-boot + /// behavior. + pub resume_on_restart: bool, + /// TTL (seconds) for resume ledger entries. 0 = no expiry (default). + pub resume_ttl_secs: u64, + /// Directory for the resume ledger file. `None` = `/.buzz-acp/`. + pub state_dir: Option, } /// Maximum length, in characters, of a session title sent to the adapter. @@ -1102,6 +1133,9 @@ impl Config { agent_owner: args.agent_owner.map(|s| s.trim().to_ascii_lowercase()), no_base_prompt: args.no_base_prompt, base_prompt_content, + resume_on_restart: args.resume_on_restart, + resume_ttl_secs: args.resume_ttl_secs, + state_dir: args.state_dir, }; Ok(config) @@ -1123,7 +1157,7 @@ impl Config { format!(" allowed_respond_to=[{}]", modes.join(",")) }; format!( - "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} {}{}", + "relay={} pubkey={} agent_cmd={} {} mcp_cmd={} idle_timeout={}s max_turn={}s agents={} heartbeat={}s subscribe={:?} dedup={:?} meh={:?} ignore_self={} context_limit={} max_turns_per_session={} presence={} typing={} memory={} model={} permission_mode={} resume={} resume_ttl={}s {}{}", self.relay_url, self.keys.public_key().to_hex(), self.agent_command, @@ -1144,6 +1178,8 @@ impl Config { self.memory_enabled, self.model.as_deref().unwrap_or("(agent default)"), self.permission_mode, + self.resume_on_restart, + self.resume_ttl_secs, respond_to_detail, allowed_respond_to_detail, ) @@ -1472,6 +1508,9 @@ mod tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + resume_on_restart: true, + resume_ttl_secs: 0, + state_dir: None, } } @@ -2840,6 +2879,49 @@ channels = "ALL" } } + /// `BUZZ_ACP_RESUME_ON_RESTART` is process-global; `resume_on_restart_ + /// env_false_disables` below sets/clears it around its parse, so any + /// other test that parses `CliArgs` without an explicit + /// `--resume-on-restart` flag (and would therefore observe the env var) + /// must serialize against it with this same lock. Tests that pass an + /// explicit flag are unaffected — clap's explicit-arg-over-env + /// precedence means the flag always wins regardless of the race. + static RESUME_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + #[test] + fn resume_on_restart_default_is_true() { + let _guard = RESUME_ENV_LOCK.lock().unwrap(); + let args = CliArgs::try_parse_from(["buzz-acp", "--private-key", TEST_PRIVATE_KEY]) + .expect("clap should parse args"); + assert!(args.resume_on_restart); + } + + #[test] + fn resume_on_restart_equals_false_disables() { + let args = CliArgs::try_parse_from([ + "buzz-acp", + "--private-key", + TEST_PRIVATE_KEY, + "--resume-on-restart=false", + ]) + .expect("clap should parse args"); + assert!(!args.resume_on_restart); + } + + #[test] + fn resume_on_restart_env_false_disables() { + // See RESUME_ENV_LOCK doc comment above — env var is process-global. + let _guard = RESUME_ENV_LOCK.lock().unwrap(); + std::env::set_var("BUZZ_ACP_RESUME_ON_RESTART", "false"); + let args = CliArgs::try_parse_from(["buzz-acp", "--private-key", TEST_PRIVATE_KEY]); + std::env::remove_var("BUZZ_ACP_RESUME_ON_RESTART"); + let args = args.expect("clap should parse args"); + assert!( + !args.resume_on_restart, + "BUZZ_ACP_RESUME_ON_RESTART=false must disable resume without a CLI flag" + ); + } + #[test] fn sanitize_session_title_collapses_whitespace_and_strips_control_chars() { assert_eq!( diff --git a/crates/buzz-acp/src/ledger.rs b/crates/buzz-acp/src/ledger.rs new file mode 100644 index 0000000000..6d4373a51d --- /dev/null +++ b/crates/buzz-acp/src/ledger.rs @@ -0,0 +1,857 @@ +//! Durable pending-turn ledger — a JSON mirror of the work `EventQueue` +//! still owes a turn for, so a restarted harness can resume without +//! re-prompting. +//! +//! This module owns the file format, atomic persistence, the per-channel +//! sync contract (with unresolved-record preservation across ordinary +//! rewrites), the unresolved-trigger lifecycle (resolve/invalidate), and +//! the staged-load + single-commit transaction boot uses. Boot +//! orchestration (membership gate, chunked REST fetch, queue wiring) is +//! `lib.rs`'s job — this module only persists and serves what it's told. + +use std::collections::{BTreeSet, HashMap}; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; +use sha2::{Digest as _, Sha256}; +use uuid::Uuid; + +use crate::queue::RecoverableTrigger; + +/// On-disk format version. Any file with a different value is treated as +/// corrupt (start fresh) rather than partially interpreted. +const LEDGER_VERSION: u32 = 1; + +/// Only the first 16 hex chars of the agent pubkey are used for the +/// filename — enough to disambiguate multiple agents sharing a nest +/// without an unwieldy path. +const PUBKEY_PREFIX_LEN: usize = 16; + +/// First 16 hex chars of the sha256 of the canonical relay URL — enough +/// to namespace ledger files per `(agent, relay)` pair so concurrent +/// harnesses on different relays never share a file. +const RELAY_HASH_LEN: usize = 16; + +/// The minimal durable identity of one recoverable event, as persisted. +/// Mirrors [`RecoverableTrigger`] field-for-field; kept as a distinct type +/// so the on-disk format doesn't silently change if the in-memory type +/// grows fields the ledger has no reason to persist. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct LedgerRecord { + pub event_id: String, + pub prompt_tag: String, + pub admission_seq: u64, + pub enqueued_at_unix: u64, + pub cap_exempt: bool, +} + +impl From<&RecoverableTrigger> for LedgerRecord { + fn from(t: &RecoverableTrigger) -> Self { + LedgerRecord { + event_id: t.event_id.clone(), + prompt_tag: t.prompt_tag.clone(), + admission_seq: t.admission_seq, + enqueued_at_unix: t.enqueued_at_unix, + cap_exempt: t.cap_exempt, + } + } +} + +/// On-disk shape: `{"version":1,"channels":{"":[, ...]}}`. +/// No `thread_root`/`created_at` — those are re-fetched with the event. +#[derive(Debug, Serialize, Deserialize)] +struct LedgerFile { + version: u32, + channels: HashMap>, +} + +/// A read-only snapshot of the ledger file as loaded from disk, staged for +/// boot recovery processing (membership gate, TTL, REST fetch — all owned +/// by `lib.rs`). Nothing here is committed to the runtime `EventQueue` +/// until the caller finishes processing and calls [`Ledger::commit`]. +#[derive(Debug, Default)] +pub struct StagedLedger { + pub channels: HashMap>, +} + +/// The durable pending-turn ledger for one agent process. +/// +/// `path == None` means the ledger is disabled for this run (state dir +/// couldn't be created) — every operation becomes a silent no-op so the +/// hot path is never blocked by a degraded filesystem. +pub struct Ledger { + path: Option, + /// Per-channel record set reflecting desired in-memory state — always + /// mutated unconditionally before any persist attempt so removals and + /// additions are never lost if a write fails. The skip-identical-write + /// check uses `last_written` for content comparison; `dirty` carries the + /// "write failed, must retry" signal. + last_written: HashMap>, + /// True when the last `persist_candidate` call failed. Forces the next + /// `sync` to attempt the write even if the record set is unchanged, + /// ensuring a transient failure is healed at the next opportunity. + dirty: bool, + /// Unresolved boot-fetch records, keyed by channel. Preserved across + /// ordinary `sync()` rewrites of the same channel until resolved + /// (event arrives live) or invalidated (channel removed). + unresolved: HashMap>, +} + +/// Delete the ledger file for `(agent_pubkey_hex, relay_url)` in `state_dir`, +/// if it exists. Called at boot when `resume_on_restart` is false so that +/// turning the toggle off truly drops any pending work — a future re-enable +/// starts clean rather than resuming a stale turn from a prior session. +/// +/// This is a best-effort operation: `NotFound` is silently ignored (nothing +/// to delete), and any other I/O error is logged and swallowed — the hot path +/// must never be blocked by a degraded filesystem, matching the crate's +/// established degraded-filesystem convention. +pub fn delete_ledger_file(state_dir: &Path, agent_pubkey_hex: &str, relay_url: &str) { + let prefix: String = agent_pubkey_hex.chars().take(PUBKEY_PREFIX_LEN).collect(); + let canonical = + buzz_core::relay::normalize_relay_url(relay_url).unwrap_or_else(|_| relay_url.to_owned()); + let relay_hash_full = hex::encode(Sha256::digest(canonical.as_bytes())); + let relay_hash: String = relay_hash_full.chars().take(RELAY_HASH_LEN).collect(); + let path = state_dir.join(format!("pending-turns-{prefix}-{relay_hash}.json")); + match std::fs::remove_file(&path) { + Ok(()) => tracing::debug!( + path = %path.display(), + "resume ledger deleted (resume_on_restart=false)" + ), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => tracing::warn!( + path = %path.display(), + error = %e, + "failed to delete resume ledger on disable — file may resurface on re-enable" + ), + } +} + +impl Ledger { + /// A ledger with no backing file — every operation is a silent no-op. + /// Used when `--resume-on-restart` is off. Call [`delete_ledger_file`] + /// before this to enforce the "off means off" product promise: any file + /// left from a previous enabled run is removed so toggling back on later + /// starts clean rather than resuming a stale turn from a prior session. + pub fn disabled() -> Ledger { + Ledger { + path: None, + last_written: HashMap::new(), + dirty: false, + unresolved: HashMap::new(), + } + } + + /// Load `/pending-turns--.json` and + /// stage its contents for boot processing. Never fails: a missing + /// file, unparseable JSON, unknown version, or uncreatable state dir + /// all degrade to an empty stage (one lost recovery at worst, logged), + /// matching today's every-restart-loses-everything behavior as the + /// floor. + /// + /// The filename includes both the first 16 hex chars of the agent + /// pubkey and the first 16 hex chars of the sha256 of the canonical + /// relay URL, so concurrent harnesses for the same agent on different + /// relays each own a distinct file. + /// + /// `ttl_secs == 0` disables TTL filtering (default); otherwise records + /// older than `ttl_secs` (by `enqueued_at_unix`) are dropped from the + /// stage before it's returned — the ledger file on disk is untouched + /// until the next write, so a filtered-out record isn't destroyed by + /// merely loading it. + pub fn load( + state_dir: &Path, + agent_pubkey_hex: &str, + relay_url: &str, + ttl_secs: u64, + ) -> (Ledger, StagedLedger) { + let empty = || { + ( + Ledger { + path: None, + last_written: HashMap::new(), + dirty: false, + unresolved: HashMap::new(), + }, + StagedLedger::default(), + ) + }; + + if let Err(e) = std::fs::create_dir_all(state_dir) { + tracing::warn!( + dir = %state_dir.display(), + error = %e, + "failed to create ACP state dir — resume ledger disabled for this run" + ); + return empty(); + } + + let prefix: String = agent_pubkey_hex.chars().take(PUBKEY_PREFIX_LEN).collect(); + let canonical_relay = buzz_core::relay::normalize_relay_url(relay_url) + .unwrap_or_else(|_| relay_url.to_owned()); + let relay_hash_full = hex::encode(Sha256::digest(canonical_relay.as_bytes())); + let relay_hash: String = relay_hash_full.chars().take(RELAY_HASH_LEN).collect(); + let path = state_dir.join(format!("pending-turns-{prefix}-{relay_hash}.json")); + + let channels = match std::fs::read(&path) { + Err(e) if e.kind() == std::io::ErrorKind::NotFound => HashMap::new(), + Err(e) => { + tracing::warn!(path = %path.display(), error = %e, "failed to read resume ledger — starting fresh"); + HashMap::new() + } + Ok(bytes) => match serde_json::from_slice::(&bytes) { + Ok(file) if file.version == LEDGER_VERSION => file.channels, + Ok(file) => { + tracing::warn!( + path = %path.display(), + found_version = file.version, + expected_version = LEDGER_VERSION, + "unsupported resume ledger version — starting fresh (one lost recovery)" + ); + let _ = std::fs::remove_file(&path); + HashMap::new() + } + Err(e) => { + tracing::warn!( + path = %path.display(), + error = %e, + "corrupt resume ledger — starting fresh (one lost recovery)" + ); + let _ = std::fs::remove_file(&path); + HashMap::new() + } + }, + }; + + let channels = apply_ttl(channels, ttl_secs); + + ( + Ledger { + path: Some(path), + last_written: HashMap::new(), + dirty: false, + unresolved: HashMap::new(), + }, + StagedLedger { channels }, + ) + } + + /// Whether this ledger is actually persisting (state dir creation + /// succeeded). Boot recovery is a no-op end to end when this is + /// `false` — there was never anything to stage. + pub fn is_enabled(&self) -> bool { + self.path.is_some() + } + + /// The on-disk path this ledger writes to, or `None` if disabled. + /// Exposed for tests that need to construct adjacent paths (e.g. the + /// `.tmp` sibling used in atomic-write tests). + #[cfg(test)] + pub(crate) fn path(&self) -> Option<&std::path::Path> { + self.path.as_deref() + } + + /// Record an unresolved boot-fetch trigger for `channel_id` — a + /// ledger entry whose event couldn't be fetched (or validated) during + /// boot recovery. Kept sorted by `admission_seq` so it merges cleanly + /// with live triggers in `sync`/`commit`. + pub fn add_unresolved(&mut self, channel_id: Uuid, record: LedgerRecord) { + let entries = self.unresolved.entry(channel_id).or_default(); + entries.push(record); + entries.sort_by_key(|r| r.admission_seq); + } + + /// Every channel with at least one unresolved record — for boot to + /// register `EventQueue` ordering barriers over. + pub fn unresolved_channels(&self) -> Vec { + self.unresolved.keys().copied().collect() + } + + /// The set of unresolved `admission_seq`s for `channel_id` — the + /// input to `EventQueue::set_unresolved_barrier`. Empty if the channel + /// has no unresolved records. + pub fn unresolved_seqs(&self, channel_id: Uuid) -> BTreeSet { + self.unresolved + .get(&channel_id) + .map(|records| records.iter().map(|r| r.admission_seq).collect()) + .unwrap_or_default() + } + + /// Look up an unresolved record by event id, without consuming it. + /// Read-only — the caller must call [`Ledger::resolve_unresolved`] + /// only *after* handing the returned record's event to the queue + /// (e.g. via `EventQueue::admit_recovered`), preserving the + /// consume-after-ownership ordering documented there. + pub fn find_unresolved(&self, channel_id: Uuid, event_id: &str) -> Option { + self.unresolved + .get(&channel_id)? + .iter() + .find(|r| r.event_id == event_id) + .cloned() + } + + /// Consume an unresolved record by event id — call **only** after the + /// event has been admitted into the queue's ownership (e.g. via + /// `EventQueue::admit_recovered`), so there is never a window where + /// the trigger exists in neither the queue nor the ledger's unresolved + /// set (consume-after-ownership ordering). + pub fn resolve_unresolved(&mut self, channel_id: Uuid, event_id: &str) { + if let Some(records) = self.unresolved.get_mut(&channel_id) { + records.retain(|r| r.event_id != event_id); + if records.is_empty() { + self.unresolved.remove(&channel_id); + } + } + } + + /// Purge every durable record — live and unresolved alike — for a + /// channel that's been removed (membership revoked). Writes + /// immediately: unlike `sync`, this can't defer to the next dirty- + /// channel drain, because a removed channel never dirties again. + pub fn invalidate_channel(&mut self, channel_id: Uuid) { + let had_unresolved = self.unresolved.remove(&channel_id).is_some(); + let had_written = self.last_written.remove(&channel_id).is_some(); + if had_unresolved || had_written { + // last_written already reflects intent (channel removed above). + // Persist; on failure set dirty so the next sync heals the file. + self.dirty = !self.persist_candidate(&self.last_written.clone()); + } + } + + /// Persist `channel_id`'s current recoverable trigger set, merged with + /// any unresolved records for that channel (the two are disjoint by + /// construction — see the module-level design doc). Skips the write + /// entirely if the merged record set is unchanged since the last + /// write to this channel AND no prior write failed (skip-identical-write + /// plus dirty-flag retry). + pub fn sync(&mut self, channel_id: Uuid, triggers: Vec) { + if self.path.is_none() { + return; + } + let records = self.merge_with_unresolved(channel_id, &triggers); + + let changed = match self.last_written.get(&channel_id) { + Some(existing) => *existing != records, + None => !records.is_empty(), + }; + if !changed && !self.dirty { + return; + } + // Unconditionally update last_written to reflect desired state before + // persisting — removals must be visible even if the write fails, so + // subsequent successful writes of any channel naturally omit removed + // entries and heal the file. + if records.is_empty() { + self.last_written.remove(&channel_id); + } else { + self.last_written.insert(channel_id, records); + } + self.dirty = !self.persist_candidate(&self.last_written.clone()); + } + + /// Boot-only: commit the single transactional snapshot computed as + /// `live ∪ unresolved` per channel, then resume normal per-mutation + /// `sync()` behavior. This is the **only** ledger write during boot + /// recovery — a crash before this call leaves the pre-crash file + /// untouched. + pub fn commit(&mut self, live: HashMap>) { + let mut channels: HashMap> = HashMap::new(); + let all_channels = live.keys().copied().chain(self.unresolved.keys().copied()); + for channel_id in all_channels { + let triggers = live.get(&channel_id).cloned().unwrap_or_default(); + let records = self.merge_with_unresolved(channel_id, &triggers); + if !records.is_empty() { + channels.insert(channel_id, records); + } + } + self.last_written = channels; + self.dirty = !self.persist_candidate(&self.last_written.clone()); + } + + fn merge_with_unresolved( + &self, + channel_id: Uuid, + triggers: &[RecoverableTrigger], + ) -> Vec { + let mut records: Vec = triggers.iter().map(LedgerRecord::from).collect(); + if let Some(unresolved) = self.unresolved.get(&channel_id) { + records.extend(unresolved.iter().cloned()); + records.sort_by_key(|r| r.admission_seq); + } + records + } + + /// Atomic write: serialize `candidate` to a temp file in the same + /// directory, then rename over the real path. Returns `true` on success. + /// A crash mid-write leaves the previous file intact; a crash after the + /// rename leaves the new one intact — there is no partially-written state. + /// The caller is responsible for advancing `last_written` only on success. + fn persist_candidate(&self, candidate: &HashMap>) -> bool { + let Some(path) = &self.path else { return true }; // no-path = in-memory only + let file = LedgerFile { + version: LEDGER_VERSION, + channels: candidate.clone(), + }; + let json = match serde_json::to_vec_pretty(&file) { + Ok(j) => j, + Err(e) => { + tracing::error!(error = %e, "failed to serialize resume ledger — write skipped"); + return false; + } + }; + let mut tmp_name = path.as_os_str().to_os_string(); + tmp_name.push(".tmp"); + let tmp_path = PathBuf::from(tmp_name); + if let Err(e) = std::fs::write(&tmp_path, &json) { + tracing::error!(path = %tmp_path.display(), error = %e, "failed to write resume ledger temp file"); + return false; + } + if let Err(e) = std::fs::rename(&tmp_path, path) { + tracing::error!(path = %path.display(), error = %e, "failed to rename resume ledger into place"); + return false; + } + true + } +} + +/// Drop records older than `ttl_secs` (by `enqueued_at_unix`), dropping +/// the whole channel entry if nothing survives. `ttl_secs == 0` disables +/// filtering entirely — the default, since the ledger only ever holds +/// never-completed turns and a long-idle relaunch must still resume them. +fn apply_ttl( + channels: HashMap>, + ttl_secs: u64, +) -> HashMap> { + if ttl_secs == 0 { + return channels; + } + let now = crate::relay::unix_now_secs(); + channels + .into_iter() + .filter_map(|(channel_id, records)| { + let kept: Vec = records + .into_iter() + .filter(|r| now.saturating_sub(r.enqueued_at_unix) <= ttl_secs) + .collect(); + if kept.is_empty() { + None + } else { + Some((channel_id, kept)) + } + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Relay URL used across all unit tests — stable across runs. + const TEST_RELAY_URL: &str = "ws://localhost:3000"; + + /// Compute the expected ledger filename for `(pubkey_hex, relay_url)`, + /// mirroring the logic in `Ledger::load`. Used by tests that need to + /// pre-seed the on-disk file before calling `load`. + fn ledger_filename(pubkey_hex: &str, relay_url: &str) -> String { + let prefix: String = pubkey_hex.chars().take(PUBKEY_PREFIX_LEN).collect(); + let canonical = buzz_core::relay::normalize_relay_url(relay_url) + .unwrap_or_else(|_| relay_url.to_owned()); + let hash_full = hex::encode(Sha256::digest(canonical.as_bytes())); + let relay_hash: String = hash_full.chars().take(RELAY_HASH_LEN).collect(); + format!("pending-turns-{prefix}-{relay_hash}.json") + } + + fn record(event_id: &str, seq: u64, enqueued_at_unix: u64, cap_exempt: bool) -> LedgerRecord { + LedgerRecord { + event_id: event_id.to_string(), + prompt_tag: "mention".to_string(), + admission_seq: seq, + enqueued_at_unix, + cap_exempt, + } + } + + fn trigger( + event_id: &str, + seq: u64, + enqueued_at_unix: u64, + cap_exempt: bool, + ) -> RecoverableTrigger { + RecoverableTrigger { + event_id: event_id.to_string(), + prompt_tag: "mention".to_string(), + admission_seq: seq, + enqueued_at_unix, + cap_exempt, + } + } + + fn read_file_channels(path: &Path) -> HashMap> { + let bytes = std::fs::read(path).expect("ledger file should exist"); + let file: LedgerFile = serde_json::from_slice(&bytes).expect("valid ledger JSON"); + assert_eq!(file.version, LEDGER_VERSION); + file.channels + } + + #[test] + fn test_load_corrupt_file_starts_fresh_and_deletes_it() { + let dir = tempfile::tempdir().unwrap(); + let path = dir + .path() + .join(ledger_filename("deadbeefdeadbeefdeadbeef", TEST_RELAY_URL)); + std::fs::write(&path, b"not json").unwrap(); + + let (ledger, staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + assert!(ledger.is_enabled()); + assert!(staged.channels.is_empty()); + assert!(!path.exists(), "corrupt file should be deleted"); + } + + #[test] + fn test_load_unknown_version_starts_fresh() { + let dir = tempfile::tempdir().unwrap(); + let path = dir + .path() + .join(ledger_filename("deadbeefdeadbeefdeadbeef", TEST_RELAY_URL)); + std::fs::write(&path, br#"{"version":99,"channels":{}}"#).unwrap(); + + let (_ledger, staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + assert!(staged.channels.is_empty()); + assert!(!path.exists()); + } + + #[test] + fn test_load_disabled_when_state_dir_uncreatable() { + let dir = tempfile::tempdir().unwrap(); + // A regular file blocking the directory path forces create_dir_all + // to fail. + let blocked = dir.path().join("state_as_file"); + std::fs::write(&blocked, b"not a dir").unwrap(); + + let (ledger, staged) = + Ledger::load(&blocked, "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + assert!(!ledger.is_enabled()); + assert!(staged.channels.is_empty()); + } + + #[test] + fn test_commit_writes_merged_snapshot_sorted_by_admission_seq() { + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + let ch = Uuid::new_v4(); + ledger.add_unresolved(ch, record("unresolved-a", 1, 100, false)); + + let mut live = HashMap::new(); + live.insert(ch, vec![trigger("live-b", 2, 200, false)]); + ledger.commit(live); + + let on_disk = read_file_channels(ledger.path.as_ref().unwrap()); + let recs = on_disk.get(&ch).expect("channel present"); + assert_eq!( + recs.iter().map(|r| r.event_id.as_str()).collect::>(), + vec!["unresolved-a", "live-b"] + ); + } + + #[test] + fn test_sync_skips_identical_write() { + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + let ch = Uuid::new_v4(); + let path = ledger.path.clone().unwrap(); + + ledger.sync(ch, vec![trigger("a", 1, 100, false)]); + assert!(path.exists()); + + // Simulate an external observer noticing the file, then remove it — + // an identical follow-up sync must NOT recreate it. + std::fs::remove_file(&path).unwrap(); + ledger.sync(ch, vec![trigger("a", 1, 100, false)]); + assert!(!path.exists(), "identical sync should not rewrite the file"); + + // A genuinely different trigger set must write. + ledger.sync( + ch, + vec![trigger("a", 1, 100, false), trigger("b", 2, 200, false)], + ); + assert!(path.exists(), "changed sync must rewrite the file"); + } + + #[test] + fn test_sync_preserves_unresolved_entries_across_channel_rewrite() { + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + let ch = Uuid::new_v4(); + ledger.add_unresolved(ch, record("unresolved-a", 1, 100, false)); + + // Unrelated live work on the same channel completes and the queue + // reports an empty recoverable set — mark_complete's sync must not + // erase the only durable proof of the unfetched trigger. + ledger.sync(ch, vec![]); + + let on_disk = read_file_channels(ledger.path.as_ref().unwrap()); + let recs = on_disk.get(&ch).expect("unresolved record must survive"); + assert_eq!(recs.len(), 1); + assert_eq!(recs[0].event_id, "unresolved-a"); + } + + #[test] + fn test_resolve_unresolved_removes_record_then_sync_omits_it() { + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + let ch = Uuid::new_v4(); + ledger.add_unresolved(ch, record("unresolved-a", 1, 100, false)); + assert_eq!(ledger.unresolved_seqs(ch), BTreeSet::from([1])); + + ledger.resolve_unresolved(ch, "unresolved-a"); + assert!(ledger.unresolved_seqs(ch).is_empty()); + + // Now admitted live (admit_recovered would push it back through the + // queue) — the next sync carries it as an ordinary trigger, and the + // ledger must not duplicate it via the (now-empty) unresolved table. + ledger.sync(ch, vec![trigger("unresolved-a", 1, 100, false)]); + let on_disk = read_file_channels(ledger.path.as_ref().unwrap()); + assert_eq!(on_disk.get(&ch).unwrap().len(), 1); + } + + #[test] + fn test_invalidate_channel_purges_unresolved_and_last_written() { + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + let ch = Uuid::new_v4(); + ledger.add_unresolved(ch, record("unresolved-a", 1, 100, false)); + ledger.sync(ch, vec![trigger("live-b", 2, 200, false)]); + assert!(!ledger.unresolved_seqs(ch).is_empty()); + + ledger.invalidate_channel(ch); + + assert!(ledger.unresolved_seqs(ch).is_empty()); + let on_disk = read_file_channels(ledger.path.as_ref().unwrap()); + assert!( + !on_disk.contains_key(&ch), + "invalidated channel must not resurrect on re-add" + ); + } + + #[test] + fn test_persist_over_existing_file_replaces_previous_snapshot() { + // `persist_candidate` writes `.tmp` then renames it over an + // already-existing ``. Guards the destination-exists case of + // that rename on every platform this test runs on — notably + // Windows, where `MoveFileExW` needs `MOVEFILE_REPLACE_EXISTING` + // for the overwrite to succeed. + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + let ch = Uuid::new_v4(); + + ledger.sync(ch, vec![trigger("snapshot-a", 1, 100, false)]); + let first = read_file_channels(ledger.path.as_ref().unwrap()); + assert_eq!(first.get(&ch).unwrap()[0].event_id, "snapshot-a"); + + // Second write to the same path: the destination now exists. + ledger.sync(ch, vec![trigger("snapshot-b", 2, 200, false)]); + assert!( + !ledger.dirty, + "overwrite persist must succeed, leaving the ledger clean" + ); + + let second = read_file_channels(ledger.path.as_ref().unwrap()); + let recs = second.get(&ch).expect("channel must still be on disk"); + assert_eq!(recs.len(), 1, "snapshot B replaces A rather than appending"); + assert_eq!( + recs[0].event_id, "snapshot-b", + "reload must observe the second snapshot, not the first" + ); + + // The temp file must not survive a successful rename. + let mut tmp = ledger.path.as_ref().unwrap().as_os_str().to_os_string(); + tmp.push(".tmp"); + assert!( + !PathBuf::from(tmp).exists(), + "temp file must be consumed by the rename" + ); + } + + #[test] + fn test_ttl_filter_drops_expired_records() { + let dir = tempfile::tempdir().unwrap(); + let ch = Uuid::new_v4(); + let path = dir + .path() + .join(ledger_filename("deadbeefdeadbeefdeadbeef", TEST_RELAY_URL)); + let mut channels = HashMap::new(); + channels.insert(ch, vec![record("ancient", 1, 0, false)]); + std::fs::write( + &path, + serde_json::to_vec(&LedgerFile { + version: 1, + channels, + }) + .unwrap(), + ) + .unwrap(); + + // enqueued_at_unix = 0 (1970) is far older than any positive TTL. + let (_ledger, staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 60); + assert!( + !staged.channels.contains_key(&ch), + "expired-only channel should be dropped entirely" + ); + } + + #[test] + fn test_load_round_trips_cap_exempt_for_promotion_rule() { + let dir = tempfile::tempdir().unwrap(); + let ch_no_exempt = Uuid::new_v4(); + let ch_one_exempt = Uuid::new_v4(); + let path = dir + .path() + .join(ledger_filename("deadbeefdeadbeefdeadbeef", TEST_RELAY_URL)); + let mut channels = HashMap::new(); + channels.insert( + ch_no_exempt, + vec![record("a", 1, 100, false), record("b", 2, 100, false)], + ); + channels.insert( + ch_one_exempt, + vec![record("c", 1, 100, true), record("d", 2, 100, false)], + ); + std::fs::write( + &path, + serde_json::to_vec(&LedgerFile { + version: 1, + channels, + }) + .unwrap(), + ) + .unwrap(); + + let (_ledger, staged) = + Ledger::load(dir.path(), "deadbeefdeadbeefdeadbeef", TEST_RELAY_URL, 0); + // Round-trip must preserve the exact persisted cap_exempt bits — + // `boot_recover`'s whole-snapshot promotion rule reads these values + // directly from the staged ledger before deciding per-channel promotion. + assert!(staged.channels[&ch_no_exempt].iter().all(|r| !r.cap_exempt)); + let one_exempt = &staged.channels[&ch_one_exempt]; + assert!(one_exempt.iter().any(|r| r.cap_exempt)); + assert!(one_exempt.iter().any(|r| !r.cap_exempt)); + } + + /// Two harnesses for the same agent on different relays share a state dir. + /// Each must load and commit to its own isolated file; neither boot + /// recovery nor a subsequent sync on one pair must drop or mutate the + /// other pair's records. + #[test] + fn test_relay_isolation_same_agent_different_relays_use_distinct_files() { + let dir = tempfile::tempdir().unwrap(); + let agent_pubkey = "deadbeefdeadbeefdeadbeef"; + let relay_a = "wss://relay-a.example"; + let relay_b = "wss://relay-b.example"; + let ch_a = Uuid::new_v4(); + let ch_b = Uuid::new_v4(); + + // Pre-seed two files as if two prior runs each wrote their own. + let file_a = dir.path().join(ledger_filename(agent_pubkey, relay_a)); + let file_b = dir.path().join(ledger_filename(agent_pubkey, relay_b)); + assert_ne!( + file_a, file_b, + "relay isolation requires distinct filenames" + ); + + let mk_file = |ch: Uuid, event_id: &str| { + let mut channels = HashMap::new(); + channels.insert(ch, vec![record(event_id, 1, 100, false)]); + serde_json::to_vec(&LedgerFile { + version: 1, + channels, + }) + .unwrap() + }; + std::fs::write(&file_a, mk_file(ch_a, "event-a")).unwrap(); + std::fs::write(&file_b, mk_file(ch_b, "event-b")).unwrap(); + + // Boot-load relay A; verify it loads only relay A's channel. + let (mut ledger_a, staged_a) = Ledger::load(dir.path(), agent_pubkey, relay_a, 0); + assert!( + staged_a.channels.contains_key(&ch_a), + "relay A must see ch_a" + ); + assert!( + !staged_a.channels.contains_key(&ch_b), + "relay A must NOT see ch_b" + ); + + // Boot-load relay B; verify it loads only relay B's channel. + let (_ledger_b, staged_b) = Ledger::load(dir.path(), agent_pubkey, relay_b, 0); + assert!( + staged_b.channels.contains_key(&ch_b), + "relay B must see ch_b" + ); + assert!( + !staged_b.channels.contains_key(&ch_a), + "relay B must NOT see ch_a" + ); + + // Commit (empty live) on relay A; relay B's file must be untouched. + ledger_a.commit(HashMap::new()); + let b_after = Ledger::load(dir.path(), agent_pubkey, relay_b, 0).1; + assert!( + b_after.channels.contains_key(&ch_b), + "relay A commit must not erase relay B's records" + ); + } + + #[test] + fn test_disable_deletes_file_and_re_enable_starts_clean() { + // Discriminating test for the "off means off" product promise: + // persist a record → boot with resume off → file is gone → + // boot with resume on → nothing resumed. + // + // Confirms that delete_ledger_file removes the correct file and that + // a subsequent Ledger::load cannot recover the deleted record. + let dir = tempfile::tempdir().unwrap(); + let pubkey = "aabbccddeeff0011"; + let relay = TEST_RELAY_URL; + + // Step 1: persist a record (resume enabled). + let (mut ledger, _staged) = Ledger::load(dir.path(), pubkey, relay, 0); + let ch = Uuid::new_v4(); + ledger.sync(ch, vec![trigger("event-abc", 1, 100, false)]); + let file_path = ledger + .path() + .expect("ledger must have a path") + .to_path_buf(); + assert!(file_path.exists(), "ledger file must exist after sync"); + + // Step 2: boot with resume off — file must be deleted. + delete_ledger_file(dir.path(), pubkey, relay); + assert!( + !file_path.exists(), + "ledger file must be deleted by delete_ledger_file" + ); + + // Step 3: boot with resume on again — staged ledger must be empty. + let (_new_ledger, staged) = Ledger::load(dir.path(), pubkey, relay, 0); + assert!( + staged.channels.is_empty(), + "re-enabled boot must not recover any records after the file was deleted" + ); + } + + #[test] + fn test_disable_delete_is_silent_noop_when_file_missing() { + // delete_ledger_file must not panic or error when no file exists. + let dir = tempfile::tempdir().unwrap(); + delete_ledger_file(dir.path(), "aabbccddeeff0011", TEST_RELAY_URL); + // If we reach here without panic, the test passes. + } +} diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 403512a322..34914c666f 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -4,6 +4,7 @@ mod acp; mod config; mod engram_fetch; mod filter; +mod ledger; mod observer; mod pool; mod pool_lifecycle; @@ -15,6 +16,7 @@ mod usage; pub use usage::TurnUsage; use std::collections::{HashMap, HashSet, VecDeque}; +use std::path::PathBuf; use std::sync::Arc; use std::time::Duration; @@ -35,13 +37,14 @@ use config::{ }; use filter::SubscriptionRule; use futures_util::FutureExt; +use ledger::Ledger; use nostr::{PublicKey, ToBech32}; use pool::{ AgentPool, ControlSignal, IdleSwitchResult, OwnedAgent, PromptContext, PromptOutcome, PromptResult, PromptSource, SessionState, TimeoutKind, }; use pool_lifecycle::PoolLifecycle; -use queue::{CancelReason, EventQueue, FlushBatch, QueuedEvent, ThreadTags}; +use queue::{BatchDisposition, CancelReason, EventQueue, FlushBatch, QueuedEvent, ThreadTags}; use relay::{HarnessRelay, RelayEventPublisher}; use tokio::sync::{mpsc, watch}; use tracing_subscriber::EnvFilter; @@ -1280,6 +1283,23 @@ async fn tokio_main() -> Result<()> { let mut config = Config::from_cli().map_err(|e| anyhow::anyhow!("configuration error: {e}"))?; + // ── Boot-boundary: enforce "off means off" before any operational branch ── + // + // When resume_on_restart is false the UI promises "turns are dropped and + // never picked up." Delete the ledger file here — before the setup-mode + // early return below — so the guarantee holds for every boot path, not + // only the normal pool path. Silent no-op if no file exists or if the + // unlink fails (degraded-filesystem convention). + if !config.resume_on_restart { + let state_dir = config.state_dir.clone().unwrap_or_else(|| { + std::env::current_dir() + .unwrap_or_else(|_| PathBuf::from(".")) + .join(".buzz-acp") + }); + let agent_pubkey_hex_boot = config.keys.public_key().to_hex(); + ledger::delete_ledger_file(&state_dir, &agent_pubkey_hex_boot, &config.relay_url); + } + // ── Setup-mode early branch ─────────────────────────────────────────────── // // When the desktop determines an agent is not ready (missing credentials, @@ -1524,6 +1544,41 @@ async fn tokio_main() -> Result<()> { ); } + // Durable pending-turn ledger (auto-resume after restart). When the flag + // is off, the boot-boundary deletion above already removed any stale file; + // just construct the no-op disabled ledger. Otherwise load now so boot + // recovery can stage it before the main loop starts. + let agent_pubkey_hex = config.keys.public_key().to_hex(); + let (mut ledger, staged_ledger) = if config.resume_on_restart { + let state_dir = config.state_dir.clone().unwrap_or_else(|| { + std::env::current_dir() + .unwrap_or_else(|_| PathBuf::from(".")) + .join(".buzz-acp") + }); + Ledger::load( + &state_dir, + &agent_pubkey_hex, + &config.relay_url, + config.resume_ttl_secs, + ) + } else { + (Ledger::disabled(), ledger::StagedLedger::default()) + }; + + // No-ops end to end when the ledger is disabled or the stage is empty. + let mut recovered_suppression: HashSet = HashSet::new(); + if ledger.is_enabled() { + boot_recover( + &mut queue, + &mut ledger, + staged_ledger, + &channel_ids.iter().copied().collect(), + &relay.rest_client(), + &mut recovered_suppression, + ) + .await; + } + let base_prompt_content = config.base_prompt_content.take(); let ctx = Arc::new(PromptContext { mcp_servers: build_mcp_servers(&config), @@ -1703,6 +1758,12 @@ async fn tokio_main() -> Result<()> { Wake(u32, Result), } + // Boot-recovery wake: dispatch any recovered work immediately so a + // quiet harness does not wait for the first 30s maintenance cycle. + for (channel_id, thread_tags) in dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) { + typing_channels.insert(channel_id, thread_tags); + } + loop { // Whether buffered work is waiting on a lazy pool. Also gates the // retry-deadline sleep arm below: a `Failed` lifecycle keeps its @@ -1773,10 +1834,17 @@ async fn tokio_main() -> Result<()> { // indefinitely on quiet channels — dispatch_pending is only // called on relay events or pool results, neither of which // arrive when the channel is silent. + // + // has_flushable_work()'s expiry loop can dirty a channel even + // when it returns false, so sync unconditionally. if queue.has_flushable_work() { - for (channel_id, thread_tags) in dispatch_pending(&mut pool, &mut queue, &ctx) { + for (channel_id, thread_tags) in + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) + { typing_channels.insert(channel_id, thread_tags); } + } else { + sync_dirty(&mut queue, &mut ledger); } } @@ -1810,7 +1878,9 @@ async fn tokio_main() -> Result<()> { // this, batches requeued during crash recovery sit idle until the // next relay event arrives — which can be minutes on quiet channels. if respawn_collected { - for (channel_id, thread_tags) in dispatch_pending(&mut pool, &mut queue, &ctx) { + for (channel_id, thread_tags) in + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) + { typing_channels.insert(channel_id, thread_tags); } } @@ -1992,6 +2062,10 @@ async fn tokio_main() -> Result<()> { } else { 0 }; + // Purge durable live + unresolved records together — + // re-adding the channel later must not resurrect + // discarded work. + ledger.invalidate_channel(ch); // Track removed channels so checked-out agents get // their sessions stripped when they return to the pool. removed_channels.insert(ch); @@ -2019,6 +2093,10 @@ async fn tokio_main() -> Result<()> { "cleaned up after membership removal" ); } + // drain_channel marked `ch` dirty; this branch + // `continue`s below rather than falling through + // to a `dispatch_pending` call, so sync here. + sync_dirty(&mut queue, &mut ledger); } continue; } @@ -2194,17 +2272,28 @@ async fn tokio_main() -> Result<()> { // backed payload) so the cost is negligible. let event_for_steer = buzz_event.event.clone(); let prompt_tag_for_steer = prompt_tag.clone(); - let accepted = queue.push(QueuedEvent { - channel_id: buzz_event.channel_id, - event: buzz_event.event, - received_at: std::time::Instant::now(), + + // Recovered-suppression seam. If this event + // was boot-recovered (its id is in the + // suppression set), it is already in the queue + // via import_recovered — the relay replay is a + // duplicate. But if the event was unresolved + // (not fetched during boot), the live arrival + // is the resolution: admit it at its original + // seq position and clear the unresolved record. + // Either way, skip try_native_steer — recovered + // admissions wait for recovery-framed batch + // dispatch, not mid-turn steering. See + // `admit_live_event`. + let (accepted, skip_steer) = admit_live_event( + &mut queue, + &mut ledger, + &mut recovered_suppression, + buzz_event.channel_id, + buzz_event.event, prompt_tag, - }); - // 👀 — immediate "seen" reaction, only if the event - // was actually queued (not dropped by DedupMode::Drop). - // Fire-and-forget: on rare fast-failure paths the - // guard's cleanup may race with this add, leaving a - // cosmetic stale 👀. Acceptable — see ReactionGuard docs. + ); + if accepted { let rc = ctx.rest_client.clone(); let eid = event_id_hex.clone(); @@ -2212,14 +2301,7 @@ async fn tokio_main() -> Result<()> { pool::reaction_add(&rc, &eid, "👀").await; }); } - // Event is already queued. If mode requires it AND - // the channel has an in-flight task, fire cancel — - // OR take the non-cancelling (ACP steer) fork for Steer signals. - if accepted && queue.is_channel_in_flight(buzz_event.channel_id) { - // Author eligibility (owner ∪ allowlist ∪ siblings) - // is already enforced by the inbound author gate - // above, so the mid-turn signal fires for every - // event that reaches here. + if accepted && !skip_steer && queue.is_channel_in_flight(buzz_event.channel_id) { let signal = mode_gate_signal( config.multiple_event_handling, &author_hex, @@ -2258,7 +2340,7 @@ async fn tokio_main() -> Result<()> { } if pool_ready { for (channel_id, thread_tags) in - dispatch_pending(&mut pool, &mut queue, &ctx) + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) { typing_channels.insert(channel_id, thread_tags); } @@ -2284,17 +2366,28 @@ async fn tokio_main() -> Result<()> { let _ = result_rx; if !pool_ready { tracing::debug!("heartbeat_skipped_pool_not_ready"); - } else if queue.has_flushable_work() { - tracing::debug!("heartbeat_skipped_events"); - for (channel_id, thread_tags) in - dispatch_pending(&mut pool, &mut queue, &ctx) - { - typing_channels.insert(channel_id, thread_tags); - } - } else if pool.any_idle() { - dispatch_heartbeat(&mut pool, &ctx, &mut heartbeat_in_flight); } else { - tracing::debug!("heartbeat_skipped_busy"); + // has_flushable_work()'s expiry loop can dirty a channel + // even when it returns false (e.g. it auto-released a + // stuck in-flight channel that still isn't flushable for + // some other reason) — sync unconditionally so that + // dirty channel is never left unsynced. + let flushable = queue.has_flushable_work(); + if flushable { + tracing::debug!("heartbeat_skipped_events"); + for (channel_id, thread_tags) in + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) + { + typing_channels.insert(channel_id, thread_tags); + } + } else { + sync_dirty(&mut queue, &mut ledger); + if pool.any_idle() { + dispatch_heartbeat(&mut pool, &ctx, &mut heartbeat_in_flight); + } else { + tracing::debug!("heartbeat_skipped_busy"); + } + } } None } @@ -2341,6 +2434,35 @@ async fn tokio_main() -> Result<()> { } None } + // Barrier-deadline timer arm (rev 6.1): fires when the + // earliest unresolved barrier expires, even when all + // external inputs are silent. Without this, held suffix + // events on quiet channels would never dispatch. + _ = async { + match queue.next_unresolved_barrier_deadline() { + Some(deadline) => { + let tokio_deadline = tokio::time::Instant::from_std(deadline); + tokio::time::sleep_until(tokio_deadline).await; + } + None => std::future::pending().await, + } + } => { + let _ = result_rx; + let expired = queue.expire_due_unresolved_barriers(std::time::Instant::now()); + if !expired.is_empty() { + tracing::info!( + channels = ?expired, + "unresolved barrier deadline expired — releasing held suffix" + ); + sync_dirty(&mut queue, &mut ledger); + for (channel_id, thread_tags) in + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) + { + typing_channels.insert(channel_id, thread_tags); + } + } + None + } _ = shutdown_rx.changed() => { tracing::info!("shutting down"); break; @@ -2368,6 +2490,9 @@ async fn tokio_main() -> Result<()> { Some(&ctx.rest_client), ) == LoopAction::Exit { + // Sync before exit: complete_batch dirtied the queue but + // the exit bypasses dispatch_pending's sync call. + sync_dirty(&mut queue, &mut ledger); break; } if drain_ready_join_results( @@ -2383,9 +2508,14 @@ async fn tokio_main() -> Result<()> { observer.clone(), ) == LoopAction::Exit { + // Sync before exit: classify dirtied the queue but the + // exit bypasses dispatch_pending's sync call. + sync_dirty(&mut queue, &mut ledger); break; } - for (channel_id, thread_tags) in dispatch_pending(&mut pool, &mut queue, &ctx) { + for (channel_id, thread_tags) in + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) + { typing_channels.insert(channel_id, thread_tags); } } @@ -2406,9 +2536,14 @@ async fn tokio_main() -> Result<()> { ); if pool.live_count() == 0 && !any_respawn_in_flight(&crash_history) { tracing::error!("all agents dead — exiting"); + // Sync before exit: complete_batch dirtied the queue but + // the exit bypasses dispatch_pending's sync call. + sync_dirty(&mut queue, &mut ledger); break; } - for (channel_id, thread_tags) in dispatch_pending(&mut pool, &mut queue, &ctx) { + for (channel_id, thread_tags) in + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) + { typing_channels.insert(channel_id, thread_tags); } } @@ -2550,7 +2685,9 @@ async fn tokio_main() -> Result<()> { // tear down the in-flight task; on its completion the // queue drains. We still try here in case the in-flight // task has already returned. - for (channel_id, thread_tags) in dispatch_pending(&mut pool, &mut queue, &ctx) { + for (channel_id, thread_tags) in + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) + { typing_channels.insert(channel_id, thread_tags); } } @@ -2577,7 +2714,7 @@ async fn tokio_main() -> Result<()> { None, ); for (channel_id, thread_tags) in - dispatch_pending(&mut pool, &mut queue, &ctx) + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx) { typing_channels.insert(channel_id, thread_tags); } @@ -2627,42 +2764,28 @@ async fn tokio_main() -> Result<()> { // 30 s is generous for in-flight prompts to be cancelled; using // max_turn_duration here would cause Ctrl+C to hang for up to an hour. let grace = Duration::from_secs(30); - // Best-effort drain of both join_set and result_rx during the grace period. - // Tasks that finish normally send their OwnedAgent through result_rx — we - // explicitly shut them down here to reap child processes. If the grace - // period expires, remaining tasks are aborted and fall back to - // AcpClient::Drop (start_kill + try_wait — best-effort, not guaranteed). - let (rx_ref, js_ref) = pool.rx_and_join_set(); - let shutdown_result = tokio::time::timeout(grace, async { - loop { - tokio::select! { - result = js_ref.join_next() => { - match result { - Some(Err(e)) => tracing::warn!("task error during shutdown: {e}"), - Some(Ok(())) => {} - None => break, // join_set empty - } - } - maybe_result = rx_ref.recv() => { - if let Some(mut pr) = maybe_result { - let idx = pr.agent.index; - pr.agent.acp.shutdown().await; - tracing::debug!(agent = idx, "reaped checked-out agent on shutdown"); - } - // If None, channel closed — tasks are done. - } - } - } - }) + drain_shutdown_grace( + &mut pool, + &mut queue, + &mut ledger, + &config, + &removed_channels, + Some(&ctx.rest_client), + grace, + ) .await; - if shutdown_result.is_err() { - tracing::warn!("grace period expired, aborting remaining tasks"); - pool.join_set.shutdown().await; - } // Drain any remaining results that arrived after join_set drained but - // before tasks were aborted. + // before tasks were aborted. Same classification as above. while let Ok(mut pr) = pool.result_rx_try_recv() { let idx = pr.agent.index; + classify_and_complete_batch( + &mut queue, + &config, + &mut pr, + &removed_channels, + Some(&ctx.rest_client), + ); + sync_dirty(&mut queue, &mut ledger); pr.agent.acp.shutdown().await; tracing::debug!(agent = idx, "reaped late-arriving agent on shutdown"); } @@ -2747,6 +2870,67 @@ fn is_owner_control_command( && event_mentions_agent(event, agent_pubkey_hex) } +// ── admit_live_event ───────────────────────────────────────────────────────── + +/// Admit a live relay event into the queue, resolving it against boot +/// recovery's suppression set and unresolved barrier first. +/// +/// Returns `(accepted, skip_steer)`: `accepted` is whether the event landed +/// in the queue (mirrors `EventQueue::push`'s admission result on the normal +/// path); `skip_steer` tells the caller whether to bypass `try_native_steer` +/// for this event — recovered admissions (whether a suppressed duplicate or +/// a just-resolved unresolved record) wait for recovery-framed batch +/// dispatch, not mid-turn steering. +/// +/// This is the exact decision the main loop's relay-event arm makes inline; +/// extracted so tests can drive it directly instead of mirroring it. +fn admit_live_event( + queue: &mut EventQueue, + ledger: &mut Ledger, + recovered_suppression: &mut HashSet, + channel_id: uuid::Uuid, + event: nostr::Event, + prompt_tag: String, +) -> (bool, bool) { + let event_id_hex = event.id.to_hex(); + let is_recovered_suppressed = recovered_suppression.remove(&event_id_hex); + if is_recovered_suppressed { + // Already in the queue from boot import — suppress the duplicate + // live push entirely. + (false, true) + } else if let Some(unresolved) = ledger.find_unresolved(channel_id, &event_id_hex) { + // Unresolved record resolving: build a recovered QueuedEvent with + // the ledger's original seq/timestamp/cap_exempt/prompt_tag. + // The persisted `prompt_tag` is used rather than the live match's + // tag — a config or rule-priority change between admission and + // restart must not alter recovery framing. + let recovered = QueuedEvent::from_recovered( + channel_id, + event, + unresolved.prompt_tag.clone(), + unresolved.admission_seq, + unresolved.enqueued_at_unix, + unresolved.cap_exempt, + ); + queue.admit_recovered(channel_id, recovered); + ledger.resolve_unresolved(channel_id, &event_id_hex); + sync_dirty(queue, ledger); + (true, true) + } else { + let ok = queue.push(QueuedEvent::new( + channel_id, + event, + std::time::Instant::now(), + prompt_tag, + )); + // While the channel has an active unresolved barrier the steer + // side-door is suppressed: the fresh live event queues normally and + // waits behind the barrier hole rather than bypassing it. + let skip_steer = queue.has_active_unresolved_barrier(channel_id); + (ok, skip_steer) + } +} + // ── signal_in_flight_task ───────────────────────────────────────────────────── /// Decide which [`ControlSignal`] (if any) to send to an in-flight turn when a @@ -2848,6 +3032,13 @@ fn try_native_steer( event, prompt_tag: prompt_tag.clone(), received_at: std::time::Instant::now(), + // Synthetic — built only to render the steer body via + // `format_event_block`; never persisted to the queue or ledger, so + // the recovery fields are inert placeholders. + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }; let event_block = queue::format_event_block(channel_id, None, &be, None); let body = format!("{header}\n\n[Buzz event: {prompt_tag}]\n{event_block}\n\n{closing}"); @@ -2904,12 +3095,234 @@ fn try_native_steer( } } +// ── boot_recover ────────────────────────────────────────────────────────────── + +/// Maximum event ids fetched per REST `/query` chunk during boot recovery. +/// One `Filter` with `ids(chunk)` per request — the relay is queried by +/// id-set, not per-id. +const BOOT_RECOVERY_CHUNK_SIZE: usize = 100; + +/// Overall wall-clock budget for the boot-recovery REST fetch phase, +/// regardless of ledger cardinality. +const BOOT_RECOVERY_FETCH_DEADLINE: Duration = Duration::from_secs(60); + +/// Unresolved-barrier timeout measured from boot commit (after fetch +/// reconciliation completes), not from the start of the fetch phase. +const BOOT_RECOVERY_BARRIER_DEADLINE: Duration = Duration::from_secs(60); + +/// Boot recovery (rev 6.1): stage → membership gate → TTL (already applied +/// by `Ledger::load`) → chunked id-set fetch under one global deadline, +/// each chunk reconciled per-event → `import_recovered` in global +/// `admission_seq` order (cap promotion) → suppression set → unresolved +/// barrier registration → single commit merging unresolved. +/// +/// A no-op (immediate return) when the staged ledger has nothing to +/// recover. Never blocks boot past `BOOT_RECOVERY_FETCH_DEADLINE`: any trigger +/// not fetched by then is retained as unresolved rather than dropped. +async fn boot_recover( + queue: &mut EventQueue, + ledger: &mut Ledger, + staged: ledger::StagedLedger, + live_channels: &HashSet, + rest: &relay::RestClient, + recovered_suppression: &mut HashSet, +) { + if staged.channels.is_empty() { + return; + } + + // Membership gate (fixes F5): a ledger channel this boot didn't + // discover/subscribe to is dropped wholesale — no speculative + // re-subscription as validation. + let mut gated: Vec<(Uuid, ledger::LedgerRecord)> = Vec::new(); + let mut dropped_channels = 0usize; + for (channel_id, records) in staged.channels { + if live_channels.contains(&channel_id) { + gated.extend(records.into_iter().map(|r| (channel_id, r))); + } else { + dropped_channels += 1; + } + } + if dropped_channels > 0 { + tracing::info!( + dropped_channels, + "boot recovery: dropped ledger channel(s) no longer in this boot's membership" + ); + } + let fetch_deadline = tokio::time::Instant::now() + BOOT_RECOVERY_FETCH_DEADLINE; + let all_ids: Vec = gated.iter().map(|(_, r)| r.event_id.clone()).collect(); + let by_id: HashMap<&str, &(Uuid, ledger::LedgerRecord)> = gated + .iter() + .map(|entry| (entry.1.event_id.as_str(), entry)) + .collect(); + + let mut fetched: HashMap = HashMap::new(); + 'chunks: for chunk in all_ids.chunks(BOOT_RECOVERY_CHUNK_SIZE) { + if tokio::time::Instant::now() >= fetch_deadline { + tracing::warn!("boot recovery: global fetch deadline reached — remaining triggers unresolved-retained"); + break 'chunks; + } + let ids: Vec = chunk + .iter() + .filter_map(|id| nostr::EventId::from_hex(id).ok()) + .collect(); + if ids.is_empty() { + continue; + } + let filter = nostr::Filter::new().ids(ids); + let remaining = fetch_deadline.saturating_duration_since(tokio::time::Instant::now()); + let query = tokio::time::timeout(remaining, rest.query(std::slice::from_ref(&filter))); + let response = match query.await { + Ok(Ok(v)) => v, + Ok(Err(e)) => { + tracing::warn!( + "boot recovery: chunk fetch failed: {e} — chunk unresolved-retained" + ); + continue; + } + Err(_) => { + tracing::warn!("boot recovery: fetch deadline reached mid-chunk — remaining triggers unresolved-retained"); + break 'chunks; + } + }; + let Some(events) = response.as_array() else { + tracing::warn!( + "boot recovery: chunk response was not a JSON array — chunk unresolved-retained" + ); + continue; + }; + let requested: HashSet<&str> = chunk.iter().map(String::as_str).collect(); + for raw in events { + // Responses are reconciled, never trusted: deserialize, verify + // signature/id, require id in the requested chunk and + // its `h` tag to match the ledger record's channel. + let event = match serde_json::from_value::(raw.clone()) { + Ok(ev) => ev, + Err(e) => { + tracing::warn!( + "boot recovery: malformed event in chunk response: {e} — skipped" + ); + continue; + } + }; + if event.verify().is_err() { + tracing::warn!(event_id = %event.id.to_hex(), "boot recovery: event failed signature verification — skipped"); + continue; + } + let id_hex = event.id.to_hex(); + if !requested.contains(id_hex.as_str()) { + tracing::warn!(event_id = %id_hex, "boot recovery: event id not in requested chunk — skipped"); + continue; + } + let Some((channel_id, _)) = by_id.get(id_hex.as_str()).copied() else { + continue; + }; + let ch_str = channel_id.to_string(); + let h_tag_matches = event.tags.iter().any(|tag| { + let v = tag.as_slice(); + v.len() >= 2 && v[0] == "h" && v[1] == ch_str + }); + if !h_tag_matches { + tracing::warn!(event_id = %id_hex, channel_id = %channel_id, "boot recovery: event's h-tag does not match ledger record's channel — skipped"); + continue; + } + fetched.entry(id_hex).or_insert(event); + } + } + + // Epoch promotion: compute per channel from ALL gated records before + // the fetch split. If no record in the channel's full snapshot is + // exempt, every record (fetched and unresolved) promotes to exempt. + let mut promote_channels: HashSet = HashSet::new(); + { + let mut has_exempt: HashMap = HashMap::new(); + for (channel_id, record) in &gated { + let entry = has_exempt.entry(*channel_id).or_insert(false); + if record.cap_exempt { + *entry = true; + } + } + for (channel_id, exempt) in has_exempt { + if !exempt { + promote_channels.insert(channel_id); + } + } + } + + // Restore fetched events per channel via import_recovered, in global + // admission_seq order; unfetched ids stay unresolved-retained. The + // settled cap class comes from the whole-snapshot promotion above. + let mut per_channel: HashMap> = HashMap::new(); + let mut max_seq: u64 = 0; + for (channel_id, mut record) in gated { + max_seq = max_seq.max(record.admission_seq); + if promote_channels.contains(&channel_id) { + record.cap_exempt = true; + } + match fetched.remove(&record.event_id) { + Some(event) => { + recovered_suppression.insert(record.event_id.clone()); + per_channel + .entry(channel_id) + .or_default() + .push(QueuedEvent::from_recovered( + channel_id, + event, + record.prompt_tag, + record.admission_seq, + record.enqueued_at_unix, + record.cap_exempt, + )); + } + None => ledger.add_unresolved(channel_id, record), + } + } + queue.set_next_admission_seq(max_seq); + for (channel_id, events) in per_channel { + queue.import_recovered(channel_id, events); + } + + // Register the unresolved ordering barrier per channel before the + // single commit, so the queue's flush gate is armed from boot. + // The barrier deadline starts NOW (after fetch reconciliation), not + // from the start of the fetch phase — a slow bridge must not consume + // the resolution window. + let barrier_deadline = std::time::Instant::now() + BOOT_RECOVERY_BARRIER_DEADLINE; + for channel_id in ledger.unresolved_channels() { + queue.set_unresolved_barrier( + channel_id, + ledger.unresolved_seqs(channel_id), + barrier_deadline, + ); + } + + // Single transactional commit: live ∪ unresolved, computed + // after every import above. Resumes normal per-mutation sync from here. + let mut live: HashMap> = HashMap::new(); + for channel_id in queue.take_dirty_channels() { + live.insert(channel_id, queue.recoverable_triggers(channel_id)); + } + ledger.commit(live); +} + // ── dispatch_pending ────────────────────────────────────────────────────────── +/// Drain every channel a public `&mut queue` mutator touched since the +/// last call and persist each one's current recoverable set. This is the +/// one sync rule the auto-resume ledger design relies on — call it after +/// *every* public queue mutation so the durable mirror never drifts from +/// the in-memory queue. +fn sync_dirty(queue: &mut EventQueue, ledger: &mut Ledger) { + for channel_id in queue.take_dirty_channels() { + ledger.sync(channel_id, queue.recoverable_triggers(channel_id)); + } +} + /// Flush queued work to available agents. fn dispatch_pending( pool: &mut AgentPool, queue: &mut EventQueue, + ledger: &mut Ledger, ctx: &Arc, ) -> Vec<(Uuid, ThreadTags)> { let mut dispatched_channels = Vec::new(); @@ -2930,8 +3343,11 @@ fn dispatch_pending( None => { let pending = queue.pending_channels(); tracing::debug!(pending_channels = pending, "pool_exhausted"); - queue.requeue_preserve_timestamps(batch); - queue.mark_complete(channel_id); + queue.complete_batch( + channel_id, + Some(batch), + queue::BatchDisposition::PreserveTimestamps, + ); break; } }; @@ -2991,6 +3407,7 @@ fn dispatch_pending( ); dispatched_channels.push((channel_id, typing_scope)); } + sync_dirty(queue, ledger); tracing::debug!( dispatched = dispatched_channels.len(), queue_depth = queue.pending_channels(), @@ -3051,151 +3468,271 @@ fn spawn_failure_notice( } } -#[allow(clippy::too_many_arguments)] -fn handle_prompt_result( - pool: &mut AgentPool, +/// Classify a completed prompt's batch (if any) into a [`BatchDisposition`] +/// and apply it via [`EventQueue::complete_batch`], posting a dead-letter +/// failure notice when retries are exhausted. Shared by the normal +/// completion path (`handle_prompt_result`) and the shutdown-drain paths +/// paths: both apply the identical disposition logic, but the shutdown +/// paths skip everything else here (respawn, heartbeat +/// bookkeeping, dispatch) — the caller decides what happens after. +fn classify_and_complete_batch( queue: &mut EventQueue, config: &Config, - mut result: PromptResult, - heartbeat_in_flight: &mut bool, + result: &mut PromptResult, removed_channels: &HashSet, - crash_history: &mut [SlotCircuit], - respawn_tx: &mpsc::Sender, - respawn_tasks: &mut tokio::task::JoinSet<()>, - observer: Option, rest_client: Option<&relay::RestClient>, -) -> LoopAction { - let before = pool.task_map().len(); - let agent_index = result.agent.index; - pool.task_map_mut() - .retain(|_, meta| meta.agent_index != agent_index); - debug_assert_eq!(before, pool.task_map().len() + 1); - - // The hard-timeout death_message (below) must describe the batch's - // *actual* fate, not just the `recently_active` eligibility flag — a - // recently-active batch that exhausts the retry budget in queue.requeue() - // is dead-lettered same as an immediate one, and both differ from a - // channel-removed drop or a heartbeat call with no batch at all. Each - // branch below records what actually happened; only the hard-timeout - // match arm in the death_message construction reads it. +) -> Option<&'static str> { + // Returns the hard-timeout fate suffix for the caller's death_message + // construction (only meaningful when outcome is Timeout(Hard { .. }); None + // for every other outcome). let mut hard_timeout_fate_suffix: Option<&'static str> = None; - - // Requeue BEFORE mark_complete: requeue() sets retry_after with a future - // deadline, and mark_complete() checks for it to decide whether to preserve - // retry_counts. If mark_complete runs first, retry_counts is cleared and - // every retry starts at attempt 1 — defeating exponential backoff and - // dead-letter protection. if let Some(batch) = result.batch.take() { + let channel_id = batch.channel_id; // Don't requeue batches for channels the agent was removed from — // those events are stale and should be silently dropped. - if !removed_channels.contains(&batch.channel_id) { - if matches!( - result.outcome, - PromptOutcome::Cancelled | PromptOutcome::CancelDrainTimeout(_) - ) { - // Cancel re-prompt: store as cancelled events so flush_next() - // merges them into the next FlushBatch.cancelled_events, - // enabling the annotated merged-prompt format. The batch's - // cancel_reason (set by the pool task per the control signal) - // selects steer vs interrupt framing. It is always set on this - // path; if somehow unset, fall back to the gentler Steer framing - // — consistent with MergeFraming::for_reason(None) and the - // system default — rather than telling the agent to supersede. - // - // CancelDrainTimeout shares this path with Cancelled: a failed - // 5s drain after a control-signal cancel is a cleanup-deadline - // problem, not the deterministic hard-cap death below — the - // original batch must survive with no retry/dead-letter - // accounting, same as a clean cancel. - let reason = batch.cancel_reason.unwrap_or(CancelReason::Steer); - queue.requeue_as_cancelled(batch, reason); - } else if matches!( - result.outcome, - PromptOutcome::Timeout(TimeoutKind::Hard { - recently_active: false - }) - ) { - tracing::error!( - channel_id = %batch.channel_id, - events = batch.events.len(), - "dead-lettering batch after hard-cap timeout (no recent activity) — discarding {} events", - batch.events.len(), - ); + if removed_channels.contains(&channel_id) { + tracing::debug!( + channel_id = %channel_id, + events = batch.events.len(), + "dropping failed batch for removed channel" + ); + queue.complete_batch(channel_id, Some(batch), BatchDisposition::Dropped); + hard_timeout_fate_suffix = Some(" — batch dropped (channel removed)"); + } else if matches!( + result.outcome, + PromptOutcome::Cancelled | PromptOutcome::CancelDrainTimeout(_) + ) { + // Cancel re-prompt: store as cancelled events so flush_next() + // merges them into the next FlushBatch.cancelled_events, + // enabling the annotated merged-prompt format. The batch's + // cancel_reason (set by the pool task per the control signal) + // selects steer vs interrupt framing. It is always set on this + // path; if somehow unset, fall back to the gentler Steer framing + // — consistent with MergeFraming::for_reason(None) and the + // system default — rather than telling the agent to supersede. + // + // CancelDrainTimeout shares this path with Cancelled: a failed + // 5s drain after a control-signal cancel is a cleanup-deadline + // problem, not the deterministic hard-cap death below — the + // original batch must survive with no retry/dead-letter + // accounting, same as a clean cancel. + let reason = batch.cancel_reason.unwrap_or(CancelReason::Steer); + queue.complete_batch(channel_id, Some(batch), BatchDisposition::Cancelled(reason)); + } else if matches!( + result.outcome, + PromptOutcome::Timeout(TimeoutKind::Hard { + recently_active: false + }) + ) { + // Hard-cap timeout with no recent activity is deterministic: + // re-running the same task will reproduce the same death. Dead-letter + // immediately without requeueing so the channel isn't subjected to + // up to 10 × 1-hour retry cycles. + tracing::error!( + channel_id = %channel_id, + events = batch.events.len(), + "dead-lettering batch after hard-cap timeout (no recent activity) — discarding {} events", + batch.events.len(), + ); + let content = format!( + "⚠️ I couldn't process the last request (the turn exceeded the maximum duration ({}s)). Please re-send if it's still needed.", + config.max_turn_duration_secs + ); + spawn_failure_notice(rest_client, &batch, content); + queue.complete_batch(channel_id, Some(batch), BatchDisposition::Dropped); + hard_timeout_fate_suffix = Some(" — dead-lettered (no recent activity)"); + } else if matches!( + result.outcome, + PromptOutcome::Timeout(TimeoutKind::Hard { + recently_active: true + }) + ) { + // Hard-cap timeout with recent activity — requeue for retry; dead-letter + // only once the retry budget is exhausted. + tracing::warn!( + channel_id = %channel_id, + events = batch.events.len(), + "hard-cap timeout with recent activity — requeueing for retry" + ); + if let Some(dead) = + queue.complete_batch(channel_id, Some(batch), BatchDisposition::Retry) + { let content = format!( - "⚠️ I couldn't process the last request (the turn exceeded the maximum duration ({}s)). Please re-send if it's still needed.", + "⚠️ I couldn't process the last request after multiple retries (the turn exceeded the maximum duration ({}s)). Please re-send if it's still needed.", config.max_turn_duration_secs ); - spawn_failure_notice(rest_client, &batch, content); - hard_timeout_fate_suffix = Some(" — dead-lettered (no recent activity)"); - } else if matches!( - result.outcome, - PromptOutcome::Timeout(TimeoutKind::Hard { - recently_active: true - }) - ) { - tracing::warn!( - channel_id = %batch.channel_id, - events = batch.events.len(), - "hard-cap timeout with recent activity — requeueing for retry" - ); - if let Some(dead) = queue.requeue(batch) { - let content = format!( - "⚠️ I couldn't process the last request after multiple retries (the turn exceeded the maximum duration ({}s)). Please re-send if it's still needed.", - config.max_turn_duration_secs - ); - spawn_failure_notice(rest_client, &dead, content); - hard_timeout_fate_suffix = Some(" — dead-lettered (retry budget exhausted)"); - } else { - hard_timeout_fate_suffix = Some(" — requeued for retry (recently active)"); + spawn_failure_notice(rest_client, &dead, content); + hard_timeout_fate_suffix = Some(" — dead-lettered (retry budget exhausted)"); + } else { + hard_timeout_fate_suffix = Some(" — requeued for retry (recently active)"); + } + } else if matches!(&result.outcome, PromptOutcome::Error(e) if is_auth_error(e)) { + // Auth errors are non-retryable: the token won't self-repair + // between retries, so requeueing only wastes attempt slots and + // delays the visible failure. Dead-letter immediately and tell + // the user to re-authenticate the CLI. + tracing::warn!( + channel_id = %channel_id, + events = batch.events.len(), + "dead-lettering batch immediately — non-retryable auth error" + ); + let content = "⚠️ I couldn't process the last request: authentication failed. \ + Please re-authenticate the CLI (e.g. run `claude /login` or `codex login`) \ + and then re-send." + .to_string(); + spawn_failure_notice(rest_client, &batch, content); + queue.complete_batch(channel_id, Some(batch), BatchDisposition::Dropped); + } else { + let outcome_reason = match &result.outcome { + PromptOutcome::Timeout(TimeoutKind::Idle) => "the turn timed out".to_string(), + PromptOutcome::Timeout(TimeoutKind::Hard { .. }) => { + "the turn exceeded the maximum duration".to_string() } - } else if matches!(&result.outcome, PromptOutcome::Error(e) if is_auth_error(e)) { - // Auth errors are non-retryable: the token won't self-repair - // between retries, so requeueing only wastes attempt slots and - // delays the visible failure. Dead-letter immediately and tell - // the user to re-authenticate the CLI. - tracing::warn!( - channel_id = %batch.channel_id, - events = batch.events.len(), - "dead-lettering batch immediately — non-retryable auth error" - ); - let content = "⚠️ I couldn't process the last request: authentication failed. \ - Please re-authenticate the CLI (e.g. run `claude /login` or `codex login`) \ - and then re-send." - .to_string(); - spawn_failure_notice(rest_client, &batch, content); - } else if let Some(dead) = queue.requeue(batch) { - let reason = match &result.outcome { - PromptOutcome::Timeout(TimeoutKind::Idle) => "the turn timed out".to_string(), - PromptOutcome::Timeout(TimeoutKind::Hard { .. }) => { - "the turn exceeded the maximum duration".to_string() - } - PromptOutcome::AgentExited => "the agent process exited".to_string(), - PromptOutcome::Error(e) => format!("{e}"), - _ => "repeated failures".to_string(), - }; + PromptOutcome::AgentExited => "the agent process exited".to_string(), + PromptOutcome::Error(e) => format!("{e}"), + _ => "repeated failures".to_string(), + }; + if let Some(dead) = + queue.complete_batch(channel_id, Some(batch), BatchDisposition::Retry) + { + // Dead-lettered: retries exhausted and the events are gone. + // Post a visible notice so the channel isn't left waiting on + // a turn that will never happen. let content = format!( - "⚠️ I couldn't process the last request after multiple retries ({reason}). Please re-send if it's still needed." + "⚠️ I couldn't process the last request after multiple retries ({outcome_reason}). Please re-send if it's still needed." ); spawn_failure_notice(rest_client, &dead, content); } - } else { - tracing::debug!( - channel_id = %batch.channel_id, - events = batch.events.len(), - "dropping failed batch for removed channel" - ); - hard_timeout_fate_suffix = Some(" — batch dropped (channel removed)"); } + } else if let PromptSource::Channel(ch) = &result.source { + queue.complete_batch(*ch, None, BatchDisposition::Success); } + hard_timeout_fate_suffix +} - match &result.source { - PromptSource::Channel(ch) => queue.mark_complete(*ch), - PromptSource::Heartbeat => *heartbeat_in_flight = false, - } - - // Strip sessions for channels the agent was removed from while this - // agent was checked out. This covers the gap where invalidate_channel_sessions - // only touches idle agents. +/// Best-effort drain of both `join_set` and `result_rx` during the shutdown +/// grace period. Tasks that finish normally send their `OwnedAgent` through +/// `result_rx` — reaped here to shut down child processes. If `grace` +/// expires with tasks still outstanding, the caller must abort the +/// remaining join_set tasks and fall back to `AcpClient::Drop` (best-effort, +/// not guaranteed). +/// +/// Extracted from `run()`'s shutdown sequence so tests can drive the real +/// `select!` arms — including a task that panics mid-grace and a result +/// that arrives mid-grace — instead of calling +/// `queue.complete_batch()` directly and asserting the same disposition the +/// production code was supposed to apply. +async fn drain_shutdown_grace( + pool: &mut AgentPool, + queue: &mut EventQueue, + ledger: &mut Ledger, + config: &Config, + removed_channels: &HashSet, + rest_client: Option<&relay::RestClient>, + grace: Duration, +) { + let (rx_ref, js_ref, task_map_ref) = pool.rx_join_set_and_task_map(); + let shutdown_result = tokio::time::timeout(grace, async { + loop { + tokio::select! { + result = js_ref.join_next_with_id() => { + match result { + Some(Err(join_error)) => { + tracing::warn!("task error during shutdown: {join_error}"); + // A task that panics during the shutdown grace + // period produces no PromptResult, so its + // in-flight triggers would otherwise stay stale + // in the ledger mirror — Queue mode skipping + // retry accounting, Drop mode resurrecting a + // batch the normal panic policy would have + // dropped. Apply the same queue-only panic + // disposition `recover_panicked_agent` uses, + // minus respawn/dispatch (shutdown is tearing + // down, not recovering). + let task_id = join_error.id(); + if let Some(meta) = task_map_ref.remove(&task_id) { + if let Some(ch) = meta.channel_id { + let disposition = if removed_channels.contains(&ch) { + BatchDisposition::Dropped + } else { + BatchDisposition::Retry + }; + queue.complete_batch(ch, meta.recoverable_batch, disposition); + sync_dirty(queue, ledger); + } + } + } + Some(Ok((_, ()))) => {} + None => break, // join_set empty + } + } + maybe_result = rx_ref.recv() => { + if let Some(mut pr) = maybe_result { + let idx = pr.agent.index; + classify_and_complete_batch( + queue, + config, + &mut pr, + removed_channels, + rest_client, + ); + sync_dirty(queue, ledger); + pr.agent.acp.shutdown().await; + tracing::debug!(agent = idx, "reaped checked-out agent on shutdown"); + } + // If None, channel closed — tasks are done. + } + } + } + }) + .await; + if shutdown_result.is_err() { + tracing::warn!("grace period expired, aborting remaining tasks"); + pool.join_set.shutdown().await; + } +} + +#[allow(clippy::too_many_arguments)] +fn handle_prompt_result( + pool: &mut AgentPool, + queue: &mut EventQueue, + config: &Config, + mut result: PromptResult, + heartbeat_in_flight: &mut bool, + removed_channels: &HashSet, + crash_history: &mut [SlotCircuit], + respawn_tx: &mpsc::Sender, + respawn_tasks: &mut tokio::task::JoinSet<()>, + observer: Option, + rest_client: Option<&relay::RestClient>, +) -> LoopAction { + let before = pool.task_map().len(); + let agent_index = result.agent.index; + pool.task_map_mut() + .retain(|_, meta| meta.agent_index != agent_index); + debug_assert_eq!(before, pool.task_map().len() + 1); + + // The hard-timeout death_message (below) must describe the batch's + // *actual* fate, not just the `recently_active` eligibility flag — a + // recently-active batch that exhausts the retry budget in queue.requeue() + // is dead-lettered same as an immediate one, and both differ from a + // channel-removed drop or a heartbeat call with no batch at all. Each + // branch below records what actually happened; only the hard-timeout + // match arm in the death_message construction reads it. + // + // `classify_and_complete_batch` owns all disposition logic (one atomic + // completion writer) and returns the fate suffix for this + // death_message. + let hard_timeout_fate_suffix = + classify_and_complete_batch(queue, config, &mut result, removed_channels, rest_client); + + if let PromptSource::Heartbeat = &result.source { + *heartbeat_in_flight = false; + } + + // Strip sessions for channels the agent was removed from while this + // agent was checked out. This covers the gap where invalidate_channel_sessions + // only touches idle agents. for ch in removed_channels { result.agent.state.invalidate_channel(ch); } @@ -3440,25 +3977,21 @@ fn recover_panicked_agent( }; let i = meta.agent_index; - // Requeue BEFORE mark_complete (same rationale as handle_prompt_result). - if let Some(batch) = meta.recoverable_batch { - if let Some(ch) = meta.channel_id { - if !removed_channels.contains(&ch) { - // Dead-letter on exhaustion is logged inside requeue(); a - // panic path has no outcome to report, so no notice here. - let _ = queue.requeue(batch); - tracing::warn!("requeued batch for panicked agent {i}"); - } else { - tracing::debug!( - channel_id = %ch, - "dropping panicked batch for removed channel" - ); - } - } - } - + // complete_batch owns disposition + mark_complete + dirty-tracking + // atomically (same rationale as handle_prompt_result). if let Some(ch) = meta.channel_id { - queue.mark_complete(ch); + let disposition = if removed_channels.contains(&ch) { + tracing::debug!( + channel_id = %ch, + "dropping panicked batch for removed channel" + ); + BatchDisposition::Dropped + } else { + // Dead-letter on exhaustion is logged inside complete_batch(); a + // panic path has no outcome to report, so no notice here. + BatchDisposition::Retry + }; + queue.complete_batch(ch, meta.recoverable_batch, disposition); typing_channels.remove(&ch); tracing::warn!("cleared wedged in-flight channel {ch} from panicked agent {i}"); } else { @@ -5035,6 +5568,9 @@ mod build_mcp_servers_tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + resume_on_restart: true, + resume_ttl_secs: 0, + state_dir: None, } } @@ -5256,6 +5792,9 @@ mod error_outcome_emission_tests { agent_owner: None, no_base_prompt: false, base_prompt_content: None, + resume_on_restart: true, + resume_ttl_secs: 0, + state_dir: None, } } @@ -5368,6 +5907,97 @@ mod error_outcome_emission_tests { turn_errors.len() } + #[tokio::test] + async fn channel_released_after_ok_completion_allows_next_dispatch() { + // Real main-loop cycle: push -> flush_next (marks in-flight) -> + // Ok completion via handle_prompt_result -> channel must be + // released so the next event can dispatch. + // + // Regression test for the merge-commit defect where `handle_prompt_result` + // kept the inline requeue block (which never calls mark_complete) instead + // of routing through `classify_and_complete_batch` (which does). Without + // the fix, every channel wedges permanently after its first turn. + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev1 = EventBuilder::new(Kind::Custom(9), "first") + .sign_with_keys(&keys) + .unwrap(); + let ev2 = EventBuilder::new(Kind::Custom(9), "second") + .sign_with_keys(&keys) + .unwrap(); + + let mut queue = EventQueue::new(config::DedupMode::Queue); + queue.push(crate::queue::QueuedEvent::for_test( + ch, + ev1, + std::time::Instant::now(), + "test".into(), + )); + let _batch = queue.flush_next().expect("first flush"); + assert!(queue.is_channel_in_flight(ch)); + + let agent = dummy_agent(0).await; + let mut pool = AgentPool::from_slots(vec![None]); + let task_id = pool.join_set.spawn(async {}).id(); + pool.task_map_mut().insert( + task_id, + crate::pool::TaskMeta { + agent_index: 0, + channel_id: Some(ch), + turn_id: "t".to_string(), + recoverable_batch: None, + control_tx: None, + steer_tx: None, + }, + ); + let config = test_config(); + let mut heartbeat_in_flight = false; + let removed_channels = HashSet::new(); + let mut crash_history = vec![SlotCircuit { + crash_times: Vec::new(), + open_until: None, + respawn_in_flight: false, + }]; + let (respawn_tx, _respawn_rx) = mpsc::channel(8); + let mut respawn_tasks = tokio::task::JoinSet::new(); + + let result = PromptResult { + agent, + source: PromptSource::Channel(ch), + turn_id: "t".to_string(), + outcome: PromptOutcome::Ok(crate::acp::StopReason::EndTurn), + batch: None, + }; + handle_prompt_result( + &mut pool, + &mut queue, + &config, + result, + &mut heartbeat_in_flight, + &removed_channels, + &mut crash_history, + &respawn_tx, + &mut respawn_tasks, + None, + None, + ); + + assert!( + !queue.is_channel_in_flight(ch), + "channel must be released after Ok completion" + ); + queue.push(crate::queue::QueuedEvent::for_test( + ch, + ev2, + std::time::Instant::now(), + "test".into(), + )); + assert!( + queue.flush_next().is_some(), + "second event must dispatch after the turn completed" + ); + } + #[tokio::test] async fn agent_exited_emits_exactly_one_feed_event() { assert_eq!(turn_errors_emitted_for(PromptOutcome::AgentExited).await, 1); @@ -5550,11 +6180,11 @@ mod error_outcome_emission_tests { .unwrap(); FlushBatch { channel_id: Uuid::new_v4(), - events: vec![BatchEvent { + events: vec![BatchEvent::for_test( event, - prompt_tag: "test".into(), - received_at: std::time::Instant::now(), - }], + "test".into(), + std::time::Instant::now(), + )], cancelled_events: vec![], cancel_reason: None, } @@ -5660,6 +6290,10 @@ mod error_outcome_emission_tests { event, prompt_tag: "test".into(), received_at: std::time::Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -5778,6 +6412,10 @@ mod error_outcome_emission_tests { .unwrap(), prompt_tag: "test".into(), received_at: std::time::Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -5871,6 +6509,10 @@ mod error_outcome_emission_tests { .unwrap(), prompt_tag: "test".into(), received_at: std::time::Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -5945,11 +6587,11 @@ mod error_outcome_emission_tests { let channel_id = Uuid::new_v4(); let batch = FlushBatch { channel_id, - events: vec![BatchEvent { - event: original_event.clone(), - prompt_tag: "test".into(), - received_at: std::time::Instant::now(), - }], + events: vec![BatchEvent::for_test( + original_event.clone(), + "test".into(), + std::time::Instant::now(), + )], cancelled_events: vec![], cancel_reason: Some(CancelReason::Steer), }; @@ -5973,12 +6615,12 @@ mod error_outcome_emission_tests { // signaling the fallback ControlSignal::Steer that ultimately times // out on drain — so it is already queued by the time // handle_prompt_result runs. - queue.push(QueuedEvent { + queue.push(QueuedEvent::for_test( channel_id, - event: new_event.clone(), - received_at: std::time::Instant::now(), - prompt_tag: "test".into(), - }); + new_event.clone(), + std::time::Instant::now(), + "test".into(), + )); let config = test_config(); let mut heartbeat_in_flight = false; let removed_channels = HashSet::new(); @@ -6270,6 +6912,10 @@ mod error_outcome_emission_tests { event, prompt_tag: "test".into(), received_at: std::time::Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -6355,6 +7001,10 @@ mod error_outcome_emission_tests { event, prompt_tag: "test".into(), received_at: std::time::Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -6674,3 +7324,1930 @@ mod observer_payload_trim_tests { assert!(leaf.contains("[elided")); } } + +// ── Plan line 246 integration tests ────────────────────────────────────────── + +#[cfg(test)] +mod boot_recovery_integration_tests { + use super::*; + use crate::config::DedupMode; + use crate::ledger::{Ledger, LedgerRecord, StagedLedger}; + use crate::queue::{EventQueue, QueuedEvent}; + use crate::relay::RestClient; + use nostr::{EventBuilder, Keys, Kind, Tag}; + use std::collections::{HashMap, HashSet}; + use std::time::{Duration, Instant}; + use tokio::io::AsyncWriteExt; + use uuid::Uuid; + + fn make_channel_event(keys: &Keys, channel_id: Uuid, content: &str) -> nostr::Event { + let h_tag = Tag::parse(["h", &channel_id.to_string()]).unwrap(); + EventBuilder::new(Kind::Custom(9), content) + .tags([h_tag]) + .sign_with_keys(keys) + .unwrap() + } + + fn ledger_record(event: &nostr::Event, seq: u64, tag: &str) -> LedgerRecord { + LedgerRecord { + event_id: event.id.to_hex(), + prompt_tag: tag.into(), + admission_seq: seq, + enqueued_at_unix: 1000 + seq, + cap_exempt: false, + } + } + + fn ledger_record_exempt(event: &nostr::Event, seq: u64, tag: &str) -> LedgerRecord { + LedgerRecord { + event_id: event.id.to_hex(), + prompt_tag: tag.into(), + admission_seq: seq, + enqueued_at_unix: 1000 + seq, + cap_exempt: true, + } + } + + fn rest_client(base_url: &str) -> RestClient { + RestClient { + http: reqwest::Client::new(), + base_url: base_url.into(), + keys: Keys::generate(), + auth_tag_json: None, + } + } + + /// Same shape as `error_outcome_emission_tests::test_config` / + /// `build_mcp_servers_tests::test_config` — duplicated here because + /// those helpers are private to their own test modules. Only used by + /// the shutdown-drain tests, which need a real `Config` to pass to + /// `drain_shutdown_grace`/`classify_and_complete_batch`. + fn test_config() -> Config { + Config { + keys: Keys::generate(), + relay_url: "ws://localhost:3000".into(), + agent_command: "goose".into(), + agent_args: vec!["acp".into()], + mcp_command: "test-mcp-server".into(), + idle_timeout_secs: config::DEFAULT_IDLE_TIMEOUT_SECS, + max_turn_duration_secs: config::DEFAULT_MAX_TURN_DURATION_SECS, + agents: 1, + heartbeat_interval_secs: 0, + turn_liveness_secs: 10, + heartbeat_prompt: None, + system_prompt: None, + team_instructions: None, + initial_message: None, + subscribe_mode: config::SubscribeMode::All, + dedup_mode: config::DedupMode::Queue, + multiple_event_handling: config::MultipleEventHandling::Queue, + ignore_self: true, + kinds_override: None, + channels_override: None, + no_mention_filter: false, + config_path: std::path::PathBuf::from("./buzz-acp.toml"), + context_message_limit: 12, + max_turns_per_session: 0, + presence_enabled: true, + typing_enabled: true, + memory_enabled: false, + model: None, + session_title: None, + permission_mode: config::PermissionMode::BypassPermissions, + respond_to: config::RespondTo::Anyone, + respond_to_allowlist: HashSet::new(), + allowed_respond_to: vec![], + persona_env_vars: vec![], + has_generated_codex_config: false, + relay_observer: false, + lazy_pool: false, + agent_owner: None, + no_base_prompt: false, + base_prompt_content: None, + resume_on_restart: true, + resume_ttl_secs: 0, + state_dir: None, + } + } + + /// Minimal `PromptContext` for driving `dispatch_pending` directly — + /// same shape as `pool::tests::make_prompt_context_impl`, duplicated + /// here because that helper is private to `pool`'s test module. + fn test_prompt_context() -> Arc { + test_prompt_context_with_dedup_mode(DedupMode::Queue) + } + + /// Variant of [`test_prompt_context`] with an explicit `dedup_mode` — + /// `dispatch_pending` reads `ctx.dedup_mode` (not the `EventQueue`'s + /// own mode) to decide whether to clone a recoverable batch into + /// `TaskMeta`, so panic/shutdown-drain tests that need to exercise + /// `DedupMode::Drop`'s "nothing recovered" behavior must set it here. + fn test_prompt_context_with_dedup_mode(dedup_mode: DedupMode) -> Arc { + let agent_keys = Keys::generate(); + Arc::new(PromptContext { + mcp_servers: vec![], + initial_message: None, + idle_timeout: Duration::from_secs(60), + max_turn_duration: Duration::from_secs(120), + turn_liveness_interval: Duration::ZERO, + dedup_mode, + system_prompt: None, + session_title: None, + team_instructions: None, + heartbeat_prompt: None, + base_prompt: None, + cwd: ".".to_string(), + rest_client: rest_client("http://127.0.0.1:0"), + channel_info: pool::ChannelInfoResolver::new( + HashMap::new(), + rest_client("http://127.0.0.1:0"), + ), + context_message_limit: 0, + max_turns_per_session: 0, + permission_mode: crate::config::PermissionMode::Default, + agent_keys: agent_keys.clone(), + agent_owner_pubkey: None, + memory_enabled: false, + harness_name: "goose".to_string(), + relay_url: "ws://localhost:3000".to_string(), + }) + } + + /// Spawn a real but inert agent subprocess (`cat`) so `dispatch_pending` + /// has a genuine `OwnedAgent` to claim — mirrors `dummy_agent` in the + /// top-level test module (private there, duplicated here for the same + /// reason as `test_prompt_context` above). + async fn dummy_agent(index: usize) -> pool::OwnedAgent { + pool::OwnedAgent { + index, + acp: acp::AcpClient::spawn("cat", &[], &[], false) + .await + .expect("spawn cat as inert agent"), + state: Default::default(), + model_capabilities: None, + desired_model: None, + model_overridden: false, + agent_name: "unknown".into(), + goose_system_prompt_supported: None, + protocol_version: 1, + } + } + + async fn mock_rest_server(events: Vec) -> (tokio::task::JoinHandle<()>, String) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let base_url = format!("http://127.0.0.1:{port}"); + let handle = tokio::spawn(async move { + let events_json = serde_json::to_string(&events).unwrap(); + loop { + let Ok((mut stream, _)) = listener.accept().await else { + break; + }; + let mut buf = vec![0u8; 8192]; + let _ = tokio::io::AsyncReadExt::read(&mut stream, &mut buf).await; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", + events_json.len(), + events_json + ); + let _ = stream.write_all(response.as_bytes()).await; + let _ = stream.shutdown().await; + } + }); + (handle, base_url) + } + + /// Like `mock_rest_server`, but serves raw JSON values instead of + /// `nostr::Event`s — lets tests inject malformed/forged/tampered + /// responses (e.g. a corrupted `sig`) that a valid `nostr::Event` could + /// never represent. + async fn mock_rest_server_values( + values: Vec, + ) -> (tokio::task::JoinHandle<()>, String) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let base_url = format!("http://127.0.0.1:{port}"); + let handle = tokio::spawn(async move { + let events_json = serde_json::to_string(&values).unwrap(); + loop { + let Ok((mut stream, _)) = listener.accept().await else { + break; + }; + let mut buf = vec![0u8; 8192]; + let _ = tokio::io::AsyncReadExt::read(&mut stream, &mut buf).await; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", + events_json.len(), + events_json + ); + let _ = stream.write_all(response.as_bytes()).await; + let _ = stream.shutdown().await; + } + }); + (handle, base_url) + } + + /// Like `mock_rest_server`, but sleeps for `delay` (a tokio timer, so it + /// respects `start_paused` + `tokio::time::advance`) after reading the + /// request and before writing the response — lets tests simulate fetch + /// latency that must be advanced through WHILE `boot_recover`'s fetch + /// loop is actually awaiting the response, not before it starts. + async fn mock_rest_server_delayed( + events: Vec, + delay: Duration, + ) -> (tokio::task::JoinHandle<()>, String) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let base_url = format!("http://127.0.0.1:{port}"); + let handle = tokio::spawn(async move { + let events_json = serde_json::to_string(&events).unwrap(); + loop { + let Ok((mut stream, _)) = listener.accept().await else { + break; + }; + let mut buf = vec![0u8; 8192]; + let _ = tokio::io::AsyncReadExt::read(&mut stream, &mut buf).await; + tokio::time::sleep(delay).await; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", + events_json.len(), + events_json + ); + let _ = stream.write_all(response.as_bytes()).await; + let _ = stream.shutdown().await; + } + }); + (handle, base_url) + } + + async fn mock_rest_server_failing() -> (tokio::task::JoinHandle<()>, String) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let base_url = format!("http://127.0.0.1:{port}"); + let handle = tokio::spawn(async move { + loop { + let Ok((mut stream, _)) = listener.accept().await else { + break; + }; + let _ = stream.shutdown().await; + } + }); + (handle, base_url) + } + + // ── Scenario 1: push → kill → boot scan → resume ───────────────────── + + #[tokio::test] + async fn test_boot_recover_resumes_queued_with_original_tag_and_flag() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "hello"); + let record_a = ledger_record(&event_a, 1, "mention"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![record_a])]), + }; + + let (server, base_url) = mock_rest_server(vec![event_a.clone()]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels: HashSet = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1); + assert_eq!(triggers[0].event_id, event_a.id.to_hex()); + assert_eq!(triggers[0].prompt_tag, "mention"); + assert_eq!(triggers[0].admission_seq, 1); + + let batch = queue.flush_next().unwrap(); + assert!(batch.events[0].restart_recovery); + + server.abort(); + } + + // ── Scenario 2: push → complete → scan → empty ─────────────────────── + + #[tokio::test] + async fn test_boot_recover_empty_ledger_produces_empty_queue() { + let staged = StagedLedger::default(); + let rest = rest_client("http://127.0.0.1:1"); + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::new(); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + assert!(queue.flush_next().is_none()); + assert!(suppression.is_empty()); + } + + // ── Scenario 3: suppression both arrival orders ────────────────────── + + #[tokio::test] + async fn test_suppression_set_prevents_duplicate_live_push() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "msg"); + let record_a = ledger_record(&event_a, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![record_a])]), + }; + + let (server, base_url) = mock_rest_server(vec![event_a.clone()]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + assert!(suppression.contains(&event_a.id.to_hex())); + + let is_suppressed = suppression.remove(&event_a.id.to_hex()); + assert!(is_suppressed); + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1, "only the boot-imported copy exists"); + + server.abort(); + } + + // "Replay-only for non-ledger events" (plan:192): an event that was + // never staged in the ledger must never appear in the suppression set, + // regardless of dedup mode — the live-admission seam's suppression + // check is keyed strictly on ids `boot_recover` actually imported. + #[tokio::test] + async fn test_suppression_does_not_affect_non_ledger_event() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_ledger = make_channel_event(&keys, ch, "recovered"); + let event_fresh = make_channel_event(&keys, ch, "never-staged"); + let record = ledger_record(&event_ledger, 1, "test"); + + let (server, base_url) = mock_rest_server(vec![event_ledger.clone()]).await; + let rest = rest_client(&base_url); + + for mode in [DedupMode::Queue, DedupMode::Drop] { + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![record.clone()])]), + }; + let mut queue = EventQueue::new(mode); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + assert!( + !suppression.contains(&event_fresh.id.to_hex()), + "a never-staged event's id must never land in the suppression set ({mode:?})" + ); + } + + server.abort(); + } + + // ── Scenario 4: membership gate drops unsubscribed channels ────────── + + #[tokio::test] + async fn test_boot_recover_membership_gate_drops_absent_channel() { + let keys = Keys::generate(); + let ch_live = Uuid::new_v4(); + let ch_gone = Uuid::new_v4(); + let ev_live = make_channel_event(&keys, ch_live, "live"); + let ev_gone = make_channel_event(&keys, ch_gone, "gone"); + let rec_live = ledger_record(&ev_live, 1, "test"); + let rec_gone = ledger_record(&ev_gone, 2, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch_live, vec![rec_live]), (ch_gone, vec![rec_gone])]), + }; + + let (server, base_url) = mock_rest_server(vec![ev_live.clone()]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::from([ch_live]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + assert_eq!(queue.recoverable_triggers(ch_live).len(), 1); + assert!(queue.recoverable_triggers(ch_gone).is_empty()); + + server.abort(); + } + + #[tokio::test] + async fn test_boot_recover_all_channels_dropped_commits_empty_ledger() { + let keys = Keys::generate(); + let ch_gone = Uuid::new_v4(); + let ev_gone = make_channel_event(&keys, ch_gone, "stale"); + + let dir = tempfile::tempdir().unwrap(); + + // Persist stale state to disk through the real API so the file + // exists before boot_recover runs. + { + let (mut ledger0, _) = + Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let trigger = queue::RecoverableTrigger { + event_id: ev_gone.id.to_hex(), + prompt_tag: "test".into(), + admission_seq: 1, + enqueued_at_unix: 1001, + cap_exempt: false, + }; + ledger0.sync(ch_gone, vec![trigger]); + } + + // Reload — staged must come from disk, not memory. + let (mut ledger, staged) = + Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + assert!( + !staged.channels.is_empty(), + "setup guard: stale record must exist on disk before recovery" + ); + + let rest = rest_client("http://127.0.0.1:1"); + let mut queue = EventQueue::new(DedupMode::Queue); + let live_channels: HashSet = HashSet::new(); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let (_ledger2, staged2) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + assert!( + staged2.channels.is_empty(), + "all-dropped boot must commit an empty ledger — stale records must not survive on disk" + ); + } + + // ── Scenario 7: failed fetch → retained → resolved on next restart ── + + #[tokio::test] + async fn test_unresolved_retained_across_restart_then_resolved() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "msg"); + let record_a = ledger_record(&event_a, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![record_a])]), + }; + + let (server, base_url) = mock_rest_server_failing().await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + assert!(ledger.find_unresolved(ch, &event_a.id.to_hex()).is_some()); + server.abort(); + + let (server2, base_url2) = mock_rest_server(vec![event_a.clone()]).await; + let rest2 = rest_client(&base_url2); + + let mut queue2 = EventQueue::new(DedupMode::Queue); + let unresolved_record = ledger.find_unresolved(ch, &event_a.id.to_hex()).unwrap(); + let staged2 = StagedLedger { + channels: HashMap::from([(ch, vec![unresolved_record])]), + }; + let mut suppression2 = HashSet::new(); + + boot_recover( + &mut queue2, + &mut ledger, + staged2, + &live_channels, + &rest2, + &mut suppression2, + ) + .await; + + let triggers = queue2.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1); + assert_eq!(triggers[0].event_id, event_a.id.to_hex()); + + server2.abort(); + } + + // ── Scenario 8: successful shutdown-grace result → no recovery ──────── + // + // All four scenarios below (8, 9, 10a, 10b) drive the real + // `drain_shutdown_grace` seam (lib.rs) — the exact `select!`/timeout + // wrapper `run()` awaits during Ctrl+C shutdown — instead of calling + // `queue.complete_batch()` directly and asserting the disposition the + // production code was "supposed to" apply. A regression that stops + // routing a shutdown-grace result/panic through `classify_and_complete_batch` + // (or drops the panic-handling arm entirely) now fails these tests. + + /// Dispatch one real in-flight task via `dispatch_pending` so the pool's + /// `task_map`/`join_set` are in the exact state `drain_shutdown_grace` + /// expects at shutdown — mirrors the production call sequence + /// (`dispatch_pending` in the main loop, then `drain_shutdown_grace` at + /// Ctrl+C) instead of hand-rolling a `TaskMeta` insert. + async fn dispatch_one_in_flight( + queue: &mut EventQueue, + ledger: &mut Ledger, + ch: Uuid, + event: &nostr::Event, + ) -> AgentPool { + queue.push(QueuedEvent::new( + ch, + event.clone(), + Instant::now(), + "test".into(), + )); + let agent = dummy_agent(0).await; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + let ctx = test_prompt_context(); + let dispatched = dispatch_pending(&mut pool, queue, ledger, &ctx); + assert_eq!(dispatched.len(), 1, "setup: exactly one batch dispatched"); + pool + } + + #[tokio::test] + async fn test_successful_shutdown_grace_result_clears_triggers() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "msg"); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut pool = dispatch_one_in_flight(&mut queue, &mut ledger, ch, &event_a).await; + assert!( + !queue.recoverable_triggers(ch).is_empty(), + "in-flight triggers present before shutdown" + ); + + // The in-flight task is `cat` — it never sends a PromptResult on its + // own, so drive success by pushing one through the same + // `result_tx` the pool hands the real spawned task, then let + // `drain_shutdown_grace` reap it. + let agent = dummy_agent(1).await; + let result_tx = pool.result_tx(); + result_tx + .send(PromptResult { + agent, + source: PromptSource::Channel(ch), + turn_id: "shutdown-test".into(), + outcome: PromptOutcome::Ok(crate::acp::StopReason::EndTurn), + batch: None, // Success path: handle_prompt_result also passes None here. + }) + .unwrap(); + + let config = test_config(); + let removed_channels = HashSet::new(); + drain_shutdown_grace( + &mut pool, + &mut queue, + &mut ledger, + &config, + &removed_channels, + None, + Duration::from_secs(5), + ) + .await; + + assert!( + queue.recoverable_triggers(ch).is_empty(), + "no triggers after successful completion drains through the real grace loop" + ); + } + + // ── Scenario 9: failed shutdown-grace result → triggers persisted ──── + + #[tokio::test] + async fn test_failed_shutdown_grace_result_persists_triggers() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "msg"); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut pool = dispatch_one_in_flight(&mut queue, &mut ledger, ch, &event_a).await; + + // Recover the batch dispatch_pending cloned into TaskMeta so the + // failure result carries the same recoverable batch the real + // read-loop would attach on an error outcome. + let recoverable_batch = pool + .task_map() + .values() + .find(|m| m.channel_id == Some(ch)) + .and_then(|m| m.recoverable_batch.clone()) + .expect("dispatch_pending must have cloned a recoverable batch in Queue mode"); + + let agent = dummy_agent(1).await; + let result_tx = pool.result_tx(); + result_tx + .send(PromptResult { + agent, + source: PromptSource::Channel(ch), + turn_id: "shutdown-test".into(), + outcome: PromptOutcome::AgentExited, + batch: Some(recoverable_batch), + }) + .unwrap(); + + let config = test_config(); + let removed_channels = HashSet::new(); + drain_shutdown_grace( + &mut pool, + &mut queue, + &mut ledger, + &config, + &removed_channels, + None, + Duration::from_secs(5), + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!( + triggers.len(), + 1, + "triggers requeued after a failure result arrives mid-grace" + ); + assert_eq!(triggers[0].event_id, event_a.id.to_hex()); + } + + // ── Scenario 10: panic during grace in both dedup modes ───────────── + + /// Drive a real join_set task panic through `drain_shutdown_grace`, + /// asserting the post-panic recoverable_triggers state for `dedup_mode`. + /// Returns `(queue, channel_id)` since the test drives channel state + /// through `recoverable_triggers`, which needs the id back. + async fn panic_during_grace(dedup_mode: DedupMode) -> (EventQueue, Uuid) { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "msg"); + + let mut queue = EventQueue::new(dedup_mode); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + queue.push(QueuedEvent::new( + ch, + event_a.clone(), + Instant::now(), + "test".into(), + )); + + let agent = dummy_agent(0).await; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + let ctx = test_prompt_context_with_dedup_mode(dedup_mode); + let dispatched = dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx); + assert_eq!(dispatched.len(), 1, "setup: exactly one batch dispatched"); + + // Real join_set task panics mid-grace: abort the dispatched task + // (the actual spawned `cat`-reading prompt task) instead of + // spawning a synthetic panicking future — this is the exact + // in-flight task drain_shutdown_grace's join_next_with_id() arm + // must classify via task_map, not a hand-inserted TaskMeta. + // `abort_all` is used (JoinSet exposes no by-id abort) — safe here + // because exactly one task is ever in flight in this harness. + assert_eq!( + pool.task_map().len(), + 1, + "exactly one in-flight task to abort" + ); + pool.join_set.abort_all(); + + let config = test_config(); + let removed_channels = HashSet::new(); + drain_shutdown_grace( + &mut pool, + &mut queue, + &mut ledger, + &config, + &removed_channels, + None, + Duration::from_secs(5), + ) + .await; + + (queue, ch) + } + + #[tokio::test] + async fn test_panic_during_grace_queue_mode_retries() { + let (queue, ch) = panic_during_grace(DedupMode::Queue).await; + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1, "Queue mode: panic retries"); + } + + #[tokio::test] + async fn test_panic_during_grace_drop_mode_recovers_nothing() { + let (queue, ch) = panic_during_grace(DedupMode::Drop).await; + assert!( + queue.recoverable_triggers(ch).is_empty(), + "Drop mode: nothing recovered after a mid-grace panic" + ); + } + + // ── Scenario 11: large ledger + hanging bridge → budget ────────────── + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_large_ledger_hanging_bridge_respects_deadline() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let mut records = Vec::new(); + let mut events = Vec::new(); + for i in 0..150 { + let ev = make_channel_event(&keys, ch, &format!("msg-{i}")); + records.push(ledger_record(&ev, i as u64, "test")); + events.push(ev); + } + + let staged = StagedLedger { + channels: HashMap::from([(ch, records)]), + }; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + let base_url = format!("http://127.0.0.1:{port}"); + let server = tokio::spawn(async move { + loop { + let Ok((mut stream, _)) = listener.accept().await else { + break; + }; + let mut buf = vec![0u8; 8192]; + let _ = tokio::io::AsyncReadExt::read(&mut stream, &mut buf).await; + tokio::time::sleep(Duration::from_secs(120)).await; + let _ = stream.shutdown().await; + } + }); + + let rest = rest_client(&base_url); + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + let start = tokio::time::Instant::now(); + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + let elapsed = start.elapsed(); + + assert!( + elapsed < Duration::from_secs(BOOT_RECOVERY_FETCH_DEADLINE.as_secs() + 15), + "boot_recover must respect its deadline, took {elapsed:?}" + ); + + let unresolved = ledger.unresolved_channels(); + assert!( + !unresolved.is_empty(), + "hanging bridge leaves triggers unresolved" + ); + + // The plan's acceptance criterion is "boot reaches dispatch within + // the global budget", not merely "returns". `boot_recover` having + // returned already proves that (the fetch loop cannot outlive + // `BOOT_RECOVERY_FETCH_DEADLINE` — asserted above); this additionally + // confirms boot reached the barrier-registration step rather than + // hanging or panicking partway through the fetch loop. + assert!( + queue.next_unresolved_barrier_deadline().is_some(), + "boot must reach the barrier-registration step, not hang in the fetch loop" + ); + + server.abort(); + } + + // ── Scenario 12: chunk reconciliation ──────────────────────────────── + + #[tokio::test] + async fn test_chunk_reconciliation_wrong_channel_event_stays_unresolved() { + let keys = Keys::generate(); + let ch_expected = Uuid::new_v4(); + let ch_wrong = Uuid::new_v4(); + let ev = make_channel_event(&keys, ch_wrong, "wrong-channel"); + let ev_id_hex = ev.id.to_hex(); + let rec = LedgerRecord { + event_id: ev_id_hex.clone(), + prompt_tag: "test".into(), + admission_seq: 1, + enqueued_at_unix: 1001, + cap_exempt: false, + }; + + let staged = StagedLedger { + channels: HashMap::from([(ch_expected, vec![rec])]), + }; + + let (server, base_url) = mock_rest_server(vec![ev]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch_expected]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + assert!(queue.recoverable_triggers(ch_expected).is_empty()); + assert!(ledger.find_unresolved(ch_expected, &ev_id_hex).is_some()); + + server.abort(); + } + + #[tokio::test] + async fn test_chunk_reconciliation_duplicate_events_import_once() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev = make_channel_event(&keys, ch, "dup"); + let rec = ledger_record(&ev, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec])]), + }; + + let (server, base_url) = mock_rest_server(vec![ev.clone(), ev.clone()]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + assert_eq!( + queue.recoverable_triggers(ch).len(), + 1, + "duplicate response imports exactly once" + ); + server.abort(); + } + + #[tokio::test] + async fn test_chunk_reconciliation_unrequested_event_id_ignored() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev_requested = make_channel_event(&keys, ch, "requested"); + let ev_extra = make_channel_event(&keys, ch, "extra-unrequested"); + let rec = ledger_record(&ev_requested, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec])]), + }; + + let (server, base_url) = mock_rest_server(vec![ev_requested.clone(), ev_extra]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1); + assert_eq!(triggers[0].event_id, ev_requested.id.to_hex()); + server.abort(); + } + + #[tokio::test] + async fn test_chunk_reconciliation_invalid_signature_stays_unresolved() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev_good = make_channel_event(&keys, ch, "valid"); + let ev_forged = make_channel_event(&keys, ch, "forged"); + let rec_good = ledger_record(&ev_good, 1, "test"); + let rec_forged = ledger_record(&ev_forged, 2, "test"); + + // Tamper the forged event's signature after signing — the id is + // still requested (matches the ledger record), but verification + // must fail, so the record must stay unresolved-retained rather + // than importing untrusted content. + let mut forged_json = serde_json::to_value(&ev_forged).unwrap(); + let tampered_sig = "0".repeat(128); + forged_json["sig"] = serde_json::Value::String(tampered_sig); + + let good_json = serde_json::to_value(&ev_good).unwrap(); + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec_good, rec_forged])]), + }; + + let (server, base_url) = mock_rest_server_values(vec![good_json, forged_json]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1, "only the validly-signed event imports"); + assert_eq!(triggers[0].event_id, ev_good.id.to_hex()); + assert!( + ledger.find_unresolved(ch, &ev_forged.id.to_hex()).is_some(), + "invalid-signature event must stay unresolved-retained, never imported" + ); + + server.abort(); + } + + #[tokio::test] + async fn test_chunk_reconciliation_omitted_id_stays_unresolved() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev_returned = make_channel_event(&keys, ch, "returned"); + let ev_omitted = make_channel_event(&keys, ch, "omitted-by-bridge"); + let rec_returned = ledger_record(&ev_returned, 1, "test"); + let rec_omitted = ledger_record(&ev_omitted, 2, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec_returned, rec_omitted])]), + }; + + // Bridge returns only one of the two requested ids — a partial + // chunk response, not a failure. + let (server, base_url) = mock_rest_server(vec![ev_returned.clone()]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1); + assert_eq!(triggers[0].event_id, ev_returned.id.to_hex()); + assert!( + ledger + .find_unresolved(ch, &ev_omitted.id.to_hex()) + .is_some(), + "requested-but-omitted id must stay unresolved-retained, never dropped" + ); + + server.abort(); + } + + // ── Scenario 13: steer short-circuit ───────────────────────────────── + + #[tokio::test] + async fn test_unresolved_resolution_skips_steer_path() { + // Drives the real admission decision (`admit_live_event`, the exact + // function the main loop's relay-event arm calls) instead of + // hand-rolling the ledger/queue calls and asserting a hardcoded + // `skip_steer = true`. A regression that stops routing resolving + // events through the unresolved-lookup branch (e.g. skips + // `find_unresolved` or returns `skip_steer = false`) now fails this + // test. + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "msg"); + let record_a = ledger_record(&event_a, 1, "test"); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut suppression: HashSet = HashSet::new(); + + ledger.add_unresolved(ch, record_a.clone()); + + // The live relay redelivers event_a — same path the main loop takes + // when a resolving event arrives for an unresolved barrier record. + let (accepted, skip_steer) = admit_live_event( + &mut queue, + &mut ledger, + &mut suppression, + ch, + event_a.clone(), + "test".into(), + ); + + assert!(accepted, "resolving event must be admitted into the queue"); + assert!( + skip_steer, + "resolved recovered event must skip native steer" + ); + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1); + assert_eq!(triggers[0].event_id, event_a.id.to_hex()); + assert!( + ledger.find_unresolved(ch, &event_a.id.to_hex()).is_none(), + "unresolved record must be consumed on resolution" + ); + } + + #[tokio::test] + async fn test_live_event_with_no_unresolved_record_takes_steer_path() { + // Companion to the scenario above: an ordinary live event with no + // matching unresolved ledger record and no suppression entry must + // go through the normal `queue.push` admission and NOT skip steer. + // Without this case, a mutation that makes `admit_live_event` + // always return `skip_steer = true` would slip through undetected. + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "unrelated"); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut suppression: HashSet = HashSet::new(); + + let (accepted, skip_steer) = admit_live_event( + &mut queue, + &mut ledger, + &mut suppression, + ch, + event_a.clone(), + "test".into(), + ); + + assert!(accepted, "ordinary live event must be admitted"); + assert!( + !skip_steer, + "an ordinary live event with no unresolved record must take the native steer path" + ); + } + + #[tokio::test] + async fn test_recovered_suppressed_event_skips_steer_and_is_not_readmitted() { + // Third branch: a duplicate live delivery for an event already + // imported by boot recovery. Must be suppressed (not pushed again) + // and must still skip steer. + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "already-imported"); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut suppression: HashSet = HashSet::from([event_a.id.to_hex()]); + + let (accepted, skip_steer) = admit_live_event( + &mut queue, + &mut ledger, + &mut suppression, + ch, + event_a.clone(), + "test".into(), + ); + + assert!( + !accepted, + "suppressed duplicate must not be pushed into the queue again" + ); + assert!(skip_steer, "suppressed duplicate must skip native steer"); + assert!( + !suppression.contains(&event_a.id.to_hex()), + "suppression entry must be consumed on the matching live delivery" + ); + } + + // ── Scenario 14: quiet-harness barrier expiry (BINDING) ────────────── + + // Not `start_paused`: `expire_due_unresolved_barriers` takes + // `std::time::Instant::now()` (real wall clock) in the production + // select! arm at :2270. Under a paused virtual clock that call can + // never observe the advance — driving it with a synthetic + // `past_deadline` (as the old version of this test did) hides that + // divergence entirely, since the synthetic value passes regardless of + // whether real time elapsed. Use a short real deadline instead and run + // the identical production expression on the real clock. + #[tokio::test] + async fn test_quiet_harness_select_timer_arm_wakes_loop() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + + let _event_a = make_channel_event(&keys, ch, "unresolved-A"); + let event_b = make_channel_event(&keys, ch, "fetched-B"); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + + let recovered_b = + QueuedEvent::from_recovered(ch, event_b.clone(), "test".into(), 2, 1002, false); + queue.import_recovered(ch, vec![recovered_b]); + + let barrier_deadline = std::time::Instant::now() + Duration::from_millis(50); + let mut unresolved_seqs = std::collections::BTreeSet::new(); + unresolved_seqs.insert(1); + queue.set_unresolved_barrier(ch, unresolved_seqs, barrier_deadline); + + queue.take_dirty_channels(); + + assert!(!queue.has_flushable_work()); + + // Same construct as the production select! timer arm at :2262-2266: + // convert the barrier's std deadline to a tokio deadline and sleep + // on it. The real clock (not virtual) must cross 50ms for this to + // resolve. + let barrier_std_deadline = queue.next_unresolved_barrier_deadline().unwrap(); + let tokio_deadline = tokio::time::Instant::from_std(barrier_std_deadline); + tokio::time::sleep_until(tokio_deadline).await; + + // Same production expression as the timer arm body at :2270: expire + // against the real, now-advanced clock — no synthetic timestamp. + let expired = queue.expire_due_unresolved_barriers(std::time::Instant::now()); + assert!( + !expired.is_empty(), + "the real clock must have crossed the barrier deadline" + ); + assert!(expired.contains(&ch)); + + sync_dirty(&mut queue, &mut ledger); + + // Drive the same post-expiry dispatch the production arm calls at + // :2277, proving B is claimed, not merely flushable. + let agent = dummy_agent(0).await; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + let ctx = test_prompt_context(); + let dispatched = dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx); + + assert_eq!( + dispatched.len(), + 1, + "B must dispatch once the barrier's real deadline has passed" + ); + assert_eq!(dispatched[0].0, ch); + } + + // ── Ledger persistence round-trip ──────────────────────────────────── + + #[tokio::test] + async fn test_boot_recover_ledger_commit_persists_to_disk() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev = make_channel_event(&keys, ch, "persist-test"); + let rec = ledger_record(&ev, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec])]), + }; + + let (server, base_url) = mock_rest_server(vec![ev.clone()]).await; + let rest = rest_client(&base_url); + + let dir = tempfile::tempdir().unwrap(); + let mut queue = EventQueue::new(DedupMode::Queue); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let (_ledger2, staged2) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + assert!( + !staged2.channels.is_empty(), + "committed data survives reload" + ); + let records = staged2.channels.get(&ch).unwrap(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].event_id, ev.id.to_hex()); + + server.abort(); + } + + // ── F1: boot-recovery wake — dispatch occurs with no external event ── + + #[tokio::test] + async fn test_boot_recover_dispatches_immediately_on_quiet_harness() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "boot-wake"); + let record_a = ledger_record(&event_a, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![record_a])]), + }; + + let (server, base_url) = mock_rest_server(vec![event_a.clone()]).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + // `has_flushable_work` is true on the old code too (boot_recover + // always left work flushable — the defect was that nothing woke up + // to *dispatch* it). Drive the actual production wake seam: the + // pre-loop `dispatch_pending` call the main loop makes immediately + // after `boot_recover` returns, with no external event, no + // maintenance tick, and no unresolved barrier. Deleting that call + // (reverting to F1-old) must fail this test. + let agent = dummy_agent(0).await; + let mut pool = AgentPool::from_slots(vec![Some(agent)]); + let ctx = test_prompt_context(); + + let dispatched = dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx); + + assert_eq!( + dispatched.len(), + 1, + "quiet-boot dispatch must claim the recovered channel's batch immediately" + ); + assert_eq!(dispatched[0].0, ch); + assert_eq!( + pool.task_map().len(), + 1, + "dispatch must spawn exactly one prompt task for the recovered batch" + ); + assert!( + !queue.has_flushable_work(), + "the recovered batch must have been claimed, not left pending" + ); + + server.abort(); + } + + // ── F2: epoch promotion from full snapshot, not just fetched subset ── + + #[tokio::test] + async fn test_boot_recover_promotion_from_full_snapshot_unfetched_promotes() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev_fetched = make_channel_event(&keys, ch, "fetched-B"); + let ev_unfetched = make_channel_event(&keys, ch, "unfetched-A"); + let rec_fetched = ledger_record(&ev_fetched, 2, "test"); + let rec_unfetched = ledger_record(&ev_unfetched, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec_unfetched, rec_fetched])]), + }; + + let (server, base_url) = mock_rest_server(vec![ev_fetched.clone()]).await; + let rest = rest_client(&base_url); + + let dir = tempfile::tempdir().unwrap(); + let mut queue = EventQueue::new(DedupMode::Queue); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1, "only fetched event in queue"); + assert!( + triggers[0].cap_exempt, + "fetched event must be promoted (no exempt in full snapshot)" + ); + + let unresolved = ledger.find_unresolved(ch, &ev_unfetched.id.to_hex()); + assert!(unresolved.is_some(), "unfetched must be unresolved"); + assert!( + unresolved.unwrap().cap_exempt, + "unresolved must also be promoted (whole-snapshot epoch rule)" + ); + + server.abort(); + } + + #[tokio::test] + async fn test_boot_recover_promotion_preserves_class_when_exempt_exists() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev_exempt = make_channel_event(&keys, ch, "exempt-A"); + let ev_counted = make_channel_event(&keys, ch, "counted-B"); + let rec_exempt = ledger_record_exempt(&ev_exempt, 1, "test"); + let rec_counted = ledger_record(&ev_counted, 2, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec_exempt, rec_counted])]), + }; + + let (server, base_url) = + mock_rest_server(vec![ev_exempt.clone(), ev_counted.clone()]).await; + let rest = rest_client(&base_url); + + let dir = tempfile::tempdir().unwrap(); + let mut queue = EventQueue::new(DedupMode::Queue); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 2); + let exempt_trigger = triggers.iter().find(|t| t.admission_seq == 1).unwrap(); + let counted_trigger = triggers.iter().find(|t| t.admission_seq == 2).unwrap(); + assert!( + exempt_trigger.cap_exempt, + "exempt record must keep its class" + ); + assert!( + !counted_trigger.cap_exempt, + "counted record must keep its class when exempt exists" + ); + + server.abort(); + } + + // An exempt record that the bridge OMITS (stays unresolved) must not + // cause the fetched, non-exempt counterpart to be promoted anyway. + // `boot_recover` decides promote_channels from the full gated snapshot + // (both records: one exempt) — so the channel is NOT in + // promote_channels, and the fetched counted record must reach + // `import_recovered` still bearing `cap_exempt = false`. Promotion must + // never be re-derived from only the fetched subset, which would see zero + // exempt records and promote it anyway. + #[tokio::test] + async fn test_boot_recover_exempt_unfetched_does_not_promote_fetched_counted() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev_exempt_unfetched = make_channel_event(&keys, ch, "exempt-omitted-by-bridge"); + let ev_counted_fetched = make_channel_event(&keys, ch, "counted-fetched"); + let rec_exempt = ledger_record_exempt(&ev_exempt_unfetched, 1, "test"); + let rec_counted = ledger_record(&ev_counted_fetched, 2, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec_exempt, rec_counted])]), + }; + + // Bridge returns only the counted record — the exempt record is + // omitted (stays unresolved), mirroring a partial chunk response. + let (server, base_url) = mock_rest_server(vec![ev_counted_fetched.clone()]).await; + let rest = rest_client(&base_url); + + let dir = tempfile::tempdir().unwrap(); + let mut queue = EventQueue::new(DedupMode::Queue); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let triggers = queue.recoverable_triggers(ch); + assert_eq!( + triggers.len(), + 1, + "only the fetched record reaches the queue" + ); + assert!( + !triggers[0].cap_exempt, + "fetched counted record must stay counted — it must not be promoted \ + just because it was the only entry import_recovered's events vec saw" + ); + + // Round-trip through commit + reload: the persisted class for the + // unresolved exempt record must also be untouched. + let unresolved = ledger.find_unresolved(ch, &ev_exempt_unfetched.id.to_hex()); + assert!(unresolved.is_some()); + assert!( + unresolved.unwrap().cap_exempt, + "unresolved exempt record must keep its class" + ); + + server.abort(); + } + + // ── F3: barrier deadline survives fetch latency ───────────────────── + + // Not `start_paused`: the barrier deadline is `std::time::Instant` + // (real wall clock), but the fetch-latency simulation lives inside the + // mock server's tokio timer. Under a paused virtual clock the two + // would never interact — `std::time::Instant::now()` cannot observe a + // virtual-clock advance — so this test needs a real sleep to actually + // race fetch latency against the barrier deadline, same as the + // large-ledger hanging-bridge test above. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_boot_recover_barrier_deadline_not_consumed_by_fetch() { + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let ev_fetched = make_channel_event(&keys, ch, "fetched-B"); + let ev_unfetched = make_channel_event(&keys, ch, "unfetched-A"); + let rec_fetched = ledger_record(&ev_fetched, 2, "test"); + let rec_unfetched = ledger_record(&ev_unfetched, 1, "test"); + + let staged = StagedLedger { + channels: HashMap::from([(ch, vec![rec_unfetched, rec_fetched])]), + }; + + // Real 50s delay inside the mock server's request handler — the + // fetch loop is genuinely awaiting the response for 50 of the 60s + // fetch budget when boot_recover reaches barrier registration. + let (server, base_url) = + mock_rest_server_delayed(vec![ev_fetched.clone()], Duration::from_secs(50)).await; + let rest = rest_client(&base_url); + + let mut queue = EventQueue::new(DedupMode::Queue); + let mut ledger = Ledger::disabled(); + let live_channels = HashSet::from([ch]); + let mut suppression = HashSet::new(); + + boot_recover( + &mut queue, + &mut ledger, + staged, + &live_channels, + &rest, + &mut suppression, + ) + .await; + + let deadline = queue.next_unresolved_barrier_deadline(); + assert!(deadline.is_some(), "barrier must be armed for unfetched A"); + + let remaining = deadline + .unwrap() + .saturating_duration_since(std::time::Instant::now()); + assert!( + remaining >= Duration::from_secs(55), + "barrier must have nearly the full 60s window remaining (got {remaining:?}), \ + not be consumed by the 50s fetch latency" + ); + + server.abort(); + } + + // ── Barrier-aware skip_steer ──────────────────────────────────────── + + #[tokio::test] + async fn test_live_event_skips_steer_while_barrier_armed() { + // A fresh live event for a channel that has an active unresolved + // barrier must return skip_steer = true (queue normally and wait + // behind the hole) rather than bypassing it through native steer. + // + // Red-on-old: before the fix, `admit_live_event`'s ordinary branch + // always returned `(ok, false)` — skip_steer was never set for + // ordinary pushes, so the assertion `skip_steer == true` failed. + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_c = make_channel_event(&keys, ch, "fresh-C"); + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut suppression: HashSet = HashSet::new(); + + // Arm a barrier for `ch` — simulates boot_recover registering an + // unfetched hole at seq 1. + let seqs = std::collections::BTreeSet::from([1u64]); + let deadline = Instant::now() + Duration::from_secs(60); + queue.set_unresolved_barrier(ch, seqs, deadline); + + // Fresh live event C arrives above the hole. + let (accepted, skip_steer) = admit_live_event( + &mut queue, + &mut ledger, + &mut suppression, + ch, + event_c.clone(), + "mention".into(), + ); + + assert!( + accepted, + "fresh event above barrier must be admitted into queue" + ); + assert!( + skip_steer, + "fresh live event must skip native steer while the channel's barrier is armed" + ); + } + + // ── Persist returns success; last_written advances only on success ── + + #[test] + fn test_failed_persist_does_not_advance_last_written_allowing_retry() { + // When persist fails (temp-file path blocked), `last_written` must + // NOT be advanced. The skip-identical check compares against + // `last_written`; if a failed write is recorded as success, the + // healing retry is silently skipped. + // + // Red-on-old: sync() advanced last_written before calling persist(), + // so the second identical sync() was suppressed even though the first + // write never reached disk. + let ch = Uuid::new_v4(); + let keys = Keys::generate(); + let event_a = make_channel_event(&keys, ch, "msg"); + + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + + // Block the .tmp path — derive the actual ledger path from the loaded + // ledger so the filename (pubkey16 + relay hash) stays in sync automatically. + let ledger_path = ledger.path().unwrap().to_path_buf(); + let tmp_path = { + let mut s = ledger_path.as_os_str().to_os_string(); + s.push(".tmp"); + std::path::PathBuf::from(s) + }; + std::fs::create_dir_all(&tmp_path).expect("create blocker dir"); + + let trigger = crate::queue::RecoverableTrigger { + event_id: event_a.id.to_hex(), + prompt_tag: "mention".into(), + admission_seq: 1, + enqueued_at_unix: 1000, + cap_exempt: false, + }; + + // First sync: write fails (path is a directory). + ledger.sync(ch, vec![trigger.clone()]); + + // Remove blocker, retry with identical data — must succeed now. + std::fs::remove_dir_all(&tmp_path).expect("remove blocker"); + ledger.sync(ch, vec![trigger.clone()]); + + let (_, staged) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + assert_eq!( + staged.channels.len(), + 1, + "after unblocked retry sync, ledger file must contain the channel record" + ); + let records = staged.channels.get(&ch).unwrap(); + assert_eq!(records.len(), 1); + assert_eq!(records[0].event_id, event_a.id.to_hex()); + } + + // ── invalidate_channel: failed persist → different channel sync → removed channel absent + + #[test] + fn test_invalidate_channel_failed_persist_heals_on_subsequent_sync() { + // Regression guard for invalidate_channel: when the invalidation + // write fails (dirty flag set), a later successful sync of a DIFFERENT + // channel must write the full map WITHOUT the removed channel, healing + // the file. + // + // Red-on-old (advance-on-success): invalidate_channel kept the removed + // channel in last_written on persist failure; subsequent syncs wrote it + // back, permanently losing the removal intent until reboot. + let ch_keep = Uuid::new_v4(); + let ch_remove = Uuid::new_v4(); + + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + + // Write both channels to disk; derive path from the ledger so the + // filename (pubkey16 + relay hash) stays in sync automatically. + let ledger_path = ledger.path().unwrap().to_path_buf(); + let tmp_path = { + let mut s = ledger_path.as_os_str().to_os_string(); + s.push(".tmp"); + std::path::PathBuf::from(s) + }; + + let trigger_keep = crate::queue::RecoverableTrigger { + event_id: "keep-event".into(), + prompt_tag: "mention".into(), + admission_seq: 1, + enqueued_at_unix: 1000, + cap_exempt: false, + }; + let trigger_remove = crate::queue::RecoverableTrigger { + event_id: "remove-event".into(), + prompt_tag: "mention".into(), + admission_seq: 2, + enqueued_at_unix: 1001, + cap_exempt: false, + }; + ledger.sync(ch_keep, vec![trigger_keep.clone()]); + ledger.sync(ch_remove, vec![trigger_remove]); + assert!(ledger_path.exists(), "setup: both channels written"); + + // Block the .tmp path so the invalidation write fails. + std::fs::create_dir_all(&tmp_path).expect("create blocker dir"); + ledger.invalidate_channel(ch_remove); + // The persist failed — but last_written must already reflect the removal. + + // Unblock and sync a DIFFERENT channel — this must write the full map + // (dirty flag forces a write even though ch_keep's content is unchanged). + std::fs::remove_dir_all(&tmp_path).expect("remove blocker"); + ledger.sync(ch_keep, vec![trigger_keep.clone()]); + + // Reload: removed channel must be absent. + let (_, staged) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + assert!( + !staged.channels.contains_key(&ch_remove), + "after invalidation + failed persist + different-channel sync, removed channel must be absent on reload (got: {:?})", + staged.channels, + ); + assert!( + staged.channels.contains_key(&ch_keep), + "kept channel must still be present" + ); + } + + #[test] + fn test_membership_removal_while_in_flight_does_not_resurrect_on_reload() { + // Regression guard (Wes, PR review): the membership-removal path runs + // `drain_channel` → `ledger.invalidate_channel` → `sync_dirty`. + // `drain_channel` deliberately preserves `in_flight_channels` / + // `in_flight_deadlines` so a live task can never wedge the channel, + // but `recoverable_triggers` also reads `in_flight_batch_triggers`. + // If the removal transition leaves that recovery mirror populated, the + // trailing `sync_dirty` re-persists the very trigger + // `invalidate_channel` just purged — so a crash before the prompt + // completes, followed by a re-add, resurrects discarded work. + // + // Field-level proof that the liveness tables survive the mirror purge + // lives in `queue.rs`'s + // `test_drain_channel_purges_recovery_mirror_but_keeps_liveness`. + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut queue = EventQueue::new(DedupMode::Queue); + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event = make_channel_event(&keys, ch, "in-flight turn"); + + // Queue an event and dispatch it, so the channel is in-flight and its + // trigger is mirrored in `in_flight_batch_triggers`. + queue.push(QueuedEvent::new( + ch, + event.clone(), + Instant::now(), + "test".into(), + )); + let batch = queue.flush_next().expect("event must dispatch"); + assert_eq!(batch.events.len(), 1); + sync_dirty(&mut queue, &mut ledger); + assert!( + Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0) + .1 + .channels + .contains_key(&ch), + "setup: in-flight turn must be durable before removal" + ); + + // Membership removal, in the exact order lib.rs performs it. + queue.drain_channel(ch); + ledger.invalidate_channel(ch); + sync_dirty(&mut queue, &mut ledger); + + let (_, staged) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + assert!( + !staged.channels.contains_key(&ch), + "removed channel must not survive on disk — a re-add would resurrect \ + discarded work (got: {:?})", + staged.channels, + ); + + // The wedge-prevention invariant the purge must not disturb: the + // in-flight task is still tracked, so it can complete or expire + // normally rather than blocking the channel forever. + assert!( + queue.is_channel_in_flight(ch), + "in_flight_channels must be preserved across removal" + ); + } + + #[test] + fn test_completion_after_membership_removal_leaves_ledger_empty() { + // Companion to the test above, covering the non-crash path: the + // in-flight prompt that survived removal eventually completes. + // `complete_batch` removes the (already-purged) trigger entry and + // syncs — the ledger must stay empty rather than being rewritten + // from the completing batch. + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut queue = EventQueue::new(DedupMode::Queue); + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event = make_channel_event(&keys, ch, "in-flight turn"); + + queue.push(QueuedEvent::new( + ch, + event.clone(), + Instant::now(), + "test".into(), + )); + let batch = queue.flush_next().expect("event must dispatch"); + queue.drain_channel(ch); + ledger.invalidate_channel(ch); + sync_dirty(&mut queue, &mut ledger); + + // Prompt returns after the agent lost access. + queue.complete_batch(ch, Some(batch), crate::queue::BatchDisposition::Success); + sync_dirty(&mut queue, &mut ledger); + + let (_, staged) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + assert!( + staged.channels.is_empty(), + "completion after removal must leave no durable record (got: {:?})", + staged.channels, + ); + assert!( + !queue.is_channel_in_flight(ch), + "completion must release the channel" + ); + } + + // ── Persisted prompt_tag through from_recovered ────────────────────── + + #[tokio::test] + async fn test_unresolved_resolution_uses_persisted_prompt_tag_not_live_tag() { + // When a live event resolves an unresolved ledger record, + // the recovered QueuedEvent must use the *persisted* prompt_tag from + // the LedgerRecord, not the live relay delivery's tag. + // + // Red-on-old: admit_live_event passed the live `prompt_tag` argument + // to from_recovered — a live delivery with a different tag would + // silently override the persisted one. + let keys = Keys::generate(); + let ch = Uuid::new_v4(); + let event_a = make_channel_event(&keys, ch, "msg"); + + let record_a = LedgerRecord { + event_id: event_a.id.to_hex(), + prompt_tag: "original".into(), + admission_seq: 1, + enqueued_at_unix: 1001, + cap_exempt: false, + }; + + let mut queue = EventQueue::new(DedupMode::Queue); + let dir = tempfile::tempdir().unwrap(); + let (mut ledger, _) = Ledger::load(dir.path(), "test_pubkey", "ws://localhost:3000", 0); + let mut suppression: HashSet = HashSet::new(); + + ledger.add_unresolved(ch, record_a); + + // Live delivery arrives with a different tag. + let (accepted, skip_steer) = admit_live_event( + &mut queue, + &mut ledger, + &mut suppression, + ch, + event_a.clone(), + "new".into(), // live delivery tag — must NOT win + ); + + assert!(accepted, "resolving event must be admitted"); + assert!(skip_steer, "resolving event skips native steer"); + + let triggers = queue.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1); + assert_eq!( + triggers[0].prompt_tag, "original", + "recovered trigger must retain the persisted prompt_tag, not the live-delivery tag" + ); + } +} diff --git a/crates/buzz-acp/src/pool.rs b/crates/buzz-acp/src/pool.rs index 158477c0af..c341dc9b34 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -696,6 +696,20 @@ impl AgentPool { (&mut self.result_rx, &mut self.join_set) } + /// Three-way split-borrow variant of [`Self::rx_and_join_set`] that also + /// exposes `task_map`. Used by the shutdown drain, which needs to look + /// up a panicking task's `TaskMeta` inside the same `select!` that's + /// already polling `result_rx`/`join_set`. + pub fn rx_join_set_and_task_map( + &mut self, + ) -> ( + &mut mpsc::UnboundedReceiver, + &mut JoinSet<()>, + &mut HashMap, + ) { + (&mut self.result_rx, &mut self.join_set, &mut self.task_map) + } + /// Non-blocking drain of the result channel. Used during shutdown to /// collect agents that completed while join_set was being drained. pub fn result_rx_try_recv(&mut self) -> Result { @@ -5000,11 +5014,11 @@ mod tests { let author_hex = event.pubkey.to_hex(); let batch = FlushBatch { channel_id: Uuid::new_v4(), - events: vec![crate::queue::BatchEvent { + events: vec![crate::queue::BatchEvent::for_test( event, - prompt_tag: "@mention".into(), - received_at: std::time::Instant::now(), - }], + "@mention".into(), + std::time::Instant::now(), + )], cancelled_events: vec![], cancel_reason: None, }; @@ -5326,11 +5340,11 @@ mod tests { .unwrap(); FlushBatch { channel_id, - events: vec![crate::queue::BatchEvent { + events: vec![crate::queue::BatchEvent::for_test( event, - prompt_tag: "test".into(), - received_at: std::time::Instant::now(), - }], + "test".into(), + std::time::Instant::now(), + )], cancelled_events: vec![], cancel_reason: None, } diff --git a/crates/buzz-acp/src/queue.rs b/crates/buzz-acp/src/queue.rs index 029bf86dbf..dc66edfe23 100644 --- a/crates/buzz-acp/src/queue.rs +++ b/crates/buzz-acp/src/queue.rs @@ -14,7 +14,7 @@ //! - **Queue** — all events accumulate; batched on the next flush cycle. use nostr::{Event, ToBech32}; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; use std::time::{Duration, Instant}; use uuid::Uuid; @@ -49,6 +49,25 @@ pub struct QueuedEvent { pub received_at: Instant, /// Tag identifying which rule (or mode) matched this event. pub prompt_tag: String, + /// Per-process monotonic admission order, stamped once at initial live + /// admission (or restored verbatim from the ledger on recovery) and + /// propagated unchanged through every transition. Never reassigned — + /// `recoverable_triggers()` sorts by this, not by `enqueued_at_unix` + /// (which is only second-resolution and routinely ties). + pub admission_seq: u64, + /// Wall-clock admission time (SystemTime, captured once at initial live + /// admission), for TTL filtering only. Ordering belongs to + /// `admission_seq`. + pub enqueued_at_unix: u64, + /// True for events re-admitted after a restart (boot import or + /// unresolved-record resolution). Prompt provenance only — never + /// consulted by cap enforcement. Default `false`. + pub restart_recovery: bool, + /// True when this event's cap class is exempt from + /// `MAX_PENDING_PER_CHANNEL` accounting/eviction. Persisted per record + /// in the ledger, assigned by the boot-time promotion rule — never set + /// on the live-admission path (`push()` always forces `false`). + pub cap_exempt: bool, } /// A single event inside a [`FlushBatch`]. @@ -57,6 +76,150 @@ pub struct BatchEvent { pub event: Event, pub prompt_tag: String, pub received_at: Instant, + /// See [`QueuedEvent::admission_seq`]; propagated unchanged. + pub admission_seq: u64, + /// See [`QueuedEvent::enqueued_at_unix`]; propagated unchanged. + pub enqueued_at_unix: u64, + /// See [`QueuedEvent::restart_recovery`]; propagated unchanged. + pub restart_recovery: bool, + /// See [`QueuedEvent::cap_exempt`]; propagated unchanged. + pub cap_exempt: bool, +} + +impl QueuedEvent { + /// Build a `QueuedEvent` for live admission via [`EventQueue::push`]. + /// `push()` overwrites `admission_seq`/`enqueued_at_unix` with real + /// values and forces `restart_recovery`/`cap_exempt` to `false` — the + /// placeholders here are never observed. Recovered events go through + /// `import_recovered`/`admit_recovered`, never this constructor. + pub fn new(channel_id: Uuid, event: Event, received_at: Instant, prompt_tag: String) -> Self { + QueuedEvent { + channel_id, + event, + received_at, + prompt_tag, + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, + } + } + + /// Test-only alias of [`Self::new`] — kept as a separate name so test + /// call sites read as "I only care about the four original fields" + /// without implying anything about live-admission stamping semantics. + #[cfg(test)] + pub(crate) fn for_test( + channel_id: Uuid, + event: Event, + received_at: Instant, + prompt_tag: String, + ) -> Self { + Self::new(channel_id, event, received_at, prompt_tag) + } + + /// Build a `QueuedEvent` for boot recovery (`EventQueue::import_recovered`) + /// or unresolved-record resolution (`EventQueue::admit_recovered`), + /// restoring exactly the fields that must survive a restart — + /// `admission_seq`/`enqueued_at_unix`/`cap_exempt` verbatim from the + /// ledger record, `restart_recovery: true` since a recovered event is + /// never a live admission. `received_at` is stamped fresh (in-process + /// fairness only; never persisted). + pub fn from_recovered( + channel_id: Uuid, + event: Event, + prompt_tag: String, + admission_seq: u64, + enqueued_at_unix: u64, + cap_exempt: bool, + ) -> Self { + QueuedEvent { + channel_id, + event, + received_at: Instant::now(), + prompt_tag, + admission_seq, + enqueued_at_unix, + restart_recovery: true, + cap_exempt, + } + } + + fn into_batch_event(self) -> BatchEvent { + BatchEvent { + event: self.event, + prompt_tag: self.prompt_tag, + received_at: self.received_at, + admission_seq: self.admission_seq, + enqueued_at_unix: self.enqueued_at_unix, + restart_recovery: self.restart_recovery, + cap_exempt: self.cap_exempt, + } + } +} + +impl BatchEvent { + /// See [`QueuedEvent::for_test`] — same rationale, for `BatchEvent` + /// literals built directly in tests (most `FlushBatch` fixtures) in + /// this module and elsewhere in the crate. + #[cfg(test)] + pub(crate) fn for_test(event: Event, prompt_tag: String, received_at: Instant) -> Self { + BatchEvent { + event, + prompt_tag, + received_at, + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, + } + } + + fn into_queued_event(self, channel_id: Uuid) -> QueuedEvent { + QueuedEvent { + channel_id, + event: self.event, + prompt_tag: self.prompt_tag, + received_at: self.received_at, + admission_seq: self.admission_seq, + enqueued_at_unix: self.enqueued_at_unix, + restart_recovery: self.restart_recovery, + cap_exempt: self.cap_exempt, + } + } +} + +/// The minimal durable identity of a queued/in-flight/cancelled/withheld +/// event the queue still owes a turn for — what the resume ledger persists. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RecoverableTrigger { + pub event_id: String, + pub prompt_tag: String, + pub admission_seq: u64, + pub enqueued_at_unix: u64, + pub cap_exempt: bool, +} + +impl RecoverableTrigger { + fn from_queued(qe: &QueuedEvent) -> Self { + RecoverableTrigger { + event_id: qe.event.id.to_hex(), + prompt_tag: qe.prompt_tag.clone(), + admission_seq: qe.admission_seq, + enqueued_at_unix: qe.enqueued_at_unix, + cap_exempt: qe.cap_exempt, + } + } + + fn from_batch(be: &BatchEvent) -> Self { + RecoverableTrigger { + event_id: be.event.id.to_hex(), + prompt_tag: be.prompt_tag.clone(), + admission_seq: be.admission_seq, + enqueued_at_unix: be.enqueued_at_unix, + cap_exempt: be.cap_exempt, + } + } } /// Why a batch's prior turn was cancelled — controls how `format_prompt` @@ -72,6 +235,43 @@ pub enum CancelReason { Steer, } +/// Which end of a channel's queue `enforce_cap` evicts from when the +/// counted-class count exceeds [`MAX_PENDING_PER_CHANNEL`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum VictimEnd { + /// `push()`: evict the oldest (front) counted event to make room for a + /// fresh arrival — today's live-admission behavior, unchanged. + Front, + /// The four restore paths (`requeue`, `requeue_preserve_timestamps`, + /// `release_native_steer`, withheld-steer deadline recovery): evict the + /// newest (back) counted event — today's restore-path behavior, + /// unchanged. + Back, +} + +/// How a completed batch's triggers should be disposed of. Applied +/// atomically by [`EventQueue::complete_batch`] — requeue/cancel semantics +/// and in-flight completion happen as one transition, so no public +/// operation ever exposes an intermediate (duplicated or half-restored) +/// recoverable state. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BatchDisposition { + /// Turn completed successfully; drop the batch. + Success, + /// Requeue with exponential backoff; dead-letter on `MAX_RETRIES` + /// exhaustion (today's `requeue()` semantics). + Retry, + /// Requeue as a cancelled batch for merged re-prompt framing (today's + /// `requeue_as_cancelled()` semantics). + Cancelled(CancelReason), + /// Requeue preserving original timestamps, no backoff (today's + /// `requeue_preserve_timestamps()` semantics — pool exhausted). + PreserveTimestamps, + /// Channel removed mid-flight, hard-cap timeout, or a Drop-mode panic: + /// discard the batch without requeuing. + Dropped, +} + /// A batch of events to prompt the agent with. #[derive(Debug, Clone)] pub struct FlushBatch { @@ -89,6 +289,18 @@ pub struct FlushBatch { pub cancel_reason: Option, } +impl FlushBatch { + /// Whether any event in this batch — fresh or merged-cancelled — is a + /// restart-recovery replay. Gates the batch-level recovery header in + /// `format_prompt`: a mixed batch of recovered + live events still needs + /// per-event markers so the agent doesn't apply the "may already be + /// done" caveat to genuinely new work. + pub fn has_recovered_events(&self) -> bool { + self.events.iter().any(|be| be.restart_recovery) + || self.cancelled_events.iter().any(|be| be.restart_recovery) + } +} + /// Per-channel event queue with per-channel in-flight enforcement. /// /// # State Machine @@ -168,6 +380,47 @@ pub struct EventQueue { /// Must be strictly greater than `max_turn_duration` so a turn running to /// the hard cap returns via `mark_complete` before the backstop fires. in_flight_deadline: Duration, + /// Per-process monotonic counter for `QueuedEvent::admission_seq` / + /// `BatchEvent::admission_seq`. Stamped once per live admission in + /// `push()`, then propagated unchanged through every transition. On + /// boot, resumed past the max persisted seq via + /// [`Self::set_next_admission_seq`] so recovered work always sorts + /// before fresh post-restart admissions. + next_admission_seq: u64, + /// Recoverable-trigger mirror of `in_flight_batch_sizes` — the + /// `RecoverableTrigger`s for each in-flight batch (covering `events` + /// plus any merged `cancelled_events`), so `recoverable_triggers()` can + /// include in-flight work without re-deriving it from a live batch the + /// queue no longer holds. Populated everywhere a channel becomes + /// in-flight. Removed at batch completion (inside `complete_batch`), at + /// both deadline-expiry blocks, and — uniquely — by `drain_channel`, which + /// purges the mirror alone so a membership removal cannot re-persist the + /// discarded turn while `in_flight_batch_sizes` keeps reporting an accurate + /// orphan count if that turn later expires. + in_flight_batch_triggers: HashMap>, + /// Channels mutated by a public operation since the last + /// [`Self::take_dirty_channels`] call. Every public `&mut self` mutator + /// inserts every channel id it touches — including channels auto- + /// expired as a side effect of an operation about a *different* + /// channel (`flush_next` / `has_flushable_work`'s expiry loops) — so + /// `lib.rs` never needs a hand-maintained list of mutation seams to + /// sync after. + dirty_channels: HashSet, + /// Per-channel unresolved-record ordering barrier. While a channel has + /// an entry here, `flush_next`/`has_flushable_work` refuse to flush + /// any of its queued events whose `admission_seq` exceeds the + /// barrier's lowest held seq. Cleared by resolution (`admit_recovered`), + /// invalidation (`drain_channel`), or deadline expiry + /// (`expire_due_unresolved_barriers`). + unresolved_barriers: HashMap, +} + +/// A channel's active unresolved-ordering barrier: the set of admission +/// seqs still owed a boot-fetch resolution, and the shared deadline (from +/// boot commit) past which the hold is lifted regardless. +struct UnresolvedBarrier { + seqs: BTreeSet, + deadline: Instant, } impl EventQueue { @@ -189,6 +442,10 @@ impl EventQueue { cancel_reasons: HashMap::new(), withheld_native_steer: HashMap::new(), in_flight_deadline: Duration::from_secs(DEFAULT_IN_FLIGHT_DEADLINE_SECS), + next_admission_seq: 0, + in_flight_batch_triggers: HashMap::new(), + dirty_channels: HashSet::new(), + unresolved_barriers: HashMap::new(), } } @@ -221,13 +478,269 @@ impl EventQueue { } } + /// Resume `admission_seq` allocation past `seq` (boot-time restore). + /// + /// Called once, before any post-boot `push()`, with the max + /// `admission_seq` observed across every persisted record (imported and + /// unresolved). Ensures fresh live admissions always sort after + /// recovered work and a later-resolving unresolved record keeps a + /// collision-free seq. + pub fn set_next_admission_seq(&mut self, seq: u64) { + if seq >= self.next_admission_seq { + self.next_admission_seq = seq + 1; + } + } + + /// Every event this channel still owes a turn for, in total admission + /// order (ascending `admission_seq`): ordinary queued events, the + /// in-flight batch (if any), cancelled-pending-redispatch events, and + /// withheld-native-steer events. Empty vec = the channel owes nothing. + /// + /// This is the full mirror `lib.rs` persists to the ledger after every + /// dirtying operation — see [`Self::take_dirty_channels`]. + pub fn recoverable_triggers(&self, channel_id: Uuid) -> Vec { + let mut triggers: Vec = Vec::new(); + if let Some(q) = self.queues.get(&channel_id) { + triggers.extend(q.iter().map(RecoverableTrigger::from_queued)); + } + if let Some(in_flight) = self.in_flight_batch_triggers.get(&channel_id) { + triggers.extend(in_flight.iter().cloned()); + } + if let Some(cancelled) = self.cancelled_batches.get(&channel_id) { + triggers.extend(cancelled.iter().map(RecoverableTrigger::from_batch)); + } + if let Some(withheld) = self.withheld_native_steer.get(&channel_id) { + triggers.extend(withheld.iter().map(RecoverableTrigger::from_queued)); + } + triggers.sort_by_key(|t| t.admission_seq); + triggers + } + + /// Drain and return every channel id touched by a public mutator since + /// the last call. `lib.rs` calls this after every public `&mut queue` + /// operation and ledger-syncs each returned channel — the one rule that + /// replaces a hand-maintained seam list (see module docs). + pub fn take_dirty_channels(&mut self) -> Vec { + std::mem::take(&mut self.dirty_channels) + .into_iter() + .collect() + } + + /// Lowest currently-unresolved `admission_seq` for `channel_id`, or + /// `None` if the channel has no active barrier. Queue invariant: every + /// per-channel table (`queues`, `cancelled_batches`) is always ascending + /// by `admission_seq` (established by construction at every mutator — + /// live `push` appends the process-max seq, all restore paths + /// front-load strictly lower seqs than what remains, and + /// `import_recovered`/`admit_recovered` insert in seq order). Events at + /// or below this floor precede every remaining hole and flush + /// normally; events above it are held. + fn barrier_floor(&self, channel_id: Uuid) -> Option { + self.unresolved_barriers + .get(&channel_id) + .and_then(|b| b.seqs.iter().next().copied()) + } + + /// Splits an ascending-by-`admission_seq` vector into (flushable, held) + /// given an optional barrier floor: `Some(k)` keeps only entries with + /// `admission_seq <= k` in the flushable half; `None` keeps everything + /// flushable. Relative order within each half is preserved regardless + /// of the input's admission_seq order. Shared by `flush_next`'s + /// cancelled-batch fallback and cancelled-events merge so both respect + /// the same per-event rule the main queue drain does. + fn split_at_barrier_floor( + entries: Vec, + floor: Option, + ) -> (Vec, Vec) { + match floor { + None => (entries, Vec::new()), + Some(floor) => { + let mut flushable = Vec::with_capacity(entries.len()); + let mut held = Vec::new(); + for be in entries { + if be.admission_seq <= floor { + flushable.push(be); + } else { + held.push(be); + } + } + (flushable, held) + } + } + } + + /// Register `channel_id`'s unresolved ordering barrier — called once + /// per channel at boot commit for every channel with unfetched + /// boot-recovery triggers. `seqs` is the channel's set of unresolved + /// `admission_seq`s; `deadline` is the shared recovery deadline + /// (measured from boot commit) at which the barrier lifts regardless of + /// resolution. A channel with no unresolved seqs gets no barrier. + pub fn set_unresolved_barrier( + &mut self, + channel_id: Uuid, + seqs: BTreeSet, + deadline: Instant, + ) { + if seqs.is_empty() { + return; + } + self.unresolved_barriers + .insert(channel_id, UnresolvedBarrier { seqs, deadline }); + } + + /// Returns `true` if `channel_id` has an active (non-expired) unresolved + /// barrier. Used by `admit_live_event` to suppress native steer while the + /// barrier is armed — a fresh live event must queue behind the hole rather + /// than bypassing it through the steer side-door. + pub fn has_active_unresolved_barrier(&self, channel_id: Uuid) -> bool { + self.unresolved_barriers.contains_key(&channel_id) + } + + /// Earliest active barrier deadline across all channels, or `None` when + /// no barrier is armed. The main `select!`'s `tokio::time::Sleep` arm + /// is reset from this whenever the barrier set changes (rev 6.1). + pub fn next_unresolved_barrier_deadline(&self) -> Option { + self.unresolved_barriers.values().map(|b| b.deadline).min() + } + + /// Expire every barrier at or past `now`, lifting the hold on its + /// channel's suffix. Called by the dedicated `select!` timer arm on + /// fire, and opportunistically at the top of `flush_next`/ + /// `has_flushable_work` as a cheap check on already-woken paths — the + /// timer arm is what makes the deadline an actual bound on a quiet + /// harness (rev 6.1). Returns the expired channel ids (already + /// inserted into `dirty_channels`) so the caller can ledger-sync them. + pub fn expire_due_unresolved_barriers(&mut self, now: Instant) -> Vec { + let due: Vec = self + .unresolved_barriers + .iter() + .filter(|(_, b)| now >= b.deadline) + .map(|(id, _)| *id) + .collect(); + for id in &due { + self.unresolved_barriers.remove(id); + self.dirty_channels.insert(*id); + } + due + } + + /// Boot-only bulk restore: re-admits a channel's complete durable + /// trigger set, in `admission_seq` order, bypassing cap eviction (the + /// durable set can legitimately exceed [`MAX_PENDING_PER_CHANNEL`] — + /// see module docs on cap enforcement). Each event's `cap_exempt` is + /// taken verbatim from the caller — this function does not compute or + /// apply the whole-snapshot promotion rule itself. The promotion + /// decision requires seeing every gated record for a channel (fetched + /// *and* unresolved), which only the caller (`boot_recover`) has in + /// scope; `events` here is just the fetched subset. Settling the class + /// here from a partial subset would re-derive the wrong answer (e.g. an + /// unfetched exempt record leaving a fetched non-exempt record as the + /// only entry in `events`, spuriously promoting it). A no-op for an + /// empty `events`. + pub fn import_recovered(&mut self, channel_id: Uuid, mut events: Vec) { + if events.is_empty() { + return; + } + events.sort_by_key(|e| e.admission_seq); + let queue = self.queues.entry(channel_id).or_default(); + for mut event in events { + event.channel_id = channel_id; + queue.push_back(event); + } + self.dirty_channels.insert(channel_id); + } + + /// Recovery-specific single-event admission for resolving an unresolved + /// boot-fetch record: bypasses `DedupMode::Drop`'s in-flight rejection + /// and cap accounting, and inserts `event` at its + /// `admission_seq` position among the channel's queued events (not the + /// tail) so the runtime queue matches the ledger's total order. Also + /// releases the resolved seq from the channel's unresolved barrier, if + /// any (release path 1 of 3 — see `set_unresolved_barrier`). + /// + /// Caller supplies `event` with its original `admission_seq`/ + /// `enqueued_at_unix`/`restart_recovery`/`cap_exempt` restored from the + /// unresolved ledger record — this does not stamp or overwrite any of + /// those fields (unlike `push`). + pub fn admit_recovered(&mut self, channel_id: Uuid, mut event: QueuedEvent) { + event.channel_id = channel_id; + let seq = event.admission_seq; + let queue = self.queues.entry(channel_id).or_default(); + let pos = queue + .iter() + .position(|qe| qe.admission_seq > seq) + .unwrap_or(queue.len()); + queue.insert(pos, event); + if let Some(barrier) = self.unresolved_barriers.get_mut(&channel_id) { + barrier.seqs.remove(&seq); + if barrier.seqs.is_empty() { + self.unresolved_barriers.remove(&channel_id); + } + } + self.dirty_channels.insert(channel_id); + } + + /// Evicts counted-class (`cap_exempt == false`) events from `victim_end` + /// while their count exceeds [`MAX_PENDING_PER_CHANNEL`]. Exempt events + /// are invisible to this accounting — they leave the queue only via + /// dispatch or dead-letter, never eviction. Shared by all five + /// cap-enforcement call sites (`push`, `requeue`, + /// `requeue_preserve_timestamps`, `release_native_steer`, withheld-steer + /// deadline recovery) so the policy lives in exactly one place. + fn enforce_cap(&mut self, channel_id: Uuid, victim_end: VictimEnd) { + let Some(queue) = self.queues.get_mut(&channel_id) else { + return; + }; + loop { + let counted = queue.iter().filter(|qe| !qe.cap_exempt).count(); + if counted <= MAX_PENDING_PER_CHANNEL { + break; + } + let victim_pos = match victim_end { + VictimEnd::Front => queue.iter().position(|qe| !qe.cap_exempt), + VictimEnd::Back => queue.iter().rposition(|qe| !qe.cap_exempt), + }; + let Some(pos) = victim_pos else { + // Unreachable given `counted > 0` above, but never loop + // forever on a bookkeeping mismatch. + break; + }; + queue.remove(pos); + let label = match victim_end { + VictimEnd::Front => "oldest", + VictimEnd::Back => "newest", + }; + tracing::warn!( + channel_id = %channel_id, + limit = MAX_PENDING_PER_CHANNEL, + "queue depth cap reached — dropped {label} counted event" + ); + } + if queue.is_empty() { + self.queues.remove(&channel_id); + } + } + /// Push an event into the queue for its channel. /// /// In [`DedupMode::Drop`], events for any currently in-flight channel are /// silently discarded (debug-logged). /// + /// Stamps `admission_seq` (this process's next monotonic value) and + /// `enqueued_at_unix` (wall-clock now), and forces `restart_recovery` / + /// `cap_exempt` to `false` — `push()` is the live-admission path only; + /// any caller-supplied values for these four fields are overwritten. + /// Recovered work is admitted via `import_recovered`/`admit_recovered`, + /// which preserve the ledger's original values instead. + /// /// Returns `true` if the event was accepted, `false` if dropped. - pub fn push(&mut self, event: QueuedEvent) -> bool { + pub fn push(&mut self, mut event: QueuedEvent) -> bool { + event.admission_seq = self.next_admission_seq; + self.next_admission_seq += 1; + event.enqueued_at_unix = crate::relay::unix_now_secs(); + event.restart_recovery = false; + event.cap_exempt = false; + if matches!(self.dedup_mode, DedupMode::Drop) && self.in_flight_channels.contains(&event.channel_id) { @@ -237,17 +750,11 @@ impl EventQueue { ); return false; } - let queue = self.queues.entry(event.channel_id).or_default(); - // Enforce per-channel depth cap: drop oldest to make room. - if queue.len() >= MAX_PENDING_PER_CHANNEL { - queue.pop_front(); - tracing::warn!( - channel_id = %event.channel_id, - limit = MAX_PENDING_PER_CHANNEL, - "queue depth cap reached — dropped oldest event" - ); - } + let channel_id = event.channel_id; + let queue = self.queues.entry(channel_id).or_default(); queue.push_back(event); + self.enforce_cap(channel_id, VictimEnd::Front); + self.dirty_channels.insert(channel_id); true } @@ -260,6 +767,12 @@ impl EventQueue { pub fn flush_next(&mut self) -> Option { let now = Instant::now(); + // Opportunistic barrier expiry — cheap check on this already-woken + // path. The dedicated select! timer arm is what makes the deadline + // an actual bound on a quiet harness (rev 6.1); this just avoids + // leaving an expired barrier armed until the next timer fire. + self.expire_due_unresolved_barriers(now); + // Auto-expire any stuck in-flight entries that missed mark_complete. let expired: Vec = self .in_flight_deadlines @@ -269,6 +782,7 @@ impl EventQueue { .collect(); for id in expired { let lost_events = self.in_flight_batch_sizes.remove(&id).unwrap_or(0); + self.in_flight_batch_triggers.remove(&id); tracing::error!( channel_id = %id, lost_events, @@ -284,10 +798,12 @@ impl EventQueue { // now-hung prompt — nothing to recover), these events were never // delivered to the agent. self.recover_withheld_for_expired_channel(id); + self.dirty_channels.insert(id); } // Find the channel whose head event has the oldest received_at, - // excluding in-flight channels and throttled channels. + // excluding in-flight channels, throttled channels, and channels + // whose head event is held behind an active unresolved barrier. let channel_id = self .queues .iter() @@ -295,31 +811,60 @@ impl EventQueue { !q.is_empty() && !self.in_flight_channels.contains(id) && self.retry_after.get(id).is_none_or(|&t| t <= now) + && q.front().unwrap().admission_seq + <= self.barrier_floor(**id).unwrap_or(u64::MAX) }) .min_by_key(|(_, q)| q.front().unwrap().received_at) .map(|(id, _)| *id); // Fallback: if no queued events are ready but a channel has cancelled // events waiting (e.g., explicit !cancel with no new @mention), flush - // those as a regular batch (re-dispatch unchanged). + // those as a regular batch (re-dispatch unchanged). Held-behind-the- + // barrier cancelled events are split out and kept for a later cycle. let channel_id = match channel_id { Some(id) => id, None => { let cancelled_id = self .cancelled_batches - .keys() - .find(|id| !self.in_flight_channels.contains(id)) - .copied(); + .iter() + .find(|(id, batch)| { + !self.in_flight_channels.contains(*id) + && batch.iter().any(|be| { + be.admission_seq <= self.barrier_floor(**id).unwrap_or(u64::MAX) + }) + }) + .map(|(id, _)| *id); match cancelled_id { Some(id) => { // Move cancelled events into the regular events slot. // No new events to merge — re-dispatch the original batch. - let cancelled = self.cancelled_batches.remove(&id).unwrap_or_default(); - let cancel_reason = self.cancel_reasons.remove(&id); + let full = self.cancelled_batches.remove(&id).unwrap_or_default(); + let stored_reason = self.cancel_reasons.remove(&id); + let floor = self.barrier_floor(id); + let (cancelled, held) = Self::split_at_barrier_floor(full, floor); + if !held.is_empty() { + self.cancelled_batches.insert(id, held); + if let Some(r) = stored_reason { + self.cancel_reasons.insert(id, r); + } + } + let cancel_reason = if cancelled.is_empty() { + None + } else { + stored_reason + }; self.in_flight_channels.insert(id); self.in_flight_deadlines .insert(id, now + self.in_flight_deadline); self.in_flight_batch_sizes.insert(id, cancelled.len()); + self.in_flight_batch_triggers.insert( + id, + cancelled + .iter() + .map(RecoverableTrigger::from_batch) + .collect(), + ); + self.dirty_channels.insert(id); return Some(FlushBatch { channel_id: id, events: cancelled, @@ -332,16 +877,19 @@ impl EventQueue { } }; - // Drain up to MAX_BATCH_EVENTS; leave any remainder in the queue. + // Drain up to MAX_BATCH_EVENTS, but never past the channel's + // unresolved barrier floor (if any) — events above it are left in + // the queue for a later cycle, after resolution/invalidation/expiry. + let floor = self.barrier_floor(channel_id).unwrap_or(u64::MAX); let queue = self.queues.entry(channel_id).or_default(); - let drain_count = MAX_BATCH_EVENTS.min(queue.len()); + let flushable_len = queue + .iter() + .take_while(|qe| qe.admission_seq <= floor) + .count(); + let drain_count = MAX_BATCH_EVENTS.min(flushable_len); let mut events: Vec = queue .drain(..drain_count) - .map(|qe| BatchEvent { - event: qe.event, - prompt_tag: qe.prompt_tag, - received_at: qe.received_at, - }) + .map(QueuedEvent::into_batch_event) .collect(); // Relay replay delivers stored events newest-first (`ORDER BY // created_at DESC`), but batch consumers — `format_prompt` scope and @@ -359,18 +907,37 @@ impl EventQueue { .insert(channel_id, now + self.in_flight_deadline); self.in_flight_batch_sizes.insert(channel_id, events.len()); - // Merge any cancelled events stored by requeue_as_cancelled(). - let cancelled_events = self + // Merge any cancelled events stored by requeue_as_cancelled(), + // splitting out any held behind the same barrier floor. + let cancelled_raw = self .cancelled_batches .remove(&channel_id) .unwrap_or_default(); + let stored_reason = self.cancel_reasons.remove(&channel_id); + let (cancelled_events, held_cancelled) = + Self::split_at_barrier_floor(cancelled_raw, Some(floor)); + if !held_cancelled.is_empty() { + self.cancelled_batches.insert(channel_id, held_cancelled); + if let Some(r) = stored_reason { + self.cancel_reasons.insert(channel_id, r); + } + } let cancel_reason = if cancelled_events.is_empty() { - self.cancel_reasons.remove(&channel_id); None } else { - self.cancel_reasons.remove(&channel_id) + stored_reason }; + self.in_flight_batch_triggers.insert( + channel_id, + events + .iter() + .map(RecoverableTrigger::from_batch) + .chain(cancelled_events.iter().map(RecoverableTrigger::from_batch)) + .collect(), + ); + self.dirty_channels.insert(channel_id); + Some(FlushBatch { channel_id, events, @@ -389,7 +956,10 @@ impl EventQueue { /// so the backoff sequence continues on the next attempt. /// /// Also cleans up any already-expired `retry_after` entry. - pub fn mark_complete(&mut self, channel_id: Uuid) { + /// + /// Private: only [`Self::complete_batch`] may call this, so no public + /// API can observe the requeue-then-complete intermediate state. + fn mark_complete(&mut self, channel_id: Uuid) { self.in_flight_channels.remove(&channel_id); self.in_flight_deadlines.remove(&channel_id); self.in_flight_batch_sizes.remove(&channel_id); @@ -426,7 +996,9 @@ impl EventQueue { /// /// Note: does NOT remove from `in_flight_channels` — caller must call /// `mark_complete` separately. - pub fn requeue(&mut self, batch: FlushBatch) -> Option { + /// + /// Private: only [`Self::complete_batch`] may call this. + fn requeue(&mut self, batch: FlushBatch) -> Option { let channel_id = batch.channel_id; let attempt = { let count = self.retry_counts.entry(channel_id).or_insert(0); @@ -475,24 +1047,9 @@ impl EventQueue { let queue = self.queues.entry(channel_id).or_default(); // Push to front in reverse order so original order is preserved. for be in batch.events.into_iter().rev() { - queue.push_front(QueuedEvent { - channel_id, - event: be.event, - prompt_tag: be.prompt_tag, - received_at: be.received_at, // preserve original timestamp (#46) - }); - } - // Enforce per-channel cap: trim oldest (back) events if requeue pushed - // the queue over the limit. Without this, repeated requeue+push cycles - // can grow the queue unboundedly. - while queue.len() > MAX_PENDING_PER_CHANNEL { - queue.pop_back(); - tracing::warn!( - channel_id = %channel_id, - limit = MAX_PENDING_PER_CHANNEL, - "requeue overflow — dropped oldest event to enforce cap" - ); + queue.push_front(be.into_queued_event(channel_id)); // preserve original timestamp (#46) } + self.enforce_cap(channel_id, VictimEnd::Back); self.retry_after.insert(channel_id, Instant::now() + delay); None } @@ -505,27 +1062,16 @@ impl EventQueue { /// /// Does NOT set `retry_after`. Does NOT remove from `in_flight_channels` — /// caller must call `mark_complete` separately. - pub fn requeue_preserve_timestamps(&mut self, batch: FlushBatch) { + /// + /// Private: only [`Self::complete_batch`] may call this. + fn requeue_preserve_timestamps(&mut self, batch: FlushBatch) { let channel_id = batch.channel_id; let queue = self.queues.entry(channel_id).or_default(); // Push to front in reverse order so original order is preserved. for be in batch.events.into_iter().rev() { - queue.push_front(QueuedEvent { - channel_id, - event: be.event, - prompt_tag: be.prompt_tag, - received_at: be.received_at, - }); - } - // Enforce per-channel cap: trim newest (back) events if over limit. - while queue.len() > MAX_PENDING_PER_CHANNEL { - queue.pop_back(); - tracing::warn!( - channel_id = %channel_id, - limit = MAX_PENDING_PER_CHANNEL, - "requeue_preserve overflow — dropped newest event to enforce cap" - ); + queue.push_front(be.into_queued_event(channel_id)); } + self.enforce_cap(channel_id, VictimEnd::Back); } /// Requeue a cancelled batch so its events appear as `cancelled_events` @@ -539,7 +1085,9 @@ impl EventQueue { /// Unlike `requeue_preserve_timestamps`, events are NOT pushed back into /// the generic queue — they are stored separately and merged by /// `flush_next()`. No retry throttle, no backoff. - pub fn requeue_as_cancelled(&mut self, batch: FlushBatch, reason: CancelReason) { + /// + /// Private: only [`Self::complete_batch`] may call this. + fn requeue_as_cancelled(&mut self, batch: FlushBatch, reason: CancelReason) { let entry = self.cancelled_batches.entry(batch.channel_id).or_default(); // Preserve any already-cancelled events from a prior cancel (double-cancel). entry.extend(batch.cancelled_events); @@ -547,6 +1095,105 @@ impl EventQueue { self.cancel_reasons.insert(batch.channel_id, reason); } + /// Apply a batch's completion disposition and mark its channel's prompt + /// complete as one atomic transition. + /// + /// `batch` is `None` for [`BatchDisposition::Success`] (nothing to + /// requeue). Every nonterminal disposition restores **both** payload + /// vectors — `events` per the disposition's own semantics, and any + /// `cancelled_events` back into `cancelled_batches` with the batch's + /// `cancel_reason` — exactly once, so a mixed batch never loses its + /// cancelled half to a `Retry` or `PreserveTimestamps` disposition. + /// + /// Returns any dead-lettered batch (`Retry` exhaustion) for the caller's + /// error log, as `requeue()` did before this replaced it. Always leaves + /// the queue in a consistent post-transition recoverable state — no + /// public operation can observe an intermediate one. + pub fn complete_batch( + &mut self, + channel_id: Uuid, + batch: Option, + disposition: BatchDisposition, + ) -> Option { + self.in_flight_batch_triggers.remove(&channel_id); + let dead_letter = match (batch, disposition) { + (None, _) => None, + (Some(batch), BatchDisposition::Success) => { + debug_assert_eq!(batch.channel_id, channel_id); + None + } + (Some(batch), BatchDisposition::Dropped) => { + debug_assert_eq!(batch.channel_id, channel_id); + None + } + (Some(batch), BatchDisposition::Retry) => { + debug_assert_eq!(batch.channel_id, channel_id); + let cancelled = batch.cancelled_events.clone(); + let cancel_reason = batch.cancel_reason; + let dead = self.requeue(FlushBatch { + cancelled_events: vec![], + ..batch + }); + match dead { + // Dead-lettered: restore the cancelled half onto the + // returned batch so the caller's error log (and any + // failure notice) names everything discarded. + Some(mut dead) => { + dead.cancelled_events = cancelled; + dead.cancel_reason = cancel_reason; + Some(dead) + } + // Requeued for another attempt: restore the cancelled + // half into cancelled_batches, preserving merged-prompt + // framing on the next flush. + None => { + if !cancelled.is_empty() { + self.requeue_as_cancelled( + FlushBatch { + channel_id, + events: vec![], + cancelled_events: cancelled, + cancel_reason: None, + }, + cancel_reason.unwrap_or(CancelReason::Steer), + ); + } + None + } + } + } + (Some(batch), BatchDisposition::PreserveTimestamps) => { + debug_assert_eq!(batch.channel_id, channel_id); + let cancelled = batch.cancelled_events.clone(); + let cancel_reason = batch.cancel_reason; + self.requeue_preserve_timestamps(FlushBatch { + cancelled_events: vec![], + ..batch + }); + if !cancelled.is_empty() { + self.requeue_as_cancelled( + FlushBatch { + channel_id, + events: vec![], + cancelled_events: cancelled, + cancel_reason: None, + }, + cancel_reason.unwrap_or(CancelReason::Steer), + ); + } + None + } + (Some(batch), BatchDisposition::Cancelled(reason)) => { + debug_assert_eq!(batch.channel_id, channel_id); + self.requeue_as_cancelled(batch, reason); + None + } + }; + self.mark_complete(channel_id); + self.dirty_channels.insert(channel_id); + dead_letter + } + /// Returns `true` if any channel has pending events that are not in-flight /// and not throttled by `retry_after`. /// @@ -556,6 +1203,9 @@ impl EventQueue { pub fn has_flushable_work(&mut self) -> bool { let now = Instant::now(); + // Opportunistic barrier expiry — see flush_next's rationale. + self.expire_due_unresolved_barriers(now); + // Auto-expire stuck in-flight entries (same logic as flush_next). let expired: Vec = self .in_flight_deadlines @@ -565,6 +1215,7 @@ impl EventQueue { .collect(); for id in expired { let lost_events = self.in_flight_batch_sizes.remove(&id).unwrap_or(0); + self.in_flight_batch_triggers.remove(&id); tracing::error!( channel_id = %id, lost_events, @@ -578,16 +1229,20 @@ impl EventQueue { // goose-native steer events for the expired channel so they are // not permanently orphaned in the side table. self.recover_withheld_for_expired_channel(id); + self.dirty_channels.insert(id); } self.queues.iter().any(|(id, q)| { !q.is_empty() && !self.in_flight_channels.contains(id) && self.retry_after.get(id).is_none_or(|&t| t <= now) - }) || self - .cancelled_batches - .keys() - .any(|id| !self.in_flight_channels.contains(id)) + && q.front().unwrap().admission_seq <= self.barrier_floor(*id).unwrap_or(u64::MAX) + }) || self.cancelled_batches.iter().any(|(id, batch)| { + !self.in_flight_channels.contains(id) + && batch + .iter() + .any(|be| be.admission_seq <= self.barrier_floor(*id).unwrap_or(u64::MAX)) + }) } /// Number of channels with pending events. @@ -614,9 +1269,10 @@ impl EventQueue { /// Drop all queued (non-in-flight) events for a channel. /// /// Used when the agent is removed from a channel — any pending events - /// for that channel are stale and should not be prompted. Does NOT - /// affect in-flight prompts (those will complete normally; the agent - /// may fail to act if it lost access, but that's handled by the relay). + /// for that channel are stale and should not be prompted. An in-flight + /// prompt still runs to completion (the agent may fail to act if it lost + /// access, but that's handled by the relay); only its durable recovery + /// mirror is dropped, so a crash mid-prompt cannot resurrect the turn. /// /// Also clears any `retry_after` throttle for the channel. /// @@ -633,11 +1289,26 @@ impl EventQueue { self.cancelled_batches.remove(&channel_id); self.cancel_reasons.remove(&channel_id); self.withheld_native_steer.remove(&channel_id); + // Invalidation (release path 2 of 3): a removed channel's unresolved + // barrier — and any unresolved ledger records it guards — must not + // survive to resurrect discarded work if the agent is re-added later. + self.unresolved_barriers.remove(&channel_id); + // Invalidation: drop the in-flight batch's *durable recovery mirror* so + // `recoverable_triggers` reports nothing for this channel and the + // caller's trailing ledger sync writes it empty in one shot — + // otherwise that sync re-persists the very trigger + // `Ledger::invalidate_channel` just purged, and a crash before the + // prompt completed would resurrect discarded work on a later re-add. + // `complete_batch` and the deadline-expiry paths `.remove()` this + // entry, so they tolerate it already being absent. + self.in_flight_batch_triggers.remove(&channel_id); + self.dirty_channels.insert(channel_id); // Preserve in_flight_channels AND in_flight_deadlines: the in-flight // task will eventually complete (calling mark_complete) or the deadline // will expire (auto-cleaning the channel). Removing deadlines without // removing in_flight_channels would disable auto-expiry and leave a - // wedged task permanently blocking the channel. + // wedged task permanently blocking the channel. Only the recovery + // mirror above is purged; liveness tracking is untouched. ids } @@ -687,6 +1358,7 @@ impl EventQueue { .entry(channel_id) .or_default() .push(qe); + self.dirty_channels.insert(channel_id); true } @@ -719,14 +1391,8 @@ impl EventQueue { // a flood of events arrived during the ack window. let queue = self.queues.entry(channel_id).or_default(); queue.push_front(qe); - while queue.len() > MAX_PENDING_PER_CHANNEL { - queue.pop_back(); - tracing::warn!( - channel_id = %channel_id, - limit = MAX_PENDING_PER_CHANNEL, - "release_native_steer overflow — dropped newest event to enforce cap" - ); - } + self.enforce_cap(channel_id, VictimEnd::Back); + self.dirty_channels.insert(channel_id); } /// Drop a specific event by id from both the side table and the main @@ -748,6 +1414,7 @@ impl EventQueue { self.queues.remove(&channel_id); } } + self.dirty_channels.insert(channel_id); } /// Bulk-release every withheld event for `channel_id` back to the queue @@ -772,14 +1439,7 @@ impl EventQueue { for qe in entries.into_iter().rev() { queue.push_front(qe); } - while queue.len() > MAX_PENDING_PER_CHANNEL { - queue.pop_back(); - tracing::warn!( - channel_id = %channel_id, - limit = MAX_PENDING_PER_CHANNEL, - "withheld-steer recovery overflow — dropped newest event to enforce cap" - ); - } + self.enforce_cap(channel_id, VictimEnd::Back); tracing::warn!( channel_id = %channel_id, recovered = n, @@ -962,6 +1622,13 @@ pub fn slash_command_for_batch(batch: &FlushBatch, known_names: &[&str]) -> Opti if batch.events.len() != 1 || !batch.cancelled_events.is_empty() { return None; } + // A recovered event must go through the wrapped-context path so the + // model sees the recovery header before any side effect fires — bare + // pass-through would re-execute a command that may have already run + // pre-restart. + if batch.events[0].restart_recovery { + return None; + } extract_slash_command(&batch.events[0].event.content, known_names) } @@ -1492,6 +2159,20 @@ pub fn format_prompt(batch: &FlushBatch, args: &FormatPromptArgs<'_>) -> Vec) -> Vec) -> Vec) -> Vec) -> Vec &'static str { + if be.restart_recovery { + " [restart recovery]" + } else { + "" + } +} + /// Prompt-framing strings for a merged (cancel + re-prompt) turn, selected by /// [`CancelReason`]. `Interrupt` frames the new events as superseding the prior /// work; `Steer` (the default mid-turn path) frames them as messages that @@ -1642,22 +2339,22 @@ mod tests { /// Build a QueuedEvent for the given channel. fn make_queued(channel_id: Uuid, content: &str) -> QueuedEvent { - QueuedEvent { + QueuedEvent::for_test( channel_id, - event: make_event(content), - received_at: Instant::now(), - prompt_tag: "test".into(), - } + make_event(content), + Instant::now(), + "test".into(), + ) } /// Build a QueuedEvent with a specific `received_at` offset from now. fn make_queued_at(channel_id: Uuid, content: &str, age: Duration) -> QueuedEvent { - QueuedEvent { + QueuedEvent::for_test( channel_id, - event: make_event(content), - received_at: Instant::now() - age, - prompt_tag: "test".into(), - } + make_event(content), + Instant::now() - age, + "test".into(), + ) } /// Build a QueuedEvent with an explicit Nostr `created_at` timestamp. @@ -1672,12 +2369,7 @@ mod tests { .tags([]) .sign_with_keys(&keys) .unwrap(); - QueuedEvent { - channel_id, - event, - received_at: Instant::now(), - prompt_tag: "test".into(), - } + QueuedEvent::for_test(channel_id, event, Instant::now(), "test".into()) } fn pending_count(q: &EventQueue) -> usize { @@ -1872,6 +2564,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -1892,6 +2588,144 @@ mod tests { assert!(!prompt.contains("--- Event 1 ---")); } + #[test] + fn test_format_prompt_recovered_single_event_gets_header_and_marker() { + let ch = Uuid::new_v4(); + let batch = FlushBatch { + channel_id: ch, + events: vec![BatchEvent { + event: make_event("original request"), + prompt_tag: "@mention".into(), + received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: true, + cap_exempt: false, + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n"); + + assert!( + prompt.contains("[Restart recovery]"), + "recovered batch must carry the scoped header: {prompt}" + ); + assert!( + prompt.contains("[Buzz event: @mention] [restart recovery]"), + "the single recovered event's header line must carry the marker: {prompt}" + ); + } + + #[test] + fn test_format_prompt_fresh_batch_has_no_recovery_header_or_marker() { + let ch = Uuid::new_v4(); + let batch = FlushBatch { + channel_id: ch, + events: vec![BatchEvent { + event: make_event("Hello @agent"), + prompt_tag: "@mention".into(), + received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, + }], + cancelled_events: vec![], + cancel_reason: None, + }; + let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n"); + + assert!(!prompt.contains("[Restart recovery]")); + assert!(!prompt.contains("[restart recovery]")); + } + + #[test] + fn test_format_prompt_mixed_batch_marks_only_recovered_event() { + // A recovered backlog event batched with a fresh live event: the + // header must appear (batch has at least one recovered event), but + // only the recovered event's header line carries the per-event + // marker — the live event must read as new, unambiguous work. + let ch = Uuid::new_v4(); + let batch = FlushBatch { + channel_id: ch, + events: vec![ + BatchEvent { + event: make_event("recovered work"), + prompt_tag: "@mention".into(), + received_at: Instant::now(), + admission_seq: 1, + enqueued_at_unix: 100, + restart_recovery: true, + cap_exempt: false, + }, + BatchEvent { + event: make_event("fresh work"), + prompt_tag: "@mention".into(), + received_at: Instant::now(), + admission_seq: 2, + enqueued_at_unix: 200, + restart_recovery: false, + cap_exempt: false, + }, + ], + cancelled_events: vec![], + cancel_reason: None, + }; + let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n"); + + assert!(prompt.contains("[Restart recovery]"), "{prompt}"); + assert!( + prompt.contains("--- Event 1 (@mention) [restart recovery] ---"), + "recovered event must be marked: {prompt}" + ); + assert!( + prompt.contains("--- Event 2 (@mention) ---"), + "fresh event must NOT be marked: {prompt}" + ); + } + + #[test] + fn test_format_prompt_recovered_marker_in_cancelled_section_only() { + // The cancelled (prior-work) half of a merge can independently carry + // recovery provenance even when the new event is fresh — the marker + // must appear in the cancelled section, not bleed into the new one. + let ch = Uuid::new_v4(); + let batch = FlushBatch { + channel_id: ch, + events: vec![BatchEvent { + event: make_event("fresh follow-up"), + prompt_tag: "@mention".into(), + received_at: Instant::now(), + admission_seq: 2, + enqueued_at_unix: 200, + restart_recovery: false, + cap_exempt: false, + }], + cancelled_events: vec![BatchEvent { + event: make_event("recovered prior task"), + prompt_tag: "@mention".into(), + received_at: Instant::now(), + admission_seq: 1, + enqueued_at_unix: 100, + restart_recovery: true, + cap_exempt: false, + }], + cancel_reason: Some(CancelReason::Steer), + }; + let prompt = format_prompt(&batch, &FormatPromptArgs::default()).join("\n\n"); + + assert!(prompt.contains("[Restart recovery]"), "{prompt}"); + assert!( + prompt.contains("--- Event 1 (@mention) [restart recovery] ---\nEvent ID:"), + "cancelled/prior event must be marked: {prompt}" + ); + assert!( + prompt.contains("--- Event 1 (@mention) ---\nEvent ID:"), + "the new event's own header (also numbered Event 1 in its own section) must NOT be marked: {prompt}" + ); + } + /// Helper: build a merged (cancel + re-prompt) batch with one cancelled /// event and one new event, framed by `reason`. fn make_merged_batch(reason: Option) -> FlushBatch { @@ -1902,11 +2736,19 @@ mod tests { event: make_event("the new message"), prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![BatchEvent { event: make_event("the original task"), prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancel_reason: reason, } @@ -2034,17 +2876,29 @@ mod tests { event: make_event("new one"), prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, BatchEvent { event: make_event("new two"), prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, ], cancelled_events: vec![BatchEvent { event: make_event("original"), prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancel_reason: Some(CancelReason::Steer), }; @@ -2090,11 +2944,19 @@ mod tests { event: steering, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![BatchEvent { event: original, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancel_reason: Some(CancelReason::Steer), }; @@ -2183,16 +3045,28 @@ mod tests { event: e1, prompt_tag: "tag-a".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, BatchEvent { event: e2, prompt_tag: "tag-b".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, BatchEvent { event: e3, prompt_tag: "tag-c".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, ], cancelled_events: vec![], @@ -2222,6 +3096,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2245,6 +3123,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2277,6 +3159,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2307,6 +3193,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2334,6 +3224,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2358,6 +3252,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2414,6 +3312,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2452,6 +3354,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -2681,6 +3587,10 @@ mod tests { event: make_event("old-msg"), received_at: old_time, prompt_tag: "test".into(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }); let batch = q.flush_next().expect("flush"); @@ -2969,6 +3879,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3000,6 +3914,10 @@ mod tests { event, prompt_tag: "dm".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3038,6 +3956,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3066,6 +3988,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3110,6 +4036,10 @@ mod tests { event, prompt_tag: "dm".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3159,6 +4089,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3366,6 +4300,10 @@ mod tests { event, prompt_tag: "dm".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3423,6 +4361,10 @@ mod tests { event, prompt_tag: "dm".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3463,6 +4405,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3487,6 +4433,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3510,6 +4460,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3592,6 +4546,28 @@ mod tests { assert!(any_in_flight(&q)); // in-flight unaffected } + #[test] + fn test_drain_channel_purges_recovery_mirror_but_keeps_liveness() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + + q.push(make_queued(ch, "msg1")); + let _batch = q.flush_next().unwrap(); + assert!(!q.in_flight_batch_triggers[&ch].is_empty()); + + q.drain_channel(ch); + + // Recovery mirror is purged: nothing left for the ledger to re-persist, + // so the membership-removal sync cannot resurrect the discarded turn. + assert!(!q.in_flight_batch_triggers.contains_key(&ch)); + assert!(q.recoverable_triggers(ch).is_empty()); + + // Liveness tracking survives: the in-flight task still completes or + // expires normally (removing either would wedge the channel). + assert!(q.in_flight_channels.contains(&ch)); + assert!(q.in_flight_deadlines.contains_key(&ch)); + } + #[test] fn test_compact_cleans_orphaned_retry_counts() { let mut q = EventQueue::new(DedupMode::Queue); @@ -3876,6 +4852,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3918,6 +4898,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3952,6 +4936,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -3981,6 +4969,10 @@ mod tests { event, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -4023,6 +5015,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -4059,6 +5055,10 @@ mod tests { event, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -4095,11 +5095,19 @@ mod tests { event: plain, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, BatchEvent { event: threaded, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, ], cancelled_events: vec![], @@ -4132,11 +5140,19 @@ mod tests { event: threaded, prompt_tag: "@mention".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, BatchEvent { event: plain, prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }, ], cancelled_events: vec![], @@ -4164,6 +5180,10 @@ mod tests { event: make_event(content), prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -4241,6 +5261,10 @@ mod tests { event: make_event("another message"), prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }); assert_eq!(slash_command_for_batch(&multi, &[]), None); @@ -4250,6 +5274,10 @@ mod tests { event: make_event("interrupted"), prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }); assert_eq!(slash_command_for_batch(&cancelled, &[]), None); @@ -4258,6 +5286,18 @@ mod tests { slash_command_for_batch(&make_single_batch("@Eva hello"), &[]), None ); + + // Recovered single-event slash command → no pass-through (must go + // through the wrapped-context path so the model sees the recovery + // header before any side effect fires — bare pass-through would + // re-execute a command that may have already run pre-restart). + let mut recovered = make_single_batch("@Eva /init"); + recovered.events[0].restart_recovery = true; + assert_eq!( + slash_command_for_batch(&recovered, &[]), + None, + "recovered slash command must not pass through unwrapped" + ); } // ── Goose-native steer withhold tests ─────────────────────────────────── @@ -4458,6 +5498,10 @@ mod tests { event: make_event("hi"), prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -4487,6 +5531,10 @@ mod tests { event: make_event("hi"), prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -4515,6 +5563,10 @@ mod tests { event: make_event("hi"), prompt_tag: "test".into(), received_at: Instant::now(), + admission_seq: 0, + enqueued_at_unix: 0, + restart_recovery: false, + cap_exempt: false, }], cancelled_events: vec![], cancel_reason: None, @@ -4756,4 +5808,469 @@ mod tests { "second extend must not move deadline backward (monotonic)" ); } + + // ── Resume-ledger seam: recoverable_triggers / dirty_channels ───────── + + #[test] + fn test_recoverable_triggers_gathers_all_four_tables_sorted_by_seq() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + + // seq 0: queued event (in-flight after flush). + q.push(make_queued(ch, "a")); + let batch = q.flush_next().expect("flush"); + // seq 1: queued while in-flight (Queue mode allows it). + q.push(make_queued(ch, "b")); + // Cancel the in-flight batch — moves seq-0 trigger into cancelled_batches. + q.complete_batch( + ch, + Some(batch), + BatchDisposition::Cancelled(CancelReason::Steer), + ); + // seq 2: withhold the queued (seq-1) event for native steer. + let seq1_id = q.recoverable_triggers(ch)[1].event_id.clone(); + assert!(q.mark_native_steer_pending(ch, &seq1_id)); + // seq 3: a fresh push lands back in queues. + q.push(make_queued(ch, "c")); + + let triggers = q.recoverable_triggers(ch); + assert_eq!(triggers.len(), 3, "cancelled + withheld + fresh queued"); + let seqs: Vec = triggers.iter().map(|t| t.admission_seq).collect(); + let mut sorted = seqs.clone(); + sorted.sort_unstable(); + assert_eq!( + seqs, sorted, + "recoverable_triggers must be admission_seq-ascending" + ); + } + + #[test] + fn test_take_dirty_channels_drains_and_resets() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.push(make_queued(ch, "a")); + assert_eq!(q.take_dirty_channels(), vec![ch]); + // Second call with no intervening mutation returns empty. + assert!(q.take_dirty_channels().is_empty()); + } + + #[test] + fn test_set_next_admission_seq_only_advances_forward() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.set_next_admission_seq(100); + q.push(make_queued(ch, "a")); + let seq = q.recoverable_triggers(ch)[0].admission_seq; + assert_eq!( + seq, 101, + "push after resume must land above the resumed floor" + ); + + // A lower resume request must not roll the counter backward. + q.set_next_admission_seq(5); + q.push(make_queued(ch, "b")); + let seq2 = q.recoverable_triggers(ch)[1].admission_seq; + assert_eq!(seq2, 102); + } + + // ── import_recovered: caller-supplied class is passed through verbatim ── + // + // The whole-snapshot promotion rule (plan:136,147-149) is computed by + // `boot_recover` over ALL gated records (fetched + unresolved) before + // the fetch split — see `test_boot_recover_promotion_from_full_snapshot_ + // unfetched_promotes` and `test_boot_recover_promotion_preserves_class_ + // when_exempt_exists` in lib.rs. `import_recovered` only ever sees the + // fetched subset, so it must not re-derive promotion from that partial + // view; these tests pin the pass-through contract at the queue level. + + #[test] + fn test_import_recovered_preserves_caller_supplied_exempt_bits() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + let mut e1 = make_queued(ch, "a"); + e1.admission_seq = 5; + e1.cap_exempt = true; // caller already decided: promote. + let mut e2 = make_queued(ch, "b"); + e2.admission_seq = 6; + e2.cap_exempt = true; + q.import_recovered(ch, vec![e2, e1]); + + let triggers = q.recoverable_triggers(ch); + assert_eq!(triggers.len(), 2); + assert!( + triggers.iter().all(|t| t.cap_exempt), + "import_recovered must not clear a caller-supplied exempt bit" + ); + // Restored in admission_seq order regardless of input order. + assert_eq!(triggers[0].admission_seq, 5); + assert_eq!(triggers[1].admission_seq, 6); + } + + #[test] + fn test_import_recovered_bypasses_cap() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + let events: Vec = (0..(MAX_PENDING_PER_CHANNEL as u64 + 50)) + .map(|seq| { + let mut e = make_queued(ch, "x"); + e.admission_seq = seq; + e + }) + .collect(); + q.import_recovered(ch, events); + assert_eq!( + q.recoverable_triggers(ch).len(), + MAX_PENDING_PER_CHANNEL + 50, + "import must not trim an over-cap snapshot" + ); + } + + // ── admit_recovered: seq-positioned insertion + barrier release ──────── + + #[test] + fn test_admit_recovered_bypasses_drop_mode_in_flight_rejection() { + let mut q = EventQueue::new(DedupMode::Drop); + let ch = Uuid::new_v4(); + q.push(make_queued(ch, "first")); + let _batch = q.flush_next().expect("flush"); + assert!(q.is_channel_in_flight(ch)); + + let mut recovered = make_queued(ch, "recovered"); + recovered.admission_seq = 999; + recovered.restart_recovery = true; + q.admit_recovered(ch, recovered); + + // Ordinary push would have silently dropped this in Drop mode; + // admit_recovered must not. + assert_eq!(q.queued_event_count(&ch), 1); + } + + #[test] + fn test_admit_recovered_releases_barrier_seq() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + let mut unresolved = make_queued(ch, "a"); + unresolved.admission_seq = 1; + let mut fetched = make_queued(ch, "b"); + fetched.admission_seq = 2; + q.import_recovered(ch, vec![fetched]); + + let deadline = Instant::now() + Duration::from_secs(60); + q.set_unresolved_barrier(ch, BTreeSet::from([1u64]), deadline); + + // B (seq 2) is held behind unresolved A (seq 1). + assert!( + q.flush_next().is_none(), + "B must not flush while A is unresolved" + ); + + // Resolution admits A and releases the barrier. + q.admit_recovered(ch, unresolved); + let batch = q.flush_next().expect("A resolved — suffix flushes"); + assert_eq!(batch.events.len(), 2); + assert_eq!(batch.events[0].event.content, "a"); + assert_eq!(batch.events[1].event.content, "b"); + } + + // ── Unresolved ordering barrier: three release paths ──────────────────── + + #[test] + fn test_barrier_holds_fresh_live_arrival_behind_the_hole() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.set_next_admission_seq(1); // fresh live pushes land above seq 1. + q.set_unresolved_barrier( + ch, + BTreeSet::from([1u64]), + Instant::now() + Duration::from_secs(60), + ); + + // Fresh live push necessarily stamps a higher seq than the hole. + q.push(make_queued(ch, "fresh")); + assert!( + q.flush_next().is_none(), + "fresh arrival behind the hole must be held" + ); + } + + #[test] + fn test_barrier_releases_on_invalidation() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + let mut fetched = make_queued(ch, "b"); + fetched.admission_seq = 2; + q.import_recovered(ch, vec![fetched]); + q.set_unresolved_barrier( + ch, + BTreeSet::from([1u64]), + Instant::now() + Duration::from_secs(60), + ); + + q.drain_channel(ch); + assert!( + q.next_unresolved_barrier_deadline().is_none(), + "invalidation must clear the barrier" + ); + } + + #[test] + fn test_barrier_releases_on_deadline_expiry() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + let mut fetched = make_queued(ch, "b"); + fetched.admission_seq = 2; + q.import_recovered(ch, vec![fetched]); + // Deadline already in the past — expiry should lift it immediately. + q.set_unresolved_barrier( + ch, + BTreeSet::from([1u64]), + Instant::now() - Duration::from_secs(1), + ); + + let batch = q + .flush_next() + .expect("expired barrier must release the suffix"); + assert_eq!(batch.events[0].event.content, "b"); + assert!(q.next_unresolved_barrier_deadline().is_none()); + } + + #[test] + fn test_next_unresolved_barrier_deadline_is_earliest_across_channels() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch_a = Uuid::new_v4(); + let ch_b = Uuid::new_v4(); + let earlier = Instant::now() + Duration::from_secs(10); + let later = Instant::now() + Duration::from_secs(60); + q.set_unresolved_barrier(ch_a, BTreeSet::from([1u64]), later); + q.set_unresolved_barrier(ch_b, BTreeSet::from([1u64]), earlier); + + assert_eq!(q.next_unresolved_barrier_deadline(), Some(earlier)); + } + + #[test] + fn test_barrier_holds_cancelled_batch_fallback_behind_the_hole() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.set_next_admission_seq(0); // next push lands at seq 1, leaving seq 0 open for the barrier. + q.push(make_queued(ch, "first")); // admission_seq 1 + let batch = q.flush_next().expect("flush"); + // Cancel it — lands in cancelled_batches at admission_seq 1. + q.complete_batch( + ch, + Some(batch), + BatchDisposition::Cancelled(CancelReason::Steer), + ); + + // Barrier holds seq 0 unresolved — strictly below the cancelled + // event's seq 1, so the cancelled-only fallback (which respects + // `split_at_barrier_floor`, the same per-event rule as the main + // drain path) must hold it rather than re-dispatch. + q.set_unresolved_barrier( + ch, + BTreeSet::from([0u64]), + Instant::now() + Duration::from_secs(60), + ); + assert!( + q.flush_next().is_none(), + "cancelled-only fallback must respect the barrier too" + ); + } + + // ── complete_batch disposition matrix (plan:244) ───────────────────── + + #[test] + fn test_complete_batch_retry_requeues_and_dead_letters_on_max() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.push(make_queued(ch, "a")); + let mut dead = None; + for _ in 0..=MAX_RETRIES { + q.retry_after + .insert(ch, Instant::now() - Duration::from_secs(1)); + let batch = q.flush_next().unwrap(); + dead = q.complete_batch(ch, Some(batch), BatchDisposition::Retry); + } + assert!( + dead.is_some(), + "after MAX_RETRIES+1 attempts, batch must be dead-lettered" + ); + assert!(q.flush_next().is_none(), "dead-lettered event is gone"); + } + + #[test] + fn test_complete_batch_cancelled_steer_preserves_events() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.push(make_queued(ch, "a")); + q.push(make_queued(ch, "b")); + let batch = q.flush_next().unwrap(); + let event_count = batch.events.len(); + + q.complete_batch( + ch, + Some(batch), + BatchDisposition::Cancelled(CancelReason::Steer), + ); + let triggers = q.recoverable_triggers(ch); + assert_eq!( + triggers.len(), + event_count, + "cancelled events must be preserved in recoverable set" + ); + } + + #[test] + fn test_complete_batch_dropped_removes_events() { + let mut q = EventQueue::new(DedupMode::Drop); + let ch = Uuid::new_v4(); + q.push(make_queued(ch, "a")); + let batch = q.flush_next().unwrap(); + + let dead = q.complete_batch(ch, Some(batch), BatchDisposition::Dropped); + assert!(dead.is_none()); + assert!(q.recoverable_triggers(ch).is_empty()); + } + + #[test] + fn test_complete_batch_preserve_timestamps_requeues() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.push(make_queued(ch, "a")); + let batch = q.flush_next().unwrap(); + let orig_seq = batch.events[0].admission_seq; + + q.complete_batch(ch, Some(batch), BatchDisposition::PreserveTimestamps); + let triggers = q.recoverable_triggers(ch); + assert_eq!(triggers.len(), 1); + assert_eq!( + triggers[0].admission_seq, orig_seq, + "PreserveTimestamps must keep original admission_seq" + ); + } + + // ── all-public-mutator dirty census ────────────────────────────────── + + #[test] + fn test_every_public_mutator_dirties_its_channel() { + let ch = Uuid::new_v4(); + + // push + let mut q = EventQueue::new(DedupMode::Queue); + q.push(make_queued(ch, "a")); + assert!(!q.take_dirty_channels().is_empty(), "push must dirty"); + + // flush_next + let batch = q.flush_next().unwrap(); + assert!(!q.take_dirty_channels().is_empty(), "flush_next must dirty"); + + // complete_batch (Success) + q.complete_batch(ch, Some(batch), BatchDisposition::Success); + assert!( + !q.take_dirty_channels().is_empty(), + "complete_batch must dirty" + ); + + // import_recovered + let mut e = make_queued(ch, "b"); + e.admission_seq = 10; + q.import_recovered(ch, vec![e]); + assert!( + !q.take_dirty_channels().is_empty(), + "import_recovered must dirty" + ); + + // admit_recovered + let mut e2 = make_queued(ch, "c"); + e2.admission_seq = 5; + e2.restart_recovery = true; + q.admit_recovered(ch, e2); + assert!( + !q.take_dirty_channels().is_empty(), + "admit_recovered must dirty" + ); + } + + // ── enforce_cap five-path matrix ───────────────────────────────────── + + #[test] + fn test_enforce_cap_push_evicts_from_front() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + for _ in 0..MAX_PENDING_PER_CHANNEL { + let mut e = make_queued(ch, "fill"); + e.cap_exempt = false; + q.push(e); + } + let first_seq = q.recoverable_triggers(ch)[0].admission_seq; + + let mut overflow = make_queued(ch, "overflow"); + overflow.cap_exempt = false; + q.push(overflow); + + let triggers = q.recoverable_triggers(ch); + let counted = triggers.iter().filter(|t| !t.cap_exempt).count(); + assert!( + counted <= MAX_PENDING_PER_CHANNEL, + "push must enforce cap on counted events" + ); + assert!( + !triggers.iter().any(|t| t.admission_seq == first_seq), + "oldest counted event must be evicted from front" + ); + } + + #[test] + fn test_enforce_cap_sustained_live_over_exempt_backlog() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + for i in 0..(MAX_PENDING_PER_CHANNEL + 100) { + let mut e = make_queued(ch, "exempt-backlog"); + e.cap_exempt = true; + e.admission_seq = i as u64; + q.import_recovered(ch, vec![e]); + } + + for _ in 0..20 { + let mut live = make_queued(ch, "live"); + live.cap_exempt = false; + q.push(live); + } + + let triggers = q.recoverable_triggers(ch); + let exempt_count = triggers.iter().filter(|t| t.cap_exempt).count(); + let counted_count = triggers.iter().filter(|t| !t.cap_exempt).count(); + assert_eq!( + exempt_count, + MAX_PENDING_PER_CHANNEL + 100, + "exempt backlog must be fully preserved" + ); + assert!( + counted_count <= MAX_PENDING_PER_CHANNEL, + "live counted events must still be cap-bounded" + ); + } + + // ── field stability across transitions ─────────────────────────────── + + #[test] + fn test_field_stability_across_push_flush_cancel_requeue() { + let mut q = EventQueue::new(DedupMode::Queue); + let ch = Uuid::new_v4(); + q.push(make_queued(ch, "stable")); + let orig = q.recoverable_triggers(ch)[0].clone(); + + let batch = q.flush_next().unwrap(); + q.complete_batch( + ch, + Some(batch), + BatchDisposition::Cancelled(CancelReason::Steer), + ); + + let after_cancel = q.recoverable_triggers(ch)[0].clone(); + assert_eq!(orig.admission_seq, after_cancel.admission_seq); + assert_eq!(orig.enqueued_at_unix, after_cancel.enqueued_at_unix); + assert_eq!(orig.cap_exempt, after_cancel.cap_exempt); + assert_eq!(orig.prompt_tag, after_cancel.prompt_tag); + assert_eq!(orig.event_id, after_cancel.event_id); + } } diff --git a/crates/buzz-acp/src/relay.rs b/crates/buzz-acp/src/relay.rs index aea5cee077..9dfc406b66 100644 --- a/crates/buzz-acp/src/relay.rs +++ b/crates/buzz-acp/src/relay.rs @@ -251,7 +251,7 @@ const REST_RETRY_BASE_DELAYS: [Duration; 3] = [ Duration::from_millis(2000), ]; -fn unix_now_secs() -> u64 { +pub(crate) fn unix_now_secs() -> u64 { std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() diff --git a/crates/buzz-acp/tests/setup_mode_ledger.rs b/crates/buzz-acp/tests/setup_mode_ledger.rs new file mode 100644 index 0000000000..91e002cdf7 --- /dev/null +++ b/crates/buzz-acp/tests/setup_mode_ledger.rs @@ -0,0 +1,99 @@ +//! Integration test: ledger deletion happens before the setup-mode early return. +//! +//! Exercises the boot-boundary ordering invariant: when `resume_on_restart=false`, +//! the ledger file must be deleted even when `BUZZ_ACP_SETUP_PAYLOAD` triggers +//! the early `run_setup_listener()` return — i.e., the deletion is NOT bypassed +//! by the setup-mode branch. +//! +//! The helper-only tests in `ledger.rs` (which call `delete_ledger_file` directly) +//! pass regardless of whether the call site in `lib.rs` is reachable. This test +//! is discriminating: it spawns the real binary and asserts the file-system +//! side-effect on the actual boot path. +//! +//! Setup: +//! - Private key: nsec of scalar 1 → known pubkey prefix `79be667ef9dcbbac`. +//! - Relay URL: `ws://127.0.0.1:1` → sha256 prefix `4d9ff4283940665b`. +//! - Ledger filename: `pending-turns-79be667ef9dcbbac-4d9ff4283940665b.json`. +//! - The binary connects to an unreachable relay (ws://127.0.0.1:1, connection +//! refused). Connection refused is non-terminal and retried with backoff, so +//! the binary keeps running — but the ledger deletion happens in the first +//! milliseconds of boot, long before the relay connect attempt. The test polls +//! for deletion and kills the child once confirmed rather than waiting for it +//! to exhaust its retry budget (~31 s). + +use std::time::Duration; + +/// nsec1 encoding of private key scalar 1. +const TEST_NSEC: &str = "nsec1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqsmhltgl"; + +/// A minimal valid `BUZZ_ACP_SETUP_PAYLOAD`. +const SETUP_PAYLOAD: &str = concat!( + r#"{"agent_name":"Test","#, + r#""agent_pubkey":"79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798","#, + r#""requirements":[]}"# +); + +/// Ledger filename for: +/// pubkey prefix = first 16 hex chars of 79be667ef9dcbbac55a06… +/// relay hash = first 16 hex chars of sha256("ws://127.0.0.1:1") +/// = 4d9ff4283940665b +const LEDGER_FILENAME: &str = "pending-turns-79be667ef9dcbbac-4d9ff4283940665b.json"; + +#[test] +fn test_setup_mode_boot_deletes_ledger_when_resume_off() { + let dir = tempfile::tempdir().expect("tempdir"); + let state_dir = dir.path(); + + // Step 1: seed the ledger file that the binary would delete. + let ledger_path = state_dir.join(LEDGER_FILENAME); + std::fs::write(&ledger_path, b"{}").expect("write seed ledger file"); + assert!(ledger_path.exists(), "seed file must exist before spawn"); + + let bin = env!("CARGO_BIN_EXE_buzz-acp"); + + // Step 2: spawn with resume_on_restart=false and a setup payload. + // The relay is unreachable so run_setup_listener exits quickly. + let mut child = std::process::Command::new(bin) + .env("BUZZ_PRIVATE_KEY", TEST_NSEC) + .env("BUZZ_RELAY_URL", "ws://127.0.0.1:1") + .env("BUZZ_ACP_RESUME_ON_RESTART", "false") + .env("BUZZ_ACP_STATE_DIR", state_dir) + .env("BUZZ_ACP_SETUP_PAYLOAD", SETUP_PAYLOAD) + .env("RUST_LOG", "error") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn buzz-acp binary"); + + // The ledger file must be deleted early in the boot sequence — before + // the relay connect attempt. The relay (`ws://127.0.0.1:1`) is + // unreachable, so the binary retries for ~31 seconds before exiting. + // We don't wait for the binary to exit; we poll for the file deletion + // (which happens in the first milliseconds of boot) and then kill the + // child once we have our answer. + let deadline = std::time::Instant::now() + Duration::from_secs(10); + let deleted = loop { + if !ledger_path.exists() { + break true; + } + match child.try_wait().expect("wait on child") { + Some(_) => break false, // exited before deleting — also a failure + None if std::time::Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(50)); + } + None => { + // Still running after 10s but file still exists — treat as failure. + break false; + } + } + }; + child.kill().ok(); + child.wait().ok(); + + // Step 3: assert the ledger file was deleted during boot (before setup-mode return). + assert!( + deleted, + "ledger file must be deleted even when BUZZ_ACP_SETUP_PAYLOAD triggers \ + the setup-listener early return (boot-boundary deletion ordering)" + ); +} diff --git a/desktop/src-tauri/src/commands/agent_config.rs b/desktop/src-tauri/src/commands/agent_config.rs index 5a26f0f645..3ae66af220 100644 --- a/desktop/src-tauri/src/commands/agent_config.rs +++ b/desktop/src-tauri/src/commands/agent_config.rs @@ -48,16 +48,15 @@ pub struct RuntimeFileConfigSubset { /// The pipeline: resolve the linked persona's prompt/model/provider, inject /// each into the record only where the record lacks its own value, let /// `read_config_surface` tag those injected fields `BuzzExplicit`, then re-tag -/// exactly the injected fields to `PersonaDefault`. -/// -/// Global defaults fill in when neither the record nor the linked persona -/// provides a value. They are re-tagged to `GlobalDefault` so the UI can -/// display "inherited from global defaults". +/// exactly the injected fields to `PersonaDefault`. Global defaults fill in +/// when neither the record nor the linked persona provides a value, and are +/// re-tagged `GlobalDefault` so the UI can show "inherited from global +/// defaults". /// /// The re-tag is triple-gated — a field is re-tagged only when (a) the record /// did not already have it (`!had_*`), (b) the surface produced the field, and -/// (c) the reader tagged it `BuzzExplicit`. A value the user set explicitly in -/// Buzz keeps `had_* == true` and is never re-tagged. +/// (c) the reader tagged it `BuzzExplicit`. A value the user set explicitly +/// keeps `had_* == true` and is never re-tagged. fn resolve_config_surface( mut record: ManagedAgentRecord, personas: &[AgentDefinition], @@ -662,6 +661,7 @@ mod tests { env_vars: Default::default(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/commands/agent_settings.rs b/desktop/src-tauri/src/commands/agent_settings.rs index 2317930c1e..38ab855b20 100644 --- a/desktop/src-tauri/src/commands/agent_settings.rs +++ b/desktop/src-tauri/src/commands/agent_settings.rs @@ -119,3 +119,54 @@ pub async fn set_managed_agent_auto_restart( .await .map_err(|e| format!("spawn_blocking failed: {e}"))? } + +#[tauri::command] +pub async fn set_managed_agent_resume_on_restart( + pubkey: String, + resume_on_restart: bool, + app: AppHandle, +) -> Result { + tokio::task::spawn_blocking(move || { + let state = app.state::(); + let _store_guard = state + .managed_agents_store_lock + .lock() + .map_err(|error| error.to_string())?; + let mut records = load_managed_agents(&app)?; + let mut runtimes = state + .managed_agent_processes + .lock() + .map_err(|error| error.to_string())?; + + let (sync_changed, exited_pubkeys) = + sync_managed_agent_processes(&mut records, &mut runtimes, ¤t_instance_id(&app)); + if sync_changed { + save_managed_agents(&app, &records)?; + } + for pubkey in &exited_pubkeys { + state.clear_agent_session_caches(pubkey); + } + + { + let record = find_managed_agent_mut(&mut records, &pubkey)?; + record.resume_on_restart = resume_on_restart; + record.updated_at = now_iso(); + } + + save_managed_agents(&app, &records)?; + let record = records + .iter() + .find(|record| record.pubkey == pubkey) + .ok_or_else(|| format!("agent {pubkey} not found"))?; + let personas = load_personas(&app).unwrap_or_default(); + build_managed_agent_summary( + &app, + record, + &runtimes, + &personas, + &crate::managed_agents::load_global_agent_config(&app).unwrap_or_default(), + ) + }) + .await + .map_err(|e| format!("spawn_blocking failed: {e}"))? +} diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 0758fc3aac..66a362d6b0 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -170,9 +170,8 @@ pub(super) fn build_agent_archive_request( /// window on 9035s. /// /// Same contract as `tombstone_managed_agent_pending`: called inside the -/// `managed_agents_store_lock`-held delete body, never across an `.await`, -/// best-effort — a failure is logged and swallowed so it never blocks the -/// disk-authoritative delete. +/// `managed_agents_store_lock`-held delete body, never across an `.await`, and +/// best-effort — a failure never blocks the disk-authoritative delete. pub(super) fn archive_managed_agent_pending(app: &AppHandle, state: &AppState, agent_pubkey: &str) { use crate::managed_agents::retention::{open_retention_db, retain_event, RetainedEvent}; use buzz_core_pkg::kind::KIND_IA_ARCHIVE_REQUEST; @@ -875,6 +874,7 @@ pub async fn create_managed_agent( input.start_on_app_launch }, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: input.backend.clone(), backend_agent_id: None, diff --git a/desktop/src-tauri/src/commands/agents_tests.rs b/desktop/src-tauri/src/commands/agents_tests.rs index 03389d1d18..be2128f0a3 100644 --- a/desktop/src-tauri/src/commands/agents_tests.rs +++ b/desktop/src-tauri/src/commands/agents_tests.rs @@ -59,6 +59,7 @@ fn bare_agent_record( catalog_source: None, relay_mesh: None, auto_restart_on_config_change: false, + resume_on_restart: true, definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, diff --git a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs index 8ff7cfbd9b..d3c9f9e645 100644 --- a/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs +++ b/desktop/src-tauri/src/commands/personas/delete_cascade_tests.rs @@ -67,6 +67,7 @@ fn make_agent( catalog_source: None, relay_mesh: None, auto_restart_on_config_change: false, + resume_on_restart: true, definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index 1005a83432..d968399ed4 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -182,6 +182,7 @@ fn local_agent() -> ManagedAgentRecord { env_vars: BTreeMap::from([("API_KEY".to_string(), "localsecret".to_string())]), start_on_app_launch: true, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: Some(1234), backend: crate::managed_agents::BackendKind::Provider { id: "buzz-backend".to_string(), diff --git a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs index 00a1457393..3637788197 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/fidelity_tests.rs @@ -36,6 +36,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { env_vars: BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: false, + resume_on_restart: true, runtime_pid: None, backend: BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/commands/personas/snapshot/import.rs b/desktop/src-tauri/src/commands/personas/snapshot/import.rs index eccf8ee601..855f265867 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -508,6 +508,7 @@ pub async fn confirm_agent_snapshot_import( env_vars: std::collections::BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: crate::managed_agents::BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs index 4289310280..07f9f8ff89 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/tests.rs @@ -45,6 +45,7 @@ fn make_definition(slug: &str) -> ManagedAgentRecord { env_vars: BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: false, + resume_on_restart: true, runtime_pid: None, backend: BackendKind::Local, backend_agent_id: None, @@ -509,8 +510,7 @@ fn import_none_level_with_entries_is_rejected() { ); } -/// A well-formed snapshot with memory level != none and non-empty entries -/// is accepted. +/// A well-formed snapshot with memory level != none and non-empty entries is accepted. #[test] fn import_memory_bearing_snapshot_is_accepted() { let entries = vec![ @@ -573,9 +573,9 @@ fn import_preview_includes_exported_definition_metadata() { // ── Import: resolve_snapshot_import_behavior — the production selection path // -// All tests below call `resolve_snapshot_import_behavior` directly. This is -// the same function invoked by `confirm_agent_snapshot_import`, so the tests -// exercise the exact production code path, not a reconstruction of it. +// All tests below call `resolve_snapshot_import_behavior` directly — the same +// function `confirm_agent_snapshot_import` invokes, so they exercise the +// production path rather than a reconstruction. /// Uppercase hex pubkeys are lowercased and duplicates are removed before the /// resolved allowlist is persisted. diff --git a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs index c60215ae4d..f64d82198f 100644 --- a/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs +++ b/desktop/src-tauri/src/commands/personas/update/name_propagation_tests.rs @@ -28,6 +28,7 @@ fn agent(persona_id: &str, name: &str, display_name: Option<&str>) -> ManagedAge env_vars: std::collections::BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: Default::default(), backend_agent_id: None, diff --git a/desktop/src-tauri/src/commands/team_snapshot.rs b/desktop/src-tauri/src/commands/team_snapshot.rs index 97cd11933d..e5e26fbfd5 100644 --- a/desktop/src-tauri/src/commands/team_snapshot.rs +++ b/desktop/src-tauri/src/commands/team_snapshot.rs @@ -576,6 +576,7 @@ pub async fn confirm_team_snapshot_import( env_vars: std::collections::BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: crate::managed_agents::BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/commands/team_snapshot/tests.rs b/desktop/src-tauri/src/commands/team_snapshot/tests.rs index c9a6d8812a..228e41321e 100644 --- a/desktop/src-tauri/src/commands/team_snapshot/tests.rs +++ b/desktop/src-tauri/src/commands/team_snapshot/tests.rs @@ -203,6 +203,7 @@ fn team_export_with_instance_and_memory_level_uses_supplied_entries() { env_vars: Default::default(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: crate::managed_agents::BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 316acec2ab..98e816ecb6 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -813,6 +813,7 @@ pub fn run() { set_agent_managed_profiles, set_managed_agent_start_on_app_launch, set_managed_agent_auto_restart, + set_managed_agent_resume_on_restart, delete_managed_agent, get_managed_agent_log, get_agent_models, diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index 4a7b80079d..6897a4fba1 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -181,6 +181,7 @@ mod tests { env_vars: BTreeMap::from([("OPENAI_API_KEY".to_string(), "sk-secret".to_string())]), start_on_app_launch: true, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: Some(4242), backend: super::super::BackendKind::Provider { id: "buzz-backend-x".to_string(), diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs index 16a0d35b23..ddcc484426 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot.rs @@ -266,8 +266,7 @@ fn resolve_avatar( return (Some(data_url), None); } } - // Fall back to URL reference (caller provided a URL avatar or bytes were - // too large). + // Fall back to URL reference (caller provided a URL avatar or bytes were too large). let url_ref = record .avatar_url .as_deref() @@ -511,6 +510,7 @@ mod tests { }, start_on_app_launch: true, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: Some(12345), // MUST NOT appear backend: BackendKind::Provider { // MUST NOT appear — carries a provider secret diff --git a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs index 4ee4ec79c3..e3f6085653 100644 --- a/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs +++ b/desktop/src-tauri/src/managed_agents/config_bridge/reader_tests.rs @@ -84,6 +84,7 @@ fn test_record() -> ManagedAgentRecord { env_vars: BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: crate::managed_agents::types::BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/managed_agents/discovery/tests.rs b/desktop/src-tauri/src/managed_agents/discovery/tests.rs index 6fe6a77521..dc73c5a0a8 100644 --- a/desktop/src-tauri/src/managed_agents/discovery/tests.rs +++ b/desktop/src-tauri/src/managed_agents/discovery/tests.rs @@ -252,6 +252,7 @@ fn record_with( persona_source_version: None, start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: Default::default(), backend_agent_id: None, @@ -396,8 +397,7 @@ fn effective_agent_command_empty_override_is_inherit() { #[test] fn effective_agent_command_falls_back_to_default() { - // No override, no persona runtime, and a deleted persona all fall back - // to the bundled default. + // No override, no persona runtime, and a deleted persona all fall back to the bundled default. let personas = vec![persona_with_runtime("p1", None)]; assert_eq!( effective_agent_command(Some("p1"), &personas, None), diff --git a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs index c8e437809c..25de717765 100644 --- a/desktop/src-tauri/src/managed_agents/effective_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/effective_config/tests.rs @@ -89,6 +89,7 @@ fn record( catalog_source: None, relay_mesh: None, auto_restart_on_config_change: false, + resume_on_restart: true, definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 592a5cbbd9..df12b2060d 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -77,6 +77,12 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ "BUZZ_ACP_RESPOND_TO", "BUZZ_ACP_RESPOND_TO_ALLOWLIST", "BUZZ_ACP_AGENT_OWNER", + // Resume toggle: overriding would make the running agent's resume + // behavior diverge from the saved/UI-visible setting. Unlike + // BUZZ_ACP_IDLE_TIMEOUT (a tuning knob with no UI promise), this key + // carries an explicit UI toggle promise — the running agent must + // always match what the UI shows. + "BUZZ_ACP_RESUME_ON_RESTART", // Readiness handoff: desktop is the ONLY readiness source. A saved or // ambient env var must not be able to forge setup mode (NotReady) on a // Ready agent or suppress it (empty/stale payload) on a NotReady one. @@ -330,5 +336,21 @@ pub(crate) fn resolve_persona_env( Ok(persona.env_vars) } +/// Emit `BUZZ_ACP_RESUME_ON_RESTART` onto a child `Command`, making the UI +/// toggle authoritative in both directions. +/// +/// Two-step: first `env_remove` clears any ambient value the desktop process +/// inherited from its own environment, then `env` writes the record value +/// unconditionally. This means `false` wins over an ambient `true` AND `true` +/// wins over an ambient `false` — no leak in either direction. +/// +/// `BUZZ_ACP_RESUME_ON_RESTART` is also in `RESERVED_ENV_KEYS`, so saved user +/// env cannot reintroduce it after this call (reserved stripping happens in +/// `merged_user_env`, applied when building `descriptor.env`). +pub(crate) fn apply_resume_env(cmd: &mut std::process::Command, resume_on_restart: bool) { + cmd.env_remove("BUZZ_ACP_RESUME_ON_RESTART"); + cmd.env("BUZZ_ACP_RESUME_ON_RESTART", resume_on_restart.to_string()); +} + #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index cf57b12546..3cd1ccf5c6 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -1,7 +1,7 @@ use std::collections::BTreeMap; use super::{ - display_invalid_key, is_derived_provider_model_key, is_reserved_env_key, + apply_resume_env, display_invalid_key, is_derived_provider_model_key, is_reserved_env_key, is_well_formed_env_key, merged_user_env, validate_user_env_keys, DERIVED_PROVIDER_MODEL_ENV_KEYS, MAX_ENV_TOTAL_BYTES, MAX_ENV_VALUE_BYTES, RESERVED_ENV_KEYS, }; @@ -488,3 +488,87 @@ fn deploy_model_precedence_none_when_both_absent() { let effective = persona_model.clone().or(record_model.clone()); assert_eq!(effective, None); } + +// ── resume_on_restart spawn-env precedence ────────────────────────────── +// +// BUZZ_ACP_RESUME_ON_RESTART is reserved — users must not override it via +// the env-vars editor — and the desktop emits it unconditionally so the +// UI toggle is the sole control plane in both directions. + +#[test] +fn resume_on_restart_is_reserved_and_stripped_from_user_env() { + // A user saving BUZZ_ACP_RESUME_ON_RESTART in the env-vars editor must + // have it rejected at validation time AND stripped at spawn time as a + // defense-in-depth guard for older on-disk records. + assert!( + is_reserved_env_key("BUZZ_ACP_RESUME_ON_RESTART"), + "BUZZ_ACP_RESUME_ON_RESTART must be reserved" + ); + + // Even if a legacy record somehow carries the key, merged_user_env must + // strip it before it reaches Command::env. + let agent = map(&[ + ("BUZZ_ACP_RESUME_ON_RESTART", "true"), + ("ANTHROPIC_API_KEY", "ok"), + ]); + let merged = merged_user_env(&BTreeMap::new(), &agent); + assert!( + !merged.contains_key("BUZZ_ACP_RESUME_ON_RESTART"), + "reserved key must not survive merged_user_env" + ); + assert_eq!( + merged.get("ANTHROPIC_API_KEY").map(String::as_str), + Some("ok"), + "unrelated key must pass through" + ); +} + +#[test] +fn resume_env_toggle_off_writes_false_overriding_ambient() { + // When the UI toggle is OFF, the child must see "false" even when the + // desktop process itself has BUZZ_ACP_RESUME_ON_RESTART=true in its env + // (ambient parent leak). Routes through the production `apply_resume_env` + // helper called by `spawn_agent_child`; regresses if that call is removed + // or reverts to conditional emission. + let mut cmd = std::process::Command::new("true"); + + // Simulate ambient parent-process env (std::process::Command inherits + // from the parent; cmd.env overwrites in the same priority order). + cmd.env("BUZZ_ACP_RESUME_ON_RESTART", "true"); // ambient "on" + + apply_resume_env(&mut cmd, false); + + let resume_val = cmd + .get_envs() + .find(|(k, _)| *k == "BUZZ_ACP_RESUME_ON_RESTART") + .and_then(|(_, v)| v) + .map(|v| v.to_string_lossy().into_owned()); + assert_eq!( + resume_val.as_deref(), + Some("false"), + "toggle off must write false even when ambient env says true" + ); +} + +#[test] +fn resume_env_toggle_on_writes_true_overriding_ambient() { + // When the UI toggle is ON, the child must see "true" even when the + // desktop process has BUZZ_ACP_RESUME_ON_RESTART=false in its env. + // Routes through the production `apply_resume_env` helper. + let mut cmd = std::process::Command::new("true"); + + cmd.env("BUZZ_ACP_RESUME_ON_RESTART", "false"); // ambient "off" + + apply_resume_env(&mut cmd, true); + + let resume_val = cmd + .get_envs() + .find(|(k, _)| *k == "BUZZ_ACP_RESUME_ON_RESTART") + .and_then(|(_, v)| v) + .map(|v| v.to_string_lossy().into_owned()); + assert_eq!( + resume_val.as_deref(), + Some("true"), + "toggle on must write true even when ambient env says false" + ); +} diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 553596e226..880a45ac11 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -349,6 +349,7 @@ fn bare_record() -> ManagedAgentRecord { catalog_source: None, relay_mesh: None, auto_restart_on_config_change: false, + resume_on_restart: true, definition_respond_to: None, definition_respond_to_allowlist: vec![], definition_parallelism: None, diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index 031b049a49..b2692aac46 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -471,6 +471,7 @@ fn make_agent(name: &str, persona_id: Option<&str>) -> ManagedAgentRecord { persona_source_version: None, start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: BackendKind::default(), backend_agent_id: None, diff --git a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs index b9542f9a87..723fa428d1 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events/tests.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events/tests.rs @@ -28,6 +28,7 @@ fn sample_record() -> ManagedAgentRecord { env_vars: BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index fa8eb36fa1..47b5e344d4 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -113,8 +113,7 @@ pub(crate) struct EffectiveHarnessDescriptor { /// persona) references a runtime id that no longer exists in the registry — /// the same typed error produced by `try_record_agent_command`. Callers that /// cannot meaningfully continue with a dangling id (e.g. `spawn_agent_child`) -/// propagate the error; callers that degrade gracefully may use -/// `.unwrap_or_else(|_| …)`. +/// propagate the error; callers that degrade gracefully may use `.unwrap_or_else(|_| …)`. /// /// Does NOT require an `AppHandle` so it is fully unit-testable. /// @@ -1502,6 +1501,7 @@ mod tests { env_vars, start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: Default::default(), backend_agent_id: None, diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 37927961ed..8695336563 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -150,13 +150,13 @@ pub fn build_managed_agent_summary( // Two-axis status model for remote agents: // // Control-plane (this field): "deployed" = provider has been invoked and - // returned a backend_agent_id. "not_deployed" = no deploy call yet (or it - // failed). This axis tracks whether infrastructure *exists*, not whether - // the process is currently running. + // returned a backend_agent_id. "not_deployed" = no deploy call yet (or + // it failed). Tracks whether infrastructure *exists*, not whether the + // process is currently running. // - // Live axis (relay presence, polled by frontend): online/away/offline. - // Shown as a PresenceDot next to the agent name. This is the real-time - // signal for whether the harness is connected. + // Live axis (relay presence, polled by frontend): online/away/offline, + // shown as a PresenceDot next to the agent name — the real-time signal + // for whether the harness is connected. // // After !shutdown the agent goes offline (presence) but stays "deployed" // (infrastructure still exists). This is intentional — the provider may @@ -243,7 +243,6 @@ pub fn build_managed_agent_summary( // Global config drives both the restart-drift hash and 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))) @@ -335,6 +334,7 @@ pub fn build_managed_agent_summary( last_error_code: record.last_error_code, start_on_app_launch: record.start_on_app_launch, auto_restart_on_config_change: record.auto_restart_on_config_change, + resume_on_restart: record.resume_on_restart, log_path, respond_to: record.respond_to, respond_to_allowlist: record.respond_to_allowlist.clone(), @@ -603,12 +603,8 @@ pub fn spawn_agent_child( // readiness predicate, and if anything is missing, serialize the payload // into BUZZ_ACP_SETUP_PAYLOAD. buzz-acp detects this env var on startup // and enters the minimal setup-listener mode instead of the agent pool. - // - // SECURITY: BUZZ_ACP_SETUP_PAYLOAD is in RESERVED_ENV_KEYS so user env - // cannot set it, but we also explicitly remove it after writing user env - // to guard against the parent-process environment. We then set it only - // when desktop has computed NotReady — the desktop is the sole readiness - // source and buzz-acp only transports the payload. + // SECURITY: the key is in RESERVED_ENV_KEYS, and desktop is the sole + // readiness source — buzz-acp only transports the payload. // // The JSON format mirrors `setup_mode::SetupPayload` in buzz-acp: // { "agent_name": "...", "agent_pubkey": "...", "requirements": [{ "surface": "...", ... }] } @@ -699,9 +695,8 @@ pub fn spawn_agent_child( // 2. This env_remove() clears any ambient parent-process value // inherited by std::process::Command before we conditionally // set the desktop-computed trusted value below. - // Note: merged_user_env() is written further below in this function; - // ordering relative to that call is NOT what makes this safe — the - // reserved-key strip (guard 1) handles user env regardless of order. + // Guard 1 handles user env regardless of where merged_user_env() is + // written relative to this call, so ordering is NOT what makes it safe. command.env_remove("BUZZ_ACP_SETUP_PAYLOAD"); // Set the payload only when desktop computed NotReady. @@ -726,6 +721,8 @@ pub fn spawn_agent_child( if let Some(max_dur) = record.max_turn_duration_seconds { command.env("BUZZ_ACP_MAX_TURN_DURATION", max_dur.to_string()); } + // Reserved and emitted unconditionally — UI toggle is authoritative, no ambient env leak. + super::env_vars::apply_resume_env(&mut command, record.resume_on_restart); command.env("BUZZ_ACP_AGENTS", record.parallelism.to_string()); command.env("BUZZ_ACP_MULTIPLE_EVENT_HANDLING", "steer"); command.env("BUZZ_ACP_DEDUP", "queue"); @@ -749,10 +746,10 @@ pub fn spawn_agent_child( // instances never consult the record's own model/provider/prompt bytes; // definition-less instances fall back to their own fields, then global. // - // Derive the mesh decision BEFORE moving fields out — `relay_mesh_model_id` - // is the single authoritative gate; the mesh-llm block below MUST use it - // rather than re-deriving from `effective_provider` to keep preflight and - // spawn semantics in lock-step (see `EffectiveAgentConfig::relay_mesh_model_id`). + // Derive the mesh decision BEFORE moving fields out — `relay_mesh_model_id` is + // the single authoritative gate; the mesh-llm block below MUST use it rather + // than re-deriving from `effective_provider`, keeping preflight and spawn + // semantics in lock-step (see `EffectiveAgentConfig::relay_mesh_model_id`). #[cfg(feature = "mesh-llm")] let mesh_model_id = effective_cfg.relay_mesh_model_id(); let effective_prompt = effective_cfg.system_prompt.value; @@ -816,13 +813,13 @@ pub fn spawn_agent_child( // ── Git credential helper for Buzz relay ────────────────────────── // - // Agents need to clone/push repos hosted on the Buzz relay's git - // server, which authenticates via NIP-98. The `git-credential-nostr` - // binary signs auth events using the agent's nostr key. + // Agents need to clone/push repos hosted on the Buzz relay's git server, + // which authenticates via NIP-98. The `git-credential-nostr` binary signs + // auth events using the agent's nostr key. // - // We configure git via GIT_CONFIG_COUNT env vars (ephemeral, no - // filesystem writes) scoped to the relay's git URL so we don't - // interfere with other remotes (e.g. GitHub). + // We configure git via GIT_CONFIG_COUNT env vars (ephemeral, no filesystem + // writes) scoped to the relay's git URL so we don't interfere with other + // remotes (e.g. GitHub). // // NOSTR_PRIVATE_KEY mirrors BUZZ_PRIVATE_KEY — keep in sync. if let Some(cred_helper) = resolve_command("git-credential-nostr") { @@ -855,8 +852,8 @@ pub fn spawn_agent_child( // baked floor → runtime metadata → definition env (harness author defaults) → // global → live persona → per-agent, with reserved-key and malformed-key filtering // applied. Writing it last lets user-provided values win over every Buzz-set env - // written above — reserved keys were already stripped from descriptor.env so they - // cannot clobber BUZZ_PRIVATE_KEY, NOSTR_PRIVATE_KEY, etc. + // written above — reserved keys were already stripped, so they cannot clobber + // BUZZ_PRIVATE_KEY, NOSTR_PRIVATE_KEY, etc. for (key, value) in &descriptor.env { command.env(key, value); } @@ -922,11 +919,10 @@ pub fn spawn_agent_child( // 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). - // Non-codex runtimes get `None` — nothing changes for them. - // When the cache is cold (e.g. Doctor just installed and cleared the cache), - // `adapter_availability_cached()` returns `None`, so the stamp is `None` and - // the drift check is skipped until discovery warms the cache — preventing a - // false restart badge immediately after auto-restart. + // Non-codex runtimes get `None` — nothing changes for them. A cold cache + // (e.g. Doctor just installed and cleared it) also stamps `None`, skipping + // the drift check until discovery warms it — this prevents a false restart + // badge immediately after auto-restart. let spawned_adapter_availability = if runtime_meta.is_some_and(|r| r.id == "codex") { super::adapter_availability_cached() } else { diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 3f6ee996f6..9e39d039fa 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -151,6 +151,7 @@ fn fixture( env_vars: std::collections::BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: Default::default(), backend_agent_id: None, @@ -251,8 +252,7 @@ fn build_env_legacy_record_without_auth_tag_emits_agent_owner() { #[test] fn build_env_legacy_record_without_owner_hex_removes_agent_owner() { - // No owner available to forward → make sure we don't inherit a leaked - // env var from the parent. + // No owner available to forward → make sure we don't inherit a leaked env var from the parent. let rec = fixture(RespondTo::OwnerOnly, vec![], None); let (_set, remove) = build_respond_to_env(&rec, None).unwrap(); assert!(remove.contains(&"BUZZ_ACP_AGENT_OWNER")); diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash.rs b/desktop/src-tauri/src/managed_agents/spawn_hash.rs index 648cc62bbe..623fe5d39a 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash.rs @@ -151,6 +151,7 @@ pub(crate) fn spawn_config_hash( } record.idle_timeout_seconds.hash(&mut hasher); record.max_turn_duration_seconds.hash(&mut hasher); + record.resume_on_restart.hash(&mut hasher); record.parallelism.hash(&mut hasher); hasher.finish() diff --git a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs index f4ad404814..0e92e49391 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_hash/tests.rs @@ -27,6 +27,7 @@ fn record() -> ManagedAgentRecord { env_vars: BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: Default::default(), backend_agent_id: None, @@ -765,3 +766,16 @@ fn spawn_hash_instance_args_win_over_definition_args() { "instance args and definition args must produce different hashes" ); } + +#[test] +fn resume_on_restart_toggle_changes_hash() { + // Flipping the toggle changes what a spawn writes + // (BUZZ_ACP_RESUME_ON_RESTART), so the restart-required badge must trip. + let on = record(); + let mut off = record(); + off.resume_on_restart = false; + assert_ne!( + spawn_config_hash(&on, &[], &[], "wss://ws.example", &Default::default()), + spawn_config_hash(&off, &[], &[], "wss://ws.example", &Default::default()) + ); +} diff --git a/desktop/src-tauri/src/managed_agents/team_snapshot.rs b/desktop/src-tauri/src/managed_agents/team_snapshot.rs index 96082acc76..cedd1613d2 100644 --- a/desktop/src-tauri/src/managed_agents/team_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/team_snapshot.rs @@ -280,6 +280,7 @@ mod tests { }, start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/managed_agents/teams_tests.rs b/desktop/src-tauri/src/managed_agents/teams_tests.rs index 1ffa60eda9..cd67b34da8 100644 --- a/desktop/src-tauri/src/managed_agents/teams_tests.rs +++ b/desktop/src-tauri/src/managed_agents/teams_tests.rs @@ -187,6 +187,7 @@ fn managed_agent(name: &str) -> ManagedAgentRecord { env_vars: std::collections::BTreeMap::new(), start_on_app_launch: false, auto_restart_on_config_change: false, + resume_on_restart: true, runtime_pid: None, backend: crate::managed_agents::BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index fcd8b13fc9..db7d23d11d 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -122,6 +122,7 @@ impl AgentDefinition { env_vars: self.env_vars, start_on_app_launch: false, auto_restart_on_config_change: true, + resume_on_restart: true, runtime_pid: None, backend: BackendKind::default(), backend_agent_id: None, @@ -222,10 +223,9 @@ pub struct ManagedAgentRecord { /// storage layer blanks this before writing JSON once the key is safely in /// the keyring, and re-hydrates it from the keyring on load. /// - /// It is only serialized inline (the `0o600` JSON fallback) when the - /// keyring is unreachable — `skip_serializing_if` keeps it out of JSON in - /// the normal keyring-backed case. `default` also lets an old build parse a - /// store whose inline key was already migrated out and blanked. + /// It is only serialized inline (the `0o600` JSON fallback) when the keyring + /// is unreachable — `skip_serializing_if` keeps it out of JSON otherwise, and + /// `default` lets an old build parse a store whose inline key was migrated out. #[serde(default, skip_serializing_if = "String::is_empty")] pub private_key_nsec: String, /// NIP-OA auth tag JSON. Computed at agent creation time. @@ -245,13 +245,13 @@ pub struct ManagedAgentRecord { pub avatar_url: Option, pub acp_command: String, pub agent_command: String, - /// Explicit per-instance harness pin. `None` (the default) means inherit - /// the harness from the linked persona's `runtime`, so persona harness - /// edits propagate on the next spawn — mirroring the opt-in `model` - /// override. `Some` is set only when the user deliberately picks a harness - /// that diverges from the persona. Resolved via `effective_agent_command`; - /// `agent_command` above is the create-time snapshot kept for avatar/legacy - /// derivations and is not authoritative for spawn. + /// Explicit per-instance harness pin. `None` (the default) inherits the + /// harness from the linked persona's `runtime`, so persona harness edits + /// propagate on the next spawn — mirroring the opt-in `model` override. + /// `Some` is set only when the user deliberately picks a harness that + /// diverges from the persona. Resolved via `effective_agent_command` + /// (`agent_command` above is a create-time snapshot for avatar/legacy + /// derivations, not authoritative for spawn). #[serde(default)] pub agent_command_override: Option, pub agent_args: Vec, @@ -260,9 +260,8 @@ pub struct ManagedAgentRecord { /// (`known_acp_runtime`) — and no longer written by updates. Kept for /// serde compatibility with existing stores. pub mcp_command: String, - /// Deprecated: `BUZZ_ACP_TURN_TIMEOUT` is ignored by the harness and the - /// desktop no longer emits or edits it. Kept for serde compatibility with - /// existing stores; use `idle_timeout_seconds` or + /// Deprecated: `BUZZ_ACP_TURN_TIMEOUT` is ignored by the harness and the desktop no + /// longer emits or edits it. Kept for serde compat; use `idle_timeout_seconds` or /// `max_turn_duration_seconds` for turn-length control. pub turn_timeout_seconds: u64, /// Idle timeout in seconds (`BUZZ_ACP_IDLE_TIMEOUT`): how long the agent @@ -276,29 +275,23 @@ pub struct ManagedAgentRecord { #[serde(default = "default_agent_parallelism")] pub parallelism: u32, pub system_prompt: Option, - /// Desired LLM model ID. Matches AgentModelInfo.id from discovery. - /// The harness re-discovers the correct ACP switching metadata at session - /// creation by matching this ID against the fresh session/new response. - /// For a linked instance this is a legacy/display snapshot only — spawn - /// and deploy resolve the effective model from the definition, never - /// from this field (see `effective_config::resolve_effective_config`). - /// For a definition-less instance this field is authoritative. + /// Desired LLM model ID. Matches `AgentModelInfo.id` from discovery; the harness + /// re-discovers the correct ACP switching metadata on session creation. For a linked + /// instance this is a legacy/display snapshot — spawn and deploy resolve from the + /// definition (see `effective_config::resolve_effective_config`). For a + /// definition-less instance this field is authoritative. #[serde(default)] pub model: Option, - /// LLM inference provider. For a linked instance this is a legacy/display - /// snapshot only — spawn and deploy resolve the effective provider from - /// the definition, never from this field (see - /// `effective_config::resolve_effective_config`). For a definition-less - /// instance this field is authoritative. `#[serde(default)]` so - /// pre-existing records deserialize as `None` and get backfilled on - /// first load. + /// LLM inference provider. Like `model` above, a linked instance's value is a + /// legacy/display snapshot — spawn and deploy resolve from the definition. For a + /// definition-less instance this field is authoritative. `#[serde(default)]` so + /// pre-existing records deserialize as `None` and backfill on first load. #[serde(default)] pub provider: Option, - /// Content hash of the persona at the time this agent was created — the - /// `persona_content_hash` of the snapshot in `system_prompt` / `model` / - /// `provider` / `env_vars`. The Agents menu compares it against the linked - /// persona's current hash to flag a stale (out-of-date) instance. `None` - /// for non-persona agents and for pre-existing records pending backfill. + /// Content hash of the persona at this agent's creation time (snapshot of + /// `system_prompt`/`model`/`provider`/`env_vars`). The Agents menu compares it + /// against the linked persona's current hash to flag a stale instance. `None` for + /// non-persona agents and pre-existing records pending backfill. #[serde(default)] pub persona_source_version: Option, /// Environment variables injected at spawn time. Layered as: desktop @@ -314,6 +307,10 @@ pub struct ManagedAgentRecord { /// frontend only fires when the agent is idle, connected, and local. #[serde(default = "default_auto_restart_on_config_change")] pub auto_restart_on_config_change: bool, + /// Replay the durable pending-turn ledger on harness boot so a crash or app + /// update does not drop in-flight work (`BUZZ_ACP_RESUME_ON_RESTART`). + #[serde(default = "default_resume_on_restart")] + pub resume_on_restart: bool, #[serde(default)] pub runtime_pid: Option, #[serde(default)] @@ -365,16 +362,15 @@ pub struct ManagedAgentRecord { /// Absorbed from `AgentDefinition.runtime` — the preferred ACP runtime ID /// (e.g. 'goose', 'claude'). Record-first command resolution reads this /// before falling back to legacy persona lookup; populated by the store - /// migration and at create time, and re-mirrored from the linked - /// definition at every snapshot apply (`apply_persona_snapshot`). + /// migration and at create time, and re-mirrored from the linked definition + /// at every snapshot apply (`apply_persona_snapshot`). /// /// `None` means "inherit from the linked definition" (the Inherit sentinel /// clears it). Serialization then omits the key, so boot-time /// `materialize_agent_runtimes` re-inserts a mirror of the definition's /// current runtime on the next launch — behaviorally identical, because /// every apply site re-mirrors the live definition anyway. A literal - /// `"runtime": null` in the store (key present, e.g. hand-edited) is - /// honored: materialization skips it and it deserializes to `None`. + /// `"runtime": null` (hand-edited) is honored: materialization skips it. #[serde(default, skip_serializing_if = "Option::is_none")] pub runtime: Option, /// Pool of short thematic names for clones of this agent. Absorbed from @@ -412,11 +408,11 @@ pub struct ManagedAgentRecord { /// NIP-AP definition-level behavioral defaults, absorbed from /// `AgentDefinition` in WIRE shape (kebab-case string / optional u32), /// distinct from the instance-side `respond_to`/`respond_to_allowlist`/ - /// `parallelism` fields above: these are what a *definition* advertises - /// and are copied onto instances at mint time only. Wire shape (not the + /// `parallelism` fields above: these are what a *definition* advertises and + /// are copied onto instances at mint time only. Wire shape (not the /// `RespondTo` enum) so absent-ness and unknown future mode strings - /// round-trip byte-identically through the store — parsed/validated - /// solely at the mint boundary. + /// round-trip byte-identically through the store — parsed/validated solely + /// at the mint boundary. #[serde(default, skip_serializing_if = "Option::is_none")] pub definition_respond_to: Option, #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -496,25 +492,23 @@ pub struct ManagedAgentSummary { pub pubkey: String, pub name: String, pub persona_id: Option, - /// The record's harness/runtime id (mirror of `ManagedAgentRecord.runtime`). - /// Lets the UI count agents referencing a harness definition (e.g. in the + /// The record's harness/runtime id (mirror of `ManagedAgentRecord.runtime`). Lets + /// the UI count agents referencing a harness definition (e.g. in the /// delete-confirmation flow). `None` = inherit from the linked persona. pub runtime: Option, pub team_id: Option, pub relay_url: String, pub acp_command: String, pub agent_command: String, - /// Mirrors `ManagedAgentRecord.agent_command_override`: `Some` when the user - /// has explicitly pinned this instance's harness, `None` when it inherits - /// from the persona. Lets the Edit dialog seed "Inherit from persona" vs a - /// concrete pin (`agent_command` above is the resolved/effective command). + /// Mirrors `ManagedAgentRecord.agent_command_override`: `Some` when the user has + /// explicitly pinned this instance's harness, `None` when it inherits from the + /// persona (`agent_command` above is the resolved/effective command). pub agent_command_override: Option, pub agent_args: Vec, - /// Catalog-derived from the effective harness (not the record's stored - /// field), so the UI always shows what a spawn would actually use. + /// Catalog-derived from the effective harness (not the stored field), so the UI + /// always shows what a spawn would actually use. pub mcp_command: String, - /// Deprecated passthrough of the stored record value; the harness ignores - /// it. Kept for wire compatibility. + /// Deprecated passthrough; the harness ignores it. Kept for wire compatibility. pub turn_timeout_seconds: u64, pub idle_timeout_seconds: Option, pub max_turn_duration_seconds: Option, @@ -538,15 +532,14 @@ pub struct ManagedAgentSummary { /// Distinct from out-of-date: there is no current persona to respawn into. /// An orphaned agent also cannot be (re)started — `spawn_agent_child` /// refuses it (see `effective_config::resolve_effective_config`'s - /// `OrphanedInstance` arm via `require_resolved`) — so the UI - /// should surface that it's stuck, not merely stale. + /// `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 was spawned with a config that no longer + /// matches what a spawn would use today. 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). pub needs_restart: bool, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub env_vars: BTreeMap, @@ -563,6 +556,7 @@ pub struct ManagedAgentSummary { pub last_error_code: Option, pub start_on_app_launch: bool, pub auto_restart_on_config_change: bool, + pub resume_on_restart: bool, pub log_path: String, pub respond_to: RespondTo, pub respond_to_allowlist: Vec, @@ -825,6 +819,10 @@ fn default_auto_restart_on_config_change() -> bool { true } +fn default_resume_on_restart() -> bool { + true +} + fn default_record_active() -> bool { true } diff --git a/desktop/src-tauri/src/managed_agents/types/tests.rs b/desktop/src-tauri/src/managed_agents/types/tests.rs index 96ed556068..9473f52bab 100644 --- a/desktop/src-tauri/src/managed_agents/types/tests.rs +++ b/desktop/src-tauri/src/managed_agents/types/tests.rs @@ -53,6 +53,70 @@ fn managed_agent_record_without_auth_tag_deserializes() { assert_eq!(record.pubkey, "abcd1234"); } +/// Records written before the resume toggle lack `resume_on_restart`. The +/// serde default must land them ON, matching the harness default, so an +/// upgrade never silently disables resume for an existing agent. +#[test] +fn managed_agent_record_without_resume_on_restart_defaults_on() { + let record: ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "abcd1234", + "name": "test-agent", + "private_key_nsec": "nsec1fake", + "relay_url": "wss://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("pre-toggle agent record should deserialize"); + + assert!(record.resume_on_restart); +} + +/// An explicit `false` must survive a save/load cycle — the whole point of +/// the toggle is that an opted-out agent stays opted out across restarts. +#[test] +fn managed_agent_record_resume_on_restart_false_round_trips() { + let record: ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "abcd1234", + "name": "test-agent", + "private_key_nsec": "nsec1fake", + "relay_url": "wss://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "resume_on_restart": false, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("record with resume_on_restart should deserialize"); + assert!(!record.resume_on_restart); + + let serialized = serde_json::to_string(&record).expect("should serialize"); + let reloaded: ManagedAgentRecord = + serde_json::from_str(&serialized).expect("round-trip should deserialize"); + assert!(!reloaded.resume_on_restart); +} + /// Agent records WITH an auth_tag round-trip correctly through serde. #[test] fn managed_agent_record_with_auth_tag_round_trips() { diff --git a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx index 79d1e9a790..4447cbcb4f 100644 --- a/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx +++ b/desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx @@ -23,7 +23,10 @@ import { Button } from "@/shared/ui/button"; import { ChooserDialogContent } from "@/shared/ui/chooser-dialog-content"; import { Dialog } from "@/shared/ui/dialog"; import { Input } from "@/shared/ui/input"; -import { setManagedAgentAutoRestart } from "@/shared/api/tauriManagedAgents"; +import { + setManagedAgentAutoRestart, + setManagedAgentResumeOnRestart, +} from "@/shared/api/tauriManagedAgents"; import { EditAgentAdvancedFields } from "./EditAgentAdvancedFields"; import { AUTO_PROVIDER_DROPDOWN_VALUE, @@ -144,6 +147,9 @@ export function AgentInstanceEditDialog({ const [envVars, setEnvVars] = React.useState(agent.envVars); const [autoRestartOnConfigChange, setAutoRestartOnConfigChange] = React.useState(agent.autoRestartOnConfigChange); + const [resumeOnRestart, setResumeOnRestart] = React.useState( + agent.resumeOnRestart, + ); const personasQuery = usePersonasQuery(); const linkedPersona = React.useMemo( () => @@ -193,6 +199,7 @@ export function AgentInstanceEditDialog({ setIsCustomProviderEditing(false); setEnvVars(agent.envVars); setAutoRestartOnConfigChange(agent.autoRestartOnConfigChange); + setResumeOnRestart(agent.resumeOnRestart); setRespondTo(agent.respondTo); setRespondToAllowlist(agent.respondToAllowlist); setAvatarUrl(agent.avatarUrl ?? ""); @@ -267,15 +274,14 @@ export function AgentInstanceEditDialog({ }, [runtimes, originalAgentCommand]); // The runtime id that will actually be active after submit. When inheriting, - // resolve from the LINKED PERSONA's runtime — that is what will run once the - // override is cleared. Deriving from agent.agentCommand here is wrong for a - // pinned agent that just toggled "Inherit runtime from template": the override - // (e.g. a Claude pin) is still present on the record, so it would resolve to - // the old pin instead of the persona's runtime, hiding required credentials. - // Fall back to the agent.agentCommand dual-match (command path, then id) only - // when there is no linked persona or its runtime is unset. This single - // prospective id feeds BOTH the block-save gate (requiredEnvKeys) and the - // submit path so they never disagree on which runtime is being saved. + // resolve from the LINKED PERSONA's runtime — that is what runs once the + // override is cleared. Deriving from agent.agentCommand is wrong for a pinned + // agent that just toggled "Inherit runtime from template": the override (e.g. + // a Claude pin) still sits on the record, so it would resolve to the old pin + // and hide required credentials. Fall back to the agent.agentCommand + // dual-match (command path, then id) only when there is no linked persona or + // its runtime is unset. This single id feeds BOTH the block-save gate + // (requiredEnvKeys) and the submit path, so they never disagree. const prospectiveRuntimeId = React.useMemo(() => { if (!inheritHarness) { return selectedRuntime?.id ?? selectedRuntimeId; @@ -378,10 +384,9 @@ export function AgentInstanceEditDialog({ // Runtime/provider-required credential state, derived from the PROSPECTIVE // post-submit runtime — see the hook for the inherit-transition rationale. - // Pass globalProvider so the hook uses it as a fallback when the per-agent - // provider is empty (global-provider-only configs must surface required keys). - // Pass globalEnvVars so keys satisfied by global config are excluded from - // requiredEnvKeys and do not block Save (display and gate agree). + // globalProvider is the fallback when the per-agent provider is empty, so + // global-provider-only configs still surface their required keys; keys + // satisfied by globalEnvVars are excluded so display and the Save gate agree. const { requiredEnvKeys, fileSatisfiedEnvKeys, requiredEnvKeyMissing } = useRequiredCredentialState({ open, @@ -396,11 +401,10 @@ export function AgentInstanceEditDialog({ const { data: bakedEnvKeys } = useBakedBuildEnvKeysQuery({ enabled: open }); // Merge global env as the base layer so credential keys satisfied via global - // config (e.g. ANTHROPIC_API_KEY) are available to model discovery. Use - // `inheritedSubmission.envVars` (the same snapshot the credential gate - // validates) rather than raw `envVars`, so an inherit-transition that layers - // in persona env vars is reflected in discovery. Agent-local env takes - // precedence, matching the agent → global → file spawn-path precedence. + // config (e.g. ANTHROPIC_API_KEY) reach model discovery. The source is + // `inheritedSubmission.envVars` (the snapshot the credential gate validates) + // rather than raw `envVars`, so an inherit-transition's persona env vars are + // reflected too. Agent-local env wins, matching agent → global → file. const envVarsForDiscovery = React.useMemo( () => ({ ...globalConfig.env_vars, ...inheritedSubmission.envVars }), [globalConfig.env_vars, inheritedSubmission.envVars], @@ -423,10 +427,9 @@ export function AgentInstanceEditDialog({ selectedRuntime, }); - // D2: derive advancedRequiredEnvKeys for EnvVarsEditor display. - // The full requiredEnvKeys/requiredEnvKeyMissing continue driving Save gating. - // D2/D3: the top-level API key owns display, while the readiness gate keeps - // the complete required-key list. The effective snapshot covers persona + // advancedRequiredEnvKeys drives EnvVarsEditor display only; the full + // requiredEnvKeys/requiredEnvKeyMissing keep driving Save gating. The + // top-level API key owns display, and the effective snapshot covers persona // inheritance during an instance inherit transition. const providerApiKeyEnvVar = getProviderApiKeyEnvVar(effectiveProvider); const personaSatisfied = @@ -512,15 +515,13 @@ export function AgentInstanceEditDialog({ const isCustomCommand = resolvedRuntimeId === "custom"; // Only pin the harness when the selection can actually supply a command: - // - "Custom command": the Advanced command input becomes editable, so the - // user provides the command. - // - a catalog entry with a concrete command: we set it below. - // A catalog entry with command:null (availability adapter_missing / - // not_installed) can't produce a runnable command — clearing inheritance - // there would omit agentCommand on Save (command unchanged) while the - // provider/model logic treats the new runtime as effective, so an inherited - // Claude agent could persist a Databricks provider while still running - // Claude. Keep inheriting in that case. + // "Custom command" makes the Advanced command input editable, and a catalog + // entry with a concrete command is set below. A catalog entry with + // command:null (availability adapter_missing / not_installed) can't produce + // a runnable command — clearing inheritance there would omit agentCommand + // on Save (command unchanged) while the provider/model logic treats the new + // runtime as effective, so an inherited Claude agent could persist a + // Databricks provider while still running Claude. Keep inheriting then. if (isCustomCommand || nextRuntime?.command) { setInheritHarness(false); } @@ -638,12 +639,12 @@ export function AgentInstanceEditDialog({ // Classify the effective post-submit runtime's provider capability as a // tri-state: "capable" persists the provider, "locked" clears it (only - // when we KNOW it's provider-locked, e.g. Claude), "unknown" OMITS it so a - // transient/custom state never becomes a destructive write. Resolved - // STATICALLY (by id) so a not-yet-loaded catalog can't misclassify a known - // runtime as "unknown" — see resolveRuntimeProviderCapability. The runtime - // id is the shared prospectiveRuntimeId, so submit and the block-save gate - // always agree on which runtime is being saved. + // when we KNOW it's provider-locked, e.g. Claude), "unknown" OMITS it so + // a transient/custom state never becomes a destructive write. Resolved + // STATICALLY (by id) so a not-yet-loaded catalog can't misclassify a + // known runtime as "unknown" — see resolveRuntimeProviderCapability. The + // shared prospectiveRuntimeId keeps submit and the block-save gate in + // agreement on which runtime is being saved. const providerRuntimeCapability = resolveRuntimeProviderCapability( prospectiveRuntimeId, runtimeSupportsLlmProviderSelection(prospectiveRuntimeId), @@ -712,12 +713,11 @@ export function AgentInstanceEditDialog({ ? undefined : submitEnvVars, respondTo: respondTo !== agent.respondTo ? respondTo : undefined, - // The allowlist is preserved across mode toggles in local UI state - // (so a user can flip away from allowlist and back without losing - // their entries), but we only send it on the wire when (a) it - // actually changed, AND (b) the saved mode will need it. Sending - // an allowlist while switching to a non-allowlist mode would be - // harmless server-side, but it's noise in the persisted record. + // The allowlist is preserved across mode toggles in local UI state (so + // a user can flip away from allowlist and back without losing entries), + // but it only goes on the wire when it actually changed AND the saved + // mode needs it. Sending it while switching to a non-allowlist mode is + // harmless server-side but noise in the persisted record. respondToAllowlist: respondTo === "allowlist" && respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",") @@ -734,13 +734,16 @@ export function AgentInstanceEditDialog({ autoRestartOnConfigChange, ); } + if (resumeOnRestart !== agent.resumeOnRestart) { + await setManagedAgentResumeOnRestart(agent.pubkey, resumeOnRestart); + } showAgentProfileSyncWarning(result.agent.name, result.profileSyncError); handleOpenChange(false); onUpdated?.(result.agent); // The auto-restart policy deliberately never fires for a stopped or - // failing agent (a broken agent must not auto-loop), so an edit meant - // to FIX one silently waits for a manual start. Offer that start - // explicitly instead of relying on the user to know the policy. + // failing agent (a broken agent must not auto-loop), so an edit meant to + // FIX one silently waits for a manual start. Offer that start explicitly + // instead of relying on the user to know the policy. if (!isManagedAgentActive(result.agent)) { const startedName = result.agent.name; toast(`${startedName} saved while stopped.`, { @@ -907,7 +910,6 @@ export function AgentInstanceEditDialog({ )}
- {/* Agent name */}
) : null} - {/* LLM provider */} {llmProviderFieldVisible ? (