From 3b6a9fa41c6a1bab85113933f28e0e7f370d77f9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:43:56 +0000 Subject: [PATCH 1/7] Skip activation secret validation when environment is configured Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a7843a88-39e3-430a-b982-450cab6805c4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/engine_helpers.go | 4 ++ pkg/workflow/secret_validation_test.go | 37 +++++++++++++++++++ .../secret_verification_output_test.go | 36 ++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index 65a6e426e7b..cde19967aa6 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -265,6 +265,10 @@ func BuildDefaultSecretValidationStep(workflowData *WorkflowData, secrets []stri engineHelpersLog.Printf("Skipping secret validation step: custom command specified (%s)", workflowData.EngineConfig.Command) return GitHubActionStep{} } + if workflowData != nil && strings.TrimSpace(workflowData.Environment) != "" { + engineHelpersLog.Print("Skipping secret validation step: top-level environment is configured") + return GitHubActionStep{} + } return GenerateMultiSecretValidationStep(secrets, name, docsURL, getEngineEnvOverrides(workflowData)) } diff --git a/pkg/workflow/secret_validation_test.go b/pkg/workflow/secret_validation_test.go index c3d7d9a6940..4ee8cd8a774 100644 --- a/pkg/workflow/secret_validation_test.go +++ b/pkg/workflow/secret_validation_test.go @@ -280,3 +280,40 @@ func TestValidationStepUsesEngineEnvOverride(t *testing.T) { }) } } + +func TestEngineSecretValidationSkippedWhenEnvironmentConfigured(t *testing.T) { + tests := []struct { + name string + engine CodingAgentEngine + }{ + { + name: "copilot engine skips validation with environment", + engine: NewCopilotEngine(), + }, + { + name: "claude engine skips validation with environment", + engine: NewClaudeEngine(), + }, + { + name: "codex engine skips validation with environment", + engine: NewCodexEngine(), + }, + { + name: "gemini engine skips validation with environment", + engine: NewGeminiEngine(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + workflowData := &WorkflowData{ + Environment: "environment: production", + } + + step := tt.engine.GetSecretValidationStep(workflowData) + if len(step) != 0 { + t.Fatalf("expected secret validation step to be skipped when environment is configured, got:\n%s", strings.Join(step, "\n")) + } + }) + } +} diff --git a/pkg/workflow/secret_verification_output_test.go b/pkg/workflow/secret_verification_output_test.go index bdf7fb55c3e..69e90efb75f 100644 --- a/pkg/workflow/secret_verification_output_test.go +++ b/pkg/workflow/secret_verification_output_test.go @@ -91,3 +91,39 @@ Test workflow` t.Error("Expected conclusion job to receive secret_verification_result from activation job") } } + +func TestSecretVerificationSkippedWhenTopLevelEnvironmentConfigured(t *testing.T) { + testDir := testutil.TempDir(t, "test-secret-verification-environment-*") + workflowFile := filepath.Join(testDir, "test-workflow.md") + + workflow := `--- +on: workflow_dispatch +engine: copilot +environment: production +--- + +Test workflow` + + if err := os.WriteFile(workflowFile, []byte(workflow), 0644); err != nil { + t.Fatalf("Failed to write test workflow: %v", err) + } + + compiler := NewCompiler() + if err := compiler.CompileWorkflow(workflowFile); err != nil { + t.Fatalf("Failed to compile workflow: %v", err) + } + + lockFile := stringutil.MarkdownToLockFile(workflowFile) + lockContent, err := os.ReadFile(lockFile) + if err != nil { + t.Fatalf("Failed to read lock file: %v", err) + } + + lockStr := string(lockContent) + if strings.Contains(lockStr, "id: validate-secret") { + t.Error("Expected validate-secret step to be skipped when top-level environment is configured") + } + if strings.Contains(lockStr, "secret_verification_result: ${{ steps.validate-secret.outputs.verification_result }}") { + t.Error("Expected secret_verification_result activation output to be skipped when top-level environment is configured") + } +} From 088ddd3b511fe004481ebafc3e17b1332a4aab66 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:45:57 +0000 Subject: [PATCH 2/7] Refine environment skip tests for activation secret validation Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a7843a88-39e3-430a-b982-450cab6805c4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/secret_verification_output_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/secret_verification_output_test.go b/pkg/workflow/secret_verification_output_test.go index 69e90efb75f..750507999ae 100644 --- a/pkg/workflow/secret_verification_output_test.go +++ b/pkg/workflow/secret_verification_output_test.go @@ -92,7 +92,7 @@ Test workflow` } } -func TestSecretVerificationSkippedWhenTopLevelEnvironmentConfigured(t *testing.T) { +func TestSecretVerificationSkippedWithEnvironment(t *testing.T) { testDir := testutil.TempDir(t, "test-secret-verification-environment-*") workflowFile := filepath.Join(testDir, "test-workflow.md") @@ -104,7 +104,7 @@ environment: production Test workflow` - if err := os.WriteFile(workflowFile, []byte(workflow), 0644); err != nil { + if err := os.WriteFile(workflowFile, []byte(workflow), 0o644); err != nil { t.Fatalf("Failed to write test workflow: %v", err) } From a9e84a8df46ba3bf67b06750e94bb76cd94e444e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:47:59 +0000 Subject: [PATCH 3/7] Tighten environment-based secret validation skip tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a7843a88-39e3-430a-b982-450cab6805c4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/secret_validation_test.go | 2 +- pkg/workflow/secret_verification_output_test.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/secret_validation_test.go b/pkg/workflow/secret_validation_test.go index 4ee8cd8a774..5733377c53b 100644 --- a/pkg/workflow/secret_validation_test.go +++ b/pkg/workflow/secret_validation_test.go @@ -307,7 +307,7 @@ func TestEngineSecretValidationSkippedWhenEnvironmentConfigured(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { workflowData := &WorkflowData{ - Environment: "environment: production", + Environment: "production", } step := tt.engine.GetSecretValidationStep(workflowData) diff --git a/pkg/workflow/secret_verification_output_test.go b/pkg/workflow/secret_verification_output_test.go index 750507999ae..2419dc2e27c 100644 --- a/pkg/workflow/secret_verification_output_test.go +++ b/pkg/workflow/secret_verification_output_test.go @@ -120,6 +120,9 @@ Test workflow` } lockStr := string(lockContent) + if !strings.Contains(lockStr, "environment: production") { + t.Error("Expected compiled workflow to include environment: production") + } if strings.Contains(lockStr, "id: validate-secret") { t.Error("Expected validate-secret step to be skipped when top-level environment is configured") } From 20a31dcfb60309a0f90ca99cea0c98a2e46b5f95 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:50:13 +0000 Subject: [PATCH 4/7] Polish environment skip tests per review feedback Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a7843a88-39e3-430a-b982-450cab6805c4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/secret_validation_test.go | 6 +++--- pkg/workflow/secret_verification_output_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/workflow/secret_validation_test.go b/pkg/workflow/secret_validation_test.go index 5733377c53b..2e996156cbc 100644 --- a/pkg/workflow/secret_validation_test.go +++ b/pkg/workflow/secret_validation_test.go @@ -310,9 +310,9 @@ func TestEngineSecretValidationSkippedWhenEnvironmentConfigured(t *testing.T) { Environment: "production", } - step := tt.engine.GetSecretValidationStep(workflowData) - if len(step) != 0 { - t.Fatalf("expected secret validation step to be skipped when environment is configured, got:\n%s", strings.Join(step, "\n")) + steps := tt.engine.GetSecretValidationStep(workflowData) + if len(steps) != 0 { + t.Fatalf("expected secret validation step to be skipped when environment is configured, got:\n%s", strings.Join(steps, "\n")) } }) } diff --git a/pkg/workflow/secret_verification_output_test.go b/pkg/workflow/secret_verification_output_test.go index 2419dc2e27c..8df48ee4806 100644 --- a/pkg/workflow/secret_verification_output_test.go +++ b/pkg/workflow/secret_verification_output_test.go @@ -93,7 +93,7 @@ Test workflow` } func TestSecretVerificationSkippedWithEnvironment(t *testing.T) { - testDir := testutil.TempDir(t, "test-secret-verification-environment-*") + testDir := testutil.TempDir(t, "secret-verify-env-*") workflowFile := filepath.Join(testDir, "test-workflow.md") workflow := `--- From 6aa55c0728efc5f39329bfbcca9a3a48a015a683 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 22:52:12 +0000 Subject: [PATCH 5/7] Rename environment skip test for clarity Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a7843a88-39e3-430a-b982-450cab6805c4 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/secret_verification_output_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/workflow/secret_verification_output_test.go b/pkg/workflow/secret_verification_output_test.go index 8df48ee4806..46ac0e5b5ae 100644 --- a/pkg/workflow/secret_verification_output_test.go +++ b/pkg/workflow/secret_verification_output_test.go @@ -92,7 +92,7 @@ Test workflow` } } -func TestSecretVerificationSkippedWithEnvironment(t *testing.T) { +func TestSecretVerificationOutputSkippedWithEnvironment(t *testing.T) { testDir := testutil.TempDir(t, "secret-verify-env-*") workflowFile := filepath.Join(testDir, "test-workflow.md") From ceffca3bb9b30ce565ac0125d1b9bf46adf0492c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 23:17:11 +0000 Subject: [PATCH 6/7] Guard nil workflow data in secret validation helper Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f835cc24-6ee6-4012-9776-08ab293cff4d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/engine_helpers.go | 2 +- pkg/workflow/secret_validation_test.go | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/engine_helpers.go b/pkg/workflow/engine_helpers.go index cde19967aa6..b8d08739885 100644 --- a/pkg/workflow/engine_helpers.go +++ b/pkg/workflow/engine_helpers.go @@ -261,7 +261,7 @@ func GenerateMultiSecretValidationStep(secretNames []string, engineName, docsURL // Returns: // - GitHubActionStep: The validation step, or an empty step if a custom command is set func BuildDefaultSecretValidationStep(workflowData *WorkflowData, secrets []string, name, docsURL string) GitHubActionStep { - if workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { + if workflowData != nil && workflowData.EngineConfig != nil && workflowData.EngineConfig.Command != "" { engineHelpersLog.Printf("Skipping secret validation step: custom command specified (%s)", workflowData.EngineConfig.Command) return GitHubActionStep{} } diff --git a/pkg/workflow/secret_validation_test.go b/pkg/workflow/secret_validation_test.go index 2e996156cbc..9832f883986 100644 --- a/pkg/workflow/secret_validation_test.go +++ b/pkg/workflow/secret_validation_test.go @@ -317,3 +317,21 @@ func TestEngineSecretValidationSkippedWhenEnvironmentConfigured(t *testing.T) { }) } } + +func TestBuildDefaultSecretValidationStepHandlesNilWorkflowData(t *testing.T) { + step := BuildDefaultSecretValidationStep( + nil, + []string{"COPILOT_GITHUB_TOKEN"}, + "GitHub Copilot CLI", + "https://github.github.com/gh-aw/reference/engines/#github-copilot-default", + ) + + if len(step) == 0 { + t.Fatal("expected non-empty validation step for nil workflowData") + } + + stepContent := strings.Join(step, "\n") + if !strings.Contains(stepContent, "Validate COPILOT_GITHUB_TOKEN secret") { + t.Errorf("expected validation step to include COPILOT_GITHUB_TOKEN check, got:\n%s", stepContent) + } +} From 5ce1103e9d7faf88e06601b89ffb386dbbaf70f8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 23:18:50 +0000 Subject: [PATCH 7/7] Use testify assertions in nil workflow data test Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f835cc24-6ee6-4012-9776-08ab293cff4d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/secret_validation_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/workflow/secret_validation_test.go b/pkg/workflow/secret_validation_test.go index 9832f883986..c658a1baba6 100644 --- a/pkg/workflow/secret_validation_test.go +++ b/pkg/workflow/secret_validation_test.go @@ -6,6 +6,9 @@ import ( "fmt" "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGenerateMultiSecretValidationStep(t *testing.T) { @@ -326,12 +329,8 @@ func TestBuildDefaultSecretValidationStepHandlesNilWorkflowData(t *testing.T) { "https://github.github.com/gh-aw/reference/engines/#github-copilot-default", ) - if len(step) == 0 { - t.Fatal("expected non-empty validation step for nil workflowData") - } + require.NotEmpty(t, step, "expected non-empty validation step for nil workflowData") stepContent := strings.Join(step, "\n") - if !strings.Contains(stepContent, "Validate COPILOT_GITHUB_TOKEN secret") { - t.Errorf("expected validation step to include COPILOT_GITHUB_TOKEN check, got:\n%s", stepContent) - } + assert.Contains(t, stepContent, "Validate COPILOT_GITHUB_TOKEN secret", "expected validation step to include COPILOT_GITHUB_TOKEN check") }