Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, { useState, useEffect, useCallback } from 'react';
import { useForm } from '@tanstack/react-form';
import { Recipe, generateDeepLink, Parameter } from '../../recipe';
import { Recipe, generateDeepLink, Parameter, stripEmptyExtensions } from '../../recipe';
import { Check, ExternalLink, Play, Save, X } from 'lucide-react';
import { Geese } from '../icons/Geese';
import Copy from '../icons/Copy';
Expand Down Expand Up @@ -132,7 +132,14 @@ export default function CreateEditRecipeModal({
}
}

return {
// Strip envs to avoid leaking secrets
const extensionsWithoutEnvs = recipeExtensions.map((extension) =>
'envs' in extension ? { ...extension, envs: undefined } : extension
) as ExtensionConfig[];

// Use stripEmptyExtensions to avoid saving empty extensions array
// Empty extensions array would prevent default extensions from loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do we? we do want to make the distinction between empty lists and no recipes. if you now load a recipe that pre-existed and had an empty list of extensions, and you save it, it now has no extensions.

you want to make sure that if we create a recipe the default extensions are undefined and then respect whatever it said already

return stripEmptyExtensions({
...recipe,
title,
description,
Expand All @@ -141,11 +148,8 @@ export default function CreateEditRecipeModal({
prompt: prompt || undefined,
parameters: formattedParameters,
response: responseConfig,
// Strip envs to avoid leaking secrets
extensions: recipeExtensions.map((extension) =>
'envs' in extension ? { ...extension, envs: undefined } : extension
) as ExtensionConfig[],
};
extensions: extensionsWithoutEnvs,
}) as Recipe;
}, [
recipe,
title,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ export default function CreateRecipeFromSessionModal({
setIsCreating(true);
try {
// Create the recipe object from form data
// Don't set extensions - let the recipe use default enabled extensions

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

typical LLM comment

const recipe: Recipe = {
title: formData.title,
description: formData.description,
Expand All @@ -181,7 +182,6 @@ export default function CreateRecipeFromSessionModal({
json_schema: JSON.parse(formData.jsonSchema),
}
: undefined,
extensions: [], // Will be populated based on current extensions
};

let recipeId = await saveRecipe(recipe, null);
Expand Down
8 changes: 4 additions & 4 deletions ui/desktop/src/components/recipes/RecipesView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {
} from '../../api';
import ImportRecipeForm, { ImportRecipeButton } from './ImportRecipeForm';
import CreateEditRecipeModal from './CreateEditRecipeModal';
import { generateDeepLink, Recipe } from '../../recipe';
import { generateDeepLink, Recipe, stripEmptyExtensions } from '../../recipe';
import { useNavigation } from '../../hooks/useNavigation';
import { CronPicker } from '../schedule/CronPicker';
import { Dialog, DialogContent, DialogHeader, DialogTitle } from '../ui/dialog';
Expand Down Expand Up @@ -138,12 +138,12 @@ export default function RecipesView() {
}
};

const handleStartRecipeChat = async (recipe: Recipe, _recipeId: string) => {
const handleStartRecipeChat = async (recipe: Recipe) => {
try {
const newAgent = await startAgent({
body: {
working_dir: getInitialWorkingDir(),
recipe,
recipe: stripEmptyExtensions(recipe) as Recipe,
},
throwOnError: true,
});
Expand Down Expand Up @@ -506,7 +506,7 @@ export default function RecipesView() {
<Button
onClick={(e) => {
e.stopPropagation();
handleStartRecipeChat(recipe, recipeManifestResponse.id);
handleStartRecipeChat(recipe);
}}
size="sm"
className="h-8 w-8 p-0"
Expand Down
33 changes: 18 additions & 15 deletions ui/desktop/src/components/schedule/ScheduleModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Button } from '../ui/button';
import { Input } from '../ui/input';
import { ScheduledJob } from '../../schedule';
import { CronPicker } from './CronPicker';
import { Recipe, decodeRecipe } from '../../recipe';
import { Recipe, decodeRecipe, stripEmptyExtensions } from '../../recipe';
import { getStorageDirectory } from '../../recipe/recipe_management';
import ClockIcon from '../../assets/clock-icon.svg';
import * as yaml from 'yaml';
Expand Down Expand Up @@ -80,25 +80,28 @@ async function parseDeepLink(deepLink: string): Promise<Recipe | null> {
}

function recipeToYaml(recipe: Recipe): string {
// Strip empty extensions to ensure scheduled recipes use default enabled extensions

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm, we discussed this, that's not how the scheduler works

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, scheduler always respect the extension config in recipe, not like Desktop. Maybe we should not apply this stripEmptyExtensions here.

const processedRecipe = stripEmptyExtensions(recipe) as Recipe;

const cleanRecipe: CleanRecipe = {
title: recipe.title,
description: recipe.description,
title: processedRecipe.title,
description: processedRecipe.description,
};

if (recipe.instructions) {
cleanRecipe.instructions = recipe.instructions;
if (processedRecipe.instructions) {
cleanRecipe.instructions = processedRecipe.instructions;
}

if (recipe.prompt) {
cleanRecipe.prompt = recipe.prompt;
if (processedRecipe.prompt) {
cleanRecipe.prompt = processedRecipe.prompt;
}

if (recipe.activities && recipe.activities.length > 0) {
cleanRecipe.activities = recipe.activities;
if (processedRecipe.activities && processedRecipe.activities.length > 0) {
cleanRecipe.activities = processedRecipe.activities;
}

if (recipe.extensions && recipe.extensions.length > 0) {
cleanRecipe.extensions = recipe.extensions.map((ext) => {
if (processedRecipe.extensions && processedRecipe.extensions.length > 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

and we're stripping it anyway here, so this feels like a no op

cleanRecipe.extensions = processedRecipe.extensions.map((ext) => {
const cleanExt: CleanExtension = {
name: ext.name,
type: 'builtin',
Expand Down Expand Up @@ -178,15 +181,15 @@ function recipeToYaml(recipe: Recipe): string {
});
}

if (recipe.author) {
if (processedRecipe.author) {
cleanRecipe.author = {
contact: recipe.author.contact || undefined,
metadata: recipe.author.metadata || undefined,
contact: processedRecipe.author.contact || undefined,
metadata: processedRecipe.author.metadata || undefined,
};
}

cleanRecipe.schedule = {
window_title: `${recipe.title} - Scheduled`,
window_title: `${processedRecipe.title} - Scheduled`,
};

return yaml.stringify(cleanRecipe);
Expand Down
13 changes: 13 additions & 0 deletions ui/desktop/src/recipe/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,16 @@ export async function generateDeepLink(recipe: Recipe): Promise<string> {
const encoded = await encodeRecipe(recipe);
return `goose://recipe?config=${encoded}`;
}

/**
* Strips empty extensions array from a recipe.
* This ensures recipes without explicit extensions will use the default enabled extensions
* from the user's config in the desktop rather than loading with no extensions.
*/
export function stripEmptyExtensions<T extends { extensions?: unknown[] | null }>(recipe: T): T {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why this complicated type? shouldn't this just be Recipe in, Recipe out? that would save us some casting above too

if (Array.isArray(recipe.extensions) && recipe.extensions.length === 0) {
const { extensions: _, ...rest } = recipe;
return rest as T;
}
return recipe;
}
Loading