From 6120bd60a35118887dfffe62986ac9bf863551c1 Mon Sep 17 00:00:00 2001 From: David Date: Mon, 20 Jul 2026 16:50:44 -0700 Subject: [PATCH] refactor: turn phase SM, Agent domains, steer cascade, typed exits Encode legal TurnPhase transitions, mirror cancel/failed cleanup for turn-scoped backgrounds and explore children, compose Agent into task/ report/workspace/subagent holders, table-drive review quality repairs from REVIEW_QUALITY_CASCADE, soft-firewall hi-tools protocol vs infra, and centralize one-shot exit codes on TurnOutcome plus TopLevelErrorKind. --- .../hi-agent/src/agent/coding_memory_turn.rs | 18 +- crates/hi-agent/src/agent/compaction_turn.rs | 8 +- crates/hi-agent/src/agent/curate_turn.rs | 4 +- crates/hi-agent/src/agent/delegate_turn.rs | 8 +- crates/hi-agent/src/agent/explore_turn.rs | 22 +- crates/hi-agent/src/agent/goal_turn.rs | 34 +- crates/hi-agent/src/agent/lifecycle.rs | 187 +++++----- crates/hi-agent/src/agent/memory_turn.rs | 2 +- crates/hi-agent/src/agent/skeptic.rs | 18 +- .../hi-agent/src/agent/turn/fast_feedback.rs | 2 +- crates/hi-agent/src/agent/turn/finalize.rs | 24 +- crates/hi-agent/src/agent/turn/loop_.rs | 120 ++++--- crates/hi-agent/src/agent/turn/model_retry.rs | 16 +- crates/hi-agent/src/agent/turn/model_round.rs | 6 +- crates/hi-agent/src/agent/turn/phase.rs | 122 ++++++- crates/hi-agent/src/agent/turn/setup.rs | 22 +- .../hi-agent/src/agent/turn/steer/cascade.rs | 340 ++++++++++++++++++ crates/hi-agent/src/agent/turn/steer/mod.rs | 1 + .../hi-agent/src/agent/turn/steer/review.rs | 324 ++--------------- crates/hi-agent/src/agent/turn/tools.rs | 4 +- .../hi-agent/src/agent/turn/verify_outcome.rs | 26 +- crates/hi-agent/src/agent/turn/verify_run.rs | 2 +- crates/hi-agent/src/domain.rs | 112 +++++- crates/hi-agent/src/lib.rs | 85 +---- crates/hi-agent/src/outcome.rs | 52 +++ crates/hi-agent/src/steering/review_repair.rs | 6 +- crates/hi-agent/src/tests/curate.rs | 8 +- crates/hi-agent/src/tests/delegate.rs | 10 +- crates/hi-agent/src/tests/explore.rs | 8 +- crates/hi-agent/src/tests/goal_contract.rs | 2 +- crates/hi-agent/src/tests/outcome.rs | 96 ++++- crates/hi-agent/src/tests/retry.rs | 2 +- crates/hi-agent/src/tests/turn.rs | 12 +- crates/hi-agent/src/tests/usage.rs | 6 +- crates/hi-cli/src/main.rs | 15 +- crates/hi-cli/src/report.rs | 16 +- crates/hi-tools/src/lib.rs | 11 + 37 files changed, 1069 insertions(+), 682 deletions(-) create mode 100644 crates/hi-agent/src/agent/turn/steer/cascade.rs diff --git a/crates/hi-agent/src/agent/coding_memory_turn.rs b/crates/hi-agent/src/agent/coding_memory_turn.rs index 51bc49ed..f2185586 100644 --- a/crates/hi-agent/src/agent/coding_memory_turn.rs +++ b/crates/hi-agent/src/agent/coding_memory_turn.rs @@ -11,20 +11,20 @@ impl crate::Agent { /// coding facts (verify command, package ownership, stack, test gate) into /// the session decision log and project memory. Best-effort, no model call. pub(crate) fn record_coding_facts_turn_end(&mut self, ui: &mut dyn Ui) { - if self.coding_facts_written >= MAX_CODING_FACTS_PER_SESSION { + if self.subagents.coding_facts_written >= MAX_CODING_FACTS_PER_SESSION { return; } - if self.last_verify != Some(true) || self.last_changed_files.is_empty() { + if self.report.last_verify != Some(true) || self.workspace.last_changed_files.is_empty() { return; } let wants_tests = self - .last_task_contract + .task.last_task_contract .as_ref() .is_some_and(|c| c.wants_tests); let facts = extract_coding_facts(&CodingFactInput { - changed_files: &self.last_changed_files, - verify_executions: &self.last_turn_telemetry.verification_executions, + changed_files: &self.workspace.last_changed_files, + verify_executions: &self.report.last_turn_telemetry.verification_executions, wants_tests, workspace_root: self.runtime.root(), }); @@ -33,7 +33,7 @@ impl crate::Agent { } // Room under the session cap. - let budget = MAX_CODING_FACTS_PER_SESSION.saturating_sub(self.coding_facts_written) as usize; + let budget = MAX_CODING_FACTS_PER_SESSION.saturating_sub(self.subagents.coding_facts_written) as usize; let facts: Vec<_> = facts.into_iter().take(budget).collect(); if facts.is_empty() { return; @@ -56,8 +56,8 @@ impl crate::Agent { } } self.decisions = next; - self.coding_facts_written = self - .coding_facts_written + self.subagents.coding_facts_written = self + .subagents.coding_facts_written .saturating_add(facts.len() as u32); self.refresh_system_message(); @@ -85,7 +85,7 @@ impl crate::Agent { } // Phase P: re-rank live memory so the next model call sees new bullets // without waiting for process restart / next session load. - let task = self.last_task_prompt.clone().unwrap_or_default(); + let task = self.task.last_task_prompt.clone().unwrap_or_default(); self.refresh_memory_context(&task); self.refresh_system_message(); let _ = recorded; diff --git a/crates/hi-agent/src/agent/compaction_turn.rs b/crates/hi-agent/src/agent/compaction_turn.rs index 4af031d7..9e369046 100644 --- a/crates/hi-agent/src/agent/compaction_turn.rs +++ b/crates/hi-agent/src/agent/compaction_turn.rs @@ -68,7 +68,7 @@ impl crate::Agent { as too large. Continue from this latest user request; ask for missing details if the \ omitted context is required.]\n\n{input}" )); - self.context_used = 0; + self.report.context_used = 0; ui.status( "provider rejected the request as too large; dropped prior conversation context and retrying", ); @@ -123,7 +123,7 @@ impl crate::Agent { ); if freed > 0 { self.runtime.invalidate_context_after_compaction(); - self.context_used = 0; + self.report.context_used = 0; ui.status(&format!( "elided ~{}k chars of old tool output before request to fit context", freed / 1000 @@ -173,7 +173,7 @@ impl crate::Agent { the model context window. Continue from this latest user request; ask for missing \ details if the omitted context is required.]\n\n{input}" )); - self.context_used = 0; + self.report.context_used = 0; dropped_prior_context = true; ui.status( "request would exceed the model context window; dropped prior conversation context and retrying", @@ -410,7 +410,7 @@ impl crate::Agent { } self.runtime.invalidate_context_after_compaction(); - self.context_used = 0; + self.report.context_used = 0; } /// Run the summarization model call over `slice`, returning the summary text diff --git a/crates/hi-agent/src/agent/curate_turn.rs b/crates/hi-agent/src/agent/curate_turn.rs index 83a0a36c..62ca9578 100644 --- a/crates/hi-agent/src/agent/curate_turn.rs +++ b/crates/hi-agent/src/agent/curate_turn.rs @@ -32,7 +32,7 @@ impl crate::Agent { /// re-checks the cap defensively. Like [`update_memory`](Self::update_memory) /// it builds a throwaway message vec and does NOT record into session history. pub(crate) async fn curate_turn_end(&mut self, turn_start: usize, ui: &mut dyn Ui) { - if self.auto_skills_written >= MAX_AUTO_SKILLS_PER_SESSION { + if self.subagents.auto_skills_written >= MAX_AUTO_SKILLS_PER_SESSION { return; } // Just this turn's trajectory (user prompt → tool calls → results), with @@ -122,7 +122,7 @@ impl crate::Agent { &skill.body, ) { Ok(Some(path)) => { - self.auto_skills_written += 1; + self.subagents.auto_skills_written += 1; ui.status(&format!("✓ curated skill → {}", path.display())); } Ok(None) => {} // a skill by this name already exists diff --git a/crates/hi-agent/src/agent/delegate_turn.rs b/crates/hi-agent/src/agent/delegate_turn.rs index ca7b5b9a..98a1736e 100644 --- a/crates/hi-agent/src/agent/delegate_turn.rs +++ b/crates/hi-agent/src/agent/delegate_turn.rs @@ -59,7 +59,7 @@ impl crate::Agent { false, ); } - if self.delegate_subagents_used >= MAX_DELEGATE_SUBAGENTS_PER_SESSION { + if self.subagents.delegate_subagents_used >= MAX_DELEGATE_SUBAGENTS_PER_SESSION { return delegate_tool_outcome( format!( "delegate budget exhausted ({MAX_DELEGATE_SUBAGENTS_PER_SESSION} this session); \ @@ -70,7 +70,7 @@ impl crate::Agent { false, ); } - let Some(runner) = self.delegate_runner.clone() else { + let Some(runner) = self.subagents.delegate_runner.clone() else { return delegate_tool_outcome( "delegate unavailable: no subagent runner is attached in this context; \ implement it directly instead.", @@ -86,8 +86,8 @@ impl crate::Agent { .map(str::to_string) .filter(|s| !s.trim().is_empty()); - self.delegate_subagents_used += 1; - let n = self.delegate_subagents_used; + self.subagents.delegate_subagents_used += 1; + let n = self.subagents.delegate_subagents_used; let summary: String = task.chars().take(72).collect(); let ellipsis = if task.chars().count() > 72 { "…" } else { "" }; ui.subagent_note(&format!( diff --git a/crates/hi-agent/src/agent/explore_turn.rs b/crates/hi-agent/src/agent/explore_turn.rs index 1d57175f..4ee26b92 100644 --- a/crates/hi-agent/src/agent/explore_turn.rs +++ b/crates/hi-agent/src/agent/explore_turn.rs @@ -57,7 +57,7 @@ impl crate::Agent { hi_tools::ToolStatus::Failed, ); } - if self.explore_subagents_used >= MAX_EXPLORE_SUBAGENTS_PER_SESSION { + if self.subagents.explore_subagents_used >= MAX_EXPLORE_SUBAGENTS_PER_SESSION { return explore_tool_outcome( format!( "explore budget exhausted ({MAX_EXPLORE_SUBAGENTS_PER_SESSION} subagents this \ @@ -66,8 +66,8 @@ impl crate::Agent { hi_tools::ToolStatus::Denied, ); } - self.explore_subagents_used += 1; - let n = self.explore_subagents_used; + self.subagents.explore_subagents_used += 1; + let n = self.subagents.explore_subagents_used; // Prominent callout so the user clearly sees a nested agent run start. let summary: String = task.chars().take(72).collect(); let ellipsis = if task.chars().count() > 72 { "…" } else { "" }; @@ -165,12 +165,20 @@ impl crate::Agent { }); explore_tool_outcome(answer, status) } - Err(err) => explore_tool_outcome( - format!("explore subagent error: {err}"), - hi_tools::ToolStatus::Failed, - ), + Err(err) => { + // Mirror parent failed-turn cleanup: kill child backgrounds and + // type an infrastructure outcome so nested escapes don't leak. + let _ = child.finalize_failed_turn(); + explore_tool_outcome( + format!("explore subagent error: {err}"), + hi_tools::ToolStatus::Failed, + ) + } } }; + // Always tear down child-owned backgrounds (read-only explore should be + // quiet, but bash-in-readonly denial paths still share the kill API). + child.kill_background_processes(); // Fold the child's token usage into the parent's session totals. self.add_side_usage(*child.totals()); ui.subagent_note(&format!("↳ explore subagent {n} done")); diff --git a/crates/hi-agent/src/agent/goal_turn.rs b/crates/hi-agent/src/agent/goal_turn.rs index 1302b9ed..68906dbd 100644 --- a/crates/hi-agent/src/agent/goal_turn.rs +++ b/crates/hi-agent/src/agent/goal_turn.rs @@ -77,7 +77,7 @@ impl crate::Agent { // Phase C: same obligation as the interactive settle path — a done-claim // via update_plan or heuristic advance is not enough without a green seal // and a non-stalled turn. Skeptic (below) is an extra gate on top. - let verified_clean = matches!(self.last_verify, Some(true)); + let verified_clean = matches!(self.report.last_verify, Some(true)); let mut clean_success = verified_clean && !stalled_unfinished && !stalled_repeating && !hit_step_cap; @@ -103,7 +103,7 @@ impl crate::Agent { // one-line code fix over the trivial-diff exemption — coding-memory // and skill curation write those after verify by design. let changed_bytes: u64 = self - .last_file_changes + .workspace.last_file_changes .iter() .filter(|change| !crate::verify::is_prose_only_path(&change.path)) .map(|change| match (change.before_len, change.after_len) { @@ -149,7 +149,7 @@ impl crate::Agent { ui.status(&format!("🔍 skeptic objected — retrying: {first}")); self.refresh_system_message(); self.persist_goal(ui); - self.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Objected); + self.report.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Objected); return false; } SkepticVerdict::Escalate(items) => { @@ -171,7 +171,7 @@ impl crate::Agent { ui.status(&format!( "🛑 skeptic escalated — step needs your judgment, skipping past it: {first}" )); - self.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Escalated); + self.report.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Escalated); self.refresh_system_message(); self.persist_goal(ui); return false; @@ -180,7 +180,7 @@ impl crate::Agent { if let Some(goal) = self.goals.structured.as_mut() { goal.last_skeptic_status = Some(SkepticStatus::Approved); } - self.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Approved); + self.report.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Approved); ui.status("🔍 skeptic approved — advancing"); } SkepticVerdict::Unavailable(reason) => { @@ -188,11 +188,11 @@ impl crate::Agent { goal.skeptic_unavailable = goal.skeptic_unavailable.saturating_add(1); goal.last_skeptic_status = Some(SkepticStatus::Unavailable); } - self.last_turn_telemetry.skeptic_unavailable_count = self - .last_turn_telemetry + self.report.last_turn_telemetry.skeptic_unavailable_count = self + .report.last_turn_telemetry .skeptic_unavailable_count .saturating_add(1); - self.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Unavailable); + self.report.last_turn_telemetry.skeptic_last_status = Some(SkepticStatus::Unavailable); ui.status(&format!( "⚠ skeptic unavailable — advancing without review: {reason}" )); @@ -223,11 +223,11 @@ impl crate::Agent { let current_pass = verified_at.is_some_and(|(verified_revision, verified)| { *verified_revision == revision && verified == &digest }); - self.last_changed_files = + self.workspace.last_changed_files = changes.iter().map(|change| change.path.clone()).collect(); - self.last_file_changes = changes; + self.workspace.last_file_changes = changes; if !current_pass { - self.last_verify = None; + self.report.last_verify = None; clean_success = false; verification_invalidated = true; self.goals.structured = goal_before.clone(); @@ -238,7 +238,7 @@ impl crate::Agent { } } Err(error) => { - self.last_verify = None; + self.report.last_verify = None; clean_success = false; verification_invalidated = true; self.goals.structured = goal_before.clone(); @@ -273,11 +273,11 @@ impl crate::Agent { // A clean read-only turn (investigation, Q&A — no edits, no verify, // no stall) is neutral: neither advance nor record failure. The sub-goal // stays active for the next turn, which should do the actual work. - let no_edit_neutral = self.last_verify.is_none() + let no_edit_neutral = self.report.last_verify.is_none() && !stalled_unfinished && !stalled_repeating && !hit_step_cap - && self.last_changed_files.is_empty(); + && self.workspace.last_changed_files.is_empty(); if no_edit_neutral { return verification_invalidated; } @@ -337,7 +337,7 @@ impl crate::Agent { // the counter also changes goal state, which resets the frontend // drive-stall counter so a long milestone isn't parked mid-build. if hit_step_cap { - let made_progress = !self.last_changed_files.is_empty(); + let made_progress = !self.workspace.last_changed_files.is_empty(); // A milestone that keeps hitting the step cap *while making progress* // is too big for one turn: decompose it into turn-sized sub-steps // rather than grind it out over dozens of turns. Snapshot the decision @@ -416,13 +416,13 @@ impl crate::Agent { // the budget is exhausted, the sub-goal (and goal) is marked Failed. let reason = if hit_step_cap { "hit the per-turn step cap" - } else if self.last_verify == Some(false) { + } else if self.report.last_verify == Some(false) { "verification failed and the turn ended without fixing it" } else if stalled_repeating { "stalled repeating the same tool call" } else if stalled_unfinished { "ended without completing the requested work" - } else if self.last_verify.is_none() && !self.last_changed_files.is_empty() { + } else if self.report.last_verify.is_none() && !self.workspace.last_changed_files.is_empty() { "ended with unverified workspace changes" } else { "verification failed and the turn ended without fixing it" diff --git a/crates/hi-agent/src/agent/lifecycle.rs b/crates/hi-agent/src/agent/lifecycle.rs index d57d380e..a684ff94 100644 --- a/crates/hi-agent/src/agent/lifecycle.rs +++ b/crates/hi-agent/src/agent/lifecycle.rs @@ -19,7 +19,7 @@ use crate::snapshot::SnapshotCache; use crate::transcript::Transcript; use crate::ui; use crate::{ - SessionSink, TurnPhase, TurnTelemetry, Ui, VerificationMode, VerifyStage, WorkspaceRuntime, + SessionSink, TurnTelemetry, Ui, VerificationMode, VerifyStage, WorkspaceRuntime, }; impl crate::Agent { @@ -59,11 +59,11 @@ impl crate::Agent { let persisted = history.len(); let mut agent = Self::with_messages(provider, config, history, persisted, None)?; agent.totals = usage; - agent.checkpoints = checkpoint_refs; - if agent.checkpoints.len() > crate::MAX_CHECKPOINTS { + agent.workspace.checkpoints = checkpoint_refs; + if agent.workspace.checkpoints.len() > crate::MAX_CHECKPOINTS { agent - .checkpoints - .drain(0..agent.checkpoints.len() - crate::MAX_CHECKPOINTS); + .workspace.checkpoints + .drain(0..agent.workspace.checkpoints.len() - crate::MAX_CHECKPOINTS); } agent.decisions = decisions; agent.goals.structured = agent @@ -118,37 +118,16 @@ impl crate::Agent { local_skeptic: None, config, runtime, - task_context: None, - memory_context: None, - last_task_prompt: None, - last_task_contract: None, + task: crate::domain::TaskContextState::default(), messages, tools, session: None, - delegate_runner: None, persisted, totals: Usage::default(), - last_turn_usage: Usage::default(), - last_user_prompt_tokens: 0, - last_verify: None, - context_used: 0, - checkpoints: Vec::new(), - last_changed_files: Vec::new(), - last_file_changes: Vec::new(), - turn_diff_cache: None, - turn_stub_scan_cache: None, - active_turn_ledger_revision: None, - active_turn_message_start: None, - auto_skills_written: 0, - coding_facts_written: 0, - explore_subagents_used: 0, - delegate_subagents_used: 0, - last_compat_fallbacks: Vec::new(), + report: crate::domain::TurnReportState::new(last_effective_route), + workspace: crate::domain::WorkspaceTurnState::default(), + subagents: crate::domain::SubagentSessionState::default(), interrupt: Arc::new(std::sync::atomic::AtomicBool::new(false)), - last_turn_telemetry: TurnTelemetry::default(), - last_turn_outcome: None, - turn_phase: TurnPhase::Setup, - last_effective_route, goals: crate::domain::GoalState::default(), decisions: DecisionLog::default(), snapshot_cache: SnapshotCache::default(), @@ -175,7 +154,7 @@ impl crate::Agent { /// checkpoint. Returns `None` if there's nothing to undo, else the number of /// files restored or removed. pub async fn undo(&mut self) -> Result> { - let Some(reference) = self.checkpoints.last().cloned() else { + let Some(reference) = self.workspace.checkpoints.last().cloned() else { return Ok(None); }; let (target, expected_current) = hi_tools::checkpoint::parse_reference(&reference)?; @@ -225,7 +204,7 @@ impl crate::Agent { .await? } }; - let mut next = self.checkpoints.clone(); + let mut next = self.workspace.checkpoints.clone(); next.pop(); let persist_result = self .session @@ -262,7 +241,7 @@ impl crate::Agent { ))), }; } - self.checkpoints = next; + self.workspace.checkpoints = next; // The working tree just changed under us, so any cached snapshot is now // stale. Without this, the next turn reuses pre-undo fingerprints and // change detection / verify gating / last_changed_files can be wrong. @@ -274,8 +253,8 @@ impl crate::Agent { // Bring the content ledger back to the restored state and do not report // the now-undone effects as the latest workspace changes. self.runtime.ledger().reconcile()?; - self.last_changed_files.clear(); - self.last_file_changes.clear(); + self.workspace.last_changed_files.clear(); + self.workspace.last_file_changes.clear(); Ok(Some(n)) } @@ -321,10 +300,10 @@ impl crate::Agent { self.messages = messages; self.persisted = persisted; self.totals = usage; - self.checkpoints = checkpoint_refs; - if self.checkpoints.len() > crate::MAX_CHECKPOINTS { - self.checkpoints - .drain(0..self.checkpoints.len() - crate::MAX_CHECKPOINTS); + self.workspace.checkpoints = checkpoint_refs; + if self.workspace.checkpoints.len() > crate::MAX_CHECKPOINTS { + self.workspace.checkpoints + .drain(0..self.workspace.checkpoints.len() - crate::MAX_CHECKPOINTS); } self.decisions = decisions; self.goals.structured = self @@ -335,9 +314,9 @@ impl crate::Agent { // what `with_messages` initializes to None/empty for a fresh agent. self.goals.free_text = None; self.goals.set_plan_if_pending(plan); - self.last_changed_files = Vec::new(); - self.last_turn_telemetry = TurnTelemetry::default(); - self.last_verify = None; + self.workspace.last_changed_files = Vec::new(); + self.report.last_turn_telemetry = TurnTelemetry::default(); + self.report.last_verify = None; self.refresh_system_message(); // The transcript was replaced, so any cached working-tree snapshot is // stale. Clear it so the next turn re-snapshots from scratch. @@ -357,7 +336,7 @@ impl crate::Agent { /// Attach the runner that executes write-capable `delegate` subagents. Without /// one, the `delegate` tool reports itself unavailable. pub fn set_delegate_runner(&mut self, runner: std::sync::Arc) { - self.delegate_runner = Some(runner); + self.subagents.delegate_runner = Some(runner); } /// Set the write-capable `delegate` policy at runtime (`/delegate on|off|risk`) @@ -508,17 +487,17 @@ impl crate::Agent { /// Token usage accumulated by the most recent user turn. pub fn last_turn_usage(&self) -> &Usage { - &self.last_turn_usage + &self.report.last_turn_usage } /// Estimated tokens in the raw user prompt for the most recent user turn. pub fn last_user_prompt_tokens(&self) -> u64 { - self.last_user_prompt_tokens + self.report.last_user_prompt_tokens } /// The context-window occupancy, as last reported by the provider. pub fn context_used(&self) -> u64 { - self.context_used + self.report.context_used } /// The configured context window, if known. @@ -553,10 +532,10 @@ impl crate::Agent { if let Some(w) = window && w > 0 { - let pct = (self.context_used * 100 / u64::from(w)).min(100); + let pct = (self.report.context_used * 100 / u64::from(w)).min(100); out.push_str(&format!( "context: {} / {} tokens ({}% used)\n", - humanize_count(self.context_used), + humanize_count(self.report.context_used), humanize_count(u64::from(w)), pct, )); @@ -566,8 +545,8 @@ impl crate::Agent { )); // How many turns until compaction triggers? let threshold = u64::from(w) * self.config.memory.auto_compact_percent / 100; - if self.context_used < threshold { - let headroom = threshold.saturating_sub(self.context_used); + if self.report.context_used < threshold { + let headroom = threshold.saturating_sub(self.report.context_used); out.push_str(&format!( " headroom before auto-compact: {} tokens ({})\n", humanize_count(headroom), @@ -585,7 +564,7 @@ impl crate::Agent { } else { out.push_str(&format!( "context: {} tokens used (window unknown)\n", - humanize_count(self.context_used), + humanize_count(self.report.context_used), )); } // Per-message breakdown (system + up to 10 recent). @@ -673,7 +652,7 @@ impl crate::Agent { usage.effective_input_tokens() }; if occupancy > 0 { - self.context_used = occupancy; + self.report.context_used = occupancy; } } @@ -685,12 +664,12 @@ impl crate::Agent { /// 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); + self.report.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; + self.report.last_turn_usage = Usage::default(); + self.report.last_user_prompt_tokens = user_prompt_tokens; } pub(crate) fn add_error_usage(&mut self, err: &anyhow::Error) { @@ -711,18 +690,18 @@ impl crate::Agent { pub(crate) fn emit_usage(&self, ui: &mut dyn Ui) { ui.usage( - self.last_user_prompt_tokens, - self.last_turn_usage.output_tokens, - self.context_used, + self.report.last_user_prompt_tokens, + self.report.last_turn_usage.output_tokens, + self.report.context_used, self.config.routing.context_window, - self.last_turn_usage.estimated, + self.report.last_turn_usage.estimated, ); ui.rate_limits(self.totals.rate_limits); } /// Number of git checkpoints created so far (for `/undo`). pub fn checkpoint_count(&self) -> usize { - self.checkpoints.len() + self.workspace.checkpoints.len() } /// Explicit root owned by this agent's workspace runtime. @@ -750,29 +729,34 @@ impl crate::Agent { /// Finalize a turn whose future was cancelled by its frontend. Reconcile /// after rollback/cleanup so reports contain the exact surviving effects /// instead of a fabricated empty list. + /// + /// Mirrors frontend cancel cleanup for turn-scoped background processes when + /// the caller has not already killed them (safe if already empty). pub fn finalize_cancelled_turn(&mut self) -> Result { - if let Some(start) = self.active_turn_message_start.take() { + self.cleanup_turn_backgrounds(); + if let Some(start) = self.workspace.active_turn_message_start.take() { self.truncate_messages(start); } self.runtime.ledger().reconcile()?; let baseline = self - .active_turn_ledger_revision + .workspace.active_turn_ledger_revision .take() .unwrap_or_else(|| self.runtime.ledger().revision()); let changes = self.runtime.ledger().changes_since(baseline); - self.last_changed_files = changes.iter().map(|change| change.path.clone()).collect(); - self.last_file_changes = changes; - self.last_verify = None; + self.workspace.last_changed_files = changes.iter().map(|change| change.path.clone()).collect(); + self.workspace.last_file_changes = changes; + self.report.last_verify = None; + self.workspace.clear_active_baselines(); let outcome = crate::TurnOutcome { status: crate::TurnStatus::Cancelled, verification: crate::VerificationStatus::Unverified, review: crate::ReviewStatus::NotRequired, stop_reason: crate::TurnStopReason::Cancelled, - changed_files: self.last_changed_files.clone(), + changed_files: self.workspace.last_changed_files.clone(), verified_workspace_revision: None, - effective_route: self.last_effective_route.clone(), + effective_route: self.report.last_effective_route.clone(), }; - self.last_turn_outcome = Some(outcome.clone()); + self.report.last_turn_outcome = Some(outcome.clone()); let _ = self.persist(); Ok(outcome) } @@ -781,27 +765,40 @@ impl crate::Agent { /// provider error before the normal common finalizer ran. Frontends call /// this before writing reports so late UI/session effects are never /// replaced by a fabricated empty change list. + /// + /// Also tears down turn-scoped background processes so secondary failure + /// paths (model retry fatal, verify re-entry escape, nested explore) match + /// the primary cancel cleanup sequence. pub fn finalize_failed_turn(&mut self) -> crate::TurnOutcome { + self.cleanup_turn_backgrounds(); let baseline = self - .active_turn_ledger_revision + .workspace.active_turn_ledger_revision .take() .unwrap_or_else(|| self.runtime.ledger().revision()); let _ = self.runtime.ledger().reconcile(); let changes = self.runtime.ledger().changes_since(baseline); - self.last_changed_files = changes.iter().map(|change| change.path.clone()).collect(); - self.last_file_changes = changes; - self.last_verify = None; - self.active_turn_message_start = None; - let route = self.last_effective_route.clone(); + self.workspace.last_changed_files = changes.iter().map(|change| change.path.clone()).collect(); + self.workspace.last_file_changes = changes; + self.report.last_verify = None; + self.workspace.clear_active_baselines(); + let route = self.report.last_effective_route.clone(); let outcome = crate::TurnOutcome::infrastructure_failure( route.model, route.provider, - self.last_changed_files.clone(), + self.workspace.last_changed_files.clone(), ); - self.last_turn_outcome = Some(outcome.clone()); + self.report.last_turn_outcome = Some(outcome.clone()); outcome } + /// Kill background processes started after this turn's baseline, if any. + /// Idempotent: missing baseline is a no-op (caller may have already cleaned up). + fn cleanup_turn_backgrounds(&self) { + if let Some(before) = self.workspace.active_turn_background_baseline.as_deref() { + let _ = self.runtime.background().kill_started_after(before); + } + } + /// A shared interrupt handle the UI can set to skip the current tool call. /// The agent checks it between tool executions; when set, the current tool's /// result is replaced with "interrupted by user" and the flag is cleared. @@ -910,13 +907,13 @@ impl crate::Agent { parts.push(t.to_string()); } } - if let Some(mem) = self.memory_context.as_deref() { + if let Some(mem) = self.task.memory_context.as_deref() { let t = mem.trim(); if !t.is_empty() { parts.push(t.to_string()); } } - if let Some(task) = self.task_context.as_deref() { + if let Some(task) = self.task.task_context.as_deref() { let t = task.trim(); if !t.is_empty() { parts.push(t.to_string()); @@ -941,8 +938,8 @@ impl crate::Agent { let project = crate::memory::read_project_annotated_at(self.runtime.root()); let global = crate::memory::read_global_memory(); let next = crate::memory::memory_section_for_task(&project, &global, task); - if next != self.memory_context { - self.memory_context = next; + if next != self.task.memory_context { + self.task.memory_context = next; } } @@ -1131,24 +1128,24 @@ impl crate::Agent { /// Whether the most recent turn's verification passed (None if not run). pub fn last_verify(&self) -> Option { - self.last_verify + self.report.last_verify } /// Files whose content or presence changed in the most recent turn. pub fn last_changed_files(&self) -> &[String] { - &self.last_changed_files + &self.workspace.last_changed_files } /// Exact structured file changes reported by tools during the last turn. pub fn last_file_changes(&self) -> &[hi_tools::FileChange] { - &self.last_file_changes + &self.workspace.last_file_changes } /// Merge repeated edits to one path into a turn-level before/after record. pub(crate) fn record_tool_effects(&mut self, effects: &hi_tools::ToolEffects) -> Result<()> { self.runtime.ledger().record_tool_effects(effects)?; if effects.mutation_applied { - if let Some(contract) = self.last_task_contract.as_mut() { + if let Some(contract) = self.task.last_task_contract.as_mut() { contract.observe_mutation(); } self.runtime.clear_repo_map_cache(); @@ -1161,7 +1158,7 @@ impl crate::Agent { pub(crate) fn reconcile_workspace_changes(&mut self) -> Result<()> { let changes = self.runtime.ledger().reconcile()?; if !changes.is_empty() { - if let Some(contract) = self.last_task_contract.as_mut() { + if let Some(contract) = self.task.last_task_contract.as_mut() { contract.observe_mutation(); } self.runtime.clear_repo_map_cache(); @@ -1174,18 +1171,18 @@ impl crate::Agent { fn merge_file_changes(&mut self, changes: &[hi_tools::FileChange]) { for change in changes { if let Some(index) = self - .last_file_changes + .workspace.last_file_changes .iter() .position(|existing| existing.path == change.path) { - let existing = &self.last_file_changes[index]; + let existing = &self.workspace.last_file_changes[index]; if existing.before_digest == change.after_digest && existing.before_mode == change.after_mode { - self.last_file_changes.remove(index); + self.workspace.last_file_changes.remove(index); continue; } - let existing = &mut self.last_file_changes[index]; + let existing = &mut self.workspace.last_file_changes[index]; existing.after_digest = change.after_digest.clone(); existing.after_len = change.after_len; existing.after_mode = change.after_mode; @@ -1199,14 +1196,14 @@ impl crate::Agent { (false, false) => change.kind, }; } else { - self.last_file_changes.push(change.clone()); + self.workspace.last_file_changes.push(change.clone()); } } } /// Compatibility fallbacks that were triggered in the most recent turn. pub fn last_compat_fallbacks(&self) -> &[String] { - &self.last_compat_fallbacks + &self.report.last_compat_fallbacks } /// Telemetry from the most recent turn: verify rounds, recovery retries, @@ -1214,23 +1211,23 @@ impl crate::Agent { /// verify failure. Lets callers diagnose *how* a turn went, not just /// whether it passed. pub fn last_turn_telemetry(&self) -> &TurnTelemetry { - &self.last_turn_telemetry + &self.report.last_turn_telemetry } /// Actual deterministic verification executions retained for the latest /// turn, including failed turns that ended during later reconciliation or /// provider recovery. pub fn last_verification_executions(&self) -> &[crate::VerificationExecution] { - &self.last_turn_telemetry.verification_executions + &self.report.last_turn_telemetry.verification_executions } /// Typed outcome of the most recent successfully finalized turn. pub fn last_turn_outcome(&self) -> Option<&crate::TurnOutcome> { - self.last_turn_outcome.as_ref() + self.report.last_turn_outcome.as_ref() } pub fn last_effective_route(&self) -> &crate::EffectiveModelRoute { - &self.last_effective_route + &self.report.last_effective_route } /// Provider label supplied by the frontend for the effective route. diff --git a/crates/hi-agent/src/agent/memory_turn.rs b/crates/hi-agent/src/agent/memory_turn.rs index 0d6dd21b..61e694f1 100644 --- a/crates/hi-agent/src/agent/memory_turn.rs +++ b/crates/hi-agent/src/agent/memory_turn.rs @@ -71,7 +71,7 @@ impl crate::Agent { // in a long session may have been incomplete or unverified, so do not // replay them into the distiller. Explicit user corrections/preferences // remain eligible independently. - let verified_turn = self.last_turn_outcome.as_ref().is_some_and(|outcome| { + let verified_turn = self.report.last_turn_outcome.as_ref().is_some_and(|outcome| { outcome.status == TurnStatus::Completed && outcome.verification == VerificationStatus::Passed && outcome.review != ReviewStatus::Objected diff --git a/crates/hi-agent/src/agent/skeptic.rs b/crates/hi-agent/src/agent/skeptic.rs index ddc55758..3d48ebc4 100644 --- a/crates/hi-agent/src/agent/skeptic.rs +++ b/crates/hi-agent/src/agent/skeptic.rs @@ -175,15 +175,15 @@ impl crate::Agent { .map(|n| format!("\n — {n}")) .collect::() }; - let verify = match self.last_verify { + let verify = match self.report.last_verify { Some(true) => "verify result: PASSED", Some(false) => "verify result: FAILED", None => "verify result: (none configured)", }; - let files = if self.last_changed_files.is_empty() { + let files = if self.workspace.last_changed_files.is_empty() { "(none detected)".to_string() } else { - self.last_changed_files.join(", ") + self.workspace.last_changed_files.join(", ") }; let stub_findings = self.turn_stub_scan(); let stubs = if stub_findings.is_empty() { @@ -305,12 +305,12 @@ impl crate::Agent { /// A reconcile that moves the revision makes the cache miss, never stale. pub(crate) async fn turn_diff(&mut self) -> String { let revision = self.runtime.ledger().revision(); - if let Some((cached_revision, diff)) = &self.turn_diff_cache + if let Some((cached_revision, diff)) = &self.workspace.turn_diff_cache && *cached_revision == revision { return diff.clone(); } - let diff = match self.checkpoints.last() { + let diff = match self.workspace.checkpoints.last() { Some(target) => hi_tools::checkpoint::diff_with_state( self.runtime.root(), target, @@ -320,7 +320,7 @@ impl crate::Agent { .unwrap_or_default(), None => String::new(), }; - self.turn_diff_cache = Some((revision, diff.clone())); + self.workspace.turn_diff_cache = Some((revision, diff.clone())); diff } @@ -329,14 +329,14 @@ impl crate::Agent { /// completion audit scan the same paths, and the scan reads each file. pub(crate) fn turn_stub_scan(&mut self) -> Vec { let revision = self.runtime.ledger().revision(); - if let Some((cached_revision, findings)) = &self.turn_stub_scan_cache + if let Some((cached_revision, findings)) = &self.workspace.turn_stub_scan_cache && *cached_revision == revision { return findings.clone(); } let findings = - hi_tools::stub_scan::scan_paths(self.runtime.root(), &self.last_changed_files, 50); - self.turn_stub_scan_cache = Some((revision, findings.clone())); + hi_tools::stub_scan::scan_paths(self.runtime.root(), &self.workspace.last_changed_files, 50); + self.workspace.turn_stub_scan_cache = Some((revision, findings.clone())); findings } } diff --git a/crates/hi-agent/src/agent/turn/fast_feedback.rs b/crates/hi-agent/src/agent/turn/fast_feedback.rs index 5092966f..4f57964b 100644 --- a/crates/hi-agent/src/agent/turn/fast_feedback.rs +++ b/crates/hi-agent/src/agent/turn/fast_feedback.rs @@ -11,7 +11,7 @@ use std::collections::BTreeSet; use std::path::PathBuf; -use hi_tools::{ +use hi_tools::infra::{ CargoCommandOutcome, affected_any_package_dirs, format_lsp_error_feedback, lsp_source_paths, run_affected_cargo_checks, run_affected_cargo_tests, run_affected_polyglot_checks, run_affected_polyglot_tests, rust_source_paths, diff --git a/crates/hi-agent/src/agent/turn/finalize.rs b/crates/hi-agent/src/agent/turn/finalize.rs index 87ef293e..4d75ea71 100644 --- a/crates/hi-agent/src/agent/turn/finalize.rs +++ b/crates/hi-agent/src/agent/turn/finalize.rs @@ -106,18 +106,18 @@ impl crate::Agent { // like "what's your name?" appear to be a 1.5k-token user prompt. let mut summary = format!( "[user prompt estimate {} · output across all model calls {}{}", - humanize_count(self.last_user_prompt_tokens), - if self.last_turn_usage.estimated { + humanize_count(self.report.last_user_prompt_tokens), + if self.report.last_turn_usage.estimated { "~" } else { "" }, - humanize_count(self.last_turn_usage.output_tokens), + humanize_count(self.report.last_turn_usage.output_tokens), ); - if self.last_turn_usage.cache_read_tokens > 0 { + if self.report.last_turn_usage.cache_read_tokens > 0 { summary.push_str(&format!( " ⟲{}", - humanize_count(self.last_turn_usage.cache_read_tokens) + humanize_count(self.report.last_turn_usage.cache_read_tokens) )); } // The context gauge is the point-in-time full request size, which is @@ -126,26 +126,26 @@ impl crate::Agent { if let Some(window) = self.config.routing.context_window && window > 0 { - let pct = (self.context_used * 100 / u64::from(window)).min(100); + let pct = (self.report.context_used * 100 / u64::from(window)).min(100); summary.push_str(&format!( " · ctx {}{pct}% ({}/{})", - if self.last_turn_usage.estimated { + if self.report.last_turn_usage.estimated { "~" } else { "" }, - humanize_count(self.context_used), + humanize_count(self.report.context_used), humanize_count(u64::from(window)), )); - } else if self.context_used > 0 { + } else if self.report.context_used > 0 { summary.push_str(&format!( " · ctx {}{}", - if self.last_turn_usage.estimated { + if self.report.last_turn_usage.estimated { "~" } else { "" }, - humanize_count(self.context_used) + humanize_count(self.report.context_used) )); } if let Some(limits) = usage.rate_limits.and_then(rate_limit_summary) { @@ -166,7 +166,7 @@ impl crate::Agent { /// turn was clean (no extra rounds of any kind, no stall). Format: /// `steer: 2 verify · 1 retry · stalled` — components omitted when zero. pub(crate) fn turn_steer(&self) -> Option { - let t = &self.last_turn_telemetry; + let t = &self.report.last_turn_telemetry; let mut parts: Vec = Vec::new(); if t.verify_rounds > 0 { parts.push(format!("{} verify", t.verify_rounds)); diff --git a/crates/hi-agent/src/agent/turn/loop_.rs b/crates/hi-agent/src/agent/turn/loop_.rs index ffbe067f..a23b8ced 100644 --- a/crates/hi-agent/src/agent/turn/loop_.rs +++ b/crates/hi-agent/src/agent/turn/loop_.rs @@ -49,6 +49,7 @@ impl crate::Agent { /// [`TurnPhase::Done`]. pub async fn run_turn(&mut self, input: &str, ui: &mut dyn Ui) -> Result { // Always land on Done, including `?` error exits mid-turn. + // Phase stamps inside the body are validated by TurnPhase::can_transition_to. let result = self.run_turn_body(input, ui).await; self.set_turn_phase(TurnPhase::Done); result @@ -65,15 +66,18 @@ impl crate::Agent { self.runtime.clear_read_cache(); // Same per-turn contract for the diff / stub-scan caches: a new turn // recomputes both against its own baseline. - self.turn_diff_cache = None; - self.turn_stub_scan_cache = None; + self.workspace.turn_diff_cache = None; + self.workspace.turn_stub_scan_cache = None; // Reconcile user/external edits before establishing this turn's // baseline so they are not attributed to the agent. self.runtime.ledger().reconcile()?; let turn_ledger_revision = self.runtime.ledger().revision(); - self.active_turn_ledger_revision = Some(turn_ledger_revision); - self.active_turn_message_start = None; + self.workspace.active_turn_ledger_revision = Some(turn_ledger_revision); + self.workspace.active_turn_message_start = None; let turn_background_baseline = self.runtime.background().ids(); + // Retained on the agent so failed/cancelled finalizers can kill only + // processes this turn started even when the frontend drops the future. + self.workspace.active_turn_background_baseline = Some(turn_background_baseline.clone()); let expanded_input = command::expand_prompt_macro(input).unwrap_or_else(|| input.to_string()); // Synthetic goal-drive text is only transport. Contracts, context @@ -99,7 +103,7 @@ impl crate::Agent { let repository_context_enabled = task_needs_repository_context(&context_task, &task_contract); let mut ranked_context_paths = self - .last_changed_files + .workspace.last_changed_files .iter() .cloned() .collect::>(); @@ -113,7 +117,7 @@ impl crate::Agent { ranked_context_paths.insert(path); } } - self.task_context = repository_context_enabled + self.task.task_context = repository_context_enabled .then(|| { let index = crate::context_index::build_task_context_index( self.runtime.root(), @@ -154,8 +158,8 @@ impl crate::Agent { task_contract.intent = TaskIntent::ReadOnly; task_contract.explicit_mutation = false; } - self.last_task_contract = Some(task_contract.clone()); - self.last_task_prompt = Some(context_task.clone()); + self.task.last_task_contract = Some(task_contract.clone()); + self.task.last_task_prompt = Some(context_task.clone()); self.refresh_system_message(); // A turn is *expected* to mutate — and ends "incomplete · stalled" // when it changes no files — only for an explicit mutation request @@ -195,8 +199,8 @@ impl crate::Agent { _ => turn_input.clone(), }; self.reset_last_turn_usage(user_prompt_tokens); - self.last_turn_outcome = None; - self.last_effective_route = effective_model_route(&self.config, None); + self.report.last_turn_outcome = None; + self.report.last_effective_route = effective_model_route(&self.config, None); // A top-level session the user restricted to ChatOnly/ReadOnly gets a // clear early "your mode blocks edits" error when the prompt clearly asks @@ -210,11 +214,11 @@ impl crate::Agent { && !self.config.subagents.is_subagent && self.tools_unavailable_for(input) { - self.last_verify = None; - self.last_changed_files.clear(); - self.last_file_changes.clear(); - self.last_compat_fallbacks.clear(); - self.last_turn_telemetry = TurnTelemetry::default(); + self.report.last_verify = None; + self.workspace.last_changed_files.clear(); + self.workspace.last_file_changes.clear(); + self.report.last_compat_fallbacks.clear(); + self.report.last_turn_telemetry = TurnTelemetry::default(); let preserve_plan = (goal_drive_turn || looks_like_continue(&context_task)) && self.goals.plan_incomplete(); if self.goals.clear_plan_unless(preserve_plan) { @@ -243,9 +247,8 @@ impl crate::Agent { verified_workspace_revision: None, effective_route: effective_model_route(&self.config, None), }; - self.last_turn_outcome = Some(outcome.clone()); - self.active_turn_ledger_revision = None; - self.active_turn_message_start = None; + self.report.last_turn_outcome = Some(outcome.clone()); + self.workspace.clear_active_baselines(); return Ok(outcome); } let mut turn_checkpoint_allowed = None; @@ -265,11 +268,11 @@ impl crate::Agent { if self.config.memory.auto_compact && let Some(window) = self.config.routing.context_window && window > 0 - && self.context_used * 100 >= u64::from(window) * self.config.memory.auto_compact_percent + && self.report.context_used * 100 >= u64::from(window) * self.config.memory.auto_compact_percent { ui.status(&format!( "context ~{}% full — compacting to free room", - self.context_used * 100 / u64::from(window) + self.report.context_used * 100 / u64::from(window) )); // Tier 1: deterministic, no model call. Only old turns are eligible. if let Some(split) = @@ -284,19 +287,19 @@ impl crate::Agent { if compaction::estimate_tokens(self.messages.as_slice()) > target { let _ = self.compact(ui).await; } - self.context_used = 0; + self.report.context_used = 0; } self.messages.strip_trailing_nudges(); self.persisted = self.persisted.min(self.messages.len()); let mut turn_start = self.messages.len(); - self.active_turn_message_start = Some(turn_start); + self.workspace.active_turn_message_start = Some(turn_start); self.messages.push_user_or_fold(&model_turn_input); - self.last_verify = None; - self.last_changed_files.clear(); - self.last_file_changes.clear(); - self.last_compat_fallbacks.clear(); - self.last_turn_telemetry.verification_executions.clear(); + self.report.last_verify = None; + self.workspace.last_changed_files.clear(); + self.workspace.last_file_changes.clear(); + self.report.last_compat_fallbacks.clear(); + self.report.last_turn_telemetry.verification_executions.clear(); // Preserve only an unfinished plan that the user explicitly continues. // Clearing must also be emitted: the TUI owns a pinned copy and cannot // infer that the agent cleared its internal state. @@ -632,7 +635,7 @@ impl crate::Agent { // reconciliation or persistence can still fail after a successful // check, and reports for those error turns need the stages that // actually ran. - self.last_turn_telemetry.verification_executions = verifier.executions().to_vec(); + self.report.last_turn_telemetry.verification_executions = verifier.executions().to_vec(); match self .handle_workspace_repair_outcome( outcome, @@ -698,7 +701,7 @@ impl crate::Agent { } }; super::settlement::reconcile_verified_revision( - &mut self.last_verify, + &mut self.report.last_verify, &mut verified_at, &mut independent_review_status, final_ledger_revision, @@ -707,17 +710,17 @@ impl crate::Agent { ui, ); } - self.last_changed_files = ledger_changes + self.workspace.last_changed_files = ledger_changes .iter() .map(|change| change.path.clone()) .collect(); - self.last_file_changes = ledger_changes; - self.last_compat_fallbacks = compat_fallbacks; + self.workspace.last_file_changes = ledger_changes; + self.report.last_compat_fallbacks = compat_fallbacks; // Flush the per-turn counters (otherwise discarded locals) into // telemetry so `--report` / the eval harness can diagnose the turn's // trajectory: how many verify rounds, recovery retries, nudges fired, // and where the last verify failure pointed. - self.last_turn_telemetry = build_turn_telemetry( + self.report.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), empty_retries, @@ -737,26 +740,26 @@ impl crate::Agent { &evidence, &review_repair, ); - self.last_turn_telemetry.checkpoint_available = + self.report.last_turn_telemetry.checkpoint_available = turn_checkpoint_allowed.map(|_| turn_checkpoint_created); - self.last_turn_telemetry.advertised_tools = advertised_tool_names.into_iter().collect(); - self.last_turn_telemetry.tool_schema_tokens = tool_schema_tokens; + self.report.last_turn_telemetry.advertised_tools = advertised_tool_names.into_iter().collect(); + self.report.last_turn_telemetry.tool_schema_tokens = tool_schema_tokens; // Verifier-gated skill auto-curation: after a turn that PASSED verification // and actually changed files, optionally distill a reusable technique into a // learned skill. The ground-truth verifier is the gate (safe with weak local // models); opt-in via `curate_skills`, and capped per session. if self.config.memory.curate_skills - && self.last_verify == Some(true) - && !self.last_changed_files.is_empty() - && self.auto_skills_written < super::super::MAX_AUTO_SKILLS_PER_SESSION + && self.report.last_verify == Some(true) + && !self.workspace.last_changed_files.is_empty() + && self.subagents.auto_skills_written < super::super::MAX_AUTO_SKILLS_PER_SESSION { self.curate_turn_end(turn_start, ui).await; } // Phase K: always-on (cheap, no model call) coding-fact extraction into // the decision log + project memory after a green file-changing turn. - if self.last_verify == Some(true) && !self.last_changed_files.is_empty() { + if self.report.last_verify == Some(true) && !self.workspace.last_changed_files.is_empty() { self.record_coding_facts_turn_end(ui); } @@ -764,8 +767,8 @@ impl crate::Agent { // without needing /diff. Skipped for read-only/Q&A turns (empty list). // Emitted BEFORE the finalize recap so the recap is the last text the // user sees (the "✓ done" marker follows it). - if !self.last_changed_files.is_empty() { - ui.changed_files(&self.last_changed_files); + if !self.workspace.last_changed_files.is_empty() { + ui.changed_files(&self.workspace.last_changed_files); } // TurnPhase::Finalize — optional tool-free recap after mutating turns. @@ -777,7 +780,7 @@ impl crate::Agent { && !flags.ended_at_cap && !flags.stalled_unfinished && !flags.stalled_repeating - && !self.last_changed_files.is_empty() + && !self.workspace.last_changed_files.is_empty() && steps < max_steps { self.finalize_turn(turn_start, ui).await; @@ -810,7 +813,7 @@ impl crate::Agent { } }; super::settlement::reconcile_verified_revision( - &mut self.last_verify, + &mut self.report.last_verify, &mut verified_at, &mut independent_review_status, settled_revision, @@ -819,11 +822,11 @@ impl crate::Agent { ui, ); } - self.last_changed_files = settled_changes + self.workspace.last_changed_files = settled_changes .iter() .map(|change| change.path.clone()) .collect(); - self.last_file_changes = settled_changes; + self.workspace.last_file_changes = settled_changes; // Long-horizon progress happens only after the final settled revision // still matches deterministic verification. @@ -874,7 +877,7 @@ impl crate::Agent { let mut ledger = self.runtime.ledger(); (ledger.revision(), ledger.workspace_revision()) }; - let changed_after_final_hooks = self.last_verify == Some(true) + let changed_after_final_hooks = self.report.last_verify == Some(true) && verified_at.as_ref().is_none_or(|(revision, digest)| { *revision != outcome_revision || digest != &outcome_digest }); @@ -887,7 +890,7 @@ impl crate::Agent { } }; let wiped = super::settlement::reconcile_verified_revision_with_message( - &mut self.last_verify, + &mut self.report.last_verify, &mut verified_at, &mut independent_review_status, outcome_revision, @@ -923,11 +926,11 @@ impl crate::Agent { ledger.had_mutation_since(turn_ledger_revision), ) }; - self.last_changed_files = final_changes + self.workspace.last_changed_files = final_changes .iter() .map(|change| change.path.clone()) .collect(); - self.last_file_changes = final_changes; + self.workspace.last_file_changes = final_changes; // `Unverified` is reserved for "checks should have run but did not // settle" (budget exhausted after a fail, post-pass code mutation, etc.). @@ -936,16 +939,16 @@ impl crate::Agent { // `NotApplicable` ("no applicable checks"), not a scary incomplete // "unverified changes" warning. Users still get `Unverified` when a // check was expected and missing. - let no_check_executed = self.last_turn_telemetry.verification_executions.is_empty(); + let no_check_executed = self.report.last_turn_telemetry.verification_executions.is_empty(); let (status, verification, review, stop_reason) = super::finalize::classify_turn_outcome( verification_infrastructure_error, verification_unstable, - self.last_verify, - &self.last_changed_files, + self.report.last_verify, + &self.workspace.last_changed_files, turn_had_mutation, no_check_executed, independent_review_status, - self.last_turn_telemetry.skeptic_last_status, + self.report.last_turn_telemetry.skeptic_last_status, flags.ended_at_cap, flags.stalled_unfinished, flags.stalled_repeating, @@ -959,7 +962,7 @@ impl crate::Agent { verification, review, stop_reason, - changed_files: self.last_changed_files.clone(), + changed_files: self.workspace.last_changed_files.clone(), verified_workspace_revision: (verification == VerificationStatus::Passed) .then(|| verified_at.as_ref().map(|(_, digest)| digest.clone())) .flatten(), @@ -968,10 +971,9 @@ impl crate::Agent { effective_fallback_route.as_deref(), ), }; - self.last_effective_route = outcome.effective_route.clone(); - self.last_turn_outcome = Some(outcome.clone()); - self.active_turn_ledger_revision = None; - self.active_turn_message_start = None; + self.report.last_effective_route = outcome.effective_route.clone(); + self.report.last_turn_outcome = Some(outcome.clone()); + self.workspace.clear_active_baselines(); Ok(outcome) } } diff --git a/crates/hi-agent/src/agent/turn/model_retry.rs b/crates/hi-agent/src/agent/turn/model_retry.rs index daea5546..1d5b2228 100644 --- a/crates/hi-agent/src/agent/turn/model_retry.rs +++ b/crates/hi-agent/src/agent/turn/model_retry.rs @@ -205,8 +205,8 @@ impl crate::Agent { self.add_error_usage(&err); self.reconcile_error_turn_changes(turn_ledger_revision)?; self.emit_usage(ui); - self.last_compat_fallbacks = compat_fallbacks.clone(); - self.last_turn_telemetry = build_turn_telemetry( + self.report.last_compat_fallbacks = compat_fallbacks.clone(); + self.report.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), *empty_retries, @@ -229,7 +229,7 @@ impl crate::Agent { let _ = self.persist(); let (kind, guidance) = crate::ui::classify_error(&err); ui.turn_error(kind, &err.to_string(), guidance); - self.last_effective_route = effective_model_route( + self.report.last_effective_route = effective_model_route( &self.config, effective_fallback_route.as_deref(), ); @@ -350,12 +350,12 @@ impl crate::Agent { self.add_error_usage(&err); self.reconcile_error_turn_changes(turn_ledger_revision)?; self.emit_usage(ui); - if self.last_changed_files.is_empty() + if self.workspace.last_changed_files.is_empty() && let Some(turn_snapshot) = turn_snapshot.as_ref() { self.messages.strip_trailing_nudges(); if let Ok(end_snapshot) = self.snapshot_cached().await { - self.last_changed_files = + self.workspace.last_changed_files = changed_files_between(turn_snapshot, &end_snapshot); } } @@ -366,8 +366,8 @@ impl crate::Agent { if !made_tool_call { self.truncate_messages(*turn_start); } - self.last_compat_fallbacks = compat_fallbacks.clone(); - self.last_turn_telemetry = build_turn_telemetry( + self.report.last_compat_fallbacks = compat_fallbacks.clone(); + self.report.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), *empty_retries, @@ -390,7 +390,7 @@ impl crate::Agent { let _ = self.persist(); let (kind, guidance) = crate::ui::classify_error(&err); ui.turn_error(kind, &err.to_string(), guidance); - self.last_effective_route = effective_model_route( + self.report.last_effective_route = effective_model_route( &self.config, effective_fallback_route.as_deref(), ); diff --git a/crates/hi-agent/src/agent/turn/model_round.rs b/crates/hi-agent/src/agent/turn/model_round.rs index dcfb93c9..23b9fc2e 100644 --- a/crates/hi-agent/src/agent/turn/model_round.rs +++ b/crates/hi-agent/src/agent/turn/model_round.rs @@ -331,8 +331,8 @@ impl crate::Agent { self.truncate_messages(turn_start); self.add_error_usage(&err); self.emit_usage(ui); - self.last_compat_fallbacks = compat_fallbacks.clone(); - self.last_turn_telemetry = build_turn_telemetry( + self.report.last_compat_fallbacks = compat_fallbacks.clone(); + self.report.last_turn_telemetry = build_turn_telemetry( max_steps, verifier.round(), empty_retries, @@ -355,7 +355,7 @@ impl crate::Agent { let _ = self.persist(); let (kind, guidance) = crate::ui::classify_error(&err); ui.turn_error(kind, &err.to_string(), guidance); - self.last_effective_route = effective_model_route( + self.report.last_effective_route = effective_model_route( &self.config, effective_fallback_route.as_deref(), ); diff --git a/crates/hi-agent/src/agent/turn/phase.rs b/crates/hi-agent/src/agent/turn/phase.rs index dca72421..dac6f146 100644 --- a/crates/hi-agent/src/agent/turn/phase.rs +++ b/crates/hi-agent/src/agent/turn/phase.rs @@ -1,9 +1,9 @@ -//! Explicit turn-loop phases. +//! Explicit turn-loop phases and legal transitions. //! -//! `run_turn` stamps [`TurnPhase`] onto [`crate::Agent::turn_phase`] at every -//! control-flow boundary. The public getter is read by the TUI debug panel and -//! is safe after errors: the outer `run_turn` wrapper always finishes on -//! [`TurnPhase::Done`]. +//! `run_turn` advances through [`TurnPhase`] via [`crate::Agent::set_turn_phase`] +//! at every control-flow boundary. Illegal transitions panic in debug builds and +//! are logged + clamped in release so a bad stamp never silently corrupts the +//! TUI debug panel. //! //! Pipeline (see `docs/architecture.md`): //! @@ -11,6 +11,12 @@ //! Setup → ( Model → Tools → Steer )* → WorkspaceRepair → Settle → Finalize → Done //! ``` //! +//! Re-entry edges (not a pure DAG): +//! - Steer → Model (quality / unfinished continue) +//! - Tools → Steer (always after a batch) +//! - WorkspaceRepair → Model (failed verify / coding obligation) +//! - Any → Done (outer `run_turn` wrapper, including `?` exits) +//! //! Two distinct "repair" concepts touch this pipeline: //! - [`TurnPhase::WorkspaceRepair`] — compile/lint/test via //! [`crate::verify::WorkspaceRepairVerifier`]; failures re-enter Model. @@ -54,18 +60,66 @@ impl TurnPhase { Self::Done => "done", } } + + /// Whether `next` is a legal successor of `self` in the turn state machine. + /// + /// `Done` is absorbing and reachable from every phase so the outer + /// `run_turn` wrapper can always land there after `?` exits. A fresh turn + /// may also restart at `Setup` from `Done` (or the default) between calls. + pub fn can_transition_to(self, next: Self) -> bool { + use TurnPhase::*; + if self == next { + // Idempotent stamps (e.g. Tools set in loop_ and again in execute_tool_batch). + return true; + } + if next == Done { + return true; + } + match self { + Setup => matches!(next, Model | Settle | Done), + // Early tool-mode denial / empty turns skip the model loop and go + // straight toward settlement classification (via Done from outer). + Model => matches!(next, Tools | Steer | Model | WorkspaceRepair | Settle | Done), + Tools => matches!(next, Steer | Model | Tools | Done), + Steer => matches!(next, Model | Tools | WorkspaceRepair | Settle | Done), + WorkspaceRepair => matches!(next, Model | Settle | WorkspaceRepair | Done), + Settle => matches!(next, Finalize | Done), + Finalize => matches!(next, Done), + Done => matches!(next, Setup | Done), + } + } } impl crate::Agent { /// Record the active turn phase (debug panel + tests). + /// + /// Enforces [`TurnPhase::can_transition_to`]. Illegal transitions are a + /// logic bug: panic in debug, warn + apply in release so UI never freezes + /// on a stale phase after a missed edge. #[inline] pub(crate) fn set_turn_phase(&mut self, phase: TurnPhase) { - self.turn_phase = phase; + let from = self.report.turn_phase; + if !from.can_transition_to(phase) { + debug_assert!( + false, + "illegal turn phase transition {} → {}", + from.label(), + phase.label() + ); + // Release builds still apply so the TUI never freezes on a stale + // phase after a missed edge; the assert catches it in tests/dev. + eprintln!( + "hi-agent: illegal turn phase transition {} → {}", + from.label(), + phase.label() + ); + } + self.report.turn_phase = phase; } /// Phase of the in-flight or most recently finished turn. pub fn turn_phase(&self) -> TurnPhase { - self.turn_phase + self.report.turn_phase } } @@ -112,4 +166,58 @@ mod tests { fn done_is_terminal_label() { assert_eq!(TurnPhase::Done.label(), "done"); } + + #[test] + fn happy_path_transitions_are_legal() { + use TurnPhase::*; + let path = [ + Setup, Model, Tools, Steer, Model, Tools, Steer, WorkspaceRepair, Settle, Finalize, + Done, + ]; + for window in path.windows(2) { + assert!( + window[0].can_transition_to(window[1]), + "{} → {}", + window[0].label(), + window[1].label() + ); + } + } + + #[test] + fn reentry_and_escape_edges() { + use TurnPhase::*; + assert!(Steer.can_transition_to(Model)); + assert!(WorkspaceRepair.can_transition_to(Model)); + assert!(WorkspaceRepair.can_transition_to(Settle)); + assert!(Model.can_transition_to(Done)); + assert!(Tools.can_transition_to(Done)); + assert!(Done.can_transition_to(Setup)); + assert!(Setup.can_transition_to(Settle)); // early-exit turns + } + + #[test] + fn illegal_edges_are_rejected() { + use TurnPhase::*; + assert!(!Finalize.can_transition_to(Model)); + assert!(!Settle.can_transition_to(Tools)); + assert!(!Done.can_transition_to(Model)); + assert!(!Setup.can_transition_to(Tools)); + } + + #[test] + fn idempotent_stamps_are_legal() { + for phase in [ + TurnPhase::Setup, + TurnPhase::Model, + TurnPhase::Tools, + TurnPhase::Steer, + TurnPhase::WorkspaceRepair, + TurnPhase::Settle, + TurnPhase::Finalize, + TurnPhase::Done, + ] { + assert!(phase.can_transition_to(phase), "{}", phase.label()); + } + } } diff --git a/crates/hi-agent/src/agent/turn/setup.rs b/crates/hi-agent/src/agent/turn/setup.rs index 537933b0..457f1089 100644 --- a/crates/hi-agent/src/agent/turn/setup.rs +++ b/crates/hi-agent/src/agent/turn/setup.rs @@ -73,8 +73,8 @@ impl crate::Agent { (Some(seed), None) => Some(seed), (None, index) => index, }; - if refreshed != self.task_context { - self.task_context = refreshed; + if refreshed != self.task.task_context { + self.task.task_context = refreshed; } } @@ -91,8 +91,8 @@ impl crate::Agent { pub(super) fn reconcile_error_turn_changes(&mut self, turn_revision: u64) -> Result<()> { self.reconcile_workspace_changes()?; let changes = self.runtime.ledger().changes_since(turn_revision); - self.last_changed_files = changes.iter().map(|change| change.path.clone()).collect(); - self.last_file_changes = changes; + self.workspace.last_changed_files = changes.iter().map(|change| change.path.clone()).collect(); + self.workspace.last_file_changes = changes; Ok(()) } @@ -128,7 +128,7 @@ impl crate::Agent { .await { hi_tools::checkpoint::CreateResult::Created(sha) => { - let mut next = self.checkpoints.clone(); + let mut next = self.workspace.checkpoints.clone(); next.push(sha); if next.len() > crate::MAX_CHECKPOINTS { next.drain(0..next.len() - crate::MAX_CHECKPOINTS); @@ -140,7 +140,7 @@ impl crate::Agent { "checkpoint was created but its reference could not be persisted: {err:#}" ) } else { - self.checkpoints = next; + self.workspace.checkpoints = next; *checkpoint_created = true; *checkpoint_allowed = Some(true); return true; @@ -163,7 +163,7 @@ impl crate::Agent { /// `/undo` will refuse this record if an editor or another process changes /// any tracked path after the turn completes. pub(super) async fn seal_turn_checkpoint(&mut self, ui: &mut dyn Ui) -> Result { - let Some(target) = self.checkpoints.last().cloned() else { + let Some(target) = self.workspace.checkpoints.last().cloned() else { return Ok(false); }; match hi_tools::checkpoint::create_detailed_with_state( @@ -174,11 +174,11 @@ impl crate::Agent { { hi_tools::checkpoint::CreateResult::Created(expected_current) => { let sealed = hi_tools::checkpoint::sealed_reference(&target, &expected_current); - if let Some(last) = self.checkpoints.last_mut() { + if let Some(last) = self.workspace.checkpoints.last_mut() { *last = sealed; } if let Some(session) = self.session.as_mut() { - session.record_checkpoints(&self.checkpoints)?; + session.record_checkpoints(&self.workspace.checkpoints)?; } Ok(true) } @@ -187,9 +187,9 @@ impl crate::Agent { // An unsealed 0.2 undo record could overwrite edits made after // this turn, so always drop it. Strict mode becomes incomplete; // YOLO continues silently and exposes the loss in telemetry. - self.checkpoints.pop(); + self.workspace.checkpoints.pop(); if let Some(session) = self.session.as_mut() { - session.record_checkpoints(&self.checkpoints)?; + session.record_checkpoints(&self.workspace.checkpoints)?; } if !self.config.gates.allow_no_checkpoint { ui.checkpoint_warning(&format!( diff --git a/crates/hi-agent/src/agent/turn/steer/cascade.rs b/crates/hi-agent/src/agent/turn/steer/cascade.rs new file mode 100644 index 00000000..a35ce5ce --- /dev/null +++ b/crates/hi-agent/src/agent/turn/steer/cascade.rs @@ -0,0 +1,340 @@ +//! Table-driven review quality-repair cascade. +//! +//! Order is frozen by [`crate::steering::REVIEW_QUALITY_CASCADE`]. This module +//! walks that table instead of an open-coded if-ladder so reorder/regressions +//! fail at the cascade constant + selector tests, not only in integration. + +use crate::config::ReviewRepairBudgets; +use crate::steering::{ + CONCRETE_REVIEW_NUDGE, EvidenceTracker, GAP_SEARCH_OVERCLAIM_NUDGE, READ_AFTER_SEARCH_NUDGE, + REVIEW_QUALITY_CASCADE, ReviewIntent, ReviewRepairMode, SECURITY_BROAD_SEARCH_NUDGE, + SECURITY_SCOPE_NUDGE, answer_says_insufficient_evidence, concrete_review_answer_problem, + deepen_review_nudge, no_evidence_review_nudge, should_deepen_review, + should_nudge_gap_search_overclaim, should_nudge_no_evidence_review, + should_nudge_read_after_search_final, should_nudge_security_broad_search, + should_nudge_security_scope, should_reject_review_repair_template, + summarize_inspected_evidence_nudge, +}; + +use super::super::retry::ReviewRepairState; + +/// What the quality cascade wants the Steer phase to do next. +#[derive(Debug)] +pub(super) enum QualityCascadeAction { + /// Spend budget and continue the model loop with a repair nudge. + Repair { + mode: ReviewRepairMode, + /// UI status / nudge line (short). + status: String, + /// Full nudge body (already includes required-next when applied). + nudge_body: String, + force_tools: bool, + force_text: bool, + /// When set, also `note` this mode (disclaimer chat-attempt accounting). + note_mode: Option, + /// When true, call `spend` on `mode`; when false, only bump quality counter. + spend: bool, + }, + /// Budget exhausted — stall incomplete. + Exhausted { + mode: ReviewRepairMode, + status: String, + }, +} + +/// Walk [`REVIEW_QUALITY_CASCADE`] and return the first applicable action. +/// +/// Returns `None` when no quality repair applies (caller emits the answer). +pub(super) fn select_review_quality_repair( + read_only_intent: Option, + evidence: &EvidenceTracker, + assistant_text: &str, + review_repair: &ReviewRepairState, + budgets: &ReviewRepairBudgets, +) -> Option { + // Special pre-step: insufficient-evidence-after-read can fire SecurityBroad + // *before* the disclaimer branch (historical order inside that arm). + if let Some(intent) = read_only_intent + && evidence.saw_read + && answer_says_insufficient_evidence(assistant_text) + { + if matches!(intent, ReviewIntent::Security) + && evidence.saw_search + && !evidence.security_search_complete() + && review_repair.has_budget(ReviewRepairMode::SecurityBroadSearch, budgets) + { + return Some(QualityCascadeAction::Repair { + mode: ReviewRepairMode::SecurityBroadSearch, + status: "security review gave a generic evidence disclaimer before searching all required pattern families; nudging the model to broaden the search".into(), + nudge_body: SECURITY_BROAD_SEARCH_NUDGE.to_string(), + force_tools: true, + force_text: false, + note_mode: None, + spend: true, + }); + } + // Fall through into cascade; InspectedDisclaimer predicate will match. + } + + for &mode in REVIEW_QUALITY_CASCADE { + if let Some(action) = + evaluate_cascade_mode(mode, read_only_intent, evidence, assistant_text, review_repair, budgets) + { + return Some(action); + } + } + None +} + +fn evaluate_cascade_mode( + mode: ReviewRepairMode, + read_only_intent: Option, + evidence: &EvidenceTracker, + assistant_text: &str, + review_repair: &ReviewRepairState, + budgets: &ReviewRepairBudgets, +) -> Option { + match mode { + ReviewRepairMode::NoEvidence => { + if !should_nudge_no_evidence_review(read_only_intent, evidence, assistant_text) { + return None; + } + let intent = read_only_intent?; + if review_repair.has_budget(mode, budgets) { + Some(QualityCascadeAction::Repair { + mode, + status: "review answer had no inspected evidence; nudging the model to inspect before answering".into(), + nudge_body: no_evidence_review_nudge(intent).to_string(), + force_tools: true, + force_text: false, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "review still had no inspected evidence after repair; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::InspectedDisclaimer | ReviewRepairMode::InspectedDisclaimerChatAttempt => { + // ChatAttempt is accounting-only; selection is driven by InspectedDisclaimer. + if mode == ReviewRepairMode::InspectedDisclaimerChatAttempt { + return None; + } + let intent = read_only_intent?; + if !(evidence.saw_read && answer_says_insufficient_evidence(assistant_text)) { + return None; + } + let chat_mode = ReviewRepairMode::InspectedDisclaimerChatAttempt; + let has_disclaimer_budget = review_repair.has_budget(mode, budgets); + let has_chat_attempt_budget = review_repair.has_budget(chat_mode, budgets); + if has_disclaimer_budget || has_chat_attempt_budget { + Some(QualityCascadeAction::Repair { + mode, + status: "review gave a generic evidence disclaimer after inspection; nudging the model to answer from inspected files".into(), + nudge_body: summarize_inspected_evidence_nudge(intent, evidence), + force_tools: false, + force_text: true, + note_mode: Some(chat_mode), + spend: has_disclaimer_budget, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "review kept returning a generic evidence disclaimer after inspection; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::GenericTemplate => { + let needs_evidence_depth_repair = evidence.listing_only() + || (evidence.saw_search && !evidence.saw_read) + || (matches!(read_only_intent, Some(ReviewIntent::Security)) + && evidence.saw_search + && !evidence.security_search_complete()); + if needs_evidence_depth_repair + || !should_reject_review_repair_template(read_only_intent, assistant_text) + { + return None; + } + let intent = read_only_intent?; + if review_repair.has_budget(mode, budgets) { + let has_inspected_evidence = evidence.saw_read || evidence.saw_search; + let nudge = if has_inspected_evidence { + summarize_inspected_evidence_nudge(intent, evidence) + } else { + deepen_review_nudge(intent).to_string() + }; + Some(QualityCascadeAction::Repair { + mode, + status: "review answer was a generic repair template; nudging the model to produce a concrete bounded review".into(), + nudge_body: nudge, + force_tools: !has_inspected_evidence, + force_text: has_inspected_evidence, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "review answer stayed generic after repair; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::ListingOnly => { + if !should_deepen_review(read_only_intent, evidence, assistant_text) { + return None; + } + let intent = read_only_intent?; + if review_repair.has_budget(mode, budgets) { + Some(QualityCascadeAction::Repair { + mode, + status: "review evidence was only a listing; nudging the model to inspect files or search results".into(), + nudge_body: deepen_review_nudge(intent).to_string(), + force_tools: true, + force_text: false, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "review still had only listing evidence after repair; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::ReadAfterSearch => { + if !should_nudge_read_after_search_final(read_only_intent, evidence, assistant_text) { + return None; + } + if review_repair.has_budget(mode, budgets) { + Some(QualityCascadeAction::Repair { + mode, + status: "review had targeted search but no file reads; nudging the model to read matching files".into(), + nudge_body: READ_AFTER_SEARCH_NUDGE.to_string(), + force_tools: true, + force_text: false, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "review still had targeted search but no file reads after repair; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::SecurityBroadSearch => { + // The insufficient-evidence special case may already have handled this. + if !should_nudge_security_broad_search(read_only_intent, evidence, assistant_text) { + return None; + } + if review_repair.has_budget(mode, budgets) { + Some(QualityCascadeAction::Repair { + mode, + status: "security review missed required pattern families; nudging the model to broaden the search".into(), + nudge_body: SECURITY_BROAD_SEARCH_NUDGE.to_string(), + force_tools: true, + force_text: false, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "security review still missed required pattern families after repair; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::SecurityScope => { + if !should_nudge_security_scope(read_only_intent, evidence, assistant_text) { + return None; + } + if review_repair.has_budget(mode, budgets) { + Some(QualityCascadeAction::Repair { + mode, + status: "security answer overclaimed repo-wide safety; nudging the model to bound findings to evidence".into(), + nudge_body: SECURITY_SCOPE_NUDGE.to_string(), + force_tools: false, + force_text: false, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "security answer still overclaimed after repair; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::GapSearchOverclaim => { + if !should_nudge_gap_search_overclaim(read_only_intent, evidence, assistant_text) { + return None; + } + if review_repair.has_budget(mode, budgets) { + Some(QualityCascadeAction::Repair { + mode, + status: "gap answer contradicted search matches; nudging the model to bound claims to inspected evidence".into(), + nudge_body: GAP_SEARCH_OVERCLAIM_NUDGE.to_string(), + force_tools: false, + force_text: false, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: "gap answer still overclaimed after search matches; stopping incomplete".into(), + }) + } + } + ReviewRepairMode::ConcreteAnswer => { + let problem = + concrete_review_answer_problem(read_only_intent, evidence, assistant_text)?; + if review_repair.has_budget(mode, budgets) { + Some(QualityCascadeAction::Repair { + mode, + status: problem.status().to_string(), + nudge_body: CONCRETE_REVIEW_NUDGE.to_string(), + force_tools: false, + force_text: true, + note_mode: None, + spend: true, + }) + } else { + Some(QualityCascadeAction::Exhausted { + mode, + status: problem.exhausted_status().to_string(), + }) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::steering::REVIEW_QUALITY_CASCADE; + + #[test] + fn selector_visits_cascade_in_spec_order() { + // The evaluator is consulted in REVIEW_QUALITY_CASCADE order; ChatAttempt + // is skipped as a primary step (accounting-only). + let primary: Vec<_> = REVIEW_QUALITY_CASCADE + .iter() + .copied() + .filter(|m| *m != ReviewRepairMode::InspectedDisclaimerChatAttempt) + .collect(); + assert_eq!(primary.first(), Some(&ReviewRepairMode::NoEvidence)); + assert_eq!(primary.last(), Some(&ReviewRepairMode::ConcreteAnswer)); + let idx = |m: ReviewRepairMode| primary.iter().position(|x| *x == m).unwrap(); + assert!(idx(ReviewRepairMode::NoEvidence) < idx(ReviewRepairMode::InspectedDisclaimer)); + assert!(idx(ReviewRepairMode::InspectedDisclaimer) < idx(ReviewRepairMode::GenericTemplate)); + assert!(idx(ReviewRepairMode::GenericTemplate) < idx(ReviewRepairMode::ListingOnly)); + assert!(idx(ReviewRepairMode::ListingOnly) < idx(ReviewRepairMode::ReadAfterSearch)); + assert!(idx(ReviewRepairMode::ReadAfterSearch) < idx(ReviewRepairMode::SecurityBroadSearch)); + assert!(idx(ReviewRepairMode::SecurityBroadSearch) < idx(ReviewRepairMode::SecurityScope)); + assert!(idx(ReviewRepairMode::SecurityScope) < idx(ReviewRepairMode::GapSearchOverclaim)); + assert!(idx(ReviewRepairMode::GapSearchOverclaim) < idx(ReviewRepairMode::ConcreteAnswer)); + } +} diff --git a/crates/hi-agent/src/agent/turn/steer/mod.rs b/crates/hi-agent/src/agent/turn/steer/mod.rs index fa09e507..5d1b4558 100644 --- a/crates/hi-agent/src/agent/turn/steer/mod.rs +++ b/crates/hi-agent/src/agent/turn/steer/mod.rs @@ -6,6 +6,7 @@ //! //! Workspace compile/lint/test repair stays in [`super::verify_run`]. +mod cascade; mod implementation; mod review; diff --git a/crates/hi-agent/src/agent/turn/steer/review.rs b/crates/hi-agent/src/agent/turn/steer/review.rs index 6c0a6330..bdbbe782 100644 --- a/crates/hi-agent/src/agent/turn/steer/review.rs +++ b/crates/hi-agent/src/agent/turn/steer/review.rs @@ -5,16 +5,10 @@ use hi_ai::Content; use crate::heuristics::looks_like_unfinished_step; use crate::steering::{ - CONCRETE_REVIEW_NUDGE, EvidenceTracker, GAP_SEARCH_OVERCLAIM_NUDGE, - IMPLEMENTATION_NO_CHANGES_NUDGE, IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE, ImplementationIntent, - ImplementationTracker, READ_AFTER_SEARCH_NUDGE, ReviewIntent, ReviewRepairMode, - SECURITY_BROAD_SEARCH_NUDGE, SECURITY_SCOPE_NUDGE, answer_says_insufficient_evidence, - concrete_review_answer_problem, deepen_review_nudge, implementation_missing_validation_nudge, - implementation_text_tool_nudge, no_evidence_review_nudge, repair_nudge_with_required_next, - should_deepen_review, should_nudge_gap_search_overclaim, should_nudge_no_evidence_review, - should_nudge_read_after_search_final, should_nudge_security_broad_search, - should_nudge_security_scope, should_reject_review_repair_template, - summarize_inspected_evidence_nudge, + EvidenceTracker, IMPLEMENTATION_NO_CHANGES_NUDGE, IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE, + ImplementationIntent, ImplementationTracker, ReviewIntent, + implementation_missing_validation_nudge, implementation_text_tool_nudge, + repair_nudge_with_required_next, summarize_inspected_evidence_nudge, }; use crate::transcript::NudgeKind; use crate::{PLAN_CONTINUE_NUDGE, SILENT_CONTINUE_NUDGE, Ui}; @@ -203,292 +197,52 @@ if implementation_intent.is_some() ui.status(INCOMPLETE_STATUS); return RoundControl::BreakInner(false); } -if should_nudge_no_evidence_review(read_only_intent, &evidence, assistant_text) -{ - let mode = ReviewRepairMode::NoEvidence; - if review_repair.spend(mode, evidence, budgets) { - *force_tools_next = true; - ui.nudge( - "review answer had no inspected evidence; nudging the model to inspect before answering", - ); - self.messages.push_assistant_repair_note(mode); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next( - mode, - no_evidence_review_nudge( - read_only_intent.expect("checked above"), - ), - ), - ); - return RoundControl::Continue; - } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.nudge( - "review still had no inspected evidence after repair; stopping incomplete", - ); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if let Some(intent) = read_only_intent - && evidence.saw_read - && answer_says_insufficient_evidence(assistant_text) -{ - if matches!(intent, ReviewIntent::Security) - && evidence.saw_search - && !evidence.security_search_complete() - && review_repair - .spend(ReviewRepairMode::SecurityBroadSearch, evidence, budgets) - { - *force_tools_next = true; - ui.nudge( - "security review gave a generic evidence disclaimer before searching all required pattern families; nudging the model to broaden the search", - ); - self.messages - .push_assistant_repair_note(ReviewRepairMode::SecurityBroadSearch); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next( - ReviewRepairMode::SecurityBroadSearch, - SECURITY_BROAD_SEARCH_NUDGE, - ), - ); - return RoundControl::Continue; - } - let mode = ReviewRepairMode::InspectedDisclaimer; - let chat_mode = ReviewRepairMode::InspectedDisclaimerChatAttempt; - let has_disclaimer_budget = review_repair.has_budget(mode, budgets); - let has_chat_attempt_budget = review_repair.has_budget(chat_mode, budgets); - if has_disclaimer_budget || has_chat_attempt_budget { - if has_disclaimer_budget { - review_repair.spend(mode, evidence, budgets); +// Table-driven review quality cascade (order = REVIEW_QUALITY_CASCADE). +match super::cascade::select_review_quality_repair( + read_only_intent, + evidence, + assistant_text, + review_repair, + budgets, +) { + Some(super::cascade::QualityCascadeAction::Repair { + mode, + status, + nudge_body, + force_tools, + force_text, + note_mode, + spend, + }) => { + if spend { + let _ = review_repair.spend(mode, evidence, budgets); } else { evidence.quality_repair_nudges = evidence.quality_repair_nudges.saturating_add(1); } - review_repair.note(chat_mode); - *force_text_answer_next = true; - *force_tools_next = false; - ui.nudge( - "review gave a generic evidence disclaimer after inspection; nudging the model to answer from inspected files", - ); - self.messages.push_assistant_repair_note(mode); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next( - mode, - summarize_inspected_evidence_nudge(intent, &evidence), - ), - ); - return RoundControl::Continue; - } - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.status( - "review kept returning a generic evidence disclaimer after inspection; stopping incomplete", - ); - let _ = (intent, &evidence); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -// (`saw_read` is implied here: the previous disjunct already -// catches search-without-read, so boolean-equivalently drop it.) -let needs_evidence_depth_repair = evidence.listing_only() - || (evidence.saw_search && !evidence.saw_read) - || (matches!(read_only_intent, Some(ReviewIntent::Security)) - && evidence.saw_search - && !evidence.security_search_complete()); -if !needs_evidence_depth_repair - && should_reject_review_repair_template(read_only_intent, assistant_text) -{ - if let Some(intent) = read_only_intent - && review_repair.spend(ReviewRepairMode::GenericTemplate, evidence, budgets) - { - let mode = ReviewRepairMode::GenericTemplate; - let has_inspected_evidence = evidence.saw_read || evidence.saw_search; - *force_text_answer_next = has_inspected_evidence; - *force_tools_next = !has_inspected_evidence; - ui.nudge( - "review answer was a generic repair template; nudging the model to produce a concrete bounded review", - ); - self.messages.push_assistant_repair_note(mode); - let nudge = if has_inspected_evidence { - summarize_inspected_evidence_nudge(intent, &evidence) - } else { - deepen_review_nudge(intent).to_string() - }; - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next(mode, nudge), - ); - return RoundControl::Continue; - } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(ReviewRepairMode::GenericTemplate); - progress_tracker.record(ProgressKind::None, reason, None); - ui.status("review answer stayed generic after repair; stopping incomplete"); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if should_deepen_review(read_only_intent, &evidence, assistant_text) { - let mode = ReviewRepairMode::ListingOnly; - if review_repair.spend(mode, evidence, budgets) { - *force_tools_next = true; - ui.nudge( - "review evidence was only a listing; nudging the model to inspect files or search results", - ); - self.messages.push_assistant_repair_note(mode); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next( - mode, - deepen_review_nudge(read_only_intent.expect("checked above")), - ), - ); - return RoundControl::Continue; - } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.nudge( - "review still had only listing evidence after repair; stopping incomplete", - ); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if should_nudge_read_after_search_final( - read_only_intent, - &evidence, - assistant_text, -) { - let mode = ReviewRepairMode::ReadAfterSearch; - if review_repair.spend(mode, evidence, budgets) { - *force_tools_next = true; - ui.nudge( - "review had targeted search but no file reads; nudging the model to read matching files", - ); - self.messages.push_assistant_repair_note(mode); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next(mode, READ_AFTER_SEARCH_NUDGE), - ); - return RoundControl::Continue; - } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.nudge( - "review still had targeted search but no file reads after repair; stopping incomplete", - ); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if should_nudge_security_broad_search( - read_only_intent, - &evidence, - assistant_text, -) { - let mode = ReviewRepairMode::SecurityBroadSearch; - if review_repair.spend(mode, evidence, budgets) { - *force_tools_next = true; - ui.nudge( - "security review missed required pattern families; nudging the model to broaden the search", - ); - self.messages.push_assistant_repair_note(mode); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next(mode, SECURITY_BROAD_SEARCH_NUDGE), - ); - return RoundControl::Continue; - } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.nudge( - "security review still missed required pattern families after repair; stopping incomplete", - ); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if should_nudge_security_scope(read_only_intent, &evidence, assistant_text) { - let mode = ReviewRepairMode::SecurityScope; - if review_repair.spend(mode, evidence, budgets) { - ui.status( - "security answer overclaimed repo-wide safety; nudging the model to bound findings to evidence", - ); - self.messages.push_assistant_repair_note(mode); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next(mode, SECURITY_SCOPE_NUDGE), - ); - return RoundControl::Continue; - } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.status( - "security answer still overclaimed after repair; stopping incomplete", - ); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if should_nudge_gap_search_overclaim( - read_only_intent, - &evidence, - assistant_text, -) { - let mode = ReviewRepairMode::GapSearchOverclaim; - if review_repair.spend(mode, evidence, budgets) { - ui.nudge( - "gap answer contradicted search matches; nudging the model to bound claims to inspected evidence", - ); + if let Some(note) = note_mode { + review_repair.note(note); + } + *force_tools_next = force_tools; + *force_text_answer_next = force_text; + ui.nudge(&status); + // Some modes use ui.status historically; keep nudge for all for visibility. self.messages.push_assistant_repair_note(mode); self.messages.push_nudge( NudgeKind::Continue, - repair_nudge_with_required_next(mode, GAP_SEARCH_OVERCLAIM_NUDGE), + repair_nudge_with_required_next(mode, nudge_body), ); return RoundControl::Continue; } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.nudge( - "gap answer still overclaimed after search matches; stopping incomplete", - ); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if let Some(problem) = - concrete_review_answer_problem(read_only_intent, &evidence, assistant_text) -{ - let mode = ReviewRepairMode::ConcreteAnswer; - if review_repair.spend(mode, evidence, budgets) { - *force_text_answer_next = true; - ui.nudge(problem.status()); - self.messages.push_assistant_repair_note(mode); - self.messages.push_nudge( - NudgeKind::Continue, - repair_nudge_with_required_next(mode, CONCRETE_REVIEW_NUDGE), - ); - return RoundControl::Continue; + Some(super::cascade::QualityCascadeAction::Exhausted { mode, status }) => { + *stalled_unfinished = true; + let reason = review_repair.exhausted(mode); + progress_tracker.record(ProgressKind::None, reason, None); + ui.nudge(&status); + ui.status(INCOMPLETE_STATUS); + return RoundControl::BreakInner(false); } - - *stalled_unfinished = true; - let reason = review_repair.exhausted(mode); - progress_tracker.record(ProgressKind::None, reason, None); - ui.nudge(problem.exhausted_status()); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); + None => {} } if buffer_read_only_review_text { let text_to_emit = if buffered_assistant_text.is_empty() { diff --git a/crates/hi-agent/src/agent/turn/tools.rs b/crates/hi-agent/src/agent/turn/tools.rs index bfea11ad..76bfdcb2 100644 --- a/crates/hi-agent/src/agent/turn/tools.rs +++ b/crates/hi-agent/src/agent/turn/tools.rs @@ -8,7 +8,7 @@ use std::collections::{BTreeMap, BTreeSet}; use anyhow::Result; use futures_util::StreamExt; -use hi_tools::{ +use hi_tools::protocol::{ execute_in_runtime, execute_prepared_in_runtime, execute_streaming_in_runtime, prepare_mutation_in_with_state, }; @@ -1049,7 +1049,7 @@ if !batch_mutated_paths.is_empty() { let paths = batch_mutated_paths.into_iter().collect::>(); let run_tests = task_contract.wants_tests || self - .last_task_contract + .task.last_task_contract .as_ref() .is_some_and(|c| c.wants_tests); let report = super::fast_feedback::run_fast_feedback( diff --git a/crates/hi-agent/src/agent/turn/verify_outcome.rs b/crates/hi-agent/src/agent/turn/verify_outcome.rs index 140a44fb..fc91cd7c 100644 --- a/crates/hi-agent/src/agent/turn/verify_outcome.rs +++ b/crates/hi-agent/src/agent/turn/verify_outcome.rs @@ -70,12 +70,12 @@ impl crate::Agent { }; if !*state.obligation_nudge_fired && let Some(reason) = super::obligation::coding_verify_obligation( - self.last_task_contract.as_ref(), + self.task.last_task_contract.as_ref(), &self.config.gates.verification, expected_mutation, &changed_now, mutation_now, - self.last_verify, + self.report.last_verify, verifier.executions().len(), ) { @@ -98,7 +98,7 @@ impl crate::Agent { } } } - if self.last_verify == Some(false) { + if self.report.last_verify == Some(false) { *state.stalled_unfinished = true; ui.status( "verification still failed after the retry budget; the task may be incomplete. /retry, or send 'continue'.", @@ -119,12 +119,12 @@ impl crate::Agent { .had_mutation_since(turn_ledger_revision); if !*state.obligation_nudge_fired && let Some(reason) = super::obligation::coding_verify_obligation( - self.last_task_contract.as_ref(), + self.task.last_task_contract.as_ref(), &self.config.gates.verification, expected_mutation, &[], mutation_now, - self.last_verify, + self.report.last_verify, verifier.executions().len(), ) { @@ -151,7 +151,7 @@ impl crate::Agent { } VerifyOutcome::Passed => { ui.status("✓ verification passed"); - self.last_verify = Some(true); + self.report.last_verify = Some(true); self.reconcile_workspace_changes()?; let (verified_revision, verified_digest, current_changes) = { let mut ledger = self.runtime.ledger(); @@ -173,7 +173,7 @@ impl crate::Agent { diff.lines().count() }; let (review_required, large_diff_review) = - self.last_task_contract.as_ref().map_or((false, false), |contract| { + self.task.last_task_contract.as_ref().map_or((false, false), |contract| { let required = contract.requires_review( self.config.gates.review, ¤t_files, @@ -198,11 +198,11 @@ impl crate::Agent { diff.push_str("\n… (bounded review diff truncated)"); } let contract = self - .last_task_contract + .task.last_task_contract .as_ref() .and_then(|contract| serde_json::to_string_pretty(contract).ok()) .unwrap_or_else(|| "(task contract unavailable)".into()); - let instructions = self.task_context.as_deref().unwrap_or("(none)"); + let instructions = self.task.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 ({file_count}):\n{files}\n\nDiff size: {diff_lines} lines\nDeterministic verification: PASSED\nStages: {stages}\nVerified workspace revision: {verified_digest}\n\nComplete bounded turn diff:\n{diff}", @@ -245,7 +245,7 @@ impl crate::Agent { { *state.independent_review_repairs = 1; *state.independent_review_status = ReviewStatus::Objected; - self.last_verify = None; + self.report.last_verify = None; *state.verified_at = None; verifier.allow_review_revalidation(); let headline = if large_diff_review { @@ -299,7 +299,7 @@ impl crate::Agent { round, } => { ui.status(&format!("✗ {} failed; iterating", stage.name)); - self.last_verify = Some(false); + self.report.last_verify = Some(false); *state.verified_at = None; let guidance = stage_guidance(&stage); // Structured failure: attributions + condensed output + optional @@ -332,7 +332,7 @@ impl crate::Agent { round, } => { *state.verification_infrastructure_error = true; - self.last_verify = None; + self.report.last_verify = None; *state.verified_at = None; ui.status(&format!( "verification infrastructure failed at {} (round {round}): {output}", @@ -347,7 +347,7 @@ impl crate::Agent { } => { *state.verification_unstable = true; *state.stalled_unfinished = true; - self.last_verify = Some(false); + self.report.last_verify = Some(false); *state.verified_at = None; ui.status(&format!( "verification is unstable in round {round}: stage {} modified {}", diff --git a/crates/hi-agent/src/agent/turn/verify_run.rs b/crates/hi-agent/src/agent/turn/verify_run.rs index d488f1eb..81e346c0 100644 --- a/crates/hi-agent/src/agent/turn/verify_run.rs +++ b/crates/hi-agent/src/agent/turn/verify_run.rs @@ -42,7 +42,7 @@ impl crate::Agent { } let baseline = self.ensure_turn_snapshot(turn_snapshot).await?; let pre_turn_checkpoint = turn_checkpoint_created - .then(|| self.checkpoints.last()) + .then(|| self.workspace.checkpoints.last()) .flatten() .and_then(|reference| { hi_tools::checkpoint::parse_reference(reference) diff --git a/crates/hi-agent/src/domain.rs b/crates/hi-agent/src/domain.rs index 11f685c5..b49b8746 100644 --- a/crates/hi-agent/src/domain.rs +++ b/crates/hi-agent/src/domain.rs @@ -1,14 +1,21 @@ //! Domain-scoped holders for cross-cutting `Agent` state. //! -//! These keep goals/plans and RSI observe-only fields grouped so the root -//! `Agent` surface reads as composition rather than a flat bag of peers. -//! Plan/snapshot mutations go through [`GoalState`] methods; field projection -//! remains available for read-heavy call sites. +//! Root `Agent` composes these so session/runtime/goals/report/workspace concerns +//! stay separated. Plan/snapshot mutations go through [`GoalState`] methods; other +//! holders expose fields for hot turn-loop projection. Cross-domain reads happen +//! at the composition layer (`Agent` methods), not inside holders. +use hi_ai::Usage; use hi_tools::{PlanStatus, PlanStep}; use crate::goal::Goal; use crate::heuristics::plan_has_pending_steps; +use crate::outcome::{EffectiveModelRoute, TurnOutcome}; +use crate::subagent::DelegateRunner; +use crate::task_contract::TaskContract; +use crate::agent::turn::TurnPhase; +use crate::TurnTelemetry; +use std::sync::Arc; /// Session goal + plan state owned by the interactive agent. #[derive(Clone, Debug, Default)] @@ -134,6 +141,103 @@ impl RsiObserveState { } } +/// Per-turn ranked task / memory prompt assembly state. +#[derive(Clone, Debug, Default)] +pub(crate) struct TaskContextState { + /// Per-turn ranked repository data and scoped instructions. + pub(crate) task_context: Option, + /// Live hierarchical memory section (task-ranked). + pub(crate) memory_context: Option, + /// Latest user/goal task text used for memory ranking. + pub(crate) last_task_prompt: Option, + pub(crate) last_task_contract: Option, +} + +/// Post-turn report surface: usage, verify, telemetry, phase, route. +#[derive(Clone, Debug)] +pub(crate) struct TurnReportState { + pub(crate) last_turn_usage: Usage, + pub(crate) last_user_prompt_tokens: u64, + pub(crate) last_verify: Option, + pub(crate) context_used: u64, + pub(crate) last_compat_fallbacks: Vec, + pub(crate) last_turn_telemetry: TurnTelemetry, + pub(crate) last_turn_outcome: Option, + pub(crate) turn_phase: TurnPhase, + pub(crate) last_effective_route: EffectiveModelRoute, +} + +impl TurnReportState { + pub(crate) fn new(route: EffectiveModelRoute) -> Self { + Self { + last_turn_usage: Usage::default(), + last_user_prompt_tokens: 0, + last_verify: None, + context_used: 0, + last_compat_fallbacks: Vec::new(), + last_turn_telemetry: TurnTelemetry::default(), + last_turn_outcome: None, + turn_phase: TurnPhase::Setup, + last_effective_route: route, + } + } +} + +impl Default for TurnReportState { + fn default() -> Self { + Self::new(EffectiveModelRoute { + provider: None, + model: String::new(), + }) + } +} + +/// Mutation/undo/reconcile state for the in-flight and last turn. +#[derive(Clone, Debug, Default)] +pub(crate) struct WorkspaceTurnState { + /// Per-turn git checkpoints (working-tree snapshots), for `/undo`. + pub(crate) checkpoints: Vec, + /// Files whose content or presence changed in the most recent turn. + pub(crate) last_changed_files: Vec, + /// Structured effects reported by mutating tools in the most recent turn. + pub(crate) last_file_changes: Vec, + /// Per-turn cache of the checkpoint diff (`turn_diff`). + pub(crate) turn_diff_cache: Option<(u64, String)>, + /// Per-turn cache of the stub scan over changed files. + pub(crate) turn_stub_scan_cache: Option<(u64, Vec)>, + /// Ledger baseline while a turn future is in flight (cancel-safe). + pub(crate) active_turn_ledger_revision: Option, + /// Message-len baseline while a turn future is in flight (cancel-safe). + pub(crate) active_turn_message_start: Option, + /// Background process ids at turn start so failed/cancelled finalizers can + /// kill only processes this turn started (mirrors frontend cancel cleanup). + pub(crate) active_turn_background_baseline: Option>, +} + +impl WorkspaceTurnState { + /// Clear cancel-safe active-turn baselines after a turn settles. + pub(crate) fn clear_active_baselines(&mut self) { + self.active_turn_ledger_revision = None; + self.active_turn_message_start = None; + self.active_turn_background_baseline = None; + } +} + +/// Session-scoped subagent caps and the optional write-capable runner. +#[derive(Default)] +pub(crate) struct SubagentSessionState { + /// Frontend-supplied runner for the write-capable `delegate` subagent. + pub(crate) delegate_runner: Option>, + /// Count of skills auto-curated this session (verifier-gated). + pub(crate) auto_skills_written: u32, + /// Count of coding facts auto-recorded this session (green-verify gate). + pub(crate) coding_facts_written: u32, + /// Count of read-only `explore` subagents run this session. + pub(crate) explore_subagents_used: u32, + /// Count of write-capable `delegate` subagents run this session. + pub(crate) delegate_subagents_used: u32, +} + /// Per-turn control flags shared across Model / Tools / Steer. /// /// Not stored on [`crate::Agent`] — constructed at turn start and passed through diff --git a/crates/hi-agent/src/lib.rs b/crates/hi-agent/src/lib.rs index eeccb3b7..2e5ded7e 100644 --- a/crates/hi-agent/src/lib.rs +++ b/crates/hi-agent/src/lib.rs @@ -81,7 +81,8 @@ pub use memory::{ pub use observation::{Observation, ObservationReceipt, ObservationSink}; pub use agent::turn::TurnPhase; pub use outcome::{ - EffectiveModelRoute, ReviewStatus, TurnOutcome, TurnStatus, TurnStopReason, VerificationStatus, + EffectiveModelRoute, ReviewStatus, TopLevelErrorKind, TurnOutcome, TurnStatus, TurnStopReason, + VerificationStatus, }; pub use session::SessionSink; pub use skills::{ @@ -617,15 +618,8 @@ pub struct Agent { pub(crate) local_skeptic: Option, pub(crate) config: AgentConfig, pub(crate) runtime: WorkspaceRuntime, - /// Per-turn ranked repository data and scoped instructions. - pub(crate) task_context: Option, - /// Live hierarchical memory section (task-ranked). Refreshed each turn and - /// after coding-fact writes so mid-session memory.md updates are visible - /// without restarting the agent (Phase P). - pub(crate) memory_context: Option, - /// Latest user/goal task text used for memory ranking (mirrors turn setup). - pub(crate) last_task_prompt: Option, - pub(crate) last_task_contract: Option, + /// Per-turn ranked task/memory prompt assembly. + pub(crate) task: crate::domain::TaskContextState, /// Conversation history, shared with in-flight `ChatRequest`s via the /// `Arc` inside [`Transcript`]. Mutations go through the `Transcript` API /// so provider-safety invariants (every `tool_use` has a matching @@ -633,74 +627,19 @@ pub struct Agent { pub(crate) messages: Transcript, pub(crate) tools: Arc<[ToolSpec]>, pub(crate) session: Option>, - /// Frontend-supplied runner for the write-capable `delegate` subagent (worktree - /// + subprocess + verify + apply-back). `None` → `delegate` is unavailable. - pub(crate) delegate_runner: Option>, /// How many messages have already been handed to the session sink. pub(crate) persisted: usize, /// Running total of tokens across the session. pub(crate) totals: Usage, - /// Token usage accumulated during the most recent `run_turn`. - pub(crate) last_turn_usage: Usage, - /// Estimated tokens in the raw user prompt for the most recent `run_turn`. - pub(crate) last_user_prompt_tokens: u64, - /// Whether the most recent turn's verification passed (None if not run). - pub(crate) last_verify: Option, - /// Input tokens of the most recent model call — a proxy for how full the - /// context window is, used to decide when to auto-compact. - pub(crate) context_used: u64, - /// Per-turn git checkpoints (working-tree snapshots), for `/undo`. - pub(crate) checkpoints: Vec, - /// Files whose content or presence changed in the most recent turn. - pub(crate) last_changed_files: Vec, - /// Structured effects reported by mutating tools in the most recent turn. - pub(crate) last_file_changes: Vec, - /// Per-turn cache of the checkpoint diff (`turn_diff`) and the stub scan - /// over `last_changed_files`. Both are recomputed at several call sites - /// (skeptic gate, completion audit, trio review, verify-review gate) even - /// though neither changes within a turn — the diff shells out to git and - /// the scan does file I/O per path. Keyed by the ledger revision they were - /// computed at so a post-computation reconcile (the workspace moved on) - /// can never serve a stale snapshot; cleared at turn start. - pub(crate) turn_diff_cache: Option<(u64, String)>, - pub(crate) turn_stub_scan_cache: Option<(u64, Vec)>, - /// Baselines retained while a turn future is in flight so a frontend that - /// cancels by dropping that future can still reconcile a truthful outcome. - pub(crate) active_turn_ledger_revision: Option, - pub(crate) active_turn_message_start: Option, - /// Count of skills auto-curated this session (verifier-gated). Capped per - /// session by [`agent::MAX_AUTO_SKILLS_PER_SESSION`] to bound skill spam. - pub(crate) auto_skills_written: u32, - /// Count of coding facts auto-recorded this session (green-verify gate). - pub(crate) coding_facts_written: u32, - /// Count of read-only `explore` subagents run this session. Capped per - /// session (see `MAX_EXPLORE_SUBAGENTS_PER_SESSION`) to bound cost if the - /// model over-delegates. - pub(crate) explore_subagents_used: u32, - /// Count of write-capable `delegate` subagents run this session. Capped by - /// `MAX_DELEGATE_SUBAGENTS_PER_SESSION`. - pub(crate) delegate_subagents_used: u32, - pub(crate) last_compat_fallbacks: Vec, - /// A shared interrupt flag. When set (by the UI on a user action like - /// pressing Esc during a tool call), the agent skips the remaining tool - /// calls in the current batch and feeds a "interrupted by user" result - /// back to the model, so it can adapt without losing the turn. + /// Post-turn report surface (usage, verify, telemetry, phase, route). + pub(crate) report: crate::domain::TurnReportState, + /// Mutation/undo/reconcile state for the in-flight and last turn. + pub(crate) workspace: crate::domain::WorkspaceTurnState, + /// Session-scoped subagent caps and optional write-capable runner. + pub(crate) subagents: crate::domain::SubagentSessionState, + /// A shared interrupt flag. When set, the current tool's result is replaced + /// with "interrupted by user" and the flag is cleared. pub(crate) interrupt: Arc, - /// Telemetry from the most recent `run_turn` (verify rounds, recovery - /// retries, nudges fired, last verify attributions). Flushed at turn end - /// from locals that would otherwise be discarded; exposed for `--report` - /// and the eval harness so they can diagnose *how* a turn went. - pub(crate) last_turn_telemetry: TurnTelemetry, - /// Typed result of the most recently completed (non-error) turn. - pub(crate) last_turn_outcome: Option, - /// Active (or last-finished) [`TurnPhase`] stamped by `run_turn` at each - /// control-flow boundary. Defaults to [`TurnPhase::Setup`] before the first - /// turn; ends on [`TurnPhase::Done`]. - pub(crate) turn_phase: TurnPhase, - /// Effective route observed during the most recent turn, retained even - /// when the turn ends with a provider/infrastructure error before a typed - /// outcome can be finalized. - pub(crate) last_effective_route: EffectiveModelRoute, /// Session goals + plan (transient free-text, durable structured goal, last plan). pub(crate) goals: GoalState, /// Durable intra-session decision log — recorded via the `record_decision` diff --git a/crates/hi-agent/src/outcome.rs b/crates/hi-agent/src/outcome.rs index b063c9f5..3cbabc56 100644 --- a/crates/hi-agent/src/outcome.rs +++ b/crates/hi-agent/src/outcome.rs @@ -95,4 +95,56 @@ impl TurnOutcome { }, } } + + /// Process exit code for one-shot CLI runs. + /// + /// - `0` completed + passed / N/A (or unverified when allowed) + /// - `1` incomplete / blocked / verify failed / unverified + /// - `3` failed / infrastructure error + /// - `130` cancelled + pub fn exit_code(&self, allow_unverified: bool) -> i32 { + match self.status { + TurnStatus::Cancelled => 130, + TurnStatus::Failed => 3, + TurnStatus::Incomplete | TurnStatus::Blocked => 1, + TurnStatus::Completed => match self.verification { + VerificationStatus::Passed | VerificationStatus::NotApplicable => 0, + VerificationStatus::Unverified if allow_unverified => 0, + VerificationStatus::Unverified | VerificationStatus::Failed => 1, + VerificationStatus::InfrastructureError => 3, + }, + } + } +} + +/// Coarse classification for top-level CLI errors that escape outside a typed +/// [`TurnOutcome`] (setup/config/parse vs infrastructure). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum TopLevelErrorKind { + /// Usage, config, or JSON parse errors → exit 2. + Usage, + /// Unrecovered setup/provider/runner failure → exit 3. + Infra, +} + +impl TopLevelErrorKind { + pub fn exit_code(self) -> i32 { + match self { + Self::Usage => 2, + Self::Infra => 3, + } + } + + /// Classify an escaped `anyhow` error from message content. + pub fn from_anyhow(error: &anyhow::Error) -> Self { + let message = format!("{error:#}").to_ascii_lowercase(); + if message.contains("usage:") + || message.contains("parsing skeptic-review json") + || message.contains("invalid configuration") + { + Self::Usage + } else { + Self::Infra + } + } } diff --git a/crates/hi-agent/src/steering/review_repair.rs b/crates/hi-agent/src/steering/review_repair.rs index 6ad5ea38..7c8b4172 100644 --- a/crates/hi-agent/src/steering/review_repair.rs +++ b/crates/hi-agent/src/steering/review_repair.rs @@ -183,9 +183,9 @@ pub(crate) fn compact_review_repair_label(label: &str) -> String { } /// Text-only Steer quality-repair cascade order (after unfinished/plan and -/// implementation-completeness gates). Keep this list aligned with -/// `steer/review.rs` — tests freeze the order so a casual reorder fails loudly. -#[cfg_attr(not(test), allow(dead_code))] +/// implementation-completeness gates). Walked by +/// [`crate::agent::turn::steer::cascade::select_review_quality_repair`] — tests +/// freeze the order so a casual reorder fails loudly. pub(crate) const REVIEW_QUALITY_CASCADE: &[ReviewRepairMode] = &[ ReviewRepairMode::NoEvidence, ReviewRepairMode::InspectedDisclaimer, diff --git a/crates/hi-agent/src/tests/curate.rs b/crates/hi-agent/src/tests/curate.rs index 352739a8..e377f0e0 100644 --- a/crates/hi-agent/src/tests/curate.rs +++ b/crates/hi-agent/src/tests/curate.rs @@ -66,7 +66,7 @@ async fn curate_writes_skill_from_verified_turn() { agent.curate_turn_end(0, &mut ui).await; assert_eq!( - agent.auto_skills_written, 1, + agent.subagents.auto_skills_written, 1, "a well-formed SKILL.md should be persisted and counted" ); let written = dir.join("reproduce-before-fixing").join("SKILL.md"); @@ -96,7 +96,7 @@ async fn curate_stays_silent_when_model_declines() { agent.curate_turn_end(0, &mut ui).await; assert_eq!( - agent.auto_skills_written, 0, + agent.subagents.auto_skills_written, 0, "a decline must write no skill" ); let empty = std::fs::read_dir(&dir) @@ -117,13 +117,13 @@ async fn curate_respects_session_cap() { let mut agent = verified_turn_agent("unused", &dir); // Already at the cap: the model is never consulted and nothing is written. - agent.auto_skills_written = crate::agent::MAX_AUTO_SKILLS_PER_SESSION; + agent.subagents.auto_skills_written = crate::agent::MAX_AUTO_SKILLS_PER_SESSION; let mut ui = NullUi; agent.curate_turn_end(0, &mut ui).await; assert_eq!( - agent.auto_skills_written, + agent.subagents.auto_skills_written, crate::agent::MAX_AUTO_SKILLS_PER_SESSION ); let empty = std::fs::read_dir(&dir) diff --git a/crates/hi-agent/src/tests/delegate.rs b/crates/hi-agent/src/tests/delegate.rs index 702a1af8..8b51070a 100644 --- a/crates/hi-agent/src/tests/delegate.rs +++ b/crates/hi-agent/src/tests/delegate.rs @@ -112,14 +112,14 @@ async fn delegate_missing_task_errors() { let out = agent.handle_delegate("{}", &mut ui).await; assert_eq!(out.status, hi_tools::ToolStatus::Failed); assert!(out.content.contains("missing"), "got: {}", out.content); - assert_eq!(agent.delegate_subagents_used, 0); + assert_eq!(agent.subagents.delegate_subagents_used, 0); } #[tokio::test] async fn delegate_respects_session_budget() { // At the cap, it returns before touching the working tree. let mut agent = agent(Vec::new(), delegate_config()); - agent.delegate_subagents_used = crate::agent::MAX_DELEGATE_SUBAGENTS_PER_SESSION; + agent.subagents.delegate_subagents_used = crate::agent::MAX_DELEGATE_SUBAGENTS_PER_SESSION; let mut ui = NullUi; let out = agent .handle_delegate(r#"{"task":"do something"}"#, &mut ui) @@ -131,7 +131,7 @@ async fn delegate_respects_session_budget() { out.content ); assert_eq!( - agent.delegate_subagents_used, + agent.subagents.delegate_subagents_used, crate::agent::MAX_DELEGATE_SUBAGENTS_PER_SESSION ); } @@ -166,7 +166,7 @@ async fn delegate_without_runner_is_unavailable() { .await; assert_eq!(out.status, hi_tools::ToolStatus::Denied); assert!(out.content.contains("unavailable"), "got: {}", out.content); - assert_eq!(agent.delegate_subagents_used, 0); + assert_eq!(agent.subagents.delegate_subagents_used, 0); } #[tokio::test] @@ -185,7 +185,7 @@ async fn delegate_invokes_runner_and_rejects_a_false_applied_claim() { ); assert!(out.effects.mutation_attempted); assert!(!out.effects.mutation_applied); - assert_eq!(agent.delegate_subagents_used, 1); + assert_eq!(agent.subagents.delegate_subagents_used, 1); assert!( ui.statuses.iter().any(|s| s.contains("delegate subagent")), "expected a delegate callout; got {:?}", diff --git a/crates/hi-agent/src/tests/explore.rs b/crates/hi-agent/src/tests/explore.rs index 285a2eee..e776fe3c 100644 --- a/crates/hi-agent/src/tests/explore.rs +++ b/crates/hi-agent/src/tests/explore.rs @@ -62,13 +62,13 @@ async fn explore_missing_task_errors() { let out = agent.handle_explore("{}", &mut ui).await; assert_eq!(out.status, hi_tools::ToolStatus::Failed); assert!(out.content.contains("missing"), "got: {}", out.content); - assert_eq!(agent.explore_subagents_used, 0); + assert_eq!(agent.subagents.explore_subagents_used, 0); } #[tokio::test] async fn explore_respects_session_budget() { let mut agent = agent(Vec::new(), explore_config()); - agent.explore_subagents_used = crate::agent::MAX_EXPLORE_SUBAGENTS_PER_SESSION; + agent.subagents.explore_subagents_used = crate::agent::MAX_EXPLORE_SUBAGENTS_PER_SESSION; let mut ui = NullUi; let out = agent .handle_explore(r#"{"task":"anything"}"#, &mut ui) @@ -81,7 +81,7 @@ async fn explore_respects_session_budget() { ); // Cap is not exceeded (no model call was made). assert_eq!( - agent.explore_subagents_used, + agent.subagents.explore_subagents_used, crate::agent::MAX_EXPLORE_SUBAGENTS_PER_SESSION ); } @@ -135,7 +135,7 @@ async fn explore_runs_child_and_returns_answer() { .unwrap(); // Exactly one subagent ran, and its answer came back as the `explore` tool result. - assert_eq!(agent.explore_subagents_used, 1); + assert_eq!(agent.subagents.explore_subagents_used, 1); assert!( ui.tool_results .iter() diff --git a/crates/hi-agent/src/tests/goal_contract.rs b/crates/hi-agent/src/tests/goal_contract.rs index 13e06e68..2ed2fd84 100644 --- a/crates/hi-agent/src/tests/goal_contract.rs +++ b/crates/hi-agent/src/tests/goal_contract.rs @@ -348,7 +348,7 @@ async fn exact_plan_goal_continuation_uses_real_context_and_implementation_guard ); assert_eq!(agent.last_turn_telemetry().effective_max_steps, 120); assert_eq!( - agent.last_task_contract.as_ref().unwrap().referenced_paths, + agent.task.last_task_contract.as_ref().unwrap().referenced_paths, vec!["plan.md"] ); let first_request = &requests.lock().unwrap()[0]; diff --git a/crates/hi-agent/src/tests/outcome.rs b/crates/hi-agent/src/tests/outcome.rs index 966a5312..ba3c4608 100644 --- a/crates/hi-agent/src/tests/outcome.rs +++ b/crates/hi-agent/src/tests/outcome.rs @@ -118,8 +118,8 @@ fn cancelled_turn_reconciles_surviving_workspace_changes() { cfg.paths.workspace_root = root.clone(); cfg.paths.state_root = root.join(".hi/state"); let mut agent = agent(Vec::new(), cfg); - agent.active_turn_ledger_revision = Some(agent.runtime.ledger().revision()); - agent.active_turn_message_start = Some(agent.messages().len()); + agent.workspace.active_turn_ledger_revision = Some(agent.runtime.ledger().revision()); + agent.workspace.active_turn_message_start = Some(agent.messages().len()); std::fs::write(root.join("survived.txt"), "kept\n").unwrap(); let outcome = agent.finalize_cancelled_turn().unwrap(); @@ -704,3 +704,95 @@ async fn infrastructure_finalizer_reconciles_ui_effects_after_session_failure() assert!(outcome.changed_files.contains(&"late.rs".to_string())); let _ = std::fs::remove_dir_all(root); } + +#[test] +fn turn_outcome_exit_codes_match_one_shot_table() { + use crate::{EffectiveModelRoute, ReviewStatus, TurnStopReason}; + + let route = EffectiveModelRoute { + provider: None, + model: "m".into(), + }; + let base = |status, verification, stop_reason| TurnOutcome { + status, + verification, + review: ReviewStatus::NotRequired, + stop_reason, + changed_files: Vec::new(), + verified_workspace_revision: None, + effective_route: route.clone(), + }; + assert_eq!( + base( + TurnStatus::Cancelled, + VerificationStatus::Unverified, + TurnStopReason::Cancelled + ) + .exit_code(false), + 130 + ); + assert_eq!( + base( + TurnStatus::Failed, + VerificationStatus::InfrastructureError, + TurnStopReason::InfrastructureFailure + ) + .exit_code(false), + 3 + ); + assert_eq!( + base( + TurnStatus::Incomplete, + VerificationStatus::Unverified, + TurnStopReason::Stalled + ) + .exit_code(false), + 1 + ); + assert_eq!( + base( + TurnStatus::Completed, + VerificationStatus::Passed, + TurnStopReason::Completed + ) + .exit_code(false), + 0 + ); + assert_eq!( + base( + TurnStatus::Completed, + VerificationStatus::Unverified, + TurnStopReason::NoApplicableVerification + ) + .exit_code(false), + 1 + ); + assert_eq!( + base( + TurnStatus::Completed, + VerificationStatus::Unverified, + TurnStopReason::NoApplicableVerification + ) + .exit_code(true), + 0 + ); +} + +#[test] +fn top_level_error_kind_classifies_usage_vs_infra() { + use crate::TopLevelErrorKind; + assert_eq!( + TopLevelErrorKind::from_anyhow(&anyhow::anyhow!("usage: missing --model")), + TopLevelErrorKind::Usage + ); + assert_eq!( + TopLevelErrorKind::from_anyhow(&anyhow::anyhow!("invalid configuration: bad tool mode")), + TopLevelErrorKind::Usage + ); + assert_eq!( + TopLevelErrorKind::from_anyhow(&anyhow::anyhow!("connection reset by peer")), + TopLevelErrorKind::Infra + ); + assert_eq!(TopLevelErrorKind::Usage.exit_code(), 2); + assert_eq!(TopLevelErrorKind::Infra.exit_code(), 3); +} diff --git a/crates/hi-agent/src/tests/retry.rs b/crates/hi-agent/src/tests/retry.rs index cc2ac1a0..b2db9b60 100644 --- a/crates/hi-agent/src/tests/retry.rs +++ b/crates/hi-agent/src/tests/retry.rs @@ -967,7 +967,7 @@ async fn terminal_error_aborts_without_retry() { async fn terminal_error_resets_stale_turn_telemetry() { let (mut agent, _requests) = scripted_agent(vec![ProviderStep::Error(ProviderErrorKind::Auth)], config()); - agent.last_turn_telemetry = TurnTelemetry { + agent.report.last_turn_telemetry = TurnTelemetry { repeat_nudges: 99, stalled_unfinished: true, tool_calls: 42, diff --git a/crates/hi-agent/src/tests/turn.rs b/crates/hi-agent/src/tests/turn.rs index fa61d36f..49267e08 100644 --- a/crates/hi-agent/src/tests/turn.rs +++ b/crates/hi-agent/src/tests/turn.rs @@ -42,7 +42,7 @@ fn resume_restores_retained_checkpoint_refs() { #[tokio::test] async fn undo_keeps_checkpoint_when_restore_fails() { let mut agent = agent(vec![], config()); - agent.checkpoints.push("not-a-valid-checkpoint".to_string()); + agent.workspace.checkpoints.push("not-a-valid-checkpoint".to_string()); let err = agent.undo().await.unwrap_err(); @@ -80,7 +80,7 @@ async fn undo_keeps_checkpoint_when_persisting_shortened_stack_fails() { cfg.paths.state_root = state.clone(); let mut agent = agent(vec![], cfg); agent - .checkpoints + .workspace.checkpoints .push(hi_tools::checkpoint::sealed_reference(&before, &after)); agent.set_session(Box::new(FailingCheckpointSession)); @@ -159,10 +159,10 @@ async fn tools_unavailable_fast_path_resets_state_and_shows_message() { let mut cfg = config(); cfg.routing.tool_mode = ToolMode::ChatOnly; let mut agent = agent(vec![], cfg); - agent.last_verify = Some(true); - agent.last_changed_files = vec!["old.rs".to_string()]; - agent.last_compat_fallbacks = vec!["compat fallback".to_string()]; - agent.last_turn_telemetry = TurnTelemetry { + agent.report.last_verify = Some(true); + agent.workspace.last_changed_files = vec!["old.rs".to_string()]; + agent.report.last_compat_fallbacks = vec!["compat fallback".to_string()]; + agent.report.last_turn_telemetry = TurnTelemetry { repeat_nudges: 7, stalled_unfinished: true, tool_calls: 3, diff --git a/crates/hi-agent/src/tests/usage.rs b/crates/hi-agent/src/tests/usage.rs index 69fa5f19..087b21c7 100644 --- a/crates/hi-agent/src/tests/usage.rs +++ b/crates/hi-agent/src/tests/usage.rs @@ -65,7 +65,7 @@ fn turn_steer_summarizes_trajectory() { assert_eq!(a.turn_steer(), None); // Noisy turn → a steer line listing each non-zero component. - a.last_turn_telemetry = TurnTelemetry { + a.report.last_turn_telemetry = TurnTelemetry { verify_rounds: 2, recovery_retries: 1, repeat_nudges: 0, @@ -92,7 +92,7 @@ fn turn_steer_summarizes_trajectory() { ); // A stall is surfaced even with no rounds. - a.last_turn_telemetry = TurnTelemetry { + a.report.last_turn_telemetry = TurnTelemetry { verify_rounds: 0, recovery_retries: 0, repeat_nudges: 0, @@ -197,7 +197,7 @@ async fn context_gauge_prefers_provider_normalized_occupancy() { agent.run_turn("go", &mut NullUi).await.unwrap(); - assert_eq!(agent.context_used(), 777); + assert_eq!(agent.report.context_used, 777); } #[tokio::test] diff --git a/crates/hi-cli/src/main.rs b/crates/hi-cli/src/main.rs index 32aeb70b..8d368e4d 100644 --- a/crates/hi-cli/src/main.rs +++ b/crates/hi-cli/src/main.rs @@ -71,18 +71,9 @@ async fn main() { } fn top_level_error_code(error: &anyhow::Error) -> i32 { - let message = format!("{error:#}").to_ascii_lowercase(); - if message.contains("usage:") - || message.contains("parsing skeptic-review json") - || message.contains("invalid configuration") - { - 2 - } else { - // Typed turn outcomes use 0/1/130 in the one-shot branch. Anything - // escaping the top-level dispatcher is unrecovered setup, provider, - // process-runner, or internal infrastructure failure. - 3 - } + // Typed turn outcomes use 0/1/130 in the one-shot branch. Anything escaping + // the top-level dispatcher is classified as usage/config (2) or infra (3). + hi_agent::TopLevelErrorKind::from_anyhow(error).exit_code() } async fn run() -> Result<()> { diff --git a/crates/hi-cli/src/report.rs b/crates/hi-cli/src/report.rs index ca58a6bc..7d131d83 100644 --- a/crates/hi-cli/src/report.rs +++ b/crates/hi-cli/src/report.rs @@ -3,9 +3,7 @@ use std::path::Path; use anyhow::{Context, Result, anyhow}; -use hi_agent::{ - Agent, Observation, ObservationSink, TurnOutcome, TurnStatus, VerificationStatus, VerifyStage, -}; +use hi_agent::{Agent, Observation, ObservationSink, TurnOutcome, VerifyStage}; use hi_rsi_runtime::ManagedRuntimeDescriptor; use hi_trace::{TraceIdentity, TraceMode, TraceSummary, TraceWriter}; @@ -28,17 +26,7 @@ pub(crate) fn pipeline_command(stages: &[VerifyStage]) -> Option { } pub(crate) fn one_shot_exit_code(outcome: &TurnOutcome, allow_unverified: bool) -> i32 { - match outcome.status { - TurnStatus::Cancelled => 130, - TurnStatus::Failed => 3, - TurnStatus::Incomplete | TurnStatus::Blocked => 1, - TurnStatus::Completed => match outcome.verification { - VerificationStatus::Passed | VerificationStatus::NotApplicable => 0, - VerificationStatus::Unverified if allow_unverified => 0, - VerificationStatus::Unverified | VerificationStatus::Failed => 1, - VerificationStatus::InfrastructureError => 3, - }, - } + outcome.exit_code(allow_unverified) } pub(crate) fn report_verification_stages( diff --git a/crates/hi-tools/src/lib.rs b/crates/hi-tools/src/lib.rs index 7d3dcc1e..58365f11 100644 --- a/crates/hi-tools/src/lib.rs +++ b/crates/hi-tools/src/lib.rs @@ -12,6 +12,17 @@ //! `hi_tools::…` call sites keep compiling. Prefer `hi_tools::protocol::…` / //! `hi_tools::infra::…` in new code when the boundary matters. //! +//! # Firewall policy (soft) +//! +//! | Prefer | Contents | +//! |--------|----------| +//! | [`protocol`] | tool catalog/execute, checkpoint, guard, sandbox, worktree, process, transactions, `ToolOutcome` family | +//! | [`infra`] | hf, local_server, web, repo_map, fast_feedback, LSP status | +//! +//! `hi-agent` turn/verify/mutation paths should depend on protocol symbols; +//! product/CLI orientation and download helpers belong under infra. Root paths +//! remain valid indefinitely for compatibility. +//! //! Richer capabilities still come from subprocess CLI tools the model invokes //! via `bash` — not a plugin runtime — so the advertised tool set stays small.