-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: handle non exist subrecipe path #5287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
zanesq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks tested and its working by not letting me load the recipe with a missing sub recipe. Can you also bubble the message up to the UI in a toast? Currently it does nothing when run recipe is clicked so its a bit confusing.
|
is that related to #5288 ? that last one might warrant a quicker fix |
|
I don't think so actually this is just validation that the sub recipe is missing so not session related |
|
how did this freeze? that doesn't seem like a state we should be able to get in to at all |
|
Freezing was not the best term it wasn't allowing the param form to submit due too a backend error from the missing sub recipe so it felt like the app was not doing anything when clicked, no error just wouldn't submit. The issue is the param entry form doesn't handle the backend error and show it to the user so nothing happened. We should follow up with better error handling for param entry form but Lifei's change circumvents it for now because the recipe won't validate and run now so the user would never reach the param form. Similar issue now though currently its just a backend error that the recipe didn't load because its missing a sub recipe. We should bubble that up to the user so its clear why it won't load the recipe after clicking the run button from recipe library. Should also show this error message when loaded from a deeplink. |
zanesq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind doesn't seem to be happening now gtg!
* main: (33 commits) Add Web Accessibility Auditor recipe to cookbook (#5318) To do mcp tutorial (#5317) workflows: add a manual trigger option to pr-smoke-test (#5302) documenting `goose recipe list` command (#5278) add a system prompt snapshot test (#5305) fix: handle non exist subrecipe path (#5287) Next camp (#5237) more lowercasing of TabItem labels (#5307) modified docs/tutorials/cicd Github Action's install path to follow download_cli script (#5240) Fix artifact download to work across workflow runs (#5304) Added extension search (#5283) docs: lowercase 'goose' in TabItem labels for consistency (#5297) feat(prompts): add format to save code snippet (#5007) fix: use Windows-compatible default path for CLI installation (#5221) feat: add Test Coverage Optimizer recipe (#5118) (#5272) Upgrade node to fix canary (#5301) Remove reliance on localstorage for pendingScheduleDeepLink when scheduling a recipe (#5290) Add historical tracking with trend indicators using artifacts (#5295) roll back vite and electron package upgrades breaking canary win and linux (#5292) Revert "Revert "Rewrite extension management tools"" (#5273) ...
Signed-off-by: Blair Allan <[email protected]>
Signed-off-by: Blair Allan <[email protected]>
Summary
Issue:
In Desktop UI is frozen after user enters the parameters of the recipe.
Why:
The subrecipe path in the main recipe does not exist
Root cause:
when we load the recipe and the subrecipe path does not exist, the code skipped the error
Fix:
When the recipe is ready to load (required parameters are available), check the subrecipe path and display a toast showing the error
Type of Change
Testing
Unit test and manual testing