From 0045bbdaa06af2eba6c1a5d38907665fb2e839e3 Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Fri, 26 Jun 2026 19:31:46 +0900 Subject: [PATCH 1/2] fix: keep completion subcommand order deterministic in help output The completion command exposes each shell (bash, zsh, fish, pwsh) as a subcommand. These were generated by ranging over the shellCompletions map, whose iteration order Go randomizes per process. As a result the order of subcommands in `completion --help` changed between runs, which broke downstream projects that generate and commit documentation from --help output. Iterate over an explicitly ordered slice of shell names so the listing is stable (bash, zsh, fish, pwsh) on every run. Co-Authored-By: Claude Opus 4.8 --- completion.go | 10 ++++++++-- completion_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/completion.go b/completion.go index 167b0c0123..f14b01430a 100644 --- a/completion.go +++ b/completion.go @@ -20,6 +20,12 @@ var ( //go:embed autocomplete autoCompleteFS embed.FS + // completionShells defines the order in which the shell completion + // subcommands appear in help output. Iterating shellCompletions directly + // would use Go's randomized map order, making the listing nondeterministic. + // Keep this in sync with shellCompletions. + completionShells = []string{"bash", "zsh", "fish", "pwsh"} + shellCompletions = map[string]renderCompletion{ "bash": func(c *Command, appName string) (string, error) { b, err := autoCompleteFS.ReadFile("autocomplete/bash_autocomplete") @@ -65,8 +71,8 @@ func buildCompletionCommand(appName string) *Command { isCompletionCommand: true, } - for shell, render := range shellCompletions { - cmd.Commands = append(cmd.Commands, buildShellCompletionSubcommand(shell, render, appName)) + for _, shell := range completionShells { + cmd.Commands = append(cmd.Commands, buildShellCompletionSubcommand(shell, shellCompletions[shell], appName)) } return cmd diff --git a/completion_test.go b/completion_test.go index b248e352db..2b23350eaa 100644 --- a/completion_test.go +++ b/completion_test.go @@ -118,6 +118,31 @@ func TestCompletionShell(t *testing.T) { } } +func TestCompletionSubcommandOrder(t *testing.T) { + // The completion subcommands must appear in a deterministic order so that + // help output (and docs generated from it) does not change between runs. + // Previously they were built by iterating a map, whose order Go randomizes. + want := []string{"bash", "zsh", "fish", "pwsh"} + + // Build several times to guard against intra-process variation. + for range 10 { + cmd := buildCompletionCommand("foo") + + got := make([]string, 0, len(cmd.Commands)) + for _, sub := range cmd.Commands { + got = append(got, sub.Name) + } + + assert.Equal(t, want, got) + } + + // Every shell in shellCompletions must be represented in the ordered list. + assert.Len(t, completionShells, len(shellCompletions)) + for shell := range shellCompletions { + assert.Contains(t, completionShells, shell) + } +} + func TestCompletionBashNoShebang(t *testing.T) { // Regression test for https://github.com/urfave/cli/issues/2259 // Bash completion scripts are sourced, not executed, so they must not @@ -494,6 +519,8 @@ func TestCompletionShellRenderError(t *testing.T) { } return "something", nil } + defer func(orig []string) { completionShells = orig }(completionShells) + completionShells = append(completionShells, unknownShellName) defer func() { delete(shellCompletions, unknownShellName) }() @@ -522,6 +549,8 @@ func TestCompletionShellWriteError(t *testing.T) { shellCompletions[shellName] = func(c *Command, appName string) (string, error) { return "something", nil } + defer func(orig []string) { completionShells = orig }(completionShells) + completionShells = append(completionShells, shellName) defer func() { delete(shellCompletions, shellName) }() From 80f262538eb6a2b0a6435d3b870419498e9a59be Mon Sep 17 00:00:00 2001 From: Shunsuke Suzuki Date: Fri, 26 Jun 2026 19:40:06 +0900 Subject: [PATCH 2/2] test: explain why completionShells is mutated in completion error tests Co-Authored-By: Claude Opus 4.8 --- completion_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/completion_test.go b/completion_test.go index 2b23350eaa..8550f6b41a 100644 --- a/completion_test.go +++ b/completion_test.go @@ -519,6 +519,9 @@ func TestCompletionShellRenderError(t *testing.T) { } return "something", nil } + // buildCompletionCommand only turns shells listed in completionShells into + // subcommands, so register the injected shell there too (restoring the + // original slice afterward) for it to be reachable. defer func(orig []string) { completionShells = orig }(completionShells) completionShells = append(completionShells, unknownShellName) defer func() { @@ -549,6 +552,9 @@ func TestCompletionShellWriteError(t *testing.T) { shellCompletions[shellName] = func(c *Command, appName string) (string, error) { return "something", nil } + // buildCompletionCommand only turns shells listed in completionShells into + // subcommands, so register the injected shell there too (restoring the + // original slice afterward) for it to be reachable. defer func(orig []string) { completionShells = orig }(completionShells) completionShells = append(completionShells, shellName) defer func() {