686191e fix(preview): DNS-safe naming, testable IsExpired, and CLI output helpers · gordon

M internal/adapters/in/cli/preview.go +10 -5


@@ -54,7 +54,7 @@ func newPreviewCreateCmd() *cobra.Command {
54	54		name = preview.SanitizeBranchName(branch)
55	55		}
56	56
57		-	fmt.Printf("Creating preview: %s\n", name)
	57	+	fmt.Fprintf(cmd.OutOrStdout(), "Creating preview: %s\n", name)
58	58		// TODO: Reuse push mechanics (build, tag with preview-{name}, push)
59	59		// TODO: Wire to remote API or local service
60	60		_ = ttl

@@ -77,7 +77,12 @@ func newPreviewListCmd() *cobra.Command {
77	77		Short: "List active preview environments",
78	78		RunE: func(cmd *cobra.Command, args []string) error {
79	79		// TODO: Wire to local service or admin API
80		-	fmt.Println("No active previews")
	80	+	if jsonOutput {
	81	+	// Output empty JSON array for now (will be wired later)
	82	+	fmt.Fprintln(cmd.OutOrStdout(), "[]")
	83	+	} else {
	84	+	fmt.Fprintln(cmd.OutOrStdout(), "No active previews")
	85	+	}
81	86		return nil
82	87		},
83	88		}

@@ -92,7 +97,7 @@ func newPreviewDeleteCmd() *cobra.Command {
92	97		Args: cobra.ExactArgs(1),
93	98		RunE: func(cmd *cobra.Command, args []string) error {
94	99		// TODO: Wire to local service or admin API
95		-	fmt.Printf("Deleting preview: %s\n", args[0])
	100	+	fmt.Fprintf(cmd.OutOrStdout(), "Deleting preview: %s\n", args[0])
96	101		return nil
97	102		},
98	103		}

@@ -106,7 +111,7 @@ func newPreviewExtendCmd() *cobra.Command {
106	111		Args: cobra.ExactArgs(1),
107	112		RunE: func(cmd *cobra.Command, args []string) error {
108	113		// TODO: Wire to local service or admin API
109		-	fmt.Printf("Extending preview %s by %s\n", args[0], ttl)
	114	+	fmt.Fprintf(cmd.OutOrStdout(), "Extending preview %s by %s\n", args[0], ttl)
110	115		return nil
111	116		},
112	117		}

@@ -117,7 +122,7 @@ func newPreviewExtendCmd() *cobra.Command {
117	122		func detectGitBranch() (string, error) {
118	123		out, err := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD").Output() // #nosec G204
119	124		if err != nil {
120		-	return "", err
	125	+	return "", fmt.Errorf("detect git branch: %w", err)
121	126		}
122	127		return strings.TrimSpace(string(out)), nil
123	128		}

 				name = preview.SanitizeBranchName(branch)
 			}
-			fmt.Printf("Creating preview: %s\n", name)
+			fmt.Fprintf(cmd.OutOrStdout(), "Creating preview: %s\n", name)
 			// TODO: Reuse push mechanics (build, tag with preview-{name}, push)
 			// TODO: Wire to remote API or local service
 			_ = ttl
 		Short: "List active preview environments",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			// TODO: Wire to local service or admin API
-			fmt.Println("No active previews")
+			if jsonOutput {
+				// Output empty JSON array for now (will be wired later)
+				fmt.Fprintln(cmd.OutOrStdout(), "[]")
+			} else {
+				fmt.Fprintln(cmd.OutOrStdout(), "No active previews")
+			}
 			return nil
 		},
 	}
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			// TODO: Wire to local service or admin API
-			fmt.Printf("Deleting preview: %s\n", args[0])
+			fmt.Fprintf(cmd.OutOrStdout(), "Deleting preview: %s\n", args[0])
 			return nil
 		},
 	}
 		Args:  cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
 			// TODO: Wire to local service or admin API
