Fix console formatter stream contracts and guard stderr usage by pelikhan · Pull Request #67423 · github/gh-aw

pelikhan · 2026-10-10T12:04:13Z

Summary

Fixes #67143 and addresses source #66976 by correcting console behavior without renaming diagnostic callers.

Existing FormatError, message formatters, RenderTable and RenderStruct default to stderr terminal/color detection. Existing *Stderr APIs remain compatible aliases; signatures and actual write destinations are unchanged.
Remove the original approximately 1,600 diagnostic selector renames. The net diff is 71 files, down from 241, including tests, documentation, analyzer integration and duplicate-prefix cleanup.
Intentional stdout data uses explicit *Stdout variants. JSON, version, completion, compact logs and WASM tab-separated output retain their contracts. RenderStructWithOptions keeps its explicit destination: Stderr: true selects stderr; false or omitted selects stdout.
The type-aware consolestderr analyzer guards direct stdout/stderr writes and explicit render options. It handles imported aliases and nested expressions; intermediate writer/formatter callbacks remain outside its dataflow scope.
Only human experiment tables opt into 80-column, Unicode-safe lossless wrapping, with labeled-row fallback. There is no universal truncation policy.

ADR: ADR-67423: Destination-aware console formatters and stderr analyzer — Proposed; maintainer acceptance remains required.

Review follow-up

Commit fa2c4300690fec4ac1e7c4adbbf5cd457c53c037 fixes all three proven indirect stream mismatches from the review:

Scanner findings use the diagnostic FormatError default for production stderr consumers.
All six mapping/resolution formatter callbacks passed to stderr-only PinContext.emitOnce use diagnostic defaults.
Bootstrap TODO headers and handoff messages use diagnostic defaults, matching both production stderr callers.

No intentional stdout callers were rerouted, and the analyzer was not broadened. TestIndirectDiagnosticStreamContracts exercises these higher-level paths with real PTYs across all eight stdout/stderr/NO_COLOR combinations. It verifies each affected callback, scanner rendering, both bootstrap messages, pin results and warning deduplication.

Validation

make build, make fmt, and make lint pass with modern Bash and cached manifest-declared Prettier 3.9.9.
Complete actionpins and scanfindings suites, console/analyzer suites, targeted bootstrap/indirect-PTY tests, compact/JSON regressions and experiment stream-contract tests pass. Native CLI and WASM builds pass.
The production blocking analyzer set plus consolestderr passes. Existing formatter tests cover the direct PTY/NO_COLOR matrix, nested/options tables, Unicode width/data preservation and version output; the new higher-level matrix closes the reviewed writer/callback gaps.
Final make agent-report-progress ran: impacted tests, standard lint and schema freshness pass. Its all-custom-analyzer stage still reports previously disclosed advisory debt and guarded-index false positives; no new diagnostics were reported for these fixes. No unrelated fixes or suppressions were added.
A previously broader test selection encountered the unrelated installer fixture's x64-only toolcache on this ARM64 host (exit 97); that fixture was not changed.
No Actions runs or merge were performed. Maintainer review, ADR acceptance and current-HEAD CI verification remain before merge.

Migrate proven direct stderr formatter calls with the type-aware analyzer, bound experiment human tables without truncating data, and preserve JSON, version, compact and WASM output contracts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-10-10T12:06:48Z

✅ Ponytail Reviewer completed successfully!

Lean already. Ship.

Generated by Ponytail Reviewer for #67423

github-actions · 2026-10-10T12:06:51Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-10-10T12:06:54Z

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-10-10T12:06:55Z

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-10-10T12:07:19Z

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-10-10T12:12:48Z

🏗️ Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (2,253 new lines across 241 files in pkg/) but does not have a linked Architecture Decision Record (ADR).

📄 Draft ADR committed: docs/adr/67423-destination-aware-console-formatters-and-stderr-analyzer.md — review and complete it before merging.

🔒 This PR cannot merge until an ADR is linked in the PR body.

📋 What to do next

Review the draft ADR committed to your branch — it was generated from the PR diff and description
Complete the missing sections — add context the AI couldn't infer, refine the decision rationale, and list real alternatives you considered
Commit the finalized ADR to docs/adr/ on your branch
Reference the ADR in this PR body by adding a line such as:

ADR: ADR-67423: Enforce Destination-Aware Console Formatters with a Dedicated Analyzer

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

🔍 Evidence used

has_implementation_label: false; business-logic additions: 2253 (threshold 100) → enforcement triggered by code volume
No .design-gate.yml present — default business directories and threshold used
No ADR in PR body, no docs/adr/67423-* on the branch, and linked issue [AW Top 10] 10 Fix console formatter stdout and stderr misuse #67143 contains no ADR sections
Decision inferred from: pkg/linters/consolestderr/consolestderr.go (new analyzer + stdout→stderr replacement table), pkg/console/table_width.go (opt-in MaxWidth budget, narrow-table fallback), pkg/console/render.go, and the PR description

❓ Why ADRs Matter

ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you.

📋 Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

