feat(cli): positional account grammar, account show, TTY remove confirm; drop whitelist flags

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/emersion/go-message v0.18.2
 	github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6
 	github.com/emersion/go-smtp v0.24.0
+	github.com/mattn/go-isatty v0.0.20
 	modernc.org/sqlite v1.53.0
 )
 
@@ -27,7 +28,6 @@ require (
 	github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/lucasb-eyer/go-colorful v1.3.0 // indirect
-	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/mattn/go-localereader v0.0.1 // indirect
 	github.com/mattn/go-runewidth v0.0.19 // indirect
 	github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect

internal/cli/account_list_test.go

@@ -10,10 +10,10 @@ import (
 // name/from/can_send, and never the IMAP host or login username.
 func TestAccountListAgentJSONView(t *testing.T) {
 	adminEnv(t) // both keys + initialized temp DB
-	run(t, "account", "add", "--name", "work", "--mode", "RW",
+	run(t, "account", "add", "work", "--mode", "RW",
 		"--imap-host", "imap.example.com", "--smtp-host", "smtp.example.com",
 		"--username", "login@example.com", "--from", "me@example.com")
-	run(t, "account", "add", "--name", "alerts", "--mode", "RO",
+	run(t, "account", "add", "alerts", "--mode", "RO",
 		"--imap-host", "imap.example.com", "--username", "alerts@example.com")
 
 	// Drop the admin key → caller is an agent.
@@ -66,7 +66,7 @@ func TestAccountListAgentJSONView(t *testing.T) {
 // With the admin key present, `account list` stays the full human-readable table.
 func TestAccountListAdminTextView(t *testing.T) {
 	adminEnv(t)
-	run(t, "account", "add", "--name", "work", "--mode", "RW",
+	run(t, "account", "add", "work", "--mode", "RW",
 		"--imap-host", "imap.example.com", "--smtp-host", "smtp.example.com",
 		"--username", "login@example.com", "--from", "me@example.com")
 

internal/cli/account_show.go

@@ -0,0 +1,49 @@
+package cli
+
+import (
+	"fmt"
+	"io"
+
+	"git.dcglab.co.uk/steve/emcli/internal/store"
+)
+
+// accountShow renders one account's configuration. The password is never shown.
+func accountShow(st *store.Store, rest []string, out, errOut io.Writer) int {
+	if len(rest) == 0 {
+		fmt.Fprintln(errOut, "usage: emcli account show <name>")
+		return 2
+	}
+	if len(rest) > 1 {
+		fmt.Fprintf(errOut, "unexpected argument %q\n", rest[1])
+		return 2
+	}
+	a, err := st.GetAccount(rest[0])
+	if err != nil {
+		fmt.Fprintf(errOut, "show: %v\n", err)
+		return 1
+	}
+	onOff := func(b bool) string {
+		if b {
+			return "enabled"
+		}
+		return "disabled"
+	}
+	smtp := "-"
+	if a.SMTPHost != "" {
+		smtp = fmt.Sprintf("%s:%d (%s)", a.SMTPHost, a.SMTPPort, a.SMTPSecurity)
+	}
+	subj := a.SubjectRegex
+	if subj == "" {
+		subj = "(none)"
+	}
+	fmt.Fprintf(out, "name:              %s\n", a.Name)
+	fmt.Fprintf(out, "mode:              %s\n", a.Mode)
+	fmt.Fprintf(out, "imap:              %s:%d (%s)\n", a.IMAPHost, a.IMAPPort, a.IMAPSecurity)
+	fmt.Fprintf(out, "smtp:              %s\n", smtp)
+	fmt.Fprintf(out, "username:          %s\n", a.Username)
+	fmt.Fprintf(out, "send-from:         %s\n", a.SendFrom())
+	fmt.Fprintf(out, "subject filter:    %s\n", subj)
+	fmt.Fprintf(out, "inbound whitelist: %s\n", onOff(a.WhitelistInEnabled))
+	fmt.Fprintf(out, "outbound whitelist:%s\n", onOff(a.WhitelistOutEnabled))
+	return 0
+}

internal/cli/account_show_test.go

@@ -0,0 +1,36 @@
+package cli
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestAccountShow(t *testing.T) {
+	adminEnv(t)
+	run(t, "account", "add", "shown", "--mode", "RW",
+		"--imap-host", "imap.x.com", "--imap-port", "993",
+		"--smtp-host", "smtp.x.com", "--smtp-port", "465",
+		"--username", "u@x.com", "--password", "secret", "--from", "me@x.com")
+	code, out, _ := run(t, "account", "show", "shown")
+	if code != 0 {
+		t.Fatalf("show exit=%d", code)
+	}
+	for _, want := range []string{"shown", "RW", "imap.x.com:993", "smtp.x.com:465", "u@x.com", "me@x.com"} {
+		if !strings.Contains(out, want) {
+			t.Fatalf("show missing %q:\n%s", want, out)
+		}
+	}
+	if strings.Contains(out, "secret") {
+		t.Fatalf("show must never print the password:\n%s", out)
+	}
+}
+
+func TestAccountShowMissingName(t *testing.T) {
+	adminEnv(t)
+	if code, _, _ := run(t, "account", "show"); code != 2 {
+		t.Fatal("show without a name must be a usage error")
+	}
+	if code, _, _ := run(t, "account", "show", "nope"); code == 0 {
+		t.Fatal("show of a missing account must be non-zero")
+	}
+}

internal/cli/admin.go

@@ -1,33 +1,45 @@
 package cli
 
 import (
+	"bufio"
 	"flag"
 	"fmt"
 	"io"
+	"os"
 	"strings"
 
+	"github.com/mattn/go-isatty"
+
 	"git.dcglab.co.uk/steve/emcli/internal/crypto"
 	"git.dcglab.co.uk/steve/emcli/internal/policy"
 	"git.dcglab.co.uk/steve/emcli/internal/store"
 	"git.dcglab.co.uk/steve/emcli/internal/tui"
 )
 
-// runAccount handles `account add|list`. Human-readable output (never JSON).
+// confirmRemoval prompts on a TTY for a y/N answer. Non-TTY callers never reach
+// here (the caller requires --yes when stdin is not a terminal).
+func confirmRemoval(name string, out io.Writer) bool {
+	fmt.Fprintf(out, "Remove account %q? [y/N]: ", name)
+	line, _ := bufio.NewReader(os.Stdin).ReadString('\n')
+	line = strings.ToLower(strings.TrimSpace(line))
+	return line == "y" || line == "yes"
+}
+
+// runAccount handles `account <add|edit|remove|show|list>`. Human-readable
+// output (except the agent-only reduced-JSON branch of `list`).
 func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 	if len(args) == 0 || helpRequested(args[0]) {
 		printCmdUsage(out, "account")
-		fmt.Fprintln(out, "\nSubcommands: add, edit, remove, list")
+		fmt.Fprintln(out, "\nSubcommands: add, edit, remove, show, list")
 		if len(args) > 0 {
-			return 0 // explicit --help
+			return 0
 		}
 		return 2
 	}
-	sub, rest := args[0], args[1:]
+	sub := normalizeVerb(args[0])
+	rest := args[1:]
 	st, err := openStore(role)
 	if err != nil {
-		// account list is an agent command (a JSON consumer), so its
-		// open/key failures are emitted as an envelope, like the other agent
-		// commands; the admin subcommands stay human-readable.
 		if sub == "list" {
 			_ = Failure(CodeConfig, err.Error()).Write(out)
 		} else {
@@ -39,17 +51,16 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 
 	switch sub {
 	case "add":
-		if len(rest) == 0 { // no flags → interactive TUI form
+		if len(rest) == 0 { // no args → interactive TUI form
 			return addInteractive(st, tui.Fields{}, out, errOut)
 		}
 		// Peel a leading positional name (if present) before flag parsing.
-		var positionalName string
+		var name string
 		if !strings.HasPrefix(rest[0], "-") {
-			positionalName, rest = rest[0], rest[1:]
+			name, rest = rest[0], rest[1:]
 		}
 		fs := flag.NewFlagSet("account add", flag.ContinueOnError)
 		fs.SetOutput(errOut)
-		name := fs.String("name", "", "account name")
 		mode := fs.String("mode", "RO", "RO|RW")
 		host := fs.String("imap-host", "", "IMAP host")
 		port := fs.Int("imap-port", 993, "IMAP port")
@@ -61,18 +72,16 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 		pass := fs.String("password", "", "login password")
 		from := fs.String("from", "", "send-as address (blank = use username)")
 		subj := fs.String("subject-regex", "", "inbound subject filter")
-		wlIn := fs.Bool("whitelist-in", false, "enable inbound whitelist")
-		wlOut := fs.Bool("whitelist-out", false, "enable outbound whitelist")
 		backlog := fs.Bool("process-backlog", false, "treat existing mail as new")
 		if err := fs.Parse(rest); err != nil {
 			return 2
 		}
-		// Positional name takes precedence; fall back to --name flag.
+		if fs.NArg() > 0 {
-		if positionalName != "" {
+			fmt.Fprintf(errOut, "unexpected argument %q\n", fs.Arg(0))
-			*name = positionalName
+			return 2
 		}
-		if *name == "" || *host == "" || *user == "" {
+		if name == "" || *host == "" || *user == "" {
-			fmt.Fprintln(errOut, "name, imap-host, and username are required")
+			fmt.Fprintln(errOut, "name, --imap-host, and --username are required")
 			return 2
 		}
 		if err := tui.ValidFromAddress(*from); err != nil {
@@ -80,26 +89,31 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 			return 2
 		}
 		acc := store.Account{
-			Name: *name, Mode: *mode, IMAPHost: *host, IMAPPort: *port, IMAPSecurity: *sec,
+			Name: name, Mode: *mode, IMAPHost: *host, IMAPPort: *port, IMAPSecurity: *sec,
 			AuthType: "password", Username: *user, Password: *pass,
-			FromAddress: *from,
+			FromAddress: *from, SubjectRegex: *subj, ProcessBacklog: *backlog,
-			SubjectRegex: *subj, WhitelistInEnabled: *wlIn, WhitelistOutEnabled: *wlOut,
-			ProcessBacklog: *backlog,
 		}
 		if *mode == "RW" {
 			acc.SMTPHost, acc.SMTPPort, acc.SMTPSecurity = *smtpHost, *smtpPort, *smtpSec
 		}
-		_, err := st.AddAccount(acc)
+		if _, err := st.AddAccount(acc); err != nil {
-		if err != nil {
 			fmt.Fprintf(errOut, "add account: %v\n", err)
 			return 1
 		}
-		fmt.Fprintf(out, "account %q added (%s)\n", *name, *mode)
+		fmt.Fprintf(out, "account %q added (%s)\n", name, *mode)
 		return 0
 	case "edit":
+		if len(rest) == 0 || strings.HasPrefix(rest[0], "-") {
+			fmt.Fprintln(errOut, "usage: emcli account edit <name> [flags]")
+			return 2
+		}
+		name := rest[0]
+		flagArgs := rest[1:]
+		if len(flagArgs) == 0 { // only name → interactive prefilled form
+			return editInteractive(st, name, out, errOut)
+		}
 		fs := flag.NewFlagSet("account edit", flag.ContinueOnError)
 		fs.SetOutput(errOut)
-		name := fs.String("name", "", "account name (required)")
 		mode := fs.String("mode", "", "RO|RW")
 		host := fs.String("imap-host", "", "IMAP host")
 		port := fs.Int("imap-port", 0, "IMAP port")
@@ -111,26 +125,22 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 		pass := fs.String("password", "", "login password (blank keeps existing)")
 		from := fs.String("from", "", "send-as address (empty reverts to username)")
 		subj := fs.String("subject-regex", "", "inbound subject filter")
-		if err := fs.Parse(rest); err != nil {
+		if err := fs.Parse(flagArgs); err != nil {
 			return 2
 		}
-		if *name == "" {
+		if fs.NArg() > 0 {
-			fmt.Fprintln(errOut, "--name is required")
+			fmt.Fprintf(errOut, "unexpected argument %q\n", fs.Arg(0))
 			return 2
 		}
-		if fs.NFlag() == 1 { // only --name → interactive TUI form, prefilled
-			return editInteractive(st, *name, out, errOut)
-		}
 		if err := tui.ValidFromAddress(*from); err != nil {
 			fmt.Fprintln(errOut, err)
 			return 2
 		}
-		acc, err := st.GetAccount(*name)
+		acc, err := st.GetAccount(name)
 		if err != nil {
 			fmt.Fprintf(errOut, "edit: %v\n", err)
 			return 1
 		}
-		// Overlay only the flags the user actually set.
 		fs.Visit(func(f *flag.Flag) {
 			switch f.Name {
 			case "mode":
@@ -157,40 +167,51 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 				acc.SubjectRegex = *subj
 			}
 		})
-		// acc.Password holds the existing (decrypted) password from GetAccount; the
-		// Visit above overwrites it only when --password was passed. UpdateAccount
-		// re-seals whatever non-empty value is present, so the password is preserved.
 		if err := st.UpdateAccount(acc); err != nil {
 			fmt.Fprintf(errOut, "edit: %v\n", err)
 			return 1
 		}
-		fmt.Fprintf(out, "account %q updated\n", *name)
+		fmt.Fprintf(out, "account %q updated\n", name)
 		return 0
 	case "remove":
-		fs := flag.NewFlagSet("account remove", flag.ContinueOnError)
+		if len(rest) == 0 || strings.HasPrefix(rest[0], "-") {
-		fs.SetOutput(errOut)
+			fmt.Fprintln(errOut, "usage: emcli account remove <name> [--yes]")
-		name := fs.String("name", "", "account name (required)")
-		yes := fs.Bool("yes", false, "skip confirmation")
-		if err := fs.Parse(rest); err != nil {
 			return 2
 		}
-		if *name == "" {
+		name := rest[0]
-			fmt.Fprintln(errOut, "--name is required")
+		fs := flag.NewFlagSet("account remove", flag.ContinueOnError)
+		fs.SetOutput(errOut)
+		yes := fs.Bool("yes", false, "skip confirmation")
+		if err := fs.Parse(rest[1:]); err != nil {
+			return 2
+		}
+		if fs.NArg() > 0 {
+			fmt.Fprintf(errOut, "unexpected argument %q\n", fs.Arg(0))
 			return 2
 		}
 		if !*yes {
-			fmt.Fprintf(errOut, "refusing to remove %q without --yes\n", *name)
+			if !isatty.IsTerminal(os.Stdin.Fd()) {
+				fmt.Fprintf(errOut, "refusing to remove %q without --yes (no terminal for confirmation)\n", name)
 				return 2
 			}
-		if err := st.DeleteAccount(*name); err != nil {
+			if !confirmRemoval(name, out) {
+				fmt.Fprintln(out, "aborted")
+				return 1
+			}
+		}
+		if err := st.DeleteAccount(name); err != nil {
 			fmt.Fprintf(errOut, "remove: %v\n", err)
 			return 1
 		}
-		fmt.Fprintf(out, "account %q removed\n", *name)
+		fmt.Fprintf(out, "account %q removed\n", name)
 		return 0
+	case "show":
+		return accountShow(st, rest, out, errOut)
 	case "list":
-		// Holding the admin key means the caller is the human admin (full
+		if len(rest) > 0 {
-		// detail). An agent holds only EMCLI_KEY and gets a reduced JSON view.
+			fmt.Fprintf(errOut, "unexpected argument %q\n", rest[0])
+			return 2
+		}
 		_, adminErr := crypto.AdminKeyFromEnv()
 		isAdmin := adminErr == nil
 		accs, err := st.ListAccounts()
@@ -206,9 +227,7 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 			items := make([]map[string]any, 0, len(accs))
 			for _, a := range accs {
 				items = append(items, map[string]any{
-					"name":     a.Name,
+					"name": a.Name, "from": a.SendFrom(), "can_send": a.Mode == "RW",
-					"from":     a.SendFrom(),
-					"can_send": a.Mode == "RW",
 				})
 			}
 			_ = Success(map[string]any{"accounts": items}).Write(out)
@@ -221,7 +240,7 @@ func runAccount(args []string, role store.Role, out, errOut io.Writer) int {
 		}
 		return 0
 	default:
-		fmt.Fprintf(errOut, "unknown account subcommand %q\n", sub)
+		fmt.Fprintf(errOut, "unknown account subcommand %q (want add|edit|remove|show|list)\n", sub)
 		return 2
 	}
 }

internal/cli/admin_test.go

@@ -84,8 +84,8 @@ func TestConfigList(t *testing.T) {
 
 func TestAccountRemove(t *testing.T) {
 	adminEnv(t)
-	run(t, "account", "add", "--name", "gone", "--imap-host", "h", "--username", "u@x.com")
+	run(t, "account", "add", "gone", "--imap-host", "h", "--username", "u@x.com")
-	if code, _, e := run(t, "account", "remove", "--name", "gone", "--yes"); code != 0 {
+	if code, _, e := run(t, "account", "remove", "gone", "--yes"); code != 0 {
 		t.Fatalf("remove failed: %s", e)
 	}
 	_, out, _ := run(t, "account", "list")
@@ -96,17 +96,26 @@ func TestAccountRemove(t *testing.T) {
 
 func TestAccountRemoveMissing(t *testing.T) {
 	adminEnv(t)
-	if code, _, _ := run(t, "account", "remove", "--name", "nope", "--yes"); code == 0 {
+	if code, _, _ := run(t, "account", "remove", "nope", "--yes"); code == 0 {
 		t.Fatal("removing a missing account must be non-zero")
 	}
 }
 
+func TestAccountRemoveNoTTYNeedsYes(t *testing.T) {
+	adminEnv(t)
+	run(t, "account", "add", "keep", "--imap-host", "h", "--username", "u@x.com")
+	// Under `go test`, stdin is not a TTY, so without --yes this must refuse.
+	code, _, errOut := run(t, "account", "remove", "keep")
+	if code != 2 || !strings.Contains(errOut, "--yes") {
+		t.Fatalf("non-TTY remove without --yes must refuse: code=%d err=%q", code, errOut)
+	}
+}
+
 func TestAccountEditPartialPreservesOtherFields(t *testing.T) {
 	db := adminEnv(t)
-	run(t, "account", "add", "--name", "ed", "--mode", "RO",
+	run(t, "account", "add", "ed", "--mode", "RO",
 		"--imap-host", "imap.x.com", "--username", "u@x.com", "--password", "orig")
-	// Edit only mode + add SMTP; imap-host, username, password must be preserved.
+	if code, _, e := run(t, "account", "edit", "ed", "--mode", "RW",
-	if code, _, e := run(t, "account", "edit", "--name", "ed", "--mode", "RW",
 		"--smtp-host", "smtp.x.com", "--smtp-port", "587", "--smtp-security", "starttls"); code != 0 {
 		t.Fatalf("edit failed: %s", e)
 	}
@@ -217,10 +226,8 @@ func TestAuditListCoreRenders(t *testing.T) {
 
 func TestAccountEditFromValidationRejectsMalformed(t *testing.T) {
 	adminEnv(t)
-	// Seed an account so the failure is from --from validation, not a missing account.
+	run(t, "account", "add", "valacc", "--imap-host", "imap.x.com", "--username", "u@x.com")
-	run(t, "account", "add", "--name", "valacc", "--imap-host", "imap.x.com", "--username", "u@x.com")
+	code, _, errStr := run(t, "account", "edit", "valacc", "--from", "not an address")
-	// A malformed --from value must be rejected with exit code 2 before touching the account.
-	code, _, errStr := run(t, "account", "edit", "--name", "valacc", "--from", "not an address")
 	if code != 2 {
 		t.Fatalf("expected exit code 2 for malformed --from, got %d (stderr: %q)", code, errStr)
 	}