Address feedback

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: JeffreyCA

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

@@ -117,7 +117,7 @@ func runInitFromManifest(
 	}
 
 	// Get or create environment
-	env := getExistingEnvironment(ctx, flags, azdClient)
+	env := getExistingEnvironment(ctx, flags.env, azdClient)
 	if env == nil {
 		fmt.Println("Lets create a new default azd environment for your project.")
 		env, err = createNewEnvironment(ctx, azdClient, flags.env)
@@ -498,15 +498,15 @@ func ensureProject(ctx context.Context, flags *initFlags, azdClient *azdext.AzdC
 	return projectResponse.Project, nil
 }
 
-func getExistingEnvironment(ctx context.Context, flags *initFlags, azdClient *azdext.AzdClient) *azdext.Environment {
+func getExistingEnvironment(ctx context.Context, envName string, azdClient *azdext.AzdClient) *azdext.Environment {
 	var env *azdext.Environment
-	if flags.env == "" {
+	if envName == "" {
 		if envResponse, err := azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{}); err == nil {
 			env = envResponse.Environment
 		}
 	} else {
 		if envResponse, err := azdClient.Environment().Get(ctx, &azdext.GetEnvironmentRequest{
-			Name: flags.env,
+			Name: envName,
 		}); err == nil {
 			env = envResponse.Environment
 		}

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

@@ -551,7 +551,7 @@ func createNewEnvironment(
 	envName string,
 ) (*azdext.Environment, error) {
 	if envName != "" {
-		if existingEnv := getExistingEnvironment(ctx, &initFlags{env: envName}, azdClient); existingEnv != nil {
+		if existingEnv := getExistingEnvironment(ctx, envName, azdClient); existingEnv != nil {
 			return existingEnv, nil
 		}
 	}
@@ -575,8 +575,11 @@ func createNewEnvironment(
 		if exterrors.IsCancellation(err) {
 			return nil, exterrors.Cancelled("environment creation was cancelled")
 		}
-		if envName != "" && isEnvironmentAlreadyExistsError(err) {
-			if existingEnv := getExistingEnvironment(ctx, &initFlags{env: envName}, azdClient); existingEnv != nil {
+		// The workflow may have created the environment on disk before returning
+		// an "already exists" error (e.g. a concurrent process raced, or a previous
+		// partial run left env files behind). Re-fetch so we can reuse it.
+		if envName != "" && status.Code(err) == codes.AlreadyExists {
+			if existingEnv := getExistingEnvironment(ctx, envName, azdClient); existingEnv != nil {
 				return existingEnv, nil
 			}
 		}
@@ -588,7 +591,7 @@ func createNewEnvironment(
 	}
 
 	// Re-fetch the environment after creation
-	env := getExistingEnvironment(ctx, &initFlags{env: envName}, azdClient)
+	env := getExistingEnvironment(ctx, envName, azdClient)
 	if env == nil {
 		return nil, exterrors.Dependency(
 			exterrors.CodeEnvironmentNotFound,
@@ -600,24 +603,6 @@ func createNewEnvironment(
 	return env, nil
 }
 
-// isEnvironmentAlreadyExistsError detects the duplicate-environment failure returned from
-// `azd env new`. Today the workflow surfaces this as a gRPC Internal error whose message
-// embeds the underlying azd error text, so we fall back to checking the status message
-// after confirming the status code when no dedicated AlreadyExists status is available.
-func isEnvironmentAlreadyExistsError(err error) bool {
-	if status.Code(err) == codes.AlreadyExists {
-		return true
-	}
-
-	if status.Code(err) != codes.Internal {
-		return false
-	}
-
-	errText := strings.ToLower(status.Convert(err).Message())
-	return strings.Contains(errText, "creating new environment: environment") &&
-		strings.Contains(errText, "already exists")
-}
-
 // loadAzureContext reads the current Azure context values (tenant, subscription, location)
 // from the azd environment and returns a populated AzureContext. Missing values are left empty.
 func loadAzureContext(

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

@@ -5,7 +5,6 @@ package cmd
 
 import (
 	"context"
-	"fmt"
 	"net"
 	"testing"
 
@@ -16,15 +15,6 @@ import (
 	"google.golang.org/grpc/status"
 )
 
-func duplicateEnvironmentWorkflowError(envName string) string {
-	return fmt.Sprintf(
-		"failed to run workflow: error executing step command 'env new %s': "+
-			"creating new environment: environment '%s' already exists",
-		envName,
-		envName,
-	)
-}
-
 func TestExtractProjectDetails(t *testing.T) {
 	t.Parallel()
 
@@ -150,8 +140,8 @@ func TestCreateNewEnvironment_ReusesExistingEnvironmentAfterAlreadyExistsError(t
 	}
 	workflowServer := &testWorkflowServiceServer{
 		runErr: status.Error(
-			codes.Internal,
-			duplicateEnvironmentWorkflowError(envName),
+			codes.AlreadyExists,
+			"environment already exists",
 		),
 		runHook: func() {
 			envServer.environments[envName] = &azdext.Environment{Name: envName}

cli/azd/internal/grpcserver/workflow_service.go

@@ -5,8 +5,10 @@ package grpcserver
 
 import (
 	"context"
+	"errors"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
+	"github.com/azure/azure-dev/cli/azd/pkg/environment"
 	"github.com/azure/azure-dev/cli/azd/pkg/workflow"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -39,6 +41,9 @@ func (s *workflowService) Run(ctx context.Context, request *azdext.RunWorkflowRe
 	}
 
 	if err := s.runner.Run(ctx, azdWorkflow); err != nil {
+		if errors.Is(err, environment.ErrExists) {
+			return nil, status.Errorf(codes.AlreadyExists, "failed to run workflow: %v", err)
+		}
 		return nil, status.Errorf(codes.Internal, "failed to run workflow: %v", err)
 	}
 

cli/azd/internal/grpcserver/workflow_service_test.go

@@ -6,13 +6,17 @@ package grpcserver
 import (
 	"context"
 	"errors"
+	"fmt"
 	"testing"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
+	"github.com/azure/azure-dev/cli/azd/pkg/environment"
 	"github.com/azure/azure-dev/cli/azd/pkg/workflow"
 	"github.com/azure/azure-dev/cli/azd/test/mocks"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func Test_WorkflowService_Run_Success(t *testing.T) {
@@ -78,11 +82,36 @@ func Test_WorkflowService_Run_Success(t *testing.T) {
 
 		// Assert
 		require.Error(t, err)
+		require.Equal(t, codes.Internal, status.Code(err))
 		require.Nil(t, resp)
 
 		// Verify that the runner's ExecuteContext was invoked with the correct args.
 		testRunner.AssertCalled(t, "ExecuteContext", contextType, []string{"provision"})
 	})
+
+	t.Run("EnvironmentAlreadyExists", func(t *testing.T) {
+		envExistsErr := fmt.Errorf("creating environment 'myenv': %w", environment.ErrExists)
+		testRunner := &TestWorkflowRunner{}
+		runner := workflow.NewRunner(testRunner, mockContext.Console)
+		testRunner.On("ExecuteContext", contextType, mock.Anything).Return(envExistsErr)
+
+		service := NewWorkflowService(runner)
+
+		req := &azdext.RunWorkflowRequest{
+			Workflow: &azdext.Workflow{
+				Name: "env new",
+				Steps: []*azdext.WorkflowStep{
+					{Command: &azdext.WorkflowCommand{Args: []string{"env", "new", "myenv"}}},
+				},
+			},
+		}
+
+		resp, err := service.Run(*mockContext.Context, req)
+
+		require.Error(t, err)
+		require.Equal(t, codes.AlreadyExists, status.Code(err))
+		require.Nil(t, resp)
+	})
 }
 
 // Updated TestWorkflowRunner using testify/mock.

cli/azd/pkg/environment/manager.go

@@ -170,7 +170,7 @@ func (m *manager) Create(ctx context.Context, spec Spec) (*Environment, error) {
 	case err != nil:
 		return nil, fmt.Errorf("checking for existing environment: %w", err)
 	default:
-		return nil, fmt.Errorf("environment '%s' already exists", spec.Name)
+		return nil, fmt.Errorf("%w: '%s'", ErrExists, spec.Name)
 	}
 
 	env := New(spec.Name)