mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
* 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]>
284 lines
7.6 KiB
Rust
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:"));
|
|
}
|
|
}
|