diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 2924b9f3d7..376da4a5bb 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -37,6 +37,37 @@ end users. Insiders-only flags are not user-toggleable. ## Declaring tool availability +### Issue artifacts + +Enable `--features=issue_artifacts` (or `GITHUB_FEATURES=issue_artifacts`) to +expose `issue_artifact_read` (`method: list | get`) and `issue_artifact_write` +(`method: create | update | delete`) in the `issues` toolset. +This flag is **off by default**, including in Insiders mode. Read-only mode +exposes only `issue_artifact_read`. + +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. +Both tools fail closed in lockdown mode because the reference artifact +response does not identify authors for content-trust checks. + +All methods require `owner`, `repo`, and `issue_number`. Get, update, and delete +also require `artifact_number`. Create requires `type`, `name`, and `body`; +an empty body is allowed. Update requires `name`, `body`, or both. Method-specific +fields are rejected when used with another method. + +Get, create, and update return an `artifact` object. List returns a `list` object +containing `artifacts`, `page`, and `perPage`; delete returns a `deleted` object +containing `issue_number`, `artifact_number`, and a confirmation message. + 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 @@ -395,6 +426,30 @@ as output formatting) won't appear here. - 'blocked_by' - the subject issue is blocked by the related issue. - 'blocking' - the subject issue blocks the related issue. (string, required) +### `issue_artifacts` + +- **issue_artifact_read** - Read issue artifacts + - **OAuth Challenge Scopes**: `repo` + - `artifact_number`: Required for get: positive issue-scoped artifact number, not its global ID (integer, optional) + - `issue_number`: Positive issue number (integer, required) + - `method`: (string, required) + - `owner`: Repository owner (string, required) + - `page`: For list only: page number, starting at 1 (default 1) (integer, optional) + - `perPage`: For list only: results per page, from 1 to 100 (default 30) (integer, optional) + - `repo`: Repository name (string, required) + - `type`: (string, optional) + +- **issue_artifact_write** - Write issue artifacts + - **OAuth Challenge Scopes**: `repo` + - `artifact_number`: Required for update and delete: positive issue-scoped artifact number, not its global ID (integer, optional) + - `body`: Required for create, optional for update: Markdown body; an empty string clears it (string, optional) + - `issue_number`: Positive issue number (integer, required) + - `method`: (string, required) + - `name`: Required for create, optional for update: nonempty artifact name (string, optional) + - `owner`: Repository owner (string, required) + - `repo`: Repository name (string, required) + - `type`: (string, optional) + ### `duplicate_detection` - **find_duplicate** - Find duplicate issues diff --git a/pkg/github/__toolsnaps__/issue_artifact_read_ff_issue_artifacts.snap b/pkg/github/__toolsnaps__/issue_artifact_read_ff_issue_artifacts.snap new file mode 100644 index 0000000000..743cbffeac --- /dev/null +++ b/pkg/github/__toolsnaps__/issue_artifact_read_ff_issue_artifacts.snap @@ -0,0 +1,182 @@ +{ + "annotations": { + "destructiveHint": false, + "idempotentHint": false, + "readOnlyHint": true, + "title": "Read issue artifacts" + }, + "description": "List or get GitHub issue artifacts including their bodies. Get requires artifact_number (issue-scoped, not its global ID). List accepts an optional type filter and page/perPage; request successive pages until a page is shorter than perPage because no Link headers are provided.", + "icons": [ + { + "mimeType": "image/png", + "src": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAAC2UlEQVRIicWVMUyTaRjHf/+vVUpOJOdilOYoUvVreq2CDmLOwdlAS5xMbrrhBifjYG7QjYuJw108nZx1huLgYlw0QshxChUK2koxHHcuCmgsFfieG0or3kGPSoz/8X3f5/97nvfN8z7wmaVqm9FodFfR/EnMugy5giCAwYwgI9Ff9Hl9L9Lp1zUBgsGO+rqGwnlhF4CdwBSyAUwvS1G220zHBSFgHtmVxYWvfp2ZGSj8LyAcjgfZphTQZtAr7OdsZvSP9RIJR+LtQhcNusF+d1aUfPp05M8NAavmg8AOk3cmN56+s1Hpa7U/cviUYbcw3jgex9ZCKoBgsKM+0PDuAdBq2He5zOiTzZiXtc+NxRw598GeLRcWTuTz+UUAp3ygrqFwHmgzeWdqNQd4PpFOy/gedNQfaDz3UQXRaHRX0fNPGdzNZUZO12q+VuHIoV7g5Hu/1/IinX7tABTNnwR2SvRsxRzAzOsBGuuWfAkoX5FZFzCVHR95tFVAbiI9DEwj6wLwl5YVMTRYLTDsxjuRrgGYYz/kxkbvbXRWMOBB24cKYI9ks1X8hXQDaAaa5XG9WjImZgV71wI+m8qAv8y0t1pSmP0ITANTmHO2qqvRZDALlTdgHKyjWkx2YvQ2cHtTacMxYBhWK5DoF4TCkXj7Jg02VKsbOwI0Y+qvALZrOQXMC13cKkDSJWBuJUCqAhgbG3uF7IpBcn/k8KlPNQ+78U5QQuhy/vHjuQoAYPndwi9gw4bd2ufGYrWatxz8No50ExhaKsz9Vl6vAPL5/KKzoiTGG0fOg1oqCbvxTp/juw/M+zynu/yTwjoD58CBQ02ez/pAR4E+M69ntf3/o1Y3dqR050oAQz7P6Z6cfPRRw647MkOhUMAfaDyH+AloBKYNPRT292rQHoMOSp09J3TZlt5ezWazxX97VR3638RiX9ct+RImSyBcrDT0ETMyMp4ptRIgVX7QL6J/ALSUEwJ5rdg2AAAAAElFTkSuQmCC", + "theme": "light" + }, + { + "mimeType": "image/png", + "src": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAABxElEQVRIibWVvW4TURCFv0vlNEDcIHAkKAERJOIKSjoUArwBPwUFFaKIIngAJARCPIgdh4cgRBYt6ZIAEYEqdhpEw0fhCbkKa68dxyNtsfNzzpnZu3NhwpYGBdUqcA+4A1wEZiK0DawD74FWSml3JAJ1CngGLAIngU1gFfgZKWeAG8AFoAu8At6mlH6VtqTOqJ/UP2pDnRuQO6c27VlbrQ0Dvq121Fulag7q5qPmW18SdSqUd9Qrw4Jn9bNR21YrRQkvYixDKy/AuB3jWjocqKpdtXFU8AxrOTqZzp2PgvnaMRDUA+tB7mypG+OCZ3hbahPgRPguAR9LihaicEu9WcKxClzOi/fU1wPAk7rjgX0uEfNG3cs7mJjtE+wA5/olpZQEHgNf6K2NJyW4NeD7v7c4WptjSc0svlMjdzyM2fbdOyOA7x/T+7mzGj9H8xgIWuquevpw4HmsivkxwBdC/WJRsBKLqqPOHgH8aqybtcJlF0m1WLndUToJ5V31q9r3NOYk7Wh1Wa0PyK3HzA3l/4H3uzIrwFNgCThF7/x/AH5EylngOnAe6AAvgXcppd9DEWRE08DdeIou/RVgJaXUGYQzUfsL+zmwV7BtIq0AAAAASUVORK5CYII=", + "theme": "dark" + } + ], + "inputSchema": { + "additionalProperties": false, + "properties": { + "artifact_number": { + "description": "Required for get: positive issue-scoped artifact number, not its global ID", + "minimum": 1, + "type": "integer" + }, + "issue_number": { + "description": "Positive issue number", + "minimum": 1, + "type": "integer" + }, + "method": { + "enum": [ + "list", + "get" + ], + "type": "string" + }, + "owner": { + "description": "Repository owner", + "minLength": 1, + "type": "string", + "x-mcp-header": "owner" + }, + "page": { + "description": "For list only: page number, starting at 1 (default 1)", + "minimum": 1, + "type": "integer" + }, + "perPage": { + "description": "For list only: results per page, from 1 to 100 (default 30)", + "maximum": 100, + "minimum": 1, + "type": "integer" + }, + "repo": { + "description": "Repository name", + "minLength": 1, + "type": "string", + "x-mcp-header": "repo" + }, + "type": { + "enum": [ + "plan", + "generic", + "link", + "instruction", + "skill", + "mcp_config", + "reference" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "method" + ], + "type": "object" + }, + "name": "issue_artifact_read", + "outputSchema": { + "additionalProperties": false, + "properties": { + "artifact": { + "additionalProperties": false, + "description": "Artifact returned by get", + "properties": { + "body": { + "type": "string" + }, + "id": { + "type": "integer" + }, + "name": { + "type": "string" + }, + "number": { + "type": "integer" + }, + "type": { + "type": "string" + } + }, + "required": [ + "id", + "number", + "type", + "name", + "body" + ], + "type": [ + "null", + "object" + ] + }, + "list": { + "additionalProperties": false, + "description": "Page of artifacts returned by list", + "properties": { + "artifacts": { + "items": { + "additionalProperties": false, + "properties": { + "body": { + "type": "string" + }, + "id": { + "type": "integer" + }, + "name": { + "type": "string" + }, + "number": { + "type": "integer" + }, + "type": { + "type": "string" + } + }, + "required": [ + "id", + "number", + "type", + "name", + "body" + ], + "type": "object" + }, + "type": [ + "null", + "array" + ] + }, + "page": { + "type": "integer" + }, + "perPage": { + "type": "integer" + } + }, + "required": [ + "artifacts", + "page", + "perPage" + ], + "type": [ + "null", + "object" + ] + } + }, + "type": [ + "null", + "object" + ] + } +} \ No newline at end of file diff --git a/pkg/github/__toolsnaps__/issue_artifact_write_ff_issue_artifacts.snap b/pkg/github/__toolsnaps__/issue_artifact_write_ff_issue_artifacts.snap new file mode 100644 index 0000000000..b0be2ed9c7 --- /dev/null +++ b/pkg/github/__toolsnaps__/issue_artifact_write_ff_issue_artifacts.snap @@ -0,0 +1,150 @@ +{ + "annotations": { + "destructiveHint": true, + "idempotentHint": false, + "readOnlyHint": false, + "title": "Write issue artifacts" + }, + "description": "Create, update, or delete a GitHub issue artifact. Create requires type, name, and body (which may be empty). Update requires artifact_number and name, body, or both; an empty body clears it and type is immutable. Delete requires artifact_number. Artifact numbers are issue-scoped, not global IDs. Experimental: endpoint availability and access requirements are unverified.", + "icons": [ + { + "mimeType": "image/png", + "src": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAAC2UlEQVRIicWVMUyTaRjHf/+vVUpOJOdilOYoUvVreq2CDmLOwdlAS5xMbrrhBifjYG7QjYuJw108nZx1huLgYlw0QshxChUK2koxHHcuCmgsFfieG0or3kGPSoz/8X3f5/97nvfN8z7wmaVqm9FodFfR/EnMugy5giCAwYwgI9Ff9Hl9L9Lp1zUBgsGO+rqGwnlhF4CdwBSyAUwvS1G220zHBSFgHtmVxYWvfp2ZGSj8LyAcjgfZphTQZtAr7OdsZvSP9RIJR+LtQhcNusF+d1aUfPp05M8NAavmg8AOk3cmN56+s1Hpa7U/cviUYbcw3jgex9ZCKoBgsKM+0PDuAdBq2He5zOiTzZiXtc+NxRw598GeLRcWTuTz+UUAp3ygrqFwHmgzeWdqNQd4PpFOy/gedNQfaDz3UQXRaHRX0fNPGdzNZUZO12q+VuHIoV7g5Hu/1/IinX7tABTNnwR2SvRsxRzAzOsBGuuWfAkoX5FZFzCVHR95tFVAbiI9DEwj6wLwl5YVMTRYLTDsxjuRrgGYYz/kxkbvbXRWMOBB24cKYI9ks1X8hXQDaAaa5XG9WjImZgV71wI+m8qAv8y0t1pSmP0ITANTmHO2qqvRZDALlTdgHKyjWkx2YvQ2cHtTacMxYBhWK5DoF4TCkXj7Jg02VKsbOwI0Y+qvALZrOQXMC13cKkDSJWBuJUCqAhgbG3uF7IpBcn/k8KlPNQ+78U5QQuhy/vHjuQoAYPndwi9gw4bd2ufGYrWatxz8No50ExhaKsz9Vl6vAPL5/KKzoiTGG0fOg1oqCbvxTp/juw/M+zynu/yTwjoD58CBQ02ez/pAR4E+M69ntf3/o1Y3dqR050oAQz7P6Z6cfPRRw647MkOhUMAfaDyH+AloBKYNPRT292rQHoMOSp09J3TZlt5ezWazxX97VR3638RiX9ct+RImSyBcrDT0ETMyMp4ptRIgVX7QL6J/ALSUEwJ5rdg2AAAAAElFTkSuQmCC", + "theme": "light" + }, + { + "mimeType": "image/png", + "src": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABgAAAAYCAYAAADgdz34AAAABmJLR0QA/wD/AP+gvaeTAAABxElEQVRIibWVvW4TURCFv0vlNEDcIHAkKAERJOIKSjoUArwBPwUFFaKIIngAJARCPIgdh4cgRBYt6ZIAEYEqdhpEw0fhCbkKa68dxyNtsfNzzpnZu3NhwpYGBdUqcA+4A1wEZiK0DawD74FWSml3JAJ1CngGLAIngU1gFfgZKWeAG8AFoAu8At6mlH6VtqTOqJ/UP2pDnRuQO6c27VlbrQ0Dvq121Fulag7q5qPmW18SdSqUd9Qrw4Jn9bNR21YrRQkvYixDKy/AuB3jWjocqKpdtXFU8AxrOTqZzp2PgvnaMRDUA+tB7mypG+OCZ3hbahPgRPguAR9LihaicEu9WcKxClzOi/fU1wPAk7rjgX0uEfNG3cs7mJjtE+wA5/olpZQEHgNf6K2NJyW4NeD7v7c4WptjSc0svlMjdzyM2fbdOyOA7x/T+7mzGj9H8xgIWuquevpw4HmsivkxwBdC/WJRsBKLqqPOHgH8aqybtcJlF0m1WLndUToJ5V31q9r3NOYk7Wh1Wa0PyK3HzA3l/4H3uzIrwFNgCThF7/x/AH5EylngOnAe6AAvgXcppd9DEWRE08DdeIou/RVgJaXUGYQzUfsL+zmwV7BtIq0AAAAASUVORK5CYII=", + "theme": "dark" + } + ], + "inputSchema": { + "additionalProperties": false, + "properties": { + "artifact_number": { + "description": "Required for update and delete: positive issue-scoped artifact number, not its global ID", + "minimum": 1, + "type": "integer" + }, + "body": { + "description": "Required for create, optional for update: Markdown body; an empty string clears it", + "type": "string" + }, + "issue_number": { + "description": "Positive issue number", + "minimum": 1, + "type": "integer" + }, + "method": { + "enum": [ + "create", + "update", + "delete" + ], + "type": "string" + }, + "name": { + "description": "Required for create, optional for update: nonempty artifact name", + "minLength": 1, + "type": "string" + }, + "owner": { + "description": "Repository owner", + "minLength": 1, + "type": "string", + "x-mcp-header": "owner" + }, + "repo": { + "description": "Repository name", + "minLength": 1, + "type": "string", + "x-mcp-header": "repo" + }, + "type": { + "enum": [ + "plan", + "generic", + "link", + "instruction", + "skill", + "mcp_config", + "reference" + ], + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "issue_number", + "method" + ], + "type": "object" + }, + "name": "issue_artifact_write", + "outputSchema": { + "additionalProperties": false, + "properties": { + "artifact": { + "additionalProperties": false, + "description": "Artifact returned by create or update", + "properties": { + "body": { + "type": "string" + }, + "id": { + "type": "integer" + }, + "name": { + "type": "string" + }, + "number": { + "type": "integer" + }, + "type": { + "type": "string" + } + }, + "required": [ + "id", + "number", + "type", + "name", + "body" + ], + "type": [ + "null", + "object" + ] + }, + "deleted": { + "additionalProperties": false, + "description": "Confirmation returned by delete", + "properties": { + "artifact_number": { + "type": "integer" + }, + "issue_number": { + "type": "integer" + }, + "message": { + "type": "string" + } + }, + "required": [ + "issue_number", + "artifact_number", + "message" + ], + "type": [ + "null", + "object" + ] + } + }, + "type": [ + "null", + "object" + ] + } +} \ No newline at end of file diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 1bf679f5c9..4957c2f71e 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -28,6 +28,10 @@ const FeatureFlagFileBlame = "file_blame" // unless explicitly opted in. const FeatureFlagIssueDependencies = "issue_dependencies" +// FeatureFlagIssueArtifacts explicitly opts into the issue artifact +// endpoints. Access requirements are unverified; insiders mode does not enable it. +const FeatureFlagIssueArtifacts = "issue_artifacts" + // FeatureFlagDuplicateDetection is the feature flag name for the find_duplicate // tool, which returns ranked duplicate candidates for an existing issue. It is // gated so the extra tool is not advertised by default, and is deliberately @@ -51,6 +55,7 @@ var AllowedFeatureFlags = []string{ FeatureFlagPullRequestsGranular, FeatureFlagFileBlame, FeatureFlagIssueDependencies, + FeatureFlagIssueArtifacts, FeatureFlagDuplicateDetection, FeatureFlagThreadResolutionReason, } diff --git a/pkg/github/issue_artifact_types.go b/pkg/github/issue_artifact_types.go new file mode 100644 index 0000000000..fe87a29019 --- /dev/null +++ b/pkg/github/issue_artifact_types.go @@ -0,0 +1,65 @@ +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"` +} + +// 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"` +} + +// IssueArtifactReadInput selects a list or get operation on an issue's artifacts. +type IssueArtifactReadInput struct { + IssueArtifactCoordinates + Method string `json:"method" jsonschema:"Read operation: list or get"` + ArtifactNumber int `json:"artifact_number,omitempty" jsonschema:"Required for get: positive issue-scoped artifact number, not its global ID"` + Type string `json:"type,omitempty" jsonschema:"Optional artifact type filter for list only"` + Page int `json:"page,omitempty" jsonschema:"For list only: page number, starting at 1 (default 1)"` + PerPage int `json:"perPage,omitempty" jsonschema:"For list only: results per page, from 1 to 100 (default 30)"` +} + +// IssueArtifactWriteInput selects a create, update, or delete operation. +type IssueArtifactWriteInput struct { + IssueArtifactCoordinates + Method string `json:"method" jsonschema:"Write operation: create, update, or delete"` + ArtifactNumber int `json:"artifact_number,omitempty" jsonschema:"Required for update and delete: positive issue-scoped artifact number, not its global ID"` + Type string `json:"type,omitempty" jsonschema:"Required for create only: artifact type; immutable after creation"` + Name *string `json:"name,omitempty" jsonschema:"Required for create, optional for update: nonempty artifact name"` + Body *string `json:"body,omitempty" jsonschema:"Required for create, optional for update: Markdown body; an empty string clears it"` +} + +// IssueArtifactReadOutput contains the result of the selected read method. +type IssueArtifactReadOutput struct { + Artifact *IssueArtifact `json:"artifact,omitempty" jsonschema:"Artifact returned by get"` + List *ListIssueArtifactsOutput `json:"list,omitempty" jsonschema:"Page of artifacts returned by list"` +} + +// IssueArtifactWriteOutput contains the result of the selected write method. +type IssueArtifactWriteOutput struct { + Artifact *IssueArtifact `json:"artifact,omitempty" jsonschema:"Artifact returned by create or update"` + Deleted *DeleteIssueArtifactOutput `json:"deleted,omitempty" jsonschema:"Confirmation returned by delete"` +} + +// 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"` +} diff --git a/pkg/github/issue_artifacts.go b/pkg/github/issue_artifacts.go new file mode 100644 index 0000000000..e14add84a9 --- /dev/null +++ b/pkg/github/issue_artifacts.go @@ -0,0 +1,258 @@ +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/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 + } + st := NewToolWithSchemaOptions(ToolsetMetadataIssues, mcp.Tool{ + Name: name, Description: t(key+"_DESCRIPTION", description), + InputSchema: schema, + 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 +} + +// IssueArtifactWrite creates, updates, or deletes an issue artifact. +func IssueArtifactWrite(t translations.TranslationHelperFunc) inventory.ServerTool { + return newIssueArtifactTool(t, "issue_artifact_write", "Write issue artifacts", + "Create, update, or delete a GitHub issue artifact. Create requires type, name, and body (which may be empty). Update requires artifact_number and name, body, or both; an empty body clears it and type is immutable. Delete requires artifact_number. Artifact numbers are issue-scoped, not global IDs. Experimental: endpoint availability and access requirements are unverified.", + false, true, []inventory.SchemaEnum{{Path: "method", Values: []string{"create", "update", "delete"}}, {Path: "type", Values: issueArtifactTypes}}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input IssueArtifactWriteInput) (*mcp.CallToolResult, *IssueArtifactWriteOutput, error) { + var artifact IssueArtifact + switch input.Method { + case "create": + if input.ArtifactNumber != 0 { + return utils.NewToolResultError("artifact_number is not supported for create"), nil, nil + } + if !slices.Contains(issueArtifactTypes, input.Type) { + return utils.NewToolResultError("unsupported artifact type"), nil, nil + } + if input.Name == nil || strings.TrimSpace(*input.Name) == "" { + return utils.NewToolResultError("name must not be empty"), nil, nil + } + if input.Body == nil { + return utils.NewToolResultError("body is required for create; use an empty string for an empty body"), nil, nil + } + body := struct { + Type string `json:"type"` + Name string `json:"name"` + Body string `json:"body"` + }{input.Type, *input.Name, *input.Body} + if result := issueArtifactRequest(ctx, deps, input.IssueArtifactCoordinates, 0, http.MethodPost, nil, body, &artifact, http.StatusCreated); result != nil { + return result, nil, nil + } + case "update": + if input.ArtifactNumber <= 0 { + return utils.NewToolResultError("artifact_number must be positive"), nil, nil + } + if input.Type != "" { + return utils.NewToolResultError("type is immutable; only supported for create"), 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} + if result := issueArtifactRequest(ctx, deps, input.IssueArtifactCoordinates, input.ArtifactNumber, http.MethodPatch, nil, body, &artifact, http.StatusOK); result != nil { + return result, nil, nil + } + case "delete": + if input.ArtifactNumber <= 0 { + return utils.NewToolResultError("artifact_number must be positive"), nil, nil + } + if input.Type != "" || input.Name != nil || input.Body != nil { + return utils.NewToolResultError("type, name, and body are not supported for delete"), 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 := &IssueArtifactWriteOutput{Deleted: &DeleteIssueArtifactOutput{IssueNumber: input.IssueNumber, ArtifactNumber: input.ArtifactNumber, Message: "artifact deleted"}} + return MarshalledTextResult(output), output, nil + default: + return utils.NewToolResultError("method must be create, update, or delete"), nil, nil + } + if err := validateIssueArtifact(&artifact, input.ArtifactNumber); err != nil { + return utils.NewToolResultErrorFromErr("invalid artifact response", err), nil, nil + } + sanitizeIssueArtifact(&artifact) + output := &IssueArtifactWriteOutput{Artifact: &artifact} + result := attachRepoVisibilityIFCLabelLazy(ctx, deps, input.Owner, input.Repo, MarshalledTextResult(output), ifc.LabelRepoUserContent) + return result, output, nil + }) +} + +// IssueArtifactRead lists issue artifacts or gets one artifact, including bodies. +func IssueArtifactRead(t translations.TranslationHelperFunc) inventory.ServerTool { + return newIssueArtifactTool(t, "issue_artifact_read", "Read issue artifacts", + "List or get GitHub issue artifacts including their bodies. Get requires artifact_number (issue-scoped, not its global ID). List accepts an optional type filter and page/perPage; request successive pages until a page is shorter than perPage because no Link headers are provided.", + true, false, []inventory.SchemaEnum{{Path: "method", Values: []string{"list", "get"}}, {Path: "type", Values: issueArtifactTypes}}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input IssueArtifactReadInput) (*mcp.CallToolResult, *IssueArtifactReadOutput, error) { + switch input.Method { + case "get": + if input.ArtifactNumber <= 0 { + return utils.NewToolResultError("artifact_number must be positive"), nil, nil + } + if input.Type != "" || input.Page != 0 || input.PerPage != 0 { + return utils.NewToolResultError("type, page, and perPage are only supported for list"), 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 + } + if err := validateIssueArtifact(response.Artifact, input.ArtifactNumber); err != nil { + return utils.NewToolResultErrorFromErr("invalid artifact response", err), nil, nil + } + sanitizeIssueArtifact(response.Artifact) + output := &IssueArtifactReadOutput{Artifact: response.Artifact} + result := attachRepoVisibilityIFCLabelLazy(ctx, deps, input.Owner, input.Repo, MarshalledTextResult(output), ifc.LabelRepoUserContent) + return result, output, nil + case "list": + if input.ArtifactNumber != 0 { + return utils.NewToolResultError("artifact_number is only supported for get"), nil, nil + } + 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 := &IssueArtifactReadOutput{List: &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 + default: + return utils.NewToolResultError("method must be list or get"), nil, 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) +} diff --git a/pkg/github/issue_artifacts_test.go b/pkg/github/issue_artifacts_test.go new file mode 100644 index 0000000000..1d1a897faa --- /dev/null +++ b/pkg/github/issue_artifacts_test.go @@ -0,0 +1,317 @@ +package github + +import ( + "context" + "encoding/json" + "maps" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/github/github-mcp-server/v2/internal/toolsnaps" + "github.com/github/github-mcp-server/v2/pkg/inventory" + "github.com/github/github-mcp-server/v2/pkg/translations" + gogithub "github.com/google/go-github/v92/github" + "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func artifactTestTools() []inventory.ServerTool { + t := translations.NullTranslationHelper + return []inventory.ServerTool{IssueArtifactRead(t), IssueArtifactWrite(t)} +} + +func artifactTestOperation(index int) (inventory.ServerTool, string) { + method := []string{"create", "update", "delete", "list", "get"}[index] + tool := IssueArtifactWrite(translations.NullTranslationHelper) + if method == "list" || method == "get" { + tool = IssueArtifactRead(translations.NullTranslationHelper) + } + return tool, method +} + +func TestIssueArtifactTools(t *testing.T) { + session := artifactTestSession(t, artifactTestTools(), BaseDeps{}) + list, err := session.ListTools(context.Background(), nil) + require.NoError(t, err) + require.Len(t, list.Tools, 2) + registered := make(map[string]*mcp.Tool, len(list.Tools)) + for _, tool := range list.Tools { + registered[tool.Name] = tool + } + for _, tool := range artifactTestTools() { + t.Run(tool.Tool.Name, func(t *testing.T) { + wireTool := registered[tool.Tool.Name] + require.NotNil(t, wireTool) + require.NoError(t, toolsnaps.Test(tool.Tool.Name+"_ff_"+FeatureFlagIssueArtifacts, wireTool)) + assert.Equal(t, ToolsetMetadataIssues.ID, tool.Toolset.ID) + assert.Equal(t, []inventory.FeatureFlag{FeatureFlagIssueArtifacts}, tool.FeatureRule.Features()) + schema := tool.Tool.InputSchema.(*jsonschema.Schema) + assert.ElementsMatch(t, schema.Required, []string{"method", "owner", "repo", "issue_number"}) + assert.NotNil(t, wireTool.OutputSchema) + assert.Equal(t, tool.Tool.Name == "issue_artifact_read", tool.IsReadOnly()) + assert.Equal(t, tool.Tool.Name == "issue_artifact_write", *tool.Tool.Annotations.DestructiveHint) + methods := []string{"list", "get"} + if tool.Tool.Name == "issue_artifact_write" { + methods = []string{"create", "update", "delete"} + } + var actualMethods []string + for _, method := range schema.Properties["method"].Enum { + actualMethods = append(actualMethods, method.(string)) + } + assert.ElementsMatch(t, methods, actualMethods) + }) + } + assert.False(t, ResolveFeatureFlags(nil, true)[FeatureFlagIssueArtifacts]) + assert.True(t, ResolveFeatureFlags([]string{FeatureFlagIssueArtifacts}, false)[FeatureFlagIssueArtifacts]) + for _, tc := range []struct { + name string + enabled bool + readOnly bool + count int + }{ + {"default", false, false, 0}, + {"enabled", true, false, 2}, + {"read only", true, true, 1}, + {"read only default", false, true, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + var flags []inventory.FeatureFlag + if tc.enabled { + flags = append(flags, FeatureFlagIssueArtifacts) + } + inv, err := inventory.NewBuilder().SetTools(AllTools(translations.NullTranslationHelper)). + WithReadOnly(tc.readOnly).WithFeatureChecker(featureCheckerFor(flags...)).Build() + require.NoError(t, err) + count := 0 + for _, tool := range inv.AvailableTools(context.Background()) { + if strings.HasPrefix(tool.Tool.Name, "issue_artifact_") { + count++ + if tc.readOnly { + assert.True(t, tool.IsReadOnly()) + } + } + } + assert.Equal(t, tc.count, count) + }) + } +} + +func artifactTestSession(t *testing.T, tools []inventory.ServerTool, deps BaseDeps) *mcp.ClientSession { + t.Helper() + server := mcp.NewServer(&mcp.Implementation{Name: "issue-artifacts-test", Version: "v1"}, nil) + server.AddReceivingMiddleware(InjectDepsMiddleware(deps)) + for _, tool := range tools { + tool.RegisterFunc(server, deps) + } + return connectCommentVisibilityClient(t, server, inventory.ProtocolVersionMultiRoundTrip) +} + +func callArtifactTool(t *testing.T, tool inventory.ServerTool, deps BaseDeps, args map[string]any) *mcp.CallToolResult { + t.Helper() + session := artifactTestSession(t, []inventory.ServerTool{tool}, deps) + result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: tool.Tool.Name, Arguments: args}) + require.NoError(t, err) + require.NotNil(t, result) + return result +} + +func TestIssueArtifactRequests(t *testing.T) { + for _, tc := range []struct { + name string + index int + method string + path string + args map[string]any + body string + status int + result string + }{ + {"create empty body", 0, "POST", "/repos/o/r/issues/12/artifacts", map[string]any{"type": "plan", "name": "Plan", "body": ""}, `{"type":"plan","name":"Plan","body":""}`, 201, `{"id":999,"number":3,"type":"plan","name":"Plan","body":""}`}, + {"update body only", 1, "PATCH", "/repos/o/r/issues/12/artifacts/3", map[string]any{"artifact_number": 3, "body": ""}, `{"body":""}`, 200, `{"id":999,"number":3,"type":"plan","name":"Plan","body":""}`}, + {"update name only", 1, "PATCH", "/repos/o/r/issues/12/artifacts/3", map[string]any{"artifact_number": 3, "name": "Renamed"}, `{"name":"Renamed"}`, 200, `{"id":999,"number":3,"type":"plan","name":"Renamed","body":"unchanged"}`}, + {"update both", 1, "PATCH", "/repos/o/r/issues/12/artifacts/3", map[string]any{"artifact_number": 3, "name": "Renamed", "body": "new"}, `{"name":"Renamed","body":"new"}`, 200, `{"id":999,"number":3,"type":"plan","name":"Renamed","body":"new"}`}, + {"delete", 2, "DELETE", "/repos/o/r/issues/12/artifacts/3", map[string]any{"artifact_number": 3}, "", 204, ""}, + {"list default", 3, "GET", "/repos/o/r/issues/12/artifacts?page=1&per_page=30", nil, "", 200, `[{"id":999,"number":3,"type":"plan","name":"Plan","body":"full body"}]`}, + {"list explicit page", 3, "GET", "/repos/o/r/issues/12/artifacts?page=2&per_page=1&type=plan", map[string]any{"page": 2, "perPage": 1, "type": "plan"}, "", 200, `[{"id":999,"number":3,"type":"plan","name":"Plan","body":"full body"}]`}, + {"list empty", 3, "GET", "/repos/o/r/issues/12/artifacts?page=1&per_page=30", nil, "", 200, `[]`}, + {"get envelope and future type", 4, "GET", "/repos/o/r/issues/12/artifacts/3", map[string]any{"artifact_number": 3}, "", 200, `{"artifact":{"id":999,"number":3,"type":"future_type","name":"Plan","body":""}}`}, + } { + t.Run(tc.name, func(t *testing.T) { + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls++ + assert.Equal(t, tc.method, r.Method) + assert.Equal(t, "/api/v3"+tc.path, r.URL.RequestURI()) + if tc.body != "" { + var body json.RawMessage + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.JSONEq(t, tc.body, string(body)) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.result)) + })) + defer server.Close() + client, err := gogithub.NewClient( + gogithub.WithHTTPClient(server.Client()), + gogithub.WithEnterpriseURLs(server.URL+"/api/v3/", server.URL+"/api/uploads/"), + ) + require.NoError(t, err) + args := map[string]any{"owner": "o", "repo": "r", "issue_number": 12} + maps.Copy(args, tc.args) + tool, method := artifactTestOperation(tc.index) + args["method"] = method + result := callArtifactTool(t, tool, BaseDeps{Client: client}, args) + require.False(t, result.IsError, "%s", getTextResult(t, result).Text) + assert.Equal(t, 1, calls) + text := getTextResult(t, result).Text + var structured any + require.NoError(t, json.Unmarshal([]byte(text), &structured)) + assert.Equal(t, structured, result.StructuredContent) + switch tc.index { + case 2: + assert.JSONEq(t, `{"deleted":{"issue_number":12,"artifact_number":3,"message":"artifact deleted"}}`, text) + case 3: + var readOutput IssueArtifactReadOutput + require.NoError(t, json.Unmarshal([]byte(text), &readOutput)) + require.Nil(t, readOutput.Artifact) + output := readOutput.List + require.NotNil(t, output) + if tc.name == "list empty" { + require.NotNil(t, output.Artifacts) + assert.Empty(t, output.Artifacts) + } else { + require.Len(t, output.Artifacts, 1) + assert.Equal(t, "full body", output.Artifacts[0].Body) + } + if tc.name == "list explicit page" { + assert.Equal(t, 2, output.Page) + assert.Equal(t, 1, output.PerPage) + } else { + assert.Equal(t, 1, output.Page) + assert.Equal(t, 30, output.PerPage) + } + default: + var output struct { + Artifact *IssueArtifact `json:"artifact"` + } + require.NoError(t, json.Unmarshal([]byte(text), &output)) + require.NotNil(t, output.Artifact) + assert.Equal(t, int64(999), output.Artifact.ID) + assert.Equal(t, 3, output.Artifact.Number) + } + }) + } +} + +func TestIssueArtifactInvalidInputs(t *testing.T) { + for _, tc := range []struct { + name string + index int + args map[string]any + }{ + {"missing body", 0, map[string]any{"type": "plan", "name": "Plan"}}, + {"empty name", 0, map[string]any{"type": "plan", "name": " ", "body": ""}}, + {"unsupported type", 0, map[string]any{"type": "unknown", "name": "Plan", "body": ""}}, + {"empty update", 1, map[string]any{"artifact_number": 3}}, + {"null update", 1, map[string]any{"artifact_number": 3, "body": nil}}, + {"immutable type", 1, map[string]any{"artifact_number": 3, "type": "plan", "body": ""}}, + {"blank replacement", 1, map[string]any{"artifact_number": 3, "name": " "}}, + {"global id not number", 4, map[string]any{"id": 999}}, + {"negative number", 4, map[string]any{"artifact_number": -1}}, + {"zero number", 2, map[string]any{"artifact_number": 0}}, + {"negative issue", 3, map[string]any{"issue_number": -1}}, + {"empty owner", 3, map[string]any{"owner": " "}}, + {"path traversal", 3, map[string]any{"repo": ".."}}, + {"slash in owner", 3, map[string]any{"owner": "o/r"}}, + {"negative page", 3, map[string]any{"page": -1}}, + {"zero page", 3, map[string]any{"page": 0}}, + {"too large page size", 3, map[string]any{"perPage": 101}}, + {"zero page size", 3, map[string]any{"perPage": 0}}, + {"unsupported filter", 3, map[string]any{"type": "unknown"}}, + {"missing read method", 3, map[string]any{"method": nil}}, + {"missing write method", 0, map[string]any{"method": nil}}, + {"unknown read method", 3, map[string]any{"method": "create"}}, + {"unknown write method", 0, map[string]any{"method": "get"}}, + {"get without number", 4, nil}, + {"update without number", 1, map[string]any{"body": ""}}, + {"delete without number", 2, nil}, + {"create without type", 0, map[string]any{"name": "Plan", "body": ""}}, + {"create without name", 0, map[string]any{"type": "plan", "body": ""}}, + {"create with null body", 0, map[string]any{"type": "plan", "name": "Plan", "body": nil}}, + {"create with number", 0, map[string]any{"artifact_number": 3, "type": "plan", "name": "Plan", "body": ""}}, + {"delete with body", 2, map[string]any{"artifact_number": 3, "body": ""}}, + {"delete with name", 2, map[string]any{"artifact_number": 3, "name": "Plan"}}, + {"delete with type", 2, map[string]any{"artifact_number": 3, "type": "plan"}}, + {"get with page", 4, map[string]any{"artifact_number": 3, "page": 1}}, + {"get with page size", 4, map[string]any{"artifact_number": 3, "perPage": 1}}, + {"get with filter", 4, map[string]any{"artifact_number": 3, "type": "plan"}}, + {"list with number", 3, map[string]any{"artifact_number": 3}}, + } { + t.Run(tc.name, func(t *testing.T) { + tool, method := artifactTestOperation(tc.index) + args := map[string]any{"owner": "o", "repo": "r", "issue_number": 12, "method": method} + maps.Copy(args, tc.args) + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + client, err := gogithub.NewClient( + gogithub.WithHTTPClient(server.Client()), + gogithub.WithEnterpriseURLs(server.URL+"/", server.URL+"/"), + ) + require.NoError(t, err) + result := callArtifactTool(t, tool, BaseDeps{Client: client}, args) + assert.True(t, result.IsError) + assert.Zero(t, calls, "invalid arguments must not make API requests") + }) + } +} + +func TestIssueArtifactFailures(t *testing.T) { + for index := range 5 { + tool, method := artifactTestOperation(index) + t.Run(method, func(t *testing.T) { + args := map[string]any{"owner": "o", "repo": "r", "issue_number": 12, "method": method} + switch method { + case "create": + args["name"], args["type"], args["body"] = "Plan", "plan", "" + case "update": + args["artifact_number"], args["body"] = 3, "" + case "get", "delete": + args["artifact_number"] = 3 + } + t.Run("lockdown", func(t *testing.T) { + result := callArtifactTool(t, tool, BaseDeps{Flags: FeatureFlags{LockdownMode: true}}, args) + assert.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "lockdown") + }) + for _, status := range []int{401, 403, 404, 422, 500} { + t.Run(http.StatusText(status), func(t *testing.T) { + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"message":"artifact access failed"}`)) + })) + defer server.Close() + client, err := gogithub.NewClient( + gogithub.WithHTTPClient(server.Client()), + gogithub.WithEnterpriseURLs(server.URL+"/", server.URL+"/"), + ) + require.NoError(t, err) + result := callArtifactTool(t, tool, BaseDeps{Client: client}, args) + assert.True(t, result.IsError) + assert.Contains(t, getTextResult(t, result).Text, "artifact") + assert.Equal(t, 1, calls, "requests must not be retried") + }) + } + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 53503f68fe..47189398b8 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -263,6 +263,8 @@ func AllTools(t translations.TranslationHelperFunc, opts ...ToolOption) []invent SubIssueWrite(t), IssueDependencyRead(t), IssueDependencyWrite(t), + IssueArtifactRead(t), + IssueArtifactWrite(t), FindDuplicate(t), // User tools