Adopt Lipgloss tree rendering for MCP hierarchy and status dependencies (#33276)

Author: Copilot

docs/adr/33276-adopt-lipgloss-tree-for-cli-hierarchies.md

@@ -0,0 +1,80 @@
+# ADR-33276: Adopt Lipgloss Tree Rendering for CLI Hierarchies
+
+**Date**: 2026-05-19
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+## Part 1 — Narrative (Human-Friendly)
+
+### Context
+
+CLI output for `mcp inspect` and `status` previously mixed ad-hoc hierarchy formatting (manual indentation, custom prefixes) with table-only views. Nested relationships — a workflow's engine and MCP servers, or a workflow's dependency graph — were hard to read because they were either flattened into tables or rendered with bespoke string-building logic in each command. The codebase already standardized terminal styling under `pkg/styles` (including `TreeEnumerator` and `TreeNode`) and has adopted `charm.land/lipgloss/v2` as the terminal rendering library, so a shared tree primitive was available but not yet used for hierarchical command output.
+
+### Decision
+
+We will adopt `charm.land/lipgloss/v2/tree` as the canonical primitive for rendering hierarchical CLI output in `pkg/cli`, starting with `mcp inspect` (workflow → engine + MCP servers) and the verbose mode of `status` (workflow → dependencies). All such trees will use `tree.RoundedEnumerator` together with the existing `styles.TreeEnumerator` and `styles.TreeNode` styles, so hierarchical output is visually consistent across commands.
+
+### Alternatives Considered
+
+#### Alternative 1: Keep manual hierarchy formatting per command
+
+Each command would continue building its own indented strings (or rely on table-only views) to convey hierarchy. Rejected because it duplicates formatting logic across commands, drifts visually as each command evolves independently, and makes nested relationships (a tree with three or more levels) hard to express without reinventing tree primitives.
+
+#### Alternative 2: Write a small in-repo tree printer
+
+We could add a focused helper under `pkg/console` or `pkg/styles` that prints trees with the project's style tokens, avoiding a new dependency surface. Rejected because `lipgloss/v2/tree` is already a transitive dependency (the project uses `lipgloss/v2` extensively for terminal styling), and reimplementing tree layout, enumerator handling, and styling would duplicate well-tested upstream code with no clear benefit.
+
+### Consequences
+
+#### Positive
+- Hierarchical CLI output is rendered consistently across commands using one library and one set of styles.
+- Adding a new hierarchical view in another command becomes a small, declarative composition of `tree.Root(...).Child(...)` calls instead of bespoke string formatting.
+- The tree rendering is testable as a pure function (see the new `*_tree_test.go` files), which is harder to do for ad-hoc string concatenation.
+
+#### Negative
+- Adds a concrete coupling between `pkg/cli` command code and the `charm.land/lipgloss/v2/tree` API; replacing the renderer in the future would require touching every site that builds a tree.
+- Tree output is written to `stderr` alongside existing info messages, which can be noisier in `mcp inspect` and in verbose `status` runs that pipe through tooling expecting a clean stderr.
+
+#### Neutral
+- The shape of `WorkflowStatus` grows a new `Dependencies []string` field (JSON-tagged `omitempty`, console-tagged `-`), so JSON consumers may now see a `dependencies` key for workflows that import or include other files.
+- Dependency extraction logic now lives in `pkg/cli/status_command.go` (frontmatter `imports` in string/list/object forms plus inline `@include`/`@import` directives, with `#section` stripping, dedup, and sort); future changes to import/include syntax must update this extractor.
+
+---
+
+## Part 2 — Normative Specification (RFC 2119)
+
+> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119).
+
+### Tree Rendering Library
+
+1. Hierarchical CLI output in `pkg/cli` **MUST** be rendered using `charm.land/lipgloss/v2/tree`.
+2. Implementations **MUST NOT** introduce a second tree-rendering library or hand-rolled ASCII tree printer in `pkg/cli` while this ADR is in effect.
+3. Tree composition **SHOULD** use `tree.Root(...).Child(...)` chains rather than mutating tree nodes after construction.
+
+### Styling Consistency
+
+1. Tree renderers in `pkg/cli` **MUST** apply `tree.RoundedEnumerator` as the enumerator.
+2. Tree renderers **MUST** apply `styles.TreeEnumerator` as the `EnumeratorStyle` and `styles.TreeNode` as the `ItemStyle`.
+3. Renderers **SHOULD NOT** override these styles inline with per-call style values; new style variants **SHOULD** be added to `pkg/styles` first.
+
+### Output Channel and Triggering
+
+1. Tree output for `mcp inspect` and `status` **MUST** be written to `stderr`, alongside existing informational messages, and **MUST NOT** be written to `stdout` when `stdout` is reserved for machine-readable output (e.g., JSON).
+2. The dependency tree in `status` **MUST** only be rendered when verbose text output is requested; it **MUST NOT** be rendered in JSON mode or in non-verbose runs.
+3. Renderers **MUST** return an empty string when there is nothing to display (e.g., zero MCP servers, zero dependencies) so the caller can skip the surrounding section header.
+
+### Dependency Extraction (Status Command)
+
+1. The status command **MUST** extract workflow dependencies from both frontmatter `imports` (accepting string, list, and object-with-`uses` forms) and inline `@include` / `@import` directives in the workflow body.
+2. Extracted dependencies **MUST** be normalized by stripping any `#section` suffix, deduplicated, and sorted before rendering.
+3. The `Dependencies` field on `WorkflowStatus` **MUST** be JSON-tagged `omitempty` so workflows without dependencies do not emit an empty list in JSON output.
+
+### Conformance
+
+An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26092090247) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/cli/mcp_inspect.go

