From 8f20f6f945f49460bf5f2d1090dae580a78c49a2 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 7 Jul 2026 12:27:47 -0600 Subject: [PATCH] fix(em): flag/star dead-case-label bug + help discoverability (Phase 1+2) Per SPEC-em-ux-refactor-2026-07-07.md, from this session's ADHD-UX audit of em. P0 fix: em()'s dispatch case statement had "star|flag)" bound to _em_star in one section AND "flag|fl)" bound to _em_flag in another, earlier section. Since zsh case matches top-down, `em flag` always resolved to _em_flag (one-way add) and never reached _em_star's toggle logic -- the "flag" alias on the star arm was dead code. Fixed the routing so each command name maps to exactly one function; also removed a harmless duplicate `move|mv)` label. _em_star extended to accept multiple IDs (toggling each independently against one consistent folder snapshot) to match flag/unflag's existing batch capability. Added tests/test-em-flag-star.zsh: routing regression tests (flag/star/ unflag each hit a distinct function), multi-ID parity tests, and a grep-based dead-code guard that specifically detects the same case-label word bound to two different handlers (not just any repeated word -- verified against a false positive on legitimate multi-word aliases). Verified the guard actually catches this exact bug class via a planted- defect positive control (reintroduced the old duplicate label, confirmed the guard fails with the right message, restored the fix). Phase 2: added `em help ` (inbox/compose/organize/manage/safety/ ai/search) so users aren't stuck scanning a ~90-line dump for one thing; no-arg `em help` output is unchanged. Documented the existing 3-tier confirmation system ([y/N/e] / type "yes" / type folder name) in the SAFETY section as an intentional design, not an incidental discovery. Updated MASTER-DISPATCHER-GUIDE.md and the man page to match, including fixing a duplicate `em move` reference row and a stale "em flag is an alias for em star" note that mirrored the same underlying bug. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 6 + docs/CHANGELOG.md | 6 + docs/reference/MASTER-DISPATCHER-GUIDE.md | 9 +- lib/dispatchers/email-dispatcher.zsh | 156 ++++++++++++++++++--- man/man1/em.1 | 17 ++- tests/run-all.sh | 1 + tests/test-em-flag-star.zsh | 160 ++++++++++++++++++++++ 7 files changed, 322 insertions(+), 33 deletions(-) create mode 100755 tests/test-em-flag-star.zsh diff --git a/CHANGELOG.md b/CHANGELOG.md index e11a5db20..2cc373de2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 upstream in favor of Antigravity/`agy`, but nothing is removed here. - **`tests/test-teach-dispatcher-characterization.zsh`** — 35 routing checks that mock action functions and verify every public `teach ` path before and after the split. +- **`em help `** — filter help output to one section (`inbox`, `compose`, `organize`, `manage`, `safety`, `ai`, `search`) instead of the full ~90-line dump; unrecognized topics list the valid names. No-arg `em help` output is unchanged. +- **`em help`'s SAFETY section** now documents the three confirmation tiers (`[y/N/e]` / type `yes` / type folder name) as an intentional graduated-friction design, not an incidental discovery. + +### Fixed + +- **`em flag`/`em star` dead-code bug** — a duplicate `case` label in `em()`'s dispatcher silently made `em flag` always resolve to the one-way `_em_flag` handler, never the toggle `_em_star` handler its `star|flag)` alias implied. Fixed the routing; `em flag`/`em unflag` remain one-way batch operations, `em star` remains the toggle — now also extended to accept multiple IDs like `flag`/`unflag` already did. See `docs/specs/SPEC-em-ux-refactor-2026-07-07.md`. ### Changed diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index a60bef042..3415a2f2a 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -19,6 +19,12 @@ The format follows [Keep a Changelog](https://keepachangelog.com/), and this pro upstream in favor of Antigravity/`agy`, but nothing is removed here. - **`tests/test-teach-dispatcher-characterization.zsh`** — 35 routing checks that mock action functions and verify every public `teach ` path before and after the split. +- **`em help `** — filter help output to one section (`inbox`, `compose`, `organize`, `manage`, `safety`, `ai`, `search`) instead of the full ~90-line dump; unrecognized topics list the valid names. No-arg `em help` output is unchanged. +- **`em help`'s SAFETY section** now documents the three confirmation tiers (`[y/N/e]` / type `yes` / type folder name) as an intentional graduated-friction design, not an incidental discovery. + +### Fixed + +- **`em flag`/`em star` dead-code bug** — a duplicate `case` label in `em()`'s dispatcher silently made `em flag` always resolve to the one-way `_em_flag` handler, never the toggle `_em_star` handler its `star|flag)` alias implied. Fixed the routing; `em flag`/`em unflag` remain one-way batch operations, `em star` remains the toggle — now also extended to accept multiple IDs like `flag`/`unflag` already did. See `docs/specs/SPEC-em-ux-refactor-2026-07-07.md`. ### Changed diff --git a/docs/reference/MASTER-DISPATCHER-GUIDE.md b/docs/reference/MASTER-DISPATCHER-GUIDE.md index c92e7c799..380404fd7 100644 --- a/docs/reference/MASTER-DISPATCHER-GUIDE.md +++ b/docs/reference/MASTER-DISPATCHER-GUIDE.md @@ -3300,10 +3300,10 @@ calendar ICS parsing, and IMAP IDLE background watching. | `em delete --purge ` | | Permanent delete (requires typing "yes") | | `em create-folder ` | `em cf` | Create a new mail folder | | `em delete-folder ` | `em df` | Delete folder (type-to-confirm) | -| `em move ` | `em mv` | Move email to folder | +| `em move [FOLDER]` | `em mv` | Move email to folder (fzf picker if no folder) | | `em restore ` | | Restore from Trash to INBOX | -| `em flag ` | `em fl` | Star email for follow-up | -| `em unflag ` | | Remove star | +| `em flag ...` | `em fl` | One-way: set starred (batch-capable) | +| `em unflag ...` | | One-way: clear starred (batch-capable) | | `em respond` | `em resp` | Batch AI drafts for actionable emails | | `em classify ` | | AI category classification | | `em summarize ` | `em sum` | One-line AI summary | @@ -3321,9 +3321,8 @@ calendar ICS parsing, and IMAP IDLE background watching. | `em watch start\|stop\|status\|log` | `em w` | IMAP IDLE background watcher [experimental] | | `em cache stats\|prune\|clear\|warm` | | Manage AI cache | | `em ai [backend]` | | Show/switch AI backend (claude, agy, gemini legacy, none, toggle, auto) | -| `em star ` | `em flag` | Toggle star (Flagged) on email | +| `em star ...` | | Toggle starred status per-ID (batch-capable) | | `em starred` | | List all starred/flagged emails | -| `em move [FOLDER]` | `em mv` | Move email to folder (fzf picker if no folder) | | `em thread ` | `em th` | Show conversation thread for email | | `em snooze