diff --git a/dispatcher.go b/dispatcher.go index 06f2991..7420e91 100644 --- a/dispatcher.go +++ b/dispatcher.go @@ -473,34 +473,35 @@ func (d *Dispatcher) findCommandWithInterspersedFlags(args []string) (*CommandEn fullArgs = append(fullArgs, args[lastCommandIndex+1:]...) } - // Try to validate our flag assumptions were correct - // Only validate flags that came before the command + // Validate that our value-consumption guesses were correct for the + // flags that came before the command. We only reject a match when a + // KNOWN flag was wrongly assumed to take a value (bool flags don't). + // + // We deliberately do NOT reject a match just because a pre-command + // flag is absent from this command's flagset: callers routinely place + // global flags (e.g. -C/--cluster) before a command whose own flagset + // doesn't declare them (such as section/namespace commands with empty + // flagsets). If a flag is genuinely bogus, fs.Parse reports it as a + // clear "unknown flag: X" instead of a misleading "unknown command". valid := true for _, fi := range skippedItems { - // Only check flags that came before the end of the command + // Only check flags that came before the end of the command; flags + // after the command are handled by Parse. if fi.index > lastCommandIndex { - continue // This flag is after the command, will be handled by Parse + continue } flagName := strings.TrimPrefix(fi.flag, "--") flagName = strings.TrimPrefix(flagName, "-") - // Check if this flag exists in the command's flagset - flagFound := false fs.VisitAll(func(f *Flag) { if (len(flagName) == 1 && f.Short == rune(flagName[0])) || f.Name == flagName { - flagFound = true - // Check if our assumption about the flag taking a value was correct + // Check if our assumption about the flag taking a value was correct. if fi.hasValue && f.Value.IsBool() { valid = false // Bool flags don't take values } } }) - - if !flagFound && !isHelpFlag(fi.flag) { - // Unknown flag (unless it's a help flag which is always valid) - valid = false - } } if valid { @@ -513,11 +514,6 @@ func (d *Dispatcher) findCommandWithInterspersedFlags(args []string) (*CommandEn return nil, args } -// isHelpFlag checks if a flag is a help flag -func isHelpFlag(flag string) bool { - return flag == "-h" || flag == "--help" -} - // normalizeCommandPath normalizes a command path for consistent lookup func normalizeCommandPath(path string) string { // Split by spaces, filter empty strings, and rejoin diff --git a/dispatcher_test.go b/dispatcher_test.go index cb18919..ba3e5de 100644 --- a/dispatcher_test.go +++ b/dispatcher_test.go @@ -1240,6 +1240,71 @@ func TestDispatcherNamespaceDiscovery(t *testing.T) { }) } +// TestDispatcherInterspersedValueFlagBeforeSection reproduces the regression where +// a value-taking global flag placed before a section-style command (a command with +// an empty flagset that returns ErrShowHelp) caused the resolver to reject the match +// and fall back to top-level help instead of the section's sub-commands. +func TestDispatcherInterspersedValueFlagBeforeSection(t *testing.T) { + // newDispatcher builds a dispatcher with a section-style "auth" command whose + // flagset declares no flags and whose Run returns ErrShowHelp, plus two + // sub-commands. A real value-taking flag "-C" is only declared on a sibling + // command, so it is unknown to the "auth" section's flagset. + newDispatcher := func() *Dispatcher { + d := NewDispatcher("myapp") + + authFs := NewFlagSet("auth") + authFs.AllowUnknownFlags(true) + d.Dispatch("auth", NewCommand(authFs, + func(fs *FlagSet, args []string) error { return ErrShowHelp }, + WithUsage("Authentication commands"))) + + d.Dispatch("auth generate", NewCommand(NewFlagSet("auth generate"), + func(fs *FlagSet, args []string) error { return nil }, + WithUsage("Generate auth config"))) + + d.Dispatch("auth provider", NewCommand(NewFlagSet("auth provider"), + func(fs *FlagSet, args []string) error { return nil }, + WithUsage("Manage identity providers"))) + + return d + } + + run := func(t *testing.T, args []string) string { + t.Helper() + old := os.Stdout + r, w, _ := os.Pipe() + os.Stdout = w + + err := newDispatcher().Execute(args) + + w.Close() + os.Stdout = old + + var buf bytes.Buffer + io.Copy(&buf, r) + + assert.NoError(t, err) + return buf.String() + } + + // Each of these must render the "auth" section's sub-commands, NOT top-level help. + cases := [][]string{ + {"-C", "prod", "auth", "help"}, + {"-C", "prod", "auth"}, + {"-C", "prod", "auth", "--help"}, + {"-C", "prod", "auth", "-h"}, + } + + for _, args := range cases { + t.Run(fmt.Sprintf("%v", args), func(t *testing.T) { + output := run(t, args) + assert.Contains(t, output, "Authentication commands") + assert.Contains(t, output, "generate") + assert.Contains(t, output, "provider") + }) + } +} + // TestDispatcherHelpShowsTypes tests that help output shows specific types for flags func TestDispatcherHelpShowsTypes(t *testing.T) { d := NewDispatcher("myapp") @@ -1773,4 +1838,3 @@ func TestHelpJSONIncludesGroup(t *testing.T) { json := string(data) assert.Contains(t, json, `"group": "Operator"`) } -