fix: azd down returns error instead of prompting init steps when no environment exists#6850
Closed
fix: azd down returns error instead of prompting init steps when no environment exists#6850
Conversation
Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
…nvironment exists Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix azd down command prompts in directory without .azure file
fix: azd down returns error instead of prompting init steps when no environment exists
Feb 23, 2026
vhvb1989
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
tg-msft,
therealjohn,
trangevi,
trrwilson,
wbreza and
weikanglim
as code owners
Member
|
/azp run azure-dev - cli |
|
Azure Pipelines successfully started running 1 pipeline(s). |
vhvb1989
reviewed
Feb 23, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates azd down to avoid triggering interactive environment initialization when no local environments exist, returning an immediate error instead and deferring environment/provisioning resolution until needed.
Changes:
- Switch
downActionto resolve environment and provisioning manager lazily rather than via eager IoC injection. - Add an early
envManager.List()guard inazd downto return a “no environments found” error when.azure/envs are missing. - Add a unit test for the no-environments path and update DI wiring to provide a lazy
*provisioning.Manager.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/cmd/down.go | Adds early “no envs” check; switches to lazy env/provisioning resolution in Run. |
| cli/azd/cmd/container.go | Registers *lazy.Lazy[*provisioning.Manager] as a scoped dependency. |
| cli/azd/cmd/down_test.go | Adds unit test covering the no-environments error path. |
| cli/azd/extensions/microsoft.azd.concurx/go.mod | Module dependency tidy/update (adds/removes indirect deps). |
| cli/azd/extensions/microsoft.azd.concurx/go.sum | Module dependency checksum updates corresponding to go.mod changes. |
| cli/azd/extensions/azure.ai.agents/go.mod | Promotes testify to a direct dependency (used by tests in that module). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Member
|
@copilot apply changes based on the comments in this thread |
…ceLocator, improve error messages and tests Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
Contributor
Author
Addressed in 06fa2aa:
Done in commit 06fa2aa:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
|
@copilot can you revert the go.mod changes in extensions? |
Co-authored-by: JeffreyCA <9157833+JeffreyCA@users.noreply.github.com>
Contributor
Author
JeffreyCA
reviewed
Feb 24, 2026
wbreza
requested changes
Mar 3, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Code Review Summary
| Priority | Count |
|---|---|
| Critical | 1 |
| High | 2 |
| Medium | 2 |
| Low | 1 |
| Total | 6 |
Overall Assessment: Request Changes
Key Finding
The fundamental issue is architectural: the HooksMiddleware attached to azd down in root.go:387 eagerly resolves *environment.Environment through the IoC container, triggering LoadOrInitInteractive() (container.go:253) before downAction.Run() ever executes. This means the early environment check and lazy resolution added in this PR are completely bypassed.
Additionally, serviceLocator.Resolve(&provisionManager) will also trigger *environment.Environment resolution because provisioning.NewManager takes env *environment.Environment as a direct constructor parameter.
What Looks Good
- Test structure with
failServiceLocatorhelper properly verifies lazy values aren't prematurely evaluated - Error messages are actionable and guide users to specific commands
ErrNotFound/ErrNameNotSpecifiederror type checking is clean and idiomatic Go
cli/azd/cmd/down.go
Outdated
| if err := a.serviceLocator.Resolve(&provisionManager); err != nil { | ||
| return nil, fmt.Errorf("getting provisioning manager: %w", err) | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
[High] serviceLocator.Resolve for provisioning.Manager will also trigger interactive environment resolution
provisioning.NewManager takes env *environment.Environment as a direct constructor parameter (manager.go:428). Resolving it via the service locator will trigger the same eager *environment.Environment registration that calls LoadOrInitInteractive(). The serviceLocator approach defers the call site but not the resolution chain.
Per @vhvb1989's earlier review comment, *provisioning.Manager can be injected directly without lazy/serviceLocator wrapping. But if the goal is true lazy resolution, the provisioning.Manager itself would need to accept *lazy.Lazy[*environment.Environment].
Contributor
Author
There was a problem hiding this comment.
Fixed in d8c9ec5. Changed provisioning.Manager to hold *lazy.Lazy[*environment.Environment] instead of *environment.Environment. NewManager now accepts the lazy type, and Deploy()/Destroy() resolve the env via m.lazyEnv.GetValue() at call time. Since the container's MustRegisterScoped(provisioning.NewManager) uses function-based IoC registration, it now automatically injects *lazy.Lazy[*environment.Environment] (non-interactive) instead of *environment.Environment (interactive) — no container change needed.
…vironment.Environment] Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
Contributor
Author
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…, inject directly, add happy path test Co-authored-by: vhvb1989 <24213737+vhvb1989@users.noreply.github.com>
JeffreyCA
reviewed
Mar 4, 2026
| type HooksMiddleware struct { | ||
| envManager environment.Manager | ||
| env *environment.Environment | ||
| lazyEnv *lazy.Lazy[*environment.Environment] |
Contributor
There was a problem hiding this comment.
Can we make sure this doesn't regress #3920?
This area has gone through a lot of churn recently:
Contributor
There was a problem hiding this comment.
I think it regressed, now it just errors out with environment not found:
wbreza
reviewed
Mar 4, 2026
Contributor
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Re-review Summary (PR #6850)
Previous Critical — Resolved
The eager env resolution issue in HooksMiddleware and provisioning.Manager has been addressed. Both now use *lazy.Lazy[*environment.Environment] with resolution deferred to execution time.
IoC Scoping — Verified Safe
provisioning.Manager: Scoped*lazy.Lazy[*environment.Environment]: Scoped (fresh per scope)downAction: Transient- No captive dependency issues.
#3920 Regression Risk — Analyzed, No Regression
Re: @JeffreyCA's concern about #3920 (hooks in CI/CD with no environment):
The behavior is functionally equivalent before and after this PR:
- Before: IoC eagerly resolves
*environment.Environment— if env can't be created, the command fails at IoC time (hooks never run) - After: IoC defers resolution via lazy — if env can't be created,
lazyEnv.GetValue()returns error when hooks try to execute (same outcome)
In both cases, HooksRunner.execHook() calls h.env.Dotenv() which panics on nil env, so nil-env was never supported. #3920's fix was about ensuring env creation happens in CI/CD (via AZURE_ENV_NAME), not about hooks running without an environment.
What Looks Good
- Lazy resolution pattern is clean and consistent
- Error messages are actionable with specific command suggestions
- 4 new tests cover the key scenarios
- Existing tests updated cleanly with
lazy.From(env)
JeffreyCA
requested changes
Mar 5, 2026
Member
|
will revisit in the future |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
azd downincorrectly triggered interactive environment creation prompts (env name, subscription, location) when run in a directory with no.azurefolder. There were two architectural root causes:HooksMiddlewareaccepted*environment.Environmentas a direct constructor parameter, causing the IoC container to callLoadOrInitInteractive()beforedownAction.Run()ever executed — completely bypassing any early environment checks in the action itself.provisioning.NewManageralso accepted*environment.Environmentdirectly, meaning any resolution of*provisioning.Manager(whether via IoC or service locator) would transitively triggerLoadOrInitInteractive().Changes
cmd/middleware/hooks.goHooksMiddlewareto accept*lazy.Lazy[*environment.Environment]instead of*environment.EnvironmentregisterCommandHooks()andregisterServiceHooks(), only when hooks are actually configured and need to rundownAction.Run()proceeds to its own early checkcmd/middleware/hooks_test.gorunMiddlewaretest helper to passlazy.From(env)instead ofenvdirectlypkg/infra/provisioning/manager.goManagerstruct andNewManagerconstructor to accept*lazy.Lazy[*environment.Environment]instead of*environment.EnvironmentDeploy()andDestroy()now resolve the environment lazily viam.lazyEnv.GetValue()at actual use timeMustRegisterScoped(provisioning.NewManager)automatically injects*lazy.Lazy[*environment.Environment](non-interactive) instead of*environment.Environment(interactive) — no container change neededpkg/infra/provisioning/manager_test.goNewManagercalls to passlazy.From(env)instead ofenvcmd/down.go*environment.Environmenteager injection — uses*lazy.Lazy[*environment.Environment](non-interactiveenvManager.Get(), respects-eflag and default env)ioc.ServiceLocatorpattern —*provisioning.Manageris now injected directly (safe sinceNewManagertakes lazy env, no prompting at construction time)envManager.List()pre-check — relies solely onlazyEnv.GetValue()error handlinginternal.ErrorWithSuggestionerrors for all failure cases:-e <missing>):"environment not found"/Run "azd env list" to see available environments..."no environment selected"/Run "azd init" or "azd env new" to create an environment, "azd env select" to set a default, or "azd down -e <name>"cmd/down_test.goerrors.As(err, &suggestionErr)to assert both the error message and the actionable suggestion textErrNameNotSpecified), environment not found (-e <missing>), no default environment selected, and a happy path test verifying that when a valid env exists the action passes all env checks and proceeds to provisioningOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.