From b1a1ee216541918ad1056f6996c31e68ea9d154f Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Thu, 1 Oct 2026 16:23:23 +0200 Subject: [PATCH 1/8] Fix -o for large and unfinished batches, usage exit codes, and CSV columns `-o` silently wrote a header-only file for batches that return `download_file` or are still verifying, and usage errors exited 1 with `unknown` even though the README promises 4 and `invalid_input` for bad input. This also makes `account status --json` pass the API body through like the other commands, shows the stored login email in `status` for API-key logins, and gives CSV exports every verify field with blanks where the API sent nothing. --- README.md | 30 +++- cmd/account.go | 7 + cmd/auth_display_test.go | 79 +++++++++ cmd/batch.go | 63 ++++++- cmd/batch_output_test.go | 156 +++++++++++++++++ cmd/root.go | 4 + cmd/skill.go | 2 +- cmd/status.go | 20 ++- cmd/usage_errors.go | 84 +++++++++ cmd/usage_errors_test.go | 100 +++++++++++ internal/api/download.go | 45 +++++ internal/api/download_test.go | 42 +++++ internal/output/download.go | 290 +++++++++++++++++++++++++++++++ internal/output/download_test.go | 154 ++++++++++++++++ internal/output/file.go | 262 ++++++++++++++++++++-------- internal/output/file_test.go | 114 +++++++++++- 16 files changed, 1358 insertions(+), 94 deletions(-) create mode 100644 cmd/auth_display_test.go create mode 100644 cmd/batch_output_test.go create mode 100644 cmd/usage_errors.go create mode 100644 cmd/usage_errors_test.go create mode 100644 internal/api/download.go create mode 100644 internal/api/download_test.go create mode 100644 internal/output/download.go create mode 100644 internal/output/download_test.go diff --git a/README.md b/README.md index f3fdfc3..9c1ced3 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,10 @@ Resolution order when multiple sources are configured: credential source without making a network call — useful for agents self-diagnosing a failure. (`emailable account status` is the separate network-backed command that fetches the owner email + remaining credits.) +When the active credential is the one saved by `emailable login` (OAuth or a +stored API key), it also shows the account email recorded at login. It +doesn't for a key from `EMAILABLE_API_KEY`, which may belong to a different +account. ```bash emailable status @@ -239,6 +243,30 @@ Flags: - `-o, --output ` — write the results to FILE (`.csv` or `.json`; format inferred from extension) +#### Saving results with `-o` + +`-o` writes every result row, whatever the batch size: + +- Batches over 1,000 emails return a `download_file` link instead of inline + results. `-o` downloads that file (a ZIP compressed CSV) and converts it to + the format the extension asks for. +- A batch that is still verifying is an error (exit code `4`) unless you pass + `--wait` to block until it finishes or `--partial` to save the rows + verified so far. +- With `--json`, the file is written as JSON whatever its extension. + +CSV files have one column per verify response field, always in this order: + +``` +email,state,reason,score,domain,user,first_name,last_name,full_name,gender,birth_year,free,role,disposable,accept_all,mailbox_full,no_reply,did_you_mean,mx_record,smtp_provider,tag,duration +``` + +A cell is blank when the API omitted the field (for example with +`--response-fields`) or returned `null`. Any field not in this list is +appended as an extra column, sorted by name. JSON files keep the API's +batch payload, with the `emails` array filled in from the download for +large batches. + ### Account Show account information and remaining credits: @@ -345,7 +373,7 @@ through verbatim. | `not_authenticated` | Missing or invalid credentials (HTTP 401) | | `forbidden` | Authenticated but not allowed (HTTP 403) | | `not_found` | Unknown resource (HTTP 404) | -| `invalid_input` | Bad request / validation failure (HTTP 400, 422) | +| `invalid_input` | Bad request / validation failure (HTTP 400, 422), or a CLI usage error (unknown command or flag, bad flag value, conflicting flags) | | `rate_limited` | Throttled by the server (HTTP 429) | | `try_again` | Verification is still processing (HTTP 249) | | `server_error` | Server-side failure (HTTP 5xx) | diff --git a/cmd/account.go b/cmd/account.go index c9f9607..0018cf9 100644 --- a/cmd/account.go +++ b/cmd/account.go @@ -9,6 +9,8 @@ func newAccountCmd() *cobra.Command { account := &cobra.Command{ Use: "account", Short: "Manage your Emailable account", + Args: unknownSubcommand, + RunE: showHelp, SilenceUsage: true, Example: ` # Show the owner email and remaining credits emailable account status`, @@ -37,6 +39,11 @@ func newAccountCmd() *cobra.Command { if err != nil { return err } + // JSON prints the response itself so its raw body passes through + // with any fields AccountView doesn't model. + if jsonOutput { + return newOutput(cmd.OutOrStdout(), true).Print(a) + } view := &output.AccountView{ OwnerEmail: a.OwnerEmail, AvailableCredits: a.AvailableCredits, diff --git a/cmd/auth_display_test.go b/cmd/auth_display_test.go new file mode 100644 index 0000000..a14347a --- /dev/null +++ b/cmd/auth_display_test.go @@ -0,0 +1,79 @@ +package cmd + +import ( + "net/http" + "strings" + "testing" + + "github.com/emailable/emailable-cli/internal/credentials" +) + +func TestAccountStatus_JSONPassesThroughUnknownFields(t *testing.T) { + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, map[string]any{"owner_email": "o@x.com", "available_credits": 5, "plan": "pro"}) + })) + env.seedAPIKey(t, "sk_test_xxx") + + res := runRoot(t, "account", "status", "--json") + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + if payload := decodeJSON(t, res.Stdout.Bytes()); payload["plan"] != "pro" { + t.Errorf("expected unmodeled field to pass through, got %v", payload) + } + + res = runRoot(t, "account", "status", "--jq", ".plan") + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + if got := strings.TrimSpace(res.Stdout.String()); got != "pro" { + t.Errorf("--jq .plan: got %q", got) + } +} + +func seedStoredKeyWithEmail(t *testing.T, env *testEnv) { + t.Helper() + creds := &credentials.Credentials{APIKey: "sk_test_xxx", OwnerEmail: "owner@example.com"} + if err := creds.Save(env.CredentialsPath); err != nil { + t.Fatal(err) + } +} + +func TestStatus_StoredAPIKeyShowsEmail(t *testing.T) { + env := newTestEnv(t, http.NotFoundHandler()) + seedStoredKeyWithEmail(t, env) + + res := runRoot(t, "status", "--json") + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + if payload := decodeJSON(t, res.Stdout.Bytes()); payload["owner_email"] != "owner@example.com" { + t.Errorf("expected owner_email, got %v", payload) + } + + res = runRoot(t, "status") + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + if !strings.Contains(res.Stdout.String(), "owner@example.com") { + t.Errorf("expected Account row, got %q", res.Stdout.String()) + } +} + +func TestStatus_EnvAPIKeyHidesStoredEmail(t *testing.T) { + env := newTestEnv(t, http.NotFoundHandler()) + seedStoredKeyWithEmail(t, env) + t.Setenv("EMAILABLE_API_KEY", "sk_env_yyy") + + res := runRoot(t, "status", "--json") + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + if payload := decodeJSON(t, res.Stdout.Bytes()); payload["owner_email"] != nil { + t.Errorf("stored email must not show for an env key, got %v", payload["owner_email"]) + } + res = runRoot(t, "status") + if strings.Contains(res.Stdout.String(), "owner@example.com") { + t.Errorf("stored email must not show for an env key, got %q", res.Stdout.String()) + } +} diff --git a/cmd/batch.go b/cmd/batch.go index 3203903..7ea3c7a 100644 --- a/cmd/batch.go +++ b/cmd/batch.go @@ -18,6 +18,8 @@ func newBatchCmd() *cobra.Command { batch := &cobra.Command{ Use: "batch", Short: "Verify a batch of emails", + Args: unknownSubcommand, + RunE: showHelp, SilenceUsage: true, Example: ` # Submit a batch and wait for completion emailable batch verify emails.csv --wait @@ -50,6 +52,9 @@ func newBatchCmd() *cobra.Command { partial, _ := cmd.Flags().GetBool("partial") outPath, _ := cmd.Flags().GetString("output") showAll, _ := cmd.Flags().GetBool("all") + if wait && partial { + return NewInvalidInput("--wait and --partial can't be combined: --wait already polls until completion") + } cctx, err := newCmdCtxFor(cmd, jsonOutput) if err != nil { @@ -61,9 +66,6 @@ func newBatchCmd() *cobra.Command { } if wait { - if partial { - return fmt.Errorf("--wait and --partial can't be combined: --wait already polls until completion") - } s, err := waitForCompletion(cmd.Context(), client, id, cctx.JSONMode || cctx.Quiet, cmd.ErrOrStderr()) if err != nil { return err @@ -283,6 +285,10 @@ func saveToFile(cmd *cobra.Command, cctx *cmdCtx, v any, path string) error { if err != nil { return err } + return reportSaved(cmd, cctx, n, path) +} + +func reportSaved(cmd *cobra.Command, cctx *cmdCtx, n int, path string) error { if !cctx.JSONMode { h := &output.Human{W: cmd.ErrOrStderr(), Quiet: cctx.Quiet} msg := savedMessage(n, path) @@ -291,6 +297,46 @@ func saveToFile(cmd *cobra.Command, cctx *cmdCtx, v any, path string) error { return nil } +// saveBatchToFile refuses to write a header-only file for a batch whose rows +// aren't available, and fetches the results file for large batches. +func saveBatchToFile(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatus, batchID, path string) error { + // Only `batch get` has --partial; the lookup fails (false) elsewhere. + partial, _ := cmd.Flags().GetBool("partial") + if !status.IsComplete() && !partial { + if processed, total, ok := status.Progress(); ok { + return NewInvalidInputf("batch %s is still verifying (%d/%d); use --wait or --partial", batchID, processed, total) + } + return NewInvalidInputf("batch %s is still verifying; use --wait or --partial", batchID) + } + if status.DownloadFile != "" { + return saveDownloadToFile(cmd, cctx, status, path) + } + if _, total, ok := status.Progress(); ok && total > 0 && status.IsComplete() && len(status.Emails) == 0 { + return fmt.Errorf("batch %s has no per-email results to save; the API stops returning them some days after completion", batchID) + } + return saveToFile(cmd, cctx, status, path) +} + +func saveDownloadToFile(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatus, path string) error { + sp := ui.NewTo(cmd.ErrOrStderr(), "Downloading results") + if !cctx.JSONMode && !cctx.Quiet { + sp.Start() + } + d, err := api.FetchDownload(cmd.Context(), status.DownloadFile, nil) + sp.Stop() + if err != nil { + return err + } + n, err := output.WriteDownload(d, status, output.SaveOptions{ + Path: path, + ForceJSON: cctx.JSONMode, + }) + if err != nil { + return err + } + return reportSaved(cmd, cctx, n, path) +} + func savedMessage(n int, path string) string { switch { case n <= 0: @@ -304,12 +350,19 @@ func savedMessage(n int, path string) string { func renderBatchOutcome(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatus, batchID, outPath string, showAll bool) error { if outPath != "" { - return saveToFile(cmd, cctx, status, outPath) + return saveBatchToFile(cmd, cctx, status, batchID, outPath) } if cctx.JSONMode { return newOutput(cmd.OutOrStdout(), true).Print(status) } - if status.DownloadFile != "" || len(status.Emails) == 0 { + if status.DownloadFile != "" { + if err := newOutput(cmd.OutOrStdout(), false).Print(status); err != nil { + return err + } + h := &output.Human{W: cmd.OutOrStdout(), Quiet: cctx.Quiet} + return h.Hint(fmt.Sprintf("Run `emailable batch get %s -o results.csv` to save it.", batchID)) + } + if len(status.Emails) == 0 { return newOutput(cmd.OutOrStdout(), false).Print(status) } diff --git a/cmd/batch_output_test.go b/cmd/batch_output_test.go new file mode 100644 index 0000000..5f3bf21 --- /dev/null +++ b/cmd/batch_output_test.go @@ -0,0 +1,156 @@ +package cmd + +import ( + "archive/zip" + "bytes" + "encoding/csv" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestBatchGet_WaitPartialBeforeAuth(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) // not logged in + res := runRoot(t, "batch", "get", "bch_1", "--wait", "--partial") + assertInvalidInput(t, res) +} + +func TestBatchGet_OutputUnfinishedBatch(t *testing.T) { + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, map[string]any{"message": "Your batch is being processed.", "processed": 1, "total": 4}) + })) + env.seedAPIKey(t, "sk_test_xxx") + out := filepath.Join(t.TempDir(), "results.csv") + + res := runRoot(t, "batch", "get", "bch_1", "-o", out) + assertInvalidInput(t, res) + if !strings.Contains(res.Err.Error(), "batch bch_1 is still verifying (1/4); use --wait or --partial") { + t.Errorf("unexpected message: %q", res.Err) + } + if _, err := os.Stat(out); !os.IsNotExist(err) { + t.Error("no file should be written for an unfinished batch") + } +} + +func TestBatchGet_OutputPartial(t *testing.T) { + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("partial") != "true" { + t.Errorf("expected partial=true, got %q", r.URL.RawQuery) + } + writeJSON(w, map[string]any{ + "id": "bch_1", + "message": "Your batch is being processed.", + "emails": []map[string]any{{"email": "a@x.com", "state": "deliverable"}}, + "total_counts": map[string]int{"processed": 1, "total": 4}, + }) + })) + env.seedAPIKey(t, "sk_test_xxx") + out := filepath.Join(t.TempDir(), "results.csv") + + res := runRoot(t, "batch", "get", "bch_1", "--partial", "-o", out) + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + rows := readCSVFile(t, out) + if len(rows) != 2 || rows[1][0] != "a@x.com" { + t.Errorf("unexpected rows: %v", rows) + } + if !strings.Contains(res.Stderr.String(), "Saved 1 result to") { + t.Errorf("expected success line, got %q", res.Stderr.String()) + } +} + +func TestBatchGet_OutputDownloadFile(t *testing.T) { + var zbuf bytes.Buffer + zw := zip.NewWriter(&zbuf) + f, _ := zw.Create("bch_big.csv") + _, _ = f.Write([]byte("email,state,score\na@x.com,deliverable,99\nb@y.com,risky,50\nc@z.com,unknown,0\n")) + if err := zw.Close(); err != nil { + t.Fatal(err) + } + + var downloadAuth string + storage := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + downloadAuth = r.Header.Get("Authorization") + w.Header().Set("Content-Type", "binary/octet-stream") + _, _ = w.Write(zbuf.Bytes()) + })) + t.Cleanup(storage.Close) + + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, map[string]any{ + "id": "bch_big", + "message": "Batch verification completed.", + "download_file": storage.URL + "/bch_big.csv?X-Amz-Signature=x", + "total_counts": map[string]int{"processed": 3, "total": 3}, + }) + })) + env.seedAPIKey(t, "sk_test_xxx") + out := filepath.Join(t.TempDir(), "results.csv") + + res := runRoot(t, "batch", "get", "bch_big", "-o", out) + if res.Err != nil { + t.Fatalf("execute: %v\nstderr: %s", res.Err, res.Stderr.String()) + } + if downloadAuth != "" { + t.Errorf("Authorization leaked to storage host: %q", downloadAuth) + } + rows := readCSVFile(t, out) + if len(rows) != 4 || rows[1][0] != "a@x.com" || rows[3][0] != "c@z.com" { + t.Errorf("unexpected rows: %v", rows) + } + if !strings.Contains(res.Stderr.String(), "Saved 3 results to") { + t.Errorf("expected row count in success line, got %q", res.Stderr.String()) + } +} + +func TestBatchGet_DownloadFileHint(t *testing.T) { + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, map[string]any{"id": "bch_big", "download_file": "https://cdn.example/x.csv"}) + })) + env.seedAPIKey(t, "sk_test_xxx") + + res := runRoot(t, "batch", "get", "bch_big") + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + if !strings.Contains(res.Stdout.String(), "emailable batch get bch_big -o results.csv") { + t.Errorf("expected save hint, got %q", res.Stdout.String()) + } +} + +func TestBatchGet_OutputExpiredResults(t *testing.T) { + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, map[string]any{ + "id": "bch_old", + "message": "Batch verification completed.", + "total_counts": map[string]int{"processed": 2, "total": 2}, + }) + })) + env.seedAPIKey(t, "sk_test_xxx") + out := filepath.Join(t.TempDir(), "results.csv") + + res := runRoot(t, "batch", "get", "bch_old", "-o", out) + if res.Err == nil || !strings.Contains(res.Err.Error(), "no per-email results") { + t.Fatalf("expected no-results error, got %v", res.Err) + } + if _, err := os.Stat(out); !os.IsNotExist(err) { + t.Error("no file should be written") + } +} + +func readCSVFile(t *testing.T, path string) [][]string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + rows, err := csv.NewReader(bytes.NewReader(data)).ReadAll() + if err != nil { + t.Fatal(err) + } + return rows +} diff --git a/cmd/root.go b/cmd/root.go index f913ceb..712fea4 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -216,6 +216,10 @@ func newRootCmd(v string) *cobra.Command { }, } root.SetVersionTemplate("{{ .Version }}\n") + // Both are inherited by every subcommand, so bad flags and unknown + // commands exit 4 / invalid_input like other bad input. + root.SetFlagErrorFunc(flagUsageError) + root.Args = unknownSubcommand root.PersistentFlags().BoolVar(&jsonOutput, "json", false, "Return JSON response") root.PersistentFlags().StringVar(&jqExpr, "jq", "", "Filter JSON output with a jq `expression` (implies --json)") diff --git a/cmd/skill.go b/cmd/skill.go index d1641fd..f47527c 100644 --- a/cmd/skill.go +++ b/cmd/skill.go @@ -23,7 +23,7 @@ func newSkillCmd() *cobra.Command { "writes SKILL.md to ~/.agents/skills/emailable and symlinks every " + "detected agent (Claude Code, OpenCode, Codex) into that canonical " + "copy, so one re-install picks up new releases everywhere.", - Args: cobra.NoArgs, + Args: unknownSubcommand, SilenceUsage: true, RunE: func(cmd *cobra.Command, _ []string) error { if jsonOutput || quietMode || !terminalsInteractive(cmd) { diff --git a/cmd/status.go b/cmd/status.go index 240e2f1..2ff8210 100644 --- a/cmd/status.go +++ b/cmd/status.go @@ -54,8 +54,8 @@ func runStatusE(cmd *cobra.Command, _ []string) error { if cctx.ProjectConfigPath != "" { payload["project_config_path"] = cctx.ProjectConfigPath } - if source == "oauth" && cctx.Credentials.OwnerEmail != "" { - payload["owner_email"] = cctx.Credentials.OwnerEmail + if email := storedOwnerEmail(cctx, source); email != "" { + payload["owner_email"] = email } if expiresAt != "" { payload["expires_at"] = expiresAt @@ -77,6 +77,18 @@ func authSourceFor(cctx *cmdCtx) (source string, loggedIn bool) { return string(apiKeySourceMissing), false } +// storedOwnerEmail returns the email saved at login, but only when the active +// credential is the stored one: a key from EMAILABLE_API_KEY may belong to a +// different account than the one that logged in. +func storedOwnerEmail(cctx *cmdCtx, source string) string { + switch apiKeySource(source) { + case apiKeySourceOAuth, apiKeySourceStored: + return cctx.Credentials.OwnerEmail + default: + return "" + } +} + func printStatusHuman(cmd *cobra.Command, cctx *cmdCtx, source string, loggedIn bool, expiresAt string, expiresIn int) error { w := cmd.OutOrStdout() stf := output.StylerFor(w) @@ -96,8 +108,8 @@ func printStatusHuman(cmd *cobra.Command, cctx *cmdCtx, source string, loggedIn {"Status:", stateStyle.Render(stateText)}, {"Source:", value.Render(source)}, } - if source == "oauth" && cctx.Credentials.OwnerEmail != "" { - rows = append(rows, [2]string{"Account:", value.Render(cctx.Credentials.OwnerEmail)}) + if email := storedOwnerEmail(cctx, source); email != "" { + rows = append(rows, [2]string{"Account:", value.Render(email)}) } if expiresAt != "" { expiry := expiresAt diff --git a/cmd/usage_errors.go b/cmd/usage_errors.go new file mode 100644 index 0000000..c1c4cd1 --- /dev/null +++ b/cmd/usage_errors.go @@ -0,0 +1,84 @@ +package cmd + +import ( + "fmt" + "os" + "strconv" + "strings" + + "github.com/emailable/emailable-cli/internal/env" + "github.com/spf13/cobra" +) + +// cliArgs returns the raw command line. A var so tests can stand in for os.Args. +var cliArgs = func() []string { return os.Args[1:] } + +// flagUsageError marks a flag parse failure as invalid input (exit 4). Parsing +// stops at the bad flag, so a later --json or --jq hasn't been bound yet; the +// raw args are scanned for them so the error still renders as JSON. +func flagUsageError(_ *cobra.Command, err error) error { + if want, explicit := jsonRequested(cliArgs()); explicit { + if want { + jsonOutput = true + } + } else { + applyConfiguredOutput() + } + return NewInvalidInput(err.Error()) +} + +// unknownSubcommand rejects positional args on a command that only groups +// subcommands, so `emailable bogus` and `emailable batch bogus` fail as +// invalid input instead of printing help (or exiting 1). +func unknownSubcommand(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + // Arg validation runs before PersistentPreRunE, which is where --jq and + // the configured output mode normally take effect. + if jqExpr != "" { + jsonOutput = true + } else if !cmd.Flags().Changed("json") { + applyConfiguredOutput() + } + msg := fmt.Sprintf("unknown command %q for %q", args[0], cmd.CommandPath()) + if cmd.SuggestionsMinimumDistance <= 0 { + cmd.SuggestionsMinimumDistance = 2 // cobra's own default for this message + } + if suggestions := cmd.SuggestionsFor(args[0]); len(suggestions) > 0 { + msg += fmt.Sprintf("; did you mean %q?", suggestions[0]) + } + return NewInvalidInput(msg) +} + +// showHelp is the RunE for grouping commands, so they stay runnable and +// unknownSubcommand gets a chance to reject stray args. +func showHelp(cmd *cobra.Command, _ []string) error { + return cmd.Help() +} + +func applyConfiguredOutput() { + if merged, err := env.MergedConfig(); err == nil && strings.EqualFold(merged.Output, "json") { + jsonOutput = true + } +} + +// jsonRequested reports whether args ask for JSON output via --json or --jq, +// and whether either flag appears at all. +func jsonRequested(args []string) (want, explicit bool) { + for i, a := range args { + if a == "--" { + break + } + switch { + case a == "--json": + want, explicit = true, true + case strings.HasPrefix(a, "--json="): + v, err := strconv.ParseBool(strings.TrimPrefix(a, "--json=")) + want, explicit = err == nil && v, true + case a == "--jq" && i+1 < len(args), strings.HasPrefix(a, "--jq="): + return true, true + } + } + return want, explicit +} diff --git a/cmd/usage_errors_test.go b/cmd/usage_errors_test.go new file mode 100644 index 0000000..1bb761a --- /dev/null +++ b/cmd/usage_errors_test.go @@ -0,0 +1,100 @@ +package cmd + +import ( + "net/http" + "strings" + "testing" +) + +func assertInvalidInput(t *testing.T, res *runResult) { + t.Helper() + if res.Err == nil { + t.Fatal("expected an error") + } + if got := errorCode(res.Err); got != codeInvalidInput { + t.Errorf("errorCode: got %q want %q (err: %v)", got, codeInvalidInput, res.Err) + } + if got := exitCode(res.Err); got != exitInput { + t.Errorf("exitCode: got %d want %d", got, exitInput) + } +} + +func TestUsageErrors_InvalidInput(t *testing.T) { + cases := [][]string{ + {"bogus"}, + {"batch", "bogus"}, + {"account", "bogus"}, + {"skill", "bogus"}, + {"--bogus"}, + {"batch", "get", "--bogus", "bch_1"}, + {"verify", "--timeout", "abc", "a@x.com"}, + } + for _, args := range cases { + t.Run(strings.Join(args, " "), func(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) + assertInvalidInput(t, runRoot(t, args...)) + }) + } +} + +func TestUsageErrors_UnknownCommandSuggests(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) + res := runRoot(t, "bacth") + assertInvalidInput(t, res) + if !strings.Contains(res.Err.Error(), `did you mean "batch"`) { + t.Errorf("expected suggestion, got %q", res.Err) + } +} + +func TestUsageErrors_GroupWithoutArgsShowsHelp(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) + res := runRoot(t, "batch") + if res.Err != nil { + t.Fatalf("execute: %v", res.Err) + } + if !strings.Contains(res.Stdout.String(), "USAGE") { + t.Errorf("expected help, got %q", res.Stdout.String()) + } +} + +func TestUsageErrors_UnknownCommandHonorsJQ(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) + res := runRoot(t, "bogus", "--jq", ".") + assertInvalidInput(t, res) + if !jsonOutput { + t.Error("expected --jq to switch the error to JSON mode") + } +} + +func TestJSONRequested(t *testing.T) { + cases := []struct { + args []string + want, explicit bool + }{ + {[]string{"--bogus", "--json"}, true, true}, + {[]string{"--json=false", "--bogus"}, false, true}, + {[]string{"--bogus", "--jq", ".x"}, true, true}, + {[]string{"--bogus", "--jq=.x"}, true, true}, + {[]string{"--bogus"}, false, false}, + {[]string{"--", "--json"}, false, false}, + } + for _, tc := range cases { + want, explicit := jsonRequested(tc.args) + if want != tc.want || explicit != tc.explicit { + t.Errorf("jsonRequested(%v) = %v,%v want %v,%v", tc.args, want, explicit, tc.want, tc.explicit) + } + } +} + +func TestFlagUsageError_JSONAfterBadFlag(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) + prev := cliArgs + cliArgs = func() []string { return []string{"--bogus", "--json"} } + t.Cleanup(func() { cliArgs = prev }) + + res := runRoot(t, "--bogus", "--json") + assertInvalidInput(t, res) + if !jsonOutput { + t.Error("expected --json after the bad flag to enable JSON errors") + } +} diff --git a/internal/api/download.go b/internal/api/download.go new file mode 100644 index 0000000..cd1c53f --- /dev/null +++ b/internal/api/download.go @@ -0,0 +1,45 @@ +package api + +import ( + "context" + "fmt" + "io" + "net/http" + "time" +) + +// downloadTimeout covers fetching a large batch's results file. +const downloadTimeout = 5 * time.Minute + +// Download is a fetched batch results file. +type Download struct { + URL string + ContentType string + Body []byte +} + +// FetchDownload GETs a batch's download_file URL. It sends no Authorization +// header: the URL is a presigned storage link, and forwarding the API token to +// the storage host would leak it. hc nil uses a client with downloadTimeout. +func FetchDownload(ctx context.Context, rawURL string, hc *http.Client) (*Download, error) { + if hc == nil { + hc = &http.Client{Timeout: downloadTimeout} + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil) + if err != nil { + return nil, fmt.Errorf("build download request: %w", err) + } + resp, err := hc.Do(req) + if err != nil { + return nil, fmt.Errorf("download results: %w", err) + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return nil, fmt.Errorf("download results: HTTP %d (the link expires; fetch the batch again for a fresh one)", resp.StatusCode) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("download results: %w", err) + } + return &Download{URL: rawURL, ContentType: resp.Header.Get("Content-Type"), Body: body}, nil +} diff --git a/internal/api/download_test.go b/internal/api/download_test.go new file mode 100644 index 0000000..a1972f6 --- /dev/null +++ b/internal/api/download_test.go @@ -0,0 +1,42 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func TestFetchDownload_NoAuthorization(t *testing.T) { + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + w.Header().Set("Content-Type", "application/zip") + _, _ = w.Write([]byte("body")) + })) + defer srv.Close() + + d, err := FetchDownload(context.Background(), srv.URL+"/f.csv?sig=x", nil) + if err != nil { + t.Fatal(err) + } + if gotAuth != "" { + t.Errorf("Authorization header sent to storage host: %q", gotAuth) + } + if string(d.Body) != "body" || d.ContentType != "application/zip" { + t.Errorf("unexpected download: %+v", d) + } +} + +func TestFetchDownload_HTTPError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + })) + defer srv.Close() + + _, err := FetchDownload(context.Background(), srv.URL, nil) + if err == nil || !strings.Contains(err.Error(), "HTTP 403") { + t.Errorf("expected HTTP 403 error, got %v", err) + } +} diff --git a/internal/output/download.go b/internal/output/download.go new file mode 100644 index 0000000..9696e3e --- /dev/null +++ b/internal/output/download.go @@ -0,0 +1,290 @@ +package output + +import ( + "archive/zip" + "bytes" + "encoding/csv" + "encoding/json" + "errors" + "fmt" + "io" + "net/url" + "path" + "strconv" + "strings" + "unicode/utf8" + + "github.com/emailable/emailable-cli/internal/api" +) + +// The API documents download_file as a ZIP compressed CSV, but its example +// URL ends in .csv, so the format is sniffed from the body rather than trusted. +const ( + formatCSV = "csv" + formatJSON = "json" +) + +var ( + zipMagic = []byte("PK\x03\x04") + utf8BOM = []byte("\xef\xbb\xbf") +) + +// Field types from the verify response docs, used to restore types when a +// downloaded CSV is converted to JSON. +var ( + boolFields = map[string]bool{"accept_all": true, "disposable": true, "free": true, "mailbox_full": true, "no_reply": true, "role": true} + intFields = map[string]bool{"score": true, "birth_year": true} + floatFields = map[string]bool{"duration": true} +) + +// WriteDownload converts a batch's downloaded results file to the format +// opts.Path asks for (CSV or JSON) and writes it atomically. batch supplies +// the surrounding document for JSON output. Returns the row count. +func WriteDownload(d *api.Download, batch *api.BatchStatus, opts SaveOptions) (int, error) { + if opts.Path == "" { + return 0, fmt.Errorf("output path is required") + } + data, name, err := unzipDownload(d.Body, d.ContentType) + if err != nil { + return 0, err + } + if name == "" { + name = urlPath(d.URL) + } + wantCSV := useCSV(opts) + + format := sniffFormat(data, d.ContentType) + if format == "" { + return writeUnknownDownload(data, name, wantCSV, opts.Path) + } + + rows, header, err := parseDownload(data, format) + if err != nil { + return 0, err + } + + if wantCSV { + if format == formatCSV && !hasKnownColumn(header) { + // Columns we don't recognize: keep the file exactly as served + // rather than emit a sheet of blank known columns. + if err := atomicWrite(opts.Path, data); err != nil { + return 0, err + } + return len(rows), nil + } + return writeCSV(rows, opts.Path) + } + + doc, err := batchDocument(batch) + if err != nil { + return 0, err + } + doc["emails"] = rows + out, err := json.MarshalIndent(doc, "", " ") + if err != nil { + return 0, fmt.Errorf("marshal json: %w", err) + } + if err := atomicWrite(opts.Path, append(out, '\n')); err != nil { + return 0, err + } + return len(rows), nil +} + +// unzipDownload returns the CSV inside a ZIP body, or body unchanged when it +// isn't a ZIP. name is the archive entry's name, when there is one. +func unzipDownload(body []byte, contentType string) ([]byte, string, error) { + if !bytes.HasPrefix(body, zipMagic) && !strings.Contains(strings.ToLower(contentType), "zip") { + return body, "", nil + } + zr, err := zip.NewReader(bytes.NewReader(body), int64(len(body))) + if err != nil { + return nil, "", fmt.Errorf("open downloaded zip: %w", err) + } + var files []*zip.File + for _, f := range zr.File { + if f.FileInfo().IsDir() || strings.HasPrefix(f.Name, "__MACOSX/") { + continue + } + files = append(files, f) + } + var pick *zip.File + for _, f := range files { + if strings.EqualFold(path.Ext(f.Name), ".csv") { + pick = f + break + } + } + if pick == nil && len(files) == 1 { + pick = files[0] + } + if pick == nil { + return nil, "", fmt.Errorf("downloaded zip has no results file (%d entries)", len(files)) + } + rc, err := pick.Open() + if err != nil { + return nil, "", fmt.Errorf("open %s in downloaded zip: %w", pick.Name, err) + } + defer rc.Close() + data, err := io.ReadAll(rc) + if err != nil { + return nil, "", fmt.Errorf("read %s in downloaded zip: %w", pick.Name, err) + } + return data, pick.Name, nil +} + +// sniffFormat returns formatCSV, formatJSON, or "" when the body is neither. +func sniffFormat(data []byte, contentType string) string { + trimmed := bytes.TrimLeft(bytes.TrimPrefix(data, utf8BOM), " \t\r\n") + if len(trimmed) == 0 { + return "" + } + if trimmed[0] == '[' || trimmed[0] == '{' || strings.Contains(strings.ToLower(contentType), "json") { + if json.Valid(trimmed) { + return formatJSON + } + return "" + } + if !utf8.Valid(trimmed) || bytes.IndexByte(trimmed, 0) >= 0 { + return "" + } + r := csv.NewReader(bytes.NewReader(trimmed)) + r.FieldsPerRecord = -1 + if _, err := r.Read(); err != nil { + return "" + } + return formatCSV +} + +func parseDownload(data []byte, format string) ([]record, []string, error) { + data = bytes.TrimPrefix(data, utf8BOM) + if format == formatJSON { + rows, err := parseJSONDownload(data) + return rows, nil, err + } + r := csv.NewReader(bytes.NewReader(data)) + r.FieldsPerRecord = -1 + all, err := r.ReadAll() + if err != nil { + return nil, nil, fmt.Errorf("parse downloaded csv: %w", err) + } + if len(all) == 0 { + return []record{}, nil, nil + } + header := all[0] + rows := make([]record, 0, len(all)-1) + for _, line := range all[1:] { + rec := make(record, len(header)) + for i, col := range header { + if i < len(line) { + rec[col] = typedCell(col, line[i]) + } else { + rec[col] = nil + } + } + rows = append(rows, rec) + } + return rows, header, nil +} + +func parseJSONDownload(data []byte) ([]record, error) { + var v any + if err := decodeNumbers(data, &v); err != nil { + return nil, fmt.Errorf("parse downloaded json: %w", err) + } + if obj, ok := v.(map[string]any); ok { + v = obj["emails"] + } + list, ok := v.([]any) + if !ok { + return nil, errors.New("downloaded json has no list of results") + } + rows := make([]record, 0, len(list)) + for _, item := range list { + obj, ok := item.(map[string]any) + if !ok { + return nil, errors.New("downloaded json results are not objects") + } + rows = append(rows, record(obj)) + } + return rows, nil +} + +// typedCell restores the documented type of a CSV cell; blank means null. +func typedCell(col, s string) any { + if s == "" { + return nil + } + switch { + case boolFields[col]: + if b, err := strconv.ParseBool(s); err == nil { + return b + } + case intFields[col]: + if _, err := strconv.ParseInt(s, 10, 64); err == nil { + return json.Number(s) + } + case floatFields[col]: + if _, err := strconv.ParseFloat(s, 64); err == nil { + return json.Number(s) + } + } + return s +} + +func hasKnownColumn(header []string) bool { + for _, h := range header { + for _, c := range CSVColumns { + if h == c { + return true + } + } + } + return false +} + +// batchDocument is the batch payload as the API sent it, so a JSON save of a +// downloaded batch has the same shape as one with inline emails. +func batchDocument(b *api.BatchStatus) (map[string]any, error) { + if b == nil { + return map[string]any{}, nil + } + if raw := b.RawJSON(); len(raw) > 0 { + var doc map[string]any + if err := decodeNumbers(raw, &doc); err == nil && doc != nil { + return doc, nil + } + } + return typedRecord(b) +} + +// writeUnknownDownload saves bytes we can't parse only when their own +// extension already matches what the user asked for. +func writeUnknownDownload(data []byte, name string, wantCSV bool, dest string) (int, error) { + ext := strings.ToLower(path.Ext(name)) + if (wantCSV && ext == ".csv") || (!wantCSV && ext == ".json") { + if err := atomicWrite(dest, data); err != nil { + return 0, err + } + return 0, nil + } + want := "JSON" + if wantCSV { + want = "CSV" + } + return 0, fmt.Errorf("can't convert the downloaded results file (%s) to %s: unrecognized format", displayName(name), want) +} + +func urlPath(raw string) string { + u, err := url.Parse(raw) + if err != nil { + return "" + } + return u.Path +} + +func displayName(name string) string { + if name == "" { + return "no file name" + } + return path.Base(name) +} diff --git a/internal/output/download_test.go b/internal/output/download_test.go new file mode 100644 index 0000000..ea73830 --- /dev/null +++ b/internal/output/download_test.go @@ -0,0 +1,154 @@ +package output + +import ( + "archive/zip" + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/emailable/emailable-cli/internal/api" +) + +const downloadCSV = "email,state,score,free,duration,extra_col\n" + + "a@x.com,deliverable,95,true,0.5,foo\n" + + "b@y.com,undeliverable,0,,,\n" + +func zipped(t *testing.T, name, body string) []byte { + t.Helper() + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + f, err := zw.Create(name) + if err != nil { + t.Fatal(err) + } + if _, err := f.Write([]byte(body)); err != nil { + t.Fatal(err) + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +func downloadBatch(t *testing.T) *api.BatchStatus { + b, _ := fetchViaAPI(t, `{"id":"bch_1","message":"Batch verification completed.","download_file":"https://cdn.example/bch_1.csv?sig=x","total_counts":{"processed":2,"total":2}}`) + return b +} + +func TestWriteDownload_ZipToCSV(t *testing.T) { + path := filepath.Join(t.TempDir(), "results.csv") + d := &api.Download{URL: "https://cdn.example/bch_1.csv", ContentType: "application/zip", Body: zipped(t, "bch_1.csv", downloadCSV)} + n, err := WriteDownload(d, downloadBatch(t), SaveOptions{Path: path}) + if err != nil { + t.Fatal(err) + } + if n != 2 { + t.Errorf("count: got %d want 2", n) + } + rows := readCSV(t, path) + want := append(append([]string(nil), CSVColumns...), "extra_col") + if strings.Join(rows[0], ",") != strings.Join(want, ",") { + t.Fatalf("header: got %v", rows[0]) + } + col := columnIndex(rows[0]) + if rows[1][col["email"]] != "a@x.com" || rows[1][col["score"]] != "95" || rows[1][col["free"]] != "true" || rows[1][col["extra_col"]] != "foo" { + t.Errorf("row 1: %v", rows[1]) + } + if rows[2][col["free"]] != "" || rows[2][col["reason"]] != "" { + t.Errorf("row 2 should have blanks: %v", rows[2]) + } +} + +func TestWriteDownload_PlainCSVToJSON(t *testing.T) { + path := filepath.Join(t.TempDir(), "results.json") + d := &api.Download{URL: "https://cdn.example/bch_1.csv", ContentType: "text/csv", Body: []byte(downloadCSV)} + n, err := WriteDownload(d, downloadBatch(t), SaveOptions{Path: path}) + if err != nil { + t.Fatal(err) + } + if n != 2 { + t.Errorf("count: got %d want 2", n) + } + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var doc struct { + ID string `json:"id"` + Emails []map[string]any `json:"emails"` + } + if err := json.Unmarshal(data, &doc); err != nil { + t.Fatalf("unmarshal: %v\n%s", err, data) + } + if doc.ID != "bch_1" || len(doc.Emails) != 2 { + t.Fatalf("unexpected doc: %s", data) + } + first := doc.Emails[0] + if first["score"] != float64(95) || first["free"] != true || first["duration"] != 0.5 || first["extra_col"] != "foo" { + t.Errorf("types not restored: %v", first) + } + if v, ok := doc.Emails[1]["free"]; !ok || v != nil { + t.Errorf("blank cell should be null, got %v (present=%v)", v, ok) + } +} + +func TestWriteDownload_UnrecognizedColumnsPassThrough(t *testing.T) { + path := filepath.Join(t.TempDir(), "results.csv") + body := "Address,Result\na@x.com,ok\n" + d := &api.Download{URL: "https://cdn.example/x.csv", Body: []byte(body)} + n, err := WriteDownload(d, downloadBatch(t), SaveOptions{Path: path}) + if err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("count: got %d want 1", n) + } + data, _ := os.ReadFile(path) + if string(data) != body { + t.Errorf("expected bytes as served, got %q", data) + } +} + +func TestWriteDownload_UnknownFormat(t *testing.T) { + binary := []byte{0x00, 0x01, 0xff, 0xfe} + dir := t.TempDir() + + // Extension mismatch: refuse rather than write garbage. + d := &api.Download{URL: "https://cdn.example/x.bin", Body: binary} + if _, err := WriteDownload(d, downloadBatch(t), SaveOptions{Path: filepath.Join(dir, "r.csv")}); err == nil || !strings.Contains(err.Error(), "unrecognized format") { + t.Errorf("expected unrecognized format error, got %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "r.csv")); !os.IsNotExist(err) { + t.Errorf("no file should be written on error") + } + + // Extension match: write as-is. + d = &api.Download{URL: "https://cdn.example/x.csv", Body: binary} + if _, err := WriteDownload(d, downloadBatch(t), SaveOptions{Path: filepath.Join(dir, "r.csv")}); err != nil { + t.Fatalf("expected as-is write, got %v", err) + } + data, _ := os.ReadFile(filepath.Join(dir, "r.csv")) + if !bytes.Equal(data, binary) { + t.Errorf("bytes changed: %v", data) + } +} + +func TestWriteDownload_JSONBody(t *testing.T) { + path := filepath.Join(t.TempDir(), "results.csv") + d := &api.Download{Body: []byte(`[{"email":"a@x.com","state":"deliverable","tag":null}]`), ContentType: "application/json"} + n, err := WriteDownload(d, downloadBatch(t), SaveOptions{Path: path}) + if err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("count: got %d want 1", n) + } + rows := readCSV(t, path) + col := columnIndex(rows[0]) + if rows[1][col["state"]] != "deliverable" || rows[1][col["tag"]] != "" { + t.Errorf("row: %v", rows[1]) + } +} diff --git a/internal/output/file.go b/internal/output/file.go index 9be9e55..9617e87 100644 --- a/internal/output/file.go +++ b/internal/output/file.go @@ -1,10 +1,13 @@ package output import ( + "bytes" "encoding/csv" + "encoding/json" "fmt" "os" "path/filepath" + "sort" "strconv" "strings" @@ -18,44 +21,34 @@ type SaveOptions struct { Stderr *os.File // nil means os.Stderr } -// Duration and other floats are omitted — not useful in a spreadsheet. -var csvHeader = []string{ - "email", "state", "score", "reason", "domain", - "disposable", "accept_all", "role", "free", - "mx_record", "smtp_provider", "did_you_mean", - "first_name", "last_name", "gender", +// CSVColumns is the fixed CSV column order: every documented field of the +// verify response, identity first, then name parts, then flags and server +// details. Keys outside this list are appended after it, sorted by name. +var CSVColumns = []string{ + "email", "state", "reason", "score", "domain", "user", + "first_name", "last_name", "full_name", "gender", "birth_year", + "free", "role", "disposable", "accept_all", "mailbox_full", "no_reply", + "did_you_mean", "mx_record", "smtp_provider", "tag", "duration", } +// record is one verify result decoded from the API's JSON, so a key the API +// omitted (e.g. via response_fields) or sent as null stays distinguishable +// from a zero value. +type record map[string]any + // WriteResults writes v to opts.Path atomically and returns the row count. // Unknown extensions fall back to JSON with a stderr note. func WriteResults(v any, opts SaveOptions) (int, error) { if opts.Path == "" { return 0, fmt.Errorf("output path is required") } - stderr := opts.Stderr - if stderr == nil { - stderr = os.Stderr - } - - ext := strings.ToLower(filepath.Ext(opts.Path)) - - useCSV := false - switch { - case opts.ForceJSON: - useCSV = false - case ext == ".csv": - useCSV = true - case ext == ".json": - useCSV = false - default: - fmt.Fprintln(stderr, "note: unrecognized extension; writing JSON") - useCSV = false - } - - if useCSV { - rows, ok := flattenForCSV(v) + if useCSV(opts) { + rows, ok, err := recordsFor(v) + if err != nil { + return 0, err + } if !ok { - fmt.Fprintln(stderr, "note: data shape not supported for CSV; writing JSON") + fmt.Fprintln(stderrFor(opts), "note: data shape not supported for CSV; writing JSON") return writeJSON(v, opts.Path) } return writeCSV(rows, opts.Path) @@ -63,27 +56,114 @@ func WriteResults(v any, opts SaveOptions) (int, error) { return writeJSON(v, opts.Path) } -func flattenForCSV(v any) ([]api.VerifyResult, bool) { +func stderrFor(opts SaveOptions) *os.File { + if opts.Stderr != nil { + return opts.Stderr + } + return os.Stderr +} + +func useCSV(opts SaveOptions) bool { + if opts.ForceJSON { + return false + } + switch strings.ToLower(filepath.Ext(opts.Path)) { + case ".csv": + return true + case ".json": + return false + default: + fmt.Fprintln(stderrFor(opts), "note: unrecognized extension; writing JSON") + return false + } +} + +// recordsFor builds CSV rows from the raw API body when available, falling +// back to re-encoding the typed struct for values built in code. +func recordsFor(v any) ([]record, bool, error) { switch t := v.(type) { case *api.VerifyResult: if t == nil { - return nil, true + return nil, true, nil } - return []api.VerifyResult{*t}, true + r, err := verifyRecord(t) + if err != nil { + return nil, true, err + } + return []record{r}, true, nil case api.VerifyResult: - return []api.VerifyResult{t}, true + return recordsFor(&t) case *api.BatchStatus: if t == nil { - return nil, true + return nil, true, nil } - return t.Emails, true + rows, err := batchRecords(t) + return rows, true, err case api.BatchStatus: - return t.Emails, true + return recordsFor(&t) case []api.VerifyResult: - return t, true + rows := make([]record, 0, len(t)) + for i := range t { + r, err := verifyRecord(&t[i]) + if err != nil { + return nil, true, err + } + rows = append(rows, r) + } + return rows, true, nil default: - return nil, false + return nil, false, nil + } +} + +func verifyRecord(r *api.VerifyResult) (record, error) { + if raw := r.RawJSON(); len(raw) > 0 { + var rec record + if err := decodeNumbers(raw, &rec); err == nil && rec != nil { + return rec, nil + } + } + return typedRecord(r) +} + +func batchRecords(b *api.BatchStatus) ([]record, error) { + if raw := b.RawJSON(); len(raw) > 0 { + var doc struct { + Emails []record `json:"emails"` + } + if err := decodeNumbers(raw, &doc); err == nil { + return doc.Emails, nil + } + } + rows := make([]record, 0, len(b.Emails)) + for i := range b.Emails { + r, err := verifyRecord(&b.Emails[i]) + if err != nil { + return nil, err + } + rows = append(rows, r) + } + return rows, nil +} + +func typedRecord(v any) (record, error) { + b, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("marshal result: %w", err) } + var rec record + if err := decodeNumbers(b, &rec); err != nil { + return nil, fmt.Errorf("decode result: %w", err) + } + return rec, nil +} + +// decodeNumbers keeps numbers as json.Number so scores and durations render +// exactly as the API sent them. +func decodeNumbers(data []byte, v any) error { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + return dec.Decode(v) } func resultCount(v any) int { @@ -120,59 +200,87 @@ func writeJSON(v any, path string) (int, error) { return resultCount(v), nil } -func writeCSV(rows []api.VerifyResult, path string) (int, error) { - tmp := path + ".tmp" - f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) - if err != nil { - return 0, fmt.Errorf("create %s: %w", tmp, err) +// csvHeaderFor returns CSVColumns followed by any other keys present in +// rows, sorted by name. +func csvHeaderFor(rows []record) []string { + known := make(map[string]bool, len(CSVColumns)) + for _, c := range CSVColumns { + known[c] = true } - cleanup := true - defer func() { - if cleanup { - _ = os.Remove(tmp) + extra := map[string]bool{} + for _, r := range rows { + for k := range r { + if !known[k] { + extra[k] = true + } } - }() + } + names := make([]string, 0, len(extra)) + for k := range extra { + names = append(names, k) + } + sort.Strings(names) + header := append([]string(nil), CSVColumns...) + return append(header, names...) +} - w := csv.NewWriter(f) - if err := w.Write(csvHeader); err != nil { - f.Close() - return 0, fmt.Errorf("write csv header: %w", err) +// csvCell renders one value; absent and null are both blank. +func csvCell(v any, present bool) string { + if !present || v == nil { + return "" } + switch t := v.(type) { + case string: + return t + case bool: + return strconv.FormatBool(t) + case json.Number: + return t.String() + case float64: + return strconv.FormatFloat(t, 'f', -1, 64) + case int: + return strconv.Itoa(t) + default: + b, err := json.Marshal(t) + if err != nil { + return fmt.Sprint(t) + } + return string(b) + } +} + +func encodeCSV(rows []record) ([]byte, error) { + header := csvHeaderFor(rows) + var buf bytes.Buffer + w := csv.NewWriter(&buf) + if err := w.Write(header); err != nil { + return nil, fmt.Errorf("write csv header: %w", err) + } + rec := make([]string, len(header)) for _, r := range rows { - rec := []string{ - r.Email, - r.State, - strconv.Itoa(r.Score), - r.Reason, - r.Domain, - strconv.FormatBool(r.Disposable), - strconv.FormatBool(r.AcceptAll), - strconv.FormatBool(r.Role), - strconv.FormatBool(r.Free), - r.MXRecord, - r.SMTPProvider, - r.DidYouMean, - r.FirstName, - r.LastName, - r.Gender, + for i, col := range header { + v, ok := r[col] + rec[i] = csvCell(v, ok) } if err := w.Write(rec); err != nil { - f.Close() - return 0, fmt.Errorf("write csv row: %w", err) + return nil, fmt.Errorf("write csv row: %w", err) } } w.Flush() if err := w.Error(); err != nil { - f.Close() - return 0, fmt.Errorf("flush csv: %w", err) + return nil, fmt.Errorf("flush csv: %w", err) } - if err := f.Close(); err != nil { - return 0, fmt.Errorf("close %s: %w", tmp, err) + return buf.Bytes(), nil +} + +func writeCSV(rows []record, path string) (int, error) { + data, err := encodeCSV(rows) + if err != nil { + return 0, err } - if err := os.Rename(tmp, path); err != nil { - return 0, fmt.Errorf("rename %s -> %s: %w", tmp, path, err) + if err := atomicWrite(path, data); err != nil { + return 0, err } - cleanup = false return len(rows), nil } diff --git a/internal/output/file_test.go b/internal/output/file_test.go index c2bf335..c13ccdc 100644 --- a/internal/output/file_test.go +++ b/internal/output/file_test.go @@ -1,8 +1,11 @@ package output import ( + "context" "encoding/csv" "encoding/json" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" @@ -51,18 +54,117 @@ func TestWriteResults_BatchCSV(t *testing.T) { if len(rows) != 4 { // header + 3 t.Fatalf("got %d rows, want 4", len(rows)) } - if rows[0][0] != "email" || rows[0][1] != "state" || rows[0][2] != "score" { + if strings.Join(rows[0], ",") != strings.Join(CSVColumns, ",") { t.Errorf("unexpected header: %v", rows[0]) } - if rows[1][0] != "a@x.com" || rows[1][1] != "deliverable" || rows[1][2] != "100" { + col := columnIndex(rows[0]) + if rows[1][col["email"]] != "a@x.com" || rows[1][col["state"]] != "deliverable" || rows[1][col["score"]] != "100" { t.Errorf("unexpected row 1: %v", rows[1]) } // Booleans render as true/false strings. - if rows[1][8] != "true" { // free - t.Errorf("expected free=true, got %q", rows[1][8]) + if rows[1][col["free"]] != "true" { + t.Errorf("expected free=true, got %q", rows[1][col["free"]]) } - if rows[2][5] != "true" { // disposable - t.Errorf("expected disposable=true, got %q", rows[2][5]) + if rows[2][col["disposable"]] != "true" { + t.Errorf("expected disposable=true, got %q", rows[2][col["disposable"]]) + } +} + +func columnIndex(header []string) map[string]int { + m := make(map[string]int, len(header)) + for i, h := range header { + m[h] = i + } + return m +} + +// fetchViaAPI runs body through the real API client so the result carries +// its raw JSON, as it does in the CLI. +func fetchViaAPI(t *testing.T, body string) (*api.BatchStatus, *api.VerifyResult) { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(srv.Close) + c := api.New(srv.URL, "k", nil) + b, err := c.Batch(context.Background(), "bch_1", false) + if err != nil { + t.Fatal(err) + } + v, err := c.Verify(context.Background(), "a@x.com", nil) + if err != nil { + t.Fatal(err) + } + return b, v +} + +func readCSV(t *testing.T, path string) [][]string { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + rows, err := csv.NewReader(strings.NewReader(string(data))).ReadAll() + if err != nil { + t.Fatal(err) + } + return rows +} + +func TestWriteResults_CSVBlankForAbsentAndNull(t *testing.T) { + // As with --response-fields email,state,free: other fields are absent. + b, _ := fetchViaAPI(t, `{"id":"bch_1","emails":[ + {"email":"a@x.com","state":"deliverable","free":false,"score":0,"tag":null,"duration":0.25}, + {"email":"b@y.com","state":"risky"} + ]}`) + path := filepath.Join(t.TempDir(), "out.csv") + n, err := WriteResults(b, SaveOptions{Path: path}) + if err != nil { + t.Fatal(err) + } + if n != 2 { + t.Errorf("count: got %d want 2", n) + } + rows := readCSV(t, path) + col := columnIndex(rows[0]) + cases := []struct { + row int + col, want string + }{ + {1, "free", "false"}, + {1, "score", "0"}, + {1, "tag", ""}, + {1, "duration", "0.25"}, + {1, "disposable", ""}, + {2, "free", ""}, + {2, "score", ""}, + {2, "state", "risky"}, + } + for _, tc := range cases { + if got := rows[tc.row][col[tc.col]]; got != tc.want { + t.Errorf("row %d %s: got %q want %q", tc.row, tc.col, got, tc.want) + } + } +} + +func TestWriteResults_CSVAppendsUnknownKeysSorted(t *testing.T) { + _, v := fetchViaAPI(t, `{"email":"a@x.com","state":"deliverable","zeta":"z","alpha":{"k":1}}`) + path := filepath.Join(t.TempDir(), "one.csv") + if _, err := WriteResults(v, SaveOptions{Path: path}); err != nil { + t.Fatal(err) + } + rows := readCSV(t, path) + want := append(append([]string(nil), CSVColumns...), "alpha", "zeta") + if strings.Join(rows[0], ",") != strings.Join(want, ",") { + t.Fatalf("header: got %v want %v", rows[0], want) + } + col := columnIndex(rows[0]) + if got := rows[1][col["alpha"]]; got != `{"k":1}` { + t.Errorf("alpha: got %q", got) + } + if got := rows[1][col["zeta"]]; got != "z" { + t.Errorf("zeta: got %q", got) } } From 746b6df59cfe4712a07151e224c54b0f98efb0d4 Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Thu, 1 Oct 2026 16:28:23 +0200 Subject: [PATCH 2/8] Drop unverified claim from the empty batch results error The message asserted an API retention behavior nobody had confirmed. --- cmd/batch.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/batch.go b/cmd/batch.go index 7ea3c7a..e0612e6 100644 --- a/cmd/batch.go +++ b/cmd/batch.go @@ -312,7 +312,7 @@ func saveBatchToFile(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatus, return saveDownloadToFile(cmd, cctx, status, path) } if _, total, ok := status.Progress(); ok && total > 0 && status.IsComplete() && len(status.Emails) == 0 { - return fmt.Errorf("batch %s has no per-email results to save; the API stops returning them some days after completion", batchID) + return fmt.Errorf("batch %s returned no per-email results to save", batchID) } return saveToFile(cmd, cctx, status, path) } From 939b9a17b8399d03eb456d403f7828e2568e97cb Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Thu, 1 Oct 2026 16:48:10 +0200 Subject: [PATCH 3/8] Stream downloaded CSV results row by row A 1M-row `download_file` export was parsed into memory whole before conversion, which could reach gigabytes. CSV output now converts the download one row at a time through the same encoder inline results use, and atomic writes stream to the temp file. --- internal/output/download.go | 104 +++++++++++++++++++++++------- internal/output/file.go | 122 ++++++++++++++++++++++++++---------- 2 files changed, 171 insertions(+), 55 deletions(-) diff --git a/internal/output/download.go b/internal/output/download.go index 9696e3e..58912aa 100644 --- a/internal/output/download.go +++ b/internal/output/download.go @@ -58,20 +58,15 @@ func WriteDownload(d *api.Download, batch *api.BatchStatus, opts SaveOptions) (i return writeUnknownDownload(data, name, wantCSV, opts.Path) } - rows, header, err := parseDownload(data, format) + if wantCSV && format == formatCSV { + return convertCSVDownload(data, opts.Path) + } + + rows, err := parseDownload(data, format) if err != nil { return 0, err } - if wantCSV { - if format == formatCSV && !hasKnownColumn(header) { - // Columns we don't recognize: keep the file exactly as served - // rather than emit a sheet of blank known columns. - if err := atomicWrite(opts.Path, data); err != nil { - return 0, err - } - return len(rows), nil - } return writeCSV(rows, opts.Path) } @@ -155,24 +150,88 @@ func sniffFormat(data []byte, contentType string) string { return formatCSV } -func parseDownload(data []byte, format string) ([]record, []string, error) { +// convertCSVDownload rewrites a downloaded CSV into the standard column +// layout one row at a time, so even a 1M-row export is never held in memory +// as parsed rows. +func convertCSVDownload(data []byte, dest string) (int, error) { + r, header, err := downloadCSVReader(data) + if err != nil { + return 0, err + } + if !hasKnownColumn(header) { + // Columns we don't recognize: keep the file exactly as served rather + // than emit a sheet of blank known columns. + n := 0 + for { + if _, err := r.Read(); err == io.EOF { + break + } else if err != nil { + return 0, fmt.Errorf("parse downloaded csv: %w", err) + } + n++ + } + if err := atomicWrite(dest, data); err != nil { + return 0, err + } + return n, nil + } + present := make(map[string]bool, len(header)) + for _, h := range header { + present[h] = true + } + return writeCSVRows(dest, csvHeaderWith(present), csvDownloadRows(r, header)) +} + +// parseDownload reads every result row, for JSON output where the whole +// document has to be built anyway. +func parseDownload(data []byte, format string) ([]record, error) { data = bytes.TrimPrefix(data, utf8BOM) if format == formatJSON { - rows, err := parseJSONDownload(data) - return rows, nil, err + return parseJSONDownload(data) + } + r, header, err := downloadCSVReader(data) + if err != nil { + return nil, err + } + next := csvDownloadRows(r, header) + rows := []record{} + for { + rec, ok, err := next() + if err != nil { + return nil, err + } + if !ok { + return rows, nil + } + rows = append(rows, rec) } - r := csv.NewReader(bytes.NewReader(data)) +} + +// downloadCSVReader returns a reader positioned after the header row. An +// empty file yields a nil header and a reader at EOF. +func downloadCSVReader(data []byte) (*csv.Reader, []string, error) { + r := csv.NewReader(bytes.NewReader(bytes.TrimPrefix(data, utf8BOM))) r.FieldsPerRecord = -1 - all, err := r.ReadAll() + r.ReuseRecord = true + header, err := r.Read() + if err == io.EOF { + return r, nil, nil + } if err != nil { return nil, nil, fmt.Errorf("parse downloaded csv: %w", err) } - if len(all) == 0 { - return []record{}, nil, nil - } - header := all[0] - rows := make([]record, 0, len(all)-1) - for _, line := range all[1:] { + return r, append([]string(nil), header...), nil +} + +func csvDownloadRows(r *csv.Reader, header []string) rowSource { + return func() (record, bool, error) { + line, err := r.Read() + if err == io.EOF { + return nil, false, nil + } + if err != nil { + return nil, false, fmt.Errorf("parse downloaded csv: %w", err) + } rec := make(record, len(header)) for i, col := range header { if i < len(line) { @@ -181,9 +240,8 @@ func parseDownload(data []byte, format string) ([]record, []string, error) { rec[col] = nil } } - rows = append(rows, rec) + return rec, true, nil } - return rows, header, nil } func parseJSONDownload(data []byte) ([]record, error) { diff --git a/internal/output/file.go b/internal/output/file.go index 9617e87..ec3090b 100644 --- a/internal/output/file.go +++ b/internal/output/file.go @@ -1,10 +1,12 @@ package output import ( + "bufio" "bytes" "encoding/csv" "encoding/json" "fmt" + "io" "os" "path/filepath" "sort" @@ -203,21 +205,27 @@ func writeJSON(v any, path string) (int, error) { // csvHeaderFor returns CSVColumns followed by any other keys present in // rows, sorted by name. func csvHeaderFor(rows []record) []string { - known := make(map[string]bool, len(CSVColumns)) - for _, c := range CSVColumns { - known[c] = true - } extra := map[string]bool{} for _, r := range rows { for k := range r { - if !known[k] { - extra[k] = true - } + extra[k] = true } } - names := make([]string, 0, len(extra)) - for k := range extra { - names = append(names, k) + return csvHeaderWith(extra) +} + +// csvHeaderWith returns CSVColumns followed by the keys of present that +// aren't already among them, sorted by name. +func csvHeaderWith(present map[string]bool) []string { + known := make(map[string]bool, len(CSVColumns)) + for _, c := range CSVColumns { + known[c] = true + } + names := make([]string, 0, len(present)) + for k := range present { + if !known[k] { + names = append(names, k) + } } sort.Strings(names) header := append([]string(nil), CSVColumns...) @@ -249,50 +257,100 @@ func csvCell(v any, present bool) string { } } -func encodeCSV(rows []record) ([]byte, error) { - header := csvHeaderFor(rows) - var buf bytes.Buffer - w := csv.NewWriter(&buf) - if err := w.Write(header); err != nil { - return nil, fmt.Errorf("write csv header: %w", err) +// rowSource yields records one at a time, so a large download converts row +// by row instead of being held in memory whole. ok=false ends the stream. +type rowSource func() (rec record, ok bool, err error) + +func sliceRows(rows []record) rowSource { + i := 0 + return func() (record, bool, error) { + if i >= len(rows) { + return nil, false, nil + } + i++ + return rows[i-1], true, nil } - rec := make([]string, len(header)) - for _, r := range rows { +} + +func encodeCSV(w io.Writer, header []string, next rowSource) (int, error) { + cw := csv.NewWriter(w) + if err := cw.Write(header); err != nil { + return 0, fmt.Errorf("write csv header: %w", err) + } + n := 0 + line := make([]string, len(header)) + for { + r, ok, err := next() + if err != nil { + return n, err + } + if !ok { + break + } for i, col := range header { - v, ok := r[col] - rec[i] = csvCell(v, ok) + v, present := r[col] + line[i] = csvCell(v, present) } - if err := w.Write(rec); err != nil { - return nil, fmt.Errorf("write csv row: %w", err) + if err := cw.Write(line); err != nil { + return n, fmt.Errorf("write csv row: %w", err) } + n++ } - w.Flush() - if err := w.Error(); err != nil { - return nil, fmt.Errorf("flush csv: %w", err) + cw.Flush() + if err := cw.Error(); err != nil { + return n, fmt.Errorf("flush csv: %w", err) } - return buf.Bytes(), nil + return n, nil } func writeCSV(rows []record, path string) (int, error) { - data, err := encodeCSV(rows) + return writeCSVRows(path, csvHeaderFor(rows), sliceRows(rows)) +} + +func writeCSVRows(path string, header []string, next rowSource) (int, error) { + var n int + err := atomicWriteWith(path, func(w io.Writer) error { + var err error + n, err = encodeCSV(w, header, next) + return err + }) if err != nil { return 0, err } - if err := atomicWrite(path, data); err != nil { - return 0, err - } - return len(rows), nil + return n, nil } func atomicWrite(path string, data []byte) error { + return atomicWriteWith(path, func(w io.Writer) error { + _, err := w.Write(data) + return err + }) +} + +// atomicWriteWith streams fill's output to a temp file beside path, then +// renames it into place so a failed write never leaves a partial file. +func atomicWriteWith(path string, fill func(io.Writer) error) error { tmp := path + ".tmp" + f, err := os.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + if err != nil { + return fmt.Errorf("write %s: %w", tmp, err) + } cleanup := true defer func() { if cleanup { _ = os.Remove(tmp) } }() - if err := os.WriteFile(tmp, data, 0o644); err != nil { + bw := bufio.NewWriter(f) + if err := fill(bw); err != nil { + _ = f.Close() + return err + } + if err := bw.Flush(); err != nil { + _ = f.Close() + return fmt.Errorf("write %s: %w", tmp, err) + } + if err := f.Close(); err != nil { return fmt.Errorf("write %s: %w", tmp, err) } if err := os.Rename(tmp, path); err != nil { From 22c3e85d4f162193c29d2fae3cce981270aac85f Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Thu, 1 Oct 2026 16:48:11 +0200 Subject: [PATCH 4/8] Classify every positional-arg error as invalid input Leaf commands using plain `cobra.NoArgs` still exited 1 with `unknown`, so the root now wraps every command's `Args` check instead of relying on each command to opt in. A trailing `--json=false` after a bad flag also now wins over an earlier `--json`. --- cmd/batch.go | 4 ++-- cmd/login.go | 2 +- cmd/root.go | 1 + cmd/usage_errors.go | 41 ++++++++++++++++++++++++++++++---------- cmd/usage_errors_test.go | 28 +++++++++++++++++++++++++++ cmd/verify.go | 2 +- 6 files changed, 64 insertions(+), 14 deletions(-) diff --git a/cmd/batch.go b/cmd/batch.go index e0612e6..4fc18fd 100644 --- a/cmd/batch.go +++ b/cmd/batch.go @@ -36,7 +36,7 @@ func newBatchCmd() *cobra.Command { "once complete. Use `--wait` to poll until completion, or " + "`--partial` to include partial results while still verifying " + "(batches ≤ 1,000 emails only).", - Args: wrapInvalidInputArgs(cobra.ExactArgs(1)), + Args: cobra.ExactArgs(1), SilenceUsage: true, Example: ` # Get the latest status / results for a batch emailable batch get bch_123 @@ -91,7 +91,7 @@ func newBatchCmd() *cobra.Command { Long: "Verify a batch of emails. Accepts one or more emails or `.csv` / " + "`.json` / `.txt` files. Prints the batch ID; use `--wait` to poll " + "until complete.", - Args: wrapInvalidInputArgs(cobra.MinimumNArgs(1)), + Args: cobra.MinimumNArgs(1), SilenceUsage: true, Example: ` # Verify a CSV file and block until results are ready emailable batch verify emails.csv --wait diff --git a/cmd/login.go b/cmd/login.go index 7aa70d1..13a819c 100644 --- a/cmd/login.go +++ b/cmd/login.go @@ -23,7 +23,7 @@ func newLoginCmd() *cobra.Command { cmd := &cobra.Command{ Use: "login", Short: "Log in to your Emailable account", - Args: wrapInvalidInputArgs(cobra.NoArgs), + Args: cobra.NoArgs, SilenceUsage: true, Example: ` # Interactive OAuth device login emailable login diff --git a/cmd/root.go b/cmd/root.go index 712fea4..eec7d5a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -253,6 +253,7 @@ func newRootCmd(v string) *cobra.Command { skillSub.GroupID = groupExtras root.AddCommand(verify, batch, account, login, logout, status, versionSub, skillSub, newManCmd()) + wrapArgsTree(root) root.CompletionOptions.HiddenDefaultCmd = true diff --git a/cmd/usage_errors.go b/cmd/usage_errors.go index c1c4cd1..c006950 100644 --- a/cmd/usage_errors.go +++ b/cmd/usage_errors.go @@ -18,9 +18,9 @@ var cliArgs = func() []string { return os.Args[1:] } // raw args are scanned for them so the error still renders as JSON. func flagUsageError(_ *cobra.Command, err error) error { if want, explicit := jsonRequested(cliArgs()); explicit { - if want { - jsonOutput = true - } + // The last --json/--jq on the line wins, even over a --json parsed + // before the bad flag. + jsonOutput = want } else { applyConfiguredOutput() } @@ -34,13 +34,6 @@ func unknownSubcommand(cmd *cobra.Command, args []string) error { if len(args) == 0 { return nil } - // Arg validation runs before PersistentPreRunE, which is where --jq and - // the configured output mode normally take effect. - if jqExpr != "" { - jsonOutput = true - } else if !cmd.Flags().Changed("json") { - applyConfiguredOutput() - } msg := fmt.Sprintf("unknown command %q for %q", args[0], cmd.CommandPath()) if cmd.SuggestionsMinimumDistance <= 0 { cmd.SuggestionsMinimumDistance = 2 // cobra's own default for this message @@ -51,12 +44,40 @@ func unknownSubcommand(cmd *cobra.Command, args []string) error { return NewInvalidInput(msg) } +// wrapArgsTree makes every command's positional-arg check fail as invalid +// input (exit 4), so a new command can't forget to. +func wrapArgsTree(c *cobra.Command) { + if fn := c.Args; fn != nil { + c.Args = func(cmd *cobra.Command, args []string) error { + if err := fn(cmd, args); err != nil { + applyEarlyOutputMode(cmd) + return NewInvalidInput(err.Error()) + } + return nil + } + } + for _, sub := range c.Commands() { + wrapArgsTree(sub) + } +} + // showHelp is the RunE for grouping commands, so they stay runnable and // unknownSubcommand gets a chance to reject stray args. func showHelp(cmd *cobra.Command, _ []string) error { return cmd.Help() } +// applyEarlyOutputMode settles JSON mode for an error raised during arg +// validation, which runs before PersistentPreRunE applies --jq and the +// configured output mode. +func applyEarlyOutputMode(cmd *cobra.Command) { + if jqExpr != "" { + jsonOutput = true + } else if !cmd.Flags().Changed("json") { + applyConfiguredOutput() + } +} + func applyConfiguredOutput() { if merged, err := env.MergedConfig(); err == nil && strings.EqualFold(merged.Output, "json") { jsonOutput = true diff --git a/cmd/usage_errors_test.go b/cmd/usage_errors_test.go index 1bb761a..5e77396 100644 --- a/cmd/usage_errors_test.go +++ b/cmd/usage_errors_test.go @@ -28,6 +28,11 @@ func TestUsageErrors_InvalidInput(t *testing.T) { {"--bogus"}, {"batch", "get", "--bogus", "bch_1"}, {"verify", "--timeout", "abc", "a@x.com"}, + {"status", "extra"}, + {"logout", "extra"}, + {"version", "extra"}, + {"account", "status", "extra"}, + {"skill", "print", "extra"}, } for _, args := range cases { t.Run(strings.Join(args, " "), func(t *testing.T) { @@ -98,3 +103,26 @@ func TestFlagUsageError_JSONAfterBadFlag(t *testing.T) { t.Error("expected --json after the bad flag to enable JSON errors") } } + +func TestFlagUsageError_LastJSONValueWins(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) + args := []string{"--json", "--bogus", "--json=false"} + prev := cliArgs + cliArgs = func() []string { return args } + t.Cleanup(func() { cliArgs = prev }) + + res := runRoot(t, args...) + assertInvalidInput(t, res) + if jsonOutput { + t.Error("expected the trailing --json=false to disable JSON errors") + } +} + +func TestLeafArgsError_HonorsJQ(t *testing.T) { + newTestEnv(t, http.NotFoundHandler()) + res := runRoot(t, "status", "extra", "--jq", ".") + assertInvalidInput(t, res) + if !jsonOutput { + t.Error("expected --jq to switch the error to JSON mode") + } +} diff --git a/cmd/verify.go b/cmd/verify.go index 1a0e064..d110814 100644 --- a/cmd/verify.go +++ b/cmd/verify.go @@ -16,7 +16,7 @@ func newVerifyCmd() *cobra.Command { # JSON output for scripts emailable verify hello@example.com --json`, - Args: wrapInvalidInputArgs(cobra.ExactArgs(1)), + Args: cobra.ExactArgs(1), SilenceUsage: true, RunE: func(cmd *cobra.Command, args []string) error { email := args[0] From 6086cbb8437cffebb836f02aa3dbc77bd0a5c59c Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Thu, 1 Oct 2026 18:04:47 +0200 Subject: [PATCH 5/8] Trim README additions for output fixes The README spelled out download handling, CSV column order, and status display rules that users never act on; `-o` and the error codes are already documented where they matter. --- README.md | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/README.md b/README.md index 9c1ced3..ad9a53a 100644 --- a/README.md +++ b/README.md @@ -166,10 +166,6 @@ Resolution order when multiple sources are configured: credential source without making a network call — useful for agents self-diagnosing a failure. (`emailable account status` is the separate network-backed command that fetches the owner email + remaining credits.) -When the active credential is the one saved by `emailable login` (OAuth or a -stored API key), it also shows the account email recorded at login. It -doesn't for a key from `EMAILABLE_API_KEY`, which may belong to a different -account. ```bash emailable status @@ -243,30 +239,6 @@ Flags: - `-o, --output ` — write the results to FILE (`.csv` or `.json`; format inferred from extension) -#### Saving results with `-o` - -`-o` writes every result row, whatever the batch size: - -- Batches over 1,000 emails return a `download_file` link instead of inline - results. `-o` downloads that file (a ZIP compressed CSV) and converts it to - the format the extension asks for. -- A batch that is still verifying is an error (exit code `4`) unless you pass - `--wait` to block until it finishes or `--partial` to save the rows - verified so far. -- With `--json`, the file is written as JSON whatever its extension. - -CSV files have one column per verify response field, always in this order: - -``` -email,state,reason,score,domain,user,first_name,last_name,full_name,gender,birth_year,free,role,disposable,accept_all,mailbox_full,no_reply,did_you_mean,mx_record,smtp_provider,tag,duration -``` - -A cell is blank when the API omitted the field (for example with -`--response-fields`) or returned `null`. Any field not in this list is -appended as an extra column, sorted by name. JSON files keep the API's -batch payload, with the `emails` array filled in from the download for -large batches. - ### Account Show account information and remaining credits: @@ -373,7 +345,7 @@ through verbatim. | `not_authenticated` | Missing or invalid credentials (HTTP 401) | | `forbidden` | Authenticated but not allowed (HTTP 403) | | `not_found` | Unknown resource (HTTP 404) | -| `invalid_input` | Bad request / validation failure (HTTP 400, 422), or a CLI usage error (unknown command or flag, bad flag value, conflicting flags) | +| `invalid_input` | Bad request or CLI usage error (HTTP 400, 422) | | `rate_limited` | Throttled by the server (HTTP 429) | | `try_again` | Verification is still processing (HTTP 249) | | `server_error` | Server-side failure (HTTP 5xx) | From 302532f5b1754e3502a53e69968a84a29c431ac7 Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Wed, 7 Oct 2026 12:56:16 -0400 Subject: [PATCH 6/8] Summarize large batches instead of printing the download URL A completed large batch printed a ~1,700 character presigned URL with broken indentation. It now shows the same per-state summary as small batches, built from `total_counts`, and points to `-o`; the URL stays in `--json` for scripts. --- cmd/batch.go | 2 +- cmd/batch_e2e_test.go | 17 ++++++++++++++--- internal/api/types.go | 8 ++++++-- internal/output/human.go | 19 ++++++++++++++----- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/cmd/batch.go b/cmd/batch.go index 5ff4753..20fa43f 100644 --- a/cmd/batch.go +++ b/cmd/batch.go @@ -436,7 +436,7 @@ func renderBatchOutcome(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatu return err } h := &output.Human{W: cmd.OutOrStdout(), Quiet: cctx.Quiet} - return h.Hint(fmt.Sprintf("Run `emailable batch get %s -o results.csv` to save it.", batchID)) + return h.Hint(fmt.Sprintf("Run `emailable batch get %s -o results.csv` to save the results.", batchID)) } if len(status.Emails) == 0 { return newOutput(cmd.OutOrStdout(), false).Print(status) diff --git a/cmd/batch_e2e_test.go b/cmd/batch_e2e_test.go index d925876..f5ac5f5 100644 --- a/cmd/batch_e2e_test.go +++ b/cmd/batch_e2e_test.go @@ -454,13 +454,24 @@ func TestRenderBatchOutcome_DownloadFile(t *testing.T) { cmd.SetOut(&stdout) cmd.SetErr(&bytes.Buffer{}) - status := &api.BatchStatus{ID: "bch_big", DownloadFile: "https://files.example/big.csv"} + status := &api.BatchStatus{ + ID: "bch_big", + DownloadFile: "https://files.example/big.csv", + TotalCounts: &api.BatchTotalCounts{Total: 3, Processed: 3, Deliverable: 2, Unknown: 1}, + } cctx := &cmdCtx{JSONMode: false} if err := renderBatchOutcome(cmd, cctx, status, "bch_big", "", false); err != nil { t.Fatalf("renderBatchOutcome: %v", err) } - if !strings.Contains(stdout.String(), "big.csv") { - t.Errorf("expected download URL in output, got %q", stdout.String()) + out := stdout.String() + if !strings.Contains(out, "Verified 3 emails: 2 Deliverable, 1 Unknown") { + t.Errorf("expected a summary from total_counts, got %q", out) + } + if !strings.Contains(out, "batch get bch_big -o results.csv") { + t.Errorf("expected the -o hint, got %q", out) + } + if strings.Contains(out, "big.csv") { + t.Errorf("the presigned URL should stay out of human output, got %q", out) } } diff --git a/internal/api/types.go b/internal/api/types.go index 7969e56..4849d44 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -51,8 +51,12 @@ type BatchSubmit struct { // BatchTotalCounts holds aggregate progress counts from a partial batch response. type BatchTotalCounts struct { - Total int `json:"total"` - Processed int `json:"processed"` + Total int `json:"total"` + Processed int `json:"processed"` + Deliverable int `json:"deliverable"` + Undeliverable int `json:"undeliverable"` + Risky int `json:"risky"` + Unknown int `json:"unknown"` } // BatchStatus merges three API payload shapes: in-progress (Total/Processed), diff --git a/internal/output/human.go b/internal/output/human.go index 982d707..a3d54a7 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -93,10 +93,7 @@ func (h *Human) Print(v any) error { return h.PrintBatchResults(x.Emails) } if x.DownloadFile != "" { - if err := h.Success("Batch complete"); err != nil { - return err - } - return h.Hint(fmt.Sprintf("Too many results to display inline — download from:\n `%s`", x.DownloadFile)) + return h.PrintBatchSummary(x) } return h.PrintBatchStatus(x) case api.BatchStatus: @@ -421,6 +418,18 @@ func (h *Human) PrintBatchSummary(s *api.BatchStatus) error { for _, e := range s.Emails { counts[e.State]++ } + verified := len(s.Emails) + // Large batches return per-state totals instead of inline rows. + if verified == 0 && s.TotalCounts != nil { + tc := s.TotalCounts + counts = map[string]int{ + "deliverable": tc.Deliverable, + "undeliverable": tc.Undeliverable, + "risky": tc.Risky, + "unknown": tc.Unknown, + } + verified = tc.Processed + } var parts []string for _, state := range []string{"deliverable", "undeliverable", "risky", "unknown"} { @@ -443,7 +452,7 @@ func (h *Human) PrintBatchSummary(s *api.BatchStatus) error { if s.IsComplete() { check := stf(lipgloss.NewStyle().Foreground(lipgloss.Color("42")).Bold(true)).Render("✓") - _, err := fmt.Fprintf(h.W, "%s Verified %d emails%s\n", check, len(s.Emails), tail) + _, err := fmt.Fprintf(h.W, "%s Verified %d emails%s\n", check, verified, tail) return err } From e11c229b228c65a03cfe4aacb0c5ff22a560b46f Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Wed, 7 Oct 2026 15:05:09 -0400 Subject: [PATCH 7/8] Refuse empty partial saves and count large batches from progress `--partial -o` on a batch with nothing verified yet still wrote a header-only file, and a large batch that reports only top-level `total`/`processed` showed "Verified 0 emails". The `--quiet` help now says it hides hints, progress, and status messages, since results still print. --- README.md | 5 ++--- cmd/batch.go | 3 +++ cmd/batch_output_test.go | 22 ++++++++++++++++++++++ cmd/root.go | 2 +- internal/output/human.go | 22 +++++++++++++--------- internal/output/human_test.go | 25 +++++++++++++++++++++++++ 6 files changed, 66 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index d396e0c..571c4bb 100644 --- a/README.md +++ b/README.md @@ -409,9 +409,8 @@ EMAILABLE_DEBUG=1 emailable verify hello@example.com ### Quiet mode -Pass `--quiet` (or `-q`) to suppress non-error human output — success -lines, hints, notices, progress bars and spinners. Errors still print, and -`--json` output is unaffected (quiet is a human-mode-only modifier). +Pass `--quiet` (or `-q`) to suppress hints, progress, and status messages. +Results and errors still print, and `--json` output is unaffected. Mirrors the convention in `curl`, `docker`, and `gh`. ```bash diff --git a/cmd/batch.go b/cmd/batch.go index 20fa43f..1bb74c1 100644 --- a/cmd/batch.go +++ b/cmd/batch.go @@ -387,6 +387,9 @@ func saveBatchToFile(cmd *cobra.Command, cctx *cmdCtx, status *api.BatchStatus, if status.DownloadFile != "" { return saveDownloadToFile(cmd, cctx, status, path) } + if !status.IsComplete() && len(status.Emails) == 0 { + return NewInvalidInputf("batch %s has no verified emails yet; try again shortly or use --wait", batchID) + } if _, total, ok := status.Progress(); ok && total > 0 && status.IsComplete() && len(status.Emails) == 0 { return fmt.Errorf("batch %s returned no per-email results to save", batchID) } diff --git a/cmd/batch_output_test.go b/cmd/batch_output_test.go index 5f3bf21..3916bfc 100644 --- a/cmd/batch_output_test.go +++ b/cmd/batch_output_test.go @@ -63,6 +63,28 @@ func TestBatchGet_OutputPartial(t *testing.T) { } } +// TestBatchGet_OutputPartialNoRowsYet: a queued batch has nothing to save, so +// --partial -o must not write a header-only file. +func TestBatchGet_OutputPartialNoRowsYet(t *testing.T) { + env := newTestEnv(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeJSON(w, map[string]any{ + "id": "bch_1", + "message": "Your batch is being processed.", + "total_counts": map[string]int{"processed": 0, "total": 4}, + }) + })) + env.seedAPIKey(t, "sk_test_xxx") + out := filepath.Join(t.TempDir(), "results.csv") + + res := runRoot(t, "batch", "get", "bch_1", "--partial", "-o", out) + if got := errorCode(res.Err); got != codeInvalidInput { + t.Fatalf("errorCode: got %q want %q (err: %v)", got, codeInvalidInput, res.Err) + } + if _, err := os.Stat(out); !os.IsNotExist(err) { + t.Errorf("expected no file to be written, stat err: %v", err) + } +} + func TestBatchGet_OutputDownloadFile(t *testing.T) { var zbuf bytes.Buffer zw := zip.NewWriter(&zbuf) diff --git a/cmd/root.go b/cmd/root.go index a7bdbbc..7eba405 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -226,7 +226,7 @@ func newRootCmd(v string) *cobra.Command { root.PersistentFlags().BoolVar(&jsonOutput, "json", false, "Return JSON response") root.PersistentFlags().StringVar(&jqExpr, "jq", "", "Filter JSON output with a jq `expression` (implies --json)") root.PersistentFlags().BoolVar(&debugMode, "debug", false, "Dump HTTP requests/responses to stderr (also EMAILABLE_DEBUG)") - root.PersistentFlags().BoolVarP(&quietMode, "quiet", "q", false, "Suppress non-error human output (success lines, hints, progress)") + root.PersistentFlags().BoolVarP(&quietMode, "quiet", "q", false, "Suppress hints, progress, and status messages") root.AddGroup( &cobra.Group{ID: groupCore, Title: "CORE COMMANDS"}, diff --git a/internal/output/human.go b/internal/output/human.go index a3d54a7..b4d7ac9 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -419,16 +419,20 @@ func (h *Human) PrintBatchSummary(s *api.BatchStatus) error { counts[e.State]++ } verified := len(s.Emails) - // Large batches return per-state totals instead of inline rows. - if verified == 0 && s.TotalCounts != nil { - tc := s.TotalCounts - counts = map[string]int{ - "deliverable": tc.Deliverable, - "undeliverable": tc.Undeliverable, - "risky": tc.Risky, - "unknown": tc.Unknown, + // Large batches return counts instead of inline rows: the verified count + // comes from progress, and the state breakdown from total_counts. + if verified == 0 { + if processed, _, ok := s.Progress(); ok { + verified = processed + } + if tc := s.TotalCounts; tc != nil { + counts = map[string]int{ + "deliverable": tc.Deliverable, + "undeliverable": tc.Undeliverable, + "risky": tc.Risky, + "unknown": tc.Unknown, + } } - verified = tc.Processed } var parts []string diff --git a/internal/output/human_test.go b/internal/output/human_test.go index a616ad6..6bfbf24 100644 --- a/internal/output/human_test.go +++ b/internal/output/human_test.go @@ -434,3 +434,28 @@ func TestHuman_QuietDefaultIsLoud(t *testing.T) { t.Errorf("expected chrome line in non-quiet mode, got %q", buf.String()) } } + +// TestPrintBatchSummary_LargeBatchCounts: without inline rows the verified +// count comes from progress (top-level or total_counts) and the breakdown +// from total_counts when present. +func TestPrintBatchSummary_LargeBatchCounts(t *testing.T) { + cases := []struct { + name string + s *api.BatchStatus + want string + }{ + {"top-level counts", &api.BatchStatus{Total: 5000, Processed: 5000, DownloadFile: "https://x/y"}, "Verified 5000 emails\n"}, + {"total_counts", &api.BatchStatus{DownloadFile: "https://x/y", TotalCounts: &api.BatchTotalCounts{Total: 3, Processed: 3, Deliverable: 2, Risky: 1}}, "Verified 3 emails: 2 Deliverable, 1 Risky\n"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + var buf bytes.Buffer + if err := (&Human{W: &buf}).PrintBatchSummary(c.s); err != nil { + t.Fatal(err) + } + if !strings.HasSuffix(buf.String(), c.want) { + t.Errorf("got %q, want suffix %q", buf.String(), c.want) + } + }) + } +} From 228c004f7a7da2dd69d589c0a7b2269189bd8c56 Mon Sep 17 00:00:00 2001 From: Jarrett Lusso Date: Wed, 7 Oct 2026 15:36:11 -0400 Subject: [PATCH 8/8] Exclude duplicates from the large-batch verified count The server's `processed` count includes duplicates, which carry no state, so a batch with duplicates claimed more verified emails than its breakdown added up to. The count now matches what was verified, and skipped duplicates are noted. --- internal/api/types.go | 1 + internal/output/human.go | 10 ++++++++++ internal/output/human_test.go | 1 + 3 files changed, 12 insertions(+) diff --git a/internal/api/types.go b/internal/api/types.go index 4849d44..60ad184 100644 --- a/internal/api/types.go +++ b/internal/api/types.go @@ -57,6 +57,7 @@ type BatchTotalCounts struct { Undeliverable int `json:"undeliverable"` Risky int `json:"risky"` Unknown int `json:"unknown"` + Duplicate int `json:"duplicate"` // included in Processed, not in the state counts } // BatchStatus merges three API payload shapes: in-progress (Total/Processed), diff --git a/internal/output/human.go b/internal/output/human.go index b4d7ac9..ffb98cb 100644 --- a/internal/output/human.go +++ b/internal/output/human.go @@ -426,6 +426,9 @@ func (h *Human) PrintBatchSummary(s *api.BatchStatus) error { verified = processed } if tc := s.TotalCounts; tc != nil { + // The server's processed count includes duplicates, which + // aren't verified and have no state. + verified = max(verified-tc.Duplicate, 0) counts = map[string]int{ "deliverable": tc.Deliverable, "undeliverable": tc.Undeliverable, @@ -453,6 +456,13 @@ func (h *Human) PrintBatchSummary(s *api.BatchStatus) error { if len(parts) > 0 { tail = ": " + strings.Join(parts, ", ") } + if tc := s.TotalCounts; tc != nil && tc.Duplicate > 0 { + note := fmt.Sprintf("(%d duplicates skipped)", tc.Duplicate) + if tc.Duplicate == 1 { + note = "(1 duplicate skipped)" + } + tail += " " + stf(lipgloss.NewStyle().Foreground(lipgloss.Color("241"))).Render(note) + } if s.IsComplete() { check := stf(lipgloss.NewStyle().Foreground(lipgloss.Color("42")).Bold(true)).Render("✓") diff --git a/internal/output/human_test.go b/internal/output/human_test.go index 6bfbf24..9a1d9b0 100644 --- a/internal/output/human_test.go +++ b/internal/output/human_test.go @@ -446,6 +446,7 @@ func TestPrintBatchSummary_LargeBatchCounts(t *testing.T) { }{ {"top-level counts", &api.BatchStatus{Total: 5000, Processed: 5000, DownloadFile: "https://x/y"}, "Verified 5000 emails\n"}, {"total_counts", &api.BatchStatus{DownloadFile: "https://x/y", TotalCounts: &api.BatchTotalCounts{Total: 3, Processed: 3, Deliverable: 2, Risky: 1}}, "Verified 3 emails: 2 Deliverable, 1 Risky\n"}, + {"duplicates excluded", &api.BatchStatus{DownloadFile: "https://x/y", TotalCounts: &api.BatchTotalCounts{Total: 15, Processed: 15, Deliverable: 2, Undeliverable: 3, Risky: 1, Unknown: 4, Duplicate: 5}}, "Verified 10 emails: 2 Deliverable, 3 Undeliverable, 1 Risky, 4 Unknown (5 duplicates skipped)\n"}, } for _, c := range cases { t.Run(c.name, func(t *testing.T) {