From 04c2323bb615a28e024e18d3a1852d1223e379df Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 13:31:03 +0000 Subject: [PATCH 1/6] Initial plan From d2734c8b7c47eb03884960124f278450066e3ae4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 13:36:42 +0000 Subject: [PATCH 2/6] Initial exploration: understand gh api usage and fallback requirements Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/ci-doctor.lock.yml | 4 ++-- .github/workflows/daily-team-status.lock.yml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index b18336e9e89..dd632bdb971 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -5,7 +5,7 @@ # # Source: githubnext/agentics/workflows/ci-doctor.md@09e77ed2e49f0612e258db12839e86e8e2a6c692 # -# Effective stop-time: 2025-11-16 13:10:44 +# Effective stop-time: 2025-11-16 13:35:36 # # Job Dependency Graph: # ```mermaid @@ -4914,7 +4914,7 @@ jobs: id: check_stop_time uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - GH_AW_STOP_TIME: 2025-11-16 13:10:44 + GH_AW_STOP_TIME: 2025-11-16 13:35:36 GH_AW_WORKFLOW_NAME: "CI Failure Doctor" with: script: | diff --git a/.github/workflows/daily-team-status.lock.yml b/.github/workflows/daily-team-status.lock.yml index 3e6cc7c83a6..ad05428a26a 100644 --- a/.github/workflows/daily-team-status.lock.yml +++ b/.github/workflows/daily-team-status.lock.yml @@ -5,7 +5,7 @@ # # Source: githubnext/agentics/workflows/daily-team-status.md@1e366aa4518cf83d25defd84e454b9a41e87cf7c # -# Effective stop-time: 2025-12-14 13:10:46 +# Effective stop-time: 2025-12-14 13:35:38 # # Job Dependency Graph: # ```mermaid @@ -4205,7 +4205,7 @@ jobs: id: check_stop_time uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - GH_AW_STOP_TIME: 2025-12-14 13:10:46 + GH_AW_STOP_TIME: 2025-12-14 13:35:38 GH_AW_WORKFLOW_NAME: "Daily Team Status" with: script: | From 23f3210bac5e8d11f08eb558ba00f00943184600 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 13:43:31 +0000 Subject: [PATCH 3/6] Add REST API fallback for gh api commands - Add ExecGHWithRESTFallback function that attempts gh CLI first, then falls back to direct REST API calls - Implement callGitHubRESTAPI for unauthenticated GitHub API requests - Implement applyJQFilter for basic jq-style field extraction - Update action_resolver.go to use REST API fallback when resolving action SHAs - Add comprehensive unit tests for jq filtering and fallback logic - Add integration tests (skip in restricted environments) - All existing tests continue to pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/ci-doctor.lock.yml | 4 +- .github/workflows/daily-team-status.lock.yml | 4 +- pkg/workflow/action_resolver.go | 8 +- pkg/workflow/gh_helper.go | 194 ++++++++++++++++ pkg/workflow/gh_helper_integration_test.go | 221 +++++++++++++++++++ pkg/workflow/gh_helper_test.go | 153 +++++++++++++ 6 files changed, 578 insertions(+), 6 deletions(-) create mode 100644 pkg/workflow/gh_helper_integration_test.go diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index dd632bdb971..30ea97d09a0 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -5,7 +5,7 @@ # # Source: githubnext/agentics/workflows/ci-doctor.md@09e77ed2e49f0612e258db12839e86e8e2a6c692 # -# Effective stop-time: 2025-11-16 13:35:36 +# Effective stop-time: 2025-11-16 13:42:48 # # Job Dependency Graph: # ```mermaid @@ -4914,7 +4914,7 @@ jobs: id: check_stop_time uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - GH_AW_STOP_TIME: 2025-11-16 13:35:36 + GH_AW_STOP_TIME: 2025-11-16 13:42:48 GH_AW_WORKFLOW_NAME: "CI Failure Doctor" with: script: | diff --git a/.github/workflows/daily-team-status.lock.yml b/.github/workflows/daily-team-status.lock.yml index ad05428a26a..fdd34101775 100644 --- a/.github/workflows/daily-team-status.lock.yml +++ b/.github/workflows/daily-team-status.lock.yml @@ -5,7 +5,7 @@ # # Source: githubnext/agentics/workflows/daily-team-status.md@1e366aa4518cf83d25defd84e454b9a41e87cf7c # -# Effective stop-time: 2025-12-14 13:35:38 +# Effective stop-time: 2025-12-14 13:42:51 # # Job Dependency Graph: # ```mermaid @@ -4205,7 +4205,7 @@ jobs: id: check_stop_time uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - GH_AW_STOP_TIME: 2025-12-14 13:35:38 + GH_AW_STOP_TIME: 2025-12-14 13:42:51 GH_AW_WORKFLOW_NAME: "Daily Team Status" with: script: | diff --git a/pkg/workflow/action_resolver.go b/pkg/workflow/action_resolver.go index b5c9d49c4b1..38a4628afe5 100644 --- a/pkg/workflow/action_resolver.go +++ b/pkg/workflow/action_resolver.go @@ -50,6 +50,7 @@ func (r *ActionResolver) ResolveSHA(repo, version string) (string, error) { } // resolveFromGitHub uses gh CLI to resolve the SHA for an action@version +// Falls back to unauthenticated REST API if gh CLI fails due to authentication func (r *ActionResolver) resolveFromGitHub(repo, version string) (string, error) { // Extract base repository (for actions like "github/codeql-action/upload-sarif") baseRepo := extractBaseRepo(repo) @@ -60,13 +61,16 @@ func (r *ActionResolver) resolveFromGitHub(repo, version string) (string, error) apiPath := fmt.Sprintf("/repos/%s/git/ref/tags/%s", baseRepo, version) resolverLog.Printf("Querying GitHub API: %s", apiPath) - cmd := ExecGH("api", apiPath, "--jq", ".object.sha") - output, err := cmd.Output() + output, fromREST, err := ExecGHWithRESTFallback("api", apiPath, "--jq", ".object.sha") if err != nil { // Try without "refs/tags/" prefix in case version is already a ref return "", fmt.Errorf("failed to resolve %s@%s: %w", repo, version, err) } + if fromREST { + resolverLog.Printf("Resolved using REST API fallback") + } + sha := strings.TrimSpace(string(output)) if sha == "" { return "", fmt.Errorf("empty SHA returned for %s@%s", repo, version) diff --git a/pkg/workflow/gh_helper.go b/pkg/workflow/gh_helper.go index 3f3421a98c4..748ec55e335 100644 --- a/pkg/workflow/gh_helper.go +++ b/pkg/workflow/gh_helper.go @@ -1,8 +1,14 @@ package workflow import ( + "encoding/json" + "fmt" + "io" + "net/http" "os" "os/exec" + "strings" + "time" "github.com/githubnext/gh-aw/pkg/logger" ) @@ -39,3 +45,191 @@ func ExecGH(args ...string) *exec.Cmd { return cmd } + +// ExecGHWithRESTFallback executes a gh CLI command with fallback to unauthenticated REST API. +// It is specifically designed for "gh api" commands. +// +// When gh CLI fails due to missing/invalid authentication, this function will attempt +// to make the same API call using direct HTTP REST API without authentication. +// +// Args: +// - args: Command arguments (e.g., "api", "/repos/owner/repo/git/ref/tags/v1.0", "--jq", ".object.sha") +// +// Returns: +// - output: The command output (either from gh CLI or REST API) +// - fromREST: true if the output came from REST API fallback, false if from gh CLI +// - err: Error if both gh CLI and REST API failed +// +// Usage: +// +// output, fromREST, err := ExecGHWithRESTFallback("api", "/repos/actions/checkout/git/ref/tags/v4", "--jq", ".object.sha") +func ExecGHWithRESTFallback(args ...string) ([]byte, bool, error) { + // First try with gh CLI + cmd := ExecGH(args...) + output, err := cmd.Output() + + if err == nil { + ghHelperLog.Printf("gh CLI succeeded") + return output, false, nil + } + + // Check if this is a gh api command that failed due to authentication + if len(args) < 2 || args[0] != "api" { + ghHelperLog.Printf("Not a gh api command or insufficient args, returning original error") + return nil, false, err + } + + // Check if error is authentication-related + exitErr, ok := err.(*exec.ExitError) + if !ok { + ghHelperLog.Printf("Not an exit error, returning original error") + return nil, false, err + } + + // Common authentication error exit codes: + // - exit status 4: authentication error in gh CLI + // - exit status 1: general error (could be auth-related) + stderr := string(exitErr.Stderr) + isAuthError := exitErr.ExitCode() == 4 || + strings.Contains(stderr, "authentication") || + strings.Contains(stderr, "HTTP 401") || + strings.Contains(stderr, "HTTP 403") || + strings.Contains(strings.ToLower(stderr), "unauthorized") + + if !isAuthError { + ghHelperLog.Printf("Error is not authentication-related, returning original error: %v", stderr) + return nil, false, err + } + + ghHelperLog.Printf("Authentication error detected, attempting REST API fallback") + + // Extract API path and jq filter + apiPath := args[1] + var jqFilter string + for i := 2; i < len(args); i++ { + if args[i] == "--jq" && i+1 < len(args) { + jqFilter = args[i+1] + break + } + } + + // Attempt REST API call + restOutput, err := callGitHubRESTAPI(apiPath, jqFilter) + if err != nil { + ghHelperLog.Printf("REST API fallback failed: %v", err) + // Return original gh CLI error since REST API also failed + return nil, false, exitErr + } + + ghHelperLog.Printf("REST API fallback succeeded") + return restOutput, true, nil +} + +// callGitHubRESTAPI makes a direct HTTP call to the GitHub REST API without authentication. +// It handles jq filtering by parsing the JSON response and extracting the specified field. +func callGitHubRESTAPI(apiPath, jqFilter string) ([]byte, error) { + // Normalize API path (remove leading slash if present) + apiPath = strings.TrimPrefix(apiPath, "/") + + // Build API URL + baseURL := "https://api.github.com" + url := fmt.Sprintf("%s/%s", baseURL, apiPath) + + ghHelperLog.Printf("Making REST API call to: %s", url) + + // Create HTTP client with timeout + client := &http.Client{ + Timeout: 30 * time.Second, + } + + // Create request + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + // Set headers + req.Header.Set("Accept", "application/vnd.github.v3+json") + req.Header.Set("User-Agent", "gh-aw") + + // Make request + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("HTTP request failed: %w", err) + } + defer resp.Body.Close() + + // Check status code + if resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return nil, fmt.Errorf("API returned status %d: %s", resp.StatusCode, string(body)) + } + + // Read response body + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response: %w", err) + } + + // If jq filter is specified, extract the field + if jqFilter != "" { + filtered, err := applyJQFilter(body, jqFilter) + if err != nil { + return nil, fmt.Errorf("failed to apply jq filter %s: %w", jqFilter, err) + } + return filtered, nil + } + + return body, nil +} + +// applyJQFilter applies a simple jq-like filter to JSON data. +// It only supports simple field extraction like ".object.sha" or ".name" +func applyJQFilter(jsonData []byte, filter string) ([]byte, error) { + // Parse filter (e.g., ".object.sha" -> ["object", "sha"]) + filter = strings.TrimPrefix(filter, ".") + fields := strings.Split(filter, ".") + + // Parse JSON + var data interface{} + if err := json.Unmarshal(jsonData, &data); err != nil { + return nil, fmt.Errorf("invalid JSON: %w", err) + } + + // Navigate through fields + current := data + for _, field := range fields { + if field == "" { + continue + } + + m, ok := current.(map[string]interface{}) + if !ok { + return nil, fmt.Errorf("cannot access field %s: not an object", field) + } + + value, exists := m[field] + if !exists { + return nil, fmt.Errorf("field %s not found", field) + } + + current = value + } + + // Convert result to string + switch v := current.(type) { + case string: + return []byte(v + "\n"), nil + case float64: + return []byte(fmt.Sprintf("%v\n", v)), nil + case bool: + return []byte(fmt.Sprintf("%v\n", v)), nil + default: + // For complex types, return JSON + result, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("failed to marshal result: %w", err) + } + return append(result, '\n'), nil + } +} diff --git a/pkg/workflow/gh_helper_integration_test.go b/pkg/workflow/gh_helper_integration_test.go new file mode 100644 index 00000000000..eee46fd71d6 --- /dev/null +++ b/pkg/workflow/gh_helper_integration_test.go @@ -0,0 +1,221 @@ +// go:build integration +//go:build integration + +package workflow + +import ( + "os" + "strings" + "testing" +) + +func TestExecGHWithRESTFallback_RealAPI(t *testing.T) { + // Save original environment + originalGHToken := os.Getenv("GH_TOKEN") + originalGitHubToken := os.Getenv("GITHUB_TOKEN") + defer func() { + os.Setenv("GH_TOKEN", originalGHToken) + os.Setenv("GITHUB_TOKEN", originalGitHubToken) + }() + + // Clear tokens to force REST API fallback + os.Unsetenv("GH_TOKEN") + os.Unsetenv("GITHUB_TOKEN") + + t.Run("fallback to REST API for public repository tag", func(t *testing.T) { + // Test with a known public repository and tag + output, fromREST, err := ExecGHWithRESTFallback( + "api", + "/repos/actions/checkout/git/ref/tags/v4", + "--jq", ".object.sha", + ) + + if err != nil { + t.Fatalf("Expected fallback to succeed, got error: %v", err) + } + + if !fromREST { + t.Logf("gh CLI succeeded (gh is configured), but we expected REST fallback") + // This is OK - if gh CLI is configured, it will succeed before trying REST + // The important thing is that the command succeeded + } + + // Verify we got a valid SHA (40 hex characters) + sha := strings.TrimSpace(string(output)) + if len(sha) != 40 { + t.Errorf("Expected 40 character SHA, got: %s (length %d)", sha, len(sha)) + } + + // Verify it's all hex characters + for _, c := range sha { + if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')) { + t.Errorf("Expected hex SHA, got character %c in: %s", c, sha) + break + } + } + + t.Logf("Successfully resolved SHA: %s (fromREST: %v)", sha, fromREST) + }) + + t.Run("fallback handles nested field extraction", func(t *testing.T) { + // Test with a repository info endpoint + output, fromREST, err := ExecGHWithRESTFallback( + "api", + "/repos/actions/checkout", + "--jq", ".name", + ) + + if err != nil { + t.Fatalf("Expected fallback to succeed, got error: %v", err) + } + + name := strings.TrimSpace(string(output)) + if name != "checkout" { + t.Errorf("Expected repository name 'checkout', got: %s", name) + } + + t.Logf("Successfully extracted name: %s (fromREST: %v)", name, fromREST) + }) + + t.Run("fallback returns error for non-existent repository", func(t *testing.T) { + output, fromREST, err := ExecGHWithRESTFallback( + "api", + "/repos/nonexistent-owner-12345/nonexistent-repo-67890/git/ref/tags/v1.0", + "--jq", ".object.sha", + ) + + // This should fail (either gh CLI or REST API) + if err == nil { + t.Errorf("Expected error for non-existent repository, but got output: %s", string(output)) + } + + t.Logf("Correctly failed for non-existent repository (fromREST: %v): %v", fromREST, err) + }) +} + +func TestActionResolver_WithRESTFallback(t *testing.T) { + // Save original environment + originalGHToken := os.Getenv("GH_TOKEN") + originalGitHubToken := os.Getenv("GITHUB_TOKEN") + defer func() { + os.Setenv("GH_TOKEN", originalGHToken) + os.Setenv("GITHUB_TOKEN", originalGitHubToken) + }() + + // Clear tokens to force REST API fallback + os.Unsetenv("GH_TOKEN") + os.Unsetenv("GITHUB_TOKEN") + + // Create a temporary directory for cache + tmpDir := t.TempDir() + cache := NewActionCache(tmpDir) + resolver := NewActionResolver(cache) + + t.Run("resolve action SHA using REST API fallback", func(t *testing.T) { + // Test resolving a real action + sha, err := resolver.ResolveSHA("actions/checkout", "v4") + + if err != nil { + t.Fatalf("Expected resolution to succeed via REST API fallback, got error: %v", err) + } + + // Verify we got a valid SHA + if len(sha) != 40 { + t.Errorf("Expected 40 character SHA, got: %s (length %d)", sha, len(sha)) + } + + t.Logf("Successfully resolved actions/checkout@v4 to SHA: %s", sha) + + // Verify caching works + cachedSHA, found := cache.Get("actions/checkout", "v4") + if !found { + t.Errorf("Expected SHA to be cached after resolution") + } + if cachedSHA != sha { + t.Errorf("Cached SHA %s doesn't match resolved SHA %s", cachedSHA, sha) + } + }) + + t.Run("resolve complex action path", func(t *testing.T) { + // Test with a complex action path (has subdirectory) + sha, err := resolver.ResolveSHA("github/codeql-action/upload-sarif", "v3") + + if err != nil { + t.Fatalf("Expected resolution to succeed, got error: %v", err) + } + + if len(sha) != 40 { + t.Errorf("Expected 40 character SHA, got: %s (length %d)", sha, len(sha)) + } + + t.Logf("Successfully resolved github/codeql-action/upload-sarif@v3 to SHA: %s", sha) + }) +} + +func TestCallGitHubRESTAPI_RealEndpoint(t *testing.T) { + t.Run("fetch repository info without authentication", func(t *testing.T) { + // Test direct REST API call to a public repository + output, err := callGitHubRESTAPI("/repos/actions/checkout", "") + + if err != nil { + t.Fatalf("Expected REST API call to succeed, got error: %v", err) + } + + // Verify we got JSON response + if len(output) == 0 { + t.Errorf("Expected non-empty response") + } + + // The response should contain repository information + if !strings.Contains(string(output), "checkout") { + t.Errorf("Expected response to contain 'checkout', got: %s", string(output[:100])) + } + + t.Logf("Successfully fetched repository info (%d bytes)", len(output)) + }) + + t.Run("fetch with jq filter", func(t *testing.T) { + output, err := callGitHubRESTAPI("/repos/actions/checkout", ".name") + + if err != nil { + t.Fatalf("Expected REST API call to succeed, got error: %v", err) + } + + name := strings.TrimSpace(string(output)) + if name != "checkout" { + t.Errorf("Expected 'checkout', got: %s", name) + } + + t.Logf("Successfully extracted name: %s", name) + }) + + t.Run("fetch nested field with jq filter", func(t *testing.T) { + output, err := callGitHubRESTAPI("/repos/actions/checkout/git/ref/tags/v4", ".object.sha") + + if err != nil { + t.Fatalf("Expected REST API call to succeed, got error: %v", err) + } + + sha := strings.TrimSpace(string(output)) + if len(sha) != 40 { + t.Errorf("Expected 40 character SHA, got: %s (length %d)", sha, len(sha)) + } + + t.Logf("Successfully extracted SHA: %s", sha) + }) + + t.Run("handle 404 error gracefully", func(t *testing.T) { + _, err := callGitHubRESTAPI("/repos/nonexistent-owner/nonexistent-repo", "") + + if err == nil { + t.Errorf("Expected error for non-existent repository") + } + + // Verify error message contains status code + if !strings.Contains(err.Error(), "404") { + t.Errorf("Expected error to mention 404, got: %v", err) + } + + t.Logf("Correctly handled 404 error: %v", err) + }) +} diff --git a/pkg/workflow/gh_helper_test.go b/pkg/workflow/gh_helper_test.go index 7a59fe8babe..544b5697f6b 100644 --- a/pkg/workflow/gh_helper_test.go +++ b/pkg/workflow/gh_helper_test.go @@ -147,3 +147,156 @@ func TestExecGHWithMultipleArgs(t *testing.T) { t.Errorf("Expected environment to contain GH_TOKEN=test-token") } } + +func TestApplyJQFilter(t *testing.T) { + tests := []struct { + name string + jsonData string + filter string + expected string + expectError bool + }{ + { + name: "simple field extraction", + jsonData: `{"name": "test"}`, + filter: ".name", + expected: "test\n", + }, + { + name: "nested field extraction", + jsonData: `{"object": {"sha": "abc123"}}`, + filter: ".object.sha", + expected: "abc123\n", + }, + { + name: "number field", + jsonData: `{"count": 42}`, + filter: ".count", + expected: "42\n", + }, + { + name: "boolean field", + jsonData: `{"enabled": true}`, + filter: ".enabled", + expected: "true\n", + }, + { + name: "missing field", + jsonData: `{"name": "test"}`, + filter: ".missing", + expectError: true, + }, + { + name: "invalid JSON", + jsonData: `{invalid}`, + filter: ".name", + expectError: true, + }, + { + name: "complex object", + jsonData: `{"meta": {"nested": {"value": "deep"}}}`, + filter: ".meta.nested.value", + expected: "deep\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result, err := applyJQFilter([]byte(tt.jsonData), tt.filter) + + if tt.expectError { + if err == nil { + t.Errorf("Expected error but got none") + } + return + } + + if err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + + if string(result) != tt.expected { + t.Errorf("Expected %q, got %q", tt.expected, string(result)) + } + }) + } +} + +func TestCallGitHubRESTAPI(t *testing.T) { + // This is a unit test that verifies the function exists and has the right signature + // Integration tests would require network access + + t.Run("function exists and accepts correct parameters", func(t *testing.T) { + // Test that the function can be called (will fail due to network, but that's expected) + _, err := callGitHubRESTAPI("/repos/actions/checkout/git/ref/tags/v4", ".object.sha") + + // We expect an error (network error in test environment), but we're just verifying + // the function signature is correct + if err == nil { + // If somehow this succeeds (network is available), that's fine too + t.Log("REST API call succeeded (network available)") + } else { + // Expected: network error or API error + t.Logf("Expected network error in test environment: %v", err) + } + }) +} + +func TestExecGHWithRESTFallback_NonAPICommand(t *testing.T) { + // Save original environment + originalGHToken := os.Getenv("GH_TOKEN") + originalGitHubToken := os.Getenv("GITHUB_TOKEN") + defer func() { + os.Setenv("GH_TOKEN", originalGHToken) + os.Setenv("GITHUB_TOKEN", originalGitHubToken) + }() + + // Clear tokens to ensure gh CLI will fail + os.Unsetenv("GH_TOKEN") + os.Unsetenv("GITHUB_TOKEN") + + t.Run("non-api command should not trigger REST fallback", func(t *testing.T) { + // This should fail with the original error, not attempt REST fallback + _, fromREST, err := ExecGHWithRESTFallback("repo", "view") + + if err == nil { + t.Logf("Command succeeded (gh is configured), skipping fallback test") + return + } + + // Should not have used REST fallback for non-api commands + if fromREST { + t.Errorf("Expected fromREST to be false for non-api commands, got true") + } + }) +} + +func TestExecGHWithRESTFallback_InsufficientArgs(t *testing.T) { + // Save original environment + originalGHToken := os.Getenv("GH_TOKEN") + originalGitHubToken := os.Getenv("GITHUB_TOKEN") + defer func() { + os.Setenv("GH_TOKEN", originalGHToken) + os.Setenv("GITHUB_TOKEN", originalGitHubToken) + }() + + // Clear tokens to ensure gh CLI will fail + os.Unsetenv("GH_TOKEN") + os.Unsetenv("GITHUB_TOKEN") + + t.Run("api command with insufficient args should not trigger REST fallback", func(t *testing.T) { + // This should fail with the original error + _, fromREST, err := ExecGHWithRESTFallback("api") + + if err == nil { + t.Logf("Command succeeded unexpectedly") + return + } + + // Should not have used REST fallback + if fromREST { + t.Errorf("Expected fromREST to be false for insufficient args, got true") + } + }) +} From 691f0872b76f9f2e8fede80fd5824336cd640385 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 13:49:08 +0000 Subject: [PATCH 4/6] Fix REST API fallback to check combined output for auth errors - Change cmd.Output() to cmd.CombinedOutput() to capture gh CLI messages - Check both combined output and stderr for authentication error indicators - Add check for "GH_TOKEN" string in output (gh CLI's auth error message) - Fix error return to use exitErr instead of err for consistency - All unit tests pass Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/ci-doctor.lock.yml | 4 ++-- .github/workflows/daily-team-status.lock.yml | 4 ++-- pkg/workflow/gh_helper.go | 13 ++++++++++--- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index 30ea97d09a0..c2504a29096 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -5,7 +5,7 @@ # # Source: githubnext/agentics/workflows/ci-doctor.md@09e77ed2e49f0612e258db12839e86e8e2a6c692 # -# Effective stop-time: 2025-11-16 13:42:48 +# Effective stop-time: 2025-11-16 13:48:24 # # Job Dependency Graph: # ```mermaid @@ -4914,7 +4914,7 @@ jobs: id: check_stop_time uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - GH_AW_STOP_TIME: 2025-11-16 13:42:48 + GH_AW_STOP_TIME: 2025-11-16 13:48:24 GH_AW_WORKFLOW_NAME: "CI Failure Doctor" with: script: | diff --git a/.github/workflows/daily-team-status.lock.yml b/.github/workflows/daily-team-status.lock.yml index fdd34101775..2d028e14f85 100644 --- a/.github/workflows/daily-team-status.lock.yml +++ b/.github/workflows/daily-team-status.lock.yml @@ -5,7 +5,7 @@ # # Source: githubnext/agentics/workflows/daily-team-status.md@1e366aa4518cf83d25defd84e454b9a41e87cf7c # -# Effective stop-time: 2025-12-14 13:42:51 +# Effective stop-time: 2025-12-14 13:48:26 # # Job Dependency Graph: # ```mermaid @@ -4205,7 +4205,7 @@ jobs: id: check_stop_time uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: - GH_AW_STOP_TIME: 2025-12-14 13:42:51 + GH_AW_STOP_TIME: 2025-12-14 13:48:26 GH_AW_WORKFLOW_NAME: "Daily Team Status" with: script: | diff --git a/pkg/workflow/gh_helper.go b/pkg/workflow/gh_helper.go index 748ec55e335..03ea717b992 100644 --- a/pkg/workflow/gh_helper.go +++ b/pkg/workflow/gh_helper.go @@ -66,7 +66,7 @@ func ExecGH(args ...string) *exec.Cmd { func ExecGHWithRESTFallback(args ...string) ([]byte, bool, error) { // First try with gh CLI cmd := ExecGH(args...) - output, err := cmd.Output() + output, err := cmd.CombinedOutput() if err == nil { ghHelperLog.Printf("gh CLI succeeded") @@ -89,16 +89,23 @@ func ExecGHWithRESTFallback(args ...string) ([]byte, bool, error) { // Common authentication error exit codes: // - exit status 4: authentication error in gh CLI // - exit status 1: general error (could be auth-related) + // Check both combined output and stderr for error messages + combinedOutput := string(output) stderr := string(exitErr.Stderr) isAuthError := exitErr.ExitCode() == 4 || + strings.Contains(combinedOutput, "GH_TOKEN") || + strings.Contains(combinedOutput, "authentication") || strings.Contains(stderr, "authentication") || + strings.Contains(combinedOutput, "HTTP 401") || strings.Contains(stderr, "HTTP 401") || + strings.Contains(combinedOutput, "HTTP 403") || strings.Contains(stderr, "HTTP 403") || + strings.Contains(strings.ToLower(combinedOutput), "unauthorized") || strings.Contains(strings.ToLower(stderr), "unauthorized") if !isAuthError { - ghHelperLog.Printf("Error is not authentication-related, returning original error: %v", stderr) - return nil, false, err + ghHelperLog.Printf("Error is not authentication-related, returning original error: %v", combinedOutput) + return nil, false, exitErr } ghHelperLog.Printf("Authentication error detected, attempting REST API fallback") From 14bee46a88f6ef46280f945ce9ed98ca0fc725c3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 15:47:02 +0000 Subject: [PATCH 5/6] Fix test: update expected action pins count to 20 After merge with main, action_pins.json now contains 20 entries instead of 18. Updated TestGetActionPinsSorting to reflect the current count. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/super-linter.lock.yml | 2 +- pkg/workflow/action_pins_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/super-linter.lock.yml b/.github/workflows/super-linter.lock.yml index 255d1dc1430..f9d8f5faec9 100644 --- a/.github/workflows/super-linter.lock.yml +++ b/.github/workflows/super-linter.lock.yml @@ -4569,7 +4569,7 @@ jobs: fi - name: Upload super-linter log if: always() - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5 with: name: super-linter-log path: super-linter.log diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index 7af2d7a7377..4d14564dc15 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -345,9 +345,9 @@ func TestApplyActionPinToStep(t *testing.T) { func TestGetActionPinsSorting(t *testing.T) { pins := getActionPins() - // Verify we got all the pins (should be 18) - if len(pins) != 18 { - t.Errorf("getActionPins() returned %d pins, expected 18", len(pins)) + // Verify we got all the pins (should be 20) + if len(pins) != 20 { + t.Errorf("getActionPins() returned %d pins, expected 20", len(pins)) } // Verify they are sorted by version (descending) then by repository name (ascending) From 6d148cea8a9cb4ad321ce14bcf75e4cfda7b3b12 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 14 Nov 2025 16:44:09 +0000 Subject: [PATCH 6/6] Specialize ExecGHWithRESTFallback for gh api commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renamed to ExecGHAPIWithRESTFallback with specialized signature: - Removed generic args parameter - Now requires apiPath and jqFilter as explicit parameters - Function signature makes it clear this is only for API calls - Updated all usages in action_resolver.go and tests - Removed obsolete tests for non-API commands - All unit tests pass ✅ Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/action_resolver.go | 2 +- pkg/workflow/gh_helper.go | 35 +++++++---------- pkg/workflow/gh_helper_integration_test.go | 17 ++++---- pkg/workflow/gh_helper_test.go | 45 +++++++--------------- 4 files changed, 35 insertions(+), 64 deletions(-) diff --git a/pkg/workflow/action_resolver.go b/pkg/workflow/action_resolver.go index 38a4628afe5..89d88ed262d 100644 --- a/pkg/workflow/action_resolver.go +++ b/pkg/workflow/action_resolver.go @@ -61,7 +61,7 @@ func (r *ActionResolver) resolveFromGitHub(repo, version string) (string, error) apiPath := fmt.Sprintf("/repos/%s/git/ref/tags/%s", baseRepo, version) resolverLog.Printf("Querying GitHub API: %s", apiPath) - output, fromREST, err := ExecGHWithRESTFallback("api", apiPath, "--jq", ".object.sha") + output, fromREST, err := ExecGHAPIWithRESTFallback(apiPath, ".object.sha") if err != nil { // Try without "refs/tags/" prefix in case version is already a ref return "", fmt.Errorf("failed to resolve %s@%s: %w", repo, version, err) diff --git a/pkg/workflow/gh_helper.go b/pkg/workflow/gh_helper.go index 03ea717b992..1678a283522 100644 --- a/pkg/workflow/gh_helper.go +++ b/pkg/workflow/gh_helper.go @@ -46,14 +46,15 @@ func ExecGH(args ...string) *exec.Cmd { return cmd } -// ExecGHWithRESTFallback executes a gh CLI command with fallback to unauthenticated REST API. -// It is specifically designed for "gh api" commands. +// ExecGHAPIWithRESTFallback executes a "gh api" command with fallback to unauthenticated REST API. +// This function is specialized for GitHub API calls only. // // When gh CLI fails due to missing/invalid authentication, this function will attempt // to make the same API call using direct HTTP REST API without authentication. // // Args: -// - args: Command arguments (e.g., "api", "/repos/owner/repo/git/ref/tags/v1.0", "--jq", ".object.sha") +// - apiPath: GitHub API path (e.g., "/repos/owner/repo/git/ref/tags/v1.0") +// - jqFilter: Optional jq filter for JSON extraction (e.g., ".object.sha"), empty string if not needed // // Returns: // - output: The command output (either from gh CLI or REST API) @@ -62,23 +63,23 @@ func ExecGH(args ...string) *exec.Cmd { // // Usage: // -// output, fromREST, err := ExecGHWithRESTFallback("api", "/repos/actions/checkout/git/ref/tags/v4", "--jq", ".object.sha") -func ExecGHWithRESTFallback(args ...string) ([]byte, bool, error) { +// output, fromREST, err := ExecGHAPIWithRESTFallback("/repos/actions/checkout/git/ref/tags/v4", ".object.sha") +func ExecGHAPIWithRESTFallback(apiPath, jqFilter string) ([]byte, bool, error) { + // Build gh api command arguments + args := []string{"api", apiPath} + if jqFilter != "" { + args = append(args, "--jq", jqFilter) + } + // First try with gh CLI cmd := ExecGH(args...) output, err := cmd.CombinedOutput() if err == nil { - ghHelperLog.Printf("gh CLI succeeded") + ghHelperLog.Printf("gh api succeeded") return output, false, nil } - // Check if this is a gh api command that failed due to authentication - if len(args) < 2 || args[0] != "api" { - ghHelperLog.Printf("Not a gh api command or insufficient args, returning original error") - return nil, false, err - } - // Check if error is authentication-related exitErr, ok := err.(*exec.ExitError) if !ok { @@ -110,16 +111,6 @@ func ExecGHWithRESTFallback(args ...string) ([]byte, bool, error) { ghHelperLog.Printf("Authentication error detected, attempting REST API fallback") - // Extract API path and jq filter - apiPath := args[1] - var jqFilter string - for i := 2; i < len(args); i++ { - if args[i] == "--jq" && i+1 < len(args) { - jqFilter = args[i+1] - break - } - } - // Attempt REST API call restOutput, err := callGitHubRESTAPI(apiPath, jqFilter) if err != nil { diff --git a/pkg/workflow/gh_helper_integration_test.go b/pkg/workflow/gh_helper_integration_test.go index eee46fd71d6..f4813099f40 100644 --- a/pkg/workflow/gh_helper_integration_test.go +++ b/pkg/workflow/gh_helper_integration_test.go @@ -9,7 +9,7 @@ import ( "testing" ) -func TestExecGHWithRESTFallback_RealAPI(t *testing.T) { +func TestExecGHAPIWithRESTFallback_RealAPI(t *testing.T) { // Save original environment originalGHToken := os.Getenv("GH_TOKEN") originalGitHubToken := os.Getenv("GITHUB_TOKEN") @@ -24,10 +24,9 @@ func TestExecGHWithRESTFallback_RealAPI(t *testing.T) { t.Run("fallback to REST API for public repository tag", func(t *testing.T) { // Test with a known public repository and tag - output, fromREST, err := ExecGHWithRESTFallback( - "api", + output, fromREST, err := ExecGHAPIWithRESTFallback( "/repos/actions/checkout/git/ref/tags/v4", - "--jq", ".object.sha", + ".object.sha", ) if err != nil { @@ -59,10 +58,9 @@ func TestExecGHWithRESTFallback_RealAPI(t *testing.T) { t.Run("fallback handles nested field extraction", func(t *testing.T) { // Test with a repository info endpoint - output, fromREST, err := ExecGHWithRESTFallback( - "api", + output, fromREST, err := ExecGHAPIWithRESTFallback( "/repos/actions/checkout", - "--jq", ".name", + ".name", ) if err != nil { @@ -78,10 +76,9 @@ func TestExecGHWithRESTFallback_RealAPI(t *testing.T) { }) t.Run("fallback returns error for non-existent repository", func(t *testing.T) { - output, fromREST, err := ExecGHWithRESTFallback( - "api", + output, fromREST, err := ExecGHAPIWithRESTFallback( "/repos/nonexistent-owner-12345/nonexistent-repo-67890/git/ref/tags/v1.0", - "--jq", ".object.sha", + ".object.sha", ) // This should fail (either gh CLI or REST API) diff --git a/pkg/workflow/gh_helper_test.go b/pkg/workflow/gh_helper_test.go index 544b5697f6b..1c4e842f243 100644 --- a/pkg/workflow/gh_helper_test.go +++ b/pkg/workflow/gh_helper_test.go @@ -243,7 +243,7 @@ func TestCallGitHubRESTAPI(t *testing.T) { }) } -func TestExecGHWithRESTFallback_NonAPICommand(t *testing.T) { +func TestExecGHAPIWithRESTFallback_BasicFunctionality(t *testing.T) { // Save original environment originalGHToken := os.Getenv("GH_TOKEN") originalGitHubToken := os.Getenv("GITHUB_TOKEN") @@ -256,47 +256,30 @@ func TestExecGHWithRESTFallback_NonAPICommand(t *testing.T) { os.Unsetenv("GH_TOKEN") os.Unsetenv("GITHUB_TOKEN") - t.Run("non-api command should not trigger REST fallback", func(t *testing.T) { - // This should fail with the original error, not attempt REST fallback - _, fromREST, err := ExecGHWithRESTFallback("repo", "view") + t.Run("function accepts API path and jq filter", func(t *testing.T) { + // The function should accept the proper parameters + // Network will likely fail in test environment, but we're testing the signature + _, fromREST, err := ExecGHAPIWithRESTFallback("/repos/actions/checkout", ".name") if err == nil { - t.Logf("Command succeeded (gh is configured), skipping fallback test") + t.Logf("Command succeeded (gh is configured or network is available)") return } - // Should not have used REST fallback for non-api commands - if fromREST { - t.Errorf("Expected fromREST to be false for non-api commands, got true") - } + // We expect either gh CLI error or REST API error, but the function call should work + t.Logf("Expected error in test environment (fromREST: %v): %v", fromREST, err) }) -} -func TestExecGHWithRESTFallback_InsufficientArgs(t *testing.T) { - // Save original environment - originalGHToken := os.Getenv("GH_TOKEN") - originalGitHubToken := os.Getenv("GITHUB_TOKEN") - defer func() { - os.Setenv("GH_TOKEN", originalGHToken) - os.Setenv("GITHUB_TOKEN", originalGitHubToken) - }() - - // Clear tokens to ensure gh CLI will fail - os.Unsetenv("GH_TOKEN") - os.Unsetenv("GITHUB_TOKEN") - - t.Run("api command with insufficient args should not trigger REST fallback", func(t *testing.T) { - // This should fail with the original error - _, fromREST, err := ExecGHWithRESTFallback("api") + t.Run("function works without jq filter", func(t *testing.T) { + // Test with empty jq filter + _, fromREST, err := ExecGHAPIWithRESTFallback("/repos/actions/checkout", "") if err == nil { - t.Logf("Command succeeded unexpectedly") + t.Logf("Command succeeded (gh is configured or network is available)") return } - // Should not have used REST fallback - if fromREST { - t.Errorf("Expected fromREST to be false for insufficient args, got true") - } + // We expect either gh CLI error or REST API error + t.Logf("Expected error in test environment (fromREST: %v): %v", fromREST, err) }) }