docs(agents): propose explicit init and add commands - #9135
Conversation
|
Azure Pipelines: 20 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Thank you for your contribution @hund030! We will review the pull request and get back to you soon. |
There was a problem hiding this comment.
Pull request overview
Proposes separating agent project creation from adding agents to existing projects.
Changes:
- Defines
agent initandagent addcontracts. - Documents source selection and
--infrabehavior. - Records scope and compatibility decisions.
| ### Sources | ||
|
|
||
| ```text | ||
| --template |
| | `init` | No | Yes | Yes | Initialize, then generate IaC. | | ||
| | `init` | No | No | No | Interactive: choose source; non-interactive: fail. | | ||
| | `init` | No | No | Yes | Interactive: choose source, initialize, generate IaC; non-interactive: fail. | | ||
| | `init` | Yes | No | Yes | Generate IaC only. | |
| add + source -> validate existing project -> merge/register agent | ||
| ``` | ||
|
|
||
| Both `init` and `add` share source resolution and agent-registration logic. Registration merges the complete service graph instead of replacing existing Foundry services one at a time. The detailed engineering design defines concurrency, retry, and compatibility mechanics. |
|
|
||
| 3. **Should `init` ever automatically call `add` when it finds a project?** Proposed: No. Fail before mutation and print the exact replacement command. | ||
|
|
||
| 4. **Should existing positional and `--src` forms remain temporarily?** Proposed: Yes, with deprecation warnings and deterministic translation to the new source options. |
jongio
left a comment
There was a problem hiding this comment.
One consistency gap I didn't see raised yet, noted inline on the decision table.
| | `init` | No | Yes | Yes | Initialize, then generate IaC. | | ||
| | `init` | No | No | No | Interactive: choose source; non-interactive: fail. | | ||
| | `init` | No | No | Yes | Interactive: choose source, initialize, generate IaC; non-interactive: fail. | | ||
| | `init` | Yes | No | Yes | Generate IaC only. | |
There was a problem hiding this comment.
The behavioral rules scope IaC-only to an existing compatible project (lines 110-111), and Non-scope rejects unsafe shapes like Aspire projects or multiple Foundry project services. This table only keys on Project found, so init --infra against an incompatible existing project resolves to Generate IaC only here instead of failing. Consider carrying the compatibility gate into the table (or a note) so an unsupported project shape maps to a fail-with-guidance result rather than IaC generation.
|
Hi @@hund030. Thank you for your interest in helping to improve the Azure Developer CLI experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
jongio
left a comment
There was a problem hiding this comment.
One structural note on the two decision surfaces, inline.
| |---|---:|---:|---:|---| | ||
| | `init` | No | Yes | No | Initialize project and agent. | | ||
| | `init` | No | Yes | Yes | Initialize, then generate IaC. | | ||
| | `init` | No | No | No | Interactive: choose source; non-interactive: fail. | |
There was a problem hiding this comment.
The Behavioral rules table above and this Decision table encode the same init/add contract with different columns, and they've already drifted. Behavioral rules restrict standalone --infra to an existing compatible project (lines 116-117), but this table's init | Yes | No | Yes row drops the compatibility qualifier and just says IaC only. The missing bare init in an existing project row that others flagged is the same failure mode. Two hand-maintained tables for one contract will keep diverging as the design evolves. Consider making one the single source of truth and deriving or dropping the other.
jongio
left a comment
There was a problem hiding this comment.
Nothing's changed since my last pass, so the structural issue I raised is still the main thing keeping this from a clean approve: the Behavioral rules table and the Decision table are two hand-maintained encodings of the same init/add contract, and they've already drifted.
Concretely, the Decision table row init | Yes | No | Yes -> Generate IaC only drops the "compatible" qualifier that the Behavioral rules table carries (init --infra in an existing compatible project). Non-scope explicitly rejects unsafe shapes like Aspire projects, multiple Foundry project services, and conflicting shared resources, so an incompatible existing project plus init --infra should fail with guidance, but the Decision table as written routes it to IaC generation. The bare init case (project found, no source, no --infra) is also still missing from the Decision table, which is exactly the parent-project mutation this proposal is trying to make deterministic.
My earlier suggestion stands: make one table the single source of truth and derive or drop the other, so the two surfaces can't keep diverging as the design evolves.
The other open threads are still unaddressed too. --template in the Sources list needs an identifier like core azd init's --template <id-or-url>, otherwise --template --no-prompt still doesn't say which template to use. The "Registration merges the complete service graph" line reads wider than the #8884 non-scope. And review question 4's "deterministic translation" of the legacy positional and --src forms still isn't defined as a concrete mapping. Worth closing those out before this locks.
jongio
left a comment
There was a problem hiding this comment.
Same commit as my last passes, so none of the open review items are addressed yet. Pulling them into one list since the PR is about to age out for inactivity, here's the set I'd want closed before approving:
-
The Behavioral rules and Decision tables encode the same init/add contract twice and have drifted.
init --infrain an existing project is gated on the project being compatible in the rules table (line 116), but the decision table'sinit | Yes | No | Yesrow (line 132) drops that qualifier and just says generate IaC. So an incompatible shape like Aspire or multiple Foundry services, which line 75 puts out of scope, maps to IaC generation here instead of a fail. Pick one table as the source of truth, or carry the compatibility gate into both. -
The decision table is still missing the bare
initin an existing project row: project found, no source, no--infra. That's the exact parent-project case this proposal exists to make deterministic. Spell out that it fails before any mutation and points the user atadd. -
Line 153 says registration merges the complete service graph. That reads like the existing-project
azure.yamlmerge that line 70 puts out of scope and hands to #8884. Narrow the wording so implementers don't pull #8884 into this command split. -
--template(line 100) carries no value, so--template --no-promptcan't say which template to use and collides with the explicit-source requirement on line 107. Coreazd initmodels this as--template <id-or-url>; define the identifier here too. -
Review question 4 (line 173) promises deterministic translation of the legacy positional and
--srcforms but never defines it. A short table mapping each legacy form to its new command and flags would close the migration gap the PR description says is documented.
Once the two tables reconcile and the migration mapping lands, I'm good to approve.
jongio
left a comment
There was a problem hiding this comment.
Took another pass, this time checking the contract against the extension code at the base commit rather than the doc's internal consistency, which is what my earlier passes covered. Two places where the contract doesn't match how the command behaves today, both inline.
The bigger one: the Project found value both tables key on has no single definition in the extension right now. The gate that exists is current-directory-only, while azd's own project resolution walks parents, and those two disagree precisely in the parent-project case the problem statement leads with. Until that predicate is pinned down, every row that depends on it is underdetermined.
The other: --image already ships on init and composes with --manifest, so listing it as a peer source in the Sources block breaks that combination without calling it out as a break.
Everything from my last pass is still open on this commit, so I won't restate it here.
|
|
||
| ## Decision table | ||
|
|
||
| | Command | Project found | Source supplied | `--infra` | Result | |
There was a problem hiding this comment.
Both tables key on a single Project found value, but the extension doesn't have one definition of that today, and the definitions it does have disagree on exactly the parent case this proposal opens with.
At this PR's base commit (19afc57), the standalone eject gate in internal/cmd/init.go is fileExists("azure.yaml"), and fileExists is a plain os.Stat on a relative path (internal/cmd/helpers.go:540), so it only ever sees the current directory. azd's own project resolution walks upward instead: NewAzdContextFromWd in pkg/environment/azdcontext/azdcontext.go searches the working directory and then each parent up to root, and the extension SDK exposes the same behavior through FindFileUpward in pkg/azdext/project.go.
Run init --infra one directory below a project root and those two answers differ: Project found = No under the current gate, Project found = Yes under azd's resolver. The tables route those to opposite outcomes, so the rows don't have a determinate meaning until the predicate is fixed.
Scope says "Detect current and parent projects consistently" but doesn't say which semantics win. Worth stating it directly: cwd-only or upward search, whether init and add resolve it the same way, and what the result is when the nearest project sits above the cwd rather than in it.
| --from-code <directory> | ||
| --manifest <path-or-url> | ||
| --definition <path> | ||
| --image <registry/image[:tag]> |
There was a problem hiding this comment.
--image already ships on init and isn't a standalone source today. At this PR's base commit, internal/cmd/init.go documents two distinct behaviors: set without --manifest, init synthesizes a minimal hosted container manifest and routes it through the manifest flow, and requires --agent-name in that case; set with --manifest, the manifest is the source and --image only overrides the image the manifest carries.
Listing it flat here makes --manifest X --image Y read as two sources, which collides with the one-source requirement on line 107 and with the single source-selection prompt in Scope. That combination works today, so as written this is a silent breaking change for anyone scripting it.
Splitting the entry would fix it: --image is a source only when --manifest is absent, and in that case it also needs --agent-name; otherwise it's a modifier on the manifest source. --manifest has a related gap since it already ships as -m, so the compatibility mapping in review question 4 should say whether the short form survives.
jongio
left a comment
There was a problem hiding this comment.
Two things on the command contract, both grounded in code that landed after this PR's base.
The --infra rows assume a project-resolution predicate that #9291 has since defined, and the terraform variant writes into a parent project's azure.yaml (line 111).
add --infra fails without naming a replacement command, which every other failure row does (line 116).
| | `init <source> --infra` with no project | Create the project/agent, then generate Bicep. | | ||
| | `init <source> --infra=terraform` with no project | Create the project/agent, then generate Terraform. | | ||
| | `init --infra` in an existing compatible project | Generate Bicep only; do not add an agent. | | ||
| | `init --infra=terraform` in an existing compatible project | Generate Terraform only; do not add an agent. | |
There was a problem hiding this comment.
#9291 landed on main one commit after this PR's base, and it settles the Project found question for the --infra path in a way this row doesn't reflect. The standalone eject gate went from fileExists("azure.yaml"), which is cwd-only, to azdext.GetProjectDir(). That reads AZD_EXEC_PROJECT_DIR first and otherwise walks up from cwd via FindFileUpward (cli/azd/pkg/azdext/project.go:22). ejectInfra then writes into the resolved root rather than cwd.
So this row is wider than it reads. Run init --infra=terraform from a subdirectory of an existing project and the eject targets the parent root. For terraform that isn't read-only: ejectInfra stamps infra.provider: terraform into the parent's azure.yaml, and the code comment calls it "the one path that mutates azure.yaml". That's the same shape as the problem on line 18, a user unknowingly modifying a parent project's azure.yaml, except it arrives through --infra instead of agent registration. The goal on line 42 only covers adding an agent, so nothing in the contract rules this out.
Worth stating directly whether standalone --infra uses the same upward resolution, and if it does, whether it should print the resolved project root before writing or require cwd to be the root. AZD_EXEC_PROJECT_DIR is also a third input to Project found that neither table accounts for.
| | `init <source> --infra` in an existing project | Fail and suggest `add <source>`, followed by standalone `init --infra`. | | ||
| | `add <source>` in an existing project | Add the agent without initializing a project. | | ||
| | `add <source>` with no project | Fail and suggest the equivalent `init` command. | | ||
| | `add --infra` | Fail because infrastructure is project-wide and remains an init option. | |
There was a problem hiding this comment.
Every other failure in both tables names the replacement: line 113 gives add <source> followed by standalone init --infra, and the decision table says "Fail with add guidance" (line 127) and "Fail with init guidance" (line 131). This row and its twin on line 132 stop at "fail".
That leaves the goal on line 45 unmet for the case a user is most likely to hit, since add <source> --infra is the natural way to ask for "add this agent and regenerate infra". It also leaves a real string undefined: extension validation errors are built from exterrors.Validation(code, message, suggestion), and the existing init failures all fill the third slot, for example "pass --infra=bicep or --infra=terraform (a bare --infra ejects Bicep)". Saying what this one suggests, presumably add <source> then init --infra, would settle it.
jongio
left a comment
There was a problem hiding this comment.
The behavioral rule for init <source> in an existing project leaves nested project creation with no path, and it diverges from how core azd init deliberately scopes project detection to cwd. Details inline.
| | `init <source> --infra=terraform` with no project | Create the project/agent, then generate Terraform. | | ||
| | `init --infra` in an existing compatible project | Generate Bicep only; do not add an agent. | | ||
| | `init --infra=terraform` in an existing compatible project | Generate Terraform only; do not add an agent. | | ||
| | `init <source>` in an existing project | Fail before mutation and suggest the equivalent `add` command. | |
There was a problem hiding this comment.
This rule closes the silent-mutation hole from line 18, but it also closes the path the user in that scenario actually wanted. The problem statement is a user who intends to create a new project and instead modifies a parent's azure.yaml. Under this row that user gets a failure, and the only replacement offered is add, which mutates the parent. The intent still has nowhere to go.
How wide that is depends on how Project found resolves. If it's the upward walk (azdext.GetProjectDir, then FindFileUpward in cli/azd/pkg/azdext/project.go), the search runs to the filesystem root, so every directory at any depth under an azd project counts as an existing project and can never run init <source>. In a monorepo with azure.yaml at the root, that's every subdirectory.
Core azd init made the opposite call on purpose. On main, cli/azd/cmd/init.go:321 builds its context with azdcontext.NewAzdContextWithDirectory(wd), pinned to cwd, while cli/azd/cmd/container.go:285 gives every other command azdcontext.NewAzdContext() and its upward search. Core init is the one command that deliberately ignores parent projects, so a nested project can still be created. This proposal would make the agents extension stricter than core azd in exactly the case core carved out.
Worth a row or an eighth review question: is nested project creation supported, and if so what is the gesture, a flag or cwd-only resolution for init <source> the way core does it? If it is intentionally unsupported, saying so in Non-scope keeps it from reading as an oversight.
jongio
left a comment
There was a problem hiding this comment.
Two new things this pass, both from checking the contract against the flags agent init actually ships on main rather than the doc's internal consistency. Everything from my earlier passes is still open on this commit, so I won't restate it here.
--manifest already accepts two different kinds of pointer, and routing it through add contradicts review question 7 and the #8884 non-scope (line 96).
[options] on add is never defined, and ten shipping flags have no home in the split (line 88).
| ```text | ||
| --template | ||
| --from-code <directory> | ||
| --manifest <path-or-url> |
There was a problem hiding this comment.
On main today -m/--manifest isn't one source, it's two. The flag's own help text calls it "Path or URI to an agent manifest, or to a sample's unified azure.yaml to adopt as the project manifest" (cli/azd/extensions/azure.ai.agents/internal/cmd/init.go:1541), and the code branches on exactly that at line 1305, detecting whether the pointer is a unified Foundry azure.yaml and adopting it as the project manifest instead of treating it as an agent manifest (#8798).
The Sources block lists it flat, and add <source> on line 114 takes any source in an existing project. So add --manifest <sample azure.yaml> resolves to adopting a complete project manifest into an existing project. Line 64 hands that exact case to #8884, and review question 7 answers it directly with unified-project adoption "for new projects only". The contract as written routes it the other way.
Neither table catches this, because both key on Source supplied as a yes/no and can't tell the two things --manifest accepts apart.
Splitting the entry would close it: --manifest <agent-manifest> is a source for both commands, unified azure.yaml adoption stays init-only per review question 7, and add rejects a unified pointer with a message pointing at #8884.
|
|
||
| ```text | ||
| azd ai agent init [source] [--infra[=bicep|terraform]] [options] | ||
| azd ai agent add [source] [options] |
There was a problem hiding this comment.
[options] never gets defined, which matters more here than on init because add becomes the only way to touch an existing project.
init ships 14 flags on main. This doc accounts for four of them: --infra, --manifest, --image, and --src via review question 4. The other ten don't appear anywhere: --agent-name, --deploy-mode, --runtime, --entry-point, --dep-resolution, --protocol, --project-id, --model, --model-deployment, --force.
Most of those shape the agent rather than the project, so they either survive on add or the capability disappears for existing projects. One is a hard dependency rather than a nice to have: --image requires --agent-name when --manifest is absent (init.go:71). Under this split, today's init --image X --agent-name Y against an existing project becomes add --image X, and --agent-name has no declared home, so the add contract can't express a working call to its own --image source.
--project-id cuts the other way. It's the brownfield Foundry input that Decision 7 and review question 6 are about, so whether it lives on add decides if an agent can be attached to an already provisioned Foundry project.
azd-update.md in this same directory handles this with a flag table. One row per shipping flag with its owning command would settle the add surface and close review question 4's migration mapping at the same time.
jongio
left a comment
There was a problem hiding this comment.
One more gap in the command contract: the [source] slot is written positionally in both signatures, but every source defined below it is a flag, and that slot already means something else on main today.
Detail inline.
| ### Commands | ||
|
|
||
| ```text | ||
| azd ai agent init [source] [--infra[=bicep|terraform]] [options] |
There was a problem hiding this comment.
[source] is written positionally here and in the add signature on the next line, and both tables follow it with init <source> / add <source>. But none of the five entries in the Sources block is positional. They're all flags. No row in either table can be typed as written.
The slot isn't free either. On main, init's Use is init [<path>] [-m <manifest pointer>] [--src <source directory>] with Args: cobra.MaximumNArgs(1), and applyPositionalArg resolves that one positional into either --manifest or --src depending on what it points at. Review question 4 proposes keeping that form temporarily with deprecation warnings, so this signature makes the spelling you're deprecating the canonical one.
add has the sharper version of the problem. It's a new command with no legacy positional to preserve, so add [source] hands it a slot that nothing in the doc defines.
There's also a rule neither table can express. Today the positional and its matching flag are mutually exclusive: applyPositionalArg returns CodeConflictingArguments for init <path> -m <manifest>. If the positional survives on either command, that rule survives with it, and a yes/no Source supplied column can't say which spelling was used, let alone that both at once is an error.
Writing the signatures as init --<source> / add --<source> and giving the legacy positional its own row would settle all three.
Summary
Design proposal for #8383.
azd ai agent initcurrently creates or modifies projects based on directory context, making behavior difficult to predict and debug.Changes
agent init/add contract: separate new-project initialization from adding agents to existing projects.--infracompatibility: retain post-init and standalone IaC generation with explicit decision rules.review decisions: document scope, migration behavior, command matrix, and open product questions.