diff --git a/cli/azd/cmd/auto_install.go b/cli/azd/cmd/auto_install.go index 937b9fb7ca0..3374834439d 100644 --- a/cli/azd/cmd/auto_install.go +++ b/cli/azd/cmd/auto_install.go @@ -16,7 +16,6 @@ import ( "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/internal/runcontext/agentdetect" "github.com/azure/azure-dev/cli/azd/internal/tracing/resource" - "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/azure/azure-dev/cli/azd/pkg/input" "github.com/azure/azure-dev/cli/azd/pkg/ioc" @@ -391,7 +390,7 @@ func ExecuteWithAutoInstall(ctx context.Context, rootContainer *ioc.NestedContai // This also enables the global options to be set in the container for support during extension framework callbacks. globalOpts := &internal.GlobalCommandOptions{} if err := ParseGlobalFlags(os.Args[1:], globalOpts); err != nil { - return fmt.Errorf("failed to parse global flags: %w", err) + return fmt.Errorf("Warning: failed to parse global flags: %w", err) } // Register GlobalCommandOptions as a singleton in the container BEFORE building the command tree. @@ -640,18 +639,6 @@ func CreateGlobalFlagSet() *pflag.FlagSet { func ParseGlobalFlags(args []string, opts *internal.GlobalCommandOptions) error { globalFlagSet := CreateGlobalFlagSet() - // Add the environment flag for early parsing. This is not in CreateGlobalFlagSet because - // it shouldn't be added to cobra's persistent flags (it's already registered per-command - // via EnvFlag.Bind). But we parse it here so GlobalCommandOptions.EnvironmentName is - // available for extension commands where DisableFlagParsing prevents cobra from parsing it. - // Guard against duplicate registration — pflag.StringP panics if the name already exists. - if globalFlagSet.Lookup(internal.EnvironmentNameFlagName) == nil { - globalFlagSet.StringP( - internal.EnvironmentNameFlagName, "e", - os.Getenv(environment.EnvNameEnvVarName), "", - ) - } - // Set output to io.Discard to suppress any error messages from pflag // Cobra will handle all user-facing output globalFlagSet.SetOutput(io.Discard) @@ -682,17 +669,6 @@ func ParseGlobalFlags(args []string, opts *internal.GlobalCommandOptions) error opts.NoPrompt = boolVal } - if strVal, err := globalFlagSet.GetString(internal.EnvironmentNameFlagName); err == nil { - if strVal != "" && !environment.IsValidEnvironmentName(strVal) { - return fmt.Errorf( - "invalid environment name '%s': must match %s", - strVal, - environment.EnvironmentNameRegexp.String(), - ) - } - opts.EnvironmentName = strVal - } - // Agent Detection: If --no-prompt was not explicitly set and we detect an AI coding agent // as the caller, automatically enable no-prompt mode for non-interactive execution. noPromptFlag := globalFlagSet.Lookup("no-prompt") diff --git a/cli/azd/cmd/auto_install_test.go b/cli/azd/cmd/auto_install_test.go index ec5be4431ca..e3295de9e84 100644 --- a/cli/azd/cmd/auto_install_test.go +++ b/cli/azd/cmd/auto_install_test.go @@ -9,7 +9,6 @@ import ( "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/internal/runcontext/agentdetect" - "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -422,128 +421,3 @@ func TestParseGlobalFlags_AgentDetection(t *testing.T) { }) } } - -func TestParseGlobalFlags_EnvironmentName(t *testing.T) { - tests := []struct { - name string - args []string - envVar string - expected string - }{ - { - name: "short flag -e", args: []string{"-e", "dev", "app", "run"}, - envVar: "", expected: "dev", - }, - { - name: "long flag --environment", - args: []string{"--environment", "staging", "app", "run"}, - envVar: "", expected: "staging", - }, - { - name: "long flag with equals", - args: []string{"--environment=prod", "app", "run"}, - envVar: "", expected: "prod", - }, - { - name: "no env flag", args: []string{"--debug", "app", "run"}, - envVar: "", expected: "", - }, - { - name: "env flag among other flags", - args: []string{"--debug", "-e", "dev", "--no-prompt"}, - envVar: "", expected: "dev", - }, - { - name: "env flag with unknown extension flags", - args: []string{"-e", "dev", "--foo", "bar"}, - envVar: "", expected: "dev", - }, - { - name: "AZURE_ENV_NAME fallback", - args: []string{"--debug", "app", "run"}, - envVar: "from-env", expected: "from-env", - }, - { - name: "-e flag overrides AZURE_ENV_NAME", - args: []string{"-e", "from-flag", "app", "run"}, - envVar: "from-env", - expected: "from-flag", - }, - { - name: "empty AZURE_ENV_NAME no effect", - args: []string{"app", "run"}, envVar: "", expected: "", - }, - { - name: "concatenated short flag -edev", - args: []string{"-edev", "app", "run"}, - envVar: "", expected: "dev", - }, - { - name: "multiple -e flags last wins", - args: []string{"-e", "first", "-e", "second"}, - envVar: "", expected: "second", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Setenv(environment.EnvNameEnvVarName, tt.envVar) - - opts := &internal.GlobalCommandOptions{} - err := ParseGlobalFlags(tt.args, opts) - require.NoError(t, err) - assert.Equal(t, tt.expected, opts.EnvironmentName) - }) - } - - // Cross-field assertion: verify -e does not interfere with adjacent flags - t.Run("env flag does not interfere with other flags", func(t *testing.T) { - t.Setenv(environment.EnvNameEnvVarName, "") - - opts := &internal.GlobalCommandOptions{} - err := ParseGlobalFlags( - []string{"--debug", "-e", "dev", "--no-prompt"}, - opts, - ) - require.NoError(t, err) - assert.Equal(t, "dev", opts.EnvironmentName) - assert.True(t, opts.EnableDebugLogging, - "--debug should be true alongside -e") - assert.True(t, opts.NoPrompt, - "--no-prompt should be true alongside -e") - }) - - // Edge case: -e at end of args with no value — pflag returns an error - t.Run("-e without value returns error", func(t *testing.T) { - t.Setenv(environment.EnvNameEnvVarName, "") - - opts := &internal.GlobalCommandOptions{} - err := ParseGlobalFlags([]string{"app", "-e"}, opts) - require.Error(t, err) - }) -} - -func TestParseGlobalFlags_InvalidEnvironmentName(t *testing.T) { - tests := []struct { - name string - args []string - }{ - { - name: "invalid characters", - args: []string{"-e", "env name with spaces"}, - }, - { - name: "special characters", - args: []string{"-e", "env@#$%"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - opts := &internal.GlobalCommandOptions{} - err := ParseGlobalFlags(tt.args, opts) - require.Error(t, err) - assert.Contains(t, err.Error(), "invalid environment name") - }) - } -} diff --git a/cli/azd/cmd/container.go b/cli/azd/cmd/container.go index 665f759528f..95cd937c48e 100644 --- a/cli/azd/cmd/container.go +++ b/cli/azd/cmd/container.go @@ -189,11 +189,7 @@ func registerCommonDependencies(container *ioc.NestedContainer) { return writer }) - container.MustRegisterScoped(func( - ctx context.Context, - cmd *cobra.Command, - globalOptions *internal.GlobalCommandOptions, - ) internal.EnvFlag { + container.MustRegisterScoped(func(ctx context.Context, cmd *cobra.Command) internal.EnvFlag { // The env flag `-e, --environment` is available on most azd commands but not all // This is typically used to override the default environment and is used for bootstrapping other components // such as the azd environment. @@ -209,24 +205,11 @@ func registerCommonDependencies(container *ioc.NestedContainer) { // If no explicit environment flag was set, but one was provided // in the context, use that instead. // This is used in workflow execution (in `up`) to influence the environment used. - // Context takes precedence over globalOptions because azd up uses - // context.WithValue to propagate the environment to sub-commands. if envFlag, ok := ctx.Value(envFlagCtxKey).(internal.EnvFlag); ok { return envFlag } } - // Fall back to the pre-parsed global options value. - // This handles extension commands (DisableFlagParsing: true) where cobra - // doesn't parse persistent flags — the value was already parsed in ParseGlobalFlags. - if envValue == "" && globalOptions.EnvironmentName != "" { - log.Printf( - "using pre-parsed environment name '%s' from global options", - globalOptions.EnvironmentName, - ) - envValue = globalOptions.EnvironmentName - } - return internal.EnvFlag{EnvironmentName: envValue} }) diff --git a/cli/azd/cmd/extensions.go b/cli/azd/cmd/extensions.go index dfc12f3dde0..32a241342f8 100644 --- a/cli/azd/cmd/extensions.go +++ b/cli/azd/cmd/extensions.go @@ -244,20 +244,22 @@ func (a *extensionAction) Run(ctx context.Context) (*actions.ActionResult, error allEnv = append(allEnv, traceEnv...) } - // Use pre-parsed global options for flag propagation. - // For extension commands (DisableFlagParsing: true), cobra doesn't parse persistent flags, - // but ParseGlobalFlags already parsed them into globalOptions before command execution. + // Read global flags for propagation via InvokeOptions + debugEnabled, _ := a.cmd.Flags().GetBool("debug") + cwd, _ := a.cmd.Flags().GetString("cwd") + envName, _ := a.cmd.Flags().GetString("environment") + options := &extensions.InvokeOptions{ Args: a.args, Env: allEnv, // cmd extensions are always interactive (connected to terminal) Interactive: true, - Debug: a.globalOptions.EnableDebugLogging, + Debug: debugEnabled, // Use globalOptions.NoPrompt which includes agent detection, // not just the --no-prompt CLI flag NoPrompt: a.globalOptions.NoPrompt, - Cwd: a.globalOptions.Cwd, - Environment: a.globalOptions.EnvironmentName, + Cwd: cwd, + Environment: envName, } _, invokeErr := a.extensionRunner.Invoke(ctx, extension, options) diff --git a/cli/azd/internal/global_command_options.go b/cli/azd/internal/global_command_options.go index 052dbe1216d..742fe421490 100644 --- a/cli/azd/internal/global_command_options.go +++ b/cli/azd/internal/global_command_options.go @@ -18,11 +18,6 @@ type GlobalCommandOptions struct { // if there is no default value the prompt returns an error. NoPrompt bool - // EnvironmentName is the name of the environment to use, set via `-e` or `--environment`. - // This is parsed early from raw args by ParseGlobalFlags so it is available even for - // commands with DisableFlagParsing: true (e.g. extension commands). - EnvironmentName string - // EnableTelemetry indicates if telemetry should be sent. // The rootCmd will disable this based if the environment variable // AZURE_DEV_COLLECT_TELEMETRY is set to 'no'. diff --git a/cli/azd/pkg/extensions/runner_test.go b/cli/azd/pkg/extensions/runner_test.go deleted file mode 100644 index 9ab0aa967a5..00000000000 --- a/cli/azd/pkg/extensions/runner_test.go +++ /dev/null @@ -1,139 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package extensions - -import ( - "context" - "os" - "path/filepath" - "testing" - - "github.com/azure/azure-dev/cli/azd/pkg/exec" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// mockCommandRunner captures the RunArgs passed to Run(). -type mockCommandRunner struct { - capturedArgs exec.RunArgs -} - -func (m *mockCommandRunner) Run(ctx context.Context, args exec.RunArgs) (exec.RunResult, error) { - m.capturedArgs = args - return exec.RunResult{}, nil -} - -func (m *mockCommandRunner) RunList(ctx context.Context, commands []string, args exec.RunArgs) (exec.RunResult, error) { - return exec.RunResult{}, nil -} - -func (m *mockCommandRunner) ToolInPath(name string) error { - return nil -} - -// TestRunnerInvoke_GlobalFlagPropagation verifies that InvokeOptions fields -// (Debug, NoPrompt, Cwd, Environment) are correctly propagated as AZD_* -// environment variables to the extension child process. -// -// This tests the critical mapping path: -// -// globalOptions.EnableDebugLogging → InvokeOptions.Debug → AZD_DEBUG=true -// globalOptions.NoPrompt → InvokeOptions.NoPrompt → AZD_NO_PROMPT=true -// globalOptions.Cwd → InvokeOptions.Cwd → AZD_CWD= -// globalOptions.EnvironmentName → InvokeOptions.Environment → AZD_ENVIRONMENT= -func TestRunnerInvoke_GlobalFlagPropagation(t *testing.T) { - // Create a temp file to act as the extension binary - tmpDir := t.TempDir() - extBin := filepath.Join(tmpDir, "test-ext") - require.NoError(t, os.WriteFile(extBin, []byte("#!/bin/sh\n"), 0o600)) //nolint:gosec - - // Point the user config dir to our temp dir so extensionPath resolves - t.Setenv("AZD_CONFIG_DIR", tmpDir) - - mock := &mockCommandRunner{} - runner := NewRunner(mock) - - ext := &Extension{ - Id: "test-ext", - Path: "test-ext", - } - - tests := []struct { - name string - options *InvokeOptions - expectEnvs map[string]string - absentEnvs []string - }{ - { - name: "all global flags set", - options: &InvokeOptions{ - Debug: true, - NoPrompt: true, - Cwd: "/custom/dir", - Environment: "dev", - }, - expectEnvs: map[string]string{ - "AZD_DEBUG": "true", - "AZD_NO_PROMPT": "true", - "AZD_CWD": "/custom/dir", - "AZD_ENVIRONMENT": "dev", - }, - }, - { - name: "only environment set", - options: &InvokeOptions{ - Environment: "staging", - }, - expectEnvs: map[string]string{ - "AZD_ENVIRONMENT": "staging", - }, - absentEnvs: []string{ - "AZD_DEBUG", - "AZD_NO_PROMPT", - "AZD_CWD", - }, - }, - { - name: "no global flags", - options: &InvokeOptions{}, - absentEnvs: []string{ - "AZD_DEBUG", - "AZD_NO_PROMPT", - "AZD_CWD", - "AZD_ENVIRONMENT", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := runner.Invoke(context.Background(), ext, tt.options) - require.NoError(t, err) - - envMap := make(map[string]string) - for _, e := range mock.capturedArgs.Env { - for i := 0; i < len(e); i++ { - if e[i] == '=' { - envMap[e[:i]] = e[i+1:] - break - } - } - } - - for key, want := range tt.expectEnvs { - got, ok := envMap[key] - assert.True(t, ok, - "expected env var %s to be set", key) - assert.Equal(t, want, got, - "env var %s", key) - } - - for _, key := range tt.absentEnvs { - _, ok := envMap[key] - assert.False(t, ok, - "expected env var %s to NOT be set", key) - } - }) - } -}