feat(routines): approval context for autonomous job execution (#577)

* feat(routines): add approval context for autonomous job execution

Routines and background jobs were unable to use any tools that required
approval (file ops, shell, message, http), making them effectively
useless. This adds an ApprovalContext system that lets autonomous jobs
pre-authorize tools at dispatch time.

- Add ApprovalContext enum with Autonomous variant that auto-approves
  UnlessAutoApproved tools and optionally pre-authorizes Always tools
- Add tool_permissions field to RoutineAction::FullJob for pre-authorizing
  Always-gated tools (e.g. destructive shell, cross-channel messaging)
- Add Scheduler::dispatch_job_with_context() to thread approval context
  through to workers
- Set message tool default channel/target from routine NotifyConfig
  so routines can send results without cross-channel approval
- Fix Completed→Completed state transition error in worker (plan marks
  job completed, then direct loop or outer run() tries again)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(routines): add E2E trace for routine news digest workflow

Add a 3-turn trace fixture and test that exercises:
- Turn 1: routine_create with full_job mode and tool_permissions
- Turn 2: Simulated digest workflow with echo + memory_write
- Turn 3: Verification via memory_search

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(test): wire RoutineEngine into test rig for routine_create E2E

- Add `with_routines()` to TestRigBuilder that passes a RoutineConfig
  to Agent::new, enabling routine tool registration during agent startup
- Add Turn 2 (routine_list) to the trace to verify routine persistence
  in the database after routine_create
- Fix formatting issues flagged by CI (cargo fmt)

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(scheduler): deduplicate dispatch_job and dispatch_job_with_context

Extract shared logic into private `dispatch_job_inner` to prevent
divergence when dispatch behavior changes in the future.

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(routines): add routine_fire tool and real E2E routine execution test

- Add `routine_fire` tool that calls `RoutineEngine::fire_manual` to
  trigger a routine on demand. Registered alongside the other 5 routine
  tools (now 6 total).

- Rewrite the routine_news_digest E2E trace to exercise the full
  execution stack end-to-end:
  1. routine_create (manual trigger, full_job, tool_permissions: [message])
  2. routine_fire → RoutineEngine → Scheduler::dispatch_job_with_context
     → autonomous Worker consuming TraceLlm steps
  3. Worker calls echo → memory_write → message (broadcast to test channel)
  4. Test verifies the message broadcast arrived, proving ApprovalContext
     correctly allowed the Always-approval message tool

- Register message tools in TestRig so routines can send messages to
  the test channel via channel_manager.broadcast().

[skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(routines): wire HttpInterceptor through scheduler for routine worker http calls

Propagate http_interceptor from AgentDeps → Scheduler → WorkerDeps → JobContext
so that routine workers (and any scheduler-dispatched workers) can use the
ReplayingHttpInterceptor for mock HTTP responses during tests.

Changes:
- Add http_interceptor field to Scheduler and WorkerDeps
- Set job_ctx.http_interceptor in Worker before tool execution
- Add with_http_exchanges() builder method to TestRigBuilder
- Replace echo tool with http tool in routine_news_digest trace
- Test now exercises real http tool with mock response → memory_write → message

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review comments from Copilot on PR #577

- Extract `ApprovalContext::is_blocked_or_default()` helper to deduplicate
  approval check logic in worker.rs and scheduler.rs
- Extract `parse_tool_permissions()` helper to deduplicate JSON array
  parsing in routine.rs and builtin/routine.rs
- Fix test name: `test_mark_completed_twice_does_not_error` →
  `test_mark_completed_twice_returns_error` (matches actual behavior)
- Fix ApprovalContext doc comment to clarify it only models autonomous mode
- Fix flaky index-based assertion in routine_news_digest test — now uses
  content-based search instead of fixed position
- Fix stale comment: echo → http in routine test header
- Add TODO for subtask approval context propagation (latent, not in
  active code paths)
- Add TODO for global message tool context race in routine_engine

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: fix formatting in is_blocked_or_default test

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(test_rig): destructure self in build() to avoid partial-move fragility

Destructure TestRigBuilder at the top of build() instead of accessing
self.* fields after moving self.http_exchanges. While the prior code
compiled (remaining fields are Copy), it was fragile and would break
if any non-Copy field were added.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: clarify that routine_fire bypasses cooldown

Manual fires are explicitly user-initiated and intentionally bypass
cooldown checks (which only apply to automated cron/event triggers).
Updated tool description and fire_manual docstring to make this clear.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(routines): fix message tool approval in routine context

Two fixes for message tool failures in autonomous routine jobs:

1. MessageTool::requires_approval() now returns UnlessAutoApproved when
   the explicit channel param matches the default channel (was Always,
   causing "requires authentication" errors for routine workers).

2. routine_create tool now accepts notify_channel and notify_user params,
   wired into NotifyConfig. Without these, routines had channel: None,
   so set_message_tool_context was never called, causing "No channel
   specified" errors.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(message): remove approval requirement from message tool

The message tool only sends to user-owned channels via
ChannelManager::broadcast (TUI, Telegram, Slack, web gateway, etc.).
It cannot reach arbitrary external services, so approval adds friction
with no security benefit. This also eliminates the routine context
errors entirely since approval is never checked.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review comments — routine_fire approval + test rename

- routine_fire now returns UnlessAutoApproved since firing a routine
  can dispatch a full_job with pre-authorized Always-gated tools
- Rename test_approval_context_never_always_passes to
  test_approval_context_never_is_not_blocked for clarity

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address review nits — update stale docs and comments

- Remove 'message' from tool_permissions example (no longer Always)
- Reword message tool approval comment for accuracy
- Clarify with_routines() docstring re: tool registration vs engine wiring

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
This commit is contained in:
Illia Polosukhin
2026-03-07 05:21:58 +00:00
committed by GitHub
co-authored by Claude Opus 4.6
parent 5c2ba44f12
commit ae89a52ac2
15 changed files with 1168 additions and 113 deletions
+15 -67
View File
@@ -207,26 +207,10 @@ impl Tool for MessageTool {
}
}
fn requires_approval(&self, params: &serde_json::Value) -> ApprovalRequirement {
// Require approval when sending to a different channel than the default
// (cross-channel messages are more sensitive)
let param_channel = params.get("channel").and_then(|v| v.as_str());
if let Some(channel) = param_channel {
// Check if it differs from the default channel
let default_channel = self
.default_channel
.read()
.unwrap_or_else(|e| e.into_inner());
if let Some(default) = default_channel.as_ref()
&& channel != default
{
return ApprovalRequirement::Always;
}
// No default set - require approval for explicit channel selection
return ApprovalRequirement::Always;
}
// No channel specified in params - uses default, less risky
ApprovalRequirement::UnlessAutoApproved
fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement {
// Message tool only delivers to channels the user has configured
// (TUI, Telegram, Slack, web gateway, etc.) via ChannelManager::broadcast.
ApprovalRequirement::Never
}
fn rate_limit_config(&self) -> Option<ToolRateLimitConfig> {
@@ -533,53 +517,17 @@ mod tests {
);
}
// ── Multi-thread runtime safety tests ─────────────────────────────
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn requires_approval_no_channel_multi_thread() {
#[test]
fn requires_approval_always_never() {
// Message tool only sends to user-owned channels, so never needs approval.
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
// No channel set, no channel param - should not panic in multi-thread runtime
let result = tool.requires_approval(&serde_json::json!({"content": "hello"}));
assert_eq!(result, ApprovalRequirement::UnlessAutoApproved);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn requires_approval_with_context_multi_thread() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// No channel param - uses default, less risky
let result = tool.requires_approval(&serde_json::json!({"content": "hello"}));
assert_eq!(result, ApprovalRequirement::UnlessAutoApproved);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn requires_approval_cross_channel_multi_thread() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Different channel than default requires approval
let result = tool.requires_approval(&serde_json::json!({
"content": "hello",
"channel": "telegram"
}));
assert_eq!(result, ApprovalRequirement::Always);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn requires_approval_same_channel_explicit_multi_thread() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Explicit channel that matches default still returns Always
// (existing behavior: any explicit channel param triggers Always)
let result = tool.requires_approval(&serde_json::json!({
"content": "hello",
"channel": "signal"
}));
assert_eq!(result, ApprovalRequirement::Always);
assert_eq!(
tool.requires_approval(&serde_json::json!({"content": "hello"})),
ApprovalRequirement::Never,
);
assert_eq!(
tool.requires_approval(&serde_json::json!({"content": "hi", "channel": "telegram"})),
ApprovalRequirement::Never,
);
}
}
+2 -1
View File
@@ -32,7 +32,8 @@ pub use memory::{MemoryReadTool, MemorySearchTool, MemoryTreeTool, MemoryWriteTo
pub use message::MessageTool;
pub use restart::RestartTool;
pub use routine::{
RoutineCreateTool, RoutineDeleteTool, RoutineHistoryTool, RoutineListTool, RoutineUpdateTool,
RoutineCreateTool, RoutineDeleteTool, RoutineFireTool, RoutineHistoryTool, RoutineListTool,
RoutineUpdateTool,
};
pub use secrets_tools::{SecretDeleteTool, SecretListTool};
pub use shell::ShellTool;
+113 -8
View File
@@ -1,10 +1,11 @@
//! LLM-facing tools for managing routines.
//!
//! Five tools let the agent manage routines conversationally:
//! Six tools let the agent manage routines conversationally:
//! - `routine_create` - Create a new routine
//! - `routine_list` - List all routines with status
//! - `routine_update` - Modify or toggle a routine
//! - `routine_delete` - Remove a routine
//! - `routine_fire` - Manually trigger a routine
//! - `routine_history` - View past runs
use std::sync::Arc;
@@ -20,7 +21,7 @@ use crate::agent::routine::{
use crate::agent::routine_engine::RoutineEngine;
use crate::context::JobContext;
use crate::db::Database;
use crate::tools::tool::{Tool, ToolError, ToolOutput, require_str};
use crate::tools::tool::{ApprovalRequirement, Tool, ToolError, ToolOutput, require_str};
// ==================== routine_create ====================
@@ -93,6 +94,19 @@ impl Tool for RoutineCreateTool {
"cooldown_secs": {
"type": "integer",
"description": "Minimum seconds between fires (default: 300)"
},
"tool_permissions": {
"type": "array",
"items": { "type": "string" },
"description": "Tool names pre-authorized for Always-approval tools in full_job mode (e.g. ['shell']). UnlessAutoApproved tools are automatically permitted in routines."
},
"notify_channel": {
"type": "string",
"description": "Channel to send results to (e.g. 'telegram', 'slack', 'tui'). Sets the default channel for message tool calls in routine jobs."
},
"notify_user": {
"type": "string",
"description": "User/target to notify (e.g. username, chat ID). Defaults to 'default'."
}
},
"required": ["name", "trigger_type", "prompt"]
@@ -192,11 +206,15 @@ impl Tool for RoutineCreateTool {
context_paths,
max_tokens: 4096,
},
"full_job" => RoutineAction::FullJob {
title: name.to_string(),
description: prompt.to_string(),
max_iterations: 10,
},
"full_job" => {
let tool_permissions = crate::agent::routine::parse_tool_permissions(&params);
RoutineAction::FullJob {
title: name.to_string(),
description: prompt.to_string(),
max_iterations: 10,
tool_permissions,
}
}
other => {
return Err(ToolError::InvalidParameters(format!(
"unknown action_type: {other}"
@@ -229,7 +247,18 @@ impl Tool for RoutineCreateTool {
max_concurrent: 1,
dedup_window: None,
},
notify: NotifyConfig::default(),
notify: NotifyConfig {
channel: params
.get("notify_channel")
.and_then(|v| v.as_str())
.map(String::from),
user: params
.get("notify_user")
.and_then(|v| v.as_str())
.unwrap_or("default")
.to_string(),
..NotifyConfig::default()
},
last_run_at: None,
next_fire_at: next_fire,
run_count: 0,
@@ -533,6 +562,82 @@ impl Tool for RoutineDeleteTool {
}
}
// ==================== routine_fire ====================
pub struct RoutineFireTool {
store: Arc<dyn Database>,
engine: Arc<RoutineEngine>,
}
impl RoutineFireTool {
pub fn new(store: Arc<dyn Database>, engine: Arc<RoutineEngine>) -> Self {
Self { store, engine }
}
}
#[async_trait]
impl Tool for RoutineFireTool {
fn name(&self) -> &str {
"routine_fire"
}
fn description(&self) -> &str {
"Manually trigger a routine to run immediately, bypassing schedule, trigger type, and cooldown."
}
fn requires_approval(&self, _params: &serde_json::Value) -> ApprovalRequirement {
// Firing a routine can dispatch a full_job with pre-authorized Always-gated tools,
// so this is a meaningful escalation that warrants auto-approval gating.
ApprovalRequirement::UnlessAutoApproved
}
fn parameters_schema(&self) -> serde_json::Value {
serde_json::json!({
"type": "object",
"properties": {
"name": {
"type": "string",
"description": "Name of the routine to fire"
}
},
"required": ["name"]
})
}
async fn execute(
&self,
params: serde_json::Value,
ctx: &JobContext,
) -> Result<ToolOutput, ToolError> {
let start = std::time::Instant::now();
let name = require_str(&params, "name")?;
let routine = self
.store
.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)))?;
let run_id = self.engine.fire_manual(routine.id).await.map_err(|e| {
ToolError::ExecutionFailed(format!("failed to fire routine '{}': {e}", name))
})?;
let result = serde_json::json!({
"name": name,
"run_id": run_id.to_string(),
"status": "fired",
});
Ok(ToolOutput::success(result, start.elapsed()))
}
fn requires_sanitization(&self) -> bool {
false
}
}
// ==================== routine_history ====================
pub struct RoutineHistoryTool {
+2 -2
View File
@@ -25,6 +25,6 @@ pub use builder::{
pub use rate_limiter::RateLimiter;
pub use registry::ToolRegistry;
pub use tool::{
ApprovalRequirement, Tool, ToolDomain, ToolError, ToolOutput, ToolRateLimitConfig,
redact_params, validate_tool_schema,
ApprovalContext, ApprovalRequirement, Tool, ToolDomain, ToolError, ToolOutput,
ToolRateLimitConfig, redact_params, validate_tool_schema,
};
+8 -3
View File
@@ -63,6 +63,7 @@ const PROTECTED_TOOL_NAMES: &[&str] = &[
"routine_list",
"routine_update",
"routine_delete",
"routine_fire",
"routine_history",
"skill_list",
"skill_search",
@@ -423,8 +424,8 @@ impl ToolRegistry {
engine: Arc<crate::agent::routine_engine::RoutineEngine>,
) {
use crate::tools::builtin::{
RoutineCreateTool, RoutineDeleteTool, RoutineHistoryTool, RoutineListTool,
RoutineUpdateTool,
RoutineCreateTool, RoutineDeleteTool, RoutineFireTool, RoutineHistoryTool,
RoutineListTool, RoutineUpdateTool,
};
self.register_sync(Arc::new(RoutineCreateTool::new(
Arc::clone(&store),
@@ -439,8 +440,12 @@ impl ToolRegistry {
Arc::clone(&store),
Arc::clone(&engine),
)));
self.register_sync(Arc::new(RoutineFireTool::new(
Arc::clone(&store),
Arc::clone(&engine),
)));
self.register_sync(Arc::new(RoutineHistoryTool::new(store)));
tracing::info!("Registered 5 routine management tools");
tracing::info!("Registered 6 routine management tools");
}
/// Register message tool for sending messages to channels.
+18 -1
View File
@@ -582,7 +582,14 @@ mod tests {
"enum": ["lightweight", "full_job"],
"description": "Execution mode"
},
"cooldown_secs": { "type": "integer", "description": "Min seconds between fires" }
"cooldown_secs": { "type": "integer", "description": "Min seconds between fires" },
"tool_permissions": {
"type": "array",
"items": { "type": "string" },
"description": "Pre-authorized tools for full_job mode"
},
"notify_channel": { "type": "string", "description": "Channel for message tool" },
"notify_user": { "type": "string", "description": "User/target to notify" }
},
"required": ["name", "trigger_type", "prompt"]
}),
@@ -619,6 +626,16 @@ mod tests {
"required": ["name"]
}),
),
(
"routine_fire",
serde_json::json!({
"type": "object",
"properties": {
"name": { "type": "string", "description": "Routine name" }
},
"required": ["name"]
}),
),
(
"routine_history",
serde_json::json!({
+121
View File
@@ -28,6 +28,62 @@ impl ApprovalRequirement {
}
}
/// Approval context for autonomous tool execution (routines, background jobs).
///
/// Interactive sessions don't use this type — they rely on session-level
/// auto-approve lists managed by the UI. This enum models only the autonomous
/// case where no interactive user is present.
#[derive(Debug, Clone)]
pub enum ApprovalContext {
/// Autonomous job with no interactive user. `UnlessAutoApproved` tools are
/// pre-approved. `Always` tools are blocked unless listed in `allowed_tools`.
Autonomous {
/// Tool names that are pre-authorized even for `Always` approval.
allowed_tools: std::collections::HashSet<String>,
},
}
impl ApprovalContext {
/// Create an autonomous context with no extra tool permissions.
pub fn autonomous() -> Self {
Self::Autonomous {
allowed_tools: std::collections::HashSet::new(),
}
}
/// Create an autonomous context with specific tools pre-authorized.
pub fn autonomous_with_tools(tools: impl IntoIterator<Item = String>) -> Self {
Self::Autonomous {
allowed_tools: tools.into_iter().collect(),
}
}
/// Check whether a tool invocation is blocked in this context.
pub fn is_blocked(&self, tool_name: &str, requirement: ApprovalRequirement) -> bool {
match self {
Self::Autonomous { allowed_tools } => match requirement {
ApprovalRequirement::Never => false,
ApprovalRequirement::UnlessAutoApproved => false,
ApprovalRequirement::Always => !allowed_tools.contains(tool_name),
},
}
}
/// Check whether a tool is blocked given an optional context.
///
/// When `None`, falls back to legacy behavior: all non-`Never` tools are blocked.
pub fn is_blocked_or_default(
context: &Option<Self>,
tool_name: &str,
requirement: ApprovalRequirement,
) -> bool {
match context {
Some(ctx) => ctx.is_blocked(tool_name, requirement),
None => requirement.is_required(),
}
}
}
/// Per-tool rate limit configuration for built-in tool invocations.
///
/// Controls how many times a tool can be invoked per user, per time window.
@@ -733,4 +789,69 @@ mod tests {
assert!(errors[0].contains("headers.items"));
assert!(errors[0].contains("\"missing_field\""));
}
#[test]
fn test_approval_context_autonomous_allows_unless_auto_approved() {
let ctx = ApprovalContext::autonomous();
assert!(!ctx.is_blocked("shell", ApprovalRequirement::Never));
assert!(!ctx.is_blocked("shell", ApprovalRequirement::UnlessAutoApproved));
assert!(ctx.is_blocked("shell", ApprovalRequirement::Always));
}
#[test]
fn test_approval_context_autonomous_with_tools_allows_always() {
let ctx =
ApprovalContext::autonomous_with_tools(["shell".to_string(), "message".to_string()]);
assert!(!ctx.is_blocked("shell", ApprovalRequirement::Always));
assert!(!ctx.is_blocked("message", ApprovalRequirement::Always));
assert!(ctx.is_blocked("http", ApprovalRequirement::Always));
}
#[test]
fn test_approval_context_never_is_not_blocked() {
let ctx = ApprovalContext::autonomous();
assert!(!ctx.is_blocked("any_tool", ApprovalRequirement::Never));
}
#[test]
fn test_is_blocked_or_default_with_none_uses_legacy() {
// None context: all non-Never tools are blocked
assert!(!ApprovalContext::is_blocked_or_default(
&None,
"any",
ApprovalRequirement::Never
));
assert!(ApprovalContext::is_blocked_or_default(
&None,
"any",
ApprovalRequirement::UnlessAutoApproved
));
assert!(ApprovalContext::is_blocked_or_default(
&None,
"any",
ApprovalRequirement::Always
));
}
#[test]
fn test_is_blocked_or_default_with_some_delegates() {
let ctx = Some(ApprovalContext::autonomous_with_tools(
["shell".to_string()],
));
assert!(!ApprovalContext::is_blocked_or_default(
&ctx,
"shell",
ApprovalRequirement::Always
));
assert!(ApprovalContext::is_blocked_or_default(
&ctx,
"other",
ApprovalRequirement::Always
));
assert!(!ApprovalContext::is_blocked_or_default(
&ctx,
"any",
ApprovalRequirement::UnlessAutoApproved
));
}
}