Skip to content

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

Open
pelikhan with Copilot wants to merge 5 commits into
mainfrom
copilot/deep-report-add-io-writer-parameter
Open

pkg/cli: add io.Writer params to renderLogs* formatters; kill captureOutput OS-swap#47372
pelikhan with Copilot wants to merge 5 commits into
mainfrom
copilot/deep-report-add-io-writer-parameter

Conversation

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

The six renderLogs* formatters in pkg/cli/ wrote directly to os.Stdout, forcing tests to use a captureOutput helper that swaps os.Stdout at the OS level behind a mutex and cannot be used with t.Parallel().

Changes

New ...ToWriter variants (matching existing renderCrossRunReportMarkdownToWriter convention)

  • renderLogsTSVToWriter(w io.Writer, data LogsData) / renderLogsTSVVerboseToWriter
  • renderLogsCompactToWriter(w io.Writer, data LogsData) / renderLogsCompactVerboseToWriter
  • renderLogsJSONToWriter(w io.Writer, data LogsData, verbose bool) error

Original functions kept as thin os.Stdout wrappers — no call-site changes needed.

renderLogsArtifactHint widened from *os.File to io.Writer.

Tests — five test files converted from captureOutput/os.Stdout pipe-swap to direct bytes.Buffer writes; t.Parallel() added:

  • logs_format_tsv_test.go
  • logs_output_hint_test.go
  • logs_json_test.go
  • logs_json_clean_test.go (TestJSONOutputNotCorruptedByStderr)
  • logs_empty_runs_test.go
// Before
func renderLogsTSV(data LogsData) {
    fmt.Fprintf(os.Stdout, "# %d runs ...", ...)
}

// After
func renderLogsTSVToWriter(w io.Writer, data LogsData) {
    fmt.Fprintf(w, "# %d runs ...", ...)
}
func renderLogsTSV(data LogsData) { renderLogsTSVToWriter(os.Stdout, data) }

Generated by 👨‍🍳 PR Sous Chef · gpt54 7.24 AIC · ⌖ 6.95 AIC · ⊞ 7K ·
Comment /souschef to run again


Run: https://github.com/github/gh-aw/actions/runs/29952896403

Generated by 👨‍🍳 PR Sous Chef · gpt54 12.1 AIC · ⌖ 6.17 AIC · ⊞ 7K ·
Comment /souschef to run again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Add io.Writer parameter to renderLogs formatters in pkg/cli pkg/cli: add io.Writer params to renderLogs* formatters; kill captureOutput OS-swap Jul 22, 2026
Copilot AI requested a review from pelikhan July 22, 2026 18:03
@pelikhan
pelikhan marked this pull request as ready for review July 22, 2026 18:31
Copilot AI review requested due to automatic review settings July 22, 2026 18:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds injectable writers to five log formatters, reducing process-global stdout capture in tests.

Changes:

  • Adds ...ToWriter variants with stdout-compatible wrappers.
  • Generalizes artifact hints to io.Writer.
  • Converts formatter tests to buffers and enables parallel execution.
Show a summary per file
File Description
pkg/cli/logs_report.go Adds JSON writer rendering.
pkg/cli/logs_orchestrator_render.go Generalizes hint output.
pkg/cli/logs_format_tsv.go Adds TSV writer variants.
pkg/cli/logs_format_compact.go Adds compact writer variants.
pkg/cli/logs_output_hint_test.go Uses buffered compact output.
pkg/cli/logs_json_test.go Captures JSON in a buffer.
pkg/cli/logs_json_clean_test.go Tests buffered JSON output.
pkg/cli/logs_format_tsv_test.go Uses parallel buffered tests.
pkg/cli/logs_empty_runs_test.go Uses parallel buffered JSON testing.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread pkg/cli/logs_report.go
// 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 {
@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

Field Value
Category refactor
Risk Low
Score 58/100
Action auto_merge

Score breakdown: Impact 22/50 · Urgency 18/30 · Quality 18/20

Rationale: Adds injectable io.Writer to log formatters, eliminating process-global stdout capture in tests. Pure internal refactor — no user-facing behavior changes. 9 files, all in pkg/cli. CI nearly all green (lint and Windows Integration still in progress but build/test pass). Good description, tests updated. Candidate for auto-merge once CI completes.

Labels applied: pr-type:refactor · pr-risk:low · pr-priority:medium · pr-action:auto_merge · pr-agent:copilot

Generated by 🔧 PR Triage Agent · sonnet46 38.6 AIC · ⌖ 7.92 AIC · ⊞ 5.7K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill. This PR looks close: all visible checks are complete and green, and I do not see outstanding review-thread cleanup from the compact context. Please confirm the branch is up to date, handle any remaining reviewer feedback if present, and proceed toward merge readiness.

Generated by 👨‍🍳 PR Sous Chef · gpt54 6.7 AIC · ⌖ 7.85 AIC · ⊞ 5.5K ·
Comment /souschef to run again

Copilot AI and others added 2 commits July 22, 2026 19:08
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…sistency)

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill. This PR looks close: all visible checks are complete and green, and I do not see outstanding review-thread cleanup from the compact context. Please confirm...

Done. Added renderLogsConsoleToWriter(w io.Writer, data LogsData) in logs_report.go and kept renderLogsConsole as a thin os.Stdout wrapper — matching the pattern used by the other five formatters. Updated logs_report_test.go to use bytes.Buffer and t.Parallel(). Local build and tests pass.

Copilot AI requested a review from gh-aw-bot July 22, 2026 19:10
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · gpt54 7.24 AIC · ⌖ 6.95 AIC · ⊞ 7K ·
Comment /souschef to run again

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address unresolved review feedback, and rerun checks once the branch is up to date.

...

renderLogsConsoleToWriter was added in commit 752ec14 and the wrapper was updated in 15b35e6. The branch is up to date with main (merge commit 15c21a8). Local make fmt, lint, and targeted tests all pass. The one remaining review thread covers the now-addressed renderLogsConsole gap — the code change is in place; a maintainer needs to resolve the thread and re-trigger CI to confirm green checks before merge.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · gpt54 12.1 AIC · ⌖ 6.17 AIC · ⊞ 7K ·
Comment /souschef to run again

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once the branch is up to date.

...

All code changes are complete and local validation passes. The one remaining item is resolving the copilot-pull-request-reviewer thread at logs_report.go:527renderLogsConsoleToWriter was added in commit 752ec14 addressing exactly that feedback, but the GraphQL mutation to resolve threads is unavailable from the agent environment. A maintainer needs to manually resolve that thread in the GitHub UI. CI is currently running on HEAD (15c21a8) and the prior run (29951255573) completed with success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[deep-report] Add io.Writer parameter to renderLogs* formatters in pkg/cli/ (kill captureOutput OS-swap)

4 participants