fix(cli): global flags visible, position-tolerant, and surface usage errors

cmd/ana/main.go

@@ -32,10 +32,13 @@ import (
 func main() {
 	stdio := cli.DefaultIO()
 	err := run(os.Args[1:], stdio, os.Getenv)
-	if err != nil && !errors.Is(err, cli.ErrUsage) && !errors.Is(err, cli.ErrHelp) {
-		// ErrUsage and ErrHelp mean help/usage text has already been emitted
-		// to the appropriate stream; any other error is a runtime failure
-		// that hasn't been reported yet, so surface it on stderr.
+	if err != nil && !errors.Is(err, cli.ErrHelp) && !errors.Is(err, cli.ErrReported) {
+		// ErrHelp means help text was already written to stdout; skip.
+		// ErrReported means the callee already wrote its diagnostic to
+		// stderr (Dispatch for bad globals / unknown commands, run() for
+		// unknown profiles). Every other error — including ErrUsage from
+		// leaves whose FlagSet output is io.Discard — still needs to
+		// surface, otherwise misplaced flags exit 1 with silent output.
 		fmt.Fprintln(stdio.Stderr, err)
 	}
 	os.Exit(cli.ExitCode(err))
@@ -56,8 +59,11 @@ func run(args []string, stdio cli.IO, env func(string) string) error {
 	// Parse global flags up front so the token-file/endpoint/profile
 	// overrides are available before we touch config on disk. cli.Dispatch
 	// re-parses globals, but doing it here lets us wire deps correctly
-	// before dispatch.
-	global, _, err := cli.ParseGlobal(args)
+	// before dispatch. StripGlobals (rather than ParseGlobal) so operators
+	// can place `--profile`/`--endpoint`/`--token-file` after the verb and
+	// still have config resolution honour them — ParseGlobal stops at the
+	// first positional, which would leave those flags invisible here.
+	global, _, err := cli.StripGlobals(args)
 	if err != nil {
 		// Don't return here — let Dispatch produce the canonical usage error
 		// (it prints root help + err to stderr). Fall through with zero
@@ -95,7 +101,7 @@ func run(args []string, stdio cli.IO, env func(string) string) error {
 		// by wrapping cli.ErrUsage.
 		if errors.Is(rerr, config.ErrUnknownProfile) {
 			fmt.Fprintf(stdio.Stderr, "ana: unknown profile %q\n", global.Profile)
-			return fmt.Errorf("%w: %s", cli.ErrUsage, rerr)
+			return errors.Join(fmt.Errorf("%w: %w", cli.ErrUsage, rerr), cli.ErrReported)
 		}
 		return rerr
 	}

cmd/ana/main_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -510,6 +511,31 @@ func TestRun_EndToEnd_ConnectorList(t *testing.T) {
 	}
 }
 
+// TestRun_LeafUsageErrorReturned asserts that an unknown flag on a leaf verb
+// surfaces as a non-nil ErrUsage-wrapped error whose message names the leaf
+// and the stdlib "flag provided but not defined" cause. main() then prints
+// the error text on stderr — previously it was swallowed because main skipped
+// any ErrUsage-wrapping error as "already reported", but leaf FlagSets use
+// io.Discard output so nothing had actually been reported.
+func TestRun_LeafUsageErrorReturned(t *testing.T) {
+	t.Parallel()
+	var out, errb bytes.Buffer
+	stdio := cli.IO{Stdin: strings.NewReader(""), Stdout: &out, Stderr: &errb, Env: func(string) string { return "" }, Now: time.Now}
+	err := run([]string{"org", "show", "--no-such-flag"}, stdio, func(string) string { return "" })
+	if err == nil {
+		t.Fatalf("want non-nil error")
+	}
+	if cli.ExitCode(err) != 1 {
+		t.Fatalf("exit code = %d, want 1 (err=%v)", cli.ExitCode(err), err)
+	}
+	if !strings.Contains(err.Error(), "flag provided but not defined") {
+		t.Errorf("err missing stdlib message: %v", err)
+	}
+	if !strings.Contains(err.Error(), "show") {
+		t.Errorf("err missing leaf name: %v", err)
+	}
+}
+
 // TestRun_UnknownProfile drives the ErrUnknownProfile branch in run: a
 // --profile pointing at a slot that doesn't exist (and no env fallback)
 // must print the canonical error to stderr and exit 1 via ErrUsage.
@@ -539,4 +565,11 @@ func TestRun_UnknownProfile(t *testing.T) {
 	if !strings.Contains(errb.String(), `unknown profile "ghost"`) {
 		t.Errorf("stderr missing message: %q", errb.String())
 	}
+	// run() writes the diagnostic itself and returns an ErrReported-wrapped
+	// err so main's fallback print is suppressed. Regression guard — if
+	// ErrReported leaks off the error, users would see the same message
+	// twice. Confirms the sentinel is actually in the chain.
+	if !errors.Is(err, cli.ErrReported) {
+		t.Errorf("err should carry cli.ErrReported to suppress main's duplicate print: %v", err)
+	}
 }

internal/cli/CLAUDE.md

@@ -6,9 +6,9 @@ Argument-dispatch core shared by every verb. Defines the `Command` interface, th
 
 - `cli.go` — `Command`, `IO`, `DefaultIO`, `Group` (nested-verb dispatcher with auto-generated help listing; optional `Flags` closure declares group-level flags that descend to every leaf via `WithAncestorFlags`), `dispatchChild` (the Group/Dispatch handoff that scans a resolved leaf's args for `--help`/`-h` and renders its `Help()` before `Run` is called — and if the leaf implements `Flagger`, appends a `Flags:` block enumerating own + ancestor flags via `renderFlagsAsText`; Groups are skipped so the flag reaches the deepest leaf, and bare positional `help` is left alone so leaves can receive it as an argument), `Flagger` (opt-in interface: leaves that implement `Flags(fs)` get the ancestor-aware `Flags:` block in `--help`), and `renderFlagsAsText` (sorted `--name <type>   usage (default: X)` enumeration). Precedence when ancestor and leaf declare the same name: **leaf wins**, because leaves call `ApplyAncestorFlags` AFTER declaring their own flags, and ancestor registrars use `DeclareString` / `DeclareBool` Lookup-guards that skip already-declared names (stdlib `flag.FlagSet.StringVar` panics on duplicate names, verified empirically).
 - `dispatch.go` — `Dispatch` (root entry: short-circuits help, parses globals, routes to the matching verb via `dispatchChild`) and `RootHelp`.
-- `root.go` — `Global` shape, `WithGlobal`/`GlobalFrom` context helpers (both require a non-nil ctx per stdlib `context.WithValue` convention — nil panics), `ParseGlobal` (strips known root flags from argv before verb dispatch), and the Phase 2 flag-registrar stack: `WithAncestorFlags(ctx, reg)` / `ApplyAncestorFlags(ctx, fs)` (context-carried slice of `func(*flag.FlagSet)` closures that `Group.Run` appends to and leaves replay on their own `FlagSet`), plus `DeclareString` / `DeclareBool` (Lookup-guarded wrappers ancestor closures use instead of raw `StringVar` / `BoolVar` to avoid the stdlib redeclaration panic when a leaf already declared the same name).
+- `root.go` — `Global` shape, `WithGlobal`/`GlobalFrom` context helpers (both require a non-nil ctx per stdlib `context.WithValue` convention — nil panics), `ParseGlobal` (stdlib-style front-anchored parse: stops at the first positional), and `StripGlobals` (position-tolerant: walks argv once, consumes known global flags wherever they appear, passes everything else through in order so the leaf's FlagSet reports unknown-flag errors). The two share the authoritative `globalFlagRegistry` list — `TestGlobalFlagsRegistrySync` enforces that the registry matches `ParseGlobal`'s FlagSet shape. `Dispatch` uses `StripGlobals`; `cmd/ana/main.go`'s early config-resolution pre-pass uses it too so `ana org show --profile prod` honours `--profile` even when it's placed after the verb. `globalFlagsHelp` renders the canonical `Global Flags:` block that both `RootHelp` and the leaf `--help` path append so `--json`/`--endpoint`/`--token-file`/`--profile` are discoverable from every help surface. Phase 2 flag-registrar stack: `WithAncestorFlags(ctx, reg)` / `ApplyAncestorFlags(ctx, fs)` (context-carried slice of `func(*flag.FlagSet)` closures that `Group.Run` appends to and leaves replay on their own `FlagSet`), plus `DeclareString` / `DeclareBool` (Lookup-guarded wrappers ancestor closures use instead of raw `StringVar` / `BoolVar` to avoid the stdlib redeclaration panic when a leaf already declared the same name).
 - `flags.go` — `ParseFlags`, which tolerates positional args interleaved with flags (stdlib `FlagSet.Parse` stops at the first non-flag, silently dropping later flags); `FlagWasSet`, the `fs.Visit` wrapper partial-update verbs use to tell "user left this alone" from "user explicitly passed the zero value"; `RequireFlags`, which emits a single sorted `missing required flags: --a, --b` usage error for any name not explicitly set on fs; and three typed `flag.Value` constructors — `EnumFlag` (allow-list validation at parse time), `IntListFlag` (CSV → `[]int` with whitespace tolerance), `SinceFlag` (accepts non-negative `time.ParseDuration` or RFC3339, stored UTC via an injected clock). The stdlib `flag.Parse` re-wraps `Set` errors with `%v`, so the `ErrUsage` chain survives only through the outer `ParseFlags` wrap — tests that exercise these helpers must go through `ParseFlags`, not bare `fs.Parse`.
 - `token.go` — `Token` (named `string` type whose `String`/`Format` always render the `RedactToken` mask, so any accidental `%s`/`%v`/`%+v`/`%#v`/`%q` on a logger or error can't leak a bearer token) and its `Value()` escape hatch. `config.Profile.Token` and `auth.Config.Token` are declared as `cli.Token`; the only authorized raw-emit call sites are `cmd/ana/main.go`'s `tokenFn` (Authorization header) and `auth/keys.go:emitPlaintextToken` (one-shot API key print). Since the underlying kind is `string`, `encoding/json` marshals/unmarshals transparently and untyped string literals still work in tests (`cli.Token("t")` — or just `"t"` in a `Token`-typed field).
 - `helpers.go` — shared verb-package helpers extracted from per-verb duplicates: `NewFlagSet`, `UsageErrf`, `WriteJSON`, `Remarshal`, `RenderOutput` (generic JSON-vs-typed-render dispatch: `--json` → `WriteJSON`; else `Remarshal` into `*T` then call the render closure), `RequireStringID`, `RequireIntID`, `RenderTwoCol`, `ReadToken` (shared login/profile-add stdin reader with JWT-sized buffer boost; trims surrounding whitespace), `ReadPassword` (same JWT-sized buffer but strips ONLY the trailing line terminator — passwords can legitimately begin or end with whitespace, so surrounding bytes must not be mutated), `NewTableWriter` (canonical tabwriter config), `FirstLine`, `DashIfEmpty` (table-cell rendering primitives; fall-through chains use stdlib `cmp.Or`), `RedactToken` (masks bearer tokens for human-readable echoes — `(unset)` or `********** (last 4: xxxx)`). Phase 0 of the shared-cli-kit refactor; Phases 1–10 migrate each verb package over and delete its local copies.
-- `errors.go` — `ErrUsage`, `ErrHelp`, and `ExitCode` (maps these plus `auth.ErrNotLoggedIn` to the process exit code).
+- `errors.go` — `ErrUsage`, `ErrHelp`, `ErrReported` (sentinel marking errors whose diagnostic text was already written to stderr by the callee — `Dispatch` wraps with it for bad globals / unknown commands, `cmd/ana/main.go`'s `run` wraps with it for unknown-profile; `main()` skips its fallback stderr print when the error chain contains this sentinel, so a leaf that needs main to print simply returns a non-wrapped `ErrUsage` like the `io.Discard` FlagSet path does), and `ExitCode` (maps these plus `auth.ErrNotLoggedIn` to the process exit code).
 - `cli_test.go`, `flags_test.go`, `helpers_test.go` — cover dispatch, help rendering, flag parsing, exit-code mapping, and every branch of the shared verb helpers. `//lint:ignore SA1012` directives mark the intentional nil-context panic assertions for `WithGlobal`/`GlobalFrom`.

internal/cli/cli.go

@@ -116,6 +116,8 @@ func dispatchChild(ctx context.Context, cmd Command, args []string, stdio IO) er
 						fmt.Fprint(stdio.Stdout, block)
 					}
 				}
+				fmt.Fprintln(stdio.Stdout)
+				fmt.Fprintln(stdio.Stdout, globalFlagsHelp())
 				return ErrHelp
 			}
 		}