Context — What is the problem? What forces are at play?
Decision — What did you decide? Why?
Alternatives Considered — What else could have been done?
Consequences — What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · opus50 · 41.9 AIC · ⌖ 50.7 AIC · ⊞ 1.8K · ◷
_{Comment /review to run again}

Copilot

🟡 Changes recommended

Three migrated warning paths still emit duplicated semantic warning prefixes.

3 open findings

What changed in this PR

This PR makes console formatting destination-aware, adds bounded human-facing tables, and enforces stderr contracts through a custom analyzer.

Changes:

Migrates CLI, parser, and compiler diagnostics to stderr-aware formatters.
Adds bounded, Unicode-aware table rendering for experiment output.
Adds and CI-enforces the consolestderr analyzer with suggested fixes.

Three warning paths still produce redundant ⚠ Warning: prefixes.

File	Description
`.github/workflows/cgo.yml`	Enforces the console-stream analyzer.
`cmd/gh-aw/main.go`	Migrates help diagnostics.
`pkg/cli/*.go` (174 files)	Migrates CLI diagnostics, tables, and reflected output to stderr-aware APIs.
`pkg/cli/console_stream_contract_test.go`	Tests stream and width contracts.
`pkg/cli/experiments_fetch_test.go`	Extends the experiment API fixture.
`pkg/console/README.md`	Documents stream and width APIs.
`pkg/console/console.go`	Adds stderr formatters and bounded tables.
`pkg/console/console_types.go`	Adds width and destination options.
`pkg/console/console_wasm.go`	Preserves WASM-compatible APIs and tables.
`pkg/console/render.go`	Propagates rendering options through nested tables.
`pkg/console/stream_contract_test.go`	Tests mixed stdout/stderr terminal behavior.
`pkg/console/table_width.go`	Implements width-aware table fallback.
`pkg/console/table_width_test.go`	Tests Unicode width and losslessness.
`pkg/envutil/envutil.go`	Migrates environment warnings.
`pkg/linters/README.md`	Documents the analyzer.
`pkg/linters/consolestderr/consolestderr.go`	Implements stderr misuse detection.
`pkg/linters/consolestderr/consolestderr_test.go`	Runs analyzer suggested-fix tests.
`pkg/linters/consolestderr/testdata/src/consolestderr/consolestderr.go`	Provides analyzer findings fixture.
`pkg/linters/consolestderr/testdata/src/consolestderr/consolestderr.go.golden`	Verifies suggested fixes.
`pkg/linters/consolestderr/testdata/src/github.com/github/gh-aw/pkg/console/console.go`	Stubs console APIs for analyzer tests.
`pkg/linters/doc.go`	Registers analyzer documentation.
`pkg/linters/doc_sync_test.go`	Updates CI-enforcement expectations.
`pkg/linters/registry.go`	Registers the analyzer.
`pkg/linters/spec_test.go`	Adds analyzer documentation coverage.
`pkg/parser/include_processor.go`	Migrates include diagnostics.
`pkg/parser/workflow_update.go`	Migrates update diagnostics.
`pkg/workflow/*.go` (41 files)	Migrates compiler and validation diagnostics to stderr-aware APIs.

🧠 Review effort: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-10-10T12:16:24Z

Comment Memory

Peek at saved memory (pr-code-quality-reviewer)

reviewed_at: 2026-10-10T12:08:34Z
review_event: REQUEST_CHANGES
top_themes:
  - consolestderr analyzer gap for RenderStructWithOptions
files_reviewed:
  - .github/workflows/cgo.yml
  - pkg/cli/experiments_command.go
  - pkg/cli/experiments_render.go
  - pkg/console/console.go
  - pkg/console/render.go
  - pkg/console/table_width.go
  - pkg/linters/consolestderr/consolestderr.go
comment_count: 1

Note

This comment is managed by comment memory.

Expand the saved memory block to view or edit the persistent context for this thread.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 158 AIC · ⌖ 5.5 AIC · ⊞ 19.9K · ◷
_{Comment /review to run again}

github-actions

Verdict

The stream-format migration is close, but the new enforcement linter still has a bypass: RenderStructWithOptions can send stdout-styled reflected output to stderr without any diagnostic.

Blocking theme

consolestderr covers RenderStruct and RenderTable, but it does not understand the newly added RenderStructWithOptions path, so the guardrail this PR is introducing is still incomplete.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 158 AIC · ⌖ 5.5 AIC · ⊞ 19.9K
_{Comment /review to run again}

github-actions

Skills-Based Review 🧠

Applied /codebase-design on the pkg/console API surface and /tdd on the new regression coverage. The core change (stderr-aware formatter API, width-budget rendering, consolestderr analyzer) is well-designed and thoroughly tested; only two minor cleanup items found, both left as inline suggestions.

📋 Key Themes & Highlights

Key Themes

