Merge pull request #1626 from entireio/chore/control-plane-output-cleanup

Standardize control-plane command output to CLI house style

Author: Soph

.golangci.yaml

@@ -107,6 +107,9 @@ linters:
         - pattern: '^.*\.Checkout$'
           msg: "go-git Checkout deletes .gitignored dirs - use CheckoutBranch() from git_operations.go"
           pkg: 'github\.com/go-git/go-git'
+        - pattern: '^.*\.(Print|Println|Printf)$'
+          msg: "cobra's Print* writes to OutOrStderr (stderr in production); use fmt.Fprint*(cmd.OutOrStdout(), ...)"
+          pkg: 'github\.com/spf13/cobra'
     govet:
       enable-all: true
       disable:

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/),
 and this project adheres to [Semantic Versioning](https://semver.org/).
 
+## [Unreleased]
+
+### Changed
+
+- Control-plane commands (`org`, `project`, `repo`, `grant`) now print human-readable confirmations by default; the wire JSON (including `repo create`'s `entire://` remote) moved behind `--json`. Empty `--json` lists emit `[]`, and success messages moved from stderr to stdout ([#1626](https://github.com/entireio/cli/pull/1626))
+
 ## [0.7.8] - 2026-06-30
 
 ### Added

cmd/entire/cli/corecmd.go

@@ -25,7 +25,7 @@ import (
 //     (local/dev deployments where the core isn't behind TLS). Hidden, as
 //     elsewhere in the CLI.
 func addControlPlaneFlags(cmd *cobra.Command) {
-	cmd.PersistentFlags().Bool("json", false, "output raw JSON instead of a table")
+	cmd.PersistentFlags().Bool("json", false, "Output raw JSON instead of a table")
 	cmd.PersistentFlags().Bool("insecure-http-auth", false, "Allow authentication over plain HTTP (insecure, for local development only)")
 	if err := cmd.PersistentFlags().MarkHidden("insecure-http-auth"); err != nil {
 		panic(fmt.Sprintf("hide insecure-http-auth flag: %v", err))
@@ -94,12 +94,12 @@ func runControlPlaneDelete(
 			// delete call — e.g. a ULID passed straight through, or a concurrent
 			// delete) is the desired end state, not an error.
 			if isCoreNotFound(err) {
-				cmd.Printf("%s not found; nothing to delete\n", label)
+				fmt.Fprintf(cmd.OutOrStdout(), "%s not found; nothing to delete\n", label)
 				return nil
 			}
 			return err
 		}
-		cmd.Printf("Deleted %s\n", label)
+		fmt.Fprintf(cmd.OutOrStdout(), "✓ Deleted %s\n", label)
 		return nil
 	})
 }
@@ -142,36 +142,42 @@ func confirmControlPlaneDeletion(ctx context.Context, w io.Writer, label string,
 }
 
 // runCoreList fetches a slice via fn and renders it as an aligned table
-// (default) or the raw wire JSON (--json). headers names the columns; row
-// maps one item to its cells in the same order. The human view keeps the
-// output actionable — only the columns a person acts on — while --json
-// preserves the full model for scripting.
-func runCoreList[T any](cmd *cobra.Command, headers []string, row func(T) []string, fn func(ctx context.Context, c *coreapi.Client) ([]T, error)) error {
-	return runCore(cmd, renderCoreList(cmd, headers, row, fn))
+// (default) or the raw wire JSON (--json). empty is the full sentence printed
+// to stdout in place of the table when there are no items (e.g. "No
+// organizations found."). headers names the columns; row maps one item to its
+// cells in the same order. The human view keeps the output actionable — only
+// the columns a person acts on — while --json preserves the full model for
+// scripting.
+func runCoreList[T any](cmd *cobra.Command, empty string, headers []string, row func(T) []string, fn func(ctx context.Context, c *coreapi.Client) ([]T, error)) error {
+	return runCore(cmd, renderCoreList(cmd, empty, headers, row, fn))
 }
 
 // runCoreListForCluster is runCoreList for a resource-provider command (see
-// runCoreForCluster): identical table/JSON rendering, but dialing the core that
-// fronts clusterHost rather than the active context.
-func runCoreListForCluster[T any](cmd *cobra.Command, clusterHost string, headers []string, row func(T) []string, fn func(ctx context.Context, c *coreapi.Client) ([]T, error)) error {
-	return runCoreForCluster(cmd, clusterHost, renderCoreList(cmd, headers, row, fn))
+// runCoreForCluster): identical table/JSON/empty-state rendering, but dialing
+// the core that fronts clusterHost rather than the active context.
+func runCoreListForCluster[T any](cmd *cobra.Command, clusterHost, empty string, headers []string, row func(T) []string, fn func(ctx context.Context, c *coreapi.Client) ([]T, error)) error {
+	return runCoreForCluster(cmd, clusterHost, renderCoreList(cmd, empty, headers, row, fn))
 }
 
 // renderCoreList builds the run-function shared by runCoreList and
-// runCoreListForCluster: fetch via fn, then render as a table (default) or raw
-// JSON (--json). Kept separate from the client-selection so the two list
-// variants differ only in which core they dial.
-func renderCoreList[T any](cmd *cobra.Command, headers []string, row func(T) []string, fn func(ctx context.Context, c *coreapi.Client) ([]T, error)) func(context.Context, *coreapi.Client) error {
+// runCoreListForCluster: fetch via fn, then render as a table (default), the
+// empty sentence (no items), or raw JSON (--json). Kept separate from the
+// client-selection so the two list variants differ only in which core they
+// dial.
+func renderCoreList[T any](cmd *cobra.Command, empty string, headers []string, row func(T) []string, fn func(ctx context.Context, c *coreapi.Client) ([]T, error)) func(context.Context, *coreapi.Client) error {
 	return func(ctx context.Context, c *coreapi.Client) error {
 		items, err := fn(ctx, c)
 		if err != nil {
 			return err
 		}
 		if jsonRequested(cmd) {
+			if items == nil {
+				items = []T{} // a nil slice encodes as null; scripts expect []
+			}
 			return printJSON(cmd.OutOrStdout(), items)
 		}
 		if len(items) == 0 {
-			fmt.Fprintln(cmd.ErrOrStderr(), "(none)")
+			fmt.Fprintln(cmd.OutOrStdout(), empty)
 			return nil
 		}
 		return printTable(cmd.OutOrStdout(), headers, items, row)
@@ -348,19 +354,26 @@ func writeTableRow(b *strings.Builder, cells []string, widths []int, styleFor fu
 	b.WriteByte('\n')
 }
 
-// runCoreJSON runs fn against an authenticated control-plane client and
-// prints its result as indented JSON. It owns the preamble every
-// control-plane command shares: silence usage so input errors don't spam
-// the usage block, build the client, and map an API error to a
-// problem-detail SilentError. Commands supply only the call + the value to
-// render.
-func runCoreJSON(cmd *cobra.Command, fn func(ctx context.Context, c *coreapi.Client) (any, error)) error {
+// runCoreMutation runs fn against the control plane and renders its outcome
+// the way the rest of the CLI renders mutations: prints the caller's
+// ✓-prefixed confirmation on stdout by default, or the wire object as JSON
+// when --json was passed. fn
+// returns both so the human line can name the created resource while --json
+// preserves the full wire model (additive-only: synthesized fields like the
+// repo remote URL are merged in, nothing is ever omitted). It owns the same
+// preamble as the other runCore variants: silence usage, build the client,
+// map API errors to problem-detail messages.
+func runCoreMutation(cmd *cobra.Command, fn func(ctx context.Context, c *coreapi.Client) (message string, wire any, err error)) error {
 	return runCore(cmd, func(ctx context.Context, c *coreapi.Client) error {
-		out, err := fn(ctx, c)
+		message, wire, err := fn(ctx, c)
 		if err != nil {
 			return err
 		}
-		return printJSON(cmd.OutOrStdout(), out)
+		if jsonRequested(cmd) {
+			return printJSON(cmd.OutOrStdout(), wire)
+		}
+		fmt.Fprintln(cmd.OutOrStdout(), message)
+		return nil
 	})
 }
 
@@ -370,11 +383,13 @@ func runCoreJSON(cmd *cobra.Command, fn func(ctx context.Context, c *coreapi.Cli
 // without standing up the auth/context/TLS stack.
 var activeCoreClient = func(context.Context) (*coreapi.Client, error) { return coreapi.New() }
 
-// runCore is the variant for commands that don't render JSON (delete,
-// revoke, remove): it runs the same preamble — silence usage, build
-// client, map API errors — and leaves any success output to fn. The client
-// dials the active context's core (coreapi.New); use runCoreForCluster for
-// commands addressed at a specific cluster.
+// runCore is the shared base for every active-context control-plane command:
+// it owns the preamble only — silence usage, build the client, map API
+// errors — and leaves all rendering to fn. The delete/revoke verbs call it
+// directly and render their own output; runCoreList, runCoreObject, and
+// runCoreMutation build on it to add their table/JSON/confirmation
+// rendering. The client dials the active context's core (coreapi.New); use
+// runCoreForCluster for commands addressed at a specific cluster.
 func runCore(cmd *cobra.Command, fn func(ctx context.Context, c *coreapi.Client) error) error {
 	return runCoreClient(cmd, activeCoreClient, fn)
 }
@@ -446,7 +461,7 @@ func renderCoreError(err error) error {
 }
 
 // printJSON writes v as indented JSON to w — the --json view for list/get
-// and the default for create commands that echo the new object.
+// and mutations.
 func printJSON(w io.Writer, v any) error {
 	enc := json.NewEncoder(w)
 	enc.SetIndent("", "  ")

cmd/entire/cli/corecmd_delete_test.go

@@ -32,10 +32,18 @@ func writeNotFoundProblem(t *testing.T, w http.ResponseWriter) {
 	}
 }
 
-// runDeleteCmd points the active-context client at srv via the activeCoreClient
-// seam, runs newCmd() with args, and returns its stdout and error. The caller
-// must not be parallel: the seam is package-global.
-func runDeleteCmd(t *testing.T, newCmd func() *cobra.Command, srvURL string, args ...string) (string, error) {
+// runCoreCmd runs any active-context control-plane command against a seamed
+// httptest core: it points the active-context client at srv via the
+// activeCoreClient seam, runs newCmd() with args, and returns its stdout,
+// stderr, and error. Commands dialing via runCoreForCluster (mirror
+// create/remove/collaborators) bypass the seam and need their own httptest
+// wiring. The caller must not be parallel: the seam is package-global.
+//
+// Note: cobra's cmd.Print* falls back to OutOrStderr(), which under SetOut
+// resolves to the stdout buffer — so Empty(errOut) assertions in these tests
+// only guard explicit ErrOrStderr writes; the Contains-on-stdout assertions
+// are what pin the production stream.
+func runCoreCmd(t *testing.T, newCmd func() *cobra.Command, srvURL string, args ...string) (stdout, stderr string, err error) {
 	t.Helper()
 	prev := activeCoreClient
 	activeCoreClient = func(context.Context) (*coreapi.Client, error) {
@@ -44,12 +52,12 @@ func runDeleteCmd(t *testing.T, newCmd func() *cobra.Command, srvURL string, arg
 	t.Cleanup(func() { activeCoreClient = prev })
 
 	cmd := newCmd()
-	var out bytes.Buffer
+	var out, errW bytes.Buffer
 	cmd.SetOut(&out)
-	cmd.SetErr(&bytes.Buffer{})
+	cmd.SetErr(&errW)
 	cmd.SetArgs(args)
-	err := cmd.ExecuteContext(t.Context())
-	return out.String(), err
+	err = cmd.ExecuteContext(t.Context())
+	return out.String(), errW.String(), err
 }
 
 // TestControlPlaneDelete_Wiring exercises the org/project/repo delete commands
@@ -79,11 +87,12 @@ func TestControlPlaneDelete_Wiring(t *testing.T) {
 			}))
 			t.Cleanup(srv.Close)
 
-			out, err := runDeleteCmd(t, tc.newCmd, srv.URL, testDeleteULID, "--force")
+			out, errOut, err := runCoreCmd(t, tc.newCmd, srv.URL, testDeleteULID, "--force")
 			require.NoError(t, err)
 			require.Equal(t, http.MethodDelete, gotMethod)
 			require.Equal(t, tc.wantPath, gotPath)
-			require.Contains(t, out, "Deleted "+tc.noun+" "+testDeleteULID)
+			require.Contains(t, out, "✓ Deleted "+tc.noun+" "+testDeleteULID)
+			require.Empty(t, errOut, "no explicit ErrOrStderr writes expected")
 		})
 
 		t.Run(tc.noun+"/already-gone is idempotent", func(t *testing.T) {
@@ -92,9 +101,10 @@ func TestControlPlaneDelete_Wiring(t *testing.T) {
 			}))
 			t.Cleanup(srv.Close)
 
-			out, err := runDeleteCmd(t, tc.newCmd, srv.URL, testDeleteULID, "--force")
+			out, errOut, err := runCoreCmd(t, tc.newCmd, srv.URL, testDeleteULID, "--force")
 			require.NoError(t, err)
 			require.Contains(t, out, "not found; nothing to delete")
+			require.Empty(t, errOut)
 		})
 
 		t.Run(tc.noun+"/refuses without --force when non-interactive", func(t *testing.T) {
@@ -104,7 +114,7 @@ func TestControlPlaneDelete_Wiring(t *testing.T) {
 			}))
 			t.Cleanup(srv.Close)
 
-			_, err := runDeleteCmd(t, tc.newCmd, srv.URL, testDeleteULID)
+			_, _, err := runCoreCmd(t, tc.newCmd, srv.URL, testDeleteULID)
 			require.Error(t, err)
 			require.Contains(t, err.Error(), "--force")
 		})

cmd/entire/cli/corecmd_list_test.go

@@ -0,0 +1,56 @@
+package cli
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/entireio/cli/internal/coreapi"
+)
+
+// serveOrgList answers GET /api/v1/orgs with the given orgs, standing in for
+// the control plane behind `entire org list`.
+func serveOrgList(t *testing.T, orgs []coreapi.Org) *httptest.Server {
+	t.Helper()
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, http.MethodGet, r.Method)
+		w.Header().Set("Content-Type", "application/json")
+		if err := writeJSON(w, &coreapi.ListOrgsOutputBody{Orgs: orgs}); err != nil {
+			t.Errorf("encode orgs: %v", err)
+		}
+	}))
+	t.Cleanup(srv.Close)
+	return srv
+}
+
+// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
+func TestRunCoreList_EmptyHumanMessageOnStdout(t *testing.T) {
+	srv := serveOrgList(t, nil)
+	out, errOut, err := runCoreCmd(t, newOrgListCmd, srv.URL)
+	require.NoError(t, err)
+	require.Contains(t, out, "No organizations found.")
+	require.Empty(t, errOut, "empty-state message must go to stdout")
+}
+
+// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
+func TestRunCoreList_EmptyJSONIsArray(t *testing.T) {
+	srv := serveOrgList(t, nil)
+	// org list's --json is persistent on the group root, so drive the full
+	// group command with "list" as a subcommand arg.
+	out, _, err := runCoreCmd(t, newOrgCmd, srv.URL, "list", "--json")
+	require.NoError(t, err)
+	require.JSONEq(t, "[]", out, "empty --json list must be [], not null")
+}
+
+// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
+func TestRunCoreList_RendersRows(t *testing.T) {
+	srv := serveOrgList(t, []coreapi.Org{{ID: testDeleteULID, Name: "acme", Region: "us"}})
+	out, errOut, err := runCoreCmd(t, newOrgListCmd, srv.URL)
+	require.NoError(t, err)
+	require.Contains(t, out, "NAME")
+	require.Contains(t, out, "acme")
+	require.Empty(t, errOut)
+}