diff --git a/pkg/cli/compile_args.go b/pkg/cli/compile_args.go index 398bc215a66..d3b78d4d5cf 100644 --- a/pkg/cli/compile_args.go +++ b/pkg/cli/compile_args.go @@ -62,7 +62,7 @@ func expandDirectoryArg(dirPath string, verbose bool) ([]string, error) { compileArgsLog.Printf("Expanding directory argument: %s", dirPath) if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Compiling all workflows in directory: "+dirPath)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Compiling all workflows in directory: "+dirPath)) } mdFiles, err := getMarkdownWorkflowFiles(dirPath) diff --git a/pkg/cli/compile_batch_notices_test.go b/pkg/cli/compile_batch_notices_test.go new file mode 100644 index 00000000000..793686f43c4 --- /dev/null +++ b/pkg/cli/compile_batch_notices_test.go @@ -0,0 +1,188 @@ +//go:build !integration + +package cli + +import ( + "bytes" + "os" + "strings" + "testing" + + "github.com/github/gh-aw/pkg/workflow" +) + +// TestDisplayBatchCompilationNotices verifies that displayBatchCompilationNotices +// aggregates experimental-feature notices and the Copilot billing tip into a single +// block rather than repeating them per workflow, and that success messages are +// suppressed when batch/quiet mode is active. +func TestDisplayBatchCompilationNotices(t *testing.T) { + tests := []struct { + name string + featureUsage map[string]int + copilotTipNeeded bool + config CompileConfig + expectedInOutput []string + notExpectedInOutput []string + }{ + { + name: "no notices when nothing was used", + featureUsage: map[string]int{}, + copilotTipNeeded: false, + config: CompileConfig{}, + expectedInOutput: []string{}, + notExpectedInOutput: []string{ + "Experimental features", + "copilot", + }, + }, + { + name: "single experimental feature shows aggregate notice", + featureUsage: map[string]int{ + "Using experimental feature: canvas": 3, + }, + copilotTipNeeded: false, + config: CompileConfig{}, + expectedInOutput: []string{ + "Experimental features in use:", + "canvas: 3 workflows", + }, + notExpectedInOutput: []string{ + "Using experimental feature: canvas", + "Using experimental feature: canvas\nUsing experimental feature: canvas", + }, + }, + { + name: "multiple experimental features sorted by count desc", + featureUsage: map[string]int{ + "Using experimental feature: canvas": 2, + "Using experimental feature: safe-read": 5, + }, + copilotTipNeeded: false, + config: CompileConfig{}, + expectedInOutput: []string{ + "Experimental features in use:", + "safe-read: 5 workflows", + "canvas: 2 workflows", + }, + notExpectedInOutput: []string{}, + }, + { + name: "copilot tip shown when needed", + featureUsage: map[string]int{}, + copilotTipNeeded: true, + config: CompileConfig{}, + expectedInOutput: []string{ + "Copilot token-based inference may be available", + }, + notExpectedInOutput: []string{}, + }, + { + name: "no output in JSON mode", + featureUsage: map[string]int{ + "Using experimental feature: canvas": 2, + }, + copilotTipNeeded: true, + config: CompileConfig{JSONOutput: true}, + expectedInOutput: []string{}, + notExpectedInOutput: []string{ + "Experimental features", + "Copilot token-based inference", + }, + }, + { + name: "no output in verbose mode", + featureUsage: map[string]int{ + "Using experimental feature: canvas": 2, + }, + copilotTipNeeded: true, + config: CompileConfig{Verbose: true}, + expectedInOutput: []string{}, + notExpectedInOutput: []string{ + "Experimental features", + "Copilot token-based inference", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + compiler := workflow.NewCompiler() + compiler.SetBatchMode(true) + compiler.SetExperimentalFeatureUsage(tt.featureUsage) + compiler.SetCopilotTipNeeded(tt.copilotTipNeeded) + + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + displayBatchCompilationNotices(compiler, tt.config) + + w.Close() + os.Stderr = oldStderr + + var buf bytes.Buffer + buf.ReadFrom(r) + output := buf.String() + + for _, expected := range tt.expectedInOutput { + if !strings.Contains(output, expected) { + t.Errorf("expected output to contain %q, got:\n%s", expected, output) + } + } + for _, notExpected := range tt.notExpectedInOutput { + if strings.Contains(output, notExpected) { + t.Errorf("expected output NOT to contain %q, got:\n%s", notExpected, output) + } + } + }) + } +} + +// TestBatchModeSetOnMultipleSpecificFiles verifies that compileSpecificFiles enables +// batch mode (quiet, aggregation) when more than one file is provided and verbose is off. +// It also checks that displayBatchCompilationNotices is called at the end by confirming +// the per-workflow success message is absent from stderr for multiple-file batches. +func TestBatchModeSetOnSpecificFilesWhenMultiple(t *testing.T) { + compiler := workflow.NewCompiler() + compiler.SetBatchMode(false) + compiler.SetQuiet(false) + + // Simulating the batch-mode condition directly: when !config.Verbose && len(files) > 1 + files := []string{"a.md", "b.md", "c.md"} + config := CompileConfig{Verbose: false} + + batchMode := !config.Verbose && len(files) > 1 + if !batchMode { + t.Fatal("expected batchMode=true for 3 non-verbose files") + } + + compiler.SetBatchMode(batchMode) + compiler.SetQuiet(batchMode) + + // Set some feature usage so the notice would appear in batch mode + compiler.SetExperimentalFeatureUsage(map[string]int{ + "Using experimental feature: canvas": 2, + }) + + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + displayBatchCompilationNotices(compiler, config) + + w.Close() + os.Stderr = oldStderr + + var buf bytes.Buffer + buf.ReadFrom(r) + output := buf.String() + + if !strings.Contains(output, "Experimental features in use:") { + t.Errorf("expected aggregated experimental features notice, got:\n%s", output) + } + // Verify each feature is mentioned only once regardless of count + occurrences := strings.Count(output, "canvas") + if occurrences != 1 { + t.Errorf("expected 'canvas' to appear exactly once in batch output, got %d occurrences:\n%s", occurrences, output) + } +} diff --git a/pkg/cli/compile_compiler_setup.go b/pkg/cli/compile_compiler_setup.go index b0be336e111..c529f5f8d0f 100644 --- a/pkg/cli/compile_compiler_setup.go +++ b/pkg/cli/compile_compiler_setup.go @@ -250,7 +250,7 @@ func setupRepositoryContext(compiler *workflow.Compiler, config CompileConfig) { parts := strings.SplitN(config.ScheduleSeed, "/", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { compileCompilerSetupLog.Printf("Invalid --schedule-seed value %q: expected 'owner/repo' format", config.ScheduleSeed) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage( + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr( fmt.Sprintf("--schedule-seed %q is not in 'owner/repo' format; ignoring and falling back to git remote detection", config.ScheduleSeed), )) } else { diff --git a/pkg/cli/compile_external_tools.go b/pkg/cli/compile_external_tools.go index ed19332f357..07b29f2fd62 100644 --- a/pkg/cli/compile_external_tools.go +++ b/pkg/cli/compile_external_tools.go @@ -117,7 +117,7 @@ func handleBatchToolError(toolName string, err error, strict, verbose bool) erro } // In non-strict mode, errors are warnings if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("%s warnings: %v", toolName, err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("%s warnings: %v", toolName, err))) } return nil } diff --git a/pkg/cli/compile_file_operations.go b/pkg/cli/compile_file_operations.go index bf0fa1c42ff..dc3739af5b6 100644 --- a/pkg/cli/compile_file_operations.go +++ b/pkg/cli/compile_file_operations.go @@ -67,7 +67,7 @@ func compileSingleFile(ctx context.Context, compiler *workflow.Compiler, file st // Regular workflow file - compile normally compileHelpersLog.Printf("Compiling as regular workflow: %s", file) if verbose { - fmt.Fprintln(os.Stderr, console.FormatProgressMessage("Compiling: "+file)) + fmt.Fprintln(os.Stderr, console.FormatProgressMessageStderr("Compiling: "+file)) } if err := CompileWorkflowWithValidation(ctx, compiler, file, CompileValidationOptions{Verbose: verbose}); err != nil { @@ -103,7 +103,7 @@ func compileAllWorkflowFiles(ctx context.Context, compiler *workflow.Compiler, w if len(mdFiles) == 0 { compileHelpersLog.Printf("No workflow markdown files found in %s after frontmatter filtering", workflowsDir) if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No workflow markdown files found in "+workflowsDir+" (workflow files must start with a frontmatter opener on the first line)")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("No workflow markdown files found in "+workflowsDir+" (workflow files must start with a frontmatter opener on the first line)")) } return stats, nil } @@ -118,7 +118,7 @@ func compileAllWorkflowFiles(ctx context.Context, compiler *workflow.Compiler, w if err != nil { compileHelpersLog.Printf("Failed to resolve absolute path for %s: %v", file, err) if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to resolve absolute path for %s: %v", file, err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to resolve absolute path for %s: %v", file, err))) } } else { file = absFile @@ -174,9 +174,9 @@ func compileModifiedFilesWithDependencies(ctx context.Context, compiler *workflo } } - fmt.Fprintln(os.Stderr, console.FormatProgressMessage("Watching for file changes")) + fmt.Fprintln(os.Stderr, console.FormatProgressMessageStderr("Watching for file changes")) if verbose { - fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Recompiling %d workflow(s) affected by %d change(s)...", len(workflowsToCompile), len(files)))) + fmt.Fprintln(os.Stderr, console.FormatProgressMessageStderr(fmt.Sprintf("Recompiling %d workflow(s) affected by %d change(s)...", len(workflowsToCompile), len(files)))) } // Reset warning count before compilation @@ -202,7 +202,7 @@ func compileModifiedFilesWithDependencies(ctx context.Context, compiler *workflo if err := actionCache.Save(); err != nil { compileHelpersLog.Printf("Failed to save action cache: %v", err) if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to save action cache: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to save action cache: %v", err))) } } else { compileHelpersLog.Print("Action cache saved successfully") @@ -214,7 +214,7 @@ func compileModifiedFilesWithDependencies(ctx context.Context, compiler *workflo if successCount > 0 || hasActionCacheEntries { if _, err := ensureGitAttributes(); err != nil { if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to update .gitattributes: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to update .gitattributes: %v", err))) } } } else { @@ -234,11 +234,11 @@ func handleFileDeleted(mdFile string, verbose bool) { if _, err := os.Stat(lockFile); err == nil { if err := os.Remove(lockFile); err != nil { if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to remove lock file %s: %v", lockFile, err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to remove lock file %s: %v", lockFile, err))) } } else { if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Removed corresponding lock file: "+lockFile)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Removed corresponding lock file: "+lockFile)) } } } diff --git a/pkg/cli/compile_infrastructure.go b/pkg/cli/compile_infrastructure.go index fb767b61a91..e1baf17a97a 100644 --- a/pkg/cli/compile_infrastructure.go +++ b/pkg/cli/compile_infrastructure.go @@ -24,14 +24,14 @@ func updateGitAttributes(successCount int, actionCache *workflow.ActionCache, ve if err != nil { compileInfrastructureLog.Printf("Failed to update .gitattributes: %v", err) if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to update .gitattributes: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to update .gitattributes: %v", err))) } return err } if updated { compileInfrastructureLog.Printf("Successfully updated .gitattributes") if verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Updated .gitattributes to mark .lock.yml files as generated")) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr("Updated .gitattributes to mark .lock.yml files as generated")) } } else { compileInfrastructureLog.Print(".gitattributes already up to date") @@ -54,14 +54,14 @@ func saveActionCache(actionCache *workflow.ActionCache, verbose bool) error { if err := actionCache.Save(); err != nil { compileInfrastructureLog.Printf("Failed to save action cache: %v", err) if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to save action cache: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to save action cache: %v", err))) } return err } compileInfrastructureLog.Print("Action cache saved successfully") if verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Action cache saved to "+actionCache.GetCachePath())) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr("Action cache saved to "+actionCache.GetCachePath())) } return nil diff --git a/pkg/cli/compile_orchestrator.go b/pkg/cli/compile_orchestrator.go index b6abecf87a9..66b49897e12 100644 --- a/pkg/cli/compile_orchestrator.go +++ b/pkg/cli/compile_orchestrator.go @@ -22,7 +22,7 @@ func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.Wo // Check context cancellation at the start select { case <-ctx.Done(): - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Operation cancelled")) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Operation cancelled")) return nil, ctx.Err() default: } @@ -108,7 +108,7 @@ func CompileWorkflows(ctx context.Context, config CompileConfig) ([]*workflow.Wo var markdownFile string if len(config.MarkdownFiles) > 0 { if len(config.MarkdownFiles) > 1 { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Watch mode only supports a single file, using the first one")) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Watch mode only supports a single file, using the first one")) } // Resolve the workflow file to get the full path resolvedFile, err := resolveWorkflowFile(config.MarkdownFiles[0], config.Verbose) diff --git a/pkg/cli/compile_pipeline.go b/pkg/cli/compile_pipeline.go index 4144dcbcf17..bb8a4941e3f 100644 --- a/pkg/cli/compile_pipeline.go +++ b/pkg/cli/compile_pipeline.go @@ -27,6 +27,8 @@ import ( "fmt" "os" "path/filepath" + "sort" + "strings" "github.com/github/gh-aw/pkg/gitutil" "github.com/github/gh-aw/pkg/stringutil" @@ -51,6 +53,10 @@ func compileSpecificFiles( ) ([]*workflow.WorkflowData, error) { compileOrchestrationLog.Printf("Compiling %d specific workflow files", len(config.MarkdownFiles)) + batchMode := !config.Verbose && len(config.MarkdownFiles) > 1 + compiler.SetBatchMode(batchMode) + compiler.SetQuiet(batchMode) + // Enable validation automatically when force-refresh-action-pins is used // to verify all resolved action SHAs are valid shouldValidate := config.Validate || config.ForceRefreshActionPins @@ -75,7 +81,7 @@ func compileSpecificFiles( // Respect context cancellation between files (e.g. Ctrl+C) select { case <-ctx.Done(): - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Operation cancelled")) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Operation cancelled")) return workflowDataList, ctx.Err() default: } @@ -138,6 +144,7 @@ func compileSpecificFiles( trackWorkflowFailure(stats, resolvedFile, len(errMsgs), errMsgs) } else { compiledCount++ + stats.Succeeded++ if fileResult.workflowData != nil { workflowDataList = append(workflowDataList, fileResult.workflowData) } @@ -258,7 +265,8 @@ func compileSpecificFiles( if config.Strict { errorCount++ stats.Errors++ - trackWorkflowFailure(stats, "grant", 1, []string{err.Error()}) + // Grant is a post-compilation tool, not a workflow; record it only in + // validationResults (for JSON output) without adding to FailureDetails. *validationResults = append(*validationResults, ValidationResult{ Workflow: "grant", Valid: false, @@ -287,6 +295,9 @@ func compileSpecificFiles( // Get warning count from compiler stats.Warnings = compiler.GetWarningCount() + // Aggregate and display batch-mode notices (experimental features, Copilot tip) + displayBatchCompilationNotices(compiler, config) + // Display schedule warnings displayScheduleWarnings(compiler, config.JSONOutput) @@ -340,7 +351,7 @@ func compileAllFilesInDirectory( compileOrchestrationLog.Printf("Scanning for markdown files in %s", workflowsDir) if config.Verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Scanning for markdown files in "+workflowsDir)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Scanning for markdown files in "+workflowsDir)) } // Find and filter markdown files (shared helper keeps logic in one place) @@ -359,9 +370,13 @@ func compileAllFilesInDirectory( compileOrchestrationLog.Printf("Found %d markdown files to compile", len(mdFiles)) if config.Verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d markdown files to compile", len(mdFiles)))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Found %d markdown files to compile", len(mdFiles)))) } + batchMode := !config.Verbose && len(mdFiles) > 1 + compiler.SetBatchMode(batchMode) + compiler.SetQuiet(batchMode) + // Handle purge logic: collect existing files before compilation var purgeData *purgeTrackingData if config.Purge { @@ -392,7 +407,7 @@ func compileAllFilesInDirectory( // Respect context cancellation between files (e.g. Ctrl+C) select { case <-ctx.Done(): - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Operation cancelled")) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Operation cancelled")) return workflowDataList, ctx.Err() default: } @@ -425,6 +440,7 @@ func compileAllFilesInDirectory( trackWorkflowFailure(stats, file, len(errMsgs), errMsgs) } else { successCount++ + stats.Succeeded++ if fileResult.workflowData != nil { workflowDataList = append(workflowDataList, fileResult.workflowData) } @@ -541,7 +557,8 @@ func compileAllFilesInDirectory( if config.Strict { errorCount++ stats.Errors++ - trackWorkflowFailure(stats, "grant", 1, []string{err.Error()}) + // Grant is a post-compilation tool, not a workflow; record it only in + // validationResults (for JSON output) without adding to FailureDetails. *validationResults = append(*validationResults, ValidationResult{ Workflow: "grant", Valid: false, @@ -573,6 +590,8 @@ func compileAllFilesInDirectory( // Get warning count from compiler stats.Warnings = compiler.GetWarningCount() + displayBatchCompilationNotices(compiler, config) + // Display schedule warnings displayScheduleWarnings(compiler, config.JSONOutput) @@ -580,7 +599,7 @@ func compileAllFilesInDirectory( displaySafeUpdateWarnings(compiler, config.JSONOutput) if config.Verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Successfully compiled %d out of %d workflow files", successCount, len(mdFiles)))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(fmt.Sprintf("Successfully compiled %d out of %d workflow files", successCount, len(mdFiles)))) } // Handle purge logic if requested @@ -614,6 +633,45 @@ func compileAllFilesInDirectory( return workflowDataList, nil } +func displayBatchCompilationNotices(compiler *workflow.Compiler, config CompileConfig) { + if config.JSONOutput || config.Verbose { + return + } + + featureUsage := compiler.GetExperimentalFeatureUsage() + if len(featureUsage) > 0 { + type featureCount struct { + name string + count int + } + features := make([]featureCount, 0, len(featureUsage)) + for message, count := range featureUsage { + features = append(features, featureCount{ + name: strings.TrimPrefix(message, "Using experimental feature: "), + count: count, + }) + } + sort.Slice(features, func(i, j int) bool { + if features[i].count != features[j].count { + return features[i].count > features[j].count + } + return features[i].name < features[j].name + }) + + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Experimental features in use:")) + for _, feature := range features { + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("%s: %s", feature.name, formatWorkflowCount(feature.count)))) + } + } + + if compiler.CopilotRequestsTipNeeded() { + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr( + "Copilot token-based inference may be available: add permissions.copilot-requests: write. "+ + "See https://github.github.com/gh-aw/reference/billing/", + )) + } +} + // purgeTrackingData holds data needed for purge operations type purgeTrackingData struct { existingLockFiles []string @@ -637,10 +695,10 @@ func collectPurgeData(workflowsDir string, mdFiles []string, verbose bool) *purg if verbose { if len(data.existingLockFiles) > 0 { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d existing .lock.yml files", len(data.existingLockFiles)))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Found %d existing .lock.yml files", len(data.existingLockFiles)))) } if len(data.existingInvalidFiles) > 0 { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d existing .invalid.yml files", len(data.existingInvalidFiles)))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Found %d existing .invalid.yml files", len(data.existingInvalidFiles)))) } } @@ -680,7 +738,7 @@ func runPostProcessing( if config.Strict { return err } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to reconcile compiler-managed Dependabot ignore entries: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to reconcile compiler-managed Dependabot ignore entries: %v", err))) } } } @@ -734,7 +792,7 @@ func runPostProcessingForDirectory( if config.Strict { return err } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to reconcile compiler-managed Dependabot ignore entries: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to reconcile compiler-managed Dependabot ignore entries: %v", err))) } } diff --git a/pkg/cli/compile_post_processing.go b/pkg/cli/compile_post_processing.go index 47c8717854b..9b8b1359a3e 100644 --- a/pkg/cli/compile_post_processing.go +++ b/pkg/cli/compile_post_processing.go @@ -64,7 +64,7 @@ func generateDependabotManifestsWrapper( return fmt.Errorf("failed to generate Dependabot manifests: %w", err) } // Non-strict mode: just report as warning - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to generate Dependabot manifests: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to generate Dependabot manifests: %v", err))) } return nil @@ -88,7 +88,7 @@ func generateMaintenanceWorkflowWrapper( if strict { return fmt.Errorf("failed to load repo config: %w", err) } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to load repo config: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to load repo config: %v", err))) repoConfig = nil } @@ -105,7 +105,7 @@ func generateMaintenanceWorkflowWrapper( return fmt.Errorf("failed to generate maintenance workflow: %w", err) } // Non-strict mode: just report as warning - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to generate maintenance workflow: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to generate maintenance workflow: %v", err))) } return nil @@ -127,7 +127,7 @@ func generateCentralSlashCommandWorkflowWrapper( if strict { return fmt.Errorf("failed to load repo config: %w", err) } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf( + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf( "Failed to load repo config; repo-config flags (e.g. help_command) will use defaults: %v", err))) repoConfig = nil } @@ -136,7 +136,7 @@ func generateCentralSlashCommandWorkflowWrapper( if strict { return fmt.Errorf("failed to generate centralized slash-command workflow: %w", err) } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to generate centralized slash-command workflow: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to generate centralized slash-command workflow: %v", err))) } return nil @@ -159,7 +159,7 @@ func purgeOrphanedLockFiles(workflowsDir string, expectedLockFiles []string, ver } if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d existing .lock.yml files", len(existingLockFiles)))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Found %d existing .lock.yml files", len(existingLockFiles)))) } // Build a set of expected lock files @@ -186,13 +186,13 @@ func purgeOrphanedLockFiles(workflowsDir string, expectedLockFiles []string, ver if len(orphanedFiles) > 0 { for _, orphanedFile := range orphanedFiles { if err := os.Remove(orphanedFile); err != nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to remove orphaned lock file %s: %v", filepath.Base(orphanedFile), err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to remove orphaned lock file %s: %v", filepath.Base(orphanedFile), err))) } else { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Removed orphaned lock file: "+filepath.Base(orphanedFile))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr("Removed orphaned lock file: "+filepath.Base(orphanedFile))) } } if verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Purged %d orphaned .lock.yml files", len(orphanedFiles)))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(fmt.Sprintf("Purged %d orphaned .lock.yml files", len(orphanedFiles)))) } } @@ -217,20 +217,20 @@ func purgeInvalidFiles(workflowsDir string, verbose bool) error { } if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d existing .invalid.yml files", len(existingInvalidFiles)))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Found %d existing .invalid.yml files", len(existingInvalidFiles)))) } // Delete all .invalid.yml files for _, invalidFile := range existingInvalidFiles { if err := os.Remove(invalidFile); err != nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to remove invalid file %s: %v", filepath.Base(invalidFile), err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to remove invalid file %s: %v", filepath.Base(invalidFile), err))) } else { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Removed invalid file: "+filepath.Base(invalidFile))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr("Removed invalid file: "+filepath.Base(invalidFile))) } } if verbose { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Purged %d .invalid.yml files", len(existingInvalidFiles)))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(fmt.Sprintf("Purged %d .invalid.yml files", len(existingInvalidFiles)))) } compilePostProcessingLog.Printf("Purged %d invalid files", len(existingInvalidFiles)) @@ -242,7 +242,7 @@ func displayScheduleWarnings(compiler *workflow.Compiler, jsonOutput bool) { scheduleWarnings := compiler.GetScheduleWarnings() if len(scheduleWarnings) > 0 && !jsonOutput { for _, warning := range scheduleWarnings { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warning)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warning)) } } } @@ -257,7 +257,7 @@ func displaySafeUpdateWarnings(compiler *workflow.Compiler, jsonOutput bool) { return } for _, w := range warnings { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(w)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(w)) } } @@ -284,12 +284,17 @@ func displayCentralizedSlashCommandRecommendation(compiler *workflow.Compiler, w return } + verb := "are" + if nonCentralizedSlashCommands == 1 { + verb = "is" + } msg := fmt.Sprintf( - "Detected %d slash_command entries in this repository; %d are not using centralized routing. Consider setting `on.slash_command.strategy: centralized` to reduce duplicate triggers and route through `agentic_commands.yml`.", + "Detected %d slash_command entries in this repository; %d %s not using centralized routing. Consider setting `on.slash_command.strategy: centralized` to reduce duplicate triggers and route through `agentic_commands.yml`.", totalSlashCommands, nonCentralizedSlashCommands, + verb, ) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(msg)) compiler.IncrementWarningCount() } diff --git a/pkg/cli/compile_post_processing_warning_test.go b/pkg/cli/compile_post_processing_warning_test.go index fa9ac5fd2ee..60f531a67ee 100644 --- a/pkg/cli/compile_post_processing_warning_test.go +++ b/pkg/cli/compile_post_processing_warning_test.go @@ -28,6 +28,16 @@ func TestDisplayCentralizedSlashCommandRecommendation(t *testing.T) { expectWarning: true, expectedWarnCount: 1, }, + { + name: "uses singular grammar for one non centralized command", + workflows: []*workflow.WorkflowData{ + {Command: []string{"a"}, CommandCentralized: false}, + {Command: []string{"b"}, CommandCentralized: true}, + {Command: []string{"c"}, CommandCentralized: true}, + }, + expectWarning: true, + expectedWarnCount: 1, + }, { name: "does not warn when fewer than three slash commands exist", workflows: []*workflow.WorkflowData{ @@ -71,6 +81,9 @@ func TestDisplayCentralizedSlashCommandRecommendation(t *testing.T) { if tt.expectWarning { require.Contains(t, stderrOutput, "Consider setting `on.slash_command.strategy: centralized`") require.Contains(t, stderrOutput, "Detected 3 slash_command entries") + if tt.name == "uses singular grammar for one non centralized command" { + require.Contains(t, stderrOutput, "1 is not using centralized routing") + } } else { require.NotContains(t, stderrOutput, "on.slash_command.strategy: centralized") } diff --git a/pkg/cli/compile_repository_manifest.go b/pkg/cli/compile_repository_manifest.go index de14b272b8d..50e024e79f5 100644 --- a/pkg/cli/compile_repository_manifest.go +++ b/pkg/cli/compile_repository_manifest.go @@ -81,7 +81,7 @@ func validateRepositoryManifestForCompilation(config CompileConfig, stats *Compi *validationResults = append(*validationResults, result) if !config.JSONOutput { for _, warning := range warnings { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warning)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warning)) } } } diff --git a/pkg/cli/compile_schedule_calendar.go b/pkg/cli/compile_schedule_calendar.go index ee2b9b9d44c..f0b5516b044 100644 --- a/pkg/cli/compile_schedule_calendar.go +++ b/pkg/cli/compile_schedule_calendar.go @@ -245,7 +245,7 @@ func displayScheduleCalendar(statsList []*WorkflowStats) { // Title fmt.Fprintln(os.Stderr) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Schedule Heatmap (UTC)")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Schedule Heatmap (UTC)")) fmt.Fprintln(os.Stderr) // Hour header row: each cell is 3 chars wide ("XX "). diff --git a/pkg/cli/compile_stats.go b/pkg/cli/compile_stats.go index a0c67304c67..b2e54e79df5 100644 --- a/pkg/cli/compile_stats.go +++ b/pkg/cli/compile_stats.go @@ -26,10 +26,11 @@ type WorkflowFailure struct { // CompilationStats tracks the results of workflow compilation type CompilationStats struct { Total int + Succeeded int // Explicitly-tracked count of workflows that compiled successfully Errors int Warnings int FailedWorkflows []string // Names of workflows that failed compilation (deprecated, use FailedWorkflowDetails) - FailureDetails []WorkflowFailure // Detailed information about failed workflows + FailureDetails []WorkflowFailure // Detailed information about failed workflows (workflow-level failures only) } // WorkflowStats holds statistics about a compiled workflow @@ -140,15 +141,23 @@ func printCompilationSummary(stats *CompilationStats, showAllErrors bool) { return } - summary := fmt.Sprintf("Compiled %d workflow(s): %d error(s), %d warning(s)", - stats.Total, stats.Errors, stats.Warnings) failedWorkflowCount := len(stats.FailureDetails) if failedWorkflowCount == 0 { failedWorkflowCount = len(stats.FailedWorkflows) } - if stats.Errors > 0 && failedWorkflowCount > 0 { - summary = fmt.Sprintf("Compiled %d workflow(s): %d error(s) across %d failed workflow(s), %d warning(s)", - stats.Total, stats.Errors, failedWorkflowCount, stats.Warnings) + successCount := stats.Succeeded + + summary := fmt.Sprintf("Compiled %s: %s, %s", + formatWorkflowCount(stats.Total), + formatSucceededCount(successCount), + formatWarningCount(stats.Warnings)) + if failedWorkflowCount > 0 { + summary = fmt.Sprintf("Compiled %s: %s, %s, %s, %s", + formatWorkflowCount(stats.Total), + formatSucceededCount(successCount), + formatFailedCount(failedWorkflowCount), + formatErrorCount(stats.Errors), + formatWarningCount(stats.Warnings)) } // Use different formatting based on whether there were errors @@ -182,28 +191,28 @@ func printCompilationSummary(stats *CompilationStats, showAllErrors bool) { for i, prioritized := range report.DisplayedErrors { heading := prioritized.Severity.Heading() if heading != lastHeading { - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("%s %s:", prioritized.Severity.Icon(), heading))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("%s %s:", prioritized.Severity.Icon(), heading))) lastHeading = heading } - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("%d. %s", i+1, prioritized.Message))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("%d. %s", i+1, prioritized.Message))) if prioritized.Suggestion != "" { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("โ†’ "+prioritized.Suggestion)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("โ†’ "+prioritized.Suggestion)) } } if report.RecoveryPlan != nil && len(report.RecoveryPlan.Steps) > 0 { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("๐Ÿ’ก Recovery plan:")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("๐Ÿ’ก Recovery plan:")) for i, step := range report.RecoveryPlan.Steps { - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("%d. %s", i+1, step))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("%d. %s", i+1, step))) } } if report.SuppressedCount > 0 { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Suppressed %d cascading error(s) until the root cause is fixed.", report.SuppressedCount))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Suppressed %d cascading error(s) until the root cause is fixed.", report.SuppressedCount))) } if !showAllErrors && report.HiddenCount > 0 { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Run 'gh aw compile --show-all' to see all %d prioritized error(s).", report.TotalCount))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Run 'gh aw compile --show-all' to see all %d prioritized error(s).", report.TotalCount))) } } } else if len(stats.FailedWorkflows) > 0 { @@ -216,12 +225,32 @@ func printCompilationSummary(stats *CompilationStats, showAllErrors bool) { fmt.Fprintln(os.Stderr) } } else if stats.Warnings > 0 { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(summary)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(summary)) } else { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(summary)) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(summary)) } } +func formatWorkflowCount(count int) string { + return fmt.Sprintf("%d %s", count, pluralize("workflow", count)) +} + +func formatSucceededCount(count int) string { + return fmt.Sprintf("%d succeeded", count) +} + +func formatFailedCount(count int) string { + return fmt.Sprintf("%d failed", count) +} + +func formatErrorCount(count int) string { + return fmt.Sprintf("%d %s", count, pluralize("error", count)) +} + +func formatWarningCount(count int) string { + return fmt.Sprintf("%d %s", count, pluralize("warning", count)) +} + // collectWorkflowStatisticsWrapper collects and returns workflow statistics func collectWorkflowStatisticsWrapper(markdownFiles []string) []*WorkflowStats { compileStatsLog.Printf("Collecting workflow statistics for %d files", len(markdownFiles)) @@ -319,13 +348,13 @@ func displayStatsTable(statsList []*WorkflowStats) { fmt.Fprint(os.Stderr, console.RenderTable(tableConfig)) // Print summary - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Summary:")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Summary:")) if len(statsList) > maxDisplay { - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("Showing top %d of %d workflows (sorted by size)", maxDisplay, len(statsList)))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("Showing top %d of %d workflows (sorted by size)", maxDisplay, len(statsList)))) } - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("Total workflows: %d", len(statsList)))) - fmt.Fprintln(os.Stderr, console.FormatListItem("Total size: "+console.FormatFileSize(totalSize))) - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("Total jobs: %d", totalJobs))) - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("Total steps: %d", totalSteps))) - fmt.Fprintln(os.Stderr, console.FormatListItem(fmt.Sprintf("Total scripts: %d (%s)", totalScripts, console.FormatFileSize(int64(totalScriptSize))))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("Total workflows: %d", len(statsList)))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr("Total size: "+console.FormatFileSize(totalSize))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("Total jobs: %d", totalJobs))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("Total steps: %d", totalSteps))) + fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("Total scripts: %d (%s)", totalScripts, console.FormatFileSize(int64(totalScriptSize))))) } diff --git a/pkg/cli/compile_summary_output_test.go b/pkg/cli/compile_summary_output_test.go index f1057454500..d4fe3aacc36 100644 --- a/pkg/cli/compile_summary_output_test.go +++ b/pkg/cli/compile_summary_output_test.go @@ -23,9 +23,10 @@ func TestPrintCompilationSummaryWithFailedWorkflows(t *testing.T) { { name: "multiple failed workflows with FailureDetails", stats: &CompilationStats{ - Total: 5, - Errors: 4, - Warnings: 1, + Total: 5, + Succeeded: 2, + Errors: 4, + Warnings: 1, FailureDetails: []WorkflowFailure{ { Path: ".github/workflows/test1.md", @@ -48,7 +49,7 @@ func TestPrintCompilationSummaryWithFailedWorkflows(t *testing.T) { }, }, expectedInOutput: []string{ - "Compiled 5 workflow(s): 4 error(s) across 3 failed workflow(s), 1 warning(s)", + "Compiled 5 workflows: 2 succeeded, 3 failed, 4 errors, 1 warning", "Failed workflows:", "โœ— test1.md", "โœ— test2.md", @@ -66,8 +67,9 @@ func TestPrintCompilationSummaryWithFailedWorkflows(t *testing.T) { { name: "single failed workflow with FailureDetails", stats: &CompilationStats{ - Total: 1, - Errors: 2, + Total: 1, + Succeeded: 0, + Errors: 2, FailureDetails: []WorkflowFailure{ { Path: ".github/workflows/workflow-single.md", @@ -77,7 +79,7 @@ func TestPrintCompilationSummaryWithFailedWorkflows(t *testing.T) { }, }, expectedInOutput: []string{ - "Compiled 1 workflow(s): 2 error(s) across 1 failed workflow(s), 0 warning(s)", + "Compiled 1 workflow: 0 succeeded, 1 failed, 2 errors, 0 warnings", "Failed workflows:", "โœ— workflow-single.md", "workflow-single.md (2 error(s)):", @@ -90,8 +92,9 @@ func TestPrintCompilationSummaryWithFailedWorkflows(t *testing.T) { name: "show-all prints every prioritized error", showAll: true, stats: &CompilationStats{ - Total: 1, - Errors: 6, + Total: 1, + Succeeded: 0, + Errors: 6, FailureDetails: []WorkflowFailure{ { Path: ".github/workflows/workflow-all.md", @@ -120,11 +123,12 @@ func TestPrintCompilationSummaryWithFailedWorkflows(t *testing.T) { name: "backward compatibility with FailedWorkflows", stats: &CompilationStats{ Total: 3, + Succeeded: 1, Errors: 2, FailedWorkflows: []string{"old-workflow1.md", "old-workflow2.md"}, }, expectedInOutput: []string{ - "Compiled 3 workflow(s): 2 error(s) across 2 failed workflow(s), 0 warning(s)", + "Compiled 3 workflows: 1 succeeded, 2 failed, 2 errors, 0 warnings", "Failed workflows:", "โœ— old-workflow1.md", "โœ— old-workflow2.md", @@ -134,12 +138,13 @@ func TestPrintCompilationSummaryWithFailedWorkflows(t *testing.T) { { name: "successful compilation without failures", stats: &CompilationStats{ - Total: 5, - Errors: 0, - Warnings: 0, + Total: 5, + Succeeded: 5, + Errors: 0, + Warnings: 0, }, expectedInOutput: []string{ - "Compiled 5 workflow(s): 0 error(s), 0 warning(s)", + "Compiled 5 workflows: 5 succeeded, 0 warnings", }, notExpectedInOutput: []string{ "Failed workflows:", diff --git a/pkg/cli/compile_update_check.go b/pkg/cli/compile_update_check.go index bb32c73c7e8..a18461ef16d 100644 --- a/pkg/cli/compile_update_check.go +++ b/pkg/cli/compile_update_check.go @@ -311,17 +311,17 @@ func printCompileUpdateNotification(notification *compileUpdateNotification) { switch notification.Kind { case compileUpdateNotificationRemovedTag: - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf( + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf( "The installed gh-aw compiler version %s is no longer available as a repository tag.", notification.CurrentVersion, ))) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf( + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf( "Update the compiler before recompiling workflows (latest release: %s).", notification.LatestVersion, ))) default: - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf( + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf( "Compiler upgrade recommended: gh-aw %s is behind the latest release %s.", notification.CurrentVersion, notification.LatestVersion, ))) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Hint: upgrade the compiler with: gh extension upgrade github/gh-aw")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Hint: upgrade the compiler with: gh extension upgrade github/gh-aw")) } fmt.Fprintln(os.Stderr) diff --git a/pkg/cli/compile_watch.go b/pkg/cli/compile_watch.go index 6f85b782a93..cf53085a493 100644 --- a/pkg/cli/compile_watch.go +++ b/pkg/cli/compile_watch.go @@ -49,11 +49,11 @@ func watchAndCompileWorkflows(ctx context.Context, markdownFile string, compiler compileWatchLog.Print("Building dependency graph for watch mode...") if err := depGraph.BuildGraph(compiler); err != nil { compileWatchLog.Printf("Warning: failed to build dependency graph: %v", err) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to build dependency graph: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Failed to build dependency graph: %v", err))) } else { compileWatchLog.Printf("Dependency graph built successfully: %d workflows", len(depGraph.nodes)) if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Dependency graph built: %d workflows", len(depGraph.nodes)))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Dependency graph built: %d workflows", len(depGraph.nodes)))) } } @@ -99,9 +99,9 @@ func watchAndCompileWorkflows(ctx context.Context, markdownFile string, compiler // Always emit the begin pattern for task integration if markdownFile != "" { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Watching for file changes to %s...", markdownFile))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Watching for file changes to %s...", markdownFile))) } else { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Watching for file changes in %s...", workflowsDir))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Watching for file changes in %s...", workflowsDir))) } if verbose { @@ -123,7 +123,7 @@ func watchAndCompileWorkflows(ctx context.Context, markdownFile string, compiler stats, err := compileAllWorkflowFiles(ctx, compiler, workflowsDir, verbose) if err != nil { // Always show initial compilation errors, not just in verbose mode - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Initial compilation failed: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Initial compilation failed: %v", err))) } // Print summary instead of just "Recompiled" printCompilationSummary(stats, false) @@ -136,7 +136,7 @@ func watchAndCompileWorkflows(ctx context.Context, markdownFile string, compiler fmt.Fprintln(os.Stderr, "Watching for file changes") if verbose { - fmt.Fprintln(os.Stderr, console.FormatProgressMessage(fmt.Sprintf("Initial compilation of %s...", markdownFile))) + fmt.Fprintln(os.Stderr, console.FormatProgressMessageStderr(fmt.Sprintf("Initial compilation of %s...", markdownFile))) } // Use compileSingleFile to handle both regular workflows and campaign files @@ -174,7 +174,7 @@ func watchAndCompileWorkflows(ctx context.Context, markdownFile string, compiler compileWatchLog.Printf("Detected change: %s (%s)", event.Name, event.Op.String()) if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Detected change: %s (%s)", event.Name, event.Op.String()))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Detected change: %s (%s)", event.Name, event.Op.String()))) } // Handle file operations @@ -220,7 +220,7 @@ func watchAndCompileWorkflows(ctx context.Context, markdownFile string, compiler } compileWatchLog.Printf("Watcher error: %v", err) if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Watcher error: %v", err))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Watcher error: %v", err))) } case <-ctx.Done(): diff --git a/pkg/cli/compile_workflow_processor.go b/pkg/cli/compile_workflow_processor.go index a3fd8ab1819..207487bb092 100644 --- a/pkg/cli/compile_workflow_processor.go +++ b/pkg/cli/compile_workflow_processor.go @@ -122,7 +122,7 @@ func compileWorkflowFile( if errors.As(err, &sharedErr) { if !opts.jsonOutput { // Print info message instead of error - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(sharedErr.Error())) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(sharedErr.Error())) } // Mark as valid but skipped result.validationResult.Valid = true @@ -139,7 +139,7 @@ func compileWorkflowFile( if errors.As(err, &redirectErr) { if !opts.jsonOutput { // Print info message instead of error - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(redirectErr.Error())) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(redirectErr.Error())) } // Mark as valid but skipped result.validationResult.Valid = true diff --git a/pkg/cli/update_actions.go b/pkg/cli/update_actions.go index 28700d7e083..588eb0eca8a 100644 --- a/pkg/cli/update_actions.go +++ b/pkg/cli/update_actions.go @@ -862,21 +862,18 @@ func updateActionsInWorkflowFiles(ctx context.Context, deps actionUpdateDeps, op fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Updated action/skill references in "+d.Name())) updatedFiles = append(updatedFiles, path) - - // Recompile the updated workflow (unless --no-compile is set) - if !opts.noCompile { - if err := compileWorkflowWithRefresh(ctx, path, opts.verbose, false, opts.engineOverride, false, opts.approve); err != nil { - if opts.verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to recompile %s: %v", path, err))) - } - } - } return nil }) if err != nil { return fmt.Errorf("failed to walk workflows directory: %w", err) } + if len(updatedFiles) > 0 && !opts.noCompile { + if err := compileWorkflowsForUpdate(ctx, updatedFiles, opts.workflowsDir, opts.engineOverride, opts.verbose, opts.approve); err != nil { + return fmt.Errorf("failed to compile workflows with updated action references: %w", err) + } + } + if len(updatedFiles) == 0 && opts.verbose { fmt.Fprintln(os.Stderr, console.FormatInfoMessage("No action references needed updating in workflow files")) } diff --git a/pkg/cli/update_command.go b/pkg/cli/update_command.go index b390543e0e1..bd3fe444612 100644 --- a/pkg/cli/update_command.go +++ b/pkg/cli/update_command.go @@ -226,8 +226,18 @@ func RunUpdateWorkflows(ctx context.Context, opts UpdateWorkflowsOptions) error coolDown: opts.CoolDown, approve: opts.Approve, }); err != nil { - // Non-fatal: warn but don't fail the update - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not update action references in workflow files: %v", err))) + var compilationErr *updateCompilationError + if errors.As(err, &compilationErr) { + compileErr := fmt.Errorf("workflow compilation after action reference update failed: %w", err) + if firstErr == nil { + firstErr = compileErr + } else { + firstErr = errors.Join(firstErr, compileErr) + } + } else { + // Non-fatal: warn but don't fail the update + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not update action references in workflow files: %v", err))) + } } // Resolve and store SHA-256 digest pins for container images referenced in lock files. @@ -248,7 +258,12 @@ func RunUpdateWorkflows(ctx context.Context, opts UpdateWorkflowsOptions) error fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Recompiling workflows to embed container digest pins...")) recompileErr := recompileAllWorkflows(ctx, opts.WorkflowsDir, opts.EngineOverride, opts.Verbose, opts.Approve) if recompileErr != nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not recompile workflows after container pin update: %v", recompileErr))) + compileErr := fmt.Errorf("workflow compilation after container pin update failed: %w", recompileErr) + if firstErr == nil { + firstErr = compileErr + } else { + firstErr = errors.Join(firstErr, compileErr) + } } } @@ -273,24 +288,7 @@ func recompileAllWorkflows(ctx context.Context, workflowsDir, engineOverride str if workflowsDir == "" { workflowsDir = getWorkflowsDir() } - - entries, err := os.ReadDir(workflowsDir) - if err != nil { - return fmt.Errorf("failed to read workflows directory: %w", err) - } - - for _, entry := range entries { - if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".md") { - continue - } - path := filepath.Join(workflowsDir, entry.Name()) - if err := compileWorkflowWithRefresh(ctx, path, verbose, true, engineOverride, false, approve); err != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to recompile %s: %v", entry.Name(), err))) - } - } - } - return nil + return compileWorkflowsForUpdate(ctx, nil, workflowsDir, engineOverride, verbose, approve) } func runUpdateForTargetRepo(ctx context.Context, targetRepo string, opts UpdateWorkflowsOptions, createPR bool, verbose bool) error { diff --git a/pkg/cli/update_compile.go b/pkg/cli/update_compile.go new file mode 100644 index 00000000000..14f9705c9c7 --- /dev/null +++ b/pkg/cli/update_compile.go @@ -0,0 +1,54 @@ +package cli + +import ( + "context" + "fmt" +) + +// updateCompilationError identifies failures from the shared compile command path. +type updateCompilationError struct { + err error +} + +func (e *updateCompilationError) Error() string { + return e.err.Error() +} + +func (e *updateCompilationError) Unwrap() error { + return e.err +} + +// compileWorkflowsForUpdate uses the same configuration and orchestration as the +// compile command, with only update's matching engine, directory, verbosity, and +// approval options applied. +func compileWorkflowsForUpdate( + ctx context.Context, + workflowFiles []string, + workflowsDir string, + engineOverride string, + verbose bool, + approve bool, +) error { + config := newUpdateCompileConfig(workflowFiles, workflowsDir, engineOverride, verbose, approve) + + if _, err := CompileWorkflows(ctx, config); err != nil { + return &updateCompilationError{err: fmt.Errorf("compile workflows: %w", err)} + } + return nil +} + +func newUpdateCompileConfig( + workflowFiles []string, + workflowsDir string, + engineOverride string, + verbose bool, + approve bool, +) CompileConfig { + return CompileConfig{ + MarkdownFiles: workflowFiles, + Verbose: verbose, + EngineOverride: engineOverride, + WorkflowDir: workflowsDir, + Approve: approve, + } +} diff --git a/pkg/cli/update_compile_test.go b/pkg/cli/update_compile_test.go new file mode 100644 index 00000000000..db1f7f1a425 --- /dev/null +++ b/pkg/cli/update_compile_test.go @@ -0,0 +1,44 @@ +package cli + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestNewUpdateCompileConfigMatchesCompileCommandDefaults(t *testing.T) { + files := []string{".github/workflows/first.md", ".github/workflows/second.md"} + + got := newUpdateCompileConfig(files, "custom/workflows", "copilot", true, true) + + require.Equal(t, CompileConfig{ + MarkdownFiles: files, + Verbose: true, + EngineOverride: "copilot", + WorkflowDir: "custom/workflows", + Approve: true, + }, got) + require.False(t, got.RefreshStopTime, "update compilation must preserve stop times like compile") +} + +func TestCompileWorkflowsForUpdatePropagatesCompileErrors(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := compileWorkflowsForUpdate(ctx, []string{"workflow.md"}, "", "", false, false) + + require.Error(t, err) + require.ErrorIs(t, err, context.Canceled) + var compilationErr *updateCompilationError + require.ErrorAs(t, err, &compilationErr) +} + +func TestRecompileAllWorkflowsPropagatesCompileErrors(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + err := recompileAllWorkflows(ctx, ".github/workflows", "", false, false) + + require.ErrorIs(t, err, context.Canceled) +} diff --git a/pkg/cli/update_manifest.go b/pkg/cli/update_manifest.go index eb93a559664..4419427f491 100644 --- a/pkg/cli/update_manifest.go +++ b/pkg/cli/update_manifest.go @@ -282,7 +282,7 @@ func updateManifestManagedWorkflow(ctx context.Context, update manifestManagedWo } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Updated %s from %s to %s", update.wf.Name, shortRef(update.currentRef), shortRef(update.latestRef)))) if !opts.NoCompile { - if err := compileWorkflowWithRefresh(ctx, update.wf.Path, opts.Verbose, false, opts.EngineOverride, true, opts.Approve); err != nil { + if err := compileWorkflowsForUpdate(ctx, []string{update.wf.Path}, opts.WorkflowsDir, opts.EngineOverride, opts.Verbose, opts.Approve); err != nil { return fmt.Errorf("failed to compile updated workflow: %w", err) } } @@ -323,7 +323,7 @@ func addManifestManagedWorkflow(ctx context.Context, targetDir, name, repo, late } fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Added new workflow from manifest: "+filepath.Base(destPath))) if !opts.NoCompile { - if err := compileWorkflowWithRefresh(ctx, destPath, opts.Verbose, false, opts.EngineOverride, true, opts.Approve); err != nil { + if err := compileWorkflowsForUpdate(ctx, []string{destPath}, opts.WorkflowsDir, opts.EngineOverride, opts.Verbose, opts.Approve); err != nil { return fmt.Errorf("failed to compile new manifest workflow: %w", err) } } diff --git a/pkg/cli/update_workflows.go b/pkg/cli/update_workflows.go index fa5d9095a69..194d29dd8ac 100644 --- a/pkg/cli/update_workflows.go +++ b/pkg/cli/update_workflows.go @@ -911,10 +911,10 @@ func updateWorkflow(ctx context.Context, wf *workflowWithSource, opts UpdateWork updateLog.Printf("Successfully updated workflow %s from %s to %s", wf.Name, currentRef, latestRef) fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Updated %s from %s to %s", wf.Name, shortRef(currentRef), shortRef(latestRef)))) - // Compile the updated workflow with refreshStopTime enabled (unless --no-compile is set) + // Compile the updated workflow using the same path as the compile command. if !opts.NoCompile { updateLog.Printf("Compiling updated workflow: %s", wf.Name) - if err := compileWorkflowWithRefresh(ctx, wf.Path, opts.Verbose, false, opts.EngineOverride, true, opts.Approve); err != nil { + if err := compileWorkflowsForUpdate(ctx, []string{wf.Path}, opts.WorkflowsDir, opts.EngineOverride, opts.Verbose, opts.Approve); err != nil { updateLog.Printf("Compilation failed for workflow %s: %v", wf.Name, err) return fmt.Errorf("failed to compile updated workflow: %w", err) } diff --git a/pkg/colorwriter/colorprofile_writer.go b/pkg/colorwriter/colorprofile_writer.go index 9c670fe82c5..5de57f71ba3 100644 --- a/pkg/colorwriter/colorprofile_writer.go +++ b/pkg/colorwriter/colorprofile_writer.go @@ -5,6 +5,7 @@ package colorwriter import ( "io" "os" + "strconv" "strings" "github.com/charmbracelet/colorprofile" @@ -28,7 +29,14 @@ func Stderr() io.Writer { // call Degrade so that the caller's output honors the color profile. func Degrade(s string, environ []string) string { var buf strings.Builder - w := colorprofile.NewWriter(&buf, environ) + profile := colorprofile.Env(environ) + if noColorEnabled(environ) { + profile = colorprofile.NoTTY + } + w := &colorprofile.Writer{ + Forward: &buf, + Profile: profile, + } // colorprofile.Writer writes synchronously and does not buffer past Write, // and strings.Builder writes cannot fail, so a write error would indicate an // unexpected future behavior change; fall back to the original string then. @@ -37,3 +45,15 @@ func Degrade(s string, environ []string) string { } return buf.String() } + +func noColorEnabled(environ []string) bool { + for _, entry := range environ { + key, value, ok := strings.Cut(entry, "=") + if !ok || key != "NO_COLOR" { + continue + } + enabled, err := strconv.ParseBool(value) + return err == nil && enabled + } + return false +} diff --git a/pkg/colorwriter/spec_test.go b/pkg/colorwriter/spec_test.go index a309490a5d8..68f6d96108d 100644 --- a/pkg/colorwriter/spec_test.go +++ b/pkg/colorwriter/spec_test.go @@ -72,6 +72,11 @@ func TestSpec_PublicAPI_Degrade(t *testing.T) { environ: []string{"TERM=xterm-256color", "CLICOLOR_FORCE=1"}, want: ansiRed, }, + { + name: "preserves ansi for interactive color terminals", + environ: []string{"TERM=xterm-256color"}, + want: ansiRed, + }, } for _, tt := range tests { diff --git a/pkg/console/console.go b/pkg/console/console.go index ec8a058952a..868f0c06c33 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -23,6 +23,7 @@ var consoleLog = logger.New("console:console") // stdoutEnviron caches the process environment on first use so stdout styling // helpers do not repeatedly copy and re-parse it while rendering output. var stdoutEnviron = sync.OnceValue(os.Environ) +var stderrEnviron = sync.OnceValue(os.Environ) // isTTY checks if stdout is a terminal func isTTY() bool { @@ -48,6 +49,17 @@ func applyStdoutStyleWithTTY(style lipgloss.Style, text string, ttyCheck func() return colorwriter.Degrade(style.Render(text), environ) } +func applyStderrStyle(style lipgloss.Style, text string) string { + return applyStderrStyleWithTTY(style, text, isStderrTTY, stderrEnviron()) +} + +func applyStderrStyleWithTTY(style lipgloss.Style, text string, ttyCheck func() bool, environ []string) string { + if !ttyCheck() { + return text + } + return colorwriter.Degrade(style.Render(text), environ) +} + // applyStyleWithTTY conditionally renders raw ANSI based on a provided TTY check. // Use this only for strings that will later be written through a color-profile- // aware writer (for example stderrWriter); stdout-facing helpers should use @@ -61,6 +73,21 @@ func applyStyleWithTTY(style lipgloss.Style, text string, ttyCheck func() bool) // FormatError formats a CompilerError with Rust-like rendering func FormatError(err CompilerError) string { + return formatErrorWithStyle(err, applyStyle) +} + +// FormatErrorStderr formats a CompilerError for stderr with stderr TTY detection. +func FormatErrorStderr(err CompilerError) string { + return formatErrorStderrWithTTY(err, isStderrTTY, stderrEnviron()) +} + +func formatErrorStderrWithTTY(err CompilerError, ttyCheck func() bool, environ []string) string { + return formatErrorWithStyle(err, func(style lipgloss.Style, text string) string { + return applyStderrStyleWithTTY(style, text, ttyCheck, environ) + }) +} + +func formatErrorWithStyle(err CompilerError, styleText func(lipgloss.Style, string) string) string { consoleLog.Printf("Formatting error: type=%s, file=%s, line=%d", err.Type, err.Position.File, err.Position.Line) var output strings.Builder @@ -92,19 +119,19 @@ func FormatError(err CompilerError) string { } else { location = relativePath + ":" } - output.WriteString(applyStyle(styles.FilePath, location)) + output.WriteString(styleText(styles.FilePath, location)) output.WriteString(" ") } // Error type and message - output.WriteString(applyStyle(typeStyle, prefix+":")) + output.WriteString(styleText(typeStyle, prefix+":")) output.WriteString(" ") output.WriteString(err.Message) output.WriteString("\n") // Context lines (Rust-like error rendering) if len(err.Context) > 0 && err.Position.Line > 0 { - output.WriteString(renderContext(err)) + output.WriteString(renderContext(err, styleText)) } // Hint for fixing the error @@ -112,7 +139,7 @@ func FormatError(err CompilerError) string { // dedicated Hint style; Info is visually distinct and non-alarming, which is // appropriate for actionable guidance. if err.Hint != "" { - output.WriteString(applyStyle(styles.Info, "hint: ")) + output.WriteString(styleText(styles.Info, "hint: ")) output.WriteString(err.Hint) output.WriteString("\n") } @@ -121,7 +148,7 @@ func FormatError(err CompilerError) string { } // renderContext renders source code context with line numbers and highlighting -func renderContext(err CompilerError) string { +func renderContext(err CompilerError, styleText func(lipgloss.Style, string) string) string { var output strings.Builder maxLineNum := err.Position.Line + len(err.Context)/2 @@ -134,7 +161,7 @@ func renderContext(err CompilerError) string { } lineNumStr := fmt.Sprintf("%*d", lineNumWidth, lineNum) - output.WriteString(applyStyle(styles.LineNumber, lineNumStr)) + output.WriteString(styleText(styles.LineNumber, lineNumStr)) output.WriteString(" | ") if lineNum == err.Position.Line { @@ -146,14 +173,14 @@ func renderContext(err CompilerError) string { if wordEnd < len(line) { after = line[wordEnd:] } - output.WriteString(applyStyle(styles.ContextLine, before)) - output.WriteString(applyStyle(styles.Highlight, highlightedPart)) - output.WriteString(applyStyle(styles.ContextLine, after)) + output.WriteString(styleText(styles.ContextLine, before)) + output.WriteString(styleText(styles.Highlight, highlightedPart)) + output.WriteString(styleText(styles.ContextLine, after)) } else { - output.WriteString(applyStyle(styles.Highlight, line)) + output.WriteString(styleText(styles.Highlight, line)) } } else { - output.WriteString(applyStyle(styles.ContextLine, line)) + output.WriteString(styleText(styles.ContextLine, line)) } output.WriteString("\n") @@ -161,7 +188,7 @@ func renderContext(err CompilerError) string { wordEnd := findWordEnd(line, err.Position.Column-1) wordLength := wordEnd - (err.Position.Column - 1) padding := strings.Repeat(" ", lineNumWidth+3+err.Position.Column-1) - pointer := applyStyle(styles.Error, strings.Repeat("^", wordLength)) + pointer := styleText(styles.Error, strings.Repeat("^", wordLength)) output.WriteString(padding) output.WriteString(pointer) output.WriteString("\n") @@ -178,17 +205,13 @@ func FormatSuccessMessage(message string) string { // FormatSuccessMessageStderr formats a success message for stderr output. func FormatSuccessMessageStderr(message string) string { - return formatSuccessMessageStderrWithTTY(message, isStderrTTY) + return applyStderrStyle(styles.Success, "โœ“ ") + message } func formatSuccessMessageWithTTY(message string, ttyCheck func() bool, environ []string) string { return applyStdoutStyleWithTTY(styles.Success, "โœ“ ", ttyCheck, environ) + message } -func formatSuccessMessageStderrWithTTY(message string, ttyCheck func() bool) string { - return applyStyleWithTTY(styles.Success, "โœ“ ", ttyCheck) + message -} - // FormatInfoMessage formats an informational message func FormatInfoMessage(message string) string { return formatInfoMessageWithTTY(message, isTTY, stdoutEnviron()) @@ -196,20 +219,16 @@ func FormatInfoMessage(message string) string { // FormatInfoMessageStderr formats an informational message for stderr output. func FormatInfoMessageStderr(message string) string { - return formatInfoMessageStderrWithTTY(message, isStderrTTY) + return applyStderrStyle(styles.Info, "i ") + message } func formatInfoMessageWithTTY(message string, ttyCheck func() bool, environ []string) string { return applyStdoutStyleWithTTY(styles.Info, "i ", ttyCheck, environ) + message } -func formatInfoMessageStderrWithTTY(message string, ttyCheck func() bool) string { - return applyStyleWithTTY(styles.Info, "i ", ttyCheck) + message -} - // FormatTableHeaderStderr formats table header text for stderr output. func FormatTableHeaderStderr(text string) string { - return formatTableHeaderWithTTY(text, isStderrTTY) + return applyStderrStyle(styles.TableHeader, text) } func formatTableHeaderWithTTY(text string, ttyCheck func() bool) string { @@ -223,7 +242,7 @@ func FormatWarningMessage(message string) string { // FormatWarningMessageStderr formats a warning message for stderr output. func FormatWarningMessageStderr(message string) string { - return applyStyleWithTTY(styles.Warning, "โš  ", isStderrTTY) + message + return applyStderrStyle(styles.Warning, "โš  ") + message } // RenderTable renders a formatted table using lipgloss/table package @@ -323,7 +342,7 @@ func FormatCommandMessage(command string) string { // FormatCommandMessageStderr formats a command execution message for stderr output. func FormatCommandMessageStderr(command string) string { - return applyStyleWithTTY(styles.Command, "$ ", isStderrTTY) + command + return applyStderrStyle(styles.Command, "$ ") + command } // FormatProgressMessage formats a progress/activity message @@ -331,6 +350,11 @@ func FormatProgressMessage(message string) string { return applyStyle(styles.Progress, "โ–ธ ") + message } +// FormatProgressMessageStderr formats a progress message for stderr output. +func FormatProgressMessageStderr(message string) string { + return applyStderrStyle(styles.Progress, "โ–ธ ") + message +} + // FormatPromptMessage formats a user prompt message func FormatPromptMessage(message string) string { return applyStyle(styles.Prompt, "? ") + message @@ -348,25 +372,21 @@ func FormatListItem(item string) string { // FormatListItemStderr formats a list item for stderr output. func FormatListItemStderr(item string) string { - return formatListItemStderrWithTTY(item, isStderrTTY) + return applyStderrStyle(styles.ListItem, " โ€ข "+item) } func formatListItemWithTTY(item string, ttyCheck func() bool, environ []string) string { return applyStdoutStyleWithTTY(styles.ListItem, " โ€ข "+item, ttyCheck, environ) } -func formatListItemStderrWithTTY(item string, ttyCheck func() bool) string { - return applyStyleWithTTY(styles.ListItem, " โ€ข "+item, ttyCheck) -} - // FormatErrorMessage formats a simple error message (for stderr output) func FormatErrorMessage(message string) string { - return applyStyleWithTTY(styles.Error, "โœ— ", isStderrTTY) + message + return applyStderrStyle(styles.Error, "โœ— ") + message } // FormatErrorTextStderr formats plain error-styled text for stderr output. func FormatErrorTextStderr(text string) string { - return formatErrorTextWithTTY(text, isStderrTTY) + return applyStderrStyle(styles.Error, text) } func formatErrorTextWithTTY(text string, ttyCheck func() bool) string { @@ -460,17 +480,13 @@ func FormatSectionHeader(header string) string { // FormatSectionHeaderStderr formats a section header for stderr output. func FormatSectionHeaderStderr(header string) string { - return formatSectionHeaderStderrWithTTY(header, isStderrTTY) + return applyStderrStyle(styles.Header, header) } func formatSectionHeaderWithTTY(header string, ttyCheck func() bool, environ []string) string { return applyStdoutStyleWithTTY(styles.Header, header, ttyCheck, environ) } -func formatSectionHeaderStderrWithTTY(header string, ttyCheck func() bool) string { - return applyStyleWithTTY(styles.Header, header, ttyCheck) -} - // RenderTitleBox renders a title with a double border box in TTY mode func RenderTitleBox(title string, width int) []string { if tty.IsStderrTerminal() { diff --git a/pkg/console/console_formatting_test.go b/pkg/console/console_formatting_test.go index a09e634fe52..883e2803f19 100644 --- a/pkg/console/console_formatting_test.go +++ b/pkg/console/console_formatting_test.go @@ -303,6 +303,53 @@ func TestFormatTableHeaderWithTTY(t *testing.T) { }) } +func TestApplyStderrStyleWithTTY(t *testing.T) { + t.Run("plain text when stderr is not tty", func(t *testing.T) { + result := applyStderrStyleWithTTY(styles.Warning, "warning", func() bool { return false }, []string{"TERM=xterm-256color"}) + if result != "warning" { + t.Fatalf("applyStderrStyleWithTTY() = %q, want plain text", result) + } + }) + + t.Run("styled text when stderr is tty", func(t *testing.T) { + result := applyStderrStyleWithTTY(styles.Warning, "warning", func() bool { return true }, []string{"TERM=xterm-256color"}) + if !strings.Contains(result, "\x1b[") { + t.Fatalf("applyStderrStyleWithTTY() = %q, want ANSI styling", result) + } + }) + + t.Run("no color disables styling", func(t *testing.T) { + result := applyStderrStyleWithTTY(styles.Warning, "warning", func() bool { return true }, []string{"TERM=xterm-256color", "NO_COLOR=1"}) + if strings.Contains(result, "\x1b[") { + t.Fatalf("applyStderrStyleWithTTY() = %q, want ANSI-free text", result) + } + }) +} + +func TestFormatErrorStderrWithTTY(t *testing.T) { + err := CompilerError{ + Position: ErrorPosition{File: "workflow.md", Line: 2, Column: 3}, + Type: "warning", + Message: "deprecated field", + Hint: "use the replacement", + } + + styled := formatErrorStderrWithTTY(err, func() bool { return true }, []string{"TERM=xterm-256color"}) + if !strings.Contains(styled, "\x1b[") { + t.Fatalf("formatErrorStderrWithTTY() = %q, want ANSI styling", styled) + } + for _, text := range []string{"workflow.md:2:3:", "warning:", "hint:"} { + if !strings.Contains(styled, text) { + t.Fatalf("formatErrorStderrWithTTY() missing %q in %q", text, styled) + } + } + + plain := formatErrorStderrWithTTY(err, func() bool { return false }, []string{"TERM=xterm-256color"}) + if strings.Contains(plain, "\x1b[") { + t.Fatalf("formatErrorStderrWithTTY() = %q, want ANSI-free text", plain) + } +} + func TestFormatErrorTextWithTTY(t *testing.T) { t.Run("plain text when not tty", func(t *testing.T) { result := formatErrorTextWithTTY("boom", func() bool { return false }) diff --git a/pkg/console/console_wasm.go b/pkg/console/console_wasm.go index 383c8be13d0..bdb66b3ac4f 100644 --- a/pkg/console/console_wasm.go +++ b/pkg/console/console_wasm.go @@ -65,6 +65,8 @@ func FormatError(err CompilerError) string { return output.String() } +func FormatErrorStderr(err CompilerError) string { return FormatError(err) } + func FormatSuccessMessage(message string) string { return "โœ“ " + message } func FormatSuccessMessageStderr(message string) string { return "โœ“ " + message } func FormatInfoMessage(message string) string { return "i " + message } @@ -78,14 +80,17 @@ func FormatLocationMessage(message string) string { return "~ " + message } func FormatCommandMessage(command string) string { return "$ " + command } func FormatCommandMessageStderr(command string) string { return "$ " + command } func FormatProgressMessage(message string) string { return "โ–ธ " + message } -func FormatPromptMessage(message string) string { return "? " + message } -func FormatCountMessage(message string) string { return "# " + message } -func FormatVerboseMessage(message string) string { return "ยป " + message } -func FormatListHeader(header string) string { return header } -func FormatListItem(item string) string { return " โ€ข " + item } -func FormatListItemStderr(item string) string { return " โ€ข " + item } -func FormatSectionHeader(header string) string { return header } -func FormatSectionHeaderStderr(header string) string { return header } +func FormatProgressMessageStderr(message string) string { + return "โ–ธ " + message +} +func FormatPromptMessage(message string) string { return "? " + message } +func FormatCountMessage(message string) string { return "# " + message } +func FormatVerboseMessage(message string) string { return "ยป " + message } +func FormatListHeader(header string) string { return header } +func FormatListItem(item string) string { return " โ€ข " + item } +func FormatListItemStderr(item string) string { return " โ€ข " + item } +func FormatSectionHeader(header string) string { return header } +func FormatSectionHeaderStderr(header string) string { return header } // FormatErrorChain formats an error and its full unwrapped chain. // In the WASM build there is no rich terminal styling; the top-level error diff --git a/pkg/workflow/agent_validation.go b/pkg/workflow/agent_validation.go index a4fb14dc302..24853dafa18 100644 --- a/pkg/workflow/agent_validation.go +++ b/pkg/workflow/agent_validation.go @@ -105,7 +105,7 @@ func (c *Compiler) validateAgentFile(workflowData *WorkflowData, markdownPath st } if c.verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage( + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr( "โœ“ Agent file exists: "+agentPath)) } @@ -227,7 +227,7 @@ func (c *Compiler) validateWebSearchSupport(tools map[string]any, engine CodingA // web-search is specified, check if the engine supports it if !engine.GetCapabilities().WebSearch { agentValidationLog.Printf("Engine %s does not natively support web-search tool, emitting warning", engine.GetID()) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Engine '%s' does not support the web-search tool. See https://github.github.com/gh-aw/guides/web-search/ for alternatives.", engine.GetID()))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Engine '%s' does not support the web-search tool. See https://github.github.com/gh-aw/guides/web-search/ for alternatives.", engine.GetID()))) c.IncrementWarningCount() } } @@ -246,7 +246,7 @@ func (c *Compiler) validateBareModeSupport(frontmatter map[string]any, engine Co if !engine.GetCapabilities().BareMode { agentValidationLog.Printf("Engine %s does not support bare mode, emitting warning", engine.GetID()) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Engine '%s' does not support bare mode (engine.bare: true). Bare mode is only supported for the 'copilot', 'claude', and 'pi' engines. The setting will be ignored.", engine.GetID()))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Engine '%s' does not support bare mode (engine.bare: true). Bare mode is only supported for the 'copilot' and 'claude' engines. The setting will be ignored.", engine.GetID()))) c.IncrementWarningCount() } } @@ -267,7 +267,7 @@ func (c *Compiler) validateWorkflowRunBranches(workflowData *WorkflowData, markd } if _, hasBranches := workflowRunMap["branches"]; hasBranches { if c.verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("โœ“ workflow_run trigger has branch restrictions")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("โœ“ workflow_run trigger has branch restrictions")) } return nil } diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index 198855c0108..e67e0bf7b41 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -143,7 +143,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP // Write the invalid YAML to a .invalid.yml file for inspection invalidFile := strings.TrimSuffix(lockFile, ".lock.yml") + ".invalid.yml" if writeErr := os.WriteFile(invalidFile, []byte(yamlContent), constants.FilePermPublic); writeErr == nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Invalid workflow YAML written to: "+console.ToRelativePath(invalidFile))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Invalid workflow YAML written to: "+console.ToRelativePath(invalidFile))) } return "", nil, nil, formattedErr } @@ -208,7 +208,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP // Write the invalid YAML to a .invalid.yml file for inspection invalidFile := strings.TrimSuffix(lockFile, ".lock.yml") + ".invalid.yml" if writeErr := os.WriteFile(invalidFile, []byte(yamlContent), constants.FilePermPublic); writeErr == nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Invalid workflow YAML written to: "+console.ToRelativePath(invalidFile))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Invalid workflow YAML written to: "+console.ToRelativePath(invalidFile))) } return "", nil, nil, formattedErr } @@ -240,7 +240,7 @@ func (c *Compiler) generateAndValidateYAML(workflowData *WorkflowData, markdownP return "", nil, nil, formatCompilerError(markdownPath, "error", fmt.Sprintf("repository feature validation failed: %v", err), err) } } else if c.verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Schema validation available but skipped (use SetSkipValidation(false) to enable)")) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Schema validation available but skipped (use SetSkipValidation(false) to enable)")) c.IncrementWarningCount() } @@ -280,7 +280,7 @@ func (c *Compiler) writeWorkflowOutput(lockFile, yamlContent string, markdownPat lockSize := console.FormatFileSize(lockFileInfo.Size()) maxSize := console.FormatFileSize(MaxLockFileSize) warningMsg := fmt.Sprintf("Generated lock file size (%s) exceeds recommended maximum size (%s)", lockSize, maxSize) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warningMsg)) } } } @@ -288,15 +288,15 @@ func (c *Compiler) writeWorkflowOutput(lockFile, yamlContent string, markdownPat // Display success message with file size if we generated a lock file (unless quiet mode) if !c.quiet { if c.noEmit { - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(console.ToRelativePath(markdownPath))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(console.ToRelativePath(markdownPath))) } else { // Get the size of the generated lock file for display if lockFileInfo, err := os.Stat(lockFile); err == nil { lockSize := console.FormatFileSize(lockFileInfo.Size()) - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("%s (%s)", console.ToRelativePath(markdownPath), lockSize))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(fmt.Sprintf("%s (%s)", console.ToRelativePath(markdownPath), lockSize))) } else { // Fallback to original display if we can't get file info - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(console.ToRelativePath(markdownPath))) + fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr(console.ToRelativePath(markdownPath))) } } } @@ -379,7 +379,7 @@ func (c *Compiler) validateTemplateInjection(yamlContent, lockFile, markdownPath // Write the invalid YAML to a .invalid.yml file for inspection invalidFile := strings.TrimSuffix(lockFile, ".lock.yml") + ".invalid.yml" if writeErr := os.WriteFile(invalidFile, []byte(yamlContent), constants.FilePermPublic); writeErr == nil { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage("Workflow with template injection risks written to: "+console.ToRelativePath(invalidFile))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr("Workflow with template injection risks written to: "+console.ToRelativePath(invalidFile))) } return formattedErr } diff --git a/pkg/workflow/compiler_error_formatter.go b/pkg/workflow/compiler_error_formatter.go index b443db477b1..0819a23255d 100644 --- a/pkg/workflow/compiler_error_formatter.go +++ b/pkg/workflow/compiler_error_formatter.go @@ -73,7 +73,7 @@ func isFormattedCompilerError(err error) bool { // cause: optional underlying error to wrap (use nil for validation errors) func formatCompilerErrorWithPosition(filePath string, line int, column int, errType string, message string, cause error) error { compilerErrorLog.Printf("Formatting compiler error: file=%s, line=%d, column=%d, type=%s, message=%s", filePath, line, column, errType, message) - formattedErr := console.FormatError(console.CompilerError{ + formattedErr := console.FormatErrorStderr(console.CompilerError{ Position: console.ErrorPosition{ File: filePath, Line: line, @@ -101,7 +101,7 @@ func formatCompilerErrorWithPosition(filePath string, line int, column int, errT // context: source code lines around the error for Rust-like snippet rendering func formatCompilerErrorWithContext(filePath string, line int, column int, errType string, message string, cause error, context []string) error { compilerErrorLog.Printf("Formatting compiler error with context: file=%s, line=%d, column=%d, type=%s, context=%d lines", filePath, line, column, errType, len(context)) - formattedErr := console.FormatError(console.CompilerError{ + formattedErr := console.FormatErrorStderr(console.CompilerError{ Position: console.ErrorPosition{ File: filePath, Line: line, @@ -120,7 +120,7 @@ func formatCompilerErrorWithContext(filePath string, line int, column int, errTy // msgType: the message type ("error" or "warning") // message: the message text func formatCompilerMessage(filePath string, msgType string, message string) string { - return console.FormatError(console.CompilerError{ + return console.FormatErrorStderr(console.CompilerError{ Position: console.ErrorPosition{ File: filePath, Line: 0, diff --git a/pkg/workflow/compiler_orchestrator_tools.go b/pkg/workflow/compiler_orchestrator_tools.go index 4e9a1fc71ce..b62ee68b51c 100644 --- a/pkg/workflow/compiler_orchestrator_tools.go +++ b/pkg/workflow/compiler_orchestrator_tools.go @@ -294,10 +294,10 @@ func (c *Compiler) adjustToolsForEngineCapabilities(frontmatter map[string]any, if agenticEngine.GetCapabilities().ToolsAllowlist { return tools } - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Using experimental %s support (engine: %s)", agenticEngine.GetDisplayName(), agenticEngine.GetID()))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("Using experimental %s support (engine: %s)", agenticEngine.GetDisplayName(), agenticEngine.GetID()))) c.IncrementWarningCount() if _, hasTools := frontmatter["tools"]; hasTools { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("'tools' section ignored when using engine: %s (%s doesn't support MCP tool allow-listing)", agenticEngine.GetID(), agenticEngine.GetDisplayName()))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(fmt.Sprintf("'tools' section ignored when using engine: %s (%s doesn't support MCP tool allow-listing)", agenticEngine.GetID(), agenticEngine.GetDisplayName()))) c.IncrementWarningCount() } return map[string]any{"github": map[string]any{}} diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 105ae537f77..a55b5b75d72 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -2,6 +2,7 @@ package workflow import ( "context" + "maps" "os" "github.com/github/gh-aw/pkg/logger" @@ -58,6 +59,7 @@ type Compiler struct { ctx context.Context // Context for network operations (e.g. SHA resolution); defaults to context.Background() verbose bool quiet bool // If true, suppress success messages (for interactive mode) + batchMode bool // If true, aggregate repetitive notices across workflows engineOverride string customOutput string // If set, output will be written to this path instead of default location version string // Version of the extension @@ -108,6 +110,8 @@ type Compiler struct { ghesArtifactCompat bool // If true, GHES compatibility mode is enabled; artifact actions still use latest non-v3 pins ownerTypeCache map[string]string // Cached GitHub owner type ("User"/"Organization"/"") keyed by owner login; not goroutine-safe (Compiler is used sequentially) copilotRequestsTipShown map[string]bool // Tracks markdown paths that already emitted the copilot-requests enable tip in this compiler instance + copilotTipNeeded bool // Tracks whether batch output should include the copilot-requests enable tip + featureUsage map[string]int // Counts experimental feature usage across workflows in batch mode permissionWarningShown map[string]string // Tracks markdown paths and last warning fingerprint (frontmatter hash when available, otherwise formatted warning text) allowedDomainsCache map[string]allowedDomain // Cached allowed-domains per markdown path with the frontmatter hash that produced it // modelPricingResolver is an optional callback for resolving per-token pricing of models that @@ -151,6 +155,7 @@ func NewCompiler(opts ...CompilerOption) *Compiler { priorManifests: make(map[string]*GHAWManifest), ownerTypeCache: make(map[string]string), // Initialize owner-type cache (keyed by owner login) copilotRequestsTipShown: make(map[string]bool), // Initialize one-time tip tracking (keyed by markdown path) + featureUsage: make(map[string]int), // Initialize batch feature usage counts permissionWarningShown: make(map[string]string), // Initialize one-time permission warning tracking (keyed by markdown path) allowedDomainsCache: make(map[string]allowedDomain), // Initialize allowed-domains cache (keyed by markdown path) gitRoot: gitRoot, // Auto-detected git root @@ -199,6 +204,35 @@ func (c *Compiler) SetQuiet(quiet bool) { c.quiet = quiet } +// SetBatchMode configures whether repetitive notices should be aggregated. +func (c *Compiler) SetBatchMode(batchMode bool) { + c.batchMode = batchMode +} + +// GetExperimentalFeatureUsage returns experimental feature usage counts collected in batch mode. +func (c *Compiler) GetExperimentalFeatureUsage() map[string]int { + usage := make(map[string]int, len(c.featureUsage)) + maps.Copy(usage, c.featureUsage) + return usage +} + +// CopilotRequestsTipNeeded reports whether batch output should show the token-based inference tip. +func (c *Compiler) CopilotRequestsTipNeeded() bool { + return c.copilotTipNeeded +} + +// SetExperimentalFeatureUsage replaces the experimental feature usage map. +// Intended for use in tests that need to exercise aggregation output. +func (c *Compiler) SetExperimentalFeatureUsage(usage map[string]int) { + c.featureUsage = usage +} + +// SetCopilotTipNeeded sets whether the Copilot billing tip should be shown. +// Intended for use in tests that need to exercise aggregation output. +func (c *Compiler) SetCopilotTipNeeded(needed bool) { + c.copilotTipNeeded = needed +} + // SetNoEmit configures whether to validate without generating lock files func (c *Compiler) SetNoEmit(noEmit bool) { c.noEmit = noEmit diff --git a/pkg/workflow/compiler_validators.go b/pkg/workflow/compiler_validators.go index 46d5f28b4af..1ff80d6cbb2 100644 --- a/pkg/workflow/compiler_validators.go +++ b/pkg/workflow/compiler_validators.go @@ -50,7 +50,7 @@ func (c *Compiler) validateExpressions(workflowData *WorkflowData, markdownPath // so they are counted and consistently formatted with all other warnings. for _, w := range subAgentWarnings { expressionValidationLog.Printf("%s", w) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(w)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(w)) c.IncrementWarningCount() } if err != nil { @@ -299,7 +299,7 @@ func (c *Compiler) emitGeneralToolWarnings(workflowData *WorkflowData, markdownP } if workflowData.SafeOutputs != nil && workflowData.SafeOutputs.AssignToAgent != nil && workflowData.SafeOutputs.GitHubApp != nil && workflowData.SafeOutputs.AssignToAgent.GitHubToken == "" { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage( + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr( "assign-to-agent does not support GitHub App tokens. "+ "The Copilot assignment API requires a fine-grained PAT. "+ "The token fallback chain (GH_AW_AGENT_TOKEN || GH_AW_GITHUB_TOKEN || GITHUB_TOKEN) will be used automatically. "+ @@ -335,12 +335,16 @@ func (c *Compiler) emitExperimentalFeatureWarnings(workflowData *WorkflowData) { } for _, warning := range warnings { if warning.enabled { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warning.message)) + if c.batchMode { + c.featureUsage[warning.message]++ + } else { + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warning.message)) + } c.IncrementWarningCount() } } if shouldWarnSparseInteractionCells(workflowData) { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage( + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr( "experiments: potential sparse interaction cells detected (multiple active experiments with weighted traffic). "+ "Reporting should include factorial K1ร—K2 cell diagnostics before recommending promotion.")) c.IncrementWarningCount() @@ -360,8 +364,8 @@ func (c *Compiler) validateGitHubToolsAndPermissions(workflowData *WorkflowData, } originalToolsets := workflowData.ParsedTools.GitHub.Toolset.ToStringSlice() if slices.Contains(originalToolsets, "projects") { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("The 'projects' toolset requires additional authentication.")) - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("See: https://github.github.com/gh-aw/reference/auth-projects/")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("The 'projects' toolset requires additional authentication.")) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("See: https://github.github.com/gh-aw/reference/auth-projects/")) } } workflowLog.Printf("Validating permissions for agentic-workflows tool") diff --git a/pkg/workflow/compiler_validators_test.go b/pkg/workflow/compiler_validators_test.go index 4a6df8fb044..c59c6896ad0 100644 --- a/pkg/workflow/compiler_validators_test.go +++ b/pkg/workflow/compiler_validators_test.go @@ -142,7 +142,9 @@ func TestEmitExperimentalFeatureWarningsGHAWDetection(t *testing.T) { tests := []struct { name string features map[string]any + batchMode bool expectWarning bool + expectedUsage int }{ { name: "gh-aw-detection enabled produces experimental warning", @@ -151,6 +153,15 @@ func TestEmitExperimentalFeatureWarningsGHAWDetection(t *testing.T) { }, expectWarning: true, }, + { + name: "batch mode aggregates gh-aw-detection usage", + features: map[string]any{ + "gh-aw-detection": true, + }, + batchMode: true, + expectWarning: false, + expectedUsage: 1, + }, { name: "gh-aw-detection disabled does not produce experimental warning", features: map[string]any{ @@ -169,6 +180,7 @@ func TestEmitExperimentalFeatureWarningsGHAWDetection(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { compiler := NewCompiler() + compiler.SetBatchMode(tt.batchMode) workflowData := &WorkflowData{ Features: tt.features, } @@ -197,8 +209,11 @@ func TestEmitExperimentalFeatureWarningsGHAWDetection(t *testing.T) { assert.Positive(t, compiler.GetWarningCount()) } else { assert.NotContains(t, stderrOutput, expectedMessage) - assert.Zero(t, compiler.GetWarningCount()) + if tt.expectedUsage == 0 { + assert.Zero(t, compiler.GetWarningCount()) + } } + assert.Equal(t, tt.expectedUsage, compiler.GetExperimentalFeatureUsage()[expectedMessage]) }) } } diff --git a/pkg/workflow/compiler_yaml_step_conversion.go b/pkg/workflow/compiler_yaml_step_conversion.go index 8db3c540789..90589354d46 100644 --- a/pkg/workflow/compiler_yaml_step_conversion.go +++ b/pkg/workflow/compiler_yaml_step_conversion.go @@ -110,7 +110,7 @@ func (c *Compiler) renderStepFromMap(out *strings.Builder, step map[string]any, if sanitized, warnings, changed := sanitizeRunStepExpressions(step); changed { stepConversionLog.Printf("Sanitized run-step expressions: %d warning(s) emitted", len(warnings)) for _, w := range warnings { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(w)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(w)) c.IncrementWarningCount() } step = sanitized diff --git a/pkg/workflow/engine_validation.go b/pkg/workflow/engine_validation.go index ad843a3f503..8ddc851d631 100644 --- a/pkg/workflow/engine_validation.go +++ b/pkg/workflow/engine_validation.go @@ -72,7 +72,7 @@ func (c *Compiler) validateEngineVersion(workflowData *WorkflowData) error { "and may introduce vulnerabilities or breaking changes. " + "Pin the engine version to a specific version for reproducibility and security." - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(warningMsg)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(warningMsg)) c.IncrementWarningCount() return nil } diff --git a/pkg/workflow/permissions_compiler_validator.go b/pkg/workflow/permissions_compiler_validator.go index bfa7888011c..16b7bfec537 100644 --- a/pkg/workflow/permissions_compiler_validator.go +++ b/pkg/workflow/permissions_compiler_validator.go @@ -179,8 +179,12 @@ Ensure proper audience validation and trust policies are configured.` } if shouldEmitCopilotRequestsEnableTip(workflowData, workflowPermissions) && !c.repositoryOwnerIsIndividualUser() { if !c.copilotRequestsTipShown[markdownPath] { - tipMsg := `Tip: set permissions.copilot-requests: write to use GitHub Actions token-based inference with the Copilot engine instead of a personal access token (COPILOT_GITHUB_TOKEN). This option requires that your organization has centralized Copilot billing enabled and may not be available in all organizations โ€” see https://github.github.com/gh-aw/reference/billing/ for details.` - fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "info", tipMsg)) + if c.batchMode { + c.copilotTipNeeded = true + } else { + tipMsg := `Tip: set permissions.copilot-requests: write to use GitHub Actions token-based inference with the Copilot engine instead of a personal access token (COPILOT_GITHUB_TOKEN). This option requires that your organization has centralized Copilot billing enabled and may not be available in all organizations โ€” see https://github.github.com/gh-aw/reference/billing/ for details.` + fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "info", tipMsg)) + } c.copilotRequestsTipShown[markdownPath] = true } } diff --git a/pkg/workflow/push_to_pull_request_branch_validation.go b/pkg/workflow/push_to_pull_request_branch_validation.go index b09434faee9..93fcb3cfc4a 100644 --- a/pkg/workflow/push_to_pull_request_branch_validation.go +++ b/pkg/workflow/push_to_pull_request_branch_validation.go @@ -126,7 +126,7 @@ func (c *Compiler) validatePushToPullRequestBranchWarnings(safeOutputs *SafeOutp " fetch: [\"*\"] # fetch all remote branches", " fetch-depth: 0 # fetch full history", }, "\n") - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(msg)) c.IncrementWarningCount() } } @@ -144,7 +144,7 @@ func (c *Compiler) validatePushToPullRequestBranchWarnings(safeOutputs *SafeOutp " required-title-prefix: \"[bot] \" # only PRs whose title starts with this prefix", " required-labels: [automated] # only PRs that carry all of these labels", }, "\n") - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessageStderr(msg)) c.IncrementWarningCount() } } diff --git a/pkg/workflow/schedule_preprocessing.go b/pkg/workflow/schedule_preprocessing.go index c850dc7edf6..df3555c48b3 100644 --- a/pkg/workflow/schedule_preprocessing.go +++ b/pkg/workflow/schedule_preprocessing.go @@ -397,7 +397,7 @@ func (c *Compiler) createTriggerParseError(filePath, content, triggerStr string, } // Format and return the error - formattedErr := console.FormatError(compilerErr) + formattedErr := console.FormatErrorStderr(compilerErr) return errors.New(formattedErr) } @@ -467,8 +467,8 @@ func (c *Compiler) addDailyCronWarning(cronExpr string) { // Construct the warning message warningMsg := fmt.Sprintf( - "Schedule uses fixed daily time (%s:%s UTC). Consider using fuzzy schedule 'daily' instead to distribute workflow execution times and reduce load spikes.", - hour, minute, + "Schedule uses fixed daily time (%s UTC). Consider using fuzzy schedule 'daily' instead to distribute workflow execution times and reduce load spikes.", + formatCronTime(hour, minute), ) c.emitScheduleWarning(warningMsg) @@ -524,14 +524,24 @@ func (c *Compiler) addWeeklyCronWarning(cronExpr string) { // Construct the warning message warningMsg := fmt.Sprintf( - "Schedule uses fixed weekly time (%s %s:%s UTC). Consider using fuzzy schedule 'weekly on %s' instead to distribute workflow execution times and reduce load spikes.", - weekdayName, hour, minute, strings.ToLower(weekdayName), + "Schedule uses fixed weekly time (%s %s UTC). Consider using fuzzy schedule 'weekly on %s' instead to distribute workflow execution times and reduce load spikes.", + weekdayName, formatCronTime(hour, minute), strings.ToLower(weekdayName), ) c.emitScheduleWarning(warningMsg) } } +func formatCronTime(hour, minute string) string { + if len(hour) == 1 { + hour = "0" + hour + } + if len(minute) == 1 { + minute = "0" + minute + } + return hour + ":" + minute +} + func (c *Compiler) emitScheduleWarning(warning string) { // This warning is added to the warning count. // It will be collected and displayed by the compilation process. diff --git a/pkg/workflow/strict_mode_permissions_validation.go b/pkg/workflow/strict_mode_permissions_validation.go index cae69c3119d..448a381e876 100644 --- a/pkg/workflow/strict_mode_permissions_validation.go +++ b/pkg/workflow/strict_mode_permissions_validation.go @@ -178,7 +178,7 @@ func (c *Compiler) validateStrictFirewall(engineID string, networkPermissions *N infoMsg := "recommend using ecosystem identifiers instead of individual domain names for better maintainability: " + strings.Join(suggestions, ", ") - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(infoMsg)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(infoMsg)) } }