-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Speed up recipe loading from deeplinks and various fixes #3662
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
…n can be pasted in easier
…ndering * 'main' of github.com:block/goose: Remove confusing status output when testing sharing url connection and it shows 404 (#3659)
| try { | ||
| // Get all registered Goose apps | ||
| console.log('Finding all registered Goose applications...'); | ||
| const lsregisterOutput = execSync(`/System/Library/Frameworks/CoreServices.framework/Versions/A/Frameworks/LaunchServices.framework/Versions/A/Support/lsregister -dump | grep -B 10 -A 10 "claimed schemes:.*${PROTOCOL}:"`, { encoding: 'utf8' }); |
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.
this looks like it will only work on mac...
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.
yup comment at top says only mac for now
| // Filter out null results and apply archived filter | ||
| const recipes: SavedRecipe[] = []; | ||
|
|
||
| for (const recipe of globalRecipes) { |
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.
should we take this as a reason to remove the local recipes? it doesn't make sense to me
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.
good question I'm on the fence about that I think @AaronGoldsmith had a good reason for adding local and I think the idea is you could have recipes that are specific to a project folder/workspace which might come in handy when we have a concept of "Projects"
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.
My thinking, for local recipes was that it follows how memory and .goosehints work. Both have global/local versions which can be used by Goose. Devs can decide whether they want to add a .goosehints and memory files at the repository level, which creates a more consistent experience/development environment for Goose users.
I envision being able to clone a repository, open it in Goose, and find a list established recipes that demonstrate how to interact or get the most use out of a project.
I'm fine with removing it if it's not getting used. It does look like there was a recent change in the cli to support "local recipes"
https://github.com/block/goose/blob/main/crates/goose-cli/src/recipes/search_recipe.rs#L127
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.
I think that uses GOOSE_RECIPE_PATH_ENV_VAR maybe?
I think we talked about this in a different context about how this should be done through the server and not have the client poke around here; I wrote up this doc:
on how I would like to see things go. local recipes are not a bad idea per se, but I think we should unify the CLI and the desktop first even if that means dropping the local recipes for now
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.
Makes sense to prioritize unifying CLI and GUI. I'm fine with local recipes getting removed for now.
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.
SGTM I'll follow up with removing local recipes in another PR 👍
* main: blog: streamlining detection development w/ recipes (#3689) fix: have option for cli providers to use their configured or default model (#3683) docs: new blog post and corrections to an old one on goosehints (#3657) Resolve sub recipe path relative to the parent recipe path (#3642) Speed up recipe loading from deeplinks and various fixes (#3662) fix cmd + , not opening settings (#3694) Add warning when JSON env parsing fails. (#3696) chore: refactor session naming into provider (#3678) feat (ui): File picker for scheduling recipes default to recipe dir (#3611) fix: address issue with streamable http interactions via mcp (#3693) Provider scenario tests (#3688) Fix conversations before they hit the LLM (#3660) cli: add detailed instruction for WSL users (#3496) feat: recipe runs will now prompt for missing extension secrets (#3668) fix: pricing integration tests -> trying more runs for cache and retries (#3546)
* main: fix: Ensures final output tool is available when using vector tool search (#3701) chore: adding in some new models token limits (#3685) blog: streamlining detection development w/ recipes (#3689) fix: have option for cli providers to use their configured or default model (#3683) docs: new blog post and corrections to an old one on goosehints (#3657) Resolve sub recipe path relative to the parent recipe path (#3642) Speed up recipe loading from deeplinks and various fixes (#3662) fix cmd + , not opening settings (#3694) Add warning when JSON env parsing fails. (#3696) chore: refactor session naming into provider (#3678) feat (ui): File picker for scheduling recipes default to recipe dir (#3611) fix: address issue with streamable http interactions via mcp (#3693) Provider scenario tests (#3688)
Signed-off-by: Adam Tarantino <[email protected]>
Users reported some recipe loading from deeplinks was freezing the ui up to 30 seconds for the recipes to show. Moved the recipe decoding to a background process to let the app continue loading faster.
Fixed issue where we weren't rendering the
:messagepills with markdown and also created a dedicated area when editing recipes for adding multiline message markdown content.Fixed issue where the splash screen was sometimes showing the animated greeting message from home/hub if there were no activities.
Fixed issue where the recipe install warning was not allowing scrolling for long recipe content. Also added markdown rendering in the install warning.
Improved loading of recipe page recipes using parallelizing rather than waiting on them to finish one by one.
Bonus: added a handy script for resetting the osx deeplink associations when testing locally.