-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: path is duplicated on tool calls causing them to fail (#4658) #4859
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
fix: path is duplicated on tool calls causing them to fail (#4658) #4859
Conversation
Signed-off-by: demetrio108 <demetrio108@protonmail.com>
be6a395 to
ab0e57c
Compare
|
thanks @demetrio108 cc @tlongwell-block wonder if this was impacting well formed diff problem you mentioned? |
|
@demetrio108 will need to run ./scripts/clippy-lint.sh (or ask goose to sort it out!) |
| let adjusted_components = base_components[..base_components.len() - max_k].to_vec(); | ||
| PathBuf::from_iter(adjusted_components) | ||
| } else { | ||
| base_dir.to_path_buf() |
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 this change the type too? if so adjust the name
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.
You mean Path to PathBuf? It's impossible to return Path. I'm not sure which name change you mean
|
|
||
| // Apply patch with fuzzy matching (70% similarity threshold) | ||
| let success = apply_patch(patch, base_dir, false, 0.7).map_err(|e| match e { | ||
| let success = apply_patch(patch, &adjusted_base_dir, false, 0.7).map_err(|e| match e { |
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 am not sure I have enough context here, but is this right? should we use the adjusted base here or should we have adjusted the patch file_path?
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.
Inside the patch there is valid path relative to current goose working directory. Base directory is passed wrong by LLM
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.
ah, thanks, that's helpful
Signed-off-by: demetrio108 <demetrio108@protonmail.com>
@michaelneale done |
DOsinga
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.
fix the n2 and let's merge!
|
@DOsinga I don't understand, what n2? |
|
@demetrio108 I think @DOsinga means the O n squared thing here #4859 (comment) - which looks like you changed some code for? |
…-unification * 'main' of github.com:block/goose: (24 commits) feat(cli): add `path` & `limit` to `session list` command (#4878) Allow better concurrent access (#4896) fix: Windows prompt cursor positioning issue with ANSI escape sequences (#4464) Fix: LiteLLM API key field not showing in UI configuration (#4105) fix: path is duplicated on tool calls causing them to fail (#4658) (#4859) add new prompt to get all available tutorials (#4802) Add filtering for agentVisible: false messages on streaming providers (#4847) alexhancock/mcp-crate-cleanup (#4885) docs: rename sub-recipe to subrecipe (#4886) docs: new multi-model section with autopilot topic (#4864) make agent manager singleton (#4880) Cli web auth token (#4456) fix(token_counter): fix panic with GitHub Copilot (#4632) Revert "Internal MCP Crate Cleanup (#4800)" (#4883) remove 2 redundant comments and one that lies (#4866) Internal MCP Crate Cleanup (#4800) Fix #4612: Return non-zero exit code when CLI session ends with error (#4621) Dead code cleanup (#4873) fix: restoring test data and correcting name (#4875) Add .goosehints file to enforce lowercase branding in documentation (#4870) ...
* remove only-pr-labels (block#4842) Signed-off-by: Angela Ning <aning@squareup.com> * Docs: Add link to Plug & Play video for Reddit MCP (block#4852) * Fix: Token count UI doesn't re-render if it's open. (block#4822) * Update databricks flash model (block#4836) * Session manager (block#4648) Co-authored-by: Douwe Osinga <douwe@squareup.com> * Add Hacktoberfest Guides (block#4830) Co-authored-by: taniashiba <126204004+taniashiba@users.noreply.github.com> * docs: goose x Hacktoberfest 2025 Blog (block#4855) Co-authored-by: Tania Chakraborty <tchakraborty@block.xyz> Co-authored-by: Angie Jones <jones.angie@gmail.com> * fix: delete some flaky and not-useful tests (block#4861) * can tell the system what shell it is using (block#4807) * new subrecipe blog post banner (block#4862) * docs: remove recipe generator link from next to extension search (block#4858) * lowercase g in goose (block#4832) * doc: file parameter recipe update (block#4594) * Fiie input recipe ref doc (block#4869) * Add .goosehints file to enforce lowercase branding in documentation (block#4870) Co-authored-by: Angie Jones <jones.angie@gmail.com> * fix: restoring test data and correcting name (block#4875) * Dead code cleanup (block#4873) Co-authored-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Michael Neale <michael.neale@gmail.com> * Fix block#4612: Return non-zero exit code when CLI session ends with error (block#4621) Signed-off-by: jalateras <jima@comware.com.au> * Internal MCP Crate Cleanup (block#4800) * remove 2 redundant comments and one that lies (block#4866) Co-authored-by: Douwe Osinga <douwe@squareup.com> * Revert "Internal MCP Crate Cleanup (block#4800)" (block#4883) * fix(token_counter): fix panic with GitHub Copilot (block#4632) Signed-off-by: sings-to-bees-on-wednesdays <222684290+sings-to-bees-on-wednesdays@users.noreply.github.com> main was broken. this seems important * Cli web auth token (block#4456) Signed-off-by: Evanfeenstra <evanfeenstra@gmail.com> * make agent manager singleton (block#4880) * docs: new multi-model section with autopilot topic (block#4864) * docs: rename sub-recipe to subrecipe (block#4886) * alexhancock/mcp-crate-cleanup (block#4885) * Temporary workaround for mcp server * Add filtering for agentVisible: false messages on streaming providers (block#4847) * add new prompt to get all available tutorials (block#4802) Signed-off-by: AdemolaAri <ademola.ari@gmail.com> * Fix for auth in extension, fix for stale keychain * fix: path is duplicated on tool calls causing them to fail (block#4658) (block#4859) Signed-off-by: demetrio108 <demetrio108@protonmail.com> * Fix: LiteLLM API key field not showing in UI configuration (block#4105) Signed-off-by: jalateras <jima@comware.com.au> Co-authored-by: Ebony Louis <55366651+EbonyLouis@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jack Amadeo <jackamadeo@squareup.com> * fix: Windows prompt cursor positioning issue with ANSI escape sequences (block#4464) Signed-off-by: Matt Donovan <mattddonovan@protonmail.com> Co-authored-by: Matt Donovan <mattddonovan@protonmail.com> * Allow better concurrent access (block#4896) Co-authored-by: Douwe Osinga <douwe@squareup.com> * feat(cli): add `path` & `limit` to `session list` command (block#4878) * Added CMD+T keyboard shortcut that takes you to the Home tab (block#4541) Signed-off-by: Guillaume Simard <2000390+GuiSim@users.noreply.github.com> --------- Signed-off-by: Angela Ning <aning@squareup.com> Signed-off-by: jalateras <jima@comware.com.au> Signed-off-by: Evanfeenstra <evanfeenstra@gmail.com> Signed-off-by: AdemolaAri <ademola.ari@gmail.com> Signed-off-by: demetrio108 <demetrio108@protonmail.com> Signed-off-by: Matt Donovan <mattddonovan@protonmail.com> Signed-off-by: Guillaume Simard <2000390+GuiSim@users.noreply.github.com> Co-authored-by: Angela Ning <32008323+angelahning@users.noreply.github.com> Co-authored-by: Emma Youndtsmith <90283317+emma-squared@users.noreply.github.com> Co-authored-by: David Katz <dkatz@squareup.com> Co-authored-by: Douwe Osinga <douwe@block.xyz> Co-authored-by: Douwe Osinga <douwe@squareup.com> Co-authored-by: Ebony Louis <55366651+EbonyLouis@users.noreply.github.com> Co-authored-by: taniashiba <126204004+taniashiba@users.noreply.github.com> Co-authored-by: taniandjerry <126204004+taniandjerry@users.noreply.github.com> Co-authored-by: Tania Chakraborty <tchakraborty@block.xyz> Co-authored-by: Angie Jones <jones.angie@gmail.com> Co-authored-by: Jack Amadeo <jackamadeo@block.xyz> Co-authored-by: Michael Neale <michael.neale@gmail.com> Co-authored-by: w. ian douglas <ian.douglas@iandouglas.com> Co-authored-by: Alex Hancock <alexhancock@block.xyz> Co-authored-by: Jarrod Sibbison <72240382+jsibbison-square@users.noreply.github.com> Co-authored-by: Rizel Scarlett <rizel@squareup.com> Co-authored-by: Jim Alateras <jima@comware.com.au> Co-authored-by: sings-to-bees-on-wednesdays <222684290+sings-to-bees-on-wednesdays@users.noreply.github.com> Co-authored-by: Evan Feenstra <evanfeenstra@gmail.com> Co-authored-by: Yingjie He <yingjiehe@squareup.com> Co-authored-by: dianed-square <73617011+dianed-square@users.noreply.github.com> Co-authored-by: Ademola Arigbabuwo <49918815+AdemolaAri@users.noreply.github.com> Co-authored-by: Demetrio ⚡️ <35406575+demetrio108@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jack Amadeo <jackamadeo@squareup.com> Co-authored-by: Matt Donovan <mattddonovan@proton.me> Co-authored-by: Matt Donovan <mattddonovan@protonmail.com> Co-authored-by: Robert Mcgregor <38837341+exitcode0@users.noreply.github.com> Co-authored-by: Guillaume Simard <2000390+GuiSim@users.noreply.github.com>
* main: (206 commits) Tiny: fix github casing (block#4903) remove anyOf from create_task tool (block#4897) chore(deps): bump tracing-subscriber from 0.3.19 to 0.3.20 (block#4442) fix optional recipe schema zod validation (block#4900) Added CMD+T keyboard shortcut that takes you to the Home tab (block#4541) feat(cli): add `path` & `limit` to `session list` command (block#4878) Allow better concurrent access (block#4896) fix: Windows prompt cursor positioning issue with ANSI escape sequences (block#4464) Fix: LiteLLM API key field not showing in UI configuration (block#4105) fix: path is duplicated on tool calls causing them to fail (block#4658) (block#4859) add new prompt to get all available tutorials (block#4802) Add filtering for agentVisible: false messages on streaming providers (block#4847) alexhancock/mcp-crate-cleanup (block#4885) docs: rename sub-recipe to subrecipe (block#4886) docs: new multi-model section with autopilot topic (block#4864) make agent manager singleton (block#4880) Cli web auth token (block#4456) fix(token_counter): fix panic with GitHub Copilot (block#4632) Revert "Internal MCP Crate Cleanup (block#4800)" (block#4883) remove 2 redundant comments and one that lies (block#4866) ...
… (block#4859) Signed-off-by: demetrio108 <demetrio108@protonmail.com> Signed-off-by: HikaruEgashira <hikaru-egashira@c-fo.com>

There is an issue opened
#4658
Path is duplicated on tool calls causing them to fail
What happens is when LLM calls text_editor in diff mode it passes
path: /a/b/c/base/src/file.go
in diff there is path: base/src/file.go
Inside mpatch they are merged and produce
path: /a/b/c/base/src/base/src/file.go
This path doesn't exist and tool fails.
Maybe it's an error from LLM side, that it calls tool incorrectly. But I tried several and it's a common issue.
So I propose to remove overlapping part from base_dir before sending to mpatch. I tested fix in Linux and it works good.