@@ -7,11 +7,15 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
+	"strings"
 	"time"
 
+	"charm.land/lipgloss/v2/tree"
 	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/parser"
+	"github.com/github/gh-aw/pkg/styles"
 	"github.com/github/gh-aw/pkg/workflow"
 	"github.com/spf13/cobra"
 )
@@ -155,6 +159,13 @@ func InspectWorkflowMCP(workflowFile string, serverFilter string, toolFilter str
 	} else {
 		fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Found %d MCP server(s) to inspect", len(mcpConfigs))))
 	}
+	if toolFilter == "" {
+		if hierarchy := renderMCPInspectionTree(workflowPath, workflowData, mcpConfigs); hierarchy != "" {
+			fmt.Fprintln(os.Stderr)
+			fmt.Fprintln(os.Stderr, console.FormatSectionHeader("MCP Server Hierarchy"))
+			fmt.Fprintln(os.Stderr, hierarchy)
+		}
+	}
 	fmt.Fprintln(os.Stderr)
 
 	for i, config := range mcpConfigs {
@@ -172,6 +183,54 @@ func InspectWorkflowMCP(workflowFile string, serverFilter string, toolFilter str
 	return nil
 }
 
+func renderMCPInspectionTree(workflowPath string, workflowData *workflow.WorkflowData, mcpConfigs []parser.RegistryMCPServerConfig) string {
+	if len(mcpConfigs) == 0 {
+		return ""
+	}
+
+	workflowLabel := workflowPath
+	if workflowData != nil && workflowData.WorkflowID != "" {
+		workflowLabel = workflowData.WorkflowID
+	} else if workflowPath != "" {
+		workflowLabel = filepath.Base(workflowPath)
+	}
+
+	serversTree := tree.Root("MCP Servers")
+	sortedConfigs := append([]parser.RegistryMCPServerConfig(nil), mcpConfigs...)
+	slices.SortFunc(sortedConfigs, func(a, b parser.RegistryMCPServerConfig) int {
+		if nameCmp := strings.Compare(a.Name, b.Name); nameCmp != 0 {
+			return nameCmp
+		}
+		return strings.Compare(a.Type, b.Type)
+	})
+
+	for _, config := range sortedConfigs {
+		serversTree.Child(fmt.Sprintf("%s (%s)", config.Name, config.Type))
+	}
+
+	return tree.
+		Root("Workflow: " + workflowLabel).
+		Child("Engine: " + resolveWorkflowEngineID(workflowData)).
+		Child(serversTree).
+		Enumerator(tree.RoundedEnumerator).
+		EnumeratorStyle(styles.TreeEnumerator).
+		ItemStyle(styles.TreeNode).
+		String()
+}
+
+func resolveWorkflowEngineID(workflowData *workflow.WorkflowData) string {
+	if workflowData == nil {
+		return "unknown"
+	}
+	if workflowData.EngineConfig != nil && workflowData.EngineConfig.ID != "" {
+		return workflowData.EngineConfig.ID
+	}
+	if workflowData.AI != "" {
+		return workflowData.AI
+	}
+	return "unknown"
+}
+
 // NewMCPInspectSubcommand creates the mcp inspect subcommand
 // This is the former mcp inspect command now nested under mcp
 func NewMCPInspectSubcommand() *cobra.Command {

pkg/cli/mcp_inspect_tree_test.go

@@ -0,0 +1,102 @@
+package cli
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/github/gh-aw/pkg/parser"
+	"github.com/github/gh-aw/pkg/types"
+	"github.com/github/gh-aw/pkg/workflow"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRenderMCPInspectionTree(t *testing.T) {
+	workflowData := &workflow.WorkflowData{
+		WorkflowID: "audit-workflows",
+		EngineConfig: &workflow.EngineConfig{
+			ID: "copilot",
+		},
+	}
+	mcpConfigs := []parser.RegistryMCPServerConfig{
+		{BaseMCPServerConfig: types.BaseMCPServerConfig{Type: "stdio"}, Name: "github"},
+		{BaseMCPServerConfig: types.BaseMCPServerConfig{Type: "http"}, Name: "playwright"},
+	}
+
+	result := renderMCPInspectionTree("/tmp/audit-workflows.md", workflowData, mcpConfigs)
+
+	expected := []string{
+		"Workflow: audit-workflows",
+		"Engine: copilot",
+		"MCP Servers",
+		"github (stdio)",
+		"playwright (http)",
+	}
+	for _, part := range expected {
+		assert.Contains(t, result, part, "tree output should include expected hierarchy node")
+	}
+}
+
+func TestRenderMCPInspectionTree_SortsServersDeterministically(t *testing.T) {
+	workflowData := &workflow.WorkflowData{
+		WorkflowID: "audit-workflows",
+		EngineConfig: &workflow.EngineConfig{
+			ID: "copilot",
+		},
+	}
+	mcpConfigs := []parser.RegistryMCPServerConfig{
+		{BaseMCPServerConfig: types.BaseMCPServerConfig{Type: "http"}, Name: "playwright"},
+		{BaseMCPServerConfig: types.BaseMCPServerConfig{Type: "stdio"}, Name: "github"},
+		{BaseMCPServerConfig: types.BaseMCPServerConfig{Type: "docker"}, Name: "github"},
+	}
+
+	result := renderMCPInspectionTree("/tmp/audit-workflows.md", workflowData, mcpConfigs)
+	githubDockerIdx := strings.Index(result, "github (docker)")
+	githubStdioIdx := strings.Index(result, "github (stdio)")
+	playwrightIdx := strings.Index(result, "playwright (http)")
+
+	assert.NotEqual(t, -1, githubDockerIdx)
+	assert.NotEqual(t, -1, githubStdioIdx)
+	assert.NotEqual(t, -1, playwrightIdx)
+	assert.Less(t, githubDockerIdx, githubStdioIdx)
+	assert.Less(t, githubStdioIdx, playwrightIdx)
+}
+
+func TestResolveWorkflowEngineID(t *testing.T) {
+	tests := []struct {
+		name         string
+		workflowData *workflow.WorkflowData
+		want         string
+	}{
+		{
+			name:         "nil workflow data",
+			workflowData: nil,
+			want:         "unknown",
+		},
+		{
+			name: "engine config id",
+			workflowData: &workflow.WorkflowData{
+				EngineConfig: &workflow.EngineConfig{ID: "copilot"},
+				AI:           "claude",
+			},
+			want: "copilot",
+		},
+		{
+			name: "fallback to ai",
+			workflowData: &workflow.WorkflowData{
+				AI: "claude",
+			},
+			want: "claude",
+		},
+		{
+			name:         "unknown",
+			workflowData: &workflow.WorkflowData{},
+			want:         "unknown",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, resolveWorkflowEngineID(tt.workflowData))
+		})
+	}
+}