docs: implementation plan for human CLI grammar redesign

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docs/superpowers/specs/2026-06-27-human-cli-grammar-redesign-design.md

@@ -0,0 +1,201 @@
+# emcli — Human-facing CLI grammar redesign
+
+Date: 2026-06-27
+Status: Approved (design)
+
+## Problem
+
+`emcli` was designed flag-first for AI agents — explicit, stable, machine-parseable. The
+human-facing **admin** commands inherited that flag-heavy grammar, and the result is inconsistent
+and error-prone for a person at a terminal. The "main noun" of each command is sometimes
+positional, sometimes a flag:
+
+| Command (today) | Names its target via |
+|---|---|
+| `config set audit_retention_days 90` | positional key + value |
+| `account remove --name bobby --yes` | flag |
+| `whitelist in add --account bobby --address x@y` | flags (×2) |
+| `audit list --account bobby` | flag |
+
+A human has no rule to remember. The concrete failure that triggered this work:
+
+```
+emcli whitelist in add --account bobby "Tk555@protonmail.com"
+```
+
+Go's `flag` parser read `--account bobby`, treated `"Tk555@protonmail.com"` as an ignored leftover
+positional, and `--address` defaulted to `""`. `AddWhitelist` then ran
+`INSERT OR IGNORE ... VALUES(id, "")` — silently inserting a blank whitelist row. No error, no
+validation.
+
+## Goals
+
+- One consistent, predictable grammar for all human-facing (admin) commands.
+- Eliminate the silently-ignored-input and empty-operand bug classes.
+- Improve discoverability (`config list`, `account show`) and onboarding (whitelists fully
+  decoupled from account creation).
+- Keep the agent JSON command surface (`list`/`get`/`search`/`ack`/`send`) **frozen**.
+
+## Non-goals
+
+- No change to agent JSON commands' flags, output, or behavior.
+- No change to the encryption/key model, store schema semantics, or policy engine logic.
+- No unrelated refactoring.
+
+## Unifying grammar
+
+> **`emcli <group> <verb> <operands…> [--flags]`**
+>
+> Primary targets — account name, address(es), config key/value — are **positional**.
+> Flags carry only options/modifiers (`--in`/`--out`, `--yes`, `--limit`, account field values
+> like `--imap-host`).
+
+Two cross-cutting safety rules enforced everywhere:
+
+1. **No silently-ignored input.** Any leftover positional argument beyond what a command expects
+   is an error (`unexpected argument "x"`). This alone prevents the triggering bug.
+2. **No empty primary operands.** Empty account / address / key is rejected with a clear message.
+
+### Verb aliases
+
+Applied at the verb position for every group (and the top-level command position, additively):
+
+- `remove` = `rm` = `del`
+- `list` = `ls`
+
+Implemented as a single `normalizeVerb(string) string` map applied before the verb `switch`.
+
+## Command surface (after)
+
+### account
+| Command | Notes |
+|---|---|
+| `account add [name] [--imap-host … --username … …]` | `name` positional. No flags → interactive TUI form (unchanged). **`--whitelist-in`/`--whitelist-out` removed** (see Whitelist decoupling). `--process-backlog` kept. |
+| `account edit <name> [--flags]` | `name` positional; only `name` → interactive form prefilled. **`--whitelist-in`/`--whitelist-out` removed.** |
+| `account remove <name> [--yes]` | `name` positional. On a TTY, prompt `Remove account "bobby"? [y/N]`; non-TTY (piped) requires `--yes`. Aliases: `rm`, `del`. |
+| `account show <name>` | **New.** Human-readable detail: mode (RO/RW), IMAP host/port/security, SMTP host/port/security, send-from address, subject regex, inbound/outbound whitelist enabled state. Never prints the password. |
+| `account list` | Unchanged table. Alias: `ls`. |
+
+### whitelist
+Direction is a **required** `--in` / `--out` flag (error if neither given — no wrong-list default).
+Whitelist management is fully self-contained in this group (enable/disable moved here from
+`account edit`).
+
+| Command | Notes |
+|---|---|
+| `whitelist add <account> <addr…> --in\|--out` | One or more addresses, each validated (see Address validation). Aliases for verb: n/a. |
+| `whitelist remove <account> <addr…> --in\|--out` | One or more addresses. Aliases: `rm`, `del`. |
+| `whitelist list <account> --in\|--out` | Header line shows `ENABLED`/`DISABLED`. Alias: `ls`. |
+| `whitelist enable <account> --in\|--out` | **New.** If the list is empty, print a warning: `warning: inbound whitelist for "bobby" is empty — this blocks ALL inbound mail until you add addresses`. Still enables (explicit user action). |
+| `whitelist disable <account> --in\|--out` | **New.** |
+
+### config
+A known-key **settings registry** backs validation and discovery. Each entry: `key`,
+`description`, and a `validate(value) error`.
+
+| Command | Notes |
+|---|---|
+| `config list` | **New.** Table: `KEY  VALUE  DESCRIPTION`. Lists every registered key, its current value (or `(unset)`), and description. Alias: `ls`. |
+| `config get <key>` | Rejects unknown key. |
+| `config set <key> <value>` | Rejects unknown key (`unknown setting "foo" (see: emcli config list)`); runs the key's validator. |
+
+Initial registry: `audit_retention_days` (description "Days to keep audit-log entries"; validator:
+integer ≥ 0 — same rule as today).
+
+### audit
+| Command | Notes |
+|---|---|
+| `audit list [account] [--limit N]` | `account` positional (optional; omitted = all accounts). `--limit` stays a flag (it's a modifier). Alias: `ls`. |
+
+### doctor (exception — dual-use)
+`doctor` is agent-runnable (`RoleAgent`) and the agentic manual documents `emcli doctor --account
+gmail`. To avoid breaking documented agent usage while still giving humans the positional form, it
+accepts **both**:
+
+| Command | Notes |
+|---|---|
+| `doctor [account]` | Positional account (human shorthand). |
+| `doctor --account <name>` | Retained for agent/script compatibility. If both a positional and `--account` are given and they differ, that's an error. |
+
+### Frozen (untouched)
+`list`, `get`, `search`, `ack`, `send` — `--account`/`--folder`/`--uid…` flags, JSON envelope
+output. **Intentional split**, documented in help: agent commands are flag-driven for parse
+stability; human admin commands are positional. `init`, `version`, `help` unchanged (except `init`
+no longer references whitelists).
+
+## Whitelist decoupling from onboarding
+
+Verified against the code: `WhitelistInEnabled`/`WhitelistOutEnabled` are stored per-account
+booleans, default `false`, read fresh on every command via `Deps.setup` (which rebuilds
+`policy.InboundRule` each call) and `SendCmd`. `policy.InboundRule.Allows` / `OutboundRule.Check`
+only consult them at evaluation time. Nothing in account creation, `init`, or the policy engine
+needs them set at creation time, and toggling them later takes effect on the next command.
+
+Therefore: remove `--whitelist-in` / `--whitelist-out` from `account add`, from `init`'s flow, and
+from the interactive TUI account form (the `Whitelist inbound`/`Whitelist outbound` toggles in
+`internal/tui/account.go`) entirely — leaving them in the form would contradict the decoupling and
+re-expose the empty-enabled-whitelist footgun. Accounts are created with whitelists off; the user
+sets them up afterward:
+
+```
+emcli whitelist add bobby alice@example.com bob@example.com --in
+emcli whitelist enable bobby --in
+```
+
+Default (whitelist off) is the existing permissive default; RO/RW mode and subject regex still
+apply regardless.
+
+## Address validation
+
+`whitelist add` / `whitelist remove` validate each address argument before touching the store.
+Accepted shapes:
+
+- A full address: `local@domain` (non-empty local part, a domain with at least one dot).
+- A domain wildcard: `@domain` (leading `@`, domain with at least one dot).
+
+Anything else (empty, no `@`, no domain) is rejected: `invalid address "x": expected
+local@domain or @domain`. Validation lives in a small reusable helper so `add` and `remove` share
+it. (Addresses are still lowercased on store as today.)
+
+## Error & output conventions (unchanged where already good)
+
+- Usage/errors → stderr; successful human output → stdout.
+- Exit codes: `0` success, `1` runtime error, `2` usage error (preserved).
+- Admin commands remain human-readable (never JSON). `account list` for an agent (agent key only)
+  still emits the reduced JSON view — unchanged.
+
+## Implementation surface
+
+- `internal/cli/admin.go` — rewrite arg parsing for `account`, `whitelist`, `config`, `audit` to
+  the positional grammar; add `account show`, `whitelist enable/disable`, `config list`; add the
+  leftover-positional and empty-operand guards; wire `normalizeVerb`.
+- `internal/cli/run.go` — `doctor` accepts positional + `--account`; routing unchanged otherwise.
+- `internal/cli/interactive.go` (`runInit`) — drop whitelist references.
+- `internal/tui/account.go` — remove the `WhitelistIn`/`WhitelistOut` form fields, their rows,
+  and their render/parse/`ToAccount`/`FieldsFromAccount` handling (new accounts are created with
+  whitelists off; managed via the `whitelist` group thereafter).
+- `internal/cli/help.go` — rewrite admin synopses to the new grammar; document the
+  agent-vs-human positional/flag split; list aliases.
+- New: settings registry (small map + validators) — location `internal/cli` (e.g.
+  `config_registry.go`) unless it needs store access, in which case `internal/store`.
+- New: address validator helper (e.g. `internal/cli` or `internal/policy`, near existing address
+  matching).
+- `internal/store` — add `SetWhitelistEnabled(account, dir, bool)` (or equivalent) if not already
+  expressible via existing account update; `account show` reads existing getters.
+- TTY detection helper for `account remove` confirmation (e.g. `term.IsTerminal` on stdin fd).
+- Docs: `USER-MANUAL.md`, `skills/emcli/AGENTIC-MANUAL.md` (the `doctor --account` example stays
+  valid; verify wording), `README.md` getting-started (no whitelist at init).
+- Tests: `admin_test.go`, `run_test.go`, plus new tests for aliases, leftover-arg rejection,
+  empty-operand rejection, address validation, `config list`/unknown-key, `whitelist
+  enable/disable` (incl. empty-list warning), `account show`, and `account remove` confirm path.
+
+## What breaks (accepted)
+
+Every current admin invocation using `--account` / `--name` / `--address` on `account`,
+`whitelist`, `config` (positional already), `audit`, plus `account edit --whitelist-in/out` and
+`account add --whitelist-in/out`. The agent path (skill + installer drive only the frozen JSON
+commands; `doctor --account` retained) is unaffected.
+
+## Open questions
+
+None outstanding — all design forks resolved with the user.

internal/cli/admin.go

@@ -330,7 +330,19 @@ func runWhitelist(args []string, role store.Role, out, errOut io.Writer) int {
 		return 2
 	}
 	dir := store.Direction(args[0])
+	if dir != store.DirIn && dir != store.DirOut {
+		fmt.Fprintf(errOut, "whitelist direction must be \"in\" or \"out\", got %q\n", args[0])
+		fmt.Fprintln(errOut, "usage: emcli whitelist <in|out> <add|remove|list> [flags]")
+		return 2
+	}
 	sub, rest := args[1], args[2:]
+	switch sub {
+	case "add", "remove", "list": // valid
+	default:
+		fmt.Fprintf(errOut, "unknown whitelist subcommand %q (want add|remove|list)\n", sub)
+		fmt.Fprintln(errOut, "usage: emcli whitelist <in|out> <add|remove|list> [flags]")
+		return 2
+	}
 	fs := flag.NewFlagSet("whitelist", flag.ContinueOnError)
 	fs.SetOutput(errOut)
 	account := fs.String("account", "", "account name")
@@ -371,9 +383,6 @@ func runWhitelist(args []string, role store.Role, out, errOut io.Writer) int {
 		for _, a := range addrs {
 			fmt.Fprintln(out, a)
 		}
-	default:
-		fmt.Fprintf(errOut, "unknown whitelist subcommand %q\n", sub)
-		return 2
 	}
 	return 0
 }

