fix(agents): prompt for unified config env values#9212
Conversation
Prompt for unset bare ${VAR} references in adopted Foundry service configuration and persist the responses to the active azd environment. Preserve defaults, Foundry expressions, existing values, hook behavior, and --no-prompt execution.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: b4617dfd-adfb-4b30-9222-477a041f8af9
📋 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
Adds prompting and persistence for missing environment references in adopted Foundry configurations.
Changes:
- Detects bare
${VAR}references while excluding defaults, hooks, and escaped references. - Masks credential-like prompts and saves responses to the active environment.
- Integrates the flow into unified
azure.yamladoption with tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
init_env.go |
Implements reference discovery, prompting, and persistence. |
init_env_test.go |
Tests detection, masking, persistence, and no-prompt behavior. |
init_adopt.go |
Invokes environment configuration during adoption. |
Co-authored-by: glharper <64209257+glharper@users.noreply.github.com>
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Traced the `` resolution through foundry.ResolveFileRefs (sibling keys overlay the loaded object) and confirmed the `${{...}}` masking is multiline-aware before the env-ref regex runs. Escape handling, `:-` default skipping, hooks exclusion, and secret-upgrade dedup all line up with the tests. Nothing blocking from me.
Mask prompted values only when the reference appears under explicit credential or secret configuration. Avoid inferring sensitivity from arbitrary environment variable names while continuing to prompt for all unset Foundry references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b4617dfd-adfb-4b30-9222-477a041f8af9
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
init_env.go:78 - the missing-value check reads from GetValues, which returns only persisted dotenv entries. Foundry expansion resolves unset refs from the process environment as well (synthesis/synthesizer.go maybeExpand falls back to os.LookupEnv). A var that is exported in the shell but absent from the azd env is treated as missing here, so we prompt for it and then persist a value that shadows the exported one. Mirroring the same dotenv-first, process-env fallback lookup (while still counting an explicitly empty dotenv entry as missing) avoids the spurious prompt. A process-env-only regression test would lock it in.
Use persisted azd values first and fall back to the process environment before prompting for unified Foundry references. Keep explicitly empty persisted values missing so users can replace them interactively. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b4617dfd-adfb-4b30-9222-477a041f8af9
Persist process-only values referenced by unified Foundry services into the active azd environment. This keeps providers that resolve only persisted values consistent while preserving explicit empty azd values as prompt-required. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b4617dfd-adfb-4b30-9222-477a041f8af9
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
cli/azd/extensions/azure.ai.agents/internal/cmd/init_env.go:84
- [azd-code-reviewer] An explicitly set but empty process variable is treated as resolved and persisted, even though this function treats an empty azd value as missing and the Foundry expander resolves the bare reference to an empty string. This leaves provisioning with the same unusable value instead of prompting. Require the process value to be non-empty and add the matching regression case.
if value, ok := os.LookupEnv(reference.Name); ok {
| value := foundryTemplateSpanPattern.ReplaceAllStringFunc(node.Value, func(span string) string { | ||
| return strings.Repeat(" ", len(span)) | ||
| }) | ||
| for _, match := range azureYamlEnvRefPattern.FindAllStringSubmatchIndex(value, -1) { |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Non-blocking follow-up: reference discovery walks every scalar in a Foundry service, but deployments are copied verbatim by the synthesizer (deployments := svc.Deployments in synthesis/synthesizer.go, with no expandValue) and read raw by foundryDeployments in init_adopt.go. So a ${VAR} inside a deployment model or sku would get prompted and persisted here, then still reach verification and provisioning as a literal ${VAR}, so the persisted value is never used for that field. Connections, metadata, and network all go through the expander, so those references are handled correctly. Scoping discovery to the fields each provider actually expands would keep the prompt from implying a field is configured when it isn't.
Summary
${VAR}references in adopted Foundry service configuration duringazd ai agent init${VAR:-default}, Foundry${{...}}, hook variables, and--no-promptbehaviorTesting
go test ./... -count=1go build ./...golangci-lint run ./internal/cmd/...Fixes #9110