From df21536f6da7bfb01c417b09dcb46b80c94a876e Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Fri, 19 Dec 2025 14:30:08 +0100 Subject: [PATCH 1/4] get_file_contents improvements --- pkg/github/repositories.go | 48 ++++++++++++++--- pkg/github/repositories_test.go | 92 +++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 6 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index d8d2b27b3..d81fa8095 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -1833,6 +1833,20 @@ func filterPaths(entries []*github.TreeEntry, path string, maxResults int) []str return matchedPaths } +// looksLikeSHA returns true if the string appears to be a Git commit SHA. +// A SHA is a 40-character hexadecimal string. +func looksLikeSHA(s string) bool { + if len(s) != 40 { + return false + } + for _, c := range s { + if (c < '0' || c > '9') && (c < 'a' || c > 'f') && (c < 'A' || c > 'F') { + return false + } + } + return true +} + // resolveGitReference takes a user-provided ref and sha and resolves them into a // definitive commit SHA and its corresponding fully-qualified reference. // @@ -1841,8 +1855,11 @@ func filterPaths(entries []*github.TreeEntry, path string, maxResults int) []str // 1. If a specific commit `sha` is provided, it takes precedence and is used directly, // and all reference resolution is skipped. // -// 2. If no `sha` is provided, the function resolves the `ref` -// string into a fully-qualified format (e.g., "refs/heads/main") by trying +// 1a. If `sha` is empty but `ref` looks like a commit SHA (7-40 hexadecimal characters), +// it is returned as-is without any API calls or reference resolution. +// +// 2. If no `sha` is provided and `ref` does not look like a SHA, the function resolves +// the `ref` string into a fully-qualified format (e.g., "refs/heads/main") by trying // the following steps in order: // a). **Empty Ref:** If `ref` is empty, the repository's default branch is used. // b). **Fully-Qualified:** If `ref` already starts with "refs/", it's considered fully @@ -1865,6 +1882,11 @@ func resolveGitReference(ctx context.Context, githubClient *github.Client, owner return &raw.ContentOpts{Ref: "", SHA: sha}, nil } + // 1a) If sha is empty but ref looks like a SHA, return it without changes + if sha == "" && looksLikeSHA(ref) { + return &raw.ContentOpts{Ref: "", SHA: ref}, nil + } + originalRef := ref // Keep original ref for clearer error messages down the line. // 2) If no SHA is provided, we try to resolve the ref into a fully-qualified format. @@ -1905,11 +1927,25 @@ func resolveGitReference(ctx context.Context, githubClient *github.Client, owner // The tag lookup also failed. Check if it was a 404 Not Found error. ghErr2, isGhErr2 := err.(*github.ErrorResponse) if isGhErr2 && ghErr2.Response.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("could not resolve ref %q as a branch or a tag", originalRef) + switch originalRef { + case "main": + // Try "master" next. + branchRef = "refs/heads/master" + reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, branchRef) + if err == nil { + ref = branchRef // It's the "master" branch. + break + } + return nil, fmt.Errorf("attempted to resolve ref %q as 'main' but not found, and 'master' also not found", originalRef) + default: + return nil, fmt.Errorf("could not resolve ref %q as a branch or a tag", originalRef) + } + } + if err != nil { + // The tag lookup failed for a different reason. + _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (tag)", resp, err) + return nil, fmt.Errorf("failed to get reference for tag '%s': %w", originalRef, err) } - // The tag lookup failed for a different reason. - _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (tag)", resp, err) - return nil, fmt.Errorf("failed to get reference for tag '%s': %w", originalRef, err) } } else { // The branch lookup failed for a different reason. diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 9d7501f35..c0bc15728 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -2889,6 +2889,72 @@ func Test_GetReleaseByTag(t *testing.T) { } } +func Test_looksLikeSHA(t *testing.T) { + tests := []struct { + name string + input string + expected bool + }{ + { + name: "full 40-character SHA", + input: "abc123def456abc123def456abc123def456abc1", + expected: true, + }, + { + name: "too short", + input: "abc123def456abc123def45", + expected: false, + }, + { + name: "too long - 41 characters", + input: "abc123def456abc123def456abc123def456abc12", + expected: false, + }, + { + name: "contains invalid character - space", + input: "abc123def456abc123def456 bc123def456abc1", + expected: false, + }, + { + name: "contains invalid character - dash", + input: "abc123def456abc123d-f456abc123def456abc1", + expected: false, + }, + { + name: "contains invalid character - g", + input: "abc123def456gbc123def456abc123def456abc1", + expected: false, + }, + { + name: "branch name with slash", + input: "feature/branch", + expected: false, + }, + { + name: "empty string", + input: "", + expected: false, + }, + { + name: "all zeros SHA", + input: "0000000000000000000000000000000000000000", + expected: true, + }, + { + name: "all f's SHA", + input: "ffffffffffffffffffffffffffffffffffffffff", + expected: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + result := looksLikeSHA(tc.input) + assert.Equal(t, tc.expected, result) + }) + } +} + func Test_filterPaths(t *testing.T) { tests := []struct { name string @@ -3203,6 +3269,32 @@ func Test_resolveGitReference(t *testing.T) { }, expectError: false, }, + { + name: "ref looks like full SHA with empty sha parameter", + ref: "abc123def456abc123def456abc123def456abc1", + sha: "", + mockSetup: func() *http.Client { + // No API calls should be made when ref looks like SHA + return mock.NewMockedHTTPClient() + }, + expectedOutput: &raw.ContentOpts{ + SHA: "abc123def456abc123def456abc123def456abc1", + }, + expectError: false, + }, + { + name: "ref looks like short SHA with empty sha parameter", + ref: "abc123d", + sha: "", + mockSetup: func() *http.Client { + // No API calls should be made when ref looks like SHA + return mock.NewMockedHTTPClient() + }, + expectedOutput: &raw.ContentOpts{ + SHA: "abc123d", + }, + expectError: false, + }, } for _, tc := range tests { From f76807b7a1707ebbf31f78dc476111c328ff0d51 Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Fri, 19 Dec 2025 14:53:02 +0100 Subject: [PATCH 2/4] Return custom errors when main ref is supplied --- pkg/github/repositories.go | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index d81fa8095..6ba365f5c 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -1855,7 +1855,7 @@ func looksLikeSHA(s string) bool { // 1. If a specific commit `sha` is provided, it takes precedence and is used directly, // and all reference resolution is skipped. // -// 1a. If `sha` is empty but `ref` looks like a commit SHA (7-40 hexadecimal characters), +// 1a. If `sha` is empty but `ref` looks like a commit SHA (40 hexadecimal characters), // it is returned as-is without any API calls or reference resolution. // // 2. If no `sha` is provided and `ref` does not look like a SHA, the function resolves @@ -1927,25 +1927,15 @@ func resolveGitReference(ctx context.Context, githubClient *github.Client, owner // The tag lookup also failed. Check if it was a 404 Not Found error. ghErr2, isGhErr2 := err.(*github.ErrorResponse) if isGhErr2 && ghErr2.Response.StatusCode == http.StatusNotFound { - switch originalRef { - case "main": - // Try "master" next. - branchRef = "refs/heads/master" - reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, branchRef) - if err == nil { - ref = branchRef // It's the "master" branch. - break - } - return nil, fmt.Errorf("attempted to resolve ref %q as 'main' but not found, and 'master' also not found", originalRef) - default: - return nil, fmt.Errorf("could not resolve ref %q as a branch or a tag", originalRef) + if originalRef == "main" { + return nil, fmt.Errorf("could not find branch or tag 'main'. Some repositories use 'master' as the default branch name") } + return nil, fmt.Errorf("could not resolve ref %q as a branch or a tag", originalRef) } - if err != nil { - // The tag lookup failed for a different reason. - _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (tag)", resp, err) - return nil, fmt.Errorf("failed to get reference for tag '%s': %w", originalRef, err) - } + + // The tag lookup failed for a different reason. + _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get reference (tag)", resp, err) + return nil, fmt.Errorf("failed to get reference for tag '%s': %w", originalRef, err) } } else { // The branch lookup failed for a different reason. @@ -1958,6 +1948,9 @@ func resolveGitReference(ctx context.Context, githubClient *github.Client, owner if reference == nil { reference, resp, err = githubClient.Git.GetRef(ctx, owner, repo, ref) if err != nil { + if ref == "refs/heads/main" { + return nil, fmt.Errorf("could not find branch 'main'. Some repositories use 'master' as the default branch name") + } _, _ = ghErrors.NewGitHubAPIErrorToCtx(ctx, "failed to get final reference", resp, err) return nil, fmt.Errorf("failed to get final reference for %q: %w", ref, err) } From 4bdf0a4b58950bb416d213fde3bd8948c45f85e9 Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Fri, 19 Dec 2025 15:06:25 +0100 Subject: [PATCH 3/4] Remove test for short sha --- pkg/github/repositories_test.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index c0bc15728..6c56d104e 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -3282,19 +3282,6 @@ func Test_resolveGitReference(t *testing.T) { }, expectError: false, }, - { - name: "ref looks like short SHA with empty sha parameter", - ref: "abc123d", - sha: "", - mockSetup: func() *http.Client { - // No API calls should be made when ref looks like SHA - return mock.NewMockedHTTPClient() - }, - expectedOutput: &raw.ContentOpts{ - SHA: "abc123d", - }, - expectError: false, - }, } for _, tc := range tests { From 0f9061b3a71964f3fa858c4b0e4205df2b1e9c1e Mon Sep 17 00:00:00 2001 From: Ksenia Bobrova Date: Fri, 19 Dec 2025 15:10:45 +0100 Subject: [PATCH 4/4] Apply Copilot suggestion --- pkg/github/repositories.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index 6ba365f5c..407915a7c 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -1883,7 +1883,7 @@ func resolveGitReference(ctx context.Context, githubClient *github.Client, owner } // 1a) If sha is empty but ref looks like a SHA, return it without changes - if sha == "" && looksLikeSHA(ref) { + if looksLikeSHA(ref) { return &raw.ContentOpts{Ref: "", SHA: ref}, nil }