Clarify symlink behavior for repository file writes
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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"
|
||||
}
|
||||
},
|
||||
|
||||
@@ -433,7 +433,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",
|
||||
@@ -1573,7 +1573,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",
|
||||
|
||||
+5
-4
@@ -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",
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user