diff --git a/docs/web-ui/agent.md b/docs/web-ui/agent.md index 63f5b708..ba47e4cc 100644 --- a/docs/web-ui/agent.md +++ b/docs/web-ui/agent.md @@ -236,7 +236,8 @@ Slash commands today: `Bus::request_compact()` flag and returns immediately. The harness runs the `/compact` at the next turn boundary (end of the in-flight turn, or `turn::run_pending_compact` when idle); output streams into - the live panel. Works mid-turn, not only when idle. + the live panel. Works mid-turn, not only when idle. Either way the live + panel ends with a `/compact done` or `/compact failed: …` note. - `/model ` — `POST /api/model` flipping `Bus::set_model`. Takes effect on the next turn; persisted to `/state/hyperhive-model` so the override survives harness diff --git a/hive-agent/src/paths.rs b/hive-agent/src/paths.rs index fdc4e968..dc4ceddc 100644 --- a/hive-agent/src/paths.rs +++ b/hive-agent/src/paths.rs @@ -131,3 +131,38 @@ pub fn claude_dir() -> PathBuf { } PathBuf::from("/root/.claude") } + +/// Run `f` with `HYPERHIVE_HARNESS_DIR` pointing at a fresh temp dir, then +/// restore the previous value. Tests that build a `Bus` or read the harness +/// state files go through this: the variable is process-wide, so the calls +/// are serialised on a lock. +#[cfg(test)] +pub fn with_harness_dir(f: impl FnOnce() -> T) -> T { + use std::ffi::OsString; + use std::sync::Mutex; + + static LOCK: Mutex<()> = Mutex::new(()); + + struct Restore(Option); + impl Drop for Restore { + fn drop(&mut self) { + // SAFETY: still holding `LOCK`, the only writer of this variable. + unsafe { + match self.0.take() { + Some(v) => std::env::set_var("HYPERHIVE_HARNESS_DIR", v), + None => std::env::remove_var("HYPERHIVE_HARNESS_DIR"), + } + } + } + } + + let _lock = LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let dir = tempfile::tempdir().unwrap(); + // Declared after `_lock` so the variable is restored before the lock drops. + let _restore = Restore(std::env::var_os("HYPERHIVE_HARNESS_DIR")); + // SAFETY: serialised by `LOCK`. + unsafe { std::env::set_var("HYPERHIVE_HARNESS_DIR", dir.path()) }; + f() +} diff --git a/hive-agent/src/swarm_inference.rs b/hive-agent/src/swarm_inference.rs index 3be47c2d..eebb3d50 100644 --- a/hive-agent/src/swarm_inference.rs +++ b/hive-agent/src/swarm_inference.rs @@ -320,34 +320,29 @@ mod tests { /// persisted files, so clearing it, or a restart, finds the local value. #[test] fn the_swarm_value_is_not_persisted_over_the_local_one() { - let dir = tempfile::tempdir().unwrap(); - let prev = std::env::var_os("HYPERHIVE_HARNESS_DIR"); - // SAFETY: no other test in this crate reads or sets this variable. - unsafe { std::env::set_var("HYPERHIVE_HARNESS_DIR", dir.path()) }; - let bus = Bus::new(); - bus.set_model("sonnet"); - bus.set_effort("low"); - let target = Harness { - bus: bus.clone(), - nix_model: None, - nix_effort: None, - }; - let mut o = Overridden::default(); - o.apply(&set(Some("opus"), Some(Effort::Max)), &target); - let swarm = (bus.model(), bus.effort()); - let swarm_sources = (bus.model_source(), bus.effort_source()); - let files = ( - crate::harness_state::load_model(), - crate::harness_state::load_effort(), - ); - o.apply(&AgentInference::default(), &target); - let cleared = (bus.model(), bus.effort()); - let cleared_sources = (bus.model_source(), bus.effort_source()); - match prev { - // SAFETY: as above. - Some(v) => unsafe { std::env::set_var("HYPERHIVE_HARNESS_DIR", v) }, - None => unsafe { std::env::remove_var("HYPERHIVE_HARNESS_DIR") }, - } + let (swarm, swarm_sources, files, cleared, cleared_sources) = + crate::paths::with_harness_dir(|| { + let bus = Bus::new(); + bus.set_model("sonnet"); + bus.set_effort("low"); + let target = Harness { + bus: bus.clone(), + nix_model: None, + nix_effort: None, + }; + let mut o = Overridden::default(); + o.apply(&set(Some("opus"), Some(Effort::Max)), &target); + let swarm = (bus.model(), bus.effort()); + let swarm_sources = (bus.model_source(), bus.effort_source()); + let files = ( + crate::harness_state::load_model(), + crate::harness_state::load_effort(), + ); + o.apply(&AgentInference::default(), &target); + let cleared = (bus.model(), bus.effort()); + let cleared_sources = (bus.model_source(), bus.effort_source()); + (swarm, swarm_sources, files, cleared, cleared_sources) + }); assert_eq!(swarm, pair("opus", "max")); assert_eq!(files, (Some("sonnet".to_owned()), Some("low".to_owned()))); assert_eq!(cleared, pair("sonnet", "low")); diff --git a/hive-agent/src/turn.rs b/hive-agent/src/turn.rs index 3bcc338f..156e83ef 100644 --- a/hive-agent/src/turn.rs +++ b/hive-agent/src/turn.rs @@ -155,7 +155,7 @@ pub async fn write_system_prompt(socket: &Path, label: &str) -> Result /// One claude turn's outcome: `Ok(compacted)` on success, or a [`TurnError`] /// the serve loop must act on. The `compacted` bool is `true` when a /// compaction ran this turn (reactively on overflow, or proactively per the -/// policy — or an operator `/compact` at turn end); it's recorded as +/// policy — or a successful operator `/compact` at turn end); it's recorded as /// `result_kind = "compacted"` in turn stats so the stats page can distinguish /// those turns. Both `Ok(true)` and `Ok(false)` are ack'd; the error cases /// each map to a distinct serve-loop action (see [`emit_turn_end`] and the @@ -461,7 +461,7 @@ pub async fn drive_turn( // Reflect `Compacting` in the UI like the idle path (`run_pending_compact`) // does; the serve loop resets to `Idle` once this turn returns. bus.set_state(crate::events::TurnState::Compacting); - let _ = session.compact(&config, &sink).await; + let compact_ok = report_compact(bus, session.compact(&config, &sink).await); // If the compact call asked to be woken (the agent's own `compact` // tool with a `wake_prompt`), stash it — the serve loop reads it // back after this turn returns and drives a synthetic follow-up @@ -470,11 +470,38 @@ pub async fn drive_turn( if let Some(prompt) = request.wake_prompt { bus.set_post_compact_wake(prompt); } - return Ok(true); + return with_turn_end_compact(outcome, compact_ok); } outcome } +/// Emit the `/compact done` / `/compact failed: …` note for a finished +/// operator or agent `/compact` and return whether it succeeded. Shared by the +/// turn-end path in [`drive_turn`] and the idle path in [`run_pending_compact`]. +fn report_compact(bus: &Bus, result: Result<(), E>) -> bool { + match result { + Ok(()) => { + bus.emit(LiveEvent::Note { + text: "/compact done".into(), + }); + true + } + Err(e) => { + bus.emit(LiveEvent::Note { + text: format!("/compact failed: {e}"), + }); + false + } + } +} + +/// Fold a turn-end `/compact` into the turn's own outcome. A compaction that +/// ran during the turn (`Ok(true)`) stays recorded even when the turn-end +/// `/compact` failed; a failed `/compact` never turns it into `Ok(false)`. +fn with_turn_end_compact(outcome: TurnOutcome, compact_ok: bool) -> TurnOutcome { + outcome.map(|turn_compacted| turn_compacted || compact_ok) +} + /// Pre-turn auto-reset check. If context is large AND the prompt cache has /// gone cold (idle time >= cache TTL), archive the current session so the /// next wake-up turn's `--resume ` misses and self-heals into a fresh @@ -600,14 +627,7 @@ pub async fn run_pending_compact(files: &TurnFiles, bus: &Bus, session: &AgentSe bus.set_state(crate::events::TurnState::Compacting); let config = claude_config(bus, files); let sink = BusSink::new(bus); - match session.compact(&config, &sink).await { - Ok(()) => bus.emit(LiveEvent::Note { - text: "/compact done".into(), - }), - Err(e) => bus.emit(LiveEvent::Note { - text: format!("/compact failed: {e}"), - }), - } + report_compact(bus, session.compact(&config, &sink).await); bus.set_state(crate::events::TurnState::Idle); if let Some(prompt) = request.wake_prompt { bus.set_post_compact_wake(prompt); @@ -821,8 +841,10 @@ fn archive_session(bus: &Bus, session: &AgentSession) { #[cfg(test)] mod tests { use super::{ - PermissionAsk, TurnError, acp_permits, nonzero_or, parse_u64, runtime_error_to_turn, + PermissionAsk, TurnError, acp_permits, nonzero_or, parse_u64, report_compact, + runtime_error_to_turn, with_turn_end_compact, }; + use crate::events::{Bus, LiveEvent}; use hive_runtime::{AcpError, Error}; #[test] @@ -959,4 +981,49 @@ mod tests { Err(TurnError::Failed(_)) )); } + + fn compact_report(result: Result<(), &str>) -> (bool, Vec<String>) { + crate::paths::with_harness_dir(|| { + let bus = Bus::new(); + let mut rx = bus.subscribe(); + let ok = report_compact(&bus, result); + let mut notes = Vec::new(); + while let Ok(ev) = rx.try_recv() { + if let LiveEvent::Note { text } = ev.event { + notes.push(text); + } + } + (ok, notes) + }) + } + + #[test] + fn a_failed_compact_is_reported_and_not_counted_as_compacted() { + let (compacted, notes) = compact_report(Err("spawn failed")); + assert!(!compacted); + assert_eq!(notes, ["/compact failed: spawn failed"]); + } + + #[test] + fn a_successful_compact_is_reported_and_counted_as_compacted() { + let (compacted, notes) = compact_report(Ok(())); + assert!(compacted); + assert_eq!(notes, ["/compact done"]); + } + + #[test] + fn a_failed_turn_end_compact_keeps_the_turns_own_compaction() { + assert!(matches!(with_turn_end_compact(Ok(true), false), Ok(true))); + } + + #[test] + fn a_failed_turn_end_compact_leaves_a_plain_turn_uncompacted() { + assert!(matches!(with_turn_end_compact(Ok(false), false), Ok(false))); + } + + #[test] + fn a_successful_turn_end_compact_marks_the_turn_compacted() { + assert!(matches!(with_turn_end_compact(Ok(false), true), Ok(true))); + assert!(matches!(with_turn_end_compact(Ok(true), true), Ok(true))); + } }