From 854426a0c932fb1ea26e02ec7d9be3ade4d45ae9 Mon Sep 17 00:00:00 2001
From: Eric Rakestraw <eric@rakestraw.us>
Date: Mon, 19 Oct 2026 15:16:07 +0000
Subject: [PATCH] Validate command alias termination

---
 docs/roadmap/implementation.md  |   8 +
 internal/alias/machine.go       |   5 +
 internal/alias/resolver_test.go | 131 ++++++++++++++++
 internal/alias/termination.go   | 262 ++++++++++++++++++++++++++++++++
 4 files changed, 406 insertions(+)
 create mode 100644 internal/alias/termination.go

diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md
index f2596e4..92a3707 100644
--- a/docs/roadmap/implementation.md
+++ b/docs/roadmap/implementation.md
@@ -321,6 +321,14 @@ finite cases. Verified with `go test ./internal/alias`.
 cases distinguish it from conservative declaration-cycle or bounded simulation
 checks. Do not integrate an approximate validator and defer correctness.
 
+**Completed:** Added worklist saturation for finite pop summaries, head-graph
+construction, cycle detection, and errors naming participating mappings.
+Terminal-marker transitions preserve the unique bottom marker, and tests verify
+markers cannot be popped. Direct, growing, and suffix-dependent loops are
+rejected; consuming repetition, longest-match suppression, and 80-rule chains
+in either registration order are accepted. Verified with
+`go test ./internal/alias -count=1`.
+
 ## Stage 4: Add captured declaration types and factories
 
 **Work**
diff --git a/internal/alias/machine.go b/internal/alias/machine.go
index 8af459a..f943014 100644
--- a/internal/alias/machine.go
+++ b/internal/alias/machine.go
@@ -39,6 +39,7 @@ type Machine struct {
 	transitions map[transitionKey]transition
 	alphabet    []symbol
 	literals    map[string]struct{}
+	mappings    []Mapping
 }
 
 // Compile builds a finite stack machine with explicit lookup, restoration,
