feat: back get/describe/delete on the OpenShell SDK (PR7a)

cmd/delete.go

@@ -1,21 +1,26 @@
 package cmd
 
 import (
+	"context"
 	"fmt"
+	"os"
+	"time"
 
+	"github.com/spf13/cobra"
 	"github.com/stackrox/harness-openshell/internal/gateway"
 	"github.com/stackrox/harness-openshell/internal/k8s"
+	"github.com/stackrox/harness-openshell/internal/openshell"
 	"github.com/stackrox/harness-openshell/internal/status"
-	"github.com/spf13/cobra"
 )
 
-func NewDeleteCmd(harnessDir, cli string) *cobra.Command {
+func NewDeleteCmd(harnessDir, cli string, newClient openshell.Factory) *cobra.Command {
 	var (
 		all       bool
 		sandboxes bool
 		providers bool
 		k8sFlag   bool
 	)
+	var gatewayName, workspace *string
 
 	cmd := &cobra.Command{
 		Use:   "delete [NAME...] [--all] [--providers] [--k8s]",
@@ -33,12 +38,27 @@ Examples:
 				return fmt.Errorf("specify sandbox name(s) or use --all, --sandboxes, --providers, --k8s")
 			}
 
-			gw := gateway.New(cli)
+			ctx := cmd.Context()
+			target := openshell.ResolveTarget(*gatewayName, *workspace, "", "", os.Getenv)
+
+			// The --k8s branch is CLI/kubectl-backed and needs no SDK client;
+			// open (and dial) one only when a sandbox/provider path will use it,
+			// so `delete --k8s` still works when the OpenShell API is down.
+			needsSDK := len(args) > 0 || all || sandboxes || providers
+			var client openshell.Client
+			if needsSDK {
+				var err error
+				client, err = newClient(ctx, target)
+				if err != nil {
+					return fmt.Errorf("create OpenShell client: %w", err)
+				}
+				defer client.Close()
+			}
 
 			// Targeted sandbox deletion
 			if len(args) > 0 {
 				for _, name := range args {
-					if err := gw.SandboxDelete(name); err != nil {
+					if err := client.DeleteSandbox(ctx, name); err != nil {
 						status.Failf("%s: %v", name, err)
 					} else {
 						status.OKf("Deleted sandbox %s", name)
@@ -49,23 +69,26 @@ Examples:
 				}
 			}
 
-			activeGW := gw.ActiveGateway()
-			if activeGW != "" {
-				status.Infof("Active gateway: %s", activeGW)
+			if target.Gateway != "" {
+				status.Infof("Active gateway: %s", target.Gateway)
 			} else {
 				status.Info("Active gateway: none")
 			}
 			fmt.Println()
 
 			if all || sandboxes {
-				teardownSandboxes(gw, activeGW)
+				deleteSandboxesSDK(ctx, client, target.Gateway)
 			}
 			if all || providers {
-				if err := teardownProviders(gw, activeGW); err != nil {
+				if err := deleteProvidersSDK(ctx, client, target.Gateway); err != nil {
 					return err
 				}
 			}
 			if all || k8sFlag {
+				// internal/gateway residual: the --k8s path stays CLI-backed
+				// until PR7b retires the legacy bridge. This is the only
+				// sanctioned use of internal/gateway and internal/k8s in delete.
+				gw := gateway.New(cli)
 				ns := k8s.DefaultNamespace()
 				gwCfg := resolveFirstRemoteGateway(harnessDir)
 				teardownK8s(gw, gwCfg, k8s.New("", ns), k8s.New("", ""))
@@ -80,6 +103,87 @@ Examples:
 	cmd.Flags().BoolVar(&sandboxes, "sandboxes", false, "Delete all sandboxes")
 	cmd.Flags().BoolVar(&providers, "providers", false, "Delete all providers")
 	cmd.Flags().BoolVar(&k8sFlag, "k8s", false, "Delete k8s resources")
+	gatewayName, workspace = registerTargetFlags(cmd)
 
 	return cmd
 }
+
+// deleteSandboxesSDK sweeps every sandbox in the target workspace over the
+// OpenShell SDK. It is delete's own SDK-backed sweep, intentionally mirroring
+// teardownSandboxes (cmd/teardown.go) on a different backing; the duplication is
+// a short-lived seam removed in PR7b when the CLI helper and teardown command
+// are retired, leaving this the single owner of the sweep.
+func deleteSandboxesSDK(ctx context.Context, client openshell.Client, activeGW string) {
+	status.Section("Sandboxes")
+	if activeGW == "" {
+		status.Info("No active gateway, skipping")
+		fmt.Println()
+		return
+	}
+
+	sandboxes, err := client.Sandboxes(ctx)
+	if err != nil {
+		status.Fail(fmt.Sprintf("could not list sandboxes: %v", err))
+		fmt.Println()
+		return
+	}
+	if len(sandboxes) == 0 {
+		status.Info("None running")
+	} else {
+		for _, s := range sandboxes {
+			status.Infof("Deleting %s", s.Name)
+			if err := client.DeleteSandbox(ctx, s.Name); err != nil {
+				status.Failf("failed to delete %s: %v", s.Name, err)
+			}
+		}
+	}
+	fmt.Println()
+}
+
+// deleteProvidersSDK sweeps every provider over the SDK, preserving the
+// running-sandbox guard from teardownProviders (cmd/teardown.go): providers are
+// refused while any sandbox is still up, with one brief retry to absorb a
+// mid-deletion race. Like deleteSandboxesSDK this is a short-lived duplicate of
+// the CLI helper, collapsed to the single owner in PR7b.
+func deleteProvidersSDK(ctx context.Context, client openshell.Client, activeGW string) error {
+	status.Section("Providers")
+	if activeGW == "" {
+		status.Info("No active gateway, skipping")
+		fmt.Println()
+		return nil
+	}
+
+	remaining, err := client.Sandboxes(ctx)
+	if err != nil {
+		return fmt.Errorf("could not check for running sandboxes: %w", err)
+	}
+	if len(remaining) > 0 {
+		// Sandbox may be mid-deletion — wait briefly and retry.
+		time.Sleep(2 * time.Second)
+		remaining, err = client.Sandboxes(ctx)
+		if err != nil {
+			return fmt.Errorf("rechecking sandboxes: %w", err)
+		}
+		if len(remaining) > 0 {
+			return fmt.Errorf("cannot delete providers with running sandboxes — run: harness delete --sandboxes")
+		}
+	}
+
+	providers, err := client.Providers(ctx)
+	if err != nil {
+		return fmt.Errorf("could not list providers: %w", err)
+	}
+	if len(providers) == 0 {
+		status.Info("None registered")
+	} else {
+		for _, p := range providers {
+			status.Infof("Deleting %s", p.Name)
+			if err := client.DeleteProvider(ctx, p.Name); err != nil {
+				status.Failf("failed to delete %s: %v", p.Name, err)
+			}
+		}
+	}
+
+	fmt.Println()
+	return nil
+}

cmd/delete_test.go

@@ -0,0 +1,116 @@
+package cmd
+
+import (
+	"context"
+	"testing"
+
+	"github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types"
+
+	"github.com/stackrox/harness-openshell/internal/openshell"
+	"github.com/stackrox/harness-openshell/internal/testutil"
+)
+
+// The delete tests use keepOpenFactory (executor_inference_test.go) so the
+// command's deferred Close doesn't shut the shared fake before the test can
+// assert the resources were actually removed, not merely that a log line printed.
+
+func sandboxNames(t *testing.T, c openshell.Client) []string {
+	t.Helper()
+	sandboxes, err := c.Sandboxes(context.Background())
+	if err != nil {
+		t.Fatalf("list sandboxes: %v", err)
+	}
+	names := make([]string, len(sandboxes))
+	for i, s := range sandboxes {
+		names[i] = s.Name
+	}
+	return names
+}
+
+func providerNames(t *testing.T, c openshell.Client) []string {
+	t.Helper()
+	providers, err := c.Providers(context.Background())
+	if err != nil {
+		t.Fatalf("list providers: %v", err)
+	}
+	names := make([]string, len(providers))
+	for i, p := range providers {
+		names[i] = p.Name
+	}
+	return names
+}
+
+func TestDeleteTargeted(t *testing.T) {
+	client, fc := testutil.NewFakeClient("default")
+	fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}})
+	fc.AddSandbox("default", &types.Sandbox{Name: "agent-b", Status: types.SandboxStatus{Phase: types.SandboxReady}})
+
+	cmd := NewDeleteCmd("", "", keepOpenFactory(client))
+	cmd.SetArgs([]string{"agent-a"})
+	if _, err := captureStdout(t, cmd.Execute); err != nil {
+		t.Fatalf("delete agent-a: %v", err)
+	}
+
+	remaining := sandboxNames(t, client)
+	if len(remaining) != 1 || remaining[0] != "agent-b" {
+		t.Errorf("targeted delete should remove only agent-a, got %v", remaining)
+	}
+}
+
+func TestDeleteSandboxesSweep(t *testing.T) {
+	client, fc := testutil.NewFakeClient("default")
+	fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}})
+	fc.AddSandbox("default", &types.Sandbox{Name: "agent-b", Status: types.SandboxStatus{Phase: types.SandboxReady}})
+
+	cmd := NewDeleteCmd("", "", keepOpenFactory(client))
+	cmd.SetArgs([]string{"--sandboxes", "--gateway", "prod"})
+	if _, err := captureStdout(t, cmd.Execute); err != nil {
+		t.Fatalf("delete --sandboxes: %v", err)
+	}
+
+	if remaining := sandboxNames(t, client); len(remaining) != 0 {
+		t.Errorf("--sandboxes should sweep every sandbox, got %v", remaining)
+	}
+}
+
+func TestDeleteProvidersGuard(t *testing.T) {
+	client, fc := testutil.NewFakeClient("default")
+	fc.AddSandbox("default", &types.Sandbox{Name: "agent-a", Status: types.SandboxStatus{Phase: types.SandboxReady}})
+	fc.AddProvider("default", &types.Provider{Name: "github", Type: "github"})
+
+	cmd := NewDeleteCmd("", "", keepOpenFactory(client))
+	cmd.SetArgs([]string{"--providers", "--gateway", "prod"})
+	_, err := captureStdout(t, cmd.Execute)
+	if err == nil {
+		t.Fatal("deleting providers with a running sandbox should be refused")
+	}
+	if !contains(err.Error(), "running sandboxes") {
+		t.Errorf("unexpected guard error: %v", err)
+	}
+
+	// The guard must prevent deletion, not delete-then-error: the provider survives.
+	if names := providerNames(t, client); len(names) != 1 || names[0] != "github" {
+		t.Errorf("guard should leave the provider untouched, got %v", names)
+	}
+}
+
+// Note: `delete --k8s`-only skipping the SDK client (CodeRabbit finding) is not
+// unit-tested — the --k8s path invokes the real, non-injectable teardownK8s,
+// which shells out to the ambient kubeconfig and would destructively act on a
+// live cluster. The gating (`needsSDK`) is a simple guard in delete.go.
+
+func TestDeleteProvidersSweep(t *testing.T) {
+	client, fc := testutil.NewFakeClient("default")
+	fc.AddProvider("default", &types.Provider{Name: "github", Type: "github"})
+	fc.AddProvider("default", &types.Provider{Name: "vertex", Type: "google-vertex-ai"})
+
+	cmd := NewDeleteCmd("", "", keepOpenFactory(client))
+	cmd.SetArgs([]string{"--providers", "--gateway", "prod"})
+	if _, err := captureStdout(t, cmd.Execute); err != nil {
+		t.Fatalf("delete --providers: %v", err)
+	}
+
+	if names := providerNames(t, client); len(names) != 0 {
+		t.Errorf("--providers should sweep every provider, got %v", names)
+	}
+}