a17d8d2 fix(cli): tighten review follow-ups · gordon

M docs/config/reference.md +1 -1


@@ -232,7 +232,7 @@ keep_last = 3 # Keep N newest tags per repository
232	232		\| `tls.acme.obtain_batch_size` \| `1` \| Maximum new ACME certificate orders per reconcile run \|
233	233		\| `auth.enabled` \| `true` \| Enable authentication; when `false`, run local-only mode (loopback-only `/v2/`, `/admin/` disabled) \|
234	234		\| `auth.secrets_backend` \| `"unsafe"` \| Secrets storage \|
235		-	\| `auth.token_expiry` \| `"720h"` \| 30 days \|
	235	+	\| `auth.token_expiry` \| `"30d"` \| 30 days \|
236	236		\| `auth.access_token_ttl` \| `"15m"` \| Ephemeral access token lifetime \|
237	237		\| `api.rate_limit.enabled` \| `true` \| Enable rate limiting \|
238	238		\| `api.rate_limit.global_rps` \| `500` \| Global requests/second \|


@@ -232,7 +232,7 @@ keep_last = 3 # Keep N newest tags per repository
232		\| `tls.acme.obtain_batch_size` \| `1` \| Maximum new ACME certificate orders per reconcile run \|	232		\| `tls.acme.obtain_batch_size` \| `1` \| Maximum new ACME certificate orders per reconcile run \|
233		\| `auth.enabled` \| `true` \| Enable authentication; when `false`, run local-only mode (loopback-only `/v2/`, `/admin/` disabled) \|	233		\| `auth.enabled` \| `true` \| Enable authentication; when `false`, run local-only mode (loopback-only `/v2/`, `/admin/` disabled) \|
234		\| `auth.secrets_backend` \| `"unsafe"` \| Secrets storage \|	234		\| `auth.secrets_backend` \| `"unsafe"` \| Secrets storage \|
235	-	\| `auth.token_expiry` \| `"720h"` \| 30 days \|	235	+	\| `auth.token_expiry` \| `"30d"` \| 30 days \|
236		\| `auth.access_token_ttl` \| `"15m"` \| Ephemeral access token lifetime \|	236		\| `auth.access_token_ttl` \| `"15m"` \| Ephemeral access token lifetime \|
237		\| `api.rate_limit.enabled` \| `true` \| Enable rate limiting \|	237		\| `api.rate_limit.enabled` \| `true` \| Enable rate limiting \|
238		\| `api.rate_limit.global_rps` \| `500` \| Global requests/second \|	238		\| `api.rate_limit.global_rps` \| `500` \| Global requests/second \|

M internal/adapters/in/cli/controlplane_local.go +3 -0


