From 9c5efca1aed2d13fd807eed1d6c67e27a03acdba Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Mon, 3 Aug 2026 11:25:13 -0400 Subject: [PATCH] feat(acp,desktop): resume interrupted turns after app restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Managed agents lost any in-flight or queued turn when the app restarted: the event queue lived only in memory, so a mention being worked on at shutdown was silently dropped and never answered. Adds `ledger.rs`, a durable pending-turn mirror written after every queue mutation, and a boot-recovery pipeline in `lib.rs`: staged load, membership gate, TTL expiry, chunked REST fetch with per-event signature/id/channel reconciliation, ordered queue import, unresolved ordering barrier, then one atomic commit. Recovered turns re-run whole (there is no mid-turn checkpoint) and carry a `[restart recovery]` marker so the agent re-reads the thread before redoing work. Exactly-once is enforced on two fronts: a suppression set keyed by event id prevents the boot fetch and live WebSocket delivery from both admitting the same event, and events whose REST fetch failed stay unresolved behind an ordering barrier with a real deadline — a dedicated timer arm in the main `select!` loop fires it even on a completely quiet harness, so the timeout is a bound rather than a hope. `queue.rs` gains payload-owning `complete_batch`, shared `enforce_cap`, `import_recovered`/`admit_recovered`, barrier registration and `dirty_channels` tracking; shutdown-drain now classifies results through `complete_batch` plus a ledger sync at both grace-period sites so a turn completed during drain is not resurrected on the next boot. Off-by-default paths are opt-out via `--resume-on-restart`/`--resume-ttl-secs`. Desktop: adds per-agent resume-on-restart toggle (Advanced settings), makes the toggle authoritative over env precedence (reserved key), and guarantees the ledger file is deleted on every boot where resume is off. Routing fix: handleEditAgent now opens the instance editor for started agents, definition editor for unstarted ones. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .github/workflows/ci.yml | 7 + Cargo.lock | 1 + crates/buzz-acp/Cargo.toml | 1 + crates/buzz-acp/src/config.rs | 84 +- crates/buzz-acp/src/ledger.rs | 857 +++++ crates/buzz-acp/src/lib.rs | 3059 +++++++++++++++-- crates/buzz-acp/src/pool.rs | 30 +- crates/buzz-acp/src/queue.rs | 1737 +++++++++- crates/buzz-acp/src/relay.rs | 2 +- crates/buzz-acp/tests/setup_mode_ledger.rs | 99 + .../src/commands/agent_config_tests.rs | 1 + .../src-tauri/src/commands/agent_settings.rs | 51 + desktop/src-tauri/src/commands/agents.rs | 6 +- .../src-tauri/src/commands/agents_tests.rs | 1 + .../commands/personas/delete_cascade_tests.rs | 1 + .../personas/inbound/inbound_tests.rs | 1 + .../personas/snapshot/fidelity_tests.rs | 1 + .../src/commands/personas/snapshot/import.rs | 1 + .../src/commands/personas/snapshot/tests.rs | 10 +- .../personas/update/name_propagation_tests.rs | 1 + .../src-tauri/src/commands/team_snapshot.rs | 1 + .../src/commands/team_snapshot/tests.rs | 1 + desktop/src-tauri/src/lib.rs | 4 +- .../src/managed_agents/agent_events.rs | 1 + .../src/managed_agents/agent_snapshot.rs | 3 +- .../managed_agents/agent_snapshot_envelope.rs | 1 + .../managed_agents/agent_snapshot_tests.rs | 1 + .../config_bridge/reader_tests.rs | 1 + .../src/managed_agents/discovery/tests.rs | 4 +- .../managed_agents/effective_config/tests.rs | 1 + .../src-tauri/src/managed_agents/env_vars.rs | 22 + .../src/managed_agents/env_vars/tests.rs | 86 +- .../src/managed_agents/global_config/tests.rs | 1 + .../src/managed_agents/nest/tests.rs | 1 + .../managed_agents/persona_events/tests.rs | 1 + .../src-tauri/src/managed_agents/readiness.rs | 4 +- .../src-tauri/src/managed_agents/runtime.rs | 62 +- .../src/managed_agents/runtime/tests.rs | 4 +- .../src/managed_agents/spawn_hash.rs | 1 + .../src/managed_agents/spawn_hash/tests.rs | 14 + .../src/managed_agents/team_snapshot.rs | 1 + .../src/managed_agents/teams_tests.rs | 1 + desktop/src-tauri/src/managed_agents/types.rs | 116 +- .../src/managed_agents/types/tests.rs | 64 + .../agents/ui/AgentInstanceEditDialog.tsx | 104 +- .../agents/ui/EditAgentAdvancedFields.tsx | 25 + .../features/profile/ui/UserProfilePanel.tsx | 23 +- desktop/src/shared/api/tauri.ts | 12 +- desktop/src/shared/api/tauriManagedAgents.ts | 14 + desktop/src/shared/api/types.ts | 17 +- desktop/src/testing/e2eBridge.ts | 19 + desktop/tests/e2e/edit-agent.spec.ts | 63 +- .../global-agent-config-screenshots.spec.ts | 16 + 53 files changed, 6089 insertions(+), 551 deletions(-) create mode 100644 crates/buzz-acp/src/ledger.rs create mode 100644 crates/buzz-acp/tests/setup_mode_ledger.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 60507182d5..52dc35de92 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -993,6 +993,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 937ead564a..18bbd15f0c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -818,6 +818,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 35aaec188d..5ba5e90e5f 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")] @@ -568,6 +591,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. @@ -1110,6 +1141,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) @@ -1131,7 +1165,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, @@ -1152,6 +1186,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, ) @@ -1481,6 +1517,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, } } @@ -2865,6 +2904,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 811253e4ac..048b7cd76a 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; @@ -1333,6 +1336,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, @@ -1577,6 +1597,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), @@ -1771,6 +1826,14 @@ 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, &mut last_activity) + { + 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 @@ -1841,12 +1904,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, &mut last_activity) + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx, &mut last_activity) { typing_channels.insert(channel_id, thread_tags); } + } else { + sync_dirty(&mut queue, &mut ledger); } } @@ -1881,7 +1949,7 @@ async fn tokio_main() -> Result<()> { // 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, &mut last_activity) + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx, &mut last_activity) { typing_channels.insert(channel_id, thread_tags); } @@ -2064,6 +2132,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); @@ -2091,6 +2163,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; } @@ -2266,17 +2342,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(); @@ -2284,14 +2371,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, @@ -2330,8 +2410,7 @@ async fn tokio_main() -> Result<()> { } if pool_ready { for (channel_id, thread_tags) in - dispatch_pending(&mut pool, &mut queue, &ctx, &mut last_activity) - { + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx, &mut last_activity) { typing_channels.insert(channel_id, thread_tags); } } @@ -2377,17 +2456,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, &mut last_activity) - { - 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, &mut last_activity) + { + 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 } @@ -2434,6 +2524,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, &mut last_activity) + { + typing_channels.insert(channel_id, thread_tags); + } + } + None + } _ = shutdown_rx.changed() => { tracing::info!("shutting down"); break; @@ -2461,6 +2580,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( @@ -2476,10 +2598,13 @@ 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, &mut last_activity) + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx, &mut last_activity) { typing_channels.insert(channel_id, thread_tags); } @@ -2501,10 +2626,13 @@ 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, &mut last_activity) + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx, &mut last_activity) { typing_channels.insert(channel_id, thread_tags); } @@ -2648,7 +2776,7 @@ async fn tokio_main() -> Result<()> { // 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, &mut last_activity) + dispatch_pending(&mut pool, &mut queue, &mut ledger, &ctx, &mut last_activity) { typing_channels.insert(channel_id, thread_tags); } @@ -2675,9 +2803,13 @@ async fn tokio_main() -> Result<()> { "ready", None, ); - for (channel_id, thread_tags) in - dispatch_pending(&mut pool, &mut queue, &ctx, &mut last_activity) - { + for (channel_id, thread_tags) in dispatch_pending( + &mut pool, + &mut queue, + &mut ledger, + &ctx, + &mut last_activity, + ) { typing_channels.insert(channel_id, thread_tags); } } @@ -2726,42 +2858,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"); } @@ -2846,6 +2964,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 @@ -2947,6 +3126,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}"); @@ -3003,12 +3189,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, last_activity: &mut tokio::time::Instant, ) -> Vec<(Uuid, ThreadTags)> { @@ -3030,8 +3438,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; } }; @@ -3092,6 +3503,7 @@ fn dispatch_pending( dispatched_channels.push((channel_id, typing_scope)); *last_activity = tokio::time::Instant::now(); } + sync_dirty(queue, ledger); tracing::debug!( dispatched = dispatched_channels.len(), queue_depth = queue.pending_channels(), @@ -3152,151 +3564,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); } @@ -3541,25 +4073,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 { @@ -5137,6 +5665,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, } } @@ -5359,6 +5890,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, } } @@ -5471,6 +6005,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); @@ -5653,11 +6278,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, } @@ -5763,6 +6388,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, @@ -5881,6 +6510,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, @@ -5974,6 +6607,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, @@ -6048,11 +6685,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), }; @@ -6076,12 +6713,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(); @@ -6373,6 +7010,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, @@ -6458,6 +7099,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, @@ -6777,3 +7422,1955 @@ 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, + exit_after_inactivity_secs: 0, + 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, + &mut tokio::time::Instant::now(), + ); + 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, + &mut tokio::time::Instant::now(), + ); + 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, + &mut tokio::time::Instant::now(), + ); + + 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, + &mut tokio::time::Instant::now(), + ); + + 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 64edf68ee2..1a8811f094 100644 --- a/crates/buzz-acp/src/pool.rs +++ b/crates/buzz-acp/src/pool.rs @@ -709,6 +709,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 { @@ -5054,11 +5068,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, }; @@ -5380,11 +5394,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 5c960de202..5ec205ba5b 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 } @@ -692,6 +1363,7 @@ impl EventQueue { .entry(channel_id) .or_default() .push(qe); + self.dirty_channels.insert(channel_id); true } @@ -724,14 +1396,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 @@ -753,6 +1419,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 @@ -777,14 +1444,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, @@ -967,6 +1627,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) } @@ -1497,6 +2164,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 @@ -1647,22 +2344,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. @@ -1677,12 +2374,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 { @@ -1877,6 +2569,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, @@ -1897,6 +2593,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 { @@ -1907,11 +2741,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, } @@ -2039,17 +2881,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), }; @@ -2095,11 +2949,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), }; @@ -2188,16 +3050,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![], @@ -2227,6 +3101,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, @@ -2250,6 +3128,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, @@ -2282,6 +3164,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, @@ -2312,6 +3198,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, @@ -2339,6 +3229,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, @@ -2363,6 +3257,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, @@ -2419,6 +3317,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, @@ -2457,6 +3359,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, @@ -2686,6 +3592,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"); @@ -2974,6 +3884,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, @@ -3005,6 +3919,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, @@ -3043,6 +3961,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, @@ -3071,6 +3993,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, @@ -3115,6 +4041,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, @@ -3164,6 +4094,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, @@ -3371,6 +4305,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, @@ -3428,6 +4366,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, @@ -3468,6 +4410,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, @@ -3492,6 +4438,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, @@ -3515,6 +4465,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, @@ -3597,6 +4551,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); @@ -3881,6 +4857,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, @@ -3923,6 +4903,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, @@ -3957,6 +4941,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, @@ -3986,6 +4974,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, @@ -4028,6 +5020,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, @@ -4064,6 +5060,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, @@ -4100,11 +5100,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![], @@ -4137,11 +5145,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![], @@ -4169,6 +5185,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, @@ -4246,6 +5266,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); @@ -4255,6 +5279,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); @@ -4263,6 +5291,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 ─────────────────────────────────── @@ -4463,6 +5503,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, @@ -4492,6 +5536,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, @@ -4520,6 +5568,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, @@ -4761,4 +5813,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_tests.rs b/desktop/src-tauri/src/commands/agent_config_tests.rs index f3667cff45..4452e65d30 100644 --- a/desktop/src-tauri/src/commands/agent_config_tests.rs +++ b/desktop/src-tauri/src/commands/agent_config_tests.rs @@ -85,6 +85,7 @@ fn agent_record() -> ManagedAgentRecord { 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 3b114b0474..dc116d0209 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 df135298c4..3c218c6229 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 b769d74d7b..dd1710ed78 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 d7f0323304..1070833dfc 100644 --- a/desktop/src-tauri/src/commands/personas/snapshot/import.rs +++ b/desktop/src-tauri/src/commands/personas/snapshot/import.rs @@ -623,6 +623,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 c453b09a9d..b7913ae123 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 c4b733e3e0..b505fca42e 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -458,8 +458,7 @@ pub fn run() { } // Store the AppHandle so huddle commands can emit `huddle-state-changed` - // events via `huddle::emit_huddle_state` without threading the handle - // through every call site. + // events via `huddle::emit_huddle_state` without threading every call site. if let Ok(mut guard) = state.app_handle.lock() { *guard = Some(app_handle.clone()); } @@ -810,6 +809,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 7c08e7095f..9619256c87 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() diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs index 8508c27073..a0ca5b8073 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs @@ -386,6 +386,7 @@ mod tests { 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::types::BackendKind::Local, backend_agent_id: None, diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs index b4492418e5..5450b63fe9 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs @@ -40,6 +40,7 @@ fn minimal_record() -> ManagedAgentRecord { }, 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 153db1bbd8..4bf32b6d31 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 1653371e7f..585cf6f2a4 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -81,6 +81,12 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ // desktop/provider-owned bounds while the saved record still promises them. "BUZZ_ACP_EXIT_AFTER_INACTIVITY", "BUZZ_ACP_NO_PRESENCE", + // 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. @@ -311,5 +317,21 @@ pub(crate) fn live_persona_env( .unwrap_or_default() } +/// 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 534c2e0835..abc21aced7 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, }; @@ -497,3 +497,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 3173126b90..995c264611 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -149,13 +149,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 @@ -242,7 +242,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))) @@ -334,6 +333,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(), @@ -602,12 +602,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": "...", ... }] } @@ -698,9 +694,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. @@ -725,6 +720,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"); @@ -748,10 +745,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; @@ -815,13 +812,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") { @@ -854,8 +851,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); } @@ -921,11 +918,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 48cde58aa7..1aaed407fd 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, @@ -147,6 +150,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( () => @@ -196,6 +202,7 @@ export function AgentInstanceEditDialog({ setIsCustomProviderEditing(false); setEnvVars(agent.envVars); setAutoRestartOnConfigChange(agent.autoRestartOnConfigChange); + setResumeOnRestart(agent.resumeOnRestart); setRespondTo(agent.respondTo); setRespondToAllowlist(agent.respondToAllowlist); setAvatarUrl(agent.avatarUrl ?? ""); @@ -270,15 +277,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; @@ -381,10 +387,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, @@ -399,11 +404,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], @@ -426,10 +430,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 = @@ -515,15 +518,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); } @@ -641,12 +642,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), @@ -715,12 +716,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(",") @@ -737,13 +737,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.`, { @@ -910,7 +913,6 @@ export function AgentInstanceEditDialog({ )}
- {/* Agent name */}
) : null} - {/* LLM provider */} {llmProviderFieldVisible ? (