Skip to content

[WIP] Add helper to wrap 'gh' calls using Exec #3952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 14, 2025
Merged

[WIP] Add helper to wrap 'gh' calls using Exec #3952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97a050e
Initial plan
Copilot Nov 14, 2025
5aa2f9d
Initial exploration of gh CLI token handling
Copilot Nov 14, 2025
9ed699d
Add ExecGH helper function with GITHUB_TOKEN fallback
Copilot Nov 14, 2025
a9a47fa
Verify implementation and final testing
Copilot Nov 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/ci-doctor.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions .github/workflows/daily-team-status.lock.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/cli/.github/aw/actions-lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"entries": {}
}
Comment on lines +1 to +3
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This appears to be a new empty actions lock file that was auto-generated. It's unclear why this file is being created in the pkg/cli/.github/aw/ directory.

Questions:

  • Is this file expected to be in the pkg/cli/ directory, or should it be at the repository root?
  • Is this related to the gh helper implementation, or an unrelated side effect?

Recommendation: If this file is unrelated to the PR's purpose, consider excluding it from this PR and investigating why it was generated.

Suggested change
{
"entries": {}
}

Copilot uses AI. Check for mistakes.
3 changes: 1 addition & 2 deletions pkg/workflow/action_resolver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package workflow

import (
"fmt"
"os/exec"
"strings"

"github.com/githubnext/gh-aw/pkg/logger"
Expand Down Expand Up @@ -61,7 +60,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)

