Skip to content

docs(agents): add infra layer design spec - #9363

Open
hund030 wants to merge 4 commits into
Azure:mainfrom
hund030:docs/agent-init-infra-layers-spec
Open

docs(agents): add infra layer design spec#9363
hund030 wants to merge 4 commits into
Azure:mainfrom
hund030:docs/agent-init-infra-layers-spec

Conversation

@hund030

@hund030 hund030 commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Design for #9126. Defines how azd ai agent init --infra can add editable Foundry IaC to projects with existing infrastructure without merging or overwriting user-owned files.

Changes

  • agent-init-infra-layers spec: documents project-shape behavior, isolated Foundry layer ownership, safety rules, scope, decisions, and rollout validation.

Manual Validation

  • Reviewed the rendered Markdown on the fork and confirmed the behavior matrix, YAML migration example, headings, and links display correctly.

Copilot AI review requested due to automatic review settings July 30, 2026 05:03
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
22 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The proposed deterministic resource-group name risks destructive collisions, and the environment-key migration and atomicity guarantees need clarification.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Defines a layered IaC design for safely ejecting Foundry infrastructure into projects with existing infrastructure.

Changes:

  • Specifies project-shape behavior and isolated Foundry layer ownership.
  • Defines safety, migration, validation, and rollout requirements.
File summaries
File Description
docs/specs/agent-init-infra-layers/spec.md Adds the infrastructure-layer design specification.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 3
  • Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment on lines +105 to +107
```text
rg-${AZURE_ENV_NAME}-foundry
```
Comment on lines +124 to +125
- Files are generated in a temporary directory, installed atomically, and
removed if the `azure.yaml` update fails.
Comment on lines +109 to +110
It publishes `AZURE_FOUNDRY_RESOURCE_GROUP` rather than replacing the existing
application's `AZURE_RESOURCE_GROUP`. This keeps targeted `azd down foundry`
Copilot AI review requested due to automatic review settings July 30, 2026 07:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The design has unresolved custom-provider migration, resource-group contract, and atomic-install concerns.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (3)

docs/specs/agent-init-infra-layers/spec.md:113

  • The new key needs an input and consumer migration contract, not only an output rename. Current eject parameters feed ${AZURE_RESOURCE_GROUP} to Terraform (init_infra.go:601-605), generated Bicep/Terraform emit that key, and activity-agent bot creation and cleanup read it (listen_activity.go:103,257). In a composed project that value belongs to the sibling application, so provisioning or bot operations can target the wrong resource group unless generated inputs and all consumers use AZURE_FOUNDRY_RESOURCE_GROUP with a legacy fallback for root projects. Add those changes and coverage to the design.
It publishes `AZURE_FOUNDRY_RESOURCE_GROUP` rather than replacing the existing
application's `AZURE_RESOURCE_GROUP`. This keeps targeted `azd down foundry`
from deleting sibling-layer resources.

docs/specs/agent-init-infra-layers/spec.md:56

  • Preserving this shape requires more than per-layer provider. Layers currently require a non-empty path (cli/azd/pkg/infra/provisioning/provider.go:206-208; both schemas also require it) and reject undeclared fields, while custom providers receive provider-specific config (cli/azd/internal/grpcserver/external_provisioning_provider.go:353-362). A root custom/fileless provider with infra.config therefore cannot be moved into a valid layer without inventing a path or dropping configuration. Define the synthetic path and add per-layer config schema support, or make these projects a refusal case.
| Existing custom/fileless provider | Preserve it as a layer and add `foundry` |

docs/specs/agent-init-infra-layers/spec.md:127

  • This all-or-nothing guarantee conflicts with the allowed non-empty infra/foundry target on line 60. A staged directory can be atomically renamed only when azd owns the whole destination; merging several files into an existing user directory requires multiple operations, so interruption can leave partial output and rollback must distinguish preexisting files. Either restrict reusable targets to absent/empty directories and rename once, or define per-file atomicity plus a recovery/rollback manifest rather than promising atomic installation of the entire set.
- Files are generated in a temporary directory, installed atomically, and
  removed if the `azure.yaml` update fails.
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Copilot AI review requested due to automatic review settings July 30, 2026 07:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Not ready to approve

