diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..f8abf61ad --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,74 @@ +Your task is to "onboard" this repository to Copilot coding agent by adding a .github/copilot-instructions.md file in the repository that contains information describing how a coding agent seeing it for the first time can work most efficiently. + +You will do this task only one time per repository and doing a good job can SIGNIFICANTLY improve the quality of the agent's work, so take your time, think carefully, and search thoroughly before writing the instructions. + + +- Reduce the likelihood of a coding agent pull request getting rejected by the user due to +generating code that fails the continuous integration build, fails a validation pipeline, or +having misbehavior. +- Minimize bash command and build failures. +- Allow the agent to complete its task more quickly by minimizing the need for exploration using grep, find, str_replace_editor, and code search tools. + + + +- Instructions must be no longer than 2 pages. +- Instructions must not be task specific. + + + + +Add the following high level details about the codebase to reduce the amount of searching the agent has to do to understand the codebase each time: + + +- A summary of what the repository does. +- High level repository information, such as the size of the repo, the type of the project, the languages, frameworks, or target runtimes in use. + + +Add information about how to build and validate changes so the agent does not need to search and find it each time. + + +- For each of bootstrap, build, test, run, lint, and any other scripted step, document the sequence of steps to take to run it successfully as well as the versions of any runtime or build tools used. +- Each command should be validated by running it to ensure that it works correctly as well as any preconditions and postconditions. +- Try cleaning the repo and environment and running commands in different orders and document errors and and misbehavior observed as well as any steps used to mitigate the problem. +- Run the tests and document the order of steps required to run the tests. +- Make a change to the codebase. Document any unexpected build issues as well as the workarounds. +- Document environment setup steps that seem optional but that you have validated are actually required. +- Document the time required for commands that failed due to timing out. +- When you find a sequence of commands that work for a particular purpose, document them in detail. +- Use language to indicate when something should always be done. For example: "always run npm install before building". +- Record any validation steps from documentation. + + +List key facts about the layout and architecture of the codebase to help the agent find where to make changes with minimal searching. + + +- A description of the major architectural elements of the project, including the relative paths to the main project files, the location +of configuration files for linting, compilation, testing, and preferences. +- A description of the checks run prior to check in, including any GitHub workflows, continuous integration builds, or other validation pipelines. +- Document the steps so that the agent can replicate these itself. +- Any explicit validation steps that the agent can consider to have further confidence in its changes. +- Dependencies that aren't obvious from the layout or file structure. +- Finally, fill in any remaining space with detailed lists of the following, in order of priority: the list of files in the repo root, the +contents of the README, the contents of any key source files, the list of files in the next level down of directories, giving priority to the more structurally important and snippets of code from key source files, such as the one containing the main method. + + + + +- Perform a comprehensive inventory of the codebase. Search for and view: +- README.md, CONTRIBUTING.md, and all other documentation files. +- Search the codebase for build steps and indications of workarounds like 'HACK', 'TODO', etc. +- All scripts, particularly those pertaining to build and repo or environment setup. +- All build and actions pipelines. +- All project files. +- All configuration and linting files. +- For each file: +- think: are the contents or the existence of the file information that the coding agent will need to implement, build, test, validate, or demo a code change? +- If yes: + - Document the command or information in detail. + - Explicitly indicate which commands work and which do not and the order in which commands should be run. + - Document any errors encountered as well as the steps taken to workaround them. +- Document any other steps or information that the agent can use to reduce time spent exploring or trying and failing to run bash commands. +- Finally, explicitly instruct the agent to trust the instructions and only perform a search if the information in the instructions is incomplete or found to be in error. + + - Document any errors encountered as well as the steps taken to work-around them. + diff --git a/.github/workflows/generator-generic-ossf-slsa3-publish.yml b/.github/workflows/generator-generic-ossf-slsa3-publish.yml new file mode 100644 index 000000000..294f77649 --- /dev/null +++ b/.github/workflows/generator-generic-ossf-slsa3-publish.yml @@ -0,0 +1,68 @@ +# This workflow uses actions that are not certified by GitHub. +# They are provided by a third-party and are governed by +# separate terms of service, privacy policy, and support +# documentation. + +# This workflow lets you generate SLSA provenance file for your project. +# The generation satisfies level 3 for the provenance requirements - see https://slsa.dev/spec/v0.1/requirements +# The project is an initiative of the OpenSSF (openssf.org) and is developed at +# https://github.com/slsa-framework/slsa-github-generator. +# The provenance file can be verified using https://github.com/slsa-framework/slsa-verifier. +# For more information about SLSA and how it improves the supply-chain, visit slsa.dev. + +name: SLSA generic generator +on: + workflow_dispatch: + release: + types: [created] + +jobs: + build: + runs-on: ubuntu-latest + outputs: + digests: ${{ steps.hash.outputs.digests }} + + steps: + - uses: actions/checkout@v4 + + # ======================================================== + # + # Step 1: Build your artifacts. + # + # ======================================================== + - name: Build artifacts + run: | + # Build the project and generate real artifacts. + npm ci + npm run build + # Package the build output as an artifact (example: tarball) + tar -czf dist.tar.gz dist/ + + # ======================================================== + # + # Step 2: Add a step to generate the provenance subjects + # as shown below. Update the sha256 sum arguments + # to include all binaries that you generate + # provenance for. + # + # ======================================================== + - name: Generate subject for provenance + id: hash + run: | + set -euo pipefail + + # List the artifacts the provenance will refer to. + files=$(ls artifact*) + # Generate the subjects (base64 encoded). + echo "digests=$(sha256sum $files | base64 -w0)" >> "${GITHUB_OUTPUT}" + + provenance: + needs: [build] + permissions: + actions: read # To read the workflow path. + id-token: write # To sign the provenance. + contents: write # To add assets to a release. + uses: slsa-framework/slsa-github-generator/.github/workflows/generator_generic_slsa3.yml@v1.4.0 + with: + base64-subjects: "${{ needs.build.outputs.digests }}" + upload-assets: true # Optional: Upload to a new release diff --git a/README.md b/README.md index a6e740e66..137cc2f08 100644 --- a/README.md +++ b/README.md @@ -533,6 +533,17 @@ The following sets of tools are available (all are on by default): - `milestone`: Milestone number (number, optional) - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) + - `title`: Issue title - must be meaningful (at least 3 characters, contain letters/numbers, not placeholder text) (string, required) + - `type`: Type of this issue (string, optional) + +- **create_sub_issue** - Create sub-issue + - `assignees`: Usernames to assign to this issue (string[], optional) + - `body`: Issue body content (string, optional) + - `labels`: Labels to apply to this issue (string[], optional) + - `milestone`: Milestone number (number, optional) + - `owner`: Repository owner (string, required) + - `parent_issue_number`: The number of the parent issue (number, required) + - `repo`: Repository name (string, required) - `title`: Issue title (string, required) - `type`: Type of this issue (string, optional) diff --git a/pkg/github/__toolsnaps__/create_sub_issue.snap b/pkg/github/__toolsnaps__/create_sub_issue.snap new file mode 100644 index 000000000..a300b00cc --- /dev/null +++ b/pkg/github/__toolsnaps__/create_sub_issue.snap @@ -0,0 +1,61 @@ +{ + "annotations": { + "title": "Create sub-issue", + "readOnlyHint": false + }, + "description": "Create a new issue and automatically add it as a sub-issue to a parent issue in a GitHub repository.", + "inputSchema": { + "properties": { + "assignees": { + "description": "Usernames to assign to this issue", + "items": { + "type": "string" + }, + "type": "array" + }, + "body": { + "description": "Issue body content", + "type": "string" + }, + "labels": { + "description": "Labels to apply to this issue", + "items": { + "type": "string" + }, + "type": "array" + }, + "milestone": { + "description": "Milestone number", + "type": "number" + }, + "owner": { + "description": "Repository owner", + "type": "string" + }, + "parent_issue_number": { + "description": "The number of the parent issue", + "type": "number" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "title": { + "description": "Issue title", + "type": "string" + }, + "type": { + "description": "Type of this issue", + "type": "string" + } + }, + "required": [ + "owner", + "repo", + "parent_issue_number", + "title" + ], + "type": "object" + }, + "name": "create_sub_issue" +} \ No newline at end of file diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 89375ae90..59a0d63c5 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -801,7 +801,7 @@ func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t if err != nil { return mcp.NewToolResultError(err.Error()), nil } - title, err := RequiredParam[string](request, "title") + title, err := RequiredMeaningfulTitle(request, "title") if err != nil { return mcp.NewToolResultError(err.Error()), nil } @@ -881,6 +881,179 @@ func CreateIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (t } } +// CreateSubIssue creates a tool to create a new issue and automatically add it as a sub-issue to a parent issue. +func CreateSubIssue(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { + return mcp.NewTool("create_sub_issue", + mcp.WithDescription(t("TOOL_CREATE_SUB_ISSUE_DESCRIPTION", "Create a new issue and automatically add it as a sub-issue to a parent issue in a GitHub repository.")), + mcp.WithToolAnnotation(mcp.ToolAnnotation{ + Title: t("TOOL_CREATE_SUB_ISSUE_USER_TITLE", "Create sub-issue"), + ReadOnlyHint: ToBoolPtr(false), + }), + mcp.WithString("owner", + mcp.Required(), + mcp.Description("Repository owner"), + ), + mcp.WithString("repo", + mcp.Required(), + mcp.Description("Repository name"), + ), + mcp.WithNumber("parent_issue_number", + mcp.Required(), + mcp.Description("The number of the parent issue"), + ), + mcp.WithString("title", + mcp.Required(), + mcp.Description("Issue title"), + ), + mcp.WithString("body", + mcp.Description("Issue body content"), + ), + mcp.WithArray("assignees", + mcp.Description("Usernames to assign to this issue"), + mcp.Items( + map[string]any{ + "type": "string", + }, + ), + ), + mcp.WithArray("labels", + mcp.Description("Labels to apply to this issue"), + mcp.Items( + map[string]any{ + "type": "string", + }, + ), + ), + mcp.WithNumber("milestone", + mcp.Description("Milestone number"), + ), + mcp.WithString("type", + mcp.Description("Type of this issue"), + ), + ), + func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { + owner, err := RequiredParam[string](request, "owner") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + repo, err := RequiredParam[string](request, "repo") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + parentIssueNumber, err := RequiredInt(request, "parent_issue_number") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + title, err := RequiredParam[string](request, "title") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Optional parameters + body, err := OptionalParam[string](request, "body") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Get assignees + assignees, err := OptionalStringArrayParam(request, "assignees") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Get labels + labels, err := OptionalStringArrayParam(request, "labels") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Get optional milestone + milestone, err := OptionalIntParam(request, "milestone") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + var milestoneNum *int + if milestone != 0 { + milestoneNum = &milestone + } + + // Get optional type + issueType, err := OptionalParam[string](request, "type") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Step 1: Create the issue + issueRequest := &github.IssueRequest{ + Title: github.Ptr(title), + Body: github.Ptr(body), + Assignees: &assignees, + Labels: &labels, + Milestone: milestoneNum, + } + + if issueType != "" { + issueRequest.Type = github.Ptr(issueType) + } + + issue, resp, err := client.Issues.Create(ctx, owner, repo, issueRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to create issue", + resp, + err, + ), nil + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != http.StatusCreated { + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to create issue: %s", string(body))), nil + } + + // Step 2: Add the created issue as a sub-issue to the parent + subIssueRequest := github.SubIssueRequest{ + SubIssueID: *issue.ID, + ReplaceParent: ToBoolPtr(false), // Default to not replacing existing parent + } + + _, resp2, err := client.SubIssue.Add(ctx, owner, repo, int64(parentIssueNumber), subIssueRequest) + if err != nil { + return ghErrors.NewGitHubAPIErrorResponse(ctx, + "failed to add issue as sub-issue (issue was created successfully)", + resp2, + err, + ), nil + } + defer func() { _ = resp2.Body.Close() }() + + if resp2.StatusCode != http.StatusCreated { + body, err := io.ReadAll(resp2.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response body: %w", err) + } + return mcp.NewToolResultError(fmt.Sprintf("failed to add issue as sub-issue (issue was created successfully): %s", string(body))), nil + } + + // Return the created issue (not the parent issue) + r, err := json.Marshal(issue) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return mcp.NewToolResultText(string(r)), nil + } +} + // ListIssues creates a tool to list and filter repository issues func ListIssues(getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("list_issues", diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 7c4983c64..98baa3657 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -681,6 +681,63 @@ func Test_CreateIssue(t *testing.T) { expectError: false, expectedErrMsg: "missing required parameter: title", }, + { + name: "issue creation fails with too short title", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Validation failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "title": "Hi", + }, + expectError: false, + expectedErrMsg: "title must be at least 3 characters long", + }, + { + name: "issue creation fails with placeholder title", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Validation failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "title": "Title", + }, + expectError: false, + expectedErrMsg: "title appears to be a placeholder", + }, + { + name: "issue creation fails with non-alphanumeric title", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesByOwnerByRepo, + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"message": "Validation failed"}`)) + }), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "title": "!!!", + }, + expectError: false, + expectedErrMsg: "title must contain at least one letter or number", + }, } for _, tc := range tests { @@ -2989,3 +3046,206 @@ func Test_ListIssueTypes(t *testing.T) { }) } } +func Test_CreateSubIssue(t *testing.T) { + // Verify tool definition once + mockClient := github.NewClient(nil) + tool, _ := CreateSubIssue(stubGetClientFn(mockClient), translations.NullTranslationHelper) + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + assert.Equal(t, "create_sub_issue", tool.Name) + assert.NotEmpty(t, tool.Description) + assert.Contains(t, tool.InputSchema.Properties, "owner") + assert.Contains(t, tool.InputSchema.Properties, "repo") + assert.Contains(t, tool.InputSchema.Properties, "parent_issue_number") + assert.Contains(t, tool.InputSchema.Properties, "title") + assert.Contains(t, tool.InputSchema.Properties, "body") + assert.Contains(t, tool.InputSchema.Properties, "assignees") + assert.Contains(t, tool.InputSchema.Properties, "labels") + assert.Contains(t, tool.InputSchema.Properties, "milestone") + assert.Contains(t, tool.InputSchema.Properties, "type") + assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo", "parent_issue_number", "title"}) + + // Setup mock issue for success case (matches GitHub API response format) + mockIssue := &github.Issue{ + Number: github.Ptr(45), + ID: github.Ptr(int64(12345)), + Title: github.Ptr("New Sub Issue"), + Body: github.Ptr("This is a new sub-issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/45"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + Labels: []*github.Label{ + { + Name: github.Ptr("enhancement"), + Color: github.Ptr("84b6eb"), + Description: github.Ptr("New feature or request"), + }, + }, + } + + // Setup mock parent issue for sub-issue response + mockParentIssue := &github.Issue{ + Number: github.Ptr(42), + Title: github.Ptr("Parent Issue"), + Body: github.Ptr("This is the parent issue with a sub-issue"), + State: github.Ptr("open"), + HTMLURL: github.Ptr("https://github.com/owner/repo/issues/42"), + User: &github.User{ + Login: github.Ptr("testuser"), + }, + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]interface{} + expectError bool + expectedIssue *github.Issue + expectedErrMsg string + }{ + { + name: "successful sub-issue creation with all parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesByOwnerByRepo, + mockResponse(t, http.StatusCreated, mockIssue), + ), + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusCreated, mockParentIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "parent_issue_number": float64(42), + "title": "New Sub Issue", + "body": "This is a new sub-issue", + "assignees": []interface{}{"testuser"}, + "labels": []interface{}{"enhancement"}, + "milestone": float64(1), + "type": "feature", + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "successful sub-issue creation with minimal parameters", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesByOwnerByRepo, + mockResponse(t, http.StatusCreated, mockIssue), + ), + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusCreated, mockParentIssue), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "parent_issue_number": float64(42), + "title": "New Sub Issue", + }, + expectError: false, + expectedIssue: mockIssue, + }, + { + name: "issue creation fails", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesByOwnerByRepo, + mockResponse(t, http.StatusUnprocessableEntity, `{"message": "Validation failed", "errors": [{"message": "Title is required"}]}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "parent_issue_number": float64(42), + "title": "Valid Title", + }, + expectError: false, + expectedErrMsg: "failed to create issue", + }, + { + name: "sub-issue addition fails after successful issue creation", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatchHandler( + mock.PostReposIssuesByOwnerByRepo, + mockResponse(t, http.StatusCreated, mockIssue), + ), + mock.WithRequestMatchHandler( + mock.PostReposIssuesSubIssuesByOwnerByRepoByIssueNumber, + mockResponse(t, http.StatusNotFound, `{"message": "Parent issue not found"}`), + ), + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "parent_issue_number": float64(999), + "title": "New Sub Issue", + }, + expectError: false, + expectedErrMsg: "failed to add issue as sub-issue (issue was created successfully)", + }, + { + name: "missing required parameter", + mockedClient: mock.NewMockedHTTPClient( + // No mocked requests needed since validation fails before HTTP call + ), + requestArgs: map[string]interface{}{ + "owner": "owner", + "repo": "repo", + "title": "New Sub Issue", + }, + expectError: false, + expectedErrMsg: "missing required parameter: parent_issue_number", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Setup client with mock + client := github.NewClient(tc.mockedClient) + _, handler := CreateSubIssue(stubGetClientFn(client), translations.NullTranslationHelper) + + // Create call request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify results + if tc.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + if tc.expectedErrMsg != "" { + require.NotNil(t, result) + textContent := getTextResult(t, result) + assert.Contains(t, textContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + + // Parse the result and get the text content if no error + textContent := getTextResult(t, result) + + // Unmarshal and verify the result + var returnedIssue github.Issue + err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) + require.NoError(t, err) + assert.Equal(t, *tc.expectedIssue.Number, *returnedIssue.Number) + assert.Equal(t, *tc.expectedIssue.Title, *returnedIssue.Title) + assert.Equal(t, *tc.expectedIssue.Body, *returnedIssue.Body) + assert.Equal(t, *tc.expectedIssue.State, *returnedIssue.State) + assert.Equal(t, *tc.expectedIssue.HTMLURL, *returnedIssue.HTMLURL) + assert.Equal(t, *tc.expectedIssue.User.Login, *returnedIssue.User.Login) + }) + } +} diff --git a/pkg/github/server.go b/pkg/github/server.go index 80a1bbac6..4cc77d0aa 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -4,6 +4,8 @@ import ( "encoding/json" "errors" "fmt" + "strings" + "unicode" "github.com/google/go-github/v74/github" "github.com/mark3labs/mcp-go/mcp" @@ -86,6 +88,64 @@ func RequiredParam[T comparable](r mcp.CallToolRequest, p string) (T, error) { return val, nil } +// RequiredMeaningfulTitle validates that a title parameter is present, non-empty, and meaningful. +// It performs additional checks beyond RequiredParam to ensure the title has substance: +// - Must be at least 3 characters long +// - Must contain at least one letter or number +// - Cannot be just whitespace or common placeholder text +func RequiredMeaningfulTitle(r mcp.CallToolRequest, p string) (string, error) { + title, err := RequiredParam[string](r, p) + if err != nil { + return "", err + } + + // Trim whitespace for validation + trimmed := strings.TrimSpace(title) + + // Check minimum length + if len(trimmed) < 3 { + return "", fmt.Errorf("title must be at least 3 characters long") + } + + // Check for at least one alphanumeric character + hasAlphaNum := false + for _, r := range trimmed { + if unicode.IsLetter(r) || unicode.IsDigit(r) { + hasAlphaNum = true + break + } + } + if !hasAlphaNum { + return "", fmt.Errorf("title must contain at least one letter or number") + } + + // Check for common placeholder patterns (case-insensitive) + lowerTitle := strings.ToLower(trimmed) + placeholders := []string{ + "title", + "add a title", + "enter title here", + "issue title", + "your title here", + "todo", + "tbd", + "to be determined", + "placeholder", + "example", + "test", + "testing", + "untitled", + } + + for _, placeholder := range placeholders { + if lowerTitle == placeholder { + return "", fmt.Errorf("title appears to be a placeholder") + } + } + + return title, nil +} + // RequiredInt is a helper function that can be used to fetch a requested parameter from the request. // It does the following checks: // 1. Checks if the parameter is present in the request. diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index f38c4dc01..1efd42f55 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -560,3 +560,124 @@ func TestOptionalPaginationParams(t *testing.T) { }) } } + +func Test_RequiredMeaningfulTitle(t *testing.T) { + tests := []struct { + name string + params map[string]interface{} + paramName string + expected string + expectError bool + errorMsg string + }{ + { + name: "valid meaningful title", + params: map[string]interface{}{"title": "Fix bug in user authentication"}, + paramName: "title", + expected: "Fix bug in user authentication", + expectError: false, + }, + { + name: "valid short meaningful title", + params: map[string]interface{}{"title": "Bug"}, + paramName: "title", + expected: "Bug", + expectError: false, + }, + { + name: "missing parameter", + params: map[string]interface{}{}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "missing required parameter: title", + }, + { + name: "empty string", + params: map[string]interface{}{"title": ""}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "missing required parameter: title", + }, + { + name: "too short", + params: map[string]interface{}{"title": "Hi"}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "title must be at least 3 characters long", + }, + { + name: "only whitespace", + params: map[string]interface{}{"title": " "}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "title must be at least 3 characters long", + }, + { + name: "only special characters", + params: map[string]interface{}{"title": "!!!"}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "title must contain at least one letter or number", + }, + { + name: "placeholder title", + params: map[string]interface{}{"title": "Title"}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "title appears to be a placeholder", + }, + { + name: "placeholder with different case", + params: map[string]interface{}{"title": "TITLE"}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "title appears to be a placeholder", + }, + { + name: "todo placeholder", + params: map[string]interface{}{"title": "TODO"}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "title appears to be a placeholder", + }, + { + name: "test placeholder", + params: map[string]interface{}{"title": "test"}, + paramName: "title", + expected: "", + expectError: true, + errorMsg: "title appears to be a placeholder", + }, + { + name: "valid title with spaces", + params: map[string]interface{}{"title": " Fix authentication bug "}, + paramName: "title", + expected: " Fix authentication bug ", + expectError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + request := createMCPRequest(tc.params) + result, err := RequiredMeaningfulTitle(request, tc.paramName) + + if tc.expectError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tc.errorMsg) + assert.Equal(t, tc.expected, result) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.expected, result) + } + }) + } +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 728d78097..97f2261e2 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -61,6 +61,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG ). AddWriteTools( toolsets.NewServerTool(CreateIssue(getClient, t)), + toolsets.NewServerTool(CreateSubIssue(getClient, t)), toolsets.NewServerTool(AddIssueComment(getClient, t)), toolsets.NewServerTool(UpdateIssue(getClient, t)), toolsets.NewServerTool(AssignCopilotToIssue(getGQLClient, t)),