fix(ai-projects): route resolveVars through foundry.ExpandEnv

cli/azd/extensions/azure.ai.agents/internal/cmd/init_env.go

@@ -23,14 +23,10 @@ import (
 // environment value because the runtime expander supplies the fallback.
 var azureYamlEnvRefPattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)(:-[^}]*)?\}`)
 
-// Escape handling must match the expander that owns each field.
-// foundry.ExpandEnv treats an odd leading '$' as an escape, while the project
-// synthesizers' resolveVars helper expands every ${VAR} match regardless of
-// a preceding '$'.
-const (
-	honorAzureYamlEnvironmentEscaping  = true
-	ignoreAzureYamlEnvironmentEscaping = false
-)
+// Escape handling must match the expander that owns each field. Every Foundry
+// field, including the project network values, expands through
+// foundry.ExpandEnv, which treats an odd leading '$' as an escape.
+const honorAzureYamlEnvironmentEscaping = true
 
 // These types mirror only the fields each Foundry provider expands from the
 // azd environment. The owning provider types are unexported or live in sibling
@@ -406,7 +402,7 @@ func collectAzureYamlServiceEnvironmentReferences(
 				collectAzureYamlEnvironmentReferences(
 					config.Network.AgentSubnet.VNet,
 					false,
-					ignoreAzureYamlEnvironmentEscaping,
+					honorAzureYamlEnvironmentEscaping,
 					references,
 					indexByName,
 				)
@@ -415,7 +411,7 @@ func collectAzureYamlServiceEnvironmentReferences(
 				collectAzureYamlEnvironmentReferences(
 					config.Network.PESubnet.VNet,
 					false,
-					ignoreAzureYamlEnvironmentEscaping,
+					honorAzureYamlEnvironmentEscaping,
 					references,
 					indexByName,
 				)
@@ -424,7 +420,7 @@ func collectAzureYamlServiceEnvironmentReferences(
 				collectAzureYamlEnvironmentReferences(
 					config.Network.DNS.Subscription,
 					false,
-					ignoreAzureYamlEnvironmentEscaping,
+					honorAzureYamlEnvironmentEscaping,
 					references,
 					indexByName,
 				)

cli/azd/extensions/azure.ai.agents/internal/cmd/init_env_test.go

@@ -135,7 +135,7 @@ services:
 			},
 		},
 		{
-			name: "project scans only expanded network fields",
+			name: "project scans only expanded network fields and honors escaping",
 			content: `name: sample
 services:
   project:
@@ -151,12 +151,14 @@ services:
       agentSubnet:
         vnet: $${AGENT_VNET_ID}
       peSubnet:
-        vnet: $${PE_VNET_ID}
+        vnet: ${PE_VNET_ID}
       dns:
         subscription: ${DNS_SUBSCRIPTION_ID}
 `,
+			// The network fields expand through foundry.ExpandEnv, so an escaped
+			// $${VAR} stays literal and needs no environment value; only the
+			// unescaped references are required.
 			want: []azureYamlEnvironmentReference{
-				{Name: "AGENT_VNET_ID"},
 				{Name: "PE_VNET_ID"},
 				{Name: "DNS_SUBSCRIPTION_ID"},
 			},

cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go

@@ -808,8 +808,10 @@ var guidPattern = regexp.MustCompile(
 // rgNamePattern matches a valid Azure resource group name.
 var rgNamePattern = regexp.MustCompile(`^[-\w._()]{1,90}$`)
 
-// varRefPattern matches a ${VAR} reference.
-var varRefPattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)\}`)
+// varRefPattern matches a ${VAR} reference, optionally carrying a
+// ${VAR:-default} fallback. Group 2 is non-empty when a default is present,
+// which means the reference resolves even with no environment value.
+var varRefPattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)(:-[^}]*)?\}`)
 
 // synthesizeNetwork validates the network: block and returns the bicep
 // parameter set plus the telemetry mode. When net is nil the returned
@@ -1010,29 +1012,49 @@ func sameVNet(a, b string) bool {
 	return strings.EqualFold(a, b)
 }
 
-// containsVarRef reports whether s still contains a ${VAR} reference.
+// containsVarRef reports whether s still contains a ${VAR} reference, including
+// the ${VAR:-default} form.
 func containsVarRef(s string) bool {
 	return varRefPattern.MatchString(s)
 }
 
 // resolveVars expands ${VAR} references in s using env first, then the
 // process environment. An unresolved reference is an error naming the
 // variable.
+//
+// Expansion routes through foundry.ExpandEnv, the shared expander every other
+// Foundry field uses, so ${VAR:-default} and the $${VAR} escape behave here
+// exactly as they do elsewhere.
+//
+// ExpandEnv resolves through a mapping callback that only receives the variable
+// name, so the set of names that must resolve is collected up front: a name is
+// required only when it appears at least once without a :- default. Names inside
+// a Foundry ${{...}} span never reach the callback, because ExpandEnv masks
+// those spans, so they cannot trip this check.
 func resolveVars(s string, env map[string]string) (string, error) {
+	required := map[string]struct{}{}
+	for _, match := range varRefPattern.FindAllStringSubmatch(s, -1) {
+		if match[2] == "" {
+			required[match[1]] = struct{}{}
+		}
+	}
+
 	var unresolved string
-	out := varRefPattern.ReplaceAllStringFunc(s, func(match string) string {
-		name := varRefPattern.FindStringSubmatch(match)[1]
+	out, err := foundry.ExpandEnv(s, func(name string) string {
 		if v, ok := env[name]; ok {
 			return v
 		}
 		if v, ok := os.LookupEnv(name); ok {
 			return v
 		}
-		if unresolved == "" {
+		if _, ok := required[name]; ok && unresolved == "" {
 			unresolved = name
 		}
-		return match
+		return ""
 	})
+	if err != nil {
+		return "", err
+	}
 	if unresolved != "" {
 		return "", fmt.Errorf("unresolved environment variable ${%s}", unresolved)
 	}

cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer.go

@@ -808,8 +808,10 @@ var guidPattern = regexp.MustCompile(
 // rgNamePattern matches a valid Azure resource group name.
 var rgNamePattern = regexp.MustCompile(`^[-\w._()]{1,90}$`)
 
-// varRefPattern matches a ${VAR} reference.
-var varRefPattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)\}`)
+// varRefPattern matches a ${VAR} reference, optionally carrying a
+// ${VAR:-default} fallback. Group 2 is non-empty when a default is present,
+// which means the reference resolves even with no environment value.
+var varRefPattern = regexp.MustCompile(`\$\{([A-Za-z_][A-Za-z0-9_]*)(:-[^}]*)?\}`)
 
 // synthesizeNetwork validates the network: block and returns the bicep
 // parameter set plus the telemetry mode. When net is nil the returned
