From e48da07db253b0ca57286088d3655dd009d7e268 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 09:05:25 +0000 Subject: [PATCH 1/4] Initial plan From 863ef1bec6c148822f5f56e1c600ce81400deb16 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 09:19:27 +0000 Subject: [PATCH 2/4] Replace raw fmt.Println with fmt.Fprintln(os.Stderr) for diagnostic output Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 2 +- pkg/workflow/cache.go | 5 +++-- pkg/workflow/claude_logs.go | 13 +++++++------ pkg/workflow/engine_firewall_support.go | 5 +++-- pkg/workflow/mcp_config_custom.go | 3 ++- pkg/workflow/mcp_renderer.go | 3 ++- pkg/workflow/push_to_pull_request_branch.go | 3 ++- pkg/workflow/stop_after.go | 18 +++++++++--------- 8 files changed, 29 insertions(+), 23 deletions(-) diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index b5c0e1015f4..966751cb98e 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -364,7 +364,7 @@ var versionCmd = &cobra.Command{ Use: "version", Short: "Show gh aw extension version information", RunE: func(cmd *cobra.Command, args []string) error { - fmt.Printf("%s version %s\n", string(constants.CLIExtensionPrefix), version) + fmt.Fprintf(os.Stderr, "%s version %s\n", string(constants.CLIExtensionPrefix), version) return nil }, } diff --git a/pkg/workflow/cache.go b/pkg/workflow/cache.go index 34a0125a1d2..a5ab60d9c0d 100644 --- a/pkg/workflow/cache.go +++ b/pkg/workflow/cache.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "os" "strings" "github.com/githubnext/gh-aw/pkg/logger" @@ -238,7 +239,7 @@ func generateCacheSteps(builder *strings.Builder, data *WorkflowData, verbose bo var topLevel map[string]any if err := yaml.Unmarshal([]byte(data.Cache), &topLevel); err != nil { if verbose { - fmt.Printf("Warning: Failed to parse cache configuration: %v\n", err) + fmt.Fprintf(os.Stderr, "Warning: Failed to parse cache configuration: %v\n", err) } return } @@ -247,7 +248,7 @@ func generateCacheSteps(builder *strings.Builder, data *WorkflowData, verbose bo cacheConfig, exists := topLevel["cache"] if !exists { if verbose { - fmt.Printf("Warning: No cache key found in parsed configuration\n") + fmt.Fprintf(os.Stderr, "Warning: No cache key found in parsed configuration\n") } return } diff --git a/pkg/workflow/claude_logs.go b/pkg/workflow/claude_logs.go index b523a751375..874145034f6 100644 --- a/pkg/workflow/claude_logs.go +++ b/pkg/workflow/claude_logs.go @@ -3,6 +3,7 @@ package workflow import ( "encoding/json" "fmt" + "os" "strings" "time" @@ -149,7 +150,7 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe // If that fails, try to parse as mixed format (debug logs + JSONL) claudeLogsLog.Print("JSON array parse failed, trying JSONL format") if verbose { - fmt.Printf("Failed to parse Claude log as JSON array, trying JSONL format: %v\n", err) + fmt.Fprintf(os.Stderr, "Failed to parse Claude log as JSON array, trying JSONL format: %v\n", err) } logEntries = []map[string]any{} @@ -208,7 +209,7 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe if err := json.Unmarshal([]byte(trimmedLine), &jsonEntry); err != nil { // Skip invalid JSON lines (could be partial debug output) if verbose { - fmt.Printf("Skipping invalid JSON line: %s\n", trimmedLine) + fmt.Fprintf(os.Stderr, "Skipping invalid JSON line: %s\n", trimmedLine) } continue } @@ -218,13 +219,13 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe if len(logEntries) == 0 { if verbose { - fmt.Printf("No valid JSON entries found in Claude log\n") + fmt.Fprintf(os.Stderr, "No valid JSON entries found in Claude log\n") } return metrics } if verbose { - fmt.Printf("Extracted %d JSON entries from mixed format Claude log\n", len(logEntries)) + fmt.Fprintf(os.Stderr, "Extracted %d JSON entries from mixed format Claude log\n", len(logEntries)) } } @@ -276,7 +277,7 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe } if verbose { - fmt.Printf("Extracted from Claude result payload: tokens=%d, cost=%.4f, turns=%d\n", + fmt.Fprintf(os.Stderr, "Extracted from Claude result payload: tokens=%d, cost=%.4f, turns=%d\n", metrics.TokenUsage, metrics.EstimatedCost, metrics.Turns) } break @@ -319,7 +320,7 @@ func (e *ClaudeEngine) parseClaudeJSONLog(logContent string, verbose bool) LogMe for _, seq := range metrics.ToolSequences { totalTools += len(seq) } - fmt.Printf("Claude parser extracted %d tool sequences with %d total tool calls\n", + fmt.Fprintf(os.Stderr, "Claude parser extracted %d tool sequences with %d total tool calls\n", len(metrics.ToolSequences), totalTools) } diff --git a/pkg/workflow/engine_firewall_support.go b/pkg/workflow/engine_firewall_support.go index d9e265fc711..b1c589ce2a0 100644 --- a/pkg/workflow/engine_firewall_support.go +++ b/pkg/workflow/engine_firewall_support.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "os" "github.com/githubnext/gh-aw/pkg/console" "github.com/githubnext/gh-aw/pkg/logger" @@ -71,7 +72,7 @@ func (c *Compiler) checkNetworkSupport(engine CodingAgentEngine, networkPermissi } // In non-strict mode, emit a warning - fmt.Println(console.FormatWarningMessage(message)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(message)) c.IncrementWarningCount() return nil @@ -99,7 +100,7 @@ func (c *Compiler) checkFirewallDisable(engine CodingAgentEngine, networkPermiss } // In non-strict mode, emit a warning - fmt.Println(console.FormatWarningMessage(message)) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(message)) c.IncrementWarningCount() } diff --git a/pkg/workflow/mcp_config_custom.go b/pkg/workflow/mcp_config_custom.go index 0aa57423337..8ec46557011 100644 --- a/pkg/workflow/mcp_config_custom.go +++ b/pkg/workflow/mcp_config_custom.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "os" "sort" "strings" @@ -122,7 +123,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma } } default: - fmt.Println(console.FormatWarningMessage(fmt.Sprintf("Custom MCP server '%s' has unsupported type '%s'. Supported types: stdio, http", toolName, mcpType))) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Custom MCP server '%s' has unsupported type '%s'. Supported types: stdio, http", toolName, mcpType))) return nil } diff --git a/pkg/workflow/mcp_renderer.go b/pkg/workflow/mcp_renderer.go index 4dadc0a59b6..c1722f8e99c 100644 --- a/pkg/workflow/mcp_renderer.go +++ b/pkg/workflow/mcp_renderer.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "os" "sort" "strings" "time" @@ -489,7 +490,7 @@ func HandleCustomMCPToolInSwitch( if toolConfig, ok := tools[toolName].(map[string]any); ok { if hasMcp, _ := hasMCPConfig(toolConfig); hasMcp { if err := renderFunc(yaml, toolName, toolConfig, isLast); err != nil { - fmt.Printf("Error generating custom MCP configuration for %s: %v\n", toolName, err) + fmt.Fprintf(os.Stderr, "Error generating custom MCP configuration for %s: %v\n", toolName, err) } return true } diff --git a/pkg/workflow/push_to_pull_request_branch.go b/pkg/workflow/push_to_pull_request_branch.go index a0de9aa986e..12c40b11c1d 100644 --- a/pkg/workflow/push_to_pull_request_branch.go +++ b/pkg/workflow/push_to_pull_request_branch.go @@ -2,6 +2,7 @@ package workflow import ( "fmt" + "os" "github.com/githubnext/gh-aw/pkg/logger" ) @@ -68,7 +69,7 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any) default: // Invalid value, use default and log warning if c.verbose { - fmt.Printf("Warning: invalid if-no-changes value '%s', using default 'warn'\n", ifNoChangesStr) + fmt.Fprintf(os.Stderr, "Warning: invalid if-no-changes value '%s', using default 'warn'\n", ifNoChangesStr) } pushToBranchConfig.IfNoChanges = "warn" } diff --git a/pkg/workflow/stop_after.go b/pkg/workflow/stop_after.go index ee8ac750635..342c6135250 100644 --- a/pkg/workflow/stop_after.go +++ b/pkg/workflow/stop_after.go @@ -78,16 +78,16 @@ func (c *Compiler) processStopAfterConfiguration(frontmatter map[string]any, wor stopAfterLog.Printf("Resolved stop time from %s to %s", originalStopTime, resolvedStopTime) if c.verbose && isRelativeStopTime(originalStopTime) { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Refreshed relative stop-after to: %s", resolvedStopTime))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Refreshed relative stop-after to: %s", resolvedStopTime))) } else if c.verbose && originalStopTime != resolvedStopTime { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Refreshed absolute stop-after from '%s' to: %s", originalStopTime, resolvedStopTime))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Refreshed absolute stop-after from '%s' to: %s", originalStopTime, resolvedStopTime))) } } else if existingStopTime != "" { // Preserve existing stop time during recompilation (default behavior) stopAfterLog.Printf("Preserving existing stop time from lock file: %s", existingStopTime) workflowData.StopTime = existingStopTime if c.verbose { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Preserving existing stop time from lock file: %s", existingStopTime))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Preserving existing stop time from lock file: %s", existingStopTime))) } } else { // First compilation or no existing stop time, generate new one @@ -100,9 +100,9 @@ func (c *Compiler) processStopAfterConfiguration(frontmatter map[string]any, wor workflowData.StopTime = resolvedStopTime if c.verbose && isRelativeStopTime(originalStopTime) { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Resolved relative stop-after to: %s", resolvedStopTime))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Resolved relative stop-after to: %s", resolvedStopTime))) } else if c.verbose && originalStopTime != resolvedStopTime { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Parsed absolute stop-after from '%s' to: %s", originalStopTime, resolvedStopTime))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Parsed absolute stop-after from '%s' to: %s", originalStopTime, resolvedStopTime))) } } } @@ -329,9 +329,9 @@ func (c *Compiler) processSkipIfMatchConfiguration(frontmatter map[string]any, w if c.verbose && workflowData.SkipIfMatch != nil { if workflowData.SkipIfMatch.Max == 1 { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Skip-if-match query configured: %s (max: 1 match)", workflowData.SkipIfMatch.Query))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skip-if-match query configured: %s (max: 1 match)", workflowData.SkipIfMatch.Query))) } else { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Skip-if-match query configured: %s (max: %d matches)", workflowData.SkipIfMatch.Query, workflowData.SkipIfMatch.Max))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skip-if-match query configured: %s (max: %d matches)", workflowData.SkipIfMatch.Query, workflowData.SkipIfMatch.Max))) } } @@ -349,9 +349,9 @@ func (c *Compiler) processSkipIfNoMatchConfiguration(frontmatter map[string]any, if c.verbose && workflowData.SkipIfNoMatch != nil { if workflowData.SkipIfNoMatch.Min == 1 { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Skip-if-no-match query configured: %s (min: 1 match)", workflowData.SkipIfNoMatch.Query))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skip-if-no-match query configured: %s (min: 1 match)", workflowData.SkipIfNoMatch.Query))) } else { - fmt.Println(console.FormatInfoMessage(fmt.Sprintf("Skip-if-no-match query configured: %s (min: %d matches)", workflowData.SkipIfNoMatch.Query, workflowData.SkipIfNoMatch.Min))) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Skip-if-no-match query configured: %s (min: %d matches)", workflowData.SkipIfNoMatch.Query, workflowData.SkipIfNoMatch.Min))) } } From 3aad1ff39527e57c0cdb99d3604af9821268f9a0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 09:27:07 +0000 Subject: [PATCH 3/4] Update AGENTS.md with explicit output routing guidelines Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- AGENTS.md | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e9f299f3d6d..f483c744808 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -394,6 +394,23 @@ fmt.Fprintln(os.Stderr, console.FormatErrorMessage(err.Error())) - **NEVER** use `fmt.Println()` or `fmt.Printf()` directly - all output should go to stderr - Use console formatting helpers with `os.Stderr` for consistent styling - For simple messages without console formatting: `fmt.Fprintf(os.Stderr, "message\n")` +- **Exception**: Structured output (JSON, hashes, graphs) goes to stdout for piping/redirection + +**Examples:** +```go +// ✅ CORRECT - Diagnostic output to stderr +fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Processing...")) +fmt.Fprintf(os.Stderr, "Warning: %s\n", msg) + +// ✅ CORRECT - Structured output to stdout +fmt.Println(string(jsonBytes)) // JSON output +fmt.Println(hash) // Hash output +fmt.Println(mermaidGraph) // Graph output + +// ❌ INCORRECT - Diagnostic output to stdout +fmt.Println("Processing...") // Should use stderr +fmt.Printf("Warning: %s\n", msg) // Should use stderr +``` ## Debug Logging @@ -525,18 +542,26 @@ if err != nil { ### Console Output Requirements ```go -// ✅ CORRECT - All output to stderr with console formatting +// ✅ CORRECT - All diagnostic output to stderr with console formatting fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Compiled successfully")) fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Processing workflow...")) fmt.Fprintln(os.Stderr, console.FormatWarningMessage("File has changes")) fmt.Fprintln(os.Stderr, console.FormatErrorMessage(err.Error())) -// ❌ INCORRECT - stdout, no formatting +// ✅ CORRECT - Structured output to stdout for piping/redirection +fmt.Println(string(jsonBytes)) // JSON output +fmt.Println(hash) // Hash output +fmt.Println(mermaidGraph) // Graph output + +// ❌ INCORRECT - Diagnostic output to stdout, no formatting fmt.Println("Success") fmt.Printf("Status: %s\n", status) ``` -**Exception**: JSON output goes to stdout, all other output to stderr +**Output Routing Rules (Unix Conventions):** +- **Diagnostic output** (messages, warnings, errors) → `stderr` +- **Structured data** (JSON, hashes, graphs) → `stdout` +- **Rationale**: Allows users to pipe/redirect data without diagnostic noise ### Help Text Standards From 5827eac615b940399021894ed8e01143e3667f1e Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Fri, 30 Jan 2026 06:21:35 -0800 Subject: [PATCH 4/4] Fix test capturing stdout when version output moved to stderr (#12750) --- cmd/gh-aw/main_entry_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/gh-aw/main_entry_test.go b/cmd/gh-aw/main_entry_test.go index 010cb58d1ca..6265122509c 100644 --- a/cmd/gh-aw/main_entry_test.go +++ b/cmd/gh-aw/main_entry_test.go @@ -347,7 +347,8 @@ func TestMainFunctionExecutionPath(t *testing.T) { cmd.Dir = "." // This should run successfully (exit code 0) even if no workflows found - output, err := cmd.Output() + // Use CombinedOutput to capture both stdout and stderr (version now outputs to stderr) + output, err := cmd.CombinedOutput() if err != nil { // Check if it's just a non-zero exit (which is okay for some commands) if exitError, ok := err.(*exec.ExitError); ok {