-			fmt.Printf("Extending preview %s by %s\n", args[0], ttl)
+			fmt.Fprintf(cmd.OutOrStdout(), "Extending preview %s by %s\n", args[0], ttl)
 			return nil
 		},
 	}
 func detectGitBranch() (string, error) {
 	out, err := exec.Command("git", "rev-parse", "--abbrev-ref", "HEAD").Output() // #nosec G204
 	if err != nil {
-		return "", err
+		return "", fmt.Errorf("detect git branch: %w", err)
 	}
 	return strings.TrimSpace(string(out)), nil
 }

M internal/domain/preview.go +2 -2


@@ -16,6 +16,6 @@ type PreviewRoute struct {
16	16		}
17	17
18	18		// IsExpired returns true if the preview has exceeded its TTL.
19		-	func (p PreviewRoute) IsExpired() bool {
20		-	return time.Now().After(p.ExpiresAt)
	19	+	func (p PreviewRoute) IsExpired(now time.Time) bool {
	20	+	return now.After(p.ExpiresAt)
21	21		}


@@ -16,6 +16,6 @@ type PreviewRoute struct {
16		}	16		}
17			17
18		// IsExpired returns true if the preview has exceeded its TTL.	18		// IsExpired returns true if the preview has exceeded its TTL.
19	-	func (p PreviewRoute) IsExpired() bool {	19	+	func (p PreviewRoute) IsExpired(now time.Time) bool {
20	-	return time.Now().After(p.ExpiresAt)	20	+	return now.After(p.ExpiresAt)
21		}	21		}

M internal/domain/preview_test.go +1 -1


@@ -20,7 +20,7 @@ func TestPreviewRoute_IsExpired(t *testing.T) {
20	20		for _, tt := range tests {
21	21		t.Run(tt.name, func(t *testing.T) {
22	22		p := PreviewRoute{ExpiresAt: tt.expiresAt}
23		-	assert.Equal(t, tt.want, p.IsExpired())
	23	+	assert.Equal(t, tt.want, p.IsExpired(time.Now()))
24	24		})
25	25		}
26	26		}


@@ -20,7 +20,7 @@ func TestPreviewRoute_IsExpired(t *testing.T) {
20		for _, tt := range tests {	20		for _, tt := range tests {
21		t.Run(tt.name, func(t *testing.T) {	21		t.Run(tt.name, func(t *testing.T) {
22		p := PreviewRoute{ExpiresAt: tt.expiresAt}	22		p := PreviewRoute{ExpiresAt: tt.expiresAt}
23	-	assert.Equal(t, tt.want, p.IsExpired())	23	+	assert.Equal(t, tt.want, p.IsExpired(time.Now()))
24		})	24		})
25		}	25		}
26		}	26		}

M internal/usecase/auto/preview/naming.go +30 -2


