From 7c67d9c1740b65a9dcfb27bc4ced34608559f841 Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 14:42:10 +0100 Subject: [PATCH 1/2] feat: Add return_resource_links parameter to GetFileContents and GetJobLogs for accessing logs via ResourceLinks --- README.md | 2 + .../__toolsnaps__/get_file_contents.snap | 4 + pkg/github/actions.go | 85 +++++++- pkg/github/actions_resource.go | 195 ++++++++++++++++++ pkg/github/actions_resource_test.go | 36 ++++ pkg/github/repositories.go | 81 ++++++++ pkg/github/repositories_test.go | 102 +++++++++ pkg/github/tools.go | 4 + 8 files changed, 505 insertions(+), 4 deletions(-) create mode 100644 pkg/github/actions_resource.go create mode 100644 pkg/github/actions_resource_test.go diff --git a/README.md b/README.md index a6e740e668..a067f746fb 100644 --- a/README.md +++ b/README.md @@ -328,6 +328,7 @@ The following sets of tools are available (all are on by default): - `owner`: Repository owner (string, required) - `repo`: Repository name (string, required) - `return_content`: Returns actual log content instead of URLs (boolean, optional) + - `return_resource_links`: Returns MCP ResourceLinks for accessing logs instead of direct content or URLs (boolean, optional) - `run_id`: Workflow run ID (required when using failed_only) (number, optional) - `tail_lines`: Number of lines to return from the end of the log (number, optional) @@ -841,6 +842,7 @@ The following sets of tools are available (all are on by default): - `path`: Path to file/directory (directories must end with a slash '/') (string, optional) - `ref`: Accepts optional git refs such as `refs/tags/{tag}`, `refs/heads/{branch}` or `refs/pull/{pr_number}/head` (string, optional) - `repo`: Repository name (string, required) + - `return_resource_links`: Return ResourceLinks instead of file content - useful for large files or when you want to reference the file for later access (boolean, optional) - `sha`: Accepts optional commit SHA. If specified, it will be used instead of ref (string, optional) - **get_latest_release** - Get latest release diff --git a/pkg/github/__toolsnaps__/get_file_contents.snap b/pkg/github/__toolsnaps__/get_file_contents.snap index 53f5a29e51..e0c467b000 100644 --- a/pkg/github/__toolsnaps__/get_file_contents.snap +++ b/pkg/github/__toolsnaps__/get_file_contents.snap @@ -23,6 +23,10 @@ "description": "Repository name", "type": "string" }, + "return_resource_links": { + "description": "Return ResourceLinks instead of file content - useful for large files or when you want to reference the file for later access", + "type": "boolean" + }, "sha": { "description": "Accepts optional commit SHA. If specified, it will be used instead of ref", "type": "string" diff --git a/pkg/github/actions.go b/pkg/github/actions.go index ace9d72886..11296d8575 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -558,6 +558,9 @@ func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc, con mcp.WithBoolean("return_content", mcp.Description("Returns actual log content instead of URLs"), ), + mcp.WithBoolean("return_resource_links", + mcp.Description("Returns MCP ResourceLinks for accessing logs instead of direct content or URLs"), + ), mcp.WithNumber("tail_lines", mcp.Description("Number of lines to return from the end of the log"), mcp.DefaultNumber(500), @@ -590,6 +593,10 @@ func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc, con if err != nil { return mcp.NewToolResultError(err.Error()), nil } + returnResourceLinks, err := OptionalParam[bool](request, "return_resource_links") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } tailLines, err := OptionalIntParam(request, "tail_lines") if err != nil { return mcp.NewToolResultError(err.Error()), nil @@ -612,12 +619,24 @@ func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc, con return mcp.NewToolResultError("job_id is required when failed_only is false"), nil } + // Validate that only one return mode is selected + returnModes := []bool{returnContent, returnResourceLinks} + activeModes := 0 + for _, mode := range returnModes { + if mode { + activeModes++ + } + } + if activeModes > 1 { + return mcp.NewToolResultError("Only one of return_content or return_resource_links can be true"), nil + } + if failedOnly && runID > 0 { // Handle failed-only mode: get logs for all failed jobs in the workflow run - return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, contentWindowSize) + return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, returnResourceLinks, tailLines, contentWindowSize) } else if jobID > 0 { // Handle single job mode - return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, contentWindowSize) + return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, returnResourceLinks, tailLines, contentWindowSize) } return mcp.NewToolResultError("Either job_id must be provided for single job logs, or run_id with failed_only=true for failed job logs"), nil @@ -625,7 +644,7 @@ func GetJobLogs(getClient GetClientFn, t translations.TranslationHelperFunc, con } // handleFailedJobLogs gets logs for all failed jobs in a workflow run -func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { +func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo string, runID int64, returnContent bool, returnResourceLinks bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { // First, get all jobs for the workflow run jobs, resp, err := client.Actions.ListWorkflowJobs(ctx, owner, repo, runID, &github.ListWorkflowJobsOptions{ Filter: "latest", @@ -654,6 +673,33 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo return mcp.NewToolResultText(string(r)), nil } + if returnResourceLinks { + // Return ResourceLinks for all failed job logs + var content []mcp.Content + + // Add summary text + summaryText := fmt.Sprintf("Found %d failed jobs in workflow run %d. ResourceLinks provided below for accessing individual job logs.", len(failedJobs), runID) + content = append(content, mcp.TextContent{ + Type: "text", + Text: summaryText, + }) + + // Add ResourceLinks for each failed job + for _, job := range failedJobs { + resourceLink := mcp.ResourceLink{ + URI: fmt.Sprintf("actions://%s/%s/jobs/%d/logs", owner, repo, job.GetID()), + Name: fmt.Sprintf("failed-job-%d-logs", job.GetID()), + Description: fmt.Sprintf("Logs for failed job: %s (ID: %d)", job.GetName(), job.GetID()), + MIMEType: "text/plain", + } + content = append(content, resourceLink) + } + + return &mcp.CallToolResult{ + Content: content, + }, nil + } + // Collect logs for all failed jobs var logResults []map[string]any for _, job := range failedJobs { @@ -690,7 +736,38 @@ func handleFailedJobLogs(ctx context.Context, client *github.Client, owner, repo } // handleSingleJobLogs gets logs for a single job -func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, returnContent bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { +func handleSingleJobLogs(ctx context.Context, client *github.Client, owner, repo string, jobID int64, returnContent bool, returnResourceLinks bool, tailLines int, contentWindowSize int) (*mcp.CallToolResult, error) { + if returnResourceLinks { + // Return a ResourceLink for the job logs + resourceLink := mcp.ResourceLink{ + URI: fmt.Sprintf("actions://%s/%s/jobs/%d/logs", owner, repo, jobID), + Name: fmt.Sprintf("job-%d-logs", jobID), + Description: fmt.Sprintf("Complete logs for job %d", jobID), + MIMEType: "text/plain", + } + + result := map[string]any{ + "message": "Job logs available via ResourceLink", + "job_id": jobID, + "resource_uri": resourceLink.URI, + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return &mcp.CallToolResult{ + Content: []mcp.Content{ + mcp.TextContent{ + Type: "text", + Text: string(r), + }, + resourceLink, + }, + }, nil + } + jobResult, resp, err := getJobLogData(ctx, client, owner, repo, jobID, "", returnContent, tailLines, contentWindowSize) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get job logs", resp, err), nil diff --git a/pkg/github/actions_resource.go b/pkg/github/actions_resource.go new file mode 100644 index 0000000000..3a3c5ce984 --- /dev/null +++ b/pkg/github/actions_resource.go @@ -0,0 +1,195 @@ +package github + +import ( + "context" + "errors" + "fmt" + "net/http" + "strconv" + + "github.com/github/github-mcp-server/internal/profiler" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/mark3labs/mcp-go/mcp" + "github.com/mark3labs/mcp-go/server" +) + +// GetWorkflowRunLogsResource defines the resource template and handler for getting workflow run logs. +func GetWorkflowRunLogsResource(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) { + return mcp.NewResourceTemplate( + "actions://{owner}/{repo}/runs/{runId}/logs", // Resource template + t("RESOURCE_WORKFLOW_RUN_LOGS_DESCRIPTION", "Workflow Run Logs"), + ), + WorkflowRunLogsResourceHandler(getClient) +} + +// GetJobLogsResource defines the resource template and handler for getting individual job logs. +func GetJobLogsResource(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.ResourceTemplate, server.ResourceTemplateHandlerFunc) { + return mcp.NewResourceTemplate( + "actions://{owner}/{repo}/jobs/{jobId}/logs", // Resource template + t("RESOURCE_JOB_LOGS_DESCRIPTION", "Job Logs"), + ), + JobLogsResourceHandler(getClient) +} + +// WorkflowRunLogsResourceHandler returns a handler function for workflow run logs requests. +func WorkflowRunLogsResourceHandler(getClient GetClientFn) func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + return func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + // Parse parameters from the URI template matcher + owner, ok := request.Params.Arguments["owner"].([]string) + if !ok || len(owner) == 0 { + return nil, errors.New("owner is required") + } + + repo, ok := request.Params.Arguments["repo"].([]string) + if !ok || len(repo) == 0 { + return nil, errors.New("repo is required") + } + + runIdStr, ok := request.Params.Arguments["runId"].([]string) + if !ok || len(runIdStr) == 0 { + return nil, errors.New("runId is required") + } + + runId, err := strconv.ParseInt(runIdStr[0], 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid runId: %w", err) + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Get the JIT URL for workflow run logs + url, resp, err := client.Actions.GetWorkflowRunLogs(ctx, owner[0], repo[0], runId, 1) + if err != nil { + return nil, fmt.Errorf("failed to get workflow run logs URL: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Download the logs content immediately using the JIT URL + content, err := downloadLogsFromJITURL(ctx, url.String()) + if err != nil { + return nil, fmt.Errorf("failed to download workflow run logs: %w", err) + } + + return []mcp.ResourceContents{ + mcp.TextResourceContents{ + URI: request.Params.URI, + MIMEType: "application/zip", + Text: fmt.Sprintf("Workflow run logs for run %d (ZIP archive)\n\nNote: This is a ZIP archive containing all job logs. Download URL was: %s\n\nContent length: %d bytes", runId, url.String(), len(content)), + }, + }, nil + } +} + +// JobLogsResourceHandler returns a handler function for individual job logs requests. +func JobLogsResourceHandler(getClient GetClientFn) func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + return func(ctx context.Context, request mcp.ReadResourceRequest) ([]mcp.ResourceContents, error) { + // Parse parameters from the URI template matcher + owner, ok := request.Params.Arguments["owner"].([]string) + if !ok || len(owner) == 0 { + return nil, errors.New("owner is required") + } + + repo, ok := request.Params.Arguments["repo"].([]string) + if !ok || len(repo) == 0 { + return nil, errors.New("repo is required") + } + + jobIdStr, ok := request.Params.Arguments["jobId"].([]string) + if !ok || len(jobIdStr) == 0 { + return nil, errors.New("jobId is required") + } + + jobId, err := strconv.ParseInt(jobIdStr[0], 10, 64) + if err != nil { + return nil, fmt.Errorf("invalid jobId: %w", err) + } + + client, err := getClient(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub client: %w", err) + } + + // Get the JIT URL for job logs + url, resp, err := client.Actions.GetWorkflowJobLogs(ctx, owner[0], repo[0], jobId, 1) + if err != nil { + return nil, fmt.Errorf("failed to get job logs URL: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + // Download the logs content immediately using the JIT URL + content, err := downloadLogsFromJITURL(ctx, url.String()) + if err != nil { + return nil, fmt.Errorf("failed to download job logs: %w", err) + } + + return []mcp.ResourceContents{ + mcp.TextResourceContents{ + URI: request.Params.URI, + MIMEType: "text/plain", + Text: content, + }, + }, nil + } +} + +// downloadLogsFromJITURL downloads content from a GitHub JIT URL +func downloadLogsFromJITURL(ctx context.Context, jitURL string) (string, error) { + prof := profiler.New(nil, profiler.IsProfilingEnabled()) + finish := prof.Start(ctx, "download_jit_logs") + + httpResp, err := http.Get(jitURL) //nolint:gosec + if err != nil { + _ = finish(0, 0) + return "", fmt.Errorf("failed to download from JIT URL: %w", err) + } + defer httpResp.Body.Close() + + if httpResp.StatusCode != http.StatusOK { + _ = finish(0, 0) + return "", fmt.Errorf("failed to download logs: HTTP %d", httpResp.StatusCode) + } + + // For large files, we should limit the content size to avoid memory issues + const maxContentSize = 10 * 1024 * 1024 // 10MB limit + + // Read the content with a size limit + content := make([]byte, 0, 1024*1024) // Start with 1MB capacity + buffer := make([]byte, 32*1024) // 32KB read buffer + totalRead := 0 + + for { + n, err := httpResp.Body.Read(buffer) + if n > 0 { + if totalRead+n > maxContentSize { + // Truncate if content is too large + remaining := maxContentSize - totalRead + content = append(content, buffer[:remaining]...) + content = append(content, []byte(fmt.Sprintf("\n\n[Content truncated - original size exceeded %d bytes]", maxContentSize))...) + break + } + content = append(content, buffer[:n]...) + totalRead += n + } + if err != nil { + if err.Error() == "EOF" { + break + } + _ = finish(0, int64(totalRead)) + return "", fmt.Errorf("failed to read response body: %w", err) + } + } + + // Count lines for profiler + lines := 1 + for _, b := range content { + if b == '\n' { + lines++ + } + } + + _ = finish(lines, int64(len(content))) + return string(content), nil +} diff --git a/pkg/github/actions_resource_test.go b/pkg/github/actions_resource_test.go new file mode 100644 index 0000000000..a10b005438 --- /dev/null +++ b/pkg/github/actions_resource_test.go @@ -0,0 +1,36 @@ +package github + +import ( + "testing" + + "github.com/github/github-mcp-server/pkg/translations" + "github.com/stretchr/testify/assert" +) + +func TestGetJobLogsWithResourceLinks(t *testing.T) { + // Test that the tool has the new parameter + tool, _ := GetJobLogs(stubGetClientFn(nil), translations.NullTranslationHelper, 1000) + + // Verify tool has the new parameter + schema := tool.InputSchema + assert.Contains(t, schema.Properties, "return_resource_links") + + // Check that the parameter exists (we can't easily check types in this interface) + resourceLinkParam := schema.Properties["return_resource_links"] + assert.NotNil(t, resourceLinkParam) +} + +func TestJobLogsResourceCreation(t *testing.T) { + // Test that we can create the resource templates without errors + jobLogsResource, jobLogsHandler := GetJobLogsResource(stubGetClientFn(nil), translations.NullTranslationHelper) + workflowRunLogsResource, workflowRunLogsHandler := GetWorkflowRunLogsResource(stubGetClientFn(nil), translations.NullTranslationHelper) + + // Verify resource templates are created + assert.NotNil(t, jobLogsResource) + assert.NotNil(t, jobLogsHandler) + assert.Equal(t, "actions://{owner}/{repo}/jobs/{jobId}/logs", jobLogsResource.URITemplate.Raw()) + + assert.NotNil(t, workflowRunLogsResource) + assert.NotNil(t, workflowRunLogsHandler) + assert.Equal(t, "actions://{owner}/{repo}/runs/{runId}/logs", workflowRunLogsResource.URITemplate.Raw()) +} diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index de2c6d01f7..a381f44eee 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -471,6 +471,9 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t mcp.WithString("sha", mcp.Description("Accepts optional commit SHA. If specified, it will be used instead of ref"), ), + mcp.WithBoolean("return_resource_links", + mcp.Description("Return ResourceLinks instead of file content - useful for large files or when you want to reference the file for later access"), + ), ), func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { owner, err := RequiredParam[string](request, "owner") @@ -493,6 +496,15 @@ func GetFileContents(getClient GetClientFn, getRawClient raw.GetRawClientFn, t t if err != nil { return mcp.NewToolResultError(err.Error()), nil } + returnResourceLinks, err := OptionalParam[bool](request, "return_resource_links") + if err != nil { + return mcp.NewToolResultError(err.Error()), nil + } + + // Handle ResourceLink request + if returnResourceLinks { + return handleFileContentsResourceLink(owner, repo, path, ref, sha) + } client, err := getClient(ctx) if err != nil { @@ -1641,3 +1653,72 @@ func resolveGitReference(ctx context.Context, githubClient *github.Client, owner sha = reference.GetObject().GetSHA() return &raw.ContentOpts{Ref: ref, SHA: sha}, nil } + +// handleFileContentsResourceLink creates a ResourceLink for file content +func handleFileContentsResourceLink(owner, repo, path, ref, sha string) (*mcp.CallToolResult, error) { + // Ensure path starts with / for consistency + if !strings.HasPrefix(path, "/") { + path = "/" + path + } + + // Determine the appropriate resource URI based on the parameters provided + var resourceURI string + var description string + + switch { + case sha != "": + resourceURI = fmt.Sprintf("repo://%s/%s/sha/%s/contents%s", owner, repo, sha, path) + description = fmt.Sprintf("File content for %s at commit %s in %s/%s", path, sha[:8], owner, repo) + case ref != "": + // Handle different ref types + if strings.HasPrefix(ref, "refs/heads/") { + branch := strings.TrimPrefix(ref, "refs/heads/") + resourceURI = fmt.Sprintf("repo://%s/%s/refs/heads/%s/contents%s", owner, repo, branch, path) + description = fmt.Sprintf("File content for %s on branch %s in %s/%s", path, branch, owner, repo) + } else if strings.HasPrefix(ref, "refs/tags/") { + tag := strings.TrimPrefix(ref, "refs/tags/") + resourceURI = fmt.Sprintf("repo://%s/%s/refs/tags/%s/contents%s", owner, repo, tag, path) + description = fmt.Sprintf("File content for %s at tag %s in %s/%s", path, tag, owner, repo) + } else if strings.HasPrefix(ref, "refs/pull/") && strings.HasSuffix(ref, "/head") { + // Extract PR number from refs/pull/{number}/head + prNumber := strings.TrimSuffix(strings.TrimPrefix(ref, "refs/pull/"), "/head") + resourceURI = fmt.Sprintf("repo://%s/%s/pulls/%s/contents%s", owner, repo, prNumber, path) + description = fmt.Sprintf("File content for %s in PR #%s in %s/%s", path, prNumber, owner, repo) + } else { + // Generic ref handling - try to clean it up + cleanRef := strings.TrimPrefix(ref, "refs/") + resourceURI = fmt.Sprintf("repo://%s/%s/refs/%s/contents%s", owner, repo, cleanRef, path) + description = fmt.Sprintf("File content for %s at ref %s in %s/%s", path, ref, owner, repo) + } + default: + // Default branch/HEAD + resourceURI = fmt.Sprintf("repo://%s/%s/contents%s", owner, repo, path) + description = fmt.Sprintf("File content for %s in %s/%s", path, owner, repo) + } + + resourceLink := mcp.ResourceLink{ + URI: resourceURI, + Description: description, + } + + result := map[string]any{ + "message": "File content available via ResourceLink", + "path": path, + "resource_uri": resourceLink.URI, + } + + r, err := json.Marshal(result) + if err != nil { + return nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return &mcp.CallToolResult{ + Content: []mcp.Content{ + mcp.TextContent{ + Type: "text", + Text: string(r), + }, + resourceLink, + }, + }, nil +} diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index f5ebfd32b7..a251e7d1c4 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -282,6 +282,108 @@ func Test_GetFileContents(t *testing.T) { } } +func Test_GetFileContents_ResourceLinks(t *testing.T) { + // Test ResourceLink functionality for GetFileContents + mockClient := github.NewClient(nil) + mockRawClient := raw.NewClient(mockClient, &url.URL{Scheme: "https", Host: "raw.githubusercontent.com", Path: "/"}) + _, handler := GetFileContents(stubGetClientFn(mockClient), stubGetRawClientFn(mockRawClient), translations.NullTranslationHelper) + + tests := []struct { + name string + requestArgs map[string]interface{} + expectedURI string + expectedDesc string + }{ + { + name: "default branch file", + requestArgs: map[string]interface{}{ + "owner": "github", + "repo": "github-mcp-server", + "path": "README.md", + "return_resource_links": true, + }, + expectedURI: "repo://github/github-mcp-server/contents/README.md", + expectedDesc: "File content for /README.md in github/github-mcp-server", + }, + { + name: "specific commit SHA", + requestArgs: map[string]interface{}{ + "owner": "github", + "repo": "github-mcp-server", + "path": "src/main.go", + "sha": "abc123def456", + "return_resource_links": true, + }, + expectedURI: "repo://github/github-mcp-server/sha/abc123def456/contents/src/main.go", + expectedDesc: "File content for /src/main.go at commit abc123de in github/github-mcp-server", + }, + { + name: "specific branch", + requestArgs: map[string]interface{}{ + "owner": "github", + "repo": "github-mcp-server", + "path": "docs/api.md", + "ref": "refs/heads/feature-branch", + "return_resource_links": true, + }, + expectedURI: "repo://github/github-mcp-server/refs/heads/feature-branch/contents/docs/api.md", + expectedDesc: "File content for /docs/api.md on branch feature-branch in github/github-mcp-server", + }, + { + name: "specific tag", + requestArgs: map[string]interface{}{ + "owner": "github", + "repo": "github-mcp-server", + "path": "VERSION", + "ref": "refs/tags/v1.0.0", + "return_resource_links": true, + }, + expectedURI: "repo://github/github-mcp-server/refs/tags/v1.0.0/contents/VERSION", + expectedDesc: "File content for /VERSION at tag v1.0.0 in github/github-mcp-server", + }, + { + name: "pull request ref", + requestArgs: map[string]interface{}{ + "owner": "github", + "repo": "github-mcp-server", + "path": "CHANGES.md", + "ref": "refs/pull/123/head", + "return_resource_links": true, + }, + expectedURI: "repo://github/github-mcp-server/pulls/123/contents/CHANGES.md", + expectedDesc: "File content for /CHANGES.md in PR #123 in github/github-mcp-server", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + // Create request + request := createMCPRequest(tc.requestArgs) + + // Call handler + result, err := handler(context.Background(), request) + + // Verify success + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError) + require.Len(t, result.Content, 2) + + // Verify text content (message) + textContent, ok := result.Content[0].(mcp.TextContent) + require.True(t, ok, "First content should be text") + assert.Equal(t, "text", textContent.Type) + assert.Contains(t, textContent.Text, "File content available via ResourceLink") + + // Verify ResourceLink + resourceLink, ok := result.Content[1].(mcp.ResourceLink) + require.True(t, ok, "Second content should be ResourceLink") + assert.Equal(t, tc.expectedURI, resourceLink.URI) + assert.Equal(t, tc.expectedDesc, resourceLink.Description) + }) + } +} + func Test_ForkRepository(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 728d780977..3cb98093d8 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -158,6 +158,10 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG toolsets.NewServerTool(RerunFailedJobs(getClient, t)), toolsets.NewServerTool(CancelWorkflowRun(getClient, t)), toolsets.NewServerTool(DeleteWorkflowRunLogs(getClient, t)), + ). + AddResourceTemplates( + toolsets.NewServerResourceTemplate(GetWorkflowRunLogsResource(getClient, t)), + toolsets.NewServerResourceTemplate(GetJobLogsResource(getClient, t)), ) securityAdvisories := toolsets.NewToolset("security_advisories", "Security advisories related tools"). From 66cdefaf02b593db9a6dbc93d57bbf71ecc604ac Mon Sep 17 00:00:00 2001 From: Matt Holloway Date: Fri, 29 Aug 2025 14:45:19 +0100 Subject: [PATCH 2/2] lint: fix if to case --- pkg/github/actions_resource.go | 18 +++++++++--------- pkg/github/repositories.go | 9 +++++---- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/pkg/github/actions_resource.go b/pkg/github/actions_resource.go index 3a3c5ce984..c7515e79ea 100644 --- a/pkg/github/actions_resource.go +++ b/pkg/github/actions_resource.go @@ -45,12 +45,12 @@ func WorkflowRunLogsResourceHandler(getClient GetClientFn) func(ctx context.Cont return nil, errors.New("repo is required") } - runIdStr, ok := request.Params.Arguments["runId"].([]string) - if !ok || len(runIdStr) == 0 { + runIDStr, ok := request.Params.Arguments["runId"].([]string) + if !ok || len(runIDStr) == 0 { return nil, errors.New("runId is required") } - runId, err := strconv.ParseInt(runIdStr[0], 10, 64) + runID, err := strconv.ParseInt(runIDStr[0], 10, 64) if err != nil { return nil, fmt.Errorf("invalid runId: %w", err) } @@ -61,7 +61,7 @@ func WorkflowRunLogsResourceHandler(getClient GetClientFn) func(ctx context.Cont } // Get the JIT URL for workflow run logs - url, resp, err := client.Actions.GetWorkflowRunLogs(ctx, owner[0], repo[0], runId, 1) + url, resp, err := client.Actions.GetWorkflowRunLogs(ctx, owner[0], repo[0], runID, 1) if err != nil { return nil, fmt.Errorf("failed to get workflow run logs URL: %w", err) } @@ -77,7 +77,7 @@ func WorkflowRunLogsResourceHandler(getClient GetClientFn) func(ctx context.Cont mcp.TextResourceContents{ URI: request.Params.URI, MIMEType: "application/zip", - Text: fmt.Sprintf("Workflow run logs for run %d (ZIP archive)\n\nNote: This is a ZIP archive containing all job logs. Download URL was: %s\n\nContent length: %d bytes", runId, url.String(), len(content)), + Text: fmt.Sprintf("Workflow run logs for run %d (ZIP archive)\n\nNote: This is a ZIP archive containing all job logs. Download URL was: %s\n\nContent length: %d bytes", runID, url.String(), len(content)), }, }, nil } @@ -97,12 +97,12 @@ func JobLogsResourceHandler(getClient GetClientFn) func(ctx context.Context, req return nil, errors.New("repo is required") } - jobIdStr, ok := request.Params.Arguments["jobId"].([]string) - if !ok || len(jobIdStr) == 0 { + jobIDStr, ok := request.Params.Arguments["jobId"].([]string) + if !ok || len(jobIDStr) == 0 { return nil, errors.New("jobId is required") } - jobId, err := strconv.ParseInt(jobIdStr[0], 10, 64) + jobID, err := strconv.ParseInt(jobIDStr[0], 10, 64) if err != nil { return nil, fmt.Errorf("invalid jobId: %w", err) } @@ -113,7 +113,7 @@ func JobLogsResourceHandler(getClient GetClientFn) func(ctx context.Context, req } // Get the JIT URL for job logs - url, resp, err := client.Actions.GetWorkflowJobLogs(ctx, owner[0], repo[0], jobId, 1) + url, resp, err := client.Actions.GetWorkflowJobLogs(ctx, owner[0], repo[0], jobID, 1) if err != nil { return nil, fmt.Errorf("failed to get job logs URL: %w", err) } diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index a381f44eee..705fb8699c 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -1671,20 +1671,21 @@ func handleFileContentsResourceLink(owner, repo, path, ref, sha string) (*mcp.Ca description = fmt.Sprintf("File content for %s at commit %s in %s/%s", path, sha[:8], owner, repo) case ref != "": // Handle different ref types - if strings.HasPrefix(ref, "refs/heads/") { + switch { + case strings.HasPrefix(ref, "refs/heads/"): branch := strings.TrimPrefix(ref, "refs/heads/") resourceURI = fmt.Sprintf("repo://%s/%s/refs/heads/%s/contents%s", owner, repo, branch, path) description = fmt.Sprintf("File content for %s on branch %s in %s/%s", path, branch, owner, repo) - } else if strings.HasPrefix(ref, "refs/tags/") { + case strings.HasPrefix(ref, "refs/tags/"): tag := strings.TrimPrefix(ref, "refs/tags/") resourceURI = fmt.Sprintf("repo://%s/%s/refs/tags/%s/contents%s", owner, repo, tag, path) description = fmt.Sprintf("File content for %s at tag %s in %s/%s", path, tag, owner, repo) - } else if strings.HasPrefix(ref, "refs/pull/") && strings.HasSuffix(ref, "/head") { + case strings.HasPrefix(ref, "refs/pull/") && strings.HasSuffix(ref, "/head"): // Extract PR number from refs/pull/{number}/head prNumber := strings.TrimSuffix(strings.TrimPrefix(ref, "refs/pull/"), "/head") resourceURI = fmt.Sprintf("repo://%s/%s/pulls/%s/contents%s", owner, repo, prNumber, path) description = fmt.Sprintf("File content for %s in PR #%s in %s/%s", path, prNumber, owner, repo) - } else { + default: // Generic ref handling - try to clean it up cleanRef := strings.TrimPrefix(ref, "refs/") resourceURI = fmt.Sprintf("repo://%s/%s/refs/%s/contents%s", owner, repo, cleanRef, path)