diff --git a/pkg/cli/add_interactive_engine.go b/pkg/cli/add_interactive_engine.go index 47a9afce748..cfdad05604d 100644 --- a/pkg/cli/add_interactive_engine.go +++ b/pkg/cli/add_interactive_engine.go @@ -133,7 +133,7 @@ func (c *AddInteractiveConfig) selectAIEngineAndKey() error { func (c *AddInteractiveConfig) collectAPIKey(engine string) error { addInteractiveLog.Printf("Collecting API key for engine: %s", engine) - // Use the unified CheckAndCollectEngineSecrets function + // Use the unified checkAndEnsureEngineSecrets function config := EngineSecretConfig{ RepoSlug: c.RepoOverride, Engine: engine, @@ -143,7 +143,7 @@ func (c *AddInteractiveConfig) collectAPIKey(engine string) error { IncludeOptional: false, } - if err := CheckAndCollectEngineSecrets(config); err != nil { + if err := checkAndEnsureEngineSecretsForEngine(config); err != nil { return err } diff --git a/pkg/cli/add_interactive_secrets.go b/pkg/cli/add_interactive_secrets.go index 02130a3b5ea..26121fe3e60 100644 --- a/pkg/cli/add_interactive_secrets.go +++ b/pkg/cli/add_interactive_secrets.go @@ -5,7 +5,6 @@ import ( "os" "strings" - "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/workflow" ) @@ -54,29 +53,20 @@ func (c *AddInteractiveConfig) addRepositorySecret(name, value string) error { func (c *AddInteractiveConfig) getSecretInfo() (name string, value string, err error) { addInteractiveLog.Printf("Getting secret info for engine: %s", c.EngineOverride) - opt := constants.GetEngineOption(c.EngineOverride) - if opt == nil { - return "", "", fmt.Errorf("unknown engine: %s", c.EngineOverride) - } - - name = opt.SecretName - - // If secret already exists in repo, we don't need a value - if c.existingSecrets[name] { - addInteractiveLog.Printf("Secret %s already exists in repository", name) - return name, "", nil + secretName, secretValue, existsInRepo, err := GetEngineSecretNameAndValue(c.EngineOverride, c.existingSecrets) + if err != nil { + return "", "", err } - // Get value from environment variable (use EnvVarName if specified, otherwise SecretName) - envVar := opt.SecretName - if opt.EnvVarName != "" { - envVar = opt.EnvVarName + // If secret exists in repo, return early + if existsInRepo { + return secretName, "", nil } - value = os.Getenv(envVar) - if value == "" { + // If value not found in environment, return error + if secretValue == "" { return "", "", fmt.Errorf("API key not found for engine %s", c.EngineOverride) } - return name, value, nil + return secretName, secretValue, nil } diff --git a/pkg/cli/engine_secrets.go b/pkg/cli/engine_secrets.go index 22cfdaaef2a..2f57f5b6a03 100644 --- a/pkg/cli/engine_secrets.go +++ b/pkg/cli/engine_secrets.go @@ -45,9 +45,9 @@ type EngineSecretConfig struct { IncludeOptional bool } -// GetRequiredSecretsForEngine returns all secrets needed for a specific engine. +// getSecretRequirementsForEngine returns all secrets needed for a specific engine. // This combines engine-specific secrets with optional system-level secrets. -func GetRequiredSecretsForEngine(engine string, includeSystemSecrets bool, includeOptional bool) []SecretRequirement { +func getSecretRequirementsForEngine(engine string, includeSystemSecrets bool, includeOptional bool) []SecretRequirement { engineSecretsLog.Printf("Getting required secrets for engine: %s (system=%v, optional=%v)", engine, includeSystemSecrets, includeOptional) var requirements []SecretRequirement @@ -101,13 +101,40 @@ func getEngineSecretDescription(opt *constants.EngineOption) string { } } -// CheckAndCollectEngineSecrets is the unified entry point for checking and collecting engine secrets. +// getMissingRequiredSecrets filters requirements to return only missing required secrets. +// It skips optional secrets and checks both primary and alternative secret names. +func getMissingRequiredSecrets(requirements []SecretRequirement, existingSecrets map[string]bool) []SecretRequirement { + var missing []SecretRequirement + for _, req := range requirements { + // Skip optional secrets - we only care about required ones + if req.Optional { + continue + } + + exists := existingSecrets[req.Name] + if !exists { + // Check alternatives + for _, alt := range req.AlternativeEnvVars { + if existingSecrets[alt] { + exists = true + break + } + } + } + if !exists { + missing = append(missing, req) + } + } + return missing +} + +// checkAndEnsureEngineSecretsForEngine is the unified entry point for checking and collecting engine secrets. // It checks existing secrets in the repository and environment, and prompts for missing ones. -func CheckAndCollectEngineSecrets(config EngineSecretConfig) error { +func checkAndEnsureEngineSecretsForEngine(config EngineSecretConfig) error { engineSecretsLog.Printf("Checking and collecting secrets for engine: %s in repo: %s", config.Engine, config.RepoSlug) // Get required secrets for the engine - requirements := GetRequiredSecretsForEngine(config.Engine, config.IncludeSystemSecrets, config.IncludeOptional) + requirements := getSecretRequirementsForEngine(config.Engine, config.IncludeSystemSecrets, config.IncludeOptional) // Check each requirement for _, req := range requirements { @@ -418,8 +445,8 @@ func stringContainsSecretName(output, secretName string) bool { return false } -// CheckExistingSecretsInRepo checks which secrets exist in the repository -func CheckExistingSecretsInRepo(repoSlug string) (map[string]bool, error) { +// getExistingSecretsInRepo checks which secrets exist in the repository +func getExistingSecretsInRepo(repoSlug string) (map[string]bool, error) { engineSecretsLog.Printf("Checking existing secrets for repo: %s", repoSlug) existingSecrets := make(map[string]bool) @@ -444,8 +471,57 @@ func CheckExistingSecretsInRepo(repoSlug string) (map[string]bool, error) { return existingSecrets, nil } -// DisplayMissingSecrets shows information about missing secrets with setup instructions -func DisplayMissingSecrets(requirements []SecretRequirement, repoSlug string, existingSecrets map[string]bool) { +// GetEngineSecretNameAndValue returns the secret name and value for an engine. +// It checks if the secret exists in the repository and retrieves the value from environment if needed. +// Returns: secretName, secretValue (empty if exists in repo or not in env), existsInRepo, error +func GetEngineSecretNameAndValue(engine string, existingSecrets map[string]bool) (string, string, bool, error) { + engineSecretsLog.Printf("Getting secret name and value for engine: %s", engine) + + opt := constants.GetEngineOption(engine) + if opt == nil { + return "", "", false, fmt.Errorf("unknown engine: %s", engine) + } + + secretName := opt.SecretName + + // Check if secret already exists in repository + if existingSecrets[secretName] { + engineSecretsLog.Printf("Secret %s already exists in repository", secretName) + return secretName, "", true, nil + } + + // Check alternative secret names in repository + for _, alt := range opt.AlternativeSecrets { + if existingSecrets[alt] { + engineSecretsLog.Printf("Alternative secret %s exists in repository", alt) + return secretName, "", true, nil + } + } + + // Get value from environment variable + // Use EnvVarName if specified, otherwise use SecretName + envVar := opt.SecretName + if opt.EnvVarName != "" { + envVar = opt.EnvVarName + } + + value := os.Getenv(envVar) + if value == "" { + // Check alternative environment variables + for _, alt := range opt.AlternativeSecrets { + value = os.Getenv(alt) + if value != "" { + engineSecretsLog.Printf("Found secret in alternative env var: %s", alt) + break + } + } + } + + return secretName, value, false, nil +} + +// displayMissingSecrets shows information about missing secrets with setup instructions +func displayMissingSecrets(requirements []SecretRequirement, repoSlug string, existingSecrets map[string]bool) { var requiredMissing, optionalMissing []SecretRequirement for _, req := range requirements { @@ -499,7 +575,71 @@ func DisplayMissingSecrets(requirements []SecretRequirement, repoSlug string, ex } fmt.Fprintln(os.Stderr, "") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("For detailed token behavior and precedence, see the GitHub Tokens reference in the documentation.")) +} + +// displaySecretsSummaryTable displays a summary table of all required secrets with their status +func displaySecretsSummaryTable(requirements []SecretRequirement, existingSecrets map[string]bool) { + // Filter to only required secrets (not optional) + var requiredOnly []SecretRequirement + for _, req := range requirements { + if !req.Optional { + requiredOnly = append(requiredOnly, req) + } + } + + // If no required secrets, don't show the table + if len(requiredOnly) == 0 { + return + } + + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Required secrets summary:")) + fmt.Fprintln(os.Stderr, "") + + // Calculate max width for alignment + maxNameWidth := 0 + for _, req := range requiredOnly { + if len(req.Name) > maxNameWidth { + maxNameWidth = len(req.Name) + } + } + + // Display each required secret with status + for _, req := range requiredOnly { + // Check if secret exists + exists := existingSecrets[req.Name] + var altUsed string + if !exists { + // Check alternatives + for _, alt := range req.AlternativeEnvVars { + if existingSecrets[alt] { + exists = true + altUsed = alt + break + } + } + } + + // Format status indicator + var statusLine string + if exists { + if altUsed != "" { + statusLine = console.FormatSuccessMessage(fmt.Sprintf("(via %s)", altUsed)) + } else { + statusLine = console.FormatSuccessMessage("") + } + } else { + statusLine = console.FormatErrorMessage("") + } + + // Format secret name with padding + nameWithPadding := fmt.Sprintf("%-*s", maxNameWidth, req.Name) + + // Display the line + fmt.Fprintf(os.Stderr, " %s %s - %s\n", statusLine, nameWithPadding, req.WhenNeeded) + } + + fmt.Fprintln(os.Stderr, "") } // splitRepoSlug splits "owner/repo" into [owner, repo] diff --git a/pkg/cli/engine_secrets_test.go b/pkg/cli/engine_secrets_test.go index 6ec6e5c7c6c..f2f64cff7fe 100644 --- a/pkg/cli/engine_secrets_test.go +++ b/pkg/cli/engine_secrets_test.go @@ -3,6 +3,7 @@ package cli import ( + "os" "testing" "github.com/github/gh-aw/pkg/constants" @@ -34,9 +35,9 @@ func TestGetRequiredSecretsForEngine(t *testing.T) { engine: string(constants.CopilotEngine), includeSystemSecrets: true, includeOptional: false, - wantSecretNames: []string{"COPILOT_GITHUB_TOKEN", "GH_AW_GITHUB_TOKEN"}, - wantMinCount: 2, - wantMaxCount: 2, + wantSecretNames: []string{"COPILOT_GITHUB_TOKEN"}, // No system secrets since all are optional + wantMinCount: 1, + wantMaxCount: 1, }, { name: "copilot engine with optional secrets", @@ -69,7 +70,7 @@ func TestGetRequiredSecretsForEngine(t *testing.T) { name: "empty engine returns only system secrets when requested", engine: "", includeSystemSecrets: true, - includeOptional: false, + includeOptional: true, // Changed to true to include optional system secrets wantSecretNames: []string{"GH_AW_GITHUB_TOKEN"}, wantMinCount: 1, wantMaxCount: 5, @@ -87,7 +88,7 @@ func TestGetRequiredSecretsForEngine(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - requirements := GetRequiredSecretsForEngine(tt.engine, tt.includeSystemSecrets, tt.includeOptional) + requirements := getSecretRequirementsForEngine(tt.engine, tt.includeSystemSecrets, tt.includeOptional) assert.GreaterOrEqual(t, len(requirements), tt.wantMinCount, "Should have at least %d requirements", tt.wantMinCount) @@ -109,7 +110,7 @@ func TestGetRequiredSecretsForEngine(t *testing.T) { func TestGetRequiredSecretsForEngineAttributes(t *testing.T) { t.Run("copilot secret has correct attributes", func(t *testing.T) { - requirements := GetRequiredSecretsForEngine(string(constants.CopilotEngine), false, false) + requirements := getSecretRequirementsForEngine(string(constants.CopilotEngine), false, false) require.Len(t, requirements, 1, "Should have exactly one requirement") req := requirements[0] @@ -122,7 +123,7 @@ func TestGetRequiredSecretsForEngineAttributes(t *testing.T) { }) t.Run("claude secret has alternative env vars", func(t *testing.T) { - requirements := GetRequiredSecretsForEngine(string(constants.ClaudeEngine), false, false) + requirements := getSecretRequirementsForEngine(string(constants.ClaudeEngine), false, false) require.Len(t, requirements, 1, "Should have exactly one requirement") req := requirements[0] @@ -132,7 +133,7 @@ func TestGetRequiredSecretsForEngineAttributes(t *testing.T) { }) t.Run("system secrets are not engine secrets", func(t *testing.T) { - requirements := GetRequiredSecretsForEngine("", true, true) + requirements := getSecretRequirementsForEngine("", true, true) for _, req := range requirements { if req.Name == "GH_AW_GITHUB_TOKEN" { @@ -305,3 +306,289 @@ func TestEngineSecretConfigStructure(t *testing.T) { assert.False(t, config.IncludeOptional) }) } + +func TestGetEngineSecretNameAndValue(t *testing.T) { + // Save current env and restore after test + oldCopilotToken := os.Getenv("COPILOT_GITHUB_TOKEN") + oldAnthropicKey := os.Getenv("ANTHROPIC_API_KEY") + oldOpenAIKey := os.Getenv("OPENAI_API_KEY") + defer func() { + if oldCopilotToken != "" { + os.Setenv("COPILOT_GITHUB_TOKEN", oldCopilotToken) + } else { + os.Unsetenv("COPILOT_GITHUB_TOKEN") + } + if oldAnthropicKey != "" { + os.Setenv("ANTHROPIC_API_KEY", oldAnthropicKey) + } else { + os.Unsetenv("ANTHROPIC_API_KEY") + } + if oldOpenAIKey != "" { + os.Setenv("OPENAI_API_KEY", oldOpenAIKey) + } else { + os.Unsetenv("OPENAI_API_KEY") + } + }() + + t.Run("secret exists in repository", func(t *testing.T) { + existingSecrets := map[string]bool{ + "COPILOT_GITHUB_TOKEN": true, + } + + name, value, existsInRepo, err := GetEngineSecretNameAndValue("copilot", existingSecrets) + + require.NoError(t, err, "Should not error when secret exists in repo") + assert.Equal(t, "COPILOT_GITHUB_TOKEN", name) + assert.Empty(t, value, "Value should be empty when secret exists in repo") + assert.True(t, existsInRepo, "Should indicate secret exists in repo") + }) + + t.Run("secret found in environment", func(t *testing.T) { + os.Setenv("ANTHROPIC_API_KEY", "test-api-key-12345") + defer os.Unsetenv("ANTHROPIC_API_KEY") + + existingSecrets := map[string]bool{} + + name, value, existsInRepo, err := GetEngineSecretNameAndValue("claude", existingSecrets) + + require.NoError(t, err, "Should not error when secret in environment") + assert.Equal(t, "ANTHROPIC_API_KEY", name) + assert.Equal(t, "test-api-key-12345", value) + assert.False(t, existsInRepo, "Should indicate secret does not exist in repo") + }) + + t.Run("secret not in repo or environment", func(t *testing.T) { + os.Unsetenv("OPENAI_API_KEY") + + existingSecrets := map[string]bool{} + + name, value, existsInRepo, err := GetEngineSecretNameAndValue("codex", existingSecrets) + + require.NoError(t, err, "Should not error even when secret not found") + assert.Equal(t, "OPENAI_API_KEY", name) + assert.Empty(t, value, "Value should be empty when not found in env") + assert.False(t, existsInRepo, "Should indicate secret does not exist in repo") + }) + + t.Run("unknown engine returns error", func(t *testing.T) { + existingSecrets := map[string]bool{} + + _, _, _, err := GetEngineSecretNameAndValue("unknown-engine", existingSecrets) + + require.Error(t, err, "Should error for unknown engine") + assert.Contains(t, err.Error(), "unknown engine", "Error should mention unknown engine") + }) + + t.Run("alternative secret exists in repo", func(t *testing.T) { + // Claude has CLAUDE_CODE_OAUTH_TOKEN as alternative + existingSecrets := map[string]bool{ + "CLAUDE_CODE_OAUTH_TOKEN": true, // Alternative for ANTHROPIC_API_KEY + } + + name, value, existsInRepo, err := GetEngineSecretNameAndValue("claude", existingSecrets) + + require.NoError(t, err, "Should not error when alternative exists") + assert.Equal(t, "ANTHROPIC_API_KEY", name) + assert.Empty(t, value, "Value should be empty when alternative exists in repo") + assert.True(t, existsInRepo, "Should indicate secret exists via alternative") + }) + + t.Run("prefers primary secret over environment", func(t *testing.T) { + os.Setenv("COPILOT_GITHUB_TOKEN", "test-token-from-env") + defer os.Unsetenv("COPILOT_GITHUB_TOKEN") + + existingSecrets := map[string]bool{ + "COPILOT_GITHUB_TOKEN": true, + } + + name, value, existsInRepo, err := GetEngineSecretNameAndValue("copilot", existingSecrets) + + require.NoError(t, err, "Should not error") + assert.Equal(t, "COPILOT_GITHUB_TOKEN", name) + assert.Empty(t, value, "Should prefer existing repo secret over environment") + assert.True(t, existsInRepo, "Should indicate secret exists in repo") + }) +} + +func TestGetMissingRequiredSecrets(t *testing.T) { + t.Run("all secrets missing", func(t *testing.T) { + requirements := []SecretRequirement{ + {Name: "SECRET1", Optional: false}, + {Name: "SECRET2", Optional: false}, + {Name: "SECRET3", Optional: false}, + } + existingSecrets := map[string]bool{} + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Len(t, missing, 3, "Should have 3 missing secrets") + assert.Equal(t, "SECRET1", missing[0].Name) + assert.Equal(t, "SECRET2", missing[1].Name) + assert.Equal(t, "SECRET3", missing[2].Name) + }) + + t.Run("all secrets exist", func(t *testing.T) { + requirements := []SecretRequirement{ + {Name: "SECRET1", Optional: false}, + {Name: "SECRET2", Optional: false}, + } + existingSecrets := map[string]bool{ + "SECRET1": true, + "SECRET2": true, + } + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Empty(t, missing, "Should have no missing secrets") + }) + + t.Run("some secrets missing", func(t *testing.T) { + requirements := []SecretRequirement{ + {Name: "SECRET1", Optional: false}, + {Name: "SECRET2", Optional: false}, + {Name: "SECRET3", Optional: false}, + } + existingSecrets := map[string]bool{ + "SECRET1": true, + } + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Len(t, missing, 2, "Should have 2 missing secrets") + assert.Equal(t, "SECRET2", missing[0].Name) + assert.Equal(t, "SECRET3", missing[1].Name) + }) + + t.Run("optional secrets are skipped", func(t *testing.T) { + requirements := []SecretRequirement{ + {Name: "REQUIRED1", Optional: false}, + {Name: "OPTIONAL1", Optional: true}, + {Name: "REQUIRED2", Optional: false}, + {Name: "OPTIONAL2", Optional: true}, + } + existingSecrets := map[string]bool{} + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Len(t, missing, 2, "Should only include required secrets") + assert.Equal(t, "REQUIRED1", missing[0].Name) + assert.Equal(t, "REQUIRED2", missing[1].Name) + }) + + t.Run("alternative secret names work", func(t *testing.T) { + requirements := []SecretRequirement{ + { + Name: "PRIMARY_SECRET", + Optional: false, + AlternativeEnvVars: []string{"ALT_SECRET1", "ALT_SECRET2"}, + }, + {Name: "OTHER_SECRET", Optional: false}, + } + existingSecrets := map[string]bool{ + "ALT_SECRET1": true, // Alternative exists + } + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Len(t, missing, 1, "Should have 1 missing secret") + assert.Equal(t, "OTHER_SECRET", missing[0].Name, "Should not include PRIMARY_SECRET since alternative exists") + }) + + t.Run("alternative secret names - second alternative", func(t *testing.T) { + requirements := []SecretRequirement{ + { + Name: "PRIMARY_SECRET", + Optional: false, + AlternativeEnvVars: []string{"ALT_SECRET1", "ALT_SECRET2"}, + }, + } + existingSecrets := map[string]bool{ + "ALT_SECRET2": true, // Second alternative exists + } + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Empty(t, missing, "Should find second alternative") + }) + + t.Run("primary secret takes precedence over alternatives", func(t *testing.T) { + requirements := []SecretRequirement{ + { + Name: "PRIMARY_SECRET", + Optional: false, + AlternativeEnvVars: []string{"ALT_SECRET"}, + }, + } + existingSecrets := map[string]bool{ + "PRIMARY_SECRET": true, + "ALT_SECRET": true, + } + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Empty(t, missing, "Should not include secret when primary exists") + }) + + t.Run("empty requirements list", func(t *testing.T) { + requirements := []SecretRequirement{} + existingSecrets := map[string]bool{ + "SECRET1": true, + } + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Empty(t, missing, "Should return empty list for empty requirements") + }) + + t.Run("empty existing secrets map", func(t *testing.T) { + requirements := []SecretRequirement{ + {Name: "SECRET1", Optional: false}, + {Name: "SECRET2", Optional: false}, + } + existingSecrets := map[string]bool{} + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Len(t, missing, 2, "Should return all required secrets as missing") + }) + + t.Run("nil existing secrets map", func(t *testing.T) { + requirements := []SecretRequirement{ + {Name: "SECRET1", Optional: false}, + } + var existingSecrets map[string]bool // nil map + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Len(t, missing, 1, "Should handle nil map and return all as missing") + }) + + t.Run("mixed required and optional with alternatives", func(t *testing.T) { + requirements := []SecretRequirement{ + { + Name: "COPILOT_GITHUB_TOKEN", + Optional: false, + IsEngineSecret: true, + AlternativeEnvVars: []string{"GITHUB_TOKEN"}, + }, + { + Name: "GH_AW_GITHUB_TOKEN", + Optional: true, + }, + { + Name: "ANTHROPIC_API_KEY", + Optional: false, + IsEngineSecret: true, + AlternativeEnvVars: []string{"CLAUDE_API_KEY"}, + }, + } + existingSecrets := map[string]bool{ + "GITHUB_TOKEN": true, // Alternative for COPILOT_GITHUB_TOKEN + } + + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + assert.Len(t, missing, 1, "Should have 1 missing required secret") + assert.Equal(t, "ANTHROPIC_API_KEY", missing[0].Name, "Should only include ANTHROPIC_API_KEY") + }) +} diff --git a/pkg/cli/status_command.go b/pkg/cli/status_command.go index d98a5f4444e..b9269832424 100644 --- a/pkg/cli/status_command.go +++ b/pkg/cli/status_command.go @@ -1,7 +1,6 @@ package cli import ( - "bufio" "encoding/json" "fmt" "os" @@ -269,100 +268,6 @@ func calculateTimeRemaining(stopTimeStr string) string { } } -// StatusWorkflows shows status of workflows -// getMarkdownWorkflowFiles finds all markdown files in .github/workflows directory -func getMarkdownWorkflowFiles(workflowDir string) ([]string, error) { - // Use provided directory or default - workflowsDir := workflowDir - if workflowsDir == "" { - workflowsDir = getWorkflowsDir() - } - - if _, err := os.Stat(workflowsDir); os.IsNotExist(err) { - return nil, fmt.Errorf("no %s directory found", workflowsDir) - } - - // Find all markdown files in workflow directory - mdFiles, err := filepath.Glob(filepath.Join(workflowsDir, "*.md")) - if err != nil { - return nil, fmt.Errorf("failed to find workflow files: %w", err) - } - - // Filter out README.md files - mdFiles = filterWorkflowFiles(mdFiles) - - return mdFiles, nil -} - -// Helper functions - -func extractWorkflowNameFromFile(filePath string) (string, error) { - content, err := os.ReadFile(filePath) - if err != nil { - return "", err - } - - // Extract markdown content (excluding frontmatter) - result, err := parser.ExtractFrontmatterFromContent(string(content)) - if err != nil { - return "", err - } - - // Look for first H1 header - scanner := bufio.NewScanner(strings.NewReader(result.Markdown)) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if strings.HasPrefix(line, "# ") { - return strings.TrimSpace(line[2:]), nil - } - } - - // No H1 header found, generate default name from filename - baseName := filepath.Base(filePath) - baseName = strings.TrimSuffix(baseName, filepath.Ext(baseName)) - baseName = strings.ReplaceAll(baseName, "-", " ") - - // Capitalize first letter of each word - words := strings.Fields(baseName) - for i, word := range words { - if len(word) > 0 { - words[i] = strings.ToUpper(word[:1]) + word[1:] - } - } - - return strings.Join(words, " "), nil -} - -// extractEngineIDFromFile extracts the engine ID from a workflow file's frontmatter -func extractEngineIDFromFile(filePath string) string { - content, err := os.ReadFile(filePath) - if err != nil { - return "" // Return empty string if file cannot be read - } - - // Parse frontmatter - result, err := parser.ExtractFrontmatterFromContent(string(content)) - if err != nil { - return "" // Return empty string if frontmatter cannot be parsed - } - - // Use the workflow package's extractEngineConfig to handle both string and object formats - compiler := &workflow.Compiler{} - engineSetting, engineConfig := compiler.ExtractEngineConfig(result.Frontmatter) - - // If engine is specified, return the ID from the config - if engineConfig != nil && engineConfig.ID != "" { - return engineConfig.ID - } - - // If we have an engine setting string, return it - if engineSetting != "" { - return engineSetting - } - - return "copilot" // Default engine -} - // fetchLatestRunsByRef fetches the latest workflow run for each workflow from a specific ref (branch or tag) func fetchLatestRunsByRef(ref string, repoOverride string, verbose bool) (map[string]*WorkflowRun, error) { statusLog.Printf("Fetching latest workflow runs for ref: %s, repo: %s", ref, repoOverride) diff --git a/pkg/cli/tokens_bootstrap.go b/pkg/cli/tokens_bootstrap.go index 786276bd285..d3d3392175b 100644 --- a/pkg/cli/tokens_bootstrap.go +++ b/pkg/cli/tokens_bootstrap.go @@ -5,7 +5,6 @@ import ( "os" "github.com/github/gh-aw/pkg/console" - "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" "github.com/spf13/cobra" ) @@ -15,34 +14,40 @@ var tokensBootstrapLog = logger.New("cli:tokens_bootstrap") // newSecretsBootstrapSubcommand creates the `secrets bootstrap` subcommand func newSecretsBootstrapSubcommand() *cobra.Command { var engineFlag string + var nonInteractiveFlag bool cmd := &cobra.Command{ Use: "bootstrap", - Short: "Check and suggest setup for gh aw GitHub token secrets", - Long: `Check which recommended GitHub token secrets (like GH_AW_GITHUB_TOKEN) -are configured for the current repository, and print least-privilege setup -instructions for any that are missing. + Short: "Analyze workflows and set up required secrets", + Long: `Analyzes all workflows in the repository to determine which secrets +are required, checks which ones are already configured, and interactively +prompts for any missing required secrets. -This command is read-only: it does not create tokens or secrets for you. -Instead, it inspects repository secrets (using the GitHub CLI where -available) and prints the exact secrets to add and suggested scopes. +This command: +- Discovers all workflow files in .github/workflows/ +- Analyzes required secrets for each workflow's engine +- Checks which secrets already exist in the repository +- Interactively prompts for missing required secrets (unless --non-interactive) + +Only required secrets are prompted for. Optional secrets are not shown. For full details, including precedence rules, see the GitHub Tokens reference in the documentation.`, RunE: func(cmd *cobra.Command, args []string) error { repo, _ := cmd.Flags().GetString("repo") - return runTokensBootstrap(engineFlag, repo) + return runTokensBootstrap(engineFlag, repo, nonInteractiveFlag) }, } + cmd.Flags().BoolVar(&nonInteractiveFlag, "non-interactive", false, "Check secrets without prompting (display-only mode)") cmd.Flags().StringVarP(&engineFlag, "engine", "e", "", "Check tokens for specific engine (copilot, claude, codex)") addRepoFlag(cmd) return cmd } -func runTokensBootstrap(engine, repo string) error { - tokensBootstrapLog.Printf("Running tokens bootstrap: engine=%s, repo=%s", engine, repo) +func runTokensBootstrap(engine, repo string, nonInteractive bool) error { + tokensBootstrapLog.Printf("Running tokens bootstrap: engine=%s, repo=%s, nonInteractive=%v", engine, repo, nonInteractive) var repoSlug string var err error @@ -56,76 +61,97 @@ func runTokensBootstrap(engine, repo string) error { } } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Checking recommended gh-aw token secrets in %s...", repoSlug))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Analyzing workflows in %s...", repoSlug))) - // Use the unified GetRequiredSecretsForEngine function - var requirements []SecretRequirement - if engine != "" { - requirements = GetRequiredSecretsForEngine(engine, true, true) - tokensBootstrapLog.Printf("Checking tokens for specific engine: %s (%d tokens)", engine, len(requirements)) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Checking tokens for engine: %s", engine))) - } else { - // When no engine specified, get all engine secrets plus system secrets - requirements = GetRequiredSecretsForEngine("", true, true) - // Add all engine-specific secrets as optional, deduplicating by secret name - // (e.g., CopilotEngine and CopilotSDKEngine both use COPILOT_GITHUB_TOKEN) - seenSecrets := make(map[string]bool) - for _, req := range requirements { - seenSecrets[req.Name] = true - } - for _, opt := range constants.EngineOptions { - if seenSecrets[opt.SecretName] { - continue // Skip duplicate secret names - } - seenSecrets[opt.SecretName] = true - requirements = append(requirements, SecretRequirement{ - Name: opt.SecretName, - WhenNeeded: opt.WhenNeeded, - Description: getEngineSecretDescription(&opt), - Optional: true, // All engines are optional when no specific engine is selected - AlternativeEnvVars: opt.AlternativeSecrets, - KeyURL: opt.KeyURL, - IsEngineSecret: true, - EngineName: opt.Value, - }) - } - tokensBootstrapLog.Printf("Checking all recommended tokens: count=%d", len(requirements)) + // Discover workflows in the repository + requirements, err := getSecretRequirements(engine) + if err != nil { + return fmt.Errorf("failed to analyze workflows: %w", err) } + tokensBootstrapLog.Printf("Collected %d required secrets from workflows", len(requirements)) + // Check existing secrets in repository - existingSecrets, err := CheckExistingSecretsInRepo(repoSlug) + existingSecrets, err := getExistingSecretsInRepo(repoSlug) if err != nil { - return fmt.Errorf("unable to inspect repository secrets: %w", err) + // If we can't check existing secrets (e.g., no gh auth), continue with empty map + tokensBootstrapLog.Printf("Could not check existing secrets: %v", err) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Unable to check existing repository secrets. Will assume all secrets need to be configured.")) + existingSecrets = make(map[string]bool) } - // Check which secrets are missing - var missing []SecretRequirement - for _, req := range requirements { - exists := existingSecrets[req.Name] - if !exists { - // Check alternatives - for _, alt := range req.AlternativeEnvVars { - if existingSecrets[alt] { - exists = true - break - } - } - } - if !exists { - missing = append(missing, req) - } - } + // Filter to only required secrets that are missing + missing := getMissingRequiredSecrets(requirements, existingSecrets) + + // Always display summary table of all required secrets with their status + displaySecretsSummaryTable(requirements, existingSecrets) if len(missing) == 0 { - tokensBootstrapLog.Print("All required tokens present") - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("All recommended gh-aw token secrets are present in this repository.")) + tokensBootstrapLog.Print("All required secrets present") + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("All required secrets are configured.")) + return nil + } + + tokensBootstrapLog.Printf("Found %d missing required secrets", len(missing)) + + // In non-interactive mode, just display what's missing + if nonInteractive { + displayMissingSecrets(missing, repoSlug, existingSecrets) return nil } - tokensBootstrapLog.Printf("Found missing tokens: count=%d", len(missing)) + // Interactive mode: prompt for missing secrets + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d missing required secret(s). You will be prompted to provide them.", len(missing)))) + fmt.Fprintln(os.Stderr, "") + + config := EngineSecretConfig{ + RepoSlug: repoSlug, + ExistingSecrets: existingSecrets, + IncludeSystemSecrets: true, + IncludeOptional: false, + } + + // Prompt for each missing secret + for _, req := range missing { + if err := promptForSecret(req, config); err != nil { + return fmt.Errorf("failed to collect secret %s: %w", req.Name, err) + } + } - // Display missing secrets using the unified helper - DisplayMissingSecrets(missing, repoSlug, existingSecrets) + fmt.Fprintln(os.Stderr, "") + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("All required secrets have been configured.")) return nil } + +// getSecretRequirements discovers all workflows and collects their required secrets +func getSecretRequirements(engineFilter string) ([]SecretRequirement, error) { + tokensBootstrapLog.Printf("Discovering workflows (engine filter: %s)", engineFilter) + + var allRequirements []SecretRequirement + + // If engine is explicitly specified, we can bootstrap without workflows + if engineFilter != "" { + tokensBootstrapLog.Printf("Engine explicitly specified, bootstrapping for %s regardless of workflows", engineFilter) + // Get engine-specific secrets and system secrets (including optional) + allRequirements = getSecretRequirementsForEngine(engineFilter, true, true) + } else { + // Discover workflow files + workflowFiles, err := getMarkdownWorkflowFiles("") + if err != nil { + return nil, fmt.Errorf("failed to discover workflows: %w", err) + } + + if len(workflowFiles) == 0 { + return nil, fmt.Errorf("no workflow files found in .github/workflows/") + } + + tokensBootstrapLog.Printf("Found %d workflow files, extracting secrets", len(workflowFiles)) + + // Use getRequiredSecretsForWorkflows to collect and deduplicate secrets + allRequirements = getSecretsRequirementsForWorkflows(workflowFiles) + } + + tokensBootstrapLog.Printf("Returning %d deduplicated secret requirements", len(allRequirements)) + return allRequirements, nil +} diff --git a/pkg/cli/tokens_bootstrap_test.go b/pkg/cli/tokens_bootstrap_test.go new file mode 100644 index 00000000000..39c8ba84a3d --- /dev/null +++ b/pkg/cli/tokens_bootstrap_test.go @@ -0,0 +1,374 @@ +//go:build !integration + +package cli + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCollectRequiredSecretsFromWorkflows_WithTestFixtures(t *testing.T) { + // Create a temporary directory with test workflow files + tempDir := t.TempDir() + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err) + + // Create test workflow files with different engines + testWorkflows := map[string]string{ + "copilot-workflow.md": `--- +engine: copilot +on: push +--- +# Copilot Workflow +This workflow uses Copilot.`, + "claude-workflow.md": `--- +engine: claude +on: pull_request +--- +# Claude Workflow +This workflow uses Claude.`, + "codex-workflow.md": `--- +engine: codex +on: workflow_dispatch +--- +# Codex Workflow +This workflow uses Codex.`, + } + + for filename, content := range testWorkflows { + path := filepath.Join(workflowsDir, filename) + err := os.WriteFile(path, []byte(content), 0644) + require.NoError(t, err) + } + + // Save original directory and change to temp directory + originalDir, err := os.Getwd() + require.NoError(t, err) + defer func() { + _ = os.Chdir(originalDir) + }() + + err = os.Chdir(tempDir) + require.NoError(t, err) + + t.Run("discovers secrets from test workflows", func(t *testing.T) { + requirements, err := getSecretRequirements("") + + require.NoError(t, err, "Should successfully collect secrets from workflows") + require.NotEmpty(t, requirements, "Should find at least one required secret") + + // Should include system secrets + hasSystemSecret := false + for _, req := range requirements { + if req.Name == "GH_AW_GITHUB_TOKEN" { + hasSystemSecret = true + assert.False(t, req.IsEngineSecret, "System secret should not be marked as engine secret") + break + } + } + assert.True(t, hasSystemSecret, "Should include system secret GH_AW_GITHUB_TOKEN") + + // Should include engine secrets for copilot, claude, and codex + engineSecrets := make(map[string]bool) + for _, req := range requirements { + if req.IsEngineSecret { + engineSecrets[req.Name] = true + assert.NotEmpty(t, req.EngineName, "Engine secret should have engine name") + assert.False(t, req.Optional, "Engine secret %s should be required", req.Name) + } + } + + assert.True(t, engineSecrets["COPILOT_GITHUB_TOKEN"], "Should include COPILOT_GITHUB_TOKEN") + assert.True(t, engineSecrets["ANTHROPIC_API_KEY"], "Should include ANTHROPIC_API_KEY") + assert.True(t, engineSecrets["OPENAI_API_KEY"], "Should include OPENAI_API_KEY") + }) + + t.Run("filters by engine", func(t *testing.T) { + requirements, err := getSecretRequirements("copilot") + + require.NoError(t, err, "Should successfully collect secrets for copilot engine") + require.NotEmpty(t, requirements, "Should find required secrets") + + // Should only include copilot-related secrets (and system secrets) + hasOnlyCopilot := true + for _, req := range requirements { + if req.IsEngineSecret && req.Name != "COPILOT_GITHUB_TOKEN" { + hasOnlyCopilot = false + break + } + } + assert.True(t, hasOnlyCopilot, "Should only include Copilot secrets when filtering by copilot") + }) + + t.Run("handles no matching workflows gracefully", func(t *testing.T) { + requirements, err := getSecretRequirements("nonexistent-engine") + + require.NoError(t, err, "Should not error when no workflows match filter") + + // Should still return system secrets even if no engine workflows match + hasSystemSecret := false + for _, req := range requirements { + if req.Name == "GH_AW_GITHUB_TOKEN" { + hasSystemSecret = true + break + } + } + assert.True(t, hasSystemSecret, "Should include system secrets even with no matching engine workflows") + + // Should not include any engine secrets + for _, req := range requirements { + if req.IsEngineSecret { + t.Errorf("Should not include engine secret %s when no workflows match", req.Name) + } + } + }) +} + +func TestCollectRequiredSecretsFromWorkflows_NoWorkflowsDir(t *testing.T) { + // Save original working directory + originalDir, err := os.Getwd() + require.NoError(t, err) + defer func() { + _ = os.Chdir(originalDir) + }() + + // Create a temporary directory without .github/workflows + tmpDir := t.TempDir() + err = os.Chdir(tmpDir) + require.NoError(t, err) + + t.Run("fails when no engine specified", func(t *testing.T) { + _, err = getSecretRequirements("") + require.Error(t, err, "Should error when no workflows directory exists and no engine specified") + assert.Contains(t, err.Error(), "failed to discover workflows", "Error should indicate workflow discovery failed") + }) + + t.Run("succeeds when engine specified", func(t *testing.T) { + requirements, err := getSecretRequirements("copilot") + require.NoError(t, err, "Should not error when engine is explicitly specified") + require.NotEmpty(t, requirements, "Should return secrets for specified engine") + + // Should include system secrets + hasSystemSecret := false + hasCopilotSecret := false + for _, req := range requirements { + if req.Name == "GH_AW_GITHUB_TOKEN" { + hasSystemSecret = true + } + if req.Name == "COPILOT_GITHUB_TOKEN" { + hasCopilotSecret = true + } + } + assert.True(t, hasSystemSecret, "Should include system secret") + assert.True(t, hasCopilotSecret, "Should include Copilot secret when engine=copilot") + }) +} + +func TestCollectRequiredSecretsFromWorkflows_EmptyWorkflowsDir(t *testing.T) { + // Save original working directory + originalDir, err := os.Getwd() + require.NoError(t, err) + defer func() { + _ = os.Chdir(originalDir) + }() + + // Create a temporary directory with empty .github/workflows + tmpDir := t.TempDir() + workflowsDir := filepath.Join(tmpDir, ".github", "workflows") + err = os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err) + + err = os.Chdir(tmpDir) + require.NoError(t, err) + + t.Run("fails when no engine specified", func(t *testing.T) { + _, err = getSecretRequirements("") + require.Error(t, err, "Should error when no workflow files found and no engine specified") + assert.Contains(t, err.Error(), "no workflow files found", "Error should indicate no workflow files") + }) + + t.Run("succeeds when engine specified", func(t *testing.T) { + requirements, err := getSecretRequirements("claude") + require.NoError(t, err, "Should not error when engine is explicitly specified") + require.NotEmpty(t, requirements, "Should return secrets for specified engine") + + // Should include system secrets + hasSystemSecret := false + hasClaudeSecret := false + for _, req := range requirements { + if req.Name == "GH_AW_GITHUB_TOKEN" { + hasSystemSecret = true + } + if req.Name == "ANTHROPIC_API_KEY" { + hasClaudeSecret = true + } + } + assert.True(t, hasSystemSecret, "Should include system secret") + assert.True(t, hasClaudeSecret, "Should include Claude API key when engine=claude") + }) +} + +func TestCollectRequiredSecretsFromWorkflows_Deduplication(t *testing.T) { + // Create a temporary directory with multiple workflows using the same engine + tempDir := t.TempDir() + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err) + + // Create multiple workflows that use the same engine + for i := 1; i <= 3; i++ { + filename := filepath.Join(workflowsDir, fmt.Sprintf("copilot-workflow-%d.md", i)) + content := `--- +engine: copilot +on: push +--- +# Copilot Workflow +This workflow uses Copilot.` + err := os.WriteFile(filename, []byte(content), 0644) + require.NoError(t, err) + } + + // Save original directory and change to temp directory + originalDir, err := os.Getwd() + require.NoError(t, err) + defer func() { + _ = os.Chdir(originalDir) + }() + + err = os.Chdir(tempDir) + require.NoError(t, err) + + requirements, err := getSecretRequirements("") + require.NoError(t, err) + + // Count occurrences of each secret name + secretCounts := make(map[string]int) + for _, req := range requirements { + secretCounts[req.Name]++ + } + + // Verify no duplicates + for secretName, count := range secretCounts { + assert.Equal(t, 1, count, "Secret %s should appear exactly once, found %d times", secretName, count) + } + + // Should have system secrets + copilot token + // (System secrets include required and optional ones) + assert.GreaterOrEqual(t, len(secretCounts), 2, "Should have at least 2 unique secrets") + assert.Contains(t, secretCounts, "GH_AW_GITHUB_TOKEN", "Should include system token") + assert.Contains(t, secretCounts, "COPILOT_GITHUB_TOKEN", "Should include Copilot token") +} +func TestExtractEnginesFromWorkflows(t *testing.T) { + // Create a temporary directory for test workflow files + tempDir := t.TempDir() + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err) + + t.Run("extracts multiple unique engines", func(t *testing.T) { + // Create test workflow files + testWorkflows := map[string]string{ + "copilot-workflow.md": `--- +engine: copilot +on: push +--- +# Copilot Workflow`, + "claude-workflow.md": `--- +engine: claude +on: pull_request +--- +# Claude Workflow`, + "codex-workflow.md": `--- +engine: codex +on: workflow_dispatch +--- +# Codex Workflow`, + } + + var workflowFiles []string + for filename, content := range testWorkflows { + path := filepath.Join(workflowsDir, filename) + err := os.WriteFile(path, []byte(content), 0644) + require.NoError(t, err) + workflowFiles = append(workflowFiles, path) + } + + engines := extractEnginesFromWorkflows(workflowFiles) + + require.Len(t, engines, 3, "Should extract 3 unique engines") + + // Convert to map for easy checking + engineMap := make(map[string]bool) + for _, engine := range engines { + engineMap[engine] = true + } + + assert.True(t, engineMap["copilot"], "Should include copilot engine") + assert.True(t, engineMap["claude"], "Should include claude engine") + assert.True(t, engineMap["codex"], "Should include codex engine") + }) + + t.Run("deduplicates duplicate engines", func(t *testing.T) { + // Create multiple workflows with the same engine + var workflowFiles []string + for i := 1; i <= 3; i++ { + path := filepath.Join(workflowsDir, fmt.Sprintf("copilot-%d.md", i)) + content := `--- +engine: copilot +on: push +--- +# Copilot Workflow` + err := os.WriteFile(path, []byte(content), 0644) + require.NoError(t, err) + workflowFiles = append(workflowFiles, path) + } + + engines := extractEnginesFromWorkflows(workflowFiles) + + assert.Len(t, engines, 1, "Should deduplicate to 1 unique engine") + assert.Equal(t, "copilot", engines[0], "Should be copilot engine") + }) + + t.Run("handles empty workflow list", func(t *testing.T) { + engines := extractEnginesFromWorkflows([]string{}) + + assert.Empty(t, engines, "Should return empty slice for empty input") + }) + + t.Run("filters subset of workflows", func(t *testing.T) { + // Create workflows with different engines + copilotPath := filepath.Join(workflowsDir, "copilot-only.md") + err := os.WriteFile(copilotPath, []byte(`--- +engine: copilot +on: push +--- +# Copilot`), 0644) + require.NoError(t, err) + + claudePath := filepath.Join(workflowsDir, "claude-only.md") + err = os.WriteFile(claudePath, []byte(`--- +engine: claude +on: push +--- +# Claude`), 0644) + require.NoError(t, err) + + // Extract engines from only the copilot workflow + engines := extractEnginesFromWorkflows([]string{copilotPath}) + + assert.Len(t, engines, 1, "Should extract only 1 engine") + assert.Equal(t, "copilot", engines[0], "Should be copilot engine") + + // Extract engines from only the claude workflow + engines = extractEnginesFromWorkflows([]string{claudePath}) + + assert.Len(t, engines, 1, "Should extract only 1 engine") + assert.Equal(t, "claude", engines[0], "Should be claude engine") + }) +} diff --git a/pkg/cli/trial_command.go b/pkg/cli/trial_command.go index 72655cbbdb1..0b9002a9214 100644 --- a/pkg/cli/trial_command.go +++ b/pkg/cli/trial_command.go @@ -334,7 +334,7 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp // When no override is specified, the workflow will use its frontmatter engine and handle secrets during compilation if opts.EngineOverride != "" { // Check what secrets already exist in the repository - existingSecrets, err := CheckExistingSecretsInRepo(hostRepoSlug) + existingSecrets, err := getExistingSecretsInRepo(hostRepoSlug) if err != nil { trialLog.Printf("Warning: could not check existing secrets: %v", err) existingSecrets = make(map[string]bool) @@ -349,7 +349,7 @@ func RunWorkflowTrials(ctx context.Context, workflowSpecs []string, opts TrialOp IncludeSystemSecrets: false, IncludeOptional: false, } - if err := CheckAndCollectEngineSecrets(secretConfig); err != nil { + if err := checkAndEnsureEngineSecretsForEngine(secretConfig); err != nil { return fmt.Errorf("failed to configure engine secret: %w", err) } } diff --git a/pkg/cli/workflow_secrets.go b/pkg/cli/workflow_secrets.go new file mode 100644 index 00000000000..c13237cf281 --- /dev/null +++ b/pkg/cli/workflow_secrets.go @@ -0,0 +1,69 @@ +package cli + +import ( + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/logger" +) + +var workflowSecretsLog = logger.New("cli:workflow_secrets") + +// getSecretsRequirementsForWorkflows collects secrets from all provided workflow files +// and returns a deduplicated list including system secrets +func getSecretsRequirementsForWorkflows(workflowFiles []string) []SecretRequirement { + workflowSecretsLog.Printf("Collecting secrets from %d workflow files", len(workflowFiles)) + + var allRequirements []SecretRequirement + seenSecrets := make(map[string]bool) + + // Map getRequiredSecretsForWorkflow over all workflows and union results + for _, workflowFile := range workflowFiles { + secrets := getSecretRequirementsForWorkflow(workflowFile) + for _, req := range secrets { + if !seenSecrets[req.Name] { + seenSecrets[req.Name] = true + allRequirements = append(allRequirements, req) + } + } + } + + // Always add system secrets (deduplicated) + for _, sys := range constants.SystemSecrets { + if seenSecrets[sys.Name] { + continue + } + seenSecrets[sys.Name] = true + allRequirements = append(allRequirements, SecretRequirement{ + Name: sys.Name, + WhenNeeded: sys.WhenNeeded, + Description: sys.Description, + Optional: sys.Optional, + IsEngineSecret: false, + }) + } + + workflowSecretsLog.Printf("Returning %d deduplicated secret requirements", len(allRequirements)) + return allRequirements +} + +// getSecretRequirementsForWorkflow extracts the engine from a workflow file and returns its required secrets +// +// NOTE: In future we will want to analyse more parts of the +// workflow to work out other secrets required, or detect that the particular +// authorization being used in a workflow means certain secrets are not required. +// For now we are only looking at the secrets implied by the engine used. +func getSecretRequirementsForWorkflow(workflowFile string) []SecretRequirement { + workflowSecretsLog.Printf("Extracting secrets for workflow: %s", workflowFile) + + // Extract engine from workflow file + engine := extractEngineIDFromFile(workflowFile) + if engine == "" { + workflowSecretsLog.Printf("No engine found in workflow %s, skipping", workflowFile) + return nil + } + + workflowSecretsLog.Printf("Workflow %s uses engine: %s", workflowFile, engine) + + // Get engine-specific secrets only (no system secrets, no optional) + // System secrets will be added separately to avoid duplication + return getSecretRequirementsForEngine(engine, true, true) +} diff --git a/pkg/cli/workflow_secrets_test.go b/pkg/cli/workflow_secrets_test.go new file mode 100644 index 00000000000..0455c7aeff4 --- /dev/null +++ b/pkg/cli/workflow_secrets_test.go @@ -0,0 +1,236 @@ +//go:build !integration + +package cli + +import ( + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/github/gh-aw/pkg/constants" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetRequiredSecretsForWorkflow(t *testing.T) { + // Create a temporary directory with test workflow files + tempDir := t.TempDir() + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Should create workflows directory") + + tests := []struct { + name string + workflowContent string + expectedSecretName string + expectNil bool + }{ + { + name: "copilot engine workflow", + workflowContent: `--- +engine: copilot +on: push +--- +# Copilot Workflow`, + expectedSecretName: "COPILOT_GITHUB_TOKEN", + expectNil: false, + }, + { + name: "claude engine workflow", + workflowContent: `--- +engine: claude +on: push +--- +# Claude Workflow`, + expectedSecretName: "ANTHROPIC_API_KEY", + expectNil: false, + }, + { + name: "workflow without engine", + workflowContent: `--- +on: push +--- +# No Engine Workflow`, + expectedSecretName: "COPILOT_GITHUB_TOKEN", // Defaults to copilot + expectNil: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create workflow file with unique name + workflowPath := filepath.Join(workflowsDir, tt.name+".md") + err := os.WriteFile(workflowPath, []byte(tt.workflowContent), 0644) + require.NoError(t, err, "Should write workflow file") + defer os.Remove(workflowPath) + + // Test the function + secrets := getSecretRequirementsForWorkflow(workflowPath) + + if tt.expectNil { + assert.Nil(t, secrets, "Should return nil for workflow without engine") + } else { + require.NotNil(t, secrets, "Should return secrets") + require.NotEmpty(t, secrets, "Should have at least one secret") + + // Check that the expected secret is present + found := false + for _, secret := range secrets { + if secret.Name == tt.expectedSecretName { + found = true + assert.True(t, secret.IsEngineSecret, "Should be marked as engine secret") + break + } + } + assert.True(t, found, "Should include expected secret %s", tt.expectedSecretName) + } + }) + } +} + +func TestGetRequiredSecretsForWorkflows(t *testing.T) { + t.Run("collects secrets from multiple workflows", func(t *testing.T) { + // Create a temporary directory with test workflow files + tempDir := t.TempDir() + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Should create workflows directory") + + // Create test workflow files with different engines + testWorkflows := map[string]string{ + "copilot-workflow.md": `--- +engine: copilot +on: push +--- +# Copilot Workflow`, + "claude-workflow.md": `--- +engine: claude +on: pull_request +--- +# Claude Workflow`, + "codex-workflow.md": `--- +engine: codex +on: workflow_dispatch +--- +# Codex Workflow`, + } + + var workflowFiles []string + for filename, content := range testWorkflows { + path := filepath.Join(workflowsDir, filename) + err := os.WriteFile(path, []byte(content), 0644) + require.NoError(t, err, "Should write workflow file %s", filename) + workflowFiles = append(workflowFiles, path) + } + + // Test the function + secrets := getSecretsRequirementsForWorkflows(workflowFiles) + + require.NotEmpty(t, secrets, "Should return secrets") + + // Convert to map for easy checking + secretMap := make(map[string]SecretRequirement) + for _, secret := range secrets { + secretMap[secret.Name] = secret + } + + // Check engine secrets + assert.Contains(t, secretMap, "COPILOT_GITHUB_TOKEN", "Should include Copilot secret") + assert.Contains(t, secretMap, "ANTHROPIC_API_KEY", "Should include Claude secret") + assert.Contains(t, secretMap, "OPENAI_API_KEY", "Should include Codex secret") + + // Check system secrets are included + for _, sys := range constants.SystemSecrets { + assert.Contains(t, secretMap, sys.Name, "Should include system secret %s", sys.Name) + } + + // Verify no duplicates + assert.Len(t, secrets, len(secretMap), "Should not have duplicate secrets") + }) + + t.Run("handles empty workflow list", func(t *testing.T) { + secrets := getSecretsRequirementsForWorkflows([]string{}) + + require.NotEmpty(t, secrets, "Should return at least system secrets") + + // Should only have system secrets + assert.Len(t, secrets, len(constants.SystemSecrets), "Should only have system secrets") + + // Verify all are system secrets + for _, secret := range secrets { + assert.False(t, secret.IsEngineSecret, "All secrets should be system secrets") + } + }) + + t.Run("deduplicates secrets from workflows with same engine", func(t *testing.T) { + // Create a temporary directory with test workflow files + tempDir := t.TempDir() + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Should create workflows directory") + + // Create multiple workflows using the same engine + var workflowFiles []string + for i := 1; i <= 3; i++ { + content := `--- +engine: copilot +on: push +--- +# Copilot Workflow` + path := filepath.Join(workflowsDir, fmt.Sprintf("copilot-workflow-%d.md", i)) + err := os.WriteFile(path, []byte(content), 0644) + require.NoError(t, err, "Should write workflow file") + workflowFiles = append(workflowFiles, path) + } + + secrets := getSecretsRequirementsForWorkflows(workflowFiles) + + // Count Copilot tokens + copilotCount := 0 + for _, secret := range secrets { + if secret.Name == "COPILOT_GITHUB_TOKEN" { + copilotCount++ + } + } + + assert.Equal(t, 1, copilotCount, "Should have exactly one Copilot token despite multiple workflows") + }) + + t.Run("skips workflows without engines", func(t *testing.T) { + // Create a temporary directory with test workflow files + tempDir := t.TempDir() + workflowsDir := filepath.Join(tempDir, ".github", "workflows") + err := os.MkdirAll(workflowsDir, 0755) + require.NoError(t, err, "Should create workflows directory") + + // Create one workflow with engine and one without + workflowWithEngine := filepath.Join(workflowsDir, "with-engine.md") + err = os.WriteFile(workflowWithEngine, []byte(`--- +engine: copilot +on: push +--- +# With Engine`), 0644) + require.NoError(t, err, "Should write workflow file") + + workflowWithoutEngine := filepath.Join(workflowsDir, "without-engine.md") + err = os.WriteFile(workflowWithoutEngine, []byte(`--- +on: push +--- +# Without Engine`), 0644) + require.NoError(t, err, "Should write workflow file") + + secrets := getSecretsRequirementsForWorkflows([]string{workflowWithEngine, workflowWithoutEngine}) + + // Count engine secrets (should only have Copilot) + engineSecretCount := 0 + for _, secret := range secrets { + if secret.IsEngineSecret { + engineSecretCount++ + assert.Equal(t, "COPILOT_GITHUB_TOKEN", secret.Name, "Should only have Copilot secret") + } + } + + assert.Equal(t, 1, engineSecretCount, "Should have exactly one engine secret") + }) +} diff --git a/pkg/cli/workflows.go b/pkg/cli/workflows.go index c0dbf2e7bd4..32f3fab6b54 100644 --- a/pkg/cli/workflows.go +++ b/pkg/cli/workflows.go @@ -254,3 +254,115 @@ func isWorkflowFile(filename string) bool { func filterWorkflowFiles(files []string) []string { return sliceutil.Filter(files, isWorkflowFile) } + +// getMarkdownWorkflowFiles discovers markdown workflow files in the specified directory +func getMarkdownWorkflowFiles(workflowDir string) ([]string, error) { + // Use provided directory or default + workflowsDir := workflowDir + if workflowsDir == "" { + workflowsDir = getWorkflowsDir() + } + + if _, err := os.Stat(workflowsDir); os.IsNotExist(err) { + return nil, fmt.Errorf("no %s directory found", workflowsDir) + } + + // Find all markdown files in workflow directory + mdFiles, err := filepath.Glob(filepath.Join(workflowsDir, "*.md")) + if err != nil { + return nil, fmt.Errorf("failed to find workflow files: %w", err) + } + + // Filter out README.md files + mdFiles = filterWorkflowFiles(mdFiles) + + return mdFiles, nil +} + +// extractWorkflowNameFromFile extracts the workflow name from a file's H1 header +func extractWorkflowNameFromFile(filePath string) (string, error) { + content, err := os.ReadFile(filePath) + if err != nil { + return "", err + } + + // Extract markdown content (excluding frontmatter) + result, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil { + return "", err + } + + // Look for first H1 header + lines := strings.Split(result.Markdown, "\n") + for _, line := range lines { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "# ") { + return strings.TrimSpace(line[2:]), nil + } + } + + // No H1 header found, generate default name from filename + baseName := filepath.Base(filePath) + baseName = strings.TrimSuffix(baseName, filepath.Ext(baseName)) + baseName = strings.ReplaceAll(baseName, "-", " ") + + // Capitalize first letter of each word + words := strings.Fields(baseName) + for i, word := range words { + if len(word) > 0 { + words[i] = strings.ToUpper(word[:1]) + word[1:] + } + } + + return strings.Join(words, " "), nil +} + +// extractEngineIDFromFile extracts the engine ID from a workflow file's frontmatter +func extractEngineIDFromFile(filePath string) string { + content, err := os.ReadFile(filePath) + if err != nil { + return "" // Return empty string if file cannot be read + } + + // Parse frontmatter + result, err := parser.ExtractFrontmatterFromContent(string(content)) + if err != nil { + return "" // Return empty string if frontmatter cannot be parsed + } + + // Use the workflow package's extractEngineConfig to handle both string and object formats + compiler := &workflow.Compiler{} + engineSetting, engineConfig := compiler.ExtractEngineConfig(result.Frontmatter) + + // If engine is specified, return the ID from the config + if engineConfig != nil && engineConfig.ID != "" { + return engineConfig.ID + } + + // If we have an engine setting string, return it + if engineSetting != "" { + return engineSetting + } + + return "copilot" // Default engine +} + +// extractEnginesFromWorkflows extracts unique engines from a list of workflow files +func extractEnginesFromWorkflows(workflowFiles []string) []string { + // Collect unique engines used across workflows + engineSet := make(map[string]bool) + for _, file := range workflowFiles { + engine := extractEngineIDFromFile(file) + engineSet[engine] = true + } + + workflowsLog.Printf("Found %d unique engines: %v", len(engineSet), engineSet) + + // Convert engine set to slice + engines := make([]string, 0, len(engineSet)) + for engine := range engineSet { + engines = append(engines, engine) + } + + return engines +} diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 6768e4f68ef..3b477e48bfc 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -755,9 +755,9 @@ type SystemSecretSpec struct { var SystemSecrets = []SystemSecretSpec{ { Name: "GH_AW_GITHUB_TOKEN", - WhenNeeded: "Cross-repo Project Ops / remote GitHub tools", - Description: "Fine-grained or classic PAT with contents/issues/pull-requests read+write on the repos gh-aw will touch.", - Optional: false, + WhenNeeded: "Enables the use of a user identity for GitHub write operations (instead of github-actions identity); may enable cross-repo project ops; may permit use of remote GitHub MCP tools", + Description: "Fine-grained or classic PAT with contents/issues/pull-requests read+write and other necessary permissions on the repos gh-aw will read or write.", + Optional: true, }, { Name: "GH_AW_AGENT_TOKEN",