Wrap org runner and MCP list-tools stderr output with console formatting helpers by Copilot · Pull Request #43589 · github/gh-aw

Copilot · 2026-07-05T16:02:09Z

This updates two CLI paths that were still writing user-facing stderr output directly, bypassing the console formatting layer. The change routes those messages through the existing stderr-aware console helpers so org summaries and MCP tool-list status output use the standard prefixes and TTY-safe formatting.

Org runner summary output
- Replaced raw stderr bullet rendering in pkg/cli/org_runner.go with console.FormatListItemStderr(...).
- Covers repository, workflow count, pending workflow updates, and compiler version summary lines.
MCP list-tools status output
- Replaced raw stderr info lines in pkg/cli/mcp_list_tools.go with console.FormatInfoMessageStderr(...).
- Covers workflow discovery messages, available-server hints, connection status, and tool-list headings.
Regression coverage
- Added focused CLI tests that assert the affected output paths render with the expected stderr console helpers.

Example of the change pattern:

fmt.Fprintln(os.Stderr, console.FormatListItemStderr("Repository: "+preview.Repo))
fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(
    fmt.Sprintf("Found MCP server '%s' in %d workflow(s): %s", mcpServerName, len(matchingWorkflows), strings.Join(matchingWorkflows, ", ")),
))

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-07-05T16:41:23Z

⚠️ PR Code Quality Reviewer failed during code quality review.

github-actions · 2026-07-05T16:41:25Z

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #43589 does not have the 'implementation' label and has 87 new lines of code in business logic directories (≤100 threshold).

github-actions · 2026-07-05T16:41:27Z

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

github-actions · 2026-07-05T16:41:32Z

✅ Test Quality Sentinel completed test quality analysis.

Copilot

Pull request overview

This PR standardizes a couple of CLI stderr output paths to go through the console formatting layer, so messages written to stderr get consistent prefixes and correct TTY-aware styling/stripping.

Changes:

Updated org runner summary lines to use console.FormatListItemStderr(...) instead of manual bullet formatting.
Updated MCP list-tools status messages to use console.FormatInfoMessageStderr(...) and added clearer blank-line separation for headings.
Added unit tests to pin the intended stderr formatting behavior for the affected output paths.

Show a summary per file

File	Description
pkg/cli/stderr_console_formatting_test.go	Adds focused unit tests asserting stderr output is routed through stderr-aware console formatters.
pkg/cli/org_runner.go	Routes org action summary “list item” lines through `FormatListItemStderr` instead of raw stderr bullets.
pkg/cli/mcp_list_tools.go	Routes MCP `list-tools` discovery/status output through `FormatInfoMessageStderr` and adjusts headings/spacing.

Review details

Tip

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

Files reviewed: 3/3 changed files
Comments generated: 3
Review effort level: Low

 func renderOrgActionSummary(preview orgRepoPreview, action string) {
 	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to "+action))
-	fmt.Fprintf(os.Stderr, "  - Repository: %s\n", preview.Repo)
+	fmt.Fprintln(os.Stderr, console.FormatListItemStderr("Repository: "+preview.Repo))


+	require.Len(t, lines, 5)
+	assert.Equal(t, console.FormatInfoMessage("Ready to update"), lines[0])
+	assert.Equal(t, console.FormatListItemStderr("Repository: octo/repo"), lines[1])


 	// Connect to the MCP server and get its tools
-	fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("📡 Connecting to MCP server: %s (%s)",
+	fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("📡 Connecting to MCP server: %s (%s)",
 		targetConfig.Name,
 		targetConfig.Type)))


github-actions · 2026-07-05T16:49:48Z

🧪 Test Quality Sentinel Report

⚠️ Test Quality Score: 60/100 — Acceptable

Analyzed 2 test(s): 2 design, 0 implementation, 0 violation(s).

📊 Metrics (2 tests)

