mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
feat: configurable tool iterations, auto-approve, and policy fix (#251)
* feat: direct agentic loop for SWE-bench benchmarks Replace the full Agent-based runner with a purpose-built agentic loop that directly calls the LLM with tools. The old path routed through SafetyLayer (which blocked SWE-bench prompts), dispatcher (capped at 10 iterations), approval flow (wasted iterations), and 20+ irrelevant builtin tools (diluted the model's focus). New architecture: - AgenticLoop: LLM call -> tool execution -> repeat (up to 30 iters) - Per-task tool scoping via BenchSuite::task_tools() with working dirs - Suite-provided system prompts via BenchSuite::system_prompt() - No safety layer, no approval flow, no sessions/threads overhead - Configurable max_iterations in BenchConfig and TOML Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: apply --model CLI override to LLM provider The --model flag was updating matrix entry labels but not the actual LLM provider, so requests were still sent using the model from .env. Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: configurable tool iterations and auto-approve for benchmarks Add max_tool_iterations and auto_approve_tools settings to AgentConfig, replacing the hardcoded MAX_TOOL_ITERATIONS constant. Fix shell_injection policy rule to not block markdown backtick code snippets. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: address benchmarks crate audit findings High: - Fix truncate_output UTF-8 panic on multi-byte char boundaries - Fix parallel results durability (write JSONL per-task, not after all) Medium: - Fix --sample to use random shuffle instead of first-N - Delegate all LlmProvider methods in InstrumentedLlm - Fix LLM-as-judge to return fail instead of misleading 0.5 - Remove unnecessary shallow clone (always gets unshallowed) - Replace .unwrap() with .expect() in LazyLock regex init Low: - Remove dead code: unused error variants, trait methods, struct fields - Remove BenchSuite::name() (redundant with id()) - Remove TaskSubmission::conversation, ConversationTurn, TurnRole - Remove unused methods from BenchChannel, results, config - Clean up ChannelCapture conversation tracking Co-Authored-By: Claude Opus 4.6 <[email protected]> * feat: add SWE-bench dataset and Docker scoring infrastructure Add the SWE-bench Lite dataset (300 tasks) and Docker files for isolated test execution and scoring of SWE-bench patches. Co-Authored-By: Claude Opus 4.6 <[email protected]> * chore: remove benchmarks (extracted to separate repo) Benchmarks crate has been extracted to its own repository. Remove the workspace member and all benchmarks/ files. Co-Authored-By: Claude Opus 4.6 <[email protected]> * fix: add missing AgentConfig fields in test initializer Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
1f18422b88
commit
e8dcb52fda
+13
-10
@@ -108,21 +108,21 @@ impl Agent {
|
||||
// Create a JobContext for tool execution (chat doesn't have a real job)
|
||||
let job_ctx = JobContext::with_user(&message.user_id, "chat", "Interactive chat session");
|
||||
|
||||
const MAX_TOOL_ITERATIONS: usize = 10;
|
||||
let max_tool_iterations = self.config.max_tool_iterations;
|
||||
// Force a text-only response on the last iteration to guarantee termination
|
||||
// instead of hard-erroring. The penultimate iteration also gets a nudge
|
||||
// message so the LLM knows it should wrap up.
|
||||
const FORCE_TEXT_AT: usize = MAX_TOOL_ITERATIONS;
|
||||
const NUDGE_AT: usize = MAX_TOOL_ITERATIONS - 1;
|
||||
let force_text_at = max_tool_iterations;
|
||||
let nudge_at = max_tool_iterations.saturating_sub(1);
|
||||
let mut iteration = 0;
|
||||
loop {
|
||||
iteration += 1;
|
||||
// Hard ceiling one past the forced-text iteration (should never be reached
|
||||
// since FORCE_TEXT_AT guarantees a text response, but kept as a safety net).
|
||||
if iteration > MAX_TOOL_ITERATIONS + 1 {
|
||||
// since force_text_at guarantees a text response, but kept as a safety net).
|
||||
if iteration > max_tool_iterations + 1 {
|
||||
return Err(crate::error::LlmError::InvalidResponse {
|
||||
provider: "agent".to_string(),
|
||||
reason: format!("Exceeded maximum tool iterations ({})", MAX_TOOL_ITERATIONS),
|
||||
reason: format!("Exceeded maximum tool iterations ({max_tool_iterations})"),
|
||||
}
|
||||
.into());
|
||||
}
|
||||
@@ -152,7 +152,7 @@ impl Agent {
|
||||
|
||||
// Inject a nudge message when approaching the iteration limit so the
|
||||
// LLM is aware it should produce a final answer on the next turn.
|
||||
if iteration == NUDGE_AT {
|
||||
if iteration == nudge_at {
|
||||
context_messages.push(ChatMessage::system(
|
||||
"You are approaching the tool call limit. \
|
||||
Provide your best final answer on the next response \
|
||||
@@ -161,7 +161,7 @@ impl Agent {
|
||||
));
|
||||
}
|
||||
|
||||
let force_text = iteration >= FORCE_TEXT_AT;
|
||||
let force_text = iteration >= force_text_at;
|
||||
|
||||
// Refresh tool definitions each iteration so newly built tools become visible
|
||||
let tool_defs = self.tools().tool_definitions().await;
|
||||
@@ -284,8 +284,9 @@ impl Agent {
|
||||
for (idx, original_tc) in tool_calls.iter().enumerate() {
|
||||
let mut tc = original_tc.clone();
|
||||
|
||||
// Check if tool requires approval
|
||||
if let Some(tool) = self.tools().get(&tc.name).await
|
||||
// Check if tool requires approval (skipped when auto_approve_tools is set)
|
||||
if !self.config.auto_approve_tools
|
||||
&& let Some(tool) = self.tools().get(&tc.name).await
|
||||
&& tool.requires_approval()
|
||||
{
|
||||
let mut is_auto_approved = {
|
||||
@@ -877,6 +878,8 @@ mod tests {
|
||||
allow_local_tools: false,
|
||||
max_cost_per_day_cents: None,
|
||||
max_actions_per_hour: None,
|
||||
max_tool_iterations: 50,
|
||||
auto_approve_tools: false,
|
||||
},
|
||||
deps,
|
||||
ChannelManager::new(),
|
||||
|
||||
@@ -23,6 +23,10 @@ pub struct AgentConfig {
|
||||
pub max_cost_per_day_cents: Option<u64>,
|
||||
/// Maximum LLM/tool actions per hour. None = unlimited.
|
||||
pub max_actions_per_hour: Option<u64>,
|
||||
/// Maximum tool-call iterations per agentic loop invocation. Default 50.
|
||||
pub max_tool_iterations: usize,
|
||||
/// When true, skip tool approval checks entirely. For benchmarks/CI.
|
||||
pub auto_approve_tools: bool,
|
||||
}
|
||||
|
||||
impl AgentConfig {
|
||||
@@ -115,6 +119,22 @@ impl AgentConfig {
|
||||
key: "MAX_ACTIONS_PER_HOUR".to_string(),
|
||||
message: format!("must be a positive integer: {e}"),
|
||||
})?,
|
||||
max_tool_iterations: optional_env("AGENT_MAX_TOOL_ITERATIONS")?
|
||||
.map(|s| s.parse())
|
||||
.transpose()
|
||||
.map_err(|e| ConfigError::InvalidValue {
|
||||
key: "AGENT_MAX_TOOL_ITERATIONS".to_string(),
|
||||
message: format!("must be a positive integer: {e}"),
|
||||
})?
|
||||
.unwrap_or(settings.agent.max_tool_iterations),
|
||||
auto_approve_tools: optional_env("AGENT_AUTO_APPROVE_TOOLS")?
|
||||
.map(|s| s.parse())
|
||||
.transpose()
|
||||
.map_err(|e| ConfigError::InvalidValue {
|
||||
key: "AGENT_AUTO_APPROVE_TOOLS".to_string(),
|
||||
message: format!("must be 'true' or 'false': {e}"),
|
||||
})?
|
||||
.unwrap_or(settings.agent.auto_approve_tools),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
+15
-2
@@ -159,11 +159,13 @@ impl Default for Policy {
|
||||
PolicyAction::Warn,
|
||||
));
|
||||
|
||||
// Block shell command injection patterns
|
||||
// Block shell command injection patterns.
|
||||
// Only match actual dangerous command sequences, NOT backticked content
|
||||
// (backticks are standard markdown code formatting, not shell injection).
|
||||
policy.add_rule(PolicyRule::new(
|
||||
"shell_injection",
|
||||
"Potential shell command injection",
|
||||
r"(?i)(;\s*rm\s+-rf|;\s*curl\s+.*\|\s*sh|`.*`)",
|
||||
r"(?i)(;\s*rm\s+-rf|;\s*curl\s+.*\|\s*sh)",
|
||||
Severity::Critical,
|
||||
PolicyAction::Block,
|
||||
));
|
||||
@@ -233,6 +235,17 @@ mod tests {
|
||||
assert!(violations.iter().any(|r| r.action == PolicyAction::Warn));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_backticked_code_is_not_blocked() {
|
||||
let policy = Policy::default();
|
||||
// Markdown code snippets should never be blocked
|
||||
assert!(!policy.is_blocked("Use `print('hello')` to debug"));
|
||||
assert!(!policy.is_blocked("Run `pytest tests/` to check"));
|
||||
assert!(!policy.is_blocked("The error is in `foo.bar.baz`"));
|
||||
// Multi-backtick code fences should also pass
|
||||
assert!(!policy.is_blocked("```python\ndef foo():\n pass\n```"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_severity_ordering() {
|
||||
assert!(Severity::Critical > Severity::High);
|
||||
|
||||
@@ -302,6 +302,14 @@ pub struct AgentSettings {
|
||||
/// longer than this are pruned from memory.
|
||||
#[serde(default = "default_session_idle_timeout")]
|
||||
pub session_idle_timeout_secs: u64,
|
||||
|
||||
/// Maximum tool-call iterations per agentic loop invocation (default: 50).
|
||||
#[serde(default = "default_max_tool_iterations")]
|
||||
pub max_tool_iterations: usize,
|
||||
|
||||
/// When true, skip tool approval checks entirely. For benchmarks/CI.
|
||||
#[serde(default)]
|
||||
pub auto_approve_tools: bool,
|
||||
}
|
||||
|
||||
fn default_agent_name() -> String {
|
||||
@@ -332,6 +340,10 @@ fn default_max_repair_attempts() -> u32 {
|
||||
3
|
||||
}
|
||||
|
||||
fn default_max_tool_iterations() -> usize {
|
||||
50
|
||||
}
|
||||
|
||||
fn default_true() -> bool {
|
||||
true
|
||||
}
|
||||
@@ -347,6 +359,8 @@ impl Default for AgentSettings {
|
||||
repair_check_interval_secs: default_repair_interval(),
|
||||
max_repair_attempts: default_max_repair_attempts(),
|
||||
session_idle_timeout_secs: default_session_idle_timeout(),
|
||||
max_tool_iterations: default_max_tool_iterations(),
|
||||
auto_approve_tools: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user