From a4769f55429dadc5c731b4cc72354ca4d31d43ab Mon Sep 17 00:00:00 2001 From: David Date: Wed, 15 Jul 2026 08:15:37 -0700 Subject: [PATCH 1/5] feat(tools): annotate silent shell results for the model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Empty and stderr-only results are where models form false premises ("did it work?" / "stderr means it failed") — the largest failure trigger in Zhao et al.'s CLI-agent trajectory study (arXiv:2607.09510, false premises = 30.7% of decisive errors). Make the verdict explicit: - empty success: [no output — command succeeded (exit 0)] - stderr-only success: [command succeeded (exit 0) — output above is stderr] - empty failure: [exit code N — no output on stdout or stderr] - successful commands containing a `cd` segment get a note that the working directory resets before the next command (just-in-time version of the system-prompt warning) Success annotations never contain "[exit code " — steering classifies that marker as failure. Also fix the steering timed-out matcher, which looked for "[timed out after " while the emitted text is "[timed out — process killed]". Co-Authored-By: Claude Fable 5 --- .../hi-agent/src/steering/implementation.rs | 12 ++- crates/hi-tools/src/process.rs | 82 +++++++++++++- crates/hi-tools/src/tools.rs | 100 +++++++++++++++++- 3 files changed, 188 insertions(+), 6 deletions(-) diff --git a/crates/hi-agent/src/steering/implementation.rs b/crates/hi-agent/src/steering/implementation.rs index b87bf9b1..cf3387b2 100644 --- a/crates/hi-agent/src/steering/implementation.rs +++ b/crates/hi-agent/src/steering/implementation.rs @@ -183,7 +183,7 @@ fn tool_result_is_failure(output: &str) -> bool { trimmed.starts_with("Error:") || trimmed.starts_with("⚠ refused:") || trimmed.contains("[exit code ") - || trimmed.contains("[timed out after ") + || trimmed.contains("[timed out") } fn filesystem_mutation_result_landed(name: &str, output: &str) -> bool { @@ -485,6 +485,16 @@ mod tests { args, "[no output]" )); + assert!(implementation_tool_result_landed_mutation( + "bash", + args, + "[no output — command succeeded (exit 0)]" + )); + assert!(!implementation_tool_result_landed_mutation( + "bash", + args, + "[timed out — process killed]" + )); } #[test] diff --git a/crates/hi-tools/src/process.rs b/crates/hi-tools/src/process.rs index 80a469d5..65fdbb03 100644 --- a/crates/hi-tools/src/process.rs +++ b/crates/hi-tools/src/process.rs @@ -41,9 +41,14 @@ pub struct ProcessExecution { impl ProcessExecution { /// Compatibility-friendly model text. Status remains authoritative. + /// + /// Invariant: success annotations must never contain "[exit code " — + /// steering classifies any result containing that marker as a failure. pub fn model_content(&self) -> String { + let stdout_empty = self.outcome.stdout_summary.is_empty(); + let stderr_empty = self.outcome.stderr_summary.is_empty(); let mut out = self.outcome.stdout_summary.clone(); - if !self.outcome.stderr_summary.is_empty() { + if !stderr_empty { if !out.is_empty() && !out.ends_with('\n') { out.push('\n'); } @@ -55,7 +60,13 @@ impl ProcessExecution { if !out.is_empty() && !out.ends_with('\n') { out.push('\n'); } - out.push_str(&format!("[exit code {code}]")); + if stdout_empty && stderr_empty { + out.push_str(&format!( + "[exit code {code} — no output on stdout or stderr]" + )); + } else { + out.push_str(&format!("[exit code {code}]")); + } } } ToolStatus::TimedOut => { @@ -64,6 +75,19 @@ impl ProcessExecution { } out.push_str("[timed out — process killed]"); } + // Empty and stderr-only results are where models form false + // premises ("did it work?" / "stderr means it failed") — state + // the verdict explicitly instead of leaving it implied. + ToolStatus::Succeeded => { + if stdout_empty && stderr_empty { + out.push_str("[no output — command succeeded (exit 0)]"); + } else if stdout_empty { + if !out.ends_with('\n') { + out.push('\n'); + } + out.push_str("[command succeeded (exit 0) — output above is stderr]"); + } + } _ => {} } if out.is_empty() { @@ -455,6 +479,60 @@ mod tests { assert_eq!(run.outcome.exit_code, None); } + #[tokio::test] + async fn empty_success_is_annotated_with_exit_zero() { + let runner = ProcessRunner::from_current_dir().unwrap(); + let run = runner + .run_shell("true", Duration::from_secs(5)) + .await + .unwrap(); + assert_eq!(run.status, ToolStatus::Succeeded); + assert_eq!( + run.model_content(), + "[no output — command succeeded (exit 0)]" + ); + } + + #[tokio::test] + async fn stderr_only_success_is_annotated() { + let runner = ProcessRunner::from_current_dir().unwrap(); + let run = runner + .run_shell("printf warning >&2", Duration::from_secs(5)) + .await + .unwrap(); + assert_eq!(run.status, ToolStatus::Succeeded); + let content = run.model_content(); + assert!(content.contains("warning"), "got: {content:?}"); + assert!( + content.ends_with("[command succeeded (exit 0) — output above is stderr]"), + "got: {content:?}" + ); + assert!( + !content.contains("[exit code "), + "success must never carry the failure marker: {content:?}" + ); + } + + #[tokio::test] + async fn empty_failure_annotates_no_output() { + let runner = ProcessRunner::from_current_dir().unwrap(); + let run = runner + .run_shell("exit 3", Duration::from_secs(5)) + .await + .unwrap(); + assert_eq!(run.status, ToolStatus::Failed); + assert_eq!( + run.model_content(), + "[exit code 3 — no output on stdout or stderr]" + ); + // Failures with output keep the bare marker. + let noisy = runner + .run_shell("printf problem >&2; exit 7", Duration::from_secs(5)) + .await + .unwrap(); + assert!(noisy.model_content().ends_with("[exit code 7]")); + } + #[tokio::test] async fn direct_program_treats_filename_as_one_argument() { let root = std::env::temp_dir().join(format!( diff --git a/crates/hi-tools/src/tools.rs b/crates/hi-tools/src/tools.rs index 2ccd99c7..ff76629c 100644 --- a/crates/hi-tools/src/tools.rs +++ b/crates/hi-tools/src/tools.rs @@ -15,7 +15,7 @@ use crate::edit::{apply_edit, plan_multi_patch}; use crate::paths::cache_key; use crate::read::{run_glob, run_grep, run_list, run_read}; use crate::transaction::{MutationPlan, PlannedFileMutation}; -use crate::{PlanStatus, PlanStep, ProcessRunner, ToolEffects, ToolOutcome}; +use crate::{PlanStatus, PlanStep, ProcessRunner, ToolEffects, ToolOutcome, ToolStatus}; /// A completely parsed and materialized file-tool invocation. /// @@ -1878,6 +1878,19 @@ pub(crate) struct BashArgs { /// Shared `bash` dispatch for both the streaming and non-streaming entry points. /// `run_in_background` short-circuits to a detached process and returns its /// handle; otherwise it runs to completion (streaming output through `on_line`). +/// True when any `;`/`&&`/`||`/pipe/newline-separated segment starts with a +/// `cd` command token (not a substring — `cdk`, `echo cd` don't count). +fn command_contains_cd(command: &str) -> bool { + command.split([';', '|', '&', '\n']).any(|segment| { + let first = segment + .trim_start() + .trim_start_matches('(') + .split_whitespace() + .next(); + first == Some("cd") + }) +} + async fn run_bash_tool( root: &Path, state_root: &Path, @@ -1961,7 +1974,20 @@ async fn run_bash_tool( } let mut outcome = match execution { Ok(execution) => { - let mut outcome = ToolOutcome::plain(execution.model_content()); + let mut content = execution.model_content(); + // Each command runs in a fresh shell pinned to the workspace root, + // so a `cd` silently evaporates before the next command — remind + // the model at the moment it matters, not just in the system prompt. + if execution.status == ToolStatus::Succeeded && command_contains_cd(&args.command) { + if !content.ends_with('\n') { + content.push('\n'); + } + content.push_str( + "[note: the working directory resets to the workspace root before the \ + next command]", + ); + } + let mut outcome = ToolOutcome::plain(content); outcome.status = execution.status; outcome.process = Some(execution.outcome); outcome.truncation = execution.truncation; @@ -2142,7 +2168,7 @@ fn is_env_assignment(tok: &str) -> bool { #[cfg(test)] mod tests { use super::{ - MINIMAL_TOOL_SPECS, TOOL_CATALOG, TOOL_SPECS, fast_check_for, + MINIMAL_TOOL_SPECS, TOOL_CATALOG, TOOL_SPECS, command_contains_cd, fast_check_for, foreground_interactive_command_reason, foreground_interactive_command_reason_at, is_filesystem_mutating, is_known_tool, is_read_only, render_untracked_files, render_untracked_files_with_contents, run_bash_streaming_with_timeout, run_check_in, @@ -2435,6 +2461,74 @@ mod tests { ); } + #[test] + fn command_contains_cd_matches_command_position_only() { + assert!(command_contains_cd("cd /tmp")); + assert!(command_contains_cd("make && cd build && make test")); + assert!(command_contains_cd("(cd sub; cargo test)")); + assert!(command_contains_cd("ls\ncd sub")); + assert!(!command_contains_cd("git cd-hook")); + assert!(!command_contains_cd("echo cd")); + assert!(!command_contains_cd("cdk deploy")); + } + + #[tokio::test] + async fn bash_cd_success_gets_cwd_reset_note() { + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); + let state = root.join(".hi-test-state"); + let lsp = std::sync::Arc::new(hi_lsp::LspManager::new(root)); + let background = crate::BackgroundRegistry::default(); + let cache = std::sync::Mutex::new(crate::ReadCache::new()); + let outcome = crate::execute_in_runtime( + root, + &state, + &lsp, + &background, + &cache, + "bash", + r#"{"command":"cd /tmp"}"#, + ) + .await; + assert!( + outcome + .content + .contains("[note: the working directory resets to the workspace root"), + "got: {}", + outcome.content + ); + assert!( + outcome + .content + .contains("[no output — command succeeded (exit 0)]"), + "got: {}", + outcome.content + ); + } + + #[tokio::test] + async fn bash_without_cd_gets_no_note() { + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")); + let state = root.join(".hi-test-state"); + let lsp = std::sync::Arc::new(hi_lsp::LspManager::new(root)); + let background = crate::BackgroundRegistry::default(); + let cache = std::sync::Mutex::new(crate::ReadCache::new()); + let outcome = crate::execute_in_runtime( + root, + &state, + &lsp, + &background, + &cache, + "bash", + r#"{"command":"true"}"#, + ) + .await; + assert!( + !outcome.content.contains("[note:"), + "got: {}", + outcome.content + ); + } + /// `--confirm-edits` must show the real change: `preview_edit` computes the /// diff without writing. #[tokio::test] From b08269ee102f120f412f9b8976de9ffe30506538 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 15 Jul 2026 08:26:14 -0700 Subject: [PATCH 2/5] feat(agent): detect same-failure verify rounds and demand a diagnosis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Repairing the wrong problem" is the costliest coding-agent failure behavior (39% of wasted execution in arXiv:2607.09510). The verifier now hashes each failure (stage, exit code, digit/hex-masked output) and compares rounds: when a repair didn't change the failure signature, the verify nudge stops saying "reconsider" and instead demands a validated diagnosis via a discriminating probe before any further edit. Line/column drift is masked so an edit that only shifts code is still recognized as the same failure. Unstable-stage failures keep their own escalation (stage_mutation_counts) and are excluded. Nudge-only in v1 — no control-flow change; `repeated_verify_failures` telemetry (report key added) will show whether burning rounds is worth it. Co-Authored-By: Claude Fable 5 --- crates/hi-agent/src/agent/turn.rs | 25 +++++- crates/hi-agent/src/lib.rs | 4 + crates/hi-agent/src/tests/verify.rs | 90 +++++++++++++++++++++ crates/hi-agent/src/verify.rs | 121 +++++++++++++++++++++++++++- crates/hi-cli/src/main.rs | 1 + crates/hi-tui/src/tests.rs | 1 + 6 files changed, 239 insertions(+), 3 deletions(-) diff --git a/crates/hi-agent/src/agent/turn.rs b/crates/hi-agent/src/agent/turn.rs index 65bfa138..4ce70cd9 100644 --- a/crates/hi-agent/src/agent/turn.rs +++ b/crates/hi-agent/src/agent/turn.rs @@ -69,6 +69,7 @@ use crate::{ fn build_turn_telemetry( effective_max_steps: u32, verify_rounds: u32, + repeated_verify_failures: u32, recovery_retries: u32, repeat_nudges: u32, continue_nudges: u32, @@ -89,6 +90,7 @@ fn build_turn_telemetry( TurnTelemetry { effective_max_steps, verify_rounds, + repeated_verify_failures, recovery_retries, repeat_nudges, continue_nudges, @@ -1383,6 +1385,7 @@ impl crate::Agent { self.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), + verifier.repeated_failure_count(), empty_retries, repeat_nudges, continue_total_nudges, @@ -1572,6 +1575,7 @@ impl crate::Agent { self.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), + verifier.repeated_failure_count(), empty_retries, repeat_nudges, continue_total_nudges, @@ -1712,6 +1716,7 @@ impl crate::Agent { self.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), + verifier.repeated_failure_count(), empty_retries, repeat_nudges, continue_total_nudges, @@ -3918,6 +3923,7 @@ If the task is already complete, stop and give your final recap." // check, and reports for those error turns need the stages that // actually ran. self.last_turn_telemetry.verification_executions = verifier.executions().to_vec(); + self.last_turn_telemetry.repeated_verify_failures = verifier.repeated_failure_count(); match outcome { VerifyOutcome::NotRun => { if self.last_verify == Some(false) { @@ -4054,6 +4060,7 @@ If the task is already complete, stop and give your final recap." stage, output, round, + repeated, } => { ui.status(&format!("✗ {} failed; iterating", stage.name)); self.last_verify = Some(false); @@ -4098,9 +4105,22 @@ If the task is already complete, stop and give your final recap." lines.join("\n") ) }; + // Paper-informed escalation (arXiv:2607.09510): repairing + // a misdiagnosed cause is the costliest failure behavior — + // when the failure signature didn't change, demand a + // validated diagnosis instead of another blind patch. + let followup = if repeated { + "This is the SAME failure as the previous verify round — your last \ + change did not alter the outcome. Do not edit again yet. First state \ + a diagnosis and validate it with a discriminating probe (run the \ + failing command with narrowed scope, add a focused assertion or \ + print, or read the exact failing code path), and only then apply a \ + fix for the validated cause." + } else { + "If a previous fix didn't work, reconsider rather than repeat it." + }; let nudge_body = format!( - "{cause_section}Verification stage `{}` failed (`{}`).\n\nOutput:\n{}\n\n{} \ - If a previous fix didn't work, reconsider rather than repeat it.", + "{cause_section}Verification stage `{}` failed (`{}`).\n\nOutput:\n{}\n\n{} {followup}", stage.name, stage.command, output, guidance ); // Replace the previous verify nudge instead of accumulating. @@ -4193,6 +4213,7 @@ If the task is already complete, stop and give your final recap." self.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), + verifier.repeated_failure_count(), empty_retries, repeat_nudges, continue_total_nudges, diff --git a/crates/hi-agent/src/lib.rs b/crates/hi-agent/src/lib.rs index 8a7ddd83..1e784ce6 100644 --- a/crates/hi-agent/src/lib.rs +++ b/crates/hi-agent/src/lib.rs @@ -129,6 +129,9 @@ pub struct TurnTelemetry { pub effective_max_steps: u32, /// How many verify rounds ran this turn (0 = verify off or skipped). pub verify_rounds: u32, + /// How many verify rounds re-failed with the same failure signature as + /// their predecessor — repair attempts that did not change the outcome. + pub repeated_verify_failures: u32, /// Times a content-less / malformed response was silently re-sampled /// (recovery sampling). 0 on a clean turn. pub recovery_retries: u32, @@ -219,6 +222,7 @@ impl Default for TurnTelemetry { Self { effective_max_steps: 0, verify_rounds: 0, + repeated_verify_failures: 0, recovery_retries: 0, repeat_nudges: 0, continue_nudges: 0, diff --git a/crates/hi-agent/src/tests/verify.rs b/crates/hi-agent/src/tests/verify.rs index cedb2cb2..e933d905 100644 --- a/crates/hi-agent/src/tests/verify.rs +++ b/crates/hi-agent/src/tests/verify.rs @@ -106,6 +106,96 @@ async fn verify_failure_exhausts_retries() { assert!(agent.last_turn_telemetry().stalled_unfinished); } +#[tokio::test] +async fn repeated_verify_failure_escalates_nudge() { + let workspace = IsolatedWorkspace::new("verify-repeat"); + // Every round fails identically — from round 2 on, the nudge must say the + // repair changed nothing and demand a validated diagnosis, and telemetry + // must count the repeats. + let mut cfg = workspace.config(); + cfg.verification = crate::VerificationMode::Explicit(vec![VerifyStage::new( + "test", + "printf 'error: same failure\\n' >&2; exit 1", + )]); + cfg.max_verify_repairs = 2; + let tmp = workspace.path("changed.rs"); + let p = tmp.to_string_lossy().to_string(); + let responses = vec![ + write_completion(&p), + completion(vec![Content::Text("attempt 1".into())], 1, 1), + completion(vec![Content::Text("attempt 2".into())], 1, 1), + completion(vec![Content::Text("attempt 3".into())], 1, 1), + completion(vec![Content::Text("attempt 4".into())], 1, 1), + ]; + let mut agent = agent(responses, cfg); + agent.run_turn("x", &mut NullUi).await.unwrap(); + assert_eq!(agent.last_verify(), Some(false)); + assert_eq!(agent.last_turn_telemetry().verify_rounds, 3); + assert_eq!(agent.last_turn_telemetry().repeated_verify_failures, 2); + let nudge = agent + .messages() + .iter() + .rev() + .find(|m| m.role == Role::User && m.text().contains("Verification stage")) + .expect("verify nudge present"); + let body = nudge.text(); + assert!(body.contains("SAME failure"), "escalated nudge: {body}"); + assert!( + body.contains("discriminating probe"), + "diagnosis demand: {body}" + ); + assert!( + !body.contains("reconsider rather than repeat it"), + "weak sentence replaced: {body}" + ); +} + +#[tokio::test] +async fn different_failures_do_not_escalate() { + let workspace = IsolatedWorkspace::new("verify-vary"); + // The failure output changes every round (varying WORD — digits are + // masked by normalization), so no round counts as a repeat and the weak + // follow-up stays. The counter lives outside the workspace root so the + // stage doesn't trip the mutation detector. + let counter = std::env::temp_dir().join("hi-verify-vary-counter"); + let _ = std::fs::remove_file(&counter); + let mut cfg = workspace.config(); + cfg.verification = crate::VerificationMode::Explicit(vec![VerifyStage::new( + "test", + &format!( + "n=$(cat {c} 2>/dev/null || echo A); echo err$n >&2; echo ${{n}}B > {c}; exit 1", + c = counter.to_string_lossy() + ), + )]); + cfg.max_verify_repairs = 1; + let tmp = workspace.path("changed.rs"); + let p = tmp.to_string_lossy().to_string(); + let responses = vec![ + write_completion(&p), + completion(vec![Content::Text("attempt 1".into())], 1, 1), + completion(vec![Content::Text("attempt 2".into())], 1, 1), + completion(vec![Content::Text("attempt 3".into())], 1, 1), + ]; + let mut agent = agent(responses, cfg); + agent.run_turn("x", &mut NullUi).await.unwrap(); + let _ = std::fs::remove_file(&counter); + assert_eq!(agent.last_verify(), Some(false)); + assert_eq!(agent.last_turn_telemetry().verify_rounds, 2); + assert_eq!(agent.last_turn_telemetry().repeated_verify_failures, 0); + let nudge = agent + .messages() + .iter() + .rev() + .find(|m| m.role == Role::User && m.text().contains("Verification stage")) + .expect("verify nudge present"); + let body = nudge.text(); + assert!( + body.contains("reconsider rather than repeat it"), + "weak sentence retained: {body}" + ); + assert!(!body.contains("SAME failure"), "no escalation: {body}"); +} + #[tokio::test] async fn verify_failure_exhaustion_does_not_finalize_as_done() { let workspace = IsolatedWorkspace::new("verify-no-finalize"); diff --git a/crates/hi-agent/src/verify.rs b/crates/hi-agent/src/verify.rs index 482cd889..bd5586ff 100644 --- a/crates/hi-agent/src/verify.rs +++ b/crates/hi-agent/src/verify.rs @@ -132,11 +132,14 @@ pub(crate) enum VerifyOutcome { /// would add noise but not verify the changed surface. SkippedProseOnly { first: bool }, /// A stage failed; its output is fed back to the model. The caller records - /// the nudge and loops. Carries the 1-based round number. + /// the nudge and loops. Carries the 1-based round number. `repeated` is + /// true when this failure has the same signature as the previous round's — + /// i.e. the repair attempt did not change the outcome. Failed { stage: VerifyStage, output: String, round: u32, + repeated: bool, }, /// The verifier itself could not run reliably (spawn/runner failure). InfrastructureError { @@ -164,6 +167,8 @@ pub(crate) struct Verifier { last_effective_stages: Vec, executions: Vec, stage_mutation_counts: std::collections::BTreeMap, + last_failure_signature: Option, + repeated_failure_count: u32, max_rounds: u32, round: u32, } @@ -178,6 +183,8 @@ impl Verifier { last_effective_stages: Vec::new(), executions: Vec::new(), stage_mutation_counts: std::collections::BTreeMap::new(), + last_failure_signature: None, + repeated_failure_count: 0, max_rounds, round: 0, } @@ -232,6 +239,31 @@ impl Verifier { &self.executions } + /// How many rounds re-failed with the same signature as their predecessor. + pub(crate) fn repeated_failure_count(&self) -> u32 { + self.repeated_failure_count + } + + /// Record this round's failure signature and report whether it matches the + /// previous round's — i.e. the repair attempt did not change the failure. + /// The signature must be computed from output WITHOUT round-dependent + /// suffixes (pre-turn attribution), or the final round never matches. + fn note_failure( + &mut self, + name: &str, + command: &str, + exit_code: Option, + output: &str, + ) -> bool { + let signature = failure_signature(name, command, exit_code, output); + let repeated = self.last_failure_signature == Some(signature); + self.last_failure_signature = Some(signature); + if repeated { + self.repeated_failure_count = self.repeated_failure_count.saturating_add(1); + } + repeated + } + /// Run one verification check against the current workspace snapshot, /// compared to the turn baseline. Gates on file changes: if nothing /// changed, returns [`VerifyOutcome::SkippedNoChanges`] (and does NOT @@ -345,10 +377,12 @@ impl Verifier { lsp_errors.len(), lsp_errors.join("\n") ); + let repeated = self.note_failure("lsp", "diagnostics", None, &output); return VerifyOutcome::Failed { stage: VerifyStage::new("lsp", "diagnostics"), output, round, + repeated, }; } if lsp_failed { @@ -433,10 +467,19 @@ impl Verifier { execution.model_content(), ), round, + // Escalation for unstable stages is stage_mutation_counts' + // job — keep it out of the repeated-failure signal. + repeated: false, }; } if execution.status != hi_tools::ToolStatus::Succeeded { let mut output = execution.model_content(); + let repeated = self.note_failure( + &stage.name, + &stage.command, + execution.outcome.exit_code, + &output, + ); if round == max_rounds { ui.status(&format!( "attributing final verification failure · {}", @@ -450,6 +493,7 @@ impl Verifier { stage: stage.clone(), output, round, + repeated, }; }; let command = stage.command.clone(); @@ -497,6 +541,7 @@ impl Verifier { stage: stage.clone(), output, round, + repeated, }; } } @@ -504,6 +549,51 @@ impl Verifier { } } +/// Hash of the stable identity of a stage failure: stage name/command, exit +/// code, and volatile-token-masked output. Used to detect "same failure, +/// different patch" across verify rounds within one turn. +fn failure_signature(name: &str, command: &str, exit_code: Option, output: &str) -> u64 { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + name.hash(&mut hasher); + command.hash(&mut hasher); + exit_code.hash(&mut hasher); + normalize_failure_output(output).hash(&mut hasher); + hasher.finish() +} + +/// Mask volatile tokens so a re-run of the same failure hashes identically: +/// digit runs become `#` (line/column numbers, durations, PIDs, counts) and +/// `0x…` hex runs become `0x#` (addresses). Input capped at 4 KB. A rare +/// false "repeated" only strengthens a nudge — it never blocks anything. +fn normalize_failure_output(output: &str) -> String { + const CAP_BYTES: usize = 4096; + let mut out = String::with_capacity(output.len().min(CAP_BYTES)); + let mut consumed = 0usize; + let mut chars = output.chars().peekable(); + while let Some(c) = chars.next() { + consumed += c.len_utf8(); + if consumed > CAP_BYTES { + break; + } + if c == '0' && matches!(chars.peek(), Some('x' | 'X')) { + consumed += chars.next().map(char::len_utf8).unwrap_or(0); + while matches!(chars.peek(), Some(h) if h.is_ascii_hexdigit()) { + consumed += chars.next().map(char::len_utf8).unwrap_or(0); + } + out.push_str("0x#"); + } else if c.is_ascii_digit() { + while matches!(chars.peek(), Some(d) if d.is_ascii_digit()) { + consumed += chars.next().map(char::len_utf8).unwrap_or(0); + } + out.push('#'); + } else { + out.push(c); + } + } + out +} + fn effective_stages( root: &std::path::Path, changed_files: &[String], @@ -785,6 +875,35 @@ mod tests { use std::path::{Path, PathBuf}; use std::sync::atomic::{AtomicU64, Ordering}; + #[test] + fn normalize_masks_volatile_tokens() { + // Line/column drift must not change the signature: an edit that + // shifts lines without fixing the error is still the same failure. + assert_eq!( + normalize_failure_output("error[E0308]: mismatched types\n --> src/lib.rs:42:18"), + normalize_failure_output("error[E0308]: mismatched types\n --> src/lib.rs:43:18"), + ); + // Durations and addresses are masked. + assert_eq!( + normalize_failure_output("test failed in 1.03s at 0xdeadbeef"), + normalize_failure_output("test failed in 2.94s at 0x7ffe1234"), + ); + // Different error text stays distinct even with digits masked: + // E0308/E0499 both normalize to E#, but the message words differ. + assert_ne!( + normalize_failure_output("error[E0308]: mismatched types"), + normalize_failure_output("error[E0499]: cannot borrow twice"), + ); + } + + #[test] + fn failure_signature_keys_on_stage_and_exit_code() { + let sig = |name: &str, code, out: &str| failure_signature(name, "cmd", code, out); + assert_eq!(sig("test", Some(1), "boom"), sig("test", Some(1), "boom")); + assert_ne!(sig("test", Some(1), "boom"), sig("lint", Some(1), "boom")); + assert_ne!(sig("test", Some(1), "boom"), sig("test", Some(2), "boom")); + } + struct NullUi; impl Ui for NullUi { diff --git a/crates/hi-cli/src/main.rs b/crates/hi-cli/src/main.rs index 3640e381..cf591f84 100644 --- a/crates/hi-cli/src/main.rs +++ b/crates/hi-cli/src/main.rs @@ -1746,6 +1746,7 @@ fn write_report( let telemetry = serde_json::json!({ "effective_max_steps": tel.effective_max_steps, "verify_rounds": tel.verify_rounds, + "repeated_verify_failures": tel.repeated_verify_failures, "recovery_retries": tel.recovery_retries, "repeat_nudges": tel.repeat_nudges, "continue_nudges": tel.continue_nudges, diff --git a/crates/hi-tui/src/tests.rs b/crates/hi-tui/src/tests.rs index cf907a6b..ab488ac4 100644 --- a/crates/hi-tui/src/tests.rs +++ b/crates/hi-tui/src/tests.rs @@ -1126,6 +1126,7 @@ fn ctrl_question_toggles_the_observability_panel() { app.last_telemetry = Some(hi_agent::TurnTelemetry { effective_max_steps: 120, verify_rounds: 2, + repeated_verify_failures: 0, recovery_retries: 1, repeat_nudges: 0, continue_nudges: 1, From d38c49ba1bc159f4859c9d78ff0eb37b9ab1e837 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 15 Jul 2026 08:31:20 -0700 Subject: [PATCH 3/5] feat(agent): thread verbatim task requirements into verify nudges and review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supplying the task's requirements to a checker is the single biggest recall lever for specification-relative failures (arXiv:2607.09510: ignored-requirement recall 3%->22%, false premises 15%->32% once the monitor sees the spec). The independent review previously saw only the derived TaskContract + diff; the verify-failure nudge saw no requirement text at all. TaskContract::requirements_digest builds a bounded (~1.5 KB) digest — derived acceptance sentences plus a verbatim prompt excerpt — computed once per turn and injected into (1) the verify-failure nudge and (2) the independent review context. Deterministic only: no side-call, no config, no latency. Co-Authored-By: Claude Fable 5 --- crates/hi-agent/src/agent/turn.rs | 8 +++- crates/hi-agent/src/task_contract.rs | 64 ++++++++++++++++++++++++++++ crates/hi-agent/src/tests/outcome.rs | 53 +++++++++++++++++++++++ crates/hi-agent/src/tests/verify.rs | 44 +++++++++++++++++++ 4 files changed, 167 insertions(+), 2 deletions(-) diff --git a/crates/hi-agent/src/agent/turn.rs b/crates/hi-agent/src/agent/turn.rs index 4ce70cd9..b882f709 100644 --- a/crates/hi-agent/src/agent/turn.rs +++ b/crates/hi-agent/src/agent/turn.rs @@ -931,6 +931,10 @@ impl crate::Agent { let mut context_generation_seen = self.runtime.context_generation(); let mut indexed_ledger_revision = self.runtime.ledger().revision(); self.last_task_contract = Some(task_contract.clone()); + // Checkers (verify-failure nudges, independent review) get the task's + // requirements verbatim — the derived contract alone can't expose + // specification-relative failures. + let requirements_digest = task_contract.requirements_digest(&context_task); self.refresh_system_message(); let implementation_candidate = if structurally_read_only_subagent { None @@ -3999,7 +4003,7 @@ If the task is already complete, stop and give your final recap." let instructions = self.task_context.as_deref().unwrap_or("(none)"); let stages = verifier.stages_summary().unwrap_or_else(|| "(none)".into()); let context = format!( - "Task contract:\n{contract}\n\nScoped instructions and relevant repository context:\n{instructions}\n\nChanged files:\n{}\n\nDeterministic verification: PASSED\nStages: {stages}\nVerified workspace revision: {}\n\nComplete bounded turn diff:\n{diff}", + "Verbatim task requirements:\n{requirements_digest}\n\nTask contract:\n{contract}\n\nScoped instructions and relevant repository context:\n{instructions}\n\nChanged files:\n{}\n\nDeterministic verification: PASSED\nStages: {stages}\nVerified workspace revision: {}\n\nComplete bounded turn diff:\n{diff}", current_files.join("\n"), verified_digest, ); @@ -4120,7 +4124,7 @@ If the task is already complete, stop and give your final recap." "If a previous fix didn't work, reconsider rather than repeat it." }; let nudge_body = format!( - "{cause_section}Verification stage `{}` failed (`{}`).\n\nOutput:\n{}\n\n{} {followup}", + "{cause_section}Task requirements:\n{requirements_digest}\n\nVerification stage `{}` failed (`{}`).\n\nOutput:\n{}\n\n{} {followup}", stage.name, stage.command, output, guidance ); // Replace the previous verify nudge instead of accumulating. diff --git a/crates/hi-agent/src/task_contract.rs b/crates/hi-agent/src/task_contract.rs index 8bc23b47..cdcabeb9 100644 --- a/crates/hi-agent/src/task_contract.rs +++ b/crates/hi-agent/src/task_contract.rs @@ -57,6 +57,33 @@ impl TaskContract { self.intent = TaskIntent::Mutation; } + /// Compact requirement digest for checkers (verify-failure nudges, the + /// independent review): the derived acceptance sentences plus a verbatim + /// prompt excerpt. Supplying the task's requirements is what lifts a + /// checker's failure-detection recall — a derived contract alone misses + /// specification-relative failures. Bounded to ~1.5 KB. + pub fn requirements_digest(&self, prompt: &str) -> String { + const MAX_ACCEPTANCE_LINES: usize = 8; + const MAX_ACCEPTANCE_LINE_CHARS: usize = 200; + const MAX_PROMPT_CHARS: usize = 700; + let mut digest = String::new(); + for sentence in self.acceptance_text.iter().take(MAX_ACCEPTANCE_LINES) { + digest.push_str("- "); + digest.push_str(truncate_chars(sentence, MAX_ACCEPTANCE_LINE_CHARS)); + digest.push('\n'); + } + if !digest.is_empty() { + digest.push('\n'); + } + digest.push_str("Task (verbatim, may be truncated): "); + let excerpt = truncate_chars(prompt.trim(), MAX_PROMPT_CHARS); + digest.push_str(excerpt); + if excerpt.len() < prompt.trim().len() { + digest.push('…'); + } + digest + } + pub fn requires_review( &self, policy: ReviewPolicy, @@ -311,6 +338,14 @@ fn referenced_paths(prompt: &str) -> Vec { paths.into_iter().collect() } +/// Clip to at most `max` characters on a char boundary. +fn truncate_chars(text: &str, max: usize) -> &str { + match text.char_indices().nth(max) { + Some((byte, _)) => &text[..byte], + None => text, + } +} + fn acceptance_text(prompt: &str) -> Vec { prompt .split(['\n', '.']) @@ -507,6 +542,35 @@ mod tests { } } + #[test] + fn requirements_digest_bounds_and_content() { + let prompt = "Fix the parser. It must handle empty input without panicking. \ + The CLI should print a warning on malformed lines."; + let contract = TaskContract::derive(prompt, VerificationMode::Auto); + let digest = contract.requirements_digest(prompt); + assert!( + digest.contains("must handle empty input"), + "acceptance sentence present: {digest}" + ); + assert!( + digest.contains("should print a warning"), + "second acceptance sentence present: {digest}" + ); + assert!( + digest.contains("Task (verbatim, may be truncated): Fix the parser."), + "verbatim excerpt present: {digest}" + ); + + // A huge prompt with no acceptance keywords still yields a bounded, + // non-empty digest, truncated on a char boundary (multibyte-safe). + let long = "é".repeat(5_000); + let plain = TaskContract::derive(&long, VerificationMode::Auto); + let digest = plain.requirements_digest(&long); + assert!(!digest.is_empty()); + assert!(digest.chars().count() < 800, "bounded: {}", digest.len()); + assert!(digest.ends_with('…'), "signals truncation: {digest}"); + } + #[test] fn explicit_requests_expect_mutation() { for prompt in [ diff --git a/crates/hi-agent/src/tests/outcome.rs b/crates/hi-agent/src/tests/outcome.rs index e94aedef..683a7d11 100644 --- a/crates/hi-agent/src/tests/outcome.rs +++ b/crates/hi-agent/src/tests/outcome.rs @@ -1,5 +1,6 @@ use super::common::{ Canned, IsolatedWorkspace, NullUi, ProviderStep, ScriptedProvider, agent, completion, config, + scripted_agent, }; use super::*; use hi_ai::{ChatRequest, StreamEvent}; @@ -315,6 +316,58 @@ async fn independent_review_status_is_emitted_in_turn_outcome() { assert_eq!(outcome.review, ReviewStatus::Passed); } +#[tokio::test] +async fn independent_review_sees_verbatim_task_requirements() { + let workspace = IsolatedWorkspace::new("outcome-review-requirements"); + // The reviewer must receive the task's requirements verbatim — the + // derived contract alone can't expose specification-relative failures. + let write = completion( + vec![Content::ToolCall { + id: "write-review".into(), + name: "write".into(), + arguments: serde_json::json!({ "path": "reviewed.rs", "content": "reviewed\n" }) + .to_string(), + }], + 1, + 1, + ); + let mut cfg = workspace.config(); + cfg.verification = VerificationMode::Explicit(vec![VerifyStage::new("test", "true")]); + cfg.review = ReviewPolicy::Always; + cfg.allow_no_checkpoint = false; + let (mut agent, requests) = scripted_agent( + vec![ + ProviderStep::Completion(write), + ProviderStep::Completion(completion(vec![Content::Text("done".into())], 1, 1)), + ProviderStep::Completion(completion(vec![Content::Text("APPROVE".into())], 1, 1)), + ], + cfg, + ); + let outcome = agent + .run_turn( + "Create the reviewed file. It must contain the word reviewed.", + &mut NullUi, + ) + .await + .unwrap(); + assert_eq!(outcome.review, ReviewStatus::Passed); + let requests = requests.lock().unwrap(); + let review_request = requests + .iter() + .flat_map(|messages| messages.iter()) + .map(|message| message.text()) + .find(|text| text.contains("Task contract:")) + .expect("review request present"); + assert!( + review_request.contains("Verbatim task requirements:"), + "requirements section in review context: {review_request}" + ); + assert!( + review_request.contains("It must contain the word reviewed"), + "acceptance sentence in review context: {review_request}" + ); +} + #[tokio::test] async fn mutation_after_verification_invalidates_pass_and_verified_revision() { let root = std::env::temp_dir().join(format!( diff --git a/crates/hi-agent/src/tests/verify.rs b/crates/hi-agent/src/tests/verify.rs index e933d905..b1f6842e 100644 --- a/crates/hi-agent/src/tests/verify.rs +++ b/crates/hi-agent/src/tests/verify.rs @@ -291,6 +291,50 @@ async fn verify_failure_nudge_carries_attribution() { ); } +#[tokio::test] +async fn verify_nudge_carries_task_requirements() { + let workspace = IsolatedWorkspace::new("verify-requirements"); + // The verify-failure nudge must anchor the repair to the task's stated + // requirements, not just the stage output — and keep the raw Output + // block (enrich-only). + let mut cfg = workspace.config(); + cfg.verification = crate::VerificationMode::Explicit(vec![VerifyStage::new("test", "false")]); + cfg.max_verify_repairs = 0; + let tmp = workspace.path("changed.rs"); + let p = tmp.to_string_lossy().to_string(); + let mut agent = agent( + vec![ + write_completion(&p), + completion(vec![Content::Text("attempt 1".into())], 1, 1), + completion(vec![Content::Text("attempt 2".into())], 1, 1), + ], + cfg, + ); + agent + .run_turn( + "Fix the parser. It must handle empty input without panicking.", + &mut NullUi, + ) + .await + .unwrap(); + let nudge = agent + .messages() + .iter() + .rev() + .find(|m| m.role == Role::User && m.text().contains("Verification stage")) + .expect("verify nudge present"); + let body = nudge.text(); + assert!( + body.contains("Task requirements:"), + "requirements section: {body}" + ); + assert!( + body.contains("must handle empty input without panicking"), + "acceptance sentence threaded in: {body}" + ); + assert!(body.contains("Output:\n"), "raw Output preserved: {body}"); +} + #[tokio::test] async fn verify_skipped_when_no_files_changed() { let workspace = IsolatedWorkspace::new("verify-no-changes"); From 71ab9fe3c4c1fa88b213050c0fff9c1a2bab0c78 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 15 Jul 2026 08:38:06 -0700 Subject: [PATCH 4/5] feat(agent): make skipped reviews and unverified turns visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fabricated success clusters exactly where verification is absent (26% of failed trajectories in arXiv:2607.09510, beginning at the point of no return). VerificationStatus::Unverified already gates status and exit codes end-to-end; this closes the residual gaps: - trio review stays fail-open, but an Unavailable verdict now books skeptic_unavailable_count / skeptic_last_status telemetry and the TUI prints an explicit "approved WITHOUT review" warning instead of a dim aside - one-shot runs print a deterministic stderr trailer when changes were made but no verification stage ran — emitted regardless of --allow-unverified (the flag changes the exit code, not the truth) - regression tests lock the honest outcomes: round-cap exhaustion reports Failed+Incomplete, mutation without stages reports Unverified, and --allow-unverified completes the turn without laundering the verification status Co-Authored-By: Claude Fable 5 --- crates/hi-agent/src/agent/trio.rs | 18 +++++- crates/hi-agent/src/tests/outcome.rs | 85 ++++++++++++++++++++++++++++ crates/hi-cli/src/main.rs | 50 +++++++++++++++- crates/hi-tui/src/app/run.rs | 7 ++- 4 files changed, 155 insertions(+), 5 deletions(-) diff --git a/crates/hi-agent/src/agent/trio.rs b/crates/hi-agent/src/agent/trio.rs index 3c6313c2..c85d23d4 100644 --- a/crates/hi-agent/src/agent/trio.rs +++ b/crates/hi-agent/src/agent/trio.rs @@ -164,6 +164,7 @@ impl crate::Agent { Err(err) => { self.add_side_error_usage(&err); // Fail-open: a reviewer error can't wedge the loop. + self.note_trio_review_unavailable(); return SkepticVerdict::Unavailable(format!("reviewer error: {err:#}")); } }; @@ -171,7 +172,22 @@ impl crate::Agent { if text.trim().is_empty() { text = content_text(&completion.content); } - parse_trio_verdict(&text) + let verdict = parse_trio_verdict(&text); + if matches!(verdict, SkepticVerdict::Unavailable(_)) { + self.note_trio_review_unavailable(); + } + verdict + } + + /// Fail-open review stays fail-open, but a skipped review must be + /// visible: an approval nobody performed is how fabricated success + /// survives. Booked into the skeptic telemetry the report already carries. + fn note_trio_review_unavailable(&mut self) { + self.last_turn_telemetry.skeptic_unavailable_count = self + .last_turn_telemetry + .skeptic_unavailable_count + .saturating_add(1); + self.last_turn_telemetry.skeptic_last_status = Some(crate::SkepticStatus::Unavailable); } } diff --git a/crates/hi-agent/src/tests/outcome.rs b/crates/hi-agent/src/tests/outcome.rs index 683a7d11..927cd921 100644 --- a/crates/hi-agent/src/tests/outcome.rs +++ b/crates/hi-agent/src/tests/outcome.rs @@ -241,6 +241,91 @@ async fn unverified_mutation_is_incomplete_without_escape_hatch() { assert!(outcome.changed_files.iter().any(|changed| changed == path)); } +#[tokio::test] +async fn trio_unavailable_review_is_recorded_in_telemetry() { + let workspace = IsolatedWorkspace::new("outcome-trio-unavailable"); + // Trio review stays fail-open, but a skipped review must be visible: + // both a reviewer error and a verdict-less reply count in telemetry. + let (mut agent, _requests) = scripted_agent( + vec![ + ProviderStep::Error(hi_ai::ProviderErrorKind::RateLimit), + ProviderStep::Completion(completion( + vec![Content::Text("looks plausible to me".into())], + 1, + 1, + )), + ], + workspace.config(), + ); + let verdict = agent.trio_review("task", "plan").await; + assert!(matches!( + verdict, + crate::agent::skeptic::SkepticVerdict::Unavailable(_) + )); + let verdict = agent.trio_review("task", "plan").await; + assert!(matches!( + verdict, + crate::agent::skeptic::SkepticVerdict::Unavailable(_) + )); + assert_eq!(agent.last_turn_telemetry().skeptic_unavailable_count, 2); + assert_eq!( + agent.last_turn_telemetry().skeptic_last_status, + Some(crate::SkepticStatus::Unavailable) + ); +} + +#[tokio::test] +async fn verify_round_cap_exhaustion_reports_failed_incomplete_outcome() { + let workspace = IsolatedWorkspace::new("outcome-verify-exhausted"); + // Exhausting the repair budget with a red pipeline must surface as a + // failed, incomplete turn — never as completed-with-caveats. + let mut cfg = workspace.config(); + cfg.verification = VerificationMode::Explicit(vec![VerifyStage::new("test", "false")]); + cfg.max_verify_repairs = 0; + let write = completion( + vec![Content::ToolCall { + id: "write-1".into(), + name: "write".into(), + arguments: serde_json::json!({ "path": "changed.rs", "content": "x\n" }).to_string(), + }], + 1, + 1, + ); + let mut agent = agent( + vec![ + write, + completion(vec![Content::Text("attempt 1".into())], 1, 1), + completion(vec![Content::Text("attempt 2".into())], 1, 1), + ], + cfg, + ); + let outcome = agent.run_turn("create the file", &mut NullUi).await.unwrap(); + assert_eq!(outcome.status, TurnStatus::Incomplete); + assert_eq!(outcome.verification, VerificationStatus::Failed); +} + +#[tokio::test] +async fn allow_unverified_completes_but_keeps_unverified_status() { + let workspace = IsolatedWorkspace::new("outcome-allow-unverified"); + // The escape hatch changes the turn status, not the verification truth. + let mut cfg = workspace.config(); + cfg.allow_unverified = true; + let write = completion( + vec![Content::ToolCall { + id: "write-1".into(), + name: "write".into(), + arguments: serde_json::json!({ "path": "created.rs", "content": "x\n" }).to_string(), + }], + 1, + 1, + ); + let done = completion(vec![Content::Text("done".into())], 1, 1); + let mut agent = agent(vec![write, done], cfg); + let outcome = agent.run_turn("create the file", &mut NullUi).await.unwrap(); + assert_eq!(outcome.status, TurnStatus::Completed); + assert_eq!(outcome.verification, VerificationStatus::Unverified); +} + #[tokio::test] async fn independent_review_retries_once_after_transient_provider_error() { // A single rate-limit blip must not downgrade the review to diff --git a/crates/hi-cli/src/main.rs b/crates/hi-cli/src/main.rs index cf591f84..ec93806b 100644 --- a/crates/hi-cli/src/main.rs +++ b/crates/hi-cli/src/main.rs @@ -647,6 +647,11 @@ async fn run() -> Result<()> { if report_result.is_err() { std::process::exit(3); } + if let Ok(outcome) = &result + && let Some(warning) = unverified_warning(outcome) + { + eprintln!("\x1b[33m{warning}\x1b[0m"); + } let exit_code = match &result { Ok(outcome) => one_shot_exit_code(outcome, cli.allow_unverified), Err(_) => 3, @@ -1639,6 +1644,16 @@ fn pipeline_command(stages: &[VerifyStage]) -> Option { ) } +/// One-line stderr trailer for one-shot runs whose changes were never +/// verified: the model's own success claim is not evidence (fabricated +/// success clusters exactly where verification is absent). Emitted regardless +/// of --allow-unverified — the flag changes the exit code, not the truth. +fn unverified_warning(outcome: &TurnOutcome) -> Option { + (outcome.verification == VerificationStatus::Unverified).then(|| { + "hi: warning — changes were made but no verification stage ran (unverified)".to_string() + }) +} + fn one_shot_exit_code(outcome: &TurnOutcome, allow_unverified: bool) -> i32 { match outcome.status { TurnStatus::Cancelled => 130, @@ -2056,7 +2071,7 @@ mod tests { use super::{ auto_memory_enabled, effective_max_tokens_for_model, memory_context, one_shot_exit_code, report_tool_records, report_verification_stages, resolve_live_model_metadata_with_timeout, - review_target_dir_from_prompt_at, top_level_error_code, + review_target_dir_from_prompt_at, top_level_error_code, unverified_warning, write_initialization_failure_report, write_landing, }; use crate::config::{ProviderName, Settings}; @@ -2173,6 +2188,39 @@ mod tests { ); } + #[test] + fn unverified_warning_fires_only_for_unverified_outcomes() { + let outcome = |status, verification| hi_agent::TurnOutcome { + status, + verification, + review: hi_agent::ReviewStatus::NotRequired, + stop_reason: hi_agent::TurnStopReason::Completed, + changed_files: Vec::new(), + verified_workspace_revision: None, + effective_route: hi_agent::EffectiveModelRoute { + provider: Some("test".into()), + model: "model".into(), + }, + }; + let warning = unverified_warning(&outcome( + hi_agent::TurnStatus::Completed, + hi_agent::VerificationStatus::Unverified, + )) + .expect("unverified changes warn"); + assert!(warning.contains("no verification stage ran")); + for verification in [ + hi_agent::VerificationStatus::Passed, + hi_agent::VerificationStatus::Failed, + hi_agent::VerificationStatus::NotApplicable, + ] { + assert_eq!( + unverified_warning(&outcome(hi_agent::TurnStatus::Completed, verification)), + None, + "no warning for {verification:?}" + ); + } + } + #[test] fn report_stages_prefer_actual_execution_evidence() { let execution = hi_agent::VerificationExecution { diff --git a/crates/hi-tui/src/app/run.rs b/crates/hi-tui/src/app/run.rs index 2f39c0dc..bc874c28 100644 --- a/crates/hi-tui/src/app/run.rs +++ b/crates/hi-tui/src/app/run.rs @@ -1738,11 +1738,12 @@ pub async fn run( app.follow(); } hi_agent::SkepticVerdict::Unavailable(msg) => { - // Fail-open: treat as approved (can't wedge the loop). + // Fail-open: treat as approved (can't wedge the loop) — + // but say plainly that no review happened. approved = true; app.push(Line::styled( - format!("trio: reviewer unavailable ({msg}) — approving (fail-open)"), - Style::default().fg(Color::DarkGray), + format!("⚠ trio: approved WITHOUT review — reviewer unavailable in round {round} ({msg})"), + Style::default().fg(Color::Yellow), )); break; } From 6d909a3717100d3283377b435006673e9d1cc0a5 Mon Sep 17 00:00:00 2001 From: David Date: Wed, 15 Jul 2026 08:42:25 -0700 Subject: [PATCH 5/5] chore: format hi-agent/hi-tui and fix trio-commit clippy warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cargo fmt on the core crates plus the two clippy -D warnings failures the trio commit introduced (doc list continuation in trio.rs, nested if in parse_trio_args). hi-mlx formatting is left untouched — it doesn't build everywhere and is outside the CI crate list, though note that `cargo fmt --all --check` still fails on it. Co-Authored-By: Claude Fable 5 --- crates/hi-agent/src/agent/trio.rs | 15 +++------ crates/hi-agent/src/command.rs | 16 ++++----- crates/hi-agent/src/lib.rs | 2 +- crates/hi-agent/src/tests/outcome.rs | 10 ++++-- crates/hi-agent/src/tests/verify.rs | 2 +- crates/hi-tui/src/app/run.rs | 49 +++++++++++++++++++++------- 6 files changed, 58 insertions(+), 36 deletions(-) diff --git a/crates/hi-agent/src/agent/trio.rs b/crates/hi-agent/src/agent/trio.rs index c85d23d4..bbb664f2 100644 --- a/crates/hi-agent/src/agent/trio.rs +++ b/crates/hi-agent/src/agent/trio.rs @@ -9,8 +9,8 @@ //! 1. **Plan** — one bounded chat-only call to `planner_model` (same as //! `/goal` decomposition) produces a lightweight plan string. If no //! planner is configured, the prompt itself is the plan. -//! 2. **Execute** — the session model runs a normal `run_turn` with the plan -//! + prompt as input. Full tools are available (unlike trio's +//! 2. **Execute** — the session model runs a normal `run_turn` with the +//! plan + prompt as input. Full tools are available (unlike trio's //! tool-restricted executor phase — we keep the existing tool surface). //! 3. **Review** — one bounded chat-only call to `skeptic_model` (same //! fail-open gate as `/goal team`) reviews the turn's diff. `Approve` @@ -104,11 +104,7 @@ impl crate::Agent { /// Run the trio reviewer side-call. Returns the verdict (Approve/Object/ /// Unavailable). Uses `skeptic_model` if configured, else the session /// model. Fail-open: errors → Approve. Books usage; records no history. - pub async fn trio_review( - &mut self, - prompt: &str, - plan: &str, - ) -> SkepticVerdict { + pub async fn trio_review(&mut self, prompt: &str, plan: &str) -> SkepticVerdict { let model = self .config .skeptic_model @@ -246,10 +242,7 @@ mod tests { parse_trio_verdict("APPROVE\nlooks good"), SkepticVerdict::Approve ); - assert_eq!( - parse_trio_verdict("**APPROVE**"), - SkepticVerdict::Approve - ); + assert_eq!(parse_trio_verdict("**APPROVE**"), SkepticVerdict::Approve); } #[test] diff --git a/crates/hi-agent/src/command.rs b/crates/hi-agent/src/command.rs index 50fdc751..9df406f5 100644 --- a/crates/hi-agent/src/command.rs +++ b/crates/hi-agent/src/command.rs @@ -205,12 +205,11 @@ fn parse_trio_args(rest: &str) -> (u8, String) { let rest = rest.trim(); if let Some(after) = rest.strip_prefix("--rounds") { let after = after.trim(); - if let Some((n_str, prompt)) = after.split_once(char::is_whitespace) { - if let Ok(n) = n_str.trim().parse::() { - if n > 0 { - return (n, prompt.trim().to_string()); - } - } + if let Some((n_str, prompt)) = after.split_once(char::is_whitespace) + && let Ok(n) = n_str.trim().parse::() + && n > 0 + { + return (n, prompt.trim().to_string()); } // `--rounds` with no valid number + prompt — fall through to treating // the whole thing as a prompt (the flag is optional). @@ -296,10 +295,7 @@ pub enum LoopArg { /// model executes it, and the reviewer model reviews the diff before /// approving or sending it back for revision. Stops when approved or /// `max_rounds` is hit. No persistent goal state — it's a transient loop. - Trio { - prompt: String, - max_rounds: u8, - }, + Trio { prompt: String, max_rounds: u8 }, /// Anything unparseable (bad interval / missing prompt / bad id). Invalid(String), } diff --git a/crates/hi-agent/src/lib.rs b/crates/hi-agent/src/lib.rs index 1e784ce6..011ea297 100644 --- a/crates/hi-agent/src/lib.rs +++ b/crates/hi-agent/src/lib.rs @@ -78,12 +78,12 @@ use { }, }; +pub use agent::skeptic::SkepticVerdict; pub use decision::{Decision, DecisionLog}; pub use goal::{ DEFAULT_SUBGOAL_RETRIES, GOAL_CONTINUE_PROMPT, GOAL_DRIVE_STALL_LIMIT, Goal, GoalStatus, SkepticStatus, SubGoal, }; -pub use agent::skeptic::SkepticVerdict; /// Crate version (from Cargo.toml). pub const VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/crates/hi-agent/src/tests/outcome.rs b/crates/hi-agent/src/tests/outcome.rs index 927cd921..3a9f42b1 100644 --- a/crates/hi-agent/src/tests/outcome.rs +++ b/crates/hi-agent/src/tests/outcome.rs @@ -299,7 +299,10 @@ async fn verify_round_cap_exhaustion_reports_failed_incomplete_outcome() { ], cfg, ); - let outcome = agent.run_turn("create the file", &mut NullUi).await.unwrap(); + let outcome = agent + .run_turn("create the file", &mut NullUi) + .await + .unwrap(); assert_eq!(outcome.status, TurnStatus::Incomplete); assert_eq!(outcome.verification, VerificationStatus::Failed); } @@ -321,7 +324,10 @@ async fn allow_unverified_completes_but_keeps_unverified_status() { ); let done = completion(vec![Content::Text("done".into())], 1, 1); let mut agent = agent(vec![write, done], cfg); - let outcome = agent.run_turn("create the file", &mut NullUi).await.unwrap(); + let outcome = agent + .run_turn("create the file", &mut NullUi) + .await + .unwrap(); assert_eq!(outcome.status, TurnStatus::Completed); assert_eq!(outcome.verification, VerificationStatus::Unverified); } diff --git a/crates/hi-agent/src/tests/verify.rs b/crates/hi-agent/src/tests/verify.rs index b1f6842e..c8b2002f 100644 --- a/crates/hi-agent/src/tests/verify.rs +++ b/crates/hi-agent/src/tests/verify.rs @@ -162,7 +162,7 @@ async fn different_failures_do_not_escalate() { let mut cfg = workspace.config(); cfg.verification = crate::VerificationMode::Explicit(vec![VerifyStage::new( "test", - &format!( + format!( "n=$(cat {c} 2>/dev/null || echo A); echo err$n >&2; echo ${{n}}B > {c}; exit 1", c = counter.to_string_lossy() ), diff --git a/crates/hi-tui/src/app/run.rs b/crates/hi-tui/src/app/run.rs index bc874c28..fc17780f 100644 --- a/crates/hi-tui/src/app/run.rs +++ b/crates/hi-tui/src/app/run.rs @@ -1533,11 +1533,16 @@ pub async fn run( } app.planning = None; if cancelled { - app.push(Line::styled("trio: planning cancelled".to_string(), dim())); + app.push(Line::styled( + "trio: planning cancelled".to_string(), + dim(), + )); app.follow(); continue; } - let plan = plan_result.unwrap_or_else(|| Ok(prompt.clone())).unwrap_or_else(|_| prompt.clone()); + let plan = plan_result + .unwrap_or_else(|| Ok(prompt.clone())) + .unwrap_or_else(|_| prompt.clone()); app.push(Line::styled( format!("trio: plan ready, executing (max {max_rounds} rounds)"), Style::default().fg(Color::Cyan), @@ -1566,7 +1571,11 @@ pub async fn run( Task: {prompt}\n\n\ Plan:\n{plan}\n\n\ Reviewer objections to address:\n{}", - last_objections.iter().map(|o| format!("• {o}")).collect::>().join("\n") + last_objections + .iter() + .map(|o| format!("• {o}")) + .collect::>() + .join("\n") ) }; @@ -1591,7 +1600,10 @@ pub async fn run( app.interrupt = Some(agent.interrupt_handle()); let (tx, rx) = mpsc::unbounded_channel(); let (confirm_tx, confirm_rx) = mpsc::unbounded_channel(); - let mut sink = ChannelUi { tx, confirmations: confirm_tx }; + let mut sink = ChannelUi { + tx, + confirmations: confirm_tx, + }; let background_before = agent.background_process_ids(); let driven = { let fut = agent.run_turn(&run_line, &mut sink); @@ -1621,7 +1633,9 @@ pub async fn run( // Full cancellation cleanup — same as the // main turn path: kill bg processes, rewind // session state, finalize the cancellation. - let killed = agent.kill_background_processes_started_after(&background_before); + let killed = agent.kill_background_processes_started_after( + &background_before, + ); if agent.checkpoint_count() > checkpoint_count && let Err(err) = agent.undo().await { @@ -1630,9 +1644,13 @@ pub async fn run( Style::default().fg(Color::Yellow), )); } - if let Err(err) = agent.rewind_to_snapshot_durable(checkpoint, &turn_snapshot) { + if let Err(err) = + agent.rewind_to_snapshot_durable(checkpoint, &turn_snapshot) + { app.push(Line::styled( - format!("couldn't persist interrupted turn discard: {err:#}"), + format!( + "couldn't persist interrupted turn discard: {err:#}" + ), Style::default().fg(Color::Yellow), )); agent.truncate_messages(checkpoint); @@ -1650,7 +1668,9 @@ pub async fn run( } } let msg = if killed > 0 { - format!("trio: cancelled; killed {killed} background process(es)") + format!( + "trio: cancelled; killed {killed} background process(es)" + ) } else { "trio: cancelled".to_string() }; @@ -1704,18 +1724,25 @@ pub async fn run( } if review_cancelled { app.push(Line::styled( - "trio: review cancelled — approving (fail-open)".to_string(), + "trio: review cancelled — approving (fail-open)" + .to_string(), Style::default().fg(Color::DarkGray), )); approved = true; break; } - let verdict = verdict_result.unwrap_or(hi_agent::SkepticVerdict::Unavailable("reviewer returned no result".into())); + let verdict = verdict_result.unwrap_or( + hi_agent::SkepticVerdict::Unavailable( + "reviewer returned no result".into(), + ), + ); match &verdict { hi_agent::SkepticVerdict::Approve => { approved = true; app.push(Line::styled( - format!("✓ trio: approved in round {round}/{max_rounds}"), + format!( + "✓ trio: approved in round {round}/{max_rounds}" + ), Style::default().fg(Color::Green), )); break;