From 6a9f70a8db9380387323db8c15e8453a29e4d461 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 19:32:52 +0000 Subject: [PATCH 01/11] feat: add tenant picker before subscription prompt for multi-tenant users When users have access to multiple Azure tenants, the subscription list now shows a tenant selection step first to scope down the list. This addresses the UX issue where users with many tenants see an overwhelming list of subscriptions from all tenants. Key behaviors: - Single tenant: skipped, goes straight to subscription selection - Multiple tenants: shows picker with display names + "All tenants" option - AZURE_TENANT_ID env var: pre-filters subscriptions in both prompt and no-prompt modes - No-prompt mode: tenant picker skipped but env var filtering still applies Both PromptSubscription paths (DefaultPrompter and promptService) are updated so extensions get the tenant picker automatically via gRPC. Fixes #2993 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/account/manager.go | 6 + cli/azd/pkg/account/subscriptions_manager.go | 22 ++ cli/azd/pkg/prompt/prompt_service.go | 71 +++- cli/azd/pkg/prompt/prompter.go | 122 ++++--- cli/azd/pkg/prompt/prompter_extra_test.go | 34 +- cli/azd/pkg/prompt/prompter_test.go | 83 ++--- cli/azd/pkg/prompt/tenant_helper.go | 180 +++++++++++ cli/azd/pkg/prompt/tenant_helper_test.go | 305 ++++++++++++++++++ .../test/mocks/mockaccount/mock_manager.go | 15 + .../mocks/mockaccount/mock_subscriptions.go | 5 + 10 files changed, 711 insertions(+), 132 deletions(-) create mode 100644 cli/azd/pkg/prompt/tenant_helper.go create mode 100644 cli/azd/pkg/prompt/tenant_helper_test.go diff --git a/cli/azd/pkg/account/manager.go b/cli/azd/pkg/account/manager.go index dbee60ea52a..28ec3d7143d 100644 --- a/cli/azd/pkg/account/manager.go +++ b/cli/azd/pkg/account/manager.go @@ -37,6 +37,7 @@ type Manager interface { GetSubscriptions(ctx context.Context) ([]Subscription, error) GetSubscriptionsWithDefaultSet(ctx context.Context) ([]Subscription, error) GetLocations(ctx context.Context, subscriptionId string) ([]Location, error) + GetTenantDisplayNames(ctx context.Context) (map[string]string, error) SetDefaultSubscription(ctx context.Context, subscriptionId string) (*Subscription, error) SetDefaultLocation(ctx context.Context, subscriptionId string, location string) (*Location, error) } @@ -140,6 +141,11 @@ func (m *manager) GetSubscriptions(ctx context.Context) ([]Subscription, error) return m.subManager.GetSubscriptions(ctx) } +// GetTenantDisplayNames returns a map of tenant ID to display name. +func (m *manager) GetTenantDisplayNames(ctx context.Context) (map[string]string, error) { + return m.subManager.GetTenantDisplayNames(ctx) +} + // Gets the available Azure locations for the specified Azure subscription. func (m *manager) GetLocations(ctx context.Context, subscriptionId string) ([]Location, error) { locations, err := m.subManager.ListLocations(ctx, subscriptionId) diff --git a/cli/azd/pkg/account/subscriptions_manager.go b/cli/azd/pkg/account/subscriptions_manager.go index de5685d0a9f..1a0cda5848d 100644 --- a/cli/azd/pkg/account/subscriptions_manager.go +++ b/cli/azd/pkg/account/subscriptions_manager.go @@ -408,6 +408,28 @@ func (m *SubscriptionsManager) getSubscription(ctx context.Context, subscription return &sub, nil } +// GetTenantDisplayNames returns a map of tenant ID to display name for all tenants +// accessible by the current account. +func (m *SubscriptionsManager) GetTenantDisplayNames(ctx context.Context) (map[string]string, error) { + tenants, err := m.service.ListTenants(ctx) + if err != nil { + return nil, fmt.Errorf("listing tenants: %w", err) + } + + result := make(map[string]string, len(tenants)) + for _, t := range tenants { + if t.TenantID != nil { + name := *t.TenantID + if t.DisplayName != nil && *t.DisplayName != "" { + name = *t.DisplayName + } + result[*t.TenantID] = name + } + } + + return result, nil +} + func toSubscriptions(azSubs []*armsubscriptions.Subscription, userAccessTenantId string) []Subscription { if azSubs == nil { return nil diff --git a/cli/azd/pkg/prompt/prompt_service.go b/cli/azd/pkg/prompt/prompt_service.go index a1d25b0c18c..8a6c5febaf4 100644 --- a/cli/azd/pkg/prompt/prompt_service.go +++ b/cli/azd/pkg/prompt/prompt_service.go @@ -7,6 +7,7 @@ import ( "context" "fmt" "io" + "log" "os" "slices" "strconv" @@ -157,6 +158,7 @@ type ResourceService interface { type SubscriptionManager interface { GetSubscriptions(ctx context.Context) ([]account.Subscription, error) GetLocations(ctx context.Context, subscriptionId string) ([]account.Location, error) + GetTenantDisplayNames(ctx context.Context) (map[string]string, error) } // PromptServiceInterface defines the methods that the PromptService must implement. @@ -211,6 +213,8 @@ func NewPromptService( } // PromptSubscription prompts the user to select an Azure subscription. +// If the user has access to multiple tenants, a tenant selection prompt is shown first +// to scope down the subscription list. func (ps *promptService) PromptSubscription( ctx context.Context, selectorOptions *SelectOptions, @@ -235,6 +239,30 @@ func (ps *promptService) PromptSubscription( return nil, err } + // Load subscriptions under a spinner first + var subscriptionList []account.Subscription + loadingSpinner := ux.NewSpinner(&ux.SpinnerOptions{ + Text: mergedOptions.LoadingMessage, + }) + + err := loadingSpinner.Run(ctx, func(ctx context.Context) error { + var loadErr error + subscriptionList, loadErr = ps.subscriptionManager.GetSubscriptions(ctx) + return loadErr + }) + if err != nil { + return nil, err + } + + // Apply tenant filtering (after spinner is done so the prompt doesn't overlap) + subscriptionList = filterByTenantEnvVar(subscriptionList) + if !ps.console.IsNoPromptMode() { + subscriptionList, err = ps.promptAndFilterByTenant(ctx, subscriptionList) + if err != nil { + return nil, err + } + } + // Get default subscription from user config var defaultSubscriptionId = "" userConfig, err := ps.userConfigManager.Load() @@ -247,19 +275,15 @@ func (ps *promptService) PromptSubscription( hideId := isDemoModeEnabled() + // Use PromptCustomResource with pre-loaded data (no LoadData spinner needed) + subscriptions := make([]*account.Subscription, len(subscriptionList)) + for i, subscription := range subscriptionList { + subscriptions[i] = &subscription + } + return PromptCustomResource(ctx, CustomResourceOptions[account.Subscription]{ SelectorOptions: mergedOptions, LoadData: func(ctx context.Context) ([]*account.Subscription, error) { - subscriptionList, err := ps.subscriptionManager.GetSubscriptions(ctx) - if err != nil { - return nil, err - } - - subscriptions := make([]*account.Subscription, len(subscriptionList)) - for i, subscription := range subscriptionList { - subscriptions[i] = &subscription - } - return subscriptions, nil }, DisplayResource: func(subscription *account.Subscription) (string, error) { @@ -271,6 +295,33 @@ func (ps *promptService) PromptSubscription( }) } +// promptAndFilterByTenant prompts the user to select a tenant when subscriptions span multiple tenants. +func (ps *promptService) promptAndFilterByTenant( + ctx context.Context, + subscriptions []account.Subscription, +) ([]account.Subscription, error) { + tenants := extractUniqueTenants(subscriptions, nil) + if len(tenants) <= 1 { + return subscriptions, nil + } + + // Only fetch tenant display names when we actually need to prompt + tenantNames, err := ps.subscriptionManager.GetTenantDisplayNames(ctx) + if err != nil { + log.Printf("failed to fetch tenant display names, using tenant IDs: %v", err) + tenantNames = map[string]string{} + } + + tenants = extractUniqueTenants(subscriptions, tenantNames) + + selectedTenantId, err := promptTenantSelection(ctx, ps.console, tenants, "") + if err != nil { + return nil, err + } + + return filterSubscriptionsByTenant(subscriptions, selectedTenantId), nil +} + // PromptLocation prompts the user to select an Azure location. func (ps *promptService) PromptLocation( ctx context.Context, diff --git a/cli/azd/pkg/prompt/prompter.go b/cli/azd/pkg/prompt/prompter.go index 6fb8c241486..a6cd916dfcf 100644 --- a/cli/azd/pkg/prompt/prompter.go +++ b/cli/azd/pkg/prompt/prompter.go @@ -71,12 +71,12 @@ func NewDefaultPrompter( } func (p *DefaultPrompter) PromptSubscription(ctx context.Context, msg string) (subscriptionId string, err error) { - subscriptionOptions, subscriptions, defaultSubscription, err := p.getSubscriptionOptions(ctx) + subscriptionInfos, err := p.accountManager.GetSubscriptions(ctx) if err != nil { - return "", err + return "", fmt.Errorf("listing accounts: %w", err) } - if len(subscriptionOptions) == 0 { + if len(subscriptionInfos) == 0 { // NOTE: Error text must contain "no subscriptions found" to match the // pattern in error_suggestions.yaml. Update both if rewording. return "", errors.New(heredoc.Docf( @@ -87,6 +87,31 @@ func (p *DefaultPrompter) PromptSubscription(ctx context.Context, msg string) (s )) } + // Filter by AZURE_TENANT_ID if set (works in both prompt and no-prompt modes) + subscriptionInfos = filterByTenantEnvVar(subscriptionInfos) + + // Tenant selection: if multiple tenants, prompt user to pick one + if !p.console.IsNoPromptMode() { + subscriptionInfos, err = p.promptAndFilterByTenant(ctx, subscriptionInfos) + if err != nil { + return "", err + } + } + + slices.SortFunc(subscriptionInfos, func(a, b account.Subscription) int { + return stringutil.CompareLower(a.Name, b.Name) + }) + + // The default value is based on AZURE_SUBSCRIPTION_ID, falling back to whatever default subscription in + // set in azd's config. + defaultSubscriptionId := os.Getenv(environment.SubscriptionIdEnvVarName) + if defaultSubscriptionId == "" { + defaultSubscriptionId = p.accountManager.GetDefaultSubscriptionID(ctx) + } + + subscriptionOptions, subscriptions, defaultSubscription := + formatSubscriptionOptions(subscriptionInfos, defaultSubscriptionId) + for subscriptionId == "" { subscriptionSelectionIndex, err := p.console.Select(ctx, input.ConsoleOptions{ Message: msg, @@ -110,6 +135,59 @@ func (p *DefaultPrompter) PromptSubscription(ctx context.Context, msg string) (s return subscriptionId, nil } +// promptAndFilterByTenant prompts the user to select a tenant when subscriptions span multiple tenants. +func (p *DefaultPrompter) promptAndFilterByTenant( + ctx context.Context, + subscriptions []account.Subscription, +) ([]account.Subscription, error) { + // Quick check without display names to avoid unnecessary API call + tenants := extractUniqueTenants(subscriptions, nil) + if len(tenants) <= 1 { + return subscriptions, nil + } + + // Only fetch tenant display names when we actually need to prompt + tenantNames, err := p.accountManager.GetTenantDisplayNames(ctx) + if err != nil { + log.Printf("failed to fetch tenant display names, using tenant IDs: %v", err) + tenantNames = map[string]string{} + } + + tenants = extractUniqueTenants(subscriptions, tenantNames) + + selectedTenantId, err := promptTenantSelection(ctx, p.console, tenants, "") + if err != nil { + return nil, err + } + + return filterSubscriptionsByTenant(subscriptions, selectedTenantId), nil +} + +// formatSubscriptionOptions formats subscription infos into display options. +func formatSubscriptionOptions( + subscriptionInfos []account.Subscription, + defaultSubscriptionId string, +) (options []string, ids []string, defaultOption any) { + options = make([]string, len(subscriptionInfos)) + ids = make([]string, len(subscriptionInfos)) + + for index, info := range subscriptionInfos { + if v, err := strconv.ParseBool(os.Getenv("AZD_DEMO_MODE")); err == nil && v { + options[index] = fmt.Sprintf("%2d. %s", index+1, info.Name) + } else { + options[index] = fmt.Sprintf("%2d. %s (%s)", index+1, info.Name, info.Id) + } + + ids[index] = info.Id + + if info.Id == defaultSubscriptionId { + defaultOption = options[index] + } + } + + return options, ids, defaultOption +} + func (p *DefaultPrompter) PromptLocation( ctx context.Context, subId string, @@ -246,44 +324,6 @@ func (p *DefaultPrompter) PromptResourceGroupFrom( return name, nil } -func (p *DefaultPrompter) getSubscriptionOptions(ctx context.Context) ([]string, []string, any, error) { - subscriptionInfos, err := p.accountManager.GetSubscriptions(ctx) - if err != nil { - return nil, nil, nil, fmt.Errorf("listing accounts: %w", err) - } - - slices.SortFunc(subscriptionInfos, func(a, b account.Subscription) int { - return stringutil.CompareLower(a.Name, b.Name) - }) - - // The default value is based on AZURE_SUBSCRIPTION_ID, falling back to whatever default subscription in - // set in azd's config. - defaultSubscriptionId := os.Getenv(environment.SubscriptionIdEnvVarName) - if defaultSubscriptionId == "" { - defaultSubscriptionId = p.accountManager.GetDefaultSubscriptionID(ctx) - } - - var subscriptionOptions = make([]string, len(subscriptionInfos)) - var subscriptions = make([]string, len(subscriptionInfos)) - var defaultSubscription any - - for index, info := range subscriptionInfos { - if v, err := strconv.ParseBool(os.Getenv("AZD_DEMO_MODE")); err == nil && v { - subscriptionOptions[index] = fmt.Sprintf("%2d. %s", index+1, info.Name) - } else { - subscriptionOptions[index] = fmt.Sprintf("%2d. %s (%s)", index+1, info.Name, info.Id) - } - - subscriptions[index] = info.Id - - if info.Id == defaultSubscriptionId { - defaultSubscription = subscriptionOptions[index] - } - } - - return subscriptionOptions, subscriptions, defaultSubscription, nil -} - func (p *DefaultPrompter) IsNoPromptMode() bool { return p.console.IsNoPromptMode() } diff --git a/cli/azd/pkg/prompt/prompter_extra_test.go b/cli/azd/pkg/prompt/prompter_extra_test.go index 2d3d0de0a2e..a65cb300934 100644 --- a/cli/azd/pkg/prompt/prompter_extra_test.go +++ b/cli/azd/pkg/prompt/prompter_extra_test.go @@ -49,8 +49,8 @@ func TestDefaultPrompter_PromptSubscription_HappyPath(t *testing.T) { mockAccount := &mockaccount.MockAccountManager{ Subscriptions: []account.Subscription{ - {Id: "sub-alpha", Name: "Alpha", TenantId: "tenant-1"}, - {Id: "sub-bravo", Name: "Bravo", TenantId: "tenant-2"}, + {Id: "sub-alpha", Name: "Alpha", TenantId: "tenant-1", UserAccessTenantId: "tenant-1"}, + {Id: "sub-bravo", Name: "Bravo", TenantId: "tenant-1", UserAccessTenantId: "tenant-1"}, }, } p, mockCtx := newTestPrompter(t, mockAccount) @@ -224,18 +224,14 @@ func TestDefaultPrompter_PromptLocation_WithDefaultSelectedLocation(t *testing.T require.Contains(t, defaultValue.(string), "West US") } -func TestDefaultPrompter_GetSubscriptionOptions_DemoMode(t *testing.T) { +func TestDefaultPrompter_FormatSubscriptionOptions_DemoMode(t *testing.T) { t.Setenv("AZD_DEMO_MODE", "true") - mockAccount := &mockaccount.MockAccountManager{ - Subscriptions: []account.Subscription{ - {Id: "sub-secret", Name: "Display Only"}, - }, + subscriptions := []account.Subscription{ + {Id: "sub-secret", Name: "Display Only"}, } - p, _ := newTestPrompter(t, mockAccount) - opts, subs, _, err := p.getSubscriptionOptions(t.Context()) - require.NoError(t, err) + opts, subs, _ := formatSubscriptionOptions(subscriptions, "") require.Len(t, opts, 1) require.Len(t, subs, 1) // In demo mode, id must not be exposed. @@ -243,20 +239,14 @@ func TestDefaultPrompter_GetSubscriptionOptions_DemoMode(t *testing.T) { require.Contains(t, opts[0], "Display Only") } -func TestDefaultPrompter_GetSubscriptionOptions_EnvVarDefault(t *testing.T) { - t.Setenv(environment.SubscriptionIdEnvVarName, "sub-env") - - mockAccount := &mockaccount.MockAccountManager{ - DefaultSubscription: "sub-config", // env var takes precedence - Subscriptions: []account.Subscription{ - {Id: "sub-env", Name: "From Env"}, - {Id: "sub-config", Name: "From Config"}, - }, +func TestDefaultPrompter_FormatSubscriptionOptions_EnvVarDefault(t *testing.T) { + subscriptions := []account.Subscription{ + {Id: "sub-env", Name: "From Env"}, + {Id: "sub-config", Name: "From Config"}, } - p, _ := newTestPrompter(t, mockAccount) - _, _, def, err := p.getSubscriptionOptions(t.Context()) - require.NoError(t, err) + // env var default takes precedence + _, _, def := formatSubscriptionOptions(subscriptions, "sub-env") require.NotNil(t, def) require.Contains(t, def.(string), "From Env") } diff --git a/cli/azd/pkg/prompt/prompter_test.go b/cli/azd/pkg/prompt/prompter_test.go index 5e59f5e2a27..cb4c118ba44 100644 --- a/cli/azd/pkg/prompt/prompter_test.go +++ b/cli/azd/pkg/prompt/prompter_test.go @@ -7,84 +7,49 @@ import ( "testing" "github.com/azure/azure-dev/cli/azd/pkg/account" - "github.com/azure/azure-dev/cli/azd/pkg/azapi" - "github.com/azure/azure-dev/cli/azd/pkg/cloud" - "github.com/azure/azure-dev/cli/azd/pkg/environment" - "github.com/azure/azure-dev/cli/azd/test/mocks" - "github.com/azure/azure-dev/cli/azd/test/mocks/mockaccount" "github.com/stretchr/testify/require" ) -func Test_getSubscriptionOptions(t *testing.T) { +func Test_formatSubscriptionOptions(t *testing.T) { t.Run("no default config set", func(t *testing.T) { - mockContext := mocks.NewMockContext(t.Context()) - env := environment.New("test") - resourceService := azapi.NewResourceService(mockContext.SubscriptionCredentialProvider, mockContext.ArmClientOptions) - mockAccount := &mockaccount.MockAccountManager{ - Subscriptions: []account.Subscription{ - { - Id: "1", - Name: "sub1", - TenantId: "", - UserAccessTenantId: "", - IsDefault: false, - }, + subscriptions := []account.Subscription{ + { + Id: "1", + Name: "sub1", + TenantId: "", + UserAccessTenantId: "", + IsDefault: false, }, } - prompter := NewDefaultPrompter( - env, - mockContext.Console, - mockAccount, - resourceService, - cloud.AzurePublic(), - ).(*DefaultPrompter) - subList, subs, result, err := prompter.getSubscriptionOptions(*mockContext.Context) + subList, subs, result := formatSubscriptionOptions(subscriptions, "") - require.Nil(t, err) require.EqualValues(t, 1, len(subList)) require.EqualValues(t, 1, len(subs)) require.EqualValues(t, nil, result) }) t.Run("default value set", func(t *testing.T) { - // mocked config defaultSubId := "SUBSCRIPTION_DEFAULT" - mockContext := mocks.NewMockContext(t.Context()) - env := environment.New("test") - resourceService := azapi.NewResourceService(mockContext.SubscriptionCredentialProvider, mockContext.ArmClientOptions) - mockAccount := &mockaccount.MockAccountManager{ - DefaultLocation: "location", - DefaultSubscription: defaultSubId, - Subscriptions: []account.Subscription{ - { - Id: defaultSubId, - Name: "DISPLAY DEFAULT", - TenantId: "TENANT", - UserAccessTenantId: "USER_TENANT", - IsDefault: true, - }, - { - Id: "SUBSCRIPTION_OTHER", - Name: "DISPLAY OTHER", - TenantId: "TENANT", - UserAccessTenantId: "USER_TENANT", - IsDefault: false, - }, + subscriptions := []account.Subscription{ + { + Id: defaultSubId, + Name: "DISPLAY DEFAULT", + TenantId: "TENANT", + UserAccessTenantId: "USER_TENANT", + IsDefault: true, + }, + { + Id: "SUBSCRIPTION_OTHER", + Name: "DISPLAY OTHER", + TenantId: "TENANT", + UserAccessTenantId: "USER_TENANT", + IsDefault: false, }, - Locations: []account.Location{}, } - prompter := NewDefaultPrompter( - env, - mockContext.Console, - mockAccount, - resourceService, - cloud.AzurePublic(), - ).(*DefaultPrompter) - subList, subs, result, err := prompter.getSubscriptionOptions(*mockContext.Context) + subList, subs, result := formatSubscriptionOptions(subscriptions, defaultSubId) - require.Nil(t, err) require.EqualValues(t, 2, len(subList)) require.EqualValues(t, 2, len(subs)) require.NotNil(t, result) diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go new file mode 100644 index 00000000000..5b16401c564 --- /dev/null +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -0,0 +1,180 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package prompt + +import ( + "cmp" + "context" + "fmt" + "os" + "slices" + "strings" + + "github.com/azure/azure-dev/cli/azd/pkg/account" + "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/input" + "github.com/azure/azure-dev/cli/azd/pkg/output" +) + +// TenantInfo holds display metadata for a tenant extracted from the subscription list. +type TenantInfo struct { + // Id is the tenant ID (GUID). + Id string + // DisplayName is the friendly name of the tenant, or the ID if no name is available. + DisplayName string + // SubscriptionCount is the number of subscriptions accessible via this tenant. + SubscriptionCount int +} + +// extractUniqueTenants extracts unique tenants from a list of subscriptions, +// grouped by UserAccessTenantId. The returned list is sorted by DisplayName. +// Tenant display names are resolved from the provided tenantDisplayNames map; +// if a tenant ID is not in the map, the ID itself is used as the display name. +func extractUniqueTenants( + subscriptions []account.Subscription, + tenantDisplayNames map[string]string, +) []TenantInfo { + tenantMap := make(map[string]*TenantInfo) + + for _, sub := range subscriptions { + tid := sub.UserAccessTenantId + if tid == "" { + tid = sub.TenantId + } + + if info, exists := tenantMap[tid]; exists { + info.SubscriptionCount++ + } else { + displayName := tid + if name, ok := tenantDisplayNames[tid]; ok && name != "" { + displayName = name + } + tenantMap[tid] = &TenantInfo{ + Id: tid, + DisplayName: displayName, + SubscriptionCount: 1, + } + } + } + + tenants := make([]TenantInfo, 0, len(tenantMap)) + for _, info := range tenantMap { + tenants = append(tenants, *info) + } + + slices.SortFunc(tenants, func(a, b TenantInfo) int { + return cmp.Compare( + strings.ToLower(a.DisplayName), + strings.ToLower(b.DisplayName), + ) + }) + + return tenants +} + +// filterSubscriptionsByTenant filters subscriptions to only those accessible +// through the specified tenant ID. If tenantId is empty, all subscriptions are returned. +func filterSubscriptionsByTenant( + subscriptions []account.Subscription, + tenantId string, +) []account.Subscription { + if tenantId == "" { + return subscriptions + } + + return slices.DeleteFunc(slices.Clone(subscriptions), func(sub account.Subscription) bool { + accessTenant := sub.UserAccessTenantId + if accessTenant == "" { + accessTenant = sub.TenantId + } + return accessTenant != tenantId + }) +} + +// filterByTenantEnvVar filters subscriptions by AZURE_TENANT_ID if set. +// This is applied in both prompt and no-prompt modes to ensure env var is authoritative. +func filterByTenantEnvVar(subscriptions []account.Subscription) []account.Subscription { + tenantId := os.Getenv(environment.TenantIdEnvVarName) + if tenantId == "" { + return subscriptions + } + + filtered := filterSubscriptionsByTenant(subscriptions, tenantId) + // If filtering produces no results, fall back to showing all subscriptions + // rather than erroring out — the tenant ID may be stale + if len(filtered) == 0 { + return subscriptions + } + + return filtered +} + +// promptTenantSelection prompts the user to select a tenant when multiple tenants are available. +// Returns the selected tenant ID, or empty string if the user chose "All tenants". +// If there is only one tenant, it is returned automatically without prompting. +func promptTenantSelection( + ctx context.Context, + console input.Console, + tenants []TenantInfo, + preSelectedTenantId string, +) (string, error) { + if len(tenants) <= 1 { + if len(tenants) == 1 { + return tenants[0].Id, nil + } + return "", nil + } + + // If a tenant is already pre-selected (e.g. from AZURE_TENANT_ID), use it directly + if preSelectedTenantId != "" { + for _, t := range tenants { + if t.Id == preSelectedTenantId { + return t.Id, nil + } + } + // Pre-selected tenant not found in available tenants; fall through to prompt + } + + allTenantsLabel := fmt.Sprintf( + "%2d. All tenants", + len(tenants)+1, + ) + + options := make([]string, len(tenants)+1) + for i, t := range tenants { + options[i] = formatTenantOption(i+1, t) + } + options[len(tenants)] = allTenantsLabel + + selectedIndex, err := console.Select(ctx, input.ConsoleOptions{ + Message: "Select a tenant", + Options: options, + }) + if err != nil { + return "", fmt.Errorf("selecting tenant: %w", err) + } + + // Last option = "All tenants" + if selectedIndex == len(tenants) { + return "", nil + } + + return tenants[selectedIndex].Id, nil +} + +func formatTenantOption(index int, t TenantInfo) string { + subCountLabel := fmt.Sprintf( + "%d subscription", t.SubscriptionCount, + ) + if t.SubscriptionCount != 1 { + subCountLabel += "s" + } + + return fmt.Sprintf( + "%2d. %s %s", + index, + t.DisplayName, + output.WithGrayFormat("(%s)", subCountLabel), + ) +} diff --git a/cli/azd/pkg/prompt/tenant_helper_test.go b/cli/azd/pkg/prompt/tenant_helper_test.go new file mode 100644 index 00000000000..a11feecd928 --- /dev/null +++ b/cli/azd/pkg/prompt/tenant_helper_test.go @@ -0,0 +1,305 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package prompt + +import ( + "strings" + "testing" + + "github.com/azure/azure-dev/cli/azd/pkg/account" + "github.com/azure/azure-dev/cli/azd/pkg/azapi" + "github.com/azure/azure-dev/cli/azd/pkg/cloud" + "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/input" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/azure/azure-dev/cli/azd/test/mocks/mockaccount" + "github.com/stretchr/testify/require" +) + +func TestExtractUniqueTenants_Empty(t *testing.T) { + tenants := extractUniqueTenants(nil, nil) + require.Empty(t, tenants) +} + +func TestExtractUniqueTenants_SingleTenant(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", UserAccessTenantId: "tid-1"}, + } + + tenants := extractUniqueTenants(subs, map[string]string{"tid-1": "Contoso"}) + require.Len(t, tenants, 1) + require.Equal(t, "tid-1", tenants[0].Id) + require.Equal(t, "Contoso", tenants[0].DisplayName) + require.Equal(t, 2, tenants[0].SubscriptionCount) +} + +func TestExtractUniqueTenants_MultipleTenants(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", UserAccessTenantId: "tid-2"}, + {Id: "sub-3", UserAccessTenantId: "tid-1"}, + } + + names := map[string]string{ + "tid-1": "Contoso", + "tid-2": "Fabrikam", + } + + tenants := extractUniqueTenants(subs, names) + require.Len(t, tenants, 2) + // Sorted alphabetically by display name + require.Equal(t, "Contoso", tenants[0].DisplayName) + require.Equal(t, 2, tenants[0].SubscriptionCount) + require.Equal(t, "Fabrikam", tenants[1].DisplayName) + require.Equal(t, 1, tenants[1].SubscriptionCount) +} + +func TestExtractUniqueTenants_FallbackToTenantId(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", TenantId: "tid-1", UserAccessTenantId: ""}, + } + + tenants := extractUniqueTenants(subs, nil) + require.Len(t, tenants, 1) + require.Equal(t, "tid-1", tenants[0].Id) + // Display name falls back to the ID when no names provided + require.Equal(t, "tid-1", tenants[0].DisplayName) +} + +func TestExtractUniqueTenants_NoDisplayNames(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", UserAccessTenantId: "tid-2"}, + } + + tenants := extractUniqueTenants(subs, nil) + require.Len(t, tenants, 2) + require.Equal(t, "tid-1", tenants[0].DisplayName) + require.Equal(t, "tid-2", tenants[1].DisplayName) +} + +func TestFilterSubscriptionsByTenant_EmptyTenantId(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", UserAccessTenantId: "tid-2"}, + } + + result := filterSubscriptionsByTenant(subs, "") + require.Len(t, result, 2) +} + +func TestFilterSubscriptionsByTenant_Filtered(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", UserAccessTenantId: "tid-2"}, + {Id: "sub-3", UserAccessTenantId: "tid-1"}, + } + + result := filterSubscriptionsByTenant(subs, "tid-1") + require.Len(t, result, 2) + require.Equal(t, "sub-1", result[0].Id) + require.Equal(t, "sub-3", result[1].Id) +} + +func TestFilterSubscriptionsByTenant_NoMatch(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + } + + result := filterSubscriptionsByTenant(subs, "tid-unknown") + require.Empty(t, result) +} + +func TestFilterByTenantEnvVar_NotSet(t *testing.T) { + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", UserAccessTenantId: "tid-2"}, + } + + result := filterByTenantEnvVar(subs) + require.Len(t, result, 2) +} + +func TestFilterByTenantEnvVar_Set(t *testing.T) { + t.Setenv("AZURE_TENANT_ID", "tid-1") + + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", UserAccessTenantId: "tid-2"}, + } + + result := filterByTenantEnvVar(subs) + require.Len(t, result, 1) + require.Equal(t, "sub-1", result[0].Id) +} + +func TestFilterByTenantEnvVar_NoMatchFallsBack(t *testing.T) { + t.Setenv("AZURE_TENANT_ID", "tid-unknown") + + subs := []account.Subscription{ + {Id: "sub-1", UserAccessTenantId: "tid-1"}, + } + + // Falls back to showing all when the env var doesn't match + result := filterByTenantEnvVar(subs) + require.Len(t, result, 1) +} + +func TestPromptTenantSelection_SingleTenant(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + + tenants := []TenantInfo{ + {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, + } + + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "") + require.NoError(t, err) + require.Equal(t, "tid-1", selected) +} + +func TestPromptTenantSelection_MultipleTenants_SelectFirst(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a tenant") + }).Respond(0) // pick first tenant + + tenants := []TenantInfo{ + {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, + {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, + } + + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "") + require.NoError(t, err) + require.Equal(t, "tid-1", selected) +} + +func TestPromptTenantSelection_MultipleTenants_SelectAllTenants(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a tenant") + }).Respond(2) // pick "All tenants" (third option with 2 tenants) + + tenants := []TenantInfo{ + {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, + {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, + } + + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "") + require.NoError(t, err) + require.Empty(t, selected) // empty string = all tenants +} + +func TestPromptTenantSelection_PreSelected(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + + tenants := []TenantInfo{ + {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, + {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, + } + + // Pre-selected tenant skips the prompt entirely + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "tid-2") + require.NoError(t, err) + require.Equal(t, "tid-2", selected) +} + +func TestPromptTenantSelection_PreSelectedNotFound(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a tenant") + }).Respond(0) + + tenants := []TenantInfo{ + {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, + {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, + } + + // Pre-selected tenant not found, falls through to prompt + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "tid-unknown") + require.NoError(t, err) + require.Equal(t, "tid-1", selected) +} + +func TestPromptTenantSelection_NoTenants(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + + selected, err := promptTenantSelection(t.Context(), mockContext.Console, nil, "") + require.NoError(t, err) + require.Empty(t, selected) +} + +func TestPromptSubscription_MultiTenant_TenantPickerShown(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + mockAccount := &mockaccount.MockAccountManager{ + Subscriptions: []account.Subscription{ + {Id: "sub-1", Name: "Alpha", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", Name: "Bravo", UserAccessTenantId: "tid-2"}, + {Id: "sub-3", Name: "Charlie", UserAccessTenantId: "tid-1"}, + }, + } + + p, _ := newTestPrompterWithCtx(t, mockAccount, mockContext) + + // First prompt: select tenant (pick tid-1) + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a tenant") + }).Respond(0) // first tenant + + // Second prompt: select subscription from filtered list + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a subscription") + }).Respond(1) // pick second in filtered list (Charlie after Alpha alphabetically) + + subId, err := p.PromptSubscription(t.Context(), "Select a subscription") + require.NoError(t, err) + // After filtering to tid-1: Alpha (sub-1) and Charlie (sub-3), sorted + require.Equal(t, "sub-3", subId) // Charlie is second alphabetically +} + +func TestPromptSubscription_MultiTenant_AllTenantsOption(t *testing.T) { + mockContext := mocks.NewMockContext(t.Context()) + mockAccount := &mockaccount.MockAccountManager{ + Subscriptions: []account.Subscription{ + {Id: "sub-1", Name: "Alpha", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", Name: "Bravo", UserAccessTenantId: "tid-2"}, + }, + } + + p, _ := newTestPrompterWithCtx(t, mockAccount, mockContext) + + // First prompt: "All tenants" (last option, index 2 with 2 tenants) + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a tenant") + }).Respond(2) + + // Second prompt: select subscription from full list + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a subscription") + }).Respond(0) // pick first subscription + + subId, err := p.PromptSubscription(t.Context(), "Select a subscription") + require.NoError(t, err) + require.Equal(t, "sub-1", subId) // Alpha is first alphabetically +} + +func newTestPrompterWithCtx( + t *testing.T, + mockAccount *mockaccount.MockAccountManager, + mockCtx *mocks.MockContext, +) (*DefaultPrompter, *mocks.MockContext) { + t.Helper() + env := environment.New("test") + resourceService := azapi.NewResourceService( + mockCtx.SubscriptionCredentialProvider, mockCtx.ArmClientOptions) + + p := NewDefaultPrompter( + env, mockCtx.Console, mockAccount, resourceService, cloud.AzurePublic(), + ).(*DefaultPrompter) + + return p, mockCtx +} diff --git a/cli/azd/test/mocks/mockaccount/mock_manager.go b/cli/azd/test/mocks/mockaccount/mock_manager.go index 1b1f21a9c6a..ee80d841084 100644 --- a/cli/azd/test/mocks/mockaccount/mock_manager.go +++ b/cli/azd/test/mocks/mockaccount/mock_manager.go @@ -100,6 +100,21 @@ func (a *MockAccountManager) SetDefaultLocation( return nil, nil } +func (a *MockAccountManager) GetTenantDisplayNames(ctx context.Context) (map[string]string, error) { + // Build display names from the subscriptions' tenant IDs + result := make(map[string]string) + for _, sub := range a.Subscriptions { + tid := sub.UserAccessTenantId + if tid == "" { + tid = sub.TenantId + } + if _, exists := result[tid]; !exists { + result[tid] = tid + } + } + return result, nil +} + // SubscriptionCredentialProviderFunc implements [account.SubscriptionCredentialProvider] using the provided function. type SubscriptionCredentialProviderFunc func(ctx context.Context, subscriptionId string) (azcore.TokenCredential, error) diff --git a/cli/azd/test/mocks/mockaccount/mock_subscriptions.go b/cli/azd/test/mocks/mockaccount/mock_subscriptions.go index 60d51a722ae..22621281f84 100644 --- a/cli/azd/test/mocks/mockaccount/mock_subscriptions.go +++ b/cli/azd/test/mocks/mockaccount/mock_subscriptions.go @@ -24,6 +24,11 @@ func (m *MockSubscriptionManager) ListLocations(ctx context.Context, subscriptio return args.Get(0).([]account.Location), args.Error(1) } +func (m *MockSubscriptionManager) GetTenantDisplayNames(ctx context.Context) (map[string]string, error) { + args := m.Called(ctx) + return args.Get(0).(map[string]string), args.Error(1) +} + func (m *MockSubscriptionManager) GetLocations(ctx context.Context, subscriptionId string) ([]account.Location, error) { args := m.Called(ctx, subscriptionId) return args.Get(0).([]account.Location), args.Error(1) From 7d434715ee05a6747c079c759eee26597e5b36a2 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 20:20:53 +0000 Subject: [PATCH 02/11] fix: address Copilot review feedback (iteration 1) - Fix loop variable pointer bug in subscription slice building - Clear loading message to avoid redundant spinner with pre-loaded data - Update filterByTenantEnvVar comment to accurately reflect fallback behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/prompt_service.go | 9 ++++++--- cli/azd/pkg/prompt/tenant_helper.go | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cli/azd/pkg/prompt/prompt_service.go b/cli/azd/pkg/prompt/prompt_service.go index 8a6c5febaf4..bc6bbc56efb 100644 --- a/cli/azd/pkg/prompt/prompt_service.go +++ b/cli/azd/pkg/prompt/prompt_service.go @@ -275,12 +275,15 @@ func (ps *promptService) PromptSubscription( hideId := isDemoModeEnabled() - // Use PromptCustomResource with pre-loaded data (no LoadData spinner needed) + // Use PromptCustomResource with pre-loaded data subscriptions := make([]*account.Subscription, len(subscriptionList)) - for i, subscription := range subscriptionList { - subscriptions[i] = &subscription + for i := range subscriptionList { + subscriptions[i] = &subscriptionList[i] } + // Clear loading message since data is already loaded (avoids a redundant spinner) + mergedOptions.LoadingMessage = "" + return PromptCustomResource(ctx, CustomResourceOptions[account.Subscription]{ SelectorOptions: mergedOptions, LoadData: func(ctx context.Context) ([]*account.Subscription, error) { diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 5b16401c564..6840a5362d8 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -93,7 +93,9 @@ func filterSubscriptionsByTenant( } // filterByTenantEnvVar filters subscriptions by AZURE_TENANT_ID if set. -// This is applied in both prompt and no-prompt modes to ensure env var is authoritative. +// This is applied in both prompt and no-prompt modes. +// If the env var is set but no subscriptions match (e.g. stale tenant ID), +// the filter is a no-op and returns all subscriptions to avoid blocking the user. func filterByTenantEnvVar(subscriptions []account.Subscription) []account.Subscription { tenantId := os.Getenv(environment.TenantIdEnvVarName) if tenantId == "" { From 140807dc3282e77a213e731deabc5d797c59e071 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 20:30:45 +0000 Subject: [PATCH 03/11] fix: address Copilot review feedback (iteration 2) - Skip spinner in PromptCustomResource when LoadingMessage is empty - Remove unused preSelectedTenantId parameter from promptTenantSelection - Update PR description to reflect graceful fallback behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/prompt_service.go | 28 +++++++++++------ cli/azd/pkg/prompt/prompter.go | 2 +- cli/azd/pkg/prompt/tenant_helper.go | 11 ------- cli/azd/pkg/prompt/tenant_helper_test.go | 40 +++--------------------- 4 files changed, 23 insertions(+), 58 deletions(-) diff --git a/cli/azd/pkg/prompt/prompt_service.go b/cli/azd/pkg/prompt/prompt_service.go index bc6bbc56efb..ddd4c7dbbd7 100644 --- a/cli/azd/pkg/prompt/prompt_service.go +++ b/cli/azd/pkg/prompt/prompt_service.go @@ -317,7 +317,7 @@ func (ps *promptService) promptAndFilterByTenant( tenants = extractUniqueTenants(subscriptions, tenantNames) - selectedTenantId, err := promptTenantSelection(ctx, ps.console, tenants, "") + selectedTenantId, err := promptTenantSelection(ctx, ps.console, tenants) if err != nil { return nil, err } @@ -822,21 +822,29 @@ func PromptCustomResource[T any](ctx context.Context, options CustomResourceOpti allowNewResource = true selectedIndex = new(0) } else { - loadingSpinner := ux.NewSpinner(&ux.SpinnerOptions{ - Text: options.SelectorOptions.LoadingMessage, - }) - - err := loadingSpinner.Run(ctx, func(ctx context.Context) error { + loadData := func(ctx context.Context) error { resourceList, err := options.LoadData(ctx) if err != nil { return err } - resources = resourceList return nil - }) - if err != nil { - return nil, err + } + + // Skip the spinner when loading message is empty (data is pre-loaded) + if mergedSelectorOptions.LoadingMessage != "" { + loadingSpinner := ux.NewSpinner(&ux.SpinnerOptions{ + Text: mergedSelectorOptions.LoadingMessage, + }) + if err := loadingSpinner.Run(ctx, func(ctx context.Context) error { + return loadData(ctx) + }); err != nil { + return nil, err + } + } else { + if err := loadData(ctx); err != nil { + return nil, err + } } if !allowNewResource && len(resources) == 0 { diff --git a/cli/azd/pkg/prompt/prompter.go b/cli/azd/pkg/prompt/prompter.go index a6cd916dfcf..26ee03ccd0f 100644 --- a/cli/azd/pkg/prompt/prompter.go +++ b/cli/azd/pkg/prompt/prompter.go @@ -155,7 +155,7 @@ func (p *DefaultPrompter) promptAndFilterByTenant( tenants = extractUniqueTenants(subscriptions, tenantNames) - selectedTenantId, err := promptTenantSelection(ctx, p.console, tenants, "") + selectedTenantId, err := promptTenantSelection(ctx, p.console, tenants) if err != nil { return nil, err } diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 6840a5362d8..fd692bcdba9 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -119,7 +119,6 @@ func promptTenantSelection( ctx context.Context, console input.Console, tenants []TenantInfo, - preSelectedTenantId string, ) (string, error) { if len(tenants) <= 1 { if len(tenants) == 1 { @@ -128,16 +127,6 @@ func promptTenantSelection( return "", nil } - // If a tenant is already pre-selected (e.g. from AZURE_TENANT_ID), use it directly - if preSelectedTenantId != "" { - for _, t := range tenants { - if t.Id == preSelectedTenantId { - return t.Id, nil - } - } - // Pre-selected tenant not found in available tenants; fall through to prompt - } - allTenantsLabel := fmt.Sprintf( "%2d. All tenants", len(tenants)+1, diff --git a/cli/azd/pkg/prompt/tenant_helper_test.go b/cli/azd/pkg/prompt/tenant_helper_test.go index a11feecd928..fec871586bd 100644 --- a/cli/azd/pkg/prompt/tenant_helper_test.go +++ b/cli/azd/pkg/prompt/tenant_helper_test.go @@ -154,7 +154,7 @@ func TestPromptTenantSelection_SingleTenant(t *testing.T) { {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, } - selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "") + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants) require.NoError(t, err) require.Equal(t, "tid-1", selected) } @@ -171,7 +171,7 @@ func TestPromptTenantSelection_MultipleTenants_SelectFirst(t *testing.T) { {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, } - selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "") + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants) require.NoError(t, err) require.Equal(t, "tid-1", selected) } @@ -188,47 +188,15 @@ func TestPromptTenantSelection_MultipleTenants_SelectAllTenants(t *testing.T) { {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, } - selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "") + selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants) require.NoError(t, err) require.Empty(t, selected) // empty string = all tenants } -func TestPromptTenantSelection_PreSelected(t *testing.T) { - mockContext := mocks.NewMockContext(t.Context()) - - tenants := []TenantInfo{ - {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, - {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, - } - - // Pre-selected tenant skips the prompt entirely - selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "tid-2") - require.NoError(t, err) - require.Equal(t, "tid-2", selected) -} - -func TestPromptTenantSelection_PreSelectedNotFound(t *testing.T) { - mockContext := mocks.NewMockContext(t.Context()) - - mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { - return strings.Contains(opts.Message, "Select a tenant") - }).Respond(0) - - tenants := []TenantInfo{ - {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, - {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, - } - - // Pre-selected tenant not found, falls through to prompt - selected, err := promptTenantSelection(t.Context(), mockContext.Console, tenants, "tid-unknown") - require.NoError(t, err) - require.Equal(t, "tid-1", selected) -} - func TestPromptTenantSelection_NoTenants(t *testing.T) { mockContext := mocks.NewMockContext(t.Context()) - selected, err := promptTenantSelection(t.Context(), mockContext.Console, nil, "") + selected, err := promptTenantSelection(t.Context(), mockContext.Console, nil) require.NoError(t, err) require.Empty(t, selected) } From aee4e2a8431a3df106716630d7eb0e7073439976 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 20:40:10 +0000 Subject: [PATCH 04/11] fix: address Copilot review feedback (iteration 3) - Add SkipLoadingSpinner field to SelectOptions for reliable spinner skip - Extract shared promptAndFilterByTenant function to eliminate duplication Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/prompt_service.go | 50 ++++++++-------------------- cli/azd/pkg/prompt/prompter.go | 31 ++--------------- cli/azd/pkg/prompt/tenant_helper.go | 35 +++++++++++++++++++ 3 files changed, 50 insertions(+), 66 deletions(-) diff --git a/cli/azd/pkg/prompt/prompt_service.go b/cli/azd/pkg/prompt/prompt_service.go index ddd4c7dbbd7..a85b66003e8 100644 --- a/cli/azd/pkg/prompt/prompt_service.go +++ b/cli/azd/pkg/prompt/prompt_service.go @@ -7,7 +7,6 @@ import ( "context" "fmt" "io" - "log" "os" "slices" "strconv" @@ -102,6 +101,9 @@ type SelectOptions struct { HelpMessage string // LoadingMessage is the loading message to display to the user. LoadingMessage string + // SkipLoadingSpinner skips the loading spinner in PromptCustomResource. + // Use this when data is pre-loaded and LoadData returns immediately. + SkipLoadingSpinner bool // DisplayNumbers specifies whether to display numbers next to the choices. DisplayNumbers *bool // DisplayCount is the number of choices to display at a time. @@ -257,7 +259,8 @@ func (ps *promptService) PromptSubscription( // Apply tenant filtering (after spinner is done so the prompt doesn't overlap) subscriptionList = filterByTenantEnvVar(subscriptionList) if !ps.console.IsNoPromptMode() { - subscriptionList, err = ps.promptAndFilterByTenant(ctx, subscriptionList) + subscriptionList, err = promptAndFilterByTenant( + ctx, ps.console, subscriptionList, ps.subscriptionManager.GetTenantDisplayNames) if err != nil { return nil, err } @@ -281,8 +284,8 @@ func (ps *promptService) PromptSubscription( subscriptions[i] = &subscriptionList[i] } - // Clear loading message since data is already loaded (avoids a redundant spinner) - mergedOptions.LoadingMessage = "" + // Skip the inner spinner since data is already loaded + mergedOptions.SkipLoadingSpinner = true return PromptCustomResource(ctx, CustomResourceOptions[account.Subscription]{ SelectorOptions: mergedOptions, @@ -298,33 +301,6 @@ func (ps *promptService) PromptSubscription( }) } -// promptAndFilterByTenant prompts the user to select a tenant when subscriptions span multiple tenants. -func (ps *promptService) promptAndFilterByTenant( - ctx context.Context, - subscriptions []account.Subscription, -) ([]account.Subscription, error) { - tenants := extractUniqueTenants(subscriptions, nil) - if len(tenants) <= 1 { - return subscriptions, nil - } - - // Only fetch tenant display names when we actually need to prompt - tenantNames, err := ps.subscriptionManager.GetTenantDisplayNames(ctx) - if err != nil { - log.Printf("failed to fetch tenant display names, using tenant IDs: %v", err) - tenantNames = map[string]string{} - } - - tenants = extractUniqueTenants(subscriptions, tenantNames) - - selectedTenantId, err := promptTenantSelection(ctx, ps.console, tenants) - if err != nil { - return nil, err - } - - return filterSubscriptionsByTenant(subscriptions, selectedTenantId), nil -} - // PromptLocation prompts the user to select an Azure location. func (ps *promptService) PromptLocation( ctx context.Context, @@ -831,8 +807,12 @@ func PromptCustomResource[T any](ctx context.Context, options CustomResourceOpti return nil } - // Skip the spinner when loading message is empty (data is pre-loaded) - if mergedSelectorOptions.LoadingMessage != "" { + // Skip the spinner when data is pre-loaded + if mergedSelectorOptions.SkipLoadingSpinner { + if err := loadData(ctx); err != nil { + return nil, err + } + } else { loadingSpinner := ux.NewSpinner(&ux.SpinnerOptions{ Text: mergedSelectorOptions.LoadingMessage, }) @@ -841,10 +821,6 @@ func PromptCustomResource[T any](ctx context.Context, options CustomResourceOpti }); err != nil { return nil, err } - } else { - if err := loadData(ctx); err != nil { - return nil, err - } } if !allowNewResource && len(resources) == 0 { diff --git a/cli/azd/pkg/prompt/prompter.go b/cli/azd/pkg/prompt/prompter.go index 26ee03ccd0f..17115163882 100644 --- a/cli/azd/pkg/prompt/prompter.go +++ b/cli/azd/pkg/prompt/prompter.go @@ -92,7 +92,8 @@ func (p *DefaultPrompter) PromptSubscription(ctx context.Context, msg string) (s // Tenant selection: if multiple tenants, prompt user to pick one if !p.console.IsNoPromptMode() { - subscriptionInfos, err = p.promptAndFilterByTenant(ctx, subscriptionInfos) + subscriptionInfos, err = promptAndFilterByTenant( + ctx, p.console, subscriptionInfos, p.accountManager.GetTenantDisplayNames) if err != nil { return "", err } @@ -135,34 +136,6 @@ func (p *DefaultPrompter) PromptSubscription(ctx context.Context, msg string) (s return subscriptionId, nil } -// promptAndFilterByTenant prompts the user to select a tenant when subscriptions span multiple tenants. -func (p *DefaultPrompter) promptAndFilterByTenant( - ctx context.Context, - subscriptions []account.Subscription, -) ([]account.Subscription, error) { - // Quick check without display names to avoid unnecessary API call - tenants := extractUniqueTenants(subscriptions, nil) - if len(tenants) <= 1 { - return subscriptions, nil - } - - // Only fetch tenant display names when we actually need to prompt - tenantNames, err := p.accountManager.GetTenantDisplayNames(ctx) - if err != nil { - log.Printf("failed to fetch tenant display names, using tenant IDs: %v", err) - tenantNames = map[string]string{} - } - - tenants = extractUniqueTenants(subscriptions, tenantNames) - - selectedTenantId, err := promptTenantSelection(ctx, p.console, tenants) - if err != nil { - return nil, err - } - - return filterSubscriptionsByTenant(subscriptions, selectedTenantId), nil -} - // formatSubscriptionOptions formats subscription infos into display options. func formatSubscriptionOptions( subscriptionInfos []account.Subscription, diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index fd692bcdba9..1aa29730f9a 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -7,6 +7,7 @@ import ( "cmp" "context" "fmt" + "log" "os" "slices" "strings" @@ -154,6 +155,40 @@ func promptTenantSelection( return tenants[selectedIndex].Id, nil } +// TenantDisplayNameProvider is a function that fetches tenant display names. +type TenantDisplayNameProvider func(ctx context.Context) (map[string]string, error) + +// promptAndFilterByTenant prompts the user to select a tenant when subscriptions span multiple tenants. +// It extracts unique tenants, fetches display names only when needed, and returns filtered subscriptions. +func promptAndFilterByTenant( + ctx context.Context, + console input.Console, + subscriptions []account.Subscription, + getTenantNames TenantDisplayNameProvider, +) ([]account.Subscription, error) { + // Quick check without display names to avoid unnecessary API call + tenants := extractUniqueTenants(subscriptions, nil) + if len(tenants) <= 1 { + return subscriptions, nil + } + + // Only fetch tenant display names when we actually need to prompt + tenantNames, err := getTenantNames(ctx) + if err != nil { + log.Printf("failed to fetch tenant display names, using tenant IDs: %v", err) + tenantNames = map[string]string{} + } + + tenants = extractUniqueTenants(subscriptions, tenantNames) + + selectedTenantId, err := promptTenantSelection(ctx, console, tenants) + if err != nil { + return nil, err + } + + return filterSubscriptionsByTenant(subscriptions, selectedTenantId), nil +} + func formatTenantOption(index int, t TenantInfo) string { subCountLabel := fmt.Sprintf( "%d subscription", t.SubscriptionCount, From a6feda460c03438f86ab206cb44cbf53547bb568 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 20:52:34 +0000 Subject: [PATCH 05/11] fix: address Copilot review feedback (iteration 4) - Make TenantInfo unexported (tenantInfo) to reduce public API surface - Update extractUniqueTenants comment to document TenantId fallback - Add test for SkipLoadingSpinner path in PromptCustomResource Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../pkg/prompt/prompt_service_extra_test.go | 21 +++++++++++++++++++ cli/azd/pkg/prompt/tenant_helper.go | 21 ++++++++++--------- cli/azd/pkg/prompt/tenant_helper_test.go | 6 +++--- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/cli/azd/pkg/prompt/prompt_service_extra_test.go b/cli/azd/pkg/prompt/prompt_service_extra_test.go index ce6c3e25972..d250297a693 100644 --- a/cli/azd/pkg/prompt/prompt_service_extra_test.go +++ b/cli/azd/pkg/prompt/prompt_service_extra_test.go @@ -252,6 +252,27 @@ func TestPromptCustomResource_NilSelectorOptions_UsesDefaultsAndForce(t *testing require.Equal(t, 42, *result) } +func TestPromptCustomResource_SkipLoadingSpinner(t *testing.T) { + t.Parallel() + + loaded := false + _, err := PromptCustomResource(t.Context(), CustomResourceOptions[string]{ + SelectorOptions: &SelectOptions{ + SkipLoadingSpinner: true, + AllowNewResource: new(false), + }, + LoadData: func(ctx context.Context) ([]*string, error) { + loaded = true + return nil, nil + }, + }) + + // LoadData should have been called directly (without spinner) + require.True(t, loaded) + // No resources and AllowNewResource=false returns the sentinel error + require.ErrorIs(t, err, ErrNoResourcesFound) +} + // helpers func emptySubs() []account.Subscription { return []account.Subscription{} } diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 1aa29730f9a..0a8331bf652 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -18,8 +18,8 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/output" ) -// TenantInfo holds display metadata for a tenant extracted from the subscription list. -type TenantInfo struct { +// tenantInfo holds display metadata for a tenant extracted from the subscription list. +type tenantInfo struct { // Id is the tenant ID (GUID). Id string // DisplayName is the friendly name of the tenant, or the ID if no name is available. @@ -29,14 +29,15 @@ type TenantInfo struct { } // extractUniqueTenants extracts unique tenants from a list of subscriptions, -// grouped by UserAccessTenantId. The returned list is sorted by DisplayName. +// grouped by UserAccessTenantId (falling back to TenantId when UserAccessTenantId is empty). +// The returned list is sorted by DisplayName. // Tenant display names are resolved from the provided tenantDisplayNames map; // if a tenant ID is not in the map, the ID itself is used as the display name. func extractUniqueTenants( subscriptions []account.Subscription, tenantDisplayNames map[string]string, -) []TenantInfo { - tenantMap := make(map[string]*TenantInfo) +) []tenantInfo { + tenantMap := make(map[string]*tenantInfo) for _, sub := range subscriptions { tid := sub.UserAccessTenantId @@ -51,7 +52,7 @@ func extractUniqueTenants( if name, ok := tenantDisplayNames[tid]; ok && name != "" { displayName = name } - tenantMap[tid] = &TenantInfo{ + tenantMap[tid] = &tenantInfo{ Id: tid, DisplayName: displayName, SubscriptionCount: 1, @@ -59,12 +60,12 @@ func extractUniqueTenants( } } - tenants := make([]TenantInfo, 0, len(tenantMap)) + tenants := make([]tenantInfo, 0, len(tenantMap)) for _, info := range tenantMap { tenants = append(tenants, *info) } - slices.SortFunc(tenants, func(a, b TenantInfo) int { + slices.SortFunc(tenants, func(a, b tenantInfo) int { return cmp.Compare( strings.ToLower(a.DisplayName), strings.ToLower(b.DisplayName), @@ -119,7 +120,7 @@ func filterByTenantEnvVar(subscriptions []account.Subscription) []account.Subscr func promptTenantSelection( ctx context.Context, console input.Console, - tenants []TenantInfo, + tenants []tenantInfo, ) (string, error) { if len(tenants) <= 1 { if len(tenants) == 1 { @@ -189,7 +190,7 @@ func promptAndFilterByTenant( return filterSubscriptionsByTenant(subscriptions, selectedTenantId), nil } -func formatTenantOption(index int, t TenantInfo) string { +func formatTenantOption(index int, t tenantInfo) string { subCountLabel := fmt.Sprintf( "%d subscription", t.SubscriptionCount, ) diff --git a/cli/azd/pkg/prompt/tenant_helper_test.go b/cli/azd/pkg/prompt/tenant_helper_test.go index fec871586bd..b874620dd89 100644 --- a/cli/azd/pkg/prompt/tenant_helper_test.go +++ b/cli/azd/pkg/prompt/tenant_helper_test.go @@ -150,7 +150,7 @@ func TestFilterByTenantEnvVar_NoMatchFallsBack(t *testing.T) { func TestPromptTenantSelection_SingleTenant(t *testing.T) { mockContext := mocks.NewMockContext(t.Context()) - tenants := []TenantInfo{ + tenants := []tenantInfo{ {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, } @@ -166,7 +166,7 @@ func TestPromptTenantSelection_MultipleTenants_SelectFirst(t *testing.T) { return strings.Contains(opts.Message, "Select a tenant") }).Respond(0) // pick first tenant - tenants := []TenantInfo{ + tenants := []tenantInfo{ {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, } @@ -183,7 +183,7 @@ func TestPromptTenantSelection_MultipleTenants_SelectAllTenants(t *testing.T) { return strings.Contains(opts.Message, "Select a tenant") }).Respond(2) // pick "All tenants" (third option with 2 tenants) - tenants := []TenantInfo{ + tenants := []tenantInfo{ {Id: "tid-1", DisplayName: "Contoso", SubscriptionCount: 3}, {Id: "tid-2", DisplayName: "Fabrikam", SubscriptionCount: 1}, } From 4d681e587a018edb3b12d5740cefb9d4df1722d9 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 21:02:42 +0000 Subject: [PATCH 06/11] fix: address Copilot review feedback (iteration 5) - Guard mock GetTenantDisplayNames against nil panic - Skip subscriptions with empty tenant IDs - Add deterministic sort tie-breaker by tenant ID Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/tenant_helper.go | 10 ++++++++-- cli/azd/test/mocks/mockaccount/mock_subscriptions.go | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 0a8331bf652..9ac94dd0150 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -44,6 +44,9 @@ func extractUniqueTenants( if tid == "" { tid = sub.TenantId } + if tid == "" { + continue + } if info, exists := tenantMap[tid]; exists { info.SubscriptionCount++ @@ -66,10 +69,13 @@ func extractUniqueTenants( } slices.SortFunc(tenants, func(a, b tenantInfo) int { - return cmp.Compare( + if c := cmp.Compare( strings.ToLower(a.DisplayName), strings.ToLower(b.DisplayName), - ) + ); c != 0 { + return c + } + return cmp.Compare(a.Id, b.Id) }) return tenants diff --git a/cli/azd/test/mocks/mockaccount/mock_subscriptions.go b/cli/azd/test/mocks/mockaccount/mock_subscriptions.go index 22621281f84..0034ed0c5da 100644 --- a/cli/azd/test/mocks/mockaccount/mock_subscriptions.go +++ b/cli/azd/test/mocks/mockaccount/mock_subscriptions.go @@ -26,6 +26,9 @@ func (m *MockSubscriptionManager) ListLocations(ctx context.Context, subscriptio func (m *MockSubscriptionManager) GetTenantDisplayNames(ctx context.Context) (map[string]string, error) { args := m.Called(ctx) + if args.Get(0) == nil { + return nil, args.Error(1) + } return args.Get(0).(map[string]string), args.Error(1) } From 94ba95864dcf42995abb085730670a3dc5218189 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 21:13:55 +0000 Subject: [PATCH 07/11] fix: address Copilot review feedback (iteration 6) - Hoist isDemoModeEnabled() call outside subscription format loop - Add nil guard for getTenantNames provider - Skip empty tenant IDs in mock GetTenantDisplayNames Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/prompt_service.go | 7 ++++--- cli/azd/pkg/prompt/prompter.go | 10 ++++++---- cli/azd/pkg/prompt/tenant_helper.go | 12 ++++++++---- cli/azd/test/mocks/mockaccount/mock_manager.go | 4 +++- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/cli/azd/pkg/prompt/prompt_service.go b/cli/azd/pkg/prompt/prompt_service.go index a85b66003e8..9061afd0749 100644 --- a/cli/azd/pkg/prompt/prompt_service.go +++ b/cli/azd/pkg/prompt/prompt_service.go @@ -284,11 +284,12 @@ func (ps *promptService) PromptSubscription( subscriptions[i] = &subscriptionList[i] } - // Skip the inner spinner since data is already loaded - mergedOptions.SkipLoadingSpinner = true + // Create selector options with spinner disabled since data is already loaded + resourceSelectorOptions := *mergedOptions + resourceSelectorOptions.SkipLoadingSpinner = true return PromptCustomResource(ctx, CustomResourceOptions[account.Subscription]{ - SelectorOptions: mergedOptions, + SelectorOptions: &resourceSelectorOptions, LoadData: func(ctx context.Context) ([]*account.Subscription, error) { return subscriptions, nil }, diff --git a/cli/azd/pkg/prompt/prompter.go b/cli/azd/pkg/prompt/prompter.go index 17115163882..09f2ada4dac 100644 --- a/cli/azd/pkg/prompt/prompter.go +++ b/cli/azd/pkg/prompt/prompter.go @@ -10,7 +10,6 @@ import ( "log" "os" "slices" - "strconv" "github.com/MakeNowJust/heredoc/v2" "github.com/azure/azure-dev/cli/azd/pkg/account" @@ -73,7 +72,7 @@ func NewDefaultPrompter( func (p *DefaultPrompter) PromptSubscription(ctx context.Context, msg string) (subscriptionId string, err error) { subscriptionInfos, err := p.accountManager.GetSubscriptions(ctx) if err != nil { - return "", fmt.Errorf("listing accounts: %w", err) + return "", fmt.Errorf("listing subscriptions: %w", err) } if len(subscriptionInfos) == 0 { @@ -144,11 +143,14 @@ func formatSubscriptionOptions( options = make([]string, len(subscriptionInfos)) ids = make([]string, len(subscriptionInfos)) + hideId := isDemoModeEnabled() + for index, info := range subscriptionInfos { - if v, err := strconv.ParseBool(os.Getenv("AZD_DEMO_MODE")); err == nil && v { + if hideId { options[index] = fmt.Sprintf("%2d. %s", index+1, info.Name) } else { - options[index] = fmt.Sprintf("%2d. %s (%s)", index+1, info.Name, info.Id) + options[index] = fmt.Sprintf( + "%2d. %s (%s)", index+1, info.Name, info.Id) } ids[index] = info.Id diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 9ac94dd0150..6973323ec81 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -180,10 +180,14 @@ func promptAndFilterByTenant( } // Only fetch tenant display names when we actually need to prompt - tenantNames, err := getTenantNames(ctx) - if err != nil { - log.Printf("failed to fetch tenant display names, using tenant IDs: %v", err) - tenantNames = map[string]string{} + var tenantNames map[string]string + if getTenantNames != nil { + var err error + tenantNames, err = getTenantNames(ctx) + if err != nil { + log.Printf("failed to fetch tenant display names: %v", err) + tenantNames = nil + } } tenants = extractUniqueTenants(subscriptions, tenantNames) diff --git a/cli/azd/test/mocks/mockaccount/mock_manager.go b/cli/azd/test/mocks/mockaccount/mock_manager.go index ee80d841084..44fafd093d9 100644 --- a/cli/azd/test/mocks/mockaccount/mock_manager.go +++ b/cli/azd/test/mocks/mockaccount/mock_manager.go @@ -101,13 +101,15 @@ func (a *MockAccountManager) SetDefaultLocation( } func (a *MockAccountManager) GetTenantDisplayNames(ctx context.Context) (map[string]string, error) { - // Build display names from the subscriptions' tenant IDs result := make(map[string]string) for _, sub := range a.Subscriptions { tid := sub.UserAccessTenantId if tid == "" { tid = sub.TenantId } + if tid == "" { + continue + } if _, exists := result[tid]; !exists { result[tid] = tid } From 00aca2aaa7604e3be717dec8de49a32dbda1c430 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 21:37:17 +0000 Subject: [PATCH 08/11] fix: address Copilot review feedback (iteration 8) - Wrap GetSubscriptions error with context in promptService - Add no-prompt mode test verifying tenant picker is skipped and env var filtering works Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/prompt_service.go | 2 +- cli/azd/pkg/prompt/tenant_helper_test.go | 29 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/cli/azd/pkg/prompt/prompt_service.go b/cli/azd/pkg/prompt/prompt_service.go index 9061afd0749..ad0b3d7117d 100644 --- a/cli/azd/pkg/prompt/prompt_service.go +++ b/cli/azd/pkg/prompt/prompt_service.go @@ -253,7 +253,7 @@ func (ps *promptService) PromptSubscription( return loadErr }) if err != nil { - return nil, err + return nil, fmt.Errorf("listing subscriptions: %w", err) } // Apply tenant filtering (after spinner is done so the prompt doesn't overlap) diff --git a/cli/azd/pkg/prompt/tenant_helper_test.go b/cli/azd/pkg/prompt/tenant_helper_test.go index b874620dd89..c4f1a2270eb 100644 --- a/cli/azd/pkg/prompt/tenant_helper_test.go +++ b/cli/azd/pkg/prompt/tenant_helper_test.go @@ -255,6 +255,35 @@ func TestPromptSubscription_MultiTenant_AllTenantsOption(t *testing.T) { require.Equal(t, "sub-1", subId) // Alpha is first alphabetically } +func TestPromptSubscription_NoPromptMode_SkipsTenantPicker(t *testing.T) { + t.Setenv("AZURE_TENANT_ID", "tid-1") + + mockContext := mocks.NewMockContext(t.Context()) + mockContext.Console.SetNoPromptMode(true) + + mockAccount := &mockaccount.MockAccountManager{ + Subscriptions: []account.Subscription{ + {Id: "sub-1", Name: "Alpha", UserAccessTenantId: "tid-1"}, + {Id: "sub-2", Name: "Bravo", UserAccessTenantId: "tid-2"}, + {Id: "sub-3", Name: "Charlie", UserAccessTenantId: "tid-1"}, + }, + } + + p, _ := newTestPrompterWithCtx(t, mockAccount, mockContext) + + // In no-prompt mode with default subscription set, PromptSubscription should + // filter by AZURE_TENANT_ID and select the default without prompting. + // Set up the subscription select to pick first. + mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { + return strings.Contains(opts.Message, "Select a subscription") + }).Respond(0) + + subId, err := p.PromptSubscription(t.Context(), "Select a subscription") + require.NoError(t, err) + // Should be filtered to tid-1 only: Alpha and Charlie + require.Equal(t, "sub-1", subId) +} + func newTestPrompterWithCtx( t *testing.T, mockAccount *mockaccount.MockAccountManager, From 831164eed150ca3aab1e27e55b68c8d39bb90c57 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Wed, 6 May 2026 22:26:50 +0000 Subject: [PATCH 09/11] fix: address Copilot review feedback (iteration 9) - Use append-based filtering instead of clone+delete for efficiency - Fix test comment to accurately describe no-prompt behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/tenant_helper.go | 10 +++++++--- cli/azd/pkg/prompt/tenant_helper_test.go | 6 +++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 6973323ec81..2c9f270b7a1 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -91,13 +91,17 @@ func filterSubscriptionsByTenant( return subscriptions } - return slices.DeleteFunc(slices.Clone(subscriptions), func(sub account.Subscription) bool { + filtered := make([]account.Subscription, 0, len(subscriptions)) + for _, sub := range subscriptions { accessTenant := sub.UserAccessTenantId if accessTenant == "" { accessTenant = sub.TenantId } - return accessTenant != tenantId - }) + if accessTenant == tenantId { + filtered = append(filtered, sub) + } + } + return filtered } // filterByTenantEnvVar filters subscriptions by AZURE_TENANT_ID if set. diff --git a/cli/azd/pkg/prompt/tenant_helper_test.go b/cli/azd/pkg/prompt/tenant_helper_test.go index c4f1a2270eb..f52366c06d7 100644 --- a/cli/azd/pkg/prompt/tenant_helper_test.go +++ b/cli/azd/pkg/prompt/tenant_helper_test.go @@ -271,9 +271,9 @@ func TestPromptSubscription_NoPromptMode_SkipsTenantPicker(t *testing.T) { p, _ := newTestPrompterWithCtx(t, mockAccount, mockContext) - // In no-prompt mode with default subscription set, PromptSubscription should - // filter by AZURE_TENANT_ID and select the default without prompting. - // Set up the subscription select to pick first. + // In no-prompt mode the tenant picker is skipped, but AZURE_TENANT_ID + // filtering still applies. Subscription selection still goes through + // Console.Select (not bypassed by no-prompt in this legacy prompter path). mockContext.Console.WhenSelect(func(opts input.ConsoleOptions) bool { return strings.Contains(opts.Message, "Select a subscription") }).Respond(0) From 3ce979ae3c9f36df94c18cd6851741364aae2e48 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 12 May 2026 22:03:14 +0000 Subject: [PATCH 10/11] fix: log when AZURE_TENANT_ID does not match any tenant Adds debug logging when the env var filter produces no results, helping users diagnose stale AZURE_TENANT_ID values with --debug. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/tenant_helper.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 2c9f270b7a1..961a32a6006 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -118,6 +118,7 @@ func filterByTenantEnvVar(subscriptions []account.Subscription) []account.Subscr // If filtering produces no results, fall back to showing all subscriptions // rather than erroring out — the tenant ID may be stale if len(filtered) == 0 { + log.Printf("AZURE_TENANT_ID=%s did not match any subscription tenants, showing all subscriptions", tenantId) return subscriptions } From e367d5477fc457e942e24724f93e3b7080baaa68 Mon Sep 17 00:00:00 2001 From: Victor Vazquez Date: Tue, 12 May 2026 22:42:09 +0000 Subject: [PATCH 11/11] fix: resolve gosec G706 log injection warning Use log.Println with a fixed message instead of interpolating the env var value to satisfy the gosec taint analysis check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/pkg/prompt/tenant_helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/azd/pkg/prompt/tenant_helper.go b/cli/azd/pkg/prompt/tenant_helper.go index 961a32a6006..9b1c665ae40 100644 --- a/cli/azd/pkg/prompt/tenant_helper.go +++ b/cli/azd/pkg/prompt/tenant_helper.go @@ -118,7 +118,7 @@ func filterByTenantEnvVar(subscriptions []account.Subscription) []account.Subscr // If filtering produces no results, fall back to showing all subscriptions // rather than erroring out — the tenant ID may be stale if len(filtered) == 0 { - log.Printf("AZURE_TENANT_ID=%s did not match any subscription tenants, showing all subscriptions", tenantId) + log.Println("AZURE_TENANT_ID did not match any subscription tenants, showing all subscriptions") return subscriptions }