From dd0a0e10abebcd7c161c6e86fb89b8bd06e38592 Mon Sep 17 00:00:00 2001 From: Nige Date: Thu, 26 Mar 2026 23:20:01 +0000 Subject: [PATCH] fix(routines): recover delete name after failed update fallback (#1108) Co-authored-by: ilblackdragon@gmail.com Co-authored-by: Claude Opus 4.6 (1M context) --- src/context/state.rs | 3 + src/tools/builtin/routine.rs | 38 +++++++++- tests/e2e_builtin_tool_coverage.rs | 43 +++++++++++- .../routine_update_fail_delete_fallback.json | 70 +++++++++++++++++++ 4 files changed, 148 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json diff --git a/src/context/state.rs b/src/context/state.rs index f5307947..0bb1f29a 100644 --- a/src/context/state.rs +++ b/src/context/state.rs @@ -192,6 +192,9 @@ pub struct JobContext { /// but subsequent tools (e.g., `json`) may need the full output. This /// stash stores the complete, unsanitized output so tools can reference /// 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)] pub tool_output_stash: Arc>>, /// User's preferred timezone (IANA name, e.g. "America/New_York"). Defaults to "UTC". diff --git a/src/tools/builtin/routine.rs b/src/tools/builtin/routine.rs index bbc24139..76f6e38b 100644 --- a/src/tools/builtin/routine.rs +++ b/src/tools/builtin/routine.rs @@ -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 { + 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> { params.get(field).and_then(Value::as_object) } @@ -1093,6 +1110,7 @@ impl Tool for RoutineCreateTool { ) -> Result { let start = std::time::Instant::now(); let normalized = parse_routine_create_request(¶ms)?; + stash_last_routine_name(ctx, &normalized.name).await; let trigger = build_routine_trigger(&normalized.trigger); let action = build_routine_action(&normalized.name, &normalized.prompt, &normalized.execution); @@ -1274,6 +1292,7 @@ impl Tool for RoutineUpdateTool { let start = std::time::Instant::now(); let name = require_str(¶ms, "name")?; + stash_last_routine_name(ctx, name).await; let mut routine = self .store @@ -1411,11 +1430,24 @@ impl Tool for RoutineDeleteTool { ) -> Result { 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 .store - .get_routine_by_name(&ctx.user_id, name) + .get_routine_by_name(&ctx.user_id, &name) .await .map_err(|e| ToolError::ExecutionFailed(format!("DB error: {e}")))? .ok_or_else(|| ToolError::ExecutionFailed(format!("routine '{}' not found", name)))?; @@ -1430,7 +1462,7 @@ impl Tool for RoutineDeleteTool { self.engine.refresh_event_cache().await; let result = serde_json::json!({ - "name": name, + "name": &name, "deleted": deleted, }); diff --git a/tests/e2e_builtin_tool_coverage.rs b/tests/e2e_builtin_tool_coverage.rs index 1c3cc6a2..7c0c7bc7 100644 --- a/tests/e2e_builtin_tool_coverage.rs +++ b/tests/e2e_builtin_tool_coverage.rs @@ -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] @@ -246,7 +283,7 @@ mod tests { } // ----------------------------------------------------------------------- - // Test 6: routine_manual_create_explicit_no_tools + // Test 7: routine_manual_create_explicit_no_tools // ----------------------------------------------------------------------- #[tokio::test] @@ -287,7 +324,7 @@ mod tests { } // ----------------------------------------------------------------------- - // Test 7: routine_history + // Test 8: routine_history // ----------------------------------------------------------------------- #[tokio::test] diff --git a/tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json b/tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json new file mode 100644 index 00000000..5c76dbb5 --- /dev/null +++ b/tests/fixtures/llm_traces/tools/routine_update_fail_delete_fallback.json @@ -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 + } + } + ] +}