Skip to content

Conversation

@almaleksia
Copy link
Contributor

@almaleksia almaleksia commented Dec 17, 2025

Summary

Treating path param of get_file_contents as optional + trimming leading slash.

Why

Param path is optional in tool description but in the tool handler we treat it as required parameter and get bunch of failures because LLM doesn't pass this parameter.

Trimming leading slash in path so that we do not end up calling <owner>/<repo>/contents//<path> which results in 404.

MCP impact

  • No tool or API changes
  • Tool schema or behavior changed
  • New tool added

Prompts tested (tool changes only)

Tested in MCP inspector.

Security / limits

  • No security or limits impact
  • Auth / permissions considered
  • Data exposure, filtering, or token/size limits considered

Lint & tests

  • Linted locally with ./script/lint
  • Tested locally with ./script/test

Docs

  • Not needed
  • Updated (README / docs / examples)

@almaleksia almaleksia requested a review from a team as a code owner December 17, 2025 13:23
Copilot AI review requested due to automatic review settings December 17, 2025 13:23
Copy link
Contributor

Copilot AI left a 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 fixes the get_file_contents tool to treat the path parameter as optional (matching the schema definition) and trims leading slashes from paths to prevent double-slash API calls that result in 404 errors.

Key Changes:

  • Changed from RequiredParam to OptionalParam for the path parameter to align handler implementation with schema
  • Added strings.TrimPrefix(path, "/") to prevent malformed API paths like /owner/repo/contents//file

Copy link
Collaborator

@SamMorrowDrums SamMorrowDrums left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yaass!!!

@almaleksia almaleksia merged commit b820435 into main Dec 17, 2025
24 checks passed
@almaleksia almaleksia deleted the almaleksia/get_file_contents-optional-path branch December 17, 2025 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants