-
Notifications
You must be signed in to change notification settings - Fork 315
feat(agents): add azd ai agent delete command #8519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,226 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "errors" | ||
| "fmt" | ||
| "log" | ||
| "net/http" | ||
|
|
||
| "azureaiagent/internal/exterrors" | ||
| "azureaiagent/internal/pkg/agents/agent_api" | ||
|
|
||
| "github.com/Azure/azure-sdk-for-go/sdk/azcore" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/azdext" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| type deleteFlags struct { | ||
| name string | ||
| force bool | ||
| output string | ||
| noPrompt bool | ||
| } | ||
|
|
||
| func newDeleteCommand(extCtx *azdext.ExtensionContext) *cobra.Command { | ||
| flags := &deleteFlags{} | ||
| extCtx = ensureExtensionContext(extCtx) | ||
|
|
||
| cmd := &cobra.Command{ | ||
| Use: "delete [name]", | ||
| Short: "Delete a hosted agent.", | ||
| Long: `Delete a hosted agent and all of its versions. | ||
|
|
||
| If the agent has active sessions, deletion will fail unless --force is passed. | ||
| Use --force to terminate active sessions and delete the agent. | ||
|
|
||
| The agent name is resolved from the azd environment when omitted.`, | ||
| Example: ` # Delete agent (auto-resolves name from azure.yaml) | ||
| azd ai agent delete | ||
|
|
||
| # Delete a specific agent by name | ||
| azd ai agent delete my-agent | ||
|
|
||
| # Force-delete even if active sessions exist | ||
| azd ai agent delete my-agent --force`, | ||
| Args: cobra.MaximumNArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if len(args) > 0 { | ||
| flags.name = args[0] | ||
| } | ||
| flags.output = extCtx.OutputFormat | ||
| flags.noPrompt = extCtx.NoPrompt | ||
|
|
||
| ctx := azdext.WithAccessToken(cmd.Context()) | ||
|
|
||
| action := &DeleteAction{flags: flags} | ||
| return action.Run(ctx) | ||
| }, | ||
| } | ||
|
|
||
| cmd.Flags().BoolVar( | ||
| &flags.force, "force", false, | ||
| "Force deletion even if the agent has active sessions", | ||
| ) | ||
|
|
||
| azdext.RegisterFlagOptions(cmd, azdext.FlagOptions{ | ||
| Name: "output", | ||
| AllowedValues: []string{"json", "none"}, | ||
| Default: "none", | ||
| }) | ||
|
|
||
| return cmd | ||
| } | ||
|
|
||
| // DeleteAction implements the agent delete command. | ||
| type DeleteAction struct { | ||
| flags *deleteFlags | ||
| } | ||
|
|
||
| func (a *DeleteAction) Run(ctx context.Context) error { | ||
| azdClient, err := azdext.NewAzdClient() | ||
| if err != nil { | ||
| return fmt.Errorf("failed to create azd client: %w", err) | ||
| } | ||
| defer azdClient.Close() | ||
|
|
||
| info, err := resolveAgentServiceFromProject(ctx, azdClient, a.flags.name, a.flags.noPrompt) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| agentName := info.AgentName | ||
| if agentName == "" { | ||
| agentName = a.flags.name | ||
| } | ||
| if agentName == "" { | ||
| return exterrors.Validation( | ||
| exterrors.CodeInvalidAgentName, | ||
| "agent name is required but could not be resolved", | ||
| "provide the agent name as a positional argument or "+ | ||
| "deploy the agent with 'azd up' first", | ||
| ) | ||
| } | ||
|
v1212 marked this conversation as resolved.
|
||
|
|
||
| // Confirmation prompt (skip in --no-prompt mode) | ||
| if !a.flags.noPrompt { | ||
| message := fmt.Sprintf("Delete agent %q and all its versions?", agentName) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we support deleting specific versions in the API? |
||
| if a.flags.force { | ||
| message = fmt.Sprintf( | ||
| "Force-delete agent %q? This will terminate all active sessions.", | ||
| agentName, | ||
| ) | ||
| } | ||
| defaultValue := false | ||
| resp, promptErr := azdClient.Prompt().Confirm(ctx, &azdext.ConfirmRequest{ | ||
| Options: &azdext.ConfirmOptions{ | ||
| Message: message, | ||
| DefaultValue: &defaultValue, | ||
| }, | ||
| }) | ||
| if promptErr != nil { | ||
| if exterrors.IsCancellation(promptErr) { | ||
| return exterrors.Cancelled("delete cancelled") | ||
| } | ||
| return fmt.Errorf("prompting for confirmation: %w", promptErr) | ||
| } | ||
| if resp.Value == nil || !*resp.Value { | ||
| return exterrors.Cancelled("delete cancelled by user") | ||
| } | ||
|
v1212 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| endpoint, err := resolveAgentEndpoint(ctx, "", "") | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| credential, err := newAgentCredential() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| client := agent_api.NewAgentClient(endpoint, credential) | ||
|
|
||
| result, err := client.DeleteAgent(ctx, agentName, DefaultAgentAPIVersion, a.flags.force) | ||
| if err != nil { | ||
| return classifyDeleteError(err, agentName) | ||
| } | ||
|
|
||
| // Best-effort: clear AGENT_{KEY}_NAME and AGENT_{KEY}_VERSION env vars | ||
| a.cleanupEnvVars(ctx, azdClient, info.ServiceName) | ||
|
|
||
| switch a.flags.output { | ||
| case "json": | ||
| data, jsonErr := json.MarshalIndent(result, "", " ") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there actually a valid json response coming from delete? Aren't deletes usually just an empty response with 200 status code? |
||
| if jsonErr != nil { | ||
| return fmt.Errorf("failed to marshal response: %w", jsonErr) | ||
| } | ||
| fmt.Println(string(data)) | ||
| default: | ||
| fmt.Printf("Agent %q deleted.\n", agentName) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // cleanupEnvVars removes AGENT_{KEY}_NAME, AGENT_{KEY}_VERSION, and | ||
| // AGENT_{KEY}_ENDPOINT from the azd environment after a successful delete. | ||
| // The SDK has no DeleteValue API, so we set values to empty string as a workaround. | ||
| func (a *DeleteAction) cleanupEnvVars(ctx context.Context, azdClient *azdext.AzdClient, serviceName string) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should also attempt to clean up any saved conversation/session IDs. We should have similar handling already existing in our |
||
| if serviceName == "" { | ||
| return | ||
| } | ||
|
|
||
| envResp, err := azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{}) | ||
| if err != nil { | ||
| return | ||
| } | ||
| envName := envResp.Environment.Name | ||
|
|
||
| serviceKey := toServiceKey(serviceName) | ||
| keys := []string{ | ||
| fmt.Sprintf("AGENT_%s_NAME", serviceKey), | ||
| fmt.Sprintf("AGENT_%s_VERSION", serviceKey), | ||
| fmt.Sprintf("AGENT_%s_ENDPOINT", serviceKey), | ||
| } | ||
|
|
||
| for _, key := range keys { | ||
| if _, err := azdClient.Environment().SetValue(ctx, &azdext.SetEnvRequest{ | ||
| EnvName: envName, | ||
| Key: key, | ||
| Value: "", | ||
| }); err != nil { | ||
| log.Printf("delete: failed to clear env var %s: %v", key, err) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // classifyDeleteError maps Azure API errors from the delete operation into | ||
| // user-friendly typed errors. Exported for testing. | ||
| func classifyDeleteError(err error, agentName string) error { | ||
| if respErr, ok := errors.AsType[*azcore.ResponseError](err); ok { | ||
| switch respErr.StatusCode { | ||
| case http.StatusNotFound: | ||
| return exterrors.Validation( | ||
| exterrors.CodeAgentNotFound, | ||
| fmt.Sprintf("agent %q not found", agentName), | ||
| "use 'azd ai agent show' to verify the agent exists", | ||
| ) | ||
| case http.StatusConflict: | ||
| return exterrors.Validation( | ||
| exterrors.CodeAgentHasActiveSessions, | ||
| fmt.Sprintf( | ||
| "agent %q has active sessions and cannot be deleted", | ||
| agentName, | ||
| ), | ||
| "pass --force to terminate active sessions and delete the agent, "+ | ||
| "or delete sessions first with 'azd ai agent sessions delete'", | ||
| ) | ||
| } | ||
| } | ||
| return exterrors.ServiceFromAzure(err, exterrors.OpDeleteAgent) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "errors" | ||
| "net/http" | ||
| "testing" | ||
|
|
||
| "azureaiagent/internal/exterrors" | ||
|
|
||
| "github.com/Azure/azure-sdk-for-go/sdk/azcore" | ||
| "github.com/azure/azure-dev/cli/azd/pkg/azdext" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestDeleteCommand_AcceptsPositionalArg(t *testing.T) { | ||
| cmd := newDeleteCommand(nil) | ||
| err := cmd.Args(cmd, []string{"my-agent"}) | ||
| assert.NoError(t, err) | ||
| } | ||
|
|
||
| func TestDeleteCommand_AcceptsNoArgs(t *testing.T) { | ||
| cmd := newDeleteCommand(nil) | ||
| err := cmd.Args(cmd, []string{}) | ||
| assert.NoError(t, err) | ||
| } | ||
|
|
||
| func TestDeleteCommand_RejectsMultipleArgs(t *testing.T) { | ||
| cmd := newDeleteCommand(nil) | ||
| err := cmd.Args(cmd, []string{"svc1", "svc2"}) | ||
| assert.Error(t, err) | ||
| } | ||
|
|
||
| func TestDeleteCommand_ForceFlag(t *testing.T) { | ||
| cmd := newDeleteCommand(nil) | ||
| flag := cmd.Flags().Lookup("force") | ||
| if flag == nil { | ||
| t.Fatal("expected --force flag to be registered") | ||
| } | ||
| if flag.DefValue != "false" { | ||
| t.Fatalf("expected --force default false, got %q", flag.DefValue) | ||
| } | ||
| } | ||
|
|
||
| func TestDeleteCommand_OutputFlagAnnotation(t *testing.T) { | ||
| cmd := newDeleteCommand(nil) | ||
| // RegisterFlagOptions stores allowed values in annotations | ||
| require.NotNil(t, cmd.Annotations) | ||
| } | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Error classification tests — calls the real classifyDeleteError from delete.go | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| func TestDeleteAgent_404_ProducesValidationError(t *testing.T) { | ||
| azErr := &azcore.ResponseError{ | ||
| StatusCode: http.StatusNotFound, | ||
| ErrorCode: "not_found", | ||
| } | ||
|
|
||
| result := classifyDeleteError(azErr, "my-agent") | ||
| require.Error(t, result) | ||
|
|
||
| var localErr *azdext.LocalError | ||
| require.True( | ||
| t, errors.As(result, &localErr), | ||
| "404 should produce a LocalError, got: %T", result, | ||
| ) | ||
| assert.Equal(t, exterrors.CodeAgentNotFound, localErr.Code) | ||
| assert.Contains(t, localErr.Message, "my-agent") | ||
| assert.Contains(t, localErr.Message, "not found") | ||
| } | ||
|
|
||
| func TestDeleteAgent_409_ProducesValidationError(t *testing.T) { | ||
| azErr := &azcore.ResponseError{ | ||
| StatusCode: http.StatusConflict, | ||
| ErrorCode: "conflict", | ||
| } | ||
|
|
||
| result := classifyDeleteError(azErr, "my-agent") | ||
| require.Error(t, result) | ||
|
|
||
| var localErr *azdext.LocalError | ||
| require.True( | ||
| t, errors.As(result, &localErr), | ||
| "409 should produce a LocalError, got: %T", result, | ||
| ) | ||
| assert.Equal(t, exterrors.CodeAgentHasActiveSessions, localErr.Code) | ||
| assert.Contains(t, localErr.Message, "active sessions") | ||
| assert.Contains(t, localErr.Suggestion, "--force") | ||
| } | ||
|
|
||
| func TestDeleteAgent_500_ProducesServiceError(t *testing.T) { | ||
| azErr := &azcore.ResponseError{ | ||
| StatusCode: http.StatusInternalServerError, | ||
| ErrorCode: "internal_error", | ||
| } | ||
|
|
||
| result := classifyDeleteError(azErr, "my-agent") | ||
| require.Error(t, result) | ||
|
|
||
| var svcErr *azdext.ServiceError | ||
| require.True( | ||
| t, errors.As(result, &svcErr), | ||
| "500 should produce a ServiceError, got: %T", result, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this block necessary? If we can't find the agent service in the project, will we run into other issues later on attempting the delete?