Fix 24 correctness, robustness, and security issues across the core crates

crates/hi-agent/src/agent/compaction_turn.rs

@@ -436,7 +436,7 @@ impl crate::Agent {
                 return Err(err);
             }
         };
-        self.add_usage(completion.usage);
+        self.add_side_usage(completion.usage);
         let _ = self.persist();
         self.emit_usage(ui);
 

crates/hi-agent/src/agent/curate_turn.rs

@@ -96,7 +96,7 @@ impl crate::Agent {
                 return;
             }
         };
-        self.add_usage(completion.usage);
+        self.add_side_usage(completion.usage);
         let _ = self.persist();
         if out.trim().is_empty() {
             for c in &completion.content {

crates/hi-agent/src/agent/explore_turn.rs

@@ -93,7 +93,7 @@ impl crate::Agent {
             }
         };
         // Fold the child's token usage into the parent's session totals.
-        self.add_usage(*child.totals());
+        self.add_side_usage(*child.totals());
         ui.subagent_note(&format!("↳ explore subagent {n} done"));
         result
     }

crates/hi-agent/src/agent/lifecycle.rs

@@ -455,15 +455,27 @@ impl crate::Agent {
         out
     }
 
+    /// Usage from a *main-conversation* model call: counts toward all totals
+    /// and refreshes the context gauge with the request's occupancy.
     pub(crate) fn add_usage(&mut self, usage: Usage) {
-        self.totals.add(usage);
-        self.last_turn_usage.add(usage);
+        self.add_side_usage(usage);
         let effective_input = usage.effective_input_tokens();
         if effective_input > 0 {
             self.context_used = effective_input;
         }
     }
 
+    /// Usage from a *side* model call (finalize, skeptic, curate, memory,
+    /// goal planning, explore children, compaction summarize): counts toward
+    /// totals and the turn's spend, but must not touch `context_used` — that
+    /// gauge tracks the main conversation's occupancy and drives
+    /// auto-compaction, and a ~3K-token side request at the end of a 150K
+    /// session would reset it to 2%, silently disabling the next compaction.
+    pub(crate) fn add_side_usage(&mut self, usage: Usage) {
+        self.totals.add(usage);
+        self.last_turn_usage.add(usage);
+    }
+
     pub(crate) fn reset_last_turn_usage(&mut self, user_prompt_tokens: u64) {
         self.last_turn_usage = Usage::default();
         self.last_user_prompt_tokens = user_prompt_tokens;

crates/hi-agent/src/agent/memory_turn.rs

@@ -115,7 +115,7 @@ impl crate::Agent {
                 return;
             }
         };
-        self.add_usage(completion.usage);
+        self.add_side_usage(completion.usage);
         let _ = self.persist();
 
         // Fall back to the final content if the provider didn't stream text.

crates/hi-agent/src/agent/plan_goal.rs

@@ -65,7 +65,7 @@ impl crate::Agent {
                 return Err(err);
             }
         };
-        self.add_usage(completion.usage);
+        self.add_side_usage(completion.usage);
         // Fall back to the completion content if the provider returned text only in
         // the final object rather than via stream deltas.
         if text.trim().is_empty() {

crates/hi-agent/src/agent/skeptic.rs

@@ -157,7 +157,7 @@ impl crate::Agent {
                 return SkepticVerdict::Approve; // fail-open on a provider error
             }
         };
-        self.add_usage(completion.usage);
+        self.add_side_usage(completion.usage);
         if text.trim().is_empty() {
             text = content_text(&completion.content);
         }

crates/hi-agent/src/agent/turn.rs

@@ -2384,11 +2384,14 @@ If the task is already complete, stop and give your final recap."
                 let mut completed = vec![false; calls.len()];
                 let mut completion_order: Vec<usize> = Vec::with_capacity(calls.len());
                 let mut scheduler_forced_skip = false;
