diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go index bafb62d4f8a..438ea0ee124 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go @@ -74,6 +74,8 @@ func preprovisionHandler(ctx context.Context, azdClient *azdext.AzdClient, args switch svc.Host { case AiAgentHost: if err := prepareContainerSettings( + ctx, + azdClient, svc, args.Project.Path, ); err != nil { @@ -245,6 +247,8 @@ func predeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *az } if err := prepareContainerSettings( + ctx, + azdClient, svc, args.Project.Path, ); err != nil { @@ -757,6 +761,8 @@ func setEnvVar(ctx context.Context, azdClient *azdext.AzdClient, envName string, } func prepareContainerSettings( + ctx context.Context, + azdClient *azdext.AzdClient, svc *azdext.ServiceConfig, projectRoot string, ) error { @@ -805,15 +811,24 @@ func prepareContainerSettings( result.Cpu = project.DefaultCpu } - if err := project.SetAgentContainerSettings( + // Persist the resolved container settings back onto the service's inline + // properties, preserving the agent definition and other config keys. + containerPath, containerValue, err := project.SetAgentContainerSettings( svc, &project.ContainerSettings{Resources: result}, - ); err != nil { - return fmt.Errorf( - "failed to update agent container settings: %w", - err, - ) + ) + if err != nil { + return fmt.Errorf("failed to update agent container settings: %w", err) } + + if _, err := azdClient.Project().SetServiceConfigValue(ctx, &azdext.SetServiceConfigValueRequest{ + ServiceName: svc.GetName(), + Path: containerPath, + Value: containerValue, + }); err != nil { + return fmt.Errorf("persisting agent container settings: %w", err) + } + return nil } diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go index 02f1fa32367..a992986bb88 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen_test.go @@ -5,6 +5,7 @@ package cmd import ( "bytes" + "context" "os" "path/filepath" "strings" @@ -20,6 +21,109 @@ import ( "google.golang.org/protobuf/types/known/structpb" ) +type containerSettingsProjectServer struct { + azdext.UnimplementedProjectServiceServer + + mu sync.Mutex + addServiceCalls int + setServiceRequests []*azdext.SetServiceConfigValueRequest +} + +func (s *containerSettingsProjectServer) AddService( + _ context.Context, + _ *azdext.AddServiceRequest, +) (*azdext.EmptyResponse, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.addServiceCalls++ + return &azdext.EmptyResponse{}, nil +} + +func (s *containerSettingsProjectServer) SetServiceConfigValue( + _ context.Context, + req *azdext.SetServiceConfigValueRequest, +) (*azdext.EmptyResponse, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.setServiceRequests = append(s.setServiceRequests, req) + return &azdext.EmptyResponse{}, nil +} + +func TestPrepareContainerSettings_UsesTargetedConfigUpdate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + legacy bool + wantPath string + }{ + { + name: "inline service properties", + wantPath: "container", + }, + { + name: "legacy config properties", + legacy: true, + wantPath: "config.container", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + props, err := structpb.NewStruct(map[string]any{ + "kind": "hosted", + "name": "my-chat-agent", + "customField": "preserved", + "container": map[string]any{ + "resources": map[string]any{ + "cpu": "1", + }, + }, + }) + require.NoError(t, err) + + svc := &azdext.ServiceConfig{ + Name: "agent", + Host: AiAgentHost, + Image: "myregistry.azurecr.io/my-agent:${MY_TAG}", + } + if tt.legacy { + svc.Config = props + } else { + svc.AdditionalProperties = props + } + + server := &containerSettingsProjectServer{} + client := newProjectRecorderClient(t, server) + + require.NoError(t, prepareContainerSettings(t.Context(), client, svc, t.TempDir())) + + server.mu.Lock() + defer server.mu.Unlock() + + require.Zero(t, server.addServiceCalls, + "full service replacement would drop fields that are not modeled by the extension") + require.Len(t, server.setServiceRequests, 1) + + req := server.setServiceRequests[0] + require.Equal(t, "agent", req.ServiceName) + require.Equal(t, tt.wantPath, req.Path) + require.Equal(t, map[string]any{ + "resources": map[string]any{ + "cpu": "1", + "memory": project.DefaultMemory, + }, + }, req.Value.AsInterface()) + + require.Equal(t, "myregistry.azurecr.io/my-agent:${MY_TAG}", svc.Image) + require.Equal(t, "preserved", + project.ServiceConfigProps(svc).GetFields()["customField"].GetStringValue()) + }) + } +} + // TestPostdeployHandler_NonHostedAgent_NoOp verifies postdeployHandler returns nil // without any RPC calls when the service is an agent but not a hosted agent (no agent.yaml // with kind: hostedAgent). With service-level event handlers, the core filters by host type, @@ -159,8 +263,9 @@ func TestPrepareContainerSettings_DoesNotPersistResolvedFileRef( RelativePath: "src/echo", AdditionalProperties: props, } + client := newProjectRecorderClient(t, &containerSettingsProjectServer{}) - err = prepareContainerSettings(svc, root) + err = prepareContainerSettings(t.Context(), client, svc, root) require.NoError(t, err) require.Equal(t, "src/echo", svc.GetRelativePath()) @@ -188,8 +293,9 @@ func TestPrepareContainerSettings_PreservesNestedFileRef(t *testing.T) { Host: AiAgentHost, AdditionalProperties: props, } + client := newProjectRecorderClient(t, &containerSettingsProjectServer{}) - err = prepareContainerSettings(svc, t.TempDir()) + err = prepareContainerSettings(t.Context(), client, svc, t.TempDir()) require.NoError(t, err) deployments, ok := svc.GetAdditionalProperties(). @@ -227,8 +333,9 @@ func TestPrepareContainerSettings_NormalizesInlineEnvironment(t *testing.T) { Host: AiAgentHost, AdditionalProperties: props, } + client := newProjectRecorderClient(t, &containerSettingsProjectServer{}) - err = prepareContainerSettings(svc, t.TempDir()) + err = prepareContainerSettings(t.Context(), client, svc, t.TempDir()) require.NoError(t, err) require.Equal( @@ -243,7 +350,10 @@ func TestPrepareContainerSettings_NormalizesInlineEnvironment(t *testing.T) { func TestPrepareContainerSettings_WithoutProperties(t *testing.T) { t.Parallel() + client := newProjectRecorderClient(t, &containerSettingsProjectServer{}) err := prepareContainerSettings( + t.Context(), + client, &azdext.ServiceConfig{Name: "echo", Host: AiAgentHost}, t.TempDir(), ) diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go index 0506dbd1c47..0d17a3208d7 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go @@ -549,14 +549,20 @@ func UpsertAgentEnvVars(svc *azdext.ServiceConfig, kv map[string]string) error { // agent service's inline properties, preserving every other key (the agent // definition and the rest of the deploy/provision config). It mutates whichever // shape the service uses (the unified AdditionalProperties, or — for older -// projects — the config-nested struct). -func SetAgentContainerSettings(svc *azdext.ServiceConfig, container *ContainerSettings) error { +// projects — the config-nested struct). The returned path and value identify +// the exact mutation for callers that need to persist it through the azd host. +func SetAgentContainerSettings( + svc *azdext.ServiceConfig, + container *ContainerSettings, +) (string, *structpb.Value, error) { legacy := false + containerPath := "container" props := svc.GetAdditionalProperties() if props == nil || len(props.GetFields()) == 0 { if cfg := svc.GetConfig(); cfg != nil && len(cfg.GetFields()) > 0 { props = cfg legacy = true + containerPath = "config.container" } else { props = &structpb.Struct{} } @@ -567,16 +573,17 @@ func SetAgentContainerSettings(svc *azdext.ServiceConfig, container *ContainerSe containerStruct, err := MarshalStruct(container) if err != nil { - return fmt.Errorf("marshaling container settings: %w", err) + return "", nil, fmt.Errorf("marshaling container settings: %w", err) } - props.Fields["container"] = structpb.NewStructValue(containerStruct) + containerValue := structpb.NewStructValue(containerStruct) + props.Fields["container"] = containerValue if legacy { svc.Config = props } else { svc.AdditionalProperties = props } - return nil + return containerPath, containerValue, nil } // agentDefinitionFromStruct builds the ContainerAgent from an inline/config diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go index 2a719fe5ee7..ff848b24930 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go @@ -157,6 +157,58 @@ func TestAgentDefinitionFromService_NoDefinition(t *testing.T) { require.False(t, found) } +func TestSetAgentContainerSettings_ReturnsPersistenceTarget(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + legacy bool + wantPath string + }{ + { + name: "inline service properties", + wantPath: "container", + }, + { + name: "legacy config properties", + legacy: true, + wantPath: "config.container", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + props, err := structpb.NewStruct(map[string]any{"customField": "preserved"}) + require.NoError(t, err) + + svc := &azdext.ServiceConfig{} + if tt.legacy { + svc.Config = props + } else { + svc.AdditionalProperties = props + } + + path, value, err := SetAgentContainerSettings(svc, &ContainerSettings{ + Resources: &ResourceSettings{Cpu: "1", Memory: "2Gi"}, + }) + require.NoError(t, err) + require.Equal(t, tt.wantPath, path) + require.Equal(t, map[string]any{ + "resources": map[string]any{ + "cpu": "1", + "memory": "2Gi", + }, + }, value.AsInterface()) + + storedProps := ServiceConfigProps(svc) + require.Equal(t, "preserved", storedProps.GetFields()["customField"].GetStringValue()) + require.Same(t, value, storedProps.GetFields()["container"]) + }) + } +} + // TestAgentDefinition_ImageRidesOnCoreServiceField verifies the prebuilt image // maps onto the core ServiceConfig.Image field (which core binds and round-trips) // rather than the inline property bag, where core would strip it on reload. diff --git a/cli/azd/internal/grpcserver/project_service.go b/cli/azd/internal/grpcserver/project_service.go index 58f1fc704d0..1421db208af 100644 --- a/cli/azd/internal/grpcserver/project_service.go +++ b/cli/azd/internal/grpcserver/project_service.go @@ -7,9 +7,11 @@ import ( "context" "fmt" "log" + "sync" "github.com/azure/azure-dev/cli/azd/internal/mapper" "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/config" "github.com/azure/azure-dev/cli/azd/pkg/environment" "github.com/azure/azure-dev/cli/azd/pkg/environment/azdcontext" "github.com/azure/azure-dev/cli/azd/pkg/ext" @@ -31,6 +33,7 @@ type projectService struct { importManager *project.ImportManager lazyProjectConfig *lazy.Lazy[*project.ProjectConfig] ghCli *github.Cli + configMutationMu sync.Mutex } // NewProjectService creates a new project service instance with lazy-loaded dependencies. @@ -172,6 +175,9 @@ func (s *projectService) AddService(ctx context.Context, req *azdext.AddServiceR return nil, status.Error(codes.InvalidArgument, "service name cannot be empty") } + s.configMutationMu.Lock() + defer s.configMutationMu.Unlock() + azdContext, err := s.lazyAzdContext.GetValue() if err != nil { return nil, err @@ -376,6 +382,9 @@ func (s *projectService) SetConfigSection( return nil, status.Error(codes.InvalidArgument, "path cannot be empty") } + s.configMutationMu.Lock() + defer s.configMutationMu.Unlock() + azdContext, err := s.lazyAzdContext.GetValue() if err != nil { return nil, err @@ -424,6 +433,9 @@ func (s *projectService) SetConfigValue( return nil, status.Error(codes.InvalidArgument, "path cannot be empty") } + s.configMutationMu.Lock() + defer s.configMutationMu.Unlock() + azdContext, err := s.lazyAzdContext.GetValue() if err != nil { return nil, err @@ -472,6 +484,9 @@ func (s *projectService) UnsetConfig( return nil, status.Error(codes.InvalidArgument, "path cannot be empty") } + s.configMutationMu.Lock() + defer s.configMutationMu.Unlock() + azdContext, err := s.lazyAzdContext.GetValue() if err != nil { return nil, err @@ -646,6 +661,9 @@ func (s *projectService) SetServiceConfigSection( return nil, status.Error(codes.InvalidArgument, "service name cannot be empty") } + s.configMutationMu.Lock() + defer s.configMutationMu.Unlock() + azdContext, err := s.lazyAzdContext.GetValue() if err != nil { return nil, err @@ -710,6 +728,9 @@ func (s *projectService) SetServiceConfigValue( return nil, status.Error(codes.InvalidArgument, "path cannot be empty") } + s.configMutationMu.Lock() + defer s.configMutationMu.Unlock() + azdContext, err := s.lazyAzdContext.GetValue() if err != nil { return nil, err @@ -726,12 +747,18 @@ func (s *projectService) SetServiceConfigValue( return nil, err } - // Construct path to service config value: "services.." - servicePath := fmt.Sprintf("services.%s.%s", req.ServiceName, req.Path) + services, ok := cfg.Raw()["services"].(map[string]any) + if !ok { + return nil, fmt.Errorf("services configuration not found") + } + serviceConfig, ok := services[req.ServiceName].(map[string]any) + if !ok { + return nil, fmt.Errorf("service configuration for '%s' not found", req.ServiceName) + } // Convert protobuf Value to interface{} value := req.Value.AsInterface() - if err := cfg.Set(servicePath, value); err != nil { + if err := config.NewConfig(serviceConfig).Set(req.Path, value); err != nil { return nil, fmt.Errorf("failed to set service config value: %w", err) } @@ -771,6 +798,9 @@ func (s *projectService) UnsetServiceConfig( return nil, status.Error(codes.InvalidArgument, "path cannot be empty") } + s.configMutationMu.Lock() + defer s.configMutationMu.Unlock() + azdContext, err := s.lazyAzdContext.GetValue() if err != nil { return nil, err diff --git a/cli/azd/internal/grpcserver/project_service_test.go b/cli/azd/internal/grpcserver/project_service_test.go index 1e9e500b2f7..a580da72e23 100644 --- a/cli/azd/internal/grpcserver/project_service_test.go +++ b/cli/azd/internal/grpcserver/project_service_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "strings" + "sync" "sync/atomic" "testing" @@ -2649,6 +2650,86 @@ func TestProjectService_SetServiceConfigValue_HappyPath(t *testing.T) { require.NoError(t, err) } +func TestProjectService_SetServiceConfigValue_DottedServiceName(t *testing.T) { + t.Parallel() + svc := newProjectServiceWithYaml(t, `name: test-project +services: + my.agent: + host: appservice +`) + + _, err := svc.SetServiceConfigValue(t.Context(), &azdext.SetServiceConfigValueRequest{ + ServiceName: "my.agent", + Path: "container.resources.cpu", + Value: structpb.NewStringValue("2"), + }) + require.NoError(t, err) + + projectService := svc.(*projectService) + azdContext, err := projectService.lazyAzdContext.GetValue() + require.NoError(t, err) + cfg, err := project.LoadConfig(t.Context(), azdContext.ProjectPath()) + require.NoError(t, err) + services, found := cfg.GetMap("services") + require.True(t, found) + serviceConfig, ok := services["my.agent"].(map[string]any) + require.True(t, ok) + value, found := config.NewConfig(serviceConfig).Get("container.resources.cpu") + require.True(t, found) + require.Equal(t, "2", value) + require.NotContains(t, services, "my") +} + +func TestProjectService_SetServiceConfigValue_Concurrent(t *testing.T) { + t.Parallel() + svc := newProjectServiceWithYaml(t, yamlWithService) + paths := []string{ + "custom.one", + "custom.two", + "custom.three", + "custom.four", + "custom.five", + "custom.six", + "custom.seven", + "custom.eight", + "custom.nine", + "custom.ten", + } + start := make(chan struct{}) + errs := make(chan error, len(paths)) + var wg sync.WaitGroup + + for _, path := range paths { + wg.Go(func() { + <-start + _, err := svc.SetServiceConfigValue(t.Context(), &azdext.SetServiceConfigValueRequest{ + ServiceName: "api", + Path: path, + Value: structpb.NewStringValue(path), + }) + errs <- err + }) + } + close(start) + wg.Wait() + close(errs) + + for err := range errs { + require.NoError(t, err) + } + + projectService := svc.(*projectService) + azdContext, err := projectService.lazyAzdContext.GetValue() + require.NoError(t, err) + cfg, err := project.LoadConfig(t.Context(), azdContext.ProjectPath()) + require.NoError(t, err) + for _, path := range paths { + value, found := cfg.Get("services.api." + path) + require.True(t, found) + require.Equal(t, path, value) + } +} + func TestProjectService_UnsetServiceConfig_HappyPath(t *testing.T) { t.Parallel() svc := newProjectServiceWithYaml(t, yamlWithService)