Improve Windows test reliability

cli/azd/AGENTS.md

@@ -359,7 +359,10 @@ This project uses Go 1.26. Use modern standard library features:
 ### Testing Best Practices
 
 - **Test the actual rules, not just the framework**: When adding YAML-based error suggestion rules, write tests that exercise the rules end-to-end through the pipeline, not just tests that validate the framework's generic matching behavior
-- **Extract shared test helpers**: Don't duplicate test utilities across files. Extract common helpers (e.g., shell wrappers, auth token fetchers, CLI runners) into shared `test_utils` packages. Duplication across 3+ files should always be refactored
+- **Search for prior art before adding helpers**: Search `test/`, `pkg/osutil`, and the package under test before adding cleanup, retry, process, or platform-specific helpers. Reuse or promote existing behavior and extract shared test utilities instead of creating parallel implementations
+- **Isolate persistent user state**: Functional tests that install extensions or mutate config, auth, cache, or other user-level state must set `AZD_CONFIG_DIR` to a dedicated `tempDirWithDiagnostics(t)` directory. Never let parallel tests read or modify the developer/agent's default `~/.azd` state
+- **Make cleanup process-safe**: Wait for spawned commands to finish, register cleanup when state is created, and report failures. Use `osutil.RemoveAll` for directories containing executed binaries because Windows may retain transient locks; keep retries at the filesystem boundary instead of adding test sleeps. Since `t.Context()` is canceled before cleanup runs, use a separate bounded context when cleanup needs one
+- **Guard fixture type assertions**: Check presence and type with `require` before casting values loaded from maps, JSON, environment files, or recordings. A raw type assertion panic can abort the package and hide the initiating failure
 - **Use correct env vars for testing**:
   - Non-interactive mode: `AZD_FORCE_TTY=false` (not `AZD_DEBUG_FORCE_NO_TTY`, which doesn't exist)
   - No-prompt mode: use the `--no-prompt` flag for core azd commands; `AZD_NO_PROMPT=true` is only used for propagating no-prompt into extension subprocesses
@@ -370,7 +373,7 @@ This project uses Go 1.26. Use modern standard library features:
 - **Cross-platform paths**: When resolving binary paths in tests, handle `.exe` suffix on Windows (e.g., `azd` vs `azd.exe` via `process.platform === "win32"`)
 - **Test new JSON fields**: When adding fields to JSON command output (e.g., `expiresOn` in `azd auth status --output json`), add a test asserting the field's presence and format
 - **No unused dependencies**: Don't add npm/pip packages that aren't imported anywhere. Remove dead `devDependencies` before submitting
-- **Never write to `os.Stdout` in tests**: Tests that write directly to `os.Stdout` (via `fmt.Print*`, `os.Stdout`, or UX components like `ux.NewSpinner` without a `Writer` option) corrupt the `go test -json` event stream under parallel execution, causing phantom test failures in CI. Use `t.Log`/`t.Logf` for diagnostic output, `io.Discard` or `&bytes.Buffer{}` for UX component writers, and `SkipLoadingSpinner: true` for prompt tests that don't need spinner behavior. Enforced by the `forbidigo` linter in `.golangci.yaml`. See [#8385](https://github.com/Azure/azure-dev/issues/8385) for details.
+- **Keep terminal output on the injected writer**: Commands and UX components must pass the caller's `io.Writer` to every nested spinner, prompt, cursor, and formatter. In tests, use `t.Log`/`t.Logf` for diagnostics and `io.Discard` or `&bytes.Buffer{}` for UX writers. Raw stdout corrupts `go test -json` and creates phantom CI failures. `forbidigo` blocks direct writes in tests, but production writer propagation still needs buffer-based coverage. See [#8385](https://github.com/Azure/azure-dev/issues/8385)
 
 ## MCP Tools
 

cli/azd/cmd/extension.go

@@ -1825,7 +1825,7 @@ func (a *extensionUninstallAction) Run(ctx context.Context) (*actions.ActionResu
 		stepMessage += fmt.Sprintf(" (%s)", installed.Version)
 		a.console.ShowSpinner(ctx, stepMessage, input.Step)
 
-		if err := a.extensionManager.Uninstall(extensionId); err != nil {
+		if err := a.extensionManager.Uninstall(ctx, extensionId); err != nil {
 			a.console.StopSpinner(ctx, stepMessage, input.StepFailed)
 			return nil, fmt.Errorf("failed to uninstall extension: %w", err)
 		}

cli/azd/cmd/tool.go

@@ -183,15 +183,18 @@ func toolActions(root *actions.ActionDescriptor) *actions.ActionDescriptor {
 type toolAction struct {
 	manager *tool.Manager
 	console input.Console
+	writer  io.Writer
 }
 
 func newToolAction(
 	manager *tool.Manager,
 	console input.Console,
+	writer io.Writer,
 ) actions.Action {
 	return &toolAction{
 		manager: manager,
 		console: console,
+		writer:  writer,
 	}
 }
 
@@ -206,6 +209,7 @@ func (a *toolAction) Run(ctx context.Context) (*actions.ActionResult, error) {
 	spinner := uxlib.NewSpinner(&uxlib.SpinnerOptions{
 		Text:        "Detecting tools...",
 		ClearOnStop: true,
+		Writer:      a.writer,
 	})
 	if err := spinner.Run(ctx, func(ctx context.Context) error {
 		var detectErr error
@@ -374,6 +378,7 @@ func (a *toolListAction) Run(ctx context.Context) (*actions.ActionResult, error)
 		spinner := uxlib.NewSpinner(&uxlib.SpinnerOptions{
 			Text:        "Checking tool status...",
 			ClearOnStop: true,
+			Writer:      a.writer,
 		})
 		if err := spinner.Run(ctx, func(ctx context.Context) error {
 			var detectErr error
@@ -897,6 +902,7 @@ func detectAllTools(
 	ctx context.Context,
 	manager *tool.Manager,
 	formatter output.Formatter,
+	writer io.Writer,
 	spinnerText string,
 ) ([]*tool.ToolStatus, error) {
 	if formatter != nil && formatter.Kind() == output.JsonFormat {
@@ -907,6 +913,7 @@ func detectAllTools(
 	spinner := uxlib.NewSpinner(&uxlib.SpinnerOptions{
 		Text:        spinnerText,
 		ClearOnStop: true,
+		Writer:      writer,
 	})
 	if err := spinner.Run(ctx, func(ctx context.Context) error {
 		var detectErr error
@@ -1080,7 +1087,7 @@ func (a *toolInstallAction) resolveToolIds(ctx context.Context) ([]string, error
 
 	// --all: install all recommended tools that are not already installed.
 	if a.flags.all {
-		statuses, err := detectAllTools(ctx, a.manager, a.formatter, "Detecting tool status...")
+		statuses, err := detectAllTools(ctx, a.manager, a.formatter, a.writer, "Detecting tool status...")
 		if err != nil {
 			return nil, fmt.Errorf("detecting tools: %w", err)
 		}
@@ -1100,7 +1107,7 @@ func (a *toolInstallAction) resolveToolIds(ctx context.Context) ([]string, error
 	}
 
 	// Interactive: let the user pick from uninstalled tools.
-	statuses, err := detectAllTools(ctx, a.manager, a.formatter, "Detecting tool status...")
+	statuses, err := detectAllTools(ctx, a.manager, a.formatter, a.writer, "Detecting tool status...")
 	if err != nil {
 		return nil, fmt.Errorf("detecting tools: %w", err)
 	}
@@ -1439,7 +1446,7 @@ func (a *toolUpgradeAction) Run(ctx context.Context) (*actions.ActionResult, err
 // detectInstalledTools runs DetectAll behind a spinner and returns the full
 // set of tool statuses. Used by the --all and interactive upgrade paths.
 func (a *toolUpgradeAction) detectInstalledTools(ctx context.Context) ([]*tool.ToolStatus, error) {
-	statuses, err := detectAllTools(ctx, a.manager, a.formatter, "Detecting installed tools...")
+	statuses, err := detectAllTools(ctx, a.manager, a.formatter, a.writer, "Detecting installed tools...")
 	if err != nil {
 		return nil, fmt.Errorf("detecting installed tools: %w", err)
 	}
@@ -1757,7 +1764,7 @@ func (a *toolUninstallAction) resolveToolIds(ctx context.Context) ([]string, err
 
 	// --all, --dry-run, and the interactive picker all need the current
 	// installed set.
-	statuses, err := detectAllTools(ctx, a.manager, a.formatter, "Detecting installed tools...")
+	statuses, err := detectAllTools(ctx, a.manager, a.formatter, a.writer, "Detecting installed tools...")
 	if err != nil {
 		return nil, fmt.Errorf("detecting installed tools: %w", err)
 	}
@@ -1920,6 +1927,7 @@ func (a *toolCheckAction) Run(ctx context.Context) (*actions.ActionResult, error
 		spinner := uxlib.NewSpinner(&uxlib.SpinnerOptions{
 			Text:        "Checking for upgrades...",
 			ClearOnStop: true,
+			Writer:      a.writer,
 		})
 		if err := spinner.Run(ctx, func(ctx context.Context) error {
 			var detectErr error
@@ -2118,6 +2126,7 @@ func (a *toolShowAction) Run(ctx context.Context) (*actions.ActionResult, error)
 		spinner := uxlib.NewSpinner(&uxlib.SpinnerOptions{
 			Text:        fmt.Sprintf("Checking %s...", toolDef.Name),
 			ClearOnStop: true,
+			Writer:      a.writer,
 		})
 		if err := spinner.Run(ctx, func(ctx context.Context) error {
 			var detectErr error

cli/azd/cmd/tool_test.go

@@ -330,6 +330,106 @@ func TestToolShowAction_InvalidArgDoesNotEmitToolId(t *testing.T) {
 		"tool.id must not be emitted when FindTool fails on an unknown tool")
 }
 
+func TestToolActionSpinnersUseInjectedWriter(t *testing.T) {
+	detectErr := errors.New("detect failed")
+
+	tests := []struct {
+		name string
+		run  func(context.Context, *bytes.Buffer) error
+	}{
+		{
+			name: "bare tool action",
+			run: func(ctx context.Context, writer *bytes.Buffer) error {
+				detector := &cmdMockDetector{
+					detectAll: func(context.Context, []*tool.ToolDefinition) ([]*tool.ToolStatus, error) {
+						return nil, detectErr
+					},
+				}
+				action := newToolAction(
+					tool.NewManager(detector, &cmdMockInstaller{}, nil),
+					mockinput.NewMockConsole(),
+					writer,
+				)
+				_, err := action.Run(ctx)
+				return err
+			},
+		},
+		{
+			name: "tool list",
+			run: func(ctx context.Context, writer *bytes.Buffer) error {
+				detector := &cmdMockDetector{
+					detectAll: func(context.Context, []*tool.ToolDefinition) ([]*tool.ToolStatus, error) {
+						return nil, detectErr
+					},
+				}
+				action := newToolListAction(
+					tool.NewManager(detector, &cmdMockInstaller{}, nil),
+					mockinput.NewMockConsole(),
+					&output.TableFormatter{},
+					writer,
+				)
+				_, err := action.Run(ctx)
+				return err
+			},
+		},
+		{
+			name: "tool check",
+			run: func(ctx context.Context, writer *bytes.Buffer) error {
+				detector := &cmdMockDetector{
+					detectAll: func(context.Context, []*tool.ToolDefinition) ([]*tool.ToolStatus, error) {
+						return nil, detectErr
+					},
+				}
+				updateChecker := tool.NewUpdateChecker(
+					&memUserConfigManager{},
+					detector,
+					func() (string, error) { return t.TempDir(), nil },
+					nil,
+				)
+				action := newToolCheckAction(
+					tool.NewManager(detector, &cmdMockInstaller{}, updateChecker),
+					mockinput.NewMockConsole(),
+					&output.TableFormatter{},
+					writer,
+				)
+				_, err := action.Run(ctx)
+				return err
+			},
+		},
+		{
+			name: "tool show",
+			run: func(ctx context.Context, writer *bytes.Buffer) error {
+				detector := &cmdMockDetector{
+					detectTool: func(context.Context, *tool.ToolDefinition) (*tool.ToolStatus, error) {
+						return nil, detectErr
+					},
+				}
+				manager := tool.NewManager(detector, &cmdMockInstaller{}, nil)
+				action := newToolShowAction(
+					[]string{manager.GetAllTools()[0].Id},
+					manager,
+					mockinput.NewMockConsole(),
+					&output.NoneFormatter{},
+					writer,
+				)
+				_, err := action.Run(ctx)
+				return err
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			var writer bytes.Buffer
+			err := test.run(t.Context(), &writer)
+
+			require.ErrorIs(t, err, detectErr)
+			assert.Contains(t, writer.String(), "\033[?25l")
+			assert.Contains(t, writer.String(), "\033[?25h")
+		})
+	}
+}
+
 // lookupToolBoolUsage returns the most recent bool-valued usage attribute
 // for the given key (assumes the key was set via .Bool()).
 func lookupToolBoolUsage(key string) (bool, bool) {
@@ -1275,13 +1375,14 @@ func TestToolUpgradeAction_All_UpgradesInstalledTools(t *testing.T) {
 	}
 	manager := tool.NewManager(detector, installer, nil)
 
+	var writer bytes.Buffer
 	action := newToolUpgradeAction(
 		nil,
 		&toolUpgradeFlags{all: true},
 		manager,
 		mockinput.NewMockConsole(),
 		&output.NoneFormatter{},
-		io.Discard,
+		&writer,
 	)
 
 	_, err := action.Run(t.Context())
@@ -1290,6 +1391,8 @@ func TestToolUpgradeAction_All_UpgradesInstalledTools(t *testing.T) {
 	require.Len(t, installedIDs, 2)
 	assert.ElementsMatch(t, installedIDs, upgradedIDs,
 		"--all must upgrade exactly the installed tools")
+	assert.Contains(t, writer.String(), "\033[?25l")
+	assert.Contains(t, writer.String(), "\033[?25h")
 }
 
 // TestToolUpgradeAction_All_JsonFormat_EmitsCleanJson exercises the reviewer's

cli/azd/docs/recording-functional-tests-guide.md

@@ -718,15 +718,23 @@ if session == nil {
 }
 ```
 
-### 7. Add Debug Logging
+### 7. Isolate State and Use Shared Cleanup
+
+Use a private `AZD_CONFIG_DIR` when a test changes user-level state so parallel tests do not share `~/.azd`. A new config directory is logged out, so tests that need live Azure access must authenticate in it. Leave `AZD_CONFIG_DIR` unset when a test only uses the existing login.
+
+Use `tempDirWithDiagnostics(t)` for generated or executed files. It retries transient Windows file locks and reports
+lock diagnostics if cleanup fails. Register other cleanup when state is created; don't duplicate retry loops or add
+fixed sleeps. Since `t.Context()` is canceled before cleanup runs, use a separate bounded context when needed.
+
+### 8. Add Debug Logging
 
 ```go
 t.Logf("Recording mode: playback=%v", session != nil && session.Playback)
 t.Logf("Environment name: %s", envName)
 t.Logf("Subscription ID: %s", subscriptionId)
 ```
 
-### 8. Handle Skipped Tests Gracefully
+### 9. Handle Skipped Tests Gracefully
 
 **DO**:
 ```go
@@ -1022,10 +1030,12 @@ os.Setenv("AZD_TEST_FIXED_CLOCK_UNIX_TIME", "1744738873")
 - [ ] Start with `recording.Start(t)`
 - [ ] Use `randomOrStoredEnvName(session)`
 - [ ] Create CLI with `azdcli.WithSession(session)`
+- [ ] Use a private `AZD_CONFIG_DIR` for user-level state changes; authenticate it if live Azure access is required
 - [ ] Use `session.ProxyClient` for HTTP operations
 - [ ] Store dynamic values in `session.Variables`
-- [ ] Add cleanup with session checks
+- [ ] Use `tempDirWithDiagnostics(t)` and register other cleanup immediately
 - [ ] Use appropriate timeouts for recording/playback
+- [ ] Guard fixture type assertions so malformed data fails the test without panicking
 
 ### Recording a Test ✓
 - [ ] Set `AZURE_RECORD_MODE=record`
@@ -1056,8 +1066,3 @@ os.Setenv("AZD_TEST_FIXED_CLOCK_UNIX_TIME", "1744738873")
 - **Test Helpers**: `cli/azd/test/azdcli/`
 - **Example Tests**: `cli/azd/test/functional/up_test.go`
 - **go-vcr Documentation**: https://github.com/dnaeon/go-vcr
-
----
-
-**Last Updated**: December 2025  
-**Maintainers**: Azure Developer CLI Team

cli/azd/pkg/extensions/manager.go

@@ -781,8 +781,8 @@ func (m *Manager) installInternal(
 	return selectedVersion, nil
 }
 
-// Uninstall an extension by name
-func (m *Manager) Uninstall(id string) error {
+// Uninstall uninstalls an extension by name.
+func (m *Manager) Uninstall(ctx context.Context, id string) error {
 	// Get the installed extension
 	extension, err := m.GetInstalled(FilterOptions{Id: id})
 	if err != nil {
@@ -795,16 +795,8 @@ func (m *Manager) Uninstall(id string) error {
 	}
 
 	extensionDir := filepath.Join(userConfigDir, "extensions", extension.Id)
-	if err := os.MkdirAll(extensionDir, os.ModePerm); err != nil {
-		return fmt.Errorf("failed to create target directory: %w", err)
-	}
-
-	// Remove the extension artifacts when it exists
-	_, err = os.Stat(extensionDir)
-	if err == nil {
-		if err := os.RemoveAll(extensionDir); err != nil {
-			return fmt.Errorf("failed to remove extension: %w", err)
-		}
+	if err := osutil.RemoveAll(ctx, extensionDir); err != nil {
+		return fmt.Errorf("failed to remove extension: %w", err)
 	}
 
 	// Update the user config
@@ -901,7 +893,7 @@ func (m *Manager) upgradeInternal(
 	opts UpgradeOptions,
 	visited map[string]struct{},
 ) (*ExtensionVersion, []UpgradeResult, error) {
-	if err := m.Uninstall(extension.Id); err != nil {
+	if err := m.Uninstall(ctx, extension.Id); err != nil {
 		return nil, nil, fmt.Errorf("failed to uninstall extension: %w", err)
 	}
 

cli/azd/pkg/extensions/manager_test.go

@@ -188,7 +188,7 @@ func Test_List_Install_Uninstall_Flow(t *testing.T) {
 	require.Greater(t, len(installed), 0)
 
 	// Uninstall the first extension
-	err = manager.Uninstall(extensions[0].Id)
+	err = manager.Uninstall(t.Context(), extensions[0].Id)
 	require.NoError(t, err)
 
 	// List installed extensions (expect 0)
@@ -320,7 +320,7 @@ func Test_Install_With_SemverConstraints(t *testing.T) {
 				require.NotNil(t, extensionVersion)
 				require.Equal(t, tc.Expected, extensionVersion.Version)
 
-				err = manager.Uninstall("test.extension")
+				err = manager.Uninstall(t.Context(), "test.extension")
 				require.NoError(t, err)
 			}
 		})
@@ -2334,7 +2334,7 @@ func Test_Upgrade_DependencyUpgrade_RefusesToDowngradeOutsideConstraint(t *testi
 	require.NoError(t, err)
 
 	// Simulate the user upgrading the dependency standalone to a version outside the constraint.
-	require.NoError(t, manager.Uninstall("test.child"))
+	require.NoError(t, manager.Uninstall(t.Context(), "test.child"))
 	children, err := manager.FindExtensions(*mockContext.Context, &FilterOptions{Id: "test.child"})
 	require.NoError(t, err)
 	_, err = manager.Install(*mockContext.Context, children[0], "2.0.0")
@@ -2560,7 +2560,7 @@ func Test_Upgrade_DependencyUpgrade_EmptyConstraint(t *testing.T) {
 
 	// Force the child to an older version to confirm dependency upgrade does NOT
 	// run when the parent's dependency constraint is empty.
-	_ = manager.Uninstall("test.child")
+	_ = manager.Uninstall(t.Context(), "test.child")
 	childMeta, err := manager.FindExtensions(*mockContext.Context, &FilterOptions{Id: "test.child"})
 	require.NoError(t, err)
 	_, err = manager.Install(*mockContext.Context, childMeta[0], "1.0.0")

cli/azd/pkg/osutil/rename_unix.go

@@ -14,3 +14,8 @@ import (
 func Rename(ctx context.Context, old, new string) error {
 	return os.Rename(old, new)
 }
+
+// RemoveAll removes path and any children it contains.
+func RemoveAll(_ context.Context, path string) error {
+	return os.RemoveAll(path)
+}