flag actioning order is not consistent

[G-Rath]

My urfave/cli version is

v3.7.0

Checklist

• Are you running the latest v3 release? The list of releases is here.
• Did you check the manual for your release? The v3 manual is here
• Did you perform a search about this problem? Here's the GitHub guide about searching.

Dependency Management

• My project is using go modules.
• My project is automatically downloading updating the latest version. (via Renovate)

Describe the bug

It appears that the order in which Flag.Action is called is not deterministic

To reproduce

package debug_test

import (
	"context"
	"fmt"
	"testing"

	"github.com/urfave/cli/v3"
)

func RunCli(args []string) string {
	str := ""

	cmd := &cli.Command{
		Flags:    []cli.Flag{},
		Action: func(_ context.Context, cmd *cli.Command) error {
			return nil
		},
	}

	for _, f := range []string{"a", "b", "c"} {
		cmd.Flags = append(cmd.Flags, &cli.BoolFlag{
			Name: f,
			Action: func(_ context.Context, _ *cli.Command, _ bool) error {
				str += f

				return nil
			},
		})
	}

	if err := cmd.Run(context.Background(), args); err != nil {
		fmt.Printf("%s\n", err)
	}

	return str
}

type Case struct {
	Name string
	Args []string
}

func Test_run(t *testing.T) {
	tests := []Case{
		{
			Name: "abc",
			Args: []string{"", "--a", "--b", "--c"},
		},
		{
			Name: "bca",
			Args: []string{"", "--b", "--c", "--a"},
		},
		{
			Name: "cba",
			Args: []string{"", "--c", "--b", "--a"},
		},
	}
	for _, tt := range tests {
		t.Run(tt.Name, func(t *testing.T) {
			str := RunCli(tt.Args)

			if str != "abc" {
				t.Errorf("expected 'abc' got '%s'", str)
			}
		})
	}
}

Observed behavior

❯ go test ./cmd/osv-scanner/debug/... -count 5
--- FAIL: Test_run (0.00s)
    --- FAIL: Test_run/bca (0.00s)
        debug_test.go:64: expected 'abc' got 'bca'
    --- FAIL: Test_run/cba (0.00s)
        debug_test.go:64: expected 'abc' got 'acb'
--- FAIL: Test_run (0.00s)
    --- FAIL: Test_run/bca (0.00s)
        debug_test.go:64: expected 'abc' got 'bca'
    --- FAIL: Test_run/cba (0.00s)
        debug_test.go:64: expected 'abc' got 'bac'
--- FAIL: Test_run (0.00s)
    --- FAIL: Test_run/cba (0.00s)
        debug_test.go:64: expected 'abc' got 'bac'
--- FAIL: Test_run (0.00s)
    --- FAIL: Test_run/abc (0.00s)
        debug_test.go:64: expected 'abc' got 'cab'
    --- FAIL: Test_run/bca (0.00s)
        debug_test.go:64: expected 'abc' got 'cab'
    --- FAIL: Test_run/cba (0.00s)
        debug_test.go:64: expected 'abc' got 'cba'
--- FAIL: Test_run (0.00s)
    --- FAIL: Test_run/abc (0.00s)
        debug_test.go:64: expected 'abc' got 'bca'
    --- FAIL: Test_run/bca (0.00s)
        debug_test.go:64: expected 'abc' got 'bca'
    --- FAIL: Test_run/cba (0.00s)
        debug_test.go:64: expected 'abc' got 'acb'
FAIL
FAIL    github.com/google/osv-scanner/v2/cmd/osv-scanner/debug  0.002s
FAIL

Expected behavior

Flag actions are invoked in a consistent order, ideally the order that they're defined in by the developer

Additional context

We found this in osv-scanner as our --format flag action changes our global logger to send everything to stderr for structured formats like json, and we deprecated a flag which logged a warning as part of its action. This introduced a race condition as depending on which action ran first, the warning would go to stdout or stderr.

I've been able to work around this by using Validator (incorrectly), but now am running into this issue with deprecating a large number of flags.

Ideally it would be preferred if flag actions were processed in the order they appear in the Flags slice for situations like the above to work, though it looks like currently flag actions are executed mostly in the order the flag appears which assumingly is because that is the order flags are parsed in.

Want to fix this yourself?

I could probably have a crack at it, once we align on what order to go with

Run go version and paste its output here

go version go1.26.0 linux/amd64

Run go env and paste its output here

AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE=''
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/home/jones/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/home/jones/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3540516710=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/home/jones/workspace/projects-oss/osv-scanner/go.mod'
GOMODCACHE='/home/jones/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/jones/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/jones/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.26.0'
GOWORK=''
PKG_CONFIG='pkg-config'

[G-Rath]

Trying to dig into this more, it looks like this is happening because "set flags" are stored in a map:

cli/command.go

Lines 147 to 148 in be9a4ba

	// flags that have been set
	setFlags map[Flag]struct{}

which gets iterated over when running actions:

cli/command.go

Lines 583 to 585 in be9a4ba

	func (cmd *Command) runFlagActions(ctx context.Context) error {
	tracef("runFlagActions")
	for fl := range cmd.setFlags {

and of course in Go map ranging is random so that explains this.

The easiest fix here that comes to mind would be to instead iterate over cmd.Flags and check if the flag is in setFlag to confirm if the flag action should actually be run - technically that'd mean more work especially if an app had a lot of flags, but I would be surprised if in practice that has a measurable performance impact