Skip to content

Commit a2ce4e3

Browse files
terassyiclaude
andauthored
feat(plan): split tree output into Tools / Privileged Tools / System sections (#244)
* feat(plan): split tree output into Tools / Privileged Tools / System sections The combined "Dependency Graph:" tree made privileged Tools indistinguishable from regular Tools in plan output, even though they route to a different bin directory and only run under --system. Split it into three sections keyed by ResourceInfo.Privileged + resource.IsSystemKind so the privilege/system boundary is visible at a glance. - graph.ResourceInfo gains a Privileged bool - graph.PrintTreeFiltered renders a filtered subtree; PrintTree becomes a thin all-pass wrapper, so existing callers are byte-identical - plan.buildResourceInfo populates Privileged from resource.IsPrivileged on the install, disabled, and removal paths so privileged removals also surface in the right section - renderSectionedTree emits each section only if non-empty, with one blank line between adjacent sections; PrintLayers' leading \n still spaces it from "Execution Order:" — no double-blank - both printTextPlan and planForResources (apply pre-confirm) use the helper so plan and apply show parallel output Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(graph): pin all-pass PrintTreeFiltered to a stable expected tree The prior "all-pass filter equals PrintTree" assertion was tautological: PrintTree now delegates to PrintTreeFiltered, so it compared the output to itself and would not catch a future regression that broke both call paths equally. Replace with a literal snapshot string and assert that both PrintTree and PrintTreeFiltered(all-pass) produce it byte-for-byte. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 17478fd commit a2ce4e3

4 files changed

Lines changed: 358 additions & 29 deletions

File tree

cmd/tomei/plan.go

Lines changed: 74 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,10 @@ func buildResourceInfo(resources []resource.Resource, updCfg engine.UpdateConfig
186186
}
187187

188188
resInfo := graph.ResourceInfo{
189-
Kind: res.Kind(),
190-
Name: res.Name(),
191-
Action: resource.ActionInstall, // default to install
189+
Kind: res.Kind(),
190+
Name: res.Name(),
191+
Action: resource.ActionInstall, // default to install
192+
Privileged: resource.IsPrivileged(res),
192193
}
193194

194195
// Get version from spec
@@ -278,10 +279,11 @@ func buildResourceInfo(resources []resource.Resource, updCfg engine.UpdateConfig
278279
action = resource.ActionSkip
279280
}
280281
info[nodeID] = graph.ResourceInfo{
281-
Kind: resource.KindTool,
282-
Name: name,
283-
Version: tool.Version,
284-
Action: action,
282+
Kind: resource.KindTool,
283+
Name: name,
284+
Version: tool.Version,
285+
Action: action,
286+
Privileged: tool.Privileged,
285287
}
286288
}
287289
}
@@ -328,10 +330,10 @@ func printTextPlan(cmd *cobra.Command, args []string, resources []resource.Resou
328330
cmd.Printf("Planning changes for %v\n\n", args)
329331
cmd.Printf("Found %d resource(s)\n\n", len(resources))
330332

331-
// Print dependency tree
332-
cmd.Println("Dependency Graph:")
333-
printer := graph.NewTreePrinter(cmd.OutOrStdout(), planCfg.noColor)
334-
printer.PrintTree(result.resolver, result.resourceInfo)
333+
// Print dependency tree, split into Tools / Privileged Tools / System sections
334+
w := cmd.OutOrStdout()
335+
printer := graph.NewTreePrinter(w, planCfg.noColor)
336+
renderSectionedTree(printer, w, result)
335337

336338
// Print execution layers
337339
printer.PrintLayers(result.filteredLayers, result.resourceInfo)
@@ -425,13 +427,69 @@ func planForResources(w io.Writer, resources []resource.Resource, disableColor b
425427

426428
fmt.Fprintf(w, "Found %d resource(s)\n\n", len(resources))
427429
printer := graph.NewTreePrinter(w, disableColor)
428-
printer.PrintTree(result.resolver, result.resourceInfo)
430+
renderSectionedTree(printer, w, result)
429431
printer.PrintLayers(result.filteredLayers, result.resourceInfo)
430432
printer.PrintSummary(result.resourceInfo)
431433

432434
return hasChanges, nil
433435
}
434436

437+
// renderSectionedTree prints the dependency tree split into three subsections
438+
// keyed by privilege/system-kind: regular Tools, Privileged Tools (require
439+
// --system), and System resources. Empty sections are skipped. A node present
440+
// in the resolver but absent from result.resourceInfo (e.g. builtin
441+
// Installers added by engine.AppendBuiltinInstallers) gets a zero-value
442+
// ResourceInfo and therefore routes to "Tools:".
443+
func renderSectionedTree(printer *graph.TreePrinter, w io.Writer, result *planResult) {
444+
type section struct {
445+
heading string
446+
include func(graph.NodeID, graph.ResourceInfo) bool
447+
}
448+
sections := []section{
449+
{"Tools:", func(_ graph.NodeID, info graph.ResourceInfo) bool {
450+
return !info.Privileged && !resource.IsSystemKind(info.Kind)
451+
}},
452+
{"Privileged Tools (--system):", func(_ graph.NodeID, info graph.ResourceInfo) bool {
453+
return info.Privileged
454+
}},
455+
{"System:", func(_ graph.NodeID, info graph.ResourceInfo) bool {
456+
return resource.IsSystemKind(info.Kind)
457+
}},
458+
}
459+
460+
// Lift (NodeID, ResourceInfo) predicates to the NodeID-only form
461+
// PrintTreeFiltered expects, looking the info up once per call.
462+
lift := func(pred func(graph.NodeID, graph.ResourceInfo) bool) func(graph.NodeID) bool {
463+
return func(id graph.NodeID) bool { return pred(id, result.resourceInfo[id]) }
464+
}
465+
466+
// A section is non-empty if any resolver node passes its predicate.
467+
// Walking resolver.GetNodes() (not just result.resourceInfo) ensures
468+
// builtin Installers — which sit in the resolver but not in the info
469+
// map — count toward the "Tools:" section.
470+
nodes := result.resolver.GetNodes()
471+
rendered := 0
472+
for _, s := range sections {
473+
include := lift(s.include)
474+
empty := true
475+
for _, n := range nodes {
476+
if include(n.ID) {
477+
empty = false
478+
break
479+
}
480+
}
481+
if empty {
482+
continue
483+
}
484+
if rendered > 0 {
485+
fmt.Fprintln(w)
486+
}
487+
fmt.Fprintln(w, s.heading)
488+
printer.PrintTreeFiltered(result.resolver, result.resourceInfo, include)
489+
rendered++
490+
}
491+
}
492+
435493
// addDisabledResourceInfo injects disabled resources into the resource info map.
436494
// Resources that already have an entry (e.g., ActionRemove for previously installed) are not overwritten.
437495
func addDisabledResourceInfo(info map[graph.NodeID]graph.ResourceInfo, disabled []resource.Resource) {
@@ -442,9 +500,10 @@ func addDisabledResourceInfo(info map[graph.NodeID]graph.ResourceInfo, disabled
442500
continue
443501
}
444502
ri := graph.ResourceInfo{
445-
Kind: res.Kind(),
446-
Name: res.Name(),
447-
Action: resource.ActionSkip,
503+
Kind: res.Kind(),
504+
Name: res.Name(),
505+
Action: resource.ActionSkip,
506+
Privileged: resource.IsPrivileged(res),
448507
}
449508
if tool, ok := res.(*resource.Tool); ok && tool.ToolSpec != nil {
450509
ri.Version = tool.ToolSpec.Version

cmd/tomei/plan_test.go

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package main
22

33
import (
4+
"bytes"
45
"encoding/json"
56
"os"
67
"path/filepath"
8+
"strings"
79
"testing"
810
"time"
911

@@ -16,6 +18,26 @@ import (
1618
"github.com/terassyi/tomei/internal/state"
1719
)
1820

21+
// stubResolver is a minimal graph.Resolver for renderSectionedTree tests.
22+
type stubResolver struct {
23+
nodes []*graph.Node
24+
edges []graph.Edge
25+
}
26+
27+
func (s *stubResolver) AddResource(resource.Resource) {}
28+
func (s *stubResolver) Resolve() ([]graph.Layer, error) {
29+
return nil, nil
30+
}
31+
func (s *stubResolver) Validate() error { return nil }
32+
func (s *stubResolver) NodeCount() int { return len(s.nodes) }
33+
func (s *stubResolver) EdgeCount() int { return len(s.edges) }
34+
func (s *stubResolver) GetEdges() []graph.Edge { return s.edges }
35+
func (s *stubResolver) GetNodes() []*graph.Node { return s.nodes }
36+
37+
func makeNode(kind resource.Kind, name string) *graph.Node {
38+
return &graph.Node{ID: graph.NewNodeID(kind, name), Kind: kind, Name: name}
39+
}
40+
1941
func TestCollectSkipInfos(t *testing.T) {
2042
t.Parallel()
2143

@@ -406,3 +428,134 @@ func TestAddSystemResourceInfo(t *testing.T) {
406428
"reconciler emits ActionRemove; the skip-downgrade loop was removed in #198 so Remove must pass through")
407429
})
408430
}
431+
432+
func TestRenderSectionedTree(t *testing.T) {
433+
t.Parallel()
434+
435+
// Common nodes used across cases.
436+
nAqua := makeNode(resource.KindInstaller, "aqua")
437+
nBat := makeNode(resource.KindTool, "bat")
438+
nLazygit := makeNode(resource.KindTool, "lazygit")
439+
nApt := makeNode(resource.KindSystemInstaller, "apt")
440+
nPkgs := makeNode(resource.KindSystemPackageSet, "build-deps")
441+
442+
mixed := func() *planResult {
443+
return &planResult{
444+
resolver: &stubResolver{
445+
nodes: []*graph.Node{nAqua, nBat, nLazygit, nApt, nPkgs},
446+
edges: []graph.Edge{
447+
{From: nBat.ID, To: nAqua.ID},
448+
{From: nLazygit.ID, To: nAqua.ID},
449+
{From: nPkgs.ID, To: nApt.ID},
450+
},
451+
},
452+
resourceInfo: map[graph.NodeID]graph.ResourceInfo{
453+
// Installer/aqua is intentionally NOT in resourceInfo to also
454+
// exercise the "node in resolver but not in info map" path
455+
// (builtin Installers added by AppendBuiltinInstallers). It must
456+
// route to "Tools:" via the zero-value ResourceInfo.
457+
nBat.ID: {Kind: resource.KindTool, Name: "bat", Version: "0.24.0", Action: resource.ActionInstall},
458+
nLazygit.ID: {Kind: resource.KindTool, Name: "lazygit", Version: "v0.62.1", Action: resource.ActionSkip, Privileged: true},
459+
nApt.ID: {Kind: resource.KindSystemInstaller, Name: "apt", Action: resource.ActionInstall},
460+
nPkgs.ID: {Kind: resource.KindSystemPackageSet, Name: "build-deps", Action: resource.ActionInstall},
461+
},
462+
}
463+
}
464+
465+
t.Run("three sections appear in order with mixed manifest", func(t *testing.T) {
466+
t.Parallel()
467+
var buf bytes.Buffer
468+
renderSectionedTree(graph.NewTreePrinter(&buf, true), &buf, mixed())
469+
out := buf.String()
470+
toolsIdx := strings.Index(out, "Tools:\n")
471+
privIdx := strings.Index(out, "Privileged Tools (--system):\n")
472+
sysIdx := strings.Index(out, "System:\n")
473+
require.NotEqual(t, -1, toolsIdx)
474+
require.NotEqual(t, -1, privIdx)
475+
require.NotEqual(t, -1, sysIdx)
476+
assert.Less(t, toolsIdx, privIdx)
477+
assert.Less(t, privIdx, sysIdx)
478+
// privileged Tool kept in its own section even with ActionSkip
479+
assert.Contains(t, out, "Tool/lazygit (v0.62.1) [⊘ skip]")
480+
// the builtin-Installer-style entry (no resourceInfo) is rendered
481+
// under "Tools:" as the root with Tool/bat as its child.
482+
assert.Contains(t, out, "Installer/aqua")
483+
assert.Contains(t, out, "Tool/bat (0.24.0) [+ install]")
484+
})
485+
486+
t.Run("blank line guard: exactly one blank between Privileged and System sections", func(t *testing.T) {
487+
t.Parallel()
488+
var buf bytes.Buffer
489+
renderSectionedTree(graph.NewTreePrinter(&buf, true), &buf, mixed())
490+
out := buf.String()
491+
// "Privileged Tools (--system):" section ends with lazygit, then
492+
// exactly one blank line, then "System:" heading.
493+
assert.Contains(t, out, "Tool/lazygit (v0.62.1) [⊘ skip]\n\nSystem:\n")
494+
// And NO double blank line (would indicate stray fmt.Fprintln).
495+
assert.NotContains(t, out, "\n\n\nSystem:\n")
496+
})
497+
498+
t.Run("no privileged → Privileged Tools section omitted", func(t *testing.T) {
499+
t.Parallel()
500+
r := &planResult{
501+
resolver: &stubResolver{
502+
nodes: []*graph.Node{nAqua, nBat, nApt, nPkgs},
503+
edges: []graph.Edge{
504+
{From: nBat.ID, To: nAqua.ID},
505+
{From: nPkgs.ID, To: nApt.ID},
506+
},
507+
},
508+
resourceInfo: map[graph.NodeID]graph.ResourceInfo{
509+
nBat.ID: {Kind: resource.KindTool, Name: "bat", Action: resource.ActionInstall},
510+
nApt.ID: {Kind: resource.KindSystemInstaller, Name: "apt", Action: resource.ActionInstall},
511+
nPkgs.ID: {Kind: resource.KindSystemPackageSet, Name: "build-deps", Action: resource.ActionInstall},
512+
},
513+
}
514+
var buf bytes.Buffer
515+
renderSectionedTree(graph.NewTreePrinter(&buf, true), &buf, r)
516+
out := buf.String()
517+
assert.Contains(t, out, "Tools:\n")
518+
assert.NotContains(t, out, "Privileged Tools")
519+
assert.Contains(t, out, "System:\n")
520+
})
521+
522+
t.Run("only privileged → only that section appears", func(t *testing.T) {
523+
t.Parallel()
524+
r := &planResult{
525+
resolver: &stubResolver{
526+
nodes: []*graph.Node{nLazygit},
527+
edges: nil,
528+
},
529+
resourceInfo: map[graph.NodeID]graph.ResourceInfo{
530+
nLazygit.ID: {Kind: resource.KindTool, Name: "lazygit", Action: resource.ActionInstall, Privileged: true},
531+
},
532+
}
533+
var buf bytes.Buffer
534+
renderSectionedTree(graph.NewTreePrinter(&buf, true), &buf, r)
535+
out := buf.String()
536+
assert.NotContains(t, out, "Tools:\n")
537+
assert.NotContains(t, out, "System:\n")
538+
assert.Contains(t, out, "Privileged Tools (--system):\n")
539+
assert.Contains(t, out, "Tool/lazygit")
540+
})
541+
542+
t.Run("privileged Tool removal entry lands under Privileged section", func(t *testing.T) {
543+
t.Parallel()
544+
// Simulates buildResourceInfo's removal-detection branch carrying
545+
// Privileged through from ToolState. Without that propagation, this
546+
// entry would render under "Tools:" instead.
547+
nGone := makeNode(resource.KindTool, "gone-priv")
548+
r := &planResult{
549+
resolver: &stubResolver{nodes: []*graph.Node{nGone}},
550+
resourceInfo: map[graph.NodeID]graph.ResourceInfo{
551+
nGone.ID: {Kind: resource.KindTool, Name: "gone-priv", Version: "1.0.0", Action: resource.ActionRemove, Privileged: true},
552+
},
553+
}
554+
var buf bytes.Buffer
555+
renderSectionedTree(graph.NewTreePrinter(&buf, true), &buf, r)
556+
out := buf.String()
557+
assert.NotContains(t, out, "Tools:\n")
558+
assert.Contains(t, out, "Privileged Tools (--system):\n")
559+
assert.Contains(t, out, "Tool/gone-priv (1.0.0) [- remove]")
560+
})
561+
}

internal/graph/tree.go

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@ import (
1313

1414
// ResourceInfo holds information about a resource for display.
1515
type ResourceInfo struct {
16-
Kind resource.Kind
17-
Name string
18-
Version string
19-
Action resource.ActionType
16+
Kind resource.Kind
17+
Name string
18+
Version string
19+
Action resource.ActionType
20+
Privileged bool
2021
}
2122

2223
// TreePrinter prints dependency graphs as ASCII trees with colors.
@@ -65,35 +66,42 @@ func NewTreePrinter(w io.Writer, noColor bool) *TreePrinter {
6566

6667
// PrintTree prints the dependency graph as an ASCII tree.
6768
func (p *TreePrinter) PrintTree(resolver Resolver, resourceInfo map[NodeID]ResourceInfo) {
69+
p.PrintTreeFiltered(resolver, resourceInfo, func(NodeID) bool { return true })
70+
}
71+
72+
// PrintTreeFiltered renders the dependency tree, including only nodes for
73+
// which includeNode returns true. A node whose passing parents are all
74+
// filtered out is promoted to a root, so removing an Installer parent
75+
// causes its (passing) children to surface as top-level entries.
76+
func (p *TreePrinter) PrintTreeFiltered(resolver Resolver, resourceInfo map[NodeID]ResourceInfo, includeNode func(NodeID) bool) {
6877
edges := resolver.GetEdges()
6978
nodes := resolver.GetNodes()
7079

71-
// Build adjacency list: node -> children (dependents)
72-
// We want to show: parent depends on children, so we reverse the edge direction
7380
children := make(map[NodeID][]NodeID)
74-
hasParent := make(map[NodeID]bool)
81+
passingParents := make(map[NodeID]int)
7582

7683
for _, edge := range edges {
77-
// edge.From depends on edge.To
78-
// In tree view: edge.To is parent, edge.From is child
84+
if !includeNode(edge.From) || !includeNode(edge.To) {
85+
continue
86+
}
7987
children[edge.To] = append(children[edge.To], edge.From)
80-
hasParent[edge.From] = true
88+
passingParents[edge.From]++
8189
}
8290

83-
// Find root nodes (no dependencies)
8491
var roots []NodeID
8592
for _, node := range nodes {
86-
if !hasParent[node.ID] {
93+
if !includeNode(node.ID) {
94+
continue
95+
}
96+
if passingParents[node.ID] == 0 {
8797
roots = append(roots, node.ID)
8898
}
8999
}
90100

91-
// Sort roots for deterministic output
92101
slices.SortFunc(roots, func(a, b NodeID) int {
93102
return strings.Compare(string(a), string(b))
94103
})
95104

96-
// Print each root tree
97105
for _, root := range roots {
98106
p.printNode(root, "", true, children, resourceInfo, true)
99107
}

0 commit comments

Comments
 (0)