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
17 changes: 14 additions & 3 deletions pkg/workflow/agentic_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,10 @@ func GenerateSecretValidationStep(secretName, engineName, docsURL string) GitHub
// secretNames: slice of secret names to validate (e.g., []string{"CODEX_API_KEY", "OPENAI_API_KEY"})
// engineName: the display name of the engine (e.g., "Codex")
// docsURL: URL to the documentation page for setting up the secret
func GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL string) GitHubActionStep {
// envOverrides: optional map of env var key to expression override (from engine.env); when set,
// the overridden expression is used instead of the default "${{ secrets.KEY }}" so the
// validation step checks the user-provided secret reference rather than the default one.
func GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL string, envOverrides map[string]string) GitHubActionStep {
if len(secretNames) == 0 {
// This is a programming error - engine configurations should always provide secrets
// Log the error and return empty step to avoid breaking compilation
Expand All @@ -463,9 +466,17 @@ func GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL
" env:",
}

// Add env section with all secrets
// Add env section with all secrets. When engine.env provides an override for a key,
// use that expression (e.g. "${{ secrets.MY_ORG_TOKEN }}") so the validation step
// validates the user-supplied secret instead of the default one.
for _, secretName := range secretNames {
stepLines = append(stepLines, fmt.Sprintf(" %s: ${{ secrets.%s }}", secretName, secretName))
expr := fmt.Sprintf("${{ secrets.%s }}", secretName)
if envOverrides != nil {
if override, ok := envOverrides[secretName]; ok {
expr = override
}
}
stepLines = append(stepLines, fmt.Sprintf(" %s: %s", secretName, expr))
}

return GitHubActionStep(stepLines)
Expand Down
1 change: 1 addition & 0 deletions pkg/workflow/claude_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func (e *ClaudeEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub
config.Secrets,
config.Name,
config.DocsURL,
getEngineEnvOverrides(workflowData),
)
steps = append(steps, secretValidation)

Expand Down
52 changes: 52 additions & 0 deletions pkg/workflow/claude_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,3 +523,55 @@ func TestClaudeEngineSkipInstallationWithCommand(t *testing.T) {
t.Errorf("Expected 0 installation steps when command is specified, got %d", len(steps))
}
}

func TestClaudeEngineEnvOverridesTokenExpression(t *testing.T) {
engine := NewClaudeEngine()

t.Run("engine env overrides default token expression", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"ANTHROPIC_API_KEY": "${{ secrets.MY_ORG_ANTHROPIC_KEY }}",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
if len(steps) != 1 {
t.Fatalf("Expected 1 step, got %d", len(steps))
}

stepContent := strings.Join([]string(steps[0]), "\n")

// engine.env override should replace the default token expression
if !strings.Contains(stepContent, "ANTHROPIC_API_KEY: ${{ secrets.MY_ORG_ANTHROPIC_KEY }}") {
t.Errorf("Expected engine.env to override ANTHROPIC_API_KEY, got:\n%s", stepContent)
}
if strings.Contains(stepContent, "ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}") {
t.Errorf("Default ANTHROPIC_API_KEY expression should be replaced by engine.env override, got:\n%s", stepContent)
}
})

t.Run("engine env adds extra environment variables", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"CUSTOM_VAR": "custom-value",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
if len(steps) != 1 {
t.Fatalf("Expected 1 step, got %d", len(steps))
}

stepContent := strings.Join([]string(steps[0]), "\n")

if !strings.Contains(stepContent, "CUSTOM_VAR: custom-value") {
t.Errorf("Expected engine.env to add CUSTOM_VAR, got:\n%s", stepContent)
}
})
}
52 changes: 52 additions & 0 deletions pkg/workflow/codex_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -770,3 +770,55 @@ func TestCodexEngineSkipInstallationWithCommand(t *testing.T) {
t.Errorf("Expected 0 installation steps when command is specified, got %d", len(steps))
}
}

func TestCodexEngineEnvOverridesTokenExpression(t *testing.T) {
engine := NewCodexEngine()

t.Run("engine env overrides default token expression", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"CODEX_API_KEY": "${{ secrets.MY_ORG_CODEX_KEY }}",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
if len(steps) != 1 {
t.Fatalf("Expected 1 step, got %d", len(steps))
}

stepContent := strings.Join([]string(steps[0]), "\n")

// engine.env override should replace the default token expression
if !strings.Contains(stepContent, "CODEX_API_KEY: ${{ secrets.MY_ORG_CODEX_KEY }}") {
t.Errorf("Expected engine.env to override CODEX_API_KEY, got:\n%s", stepContent)
}
if strings.Contains(stepContent, "CODEX_API_KEY: ${{ secrets.CODEX_API_KEY || secrets.OPENAI_API_KEY }}") {
t.Errorf("Default CODEX_API_KEY expression should be replaced by engine.env override, got:\n%s", stepContent)
}
})

t.Run("engine env adds extra environment variables", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"CUSTOM_VAR": "custom-value",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
if len(steps) != 1 {
t.Fatalf("Expected 1 step, got %d", len(steps))
}

stepContent := strings.Join([]string(steps[0]), "\n")

if !strings.Contains(stepContent, "CUSTOM_VAR: custom-value") {
t.Errorf("Expected engine.env to add CUSTOM_VAR, got:\n%s", stepContent)
}
})
}
1 change: 1 addition & 0 deletions pkg/workflow/copilot_engine_installation.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func (e *CopilotEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHu
config.Secrets,
config.Name,
config.DocsURL,
getEngineEnvOverrides(workflowData),
)
steps = append(steps, secretValidation)

Expand Down
52 changes: 52 additions & 0 deletions pkg/workflow/copilot_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1505,3 +1505,55 @@ func TestGenerateCopilotSessionFileCopyStep(t *testing.T) {
t.Error("Step should be marked continue-on-error")
}
}

