mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-31 00:29:24 +00:00
merge: Resolve conflicts with main, add user-scoped DB methods
Merge main's security hardening (user-scoped job/conversation access, cargo-dist config, CI improvements) into turso branch. Add Database trait methods for user-scoped operations: - list_sandbox_jobs_for_user - sandbox_job_summary_for_user - sandbox_job_belongs_to_user - conversation_belongs_to_user Implemented in both postgres and libsql backends. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This commit is contained in:
+37
-3
@@ -1082,9 +1082,16 @@ impl Agent {
|
||||
m
|
||||
});
|
||||
|
||||
let result = reasoning.respond_with_tools(&context).await?;
|
||||
let output = reasoning.respond_with_tools(&context).await?;
|
||||
|
||||
match result {
|
||||
// Track token usage for budget enforcement
|
||||
tracing::debug!(
|
||||
"LLM call used {} input + {} output tokens",
|
||||
output.usage.input_tokens,
|
||||
output.usage.output_tokens
|
||||
);
|
||||
|
||||
match output.result {
|
||||
RespondResult::Text(text) => {
|
||||
// If no tools have been executed yet, prompt the LLM to use tools
|
||||
// This handles the case where the model explains what it will do
|
||||
@@ -1148,11 +1155,38 @@ impl Agent {
|
||||
if let Some(tool) = self.tools().get(&tc.name).await {
|
||||
if tool.requires_approval() {
|
||||
// Check if auto-approved for this session
|
||||
let is_auto_approved = {
|
||||
let mut is_auto_approved = {
|
||||
let sess = session.lock().await;
|
||||
sess.is_tool_auto_approved(&tc.name)
|
||||
};
|
||||
|
||||
// For shell commands, override auto-approval for
|
||||
// destructive patterns that should always require
|
||||
// explicit per-invocation approval.
|
||||
if is_auto_approved && tc.name == "shell" {
|
||||
if let Some(cmd) = tc
|
||||
.arguments
|
||||
.as_str()
|
||||
.and_then(|s| {
|
||||
serde_json::from_str::<serde_json::Value>(s).ok()
|
||||
})
|
||||
.and_then(|v| {
|
||||
v.get("command")
|
||||
.and_then(|c| c.as_str().map(String::from))
|
||||
})
|
||||
{
|
||||
if crate::tools::builtin::shell::requires_explicit_approval(
|
||||
&cmd,
|
||||
) {
|
||||
tracing::info!(
|
||||
"Shell command '{}' requires explicit approval despite auto-approve",
|
||||
cmd.chars().take(80).collect::<String>()
|
||||
);
|
||||
is_auto_approved = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if !is_auto_approved {
|
||||
// Need approval - store pending request and return
|
||||
let pending = PendingApproval {
|
||||
|
||||
+11
-15
@@ -11,6 +11,7 @@
|
||||
//! Full-job routines are delegated to the existing `Scheduler`.
|
||||
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::time::Duration;
|
||||
|
||||
use chrono::Utc;
|
||||
@@ -36,7 +37,7 @@ pub struct RoutineEngine {
|
||||
/// Sender for notifications (routed to channel manager).
|
||||
notify_tx: mpsc::Sender<OutgoingResponse>,
|
||||
/// Currently running routine count (across all routines).
|
||||
running_count: Arc<RwLock<usize>>,
|
||||
running_count: Arc<AtomicUsize>,
|
||||
/// Compiled event regex cache: routine_id -> compiled regex.
|
||||
event_cache: Arc<RwLock<Vec<(Uuid, Routine, Regex)>>>,
|
||||
}
|
||||
@@ -55,7 +56,7 @@ impl RoutineEngine {
|
||||
llm,
|
||||
workspace,
|
||||
notify_tx,
|
||||
running_count: Arc::new(RwLock::new(0)),
|
||||
running_count: Arc::new(AtomicUsize::new(0)),
|
||||
event_cache: Arc::new(RwLock::new(Vec::new())),
|
||||
}
|
||||
}
|
||||
@@ -126,7 +127,7 @@ impl RoutineEngine {
|
||||
}
|
||||
|
||||
// Global capacity check
|
||||
if *self.running_count.read().await >= self.config.max_concurrent_routines {
|
||||
if self.running_count.load(Ordering::Relaxed) >= self.config.max_concurrent_routines {
|
||||
tracing::warn!(routine = %routine.name, "Skipped: global max concurrent reached");
|
||||
continue;
|
||||
}
|
||||
@@ -150,7 +151,7 @@ impl RoutineEngine {
|
||||
};
|
||||
|
||||
for routine in routines {
|
||||
if *self.running_count.read().await >= self.config.max_concurrent_routines {
|
||||
if self.running_count.load(Ordering::Relaxed) >= self.config.max_concurrent_routines {
|
||||
tracing::warn!("Global max concurrent routines reached, skipping remaining");
|
||||
break;
|
||||
}
|
||||
@@ -297,17 +298,14 @@ struct EngineContext {
|
||||
llm: Arc<dyn LlmProvider>,
|
||||
workspace: Arc<Workspace>,
|
||||
notify_tx: mpsc::Sender<OutgoingResponse>,
|
||||
running_count: Arc<RwLock<usize>>,
|
||||
running_count: Arc<AtomicUsize>,
|
||||
max_lightweight_tokens: u32,
|
||||
}
|
||||
|
||||
/// Execute a routine run. Handles both lightweight and full_job modes.
|
||||
async fn execute_routine(ctx: EngineContext, routine: Routine, run: RoutineRun) {
|
||||
// Increment running count
|
||||
{
|
||||
let mut count = ctx.running_count.write().await;
|
||||
*count += 1;
|
||||
}
|
||||
// Increment running count (atomic: survives panics in the execution below)
|
||||
ctx.running_count.fetch_add(1, Ordering::Relaxed);
|
||||
|
||||
let result = match &routine.action {
|
||||
RoutineAction::Lightweight {
|
||||
@@ -327,10 +325,7 @@ async fn execute_routine(ctx: EngineContext, routine: Routine, run: RoutineRun)
|
||||
};
|
||||
|
||||
// Decrement running count
|
||||
{
|
||||
let mut count = ctx.running_count.write().await;
|
||||
*count = count.saturating_sub(1);
|
||||
}
|
||||
ctx.running_count.fetch_sub(1, Ordering::Relaxed);
|
||||
|
||||
// Process result
|
||||
let (status, summary, tokens) = match result {
|
||||
@@ -568,7 +563,8 @@ fn truncate(s: &str, max: usize) -> String {
|
||||
if s.len() <= max {
|
||||
s.to_string()
|
||||
} else {
|
||||
format!("{}...", &s[..max])
|
||||
let end = crate::util::floor_char_boundary(s, max);
|
||||
format!("{}...", &s[..end])
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+53
-53
@@ -79,63 +79,63 @@ impl Scheduler {
|
||||
|
||||
/// Schedule a job for execution.
|
||||
pub async fn schedule(&self, job_id: Uuid) -> Result<(), JobError> {
|
||||
// Check if already scheduled
|
||||
if self.jobs.read().await.contains_key(&job_id) {
|
||||
return Ok(());
|
||||
}
|
||||
// Hold write lock for the entire check-insert sequence to prevent
|
||||
// TOCTOU races where two concurrent calls both pass the checks.
|
||||
{
|
||||
let mut jobs = self.jobs.write().await;
|
||||
|
||||
// Check capacity
|
||||
let current_count = self.jobs.read().await.len();
|
||||
if current_count >= self.config.max_parallel_jobs {
|
||||
return Err(JobError::MaxJobsExceeded {
|
||||
max: self.config.max_parallel_jobs,
|
||||
});
|
||||
}
|
||||
|
||||
// Transition job to in_progress
|
||||
self.context_manager
|
||||
.update_context(job_id, |ctx| {
|
||||
ctx.transition_to(
|
||||
JobState::InProgress,
|
||||
Some("Scheduled for execution".to_string()),
|
||||
)
|
||||
})
|
||||
.await?
|
||||
.map_err(|s| JobError::ContextError {
|
||||
id: job_id,
|
||||
reason: s,
|
||||
})?;
|
||||
|
||||
// Create worker channel
|
||||
let (tx, rx) = mpsc::channel(16);
|
||||
|
||||
// Create worker with shared dependencies
|
||||
let deps = WorkerDeps {
|
||||
context_manager: self.context_manager.clone(),
|
||||
llm: self.llm.clone(),
|
||||
safety: self.safety.clone(),
|
||||
tools: self.tools.clone(),
|
||||
store: self.store.clone(),
|
||||
timeout: self.config.job_timeout,
|
||||
use_planning: self.config.use_planning,
|
||||
};
|
||||
let worker = Worker::new(job_id, deps);
|
||||
|
||||
// Spawn worker task
|
||||
let handle = tokio::spawn(async move {
|
||||
if let Err(e) = worker.run(rx).await {
|
||||
tracing::error!("Worker for job {} failed: {}", job_id, e);
|
||||
if jobs.contains_key(&job_id) {
|
||||
return Ok(());
|
||||
}
|
||||
});
|
||||
|
||||
// Start the worker
|
||||
let _ = tx.send(WorkerMessage::Start).await;
|
||||
if jobs.len() >= self.config.max_parallel_jobs {
|
||||
return Err(JobError::MaxJobsExceeded {
|
||||
max: self.config.max_parallel_jobs,
|
||||
});
|
||||
}
|
||||
|
||||
// Store the scheduled job
|
||||
self.jobs
|
||||
.write()
|
||||
.await
|
||||
.insert(job_id, ScheduledJob { handle, tx });
|
||||
// Transition job to in_progress
|
||||
self.context_manager
|
||||
.update_context(job_id, |ctx| {
|
||||
ctx.transition_to(
|
||||
JobState::InProgress,
|
||||
Some("Scheduled for execution".to_string()),
|
||||
)
|
||||
})
|
||||
.await?
|
||||
.map_err(|s| JobError::ContextError {
|
||||
id: job_id,
|
||||
reason: s,
|
||||
})?;
|
||||
|
||||
// Create worker channel
|
||||
let (tx, rx) = mpsc::channel(16);
|
||||
|
||||
// Create worker with shared dependencies
|
||||
let deps = WorkerDeps {
|
||||
context_manager: self.context_manager.clone(),
|
||||
llm: self.llm.clone(),
|
||||
safety: self.safety.clone(),
|
||||
tools: self.tools.clone(),
|
||||
store: self.store.clone(),
|
||||
timeout: self.config.job_timeout,
|
||||
use_planning: self.config.use_planning,
|
||||
};
|
||||
let worker = Worker::new(job_id, deps);
|
||||
|
||||
// Spawn worker task
|
||||
let handle = tokio::spawn(async move {
|
||||
if let Err(e) = worker.run(rx).await {
|
||||
tracing::error!("Worker for job {} failed: {}", job_id, e);
|
||||
}
|
||||
});
|
||||
|
||||
// Start the worker
|
||||
let _ = tx.send(WorkerMessage::Start).await;
|
||||
|
||||
// Insert while still holding the write lock
|
||||
jobs.insert(job_id, ScheduledJob { handle, tx });
|
||||
}
|
||||
|
||||
// Cleanup task for this job to avoid capacity leaks
|
||||
let jobs = Arc::clone(&self.jobs);
|
||||
|
||||
+76
-19
@@ -248,16 +248,15 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
|
||||
|
||||
if selections.is_empty() {
|
||||
// No tools from select_tools, ask LLM directly (may still return tool calls)
|
||||
let respond_result = reasoning.respond_with_tools(reason_ctx).await?;
|
||||
let respond_output = reasoning.respond_with_tools(reason_ctx).await?;
|
||||
|
||||
match respond_result {
|
||||
match respond_output.result {
|
||||
RespondResult::Text(response) => {
|
||||
// Check for completion keywords
|
||||
let response_lower = response.to_lowercase();
|
||||
if response_lower.contains("complete")
|
||||
|| response_lower.contains("finished")
|
||||
|| response_lower.contains("done")
|
||||
{
|
||||
// Check for explicit completion phrases. Use word-boundary
|
||||
// aware checks to avoid false positives like "incomplete",
|
||||
// "not done", or "unfinished". Only the LLM's own response
|
||||
// (not tool output) can trigger this.
|
||||
if crate::util::llm_signals_completion(&response) {
|
||||
self.mark_completed().await?;
|
||||
return Ok(());
|
||||
}
|
||||
@@ -571,12 +570,9 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
|
||||
wrapped,
|
||||
));
|
||||
|
||||
// Check if job is complete
|
||||
if output.contains("TASK_COMPLETE") || output.contains("JOB_DONE") {
|
||||
self.mark_completed().await?;
|
||||
return Ok(true);
|
||||
}
|
||||
|
||||
// Tool output never drives job completion. A malicious tool could
|
||||
// emit "TASK_COMPLETE" to force premature completion. Only the LLM's
|
||||
// own structured response (in execution_loop) can mark a job done.
|
||||
Ok(false)
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -680,11 +676,7 @@ Report when the job is complete or if you encounter issues you cannot resolve."#
|
||||
let response = reasoning.respond(reason_ctx).await?;
|
||||
reason_ctx.messages.push(ChatMessage::assistant(&response));
|
||||
|
||||
let response_lower = response.to_lowercase();
|
||||
if response_lower.contains("complete")
|
||||
|| response_lower.contains("finished")
|
||||
|| response_lower.contains("done")
|
||||
{
|
||||
if crate::util::llm_signals_completion(&response) {
|
||||
self.mark_completed().await?;
|
||||
} else {
|
||||
// Job not complete, could re-plan or fall back to direct selection
|
||||
@@ -779,3 +771,68 @@ impl From<TaskOutput> for Result<String, Error> {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::util::llm_signals_completion;
|
||||
|
||||
#[test]
|
||||
fn test_completion_positive_signals() {
|
||||
assert!(llm_signals_completion("The job is complete."));
|
||||
assert!(llm_signals_completion(
|
||||
"I have completed the task successfully."
|
||||
));
|
||||
assert!(llm_signals_completion("The task is done."));
|
||||
assert!(llm_signals_completion("The task is finished."));
|
||||
assert!(llm_signals_completion(
|
||||
"All steps are complete and verified."
|
||||
));
|
||||
assert!(llm_signals_completion(
|
||||
"I've done all the work. The work is done."
|
||||
));
|
||||
assert!(llm_signals_completion(
|
||||
"Successfully completed the migration."
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_completion_negative_signals_block_false_positives() {
|
||||
// These contain completion keywords but also negation, should NOT trigger.
|
||||
assert!(!llm_signals_completion("The task is not complete yet."));
|
||||
assert!(!llm_signals_completion("This is not done."));
|
||||
assert!(!llm_signals_completion("The work is incomplete."));
|
||||
assert!(!llm_signals_completion(
|
||||
"The migration is not yet finished."
|
||||
));
|
||||
assert!(!llm_signals_completion("The job isn't done yet."));
|
||||
assert!(!llm_signals_completion("This remains unfinished."));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_completion_does_not_match_bare_substrings() {
|
||||
// Bare words embedded in other text should NOT trigger completion.
|
||||
assert!(!llm_signals_completion(
|
||||
"I need to complete more work first."
|
||||
));
|
||||
assert!(!llm_signals_completion(
|
||||
"Let me finish the remaining steps."
|
||||
));
|
||||
assert!(!llm_signals_completion(
|
||||
"I'm done analyzing, now let me fix it."
|
||||
));
|
||||
assert!(!llm_signals_completion(
|
||||
"I completed step 1 but step 2 remains."
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_completion_tool_output_injection() {
|
||||
// A malicious tool output echoed by the LLM should not trigger
|
||||
// completion unless it forms a genuine completion phrase.
|
||||
assert!(!llm_signals_completion("TASK_COMPLETE"));
|
||||
assert!(!llm_signals_completion("JOB_DONE"));
|
||||
assert!(!llm_signals_completion(
|
||||
"The tool returned: TASK_COMPLETE signal"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user