Two small dead/redundant-code nits in pkg/console/render.go and experiments_render.go (see inline comments) — neither is a correctness issue.
Note on scope/trust: the pre-fetched diff was truncated at 3000 lines (53 of 241 changed files). I supplemented it with targeted API fetches of pkg/console/*, pkg/linters/consolestderr/*, pkg/linters/registry.go, and spot-checked ~10 of the ~230 mechanical migration files in pkg/cli for pattern consistency — all sampled migrations correctly preserved streams/args/newlines.
pr-triage agent invocation did not return structured output within the available context; I applied the fallback heuristic (classify directly from pr-meta.json/diff, high-impact files = non-generated files with largest delta) as instructed.

Positive Highlights

✅ consolestderr analyzer is type-aware (resolves imports via go/types, handles aliased imports, parenthesized expressions, suppressions via (nolint/redacted):consolestderr), with a solid golden-fixture test including a deliberate "shadowed os" false-positive guard.
✅ stream_contract_test.go's PTY matrix (stdout/stderr TTY × NO_COLOR, 8 combinations) is a strong, direct regression test for the exact stream-contract bug class this PR fixes — a good /tdd example of testing the actual observable contract rather than internals.
✅ table_width_test.go explicitly asserts no data loss (counts of x/界/q characters preserved) and keeps combining marks (e\u0301) intact when wrapping — this guards the opt-in width feature against the classic "silently truncates Unicode" regression.
✅ Width budgeting is correctly scoped as opt-in (MaxWidth zero-value keeps unbounded output); JSON/version/completion/compact-log call sites were left untouched, matching the PR's stated non-goal of a universal width change.
✅ Documentation (pkg/console/README.md), linter registry/README, and doc_sync_test.go/spec_test.go were all updated in lockstep with the new analyzer and APIs.

Acknowledged (non-blocking) scope limitation

The PR description is upfront that the analyzer doesn't trace intermediate writer/formatter aliases (dataflow). That's a reasonable, explicitly documented boundary for a mechanical migration of this size and not something to block on.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 183 AIC · ⌖ 16.1 AIC · ⊞ 10.3K
_{Comment /matt to run again}

Guard explicit render destinations, remove duplicate warning prefixes and redundant rendering wrappers, and finalize the proposed ADR. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

done

github-actions · 2026-10-10T13:02:42Z

Thanks for the console formatter work! 🙌 This PR is large (~4,200 lines, 246 files) and mixes several concerns (stderr migration, analyzer, width budgets, error-chain styling, table renderers). Please consider splitting it into smaller focused PRs, and note the referenced ADR still needs maintainer acceptance.

Split this PR into focused PRs: (1) stderr stream contract migration, (2) stderr guard analyzer, (3) width budgets and error-chain styling.

Generated by ✅ Contribution Check · copilot · auto · 48.1 AIC · ⌖ 0.613 AIC · ⊞ 9.2K · ◷

Undo the bulk diagnostic selector migration, preserve existing stderr aliases, and opt intentional stdout callers into explicit stdout styling. Check both output destinations without changing actual writes or machine-output contracts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Correct scanner rendering, pin formatter callbacks, and bootstrap TODO output. Add higher-level mixed-PTY and NO_COLOR coverage for all affected paths without changing intentional stdout output or analyzer scope. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

pelikhan marked this pull request as ready for review October 10, 2026 12:05

Copilot AI balanced review requested due to automatic review settings October 10, 2026 12:05

Copilot started reviewing on behalf of pelikhan October 10, 2026 12:06 View session

docs(adr): add draft ADR for console stream contracts

3ebf06c

Copilot AI reviewed Oct 10, 2026

View reviewed changes

Comment thread pkg/workflow/expression_secrets_serialization_validation.go Outdated

Comment thread pkg/workflow/step_shell_validator.go Outdated

Comment thread pkg/workflow/strict_mode_env_validation.go Outdated

github-actions Bot previously requested changes Oct 10, 2026

View reviewed changes

Comment thread pkg/linters/consolestderr/consolestderr.go Outdated

github-actions Bot reviewed Oct 10, 2026

View reviewed changes

Comment thread pkg/console/render.go Outdated

Comment thread pkg/console/console.go

fix: address console stream review feedback

97378b4

Guard explicit render destinations, remove duplicate warning prefixes and redundant rendering wrappers, and finalize the proposed ADR. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions Bot mentioned this pull request Oct 10, 2026

[Contribution Check Report] Contribution Check — 2026-10-10 #67317

Closed

pelikhan and others added 2 commits October 10, 2026 06:05

pelikhan merged commit 77bb415 into main Oct 10, 2026
36 checks passed

pelikhan deleted the pelikhan-console-output-streams branch October 10, 2026 13:27

pelikhan mentioned this pull request Oct 10, 2026

[deep-report] Add a lint rule to catch Format*Message (non-Stderr) output written to os.Stderr in pkg/cli #66976

Closed

github-actions Bot mentioned this pull request Oct 11, 2026

consolestderr (78th linter) CI-enforced on native only - omitted from the wasm custom-linters run (ci_target_asymmetry recurrenc #67609

Closed

Conversation

pelikhan commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Review follow-up

Validation

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026

🏗️ Design Decision Gate — ADR Required

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026

Comment Memory

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Verdict

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Acknowledged (non-blocking) scope limitation

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pelikhan commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading

github-actions Bot commented Oct 10, 2026 •

edited

Loading