The design permits unintended Terraform merging and does not define how existing consumers safely adopt the new resource-group key.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (2)

docs/specs/agent-init-infra-layers/spec.md:112

  • azd-code-reviewer: The new environment key needs a compatibility and consumer contract. The microsoft.foundry provider currently initializes and destroys from AZURE_RESOURCE_GROUP (foundry_provisioning_provider.go:458,1324), and agent flows such as listen_activity.go:103,257 also read that key. In a composed project they would target the application's sibling resource group instead of Foundry's. Specify that Foundry lifecycle code and consumers prefer AZURE_FOUNDRY_RESOURCE_GROUP with an AZURE_RESOURCE_GROUP fallback for root-layout projects, including how the value exists before first provision and which key targeted teardown invalidates.
It publishes `AZURE_FOUNDRY_RESOURCE_GROUP` rather than replacing the existing
application's `AZURE_RESOURCE_GROUP`. This keeps targeted `azd down foundry`
from deleting sibling-layer resources.

docs/specs/agent-init-infra-layers/spec.md:60

  • azd-code-reviewer: Terraform loads every .tf file in a directory as one module, so adding generated files to a non-empty, unowned infra/foundry directory performs the semantic merge rejected by this design even when filenames do not collide. Refuse a non-empty target (or at minimum any existing Terraform configuration) unless it is already the declared compatible layer.
| Existing `infra/foundry` directory without a layer | Add only non-conflicting files; refuse generated-file collisions |
  • Files reviewed: 1/1 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

@jongio jongio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two checks are red and both block merge.

cspell-lint fails on fileless (line 56). CI runs cspell lint '**/*' --config ./.vscode/cspell.misc.yaml over docs/**, and that word isn't in the global dictionary, has no override for this path, and doesn't appear anywhere else in the repo. Fix is in the inline comment.

PR Governance fails with "PR must be linked to a GitHub issue." .github/scripts/pr-governance-issue-check.js only accepts a closing keyword (Fixes #123, Closes, Resolves) or a sidebar development link. "Design for #9126" and the Source issue link in the body don't match either form. Since this spec on its own doesn't close #9126, the sidebar link is probably the better choice over Fixes.

The rest are design questions, inline.

|---|---|
| New or Foundry-only project | Generate the existing root layout under `infra/` |
| Existing single-layer Bicep or Terraform project | Preserve it as a layer and add `foundry` at `infra/foundry` |
| Existing custom/fileless provider | Preserve it as a layer and add `foundry` |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileless is what's breaking cspell-lint on this PR. It isn't in .vscode/cspell.global.yaml, there's no override covering docs/specs/agent-init-infra-layers/, and the word doesn't appear anywhere else in the repo.

The repo pattern for one-off terms is a file-scoped override in .vscode/cspell.misc.yaml:

    - filename: ./docs/specs/agent-init-infra-layers/**
      words:
          - fileless

Alternatively reword the row, if there's already an established term for a provider that ships no local templates.

path: infra/foundry
provider: microsoft.foundry
dependsOn:
- infra

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does foundry depend on infra? Per the section right below, the Foundry layer owns an isolated resource group and publishes its own env var instead of reading the app's. If it consumes nothing from infra, this edge only serializes two layers that could otherwise provision in parallel.

The dependsOn description in schemas/alpha/azure.yaml.json scopes it to hook-mediated edges the static analyzer can't infer from .bicepparam / .parameters.json. It also doesn't help teardown ordering, since cli/azd/cmd/down.go reverses the declared layer array rather than walking the graph.

Worth either dropping it from the example or naming the hidden edge, because this snippet is what people will copy.

```

The existing `infra/main.bicep` remains unchanged. Foundry files are generated
under `infra/foundry/`.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivating scenario is "add a Foundry agent to an existing web application or API," but the design stops at file layout and resource-group ownership. It never says how the existing infra layer or the app's services get the Foundry outputs (project endpoint, connection info) once Foundry sits in its own layer with its own resource group.

That's the part the user actually cares about after eject. If the wiring is expected to be manual, it belongs in Out of scope. If azd is supposed to plumb it, it changes both the safety rules and the dependsOn direction above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants