From 0546c935c6f0567e5cb75cf6508a7cc865efcf29 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 19 Jun 2026 17:18:31 +0000 Subject: [PATCH 1/3] Initial plan From fda7f6c28f8fb7b2427455f8deae9654f7ad890f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 19 Jun 2026 17:41:31 +0000 Subject: [PATCH 2/3] fix deterministic safe-job ordering Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/safe_jobs.go | 10 ++++++- pkg/workflow/safe_jobs_test.go | 48 ++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 7bf5709accc..53967e31f61 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -3,6 +3,7 @@ package workflow import ( "fmt" "maps" + "sort" "strings" "github.com/github/gh-aw/pkg/constants" @@ -161,7 +162,14 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool safeJobsLog.Printf("Building %d safe-jobs, threatDetectionEnabled=%v", len(data.SafeOutputs.Jobs), threatDetectionEnabled) var safeJobNames []string - for jobName, jobConfig := range data.SafeOutputs.Jobs { + jobNames := make([]string, 0, len(data.SafeOutputs.Jobs)) + for jobName := range data.SafeOutputs.Jobs { + jobNames = append(jobNames, jobName) + } + sort.Strings(jobNames) + + for _, jobName := range jobNames { + jobConfig := data.SafeOutputs.Jobs[jobName] // Normalize job name to use underscores for consistency normalizedJobName := stringutil.NormalizeSafeOutputIdentifier(jobName) diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index 747ccf72e1b..6c9684ba2f6 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.go @@ -3,9 +3,12 @@ package workflow import ( + "os" + "path/filepath" "strings" "testing" + "github.com/github/gh-aw/pkg/testutil" "github.com/stretchr/testify/require" ) @@ -242,6 +245,51 @@ func TestBuildSafeJobs(t *testing.T) { } } +func TestCompileWorkflowConclusionNeedsSafeJobsDeterministically(t *testing.T) { + tmpDir := testutil.TempDir(t, "safe-jobs-ordering") + + testContent := `--- +on: + issues: + types: [opened] +permissions: + contents: read +engine: copilot +safe-outputs: + jobs: + zebra-job: + steps: + - name: Zebra + run: echo "zebra" + alpha-job: + steps: + - name: Alpha + run: echo "alpha" +--- + +# Test deterministic safe-job ordering +` + + testFile := filepath.Join(tmpDir, "test-safe-jobs-ordering.md") + require.NoError(t, os.WriteFile(testFile, []byte(testContent), 0o644)) + + compiler := NewCompiler() + require.NoError(t, compiler.CompileWorkflow(testFile)) + + lockFile := filepath.Join(tmpDir, "test-safe-jobs-ordering.lock.yml") + compiled, err := os.ReadFile(lockFile) + require.NoError(t, err) + + conclusionSection := extractJobSection(string(compiled), "conclusion") + require.NotEmpty(t, conclusionSection) + + alphaIdx := strings.Index(conclusionSection, "- alpha_job") + zebraIdx := strings.Index(conclusionSection, "- zebra_job") + require.NotEqual(t, -1, alphaIdx, "conclusion job should depend on alpha_job") + require.NotEqual(t, -1, zebraIdx, "conclusion job should depend on zebra_job") + require.Less(t, alphaIdx, zebraIdx, "conclusion job should list safe-jobs in deterministic sorted order") +} + func TestBuildSafeJobsWithNoConfiguration(t *testing.T) { c := NewCompiler() From ff7307a9979852ac83b3ee1beb3f2aba3d0458e9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 19 Jun 2026 18:28:47 +0000 Subject: [PATCH 3/3] fix: sort safe-jobs by normalized name; use indexInNonCommentLines in test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/safe_jobs.go | 31 ++++++++++++++++++++----------- pkg/workflow/safe_jobs_test.go | 4 ++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/pkg/workflow/safe_jobs.go b/pkg/workflow/safe_jobs.go index 53967e31f61..d2fc4f09b31 100644 --- a/pkg/workflow/safe_jobs.go +++ b/pkg/workflow/safe_jobs.go @@ -162,16 +162,25 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool safeJobsLog.Printf("Building %d safe-jobs, threatDetectionEnabled=%v", len(data.SafeOutputs.Jobs), threatDetectionEnabled) var safeJobNames []string - jobNames := make([]string, 0, len(data.SafeOutputs.Jobs)) - for jobName := range data.SafeOutputs.Jobs { - jobNames = append(jobNames, jobName) + + // Collect normalized names and a config lookup map, then sort by normalized name. + // Sorting on the normalized form (rather than the raw key) guarantees that the + // returned safeJobNames slice and the conclusion job's needs: list are ordered + // consistently with the names that appear in the compiled YAML, even when raw + // names contain characters (e.g. '.', '-') that normalize differently from '_'. + type safeJobEntry struct { + normalizedName string + config *SafeJobConfig + } + entries := make([]safeJobEntry, 0, len(data.SafeOutputs.Jobs)) + for rawName, cfg := range data.SafeOutputs.Jobs { + entries = append(entries, safeJobEntry{stringutil.NormalizeSafeOutputIdentifier(rawName), cfg}) } - sort.Strings(jobNames) + sort.Slice(entries, func(i, j int) bool { return entries[i].normalizedName < entries[j].normalizedName }) - for _, jobName := range jobNames { - jobConfig := data.SafeOutputs.Jobs[jobName] - // Normalize job name to use underscores for consistency - normalizedJobName := stringutil.NormalizeSafeOutputIdentifier(jobName) + for _, entry := range entries { + jobConfig := entry.config + normalizedJobName := entry.normalizedName job := &Job{ Name: normalizedJobName, @@ -273,7 +282,7 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool // Convert to typed step for action pinning typedStep, err := MapToStep(stepMap) if err != nil { - return nil, fmt.Errorf("failed to convert step to typed step for safe job %s: %w", jobName, err) + return nil, fmt.Errorf("failed to convert step to typed step for safe job %s: %w", normalizedJobName, err) } // Inject setup env vars so user steps can access GH_AW_AGENT_OUTPUT @@ -293,7 +302,7 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool // Convert back to map for YAML generation stepYAML, err := ConvertStepToYAML(pinnedStep.ToMap()) if err != nil { - return nil, fmt.Errorf("failed to convert step to YAML for safe job %s: %w", jobName, err) + return nil, fmt.Errorf("failed to convert step to YAML for safe job %s: %w", normalizedJobName, err) } steps = append(steps, stepYAML) } @@ -315,7 +324,7 @@ func (c *Compiler) buildSafeJobs(data *WorkflowData, threatDetectionEnabled bool // Add the job to the job manager if err := c.jobManager.AddJob(job); err != nil { safeJobsLog.Printf("Failed to add safe-job %s: %v", normalizedJobName, err) - return nil, fmt.Errorf("failed to add safe job %s: %w", jobName, err) + return nil, fmt.Errorf("failed to add safe job %s: %w", normalizedJobName, err) } safeJobsLog.Printf("Created safe-job: %s with %d dependencies and %d steps", normalizedJobName, len(job.Needs), len(job.Steps)) safeJobNames = append(safeJobNames, normalizedJobName) diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index 6c9684ba2f6..6156e11e9cb 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.go @@ -283,8 +283,8 @@ safe-outputs: conclusionSection := extractJobSection(string(compiled), "conclusion") require.NotEmpty(t, conclusionSection) - alphaIdx := strings.Index(conclusionSection, "- alpha_job") - zebraIdx := strings.Index(conclusionSection, "- zebra_job") + alphaIdx := indexInNonCommentLines(conclusionSection, "- alpha_job") + zebraIdx := indexInNonCommentLines(conclusionSection, "- zebra_job") require.NotEqual(t, -1, alphaIdx, "conclusion job should depend on alpha_job") require.NotEqual(t, -1, zebraIdx, "conclusion job should depend on zebra_job") require.Less(t, alphaIdx, zebraIdx, "conclusion job should list safe-jobs in deterministic sorted order")