-                // Pre-pass: handle `record_decision` calls serially. They mutate
-                // agent state (`self.decisions`) and aren't real tool dispatches,
-                // so they can't run in the parallel `execute` stream (no `&mut
-                // self` there). They're instantaneous and have no deps that
-                // matter, so handling them up front is safe.
+                // Pre-pass: resolve calls blocked by read-only intent up front.
+                // They produce instant synthetic error results and mutate
+                // nothing, so completing them out of dep order is safe.
+                // (`explore`/`delegate`/`record_decision` used to run here too,
+                // but they *do* have deps that matter — running a subagent
+                // before an earlier `write` in the same batch handed it a stale
+                // tree — so they now dispatch inside the dep-aware scheduler
+                // loop below.)
                 for (i, (id, name, arguments)) in calls.iter().enumerate() {
                     if read_only_blocks_tool(read_only_intent, name) {
                         ui.tool_call(name, arguments);
@@ -2405,68 +2408,7 @@ If the task is already complete, stop and give your final recap."
                         results[i] = Some((id.clone(), content));
                         completed[i] = true;
                         completion_order.push(i);
-                        continue;
-                    }
-                    if name == "explore" {
-                        // Read-only subagent: runs a bounded child turn (own context,
-                        // read-only tools) sharing this agent's provider, then returns a
-                        // concise answer. Handled here (not in the parallel `execute`
-                        // stream) because it needs `&mut self` and is itself an agent turn.
-                        ui.tool_call(name, arguments);
-                        let content = self.handle_explore(arguments, &mut *ui).await;
-                        emit_tool_output(
-                            &mut *ui,
-                            name,
-                            &hi_tools::ToolOutput {
-                                content: content.clone(),
-                                display: None,
-                                plan: None,
-                            },
-                        );
-                        results[i] = Some((id.clone(), content));
-                        completed[i] = true;
-                        completion_order.push(i);
-                        continue;
                     }
-                    if name == "delegate" {
-                        // Write-capable subagent: runs a child turn that edits + verifies
-                        // in the same tree, checkpoint-guarded so a failed delegation rolls
-                        // back. Like `explore`, handled here because it needs `&mut self`
-                        // and is itself an agent turn.
-                        //
-                        // Capture the turn baseline + checkpoint BEFORE it mutates the
-                        // tree — otherwise the later lazy snapshot (verify gate) would
-                        // record delegate's own output as the baseline, making the
-                        // parent's verify + changed-files see "no changes", and leaving
-                        // no pre-delegate checkpoint for `/undo` to isolate this turn.
-                        self.ensure_turn_snapshot(&mut turn_snapshot).await;
-                        self.ensure_turn_checkpoint(&mut turn_checkpoint_created, ui)
-                            .await;
-                        ui.tool_call(name, arguments);
-                        let content = self.handle_delegate(arguments, &mut *ui).await;
-                        emit_tool_output(
-                            &mut *ui,
-                            name,
-                            &hi_tools::ToolOutput {
-                                content: content.clone(),
-                                display: None,
-                                plan: None,
-                            },
-                        );
-                        results[i] = Some((id.clone(), content));
-                        completed[i] = true;
-                        completion_order.push(i);
-                        continue;
-                    }
-                    if name != "record_decision" {
-                        continue;
-                    }
-                    ui.tool_call(name, arguments);
-                    let content = self.handle_record_decision(arguments);
-                    ui.tool_result(name, &content);
-                    results[i] = Some((id.clone(), content));
-                    completed[i] = true;
-                    completion_order.push(i);
                 }
                 let mut done = completion_order.len();
                 // Proactive per-edit checks: kicked off in the background as
@@ -2618,6 +2560,62 @@ If the task is already complete, stop and give your final recap."
                         sched_max_concurrent = sched_max_concurrent.max(1);
                         continue;
                     }
