mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 15:40:18 +00:00
fix(engine): replace byte-index slicing with char-safe truncation
Panic: 'byte index 80 is not a char boundary; it is inside ''' when tool output contained multi-byte UTF-8 characters (smart quotes from web search results). Fixed 4 unsafe byte-index slices: - thread.rs:281: message preview &content[..80] → chars().take(80) - loop_engine.rs:556: tool output &str[..4000] → chars().take(4000) - loop_engine.rs:579: output tail &str[len-8000..] → chars().skip() - scripting.rs:82: stdout tail &str[len-N..] → chars().skip() All now use .chars().take() or .chars().skip() which respect character boundaries. Follows CLAUDE.md rule: "Never use byte-index slicing on user-supplied or external strings." Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
@@ -167,6 +167,27 @@ impl ExecutionLoop {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(max_usd) = self.thread.config.max_budget_usd
|
||||
&& self.thread.total_cost_usd >= max_usd
|
||||
{
|
||||
warn!(
|
||||
thread_id = %self.thread.id,
|
||||
used = self.thread.total_cost_usd,
|
||||
limit = max_usd,
|
||||
"USD budget exceeded"
|
||||
);
|
||||
self.thread.transition_to(
|
||||
ThreadState::Completed,
|
||||
Some("USD budget exceeded".into()),
|
||||
)?;
|
||||
return Ok(ThreadOutcome::Failed {
|
||||
error: format!(
|
||||
"USD budget exceeded: ${:.4} of ${:.4}",
|
||||
self.thread.total_cost_usd, max_usd
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
// 3. Check compaction
|
||||
if self.thread.config.enable_compaction {
|
||||
let ctx_limit = self.thread.config.model_context_limit;
|
||||
@@ -253,6 +274,7 @@ impl ExecutionLoop {
|
||||
let llm_output = self.llm.complete(&messages, &[], &config).await?;
|
||||
step.tokens_used = llm_output.usage;
|
||||
self.thread.total_tokens_used += llm_output.usage.total();
|
||||
self.thread.total_cost_usd += llm_output.usage.cost_usd;
|
||||
step.llm_response = Some(llm_output.response.clone());
|
||||
|
||||
debug!(
|
||||
@@ -318,7 +340,7 @@ impl ExecutionLoop {
|
||||
self.thread
|
||||
.add_message(ThreadMessage::assistant(text));
|
||||
self.thread
|
||||
.add_message(ThreadMessage::system(intent::TOOL_INTENT_NUDGE));
|
||||
.add_message(ThreadMessage::user(intent::TOOL_INTENT_NUDGE));
|
||||
|
||||
step.status = StepStatus::Completed;
|
||||
step.completed_at = Some(chrono::Utc::now());
|
||||
@@ -437,7 +459,7 @@ impl ExecutionLoop {
|
||||
if self.thread.step_count == 0 {
|
||||
let preamble =
|
||||
crate::executor::scripting::build_orientation_preamble(&self.thread);
|
||||
self.thread.add_message(ThreadMessage::system(preamble));
|
||||
self.thread.add_message(ThreadMessage::user(preamble));
|
||||
}
|
||||
|
||||
let exec_ctx = ThreadExecutionContext {
|
||||
@@ -531,7 +553,8 @@ impl ExecutionLoop {
|
||||
for result in &step.action_results {
|
||||
let output_str = serde_json::to_string(&result.output).unwrap_or_default();
|
||||
let truncated = if output_str.len() > 4000 {
|
||||
format!("{}... [truncated, {} total chars]", &output_str[..4000], output_str.len())
|
||||
let prefix: String = output_str.chars().take(4000).collect();
|
||||
format!("{prefix}... [truncated, {} total chars]", output_str.len())
|
||||
} else {
|
||||
output_str
|
||||
};
|
||||
@@ -553,8 +576,9 @@ impl ExecutionLoop {
|
||||
output_parts.join("\n")
|
||||
};
|
||||
// Truncate total output to prevent context bloat
|
||||
let mut metadata = if output_text.len() > 8000 {
|
||||
format!("[TRUNCATED: last 8000 of {} chars]\n{}", output_text.len(), &output_text[output_text.len()-8000..])
|
||||
let mut metadata = if output_text.chars().count() > 8000 {
|
||||
let tail: String = output_text.chars().skip(output_text.chars().count() - 8000).collect();
|
||||
format!("[TRUNCATED: last 8000 of {} chars]\n{tail}", output_text.chars().count())
|
||||
} else {
|
||||
output_text
|
||||
};
|
||||
@@ -571,7 +595,7 @@ impl ExecutionLoop {
|
||||
));
|
||||
}
|
||||
|
||||
self.thread.add_message(ThreadMessage::system(metadata));
|
||||
self.thread.add_message(ThreadMessage::user(metadata));
|
||||
|
||||
step.status = StepStatus::Completed;
|
||||
step.completed_at = Some(chrono::Utc::now());
|
||||
|
||||
@@ -78,8 +78,8 @@ pub fn compact_output_metadata(stdout: &str, return_value: &serde_json::Value) -
|
||||
let mut parts = Vec::new();
|
||||
|
||||
if !stdout.is_empty() {
|
||||
if stdout.len() > OUTPUT_TRUNCATE_LEN {
|
||||
let truncated = &stdout[stdout.len() - OUTPUT_TRUNCATE_LEN..];
|
||||
if stdout.chars().count() > OUTPUT_TRUNCATE_LEN {
|
||||
let truncated: String = stdout.chars().skip(stdout.chars().count() - OUTPUT_TRUNCATE_LEN).collect();
|
||||
parts.push(format!(
|
||||
"[TRUNCATED: last {OUTPUT_TRUNCATE_LEN} of {} chars shown]\n{truncated}",
|
||||
stdout.len()
|
||||
@@ -374,6 +374,21 @@ pub async fn execute_code(
|
||||
.await
|
||||
}
|
||||
|
||||
// rlm_query(prompt) — full recursive sub-agent with own CodeAct loop
|
||||
"rlm_query" => {
|
||||
handle_rlm_query(
|
||||
&call.args,
|
||||
&call.kwargs,
|
||||
thread,
|
||||
llm,
|
||||
effects,
|
||||
leases,
|
||||
policy,
|
||||
&mut recursive_tokens,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
// Regular tool dispatch
|
||||
_ => {
|
||||
let dispatch = dispatch_action(
|
||||
@@ -679,6 +694,153 @@ async fn handle_llm_query_batched(
|
||||
ExtFunctionResult::Return(MontyObject::List(results))
|
||||
}
|
||||
|
||||
// ── rlm_query() — full recursive sub-agent (RLM 3.5) ─────────
|
||||
|
||||
/// Handle `rlm_query(prompt)` — spawn a child CodeAct thread with its own
|
||||
/// execution loop, tools, and iteration budget.
|
||||
///
|
||||
/// Unlike `llm_query()` (single-shot LLM call), `rlm_query()` creates a
|
||||
/// child thread with full CodeAct capabilities. The child inherits the
|
||||
/// parent's remaining budget and tool access.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn handle_rlm_query(
|
||||
args: &[MontyObject],
|
||||
kwargs: &[(MontyObject, MontyObject)],
|
||||
parent_thread: &Thread,
|
||||
llm: &Arc<dyn LlmBackend>,
|
||||
effects: &Arc<dyn EffectExecutor>,
|
||||
leases: &LeaseManager,
|
||||
policy: &PolicyEngine,
|
||||
recursive_tokens: &mut TokenUsage,
|
||||
) -> ExtFunctionResult {
|
||||
let prompt = extract_string_arg(args, kwargs, "prompt", 0);
|
||||
let prompt = match prompt {
|
||||
Some(p) => p,
|
||||
None => {
|
||||
return ExtFunctionResult::Error(MontyException::new(
|
||||
ExcType::TypeError,
|
||||
Some("rlm_query() requires a 'prompt' argument".into()),
|
||||
));
|
||||
}
|
||||
};
|
||||
|
||||
// Depth check — refuse if at max recursion depth
|
||||
let current_depth = parent_thread.config.depth;
|
||||
let max_depth = parent_thread.config.max_depth;
|
||||
if current_depth >= max_depth {
|
||||
return ExtFunctionResult::Error(MontyException::new(
|
||||
ExcType::RuntimeError,
|
||||
Some(format!(
|
||||
"rlm_query() depth limit reached: depth {current_depth} >= max {max_depth}"
|
||||
)),
|
||||
));
|
||||
}
|
||||
|
||||
// Build child thread with inherited budget
|
||||
let child_config = crate::types::thread::ThreadConfig {
|
||||
max_iterations: parent_thread.config.max_iterations.min(20), // cap child iterations
|
||||
enable_reflection: false,
|
||||
enable_tool_intent_nudge: false,
|
||||
max_tokens_total: parent_thread.config.max_tokens_total.map(|max| {
|
||||
max.saturating_sub(parent_thread.total_tokens_used)
|
||||
}),
|
||||
max_budget_usd: parent_thread.config.max_budget_usd.map(|max| {
|
||||
(max - parent_thread.total_cost_usd).max(0.0)
|
||||
}),
|
||||
max_duration: parent_thread.config.max_duration,
|
||||
depth: current_depth + 1,
|
||||
max_depth,
|
||||
..crate::types::thread::ThreadConfig::default()
|
||||
};
|
||||
|
||||
let mut child_thread = crate::types::thread::Thread::new(
|
||||
&prompt,
|
||||
crate::types::thread::ThreadType::Research,
|
||||
parent_thread.project_id,
|
||||
child_config,
|
||||
)
|
||||
.with_parent(parent_thread.id);
|
||||
|
||||
// Add the prompt as a user message
|
||||
child_thread.add_message(ThreadMessage::user(&prompt));
|
||||
|
||||
// Create signal channel and child's lease manager
|
||||
let (_tx, rx) = crate::runtime::messaging::signal_channel(8);
|
||||
let child_leases = Arc::new(LeaseManager::new());
|
||||
|
||||
// Grant the child the same leases as the parent (in the child's manager)
|
||||
let parent_leases = leases.active_for_thread(parent_thread.id).await;
|
||||
let now = chrono::Utc::now();
|
||||
for parent_lease in &parent_leases {
|
||||
// Convert parent's expires_at to remaining duration
|
||||
let remaining_duration = parent_lease
|
||||
.expires_at
|
||||
.and_then(|exp| (exp - now).to_std().ok())
|
||||
.map(|d| chrono::Duration::from_std(d).unwrap_or(chrono::Duration::hours(1)));
|
||||
let lease = child_leases
|
||||
.grant(
|
||||
child_thread.id,
|
||||
&parent_lease.capability_name,
|
||||
parent_lease.granted_actions.clone(),
|
||||
remaining_duration,
|
||||
parent_lease.max_uses,
|
||||
)
|
||||
.await;
|
||||
child_thread.capability_leases.push(lease.id);
|
||||
}
|
||||
let mut child_policy_engine = PolicyEngine::new();
|
||||
// Copy denied effects from parent policy
|
||||
for effect in &policy.denied_effects {
|
||||
child_policy_engine.deny_effect(*effect);
|
||||
}
|
||||
let child_policy = Arc::new(child_policy_engine);
|
||||
|
||||
let mut child_loop = crate::executor::ExecutionLoop::new(
|
||||
child_thread,
|
||||
Arc::clone(llm),
|
||||
Arc::clone(effects),
|
||||
child_leases,
|
||||
child_policy,
|
||||
rx,
|
||||
"rlm_child".to_string(),
|
||||
);
|
||||
|
||||
debug!(
|
||||
parent_thread = %parent_thread.id,
|
||||
depth = current_depth + 1,
|
||||
prompt_len = prompt.len(),
|
||||
"rlm_query: spawning child CodeAct thread"
|
||||
);
|
||||
|
||||
// Run the child loop (Box::pin to avoid infinite future size from recursion)
|
||||
match Box::pin(child_loop.run()).await {
|
||||
Ok(outcome) => {
|
||||
// Track child's token usage
|
||||
recursive_tokens.input_tokens += child_loop.thread.total_tokens_used;
|
||||
recursive_tokens.cost_usd += child_loop.thread.total_cost_usd;
|
||||
|
||||
let response = match outcome {
|
||||
crate::runtime::messaging::ThreadOutcome::Completed { response } => {
|
||||
response.unwrap_or_default()
|
||||
}
|
||||
crate::runtime::messaging::ThreadOutcome::Failed { error } => {
|
||||
format!("rlm_query child failed: {error}")
|
||||
}
|
||||
crate::runtime::messaging::ThreadOutcome::MaxIterations => {
|
||||
"rlm_query child reached max iterations".to_string()
|
||||
}
|
||||
_ => String::new(),
|
||||
};
|
||||
|
||||
ExtFunctionResult::Return(MontyObject::String(response))
|
||||
}
|
||||
Err(e) => ExtFunctionResult::Error(MontyException::new(
|
||||
ExcType::RuntimeError,
|
||||
Some(format!("rlm_query failed: {e}")),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
// ── Helpers ─────────────────────────────────────────────────
|
||||
|
||||
fn extract_string_arg(
|
||||
|
||||
@@ -146,6 +146,9 @@ pub struct ThreadConfig {
|
||||
/// Compaction threshold as fraction of model_context_limit (0.0-1.0).
|
||||
/// Default: 0.85 (matching official RLM).
|
||||
pub compaction_threshold: f64,
|
||||
/// Maximum cumulative USD cost before termination.
|
||||
/// Requires the LlmBackend to populate `TokenUsage::cost_usd`.
|
||||
pub max_budget_usd: Option<f64>,
|
||||
/// Depth of this thread in the recursive call tree.
|
||||
/// Root threads are depth 0. Sub-calls via rlm_query() increment depth.
|
||||
pub depth: u32,
|
||||
@@ -163,6 +166,7 @@ impl Default for ThreadConfig {
|
||||
max_tool_intent_nudges: 2,
|
||||
max_tokens_total: None,
|
||||
max_consecutive_errors: None,
|
||||
max_budget_usd: None,
|
||||
model_context_limit: 128_000,
|
||||
enable_compaction: false,
|
||||
compaction_threshold: 0.85,
|
||||
@@ -193,6 +197,8 @@ pub struct Thread {
|
||||
pub completed_at: Option<DateTime<Utc>>,
|
||||
pub step_count: usize,
|
||||
pub total_tokens_used: u64,
|
||||
/// Cumulative USD cost across all steps.
|
||||
pub total_cost_usd: f64,
|
||||
}
|
||||
|
||||
impl Thread {
|
||||
@@ -221,6 +227,7 @@ impl Thread {
|
||||
completed_at: None,
|
||||
step_count: 0,
|
||||
total_tokens_used: 0,
|
||||
total_cost_usd: 0.0,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -270,8 +277,9 @@ impl Thread {
|
||||
|
||||
/// Add a message to this thread's conversation.
|
||||
pub fn add_message(&mut self, message: ThreadMessage) {
|
||||
let preview = if message.content.len() > 80 {
|
||||
format!("{}...", &message.content[..80])
|
||||
let preview = if message.content.chars().count() > 80 {
|
||||
let p: String = message.content.chars().take(80).collect();
|
||||
format!("{p}...")
|
||||
} else {
|
||||
message.content.clone()
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user