docs: spec for human-facing CLI grammar redesign

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-06-27 11:21:23 +01:00
parent 3c5e0a26f3
commit 7a4d2881ba
@@ -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 <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` 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.