+                    // Self-dispatched calls: `explore`/`delegate` run a child
+                    // agent turn and `record_decision` mutates agent state, so
+                    // all three need `&mut self` and can't join the parallel
+                    // `execute` stream. Run one alone when it's ready — the dep
+                    // graph then guarantees earlier mutations in the batch have
+                    // landed before a subagent sees the tree.
+                    let self_idx = ready.iter().copied().find(|&i| {
+                        matches!(
+                            calls[i].1.as_str(),
+                            "explore" | "delegate" | "record_decision"
+                        )
+                    });
+                    if let Some(i) = self_idx {
+                        let (id, name, arguments) = &calls[i];
+                        if name == "delegate" {
+                            // Write-capable subagent: capture the turn baseline +
+                            // checkpoint BEFORE it mutates the tree — otherwise the
+                            // later lazy snapshot (verify gate) would record
+                            // delegate's own output as the baseline, making the
+                            // parent's verify + changed-files see "no changes", and
+                            // leaving no pre-delegate checkpoint for `/undo` to
+                            // isolate this turn.
+                            self.ensure_turn_snapshot(&mut turn_snapshot).await;
+                            self.ensure_turn_checkpoint(&mut turn_checkpoint_created, ui)
+                                .await;
+                        }
+                        ui.tool_call(name, arguments);
+                        let content = match name.as_str() {
+                            "explore" => self.handle_explore(arguments, &mut *ui).await,
+                            "delegate" => self.handle_delegate(arguments, &mut *ui).await,
+                            _ => self.handle_record_decision(arguments),
+                        };
+                        emit_tool_output(
+                            &mut *ui,
+                            name,
+                            &hi_tools::ToolOutput {
+                                content: content.clone(),
+                                display: None,
+                                plan: None,
+                            },
+                        );
+                        results[i] = Some((id.clone(), content));
+                        if name == "delegate" {
+                            // The child turn edits the same tree — a dependent
+                            // read after it must re-walk.
+                            self.invalidate_snapshot();
+                        }
+                        completed[i] = true;
+                        completion_order.push(i);
+                        done += 1;
+                        // Runs alone, like bash.
+                        sched_tool_calls += 1;
+                        sched_serial_runs += 1;
+                        sched_max_concurrent = sched_max_concurrent.max(1);
+                        continue;
+                    }
                     // Run all ready non-bash calls concurrently. Record the
                     // completion order as the ready order (within a concurrent
                     // batch, relative order doesn't matter — none depend on
@@ -2824,6 +2822,12 @@ If the task is already complete, stop and give your final recap."
                         done += 1;
                     }
                 }
+                // Consume an interrupt that landed while (or just after) the
+                // batch's last call finished — the loop above only polls the
+                // flag between rounds, so a leftover flag would spuriously
+                // cancel the next round's (or even the next turn's) batch.
+                self.interrupt
+                    .store(false, std::sync::atomic::Ordering::Relaxed);
                 debug_assert_eq!(
                     done,
                     calls.len(),
@@ -3221,7 +3225,9 @@ If the task is already complete, stop and give your final recap."
             }
         };
 
-        self.add_usage(completion.usage);
+        // Side call: spend counts, but its small request must not clobber the
+        // main conversation's context gauge (see add_side_usage).
+        self.add_side_usage(completion.usage);
         self.emit_usage(ui);
 
         // Fall back to the final content if the provider didn't stream text.

crates/hi-agent/src/goal.rs

@@ -282,7 +282,11 @@ impl Goal {
                 .any(|s| s.status == GoalStatus::Active)
         {
             self.status = GoalStatus::Failed;
-        } else if self.status == GoalStatus::Done {
+        } else {
+            // Per the contract above: not all done, not failed-with-no-active
+            // → Active. This must also revive a previously `Failed` goal whose
+            // plan was revised to activate new work — leaving it `Failed`
+            // would permanently disable auto-drive despite live sub-goals.
             self.status = GoalStatus::Active;
         }
     }

crates/hi-agent/src/heuristics.rs