@@ -1010,29 +1012,49 @@ func sameVNet(a, b string) bool {
 	return strings.EqualFold(a, b)
 }
 
-// containsVarRef reports whether s still contains a ${VAR} reference.
+// containsVarRef reports whether s still contains a ${VAR} reference, including
+// the ${VAR:-default} form.
 func containsVarRef(s string) bool {
 	return varRefPattern.MatchString(s)
 }
 
 // resolveVars expands ${VAR} references in s using env first, then the
 // process environment. An unresolved reference is an error naming the
 // variable.
+//
+// Expansion routes through foundry.ExpandEnv, the shared expander every other
+// Foundry field uses, so ${VAR:-default} and the $${VAR} escape behave here
+// exactly as they do elsewhere.
+//
+// ExpandEnv resolves through a mapping callback that only receives the variable
+// name, so the set of names that must resolve is collected up front: a name is
+// required only when it appears at least once without a :- default. Names inside
+// a Foundry ${{...}} span never reach the callback, because ExpandEnv masks
+// those spans, so they cannot trip this check.
 func resolveVars(s string, env map[string]string) (string, error) {
+	required := map[string]struct{}{}
+	for _, match := range varRefPattern.FindAllStringSubmatch(s, -1) {
+		if match[2] == "" {
+			required[match[1]] = struct{}{}
+		}
+	}
+
 	var unresolved string
-	out := varRefPattern.ReplaceAllStringFunc(s, func(match string) string {
-		name := varRefPattern.FindStringSubmatch(match)[1]
+	out, err := foundry.ExpandEnv(s, func(name string) string {
 		if v, ok := env[name]; ok {
 			return v
 		}
 		if v, ok := os.LookupEnv(name); ok {
 			return v
 		}
-		if unresolved == "" {
+		if _, ok := required[name]; ok && unresolved == "" {
 			unresolved = name
 		}
-		return match
+		return ""
 	})
+	if err != nil {
+		return "", err
+	}
 	if unresolved != "" {
 		return "", fmt.Errorf("unresolved environment variable ${%s}", unresolved)
 	}

cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer_test.go

@@ -1544,3 +1544,141 @@ services:
 		})
 	}
 }
