Add services.LookupAny + fix 6 binary-service silent failures

cmd/doctor.go

@@ -616,6 +616,16 @@ func runServiceChecks(reg *registry.Registry) sectionResult {
 	for key, svc := range svcs {
 		svcName, version := services.ParseServiceKey(key)
 
+		// Skip binary services: they have no Docker container to probe.
+		// Binary supervision health is reported via `pv service:list` and
+		// `pv service:status`, which read the daemon's status snapshot
+		// directly. Including them here would couple doctor to the daemon's
+		// internal status format and produce misleading "lookup_error"
+		// output for healthy services (mail, s3).
+		if kind, _, _, _ := services.LookupAny(svcName); kind == services.KindBinary {
+			continue
+		}
+
 		status := "unknown"
 		if engine != nil {
 			svcDef, lookupErr := services.Lookup(svcName)

cmd/doctor_test.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/prvious/pv/internal/config"
@@ -103,3 +104,68 @@ func TestDoctor_WithValidProject(t *testing.T) {
 	cmd.SetArgs([]string{"doctor"})
 	_ = cmd.Execute()
 }
+
+// TestDoctor_SkipsBinaryServices verifies that registering a binary service
+// does NOT add a "lookup_error" / "unknown service type" entry to doctor's
+// output. Binary services have their own observability and are intentionally
+// skipped by the doctor's Docker-container check.
+func TestDoctor_SkipsBinaryServices(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+
+	if err := config.EnsureDirs(); err != nil {
+		t.Fatalf("EnsureDirs() error = %v", err)
+	}
+
+	// Register a binary service in the registry on disk.
+	reg, err := registry.Load()
+	if err != nil {
+		t.Fatalf("registry.Load() error = %v", err)
+	}
+	tru := true
+	reg.Services["mail"] = &registry.ServiceInstance{
+		Kind:    "binary",
+		Port:    1025,
+		Enabled: &tru,
+	}
+	if err := reg.Save(); err != nil {
+		t.Fatalf("reg.Save() error = %v", err)
+	}
+
+	// Capture stderr output from doctor.
+	r, w, pipeErr := os.Pipe()
+	if pipeErr != nil {
+		t.Fatalf("os.Pipe() error = %v", pipeErr)
+	}
+	prevStderr := os.Stderr
+	os.Stderr = w
+
+	cmd := newDoctorCmd()
+	cmd.SetArgs([]string{"doctor"})
+	_ = cmd.Execute()
+
+	w.Close()
+	os.Stderr = prevStderr
+
+	buf := make([]byte, 64*1024)
+	n, _ := r.Read(buf)
+	output := string(buf[:n])
+
+	if strings.Contains(output, "lookup_error") {
+		t.Errorf("doctor output should not contain \"lookup_error\" for binary service; got:\n%s", output)
+	}
+	if strings.Contains(output, "unknown service type") {
+		t.Errorf("doctor output should not contain \"unknown service type\" for binary service; got:\n%s", output)
+	}
+	if strings.Contains(output, "registry may be out of date") {
+		t.Errorf("doctor output should not contain \"registry may be out of date\" for binary service; got:\n%s", output)
+	}
+	// The three asserts above only fire when Docker is reachable (the
+	// lookup_error branch is gated behind engine != nil). In a Docker-less
+	// test env, pre-fix code would still surface mail as a failing check
+	// whose Fix line reads "pv service:start mail". This positive assertion
+	// catches the regression in either environment.
+	if strings.Contains(output, "pv service:start mail") {
+		t.Errorf("doctor output should not suggest restarting binary service mail; got:\n%s", output)
+	}
+}

cmd/install.go

@@ -58,7 +58,7 @@ func parseWith(raw string) (withSpec, error) {
 			if len(parts) > 1 {
 				s.version = parts[1]
 			}
-			if _, err := services.Lookup(s.name); err != nil {
+			if k, _, _, _ := services.LookupAny(s.name); k == services.KindUnknown {
 				return spec, fmt.Errorf("unknown service %q in --with (available: %s)", s.name, strings.Join(services.Available(), ", "))
 			}
 			spec.services = append(spec.services, s)

cmd/install_test.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -160,6 +161,42 @@ func TestParseWith_ServiceNoVersion(t *testing.T) {
 	}
 }
 
+func TestParseWith_BinaryServiceS3(t *testing.T) {
+	spec, err := parseWith("service[s3]")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(spec.services) != 1 {
+		t.Fatalf("expected 1 service, got %d", len(spec.services))
+	}
+	if spec.services[0].name != "s3" {
+		t.Errorf("service[0].name = %q, want s3", spec.services[0].name)
+	}
+}
+
+func TestParseWith_BinaryServiceMail(t *testing.T) {
+	spec, err := parseWith("service[mail]")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(spec.services) != 1 {
+		t.Fatalf("expected 1 service, got %d", len(spec.services))
+	}
+	if spec.services[0].name != "mail" {
+		t.Errorf("service[0].name = %q, want mail", spec.services[0].name)
+	}
+}
+
+func TestParseWith_UnknownServiceMongodb(t *testing.T) {
+	_, err := parseWith("service[mongodb]")
+	if err == nil {
+		t.Fatal("expected error for unknown service")
+	}
+	if !strings.Contains(err.Error(), `unknown service "mongodb"`) {
+		t.Errorf("error %q missing expected text", err)
+	}
+}
+
 func TestInstallCmd_AlreadyInstalled(t *testing.T) {
 	home := t.TempDir()
 	t.Setenv("HOME", home)

cmd/setup.go

@@ -66,13 +66,7 @@ var setupCmd = &cobra.Command{
 		}
 
 		// Service options.
-		var svcOpts []selectOption
-		for _, name := range services.Available() {
-			svc, _ := services.Lookup(name)
-			if svc != nil {
-				svcOpts = append(svcOpts, selectOption{label: svc.DisplayName(), value: name})
-			}
-		}
+		svcOpts := buildServiceOptions()
 
 		// Load settings, falling back to defaults on error.
 		settings, err := config.LoadSettings()
@@ -249,11 +243,20 @@ var setupCmd = &cobra.Command{
 		if len(selectedServices) > 0 {
 			fmt.Fprintln(os.Stderr)
 			for _, name := range selectedServices {
-				svc, _ := services.Lookup(name)
-				if svc == nil {
+				kind, _, docSvc, lookupErr := services.LookupAny(name)
+				if lookupErr != nil {
+					ui.Fail(fmt.Sprintf("Service %s: %v", name, lookupErr))
 					continue
 				}
-				svcArgs := []string{name, svc.DefaultVersion()}
+				var svcArgs []string
+				switch kind {
+				case services.KindBinary:
+					// Binary services are unversioned; service:add ignores any
+					// explicit version (verified in Task 1).
+					svcArgs = []string{name}
+				case services.KindDocker:
+					svcArgs = []string{name, docSvc.DefaultVersion()}
+				}
 				if err := service.RunAdd(svcArgs); err != nil {
 					if !errors.Is(err, ui.ErrAlreadyPrinted) {
 						ui.Fail(fmt.Sprintf("Service %s failed: %v", name, err))
@@ -271,3 +274,29 @@ var setupCmd = &cobra.Command{
 func init() {
 	rootCmd.AddCommand(setupCmd)
 }
+
+// buildServiceOptions returns the wizard's service multi-select options.
+// Both Docker and binary services are listed using their DisplayName so that
+// binary-only services (mail, s3) are visible in the picker.
+func buildServiceOptions() []selectOption {
+	names := services.Available()
+	out := make([]selectOption, 0, len(names))
+	for _, name := range names {
+		kind, binSvc, docSvc, err := services.LookupAny(name)
+		if err != nil {
+			// Available() is the union of both registries; LookupAny over
+			// the same names should never miss. If it does, skip silently
+			// rather than break the wizard.
+			continue
+		}
+		var label string
+		switch kind {
+		case services.KindBinary:
+			label = binSvc.DisplayName()
+		case services.KindDocker:
+			label = docSvc.DisplayName()
+		}
+		out = append(out, selectOption{label: label, value: name})
+	}
+	return out
+}

cmd/setup_test.go

@@ -0,0 +1,44 @@
+package cmd
+
+import (
+	"testing"
+)
+
+func TestBuildServiceOptions_IncludesBothKinds(t *testing.T) {
+	opts := buildServiceOptions()
+	if len(opts) == 0 {
+		t.Fatal("buildServiceOptions() returned empty; want at least one option")
+	}
+
+	// Pin specific names from each registry. mysql is Docker; mail and s3 are binary.
+	want := []string{"mysql", "postgres", "redis", "mail", "s3"}
+	for _, name := range want {
+		found := false
+		for _, opt := range opts {
+			if opt.value == name {
+				found = true
+				if opt.label == "" {
+					t.Errorf("option %q has empty label", name)
+				}
+				break
+			}
+		}
+		if !found {
+			t.Errorf("buildServiceOptions() missing %q", name)
+		}
+	}
+}
+
+func TestBuildServiceOptions_LabelsUseDisplayName(t *testing.T) {
+	opts := buildServiceOptions()
+	for _, opt := range opts {
+		// DisplayName for mail should be "Mail (Mailpit)" — not "mail".
+		if opt.value == "mail" && opt.label != "Mail (Mailpit)" {
+			t.Errorf("mail label = %q, want %q", opt.label, "Mail (Mailpit)")
+		}
+		// DisplayName for s3 should be "S3 Storage (RustFS)".
+		if opt.value == "s3" && opt.label != "S3 Storage (RustFS)" {
+			t.Errorf("s3 label = %q, want %q", opt.label, "S3 Storage (RustFS)")
+		}
+	}
+}