From 7c0ebf9734f940aa2a866ae418f9d5ce5cf71ec0 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Mon, 6 Oct 2025 21:23:12 +1100 Subject: [PATCH 01/22] applied server side call save recipe --- crates/goose-server/src/routes/recipe.rs | 31 ++++++++----- .../goose-server/src/routes/recipe_utils.rs | 34 ++++++++++++++ crates/goose-server/src/state.rs | 10 +---- ui/desktop/src/components/BaseChat.tsx | 2 + ui/desktop/src/components/ChatInput.tsx | 3 ++ .../recipes/CreateEditRecipeModal.tsx | 28 +++++++----- .../recipes/CreateRecipeFromSessionModal.tsx | 11 ++--- .../components/recipes/ImportRecipeForm.tsx | 45 ++----------------- .../src/components/recipes/RecipesView.tsx | 9 ++-- .../recipes/shared/SaveRecipeDialog.tsx | 15 +++---- .../models/bottom_bar/ModelsBottomBar.tsx | 4 ++ ui/desktop/src/hooks/useAgent.ts | 1 - ui/desktop/src/hooks/useRecipeManager.ts | 4 ++ ui/desktop/src/main.ts | 38 +++++++++++----- ui/desktop/src/preload.ts | 17 +++++-- ui/desktop/src/recipe/recipe_management.ts | 24 ++++++++++ 16 files changed, 168 insertions(+), 108 deletions(-) create mode 100644 ui/desktop/src/recipe/recipe_management.ts diff --git a/crates/goose-server/src/routes/recipe.rs b/crates/goose-server/src/routes/recipe.rs index 26c79c8b543f..6b2530122203 100644 --- a/crates/goose-server/src/routes/recipe.rs +++ b/crates/goose-server/src/routes/recipe.rs @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; use utoipa::ToSchema; use crate::routes::errors::ErrorResponse; -use crate::routes::recipe_utils::get_all_recipes_manifests; +use crate::routes::recipe_utils::{get_all_recipes_manifests, save_recipe_file_hash_map, find_recipe_file_path_by_id}; use crate::state::AppState; #[derive(Debug, Deserialize, ToSchema)] @@ -250,7 +250,6 @@ async fn scan_recipe( tag = "Recipe Management" )] async fn list_recipes( - State(state): State>, ) -> Result, StatusCode> { let recipe_manifest_with_paths = get_all_recipes_manifests().unwrap_or_default(); let mut recipe_file_hash_map = HashMap::new(); @@ -268,7 +267,10 @@ async fn list_recipes( } }) .collect::>(); - state.set_recipe_file_hash_map(recipe_file_hash_map).await; + if let Err(e) = save_recipe_file_hash_map(&recipe_file_hash_map) { + tracing::error!("Failed to save recipe file hash map to temp file: {}", e); + return Err(StatusCode::INTERNAL_SERVER_ERROR); + } Ok(Json(ListRecipeResponse { recipe_manifest_responses, @@ -288,13 +290,14 @@ async fn list_recipes( tag = "Recipe Management" )] async fn delete_recipe( - State(state): State>, Json(request): Json, ) -> StatusCode { - let recipe_file_hash_map = state.recipe_file_hash_map.lock().await; - let file_path = match recipe_file_hash_map.get(&request.id) { - Some(path) => path, - None => return StatusCode::NOT_FOUND, + let file_path = match find_recipe_file_path_by_id(&request.id) { + Ok(path) => path, + Err(e) => { + tracing::error!("Failed to find recipe file path: {}", e); + return StatusCode::NOT_FOUND; + } }; if fs::remove_file(file_path).is_err() { @@ -316,11 +319,19 @@ async fn delete_recipe( tag = "Recipe Management" )] async fn save_recipe( - State(state): State>, Json(request): Json, ) -> Result { let file_path = match request.id { - Some(id) => state.recipe_file_hash_map.lock().await.get(&id).cloned(), + Some(id) => match find_recipe_file_path_by_id(&id) { + Ok(path) => Some(path), + Err(e) => { + tracing::error!("Failed to find recipe file path: {}", e); + return Err(ErrorResponse { + message: format!("Recipe not found: {}", e), + status: StatusCode::NOT_FOUND, + }); + } + }, None => None, }; diff --git a/crates/goose-server/src/routes/recipe_utils.rs b/crates/goose-server/src/routes/recipe_utils.rs index 1a76d947a661..0ce7d04e0353 100644 --- a/crates/goose-server/src/routes/recipe_utils.rs +++ b/crates/goose-server/src/routes/recipe_utils.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::fs; use std::hash::DefaultHasher; use std::hash::{Hash, Hasher}; @@ -58,6 +59,39 @@ pub fn get_all_recipes_manifests() -> Result> { Ok(recipe_manifests_with_path) } +fn get_recipe_temp_file_path() -> std::path::PathBuf { + std::env::temp_dir().join("goose_recipe_file_map.json") +} + +pub fn save_recipe_file_hash_map(hash_map: &HashMap) -> Result<()> { + let temp_path = get_recipe_temp_file_path(); + let json_data = serde_json::to_string(hash_map) + .map_err(|e| anyhow::anyhow!("Failed to serialize hash map: {}", e))?; + fs::write(temp_path, json_data) + .map_err(|e| anyhow::anyhow!("Failed to write recipe id to file: {}", e))?; + Ok(()) +} + +fn load_recipe_file_hash_map() -> Result> { + let temp_path = get_recipe_temp_file_path(); + if !temp_path.exists() { + return Ok(HashMap::new()); + } + let json_data = fs::read_to_string(temp_path) + .map_err(|e| anyhow::anyhow!("Failed to read recipe id to file: {}", e))?; + let hash_map = serde_json::from_str(&json_data) + .map_err(|e| anyhow::anyhow!("Failed to deserialize hash map: {}", e))?; + Ok(hash_map) +} + +pub fn find_recipe_file_path_by_id(recipe_id: &str) -> Result { + let recipe_file_hash_map = load_recipe_file_hash_map()?; + recipe_file_hash_map + .get(recipe_id) + .cloned() + .ok_or_else(|| anyhow::anyhow!("Recipe not found with id: {}", recipe_id)) +} + // this is a temporary struct to deserilize the UI recipe files. should not be used for other purposes. #[derive(Serialize, Deserialize, Debug, Clone, ToSchema)] struct RecipeManifestMetadata { diff --git a/crates/goose-server/src/state.rs b/crates/goose-server/src/state.rs index b8b916bf9102..8be979922954 100644 --- a/crates/goose-server/src/state.rs +++ b/crates/goose-server/src/state.rs @@ -1,15 +1,13 @@ use axum::http::StatusCode; use goose::execution::manager::AgentManager; use goose::scheduler_trait::SchedulerTrait; -use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; +use std::collections::HashSet; use std::sync::atomic::AtomicUsize; use std::sync::Arc; use tokio::sync::Mutex; #[derive(Clone)] pub struct AppState { pub(crate) agent_manager: Arc, - pub recipe_file_hash_map: Arc>>, pub session_counter: Arc, /// Tracks sessions that have already emitted recipe telemetry to prevent double counting. recipe_session_tracker: Arc>>, @@ -20,7 +18,6 @@ impl AppState { let agent_manager = AgentManager::instance().await?; Ok(Arc::new(Self { agent_manager, - recipe_file_hash_map: Arc::new(Mutex::new(HashMap::new())), session_counter: Arc::new(AtomicUsize::new(0)), recipe_session_tracker: Arc::new(Mutex::new(HashSet::new())), })) @@ -30,11 +27,6 @@ impl AppState { self.agent_manager.scheduler().await } - pub async fn set_recipe_file_hash_map(&self, hash_map: HashMap) { - let mut map = self.recipe_file_hash_map.lock().await; - *map = hash_map; - } - pub async fn mark_recipe_run_if_absent(&self, session_id: &str) -> bool { let mut sessions = self.recipe_session_tracker.lock().await; if sessions.contains(session_id) { diff --git a/ui/desktop/src/components/BaseChat.tsx b/ui/desktop/src/components/BaseChat.tsx index 07231583ab38..34fc7ca872c9 100644 --- a/ui/desktop/src/components/BaseChat.tsx +++ b/ui/desktop/src/components/BaseChat.tsx @@ -157,6 +157,7 @@ function BaseChatContent({ // Use shared recipe manager const { recipe, + recipeId, recipeParameters, filteredParameters, initialPrompt, @@ -478,6 +479,7 @@ function BaseChatContent({ sessionCosts={sessionCosts} setIsGoosehintsModalOpen={setIsGoosehintsModalOpen} recipe={recipe} + recipeId={recipeId} recipeAccepted={recipeAccepted} initialPrompt={initialPrompt} toolCount={toolCount || 0} diff --git a/ui/desktop/src/components/ChatInput.tsx b/ui/desktop/src/components/ChatInput.tsx index 9d2d34722a39..1daa742c11ec 100644 --- a/ui/desktop/src/components/ChatInput.tsx +++ b/ui/desktop/src/components/ChatInput.tsx @@ -82,6 +82,7 @@ interface ChatInputProps { setIsGoosehintsModalOpen?: (isOpen: boolean) => void; disableAnimation?: boolean; recipe?: Recipe | null; + recipeId?: string | null; recipeAccepted?: boolean; initialPrompt?: string; toolCount: number; @@ -109,6 +110,7 @@ export default function ChatInput({ sessionCosts, setIsGoosehintsModalOpen, recipe, + recipeId, recipeAccepted, initialPrompt, toolCount, @@ -1625,6 +1627,7 @@ export default function ChatInput({ setView={setView} alerts={alerts} recipe={recipe} + recipeId={recipeId} hasMessages={messages.length > 0} /> diff --git a/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx b/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx index d044d044866c..35f3d60e04c3 100644 --- a/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx +++ b/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx @@ -11,8 +11,8 @@ import { Button } from '../ui/button'; import { RecipeFormFields } from './shared/RecipeFormFields'; import { RecipeFormData } from './shared/recipeFormSchema'; -import { saveRecipe, generateRecipeFilename } from '../../recipe/recipeStorage'; import { toastSuccess, toastError } from '../../toasts'; +import { saveRecipe } from '../../recipe/recipe_management'; interface CreateEditRecipeModalProps { isOpen: boolean; @@ -20,6 +20,7 @@ interface CreateEditRecipeModalProps { recipe?: Recipe; recipeName?: string; isCreateMode?: boolean; + recipeId?: string | null; } export default function CreateEditRecipeModal({ @@ -28,6 +29,7 @@ export default function CreateEditRecipeModal({ recipe, recipeName: initialRecipeName, isCreateMode = false, + recipeId, }: CreateEditRecipeModalProps) { const { getExtensions } = useConfig(); @@ -312,10 +314,7 @@ export default function CreateEditRecipeModal({ try { const recipe = getCurrentRecipe(); - await saveRecipe(recipe, { - name: (recipeName || '').trim(), - global: global, - }); + await saveRecipe(recipe, global, recipeId); onClose(true); @@ -348,18 +347,22 @@ export default function CreateEditRecipeModal({ setIsSaving(true); try { const recipe = getCurrentRecipe(); - const recipeName = generateRecipeFilename(recipe); - await saveRecipe(recipe, { - name: recipeName, - global: true, - }); + await saveRecipe(recipe, true, recipeId); // Close modal first onClose(true); // Open recipe in a new window instead of navigating in the same window - window.electron.createChatWindow(undefined, undefined, undefined, undefined, recipe); + window.electron.createChatWindow( + undefined, + undefined, + undefined, + undefined, + recipe, + undefined, + recipeId ?? undefined + ); toastSuccess({ title: recipeName, @@ -509,7 +512,8 @@ export default function CreateEditRecipeModal({ undefined, undefined, undefined, - 'schedules' + 'schedules', + undefined ); // Store the deep link in localStorage for the schedules view to pick up localStorage.setItem('pendingScheduleDeepLink', deepLink); diff --git a/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx b/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx index 5cf5f357497d..79abe7e40932 100644 --- a/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx +++ b/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx @@ -9,7 +9,7 @@ import { RecipeFormData } from './shared/recipeFormSchema'; import { createRecipe } from '../../api/sdk.gen'; import { RecipeParameter } from './shared/recipeFormSchema'; import { toastError } from '../../toasts'; -import { generateRecipeFilename } from '../../recipe/recipeStorage'; +import { saveRecipe } from '../../recipe/recipe_management'; interface CreateRecipeFromSessionModalProps { isOpen: boolean; @@ -91,7 +91,6 @@ export default function CreateRecipeFromSessionModal({ form.setFieldValue('instructions', recipe.instructions || ''); form.setFieldValue('activities', recipe.activities || []); form.setFieldValue('parameters', recipe.parameters || []); - form.setFieldValue('recipeName', generateRecipeFilename(recipe)); if (recipe.response?.json_schema) { form.setFieldValue( @@ -184,12 +183,8 @@ export default function CreateRecipeFromSessionModal({ extensions: [], // Will be populated based on current extensions }; - const { saveRecipe } = await import('../../recipe/recipeStorage'); - await saveRecipe(recipe, { - name: formData.recipeName || formData.title, - title: formData.title, - global: formData.global, - }); + // const { saveRecipe } = await import('../../recipe/recipeStorage'); + await saveRecipe(recipe, formData.global, null); onRecipeCreated?.(recipe); onClose(); diff --git a/ui/desktop/src/components/recipes/ImportRecipeForm.tsx b/ui/desktop/src/components/recipes/ImportRecipeForm.tsx index 9aaec3656219..8dec37f46417 100644 --- a/ui/desktop/src/components/recipes/ImportRecipeForm.tsx +++ b/ui/desktop/src/components/recipes/ImportRecipeForm.tsx @@ -5,17 +5,12 @@ import { Download } from 'lucide-react'; import { Button } from '../ui/button'; import { Input } from '../ui/input'; import { Recipe, decodeRecipe } from '../../recipe'; -import { saveRecipe } from '../../recipe/recipeStorage'; import * as yaml from 'yaml'; import { toastSuccess, toastError } from '../../toasts'; import { useEscapeKey } from '../../hooks/useEscapeKey'; import { RecipeTitleField } from './shared/RecipeTitleField'; -import { listSavedRecipes } from '../../recipe/recipeStorage'; -import { - validateRecipe, - getValidationErrorMessages, - getRecipeJsonSchema, -} from '../../recipe/validation'; +import { getRecipeJsonSchema } from '../../recipe/validation'; +import { saveRecipe } from '../../recipe/recipe_management'; interface ImportRecipeFormProps { isOpen: boolean; @@ -117,25 +112,6 @@ export default function ImportRecipeForm({ isOpen, onClose, onSuccess }: ImportR return recipe as Recipe; }; - const validateTitleUniqueness = async (title: string): Promise => { - if (!title.trim()) return undefined; - - try { - const existingRecipes = await listSavedRecipes(); - const titleExists = existingRecipes.some( - (recipe) => recipe.recipe.title?.toLowerCase() === title.toLowerCase() - ); - - if (titleExists) { - return `A recipe with the same title already exists`; - } - } catch (error) { - console.warn('Failed to validate title uniqueness:', error); - } - - return undefined; - }; - const importRecipeForm = useForm({ defaultValues: { deeplink: '', @@ -165,22 +141,7 @@ export default function ImportRecipeForm({ isOpen, onClose, onSuccess }: ImportR recipe.title = value.recipeTitle.trim(); - const titleValidationError = await validateTitleUniqueness(value.recipeTitle.trim()); - if (titleValidationError) { - throw new Error(titleValidationError); - } - - const validationResult = validateRecipe(recipe); - if (!validationResult.success) { - const errorMessages = getValidationErrorMessages(validationResult.errors); - throw new Error(`Recipe validation failed: ${errorMessages.join(', ')}`); - } - - await saveRecipe(recipe, { - name: '', - title: value.recipeTitle.trim(), - global: value.global, - }); + await saveRecipe(recipe, value.global, null); // Reset dialog state importRecipeForm.reset({ diff --git a/ui/desktop/src/components/recipes/RecipesView.tsx b/ui/desktop/src/components/recipes/RecipesView.tsx index fa2a4a804b98..cbf37287b2ca 100644 --- a/ui/desktop/src/components/recipes/RecipesView.tsx +++ b/ui/desktop/src/components/recipes/RecipesView.tsx @@ -65,8 +65,9 @@ export default function RecipesView() { } }; - const handleLoadRecipe = async (recipe: Recipe) => { + const handleLoadRecipe = async (recipe: Recipe, recipeId: string) => { try { + console.log('====== RecipesView handleLoadRecipe recipeId:', recipeId); // onLoadRecipe is not working for loading recipes. It looks correct // but the instructions are not flowing through to the server. // Needs a fix but commenting out to get prod back up and running. @@ -82,7 +83,8 @@ export default function RecipesView() { undefined, // version undefined, // resumeSessionId recipe, // recipe config - undefined // view type + undefined, // view type, + recipeId // recipe id ); // } } catch (err) { @@ -174,7 +176,7 @@ export default function RecipesView() { - - {showSaveAndRun && ( - - )} - - - - ); -} diff --git a/ui/desktop/src/components/recipes/shared/__tests__/RecipeFormFields.test.tsx b/ui/desktop/src/components/recipes/shared/__tests__/RecipeFormFields.test.tsx index f46c507074a9..20dedcd8d8e1 100644 --- a/ui/desktop/src/components/recipes/shared/__tests__/RecipeFormFields.test.tsx +++ b/ui/desktop/src/components/recipes/shared/__tests__/RecipeFormFields.test.tsx @@ -16,8 +16,6 @@ describe('RecipeFormFields', () => { activities: [], parameters: [], jsonSchema: '', - recipeName: '', - global: true, ...initialValues, }; @@ -129,19 +127,6 @@ describe('RecipeFormFields', () => { }); }); - describe('Always Visible Fields', () => { - it('always shows recipe name field', () => { - render(); - expect(screen.getByText('Recipe Name')).toBeInTheDocument(); - }); - - it('always shows save location field', () => { - render(); - expect(screen.getByText('Save Location')).toBeInTheDocument(); - expect(screen.getByText('Global - Available across all Goose sessions')).toBeInTheDocument(); - }); - }); - describe('Pre-filled Values', () => { it('displays pre-filled form values', () => { const initialValues: Partial = { @@ -262,8 +247,6 @@ describe('RecipeFormFields', () => { activities: [], parameters: [], jsonSchema: '', - recipeName: '', - global: true, } as RecipeFormData, onSubmit: async ({ value }) => { console.log('Form submitted:', value); @@ -355,8 +338,6 @@ describe('RecipeFormFields', () => { }, ], jsonSchema: '', - recipeName: '', - global: true, } as RecipeFormData, onSubmit: async ({ value }) => { console.log('Form submitted:', value); @@ -522,8 +503,6 @@ describe('RecipeFormFields', () => { }, ], jsonSchema: '', - recipeName: '', - global: true, } as RecipeFormData, onSubmit: async ({ value }) => { console.log('Form submitted:', value); @@ -597,8 +576,6 @@ describe('RecipeFormFields', () => { }, ], jsonSchema: '', - recipeName: '', - global: true, } as RecipeFormData, onSubmit: async ({ value }) => { console.log('Form submitted:', value); diff --git a/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts b/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts index 34255ec44fb7..2f0fcedfe6c2 100644 --- a/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts +++ b/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts @@ -17,8 +17,6 @@ describe('recipeFormSchema', () => { }, ], jsonSchema: '{"type": "object"}', - recipeName: 'test_recipe', - global: true, }; describe('Zod Schema Validation', () => { @@ -177,31 +175,6 @@ describe('recipeFormSchema', () => { }); }); - describe('Recipe Name Validation', () => { - it('allows empty recipe name', () => { - const validData = { ...validFormData, recipeName: '' }; - const result = recipeFormSchema.safeParse(validData); - expect(result.success).toBe(true); - }); - - it('allows undefined recipe name', () => { - const validData = { ...validFormData, recipeName: undefined }; - const result = recipeFormSchema.safeParse(validData); - expect(result.success).toBe(true); - }); - - it('rejects invalid recipe name characters', () => { - // The regex /^[^<>:"/\\|?*]+$/ rejects these specific characters - const invalidData = { ...validFormData, recipeName: 'invalid issue.path.includes('recipeName')); - expect(nameError?.message).toContain('invalid characters'); - } - }); - }); - describe('Parameter Validation', () => { it('validates parameters with all required fields', () => { const validData = { @@ -305,20 +278,6 @@ describe('recipeFormSchema', () => { }); }); - describe('Global Field Validation', () => { - it('validates global field as boolean true', () => { - const validData = { ...validFormData, global: true }; - const result = recipeFormSchema.safeParse(validData); - expect(result.success).toBe(true); - }); - - it('validates global field as boolean false', () => { - const validData = { ...validFormData, global: false }; - const result = recipeFormSchema.safeParse(validData); - expect(result.success).toBe(true); - }); - }); - describe('Multiple Validation Errors', () => { it('handles multiple validation errors', () => { const invalidData = { diff --git a/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts b/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts index 6de1c423ef57..99a680bef76b 100644 --- a/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts +++ b/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts @@ -52,16 +52,6 @@ export const recipeFormSchema = z.object({ return false; } }, 'Invalid JSON schema format'), - - recipeName: z - .string() - .optional() - .refine((name) => { - if (!name || !name.trim()) return true; - return /^[^<>:"/\\|?*]+$/.test(name.trim()); - }, 'Recipe name contains invalid characters (< > : " / \\ | ? *)'), - - global: z.boolean().default(true), }); export type RecipeFormData = z.infer; diff --git a/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx b/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx index 8117466eb5da..e9a4c4f6c9c7 100644 --- a/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx +++ b/ui/desktop/src/components/settings/models/bottom_bar/ModelsBottomBar.tsx @@ -1,4 +1,4 @@ -import { Sliders, ChefHat, Bot, Eye, Save } from 'lucide-react'; +import { Sliders, ChefHat, Bot, Eye } from 'lucide-react'; import React, { useEffect, useState } from 'react'; import { useModelAndProvider } from '../../../ModelAndProviderContext'; import { SwitchModelModal } from '../subcomponents/SwitchModelModal'; @@ -17,9 +17,7 @@ import { getProviderMetadata } from '../modelInterface'; import { Alert } from '../../../alerts'; import BottomMenuAlertPopover from '../../../bottom_menu/BottomMenuAlertPopover'; import { Recipe } from '../../../../recipe'; -import { generateRecipeFilename } from '../../../../recipe/recipeStorage'; import CreateEditRecipeModal from '../../../recipes/CreateEditRecipeModal'; -import SaveRecipeDialog from '../../../recipes/shared/SaveRecipeDialog'; interface ModelsBottomBarProps { sessionId: string | null; @@ -56,9 +54,6 @@ export default function ModelsBottomBar({ const [isLeadWorkerActive, setIsLeadWorkerActive] = useState(false); const [providerDefaultModel, setProviderDefaultModel] = useState(null); - // Save recipe dialog state - const [showSaveDialog, setShowSaveDialog] = useState(false); - // View recipe modal state const [showViewRecipeModal, setShowViewRecipeModal] = useState(false); @@ -176,13 +171,6 @@ export default function ModelsBottomBar({ } }; - // Handle save recipe - show save dialog - const handleSaveRecipeClick = () => { - if (recipe) { - setShowSaveDialog(true); - } - }; - return (
@@ -221,10 +209,6 @@ export default function ModelsBottomBar({ View/Edit Recipe - - Save Recipe - - )} @@ -258,16 +242,6 @@ export default function ModelsBottomBar({ ) : null} - {/* Save Recipe Dialog */} - {showSaveDialog && recipe && ( - setShowSaveDialog(false)} - recipe={recipe} - recipeId={recipeId} - /> - )} - {/* View Recipe Modal */} {/* todo: we don't have the actual recipe name when in chat only in recipes list view so we generate it for now */} {recipe && ( @@ -276,7 +250,6 @@ export default function ModelsBottomBar({ onClose={() => setShowViewRecipeModal(false)} recipe={recipe} recipeId={recipeId} - recipeName={generateRecipeFilename(recipe)} /> )}
From 53396ef19808cfddcfb8489e036fcac451831f66 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 16:36:43 +1100 Subject: [PATCH 08/22] removed unused functions --- ui/desktop/src/recipe/recipeStorage.ts | 110 +----- ui/desktop/src/recipe/validation.test.ts | 443 +---------------------- ui/desktop/src/recipe/validation.ts | 209 +---------- 3 files changed, 3 insertions(+), 759 deletions(-) diff --git a/ui/desktop/src/recipe/recipeStorage.ts b/ui/desktop/src/recipe/recipeStorage.ts index c012c5dc9bd1..4483967f64d8 100644 --- a/ui/desktop/src/recipe/recipeStorage.ts +++ b/ui/desktop/src/recipe/recipeStorage.ts @@ -1,14 +1,7 @@ import { listRecipes, RecipeManifestResponse } from '../api'; import { Recipe } from './index'; -import * as yaml from 'yaml'; -import { validateRecipe, getValidationErrorMessages } from './validation'; - -export interface SaveRecipeOptions { - name: string; - title?: string; - global?: boolean; // true for global (~/.config/goose/recipes/), false for project-specific (.goose/recipes/) -} +// TODO: Lifei Remove this export interface SavedRecipe { name: string; recipe: Recipe; @@ -18,13 +11,6 @@ export interface SavedRecipe { filename: string; // The actual filename used } -/** - * Sanitize a recipe name to be safe for use as a filename - */ -function sanitizeRecipeName(name: string): string { - return name.replace(/[^a-zA-Z0-9-_\s]/g, '').trim(); -} - /** * Parse a lastModified value that could be a string or Date */ @@ -45,84 +31,6 @@ export function getStorageDirectory(isGlobal: boolean): string { } } -/** - * Get the file path for a recipe based on its name - */ -function getRecipeFilePath(recipeName: string, isGlobal: boolean): string { - const dir = getStorageDirectory(isGlobal); - return `${dir}/${recipeName}.yaml`; -} - -/** - * Save recipe to file - */ -async function saveRecipeToFile(recipe: SavedRecipe): Promise { - const filePath = getRecipeFilePath(recipe.name, recipe.isGlobal); - - // Ensure directory exists - const dirPath = getStorageDirectory(recipe.isGlobal); - await window.electron.ensureDirectory(dirPath); - - // Convert to YAML and save - const yamlContent = yaml.stringify(recipe); - return await window.electron.writeFile(filePath, yamlContent); -} -/** - * Save a recipe to a file using IPC. - */ -export async function saveRecipe(recipe: Recipe, options: SaveRecipeOptions): Promise { - const { name, title, global = true } = options; - - let sanitizedName: string; - - if (title) { - recipe.title = title.trim(); - sanitizedName = generateRecipeFilename(recipe); - if (!sanitizedName) { - throw new Error('Invalid recipe title - cannot generate filename'); - } - } else { - // This branch should now be considered deprecated and will be removed once the same functionality - // is incorporated in CreateRecipeForm - sanitizedName = sanitizeRecipeName(name); - if (!sanitizedName) { - throw new Error('Invalid recipe name'); - } - } - - const validationResult = validateRecipe(recipe); - if (!validationResult.success) { - const errorMessages = getValidationErrorMessages(validationResult.errors); - throw new Error(`Recipe validation failed: ${errorMessages.join(', ')}`); - } - - try { - // Create saved recipe object - const savedRecipe: SavedRecipe = { - name: sanitizedName, - filename: sanitizedName, - recipe: recipe, - isGlobal: global, - lastModified: new Date(), - isArchived: false, - }; - - // Save to file - const success = await saveRecipeToFile(savedRecipe); - - if (!success) { - throw new Error('Failed to save recipe file'); - } - - // Return identifier for the saved recipe - return `${global ? 'global' : 'local'}:${sanitizedName}`; - } catch (error) { - throw new Error( - `Failed to save recipe: ${error instanceof Error ? error.message : 'Unknown error'}` - ); - } -} - export async function listSavedRecipes(): Promise { try { const listRecipeResponse = await listRecipes(); @@ -139,19 +47,3 @@ export function convertToLocaleDateString(lastModified: string): string { } return ''; } - -/** - * Generate a suggested filename for a recipe based on its title. - * - * @param recipe The recipe to generate a filename for - * @returns A sanitized filename suitable for use as a recipe name - */ -export function generateRecipeFilename(recipe: Recipe): string { - const baseName = recipe.title - .toLowerCase() - .replace(/[^a-zA-Z0-9\s-]/g, '') - .replace(/\s+/g, '-') - .trim(); - - return baseName || 'untitled-recipe'; -} diff --git a/ui/desktop/src/recipe/validation.test.ts b/ui/desktop/src/recipe/validation.test.ts index 1d6824c654c3..04bb0f3ccb63 100644 --- a/ui/desktop/src/recipe/validation.test.ts +++ b/ui/desktop/src/recipe/validation.test.ts @@ -1,272 +1,7 @@ import { describe, it, expect } from 'vitest'; -import { - validateRecipe, - validateJsonSchema, - getValidationErrorMessages, - getRecipeJsonSchema, -} from './validation'; -import type { Recipe } from '../api/types.gen'; +import { validateJsonSchema, getRecipeJsonSchema } from './validation'; describe('Recipe Validation', () => { - const validRecipe: Recipe = { - version: '1.0.0', - title: 'Test Recipe', - description: 'A test recipe for validation', - instructions: 'Do something useful', - activities: ['Test activity 1', 'Test activity 2'], - extensions: [ - { - type: 'builtin', - name: 'developer', - display_name: 'Developer', - description: 'Developer', - timeout: 300, - bundled: true, - }, - ], - }; - - const validRecipeWithPrompt: Recipe = { - version: '1.0.0', - title: 'Prompt Recipe', - description: 'A recipe using prompt instead of instructions', - prompt: 'You are a helpful assistant', - activities: ['Help users'], - extensions: [ - { - type: 'builtin', - name: 'developer', - description: 'Developer', - }, - ], - }; - - const validRecipeWithParameters: Recipe = { - version: '1.0.0', - title: 'Parameterized Recipe', - description: 'A recipe with parameters', - instructions: 'Process the file at {{ file_path }}', - parameters: [ - { - key: 'file_path', - input_type: 'string', - requirement: 'required', - description: 'Path to the file to process', - }, - ], - activities: ['Process file'], - extensions: [ - { - type: 'builtin', - name: 'developer', - description: 'developer', - }, - ], - }; - - const validRecipeWithAuthor: Recipe = { - version: '1.0.0', - title: 'Authored Recipe', - author: { - contact: 'test@example.com', - }, - description: 'A recipe with author information', - instructions: 'Do something', - activities: ['Activity'], - extensions: [ - { - type: 'builtin', - name: 'developer', - description: 'developer', - }, - ], - }; - - describe('validateRecipe', () => { - describe('valid recipes', () => { - it('validates a basic valid recipe', () => { - const result = validateRecipe(validRecipe); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - expect(result.data).toEqual(validRecipe); - }); - - it('validates a recipe with prompt instead of instructions', () => { - const result = validateRecipe(validRecipeWithPrompt); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - expect(result.data).toEqual(validRecipeWithPrompt); - }); - - it('validates a recipe with parameters', () => { - const result = validateRecipe(validRecipeWithParameters); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - expect(result.data).toEqual(validRecipeWithParameters); - }); - - it('validates a recipe with author information', () => { - const result = validateRecipe(validRecipeWithAuthor); - if (!result.success) { - console.log('Author validation errors:', result.errors); - } - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - - it('validates a recipe with minimal required fields', () => { - const minimalRecipe = { - version: '1.0.0', - title: 'Minimal', - description: 'Minimal recipe', - instructions: 'Do something', - activities: ['Activity'], - extensions: [], - }; - - const result = validateRecipe(minimalRecipe); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - }); - }); - - describe('invalid recipes', () => { - it('rejects recipe without title', () => { - const invalidRecipe = { - ...validRecipe, - title: undefined, - }; - - const result = validateRecipe(invalidRecipe); - expect(result.success).toBe(false); - expect(result.errors.length).toBeGreaterThan(0); - expect(result.data).toBeUndefined(); - }); - - it('rejects recipe without description', () => { - const invalidRecipe = { - ...validRecipe, - description: undefined, - }; - - const result = validateRecipe(invalidRecipe); - expect(result.success).toBe(false); - expect(result.errors.length).toBeGreaterThan(0); - }); - - it('allows recipe without version (version is optional)', () => { - const recipeWithoutVersion = { - ...validRecipe, - version: undefined, - }; - - const result = validateRecipe(recipeWithoutVersion); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - }); - - it('rejects recipe without instructions or prompt', () => { - const invalidRecipe = { - ...validRecipe, - instructions: undefined, - prompt: undefined, - }; - - const result = validateRecipe(invalidRecipe); - expect(result.success).toBe(false); - expect(result.errors).toContain('Either instructions or prompt must be provided'); - }); - - it('validates recipe with minimal extension structure', () => { - const recipeWithMinimalExtension = { - ...validRecipe, - extensions: [ - { - type: 'builtin', - name: 'developer', - description: 'description', - }, - ], - }; - - const result = validateRecipe(recipeWithMinimalExtension); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - }); - - it('validates recipe with incomplete parameter structure', () => { - const recipeWithIncompleteParam = { - ...validRecipe, - parameters: [ - { - key: 'test', - }, - ], - }; - - const result = validateRecipe(recipeWithIncompleteParam); - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - - it('rejects non-object input', () => { - const result = validateRecipe('not an object'); - expect(result.success).toBe(false); - expect(result.errors.length).toBeGreaterThan(0); - }); - - it('rejects null input', () => { - const result = validateRecipe(null); - expect(result.success).toBe(false); - expect(result.errors.length).toBeGreaterThan(0); - }); - - it('rejects undefined input', () => { - const result = validateRecipe(undefined); - expect(result.success).toBe(false); - expect(result.errors.length).toBeGreaterThan(0); - }); - }); - - describe('edge cases', () => { - it('handles empty arrays gracefully', () => { - const recipeWithEmptyArrays = { - ...validRecipe, - activities: [], - extensions: [], - parameters: [], - }; - - const result = validateRecipe(recipeWithEmptyArrays); - expect(result.success).toBe(true); - }); - - it('handles extra properties', () => { - const recipeWithExtra = { - ...validRecipe, - extraField: 'should be ignored or handled gracefully', - }; - - const result = validateRecipe(recipeWithExtra); - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - - it('handles very long strings', () => { - const longString = 'a'.repeat(10000); - const recipeWithLongStrings = { - ...validRecipe, - title: longString, - description: longString, - instructions: longString, - }; - - const result = validateRecipe(recipeWithLongStrings); - expect(typeof result.success).toBe('boolean'); - }); - }); - }); - describe('validateJsonSchema', () => { describe('valid JSON schemas', () => { it('validates a simple JSON schema', () => { @@ -356,24 +91,6 @@ describe('Recipe Validation', () => { }); }); - describe('helper functions', () => { - describe('getValidationErrorMessages', () => { - it('returns the same array of error messages', () => { - const errors = ['title: Required', 'description: Required', 'Invalid format']; - const messages = getValidationErrorMessages(errors); - expect(messages).toEqual(errors); - expect(messages).toHaveLength(3); - }); - - it('handles empty array', () => { - const errors: string[] = []; - const messages = getValidationErrorMessages(errors); - expect(messages).toHaveLength(0); - expect(messages).toEqual([]); - }); - }); - }); - describe('getRecipeJsonSchema', () => { it('returns a valid JSON schema object', () => { const schema = getRecipeJsonSchema(); @@ -401,162 +118,4 @@ describe('Recipe Validation', () => { expect(schema1).toEqual(schema2); }); }); - - describe('error handling and edge cases', () => { - it('handles validation errors gracefully', () => { - // Test with malformed data that might cause validation to throw - const malformedData = { - version: { not: 'a string' }, - title: ['not', 'a', 'string'], - description: 123, - instructions: null, - activities: 'not an array', - extensions: 'not an array', - }; - - const result = validateRecipe(malformedData); - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - - it('handles circular references gracefully', () => { - const circularObj: Record = { title: 'Test' }; - (circularObj as Record).self = circularObj; - - const result = validateRecipe(circularObj); - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - - it('handles very deep nested objects', () => { - let deepObj: Record = { - version: '1.0.0', - title: 'Deep', - description: 'Test', - }; - let current: Record = deepObj; - - // Create a deeply nested structure - for (let i = 0; i < 100; i++) { - const nested = { level: i }; - current.nested = nested; - current = nested as Record; - } - - const result = validateRecipe(deepObj); - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - }); - - describe('real-world recipe examples', () => { - it('validates readme-bot style recipe', () => { - const readmeBotRecipe = { - version: '1.0.0', - title: 'Readme Bot', - author: { - contact: 'DOsinga', - }, - description: 'Generates or updates a readme', - instructions: 'You are a documentation expert', - activities: [ - 'Scan project directory for documentation context', - 'Generate a new README draft', - 'Compare new draft with existing README.md', - ], - extensions: [ - { - type: 'builtin', - name: 'developer', - display_name: 'Developer', - timeout: 300, - bundled: true, - }, - ], - prompt: "Here's what to do step by step: 1. The current folder is a software project...", - }; - - const result = validateRecipe(readmeBotRecipe); - if (!result.success) { - console.log('ReadmeBot validation errors:', result.errors); - } - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - - it('validates lint-my-code style recipe with parameters', () => { - const lintRecipe = { - version: '1.0.0', - title: 'Lint My Code', - author: { - contact: 'iandouglas', - }, - description: - 'Analyzes code files for syntax and layout issues using available linting tools', - instructions: - 'You are a code quality expert that helps identify syntax and layout issues in code files', - activities: [ - 'Detect file type and programming language', - 'Check for available linting tools in the project', - 'Run appropriate linters for syntax and layout checking', - 'Provide recommendations if no linters are found', - ], - parameters: [ - { - key: 'file_path', - input_type: 'string', - requirement: 'required', - description: 'Path to the file you want to lint', - }, - ], - extensions: [ - { - type: 'builtin', - name: 'developer', - display_name: 'Developer', - timeout: 300, - bundled: true, - }, - ], - prompt: - 'I need you to lint the file at {{ file_path }} for syntax and layout issues only...', - }; - - const result = validateRecipe(lintRecipe); - if (!result.success) { - console.log('LintRecipe validation errors:', result.errors); - } - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - - it('validates 404Portfolio style recipe with multiple extensions', () => { - const portfolioRecipe = { - version: '1.0.0', - title: '404Portfolio', - description: 'Create personalized, creative 404 pages using public profile data', - instructions: 'Create an engaging 404 error page that tells a creative story...', - activities: [ - 'Build error page from GitHub repos', - 'Generate error page from dev.to blog posts', - 'Create a 404 page featuring Bluesky bio', - ], - extensions: [ - { - type: 'builtin', - name: 'developer', - description: 'developer', - }, - { - type: 'builtin', - name: 'computercontroller', - description: 'computercontroller', - }, - ], - }; - - const result = validateRecipe(portfolioRecipe); - expect(result.success).toBe(true); - }); - }); }); diff --git a/ui/desktop/src/recipe/validation.ts b/ui/desktop/src/recipe/validation.ts index 065e2c2f3e41..7a507e716b4e 100644 --- a/ui/desktop/src/recipe/validation.ts +++ b/ui/desktop/src/recipe/validation.ts @@ -1,4 +1,3 @@ -import { z } from 'zod'; import type { Recipe } from '../api/types.gen'; /** @@ -121,206 +120,7 @@ export type RecipeValidationResult = { data?: Recipe | unknown; }; -/** - * Converts an OpenAPI schema to a Zod schema dynamically - */ -function openApiSchemaToZod(schema: Record): z.ZodTypeAny { - if (!schema) { - return z.any(); - } - - // Handle different schema types - switch (schema.type) { - case 'string': { - let stringSchema = z.string(); - if (typeof schema.minLength === 'number') { - stringSchema = stringSchema.min(schema.minLength); - } - if (typeof schema.maxLength === 'number') { - stringSchema = stringSchema.max(schema.maxLength); - } - if (Array.isArray(schema.enum)) { - return z.enum(schema.enum as [string, ...string[]]); - } - if (schema.format === 'date-time') { - stringSchema = stringSchema.datetime(); - } - if (typeof schema.pattern === 'string') { - stringSchema = stringSchema.regex(new RegExp(schema.pattern)); - } - return schema.nullable ? stringSchema.nullable() : stringSchema; - } - - case 'number': - case 'integer': { - let numberSchema = schema.type === 'integer' ? z.number().int() : z.number(); - if (typeof schema.minimum === 'number') { - numberSchema = numberSchema.min(schema.minimum); - } - if (typeof schema.maximum === 'number') { - numberSchema = numberSchema.max(schema.maximum); - } - return schema.nullable ? numberSchema.nullable() : numberSchema; - } - - case 'boolean': - return schema.nullable ? z.boolean().nullable() : z.boolean(); - - case 'array': { - const itemSchema = schema.items - ? openApiSchemaToZod(schema.items as Record) - : z.any(); - let arraySchema = z.array(itemSchema); - if (typeof schema.minItems === 'number') { - arraySchema = arraySchema.min(schema.minItems); - } - if (typeof schema.maxItems === 'number') { - arraySchema = arraySchema.max(schema.maxItems); - } - return schema.nullable ? arraySchema.nullable() : arraySchema; - } - - case 'object': - if (schema.properties && typeof schema.properties === 'object') { - const shape: Record = {}; - for (const [propName, propSchema] of Object.entries(schema.properties)) { - shape[propName] = openApiSchemaToZod(propSchema as Record); - } - - // Make optional properties optional based on required array - const optionalShape: Record = {}; - const requiredFields = - schema.required && Array.isArray(schema.required) ? schema.required : []; - - for (const [propName, zodSchema] of Object.entries(shape)) { - if (requiredFields.includes(propName)) { - optionalShape[propName] = zodSchema; - } else { - optionalShape[propName] = zodSchema.optional(); - } - } - - let objectSchema = z.object(optionalShape); - - if (schema.additionalProperties === true) { - return schema.nullable - ? objectSchema.passthrough().nullable() - : objectSchema.passthrough(); - } else if (schema.additionalProperties === false) { - return schema.nullable ? objectSchema.strict().nullable() : objectSchema.strict(); - } - - return schema.nullable ? objectSchema.nullable() : objectSchema; - } - return schema.nullable ? z.record(z.any()).nullable() : z.record(z.any()); - - default: - // Handle $ref, allOf, oneOf, anyOf, etc. - if (typeof schema.$ref === 'string') { - // Resolve the $ref and convert the resolved schema to Zod - const resolvedSchema = resolveRefs(schema, openApiSpec as Record); - // If resolution changed the schema, convert the resolved version - if (resolvedSchema !== schema) { - return openApiSchemaToZod(resolvedSchema); - } - // If resolution failed, fall back to z.any() - return z.any(); - } - - if (Array.isArray(schema.allOf)) { - // Intersection of all schemas - return schema.allOf.reduce((acc: z.ZodTypeAny, subSchema: unknown) => { - return acc.and(openApiSchemaToZod(subSchema as Record)); - }, z.any()); - } - - if (Array.isArray(schema.oneOf) || Array.isArray(schema.anyOf)) { - // Union of schemas - const schemaArray = (schema.oneOf || schema.anyOf) as unknown[]; - const schemas = schemaArray.map((subSchema: unknown) => - openApiSchemaToZod(subSchema as Record) - ); - return z.union(schemas as [z.ZodTypeAny, z.ZodTypeAny, ...z.ZodTypeAny[]]); - } - - return z.any(); - } -} - -/** - * Validates a value against an OpenAPI schema using Zod - */ -function validateAgainstSchema(value: unknown, schema: Record): string[] { - if (!schema) { - return ['Schema not found']; - } - - try { - // Resolve $refs in the schema before converting to Zod - const resolvedSchema = resolveRefs(schema, openApiSpec as Record); - const zodSchema = openApiSchemaToZod(resolvedSchema); - const result = zodSchema.safeParse(value); - - if (result.success) { - return []; - } else { - return result.error.errors.map((err) => { - const path = err.path.length > 0 ? `${err.path.join('.')}: ` : ''; - return `${path}${err.message}`; - }); - } - } catch (error) { - return [`Schema conversion error: ${error instanceof Error ? error.message : 'Unknown error'}`]; - } -} - -/** - * Validates a recipe object against the OpenAPI-derived schema. - * This provides structural validation that automatically stays in sync - * with the backend's OpenAPI specification. - */ -export function validateRecipe(recipe: unknown): RecipeValidationResult { - try { - const schema = getRecipeSchema(); - if (!schema) { - return { - success: false, - errors: ['Recipe schema not found in OpenAPI specification'], - }; - } - - const errors = validateAgainstSchema(recipe, schema as Record); - - // Additional business logic validation - if (typeof recipe === 'object' && recipe !== null) { - const recipeObj = recipe as Partial; - if (!recipeObj.instructions && !recipeObj.prompt) { - errors.push('Either instructions or prompt must be provided'); - } - } - - if (errors.length === 0) { - return { - success: true, - errors: [], - data: recipe as Recipe, - }; - } else { - return { - success: false, - errors, - data: undefined, - }; - } - } catch (error) { - return { - success: false, - errors: [`Validation error: ${error instanceof Error ? error.message : 'Unknown error'}`], - data: undefined, - }; - } -} - +// TODO: Lifei Remove this /** * JSON schema validation for the response.json_schema field. * Uses basic structural validation instead of AJV to avoid CSP eval security issues. @@ -387,13 +187,6 @@ export function validateJsonSchema(schema: unknown): RecipeValidationResult { } } -/** - * Helper function to format validation error messages - */ -export function getValidationErrorMessages(errors: string[]): string[] { - return errors; -} - /** * Returns a JSON schema representation derived directly from the OpenAPI specification. * This schema is used for documentation in form help text. From ab07266887a191e2bface6912dd19924712e2355 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 16:50:38 +1100 Subject: [PATCH 09/22] removed recipe name and is global on the server side when listing and save recipe --- crates/goose-server/src/routes/recipe.rs | 9 +--- .../goose-server/src/routes/recipe_utils.rs | 54 ------------------- crates/goose/src/recipe/local_recipes.rs | 10 +--- .../src/components/recipes/RecipesView.tsx | 4 +- 4 files changed, 5 insertions(+), 72 deletions(-) diff --git a/crates/goose-server/src/routes/recipe.rs b/crates/goose-server/src/routes/recipe.rs index 5e793b770910..53e823650613 100644 --- a/crates/goose-server/src/routes/recipe.rs +++ b/crates/goose-server/src/routes/recipe.rs @@ -21,7 +21,6 @@ use crate::state::AppState; #[derive(Debug, Deserialize, ToSchema)] pub struct CreateRecipeRequest { session_id: String, - // Optional fields #[serde(default)] author: Option, } @@ -74,7 +73,6 @@ pub struct ScanRecipeResponse { pub struct SaveRecipeRequest { recipe: Recipe, id: Option, - is_global: Option, } #[derive(Debug, Deserialize, ToSchema)] pub struct ParseRecipeRequest { @@ -88,7 +86,6 @@ pub struct ParseRecipeResponse { #[derive(Debug, Serialize, ToSchema)] pub struct RecipeManifestResponse { - name: String, recipe: Recipe, #[serde(rename = "lastModified")] last_modified: String, @@ -117,7 +114,6 @@ pub struct ListRecipeResponse { ), tag = "Recipe Management" )] -/// Create a Recipe configuration from the current session async fn create_recipe( State(state): State>, Json(request): Json, @@ -127,7 +123,6 @@ async fn create_recipe( request.session_id ); - // Load messages from session let session = match SessionManager::get_session(&request.session_id, true).await { Ok(session) => session, Err(e) => { @@ -150,7 +145,6 @@ async fn create_recipe( let agent = state.get_agent_for_route(request.session_id).await?; - // Create base recipe from agent state and messages let recipe_result = agent.create_recipe(conversation).await; match recipe_result { @@ -263,7 +257,6 @@ async fn list_recipes( let file_path = recipe_manifest_with_path.file_path.clone(); recipe_file_hash_map.insert(id.clone(), file_path); RecipeManifestResponse { - name: recipe_manifest_with_path.name.clone(), recipe: recipe_manifest_with_path.recipe.clone(), id: id.clone(), last_modified: recipe_manifest_with_path.last_modified.clone(), @@ -326,7 +319,7 @@ async fn save_recipe( None => None, }; - match local_recipes::save_recipe_to_file(request.recipe, request.is_global, file_path) { + match local_recipes::save_recipe_to_file(request.recipe, file_path) { Ok(_) => Ok(StatusCode::NO_CONTENT), Err(e) => Err(ErrorResponse { message: e.to_string(), diff --git a/crates/goose-server/src/routes/recipe_utils.rs b/crates/goose-server/src/routes/recipe_utils.rs index 1a76d947a661..49181a246986 100644 --- a/crates/goose-server/src/routes/recipe_utils.rs +++ b/crates/goose-server/src/routes/recipe_utils.rs @@ -8,14 +8,8 @@ use anyhow::Result; use goose::recipe::local_recipes::list_local_recipes; use goose::recipe::Recipe; -use std::path::Path; - -use serde::{Deserialize, Serialize}; -use utoipa::ToSchema; - pub struct RecipeManifestWithPath { pub id: String, - pub name: String, pub recipe: Recipe, pub file_path: PathBuf, pub last_modified: String, @@ -37,16 +31,9 @@ pub fn get_all_recipes_manifests() -> Result> { else { continue; }; - let recipe_metadata = - RecipeManifestMetadata::from_yaml_file(&file_path).unwrap_or_else(|_| { - RecipeManifestMetadata { - name: recipe.title.clone(), - } - }); let manifest_with_path = RecipeManifestWithPath { id: short_id_from_path(file_path.to_string_lossy().as_ref()), - name: recipe_metadata.name, recipe, file_path, last_modified, @@ -57,44 +44,3 @@ pub fn get_all_recipes_manifests() -> Result> { Ok(recipe_manifests_with_path) } - -// this is a temporary struct to deserilize the UI recipe files. should not be used for other purposes. -#[derive(Serialize, Deserialize, Debug, Clone, ToSchema)] -struct RecipeManifestMetadata { - pub name: String, -} - -impl RecipeManifestMetadata { - pub fn from_yaml_file(path: &Path) -> Result { - let content = fs::read_to_string(path) - .map_err(|e| anyhow::anyhow!("Failed to read file {}: {}", path.display(), e))?; - let metadata = serde_yaml::from_str::(&content) - .map_err(|e| anyhow::anyhow!("Failed to parse YAML: {}", e))?; - Ok(metadata) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use std::fs; - use tempfile::tempdir; - - #[test] - fn test_from_yaml_file_success() { - let temp_dir = tempdir().unwrap(); - let file_path = temp_dir.path().join("test_recipe.yaml"); - - let yaml_content = r#" -name: "Test Recipe" -isGlobal: true -recipe: recipe_content -"#; - - fs::write(&file_path, yaml_content).unwrap(); - - let result = RecipeManifestMetadata::from_yaml_file(&file_path).unwrap(); - - assert_eq!(result.name, "Test Recipe"); - } -} diff --git a/crates/goose/src/recipe/local_recipes.rs b/crates/goose/src/recipe/local_recipes.rs index 3a905ede8366..63ad5d56f3df 100644 --- a/crates/goose/src/recipe/local_recipes.rs +++ b/crates/goose/src/recipe/local_recipes.rs @@ -165,14 +165,8 @@ fn generate_recipe_filename(title: &str, recipe_library_dir: &Path) -> PathBuf { } } -pub fn save_recipe_to_file( - recipe: Recipe, - is_global: Option, - file_path: Option, -) -> anyhow::Result { - let is_global_value = is_global.unwrap_or(true); - - let recipe_library_dir = get_recipe_library_dir(is_global_value); +pub fn save_recipe_to_file(recipe: Recipe, file_path: Option) -> anyhow::Result { + let recipe_library_dir = get_recipe_library_dir(true); let file_path_value = match file_path { Some(path) => path, diff --git a/ui/desktop/src/components/recipes/RecipesView.tsx b/ui/desktop/src/components/recipes/RecipesView.tsx index a1bb7472ee55..f1b052486266 100644 --- a/ui/desktop/src/components/recipes/RecipesView.tsx +++ b/ui/desktop/src/components/recipes/RecipesView.tsx @@ -99,7 +99,7 @@ export default function RecipesView() { buttons: ['Cancel', 'Delete'], defaultId: 0, title: 'Delete Recipe', - message: `Are you sure you want to delete "${recipeManifest.name}"?`, + message: `Are you sure you want to delete "${recipeManifest.recipe.title}"?`, detail: 'Recipe file will be deleted.', }); @@ -111,7 +111,7 @@ export default function RecipesView() { await deleteRecipe({ body: { id: recipeManifest.id } }); await loadSavedRecipes(); toastSuccess({ - title: recipeManifest.name, + title: recipeManifest.recipe.title, msg: 'Recipe deleted successfully', }); } catch (err) { From 5ab4c55ecb364422900d975e18658bb5f26178c4 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 17:04:14 +1100 Subject: [PATCH 10/22] update ui matching server side change --- ui/desktop/openapi.json | 9 --------- ui/desktop/src/api/sdk.gen.ts | 3 --- ui/desktop/src/api/types.gen.ts | 2 -- .../src/components/recipes/CreateEditRecipeModal.tsx | 4 ++-- .../components/recipes/CreateRecipeFromSessionModal.tsx | 2 +- ui/desktop/src/components/recipes/ImportRecipeForm.tsx | 2 +- ui/desktop/src/recipe/recipe_management.ts | 7 +------ 7 files changed, 5 insertions(+), 24 deletions(-) diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index a8398f713d3d..39705dd109e7 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -931,7 +931,6 @@ "tags": [ "Recipe Management" ], - "summary": "Create a Recipe configuration from the current session", "operationId": "create_recipe", "requestBody": { "content": { @@ -3519,7 +3518,6 @@ "RecipeManifestResponse": { "type": "object", "required": [ - "name", "recipe", "lastModified", "id" @@ -3531,9 +3529,6 @@ "lastModified": { "type": "string" }, - "name": { - "type": "string" - }, "recipe": { "$ref": "#/components/schemas/Recipe" } @@ -3743,10 +3738,6 @@ "type": "string", "nullable": true }, - "is_global": { - "type": "boolean", - "nullable": true - }, "recipe": { "$ref": "#/components/schemas/Recipe" } diff --git a/ui/desktop/src/api/sdk.gen.ts b/ui/desktop/src/api/sdk.gen.ts index 60dc95ced607..e33e91f5ea60 100644 --- a/ui/desktop/src/api/sdk.gen.ts +++ b/ui/desktop/src/api/sdk.gen.ts @@ -274,9 +274,6 @@ export const startTetrateSetup = (options? }); }; -/** - * Create a Recipe configuration from the current session - */ export const createRecipe = (options: Options) => { return (options.client ?? _heyApiClient).post({ url: '/recipes/create', diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index bf69f3e86789..dadaf3b6bd85 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -567,7 +567,6 @@ export type Recipe = { export type RecipeManifestResponse = { id: string; lastModified: string; - name: string; recipe: Recipe; }; @@ -646,7 +645,6 @@ export type RunNowResponse = { export type SaveRecipeRequest = { id?: string | null; - is_global?: boolean | null; recipe: Recipe; }; diff --git a/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx b/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx index 70a24309db5b..3a02381be0bf 100644 --- a/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx +++ b/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx @@ -304,7 +304,7 @@ export default function CreateEditRecipeModal({ try { const recipe = getCurrentRecipe(); - await saveRecipe(recipe, true, recipeId); + await saveRecipe(recipe, recipeId); onClose(true); @@ -338,7 +338,7 @@ export default function CreateEditRecipeModal({ try { const recipe = getCurrentRecipe(); - await saveRecipe(recipe, true, recipeId); + await saveRecipe(recipe, recipeId); // Close modal first onClose(true); diff --git a/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx b/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx index fb44fc5d948b..2c6c2b916d94 100644 --- a/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx +++ b/ui/desktop/src/components/recipes/CreateRecipeFromSessionModal.tsx @@ -183,7 +183,7 @@ export default function CreateRecipeFromSessionModal({ extensions: [], // Will be populated based on current extensions }; - await saveRecipe(recipe, true, null); + await saveRecipe(recipe, null); onRecipeCreated?.(recipe); onClose(); diff --git a/ui/desktop/src/components/recipes/ImportRecipeForm.tsx b/ui/desktop/src/components/recipes/ImportRecipeForm.tsx index 72d23c713f20..0adba1a9830c 100644 --- a/ui/desktop/src/components/recipes/ImportRecipeForm.tsx +++ b/ui/desktop/src/components/recipes/ImportRecipeForm.tsx @@ -110,7 +110,7 @@ export default function ImportRecipeForm({ isOpen, onClose, onSuccess }: ImportR recipe = await parseRecipeFromFile(fileContent); } - await saveRecipe(recipe, true, null); + await saveRecipe(recipe, null); // Reset dialog state importRecipeForm.reset({ diff --git a/ui/desktop/src/recipe/recipe_management.ts b/ui/desktop/src/recipe/recipe_management.ts index 2b9e9f247af3..b4aeef30ffac 100644 --- a/ui/desktop/src/recipe/recipe_management.ts +++ b/ui/desktop/src/recipe/recipe_management.ts @@ -1,16 +1,11 @@ import { Recipe, saveRecipe as saveRecipeApi } from '../api'; -export async function saveRecipe( - recipe: Recipe, - isGlobal: boolean | null, - recipeId?: string | null -): Promise { +export async function saveRecipe(recipe: Recipe, recipeId?: string | null): Promise { try { await saveRecipeApi({ body: { recipe, id: recipeId, - is_global: isGlobal, }, throwOnError: true, }); From 780b84dcefc51c3251f3ff5e19a1db2c841c54b6 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 20:23:27 +1100 Subject: [PATCH 11/22] refactored recipe validation --- crates/goose-cli/src/cli.rs | 2 +- crates/goose-cli/src/commands/recipe.rs | 10 +- crates/goose-cli/src/recipes/github_recipe.rs | 2 +- crates/goose-cli/src/recipes/recipe.rs | 35 +---- crates/goose-server/src/routes/schedule.rs | 12 +- crates/goose/src/recipe/build_recipe/mod.rs | 101 +------------ crates/goose/src/recipe/mod.rs | 3 +- crates/goose/src/recipe/template_recipe.rs | 37 ++--- crates/goose/src/recipe/validate_recipe.rs | 138 ++++++++++++++++++ 9 files changed, 185 insertions(+), 155 deletions(-) create mode 100644 crates/goose/src/recipe/validate_recipe.rs diff --git a/crates/goose-cli/src/cli.rs b/crates/goose-cli/src/cli.rs index cc83fe0271fa..2e38b0e9d787 100644 --- a/crates/goose-cli/src/cli.rs +++ b/crates/goose-cli/src/cli.rs @@ -1011,7 +1011,7 @@ pub async fn cli() -> Result<()> { .and_then(|rf| { goose::recipe::template_recipe::parse_recipe_content( &rf.content, - rf.parent_dir.to_string_lossy().to_string(), + Some(rf.parent_dir.to_string_lossy().to_string()), ) .ok() .map(|(r, _)| r.version) diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index 0e71dcae31c6..c5767038646d 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -1,9 +1,9 @@ use anyhow::Result; use console::style; +use goose::recipe::validate_recipe::validate_recipe_template_from_file; use crate::recipes::github_recipe::RecipeSource; -use crate::recipes::recipe::load_recipe_for_validation; -use crate::recipes::search_recipe::list_available_recipes; +use crate::recipes::search_recipe::{list_available_recipes, load_recipe_file}; use goose::recipe_deeplink; /// Validates a recipe file @@ -17,7 +17,8 @@ use goose::recipe_deeplink; /// Result indicating success or failure pub fn handle_validate(recipe_name: &str) -> Result<()> { // Load and validate the recipe file - match load_recipe_for_validation(recipe_name) { + let recipe_file = load_recipe_file(recipe_name)?; + match validate_recipe_template_from_file(&recipe_file) { Ok(_) => { println!("{} recipe file is valid", style("✓").green().bold()); Ok(()) @@ -39,8 +40,9 @@ pub fn handle_validate(recipe_name: &str) -> Result<()> { /// /// Result indicating success or failure pub fn handle_deeplink(recipe_name: &str) -> Result { + let recipe_file = load_recipe_file(recipe_name)?; // Load the recipe file first to validate it - match load_recipe_for_validation(recipe_name) { + match validate_recipe_template_from_file(&recipe_file) { Ok(recipe) => match recipe_deeplink::encode(&recipe) { Ok(encoded) => { println!( diff --git a/crates/goose-cli/src/recipes/github_recipe.rs b/crates/goose-cli/src/recipes/github_recipe.rs index c7f88b823fe4..bfe297d8dd89 100644 --- a/crates/goose-cli/src/recipes/github_recipe.rs +++ b/crates/goose-cli/src/recipes/github_recipe.rs @@ -331,7 +331,7 @@ fn get_github_recipe_info(repo: &str, dir_name: &str, recipe_filename: &str) -> .map_err(|e| anyhow!("Failed to convert content to string: {}", e))?; // Parse the recipe content - let (recipe, _) = parse_recipe_content(&content, format!("{}/{}", repo, dir_name))?; + let (recipe, _) = parse_recipe_content(&content, Some(format!("{}/{}", repo, dir_name)))?; return Ok(RecipeInfo { name: dir_name.to_string(), diff --git a/crates/goose-cli/src/recipes/recipe.rs b/crates/goose-cli/src/recipes/recipe.rs index 32b05f551561..8b1fe3a6ab18 100644 --- a/crates/goose-cli/src/recipes/recipe.rs +++ b/crates/goose-cli/src/recipes/recipe.rs @@ -7,13 +7,13 @@ use crate::recipes::secret_discovery::{discover_recipe_secrets, SecretRequiremen use anyhow::Result; use goose::config::Config; use goose::recipe::build_recipe::{ - apply_values_to_parameters, build_recipe_from_template, validate_recipe_parameters, RecipeError, + apply_values_to_parameters, build_recipe_from_template, RecipeError, }; use goose::recipe::read_recipe_file_content::RecipeFile; use goose::recipe::template_recipe::render_recipe_for_preview; +use goose::recipe::validate_recipe::validate_recipe_parameters; use goose::recipe::Recipe; use serde_json::Value; -use std::collections::HashMap; fn create_user_prompt_callback() -> impl Fn(&str, &str) -> Result { |key: &str, description: &str| -> Result { @@ -131,29 +131,11 @@ pub fn render_recipe_as_yaml(recipe_name: &str, params: Vec<(String, String)>) - } } -pub fn load_recipe_for_validation(recipe_name: &str) -> Result { - let (recipe_file, recipe_dir_str) = load_recipe_file_with_dir(recipe_name)?; - let recipe_file_content = &recipe_file.content; - validate_recipe_parameters(recipe_file_content, &recipe_dir_str)?; - let recipe = render_recipe_for_preview( - recipe_file_content, - recipe_dir_str.to_string(), - &HashMap::new(), - )?; - - if let Some(response) = &recipe.response { - if let Some(json_schema) = &response.json_schema { - validate_json_schema(json_schema)?; - } - } - - Ok(recipe) -} - pub fn explain_recipe(recipe_name: &str, params: Vec<(String, String)>) -> Result<()> { let (recipe_file, recipe_dir_str) = load_recipe_file_with_dir(recipe_name)?; let recipe_file_content = &recipe_file.content; - let recipe_parameters = validate_recipe_parameters(recipe_file_content, &recipe_dir_str)?; + let recipe_parameters = + validate_recipe_parameters(recipe_file_content, Some(recipe_dir_str.clone()))?; let (params_for_template, missing_params) = apply_values_to_parameters( ¶ms, @@ -163,7 +145,7 @@ pub fn explain_recipe(recipe_name: &str, params: Vec<(String, String)>) -> Resul )?; let recipe = render_recipe_for_preview( recipe_file_content, - recipe_dir_str.to_string(), + Some(recipe_dir_str.clone()), ¶ms_for_template, )?; print_recipe_explanation(&recipe); @@ -172,13 +154,6 @@ pub fn explain_recipe(recipe_name: &str, params: Vec<(String, String)>) -> Resul Ok(()) } -fn validate_json_schema(schema: &serde_json::Value) -> Result<()> { - match jsonschema::validator_for(schema) { - Ok(_) => Ok(()), - Err(err) => Err(anyhow::anyhow!("JSON schema validation failed: {}", err)), - } -} - #[cfg(test)] mod tests { use goose::recipe::{RecipeParameterInputType, RecipeParameterRequirement}; diff --git a/crates/goose-server/src/routes/schedule.rs b/crates/goose-server/src/routes/schedule.rs index 5203e8be703b..a9ce8f399bec 100644 --- a/crates/goose-server/src/routes/schedule.rs +++ b/crates/goose-server/src/routes/schedule.rs @@ -237,11 +237,13 @@ async fn run_now_handler( .and_then(|content| { goose::recipe::template_recipe::parse_recipe_content( &content, - std::path::Path::new(&job.source) - .parent() - .unwrap_or_else(|| std::path::Path::new("")) - .to_string_lossy() - .to_string(), + Some( + std::path::Path::new(&job.source) + .parent() + .unwrap_or_else(|| std::path::Path::new("")) + .to_string_lossy() + .to_string(), + ), ) .ok() .map(|(r, _)| r.version) diff --git a/crates/goose/src/recipe/build_recipe/mod.rs b/crates/goose/src/recipe/build_recipe/mod.rs index 220148742a37..8fa26fab0d84 100644 --- a/crates/goose/src/recipe/build_recipe/mod.rs +++ b/crates/goose/src/recipe/build_recipe/mod.rs @@ -1,11 +1,12 @@ use crate::recipe::read_recipe_file_content::{read_parameter_file_content, RecipeFile}; -use crate::recipe::template_recipe::{parse_recipe_content, render_recipe_content_with_params}; +use crate::recipe::template_recipe::render_recipe_content_with_params; +use crate::recipe::validate_recipe::validate_recipe_parameters; use crate::recipe::{ Recipe, RecipeParameter, RecipeParameterInputType, RecipeParameterRequirement, BUILT_IN_RECIPE_DIR_PARAM, }; use anyhow::Result; -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::path::Path; #[derive(Debug, thiserror::Error)] @@ -34,7 +35,8 @@ where let recipe_dir_str = recipe_parent_dir .to_str() .ok_or_else(|| anyhow::anyhow!("Error getting recipe directory"))?; - let recipe_parameters = validate_recipe_parameters(&recipe_file_content, recipe_dir_str)?; + let recipe_parameters = + validate_recipe_parameters(&recipe_file_content, Some(recipe_dir_str.to_string()))?; let (params_for_template, missing_params) = apply_values_to_parameters(¶ms, recipe_parameters, recipe_dir_str, user_prompt_fn)?; @@ -48,18 +50,6 @@ where Ok((rendered_content, missing_params)) } -pub fn validate_recipe_parameters( - recipe_file_content: &str, - recipe_dir_str: &str, -) -> Result>> { - let (raw_recipe, template_variables) = - parse_recipe_content(recipe_file_content, recipe_dir_str.to_string())?; - let recipe_parameters = raw_recipe.parameters; - validate_optional_parameters(&recipe_parameters)?; - validate_parameters_in_template(&recipe_parameters, &template_variables)?; - Ok(recipe_parameters) -} - pub fn build_recipe_from_template( recipe_file: RecipeFile, params: Vec<(String, String)>, @@ -94,87 +84,6 @@ where Ok(recipe) } -fn validate_parameters_in_template( - recipe_parameters: &Option>, - template_variables: &HashSet, -) -> Result<()> { - let mut template_variables = template_variables.clone(); - template_variables.remove(BUILT_IN_RECIPE_DIR_PARAM); - - let param_keys: HashSet = recipe_parameters - .as_ref() - .unwrap_or(&vec![]) - .iter() - .map(|p| p.key.clone()) - .collect(); - - let missing_keys = template_variables - .difference(¶m_keys) - .collect::>(); - - let extra_keys = param_keys - .difference(&template_variables) - .collect::>(); - - if missing_keys.is_empty() && extra_keys.is_empty() { - return Ok(()); - } - - let mut message = String::new(); - - if !missing_keys.is_empty() { - message.push_str(&format!( - "Missing definitions for parameters in the recipe file: {}.", - missing_keys - .iter() - .map(|s| s.to_string()) - .collect::>() - .join(", ") - )); - } - - if !extra_keys.is_empty() { - message.push_str(&format!( - "\nUnnecessary parameter definitions: {}.", - extra_keys - .iter() - .map(|s| s.to_string()) - .collect::>() - .join(", ") - )); - } - Err(anyhow::anyhow!("{}", message.trim_end())) -} - -fn validate_optional_parameters(parameters: &Option>) -> Result<()> { - let empty_params = vec![]; - let params = parameters.as_ref().unwrap_or(&empty_params); - - let file_params_with_defaults: Vec = params - .iter() - .filter(|p| matches!(p.input_type, RecipeParameterInputType::File) && p.default.is_some()) - .map(|p| p.key.clone()) - .collect(); - - if !file_params_with_defaults.is_empty() { - return Err(anyhow::anyhow!("File parameters cannot have default values to avoid importing sensitive user files: {}", file_params_with_defaults.join(", "))); - } - - let optional_params_without_default_values: Vec = params - .iter() - .filter(|p| { - matches!(p.requirement, RecipeParameterRequirement::Optional) && p.default.is_none() - }) - .map(|p| p.key.clone()) - .collect(); - - if optional_params_without_default_values.is_empty() { - Ok(()) - } else { - Err(anyhow::anyhow!("Optional parameters missing default values in the recipe: {}. Please provide defaults.", optional_params_without_default_values.join(", "))) - } -} - pub fn apply_values_to_parameters( user_params: &[(String, String)], recipe_parameters: Option>, diff --git a/crates/goose/src/recipe/mod.rs b/crates/goose/src/recipe/mod.rs index 12ba94cba7d7..32ce6dd629a1 100644 --- a/crates/goose/src/recipe/mod.rs +++ b/crates/goose/src/recipe/mod.rs @@ -16,6 +16,7 @@ pub mod build_recipe; pub mod local_recipes; pub mod read_recipe_file_content; pub mod template_recipe; +pub mod validate_recipe; pub const BUILT_IN_RECIPE_DIR_PARAM: &str = "recipe_dir"; pub const RECIPE_FILE_EXTENSIONS: &[&str] = &["yaml", "json"]; @@ -775,7 +776,7 @@ isGlobal: true"#; } = &extensions[0] { assert_eq!(name, "test_extension"); - assert_eq!(description, "test_extension"); + assert_eq!(description, ""); } else { panic!("Expected Stdio extension"); } diff --git a/crates/goose/src/recipe/template_recipe.rs b/crates/goose/src/recipe/template_recipe.rs index 3190195b2b54..f9ffefa4ed21 100644 --- a/crates/goose/src/recipe/template_recipe.rs +++ b/crates/goose/src/recipe/template_recipe.rs @@ -98,7 +98,7 @@ pub fn render_recipe_content_with_params( let env = add_template_in_env( &content_with_safe_variables, - params.get(BUILT_IN_RECIPE_DIR_PARAM).unwrap().clone(), + params.get(BUILT_IN_RECIPE_DIR_PARAM).cloned(), UndefinedBehavior::Strict, )?; let template = env.get_template(CURRENT_TEMPLATE_NAME).unwrap(); @@ -110,23 +110,26 @@ pub fn render_recipe_content_with_params( fn add_template_in_env( content: &str, - recipe_dir: String, + recipe_dir: Option, undefined_behavior: UndefinedBehavior, ) -> Result> { let mut env = minijinja::Environment::new(); env.set_undefined_behavior(undefined_behavior); - env.set_loader(move |name| { - let path = Path::new(recipe_dir.as_str()).join(name); - match std::fs::read_to_string(&path) { - Ok(content) => Ok(Some(content)), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), - Err(e) => Err(minijinja::Error::new( - minijinja::ErrorKind::InvalidOperation, - "could not read template", - ) - .with_source(e)), - } - }); + + if let Some(recipe_dir) = recipe_dir { + env.set_loader(move |name| { + let path = Path::new(recipe_dir.as_str()).join(name); + match std::fs::read_to_string(&path) { + Ok(content) => Ok(Some(content)), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(minijinja::Error::new( + minijinja::ErrorKind::InvalidOperation, + "could not read template", + ) + .with_source(e)), + } + }); + } env.add_template(CURRENT_TEMPLATE_NAME, content)?; Ok(env) @@ -134,7 +137,7 @@ fn add_template_in_env( fn get_env_with_template_variables( content: &str, - recipe_dir: String, + recipe_dir: Option, undefined_behavior: UndefinedBehavior, ) -> Result<(Environment<'_>, HashSet)> { let env = add_template_in_env(content, recipe_dir, undefined_behavior)?; @@ -149,7 +152,7 @@ fn get_env_with_template_variables( pub fn parse_recipe_content( content: &str, - recipe_dir: String, + recipe_dir: Option, ) -> Result<(Recipe, HashSet)> { // Pre-process template variables to handle invalid variable names let preprocessed_content = preprocess_template_variables(content)?; @@ -171,7 +174,7 @@ pub fn parse_recipe_content( // render the recipe for validation, deeplink and explain, etc. pub fn render_recipe_for_preview( content: &str, - recipe_dir: String, + recipe_dir: Option, params: &HashMap, ) -> Result { // Pre-process template variables to handle invalid variable names diff --git a/crates/goose/src/recipe/validate_recipe.rs b/crates/goose/src/recipe/validate_recipe.rs new file mode 100644 index 000000000000..966b2147be7f --- /dev/null +++ b/crates/goose/src/recipe/validate_recipe.rs @@ -0,0 +1,138 @@ +use crate::recipe::read_recipe_file_content::RecipeFile; +use crate::recipe::template_recipe::{parse_recipe_content, render_recipe_for_preview}; +use crate::recipe::{ + Recipe, RecipeParameter, RecipeParameterInputType, RecipeParameterRequirement, + BUILT_IN_RECIPE_DIR_PARAM, +}; +use anyhow::Result; +use std::collections::{HashMap, HashSet}; + +pub fn validate_recipe_parameters( + recipe_file_content: &str, + recipe_dir_str: Option, +) -> Result>> { + let (recipe_template, template_variables) = + parse_recipe_content(recipe_file_content, recipe_dir_str)?; + let recipe_parameters = recipe_template.parameters; + validate_optional_parameters(&recipe_parameters)?; + validate_parameters_in_template(&recipe_parameters, &template_variables)?; + Ok(recipe_parameters) +} + +fn validate_json_schema(schema: &serde_json::Value) -> Result<()> { + match jsonschema::validator_for(schema) { + Ok(_) => Ok(()), + Err(err) => Err(anyhow::anyhow!("JSON schema validation failed: {}", err)), + } +} + +pub fn validate_recipe_template_from_file(recipe_file: &RecipeFile) -> Result { + let recipe_dir = recipe_file + .parent_dir + .to_str() + .ok_or_else(|| anyhow::anyhow!("Error getting recipe directory"))? + .to_string(); + + validate_recipe_template(&recipe_file.content, Some(recipe_dir)) +} + +pub fn validate_recipe_template_from_content( + recipe_content: &str, + recipe_dir: Option, +) -> Result { + validate_recipe_template(recipe_content, recipe_dir) +} + +fn validate_recipe_template(recipe_content: &str, recipe_dir: Option) -> Result { + validate_recipe_parameters(recipe_content, recipe_dir.clone())?; + let recipe = render_recipe_for_preview(recipe_content, recipe_dir, &HashMap::new())?; + + if let Some(response) = &recipe.response { + if let Some(json_schema) = &response.json_schema { + validate_json_schema(json_schema)?; + } + } + + Ok(recipe) +} + +fn validate_parameters_in_template( + recipe_parameters: &Option>, + template_variables: &HashSet, +) -> Result<()> { + let mut template_variables = template_variables.clone(); + template_variables.remove(BUILT_IN_RECIPE_DIR_PARAM); + + let param_keys: HashSet = recipe_parameters + .as_ref() + .unwrap_or(&vec![]) + .iter() + .map(|p| p.key.clone()) + .collect(); + + let missing_keys = template_variables + .difference(¶m_keys) + .collect::>(); + + let extra_keys = param_keys + .difference(&template_variables) + .collect::>(); + + if missing_keys.is_empty() && extra_keys.is_empty() { + return Ok(()); + } + + let mut message = String::new(); + + if !missing_keys.is_empty() { + message.push_str(&format!( + "Missing definitions for parameters in the recipe file: {}.", + missing_keys + .iter() + .map(|s| s.to_string()) + .collect::>() + .join(", ") + )); + } + + if !extra_keys.is_empty() { + message.push_str(&format!( + "\nUnnecessary parameter definitions: {}.", + extra_keys + .iter() + .map(|s| s.to_string()) + .collect::>() + .join(", ") + )); + } + Err(anyhow::anyhow!("{}", message.trim_end())) +} + +fn validate_optional_parameters(parameters: &Option>) -> Result<()> { + let empty_params = vec![]; + let params = parameters.as_ref().unwrap_or(&empty_params); + + let file_params_with_defaults: Vec = params + .iter() + .filter(|p| matches!(p.input_type, RecipeParameterInputType::File) && p.default.is_some()) + .map(|p| p.key.clone()) + .collect(); + + if !file_params_with_defaults.is_empty() { + return Err(anyhow::anyhow!("File parameters cannot have default values to avoid importing sensitive user files: {}", file_params_with_defaults.join(", "))); + } + + let optional_params_without_default_values: Vec = params + .iter() + .filter(|p| { + matches!(p.requirement, RecipeParameterRequirement::Optional) && p.default.is_none() + }) + .map(|p| p.key.clone()) + .collect(); + + if optional_params_without_default_values.is_empty() { + Ok(()) + } else { + Err(anyhow::anyhow!("Optional parameters missing default values in the recipe: {}. Please provide defaults.", optional_params_without_default_values.join(", "))) + } +} From 2f904bfd11f41c6e7ead31bc95cfb7bf51be4b4d Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 20:41:16 +1100 Subject: [PATCH 12/22] fixed error messages --- crates/goose-cli/src/commands/recipe.rs | 19 +++---------------- crates/goose/src/recipe/mod.rs | 2 +- 2 files changed, 4 insertions(+), 17 deletions(-) diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index c5767038646d..6a7b6513c522 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -23,10 +23,7 @@ pub fn handle_validate(recipe_name: &str) -> Result<()> { println!("{} recipe file is valid", style("✓").green().bold()); Ok(()) } - Err(err) => { - println!("{} {}", style("✗").red().bold(), err); - Err(err) - } + Err(err) => Err(err), } } @@ -54,19 +51,9 @@ pub fn handle_deeplink(recipe_name: &str) -> Result { println!("{}", full_url); Ok(full_url) } - Err(err) => { - println!( - "{} Failed to encode recipe: {}", - style("✗").red().bold(), - err - ); - Err(anyhow::anyhow!("Failed to encode recipe: {}", err)) - } + Err(err) => Err(anyhow::anyhow!("Failed to encode recipe: {}", err)), }, - Err(err) => { - println!("{} {}", style("✗").red().bold(), err); - Err(err) - } + Err(err) => Err(err), } } diff --git a/crates/goose/src/recipe/mod.rs b/crates/goose/src/recipe/mod.rs index 32ce6dd629a1..beea2fd025b9 100644 --- a/crates/goose/src/recipe/mod.rs +++ b/crates/goose/src/recipe/mod.rs @@ -278,7 +278,7 @@ impl Recipe { } let recipe: Recipe = serde_yaml::from_value(value) - .map_err(|e| anyhow::anyhow!("Failed to deserialize recipe: {}", e))?; + .map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))?; if let Some(ref retry_config) = recipe.retry { if let Err(validation_error) = retry_config.validate() { From 4ebddb73feff05663bff4c065933b540dd90bc1b Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 20:53:39 +1100 Subject: [PATCH 13/22] fixed merge issue --- crates/goose-cli/src/commands/recipe.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index a7b6d3b16e36..47d435e923fa 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -37,7 +37,6 @@ pub fn handle_validate(recipe_name: &str) -> Result<()> { /// /// Result indicating success or failure pub fn handle_deeplink(recipe_name: &str) -> Result { - match generate_deeplink(recipe_name) { Ok((deeplink_url, recipe)) => { println!( @@ -177,8 +176,9 @@ pub fn handle_list(format: &str, verbose: bool) -> Result<()> { /// /// Result containing the deeplink URL and recipe fn generate_deeplink(recipe_name: &str) -> Result<(String, goose::recipe::Recipe)> { + let recipe_file = load_recipe_file(recipe_name)?; // Load the recipe file first to validate it - let recipe = load_recipe_for_validation(recipe_name)?; + let recipe = validate_recipe_template_from_file(&recipe_file)?; match recipe_deeplink::encode(&recipe) { Ok(encoded) => { let full_url = format!("goose://recipe?config={}", encoded); From e09192cf6def66ab837936ccf100e2121251fdc4 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 21:45:22 +1100 Subject: [PATCH 14/22] added default value (empty string) for deserialization on extension config description --- crates/goose/src/agents/extension.rs | 16 +++++++- crates/goose/src/recipe/mod.rs | 38 +++++++------------ .../CreateRecipeFromSessionModal.test.tsx | 32 ---------------- 3 files changed, 28 insertions(+), 58 deletions(-) diff --git a/crates/goose/src/agents/extension.rs b/crates/goose/src/agents/extension.rs index 0af92592c464..2659aceab7d1 100644 --- a/crates/goose/src/agents/extension.rs +++ b/crates/goose/src/agents/extension.rs @@ -9,11 +9,18 @@ use once_cell::sync::Lazy; use rmcp::model::Tool; use rmcp::service::ClientInitializeError; use rmcp::ServiceError as ClientError; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use thiserror::Error; use tracing::warn; use utoipa::ToSchema; +fn deserialize_string_or_empty<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) +} + #[derive(Error, Debug)] #[error("process quit before initialization: stderr = {stderr}")] pub struct ProcessExit { @@ -184,6 +191,7 @@ pub enum ExtensionConfig { Sse { /// The name used to identify this extension name: String, + #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, uri: String, #[serde(default)] @@ -203,6 +211,7 @@ pub enum ExtensionConfig { Stdio { /// The name used to identify this extension name: String, + #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, cmd: String, args: Vec, @@ -221,6 +230,7 @@ pub enum ExtensionConfig { Builtin { /// The name used to identify this extension name: String, + #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, display_name: Option, // needed for the UI timeout: Option, @@ -234,6 +244,7 @@ pub enum ExtensionConfig { Platform { /// The name used to identify this extension name: String, + #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, #[serde(default)] bundled: Option, @@ -245,6 +256,7 @@ pub enum ExtensionConfig { StreamableHttp { /// The name used to identify this extension name: String, + #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, uri: String, #[serde(default)] @@ -266,6 +278,7 @@ pub enum ExtensionConfig { Frontend { /// The name used to identify this extension name: String, + #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, /// The tools provided by the frontend tools: Vec, @@ -281,6 +294,7 @@ pub enum ExtensionConfig { InlinePython { /// The name used to identify this extension name: String, + #[serde(default)] description: String, /// The Python code to execute code: String, diff --git a/crates/goose/src/recipe/mod.rs b/crates/goose/src/recipe/mod.rs index beea2fd025b9..2373b28bdaa0 100644 --- a/crates/goose/src/recipe/mod.rs +++ b/crates/goose/src/recipe/mod.rs @@ -254,31 +254,19 @@ impl Recipe { } pub fn from_content(content: &str) -> Result { - // Parse using YAML parser (since JSON is a subset of YAML, this handles both) - let mut value: serde_yaml::Value = serde_yaml::from_str(content) - .map_err(|e| anyhow::anyhow!("Failed to parse recipe content as YAML/JSON: {}", e))?; - - // Handle nested legacy recipe format - if let Some(nested_recipe) = value.get("recipe") { - value = nested_recipe.clone(); - } - - if let Some(extensions) = value - .get_mut("extensions") - .and_then(|v| v.as_sequence_mut()) - { - extensions - .iter_mut() - .filter_map(|ext| ext.as_mapping_mut()) - .for_each(|obj| { - if obj.get("description").is_none_or(|v| v.is_null()) { - obj.insert("description".into(), "".into()); - } - }); - } - - let recipe: Recipe = serde_yaml::from_value(value) - .map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))?; + let recipe: Recipe = match serde_yaml::from_str::(content) { + Ok(yaml_value) => { + if let Some(nested_recipe) = yaml_value.get("recipe") { + serde_yaml::from_value(nested_recipe.clone()) + .map_err(|e| anyhow::anyhow!("Failed to parse nested recipe: {}", e))? + } else { + serde_yaml::from_str(content) + .map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))? + } + } + Err(_) => serde_yaml::from_str(content) + .map_err(|e| anyhow::anyhow!("Failed to parse recipe: {}", e))?, + }; if let Some(ref retry_config) = recipe.retry { if let Err(validation_error) = retry_config.validate() { diff --git a/ui/desktop/src/components/recipes/__tests__/CreateRecipeFromSessionModal.test.tsx b/ui/desktop/src/components/recipes/__tests__/CreateRecipeFromSessionModal.test.tsx index 0d2fe283ff56..64de8f243ad8 100644 --- a/ui/desktop/src/components/recipes/__tests__/CreateRecipeFromSessionModal.test.tsx +++ b/ui/desktop/src/components/recipes/__tests__/CreateRecipeFromSessionModal.test.tsx @@ -146,8 +146,6 @@ describe('CreateRecipeFromSessionModal', () => { expect(screen.getByDisplayValue('Analyzed instructions with {{param1}}')).toBeInTheDocument(); const promptInput = screen.getByTestId('prompt-input'); expect(promptInput).toBeInTheDocument(); - const recipeNameInput = screen.getByTestId('recipe-name-input'); - expect(recipeNameInput).toBeInTheDocument(); }); it('shows recipe form fields after analysis', async () => { @@ -164,21 +162,6 @@ describe('CreateRecipeFromSessionModal', () => { expect(screen.getByTestId('description-input')).toBeInTheDocument(); expect(screen.getByTestId('instructions-input')).toBeInTheDocument(); expect(screen.getByTestId('prompt-input')).toBeInTheDocument(); - expect(screen.getByTestId('recipe-name-input')).toBeInTheDocument(); - }); - - it('shows save location options', async () => { - render(); - - await waitFor( - () => { - expect(screen.getByTestId('save-location-field')).toBeInTheDocument(); - }, - { timeout: 2000 } - ); - - expect(screen.getByTestId('global-radio')).toBeInTheDocument(); - expect(screen.getByTestId('directory-radio')).toBeInTheDocument(); }); }); @@ -201,21 +184,6 @@ describe('CreateRecipeFromSessionModal', () => { expect(screen.getByDisplayValue('Modified Title')).toBeInTheDocument(); }); - it('allows changing save location', async () => { - const user = userEvent.setup(); - render(); - - await waitFor( - () => { - expect(screen.getByTestId('directory-radio')).toBeInTheDocument(); - }, - { timeout: 2000 } - ); - - await user.click(screen.getByTestId('directory-radio')); - expect(screen.getByTestId('directory-radio')).toBeChecked(); - }); - it('validates required fields', async () => { const user = userEvent.setup(); render(); From d68ab75d4a764bbc8a4d18f206da7e658b246199 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 23:41:26 +1100 Subject: [PATCH 15/22] applied more validation on parse and save recipe --- Cargo.lock | 24 +++-- crates/goose-server/Cargo.toml | 1 + crates/goose-server/src/routes/recipe.rs | 78 ++++++++++++++-- crates/goose/src/recipe/validate_recipe.rs | 7 +- ui/desktop/openapi.json | 7 -- ui/desktop/src/api/types.gen.ts | 14 +-- .../components/recipes/ImportRecipeForm.tsx | 24 +++-- .../recipes/shared/recipeFormSchema.ts | 15 +-- ui/desktop/src/recipe/validation.test.ts | 91 +------------------ ui/desktop/src/recipe/validation.ts | 75 --------------- 10 files changed, 117 insertions(+), 219 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3e417bacd754..479b7db5558d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2817,6 +2817,7 @@ dependencies = [ "schemars", "serde", "serde_json", + "serde_path_to_error", "serde_yaml", "tempfile", "thiserror 1.0.69", @@ -5867,18 +5868,28 @@ checksum = "56e6fa9c48d24d85fb3de5ad847117517440f6beceb7798af16b4a87d616b8d0" [[package]] name = "serde" -version = "1.0.218" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" +dependencies = [ + "serde_core", + "serde_derive", +] + +[[package]] +name = "serde_core" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8dfc9d19bdbf6d17e22319da49161d5d0108e4188e8b680aef6299eed22df60" +checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.218" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f09503e191f4e797cb8aac08e9a4a4695c5edf6a2e70e376d961ddd5c969f82b" +checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", @@ -5911,12 +5922,13 @@ dependencies = [ [[package]] name = "serde_path_to_error" -version = "0.1.17" +version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59fab13f937fa393d08645bf3a84bdfe86e296747b506ada67bb15f10f218b2a" +checksum = "10a9ff822e371bb5403e391ecd83e182e0e77ba7f6fe0160b795797109d1b457" dependencies = [ "itoa", "serde", + "serde_core", ] [[package]] diff --git a/crates/goose-server/Cargo.toml b/crates/goose-server/Cargo.toml index 611ed4fdf9fc..eefd68f938fc 100644 --- a/crates/goose-server/Cargo.toml +++ b/crates/goose-server/Cargo.toml @@ -39,6 +39,7 @@ utoipa = { version = "4.1", features = ["axum_extras", "chrono"] } reqwest = { version = "0.12.9", features = ["json", "rustls-tls", "blocking", "multipart"], default-features = false } tokio-util = "0.7.15" uuid = { version = "1.11", features = ["v4"] } +serde_path_to_error = "0.1.20" [[bin]] name = "goosed" diff --git a/crates/goose-server/src/routes/recipe.rs b/crates/goose-server/src/routes/recipe.rs index 53e823650613..6a69e2349443 100644 --- a/crates/goose-server/src/routes/recipe.rs +++ b/crates/goose-server/src/routes/recipe.rs @@ -7,13 +7,36 @@ use axum::extract::rejection::JsonRejection; use axum::routing::get; use axum::{extract::State, http::StatusCode, routing::post, Json, Router}; use goose::recipe::local_recipes; +use goose::recipe::validate_recipe::validate_recipe_template_from_content; use goose::recipe::Recipe; use goose::recipe_deeplink; use goose::session::SessionManager; use serde::{Deserialize, Serialize}; +use serde_json::Value; +use serde_path_to_error::deserialize as deserialize_with_path; use utoipa::ToSchema; +fn format_json_rejection_message(rejection: &JsonRejection) -> String { + match rejection { + JsonRejection::JsonDataError(err) => { + format!("Request body validation failed: {}", clean_data_error(err)) + } + JsonRejection::JsonSyntaxError(err) => format!("Invalid JSON payload: {}", err.body_text()), + JsonRejection::MissingJsonContentType(err) => err.body_text(), + JsonRejection::BytesRejection(err) => err.body_text(), + _ => rejection.body_text(), + } +} + +fn clean_data_error(err: &axum::extract::rejection::JsonDataError) -> String { + let message = err.body_text(); + message + .strip_prefix("Failed to deserialize the JSON body into the target type: ") + .map(|s| s.to_string()) + .unwrap_or_else(|| message.to_string()) +} + use crate::routes::errors::ErrorResponse; use crate::routes::recipe_utils::get_all_recipes_manifests; use crate::state::AppState; @@ -311,9 +334,12 @@ async fn delete_recipe( )] async fn save_recipe( State(state): State>, - payload: Result, JsonRejection>, + payload: Result, JsonRejection>, ) -> Result { - let Json(request) = payload.map_err(json_rejection_to_error_response)?; + let Json(raw_json) = payload.map_err(json_rejection_to_error_response)?; + let request = deserialize_save_recipe_request(raw_json)?; + validate_incoming_recipe(&request.recipe)?; + let file_path = match request.id.as_ref() { Some(id) => Some(get_recipe_file_path_by_id(state.clone(), id).await?), None => None, @@ -330,11 +356,48 @@ async fn save_recipe( fn json_rejection_to_error_response(rejection: JsonRejection) -> ErrorResponse { ErrorResponse { - message: rejection.body_text(), + message: format_json_rejection_message(&rejection), status: StatusCode::BAD_REQUEST, } } +fn validate_incoming_recipe(recipe: &Recipe) -> Result<(), ErrorResponse> { + let recipe_json = serde_json::to_string(recipe).map_err(|err| ErrorResponse { + message: format!("Failed to prepare recipe for validation: {}", err), + status: StatusCode::BAD_REQUEST, + })?; + + validate_recipe_template_from_content(&recipe_json).map_err(|err| ErrorResponse { + message: err.to_string(), + status: StatusCode::BAD_REQUEST, + })?; + + Ok(()) +} + +fn deserialize_save_recipe_request(value: Value) -> Result { + let payload = value.to_string(); + let mut deserializer = serde_json::Deserializer::from_str(&payload); + let result: Result = deserialize_with_path(&mut deserializer); + result.map_err(|err| { + let path = err.path().to_string(); + let inner = err.into_inner(); + let message = if path.is_empty() { + format!("Recipe validation failed: {}", inner) + } else { + format!( + "Request validation failed at {}: {}", + path.trim_start_matches('.'), + inner + ) + }; + ErrorResponse { + message, + status: StatusCode::BAD_REQUEST, + } + }) +} + async fn get_recipe_file_path_by_id( state: Arc, id: &str, @@ -384,10 +447,11 @@ async fn get_recipe_file_path_by_id( async fn parse_recipe( Json(request): Json, ) -> Result, ErrorResponse> { - let recipe = Recipe::from_content(&request.content).map_err(|e| ErrorResponse { - message: format!("Invalid recipe format: {}", e), - status: StatusCode::BAD_REQUEST, - })?; + let recipe = + validate_recipe_template_from_content(&request.content).map_err(|e| ErrorResponse { + message: format!("Invalid recipe format: {}", e), + status: StatusCode::BAD_REQUEST, + })?; Ok(Json(ParseRecipeResponse { recipe })) } diff --git a/crates/goose/src/recipe/validate_recipe.rs b/crates/goose/src/recipe/validate_recipe.rs index 966b2147be7f..e92e685ccd16 100644 --- a/crates/goose/src/recipe/validate_recipe.rs +++ b/crates/goose/src/recipe/validate_recipe.rs @@ -36,11 +36,8 @@ pub fn validate_recipe_template_from_file(recipe_file: &RecipeFile) -> Result, -) -> Result { - validate_recipe_template(recipe_content, recipe_dir) +pub fn validate_recipe_template_from_content(recipe_content: &str) -> Result { + validate_recipe_template(recipe_content, None) } fn validate_recipe_template(recipe_content: &str, recipe_dir: Option) -> Result { diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 39705dd109e7..3733008f685e 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -2338,7 +2338,6 @@ "description": "Server-sent events client with a URI endpoint", "required": [ "name", - "description", "uri", "type" ], @@ -2391,7 +2390,6 @@ "description": "Standard I/O client with command and arguments", "required": [ "name", - "description", "cmd", "args", "type" @@ -2451,7 +2449,6 @@ "description": "Built-in extension that is part of the bundled goose MCP server", "required": [ "name", - "description", "type" ], "properties": { @@ -2495,7 +2492,6 @@ "description": "Platform extensions that have direct access to the agent etc and run in the agent process", "required": [ "name", - "description", "type" ], "properties": { @@ -2529,7 +2525,6 @@ "description": "Streamable HTTP client with a URI endpoint using MCP Streamable HTTP specification", "required": [ "name", - "description", "uri", "type" ], @@ -2588,7 +2583,6 @@ "description": "Frontend-provided tools that will be called through the frontend", "required": [ "name", - "description", "tools", "type" ], @@ -2635,7 +2629,6 @@ "description": "Inline Python code that will be executed using uvx", "required": [ "name", - "description", "code", "type" ], diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index dadaf3b6bd85..a3a35d10459c 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -189,7 +189,7 @@ export type ExtendPromptResponse = { export type ExtensionConfig = { available_tools?: Array; bundled?: boolean | null; - description: string; + description?: string; env_keys?: Array; envs?: Envs; /** @@ -204,7 +204,7 @@ export type ExtensionConfig = { available_tools?: Array; bundled?: boolean | null; cmd: string; - description: string; + description?: string; env_keys?: Array; envs?: Envs; /** @@ -216,7 +216,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description: string; + description?: string; display_name?: string | null; /** * The name used to identify this extension @@ -227,7 +227,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description: string; + description?: string; /** * The name used to identify this extension */ @@ -236,7 +236,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description: string; + description?: string; env_keys?: Array; envs?: Envs; headers?: { @@ -252,7 +252,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description: string; + description?: string; /** * Instructions for how to use these tools */ @@ -276,7 +276,7 @@ export type ExtensionConfig = { * Python package dependencies required by this extension */ dependencies?: Array | null; - description: string; + description?: string; /** * The name used to identify this extension */ diff --git a/ui/desktop/src/components/recipes/ImportRecipeForm.tsx b/ui/desktop/src/components/recipes/ImportRecipeForm.tsx index 0adba1a9830c..c9c4de9f6784 100644 --- a/ui/desktop/src/components/recipes/ImportRecipeForm.tsx +++ b/ui/desktop/src/components/recipes/ImportRecipeForm.tsx @@ -76,13 +76,21 @@ export default function ImportRecipeForm({ isOpen, onClose, onSuccess }: ImportR }; const parseRecipeFromFile = async (fileContent: string): Promise => { - let response = await parseRecipe({ - body: { - content: fileContent, - }, - throwOnError: true, - }); - return response.data.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({ @@ -175,7 +183,7 @@ export default function ImportRecipeForm({ isOpen, onClose, onSuccess }: ImportR } catch (error) { toastError({ title: 'Invalid Recipe File', - msg: `The recipe file format is invalid: ${error instanceof Error ? error.message : 'Unknown error'}`, + msg: error instanceof Error ? error.message : 'Unknown error', }); } } diff --git a/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts b/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts index 99a680bef76b..3eda6f8dedf6 100644 --- a/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts +++ b/ui/desktop/src/components/recipes/shared/recipeFormSchema.ts @@ -1,5 +1,4 @@ import { z } from 'zod'; -import { validateJsonSchema } from '../../../recipe/validation'; // Zod schema for Parameter - matching API RecipeParameter type const parameterSchema = z.object({ @@ -39,19 +38,7 @@ export const recipeFormSchema = z.object({ parameters: z.array(parameterSchema).default([]), - jsonSchema: z - .string() - .optional() - .refine((value) => { - if (!value || !value.trim()) return true; - try { - const parsed = JSON.parse(value.trim()); - const validationResult = validateJsonSchema(parsed); - return validationResult.success; - } catch { - return false; - } - }, 'Invalid JSON schema format'), + jsonSchema: z.string().optional(), }); export type RecipeFormData = z.infer; diff --git a/ui/desktop/src/recipe/validation.test.ts b/ui/desktop/src/recipe/validation.test.ts index 04bb0f3ccb63..b56c322b2294 100644 --- a/ui/desktop/src/recipe/validation.test.ts +++ b/ui/desktop/src/recipe/validation.test.ts @@ -1,96 +1,7 @@ import { describe, it, expect } from 'vitest'; -import { validateJsonSchema, getRecipeJsonSchema } from './validation'; +import { getRecipeJsonSchema } from './validation'; describe('Recipe Validation', () => { - describe('validateJsonSchema', () => { - describe('valid JSON schemas', () => { - it('validates a simple JSON schema', () => { - const schema = { - type: 'object', - properties: { - name: { type: 'string' }, - age: { type: 'number' }, - }, - required: ['name'], - }; - - const result = validateJsonSchema(schema); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - expect(result.data).toEqual(schema); - }); - - it('validates null schema', () => { - const result = validateJsonSchema(null); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - expect(result.data).toBe(null); - }); - - it('validates undefined schema', () => { - const result = validateJsonSchema(undefined); - expect(result.success).toBe(true); - expect(result.errors).toHaveLength(0); - expect(result.data).toBe(undefined); - }); - - it('validates complex JSON schema', () => { - const schema = { - $schema: 'http://json-schema.org/draft-07/schema#', - type: 'object', - properties: { - users: { - type: 'array', - items: { - type: 'object', - properties: { - id: { type: 'number' }, - profile: { - type: 'object', - properties: { - name: { type: 'string' }, - email: { type: 'string' }, - }, - }, - }, - }, - }, - }, - }; - - const result = validateJsonSchema(schema); - expect(result.success).toBe(true); - expect(result.data).toEqual(schema); - }); - }); - - describe('invalid JSON schemas', () => { - it('rejects string input', () => { - const result = validateJsonSchema('not an object'); - expect(result.success).toBe(false); - expect(result.errors).toContain('JSON Schema must be an object'); - }); - - it('rejects number input', () => { - const result = validateJsonSchema(42); - expect(result.success).toBe(false); - expect(result.errors).toContain('JSON Schema must be an object'); - }); - - it('rejects boolean input', () => { - const result = validateJsonSchema(true); - expect(result.success).toBe(false); - expect(result.errors).toContain('JSON Schema must be an object'); - }); - - it('validates array input as valid JSON schema', () => { - const result = validateJsonSchema(['not', 'an', 'object']); - expect(typeof result.success).toBe('boolean'); - expect(Array.isArray(result.errors)).toBe(true); - }); - }); - }); - describe('getRecipeJsonSchema', () => { it('returns a valid JSON schema object', () => { const schema = getRecipeJsonSchema(); diff --git a/ui/desktop/src/recipe/validation.ts b/ui/desktop/src/recipe/validation.ts index 7a507e716b4e..431cb4ce5b03 100644 --- a/ui/desktop/src/recipe/validation.ts +++ b/ui/desktop/src/recipe/validation.ts @@ -1,5 +1,3 @@ -import type { Recipe } from '../api/types.gen'; - /** * OpenAPI-based validation utilities for Recipe objects. * @@ -114,79 +112,6 @@ function resolveRefs( return schema; } -export type RecipeValidationResult = { - success: boolean; - errors: string[]; - data?: Recipe | unknown; -}; - -// TODO: Lifei Remove this -/** - * JSON schema validation for the response.json_schema field. - * Uses basic structural validation instead of AJV to avoid CSP eval security issues. - */ -export function validateJsonSchema(schema: unknown): RecipeValidationResult { - try { - // Allow null/undefined schemas - if (schema === null || schema === undefined) { - return { success: true, errors: [], data: schema as unknown }; - } - - if (typeof schema !== 'object') { - return { - success: false, - errors: ['JSON Schema must be an object'], - data: undefined, - }; - } - - const schemaObj = schema as Record; - const errors: string[] = []; - - // Check for valid JSON Schema structure - if (schemaObj.type && typeof schemaObj.type !== 'string' && !Array.isArray(schemaObj.type)) { - errors.push('Invalid type field: must be a string or array'); - } - - // Check for valid properties structure if it exists - if (schemaObj.properties && typeof schemaObj.properties !== 'object') { - errors.push('Invalid properties field: must be an object'); - } - - // Check for valid required array if it exists - if (schemaObj.required && !Array.isArray(schemaObj.required)) { - errors.push('Invalid required field: must be an array'); - } - - // Check for valid items structure if it exists (for array types) - if (schemaObj.items && typeof schemaObj.items !== 'object' && !Array.isArray(schemaObj.items)) { - errors.push('Invalid items field: must be an object or array'); - } - - if (errors.length > 0) { - return { - success: false, - errors: errors.map((err) => `Invalid JSON Schema: ${err}`), - data: undefined, - }; - } - - return { - success: true, - errors: [], - data: schema as unknown, - }; - } catch (error) { - return { - success: false, - errors: [ - `JSON Schema validation error: ${error instanceof Error ? error.message : 'Unknown error'}`, - ], - data: undefined, - }; - } -} - /** * Returns a JSON schema representation derived directly from the OpenAPI specification. * This schema is used for documentation in form help text. From 5a46123d8ee279e38d58021d5691f182cca115f1 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Tue, 7 Oct 2025 23:51:14 +1100 Subject: [PATCH 16/22] removed recipeStorage --- .../src/components/recipes/RecipesView.tsx | 2 +- .../schedule/CreateScheduleModal.tsx | 2 +- ui/desktop/src/recipe/recipeStorage.ts | 49 ------------------- ui/desktop/src/recipe/recipe_management.ts | 33 ++++++++++++- 4 files changed, 34 insertions(+), 52 deletions(-) delete mode 100644 ui/desktop/src/recipe/recipeStorage.ts diff --git a/ui/desktop/src/components/recipes/RecipesView.tsx b/ui/desktop/src/components/recipes/RecipesView.tsx index f1b052486266..c6b2eba20090 100644 --- a/ui/desktop/src/components/recipes/RecipesView.tsx +++ b/ui/desktop/src/components/recipes/RecipesView.tsx @@ -1,5 +1,5 @@ import { useState, useEffect } from 'react'; -import { listSavedRecipes, convertToLocaleDateString } from '../../recipe/recipeStorage'; +import { listSavedRecipes, convertToLocaleDateString } from '../../recipe/recipe_management'; import { FileText, Edit, Trash2, Play, Calendar, AlertCircle, Link } from 'lucide-react'; import { ScrollArea } from '../ui/scroll-area'; import { Card } from '../ui/card'; diff --git a/ui/desktop/src/components/schedule/CreateScheduleModal.tsx b/ui/desktop/src/components/schedule/CreateScheduleModal.tsx index 855f7b0759da..f0edaa42cf88 100644 --- a/ui/desktop/src/components/schedule/CreateScheduleModal.tsx +++ b/ui/desktop/src/components/schedule/CreateScheduleModal.tsx @@ -6,7 +6,7 @@ import { Select } from '../ui/Select'; import cronstrue from 'cronstrue'; import * as yaml from 'yaml'; import { Recipe, decodeRecipe } from '../../recipe'; -import { getStorageDirectory } from '../../recipe/recipeStorage'; +import { getStorageDirectory } from '../../recipe/recipe_management'; import ClockIcon from '../../assets/clock-icon.svg'; type FrequencyValue = 'once' | 'every' | 'daily' | 'weekly' | 'monthly'; diff --git a/ui/desktop/src/recipe/recipeStorage.ts b/ui/desktop/src/recipe/recipeStorage.ts deleted file mode 100644 index 4483967f64d8..000000000000 --- a/ui/desktop/src/recipe/recipeStorage.ts +++ /dev/null @@ -1,49 +0,0 @@ -import { listRecipes, RecipeManifestResponse } from '../api'; -import { Recipe } from './index'; - -// TODO: Lifei Remove this -export interface SavedRecipe { - name: string; - recipe: Recipe; - isGlobal: boolean; - lastModified: Date; - isArchived?: boolean; - filename: string; // The actual filename used -} - -/** - * Parse a lastModified value that could be a string or Date - */ -function parseLastModified(val: string | Date): Date { - return val instanceof Date ? val : new Date(val); -} - -/** - * Get the storage directory path for recipes - */ -export function getStorageDirectory(isGlobal: boolean): string { - if (isGlobal) { - return '~/.config/goose/recipes'; - } else { - // For directory recipes, build absolute path using working directory - const workingDir = window.appConfig.get('GOOSE_WORKING_DIR') as string; - return `${workingDir}/.goose/recipes`; - } -} - -export async function listSavedRecipes(): Promise { - try { - const listRecipeResponse = await listRecipes(); - return listRecipeResponse?.data?.recipe_manifest_responses ?? []; - } catch (error) { - console.warn('Failed to list saved recipes:', error); - return []; - } -} - -export function convertToLocaleDateString(lastModified: string): string { - if (lastModified) { - return parseLastModified(lastModified).toLocaleDateString(); - } - return ''; -} diff --git a/ui/desktop/src/recipe/recipe_management.ts b/ui/desktop/src/recipe/recipe_management.ts index b4aeef30ffac..2af05824f676 100644 --- a/ui/desktop/src/recipe/recipe_management.ts +++ b/ui/desktop/src/recipe/recipe_management.ts @@ -1,4 +1,4 @@ -import { Recipe, saveRecipe as saveRecipeApi } from '../api'; +import { Recipe, saveRecipe as saveRecipeApi, listRecipes, RecipeManifestResponse } from '../api'; export async function saveRecipe(recipe: Recipe, recipeId?: string | null): Promise { try { @@ -17,3 +17,34 @@ export async function saveRecipe(recipe: Recipe, recipeId?: string | null): Prom throw new Error(`Failed to save recipe: ${error_message}`); } } + +export async function listSavedRecipes(): Promise { + try { + const listRecipeResponse = await listRecipes(); + return listRecipeResponse?.data?.recipe_manifest_responses ?? []; + } catch (error) { + console.warn('Failed to list saved recipes:', error); + return []; + } +} + +function parseLastModified(val: string | Date): Date { + return val instanceof Date ? val : new Date(val); +} + +export function convertToLocaleDateString(lastModified: string): string { + if (lastModified) { + return parseLastModified(lastModified).toLocaleDateString(); + } + return ''; +} + +export function getStorageDirectory(isGlobal: boolean): string { + if (isGlobal) { + return '~/.config/goose/recipes'; + } else { + // For directory recipes, build absolute path using working directory + const workingDir = window.appConfig.get('GOOSE_WORKING_DIR') as string; + return `${workingDir}/.goose/recipes`; + } +} From 6a2cfb2c0555a4416ba0a640f4e06223d3f0816e Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Wed, 8 Oct 2025 00:35:34 +1100 Subject: [PATCH 17/22] deleted tests --- .../shared/__tests__/recipeFormSchema.test.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts b/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts index 2f0fcedfe6c2..2ebd9591bc57 100644 --- a/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts +++ b/ui/desktop/src/components/recipes/shared/__tests__/recipeFormSchema.test.ts @@ -152,16 +152,6 @@ describe('recipeFormSchema', () => { expect(result.success).toBe(true); }); - it('rejects invalid JSON schema', () => { - const invalidData = { ...validFormData, jsonSchema: 'invalid json' }; - const result = recipeFormSchema.safeParse(invalidData); - expect(result.success).toBe(false); - if (!result.success) { - const jsonError = result.error.issues.find((issue) => issue.path.includes('jsonSchema')); - expect(jsonError?.message).toBe('Invalid JSON schema format'); - } - }); - it('allows empty JSON schema', () => { const validData = { ...validFormData, jsonSchema: '' }; const result = recipeFormSchema.safeParse(validData); @@ -285,7 +275,6 @@ describe('recipeFormSchema', () => { title: 'AB', // Too short description: 'Short', // Too short instructions: 'Short', // Too short - jsonSchema: 'invalid json', }; const result = recipeFormSchema.safeParse(invalidData); expect(result.success).toBe(false); @@ -298,7 +287,6 @@ describe('recipeFormSchema', () => { expect(result.error.issues.some((issue) => issue.path.includes('instructions'))).toBe( true ); - expect(result.error.issues.some((issue) => issue.path.includes('jsonSchema'))).toBe(true); } }); }); From 91510ee075fa987dded244e3f7450051ea9144cb Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Wed, 8 Oct 2025 16:02:59 +1100 Subject: [PATCH 18/22] used recipeExtensionConfigInternal deserializer --- crates/goose/src/agents/extension.rs | 15 +- crates/goose/src/recipe/mod.rs | 7 +- .../src/recipe/recipe_extension_adapter.rs | 246 ++++++++++++++++++ ui/desktop/openapi.json | 6 + ui/desktop/src/api/types.gen.ts | 12 +- 5 files changed, 265 insertions(+), 21 deletions(-) create mode 100644 crates/goose/src/recipe/recipe_extension_adapter.rs diff --git a/crates/goose/src/agents/extension.rs b/crates/goose/src/agents/extension.rs index 2659aceab7d1..85ac813db720 100644 --- a/crates/goose/src/agents/extension.rs +++ b/crates/goose/src/agents/extension.rs @@ -9,18 +9,11 @@ use once_cell::sync::Lazy; use rmcp::model::Tool; use rmcp::service::ClientInitializeError; use rmcp::ServiceError as ClientError; -use serde::{Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Serialize}; use thiserror::Error; use tracing::warn; use utoipa::ToSchema; -fn deserialize_string_or_empty<'de, D>(deserializer: D) -> Result -where - D: Deserializer<'de>, -{ - Ok(Option::::deserialize(deserializer)?.unwrap_or_default()) -} - #[derive(Error, Debug)] #[error("process quit before initialization: stderr = {stderr}")] pub struct ProcessExit { @@ -191,7 +184,6 @@ pub enum ExtensionConfig { Sse { /// The name used to identify this extension name: String, - #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, uri: String, #[serde(default)] @@ -211,7 +203,6 @@ pub enum ExtensionConfig { Stdio { /// The name used to identify this extension name: String, - #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, cmd: String, args: Vec, @@ -230,7 +221,6 @@ pub enum ExtensionConfig { Builtin { /// The name used to identify this extension name: String, - #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, display_name: Option, // needed for the UI timeout: Option, @@ -244,7 +234,6 @@ pub enum ExtensionConfig { Platform { /// The name used to identify this extension name: String, - #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, #[serde(default)] bundled: Option, @@ -256,7 +245,6 @@ pub enum ExtensionConfig { StreamableHttp { /// The name used to identify this extension name: String, - #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, uri: String, #[serde(default)] @@ -278,7 +266,6 @@ pub enum ExtensionConfig { Frontend { /// The name used to identify this extension name: String, - #[serde(default, deserialize_with = "deserialize_string_or_empty")] description: String, /// The tools provided by the frontend tools: Vec, diff --git a/crates/goose/src/recipe/mod.rs b/crates/goose/src/recipe/mod.rs index 2373b28bdaa0..9f6e27ce1127 100644 --- a/crates/goose/src/recipe/mod.rs +++ b/crates/goose/src/recipe/mod.rs @@ -15,6 +15,7 @@ use utoipa::ToSchema; pub mod build_recipe; pub mod local_recipes; pub mod read_recipe_file_content; +mod recipe_extension_adapter; pub mod template_recipe; pub mod validate_recipe; @@ -43,7 +44,11 @@ pub struct Recipe { #[serde(skip_serializing_if = "Option::is_none")] pub prompt: Option, // the prompt to start the session with - #[serde(skip_serializing_if = "Option::is_none")] + #[serde( + skip_serializing_if = "Option::is_none", + default, + deserialize_with = "recipe_extension_adapter::deserialize_recipe_extensions" + )] pub extensions: Option>, // a list of extensions to enable #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/goose/src/recipe/recipe_extension_adapter.rs b/crates/goose/src/recipe/recipe_extension_adapter.rs new file mode 100644 index 000000000000..86559ec6884e --- /dev/null +++ b/crates/goose/src/recipe/recipe_extension_adapter.rs @@ -0,0 +1,246 @@ +use crate::agents::extension::{Envs, ExtensionConfig}; +use rmcp::model::Tool; +use serde::de::Deserializer; +use serde::Deserialize; +use std::collections::HashMap; + +#[derive(Deserialize)] +#[serde(tag = "type")] +enum RecipeExtensionConfigInternal { + #[serde(rename = "sse")] + Sse { + name: String, + #[serde(default)] + description: Option, + uri: String, + #[serde(default)] + envs: Envs, + #[serde(default)] + env_keys: Vec, + timeout: Option, + #[serde(default)] + bundled: Option, + #[serde(default)] + available_tools: Vec, + }, + #[serde(rename = "stdio")] + Stdio { + name: String, + #[serde(default)] + description: Option, + cmd: String, + args: Vec, + #[serde(default)] + envs: Envs, + #[serde(default)] + env_keys: Vec, + timeout: Option, + #[serde(default)] + bundled: Option, + #[serde(default)] + available_tools: Vec, + }, + #[serde(rename = "builtin")] + Builtin { + name: String, + #[serde(default)] + description: Option, + display_name: Option, + timeout: Option, + #[serde(default)] + bundled: Option, + #[serde(default)] + available_tools: Vec, + }, + #[serde(rename = "platform")] + Platform { + name: String, + #[serde(default)] + description: Option, + #[serde(default)] + bundled: Option, + #[serde(default)] + available_tools: Vec, + }, + #[serde(rename = "streamable_http")] + StreamableHttp { + name: String, + #[serde(default)] + description: Option, + uri: String, + #[serde(default)] + envs: Envs, + #[serde(default)] + env_keys: Vec, + #[serde(default)] + headers: HashMap, + timeout: Option, + #[serde(default)] + bundled: Option, + #[serde(default)] + available_tools: Vec, + }, + #[serde(rename = "frontend")] + Frontend { + name: String, + #[serde(default)] + description: Option, + tools: Vec, + instructions: Option, + #[serde(default)] + bundled: Option, + #[serde(default)] + available_tools: Vec, + }, + #[serde(rename = "inline_python")] + InlinePython { + name: String, + #[serde(default)] + description: Option, + code: String, + timeout: Option, + #[serde(default)] + dependencies: Option>, + #[serde(default)] + available_tools: Vec, + }, +} + +impl From for ExtensionConfig { + fn from(internal_variant: RecipeExtensionConfigInternal) -> Self { + match internal_variant { + RecipeExtensionConfigInternal::Sse { + name, + description, + uri, + envs, + env_keys, + timeout, + bundled, + available_tools, + } => ExtensionConfig::Sse { + name, + description: description.unwrap_or_default(), + uri, + envs, + env_keys, + timeout, + bundled, + available_tools, + }, + RecipeExtensionConfigInternal::Stdio { + name, + description, + cmd, + args, + envs, + env_keys, + timeout, + bundled, + available_tools, + } => ExtensionConfig::Stdio { + name, + description: description.unwrap_or_default(), + cmd, + args, + envs, + env_keys, + timeout, + bundled, + available_tools, + }, + RecipeExtensionConfigInternal::Builtin { + name, + description, + display_name, + timeout, + bundled, + available_tools, + } => ExtensionConfig::Builtin { + name, + description: description.unwrap_or_default(), + display_name, + timeout, + bundled, + available_tools, + }, + RecipeExtensionConfigInternal::Platform { + name, + description, + bundled, + available_tools, + } => ExtensionConfig::Platform { + name, + description: description.unwrap_or_default(), + bundled, + available_tools, + }, + RecipeExtensionConfigInternal::StreamableHttp { + name, + description, + uri, + envs, + env_keys, + headers, + timeout, + bundled, + available_tools, + } => ExtensionConfig::StreamableHttp { + name, + description: description.unwrap_or_default(), + uri, + envs, + env_keys, + headers, + timeout, + bundled, + available_tools, + }, + RecipeExtensionConfigInternal::Frontend { + name, + description, + tools, + instructions, + bundled, + available_tools, + } => ExtensionConfig::Frontend { + name, + description: description.unwrap_or_default(), + tools, + instructions, + bundled, + available_tools, + }, + RecipeExtensionConfigInternal::InlinePython { + name, + description, + code, + timeout, + dependencies, + available_tools, + } => ExtensionConfig::InlinePython { + name, + description: description.unwrap_or_default(), + code, + timeout, + dependencies, + available_tools, + }, + } + } +} + +pub fn deserialize_recipe_extensions<'de, D>( + deserializer: D, +) -> Result>, D::Error> +where + D: Deserializer<'de>, +{ + let remotes = Option::>::deserialize(deserializer)?; + Ok(remotes.map(|items| { + items + .into_iter() + .map(ExtensionConfig::from) + .collect::>() + })) +} diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index 3733008f685e..c9d780238539 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -2338,6 +2338,7 @@ "description": "Server-sent events client with a URI endpoint", "required": [ "name", + "description", "uri", "type" ], @@ -2390,6 +2391,7 @@ "description": "Standard I/O client with command and arguments", "required": [ "name", + "description", "cmd", "args", "type" @@ -2449,6 +2451,7 @@ "description": "Built-in extension that is part of the bundled goose MCP server", "required": [ "name", + "description", "type" ], "properties": { @@ -2492,6 +2495,7 @@ "description": "Platform extensions that have direct access to the agent etc and run in the agent process", "required": [ "name", + "description", "type" ], "properties": { @@ -2525,6 +2529,7 @@ "description": "Streamable HTTP client with a URI endpoint using MCP Streamable HTTP specification", "required": [ "name", + "description", "uri", "type" ], @@ -2583,6 +2588,7 @@ "description": "Frontend-provided tools that will be called through the frontend", "required": [ "name", + "description", "tools", "type" ], diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index a3a35d10459c..f5053da507cd 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -189,7 +189,7 @@ export type ExtendPromptResponse = { export type ExtensionConfig = { available_tools?: Array; bundled?: boolean | null; - description?: string; + description: string; env_keys?: Array; envs?: Envs; /** @@ -204,7 +204,7 @@ export type ExtensionConfig = { available_tools?: Array; bundled?: boolean | null; cmd: string; - description?: string; + description: string; env_keys?: Array; envs?: Envs; /** @@ -216,7 +216,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description?: string; + description: string; display_name?: string | null; /** * The name used to identify this extension @@ -227,7 +227,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description?: string; + description: string; /** * The name used to identify this extension */ @@ -236,7 +236,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description?: string; + description: string; env_keys?: Array; envs?: Envs; headers?: { @@ -252,7 +252,7 @@ export type ExtensionConfig = { } | { available_tools?: Array; bundled?: boolean | null; - description?: string; + description: string; /** * Instructions for how to use these tools */ From 726f6756bf4752293f291191dc4609df7d676078 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Wed, 8 Oct 2025 16:24:21 +1100 Subject: [PATCH 19/22] keep the recipe extensions even they are not defined in the config --- .../recipes/CreateEditRecipeModal.tsx | 57 +++---------------- 1 file changed, 7 insertions(+), 50 deletions(-) diff --git a/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx b/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx index 3a02381be0bf..3f02f062e024 100644 --- a/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx +++ b/ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx @@ -4,8 +4,7 @@ import { Recipe, generateDeepLink, Parameter } from '../../recipe'; import { Geese } from '../icons/Geese'; import Copy from '../icons/Copy'; import { Check, Save, Calendar, X, Play } from 'lucide-react'; -import { ExtensionConfig, useConfig } from '../ConfigContext'; -import { FixedExtensionEntry } from '../ConfigContext'; +import { ExtensionConfig } from '../ConfigContext'; import { ScheduleFromRecipeModal } from '../schedule/ScheduleFromRecipeModal'; import { Button } from '../ui/button'; @@ -29,8 +28,6 @@ export default function CreateEditRecipeModal({ isCreateMode = false, recipeId, }: CreateEditRecipeModalProps) { - const { getExtensions } = useConfig(); - const getInitialValues = React.useCallback((): RecipeFormData => { if (recipe) { return { @@ -81,16 +78,14 @@ export default function CreateEditRecipeModal({ setJsonSchema(form.state.values.jsonSchema); }); }, [form]); - const [extensionOptions, setExtensionOptions] = useState([]); - const [extensionsLoaded, setExtensionsLoaded] = useState(false); const [copied, setCopied] = useState(false); const [isScheduleModalOpen, setIsScheduleModalOpen] = useState(false); const [isSaving, setIsSaving] = useState(false); // Initialize selected extensions for the recipe - const [recipeExtensions] = useState(() => { + const [recipeExtensions] = useState(() => { if (recipe?.extensions) { - return recipe.extensions.map((ext) => ext.name); + return recipe.extensions; } return []; }); @@ -103,31 +98,6 @@ export default function CreateEditRecipeModal({ } }, [recipe, form, getInitialValues]); - // Load extensions when modal opens - useEffect(() => { - if (isOpen && !extensionsLoaded) { - const loadExtensions = async () => { - try { - const extensions = await getExtensions(false); - console.log('Loading extensions for recipe modal'); - - if (extensions && extensions.length > 0) { - const initializedExtensions = extensions.map((ext) => ({ - ...ext, - enabled: recipeExtensions.includes(ext.name), - })); - - setExtensionOptions(initializedExtensions); - setExtensionsLoaded(true); - } - } catch (error) { - console.error('Failed to load extensions:', error); - } - }; - loadExtensions(); - } - }, [isOpen, getExtensions, recipeExtensions, extensionsLoaded]); - const getCurrentRecipe = useCallback((): Recipe => { // Transform the internal parameters state into the desired output format. const formattedParameters = parameters.map((param) => { @@ -172,22 +142,10 @@ export default function CreateEditRecipeModal({ prompt: prompt || undefined, parameters: formattedParameters, response: responseConfig, - extensions: recipeExtensions - .map((name) => { - const extension = extensionOptions.find((e) => e.name === name); - if (!extension) return null; - - // Create a clean copy of the extension configuration - const { enabled: _enabled, ...cleanExtension } = extension; - // Remove legacy envs which could potentially include secrets - if ('envs' in cleanExtension) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const { envs: _envs, ...finalExtension } = cleanExtension as any; - return finalExtension; - } - return cleanExtension; - }) - .filter(Boolean) as ExtensionConfig[], + // Strip envs to avoid leaking secrets + extensions: recipeExtensions.map((extension) => + 'envs' in extension ? { ...extension, envs: undefined } : extension + ) as ExtensionConfig[], }; }, [ recipe, @@ -199,7 +157,6 @@ export default function CreateEditRecipeModal({ parameters, jsonSchema, recipeExtensions, - extensionOptions, ]); const requiredFieldsAreFilled = () => { From 5b0399f0a6bdc70e59fe8c8dc39d279d9f4fbb44 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Wed, 8 Oct 2025 19:12:23 +1100 Subject: [PATCH 20/22] clean up error message and make the adapter concise --- crates/goose-cli/src/commands/recipe.rs | 10 +- crates/goose-server/src/routes/recipe.rs | 10 +- crates/goose/src/agents/extension.rs | 1 - crates/goose/src/recipe/local_recipes.rs | 12 +- .../src/recipe/recipe_extension_adapter.rs | 209 +++++++++++------- ui/desktop/openapi.json | 1 + ui/desktop/src/api/types.gen.ts | 2 +- ui/desktop/src/recipe/recipe_management.ts | 2 +- 8 files changed, 144 insertions(+), 103 deletions(-) diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index 47d435e923fa..b3220c85428a 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -18,13 +18,9 @@ use goose::recipe_deeplink; pub fn handle_validate(recipe_name: &str) -> Result<()> { // Load and validate the recipe file let recipe_file = load_recipe_file(recipe_name)?; - match validate_recipe_template_from_file(&recipe_file) { - Ok(_) => { - println!("{} recipe file is valid", style("✓").green().bold()); - Ok(()) - } - Err(err) => Err(err), - } + validate_recipe_template_from_file(&recipe_file)?; + println!("{} recipe file is valid", style("✓").green().bold()); + Ok(()) } /// Generates a deeplink for a recipe file diff --git a/crates/goose-server/src/routes/recipe.rs b/crates/goose-server/src/routes/recipe.rs index 6a69e2349443..71010da0e774 100644 --- a/crates/goose-server/src/routes/recipe.rs +++ b/crates/goose-server/src/routes/recipe.rs @@ -338,7 +338,7 @@ async fn save_recipe( ) -> Result { let Json(raw_json) = payload.map_err(json_rejection_to_error_response)?; let request = deserialize_save_recipe_request(raw_json)?; - validate_incoming_recipe(&request.recipe)?; + validate_recipe(&request.recipe)?; let file_path = match request.id.as_ref() { Some(id) => Some(get_recipe_file_path_by_id(state.clone(), id).await?), @@ -361,9 +361,9 @@ fn json_rejection_to_error_response(rejection: JsonRejection) -> ErrorResponse { } } -fn validate_incoming_recipe(recipe: &Recipe) -> Result<(), ErrorResponse> { +fn validate_recipe(recipe: &Recipe) -> Result<(), ErrorResponse> { let recipe_json = serde_json::to_string(recipe).map_err(|err| ErrorResponse { - message: format!("Failed to prepare recipe for validation: {}", err), + message: err.to_string(), status: StatusCode::BAD_REQUEST, })?; @@ -383,10 +383,10 @@ fn deserialize_save_recipe_request(value: Value) -> Result Result> { if path.is_file() { if let Some(extension) = path.extension() { if RECIPE_FILE_EXTENSIONS.contains(&extension.to_string_lossy().as_ref()) { - if let Ok(recipe) = Recipe::from_file_path(&path) { - recipes.push((path.clone(), recipe)); + match Recipe::from_file_path(&path) { + Ok(recipe) => recipes.push((path.clone(), recipe)), + Err(e) => { + let error_message = format!( + "Failed to load recipe from file {}: {}", + path.display(), + e + ); + tracing::error!("{}", error_message); + } } } } diff --git a/crates/goose/src/recipe/recipe_extension_adapter.rs b/crates/goose/src/recipe/recipe_extension_adapter.rs index 86559ec6884e..c9c538ca6793 100644 --- a/crates/goose/src/recipe/recipe_extension_adapter.rs +++ b/crates/goose/src/recipe/recipe_extension_adapter.rs @@ -106,127 +106,77 @@ enum RecipeExtensionConfigInternal { }, } +macro_rules! map_recipe_extensions { + ($value:expr; $( $variant:ident { $( $field:ident ),* $(,)? } ),+ $(,)?) => {{ + match $value { + $( + RecipeExtensionConfigInternal::$variant { + name, + description, + $( $field ),* + } => ExtensionConfig::$variant { + name, + description: description.unwrap_or_default(), + $( $field ),* + }, + )+ + } + }}; +} + impl From for ExtensionConfig { fn from(internal_variant: RecipeExtensionConfigInternal) -> Self { - match internal_variant { - RecipeExtensionConfigInternal::Sse { - name, - description, - uri, - envs, - env_keys, - timeout, - bundled, - available_tools, - } => ExtensionConfig::Sse { - name, - description: description.unwrap_or_default(), + map_recipe_extensions!( + internal_variant; + Sse { uri, envs, env_keys, timeout, bundled, - available_tools, + available_tools }, - RecipeExtensionConfigInternal::Stdio { - name, - description, - cmd, - args, - envs, - env_keys, - timeout, - bundled, - available_tools, - } => ExtensionConfig::Stdio { - name, - description: description.unwrap_or_default(), + Stdio { cmd, args, envs, env_keys, timeout, bundled, - available_tools, + available_tools }, - RecipeExtensionConfigInternal::Builtin { - name, - description, - display_name, - timeout, - bundled, - available_tools, - } => ExtensionConfig::Builtin { - name, - description: description.unwrap_or_default(), + Builtin { display_name, timeout, bundled, - available_tools, + available_tools }, - RecipeExtensionConfigInternal::Platform { - name, - description, + Platform { bundled, - available_tools, - } => ExtensionConfig::Platform { - name, - description: description.unwrap_or_default(), - bundled, - available_tools, + available_tools }, - RecipeExtensionConfigInternal::StreamableHttp { - name, - description, + StreamableHttp { uri, envs, env_keys, headers, timeout, bundled, - available_tools, - } => ExtensionConfig::StreamableHttp { - name, - description: description.unwrap_or_default(), - uri, - envs, - env_keys, - headers, - timeout, - bundled, - available_tools, + available_tools }, - RecipeExtensionConfigInternal::Frontend { - name, - description, - tools, - instructions, - bundled, - available_tools, - } => ExtensionConfig::Frontend { - name, - description: description.unwrap_or_default(), + Frontend { tools, instructions, bundled, - available_tools, + available_tools }, - RecipeExtensionConfigInternal::InlinePython { - name, - description, + InlinePython { code, timeout, dependencies, - available_tools, - } => ExtensionConfig::InlinePython { - name, - description: description.unwrap_or_default(), - code, - timeout, - dependencies, - available_tools, - }, - } + available_tools + } + ) } } @@ -244,3 +194,90 @@ where .collect::>() })) } + +#[cfg(test)] +mod tests { + use super::*; + use serde::Deserialize; + use serde_json::json; + + #[derive(Deserialize)] + struct Wrapper { + #[serde(deserialize_with = "deserialize_recipe_extensions")] + extensions: Option>, + } + + #[test] + fn builtin_extension_defaults_description() { + let wrapper: Wrapper = serde_json::from_value(json!({ + "extensions": [{ + "type": "builtin", + "name": "test-builtin", + "display_name": "Test Builtin", + "timeout": 120, + "bundled": true, + "available_tools": ["tool_a", "tool_b"], + }] + })) + .expect("failed to deserialize extensions"); + + let extensions = wrapper.extensions.expect("expected extensions"); + assert_eq!(extensions.len(), 1); + + match &extensions[0] { + ExtensionConfig::Builtin { + name, + description, + display_name, + timeout, + bundled, + available_tools, + } => { + assert_eq!(name, "test-builtin"); + assert_eq!(description, ""); + assert_eq!(display_name.as_deref(), Some("Test Builtin")); + assert_eq!(*timeout, Some(120)); + assert_eq!(*bundled, Some(true)); + assert_eq!( + available_tools, + &vec!["tool_a".to_string(), "tool_b".to_string()] + ); + } + other => panic!("unexpected extension variant: {:?}", other), + } + } + + #[test] + fn builtin_extension_null_description_defaults_to_empty() { + let wrapper: Wrapper = serde_json::from_value(json!({ + "extensions": [{ + "type": "builtin", + "name": "null-description-builtin", + "description": null, + }] + })) + .expect("failed to deserialize extensions with null description"); + + let extensions = wrapper.extensions.expect("expected extensions"); + assert_eq!(extensions.len(), 1); + + match &extensions[0] { + ExtensionConfig::Builtin { + name, + description, + display_name, + timeout, + bundled, + available_tools, + } => { + assert_eq!(name, "null-description-builtin"); + assert_eq!(description, ""); + assert!(display_name.is_none()); + assert!(timeout.is_none()); + assert!(bundled.is_none()); + assert!(available_tools.is_empty()); + } + other => panic!("unexpected extension variant: {:?}", other), + } + } +} diff --git a/ui/desktop/openapi.json b/ui/desktop/openapi.json index c9d780238539..39705dd109e7 100644 --- a/ui/desktop/openapi.json +++ b/ui/desktop/openapi.json @@ -2635,6 +2635,7 @@ "description": "Inline Python code that will be executed using uvx", "required": [ "name", + "description", "code", "type" ], diff --git a/ui/desktop/src/api/types.gen.ts b/ui/desktop/src/api/types.gen.ts index f5053da507cd..dadaf3b6bd85 100644 --- a/ui/desktop/src/api/types.gen.ts +++ b/ui/desktop/src/api/types.gen.ts @@ -276,7 +276,7 @@ export type ExtensionConfig = { * Python package dependencies required by this extension */ dependencies?: Array | null; - description?: string; + description: string; /** * The name used to identify this extension */ diff --git a/ui/desktop/src/recipe/recipe_management.ts b/ui/desktop/src/recipe/recipe_management.ts index 2af05824f676..35083f44ec45 100644 --- a/ui/desktop/src/recipe/recipe_management.ts +++ b/ui/desktop/src/recipe/recipe_management.ts @@ -14,7 +14,7 @@ export async function saveRecipe(recipe: Recipe, recipeId?: string | null): Prom if (typeof error === 'object' && error !== null && 'message' in error) { error_message = error.message as string; } - throw new Error(`Failed to save recipe: ${error_message}`); + throw new Error(error_message); } } From 2ab1a90e293d0ff0544277ed6188d5f9a9764309 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 9 Oct 2025 15:02:00 +1100 Subject: [PATCH 21/22] added validation and show error message --- crates/goose-cli/src/commands/recipe.rs | 63 +++++-------------- crates/goose-server/src/routes/recipe.rs | 9 +-- crates/goose/src/recipe/build_recipe/mod.rs | 9 ++- crates/goose/src/recipe/build_recipe/tests.rs | 21 +++++++ crates/goose/src/recipe/validate_recipe.rs | 33 ++++++++-- 5 files changed, 73 insertions(+), 62 deletions(-) diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index b3220c85428a..ecc67a8c96dc 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -6,32 +6,25 @@ use crate::recipes::github_recipe::RecipeSource; use crate::recipes::search_recipe::{list_available_recipes, load_recipe_file}; use goose::recipe_deeplink; -/// Validates a recipe file -/// -/// # Arguments -/// -/// * `file_path` - Path to the recipe file to validate -/// -/// # Returns -/// -/// Result indicating success or failure pub fn handle_validate(recipe_name: &str) -> Result<()> { // Load and validate the recipe file let recipe_file = load_recipe_file(recipe_name)?; - validate_recipe_template_from_file(&recipe_file)?; - println!("{} recipe file is valid", style("✓").green().bold()); - Ok(()) + match validate_recipe_template_from_file(&recipe_file) { + Ok(_) => { + println!("{} recipe file is valid", style("✓").green().bold()); + Ok(()) + } + Err(err) => { + println!( + "{} recipe file is invalid: {}", + style("✗").red().bold(), + err + ); + Ok(()) + } + } } -/// Generates a deeplink for a recipe file -/// -/// # Arguments -/// -/// * `recipe_name` - Path to the recipe file -/// -/// # Returns -/// -/// Result indicating success or failure pub fn handle_deeplink(recipe_name: &str) -> Result { match generate_deeplink(recipe_name) { Ok((deeplink_url, recipe)) => { @@ -54,15 +47,6 @@ pub fn handle_deeplink(recipe_name: &str) -> Result { } } -/// Opens a recipe in Goose Desktop -/// -/// # Arguments -/// -/// * `recipe_name` - Path to the recipe file -/// -/// # Returns -/// -/// Result indicating success or failure pub fn handle_open(recipe_name: &str) -> Result<()> { // Generate the deeplink using the helper function (no printing) // This reuses all the validation and encoding logic @@ -101,16 +85,6 @@ pub fn handle_open(recipe_name: &str) -> Result<()> { } } -/// Lists all available recipes from local paths and GitHub repositories -/// -/// # Arguments -/// -/// * `format` - Output format ("text" or "json") -/// * `verbose` - Whether to show detailed information -/// -/// # Returns -/// -/// Result indicating success or failure pub fn handle_list(format: &str, verbose: bool) -> Result<()> { let recipes = match list_available_recipes() { Ok(recipes) => recipes, @@ -162,15 +136,6 @@ pub fn handle_list(format: &str, verbose: bool) -> Result<()> { Ok(()) } -/// Helper function to generate a deeplink -/// -/// # Arguments -/// -/// * `recipe_name` - Path to the recipe file -/// -/// # Returns -/// -/// Result containing the deeplink URL and recipe fn generate_deeplink(recipe_name: &str) -> Result<(String, goose::recipe::Recipe)> { let recipe_file = load_recipe_file(recipe_name)?; // Load the recipe file first to validate it diff --git a/crates/goose-server/src/routes/recipe.rs b/crates/goose-server/src/routes/recipe.rs index 71010da0e774..1c6da4dbbbcc 100644 --- a/crates/goose-server/src/routes/recipe.rs +++ b/crates/goose-server/src/routes/recipe.rs @@ -367,7 +367,7 @@ fn validate_recipe(recipe: &Recipe) -> Result<(), ErrorResponse> { status: StatusCode::BAD_REQUEST, })?; - validate_recipe_template_from_content(&recipe_json).map_err(|err| ErrorResponse { + validate_recipe_template_from_content(&recipe_json, None).map_err(|err| ErrorResponse { message: err.to_string(), status: StatusCode::BAD_REQUEST, })?; @@ -447,11 +447,12 @@ async fn get_recipe_file_path_by_id( async fn parse_recipe( Json(request): Json, ) -> Result, ErrorResponse> { - let recipe = - validate_recipe_template_from_content(&request.content).map_err(|e| ErrorResponse { + let recipe = validate_recipe_template_from_content(&request.content, None).map_err(|e| { + ErrorResponse { message: format!("Invalid recipe format: {}", e), status: StatusCode::BAD_REQUEST, - })?; + } + })?; Ok(Json(ParseRecipeResponse { recipe })) } diff --git a/crates/goose/src/recipe/build_recipe/mod.rs b/crates/goose/src/recipe/build_recipe/mod.rs index 8fa26fab0d84..05b23ff750f2 100644 --- a/crates/goose/src/recipe/build_recipe/mod.rs +++ b/crates/goose/src/recipe/build_recipe/mod.rs @@ -1,6 +1,6 @@ use crate::recipe::read_recipe_file_content::{read_parameter_file_content, RecipeFile}; use crate::recipe::template_recipe::render_recipe_content_with_params; -use crate::recipe::validate_recipe::validate_recipe_parameters; +use crate::recipe::validate_recipe::validate_recipe_template_from_content; use crate::recipe::{ Recipe, RecipeParameter, RecipeParameterInputType, RecipeParameterRequirement, BUILT_IN_RECIPE_DIR_PARAM, @@ -35,8 +35,11 @@ where let recipe_dir_str = recipe_parent_dir .to_str() .ok_or_else(|| anyhow::anyhow!("Error getting recipe directory"))?; - let recipe_parameters = - validate_recipe_parameters(&recipe_file_content, Some(recipe_dir_str.to_string()))?; + let recipe_parameters = validate_recipe_template_from_content( + &recipe_file_content, + Some(recipe_dir_str.to_string()), + )? + .parameters; let (params_for_template, missing_params) = apply_values_to_parameters(¶ms, recipe_parameters, recipe_dir_str, user_prompt_fn)?; diff --git a/crates/goose/src/recipe/build_recipe/tests.rs b/crates/goose/src/recipe/build_recipe/tests.rs index e84279186675..f38240c8ca4b 100644 --- a/crates/goose/src/recipe/build_recipe/tests.rs +++ b/crates/goose/src/recipe/build_recipe/tests.rs @@ -303,6 +303,27 @@ fn test_build_recipe_from_template_success_without_parameters() { assert!(recipe.parameters.is_none()); } +#[test] +fn test_build_recipe_from_template_missing_prompt_and_instructions() { + let instructions_and_parameters = ""; + let (_temp_dir, recipe_file) = setup_recipe_file(instructions_and_parameters); + + let build_recipe_result = build_recipe_from_template(recipe_file, Vec::new(), NO_USER_PROMPT); + assert!(build_recipe_result.is_err()); + let err = build_recipe_result.unwrap_err(); + println!("{}", err); + + match err { + RecipeError::TemplateRendering { source } => { + let err_str = source.to_string(); + assert!( + err_str.contains("Recipe must specify at least one of `instructions` or `prompt`.") + ); + } + _ => panic!("Expected TemplateRendering error"), + } +} + #[test] fn test_template_inheritance() { let parent_content = r#" diff --git a/crates/goose/src/recipe/validate_recipe.rs b/crates/goose/src/recipe/validate_recipe.rs index e92e685ccd16..32c62057b04b 100644 --- a/crates/goose/src/recipe/validate_recipe.rs +++ b/crates/goose/src/recipe/validate_recipe.rs @@ -33,17 +33,17 @@ pub fn validate_recipe_template_from_file(recipe_file: &RecipeFile) -> Result Result { - validate_recipe_template(recipe_content, None) -} - -fn validate_recipe_template(recipe_content: &str, recipe_dir: Option) -> Result { +pub fn validate_recipe_template_from_content( + recipe_content: &str, + recipe_dir: Option, +) -> Result { validate_recipe_parameters(recipe_content, recipe_dir.clone())?; let recipe = render_recipe_for_preview(recipe_content, recipe_dir, &HashMap::new())?; + validate_prompt_or_instructions(&recipe)?; if let Some(response) = &recipe.response { if let Some(json_schema) = &response.json_schema { validate_json_schema(json_schema)?; @@ -53,6 +53,27 @@ fn validate_recipe_template(recipe_content: &str, recipe_dir: Option) -> Ok(recipe) } +fn validate_prompt_or_instructions(recipe: &Recipe) -> Result<()> { + let has_instructions = recipe + .instructions + .as_ref() + .map(|value| !value.trim().is_empty()) + .unwrap_or(false); + let has_prompt = recipe + .prompt + .as_ref() + .map(|value| !value.trim().is_empty()) + .unwrap_or(false); + + if has_instructions || has_prompt { + return Ok(()); + } + + Err(anyhow::anyhow!( + "Recipe must specify at least one of `instructions` or `prompt`." + )) +} + fn validate_parameters_in_template( recipe_parameters: &Option>, template_variables: &HashSet, From f42e8e2b95028197e50f44dd665c96fd73728680 Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Thu, 9 Oct 2025 15:34:44 +1100 Subject: [PATCH 22/22] fixed test --- crates/goose-cli/src/commands/recipe.rs | 54 +++++-------------------- 1 file changed, 9 insertions(+), 45 deletions(-) diff --git a/crates/goose-cli/src/commands/recipe.rs b/crates/goose-cli/src/commands/recipe.rs index ecc67a8c96dc..caeeddeaa63c 100644 --- a/crates/goose-cli/src/commands/recipe.rs +++ b/crates/goose-cli/src/commands/recipe.rs @@ -9,20 +9,15 @@ use goose::recipe_deeplink; pub fn handle_validate(recipe_name: &str) -> Result<()> { // Load and validate the recipe file let recipe_file = load_recipe_file(recipe_name)?; - match validate_recipe_template_from_file(&recipe_file) { - Ok(_) => { - println!("{} recipe file is valid", style("✓").green().bold()); - Ok(()) - } - Err(err) => { - println!( - "{} recipe file is invalid: {}", - style("✗").red().bold(), - err - ); - Ok(()) - } - } + validate_recipe_template_from_file(&recipe_file).map_err(|err| { + anyhow::anyhow!( + "{} recipe file is invalid: {}", + style("✗").red().bold(), + err + ) + })?; + println!("{} recipe file is valid", style("✓").green().bold()); + Ok(()) } pub fn handle_deeplink(recipe_name: &str) -> Result { @@ -187,20 +182,6 @@ prompt: "Test prompt content {{ name }}" instructions: "Test instructions" "#; - const RECIPE_WITH_INVALID_JSON_SCHEMA: &str = r#" -title: "Test Recipe with Invalid JSON Schema" -description: "A test recipe with invalid JSON schema" -prompt: "Test prompt content" -instructions: "Test instructions" -response: - json_schema: - type: invalid_type - properties: - result: - type: unknown_type - required: "should_be_array_not_string" -"#; - #[test] fn test_handle_deeplink_valid_recipe() { let temp_dir = TempDir::new().expect("Failed to create temp directory"); @@ -265,23 +246,6 @@ response: assert!(result.is_err()); } - #[test] - fn test_handle_validation_recipe_with_invalid_json_schema() { - let temp_dir = TempDir::new().expect("Failed to create temp directory"); - let recipe_path = create_test_recipe_file( - &temp_dir, - "test_recipe.yaml", - RECIPE_WITH_INVALID_JSON_SCHEMA, - ); - - let result = handle_validate(&recipe_path); - assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("JSON schema validation failed")); - } - #[test] fn test_generate_deeplink_valid_recipe() { let temp_dir = TempDir::new().expect("Failed to create temp directory");