From 0948c225ad9efc6f3216bf04c2642676392d043c Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Wed, 25 Mar 2026 07:30:33 -0700 Subject: [PATCH 1/6] Add reserved global flags registry for extensions Introduce a formal registry of azd's global flags that extensions must not reuse. This prevents flag collisions like the -e conflict that caused #7271 where an extension's -e (--project-endpoint) shadowed azd's -e (--environment). The registry lists all global flags (-e, -C, -o, -h, --debug, --no-prompt, --docs, --trace-log-file, --trace-log-url) with lookup helpers that can be used for build-time validation and runtime conflict detection. Closes #7307 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/internal/reserved_flags.go | 77 ++++++++++++++++ cli/azd/internal/reserved_flags_test.go | 116 ++++++++++++++++++++++++ 2 files changed, 193 insertions(+) create mode 100644 cli/azd/internal/reserved_flags.go create mode 100644 cli/azd/internal/reserved_flags_test.go diff --git a/cli/azd/internal/reserved_flags.go b/cli/azd/internal/reserved_flags.go new file mode 100644 index 00000000000..2480ac13782 --- /dev/null +++ b/cli/azd/internal/reserved_flags.go @@ -0,0 +1,77 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package internal + +// ReservedFlag describes a global flag that is owned by azd and must not be reused +// by extensions for a different purpose. Extensions that register a flag with the +// same short or long name will shadow the global flag, causing unpredictable behavior +// when azd tries to parse the command line before dispatching to the extension. +type ReservedFlag struct { + // Long is the full flag name (e.g. "environment"). Always present. + Long string + // Short is the single-character alias (e.g. "e"). Empty when there is no short form. + Short string + // Description explains the flag's purpose in azd. + Description string +} + +// ReservedFlags is the canonical list of global flags that extensions must not reuse. +// It is derived from CreateGlobalFlagSet (auto_install.go), the root command's +// persistent flags, and the extension SDK's built-in flag set (extension_command.go). +// +// Keep this list in sync whenever a new global flag is added to azd. +var ReservedFlags = []ReservedFlag{ + {Long: "environment", Short: "e", Description: "The name of the environment to use."}, + {Long: "cwd", Short: "C", Description: "Sets the current working directory."}, + {Long: "debug", Short: "", Description: "Enables debugging and diagnostics logging."}, + {Long: "no-prompt", Short: "", Description: "Accepts the default value instead of prompting."}, + {Long: "output", Short: "o", Description: "The output format (json, table, none)."}, + {Long: "help", Short: "h", Description: "Help for the current command."}, + {Long: "docs", Short: "", Description: "Opens the documentation for the current command."}, + {Long: "trace-log-file", Short: "", Description: "Write a diagnostics trace to a file."}, + {Long: "trace-log-url", Short: "", Description: "Send traces to an Open Telemetry compatible endpoint."}, +} + +// reservedShortFlags is an index of short flag names built once at init time. +var reservedShortFlags map[string]ReservedFlag + +// reservedLongFlags is an index of long flag names built once at init time. +var reservedLongFlags map[string]ReservedFlag + +func init() { + reservedShortFlags = make(map[string]ReservedFlag, len(ReservedFlags)) + reservedLongFlags = make(map[string]ReservedFlag, len(ReservedFlags)) + for _, f := range ReservedFlags { + reservedLongFlags[f.Long] = f + if f.Short != "" { + reservedShortFlags[f.Short] = f + } + } +} + +// IsReservedShortFlag returns true when the given single-character flag name +// (without the leading "-") is reserved by azd as a global flag. +func IsReservedShortFlag(short string) bool { + _, ok := reservedShortFlags[short] + return ok +} + +// IsReservedLongFlag returns true when the given long flag name +// (without the leading "--") is reserved by azd as a global flag. +func IsReservedLongFlag(long string) bool { + _, ok := reservedLongFlags[long] + return ok +} + +// GetReservedShortFlag returns the ReservedFlag for the given short name, if any. +func GetReservedShortFlag(short string) (ReservedFlag, bool) { + f, ok := reservedShortFlags[short] + return f, ok +} + +// GetReservedLongFlag returns the ReservedFlag for the given long name, if any. +func GetReservedLongFlag(long string) (ReservedFlag, bool) { + f, ok := reservedLongFlags[long] + return f, ok +} diff --git a/cli/azd/internal/reserved_flags_test.go b/cli/azd/internal/reserved_flags_test.go new file mode 100644 index 00000000000..69629b9e663 --- /dev/null +++ b/cli/azd/internal/reserved_flags_test.go @@ -0,0 +1,116 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package internal + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestReservedFlagsRegistryPopulated(t *testing.T) { + // Sanity check: the registry should contain the known global flags. + require.GreaterOrEqual(t, len(ReservedFlags), 9, "expected at least 9 reserved flags") +} + +func TestIsReservedShortFlag(t *testing.T) { + tests := []struct { + short string + reserved bool + }{ + {"e", true}, + {"C", true}, + {"o", true}, + {"h", true}, + // Flags with no short form should NOT appear as short flags. + {"", false}, + // Arbitrary letters should not be reserved. + {"x", false}, + {"z", false}, + {"p", false}, + } + + for _, tt := range tests { + t.Run("short="+tt.short, func(t *testing.T) { + require.Equal(t, tt.reserved, IsReservedShortFlag(tt.short)) + }) + } +} + +func TestIsReservedLongFlag(t *testing.T) { + tests := []struct { + long string + reserved bool + }{ + {"environment", true}, + {"cwd", true}, + {"debug", true}, + {"no-prompt", true}, + {"output", true}, + {"help", true}, + {"docs", true}, + {"trace-log-file", true}, + {"trace-log-url", true}, + // Not reserved. + {"verbose", false}, + {"project-endpoint", false}, + {"", false}, + } + + for _, tt := range tests { + t.Run("long="+tt.long, func(t *testing.T) { + require.Equal(t, tt.reserved, IsReservedLongFlag(tt.long)) + }) + } +} + +func TestGetReservedShortFlag(t *testing.T) { + f, ok := GetReservedShortFlag("e") + require.True(t, ok) + require.Equal(t, "environment", f.Long) + require.Equal(t, "e", f.Short) + + f, ok = GetReservedShortFlag("C") + require.True(t, ok) + require.Equal(t, "cwd", f.Long) + + _, ok = GetReservedShortFlag("x") + require.False(t, ok) +} + +func TestGetReservedLongFlag(t *testing.T) { + f, ok := GetReservedLongFlag("debug") + require.True(t, ok) + require.Equal(t, "debug", f.Long) + require.Empty(t, f.Short, "debug has no short form") + + _, ok = GetReservedLongFlag("nonexistent") + require.False(t, ok) +} + +func TestReservedFlagsNoDuplicates(t *testing.T) { + seenLong := make(map[string]bool) + seenShort := make(map[string]bool) + + for _, f := range ReservedFlags { + require.NotEmpty(t, f.Long, "every reserved flag must have a long name") + require.False(t, seenLong[f.Long], "duplicate long flag: %s", f.Long) + seenLong[f.Long] = true + + if f.Short != "" { + require.False(t, seenShort[f.Short], "duplicate short flag: %s", f.Short) + seenShort[f.Short] = true + } + } +} + +func TestReservedFlagsConsistentWithLookups(t *testing.T) { + // Every entry in ReservedFlags must be findable via the lookup helpers. + for _, f := range ReservedFlags { + require.True(t, IsReservedLongFlag(f.Long), "long flag %q not found via IsReservedLongFlag", f.Long) + if f.Short != "" { + require.True(t, IsReservedShortFlag(f.Short), "short flag %q not found via IsReservedShortFlag", f.Short) + } + } +} From c9fdf47ff6b342f501b341e3eaaef4c7cd8c9f48 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Wed, 25 Mar 2026 10:34:11 -0700 Subject: [PATCH 2/6] Add SDK-level reserved flag enforcement in azdext.Run() ValidateNoReservedFlagConflicts() walks the extension command tree before execution and errors if any extension-defined flag collides with a reserved azd global flag (e.g. -e, -C, -o). This catches conflicts at extension development/test time, regardless of which repo the extension lives in. The SDK's own root persistent flags (--environment, --debug, etc.) are excluded from the check since they intentionally mirror azd globals. A sync test ensures the SDK and internal reserved flag lists stay aligned. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/azdext/reserved_flags.go | 181 ++++++++++++++++++++++ cli/azd/pkg/azdext/reserved_flags_test.go | 147 ++++++++++++++++++ cli/azd/pkg/azdext/run.go | 7 + 3 files changed, 335 insertions(+) create mode 100644 cli/azd/pkg/azdext/reserved_flags.go create mode 100644 cli/azd/pkg/azdext/reserved_flags_test.go diff --git a/cli/azd/pkg/azdext/reserved_flags.go b/cli/azd/pkg/azdext/reserved_flags.go new file mode 100644 index 00000000000..2a23adceb1f --- /dev/null +++ b/cli/azd/pkg/azdext/reserved_flags.go @@ -0,0 +1,181 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azdext + +import ( + "fmt" + "strings" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// reservedFlag describes a global flag owned by azd that extensions must not reuse +// for a different purpose. +type reservedFlag struct { + Long string + Short string +} + +// reservedGlobalFlags is the canonical list of global flags that extensions must not reuse. +// Keep this in sync with internal.ReservedFlags and CreateGlobalFlagSet (auto_install.go). +var reservedGlobalFlags = []reservedFlag{ + {Long: "environment", Short: "e"}, + {Long: "cwd", Short: "C"}, + {Long: "debug", Short: ""}, + {Long: "no-prompt", Short: ""}, + {Long: "output", Short: "o"}, + {Long: "help", Short: "h"}, + {Long: "docs", Short: ""}, + {Long: "trace-log-file", Short: ""}, + {Long: "trace-log-url", Short: ""}, +} + +// reservedShorts and reservedLongs are lookup maps built at init time. +var ( + reservedShorts map[string]string // short -> long + reservedLongs map[string]bool +) + +func init() { + reservedShorts = make(map[string]string, len(reservedGlobalFlags)) + reservedLongs = make(map[string]bool, len(reservedGlobalFlags)) + for _, f := range reservedGlobalFlags { + reservedLongs[f.Long] = true + if f.Short != "" { + reservedShorts[f.Short] = f.Long + } + } +} + +// ReservedFlagNames returns the long names of all reserved global flags. +// This is intended for documentation and error messages. +func ReservedFlagNames() []string { + names := make([]string, len(reservedGlobalFlags)) + for i, f := range reservedGlobalFlags { + names[i] = f.Long + } + return names +} + +// FlagConflict describes a single flag that conflicts with a reserved azd global flag. +type FlagConflict struct { + // Command is the full command path (e.g. "model custom create"). + Command string + // FlagName is the long name of the conflicting flag. + FlagName string + // FlagShort is the short name of the conflicting flag (may be empty). + FlagShort string + // ReservedLong is the long name of the reserved flag it conflicts with. + ReservedLong string + // Reason describes why it conflicts (e.g. "short flag -e is reserved"). + Reason string +} + +func (c FlagConflict) String() string { + return fmt.Sprintf("command %q: flag %s conflicts with reserved global flag --%s (%s)", + c.Command, c.flagDisplay(), c.ReservedLong, c.Reason) +} + +func (c FlagConflict) flagDisplay() string { + if c.FlagShort != "" { + return fmt.Sprintf("--%s/-%s", c.FlagName, c.FlagShort) + } + return fmt.Sprintf("--%s", c.FlagName) +} + +// ValidateNoReservedFlagConflicts walks the command tree rooted at cmd and +// returns an error listing every extension-defined flag that collides with an +// azd reserved global flag. +// +// Flags registered on the root command's persistent flag set are allowed because +// the extension SDK intentionally mirrors azd's global flags there (see +// NewExtensionRootCommand). Only flags added by the extension on subcommands +// (local or inherited persistent flags not from root) are checked. +func ValidateNoReservedFlagConflicts(root *cobra.Command) error { + conflicts := collectConflicts(root, root) + if len(conflicts) == 0 { + return nil + } + + var b strings.Builder + b.WriteString("extension defines flags that conflict with reserved azd global flags:\n") + for _, c := range conflicts { + b.WriteString(" - ") + b.WriteString(c.String()) + b.WriteString("\n") + } + b.WriteString("Remove or rename these flags to avoid conflicts with azd's global flags.\n") + b.WriteString("Reserved flags: ") + b.WriteString(strings.Join(ReservedFlagNames(), ", ")) + return fmt.Errorf("%s", b.String()) +} + +// collectConflicts recursively collects flag conflicts from the command tree. +func collectConflicts(root, cmd *cobra.Command) []FlagConflict { + var conflicts []FlagConflict + + // Check flags defined directly on this command (not inherited from parents). + // We use cmd.Flags() instead of cmd.LocalFlags() because LocalFlags() + // triggers cobra's mergePersistentFlags, which panics on shorthand conflicts. + cmd.Flags().VisitAll(func(f *pflag.Flag) { + // Skip flags that are defined on the root's persistent flag set. + // Those are the SDK-provided azd-compatible flags and are intentional. + if rootFlag := root.PersistentFlags().Lookup(f.Name); rootFlag != nil { + return + } + + if c, ok := checkFlag(cmd, f); ok { + conflicts = append(conflicts, c) + } + }) + + // Recurse into subcommands. + for _, sub := range cmd.Commands() { + conflicts = append(conflicts, collectConflicts(root, sub)...) + } + + return conflicts +} + +// checkFlag checks a single flag against the reserved lists. +func checkFlag(cmd *cobra.Command, f *pflag.Flag) (FlagConflict, bool) { + cmdPath := commandPath(cmd) + + // Check short flag collision. + if f.Shorthand != "" { + if reservedLong, ok := reservedShorts[f.Shorthand]; ok { + return FlagConflict{ + Command: cmdPath, + FlagName: f.Name, + FlagShort: f.Shorthand, + ReservedLong: reservedLong, + Reason: fmt.Sprintf("short flag -%s is reserved by azd for --%s", f.Shorthand, reservedLong), + }, true + } + } + + // Check long flag collision (only if the long name is the same but used for + // a different purpose — identified by being on a subcommand, not root). + if reservedLongs[f.Name] { + return FlagConflict{ + Command: cmdPath, + FlagName: f.Name, + FlagShort: f.Shorthand, + ReservedLong: f.Name, + Reason: fmt.Sprintf("long flag --%s is reserved by azd", f.Name), + }, true + } + + return FlagConflict{}, false +} + +// commandPath returns the space-separated command path (excluding root). +func commandPath(cmd *cobra.Command) string { + var parts []string + for c := cmd; c != nil && c.HasParent(); c = c.Parent() { + parts = append([]string{c.Name()}, parts...) + } + return strings.Join(parts, " ") +} diff --git a/cli/azd/pkg/azdext/reserved_flags_test.go b/cli/azd/pkg/azdext/reserved_flags_test.go new file mode 100644 index 00000000000..37b64e337fa --- /dev/null +++ b/cli/azd/pkg/azdext/reserved_flags_test.go @@ -0,0 +1,147 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package azdext + +import ( + "testing" + + "github.com/azure/azure-dev/cli/azd/internal" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +// newPlainRoot creates a plain cobra root command (no SDK persistent flags). +// This simulates an extension that doesn't use NewExtensionRootCommand. +func newPlainRoot(name string) *cobra.Command { + return &cobra.Command{Use: name} +} + +func TestValidateNoReservedFlagConflicts_Clean(t *testing.T) { + root, _ := NewExtensionRootCommand(ExtensionCommandOptions{Name: "test"}) + sub := &cobra.Command{Use: "do"} + sub.Flags().StringP("endpoint", "p", "", "project endpoint") + sub.Flags().String("subscription", "", "subscription id") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.NoError(t, err, "no conflict expected for flags that don't collide") +} + +func TestValidateNoReservedFlagConflicts_ShortCollision(t *testing.T) { + // Use a plain root because cobra panics if a child redefines a shorthand + // already claimed by a parent's persistent flags (the SDK root has -e). + root := newPlainRoot("test") + sub := &cobra.Command{Use: "create"} + sub.Flags().StringP("project-endpoint", "e", "", "project endpoint") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "short flag -e is reserved") + require.Contains(t, err.Error(), "project-endpoint") +} + +func TestValidateNoReservedFlagConflicts_LongCollision(t *testing.T) { + root := newPlainRoot("test") + sub := &cobra.Command{Use: "init"} + sub.Flags().StringP("environment", "n", "", "environment name or ID") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "long flag --environment is reserved") +} + +func TestValidateNoReservedFlagConflicts_RootFlagsAllowed(t *testing.T) { + // The SDK's own root persistent flags (--environment, --debug, --cwd, etc.) + // mirror azd globals intentionally and must be allowed. + root, _ := NewExtensionRootCommand(ExtensionCommandOptions{Name: "test"}) + sub := &cobra.Command{Use: "list"} + sub.Flags().String("filter", "", "filter expression") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.NoError(t, err, "root persistent flags from SDK should be allowed") +} + +func TestValidateNoReservedFlagConflicts_MultipleCollisions(t *testing.T) { + root := newPlainRoot("test") + sub := &cobra.Command{Use: "create"} + sub.Flags().StringP("project-endpoint", "e", "", "endpoint") + sub.Flags().StringP("output-format", "o", "", "output format") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "-e is reserved") + require.Contains(t, err.Error(), "-o is reserved") +} + +func TestValidateNoReservedFlagConflicts_NestedSubcommand(t *testing.T) { + root := newPlainRoot("test") + parent := &cobra.Command{Use: "model"} + child := &cobra.Command{Use: "create"} + child.Flags().StringP("project-endpoint", "e", "", "endpoint") + parent.AddCommand(child) + root.AddCommand(parent) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "model create") + require.Contains(t, err.Error(), "-e is reserved") +} + +func TestValidateNoReservedFlagConflicts_SDKRootWithCollision(t *testing.T) { + // When using the SDK root, -C is already registered as a persistent flag. + // A subcommand that uses -C for a different flag (via StringVar + Shorthand) + // would be caught. We use a long-name collision instead to avoid cobra panic. + root, _ := NewExtensionRootCommand(ExtensionCommandOptions{Name: "test"}) + sub := &cobra.Command{Use: "init"} + // --docs collides with reserved long flag + sub.Flags().Bool("docs", false, "generate docs") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "long flag --docs is reserved") +} + +func TestReservedFlagNames(t *testing.T) { + names := ReservedFlagNames() + require.NotEmpty(t, names) + require.Contains(t, names, "environment") + require.Contains(t, names, "cwd") + require.Contains(t, names, "debug") + require.Contains(t, names, "output") + require.Contains(t, names, "help") +} + +func TestReservedFlagsInSyncWithInternal(t *testing.T) { + // Verify the SDK reserved flag list stays in sync with the internal registry. + // If this test fails, you added a flag to one list but not the other. + sdkNames := ReservedFlagNames() + internalFlags := internal.ReservedFlags + + sdkSet := make(map[string]bool, len(sdkNames)) + for _, name := range sdkNames { + sdkSet[name] = true + } + + internalSet := make(map[string]bool, len(internalFlags)) + for _, f := range internalFlags { + internalSet[f.Long] = true + } + + for name := range sdkSet { + require.True(t, internalSet[name], + "azdext reserved_flags.go has %q but internal.ReservedFlags does not — add it to internal/reserved_flags.go", + name) + } + + for name := range internalSet { + require.True(t, sdkSet[name], + "internal.ReservedFlags has %q but azdext reserved_flags.go does not — add it to pkg/azdext/reserved_flags.go", + name) + } +} diff --git a/cli/azd/pkg/azdext/run.go b/cli/azd/pkg/azdext/run.go index f4509e37f67..c420fd24caa 100644 --- a/cli/azd/pkg/azdext/run.go +++ b/cli/azd/pkg/azdext/run.go @@ -61,6 +61,13 @@ func Run(rootCmd *cobra.Command, opts ...RunOption) { o(&cfg) } + // Validate that extension-defined flags do not collide with azd reserved global flags. + // This check runs before execution so extension developers see the error immediately. + if err := ValidateNoReservedFlagConflicts(rootCmd); err != nil { + printError(err) + os.Exit(1) + } + if cfg.preExecute != nil { if err := cfg.preExecute(ctx, rootCmd); err != nil { if reportErr := ReportError(ctx, err); reportErr != nil { From fbe2781c3557ecd385b08bfb039ed397524efae1 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Wed, 25 Mar 2026 11:02:32 -0700 Subject: [PATCH 3/6] docs: add extension flag architecture spec and style guide reserved flags section - Add docs/specs/extension-flags/spec.md: core architecture spec documenting how azd pre-parses global flags, propagates via env vars, and dispatches to extensions. Includes reserved flag registry and Adding a New Global Flag checklist. - Update cli/azd/docs/extensions/extensions-style-guide.md: add Reserved Global Flags section under Parameter and Flag Consistency with flag table, DO/DON'T guidance, collision error example, and link to core spec. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../docs/extensions/extensions-style-guide.md | 57 ++++++ docs/specs/extension-flags/spec.md | 188 ++++++++++++++++++ 2 files changed, 245 insertions(+) create mode 100644 docs/specs/extension-flags/spec.md diff --git a/cli/azd/docs/extensions/extensions-style-guide.md b/cli/azd/docs/extensions/extensions-style-guide.md index c478688fda8..4f438025713 100644 --- a/cli/azd/docs/extensions/extensions-style-guide.md +++ b/cli/azd/docs/extensions/extensions-style-guide.md @@ -17,6 +17,63 @@ This guide provides design guidelines and best practices for developing extensio - Reuse established parameter patterns across new commands - Maintain consistent naming conventions (e.g., `--subscription`, `--name`, `--type`) - Provide sensible defaults to reduce cognitive load +- **Do not reuse reserved global flag names** — see section below + +### Reserved Global Flags + +azd pre-parses a set of global flags from the command line **before** dispatching to extensions. +The extension SDK (`NewExtensionRootCommand`) also registers these same flags on every +extension's root command. Because both azd and the extension parse the same `argv`, extensions +**must not** register flags that collide with these reserved names. + +This is not a new restriction — it has been true since the extension system was designed. +The SDK now enforces it at startup via `ValidateNoReservedFlagConflicts()`. + +#### Reserved flag names + +| Long Name | Short | Purpose | +|-----------|-------|---------| +| `environment` | `e` | Selects the azd environment | +| `cwd` | `C` | Sets the working directory | +| `debug` | — | Enables debug logging | +| `no-prompt` | — | Non-interactive mode | +| `output` | `o` | Output format (json, table, none) | +| `help` | `h` | Command help (cobra built-in) | +| `docs` | — | Opens command documentation | +| `trace-log-file` | — | Diagnostics trace file | +| `trace-log-url` | — | OpenTelemetry trace endpoint | + +#### What this means for extension authors + +**DO:** +- Use any flag name that is not in the table above +- Use any single-letter short flag except `e`, `C`, `o`, `h` +- Access the environment name via `extCtx.Environment` (the SDK provides it automatically) +- Ignore reserved flags you don't need — the SDK handles them + +**DON'T:** +- Register `--environment` or `-e` on any subcommand (use `--env-name` or `--target-env` if you need a second environment reference) +- Register `--debug`, `--cwd`, `--output`, `--help`, or their short forms for a different purpose +- Assume you can "override" a global flag on a subcommand — azd's pre-parser will consume it first + +#### What happens if you collide + +If your extension uses the azd SDK (`azdext.Run()`), the SDK validates all flags at startup. +A collision produces a clear error: + +``` +extension defines flags that conflict with reserved azd global flags: + - command "custom create": flag --endpoint/-e conflicts with reserved global flag --environment + (short flag -e is reserved by azd for --environment) +Remove or rename these flags to avoid conflicts with azd's global flags. +Reserved flags: environment, cwd, debug, no-prompt, output, help, docs, trace-log-file, trace-log-url +``` + +#### Background + +For the full technical specification of how flags flow between azd and extensions, including +the pre-parsing pipeline, environment variable propagation, and enforcement implementation, +see [Extension Flag Architecture Spec](../specs/extension-flags/spec.md). ### 3. **Help and Discoverability** diff --git a/docs/specs/extension-flags/spec.md b/docs/specs/extension-flags/spec.md new file mode 100644 index 00000000000..0efc8f5470c --- /dev/null +++ b/docs/specs/extension-flags/spec.md @@ -0,0 +1,188 @@ +# Spec: Global Flags and Extension Flag Dispatch + +## Status + +**Documenting existing behavior** — this spec formalizes the flag contract that already exists in the azd + extension SDK implementation, and adds enforcement that was previously missing. + +## Goal + +Define how azd handles global flags when dispatching to extensions, including the pre-parsing pipeline, environment variable propagation, and the reserved flag registry that prevents namespace collisions. + +## Background + +azd extensions are standalone binaries that azd discovers, installs, and invokes as subcommands. When a user runs `azd model custom create --endpoint https://...`, azd: + +1. Pre-parses its own global flags from the full argument list +2. Launches the extension binary as a child process +3. Passes the raw arguments **and** global flag values (via environment variables) to the extension + +This creates a **shared flag namespace** — both azd and the extension parse the same `argv`. If an extension registers a flag that collides with an azd global flag (e.g., both use `-e`), azd's pre-parser consumes the value for its own purpose, and the extension either gets the wrong value or causes azd to error. + +Issue [#7271](https://github.com/Azure/azure-dev/issues/7271) demonstrated this: the `azd model` extension used `-e` for `--endpoint` (a URL), but azd's pre-parser treated the URL as an environment name and failed validation. + +## Architecture + +### Flag Flow Diagram + +``` +User runs: azd model custom create -e https://example.com/api + + ┌─────────────────────────────────────────────────────┐ + │ azd host process │ + │ │ + │ 1. ParseGlobalFlags(args) │ + │ - Reads: -e/--environment, --debug, --cwd, etc. │ + │ - UnknownFlags: true (ignores extension flags) │ + │ - Populates GlobalCommandOptions │ + │ │ + │ 2. extensions.go: DisableFlagParsing: true │ + │ - Cobra does NOT parse extension-specific flags │ + │ - Raw args passed through to extension │ + │ │ + │ 3. runner.go: Invoke() │ + │ - Converts GlobalCommandOptions → AZD_* env vars │ + │ - Launches extension binary with: │ + │ - Args: original argv (including -e value) │ + │ - Env: AZD_ENVIRONMENT, AZD_DEBUG, etc. │ + └──────────────────────┬──────────────────────────────┘ + │ + ▼ + ┌─────────────────────────────────────────────────────┐ + │ Extension binary (child process) │ + │ │ + │ 1. NewExtensionRootCommand() [SDK] │ + │ - Registers SAME global flags: │ + │ --environment/-e, --debug, --cwd/-C, etc. │ + │ - Falls back to AZD_* env vars if not on CLI │ + │ │ + │ 2. Extension-specific subcommands │ + │ - Register their OWN flags (--model, --version) │ + │ - Must NOT collide with reserved flags │ + └─────────────────────────────────────────────────────┘ +``` + +### Key Insight + +Both azd and the extension parse the **same arguments**. azd does not strip its global flags before passing args to extensions. This means: + +- If an extension reuses `-e` for `--endpoint`, azd's pre-parser sees `-e https://example.com/api` and tries to use the URL as an environment name +- The extension then also receives `-e https://example.com/api` in its args, but the SDK's root command binds `-e` to `--environment`, so the extension's own `-e` flag on a subcommand creates a conflict + +This is not a new restriction — it has been true since the extension system was designed. + +## Global Flags (Host Side) + +These flags are registered in `CreateGlobalFlagSet()` (`cmd/auto_install.go`) and pre-parsed by `ParseGlobalFlags()` before command dispatch: + +| Long Name | Short | Type | Default | Hidden | Description | +|-----------|-------|------|---------|--------|-------------| +| `environment` | `e` | string | `$AZURE_ENV_NAME` | No | The name of the environment to use | +| `cwd` | `C` | string | `""` | No | Sets the current working directory | +| `debug` | — | bool | `false` | No | Enables debugging and diagnostics logging | +| `no-prompt` | — | bool | `false` | No | Accepts default value instead of prompting | +| `output` | `o` | string | `"default"` | No | The output format (json, table, none) | +| `help` | `h` | — | — | No | Help for the current command (cobra built-in) | +| `docs` | — | — | — | No | Opens documentation for the current command | +| `trace-log-file` | — | string | `""` | Yes | Write a diagnostics trace to a file | +| `trace-log-url` | — | string | `""` | Yes | Send traces to an OpenTelemetry endpoint | + +### Pre-parsing Behavior + +`ParseGlobalFlags` uses `pflag.ParseErrorsAllowlist{UnknownFlags: true}` to silently ignore flags it doesn't recognize. This allows extension-specific flags (like `--model`, `--version`) to pass through without error. However, any flag that matches an azd global flag **will be consumed** by the pre-parser. + +## Global Flags (Extension SDK Side) + +The extension SDK's `NewExtensionRootCommand()` (`pkg/azdext/extension_command.go`) registers these persistent flags on every extension's root command: + +| Long Name | Short | Type | Default | Env Var Fallback | +|-----------|-------|------|---------|-----------------| +| `environment` | `e` | string | `""` | `AZD_ENVIRONMENT` | +| `cwd` | `C` | string | `""` | `AZD_CWD` | +| `debug` | — | bool | `false` | `AZD_DEBUG` | +| `no-prompt` | — | bool | `false` | `AZD_NO_PROMPT` | +| `output` | `o` | string | `"default"` | — | +| `trace-log-file` | — | string | `""` | — | +| `trace-log-url` | — | string | `""` | — | + +### Env Var Propagation + +azd passes global flag values to extensions via two mechanisms: + +1. **Environment variables** (`runner.go`): `AZD_DEBUG`, `AZD_NO_PROMPT`, `AZD_CWD`, `AZD_ENVIRONMENT` +2. **Raw args**: The original command-line arguments are passed through unchanged + +The SDK's `PersistentPreRunE` checks if each flag was explicitly set on the command line; if not, it falls back to the corresponding `AZD_*` environment variable. This dual-path design ensures global values are available whether the extension is invoked via azd or directly during development. + +## Reserved Flags + +### Definition + +A **reserved flag** is any flag that azd pre-parses from the command line before dispatching to extensions, or that the extension SDK registers on the root command. Extensions must not register flags with the same long name or short name on their subcommands. + +### The Reserved List + +The canonical list is maintained in two locations that are kept in sync by a test: + +- **Host side**: `internal/reserved_flags.go` — `ReservedFlags` slice with lookup helpers +- **SDK side**: `pkg/azdext/reserved_flags.go` — `reservedGlobalFlags` slice with validation + +| Long Name | Short | Reason Reserved | +|-----------|-------|----------------| +| `environment` | `e` | azd pre-parses for env selection; SDK registers on root | +| `cwd` | `C` | azd pre-parses for working directory; SDK registers on root | +| `debug` | — | azd pre-parses for debug mode; SDK registers on root | +| `no-prompt` | — | azd pre-parses for non-interactive mode; SDK registers on root | +| `output` | `o` | SDK registers on root for output format | +| `help` | `h` | cobra built-in; universal across all commands | +| `docs` | — | azd root command flag | +| `trace-log-file` | — | azd pre-parses for telemetry; SDK registers on root | +| `trace-log-url` | — | azd pre-parses for telemetry; SDK registers on root | + +## Enforcement + +### SDK-Level Validation + +`ValidateNoReservedFlagConflicts(root)` is called in `azdext.Run()` before command execution. It: + +1. Walks the entire command tree +2. For each command's flags, checks both long and short names against the reserved list +3. Skips flags on the root command's persistent flag set (those are the SDK-provided azd-compatible flags) +4. Returns a detailed error listing all conflicts with remediation guidance + +Any extension built with the azd SDK gets this check automatically — no opt-in required. + +### Sync Test + +A test (`TestReservedFlagNames_SyncWithInternal`) ensures the SDK-side and host-side reserved flag lists stay in sync. If a developer adds a new global flag to one list but not the other, the test fails. + +## Adding a New Global Flag + +When azd needs a new global flag: + +1. Add the flag to `CreateGlobalFlagSet()` in `cmd/auto_install.go` +2. Add parsing logic to `ParseGlobalFlags()` in `cmd/auto_install.go` +3. Add to `ReservedFlags` in `internal/reserved_flags.go` +4. Add to `reservedGlobalFlags` in `pkg/azdext/reserved_flags.go` +5. If it should be available to extensions: + - Register it in `NewExtensionRootCommand()` in `pkg/azdext/extension_command.go` + - Add env var propagation in `runner.go` + - Add env var fallback in `NewExtensionRootCommand`'s `PersistentPreRunE` +6. Run tests — the sync test will catch mismatches between steps 3 and 4 + +The reserved flag registry makes this process explicit and safe: any extension that happens to use the new flag name will get a clear error at startup instead of a mysterious runtime failure. + +## Implementation References + +| Component | File | +|-----------|------| +| Global flag registration | `cmd/auto_install.go` — `CreateGlobalFlagSet()` | +| Global flag pre-parsing | `cmd/auto_install.go` — `ParseGlobalFlags()` | +| Extension dispatch | `cmd/extensions.go` — `DisableFlagParsing: true` | +| InvokeOptions construction | `cmd/extensions.go` — global options propagation | +| Env var propagation | `pkg/extensions/runner.go` — `Invoke()` | +| SDK flag registration | `pkg/azdext/extension_command.go` — `NewExtensionRootCommand()` | +| SDK env var fallback | `pkg/azdext/extension_command.go` — `PersistentPreRunE` | +| Reserved flags (host) | `internal/reserved_flags.go` | +| Reserved flags (SDK) | `pkg/azdext/reserved_flags.go` | +| SDK enforcement | `pkg/azdext/reserved_flags.go` — `ValidateNoReservedFlagConflicts()` | +| Enforcement hook | `pkg/azdext/run.go` | From d8e389ebcf14a9452b5fdf0200fc89261b5056c6 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Wed, 25 Mar 2026 11:09:03 -0700 Subject: [PATCH 4/6] docs: move extension flag spec to cli/azd/docs/design/ Move spec from docs/specs/extension-flags/spec.md to cli/azd/docs/design/extension-flag-architecture.md to follow the existing convention for azd design docs. Update cross-reference in extensions style guide. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azd/docs/design/extension-flag-architecture.md | 0 cli/azd/docs/extensions/extensions-style-guide.md | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename docs/specs/extension-flags/spec.md => cli/azd/docs/design/extension-flag-architecture.md (100%) diff --git a/docs/specs/extension-flags/spec.md b/cli/azd/docs/design/extension-flag-architecture.md similarity index 100% rename from docs/specs/extension-flags/spec.md rename to cli/azd/docs/design/extension-flag-architecture.md diff --git a/cli/azd/docs/extensions/extensions-style-guide.md b/cli/azd/docs/extensions/extensions-style-guide.md index 4f438025713..58229e4479a 100644 --- a/cli/azd/docs/extensions/extensions-style-guide.md +++ b/cli/azd/docs/extensions/extensions-style-guide.md @@ -73,7 +73,7 @@ Reserved flags: environment, cwd, debug, no-prompt, output, help, docs, trace-lo For the full technical specification of how flags flow between azd and extensions, including the pre-parsing pipeline, environment variable propagation, and enforcement implementation, -see [Extension Flag Architecture Spec](../specs/extension-flags/spec.md). +see [Extension Flag Architecture Spec](../design/extension-flag-architecture.md). ### 3. **Help and Discoverability** From 0f9a128e6ee6e3aeb17c1ee2772962fe1cd64362 Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Wed, 25 Mar 2026 12:48:10 -0700 Subject: [PATCH 5/6] fix: make reserved flags slice unexported with getter, fix init ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Change ReservedFlags (exported slice) to reservedFlags (unexported) - Add ReservedFlags() function returning a defensive copy - Replace init() with func-literal var initializers to avoid ordering hazards - Fix 'Open Telemetry' → 'OpenTelemetry-compatible' spelling - Update test files and docs to use ReservedFlags() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../design/extension-flag-architecture.md | 4 +- cli/azd/internal/reserved_flags.go | 41 ++++++++++++------- cli/azd/internal/reserved_flags_test.go | 6 +-- cli/azd/pkg/azdext/reserved_flags_test.go | 6 +-- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/cli/azd/docs/design/extension-flag-architecture.md b/cli/azd/docs/design/extension-flag-architecture.md index 0efc8f5470c..d1d6c304b3c 100644 --- a/cli/azd/docs/design/extension-flag-architecture.md +++ b/cli/azd/docs/design/extension-flag-architecture.md @@ -123,7 +123,7 @@ A **reserved flag** is any flag that azd pre-parses from the command line before The canonical list is maintained in two locations that are kept in sync by a test: -- **Host side**: `internal/reserved_flags.go` — `ReservedFlags` slice with lookup helpers +- **Host side**: `internal/reserved_flags.go` — `reservedFlags` slice with `ReservedFlags()` getter and lookup helpers - **SDK side**: `pkg/azdext/reserved_flags.go` — `reservedGlobalFlags` slice with validation | Long Name | Short | Reason Reserved | @@ -161,7 +161,7 @@ When azd needs a new global flag: 1. Add the flag to `CreateGlobalFlagSet()` in `cmd/auto_install.go` 2. Add parsing logic to `ParseGlobalFlags()` in `cmd/auto_install.go` -3. Add to `ReservedFlags` in `internal/reserved_flags.go` +3. Add to `reservedFlags` in `internal/reserved_flags.go` 4. Add to `reservedGlobalFlags` in `pkg/azdext/reserved_flags.go` 5. If it should be available to extensions: - Register it in `NewExtensionRootCommand()` in `pkg/azdext/extension_command.go` diff --git a/cli/azd/internal/reserved_flags.go b/cli/azd/internal/reserved_flags.go index 2480ac13782..f7c3ca87d71 100644 --- a/cli/azd/internal/reserved_flags.go +++ b/cli/azd/internal/reserved_flags.go @@ -16,12 +16,12 @@ type ReservedFlag struct { Description string } -// ReservedFlags is the canonical list of global flags that extensions must not reuse. +// reservedFlags is the canonical list of global flags that extensions must not reuse. // It is derived from CreateGlobalFlagSet (auto_install.go), the root command's // persistent flags, and the extension SDK's built-in flag set (extension_command.go). // // Keep this list in sync whenever a new global flag is added to azd. -var ReservedFlags = []ReservedFlag{ +var reservedFlags = []ReservedFlag{ {Long: "environment", Short: "e", Description: "The name of the environment to use."}, {Long: "cwd", Short: "C", Description: "Sets the current working directory."}, {Long: "debug", Short: "", Description: "Enables debugging and diagnostics logging."}, @@ -30,25 +30,36 @@ var ReservedFlags = []ReservedFlag{ {Long: "help", Short: "h", Description: "Help for the current command."}, {Long: "docs", Short: "", Description: "Opens the documentation for the current command."}, {Long: "trace-log-file", Short: "", Description: "Write a diagnostics trace to a file."}, - {Long: "trace-log-url", Short: "", Description: "Send traces to an Open Telemetry compatible endpoint."}, + {Long: "trace-log-url", Short: "", Description: "Send traces to an OpenTelemetry-compatible endpoint."}, } -// reservedShortFlags is an index of short flag names built once at init time. -var reservedShortFlags map[string]ReservedFlag - -// reservedLongFlags is an index of long flag names built once at init time. -var reservedLongFlags map[string]ReservedFlag +// ReservedFlags returns a copy of the reserved flags list. +// The copy prevents callers from mutating the canonical list. +func ReservedFlags() []ReservedFlag { + out := make([]ReservedFlag, len(reservedFlags)) + copy(out, reservedFlags) + return out +} -func init() { - reservedShortFlags = make(map[string]ReservedFlag, len(ReservedFlags)) - reservedLongFlags = make(map[string]ReservedFlag, len(ReservedFlags)) - for _, f := range ReservedFlags { - reservedLongFlags[f.Long] = f +// reservedShortFlags is an index of short flag names built once at initialization time. +var reservedShortFlags = func() map[string]ReservedFlag { + m := make(map[string]ReservedFlag, len(reservedFlags)) + for _, f := range reservedFlags { if f.Short != "" { - reservedShortFlags[f.Short] = f + m[f.Short] = f } } -} + return m +}() + +// reservedLongFlags is an index of long flag names built once at initialization time. +var reservedLongFlags = func() map[string]ReservedFlag { + m := make(map[string]ReservedFlag, len(reservedFlags)) + for _, f := range reservedFlags { + m[f.Long] = f + } + return m +}() // IsReservedShortFlag returns true when the given single-character flag name // (without the leading "-") is reserved by azd as a global flag. diff --git a/cli/azd/internal/reserved_flags_test.go b/cli/azd/internal/reserved_flags_test.go index 69629b9e663..1e4972a5de0 100644 --- a/cli/azd/internal/reserved_flags_test.go +++ b/cli/azd/internal/reserved_flags_test.go @@ -11,7 +11,7 @@ import ( func TestReservedFlagsRegistryPopulated(t *testing.T) { // Sanity check: the registry should contain the known global flags. - require.GreaterOrEqual(t, len(ReservedFlags), 9, "expected at least 9 reserved flags") + require.GreaterOrEqual(t, len(ReservedFlags()), 9, "expected at least 9 reserved flags") } func TestIsReservedShortFlag(t *testing.T) { @@ -93,7 +93,7 @@ func TestReservedFlagsNoDuplicates(t *testing.T) { seenLong := make(map[string]bool) seenShort := make(map[string]bool) - for _, f := range ReservedFlags { + for _, f := range ReservedFlags() { require.NotEmpty(t, f.Long, "every reserved flag must have a long name") require.False(t, seenLong[f.Long], "duplicate long flag: %s", f.Long) seenLong[f.Long] = true @@ -107,7 +107,7 @@ func TestReservedFlagsNoDuplicates(t *testing.T) { func TestReservedFlagsConsistentWithLookups(t *testing.T) { // Every entry in ReservedFlags must be findable via the lookup helpers. - for _, f := range ReservedFlags { + for _, f := range ReservedFlags() { require.True(t, IsReservedLongFlag(f.Long), "long flag %q not found via IsReservedLongFlag", f.Long) if f.Short != "" { require.True(t, IsReservedShortFlag(f.Short), "short flag %q not found via IsReservedShortFlag", f.Short) diff --git a/cli/azd/pkg/azdext/reserved_flags_test.go b/cli/azd/pkg/azdext/reserved_flags_test.go index 37b64e337fa..6106846f653 100644 --- a/cli/azd/pkg/azdext/reserved_flags_test.go +++ b/cli/azd/pkg/azdext/reserved_flags_test.go @@ -121,7 +121,7 @@ func TestReservedFlagsInSyncWithInternal(t *testing.T) { // Verify the SDK reserved flag list stays in sync with the internal registry. // If this test fails, you added a flag to one list but not the other. sdkNames := ReservedFlagNames() - internalFlags := internal.ReservedFlags + internalFlags := internal.ReservedFlags() sdkSet := make(map[string]bool, len(sdkNames)) for _, name := range sdkNames { @@ -135,13 +135,13 @@ func TestReservedFlagsInSyncWithInternal(t *testing.T) { for name := range sdkSet { require.True(t, internalSet[name], - "azdext reserved_flags.go has %q but internal.ReservedFlags does not — add it to internal/reserved_flags.go", + "azdext reserved_flags.go has %q but internal.ReservedFlags() does not — add it to internal/reserved_flags.go", name) } for name := range internalSet { require.True(t, sdkSet[name], - "internal.ReservedFlags has %q but azdext reserved_flags.go does not — add it to pkg/azdext/reserved_flags.go", + "internal.ReservedFlags() has %q but azdext reserved_flags.go does not — add it to pkg/azdext/reserved_flags.go", name) } } From 517a3cc8791ebd9b28b0c66cc947b203ba42a75b Mon Sep 17 00:00:00 2001 From: Jon Gallant <2163001+jongio@users.noreply.github.com> Date: Thu, 26 Mar 2026 08:17:48 -0700 Subject: [PATCH 6/6] =?UTF-8?q?fix:=20address=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20spec=20accuracy,=20short-name=20sync,=20PersistentF?= =?UTF-8?q?lags=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix spec test name reference (TestReservedFlagsInSyncWithInternal) - Categorize host-side flags by registration mechanism (CreateGlobalFlagSet vs root persistent vs cobra built-in) - Add short-name parity assertion to sync test - Check cmd.PersistentFlags() in collectConflicts (not just cmd.Flags()) - Add PersistentFlagCollision test case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../design/extension-flag-architecture.md | 30 +++++++++--- cli/azd/pkg/azdext/reserved_flags.go | 26 ++++++++-- cli/azd/pkg/azdext/reserved_flags_test.go | 48 ++++++++++++++----- 3 files changed, 80 insertions(+), 24 deletions(-) diff --git a/cli/azd/docs/design/extension-flag-architecture.md b/cli/azd/docs/design/extension-flag-architecture.md index d1d6c304b3c..f356df63bbf 100644 --- a/cli/azd/docs/design/extension-flag-architecture.md +++ b/cli/azd/docs/design/extension-flag-architecture.md @@ -72,19 +72,35 @@ This is not a new restriction — it has been true since the extension system wa ## Global Flags (Host Side) -These flags are registered in `CreateGlobalFlagSet()` (`cmd/auto_install.go`) and pre-parsed by `ParseGlobalFlags()` before command dispatch: +azd's global flags are registered through several mechanisms. All of them are reserved — extensions must not reuse these names regardless of how they are registered. + +### Pre-parsed Global Flags + +These flags are registered in `CreateGlobalFlagSet()` (`cmd/auto_install.go`) and pre-parsed by `ParseGlobalFlags()` **before** command dispatch. They are available to middleware and DI resolution even for extension commands: | Long Name | Short | Type | Default | Hidden | Description | |-----------|-------|------|---------|--------|-------------| -| `environment` | `e` | string | `$AZURE_ENV_NAME` | No | The name of the environment to use | | `cwd` | `C` | string | `""` | No | Sets the current working directory | | `debug` | — | bool | `false` | No | Enables debugging and diagnostics logging | | `no-prompt` | — | bool | `false` | No | Accepts default value instead of prompting | -| `output` | `o` | string | `"default"` | No | The output format (json, table, none) | -| `help` | `h` | — | — | No | Help for the current command (cobra built-in) | -| `docs` | — | — | — | No | Opens documentation for the current command | | `trace-log-file` | — | string | `""` | Yes | Write a diagnostics trace to a file | -| `trace-log-url` | — | string | `""` | Yes | Send traces to an OpenTelemetry endpoint | +| `trace-log-url` | — | string | `""` | Yes | Send traces to an OpenTelemetry-compatible endpoint | + +### Root Command Persistent Flags + +These flags are registered on the root command's persistent flag set in `root.go` and are available to all subcommands: + +| Long Name | Short | Type | Default | Description | +|-----------|-------|------|---------|-------------| +| `environment` | `e` | string | `$AZURE_ENV_NAME` | The name of the environment to use | +| `output` | `o` | string | `"default"` | The output format (json, table, none) | + +### Cobra Built-in and Custom Command Flags + +| Long Name | Short | Description | +|-----------|-------|-------------| +| `help` | `h` | Help for the current command (cobra built-in) | +| `docs` | — | Opens documentation for the current command | ### Pre-parsing Behavior @@ -153,7 +169,7 @@ Any extension built with the azd SDK gets this check automatically — no opt-in ### Sync Test -A test (`TestReservedFlagNames_SyncWithInternal`) ensures the SDK-side and host-side reserved flag lists stay in sync. If a developer adds a new global flag to one list but not the other, the test fails. +A test (`TestReservedFlagsInSyncWithInternal`) ensures the SDK-side and host-side reserved flag lists stay in sync. If a developer adds a new global flag to one list but not the other, the test fails. ## Adding a New Global Flag diff --git a/cli/azd/pkg/azdext/reserved_flags.go b/cli/azd/pkg/azdext/reserved_flags.go index 2a23adceb1f..e8b60a58f1c 100644 --- a/cli/azd/pkg/azdext/reserved_flags.go +++ b/cli/azd/pkg/azdext/reserved_flags.go @@ -116,10 +116,16 @@ func ValidateNoReservedFlagConflicts(root *cobra.Command) error { func collectConflicts(root, cmd *cobra.Command) []FlagConflict { var conflicts []FlagConflict - // Check flags defined directly on this command (not inherited from parents). - // We use cmd.Flags() instead of cmd.LocalFlags() because LocalFlags() - // triggers cobra's mergePersistentFlags, which panics on shorthand conflicts. - cmd.Flags().VisitAll(func(f *pflag.Flag) { + // Track which flags we've already checked to avoid duplicate reports + // when a flag appears in both Flags() and PersistentFlags(). + checked := make(map[string]struct{}) + + checkFlagOnce := func(f *pflag.Flag) { + if _, seen := checked[f.Name]; seen { + return + } + checked[f.Name] = struct{}{} + // Skip flags that are defined on the root's persistent flag set. // Those are the SDK-provided azd-compatible flags and are intentional. if rootFlag := root.PersistentFlags().Lookup(f.Name); rootFlag != nil { @@ -129,7 +135,17 @@ func collectConflicts(root, cmd *cobra.Command) []FlagConflict { if c, ok := checkFlag(cmd, f); ok { conflicts = append(conflicts, c) } - }) + } + + // Check flags defined directly on this command (not inherited from parents). + // We use cmd.Flags() instead of cmd.LocalFlags() because LocalFlags() + // triggers cobra's mergePersistentFlags, which panics on shorthand conflicts. + cmd.Flags().VisitAll(checkFlagOnce) + + // Also check persistent flags defined on this command. This catches cases where + // an extension defines a persistent flag (e.g. on a subcommand) that conflicts + // with a reserved flag but wouldn't appear in cmd.Flags(). + cmd.PersistentFlags().VisitAll(checkFlagOnce) // Recurse into subcommands. for _, sub := range cmd.Commands() { diff --git a/cli/azd/pkg/azdext/reserved_flags_test.go b/cli/azd/pkg/azdext/reserved_flags_test.go index 6106846f653..049283c4007 100644 --- a/cli/azd/pkg/azdext/reserved_flags_test.go +++ b/cli/azd/pkg/azdext/reserved_flags_test.go @@ -107,6 +107,19 @@ func TestValidateNoReservedFlagConflicts_SDKRootWithCollision(t *testing.T) { require.Contains(t, err.Error(), "long flag --docs is reserved") } +func TestValidateNoReservedFlagConflicts_PersistentFlagCollision(t *testing.T) { + // A subcommand that defines a persistent flag colliding with a reserved name + // should be caught even though it uses PersistentFlags() not Flags(). + root := newPlainRoot("test") + sub := &cobra.Command{Use: "deploy"} + sub.PersistentFlags().Bool("debug", false, "extension debug mode") + root.AddCommand(sub) + + err := ValidateNoReservedFlagConflicts(root) + require.Error(t, err) + require.Contains(t, err.Error(), "long flag --debug is reserved") +} + func TestReservedFlagNames(t *testing.T) { names := ReservedFlagNames() require.NotEmpty(t, names) @@ -120,28 +133,39 @@ func TestReservedFlagNames(t *testing.T) { func TestReservedFlagsInSyncWithInternal(t *testing.T) { // Verify the SDK reserved flag list stays in sync with the internal registry. // If this test fails, you added a flag to one list but not the other. - sdkNames := ReservedFlagNames() + sdkFlags := reservedGlobalFlags internalFlags := internal.ReservedFlags() - sdkSet := make(map[string]bool, len(sdkNames)) - for _, name := range sdkNames { - sdkSet[name] = true + // Build maps of long name -> short name for both sides. + sdkMap := make(map[string]string, len(sdkFlags)) + for _, f := range sdkFlags { + sdkMap[f.Long] = f.Short } - internalSet := make(map[string]bool, len(internalFlags)) + internalMap := make(map[string]string, len(internalFlags)) for _, f := range internalFlags { - internalSet[f.Long] = true + internalMap[f.Long] = f.Short } - for name := range sdkSet { - require.True(t, internalSet[name], + // Check every SDK flag exists in internal with matching short name. + for long, short := range sdkMap { + internalShort, ok := internalMap[long] + require.True(t, ok, "azdext reserved_flags.go has %q but internal.ReservedFlags() does not — add it to internal/reserved_flags.go", - name) + long) + require.Equal(t, internalShort, short, + "short name mismatch for %q: internal has %q, SDK has %q", + long, internalShort, short) } - for name := range internalSet { - require.True(t, sdkSet[name], + // Check every internal flag exists in SDK with matching short name. + for long, short := range internalMap { + sdkShort, ok := sdkMap[long] + require.True(t, ok, "internal.ReservedFlags() has %q but azdext reserved_flags.go does not — add it to pkg/azdext/reserved_flags.go", - name) + long) + require.Equal(t, sdkShort, short, + "short name mismatch for %q: SDK has %q, internal has %q", + long, sdkShort, short) } }