-
Notifications
You must be signed in to change notification settings - Fork 2.3k
migrating back with new chatrecall non underscore name #5223
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
|
FYI this is meant to be off by default, but I think the default setting for all buildins is ignored currently (ie this and todo will always be on until you turn it off) is that ok? |
|
huh, we should fix both those things in the frontend. it has no business trying to change the name of the extensions. I think a similar thing might have happened with @angelahning 's change. |
|
I looked into this briefly and this is such a mess. both the client and the server have a normalize name function. but they are not the same. one tolerates underscores, the other does not. and obviously the CLI takes a different path. @alexhancock @zanesq your handles came up in the code about this, do you know why we do this at all? we also seem to expand the path to the exectuable in the client. this all seems a generous invitation to bugs |
|
I wonder if expanding to executable was for security (ie so it can only run the binaries that came with it?) - struggling to think why that would be (it may not have it on PATH otherwise). @DOsinga should I hold off merging this for stabilisation? |
|
@DOsinga @michaelneale I also found many things in this area that were confusing and messy last week. Multiple attempts at different places to replace paths to executable versions with "shims" etc. It seemed to me perhaps the best thing to do is simply not to touch any commands, but make sure the dir with the hermit executables ( I can look at this in a separate change |
It doesn't make any sense what we are doing right now, we mess around with names and executable etc but only in some paths. then in other paths we either do some of it on the server or not at all. we should treat the extension definitions as readonly. rip it all out! and then when it comes to actually executing the thing, maybe at that point we need to resolve the path to the executable, but that should be in one place in the extension manager I would thinkg |
|
@DOsinga makes sense - I can give it a go |
|
yeah - I think setting $PATH correctly is the go. Previously we would inherit the PATH from users (interactive) so didn't want to touch that, but now we dont' so can take more control I think. Ideally we just want hermit for pre-installing the things we know are problematic (python and node tool chains mainly) |
|
@alexhancock want me to hold on this until you are happy with things, tag me when ready and I can pick up this PR again? and check it (could even try slapping in an underscore again!) - or this is orthogonal? |
|
@DOsinga and I had a change in progress today, but if you've tested this and it doesn't have the double add bug, I say go ahead and then it'll just be another improvement when we merge the thing that improves extension management. |
|
do we know where the double add comes from? I would like to overhaul how we set the default list of extensions in a sensible way, but I don't quite understand how it is supposed to work also with the block version of this |
|
@DOsinga if by double add you mean chatrecall and chat_recall - in that case it was as cli allowed it, and then desktop (incorrectly) renamed it and added it with new name (won't do that in this case anymore). |
* main: fixing typo in blog metadata (#5382) feat: add new blog entry on adopting Goose in the enterprise (#5381) Blog/acp intro oct 2024 (#5379) feat: add A/B test framework generator recipe (#5378) Doc: (blog) - Deep Dive into goose's Extension System and Model Context Protocol (MCP) (#5291) Some system prompt tidying (#5313) Fix scheduler jobs dates formatting (#5368) Use Instructions as Prompt in Scheduler (#5359) feat(snowflake): add support for newer Claude 4.5 and 4 models (#5350) Add bottom menu extension selection (#5352) (re)Standardize Session Name Attribute (#5279) chore: improve timeout for entering password when running goose ui from source (#5349)
* main: migrating back with new chatrecall non underscore name (#5223)
* main: Auto-compact Threshold UI improvements (#5354) Filter preserved user messages to be text only. (#5391) include sessionId in tool request (#5394) feat: add PR Impact Analyzer prompt (#5375) docs: add blog post on configuring goose for team environments (#5380) migrating back with new chatrecall non underscore name (#5223) fixing typo in blog metadata (#5382) feat: add new blog entry on adopting Goose in the enterprise (#5381) Blog/acp intro oct 2024 (#5379) feat: add A/B test framework generator recipe (#5378) Doc: (blog) - Deep Dive into goose's Extension System and Model Context Protocol (MCP) (#5291) Some system prompt tidying (#5313) Fix scheduler jobs dates formatting (#5368) Use Instructions as Prompt in Scheduler (#5359) feat(snowflake): add support for newer Claude 4.5 and 4 models (#5350)
* main: Feat/add mermaid chart rendering (#5377) Set up Datadog metrics for prompt injection detection (#5385) fix: restore --resume functionality for most recent session (#5401) Gemini again (#5390) docs(prompt-library): add github-issue-labeler intermediate prompt (#5374) docs: add Linux and Windows paths to uninstall section (#5371) fix: --session-id shouldn't work without --resume, but --name should (#5360) Auto-compact Threshold UI improvements (#5354) Filter preserved user messages to be text only. (#5391) include sessionId in tool request (#5394) feat: add PR Impact Analyzer prompt (#5375) docs: add blog post on configuring goose for team environments (#5380) migrating back with new chatrecall non underscore name (#5223)
* 'main' of github.com:block/goose: (132 commits) Fix/icon ii (#5413) Enable runtime access to provider name (#5399) fix: ensure trailing newline in files created by `text_editor` tool (#5336) docs: September 2025 Community All-Stars (#5411) make supports_cache_control async to avoid block in place (#5362) Send all the logs we output (#5363) Recipe variables (#5365) Feat/add mermaid chart rendering (#5377) Set up Datadog metrics for prompt injection detection (#5385) fix: restore --resume functionality for most recent session (#5401) Gemini again (#5390) docs(prompt-library): add github-issue-labeler intermediate prompt (#5374) docs: add Linux and Windows paths to uninstall section (#5371) fix: --session-id shouldn't work without --resume, but --name should (#5360) Auto-compact Threshold UI improvements (#5354) Filter preserved user messages to be text only. (#5391) include sessionId in tool request (#5394) feat: add PR Impact Analyzer prompt (#5375) docs: add blog post on configuring goose for team environments (#5380) migrating back with new chatrecall non underscore name (#5223) ...
Signed-off-by: Blair Allan <[email protected]>
fixes the reverted recall: #5177 tool
It was broken as it was called "chat_recall" but the GUI code strips (for some reason) underscores when it saves config.yaml (cli doesn't) which will break any extension with _ in its name (this just removes it from the name) - same otherwise.