@@ -165,6 +165,9 @@ func (l *localControlPlane) GetRouteCleanupPreview(ctx context.Context, routeDom
165	165		if err != nil {
166	166		return nil, err
167	167		}
	168	+	if report == nil {
	169	+	return nil, nil
	170	+	}
168	171		if l.volumeSvc == nil {
169	172		return report, nil
170	173		}


@@ -165,6 +165,9 @@ func (l *localControlPlane) GetRouteCleanupPreview(ctx context.Context, routeDom
165	if err != nil {	165		if err != nil {
166	return nil, err	166		return nil, err
167	}	167		}
		168	+	if report == nil {
		169	+	return nil, nil
		170	+	}
168	if l.volumeSvc == nil {	171		if l.volumeSvc == nil {
169	return report, nil	172		return report, nil
170	}	173		}

M internal/adapters/in/cli/controlplane_local_test.go +28 -0


@@ -13,6 +13,18 @@ import (
13	13		"github.com/stretchr/testify/require"
14	14		)
15	15
	16	+	type previewContainerService struct {
	17	+	*inmocks.MockContainerService
	18	+	preview func(context.Context, string) (*domain.CleanupReport, error)
	19	+	}
	20	+
	21	+	func (s previewContainerService) PreviewRemovedRouteCleanup(ctx context.Context, routeDomain string) (domain.CleanupReport, error) {
	22	+	if s.preview == nil {
	23	+	return nil, nil
	24	+	}
	25	+	return s.preview(ctx, routeDomain)
	26	+	}
	27	+
16	28		func TestLocalControlPlane_GetStatus(t *testing.T) {
17	29		t.Parallel()
18	30

@@ -216,6 +228,22 @@ func TestLocalControlPlane_GetContainerLogs(t *testing.T) {
216	228		require.Equal(t, []string{"line1", "line2"}, lines)
217	229		}
218	230
	231	+	func TestLocalControlPlane_GetRouteCleanupPreview_ReturnsNilReportWithoutPanic(t *testing.T) {
	232	+	ctx := context.Background()
	233	+	volumeSvc := inmocks.NewMockVolumeService(t)
	234	+	containerSvc := &previewContainerService{
	235	+	MockContainerService: inmocks.NewMockContainerService(t),
	236	+	preview: func(context.Context, string) (*domain.CleanupReport, error) {
	237	+	return nil, nil
	238	+	},
	239	+	}
	240	+	cp := &localControlPlane{containerSvc: containerSvc, volumeSvc: volumeSvc}
	241	+
	242	+	report, err := cp.GetRouteCleanupPreview(ctx, "app.local")
	243	+	require.NoError(t, err)
	244	+	assert.Nil(t, report)
	245	+	}
	246	+
219	247		func TestLocalControlPlane_RemoveRouteReconcilesRuntimeWhenRouteAlreadyMissing(t *testing.T) {
220	248		ctx := context.Background()
221	249		configSvc := inmocks.NewMockConfigService(t)


@@ -13,6 +13,18 @@ import (
13	"github.com/stretchr/testify/require"	13		"github.com/stretchr/testify/require"
14	)	14		)
15		15
		16	+	type previewContainerService struct {
		17	+	*inmocks.MockContainerService
		18	+	preview func(context.Context, string) (*domain.CleanupReport, error)
		19	+	}
		20	+
		21	+	func (s previewContainerService) PreviewRemovedRouteCleanup(ctx context.Context, routeDomain string) (domain.CleanupReport, error) {
		22	+	if s.preview == nil {
		23	+	return nil, nil
		24	+	}
		25	+	return s.preview(ctx, routeDomain)
		26	+	}
		27	+
16	func TestLocalControlPlane_GetStatus(t *testing.T) {	28		func TestLocalControlPlane_GetStatus(t *testing.T) {
17	t.Parallel()	29		t.Parallel()
18		30

@@ -216,6 +228,22 @@ func TestLocalControlPlane_GetContainerLogs(t *testing.T) {
216	require.Equal(t, []string{"line1", "line2"}, lines)	228		require.Equal(t, []string{"line1", "line2"}, lines)
217	}	229		}
218		230
		231	+	func TestLocalControlPlane_GetRouteCleanupPreview_ReturnsNilReportWithoutPanic(t *testing.T) {
		232	+	ctx := context.Background()
		233	+	volumeSvc := inmocks.NewMockVolumeService(t)
		234	+	containerSvc := &previewContainerService{
		235	+	MockContainerService: inmocks.NewMockContainerService(t),
		236	+	preview: func(context.Context, string) (*domain.CleanupReport, error) {
		237	+	return nil, nil
		238	+	},
		239	+	}
		240	+	cp := &localControlPlane{containerSvc: containerSvc, volumeSvc: volumeSvc}
		241	+
		242	+	report, err := cp.GetRouteCleanupPreview(ctx, "app.local")
		243	+	require.NoError(t, err)
		244	+	assert.Nil(t, report)
		245	+	}
		246	+
219	func TestLocalControlPlane_RemoveRouteReconcilesRuntimeWhenRouteAlreadyMissing(t *testing.T) {	247		func TestLocalControlPlane_RemoveRouteReconcilesRuntimeWhenRouteAlreadyMissing(t *testing.T) {
220	ctx := context.Background()	248		ctx := context.Background()
221	configSvc := inmocks.NewMockConfigService(t)	249		configSvc := inmocks.NewMockConfigService(t)

M internal/adapters/in/cli/pin.go +2 -2


@@ -240,9 +240,9 @@ func deploySelectedTag(ctx context.Context, cp ControlPlane, route *domain.Route
240	240		route.Image = oldImage
241	241		if revertErr := cp.UpdateRoute(ctx, *route); revertErr != nil {
242	242		_ = cliWritef(ew, "WARNING: deploy failed and could not revert route: %v\n", revertErr)
243		-	return fmt.Errorf("failed to deploy; revert failed: %v; deploy error: %w", revertErr, formattedErr)
	243	+	return fmt.Errorf("failed to deploy; revert failed: %w; deploy error: %v", revertErr, formattedErr)
244	244		}
245		-	return fmt.Errorf("%w\nroute reverted to previous image", formattedErr)
	245	+	return fmt.Errorf("deploy failed; route reverted to previous image: %w", formattedErr)
246	246		}
247	247		containerID := shortContainerID(result.ContainerID)
248	248		return cliWriteLine(out, styles.RenderSuccess(fmt.Sprintf("Pinned %s to %s (container: %s)", pinDomain, selectedTag, containerID)))


@@ -240,9 +240,9 @@ func deploySelectedTag(ctx context.Context, cp ControlPlane, route *domain.Route
240		route.Image = oldImage	240		route.Image = oldImage
241		if revertErr := cp.UpdateRoute(ctx, *route); revertErr != nil {	241		if revertErr := cp.UpdateRoute(ctx, *route); revertErr != nil {
242		_ = cliWritef(ew, "WARNING: deploy failed and could not revert route: %v\n", revertErr)	242		_ = cliWritef(ew, "WARNING: deploy failed and could not revert route: %v\n", revertErr)
243	-	return fmt.Errorf("failed to deploy; revert failed: %v; deploy error: %w", revertErr, formattedErr)	243	+	return fmt.Errorf("failed to deploy; revert failed: %w; deploy error: %v", revertErr, formattedErr)
244		}	244		}
245	-	return fmt.Errorf("%w\nroute reverted to previous image", formattedErr)	245	+	return fmt.Errorf("deploy failed; route reverted to previous image: %w", formattedErr)
246		}	246		}
247		containerID := shortContainerID(result.ContainerID)	247		containerID := shortContainerID(result.ContainerID)
248		return cliWriteLine(out, styles.RenderSuccess(fmt.Sprintf("Pinned %s to %s (container: %s)", pinDomain, selectedTag, containerID)))	248		return cliWriteLine(out, styles.RenderSuccess(fmt.Sprintf("Pinned %s to %s (container: %s)", pinDomain, selectedTag, containerID)))

M internal/adapters/in/cli/pin_test.go +1 -1


@@ -100,5 +100,5 @@ func TestDeploySelectedTag_FormatsDeployFailureAfterRevert(t *testing.T) {
100	100		assert.Contains(t, err.Error(), "failed to deploy")
101	101		assert.Contains(t, err.Error(), "Cause: health check failed")
102	102		assert.Contains(t, err.Error(), "Hint: check DATABASE_URL")
103		-	assert.Contains(t, err.Error(), "route reverted")
	103	+	assert.Contains(t, err.Error(), "route reverted to previous image")
104	104		}


@@ -100,5 +100,5 @@ func TestDeploySelectedTag_FormatsDeployFailureAfterRevert(t *testing.T) {
100		assert.Contains(t, err.Error(), "failed to deploy")	100		assert.Contains(t, err.Error(), "failed to deploy")
101		assert.Contains(t, err.Error(), "Cause: health check failed")	101		assert.Contains(t, err.Error(), "Cause: health check failed")
102		assert.Contains(t, err.Error(), "Hint: check DATABASE_URL")	102		assert.Contains(t, err.Error(), "Hint: check DATABASE_URL")
103	-	assert.Contains(t, err.Error(), "route reverted")	103	+	assert.Contains(t, err.Error(), "route reverted to previous image")
104		}	104		}

M internal/adapters/in/cli/remote/config.go +6 -3


@@ -249,17 +249,20 @@ func Resolve(flagRemote, flagToken string, flagInsecure bool) (*ResolvedRemote,
249	249		// when an explicit remote selector is invalid instead of silently falling back
250	250		// to local mode.
251	251		func ResolveStrict(flagRemote, flagToken string, flagInsecure bool) (*ResolvedRemote, bool, error) {
252		-	remotes, _ := LoadRemotes("")
	252	+	remotes, err := LoadRemotes("")
	253	+	if err != nil {
	254	+	return nil, false, fmt.Errorf("loading remotes: %w", err)
	255	+	}
253	256
254	257		name, url, found := resolveTarget(flagRemote, remotes)
255	258		if !found && flagRemote != "" {
256		-	return nil, false, fmt.Errorf("remote %q not found", flagRemote)
	259	+	return nil, false, fmt.Errorf("remote %q: %w", flagRemote, domain.ErrRemoteNotFound)
257	260		}
258	261		if !found {
259	262		if envRemote := os.Getenv("GORDON_REMOTE"); envRemote != "" {
260	263		name, url, found = resolveTarget(envRemote, remotes)
261	264		if !found {
262		-	return nil, false, fmt.Errorf("remote %q from GORDON_REMOTE not found", envRemote)
	265	+	return nil, false, fmt.Errorf("remote %q from GORDON_REMOTE: %w", envRemote, domain.ErrRemoteNotFound)
263	266		}
264	267		}
265	268		}

 // when an explicit remote selector is invalid instead of silently falling back
 // to local mode.
 func ResolveStrict(flagRemote, flagToken string, flagInsecure bool) (*ResolvedRemote, bool, error) {
-	remotes, _ := LoadRemotes("")
+	remotes, err := LoadRemotes("")
+	if err != nil {
+		return nil, false, fmt.Errorf("loading remotes: %w", err)
+	}
 	name, url, found := resolveTarget(flagRemote, remotes)
 	if !found && flagRemote != "" {
-		return nil, false, fmt.Errorf("remote %q not found", flagRemote)
+		return nil, false, fmt.Errorf("remote %q: %w", flagRemote, domain.ErrRemoteNotFound)
 	}
 	if !found {
 		if envRemote := os.Getenv("GORDON_REMOTE"); envRemote != "" {
 			name, url, found = resolveTarget(envRemote, remotes)
 			if !found {
-				return nil, false, fmt.Errorf("remote %q from GORDON_REMOTE not found", envRemote)
+				return nil, false, fmt.Errorf("remote %q from GORDON_REMOTE: %w", envRemote, domain.ErrRemoteNotFound)
 			}
 		}
 	}

M internal/adapters/in/cli/remote/resolve_test.go +34 -0

@@ -1,10 +1,15 @@
1	1		package remote
2	2
3	3		import (
	4	+	"errors"
	5	+	"os"
	6	+	"path/filepath"
4	7		"testing"
5	8
6	9		"github.com/stretchr/testify/assert"
7	10		"github.com/stretchr/testify/require"
	11	+
	12	+	"github.com/bnema/gordon/internal/domain"
8	13		)
9	14
10	15		func TestResolve_FlagNameLookup(t *testing.T) {

@@ -202,3 +207,32 @@ func TestResolve_UnknownNameReturnsNotFound(t *testing.T) {
202	207		assert.False(t, ok)
203	208		assert.Nil(t, resolved)
204	209		}
	210	+
	211	+	func TestResolveStrict_UnknownFlagRemoteWrapsErrRemoteNotFound(t *testing.T) {
	212	+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
	213	+	t.Setenv("GORDON_REMOTE", "")
	214	+	t.Setenv("GORDON_TOKEN", "")
	215	+
	216	+	resolved, ok, err := ResolveStrict("nonexistent", "", false)
	217	+	require.Error(t, err)
	218	+	assert.False(t, ok)
	219	+	assert.Nil(t, resolved)
	220	+	assert.ErrorIs(t, err, domain.ErrRemoteNotFound)
	221	+	}
	222	+
	223	+	func TestResolveStrict_LoadRemotesErrorIsReturned(t *testing.T) {
	224	+	configHome := t.TempDir()
	225	+	t.Setenv("XDG_CONFIG_HOME", configHome)
	226	+	t.Setenv("GORDON_REMOTE", "")
	227	+	t.Setenv("GORDON_TOKEN", "")
	228	+
	229	+	remotesPath := filepath.Join(configHome, "gordon", "remotes.toml")
	230	+	require.NoError(t, os.MkdirAll(remotesPath, 0o755))
	231	+
	232	+	resolved, ok, err := ResolveStrict("", "", false)
	233	+	require.Error(t, err)
	234	+	assert.False(t, ok)
	235	+	assert.Nil(t, resolved)
	236	+	assert.Contains(t, err.Error(), "loading remotes")
	237	+	assert.False(t, errors.Is(err, domain.ErrRemoteNotFound))
	238	+	}

@@ -1,10 +1,15 @@
1	package remote	1		package remote
2		2
3	import (	3		import (
		4	+	"errors"
		5	+	"os"
		6	+	"path/filepath"
4	"testing"	7		"testing"
5		8
6	"github.com/stretchr/testify/assert"	9		"github.com/stretchr/testify/assert"
7	"github.com/stretchr/testify/require"	10		"github.com/stretchr/testify/require"
		11	+
		12	+	"github.com/bnema/gordon/internal/domain"
8	)	13		)
9		14
10	func TestResolve_FlagNameLookup(t *testing.T) {	15		func TestResolve_FlagNameLookup(t *testing.T) {

@@ -202,3 +207,32 @@ func TestResolve_UnknownNameReturnsNotFound(t *testing.T) {
202	assert.False(t, ok)	207		assert.False(t, ok)
203	assert.Nil(t, resolved)	208		assert.Nil(t, resolved)
204	}	209		}
		210	+
		211	+	func TestResolveStrict_UnknownFlagRemoteWrapsErrRemoteNotFound(t *testing.T) {
		212	+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
		213	+	t.Setenv("GORDON_REMOTE", "")
		214	+	t.Setenv("GORDON_TOKEN", "")
		215	+
		216	+	resolved, ok, err := ResolveStrict("nonexistent", "", false)
		217	+	require.Error(t, err)
		218	+	assert.False(t, ok)
		219	+	assert.Nil(t, resolved)
		220	+	assert.ErrorIs(t, err, domain.ErrRemoteNotFound)
		221	+	}
		222	+
		223	+	func TestResolveStrict_LoadRemotesErrorIsReturned(t *testing.T) {
		224	+	configHome := t.TempDir()
		225	+	t.Setenv("XDG_CONFIG_HOME", configHome)
		226	+	t.Setenv("GORDON_REMOTE", "")
		227	+	t.Setenv("GORDON_TOKEN", "")
		228	+
		229	+	remotesPath := filepath.Join(configHome, "gordon", "remotes.toml")
		230	+	require.NoError(t, os.MkdirAll(remotesPath, 0o755))
		231	+
		232	+	resolved, ok, err := ResolveStrict("", "", false)
		233	+	require.Error(t, err)
		234	+	assert.False(t, ok)
		235	+	assert.Nil(t, resolved)
		236	+	assert.Contains(t, err.Error(), "loading remotes")
		237	+	assert.False(t, errors.Is(err, domain.ErrRemoteNotFound))
		238	+	}

M internal/adapters/in/cli/routes.go +2 -3


@@ -1579,7 +1579,7 @@ Examples:
1579	1579		gordon --remote https://gordon.mydomain.com routes add api.mydomain.com api:v2`,
1580	1580		Args: cobra.RangeArgs(1, 2),
1581	1581		RunE: func(cmd *cobra.Command, args []string) error {
1582		-	ctx := context.Background()
	1582	+	ctx := cmd.Context()
1583	1583		routeDomain := args[0]
1584	1584
1585	1585		// Resolve image from positional argument or flag

@@ -1623,8 +1623,7 @@ Examples:
1623	1623		}
1624	1624		}
1625	1625
1626		-	fmt.Println(styles.RenderSuccess(fmt.Sprintf("Route configured: %s -> %s", routeDomain, image)))
1627		-	return nil
	1626	+	return cliWriteLine(cmd.OutOrStdout(), styles.RenderSuccess(fmt.Sprintf("Route configured: %s -> %s", routeDomain, image)))
1628	1627		},
1629	1628		}
1630	1629

   gordon --remote https://gordon.mydomain.com routes add api.mydomain.com api:v2`,
 		Args: cobra.RangeArgs(1, 2),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			ctx := context.Background()
+			ctx := cmd.Context()
 			routeDomain := args[0]
 			// Resolve image from positional argument or flag
 				}
 			}
-			fmt.Println(styles.RenderSuccess(fmt.Sprintf("Route configured: %s -> %s", routeDomain, image)))
+			return cliWriteLine(cmd.OutOrStdout(), styles.RenderSuccess(fmt.Sprintf("Route configured: %s -> %s", routeDomain, image)))
-			return nil
 		},
 	}