internal/cli/admin_test.go

@@ -110,6 +110,51 @@ func TestAccountEditPartialPreservesOtherFields(t *testing.T) {
 	}
 }
 
+// A missing direction (e.g. `whitelist list`) must report the real problem —
+// the in|out direction — not the misleading "--account is required".
+func TestWhitelistMissingDirectionReported(t *testing.T) {
+	adminEnv(t)
+	code, _, errOut := run(t, "whitelist", "list", "--account", "bobby")
+	if code == 0 {
+		t.Fatal("missing direction must be a usage error")
+	}
+	if strings.Contains(errOut, "--account is required") {
+		t.Fatalf("misleading error; want a direction complaint, got: %q", errOut)
+	}
+	if !strings.Contains(errOut, "in") || !strings.Contains(errOut, "out") {
+		t.Fatalf("error should name the in|out direction, got: %q", errOut)
+	}
+}
+
+// A missing subcommand (e.g. `whitelist out --account x`) must report the real
+// problem — the add|remove|list subcommand — not "--account is required".
+func TestWhitelistMissingSubcommandReported(t *testing.T) {
+	adminEnv(t)
+	code, _, errOut := run(t, "whitelist", "out", "--account", "bobby")
+	if code == 0 {
+		t.Fatal("missing subcommand must be a usage error")
+	}
+	if strings.Contains(errOut, "--account is required") {
+		t.Fatalf("misleading error; want a subcommand complaint, got: %q", errOut)
+	}
+	if !strings.Contains(errOut, "add") || !strings.Contains(errOut, "list") {
+		t.Fatalf("error should name the add|remove|list subcommand, got: %q", errOut)
+	}
+}
+
+// The happy path still works after the direction/subcommand validation.
+func TestWhitelistListWorks(t *testing.T) {
+	adminEnv(t)
+	run(t, "account", "add", "--name", "bobby", "--imap-host", "h", "--username", "u@x.com")
+	if code, _, e := run(t, "whitelist", "out", "add", "--account", "bobby", "--address", "@x.com"); code != 0 {
+		t.Fatalf("add failed: %s", e)
+	}
+	code, out, _ := run(t, "whitelist", "out", "list", "--account", "bobby")
+	if code != 0 || !strings.Contains(out, "@x.com") {
+		t.Fatalf("list: code=%d out=%q", code, out)
+	}
+}
+
 func TestAuditListCoreRenders(t *testing.T) {
 	st, err := store.Open(filepath.Join(t.TempDir(), "e.db"))
 	if err != nil {

internal/store/store.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strconv"
+	"strings"
 
 	_ "modernc.org/sqlite"
 )
@@ -77,10 +78,24 @@ func (s *Store) migrate() error {
 
 func (s *Store) Close() error { return s.db.Close() }
 
+// expandUserHome replaces a leading "~" or "~/" in p with the user's home
+// directory. Only a leading tilde is expanded (the usual shell convention) —
+// "~user" and a tilde elsewhere in the path are left untouched. This guards
+// against an EMCLI_DB set to a literal "~/..." (no shell to expand it), which
+// would otherwise be opened relative to the cwd and create a stray "~" dir.
+func expandUserHome(p string) string {
+	if p == "~" || strings.HasPrefix(p, "~/") {
+		if home, err := os.UserHomeDir(); err == nil {
+			return filepath.Join(home, strings.TrimPrefix(p[1:], "/"))
+		}
+	}
+	return p
+}
+
 // DefaultDBPath resolves EMCLI_DB or the per-OS default location.
 func DefaultDBPath() (string, error) {
 	if p := os.Getenv("EMCLI_DB"); p != "" {
-		return p, nil
+		return expandUserHome(p), nil
 	}
 	if runtime.GOOS == "windows" {
 		if dir := os.Getenv("AppData"); dir != "" {

internal/store/store_test.go

@@ -2,10 +2,53 @@ package store
 
 import (
 	"database/sql"
+	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
+// A leading "~" in EMCLI_DB must be expanded to the home dir, so a literal
+// tilde (no shell to expand it) can't be opened relative to the cwd and
+// silently create a stray "~" directory.
+func TestDefaultDBPathExpandsLeadingTilde(t *testing.T) {
+	home, err := os.UserHomeDir()
+	if err != nil {
+		t.Skipf("no home dir: %v", err)
+	}
+	cases := map[string]string{
+		"~/.config/emcli/emcli.db": filepath.Join(home, ".config", "emcli", "emcli.db"),
+		"~":                        home,
+	}
+	for in, want := range cases {
+		t.Setenv("EMCLI_DB", in)
+		got, err := DefaultDBPath()
+		if err != nil {
+			t.Fatalf("DefaultDBPath(%q): %v", in, err)
+		}
+		if got != want {
+			t.Fatalf("EMCLI_DB=%q -> %q, want %q", in, got, want)
+		}
+		if strings.Contains(got, "~") {
+			t.Fatalf("EMCLI_DB=%q left a literal tilde: %q", in, got)
+		}
+	}
+}
+
+// A non-leading tilde or "~user" is NOT a path we should rewrite — leave it be.
+func TestDefaultDBPathLeavesOtherPathsUntouched(t *testing.T) {
+	for _, p := range []string{"/var/lib/emcli.db", "./rel/emcli.db", "~user/db"} {
+		t.Setenv("EMCLI_DB", p)
+		got, err := DefaultDBPath()
+		if err != nil {
+			t.Fatalf("DefaultDBPath(%q): %v", p, err)
+		}
+		if got != p {
+			t.Fatalf("EMCLI_DB=%q was rewritten to %q", p, got)
+		}
+	}
+}
+
 // openTemp opens a fresh store in a temp dir and initialises keys so that
 // account tests (which do crypto) work without needing their own setup.
 func openTemp(t *testing.T) *Store {

skills/emcli/AGENTIC-MANUAL.md

@@ -54,7 +54,7 @@ checksum, makes it executable in `~/.local/bin` (ensure that's on your PATH), an
 
 | Variable | Default | Purpose |
 |---|---|---|
-| `EMCLI_VERSION` | `v0.5.1` | Release tag to fetch |
+| `EMCLI_VERSION` | `v0.5.2` | Release tag to fetch |
 | `EMCLI_BASE_URL` | `https://gitea.dcglab.co.uk/steve/emcli` | Repo base URL |
 | `EMCLI_INSTALL_DIR` | `$HOME/.local/bin` | Install location |
 

skills/emcli/scripts/install.sh

@@ -7,17 +7,17 @@
 #   bash install.sh
 #
 # Environment overrides:
-#   EMCLI_VERSION      release tag to fetch          (default: v0.5.1)
+#   EMCLI_VERSION      release tag to fetch          (default: v0.5.2)
 #   EMCLI_BASE_URL     repo base URL                 (default: https://gitea.dcglab.co.uk/steve/emcli)
 #   EMCLI_INSTALL_DIR  where to put the binary        (default: $HOME/.local/bin)
 #
 # Release assets follow this naming scheme:
-#   emcli_<version>_<os>_<arch>[.exe]   e.g. emcli_0.5.1_linux_amd64
+#   emcli_<version>_<os>_<arch>[.exe]   e.g. emcli_0.5.2_linux_amd64
 #   checksums.txt                       (sha256, one "<sum>  <asset>" line per asset)
 
 set -euo pipefail
 
-VERSION="${EMCLI_VERSION:-v0.5.1}"
+VERSION="${EMCLI_VERSION:-v0.5.2}"
 BASE_URL="${EMCLI_BASE_URL:-https://gitea.dcglab.co.uk/steve/emcli}"
 INSTALL_DIR="${EMCLI_INSTALL_DIR:-$HOME/.local/bin}"