Fix duplicate GitHub App token step in safe-outputs job (#16135)

Author: Copilot

pkg/workflow/compiler_safe_outputs_job.go

@@ -35,13 +35,8 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa
 	// Track whether threat detection job is enabled for step conditions
 	threatDetectionEnabled := data.SafeOutputs.ThreatDetection != nil
 
-	// Add GitHub App token minting step if app is configured
-	if data.SafeOutputs.App != nil {
-		consolidatedSafeOutputsJobLog.Print("Adding GitHub App token minting step")
-		// Prepend GitHub App token step before other steps
-		appTokenSteps := c.buildGitHubAppTokenMintStep(data.SafeOutputs.App, permissions)
-		steps = append(steps, appTokenSteps...)
-	}
+	// Note: GitHub App token minting step is added later (after setup/downloads)
+	// to ensure proper step ordering. See insertion logic below.
 
 	// Add setup action to copy JavaScript files
 	setupActionRef := c.resolveActionReference("./actions/setup", data)
@@ -244,7 +239,7 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa
 	// Add GitHub App token minting step at the beginning if app is configured
 	if data.SafeOutputs.App != nil {
 		appTokenSteps := c.buildGitHubAppTokenMintStep(data.SafeOutputs.App, permissions)
-		// Calculate insertion index: after setup action (if present) and artifact downloads, but before safe output steps
+		// Calculate insertion index: after setup action (if present) and artifact downloads, but before checkout and safe output steps
 		insertIndex := 0
 
 		// Count setup action steps (checkout + setup if in dev mode without action-tag, or just setup)
@@ -271,6 +266,9 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa
 			insertIndex += len(patchDownloadSteps)
 		}
 
+		// Note: App token step must be inserted BEFORE shared checkout steps
+		// because those steps reference steps.safe-outputs-app-token.outputs.token
+
 		// Insert app token steps
 		var newSteps []string
 		newSteps = append(newSteps, steps[:insertIndex]...)

pkg/workflow/compiler_safe_outputs_job_test.go

@@ -466,3 +466,45 @@ func TestJobDependencies(t *testing.T) {
 		})
 	}
 }
+
+// TestGitHubAppWithPushToPRBranch tests that GitHub App token step is not duplicated
+// when both app and push-to-pull-request-branch are configured
+// Regression test for duplicate step bug reported in issue
+func TestGitHubAppWithPushToPRBranch(t *testing.T) {
+	compiler := NewCompiler()
+	compiler.jobManager = NewJobManager()
+
+	workflowData := &WorkflowData{
+		Name: "Test Workflow",
+		SafeOutputs: &SafeOutputsConfig{
+			App: &GitHubAppConfig{
+				AppID:      "${{ vars.ACTIONS_APP_ID }}",
+				PrivateKey: "${{ secrets.ACTIONS_PRIVATE_KEY }}",
+			},
+			PushToPullRequestBranch: &PushToPullRequestBranchConfig{},
+		},
+	}
+
+	job, _, err := compiler.buildConsolidatedSafeOutputsJob(workflowData, string(constants.AgentJobName), "test.md")
+
+	require.NoError(t, err, "Should successfully build job")
+	require.NotNil(t, job, "Job should not be nil")
+
+	stepsContent := strings.Join(job.Steps, "")
+
+	// Should include app token minting step exactly once
+	tokenMintCount := strings.Count(stepsContent, "Generate GitHub App token")
+	assert.Equal(t, 1, tokenMintCount, "App token minting step should appear exactly once, found %d times", tokenMintCount)
+
+	// Should include app token invalidation step exactly once
+	tokenInvalidateCount := strings.Count(stepsContent, "Invalidate GitHub App token")
+	assert.Equal(t, 1, tokenInvalidateCount, "App token invalidation step should appear exactly once, found %d times", tokenInvalidateCount)
+
+	// Token step should come before checkout step (checkout references the token)
+	tokenIndex := strings.Index(stepsContent, "Generate GitHub App token")
+	checkoutIndex := strings.Index(stepsContent, "Checkout repository")
+	assert.Less(t, tokenIndex, checkoutIndex, "Token minting step should come before checkout step")
+
+	// Verify step ID is set correctly
+	assert.Contains(t, stepsContent, "id: safe-outputs-app-token")
+}