cmd := exec.Command("gh", "api", apiPath, "--jq", ".object.sha")
cmd := ExecGH("api", apiPath, "--jq", ".object.sha")
output, err := cmd.Output()
if err != nil {
// Try without "refs/tags/" prefix in case version is already a ref
Expand Down
41 changes: 41 additions & 0 deletions pkg/workflow/gh_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package workflow

import (
"os"
"os/exec"

"github.com/githubnext/gh-aw/pkg/logger"
)

var ghHelperLog = logger.New("workflow:gh_helper")

// ExecGH wraps exec.Command for "gh" CLI calls and ensures proper token configuration.
// It sets GH_TOKEN from GITHUB_TOKEN if GH_TOKEN is not already set.
// This ensures gh CLI commands work in environments where GITHUB_TOKEN is set but GH_TOKEN is not.
//
// Usage:
//
// cmd := ExecGH("api", "/user")
// output, err := cmd.Output()
func ExecGH(args ...string) *exec.Cmd {
cmd := exec.Command("gh", args...)

// Check if GH_TOKEN is already set
ghToken := os.Getenv("GH_TOKEN")
if ghToken != "" {
ghHelperLog.Printf("GH_TOKEN is set, using it for gh CLI")
return cmd
}

// Fall back to GITHUB_TOKEN if available
githubToken := os.Getenv("GITHUB_TOKEN")
if githubToken != "" {
ghHelperLog.Printf("GH_TOKEN not set, using GITHUB_TOKEN as fallback for gh CLI")
// Set GH_TOKEN in the command's environment
cmd.Env = append(os.Environ(), "GH_TOKEN="+githubToken)
} else {
ghHelperLog.Printf("Neither GH_TOKEN nor GITHUB_TOKEN is set, gh CLI will use default authentication")
}

Comment on lines +23 to +39
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token precedence order is inconsistent with the existing GetGitHubToken() function in pkg/parser/github.go.

Current implementation (this PR):

  1. GH_TOKEN
  2. GITHUB_TOKEN
  3. Default gh CLI auth

Existing pattern in pkg/parser/github.go:

  1. GITHUB_TOKEN
  2. GH_TOKEN
  3. gh auth token command

The PR description states "make sure it uses GITHUB_TOKEN if GH_TOKEN is not available" and mentions following existing code patterns. However, this implementation prioritizes GH_TOKEN first, which contradicts the stated goal.

Recommendation: Either align with the existing GetGitHubToken() precedence (prefer GITHUB_TOKEN first), or document why this helper should use a different order. The inconsistency could lead to confusion and unexpected behavior across the codebase.

Suggested change
// Check if GH_TOKEN is already set
ghToken := os.Getenv("GH_TOKEN")
if ghToken != "" {
ghHelperLog.Printf("GH_TOKEN is set, using it for gh CLI")
return cmd
}
// Fall back to GITHUB_TOKEN if available
githubToken := os.Getenv("GITHUB_TOKEN")
if githubToken != "" {
ghHelperLog.Printf("GH_TOKEN not set, using GITHUB_TOKEN as fallback for gh CLI")
// Set GH_TOKEN in the command's environment
cmd.Env = append(os.Environ(), "GH_TOKEN="+githubToken)
} else {
ghHelperLog.Printf("Neither GH_TOKEN nor GITHUB_TOKEN is set, gh CLI will use default authentication")
}
// Prefer GITHUB_TOKEN over GH_TOKEN for consistency with GetGitHubToken()
githubToken := os.Getenv("GITHUB_TOKEN")
if githubToken != "" {
ghHelperLog.Printf("GITHUB_TOKEN is set, using it for gh CLI (sets GH_TOKEN in env)")
cmd.Env = append(os.Environ(), "GH_TOKEN="+githubToken)
return cmd
}
// Fall back to GH_TOKEN if available
ghToken := os.Getenv("GH_TOKEN")
if ghToken != "" {
ghHelperLog.Printf("GITHUB_TOKEN not set, using GH_TOKEN for gh CLI")
// No need to set env, inherited from parent
return cmd
}
ghHelperLog.Printf("Neither GITHUB_TOKEN nor GH_TOKEN is set, gh CLI will use default authentication")

Copilot uses AI. Check for mistakes.
return cmd
}
149 changes: 149 additions & 0 deletions pkg/workflow/gh_helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
package workflow

import (
"os"
"strings"
"testing"
)

func TestExecGH(t *testing.T) {
tests := []struct {
name string
ghToken string
githubToken string
expectGHToken bool
expectValue string
}{
{
name: "GH_TOKEN is set",
ghToken: "gh-token-123",
githubToken: "",
expectGHToken: false, // Should use existing GH_TOKEN from environment
expectValue: "",
},
{
name: "GITHUB_TOKEN is set, GH_TOKEN is not",
ghToken: "",
githubToken: "github-token-456",
expectGHToken: true,
expectValue: "github-token-456",
},
{
name: "Both GH_TOKEN and GITHUB_TOKEN are set",
ghToken: "gh-token-123",
githubToken: "github-token-456",
expectGHToken: false, // Should prefer existing GH_TOKEN
expectValue: "",
},
{
name: "Neither GH_TOKEN nor GITHUB_TOKEN is set",
ghToken: "",
githubToken: "",
expectGHToken: false,
expectValue: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(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)
Comment on lines +50 to +54
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment restoration logic has a bug. If the original environment variables were unset (empty string returned by os.Getenv), calling os.Setenv with an empty string will set them to an empty value instead of unsetting them.

Issue:

originalGHToken := os.Getenv("GH_TOKEN")  // Returns "" if unset
defer func() {
    os.Setenv("GH_TOKEN", originalGHToken)  // Sets to "" instead of unsetting
}()

Fix:

originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN")
originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN")
defer func() {
    if hadGHToken {
        os.Setenv("GH_TOKEN", originalGHToken)
    } else {
        os.Unsetenv("GH_TOKEN")
    }
    if hadGitHubToken {
        os.Setenv("GITHUB_TOKEN", originalGitHubToken)
    } else {
        os.Unsetenv("GITHUB_TOKEN")
    }
}()

This ensures proper cleanup regardless of the original environment state.

Suggested change
originalGHToken := os.Getenv("GH_TOKEN")
originalGitHubToken := os.Getenv("GITHUB_TOKEN")
defer func() {
os.Setenv("GH_TOKEN", originalGHToken)
os.Setenv("GITHUB_TOKEN", originalGitHubToken)
originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN")
originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN")
defer func() {
if hadGHToken {
os.Setenv("GH_TOKEN", originalGHToken)
} else {
os.Unsetenv("GH_TOKEN")
}
if hadGitHubToken {
os.Setenv("GITHUB_TOKEN", originalGitHubToken)
} else {
os.Unsetenv("GITHUB_TOKEN")
}

Copilot uses AI. Check for mistakes.
}()

// Set up test environment
if tt.ghToken != "" {
os.Setenv("GH_TOKEN", tt.ghToken)
} else {
os.Unsetenv("GH_TOKEN")
}
if tt.githubToken != "" {
os.Setenv("GITHUB_TOKEN", tt.githubToken)
} else {
os.Unsetenv("GITHUB_TOKEN")
}

// Execute the helper
cmd := ExecGH("api", "/user")

// Verify the command
if cmd.Path != "gh" && !strings.HasSuffix(cmd.Path, "/gh") {
t.Errorf("Expected command path to be 'gh', got: %s", cmd.Path)
}

// Verify arguments
if len(cmd.Args) != 3 || cmd.Args[1] != "api" || cmd.Args[2] != "/user" {
t.Errorf("Expected args [gh api /user], got: %v", cmd.Args)
}

// Verify environment
if tt.expectGHToken {
found := false
expectedEnv := "GH_TOKEN=" + tt.expectValue
for _, env := range cmd.Env {
if env == expectedEnv {
found = true
break
}
}
if !found {
t.Errorf("Expected environment to contain %s, but it wasn't found", expectedEnv)
}
} else {
// When GH_TOKEN is already set or neither token is set, cmd.Env should be nil (uses parent process env)
if cmd.Env != nil {
t.Errorf("Expected cmd.Env to be nil (inherit parent environment), got: %v", cmd.Env)
}
}
})
}
}

func TestExecGHWithMultipleArgs(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)
Comment on lines +107 to +111
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment restoration logic has a bug. If the original environment variables were unset (empty string returned by os.Getenv), calling os.Setenv with an empty string will set them to an empty value instead of unsetting them.

Issue:

originalGHToken := os.Getenv("GH_TOKEN")  // Returns "" if unset
defer func() {
    os.Setenv("GH_TOKEN", originalGHToken)  // Sets to "" instead of unsetting
}()

Fix:

originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN")
originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN")
defer func() {
    if hadGHToken {
        os.Setenv("GH_TOKEN", originalGHToken)
    } else {
        os.Unsetenv("GH_TOKEN")
    }
    if hadGitHubToken {
        os.Setenv("GITHUB_TOKEN", originalGitHubToken)
    } else {
        os.Unsetenv("GITHUB_TOKEN")
    }
}()

This ensures proper cleanup regardless of the original environment state.

Suggested change
originalGHToken := os.Getenv("GH_TOKEN")
originalGitHubToken := os.Getenv("GITHUB_TOKEN")
defer func() {
os.Setenv("GH_TOKEN", originalGHToken)
os.Setenv("GITHUB_TOKEN", originalGitHubToken)
originalGHToken, hadGHToken := os.LookupEnv("GH_TOKEN")
originalGitHubToken, hadGitHubToken := os.LookupEnv("GITHUB_TOKEN")
defer func() {
if hadGHToken {
os.Setenv("GH_TOKEN", originalGHToken)
} else {
os.Unsetenv("GH_TOKEN")
}
if hadGitHubToken {
os.Setenv("GITHUB_TOKEN", originalGitHubToken)
} else {
os.Unsetenv("GITHUB_TOKEN")
}

Copilot uses AI. Check for mistakes.
}()

// Set up test environment
os.Unsetenv("GH_TOKEN")
os.Setenv("GITHUB_TOKEN", "test-token")

// Test with multiple arguments
cmd := ExecGH("api", "repos/owner/repo/git/ref/tags/v1.0", "--jq", ".object.sha")

// Verify command
if cmd.Path != "gh" && !strings.HasSuffix(cmd.Path, "/gh") {
t.Errorf("Expected command path to be 'gh', got: %s", cmd.Path)
}

// Verify all arguments are preserved
expectedArgs := []string{"gh", "api", "repos/owner/repo/git/ref/tags/v1.0", "--jq", ".object.sha"}
if len(cmd.Args) != len(expectedArgs) {
t.Errorf("Expected %d args, got %d: %v", len(expectedArgs), len(cmd.Args), cmd.Args)
}

for i, expected := range expectedArgs {
if i >= len(cmd.Args) || cmd.Args[i] != expected {
t.Errorf("Arg %d: expected %s, got %s", i, expected, cmd.Args[i])
}
}

// Verify environment contains GH_TOKEN
found := false
for _, env := range cmd.Env {
if env == "GH_TOKEN=test-token" {
found = true
break
}
}
if !found {
t.Errorf("Expected environment to contain GH_TOKEN=test-token")
}
}
Loading