Metric	Value
Analyzed	2 (Go: 2, JS: 0)
✅ Design	2 (100%)
⚠️ Implementation	0 (0%)
Edge/error coverage	0 (0%)
Duplicate clusters	0
Inflation	YES — 69 test lines added vs. 18 production lines (ratio ≈ 3.8:1)
🚨 Violations	0

Test	File	Classification	Issues
`TestRenderOrgActionSummaryUsesConsoleFormatListItemStderr`	`pkg/cli/stderr_console_formatting_test.go`	design_test / behavioral_contract	Happy-path only; assertions lack descriptive failure messages
`TestFindWorkflowsWithMCPServerUsesConsoleFormatInfoMessageStderr`	`pkg/cli/stderr_console_formatting_test.go`	design_test / behavioral_contract	Happy-path only; assertions lack descriptive failure messages

⚠️ Flagged Tests (2)

TestRenderOrgActionSummaryUsesConsoleFormatListItemStderr (stderr_console_formatting_test.go:18) — design_test, behavioral_contract.
Verifies that renderOrgActionSummary routes all list-item lines through console.FormatListItemStderr. High value — catches regressions where a format helper is bypassed. No error branch is exercised (e.g., empty Workflows slice with non-zero TotalWorkflows, or the version-upgrade path). All assert.Equal calls omit a descriptive failure message; prefer assert.Equal(t, expected, actual, "description of what is checked").

TestFindWorkflowsWithMCPServerUsesConsoleFormatInfoMessageStderr (stderr_console_formatting_test.go:42) — design_test, behavioral_contract.
Verifies findWorkflowsWithMCPServer routes output through console.FormatInfoMessageStderr. Setup is appropriately thorough (temp dir, real file I/O, os.Chdir). No negative path tested (e.g., directory with no matching workflows). Assertions also lack descriptive failure messages.

Test inflation note: 69 lines added in the test file vs. 18 lines added across both production files (ratio ≈ 3.8:1). The new test file covers all recently changed output calls in a single focused file — the inflation is acceptable given the breadth of coverage, but it still triggered the 2:1 threshold.

Verdict

✅ Passed. 0% implementation tests (threshold: 30%). No guideline violations. Score docked for missing edge-case coverage (0/2 tests have error/negative paths) and test inflation (3.8:1). Consider adding a test for the no-match path of findWorkflowsWithMCPServer and the version-upgrade branch of renderOrgActionSummary to reach the Excellent range.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

🧪 Test quality analysis by Test Quality Sentinel · 41 AIC · ⌖ 9.78 AIC · ⊞ 6.8K · ◷
_{Comment /review to run again}

github-actions

✅ Test Quality Sentinel: 60/100. 0% implementation tests (threshold: 30%). No violations. See comment for improvement suggestions (edge-case coverage and assertion failure messages).

github-actions · 2026-07-05T16:51:00Z

Skills-Based Review 🧠

Applied /codebase-design and /tdd — requesting changes on two related issues.

📋 Key Themes & Highlights

Key Themes

Missed formatter on org_runner.go:86: The "Ready to action" heading writes to os.Stderr but still calls FormatInfoMessage (stdout TTY check). When stdout is piped and stderr is a terminal the heading will silently lose ANSI styling while the surrounding bullet lines render correctly — the exact split-TTY inconsistency this PR set out to fix.
Test encodes the wrong expectation: TestRenderOrgActionSummaryUsesConsoleFormatListItemStderr asserts console.FormatInfoMessage(...) for lines[0]. In non-TTY environments (CI) both formatters return identical plain strings, so the test passes even when the wrong helper is called. The test will need to be updated alongside the production fix.

Positive Highlights

✅ Systematic sweep of mcp_list_tools.go — every former raw fmt.Fprintf(os.Stderr) call is now wrapped correctly.
✅ Clean blank-line separators as standalone fmt.Fprintln(os.Stderr, "") calls — avoids embedding newlines inside formatter calls.
✅ New test file targets the right layer (CLI functions → stderr capture) rather than re-testing console helpers.
✅ Descriptive test names that read as specifications.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 55.2 AIC · ⌖ 6.85 AIC · ⊞ 6.7K · ◷
_{Comment /matt to run again}