@@ -10,7 +10,11 @@ import (
10	10		// "preview-login-redesign" with pattern "preview-*" → "login-redesign"
11	11		func ExtractPreviewName(tag string, patterns []string) string {
12	12		for _, pattern := range patterns {
13		-	if matched, _ := filepath.Match(pattern, tag); matched {
	13	+	matched, err := filepath.Match(pattern, tag)
	14	+	if err != nil {
	15	+	continue // skip malformed patterns
	16	+	}
	17	+	if matched {
14	18		prefix := strings.TrimSuffix(pattern, "*")
15	19		return strings.TrimPrefix(tag, prefix)
16	20		}

@@ -30,7 +34,7 @@ func GeneratePreviewDomain(baseRoute, previewName, separator string) (string, er
30	34		return app + separator + previewName + rest, nil
31	35		}
32	36
33		-	// SanitizeBranchName converts a git branch name to a valid preview name.
	37	+	// SanitizeBranchName converts a git branch name to a valid DNS-safe preview name.
34	38		// "feat/login-redesign" → "login-redesign"
35	39		func SanitizeBranchName(branch string) string {
36	40		branch = strings.ToLower(branch)

@@ -40,6 +44,30 @@ func SanitizeBranchName(branch string) string {
40	44		break
41	45		}
42	46		}
	47	+	// Replace disallowed characters with dashes
43	48		branch = strings.ReplaceAll(branch, "/", "-")
	49	+	branch = strings.ReplaceAll(branch, "_", "-")
	50	+	branch = strings.ReplaceAll(branch, ".", "-")
	51	+
	52	+	// Remove any remaining non DNS-safe characters
	53	+	var result strings.Builder
	54	+	for _, c := range branch {
	55	+	if (c >= 'a' && c <= 'z') \|\| (c >= '0' && c <= '9') \|\| c == '-' {
	56	+	result.WriteRune(c)
	57	+	}
	58	+	}
	59	+	branch = result.String()
	60	+
	61	+	// Collapse consecutive dashes and trim
	62	+	for strings.Contains(branch, "--") {
	63	+	branch = strings.ReplaceAll(branch, "--", "-")
	64	+	}
	65	+	branch = strings.Trim(branch, "-")
	66	+
	67	+	// Truncate to 63 chars (DNS label limit)
	68	+	if len(branch) > 63 {
	69	+	branch = branch[:63]
	70	+	branch = strings.TrimRight(branch, "-")
	71	+	}
44	72		return branch
45	73		}


@@ -10,7 +10,11 @@ import (
10		// "preview-login-redesign" with pattern "preview-*" → "login-redesign"	10		// "preview-login-redesign" with pattern "preview-*" → "login-redesign"
11		func ExtractPreviewName(tag string, patterns []string) string {	11		func ExtractPreviewName(tag string, patterns []string) string {
12		for _, pattern := range patterns {	12		for _, pattern := range patterns {
13	-	if matched, _ := filepath.Match(pattern, tag); matched {	13	+	matched, err := filepath.Match(pattern, tag)
			14	+	if err != nil {
			15	+	continue // skip malformed patterns
			16	+	}
			17	+	if matched {
14		prefix := strings.TrimSuffix(pattern, "*")	18		prefix := strings.TrimSuffix(pattern, "*")
15		return strings.TrimPrefix(tag, prefix)	19		return strings.TrimPrefix(tag, prefix)
16		}	20		}

@@ -30,7 +34,7 @@ func GeneratePreviewDomain(baseRoute, previewName, separator string) (string, er
30		return app + separator + previewName + rest, nil	34		return app + separator + previewName + rest, nil
31		}	35		}
32			36
33	-	// SanitizeBranchName converts a git branch name to a valid preview name.	37	+	// SanitizeBranchName converts a git branch name to a valid DNS-safe preview name.
34		// "feat/login-redesign" → "login-redesign"	38		// "feat/login-redesign" → "login-redesign"
35		func SanitizeBranchName(branch string) string {	39		func SanitizeBranchName(branch string) string {
36		branch = strings.ToLower(branch)	40		branch = strings.ToLower(branch)

@@ -40,6 +44,30 @@ func SanitizeBranchName(branch string) string {
40		break	44		break
41		}	45		}
42		}	46		}
			47	+	// Replace disallowed characters with dashes
43		branch = strings.ReplaceAll(branch, "/", "-")	48		branch = strings.ReplaceAll(branch, "/", "-")
			49	+	branch = strings.ReplaceAll(branch, "_", "-")
			50	+	branch = strings.ReplaceAll(branch, ".", "-")
			51	+
			52	+	// Remove any remaining non DNS-safe characters
			53	+	var result strings.Builder
			54	+	for _, c := range branch {
			55	+	if (c >= 'a' && c <= 'z') \|\| (c >= '0' && c <= '9') \|\| c == '-' {
			56	+	result.WriteRune(c)
			57	+	}
			58	+	}
			59	+	branch = result.String()
			60	+
			61	+	// Collapse consecutive dashes and trim
			62	+	for strings.Contains(branch, "--") {
			63	+	branch = strings.ReplaceAll(branch, "--", "-")
			64	+	}
			65	+	branch = strings.Trim(branch, "-")
			66	+
			67	+	// Truncate to 63 chars (DNS label limit)
			68	+	if len(branch) > 63 {
			69	+	branch = branch[:63]
			70	+	branch = strings.TrimRight(branch, "-")
			71	+	}
44		return branch	72		return branch
45		}	73		}

M internal/usecase/auto/preview/naming_test.go +5 -0

@@ -1,6 +1,7 @@
1	1		package preview
2	2
3	3		import (
	4	+	"strings"
4	5		"testing"
5	6
6	7		"github.com/stretchr/testify/assert"

@@ -64,6 +65,10 @@ func TestSanitizeBranchName(t *testing.T) {
64	65		{"simple", "main", "main"},
65	66		{"nested", "feat/ui/button", "ui-button"},
66	67		{"uppercase", "Feat/MyBranch", "mybranch"},
	68	+	{"dots and underscores", "feat/my_branch.name", "my-branch-name"},
	69	+	{"special chars", "feat/hello@world!", "helloworld"},
	70	+	{"long branch", strings.Repeat("a", 70), strings.Repeat("a", 63)},
	71	+	{"consecutive dashes", "feat/a--b--c", "a-b-c"},
67	72		}
68	73		for _, tt := range tests {
69	74		t.Run(tt.name, func(t *testing.T) {

@@ -1,6 +1,7 @@
1	package preview	1		package preview
2		2
3	import (	3		import (
		4	+	"strings"
4	"testing"	5		"testing"
5		6
6	"github.com/stretchr/testify/assert"	7		"github.com/stretchr/testify/assert"

@@ -64,6 +65,10 @@ func TestSanitizeBranchName(t *testing.T) {
64	{"simple", "main", "main"},	65		{"simple", "main", "main"},
65	{"nested", "feat/ui/button", "ui-button"},	66		{"nested", "feat/ui/button", "ui-button"},
66	{"uppercase", "Feat/MyBranch", "mybranch"},	67		{"uppercase", "Feat/MyBranch", "mybranch"},
		68	+	{"dots and underscores", "feat/my_branch.name", "my-branch-name"},
		69	+	{"special chars", "feat/hello@world!", "helloworld"},
		70	+	{"long branch", strings.Repeat("a", 70), strings.Repeat("a", 63)},
		71	+	{"consecutive dashes", "feat/a--b--c", "a-b-c"},
67	}	72		}
68	for _, tt := range tests {	73		for _, tt := range tests {
69	t.Run(tt.name, func(t *testing.T) {	74		t.Run(tt.name, func(t *testing.T) {

M internal/usecase/auto/preview/service.go +23 -6


@@ -6,6 +6,8 @@ import (
6	6		"sync"
7	7		"time"
8	8
	9	+	"github.com/bnema/zerowrap"
	10	+
9	11		"github.com/bnema/gordon/internal/boundaries/out"
10	12		"github.com/bnema/gordon/internal/domain"
11	13		)

@@ -103,7 +105,7 @@ func (s *Service) GetExpired() []domain.PreviewRoute {
103	105		defer s.mu.RUnlock()
104	106		var expired []domain.PreviewRoute
105	107		for _, p := range s.previews {
106		-	if p.IsExpired() {
	108	+	if p.IsExpired(time.Now()) {
107	109		expired = append(expired, p)
108	110		}
109	111		}

@@ -117,7 +119,7 @@ func (s *Service) CleanupExpired(ctx context.Context) []domain.PreviewRoute {
117	119		var expired []domain.PreviewRoute
118	120		var active []domain.PreviewRoute
119	121		for _, p := range s.previews {
120		-	if p.IsExpired() {
	122	+	if p.IsExpired(time.Now()) {
121	123		expired = append(expired, p)
122	124		} else {
123	125		active = append(active, p)

@@ -129,7 +131,11 @@ func (s *Service) CleanupExpired(ctx context.Context) []domain.PreviewRoute {
129	131		s.mu.Unlock()
130	132
131	133		if len(expired) > 0 {
132		-	_ = s.store.Save(ctx, cp)
	134	+	if err := s.store.Save(ctx, cp); err != nil {
	135	+	// Log but don't fail — expired previews are already removed from memory
	136	+	log := zerowrap.FromCtx(ctx)
	137	+	log.Warn().Err(err).Msg("failed to persist preview state after cleanup")
	138	+	}
133	139		}
134	140		return expired
135	141		}

@@ -165,6 +171,13 @@ func (s Service) AcquireNameLock(name string) sync.Mutex {
165	171		return mu
166	172		}
167	173
	174	+	// ReleaseNameLock removes the per-name mutex from the map after use.
	175	+	func (s *Service) ReleaseNameLock(name string) {
	176	+	s.nameLockMu.Lock()
	177	+	defer s.nameLockMu.Unlock()
	178	+	delete(s.nameLocks, name)
	179	+	}
	180	+
168	181		// CreatePreviewRequest contains parameters for creating a preview.
169	182		type CreatePreviewRequest struct {
170	183		Name string

@@ -176,10 +189,13 @@ type CreatePreviewRequest struct {
176	189		}
177	190
178	191		// CreatePreview orchestrates the full preview creation: lock, clone volumes, start containers, register.
179		-	func (s *Service) CreatePreview(ctx context.Context, req CreatePreviewRequest) {
	192	+	func (s *Service) CreatePreview(ctx context.Context, req CreatePreviewRequest) error {
180	193		lock := s.AcquireNameLock(req.Name)
181	194		lock.Lock()
182		-	defer lock.Unlock()
	195	+	defer func() {
	196	+	lock.Unlock()
	197	+	s.ReleaseNameLock(req.Name)
	198	+	}()
183	199
184	200		now := time.Now()
185	201		preview := domain.PreviewRoute{

@@ -194,6 +210,7 @@ func (s *Service) CreatePreview(ctx context.Context, req CreatePreviewRequest) {
194	210
195	211		// TODO: Volume cloning and container startup will be wired in Task 13
196	212		if err := s.Add(ctx, preview); err != nil {
197		-	return
	213	+	return fmt.Errorf("register preview %q: %w", req.Name, err)
198	214		}
	215	+	return nil
199	216		}

 	"sync"
 	"time"
+	"github.com/bnema/zerowrap"
 	"github.com/bnema/gordon/internal/boundaries/out"
 	"github.com/bnema/gordon/internal/domain"
 )
 	defer s.mu.RUnlock()
 	var expired []domain.PreviewRoute
 	for _, p := range s.previews {
-		if p.IsExpired() {
+		if p.IsExpired(time.Now()) {
 			expired = append(expired, p)
 		}
 	}
 	var expired []domain.PreviewRoute
 	var active []domain.PreviewRoute
 	for _, p := range s.previews {
-		if p.IsExpired() {
+		if p.IsExpired(time.Now()) {
 			expired = append(expired, p)
 		} else {
 			active = append(active, p)
 	s.mu.Unlock()
 	if len(expired) > 0 {
-		_ = s.store.Save(ctx, cp)
+		if err := s.store.Save(ctx, cp); err != nil {
+			// Log but don't fail — expired previews are already removed from memory
+			log := zerowrap.FromCtx(ctx)
+			log.Warn().Err(err).Msg("failed to persist preview state after cleanup")
+		}
 	}
 	return expired
 }
 	return mu
 }
+// ReleaseNameLock removes the per-name mutex from the map after use.
+func (s *Service) ReleaseNameLock(name string) {
+	s.nameLockMu.Lock()
+	defer s.nameLockMu.Unlock()
+	delete(s.nameLocks, name)
+}
 // CreatePreviewRequest contains parameters for creating a preview.
 type CreatePreviewRequest struct {
 	Name          string
 }
 // CreatePreview orchestrates the full preview creation: lock, clone volumes, start containers, register.
-func (s *Service) CreatePreview(ctx context.Context, req CreatePreviewRequest) {
+func (s *Service) CreatePreview(ctx context.Context, req CreatePreviewRequest) error {
 	lock := s.AcquireNameLock(req.Name)
 	lock.Lock()
-	defer lock.Unlock()
+	defer func() {
+		lock.Unlock()
+		s.ReleaseNameLock(req.Name)
+	}()
 	now := time.Now()
 	preview := domain.PreviewRoute{
 	// TODO: Volume cloning and container startup will be wired in Task 13
 	if err := s.Add(ctx, preview); err != nil {
-		return
+		return fmt.Errorf("register preview %q: %w", req.Name, err)
 	}
+	return nil
 }