diff --git a/README.md b/README.md index 6585ab30f6..8ffe0c1d39 100644 --- a/README.md +++ b/README.md @@ -1288,7 +1288,7 @@ The following sets of tools are available: - `content`: Content of the file, exactly as it should appear once written. Do not base64-encode it; this server does that before calling the REST API. (string, required) - `message`: Commit message (string, required) - `owner`: Repository owner (username or organization) (string, required) - - `path`: Path where to create/update the file (string, required) + - `path`: Exact Git path to write. Writing to a symbolic link path rewrites the symbolic link's target path; use the linked file's path to update its contents. (string, required) - `repo`: Repository name (string, required) - `sha`: The blob SHA of the file being replaced. Required if the file already exists. (string, optional) diff --git a/pkg/github/__toolsnaps__/create_or_update_file.snap b/pkg/github/__toolsnaps__/create_or_update_file.snap index 85ad887649..3f899f5c4e 100644 --- a/pkg/github/__toolsnaps__/create_or_update_file.snap +++ b/pkg/github/__toolsnaps__/create_or_update_file.snap @@ -24,7 +24,7 @@ "type": "string" }, "path": { - "description": "Path where to create/update the file", + "description": "Exact Git path to write. Writing to a symbolic link path rewrites the symbolic link's target path; use the linked file's path to update its contents.", "type": "string" }, "repo": { diff --git a/pkg/github/__toolsnaps__/push_files.snap b/pkg/github/__toolsnaps__/push_files.snap index 798ad18451..9981878c69 100644 --- a/pkg/github/__toolsnaps__/push_files.snap +++ b/pkg/github/__toolsnaps__/push_files.snap @@ -21,7 +21,7 @@ "type": "string" }, "path": { - "description": "path to the file", + "description": "Exact Git path to write. Writing to a symbolic link path replaces the link with a regular file; use the linked file's path to update its contents.", "type": "string" } }, diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 560e8c1bac..09ce487297 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -432,7 +432,7 @@ SHA MUST be provided for existing file updates. }, "path": { Type: "string", - Description: "Path where to create/update the file", + Description: "Exact Git path to write. Writing to a symbolic link path rewrites the symbolic link's target path; use the linked file's path to update its contents.", }, "content": { Type: "string", @@ -1381,7 +1381,7 @@ func PushFiles(t translations.TranslationHelperFunc) inventory.ServerTool { Properties: map[string]*jsonschema.Schema{ "path": { Type: "string", - Description: "path to the file", + Description: "Exact Git path to write. Writing to a symbolic link path replaces the link with a regular file; use the linked file's path to update its contents.", }, "content": { Type: "string", diff --git a/pkg/github/tools.go b/pkg/github/tools.go index af571f8426..6ecd5c3d85 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -38,10 +38,11 @@ var ( InstructionsFunc: generateContextToolsetInstructions, } ToolsetMetadataRepos = inventory.ToolsetMetadata{ - ID: "repos", - Description: "GitHub Repository related tools", - Default: true, - Icon: "repo", + ID: "repos", + Description: "GitHub Repository related tools", + Default: true, + Icon: "repo", + InstructionsFunc: generateReposToolsetInstructions, } ToolsetMetadataGit = inventory.ToolsetMetadata{ ID: "git", diff --git a/pkg/github/toolset_instructions.go b/pkg/github/toolset_instructions.go index 3b3a54eadd..8cd908f31d 100644 --- a/pkg/github/toolset_instructions.go +++ b/pkg/github/toolset_instructions.go @@ -9,6 +9,12 @@ func generateContextToolsetInstructions(_ *inventory.Inventory) string { return "Always call 'get_me' first to understand current user permissions and context." } +func generateReposToolsetInstructions(_ *inventory.Inventory) string { + return `## Repository file writes + +'get_file_contents' may return the target contents when a path is a symbolic link, but repository file writes use exact Git paths and do not follow symbolic links. To edit content that a symlink points to, write to the target path.` +} + func generateIssuesToolsetInstructions(_ *inventory.Inventory) string { return `## Issues diff --git a/pkg/github/toolset_instructions_test.go b/pkg/github/toolset_instructions_test.go new file mode 100644 index 0000000000..5e7dbb4cdc --- /dev/null +++ b/pkg/github/toolset_instructions_test.go @@ -0,0 +1,86 @@ +package github + +import ( + "strings" + "testing" + + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/google/jsonschema-go/jsonschema" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRepositoryInstructionsExplainSymlinkWriteSemantics(t *testing.T) { + t.Setenv("DISABLE_INSTRUCTIONS", "false") + + reposInventory, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{{Toolset: ToolsetMetadataRepos}}). + WithToolsets([]string{"repos"}). + WithServerInstructions(). + Build() + require.NoError(t, err) + + instructions := strings.ToLower(reposInventory.Instructions()) + assert.Contains(t, instructions, "## repository file writes") + assert.Contains(t, instructions, "may return the target contents") + assert.Contains(t, instructions, "do not follow symbolic links") + + defaultInventory, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{ + {Toolset: ToolsetMetadataContext}, + {Toolset: ToolsetMetadataRepos}, + }). + WithToolsets([]string{"default"}). + WithServerInstructions(). + Build() + require.NoError(t, err) + assert.Contains(t, strings.ToLower(defaultInventory.Instructions()), "## repository file writes") + + contextInventory, err := inventory.NewBuilder(). + SetTools([]inventory.ServerTool{{Toolset: ToolsetMetadataContext}}). + WithToolsets([]string{"context"}). + WithServerInstructions(). + Build() + require.NoError(t, err) + assert.NotContains(t, strings.ToLower(contextInventory.Instructions()), "## repository file writes") +} + +func TestFileWritePathsExplainSymlinkWriteSemantics(t *testing.T) { + createSchema, ok := CreateOrUpdateFile(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok) + pushSchema, ok := PushFiles(translations.NullTranslationHelper).Tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok) + createPath := createSchema.Properties["path"] + require.NotNil(t, createPath) + pushFiles := pushSchema.Properties["files"] + require.NotNil(t, pushFiles) + require.NotNil(t, pushFiles.Items) + pushPath := pushFiles.Items.Properties["path"] + require.NotNil(t, pushPath) + + tools := []struct { + name string + description string + expectedBehavior string + }{ + { + name: "create_or_update_file", + description: createPath.Description, + expectedBehavior: "rewrites the symbolic link's target path", + }, + { + name: "push_files", + description: pushPath.Description, + expectedBehavior: "replaces the link with a regular file", + }, + } + + for _, tool := range tools { + t.Run(tool.name, func(t *testing.T) { + description := strings.ToLower(tool.description) + assert.Contains(t, description, "exact git path") + assert.Contains(t, description, tool.expectedBehavior) + }) + } +}