-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Trim leading slash in path parameter #1569
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
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.
Pull request overview
This PR adds auto-correction for path parameters passed with leading slashes to the create_or_update_file tool to prevent unnecessary tool call failures.
Key Changes:
- Trim leading slash from path parameter in
CreateOrUpdateFilefunction usingstrings.TrimPrefix(path, "/")
| return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) | ||
| } | ||
|
|
||
| path = strings.TrimPrefix(path, "/") |
Copilot
AI
Dec 10, 2025
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.
The GetFileContents function also accepts a path parameter that is passed directly to client.Repositories.GetContents() and rawClient.GetRawContent() at lines 619 and 639. For consistency and to prevent similar failures, consider adding path = strings.TrimPrefix(path, "/") after the path parameter is extracted (around line 600).
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.
- [ ]
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.
- [ ]
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.
- [ ]
| return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) | ||
| } | ||
|
|
||
| path = strings.TrimPrefix(path, "/") |
Copilot
AI
Dec 10, 2025
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.
The DeleteFile function also accepts a path parameter that is used in a TreeEntry at line 950. For consistency and to prevent similar failures, consider adding path = strings.TrimPrefix(path, "/") after the path parameter is extracted (around line 920).
| return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) | ||
| } | ||
|
|
||
| path = strings.TrimPrefix(path, "/") |
Copilot
AI
Dec 10, 2025
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.
Consider adding a test case in Test_CreateOrUpdateFile that verifies a path with a leading slash is handled correctly. This would document the auto-correction behavior and prevent regressions.
Sometimes path is passed with leading slash. We can auto-correct to make sure we do not get unnecessary tool call failures.