@@ -55,11 +56,15 @@ func compileMachine(resolver *Resolver) *Machine {
 	// Trie controls only move to child nodes. Helper controls prepend a fixed
 	// number of saved symbols and then halt or return to the trie root after a
 	// selected mapping has been completed.
+	// End and boundary symbols are only restored as the final item below any
+	// emitted command tokens. No transition places a marker above ordinary
+	// input, so each remains the unique bottom marker of a valid stack.
 	machine := &Machine{
 		start:       0,
 		halt:        len(resolver.trie),
 		transitions: make(map[transitionKey]transition),
 		literals:    make(map[string]struct{}),
+		mappings:    resolver.mappings,
 	}
 	for _, mapping := range resolver.mappings {
 		for _, path := range []commandpath.Path{mapping.Source, mapping.Target} {
diff --git a/internal/alias/resolver_test.go b/internal/alias/resolver_test.go
index 773923c..d418cf8 100644
--- a/internal/alias/resolver_test.go
+++ b/internal/alias/resolver_test.go
@@ -1,7 +1,10 @@
 package alias
 
 import (
+	"errors"
 	"reflect"
+	"strconv"
+	"strings"
 	"testing"
 
 	"gitea.maximumdirect.net/eric/CommandKit/internal/commandpath"
@@ -138,6 +141,134 @@ func TestResolverRejectsInvalidAndConflictingMappings(t *testing.T) {
 	}
 }
 
+func TestTerminationRejectsCyclesAndGrowingPaths(t *testing.T) {
+	for name, mappings := range map[string][]Mapping{
+		"direct cycle": {
+			{Source: commandpath.Path{"a"}, Target: commandpath.Path{"b"}},
+			{Source: commandpath.Path{"b"}, Target: commandpath.Path{"a"}},
+		},
+		"growing path": {
+			{Source: commandpath.Path{"a"}, Target: commandpath.Path{"b", "extra"}},
+			{Source: commandpath.Path{"b"}, Target: commandpath.Path{"a"}},
+		},
+		"suffix-dependent cycle": {
+			{Source: commandpath.Path{"a"}, Target: commandpath.Path{"b"}},
+			{Source: commandpath.Path{"b"}, Target: commandpath.Path{"group"}},
+			{Source: commandpath.Path{"b", "x"}, Target: commandpath.Path{"a", "x"}},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			err := ValidateTermination(mappings)
+			var loop *NonTerminationError
+			if !errors.As(err, &loop) {
+				t.Fatalf("ValidateTermination() error = %v, want NonTerminationError", err)
+			}
+			if len(loop.Mappings) == 0 {
+				t.Fatal("NonTerminationError has no participating mappings")
+			}
+			for _, mapping := range loop.Mappings {
+				if !strings.Contains(err.Error(), mapping.Source.String()) {
+					t.Fatalf("error %q omits participating source %q", err, mapping.Source.String())
+				}
+			}
+			if name == "suffix-dependent cycle" {
+				resolver, err := NewResolver(mappings)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if got := resolver.Resolve(commandpath.Path{"a"}); !got.Equal(commandpath.Path{"group"}) {
+					t.Fatalf("bare route resolves to %#v, want [group]", got)
+				}
+				first, _, matched := resolver.ResolveOnce(commandpath.Path{"a", "x"})
+				second, _, matchedAgain := resolver.ResolveOnce(first)
+				if !matched || !matchedAgain || !second.Equal(commandpath.Path{"a", "x"}) {
+					t.Fatalf("suffix cycle is not demonstrated: first %#v, second %#v", first, second)
+				}
+			}
+		})
+	}
+}
+
+func TestTerminationAcceptsConsumingRepetition(t *testing.T) {
+	mappings := []Mapping{
+		{Source: commandpath.Path{"a"}, Target: commandpath.Path{"group"}},
+		{Source: commandpath.Path{"group", "again"}, Target: commandpath.Path{"a"}},
+	}
+	if err := ValidateTermination(mappings); err != nil {
+		t.Fatalf("ValidateTermination() = %v", err)
+	}
+	resolver, err := NewResolver(mappings)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := resolver.Resolve(commandpath.Path{"a", "again", "again"})
+	if want := (commandpath.Path{"group"}); !got.Equal(want) {
+		t.Fatalf("Resolve() = %#v, want %#v", got, want)
+	}
+}
+
+func TestTerminationUsesLongestMatchBeforeFollowingShorterRule(t *testing.T) {
+	mappings := []Mapping{
+		{Source: commandpath.Path{"a"}, Target: commandpath.Path{"b", "x"}},
+		{Source: commandpath.Path{"b"}, Target: commandpath.Path{"a"}},
+		{Source: commandpath.Path{"b", "x"}, Target: commandpath.Path{"group"}},
+	}
+	if err := ValidateTermination(mappings); err != nil {
+		t.Fatalf("ValidateTermination() = %v", err)
+	}
+	resolver, err := NewResolver(mappings)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := resolver.Resolve(commandpath.Path{"a"}); !got.Equal(commandpath.Path{"group"}) {
+		t.Fatalf("Resolve() = %#v, want [group]", got)
+	}
+}
+
+func TestTerminationAcceptsLongChainsInAnyRegistrationOrder(t *testing.T) {
+	const count = 80
+	mappings := make([]Mapping, 0, count)
+	for index := 0; index < count; index++ {
+		source := commandpath.Path{fmtAliasName(index)}
+		target := commandpath.Path{fmtAliasName(index + 1)}
+		if index == count-1 {
+			target = commandpath.Path{"canonical"}
+		}
+		mappings = append(mappings, Mapping{Source: source, Target: target})
+	}
+	if err := ValidateTermination(mappings); err != nil {
+		t.Fatalf("forward registration order: %v", err)
+	}
+	for left, right := 0, len(mappings)-1; left < right; left, right = left+1, right-1 {
+		mappings[left], mappings[right] = mappings[right], mappings[left]
+	}
+	if err := ValidateTermination(mappings); err != nil {
+		t.Fatalf("reverse registration order: %v", err)
+	}
+}
+
+func TestTerminalMarkersCannotBePopped(t *testing.T) {
+	_, machine, err := Compile([]Mapping{{Source: commandpath.Path{"a"}, Target: commandpath.Path{"b"}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	pop := saturatePopSummaries(machine)
+	for markerName, marker := range map[string]symbolKind{"end": endSymbol, "boundary": boundarySymbol} {
+		for from := range pop {
+			if from.symbol.kind == marker {
+				t.Fatalf("%s marker can be popped from control %d", markerName, from.control)
+			}
+		}
+	}
+	if err := ValidateTermination(nil); err != nil {
+		t.Fatalf("empty mapping set: %v", err)
+	}
+}
+
+func fmtAliasName(index int) string {
+	return "alias" + strconv.Itoa(index)
+}
+
 func expectedSymbols(machine *Machine, path commandpath.Path, marker symbolKind) []symbol {
 	result := make([]symbol, 0, len(path)+1)
 	for _, token := range path {
diff --git a/internal/alias/termination.go b/internal/alias/termination.go
new file mode 100644
index 0000000..f6bab45
--- /dev/null
+++ b/internal/alias/termination.go
@@ -0,0 +1,262 @@
+package alias
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+)
+
+type head struct {
+	control int
+	symbol  symbol
+}
+
+type popKey struct {
+	from head
+	to   int
+}
+
+type popSummary struct {
+	rules map[int]struct{}
+}
+
+type popWork struct {
+	key     popKey
+	summary popSummary
+}
+
+type transitionUse struct {
+	key        transitionKey
+	transition transition
+}
+
+type graphEdge struct {
+	to    head
+	rules map[int]struct{}
+}
+
+// NonTerminationError identifies mappings that participate in an alias loop.
+type NonTerminationError struct {
+	Mappings []Mapping
+}
+
+func (err *NonTerminationError) Error() string {
+	if err == nil || len(err.Mappings) == 0 {
+		return "command alias rewriting does not terminate"
+	}
+	parts := make([]string, 0, len(err.Mappings))
+	for _, mapping := range err.Mappings {
+		parts = append(parts, fmt.Sprintf("%q → %q", mapping.Source.String(), mapping.Target.String()))
+	}
+	return "command aliases do not terminate: " + strings.Join(parts, "; ")
+}
+
+// ValidateTermination rejects mapping sets whose deterministic prefix
+// rewriting can repeat forever for some finite command path.
+func ValidateTermination(mappings []Mapping) error {
+	_, machine, err := Compile(mappings)
+	if err != nil {
+		return err
+	}
+	if cycle := findHeadCycle(machine); cycle != nil {
+		return &NonTerminationError{Mappings: cycle}
+	}
+	return nil
+}
+
+func findHeadCycle(machine *Machine) []Mapping {
+	// Pop summaries exactly describe finite paths that remove one top symbol
+	// without inspecting below it. The head graph then represents nonempty
+	// paths that never pop below their starting level. Its cycle witnesses a
+	// repeatable computation even when the complete command path keeps growing.
+	pop := saturatePopSummaries(machine)
+	graph := buildHeadGraph(machine, pop)
+	colors := make(map[head]uint8, len(graph))
+	path := make([]head, 0)
+	pathEdges := make([]graphEdge, 0)
+	pathIndex := make(map[head]int)
+	var cycleRules map[int]struct{}
+
+	var visit func(head) bool
+	visit = func(current head) bool {
+		colors[current] = 1
+		pathIndex[current] = len(path)
+		path = append(path, current)
+		for _, edge := range graph[current] {
+			switch colors[edge.to] {
+			case 0:
+				pathEdges = append(pathEdges, edge)
+				if visit(edge.to) {
+					return true
+				}
+				pathEdges = pathEdges[:len(pathEdges)-1]
+			case 1:
+				start := pathIndex[edge.to]
+				cycleRules = make(map[int]struct{})
+				for _, cycleEdge := range pathEdges[start:] {
+					mergeRules(cycleRules, cycleEdge.rules)
+				}
+				mergeRules(cycleRules, edge.rules)
+				return true
+			}
+		}
+		path = path[:len(path)-1]
+		delete(pathIndex, current)
+		colors[current] = 2
+		return false
+	}
+
+	for current := range graph {
+		if colors[current] == 0 && visit(current) {
+			indices := make([]int, 0, len(cycleRules))
+			for index := range cycleRules {
+				if index >= 0 {
+					indices = append(indices, index)
+				}
+			}
+			sort.Ints(indices)
+			mappings := make([]Mapping, 0, len(indices))
+			for _, index := range indices {
+				if index < len(machine.mappings) {
+					mapping := machine.mappings[index]
+					mappings = append(mappings, Mapping{Source: mapping.Source.Clone(), Target: mapping.Target.Clone()})
+				}
+			}
+			return mappings
+		}
+	}
+	return nil
+}
+
+func saturatePopSummaries(machine *Machine) map[head]map[int]popSummary {
+	// Zero-symbol pushes seed the relation. Each newly found summary composes
+	// with transitions that push one symbol and with either side of transitions
+	// that push two symbols until no new finite pop path can be derived.
+	byHead := make(map[head]map[int]popSummary)
+	queue := make([]popWork, 0)
+	add := func(key popKey, rules map[int]struct{}) {
+		if byHead[key.from] == nil {
+			byHead[key.from] = make(map[int]popSummary)
+		}
+		if _, exists := byHead[key.from][key.to]; exists {
+			return
+		}
+		summary := popSummary{rules: cloneRules(rules)}
+		byHead[key.from][key.to] = summary
+		queue = append(queue, popWork{key: key, summary: summary})
+	}
+
+	ones := make(map[head][]transitionUse)
+	twosByPrefix := make(map[head][]transitionUse)
+	twosBySuffix := make(map[symbol][]transitionUse)
+	for key, operation := range machine.transitions {
+		switch len(operation.push) {
+		case 0:
+			rules := make(map[int]struct{})
+			addRule(rules, operation.rule)
+			add(popKey{from: head{control: key.control, symbol: key.read}, to: operation.next}, rules)
+		case 1:
+			ones[head{control: operation.next, symbol: operation.push[0]}] = append(ones[head{control: operation.next, symbol: operation.push[0]}], transitionUse{key: key, transition: operation})
+		case 2:
+			prefix := head{control: operation.next, symbol: operation.push[0]}
+			twosByPrefix[prefix] = append(twosByPrefix[prefix], transitionUse{key: key, transition: operation})
+			twosBySuffix[operation.push[1]] = append(twosBySuffix[operation.push[1]], transitionUse{key: key, transition: operation})
+		}
+	}
+
+	for cursor := 0; cursor < len(queue); cursor++ {
+		work := queue[cursor]
+		for _, use := range ones[work.key.from] {
+			add(popKey{
+				from: head{control: use.key.control, symbol: use.key.read},
+				to:   work.key.to,
+			}, combineRules(use.transition.rule, work.summary.rules))
+		}
+
+		for _, use := range twosByPrefix[work.key.from] {
+			suffix := head{control: work.key.to, symbol: use.transition.push[1]}
+			finals := make([]int, 0, len(byHead[suffix]))
+			for final := range byHead[suffix] {
+				finals = append(finals, final)
+			}
+			sort.Ints(finals)
+			for _, final := range finals {
+				summary := byHead[suffix][final]
+				add(popKey{
+					from: head{control: use.key.control, symbol: use.key.read},
+					to:   final,
+				}, combineRules(use.transition.rule, work.summary.rules, summary.rules))
+			}
+		}
+
+		for _, use := range twosBySuffix[work.key.from.symbol] {
+			if work.key.from.symbol != use.transition.push[1] {
+				continue
+			}
+			prefix := head{control: use.transition.next, symbol: use.transition.push[0]}
+			if first, exists := byHead[prefix][work.key.from.control]; exists {
+				add(popKey{
+					from: head{control: use.key.control, symbol: use.key.read},
+					to:   work.key.to,
+				}, combineRules(use.transition.rule, first.rules, work.summary.rules))
+			}
+		}
+	}
+	return byHead
+}
+
+func buildHeadGraph(machine *Machine, pop map[head]map[int]popSummary) map[head][]graphEdge {
+	graph := make(map[head][]graphEdge)
+	for key, operation := range machine.transitions {
+		source := head{control: key.control, symbol: key.read}
+		rules := make(map[int]struct{})
+		addRule(rules, operation.rule)
+		switch len(operation.push) {
+		case 1:
+			graph[source] = append(graph[source], graphEdge{
+				to:    head{control: operation.next, symbol: operation.push[0]},
+				rules: cloneRules(rules),
+			})
+		case 2:
+			graph[source] = append(graph[source], graphEdge{
+				to:    head{control: operation.next, symbol: operation.push[0]},
+				rules: cloneRules(rules),
+			})
+			prefix := head{control: operation.next, symbol: operation.push[0]}
+			for nextControl, summary := range pop[prefix] {
+				graph[source] = append(graph[source], graphEdge{
+					to:    head{control: nextControl, symbol: operation.push[1]},
+					rules: combineRules(operation.rule, summary.rules),
+				})
+			}
+		}
+	}
+	return graph
+}
+
+func addRule(rules map[int]struct{}, rule int) {
+	if rule >= 0 {
+		rules[rule] = struct{}{}
+	}
+}
+
+func cloneRules(rules map[int]struct{}) map[int]struct{} {
+	copy := make(map[int]struct{}, len(rules))
+	mergeRules(copy, rules)
+	return copy
+}
+
+func combineRules(rule int, groups ...map[int]struct{}) map[int]struct{} {
+	combined := make(map[int]struct{})
+	addRule(combined, rule)
+	for _, group := range groups {
+		mergeRules(combined, group)
+	}
+	return combined
+}
+
+func mergeRules(target, source map[int]struct{}) {
+	for rule := range source {
+		target[rule] = struct{}{}
+	}
+}