+
+// TestResolveVars_MatchesFoundryExpandEnv locks in that the three project
+// network fields resolved through resolveVars use the same expander semantics
+// as every other Foundry field: ${VAR:-default} falls back, $${VAR} stays
+// literal, and a reference with neither a value nor a default is still a
+// load-bearing error naming the variable.
+func TestResolveVars_MatchesFoundryExpandEnv(t *testing.T) {
+	env := map[string]string{
+		"SET_VAR":   "set-value",
+		"EMPTY_VAR": "",
+	}
+
+	tests := []struct {
+		name    string
+		in      string
+		want    string
+		wantErr string
+	}{
+		{name: "plain reference", in: "${SET_VAR}", want: "set-value"},
+		{name: "default is unused when set", in: "${SET_VAR:-fallback}", want: "set-value"},
+		{name: "default fills in when unset", in: "${MISSING_VAR_XYZ:-fallback}", want: "fallback"},
+		{name: "empty default is allowed", in: "${MISSING_VAR_XYZ:-}", want: ""},
+		{name: "empty env value takes the default", in: "${EMPTY_VAR:-fallback}", want: "fallback"},
+		{name: "escaped reference stays literal", in: "$${MISSING_VAR_XYZ}", want: "${MISSING_VAR_XYZ}"},
+		{name: "no references", in: "/subscriptions/abc", want: "/subscriptions/abc"},
+		{
+			name: "default inside a resource id",
+			in:   "${MISSING_VAR_XYZ:-/subscriptions/s/resourceGroups/rg}",
+			want: "/subscriptions/s/resourceGroups/rg",
+		},
+		{
+			name:    "unresolved reference errors",
+			in:      "${MISSING_VAR_XYZ}",
+			wantErr: "unresolved environment variable ${MISSING_VAR_XYZ}",
+		},
+		{
+			name:    "first unresolved reference is named",
+			in:      "${MISSING_A_XYZ}/${MISSING_B_XYZ}",
+			wantErr: "unresolved environment variable ${MISSING_A_XYZ}",
+		},
+		{
+			name:    "a default elsewhere does not excuse a bare reference",
+			in:      "${MISSING_VAR_XYZ:-ok}/${MISSING_VAR_XYZ}",
+			wantErr: "unresolved environment variable ${MISSING_VAR_XYZ}",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := resolveVars(tt.in, env)
+			if tt.wantErr != "" {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tt.wantErr)
+				return
+			}
+			require.NoError(t, err)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// TestContainsVarRef_RecognizesDefaults guards the eject path: a reference with
+// a default must be recognized as still-unresolved so the value is kept
+// verbatim and the ARM-shape checks are deferred to provision time, instead of
+// being rejected as a malformed resource id.
+func TestContainsVarRef_RecognizesDefaults(t *testing.T) {
+	tests := []struct {
+		in   string
+		want bool
+	}{
+		{in: "${VNET}", want: true},
+		{in: "${VNET:-/subscriptions/s}", want: true},
+		{in: "/subscriptions/s/resourceGroups/rg", want: false},
+		{in: "", want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.in, func(t *testing.T) {
+			assert.Equal(t, tt.want, containsVarRef(tt.in))
+		})
+	}
+}
+
+// TestSynthesize_NetworkVarRefDefaults covers the reported bug end to end:
+// ${VAR:-default} on the three network fields previously fell through
+// resolveVars unchanged and was then rejected by the ARM id / subscription
+// shape checks, blaming the resource id instead of the unsupported syntax.
+func TestSynthesize_NetworkVarRefDefaults(t *testing.T) {
+	const (
+		fallbackVNet = "/subscriptions/00000000-0000-0000-0000-000000000000" +
+			"/resourceGroups/rg/providers/Microsoft.Network/virtualNetworks/default"
+		fallbackSub = "11111111-1111-1111-1111-111111111111"
+	)
+
+	yaml := `
+services:
+  my-project:
+    host: azure.ai.project
+    network:
+      peSubnet: {vnet: "${MISSING_VNET_XYZ:-` + fallbackVNet + `}", name: pe-subnet}
+      dns:
+        subscription: "${MISSING_SUB_XYZ:-` + fallbackSub + `}"
+`
+	res, err := Synthesize(Input{
+		RawAzureYAML:  []byte(yaml),
+		ServiceName:   "my-project",
+		AcceptedHosts: []string{"azure.ai.project"},
+	})
+	require.NoError(t, err)
+	require.NotNil(t, res)
+	assert.Equal(t, fallbackVNet, res.Parameters["vnetId"])
+	assert.Equal(t, fallbackSub, res.Parameters["dnsZonesSubscription"])
+}
+
+// TestSynthesize_NetworkPreserveVarRefsWithDefault is the eject-path half of the
+// same bug: a defaulted reference must survive verbatim rather than being
+// rejected as a malformed VNet id.
+func TestSynthesize_NetworkPreserveVarRefsWithDefault(t *testing.T) {
+	const ref = "${AZURE_VNET_ID:-/subscriptions/s/resourceGroups/rg" +
+		"/providers/Microsoft.Network/virtualNetworks/default}"
+
+	yaml := `
+services:
+  my-project:
+    host: azure.ai.project
+    network:
+      peSubnet: {vnet: "` + ref + `", name: pe-subnet}
+`
+	res, err := Synthesize(Input{
+		RawAzureYAML:    []byte(yaml),
+		ServiceName:     "my-project",
+		AcceptedHosts:   []string{"azure.ai.project"},
+		PreserveVarRefs: true,
+	})
+	require.NoError(t, err, "a defaulted ${VAR} must not fail on the eject path")
+	require.NotNil(t, res)
+	assert.Equal(t, ref, res.Parameters["vnetId"])
+}