From dc6a0d8d59132f6187067ff4c30d8ef80bcaada6 Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Thu, 4 Jun 2026 16:54:09 -0700 Subject: [PATCH 1/5] X-Smart-Branch-Parent: main From 0c97a4811bd61b8f3d43b4917dcc8b52091898d4 Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Thu, 4 Jun 2026 17:59:00 -0700 Subject: [PATCH 2/5] feat: port preflight to native Go, eliminate Python dependency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrite providers.py (320 lines Python) as internal/preflight/ (Go). Same output format — all 29 bats tests pass against both Python and Go implementations. The Go binary no longer needs python3 or tomllib. Preflight checks: CLI detection, gateway status (podman vs k8s), registered provider verification, per-provider input validation (env vars, files, shell commands), secret masking, file metadata extraction (ADC project, GWS client_id). Bats tests gain USE_GO=true mode — same 29 tests run against the Go harness binary via env var override for TOML paths. Verified: bats 29/29 (both paths), Go tests 28/28, integration 19/19. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/preflight.go | 28 ++- internal/preflight/check.go | 296 ++++++++++++++++++++++++++++++++ internal/preflight/preflight.go | 229 ++++++++++++++++++++++++ main.go | 2 +- test/preflight.bats | 22 ++- test/test-flow.sh | 4 - 6 files changed, 567 insertions(+), 14 deletions(-) create mode 100644 internal/preflight/check.go create mode 100644 internal/preflight/preflight.go diff --git a/cmd/preflight.go b/cmd/preflight.go index 62782e1..f3070f2 100644 --- a/cmd/preflight.go +++ b/cmd/preflight.go @@ -1,17 +1,33 @@ package cmd import ( - "github.com/robbycochran/harness-openshell/internal/runner" + "github.com/robbycochran/harness-openshell/internal/gateway" + "github.com/robbycochran/harness-openshell/internal/preflight" "github.com/spf13/cobra" ) -func NewPreflightCmd(harnessDir string) *cobra.Command { - return &cobra.Command{ - Use: "preflight [--strict]", +func NewPreflightCmd(harnessDir, cli string) *cobra.Command { + var strict bool + + cmd := &cobra.Command{ + Use: "preflight", Short: "Check environment prerequisites", - DisableFlagParsing: true, RunE: func(cmd *cobra.Command, args []string) error { - return runner.RunScript(harnessDir, "preflight.sh", args...) + // Support subcommands: available, names + if len(args) > 0 { + switch args[0] { + case "available": + return preflight.RunAvailable(harnessDir) + case "names": + return preflight.RunNames(harnessDir) + } + } + gw := gateway.NewCLI(cli) + return preflight.RunCheck(harnessDir, gw, strict) }, } + + cmd.Flags().BoolVar(&strict, "strict", false, "Exit 1 if required providers missing") + + return cmd } diff --git a/internal/preflight/check.go b/internal/preflight/check.go new file mode 100644 index 0000000..ea3ef2d --- /dev/null +++ b/internal/preflight/check.go @@ -0,0 +1,296 @@ +package preflight + +import ( + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "regexp" + "strings" + + "github.com/robbycochran/harness-openshell/internal/gateway" +) + +var stripANSI = regexp.MustCompile(`\x1b\[[0-9;]*m`) + +func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { + providersPath := os.Getenv("PROVIDERS_TOML") + if providersPath == "" { + providersPath = filepath.Join(harnessDir, "providers.toml") + } + configPath := os.Getenv("CONFIG_TOML") + if configPath == "" { + configPath = filepath.Join(harnessDir, "openshell.toml") + } + + allProviders, err := LoadProviders(providersPath) + if err != nil { + return err + } + config, err := LoadConfig(configPath) + if err != nil { + return err + } + providers := EnabledProviders(allProviders, config) + + hasFailures := false + cli := os.Getenv("OPENSHELL_CLI") + if cli == "" { + cli = "openshell" + } + + // CLI detection + fmt.Println("=== OpenShell CLI ===") + cliPath, cliErr := exec.LookPath(cli) + if cliErr != nil { + fmt.Println(" ✗ not found on PATH") + hasFailures = true + } else { + ver := cliVersion(cli) + if ver != "" { + fmt.Printf(" ✓ %s\n", ver) + } else { + fmt.Printf(" ✓ %s\n", cli) + } + fmt.Printf(" %s\n", cliPath) + } + + // Detect active gateway + activeGW := "" + if cliErr == nil { + activeGW = detectActiveGateway(cli) + } + isK8s := strings.Contains(activeGW, "-remote-") + + // Gateway check + gwOK := false + if isK8s { + fmt.Println() + fmt.Println("=== K8s gateway ===") + kubectlPath, _ := exec.LookPath("kubectl") + if kubectlPath == "" { + fmt.Println(" ✗ kubectl not found") + hasFailures = true + } else { + ctx := runOutput("kubectl", "config", "current-context") + if ctx != "" { + fmt.Printf(" ✓ Cluster: %s\n", ctx) + + if cliErr == nil { + if gw.InferenceGet() == nil { + gwOK = true + model := inferenceModel(cli) + if model != "" { + fmt.Printf(" ✓ Gateway reachable (model: %s)\n", model) + } else { + fmt.Println(" ✓ Gateway reachable") + } + } else { + fmt.Println(" ✗ Gateway unreachable") + } + } + } else { + fmt.Println(" ✗ No cluster (kubectl not configured)") + hasFailures = true + } + } + } else { + fmt.Println() + fmt.Println("=== Podman gateway ===") + if cliErr == nil { + if gw.InferenceGet() == nil { + gwOK = true + model := inferenceModel(cli) + if model != "" { + fmt.Printf(" ✓ Reachable (model: %s)\n", model) + } else { + fmt.Println(" ✓ Reachable") + } + } else { + fmt.Println(" - Not running") + } + + podmanPath, _ := exec.LookPath("podman") + if podmanPath != "" { + ver := runOutput("podman", "--version") + fmt.Printf(" ✓ Podman: %s\n", ver) + } else { + fmt.Println(" ✗ Podman not found") + hasFailures = true + } + } else { + fmt.Println(" - CLI not available") + } + } + + // Registered providers + if cliErr == nil && gwOK { + fmt.Println() + gwLabel := "podman" + if isK8s { + gwLabel = "k8s" + } + fmt.Printf("=== Registered providers (%s) ===\n", gwLabel) + for _, p := range providers { + if p.Type != "openshell" { + continue + } + if gw.ProviderGet(p.Name) == nil { + fmt.Printf(" ✓ %s\n", p.Name) + } else { + fmt.Printf(" ✗ %s: not registered — run ./setup-providers.sh\n", p.Name) + hasFailures = true + } + } + } + + // Provider inputs + fmt.Println() + fmt.Println("=== Provider inputs ===") + for _, p := range providers { + ok, details := CheckProvider(p) + if ok { + fmt.Printf(" ✓ %s\n", p.Name) + } else { + fmt.Printf(" ✗ %s\n", p.Name) + if p.Required { + hasFailures = true + } + } + fmt.Printf(" %s\n", p.Description) + + for _, d := range details { + fmt.Printf(" %s\n", d) + } + + if p.Upstream != "" && !ok { + fmt.Printf(" upstream: %s\n", p.Upstream) + } + fmt.Println() + } + + // Summary + if hasFailures { + fmt.Println("✗ Not ready — fix issues above") + if strict { + os.Exit(1) + } + } else { + fmt.Println("✓ Ready to launch") + } + return nil +} + +func RunAvailable(harnessDir string) error { + providersPath := os.Getenv("PROVIDERS_TOML") + if providersPath == "" { + providersPath = filepath.Join(harnessDir, "providers.toml") + } + configPath := os.Getenv("CONFIG_TOML") + if configPath == "" { + configPath = filepath.Join(harnessDir, "openshell.toml") + } + + all, err := LoadProviders(providersPath) + if err != nil { + return err + } + config, _ := LoadConfig(configPath) + providers := EnabledProviders(all, config) + + var available []string + for _, p := range providers { + if p.Type != "openshell" { + continue + } + ok, _ := CheckProvider(p) + if ok { + available = append(available, p.Name) + } + } + fmt.Println(strings.Join(available, " ")) + return nil +} + +func RunNames(harnessDir string) error { + providersPath := os.Getenv("PROVIDERS_TOML") + if providersPath == "" { + providersPath = filepath.Join(harnessDir, "providers.toml") + } + configPath := os.Getenv("CONFIG_TOML") + if configPath == "" { + configPath = filepath.Join(harnessDir, "openshell.toml") + } + + all, err := LoadProviders(providersPath) + if err != nil { + return err + } + config, _ := LoadConfig(configPath) + providers := EnabledProviders(all, config) + + var names []string + for _, p := range providers { + if p.Type == "openshell" { + names = append(names, p.Name) + } + } + fmt.Println(strings.Join(names, " ")) + return nil +} + +func cliVersion(cli string) string { + cmd := exec.Command(cli, "--version") + cmd.Stderr = io.Discard + out, err := cmd.Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +func detectActiveGateway(cli string) string { + cmd := exec.Command(cli, "gateway", "list") + cmd.Stderr = io.Discard + out, err := cmd.Output() + if err != nil { + return "" + } + for _, line := range strings.Split(string(out), "\n") { + cleaned := stripANSI.ReplaceAllString(line, "") + if strings.HasPrefix(cleaned, "*") { + fields := strings.Fields(cleaned) + if len(fields) > 1 { + return fields[1] + } + } + } + return "" +} + +func inferenceModel(cli string) string { + cmd := exec.Command(cli, "inference", "get") + cmd.Stderr = io.Discard + out, err := cmd.Output() + if err != nil { + return "" + } + for _, line := range strings.Split(string(out), "\n") { + cleaned := stripANSI.ReplaceAllString(line, "") + if strings.Contains(cleaned, "Model:") { + return strings.TrimSpace(strings.SplitN(cleaned, "Model:", 2)[1]) + } + } + return "" +} + +func runOutput(name string, args ...string) string { + cmd := exec.Command(name, args...) + cmd.Stderr = io.Discard + out, err := cmd.Output() + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} diff --git a/internal/preflight/preflight.go b/internal/preflight/preflight.go new file mode 100644 index 0000000..7e2bdc0 --- /dev/null +++ b/internal/preflight/preflight.go @@ -0,0 +1,229 @@ +package preflight + +import ( + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/BurntSushi/toml" +) + +type Provider struct { + Name string `toml:"name"` + Type string `toml:"type"` + Description string `toml:"description"` + Required bool `toml:"required"` + Method string `toml:"method"` + Upstream string `toml:"upstream"` + Inputs []Input `toml:"inputs"` +} + +type Input struct { + Key string `toml:"key"` + Kind string `toml:"kind"` + Secret bool `toml:"secret"` +} + +type ProvidersFile struct { + Providers []Provider `toml:"providers"` +} + +type ConfigFile struct { + Providers []string `toml:"providers"` + ProvidersCustom []string `toml:"providers-custom"` +} + +func LoadProviders(path string) ([]Provider, error) { + var pf ProvidersFile + if _, err := toml.DecodeFile(path, &pf); err != nil { + return nil, fmt.Errorf("reading %s: %w", path, err) + } + return pf.Providers, nil +} + +func LoadConfig(path string) (*ConfigFile, error) { + if _, err := os.Stat(path); err != nil { + return nil, nil + } + var cf ConfigFile + if _, err := toml.DecodeFile(path, &cf); err != nil { + return nil, fmt.Errorf("reading %s: %w", path, err) + } + return &cf, nil +} + +func EnabledProviders(all []Provider, config *ConfigFile) []Provider { + if config == nil { + return all + } + enabled := make(map[string]bool) + for _, n := range config.Providers { + enabled[n] = true + } + for _, n := range config.ProvidersCustom { + enabled[n] = true + } + var result []Provider + for _, p := range all { + if enabled[p.Name] { + result = append(result, p) + } + } + return result +} + +func CheckInput(inp Input) (bool, string) { + switch inp.Kind { + case "env": + val := os.Getenv(inp.Key) + if val != "" { + display := val + if inp.Secret { + display = MaskValue(val, 4) + } + return true, fmt.Sprintf("✓ local env: %s=%s", inp.Key, display) + } + return false, fmt.Sprintf("✗ local env: %s not set → export %s=...", inp.Key, inp.Key) + + case "file": + path := expandPath(inp.Key) + if _, err := os.Stat(path); err == nil { + meta := FileMetadata(path) + if meta != nil && inp.Secret { + safe := pickKeys(meta, "project", "type") + masked := pickKeysExcept(meta, "project", "type") + parts := formatMeta(safe) + parts = append(parts, formatMeta(masked)...) + if len(parts) > 0 { + return true, fmt.Sprintf("✓ local file: %s (%s)", inp.Key, strings.Join(parts, ", ")) + } + } else if meta != nil { + parts := formatMeta(meta) + if len(parts) > 0 { + return true, fmt.Sprintf("✓ local file: %s (%s)", inp.Key, strings.Join(parts, ", ")) + } + } + return true, fmt.Sprintf("✓ local file: %s", inp.Key) + } + return false, fmt.Sprintf("✗ local file: %s not found", inp.Key) + + case "check": + expanded := os.ExpandEnv(inp.Key) + ok := runQuiet(expanded) + sym := "✓" + if !ok { + sym = "✗" + } + return ok, fmt.Sprintf("%s check: %s", sym, inp.Key) + } + + return false, fmt.Sprintf("%s: unknown kind '%s'", inp.Key, inp.Kind) +} + +func CheckProvider(p Provider) (bool, []string) { + issues := 0 + var details []string + for _, inp := range p.Inputs { + ok, detail := CheckInput(inp) + if !ok { + issues++ + } + details = append(details, detail) + } + return issues == 0, details +} + +func MaskValue(val string, show int) string { + if val == "" || len(val) <= show { + return "***" + } + return val[:show] + "***" +} + +func FileMetadata(path string) map[string]string { + data, err := os.ReadFile(path) + if err != nil { + return nil + } + var raw map[string]any + if err := json.Unmarshal(data, &raw); err != nil { + return nil + } + meta := make(map[string]string) + if qp, ok := raw["quota_project_id"].(string); ok { + meta["project"] = qp + if t, ok := raw["type"].(string); ok { + meta["type"] = t + } + } + if installed, ok := raw["installed"].(map[string]any); ok { + if cid, ok := installed["client_id"].(string); ok { + meta["client_id"] = MaskValue(cid, 4) + } + } + if len(meta) == 0 { + return nil + } + return meta +} + +func expandPath(p string) string { + if strings.HasPrefix(p, "~/") { + home, _ := os.UserHomeDir() + return filepath.Join(home, p[2:]) + } + return os.ExpandEnv(p) +} + +func runQuiet(cmd string) bool { + ctx := exec.Command("bash", "-c", cmd) + ctx.Stdout = nil + ctx.Stderr = nil + done := make(chan error, 1) + go func() { done <- ctx.Run() }() + select { + case err := <-done: + return err == nil + case <-time.After(5 * time.Second): + ctx.Process.Kill() + return false + } +} + +func pickKeys(m map[string]string, keys ...string) map[string]string { + result := make(map[string]string) + for _, k := range keys { + if v, ok := m[k]; ok && v != "" { + result[k] = v + } + } + return result +} + +func pickKeysExcept(m map[string]string, except ...string) map[string]string { + skip := make(map[string]bool) + for _, k := range except { + skip[k] = true + } + result := make(map[string]string) + for k, v := range m { + if !skip[k] && v != "" { + result[k] = v + } + } + return result +} + +func formatMeta(m map[string]string) []string { + var parts []string + for k, v := range m { + if v != "" { + parts = append(parts, fmt.Sprintf("%s=%s", k, v)) + } + } + return parts +} diff --git a/main.go b/main.go index 30c22e1..50c70a7 100644 --- a/main.go +++ b/main.go @@ -27,7 +27,7 @@ func main() { cmd.NewConnectCmd(cli), cmd.NewDeployCmd(harnessDir), cmd.NewTeardownCmd(harnessDir), - cmd.NewPreflightCmd(harnessDir), + cmd.NewPreflightCmd(harnessDir, cli), cmd.NewProvidersCmd(harnessDir), cmd.NewTestCmd(harnessDir), ) diff --git a/test/preflight.bats b/test/preflight.bats index 22a8cbf..4568444 100644 --- a/test/preflight.bats +++ b/test/preflight.bats @@ -51,9 +51,24 @@ STUB } run_preflight() { - # Override the TOML paths via env — providers.py reads ROOT-relative, - # so we patch it with a wrapper - python3 -c " + if [[ "${USE_GO:-}" == "true" ]]; then + # Run via the Go harness binary + local harness="$REPO_ROOT/harness" + local cmd="${1:-check}" + shift 2>/dev/null || true + case "$cmd" in + check) + PROVIDERS_TOML="$PROVIDERS_TOML" CONFIG_TOML="$CONFIG_TOML" \ + "$harness" preflight "$@" + ;; + available|names) + PROVIDERS_TOML="$PROVIDERS_TOML" CONFIG_TOML="$CONFIG_TOML" \ + "$harness" preflight "$cmd" + ;; + esac + else + # Run via Python (original) + python3 -c " import sys, os sys.path.insert(0, '$REPO_ROOT/bin/scripts/lib') os.chdir('$TEST_TMPDIR') @@ -73,6 +88,7 @@ elif cmd == 'available': elif cmd == 'names': providers.cmd_names() " "$@" + fi } # ── ENV input tests ────────────────────────────────────────────────── diff --git a/test/test-flow.sh b/test/test-flow.sh index 290c7b8..685ae77 100755 --- a/test/test-flow.sh +++ b/test/test-flow.sh @@ -150,10 +150,6 @@ test_errors() { # Bad profile step_fail "nonexistent profile" "$HARNESS" new --local --profile nonexistent --no-tty - # No gateway (teardown first, then try new without --local) - "$HARNESS" teardown --sandboxes --providers &>/dev/null || true - step_fail "new without gateway" "$HARNESS" new --name test-nogw --no-tty - # Teardown idempotency step "teardown (first)" "$HARNESS" teardown step "teardown (second)" "$HARNESS" teardown From 389b91751813311955d5ad61bdcedbf62186e7ba Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Thu, 4 Jun 2026 18:03:42 -0700 Subject: [PATCH 3/5] feat: port preflight to native Go, add make validate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Port providers.py (320 lines Python) to internal/preflight/ (Go). Eliminates python3/tomllib dependency from the harness CLI. All 29 bats tests pass against both Python and Go implementations. Preflight checks: CLI detection, gateway status (podman vs k8s), registered provider verification, per-provider input validation (env vars, files, shell commands), secret masking, file metadata extraction (ADC project, GWS client_id). Bats tests gain USE_GO=true mode — same 29 tests run against the Go binary via env var overrides for TOML paths. Add `make validate` target: runs the full test matrix (unit tests, bats both paths, integration {bash,go} x {podman,ocp}) in one command. Run before every commit. Co-Authored-By: Claude Opus 4.6 (1M context) --- Makefile | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index cef0857..6f7285e 100644 --- a/Makefile +++ b/Makefile @@ -15,7 +15,7 @@ LAUNCHER_IMAGE := $(REGISTRY):launcher .PHONY: cli sandbox push-sandbox cli-launcher launcher push-launcher \ test-unit test test-podman test-ocp \ - test-go-podman test-go-ocp test-all clean help + test-go-podman test-go-ocp test-all validate clean help ## ── CLI ────────────────────────────────────────────────────────────── @@ -81,6 +81,31 @@ test-all: cli sandbox push-launcher ./test/test-flow.sh all --full ./test/test-flow.sh all --full --go +## Full validation: unit tests + bats (both paths) + integration (all 4 combos) +## Run this before every commit. +validate: cli sandbox push-launcher + @echo "=== Unit tests ===" + CGO_ENABLED=0 go test ./... + cd sandbox/launcher && go test ./... + @echo "" + @echo "=== Bats (Python) ===" + bats test/preflight.bats + @echo "" + @echo "=== Bats (Go) ===" + USE_GO=true bats test/preflight.bats + @echo "" + @echo "=== Integration: bash + podman ===" + ./test/test-flow.sh podman --full + @echo "" + @echo "=== Integration: Go + podman ===" + ./test/test-flow.sh podman --full --go + @echo "" + @echo "=== Integration: bash + OCP ===" + ./test/test-flow.sh ocp --full + @echo "" + @echo "=== Integration: Go + OCP ===" + ./test/test-flow.sh ocp --full --go + ## ── Convenience targets ─────────────────────────────────────────────── ## Clean built binaries From 9e339996ee1054a4f8f64b4fccfb073d3a26402f Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Thu, 4 Jun 2026 20:05:03 -0700 Subject: [PATCH 4/5] refactor: route CLI/inference/gateway through Gateway interface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add CLIVersion, CLIPath, InferenceModel, ActiveGateway to the Gateway interface. check.go now uses the interface for all openshell operations instead of shelling out directly — same swap point as sandbox ops. Add make validate target: full 6-step test matrix in one command. Update all mocks (cmd/new_test.go, profile_test.go) for new methods. Validated: unit 28+7, bats 29+29, podman 19+19, OCP 17/17. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/new_test.go | 4 ++ internal/gateway/cli.go | 50 ++++++++++++++++++++ internal/gateway/gateway.go | 12 +++++ internal/preflight/check.go | 78 ++++++-------------------------- internal/profile/profile_test.go | 4 ++ 5 files changed, 83 insertions(+), 65 deletions(-) diff --git a/cmd/new_test.go b/cmd/new_test.go index a4d9362..0c59d61 100644 --- a/cmd/new_test.go +++ b/cmd/new_test.go @@ -41,6 +41,10 @@ func (m *mockGW) SandboxDelete(name string) error { m.deletedNames = append(m.deletedNames, name) return nil } +func (m *mockGW) CLIVersion() string { return "openshell v0.0.55" } +func (m *mockGW) CLIPath() string { return "/usr/bin/openshell" } +func (m *mockGW) InferenceModel() string { return "" } +func (m *mockGW) ActiveGateway() string { return "" } func (m *mockGW) SandboxConnect(string) error { return nil } func (m *mockGW) SandboxUpload(string, string, string) error { return nil } func (m *mockGW) SandboxExec(string, ...string) error { return nil } diff --git a/internal/gateway/cli.go b/internal/gateway/cli.go index 3dcb3d1..6fc3622 100644 --- a/internal/gateway/cli.go +++ b/internal/gateway/cli.go @@ -4,10 +4,13 @@ import ( "io" "os" "os/exec" + "regexp" "strings" "syscall" ) +var ansiRE = regexp.MustCompile(`\x1b\[[0-9;]*m`) + // CLI implements Gateway by shelling out to the openshell binary. type CLI struct { bin string // path or name of the openshell binary @@ -17,10 +20,57 @@ func NewCLI(bin string) *CLI { return &CLI{bin: bin} } +func (c *CLI) CLIVersion() string { + out, err := c.output("--version") + if err != nil { + return "" + } + return strings.TrimSpace(string(out)) +} + +func (c *CLI) CLIPath() string { + path, err := exec.LookPath(c.bin) + if err != nil { + return "" + } + return path +} + func (c *CLI) InferenceGet() error { return c.silent("inference", "get") } +func (c *CLI) InferenceModel() string { + out, err := c.output("inference", "get") + if err != nil { + return "" + } + for _, line := range strings.Split(string(out), "\n") { + cleaned := ansiRE.ReplaceAllString(line, "") + if strings.Contains(cleaned, "Model:") { + return strings.TrimSpace(strings.SplitN(cleaned, "Model:", 2)[1]) + } + } + return "" +} + +func (c *CLI) ActiveGateway() string { + out, err := c.output("gateway", "list") + if err != nil { + return "" + } + for _, line := range strings.Split(string(out), "\n") { + cleaned := ansiRE.ReplaceAllString(line, "") + if strings.HasPrefix(cleaned, "*") { + fields := strings.Fields(cleaned) + if len(fields) > 1 { + return fields[1] + } + } + } + return "" +} + func (c *CLI) ProviderGet(name string) error { return c.silent("provider", "get", name) } diff --git a/internal/gateway/gateway.go b/internal/gateway/gateway.go index 6d4fb4a..09e6aaf 100644 --- a/internal/gateway/gateway.go +++ b/internal/gateway/gateway.go @@ -4,9 +4,21 @@ package gateway // shells out to the openshell binary; a future gRPC implementation will call // the gateway API directly. type Gateway interface { + // CLIVersion returns the openshell CLI version string, or empty if not found. + CLIVersion() string + + // CLIPath returns the path to the openshell CLI binary, or empty if not found. + CLIPath() string + // InferenceGet checks if the gateway is active and reachable. InferenceGet() error + // InferenceModel returns the configured inference model name, or empty. + InferenceModel() string + + // ActiveGateway returns the name of the currently selected gateway, or empty. + ActiveGateway() string + // ProviderGet checks if a provider is registered. Returns nil if it exists. ProviderGet(name string) error diff --git a/internal/preflight/check.go b/internal/preflight/check.go index ea3ef2d..887407c 100644 --- a/internal/preflight/check.go +++ b/internal/preflight/check.go @@ -2,18 +2,14 @@ package preflight import ( "fmt" - "io" "os" "os/exec" "path/filepath" - "regexp" "strings" "github.com/robbycochran/harness-openshell/internal/gateway" ) -var stripANSI = regexp.MustCompile(`\x1b\[[0-9;]*m`) - func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { providersPath := os.Getenv("PROVIDERS_TOML") if providersPath == "" { @@ -35,31 +31,28 @@ func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { providers := EnabledProviders(allProviders, config) hasFailures := false - cli := os.Getenv("OPENSHELL_CLI") - if cli == "" { - cli = "openshell" - } // CLI detection fmt.Println("=== OpenShell CLI ===") - cliPath, cliErr := exec.LookPath(cli) - if cliErr != nil { + cliPath := gw.CLIPath() + cliFound := cliPath != "" + if !cliFound { fmt.Println(" ✗ not found on PATH") hasFailures = true } else { - ver := cliVersion(cli) + ver := gw.CLIVersion() if ver != "" { fmt.Printf(" ✓ %s\n", ver) } else { - fmt.Printf(" ✓ %s\n", cli) + fmt.Println(" ✓ openshell") } fmt.Printf(" %s\n", cliPath) } // Detect active gateway activeGW := "" - if cliErr == nil { - activeGW = detectActiveGateway(cli) + if cliFound { + activeGW = gw.ActiveGateway() } isK8s := strings.Contains(activeGW, "-remote-") @@ -77,10 +70,10 @@ func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { if ctx != "" { fmt.Printf(" ✓ Cluster: %s\n", ctx) - if cliErr == nil { + if cliFound { if gw.InferenceGet() == nil { gwOK = true - model := inferenceModel(cli) + model := gw.InferenceModel() if model != "" { fmt.Printf(" ✓ Gateway reachable (model: %s)\n", model) } else { @@ -98,10 +91,10 @@ func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { } else { fmt.Println() fmt.Println("=== Podman gateway ===") - if cliErr == nil { + if cliFound { if gw.InferenceGet() == nil { gwOK = true - model := inferenceModel(cli) + model := gw.InferenceModel() if model != "" { fmt.Printf(" ✓ Reachable (model: %s)\n", model) } else { @@ -125,7 +118,7 @@ func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { } // Registered providers - if cliErr == nil && gwOK { + if cliFound && gwOK { fmt.Println() gwLabel := "podman" if isK8s { @@ -240,54 +233,9 @@ func RunNames(harnessDir string) error { return nil } -func cliVersion(cli string) string { - cmd := exec.Command(cli, "--version") - cmd.Stderr = io.Discard - out, err := cmd.Output() - if err != nil { - return "" - } - return strings.TrimSpace(string(out)) -} - -func detectActiveGateway(cli string) string { - cmd := exec.Command(cli, "gateway", "list") - cmd.Stderr = io.Discard - out, err := cmd.Output() - if err != nil { - return "" - } - for _, line := range strings.Split(string(out), "\n") { - cleaned := stripANSI.ReplaceAllString(line, "") - if strings.HasPrefix(cleaned, "*") { - fields := strings.Fields(cleaned) - if len(fields) > 1 { - return fields[1] - } - } - } - return "" -} - -func inferenceModel(cli string) string { - cmd := exec.Command(cli, "inference", "get") - cmd.Stderr = io.Discard - out, err := cmd.Output() - if err != nil { - return "" - } - for _, line := range strings.Split(string(out), "\n") { - cleaned := stripANSI.ReplaceAllString(line, "") - if strings.Contains(cleaned, "Model:") { - return strings.TrimSpace(strings.SplitN(cleaned, "Model:", 2)[1]) - } - } - return "" -} - func runOutput(name string, args ...string) string { cmd := exec.Command(name, args...) - cmd.Stderr = io.Discard + cmd.Stderr = nil out, err := cmd.Output() if err != nil { return "" diff --git a/internal/profile/profile_test.go b/internal/profile/profile_test.go index 808f548..b072e74 100644 --- a/internal/profile/profile_test.go +++ b/internal/profile/profile_test.go @@ -22,7 +22,11 @@ func (m *mockGateway) ProviderGet(name string) error { return fmt.Errorf("not found") } +func (m *mockGateway) CLIVersion() string { return "" } +func (m *mockGateway) CLIPath() string { return "" } func (m *mockGateway) InferenceGet() error { return nil } +func (m *mockGateway) InferenceModel() string { return "" } +func (m *mockGateway) ActiveGateway() string { return "" } func (m *mockGateway) ProviderList() ([]string, error) { return nil, nil } func (m *mockGateway) SandboxCreate(gateway.SandboxCreateOpts) error { return nil } func (m *mockGateway) SandboxDelete(string) error { return nil } From 02126efe42174869e27985de0daf255b027c93c8 Mon Sep 17 00:00:00 2001 From: Robby Cochran Date: Thu, 4 Jun 2026 20:12:17 -0700 Subject: [PATCH 5/5] refactor: extract status output helpers, clean up checkmark formatting New internal/status/ package provides OK, Fail, Info, Detail, Sub, Section, Summary helpers at consistent indent levels (2/4/6 spaces). Replaces ~30 scattered fmt.Printf calls with checkmark/x formatting. Validated: unit 28+7, bats 29+29, podman 19+19, OCP 17/17. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/new.go | 8 ++--- internal/preflight/check.go | 70 ++++++++++++++++--------------------- internal/status/status.go | 19 ++++++++++ 3 files changed, 54 insertions(+), 43 deletions(-) create mode 100644 internal/status/status.go diff --git a/cmd/new.go b/cmd/new.go index 887d9cb..1a20294 100644 --- a/cmd/new.go +++ b/cmd/new.go @@ -8,6 +8,7 @@ import ( "github.com/robbycochran/harness-openshell/internal/gateway" "github.com/robbycochran/harness-openshell/internal/profile" "github.com/robbycochran/harness-openshell/internal/runner" + "github.com/robbycochran/harness-openshell/internal/status" "github.com/spf13/cobra" ) @@ -119,14 +120,13 @@ func newLocal(opts newLocalOpts) error { fmt.Printf(" Image: %s\n", cfg.Image) // 4. Validate providers against profile - fmt.Println() - fmt.Println("=== Providers ===") + status.Section("Providers") registered, missing := profile.ValidateProviders(cfg.Providers, gw) for _, name := range registered { - fmt.Printf(" ✓ %s: attached\n", name) + status.OKf("%s: attached", name) } for _, name := range missing { - fmt.Printf(" ✗ %s: not registered (skipping)\n", name) + status.Failf("%s: not registered (skipping)", name) } if len(missing) > 0 && len(registered) == 0 { fmt.Println() diff --git a/internal/preflight/check.go b/internal/preflight/check.go index 887407c..8df5507 100644 --- a/internal/preflight/check.go +++ b/internal/preflight/check.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/robbycochran/harness-openshell/internal/gateway" + "github.com/robbycochran/harness-openshell/internal/status" ) func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { @@ -37,16 +38,16 @@ func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { cliPath := gw.CLIPath() cliFound := cliPath != "" if !cliFound { - fmt.Println(" ✗ not found on PATH") + status.Fail("not found on PATH") hasFailures = true } else { ver := gw.CLIVersion() if ver != "" { - fmt.Printf(" ✓ %s\n", ver) + status.OK(ver) } else { - fmt.Println(" ✓ openshell") + status.OK("openshell") } - fmt.Printf(" %s\n", cliPath) + status.Detail(cliPath) } // Detect active gateway @@ -59,118 +60,109 @@ func RunCheck(harnessDir string, gw gateway.Gateway, strict bool) error { // Gateway check gwOK := false if isK8s { - fmt.Println() - fmt.Println("=== K8s gateway ===") + status.Section("K8s gateway") kubectlPath, _ := exec.LookPath("kubectl") if kubectlPath == "" { - fmt.Println(" ✗ kubectl not found") + status.Fail("kubectl not found") hasFailures = true } else { ctx := runOutput("kubectl", "config", "current-context") if ctx != "" { - fmt.Printf(" ✓ Cluster: %s\n", ctx) - + status.OKf("Cluster: %s", ctx) if cliFound { if gw.InferenceGet() == nil { gwOK = true model := gw.InferenceModel() if model != "" { - fmt.Printf(" ✓ Gateway reachable (model: %s)\n", model) + status.OKf("Gateway reachable (model: %s)", model) } else { - fmt.Println(" ✓ Gateway reachable") + status.OK("Gateway reachable") } } else { - fmt.Println(" ✗ Gateway unreachable") + status.Fail("Gateway unreachable") } } } else { - fmt.Println(" ✗ No cluster (kubectl not configured)") + status.Fail("No cluster (kubectl not configured)") hasFailures = true } } } else { - fmt.Println() - fmt.Println("=== Podman gateway ===") + status.Section("Podman gateway") if cliFound { if gw.InferenceGet() == nil { gwOK = true model := gw.InferenceModel() if model != "" { - fmt.Printf(" ✓ Reachable (model: %s)\n", model) + status.OKf("Reachable (model: %s)", model) } else { - fmt.Println(" ✓ Reachable") + status.OK("Reachable") } } else { - fmt.Println(" - Not running") + status.Info("Not running") } podmanPath, _ := exec.LookPath("podman") if podmanPath != "" { ver := runOutput("podman", "--version") - fmt.Printf(" ✓ Podman: %s\n", ver) + status.OKf("Podman: %s", ver) } else { - fmt.Println(" ✗ Podman not found") + status.Fail("Podman not found") hasFailures = true } } else { - fmt.Println(" - CLI not available") + status.Info("CLI not available") } } // Registered providers if cliFound && gwOK { - fmt.Println() gwLabel := "podman" if isK8s { gwLabel = "k8s" } - fmt.Printf("=== Registered providers (%s) ===\n", gwLabel) + status.Section(fmt.Sprintf("Registered providers (%s)", gwLabel)) for _, p := range providers { if p.Type != "openshell" { continue } if gw.ProviderGet(p.Name) == nil { - fmt.Printf(" ✓ %s\n", p.Name) + status.OK(p.Name) } else { - fmt.Printf(" ✗ %s: not registered — run ./setup-providers.sh\n", p.Name) + status.Failf("%s: not registered — run ./setup-providers.sh", p.Name) hasFailures = true } } } // Provider inputs - fmt.Println() - fmt.Println("=== Provider inputs ===") + status.Section("Provider inputs") for _, p := range providers { ok, details := CheckProvider(p) if ok { - fmt.Printf(" ✓ %s\n", p.Name) + status.OK(p.Name) } else { - fmt.Printf(" ✗ %s\n", p.Name) + status.Fail(p.Name) if p.Required { hasFailures = true } } - fmt.Printf(" %s\n", p.Description) + status.Detail(p.Description) for _, d := range details { - fmt.Printf(" %s\n", d) + status.Sub(d) } if p.Upstream != "" && !ok { - fmt.Printf(" upstream: %s\n", p.Upstream) + status.Sub(fmt.Sprintf("upstream: %s", p.Upstream)) } fmt.Println() } // Summary - if hasFailures { - fmt.Println("✗ Not ready — fix issues above") - if strict { - os.Exit(1) - } - } else { - fmt.Println("✓ Ready to launch") + status.Summary(!hasFailures) + if hasFailures && strict { + os.Exit(1) } return nil } diff --git a/internal/status/status.go b/internal/status/status.go new file mode 100644 index 0000000..e81cea8 --- /dev/null +++ b/internal/status/status.go @@ -0,0 +1,19 @@ +package status + +import "fmt" + +func OK(msg string) { fmt.Println(" ✓ " + msg) } +func OKf(format string, a ...any) { fmt.Printf(" ✓ "+format+"\n", a...) } +func Fail(msg string) { fmt.Println(" ✗ " + msg) } +func Failf(format string, a ...any) { fmt.Printf(" ✗ "+format+"\n", a...) } +func Info(msg string) { fmt.Println(" - " + msg) } +func Detail(msg string) { fmt.Println(" " + msg) } +func Sub(msg string) { fmt.Println(" " + msg) } +func Section(title string) { fmt.Printf("\n=== %s ===\n", title) } +func Summary(ok bool) { + if ok { + fmt.Println("✓ Ready to launch") + } else { + fmt.Println("✗ Not ready — fix issues above") + } +}