From 2e309cbfd24fd5dfae598a780aeced51e56bc631 Mon Sep 17 00:00:00 2001
From: Clovis Muneza <mugaro234@gmail.com>
Date: Tue, 14 Apr 2026 19:50:43 -0400
Subject: [PATCH 1/5] Add Mailpit binary descriptor and installMailpit

Mirrors the Mago pattern: .tar.gz download + ExtractTarGz + chmod.
Wired into InstallBinary via a new mailpit case. Not added to
Tools() because Mailpit is a backing service, not a CLI tool.
---
 internal/binaries/install.go      | 17 ++++++++
 internal/binaries/mailpit.go      | 43 ++++++++++++++++++++
 internal/binaries/mailpit_test.go | 65 +++++++++++++++++++++++++++++++
 internal/binaries/manager.go      |  4 ++
 4 files changed, 129 insertions(+)
 create mode 100644 internal/binaries/mailpit.go
 create mode 100644 internal/binaries/mailpit_test.go

diff --git a/internal/binaries/install.go b/internal/binaries/install.go
index c986805..5ba6d7e 100644
--- a/internal/binaries/install.go
+++ b/internal/binaries/install.go
@@ -33,6 +33,8 @@ func InstallBinaryProgress(client *http.Client, b Binary, version string, progre
 		return installMago(client, url, progress)
 	case "rustfs":
 		return installRustfs(client, url, progress)
+	case "mailpit":
+		return installMailpit(client, url, progress)
 	default:
 		return fmt.Errorf("unknown binary: %s", b.Name)
 	}
@@ -70,6 +72,21 @@ func installRustfs(client *http.Client, url string, progress ProgressFunc) error
 	return MakeExecutable(destPath)
 }
 
