Skip to content

Wrap org runner and MCP list-tools stderr output with console formatting helpers #43589

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 6 commits into from
Jul 5, 2026
+89 −16
Merged

Wrap org runner and MCP list-tools stderr output with console formatting helpers #43589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
df7f0e6
Initial plan
Copilot Jul 5, 2026
aed3f80
fix: wrap cli stderr output with console helpers
Copilot Jul 5, 2026
9b078fe
chore: address stderr formatting review note
Copilot Jul 5, 2026
0669171
test: clarify stderr formatting regression names
Copilot Jul 5, 2026
bd34232
Merge branch 'main' into copilot/wrap-org-runner-and-mcp-list-tools-s…
github-actions[bot] Jul 5, 2026
ab29367
Fix stderr console formatting: use *Stderr variants throughout
Copilot Jul 5, 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
22 changes: 13 additions & 9 deletions pkg/cli/mcp_list_tools.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func ListToolsForMCP(workflowFile string, mcpServerName string, verbose bool) er
}

if verbose {
fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Looking for MCP server '%s' in: %s", mcpServerName, workflowPath)))
fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("Looking for MCP server '%s' in: %s", mcpServerName, workflowPath)))
}

// Parse the workflow file and extract MCP configurations
Expand All @@ -74,17 +74,16 @@ func ListToolsForMCP(workflowFile string, mcpServerName string, verbose bool) er

// Show available servers
if len(mcpConfigs) > 0 {
fmt.Fprintf(os.Stderr, "Available MCP servers: ")
serverNames := sliceutil.Map(mcpConfigs, func(config parser.RegistryMCPServerConfig) string { return config.Name })
fmt.Fprintf(os.Stderr, "%s\n", strings.Join(serverNames, ", "))
fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr("Available MCP servers: "+strings.Join(serverNames, ", ")))
}
return nil
}

mcpListToolsLog.Printf("Found MCP server: name=%s, type=%s", targetConfig.Name, targetConfig.Type)

// 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)))
Comment on lines 85 to 88

Expand All @@ -96,7 +95,7 @@ func ListToolsForMCP(workflowFile string, mcpServerName string, verbose bool) er
mcpListToolsLog.Printf("Connected to MCP server: tools=%d", len(info.Tools))

if verbose {
fmt.Fprintln(os.Stderr, console.FormatSuccessMessage("Successfully connected to MCP server"))
fmt.Fprintln(os.Stderr, console.FormatSuccessMessageStderr("Successfully connected to MCP server"))
}

// Display the tools
Expand Down Expand Up @@ -131,9 +130,13 @@ func findWorkflowsWithMCPServer(workflowsDir string, mcpServerName string, verbo
}

// Display matching workflows and suggest using one
fmt.Fprintf(os.Stderr, "Found MCP server '%s' in %d workflow(s): %s\n",
mcpServerName, len(matchingWorkflows), strings.Join(matchingWorkflows, ", "))
fmt.Fprintf(os.Stderr, "\nRun 'gh aw mcp list-tools <workflow-name> --server %s' to list tools for a specific workflow\n", mcpServerName)
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, "")

@github-actions github-actions Bot Jul 5, 2026

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.

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.

fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(
fmt.Sprintf("Run 'gh aw mcp list-tools <workflow-name> --server %s' to list tools for a specific workflow", mcpServerName),
))

return nil
}
Expand All @@ -145,7 +148,8 @@ func displayToolsList(info *parser.MCPServerInfo, verbose bool) {
return
}

fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("\n🛠️ Available Tools (%d total)", len(info.Tools))))
fmt.Fprintln(os.Stderr, "")
fmt.Fprintln(os.Stderr, console.FormatInfoMessageStderr(fmt.Sprintf("🛠️ Available Tools (%d total)", len(info.Tools))))

