From dea789cca9853ee814ae05c565de4e84684801b5 Mon Sep 17 00:00:00 2001 From: Henry Park Date: Mon, 23 Mar 2026 11:01:26 -0700 Subject: [PATCH] Default new lightweight routines to tools-enabled (#1573) * Default new lightweight routines to tools-enabled * Fix fmt and clippy on lightweight routine PR * Use grouped execution field in routine no-tools fixture * Align CLI routine defaults with tools-enabled lightweight mode --- src/agent/routine_engine.rs | 1 + src/cli/routines.rs | 49 ++++++++++- src/tools/builtin/routine.rs | 87 +++++++++++++++++-- tests/e2e_builtin_tool_coverage.rs | 53 +++++++++-- .../tools/routine_manual_create_no_tools.json | 39 +++++++++ 5 files changed, 213 insertions(+), 16 deletions(-) create mode 100644 tests/fixtures/llm_traces/tools/routine_manual_create_no_tools.json diff --git a/src/agent/routine_engine.rs b/src/agent/routine_engine.rs index de2879b4..9b554582 100644 --- a/src/agent/routine_engine.rs +++ b/src/agent/routine_engine.rs @@ -1440,6 +1440,7 @@ fn handle_text_response( /// This is a simplified version of the full dispatcher loop: /// - Max 3-5 iterations (configurable) /// - Sequential tool execution (not parallel) +/// - Uses the owner's live autonomous tool scope when lightweight tools are enabled /// - Auto-approval of non-Always tools /// - No hooks or approval dialogs async fn execute_lightweight_with_tools( diff --git a/src/cli/routines.rs b/src/cli/routines.rs index dd8a2fa3..ebef8839 100644 --- a/src/cli/routines.rs +++ b/src/cli/routines.rs @@ -340,8 +340,8 @@ async fn create( prompt: prompt.to_string(), context_paths: Vec::new(), max_tokens: 4096, - use_tools: false, - max_tool_rounds: 0, + use_tools: true, + max_tool_rounds: 3, }, guardrails: RoutineGuardrails { cooldown: std::time::Duration::from_secs(cooldown_secs), @@ -685,6 +685,7 @@ fn truncate(s: &str, max_chars: usize) -> String { #[cfg(test)] mod tests { use super::*; + use crate::agent::routine::RoutineAction; #[test] fn format_relative_future() { @@ -743,4 +744,48 @@ mod tests { assert!(notify.on_failure); // safety: test-only assertion assert!(!notify.on_success); // safety: test-only assertion } + + #[cfg(feature = "libsql")] + #[tokio::test] + async fn cli_create_defaults_lightweight_routines_to_tools_enabled() { + let harness = crate::testing::TestHarnessBuilder::new().build().await; + let db = harness.db.clone(); + + run_routines_command( + RoutinesCommand::Create { + name: "cli-digest".to_string(), + schedule: "0 0 9 * * *".to_string(), + prompt: "Prepare the morning digest.".to_string(), + description: "CLI created routine".to_string(), + timezone: Some("UTC".to_string()), + cooldown: 300, + notify_channel: None, + }, + db.clone(), + "user1", + ) + .await + .expect("create routine"); + + let routine = db + .get_routine_by_name("user1", "cli-digest") + .await + .expect("get routine by name") + .expect("cli-digest should exist"); + + match routine.action { + RoutineAction::Lightweight { + use_tools, + max_tool_rounds, + .. + } => { + assert!( + use_tools, + "CLI-created lightweight routines should default to tools" + ); + assert_eq!(max_tool_rounds, 3); + } + other => panic!("expected lightweight action, got {other:?}"), + } + } } diff --git a/src/tools/builtin/routine.rs b/src/tools/builtin/routine.rs index c197fe25..17e17ba1 100644 --- a/src/tools/builtin/routine.rs +++ b/src/tools/builtin/routine.rs @@ -140,7 +140,8 @@ fn execution_properties() -> Value { }, "use_tools": { "type": "boolean", - "description": "Only applies to lightweight mode. When true, safe non-approval tools are available." + "default": true, + "description": "Only applies to lightweight mode. New lightweight routines default this to true; when enabled, the routine can use the owner's live autonomous tool scope." }, "max_tool_rounds": { "type": "integer", @@ -290,7 +291,7 @@ fn routine_request_discovery_schema() -> Value { fn lightweight_execution_variant() -> Value { serde_json::json!({ "type": "object", - "description": "Default lightweight execution. Applies when execution is omitted or execution.mode='lightweight'.", + "description": "Default lightweight execution. Applies when execution is omitted or execution.mode='lightweight'. New lightweight routines default to tools enabled unless execution.use_tools=false is set.", "properties": { "mode": { "type": "string", @@ -304,7 +305,8 @@ fn lightweight_execution_variant() -> Value { }, "use_tools": { "type": "boolean", - "description": "When true, safe non-approval tools are available." + "default": true, + "description": "Defaults to true for new lightweight routines. When enabled, the routine can use the owner's live autonomous tool scope." }, "max_tool_rounds": { "type": "integer", @@ -335,7 +337,7 @@ fn full_job_execution_variant() -> Value { fn execution_discovery_schema() -> Value { serde_json::json!({ "type": "object", - "description": "Optional execution settings. Omit this block for the default lightweight mode.", + "description": "Optional execution settings. Omit this block for the default lightweight mode with tools enabled.", "properties": execution_properties(), "oneOf": [ lightweight_execution_variant(), @@ -408,7 +410,8 @@ fn routine_create_tool_summary() -> ToolDiscoverySummary { "execution.mode='full_job' uses the owner's live autonomous tool scope and ignores use_tools, max_tool_rounds, and context_paths.".into(), ], notes: vec![ - "Omitting execution defaults to lightweight mode.".into(), + "Omitting execution defaults to lightweight mode with tools enabled.".into(), + "Set execution.use_tools=false to keep a new lightweight routine text-only.".into(), "Omitting delivery.user falls back to the owner's last-seen notification target.".into(), "advanced.cooldown_secs defaults to 300.".into(), "Legacy flat aliases are still accepted for compatibility, but grouped fields are preferred.".into(), @@ -852,11 +855,15 @@ fn parse_execution_mode(value: Option) -> Result Result { +fn parse_routine_execution( + params: &Value, + default_use_tools: bool, +) -> Result { let mode = parse_execution_mode(string_field(params, "execution", "mode", &["action_type"]))?; let context_paths = string_array_field(params, "execution", "context_paths", &["context_paths"]); - let use_tools = bool_field(params, "execution", "use_tools", &["use_tools"]).unwrap_or(false); + let use_tools = + bool_field(params, "execution", "use_tools", &["use_tools"]).unwrap_or(default_use_tools); let max_tool_rounds = u64_field(params, "execution", "max_tool_rounds", &["max_tool_rounds"]) .unwrap_or(3) .clamp(1, crate::agent::routine::MAX_TOOL_ROUNDS_LIMIT as u64) @@ -888,7 +895,7 @@ fn parse_routine_create_request( .unwrap_or("") .to_string(); let trigger = parse_routine_trigger(params)?; - let execution = parse_routine_execution(params)?; + let execution = parse_routine_execution(params, true)?; let delivery = parse_routine_delivery(params); let cooldown_secs = u64_field(params, "advanced", "cooldown_secs", &["cooldown_secs"]).unwrap_or(300); @@ -1863,6 +1870,56 @@ mod tests { ); } + #[test] + fn parses_lightweight_create_with_tools_enabled_by_default() { + let params = serde_json::json!({ + "name": "manual-check", + "prompt": "Inspect the repo for issues.", + "request": { + "kind": "manual" + } + }); + + let parsed = parse_routine_create_request(¶ms).expect("parse default lightweight"); + + assert!( + matches!(parsed.execution.mode, NormalizedExecutionMode::Lightweight), + "expected lightweight execution mode", + ); + assert!( + parsed.execution.use_tools, + "new lightweight routines should default use_tools=true", + ); + assert_eq!(parsed.execution.max_tool_rounds, 3); + } + + #[test] + fn parses_lightweight_create_with_explicit_tools_disabled() { + let params = serde_json::json!({ + "name": "manual-check", + "prompt": "Inspect the repo for issues.", + "request": { + "kind": "manual" + }, + "execution": { + "use_tools": false + } + }); + + let parsed = + parse_routine_create_request(¶ms).expect("parse lightweight with tools disabled"); + + assert!( + matches!(parsed.execution.mode, NormalizedExecutionMode::Lightweight), + "expected lightweight execution mode", + ); + assert!( + !parsed.execution.use_tools, + "explicit use_tools=false should be preserved", + ); + assert_eq!(parsed.execution.max_tool_rounds, 3); + } + #[test] fn parses_context_paths_with_trim_drop_empty_and_stable_dedupe() { let params = serde_json::json!({ @@ -2201,6 +2258,20 @@ mod tests { .any(|rule| rule.contains("request.kind='cron'")), "summary should explain cron requirement", ); + assert!( + summary + .notes + .iter() + .any(|note| note.contains("lightweight mode with tools enabled")), + "summary should mention the new lightweight default", + ); + assert!( + summary + .notes + .iter() + .any(|note| note.contains("execution.use_tools=false")), + "summary should mention the text-only opt-out", + ); assert!( summary .notes diff --git a/tests/e2e_builtin_tool_coverage.rs b/tests/e2e_builtin_tool_coverage.rs index 69982b84..42d7fb75 100644 --- a/tests/e2e_builtin_tool_coverage.rs +++ b/tests/e2e_builtin_tool_coverage.rs @@ -205,11 +205,11 @@ mod tests { } // ----------------------------------------------------------------------- - // Test 5: routine_manual_create + // Test 5: routine_manual_create_defaults_to_tools_enabled // ----------------------------------------------------------------------- #[tokio::test] - async fn routine_manual_create() { + async fn routine_manual_create_defaults_to_tools_enabled() { let trace = LlmTrace::from_file(concat!( env!("CARGO_MANIFEST_DIR"), "/tests/fixtures/llm_traces/tools/routine_manual_create.json" @@ -237,8 +237,8 @@ mod tests { assert!(matches!(routine.trigger, Trigger::Manual)); assert!( - matches!(&routine.action, RoutineAction::Lightweight { use_tools, .. } if !*use_tools), - "manual routine should default to lightweight without tools: {:?}", + matches!(&routine.action, RoutineAction::Lightweight { use_tools, .. } if *use_tools), + "manual routine should default to lightweight with tools enabled: {:?}", routine.action ); @@ -246,7 +246,48 @@ mod tests { } // ----------------------------------------------------------------------- - // Test 6: routine_history + // Test 6: routine_manual_create_explicit_no_tools + // ----------------------------------------------------------------------- + + #[tokio::test] + async fn routine_manual_create_explicit_no_tools() { + let trace = LlmTrace::from_file(concat!( + env!("CARGO_MANIFEST_DIR"), + "/tests/fixtures/llm_traces/tools/routine_manual_create_no_tools.json" + )) + .expect("failed to load routine_manual_create_no_tools.json"); + + let rig = TestRigBuilder::new() + .with_trace(trace.clone()) + .with_auto_approve_tools(true) + .build() + .await; + + rig.send_message("Create a manual routine for quiet text-only bug triage") + .await; + let responses = rig.wait_for_responses(1, Duration::from_secs(15)).await; + + rig.verify_trace_expects(&trace, &responses); + + let routine = rig + .database() + .get_routine_by_name("test-user", "manual-triage-no-tools") + .await + .expect("get_routine_by_name") + .expect("manual-triage-no-tools should exist"); + + assert!(matches!(routine.trigger, Trigger::Manual)); + assert!( + matches!(&routine.action, RoutineAction::Lightweight { use_tools, .. } if !*use_tools), + "manual routine should preserve explicit use_tools=false: {:?}", + routine.action + ); + + rig.shutdown(); + } + + // ----------------------------------------------------------------------- + // Test 7: routine_history // ----------------------------------------------------------------------- #[tokio::test] @@ -283,7 +324,7 @@ mod tests { } // ----------------------------------------------------------------------- - // Test 7: routine_system_event_emit + // Test 8: routine_system_event_emit // ----------------------------------------------------------------------- #[tokio::test] diff --git a/tests/fixtures/llm_traces/tools/routine_manual_create_no_tools.json b/tests/fixtures/llm_traces/tools/routine_manual_create_no_tools.json new file mode 100644 index 00000000..275f2269 --- /dev/null +++ b/tests/fixtures/llm_traces/tools/routine_manual_create_no_tools.json @@ -0,0 +1,39 @@ +{ + "model_name": "test-routine-manual-create-no-tools", + "expects": { + "tools_used": ["routine_create"], + "all_tools_succeeded": true, + "min_responses": 1 + }, + "steps": [ + { + "response": { + "type": "tool_calls", + "tool_calls": [ + { + "id": "call_rc_manual_2", + "name": "routine_create", + "arguments": { + "name": "manual-triage-no-tools", + "trigger_type": "manual", + "prompt": "Summarize the latest bug reports when this routine is fired.", + "execution": { + "use_tools": false + } + } + } + ], + "input_tokens": 90, + "output_tokens": 24 + } + }, + { + "response": { + "type": "text", + "content": "Created the manual-triage-no-tools routine. It will only run when explicitly fired and stay text-only.", + "input_tokens": 140, + "output_tokens": 18 + } + } + ] +}