From 3fba5487c292c8e5b05d35dc5f6a206226d30919 Mon Sep 17 00:00:00 2001 From: MayorFaj Date: Wed, 13 May 2026 23:01:48 +0100 Subject: [PATCH 1/2] feat: add get_file_blame tool --- README.md | 11 + pkg/github/__toolsnaps__/get_file_blame.snap | 55 ++ pkg/github/repositories.go | 363 +++++++++++++ pkg/github/repositories_test.go | 531 +++++++++++++++++++ pkg/github/tools.go | 1 + 5 files changed, 961 insertions(+) create mode 100644 pkg/github/__toolsnaps__/get_file_blame.snap diff --git a/README.md b/README.md index a12f1531ac..3aa721454a 100644 --- a/README.md +++ b/README.md @@ -1205,6 +1205,17 @@ The following sets of tools are available: - `repo`: Repository name (string, required) - `sha`: Commit SHA, branch name, or tag name (string, required) +- **get_file_blame** - Get file blame information + - **Required OAuth Scopes**: `repo` + - `end_line`: Optional 1-based ending line of the window of interest. Must be >= start_line when both are provided. (number, optional) + - `owner`: Repository owner (username or organization) (string, required) + - `page`: Page number for pagination (min 1) (number, optional) + - `path`: Path to the file in the repository, relative to the repository root (string, required) + - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) + - `ref`: Git reference (branch, tag, or commit SHA). Defaults to the repository's default branch (HEAD). (string, optional) + - `repo`: Repository name (string, required) + - `start_line`: Optional 1-based starting line of the window of interest. Only ranges overlapping [start_line, end_line] are returned, clamped to the window. (number, optional) + - **get_file_contents** - Get file or directory contents - **Required OAuth Scopes**: `repo` - `owner`: Repository owner (username or organization) (string, required) diff --git a/pkg/github/__toolsnaps__/get_file_blame.snap b/pkg/github/__toolsnaps__/get_file_blame.snap new file mode 100644 index 0000000000..bcba6fa1a2 --- /dev/null +++ b/pkg/github/__toolsnaps__/get_file_blame.snap @@ -0,0 +1,55 @@ +{ + "annotations": { + "readOnlyHint": true, + "title": "Get file blame information" + }, + "description": "Get git blame information for a file, showing the commit that last modified each line. Ranges share commit metadata via the top-level 'commits' map keyed by SHA. Use 'start_line'/'end_line' to restrict the result to a window of the file, and 'page'/'perPage' to page through returned ranges. Matching ranges are capped at 1000; when the cap is hit 'truncated' is set to true and 'total_ranges' reports the pre-cap match count.", + "inputSchema": { + "properties": { + "end_line": { + "description": "Optional 1-based ending line of the window of interest. Must be \u003e= start_line when both are provided.", + "minimum": 1, + "type": "number" + }, + "owner": { + "description": "Repository owner (username or organization)", + "type": "string" + }, + "page": { + "description": "Page number for pagination (min 1)", + "minimum": 1, + "type": "number" + }, + "path": { + "description": "Path to the file in the repository, relative to the repository root", + "type": "string" + }, + "perPage": { + "description": "Results per page for pagination (min 1, max 100)", + "maximum": 100, + "minimum": 1, + "type": "number" + }, + "ref": { + "description": "Git reference (branch, tag, or commit SHA). Defaults to the repository's default branch (HEAD).", + "type": "string" + }, + "repo": { + "description": "Repository name", + "type": "string" + }, + "start_line": { + "description": "Optional 1-based starting line of the window of interest. Only ranges overlapping [start_line, end_line] are returned, clamped to the window.", + "minimum": 1, + "type": "number" + } + }, + "required": [ + "owner", + "repo", + "path" + ], + "type": "object" + }, + "name": "get_file_blame" +} diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 507677ee57..c6b828eba3 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "slices" "strings" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -19,6 +20,7 @@ import ( "github.com/google/go-github/v82/github" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" ) func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool { @@ -2240,3 +2242,364 @@ func UnstarRepository(t translations.TranslationHelperFunc) inventory.ServerTool }, ) } + +// maxBlameRanges caps the number of matching blame ranges considered for one response. +const maxBlameRanges = 1000 + +// BlameAuthor describes the author of a commit referenced by a BlameRange. +type BlameAuthor struct { + Name string `json:"name"` + Email string `json:"email"` + Login *string `json:"login,omitempty"` + URL *string `json:"url,omitempty"` +} + +// BlameCommit holds commit metadata shared by one or more blame ranges. +type BlameCommit struct { + SHA string `json:"sha"` + MessageHeadline string `json:"message_headline"` + CommittedDate string `json:"committed_date"` + Author BlameAuthor `json:"author"` +} + +// BlameRange is a contiguous run of lines attributed to a single commit. +// +// Age is the relative position of this range's commit among distinct commits +// touching the file (0 = newest), not an absolute time delta. See: +// https://docs.github.com/en/graphql/reference/objects#blamerange +type BlameRange struct { + StartingLine int `json:"starting_line"` + EndingLine int `json:"ending_line"` + Age int `json:"age"` + CommitSHA string `json:"commit_sha"` +} + +// BlameResult is the response payload returned by the get_file_blame tool. +// +// Commits is keyed by SHA. TotalRanges counts matching ranges before +// pagination or truncation. Truncated reports whether maxBlameRanges was hit. +type BlameResult struct { + Repository string `json:"repository"` + Path string `json:"path"` + Ref string `json:"ref"` + Ranges []BlameRange `json:"ranges"` + Commits map[string]BlameCommit `json:"commits"` + TotalRanges int `json:"total_ranges"` + Truncated bool `json:"truncated,omitempty"` +} + +// validateBlamePath rejects empty, leading-slash, traversal-laden, or +// control-character paths before any network call is made. +func validateBlamePath(p string) error { + if strings.TrimSpace(p) == "" { + return fmt.Errorf("path must not be empty") + } + if strings.HasPrefix(p, "/") { + return fmt.Errorf("path must be relative to the repository root (no leading '/')") + } + if slices.Contains(strings.Split(p, "/"), "..") { + return fmt.Errorf("path must not contain '..' segments") + } + for _, r := range p { + if r < 0x20 || r == 0x7f { + return fmt.Errorf("path must not contain control characters") + } + } + return nil +} + +func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { + return NewTool( + ToolsetMetadataRepos, + mcp.Tool{ + Name: "get_file_blame", + Description: t("TOOL_GET_FILE_BLAME_DESCRIPTION", + "Get git blame information for a file, showing the commit that last modified each line. "+ + "Ranges share commit metadata via the top-level 'commits' map keyed by SHA. "+ + "Use 'start_line'/'end_line' to restrict the result to a window of the file, and "+ + "'page'/'perPage' to page through returned ranges. Matching ranges are capped at "+ + "1000; when the cap is hit 'truncated' is set to true and 'total_ranges' reports the pre-cap match count.", + ), + Annotations: &mcp.ToolAnnotations{ + Title: t("TOOL_GET_FILE_BLAME_USER_TITLE", "Get file blame information"), + ReadOnlyHint: true, + }, + InputSchema: WithPagination(&jsonschema.Schema{ + Type: "object", + Properties: map[string]*jsonschema.Schema{ + "owner": { + Type: "string", + Description: "Repository owner (username or organization)", + }, + "repo": { + Type: "string", + Description: "Repository name", + }, + "path": { + Type: "string", + Description: "Path to the file in the repository, relative to the repository root", + }, + "ref": { + Type: "string", + Description: "Git reference (branch, tag, or commit SHA). Defaults to the repository's default branch (HEAD).", + }, + "start_line": { + Type: "number", + Description: "Optional 1-based starting line of the window of interest. Only ranges overlapping [start_line, end_line] are returned, clamped to the window.", + Minimum: jsonschema.Ptr(1.0), + }, + "end_line": { + Type: "number", + Description: "Optional 1-based ending line of the window of interest. Must be >= start_line when both are provided.", + Minimum: jsonschema.Ptr(1.0), + }, + }, + Required: []string{"owner", "repo", "path"}, + }), + }, + []scopes.Scope{scopes.Repo}, + func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, args map[string]any) (*mcp.CallToolResult, any, error) { + owner, err := RequiredParam[string](args, "owner") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + repo, err := RequiredParam[string](args, "repo") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + path, err := RequiredParam[string](args, "path") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if err := validateBlamePath(path); err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + ref, err := OptionalParam[string](args, "ref") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + _, hasStartLine := args["start_line"] + startLine, err := OptionalIntParam(args, "start_line") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if hasStartLine && startLine < 1 { + return utils.NewToolResultError("start_line must be omitted or >= 1"), nil, nil + } + _, hasEndLine := args["end_line"] + endLine, err := OptionalIntParam(args, "end_line") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if hasEndLine && endLine < 1 { + return utils.NewToolResultError("end_line must be omitted or >= 1"), nil, nil + } + if hasStartLine && hasEndLine && endLine < startLine { + return utils.NewToolResultError("end_line must be >= start_line when both are provided"), nil, nil + } + page := 1 + if _, hasPage := args["page"]; hasPage { + page, err = OptionalIntParam(args, "page") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if page < 1 { + return utils.NewToolResultError("page must be >= 1 when provided"), nil, nil + } + } + perPage := 30 + if _, hasPerPage := args["perPage"]; hasPerPage { + perPage, err = OptionalIntParam(args, "perPage") + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil + } + if perPage < 1 || perPage > 100 { + return utils.NewToolResultError("perPage must be between 1 and 100 when provided"), nil, nil + } + } + + client, err := deps.GetGQLClient(ctx) + if err != nil { + return nil, nil, fmt.Errorf("failed to get GitHub GraphQL client: %w", err) + } + + // Default to HEAD and fetch defaultBranchRef.name in the same query + // so the response can echo a readable ref. + refExpression := ref + if refExpression == "" { + refExpression = "HEAD" + } + + var blameQuery struct { + Repository struct { + DefaultBranchRef struct { + Name githubv4.String + } + Object struct { + Typename githubv4.String `graphql:"__typename"` + Commit struct { + Blame struct { + Ranges []struct { + StartingLine githubv4.Int + EndingLine githubv4.Int + Age githubv4.Int + Commit struct { + OID githubv4.String + Message githubv4.String + CommittedDate githubv4.DateTime + Author struct { + Name githubv4.String + Email githubv4.String + User *struct { + Login githubv4.String + URL githubv4.String + } + } + } + } + } `graphql:"blame(path: $path)"` + } `graphql:"... on Commit"` + } `graphql:"object(expression: $ref)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + vars := map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "ref": githubv4.String(refExpression), + "path": githubv4.String(path), + } + + if err := client.Query(ctx, &blameQuery, vars); err != nil { + return ghErrors.NewGitHubGraphQLErrorResponse(ctx, + fmt.Sprintf("failed to get blame for file: %s", path), + err, + ), nil, nil + } + + // The ref must resolve to a commit. Empty typename means not found; + // any other type is unsupported for blame. + objectTypename := string(blameQuery.Repository.Object.Typename) + if objectTypename == "" { + return utils.NewToolResultError( + fmt.Sprintf("ref %q was not found in %s/%s", refExpression, owner, repo), + ), nil, nil + } + if objectTypename != "Commit" { + return utils.NewToolResultError( + fmt.Sprintf("ref %q did not resolve to a commit in %s/%s (resolved to %s)", + refExpression, owner, repo, objectTypename), + ), nil, nil + } + + // Echo the caller's ref, otherwise prefer the default branch name. + responseRef := ref + if responseRef == "" { + if name := string(blameQuery.Repository.DefaultBranchRef.Name); name != "" { + responseRef = name + } else { + responseRef = refExpression + } + } + + rawRanges := blameQuery.Repository.Object.Commit.Blame.Ranges + + // Filter / clamp to the requested line window. + windowed := make([]BlameRange, 0, len(rawRanges)) + for _, r := range rawRanges { + start := int(r.StartingLine) + end := int(r.EndingLine) + if startLine > 0 && end < startLine { + continue + } + if endLine > 0 && start > endLine { + continue + } + if startLine > 0 && start < startLine { + start = startLine + } + if endLine > 0 && end > endLine { + end = endLine + } + windowed = append(windowed, BlameRange{ + StartingLine: start, + EndingLine: end, + Age: int(r.Age), + CommitSHA: string(r.Commit.OID), + }) + } + + totalRanges := len(windowed) + + // Cap before pagination so truncation is reported consistently. + truncated := false + if len(windowed) > maxBlameRanges { + truncated = true + windowed = windowed[:maxBlameRanges] + } + + // Apply page/perPage pagination over the (filtered, capped) set. + cappedRanges := len(windowed) + offset := min((page-1)*perPage, cappedRanges) + end := min(offset+perPage, cappedRanges) + pageRanges := windowed[offset:end] + + // Collect commit metadata only for SHAs referenced by this page. + needed := make(map[string]struct{}, len(pageRanges)) + for _, br := range pageRanges { + needed[br.CommitSHA] = struct{}{} + } + commits := make(map[string]BlameCommit, len(needed)) + for _, r := range rawRanges { + sha := string(r.Commit.OID) + if _, want := needed[sha]; !want { + continue + } + if _, seen := commits[sha]; seen { + continue + } + headline := string(r.Commit.Message) + if idx := strings.IndexByte(headline, '\n'); idx >= 0 { + headline = headline[:idx] + } + headline = strings.TrimRight(headline, " \t\r") + bc := BlameCommit{ + SHA: sha, + MessageHeadline: headline, + CommittedDate: r.Commit.CommittedDate.Format("2006-01-02T15:04:05Z"), + Author: BlameAuthor{ + Name: string(r.Commit.Author.Name), + Email: string(r.Commit.Author.Email), + }, + } + if r.Commit.Author.User != nil { + login := string(r.Commit.Author.User.Login) + url := string(r.Commit.Author.User.URL) + bc.Author.Login = &login + bc.Author.URL = &url + } + commits[sha] = bc + } + + result := BlameResult{ + Repository: fmt.Sprintf("%s/%s", owner, repo), + Path: path, + Ref: responseRef, + Ranges: pageRanges, + Commits: commits, + TotalRanges: totalRanges, + Truncated: truncated, + } + if result.Ranges == nil { + result.Ranges = []BlameRange{} + } + + payload, err := json.Marshal(result) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) + } + + return utils.NewToolResultText(string(payload)), nil, nil + }, + ) +} diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index ceaa959019..53b4812525 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/github/github-mcp-server/internal/githubv4mock" "github.com/github/github-mcp-server/internal/toolsnaps" "github.com/github/github-mcp-server/pkg/raw" "github.com/github/github-mcp-server/pkg/translations" @@ -17,6 +18,7 @@ import ( "github.com/google/go-github/v82/github" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -4376,3 +4378,532 @@ func Test_UnstarRepository(t *testing.T) { }) } } +func Test_GetFileBlame(t *testing.T) { + // Verify tool definition once + serverTool := GetFileBlame(translations.NullTranslationHelper) + tool := serverTool.Tool + require.NoError(t, toolsnaps.Test(tool.Name, tool)) + + schema, ok := tool.InputSchema.(*jsonschema.Schema) + require.True(t, ok, "InputSchema should be *jsonschema.Schema") + + assert.Equal(t, "get_file_blame", tool.Name) + assert.NotEmpty(t, tool.Description) + for _, key := range []string{"owner", "repo", "path", "ref", "start_line", "end_line", "page", "perPage"} { + assert.Contains(t, schema.Properties, key, "schema missing property %q", key) + } + assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "path"}) + require.NotNil(t, tool.Annotations) + assert.True(t, tool.Annotations.ReadOnlyHint, "blame is read-only") + + // blameQueryShape is the GraphQL query shape used by all + // network-touching subtests below. Defined once so changes to the wire + // schema are made in a single place. + type blameQueryShape = struct { + Repository struct { + DefaultBranchRef struct { + Name githubv4.String + } + Object struct { + Typename githubv4.String `graphql:"__typename"` + Commit struct { + Blame struct { + Ranges []struct { + StartingLine githubv4.Int + EndingLine githubv4.Int + Age githubv4.Int + Commit struct { + OID githubv4.String + Message githubv4.String + CommittedDate githubv4.DateTime + Author struct { + Name githubv4.String + Email githubv4.String + User *struct { + Login githubv4.String + URL githubv4.String + } + } + } + } + } `graphql:"blame(path: $path)"` + } `graphql:"... on Commit"` + } `graphql:"object(expression: $ref)"` + } `graphql:"repository(owner: $owner, name: $repo)"` + } + + makeBlameVars := func(owner, repo, ref, path string) map[string]any { + return map[string]any{ + "owner": githubv4.String(owner), + "repo": githubv4.String(repo), + "ref": githubv4.String(ref), + "path": githubv4.String(path), + } + } + + tests := []struct { + name string + mockedClient *http.Client + requestArgs map[string]any + expectError bool + expectedErrMsg string + validateResponse func(t *testing.T, result string) + }{ + { + name: "successful blame using default branch (HEAD)", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "HEAD", "README.md"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": map[string]any{ + "__typename": "Commit", + "blame": map[string]any{ + "ranges": []map[string]any{ + { + "startingLine": 1, "endingLine": 5, "age": 2, + "commit": map[string]any{ + "oid": "abc123def456", + "message": "Initial commit\n\nLong body that should not appear in the response.", + "committedDate": "2024-01-01T12:00:00Z", + "author": map[string]any{ + "name": "John Doe", "email": "john@example.com", + "user": map[string]any{"login": "johndoe", "url": "https://github.com/johndoe"}, + }, + }, + }, + { + // Same commit as the first range -> must be deduplicated. + "startingLine": 6, "endingLine": 7, "age": 2, + "commit": map[string]any{ + "oid": "abc123def456", + "message": "Initial commit\n\nLong body that should not appear in the response.", + "committedDate": "2024-01-01T12:00:00Z", + "author": map[string]any{ + "name": "John Doe", "email": "john@example.com", + "user": map[string]any{"login": "johndoe", "url": "https://github.com/johndoe"}, + }, + }, + }, + { + "startingLine": 8, "endingLine": 10, "age": 1, + "commit": map[string]any{ + "oid": "def456ghi789", + "message": "Update README", + "committedDate": "2024-01-02T15:30:00Z", + "author": map[string]any{ + "name": "Jane Smith", "email": "jane@example.com", + "user": map[string]any{"login": "janesmith", "url": "https://github.com/janesmith"}, + }, + }, + }, + }, + }, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "README.md", + }, + validateResponse: func(t *testing.T, result string) { + var br BlameResult + require.NoError(t, json.Unmarshal([]byte(result), &br)) + assert.Equal(t, "testowner/testrepo", br.Repository) + assert.Equal(t, "README.md", br.Path) + assert.Equal(t, "main", br.Ref, "ref should resolve to default branch name") + assert.False(t, br.Truncated) + assert.Equal(t, 3, br.TotalRanges) + require.Len(t, br.Ranges, 3) + // Commits map is deduplicated. + require.Len(t, br.Commits, 2) + require.Contains(t, br.Commits, "abc123def456") + require.Contains(t, br.Commits, "def456ghi789") + // Multi-line message must be reduced to its headline. + assert.Equal(t, "Initial commit", br.Commits["abc123def456"].MessageHeadline) + assert.NotContains(t, result, "Long body that should not appear") + // Login/URL pointers populated. + require.NotNil(t, br.Commits["abc123def456"].Author.Login) + assert.Equal(t, "johndoe", *br.Commits["abc123def456"].Author.Login) + }, + }, + { + name: "successful blame with explicit ref", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "feature-branch", "src/main.go"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": map[string]any{ + "__typename": "Commit", + "blame": map[string]any{ + "ranges": []map[string]any{ + { + "startingLine": 1, "endingLine": 3, "age": 1, + "commit": map[string]any{ + "oid": "xyz789abc123", + "message": "Add main function", + "committedDate": "2024-01-03T10:00:00Z", + "author": map[string]any{ + "name": "Bob Developer", "email": "bob@example.com", + "user": nil, + }, + }, + }, + }, + }, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "src/main.go", + "ref": "feature-branch", + }, + validateResponse: func(t *testing.T, result string) { + var br BlameResult + require.NoError(t, json.Unmarshal([]byte(result), &br)) + assert.Equal(t, "feature-branch", br.Ref, "explicit ref echoed back") + require.Len(t, br.Ranges, 1) + require.Contains(t, br.Commits, "xyz789abc123") + assert.Nil(t, br.Commits["xyz789abc123"].Author.Login, "anonymous author has no login") + }, + }, + { + name: "empty blame ranges", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "HEAD", "EMPTY.md"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": map[string]any{ + "__typename": "Commit", + "blame": map[string]any{"ranges": []map[string]any{}}, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "EMPTY.md", + }, + validateResponse: func(t *testing.T, result string) { + var br BlameResult + require.NoError(t, json.Unmarshal([]byte(result), &br)) + assert.Equal(t, 0, br.TotalRanges) + assert.Empty(t, br.Ranges) + assert.Empty(t, br.Commits) + assert.False(t, br.Truncated) + // Ranges should marshal as an empty array, not null. + assert.Contains(t, result, `"ranges":[]`) + }, + }, + { + name: "ref resolves to non-commit object", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "main", "docs"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": map[string]any{ + "__typename": "Tree", + "blame": map[string]any{"ranges": []map[string]any{}}, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "docs", + "ref": "main", + }, + expectError: true, + expectedErrMsg: "did not resolve to a commit", + }, + { + name: "ref not found", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "no-such-ref", "README.md"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": nil, + }, + }), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "README.md", + "ref": "no-such-ref", + }, + expectError: true, + expectedErrMsg: "was not found", + }, + { + name: "line-range filter clamps and drops ranges", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "HEAD", "src/big.go"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": map[string]any{ + "__typename": "Commit", + "blame": map[string]any{ + "ranges": []map[string]any{ + { + "startingLine": 1, "endingLine": 5, "age": 1, + "commit": map[string]any{ + "oid": "sha-A", "message": "A", "committedDate": "2024-01-01T00:00:00Z", + "author": map[string]any{"name": "a", "email": "a@x", "user": nil}, + }, + }, + { + "startingLine": 6, "endingLine": 12, "age": 1, + "commit": map[string]any{ + "oid": "sha-B", "message": "B", "committedDate": "2024-01-01T00:00:00Z", + "author": map[string]any{"name": "b", "email": "b@x", "user": nil}, + }, + }, + { + "startingLine": 13, "endingLine": 20, "age": 1, + "commit": map[string]any{ + "oid": "sha-C", "message": "C", "committedDate": "2024-01-01T00:00:00Z", + "author": map[string]any{"name": "c", "email": "c@x", "user": nil}, + }, + }, + }, + }, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "src/big.go", + "start_line": float64(8), + "end_line": float64(15), + }, + validateResponse: func(t *testing.T, result string) { + var br BlameResult + require.NoError(t, json.Unmarshal([]byte(result), &br)) + // First range (1-5) is dropped; middle clamped to 8-12; + // last clamped to 13-15. + require.Len(t, br.Ranges, 2) + assert.Equal(t, 8, br.Ranges[0].StartingLine) + assert.Equal(t, 12, br.Ranges[0].EndingLine) + assert.Equal(t, "sha-B", br.Ranges[0].CommitSHA) + assert.Equal(t, 13, br.Ranges[1].StartingLine) + assert.Equal(t, 15, br.Ranges[1].EndingLine) + assert.Equal(t, "sha-C", br.Ranges[1].CommitSHA) + assert.NotContains(t, br.Commits, "sha-A", "filtered-out commit must not appear") + }, + }, + { + name: "GraphQL error is surfaced", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "main", "nonexistent.txt"), + githubv4mock.ErrorResponse("file not found"), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "nonexistent.txt", + "ref": "main", + }, + expectError: true, + expectedErrMsg: "file not found", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + client := githubv4.NewClient(tc.mockedClient) + deps := BaseDeps{GQLClient: client} + handler := serverTool.Handler(deps) + + request := createMCPRequest(tc.requestArgs) + result, err := handler(ContextWithDeps(context.Background(), deps), &request) + + if tc.expectError { + require.NoError(t, err) + require.True(t, result.IsError) + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + + require.NoError(t, err) + require.False(t, result.IsError) + textContent := getTextResult(t, result) + if tc.validateResponse != nil { + tc.validateResponse(t, textContent.Text) + } + }) + } + + // Path validation must short-circuit before any network call. We supply + // a client with no matchers so any HTTP attempt would fail loudly. + t.Run("path validation rejects bad inputs", func(t *testing.T) { + client := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{GQLClient: client} + handler := serverTool.Handler(deps) + + cases := []struct { + name string + path string + want string + }{ + {"empty", " ", "must not be empty"}, + {"absolute", "/etc/passwd", "must be relative"}, + {"traversal", "src/../../../etc/passwd", "must not contain '..'"}, + {"control char", "src/\x00bad.go", "control characters"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := createMCPRequest(map[string]any{ + "owner": "o", "repo": "r", "path": c.path, + }) + result, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, result.IsError, "expected validation error for %q", c.path) + assert.Contains(t, getErrorResult(t, result).Text, c.want) + }) + } + }) + + // Line-window and pagination validation also short-circuits. + t.Run("line-range argument validation", func(t *testing.T) { + client := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) + deps := BaseDeps{GQLClient: client} + handler := serverTool.Handler(deps) + + cases := []struct { + name string + args map[string]any + want string + }{ + { + "end before start", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "start_line": float64(10), "end_line": float64(5)}, + "end_line must be >= start_line when both are provided", + }, + { + "start line zero", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "start_line": float64(0)}, + "start_line must be omitted or >= 1", + }, + { + "end line zero", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "end_line": float64(0)}, + "end_line must be omitted or >= 1", + }, + { + "invalid page", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "page": float64(-1)}, + "page must be >= 1 when provided", + }, + { + "page zero", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "page": float64(0)}, + "page must be >= 1 when provided", + }, + { + "perPage too large", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "perPage": float64(101)}, + "perPage must be between 1 and 100 when provided", + }, + { + "perPage zero", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "perPage": float64(0)}, + "perPage must be between 1 and 100 when provided", + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + req := createMCPRequest(c.args) + result, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Contains(t, getErrorResult(t, result).Text, c.want) + }) + } + }) + + // Truncation: hand-build a response with > maxBlameRanges to verify + // the cap is applied and surfaced. + t.Run("truncation at maxBlameRanges", func(t *testing.T) { + ranges := make([]map[string]any, 0, maxBlameRanges+5) + for i := range maxBlameRanges + 5 { + ranges = append(ranges, map[string]any{ + "startingLine": i + 1, "endingLine": i + 1, "age": 0, + "commit": map[string]any{ + "oid": "sha-shared", + "message": "shared", + "committedDate": "2024-01-01T00:00:00Z", + "author": map[string]any{"name": "n", "email": "e@x", "user": nil}, + }, + }) + } + mocked := githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("o", "r", "HEAD", "huge.txt"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": map[string]any{ + "__typename": "Commit", + "blame": map[string]any{"ranges": ranges}, + }, + }, + }), + ), + ) + // Use a large perPage so the truncated set is observable on a + // single page. + req := createMCPRequest(map[string]any{ + "owner": "o", "repo": "r", "path": "huge.txt", "perPage": float64(100), + }) + client := githubv4.NewClient(mocked) + deps := BaseDeps{GQLClient: client} + handler := serverTool.Handler(deps) + result, err := handler(ContextWithDeps(context.Background(), deps), &req) + require.NoError(t, err) + require.False(t, result.IsError) + + var br BlameResult + require.NoError(t, json.Unmarshal([]byte(getTextResult(t, result).Text), &br)) + assert.True(t, br.Truncated, "truncation flag must be set") + assert.Equal(t, maxBlameRanges+5, br.TotalRanges) + assert.Len(t, br.Ranges, 100, "perPage limits the page size") + }) +} diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 559088f6d6..f9327ab893 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -184,6 +184,7 @@ func AllTools(t translations.TranslationHelperFunc) []inventory.ServerTool { ListCommits(t), SearchCode(t), GetCommit(t), + GetFileBlame(t), ListBranches(t), ListTags(t), GetTag(t), From ceff0c9a8295668a9d1ef0b953b1a3b1ad213347 Mon Sep 17 00:00:00 2001 From: MayorFaj Date: Thu, 14 May 2026 18:45:16 +0100 Subject: [PATCH 2/2] feat: implement cursor-based pagination for get_file_blame tool --- README.md | 2 +- pkg/github/__toolsnaps__/get_file_blame.snap | 11 +- pkg/github/repositories.go | 118 ++++++++++++------- pkg/github/repositories_test.go | 87 ++++++++++++-- 4 files changed, 160 insertions(+), 58 deletions(-) diff --git a/README.md b/README.md index 3aa721454a..91c1ee6dfd 100644 --- a/README.md +++ b/README.md @@ -1207,9 +1207,9 @@ The following sets of tools are available: - **get_file_blame** - Get file blame information - **Required OAuth Scopes**: `repo` + - `after`: Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs. (string, optional) - `end_line`: Optional 1-based ending line of the window of interest. Must be >= start_line when both are provided. (number, optional) - `owner`: Repository owner (username or organization) (string, required) - - `page`: Page number for pagination (min 1) (number, optional) - `path`: Path to the file in the repository, relative to the repository root (string, required) - `perPage`: Results per page for pagination (min 1, max 100) (number, optional) - `ref`: Git reference (branch, tag, or commit SHA). Defaults to the repository's default branch (HEAD). (string, optional) diff --git a/pkg/github/__toolsnaps__/get_file_blame.snap b/pkg/github/__toolsnaps__/get_file_blame.snap index bcba6fa1a2..6e4a19862d 100644 --- a/pkg/github/__toolsnaps__/get_file_blame.snap +++ b/pkg/github/__toolsnaps__/get_file_blame.snap @@ -3,9 +3,13 @@ "readOnlyHint": true, "title": "Get file blame information" }, - "description": "Get git blame information for a file, showing the commit that last modified each line. Ranges share commit metadata via the top-level 'commits' map keyed by SHA. Use 'start_line'/'end_line' to restrict the result to a window of the file, and 'page'/'perPage' to page through returned ranges. Matching ranges are capped at 1000; when the cap is hit 'truncated' is set to true and 'total_ranges' reports the pre-cap match count.", + "description": "Get git blame information for a file, showing the commit that last modified each line. Ranges share commit metadata via the top-level 'commits' map keyed by SHA. Use 'start_line'/'end_line' to restrict the result to a window of the file, and 'perPage'/'after' to cursor-page through returned ranges. Matching ranges are capped at 1000; when the cap is hit 'truncated' is set to true and 'total_ranges' reports the pre-cap match count.", "inputSchema": { "properties": { + "after": { + "description": "Cursor for pagination. Use the endCursor from the previous page's PageInfo for GraphQL APIs.", + "type": "string" + }, "end_line": { "description": "Optional 1-based ending line of the window of interest. Must be \u003e= start_line when both are provided.", "minimum": 1, @@ -15,11 +19,6 @@ "description": "Repository owner (username or organization)", "type": "string" }, - "page": { - "description": "Page number for pagination (min 1)", - "minimum": 1, - "type": "number" - }, "path": { "description": "Path to the file in the repository, relative to the repository root", "type": "string" diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 0aa05808b3..8e6a849ce6 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -8,6 +8,7 @@ import ( "io" "net/http" "slices" + "strconv" "strings" ghErrors "github.com/github/github-mcp-server/pkg/errors" @@ -2208,6 +2209,35 @@ func UnstarRepository(t translations.TranslationHelperFunc) inventory.ServerTool // maxBlameRanges caps the number of matching blame ranges considered for one response. const maxBlameRanges = 1000 +const blameCursorPrefix = "blame-range:" + +func encodeBlameCursor(offset int) string { + return base64.RawURLEncoding.EncodeToString(fmt.Appendf(nil, "%s%d", blameCursorPrefix, offset)) +} + +func decodeBlameCursor(cursor string) (int, error) { + if cursor == "" { + return 0, nil + } + + decoded, err := base64.RawURLEncoding.DecodeString(cursor) + if err != nil { + return 0, fmt.Errorf("after cursor is invalid") + } + + value := string(decoded) + if !strings.HasPrefix(value, blameCursorPrefix) { + return 0, fmt.Errorf("after cursor is invalid") + } + + offset, err := strconv.Atoi(strings.TrimPrefix(value, blameCursorPrefix)) + if err != nil || offset < 0 { + return 0, fmt.Errorf("after cursor is invalid") + } + + return offset, nil +} + // BlameAuthor describes the author of a commit referenced by a BlameRange. type BlameAuthor struct { Name string `json:"name"` @@ -2238,7 +2268,7 @@ type BlameRange struct { // BlameResult is the response payload returned by the get_file_blame tool. // -// Commits is keyed by SHA. TotalRanges counts matching ranges before +// Commits is keyed by SHA. TotalRanges counts matching ranges before cursor // pagination or truncation. Truncated reports whether maxBlameRanges was hit. type BlameResult struct { Repository string `json:"repository"` @@ -2246,6 +2276,7 @@ type BlameResult struct { Ref string `json:"ref"` Ranges []BlameRange `json:"ranges"` Commits map[string]BlameCommit `json:"commits"` + PageInfo MinimalPageInfo `json:"pageInfo"` TotalRanges int `json:"total_ranges"` Truncated bool `json:"truncated,omitempty"` } @@ -2279,14 +2310,14 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { "Get git blame information for a file, showing the commit that last modified each line. "+ "Ranges share commit metadata via the top-level 'commits' map keyed by SHA. "+ "Use 'start_line'/'end_line' to restrict the result to a window of the file, and "+ - "'page'/'perPage' to page through returned ranges. Matching ranges are capped at "+ + "'perPage'/'after' to cursor-page through returned ranges. Matching ranges are capped at "+ "1000; when the cap is hit 'truncated' is set to true and 'total_ranges' reports the pre-cap match count.", ), Annotations: &mcp.ToolAnnotations{ Title: t("TOOL_GET_FILE_BLAME_USER_TITLE", "Get file blame information"), ReadOnlyHint: true, }, - InputSchema: WithPagination(&jsonschema.Schema{ + InputSchema: WithCursorPagination(&jsonschema.Schema{ Type: "object", Properties: map[string]*jsonschema.Schema{ "owner": { @@ -2359,25 +2390,26 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { if hasStartLine && hasEndLine && endLine < startLine { return utils.NewToolResultError("end_line must be >= start_line when both are provided"), nil, nil } - page := 1 if _, hasPage := args["page"]; hasPage { - page, err = OptionalIntParam(args, "page") - if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil - } - if page < 1 { - return utils.NewToolResultError("page must be >= 1 when provided"), nil, nil - } + return utils.NewToolResultError("This tool uses cursor-based pagination. Use the 'after' parameter with the 'endCursor' value from the previous response instead of 'page'."), nil, nil + } + pagination, err := OptionalCursorPaginationParams(args) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } - perPage := 30 if _, hasPerPage := args["perPage"]; hasPerPage { - perPage, err = OptionalIntParam(args, "perPage") + perPage, err := OptionalIntParam(args, "perPage") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } if perPage < 1 || perPage > 100 { return utils.NewToolResultError("perPage must be between 1 and 100 when provided"), nil, nil } + pagination.PerPage = perPage + } + afterOffset, err := decodeBlameCursor(pagination.After) + if err != nil { + return utils.NewToolResultError(err.Error()), nil, nil } client, err := deps.GetGQLClient(ctx) @@ -2465,9 +2497,11 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { } rawRanges := blameQuery.Repository.Object.Commit.Blame.Ranges + pageRanges := make([]BlameRange, 0, pagination.PerPage) + commits := make(map[string]BlameCommit) + totalRanges := 0 + truncated := false - // Filter / clamp to the requested line window. - windowed := make([]BlameRange, 0, len(rawRanges)) for _, r := range rawRanges { start := int(r.StartingLine) end := int(r.EndingLine) @@ -2483,40 +2517,26 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { if endLine > 0 && end > endLine { end = endLine } - windowed = append(windowed, BlameRange{ + + matchIndex := totalRanges + totalRanges++ + if matchIndex >= maxBlameRanges { + truncated = true + continue + } + if matchIndex < afterOffset || len(pageRanges) >= pagination.PerPage { + continue + } + + blameRange := BlameRange{ StartingLine: start, EndingLine: end, Age: int(r.Age), CommitSHA: string(r.Commit.OID), - }) - } - - totalRanges := len(windowed) - - // Cap before pagination so truncation is reported consistently. - truncated := false - if len(windowed) > maxBlameRanges { - truncated = true - windowed = windowed[:maxBlameRanges] - } - - // Apply page/perPage pagination over the (filtered, capped) set. - cappedRanges := len(windowed) - offset := min((page-1)*perPage, cappedRanges) - end := min(offset+perPage, cappedRanges) - pageRanges := windowed[offset:end] + } + pageRanges = append(pageRanges, blameRange) - // Collect commit metadata only for SHAs referenced by this page. - needed := make(map[string]struct{}, len(pageRanges)) - for _, br := range pageRanges { - needed[br.CommitSHA] = struct{}{} - } - commits := make(map[string]BlameCommit, len(needed)) - for _, r := range rawRanges { sha := string(r.Commit.OID) - if _, want := needed[sha]; !want { - continue - } if _, seen := commits[sha]; seen { continue } @@ -2543,12 +2563,24 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { commits[sha] = bc } + cappedRanges := min(totalRanges, maxBlameRanges) + consumedRanges := min(afterOffset+len(pageRanges), cappedRanges) + pageInfo := MinimalPageInfo{ + HasNextPage: consumedRanges < cappedRanges, + HasPreviousPage: afterOffset > 0, + } + if len(pageRanges) > 0 { + pageInfo.StartCursor = encodeBlameCursor(afterOffset) + pageInfo.EndCursor = encodeBlameCursor(consumedRanges) + } + result := BlameResult{ Repository: fmt.Sprintf("%s/%s", owner, repo), Path: path, Ref: responseRef, Ranges: pageRanges, Commits: commits, + PageInfo: pageInfo, TotalRanges: totalRanges, Truncated: truncated, } diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 204a1124d8..62f1d6abe8 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -4381,9 +4381,10 @@ func Test_GetFileBlame(t *testing.T) { assert.Equal(t, "get_file_blame", tool.Name) assert.NotEmpty(t, tool.Description) - for _, key := range []string{"owner", "repo", "path", "ref", "start_line", "end_line", "page", "perPage"} { + for _, key := range []string{"owner", "repo", "path", "ref", "start_line", "end_line", "perPage", "after"} { assert.Contains(t, schema.Properties, key, "schema missing property %q", key) } + assert.NotContains(t, schema.Properties, "page") assert.ElementsMatch(t, schema.Required, []string{"owner", "repo", "path"}) require.NotNil(t, tool.Annotations) assert.True(t, tool.Annotations.ReadOnlyHint, "blame is read-only") @@ -4511,6 +4512,10 @@ func Test_GetFileBlame(t *testing.T) { assert.Equal(t, "main", br.Ref, "ref should resolve to default branch name") assert.False(t, br.Truncated) assert.Equal(t, 3, br.TotalRanges) + assert.False(t, br.PageInfo.HasNextPage) + assert.False(t, br.PageInfo.HasPreviousPage) + assert.NotEmpty(t, br.PageInfo.StartCursor) + assert.NotEmpty(t, br.PageInfo.EndCursor) require.Len(t, br.Ranges, 3) // Commits map is deduplicated. require.Len(t, br.Commits, 2) @@ -4599,6 +4604,8 @@ func Test_GetFileBlame(t *testing.T) { assert.Equal(t, 0, br.TotalRanges) assert.Empty(t, br.Ranges) assert.Empty(t, br.Commits) + assert.False(t, br.PageInfo.HasNextPage) + assert.False(t, br.PageInfo.HasPreviousPage) assert.False(t, br.Truncated) // Ranges should marshal as an empty array, not null. assert.Contains(t, result, `"ranges":[]`) @@ -4716,6 +4723,68 @@ func Test_GetFileBlame(t *testing.T) { assert.NotContains(t, br.Commits, "sha-A", "filtered-out commit must not appear") }, }, + { + name: "cursor pagination returns requested page", + mockedClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + blameQueryShape{}, + makeBlameVars("testowner", "testrepo", "HEAD", "src/paged.go"), + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "defaultBranchRef": map[string]any{"name": "main"}, + "object": map[string]any{ + "__typename": "Commit", + "blame": map[string]any{ + "ranges": []map[string]any{ + { + "startingLine": 1, "endingLine": 1, "age": 1, + "commit": map[string]any{ + "oid": "sha-A", "message": "A", "committedDate": "2024-01-01T00:00:00Z", + "author": map[string]any{"name": "a", "email": "a@x", "user": nil}, + }, + }, + { + "startingLine": 2, "endingLine": 2, "age": 1, + "commit": map[string]any{ + "oid": "sha-B", "message": "B", "committedDate": "2024-01-01T00:00:00Z", + "author": map[string]any{"name": "b", "email": "b@x", "user": nil}, + }, + }, + { + "startingLine": 3, "endingLine": 3, "age": 1, + "commit": map[string]any{ + "oid": "sha-C", "message": "C", "committedDate": "2024-01-01T00:00:00Z", + "author": map[string]any{"name": "c", "email": "c@x", "user": nil}, + }, + }, + }, + }, + }, + }, + }), + ), + ), + requestArgs: map[string]any{ + "owner": "testowner", + "repo": "testrepo", + "path": "src/paged.go", + "perPage": float64(1), + "after": encodeBlameCursor(1), + }, + validateResponse: func(t *testing.T, result string) { + var br BlameResult + require.NoError(t, json.Unmarshal([]byte(result), &br)) + assert.Equal(t, 3, br.TotalRanges) + require.Len(t, br.Ranges, 1) + assert.Equal(t, "sha-B", br.Ranges[0].CommitSHA) + require.Len(t, br.Commits, 1) + require.Contains(t, br.Commits, "sha-B") + assert.True(t, br.PageInfo.HasNextPage) + assert.True(t, br.PageInfo.HasPreviousPage) + assert.Equal(t, encodeBlameCursor(1), br.PageInfo.StartCursor) + assert.Equal(t, encodeBlameCursor(2), br.PageInfo.EndCursor) + }, + }, { name: "GraphQL error is surfaced", mockedClient: githubv4mock.NewMockedHTTPClient( @@ -4792,7 +4861,7 @@ func Test_GetFileBlame(t *testing.T) { } }) - // Line-window and pagination validation also short-circuits. + // Line-window and cursor pagination validation also short-circuits. t.Run("line-range argument validation", func(t *testing.T) { client := githubv4.NewClient(githubv4mock.NewMockedHTTPClient()) deps := BaseDeps{GQLClient: client} @@ -4819,14 +4888,14 @@ func Test_GetFileBlame(t *testing.T) { "end_line must be omitted or >= 1", }, { - "invalid page", - map[string]any{"owner": "o", "repo": "r", "path": "f.go", "page": float64(-1)}, - "page must be >= 1 when provided", + "page not supported", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "page": float64(1)}, + "cursor-based pagination", }, { - "page zero", - map[string]any{"owner": "o", "repo": "r", "path": "f.go", "page": float64(0)}, - "page must be >= 1 when provided", + "invalid after cursor", + map[string]any{"owner": "o", "repo": "r", "path": "f.go", "after": "not-a-cursor"}, + "after cursor is invalid", }, { "perPage too large", @@ -4897,5 +4966,7 @@ func Test_GetFileBlame(t *testing.T) { assert.True(t, br.Truncated, "truncation flag must be set") assert.Equal(t, maxBlameRanges+5, br.TotalRanges) assert.Len(t, br.Ranges, 100, "perPage limits the page size") + assert.True(t, br.PageInfo.HasNextPage) + assert.NotEmpty(t, br.PageInfo.EndCursor) }) }