feat(doctor): rewire online checks onto the SDK via injected Factory (PR1 S5)

cmd/doctor.go

@@ -1,15 +1,17 @@
 package cmd
 
 import (
+	"context"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
 
-	"github.com/stackrox/harness-openshell/internal/agent"
-	"github.com/stackrox/harness-openshell/internal/gateway"
 	"github.com/spf13/cobra"
+	"github.com/stackrox/harness-openshell/internal/agent"
+	"github.com/stackrox/harness-openshell/internal/openshell"
 	"gopkg.in/yaml.v3"
 )
 
@@ -22,11 +24,13 @@ type CheckResult struct {
 
 type CheckFunc func(cfg *agent.AgentConfig, cli, harnessDir string) []CheckResult
 
-func NewDoctorCmd(harnessDir, cli string) *cobra.Command {
+func NewDoctorCmd(harnessDir, cli string, newClient openshell.Factory) *cobra.Command {
 	var (
-		agentFile string
-		agentName string
-		output    string
+		agentFile   string
+		agentName   string
+		output      string
+		gatewayName string
+		workspace   string
 	)
 
 	cmd := &cobra.Command{
@@ -60,7 +64,16 @@ Phase 2 (online): if the gateway is reachable, checks provider registration.`,
 				results = append(results, fn(h.Agent, cli, harnessDir)...)
 			}
 
-			results = append(results, checkOnline(h.Agent, cli)...)
+			providerProfiles := make([]string, 0, len(h.Agent.Providers))
+			for _, p := range h.Agent.Providers {
+				providerProfiles = append(providerProfiles, p.Profile)
+			}
+			// Flag resolution order (AGENTS.md): explicit flag > OPENSHELL_* env
+			// var > default. Empty flag defaults let the env fallback apply; an
+			// unset gateway (flag and env both empty) skips Phase 2.
+			gw := resolveOnlineFlag(gatewayName, "OPENSHELL_GATEWAY", "")
+			ws := resolveOnlineFlag(workspace, "OPENSHELL_WORKSPACE", defaultDoctorWorkspace)
+			results = append(results, runOnlineChecks(cmd.Context(), newClient, gw, ws, providerProfiles)...)
 
 			if format != formatTable {
 				return printStructured(format, results)
@@ -80,6 +93,8 @@ Phase 2 (online): if the gateway is reachable, checks provider registration.`,
 	cmd.Flags().StringVarP(&agentFile, "file", "f", "", "Path to harness YAML")
 	cmd.Flags().StringVar(&agentName, "agent", "default", "Agent config name")
 	cmd.Flags().StringVarP(&output, "output", "o", "", "Output format (table, json, yaml)")
+	cmd.Flags().StringVar(&gatewayName, "gateway", "", "OpenShell registration name for online checks (Phase 2). Defaults to $OPENSHELL_GATEWAY; empty skips online checks.")
+	cmd.Flags().StringVar(&workspace, "workspace", "", "Workspace for provider registration checks (default \"default\"; overridable via $OPENSHELL_WORKSPACE)")
 
 	return cmd
 }
@@ -179,8 +194,8 @@ func checkRemoteDeps() []CheckResult {
 }
 
 type providerProfile struct {
-	ID          string              `yaml:"id"`
-	DisplayName string              `yaml:"display_name"`
+	ID          string               `yaml:"id"`
+	DisplayName string               `yaml:"display_name"`
 	Credentials []providerCredential `yaml:"credentials"`
 }
 
@@ -328,47 +343,126 @@ func loadProfileFromDisk(name, harnessDir string) *providerProfile {
 	return nil
 }
 
-func checkOnline(cfg *agent.AgentConfig, cli string) []CheckResult {
-	gw := gateway.New(cli)
-	if gw.ActiveGateway() == "" {
+// defaultDoctorWorkspace is the workspace used when neither --workspace nor
+// $OPENSHELL_WORKSPACE is set. sdkclient also defaults "" -> "default"; this
+// keeps the resolved value explicit for logging and table output.
+const defaultDoctorWorkspace = "default"
+
+// resolveOnlineFlag applies the repo's standard flag-resolution order
+// (AGENTS.md: explicit flag > OPENSHELL_* env var > default) for doctor's
+// online flags. An empty flag value is treated as "unset" so the env var can
+// take effect.
+func resolveOnlineFlag(flagVal, envKey, def string) string {
+	if flagVal != "" {
+		return flagVal
+	}
+	if v := os.Getenv(envKey); v != "" {
+		return v
+	}
+	return def
+}
+
+// runOnlineChecks performs Phase 2 (online) checks via the SDK. It is
+// non-fatal by construction: a missing --gateway or any client-construction
+// failure yields a single warn (Phase 2 skipped), never a fail, preserving
+// doctor's long-standing "online failures don't break the build" contract.
+func runOnlineChecks(ctx context.Context, newClient openshell.Factory, gatewayName, workspace string, providers []string) []CheckResult {
+	if gatewayName == "" {
 		return []CheckResult{{
 			Group:   "gateway",
 			Name:    "status",
 			Status:  "warn",
-			Message: "no active gateway (Phase 2 checks skipped)",
+			Message: "Phase 2 (online) checks skipped: no --gateway specified",
 		}}
 	}
 
-	_, err := gw.ProviderList()
+	client, err := newClient(ctx, openshell.Target{Gateway: gatewayName, Workspace: workspace})
 	if err != nil {
+		return []CheckResult{{
+			Group:   "gateway",
+			Name:    "status",
+			Status:  "warn",
+			Message: fmt.Sprintf("Phase 2 (online) checks skipped: %v", err),
+		}}
+	}
+	defer client.Close()
+
+	return checkOnlineSDK(ctx, client, providers)
+}
+
+// checkOnlineSDK reads gateway health and provider registration through the
+// SDK client. Health maps: healthy -> pass; ErrUnauthenticated -> fail (the
+// one actionable online failure); ErrUnavailable and any other error -> warn
+// (non-fatal). Provider rows are only produced when the gateway is healthy.
+func checkOnlineSDK(ctx context.Context, client openshell.Client, providers []string) []CheckResult {
+	h, err := client.Health(ctx)
+	switch {
+	case err == nil && h.Healthy:
+		// fall through to provider checks below
+	case errors.Is(err, openshell.ErrUnauthenticated):
+		return []CheckResult{{
+			Group:   "gateway",
+			Name:    "status",
+			Status:  "fail",
+			Message: "authentication failed — check gateway credentials",
+		}}
+	case errors.Is(err, openshell.ErrUnavailable):
 		return []CheckResult{{
 			Group:   "gateway",
 			Name:    "status",
 			Status:  "warn",
 			Message: "gateway not reachable (Phase 2 checks skipped)",
 		}}
+	case err != nil:
+		return []CheckResult{{
+			Group:   "gateway",
+			Name:    "status",
+			Status:  "warn",
+			Message: fmt.Sprintf("gateway health check failed: %v (Phase 2 checks skipped)", err),
+		}}
+	default: // err == nil but not healthy
+		return []CheckResult{{
+			Group:   "gateway",
+			Name:    "status",
+			Status:  "warn",
+			Message: "gateway reports unhealthy (Phase 2 checks skipped)",
+		}}
 	}
 
-	var results []CheckResult
-	results = append(results, CheckResult{
+	results := []CheckResult{{
 		Group:   "gateway",
 		Name:    "status",
 		Status:  "pass",
 		Message: "connected",
-	})
+	}}
 
-	for _, p := range cfg.Providers {
-		if gw.ProviderGet(p.Profile) == nil {
+	provs, err := client.Providers(ctx)
+	if err != nil {
+		results = append(results, CheckResult{
+			Group:   "gateway",
+			Name:    "providers",
+			Status:  "warn",
+			Message: fmt.Sprintf("could not list providers: %v", err),
+		})
+		return results
+	}
+
+	registered := make(map[string]bool, len(provs))
+	for _, p := range provs {
+		registered[p.Name] = true
+	}
+	for _, name := range providers {
+		if registered[name] {
 			results = append(results, CheckResult{
 				Group:   "gateway",
-				Name:    p.Profile,
+				Name:    name,
 				Status:  "pass",
 				Message: "registered",
 			})
 		} else {
 			results = append(results, CheckResult{
 				Group:   "gateway",
-				Name:    p.Profile,
+				Name:    name,
 				Status:  "warn",
 				Message: "not registered (will be registered on apply)",
 			})

cmd/doctor_test.go

@@ -1,11 +1,16 @@
 package cmd
 
 import (
+	"context"
 	"os"
 	"path/filepath"
 	"testing"
 
+	fake "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/fake"
+	"github.com/NVIDIA/OpenShell/sdk/go/openshell/v1/types"
 	"github.com/stackrox/harness-openshell/internal/agent"
+	"github.com/stackrox/harness-openshell/internal/openshell"
+	"github.com/stackrox/harness-openshell/internal/testutil"
 )
 
 func TestCheckOpenShell_Found(t *testing.T) {
@@ -251,3 +256,84 @@ func writeProviderProfile(t *testing.T, harnessDir, name, content string) {
 		t.Fatal(err)
 	}
 }
+
+func TestCheckOnlineSDK_Healthy(t *testing.T) {
+	c := testutil.NewFake("default", fake.WithHealthResult(&types.HealthResult{Healthy: true, Version: "1.0.0"}))
+	results := checkOnlineSDK(context.Background(), c, nil)
+	if len(results) != 1 {
+		t.Fatalf("expected 1 result, got %d", len(results))
+	}
+	if results[0].Group != "gateway" || results[0].Name != "status" || results[0].Status != "pass" {
+		t.Errorf("unexpected status result: %+v", results[0])
+	}
+}
+
+func TestCheckOnlineSDK_ProviderRegistered(t *testing.T) {
+	c, raw := testutil.NewFakeClient("default", fake.WithHealthResult(&types.HealthResult{Healthy: true}))
+	raw.AddProvider("default", &types.Provider{Name: "github", Type: "git"})
+	results := checkOnlineSDK(context.Background(), c, []string{"github"})
+	// results[0] is gateway/status pass; results[1] is the provider row.
+	if len(results) != 2 {
+		t.Fatalf("expected 2 results, got %d: %+v", len(results), results)
+	}
+	if results[1].Name != "github" || results[1].Status != "pass" || results[1].Message != "registered" {
+		t.Errorf("expected github registered pass, got %+v", results[1])
+	}
+}
+
+func TestCheckOnlineSDK_ProviderNotRegistered(t *testing.T) {
+	c := testutil.NewFake("default", fake.WithHealthResult(&types.HealthResult{Healthy: true}))
+	results := checkOnlineSDK(context.Background(), c, []string{"github"})
+	if len(results) != 2 {
+		t.Fatalf("expected 2 results, got %d: %+v", len(results), results)
+	}
+	if results[1].Name != "github" || results[1].Status != "warn" {
+		t.Errorf("expected github not-registered warn, got %+v", results[1])
+	}
+}
+
+func TestRunOnlineChecks_NoGateway(t *testing.T) {
+	// No --gateway: single non-fatal warn, factory never called.
+	called := false
+	f := func(ctx context.Context, tgt openshell.Target) (openshell.Client, error) {
+		called = true
+		return nil, nil
+	}
+	results := runOnlineChecks(context.Background(), f, "", "default", nil)
+	if len(results) != 1 || results[0].Status != "warn" {
+		t.Fatalf("expected 1 warn result, got %+v", results)
+	}
+	if called {
+		t.Error("factory should not be called when --gateway is empty")
+	}
+}
+
+func TestRunOnlineChecks_HealthyViaFactory(t *testing.T) {
+	c := testutil.NewFake("default", fake.WithHealthResult(&types.HealthResult{Healthy: true, Version: "1.0.0"}))
+	f := testutil.FakeFactory(c)
+	results := runOnlineChecks(context.Background(), f, "some-gateway", "default", nil)
+	if len(results) != 1 || results[0].Status != "pass" {
+		t.Fatalf("expected 1 pass result, got %+v", results)
+	}
+}
+
+func TestResolveOnlineFlag(t *testing.T) {
+	const envKey = "OPENSHELL_TEST_RESOLVE"
+
+	// Explicit flag wins over env var and default.
+	t.Setenv(envKey, "from-env")
+	if got := resolveOnlineFlag("from-flag", envKey, "from-default"); got != "from-flag" {
+		t.Errorf("flag precedence: got %q, want from-flag", got)
+	}
+
+	// Empty flag falls back to the env var over the default.
+	if got := resolveOnlineFlag("", envKey, "from-default"); got != "from-env" {
+		t.Errorf("env precedence: got %q, want from-env", got)
+	}
+
+	// Empty flag and unset env fall back to the default.
+	t.Setenv(envKey, "")
+	if got := resolveOnlineFlag("", envKey, "from-default"); got != "from-default" {
+		t.Errorf("default fallback: got %q, want from-default", got)
+	}
+}

main.go

@@ -6,9 +6,10 @@ import (
 	"os"
 	"path/filepath"
 
+	"github.com/spf13/cobra"
 	"github.com/stackrox/harness-openshell/cmd"
+	"github.com/stackrox/harness-openshell/internal/openshell/sdkclient"
 	"github.com/stackrox/harness-openshell/internal/status"
-	"github.com/spf13/cobra"
 )
 
 var version = "dev"
@@ -54,9 +55,9 @@ func main() {
 	cmd.Version = version
 	cmd.DefaultAgentConfig = defaultAgentConfig
 	cmd.EmbeddedGatewayProfiles = map[string][]byte{
-		"local-container":        localContainerGatewayProfile,
-		"helm":          helmNodeportGatewayProfile,
-		"openshift":   helmOpenshiftRouteGatewayProfile,
+		"local-container": localContainerGatewayProfile,
+		"helm":            helmNodeportGatewayProfile,
+		"openshift":       helmOpenshiftRouteGatewayProfile,
 	}
 	root.CompletionOptions.HiddenDefaultCmd = true
 
@@ -66,7 +67,7 @@ func main() {
 		cmd.NewDescribeCmd(harnessDir, cli),
 		cmd.NewDeleteCmd(harnessDir, cli),
 		cmd.NewDeployCmd(harnessDir, cli),
-		cmd.NewDoctorCmd(harnessDir, cli),
+		cmd.NewDoctorCmd(harnessDir, cli, sdkclient.New),
 		cmd.NewInitCmd(harnessDir),
 	)