func TestCopilotEngineEnvOverridesTokenExpression(t *testing.T) {
engine := NewCopilotEngine()

t.Run("engine env overrides default token expression", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"COPILOT_GITHUB_TOKEN": "${{ secrets.MY_ORG_COPILOT_TOKEN }}",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
if len(steps) != 1 {
t.Fatalf("Expected 1 step, got %d", len(steps))
}

stepContent := strings.Join([]string(steps[0]), "\n")

// engine.env override should replace the default token expression
if !strings.Contains(stepContent, "COPILOT_GITHUB_TOKEN: ${{ secrets.MY_ORG_COPILOT_TOKEN }}") {
t.Errorf("Expected engine.env to override COPILOT_GITHUB_TOKEN, got:\n%s", stepContent)
}
if strings.Contains(stepContent, "COPILOT_GITHUB_TOKEN: ${{ secrets.COPILOT_GITHUB_TOKEN }}") {
t.Errorf("Default COPILOT_GITHUB_TOKEN expression should be replaced by engine.env override, got:\n%s", stepContent)
}
})

t.Run("engine env adds extra environment variables", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"CUSTOM_VAR": "custom-value",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/gh-aw/test.log")
if len(steps) != 1 {
t.Fatalf("Expected 1 step, got %d", len(steps))
}

stepContent := strings.Join([]string(steps[0]), "\n")

if !strings.Contains(stepContent, "CUSTOM_VAR: custom-value") {
t.Errorf("Expected engine.env to add CUSTOM_VAR, got:\n%s", stepContent)
}
})
}
11 changes: 11 additions & 0 deletions pkg/workflow/engine_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ type EngineInstallConfig struct {
InstallStepName string
}

// getEngineEnvOverrides returns the engine.env map from workflowData, or nil if not set.
// This is used to pass user-provided env overrides to steps such as secret validation,
// so that overridden token expressions are used instead of the default "${{ secrets.KEY }}".
func getEngineEnvOverrides(workflowData *WorkflowData) map[string]string {
if workflowData == nil || workflowData.EngineConfig == nil {
return nil
}
return workflowData.EngineConfig.Env
}

// GetBaseInstallationSteps returns the common installation steps for an engine.
// This includes secret validation and npm package installation steps that are
// shared across all engines.
Expand All @@ -81,6 +91,7 @@ func GetBaseInstallationSteps(config EngineInstallConfig, workflowData *Workflow
config.Secrets,
config.Name,
config.DocsURL,
getEngineEnvOverrides(workflowData),
)
steps = append(steps, secretValidation)

Expand Down
16 changes: 16 additions & 0 deletions pkg/workflow/gemini_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package workflow

import (
"fmt"
"maps"

"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/logger"
Expand Down Expand Up @@ -111,6 +112,7 @@ func (e *GeminiEngine) GetInstallationSteps(workflowData *WorkflowData) []GitHub
config.Secrets,
config.Name,
config.DocsURL,
getEngineEnvOverrides(workflowData),
)
steps = append(steps, secretValidation)

Expand Down Expand Up @@ -278,6 +280,20 @@ func (e *GeminiEngine) GetExecutionSteps(workflowData *WorkflowData, logFile str
env[constants.GeminiCLIModelEnvVar] = workflowData.EngineConfig.Model
}

// Add custom environment variables from engine config.
// This allows users to override the default engine token expression (e.g.
// GEMINI_API_KEY: ${{ secrets.MY_ORG_GEMINI_KEY }}) via engine.env.
if workflowData.EngineConfig != nil && len(workflowData.EngineConfig.Env) > 0 {
maps.Copy(env, workflowData.EngineConfig.Env)
}

// Add custom environment variables from agent config
agentConfig := getAgentConfig(workflowData)
if agentConfig != nil && len(agentConfig.Env) > 0 {
maps.Copy(env, agentConfig.Env)
geminiLog.Printf("Added %d custom env vars from agent config", len(agentConfig.Env))
}

// Generate the execution step
stepLines := []string{
" - name: Execute Gemini CLI",
Expand Down
38 changes: 38 additions & 0 deletions pkg/workflow/gemini_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,44 @@ func TestGeminiEngineExecution(t *testing.T) {
assert.Contains(t, stepContent, "GEMINI_MODEL: gemini-2.0-flash", "Should set GEMINI_MODEL when model is explicitly configured")
})

t.Run("engine env overrides default token expression", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"GEMINI_API_KEY": "${{ secrets.MY_ORG_GEMINI_KEY }}",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/test.log")
require.Len(t, steps, 2, "Should generate settings step and execution step")

stepContent := strings.Join(steps[1], "\n")

// The user-provided value should override the default token expression
assert.Contains(t, stepContent, "GEMINI_API_KEY: ${{ secrets.MY_ORG_GEMINI_KEY }}", "engine.env should override the default GEMINI_API_KEY expression")
assert.NotContains(t, stepContent, "GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }}", "Default GEMINI_API_KEY expression should be replaced by engine.env")
})

t.Run("engine env adds custom non-secret env vars", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
EngineConfig: &EngineConfig{
Env: map[string]string{
"CUSTOM_VAR": "custom-value",
},
},
}

steps := engine.GetExecutionSteps(workflowData, "/tmp/test.log")
require.Len(t, steps, 2, "Should generate settings step and execution step")

stepContent := strings.Join(steps[1], "\n")

assert.Contains(t, stepContent, "CUSTOM_VAR: custom-value", "engine.env non-secret vars should be included")
})

t.Run("settings step is first", func(t *testing.T) {
workflowData := &WorkflowData{
Name: "test-workflow",
Expand Down
Loading
Loading