Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 5 additions & 11 deletions pkg/cli/docker_images.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"sync"
"time"

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

Expand Down Expand Up @@ -67,17 +68,10 @@ var pullState = &dockerPullState{
mockDockerAvailable: true,
}

func normalizeDockerContext(ctx context.Context) context.Context {
if ctx == nil {
return context.TODO()
}
return ctx
}

// isDockerImageAvailableUnlocked checks if a Docker image is available locally
// This function must be called with pullState.mu held (either RLock or Lock)
func isDockerImageAvailableUnlocked(ctx context.Context, image string) bool {
ctx = normalizeDockerContext(ctx)
ctx = ctxutil.OrBackground(ctx)

// Check if we're in mock mode (for testing)
if pullState.mockAvailableInUse {
Expand All @@ -100,7 +94,7 @@ func isDockerImageAvailableUnlocked(ctx context.Context, image string) bool {

// IsDockerImageAvailable checks if a Docker image is available locally
func IsDockerImageAvailable(ctx context.Context, image string) bool {
ctx = normalizeDockerContext(ctx)
ctx = ctxutil.OrBackground(ctx)

pullState.mu.RLock()
defer pullState.mu.RUnlock()
Expand All @@ -116,7 +110,7 @@ func IsDockerImageDownloading(image string) bool {

// IsDockerAvailable checks if the Docker daemon is running and accessible
func IsDockerAvailable(ctx context.Context) bool {
ctx = normalizeDockerContext(ctx)
ctx = ctxutil.OrBackground(ctx)

mockEnabled, mockAvailable := func() (bool, bool) {
pullState.mu.RLock()
Expand Down Expand Up @@ -146,7 +140,7 @@ func IsDockerAvailable(ctx context.Context) bool {
// The returned join function blocks until the download goroutine exits and returns
// any error that occurred (nil on success or context cancellation).
func StartDockerImageDownload(ctx context.Context, image string) (bool, func() error) {
ctx = normalizeDockerContext(ctx)
ctx = ctxutil.OrBackground(ctx)

// Check availability and downloading status atomically under lock
pullState.mu.Lock()
Expand Down
22 changes: 0 additions & 22 deletions pkg/cli/docker_images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,28 +286,6 @@ func TestMockImageAvailability(t *testing.T) {
ResetDockerPullState()
}

func TestNormalizeDockerContext_NilContextReturnsTODO(t *testing.T) {
//nolint:staticcheck // Intentionally validating nil context normalization behavior.
ctx := normalizeDockerContext(nil)

if ctx == nil {
t.Fatal("Expected nil context to be replaced")
}

if err := ctx.Err(); err != nil {
t.Fatalf("Expected replacement context to be active, got err: %v", err)
}
}

func TestNormalizeDockerContext_PreservesNonNilContext(t *testing.T) {
ctx, cancel := context.WithCancel(t.Context())
defer cancel()

if normalizeDockerContext(ctx) != ctx {
t.Fatal("Expected non-nil context to be preserved")
}
}

func TestIsDockerAvailable_NilContext(t *testing.T) {
ResetDockerPullState()
SetMockDockerAvailable(true)
Expand Down
7 changes: 3 additions & 4 deletions pkg/workflow/github_cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"sync"

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/ctxutil"
"github.com/github/gh-aw/pkg/logger"
"github.com/github/gh-aw/pkg/tty"
)
Expand All @@ -39,16 +40,14 @@ func getDefaultGHHost() string {

// setupGHCommand creates an exec.Cmd for gh CLI with proper token configuration.
// This is the core implementation shared by ExecGH and ExecGHContext.
// When ctx is nil, it falls back to context.TODO().
// When ctx is nil, it falls back to context.Background().
func setupGHCommand(ctx context.Context, args ...string) *exec.Cmd {
// Check if GH_TOKEN or GITHUB_TOKEN is available
ghToken := lookupProcessEnv("GH_TOKEN")
githubToken := lookupProcessEnv("GITHUB_TOKEN")
ghHost := lookupProcessEnv("GH_HOST")

if ctx == nil {
ctx = context.TODO()
}
ctx = ctxutil.OrBackground(ctx)
cmd := exec.CommandContext(ctx, "gh", args...)

if ghToken != "" || githubToken != "" {
Expand Down
10 changes: 3 additions & 7 deletions pkg/workflow/skills_ref_resolution.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package workflow

import (
"context"
"fmt"
"os"

"github.com/github/gh-aw/pkg/ctxutil"
)

// resolveFrontmatterSkillRefs pins non-SHA remote skill refs (owner/repo[/path]@ref) to
Expand Down Expand Up @@ -67,12 +68,7 @@ func (c *Compiler) resolveSkillRefSpec(data *WorkflowData, markdownPath, spec st
return spec
}

ctx := data.Ctx
if ctx == nil {
ctx = context.Background()
}

sha, err := data.ActionResolver.ResolveSHA(ctx, parsed.repoPath, parsed.ref)
sha, err := data.ActionResolver.ResolveSHA(ctxutil.OrBackground(data.Ctx), parsed.repoPath, parsed.ref)
if err != nil {
skillsFrontmatterLog.Printf("skills[%d]: failed to resolve ref %q for %q to a SHA: %v", idx, parsed.ref, parsed.repoPath, err)
fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning",
Expand Down
21 changes: 21 additions & 0 deletions pkg/workflow/skills_ref_resolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,27 @@ func TestResolveFrontmatterSkillRefs_PinsNonSHARefUsingCache(t *testing.T) {
assert.Empty(t, strings.TrimSpace(output), "no warning expected when resolution succeeds")
}

func TestResolveFrontmatterSkillRefs_PinsNonSHARefWithNilContext(t *testing.T) {
tmpDir := testutil.TempDir(t, "skill-ref-cache-nil-context")
cache := NewActionCache(tmpDir)
resolver := NewActionResolver(cache)
const sha = "1f181b37d3fe5862ab590648f25a292e345b5de6"
cache.Set("githubnext/skills", "main", sha)

compiler := NewCompiler(WithVersion("dev"))
data := newTestSkillWorkflowData([]string{"githubnext/skills@main"})
data.Ctx = nil
data.ActionResolver = resolver

output := withCapturedStderr(t, func() {
compiler.resolveFrontmatterSkillRefs(data, "workflow.md")
})

assert.Equal(t, "githubnext/skills@"+sha, data.Skills[0])
assert.Equal(t, "githubnext/skills@"+sha, data.SkillReferences[0].Skill)
assert.Empty(t, strings.TrimSpace(output), "no warning expected when resolution succeeds")
}

func TestResolveFrontmatterSkillRefs_LeavesFullSHAUnchanged(t *testing.T) {
compiler := NewCompiler(WithVersion("dev"))
const sha = "1f181b37d3fe5862ab590648f25a292e345b5de6"
Expand Down
Loading