From 66401a36bc34176c2631127e02dc7275d1d5cea7 Mon Sep 17 00:00:00 2001 From: David Date: Mon, 20 Jul 2026 17:47:05 -0700 Subject: [PATCH] refactor: turn cleanup API, TurnState, impl cascade, tools split Unify abnormal turn teardown via cleanup_turn (Cancel/Fail) across CLI, TUI, REPL, sync, and explore children. Own per-turn loop state in TurnState, table-drive implementation completeness gates, add domain setter APIs, lint hi-tools protocol/infra imports in the turn loop, and split tools into tools/batch plus model_request helpers. --- crates/hi-agent/src/agent/delegate_turn.rs | 9 +- crates/hi-agent/src/agent/explore_turn.rs | 17 +- crates/hi-agent/src/agent/lifecycle.rs | 114 +++-- crates/hi-agent/src/agent/turn/loop_.rs | 449 +++++++++--------- crates/hi-agent/src/agent/turn/mod.rs | 2 + .../hi-agent/src/agent/turn/model_request.rs | 61 +++ crates/hi-agent/src/agent/turn/model_round.rs | 21 +- crates/hi-agent/src/agent/turn/setup.rs | 2 +- crates/hi-agent/src/agent/turn/state.rs | 178 +++++++ .../src/agent/turn/steer/impl_cascade.rs | 178 +++++++ crates/hi-agent/src/agent/turn/steer/mod.rs | 1 + .../hi-agent/src/agent/turn/steer/review.rs | 115 +---- .../agent/turn/{tools.rs => tools/batch.rs} | 26 +- crates/hi-agent/src/agent/turn/tools/mod.rs | 7 + .../hi-agent/src/agent/turn/verify_outcome.rs | 10 +- crates/hi-agent/src/domain.rs | 84 ++++ crates/hi-agent/src/lib.rs | 4 +- crates/hi-agent/src/outcome.rs | 26 + crates/hi-agent/src/tests.rs | 1 + .../src/tests/protocol_import_lint.rs | 62 +++ crates/hi-cli/src/main.rs | 26 +- crates/hi-cli/src/repl.rs | 12 +- crates/hi-cli/src/sync.rs | 4 +- crates/hi-tui/src/app/run/mod.rs | 56 ++- 24 files changed, 1045 insertions(+), 420 deletions(-) create mode 100644 crates/hi-agent/src/agent/turn/model_request.rs create mode 100644 crates/hi-agent/src/agent/turn/state.rs create mode 100644 crates/hi-agent/src/agent/turn/steer/impl_cascade.rs rename crates/hi-agent/src/agent/turn/{tools.rs => tools/batch.rs} (98%) create mode 100644 crates/hi-agent/src/agent/turn/tools/mod.rs create mode 100644 crates/hi-agent/src/tests/protocol_import_lint.rs diff --git a/crates/hi-agent/src/agent/delegate_turn.rs b/crates/hi-agent/src/agent/delegate_turn.rs index 98a1736..909b80c 100644 --- a/crates/hi-agent/src/agent/delegate_turn.rs +++ b/crates/hi-agent/src/agent/delegate_turn.rs @@ -59,6 +59,8 @@ impl crate::Agent { false, ); } + // Budget before runner so exhausted sessions get a clear budget message + // even when a runner is attached (and tests that only set the counter). if self.subagents.delegate_subagents_used >= MAX_DELEGATE_SUBAGENTS_PER_SESSION { return delegate_tool_outcome( format!( @@ -79,15 +81,16 @@ impl crate::Agent { false, ); }; + let n = self + .subagents + .try_begin_delegate(MAX_DELEGATE_SUBAGENTS_PER_SESSION) + .expect("budget checked above"); let verify = parsed .as_ref() .and_then(|v| v.get("verify").and_then(Value::as_str)) .map(str::to_string) .filter(|s| !s.trim().is_empty()); - - 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 4ee26b9..c5dabe2 100644 --- a/crates/hi-agent/src/agent/explore_turn.rs +++ b/crates/hi-agent/src/agent/explore_turn.rs @@ -57,7 +57,10 @@ impl crate::Agent { hi_tools::ToolStatus::Failed, ); } - if self.subagents.explore_subagents_used >= MAX_EXPLORE_SUBAGENTS_PER_SESSION { + let Some(n) = self + .subagents + .try_begin_explore(MAX_EXPLORE_SUBAGENTS_PER_SESSION) + else { return explore_tool_outcome( format!( "explore budget exhausted ({MAX_EXPLORE_SUBAGENTS_PER_SESSION} subagents this \ @@ -65,9 +68,7 @@ impl crate::Agent { ), hi_tools::ToolStatus::Denied, ); - } - 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 { "" }; @@ -166,9 +167,8 @@ impl crate::Agent { explore_tool_outcome(answer, status) } 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(); + // Nested escapes: typed fail cleanup (turn-scoped bg kill). + let _ = child.cleanup_turn(crate::TurnCleanupKind::Fail).await; explore_tool_outcome( format!("explore subagent error: {err}"), hi_tools::ToolStatus::Failed, @@ -176,8 +176,7 @@ impl crate::Agent { } } }; - // Always tear down child-owned backgrounds (read-only explore should be - // quiet, but bash-in-readonly denial paths still share the kill API). + // Throwaway child runtime: full kill (local skeptic + any leftover bg). child.kill_background_processes(); // Fold the child's token usage into the parent's session totals. self.add_side_usage(*child.totals()); diff --git a/crates/hi-agent/src/agent/lifecycle.rs b/crates/hi-agent/src/agent/lifecycle.rs index a684ff9..8701b0d 100644 --- a/crates/hi-agent/src/agent/lifecycle.rs +++ b/crates/hi-agent/src/agent/lifecycle.rs @@ -726,26 +726,84 @@ impl crate::Agent { self.stop_local_skeptic_server(); } - /// 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. + /// Single entry point for abnormal turn teardown (cancel / infrastructure fail). /// - /// Mirrors frontend cancel cleanup for turn-scoped background processes when - /// the caller has not already killed them (safe if already empty). + /// Owns turn-scoped background kill via [`WorkspaceTurnState::active_turn_background_baseline`] + /// (taken once — second call is a no-op). Frontends should prefer this over + /// ad-hoc kill + finalize sequences. + /// + /// Normal successful turns clear baselines inside `run_turn` and must not call this. + pub async fn cleanup_turn( + &mut self, + kind: crate::TurnCleanupKind, + ) -> Result { + match kind { + crate::TurnCleanupKind::Cancel { session } => { + let killed = self.take_and_kill_turn_backgrounds(); + match session { + crate::SessionRollback::AlreadyApplied => { + // Frontend already rewound transcript/goals; don't truncate again. + let _ = self.workspace.active_turn_message_start.take(); + } + crate::SessionRollback::AgentOwned { + checkpoint_count_before, + } => { + if self.checkpoint_count() > checkpoint_count_before + && let Err(err) = self.undo().await + { + eprintln!( + "hi-agent: couldn't roll back cancelled workspace edits: {err:#}" + ); + } + if let Some(start) = self.workspace.active_turn_message_start.take() { + self.truncate_messages(start); + } + } + } + let outcome = self.finalize_cancelled_turn_inner()?; + Ok(crate::TurnCleanupResult { + outcome, + killed_backgrounds: killed, + }) + } + crate::TurnCleanupKind::Fail => { + let killed = self.take_and_kill_turn_backgrounds(); + let outcome = self.finalize_failed_turn_inner(); + Ok(crate::TurnCleanupResult { + outcome, + killed_backgrounds: killed, + }) + } + } + } + + /// Finalize a cancelled turn. Prefer [`Self::cleanup_turn`] so background kill + /// and session rollback stay consistent across frontends. pub fn finalize_cancelled_turn(&mut self) -> Result { - self.cleanup_turn_backgrounds(); + let _ = self.take_and_kill_turn_backgrounds(); + self.finalize_cancelled_turn_inner() + } + + /// Finalize a failed turn. Prefer [`Self::cleanup_turn`]([`TurnCleanupKind::Fail`]). + pub fn finalize_failed_turn(&mut self) -> crate::TurnOutcome { + let _ = self.take_and_kill_turn_backgrounds(); + self.finalize_failed_turn_inner() + } + + fn finalize_cancelled_turn_inner(&mut self) -> Result { + // Message truncate only if still set (AlreadyApplied path takes it first). if let Some(start) = self.workspace.active_turn_message_start.take() { self.truncate_messages(start); } self.runtime.ledger().reconcile()?; let baseline = self - .workspace.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.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.record_changes(changes, true); + self.report.clear_verify(); self.workspace.clear_active_baselines(); let outcome = crate::TurnOutcome { status: crate::TurnStatus::Cancelled, @@ -756,30 +814,21 @@ impl crate::Agent { verified_workspace_revision: None, effective_route: self.report.last_effective_route.clone(), }; - self.report.last_turn_outcome = Some(outcome.clone()); + self.report.set_outcome(outcome.clone()); let _ = self.persist(); Ok(outcome) } - /// Reconcile and type a turn that escaped through an infrastructure or - /// 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(); + fn finalize_failed_turn_inner(&mut self) -> crate::TurnOutcome { let baseline = self - .workspace.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.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.record_changes(changes, true); + self.report.clear_verify(); self.workspace.clear_active_baselines(); let route = self.report.last_effective_route.clone(); let outcome = crate::TurnOutcome::infrastructure_failure( @@ -787,15 +836,16 @@ impl crate::Agent { route.provider, self.workspace.last_changed_files.clone(), ); - self.report.last_turn_outcome = Some(outcome.clone()); + self.report.set_outcome(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); + /// Take the turn background baseline and kill anything started after it. + /// Second call is a no-op (baseline already taken). + fn take_and_kill_turn_backgrounds(&mut self) -> usize { + match self.workspace.active_turn_background_baseline.take() { + Some(before) => self.runtime.background().kill_started_after(&before), + None => 0, } } @@ -939,7 +989,7 @@ impl crate::Agent { let global = crate::memory::read_global_memory(); let next = crate::memory::memory_section_for_task(&project, &global, task); if next != self.task.memory_context { - self.task.memory_context = next; + self.task.set_memory_context(next); } } diff --git a/crates/hi-agent/src/agent/turn/loop_.rs b/crates/hi-agent/src/agent/turn/loop_.rs index a23b8ce..1e1415e 100644 --- a/crates/hi-agent/src/agent/turn/loop_.rs +++ b/crates/hi-agent/src/agent/turn/loop_.rs @@ -64,20 +64,14 @@ impl crate::Agent { // its documented per-turn contract — so a file changed outside `hi` // between turns is re-read fresh, not served from a prior turn's cache. 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.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.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()); + // Ledger + bg baselines + per-turn caches (cancel-safe finalizers). + self.workspace + .begin_turn(turn_ledger_revision, 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 @@ -117,26 +111,28 @@ impl crate::Agent { ranked_context_paths.insert(path); } } - self.task.task_context = repository_context_enabled - .then(|| { - let index = crate::context_index::build_task_context_index( - self.runtime.root(), - &context_task, - &ranked_context_paths.iter().cloned().collect::>(), - &self.config.memory.context_exclusions, - ); - let orientation = hi_tools::orientation_for_task( - self.runtime.root(), - &context_task, - self.runtime.repo_map(), - ); - match (orientation, index) { - (Some(seed), Some(index)) => Some(format!("{seed}\n\n{index}")), - (Some(seed), None) => Some(seed), - (None, index) => index, - } - }) - .flatten(); + self.task.set_task_context( + repository_context_enabled + .then(|| { + let index = crate::context_index::build_task_context_index( + self.runtime.root(), + &context_task, + &ranked_context_paths.iter().cloned().collect::>(), + &self.config.memory.context_exclusions, + ); + let orientation = hi_tools::orientation_for_task( + self.runtime.root(), + &context_task, + self.runtime.repo_map(), + ); + match (orientation, index) { + (Some(seed), Some(index)) => Some(format!("{seed}\n\n{index}")), + (Some(seed), None) => Some(seed), + (None, index) => index, + } + }) + .flatten(), + ); let mut context_generation_seen = self.runtime.context_generation(); let mut indexed_ledger_revision = self.runtime.ledger().revision(); let read_only_intent = classify_read_only_intent(&context_task); @@ -158,8 +154,10 @@ impl crate::Agent { task_contract.intent = TaskIntent::ReadOnly; task_contract.explicit_mutation = false; } - self.task.last_task_contract = Some(task_contract.clone()); - self.task.last_task_prompt = Some(context_task.clone()); + self.task.set_task( + Some(context_task.clone()), + Some(task_contract.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 @@ -293,9 +291,9 @@ impl crate::Agent { self.messages.strip_trailing_nudges(); self.persisted = self.persisted.min(self.messages.len()); let mut turn_start = self.messages.len(); - self.workspace.active_turn_message_start = Some(turn_start); + self.workspace.set_message_start(turn_start); self.messages.push_user_or_fold(&model_turn_input); - self.report.last_verify = None; + self.report.set_verify(None); self.workspace.last_changed_files.clear(); self.workspace.last_file_changes.clear(); self.report.last_compat_fallbacks.clear(); @@ -480,8 +478,73 @@ impl crate::Agent { // Snapshot from the most recent verify check. Reused at turn end to // avoid a second full tree walk when verify already took one. - if empty_tui_needs_project { - flags.force_tools_next = true; + // Owned per-turn bag — Model/Tools/Steer/Verify project from this. + let mut turn = super::state::TurnState { + user_prompt_tokens, + turn_ledger_revision, + turn_background_baseline: turn_background_baseline.clone(), + context_task: context_task.clone(), + goal_drive_turn, + task_contract: task_contract.clone(), + repository_context_enabled, + ranked_context_paths, + context_generation_seen, + indexed_ledger_revision, + read_only_intent, + implementation_intent, + expected_mutation, + inspection_sprawl_intent, + read_only_inspection_cap, + turn_input: input.to_string(), + turn_checkpoint_allowed, + turn_checkpoint_created, + verifier, + fast_feedback, + max_steps, + max_parallel_tools, + steps, + empty_retries, + truncation_retries, + truncation_total_retries, + silent_continues, + continue_total_nudges, + repeat_nudges, + repeat_sampling_rounds, + flags, + mutation_recovery, + plan_updated_goal, + proposed_goal, + goal_before: goal_before.clone(), + progress_tracker, + evidence, + implementation_tracker, + review_repair, + tool_guardrail, + empty_tui_needs_project, + sched_tool_calls, + sched_max_concurrent, + sched_serial_runs, + tool_timeline, + advertised_tool_names, + tool_schema_tokens, + prev_call_sig, + prev_added_no_evidence, + retry_state, + request_max_tokens_override, + compat_fallbacks, + effective_fallback_route, + independent_review_status, + independent_review_repairs, + verification_infrastructure_error, + verification_unstable, + verified_at, + last_verify_attributions, + turn_snapshot, + turn_start, + }; + + if turn.empty_tui_needs_project { + turn.flags.force_tools_next = true; self.messages .push_nudge(NudgeKind::Continue, IMPLEMENTATION_EMPTY_TUI_NUDGE); } @@ -491,60 +554,7 @@ impl crate::Agent { let hit_cap = loop { match self .run_model_round( - &mut super::model_round::ModelRoundState { - steps: &mut steps, - empty_retries: &mut empty_retries, - truncation_retries: &mut truncation_retries, - truncation_total_retries: &mut truncation_total_retries, - silent_continues: &mut silent_continues, - continue_total_nudges: &mut continue_total_nudges, - repeat_nudges: &mut repeat_nudges, - repeat_sampling_rounds: &mut repeat_sampling_rounds, - force_tools_next: &mut flags.force_tools_next, - text_tool_fallback_next: &mut flags.text_tool_fallback_next, - force_text_answer_next: &mut flags.force_text_answer_next, - force_no_progress_final_answer_next: &mut flags.force_no_progress_final_answer_next, - suppress_bookkeeping_tools_next: &mut flags.suppress_bookkeeping_tools_next, - prev_call_sig: &mut prev_call_sig, - prev_added_no_evidence: &mut prev_added_no_evidence, - made_tool_call: &mut flags.made_tool_call, - retry_state: &mut retry_state, - request_max_tokens_override: &mut request_max_tokens_override, - turn_start: &mut turn_start, - stalled_repeating: &mut flags.stalled_repeating, - stalled_unfinished: &mut flags.stalled_unfinished, - compat_fallbacks: &mut compat_fallbacks, - effective_fallback_route: &mut effective_fallback_route, - ranked_context_paths: &mut ranked_context_paths, - context_generation_seen: &mut context_generation_seen, - indexed_ledger_revision: &mut indexed_ledger_revision, - progress_tracker: &mut progress_tracker, - evidence: &mut evidence, - implementation_tracker: &mut implementation_tracker, - review_repair: &mut review_repair, - tool_guardrail: &mut tool_guardrail, - last_verify_attributions: &mut last_verify_attributions, - tool_timeline: &mut tool_timeline, - sched_tool_calls: &mut sched_tool_calls, - sched_max_concurrent: &mut sched_max_concurrent, - sched_serial_runs: &mut sched_serial_runs, - advertised_tool_names: &mut advertised_tool_names, - tool_schema_tokens: &mut tool_schema_tokens, - ended_at_cap: &mut flags.ended_at_cap, - turn_snapshot: &mut turn_snapshot, - max_steps, - context_task: &context_task, - repository_context_enabled, - turn_ledger_revision, - read_only_intent, - implementation_intent, - read_only_inspection_cap, - expected_mutation, - input, - user_prompt_tokens, - inspection_sprawl_intent, - verifier: &verifier, - }, + &mut turn.as_model_round_state(), ui, ) .await? @@ -556,52 +566,52 @@ impl crate::Agent { completion_content, } => { let mut completion_content = completion_content; - flags.made_tool_call = true; - silent_continues = 0; + turn.flags.made_tool_call = true; + turn.silent_continues = 0; // Tools ran — drop one-shot force flags for the next Model round. - flags.clear_one_shot_forces(); + turn.flags.clear_one_shot_forces(); self.set_turn_phase(TurnPhase::Tools); let batch = self .execute_tool_batch( &calls, &mut completion_content, - read_only_intent, - max_parallel_tools, - &task_contract, - &mut implementation_tracker, - &mut evidence, - &mut tool_guardrail, - &mut progress_tracker, - &mut tool_timeline, - &mut sched_tool_calls, - &mut sched_max_concurrent, - &mut sched_serial_runs, - &mut plan_updated_goal, - &mut proposed_goal, - &mut turn_snapshot, - &mut turn_checkpoint_allowed, - &mut turn_checkpoint_created, - &mut fast_feedback, + turn.read_only_intent, + turn.max_parallel_tools, + &turn.task_contract, + &mut turn.implementation_tracker, + &mut turn.evidence, + &mut turn.tool_guardrail, + &mut turn.progress_tracker, + &mut turn.tool_timeline, + &mut turn.sched_tool_calls, + &mut turn.sched_max_concurrent, + &mut turn.sched_serial_runs, + &mut turn.plan_updated_goal, + &mut turn.proposed_goal, + &mut turn.turn_snapshot, + &mut turn.turn_checkpoint_allowed, + &mut turn.turn_checkpoint_created, + &mut turn.fast_feedback, ui, ) .await?; match self.steer_after_tools( &calls, &batch, - expected_mutation, - read_only_intent, - implementation_intent, - &mut implementation_tracker, - &mut evidence, - &mut mutation_recovery, - &mut progress_tracker, - &mut repeat_nudges, - &mut flags.force_tools_next, - &mut flags.text_tool_fallback_next, - &mut flags.force_no_progress_final_answer_next, - &mut prev_added_no_evidence, - &mut flags.stalled_repeating, - &mut flags.stalled_unfinished, + turn.expected_mutation, + turn.read_only_intent, + turn.implementation_intent, + &mut turn.implementation_tracker, + &mut turn.evidence, + &mut turn.mutation_recovery, + &mut turn.progress_tracker, + &mut turn.repeat_nudges, + &mut turn.flags.force_tools_next, + &mut turn.flags.text_tool_fallback_next, + &mut turn.flags.force_no_progress_final_answer_next, + &mut turn.prev_added_no_evidence, + &mut turn.flags.stalled_repeating, + &mut turn.flags.stalled_unfinished, ui, ) { super::steer::RoundControl::Continue => {} @@ -612,8 +622,8 @@ impl crate::Agent { }; if hit_cap { - ui.status(&format!("reached step limit ({max_steps}); stopping turn")); - flags.ended_at_cap = true; + ui.status(&format!("reached step limit ({}); stopping turn", turn.max_steps)); + turn.flags.ended_at_cap = true; break 'turn; } @@ -622,41 +632,41 @@ impl crate::Agent { self.set_turn_phase(TurnPhase::WorkspaceRepair); let outcome = self .run_workspace_repair_verification( - &mut verifier, - &turn_background_baseline, - &mut turn_snapshot, - turn_checkpoint_created, - turn_ledger_revision, - &fast_feedback, + &mut turn.verifier, + &turn.turn_background_baseline, + &mut turn.turn_snapshot, + turn.turn_checkpoint_created, + turn.turn_ledger_revision, + &turn.fast_feedback, ui, ) .await?; - // Retain evidence immediately, not only in the common finalizer: + // Retain turn.evidence immediately, not only in the common finalizer: // reconciliation or persistence can still fail after a successful // check, and reports for those error turns need the stages that // actually ran. - self.report.last_turn_telemetry.verification_executions = verifier.executions().to_vec(); + self.report.last_turn_telemetry.verification_executions = turn.verifier.executions().to_vec(); match self .handle_workspace_repair_outcome( outcome, - &mut verifier, - turn_ledger_revision, - expected_mutation, - &context_task, - repository_context_enabled, + &mut turn.verifier, + turn.turn_ledger_revision, + turn.expected_mutation, + &turn.context_task, + turn.repository_context_enabled, &mut super::verify_outcome::VerifyOutcomeState { - obligation_nudge_fired: &mut flags.obligation_nudge_fired, - force_tools_next: &mut flags.force_tools_next, - verified_at: &mut verified_at, - independent_review_status: &mut independent_review_status, - independent_review_repairs: &mut independent_review_repairs, - stalled_unfinished: &mut flags.stalled_unfinished, - verification_infrastructure_error: &mut verification_infrastructure_error, - verification_unstable: &mut verification_unstable, - last_verify_attributions: &mut last_verify_attributions, - ranked_context_paths: &mut ranked_context_paths, - context_generation_seen: &mut context_generation_seen, - indexed_ledger_revision: &mut indexed_ledger_revision, + obligation_nudge_fired: &mut turn.flags.obligation_nudge_fired, + force_tools_next: &mut turn.flags.force_tools_next, + verified_at: &mut turn.verified_at, + independent_review_status: &mut turn.independent_review_status, + independent_review_repairs: &mut turn.independent_review_repairs, + stalled_unfinished: &mut turn.flags.stalled_unfinished, + verification_infrastructure_error: &mut turn.verification_infrastructure_error, + verification_unstable: &mut turn.verification_unstable, + last_verify_attributions: &mut turn.last_verify_attributions, + ranked_context_paths: &mut turn.ranked_context_paths, + context_generation_seen: &mut turn.context_generation_seen, + indexed_ledger_revision: &mut turn.indexed_ledger_revision, }, ui, ) @@ -673,12 +683,12 @@ impl crate::Agent { // Seal first: checkpoint creation may take long enough for an owned // process or editor to move the tree. The authoritative reconciliation // below therefore happens after this final asynchronous safety step. - if turn_checkpoint_created && !self.seal_turn_checkpoint(ui).await? { - turn_checkpoint_created = false; + if turn.turn_checkpoint_created && !self.seal_turn_checkpoint(ui).await? { + turn.turn_checkpoint_created = false; // Default YOLO permits checkpoint-free mutation. A seal failure // must be silent and non-terminal there; strict confirmation mode // still treats loss of its promised undo record as incomplete. - flags.stalled_unfinished |= !self.config.gates.allow_no_checkpoint; + turn.flags.stalled_unfinished |= !self.config.gates.allow_no_checkpoint; } // The ledger is the authoritative source for exact effects, including // shell/delegate/background changes that did not flow through a file @@ -689,21 +699,21 @@ impl crate::Agent { ( ledger.revision(), ledger.workspace_revision(), - ledger.changes_since(turn_ledger_revision), + ledger.changes_since(turn.turn_ledger_revision), ) }; { let delta = { let ledger = self.runtime.ledger(); - match verified_at.as_ref() { + match turn.verified_at.as_ref() { Some((revision, _)) => ledger.changes_since(*revision), None => ledger_changes.clone(), } }; super::settlement::reconcile_verified_revision( &mut self.report.last_verify, - &mut verified_at, - &mut independent_review_status, + &mut turn.verified_at, + &mut turn.independent_review_status, final_ledger_revision, final_workspace_revision.clone(), &delta, @@ -715,39 +725,39 @@ impl crate::Agent { .map(|change| change.path.clone()) .collect(); self.workspace.last_file_changes = ledger_changes; - self.report.last_compat_fallbacks = compat_fallbacks; + self.report.last_compat_fallbacks = turn.compat_fallbacks.clone(); // 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.report.last_turn_telemetry = build_turn_telemetry( - max_steps, - verifier.round(), - empty_retries, - repeat_nudges, - continue_total_nudges, - truncation_total_retries, - &progress_tracker, - flags.ended_at_cap, - flags.stalled_unfinished, - flags.stalled_repeating, - &last_verify_attributions, - verifier.executions(), - sched_tool_calls, - sched_max_concurrent, - sched_serial_runs, - &tool_timeline, - &evidence, - &review_repair, + turn.max_steps, + turn.verifier.round(), + turn.empty_retries, + turn.repeat_nudges, + turn.continue_total_nudges, + turn.truncation_total_retries, + &turn.progress_tracker, + turn.flags.ended_at_cap, + turn.flags.stalled_unfinished, + turn.flags.stalled_repeating, + &turn.last_verify_attributions, + turn.verifier.executions(), + turn.sched_tool_calls, + turn.sched_max_concurrent, + turn.sched_serial_runs, + &turn.tool_timeline, + &turn.evidence, + &turn.review_repair, ); self.report.last_turn_telemetry.checkpoint_available = - turn_checkpoint_allowed.map(|_| turn_checkpoint_created); - self.report.last_turn_telemetry.advertised_tools = advertised_tool_names.into_iter().collect(); - self.report.last_turn_telemetry.tool_schema_tokens = tool_schema_tokens; + turn.turn_checkpoint_allowed.map(|_| turn.turn_checkpoint_created); + self.report.last_turn_telemetry.advertised_tools = turn.advertised_tool_names.iter().cloned().collect(); + self.report.last_turn_telemetry.tool_schema_tokens = turn.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 + // learned skill. The ground-truth turn.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.report.last_verify == Some(true) @@ -776,12 +786,12 @@ impl crate::Agent { // on step cap / stall (work may be incomplete). self.set_turn_phase(TurnPhase::Finalize); if self.config.memory.finalize - && flags.made_tool_call - && !flags.ended_at_cap - && !flags.stalled_unfinished - && !flags.stalled_repeating + && turn.flags.made_tool_call + && !turn.flags.ended_at_cap + && !turn.flags.stalled_unfinished + && !turn.flags.stalled_repeating && !self.workspace.last_changed_files.is_empty() - && steps < max_steps + && steps < turn.max_steps { self.finalize_turn(turn_start, ui).await; // finalize_turn appended a [user: finalize-nudge][assistant: recap] @@ -801,21 +811,21 @@ impl crate::Agent { ( ledger.revision(), ledger.workspace_revision(), - ledger.changes_since(turn_ledger_revision), + ledger.changes_since(turn.turn_ledger_revision), ) }; { let delta = { let ledger = self.runtime.ledger(); - match verified_at.as_ref() { + match turn.verified_at.as_ref() { Some((revision, _)) => ledger.changes_since(*revision), None => settled_changes.clone(), } }; super::settlement::reconcile_verified_revision( &mut self.report.last_verify, - &mut verified_at, - &mut independent_review_status, + &mut turn.verified_at, + &mut turn.independent_review_status, settled_revision, settled_digest.clone(), &delta, @@ -833,26 +843,26 @@ impl crate::Agent { // Keep the pre-turn goal until every user/session callback has // finished. A late workspace mutation must also roll back progress // that this hook tentatively advances. - let goal_before_final_settlement = goal_before.clone(); + let goal_before_final_settlement = turn.goal_before.clone(); let goal_invalidated_verification = self .goal_turn_end( super::super::goal_turn::GoalTurnState { - stalled_unfinished: flags.stalled_unfinished, - stalled_repeating: flags.stalled_repeating, - hit_step_cap: flags.ended_at_cap, - plan_updated_goal, - proposed_goal, - goal_before, - verified_at: verified_at.as_ref(), - turn_ledger_revision, + stalled_unfinished: turn.flags.stalled_unfinished, + stalled_repeating: turn.flags.stalled_repeating, + hit_step_cap: turn.flags.ended_at_cap, + plan_updated_goal: turn.plan_updated_goal, + proposed_goal: turn.proposed_goal.clone(), + goal_before: turn.goal_before.clone(), + verified_at: turn.verified_at.as_ref(), + turn_ledger_revision: turn.turn_ledger_revision, }, ui, ) .await; if goal_invalidated_verification { - verified_at = None; - if independent_review_status == ReviewStatus::Passed { - independent_review_status = ReviewStatus::Unavailable; + turn.verified_at = None; + if turn.independent_review_status == ReviewStatus::Passed { + turn.independent_review_status = ReviewStatus::Unavailable; } } @@ -868,7 +878,7 @@ impl crate::Agent { self.persist()?; // `goal_turn_end`, `Ui::turn_end`, and a session sink are extension - // points outside the verifier. Reconcile after all of them and before + // points outside the turn.verifier. Reconcile after all of them and before // constructing the typed outcome so none can create a false current- // revision pass. There are deliberately no callbacks after this // settlement point. @@ -878,21 +888,21 @@ impl crate::Agent { (ledger.revision(), ledger.workspace_revision()) }; let changed_after_final_hooks = self.report.last_verify == Some(true) - && verified_at.as_ref().is_none_or(|(revision, digest)| { + && turn.verified_at.as_ref().is_none_or(|(revision, digest)| { *revision != outcome_revision || digest != &outcome_digest }); if changed_after_final_hooks { let delta = { let ledger = self.runtime.ledger(); - match verified_at.as_ref() { + match turn.verified_at.as_ref() { Some((revision, _)) => ledger.changes_since(*revision), - None => ledger.changes_since(turn_ledger_revision), + None => ledger.changes_since(turn.turn_ledger_revision), } }; let wiped = super::settlement::reconcile_verified_revision_with_message( &mut self.report.last_verify, - &mut verified_at, - &mut independent_review_status, + &mut turn.verified_at, + &mut turn.independent_review_status, outcome_revision, outcome_digest.clone(), &delta, @@ -922,8 +932,8 @@ impl crate::Agent { let (final_changes, turn_had_mutation) = { let ledger = self.runtime.ledger(); ( - ledger.changes_since(turn_ledger_revision), - ledger.had_mutation_since(turn_ledger_revision), + ledger.changes_since(turn.turn_ledger_revision), + ledger.had_mutation_since(turn.turn_ledger_revision), ) }; self.workspace.last_changed_files = final_changes @@ -941,18 +951,18 @@ impl crate::Agent { // check was expected and missing. 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, + turn.verification_infrastructure_error, + turn.verification_unstable, self.report.last_verify, &self.workspace.last_changed_files, turn_had_mutation, no_check_executed, - independent_review_status, + turn.independent_review_status, self.report.last_turn_telemetry.skeptic_last_status, - flags.ended_at_cap, - flags.stalled_unfinished, - flags.stalled_repeating, - expected_mutation, + turn.flags.ended_at_cap, + turn.flags.stalled_unfinished, + turn.flags.stalled_repeating, + turn.expected_mutation, self.config.gates.allow_unverified, ); // Outer `run_turn` also stamps Done (covers `?` paths); keep the success path explicit. @@ -964,15 +974,14 @@ impl crate::Agent { stop_reason, changed_files: self.workspace.last_changed_files.clone(), verified_workspace_revision: (verification == VerificationStatus::Passed) - .then(|| verified_at.as_ref().map(|(_, digest)| digest.clone())) + .then(|| turn.verified_at.as_ref().map(|(_, digest)| digest.clone())) .flatten(), effective_route: effective_model_route( &self.config, - effective_fallback_route.as_deref(), + turn.effective_fallback_route.as_deref(), ), }; - self.report.last_effective_route = outcome.effective_route.clone(); - self.report.last_turn_outcome = Some(outcome.clone()); + self.report.set_outcome(outcome.clone()); self.workspace.clear_active_baselines(); Ok(outcome) } diff --git a/crates/hi-agent/src/agent/turn/mod.rs b/crates/hi-agent/src/agent/turn/mod.rs index ffc67de..4c46be7 100644 --- a/crates/hi-agent/src/agent/turn/mod.rs +++ b/crates/hi-agent/src/agent/turn/mod.rs @@ -24,6 +24,7 @@ mod fast_feedback; mod finalize; mod helpers; mod loop_; +mod model_request; mod model_round; mod model_retry; mod obligation; @@ -32,6 +33,7 @@ mod progress; mod retry; mod setup; mod settlement; +mod state; mod steer; mod tools; mod verify_outcome; diff --git a/crates/hi-agent/src/agent/turn/model_request.rs b/crates/hi-agent/src/agent/turn/model_request.rs new file mode 100644 index 0000000..c4b350c --- /dev/null +++ b/crates/hi-agent/src/agent/turn/model_request.rs @@ -0,0 +1,61 @@ +//! ChatRequest assembly helpers for the Model phase. +//! +//! Keeps tool-list filtering and schema accounting out of the main +//! `run_model_round` body. + +use std::collections::BTreeSet; +use std::sync::Arc; + +use hi_ai::ToolSpec; + +use super::retry::estimate_tool_schema_tokens; + +/// Drop coordination/bookkeeping tools when the one-shot suppress flag is set, +/// unless that would leave the list empty under a required tool choice. +pub(super) fn apply_bookkeeping_suppress( + tools: Arc<[ToolSpec]>, + suppress: bool, +) -> Arc<[ToolSpec]> { + if !suppress { + return tools; + } + if tools.iter().any(|tool| !hi_tools::is_coordination(&tool.name)) { + tools + .iter() + .filter(|tool| !hi_tools::is_coordination(&tool.name)) + .cloned() + .collect::>() + .into() + } else { + // Cannot suppress — would empty the list. + tools + } +} + +/// Track advertised names and peak schema tokens for telemetry. +pub(super) fn note_advertised_tools( + tools: &[ToolSpec], + advertised: &mut BTreeSet, + tool_schema_tokens: &mut u64, +) -> u64 { + advertised.extend(tools.iter().map(|tool| tool.name.clone())); + let tokens = estimate_tool_schema_tokens(tools); + *tool_schema_tokens = (*tool_schema_tokens).max(tokens); + tokens +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn suppress_noop_when_flag_clear() { + let tools: Arc<[ToolSpec]> = Arc::from([ToolSpec { + name: "bash".into(), + description: String::new(), + parameters: serde_json::json!({"type": "object"}), + }]); + let out = apply_bookkeeping_suppress(tools.clone(), false); + assert_eq!(out.len(), 1); + } +} diff --git a/crates/hi-agent/src/agent/turn/model_round.rs b/crates/hi-agent/src/agent/turn/model_round.rs index 23b9fc2..41cc25b 100644 --- a/crates/hi-agent/src/agent/turn/model_round.rs +++ b/crates/hi-agent/src/agent/turn/model_round.rs @@ -301,22 +301,13 @@ impl crate::Agent { let mut request_tools = self.request_tools_for(tool_availability_mode); if suppress_bookkeeping_tools_next { suppress_bookkeeping_tools_next = false; - // Only withhold when other tools remain — an empty tool - // list with tool_choice=required would be a provider error. - if request_tools - .iter() - .any(|tool| !hi_tools::is_coordination(&tool.name)) - { - request_tools = request_tools - .iter() - .filter(|tool| !hi_tools::is_coordination(&tool.name)) - .cloned() - .collect(); - } + request_tools = super::model_request::apply_bookkeeping_suppress(request_tools, true); } - advertised_tool_names.extend(request_tools.iter().map(|tool| tool.name.clone())); - let request_tool_schema_tokens = estimate_tool_schema_tokens(&request_tools); - tool_schema_tokens = tool_schema_tokens.max(request_tool_schema_tokens); + let request_tool_schema_tokens = super::model_request::note_advertised_tools( + &request_tools, + &mut advertised_tool_names, + &mut tool_schema_tokens, + ); let context_preflight = match self.ensure_request_fits_context( input, turn_start, diff --git a/crates/hi-agent/src/agent/turn/setup.rs b/crates/hi-agent/src/agent/turn/setup.rs index 457f108..ac578ee 100644 --- a/crates/hi-agent/src/agent/turn/setup.rs +++ b/crates/hi-agent/src/agent/turn/setup.rs @@ -74,7 +74,7 @@ impl crate::Agent { (None, index) => index, }; if refreshed != self.task.task_context { - self.task.task_context = refreshed; + self.task.set_task_context(refreshed); } } diff --git a/crates/hi-agent/src/agent/turn/state.rs b/crates/hi-agent/src/agent/turn/state.rs new file mode 100644 index 0000000..f7992d4 --- /dev/null +++ b/crates/hi-agent/src/agent/turn/state.rs @@ -0,0 +1,178 @@ +//! Owned per-turn state bag for the interactive loop. +//! +//! Built once in `run_turn_body` and passed through Model / Tools / Steer / +//! WorkspaceRepair instead of a long list of locals + borrow bags. + +use std::collections::BTreeSet; + +use crate::domain::TurnControlFlags; +use crate::steering::{ + EvidenceTracker, ImplementationIntent, ImplementationTracker, MutationRecovery, ReviewIntent, + ToolLoopGuardrail, +}; +use crate::verify::{Snapshot, WorkspaceRepairVerifier}; +use crate::{ReviewStatus, TaskContract, ToolCallEntry}; +use crate::agent::turn::fast_feedback::FastFeedbackState; +use crate::agent::turn::progress::ProgressTracker; +use crate::agent::turn::retry::{ReviewRepairState, TurnRetryState}; + +/// All mutable state that lives for one `run_turn` invocation. +pub(super) struct TurnState { + // --- identity / setup --- + pub user_prompt_tokens: u64, + pub turn_ledger_revision: u64, + pub turn_background_baseline: Vec, + pub context_task: String, + pub goal_drive_turn: bool, + pub task_contract: TaskContract, + pub repository_context_enabled: bool, + pub ranked_context_paths: BTreeSet, + pub context_generation_seen: u64, + pub indexed_ledger_revision: u64, + pub read_only_intent: Option, + pub implementation_intent: Option, + pub expected_mutation: bool, + /// When set, inspection-sprawl caps apply (same type as read-only intent). + pub inspection_sprawl_intent: Option, + pub read_only_inspection_cap: Option, + pub turn_input: String, + + // --- checkpoints / verify harness --- + pub turn_checkpoint_allowed: Option, + pub turn_checkpoint_created: bool, + pub verifier: WorkspaceRepairVerifier, + pub fast_feedback: FastFeedbackState, + pub max_steps: u32, + pub max_parallel_tools: usize, + + // --- loop budgets --- + pub steps: u32, + pub empty_retries: u32, + pub truncation_retries: u32, + pub truncation_total_retries: u32, + pub silent_continues: u32, + pub continue_total_nudges: u32, + pub repeat_nudges: u32, + pub repeat_sampling_rounds: u32, + + // --- control / trackers --- + pub flags: TurnControlFlags, + pub mutation_recovery: MutationRecovery, + pub plan_updated_goal: bool, + pub proposed_goal: Option, + pub goal_before: Option, + pub progress_tracker: ProgressTracker, + pub evidence: EvidenceTracker, + pub implementation_tracker: ImplementationTracker, + pub review_repair: ReviewRepairState, + pub tool_guardrail: ToolLoopGuardrail, + pub empty_tui_needs_project: bool, + + // --- scheduler / tools --- + pub sched_tool_calls: u32, + pub sched_max_concurrent: u32, + pub sched_serial_runs: u32, + pub tool_timeline: Vec, + pub advertised_tool_names: BTreeSet, + pub tool_schema_tokens: u64, + pub prev_call_sig: Option>, + pub prev_added_no_evidence: bool, + + // --- provider retry --- + pub retry_state: TurnRetryState, + pub request_max_tokens_override: Option, + pub compat_fallbacks: Vec, + pub effective_fallback_route: Option, + + // --- verify / settle --- + pub independent_review_status: ReviewStatus, + pub independent_review_repairs: u32, + pub verification_infrastructure_error: bool, + pub verification_unstable: bool, + pub verified_at: Option<(u64, String)>, + pub last_verify_attributions: Vec, + pub turn_snapshot: Option, + pub turn_start: usize, +} + +impl TurnState { + /// Project verify-outcome mutables without a separate lifetime bag. + pub(super) fn as_verify_outcome_state( + &mut self, + ) -> super::verify_outcome::VerifyOutcomeState<'_> { + super::verify_outcome::VerifyOutcomeState { + obligation_nudge_fired: &mut self.flags.obligation_nudge_fired, + force_tools_next: &mut self.flags.force_tools_next, + verified_at: &mut self.verified_at, + independent_review_status: &mut self.independent_review_status, + independent_review_repairs: &mut self.independent_review_repairs, + stalled_unfinished: &mut self.flags.stalled_unfinished, + verification_infrastructure_error: &mut self.verification_infrastructure_error, + verification_unstable: &mut self.verification_unstable, + last_verify_attributions: &mut self.last_verify_attributions, + ranked_context_paths: &mut self.ranked_context_paths, + context_generation_seen: &mut self.context_generation_seen, + indexed_ledger_revision: &mut self.indexed_ledger_revision, + } + } + + /// Project model-round mutables from this owned bag. + pub(super) fn as_model_round_state(&mut self) -> super::model_round::ModelRoundState<'_> { + super::model_round::ModelRoundState { + steps: &mut self.steps, + empty_retries: &mut self.empty_retries, + truncation_retries: &mut self.truncation_retries, + truncation_total_retries: &mut self.truncation_total_retries, + silent_continues: &mut self.silent_continues, + continue_total_nudges: &mut self.continue_total_nudges, + repeat_nudges: &mut self.repeat_nudges, + repeat_sampling_rounds: &mut self.repeat_sampling_rounds, + force_tools_next: &mut self.flags.force_tools_next, + text_tool_fallback_next: &mut self.flags.text_tool_fallback_next, + force_text_answer_next: &mut self.flags.force_text_answer_next, + force_no_progress_final_answer_next: &mut self.flags.force_no_progress_final_answer_next, + suppress_bookkeeping_tools_next: &mut self.flags.suppress_bookkeeping_tools_next, + made_tool_call: &mut self.flags.made_tool_call, + stalled_repeating: &mut self.flags.stalled_repeating, + stalled_unfinished: &mut self.flags.stalled_unfinished, + ended_at_cap: &mut self.flags.ended_at_cap, + prev_added_no_evidence: &mut self.prev_added_no_evidence, + turn_start: &mut self.turn_start, + context_generation_seen: &mut self.context_generation_seen, + indexed_ledger_revision: &mut self.indexed_ledger_revision, + sched_tool_calls: &mut self.sched_tool_calls, + sched_max_concurrent: &mut self.sched_max_concurrent, + sched_serial_runs: &mut self.sched_serial_runs, + tool_schema_tokens: &mut self.tool_schema_tokens, + prev_call_sig: &mut self.prev_call_sig, + retry_state: &mut self.retry_state, + request_max_tokens_override: &mut self.request_max_tokens_override, + compat_fallbacks: &mut self.compat_fallbacks, + effective_fallback_route: &mut self.effective_fallback_route, + ranked_context_paths: &mut self.ranked_context_paths, + progress_tracker: &mut self.progress_tracker, + evidence: &mut self.evidence, + implementation_tracker: &mut self.implementation_tracker, + review_repair: &mut self.review_repair, + tool_guardrail: &mut self.tool_guardrail, + last_verify_attributions: &mut self.last_verify_attributions, + tool_timeline: &mut self.tool_timeline, + advertised_tool_names: &mut self.advertised_tool_names, + turn_snapshot: &mut self.turn_snapshot, + max_steps: self.max_steps, + context_task: &self.context_task, + repository_context_enabled: self.repository_context_enabled, + turn_ledger_revision: self.turn_ledger_revision, + read_only_intent: self.read_only_intent, + implementation_intent: self.implementation_intent, + read_only_inspection_cap: self.read_only_inspection_cap, + expected_mutation: self.expected_mutation, + input: &self.turn_input, + user_prompt_tokens: self.user_prompt_tokens, + inspection_sprawl_intent: self.inspection_sprawl_intent, + verifier: &self.verifier, + } + } +} + + diff --git a/crates/hi-agent/src/agent/turn/steer/impl_cascade.rs b/crates/hi-agent/src/agent/turn/steer/impl_cascade.rs new file mode 100644 index 0000000..6113497 --- /dev/null +++ b/crates/hi-agent/src/agent/turn/steer/impl_cascade.rs @@ -0,0 +1,178 @@ +//! Table-driven implementation completeness gates (text-only Steer). +//! +//! Separate from [`super::cascade`] (review quality): different counters, +//! budgets (hardcoded ×2), and force-flag semantics. + +use crate::steering::{ + IMPLEMENTATION_NO_CHANGES_NUDGE, IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE, ImplementationIntent, + ImplementationTracker, implementation_missing_validation_nudge, implementation_text_tool_nudge, +}; + +/// Ordered implementation completeness steps after unfinished/plan gates. +pub(super) const IMPLEMENTATION_COMPLETENESS_CASCADE: &[ImplementationGate] = &[ + ImplementationGate::NoChanges, + ImplementationGate::ScaffoldOnly, + ImplementationGate::MissingValidation, +]; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(super) enum ImplementationGate { + NoChanges, + ScaffoldOnly, + MissingValidation, +} + +impl ImplementationGate { + pub(super) fn budget(self) -> u32 { + 2 + } + + fn counter(self, tracker: &ImplementationTracker) -> u32 { + match self { + Self::NoChanges => tracker.no_change_nudges, + Self::ScaffoldOnly => tracker.scaffold_only_nudges, + Self::MissingValidation => tracker.missing_validation_nudges, + } + } + + fn bump(self, tracker: &mut ImplementationTracker) { + match self { + Self::NoChanges => tracker.no_change_nudges += 1, + Self::ScaffoldOnly => tracker.scaffold_only_nudges += 1, + Self::MissingValidation => tracker.missing_validation_nudges += 1, + } + } +} + +#[derive(Debug)] +pub(super) enum ImplementationCascadeAction { + Repair { + gate: ImplementationGate, + status: &'static str, + nudge_body: String, + force_tools: bool, + text_tool_fallback: bool, + }, + Exhausted { + status: &'static str, + }, +} + +/// First matching implementation completeness gate, or `None` if satisfied. +pub(super) fn select_implementation_completeness( + implementation_intent: Option, + tracker: &ImplementationTracker, +) -> Option { + if implementation_intent.is_none() { + return None; + } + for &gate in IMPLEMENTATION_COMPLETENESS_CASCADE { + if let Some(action) = evaluate_gate(gate, tracker) { + return Some(action); + } + } + None +} + +fn evaluate_gate( + gate: ImplementationGate, + tracker: &ImplementationTracker, +) -> Option { + let applies = match gate { + ImplementationGate::NoChanges => !tracker.mutation_seen, + ImplementationGate::ScaffoldOnly => { + tracker.mutation_seen && !tracker.substantive_edit_seen + } + ImplementationGate::MissingValidation => { + tracker.mutation_seen && !tracker.validation_after_last_mutation + } + }; + if !applies { + return None; + } + let used = gate.counter(tracker); + if used < gate.budget() { + let next = used + 1; + let use_text_fallback = next >= gate.budget(); + let (status, body) = match gate { + ImplementationGate::NoChanges => ( + "implementation answer had no file changes; nudging the model to edit or scaffold", + IMPLEMENTATION_NO_CHANGES_NUDGE.to_string(), + ), + ImplementationGate::ScaffoldOnly => ( + "implementation only scaffolded setup files; nudging the model to edit source files", + IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE.to_string(), + ), + ImplementationGate::MissingValidation => ( + "implementation changed files without validation; nudging the model to run tests or build", + implementation_missing_validation_nudge(tracker), + ), + }; + let nudge_body = if use_text_fallback { + implementation_text_tool_nudge(&body) + } else { + body + }; + Some(ImplementationCascadeAction::Repair { + gate, + status, + nudge_body, + force_tools: !use_text_fallback, + text_tool_fallback: use_text_fallback, + }) + } else { + let status = match gate { + ImplementationGate::NoChanges => { + "implementation still had no file changes after repair" + } + ImplementationGate::ScaffoldOnly => { + "implementation still only had scaffold/setup changes after repair" + } + ImplementationGate::MissingValidation => { + "implementation still lacked validation after repair" + } + }; + Some(ImplementationCascadeAction::Exhausted { status }) + } +} + +/// Apply counter bump when spending a repair action. +pub(super) fn spend_implementation_gate( + gate: ImplementationGate, + tracker: &mut ImplementationTracker, +) { + gate.bump(tracker); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cascade_order_is_no_change_scaffold_validation() { + assert_eq!( + IMPLEMENTATION_COMPLETENESS_CASCADE, + &[ + ImplementationGate::NoChanges, + ImplementationGate::ScaffoldOnly, + ImplementationGate::MissingValidation, + ] + ); + } + + #[test] + fn no_mutation_selects_no_changes() { + let tracker = ImplementationTracker::default(); + let action = select_implementation_completeness( + Some(ImplementationIntent { tui: false }), + &tracker, + ); + assert!(matches!( + action, + Some(ImplementationCascadeAction::Repair { + gate: ImplementationGate::NoChanges, + .. + }) + )); + } +} diff --git a/crates/hi-agent/src/agent/turn/steer/mod.rs b/crates/hi-agent/src/agent/turn/steer/mod.rs index 5d1b455..72e101a 100644 --- a/crates/hi-agent/src/agent/turn/steer/mod.rs +++ b/crates/hi-agent/src/agent/turn/steer/mod.rs @@ -7,6 +7,7 @@ //! Workspace compile/lint/test repair stays in [`super::verify_run`]. mod cascade; +mod impl_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 bdbbe78..9f05ba6 100644 --- a/crates/hi-agent/src/agent/turn/steer/review.rs +++ b/crates/hi-agent/src/agent/turn/steer/review.rs @@ -5,9 +5,7 @@ use hi_ai::Content; use crate::heuristics::looks_like_unfinished_step; use crate::steering::{ - EvidenceTracker, IMPLEMENTATION_NO_CHANGES_NUDGE, IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE, - ImplementationIntent, ImplementationTracker, ReviewIntent, - implementation_missing_validation_nudge, implementation_text_tool_nudge, + EvidenceTracker, ImplementationIntent, ImplementationTracker, ReviewIntent, repair_nudge_with_required_next, summarize_inspected_evidence_nudge, }; use crate::transcript::NudgeKind; @@ -104,98 +102,35 @@ if let Some(intent) = read_only_intent return RoundControl::Continue; } } -if implementation_intent.is_some() && !implementation_tracker.mutation_seen { - if implementation_tracker.no_change_nudges < 2 { - implementation_tracker.no_change_nudges += 1; - evidence.quality_repair_nudges = - evidence.quality_repair_nudges.saturating_add(1); - let use_text_fallback = implementation_tracker.no_change_nudges >= 2; - *force_tools_next = !use_text_fallback; - *text_tool_fallback_next = use_text_fallback; - ui.nudge( - "implementation answer had no file changes; nudging the model to edit or scaffold", - ); - self.messages - .push_assistant(std::mem::take(completion_content)); - let nudge = if use_text_fallback { - implementation_text_tool_nudge(IMPLEMENTATION_NO_CHANGES_NUDGE) - } else { - IMPLEMENTATION_NO_CHANGES_NUDGE.to_string() - }; - self.messages.push_nudge(NudgeKind::Continue, nudge); - return RoundControl::Continue; - } - - *stalled_unfinished = true; - ui.nudge("implementation still had no file changes after repair"); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if implementation_intent.is_some() - && implementation_tracker.mutation_seen - && !implementation_tracker.substantive_edit_seen -{ - if implementation_tracker.scaffold_only_nudges < 2 { - implementation_tracker.scaffold_only_nudges += 1; - evidence.quality_repair_nudges = - evidence.quality_repair_nudges.saturating_add(1); - let use_text_fallback = - implementation_tracker.scaffold_only_nudges >= 2; - *force_tools_next = !use_text_fallback; - *text_tool_fallback_next = use_text_fallback; - ui.nudge( - "implementation only scaffolded setup files; nudging the model to edit source files", - ); +// Table-driven implementation completeness (order = IMPLEMENTATION_COMPLETENESS_CASCADE). +match super::impl_cascade::select_implementation_completeness( + implementation_intent, + implementation_tracker, +) { + Some(super::impl_cascade::ImplementationCascadeAction::Repair { + gate, + status, + nudge_body, + force_tools, + text_tool_fallback, + }) => { + super::impl_cascade::spend_implementation_gate(gate, implementation_tracker); + evidence.quality_repair_nudges = evidence.quality_repair_nudges.saturating_add(1); + *force_tools_next = force_tools; + *text_tool_fallback_next = text_tool_fallback; + ui.nudge(status); self.messages .push_assistant(std::mem::take(completion_content)); - let nudge = if use_text_fallback { - implementation_text_tool_nudge(IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE) - } else { - IMPLEMENTATION_SCAFFOLD_ONLY_NUDGE.to_string() - }; - self.messages.push_nudge(NudgeKind::Continue, nudge); + self.messages.push_nudge(NudgeKind::Continue, nudge_body); return RoundControl::Continue; } - - *stalled_unfinished = true; - ui.nudge( - "implementation still only had scaffold/setup changes after repair", - ); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); -} -if implementation_intent.is_some() - && implementation_tracker.mutation_seen - && !implementation_tracker.validation_after_last_mutation -{ - if implementation_tracker.missing_validation_nudges < 2 { - implementation_tracker.missing_validation_nudges += 1; - evidence.quality_repair_nudges = - evidence.quality_repair_nudges.saturating_add(1); - let use_text_fallback = - implementation_tracker.missing_validation_nudges >= 2; - *force_tools_next = !use_text_fallback; - *text_tool_fallback_next = use_text_fallback; - ui.nudge( - "implementation changed files without validation; nudging the model to run tests or build", - ); - self.messages - .push_assistant(std::mem::take(completion_content)); - let validation_nudge = - implementation_missing_validation_nudge(&implementation_tracker); - let nudge = if use_text_fallback { - implementation_text_tool_nudge(&validation_nudge) - } else { - validation_nudge - }; - self.messages.push_nudge(NudgeKind::Continue, nudge); - return RoundControl::Continue; + Some(super::impl_cascade::ImplementationCascadeAction::Exhausted { status }) => { + *stalled_unfinished = true; + ui.nudge(status); + ui.status(INCOMPLETE_STATUS); + return RoundControl::BreakInner(false); } - - *stalled_unfinished = true; - ui.nudge("implementation still lacked validation after repair"); - ui.status(INCOMPLETE_STATUS); - return RoundControl::BreakInner(false); + None => {} } // Table-driven review quality cascade (order = REVIEW_QUALITY_CASCADE). match super::cascade::select_review_quality_repair( diff --git a/crates/hi-agent/src/agent/turn/tools.rs b/crates/hi-agent/src/agent/turn/tools/batch.rs similarity index 98% rename from crates/hi-agent/src/agent/turn/tools.rs rename to crates/hi-agent/src/agent/turn/tools/batch.rs index 76bfdcb..2aea0b7 100644 --- a/crates/hi-agent/src/agent/turn/tools.rs +++ b/crates/hi-agent/src/agent/turn/tools/batch.rs @@ -35,25 +35,25 @@ use crate::steering::implementation_tool_call_mutates; use crate::heuristics::plan_has_pending_steps; use crate::apply_plan_to_goal; -use super::helpers::{synthetic_tool_outcome, tool_entry, tool_satisfies_validation}; -use super::phase::TurnPhase; -use super::progress::{ +use super::super::helpers::{synthetic_tool_outcome, tool_entry, tool_satisfies_validation}; +use super::super::phase::TurnPhase; +use super::super::progress::{ ProgressKind, ProgressTracker, ToolProgressLabel, classify_tool_progress, signature_seen, }; /// Outcomes and counters produced by one Tools-phase batch. -pub(super) struct ToolBatchOutcome { - pub(super) hash_guard_applies: bool, - pub(super) hashable_idempotent_results: usize, - pub(super) repeated_idempotent_results: usize, - pub(super) tool_progress_labels: Vec, - pub(super) plan_changed_this_batch: bool, +pub(in crate::agent::turn) struct ToolBatchOutcome { + pub(in crate::agent::turn) hash_guard_applies: bool, + pub(in crate::agent::turn) hashable_idempotent_results: usize, + pub(in crate::agent::turn) repeated_idempotent_results: usize, + pub(in crate::agent::turn) tool_progress_labels: Vec, + pub(in crate::agent::turn) plan_changed_this_batch: bool, } impl crate::Agent { /// Execute `calls` for the current round and append assistant+results. #[allow(clippy::too_many_arguments)] - pub(super) async fn execute_tool_batch( + pub(in crate::agent::turn) async fn execute_tool_batch( &mut self, calls: &[(String, String, String)], completion_content: &mut Vec, @@ -73,7 +73,7 @@ impl crate::Agent { turn_snapshot: &mut Option, turn_checkpoint_allowed: &mut Option, turn_checkpoint_created: &mut bool, - fast_feedback: &mut super::fast_feedback::FastFeedbackState, + fast_feedback: &mut super::super::fast_feedback::FastFeedbackState, ui: &mut dyn Ui, ) -> Result { let hash_guard_applies = calls.iter().all(|(_, name, args)| { @@ -1052,11 +1052,11 @@ if !batch_mutated_paths.is_empty() { .task.last_task_contract .as_ref() .is_some_and(|c| c.wants_tests); - let report = super::fast_feedback::run_fast_feedback( + let report = super::super::fast_feedback::run_fast_feedback( &self.runtime, &paths, fast_feedback, - super::fast_feedback::FastFeedbackOptions { run_tests }, + super::super::fast_feedback::FastFeedbackOptions { run_tests }, ui, ) .await; diff --git a/crates/hi-agent/src/agent/turn/tools/mod.rs b/crates/hi-agent/src/agent/turn/tools/mod.rs new file mode 100644 index 0000000..a7fc94c --- /dev/null +++ b/crates/hi-agent/src/agent/turn/tools/mod.rs @@ -0,0 +1,7 @@ +//! Tool-batch execution (TurnPhase::Tools). +//! +//! - [`batch`] — dep-aware scheduler + execute path for one model round + +mod batch; + +pub(in crate::agent::turn) use batch::ToolBatchOutcome; diff --git a/crates/hi-agent/src/agent/turn/verify_outcome.rs b/crates/hi-agent/src/agent/turn/verify_outcome.rs index fc91cd7..b2c7992 100644 --- a/crates/hi-agent/src/agent/turn/verify_outcome.rs +++ b/crates/hi-agent/src/agent/turn/verify_outcome.rs @@ -151,7 +151,7 @@ impl crate::Agent { } VerifyOutcome::Passed => { ui.status("✓ verification passed"); - self.report.last_verify = Some(true); + self.report.set_verify(Some(true)); self.reconcile_workspace_changes()?; let (verified_revision, verified_digest, current_changes) = { let mut ledger = self.runtime.ledger(); @@ -245,7 +245,7 @@ impl crate::Agent { { *state.independent_review_repairs = 1; *state.independent_review_status = ReviewStatus::Objected; - self.report.last_verify = None; + self.report.set_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.report.last_verify = Some(false); + self.report.set_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.report.last_verify = None; + self.report.set_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.report.last_verify = Some(false); + self.report.set_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/domain.rs b/crates/hi-agent/src/domain.rs index b49b874..42388eb 100644 --- a/crates/hi-agent/src/domain.rs +++ b/crates/hi-agent/src/domain.rs @@ -181,6 +181,22 @@ impl TurnReportState { last_effective_route: route, } } + + /// Stamp the typed outcome and keep the effective route in sync. + pub(crate) fn set_outcome(&mut self, outcome: TurnOutcome) { + self.last_effective_route = outcome.effective_route.clone(); + self.last_turn_outcome = Some(outcome); + } + + /// Clear verify flag (e.g. cancel/fail finalizers). + pub(crate) fn clear_verify(&mut self) { + self.last_verify = None; + } + + /// Record a verify boolean for the settle/report surface. + pub(crate) fn set_verify(&mut self, passed: Option) { + self.last_verify = passed; + } } impl Default for TurnReportState { @@ -221,6 +237,74 @@ impl WorkspaceTurnState { self.active_turn_message_start = None; self.active_turn_background_baseline = None; } + + /// Install ledger-derived change lists for the last turn. + pub(crate) fn record_changes( + &mut self, + changes: Vec, + clear_verify: bool, + ) { + self.last_changed_files = changes.iter().map(|c| c.path.clone()).collect(); + self.last_file_changes = changes; + let _ = clear_verify; // verify clear lives on TurnReportState; callers pair both. + } + + /// Clear per-turn diff/stub caches at turn start. + pub(crate) fn clear_turn_caches(&mut self) { + self.turn_diff_cache = None; + self.turn_stub_scan_cache = None; + } + + /// Begin a turn: record ledger + background baselines. + pub(crate) fn begin_turn(&mut self, ledger_revision: u64, background_ids: Vec) { + self.active_turn_ledger_revision = Some(ledger_revision); + self.active_turn_message_start = None; + self.active_turn_background_baseline = Some(background_ids); + self.clear_turn_caches(); + } + + /// Mark the transcript index of the user message that opened this turn. + pub(crate) fn set_message_start(&mut self, start: usize) { + self.active_turn_message_start = Some(start); + } +} + +impl TaskContextState { + /// Refresh the ranked task context string when it changed. + pub(crate) fn set_task_context(&mut self, context: Option) { + self.task_context = context; + } + + /// Store the latest task prompt + derived contract. + pub(crate) fn set_task(&mut self, prompt: Option, contract: Option) { + self.last_task_prompt = prompt; + self.last_task_contract = contract; + } + + /// Refresh live memory injection text. + pub(crate) fn set_memory_context(&mut self, context: Option) { + self.memory_context = context; + } +} + +impl SubagentSessionState { + /// Try to consume one explore slot; returns the 1-based slot number or `None` if exhausted. + pub(crate) fn try_begin_explore(&mut self, max: u32) -> Option { + if self.explore_subagents_used >= max { + return None; + } + self.explore_subagents_used += 1; + Some(self.explore_subagents_used) + } + + /// Try to consume one delegate slot; returns the 1-based slot number or `None` if exhausted. + pub(crate) fn try_begin_delegate(&mut self, max: u32) -> Option { + if self.delegate_subagents_used >= max { + return None; + } + self.delegate_subagents_used += 1; + Some(self.delegate_subagents_used) + } } /// Session-scoped subagent caps and the optional write-capable runner. diff --git a/crates/hi-agent/src/lib.rs b/crates/hi-agent/src/lib.rs index 2e5ded7..818520a 100644 --- a/crates/hi-agent/src/lib.rs +++ b/crates/hi-agent/src/lib.rs @@ -81,8 +81,8 @@ pub use memory::{ pub use observation::{Observation, ObservationReceipt, ObservationSink}; pub use agent::turn::TurnPhase; pub use outcome::{ - EffectiveModelRoute, ReviewStatus, TopLevelErrorKind, TurnOutcome, TurnStatus, TurnStopReason, - VerificationStatus, + EffectiveModelRoute, ReviewStatus, SessionRollback, TopLevelErrorKind, TurnCleanupKind, + TurnCleanupResult, TurnOutcome, TurnStatus, TurnStopReason, VerificationStatus, }; pub use session::SessionSink; pub use skills::{ diff --git a/crates/hi-agent/src/outcome.rs b/crates/hi-agent/src/outcome.rs index 3cbabc5..69050c4 100644 --- a/crates/hi-agent/src/outcome.rs +++ b/crates/hi-agent/src/outcome.rs @@ -117,6 +117,32 @@ impl TurnOutcome { } } +/// How session state was handled before [`crate::Agent::cleanup_turn`] on cancel. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum SessionRollback { + /// Frontend already rewound transcript/goals/plan; agent must not truncate again. + AlreadyApplied, + /// Agent should undo new checkpoints (if any) and truncate to the turn message start. + AgentOwned { checkpoint_count_before: usize }, +} + +/// Abnormal turn teardown requested by a frontend (not used on successful `run_turn`). +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum TurnCleanupKind { + /// User interrupt / dropped turn future. + Cancel { session: SessionRollback }, + /// `run_turn` returned `Err` or escaped before the normal finalizer. + Fail, +} + +/// Result of [`crate::Agent::cleanup_turn`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct TurnCleanupResult { + pub outcome: TurnOutcome, + /// Background processes killed via the turn-scoped baseline (for UI copy). + pub killed_backgrounds: usize, +} + /// Coarse classification for top-level CLI errors that escape outside a typed /// [`TurnOutcome`] (setup/config/parse vs infrastructure). #[derive(Clone, Copy, Debug, PartialEq, Eq)] diff --git a/crates/hi-agent/src/tests.rs b/crates/hi-agent/src/tests.rs index f640c90..c8c5189 100644 --- a/crates/hi-agent/src/tests.rs +++ b/crates/hi-agent/src/tests.rs @@ -17,6 +17,7 @@ mod memory; mod mutation_recovery; mod outcome; mod plan; +mod protocol_import_lint; mod retry; mod scheduler; mod steering; diff --git a/crates/hi-agent/src/tests/protocol_import_lint.rs b/crates/hi-agent/src/tests/protocol_import_lint.rs new file mode 100644 index 0000000..e77b452 --- /dev/null +++ b/crates/hi-agent/src/tests/protocol_import_lint.rs @@ -0,0 +1,62 @@ +//! Soft firewall: turn-loop sources should prefer `hi_tools::protocol` for tool +//! protocol symbols and `hi_tools::infra` for product infrastructure. + +use std::fs; +use std::path::PathBuf; + +fn turn_rs_files() -> Vec { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("src/agent/turn"); + let mut out = Vec::new(); + fn walk(dir: &std::path::Path, out: &mut Vec) { + for entry in fs::read_dir(dir).unwrap() { + let entry = entry.unwrap(); + let path = entry.path(); + if path.is_dir() { + walk(&path, out); + } else if path.extension().and_then(|s| s.to_str()) == Some("rs") { + out.push(path); + } + } + } + walk(&root, &mut out); + out +} + +#[test] +fn turn_loop_prefers_protocol_namespace_for_execute() { + // New execute/mutation imports in the turn loop should use hi_tools::protocol. + let mut offenders = Vec::new(); + for path in turn_rs_files() { + let text = fs::read_to_string(&path).unwrap(); + for (i, line) in text.lines().enumerate() { + let trimmed = line.trim_start(); + if trimmed.starts_with("//") { + continue; + } + // Disallow root-path execute imports in new turn code. + if trimmed.contains("use hi_tools::{") + && (trimmed.contains("execute_in_runtime") + || trimmed.contains("prepare_mutation") + || trimmed.contains("execute_streaming")) + { + offenders.push(format!("{}:{}: {trimmed}", path.display(), i + 1)); + } + } + } + assert!( + offenders.is_empty(), + "turn loop should import execute/mutation helpers via hi_tools::protocol::…\n{}", + offenders.join("\n") + ); +} + +#[test] +fn turn_fast_feedback_uses_infra_namespace() { + let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("src/agent/turn/fast_feedback.rs"); + let text = fs::read_to_string(path).unwrap(); + assert!( + text.contains("use hi_tools::infra::"), + "fast_feedback should import via hi_tools::infra" + ); +} diff --git a/crates/hi-cli/src/main.rs b/crates/hi-cli/src/main.rs index 8d368e4..44f1757 100644 --- a/crates/hi-cli/src/main.rs +++ b/crates/hi-cli/src/main.rs @@ -538,15 +538,25 @@ async fn run() -> Result<()> { let result = if let Some(result) = result { result } else { - agent.kill_background_processes(); - if agent.checkpoint_count() > checkpoint_count_before_turn - && let Err(err) = agent.undo().await - { - eprintln!("\x1b[33mcouldn't roll back cancelled workspace edits: {err:#}\x1b[0m"); - } - agent.finalize_cancelled_turn() + agent + .cleanup_turn(hi_agent::TurnCleanupKind::Cancel { + session: hi_agent::SessionRollback::AgentOwned { + checkpoint_count_before: checkpoint_count_before_turn, + }, + }) + .await + .map(|r| r.outcome) + }; + let failed_outcome = match &result { + Err(_) => Some( + agent + .cleanup_turn(hi_agent::TurnCleanupKind::Fail) + .await + .map(|r| r.outcome) + .unwrap_or_else(|_| agent.finalize_failed_turn()), + ), + Ok(_) => None, }; - let failed_outcome = result.as_ref().err().map(|_| agent.finalize_failed_turn()); let rsi_summary = finish_turn_trace( rsi.observer.as_ref(), &agent, diff --git a/crates/hi-cli/src/repl.rs b/crates/hi-cli/src/repl.rs index 4812357..b1f3316 100644 --- a/crates/hi-cli/src/repl.rs +++ b/crates/hi-cli/src/repl.rs @@ -721,7 +721,6 @@ pub(crate) async fn repl( last_turn_start = checkpoint; let turn_snapshot = agent.state_snapshot(); last_turn_snapshot = Some(turn_snapshot.clone()); - let background_before = agent.background_process_ids(); let progress = Arc::new(AtomicBool::new(false)); let driven = { let mut plain = PlainUi::with_progress(progress.clone()); @@ -729,7 +728,6 @@ pub(crate) async fn repl( }; let cancelled = driven.is_none(); if cancelled { - let killed = agent.kill_background_processes_started_after(&background_before); if agent.checkpoint_count() > checkpoint_count && let Err(err) = agent.undo().await { @@ -744,6 +742,13 @@ pub(crate) async fn repl( agent.truncate_messages(checkpoint); agent.restore_state_snapshot(&turn_snapshot); } + let killed = agent + .cleanup_turn(hi_agent::TurnCleanupKind::Cancel { + session: hi_agent::SessionRollback::AlreadyApplied, + }) + .await + .map(|r| r.killed_backgrounds) + .unwrap_or(0); if killed > 0 { println!( "\x1b[33m^C — interrupted; turn discarded; killed {killed} background process(es) started by it\x1b[0m" @@ -758,9 +763,8 @@ pub(crate) async fn repl( "\x1b[33mgoal drive interrupted — paused; /goal resume to continue\x1b[0m" ); } - let _ = agent.finalize_cancelled_turn(); } else if driven.as_ref().is_some_and(Result::is_err) { - agent.finalize_failed_turn(); + let _ = agent.cleanup_turn(hi_agent::TurnCleanupKind::Fail).await; } else { // Long-horizon auto-drive: keep pulling toward an active goal. // Drive turns that change nothing count toward a stall stop; diff --git a/crates/hi-cli/src/sync.rs b/crates/hi-cli/src/sync.rs index ff03aa2..d8e1495 100644 --- a/crates/hi-cli/src/sync.rs +++ b/crates/hi-cli/src/sync.rs @@ -1407,7 +1407,7 @@ pub async fn run_daemon_loop( eprintln!("\x1b[31m{kind}: {err:#} — {guidance}\x1b[0m"); } if result.is_err() { - agent.finalize_failed_turn(); + let _ = agent.cleanup_turn(hi_agent::TurnCleanupKind::Fail).await; } // Flush sync records + live events to ipop. @@ -2238,7 +2238,7 @@ pub async fn run_resume_local( eprintln!("\x1b[31m{kind}: {err:#} — {guidance}\x1b[0m"); } if result.is_err() { - agent.finalize_failed_turn(); + let _ = agent.cleanup_turn(hi_agent::TurnCleanupKind::Fail).await; } if let Err(err) = sync_handle.flush().await { eprintln!("\x1b[33msync: {err:#}\x1b[0m"); diff --git a/crates/hi-tui/src/app/run/mod.rs b/crates/hi-tui/src/app/run/mod.rs index dbc3838..9f9c659 100644 --- a/crates/hi-tui/src/app/run/mod.rs +++ b/crates/hi-tui/src/app/run/mod.rs @@ -1857,7 +1857,7 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { tx, confirmations: confirm_tx, }; - let background_before = agent.background_process_ids(); + let _background_before = agent.background_process_ids(); let interject = agent.interjection_inbox(); let driven = { let fut = agent.run_turn(&run_line, &mut sink); @@ -1878,7 +1878,11 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { if let Some(outcome) = &driven.value { app.note_turn_outcome(outcome); } else if !cancelled { - let outcome = agent.finalize_failed_turn(); + let outcome = agent + .cleanup_turn(hi_agent::TurnCleanupKind::Fail) + .await + .map(|r| r.outcome) + .unwrap_or_else(|_| agent.finalize_failed_turn()); app.note_turn_outcome(&outcome); } app.set_working(false); @@ -1888,9 +1892,7 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { // Full cancellation cleanup — same as the // main turn path: kill bg processes, rewind // session state, finalize the cancellation. - let killed = agent.kill_background_processes_started_after( - &background_before, - ); + // Turn-scoped bg kill owned by cleanup_turn below. if agent.checkpoint_count() > checkpoint_count && let Err(err) = agent.undo().await { @@ -1911,8 +1913,16 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { agent.truncate_messages(checkpoint); agent.restore_state_snapshot(&turn_snapshot); } - match agent.finalize_cancelled_turn() { - Ok(outcome) => app.note_turn_outcome(&outcome), + let killed = match agent + .cleanup_turn(hi_agent::TurnCleanupKind::Cancel { + session: hi_agent::SessionRollback::AlreadyApplied, + }) + .await + { + Ok(r) => { + app.note_turn_outcome(&r.outcome); + r.killed_backgrounds + } Err(err) => { app.last_turn_state = TurnState::Cancelled; app.status = "cancelled".to_string(); @@ -1920,9 +1930,10 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { format!("couldn't finalize typed cancellation outcome: {err:#}"), Style::default().fg(crate::theme::theme().warning), )); + 0 } - } - let msg = if killed > 0 { + }; + let msg = if killed > 0 { format!( "trio: cancelled; killed {killed} background process(es)" ) @@ -2395,7 +2406,7 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { tx, confirmations: confirm_tx, }; - let background_before = agent.background_process_ids(); + let _background_before = agent.background_process_ids(); let interject = agent.interjection_inbox(); let driven = { let fut = agent.run_turn(&run_line, &mut sink); @@ -2420,12 +2431,16 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { // before its normal finalizer. Reconcile the surviving workspace // effects and retain the same typed infrastructure outcome used by // one-shot reports. - let outcome = agent.finalize_failed_turn(); + let outcome = agent + .cleanup_turn(hi_agent::TurnCleanupKind::Fail) + .await + .map(|r| r.outcome) + .unwrap_or_else(|_| agent.finalize_failed_turn()); app.note_turn_outcome(&outcome); } if cancelled { - let killed = agent.kill_background_processes_started_after(&background_before); + // Turn-scoped bg kill owned by cleanup_turn below. if agent.checkpoint_count() > checkpoint_count && let Err(err) = agent.undo().await { @@ -2442,8 +2457,16 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { agent.truncate_messages(checkpoint); agent.restore_state_snapshot(&turn_snapshot); } - match agent.finalize_cancelled_turn() { - Ok(outcome) => app.note_turn_outcome(&outcome), + let killed = match agent + .cleanup_turn(hi_agent::TurnCleanupKind::Cancel { + session: hi_agent::SessionRollback::AlreadyApplied, + }) + .await + { + Ok(r) => { + app.note_turn_outcome(&r.outcome); + r.killed_backgrounds + } Err(err) => { app.last_turn_state = TurnState::Cancelled; app.status = "cancelled".to_string(); @@ -2451,9 +2474,10 @@ pub async fn run(agent: &mut Agent, options: crate::RunOptions) -> Result<()> { format!("couldn't finalize typed cancellation outcome: {err:#}"), Style::default().fg(crate::theme::theme().warning), )); + 0 } - } - let dropped = app.queue.len(); + }; + let dropped = app.queue.len(); app.queue.clear(); app.mid_turn_offered.clear(); let msg = if dropped > 0 {