@@ -271,15 +271,18 @@ fn strip_chatml_tokens(text: &str) -> String {
     let mut last = 0;
     let mut i = 0;
     while i < bytes.len() {
+        // Search the suffix from after the `<|` prefix so the prefix's own `|`
+        // can't match `|>` (the text `<|>` produced a reversed slice range and
+        // panicked).
         if bytes[i] == b'<'
             && i + 1 < bytes.len()
             && bytes[i + 1] == b'|'
-            && let Some(end) = text[i..].find("|>")
+            && let Some(end) = text[i + 2..].find("|>")
         {
-            let inner = &text[i + 2..i + end];
+            let inner = &text[i + 2..i + 2 + end];
             if !inner.is_empty() && inner.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') {
                 out.push_str(&text[last..i]);
-                last = i + end + 2;
+                last = i + 2 + end + 2;
                 i = last;
                 continue;
             }

crates/hi-agent/src/memory.rs

@@ -339,6 +339,11 @@ fn looks_like_path_or_command(bullet: &str) -> Option<String> {
     for word in body.split_whitespace() {
         let word =
             word.trim_matches(|c: char| !c.is_alphanumeric() && c != '/' && c != '.' && c != '-');
+        // A trailing '.' is sentence punctuation, not part of a path: without
+        // this, "src/main.rs." never exists and any prose word ending a
+        // sentence ("messages.") reads as having an (empty) extension — either
+        // way the bullet gets dropped as an implausible path.
+        let word = word.trim_end_matches('.');
         if word.contains('/') || has_extension(word) {
             return Some(word.to_string());
         }
@@ -347,9 +352,16 @@ fn looks_like_path_or_command(bullet: &str) -> Option<String> {
 }
 
 fn has_extension(s: &str) -> bool {
-    Path::new(s)
-        .extension()
-        .is_some_and(|e| e.to_string_lossy().len() <= 6)
+    Path::new(s).extension().is_some_and(|e| {
+        let e = e.to_string_lossy();
+        // Real file extensions (rs, py, json, mp4, …) are short alphanumerics
+        // with at least one letter. This keeps version numbers ("v1.2") and
+        // abbreviations from classifying prose as a path to be verified.
+        !e.is_empty()
+            && e.len() <= 6
+            && e.chars().all(|c| c.is_ascii_alphanumeric())
+            && e.chars().any(|c| c.is_ascii_alphabetic())
+    })
 }
 
 /// Verify distilled bullets that look like paths or commands against the current
@@ -582,15 +594,35 @@ impl Drop for LockGuard {
     }
 }
 
+/// A lock older than this is presumed abandoned (the holder crashed or was
+/// killed before its `Drop` ran). Memory writes are sub-second, so minutes of
+/// age means no live holder — without this, one SIGKILL would disable memory
+/// persistence for every future session until the file is deleted by hand.
+const LOCK_STALE_AFTER: std::time::Duration = std::time::Duration::from_secs(10 * 60);
+
 fn take_lock(parent: &Path) -> Result<LockGuard, String> {
     let lock = parent.join(".memory.lock");
-    match OpenOptions::new().write(true).create_new(true).open(&lock) {
-        Ok(_) => Ok(LockGuard(lock)),
-        Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
-            Err("another session is updating memory".to_string())
+    for _ in 0..2 {
+        match OpenOptions::new().write(true).create_new(true).open(&lock) {
+            Ok(_) => return Ok(LockGuard(lock)),
+            Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
+                let stale = fs::metadata(&lock)
+                    .and_then(|m| m.modified())
+                    .ok()
+                    .and_then(|t| t.elapsed().ok())
+                    .is_some_and(|age| age > LOCK_STALE_AFTER);
+                if !stale {
+                    return Err("another session is updating memory".to_string());
+                }
+                // Break the stale lock and retry the exclusive create once (a
+                // concurrent session may break it first — losing that race
+                // lands in AlreadyExists again and errors out normally).
+                let _ = fs::remove_file(&lock);
+            }
+            Err(e) => return Err(format!("couldn't take memory lock: {e}")),
         }
-        Err(e) => Err(format!("couldn't take memory lock: {e}")),
     }
+    Err("another session is updating memory".to_string())
 }
 
 // Forward the message types so callers don't need a separate import.