From 95f9ff2935e2e705c8903b30e0c4daa043d76ebb Mon Sep 17 00:00:00 2001 From: calvinsturm Date: Fri, 3 Jul 2026 18:14:51 -0700 Subject: [PATCH] Add validator-driven repair continuation --- src/agent.rs | 124 ++++++- src/agent/completion_policy.rs | 55 ++- src/agent/phase_transitions.rs | 33 +- src/agent/response_guards.rs | 13 + src/agent/run_finalize.rs | 62 +++- src/agent/runtime_completion.rs | 2 +- src/agent/runtime_effects.rs | 1 + src/agent/validators.rs | 144 ++++++++ src/agent_runtime/checkpoint.rs | 3 + src/agent_runtime/state.rs | 4 + src/agent_tests.rs | 593 ++++++++++++++++++++++++++++++++ src/cli_dispatch_eval_replay.rs | 1 + 12 files changed, 995 insertions(+), 40 deletions(-) create mode 100644 src/agent/validators.rs diff --git a/src/agent.rs b/src/agent.rs index c2cba51..cd906f6 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -41,6 +41,7 @@ pub mod task_contract; mod timeouts; pub mod tool_facts; mod tool_helpers; +pub(crate) mod validators; pub use agent_types::{ AgentExitReason, AgentOutcome, AgentTaintRecord, McpPinEnforcementMode, McpRuntimeTraceEntry, PlanStepConstraint, PlanToolEnforcementMode, PolicyLoadedInfo, ToolCallBudget, @@ -295,6 +296,7 @@ impl Agent

{ required_command: self .required_validation_command(user_prompt) .map(ToOwned::to_owned), + inferred_validator: None, satisfied: false, repair_mode: false, exact_final_answer_required: self.exact_final_answer_required(user_prompt), @@ -343,6 +345,17 @@ impl Agent

{ tool_calls: &mut Vec, ) -> bool { if tool_calls.len() == 1 && tool_calls[0].name == "shell" { + if normalized_shell_command(&tool_calls[0].arguments).as_deref() + != Some(required_command) + { + tool_calls[0] = ToolCall { + id: format!("tc_validation_shell_{}", Uuid::new_v4().simple()), + name: "shell".to_string(), + arguments: json!({ "command": required_command }), + }; + assistant.content = Some(String::new()); + return true; + } if !assistant .content .as_deref() @@ -447,7 +460,12 @@ impl Agent

{ if runtime_checkpoint.phase != crate::agent_runtime::state::RunPhase::Validating { return Ok(PhaseLoopControl::Proceed); } - if !self.validation_shell_available() { + if !self.validation_shell_available() + && runtime_checkpoint + .validation_state + .inferred_validator + .is_none() + { runtime_checkpoint.phase = crate::agent_runtime::state::RunPhase::Executing; runtime_checkpoint .retry_state @@ -477,13 +495,21 @@ impl Agent

{ ); } } + let validation_phase_message = runtime_checkpoint + .validation_state + .required_command + .as_deref() + .map(|command| { + crate::agent::completion_policy::required_validation_phase_message(command) + }) + .unwrap_or_else(|| self.required_validation_phase_message(user_prompt)); let decision = decide_required_validation_phase_response( user_prompt, self.validation_shell_available(), runtime_checkpoint, &resp.assistant, &resp.tool_calls, - self.required_validation_phase_message(user_prompt), + validation_phase_message, ); match apply_required_validation_guard_decision(decision, &resp.assistant, messages) { GuardEffect::Proceed => Ok(PhaseLoopControl::Proceed), @@ -2831,15 +2857,47 @@ impl Agent

{ Err(outcome) => return Err(outcome), } - if let Some(effect) = - completion_blocked_effect_from_post_tool_refresh(refresh_phase_state_from_tool_facts( - user_prompt, - runtime_checkpoint, - observed_tool_calls, - observed_tool_executions, - successful_write_tool_ok_this_step, - )) + let refresh = refresh_phase_state_from_tool_facts( + user_prompt, + runtime_checkpoint, + observed_tool_calls, + observed_tool_executions, + successful_write_tool_ok_this_step, + ); + if let crate::agent::phase_transitions::PostToolPhaseRefreshEffect::StopAfterRepairBudgetExhausted { + reason, + } = &refresh { + self.emit_event( + run_id, + step, + EventKind::Error, + serde_json::json!({ + "error": reason, + "source": "runtime_required_validation_guard", + "failure_class": "E_RUNTIME_VALIDATION_REPAIR_EXHAUSTED", + "repair_budget_max": 1 + }), + ); + return Err(self.finalize_planner_error_with_end( + step, + run_id.to_string(), + started_at.to_string(), + reason.clone(), + messages.clone(), + observed_tool_calls.to_vec(), + observed_tool_decisions.to_vec(), + context_size_chars(messages), + last_compaction_report.clone(), + hook_invocations.clone(), + *provider_retry_count, + *provider_error_count, + *saw_token_usage, + total_token_usage, + taint_state, + )); + } + if let Some(effect) = completion_blocked_effect_from_post_tool_refresh(refresh) { self.emit_phase_transition(run_id, step, &effect.transition); self.emit_event( run_id, @@ -2878,6 +2936,52 @@ impl Agent

{ successful_write_tool_ok_this_step: bool, request_context_chars: usize, ) -> Result { + if let Some(validator) = runtime_checkpoint + .validation_state + .inferred_validator + .clone() + { + if runtime_checkpoint.validation_state.satisfied + && !runtime_checkpoint + .validation_state + .exact_final_answer_required + && observed_tool_executions + .last() + .is_some_and(|execution| execution.name == "shell" && execution.ok) + { + let outcome = crate::agent::validators::ValidatorOutcome::Passed; + self.emit_event( + run_id, + step, + EventKind::CompletionSatisfied, + serde_json::json!({ + "reason": "inferred_validator_passed", + "validator_kind": validator.kind, + "validator_label": validator.label, + "command": validator.command, + "outcome": outcome + }), + ); + return Err(self.finalize_ok_with_end( + step, + run_id.to_string(), + started_at.to_string(), + format!("Validation passed: `{}`.", validator.command), + messages.clone(), + observed_tool_calls.to_vec(), + observed_tool_decisions.to_vec(), + context_size_chars(messages), + last_compaction_report.clone(), + hook_invocations.to_vec(), + *provider_retry_count, + *provider_error_count, + *saw_token_usage, + total_token_usage, + taint_state, + )); + } + } + if let Some(control) = self .handle_verified_write_follow_on_phase( user_prompt, diff --git a/src/agent/completion_policy.rs b/src/agent/completion_policy.rs index 9619882..8b1f374 100644 --- a/src/agent/completion_policy.rs +++ b/src/agent/completion_policy.rs @@ -1,7 +1,17 @@ pub(crate) enum VerifiedWriteCompletionDecision { FinalizeNow, StartFinalAnswerPhase(String), - StartRequiredValidationPhase(String), + StartRequiredValidationPhase { + developer_message: String, + command: String, + inferred_validator: Option, + }, +} + +pub(crate) struct RequiredValidationFollowOn { + pub(crate) developer_message: String, + pub(crate) command: String, + pub(crate) inferred_validator: Option, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -55,7 +65,7 @@ pub(crate) struct RuntimePhaseTransitionDecision { pub(crate) completion_reason: &'static str, } -fn required_validation_phase_message(required_command: &str) -> String { +pub(crate) fn required_validation_phase_message(required_command: &str) -> String { format!( "Validation required now. Return exactly one shell tool call and no prose. Run `{required_command}`. Example arguments: {{\"command\":\"{required_command}\"}}. After it succeeds, reply with the final answer only." ) @@ -302,18 +312,18 @@ pub(crate) fn decide_validation_phase_transition( } pub(crate) fn decide_verified_write_completion( - required_validation_command: Option<&str>, + required_validation: Option, exact_final_answer_required: bool, _verified_paths: &[String], has_post_tool_assistant_closeout: bool, _post_write_follow_on_turn_count: u32, ) -> VerifiedWriteCompletionDecision { - if required_validation_command.is_some() && !has_post_tool_assistant_closeout { - return VerifiedWriteCompletionDecision::StartRequiredValidationPhase( - required_validation_phase_message( - required_validation_command.unwrap_or("the required validation command"), - ), - ); + if let Some(required_validation) = required_validation { + return VerifiedWriteCompletionDecision::StartRequiredValidationPhase { + developer_message: required_validation.developer_message, + command: required_validation.command, + inferred_validator: required_validation.inferred_validator, + }; } if !has_post_tool_assistant_closeout { return VerifiedWriteCompletionDecision::StartFinalAnswerPhase( @@ -338,17 +348,32 @@ mod tests { post_validation_final_answer_transition_decision, required_validation_boundary_transition_decision, resume_phase_from_checkpoint_state, validation_resume_execution_transition_decision, RequiredValidationCompletionDecision, - RuntimeCheckpointResumeKind, ValidationFacts, ValidationPhaseTransitionDecision, - VerifiedWriteCompletionDecision, + RequiredValidationFollowOn, RuntimeCheckpointResumeKind, ValidationFacts, + ValidationPhaseTransitionDecision, VerifiedWriteCompletionDecision, }; use crate::agent::ToolFactV1; #[test] fn verified_write_policy_starts_validation_when_prompt_requires_it() { - let decision = decide_verified_write_completion(Some("cargo test"), false, &[], false, 0); + let decision = decide_verified_write_completion( + Some(RequiredValidationFollowOn { + developer_message: "run cargo test".to_string(), + command: "cargo test".to_string(), + inferred_validator: None, + }), + false, + &[], + false, + 0, + ); match decision { - VerifiedWriteCompletionDecision::StartRequiredValidationPhase(message) => { - assert!(message.contains("cargo test")); + VerifiedWriteCompletionDecision::StartRequiredValidationPhase { + developer_message, + command, + .. + } => { + assert!(developer_message.contains("cargo test")); + assert_eq!(command, "cargo test"); } _ => panic!("expected validation phase"), } @@ -589,6 +614,7 @@ mod tests { tool_protocol_state: crate::agent_runtime::state::ToolProtocolState::default(), validation_state: crate::agent_runtime::state::ValidationState { required_command: Some("cargo test".to_string()), + inferred_validator: None, satisfied: false, repair_mode: false, exact_final_answer_required: false, @@ -643,6 +669,7 @@ mod tests { tool_protocol_state: crate::agent_runtime::state::ToolProtocolState::default(), validation_state: crate::agent_runtime::state::ValidationState { required_command: Some("cargo test".to_string()), + inferred_validator: None, satisfied: true, repair_mode: false, exact_final_answer_required: true, diff --git a/src/agent/phase_transitions.rs b/src/agent/phase_transitions.rs index 5f07b00..894839b 100644 --- a/src/agent/phase_transitions.rs +++ b/src/agent/phase_transitions.rs @@ -17,6 +17,9 @@ pub(crate) enum PostToolPhaseRefreshEffect { transition: RuntimePhaseTransitionDecision, reason: String, }, + StopAfterRepairBudgetExhausted { + reason: String, + }, } pub(crate) struct VerifiedWriteFollowOnUpdate { @@ -136,6 +139,18 @@ pub(crate) fn refresh_phase_state_from_tool_facts( match decide_validation_phase_transition(&validation_facts, successful_write_tool_ok_this_step) { ValidationPhaseTransitionDecision::EnterRepair => { + let budget = crate::agent::validators::RepairBudget::one_repair( + runtime_checkpoint.retry_state.validation_repair_turn_count, + ); + if !budget.can_authorize_repair() { + return PostToolPhaseRefreshEffect::StopAfterRepairBudgetExhausted { + reason: "validation failed after the single allowed repair turn".to_string(), + }; + } + runtime_checkpoint.retry_state.validation_repair_turn_count = runtime_checkpoint + .retry_state + .validation_repair_turn_count + .saturating_add(1); runtime_checkpoint.phase = crate::agent_runtime::state::RunPhase::Executing; runtime_checkpoint.validation_state.repair_mode = true; runtime_checkpoint.validation_state.collecting_final_answer = false; @@ -171,7 +186,7 @@ pub(crate) fn refresh_phase_state_from_tool_facts( } pub(crate) fn apply_verified_write_follow_on( - user_prompt: &str, + _user_prompt: &str, runtime_checkpoint: &mut crate::agent_runtime::state::RunCheckpointV1, result: &VerifiedWriteResult, ) -> Option { @@ -199,24 +214,18 @@ pub(crate) fn apply_verified_write_follow_on( developer_message: message.clone(), }) } - VerifiedWriteResult::StartRequiredValidationPhase(message) => { + VerifiedWriteResult::StartRequiredValidationPhase(follow_on) => { runtime_checkpoint.retry_state.post_write_guard_retry_count = 0; runtime_checkpoint .retry_state .blocked_runtime_completion_count = 0; runtime_checkpoint.phase = crate::agent_runtime::state::RunPhase::Validating; - if runtime_checkpoint - .validation_state - .required_command - .is_none() - { - runtime_checkpoint.validation_state.required_command = - crate::agent_impl_guard::prompt_required_validation_command(user_prompt) - .map(str::to_string); - } + runtime_checkpoint.validation_state.required_command = Some(follow_on.command.clone()); + runtime_checkpoint.validation_state.inferred_validator = + follow_on.inferred_validator.clone(); Some(VerifiedWriteFollowOnUpdate { control: PhaseLoopControl::ContinueAgentStep, - developer_message: message.clone(), + developer_message: follow_on.developer_message.clone(), }) } VerifiedWriteResult::Done(_) => None, diff --git a/src/agent/response_guards.rs b/src/agent/response_guards.rs index f176b16..17e2a93 100644 --- a/src/agent/response_guards.rs +++ b/src/agent/response_guards.rs @@ -42,6 +42,19 @@ pub(crate) fn decide_required_validation_phase_response( return RequiredValidationPhaseDecision::Proceed; } if !validation_shell_available { + if runtime_checkpoint + .validation_state + .inferred_validator + .is_some() + { + runtime_checkpoint + .retry_state + .blocked_required_validation_phase_count = 0; + return RequiredValidationPhaseDecision::PlannerError { + reason: "inferred validator could not run because the shell tool is unavailable or disabled by policy".to_string(), + blocked_count: 0, + }; + } runtime_checkpoint.phase = RunPhase::Executing; runtime_checkpoint .retry_state diff --git a/src/agent/run_finalize.rs b/src/agent/run_finalize.rs index 34a8e23..ba0411f 100644 --- a/src/agent/run_finalize.rs +++ b/src/agent/run_finalize.rs @@ -139,8 +139,54 @@ impl Agent

{ .to_string(); let has_post_tool_assistant_closeout = self.has_user_facing_assistant_closeout_after_last_tool(&messages); + let prompt_required_validation = + self.required_validation_command(user_prompt) + .map( + |command| crate::agent::completion_policy::RequiredValidationFollowOn { + developer_message: + crate::agent::completion_policy::required_validation_phase_message( + command, + ), + command: command.to_string(), + inferred_validator: None, + }, + ); + let inferred_validator = crate::agent::validators::infer_validator_command_from_executions( + observed_tool_executions, + ); + if prompt_required_validation.is_none() + && inferred_validator.is_some() + && !self.validation_shell_available() + { + self.emit_event( + &run_id, + step, + EventKind::Error, + serde_json::json!({ + "source": "runtime_inferred_validator", + "non_fatal": true, + "reason": "inferred_validator_shell_unavailable", + "message": "inferred validator was not run because shell execution is unavailable or disabled" + }), + ); + } + let inferred_required_validation = || { + if !self.validation_shell_available() { + return None; + } + inferred_validator.map(|validator| { + crate::agent::completion_policy::RequiredValidationFollowOn { + developer_message: + crate::agent::completion_policy::required_validation_phase_message( + &validator.command, + ), + command: validator.command.clone(), + inferred_validator: Some(validator), + } + }) + }; match crate::agent::completion_policy::decide_verified_write_completion( - self.required_validation_command(user_prompt), + prompt_required_validation.or_else(inferred_required_validation), self.exact_final_answer_required(user_prompt), &verified_paths, has_post_tool_assistant_closeout, @@ -158,7 +204,11 @@ impl Agent

{ ); return VerifiedWriteResult::StartFinalAnswerPhase(message); } - crate::agent::completion_policy::VerifiedWriteCompletionDecision::StartRequiredValidationPhase(message) => { + crate::agent::completion_policy::VerifiedWriteCompletionDecision::StartRequiredValidationPhase { + developer_message, + command, + inferred_validator, + } => { self.emit_event( &run_id, step, @@ -168,7 +218,13 @@ impl Agent

{ "source": "runtime_post_write_follow_on" }), ); - return VerifiedWriteResult::StartRequiredValidationPhase(message); + return VerifiedWriteResult::StartRequiredValidationPhase( + crate::agent::completion_policy::RequiredValidationFollowOn { + developer_message, + command, + inferred_validator, + }, + ); } crate::agent::completion_policy::VerifiedWriteCompletionDecision::FinalizeNow => {} } diff --git a/src/agent/runtime_completion.rs b/src/agent/runtime_completion.rs index c3f4249..093860b 100644 --- a/src/agent/runtime_completion.rs +++ b/src/agent/runtime_completion.rs @@ -824,5 +824,5 @@ pub(super) enum VerifiedWriteResult { Done(Box), GuardRetry(String), StartFinalAnswerPhase(String), - StartRequiredValidationPhase(String), + StartRequiredValidationPhase(crate::agent::completion_policy::RequiredValidationFollowOn), } diff --git a/src/agent/runtime_effects.rs b/src/agent/runtime_effects.rs index e2372b6..bd4c1ac 100644 --- a/src/agent/runtime_effects.rs +++ b/src/agent/runtime_effects.rs @@ -111,6 +111,7 @@ pub(crate) fn completion_blocked_effect_from_post_tool_refresh( PostToolPhaseRefreshEffect::EmitCompletionBlocked { transition, reason } => { Some(CompletionBlockedEffect { transition, reason }) } + PostToolPhaseRefreshEffect::StopAfterRepairBudgetExhausted { .. } => None, } } diff --git a/src/agent/validators.rs b/src/agent/validators.rs new file mode 100644 index 0000000..adaf418 --- /dev/null +++ b/src/agent/validators.rs @@ -0,0 +1,144 @@ +use serde::{Deserialize, Serialize}; + +use crate::agent_impl_guard::ToolExecutionRecord; + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub(crate) enum ValidatorKind { + Rust, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub(crate) struct ValidatorCommand { + pub(crate) kind: ValidatorKind, + pub(crate) label: String, + pub(crate) command: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub(crate) struct ValidatorTrigger { + pub(crate) changed_paths: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub(crate) enum ValidatorOutcome { + Passed, + Failed, + Blocked, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub(crate) struct RepairBudget { + pub(crate) used: u32, + pub(crate) max: u32, +} + +impl RepairBudget { + pub(crate) fn one_repair(used: u32) -> Self { + Self { used, max: 1 } + } + + pub(crate) fn can_authorize_repair(&self) -> bool { + self.used < self.max + } +} + +pub(crate) fn infer_validator_command_from_executions( + executions: &[ToolExecutionRecord], +) -> Option { + let changed_paths = executions + .iter() + .filter(|execution| { + execution.ok + && matches!( + execution.name.as_str(), + "apply_patch" | "edit" | "write_file" | "str_replace" + ) + && execution.changed.unwrap_or(true) + }) + .filter_map(|execution| execution.path.clone()) + .collect::>(); + infer_validator_command(&ValidatorTrigger { changed_paths }) +} + +pub(crate) fn infer_validator_command(trigger: &ValidatorTrigger) -> Option { + if trigger + .changed_paths + .iter() + .any(|path| path_triggers_rust_validator(path)) + { + return Some(rust_validator_command()); + } + None +} + +fn rust_validator_command() -> ValidatorCommand { + ValidatorCommand { + kind: ValidatorKind::Rust, + label: "rust_default".to_string(), + command: "cargo fmt --check && cargo test && cargo clippy -- -D warnings".to_string(), + } +} + +fn path_triggers_rust_validator(path: &str) -> bool { + let normalized = path.replace('\\', "/").to_ascii_lowercase(); + normalized.ends_with(".rs") || normalized == "cargo.toml" || normalized.ends_with("/cargo.toml") +} + +#[cfg(test)] +mod tests { + use super::{ + infer_validator_command, infer_validator_command_from_executions, RepairBudget, + ValidatorKind, ValidatorTrigger, + }; + use crate::agent_impl_guard::ToolExecutionRecord; + + #[test] + fn rust_source_write_infers_rust_validator_suite() { + let command = infer_validator_command(&ValidatorTrigger { + changed_paths: vec!["src/lib.rs".into()], + }) + .expect("rust validator"); + assert_eq!(command.kind, ValidatorKind::Rust); + assert!(command.command.contains("cargo fmt --check")); + assert!(command.command.contains("cargo test")); + assert!(command.command.contains("cargo clippy -- -D warnings")); + } + + #[test] + fn cargo_toml_write_infers_rust_validator_suite() { + assert!(infer_validator_command(&ValidatorTrigger { + changed_paths: vec!["Cargo.toml".into()] + }) + .is_some()); + } + + #[test] + fn read_only_execution_does_not_infer_validator() { + let executions = vec![ToolExecutionRecord { + name: "read_file".to_string(), + path: Some("src/lib.rs".to_string()), + ok: true, + changed: None, + }]; + assert!(infer_validator_command_from_executions(&executions).is_none()); + } + + #[test] + fn unchanged_write_does_not_infer_validator() { + let executions = vec![ToolExecutionRecord { + name: "edit".to_string(), + path: Some("src/lib.rs".to_string()), + ok: true, + changed: Some(false), + }]; + assert!(infer_validator_command_from_executions(&executions).is_none()); + } + + #[test] + fn repair_budget_allows_one_repair_only() { + assert!(RepairBudget::one_repair(0).can_authorize_repair()); + assert!(!RepairBudget::one_repair(1).can_authorize_repair()); + } +} diff --git a/src/agent_runtime/checkpoint.rs b/src/agent_runtime/checkpoint.rs index b97a0d6..1e8841c 100644 --- a/src/agent_runtime/checkpoint.rs +++ b/src/agent_runtime/checkpoint.rs @@ -298,6 +298,7 @@ pub(super) fn runtime_state_checkpoint_for_outcome( tool_protocol_state: crate::agent_runtime::state::ToolProtocolState::default(), validation_state: ValidationState { required_command: required_command.clone(), + inferred_validator: None, satisfied: crate::agent::required_validation_command_satisfied_from_facts( required_command.as_deref(), &tool_fact_envelopes @@ -1006,6 +1007,7 @@ mod tests { tool_protocol_state: ToolProtocolState::default(), validation_state: ValidationState { required_command: Some("cargo test".to_string()), + inferred_validator: None, satisfied: true, repair_mode: false, exact_final_answer_required: false, @@ -1032,6 +1034,7 @@ mod tests { tool_protocol_state: ToolProtocolState::default(), validation_state: ValidationState { required_command: Some("cargo test".to_string()), + inferred_validator: None, satisfied: false, repair_mode: false, exact_final_answer_required: false, diff --git a/src/agent_runtime/state.rs b/src/agent_runtime/state.rs index 32fca85..8f004a8 100644 --- a/src/agent_runtime/state.rs +++ b/src/agent_runtime/state.rs @@ -50,6 +50,8 @@ pub struct RetryState { pub blocked_validation_failure_repair_count: u32, #[serde(default)] pub blocked_post_validation_final_answer_count: u32, + #[serde(default)] + pub validation_repair_turn_count: u32, } #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] @@ -68,6 +70,8 @@ pub struct ToolProtocolState { pub struct ValidationState { #[serde(skip_serializing_if = "Option::is_none")] pub required_command: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) inferred_validator: Option, #[serde(default)] pub satisfied: bool, #[serde(default)] diff --git a/src/agent_tests.rs b/src/agent_tests.rs index a5c3091..4f953ae 100644 --- a/src/agent_tests.rs +++ b/src/agent_tests.rs @@ -1244,6 +1244,185 @@ impl crate::events::EventSink for EventCaptureSink { } } +struct DenyShellGate; + +impl crate::gate::ToolGate for DenyShellGate { + fn decide( + &mut self, + _ctx: &GateContext, + call: &crate::types::ToolCall, + ) -> crate::gate::GateDecision { + if call.name == "shell" { + return crate::gate::GateDecision::Deny { + reason: "validator shell denied by test policy".to_string(), + approval_key: None, + source: Some("test_policy".to_string()), + taint_enforced: false, + escalated: false, + escalation_reason: None, + }; + } + crate::gate::GateDecision::Allow { + approval_id: None, + approval_key: None, + reason: None, + source: None, + taint_enforced: false, + escalated: false, + escalation_reason: None, + } + } + + fn record(&mut self, _event: crate::gate::GateEvent) {} +} + +fn rust_validation_tool_defs(include_shell: bool) -> Vec { + let mut tools = vec![ + crate::types::ToolDef { + name: "read_file".to_string(), + description: "d".to_string(), + parameters: serde_json::json!({ + "type":"object", + "properties":{"path":{"type":"string"}}, + "required":["path"] + }), + side_effects: crate::types::SideEffects::FilesystemRead, + }, + crate::types::ToolDef { + name: "apply_patch".to_string(), + description: "d".to_string(), + parameters: serde_json::json!({ + "type":"object", + "properties":{"path":{"type":"string"},"patch":{"type":"string"}}, + "required":["path","patch"] + }), + side_effects: crate::types::SideEffects::FilesystemWrite, + }, + crate::types::ToolDef { + name: "edit".to_string(), + description: "d".to_string(), + parameters: serde_json::json!({ + "type":"object", + "properties":{ + "path":{"type":"string"}, + "old_string":{"type":"string"}, + "new_string":{"type":"string"} + }, + "required":["path","old_string","new_string"] + }), + side_effects: crate::types::SideEffects::FilesystemWrite, + }, + ]; + if include_shell { + tools.push(crate::types::ToolDef { + name: "shell".to_string(), + description: "d".to_string(), + parameters: serde_json::json!({ + "type":"object", + "properties":{"command":{"type":"string"}}, + "required":["command"] + }), + side_effects: crate::types::SideEffects::ShellExec, + }); + } + tools +} + +fn coding_agent( + provider: P, + workdir: std::path::PathBuf, + exec_target: Arc, + allow_shell: bool, + include_shell_tool: bool, + events: Arc>>, + max_steps: usize, +) -> Agent

{ + Agent { + provider, + model: "m".to_string(), + temperature: None, + top_p: None, + max_tokens: None, + seed: None, + tools: rust_validation_tool_defs(include_shell_tool), + max_steps, + tool_rt: ToolRuntime { + workdir: workdir.clone(), + allow_shell, + allow_shell_in_workdir_only: false, + allow_write: true, + max_tool_output_bytes: 200_000, + max_read_bytes: 200_000, + unsafe_bypass_allow_flags: false, + tool_args_strict: ToolArgsStrict::On, + lsp: None, + exec_target_kind: ExecTargetKind::Host, + exec_target, + }, + gate: Box::new(NoGate::new()), + gate_ctx: GateContext { + workdir, + allow_shell, + allow_write: true, + approval_mode: ApprovalMode::Interrupt, + auto_approve_scope: AutoApproveScope::Run, + unsafe_mode: false, + unsafe_bypass_allow_flags: false, + run_id: None, + enable_write_tools: true, + max_tool_output_bytes: 200_000, + max_read_bytes: 200_000, + provider: ProviderKind::Ollama, + model: "m".to_string(), + exec_target: ExecTargetKind::Host, + approval_key_version: crate::gate::ApprovalKeyVersion::V1, + tool_schema_hashes: std::collections::BTreeMap::new(), + hooks_config_hash_hex: None, + planner_hash_hex: None, + taint_enabled: false, + taint_mode: crate::taint::TaintMode::Propagate, + taint_overall: crate::taint::TaintLevel::Clean, + taint_sources: Vec::new(), + }, + validation_requirement: None, + final_answer_mode: None, + mcp_registry: None, + stream: false, + event_sink: Some(Box::new(EventCaptureSink { events })), + compaction_settings: CompactionSettings { + max_context_chars: 0, + mode: CompactionMode::Off, + keep_last: 20, + tool_result_persist: ToolResultPersist::Digest, + }, + hooks: HookManager::build(HookRuntimeConfig { + mode: HooksMode::Off, + config_path: std::env::temp_dir().join("unused_hooks.yaml"), + strict: false, + timeout_ms: 1000, + max_stdout_bytes: 200_000, + }) + .expect("hooks"), + policy_loaded: None, + policy_for_taint: None, + taint_toggle: crate::taint::TaintToggle::Off, + taint_mode: crate::taint::TaintMode::Propagate, + taint_digest_bytes: 4096, + run_id_override: None, + omit_tools_field_when_empty: false, + plan_tool_enforcement: PlanToolEnforcementMode::Off, + mcp_pin_enforcement: McpPinEnforcementMode::Hard, + raw_tool_result_dir: None, + plan_step_constraints: Vec::new(), + current_plan: Vec::new(), + tool_call_budget: ToolCallBudget::default(), + mcp_runtime_trace: Vec::new(), + operator_queue: PendingMessageQueue::default(), + operator_queue_limits: QueueLimits::default(), + operator_queue_rx: None, + } +} + struct ToolCallProvider { calls: Arc, } @@ -1489,6 +1668,10 @@ struct ReadPatchThenExactThenBadShellRetryThenFixThenShellThenExactProvider { exact_answer: &'static str, } +struct ReadPatchThenDoneThenEditThenDoneProvider { + calls: Arc, +} + struct ReadThenRepeatedBadApplyPatchThenEditThenReadThenDoneProvider { calls: Arc, } @@ -2215,6 +2398,78 @@ impl ModelProvider for ReadPatchThenShellThenExactProvider { } } +#[async_trait] +impl ModelProvider for ReadPatchThenDoneThenEditThenDoneProvider { + async fn generate(&self, _req: GenerateRequest) -> anyhow::Result { + let n = self.calls.fetch_add(1, Ordering::SeqCst); + match n { + 0 => Ok(GenerateResponse { + assistant: Message { + role: Role::Assistant, + content: Some(String::new()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }, + tool_calls: vec![crate::types::ToolCall { + id: "tc_read".to_string(), + name: "read_file".to_string(), + arguments: serde_json::json!({"path":"main.rs"}), + }], + usage: None, + }), + 1 => Ok(GenerateResponse { + assistant: Message { + role: Role::Assistant, + content: Some(String::new()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }, + tool_calls: vec![crate::types::ToolCall { + id: "tc_patch".to_string(), + name: "apply_patch".to_string(), + arguments: serde_json::json!({ + "path":"main.rs", + "patch":"@@ -1,3 +1,3 @@\n fn answer() -> i32 {\n- return 1;\n+ return 2;\n }\n" + }), + }], + usage: None, + }), + 3 => Ok(GenerateResponse { + assistant: Message { + role: Role::Assistant, + content: Some(String::new()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }, + tool_calls: vec![crate::types::ToolCall { + id: "tc_repair".to_string(), + name: "edit".to_string(), + arguments: serde_json::json!({ + "path":"main.rs", + "old_string":"return 2;", + "new_string":"return 3;" + }), + }], + usage: None, + }), + _ => Ok(GenerateResponse { + assistant: Message { + role: Role::Assistant, + content: Some("done".to_string()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }, + tool_calls: Vec::new(), + usage: None, + }), + } + } +} + #[async_trait] impl ModelProvider for ReadThenRepeatedBadStrReplaceThenPatchThenReadThenDoneProvider { async fn generate(&self, _req: GenerateRequest) -> anyhow::Result { @@ -2957,6 +3212,11 @@ struct FailThenSucceedShellExecTarget { shell_calls: Arc, } +#[derive(Clone, Default)] +struct AlwaysFailShellExecTarget { + host: HostTarget, +} + #[async_trait] impl ExecTarget for ShellSuccessExecTarget { fn kind(&self) -> ExecTargetKind { @@ -3054,6 +3314,47 @@ impl ExecTarget for FailThenSucceedShellExecTarget { } } +#[async_trait] +impl ExecTarget for AlwaysFailShellExecTarget { + fn kind(&self) -> ExecTargetKind { + ExecTargetKind::Host + } + + fn describe(&self) -> TargetDescribe { + self.host.describe() + } + + async fn exec_shell(&self, _req: ShellReq) -> TargetResult { + TargetResult { + ok: false, + content: "tests failed\nerror: inferred validator failed".to_string(), + truncated: false, + bytes: None, + exit_code: Some(1), + stderr_truncated: None, + stdout_truncated: None, + execution_target: ExecTargetKind::Host, + docker: None, + } + } + + async fn read_file(&self, req: ReadReq) -> TargetResult { + self.host.read_file(req).await + } + + async fn list_dir(&self, req: ListReq) -> TargetResult { + self.host.list_dir(req).await + } + + async fn write_file(&self, req: WriteReq) -> TargetResult { + self.host.write_file(req).await + } + + async fn apply_patch(&self, req: PatchReq) -> TargetResult { + self.host.apply_patch(req).await + } +} + #[async_trait] impl ModelProvider for StaticContentProvider { async fn generate(&self, _req: GenerateRequest) -> anyhow::Result { @@ -5244,6 +5545,298 @@ async fn runtime_post_write_missing_closeout_gets_one_bounded_final_answer_turn( ); } +#[tokio::test] +async fn inferred_rust_validator_runs_after_rust_write_and_success_stops() { + let tmp = tempfile::tempdir().expect("tmp"); + tokio::fs::write( + tmp.path().join("main.rs"), + "fn answer() -> i32 {\n return 1;\n}\n", + ) + .await + .expect("seed"); + let calls = Arc::new(AtomicUsize::new(0)); + let events = Arc::new(Mutex::new(Vec::::new())); + let mut agent = coding_agent( + ReadPatchThenDoneProvider { + calls: calls.clone(), + }, + tmp.path().to_path_buf(), + Arc::new(ShellSuccessExecTarget::default()), + true, + true, + events.clone(), + 6, + ); + let out = agent + .run( + "Edit main.rs to return 2.", + vec![], + vec![Message { + role: Role::System, + content: Some(crate::agent::INTERNAL_ENFORCE_IMPLEMENTATION_GUARD_FLAG.to_string()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }], + ) + .await; + assert!(matches!(out.exit_reason, AgentExitReason::Ok), "{out:?}"); + assert_eq!( + out.final_output, + "Validation passed: `cargo fmt --check && cargo test && cargo clippy -- -D warnings`." + ); + assert_eq!( + calls.load(Ordering::SeqCst), + 3, + "validation success should stop without a final-answer model turn" + ); + let evs = events.lock().expect("lock"); + assert!(evs.iter().any(|e| { + matches!(e.kind, crate::events::EventKind::StepBlocked) + && e.data + .get("reason") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "post_write_validation_only_phase") + })); + assert!(evs.iter().any(|e| { + matches!(e.kind, crate::events::EventKind::ToolDecision) + && e.data + .get("name") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "shell") + && e.data + .get("decision") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "allow") + })); +} + +#[tokio::test] +async fn inferred_rust_validator_reports_shell_unavailable_without_bypass() { + let tmp = tempfile::tempdir().expect("tmp"); + tokio::fs::write( + tmp.path().join("main.rs"), + "fn answer() -> i32 {\n return 1;\n}\n", + ) + .await + .expect("seed"); + let calls = Arc::new(AtomicUsize::new(0)); + let events = Arc::new(Mutex::new(Vec::::new())); + let mut agent = coding_agent( + ReadPatchThenDoneProvider { + calls: calls.clone(), + }, + tmp.path().to_path_buf(), + Arc::new(HostTarget), + false, + false, + events.clone(), + 6, + ); + let out = agent + .run( + "Edit main.rs to return 2.", + vec![], + vec![Message { + role: Role::System, + content: Some(crate::agent::INTERNAL_ENFORCE_IMPLEMENTATION_GUARD_FLAG.to_string()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }], + ) + .await; + assert!(matches!(out.exit_reason, AgentExitReason::Ok), "{out:?}"); + assert!(out.error.is_none(), "{out:?}"); + let evs = events.lock().expect("lock"); + assert!(!evs.iter().any(|e| { + matches!(e.kind, crate::events::EventKind::ToolExecStart) + && e.data + .get("name") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "shell") + })); + assert!(evs.iter().any(|e| { + matches!(e.kind, crate::events::EventKind::Error) + && e.data + .get("reason") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "inferred_validator_shell_unavailable") + && e.data + .get("non_fatal") + .and_then(|v| v.as_bool()) + .unwrap_or(false) + })); +} + +#[tokio::test] +async fn inferred_rust_validator_permission_denial_uses_gate_path() { + let tmp = tempfile::tempdir().expect("tmp"); + tokio::fs::write( + tmp.path().join("main.rs"), + "fn answer() -> i32 {\n return 1;\n}\n", + ) + .await + .expect("seed"); + let calls = Arc::new(AtomicUsize::new(0)); + let events = Arc::new(Mutex::new(Vec::::new())); + let mut agent = coding_agent( + ReadPatchThenDoneProvider { + calls: calls.clone(), + }, + tmp.path().to_path_buf(), + Arc::new(ShellSuccessExecTarget::default()), + true, + true, + events.clone(), + 6, + ); + agent.gate = Box::new(DenyShellGate); + let out = agent + .run( + "Edit main.rs to return 2.", + vec![], + vec![Message { + role: Role::System, + content: Some(crate::agent::INTERNAL_ENFORCE_IMPLEMENTATION_GUARD_FLAG.to_string()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }], + ) + .await; + assert!( + matches!(out.exit_reason, AgentExitReason::Denied), + "{out:?}" + ); + assert!(out + .final_output + .contains("validator shell denied by test policy")); + let evs = events.lock().expect("lock"); + assert!(evs.iter().any(|e| { + matches!(e.kind, crate::events::EventKind::ToolDecision) + && e.data + .get("name") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "shell") + && e.data + .get("decision") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "deny") + && e.data + .get("source") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "test_policy") + })); +} + +#[tokio::test] +async fn inferred_rust_validator_failure_allows_one_repair_then_passes() { + let tmp = tempfile::tempdir().expect("tmp"); + tokio::fs::write( + tmp.path().join("main.rs"), + "fn answer() -> i32 {\n return 1;\n}\n", + ) + .await + .expect("seed"); + let calls = Arc::new(AtomicUsize::new(0)); + let events = Arc::new(Mutex::new(Vec::::new())); + let mut agent = coding_agent( + ReadPatchThenDoneThenEditThenDoneProvider { + calls: calls.clone(), + }, + tmp.path().to_path_buf(), + Arc::new(FailThenSucceedShellExecTarget::default()), + true, + true, + events.clone(), + 8, + ); + let out = agent + .run( + "Edit main.rs to return 2.", + vec![], + vec![Message { + role: Role::System, + content: Some(crate::agent::INTERNAL_ENFORCE_IMPLEMENTATION_GUARD_FLAG.to_string()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }], + ) + .await; + assert!(matches!(out.exit_reason, AgentExitReason::Ok), "{out:?}"); + let main = tokio::fs::read_to_string(tmp.path().join("main.rs")) + .await + .expect("read main"); + assert!(main.contains("return 3;"), "{main}"); + let evs = events.lock().expect("lock"); + assert!(evs.iter().any(|e| { + matches!(e.kind, crate::events::EventKind::CompletionBlocked) + && e.data + .get("reason") + .and_then(|v| v.as_str()) + .is_some_and(|s| s.contains("validation failed")) + })); +} + +#[tokio::test] +async fn inferred_rust_validator_second_failure_stops_with_evidence() { + let tmp = tempfile::tempdir().expect("tmp"); + tokio::fs::write( + tmp.path().join("main.rs"), + "fn answer() -> i32 {\n return 1;\n}\n", + ) + .await + .expect("seed"); + let calls = Arc::new(AtomicUsize::new(0)); + let events = Arc::new(Mutex::new(Vec::::new())); + let mut agent = coding_agent( + ReadPatchThenDoneThenEditThenDoneProvider { + calls: calls.clone(), + }, + tmp.path().to_path_buf(), + Arc::new(AlwaysFailShellExecTarget::default()), + true, + true, + events.clone(), + 8, + ); + let out = agent + .run( + "Edit main.rs to return 2.", + vec![], + vec![Message { + role: Role::System, + content: Some(crate::agent::INTERNAL_ENFORCE_IMPLEMENTATION_GUARD_FLAG.to_string()), + tool_call_id: None, + tool_name: None, + tool_calls: None, + }], + ) + .await; + assert!(matches!(out.exit_reason, AgentExitReason::PlannerError)); + assert!(out + .error + .as_deref() + .unwrap_or_default() + .contains("single allowed repair turn")); + assert!(out.messages.iter().any(|m| { + matches!(m.role, Role::Tool) + && m.content + .as_deref() + .is_some_and(|content| content.contains("openagent.observation.v1")) + })); + let evs = events.lock().expect("lock"); + assert!(evs.iter().any(|e| { + matches!(e.kind, crate::events::EventKind::Error) + && e.data + .get("failure_class") + .and_then(|v| v.as_str()) + .is_some_and(|s| s == "E_RUNTIME_VALIDATION_REPAIR_EXHAUSTED") + })); +} + #[tokio::test] async fn runtime_pre_tool_plan_text_does_not_count_as_post_tool_closeout() { let tmp = tempfile::tempdir().expect("tmp"); diff --git a/src/cli_dispatch_eval_replay.rs b/src/cli_dispatch_eval_replay.rs index cbdb823..9421b5d 100644 --- a/src/cli_dispatch_eval_replay.rs +++ b/src/cli_dispatch_eval_replay.rs @@ -997,6 +997,7 @@ mod tests { checkpoint.runtime_state_checkpoint.validation_state = crate::agent_runtime::state::ValidationState { required_command: Some("cargo test".to_string()), + inferred_validator: None, satisfied: false, repair_mode: false, exact_final_answer_required: false,