From 7a4d2881ba06933934469ede88b1f4fac35e533d Mon Sep 17 00:00:00 2001 From: Steve Cliff Date: Sat, 27 Jun 2026 11:21:23 +0100 Subject: [PATCH] docs: spec for human-facing CLI grammar redesign Co-Authored-By: Claude Opus 4.8 (1M context) --- ...06-27-human-cli-grammar-redesign-design.md | 195 ++++++++++++++++++ 1 file changed, 195 insertions(+) create mode 100644 docs/superpowers/specs/2026-06-27-human-cli-grammar-redesign-design.md diff --git a/docs/superpowers/specs/2026-06-27-human-cli-grammar-redesign-design.md b/docs/superpowers/specs/2026-06-27-human-cli-grammar-redesign-design.md new file mode 100644 index 0000000..a744f0c --- /dev/null +++ b/docs/superpowers/specs/2026-06-27-human-cli-grammar-redesign-design.md @@ -0,0 +1,195 @@ +# 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 [--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 [--flags]` | `name` positional; only `name` → interactive form prefilled. **`--whitelist-in`/`--whitelist-out` removed.** | +| `account remove [--yes]` | `name` positional. On a TTY, prompt `Remove account "bobby"? [y/N]`; non-TTY (piped) requires `--yes`. Aliases: `rm`, `del`. | +| `account show ` | **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 --in\|--out` | One or more addresses, each validated (see Address validation). Aliases for verb: n/a. | +| `whitelist remove --in\|--out` | One or more addresses. Aliases: `rm`, `del`. | +| `whitelist list --in\|--out` | Header line shows `ENABLED`/`DISABLED`. Alias: `ls`. | +| `whitelist enable --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 --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 ` | Rejects unknown key. | +| `config set ` | 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 ` | 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` and from `init`'s flow +entirely. 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/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.