feat: edit model and extensions of a recipe from GUI#6804
feat: edit model and extensions of a recipe from GUI#6804zanesq merged 10 commits intoblock:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds GUI support for editing a recipe’s model/provider settings and enabled extensions, so users don’t need to manually edit recipe YAML.
Changes:
- Extend the recipe form schema to include
model,provider, andextensions. - Add new UI components for selecting provider/model and toggling recipe extensions.
- Wire new fields into the recipe form and ensure they’re included in the recipe payload on save.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/desktop/src/components/recipes/shared/recipeFormSchema.ts | Adds optional model, provider, and extensions to the form schema. |
| ui/desktop/src/components/recipes/shared/RecipeModelSelector.tsx | New selector UI for choosing provider + model from configured providers. |
| ui/desktop/src/components/recipes/shared/RecipeExtensionSelector.tsx | New selector UI for choosing extensions from the configured extension list. |
| ui/desktop/src/components/recipes/shared/RecipeFormFields.tsx | Adds the new selectors to the Advanced Options section and updates “has advanced data” detection. |
| ui/desktop/src/components/recipes/CreateEditRecipeModal.tsx | Persists selected model/provider/extensions into the saved recipe payload. |
ui/desktop/src/components/recipes/shared/__tests__/RecipeFormFields.test.tsx
Show resolved
Hide resolved
ui/desktop/src/components/recipes/shared/RecipeModelSelector.tsx
Outdated
Show resolved
Hide resolved
ui/desktop/src/components/recipes/shared/__tests__/RecipeFormFields.test.tsx
Show resolved
Hide resolved
|
UI looks good! From initial testing I think I found a bug where editing a recipe that already has extensions defined isn't showing the extensions as enabled. Can you take a look? Goose also confirmed it but I haven't checked its work 🐛 Bug Found: Extensions Not Loading from Existing RecipeThe ProblemIn const { extensionsList: allExtensions } = useConfig();This The component then checks if a recipe's extension is selected by matching names: const selectedExtensionNames = useMemo(
() => new Set(selectedExtensions.map((ext) => ext.name)),
[selectedExtensions]
);And displays only extensions from const sortedExtensions = useMemo(() => {
return [...filteredExtensions].sort((a, b) => {
const aSelected = selectedExtensionNames.has(a.name);
// ...
});
}, [filteredExtensions, selectedExtensionNames]);The BugIf a recipe has extensions that are NOT in the user's current
This happens because:
How to Reproduce
Suggested FixThe component should merge the recipe's const displayExtensions = useMemo(() => {
// Start with all available extensions
const extensionMap = new Map(allExtensions.map(ext => [ext.name, ext]));
// Add any selected extensions that aren't in the current config
selectedExtensions.forEach(ext => {
if (!extensionMap.has(ext.name)) {
extensionMap.set(ext.name, ext);
}
});
return Array.from(extensionMap.values());
}, [allExtensions, selectedExtensions]);
Then use `displayExtensions` instead of `filteredExtensions` for filtering/sorting. |
80e81fd to
40be18b
Compare
ui/desktop/src/components/recipes/shared/RecipeExtensionSelector.tsx
Outdated
Show resolved
Hide resolved
|
@Abhijay007 another thought I had is goose desktop actually respecting the model choice when the recipe is loaded? I thought models was a global thing now in the desktop. If its not working maybe we should remove model selection until it is supported? |
I added a PR for this yesterday : #6884 |
|
Nice will take a look! |
zanesq
left a comment
There was a problem hiding this comment.
Looking good, only 2 more pieces of feedback to wrap this up:
- Can you add error handling in RecipeModelSelector so it shows a message that fetching models failed rather than swallowing the error?
- Can you align the extensions selected count message to the right side? Visually it gets in the way when its on the left.
cb0bc44 to
b446857
Compare
| const filteredModelOptions = selectedProvider | ||
| ? modelOptions.filter((group) => group.options[0]?.provider === selectedProvider) | ||
| : []; | ||
|
|
There was a problem hiding this comment.
filteredModelOptions becomes [] when selectedProvider is unset, which makes it impossible to view/select a model for recipes that only set settings.goose_model while relying on the default provider (this is supported by the CLI/session builder logic). Consider allowing model entry/selection when selectedProvider is undefined (e.g., fall back to a free-text model input, and treat a pre-filled selectedModel with no provider as custom).
| it('allows selecting extensions', async () => { | ||
| const user = userEvent.setup(); | ||
| const TestComponent = () => { | ||
| const form = useForm({ | ||
| defaultValues: { | ||
| title: 'Test Recipe', | ||
| description: 'Test', | ||
| instructions: 'Test', | ||
| prompt: 'Test', | ||
| activities: [], | ||
| parameters: [], | ||
| jsonSchema: '', | ||
| model: undefined, | ||
| provider: undefined, | ||
| extensions: undefined, | ||
| } as RecipeFormData, | ||
| onSubmit: async ({ value }) => { | ||
| console.log('Form submitted:', value); | ||
| }, | ||
| }); | ||
|
|
||
| return <RecipeFormFields form={form} />; | ||
| }; | ||
|
|
||
| render(<TestComponent />); | ||
|
|
||
| await expandAdvancedSection(user); | ||
|
|
||
| expect(screen.getByText('Extensions (Optional)')).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
This test name says it "allows selecting extensions", but it only asserts that the Extensions label renders, so it doesn’t validate the toggle/selection behavior; either drive the UI to toggle an extension and assert the selected count/value, or rename the test to reflect a render-only check.
| <form.Field name="provider"> | ||
| {(providerField: FormFieldApi<string | undefined>) => ( | ||
| <form.Field name="model"> | ||
| {(modelField: FormFieldApi<string | undefined>) => ( | ||
| <RecipeModelSelector | ||
| selectedProvider={providerField.state.value} | ||
| selectedModel={modelField.state.value} | ||
| onProviderChange={(provider) => providerField.handleChange(provider)} | ||
| onModelChange={(model) => modelField.handleChange(model)} | ||
| /> | ||
| )} | ||
| </form.Field> | ||
| )} | ||
| </form.Field> | ||
|
|
||
| {/* Extensions Field */} | ||
| <form.Field name="extensions"> | ||
| {(field: FormFieldApi<ExtensionConfig[] | undefined>) => ( | ||
| <RecipeExtensionSelector | ||
| selectedExtensions={field.state.value || []} | ||
| onExtensionsChange={(extensions) => | ||
| field.handleChange(extensions.length > 0 ? extensions : undefined) | ||
| } | ||
| /> | ||
| )} | ||
| </form.Field> |
There was a problem hiding this comment.
RecipeFormFields now always renders provider/model/extensions fields, but some submit flows (e.g., CreateRecipeFromSessionModal builds the Recipe payload without mapping these fields) will silently drop user selections; consider either plumbing these values through all consumers or making these selectors opt-in via props for contexts that don't persist them.
| <form.Field name="provider"> | |
| {(providerField: FormFieldApi<string | undefined>) => ( | |
| <form.Field name="model"> | |
| {(modelField: FormFieldApi<string | undefined>) => ( | |
| <RecipeModelSelector | |
| selectedProvider={providerField.state.value} | |
| selectedModel={modelField.state.value} | |
| onProviderChange={(provider) => providerField.handleChange(provider)} | |
| onModelChange={(model) => modelField.handleChange(model)} | |
| /> | |
| )} | |
| </form.Field> | |
| )} | |
| </form.Field> | |
| {/* Extensions Field */} | |
| <form.Field name="extensions"> | |
| {(field: FormFieldApi<ExtensionConfig[] | undefined>) => ( | |
| <RecipeExtensionSelector | |
| selectedExtensions={field.state.value || []} | |
| onExtensionsChange={(extensions) => | |
| field.handleChange(extensions.length > 0 ? extensions : undefined) | |
| } | |
| /> | |
| )} | |
| </form.Field> | |
| {(() => { | |
| const values = (form as any)?.state?.values ?? {}; | |
| const showProviderModelFields = 'provider' in values || 'model' in values; | |
| if (!showProviderModelFields) { | |
| return null; | |
| } | |
| return ( | |
| <form.Field name="provider"> | |
| {(providerField: FormFieldApi<string | undefined>) => ( | |
| <form.Field name="model"> | |
| {(modelField: FormFieldApi<string | undefined>) => ( | |
| <RecipeModelSelector | |
| selectedProvider={providerField.state.value} | |
| selectedModel={modelField.state.value} | |
| onProviderChange={(provider) => providerField.handleChange(provider)} | |
| onModelChange={(model) => modelField.handleChange(model)} | |
| /> | |
| )} | |
| </form.Field> | |
| )} | |
| </form.Field> | |
| ); | |
| })()} | |
| {/* Extensions Field */} | |
| {(() => { | |
| const values = (form as any)?.state?.values ?? {}; | |
| const showExtensionsField = 'extensions' in values; | |
| if (!showExtensionsField) { | |
| return null; | |
| } | |
| return ( | |
| <form.Field name="extensions"> | |
| {(field: FormFieldApi<ExtensionConfig[] | undefined>) => ( | |
| <RecipeExtensionSelector | |
| selectedExtensions={field.state.value || []} | |
| onExtensionsChange={(extensions) => | |
| field.handleChange(extensions.length > 0 ? extensions : undefined) | |
| } | |
| /> | |
| )} | |
| </form.Field> | |
| ); | |
| })()} |
|
@Abhijay007 why do we need the |
I will add another PR handling that refactor I think that would be better |
|
What about the useMemos? Do we need them? Don't want to add complexity if not needed. |
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
… nits Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
Signed-off-by: Abhijay007 <Abhijay007j@gmail.com>
b446857 to
cc5888e
Compare
removed it 👍 |
* main: fix text editor view broken (#7167) docs: White label guide (#6857) Add PATH detection back to developer extension (#7161) docs: pin version in ci/cd (#7168) Desktop: - No Custom Headers field for custom OpenAI-compatible providers (#6681) feat: edit model and extensions of a recipe from GUI (#6804) feat: MCP support for agentic CLI providers (#6972)
* origin/main: (33 commits) fix: replace panic with proper error handling in get_tokenizer (#7175) Lifei/smoke test for developer (#7174) fix text editor view broken (#7167) docs: White label guide (#6857) Add PATH detection back to developer extension (#7161) docs: pin version in ci/cd (#7168) Desktop: - No Custom Headers field for custom OpenAI-compatible providers (#6681) feat: edit model and extensions of a recipe from GUI (#6804) feat: MCP support for agentic CLI providers (#6972) docs: keyring fallback to secrets.yaml (#7165) feat: load provider/model specified inside the recipe config (#6884) fix ask-ai bot hitting tool call limits (#7162) fix flatpak icon (#7154) [docs] Skills Marketplace UI Improvements (#7158) More no-window flags (#7122) feat: Allow overriding default bat themes using environment variables (#7140) Make the system prompt smaller (#6991) Pre release script (#7145) Spelling (#7137) feat(mcp): upgrade rmcp to 0.15.0 and advertise MCP Apps UI extension capability (#6927) ...
Closes #6652
Summary
Type of Change
AI Assistance
Testing
Screenshots/Demos (for UX changes)
Before:
After: