Skip to content

pkg/cli: add io.Writer params to renderLogs* formatters; kill captureOutput OS-swap #47372

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 5 commits into from
Jul 22, 2026
Merged

pkg/cli: add io.Writer params to renderLogs* formatters; kill captureOutput OS-swap #47372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f27dff4
Initial plan
Copilot Jul 22, 2026
fd6fbec
Add io.Writer parameters to renderLogs* formatters (ToWriter variants)
Copilot Jul 22, 2026
752ec14
pkg/cli: add renderLogsConsoleToWriter to complete 6-formatter refactor
Copilot Jul 22, 2026
15b35e6
pkg/cli: use multi-line body for renderLogsConsole wrapper (style con…
Copilot Jul 22, 2026
15c21a8
Merge branch 'main' into copilot/deep-report-add-io-writer-parameter
github-actions[bot] Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 4 additions & 15 deletions pkg/cli/logs_empty_runs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package cli
import (
"bytes"
"encoding/json"
"os"
"testing"

"github.com/github/gh-aw/pkg/testutil"
Expand All @@ -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
Expand Down
112 changes: 62 additions & 50 deletions pkg/cli/logs_format_compact.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cli

import (
"fmt"
"io"
"os"
"strconv"
"strings"
Expand Down Expand Up @@ -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:
//
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}

Expand All @@ -153,76 +155,81 @@ 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)
}
}
}

// [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
hint := "use --json for full details, -v for verbose, --format console for tables"
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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 "-"
Expand Down
Loading
Loading