diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go index 4dd0c4f0a7a..0242734ccee 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go @@ -441,7 +441,7 @@ func detectStartupCommand(projectDir string) string { } func fileExists(path string) bool { - _, err := os.Stat(path) + _, err := os.Stat(path) //nolint:gosec // path is derived from controlled inputs (agent ID + "src/" prefix) return err == nil } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go index 5cb06956db6..fd23fdd36e8 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -68,9 +68,10 @@ type InitAction struct { flags *initFlags models *modelSelector - deploymentDetails []project.Deployment - containerSettings *project.ContainerSettings - httpClient *http.Client + deploymentDetails []project.Deployment + containerSettings *project.ContainerSettings + httpClient *http.Client + serviceNameOverride string // when set, addToProject uses this instead of the manifest name } // modelSelector encapsulates the dependencies needed for model selection and @@ -581,6 +582,22 @@ func ensureProject(ctx context.Context, flags *initFlags, azdClient *azdext.AzdC } fmt.Println() + } else if projectResponse.Project != nil { + // An existing azd project was found — tell the user so the skipped template + // download isn't a mystery. Also warn if the project lacks an infra/ directory, + // since deployment may require infrastructure scaffolding. + fmt.Println(output.WithGrayFormat( + "Found existing azd project at %q. Adding agent to it.", projectResponse.Project.Path, + )) + + infraDir := filepath.Join(projectResponse.Project.Path, "infra") + if _, statErr := os.Stat(infraDir); os.IsNotExist(statErr) { + fmt.Printf("%s", output.WithWarningFormat( + "No infra/ directory found in the project. If you need Azure infrastructure "+ + "for deployment, run 'azd init -t Azure-Samples/azd-ai-starter-basic' in an empty "+ + "directory first, then re-run this command from there.\n", + )) + } } if projectResponse.Project == nil { @@ -637,8 +654,25 @@ func manifestHasModelResources(manifest *agent_yaml.AgentManifest) bool { func (a *InitAction) configureModelChoice( ctx context.Context, agentManifest *agent_yaml.AgentManifest, ) (*agent_yaml.AgentManifest, error) { - // If --project-id is provided, validate the ARM format and extract the subscription ID - // so ensureSubscription can skip the prompt and just resolve the tenant + // When no --project-id flag was given, check whether the azd environment already + // has a Foundry project configured from a previous init. If so, reuse it so the + // user isn't prompted to select a project they already chose. + if a.flags.projectResourceId == "" { + if existing, err := a.azdClient.Environment().GetValue(ctx, &azdext.GetEnvRequest{ + EnvName: a.environment.Name, + Key: "AZURE_AI_PROJECT_ID", + }); err == nil && existing.Value != "" { + a.flags.projectResourceId = existing.Value + log.Printf("Reusing existing Foundry project from environment: %s", existing.Value) + fmt.Println(output.WithGrayFormat( + "Using Foundry project from environment: %s", existing.Value, + )) + } + } + + // If --project-id is provided (or reused from environment), validate the ARM + // format and extract the subscription ID so ensureSubscription can skip the + // prompt and just resolve the tenant. if a.flags.projectResourceId != "" { projectDetails, err := extractProjectDetails(a.flags.projectResourceId) if err != nil { @@ -1291,12 +1325,28 @@ func (a *InitAction) downloadAgentYaml( fmt.Println(output.WithGrayFormat("✓ Manifest validated successfully")) agentId := agentManifest.Name + serviceName := strings.ReplaceAll(agentId, " ", "") // Use targetDir if provided, otherwise default to "src/{agentId}" - if targetDir == "" { + autoDir := targetDir == "" + if autoDir { targetDir = filepath.Join("src", agentId) } + // When the target directory was auto-computed (no --src flag), check for + // collisions with an existing directory or an existing azure.yaml service. + // If a collision is found, prompt for a new service name (or auto-suffix + // in no-prompt mode). + if autoDir { + targetDir, serviceName, err = a.resolveCollisions( + ctx, agentId, targetDir, serviceName, + ) + if err != nil { + return nil, "", err + } + } + a.serviceNameOverride = serviceName + // Safety checks for local container-based agents should happen before prompting for model SKU, etc. if a.isLocalFilePath(manifestPointer) { if _, isContainerAgent := agentManifest.Template.(agent_yaml.ContainerAgent); isContainerAgent { @@ -1474,7 +1524,7 @@ func (a *InitAction) addToProject(ctx context.Context, targetDir string, agentMa } serviceConfig := &azdext.ServiceConfig{ - Name: strings.ReplaceAll(agentDef.Name, " ", ""), + Name: a.serviceNameOverride, RelativePath: targetDir, Host: AiAgentHost, Language: "docker", @@ -1494,18 +1544,229 @@ func (a *InitAction) addToProject(ctx context.Context, targetDir string, agentMa return fmt.Errorf("adding agent service to project: %w", err) } - fmt.Printf("\nAdded your agent as a service entry named '%s' under the file azure.yaml.\n", agentDef.Name) + fmt.Printf( + "\nAdded your agent as a service entry named '%s' under the file azure.yaml.\n", + a.serviceNameOverride, + ) if projectID, _ := a.azdClient.Environment().GetValue(ctx, &azdext.GetEnvRequest{ EnvName: a.environment.Name, Key: "AZURE_AI_PROJECT_ID", }); projectID != nil && projectID.Value != "" { - fmt.Printf("To deploy your agent, use %s.\n", color.HiBlueString("azd deploy %s", agentDef.Name)) + fmt.Printf("To deploy your agent, use %s.\n", + color.HiBlueString("azd deploy %s", a.serviceNameOverride)) } else { - fmt.Printf("To provision and deploy the whole solution, use %s.\n", color.HiBlueString("azd up")) + fmt.Printf( + "To provision and deploy the whole solution, use %s.\n", + color.HiBlueString("azd up"), + ) } return nil } +// resolveCollisions checks whether the auto-computed target directory or +// service name already exist. When a collision is detected, the user is +// prompted for a new name (or a numeric suffix is appended in no-prompt +// mode). Returns the (possibly adjusted) targetDir and serviceName. +func (a *InitAction) resolveCollisions( + ctx context.Context, + agentId string, + targetDir string, + serviceName string, +) (string, string, error) { + dirExists := fileExists(targetDir) + + serviceExists := false + if a.projectConfig != nil { + for _, svc := range a.projectConfig.Services { + if svc.Name == serviceName { + serviceExists = true + break + } + } + } + + if !dirExists && !serviceExists { + return targetDir, serviceName, nil + } + + // Find the next available name for use as the default suggestion + // (interactive) or the final answer (no-prompt). + suggestion, suggestionDir, suggestionSvc, err := + a.nextAvailableName(agentId) + if err != nil { + return "", "", err + } + + if a.flags.NoPrompt { + log.Printf( + "Collision on %q; using %q", agentId, suggestion, + ) + return suggestionDir, suggestionSvc, nil + } + + // Build a collision message tailored to what actually collided. + collisionMsg := buildCollisionMessage( + dirExists, serviceExists, targetDir, serviceName, + ) + + // Interactive mode: let the user choose. + choices := []*azdext.SelectChoice{ + { + Label: "Overwrite existing", + Value: "overwrite", + }, + { + Label: "Use a different service name", + Value: "rename", + }, + } + + defaultIdx := int32(1) + resp, err := a.azdClient.Prompt().Select(ctx, &azdext.SelectRequest{ + Options: &azdext.SelectOptions{ + Message: collisionMsg, + Choices: choices, + SelectedIndex: &defaultIdx, + }, + }) + if err != nil { + if exterrors.IsCancellation(err) { + return "", "", exterrors.Cancelled( + "initialization was cancelled", + ) + } + return "", "", fmt.Errorf( + "prompting for collision resolution: %w", err, + ) + } + + if choices[*resp.Value].Value == "overwrite" { + return targetDir, serviceName, nil + } + + // Prompt for a new name — default to the next available suffix. + nameResp, err := a.azdClient.Prompt().Prompt(ctx, &azdext.PromptRequest{ + Options: &azdext.PromptOptions{ + Message: "Enter a new service name for this agent", + DefaultValue: suggestion, + IgnoreHintKeys: true, + }, + }) + if err != nil { + if exterrors.IsCancellation(err) { + return "", "", exterrors.Cancelled( + "initialization was cancelled", + ) + } + return "", "", fmt.Errorf( + "prompting for new service name: %w", err, + ) + } + + newName := strings.TrimSpace(nameResp.Value) + if newName == "" { + newName = suggestion + } + + newDir, newSvc, err := validateRenameInput(newName) + if err != nil { + return "", "", err + } + return newDir, newSvc, nil +} + +// validateRenameInput validates a user-provided rename input and returns +// the target directory and sanitized service name. It rejects names with +// path separators, dot segments, or invalid service-name characters. +func validateRenameInput(newName string) (string, string, error) { + if filepath.IsAbs(newName) || + strings.ContainsAny(newName, `/\`) || + newName == "." || + newName == ".." { + return "", "", fmt.Errorf( + "invalid service name %q: name must be a single directory"+ + " name without path separators or dot segments", + newName, + ) + } + + newSvc := strings.ReplaceAll(newName, " ", "") + if err := azdext.ValidateServiceName(newSvc); err != nil { + return "", "", fmt.Errorf( + "invalid service name %q: %w", newName, err, + ) + } + + newDir := filepath.Join("src", newName) + return newDir, newSvc, nil +} + +// buildCollisionMessage returns a user-facing prompt string tailored to the +// type of collision detected (directory, service name, or both). +func buildCollisionMessage( + dirExists, serviceExists bool, + targetDir, serviceName string, +) string { + switch { + case dirExists && serviceExists: + return fmt.Sprintf( + "A service named '%s' and its directory '%s' already exist."+ + " Overwrite or use a different name?", + serviceName, targetDir, + ) + case serviceExists: + return fmt.Sprintf( + "A service named '%s' already exists in your azure.yaml."+ + " Overwrite it or use a different name?", + serviceName, + ) + default: // dirExists only + return fmt.Sprintf( + "The directory '%s' already exists."+ + " Overwrite it or use a different name?", + targetDir, + ) + } +} + +// nextAvailableName finds the next unused name by appending -2, -3, etc. +// Returns the candidate name, directory, and service name. +func (a *InitAction) nextAvailableName( + agentId string, +) (string, string, string, error) { + const maxAttempts = 100 + for i := 2; i <= maxAttempts; i++ { + candidate := fmt.Sprintf("%s-%d", agentId, i) + candidateDir := filepath.Join("src", candidate) + candidateSvc := strings.ReplaceAll(candidate, " ", "") + + if fileExists(candidateDir) { + continue + } + + svcTaken := false + if a.projectConfig != nil { + for _, svc := range a.projectConfig.Services { + if svc.Name == candidateSvc { + svcTaken = true + break + } + } + } + if svcTaken { + continue + } + + return candidate, candidateDir, candidateSvc, nil + } + + return "", "", "", fmt.Errorf( + "could not find a unique name after %d attempts "+ + "(tried %s-2 through %s-%d)", + maxAttempts-1, agentId, agentId, maxAttempts, + ) +} + func (a *InitAction) populateContainerSettings( ctx context.Context, manifestResources *agent_yaml.ContainerResources, diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go index d31e9c246a9..5720686bfcb 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go @@ -466,7 +466,7 @@ func TestCheckNotDirectory_NoSuggestionForAgentDefinition(t *testing.T) { t.Parallel() dir := t.TempDir() - // An AgentDefinition has "kind" at root but no "template" — should NOT + // An AgentDefinition has "kind" at root but no "template" ΓÇö should NOT // be suggested as a manifest file. defContent := "kind: hosted\nname: my-agent\n" //nolint:gosec // test fixture file permissions are intentional @@ -829,3 +829,378 @@ func TestApplyPositionalArg_NonExistentYamlSetsManifest(t *testing.T) { t.Errorf("manifestPointer = %q, want %q", flags.manifestPointer, yamlPath) } } + +// --------------------------------------------------------------------------- +// validateRenameInput (covers PR review ΓÇö input validation for user-provided +// rename names in resolveCollisions) +// --------------------------------------------------------------------------- + +func TestValidateRenameInput(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantDir string + wantSvc string + wantErr bool + errContain string + }{ + { + name: "simple valid name", + input: "my-agent", + wantDir: filepath.Join("src", "my-agent"), + wantSvc: "my-agent", + }, + { + name: "name with spaces produces valid svc", + input: "my agent", + wantDir: filepath.Join("src", "my agent"), + wantSvc: "myagent", + }, + { + name: "path separator forward slash rejected", + input: "../escape", + wantErr: true, + errContain: "path separators or dot segments", + }, + { + name: "path separator backslash rejected", + input: `sub\dir`, + wantErr: true, + errContain: "path separators or dot segments", + }, + { + name: "single dot rejected", + input: ".", + wantErr: true, + errContain: "path separators or dot segments", + }, + { + name: "double dot rejected", + input: "..", + wantErr: true, + errContain: "path separators or dot segments", + }, + { + name: "absolute path rejected", + input: "/etc/passwd", + wantErr: true, + errContain: "path separators or dot segments", + }, + { + name: "empty name fails service validation", + input: "", + wantErr: true, + errContain: "invalid service name", + }, + { + name: "invalid characters fail service validation", + input: "agent@name!", + wantErr: true, + errContain: "invalid service name", + }, + { + name: "name with dots and hyphens is valid", + input: "agent.v2-beta", + wantDir: filepath.Join("src", "agent.v2-beta"), + wantSvc: "agent.v2-beta", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + gotDir, gotSvc, err := validateRenameInput(tt.input) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + if tt.errContain != "" && + !strings.Contains(err.Error(), tt.errContain) { + t.Errorf("error = %q, want containing %q", + err.Error(), tt.errContain) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if gotDir != tt.wantDir { + t.Errorf("dir = %q, want %q", gotDir, tt.wantDir) + } + if gotSvc != tt.wantSvc { + t.Errorf("svc = %q, want %q", gotSvc, tt.wantSvc) + } + }) + } +} + +// --------------------------------------------------------------------------- +// buildCollisionMessage (covers PR review ΓÇö tailored collision messages) +// --------------------------------------------------------------------------- + +func TestBuildCollisionMessage(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + dirExists bool + serviceExists bool + targetDir string + serviceName string + wantContain string + wantNotContain string + }{ + { + name: "both collisions mentions service and directory", + dirExists: true, + serviceExists: true, + targetDir: "src/agent", + serviceName: "agent", + wantContain: "src/agent", + }, + { + name: "service-only collision mentions azure.yaml", + dirExists: false, + serviceExists: true, + targetDir: "src/agent", + serviceName: "agent", + wantContain: "azure.yaml", + }, + { + name: "dir-only collision does not mention azure.yaml", + dirExists: true, + serviceExists: false, + targetDir: "src/agent", + serviceName: "agent", + wantContain: "src/agent", + wantNotContain: "azure.yaml", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + msg := buildCollisionMessage( + tt.dirExists, tt.serviceExists, + tt.targetDir, tt.serviceName, + ) + if !strings.Contains(msg, tt.wantContain) { + t.Errorf("message = %q, want containing %q", + msg, tt.wantContain) + } + if tt.wantNotContain != "" && + strings.Contains(msg, tt.wantNotContain) { + t.Errorf("message = %q, should NOT contain %q", + msg, tt.wantNotContain) + } + }) + } +} + +// --------------------------------------------------------------------------- +// nextAvailableName (covers PR review ΓÇö collision-resolution naming logic) +// --------------------------------------------------------------------------- + +func TestNextAvailableName(t *testing.T) { + tests := []struct { + name string + agentId string + existingDirs []string // dirs to create under src/ + existingSvcs []string // service names in projectConfig + wantCandidate string + wantDir string + wantSvc string + wantErr bool + }{ + { + name: "no collisions picks -2", + agentId: "my-agent", + wantCandidate: "my-agent-2", + wantDir: filepath.Join("src", "my-agent-2"), + wantSvc: "my-agent-2", + }, + { + name: "dir collision skips to -3", + agentId: "my-agent", + existingDirs: []string{"my-agent-2"}, + wantCandidate: "my-agent-3", + wantDir: filepath.Join("src", "my-agent-3"), + wantSvc: "my-agent-3", + }, + { + name: "service collision skips to -3", + agentId: "my-agent", + existingSvcs: []string{"my-agent-2"}, + wantCandidate: "my-agent-3", + wantDir: filepath.Join("src", "my-agent-3"), + wantSvc: "my-agent-3", + }, + { + name: "both dir and svc collisions skip", + agentId: "my-agent", + existingDirs: []string{"my-agent-2"}, + existingSvcs: []string{"my-agent-3"}, + wantCandidate: "my-agent-4", + wantDir: filepath.Join("src", "my-agent-4"), + wantSvc: "my-agent-4", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + for _, d := range tt.existingDirs { + dirPath := filepath.Join("src", d) + //nolint:gosec // test fixture directory permissions are intentional + if err := os.MkdirAll(dirPath, 0o755); err != nil { + t.Fatalf("setup: MkdirAll(%q): %v", dirPath, err) + } + } + + var projectCfg *azdext.ProjectConfig + if len(tt.existingSvcs) > 0 { + svcs := make(map[string]*azdext.ServiceConfig, len(tt.existingSvcs)) + for _, svcName := range tt.existingSvcs { + svcs[svcName] = &azdext.ServiceConfig{Name: svcName} + } + projectCfg = &azdext.ProjectConfig{Services: svcs} + } + + action := &InitAction{projectConfig: projectCfg} + candidate, dir, svc, err := action.nextAvailableName(tt.agentId) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if candidate != tt.wantCandidate { + t.Errorf("candidate = %q, want %q", + candidate, tt.wantCandidate) + } + if dir != tt.wantDir { + t.Errorf("dir = %q, want %q", dir, tt.wantDir) + } + if svc != tt.wantSvc { + t.Errorf("svc = %q, want %q", svc, tt.wantSvc) + } + }) + } +} + +// --------------------------------------------------------------------------- +// resolveCollisions ΓÇö no collision / no-prompt paths +// (covers PR review ΓÇö collision resolution unit tests) +// --------------------------------------------------------------------------- + +func TestResolveCollisions_NoCollision(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + action := &InitAction{ + flags: &initFlags{rootFlagsDefinition: &rootFlagsDefinition{}}, + } + + dir, svc, err := action.resolveCollisions( + t.Context(), "agent", + filepath.Join("src", "agent"), "agent", + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if dir != filepath.Join("src", "agent") { + t.Errorf("dir = %q, want %q", + dir, filepath.Join("src", "agent")) + } + if svc != "agent" { + t.Errorf("svc = %q, want %q", svc, "agent") + } +} + +func TestResolveCollisions_NoPrompt(t *testing.T) { + tests := []struct { + name string + agentId string + existingDirs []string + existingSvcs []string + wantDir string + wantSvc string + }{ + { + name: "dir-only collision auto-suffixes", + agentId: "agent", + existingDirs: []string{"agent"}, + wantDir: filepath.Join("src", "agent-2"), + wantSvc: "agent-2", + }, + { + name: "service-only collision auto-suffixes", + agentId: "agent", + existingSvcs: []string{"agent"}, + wantDir: filepath.Join("src", "agent-2"), + wantSvc: "agent-2", + }, + { + name: "both collisions auto-suffix", + agentId: "agent", + existingDirs: []string{"agent"}, + existingSvcs: []string{"agent"}, + wantDir: filepath.Join("src", "agent-2"), + wantSvc: "agent-2", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + for _, d := range tt.existingDirs { + dirPath := filepath.Join("src", d) + //nolint:gosec // test fixture directory permissions are intentional + if err := os.MkdirAll(dirPath, 0o755); err != nil { + t.Fatalf("setup: MkdirAll(%q): %v", dirPath, err) + } + } + + var projectCfg *azdext.ProjectConfig + svcs := make(map[string]*azdext.ServiceConfig, len(tt.existingSvcs)) + for _, svcName := range tt.existingSvcs { + svcs[svcName] = &azdext.ServiceConfig{Name: svcName} + } + if len(svcs) > 0 { + projectCfg = &azdext.ProjectConfig{Services: svcs} + } + + action := &InitAction{ + projectConfig: projectCfg, + flags: &initFlags{ + rootFlagsDefinition: &rootFlagsDefinition{ + NoPrompt: true, + }, + }, + } + + targetDir := filepath.Join("src", tt.agentId) + dir, svc, err := action.resolveCollisions( + t.Context(), tt.agentId, targetDir, tt.agentId, + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if dir != tt.wantDir { + t.Errorf("dir = %q, want %q", dir, tt.wantDir) + } + if svc != tt.wantSvc { + t.Errorf("svc = %q, want %q", svc, tt.wantSvc) + } + }) + } +}