Skip to content

refactor: clean up client side code for creating schedule #6805

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lifeizhou-ap merged 11 commits into from
Feb 2, 2026
Merged

refactor: clean up client side code for creating schedule #6805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c283ff8
used server side recipeToYaml
lifeizhou-ap Jan 29, 2026
0a3073e
refactored create scheduler api
lifeizhou-ap Jan 29, 2026
ec55327
address copilot comments
lifeizhou-ap Jan 29, 2026
d9acf31
parse error message from server side correctly
lifeizhou-ap Jan 29, 2026
768ec8f
Merge branch 'main' into lifei/clean-up-schedule-client-side-function
lifeizhou-ap Jan 29, 2026
9716875
trim the schedule id/name
lifeizhou-ap Jan 29, 2026
42b23f3
more cleanup
lifeizhou-ap Jan 30, 2026
5433159
removed bot in messages
lifeizhou-ap Jan 30, 2026
1f93883
address pr comment
lifeizhou-ap Jan 30, 2026
8fb0fe1
added error handling
lifeizhou-ap Feb 2, 2026
8dd57a0
Merge branch 'main' into lifei/clean-up-schedule-client-side-function
lifeizhou-ap Feb 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 52 additions & 6 deletions crates/goose-server/src/routes/schedule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::sync::Arc;
use tokio::fs;

