From 9e8ab19d4ab3920a4f84e4392f8af253cf2d22fd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 2 May 2026 01:04:40 +0000 Subject: [PATCH 1/2] Initial plan From 7a2019f39eb33cb1d31e806052e3143e04397b84 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 2 May 2026 01:17:12 +0000 Subject: [PATCH 2/2] refactor: semantic function clustering - GitHub App helpers and slice utilities - Add resolveGitHubAppFromImports helper to eliminate ~12 lines of duplicated JSON-parse-and-validate logic shared between resolveTopLevelGitHubApp and resolveActivationGitHubApp - Move 4 activation GitHub app/token functions from role_checks.go to workflow_github_app.go, co-locating all GitHub App resolution logic - Remove unused encoding/json and parser imports from role_checks.go - Create slice_helpers.go with mergeUnique and excludeFromSlice, moved from runtime_definitions.go where they were outliers Agent-Logs-Url: https://github.com/github/gh-aw/sessions/4aeebc97-bca4-4e92-a53c-0255cfdca3ad Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/workflow/role_checks.go | 72 ---------------------- pkg/workflow/runtime_definitions.go | 40 ------------ pkg/workflow/slice_helpers.go | 53 ++++++++++++++++ pkg/workflow/workflow_github_app.go | 95 ++++++++++++++++++++++++++--- 4 files changed, 140 insertions(+), 120 deletions(-) create mode 100644 pkg/workflow/slice_helpers.go diff --git a/pkg/workflow/role_checks.go b/pkg/workflow/role_checks.go index cc6451d321e..36908c4cf61 100644 --- a/pkg/workflow/role_checks.go +++ b/pkg/workflow/role_checks.go @@ -1,7 +1,6 @@ package workflow import ( - "encoding/json" "fmt" "slices" "sort" @@ -9,7 +8,6 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" - "github.com/github/gh-aw/pkg/parser" "github.com/github/gh-aw/pkg/sliceutil" ) @@ -567,73 +565,3 @@ func (c *Compiler) mergeSkipBots(topSkipBots []string, importedSkipBots []string func (c *Compiler) mergeBots(topBots []string, importedBots []string) []string { return mergeStringSlicesDedup(topBots, importedBots, "bots") } - -// extractActivationGitHubToken extracts the 'github-token' field from the 'on:' section of frontmatter. -// This token is used for pre-activation reactions and activation status comments. -func (c *Compiler) extractActivationGitHubToken(frontmatter map[string]any) string { - if onValue, exists := frontmatter["on"]; exists { - if onMap, ok := onValue.(map[string]any); ok { - if tokenValue, hasToken := onMap["github-token"]; hasToken { - if tokenStr, ok := tokenValue.(string); ok { - roleLog.Printf("Extracted activation github-token from on section") - return tokenStr - } - } - } - } - return "" -} - -// extractActivationGitHubApp extracts the 'github-app' field from the 'on:' section of frontmatter. -// When configured, a GitHub App installation access token is minted for use in reactions and status comments. -func (c *Compiler) extractActivationGitHubApp(frontmatter map[string]any) *GitHubAppConfig { - if onValue, exists := frontmatter["on"]; exists { - if onMap, ok := onValue.(map[string]any); ok { - if appValue, hasApp := onMap["github-app"]; hasApp { - if appMap, ok := appValue.(map[string]any); ok { - roleLog.Printf("Extracted activation github-app from on section") - return parseAppConfig(appMap) - } - } - } - } - return nil -} - -// resolveActivationGitHubToken returns the GitHub token to use for activation operations -// (reactions, status comments, skip-if checks). Precedence: -// 1. Current workflow's on.github-token (explicit override wins) -// 2. First on.github-token found across imported shared workflows -// 3. Empty string (use default GITHUB_TOKEN) -func (c *Compiler) resolveActivationGitHubToken(frontmatter map[string]any, importsResult *parser.ImportsResult) string { - if token := c.extractActivationGitHubToken(frontmatter); token != "" { - return token - } - if importsResult != nil && importsResult.MergedActivationGitHubToken != "" { - roleLog.Print("Using on.github-token from imported shared workflow") - return importsResult.MergedActivationGitHubToken - } - return "" -} - -// resolveActivationGitHubApp returns the GitHub App config to use for activation operations -// (reactions, status comments, skip-if checks). Precedence: -// 1. Current workflow's on.github-app (explicit override wins) -// 2. First on.github-app found across imported shared workflows -// 3. Nil (use default GITHUB_TOKEN) -func (c *Compiler) resolveActivationGitHubApp(frontmatter map[string]any, importsResult *parser.ImportsResult) *GitHubAppConfig { - if app := c.extractActivationGitHubApp(frontmatter); app != nil { - return app - } - if importsResult != nil && importsResult.MergedActivationGitHubApp != "" { - var appMap map[string]any - if err := json.Unmarshal([]byte(importsResult.MergedActivationGitHubApp), &appMap); err == nil { - app := parseAppConfig(appMap) - if app.AppID != "" && app.PrivateKey != "" { - roleLog.Print("Using on.github-app from imported shared workflow") - return app - } - } - } - return nil -} diff --git a/pkg/workflow/runtime_definitions.go b/pkg/workflow/runtime_definitions.go index 669ed94f9ae..a8d691f145a 100644 --- a/pkg/workflow/runtime_definitions.go +++ b/pkg/workflow/runtime_definitions.go @@ -237,46 +237,6 @@ func getDotFolderExcludes(excludeFiles []string) []string { return result } -// excludeFromSlice returns a new slice containing the items from base -// that do not appear in the exclude set. Order of remaining items is preserved. -// Always returns a fresh slice (never aliases base) even when no items are removed. -func excludeFromSlice(base []string, exclude ...string) []string { - if len(exclude) == 0 { - return append([]string(nil), base...) - } - excluded := make(map[string]bool, len(exclude)) - for _, v := range exclude { - excluded[v] = true - } - result := make([]string, 0, len(base)) - for _, v := range base { - if !excluded[v] { - result = append(result, v) - } - } - return result -} - -// mergeUnique returns a deduplicated slice that starts with base and appends any -// items from extra that are not already present in base. Order is preserved. -func mergeUnique(base []string, extra ...string) []string { - seen := make(map[string]bool, len(base)+len(extra)) - result := make([]string, 0, len(base)+len(extra)) - for _, v := range base { - if !seen[v] { - seen[v] = true - result = append(result, v) - } - } - for _, v := range extra { - if !seen[v] { - seen[v] = true - result = append(result, v) - } - } - return result -} - // findRuntimeByID finds a runtime configuration by its ID func findRuntimeByID(id string) *Runtime { runtimeDefLog.Printf("Finding runtime by ID: %s", id) diff --git a/pkg/workflow/slice_helpers.go b/pkg/workflow/slice_helpers.go new file mode 100644 index 00000000000..69a9c535da8 --- /dev/null +++ b/pkg/workflow/slice_helpers.go @@ -0,0 +1,53 @@ +// This file provides generic slice utilities. +// +// These utilities are used throughout the workflow compilation process +// to safely manipulate string slices. They complement the map utilities +// in map_helpers.go. +// +// # Key Functions +// +// Slice Operations: +// - mergeUnique() - Merge two slices while preserving order and deduplicating +// - excludeFromSlice() - Create a new slice excluding specified values + +package workflow + +// excludeFromSlice returns a new slice containing the items from base +// that do not appear in the exclude set. Order of remaining items is preserved. +// Always returns a fresh slice (never aliases base) even when no items are removed. +func excludeFromSlice(base []string, exclude ...string) []string { + if len(exclude) == 0 { + return append([]string(nil), base...) + } + excluded := make(map[string]bool, len(exclude)) + for _, v := range exclude { + excluded[v] = true + } + result := make([]string, 0, len(base)) + for _, v := range base { + if !excluded[v] { + result = append(result, v) + } + } + return result +} + +// mergeUnique returns a deduplicated slice that starts with base and appends any +// items from extra that are not already present in base. Order is preserved. +func mergeUnique(base []string, extra ...string) []string { + seen := make(map[string]bool, len(base)+len(extra)) + result := make([]string, 0, len(base)+len(extra)) + for _, v := range base { + if !seen[v] { + seen[v] = true + result = append(result, v) + } + } + for _, v := range extra { + if !seen[v] { + seen[v] = true + result = append(result, v) + } + } + return result +} diff --git a/pkg/workflow/workflow_github_app.go b/pkg/workflow/workflow_github_app.go index c3fe464a0bf..5a0b103d317 100644 --- a/pkg/workflow/workflow_github_app.go +++ b/pkg/workflow/workflow_github_app.go @@ -9,6 +9,23 @@ import ( var workflowGitHubAppLog = logger.New("workflow:workflow_github_app") +// resolveGitHubAppFromImports parses a GitHubAppConfig from a JSON-encoded +// merged import string. Returns nil if the string is empty, unparseable, or incomplete. +func resolveGitHubAppFromImports(mergedJSON string) *GitHubAppConfig { + if mergedJSON == "" { + return nil + } + var appMap map[string]any + if err := json.Unmarshal([]byte(mergedJSON), &appMap); err != nil { + return nil + } + app := parseAppConfig(appMap) + if app.AppID != "" && app.PrivateKey != "" { + return app + } + return nil +} + // extractTopLevelGitHubApp extracts the 'github-app' field from the top-level frontmatter. // This provides a single GitHub App configuration that serves as a fallback for all nested // github-app token minting operations (on, safe-outputs, checkout, tools.github, dependencies). @@ -37,14 +54,10 @@ func resolveTopLevelGitHubApp(frontmatter map[string]any, importsResult *parser. if app := extractTopLevelGitHubApp(frontmatter); app != nil { return app } - if importsResult != nil && importsResult.MergedTopLevelGitHubApp != "" { - var appMap map[string]any - if err := json.Unmarshal([]byte(importsResult.MergedTopLevelGitHubApp), &appMap); err == nil { - app := parseAppConfig(appMap) - if app.AppID != "" && app.PrivateKey != "" { - workflowGitHubAppLog.Print("Using top-level github-app from imported shared workflow") - return app - } + if importsResult != nil { + if app := resolveGitHubAppFromImports(importsResult.MergedTopLevelGitHubApp); app != nil { + workflowGitHubAppLog.Print("Using top-level github-app from imported shared workflow") + return app } } return nil @@ -130,3 +143,69 @@ func applyTopLevelGitHubAppFallbacks(data *WorkflowData) { } } } + +// extractActivationGitHubToken extracts the 'github-token' field from the 'on:' section of frontmatter. +// This token is used for pre-activation reactions and activation status comments. +func (c *Compiler) extractActivationGitHubToken(frontmatter map[string]any) string { + if onValue, exists := frontmatter["on"]; exists { + if onMap, ok := onValue.(map[string]any); ok { + if tokenValue, hasToken := onMap["github-token"]; hasToken { + if tokenStr, ok := tokenValue.(string); ok { + workflowGitHubAppLog.Printf("Extracted activation github-token from on section") + return tokenStr + } + } + } + } + return "" +} + +// extractActivationGitHubApp extracts the 'github-app' field from the 'on:' section of frontmatter. +// When configured, a GitHub App installation access token is minted for use in reactions and status comments. +func (c *Compiler) extractActivationGitHubApp(frontmatter map[string]any) *GitHubAppConfig { + if onValue, exists := frontmatter["on"]; exists { + if onMap, ok := onValue.(map[string]any); ok { + if appValue, hasApp := onMap["github-app"]; hasApp { + if appMap, ok := appValue.(map[string]any); ok { + workflowGitHubAppLog.Printf("Extracted activation github-app from on section") + return parseAppConfig(appMap) + } + } + } + } + return nil +} + +// resolveActivationGitHubToken returns the GitHub token to use for activation operations +// (reactions, status comments, skip-if checks). Precedence: +// 1. Current workflow's on.github-token (explicit override wins) +// 2. First on.github-token found across imported shared workflows +// 3. Empty string (use default GITHUB_TOKEN) +func (c *Compiler) resolveActivationGitHubToken(frontmatter map[string]any, importsResult *parser.ImportsResult) string { + if token := c.extractActivationGitHubToken(frontmatter); token != "" { + return token + } + if importsResult != nil && importsResult.MergedActivationGitHubToken != "" { + workflowGitHubAppLog.Print("Using on.github-token from imported shared workflow") + return importsResult.MergedActivationGitHubToken + } + return "" +} + +// resolveActivationGitHubApp returns the GitHub App config to use for activation operations +// (reactions, status comments, skip-if checks). Precedence: +// 1. Current workflow's on.github-app (explicit override wins) +// 2. First on.github-app found across imported shared workflows +// 3. Nil (use default GITHUB_TOKEN) +func (c *Compiler) resolveActivationGitHubApp(frontmatter map[string]any, importsResult *parser.ImportsResult) *GitHubAppConfig { + if app := c.extractActivationGitHubApp(frontmatter); app != nil { + return app + } + if importsResult != nil { + if app := resolveGitHubAppFromImports(importsResult.MergedActivationGitHubApp); app != nil { + workflowGitHubAppLog.Print("Using on.github-app from imported shared workflow") + return app + } + } + return nil +}