Files
optimclaw/src/tools/execute.rs
T
88f4894a18 merge: resolve conflicts for PR #800 and #822 into staging (#881)
* fix(ci): secrets can't be used in step if conditions [skip-regression-check] (#787)

GitHub Actions step-level `if:` doesn't have access to `secrets` context.
Replace `if: secrets.X != ''` with `continue-on-error: true` and let
the Set token step handle the fallback.

Co-authored-by: Claude Sonnet 4.6 <[email protected]>

* fix(ci): clean up staging pipeline — remove hacks, skip redundant checks [skip-regression-check] (#794)

- Remove continue-on-error from staging-ci.yml app token steps (secrets are configured)
- Skip test.yml and code_style.yml on PRs targeting staging (staging-ci.yml
  already runs tests before promoting, promotion PR gets full CI on main)
- Allow ironclaw-ci[bot] in Claude Code review for bot-created promotion PRs

Co-authored-by: Claude Opus 4.6 <[email protected]>

* fix(ci): run fmt + clippy on staging PRs, skip Windows clippy [skip-regression-check] (#802)

- Remove branches:[main] filter from code_style.yml so it runs on all PRs
- Gate clippy-windows with `if: github.base_ref == 'main'` (skip on staging PRs)
- Update rollup job to allow skipped clippy-windows
- Simplify claude-review.yml to only trigger on labeled event (avoids duplicate runs)

Co-authored-by: Claude Opus 4.6 <[email protected]>

* feat: persist user_id in save_job and expose job_id on routine runs (#709)

* feat: persist worker events to DB and fix activity tab rendering

In-process Worker (used by Scheduler::dispatch_job) now persists events
via save_job_event at key execution points: plan creation, LLM
responses, tool_use, tool_result, and job completion/failure/stuck.
Event data shapes match the container worker format so the gateway
activity tab renders them correctly.

Frontend: tool_result errors now show a red X icon with danger styling
instead of a silent empty output. The result event falls back to the
error field when message is absent.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: wire RoutineEngine into gateway for direct manual trigger firing

Replace the message-channel hack in routines_trigger_handler with a
direct call to RoutineEngine::fire_manual(), ensuring FullJob routines
dispatch correctly when triggered from the web UI. Inject the engine
into GatewayState from Agent::run after construction.

Also persists user_id in save_job for both PG and libSQL backends,
removes the source='sandbox' filter so all jobs are visible, and
exposes job_id on RoutineRunInfo for the frontend job link.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: remove stale gateway_state argument from Agent::new test call sites

The gateway_state parameter was removed from Agent::new during rebase
(replaced by post-construction set_routine_engine_slot), but three test
call sites still passed the extra None argument.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR review — restore sandbox source filter, remove blank lines

- Revert removal of `source = 'sandbox'` filter in all SandboxStore
  queries (8 sites across PG and libSQL). Sandbox-specific APIs should
  stay scoped to sandbox jobs; unified job listing for the Jobs tab
  should use a separate query path.
- Remove extra blank lines in agent_loop.rs and worker.rs that caused
  formatting CI failure.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review — regenerate Cargo.lock, add user_id regression test

- Regenerate Cargo.lock from main's lockfile to eliminate dependency
  version downgrades (anyhow, syn, etc.) that were churn from rebase.
- Add regression test verifying user_id round-trips through save_job
  and get_job in the libSQL backend.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: remove trailing blank line in libsql jobs.rs

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: add Postgres-side regression test for user_id persistence in save_job

Mirrors the existing libSQL test (test_save_job_persists_user_id) for the
Postgres backend. Gated behind #[cfg(feature = "postgres")] + #[ignore]
since it requires a running PostgreSQL instance (integration tier).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>

* refactor: unify three agentic loops into single AgenticLoop engine (#654)

Replace three independent copy-pasted agentic loops (dispatcher, worker,
container runtime) with a single shared engine in `agentic_loop.rs` that
all consumers customize via the `LoopDelegate` trait.

Phase 1 — Shared engine (`src/agent/agentic_loop.rs`, 205 lines):
  - `run_agentic_loop()` owns the core LLM → tool exec → repeat cycle
  - `LoopDelegate` trait (Send + Sync, &dyn dispatch) with 6 hook points
  - Tool intent nudge logic consolidated (was duplicated in 3 files)
  - Iteration limit + force-text behavior preserved

Phase 2 — Three delegate implementations:
  - `ChatDelegate` (dispatcher.rs): 3-phase approval flow, hooks, cost
    guard, context compaction, skill attenuation, interruption
  - `JobDelegate` (worker/job.rs): planning pre-loop phase, parallel
    JoinSet exec, mark_completed/stuck/failed, SSE streaming, self-repair
  - `ContainerDelegate` (worker/container.rs): sequential tool exec,
    HTTP-proxied LLM, container-safe tools, credential injection

Phase 3 — File moves and cleanup:
  - Delete `src/agent/worker.rs` — job logic moved to `src/worker/job.rs`
  - Rename `src/worker/runtime.rs` → `src/worker/container.rs`
  - Re-export `Worker`/`WorkerDeps` from `crate::worker` in `agent/mod.rs`
  - Update `scheduler.rs` imports to new worker location

Shared helpers (`src/tools/execute.rs`):
  - `execute_tool_with_safety()` replaces 4 copies of validate → timeout
    → execute → serialize
  - `process_tool_result()` replaces 3 copies of sanitize → wrap →
    ChatMessage (also used by thread_ops.rs approval resume paths)

Net result: -2,408 lines, zero duplicated loop logic, single code path
for tool intent nudge and completion detection.

Closes #654

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review feedback from Copilot

1. scheduler.rs: Replace `unwrap_or` fallback with proper error
   propagation when parsing tool output JSON — surfaces bugs instead
   of silently changing the output type.

2. worker/job.rs: Drop MutexGuard before the cancellation `.await` in
   `check_signals()` to avoid holding a lock across an async I/O call
   (prevents `await_holding_lock` lint).

3. worker/job.rs: Restore consecutive rate-limit counter
   (MAX_CONSECUTIVE_RATE_LIMITS = 10) so sustained rate limiting marks
   the job stuck with "Persistent rate limiting" instead of silently
   burning through max_iterations.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: incorporate staging changes — token budget tracking + mark_failed

Merge staging's changes into the refactored JobDelegate:
- Add token budget tracking in call_llm (update_context/add_tokens)
- mark_stuck → mark_failed for iteration cap and rate-limit exhaustion
  (aligns with staging's #788 fix)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address zmanian's PR review — eliminate type erasure, clean up

Address all 6 review points from zmanian on PR #800:

1. Replace LoopOutcome::Custom(Box<dyn Any>) with typed
   LoopOutcome::NeedApproval(Box<PendingApproval>) — eliminates
   type erasure and downcast, resolves clippy large_enum_variant.

2. Remove dead max_tool_iterations field from ChatDelegate struct.

3. Add on_tool_intent_nudge() hook to LoopDelegate trait with
   implementations in Job and Container delegates for observability.

4. Fix SSE events in job worker to emit raw sanitized content
   instead of XML-wrapped <tool_output> tags.

5. Remove 4 duplicate completion tests from job.rs that were
   already covered by the shared util module.

6. Avoid logging full tool results — use result_size_bytes in
   debug logs (execute.rs, job.rs).

Also updates path references in CLAUDE.md, COVERAGE_PLAN.md,
and add-sse-event.md command.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(doctor): expand diagnostics from 7 to 16 health checks

* test: add unit tests for agentic_loop and execute shared modules

Add 16 tests covering the two new critical shared modules:

agentic_loop.rs (10 tests):
- Text response exits loop immediately
- Tool call → text response continuation
- LoopSignal::Stop exits before LLM call
- LoopSignal::InjectMessage adds user message to context
- Max iterations terminates with LoopOutcome::MaxIterations
- Tool intent nudge fires twice then caps
- before_llm_call early exit bypasses LLM
- truncate_for_preview: short string, long string, multibyte safety

execute.rs (6 tests):
- execute_tool_with_safety success path
- Missing tool returns ToolError::NotFound
- Tool execution failure propagates
- Per-tool timeout enforcement (50ms)
- process_tool_result XML wrapping on success
- process_tool_result error formatting

All 2,777 unit tests pass, 0 clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: cargo fmt

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address code review — 9 issues across agentic loop, job worker, container

CRITICAL fixes:
- Rate-limit exhaustion now returns Err(LlmError::RateLimited) instead of
  Ok(Text("")), stopping the loop immediately with no ghost iteration.
  Below-threshold retries still use Text("") with an explicit empty-string
  guard in handle_text_response to skip injection.
- check_signals drains the entire message channel before returning,
  prioritizing Stop over UserMessage. Previously returned early on first
  UserMessage, silently dropping any queued Stop or additional messages.
- check_signals now detects all non-progressing job states (Cancelled,
  Failed, Stuck, Completed, Submitted, Accepted) instead of only
  Cancelled and Failed.

HIGH fixes:
- Error path in process_tool_result_job applies truncate_for_preview to
  bound error strings in SSE/DB events (was unbounded).
- Document Send+Sync lifetime constraint on LoopDelegate trait.
- Test mock before_llm_call refactored from double-lock to single lock
  acquisition, eliminating deadlock risk on refactor.

MEDIUM fixes:
- CompletionReport includes actual iteration count via shared
  Arc<Mutex<u32>> tracker (was hardcoded 0).
- process_tool_result_job return type changed from Result<bool> to
  Result<()> — the bool was always false (dead API).
- Deduplicate truncate in container.rs; now uses truncate_for_preview
  from agentic_loop.

Verified: 0 clippy warnings, 2781 tests pass, cargo fmt clean.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Henry Park <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: Illia Polosukhin <[email protected]>
Co-authored-by: Umesh Kumar Singh <[email protected]>
Co-authored-by: reidliu41 <[email protected]>
2026-03-10 11:19:23 -07:00

392 lines
12 KiB
Rust

//! Shared tool execution pipeline.
//!
//! Provides a single implementation of the validate → timeout → execute → serialize
//! pipeline used by all agentic loop consumers (chat, job, container) and the
//! scheduler's subtask execution.
use crate::context::JobContext;
use crate::error::Error;
use crate::llm::ChatMessage;
use crate::safety::SafetyLayer;
use crate::tools::{ToolRegistry, redact_params};
/// Execute a tool with safety checks: lookup → validate → timeout → execute → serialize.
///
/// This is the single canonical implementation of tool execution. All consumers
/// (chat dispatcher, job worker, container runtime, scheduler subtasks) use this
/// function instead of maintaining their own copies.
pub async fn execute_tool_with_safety(
tools: &ToolRegistry,
safety: &SafetyLayer,
tool_name: &str,
params: &serde_json::Value,
job_ctx: &JobContext,
) -> Result<String, Error> {
let tool = tools
.get(tool_name)
.await
.ok_or_else(|| crate::error::ToolError::NotFound {
name: tool_name.to_string(),
})?;
// Validate tool parameters
let validation = safety.validator().validate_tool_params(params);
if !validation.is_valid {
let details = validation
.errors
.iter()
.map(|e| format!("{}: {}", e.field, e.message))
.collect::<Vec<_>>()
.join("; ");
return Err(crate::error::ToolError::InvalidParameters {
name: tool_name.to_string(),
reason: format!("Invalid tool parameters: {}", details),
}
.into());
}
let safe_params = redact_params(params, tool.sensitive_params());
tracing::debug!(
tool = %tool_name,
params = %safe_params,
"Tool call started"
);
// Execute with per-tool timeout
let timeout = tool.execution_timeout();
let start = std::time::Instant::now();
let result = tokio::time::timeout(timeout, async {
tool.execute(params.clone(), job_ctx).await
})
.await;
let elapsed = start.elapsed();
match &result {
Ok(Ok(output)) => {
let result_size = serde_json::to_string(&output.result)
.map(|s| s.len())
.unwrap_or(0);
tracing::debug!(
tool = %tool_name,
elapsed_ms = elapsed.as_millis() as u64,
result_size_bytes = result_size,
"Tool call succeeded"
);
}
Ok(Err(e)) => {
tracing::debug!(
tool = %tool_name,
elapsed_ms = elapsed.as_millis() as u64,
error = %e,
"Tool call failed"
);
}
Err(_) => {
tracing::debug!(
tool = %tool_name,
elapsed_ms = elapsed.as_millis() as u64,
timeout_secs = timeout.as_secs(),
"Tool call timed out"
);
}
}
let result = result
.map_err(|_| crate::error::ToolError::Timeout {
name: tool_name.to_string(),
timeout,
})?
.map_err(|e| crate::error::ToolError::ExecutionFailed {
name: tool_name.to_string(),
reason: e.to_string(),
})?;
serde_json::to_string_pretty(&result.result).map_err(|e| {
crate::error::ToolError::ExecutionFailed {
name: tool_name.to_string(),
reason: format!("Failed to serialize result: {}", e),
}
.into()
})
}
/// Process a tool result into a `ChatMessage::tool_result` with safety sanitization.
///
/// On success: sanitize → wrap → ChatMessage::tool_result.
/// On error: format error → ChatMessage::tool_result.
///
/// Returns the content string and the ChatMessage.
pub fn process_tool_result(
safety: &SafetyLayer,
tool_name: &str,
tool_call_id: &str,
result: &Result<String, impl std::fmt::Display>,
) -> (String, ChatMessage) {
let content = match result {
Ok(output) => {
let sanitized = safety.sanitize_tool_output(tool_name, output);
safety.wrap_for_llm(tool_name, &sanitized.content, sanitized.was_modified)
}
Err(e) => format!("Error: {}", e),
};
let message = ChatMessage::tool_result(tool_call_id, tool_name, content.clone());
(content, message)
}
/// Execute a tool with safety checks, returning a string error (for container runtime).
///
/// This is a thin wrapper around `execute_tool_with_safety` that converts
/// `Error` to `String` for the container runtime's simpler error model.
pub async fn execute_tool_simple(
tools: &ToolRegistry,
safety: &SafetyLayer,
tool_name: &str,
params: &serde_json::Value,
job_ctx: &JobContext,
) -> Result<String, String> {
execute_tool_with_safety(tools, safety, tool_name, params, job_ctx)
.await
.map_err(|e| e.to_string())
}
#[cfg(test)]
mod tests {
use super::*;
use crate::tools::tool::{Tool, ToolError, ToolOutput};
use std::sync::Arc;
use std::time::Duration;
struct EchoTool;
#[async_trait::async_trait]
impl Tool for EchoTool {
fn name(&self) -> &str {
"echo"
}
fn description(&self) -> &str {
"Echoes input"
}
fn parameters_schema(&self) -> serde_json::Value {
serde_json::json!({"type": "object", "properties": {}})
}
async fn execute(
&self,
params: serde_json::Value,
_ctx: &JobContext,
) -> Result<ToolOutput, ToolError> {
Ok(ToolOutput::success(params, Duration::default()))
}
fn requires_sanitization(&self) -> bool {
false
}
}
struct FailTool;
#[async_trait::async_trait]
impl Tool for FailTool {
fn name(&self) -> &str {
"fail_tool"
}
fn description(&self) -> &str {
"Always fails"
}
fn parameters_schema(&self) -> serde_json::Value {
serde_json::json!({"type": "object", "properties": {}})
}
async fn execute(
&self,
_: serde_json::Value,
_: &JobContext,
) -> Result<ToolOutput, ToolError> {
Err(ToolError::ExecutionFailed(
"intentional failure".to_string(),
))
}
fn requires_sanitization(&self) -> bool {
false
}
}
struct SlowTool;
#[async_trait::async_trait]
impl Tool for SlowTool {
fn name(&self) -> &str {
"slow_tool"
}
fn description(&self) -> &str {
"Sleeps forever"
}
fn parameters_schema(&self) -> serde_json::Value {
serde_json::json!({"type": "object", "properties": {}})
}
async fn execute(
&self,
_: serde_json::Value,
_: &JobContext,
) -> Result<ToolOutput, ToolError> {
tokio::time::sleep(Duration::from_secs(60)).await;
unreachable!()
}
fn execution_timeout(&self) -> Duration {
Duration::from_millis(50)
}
fn requires_sanitization(&self) -> bool {
false
}
}
fn test_safety() -> SafetyLayer {
SafetyLayer::new(&crate::config::SafetyConfig {
max_output_length: 100_000,
injection_check_enabled: false,
})
}
fn test_job_ctx() -> JobContext {
JobContext::default()
}
async fn registry_with(tools: Vec<Arc<dyn Tool>>) -> ToolRegistry {
let registry = ToolRegistry::new();
for tool in tools {
registry.register(tool).await;
}
registry
}
#[tokio::test]
async fn test_execute_success() {
let registry = registry_with(vec![Arc::new(EchoTool)]).await;
let safety = test_safety();
let params = serde_json::json!({"message": "hello"});
let result =
execute_tool_with_safety(&registry, &safety, "echo", &params, &test_job_ctx()).await;
assert!(result.is_ok(), "Echo tool should succeed");
let output = result.unwrap();
assert!(
output.contains("hello"),
"Output should contain the echoed input"
);
}
#[tokio::test]
async fn test_execute_missing_tool() {
let registry = registry_with(vec![]).await;
let safety = test_safety();
let result = execute_tool_with_safety(
&registry,
&safety,
"nonexistent",
&serde_json::json!({}),
&test_job_ctx(),
)
.await;
assert!(result.is_err(), "Missing tool should return error");
let err = result.unwrap_err().to_string();
assert!(
err.contains("nonexistent") || err.contains("not found"),
"Error should mention the tool: {}",
err
);
}
#[tokio::test]
async fn test_execute_tool_failure() {
let registry = registry_with(vec![Arc::new(FailTool)]).await;
let safety = test_safety();
let result = execute_tool_with_safety(
&registry,
&safety,
"fail_tool",
&serde_json::json!({}),
&test_job_ctx(),
)
.await;
assert!(result.is_err(), "FailTool should return error");
let err = result.unwrap_err().to_string();
assert!(
err.contains("intentional failure"),
"Error should contain the failure reason: {}",
err
);
}
#[tokio::test]
async fn test_execute_tool_timeout() {
let registry = registry_with(vec![Arc::new(SlowTool)]).await;
let safety = test_safety();
let start = std::time::Instant::now();
let result = execute_tool_with_safety(
&registry,
&safety,
"slow_tool",
&serde_json::json!({}),
&test_job_ctx(),
)
.await;
let elapsed = start.elapsed();
assert!(result.is_err(), "SlowTool should timeout");
let err = result.unwrap_err().to_string();
assert!(
err.to_lowercase().contains("timeout") || err.to_lowercase().contains("timed out"),
"Error should mention timeout: {}",
err
);
assert!(
elapsed < Duration::from_secs(1),
"Should timeout quickly, not wait 60s"
);
}
#[test]
fn test_process_tool_result_success() {
let safety = test_safety();
let result: Result<String, String> = Ok("tool output data".to_string());
let (content, message) = process_tool_result(&safety, "echo", "call_1", &result);
assert!(
content.contains("tool_output"),
"Content should be XML-wrapped: {}",
content
);
assert!(
content.contains("tool output data"),
"Content should contain the output: {}",
content
);
assert_eq!(message.role, crate::llm::Role::Tool);
assert_eq!(message.name.as_deref(), Some("echo"));
}
#[test]
fn test_process_tool_result_error() {
let safety = test_safety();
let result: Result<String, String> = Err("something went wrong".to_string());
let (content, message) = process_tool_result(&safety, "echo", "call_1", &result);
assert!(
content.contains("Error:"),
"Error content should start with 'Error:': {}",
content
);
assert!(
content.contains("something went wrong"),
"Error content should contain the message: {}",
content
);
assert_eq!(message.role, crate::llm::Role::Tool);
}
}