refactor: semantic function clustering in pkg/workflow (#21277)

Author: Copilot

pkg/workflow/frontmatter_extraction_metadata.go

@@ -3,7 +3,6 @@ package workflow
 import (
 	"fmt"
 	"maps"
-	"math"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
@@ -144,12 +143,7 @@ func buildSourceURL(source string) string {
 }
 
 // safeUintToInt safely converts uint to int, returning 0 if overflow would occur
-func safeUintToInt(u uint) int {
-	if u > math.MaxInt {
-		return 0 // Return 0 (engine default) if value would overflow
-	}
-	return int(u)
-}
+func safeUintToInt(u uint) int { return safeUint64ToInt(uint64(u)) }
 
 // extractToolsTimeout extracts the timeout setting from tools
 // Returns 0 if not set (engines will use their own defaults)

pkg/workflow/safe_outputs_env.go

@@ -0,0 +1,292 @@
+package workflow
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+
+	"github.com/github/gh-aw/pkg/logger"
+)
+
+var safeOutputsEnvLog = logger.New("workflow:safe_outputs_env")
+
+// ========================================
+// Safe Output Environment Variables
+// ========================================
+
+// applySafeOutputEnvToMap adds safe-output related environment variables to an env map
+// This extracts the duplicated safe-output env setup logic across all engines (copilot, codex, claude, custom)
+func applySafeOutputEnvToMap(env map[string]string, data *WorkflowData) {
+	if data.SafeOutputs == nil {
+		return
+	}
+
+	safeOutputsEnvLog.Printf("Applying safe output env vars: trial_mode=%t, staged=%t", data.TrialMode, data.SafeOutputs.Staged)
+
+	env["GH_AW_SAFE_OUTPUTS"] = "${{ env.GH_AW_SAFE_OUTPUTS }}"
+
+	// Add staged flag if specified
+	if data.TrialMode || data.SafeOutputs.Staged {
+		env["GH_AW_SAFE_OUTPUTS_STAGED"] = "true"
+	}
+	if data.TrialMode && data.TrialLogicalRepo != "" {
+		env["GH_AW_TARGET_REPO_SLUG"] = data.TrialLogicalRepo
+	}
+
+	// Add branch name if upload assets is configured
+	if data.SafeOutputs.UploadAssets != nil {
+		safeOutputsEnvLog.Printf("Adding upload assets env vars: branch=%s", data.SafeOutputs.UploadAssets.BranchName)
+		env["GH_AW_ASSETS_BRANCH"] = fmt.Sprintf("%q", data.SafeOutputs.UploadAssets.BranchName)
+		env["GH_AW_ASSETS_MAX_SIZE_KB"] = strconv.Itoa(data.SafeOutputs.UploadAssets.MaxSizeKB)
+		env["GH_AW_ASSETS_ALLOWED_EXTS"] = fmt.Sprintf("%q", strings.Join(data.SafeOutputs.UploadAssets.AllowedExts, ","))
+	}
+}
+
+// buildWorkflowMetadataEnvVars builds workflow name and source environment variables
+// This extracts the duplicated workflow metadata setup logic from safe-output job builders
+func buildWorkflowMetadataEnvVars(workflowName string, workflowSource string) []string {
+	var customEnvVars []string
+
+	// Add workflow name
+	customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_WORKFLOW_NAME: %q\n", workflowName))
+
+	// Add workflow source and source URL if present
+	if workflowSource != "" {
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_WORKFLOW_SOURCE: %q\n", workflowSource))
+		sourceURL := buildSourceURL(workflowSource)
+		if sourceURL != "" {
+			customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_WORKFLOW_SOURCE_URL: %q\n", sourceURL))
+		}
+	}
+
+	return customEnvVars
+}
+
+// buildWorkflowMetadataEnvVarsWithTrackerID builds workflow metadata env vars including tracker-id
+func buildWorkflowMetadataEnvVarsWithTrackerID(workflowName string, workflowSource string, trackerID string) []string {
+	customEnvVars := buildWorkflowMetadataEnvVars(workflowName, workflowSource)
+
+	// Add tracker-id if present
+	if trackerID != "" {
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_TRACKER_ID: %q\n", trackerID))
+	}
+
+	return customEnvVars
+}
+
+// buildSafeOutputJobEnvVars builds environment variables for safe-output jobs with staged/target repo handling
+// This extracts the duplicated env setup logic in safe-output job builders (create_issue, add_comment, etc.)
+func buildSafeOutputJobEnvVars(trialMode bool, trialLogicalRepoSlug string, staged bool, targetRepoSlug string) []string {
+	var customEnvVars []string
+
+	// Pass the staged flag if it's set to true
+	if trialMode || staged {
+		safeOutputsEnvLog.Printf("Setting staged flag: trial_mode=%t, staged=%t", trialMode, staged)
+		customEnvVars = append(customEnvVars, "          GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n")
+	}
+
+	// Set GH_AW_TARGET_REPO_SLUG - prefer target-repo config over trial target repo
+	if targetRepoSlug != "" {
+		safeOutputsEnvLog.Printf("Setting target repo slug from config: %s", targetRepoSlug)
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_TARGET_REPO_SLUG: %q\n", targetRepoSlug))
+	} else if trialMode && trialLogicalRepoSlug != "" {
+		safeOutputsEnvLog.Printf("Setting target repo slug from trial mode: %s", trialLogicalRepoSlug)
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_TARGET_REPO_SLUG: %q\n", trialLogicalRepoSlug))
+	}
+
+	return customEnvVars
+}
+
+// buildStandardSafeOutputEnvVars builds the standard set of environment variables
+// that all safe-output job builders need: metadata + staged/target repo handling
+// This reduces duplication in safe-output job builders
+func (c *Compiler) buildStandardSafeOutputEnvVars(data *WorkflowData, targetRepoSlug string) []string {
+	var customEnvVars []string
+
+	// Add workflow metadata (name, source, and tracker-id)
+	customEnvVars = append(customEnvVars, buildWorkflowMetadataEnvVarsWithTrackerID(data.Name, data.Source, data.TrackerID)...)
+
+	// Add engine metadata (id, version, model) for XML comment marker
+	customEnvVars = append(customEnvVars, buildEngineMetadataEnvVars(data.EngineConfig)...)
+
+	// Add common safe output job environment variables (staged/target repo)
+	customEnvVars = append(customEnvVars, buildSafeOutputJobEnvVars(
+		c.trialMode,
+		c.trialLogicalRepoSlug,
+		data.SafeOutputs.Staged,
+		targetRepoSlug,
+	)...)
+
+	// Add messages config if present
+	if data.SafeOutputs.Messages != nil {
+		messagesJSON, err := serializeMessagesConfig(data.SafeOutputs.Messages)
+		if err != nil {
+			safeOutputsEnvLog.Printf("Warning: failed to serialize messages config: %v", err)
+		} else if messagesJSON != "" {
+			customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_SAFE_OUTPUT_MESSAGES: %q\n", messagesJSON))
+		}
+	}
+
+	return customEnvVars
+}
+
+// buildStepLevelSafeOutputEnvVars builds environment variables for consolidated safe output steps
+// This excludes variables that are already set at the job level in consolidated jobs
+func (c *Compiler) buildStepLevelSafeOutputEnvVars(data *WorkflowData, targetRepoSlug string) []string {
+	var customEnvVars []string
+
+	// Only add target repo slug if it's different from the job-level setting
+	// (i.e., this step has a specific target-repo config that overrides the global trial mode target)
+	if targetRepoSlug != "" {
+		// Step-specific target repo overrides job-level setting
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_TARGET_REPO_SLUG: %q\n", targetRepoSlug))
+	} else if !c.trialMode && data.SafeOutputs.Staged {
+		// Step needs staged flag but there's no job-level target repo (not in trial mode)
+		// Job level only sets this if trialMode is true
+		customEnvVars = append(customEnvVars, "          GH_AW_SAFE_OUTPUTS_STAGED: \"true\"\n")
+	}
+
+	// Note: The following are now set at job level and should NOT be included here:
+	// - GH_AW_WORKFLOW_NAME
+	// - GH_AW_WORKFLOW_SOURCE
+	// - GH_AW_WORKFLOW_SOURCE_URL
+	// - GH_AW_TRACKER_ID
+	// - GH_AW_ENGINE_ID
+	// - GH_AW_ENGINE_VERSION
+	// - GH_AW_ENGINE_MODEL
+	// - GH_AW_SAFE_OUTPUTS_STAGED (if in trial mode)
+	// - GH_AW_TARGET_REPO_SLUG (if in trial mode and no step override)
+	// - GH_AW_SAFE_OUTPUT_MESSAGES
+
+	return customEnvVars
+}
+
+// buildEngineMetadataEnvVars builds engine metadata environment variables (id, version, model)
+// These are used by the JavaScript footer generation to create XML comment markers for traceability
+func buildEngineMetadataEnvVars(engineConfig *EngineConfig) []string {
+	var customEnvVars []string
+
+	if engineConfig == nil {
+		return customEnvVars
+	}
+
+	safeOutputsEnvLog.Printf("Building engine metadata env vars: id=%s, version=%s", engineConfig.ID, engineConfig.Version)
+
+	// Add engine ID if present
+	if engineConfig.ID != "" {
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_ENGINE_ID: %q\n", engineConfig.ID))
+	}
+
+	// Add engine version if present
+	if engineConfig.Version != "" {
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_ENGINE_VERSION: %q\n", engineConfig.Version))
+	}
+
+	// Add engine model if present
+	if engineConfig.Model != "" {
+		customEnvVars = append(customEnvVars, fmt.Sprintf("          GH_AW_ENGINE_MODEL: %q\n", engineConfig.Model))
+	}
+
+	return customEnvVars
+}
+
+// ========================================
+// Safe Output Environment Helpers
+// ========================================
+
+// addCustomSafeOutputEnvVars adds custom environment variables to safe output job steps
+func (c *Compiler) addCustomSafeOutputEnvVars(steps *[]string, data *WorkflowData) {
+	if data.SafeOutputs != nil && len(data.SafeOutputs.Env) > 0 {
+		for key, value := range data.SafeOutputs.Env {
+			*steps = append(*steps, fmt.Sprintf("          %s: %s\n", key, value))
+		}
+	}
+}
+
+// addSafeOutputGitHubTokenForConfig adds github-token to the with section, preferring per-config token over global
+// Uses precedence: config token > safe-outputs global github-token > GH_AW_GITHUB_TOKEN || GITHUB_TOKEN
+func (c *Compiler) addSafeOutputGitHubTokenForConfig(steps *[]string, data *WorkflowData, configToken string) {
+	var safeOutputsToken string
+	if data.SafeOutputs != nil {
+		safeOutputsToken = data.SafeOutputs.GitHubToken
+	}
+
+	// If app is configured, use app token
+	if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp != nil {
+		*steps = append(*steps, "          github-token: ${{ steps.safe-outputs-app-token.outputs.token }}\n")
+		return
+	}
+
+	// Choose the first non-empty custom token for precedence
+	effectiveCustomToken := configToken
+	if effectiveCustomToken == "" {
+		effectiveCustomToken = safeOutputsToken
+	}
+
+	// Get effective token
+	effectiveToken := getEffectiveSafeOutputGitHubToken(effectiveCustomToken)
+	*steps = append(*steps, fmt.Sprintf("          github-token: %s\n", effectiveToken))
+}
+
+// addSafeOutputCopilotGitHubTokenForConfig adds github-token to the with section for Copilot-related operations
+// Uses precedence: config token > safe-outputs global github-token > COPILOT_GITHUB_TOKEN
+func (c *Compiler) addSafeOutputCopilotGitHubTokenForConfig(steps *[]string, data *WorkflowData, configToken string) {
+	var safeOutputsToken string
+	if data.SafeOutputs != nil {
+		safeOutputsToken = data.SafeOutputs.GitHubToken
+	}
+
+	// If app is configured, use app token
+	if data.SafeOutputs != nil && data.SafeOutputs.GitHubApp != nil {
+		*steps = append(*steps, "          github-token: ${{ steps.safe-outputs-app-token.outputs.token }}\n")
+		return
+	}
+
+	// Choose the first non-empty custom token for precedence
+	effectiveCustomToken := configToken
+	if effectiveCustomToken == "" {
+		effectiveCustomToken = safeOutputsToken
+	}
+
+	// Get effective token
+	effectiveToken := getEffectiveCopilotRequestsToken(effectiveCustomToken)
+	*steps = append(*steps, fmt.Sprintf("          github-token: %s\n", effectiveToken))
+}
+
+// addSafeOutputAgentGitHubTokenForConfig adds github-token to the with section for agent assignment operations
+// Uses precedence: config token > safe-outputs token > GH_AW_AGENT_TOKEN || GH_AW_GITHUB_TOKEN || GITHUB_TOKEN
+// This is specifically for assign-to-agent operations which require elevated permissions.
+//
+// Note: GitHub App tokens are intentionally NOT used here, even when github-app: is configured.
+// The Copilot assignment API only accepts PATs (fine-grained or classic), not GitHub App
+// installation tokens. Callers must provide an explicit github-token or rely on GH_AW_AGENT_TOKEN.
+func (c *Compiler) addSafeOutputAgentGitHubTokenForConfig(steps *[]string, data *WorkflowData, configToken string) {
+	// Get safe-outputs level token
+	var safeOutputsToken string
+	if data.SafeOutputs != nil {
+		safeOutputsToken = data.SafeOutputs.GitHubToken
+	}
+
+	// Choose the first non-empty custom token for precedence
+	effectiveCustomToken := configToken
+	if effectiveCustomToken == "" {
+		effectiveCustomToken = safeOutputsToken
+	}
+
+	// Get effective token - falls back to ${{ secrets.GH_AW_AGENT_TOKEN || secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}
+	// when no explicit token is provided. GitHub App tokens are never used here because the
+	// Copilot assignment API rejects them.
+	effectiveToken := getEffectiveCopilotCodingAgentGitHubToken(effectiveCustomToken)
+	*steps = append(*steps, fmt.Sprintf("          github-token: %s\n", effectiveToken))
+}
+
+// buildAllowedReposEnvVar builds an allowed-repos environment variable line for safe-output jobs.
+// envVarName should be the full env var name like "GH_AW_ALLOWED_REPOS".
+// Returns an empty slice if allowedRepos is empty.
+func buildAllowedReposEnvVar(envVarName string, allowedRepos []string) []string {
+	if len(allowedRepos) == 0 {
+		return nil
+	}
+	reposStr := strings.Join(allowedRepos, ",")
+	return []string{fmt.Sprintf("          %s: %q\n", envVarName, reposStr)}
+}