get_file_contents improvements

Author: almaleksia

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.

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 {