Repository navigation
Add two experimental MCP tools to create, read, update, and delete artifacts attached #3554
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
Merged
+1,151
−0
Merged
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
fa4e8b1
adding an initial draft of the MCP tooling
veralizeth 4b58040
Fixing flacky test
veralizeth 04afa0b
Fixing flaky testss
veralizeth 506fcad
Added all five required snapshots in toolsnaps.
veralizeth 944ad89
Merge branch 'main' into veralizeth/artifacts-mcp-tooling
veralizeth b632f19
Fixing wording
veralizeth f256843
Merge remote updates into issue artifact tooling
veralizeth 85a7b08
chaning to add 2 actions and methods
veralizeth be2e975
fix: align issue artifact tools with REST API types
veralizeth 4d76875
Merge branch 'main' into veralizeth/artifacts-mcp-tooling
veralizeth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,28 @@ end users. Insiders-only flags are not user-toggleable. | |||||||
|
|
||||||||
| ## Declaring tool availability | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
|
||||||||
| ### Experimental issue artifacts | ||||||||
|
|
||||||||
| Enable `--features=issue_artifacts` (or `GITHUB_FEATURES=issue_artifacts`) to | ||||||||
| expose `create_issue_artifact`, `update_issue_artifact`, `delete_issue_artifact`, | ||||||||
| `list_issue_artifacts`, and `get_issue_artifact` in the `issues` toolset. | ||||||||
| This flag is **off by default**, including in Insiders mode. Read-only mode | ||||||||
| exposes only list and get. | ||||||||
|
|
||||||||
| The MCP flag only exposes tools; it does not grant API access. The tools | ||||||||
| use the configured authenticated GitHub client and base URL without an | ||||||||
| endpoint-specific preview header. They declare `repo` in their OAuth challenge | ||||||||
| metadata; API access also depends on the token's permissions and repository | ||||||||
| access. API failures are returned explicitly; writes are not retried. | ||||||||
|
|
||||||||
| Artifact numbers are scoped to their owning issue, not global IDs. Updates | ||||||||
| preserve omitted fields, allow `body: ""` to clear the body, and cannot change | ||||||||
| type. Lists return one explicit `page`/`perPage` with bodies; request successive | ||||||||
| pages until fewer than `perPage` artifacts are returned (there are no `Link` | ||||||||
| headers). Artifact names and bodies are sanitized as user-authored content. | ||||||||
| All five tools fail closed in lockdown mode because the reference artifact | ||||||||
| response does not identify authors for content-trust checks. | ||||||||
|
|
||||||||
| Tools, resources, and prompts use `inventory.NewFeatureRule` when feature flags | ||||||||
| change whether they are available. Each rule declares the flags it references | ||||||||
| and evaluates them with a fail-closed `FeatureResolver`, so normal Go boolean | ||||||||
|
|
||||||||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package github | ||
|
|
||
| // IssueArtifact identifies a Markdown artifact by both its global ID and its | ||
| // issue-scoped number. Requests use Number, never ID. | ||
| type IssueArtifact struct { | ||
| ID int64 `json:"id"` | ||
| Number int `json:"number"` | ||
| Type string `json:"type"` | ||
| Name string `json:"name"` | ||
| Body string `json:"body"` | ||
|
veralizeth marked this conversation as resolved.
Dismissed
|
||
| } | ||
|
|
||
| // IssueArtifactCoordinates identifies the issue that owns an artifact. | ||
| type IssueArtifactCoordinates struct { | ||
| Owner string `json:"owner" jsonschema:"Repository owner"` | ||
| Repo string `json:"repo" jsonschema:"Repository name"` | ||
| IssueNumber int `json:"issue_number" jsonschema:"Positive issue number"` | ||
| } | ||
|
|
||
| // CreateIssueArtifactInput creates an artifact, including an optionally empty body. | ||
| type CreateIssueArtifactInput struct { | ||
| IssueArtifactCoordinates | ||
| Type string `json:"type" jsonschema:"Artifact type"` | ||
| Name string `json:"name" jsonschema:"Nonempty artifact name"` | ||
| Body string `json:"body" jsonschema:"Markdown body; may be empty"` | ||
| } | ||
|
|
||
| // UpdateIssueArtifactInput preserves omitted fields; an empty body clears it. | ||
| // The artifact's type cannot be changed. | ||
| type UpdateIssueArtifactInput struct { | ||
| IssueArtifactCoordinates | ||
| ArtifactNumber int `json:"artifact_number" jsonschema:"Positive issue-scoped artifact number, not its global ID"` | ||
| Name *string `json:"name,omitempty" jsonschema:"Replacement nonempty name"` | ||
| Body *string `json:"body,omitempty" jsonschema:"Replacement Markdown body; an empty string clears it"` | ||
| } | ||
|
|
||
| // GetIssueArtifactInput addresses one artifact for reading or deletion. | ||
| type GetIssueArtifactInput struct { | ||
| IssueArtifactCoordinates | ||
| ArtifactNumber int `json:"artifact_number" jsonschema:"Positive issue-scoped artifact number, not its global ID"` | ||
| } | ||
|
|
||
| // ListIssueArtifactsInput selects an explicit page, without relying on Link headers. | ||
| type ListIssueArtifactsInput struct { | ||
| IssueArtifactCoordinates | ||
| Type string `json:"type,omitempty" jsonschema:"Optional artifact type filter"` | ||
| Page int `json:"page,omitempty" jsonschema:"Page number, starting at 1 (default 1)"` | ||
| PerPage int `json:"perPage,omitempty" jsonschema:"Results per page, from 1 to 100 (default 30)"` | ||
| } | ||
|
|
||
| // ListIssueArtifactsOutput includes bodies and the requested page coordinates. | ||
| // A full page may have a successor; callers can request the next explicit page. | ||
| type ListIssueArtifactsOutput struct { | ||
| Artifacts []IssueArtifact `json:"artifacts"` | ||
| Page int `json:"page"` | ||
| PerPage int `json:"perPage"` | ||
| } | ||
|
|
||
| // DeleteIssueArtifactOutput confirms the issue-scoped artifact that was deleted. | ||
| type DeleteIssueArtifactOutput struct { | ||
| IssueNumber int `json:"issue_number"` | ||
| ArtifactNumber int `json:"artifact_number"` | ||
| Message string `json:"message"` | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,255 @@ | ||
| package github | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "net/http" | ||
| "net/url" | ||
| "slices" | ||
| "strconv" | ||
| "strings" | ||
|
|
||
| ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" | ||
| "github.com/github/github-mcp-server/v2/pkg/ifc" | ||
| "github.com/github/github-mcp-server/v2/pkg/inventory" | ||
| "github.com/github/github-mcp-server/v2/pkg/sanitize" | ||
| "github.com/github/github-mcp-server/v2/pkg/scopes" | ||
| "github.com/github/github-mcp-server/v2/pkg/translations" | ||
| "github.com/github/github-mcp-server/v2/pkg/utils" | ||
| "github.com/google/jsonschema-go/jsonschema" | ||
| "github.com/modelcontextprotocol/go-sdk/mcp" | ||
| ) | ||
|
|
||
| var issueArtifactTypes = []string{"plan", "generic", "link", "instruction", "skill", "mcp_config", "reference"} | ||
|
|
||
| func newIssueArtifactTool[In, Out any](t translations.TranslationHelperFunc, name, title, description string, readOnly, destructive bool, enums []inventory.SchemaEnum, | ||
| handler func(context.Context, ToolDependencies, *mcp.CallToolRequest, In) (*mcp.CallToolResult, Out, error), | ||
| ) inventory.ServerTool { | ||
| key := "TOOL_" + strings.ToUpper(name) | ||
| schema := issueDTOschema[In]() | ||
| for _, field := range []string{"issue_number", "artifact_number", "page", "perPage"} { | ||
| if property := schema.Properties[field]; property != nil { | ||
| property.Minimum = new(1.0) | ||
| } | ||
| } | ||
| for _, field := range []string{"owner", "repo", "name"} { | ||
| if property := schema.Properties[field]; property != nil { | ||
| property.MinLength = new(1) | ||
| } | ||
| } | ||
| if property := schema.Properties["perPage"]; property != nil { | ||
| property.Maximum = new(100.0) | ||
| } | ||
| for _, enum := range enums { | ||
| schema.Properties[enum.Path] = inventory.EnumSchema(enum.Values...) | ||
| } | ||
| if schema.Properties["name"] != nil && !slices.Contains(schema.Required, "name") { | ||
| schema.Properties["name"].Type, schema.Properties["name"].Types = "string", nil | ||
| schema.Properties["body"].Type, schema.Properties["body"].Types = "string", nil | ||
| schema.AnyOf = []*jsonschema.Schema{{Required: []string{"name"}}, {Required: []string{"body"}}} | ||
|
veralizeth marked this conversation as resolved.
Resolved
|
||
| } | ||
| st := NewToolWithSchemaOptions(ToolsetMetadataIssues, mcp.Tool{ | ||
| Name: name, Description: t(key+"_DESCRIPTION", description), | ||
| InputSchema: schema, | ||
|
veralizeth marked this conversation as resolved.
Resolved
|
||
| Annotations: &mcp.ToolAnnotations{ | ||
| Title: t(key+"_USER_TITLE", title), ReadOnlyHint: readOnly, | ||
| DestructiveHint: new(destructive), | ||
| }, | ||
| }, scopes.RequireAll(scopes.Repo), inventory.TypedSchemaOptions{ | ||
| PreserveHandlerContent: true, | ||
| }, handler) | ||
| st.FeatureRule = featureEnabledRule(FeatureFlagIssueArtifacts) | ||
| return st | ||
| } | ||
|
|
||
| // CreateIssueArtifact creates a Markdown artifact attached to an issue. | ||
| func CreateIssueArtifact(t translations.TranslationHelperFunc) inventory.ServerTool { | ||
| return newIssueArtifactTool(t, "create_issue_artifact", "Create issue artifact", | ||
| "Create a Markdown artifact attached to a GitHub issue. Experimental: endpoint availability and access requirements are unverified.", | ||
| false, false, []inventory.SchemaEnum{{Path: "type", Values: issueArtifactTypes}}, | ||
| func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input CreateIssueArtifactInput) (*mcp.CallToolResult, *IssueArtifact, error) { | ||
| if !slices.Contains(issueArtifactTypes, input.Type) { | ||
| return utils.NewToolResultError("unsupported artifact type"), nil, nil | ||
| } | ||
| if strings.TrimSpace(input.Name) == "" { | ||
| return utils.NewToolResultError("name must not be empty"), nil, nil | ||
| } | ||
| body := struct { | ||
| Type string `json:"type"` | ||
| Name string `json:"name"` | ||
| Body string `json:"body"` | ||
| }{input.Type, input.Name, input.Body} | ||
| var output IssueArtifact | ||
| if result := issueArtifactRequest(ctx, deps, input.IssueArtifactCoordinates, 0, http.MethodPost, nil, body, &output, http.StatusCreated); result != nil { | ||
| return result, nil, nil | ||
| } | ||
| return issueArtifactResult(ctx, deps, input.IssueArtifactCoordinates, &output, 0) | ||
| }) | ||
| } | ||
|
|
||
| // UpdateIssueArtifact updates only the supplied name and/or body. | ||
| func UpdateIssueArtifact(t translations.TranslationHelperFunc) inventory.ServerTool { | ||
| return newIssueArtifactTool(t, "update_issue_artifact", "Update issue artifact", | ||
| "Update an issue artifact by its issue-scoped number. Supply name, body, or both. An empty body clears it; type is immutable.", | ||
| false, true, nil, | ||
| func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input UpdateIssueArtifactInput) (*mcp.CallToolResult, *IssueArtifact, error) { | ||
| if input.ArtifactNumber <= 0 { | ||
| return utils.NewToolResultError("artifact_number must be positive"), nil, nil | ||
| } | ||
| if input.Name == nil && input.Body == nil { | ||
| return utils.NewToolResultError("supply name, body, or both"), nil, nil | ||
| } | ||
| if input.Name != nil && strings.TrimSpace(*input.Name) == "" { | ||
| return utils.NewToolResultError("name must not be empty"), nil, nil | ||
| } | ||
| body := struct { | ||
| Name *string `json:"name,omitempty"` | ||
| Body *string `json:"body,omitempty"` | ||
| }{input.Name, input.Body} | ||
| var output IssueArtifact | ||
| if result := issueArtifactRequest(ctx, deps, input.IssueArtifactCoordinates, input.ArtifactNumber, http.MethodPatch, nil, body, &output, http.StatusOK); result != nil { | ||
| return result, nil, nil | ||
| } | ||
| return issueArtifactResult(ctx, deps, input.IssueArtifactCoordinates, &output, input.ArtifactNumber) | ||
| }) | ||
| } | ||
|
|
||
| // DeleteIssueArtifact deletes an artifact by its issue-scoped number. | ||
| func DeleteIssueArtifact(t translations.TranslationHelperFunc) inventory.ServerTool { | ||
| return newIssueArtifactTool(t, "delete_issue_artifact", "Delete issue artifact", | ||
| "Delete a GitHub issue artifact by its issue-scoped number, not its global ID.", | ||
| false, true, nil, | ||
| func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input GetIssueArtifactInput) (*mcp.CallToolResult, *DeleteIssueArtifactOutput, error) { | ||
| if input.ArtifactNumber <= 0 { | ||
| return utils.NewToolResultError("artifact_number must be positive"), nil, nil | ||
| } | ||
| if result := issueArtifactRequest(ctx, deps, input.IssueArtifactCoordinates, input.ArtifactNumber, http.MethodDelete, nil, nil, nil, http.StatusNoContent); result != nil { | ||
| return result, nil, nil | ||
| } | ||
| output := &DeleteIssueArtifactOutput{IssueNumber: input.IssueNumber, ArtifactNumber: input.ArtifactNumber, Message: "artifact deleted"} | ||
| return MarshalledTextResult(output), output, nil | ||
| }) | ||
| } | ||
|
|
||
| // GetIssueArtifact reads one artifact, including its body. | ||
| func GetIssueArtifact(t translations.TranslationHelperFunc) inventory.ServerTool { | ||
| return newIssueArtifactTool(t, "get_issue_artifact", "Get issue artifact", | ||
| "Get a GitHub issue artifact including its body by its issue-scoped number, not its global ID.", | ||
| true, false, nil, | ||
| func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input GetIssueArtifactInput) (*mcp.CallToolResult, *IssueArtifact, error) { | ||
| if input.ArtifactNumber <= 0 { | ||
| return utils.NewToolResultError("artifact_number must be positive"), nil, nil | ||
| } | ||
| var response struct { | ||
| Artifact *IssueArtifact `json:"artifact"` | ||
| } | ||
| if result := issueArtifactRequest(ctx, deps, input.IssueArtifactCoordinates, input.ArtifactNumber, http.MethodGet, nil, nil, &response, http.StatusOK); result != nil { | ||
| return result, nil, nil | ||
| } | ||
| return issueArtifactResult(ctx, deps, input.IssueArtifactCoordinates, response.Artifact, input.ArtifactNumber) | ||
| }) | ||
| } | ||
|
|
||
| // ListIssueArtifacts reads an explicit page of artifacts, including bodies. | ||
| func ListIssueArtifacts(t translations.TranslationHelperFunc) inventory.ServerTool { | ||
| return newIssueArtifactTool(t, "list_issue_artifacts", "List issue artifacts", | ||
| "List a page of GitHub issue artifacts including their bodies, optionally filtered by type. No Link headers are provided: request successive pages until a page is shorter than perPage.", | ||
| true, false, []inventory.SchemaEnum{{Path: "type", Values: issueArtifactTypes}}, | ||
| func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input ListIssueArtifactsInput) (*mcp.CallToolResult, *ListIssueArtifactsOutput, error) { | ||
| if input.Type != "" && !slices.Contains(issueArtifactTypes, input.Type) { | ||
| return utils.NewToolResultError("unsupported artifact type"), nil, nil | ||
| } | ||
| if input.Page < 0 || input.PerPage < 0 || input.PerPage > 100 { | ||
| return utils.NewToolResultError("page must be positive and perPage must be between 1 and 100"), nil, nil | ||
| } | ||
| if input.Page == 0 { | ||
| input.Page = 1 | ||
| } | ||
| if input.PerPage == 0 { | ||
| input.PerPage = 30 | ||
| } | ||
| query := url.Values{"page": {strconv.Itoa(input.Page)}, "per_page": {strconv.Itoa(input.PerPage)}} | ||
| if input.Type != "" { | ||
| query.Set("type", input.Type) | ||
| } | ||
| var artifacts []IssueArtifact | ||
| if result := issueArtifactRequest(ctx, deps, input.IssueArtifactCoordinates, 0, http.MethodGet, query, nil, &artifacts, http.StatusOK); result != nil { | ||
| return result, nil, nil | ||
| } | ||
| if artifacts == nil || len(artifacts) > input.PerPage { | ||
| return utils.NewToolResultError("invalid artifact list response"), nil, nil | ||
| } | ||
| for i := range artifacts { | ||
| if err := validateIssueArtifact(&artifacts[i], 0); err != nil { | ||
| return utils.NewToolResultErrorFromErr("invalid artifact list response", err), nil, nil | ||
| } | ||
| sanitizeIssueArtifact(&artifacts[i]) | ||
| } | ||
| output := &ListIssueArtifactsOutput{Artifacts: artifacts, Page: input.Page, PerPage: input.PerPage} | ||
| result := attachRepoVisibilityIFCLabelLazy(ctx, deps, input.Owner, input.Repo, MarshalledTextResult(output), ifc.LabelRepoUserContent) | ||
| return result, output, nil | ||
| }) | ||
| } | ||
|
|
||
| func issueArtifactRequest(ctx context.Context, deps ToolDependencies, coordinates IssueArtifactCoordinates, number int, method string, query url.Values, body, output any, status int) *mcp.CallToolResult { | ||
| for _, segment := range []string{coordinates.Owner, coordinates.Repo} { | ||
| if strings.TrimSpace(segment) == "" || segment == "." || segment == ".." || strings.ContainsAny(segment, "/\\") { | ||
| return utils.NewToolResultError("owner and repo must be nonempty repository path segments") | ||
| } | ||
| } | ||
| if coordinates.IssueNumber <= 0 { | ||
| return utils.NewToolResultError("issue_number must be positive") | ||
| } | ||
| // Artifact responses do not identify authors, so lockdown cannot establish | ||
| // whether their content is safe to return. | ||
| if deps.GetFlags(ctx).LockdownMode { | ||
| return utils.NewToolResultError("issue artifacts are unavailable in lockdown mode because artifact authors cannot be verified") | ||
| } | ||
| client, err := deps.GetClient(ctx) | ||
| if err != nil { | ||
| return utils.NewToolResultErrorFromErr("failed to get GitHub client", err) | ||
| } | ||
| path := fmt.Sprintf("repos/%s/%s/issues/%d/artifacts", url.PathEscape(coordinates.Owner), url.PathEscape(coordinates.Repo), coordinates.IssueNumber) | ||
| if number > 0 { | ||
| path += "/" + strconv.Itoa(number) | ||
| } | ||
| if len(query) > 0 { | ||
| path += "?" + query.Encode() | ||
| } | ||
| request, err := client.NewRequest(ctx, method, path, body) | ||
| if err != nil { | ||
| return utils.NewToolResultErrorFromErr("failed to create issue artifact request", err) | ||
| } | ||
| response, err := client.Do(request, output) | ||
| if err != nil { | ||
| return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to "+strings.ToLower(method)+" issue artifact", response, err) | ||
| } | ||
| if response.StatusCode != status { | ||
| return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "unexpected issue artifact response", response, nil) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func validateIssueArtifact(artifact *IssueArtifact, number int) error { | ||
| if artifact == nil || artifact.ID <= 0 || artifact.Number <= 0 || artifact.Type == "" || strings.TrimSpace(artifact.Name) == "" { | ||
| return fmt.Errorf("response omitted required artifact metadata") | ||
| } | ||
| if number > 0 && artifact.Number != number { | ||
| return fmt.Errorf("response artifact number %d does not match requested number %d", artifact.Number, number) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func sanitizeIssueArtifact(artifact *IssueArtifact) { | ||
| artifact.Name = sanitize.PlainText(artifact.Name) | ||
| artifact.Body = sanitize.Content(artifact.Body) | ||
|
veralizeth marked this conversation as resolved.
Dismissed
|
||
| } | ||
|
|
||
| func issueArtifactResult(ctx context.Context, deps ToolDependencies, coordinates IssueArtifactCoordinates, artifact *IssueArtifact, number int) (*mcp.CallToolResult, *IssueArtifact, error) { | ||
| if err := validateIssueArtifact(artifact, number); err != nil { | ||
| return utils.NewToolResultErrorFromErr("invalid artifact response", err), nil, nil | ||
| } | ||
| sanitizeIssueArtifact(artifact) | ||
| result := attachRepoVisibilityIFCLabelLazy(ctx, deps, coordinates.Owner, coordinates.Repo, MarshalledTextResult(artifact), ifc.LabelRepoUserContent) | ||
| return result, artifact, nil | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
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 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.