Make safe-job conclusion dependencies deterministic

pkg/workflow/safe_jobs.go

@@ -3,6 +3,7 @@ package workflow
 import (
 	"fmt"
 	"maps"
+	"sort"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/constants"
@@ -161,9 +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
-	for jobName, jobConfig := range data.SafeOutputs.Jobs {
-		// Normalize job name to use underscores for consistency
-		normalizedJobName := stringutil.NormalizeSafeOutputIdentifier(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.Slice(entries, func(i, j int) bool { return entries[i].normalizedName < entries[j].normalizedName })
+
+	for _, entry := range entries {
+		jobConfig := entry.config
+		normalizedJobName := entry.normalizedName
 
 		job := &Job{
 			Name:        normalizedJobName,
@@ -265,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
@@ -285,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)
 				}
@@ -307,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)

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 := 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")
+}
+
 func TestBuildSafeJobsWithNoConfiguration(t *testing.T) {
 	c := NewCompiler()