Fix: exclude platform_schedule_tool in CLI (#6442)
This commit is contained in:
@@ -4,6 +4,15 @@ use goose::scheduler::{
|
||||
SchedulerError,
|
||||
};
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
async fn create_scheduler() -> Result<Arc<Scheduler>> {
|
||||
let storage_path =
|
||||
get_default_scheduler_storage_path().context("Failed to get scheduler storage path")?;
|
||||
Scheduler::new(storage_path)
|
||||
.await
|
||||
.context("Failed to initialize scheduler")
|
||||
}
|
||||
|
||||
fn validate_cron_expression(cron: &str) -> Result<()> {
|
||||
// Basic validation and helpful suggestions
|
||||
@@ -88,11 +97,7 @@ pub async fn handle_schedule_add(
|
||||
process_start_time: None,
|
||||
};
|
||||
|
||||
let scheduler_storage_path =
|
||||
get_default_scheduler_storage_path().context("Failed to get scheduler storage path")?;
|
||||
let scheduler = Scheduler::new(scheduler_storage_path)
|
||||
.await
|
||||
.context("Failed to initialize scheduler")?;
|
||||
let scheduler = create_scheduler().await?;
|
||||
|
||||
match scheduler.add_scheduled_job(job, true).await {
|
||||
Ok(_) => {
|
||||
@@ -134,11 +139,7 @@ pub async fn handle_schedule_add(
|
||||
}
|
||||
|
||||
pub async fn handle_schedule_list() -> Result<()> {
|
||||
let scheduler_storage_path =
|
||||
get_default_scheduler_storage_path().context("Failed to get scheduler storage path")?;
|
||||
let scheduler = Scheduler::new(scheduler_storage_path)
|
||||
.await
|
||||
.context("Failed to initialize scheduler")?;
|
||||
let scheduler = create_scheduler().await?;
|
||||
|
||||
let jobs = scheduler.list_scheduled_jobs().await;
|
||||
if jobs.is_empty() {
|
||||
@@ -169,11 +170,7 @@ pub async fn handle_schedule_list() -> Result<()> {
|
||||
}
|
||||
|
||||
pub async fn handle_schedule_remove(schedule_id: String) -> Result<()> {
|
||||
let scheduler_storage_path =
|
||||
get_default_scheduler_storage_path().context("Failed to get scheduler storage path")?;
|
||||
let scheduler = Scheduler::new(scheduler_storage_path)
|
||||
.await
|
||||
.context("Failed to initialize scheduler")?;
|
||||
let scheduler = create_scheduler().await?;
|
||||
|
||||
match scheduler.remove_scheduled_job(&schedule_id, true).await {
|
||||
Ok(_) => {
|
||||
@@ -196,11 +193,7 @@ pub async fn handle_schedule_remove(schedule_id: String) -> Result<()> {
|
||||
}
|
||||
|
||||
pub async fn handle_schedule_sessions(schedule_id: String, limit: Option<usize>) -> Result<()> {
|
||||
let scheduler_storage_path =
|
||||
get_default_scheduler_storage_path().context("Failed to get scheduler storage path")?;
|
||||
let scheduler = Scheduler::new(scheduler_storage_path)
|
||||
.await
|
||||
.context("Failed to initialize scheduler")?;
|
||||
let scheduler = create_scheduler().await?;
|
||||
|
||||
match scheduler.sessions(&schedule_id, limit.unwrap_or(50)).await {
|
||||
Ok(sessions) => {
|
||||
@@ -232,11 +225,7 @@ pub async fn handle_schedule_sessions(schedule_id: String, limit: Option<usize>)
|
||||
}
|
||||
|
||||
pub async fn handle_schedule_run_now(schedule_id: String) -> Result<()> {
|
||||
let scheduler_storage_path =
|
||||
get_default_scheduler_storage_path().context("Failed to get scheduler storage path")?;
|
||||
let scheduler = Scheduler::new(scheduler_storage_path)
|
||||
.await
|
||||
.context("Failed to initialize scheduler")?;
|
||||
let scheduler = create_scheduler().await?;
|
||||
|
||||
match scheduler.run_now(&schedule_id).await {
|
||||
Ok(session_id) => {
|
||||
|
||||
@@ -737,7 +737,9 @@ impl Agent {
|
||||
.unwrap_or_default();
|
||||
|
||||
let subagents_enabled = self.subagents_enabled().await;
|
||||
if extension_name.is_none() || extension_name.as_deref() == Some("platform") {
|
||||
if (extension_name.is_none() || extension_name.as_deref() == Some("platform"))
|
||||
&& self.scheduler_service.lock().await.is_some()
|
||||
{
|
||||
prefixed_tools.push(platform_tools::manage_schedule_tool());
|
||||
}
|
||||
|
||||
|
||||
@@ -7,18 +7,18 @@ pub fn manage_schedule_tool() -> Tool {
|
||||
Tool::new(
|
||||
PLATFORM_MANAGE_SCHEDULE_TOOL_NAME.to_string(),
|
||||
indoc! {r#"
|
||||
Manage scheduled recipe execution for this goose instance.
|
||||
|
||||
Manage goose's internal scheduled recipe execution.
|
||||
|
||||
Actions:
|
||||
- "list": List all scheduled jobs
|
||||
- "create": Create a new scheduled job from a recipe file
|
||||
- "run_now": Execute a scheduled job immediately
|
||||
- "pause": Pause a scheduled job
|
||||
- "unpause": Resume a paused job
|
||||
- "delete": Remove a scheduled job
|
||||
- "kill": Terminate a currently running job
|
||||
- "inspect": Get details about a running job
|
||||
- "sessions": List execution history for a job
|
||||
- "list": List all goose scheduled jobs
|
||||
- "create": Create a new goose scheduled job from a recipe file
|
||||
- "run_now": Execute a goose scheduled job immediately
|
||||
- "pause": Pause a goose scheduled job
|
||||
- "unpause": Resume a paused goose scheduled job
|
||||
- "delete": Remove a goose scheduled job
|
||||
- "kill": Terminate a currently running goose scheduled job
|
||||
- "inspect": Get details about a running goose scheduled job
|
||||
- "sessions": List execution history for a goose scheduled job
|
||||
- "session_content": Get the full content (messages) of a specific session
|
||||
"#}
|
||||
.to_string(),
|
||||
|
||||
@@ -437,7 +437,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn prepare_tools_sorts_and_includes_frontend_and_list_tools() -> anyhow::Result<()> {
|
||||
async fn prepare_tools_returns_sorted_tools_including_frontend() -> anyhow::Result<()> {
|
||||
let agent = crate::agents::Agent::new();
|
||||
|
||||
let session = SessionManager::create_session(
|
||||
@@ -481,9 +481,7 @@ mod tests {
|
||||
let (tools, _toolshim_tools, _system_prompt) =
|
||||
agent.prepare_tools_and_prompt(&working_dir).await?;
|
||||
|
||||
// Ensure both platform and frontend tools are present
|
||||
let names: Vec<String> = tools.iter().map(|t| t.name.clone().into_owned()).collect();
|
||||
assert!(names.iter().any(|n| n.starts_with("platform__")));
|
||||
assert!(names.iter().any(|n| n == "frontend__a_tool"));
|
||||
assert!(names.iter().any(|n| n == "frontend__z_tool"));
|
||||
|
||||
|
||||
@@ -20,16 +20,20 @@ impl Agent {
|
||||
arguments: serde_json::Value,
|
||||
_request_id: String,
|
||||
) -> ToolResult<Vec<Content>> {
|
||||
let scheduler = match self.scheduler_service.lock().await.as_ref() {
|
||||
Some(s) => s.clone(),
|
||||
None => {
|
||||
return Err(ErrorData::new(
|
||||
let scheduler = self
|
||||
.scheduler_service
|
||||
.lock()
|
||||
.await
|
||||
.as_ref()
|
||||
.ok_or_else(|| {
|
||||
ErrorData::new(
|
||||
ErrorCode::INTERNAL_ERROR,
|
||||
"Scheduler not available. This tool only works in server mode.".to_string(),
|
||||
"Scheduler service should be available when schedule tool is called"
|
||||
.to_string(),
|
||||
None,
|
||||
))
|
||||
}
|
||||
};
|
||||
)
|
||||
})?
|
||||
.clone();
|
||||
|
||||
let action = arguments
|
||||
.get("action")
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
use crate::agents::extension::PlatformExtensionContext;
|
||||
use crate::agents::Agent;
|
||||
use crate::config::paths::Paths;
|
||||
use crate::config::Config;
|
||||
use crate::scheduler::Scheduler;
|
||||
use crate::scheduler::{get_default_scheduler_storage_path, Scheduler};
|
||||
use crate::scheduler_trait::SchedulerTrait;
|
||||
use anyhow::Result;
|
||||
use lru::LruCache;
|
||||
@@ -34,7 +33,7 @@ impl AgentManager {
|
||||
}
|
||||
|
||||
async fn new(max_sessions: Option<usize>) -> Result<Self> {
|
||||
let schedule_file_path = Paths::data_dir().join("schedule.json");
|
||||
let schedule_file_path = get_default_scheduler_storage_path()?;
|
||||
|
||||
let scheduler = Scheduler::new(schedule_file_path).await?;
|
||||
|
||||
|
||||
@@ -31,7 +31,7 @@ type JobsMap = HashMap<String, (JobId, ScheduledJob)>;
|
||||
pub fn get_default_scheduler_storage_path() -> Result<PathBuf, io::Error> {
|
||||
let data_dir = Paths::data_dir();
|
||||
fs::create_dir_all(&data_dir)?;
|
||||
Ok(data_dir.join("schedules.json"))
|
||||
Ok(data_dir.join("schedule.json"))
|
||||
}
|
||||
|
||||
pub fn get_default_scheduled_recipes_dir() -> Result<PathBuf, SchedulerError> {
|
||||
@@ -395,7 +395,8 @@ impl Scheduler {
|
||||
Ok(data) => data,
|
||||
Err(e) => {
|
||||
tracing::error!(
|
||||
"Failed to read schedules.json: {}. Starting with empty schedule list.",
|
||||
"Failed to read {}: {}. Starting with empty schedule list.",
|
||||
self.storage_path.display(),
|
||||
e
|
||||
);
|
||||
return;
|
||||
@@ -409,7 +410,8 @@ impl Scheduler {
|
||||
Ok(jobs) => jobs,
|
||||
Err(e) => {
|
||||
tracing::error!(
|
||||
"Failed to parse schedules.json: {}. Starting with empty schedule list.",
|
||||
"Failed to parse {}: {}. Starting with empty schedule list.",
|
||||
self.storage_path.display(),
|
||||
e
|
||||
);
|
||||
return;
|
||||
@@ -926,7 +928,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_job_runs_on_schedule() {
|
||||
let temp_dir = tempdir().unwrap();
|
||||
let storage_path = temp_dir.path().join("schedules.json");
|
||||
let storage_path = temp_dir.path().join("schedule.json");
|
||||
let recipe_path = create_test_recipe(temp_dir.path(), "scheduled_job");
|
||||
let scheduler = Scheduler::new(storage_path).await.unwrap();
|
||||
|
||||
@@ -951,7 +953,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_paused_job_does_not_run() {
|
||||
let temp_dir = tempdir().unwrap();
|
||||
let storage_path = temp_dir.path().join("schedules.json");
|
||||
let storage_path = temp_dir.path().join("schedule.json");
|
||||
let recipe_path = create_test_recipe(temp_dir.path(), "paused_job");
|
||||
let scheduler = Scheduler::new(storage_path).await.unwrap();
|
||||
|
||||
|
||||
@@ -114,12 +114,6 @@ pub async fn generate_diagnostics(session_id: &str) -> anyhow::Result<Vec<u8>> {
|
||||
zip.write_all(&fs::read(&schedule_json)?)?;
|
||||
}
|
||||
|
||||
let schedules_json = data_dir.join("schedules.json");
|
||||
if schedules_json.exists() {
|
||||
zip.start_file("schedules.json", options)?;
|
||||
zip.write_all(&fs::read(&schedules_json)?)?;
|
||||
}
|
||||
|
||||
let scheduled_recipes_dir = data_dir.join("scheduled_recipes");
|
||||
if scheduled_recipes_dir.exists() && scheduled_recipes_dir.is_dir() {
|
||||
for entry in fs::read_dir(&scheduled_recipes_dir)? {
|
||||
|
||||
@@ -130,25 +130,26 @@ mod tests {
|
||||
.description
|
||||
.clone()
|
||||
.unwrap_or_default()
|
||||
.contains("Manage scheduled recipe execution"));
|
||||
.contains("Manage goose's internal scheduled recipe execution"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_schedule_management_tool_no_scheduler() {
|
||||
async fn test_no_schedule_management_tool_without_scheduler() {
|
||||
let agent = Agent::new();
|
||||
// Don't set scheduler - test that the tool still appears in the list
|
||||
// but would fail if actually called (which we can't test directly through public API)
|
||||
|
||||
let tools = agent.list_tools(None).await;
|
||||
let schedule_tool = tools
|
||||
.iter()
|
||||
.find(|tool| tool.name == PLATFORM_MANAGE_SCHEDULE_TOOL_NAME);
|
||||
assert!(schedule_tool.is_some());
|
||||
assert!(schedule_tool.is_none());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_schedule_management_tool_in_platform_tools() {
|
||||
let agent = Agent::new();
|
||||
let mock_scheduler = Arc::new(MockScheduler::new());
|
||||
agent.set_scheduler(mock_scheduler.clone()).await;
|
||||
|
||||
let tools = agent.list_tools(Some("platform".to_string())).await;
|
||||
|
||||
// Check that the schedule management tool is included in platform tools
|
||||
@@ -162,7 +163,7 @@ mod tests {
|
||||
.description
|
||||
.clone()
|
||||
.unwrap_or_default()
|
||||
.contains("Manage scheduled recipe execution"));
|
||||
.contains("Manage goose's internal scheduled recipe execution"));
|
||||
|
||||
// Verify the tool has the expected actions in its schema
|
||||
if let Some(properties) = tool.input_schema.get("properties") {
|
||||
@@ -188,6 +189,9 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_schedule_management_tool_schema_validation() {
|
||||
let agent = Agent::new();
|
||||
let mock_scheduler = Arc::new(MockScheduler::new());
|
||||
agent.set_scheduler(mock_scheduler.clone()).await;
|
||||
|
||||
let tools = agent.list_tools(None).await;
|
||||
let schedule_tool = tools
|
||||
.iter()
|
||||
@@ -460,7 +464,7 @@ mod tests {
|
||||
config: ExtensionConfig::Platform {
|
||||
name: "todo".to_string(),
|
||||
description:
|
||||
"Enable a todo list for Goose so it can keep track of what it is doing"
|
||||
"Enable a todo list for goose so it can keep track of what it is doing"
|
||||
.to_string(),
|
||||
bundled: Some(true),
|
||||
available_tools: vec![],
|
||||
|
||||
@@ -164,8 +164,10 @@ export GOOSE_AUTO_COMPACT_THRESHOLD=0.005
|
||||
|
||||
OUTPUT=$(mktemp)
|
||||
|
||||
echo "Step 1: Creating session with first message..."
|
||||
(cd "$TESTDIR" && "$GOOSE_BIN" run --text "hello" 2>&1) | tee "$OUTPUT"
|
||||
LONG_RESPONSE_PROMPT="Count from 1 to 200, one number per line."
|
||||
|
||||
echo "Step 1: Creating session with first message (generating tokens for threshold)..."
|
||||
(cd "$TESTDIR" && "$GOOSE_BIN" run --text "$LONG_RESPONSE_PROMPT" 2>&1) | tee "$OUTPUT"
|
||||
|
||||
if ! command -v jq &> /dev/null; then
|
||||
echo "✗ FAILED: jq is required for this test"
|
||||
|
||||
Reference in New Issue
Block a user