quality: apply max-quality wave 1-3 fixes

- Pre-lowercase extensionResourceTypePrefixes for O(1) lookup
- Add trust-boundary comment at Tier 1 entry
- Correct goroutine invariant comment (sends at most once)
- Log foreign resource names in Tier 4 veto
- Add hash case-sensitivity comment
- Improve Interactive field doc comment
- Use atomic.Bool/Int32 for test concurrency counters
- Remove duplicate 404 test, add non-azcore error test
- Modernize map key collection with slices.Collect(maps.Keys)
- Improve getResourceGroupTags doc (error-handling asymmetry)
- Guard nil env tag pointer in standard_deployments.go
- Fix architecture doc evaluation order
- Add diagnosticsettings to cspell dictionary
- Promote armlocks to direct dependency (go mod tidy)
- Apply gofmt to all changed files

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: jongio

cli/azd/.vscode/cspell.yaml

@@ -22,6 +22,7 @@ words:
   - cooldown
   - customtype
   - devcontainers
+  - diagnosticsettings
   - errgroup
   - errorhandler
   - extendee

cli/azd/go.mod

@@ -22,6 +22,7 @@ require (
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/operationalinsights/armoperationalinsights/v2 v2.0.2
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resourcegraph/armresourcegraph v0.9.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armdeploymentstacks v1.0.1
+	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armlocks v1.2.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armresources v1.2.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions v1.3.0
 	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql/v2 v2.0.0-beta.7
@@ -93,7 +94,6 @@ require (
 
 require (
 	github.com/Azure/azure-sdk-for-go/sdk/internal v1.11.2 // indirect
-	github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armlocks v1.2.0 // indirect
 	github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/internal v1.2.0 // indirect
 	github.com/alecthomas/chroma/v2 v2.20.0 // indirect
 	github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect

cli/azd/pkg/azapi/resource_group_classifier.go

@@ -45,8 +45,10 @@ type ClassifyOptions struct {
 	// ForceMode runs only Tier 1 (zero API calls). External RGs identified by
 	// deployment operations are still protected; unknown RGs are treated as owned.
 	// Tier 2/3/4 callbacks are not invoked.
-	ForceMode   bool
-	Interactive bool   // Whether to prompt for unknown RGs
+	ForceMode bool
+	// Interactive enables per-RG prompts for unknown and foreign-resource RGs.
+	// When false, unknown/unverified RGs are always skipped without deletion.
+	Interactive bool
 	EnvName     string // Current azd environment name for tag matching
 
 	// ExpectedProvisionParamHash is the expected value of the azd-provision-param-hash tag.
@@ -183,6 +185,10 @@ func ClassifyResourceGroups(
 		rg     string
 		reason string
 	}
+	// Tier 4 goroutine invariant: every RG either (a) enters wg.Go — which
+	// sends at most once to vetoCh or promptCh (clean RGs send to neither) —
+	// or (b) sends to vetoCh directly (cancelled context). Both channels
+	// are buffered to len(owned) so sends never block and goroutines never leak.
 	vetoCh := make(chan veto, len(owned))
 	promptCh := make(chan pendingPrompt, len(owned))
 	sem := make(chan struct{}, cTier4Parallelism)
@@ -282,6 +288,9 @@ func classifyTier1(
 		tier1[rg] = tier1Info{result: tier1Unknown}
 	}
 	for _, op := range operations {
+		// TRUST ASSUMPTION: ARM ProvisioningOperation=Create is only emitted for RGs
+		// that were actually created by this deployment, never for `existing` references.
+		// Tier 4 (locks + foreign resources) provides defense-in-depth for all owned RGs.
 		if name, ok := operationTargetsRG(op, cProvisionOpCreate); ok {
 			if _, tracked := tier1[name]; tracked {
 				tier1[name] = tier1Info{result: tier1Owned}
@@ -365,6 +374,8 @@ func classifyTier2(ctx context.Context, rgName string, opts ClassifyOptions) (*C
 	hashTag := tagValue(tags, cAzdProvisionHashTag)
 	if envTag != "" && hashTag != "" && strings.EqualFold(envTag, opts.EnvName) {
 		// If an expected hash is provided, verify it matches.
+		// Case-sensitive comparison is intentional — hash values must match exactly.
+		// Mismatch falls safely to Tier 3 (more scrutiny, not less).
 		// If not provided, presence of both tags is sufficient (backward compat).
 		if opts.ExpectedProvisionParamHash != "" &&
 			hashTag != opts.ExpectedProvisionParamHash {
@@ -434,6 +445,7 @@ func classifyTier4(ctx context.Context, rgName string, opts ClassifyOptions) (st
 			reason := fmt.Sprintf(
 				"vetoed (Tier 4: %d foreign resource(s) without azd-env-name=%q)", len(foreign), opts.EnvName,
 			)
+			log.Printf("classify rg=%s tier=4: foreign resources: %v", rgName, foreign)
 			return reason, true, true, nil
 		}
 	}
@@ -512,18 +524,19 @@ func tagValue(tags map[string]*string, key string) string {
 // resources that don't support tags. These are skipped during Tier 4
 // foreign-resource detection to avoid false-positive vetoes on resources
 // commonly created by azd scaffold templates.
+// All values are pre-lowercased for efficient case-insensitive comparison.
 var extensionResourceTypePrefixes = []string{
-	"Microsoft.Authorization/",
-	"Microsoft.Insights/diagnosticSettings",
-	"Microsoft.Resources/links",
+	"microsoft.authorization/",
+	"microsoft.insights/diagnosticsettings",
+	"microsoft.resources/links",
 }
 
 // isExtensionResourceType returns true if the given ARM resource type is a
 // known extension resource that does not support tags.
 func isExtensionResourceType(resourceType string) bool {
 	lower := strings.ToLower(resourceType)
 	for _, prefix := range extensionResourceTypePrefixes {
-		if strings.HasPrefix(lower, strings.ToLower(prefix)) {
+		if strings.HasPrefix(lower, prefix) {
 			return true
 		}
 	}

cli/azd/pkg/azapi/resource_group_classifier_test.go

@@ -213,21 +213,6 @@ func TestClassifyResourceGroups(t *testing.T) {
 		assert.Contains(t, res.Skipped[0].Reason, "Tier 3")
 	})
 
-	t.Run("Tier2 tag fetch 404 — already deleted skip", func(t *testing.T) {
-		t.Parallel()
-		opts := ClassifyOptions{
-			EnvName: envName,
-			GetResourceGroupTags: func(_ context.Context, _ string) (map[string]*string, error) {
-				return nil, makeResponseError(http.StatusNotFound)
-			},
-		}
-		res, err := ClassifyResourceGroups(t.Context(), nil, []string{rgA}, opts)
-		require.NoError(t, err)
-		assert.Empty(t, res.Owned)
-		require.Len(t, res.Skipped, 1)
-		assert.Contains(t, res.Skipped[0].Reason, "already deleted")
-	})
-
 	t.Run("Tier2 tag fetch 403 — falls to Tier3 non-interactive skip", func(t *testing.T) {
 		t.Parallel()
 		opts := ClassifyOptions{
@@ -368,23 +353,23 @@ func TestClassifyResourceGroups(t *testing.T) {
 
 	t.Run("Tier3 non-interactive — unknown skipped without prompt", func(t *testing.T) {
 		t.Parallel()
-		prompted := false
+		var prompted atomic.Bool
 		opts := ClassifyOptions{
 			EnvName:     envName,
 			Interactive: false,
 			GetResourceGroupTags: func(_ context.Context, _ string) (map[string]*string, error) {
 				return nil, nil
 			},
 			Prompter: func(_, _ string) (bool, error) {
-				prompted = true
+				prompted.Store(true)
 				return true, nil
 			},
 		}
 		res, err := ClassifyResourceGroups(t.Context(), nil, []string{rgA}, opts)
 		require.NoError(t, err)
 		assert.Empty(t, res.Owned)
 		require.Len(t, res.Skipped, 1)
-		assert.False(t, prompted, "prompter should not be called in non-interactive mode")
+		assert.False(t, prompted.Load(), "prompter should not be called in non-interactive mode")
 	})
 
 	t.Run("multiple RGs — mix of owned, external, unknown", func(t *testing.T) {
@@ -563,7 +548,7 @@ func TestClassifyResourceGroups(t *testing.T) {
 	t.Run("Tier4 foreign resources sequential prompt (not concurrent)", func(t *testing.T) {
 		t.Parallel()
 		rgOp := "Microsoft.Resources/resourceGroups"
-		promptCount := 0
+		var promptCount atomic.Int32
 		opts := ClassifyOptions{
 			EnvName:     envName,
 			Interactive: true,
@@ -573,7 +558,7 @@ func TestClassifyResourceGroups(t *testing.T) {
 				}, nil
 			},
 			Prompter: func(_, _ string) (bool, error) {
-				promptCount++
+				promptCount.Add(1)
 				return false, nil // deny all
 			},
 		}
@@ -584,7 +569,7 @@ func TestClassifyResourceGroups(t *testing.T) {
 		res, err := ClassifyResourceGroups(t.Context(), ops, []string{rgA, rgB}, opts)
 		require.NoError(t, err)
 		assert.Empty(t, res.Owned)
-		assert.Equal(t, 2, promptCount, "both RGs should be prompted sequentially")
+		assert.Equal(t, int32(2), promptCount.Load(), "both RGs should be prompted sequentially")
 	})
 
 	t.Run("Tier4 500 error treated as veto (fail-safe)", func(t *testing.T) {
@@ -1001,6 +986,22 @@ func TestClassifyResourceGroups(t *testing.T) {
 		assert.Contains(t, res.Skipped[0].Reason, "error during safety check")
 	})
 
+	t.Run("Tier4 non-azcore network error on resource listing treated as veto (fail-safe)", func(t *testing.T) {
+		t.Parallel()
+		opts := ClassifyOptions{
+			EnvName: envName,
+			ListResourceGroupResources: func(_ context.Context, _ string) ([]*ResourceWithTags, error) {
+				return nil, fmt.Errorf("dial tcp: connection refused")
+			},
+		}
+		ops := []*armresources.DeploymentOperation{makeOperation("Create", rgOp, rgA)}
+		res, err := ClassifyResourceGroups(t.Context(), ops, []string{rgA}, opts)
+		require.NoError(t, err)
+		assert.Empty(t, res.Owned, "non-azcore error on resource listing should veto")
+		require.Len(t, res.Skipped, 1)
+		assert.Contains(t, res.Skipped[0].Reason, "error during safety check")
+	})
+
 	t.Run("Tier4 extension resource types skipped in foreign check", func(t *testing.T) {
 		t.Parallel()
 		opts := ClassifyOptions{

cli/azd/pkg/azapi/standard_deployments.go

@@ -508,7 +508,7 @@ func (ds *StandardDeployments) voidSubscriptionDeploymentState(
 	}
 
 	envName, has := deployment.Tags[azure.TagKeyAzdEnvName]
-	if has {
+	if has && envName != nil && *envName != "" {
 		var emptyTemplate json.RawMessage = []byte(emptySubscriptionArmTemplate)
 		emptyDeploymentName := ds.GenerateDeploymentName(*envName)
 		tags := map[string]*string{

cli/azd/pkg/infra/provisioning/bicep/bicep_destroy.go

@@ -8,6 +8,8 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"maps"
+	"slices"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
@@ -75,10 +77,7 @@ func (p *BicepProvider) classifyResourceGroups(
 	options provisioning.DestroyOptions,
 ) (owned []string, skipped []azapi.ClassifiedSkip, err error) {
 	// Extract RG names from the grouped resources map.
-	rgNames := make([]string, 0, len(groupedResources))
-	for rgName := range groupedResources {
-		rgNames = append(rgNames, rgName)
-	}
+	rgNames := slices.Collect(maps.Keys(groupedResources))
 
 	// Get deployment info for classification (used for logging and hash derivation).
 	deploymentInfo, deployInfoErr := deployment.Get(ctx)
@@ -116,7 +115,7 @@ func (p *BicepProvider) classifyResourceGroups(
 	subscriptionId := deployment.SubscriptionId()
 	classifyOpts := azapi.ClassifyOptions{
 		Interactive:                !p.console.IsNoPromptMode(),
-		ForceMode:                 options.Force(),
+		ForceMode:                  options.Force(),
 		EnvName:                    p.env.Name(),
 		ExpectedProvisionParamHash: expectedHash,
 	}
@@ -230,7 +229,10 @@ func (p *BicepProvider) deleteRGList(
 // getResourceGroupTags retrieves the tags for a resource group using the ARM API.
 // It uses the service locator to resolve the credential provider and ARM client options.
 // Returns nil tags (no error) as a graceful fallback if dependencies cannot be resolved,
-// which causes the classifier to fall back to Tier 2/3.
+// which causes the classifier to fall to Tier 3 (more scrutiny — safe direction).
+// This differs from listResourceGroupLocks/listResourceGroupResourcesWithTags which
+// return errors → fail-safe veto. The asymmetry is intentional: missing tags means
+// "try harder to verify," while missing lock/resource data means "don't delete."
 func (p *BicepProvider) getResourceGroupTags(
 	ctx context.Context,
 	subscriptionId string,

cli/azd/pkg/infra/provisioning/bicep/bicep_provider_test.go

@@ -1362,9 +1362,9 @@ func httpRespondFn(request *http.Request) (*http.Response, error) {
 
 // classifyMockCfg configures a multi-RG destroy test scenario.
 type classifyMockCfg struct {
-	rgNames            []string                            // RG names referenced in the deployment
-	operations         []*armresources.DeploymentOperation // Tier 1 classification operations
-	withPurgeResources bool                                // adds a KeyVault to each RG for purge testing
+	rgNames            []string                                    // RG names referenced in the deployment
+	operations         []*armresources.DeploymentOperation         // Tier 1 classification operations
+	withPurgeResources bool                                        // adds a KeyVault to each RG for purge testing
 	rgLocks            map[string][]*armlocks.ManagementLockObject // per-RG locks (nil key = empty locks)
 }
 

docs/azd-down-resource-group-safety/architecture.md

@@ -304,10 +304,10 @@ By layering signals, the system tolerates any single signal being unavailable
 or compromised. The key insight is that each tier's failure mode is "skip"
 (safe) not "delete" (unsafe).
 
-**Evaluation order**: Tier 4 (always-on vetoes) runs first because it can
-immediately exclude RGs regardless of what other tiers say. Then Tier 1
-(highest confidence) through Tier 3 (lowest confidence) run in sequence,
-stopping at the first tier that produces a definitive answer.
+**Evaluation order**: Tier 1 (highest confidence, zero API calls) through Tier 3
+(lowest confidence) run in sequence, stopping at the first tier that produces a
+definitive answer. Tier 4 (always-on vetoes) then runs on ALL deletion candidates
+to apply lock and foreign-resource checks regardless of which tier classified them.
 
 ### Decision 2: Deployment Operations as Primary Signal (Tier 1)