// Configure options based on verbose flag
opts := MCPToolTableOptions{
Expand Down
14 changes: 7 additions & 7 deletions pkg/cli/org_runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,24 @@ type orgRunCallbacks struct {
}

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.FormatInfoMessageStderr("Ready to "+action))
fmt.Fprintln(os.Stderr, console.FormatListItemStderr("Repository: "+preview.Repo))
Comment on lines 85 to +87
if preview.TotalWorkflows > 0 {
fmt.Fprintf(os.Stderr, " - Workflows: %d\n", preview.TotalWorkflows)
fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("Workflows: %d", preview.TotalWorkflows)))
}
if len(preview.Workflows) > 0 {
fmt.Fprintln(os.Stderr, " - Pending workflow updates:")
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)))

@github-actions github-actions Bot Jul 5, 2026

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.

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.

}
}
currentVersion := normalizeDisplayVersion(preview.CurrentVersion)
targetVersion := normalizeDisplayVersion(GetVersion())
if currentVersion != "" {
if targetVersion != "" && targetVersion != currentVersion {
fmt.Fprintf(os.Stderr, " - Compiler version: %s -> %s\n", currentVersion, targetVersion)
fmt.Fprintln(os.Stderr, console.FormatListItemStderr(fmt.Sprintf("Compiler version: %s -> %s", currentVersion, targetVersion)))
} else {
fmt.Fprintf(os.Stderr, " - Compiler version: %s\n", currentVersion)
fmt.Fprintln(os.Stderr, console.FormatListItemStderr("Compiler version: "+currentVersion))
}
}
}
Expand Down
69 changes: 69 additions & 0 deletions pkg/cli/stderr_console_formatting_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
//go:build !integration

package cli

import (
"os"
"path/filepath"
"strings"
"testing"

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/constants"
"github.com/github/gh-aw/pkg/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestRenderOrgActionSummaryUsesConsoleFormatListItemStderr(t *testing.T) {
preview := orgRepoPreview{
Repo: "octo/repo",
TotalWorkflows: 2,
Workflows: []orgWorkflowPreview{
{Name: "repo-assist", CurrentRef: "v1.0.0", LatestRef: "v1.1.0"},
},
}

_, stderr := captureOutput(t, func() error {
renderOrgActionSummary(preview, "update")
return nil
})

lines := strings.Split(strings.TrimSuffix(stderr, "\n"), "\n")
require.Len(t, lines, 5)
assert.Equal(t, console.FormatInfoMessageStderr("Ready to update"), lines[0])
assert.Equal(t, console.FormatListItemStderr("Repository: octo/repo"), lines[1])
Comment thread
Copilot marked this conversation as resolved.
assert.Equal(t, console.FormatListItemStderr("Workflows: 2"), lines[2])
assert.Equal(t, console.FormatListItemStderr("Pending workflow updates:"), lines[3])
assert.Equal(t, console.FormatListItemStderr("repo-assist: v1.0.0 -> v1.1.0"), lines[4])
}

func TestFindWorkflowsWithMCPServerUsesConsoleFormatInfoMessageStderr(t *testing.T) {
tmpDir := testutil.TempDir(t, "test-*")
workflowsDir := filepath.Join(tmpDir, constants.GetWorkflowDir())
require.NoError(t, os.MkdirAll(workflowsDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "workflow1.md"), []byte(`---
tools:
github:
allowed: ["create_issue"]
---
# Workflow 1
`), 0o644))

originalDir, err := os.Getwd()
require.NoError(t, err)
defer func() {
_ = os.Chdir(originalDir)
}()
require.NoError(t, os.Chdir(tmpDir))

_, stderr := captureOutput(t, func() error {
return findWorkflowsWithMCPServer(workflowsDir, "github", false)
})

lines := strings.Split(strings.TrimSuffix(stderr, "\n"), "\n")
require.Len(t, lines, 3)
assert.Equal(t, console.FormatInfoMessageStderr("Found MCP server 'github' in 1 workflow(s): workflow1"), lines[0])
assert.Empty(t, lines[1])
assert.Equal(t, console.FormatInfoMessageStderr("Run 'gh aw mcp list-tools <workflow-name> --server github' to list tools for a specific workflow"), lines[2])
}
Loading