use axum::{
extract::{Path, Query, State},
Expand All @@ -9,13 +10,30 @@ use axum::{
use serde::{Deserialize, Serialize};

use crate::routes::errors::ErrorResponse;
use crate::routes::recipe_utils::validate_recipe;
use crate::state::AppState;
use goose::scheduler::ScheduledJob;
use goose::recipe::Recipe;
use goose::scheduler::{get_default_scheduled_recipes_dir, ScheduledJob};

fn validate_schedule_id(id: &str) -> Result<(), ErrorResponse> {
let is_valid = !id.is_empty()
&& id
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == ' ');

if !is_valid {
return Err(ErrorResponse::bad_request(
"Schedule name must use only alphanumeric characters, hyphens, underscores, or spaces"
Comment on lines +19 to +26
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_schedule_id allows leading/trailing spaces, which can create hard-to-reference schedule IDs (and awkward filenames like "foo .yaml"); consider trimming id before validation and/or rejecting IDs that start/end with whitespace.

Suggested change
let is_valid = !id.is_empty()
&& id
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == ' ');
if !is_valid {
return Err(ErrorResponse::bad_request(
"Schedule name must use only alphanumeric characters, hyphens, underscores, or spaces"
// Reject empty or all-whitespace IDs, and disallow leading/trailing whitespace.
let trimmed = id.trim();
let is_valid = !trimmed.is_empty()
&& trimmed.len() == id.len()
&& id
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == ' ');
if !is_valid {
return Err(ErrorResponse::bad_request(
"Schedule name must use only alphanumeric characters, hyphens, underscores, or spaces, and must not start or end with whitespace"

Copilot uses AI. Check for mistakes.
.to_string(),
));
}
Ok(())
}

#[derive(Deserialize, Serialize, utoipa::ToSchema)]
pub struct CreateScheduleRequest {
id: String,
recipe_source: String,
recipe: Recipe,
cron: String,
}

Expand Down Expand Up @@ -89,20 +107,47 @@ async fn create_schedule(
State(state): State<Arc<AppState>>,
Json(req): Json<CreateScheduleRequest>,
) -> Result<Json<ScheduledJob>, ErrorResponse> {
let scheduler = state.scheduler();
let id = req.id.trim().to_string();
validate_schedule_id(&id)?;

if req.recipe.check_for_security_warnings() {
return Err(ErrorResponse::bad_request(
"This recipe contains hidden characters that could be malicious. Please remove them before trying to save.".to_string(),
));
}
if let Err(err) = validate_recipe(&req.recipe) {
return Err(ErrorResponse {
message: err.message,
status: err.status,
});
}
let scheduled_recipes_dir = get_default_scheduled_recipes_dir().map_err(|e| {
ErrorResponse::internal(format!("Failed to get scheduled recipes directory: {}", e))
})?;

let recipe_path = scheduled_recipes_dir.join(format!("{}.yaml", id));
let yaml_content = req
.recipe
.to_yaml()
.map_err(|e| ErrorResponse::internal(format!("Failed to convert recipe to YAML: {}", e)))?;
fs::write(&recipe_path, yaml_content)
.await
.map_err(|e| ErrorResponse::internal(format!("Failed to save recipe file: {}", e)))?;
Comment on lines +133 to +135
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fs::write(&recipe_path, ...) will overwrite an existing scheduled recipe file (and can leave an orphan file) if add_scheduled_job later fails (e.g., JobIdExists or invalid cron); consider checking for an existing job/file first and/or writing to a temp file and cleaning it up on scheduler failure.

Copilot uses AI. Check for mistakes.

let job = ScheduledJob {
id: req.id,
source: req.recipe_source,
id,
source: recipe_path.to_string_lossy().into_owned(),
cron: req.cron,
last_run: None,
currently_running: false,
paused: false,
current_session_id: None,
process_start_time: None,
};

let scheduler = state.scheduler();
scheduler
.add_scheduled_job(job.clone(), true)
.add_scheduled_job(job.clone(), false)
.await
Comment on lines +124 to 151
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recipe YAML is written to {scheduled_recipes_dir}/{id}.yaml before add_scheduled_job checks for duplicate IDs, so a request with an existing id (or one that later fails cron validation) can overwrite an existing schedule’s recipe file or leave an orphaned file. Consider checking for JobIdExists before writing and/or writing to a temp file and only renaming after add_scheduled_job succeeds (and cleaning up the temp file on error).

Copilot uses AI. Check for mistakes.
.map_err(|e| match e {
goose::scheduler::SchedulerError::CronParseError(msg) => {
Expand All @@ -117,6 +162,7 @@ async fn create_schedule(
},
_ => ErrorResponse::internal(format!("Error creating schedule: {}", e)),
})?;

Ok(Json(job))
}

Expand Down
6 changes: 3 additions & 3 deletions ui/desktop/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3437,7 +3437,7 @@
"type": "object",
"required": [
"id",
"recipe_source",
"recipe",
"cron"
],
"properties": {
Expand All @@ -3447,8 +3447,8 @@
"id": {
"type": "string"
},
"recipe_source": {
"type": "string"
"recipe": {
"$ref": "#/components/schemas/Recipe"
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion ui/desktop/src/api/types.gen.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export type CreateRecipeResponse = {
export type CreateScheduleRequest = {
cron: string;
id: string;
recipe_source: string;
recipe: Recipe;
};

/**
Expand Down
51 changes: 1 addition & 50 deletions ui/desktop/src/components/recipes/ImportRecipeForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import { z } from 'zod';
import { Download } from 'lucide-react';
import { Button } from '../ui/button';
import { Input } from '../ui/input';
import { Recipe, decodeRecipe } from '../../recipe';
import { Recipe, parseDeeplink, parseRecipeFromFile } from '../../recipe';
import { toastSuccess, toastError } from '../../toasts';
import { useEscapeKey } from '../../hooks/useEscapeKey';
import { getRecipeJsonSchema } from '../../recipe/validation';
import { saveRecipe } from '../../recipe/recipe_management';
import { parseRecipe } from '../../api';
import { errorMessage } from '../../utils/conversionUtils';

interface ImportRecipeFormProps {
Expand Down Expand Up @@ -46,54 +45,6 @@ export default function ImportRecipeForm({ isOpen, onClose, onSuccess }: ImportR

useEscapeKey(isOpen, onClose);

const parseDeeplink = async (deeplink: string): Promise<Recipe | null> => {
try {
const cleanLink = deeplink.trim();

if (!cleanLink.startsWith('goose://recipe?config=')) {
throw new Error('Invalid deeplink format. Expected: goose://recipe?config=...');
}

const recipeEncoded = cleanLink.replace('goose://recipe?config=', '');

if (!recipeEncoded) {
throw new Error('No recipe configuration found in deeplink');
}
const recipe = await decodeRecipe(recipeEncoded);

if (!recipe.title || !recipe.description) {
throw new Error('Recipe is missing required fields (title, description)');
}

if (!recipe.instructions && !recipe.prompt) {
throw new Error('Recipe must have either instructions or prompt');
}

return recipe;
} catch (error) {
console.error('Failed to parse deeplink:', error);
return null;
}
};

const parseRecipeFromFile = async (fileContent: string): Promise<Recipe> => {
try {
let response = await parseRecipe({
body: {
content: fileContent,
},
throwOnError: true,
});
return response.data.recipe;
} catch (error) {
let error_message = 'unknown error';
if (typeof error === 'object' && error !== null && 'message' in error) {
error_message = error.message as string;
}
throw new Error(error_message);
}
};

const importRecipeForm = useForm({
defaultValues: {
deeplink: '',
Expand Down
Loading
Loading