Address PR review findings · Entire

Address PR review findings

21e2d8d→main· peyton-alt·2w ago·12 files·+227 added/-88 removed

Append the server's 404 detail to the mirror-remove error (extracted to a testable helper with coverage), close the review's test gaps (repo create assembly, grant revoke output, real-command list harness), fix stale group doc comments, add a changelog entry for the new output contract, and add a forbidigo guard so cobra Print* (stderr fallback) can't reintroduce the bug this PR fixed.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

5e25c75a4297View transcript

[?
Improve Control-Plane CLI Output and UXClaude Code·14 steps](/content/gh/entireio/cli/session/951dfb50-f9e5-45be-8575-9dd17e7639e5#timeline-5e25c75a4297/index.html)

Changes

M.golangci.yaml+3
MCHANGELOG.md+6
cmd/entire/cli
Mcorecmd.go+10/-7
- Mcorecmd_delete_test.go+12/-5
- Mcorecmd_list_test.go+29/-39
- Mcorecmd_mutation_test.go+49/-1
- Mgrant_wiring_test.go+16/-5
- Morg.go+3/-3
- Mproject.go+3/-2
- Mrepo.go+5/-4
- Mrepo_mirror.go+29/-22
- Mrepo_mirror_test.go+62

106 unmodified lines

107
108
109
110
111
112
113
114
115

106 unmodified lines

- 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:

The format is based on Keep a Changelog, and this project adheres to Semantic Versioning.

[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)

[0.7.8] - 2026-06-30

Added


MCHANGELOG.md+6

354 unmodified lines

355 356 357 358 359 358 359 360 361 362 363 19 unmodified lines

383 384 385 385 386 387 388 389 386 387 388 389 390 391 392 393 394 395

354 unmodified lines

// runCoreMutation runs fn against the control plane and renders its outcome // the way the rest of the CLI renders mutations: the ✓ human confirmation on // stdout by default, or the wire object as JSON when --json was passed. fn // 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 19 unmodified lines

// 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) }


Mcmd/entire/cli/corecmd.go+10/-7

31 unmodified lines

32 33 34 35 36 37 38 35 36 37 38 39 40 41 42 43 44 45 46 47 48 43 unmodified lines

92 93 94 88 95 96 97 98

31 unmodified lines

}

// runCoreCmd runs any 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. The // caller must not be parallel: the seam is package-global. // 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 43 unmodified lines

require.Equal(t, http.MethodDelete, gotMethod) require.Equal(t, tc.wantPath, gotPath) require.Contains(t, out, "✓ Deleted "+tc.noun+" "+testDeleteULID) require.Empty(t, errOut, "success output must go to stdout, not stderr") require.Empty(t, errOut, "no explicit ErrOrStderr writes expected") })

t.Run(tc.noun+"/already-gone is idempotent", func(t *testing.T) {


Mcmd/entire/cli/corecmd_delete_test.go+12/-5

1 2 3 4 5 4 5 6 7 8 8 9 10 11 12 13 14 15 16 17 14 15 16 17 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 18 19 20 21 22 23 24 25 26 27 28 47 29 30 49 31 32 33 34 35 36 37 54 38 39 56 40 41 42 43 44 45 46 47 60 48 49 62 50 51 52 53 54 55


package cli

import (
    "bytes"
    "context"
    "net/http"
    "net/http/httptest"
    "testing"

"github.com/spf13/cobra"
    "github.com/stretchr/testify/assert"
    "github.com/stretchr/testify/require"

"github.com/entireio/cli/internal/coreapi"
)

// runListRender builds a minimal command wired to renderCoreList via runCoreList so
// the rendering path (table / empty state / --json) is exercised without a
// server: fn returns items directly.
func runListRender(t *testing.T, jsonFlag bool, items []coreapi.Org) (stdout, stderr string) {
// 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()
    prev := activeCoreClient
    activeCoreClient = func(context.Context) (*coreapi.Client, error) {
        // The URL is never contacted: fn below returns items directly, so
        // the client only needs to construct.
        return coreapi.NewWithBearer("http://127.0.0.1:0", "tok")
    }

t.Cleanup(func() { activeCoreClient = prev })

cmd := &cobra.Command{
        Use: "list",
        RunE: func(cmd *cobra.Command, _ []string) error {
            return runCoreList(cmd, "No organizations found.", orgColumns, orgRow,
                func(context.Context, *coreapi.Client) ([]coreapi.Org, error) { return items, nil })
        },
    }
    cmd.Flags().Bool("json", false, "Output raw JSON instead of a table")
    var out, errW bytes.Buffer
    cmd.SetOut(&out)
    cmd.SetErr(&errW)
    args := []string{}
    if jsonFlag {
        args = append(args, "--json")
    }
    cmd.SetArgs(args)
    require.NoError(t, cmd.ExecuteContext(t.Context()))
    return out.String(), errW.String()
    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: swaps the package-level activeCoreClient seam.
// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
func TestRunCoreList_EmptyHumanMessageOnStdout(t *testing.T) {
    out, errOut := runListRender(t, false, nil)
    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: swaps the package-level activeCoreClient seam.
// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
func TestRunCoreList_EmptyJSONIsArray(t *testing.T) {
    out, _ := runListRender(t, true, nil)
    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: swaps the package-level activeCoreClient seam.
// Not parallel: runCoreCmd swaps the package-level activeCoreClient seam.
func TestRunCoreList_RendersRows(t *testing.T) {
    out, errOut := runListRender(t, false, []coreapi.Org{{ID: testDeleteULID, Name: "acme", Region: "us"}})
    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)