diff --git a/pkg/cli/logs_empty_runs_test.go b/pkg/cli/logs_empty_runs_test.go index fce953a2ce9..0507bbef145 100644 --- a/pkg/cli/logs_empty_runs_test.go +++ b/pkg/cli/logs_empty_runs_test.go @@ -5,7 +5,6 @@ package cli import ( "bytes" "encoding/json" - "os" "testing" "github.com/github/gh-aw/pkg/testutil" @@ -31,28 +30,18 @@ func TestBuildLogsDataEmptyRuns(t *testing.T) { // TestRenderLogsJSONEmptyRuns tests that JSON rendering works correctly with zero runs func TestRenderLogsJSONEmptyRuns(t *testing.T) { + t.Parallel() tmpDir := testutil.TempDir(t, "test-*") // Create logs data with no runs logsData := buildLogsData([]ProcessedRun{}, tmpDir, nil) - // Redirect stdout to capture JSON output - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - // Render JSON - err := renderLogsJSON(logsData, true) + // Render JSON to buffer + var buf bytes.Buffer + err := renderLogsJSONToWriter(&buf, logsData, true) if err != nil { t.Fatalf("Failed to render JSON: %v", err) } - - // Restore stdout and read captured output - w.Close() - os.Stdout = oldStdout - - var buf bytes.Buffer - _, _ = buf.ReadFrom(r) output := buf.String() // Verify it's valid JSON diff --git a/pkg/cli/logs_format_compact.go b/pkg/cli/logs_format_compact.go index 839c133501c..28a32b3ad53 100644 --- a/pkg/cli/logs_format_compact.go +++ b/pkg/cli/logs_format_compact.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "io" "os" "strconv" "strings" @@ -41,8 +42,9 @@ func workflowIDFromRun(path, name string) string { return id } -// renderLogsCompact outputs maximally information-dense output optimized for agentic consumption. -// Designed for LLM context windows: minimal formatting, no decoration, structured but flat. +// renderLogsCompactToWriter outputs maximally information-dense output optimized for agentic +// consumption to w. Designed for LLM context windows: minimal formatting, no decoration, +// structured but flat. // // Format sections: // @@ -53,7 +55,7 @@ func workflowIDFromRun(path, name string) string { // [firewall] firewall summary with per-domain breakdown // [tools] top tool usage (only if present) // [mcp] MCP failures (only if present) -func renderLogsCompact(data LogsData) { +func renderLogsCompactToWriter(w io.Writer, data LogsData) { logsCompactLog.Printf("Rendering %d runs in compact format", data.Summary.TotalRuns) s := data.Summary @@ -97,16 +99,16 @@ func renderLogsCompact(data LogsData) { summaryParts = append(summaryParts, "acceptance="+fmt.Sprintf("%.0f%%", s.OutcomeAcceptanceRate*100)) } } - fmt.Fprintf(os.Stdout, "[summary] %s\n", strings.Join(summaryParts, " ")) + fmt.Fprintf(w, "[summary] %s\n", strings.Join(summaryParts, " ")) if len(data.Runs) == 0 { return } // [runs] aligned table using tabwriter - fmt.Fprintln(os.Stdout, "[runs]") - w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) - fmt.Fprintln(w, "RUNID\tWORKFLOW\tENGINE\tSTATUS\tDUR\tTOKENS\tAIC\tTURNS\tERR\tEVENT\tACTOR\tBRANCH") + fmt.Fprintln(w, "[runs]") + tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "RUNID\tWORKFLOW\tENGINE\tSTATUS\tDUR\tTOKENS\tAIC\tTURNS\tERR\tEVENT\tACTOR\tBRANCH") for _, r := range data.Runs { status := r.Conclusion @@ -127,19 +129,19 @@ func renderLogsCompact(data LogsData) { } wfID := workflowIDFromRun(r.WorkflowPath, r.WorkflowName) - fmt.Fprintf(w, "%d\t%s\t%s\t%s\t%s\t%d\t%s\t%d\t%d\t%s\t%s\t%s\n", + fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%d\t%s\t%d\t%d\t%s\t%s\t%s\n", r.RunID, wfID, r.EngineID, status, dur, r.TokenUsage, formatCompactAIC(r.AIC), r.Turns, r.ErrorCount, r.Event, actor, branch) } - w.Flush() + tw.Flush() // [errors] — aggregated error/warning messages if len(data.ErrorsAndWarnings) > 0 { - fmt.Fprintln(os.Stdout, "[errors]") + fmt.Fprintln(w, "[errors]") for _, ew := range data.ErrorsAndWarnings { msg := stringutil.Truncate(ew.Message, 120) - fmt.Fprintf(os.Stdout, "%s run=%d count=%d: %s\n", ew.Type, ew.RunID, ew.Count, msg) + fmt.Fprintf(w, "%s run=%d count=%d: %s\n", ew.Type, ew.RunID, ew.Count, msg) } } @@ -153,12 +155,12 @@ func renderLogsCompact(data LogsData) { } } if hasActionable { - fmt.Fprintln(os.Stdout, "[insights]") + fmt.Fprintln(w, "[insights]") for _, obs := range data.Observability { if obs.Severity == "info" { continue } - fmt.Fprintf(os.Stdout, "[%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) + fmt.Fprintf(w, "[%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) } } } @@ -166,51 +168,51 @@ func renderLogsCompact(data LogsData) { // [firewall] — summary + per-domain breakdown if data.FirewallLog != nil && data.FirewallLog.TotalRequests > 0 { fw := data.FirewallLog - fmt.Fprintf(os.Stdout, "[firewall] requests=%d allowed=%d blocked=%d\n", + fmt.Fprintf(w, "[firewall] requests=%d allowed=%d blocked=%d\n", fw.TotalRequests, fw.AllowedRequests, fw.BlockedRequests) if len(fw.RequestsByDomain) > 0 { for domain, counts := range fw.RequestsByDomain { if counts.Blocked > 0 { - fmt.Fprintf(os.Stdout, " %s allowed=%d blocked=%d\n", domain, counts.Allowed, counts.Blocked) + fmt.Fprintf(w, " %s allowed=%d blocked=%d\n", domain, counts.Allowed, counts.Blocked) } } } else if len(fw.BlockedDomains) > 0 { - fmt.Fprintf(os.Stdout, " blocked: %s\n", strings.Join(fw.BlockedDomains, " ")) + fmt.Fprintf(w, " blocked: %s\n", strings.Join(fw.BlockedDomains, " ")) } } // [tools] — top tools by call count if len(data.ToolUsage) > 0 { - fmt.Fprintln(os.Stdout, "[tools]") + fmt.Fprintln(w, "[tools]") limit := min(10, len(data.ToolUsage)) for i := range limit { t := data.ToolUsage[i] - fmt.Fprintf(os.Stdout, "%s calls=%d runs=%d\n", t.Name, t.TotalCalls, t.Runs) + fmt.Fprintf(w, "%s calls=%d runs=%d\n", t.Name, t.TotalCalls, t.Runs) } if len(data.ToolUsage) > limit { - fmt.Fprintf(os.Stdout, "... +%d more tools\n", len(data.ToolUsage)-limit) + fmt.Fprintf(w, "... +%d more tools\n", len(data.ToolUsage)-limit) } } // [mcp-failures] if len(data.MCPFailures) > 0 { - fmt.Fprintln(os.Stdout, "[mcp-failures]") + fmt.Fprintln(w, "[mcp-failures]") for _, f := range data.MCPFailures { - fmt.Fprintf(os.Stdout, "server=%s count=%d runs=%v\n", f.ServerName, f.Count, f.RunIDs) + fmt.Fprintf(w, "server=%s count=%d runs=%v\n", f.ServerName, f.Count, f.RunIDs) } } // [missing-tools] — missing tool summary if len(data.MissingTools) > 0 { - fmt.Fprintln(os.Stdout, "[missing-tools]") + fmt.Fprintln(w, "[missing-tools]") for _, mt := range data.MissingTools { - fmt.Fprintf(os.Stdout, "%s count=%d runs=%v\n", mt.Tool, mt.Count, mt.RunIDs) + fmt.Fprintf(w, "%s count=%d runs=%v\n", mt.Tool, mt.Count, mt.RunIDs) } } // [location] if data.LogsLocation != "" { - fmt.Fprintf(os.Stdout, "[location] %s\n", data.LogsLocation) + fmt.Fprintf(w, "[location] %s\n", data.LogsLocation) } // [hint] — dynamic artifact hint + static usage guidance rendered as a single line @@ -218,11 +220,16 @@ func renderLogsCompact(data LogsData) { if data.Message != "" { hint = data.Message + " " + hint } - fmt.Fprintf(os.Stdout, "[hint] %s\n", hint) + fmt.Fprintf(w, "[hint] %s\n", hint) } -// renderLogsCompactVerbose adds extra columns and sections for deeper analysis. -func renderLogsCompactVerbose(data LogsData) { +// renderLogsCompact outputs maximally information-dense output to os.Stdout. +func renderLogsCompact(data LogsData) { + renderLogsCompactToWriter(os.Stdout, data) +} + +// renderLogsCompactVerboseToWriter adds extra columns and sections for deeper analysis, writing to w. +func renderLogsCompactVerboseToWriter(w io.Writer, data LogsData) { logsCompactLog.Printf("Rendering %d runs in verbose compact format", data.Summary.TotalRuns) s := data.Summary @@ -263,16 +270,16 @@ func renderLogsCompactVerbose(data LogsData) { summaryParts = append(summaryParts, "waste="+fmt.Sprintf("%.0f%%", s.OutcomeWasteRate*100)) } } - fmt.Fprintf(os.Stdout, "[summary] %s\n", strings.Join(summaryParts, " ")) + fmt.Fprintf(w, "[summary] %s\n", strings.Join(summaryParts, " ")) if len(data.Runs) == 0 { return } // [runs] verbose aligned table - fmt.Fprintln(os.Stdout, "[runs]") - w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) - fmt.Fprintln(w, "RUNID\tWORKFLOW\tENGINE\tSTATUS\tDUR\tTOKENS\tAIC\tTURNS\tERR\tWARN\tEVENT\tACTOR\tTBT\tCLASS\tCREATED\tBRANCH") + fmt.Fprintln(w, "[runs]") + tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) + fmt.Fprintln(tw, "RUNID\tWORKFLOW\tENGINE\tSTATUS\tDUR\tTOKENS\tAIC\tTURNS\tERR\tWARN\tEVENT\tACTOR\tTBT\tCLASS\tCREATED\tBRANCH") for _, r := range data.Runs { status := r.Conclusion @@ -300,90 +307,95 @@ func renderLogsCompactVerbose(data LogsData) { } wfID := workflowIDFromRun(r.WorkflowPath, r.WorkflowName) - fmt.Fprintf(w, "%d\t%s\t%s\t%s\t%s\t%d\t%s\t%d\t%d\t%d\t%s\t%s\t%s\t%s\t%s\t%s\n", + fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%d\t%s\t%d\t%d\t%d\t%s\t%s\t%s\t%s\t%s\t%s\n", r.RunID, wfID, r.EngineID, status, dur, r.TokenUsage, formatCompactAIC(r.AIC), r.Turns, r.ErrorCount, r.WarningCount, r.Event, actor, tbt, classification, r.CreatedAt.Format("01-02 15:04"), r.Branch) } - w.Flush() + tw.Flush() // [errors] if len(data.ErrorsAndWarnings) > 0 { - fmt.Fprintln(os.Stdout, "[errors]") + fmt.Fprintln(w, "[errors]") for _, ew := range data.ErrorsAndWarnings { - fmt.Fprintf(os.Stdout, "%s run=%d count=%d: %s\n", ew.Type, ew.RunID, ew.Count, ew.Message) + fmt.Fprintf(w, "%s run=%d count=%d: %s\n", ew.Type, ew.RunID, ew.Count, ew.Message) } } // [insights] — all severities in verbose mode if len(data.Observability) > 0 { - fmt.Fprintln(os.Stdout, "[insights]") + fmt.Fprintln(w, "[insights]") for _, obs := range data.Observability { - fmt.Fprintf(os.Stdout, "[%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) + fmt.Fprintf(w, "[%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) } } // [firewall] — full breakdown if data.FirewallLog != nil && data.FirewallLog.TotalRequests > 0 { fw := data.FirewallLog - fmt.Fprintf(os.Stdout, "[firewall] requests=%d allowed=%d blocked=%d\n", + fmt.Fprintf(w, "[firewall] requests=%d allowed=%d blocked=%d\n", fw.TotalRequests, fw.AllowedRequests, fw.BlockedRequests) if len(fw.RequestsByDomain) > 0 { for domain, counts := range fw.RequestsByDomain { - fmt.Fprintf(os.Stdout, " %s allowed=%d blocked=%d\n", domain, counts.Allowed, counts.Blocked) + fmt.Fprintf(w, " %s allowed=%d blocked=%d\n", domain, counts.Allowed, counts.Blocked) } } } // [tools] if len(data.ToolUsage) > 0 { - fmt.Fprintln(os.Stdout, "[tools]") + fmt.Fprintln(w, "[tools]") for _, t := range data.ToolUsage { - fmt.Fprintf(os.Stdout, "%s calls=%d runs=%d\n", t.Name, t.TotalCalls, t.Runs) + fmt.Fprintf(w, "%s calls=%d runs=%d\n", t.Name, t.TotalCalls, t.Runs) } } // [mcp-tools] if data.MCPToolUsage != nil && len(data.MCPToolUsage.Summary) > 0 { - fmt.Fprintln(os.Stdout, "[mcp-tools]") + fmt.Fprintln(w, "[mcp-tools]") for _, t := range data.MCPToolUsage.Summary { - fmt.Fprintf(os.Stdout, "%s.%s calls=%d\n", t.ServerName, t.ToolName, t.CallCount) + fmt.Fprintf(w, "%s.%s calls=%d\n", t.ServerName, t.ToolName, t.CallCount) } } // [mcp-failures] if len(data.MCPFailures) > 0 { - fmt.Fprintln(os.Stdout, "[mcp-failures]") + fmt.Fprintln(w, "[mcp-failures]") for _, f := range data.MCPFailures { - fmt.Fprintf(os.Stdout, "server=%s count=%d runs=%v\n", f.ServerName, f.Count, f.RunIDs) + fmt.Fprintf(w, "server=%s count=%d runs=%v\n", f.ServerName, f.Count, f.RunIDs) } } // [missing-tools] if len(data.MissingTools) > 0 { - fmt.Fprintln(os.Stdout, "[missing-tools]") + fmt.Fprintln(w, "[missing-tools]") for _, mt := range data.MissingTools { - fmt.Fprintf(os.Stdout, "%s count=%d runs=%v\n", mt.Tool, mt.Count, mt.RunIDs) + fmt.Fprintf(w, "%s count=%d runs=%v\n", mt.Tool, mt.Count, mt.RunIDs) } } // [episodes] if len(data.Episodes) > 0 { - fmt.Fprintln(os.Stdout, "[episodes]") + fmt.Fprintln(w, "[episodes]") for _, ep := range data.Episodes { - fmt.Fprintf(os.Stdout, "%s runs=%d conf=%s duration=%s\n", + fmt.Fprintf(w, "%s runs=%d conf=%s duration=%s\n", ep.Kind, ep.TotalRuns, ep.Confidence, ep.TotalDuration) } } // [location] if data.LogsLocation != "" { - fmt.Fprintf(os.Stdout, "[location] %s\n", data.LogsLocation) + fmt.Fprintf(w, "[location] %s\n", data.LogsLocation) } } +// renderLogsCompactVerbose adds extra columns and sections for deeper analysis, writing to os.Stdout. +func renderLogsCompactVerbose(data LogsData) { + renderLogsCompactVerboseToWriter(os.Stdout, data) +} + func formatCompactAIC(value float64) string { if value <= 0 { return "-" diff --git a/pkg/cli/logs_format_tsv.go b/pkg/cli/logs_format_tsv.go index 20768dde1ce..6ae64ff8c4a 100644 --- a/pkg/cli/logs_format_tsv.go +++ b/pkg/cli/logs_format_tsv.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "io" "os" "strconv" "strings" @@ -19,7 +20,7 @@ func formatTSVSummaryTokens(totalTokens int) string { return strconv.Itoa(totalTokens) } -// renderLogsTSV outputs the logs data as tab-separated values for maximum token efficiency. +// renderLogsTSVToWriter outputs the logs data as tab-separated values to w. // This format is ~24x more compact than JSON, making it ideal for agentic consumption // where LLM context window tokens are the primary constraint. // @@ -28,12 +29,12 @@ func formatTSVSummaryTokens(totalTokens int) string { // Line 1: Summary line (total_runs, total_duration, total_tokens, total_turns, total_errors) // Line 2: Column headers // Lines 3+: One line per run with tab-separated fields -func renderLogsTSV(data LogsData) { +func renderLogsTSVToWriter(w io.Writer, data LogsData) { logsTSVLog.Printf("Rendering %d runs as TSV", data.Summary.TotalRuns) s := data.Summary // Summary line with key aggregates - fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %s tokens | %d turns | %d errors\n", + fmt.Fprintf(w, "# %d runs | %s duration | %s tokens | %d turns | %d errors\n", s.TotalRuns, s.TotalDuration, formatTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors) if len(data.Runs) == 0 { @@ -46,7 +47,7 @@ func renderLogsTSV(data LogsData) { "tokens", "aic", "turns", "errors", "event", "branch", "created_at", "classification", "url", } - fmt.Fprintln(os.Stdout, strings.Join(headers, "\t")) + fmt.Fprintln(w, strings.Join(headers, "\t")) // Rows for _, r := range data.Runs { @@ -84,20 +85,20 @@ func renderLogsTSV(data LogsData) { classification, url, } - fmt.Fprintln(os.Stdout, strings.Join(fields, "\t")) + fmt.Fprintln(w, strings.Join(fields, "\t")) } // Append observability insights as comments (high signal density) if len(data.Observability) > 0 { - fmt.Fprintln(os.Stdout, "# insights:") + fmt.Fprintln(w, "# insights:") for _, obs := range data.Observability { - fmt.Fprintf(os.Stdout, "# [%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) + fmt.Fprintf(w, "# [%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) } } // Append firewall summary if present if data.FirewallLog != nil && data.FirewallLog.TotalRequests > 0 { - fmt.Fprintf(os.Stdout, "# firewall: %d requests (%d allowed, %d blocked)\n", + fmt.Fprintf(w, "# firewall: %d requests (%d allowed, %d blocked)\n", data.FirewallLog.TotalRequests, data.FirewallLog.AllowedRequests, data.FirewallLog.BlockedRequests) } @@ -107,16 +108,21 @@ func renderLogsTSV(data LogsData) { for engine, count := range data.Summary.EngineCounts { parts = append(parts, fmt.Sprintf("%s:%d", engine, count)) } - fmt.Fprintf(os.Stdout, "# engines: %s\n", strings.Join(parts, " ")) + fmt.Fprintf(w, "# engines: %s\n", strings.Join(parts, " ")) } } -// renderLogsTSVVerbose outputs a more detailed TSV with additional columns for audit use. -func renderLogsTSVVerbose(data LogsData) { +// renderLogsTSV outputs the logs data as tab-separated values to os.Stdout. +func renderLogsTSV(data LogsData) { + renderLogsTSVToWriter(os.Stdout, data) +} + +// renderLogsTSVVerboseToWriter outputs a more detailed TSV with additional columns for audit use to w. +func renderLogsTSVVerboseToWriter(w io.Writer, data LogsData) { logsTSVLog.Printf("Rendering %d runs as verbose TSV", data.Summary.TotalRuns) s := data.Summary - fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %s tokens | %d turns | %d errors | %d missing_tools | %d github_api_calls\n", + fmt.Fprintf(w, "# %d runs | %s duration | %s tokens | %d turns | %d errors | %d missing_tools | %d github_api_calls\n", s.TotalRuns, s.TotalDuration, formatTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors, s.TotalMissingTools, s.TotalGitHubAPICalls) if len(data.Runs) == 0 { @@ -130,7 +136,7 @@ func renderLogsTSVVerbose(data LogsData) { "event", "branch", "actor", "created_at", "tbt", "classification", "action_min", "display_title", "url", } - fmt.Fprintln(os.Stdout, strings.Join(headers, "\t")) + fmt.Fprintln(w, strings.Join(headers, "\t")) for _, r := range data.Runs { conclusion := r.Conclusion @@ -175,13 +181,18 @@ func renderLogsTSVVerbose(data LogsData) { displayTitle, r.URL, } - fmt.Fprintln(os.Stdout, strings.Join(fields, "\t")) + fmt.Fprintln(w, strings.Join(fields, "\t")) } if len(data.Observability) > 0 { - fmt.Fprintln(os.Stdout, "# insights:") + fmt.Fprintln(w, "# insights:") for _, obs := range data.Observability { - fmt.Fprintf(os.Stdout, "# [%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) + fmt.Fprintf(w, "# [%s] %s: %s\n", obs.Severity, obs.Title, obs.Summary) } } } + +// renderLogsTSVVerbose outputs a more detailed TSV with additional columns to os.Stdout. +func renderLogsTSVVerbose(data LogsData) { + renderLogsTSVVerboseToWriter(os.Stdout, data) +} diff --git a/pkg/cli/logs_format_tsv_test.go b/pkg/cli/logs_format_tsv_test.go index e13ce2a9fea..fb29b7174fd 100644 --- a/pkg/cli/logs_format_tsv_test.go +++ b/pkg/cli/logs_format_tsv_test.go @@ -3,22 +3,23 @@ package cli import ( + "bytes" "strings" "testing" ) func TestRenderLogsTSVSummaryPreservesTokenField(t *testing.T) { - output, _ := captureOutput(t, func() error { - renderLogsTSV(LogsData{ - Summary: LogsSummary{ - TotalRuns: 2, - TotalDuration: "8m0s", - TotalTurns: 5, - TotalErrors: 1, - }, - }) - return nil + t.Parallel() + var buf bytes.Buffer + renderLogsTSVToWriter(&buf, LogsData{ + Summary: LogsSummary{ + TotalRuns: 2, + TotalDuration: "8m0s", + TotalTurns: 5, + TotalErrors: 1, + }, }) + output := buf.String() lines := strings.Split(strings.TrimSpace(output), "\n") if len(lines) == 0 { @@ -30,20 +31,20 @@ func TestRenderLogsTSVSummaryPreservesTokenField(t *testing.T) { } func TestRenderLogsTSVVerboseSummaryPreservesTokenField(t *testing.T) { - output, _ := captureOutput(t, func() error { - renderLogsTSVVerbose(LogsData{ - Summary: LogsSummary{ - TotalRuns: 2, - TotalDuration: "8m0s", - TotalTokens: 1500, - TotalTurns: 5, - TotalErrors: 1, - TotalMissingTools: 2, - TotalGitHubAPICalls: 3, - }, - }) - return nil + t.Parallel() + var buf bytes.Buffer + renderLogsTSVVerboseToWriter(&buf, LogsData{ + Summary: LogsSummary{ + TotalRuns: 2, + TotalDuration: "8m0s", + TotalTokens: 1500, + TotalTurns: 5, + TotalErrors: 1, + TotalMissingTools: 2, + TotalGitHubAPICalls: 3, + }, }) + output := buf.String() lines := strings.Split(strings.TrimSpace(output), "\n") if len(lines) == 0 { diff --git a/pkg/cli/logs_json_clean_test.go b/pkg/cli/logs_json_clean_test.go index f4f217d9a38..a040ae56aa9 100644 --- a/pkg/cli/logs_json_clean_test.go +++ b/pkg/cli/logs_json_clean_test.go @@ -15,30 +15,19 @@ import ( // TestJSONOutputNotCorruptedByStderr is a unit test that verifies JSON output // is not corrupted when stderr messages are present. This simulates the CI test scenario. func TestJSONOutputNotCorruptedByStderr(t *testing.T) { + t.Parallel() tmpDir := testutil.TempDir(t, "test-json-clean-*") // Build logs data with empty runs (simulating no matching workflow runs) logsData := buildLogsData([]ProcessedRun{}, tmpDir, nil) - // Capture stdout (JSON output) - oldStdout := os.Stdout - stdoutR, stdoutW, _ := os.Pipe() - os.Stdout = stdoutW - - // Render JSON - err := renderLogsJSON(logsData, true) + // Render JSON to buffer + var jsonBuf bytes.Buffer + err := renderLogsJSONToWriter(&jsonBuf, logsData, true) if err != nil { t.Fatalf("Failed to render JSON: %v", err) } - - // Close stdout writer and restore - stdoutW.Close() - os.Stdout = oldStdout - - // Read stdout - var stdoutBuf bytes.Buffer - stdoutBuf.ReadFrom(stdoutR) - jsonOutput := stdoutBuf.String() + jsonOutput := jsonBuf.String() // Verify the output is valid JSON (no corruption) if len(jsonOutput) == 0 { diff --git a/pkg/cli/logs_json_test.go b/pkg/cli/logs_json_test.go index 23372a2d607..7e58c83e3c3 100644 --- a/pkg/cli/logs_json_test.go +++ b/pkg/cli/logs_json_test.go @@ -3,6 +3,7 @@ package cli import ( + "bytes" "encoding/json" "os" "path/filepath" @@ -247,24 +248,13 @@ func TestRenderLogsJSON(t *testing.T) { LogsLocation: tmpDir, } - // Redirect stdout to capture JSON output - oldStdout := os.Stdout - r, w, _ := os.Pipe() - os.Stdout = w - - // Render JSON - err := renderLogsJSON(logsData, true) + // Render JSON to buffer + var buf bytes.Buffer + err := renderLogsJSONToWriter(&buf, logsData, true) if err != nil { t.Fatalf("Failed to render JSON: %v", err) } - - // Restore stdout and read captured output - w.Close() - os.Stdout = oldStdout - - buf := make([]byte, 4096) - n, _ := r.Read(buf) - output := string(buf[:n]) + output := buf.String() // Verify it's valid JSON var parsedData LogsData diff --git a/pkg/cli/logs_orchestrator_render.go b/pkg/cli/logs_orchestrator_render.go index 5bd5a7e37ba..040fb8ae663 100644 --- a/pkg/cli/logs_orchestrator_render.go +++ b/pkg/cli/logs_orchestrator_render.go @@ -7,6 +7,7 @@ package cli import ( "fmt" + "io" "os" "path/filepath" @@ -147,7 +148,7 @@ func renderLogsOutput(processedRuns []ProcessedRun, opts renderLogsOutputOptions } // renderLogsArtifactHint writes a [hint] line to w when message is non-empty. -func renderLogsArtifactHint(w *os.File, message string) { +func renderLogsArtifactHint(w io.Writer, message string) { if message == "" { return } diff --git a/pkg/cli/logs_output_hint_test.go b/pkg/cli/logs_output_hint_test.go index 030d25530db..2eb01e5b1ec 100644 --- a/pkg/cli/logs_output_hint_test.go +++ b/pkg/cli/logs_output_hint_test.go @@ -3,6 +3,7 @@ package cli import ( + "bytes" "strings" "testing" "time" @@ -11,19 +12,19 @@ import ( ) func TestRenderLogsCompactEmitsSingleHintLine(t *testing.T) { - stdout, _ := captureOutput(t, func() error { - renderLogsCompact(LogsData{ - Summary: LogsSummary{TotalRuns: 1}, - Runs: []RunData{{ - RunID: 1, - WorkflowName: "logs", - Status: "completed", - CreatedAt: time.Now(), - }}, - Message: usageOnlyArtifactHintMessage(), - }) - return nil + t.Parallel() + var buf bytes.Buffer + renderLogsCompactToWriter(&buf, LogsData{ + Summary: LogsSummary{TotalRuns: 1}, + Runs: []RunData{{ + RunID: 1, + WorkflowName: "logs", + Status: "completed", + CreatedAt: time.Now(), + }}, + Message: usageOnlyArtifactHintMessage(), }) + stdout := buf.String() assert.Equal(t, 1, strings.Count(stdout, "[hint] "), "compact output should emit a single hint line") assert.Contains(t, stdout, usageOnlyArtifactHintMessage()) diff --git a/pkg/cli/logs_report.go b/pkg/cli/logs_report.go index 923b53cd078..45667ba2b7d 100644 --- a/pkg/cli/logs_report.go +++ b/pkg/cli/logs_report.go @@ -3,6 +3,7 @@ package cli import ( "encoding/json" "fmt" + "io" "os" "path/filepath" "strings" @@ -521,20 +522,26 @@ func deriveRunClassification(comparison *AuditComparisonData) string { return "normal" } -// renderLogsJSON outputs the logs data as JSON. +// renderLogsJSONToWriter outputs the logs data as JSON to w. // When verbose is false, audit-heavy fields are stripped for compact agentic consumption. -func renderLogsJSON(data LogsData, verbose bool) error { +func renderLogsJSONToWriter(w io.Writer, data LogsData, verbose bool) error { reportLog.Printf("Rendering logs data as JSON: %d runs, verbose=%v", data.Summary.TotalRuns, verbose) if !verbose { data = compactLogsData(data) } - encoder := json.NewEncoder(os.Stdout) + encoder := json.NewEncoder(w) encoder.SetIndent("", " ") return encoder.Encode(data) } +// renderLogsJSON outputs the logs data as JSON to os.Stdout. +// When verbose is false, audit-heavy fields are stripped for compact agentic consumption. +func renderLogsJSON(data LogsData, verbose bool) error { + return renderLogsJSONToWriter(os.Stdout, data, verbose) +} + // compactLogsData strips audit-heavy fields from LogsData for token-efficient agentic output. // Removes: comparison, behavior_fingerprint, task_domain, agentic_assessments, // token_usage_summary, experiments, ambient_context from each run. @@ -611,13 +618,13 @@ func writeSummaryFile(path string, data LogsData, verbose bool) error { return nil } -// renderLogsConsole outputs the logs data as formatted console output -func renderLogsConsole(data LogsData) { +// renderLogsConsoleToWriter outputs the logs data as formatted console output to w. +func renderLogsConsoleToWriter(w io.Writer, data LogsData) { reportLog.Printf("Rendering logs data to console: %d runs, %d errors, %d warnings", data.Summary.TotalRuns, data.Summary.TotalErrors, data.Summary.TotalWarnings) // Use unified console rendering for the entire logs data structure - fmt.Print(console.RenderStruct(data)) + fmt.Fprint(w, console.RenderStruct(data)) // Display concise summary at the end fmt.Fprintln(os.Stderr, "") // Blank line for spacing @@ -645,3 +652,8 @@ func renderLogsConsole(data LogsData) { renderObservabilityInsights(data.Observability) } } + +// renderLogsConsole outputs the logs data as formatted console output to os.Stdout. +func renderLogsConsole(data LogsData) { + renderLogsConsoleToWriter(os.Stdout, data) +} diff --git a/pkg/cli/logs_report_test.go b/pkg/cli/logs_report_test.go index d31b44e2f8e..d7771ffcfc5 100644 --- a/pkg/cli/logs_report_test.go +++ b/pkg/cli/logs_report_test.go @@ -3,6 +3,7 @@ package cli import ( + "bytes" "encoding/json" "os" "path/filepath" @@ -15,6 +16,7 @@ import ( // TestRenderLogsConsoleUnified tests the unified console rendering func TestRenderLogsConsoleUnified(t *testing.T) { + t.Parallel() // Create test data data := LogsData{ Summary: LogsSummary{ @@ -99,12 +101,13 @@ func TestRenderLogsConsoleUnified(t *testing.T) { // Test unified rendering - should not panic defer func() { if r := recover(); r != nil { - t.Errorf("renderLogsConsole panicked: %v", r) + t.Errorf("renderLogsConsoleToWriter panicked: %v", r) } }() - renderLogsConsole(data) - renderLogsConsole(data) + var buf bytes.Buffer + renderLogsConsoleToWriter(&buf, data) + renderLogsConsoleToWriter(&buf, data) } // TestBuildToolUsageSummaryPopulatesDisplay tests that buildToolUsageSummary works correctly