Skip to content

Commit da75a17

Browse files
Antriksh JainCopilot
andcommitted
feat(nextstep): surface {{NAME}} placeholders left over in agent.yaml
Phase 2 commit 4.6 (MVP follow-up to 4.5). The toolbox sample exposes a second MVP-level next-step bug: after `azd ai agent init` selects "Use existing model deployment(s) from a Foundry project", the processed `agent.yaml` retains a literal Mustache-style placeholder `'{{TOOLBOX_ENDPOINT}}'` inside `environment_variables`. The current Next: block flags only the sibling `${TOOLBOX_WEB_SEARCH_TOOLS_MCP_ENDPOINT}` ref, leaving the user with the impression that one `azd env set` line is the entire fix-up — when in reality the literal `{{TOOLBOX_ENDPOINT}}` would land in the container as-is and break the agent on deploy. Root cause: agent.manifest.yaml's parameter-substitution step (`pkg/agents/agent_yaml/parameters.go:injectParameterValues`) is supposed to replace every `{{NAME}}` and `{{ NAME }}` token with its parameter value during init. When the manifest declares the placeholder but no matching `parameters:` entry exists (or the user opts out at the prompt), init prints a weak `Warning: Template contains unresolved placeholders.` line at parameters.go:248 and moves on — the literal `{{NAME}}` carries forward into the final agent.yaml. The nextstep resolver had no concept of these placeholders: `extractAgentYamlEnvRefs` matched only `${VAR}` refs, so the `{{NAME}}` was invisible. Fix (resolver-side surfacing, as confirmed with user): 1. `types.go` — new `State.UnresolvedPlaceholders []string` field with a doc-comment that explicitly contrasts placeholders against Missing*Vars (cannot be supplied via `azd env set`; the literal lives in agent.yaml itself). 2. `state.go` — new `placeholderPattern` regex `\{\{\s*([A-Za-z_][A-Za-z0-9_]*)\s*\}\}`. Allows optional internal whitespace because `injectParameterValues` substitutes both `{{NAME}}` and `{{ NAME }}` forms (parameters.go:238,242). `extractAgentYamlEnvRefs` signature changes from `[]string` to `(refs, placeholders []string)`. `detectMissingVars` signature extends from two return values to three (infra, manual, placeholders). `assembleState` callsite captures the third slice into `state.UnresolvedPlaceholders`. Read errors stay silent (same best-effort contract as before). 3. `resolver.go` — `ResolveAfterInit` decision tree extended: * Placeholder fix-ups ALWAYS come first when present, regardless of other branches. One `edit agent.yaml: replace {{NAME}} with the actual value` line per placeholder, capped at maxFixupLines (3), sorted ascending. Rationale: deploy-time landmines that block both `run` (literal `{{NAME}}` in container env) and `deploy`. They never reach `azd env set`. * Existing provision branch unchanged (still wins for `!HasProjectEndpoint || MissingInfraVars`). * Existing manual-vars branch unchanged. * New `case hasPlaceholders` (sentinel) deliberately suppresses `azd ai agent run` when only placeholders remain — running locally with literal `{{NAME}}` values produces a broken agent, so we don't suggest a path we know will fail. The constant `maxManualVarLines` renames to `maxFixupLines` (now shared between manual-vars and placeholder caps; same value = 3). Decision scenarios validated (each covered by a new test case): * Fresh init + Deploy new models → `azd provision` + deploy. * Fresh init + Use existing + bare ${VAR} → `azd env set VAR` + deploy. * Toolbox bug → `edit agent.yaml: replace {{X}}` + `azd env set Y` + deploy. * Placeholders only → `edit agent.yaml: replace {{X}}` + deploy (NO run). * Placeholders + no project endpoint → `edit agent.yaml: replace {{X}}` + `azd provision` + deploy. * Happy post-provision (no missing) → `azd ai agent run` + deploy. Tests: * `state_test.go` — extended `TestExtractAgentYamlEnvRefs` table to a two-output schema (`wantRefs`, `wantPlaceholders`); 4 new table cases cover placeholders alone, internal-whitespace form, dedup, and ref+placeholder coexistence. New `TestAssembleState_PopulatesUnresolvedPlaceholders` reproduces the toolbox sample directly. New `TestAssembleState_PlaceholdersDeduped AcrossServices` locks cross-service dedup. * `resolver_test.go` — new `TestResolveAfterInit_UnresolvedPlaceholders` table with 5 cases covering placeholders-alone, placeholders+manual, placeholders+missing-endpoint, sort-ascending, and >3 cap. Preflight clean (gofmt, vet, build, full extension test suite green, golangci-lint 0 issues, cspell 0 issues on the 3 production files). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d1d213c commit da75a17

5 files changed

Lines changed: 387 additions & 69 deletions

File tree

cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver.go

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,23 @@ const (
2323
invokeInvocationsPayload = `'{"message": "Hello!"}'`
2424
invokeResponsesPayload = `"Hello!"`
2525

26-
// maxManualVarLines caps the number of `azd env set` hints emitted by
27-
// ResolveAfterInit so the block stays scannable even when an agent
28-
// declares many manual variables.
29-
maxManualVarLines = 3
26+
// maxFixupLines caps the number of `azd env set` / `edit agent.yaml`
27+
// hints emitted by ResolveAfterInit per missing-input category so the
28+
// block stays scannable even when an agent declares many manual
29+
// variables or unresolved placeholders.
30+
maxFixupLines = 3
3031
)
3132

3233
// ResolveAfterInit produces the Next: block printed at the end of a
3334
// successful `azd ai agent init`. Pure function over *State.
3435
//
3536
// Decision tree:
37+
// - UnresolvedPlaceholders (always shown first when present, regardless
38+
// of other branches) → one "edit agent.yaml: replace {{NAME}}" line
39+
// per unresolved Mustache placeholder (up to maxFixupLines). These
40+
// are deploy-time landmines: the literal `{{NAME}}` would otherwise
41+
// land in the container. They never reach `azd env set` because the
42+
// value lives in agent.yaml itself, not the azd environment.
3643
// - !HasProjectEndpoint OR MissingInfraVars → `azd provision`
3744
// The project endpoint is the canonical "provision finished"
3845
// marker — it is set by `azd provision` as a Bicep output, or by
@@ -46,8 +53,10 @@ const (
4653
// (a new ${AZURE_*} reference was added to agent.yaml after the
4754
// last provision run).
4855
// - MissingManualVars → one `azd env set <KEY> <value>` per missing var
49-
// (up to maxManualVarLines)
56+
// (up to maxFixupLines)
5057
// - Otherwise → `azd ai agent run`
58+
// Skipped when only UnresolvedPlaceholders are present, because
59+
// running locally with literal `{{NAME}}` values is broken too.
5160
//
5261
// All paths append the static "When ready to deploy to Azure…" tail.
5362
func ResolveAfterInit(state *State) []Suggestion {
@@ -56,30 +65,56 @@ func ResolveAfterInit(state *State) []Suggestion {
5665
}
5766

5867
out := make([]Suggestion, 0, 4)
68+
priority := 5
69+
70+
// Placeholder fix-ups always come first when present: they are broken
71+
// state in agent.yaml itself and block both `run` and `deploy`. The
72+
// user has to edit agent.yaml (or define a matching parameter in
73+
// agent.manifest.yaml) — `azd env set` cannot reach them.
74+
hasPlaceholders := len(state.UnresolvedPlaceholders) > 0
75+
if hasPlaceholders {
76+
placeholders := slices.Clone(state.UnresolvedPlaceholders)
77+
slices.Sort(placeholders)
78+
limit := min(len(placeholders), maxFixupLines)
79+
for _, name := range placeholders[:limit] {
80+
out = append(out, Suggestion{
81+
Command: fmt.Sprintf("edit agent.yaml: replace {{%s}} with the actual value", name),
82+
Description: "agent.yaml has unresolved manifest placeholders",
83+
Priority: priority,
84+
})
85+
priority++
86+
}
87+
}
5988

6089
switch {
6190
case !state.HasProjectEndpoint || len(state.MissingInfraVars) > 0:
6291
out = append(out, Suggestion{
6392
Command: "azd provision",
6493
Description: "set up your Foundry project, models, and connections",
65-
Priority: 10,
94+
Priority: priority,
6695
})
6796
case len(state.MissingManualVars) > 0:
6897
manual := slices.Clone(state.MissingManualVars)
6998
slices.Sort(manual)
70-
limit := min(len(manual), maxManualVarLines)
71-
for i, key := range manual[:limit] {
99+
limit := min(len(manual), maxFixupLines)
100+
for _, key := range manual[:limit] {
72101
out = append(out, Suggestion{
73102
Command: fmt.Sprintf("azd env set %s <value>", key),
74103
Description: "supply the agent.yaml variable",
75-
Priority: 20 + i,
104+
Priority: priority,
76105
})
106+
priority++
77107
}
108+
case hasPlaceholders:
109+
// Only unresolved placeholders remain — do not emit
110+
// `azd ai agent run` because running locally with literal
111+
// `{{NAME}}` values produces a broken agent. The placeholder
112+
// fix-ups above already tell the user what to do.
78113
default:
79114
out = append(out, Suggestion{
80115
Command: "azd ai agent run",
81116
Description: "start the agent locally",
82-
Priority: 10,
117+
Priority: priority,
83118
})
84119
}
85120

cli/azd/extensions/azure.ai.agents/internal/cmd/nextstep/resolver_test.go

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,120 @@ func TestResolveAfterInit_NilState(t *testing.T) {
107107
assert.Nil(t, ResolveAfterInit(nil))
108108
}
109109

110+
func TestResolveAfterInit_UnresolvedPlaceholders(t *testing.T) {
111+
t.Parallel()
112+
113+
tests := []struct {
114+
name string
115+
state *State
116+
wantPlaceholders []string // expected `{{NAME}}` names in order
117+
wantMiddle string // expected non-trailing, non-placeholder primary (e.g., "azd provision", "azd env set FOO", or "" if none)
118+
wantHasRun bool // expect `azd ai agent run` to appear?
119+
wantHasDeploy bool // expect `azd deploy` trailing?
120+
}{
121+
{
122+
name: "placeholders alone → edit lines + deploy, no run",
123+
state: &State{
124+
HasProjectEndpoint: true,
125+
UnresolvedPlaceholders: []string{"TOOLBOX_ENDPOINT"},
126+
},
127+
wantPlaceholders: []string{"TOOLBOX_ENDPOINT"},
128+
wantHasRun: false,
129+
wantHasDeploy: true,
130+
},
131+
{
132+
name: "placeholders + missing manual vars → both surfaced, no run",
133+
state: &State{
134+
HasProjectEndpoint: true,
135+
UnresolvedPlaceholders: []string{"TOOLBOX_ENDPOINT"},
136+
MissingManualVars: []string{"TOOLBOX_MCP_ENDPOINT"},
137+
},
138+
wantPlaceholders: []string{"TOOLBOX_ENDPOINT"},
139+
wantMiddle: "azd env set TOOLBOX_MCP_ENDPOINT",
140+
wantHasRun: false,
141+
wantHasDeploy: true,
142+
},
143+
{
144+
name: "placeholders + project endpoint missing → placeholders + provision",
145+
state: &State{
146+
HasProjectEndpoint: false,
147+
UnresolvedPlaceholders: []string{"TOOLBOX_ENDPOINT"},
148+
},
149+
wantPlaceholders: []string{"TOOLBOX_ENDPOINT"},
150+
wantMiddle: "azd provision",
151+
wantHasRun: false,
152+
wantHasDeploy: true,
153+
},
154+
{
155+
name: "multiple placeholders sorted ascending",
156+
state: &State{
157+
HasProjectEndpoint: true,
158+
UnresolvedPlaceholders: []string{"CHARLIE", "ALPHA", "BRAVO"},
159+
},
160+
wantPlaceholders: []string{"ALPHA", "BRAVO", "CHARLIE"},
161+
wantHasRun: false,
162+
wantHasDeploy: true,
163+
},
164+
{
165+
name: "more than three placeholders capped at three",
166+
state: &State{
167+
HasProjectEndpoint: true,
168+
UnresolvedPlaceholders: []string{"P1", "P2", "P3", "P4", "P5"},
169+
},
170+
wantPlaceholders: []string{"P1", "P2", "P3"},
171+
wantHasRun: false,
172+
wantHasDeploy: true,
173+
},
174+
}
175+
176+
for _, tt := range tests {
177+
t.Run(tt.name, func(t *testing.T) {
178+
t.Parallel()
179+
out := ResolveAfterInit(tt.state)
180+
require.NotEmpty(t, out)
181+
182+
// Walk the output:
183+
// 1. leading run of placeholder fix-ups (one per wantPlaceholders[i])
184+
// 2. optional middle command (provision / env set)
185+
// 3. optional `azd ai agent run`
186+
// 4. trailing `azd deploy`
187+
for i, name := range tt.wantPlaceholders {
188+
require.Less(t, i, len(out))
189+
assert.Equal(t,
190+
"edit agent.yaml: replace {{"+name+"}} with the actual value",
191+
out[i].Command,
192+
)
193+
}
194+
195+
// The middle (if any) sits just past the placeholders.
196+
if tt.wantMiddle != "" {
197+
idx := len(tt.wantPlaceholders)
198+
require.Less(t, idx, len(out))
199+
assert.True(t,
200+
strings.HasPrefix(out[idx].Command, tt.wantMiddle),
201+
"middle suggestion %q does not have prefix %q",
202+
out[idx].Command, tt.wantMiddle,
203+
)
204+
}
205+
206+
hasRun := false
207+
hasDeploy := false
208+
for _, s := range out {
209+
switch {
210+
case s.Command == "azd ai agent run":
211+
hasRun = true
212+
case s.Command == "azd deploy" && s.Trailing:
213+
hasDeploy = true
214+
}
215+
}
216+
assert.Equal(t, tt.wantHasRun, hasRun,
217+
"presence of `azd ai agent run` mismatched")
218+
assert.Equal(t, tt.wantHasDeploy, hasDeploy,
219+
"presence of trailing `azd deploy` mismatched")
220+
})
221+
}
222+
}
223+
110224
func TestResolveAfterRun(t *testing.T) {
111225
t.Parallel()
112226

0 commit comments

Comments
 (0)