fix(agents): resolve project root for infra eject - #9291
Conversation
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Fixes infrastructure ejection after agent initialization and resolves the owning project root from nested directories.
Changes:
- Chains ejection after unified-manifest adoption and definition reuse.
- Uses upward project discovery.
- Adds project-resolution tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
init.go |
Connects init flows to infrastructure ejection. |
init_infra.go |
Resolves the project root before ejection. |
init_infra_test.go |
Tests parent-project and missing-project behavior. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/init_infra.go:92
- azd-code-reviewer: This suppresses every validation error from
findFoundryServiceForEject, not only the intended no-Foundry case. For example, unified manifests are adopted when they contain at least one Foundry service, but the eject helper rejects multiple Foundry project services; this branch would then report success without generatinginfra/, recreating the silent failure this PR addresses. Ignore onlyCodeInfraEjectNoFoundryServiceand propagate other errors.
if _, err := findFoundryServiceForEject(rawYAML); err != nil {
return nil
jongio
left a comment
There was a problem hiding this comment.
Left one optional, non-blocking suggestion inline about test coverage for the post-init no-foundry-service skip path.
jongio
left a comment
There was a problem hiding this comment.
Re-reviewed at db9d0e9. The propagation fix now only swallows the no-foundry-service case and surfaces every other validation error, so a malformed or ambiguous manifest no longer skips eject silently. Resolving the root through GetProjectDir matches how the rest of azd finds azure.yaml, and the new tests (parent-project, no-project, multiple-foundry-service) pass locally. My earlier note about a test for the silent-skip branch is still optional, not blocking.
jongio
left a comment
There was a problem hiding this comment.
Reconfirming approval. --infra now chains through ejectInfraAfterInit with upward project resolution, so eject runs after adopt and bare agent.yaml reuse, and from a nested source directory it writes infra/ at the parent project root. The foundry check only swallows the infra_eject_no_foundry_service code, so malformed or multi-service manifests still surface as errors instead of silently skipping. New tests cover the parent-project, no-project, and error-propagation paths, and CI is green.
jongio
left a comment
There was a problem hiding this comment.
Post-init eject now resolves the owning azd project via upward discovery, so --infra works from a nested source directory and writes infra/ beside the project's azure.yaml. The validation-error handling in ejectInfraAfterInit skips only the no-foundry-service case and propagates the multiple-foundry-service and invalid-manifest cases, so --infra no longer reports success without generating infra when the manifest is ambiguous.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
cli/azd/extensions/azure.ai.agents/internal/cmd/init.go:1131
- azd-code-reviewer: The parent-project regression is not covered at this branch.
TestEjectInfraAfterInit_ResolvesParentProjectcalls the post-init helper directly, so it would still pass if standalone eject reverted to the old cwd-only check. Add a command-level test that invokesinit --infrafrom a nested directory and verifiesinfra/is created at the parent project root.
projectRoot, projectRootErr := azdext.GetProjectDir()
cli/azd/extensions/azure.ai.agents/internal/cmd/init.go:1293
- azd-code-reviewer: This fixes the bare-definition early return, but no automated test executes a successful reuse flow with
--infra; the existing reuse tests explicitly cover only pre-gRPC failures. Add an integration or command-level regression test that starts with a bareagent.yamland asserts that bothazure.yamlandinfra/main.bicepare produced.
return ejectInfraAfterInit(infraProvider)
cli/azd/extensions/azure.ai.agents/internal/cmd/init.go:1319
- azd-code-reviewer: The unified-manifest regression was an orchestration bug at this early return, but the added helper tests do not execute this path. Add an automated command/integration test for unified
azure.yamladoption with--infrathat verifies the adopted project receivesinfra/main.bicep; otherwise removing this call would leave the suite green.
return ejectInfraAfterInit(infraProvider)
jongio
left a comment
There was a problem hiding this comment.
Re-reviewed at 976d2f2. Both open points are closed.
TestEjectInfraAfterInit_SkipsProjectWithoutFoundryService covers the post-init skip path I asked about. Since TestEjectInfraAfterInit_NoProject already covers project-not-found and this case writes an azure.yaml, the only way to reach a nil return is the CodeInfraEjectNoFoundryService swallow, so the branch is pinned.
The stdout capture on TestEjectInfraAfterInit_ResolvesParentProject is fixed. Before the wrap, go test -run TestEjectInfraAfterInit -v printed the whole eject summary between === RUN and --- PASS; at 976d2f2 that output is gone. forbidigo would not have caught it, since the write comes from printEjectSummary in production code rather than from the test, so the capture is the only guard.
All of internal/cmd passes locally, go vet ./... is clean, and CI is green on this commit.
jongio
left a comment
There was a problem hiding this comment.
Re-checked at 976d2f2. The code is correct and go test ./internal/cmd/ is green, but scenario 3 from #9287 (standalone eject from a subdirectory) has no regression guard. I reverted this branch locally to the old cwd-only lookup and the entire package suite still passed. Details inline, with a test that fails on the old behavior.
jongio
left a comment
There was a problem hiding this comment.
Incremental change since my last review is the new TestInitInfra_StandaloneEjectResolvesParentProject.
I verified it actually covers the branch I flagged rather than just passing alongside it. Reverting init.go back to fileExists("azure.yaml") plus os.Getwd() makes that test fail (--- FAIL: TestInitInfra_StandaloneEjectResolvesParentProject), while TestEjectInfraAfterInit_ResolvesParentProject still passes. So the standalone eject path from a subdirectory (scenario 3 in #9287) is genuinely covered now, and the rest of the package is green.
I also checked the --infra plus -m/--src combination from a nested directory. It returns the conflicting-arguments validation error, which matches what already happened from the project root, so this makes that behavior consistent instead of adding a new restriction.
…leases (#9371) * chore: prepare azure.ai.agents 1.0.0-beta.8 release Bump version and add changelog for 10 PRs since beta.7: - Features: max_stalls early-stopping (#9314), invocations protocol 2.0.0 default (#9327) - Bug fixes: #9365, #9328, #9291, #9290, #9212, #9211, #9280, #9237 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * chore: prepare azure.ai.projects 1.0.0-beta.4 release Bump version and add changelog for 1 PR since beta.3: - Bug fix: cap Foundry ARM deployment names to 64 chars (#9292) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * chore: add imatiach to cspell contributor aliases Fix cspell-ext CI gate failure for the azure.ai.agents CHANGELOG entry referencing @imatiach-msft (PR #9314). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
Fixes #9287.
azd ai agent init --infranow generates infrastructure after unified-manifest adoption and bare-definition reuse, including when invoked below the azd project root.Changes
agent initflow: chain infrastructure ejection after successful unifiedazure.yamladoption and bareagent.yamlreuse.infra/beside itsazure.yaml.Manual Validation
--infra --no-prompt; Bicep files were generated in the adopted project.--infrafrom a nested agent source directory; files were generated at the parent project root, with no nestedinfra/.agent.yamlwith--infra --no-prompt; init createdazure.yamland generatedinfra/main.bicep.