+func installMailpit(client *http.Client, url string, progress ProgressFunc) error {
+	internalBin := config.InternalBinDir()
+	archivePath := filepath.Join(internalBin, "mailpit.tar.gz")
+	destPath := filepath.Join(internalBin, "mailpit")
+
+	if err := DownloadProgress(client, url, archivePath, progress); err != nil {
+		return err
+	}
+	if err := ExtractTarGz(archivePath, destPath, "mailpit"); err != nil {
+		return err
+	}
+	os.Remove(archivePath)
+	return MakeExecutable(destPath)
+}
+
 func installComposer(client *http.Client, url string, b Binary, version string, progress ProgressFunc) error {
 	destPath := config.ComposerPharPath()
 
diff --git a/internal/binaries/mailpit.go b/internal/binaries/mailpit.go
new file mode 100644
index 0000000..e26d77a
--- /dev/null
+++ b/internal/binaries/mailpit.go
@@ -0,0 +1,43 @@
+package binaries
+
+import (
+	"fmt"
+	"runtime"
+)
+
+var Mailpit = Binary{
+	Name:         "mailpit",
+	DisplayName:  "Mailpit",
+	NeedsExtract: true,
+}
+
+var mailpitPlatformNames = map[string]map[string]string{
+	"darwin": {
+		"arm64": "darwin-arm64",
+		"amd64": "darwin-amd64",
+	},
+	"linux": {
+		"amd64": "linux-amd64",
+		"arm64": "linux-arm64",
+	},
+}
+
+func mailpitArchiveName() (string, error) {
+	archMap, ok := mailpitPlatformNames[runtime.GOOS]
+	if !ok {
+		return "", fmt.Errorf("unsupported OS for Mailpit: %s", runtime.GOOS)
+	}
+	platform, ok := archMap[runtime.GOARCH]
+	if !ok {
+		return "", fmt.Errorf("unsupported architecture for Mailpit: %s/%s", runtime.GOOS, runtime.GOARCH)
+	}
+	return fmt.Sprintf("mailpit-%s.tar.gz", platform), nil
+}
+
+func mailpitURL(version string) (string, error) {
+	archive, err := mailpitArchiveName()
+	if err != nil {
+		return "", err
+	}
+	return fmt.Sprintf("https://github.com/axllent/mailpit/releases/download/%s/%s", version, archive), nil
+}
diff --git a/internal/binaries/mailpit_test.go b/internal/binaries/mailpit_test.go
new file mode 100644
index 0000000..0253d1d
--- /dev/null
+++ b/internal/binaries/mailpit_test.go
@@ -0,0 +1,65 @@
+package binaries
+
+import (
+	"runtime"
+	"strings"
+	"testing"
+)
+
+func TestMailpitURL_CurrentPlatform(t *testing.T) {
+	url, err := mailpitURL("v1.29.6")
+	if err != nil {
+		t.Fatalf("unexpected error for %s/%s: %v", runtime.GOOS, runtime.GOARCH, err)
+	}
+	if !strings.HasPrefix(url, "https://github.com/axllent/mailpit/releases/download/v1.29.6/") {
+		t.Errorf("URL = %q; missing expected prefix", url)
+	}
+	if !strings.HasSuffix(url, ".tar.gz") {
+		t.Errorf("URL = %q; expected .tar.gz suffix", url)
+	}
+}
+
+func TestMailpitArchiveName_AllPlatforms(t *testing.T) {
+	tests := []struct {
+		goos, goarch, want string
+	}{
+		{"darwin", "arm64", "mailpit-darwin-arm64.tar.gz"},
+		{"darwin", "amd64", "mailpit-darwin-amd64.tar.gz"},
+		{"linux", "amd64", "mailpit-linux-amd64.tar.gz"},
+		{"linux", "arm64", "mailpit-linux-arm64.tar.gz"},
+	}
+	for _, tc := range tests {
+		archMap, ok := mailpitPlatformNames[tc.goos]
+		if !ok {
+			t.Errorf("no entry for GOOS=%s", tc.goos)
+			continue
+		}
+		platform, ok := archMap[tc.goarch]
+		if !ok {
+			t.Errorf("no entry for GOARCH=%s on %s", tc.goarch, tc.goos)
+			continue
+		}
+		got := "mailpit-" + platform + ".tar.gz"
+		if got != tc.want {
+			t.Errorf("%s/%s: got %q, want %q", tc.goos, tc.goarch, got, tc.want)
+		}
+	}
+}
+
+func TestDownloadURL_MailpitCase(t *testing.T) {
+	url, err := DownloadURL(Mailpit, "v1.29.6")
+	if err != nil {
+		t.Fatalf("DownloadURL returned error: %v", err)
+	}
+	if url == "" {
+		t.Error("DownloadURL returned empty string")
+	}
+}
+
+func TestLatestVersionURL_MailpitCase(t *testing.T) {
+	got := LatestVersionURL(Mailpit)
+	want := "https://api.github.com/repos/axllent/mailpit/releases/latest"
+	if got != want {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}
diff --git a/internal/binaries/manager.go b/internal/binaries/manager.go
index 89587ca..a548819 100644
--- a/internal/binaries/manager.go
+++ b/internal/binaries/manager.go
@@ -38,6 +38,8 @@ func DownloadURL(b Binary, version string) (string, error) {
 		return composerURL(), nil
 	case "rustfs":
 		return rustfsURL(version)
+	case "mailpit":
+		return mailpitURL(version)
 	default:
 		return "", fmt.Errorf("unknown binary: %s", b.Name)
 	}
@@ -60,6 +62,8 @@ func LatestVersionURL(b Binary) string {
 		return "https://api.github.com/repos/carthage-software/mago/releases/latest"
 	case "rustfs":
 		return "https://api.github.com/repos/rustfs/rustfs/releases?per_page=1"
+	case "mailpit":
+		return "https://api.github.com/repos/axllent/mailpit/releases/latest"
 	default:
 		return ""
 	}

From c9ec675c4ebf17dc7bf3c8ec3dc38047ed122979 Mon Sep 17 00:00:00 2001
From: Clovis Muneza <mugaro234@gmail.com>
Date: Tue, 14 Apr 2026 19:56:24 -0400
Subject: [PATCH 2/5] Add Mailpit service implementation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Mailpit registers itself as "mail" in the binary registry. Uses
the HTTP ReadyCheck variant pointing at /livez — the first user of
the HTTP probe since RustFS uses TCP. EnvVars keys/values are pinned
against the current Docker Mail service so linked projects do not
need .env rewrites after the migration.

Also deduplicates Available() so "mail" is not double-counted while
both Docker registry and binaryRegistry carry the key during migration
(Task 4 will remove the Docker side).
---
 internal/services/mailpit.go      | 55 ++++++++++++++++++
 internal/services/mailpit_test.go | 92 +++++++++++++++++++++++++++++++
 internal/services/service.go      |  9 ++-
 3 files changed, 154 insertions(+), 2 deletions(-)
 create mode 100644 internal/services/mailpit.go
 create mode 100644 internal/services/mailpit_test.go

diff --git a/internal/services/mailpit.go b/internal/services/mailpit.go
new file mode 100644
index 0000000..65d1c90
--- /dev/null
+++ b/internal/services/mailpit.go
@@ -0,0 +1,55 @@
+package services
+
+import (
+	"time"
+
+	"github.com/prvious/pv/internal/binaries"
+)
+
+type Mailpit struct{}
+
+func (m *Mailpit) Name() string        { return "mail" }
+func (m *Mailpit) DisplayName() string { return "Mail (Mailpit)" }
+
+func (m *Mailpit) Binary() binaries.Binary { return binaries.Mailpit }
+
+func (m *Mailpit) Args(dataDir string) []string {
+	// Flag names verified in Task 1; adjust here if reality differs.
+	return []string{
+		"--smtp", ":1025",
+		"--listen", ":8025",
+		"--database", dataDir + "/mailpit.db",
+	}
+}
+
+func (m *Mailpit) Env() []string { return nil }
+
+func (m *Mailpit) Port() int        { return 1025 }
+func (m *Mailpit) ConsolePort() int { return 8025 }
+
+func (m *Mailpit) WebRoutes() []WebRoute {
+	return []WebRoute{
+		{Subdomain: "mail", Port: 8025},
+	}
+}
+
+func (m *Mailpit) EnvVars(_ string) map[string]string {
+	return map[string]string{
+		"MAIL_MAILER":   "smtp",
+		"MAIL_HOST":     "127.0.0.1",
+		"MAIL_PORT":     "1025",
+		"MAIL_USERNAME": "",
+		"MAIL_PASSWORD": "",
+	}
+}
+
+func (m *Mailpit) ReadyCheck() ReadyCheck {
+	return ReadyCheck{
+		HTTPEndpoint: "http://127.0.0.1:8025/livez",
+		Timeout:      30 * time.Second,
+	}
+}
+
+func init() {
+	binaryRegistry["mail"] = &Mailpit{}
+}
diff --git a/internal/services/mailpit_test.go b/internal/services/mailpit_test.go
new file mode 100644
index 0000000..ad62146
--- /dev/null
+++ b/internal/services/mailpit_test.go
@@ -0,0 +1,92 @@
+package services
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestMailpit_RegisteredAsMail(t *testing.T) {
+	svc, ok := LookupBinary("mail")
+	if !ok {
+		t.Fatal("LookupBinary(\"mail\") returned ok=false")
+	}
+	if _, isMailpit := svc.(*Mailpit); !isMailpit {
+		t.Errorf("expected *Mailpit, got %T", svc)
+	}
+}
+
+func TestMailpit_Name(t *testing.T) {
+	m := &Mailpit{}
+	if m.Name() != "mail" {
+		t.Errorf("Name() = %q, want mail", m.Name())
+	}
+}
+
+func TestMailpit_Ports(t *testing.T) {
+	m := &Mailpit{}
+	if m.Port() != 1025 {
+		t.Errorf("Port() = %d, want 1025", m.Port())
+	}
+	if m.ConsolePort() != 8025 {
+		t.Errorf("ConsolePort() = %d, want 8025", m.ConsolePort())
+	}
+}
+
+func TestMailpit_WebRoutes(t *testing.T) {
+	m := &Mailpit{}
+	want := []WebRoute{
+		{Subdomain: "mail", Port: 8025},
+	}
+	got := m.WebRoutes()
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("WebRoutes() = %#v, want %#v", got, want)
+	}
+}
+
+func TestMailpit_EnvVars_Golden(t *testing.T) {
+	// Pinned against the exact keys/values the old Docker Mail service
+	// produced so linked projects do not need .env rewrites post-migration.
+	m := &Mailpit{}
+	got := m.EnvVars("anyproject")
+	want := map[string]string{
+		"MAIL_MAILER":   "smtp",
+		"MAIL_HOST":     "127.0.0.1",
+		"MAIL_PORT":     "1025",
+		"MAIL_USERNAME": "",
+		"MAIL_PASSWORD": "",
+	}
+	if !reflect.DeepEqual(got, want) {
+		t.Errorf("EnvVars() = %#v, want %#v", got, want)
+	}
+}
+
+func TestMailpit_Args_UsesDataDir(t *testing.T) {
+	m := &Mailpit{}
+	args := m.Args("/tmp/mailpit-data")
+	// The arg list must mention the provided dataDir somewhere (the flag
+	// name might be --database or --data depending on Task 1 verification).
+	found := false
+	for _, a := range args {
+		if a == "/tmp/mailpit-data" || a == "/tmp/mailpit-data/mailpit.db" {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Errorf("Args() did not include the data dir; got %v", args)
+	}
+}
+
+func TestMailpit_ReadyCheck_HTTPLivez(t *testing.T) {
+	m := &Mailpit{}
+	rc := m.ReadyCheck()
+	if rc.HTTPEndpoint == "" {
+		t.Error("ReadyCheck.HTTPEndpoint must be set (Mailpit uses HTTP probe, not TCP)")
+	}
+	if rc.TCPPort != 0 {
+		t.Errorf("ReadyCheck.TCPPort = %d, want 0", rc.TCPPort)
+	}
+	if rc.Timeout == 0 {
+		t.Error("ReadyCheck.Timeout must be non-zero")
+	}
+}
diff --git a/internal/services/service.go b/internal/services/service.go
index 8389fce..1d883b5 100644
--- a/internal/services/service.go
+++ b/internal/services/service.go
@@ -49,12 +49,17 @@ func Lookup(name string) (Service, error) {
 }
 
 // Available returns the union of Docker and binary service names, sorted.
+// Deduplicates names that appear in both registries (e.g. "mail" during migration).
 func Available() []string {
-	names := make([]string, 0, len(registry)+len(binaryRegistry))
+	seen := make(map[string]struct{}, len(registry)+len(binaryRegistry))
 	for n := range registry {
-		names = append(names, n)
+		seen[n] = struct{}{}
 	}
 	for n := range binaryRegistry {
+		seen[n] = struct{}{}
+	}
+	names := make([]string, 0, len(seen))
+	for n := range seen {
 		names = append(names, n)
 	}
 	sort.Strings(names)

From fd2a33a2989ebb86e03bf579d6049f890d4fe78c Mon Sep 17 00:00:00 2001
From: Clovis Muneza <mugaro234@gmail.com>
Date: Tue, 14 Apr 2026 20:02:08 -0400
Subject: [PATCH 3/5] Remove Docker Mail service; mail is now binary-only

The Mailpit BinaryService registered itself as "mail" in Task 3;
the Docker Mail struct is removed along with its tests. The Docker
registry map no longer contains mail. No other code changes needed
because the service:* command dispatcher (from the rustfs plan)
already routes "mail" to the binary path via resolveKind.
---
 internal/services/mail.go         | 69 -------------------------------
 internal/services/mail_test.go    | 52 -----------------------
 internal/services/service.go      |  1 -
 internal/services/service_test.go |  6 +--
 4 files changed, 3 insertions(+), 125 deletions(-)
 delete mode 100644 internal/services/mail.go
 delete mode 100644 internal/services/mail_test.go

diff --git a/internal/services/mail.go b/internal/services/mail.go
deleted file mode 100644
index 757f651..0000000
--- a/internal/services/mail.go
+++ /dev/null
@@ -1,69 +0,0 @@
-package services
-
-import (
-	"github.com/prvious/pv/internal/config"
-	"github.com/prvious/pv/internal/container"
-)
-
-type Mail struct{}
-
-func (m *Mail) Name() string        { return "mail" }
-func (m *Mail) DisplayName() string { return "Mail" }
-
-func (m *Mail) DefaultVersion() string { return "latest" }
-
-func (m *Mail) ImageName(version string) string {
-	return "axllent/mailpit:" + version
-}
-
-func (m *Mail) ContainerName(version string) string {
-	return "pv-mail-" + version
-}
-
-func (m *Mail) Port(_ string) int        { return 1025 }
-func (m *Mail) ConsolePort(_ string) int { return 8025 }
-
-func (m *Mail) WebRoutes() []WebRoute {
-	return []WebRoute{
-		{Subdomain: "mail", Port: 8025},
-	}
-}
-
-func (m *Mail) CreateOpts(version string) container.CreateOpts {
-	return container.CreateOpts{
-		Name:  m.ContainerName(version),
-		Image: m.ImageName(version),
-		Ports: map[int]int{
-			1025: 1025,
-			8025: 8025,
-		},
-		Volumes: map[string]string{
-			config.ServiceDataDir("mail", version): "/data",
-		},
-		Labels: map[string]string{
-			"dev.prvious.pv":         "true",
-			"dev.prvious.pv.service": "mail",
-			"dev.prvious.pv.version": version,
-		},
-		HealthCmd:      []string{"CMD-SHELL", "wget -q --spider http://localhost:8025/livez || exit 1"},
-		HealthInterval: "2s",
-		HealthTimeout:  "5s",
-		HealthRetries:  15,
-	}
-}
-
-func (m *Mail) EnvVars(_ string, _ int) map[string]string {
-	return map[string]string{
-		"MAIL_MAILER":   "smtp",
-		"MAIL_HOST":     "127.0.0.1",
-		"MAIL_PORT":     "1025",
-		"MAIL_USERNAME": "",
-		"MAIL_PASSWORD": "",
-	}
-}
-
-func (m *Mail) CreateDatabase(_ *container.Engine, _, _ string) error {
-	return nil
-}
-
-func (m *Mail) HasDatabases() bool { return false }
diff --git a/internal/services/mail_test.go b/internal/services/mail_test.go
deleted file mode 100644
index b9cab3d..0000000
--- a/internal/services/mail_test.go
+++ /dev/null
@@ -1,52 +0,0 @@
-package services
-
-import "testing"
-
-func TestMailPorts(t *testing.T) {
-	m := &Mail{}
-	if got := m.Port("latest"); got != 1025 {
-		t.Errorf("Port = %d, want 1025", got)
-	}
-	if got := m.ConsolePort("latest"); got != 8025 {
-		t.Errorf("ConsolePort = %d, want 8025", got)
-	}
-}
-
-func TestMailImageName(t *testing.T) {
-	m := &Mail{}
-	if got := m.ImageName("latest"); got != "axllent/mailpit:latest" {
-		t.Errorf("ImageName = %q, want %q", got, "axllent/mailpit:latest")
-	}
-}
-
-func TestMailEnvVars(t *testing.T) {
-	m := &Mail{}
-	env := m.EnvVars("", 0)
-	if env["MAIL_MAILER"] != "smtp" {
-		t.Errorf("MAIL_MAILER = %q", env["MAIL_MAILER"])
-	}
-	if env["MAIL_HOST"] != "127.0.0.1" {
-		t.Errorf("MAIL_HOST = %q", env["MAIL_HOST"])
-	}
-	if env["MAIL_PORT"] != "1025" {
-		t.Errorf("MAIL_PORT = %q", env["MAIL_PORT"])
-	}
-}
-
-func TestMailWebRoutes(t *testing.T) {
-	m := &Mail{}
-	routes := m.WebRoutes()
-	if len(routes) != 1 {
-		t.Fatalf("WebRoutes len = %d, want 1", len(routes))
-	}
-	if routes[0].Subdomain != "mail" || routes[0].Port != 8025 {
-		t.Errorf("route[0] = %+v, want {mail 8025}", routes[0])
-	}
-}
-
-func TestMailName(t *testing.T) {
-	m := &Mail{}
-	if m.Name() != "mail" {
-		t.Errorf("Name = %q, want %q", m.Name(), "mail")
-	}
-}
diff --git a/internal/services/service.go b/internal/services/service.go
index 1d883b5..470b6c3 100644
--- a/internal/services/service.go
+++ b/internal/services/service.go
@@ -34,7 +34,6 @@ type Service interface {
 }
 
 var registry = map[string]Service{
-	"mail":     &Mail{},
 	"mysql":    &MySQL{},
 	"postgres": &Postgres{},
 	"redis":    &Redis{},
diff --git a/internal/services/service_test.go b/internal/services/service_test.go
index 1f9d9f8..04c2d11 100644
--- a/internal/services/service_test.go
+++ b/internal/services/service_test.go
@@ -5,8 +5,8 @@ import (
 )
 
 func TestLookup_Valid(t *testing.T) {
-	// s3 is now a BinaryService (RustFS), not in the Docker registry.
-	for _, name := range []string{"mail", "mysql", "postgres", "redis"} {
+	// s3 and mail are now BinaryServices, not in the Docker registry.
+	for _, name := range []string{"mysql", "postgres", "redis"} {
 		svc, err := Lookup(name)
 		if err != nil {
 			t.Errorf("Lookup(%q) error = %v", name, err)
@@ -44,7 +44,7 @@ func TestServiceKey(t *testing.T) {
 
 func TestAvailable(t *testing.T) {
 	names := Available()
-	// 4 Docker services (mail, mysql, postgres, redis) + 1 binary service (s3 via RustFS).
+	// 3 Docker services (mysql, postgres, redis) + 2 binary services (s3, mail).
 	if len(names) != 5 {
 		t.Errorf("Available() returned %d services, want 5", len(names))
 	}

From b876a3b23013bf3106e74b3ed5cadbb3be3e73cf Mon Sep 17 00:00:00 2001
From: Clovis Muneza <mugaro234@gmail.com>
Date: Tue, 14 Apr 2026 20:06:22 -0400
Subject: [PATCH 4/5] Add E2E phase for Mail (Mailpit) binary service lifecycle

Mirrors the rustfs E2E: service:add, stop, start, destroy. Uses
curl on /livez rather than nc, exercising the HTTP ReadyCheck path
for the first time in CI.
---
 .github/workflows/e2e.yml  |  9 +++--
 scripts/e2e/mail-binary.sh | 74 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 2 deletions(-)
 create mode 100755 scripts/e2e/mail-binary.sh

diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml
index a17858e..4dfb80c 100644
--- a/.github/workflows/e2e.yml
+++ b/.github/workflows/e2e.yml
@@ -117,13 +117,18 @@ jobs:
         timeout-minutes: 3
         run: scripts/e2e/s3-binary.sh
 
-      # ── Phase 21: Uninstall ───────────────────────────────────────
+      # ── Phase 21: Mail binary service lifecycle ────────────────────
+      - name: E2E — Mail binary service lifecycle
+        timeout-minutes: 3
+        run: scripts/e2e/mail-binary.sh
+
+      # ── Phase 22: Uninstall ───────────────────────────────────────
       # TODO: frankenphp untrust hangs in CI (internal sudo prompt, no terminal)
       # - name: Test pv uninstall
       #   timeout-minutes: 1
       #   run: scripts/e2e/uninstall.sh
 
-      # ── Phase 22: Failure Diagnostics & Cleanup ────────────────────
+      # ── Phase 23: Failure Diagnostics & Cleanup ────────────────────
       - name: Dump logs on failure
         if: failure()
         run: scripts/e2e/diagnostics.sh
diff --git a/scripts/e2e/mail-binary.sh b/scripts/e2e/mail-binary.sh
new file mode 100755
index 0000000..a17b58b
--- /dev/null
+++ b/scripts/e2e/mail-binary.sh
@@ -0,0 +1,74 @@
+#!/usr/bin/env bash
+set -euo pipefail
+source "$(dirname "$0")/helpers.sh"
+
+echo "==> Phase: Mail binary service (Mailpit) lifecycle"
+
+# Start pv in the background so we have a live daemon for service: commands.
+pv start >/tmp/pv-mail-e2e.log 2>&1 &
+START_PID=$!
+sleep 3
+
+cleanup() {
+  kill "$START_PID" 2>/dev/null || true
+  pv stop >/dev/null 2>&1 || true
+}
+trap cleanup EXIT
+
+echo "==> service:add mail"
+pv service:add mail || { echo "FAIL: pv service:add mail failed"; exit 1; }
+
+echo "==> Verify mailpit binary exists"
+test -x "$HOME/.pv/internal/bin/mailpit" || { echo "FAIL: mailpit binary not installed"; exit 1; }
+echo "OK: mailpit binary at ~/.pv/internal/bin/mailpit"
+
+echo "==> Verify daemon-status.json lists mailpit"
+test -f "$HOME/.pv/daemon-status.json" || { echo "FAIL: daemon-status.json missing"; exit 1; }
+grep -q '"mailpit"' "$HOME/.pv/daemon-status.json" || {
+    echo "FAIL: daemon-status.json does not contain mailpit entry";
+    cat "$HOME/.pv/daemon-status.json";
+    exit 1;
+}
+echo "OK: daemon-status.json advertises mailpit"
+
+echo "==> Verify HTTP /livez on port 8025 responds"
+for i in $(seq 1 20); do
+    if curl -fsS http://127.0.0.1:8025/livez 2>/dev/null; then break; fi
+    sleep 1
+done
+curl -fsS http://127.0.0.1:8025/livez || { echo "FAIL: /livez not reachable after service:add"; exit 1; }
+echo "OK: /livez reachable on port 8025"
+
+echo "==> Verify SMTP port 1025 is reachable"
+nc -z 127.0.0.1 1025 || { echo "FAIL: SMTP port 1025 not reachable after service:add"; exit 1; }
+echo "OK: SMTP port 1025 reachable"
+
+echo "==> service:stop mail"
+pv service:stop mail
+sleep 2
+if curl -fsS http://127.0.0.1:8025/livez 2>/dev/null; then
+    echo "FAIL: /livez still answering after service:stop"
+    exit 1
+fi
+echo "OK: /livez silent after service:stop"
+
+echo "==> service:start mail"
+pv service:start mail
+for i in $(seq 1 20); do
+    if curl -fsS http://127.0.0.1:8025/livez 2>/dev/null; then break; fi
+    sleep 1
+done
+curl -fsS http://127.0.0.1:8025/livez || { echo "FAIL: /livez not reachable after service:start"; exit 1; }
+echo "OK: /livez reachable after service:start"
+
+echo "==> service:destroy mail"
+pv service:destroy mail
+test ! -f "$HOME/.pv/internal/bin/mailpit" || { echo "FAIL: mailpit binary not deleted after destroy"; exit 1; }
+test ! -d "$HOME/.pv/services/mail/latest/data" || { echo "FAIL: data dir not deleted after destroy"; exit 1; }
+echo "OK: binary and data removed"
+
+echo "==> pv stop"
+pv stop || true
+trap - EXIT
+
+echo "OK: Mail binary service lifecycle passed"

From ba5da4f251628101034f7e82d37d246a044efadd Mon Sep 17 00:00:00 2001
From: Clovis Muneza <mugaro234@gmail.com>
Date: Wed, 15 Apr 2026 00:52:35 -0400
Subject: [PATCH 5/5] Address PR review: surface ignored errors, strengthen
 test, polish comments
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- destroy.go + remove.go: stop swallowing errors on os.Remove, LoadVersions,
  vs.Save, and SignalDaemon (destroy.go only — remove.go already logged it);
  surface them via ui.Subtle so stale versions-manifest state or left-behind
  binaries are visible to the user instead of silent.
- TestMailpit_Args_PinsBindAddresses: new test pinning --smtp :1025 and
  --listen :8025 to catch port/Args drift against Port / ConsolePort / the
  EnvVars golden map.
- mailpit.go: doc comment explaining the deliberate Mailpit→"mail" naming
  mismatch (preserves the old Docker service key); removed "Task 1" reference
  and added a note on /livez; documented init() self-registration.
- service.go: rephrased Available() dedup comment without transient
  "during migration" language.
---
 internal/commands/service/destroy.go | 17 ++++++++++++----
 internal/commands/service/remove.go  | 12 +++++++++---
 internal/services/mailpit.go         | 12 +++++++++++-
 internal/services/mailpit_test.go    | 29 ++++++++++++++++++++++++++--
 internal/services/service.go         |  3 ++-
 5 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/internal/commands/service/destroy.go b/internal/commands/service/destroy.go
index 8ee93ff..c62e920 100644
--- a/internal/commands/service/destroy.go
+++ b/internal/commands/service/destroy.go
@@ -47,10 +47,17 @@ var destroyCmd = &cobra.Command{
 			}
 
 			binPath := filepath.Join(config.InternalBinDir(), binSvc.Binary().Name)
-			_ = os.Remove(binPath)
-			if vs, vsErr := binaries.LoadVersions(); vsErr == nil {
+			if err := os.Remove(binPath); err != nil && !os.IsNotExist(err) {
+				ui.Subtle(fmt.Sprintf("Could not remove %s: %v (file left behind)", binPath, err))
+			}
+			// Clear the tracked version so a future service:add redownloads.
+			if vs, vsErr := binaries.LoadVersions(); vsErr != nil {
+				ui.Subtle(fmt.Sprintf("Could not load versions file: %v (manifest may be stale)", vsErr))
+			} else {
 				vs.Set(binSvc.Binary().Name, "")
-				_ = vs.Save()
+				if err := vs.Save(); err != nil {
+					ui.Subtle(fmt.Sprintf("Could not save versions file: %v", err))
+				}
 			}
 
 			dataDir := config.ServiceDataDir(name, "latest")
@@ -62,7 +69,9 @@ var destroyCmd = &cobra.Command{
 				ui.Subtle(fmt.Sprintf("Could not regenerate service site config: %v", err))
 			}
 			if server.IsRunning() {
-				_ = server.SignalDaemon()
+				if err := server.SignalDaemon(); err != nil {
+					ui.Subtle(fmt.Sprintf("Could not signal daemon: %v", err))
+				}
 			}
 			ui.Success(fmt.Sprintf("%s destroyed (binary + data gone)", binSvc.DisplayName()))
 			return nil
diff --git a/internal/commands/service/remove.go b/internal/commands/service/remove.go
index 4db4934..e1cb184 100644
--- a/internal/commands/service/remove.go
+++ b/internal/commands/service/remove.go
@@ -46,11 +46,17 @@ var removeCmd = &cobra.Command{
 			}
 			// Delete the binary.
 			binPath := filepath.Join(config.InternalBinDir(), binSvc.Binary().Name)
-			_ = os.Remove(binPath)
+			if err := os.Remove(binPath); err != nil && !os.IsNotExist(err) {
+				ui.Subtle(fmt.Sprintf("Could not remove %s: %v (file left behind)", binPath, err))
+			}
 			// Clear the tracked version so a future `service:add` redownloads.
-			if vs, vsErr := binaries.LoadVersions(); vsErr == nil {
+			if vs, vsErr := binaries.LoadVersions(); vsErr != nil {
+				ui.Subtle(fmt.Sprintf("Could not load versions file: %v (manifest may be stale)", vsErr))
+			} else {
 				vs.Set(binSvc.Binary().Name, "")
-				_ = vs.Save()
+				if err := vs.Save(); err != nil {
+					ui.Subtle(fmt.Sprintf("Could not save versions file: %v", err))
+				}
 			}
 
 			// Regenerate Caddy configs (remove s3.pv.test / s3-api.pv.test routes).
diff --git a/internal/services/mailpit.go b/internal/services/mailpit.go
index 65d1c90..0cbd057 100644
--- a/internal/services/mailpit.go
+++ b/internal/services/mailpit.go
@@ -6,6 +6,11 @@ import (
 	"github.com/prvious/pv/internal/binaries"
 )
 
+// Mailpit is a BinaryService wrapper around the upstream Mailpit binary.
+// Name() returns "mail" (not "mailpit") to preserve the service key used by
+// the previous Docker-based mail service — renaming breaks existing pv.yml
+// references and linked projects' .env files. See TestMailpit_EnvVars_Golden
+// for the migration contract with the old Docker service.
 type Mailpit struct{}
 
 func (m *Mailpit) Name() string        { return "mail" }
@@ -13,8 +18,10 @@ func (m *Mailpit) DisplayName() string { return "Mail (Mailpit)" }
 
 func (m *Mailpit) Binary() binaries.Binary { return binaries.Mailpit }
 
+// Args pins the SMTP and HTTP bind addresses to :1025 and :8025; the values
+// must agree with Port() and ConsolePort() or MAIL_PORT / WebRoutes drift.
+// Flag names match `mailpit --help` for v1.29.6.
 func (m *Mailpit) Args(dataDir string) []string {
-	// Flag names verified in Task 1; adjust here if reality differs.
 	return []string{
 		"--smtp", ":1025",
 		"--listen", ":8025",
@@ -43,6 +50,8 @@ func (m *Mailpit) EnvVars(_ string) map[string]string {
 	}
 }
 
+// ReadyCheck uses Mailpit's documented /livez endpoint, which returns 200 once
+// both the SMTP and HTTP servers are listening.
 func (m *Mailpit) ReadyCheck() ReadyCheck {
 	return ReadyCheck{
 		HTTPEndpoint: "http://127.0.0.1:8025/livez",
@@ -50,6 +59,7 @@ func (m *Mailpit) ReadyCheck() ReadyCheck {
 	}
 }
 
+// Self-registers on import; see binaryRegistry in binary.go for the pattern.
 func init() {
 	binaryRegistry["mail"] = &Mailpit{}
 }
diff --git a/internal/services/mailpit_test.go b/internal/services/mailpit_test.go
index ad62146..67212fa 100644
--- a/internal/services/mailpit_test.go
+++ b/internal/services/mailpit_test.go
@@ -63,8 +63,6 @@ func TestMailpit_EnvVars_Golden(t *testing.T) {
 func TestMailpit_Args_UsesDataDir(t *testing.T) {
 	m := &Mailpit{}
 	args := m.Args("/tmp/mailpit-data")
-	// The arg list must mention the provided dataDir somewhere (the flag
-	// name might be --database or --data depending on Task 1 verification).
 	found := false
 	for _, a := range args {
 		if a == "/tmp/mailpit-data" || a == "/tmp/mailpit-data/mailpit.db" {
@@ -77,6 +75,33 @@ func TestMailpit_Args_UsesDataDir(t *testing.T) {
 	}
 }
 
+// TestMailpit_Args_PinsBindAddresses locks the --smtp / --listen values to
+// :1025 and :8025 so a future edit to one side (Args or Port/ConsolePort) can
+// not silently drift the other. Port/Args agreement is load-bearing: the
+// EnvVars golden map pins MAIL_PORT=1025 against the SMTP flag, and the Caddy
+// WebRoute for mail.pv.{tld} points at 8025 from the --listen flag.
+func TestMailpit_Args_PinsBindAddresses(t *testing.T) {
+	m := &Mailpit{}
+	args := m.Args("/tmp/mailpit-data")
+	want := map[string]string{
+		"--smtp":     ":1025",
+		"--listen":   ":8025",
+		"--database": "/tmp/mailpit-data/mailpit.db",
+	}
+	for flag, value := range want {
+		found := false
+		for i := 0; i < len(args)-1; i++ {
+			if args[i] == flag && args[i+1] == value {
+				found = true
+				break
+			}
+		}
+		if !found {
+			t.Errorf("Args() missing %q %q; got %v", flag, value, args)
+		}
+	}
+}
+
 func TestMailpit_ReadyCheck_HTTPLivez(t *testing.T) {
 	m := &Mailpit{}
 	rc := m.ReadyCheck()
diff --git a/internal/services/service.go b/internal/services/service.go
index 470b6c3..7bc2e2a 100644
--- a/internal/services/service.go
+++ b/internal/services/service.go
@@ -48,7 +48,8 @@ func Lookup(name string) (Service, error) {
 }
 
 // Available returns the union of Docker and binary service names, sorted.
-// Deduplicates names that appear in both registries (e.g. "mail" during migration).
+// A set deduplicates entries in case a name ever appears in both registries —
+// not currently the case, but not prevented by the type system either.
 func Available() []string {
 	seen := make(map[string]struct{}, len(registry)+len(binaryRegistry))
 	for n := range registry {