fix: path is duplicated on tool calls causing them to fail (#4658)#4859
Merged
michaelneale merged 2 commits intoblock:mainfrom Sep 29, 2025
Merged
fix: path is duplicated on tool calls causing them to fail (#4658)#4859michaelneale merged 2 commits intoblock:mainfrom
michaelneale merged 2 commits intoblock:mainfrom
Conversation
Signed-off-by: demetrio108 <[email protected]>
demetrio108
force-pushed
the
fix-path-overlapping-4658
branch
from
be6a395 to
ab0e57c
Compare
Collaborator
|
thanks @demetrio108 cc @tlongwell-block wonder if this was impacting well formed diff problem you mentioned? |
Collaborator
|
@demetrio108 will need to run ./scripts/clippy-lint.sh (or ask goose to sort it out!) |
DOsinga
reviewed
Sep 29, 2025
| let adjusted_components = base_components[..base_components.len() - max_k].to_vec(); | ||
| PathBuf::from_iter(adjusted_components) | ||
| } else { | ||
| base_dir.to_path_buf() |
Collaborator
There was a problem hiding this comment.
should this change the type too? if so adjust the name
Contributor
Author
There was a problem hiding this comment.
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 { |
Collaborator
There was a problem hiding this comment.
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?
Contributor
Author
There was a problem hiding this comment.
Inside the patch there is valid path relative to current goose working directory. Base directory is passed wrong by LLM
Collaborator
There was a problem hiding this comment.
ah, thanks, that's helpful
Signed-off-by: demetrio108 <[email protected]>
Contributor
Author
@michaelneale done |
DOsinga
approved these changes
Sep 29, 2025
Collaborator
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
fix the n2 and let's merge!
Contributor
Author
|
@DOsinga I don't understand, what n2? |
Collaborator
|
@demetrio108 I think @DOsinga means the O n squared thing here #4859 (comment) - which looks like you changed some code for? |
Contributor
Author
There's KMP algorithm with O(m+n) complexity, but it's an overkill in current case and it's not elegant |
zanesq
added a commit
that referenced
this pull request
Sep 30, 2025
…-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) ...
wpfleger96
added a commit
to wpfleger96/goose
that referenced
this pull request
Oct 1, 2025
* 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) ...
HikaruEgashira
pushed a commit
to HikaruEgashira/goose
that referenced
this pull request
Oct 3, 2025
… (block#4859) Signed-off-by: demetrio108 <[email protected]> Signed-off-by: HikaruEgashira <[email protected]>
This was referenced Oct 8, 2025
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.