From acbe5537dc7cd5dcd1d62e8b76607d725d44e6c5 Mon Sep 17 00:00:00 2001 From: naliyi <154817482+naliyi@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:58:25 +0000 Subject: [PATCH 1/4] feat(cli): cashctl-style errors, help, and a network spinner Align ncli's command UX with cashctl's. A failure now takes one of three text shapes instead of a timestamped zerolog line plus a help dump on the wrong stream: bare invocation help alone, no error line wrong invocation "Error: ", blank line, then help runtime failure "Error: " alone All three go to stderr; help used to land on stdout, which contradicted "a command's result goes to stdout only" and could pollute a pipe into jq. "Error:" is red only on a real terminal, and never under NO_COLOR -- which ncli did not honour anywhere before. Exit codes are unchanged. A bare group command still exits 2, so the friendlier output never turns a mistyped subcommand into apparent success, and --json still emits exactly one structured line and never help. Help is opt-in, mirroring cashctl's split: UsageError stays help-free for a usage-coded runtime refusal (a relay answering 501 because membership is off, a vault password prompt that can't run), InvocationError adds the error-then-help shape, and HelpError prints help alone. InvocationOrHelp picks between the last two from the args a validator already has, since the same check fires both for "you gave me nothing" and "you gave me the wrong thing". Fixes two contract violations found while verifying: - An unknown flag was reported twice -- cobra's own "Error:" plus usage dump, and ncli's own line stacked under it -- and exited 1 instead of 2. Silencing cobra on the root and classifying ExecuteC's own errors in main fixes that, and with it every unwrapped MarkFlagRequired. - "bunker sessions revoke-grant" with no --method exited 1 as internal rather than 2 as usage; it was the one command with MarkFlagRequired and no ValidateRequiredFlags. Commands that block on the network now animate a spinner on stderr. Log records go through a writer that wipes the in-flight frame first, so the existing per-relay narration still prints and the next tick redraws beneath it. Off under --json, -q, NO_COLOR and a non-terminal stderr, and never wired into the commands that own the terminal (apply, ping --tui, bunker, the delegate wizard) or into the long-running relay server. EmitError keeps recording failures to ncli.log through a file-only logger, since it no longer prints via zerolog. Help text: trimmed the bulkiest Long blocks and tightened the Short lines over 60 characters. That is not cosmetic any more -- help is printed on every mis-invocation now, so find's 17-line Long was about to become the common case (find --help: 43 lines -> 35). --- cli/blossom/command.go | 15 ++- cli/blossom/download.go | 9 +- cli/blossom/list.go | 32 +++-- cli/blossom/mirror.go | 51 ++++---- cli/blossom/report.go | 11 +- cli/blossom/rm.go | 43 +++---- cli/blossom/servers.go | 22 +++- cli/blossom/shared.go | 9 ++ cli/blossom/upload.go | 47 ++++---- cli/bunker/command.go | 21 ++-- cli/bunker/identity.go | 4 +- cli/common/args.go | 127 ++++++++++++++++---- cli/common/errors.go | 68 ++++++++++- cli/common/errors_render_test.go | 198 +++++++++++++++++++++++++++++++ cli/common/logging.go | 30 ++++- cli/common/spinner.go | 138 +++++++++++++++++++++ cli/common/spinner_test.go | 126 ++++++++++++++++++++ cli/delegate/command.go | 4 +- cli/ncli/apply.go | 2 +- cli/ncli/decode.go | 20 ++-- cli/ncli/dump.go | 22 ++-- cli/ncli/find.go | 39 +++--- cli/ncli/id.go | 11 +- cli/ncli/id_sign.go | 6 +- cli/ncli/miner.go | 76 ++++++------ cli/ncli/ping.go | 48 ++++---- cli/ncli/publish.go | 11 +- cli/ncli/query.go | 14 +++ cli/ncli/root.go | 8 ++ cli/relay/admin.go | 45 +++---- cli/relay/command.go | 16 ++- cli/relay/context_run.go | 2 +- cmd/ncli/main.go | 39 ++++++ 33 files changed, 1026 insertions(+), 288 deletions(-) create mode 100644 cli/common/errors_render_test.go create mode 100644 cli/common/spinner.go create mode 100644 cli/common/spinner_test.go diff --git a/cli/blossom/command.go b/cli/blossom/command.go index 5fb1d9c..88b4781 100644 --- a/cli/blossom/command.go +++ b/cli/blossom/command.go @@ -17,14 +17,13 @@ func NewBlossomCommand() *cobra.Command { Long: `A client for the Blossom protocol (BUD-01..12): content-addressed blob storage authenticated with a Nostr identity instead of a login. -Every write (upload, rm, mirror) targets every server from --server, or -the default list from "ncli blossom servers add" -- reporting a result -per (item, server) pair, and exiting non-zero if any pair failed. -"download" tries the configured servers in order, stopping at the first -that answers; "list" queries one server by default, or every server with ---all.`, - Example: ` ncli blossom upload ./photo.jpg --identity satoshi`, - RunE: common.RequireSubcommand, +Writes (upload, rm, mirror) fan out to every configured server and exit +non-zero if any one failed; download tries them in order until one +answers.`, + Example: ` ncli blossom upload ./photo.jpg --identity satoshi + ncli blossom download -o photo.jpg + ncli blossom servers list`, + RunE: common.RequireSubcommand, } cmd.PersistentFlags().String("identity", "", "Identity to sign with -- vault label, nsec, npub, hex, nprofile, or nip-05") diff --git a/cli/blossom/download.go b/cli/blossom/download.go index eec2e0b..90602e7 100644 --- a/cli/blossom/download.go +++ b/cli/blossom/download.go @@ -3,6 +3,7 @@ package blossom import ( "fmt" "io" + "net/http" "os" "os/signal" "regexp" @@ -69,7 +70,13 @@ omitted, or streams to stdout with "-o -" (suppressing the summary line).`, timeout, _ := cmd.Flags().GetDuration("timeout") hc := newHTTPClient(timeout) - resp, usedServer, err := hc.GetFromServers(ctx, servers, hash, bclient.GetOptions{Ext: ext, Auth: auth}) + var resp *http.Response + var usedServer string + err = common.WithSpinner(cmd, fmt.Sprintf("downloading %s", shortHash(hash)), func() error { + var gErr error + resp, usedServer, gErr = hc.GetFromServers(ctx, servers, hash, bclient.GetOptions{Ext: ext, Auth: auth}) + return gErr + }) if err != nil { return classifyHTTPError(cmd, hash, err) } diff --git a/cli/blossom/list.go b/cli/blossom/list.go index cf04292..63d475a 100644 --- a/cli/blossom/list.go +++ b/cli/blossom/list.go @@ -25,15 +25,14 @@ func newListCommand() *cobra.Command { cmd := &cobra.Command{ Use: "list [identifier]", Short: "List blobs stored under a pubkey", - Long: `Query one Blossom server's GET /list/ (--server, or the first -configured server), or every configured server with --all, merged and -deduped by hash. + Long: `List the blobs one pubkey has stored, on the first configured server or +on every one with --all, merged and deduped by hash. -identifier may be a vault label, nsec, npub, hex pubkey, nprofile, or -nip-05 address, resolved to a hex pubkey; defaults to --identity's -resolved pubkey when omitted.`, +identifier accepts a vault label, npub, hex pubkey, nprofile or nip-05 +address, and defaults to --identity's pubkey when omitted.`, Example: ` ncli blossom list --identity satoshi - ncli blossom list --identity satoshi --all`, + ncli blossom list --identity satoshi --all + ncli blossom list name@example.com`, Args: common.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { ctx, cancel := signal.NotifyContext(cmd.Context(), syscall.SIGINT, syscall.SIGTERM) @@ -66,7 +65,7 @@ resolved pubkey when omitted.`, } } if pubKeyHex == "" { - return common.UsageError(cmd, fmt.Errorf("a pubkey argument or --identity is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("a pubkey argument or --identity is required")) } servers, err := resolveServers(cmd) @@ -84,11 +83,20 @@ resolved pubkey when omitted.`, hc := newHTTPClient(timeout) var descriptors []nipB7.BlobDescriptor - if all, _ := cmd.Flags().GetBool("all"); all { - descriptors, err = listAllServers(ctx, hc, servers, pubKeyHex, query, auth) - } else { - descriptors, err = hc.List(ctx, servers[0], pubKeyHex, query, auth) + all, _ := cmd.Flags().GetBool("all") + message := fmt.Sprintf("listing blobs on %s", servers[0]) + if all { + message = fmt.Sprintf("listing blobs on %d server(s)", len(servers)) } + err = common.WithSpinner(cmd, message, func() error { + var lErr error + if all { + descriptors, lErr = listAllServers(ctx, hc, servers, pubKeyHex, query, auth) + } else { + descriptors, lErr = hc.List(ctx, servers[0], pubKeyHex, query, auth) + } + return lErr + }) if err != nil { return classifyListError(cmd, pubKeyHex, err) } diff --git a/cli/blossom/mirror.go b/cli/blossom/mirror.go index 27dc47b..ac65e03 100644 --- a/cli/blossom/mirror.go +++ b/cli/blossom/mirror.go @@ -13,7 +13,7 @@ import ( func newMirrorCommand() *cobra.Command { cmd := &cobra.Command{ Use: "mirror ", - Short: "Ask your Blossom server(s) to fetch and store a blob from a URL", + Short: "Mirror a blob from a URL onto your Blossom server(s)", Long: `Sign a BUD-11 authorization and PUT /mirror to every target server (--server, or the configured default list) -- each server fetches source-url itself; no bytes pass through ncli. Reports a result per @@ -21,10 +21,10 @@ server.`, Example: ` ncli blossom mirror https://example.com/file.jpg --identity satoshi`, Args: func(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return common.UsageError(cmd, fmt.Errorf("exactly one source URL is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("exactly one source URL is required")) } if identity, _ := cmd.Flags().GetString("identity"); identity == "" { - return common.UsageError(cmd, fmt.Errorf("--identity is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--identity is required")) } return nil }, @@ -67,30 +67,33 @@ server.`, hc := newHTTPClient(timeout) report := &fanoutReport{} - for _, server := range servers { - res := serverResult{Item: sourceURL, Server: server} + _ = common.WithSpinner(cmd, fmt.Sprintf("mirroring to %d server(s)", len(servers)), func() error { + for _, server := range servers { + res := serverResult{Item: sourceURL, Server: server} - // Signed fresh per server, not once for the whole batch -- - // see the identical comment in upload.go. - auth, err := buildAuth(privKeyHex, pubKeyHex, nipB7.VerbUpload, hashes, ttl) - if err != nil { - res.Error = err.Error() - report.add(res) - continue - } + // Signed fresh per server, not once for the whole batch -- + // see the identical comment in upload.go. + auth, err := buildAuth(privKeyHex, pubKeyHex, nipB7.VerbUpload, hashes, ttl) + if err != nil { + res.Error = err.Error() + report.add(res) + continue + } - descriptor, err := hc.Mirror(ctx, server, sourceURL, auth) - if err != nil { - res.Error = describeError(err) + uploadErrorHint - } else { - res.OK = true - res.URL = descriptor.URL - res.Sha256 = descriptor.Sha256 - res.Size = descriptor.Size - res.Type = descriptor.Type + descriptor, err := hc.Mirror(ctx, server, sourceURL, auth) + if err != nil { + res.Error = describeError(err) + uploadErrorHint + } else { + res.OK = true + res.URL = descriptor.URL + res.Sha256 = descriptor.Sha256 + res.Size = descriptor.Size + res.Type = descriptor.Type + } + report.add(res) } - report.add(res) - } + return nil + }) printFanoutReport(jsonMode, report) if !report.allSucceeded() { diff --git a/cli/blossom/report.go b/cli/blossom/report.go index ca3c6d8..a618b46 100644 --- a/cli/blossom/report.go +++ b/cli/blossom/report.go @@ -20,16 +20,16 @@ server: --server, or the first configured default.`, Example: ` ncli blossom report --identity satoshi`, Args: func(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return common.UsageError(cmd, fmt.Errorf("exactly one hash is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("exactly one hash is required")) } if !nipB7.IsSHA256Hex(args[0]) { return common.InvalidInputError(cmd, args[0], fmt.Errorf("not a valid sha256 hash")) } if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if identity, _ := cmd.Flags().GetString("identity"); identity == "" { - return common.UsageError(cmd, fmt.Errorf("--identity is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--identity is required")) } return nil }, @@ -65,7 +65,10 @@ server: --server, or the first configured default.`, timeout, _ := cmd.Flags().GetDuration("timeout") hc := newHTTPClient(timeout) - if err := hc.Report(ctx, server, event); err != nil { + err = common.WithSpinner(cmd, fmt.Sprintf("reporting %s to %s", shortHash(hash), server), func() error { + return hc.Report(ctx, server, event) + }) + if err != nil { return classifyHTTPError(cmd, server, err) } diff --git a/cli/blossom/rm.go b/cli/blossom/rm.go index 47cd912..d347602 100644 --- a/cli/blossom/rm.go +++ b/cli/blossom/rm.go @@ -26,13 +26,13 @@ result per server. Requires --yes in a non-interactive session.`, Example: ` ncli blossom rm --identity satoshi --yes`, Args: func(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return common.UsageError(cmd, fmt.Errorf("exactly one hash is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("exactly one hash is required")) } if !nipB7.IsSHA256Hex(args[0]) { return common.InvalidInputError(cmd, args[0], fmt.Errorf("not a valid sha256 hash")) } if identity, _ := cmd.Flags().GetString("identity"); identity == "" { - return common.UsageError(cmd, fmt.Errorf("--identity is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--identity is required")) } return nil }, @@ -45,7 +45,7 @@ result per server. Requires --yes in a non-interactive session.`, if !yes { if !term.IsTerminal(int(os.Stdin.Fd())) { - return common.UsageError(cmd, fmt.Errorf("refusing to delete %s without --yes in a non-interactive session", hash)) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("refusing to delete %s without --yes in a non-interactive session", hash)) } if !promptYesNo(bufio.NewReader(os.Stdin), fmt.Sprintf("Delete blob %s from all target servers? [y/N] ", hash)) { return common.RuntimeError(cmd, fmt.Errorf("aborted")) @@ -69,25 +69,28 @@ result per server. Requires --yes in a non-interactive session.`, hc := newHTTPClient(timeout) report := &fanoutReport{} - for _, server := range servers { - res := serverResult{Item: hash, Server: server} - - // Signed fresh per server, not once for the whole batch -- - // see the identical comment in upload.go. - auth, err := buildAuth(privKeyHex, pubKeyHex, nipB7.VerbDelete, []string{hash}, ttl) - if err != nil { - res.Error = err.Error() + _ = common.WithSpinner(cmd, fmt.Sprintf("deleting %s from %d server(s)", shortHash(hash), len(servers)), func() error { + for _, server := range servers { + res := serverResult{Item: hash, Server: server} + + // Signed fresh per server, not once for the whole batch -- + // see the identical comment in upload.go. + auth, err := buildAuth(privKeyHex, pubKeyHex, nipB7.VerbDelete, []string{hash}, ttl) + if err != nil { + res.Error = err.Error() + report.add(res) + continue + } + + if err := hc.Delete(ctx, server, hash, auth); err != nil { + res.Error = describeError(err) + } else { + res.OK = true + } report.add(res) - continue } - - if err := hc.Delete(ctx, server, hash, auth); err != nil { - res.Error = describeError(err) - } else { - res.OK = true - } - report.add(res) - } + return nil + }) printFanoutReport(jsonMode, report) if !report.allSucceeded() { diff --git a/cli/blossom/servers.go b/cli/blossom/servers.go index 14daf44..4201b75 100644 --- a/cli/blossom/servers.go +++ b/cli/blossom/servers.go @@ -43,7 +43,7 @@ func requirePublishIdentity(cmd *cobra.Command, publish bool) error { return nil } if identity, _ := cmd.Flags().GetString("identity"); identity == "" { - return common.UsageError(cmd, fmt.Errorf("--identity is required with --publish")) + return common.InvocationError(cmd, fmt.Errorf("--identity is required with --publish")) } return nil } @@ -68,7 +68,12 @@ func publishServerList(ctx context.Context, cmd *cobra.Command, jsonMode bool, i if err != nil { return "", common.NotFoundError(cmd, "", err) } - report, err := client.PublishToTargets(ctx, targets, []*nip01.Event{event}) + var report *client.PublishReport + err = common.WithSpinner(cmd, "publishing server list", func() error { + var pErr error + report, pErr = client.PublishToTargets(ctx, targets, []*nip01.Event{event}) + return pErr + }) if err != nil { return "", common.NetworkError(cmd, "", err) } @@ -87,7 +92,7 @@ func newServersAddCommand() *cobra.Command { Example: ` ncli blossom servers add https://blossom.example.com`, Args: func(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return common.UsageError(cmd, fmt.Errorf("exactly one server url is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("exactly one server url is required")) } return requirePublishIdentity(cmd, publish) }, @@ -157,7 +162,7 @@ func newServersRemoveCommand() *cobra.Command { Example: ` ncli blossom servers remove https://blossom.example.com`, Args: func(cmd *cobra.Command, args []string) error { if len(args) != 1 { - return common.UsageError(cmd, fmt.Errorf("exactly one server url is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("exactly one server url is required")) } return requirePublishIdentity(cmd, publish) }, @@ -262,7 +267,7 @@ func newServersDiscoverCommand() *cobra.Command { cmd := &cobra.Command{ Use: "discover ", - Short: "Discover another identity's published Blossom server list (BUD-03)", + Short: "Discover another identity's published server list (BUD-03)", Long: `Resolves (vault label/nsec/npub/hex pubkey/nprofile/nip-05) to a pubkey, then queries your configured Nostr relays for that pubkey's most recent kind:10063 server-list event, and prints the servers it @@ -295,7 +300,12 @@ this looks up someone else's published servers.`, }) timeout, _ := cmd.Flags().GetDuration("timeout") - events, err := client.QueryTargets(ctx, targets, filters, timeout) + var events []*nip01.Event + err = common.WithSpinner(cmd, fmt.Sprintf("discovering server list for %s", resolved.Npub), func() error { + var qErr error + events, qErr = client.QueryTargets(ctx, targets, filters, timeout) + return qErr + }) if err != nil { if errors.Is(err, client.ErrNoReachableTargets) { return common.NetworkError(cmd, "", err) diff --git a/cli/blossom/shared.go b/cli/blossom/shared.go index 024fc02..20f7311 100644 --- a/cli/blossom/shared.go +++ b/cli/blossom/shared.go @@ -37,6 +37,15 @@ func newHTTPClient(timeout time.Duration) *bclient.Client { return &bclient.Client{HTTPClient: &http.Client{Timeout: timeout}} } +// shortHash abbreviates a sha256 for a progress message, where the full 64 +// characters would push everything else off the line. +func shortHash(h string) string { + if len(h) <= 12 { + return h + } + return h[:8] + "…" +} + // resolveServers returns the server list a subcommand should operate // against: explicit --server flags if any were given, else the configured // default list from prefs.yaml. diff --git a/cli/blossom/upload.go b/cli/blossom/upload.go index 474953e..a977d88 100644 --- a/cli/blossom/upload.go +++ b/cli/blossom/upload.go @@ -33,10 +33,10 @@ PUT /media) instead of a byte-for-byte store.`, ncli blossom upload ./photo.jpg --identity satoshi --server https://blossom.example.com`, Args: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { - return common.UsageError(cmd, fmt.Errorf("at least one file is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("at least one file is required")) } if identity, _ := cmd.Flags().GetString("identity"); identity == "" { - return common.UsageError(cmd, fmt.Errorf("--identity is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--identity is required")) } for _, path := range args { info, err := os.Stat(path) @@ -76,30 +76,33 @@ PUT /media) instead of a byte-for-byte store.`, hc := newHTTPClient(timeout) report := &fanoutReport{} - for _, path := range args { - contentType, err := detectContentType(path) - if err != nil { - for _, server := range servers { - report.add(serverResult{Item: path, Server: server, Error: err.Error()}) - } - continue - } - - for _, server := range servers { - // Signed fresh for every (file, server) pair rather - // than once for the whole batch: a large upload can - // take longer than one token's TTL, and a token - // signed at the start would then expire partway - // through instead of every request getting a full - // fresh window. - auth, err := buildAuth(privKeyHex, pubKeyHex, verb, nil, ttl) + _ = common.WithSpinner(cmd, fmt.Sprintf("uploading %d file(s) to %d server(s)", len(args), len(servers)), func() error { + for _, path := range args { + contentType, err := detectContentType(path) if err != nil { - report.add(serverResult{Item: path, Server: server, Error: err.Error()}) + for _, server := range servers { + report.add(serverResult{Item: path, Server: server, Error: err.Error()}) + } continue } - report.add(uploadOne(ctx, hc, server, path, contentType, auth, optimize)) + + for _, server := range servers { + // Signed fresh for every (file, server) pair rather + // than once for the whole batch: a large upload can + // take longer than one token's TTL, and a token + // signed at the start would then expire partway + // through instead of every request getting a full + // fresh window. + auth, err := buildAuth(privKeyHex, pubKeyHex, verb, nil, ttl) + if err != nil { + report.add(serverResult{Item: path, Server: server, Error: err.Error()}) + continue + } + report.add(uploadOne(ctx, hc, server, path, contentType, auth, optimize)) + } } - } + return nil + }) printFanoutReport(jsonMode, report) if !report.allSucceeded() { diff --git a/cli/bunker/command.go b/cli/bunker/command.go index 2252729..9e248ad 100644 --- a/cli/bunker/command.go +++ b/cli/bunker/command.go @@ -35,16 +35,15 @@ func NewBunkerCommand() *cobra.Command { cmd := &cobra.Command{ Use: "bunker", Short: "Run ncli as a NIP-46 remote signer", - Long: `Run ncli as a NIP-46 "bunker": listen on one or more relays for signing -requests from other Nostr clients, approve or reject them from a live -TUI, and remember per-app decisions so you aren't re-prompted every time. - -On Linux/macOS this starts (or reattaches to) a background daemon that -keeps running after the TUI is closed with "b" or "q" -- reattach any -time with "ncli bunker attach". On Windows the TUI runs directly with no -background support.`, + Long: `Run ncli as a NIP-46 "bunker": listen on relays for other clients' +signing requests, approve or reject them from a live TUI, and remember +per-app decisions so you aren't re-prompted every time. + +On Linux/macOS this leaves a background daemon running when the TUI +closes; reattach with "ncli bunker attach".`, Example: ` ncli bunker - ncli bunker --relay wss://relay.example.com`, + ncli bunker --identity satoshi + ncli bunker attach`, RunE: func(cmd *cobra.Command, args []string) error { if err := requireInteractive(cmd); err != nil { return err @@ -92,7 +91,7 @@ func requireInteractive(cmd *cobra.Command) error { func newAttachCommand() *cobra.Command { return &cobra.Command{ Use: "attach", - Short: "Reattach the TUI to an already-running background bunker daemon", + Short: "Reattach the TUI to a running bunker daemon", Long: `Reconnect the interactive TUI to a bunker daemon already started with "ncli bunker" and left running in the background. Never starts one itself -- fails if none is running (use "ncli bunker" for that).`, @@ -395,7 +394,7 @@ func newSessionsCommand() *cobra.Command { revokeGrantCmd := &cobra.Command{ Use: "revoke-grant ", - Short: "Revoke one remembered permission for an app, leaving the rest", + Short: "Revoke one remembered permission, leaving the rest", Example: ` ncli bunker sessions revoke-grant --method sign_event`, Args: common.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/cli/bunker/identity.go b/cli/bunker/identity.go index 3a76f43..57f4b8e 100644 --- a/cli/bunker/identity.go +++ b/cli/bunker/identity.go @@ -39,11 +39,11 @@ func ResolveSignerKey(cmd *cobra.Command, jsonMode bool, identityFlag string) (p } switch len(entries) { case 0: - return "", "", "", common.UsageError(cmd, fmt.Errorf("--identity is required (or set NCLI_BUNKER_IDENTITY/bunker.identity): no vault identity to fall back to")) + return "", "", "", common.InvocationError(cmd, fmt.Errorf("--identity is required (or set NCLI_BUNKER_IDENTITY/bunker.identity): no vault identity to fall back to")) case 1: identity = entries[0].Label default: - return "", "", "", common.UsageError(cmd, fmt.Errorf("--identity is required (or set NCLI_BUNKER_IDENTITY/bunker.identity): the vault has %d saved identities, none chosen by default", len(entries))) + return "", "", "", common.InvocationError(cmd, fmt.Errorf("--identity is required (or set NCLI_BUNKER_IDENTITY/bunker.identity): the vault has %d saved identities, none chosen by default", len(entries))) } } diff --git a/cli/common/args.go b/cli/common/args.go index d42d078..50fbe9c 100644 --- a/cli/common/args.go +++ b/cli/common/args.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/spf13/cobra" + "github.com/spf13/pflag" ) // silence marks cmd's error as already reported, so cobra doesn't also @@ -14,23 +15,83 @@ func silence(cmd *cobra.Command) { cmd.SilenceErrors = true } -// UsageError marks cmd as badly called -- a missing/conflicting flag, wrong -// arg count, or similar invocation-shape mistake. It prints cmd's own help -// immediately in text mode (so the caller sees correct usage without a -// separate --help run); skipped in --json mode, where cmd.Help()'s default -// stdout destination would otherwise pollute the stream a script expects to -// hold nothing but clean data. Returns err unchanged so callers can -// propagate it from an Args validator -- which cobra runs before -// PersistentPreRun, so a bad invocation is rejected before config loading -// or logging setup ever runs. +// UsageError marks err as carrying the usage code without asking for a help +// dump -- a refusal that happens to be the caller's fault but that usage text +// wouldn't explain (a relay answering 501 because membership is switched off, +// say). For an actual mechanical mis-invocation, reach for InvocationError or +// HelpError instead, which are the same classification plus help. +// +// Returns err unchanged so callers can propagate it from an Args validator -- +// which cobra runs before PersistentPreRun, so a bad invocation is rejected +// before config loading or logging setup ever runs. Nothing is printed here; +// EmitError is the single place any of this is rendered. func UsageError(cmd *cobra.Command, err error) error { silence(cmd) - if jsonMode, _ := cmd.Flags().GetBool("json"); !jsonMode { - _ = cmd.Help() - } return wrapCLIError(CodeUsage, "", err) } +// InvocationError is UsageError for a mistake that is unambiguously +// mechanical -- a wrong argument count, an unknown flag or subcommand, a +// missing required flag, two conflicting ways of supplying the same value. +// The caller supplied *something* and it was wrong, so the report leads with +// the error and follows it with help. +func InvocationError(cmd *cobra.Command, err error) error { + return withHelp(UsageError(cmd, err), HelpAfterError) +} + +// HelpError is UsageError for a command invoked with nothing of its own -- +// bare "ncli decode", bare "ncli miner". There is no mistake worth narrating, +// so it prints help alone, with no "Error:" line. The usage code, the exit +// status (2) and the --json structured error are all unchanged: an agent that +// mistypes a subcommand still sees a failure, it just isn't shouted at. +func HelpError(cmd *cobra.Command, err error) error { + return withHelp(UsageError(cmd, err), HelpOnly) +} + +// InvocationOrHelp is the shape most Args validators want: the same check +// fires both when the caller supplied nothing ("ncli find") and when they +// supplied the wrong thing ("ncli find a b"), and only the second deserves to +// be called a mistake. Pass the validator's own args; it picks HelpError or +// InvocationError accordingly. +func InvocationOrHelp(cmd *cobra.Command, args []string, err error) error { + if IsBareInvocation(cmd, args) { + return HelpError(cmd, err) + } + return InvocationError(cmd, err) +} + +// withHelp tags a freshly-classified usage error with mode. Guarded on +// CodeUsage so an already-classified deeper error -- a not_found surfacing +// through an outer invocation check, say -- keeps its own classification and +// doesn't retroactively grow a help dump it never asked for. +func withHelp(err error, mode HelpMode) error { + if ce, ok := err.(*CLIError); ok && ce.Code == CodeUsage { + ce.Help = mode + } + return err +} + +// IsBareInvocation reports whether cmd was called with nothing of its own: no +// positional arguments and no command-specific flag set. Inherited flags +// (--json, --quiet, --config) don't count, so "ncli decode --json" is still a +// bare decode and still answers with help rather than a scolding. +func IsBareInvocation(cmd *cobra.Command, args []string) bool { + if len(args) > 0 { + return false + } + // VisitAll + Changed, not Visit: LocalNonPersistentFlags builds a fresh + // FlagSet, and pflag tracks "was set" per FlagSet, so Visit on the copy + // sees nothing at all -- which would make every flags-only invocation + // look bare. + bare := true + cmd.LocalNonPersistentFlags().VisitAll(func(f *pflag.Flag) { + if f.Changed { + bare = false + } + }) + return bare +} + // InvalidInputError marks err as caused by a supplied value that failed // validation or parsing (a malformed identifier, relay URL, duration, // private key, ...) -- distinct from UsageError (the invocation's shape, @@ -103,19 +164,27 @@ func UnsupportedError(cmd *cobra.Command, input string, err error) error { // full help dump printed on top, a contract violation even under --json // (see AGENTS.md's error table and followup issue #1). Use these in place // of the cobra.* equivalents on any command's Args field. +// A command that wants n args and was handed nothing at all hasn't made a +// mistake worth narrating -- it just hasn't been told what to do yet -- so it +// answers with HelpError. Anything else supplied the wrong number on purpose +// and gets InvocationError's "Error: ..." line above the same help. func ExactArgs(n int) cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { - if len(args) != n { - return UsageError(cmd, fmt.Errorf("accepts %d arg(s), received %d", n, len(args))) + if len(args) == n { + return nil } - return nil + err := fmt.Errorf("accepts %d arg(s), received %d", n, len(args)) + if n > 0 && IsBareInvocation(cmd, args) { + return HelpError(cmd, err) + } + return InvocationError(cmd, err) } } func MaximumNArgs(n int) cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { if len(args) > n { - return UsageError(cmd, fmt.Errorf("accepts at most %d arg(s), received %d", n, len(args))) + return InvocationError(cmd, fmt.Errorf("accepts at most %d arg(s), received %d", n, len(args))) } return nil } @@ -123,16 +192,20 @@ func MaximumNArgs(n int) cobra.PositionalArgs { func MinimumNArgs(n int) cobra.PositionalArgs { return func(cmd *cobra.Command, args []string) error { - if len(args) < n { - return UsageError(cmd, fmt.Errorf("requires at least %d arg(s), only received %d", n, len(args))) + if len(args) >= n { + return nil } - return nil + err := fmt.Errorf("requires at least %d arg(s), only received %d", n, len(args)) + if n > 0 && IsBareInvocation(cmd, args) { + return HelpError(cmd, err) + } + return InvocationError(cmd, err) } } func NoArgs(cmd *cobra.Command, args []string) error { if len(args) > 0 { - return UsageError(cmd, fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath())) + return InvocationError(cmd, fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath())) } return nil } @@ -144,14 +217,18 @@ func NoArgs(cmd *cobra.Command, args []string) error { // child, whether that's a bare "ncli relay members" or a typo'd "ncli // miner mnie" -- silently treating a missing/misspelled subcommand as // success, on stdout, even in --json mode. Wiring this as the group's own -// RunE routes that same situation through UsageError instead, so it gets -// exit 2, stderr-only reporting, and a structured error under --json like -// every other invocation mistake. +// RunE routes that same situation through the usage classifiers instead, so +// it gets exit 2, stderr-only reporting, and a structured error under --json +// like every other invocation mistake. +// +// A bare group still prints help -- that's the friendly answer to "ncli +// miner" -- but it exits 2 rather than 0, so a misspelled subcommand is never +// mistaken for success. A misspelled one additionally names what went wrong. func RequireSubcommand(cmd *cobra.Command, args []string) error { if len(args) > 0 { - return UsageError(cmd, fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath())) + return InvocationError(cmd, fmt.Errorf("unknown command %q for %q", args[0], cmd.CommandPath())) } - return UsageError(cmd, fmt.Errorf("%q requires a subcommand", cmd.CommandPath())) + return HelpError(cmd, fmt.Errorf("%q requires a subcommand", cmd.CommandPath())) } // RuntimeError is the fallback bucket for a failure that doesn't cleanly diff --git a/cli/common/errors.go b/cli/common/errors.go index 63902c6..4511f45 100644 --- a/cli/common/errors.go +++ b/cli/common/errors.go @@ -6,8 +6,8 @@ import ( "os" "strings" - "github.com/rs/zerolog/log" "github.com/spf13/cobra" + "golang.org/x/term" ) // ErrorCode classifies a CLIError so an agent can branch on failure kind @@ -58,10 +58,29 @@ var retryableCodes = map[ErrorCode]bool{ // Input is left blank when there's no one clean value to echo, or when the // value is sensitive (a private key, a vault password) and must not be // echoed back at all. +// HelpMode says how much of a command's own help EmitError prints alongside +// a failure. The zero value is HelpNone, so an error that was never +// deliberately classified can't accidentally dump thirty lines of help. +type HelpMode uint8 + +const ( + // HelpNone prints the error line alone -- the invocation was fine, the + // operation failed, and repeating usage wouldn't tell the caller anything. + HelpNone HelpMode = iota + // HelpOnly prints help and no error line: nothing was supplied, so there's + // no mistake to narrate, just a command that needs to be told what to do. + // The exit code and the --json structured error are unaffected. + HelpOnly + // HelpAfterError prints the error line, a blank line, then help -- + // something *was* supplied and it was wrong. + HelpAfterError +) + type CLIError struct { Err error Code ErrorCode Input string + Help HelpMode } func (e *CLIError) Error() string { return e.Err.Error() } @@ -129,7 +148,52 @@ func EmitError(cmd *cobra.Command, err error) { return } - log.Error().Msg(err.Error()) + // The file log still gets the failure as an ordinary record, exactly as it + // did when this printed through zerolog -- only the human-facing console + // line changes shape. + LogFailureToFile(err.Error()) + + mode := HelpNone + if ce, ok := err.(*CLIError); ok { + mode = ce.Help + } + + if mode != HelpOnly { + fmt.Fprintf(os.Stderr, "%s %s\n", errorPrefix(isColorTerminal(os.Stderr)), err.Error()) + } + if mode == HelpNone || cmd == nil { + return + } + if mode == HelpAfterError { + fmt.Fprintln(os.Stderr) + } + // Reuse cobra's own help template rather than hand-rolling one, pointed at + // stderr: stdout carries a command's result and nothing else. A plain + // `--help` run (exit 0, not a failure) never comes through here, so it + // still prints to stdout as usual. + cmd.SetOut(os.Stderr) + cmd.SetErr(os.Stderr) + _ = cmd.Help() +} + +// errorPrefix returns "Error:", in ANSI red when colored -- split out from +// EmitError so the wrapping stays testable without a real terminal. +func errorPrefix(colored bool) string { + if !colored { + return "Error:" + } + return "\x1b[31mError:\x1b[0m" +} + +// isColorTerminal reports whether w is a real terminal that should receive +// ANSI color -- false when output is piped, redirected, or NO_COLOR is set +// (https://no-color.org), so anything capturing ncli's stderr gets plain text +// rather than escape codes mixed into what it parses. +func isColorTerminal(w *os.File) bool { + if os.Getenv("NO_COLOR") != "" { + return false + } + return term.IsTerminal(int(w.Fd())) } // ExitCode picks the process exit code for err from its ErrorCode (see diff --git a/cli/common/errors_render_test.go b/cli/common/errors_render_test.go new file mode 100644 index 0000000..066e9a4 --- /dev/null +++ b/cli/common/errors_render_test.go @@ -0,0 +1,198 @@ +package common + +import ( + "errors" + "io" + "os" + "strings" + "sync" + "testing" + + "github.com/spf13/cobra" +) + +func TestErrorPrefix(t *testing.T) { + if got := errorPrefix(false); got != "Error:" { + t.Errorf("errorPrefix(false) = %q, want plain %q", got, "Error:") + } + got := errorPrefix(true) + if !strings.Contains(got, "Error:") { + t.Errorf("errorPrefix(true) = %q, want it to still contain the word", got) + } + if !strings.HasPrefix(got, "\x1b[31m") || !strings.HasSuffix(got, "\x1b[0m") { + t.Errorf("errorPrefix(true) = %q, want it wrapped in red ANSI", got) + } +} + +// TestIsColorTerminal_NoColor pins the https://no-color.org contract: even +// on a real terminal, NO_COLOR turns color off. +func TestIsColorTerminal_NoColor(t *testing.T) { + t.Setenv("NO_COLOR", "1") + if isColorTerminal(os.Stdout) { + t.Error("isColorTerminal = true with NO_COLOR set, want false") + } +} + +func newRenderTestCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "widget ", + Short: "Do a widget thing", + Example: ` ncli widget thing`, + RunE: func(*cobra.Command, []string) error { return nil }, + } + cmd.Flags().String("size", "", "how big") + cmd.Flags().Bool("json", false, "json output") + return cmd +} + +// captureStd runs fn with both standard streams replaced by pipes, so a test +// can assert on exactly what a user would have seen -- and, just as +// importantly, that stdout stayed empty. +func captureStd(t *testing.T, fn func()) (stdout, stderr string) { + t.Helper() + origOut, origErr := os.Stdout, os.Stderr + outR, outW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + errR, errW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + os.Stdout, os.Stderr = outW, errW + + var wg sync.WaitGroup + var outBytes, errBytes []byte + wg.Add(2) + go func() { defer wg.Done(); outBytes, _ = io.ReadAll(outR) }() + go func() { defer wg.Done(); errBytes, _ = io.ReadAll(errR) }() + + fn() + + _ = outW.Close() + _ = errW.Close() + wg.Wait() + os.Stdout, os.Stderr = origOut, origErr + return string(outBytes), string(errBytes) +} + +// TestEmitError_HelpModes is the whole point of the cashctl alignment: three +// distinct shapes, all on stderr, stdout untouched in every one. +func TestEmitError_HelpModes(t *testing.T) { + cases := []struct { + name string + mode HelpMode + wantErrLine bool + wantHelp bool + }{ + {"runtime failure prints the error alone", HelpNone, true, false}, + {"bare invocation prints help alone", HelpOnly, false, true}, + {"wrong invocation prints both", HelpAfterError, true, true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cmd := newRenderTestCmd() + err := &CLIError{Err: errors.New("boom"), Code: CodeUsage, Help: tc.mode} + + stdout, stderr := captureStd(t, func() { EmitError(cmd, err) }) + + if stdout != "" { + t.Errorf("stdout = %q, want empty -- a failure must never touch the result stream", stdout) + } + if got := strings.Contains(stderr, "Error: boom"); got != tc.wantErrLine { + t.Errorf("stderr has error line = %v, want %v\nstderr:\n%s", got, tc.wantErrLine, stderr) + } + if got := strings.Contains(stderr, "Usage:"); got != tc.wantHelp { + t.Errorf("stderr has help = %v, want %v\nstderr:\n%s", got, tc.wantHelp, stderr) + } + if tc.wantErrLine && tc.wantHelp { + if strings.Index(stderr, "Error: boom") > strings.Index(stderr, "Usage:") { + t.Errorf("error line must come before help, got:\n%s", stderr) + } + } + }) + } +} + +// TestEmitError_JSONSkipsHelp guards the agent-facing contract: --json gets +// one structured line and never a help dump, whatever the HelpMode says. +func TestEmitError_JSONSkipsHelp(t *testing.T) { + cmd := newRenderTestCmd() + _ = cmd.Flags().Set("json", "true") + err := &CLIError{Err: errors.New("boom"), Code: CodeUsage, Help: HelpAfterError} + + stdout, stderr := captureStd(t, func() { EmitError(cmd, err) }) + + if stdout != "" { + t.Errorf("stdout = %q, want empty", stdout) + } + if strings.Contains(stderr, "Usage:") { + t.Errorf("--json must not dump help, got:\n%s", stderr) + } + if lines := strings.Split(strings.TrimRight(stderr, "\n"), "\n"); len(lines) != 1 { + t.Errorf("stderr = %d lines, want exactly 1 JSON line:\n%s", len(lines), stderr) + } + if !strings.Contains(stderr, `"code":"usage"`) { + t.Errorf("stderr missing structured code, got: %s", stderr) + } +} + +// TestInvocationClassifiers checks the three constructors agree on code and +// exit status while differing only in how much help they ask for. +func TestInvocationClassifiers(t *testing.T) { + cases := []struct { + name string + make func(*cobra.Command) error + want HelpMode + }{ + {"UsageError", func(c *cobra.Command) error { return UsageError(c, errors.New("x")) }, HelpNone}, + {"HelpError", func(c *cobra.Command) error { return HelpError(c, errors.New("x")) }, HelpOnly}, + {"InvocationError", func(c *cobra.Command) error { return InvocationError(c, errors.New("x")) }, HelpAfterError}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + cmd := newRenderTestCmd() + err := tc.make(cmd) + + ce, ok := err.(*CLIError) + if !ok { + t.Fatalf("got %T, want *CLIError", err) + } + if ce.Code != CodeUsage { + t.Errorf("Code = %q, want %q", ce.Code, CodeUsage) + } + if ce.Help != tc.want { + t.Errorf("Help = %d, want %d", ce.Help, tc.want) + } + if ExitCode(err) != 2 { + t.Errorf("ExitCode = %d, want 2", ExitCode(err)) + } + if !cmd.SilenceUsage || !cmd.SilenceErrors { + t.Error("cobra's own reporting must be silenced") + } + }) + } +} + +// TestIsBareInvocation: inherited flags don't make an invocation non-bare, +// which is what lets `ncli decode --json` still answer with help. +func TestIsBareInvocation(t *testing.T) { + t.Run("no args, no flags", func(t *testing.T) { + if !IsBareInvocation(newRenderTestCmd(), nil) { + t.Error("want bare") + } + }) + t.Run("positional arg makes it non-bare", func(t *testing.T) { + if IsBareInvocation(newRenderTestCmd(), []string{"x"}) { + t.Error("want non-bare") + } + }) + t.Run("command-specific flag makes it non-bare", func(t *testing.T) { + cmd := newRenderTestCmd() + _ = cmd.Flags().Set("size", "big") + if IsBareInvocation(cmd, nil) { + t.Error("want non-bare once --size was set") + } + }) +} diff --git a/cli/common/logging.go b/cli/common/logging.go index 066861c..2cf913b 100644 --- a/cli/common/logging.go +++ b/cli/common/logging.go @@ -56,6 +56,13 @@ func WithJSON(enabled bool) LoggingOption { // without callers having to remember their own configuration. var current loggingConfig +// fileOnly logs to the configured file writer alone, bypassing the console. +// EmitError needs it because a failure has to land in ncli.log the way every +// other log record does, while its human-facing line is printed separately in +// cashctl's "Error: ..." shape rather than zerolog's timestamped console one. +// Nil until ConfigureLogging is called with WithFileWriter. +var fileOnly *zerolog.Logger + // ConfigureLogging points the process-global zerolog logger at the writers // selected by opts. This is the one place ncli's CLI entrypoints (root, // apply, relay, reindex) set up logging, instead of each hand-rolling its @@ -117,9 +124,13 @@ func RedirectStderrToCrashLog(path string) (restore func(), err error) { func apply(cfg loggingConfig) { var writers []io.Writer + // Console records go through spinnerSafeWriter so that, while a spinner is + // animating, each log line wipes the in-flight frame and prints on a clean + // line -- the narration and the spinner coexist instead of smearing. + console := io.Writer(spinnerSafeWriter{os.Stderr}) if cfg.console { if cfg.jsonOutput { - writers = append(writers, os.Stderr) + writers = append(writers, console) } else { // NoColor is keyed off whether stderr is an actual terminal, not // --json (handled above) -- otherwise a redirected/piped/ @@ -129,14 +140,18 @@ func apply(cfg loggingConfig) { // term.IsTerminal check apply's/ping's own TUI-vs-headless // decision already uses (client/client.go, client/ping.go). writers = append(writers, zerolog.ConsoleWriter{ - Out: os.Stderr, + Out: console, TimeFormat: time.RFC3339, NoColor: !term.IsTerminal(int(os.Stderr.Fd())), }) } } + fileOnly = nil if cfg.fileWriter != nil { - writers = append(writers, zerolog.ConsoleWriter{Out: cfg.fileWriter, TimeFormat: time.RFC3339, NoColor: true}) + fw := zerolog.ConsoleWriter{Out: cfg.fileWriter, TimeFormat: time.RFC3339, NoColor: true} + writers = append(writers, fw) + l := zerolog.New(fw).With().Timestamp().Logger() + fileOnly = &l } if len(writers) == 0 { return @@ -148,3 +163,12 @@ func apply(cfg loggingConfig) { } log.Logger = zerolog.New(w).With().Timestamp().Logger() } + +// LogFailureToFile records msg at error level in the log file only, printing +// nothing to the console. A no-op when no file writer is configured. +func LogFailureToFile(msg string) { + if fileOnly == nil { + return + } + fileOnly.Error().Msg(msg) +} diff --git a/cli/common/spinner.go b/cli/common/spinner.go new file mode 100644 index 0000000..d584c52 --- /dev/null +++ b/cli/common/spinner.go @@ -0,0 +1,138 @@ +package common + +import ( + "fmt" + "io" + "os" + "sync" + "time" + + "github.com/spf13/cobra" +) + +// eraseLine returns the cursor to column 0 and clears to end of line -- how +// an in-flight spinner frame is wiped before anything else prints. +const eraseLine = "\r\x1b[K" + +// spinnerFrames animates WithSpinner's "waiting on the network" indicator. +var spinnerFrames = []string{"⠋", "⠙", "⠹", "⠸", "⠼", "⠴", "⠦", "⠧", "⠇", "⠏"} + +// spinnerState is the process-wide handle on whichever spinner is currently +// drawing. Every write to the spinner's stream goes through withStderrLock, +// including zerolog's console output (see spinnerSafeWriter), so a log line +// arriving mid-spin erases the frame, prints on a clean line, and lets the +// next tick redraw underneath it -- instead of the two interleaving into an +// unreadable smear. +var spinnerState struct { + mu sync.Mutex + active bool + out io.Writer +} + +// withStderrLock runs fn with exclusive access to the spinner's line, wiping +// an in-flight frame first so fn starts on a clean one. +func withStderrLock(fn func()) { + spinnerState.mu.Lock() + defer spinnerState.mu.Unlock() + if spinnerState.active && spinnerState.out != nil { + fmt.Fprint(spinnerState.out, eraseLine) + } + fn() +} + +// spinnerSafeWriter serializes a log writer against the spinner. Wrapping +// zerolog's console destination in one is what keeps ncli's existing +// per-relay narration ("querying relay.example.com") visible while a spinner +// runs, rather than having to suppress it for the spinner's benefit. +type spinnerSafeWriter struct{ w io.Writer } + +func (s spinnerSafeWriter) Write(p []byte) (n int, err error) { + withStderrLock(func() { n, err = s.w.Write(p) }) + return n, err +} + +// WithSpinner runs fn while animating message on stderr -- the shared +// "waiting on a network call" indicator, so every command that blocks on one +// looks the same instead of each inventing its own. +// +// It animates only when stderr is a real terminal and the caller isn't a +// script: --json and -q/--quiet both switch it off, as does NO_COLOR or a +// redirected/piped stderr (via isColorTerminal), because the \r-and-erase +// frames are just noise in a log file. Commands that hand the terminal to a +// TUI must not call this at all. +func WithSpinner(cmd *cobra.Command, message string, fn func() error) error { + return withSpinner(os.Stderr, SpinnerEnabled(cmd), message, fn) +} + +// SpinnerEnabled reports whether cmd should animate. Exported so a command +// that renders its own progress can make the same decision the same way. +func SpinnerEnabled(cmd *cobra.Command) bool { + if cmd != nil { + if jsonMode, _ := cmd.Flags().GetBool("json"); jsonMode { + return false + } + if quiet, _ := cmd.Flags().GetBool("quiet"); quiet { + return false + } + } + return isColorTerminal(os.Stderr) +} + +// withSpinner is WithSpinner with its output and on/off decision injected, so +// the animation's own behaviour (frames written, goroutine stopped before +// return) stays unit-testable without a real terminal. +func withSpinner(w io.Writer, animate bool, message string, fn func() error) error { + // Never stack two spinners: an outer one is already drawing, and a second + // would fight it for the same line. + spinnerState.mu.Lock() + busy := spinnerState.active + spinnerState.mu.Unlock() + if !animate || busy { + return fn() + } + + spinnerState.mu.Lock() + spinnerState.active = true + spinnerState.out = w + spinnerState.mu.Unlock() + + stop := make(chan struct{}) + stopped := make(chan struct{}) + go func() { + defer close(stopped) + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + frame := 0 + draw := func() { + withStderrLock(func() { + fmt.Fprintf(w, "\r%s %s", spinnerFrames[frame%len(spinnerFrames)], message) + }) + } + draw() + for { + select { + case <-stop: + return + case <-ticker.C: + frame++ + draw() + } + } + }() + + err := fn() + + // Stop the goroutine *and wait for it* before clearing, so its last frame + // can never land after the line is wiped -- otherwise a stray frame would + // sit above whatever the caller (or EmitError) prints next. + close(stop) + <-stopped + + spinnerState.mu.Lock() + spinnerState.active = false + spinnerState.out = nil + fmt.Fprint(w, eraseLine) + spinnerState.mu.Unlock() + + return err +} diff --git a/cli/common/spinner_test.go b/cli/common/spinner_test.go new file mode 100644 index 0000000..cb8dfd9 --- /dev/null +++ b/cli/common/spinner_test.go @@ -0,0 +1,126 @@ +package common + +import ( + "bytes" + "strings" + "sync" + "testing" + "time" +) + +// syncBuffer is a bytes.Buffer safe to read while the spinner goroutine is +// still writing to it. +type syncBuffer struct { + mu sync.Mutex + b bytes.Buffer +} + +func (s *syncBuffer) Write(p []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.b.Write(p) +} + +func (s *syncBuffer) String() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.b.String() +} + +// TestWithSpinner_DisabledWritesNothing is the contract that keeps --json, +// -q and a piped stderr byte-identical to before: no frames, no escape +// codes, nothing at all. +func TestWithSpinner_DisabledWritesNothing(t *testing.T) { + var buf syncBuffer + ran := false + + err := withSpinner(&buf, false, "working", func() error { + ran = true + return nil + }) + + if err != nil { + t.Fatalf("err = %v, want nil", err) + } + if !ran { + t.Error("fn must still run when the animation is off") + } + if got := buf.String(); got != "" { + t.Errorf("wrote %q, want nothing when disabled", got) + } +} + +func TestWithSpinner_AnimatesAndClears(t *testing.T) { + var buf syncBuffer + + err := withSpinner(&buf, true, "working", func() error { + // Long enough for at least one tick past the initial frame. + time.Sleep(250 * time.Millisecond) + return nil + }) + if err != nil { + t.Fatalf("err = %v, want nil", err) + } + + got := buf.String() + if !strings.Contains(got, "working") { + t.Errorf("output missing the message, got %q", got) + } + if !strings.Contains(got, spinnerFrames[0]) { + t.Errorf("output missing the first frame, got %q", got) + } + if !strings.HasSuffix(got, eraseLine) { + t.Errorf("output must end by erasing the spinner line, got %q", got) + } +} + +// TestWithSpinner_StoppedBeforeReturn is what keeps a stray frame from +// landing on top of whatever EmitError prints next: once withSpinner +// returns, its goroutine is joined, so nothing more can arrive. +func TestWithSpinner_StoppedBeforeReturn(t *testing.T) { + var buf syncBuffer + + _ = withSpinner(&buf, true, "working", func() error { + time.Sleep(150 * time.Millisecond) + return nil + }) + + settled := buf.String() + time.Sleep(250 * time.Millisecond) // several ticks' worth + if after := buf.String(); after != settled { + t.Errorf("spinner wrote %d more bytes after returning; goroutine outlived the call", + len(after)-len(settled)) + } +} + +// TestWithSpinner_PropagatesError confirms the wrapper is transparent: the +// spinner must never swallow or replace what the work returned. +func TestWithSpinner_PropagatesError(t *testing.T) { + var buf syncBuffer + want := errSentinel{} + + if got := withSpinner(&buf, true, "working", func() error { return want }); got != error(want) { + t.Errorf("err = %v, want the original %v", got, want) + } +} + +type errSentinel struct{} + +func (errSentinel) Error() string { return "sentinel" } + +// TestWithSpinner_NoNesting: an inner spinner must not fight the outer one +// for the same line. +func TestWithSpinner_NoNesting(t *testing.T) { + var outer, inner syncBuffer + + _ = withSpinner(&outer, true, "outer", func() error { + return withSpinner(&inner, true, "inner", func() error { + time.Sleep(120 * time.Millisecond) + return nil + }) + }) + + if got := inner.String(); got != "" { + t.Errorf("inner spinner wrote %q, want nothing while an outer one is drawing", got) + } +} diff --git a/cli/delegate/command.go b/cli/delegate/command.go index a444792..f2fe26f 100644 --- a/cli/delegate/command.go +++ b/cli/delegate/command.go @@ -58,7 +58,7 @@ and is rejected.`, jsonMode, _ := cmd.Flags().GetBool("json") interactive := term.IsTerminal(int(os.Stdin.Fd())) && term.IsTerminal(int(os.Stdout.Fd())) if jsonMode || !interactive { - return common.UsageError(cmd, errors.New("--issuer is required (or set NCLI_DELEGATE_ISSUER) when not running interactively")) + return common.InvocationError(cmd, errors.New("--issuer is required (or set NCLI_DELEGATE_ISSUER) when not running interactively")) } if err := RunWizard(); err != nil { return common.RuntimeError(cmd, err) @@ -84,7 +84,7 @@ func runNonInteractive(cmd *cobra.Command, issuer string) error { delegatee, _ := cmd.Flags().GetString("delegatee") if delegatee == "" { - return common.UsageError(cmd, errors.New("--delegatee is required")) + return common.InvocationError(cmd, errors.New("--delegatee is required")) } issuerPrivKeyHex, issuerPubHex, err := resolveDelegationKey(cmd, jsonMode, issuer) diff --git a/cli/ncli/apply.go b/cli/ncli/apply.go index e6c28ab..55a7dbe 100644 --- a/cli/ncli/apply.go +++ b/cli/ncli/apply.go @@ -20,7 +20,7 @@ var ( ncli apply -f sync.yaml --strict-pow`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } return nil }, diff --git a/cli/ncli/decode.go b/cli/ncli/decode.go index 208a3e4..646847e 100644 --- a/cli/ncli/decode.go +++ b/cli/ncli/decode.go @@ -11,21 +11,15 @@ import ( var decodeCmd = &cobra.Command{ Use: "decode ", - Short: "Decode a NIP-19 entity, cash token, or circlehub1... connection", - Long: `Decodes whichever bech32 shape you paste in: + Short: "Decode a NIP-19 entity, cash token, or hub connection", + Long: `Decodes whichever bech32 shape you paste in -- a NIP-19 entity (npub, +nsec, note, nprofile, nevent, naddr), a NIP-CASH cash token, or a NIP-CW +circlehub1... connection -- into its hex keys, relay hints and metadata. - - a NIP-19 entity -- npub, nsec, note, nprofile, nevent, or naddr -- - into its hex key/ID plus any embedded relay hints, author, or kind - - a NIP-CASH cash-token-family string (lokicash1..., satscash1..., ...) - into its wallet pubkey, relay hints, and optional identity-required/ - mint-provenance fields - - a NIP-CW circlehub1... Circle Hub connection into its wallet pubkey, - relay hints, and optional label - -A pairing secret is never included in the output, for either of the two -connection shapes. --json switches to structured JSON output on stdout.`, +A pairing secret is never included in the output.`, Example: ` ncli decode npub1... - ncli decode nevent1...`, + ncli decode nevent1... + ncli decode npub1... --json`, Args: common.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") diff --git a/cli/ncli/dump.go b/cli/ncli/dump.go index 6684923..cd70d1e 100644 --- a/cli/ncli/dump.go +++ b/cli/ncli/dump.go @@ -21,24 +21,25 @@ var dumpCmd = &cobra.Command{ Long: `Export events matching a filter to a JSON file, merged and deduplicated by event ID across every target. -Targets and filters come from --targets (a YAML file), or --relays plus -inline filter flags -- pick one, not both. Omitting both falls back to -the relays configured via "ncli prefs relays add".`, +Targets and filters come from --targets, or --relays plus inline filter +flags -- pick one, not both. Omitting both falls back to "ncli prefs +relays".`, Example: ` ncli dump -o events.json - ncli dump -t targets.yaml -o events.json`, + ncli dump -t targets.yaml -o events.json + ncli dump -s wss://relay.example.com -k 1 --since 24h -o recent.json`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if err := queryMutualExclusionCheck(cmd); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if _, err := validateArgFile(cmd, "out", false, ".json", ".jsonp"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if cmd.Flags().Changed("targets") { if _, err := validateArgFile(cmd, "targets", true, ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } return nil @@ -63,7 +64,10 @@ the relays configured via "ncli prefs relays add".`, return common.NetworkError(cmd, "", err) } - if err := client.DumpFromTargets(ctx, targetsSpec, outFile, filtersSpec, timeout); err != nil { + err = common.WithSpinner(cmd, targetsMessage("exporting from", targetsSpec), func() error { + return client.DumpFromTargets(ctx, targetsSpec, outFile, filtersSpec, timeout) + }) + if err != nil { if errors.Is(err, client.ErrNoReachableTargets) { return common.NetworkError(cmd, "", err) } diff --git a/cli/ncli/find.go b/cli/ncli/find.go index 256e4f5..d4512eb 100644 --- a/cli/ncli/find.go +++ b/cli/ncli/find.go @@ -18,45 +18,37 @@ var ( findCmd = &cobra.Command{ Use: "find [identifier]", Short: "Query events by ID and/or filter", - Long: `Look up events by ID and/or filter across one or more relays or local -stores, stopping at the first target with a match. + Long: `Look up events by ID and/or filter across relays or local stores, +stopping at the first target with a match. An npub or nip-05 identifier +defaults to that author's profile (kind 0); pass --kinds to widen it. -identifier is a positional argument: - - event: a hex event ID, or note1.../nevent1... - - author: npub1.../nprofile1..., or a nip-05 address -- ANDed with any - other filters given. With no other filters, defaults to just their - profile (kind 0); pass --kinds to widen it. - -Targets and filters come from --targets (a YAML file), or --relays plus -inline filter flags -- pick one, not both. Omitting both falls back to -the relays configured via "ncli prefs relays add". - -Always prints a single JSON array to stdout. --quiet also drops the -progress narration on stderr.`, +Targets and filters come from --targets, or --relays plus inline filter +flags -- pick one, not both. Omitting both falls back to "ncli prefs +relays". Always prints a single JSON array to stdout.`, Example: ` ncli find note1... ncli find npub1... - ncli find --authors npub1... -s wss://relay.example.com`, + ncli find --authors npub1... --kinds 1 -s wss://relay.example.com`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if err := queryMutualExclusionCheck(cmd); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if len(args) > 1 { - return common.UsageError(cmd, fmt.Errorf("at most one identifier argument is allowed")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("at most one identifier argument is allowed")) } if !cmd.Flags().Changed("targets") && len(args) == 0 && !inlineFilterFlagsChanged(cmd) { - return common.UsageError(cmd, fmt.Errorf("at least one of an identifier argument, an inline filter flag, or --targets is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("at least one of an identifier argument, an inline filter flag, or --targets is required")) } if cmd.Flags().Changed("targets") { if _, err := validateArgFile(cmd, "targets", true, ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } if cmd.Flags().Changed("out") { if _, err := validateArgFile(cmd, "out", false, ".json", ".jsonp"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } return nil @@ -95,7 +87,10 @@ progress narration on stderr.`, } } - if err := client.Find(ctx, idFilter, filtersSpec, targetsSpec, outPath, timeout); err != nil { + err = common.WithSpinner(cmd, targetsMessage("querying", targetsSpec), func() error { + return client.Find(ctx, idFilter, filtersSpec, targetsSpec, outPath, timeout) + }) + if err != nil { if errors.Is(err, client.ErrNoReachableTargets) { return common.NetworkError(cmd, "", err) } diff --git a/cli/ncli/id.go b/cli/ncli/id.go index 93b4dc3..58c752c 100644 --- a/cli/ncli/id.go +++ b/cli/ncli/id.go @@ -24,10 +24,11 @@ var idCmd = &cobra.Command{ a vault label, npub, hex pubkey, nsec, nprofile, or nip-05 address -- resolves and displays it instead. ---json disables interactive prompts: saves only with --save, labels only -from --label, and reads the vault password from NCLI_VAULT_PASSWORD.`, +--json disables interactive prompts and reads the vault password from +NCLI_VAULT_PASSWORD.`, Example: ` ncli id - ncli id satoshi`, + ncli id satoshi + ncli id --save --label satoshi`, Args: common.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { @@ -81,7 +82,7 @@ func runIDInspect(cmd *cobra.Command, arg string) error { password, err := keyresolve.ResolveVaultPassword(jsonMode, "Vault password: ") if err != nil { - return common.UsageError(cmd, err) + return common.InvocationError(cmd, err) } vaultPrivKeyHex, err := client.UnlockVaultIdentity(password) if err != nil { @@ -250,7 +251,7 @@ func runIDList(cmd *cobra.Command) error { if reveal && len(entries) > 0 { password, err := keyresolve.ResolveVaultPassword(jsonMode, "Vault password: ") if err != nil { - return common.UsageError(cmd, err) + return common.InvocationError(cmd, err) } vaultPrivKeyHex, err = client.UnlockVaultIdentity(password) if err != nil { diff --git a/cli/ncli/id_sign.go b/cli/ncli/id_sign.go index b03fd98..2038047 100644 --- a/cli/ncli/id_sign.go +++ b/cli/ncli/id_sign.go @@ -24,13 +24,13 @@ Fails if an event already declares a pubkey that conflicts with Example: ` ncli id sign -e events.json -o signed.json --identity satoshi`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if _, err := validateArgFile(cmd, "events", true, ".json", ".jsonp", ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if _, err := validateArgFile(cmd, "out", false, ".json", ".jsonp", ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } return nil }, diff --git a/cli/ncli/miner.go b/cli/ncli/miner.go index 84058b7..67efd71 100644 --- a/cli/ncli/miner.go +++ b/cli/ncli/miner.go @@ -30,22 +30,18 @@ var minerCmd = &cobra.Command{ var minerMineCmd = &cobra.Command{ Use: "mine", Short: "Mine proof-of-work into an unsigned event", - Long: `Mine proof-of-work (NIP-13) for an event, writing the result to --out (or -back to --event with --in-place). Runs across multiple CPU cores by -default (see --workers). + Long: `Mine NIP-13 proof-of-work for an event across multiple CPU cores. -The event comes from --event (a NIP-01 event file), or inline from ---content/--content-file plus --kind/--tag -- pick one, not both. ---content/--content-file mode fills in created_at and kind (default 1). - -If --identity resolves to a private key, the mined event is signed -automatically before being written. A pubkey-only identity mines but -can't sign (logged, not silent).`, +The event comes from --event, or inline from --content/--content-file -- +pick one, not both. Exactly one of --out or --in-place says where the +result goes. If --identity resolves to a private key, the mined event is +signed before it's written.`, Example: ` ncli miner mine -e event.json -o mined.json - ncli miner mine -e event.json --in-place --workers 4`, + ncli miner mine -e event.json --in-place --workers 4 + ncli miner mine --content "hello" --identity satoshi -d 20 -o mined.json`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } hasEvent := cmd.Flags().Changed("event") @@ -54,27 +50,27 @@ can't sign (logged, not silent).`, switch { case hasEvent && (hasContent || hasContentFile): - return common.UsageError(cmd, fmt.Errorf("--event is mutually exclusive with --content/--content-file; a structured event file already declares kind/content/tags")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--event is mutually exclusive with --content/--content-file; a structured event file already declares kind/content/tags")) case hasContent && hasContentFile: - return common.UsageError(cmd, fmt.Errorf("--content and --content-file are mutually exclusive")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--content and --content-file are mutually exclusive")) case !hasEvent && !hasContent && !hasContentFile: - return common.UsageError(cmd, fmt.Errorf("specify --event, or --content/--content-file to author a draft inline")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("specify --event, or --content/--content-file to author a draft inline")) } if hasEvent && cmd.Flags().Changed("kind") { - return common.UsageError(cmd, fmt.Errorf("--kind only applies to --content/--content-file mode; --event's file already declares kind")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--kind only applies to --content/--content-file mode; --event's file already declares kind")) } if hasEvent && cmd.Flags().Changed("tag") { - return common.UsageError(cmd, fmt.Errorf("--tag only applies to --content/--content-file mode; --event's file already declares tags")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--tag only applies to --content/--content-file mode; --event's file already declares tags")) } if hasEvent { if _, err := validateArgFile(cmd, "event", true, ".json", ".jsonp", ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } if hasContentFile { if _, err := validateArgFile(cmd, "content-file", true, ".txt"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } @@ -82,15 +78,15 @@ can't sign (logged, not silent).`, inPlace, _ := cmd.Flags().GetBool("in-place") switch { case out == "" && !inPlace: - return common.UsageError(cmd, fmt.Errorf("exactly one of --out or --in-place is required")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("exactly one of --out or --in-place is required")) case out != "" && inPlace: - return common.UsageError(cmd, fmt.Errorf("--out and --in-place are mutually exclusive")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--out and --in-place are mutually exclusive")) case inPlace && !hasEvent: - return common.UsageError(cmd, fmt.Errorf("--in-place requires --event; --content/--content-file mode has no input file to overwrite")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--in-place requires --event; --content/--content-file mode has no input file to overwrite")) } if out != "" { if _, err := validateArgFile(cmd, "out", false, ".json", ".jsonp", ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } return nil @@ -164,7 +160,7 @@ can't sign (logged, not silent).`, } if draft.PubKey == "" && opts.IdentityPubKeyHex == "" { - return common.UsageError(cmd, fmt.Errorf("--content/--content-file mode has no pubkey source other than --identity; pass --identity ")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--content/--content-file mode has no pubkey source other than --identity; pass --identity ")) } // Progress is periodic log narration, not a prompt -- but it's still @@ -269,21 +265,17 @@ func parseTagFlags(pairs []string) ([][]string, error) { var minerCheckCmd = &cobra.Command{ Use: "check", Short: "Verify proof-of-work", - Long: `Verify proof-of-work (NIP-13) on already-mined events, sourced from a -JSON file (--events, e.g. from "ncli dump") or fetched live, merged -across every target. - -Live mode's targets and filters come from --targets (a YAML file), or ---relays plus inline filter flags -- pick one, not both. Omitting both -falls back to the relays configured via "ncli prefs relays add". ---identity further narrows live mode to one identity's own events. + Long: `Verify NIP-13 proof-of-work on already-mined events, read from --events +or fetched live across every target. Exits non-zero if any event fails, +so it drops straight into CI. -Exits non-zero if any checked event fails.`, +Live mode takes --targets, or --relays plus inline filter flags -- pick +one, not both. Omitting both falls back to "ncli prefs relays".`, Example: ` ncli miner check -e events.json ncli miner check -t targets.yaml`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } fileMode := cmd.Flags().Changed("events") @@ -291,23 +283,23 @@ Exits non-zero if any checked event fails.`, switch { case fileMode && liveMode: - return common.UsageError(cmd, fmt.Errorf("--events (file mode) cannot be combined with --targets/--relays/--identity/inline filter flags (live mode); use one or the other")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--events (file mode) cannot be combined with --targets/--relays/--identity/inline filter flags (live mode); use one or the other")) case !fileMode && !liveMode: - return common.UsageError(cmd, fmt.Errorf("specify --events for file mode, or --targets/--relays/--identity/an inline filter flag for live mode")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("specify --events for file mode, or --targets/--relays/--identity/an inline filter flag for live mode")) } if err := queryMutualExclusionCheck(cmd); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if fileMode { if _, err := validateArgFile(cmd, "events", true, ".json", ".jsonp"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } if cmd.Flags().Changed("targets") { if _, err := validateArgFile(cmd, "targets", true, ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } return nil @@ -348,7 +340,11 @@ Exits non-zero if any checked event fails.`, } } - report, err = client.CheckPOWLive(ctx, targetsSpec, filtersSpec) + err = common.WithSpinner(cmd, targetsMessage("checking proof-of-work across", targetsSpec), func() error { + var cErr error + report, cErr = client.CheckPOWLive(ctx, targetsSpec, filtersSpec) + return cErr + }) } if err != nil { diff --git a/cli/ncli/ping.go b/cli/ncli/ping.go index 2685346..f3a006a 100644 --- a/cli/ncli/ping.go +++ b/cli/ncli/ping.go @@ -15,29 +15,26 @@ import ( var pingCmd = &cobra.Command{ Use: "ping [relay...]", Short: "Test relay connectivity", - Long: `Probe reachability of each target relay with a Limit-1 subscription. -Local store paths in the target list are skipped. + Long: `Probe each target relay with a Limit-1 subscription. Exits non-zero if +any relay was unreachable -- unlike find/dump, which tolerate a dead +target, reachability is the whole point here. -Give relays as positional arguments, or --targets for a relay -list file -- pick one, not both. Omitting both falls back to the relays -configured via "ncli prefs relays add". - -Results narrate as plain log lines on stderr by default. --tui shows a -live interactive board instead (requires a real terminal; ignored with ---json/--quiet). --json prints a structured report to stdout instead of -narrating. Exits non-zero if any relay was unreachable.`, +Give relays as positional arguments, or --targets -- pick one, not both. +Omitting both falls back to "ncli prefs relays". --tui shows a live +board instead of log lines.`, Example: ` ncli ping wss://relay.example.com - ncli ping -t targets.yaml`, + ncli ping -t targets.yaml + ncli ping --tui wss://relay.example.com`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if cmd.Flags().Changed("targets") && len(args) > 0 { - return common.UsageError(cmd, fmt.Errorf("--targets is mutually exclusive with relay arguments; a --targets file already declares its own relays")) + return common.InvocationOrHelp(cmd, args, fmt.Errorf("--targets is mutually exclusive with relay arguments; a --targets file already declares its own relays")) } if cmd.Flags().Changed("targets") { if _, err := validateArgFile(cmd, "targets", true, ".yaml", ".yml"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } } return nil @@ -78,12 +75,23 @@ narrating. Exits non-zero if any relay was unreachable.`, } } - report := client.Ping(ctx, targetsSpec, client.PingOptions{ - JSON: jsonMode, - Quiet: quiet, - TUI: tuiMode, - Timeout: timeout, - }) + var report *client.PingReport + runPing := func() error { + report = client.Ping(ctx, targetsSpec, client.PingOptions{ + JSON: jsonMode, + Quiet: quiet, + TUI: tuiMode, + Timeout: timeout, + }) + return nil + } + // --tui hands the terminal to the ping dashboard, which draws its own + // progress; a spinner underneath it would fight for the same screen. + if tuiMode { + _ = runPing() + } else { + _ = common.WithSpinner(cmd, targetsMessage("pinging", targetsSpec), runPing) + } if jsonMode { common.PrintJSON(report) diff --git a/cli/ncli/publish.go b/cli/ncli/publish.go index b09f557..0917f06 100644 --- a/cli/ncli/publish.go +++ b/cli/ncli/publish.go @@ -25,10 +25,10 @@ Exits non-zero if any pair fails.`, ncli publish -e signed.json -s wss://relay.example.com`, Args: func(cmd *cobra.Command, args []string) error { if err := cmd.ValidateRequiredFlags(); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } if _, err := validateArgFile(cmd, "events", true, ".json", ".jsonp"); err != nil { - return common.UsageError(cmd, err) + return common.InvocationOrHelp(cmd, args, err) } return nil }, @@ -61,7 +61,12 @@ Exits non-zero if any pair fails.`, } } - report, err := client.PublishToTargets(ctx, targetsSpec, events) + var report *client.PublishReport + err = common.WithSpinner(cmd, targetsMessage(fmt.Sprintf("publishing %d event(s) to", len(events)), targetsSpec), func() error { + var pErr error + report, pErr = client.PublishToTargets(ctx, targetsSpec, events) + return pErr + }) if err != nil { if errors.Is(err, client.ErrNoReachableTargets) { return common.NetworkError(cmd, "", err) diff --git a/cli/ncli/query.go b/cli/ncli/query.go index 696c300..a75ba71 100644 --- a/cli/ncli/query.go +++ b/cli/ncli/query.go @@ -8,6 +8,20 @@ import ( "github.com/spf13/cobra" ) +// targetsMessage builds a spinner label like "querying 3 targets" for the +// commands that fan out over a TargetsSpec, so find/dump/miner check all +// phrase the wait the same way. +func targetsMessage(verb string, targets *client.TargetsSpec) string { + n := 0 + if targets != nil { + n = len(targets.Relays) + } + if n == 1 { + return fmt.Sprintf("%s 1 target", verb) + } + return fmt.Sprintf("%s %d targets", verb, n) +} + // registerQueryFlags adds the targets+filters query trio shared by find, // dump, and miner check's live mode: a combined --targets YAML file // (relays and/or filters), --relays as its comma-separated command-line diff --git a/cli/ncli/root.go b/cli/ncli/root.go index 8a8ba5e..49037af 100644 --- a/cli/ncli/root.go +++ b/cli/ncli/root.go @@ -34,6 +34,14 @@ var RootCmd = &cobra.Command{ Long: `Run and operate Nostr relays, and manage events: serve, stream, sync, inspect, export, delegate, administer, and mine.`, Example: ` ncli id ncli find npub1...`, + + // Cobra prints its own "Error: ..." plus a usage dump for anything it + // rejects before a command's Args validator runs -- an unknown flag, an + // unknown command -- which stacked a second report on top of the one + // EmitError already writes. Silencing both here makes main.go's + // classifyRootErr the single sink for those too. + SilenceUsage: true, + SilenceErrors: true, } func init() { diff --git a/cli/relay/admin.go b/cli/relay/admin.go index 85e31cd..589880e 100644 --- a/cli/relay/admin.go +++ b/cli/relay/admin.go @@ -54,7 +54,7 @@ func addRemoteAdminCommands(cmd *cobra.Command) { reindexCmd := &cobra.Command{ Use: "reindex", - Short: "Trigger a reindex on the running relay, without restarting it", + Short: "Reindex a running relay without restarting it", Example: ` ncli relay reindex search --config relay.yaml`, RunE: common.RequireSubcommand, } @@ -64,7 +64,7 @@ func addRemoteAdminCommands(cmd *cobra.Command) { clearCmd := &cobra.Command{ Use: "clear", - Short: "Clear indexes on the running relay, without restarting it", + Short: "Clear a running relay's indexes without restarting it", Example: ` ncli relay clear search --config relay.yaml`, RunE: common.RequireSubcommand, } @@ -216,8 +216,8 @@ func getAdminConfig() (string, int, string, error) { // adminRequest issues a NIP-98-authenticated admin HTTP request with no // body -- the shape every pre-existing stats/reindex/clear caller needs. -func adminRequest(method, path string) (map[string]interface{}, error) { - return adminRequestBody(method, path, nil) +func adminRequest(cmd *cobra.Command, method, path string) (map[string]interface{}, error) { + return adminRequestBody(cmd, method, path, nil) } // adminRequestBody is adminRequest's superset: body, if non-nil, is @@ -226,7 +226,7 @@ func adminRequest(method, path string) (map[string]interface{}, error) { // function rather than a second HTTP client -- every admin command, with or // without a body, shares the same signing/timeout/error-classification // path. -func adminRequestBody(method, path string, body interface{}) (map[string]interface{}, error) { +func adminRequestBody(cmd *cobra.Command, method, path string, body interface{}) (map[string]interface{}, error) { privKey, port, _, err := getAdminConfig() if err != nil { // nip11.privkey missing from config -- a missing-required-config @@ -264,7 +264,12 @@ func adminRequestBody(method, path string, body interface{}) (map[string]interfa } client := &http.Client{Timeout: 10 * time.Second} - resp, err := client.Do(req) + var resp *http.Response + err = common.WithSpinner(cmd, fmt.Sprintf("contacting relay on port %d", port), func() error { + var dErr error + resp, dErr = client.Do(req) + return dErr + }) if err != nil { return nil, &common.CLIError{ Err: fmt.Errorf("request failed (is the relay running on port %d?): %w", port, err), @@ -315,7 +320,7 @@ func adminRequestBody(method, path string, body interface{}) (map[string]interfa func runStats(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - stats, err := adminRequest("GET", "/admin/worker/stats") + stats, err := adminRequest(cmd, "GET", "/admin/worker/stats") if err != nil { return common.RuntimeError(cmd, err) } @@ -382,7 +387,7 @@ func renderReindexStats(label string, data interface{}) { func runReindexSearch(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - resp, err := adminRequest("POST", "/admin/reindex/search") + resp, err := adminRequest(cmd, "POST", "/admin/reindex/search") if err != nil { return common.RuntimeError(cmd, err) } @@ -398,7 +403,7 @@ func runReindexSearch(cmd *cobra.Command, args []string) error { func runReindexZaps(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - resp, err := adminRequest("POST", "/admin/reindex/zaps") + resp, err := adminRequest(cmd, "POST", "/admin/reindex/zaps") if err != nil { return common.RuntimeError(cmd, err) } @@ -414,7 +419,7 @@ func runReindexZaps(cmd *cobra.Command, args []string) error { func runClearSearch(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - resp, err := adminRequest("DELETE", "/admin/search") + resp, err := adminRequest(cmd, "DELETE", "/admin/search") if err != nil { return common.RuntimeError(cmd, err) } @@ -429,7 +434,7 @@ func runClearSearch(cmd *cobra.Command, args []string) error { func runClearZaps(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - resp, err := adminRequest("DELETE", "/admin/zaps") + resp, err := adminRequest(cmd, "DELETE", "/admin/zaps") if err != nil { return common.RuntimeError(cmd, err) } @@ -538,7 +543,7 @@ func joinOrDash(items []string) string { func runMembersList(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - resp, err := adminRequest("GET", "/admin/membership/members") + resp, err := adminRequest(cmd, "GET", "/admin/membership/members") if err != nil { return common.RuntimeError(cmd, err) } @@ -570,7 +575,7 @@ func runMembersList(cmd *cobra.Command, args []string) error { func runMembersShow(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") pubkey := args[0] - resp, err := adminRequest("GET", "/admin/membership/members/"+pubkey) + resp, err := adminRequest(cmd, "GET", "/admin/membership/members/"+pubkey) if err != nil { return common.RuntimeError(cmd, err) } @@ -591,7 +596,7 @@ func runMembersAdd(cmd *cobra.Command, args []string) error { pubkey := args[0] roles, _ := cmd.Flags().GetStringArray("role") - resp, err := adminRequestBody("POST", "/admin/membership/members", map[string]interface{}{"pubkey": pubkey, "roles": roles}) + resp, err := adminRequestBody(cmd, "POST", "/admin/membership/members", map[string]interface{}{"pubkey": pubkey, "roles": roles}) if err != nil { return common.RuntimeError(cmd, err) } @@ -612,7 +617,7 @@ func runMembersAdd(cmd *cobra.Command, args []string) error { func runMembersRemove(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") pubkey := args[0] - resp, err := adminRequest("DELETE", "/admin/membership/members/"+pubkey) + resp, err := adminRequest(cmd, "DELETE", "/admin/membership/members/"+pubkey) if err != nil { return common.RuntimeError(cmd, err) } @@ -640,7 +645,7 @@ func runInvitesCreate(cmd *cobra.Command, args []string) error { body["ttl"] = ttl.String() } - resp, err := adminRequestBody("POST", "/admin/membership/invites", body) + resp, err := adminRequestBody(cmd, "POST", "/admin/membership/invites", body) if err != nil { return common.RuntimeError(cmd, err) } @@ -666,7 +671,7 @@ func runInvitesCreate(cmd *cobra.Command, args []string) error { func runInvitesList(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - resp, err := adminRequest("GET", "/admin/membership/invites") + resp, err := adminRequest(cmd, "GET", "/admin/membership/invites") if err != nil { return common.RuntimeError(cmd, err) } @@ -703,7 +708,7 @@ func runInvitesList(cmd *cobra.Command, args []string) error { func runInvitesRevoke(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") code := args[0] - resp, err := adminRequest("DELETE", "/admin/membership/invites/"+code) + resp, err := adminRequest(cmd, "DELETE", "/admin/membership/invites/"+code) if err != nil { return common.RuntimeError(cmd, err) } @@ -722,7 +727,7 @@ func runInvitesRevoke(cmd *cobra.Command, args []string) error { func runRolesList(cmd *cobra.Command, args []string) error { jsonMode, _ := cmd.Flags().GetBool("json") - resp, err := adminRequest("GET", "/admin/membership/roles") + resp, err := adminRequest(cmd, "GET", "/admin/membership/roles") if err != nil { return common.RuntimeError(cmd, err) } @@ -774,7 +779,7 @@ func runRolesCreate(cmd *cobra.Command, args []string) error { body.Order = &o } - resp, err := adminRequestBody("POST", "/admin/membership/roles", body) + resp, err := adminRequestBody(cmd, "POST", "/admin/membership/roles", body) if err != nil { return common.RuntimeError(cmd, err) } diff --git a/cli/relay/command.go b/cli/relay/command.go index 6cdb14c..479d29e 100644 --- a/cli/relay/command.go +++ b/cli/relay/command.go @@ -230,16 +230,14 @@ func NewRelayCommand() *cobra.Command { cmd := &cobra.Command{ Use: "relay", Short: "Run the relay server, or operate one that's already running", - Long: `Bare invocation runs the Nostr relay server: WebSocket protocol, -NIP-11 metadata, and optional search. Its subcommands instead operate a -relay that's already running, over NIP-98 authenticated HTTP. - --c/--context runs directly against a named relay context (see -"relay context"), creating it on the spot -- a minimal config, backed by a -freshly generated identity under .../relays// -- if that name isn't -saved yet.`, + Long: `Bare invocation runs the Nostr relay server. Its subcommands instead +operate a relay that's already running, over NIP-98 authenticated HTTP. + +-c/--context runs against a named relay context, creating a +minimal one backed by a fresh identity if that name isn't saved yet.`, Example: ` ncli relay --config relay.yaml - ncli relay --context myrelay`, + ncli relay --context myrelay + ncli relay stats --config relay.yaml`, PreRunE: func(cmd *cobra.Command, args []string) error { ctxName, _ := cmd.Flags().GetString("context") if ctxName != "" { diff --git a/cli/relay/context_run.go b/cli/relay/context_run.go index e7df0c1..df2b242 100644 --- a/cli/relay/context_run.go +++ b/cli/relay/context_run.go @@ -38,7 +38,7 @@ const relaysDirName = "relays" // resolveConfigFile already loaded. func runOrCreateRelayContext(cmd *cobra.Command, name string) error { if cfgFile, _ := cmd.Flags().GetString("config"); cfgFile != "" { - return common.UsageError(cmd, errors.New("--config and --context/-c are mutually exclusive")) + return common.InvocationError(cmd, errors.New("--config and --context/-c are mutually exclusive")) } if !validRelayContextName(name) { return common.InvalidInputError(cmd, name, fmt.Errorf("relay context name %q must not be empty, \".\", \"..\", or contain a path separator", name)) diff --git a/cmd/ncli/main.go b/cmd/ncli/main.go index 8ffeb6e..75b25de 100644 --- a/cmd/ncli/main.go +++ b/cmd/ncli/main.go @@ -2,6 +2,7 @@ package main import ( "os" + "strings" "github.com/ohstr/ncli/cli/blossom" "github.com/ohstr/ncli/cli/bunker" @@ -52,7 +53,45 @@ func main() { // printing (and exiting) its own way. cmd, err := ncli.RootCmd.ExecuteC() if err != nil { + err = classifyRootErr(cmd, err) common.EmitError(cmd, err) os.Exit(common.ExitCode(err)) } } + +// classifyRootErr catches the failures cobra raises inside ExecuteC before +// any Args validator or RunE runs -- an unknown flag, an unknown command, a +// required flag left unset on a command that doesn't call +// ValidateRequiredFlags itself. Left alone these come back as bare errors, so +// ExitCode fell through to CodeInternal/exit 1 and cobra printed its own +// report alongside ncli's. Routing them through InvocationError gives them +// the same "usage", exit 2, Error-then-help shape as every other +// mis-invocation. +func classifyRootErr(cmd *cobra.Command, err error) error { + if _, ok := err.(*common.CLIError); ok { + return err // already classified further down the call stack + } + // Flag parsing is what failed, so cmd's own --json is unreliable here -- + // re-scan argv for it, or a script would get a human-shaped line. + if cmd != nil && jsonRequested(os.Args[1:]) { + _ = cmd.Flags().Set("json", "true") + } + return common.InvocationError(cmd, err) +} + +// jsonRequested hand-parses --json out of argv, last occurrence winning and +// stopping at a bare "--". Only used when cobra's own parse already failed. +func jsonRequested(args []string) bool { + found := false + for _, a := range args { + switch { + case a == "--": + return found + case a == "--json": + found = true + case strings.HasPrefix(a, "--json="): + found = a[len("--json="):] == "true" || a[len("--json="):] == "1" + } + } + return found +} From 412d510e8e63d16d1fb24565e6089e7c2b37fe73 Mon Sep 17 00:00:00 2001 From: naliyi <154817482+naliyi@users.noreply.github.com> Date: Wed, 23 Sep 2026 13:58:45 +0000 Subject: [PATCH 2/4] feat(profile): add ncli profile Looking someone up was "ncli find npub1..." returning raw JSON. "ncli profile " answers the same question as a readable card, from a single subscription covering the four records that describe an identity: kind:0 metadata, kind:3 (the following count), kind:10002 (NIP-65 relays, with read/write markers) and kind:10063 (Blossom servers). It shows the lightning address from lud16, falling back to lud06. It takes whatever ResolveIdentifier already takes -- npub, hex, nprofile, a nip-05 address, or a vault label, so "ncli profile