fix(cron): persist allowed_tools for agent jobs (#3993)
Persist allowed_tools in cron_jobs table, threading it through CLI add/update and cron_add/cron_update tool APIs. Add regression coverage for store, tool, and CLI roundtrip paths. Fixups over original PR #3929: add allowed_tools to all_overdue_jobs SELECT (merge gap), resolve merge conflicts. Closes #3920 Supersedes #3929
This commit is contained in:
+47
-1
@@ -130,6 +130,11 @@ impl Tool for CronAddTool {
|
||||
"type": "string",
|
||||
"description": "Optional model override for agent jobs, e.g. 'x-ai/grok-4-1-fast'"
|
||||
},
|
||||
"allowed_tools": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Optional allowlist of tool names for agent jobs. When omitted, all tools remain available."
|
||||
},
|
||||
"delivery": {
|
||||
"type": "object",
|
||||
"description": "Optional delivery config to send job output to a channel after each run. When provided, all three of mode, channel, and to are expected.",
|
||||
@@ -288,6 +293,19 @@ impl Tool for CronAddTool {
|
||||
.get("model")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(str::to_string);
|
||||
let allowed_tools = match args.get("allowed_tools") {
|
||||
Some(v) => match serde_json::from_value::<Vec<String>>(v.clone()) {
|
||||
Ok(v) => Some(v),
|
||||
Err(e) => {
|
||||
return Ok(ToolResult {
|
||||
success: false,
|
||||
output: String::new(),
|
||||
error: Some(format!("Invalid allowed_tools: {e}")),
|
||||
});
|
||||
}
|
||||
},
|
||||
None => None,
|
||||
};
|
||||
|
||||
let delivery = match args.get("delivery") {
|
||||
Some(v) => match serde_json::from_value::<DeliveryConfig>(v.clone()) {
|
||||
@@ -316,6 +334,7 @@ impl Tool for CronAddTool {
|
||||
model,
|
||||
delivery,
|
||||
delete_after_run,
|
||||
allowed_tools,
|
||||
)
|
||||
}
|
||||
};
|
||||
@@ -329,7 +348,8 @@ impl Tool for CronAddTool {
|
||||
"job_type": job.job_type,
|
||||
"schedule": job.schedule,
|
||||
"next_run": job.next_run,
|
||||
"enabled": job.enabled
|
||||
"enabled": job.enabled,
|
||||
"allowed_tools": job.allowed_tools
|
||||
}))?,
|
||||
error: None,
|
||||
}),
|
||||
@@ -612,6 +632,32 @@ mod tests {
|
||||
.contains("Missing 'prompt'"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn agent_job_persists_allowed_tools() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cfg = test_config(&tmp).await;
|
||||
let tool = CronAddTool::new(cfg.clone(), test_security(&cfg));
|
||||
|
||||
let result = tool
|
||||
.execute(json!({
|
||||
"schedule": { "kind": "cron", "expr": "*/5 * * * *" },
|
||||
"job_type": "agent",
|
||||
"prompt": "check status",
|
||||
"allowed_tools": ["file_read", "web_search"]
|
||||
}))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(result.success, "{:?}", result.error);
|
||||
|
||||
let jobs = cron::list_jobs(&cfg).unwrap();
|
||||
assert_eq!(jobs.len(), 1);
|
||||
assert_eq!(
|
||||
jobs[0].allowed_tools,
|
||||
Some(vec!["file_read".into(), "web_search".into()])
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn delivery_schema_includes_matrix_channel() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
@@ -89,6 +89,11 @@ impl Tool for CronUpdateTool {
|
||||
"type": "string",
|
||||
"description": "Model override for agent jobs, e.g. 'x-ai/grok-4-1-fast'"
|
||||
},
|
||||
"allowed_tools": {
|
||||
"type": "array",
|
||||
"items": { "type": "string" },
|
||||
"description": "Optional replacement allowlist of tool names for agent jobs"
|
||||
},
|
||||
"session_target": {
|
||||
"type": "string",
|
||||
"enum": ["isolated", "main"],
|
||||
@@ -403,6 +408,7 @@ mod tests {
|
||||
"command",
|
||||
"prompt",
|
||||
"model",
|
||||
"allowed_tools",
|
||||
"session_target",
|
||||
"delete_after_run",
|
||||
"schedule",
|
||||
@@ -501,4 +507,40 @@ mod tests {
|
||||
.contains("Rate limit exceeded"));
|
||||
assert!(cron::get_job(&cfg, &job.id).unwrap().enabled);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn updates_agent_allowed_tools() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let cfg = test_config(&tmp).await;
|
||||
let job = cron::add_agent_job(
|
||||
&cfg,
|
||||
None,
|
||||
crate::cron::Schedule::Cron {
|
||||
expr: "*/5 * * * *".into(),
|
||||
tz: None,
|
||||
},
|
||||
"check status",
|
||||
crate::cron::SessionTarget::Isolated,
|
||||
None,
|
||||
None,
|
||||
false,
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
let tool = CronUpdateTool::new(cfg.clone(), test_security(&cfg));
|
||||
|
||||
let result = tool
|
||||
.execute(json!({
|
||||
"job_id": job.id,
|
||||
"patch": { "allowed_tools": ["file_read", "web_search"] }
|
||||
}))
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
assert!(result.success, "{:?}", result.error);
|
||||
assert_eq!(
|
||||
cron::get_job(&cfg, &job.id).unwrap().allowed_tools,
|
||||
Some(vec!["file_read".into(), "web_search".into()])
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user