fix: align agent workflows with unified azure.yaml#9081
Conversation
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
There was a problem hiding this comment.
Pull request overview
Aligns agent workflows and related Foundry resource extensions with unified azure.yaml, including $ref resolution and legacy fallbacks.
Changes:
- Resolves unified and referenced agent, connection, toolbox, and deployment configurations.
- Updates local run, deployment, guidance, and doctor behavior.
- Adds regression tests and changelog entries.
Reviewed changes
Copilot reviewed 44 out of 44 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
azure.ai.toolboxes/internal/cmd/service_target.go |
Resolves toolbox $ref files. |
azure.ai.toolboxes/internal/cmd/service_target_test.go |
Tests referenced toolbox config. |
azure.ai.toolboxes/CHANGELOG.md |
Documents toolbox fix. |
azure.ai.connections/internal/cmd/service_target.go |
Resolves connection $ref files. |
azure.ai.connections/internal/cmd/service_target_test.go |
Tests referenced connection config. |
azure.ai.connections/CHANGELOG.md |
Documents connection fix. |
azure.ai.agents/internal/synthesis/synthesizer.go |
Resolves referenced provisioning inputs. |
azure.ai.agents/internal/synthesis/synthesizer_test.go |
Tests referenced deployments and agents. |
azure.ai.agents/internal/project/service_update.go |
Serializes project service writes. |
azure.ai.agents/internal/project/service_target_agent.go |
Resolves and deploys unified agent config. |
azure.ai.agents/internal/project/service_target_agent_test.go |
Tests references and environment merging. |
azure.ai.agents/internal/project/foundry_provisioning_provider.go |
Resolves referenced brownfield config. |
azure.ai.agents/internal/project/foundry_provisioning_provider_test.go |
Tests referenced project endpoints. |
azure.ai.agents/internal/project/config.go |
Accepts split toolbox names. |
azure.ai.agents/internal/project/agent_definition.go |
Adds unified $ref resolution helpers. |
azure.ai.agents/internal/project/agent_definition_test.go |
Tests unified and legacy definitions. |
azure.ai.agents/internal/cmd/run.go |
Merges service environment values locally. |
azure.ai.agents/internal/cmd/run_test.go |
Tests local environment resolution. |
azure.ai.agents/internal/cmd/resource_services.go |
Resolves split Foundry resources. |
azure.ai.agents/internal/cmd/resource_services_test.go |
Tests referenced resource collection. |
azure.ai.agents/internal/cmd/nextstep/types.go |
Extends guidance state metadata. |
azure.ai.agents/internal/cmd/nextstep/state.go |
Reads effective unified agent config. |
azure.ai.agents/internal/cmd/nextstep/state_test.go |
Tests unified guidance state. |
azure.ai.agents/internal/cmd/nextstep/resolver.go |
Selects deploy or provision guidance. |
azure.ai.agents/internal/cmd/nextstep/resolver_test.go |
Tests split-resource guidance. |
azure.ai.agents/internal/cmd/nextstep/manifest.go |
Collects unified and legacy resources. |
azure.ai.agents/internal/cmd/nextstep/manifest_test.go |
Tests resource collection and errors. |
azure.ai.agents/internal/cmd/nextstep/format.go |
Updates guidance documentation. |
azure.ai.agents/internal/cmd/nextstep/config.go |
Loads effective guidance configuration. |
azure.ai.agents/internal/cmd/listen.go |
Resolves config during lifecycle events. |
azure.ai.agents/internal/cmd/listen_test.go |
Tests referenced container settings. |
azure.ai.agents/internal/cmd/helpers.go |
Adds environment to local run context. |
azure.ai.agents/internal/cmd/doctor/checks_toolboxes.go |
Updates toolbox diagnostics. |
azure.ai.agents/internal/cmd/doctor/checks_toolboxes_test.go |
Tests toolbox ownership guidance. |
azure.ai.agents/internal/cmd/doctor/checks_remote_test.go |
Updates remote check expectations. |
azure.ai.agents/internal/cmd/doctor/checks_project.go |
Validates unified agent definitions. |
azure.ai.agents/internal/cmd/doctor/checks_project_test.go |
Tests unified definition diagnostics. |
azure.ai.agents/internal/cmd/doctor/checks_manual_env.go |
Updates environment diagnostics. |
azure.ai.agents/internal/cmd/doctor/checks_manual_env_test.go |
Updates diagnostic assertions. |
azure.ai.agents/internal/cmd/doctor/checks_local.go |
Updates local-check documentation. |
azure.ai.agents/internal/cmd/doctor/checks_local_test.go |
Updates local-check expectations. |
azure.ai.agents/internal/cmd/doctor/checks_connections.go |
Updates connection diagnostics. |
azure.ai.agents/internal/cmd/doctor/checks_connections_test.go |
Tests connection ownership guidance. |
azure.ai.agents/CHANGELOG.md |
Documents unified configuration fixes. |
jongio
left a comment
There was a problem hiding this comment.
I looked into the two $ref overlay comments already on this PR and the mechanism holds up. foundry.ResolveFileRefs overlays a directive's sibling keys on top of the referenced file wholesale (the sibling loop in resolveRef), so any field persisted next to $ref wins over that same field in the referenced file on every later run.
One thing worth adding beyond what's already noted: in prepareContainerSettings, SetAgentContainerSettings writes container onto the original $ref-bearing properties and fills in DefaultMemory/DefaultCpu whenever they're unset. populateContainerSettings runs for every agent service, not just ones that actually set resources. So after the first preprovision run a $ref-backed service ends up with a container block (often just the injected defaults) pinned beside $ref, and later edits to container in the referenced agent file stop taking effect. The resolved project/RelativePath write in persistBuildService has the same shape.
For $ref-backed services, persisting only the fields the user authored inline, or writing derived defaults into the referenced file through the ref-aware editor, would avoid pinning values that silently override the source file.
| @description('Foundry project connections to create (host: azure.ai.connection services).') | ||
| param connections connectionsType = [] |
| @description('Foundry project connections to create on the existing project (host: azure.ai.connection services).') | ||
| param connections connectionsType = [] |
| Category string `yaml:"category,omitempty"` | ||
| Target string `yaml:"target,omitempty"` | ||
| AuthType string `yaml:"authType,omitempty"` | ||
| Credentials map[string]any `yaml:"credentials,omitempty"` | ||
| Metadata map[string]string `yaml:"metadata,omitempty"` |
jongio
left a comment
There was a problem hiding this comment.
I re-checked the new commit against the $ref pinning I raised earlier. The persist-then-restore path plus not persisting resolved container settings for $ref-backed services closes that gap, and the added tests cover both the restore and the package-level revert.
I'm not approving yet because of a separate, unresolved issue in the connection templates. In main.bicep (and brownfield.bicep), connections is an ordinary array parameter:
param connections connectionsType = []
and connectionType carries credentials: object?. Non-secure deployment parameters are stored in the ARM deployment history in plaintext, so anyone with reader access on the deployment can recover those credentials. Moving the secret values into a @secure() object parameter, keeping only non-secret metadata in the array, and regenerating main.arm.json / brownfield.arm.json would close that off.
Worth also confirming the two related points already raised on this PR: the connection decoder in synthesizer.go dropping advanced auth properties (OAuth URLs/scopes, sharing, managed-identity), and the doctor check in checks_connections.go still pointing at azd deploy for split connections that are now provision-managed.
Once the credential handling is sorted this is good to approve.
trangevi
left a comment
There was a problem hiding this comment.
I think there was maybe a bad merge here, I'm seeing a number of changes that I believe I've reviewed elsewhere and should already be in main.
jongio
left a comment
There was a problem hiding this comment.
Following up on @trangevi's bad-merge note, since it checks out and I can pin down what happened.
The merge-base between this branch and main is 105d7e9f, and the branch is 2 commits behind main (missing #9007 and #9103, including the pin to the released azd module). The resolve unified workflow conflicts merge (18dd0d2e) pulled in changes from other in-flight PRs that have nothing to do with aligning agent workflows to unified azure.yaml.
The clearest example is an entire PR-registry-check tooling subsystem from #9042 that isn't in main and isn't part of this change:
.github/scripts/src/ext-registry-check.js(+807),.github/scripts/test/ext-registry-check.test.js(+1089).github/scripts/package.json,package-lock.json,jsconfig.json.github/workflows/ext-registry-check.yml,.github/workflows/scripts-ci.yml
None of those exist in main, so they're riding in from #9042 rather than belonging here. They account for a large chunk of the +8920/72-file size and make the actual unified-azure.yaml changes hard to isolate.
Rebasing onto current main (or dropping the merge and re-merging cleanly) so the diff only contains the agent-workflow work would resolve this and give the substantive changes a clean review. Worth a second pass over the merged-in toolbox commits (#9046, the toolbox-branch fix) to confirm which of those are genuinely part of this PR versus riding in from elsewhere.
This is separate from the credential-handling issue I raised earlier, which is still open.
4b33245 to
5689831
Compare
jongio
left a comment
There was a problem hiding this comment.
The rebase cleaned this up, and my earlier blocking concerns are resolved:
- The branch no longer carries the #9042 registry-check tooling or the other in-flight commits it had picked up. It is down to 2 commits / 46 files, all under
cli/azd/extensions/. - The connection bicep templates that stored resolved credentials in a plain (non-
@secure()) ARM array param are no longer in this diff, so the deployment-history exposure I raised earlier is out of scope here. - The
$refpinning is handled now:persistBuildServicewrites the resolved path for the build, then the deferredcombineBuildServiceRestorewrites the original$refconfig back afterPackage, and file-ref services no longer persist resolved container settings inprepareContainerSettings.
The three open items from the latest automated pass still hold. I checked them against the current HEAD: the ${{...}} env values getting pre-expanded by core in the helpers/deploy paths, ProjectRoot not reaching collectConnections so split-connection $ref files stay unresolved, and the env decode rejecting non-string YAML scalars in config.go. Those are already captured, so I am not repeating them inline.
One net-new item below.
| if err := AddServiceSerialized( | ||
| ctx, | ||
| p.azdClient, | ||
| persisted, | ||
| ); err != nil { |
| resolved, err := foundry.ResolveFileRefs( | ||
| props.AsMap(), | ||
| projectRoot, | ||
| ) |
| func agentEnvironmentValues( | ||
| agentDef guidanceAgentDefinition, | ||
| cfg *guidanceServiceConfig, | ||
| ) []string { | ||
| var values []string | ||
| if agentDef.EnvironmentVariables != nil { | ||
| for _, envVar := range *agentDef.EnvironmentVariables { | ||
| values = append(values, envVar.Value) | ||
| } | ||
| } | ||
| if cfg != nil { | ||
| for _, key := range slices.Sorted(maps.Keys(cfg.Environment)) { | ||
| values = append(values, cfg.Environment[key]) | ||
| } | ||
| } | ||
| return values | ||
| } |
| ca, isHosted, err := agentDefinitionFromStruct( | ||
| resolved, | ||
| image, | ||
| ) |
| if err := unmarshalServiceProps( | ||
| props, | ||
| &candidateCfg, | ||
| ); err != nil { |
| definitions, err := agent_yaml.ExtractResourceDefinitions(data) | ||
| if err != nil { | ||
| appendResourceLoadError( | ||
| errs, | ||
| resources, | ||
| svc.Name, | ||
| err, |
| // Placeholder fix-ups always come first when present: they are broken | ||
| // state in azure.yaml itself and block both `run` and `deploy`. The | ||
| // user has to edit azure.yaml (or define a matching parameter in | ||
| // agent.manifest.yaml) — `azd env set` cannot reach them. | ||
| // user has to edit azure.yaml; `azd env set` cannot reach them. |
| resolved, err := foundry.ResolveFileRefs( | ||
| props.AsMap(), | ||
| projectRoot, | ||
| ) |
| if err := unmarshalServiceProps( | ||
| props, | ||
| &candidateCfg, | ||
| ); err != nil { | ||
| return agentDef, cfg, props, err |
| if !isHosted { | ||
| failures = append( | ||
| failures, | ||
| fmt.Sprintf( | ||
| "%s: unsupported agent kind", | ||
| svc.GetName(), | ||
| ), | ||
| ) |
| if err := AddServiceSerialized( | ||
| ctx, | ||
| p.azdClient, | ||
| persisted, | ||
| ); err != nil { |
| // Placeholder fix-ups always come first when present: they are broken | ||
| // state in azure.yaml itself and block both `run` and `deploy`. The | ||
| // user has to edit azure.yaml (or define a matching parameter in | ||
| // agent.manifest.yaml) — `azd env set` cannot reach them. | ||
| // user has to edit azure.yaml; `azd env set` cannot reach them. |
Summary
envvalues, split Foundry resources, and local$reffiles from unifiedazure.yamlwhile retaining legacy config and sidecar fallbacks.Testing
go build ./...andgo test ./... -count=1inazure.ai.agents,azure.ai.connections, andazure.ai.toolboxesgolangci-lint run ./...in all three extensionscspellchecks for Go and Markdown files in all three extensionsCloses #8710