Fix init with project-id when agent.yaml doesn't contain a model resource#7410
Fix init with project-id when agent.yaml doesn't contain a model resource#7410therealjohn wants to merge 2 commits intoAzure:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes azd ai agent init --project-id being ignored when the agent manifest contains no model resources, so init can still derive location and Foundry environment variables from the provided Foundry project instead of prompting to create new resources.
Changes:
- Update
configureModelChoice()to honor--project-ideven when the manifest has no model resources by selecting the Foundry project (and deriving location/env vars) before returning early. - Change the agent template catalog URL constant used by the template-init flow.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cli/azd/extensions/azure.ai.agents/internal/cmd/init.go |
Ensures the no-model-resources init path still uses --project-id to select a Foundry project and set location/env vars, avoiding the incorrect “prompt for location / create new resources” flow. |
cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_templates_helpers.go |
Updates the remote template catalog URL constant (currently to a non-Microsoft, branch-based raw GitHub URL). |
| ) | ||
|
|
||
| const agentTemplatesURL = "https://aka.ms/foundry-agents" | ||
| const agentTemplatesURL = "https://raw.githubusercontent.com/therealjohn/awesome-azd/refs/heads/agent-tooling-templates/website/static/agent-templates.json" |
There was a problem hiding this comment.
agentTemplatesURL was changed from the stable aka.ms link to a raw.githubusercontent.com/therealjohn/.../refs/heads/... URL. This points to a personal repo/branch and is not a stable or trusted distribution channel, creating a supply-chain and reliability risk (and it’s unrelated to the PR’s stated goal). Please revert to the official Microsoft-owned endpoint (e.g., the previous https://aka.ms/foundry-agents), or move the catalog to a Microsoft-controlled repo and reference an immutable tag/commit via an aka.ms redirect.
| const agentTemplatesURL = "https://raw.githubusercontent.com/therealjohn/awesome-azd/refs/heads/agent-tooling-templates/website/static/agent-templates.json" | |
| const agentTemplatesURL = "https://aka.ms/foundry-agents" |
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7410
Fix init with project-id when agent.yaml doesn't contain a model resource by @therealjohn
Summary
What: When --project-id is provided but the agent manifest has no model resources, configureModelChoice now calls ensureSubscription + selectFoundryProject (same as the "existing project" path) instead of skipping straight to ensureSubscriptionAndLocation. This makes the no-model-resources path honor --project-id and derive location + Foundry env vars from the project.
Why: Fixes #7409 - the no-model-resources branch was ignoring --project-id and always prompting for a new location.
Assessment: The fix correctly mirrors the existing-project pattern and addresses the root cause identified in the issue. The logic is sound. One concern: the URL constant change in init_from_templates_helpers.go is unrelated to the bug fix and points to a personal repo branch (already flagged by existing review).
Findings
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic | 0 | 0 | 1 | 0 |
| Tests | 0 | 0 | 0 | 1 |
| Refactoring | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 1 | 2 |
Key Findings
- [MEDIUM] Logic:
selectedProject == nilguard is unreachable whenprojectResourceIdis set -selectFoundryProjectalways returns a project or an error in that case. See inline comment. - [LOW] Tests: No regression test for the exact scenario that was broken (no model resources + --project-id). No tests exist for
configureModelChoiceat all. - [LOW] Refactoring: The ensureSubscription + selectFoundryProject + ensureLocation pattern is duplicated between this new block and the "existing" model config case (line 590-618). Could extract a shared helper.
Test Coverage Estimate
- No tests exist for
configureModelChoice- the function is untested. - The new branch (projectResourceId + no model resources) has zero coverage.
- Both the happy path (project found, env vars configured) and error paths (invalid project ID, API failure) aren't covered.
What's Done Well
- Fix correctly mirrors the existing-project pattern from the "existing" model config case.
- Comments updated to explain the new --project-id behavior clearly.
- Error handling is consistent with the rest of the function.
1 inline comment below.
| if selectedProject == nil { | ||
| if err := ensureLocation(ctx, a.azdClient, a.azureContext, a.environment.Name); err != nil { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
[MEDIUM] Logic: Unreachable nil-project guard
When projectResourceId is explicitly provided, selectFoundryProject calls getFoundryProject which either returns a valid project (success) or returns an error (failure). It can't return (nil, nil) - doing so would panic at line 926 of init_foundry_resources_helpers.go (*project dereference).
This selectedProject == nil block is dead code in the projectResourceId != "" branch. It's cargo-culted from the "existing" model config case (line 610) where it handles the interactive "Create new" choice - which doesn't apply when a specific project ID is passed.
Not harmful, but misleading to a reader who might think nil is a valid outcome here. Consider replacing with a defensive error:
| if selectedProject == nil { | |
| if err := ensureLocation(ctx, a.azdClient, a.azureContext, a.environment.Name); err != nil { | |
| return nil, err | |
| } | |
| } | |
| if selectedProject == nil { | |
| return nil, fmt.Errorf("foundry project not found: %s", a.flags.projectResourceId) | |
| } |
wbreza
left a comment
There was a problem hiding this comment.
PR Review — #7410
Fix init with project-id when agent.yaml doesn't contain a model resource by @therealjohn
Summary
What: When --project-id is provided but the agent manifest has no model resources, configureModelChoice now calls ensureSubscription + selectFoundryProject (mirroring the existing-project pattern) to derive location and Foundry env vars from the specified project instead of prompting.
Why: Fixes #7409 — the no-model-resources path was ignoring --project-id and always prompting for location.
Assessment: The bug fix is correct and mirrors the established pattern well. One concern: the template URL constant change is unrelated to the fix and points to a personal repo branch.
Findings
| Priority | Count |
|---|---|
| High | 1 |
| Low | 2 |
| Total | 3 |
What's Done Well
- Fix correctly mirrors the existing-project pattern from the "existing" model config case
- Updated comment clearly explains the
--project-idbehavior - All error paths properly propagated
Review performed with GitHub Copilot CLI
| ) | ||
|
|
||
| const agentTemplatesURL = "https://aka.ms/foundry-agents" | ||
| const agentTemplatesURL = "https://raw.githubusercontent.com/therealjohn/awesome-azd/refs/heads/agent-tooling-templates/website/static/agent-templates.json" |
There was a problem hiding this comment.
[HIGH] Template URL changed to personal repo branch — unrelated to bug fix
This changed from https://aka.ms/foundry-agents (stable Microsoft shortlink) to a raw GitHub URL on a personal repo's feature branch. Risks:
- Personal repos can be deleted/renamed
- Feature branch
agent-tooling-templatesmay be deleted after merge - The
aka.mslink was a stable, owned endpoint
Consider reverting this change and either updating the aka.ms redirect target, or moving this URL change to a separate PR once the content is published to an official endpoint.
Copilot created
Fixes #7409