Files
optimclaw/src/agent/task.rs
T
3f135bdde9 fix: persist turns after approval and add agent-level tests (#250)
* fix: persist turns after approval and add agent-level tests

Port relevant changes from PR #112 that were not carried over to #237:

- Add persist_turn calls in process_approval for the response, error,
  and auth-required paths. Previously, turns completed after tool
  approval were never persisted to DB — if the process crashed after
  approval the entire turn (user message + assistant response) was lost.

- Add agent-level unit tests: StaticLlmProvider mock, make_test_agent
  helper, tests for auto-approval logic, destructive shell command
  detection, and PendingApproval backward-compatible deserialization
  (without deferred_tool_calls field).

- Remove unused _thread_state binding in process_approval.

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

* fix: address 14 audit findings in src/agent/

Audit of the agent module found 2 High, 7 Medium, 3 Low, and 2 Nit
severity issues. This commit fixes all of them:

High:
- Remove 4 `.expect()` calls in session.rs (entry API, match, direct
  indexing, if-let) to eliminate panic paths in production
- Add typed RoutineError enum replacing Result<_, String> across
  routine.rs, routine_engine.rs, and callers in history/store.rs and
  db/libsql/mod.rs

Medium:
- Sanitize routine names in path construction to prevent directory
  traversal (routine_engine.rs)
- Log warnings for 5 silently-swallowed errors in scheduler.rs,
  compaction.rs, and worker.rs
- Extract shared handle_auth_intercept helper to deduplicate auth
  interception in thread_ops.rs
- Add session count warning threshold in session_manager.rs
- Make FullJob stub degradation visible via warn-level log and
  prepended warning in output

Low:
- Restrict dead code visibility with #[cfg(test)] on 19 unused items
  in submission.rs, task.rs, and undo.rs
- Narrow pub to pub(crate) on self_repair.rs builder methods
- Remove TaskStatus from mod.rs re-exports (test-only type)

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

* fix: address PR review comments

- Reorder persist_turn before persist_response_chain so the
  conversation row exists before the metadata UPDATE runs
- Add persist_response_chain call to handle_auth_intercept so
  auth-required paths preserve the response chain
- Harden sanitize_routine_name to use allowlist (alphanumeric,
  dash, underscore) instead of denylist replacements
- Fix stale active_thread ID in get_or_create_thread: fall back
  to create_thread() when the stored ID is missing from the map
- Persist turn on approval rejection so user messages survive
  crashes after a tool is rejected

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

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-20 02:28:15 +00:00

284 lines
7.6 KiB
Rust

//! Task types for the scheduler.
//!
//! Tasks are the unit of work that can be scheduled for execution.
//! They can represent full LLM-driven jobs, parallel tool batches,
//! or background computations.
use std::fmt;
use std::time::Duration;
use async_trait::async_trait;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
use crate::error::Error;
/// Result of a task execution.
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct TaskOutput {
/// The result data.
pub result: serde_json::Value,
/// Time taken to execute.
pub duration: Duration,
}
impl TaskOutput {
/// Create a new task output.
pub fn new(result: serde_json::Value, duration: Duration) -> Self {
Self { result, duration }
}
/// Create a text result.
#[cfg(test)]
pub fn text(text: impl Into<String>, duration: Duration) -> Self {
Self {
result: serde_json::Value::String(text.into()),
duration,
}
}
/// Create an empty success result.
#[cfg(test)]
pub fn empty(duration: Duration) -> Self {
Self {
result: serde_json::Value::Null,
duration,
}
}
}
/// Context passed to task handlers.
#[derive(Debug, Clone)]
pub struct TaskContext {
/// Task ID.
pub task_id: Uuid,
/// Parent task ID (if this is a sub-task).
pub parent_id: Option<Uuid>,
/// Arbitrary metadata for the task.
pub metadata: serde_json::Value,
}
impl TaskContext {
/// Create a new task context.
pub fn new(task_id: Uuid) -> Self {
Self {
task_id,
parent_id: None,
metadata: serde_json::Value::Null,
}
}
/// Set the parent task ID.
pub fn with_parent(mut self, parent_id: Uuid) -> Self {
self.parent_id = Some(parent_id);
self
}
/// Set metadata.
pub fn with_metadata(mut self, metadata: serde_json::Value) -> Self {
self.metadata = metadata;
self
}
}
/// Handler for custom background tasks.
#[async_trait]
pub trait TaskHandler: Send + Sync {
/// Run the task and return the result.
async fn run(&self, ctx: TaskContext) -> Result<TaskOutput, Error>;
/// Get a description of this handler for logging.
fn description(&self) -> &str {
"background task"
}
}
/// A task that can be scheduled for execution.
#[derive(Clone)]
pub enum Task {
/// Full LLM-driven job (current Worker behavior).
Job {
id: Uuid,
title: String,
description: String,
},
/// Single tool execution as a sub-task.
ToolExec {
/// ID of the parent job this tool execution belongs to.
parent_id: Uuid,
/// Name of the tool to execute.
tool_name: String,
/// Parameters to pass to the tool.
params: serde_json::Value,
},
/// Background computation (no LLM, uses a custom handler).
/// Note: The handler is wrapped in Arc for cloning.
Background {
id: Uuid,
handler: std::sync::Arc<dyn TaskHandler>,
},
}
impl Task {
/// Create a new Job task.
pub fn job(title: impl Into<String>, description: impl Into<String>) -> Self {
Self::Job {
id: Uuid::new_v4(),
title: title.into(),
description: description.into(),
}
}
/// Create a new Job task with a specific ID.
#[cfg(test)]
pub fn job_with_id(id: Uuid, title: impl Into<String>, description: impl Into<String>) -> Self {
Self::Job {
id,
title: title.into(),
description: description.into(),
}
}
/// Create a new ToolExec task.
pub fn tool_exec(
parent_id: Uuid,
tool_name: impl Into<String>,
params: serde_json::Value,
) -> Self {
Self::ToolExec {
parent_id,
tool_name: tool_name.into(),
params,
}
}
/// Create a new Background task.
#[cfg(test)]
pub fn background(handler: std::sync::Arc<dyn TaskHandler>) -> Self {
Self::Background {
id: Uuid::new_v4(),
handler,
}
}
/// Create a new Background task with a specific ID.
#[cfg(test)]
pub fn background_with_id(id: Uuid, handler: std::sync::Arc<dyn TaskHandler>) -> Self {
Self::Background { id, handler }
}
/// Get the task ID, if applicable.
pub fn id(&self) -> Option<Uuid> {
match self {
Self::Job { id, .. } => Some(*id),
Self::ToolExec { .. } => None, // Tool execs don't have their own ID
Self::Background { id, .. } => Some(*id),
}
}
/// Get the parent ID for sub-tasks.
#[cfg(test)]
pub fn parent_id(&self) -> Option<Uuid> {
match self {
Self::Job { .. } => None,
Self::ToolExec { parent_id, .. } => Some(*parent_id),
Self::Background { .. } => None,
}
}
/// Get a short description for logging.
pub fn description(&self) -> String {
match self {
Self::Job { title, .. } => format!("job: {}", title),
Self::ToolExec { tool_name, .. } => format!("tool: {}", tool_name),
Self::Background { handler, .. } => format!("background: {}", handler.description()),
}
}
}
impl fmt::Debug for Task {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Job {
id,
title,
description,
} => f
.debug_struct("Task::Job")
.field("id", id)
.field("title", title)
.field("description", description)
.finish(),
Self::ToolExec {
parent_id,
tool_name,
params,
} => f
.debug_struct("Task::ToolExec")
.field("parent_id", parent_id)
.field("tool_name", tool_name)
.field("params", params)
.finish(),
Self::Background { id, handler } => f
.debug_struct("Task::Background")
.field("id", id)
.field("handler", &handler.description())
.finish(),
}
}
}
/// Status of a scheduled task.
#[cfg(test)]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum TaskStatus {
/// Task is queued waiting for execution.
Queued,
/// Task is currently running.
Running,
/// Task completed successfully.
Completed,
/// Task failed with an error.
Failed,
/// Task was cancelled.
Cancelled,
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_task_output() {
let output = TaskOutput::text("hello", Duration::from_secs(1));
assert_eq!(output.result, serde_json::json!("hello"));
assert_eq!(output.duration, Duration::from_secs(1));
}
#[test]
fn test_task_context() {
let parent = Uuid::new_v4();
let ctx = TaskContext::new(Uuid::new_v4()).with_parent(parent);
assert_eq!(ctx.parent_id, Some(parent));
}
#[test]
fn test_task_job() {
let task = Task::job("Test Job", "Test description");
assert!(task.id().is_some());
assert!(task.parent_id().is_none());
assert!(task.description().contains("job:"));
}
#[test]
fn test_task_tool_exec() {
let parent_id = Uuid::new_v4();
let task = Task::tool_exec(parent_id, "echo", serde_json::json!({"message": "hi"}));
assert!(task.id().is_none());
assert_eq!(task.parent_id(), Some(parent_id));
assert!(task.description().contains("tool:"));
}
}