Skip to content

Commit 510764b

Browse files
JeffreyCACopilot
andcommitted
Address PR review: check process env for existing FOUNDRY_* vars and fix comment
- appendFoundryEnvVars now also checks the existing env slice (from os.Environ()) before appending FOUNDRY_* translations, preventing user shell-set values from being silently overridden. - Extract envSliceHasKey helper for the prefix check. - Add test covering the process env scenario. - Fix misleading test comment about AZURE_OPENAI_ENDPOINT auto-injection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bd7c8b5 commit 510764b

File tree

3 files changed

+58
-4
lines changed

3 files changed

+58
-4
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,8 @@ func TestWriteDefinitionToSrcDir(t *testing.T) {
441441
if !containsAll(contentStr, "name: test-agent", "kind: hosted", "responses", "AZURE_AI_MODEL_DEPLOYMENT_NAME") {
442442
t.Errorf("written content missing expected fields:\n%s", contentStr)
443443
}
444-
// AZURE_OPENAI_ENDPOINT and AZURE_AI_PROJECT_ENDPOINT should NOT be in agent.yaml
445-
// (they are auto-injected as FOUNDRY_PROJECT_ENDPOINT by the hosted agent platform)
444+
// AZURE_OPENAI_ENDPOINT and AZURE_AI_PROJECT_ENDPOINT should NOT be written to agent.yaml.
445+
// Hosted agents receive platform-provided FOUNDRY_* variables such as FOUNDRY_PROJECT_ENDPOINT instead.
446446
if strings.Contains(contentStr, "AZURE_OPENAI_ENDPOINT") || strings.Contains(contentStr, "AZURE_AI_PROJECT_ENDPOINT") {
447447
t.Errorf("agent.yaml should not contain AZURE_OPENAI_ENDPOINT or AZURE_AI_PROJECT_ENDPOINT:\n%s", contentStr)
448448
}

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

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"os/signal"
1212
"path/filepath"
1313
"runtime"
14+
"slices"
1415
"strings"
1516
"syscall"
1617

@@ -389,7 +390,7 @@ func appendFoundryEnvVars(env []string, azdEnv map[string]string, serviceName st
389390

390391
for _, m := range staticMappings {
391392
if v := azdEnv[m.azdKey]; v != "" {
392-
if _, exists := azdEnv[m.foundryKey]; !exists {
393+
if _, exists := azdEnv[m.foundryKey]; !exists && !envSliceHasKey(env, m.foundryKey) {
393394
env = append(env, fmt.Sprintf("%s=%s", m.foundryKey, v))
394395
}
395396
}
@@ -409,7 +410,7 @@ func appendFoundryEnvVars(env []string, azdEnv map[string]string, serviceName st
409410
for _, m := range agentMappings {
410411
azdKey := fmt.Sprintf(m.azdKeyFmt, serviceKey)
411412
if v := azdEnv[azdKey]; v != "" {
412-
if _, exists := azdEnv[m.foundryKey]; !exists {
413+
if _, exists := azdEnv[m.foundryKey]; !exists && !envSliceHasKey(env, m.foundryKey) {
413414
env = append(env, fmt.Sprintf("%s=%s", m.foundryKey, v))
414415
}
415416
}
@@ -419,6 +420,14 @@ func appendFoundryEnvVars(env []string, azdEnv map[string]string, serviceName st
419420
return env
420421
}
421422

423+
// envSliceHasKey reports whether the env slice already contains an entry for the given key.
424+
func envSliceHasKey(env []string, key string) bool {
425+
prefix := key + "="
426+
return slices.ContainsFunc(env, func(entry string) bool {
427+
return strings.HasPrefix(entry, prefix)
428+
})
429+
}
430+
422431
// loadAzdEnvironment reads all key-value pairs from the current azd environment.
423432
func loadAzdEnvironment(ctx context.Context, azdClient *azdext.AzdClient) (map[string]string, error) {
424433
envResponse, err := azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{})

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

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,4 +290,49 @@ func TestAppendFoundryEnvVars(t *testing.T) {
290290
t.Errorf("expected no translated env vars, got %v", env)
291291
}
292292
})
293+
294+
t.Run("skips foundry key when already set in process env slice", func(t *testing.T) {
295+
t.Parallel()
296+
// Simulate os.Environ() already containing FOUNDRY_* vars set by the user's shell
297+
existingEnv := []string{
298+
"HOME=/home/user",
299+
"FOUNDRY_PROJECT_ENDPOINT=https://user-shell.services.ai.azure.com",
300+
"FOUNDRY_AGENT_NAME=shell-agent",
301+
}
302+
azdEnv := map[string]string{
303+
"AZURE_AI_PROJECT_ENDPOINT": "https://from-azd.services.ai.azure.com",
304+
"AZURE_AI_PROJECT_ID": "/subscriptions/sub/rg/rg/acct/proj",
305+
"AGENT_MY_SVC_NAME": "my-agent",
306+
"AGENT_MY_SVC_VERSION": "v2",
307+
}
308+
env := appendFoundryEnvVars(existingEnv, azdEnv, "my-svc")
309+
310+
// FOUNDRY_PROJECT_ENDPOINT and FOUNDRY_AGENT_NAME should NOT be appended
311+
// because they already exist in the process env slice.
312+
foundryEndpointCount := 0
313+
foundryAgentNameCount := 0
314+
for _, entry := range env {
315+
if strings.HasPrefix(entry, "FOUNDRY_PROJECT_ENDPOINT=") {
316+
foundryEndpointCount++
317+
}
318+
if strings.HasPrefix(entry, "FOUNDRY_AGENT_NAME=") {
319+
foundryAgentNameCount++
320+
}
321+
}
322+
if foundryEndpointCount != 1 {
323+
t.Errorf("expected exactly 1 FOUNDRY_PROJECT_ENDPOINT entry (from shell), got %d in %v", foundryEndpointCount, env)
324+
}
325+
if foundryAgentNameCount != 1 {
326+
t.Errorf("expected exactly 1 FOUNDRY_AGENT_NAME entry (from shell), got %d in %v", foundryAgentNameCount, env)
327+
}
328+
329+
// FOUNDRY_PROJECT_ARM_ID and FOUNDRY_AGENT_VERSION should still be translated
330+
// since they are NOT already present in the env slice.
331+
if !slices.Contains(env, "FOUNDRY_PROJECT_ARM_ID=/subscriptions/sub/rg/rg/acct/proj") {
332+
t.Errorf("expected FOUNDRY_PROJECT_ARM_ID to be translated, got %v", env)
333+
}
334+
if !slices.Contains(env, "FOUNDRY_AGENT_VERSION=v2") {
335+
t.Errorf("expected FOUNDRY_AGENT_VERSION to be translated, got %v", env)
336+
}
337+
})
293338
}

0 commit comments

Comments
 (0)