mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix(routines): recover delete name after failed update fallback (#1108)
Co-authored-by: [email protected] <[email protected]> Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
@@ -192,6 +192,9 @@ pub struct JobContext {
|
|||||||
/// but subsequent tools (e.g., `json`) may need the full output. This
|
/// but subsequent tools (e.g., `json`) may need the full output. This
|
||||||
/// stash stores the complete, unsanitized output so tools can reference
|
/// stash stores the complete, unsanitized output so tools can reference
|
||||||
/// previous results by ID via `$tool_call_id` parameter syntax.
|
/// previous results by ID via `$tool_call_id` parameter syntax.
|
||||||
|
///
|
||||||
|
/// Also used for cross-tool implicit state (keys prefixed with `__`) such
|
||||||
|
/// as `__routine_last_name` for fallback recovery in routine tool chains.
|
||||||
#[serde(skip)]
|
#[serde(skip)]
|
||||||
pub tool_output_stash: Arc<tokio::sync::RwLock<HashMap<String, String>>>,
|
pub tool_output_stash: Arc<tokio::sync::RwLock<HashMap<String, String>>>,
|
||||||
/// User's preferred timezone (IANA name, e.g. "America/New_York"). Defaults to "UTC".
|
/// User's preferred timezone (IANA name, e.g. "America/New_York"). Defaults to "UTC".
|
||||||
|
|||||||
@@ -650,6 +650,23 @@ pub(crate) fn routine_update_parameters_schema() -> Value {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const ROUTINE_LAST_NAME_STASH_KEY: &str = "__routine_last_name";
|
||||||
|
|
||||||
|
async fn stash_last_routine_name(ctx: &JobContext, name: &str) {
|
||||||
|
ctx.tool_output_stash
|
||||||
|
.write()
|
||||||
|
.await
|
||||||
|
.insert(ROUTINE_LAST_NAME_STASH_KEY.to_string(), name.to_string());
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn restore_last_routine_name(ctx: &JobContext) -> Option<String> {
|
||||||
|
ctx.tool_output_stash
|
||||||
|
.read()
|
||||||
|
.await
|
||||||
|
.get(ROUTINE_LAST_NAME_STASH_KEY)
|
||||||
|
.cloned()
|
||||||
|
}
|
||||||
|
|
||||||
fn nested_object<'a>(params: &'a Value, field: &str) -> Option<&'a Map<String, Value>> {
|
fn nested_object<'a>(params: &'a Value, field: &str) -> Option<&'a Map<String, Value>> {
|
||||||
params.get(field).and_then(Value::as_object)
|
params.get(field).and_then(Value::as_object)
|
||||||
}
|
}
|
||||||
@@ -1093,6 +1110,7 @@ impl Tool for RoutineCreateTool {
|
|||||||
) -> Result<ToolOutput, ToolError> {
|
) -> Result<ToolOutput, ToolError> {
|
||||||
let start = std::time::Instant::now();
|
let start = std::time::Instant::now();
|
||||||
let normalized = parse_routine_create_request(¶ms)?;
|
let normalized = parse_routine_create_request(¶ms)?;
|
||||||
|
stash_last_routine_name(ctx, &normalized.name).await;
|
||||||
let trigger = build_routine_trigger(&normalized.trigger);
|
let trigger = build_routine_trigger(&normalized.trigger);
|
||||||
let action =
|
let action =
|
||||||
build_routine_action(&normalized.name, &normalized.prompt, &normalized.execution);
|
build_routine_action(&normalized.name, &normalized.prompt, &normalized.execution);
|
||||||
@@ -1274,6 +1292,7 @@ impl Tool for RoutineUpdateTool {
|
|||||||
let start = std::time::Instant::now();
|
let start = std::time::Instant::now();
|
||||||
|
|
||||||
let name = require_str(¶ms, "name")?;
|
let name = require_str(¶ms, "name")?;
|
||||||
|
stash_last_routine_name(ctx, name).await;
|
||||||
|
|
||||||
let mut routine = self
|
let mut routine = self
|
||||||
.store
|
.store
|
||||||
@@ -1411,11 +1430,24 @@ impl Tool for RoutineDeleteTool {
|
|||||||
) -> Result<ToolOutput, ToolError> {
|
) -> Result<ToolOutput, ToolError> {
|
||||||
let start = std::time::Instant::now();
|
let start = std::time::Instant::now();
|
||||||
|
|
||||||
let name = require_str(¶ms, "name")?;
|
let name = if let Some(name) = params.get("name").and_then(|v| v.as_str()) {
|
||||||
|
if name.trim().is_empty() {
|
||||||
|
return Err(ToolError::InvalidParameters(
|
||||||
|
"'name' parameter cannot be empty".to_string(),
|
||||||
|
));
|
||||||
|
}
|
||||||
|
name.to_string()
|
||||||
|
} else {
|
||||||
|
restore_last_routine_name(ctx).await.ok_or_else(|| {
|
||||||
|
ToolError::InvalidParameters(
|
||||||
|
"missing 'name' parameter and no previous routine target to infer".to_string(),
|
||||||
|
)
|
||||||
|
})?
|
||||||
|
};
|
||||||
|
|
||||||
let routine = self
|
let routine = self
|
||||||
.store
|
.store
|
||||||
.get_routine_by_name(&ctx.user_id, name)
|
.get_routine_by_name(&ctx.user_id, &name)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| ToolError::ExecutionFailed(format!("DB error: {e}")))?
|
.map_err(|e| ToolError::ExecutionFailed(format!("DB error: {e}")))?
|
||||||
.ok_or_else(|| ToolError::ExecutionFailed(format!("routine '{}' not found", name)))?;
|
.ok_or_else(|| ToolError::ExecutionFailed(format!("routine '{}' not found", name)))?;
|
||||||
@@ -1430,7 +1462,7 @@ impl Tool for RoutineDeleteTool {
|
|||||||
self.engine.refresh_event_cache().await;
|
self.engine.refresh_event_cache().await;
|
||||||
|
|
||||||
let result = serde_json::json!({
|
let result = serde_json::json!({
|
||||||
"name": name,
|
"name": &name,
|
||||||
"deleted": deleted,
|
"deleted": deleted,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -205,7 +205,44 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
// Test 5: routine_manual_create_defaults_to_tools_enabled
|
// Test 5: routine_update_fail_delete_fallback
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn routine_update_fail_delete_fallback() {
|
||||||
|
let trace = LlmTrace::from_file(concat!(
|
||||||
|
env!("CARGO_MANIFEST_DIR"),
|
||||||
|
"/tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json"
|
||||||
|
))
|
||||||
|
.expect("failed to load routine_update_fail_delete_fallback.json");
|
||||||
|
|
||||||
|
let rig = TestRigBuilder::new()
|
||||||
|
.with_trace(trace.clone())
|
||||||
|
.with_auto_approve_tools(true)
|
||||||
|
.build()
|
||||||
|
.await;
|
||||||
|
|
||||||
|
rig.send_message("Try converting a routine trigger, then recover by deleting it")
|
||||||
|
.await;
|
||||||
|
let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await;
|
||||||
|
|
||||||
|
rig.verify_trace_expects(&trace, &responses);
|
||||||
|
|
||||||
|
let completed = rig.tool_calls_completed();
|
||||||
|
assert!(
|
||||||
|
completed.iter().any(|(n, ok)| n == "routine_update" && !ok),
|
||||||
|
"routine_update should fail in this regression path: {completed:?}"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
completed.iter().any(|(n, ok)| n == "routine_delete" && *ok),
|
||||||
|
"routine_delete should recover successfully via preserved routine identity: {completed:?}"
|
||||||
|
);
|
||||||
|
|
||||||
|
rig.shutdown();
|
||||||
|
}
|
||||||
|
|
||||||
|
// -----------------------------------------------------------------------
|
||||||
|
// Test 6: routine_manual_create_defaults_to_tools_enabled
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
@@ -246,7 +283,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
// Test 6: routine_manual_create_explicit_no_tools
|
// Test 7: routine_manual_create_explicit_no_tools
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
@@ -287,7 +324,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
// Test 7: routine_history
|
// Test 8: routine_history
|
||||||
// -----------------------------------------------------------------------
|
// -----------------------------------------------------------------------
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
|
|||||||
@@ -0,0 +1,70 @@
|
|||||||
|
{
|
||||||
|
"model_name": "test-routine-update-fail-delete-fallback",
|
||||||
|
"expects": {
|
||||||
|
"tools_used": ["routine_create", "routine_update", "routine_delete"],
|
||||||
|
"tool_results_contain": {
|
||||||
|
"routine_update": "Cannot update schedule or timezone on a non-cron routine.",
|
||||||
|
"routine_delete": "temp-routine"
|
||||||
|
},
|
||||||
|
"min_responses": 1
|
||||||
|
},
|
||||||
|
"steps": [
|
||||||
|
{
|
||||||
|
"response": {
|
||||||
|
"type": "tool_calls",
|
||||||
|
"tool_calls": [
|
||||||
|
{
|
||||||
|
"id": "call_rc_fallback",
|
||||||
|
"name": "routine_create",
|
||||||
|
"arguments": {
|
||||||
|
"name": "temp-routine",
|
||||||
|
"trigger_type": "manual",
|
||||||
|
"prompt": "Temporary routine for fallback test."
|
||||||
|
}
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"input_tokens": 120,
|
||||||
|
"output_tokens": 40
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"response": {
|
||||||
|
"type": "tool_calls",
|
||||||
|
"tool_calls": [
|
||||||
|
{
|
||||||
|
"id": "call_ru_fallback",
|
||||||
|
"name": "routine_update",
|
||||||
|
"arguments": {
|
||||||
|
"name": "temp-routine",
|
||||||
|
"schedule": "0 */10 * * * *"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"input_tokens": 200,
|
||||||
|
"output_tokens": 30
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"response": {
|
||||||
|
"type": "tool_calls",
|
||||||
|
"tool_calls": [
|
||||||
|
{
|
||||||
|
"id": "call_rd_fallback",
|
||||||
|
"name": "routine_delete",
|
||||||
|
"arguments": {}
|
||||||
|
}
|
||||||
|
],
|
||||||
|
"input_tokens": 300,
|
||||||
|
"output_tokens": 20
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"response": {
|
||||||
|
"type": "text",
|
||||||
|
"content": "I recovered from the failed update and cleaned up the original routine.",
|
||||||
|
"input_tokens": 380,
|
||||||
|
"output_tokens": 25
|
||||||
|
}
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user