github-actions

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 55.2 AIC · ⌖ 6.85 AIC · ⊞ 6.7K
_{Comment /matt to run again}

github-actions · 2026-07-05T16:51:01Z

@@ -84,23 +84,23 @@ type orgRunCallbacks struct {

 func renderOrgActionSummary(preview orgRepoPreview, action string) {
 	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to "+action))


[/codebase-design] This line writes to os.Stderr but calls FormatInfoMessage (stdout TTY check), which is exactly the inconsistency this PR aims to fix — it should use FormatInfoMessageStderr.

💡 Suggested fix

fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Ready to "+action))

FormatInfoMessage calls isTTY (stdout), while FormatInfoMessageStderr calls isStderrTTY. When stdout is redirected but stderr is a terminal (e.g. piped commands), this line will lose its ANSI styling while the surrounding bullet lines render correctly — exactly the TTY-safe inconsistency the PR description says it resolves.

@copilot please address this.

github-actions · 2026-07-05T16:51:01Z

+
+	lines := strings.Split(strings.TrimSuffix(stderr, "\n"), "\n")
+	require.Len(t, lines, 5)
+	assert.Equal(t, console.FormatInfoMessage("Ready to update"), lines[0])


[/tdd] The test asserts console.FormatInfoMessage for lines[0], but that's the stdout-TTY variant. If the production fix on line 86 of org_runner.go is made (using FormatInfoMessageStderr), this assertion will need to change too — and the test would currently pass for the wrong reason.

💡 Suggested fix

assert.Equal(t, console.FormatInfoMessageStderr("Ready to update"), lines[0])

Tests that encode the wrong helper as the expected value become false-positive greenlight for the bug they were meant to catch. The test name says "UsesConsoleFormatListItemStderr" but the heading assertion silently accepts a non-stderr formatter.

@copilot please address this.

github-actions

Review: Wrap org runner and MCP list-tools stderr output with console formatting helpers

Good direction — routing raw fmt.Fprintf calls through the console formatting layer is the right approach. Three issues need attention before this lands:

Blocking:

Flat hierarchy for nested workflow items (line 94, org_runner.go): Sub-items under "Pending workflow updates:" now render at the same • indent level as the parent, erasing the previous 4-space indent that distinguished them. The formatted output regresses in readability for workflows with pending updates.
FormatInfoMessage (stdout TTY) on a stderr write (line 86, org_runner.go): The PR migrated all surrounding lines to Stderr-variants but left the "Ready to (action)" header using the stdout-TTY FormatInfoMessage. In a non-TTY stdout / TTY stderr context, the prefix formatting will be wrong for that one line.

Non-blocking:

Test asserts the wrong variant (line 34, test file): The new test anchors the header on FormatInfoMessage, which cements the inconsistency rather than verifying the intended stderr-safe path.

See inline comments for specific locations.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 60.9 AIC · ⌖ 5.25 AIC · ⊞ 4.9K

github-actions · 2026-07-05T16:53:04Z

+		fmt.Fprintln(os.Stderr, console.FormatListItemStderr("Pending workflow updates:"))
 		for _, wf := range preview.Workflows {
-			fmt.Fprintf(os.Stderr, "    - %s: %s -> %s\n", wf.Name, wf.CurrentRef, wf.LatestRef)
+			fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("%s: %s -> %s", wf.Name, wf.CurrentRef, wf.LatestRef)))


The sub-items for each workflow entry (e.g. repo-assist: v1.0.0 -> v1.1.0) now use the same FormatListItemStderr level as the parent "Pending workflow updates:" heading. Previously these were rendered with 4-space indent ( - name: old -> new), providing clear visual nesting under their parent.

After this change the output is flat:

• Pending workflow updates: • repo-assist: v1.0.0 -> v1.1.0

Consider adding a FormatSubListItemStderr helper (or similar) to preserve the two-level hierarchy, or explicitly document that this flattening is an intentional design simplification.

@copilot please address this.

github-actions · 2026-07-05T16:53:04Z

@@ -84,23 +84,23 @@ type orgRunCallbacks struct {

 func renderOrgActionSummary(preview orgRepoPreview, action string) {
 	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to "+action))


FormatInfoMessage uses the stdout TTY check (isTTY) while the output is going to os.Stderr. All surrounding lines (87–103) correctly use the Stderr variants. The "Ready to (action)" header line should use FormatInfoMessageStderr for consistency:

fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Ready to "+action))

This is a pre-existing inconsistency that still remains on this line — the PR fixed the sibling calls but missed this one.

@copilot please address this.

github-actions · 2026-07-05T16:53:04Z

+
+	lines := strings.Split(strings.TrimSuffix(stderr, "\n"), "\n")
+	require.Len(t, lines, 5)
+	assert.Equal(t, console.FormatInfoMessage("Ready to update"), lines[0])


Line 34 asserts the header uses FormatInfoMessage (stdout-TTY variant), which matches the current code but perpetuates the inconsistency. If the intent is to migrate all renderOrgActionSummary output to the stderr-aware helpers, this assertion should verify FormatInfoMessageStderr instead. As-is the test encodes the inconsistency rather than driving the correct behaviour.

@copilot please address this.

github-actions

REQUEST_CHANGES — two correctness issues, one maintainability issue

The console-layer migration is the right approach, but there are two bugs that must be fixed before merging.

Blocking issues (2 correctness)

1. org_runner.go:86 — FormatInfoMessage on a stderr write
The header line Ready to <action> still uses FormatInfoMessage, which checks isTTY() (stdout TTY), while every list item beneath it uses FormatListItemStderr (stderr TTY). In any piped invocation where stdout is redirected but stderr is not, the header will render without color/prefix while its children are styled—a visually broken block. Fix: console.FormatInfoMessageStderr("Ready to "+action).

2. stderr_console_formatting_test.go:34 — test asserts the wrong variant
The test asserting line 0 uses console.FormatInfoMessage, locking in the bug instead of catching it. Fix alongside the production code change.

Non-blocking (1 maintainability)

mcp_list_tools.go:136, 151 — raw fmt.Fprintln(os.Stderr, "") for blank lines
Two blank-line separators bypass the console layer entirely. Not a runtime bug, but breaks the abstraction the PR is establishing. Consider a console.PrintBlankLineStderr() helper or at least a comment noting the intentional bypass.

🔎 Code quality review by PR Code Quality Reviewer · 221.1 AIC · ⌖ 9.22 AIC · ⊞ 5.4K
_{Comment /review to run again}

github-actions · 2026-07-05T16:59:21Z

@@ -84,23 +84,23 @@ type orgRunCallbacks struct {

 func renderOrgActionSummary(preview orgRepoPreview, action string) {
 	fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to "+action))


Wrong TTY check on the Ready to header line: FormatInfoMessage checks stdout TTY while the line is written to stderr; all child list items correctly use FormatListItemStderr (stderr TTY), so this header will lose color/styling whenever stdout is not a TTY but stderr is—producing an inconsistently formatted block.

💡 Suggested fix

Change line 86 in org_runner.go to use the stderr-aware variant:

fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Ready to "+action))

The surrounding list items already use FormatListItemStderr, so this header must match. FormatInfoMessage routes the TTY check through isTTY() (stdout), which can disagree with isStderrTTY() (stderr) in piped/redirected invocations.

github-actions · 2026-07-05T16:59:22Z

+
+	lines := strings.Split(strings.TrimSuffix(stderr, "\n"), "\n")
+	require.Len(t, lines, 5)
+	assert.Equal(t, console.FormatInfoMessage("Ready to update"), lines[0])


Test asserts the wrong format function, hardcoding a bug: Line 34 asserts console.FormatInfoMessage("Ready to update") (stdout-TTY-aware), but the production code at org_runner.go:86 actually calls console.FormatInfoMessage too—both agree, but both are wrong (should be FormatInfoMessageStderr). This test cements the inconsistency rather than catching it; it will pass even after the TTY-check bug is introduced or persists.

💡 Suggested fix

Fix the assertion to expect the stderr-aware variant (and fix the production code on org_runner.go:86 to match):

assert.Equal(t, console.FormatInfoMessageStderr("Ready to update"), lines[0])

Once org_runner.go:86 is also updated to FormatInfoMessageStderr, this test will correctly gate against future regressions where someone re-introduces the stdout-TTY variant.

github-actions · 2026-07-05T16:59:22Z

+	fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(
+		fmt.Sprintf("Found MCP server '%s' in %d workflow(s): %s", mcpServerName, len(matchingWorkflows), strings.Join(matchingWorkflows, ", ")),
+	))
+	fmt.Fprintln(os.Stderr, "")


Raw blank-line emission bypasses the console abstraction: fmt.Fprintln(os.Stderr, "") is a naked call that bypasses the console formatting layer the rest of this PR is trying to enforce. It mixes raw stderr writes with console-wrapped writes in the same output stream.

💡 Suggested fix

While this is a cosmetic emission, the inconsistency matters for consumers (e.g. log capturers, test helpers) that strip or transform console-prefixed lines. If the console package has a blank-line helper, use it; otherwise at minimum document the intentional bypass. For example:

// Blank separator: intentionally raw (no console prefix) _, _ = fmt.Fprintln(os.Stderr)

Or, if the same pattern is needed in multiple places, add a console.PrintBlankLineStderr() helper to keep all stderr I/O routed through one layer. The same pattern appears at line 151 in displayToolsList.

Initial plan

df7f0e6

Copilot AI assigned Copilot and pelikhan Jul 5, 2026

Copilot started work on behalf of pelikhan July 5, 2026 16:18 View session

Copilot AI linked an issue Jul 5, 2026 that may be closed by this pull request

[plan] Wrap org_runner.go and mcp_list_tools.go raw stderr output with console helpers #43576

Open

5 tasks

Copilot AI and others added 3 commits July 5, 2026 16:31

fix: wrap cli stderr output with console helpers

aed3f80

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

chore: address stderr formatting review note

9b078fe

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

test: clarify stderr formatting regression names

0669171

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Wrap org_runner.go and mcp_list_tools.go stderr output with console helpers~~ Wrap org runner and MCP list-tools stderr output with console formatting helpers Jul 5, 2026

Copilot AI requested a review from pelikhan July 5, 2026 16:37

Copilot finished work on behalf of pelikhan July 5, 2026 16:37

pelikhan marked this pull request as ready for review July 5, 2026 16:40

Copilot AI review requested due to automatic review settings July 5, 2026 16:40

Copilot started reviewing on behalf of pelikhan July 5, 2026 16:40 View session

Copilot AI reviewed Jul 5, 2026

View reviewed changes

github-actions Bot mentioned this pull request Jul 5, 2026

[aw] No-Op Runs #43499

Open

github-actions Bot approved these changes Jul 5, 2026

View reviewed changes

github-actions Bot reviewed Jul 5, 2026

View reviewed changes

github-actions Bot requested changes Jul 5, 2026

View reviewed changes

		@@ -84,23 +84,23 @@ type orgRunCallbacks struct {

		func renderOrgActionSummary(preview orgRepoPreview, action string) {
		fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Ready to "+action))

Uh oh!

Conversation

Copilot AI commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot commented Jul 5, 2026

🧪 Test Quality Sentinel Report

Verdict

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 5, 2026

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Review: Wrap org runner and MCP list-tools stderr output with console formatting helpers

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

REQUEST_CHANGES — two correctness issues, one maintainability issue

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented Jul 5, 2026 •

edited

Loading

github-actions Bot commented Jul 5, 2026 •

edited

Loading

github-actions Bot commented Jul 5, 2026 •

edited

Loading

github-actions Bot commented Jul 5, 2026 •

edited

Loading

github-actions Bot commented Jul 5, 2026 •

edited

Loading