diff --git a/AGENTS.md b/AGENTS.md index 7efb1ff5..84ea872a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -546,7 +546,32 @@ makes a reconnect safe: the cursor, not the notification, is the source of truth missed broadcast costs nothing, and a 409 means catch up in full instead. A read that fails leaves the cursor where it was and is retried on a doubling backoff, so a change isn't lost with the notification that announced it, and a subscription that closes without -the watch being interrupted is an error rather than a quiet exit. +the watch being interrupted is an error rather than a quiet exit. `ready` waits for that +retry too (`catchUp`/`retryUnread`/`readyOnceCaughtUp`): a catch-up that left a box behind +owes its ready until the box is read, and a drop in between cancels the debt — the +reconnect's catch-up announces its own. The look at the transition queue and the ready +announcement are one critical section with `noteConnection`'s queueing, which is what keeps +a drop from landing between them. + +New mail is a watch event, not a flag: every added and updated line carries `"new": +true|false` and `--events new` selects the true ones (`internal/cmd/watch_new.go`). New is +unseen, not muted, and `active_at` later than the watch's record of the thread — or than +the watch's start, on HEY's clock (`serverNow`), for a thread it has no record of — and +every posting the watch reads is recorded, in every box and whatever `--events` or `--box` +says (`--box` picks the boxes whose changes are reported; every box is followed), each +posting recorded as soon as it is classified. The +start is the Date header translated back to when the request was made (mail that lands while +the server answers is later than the start), it is taken before the box list, and each +box's cursor starts no later than it (`noLaterThan`): the server bakes the box's last posting +activity into the cursor, so mail that landed in between would otherwise sit behind the +cursor, read by nothing. That +is HEY's semantics and state across events, so the CLI decides it once; what to do about +it is the reader's. A 409 skip-ahead sets that box's floor at the cursor it skipped to +(`newMail.skippedTo`): activity at or before it is never new there, known thread or not, +because the watch never read the gap. `resync` is an event of its own — reported by default, +left out by `--events new` — so a script for new mail never runs on one. The Omarchy bar plugin toasts from those lines itself (app-name, glyph, +click-to-focus and the replace-not-stack id all live in the plugin), and nothing +desktop-shaped lives in `watch*.go`. The TUI's mail list follows the same channel and wants less from it. `internal/cmd/tui_watch.go` subscribes and relays the changed box IDs down a channel; `internal/tui/live.go` defines diff --git a/README.md b/README.md index 2bd256cc..4720da3b 100644 --- a/README.md +++ b/README.md @@ -388,16 +388,29 @@ hey watch # follow every box, a line of JSON per c hey watch --box imbox --events added # only new postings in the Imbox hey watch --box imbox --exit-on-first # block until something lands, then exit hey watch --since 2026-08-18T09:00:00Z # catch up from a time first, then follow -hey watch --run-async 'notify-send "New mail in $HEY_BOX_KIND"' +hey watch --box imbox --events new # new mail only: unseen, unmuted, active since the watch began +hey watch --box imbox --events new --exit-on-first # block until new mail +hey watch --box imbox --events new --run-async 'notify-send -a HEY "New mail in HEY"' hey watch --run-sync ./triage.sh # one at a time, waiting for each ``` Runs until interrupted, printing changes as they happen, one line each: ```json -{"change":"added","at":"2026-08-18T09:14:22.031Z","box":{"id":24088,"kind":"imbox","name":"Imbox"},"posting_id":98765,"thread_id":54321,"posting":{}} +{"change":"added","at":"2026-08-18T09:14:22.031Z","box":{"id":24088,"kind":"imbox","name":"Imbox"},"posting_id":98765,"thread_id":54321,"new":true,"posting":{}} ``` +Every `added` and `updated` line says whether the posting is new mail: unseen, not muted, +and active since the watch last saw the thread — or since the watch began, for a thread it +has not seen, so a box's backlog is never new. Reading, muting or moving a thread is not new +activity; a reply on a known thread is. `--events new` selects the new ones, alone or in a +union with `added`, `updated`, `deleted` and `resync` — the default is everything but `new`, +and `new` alone leaves a `resync` out, so a script for new mail never runs on one. `--box` +picks the boxes whose changes are reported; every box is followed regardless, so what is new +is judged across all of them — a reply in The Feed and then a move into the Imbox is not. The +one-liner above is what any desktop does with it; on Omarchy the bar plugin reads the same +lines and sends one batched, replacing toast instead. + A change can drive a command instead of being printed, and there's a choice to make between two behaviours — pass one or the other, not both. `--run-async` spawns the command per change and moves on, so a slow one never holds up the watch and two can @@ -406,7 +419,8 @@ slow one delays the next. Both hand the JSON to the command on its stdin, and the same fields as `HEY_CHANGE`, `HEY_AT`, `HEY_BOX_ID`, `HEY_BOX_KIND`, `HEY_BOX_NAME`, `HEY_POSTING_ID` and -`HEY_THREAD_ID`. Both also take over stdout, so the JSON isn't printed as well. +`HEY_THREAD_ID`, with `HEY_NEW=1` for new mail and `HEY_NEW=0` otherwise. Both also take over +stdout, so the JSON isn't printed as well. ### Calendars @@ -491,21 +505,29 @@ read from `~/.local/state/omarchy/current/theme/`, and restyles live when you ru — an explicitly chosen file is trusted as written — or `NO_COLOR=1` to turn color off. ```bash -yay -S hey-cli # hey-cli is on the AUR +omarchy pkg aur add hey-cli # hey-cli is on the AUR +omarchy plugin add https://github.com/basecamp/omarchy-hey-plugin.git --enable hey setup omarchy # install into the desktop -hey setup omarchy --notify # also toast new Imbox mail (--no-notify turns it off) +hey setup omarchy --notify # toast new mail from the bar plugin (--no-notify turns it off) hey setup omarchy --remove # take it all out again ``` -Setup installs a `HEY TUI` launcher entry, a `HEY` row in the SUPER+SPACE menu, a bar -indicator that lights when the Imbox has unread mail (no count, by design), and a +The bar gets the [HEY plugin](https://github.com/basecamp/omarchy-hey-plugin): the HEY +logo lights when the Imbox has unseen mail and opens a panel of recent threads. hey-cli is +its engine — the plugin reads the Imbox with `hey box imbox` and runs `hey watch` so the +bar is live: a thread you archive in the TUI, on your phone or in the web app leaves the +panel within a second, and after a disconnect the watch catches up from where it left off. + +Setup installs a `HEY TUI` launcher entry, a `HEY` row in the SUPER+SPACE menu, and a `hey.toml.tpl` theme template so theme authors can tune the overlay. It prints the `bindings.lua` snippet for a keybinding rather than editing your file. Omarchy's shipped HEY web app, its SUPER+SHIFT+E binding and the mailto handler are left untouched. -`--notify` turns on new-mail toasts, off by default: the bar indicator's poll also sends -at most one notification per interval — `Sender — Subject` for one new thread, `N new in -Imbox` for more — replacing the previous toast rather than stacking, and clicking it +`--notify` turns on new-mail toasts, off by default, by flipping the plugin's `notify` +setting (the panel's toggle and `omarchy bar set 37signals.hey notify true --json` do the +same). The plugin runs `hey watch` and toasts its new-mail lines for the Imbox itself — at +most one notification per burst of changes, `Sender — Subject` for one new thread, `N new +in Imbox` for more, replacing the previous toast rather than stacking, and clicking it focuses the TUI. Omarchy's notification silencing (SUPER+CTRL+comma) mutes them like any other app. See [docs/omarchy.md](docs/omarchy.md) for the details and what is planned next. diff --git a/docs/omarchy.md b/docs/omarchy.md index fd6ad376..c4be4974 100644 --- a/docs/omarchy.md +++ b/docs/omarchy.md @@ -61,14 +61,15 @@ not themed — that is the point of ANSI. ### `hey setup omarchy` -Idempotent; `--remove` reverses every piece; each step is reported separately and one -failing step does not stop the others. +Idempotent; `--remove` reverses every piece but one — the bar plugin's `notify` setting, +which is the plugin's own (see its row) — and each step is reported separately; one failing +step does not stop the others. | Piece | Where | Notes | |---|---|---| | Desktop entry | `~/.local/share/applications/HEY TUI.desktop` | Distinct from Omarchy's shipped `HEY.desktop` web app. Launches under app-id `org.omarchy.hey` | | Menu row | marker block in `~/.config/omarchy/extensions/omarchy-menu.jsonc` | one root `HEY` row that focuses or launches the TUI; its guard is a PATH lookup, never network or `hey` itself. Becomes a submenu once there is more than one thing to open | -| Bar indicator | inline command module `hey-unread` in `~/.config/omarchy/shell.json` | runs `hey omarchy bar-status` every 3 minutes; click focuses or launches the TUI. `--notify` / `--no-notify` toggle new-mail toasts by rewriting the module's exec — enablement lives where it acts, no config key | +| Bar plugin | the `37signals.hey` entry in `~/.config/omarchy/shell.json`'s bar layout | not installed by setup — `omarchy plugin add https://github.com/basecamp/omarchy-hey-plugin.git --enable` does that — but configured by it: `--notify` / `--no-notify` set or delete the entry's `notify` key, which the shell hot-reloads and the plugin reads to decide whether to toast. `--remove` leaves the key alone: it is set as readily from the panel or `omarchy bar set` as from here, so removal cannot tell a preference it wrote from one it didn't — `--no-notify` is the off switch. An earlier inline `hey-unread` module is removed on sight, its notify choice carried over | | Theme template | `~/.config/omarchy/themed/hey.toml.tpl` | renders `hey.toml` into every theme so theme authors can override the overlay; triggers `omarchy-theme-refresh` | | Keybinding | printed, never written | `o.bind("SUPER + SHIFT + ALT + H", "HEY TUI", "omarchy-launch-or-focus-tui --app-id=org.omarchy.hey hey tui")`; SUPER+SHIFT+E keeps opening the web app unless you `hl.unbind` it. Spelled out rather than `{ tui = "hey tui" }` because the lua helper quotes that into one word and the app-id derived from it would never match | @@ -77,67 +78,118 @@ Every surface — launcher, menu, bar click, keybinding — uses the same app-id rather than `TUI.float`: the float class is shared by every floating TUI, and focus-or-launch would grab whichever one was open. -If the user's `shell.json` has no `bar.layout` yet, the default layout from -`$OMARCHY_PATH/config/omarchy/shell.json` is copied in first; the shell treats a missing -layout as "use the defaults", so adding one module means spelling out the rest. A +Setup never adds a bar module of its own any more, so it never has to seed a layout. The +one place it still reads Omarchy's default layout is the legacy removal: the old install +copied the defaults in just to hold its module, and if removing the module leaves exactly +the current defaults, the layout goes too and the user is back to inheriting them. A `shell.json` that is not plain JSON is left alone and the step reports failure. -### `hey omarchy bar-status` - -Hidden command the bar module runs. Prints -`{"text":"","tooltip":"Unread in Imbox","class":"active"}` when the Imbox has unread -mail and nothing otherwise (the `text` is the nf-fa-envelope glyph U+F0E0, which most -browsers render as nothing — it is not empty). HEY orders Imbox postings unseen-first, -so one page decides: any unread mail is on page 1. Logged out or offline also prints nothing and exits 0 — a bar -is no place for an error message. Credentials come from the keyring or the -`credentials.json` fallback exactly as for any other command, so it works from the -shell's headless context; token refresh happens in-process. - -### New-mail toasts (default off) - -`hey setup omarchy --notify` rewrites the bar module's exec to -`hey omarchy bar-status --notify`: the same 3-minute poll that lights the indicator also -diffs the unseen Imbox postings against a fingerprint file -(`~/.local/state/hey-cli/omarchy-poll.json`) and sends **at most one toast per tick** via -`omarchy-notification-send` — sparse notices, never a per-message firehose. One Imbox -fetch serves both the indicator and the toasts. - -- **What counts as new**: an unseen posting not fingerprinted yet, or one whose - `visible_entry_count` grew (a new reply on a known thread). Fingerprints avoid - `updated_at` (it churns) and `seen` (it flips on read). Muted threads are fingerprinted - but never toast. -- **First run seeds silently.** No state file means write the fingerprints and toast - nothing — never toast the backlog. The fingerprints carry the identity they were taken - for — server, account filter and the signed-in user's id — so after - `hey accounts use`, a base URL change, or signing in as someone else by any route - (login, logout, `HEY_TOKEN`) the next tick reseeds silently instead of toasting the - other identity's backlog. Re-enabling with `--notify` after a `--no-notify` stretch - drops stale fingerprints for the same reason, and `--remove` keeps them while the bar - module could not actually be removed. -- **The whole unseen set is read when seeding.** HEY sorts Imbox postings unseen-first, - so the poll follows pages while they are all-unseen and stops at the first seen - posting. A seed (first run, or a new identity) reads them all, so no pre-existing - thread can later surface as new; a steady-state tick stops at ten pages, because new - mail always lands on page 1 and older threads are already fingerprinted. The - indicator-only path reads one page. Fingerprints prune to the postings still unseen - once the snapshot is complete; a truncated snapshot (cap reached, a page fetch - failed) keeps absent fingerprints instead. -- **One toast, replaced not stacked.** `Sender — Subject` for one new thread, `N new in - Imbox` with the first few senders for more. The daemon's printed id (`-r -p`, the - `omarchy-display-text-size` pattern) is cached so the next tick replaces the on-screen - toast instead of stacking; a stale id after a shell restart just makes a fresh toast. -- **DND is honored.** The toast passes `--app-name HEY` deliberately: omarchy's default - app-name `omarchy-action` bypasses notification silencing, so identifying as HEY is - what makes SUPER+CTRL+comma mute the toasts (into history) like any other app. -- **Clicking focuses the TUI** via the shared `omarchy-launch-or-focus-tui` exec hint, - which the shell runs itself so it survives shell restarts. -- **Same silence discipline as the bar**: any error — auth, network, a failed send — - produces no output beyond the bar JSON and exits 0. A failed fetch leaves the - fingerprints untouched, and a failed send keeps the undelivered postings out of them - so the toast retries on the next tick. - -`hey setup omarchy --no-notify` reverts the exec; a plain re-run leaves it as it is; -`--remove` deletes the state file along with everything else. +### The bar plugin + +[`basecamp/omarchy-hey-plugin`](https://github.com/basecamp/omarchy-hey-plugin) is the +face: the HEY logo in the bar (tinted when there is unseen mail), the panel with its +account switcher, `New for you` / `Previously seen` tabs, the Screener count, mark-as-seen, +and the setup flow that installs `hey-cli` from the AUR and signs you in. hey-cli is the +engine, and the plugin composes its generic commands: `hey box imbox --json` for the Imbox, +`hey watch` to know when to read it again, `hey screener list --count` for the Screener, +`hey seen` for marking, `hey accounts list` and `hey auth status` for the rest. The +division was settled when both sides turned out to have built a bar indicator without +knowing of the other — two HEY icons, two Imbox pollers, two product stances in one slot — +and the answer was: **the CLI is the engine, the plugin is the face.** + +The plugin is an Omarchy `service` plugin as well as a bar widget. The shell instantiates +the service once, so one `hey watch` runs per shell however many monitors carry the bar, +and every bar widget reads the shared service. A watch event is a wake-up, not a delta — +any line on the watch's stdout re-reads `hey box imbox`, debounced so a burst of changes +costs one read (plus one follow-up when changes land while a read is in flight, since that +read may predate them). `hey watch` says `ready` once every box is caught up and its +subscription is live, and again after every reconnect's catch-up; the plugin's read on that +line is what makes the picture gap-free — anything before the cursor is in the read, +anything after it is an event — rather than the order the two processes were started in. +It says `disconnected` when the cable drops, which is what the panel's live state follows, +and `resync` when a box changed more than the feed can list, which is just another reason +to re-read. `hey watch` catches up from its cursor on reconnect and skips ahead on that 409, +so a laptop back from suspend is current within seconds; the plugin's timer poll stays only +as a safety net. + +The watch covers every box, not just the Imbox. A move out of the Imbox writes nothing in +the Imbox's own feed — it is an upsert in the box the thread went to — so an Imbox-only +watch would never see a thread leave. And the TUI's mutations come back over the cable like +anyone else's, within a second, which is why the TUI does not nudge the plugin over IPC. + +When the watch exits with the `auth` error envelope the service shows the sign-in state and +stops restarting it until the next probe succeeds; any other exit restarts it on a backoff. + +Settings flow the way every Omarchy bar widget's do: the plugin's manifest declares +`notify` (default off) alongside its refresh interval and thread limit, and the values +live as extra keys on the `{"id":"37signals.hey"}` entry in `shell.json`, hot-reloaded by +the shell. Three ways to flip a key, all equivalent: `hey setup omarchy --notify`, the +toggle in the panel header, or `omarchy bar set 37signals.hey notify true --json`. Flipping +`notify` only gates the plugin's toasts; the watch runs on regardless, and nothing restarts. + +### New mail is a watch event + +What counts as new mail needs HEY's semantics and state across events, so the CLI decides +it once: every `added` and `updated` line `hey watch` writes carries `"new": true|false`, +and `--events new` selects the true ones. The rule: + +- **New** is a posting that is unseen, not muted, and whose `active_at` is later than the + watch last recorded for the thread — or later than the watch's start, when it has no + record. That start is read off HEY's own clock (the `Date` header of one request, + translated back to the moment the request was made), the clock every `active_at` is on, + so a workstation running fast or slow neither calls the backlog new nor sits on new mail; + whole seconds, rounded down, so the doubt falls on the side of calling mail a moment old + new, and every box's cursor starts no later than that start, so mail that lands while the + watch is starting up is read and is new. `active_at` moves on new mail only, not on a + seen flip, a mute or a move, so reading a thread, marking it unseen again or moving it + into a box is never new, and a reply on a known thread is. A box's first read is its + catch-up from the server's cursor — the box's last activity, not this moment — so it + carries backlog, which the start-time rule keeps out, alongside anything that arrived + while the watch was starting, which is new. +- **Every posting the watch reads is recorded**, in every box and whatever `--events` or + `--box` reports — `--box` picks what is reported, every box is followed — so a thread + known from a filtered-out change, or from another box, is never mistaken for new when its + next change is reported. A read is classified before it is + recorded. There is no state file: the record lives and dies with the watch. +- **`--events` is a union.** `new` alone is new mail only — a `resync` is not, so a script + for new mail never runs on one; `added,new` is every arrival plus new activity on known + threads; the default `added,updated,deleted,resync` is every line, each saying whether it + is new. `hey watch --box imbox --events new --exit-on-first` is "block until new mail", and + a `--run-*` script sees `HEY_NEW=1` or `HEY_NEW=0`. +- **A skip-ahead sets a floor.** A box that answered 409 was never read across the gap, so + the cursor it skips to — the box's last posting activity — becomes that box's floor: + activity at or before it is never new there, on a thread the watch knows or one it does + not, so a reply the watch missed and then a move while still unseen is not new mail. Mail + after the floor is. The floor is the box's own; a gap thread moved to another box is + measured there and may read as new once — the `resync` line is the cue to re-read. + +### The plugin toasts + +The toast is presentation, and a desktop's business: the plugin watches every box (it has +to, to see a thread leave the Imbox), reads the `new` lines whose box is the Imbox — HEY's +attention model puts new mail in one place — and sends the toast itself through +`omarchy-notification-send`, the shell's own wrapper over `notify-send`. + +- **One toast per burst, replaced not stacked.** The new lines of one read arrive together; + a short debounce collects them into one toast — `Sender — Subject` for one thread, `N new + in Imbox` with the first few senders for more. The daemon's printed id (`-p`) is kept in + memory and passed back as `-r` for ten minutes, so the next burst replaces the toast on + screen instead of stacking; after that a fresh toast, since ids are daemon-local and a + shell restart may have handed the number to another application. +- **DND is honoured.** The toast goes out under `--app-name HEY`: Omarchy's default + app-name `omarchy-action` deliberately bypasses notification silencing, so identifying + as HEY is what makes SUPER+CTRL+comma mute the toasts (into history) like any other app. +- **Click focuses the TUI.** `--exec omarchy-launch-or-focus-tui --app-id=org.omarchy.hey + hey tui` — the shell runs it daemon-side, so it survives shell restarts — with the bar's + envelope as the glyph. +- **No state file.** Nothing is remembered but the last toast's id, in the shell's memory; + turning toasts on is nothing more than setting the key. +- **Mail text never reads as an option.** A subject or sender can start with a dash, and + `notify-send` parses one wherever it appears; a leading word joiner (U+2060) makes the + argument a plain positional and is invisible on screen. + +Any other desktop gets the same stream with its own face: `hey watch --box imbox --events +new --run-async 'notify-send -a HEY "New mail in HEY"'` is the one-liner. ## Decisions @@ -149,8 +201,28 @@ fetch serves both the indicator and the toasts. is the cautionary precedent. - **Complement the shipped web app, never replace it.** Distinct desktop name, printed keybinding, the mailto handler left alone. -- **No HTML scraping to feed widgets.** The indicator uses the same typed SDK read as - `hey box imbox`. +- **No HTML scraping to feed widgets.** The plugin reads with `hey box imbox`, the same + typed SDK read as everyone else. +- **CLI is the engine, plugin is the face.** The plugin owns rendering and settings — the + toast included; the CLI owns what needs HEY's semantics — the changes cursor and catching + up after a disconnect, what counts as new mail — where it is Go-tested once instead of + re-derived in QML. The plugin's panel does show an unread *number* per + account; the bar icon itself stays a glyph that lights or does not, and the toast never + carries the unread total either — `N new in Imbox` is how many threads arrived in one + batch, not how many are waiting. That divergence is the plugin's call and is recorded + here rather than papered over. +- **Generic commands; the face presents.** There is no `hey omarchy` group and nothing + desktop-shaped in the CLI at all: the plugin composes `hey box`, `hey watch`, `hey + screener` and `hey seen`, and a user can run any of them by hand. What counts as new + mail is a watch event — HEY semantics, decided once — and what to do about it (a toast + under the shell's app-name, a glyph, a click that focuses the TUI) is the plugin's. + (Two earlier cuts were built and superseded before release: a `hey omarchy poll` — one + Imbox read that also diffed a fingerprint file to toast — that was ten minutes stale by + construction and an Omarchy-named command on a general CLI; and a `hey watch --notify` + that sent the toast itself through `notify-send` with Omarchy hints, which bundled a + desktop's presentation into a generic command and scoped the Imbox with a flag of its + own, where `--events new` and the plugin's own read of the box do the same without + either.) ## Follow-ups, in rough order @@ -162,16 +234,13 @@ fetch serves both the indicator and the toasts. `claude.json.tpl`; an Install-menu TUI row; possibly branching the mailto handler to the TUI when installed. (An AUR package already ships: `yay -S hey-cli`, published by the release workflow.) -4. **Shell plugin graduation** for the bar widget: `manifest.json`, a settings panel, - and event-driven freshness — refreshing the indicator the moment a thread is - archived in the TUI. That needs a real widget plugin: inline `command` modules are - interval-only, with no IPC to force a re-run (`Bar.qml` has no `IpcHandler` and - `omarchy bar` has no refresh verb), which is also why the toasts share the interval - poll rather than pushing. +4. **Deltas applied in the plugin**: every watch event already carries the full posting, + but the plugin re-reads the Imbox per batch today. Applying events in place — reading + only on `ready` and `resync` — would make an ordinary change cost no API read at all. ## Anti-features, recorded -- No unread **count**. +- No unread **count** in the bar icon or the toast. - No per-message notification firehose. - No full hex theme port. - No auto-editing `~/.config/hypr/bindings.lua`. diff --git a/internal/cmd/accounts_test.go b/internal/cmd/accounts_test.go index 1b94f582..d28584c5 100644 --- a/internal/cmd/accounts_test.go +++ b/internal/cmd/accounts_test.go @@ -28,7 +28,6 @@ func TestCommandAccountScopePolicy(t *testing.T) { {args: []string{"accounts", "list"}, want: false}, {args: []string{"auth", "status"}, want: false}, {args: []string{"config", "show"}, want: false}, - {args: []string{"omarchy", "bar-status"}, want: false}, } { command, _, err := root.Find(test.args) if err != nil { diff --git a/internal/cmd/local_config_trust.go b/internal/cmd/local_config_trust.go index cd638f50..7ed38245 100644 --- a/internal/cmd/local_config_trust.go +++ b/internal/cmd/local_config_trust.go @@ -50,18 +50,15 @@ func ensureLocalConfigTrusted(cmd *cobra.Command) error { // commandIgnoresLocalConfig reports whether a command reads only the global and // environment configuration, so a repository-local .hey/config.json is never -// even parsed for it. The bar poller runs from the shell's working directory, -// wherever that happens to be: a local config must neither redirect it to -// another server nor fail it (trust gate or malformed file) — the indicator -// has to stay dark rather than error. setup omarchy only edits fixed desktop -// paths and must not be blocked by a checkout's config either. +// even parsed for it — neither to redirect it to another server nor to fail it +// (trust gate or malformed file). func commandIgnoresLocalConfig(cmd *cobra.Command) bool { parts := strings.Fields(cmd.CommandPath()) if len(parts) < 2 { return false } switch parts[1] { - case "omarchy", "skill": + case "skill": return true case "setup": // The wizard itself uses the effective server; its subcommands diff --git a/internal/cmd/omarchy.go b/internal/cmd/omarchy.go index d1fa14b7..b4b3a31b 100644 --- a/internal/cmd/omarchy.go +++ b/internal/cmd/omarchy.go @@ -16,17 +16,13 @@ import ( "github.com/spf13/cobra" - "github.com/basecamp/hey-sdk/go/pkg/generated" - hey "github.com/basecamp/hey-sdk/go/pkg/hey" - "github.com/basecamp/hey-cli/internal/apierr" - "github.com/basecamp/hey-cli/internal/mail" "github.com/basecamp/hey-cli/internal/output" ) // Omarchy integration: `hey setup omarchy` installs hey-cli into the desktop -// (launcher entry, menu rows, bar indicator, theme template) and `hey omarchy -// bar-status` is the command the bar indicator runs. +// (launcher entry, menu rows, theme template) and configures the 37signals.hey +// bar plugin, which runs `hey watch` (watch.go, watch_new.go). // // Omarchy already ships HEY as a web app (SUPER+SHIFT+E, the mailto handler, a // HEY.desktop). Everything here complements that under its own names and never @@ -38,7 +34,8 @@ var omarchyThemeTemplate string const ( omarchyAppID = "org.omarchy.hey" omarchyDesktopName = "HEY TUI" - omarchyBarModuleID = "hey-unread" + omarchyBarModuleID = "hey-unread" // the inline module earlier releases installed; removed on sight + omarchyBarPluginID = "37signals.hey" // the bar plugin's module id in shell.json's layout omarchyMenuBegin = " // >>> hey-cli — managed by `hey setup omarchy`, do not edit between the markers" omarchyMenuEnd = " // <<< hey-cli" omarchyFocusCommand = "omarchy-launch-or-focus-tui --app-id=" + omarchyAppID + " hey tui" @@ -66,7 +63,6 @@ type omarchyEnv struct { omarchyPath string iconRoots []string // icon theme roots searched for Omarchy's HEY icon run func(name string, args ...string) error - runOutput func(name string, args ...string) (string, error) } func liveOmarchyEnv() omarchyEnv { @@ -85,15 +81,6 @@ func liveOmarchyEnv() omarchyEnv { cmd.Stdout, cmd.Stderr = io.Discard, io.Discard return cmd.Run() }, - runOutput: func(name string, args ...string) (string, error) { - if _, err := exec.LookPath(name); err != nil { - return "", err - } - ctx, cancel := context.WithTimeout(context.Background(), omarchyCommandTimeout) - defer cancel() - out, err := exec.CommandContext(ctx, name, args...).Output() //nolint:gosec // G204: fixed omarchy command names - return string(out), err - }, } } @@ -162,33 +149,19 @@ type omarchyStep struct { type omarchySetup struct { env omarchyEnv - notify *bool // nil keeps the bar module's current exec as it is + notify *bool // nil keeps the bar plugin's notify setting as it is } func (s omarchySetup) apply() []omarchyStep { - return []omarchyStep{ - s.installDesktop(), - s.installMenu(), - s.installBar(), - s.installTemplate(), - } + steps := []omarchyStep{s.installDesktop(), s.installMenu()} + steps = append(steps, s.configureBarPlugin()...) + return append(steps, s.installTemplate()) } func (s omarchySetup) remove() []omarchyStep { - steps := []omarchyStep{ - s.removeDesktop(), - s.removeMenu(), - s.removeBar(), - s.removeTemplate(), - } - // The fingerprints go only once the module that uses them is gone: while a - // failed bar removal leaves the poller scheduled, deleting them would make - // its next tick reseed and swallow the mail that arrived in between. - if steps[2].failure != nil { - return append(steps, omarchyStep{Name: "poll state", Path: omarchyPollStatePath(), Status: "kept", - Detail: "bar module still installed; fingerprints kept for its next tick"}) - } - return append(steps, s.removePollState()) + steps := []omarchyStep{s.removeDesktop(), s.removeMenu()} + steps = append(steps, s.removeBar()...) + return append(steps, s.removeTemplate()) } func stepResult(name, path string, changed bool, err error, installed, unchanged string) omarchyStep { @@ -344,28 +317,18 @@ func stripMenuBlock(content string) string { return content[:start] + after } -// Bar: an inline command module in shell.json's bar layout. The shell hot-reloads -// the file, so the indicator appears as soon as it is written. Toast enablement -// lives in the module's exec string — no config key, visible where it acts, -// removed with --remove. - -func omarchyBarExec(notify bool) string { - if notify { - return "hey omarchy bar-status --notify" - } - return "hey omarchy bar-status" -} +// Bar: the unread indicator is the 37signals.hey bar plugin +// (github.com/basecamp/omarchy-hey-plugin), which runs `hey watch`. Setup +// does not install it — a plugin is a git clone the user adds with `omarchy +// plugin add` — but it finds the plugin's layout entry in shell.json and flips +// its `notify` setting there, the way `omarchy bar set` would; the shell +// hot-reloads the file. Earlier releases installed an inline `hey-unread` +// command module instead, which setup now removes — two pollers in one slot — +// carrying its notify choice over to the plugin when the plugin has none. +// The toasts are the plugin's own, composed from the new-mail lines of `hey +// watch`: no state file, so turning them on is nothing more than setting the key. -func omarchyBarModule(notify bool) map[string]any { - return map[string]any{ - "id": omarchyBarModuleID, - "type": "command", - "exec": omarchyBarExec(notify), - "interval": 180, - "tooltip": "HEY", - "onClick": omarchyFocusCommand, - } -} +const omarchyBarPluginInstall = "omarchy plugin add https://github.com/basecamp/omarchy-hey-plugin.git --enable" func notifyDetail(notify bool) string { if notify { @@ -374,93 +337,129 @@ func notifyDetail(notify bool) string { return "notifications off" } -func (s omarchySetup) installBar() omarchyStep { +// configureBarPlugin reports one step for the plugin and, only when there was +// one to remove, a step for the legacy module before it. +func (s omarchySetup) configureBarPlugin() []omarchyStep { path := s.env.shellPath() shell, err := s.loadShellConfig() if err != nil { - return stepResult("bar indicator", path, false, err, "", "") + return []omarchyStep{stepResult("bar plugin", path, false, err, "", "")} } - layout, err := s.barLayout(shell) + layout, err := existingBarLayout(shell) if err != nil { - return stepResult("bar indicator", path, false, err, "", "") + return []omarchyStep{stepResult("bar plugin", path, false, err, "", "")} } - module := barLayoutModule(layout, omarchyBarModuleID) - notify := s.notify != nil && *s.notify - if module == nil { - if notify { - // Enabling toasts with stale fingerprints around would toast the - // accumulated diff; if they cannot be dropped, do not enable. - if _, err := removeFileIfPresent(omarchyPollStatePath()); err != nil { - return stepResult("bar indicator", path, false, fmt.Errorf("cannot drop stale poll state: %w", err), "", "") - } - } - right, ok := layout["right"].([]any) - if raw, present := layout["right"]; present && raw != nil && !ok { - return stepResult("bar indicator", path, false, fmt.Errorf("shell.json: bar.layout.right is %T, not a list", raw), "", "") + legacy, legacyNotified := s.removeLegacyBarModule(shell, layout) + var steps []omarchyStep + if legacy { + steps = append(steps, omarchyStep{Name: "bar indicator", Path: path, Status: "removed", + Detail: "hey-unread module removed; the " + omarchyBarPluginID + " plugin replaces it"}) + } + + plugin := barLayoutModule(layout, omarchyBarPluginID) + if plugin == nil { + detail := "install the bar plugin: " + omarchyBarPluginInstall + if legacyNotified { + detail += "; then hey setup omarchy --notify to keep the toasts" } - layout["right"] = append([]any{omarchyBarModule(notify)}, right...) - changed, err := writeJSONFile(path, shell) - step := stepResult("bar indicator", path, changed, err, "installed", "unchanged") - if err == nil && s.notify != nil { - step.Detail = notifyDetail(notify) + steps = append(steps, omarchyStep{Name: "bar plugin", Path: path, Status: "skipped", Detail: detail}) + return s.writeBarSteps(steps, shell, legacy) + } + + want := s.notify + if want == nil && legacyNotified { + // The legacy module was toasting and the plugin has not been told + // either way: keep the user's choice rather than silently turning + // the toasts off with the module. + if _, has := plugin["notify"]; !has { + on := true + want = &on } - return step } - // An existing module is reconciled field by field, keeping its section and - // position, so a re-run after an upgrade picks up a changed exec, click - // command or interval; only the notify choice is preserved when the caller - // did not state one. - exec, _ := module["exec"].(string) - wasNotifying := strings.HasSuffix(exec, " --notify") - if s.notify == nil { - notify = wasNotifying - } - if notify && !wasNotifying { - // Turning toasts (back) on: drop any stale fingerprints so the first - // tick reseeds from the current Imbox instead of toasting whatever - // accumulated while they were off. If they cannot be dropped, fail - // rather than enable a poller that would toast the backlog. - if _, err := removeFileIfPresent(omarchyPollStatePath()); err != nil { - return stepResult("bar indicator", path, false, fmt.Errorf("cannot drop stale poll state: %w", err), "", "") + // The plugin step reports its own setting only: a legacy removal in the + // same pass is the other step's news. + settingChanged := false + if want != nil { + current, has := plugin["notify"] + switch { + case *want && current != true: + plugin["notify"] = true + settingChanged = true + case !*want && has: + delete(plugin, "notify") + settingChanged = true } } - desired := omarchyBarModule(notify) - changed := !sameJSON(module, desired) - if changed { - clear(module) - for key, value := range desired { - module[key] = value - } - if _, err := writeJSONFile(path, shell); err != nil { - return stepResult("bar indicator", path, false, err, "", "") - } + step := omarchyStep{Name: "bar plugin", Path: path, Status: "unchanged", Detail: notifyDetail(barPluginNotifies(plugin))} + if settingChanged { + step.Status = "installed" } - step := stepResult("bar indicator", path, changed, nil, "installed", "unchanged") - if s.notify != nil { - step.Detail = notifyDetail(notify) + return s.writeBarSteps(append(steps, step), shell, legacy || settingChanged) +} + +// writeBarSteps writes shell.json when anything changed and turns every +// reported step into a failure when the write did not land — a removal or a +// setting that is not on disk did not happen. +func (s omarchySetup) writeBarSteps(steps []omarchyStep, shell map[string]any, changed bool) []omarchyStep { + if !changed { + return steps } - return step + if err := writeJSONFile(s.env.shellPath(), shell); err != nil { + for i := range steps { + steps[i].Status, steps[i].Detail, steps[i].failure = "failed", err.Error(), err + } + } + return steps } -// sameJSON compares two values by their JSON encoding, which is what makes a -// decoded float64(180) and a literal 180 read as equal. -func sameJSON(a, b any) bool { - left, errA := json.Marshal(a) - right, errB := json.Marshal(b) - return errA == nil && errB == nil && bytes.Equal(left, right) +// barPluginNotifies reads the plugin entry's notify setting as the plugin does: +// only a JSON true turns toasts on. +func barPluginNotifies(plugin map[string]any) bool { + notify, _ := plugin["notify"].(bool) + return notify } -func (s omarchySetup) removeBar() omarchyStep { +// removeBar takes out what setup wrote into the bar layout: a legacy hey-unread +// module. The plugin entry and its notify setting stay — the entry is the +// user's, added with omarchy plugin add, and the setting is the plugin's, set +// as readily from its panel or `omarchy bar set` as from here, so removal +// cannot tell a preference it wrote from one it didn't. --no-notify is the +// explicit way to turn the toasts off. +func (s omarchySetup) removeBar() []omarchyStep { path := s.env.shellPath() shell, err := s.loadShellConfig() if err != nil { - return stepResult("bar indicator", path, false, err, "", "") + return []omarchyStep{stepResult("bar indicator", path, false, err, "", ""), stepResult("bar plugin", path, false, err, "", "")} } bar, _ := shell["bar"].(map[string]any) layout, _ := bar["layout"].(map[string]any) - if barLayoutModule(layout, omarchyBarModuleID) == nil { - return stepResult("bar indicator", path, false, nil, "", "absent") + legacy, _ := s.removeLegacyBarModule(shell, layout) + plugin := omarchyStep{Name: "bar plugin", Path: path, Status: "absent"} + if entry := barLayoutModule(layout, omarchyBarPluginID); entry != nil { + plugin.Status = "kept" + plugin.Detail = "the plugin and its notify setting are yours; hey setup omarchy --no-notify turns the toasts off" + if !barPluginNotifies(entry) { + plugin.Detail = "the plugin is yours; notifications are off" + } } + steps := []omarchyStep{stepResult("bar indicator", path, legacy, nil, "removed", "absent"), plugin} + return s.writeBarSteps(steps, shell, legacy) +} + +// removeLegacyBarModule drops the inline hey-unread module earlier releases +// installed, reporting whether there was one and whether it was toasting. +// Only the map form is ours: a string-form entry sharing the id is unowned +// and stays. Install used to seed the layout from Omarchy's defaults just to +// hold the module; if what remains is exactly the current defaults, the +// layout goes too, so the user is back to inheriting future default-layout +// changes. +func (s omarchySetup) removeLegacyBarModule(shell, layout map[string]any) (removed, notified bool) { + module := barLayoutModule(layout, omarchyBarModuleID) + if module == nil { + return false, false + } + exec, _ := module["exec"].(string) + notified = strings.HasSuffix(exec, " --notify") for section, entries := range layout { list, ok := entries.([]any) if !ok { @@ -468,25 +467,28 @@ func (s omarchySetup) removeBar() omarchyStep { } kept := make([]any, 0, len(list)) for _, entry := range list { - // Only the map form is ours: install treats string-form entries as - // unowned, so removal must too. if _, isMap := entry.(map[string]any); !isMap || barEntryID(entry) != omarchyBarModuleID { kept = append(kept, entry) } } layout[section] = kept } - // Install may have seeded the layout from Omarchy's defaults just to hold our - // module. If what remains is exactly the current defaults, drop it so the user - // goes back to inheriting future default-layout changes. - if defaults, defErr := s.defaultBarLayout(); defErr == nil && sameJSON(layout, defaults) { + if defaults, err := s.defaultBarLayout(); err == nil && sameJSON(layout, defaults) { + bar, _ := shell["bar"].(map[string]any) delete(bar, "layout") if len(bar) == 0 { delete(shell, "bar") } } - changed, err := writeJSONFile(path, shell) - return stepResult("bar indicator", path, changed, err, "removed", "absent") + return true, notified +} + +// sameJSON compares two values by their JSON encoding, which is what makes a +// decoded float64(180) and a literal 180 read as equal. +func sameJSON(a, b any) bool { + left, errA := json.Marshal(a) + right, errB := json.Marshal(b) + return errA == nil && errB == nil && bytes.Equal(left, right) } // loadShellConfig reads the user's shell.json. The shell ignores any config @@ -532,32 +534,24 @@ func decodeJSONObject(data []byte) (map[string]any, error) { return object, nil } -// barLayout returns the user's bar layout, seeding it from Omarchy's default layout -// when the user has never customized the bar — the shell treats a missing layout -// as "use the defaults", so adding one module means spelling the rest out too. -func (s omarchySetup) barLayout(shell map[string]any) (map[string]any, error) { - // A present value of the wrong type is someone's configuration, not an - // absence: refuse to replace it. +// existingBarLayout returns the user's bar layout, or nil when shell.json has +// none — the shell then uses its defaults, and a plugin entry can only be in a +// layout that is spelled out. A present value of the wrong type is someone's +// configuration, not an absence: refuse to touch the file. +func existingBarLayout(shell map[string]any) (map[string]any, error) { bar, ok := shell["bar"].(map[string]any) if raw, present := shell["bar"]; present && raw != nil && !ok { return nil, fmt.Errorf("shell.json: bar is %T, not an object", raw) } - if bar == nil { - bar = map[string]any{} - shell["bar"] = bar - } layout, ok := bar["layout"].(map[string]any) if raw, present := bar["layout"]; present && raw != nil && !ok { return nil, fmt.Errorf("shell.json: bar.layout is %T, not an object", raw) } - if ok { - return layout, nil - } - layout, err := s.defaultBarLayout() - if err != nil { - return nil, fmt.Errorf("no bar layout in shell.json to extend: %w", err) + for section, entries := range layout { + if _, isList := entries.([]any); !isList && entries != nil { + return nil, fmt.Errorf("shell.json: bar.layout.%s is %T, not a list", section, entries) + } } - bar["layout"] = layout return layout, nil } @@ -671,15 +665,6 @@ func (s omarchySetup) removeTemplate() omarchyStep { return stepResult("theme template", path, changed, err, "removed", "absent") } -// Poll state: the new-mail fingerprint file bar-status --notify keeps. Setup -// never creates it, but --remove takes it out with everything else. - -func (s omarchySetup) removePollState() omarchyStep { - path := omarchyPollStatePath() - changed, err := removeFileIfPresent(path) - return stepResult("poll state", path, changed, err, "removed", "absent") -} - // --- File helpers --- // writeFileIfChanged writes via a temp file and rename, the way Omarchy's own @@ -730,15 +715,16 @@ func removeFileIfPresent(path string) (bool, error) { return err == nil, err } -func writeJSONFile(path string, value any) (bool, error) { +func writeJSONFile(path string, value any) error { var buf bytes.Buffer encoder := json.NewEncoder(&buf) encoder.SetIndent("", " ") encoder.SetEscapeHTML(false) if err := encoder.Encode(value); err != nil { - return false, err + return err } - return writeFileIfChanged(path, buf.Bytes(), 0o644) + _, err := writeFileIfChanged(path, buf.Bytes(), 0o644) + return err } // --- hey setup omarchy --- @@ -758,12 +744,16 @@ func newSetupOmarchyCommand() *setupOmarchyCommand { Args: cobra.NoArgs, Short: "Install hey into the Omarchy desktop", Long: `Install hey into the Omarchy desktop: a launcher entry, rows in the SUPER+SPACE -menu, an unread indicator on the bar, and a theme template so themes can tune the -TUI's accent colors. Every piece is idempotent and --remove takes them all out again. +menu, and a theme template so themes can tune the TUI's accent colors. Every piece +is idempotent and --remove takes them all out again — all but the bar plugin's notify +setting, which is the plugin's own; --no-notify is how to turn that off. ---notify also toasts new Imbox mail on the indicator's poll — at most one toast per -interval, replaced rather than stacked, silenced by the notification DND toggle. ---no-notify turns the toasts back off; a plain re-run leaves them as they are. +The bar indicator is the 37signals.hey plugin, which runs hey watch; install it +with omarchy plugin add https://github.com/basecamp/omarchy-hey-plugin.git --enable. +--notify turns on the bar plugin's new-mail toasts (one per batch of changes at +most, replaced rather than stacked, silenced by the notification DND toggle) by +setting notify on the plugin's entry in shell.json. --no-notify turns them back +off; a plain re-run leaves them as they are. Theming needs none of this: on Omarchy the TUI already follows the active theme.`, Example: ` hey setup omarchy @@ -775,7 +765,7 @@ Theming needs none of this: on Omarchy the TUI already follows the active theme. RunE: setupOmarchyCommand.run, } setupOmarchyCommand.cmd.Flags().BoolVar(&setupOmarchyCommand.remove, "remove", false, "Remove everything hey setup omarchy installed") - setupOmarchyCommand.cmd.Flags().BoolVar(&setupOmarchyCommand.notify, "notify", false, "Toast new Imbox mail when the bar indicator polls") + setupOmarchyCommand.cmd.Flags().BoolVar(&setupOmarchyCommand.notify, "notify", false, "Turn on the bar plugin's new-mail toasts") setupOmarchyCommand.cmd.Flags().BoolVar(&setupOmarchyCommand.noNotify, "no-notify", false, "Turn new-mail toasts back off") setupOmarchyCommand.cmd.MarkFlagsMutuallyExclusive("notify", "no-notify") setupOmarchyCommand.cmd.MarkFlagsMutuallyExclusive("notify", "remove") @@ -858,150 +848,3 @@ func (c *setupOmarchyCommand) run(cmd *cobra.Command, args []string) error { } return writeOK(data, output.WithSummary(summary)) } - -// --- hey omarchy bar-status --- - -type omarchyCommand struct { - cmd *cobra.Command -} - -func newOmarchyCommand() *omarchyCommand { - omarchyCommand := &omarchyCommand{} - omarchyCommand.cmd = &cobra.Command{ - Use: "omarchy", - Short: "Commands the Omarchy desktop integration runs", - Hidden: true, - } - omarchyCommand.cmd.AddCommand(newOmarchyBarStatusCommand().cmd) - return omarchyCommand -} - -type omarchyBarStatusCommand struct { - cmd *cobra.Command - notify bool - env omarchyEnv -} - -func newOmarchyBarStatusCommand() *omarchyBarStatusCommand { - omarchyBarStatusCommand := &omarchyBarStatusCommand{env: liveOmarchyEnv()} - omarchyBarStatusCommand.cmd = &cobra.Command{ - Use: "bar-status", - Short: "Print the bar indicator for unread Imbox mail", - Long: `Print a Waybar-style JSON module when the Imbox has unread mail and nothing when -it does not. Never fails: when hey is logged out or offline the indicator simply -stays dark, because a bar is no place for an error message. - -With --notify, also toast newly unseen Imbox mail via omarchy-notification-send — -at most one toast per run, replacing the previous one rather than stacking.`, - Args: cobra.NoArgs, - RunE: omarchyBarStatusCommand.run, - } - omarchyBarStatusCommand.cmd.Flags().BoolVar(&omarchyBarStatusCommand.notify, "notify", false, "Toast new unseen Imbox mail") - return omarchyBarStatusCommand -} - -// configDegraded is set by the root pre-run when the global configuration could -// not be loaded and a config-ignoring command went on with the defaults. The -// poller then stays dark: lighting the indicator against a guessed server would -// be a lie, and an error is not an option either. -var configDegraded bool - -func (c *omarchyBarStatusCommand) run(cmd *cobra.Command, args []string) error { - if configDegraded || !authMgr.IsAuthenticated() { - return nil - } - // The omarchy command is exempt from pre-run account scoping so a failed - // selection (offline, account gone) can never surface as an error here; - // the configured account still applies when it can be selected. - if err := selectConfiguredAccount(cmd.Context()); err != nil { - return nil //nolint:nilerr // a bar is no place for an error message - } - // The indicator only needs to know whether anything is unseen, which the - // first page answers. Toasts need the unseen set: capped on a steady-state - // tick (new mail always lands on page 1), exhaustive when seeding — a first - // run or a new identity — so no pre-existing thread can later read as new. - pages, identity, notify := 1, "", c.notify - if notify { - var ok bool - if identity, ok = omarchyPollIdentity(cmd.Context()); !ok { - notify = false - } else if state, existed := loadOmarchyPollState(); !existed || state.Identity != identity { - pages = unseenSeedPageCap - } else { - pages = unseenPageCap - } - } - unseen, complete, ok := unseenImboxPostings(cmd.Context(), pages) - if !ok { - return nil - } - if notify { - notifyNewMail(c.env, identity, unseen, complete) - } - if len(unseen) == 0 { - return nil - } - _, err := fmt.Fprintln(cmd.OutOrStdout(), omarchyBarModuleJSON()) - return err -} - -// omarchyBarModuleJSON is the Waybar-style module the bar shows when mail is -// unread. The "active" class is what the shell's command widget highlights. -func omarchyBarModuleJSON() string { - module, _ := json.Marshal(map[string]string{ // fixed strings cannot fail to marshal - "text": omarchyBarGlyph, - "tooltip": "Unread in Imbox", - "class": "active", - }) - return string(module) -} - -// Page limits for following an all-unseen Imbox. A steady-state tick stops at -// ten pages — three hundred unseen threads — because new mail always lands on -// page 1 and older threads are already fingerprinted. Seeding reads the whole -// unseen set (bounded only as the box command is) so that no pre-existing -// thread can later surface as new; it happens once per identity. -var ( - unseenPageCap = 10 - // maxPostingPages counts the initial page, as `hey box` does, so a box whose - // unseen set spans exactly the cap still finds its closing seen page and - // seeds completely. - unseenSeedPageCap = maxPostingPages -) - -// unseenImboxPostings returns the unseen Imbox postings, whether they are the -// complete unseen set, and whether the fetch succeeded. HEY orders Imbox -// postings unseen-first, so a page holding any seen posting (or nothing at -// all) closes the unseen set; while a page is all unseen the next one is -// fetched, up to maxPages. Unknown (offline, server error) counts as clear: -// the indicator stays dark rather than lying either way loudly, and the notify -// fingerprints stay untouched. -func unseenImboxPostings(ctx context.Context, maxPages int) (unseen []generated.Posting, complete, ok bool) { - imbox, err := sdk.Boxes().GetImbox(ctx, nil) - if err != nil || imbox == nil { - return nil, false, false - } - source := mail.Source{Kind: mail.KindBox, ID: imbox.Id, BoxKind: hey.BoxKindImbox} - page := mail.Page{Postings: imbox.Postings, Cursor: imbox.NextHistoryUrl} - for pages := 1; ; pages++ { - seenOnPage := false - for _, posting := range page.Postings { - if posting.Seen { - seenOnPage = true - } else { - unseen = append(unseen, posting) - } - } - if seenOnPage || len(page.Postings) == 0 || page.Cursor == "" { - return unseen, true, true - } - if pages >= maxPages { - return unseen, false, true - } - // A page that cannot be fetched leaves what was read as a truncated - // snapshot: still enough to light the bar, not enough to prune by. - if page, err = mail.ReadPage(ctx, sdk, source, page.Cursor); err != nil { - return unseen, false, true - } - } -} diff --git a/internal/cmd/omarchy_notify.go b/internal/cmd/omarchy_notify.go deleted file mode 100644 index 41e87abf..00000000 --- a/internal/cmd/omarchy_notify.go +++ /dev/null @@ -1,257 +0,0 @@ -package cmd - -import ( - "context" - "encoding/json" - "fmt" - "os" - "path/filepath" - "strconv" - "strings" - "time" - - "github.com/basecamp/hey-sdk/go/pkg/generated" - - "github.com/basecamp/hey-cli/internal/config" -) - -// New-mail toasts, driven by the same bar tick as the unread indicator so the -// Imbox is fetched once per interval. `hey omarchy bar-status --notify` diffs -// the unseen postings against a fingerprint file and sends at most one toast -// per tick via omarchy-notification-send — sparse notices, never a firehose. -// -// The app-name matters: omarchy's default `omarchy-action` deliberately pops -// through Do Not Disturb, so the toast identifies as HEY and -// omarchy-toggle-notification-silencing is honored for free. - -const omarchyNotifyAppName = "HEY" - -// omarchyPollState fingerprints the unseen Imbox postings a previous tick saw. -// Seen maps posting id to its visible entry count, so a new reply on a known -// thread (count grew) toasts like a new thread does. ToastID is the daemon's -// id for our last toast; passing it back with -r replaces the on-screen toast -// instead of stacking a new one each tick. Identity records which server and -// account the fingerprints belong to: after `hey accounts use` or a base URL -// change the state reseeds silently instead of toasting the other account's -// backlog. -type omarchyPollState struct { - Identity string `json:"identity,omitempty"` - Seen map[string]int32 `json:"seen"` - ToastID int `json:"toast_id,omitempty"` - ToastAt int64 `json:"toast_at,omitempty"` // unix seconds the toast was sent -} - -// toastReplaceWindow bounds how long a cached toast id is reused. Notification -// ids are daemon-local, not stable identities: after a reboot or a shell -// restart the same number may belong to another application's notification, -// and -r would overwrite that instead of replacing ours. Replacement only ever -// matters for back-to-back ticks, so a short window loses nothing. -const toastReplaceWindow = 10 * time.Minute - -// replaceableToastID returns the cached toast id when it is recent enough to -// trust, and 0 otherwise. -func (s omarchyPollState) replaceableToastID(now time.Time) int { - if s.ToastID <= 0 || now.Sub(time.Unix(s.ToastAt, 0)) > toastReplaceWindow { - return 0 - } - return s.ToastID -} - -// omarchyPollIdentity names who the poll runs as: the server (spelled the way -// auth.Manager keys credentials, without a trailing slash), the account -// filter, and the signed-in user's id from the identity endpoint. Keying on -// the user rather than on credentials is what makes every way of becoming -// someone else — login, logout, HEY_TOKEN set, changed or unset — reseed -// silently on the next tick, while token refreshes and rotations change -// nothing. When the identity cannot be fetched the tick skips notifying and -// leaves the fingerprints untouched, exactly as a failed Imbox fetch does. -func omarchyPollIdentity(ctx context.Context) (string, bool) { - identity, err := rootSDK.Identity().GetIdentity(ctx) - if err != nil || identity == nil || identity.Id == 0 { - return "", false - } - return pollIdentity(cfg.BaseURL, cfg.AccountID, strconv.FormatInt(identity.Id, 10)), true -} - -func pollIdentity(baseURL, account, userID string) string { - identity := strings.TrimRight(baseURL, "/") + " " + account - if userID == "" { - return identity - } - return identity + " user:" + userID -} - -func omarchyPollStatePath() string { - return filepath.Join(config.StateDir(), "omarchy-poll.json") -} - -// loadOmarchyPollState returns the saved state and whether a state file -// existed. No file means first run: seed the fingerprints, toast nothing — -// never toast the backlog. -func loadOmarchyPollState() (omarchyPollState, bool) { - state := omarchyPollState{Seen: map[string]int32{}} - data, err := os.ReadFile(omarchyPollStatePath()) - if err != nil { - return state, false - } - if err := json.Unmarshal(data, &state); err != nil { - return omarchyPollState{Seen: map[string]int32{}}, false - } - if state.Seen == nil { - state.Seen = map[string]int32{} - } - return state, true -} - -func saveOmarchyPollState(state omarchyPollState) error { - _, err := writeJSONFile(omarchyPollStatePath(), state) - return err -} - -// notifyNewMail diffs the unseen postings against the fingerprint file and -// sends at most one toast. Errors are swallowed: the bar tick must never turn -// into an error message, and a failed send retries on the next tick because -// the undelivered postings keep their previous fingerprints. complete reports -// whether unseen is the whole unseen Imbox: HEY sorts unseen postings first, -// so a page with any seen posting (or none at all) proves completeness, while -// an all-unseen page may be truncated and then pruning must wait — a thread -// pushed off the page would otherwise toast again when it comes back. -func notifyNewMail(env omarchyEnv, identity string, unseen []generated.Posting, complete bool) { - previous, existed := loadOmarchyPollState() - if previous.Identity != identity { - // Another server or account's fingerprints: reseed silently. - previous, existed = omarchyPollState{Seen: map[string]int32{}}, false - } - - next := omarchyPollState{Identity: identity, Seen: make(map[string]int32, len(unseen)), ToastID: previous.ToastID, ToastAt: previous.ToastAt} - var fresh []generated.Posting - for _, posting := range unseen { - id := strconv.FormatInt(posting.Id, 10) - next.Seen[id] = posting.VisibleEntryCount - known, seenBefore := previous.Seen[id] - if posting.Muted || (seenBefore && posting.VisibleEntryCount <= known) { - continue - } - fresh = append(fresh, posting) - } - if !complete { - for id, count := range previous.Seen { - if _, onPage := next.Seen[id]; !onPage { - next.Seen[id] = count - } - } - } - - if !existed { - // A seed must be the whole unseen set: persisting a snapshot that lost a - // page would leave every thread beyond it unknown, to be toasted as - // backlog the moment it surfaces. Leave no state and seed next tick. - if complete { - _ = saveOmarchyPollState(next) - } - return - } - if len(fresh) == 0 { - _ = saveOmarchyPollState(next) - return - } - // Persist before delivering: a toast whose fingerprints could not be saved - // would come back every tick, so an unsaveable state means no toast at all. - if err := saveOmarchyPollState(next); err != nil { - return - } - now := time.Now() - if id, err := sendMailToast(env, fresh, previous.replaceableToastID(now)); err == nil { - next.ToastID, next.ToastAt = id, now.Unix() - } else { - // Undelivered: restore the fresh postings' previous fingerprints so - // they still diff as new next tick. - for _, posting := range fresh { - id := strconv.FormatInt(posting.Id, 10) - if known, ok := previous.Seen[id]; ok { - next.Seen[id] = known - } else { - delete(next.Seen, id) - } - } - } - _ = saveOmarchyPollState(next) -} - -func sendMailToast(env omarchyEnv, fresh []generated.Posting, replaceID int) (int, error) { - headline, description := composeMailToast(fresh) - args := []string{ - "--glyph", omarchyBarGlyph, - "--app-name", omarchyNotifyAppName, - "-u", "low", - "--exec", omarchyFocusCommand, - notificationText(headline), - } - if description != "" { - args = append(args, notificationText(description)) - } - if replaceID > 0 { - args = append(args, "-r", strconv.Itoa(replaceID)) - } - args = append(args, "-p") - out, err := env.runOutput("omarchy-notification-send", args...) - if err != nil { - return replaceID, err - } - if id, err := strconv.Atoi(strings.TrimSpace(out)); err == nil && id > 0 { - return id, nil - } - return replaceID, nil -} - -// notificationText keeps mail-derived text from being read as an option: -// omarchy-notification-send and notify-send both parse a leading dash, and a -// subject or summary can start with one. A word joiner is invisible on screen -// but makes the argument a plain positional. -func notificationText(text string) string { - if strings.HasPrefix(text, "-") { - return "\u2060" + text - } - return text -} - -// composeMailToast turns the fresh postings into one headline and description: -// `Sender — Subject` for a single thread, a count with the first few senders -// for more. -func composeMailToast(fresh []generated.Posting) (string, string) { - if len(fresh) == 1 { - posting := fresh[0] - description := posting.Summary - if description == postingSubject(posting) { - description = "" // Summary already stood in for a missing subject - } - return postingSender(posting) + " — " + postingSubject(posting), description - } - senders := make([]string, 0, 3) - for _, posting := range fresh { - if len(senders) == 3 { - senders = append(senders, "…") - break - } - senders = append(senders, postingSender(posting)) - } - return fmt.Sprintf("%d new in Imbox", len(fresh)), strings.Join(senders, ", ") -} - -func postingSender(posting generated.Posting) string { - switch { - case posting.AlternativeSenderName != "": - return posting.AlternativeSenderName - case posting.Creator.Name != "": - return posting.Creator.Name - default: - return posting.Creator.EmailAddress - } -} - -func postingSubject(posting generated.Posting) string { - if posting.Name != "" { - return posting.Name - } - return posting.Summary -} diff --git a/internal/cmd/omarchy_notify_test.go b/internal/cmd/omarchy_notify_test.go deleted file mode 100644 index 12a49ee2..00000000 --- a/internal/cmd/omarchy_notify_test.go +++ /dev/null @@ -1,723 +0,0 @@ -package cmd - -import ( - "encoding/json" - "errors" - "fmt" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "slices" - "strings" - "testing" - "time" - - "github.com/basecamp/hey-sdk/go/pkg/generated" -) - -// testNotifyEnv records every runOutput invocation and answers with the given -// stdout, the way omarchy-notification-send -p prints the toast id. -func testNotifyEnv(stdout string) (omarchyEnv, *[][]string) { - var calls [][]string - env := omarchyEnv{ - runOutput: func(name string, args ...string) (string, error) { - calls = append(calls, append([]string{name}, args...)) - return stdout, nil - }, - } - return env, &calls -} - -func unseenPosting(id int64, sender, name string, entries int32) generated.Posting { - return generated.Posting{Id: id, Name: name, VisibleEntryCount: entries, - Creator: generated.Contact{Name: sender}} -} - -// notifyAll runs a tick with a fixed identity and a complete unseen snapshot — -// the common case the older tests exercise. -func notifyAll(env omarchyEnv, unseen []generated.Posting) { - notifyNewMail(env, "test", unseen, true) -} - -func TestNotifyNewMailSeedsStateWithoutToasting(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - notifyAll(env, []generated.Posting{ - unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1), - unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 3), - }) - - if len(*calls) != 0 { - t.Errorf("first run must never toast the backlog, ran %v", *calls) - } - state, existed := loadOmarchyPollState() - if !existed || state.Seen["101"] != 1 || state.Seen["102"] != 3 { - t.Errorf("first run should seed the fingerprints, got %+v (existed=%v)", state, existed) - } -} - -func TestNotifyNewMailToastsOneNewThread(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("42\n") - - known := unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1) - notifyAll(env, []generated.Posting{known}) - fresh := unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1) - fresh.Summary = "Your August invoice is attached." - notifyAll(env, []generated.Posting{known, fresh}) - - if len(*calls) != 1 { - t.Fatalf("want exactly one toast, ran %v", *calls) - } - argv := (*calls)[0] - if argv[0] != "omarchy-notification-send" { - t.Errorf("wrong command: %v", argv) - } - for flag, value := range map[string]string{ - "--glyph": omarchyBarGlyph, "--app-name": "HEY", "-u": "low", "--exec": omarchyFocusCommand, - } { - i := slices.Index(argv, flag) - if i < 0 || argv[i+1] != value { - t.Errorf("%s %q missing from %v", flag, value, argv) - } - } - if !slices.Contains(argv, "Northwind Invoicing — Invoice #4021") || - !slices.Contains(argv, "Your August invoice is attached.") { - t.Errorf("headline/description missing from %v", argv) - } - if slices.Contains(argv, "-r") { - t.Errorf("no cached toast id yet, must not pass -r: %v", argv) - } - if argv[len(argv)-1] != "-p" { - t.Errorf("-p must be passed to learn the toast id: %v", argv) - } - if state, _ := loadOmarchyPollState(); state.ToastID != 42 { - t.Errorf("printed toast id should be cached, got %+v", state) - } -} - -func TestNotifyNewMailReplacesThePreviousToast(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("42\n") - - notifyAll(env, nil) - notifyAll(env, []generated.Posting{unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1)}) - notifyAll(env, []generated.Posting{ - unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1), - unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1), - }) - - if len(*calls) != 2 { - t.Fatalf("want two toasts, ran %v", *calls) - } - argv := (*calls)[1] - i := slices.Index(argv, "-r") - if i < 0 || argv[i+1] != "42" { - t.Errorf("second toast should replace the first via -r 42: %v", argv) - } -} - -func TestNotifyNewMailDoesNotReuseAStaleToastID(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - // A toast id from before a reboot may now belong to another application. - if err := saveOmarchyPollState(omarchyPollState{Identity: "test", Seen: map[string]int32{}, - ToastID: 42, ToastAt: time.Now().Add(-time.Hour).Unix()}); err != nil { - t.Fatal(err) - } - notifyAll(env, []generated.Posting{unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1)}) - - if len(*calls) != 1 || slices.Contains((*calls)[0], "-r") { - t.Errorf("a stale toast id must not be passed as -r, ran %v", *calls) - } - if state, _ := loadOmarchyPollState(); state.ToastID != 7 || state.ToastAt == 0 { - t.Errorf("the fresh toast id and time should be cached, got %+v", state) - } -} - -func TestNotifyNewMailKeepsMailTextOutOfOptionParsing(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - notifyAll(env, nil) - fresh := unseenPosting(101, "-r Systems Ltd", "--help with the quarterly numbers", 1) - fresh.Summary = "-p please see attached" - notifyAll(env, []generated.Posting{fresh}) - - if len(*calls) != 1 { - t.Fatalf("want one toast, ran %v", *calls) - } - argv := (*calls)[0] - for _, arg := range argv[1:] { - if strings.HasPrefix(arg, "-") && !slices.Contains([]string{"--glyph", "--app-name", "-u", "--exec", "-r", "-p"}, arg) { - t.Errorf("mail-derived text must never arrive as an option-looking argument: %q in %v", arg, argv) - } - } - if !slices.Contains(argv, "\u2060-r Systems Ltd — --help with the quarterly numbers") { - t.Errorf("the text itself must be preserved behind the word joiner: %v", argv) - } -} - -func TestPollIdentityIsKeyedOnTheUser(t *testing.T) { - alice := pollIdentity("https://app.hey.com/", "all", "1001") - bob := pollIdentity("https://app.hey.com", "all", "1002") - if alice == bob { - t.Error("a different user on the same server and account is a different identity") - } - if alice != pollIdentity("https://app.hey.com", "all", "1001") { - t.Error("the server spelling must be normalized and the identity stable for one user") - } - if pollIdentity("https://app.hey.com", "all", "") != "https://app.hey.com all" { - t.Error("without a user the identity is just server and account") - } -} - -func TestBarStatusNotifySkipsWhenTheIdentityIsUnavailable(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/imbox.json" { - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"id": 1, "name": "Imbox", "kind": "inbox", "postings": [{"id": 5, "name": "Invoice #4021", "seen": false}]}`)) - return - } - w.WriteHeader(500) // the identity endpoint is down - })) - defer server.Close() - - out, err := runBarStatus(t, server.URL, true, "--notify") - if err != nil || !strings.Contains(out, "active") { - t.Errorf("the bar must still light, got %q, %v", out, err) - } - if _, existed := loadOmarchyPollState(); existed { - t.Error("without knowing who the poll runs as, the fingerprints must stay untouched") - } -} - -func TestOmarchyRemoveKeepsPollStateWhileBarRemovalFails(t *testing.T) { - if os.Geteuid() == 0 { - t.Skip("root can write anywhere") - } - env, _ := testOmarchyEnv(t) - setup := omarchySetup{env: env} - setup.apply() - if err := saveOmarchyPollState(omarchyPollState{Identity: "test", Seen: map[string]int32{"1": 1}}); err != nil { - t.Fatal(err) - } - if err := os.Chmod(env.configDir(), 0o500); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Chmod(env.configDir(), 0o700) }) - - steps := statuses(setup.remove()) - if steps["bar indicator"] != "failed" || steps["poll state"] != "kept" { - t.Errorf("while the bar module cannot be removed the fingerprints must stay, got %v", steps) - } - if _, existed := loadOmarchyPollState(); !existed { - t.Error("poll state was deleted although the poller is still scheduled") - } -} - -func TestNotifyNewMailToastsWhenAThreadGrows(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - thread := unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1) - notifyAll(env, []generated.Posting{thread}) - notifyAll(env, []generated.Posting{thread}) - if len(*calls) != 0 { - t.Fatalf("an unchanged thread must not toast, ran %v", *calls) - } - - thread.VisibleEntryCount = 2 - notifyAll(env, []generated.Posting{thread}) - if len(*calls) != 1 || !slices.Contains((*calls)[0], "Maria Delgado — Lunch on Thursday?") { - t.Errorf("a new reply on a known thread should toast, ran %v", *calls) - } -} - -func TestNotifyNewMailSkipsMutedButRemembersThem(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - notifyAll(env, nil) - muted := unseenPosting(103, "Weekend Deals", "48 hours only", 1) - muted.Muted = true - notifyAll(env, []generated.Posting{muted}) - - if len(*calls) != 0 { - t.Errorf("muted threads must never toast, ran %v", *calls) - } - if state, _ := loadOmarchyPollState(); state.Seen["103"] != 1 { - t.Errorf("muted threads should still be fingerprinted, got %+v", state) - } -} - -func TestNotifyNewMailBatchesIntoOneToast(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - notifyAll(env, nil) - batch := []generated.Posting{ - unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1), - unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1), - unseenPosting(103, "Sam Whitfield", "Draft agenda for Monday", 1), - unseenPosting(104, "Priya Raman", "Photos from the offsite", 1), - } - batch[0].AlternativeSenderName = "Maria (personal)" - notifyAll(env, batch) - - if len(*calls) != 1 { - t.Fatalf("a batch must collapse to one toast, ran %v", *calls) - } - argv := (*calls)[0] - if !slices.Contains(argv, "4 new in Imbox") { - t.Errorf("batch headline missing: %v", argv) - } - if !slices.Contains(argv, "Maria (personal), Northwind Invoicing, Sam Whitfield, …") { - t.Errorf("batch description should list the first senders: %v", argv) - } -} - -func TestNotifyNewMailPrunesDepartedThreads(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, _ := testNotifyEnv("7\n") - - notifyAll(env, []generated.Posting{ - unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1), - unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1), - }) - notifyAll(env, []generated.Posting{unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1)}) - - state, _ := loadOmarchyPollState() - if _, kept := state.Seen["101"]; kept || state.Seen["102"] != 1 { - t.Errorf("fingerprints should prune to the postings still unseen, got %+v", state) - } -} - -func TestNotifyNewMailDoesNotPersistATruncatedSeed(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - // The first tick could not read every unseen page: no seed is written. - notifyNewMail(env, "test", []generated.Posting{unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1)}, false) - if _, existed := loadOmarchyPollState(); existed { - t.Fatal("an incomplete seed must not be persisted") - } - - // The next complete tick seeds silently, including the thread the failed - // page had hidden — which therefore never reads as new. - notifyAll(env, []generated.Posting{ - unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1), - unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1), - }) - if len(*calls) != 0 { - t.Errorf("the retried seed must still be silent, ran %v", *calls) - } - if state, existed := loadOmarchyPollState(); !existed || state.Seen["102"] != 1 { - t.Errorf("the retried seed should fingerprint everything, got %+v", state) - } -} - -func TestNotifyNewMailKeepsFingerprintsOffATruncatedPage(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - notifyAll(env, []generated.Posting{ - unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1), - unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1), - }) - - // An all-unseen page may be cut off; 101 falling off it must not be pruned… - notifyNewMail(env, "test", []generated.Posting{unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1)}, false) - state, _ := loadOmarchyPollState() - if state.Seen["101"] != 1 { - t.Errorf("a thread off a truncated page must keep its fingerprint, got %+v", state) - } - - // …so its return to the page is not mistaken for new mail. - notifyAll(env, []generated.Posting{ - unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1), - unseenPosting(102, "Northwind Invoicing", "Invoice #4021", 1), - }) - if len(*calls) != 0 { - t.Errorf("a re-appearing known thread must not toast, ran %v", *calls) - } -} - -func TestNotifyNewMailRetriesAfterAFailedSend(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - var calls [][]string - failing := omarchyEnv{runOutput: func(name string, args ...string) (string, error) { - calls = append(calls, append([]string{name}, args...)) - return "", errors.New("no notification daemon") - }} - - notifyAll(failing, nil) - fresh := unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1) - notifyAll(failing, []generated.Posting{fresh}) - if len(calls) != 1 { - t.Fatalf("the failed send should have been attempted once, ran %v", calls) - } - if state, _ := loadOmarchyPollState(); state.Seen["101"] != 0 { - t.Errorf("an undelivered posting must not be fingerprinted, got %+v", state) - } - - working, sent := testNotifyEnv("42\n") - notifyAll(working, []generated.Posting{fresh}) - if len(*sent) != 1 { - t.Errorf("the toast must retry on the next tick, ran %v", *sent) - } -} - -func TestNotifyNewMailReseedsWhenIdentityChanges(t *testing.T) { - t.Setenv("XDG_STATE_HOME", t.TempDir()) - env, calls := testNotifyEnv("7\n") - - notifyNewMail(env, "https://app.hey.com all", nil, true) - backlog := unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1) - notifyNewMail(env, "https://app.hey.com 12345", []generated.Posting{backlog}, true) - - if len(*calls) != 0 { - t.Errorf("another account's backlog must reseed silently, ran %v", *calls) - } - state, _ := loadOmarchyPollState() - if state.Identity != "https://app.hey.com 12345" || state.Seen["101"] != 1 { - t.Errorf("state should now fingerprint the new identity, got %+v", state) - } -} - -func TestBarStatusNotifySeedsEveryUnseenPage(t *testing.T) { - var server *httptest.Server - server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch { - case r.URL.Path == "/identity.json": - _, _ = w.Write([]byte(`{"id": 7, "name": "Maria Delgado"}`)) - case r.URL.Path == "/imbox.json" && r.URL.Query().Get("page") == "": - // An all-unseen first page: the unseen set may continue. - fmt.Fprintf(w, `{"id": 1, "name": "Imbox", "kind": "inbox", "next_history_url": %q, - "postings": [{"id": 1, "name": "Newest", "seen": false, "visible_entry_count": 1}, - {"id": 2, "name": "Newer", "seen": false, "visible_entry_count": 1}]}`, - server.URL+"/imbox.json?page=2") - case r.URL.Path == "/imbox.json" && r.URL.Query().Get("page") == "2": - // Older unseen threads, then the first seen one closes the set. - _, _ = w.Write([]byte(`{"id": 1, "name": "Imbox", "kind": "inbox", "next_history_url": "", - "postings": [{"id": 3, "name": "Older but unseen", "seen": false, "visible_entry_count": 4}, - {"id": 4, "name": "Already read", "seen": true, "visible_entry_count": 1}]}`)) - default: - w.WriteHeader(404) - } - })) - defer server.Close() - - out, err := runBarStatus(t, server.URL, true, "--notify") - if err != nil || !strings.Contains(out, "active") { - t.Fatalf("bar JSON unchanged by pagination, got %q, %v", out, err) - } - state, existed := loadOmarchyPollState() - if !existed || state.Seen["1"] != 1 || state.Seen["2"] != 1 || state.Seen["3"] != 4 { - t.Errorf("the first seed must fingerprint every unseen thread across pages, got %+v", state) - } - if _, fingerprinted := state.Seen["4"]; fingerprinted { - t.Errorf("seen threads are not fingerprinted: %+v", state) - } -} - -// unseenPagesServer serves `pages` all-unseen Imbox pages (one posting each, -// ids 1..pages) followed by a page whose first posting is seen, and counts -// the page fetches. -func unseenPagesServer(t *testing.T, pages int) (*httptest.Server, *int) { - t.Helper() - fetched := 0 - var server *httptest.Server - server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - if r.URL.Path == "/identity.json" { - _, _ = w.Write([]byte(`{"id": 7, "name": "Maria Delgado"}`)) - return - } - if r.URL.Path != "/imbox.json" { - w.WriteHeader(404) - return - } - fetched++ - page := 1 - if p := r.URL.Query().Get("page"); p != "" { - fmt.Sscanf(p, "%d", &page) - } - if page > pages { - _, _ = w.Write([]byte(`{"id": 1, "name": "Imbox", "kind": "inbox", "postings": [{"id": 999, "name": "Already read", "seen": true}]}`)) - return - } - fmt.Fprintf(w, `{"id": 1, "name": "Imbox", "kind": "inbox", "next_history_url": %q, - "postings": [{"id": %d, "name": "Thread %d", "seen": false, "visible_entry_count": 1}]}`, - fmt.Sprintf("%s/imbox.json?page=%d", server.URL, page+1), page, page) - })) - t.Cleanup(server.Close) - return server, &fetched -} - -func TestBarStatusWithoutNotifyReadsOnlyOnePage(t *testing.T) { - server, fetched := unseenPagesServer(t, 3) - - out, err := runBarStatus(t, server.URL, true) - if err != nil || !strings.Contains(out, "active") { - t.Fatalf("bar should light, got %q, %v", out, err) - } - if *fetched != 1 { - t.Errorf("the indicator needs one page, fetched %d", *fetched) - } -} - -func TestBarStatusNotifySeedsExhaustivelyThenCapsSteadyTicks(t *testing.T) { - server, fetched := unseenPagesServer(t, 3) - old := unseenPageCap - unseenPageCap = 2 - t.Cleanup(func() { unseenPageCap = old }) - - // First seed: every unseen page is read, cap or no cap. - stateHome := t.TempDir() - if _, err := runBarStatusWithState(t, stateHome, server.URL, true, "--notify"); err != nil { - t.Fatal(err) - } - state, _ := loadOmarchyPollState() - if len(state.Seen) != 3 { - t.Errorf("the seed must fingerprint every unseen thread, got %+v", state) - } - if *fetched != 4 { - t.Errorf("the seed should read all three unseen pages and the closing one, fetched %d", *fetched) - } - - // Steady tick: the cap applies and the snapshot is incomplete, so the - // thread beyond the cap keeps its fingerprint rather than being pruned. - *fetched = 0 - if _, err := runBarStatusWithState(t, stateHome, server.URL, true, "--notify"); err != nil { - t.Fatal(err) - } - if *fetched != 2 { - t.Errorf("a steady tick stops at the cap, fetched %d", *fetched) - } - if state, _ = loadOmarchyPollState(); len(state.Seen) != 3 { - t.Errorf("an incomplete snapshot must keep the fingerprints it could not see, got %+v", state) - } -} - -func TestSeedPageCapCountsTheInitialPage(t *testing.T) { - // Two all-unseen pages plus the closing seen page: a seed cap of three - // total pages must reach the closing page and complete. - server, fetched := unseenPagesServer(t, 2) - old := unseenSeedPageCap - unseenSeedPageCap = 3 - t.Cleanup(func() { unseenSeedPageCap = old }) - - if _, err := runBarStatus(t, server.URL, true, "--notify"); err != nil { - t.Fatal(err) - } - if state, existed := loadOmarchyPollState(); !existed || len(state.Seen) != 2 { - t.Errorf("a seed whose unseen set fits the cap must complete, got existed=%v %+v", existed, state) - } - if *fetched != 3 { - t.Errorf("the closing page is within the cap, fetched %d", *fetched) - } - - // One page tighter and the seed is incomplete: nothing may be persisted. - unseenSeedPageCap = 2 - if _, err := runBarStatus(t, server.URL, true, "--notify"); err != nil { - t.Fatal(err) - } - if _, existed := loadOmarchyPollState(); existed { - t.Error("an over-cap seed is incomplete and must not be persisted") - } -} - -func TestOmarchySetupNotifyFailsWhenStaleStateCannotBeDropped(t *testing.T) { - if os.Geteuid() == 0 { - t.Skip("root can remove anything") - } - env, _ := testOmarchyEnv(t) - on := true - omarchySetup{env: env}.apply() - if err := saveOmarchyPollState(omarchyPollState{Identity: "test", Seen: map[string]int32{"1": 1}}); err != nil { - t.Fatal(err) - } - stateDir := filepath.Dir(omarchyPollStatePath()) - if err := os.Chmod(stateDir, 0o500); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Chmod(stateDir, 0o700) }) - - steps := statuses(omarchySetup{env: env, notify: &on}.apply()) - if steps["bar indicator"] != "failed" { - t.Errorf("enabling toasts over undroppable stale fingerprints must fail, got %q", steps["bar indicator"]) - } - if module := readText(t, env.shellPath()); strings.Contains(module, "--notify") { - t.Error("the module must not be switched to --notify when the reseed could not be prepared") - } -} - -func TestBarStatusNotifySeedsStateAndPrintsBarJSON(t *testing.T) { - server := imboxServer(t, `[{"id": 5, "name": "Invoice #4021", "seen": false, "visible_entry_count": 2}]`) - defer server.Close() - - out, err := runBarStatus(t, server.URL, true, "--notify") - if err != nil || !strings.Contains(out, "active") { - t.Errorf("bar JSON must be unchanged by --notify, got %q, %v", out, err) - } - state, existed := loadOmarchyPollState() - if !existed || state.Seen["5"] != 2 { - t.Errorf("--notify should seed state on first run, got %+v (existed=%v)", state, existed) - } -} - -func TestBarStatusNotifySilentWhenUnauthenticatedOrOffline(t *testing.T) { - server := imboxServer(t, `[]`) - - out, err := runBarStatus(t, server.URL, false, "--notify") - if err != nil || out != "" { - t.Errorf("logged out should stay silent, got %q, %v", out, err) - } - if _, existed := loadOmarchyPollState(); existed { - t.Error("logged out must not touch state") - } - - server.Close() - out, err = runBarStatus(t, server.URL, true, "--notify") - if err != nil || out != "" { - t.Errorf("offline should stay silent, got %q, %v", out, err) - } - if _, existed := loadOmarchyPollState(); existed { - t.Error("offline must not touch state") - } -} - -func TestOmarchySetupNotifyTogglesBarExec(t *testing.T) { - env, _ := testOmarchyEnv(t) - on, off := true, false - - barExec := func() string { - var shell map[string]any - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { - t.Fatal(err) - } - layout := shell["bar"].(map[string]any)["layout"].(map[string]any) - return barLayoutModule(layout, omarchyBarModuleID)["exec"].(string) - } - - omarchySetup{env: env}.apply() - if barExec() != "hey omarchy bar-status" { - t.Fatalf("default install must not notify, exec = %q", barExec()) - } - - steps := statuses(omarchySetup{env: env, notify: &on}.apply()) - if steps["bar indicator"] != "installed" || barExec() != "hey omarchy bar-status --notify" { - t.Errorf("--notify should rewrite the exec, got %q / %q", steps["bar indicator"], barExec()) - } - - steps = statuses(omarchySetup{env: env, notify: &on}.apply()) - if steps["bar indicator"] != "unchanged" { - t.Errorf("--notify twice should be idempotent, got %q", steps["bar indicator"]) - } - - steps = statuses(omarchySetup{env: env}.apply()) - if steps["bar indicator"] != "unchanged" || barExec() != "hey omarchy bar-status --notify" { - t.Errorf("a plain re-run must leave notifications as they are, got %q / %q", steps["bar indicator"], barExec()) - } - - steps = statuses(omarchySetup{env: env, notify: &off}.apply()) - if steps["bar indicator"] != "installed" || barExec() != "hey omarchy bar-status" { - t.Errorf("--no-notify should revert the exec, got %q / %q", steps["bar indicator"], barExec()) - } -} - -func TestNotifyNewMailSkipsTheToastWhenStateCannotBeSaved(t *testing.T) { - if os.Geteuid() == 0 { - t.Skip("root can write anywhere") - } - stateHome := t.TempDir() - t.Setenv("XDG_STATE_HOME", stateHome) - env, calls := testNotifyEnv("7\n") - - notifyAll(env, nil) - stateDir := filepath.Dir(omarchyPollStatePath()) - if err := os.Chmod(stateDir, 0o500); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { _ = os.Chmod(stateDir, 0o700) }) - - notifyAll(env, []generated.Posting{unseenPosting(101, "Maria Delgado", "Lunch on Thursday?", 1)}) - if len(*calls) != 0 { - t.Errorf("a toast whose fingerprints cannot be saved would repeat every tick; it must be skipped, ran %v", *calls) - } -} - -func TestOmarchySetupNotifyReenableReseedsState(t *testing.T) { - env, _ := testOmarchyEnv(t) - on, off := true, false - - omarchySetup{env: env, notify: &on}.apply() - if err := saveOmarchyPollState(omarchyPollState{Identity: "test", Seen: map[string]int32{"101": 1}}); err != nil { - t.Fatal(err) - } - - omarchySetup{env: env, notify: &off}.apply() - if _, existed := loadOmarchyPollState(); !existed { - t.Fatal("turning toasts off should not touch the fingerprints") - } - - omarchySetup{env: env, notify: &on}.apply() - if _, existed := loadOmarchyPollState(); existed { - t.Error("turning toasts back on must drop stale fingerprints so the first tick reseeds") - } -} - -func TestOmarchySetupNotifyOnFreshInstall(t *testing.T) { - env, _ := testOmarchyEnv(t) - on := true - - omarchySetup{env: env, notify: &on}.apply() - - shell := readText(t, env.shellPath()) - if !strings.Contains(shell, "hey omarchy bar-status --notify") { - t.Errorf("fresh install with --notify should enable toasts:\n%s", shell) - } -} - -func TestOmarchySetupReconcileKeepsNotifyChoice(t *testing.T) { - env, _ := testOmarchyEnv(t) - if err := os.MkdirAll(env.configDir(), 0o755); err != nil { - t.Fatal(err) - } - stale := `{"version":1,"bar":{"layout":{"left":[],"center":[],"right":[ - {"id":"hey-unread","type":"command","exec":"hey omarchy bar-status --notify","interval":60, - "tooltip":"HEY","onClick":"omarchy-launch-or-focus-tui --app-id=org.omarchy.hey hey"}, - {"id":"omarchy.tray"}]}}}` - if err := os.WriteFile(env.shellPath(), []byte(stale), 0o644); err != nil { - t.Fatal(err) - } - - steps := statuses(omarchySetup{env: env}.apply()) - if steps["bar indicator"] != "installed" { - t.Errorf("a stale module should be rewritten on a plain re-run, got %q", steps["bar indicator"]) - } - var shell map[string]any - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { - t.Fatal(err) - } - layout := shell["bar"].(map[string]any)["layout"].(map[string]any) - module := barLayoutModule(layout, omarchyBarModuleID) - if module["onClick"] != omarchyFocusCommand || module["interval"] != float64(180) { - t.Errorf("click command and interval should be reconciled: %v", module) - } - if module["exec"] != "hey omarchy bar-status --notify" { - t.Errorf("the notify choice must survive a plain re-run: %v", module) - } - if right := layout["right"].([]any); len(right) != 2 || barEntryID(right[1]) != "omarchy.tray" { - t.Errorf("module position and neighbours must be kept: %v", right) - } - - if again := statuses(omarchySetup{env: env}.apply()); again["bar indicator"] != "unchanged" { - t.Errorf("reconciled module must be stable, got %q", again["bar indicator"]) - } -} diff --git a/internal/cmd/omarchy_test.go b/internal/cmd/omarchy_test.go index c9f3422f..feeda3f2 100644 --- a/internal/cmd/omarchy_test.go +++ b/internal/cmd/omarchy_test.go @@ -4,15 +4,12 @@ import ( "bytes" "encoding/json" "errors" - "net/http" - "net/http/httptest" "os" "path/filepath" "strings" "testing" "github.com/basecamp/hey-cli/internal/apierr" - "github.com/basecamp/hey-cli/internal/config" ) const defaultShellJSON = `{ @@ -29,11 +26,9 @@ const defaultShellJSON = `{ ` // testOmarchyEnv fakes an Omarchy install: a home dir, an OMARCHY_PATH with the -// default shell.json, a sandboxed state dir, and a recorder for the commands -// setup would run. +// default shell.json, and a recorder for the commands setup would run. func testOmarchyEnv(t *testing.T) (omarchyEnv, *[]string) { t.Helper() - t.Setenv("XDG_STATE_HOME", t.TempDir()) home := t.TempDir() omarchyPath := t.TempDir() if err := os.MkdirAll(filepath.Join(omarchyPath, "config", "omarchy"), 0o755); err != nil { @@ -73,14 +68,23 @@ func readText(t *testing.T, path string) string { func TestOmarchySetupInstallsEverythingOnce(t *testing.T) { env, ran := testOmarchyEnv(t) + writeShell(t, env, pluginShellJSON) setup := omarchySetup{env: env} - first := statuses(setup.apply()) - for _, name := range []string{"desktop entry", "menu", "bar indicator", "theme template"} { - if first[name] != "installed" { - t.Errorf("%s: first run = %q, want installed", name, first[name]) + first := setup.apply() + for _, name := range []string{"desktop entry", "menu", "theme template"} { + if status := statuses(first)[name]; status != "installed" { + t.Errorf("%s: first run = %q, want installed", name, status) } } + // The plugin entry is already there and nothing asked for a change: the + // step reports the current notify setting without rewriting shell.json. + if bar := stepNamed(first, "bar plugin"); bar.Status != "unchanged" || bar.Detail != "notifications off" { + t.Errorf("bar plugin: first run = %q %q, want unchanged / notifications off", bar.Status, bar.Detail) + } + if readText(t, env.shellPath()) != pluginShellJSON { + t.Error("a plain run must not rewrite shell.json") + } desktop := readText(t, env.desktopPath()) if !strings.Contains(desktop, "Exec=xdg-terminal-exec --app-id=org.omarchy.hey -e hey tui") { @@ -95,18 +99,6 @@ func TestOmarchySetupInstallsEverythingOnce(t *testing.T) { t.Errorf("menu block not written:\n%s", menu) } - var shell map[string]any - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { - t.Fatal(err) - } - right := shell["bar"].(map[string]any)["layout"].(map[string]any)["right"].([]any) - if barEntryID(right[0]) != "hey-unread" || barEntryID(right[1]) != "omarchy.tray" { - t.Errorf("bar module should lead the right section seeded from defaults: %v", right) - } - if module := right[0].(map[string]any); module["exec"] != "hey omarchy bar-status" || module["type"] != "command" { - t.Errorf("bar module malformed: %v", module) - } - if readText(t, env.templatePath()) != omarchyThemeTemplate { t.Error("theme template not written") } @@ -127,7 +119,9 @@ func TestOmarchySetupInstallsEverythingOnce(t *testing.T) { func TestOmarchySetupRemoveReversesEveryPiece(t *testing.T) { env, _ := testOmarchyEnv(t) - setup := omarchySetup{env: env} + writeShell(t, env, pluginShellJSON) + on := true + setup := omarchySetup{env: env, notify: &on} menuBefore := "{\n // my rows\n \"notes\": {\"icon\":\"\",\"label\":\"Notes\",\"action\":\"omarchy-launch-editor ~/notes\"},\n}\n" if err := os.MkdirAll(filepath.Dir(env.menuPath()), 0o755); err != nil { @@ -137,23 +131,31 @@ func TestOmarchySetupRemoveReversesEveryPiece(t *testing.T) { t.Fatal(err) } setup.apply() - if err := saveOmarchyPollState(omarchyPollState{Seen: map[string]int32{"1": 1}}); err != nil { - t.Fatal(err) - } if menu := readText(t, env.menuPath()); !strings.Contains(menu, `"notes"`) || !strings.Contains(menu, `"hey-tui"`) { t.Errorf("install should keep the user's rows alongside ours:\n%s", menu) } + if entry := pluginEntry(t, env); entry["notify"] != true { + t.Fatalf("--notify should set notify on the plugin entry, got %v", entry) + } - removed := statuses(setup.remove()) + removed := statuses(omarchySetup{env: env}.remove()) for name, status := range removed { - if status != "removed" { - t.Errorf("%s: remove = %q, want removed", name, status) + switch name { + case "bar indicator": + if status != "absent" { + t.Errorf("no legacy module was installed, got %q", status) + } + case "bar plugin": + if status != "kept" { + t.Errorf("the plugin's notify setting is the plugin's own, got %q", status) + } + default: + if status != "removed" { + t.Errorf("%s: remove = %q, want removed", name, status) + } } } - if _, err := os.Stat(omarchyPollStatePath()); !os.IsNotExist(err) { - t.Error("poll state still present") - } if _, err := os.Stat(env.desktopPath()); !os.IsNotExist(err) { t.Error("desktop entry still present") } @@ -163,43 +165,31 @@ func TestOmarchySetupRemoveReversesEveryPiece(t *testing.T) { if menu := readText(t, env.menuPath()); menu != menuBefore { t.Errorf("menu should be restored byte for byte:\n%s", menu) } - if shell := readText(t, env.shellPath()); strings.Contains(shell, "hey-unread") { - t.Errorf("bar module still present:\n%s", shell) + // The plugin entry is the user's (omarchy plugin add wrote it), and its + // notify setting is set as readily from the panel as from here: removal + // cannot tell a preference it wrote from one it didn't, so it leaves it. + entry := pluginEntry(t, env) + if entry == nil { + t.Fatal("remove must not delete the plugin's layout entry") + } + if entry["notify"] != true { + t.Errorf("remove must leave the plugin's notify setting alone, got %v", entry) } - again := statuses(setup.remove()) + again := statuses(omarchySetup{env: env}.remove()) for name, status := range again { + if name == "bar plugin" { + if status != "kept" { + t.Errorf("second remove: bar plugin = %q, want kept", status) + } + continue + } if status != "absent" { t.Errorf("%s: second remove = %q, want absent", name, status) } } } -func TestOmarchySetupKeepsExistingBarLayout(t *testing.T) { - env, _ := testOmarchyEnv(t) - if err := os.MkdirAll(env.configDir(), 0o755); err != nil { - t.Fatal(err) - } - custom := `{"version":1,"bar":{"layout":{"left":[{"id":"omarchy.menu"}],"center":[],"right":["omarchy.audio"]}},"idle":{"lock":600}}` - if err := os.WriteFile(env.shellPath(), []byte(custom), 0o644); err != nil { - t.Fatal(err) - } - - omarchySetup{env: env}.apply() - - var shell map[string]any - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { - t.Fatal(err) - } - if shell["idle"].(map[string]any)["lock"] != float64(600) { - t.Error("unrelated settings must survive") - } - right := shell["bar"].(map[string]any)["layout"].(map[string]any)["right"].([]any) - if len(right) != 2 || barEntryID(right[1]) != "omarchy.audio" { - t.Errorf("string-form entries must be kept: %v", right) - } -} - func TestOmarchySetupRejectsNonJSONShellConfig(t *testing.T) { env, _ := testOmarchyEnv(t) if err := os.MkdirAll(env.configDir(), 0o755); err != nil { @@ -209,8 +199,8 @@ func TestOmarchySetupRejectsNonJSONShellConfig(t *testing.T) { t.Fatal(err) } steps := statuses(omarchySetup{env: env}.apply()) - if steps["bar indicator"] != "failed" { - t.Errorf("a shell.json we cannot round-trip must fail rather than be rewritten, got %q", steps["bar indicator"]) + if steps["bar plugin"] != "failed" { + t.Errorf("a shell.json we cannot round-trip must fail rather than be rewritten, got %q", steps["bar plugin"]) } if steps["menu"] != "installed" { t.Error("one failing step must not stop the others") @@ -227,8 +217,8 @@ func TestOmarchySetupRejectsMalformedShellConfig(t *testing.T) { if err := os.WriteFile(env.shellPath(), []byte("null"), 0o644); err != nil { t.Fatal(err) } - if steps := statuses(omarchySetup{env: env}.apply()); steps["bar indicator"] != "failed" { - t.Errorf("a null shell.json must fail the bar step, got %q", steps["bar indicator"]) + if steps := statuses(omarchySetup{env: env}.apply()); steps["bar plugin"] != "failed" { + t.Errorf("a null shell.json must fail the bar step, got %q", steps["bar plugin"]) } // A version-less object is ignored by the shell; refuse to edit it. @@ -237,7 +227,7 @@ func TestOmarchySetupRejectsMalformedShellConfig(t *testing.T) { } steps := omarchySetup{env: env}.apply() for _, step := range steps { - if step.Name == "bar indicator" { + if step.Name == "bar plugin" { if step.Status != "failed" || !strings.Contains(step.Detail, "version") { t.Errorf("a version-less shell.json must fail with a version hint, got %q %q", step.Status, step.Detail) } @@ -259,8 +249,8 @@ func TestOmarchySetupRefusesWrongTypedBarFields(t *testing.T) { if err := os.WriteFile(env.shellPath(), []byte(shell), 0o644); err != nil { t.Fatal(err) } - if steps := statuses(omarchySetup{env: env}.apply()); steps["bar indicator"] != "failed" { - t.Errorf("%s: a wrong-typed value must fail, not be replaced, got %q", shell, steps["bar indicator"]) + if steps := statuses(omarchySetup{env: env}.apply()); steps["bar plugin"] != "failed" { + t.Errorf("%s: a wrong-typed value must fail, not be replaced, got %q", shell, steps["bar plugin"]) } if readText(t, env.shellPath()) != shell { t.Errorf("%s: the file must be left untouched", shell) @@ -268,38 +258,6 @@ func TestOmarchySetupRefusesWrongTypedBarFields(t *testing.T) { } } -func TestBarStatusIgnoresRepositoryLocalConfig(t *testing.T) { - repo := t.TempDir() - if err := os.MkdirAll(filepath.Join(repo, ".hey"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(repo, ".hey", "config.json"), []byte(`{"base_url":"https://untrusted.example.com"}`), 0o644); err != nil { - t.Fatal(err) - } - t.Chdir(repo) - server := imboxServer(t, `[]`) - defer server.Close() - - // An untrusted checkout must neither fail the poller at the trust gate - // nor point it at the checkout's server. - out, err := runBarStatus(t, server.URL, true) - if err != nil || out != "" { - t.Errorf("the poller must stay dark and silent from an untrusted checkout, got %q, %v", out, err) - } - if cfg.UntrustedLocalConfig() != nil || cfg.SourceOf("base_url") != config.SourceFlag { - t.Errorf("the poller must load global configuration only, got base_url from %v", cfg.SourceOf("base_url")) - } - - root := newRootCmd() - command, _, _ := root.Find([]string{"setup", "omarchy"}) - if !commandIgnoresLocalConfig(command) { - t.Error("setup omarchy edits fixed desktop paths only and must ignore a checkout's config too") - } - if boxes, _, _ := root.Find([]string{"boxes"}); commandIgnoresLocalConfig(boxes) { - t.Error("ordinary commands keep honouring repository-local config") - } -} - func TestSetupOmarchyRemoveIgnoresMalformedLocalConfig(t *testing.T) { repo := t.TempDir() if err := os.MkdirAll(filepath.Join(repo, ".hey"), 0o755); err != nil { @@ -347,111 +305,6 @@ func TestSetupOmarchyFailsWithoutAHomeDirectory(t *testing.T) { } } -func TestOmarchySetupSeedsVersionAndRestoresDefaultsOnRemove(t *testing.T) { - env, _ := testOmarchyEnv(t) - setup := omarchySetup{env: env} - - setup.apply() - var shell map[string]any - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { - t.Fatal(err) - } - if shell["version"] != float64(1) { - t.Errorf(`a freshly created shell.json needs "version": 1 or the shell ignores it: %v`, shell) - } - - removed := setup.remove() - if steps := statuses(removed); steps["bar indicator"] != "removed" { - for _, step := range removed { - t.Logf("%s: %s %s", step.Name, step.Status, step.Detail) - } - t.Fatalf("bar step = %q, want removed", steps["bar indicator"]) - } - var after map[string]any // a fresh map: Unmarshal into a non-nil map merges keys - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &after); err != nil { - t.Fatal(err) - } - if _, has := after["bar"]; has { - t.Errorf("removing from a defaults-seeded layout should restore inheriting the defaults: %v", after) - } -} - -func TestOmarchySetupReconcilesStaleBarModule(t *testing.T) { - env, _ := testOmarchyEnv(t) - if err := os.MkdirAll(env.configDir(), 0o755); err != nil { - t.Fatal(err) - } - stale := `{"version":1,"bar":{"layout":{"left":[],"center":[],"right":[ - {"id":"hey-unread","type":"command","exec":"hey omarchy bar-status --old-flag","interval":60, - "tooltip":"HEY","onClick":"omarchy-launch-or-focus-tui --app-id=org.omarchy.hey hey"}, - {"id":"omarchy.tray"}]}}}` - if err := os.WriteFile(env.shellPath(), []byte(stale), 0o644); err != nil { - t.Fatal(err) - } - - if steps := statuses(omarchySetup{env: env}.apply()); steps["bar indicator"] != "installed" { - t.Errorf("a stale module should be rewritten, got %q", steps["bar indicator"]) - } - var shell map[string]any - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { - t.Fatal(err) - } - layout := shell["bar"].(map[string]any)["layout"].(map[string]any) - module := barLayoutModule(layout, omarchyBarModuleID) - if module["exec"] != "hey omarchy bar-status" || module["onClick"] != omarchyFocusCommand || module["interval"] != float64(180) { - t.Errorf("stale fields should be reconciled: %v", module) - } - if right := layout["right"].([]any); len(right) != 2 || barEntryID(right[1]) != "omarchy.tray" { - t.Errorf("module position and neighbours must be kept: %v", right) - } - if neighbour := barLayoutModule(layout, "omarchy.tray"); neighbour == nil || neighbour["id"] != "omarchy.tray" { - t.Errorf("other inline modules must be findable too: %v", neighbour) - } - if again := statuses(omarchySetup{env: env}.apply()); again["bar indicator"] != "unchanged" { - t.Errorf("reconciled module must be stable, got %q", again["bar indicator"]) - } - // A string-form entry that happens to share our id is not ours: neither - // found by the module lookup nor deleted on removal. - layout["left"] = append(layout["left"].([]any), "hey-unread") - if barLayoutModule(layout, omarchyBarModuleID)["type"] != "command" { - t.Error("the string-form entry must not shadow the managed map") - } - withString, err := json.Marshal(shell) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(env.shellPath(), withString, 0o644); err != nil { - t.Fatal(err) - } - setup := omarchySetup{env: env} - if steps := statuses(setup.remove()); steps["bar indicator"] != "removed" { - t.Errorf("remove should still find the managed map, got %q", steps["bar indicator"]) - } - if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { - t.Fatal(err) - } - left := shell["bar"].(map[string]any)["layout"].(map[string]any)["left"].([]any) - if len(left) != 1 || left[0] != "hey-unread" { - t.Errorf("a string-form entry sharing our id is unowned and must survive removal: %v", left) - } -} - -func TestOmarchyDefaultShellPathFallsBackToUserTree(t *testing.T) { - env, _ := testOmarchyEnv(t) - env.omarchyPath = "" // no OMARCHY_PATH, as in a non-login or agent shell - userTree := filepath.Join(env.home, ".local", "share", "omarchy", "config", "omarchy") - if err := os.MkdirAll(userTree, 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(userTree, "shell.json"), []byte(defaultShellJSON), 0o644); err != nil { - t.Fatal(err) - } - - if steps := statuses(omarchySetup{env: env}.apply()); steps["bar indicator"] != "installed" { - t.Errorf("the per-user omarchy tree should seed the layout, got %q", steps["bar indicator"]) - } -} - func TestOmarchyRemoveTemplateReportsDeferredRender(t *testing.T) { env, _ := testOmarchyEnv(t) env.run = func(name string, args ...string) error { return errors.New("not on PATH") } @@ -665,17 +518,12 @@ func TestSetupOmarchyIsNotBlockedByAnUntrustedLocalConfig(t *testing.T) { func TestOmarchySetupPreservesLargeIntegersInShellConfig(t *testing.T) { env, _ := testOmarchyEnv(t) - if err := os.MkdirAll(env.configDir(), 0o755); err != nil { - t.Fatal(err) - } // 2^53+1 is not representable as a float64; a naive round trip would round it. - shell := `{"version":1,"bar":{"layout":{"left":[],"center":[],"right":[]}},"custom":{"token":9007199254740993}}` - if err := os.WriteFile(env.shellPath(), []byte(shell), 0o644); err != nil { - t.Fatal(err) - } + writeShell(t, env, `{"version":1,"bar":{"layout":{"left":[],"center":[],"right":[{"id":"37signals.hey"}]}},"custom":{"token":9007199254740993}}`) + on := true - if steps := statuses(omarchySetup{env: env}.apply()); steps["bar indicator"] != "installed" { - t.Fatalf("bar step = %q", steps["bar indicator"]) + if steps := statuses(omarchySetup{env: env, notify: &on}.apply()); steps["bar plugin"] != "installed" { + t.Fatalf("bar step = %q", steps["bar plugin"]) } if out := readText(t, env.shellPath()); !strings.Contains(out, "9007199254740993") { t.Errorf("an unrelated large integer must survive the rewrite exactly:\n%s", out) @@ -725,43 +573,6 @@ func TestOmarchySetupKeepsDanglingTemplateSymlink(t *testing.T) { } } -func TestBarStatusStaysDarkOnAMalformedGlobalConfig(t *testing.T) { - server := imboxServer(t, `[{"id": 1, "name": "Invoice #4021", "seen": false}]`) - defer server.Close() - t.Setenv("HEY_TOKEN", "test-token") - t.Setenv("HEY_NO_KEYRING", "1") - t.Setenv("HEY_BASE_URL", "") - configHome := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", configHome) - t.Setenv("XDG_STATE_HOME", t.TempDir()) - if err := os.MkdirAll(filepath.Join(configHome, "hey-cli"), 0o755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(configHome, "hey-cli", "config.json"), []byte("{broken"), 0o644); err != nil { - t.Fatal(err) - } - - root := newRootCmd() - var buf bytes.Buffer - root.SetOut(&buf) - root.SetErr(&buf) - root.SetArgs([]string{"omarchy", "bar-status", "--base-url", server.URL}) - if err := root.Execute(); err != nil { - t.Fatalf("a broken global config must not make the poller exit nonzero, got %v", err) - } - if buf.String() != "" { - t.Errorf("with no trustworthy configuration the poller must stay dark, not light against a guessed server: %q", buf.String()) - } - - root = newRootCmd() - root.SetOut(&buf) - root.SetErr(&buf) - root.SetArgs([]string{"boxes"}) - if err := root.Execute(); err == nil { - t.Error("ordinary commands still report a broken global config") - } -} - func TestOmarchySetupKeepsForeignTemplate(t *testing.T) { env, ran := testOmarchyEnv(t) foreign := "# my own hey theme template\naccent = \"#ff00ff\"\n" @@ -847,91 +658,210 @@ func TestSetupOmarchyRequiresOmarchy(t *testing.T) { } } -func imboxServer(t *testing.T, postings string) *httptest.Server { +// pluginShellJSON is a shell.json whose bar layout carries the 37signals.hey +// plugin entry, as `omarchy plugin add --enable` writes it. +const pluginShellJSON = `{"version":1,"bar":{"layout":{"left":[{"id":"omarchy.menu"}],"center":[{"id":"omarchy.clock"}],"right":[{"id":"37signals.hey"},{"id":"omarchy.tray"}]}}}` + +// legacyShellJSON is what an earlier `hey setup omarchy` wrote: the layout +// seeded from Omarchy's defaults with the inline hey-unread module in front. +const legacyShellJSON = `{"version":1,"bar":{"layout":{"left":[{"id":"omarchy.menu"}],"center":[{"id":"omarchy.clock"}],"right":[ + {"id":"hey-unread","type":"command","exec":"hey omarchy bar-status --notify","interval":180,"tooltip":"HEY","onClick":"omarchy-launch-or-focus-tui --app-id=org.omarchy.hey hey tui"}, + {"id":"omarchy.tray"},{"id":"omarchy.power"}]}}}` + +func writeShell(t *testing.T, env omarchyEnv, content string) { t.Helper() - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case r.Method == "GET" && r.URL.Path == "/identity.json": - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"id": 7, "name": "Maria Delgado"}`)) - case r.Method == "GET" && r.URL.Path == "/imbox.json": - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"id": 1, "name": "Imbox", "kind": "inbox", "postings": ` + postings + `}`)) - default: - w.WriteHeader(404) - } - })) + if err := os.MkdirAll(env.configDir(), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(env.shellPath(), []byte(content), 0o644); err != nil { + t.Fatal(err) + } } -func runBarStatus(t *testing.T, serverURL string, authenticated bool, extraArgs ...string) (string, error) { +func readShell(t *testing.T, env omarchyEnv) map[string]any { t.Helper() - return runBarStatusWithState(t, t.TempDir(), serverURL, authenticated, extraArgs...) + var shell map[string]any + if err := json.Unmarshal([]byte(readText(t, env.shellPath())), &shell); err != nil { + t.Fatal(err) + } + return shell } -// runBarStatusWithState runs bar-status against a given state directory, so a -// test can tick more than once over the same fingerprints. -func runBarStatusWithState(t *testing.T, stateHome, serverURL string, authenticated bool, extraArgs ...string) (string, error) { +// pluginEntry returns the 37signals.hey layout entry, or nil. +func pluginEntry(t *testing.T, env omarchyEnv) map[string]any { t.Helper() - if authenticated { - t.Setenv("HEY_TOKEN", "test-token") - } else { - t.Setenv("HEY_TOKEN", "") + bar, _ := readShell(t, env)["bar"].(map[string]any) + layout, _ := bar["layout"].(map[string]any) + return barLayoutModule(layout, omarchyBarPluginID) +} + +func stepNamed(steps []omarchyStep, name string) omarchyStep { + for _, step := range steps { + if step.Name == name { + return step + } } - t.Setenv("HEY_NO_KEYRING", "1") - t.Setenv("HEY_BASE_URL", "") - tmpDir := t.TempDir() - t.Setenv("XDG_CONFIG_HOME", tmpDir) - t.Setenv("XDG_STATE_HOME", stateHome) - t.Setenv("XDG_CACHE_HOME", tmpDir) + return omarchyStep{} +} - root := newRootCmd() - var buf bytes.Buffer - root.SetOut(&buf) - root.SetErr(&buf) - root.SetArgs(append([]string{"omarchy", "bar-status", "--base-url", serverURL}, extraArgs...)) - err := root.Execute() - return buf.String(), err +func TestOmarchySetupSkipsTheBarWithoutThePlugin(t *testing.T) { + env, _ := testOmarchyEnv(t) + on := true + + // No shell.json at all: nothing to configure, and nothing is created — + // setup never adds a layout entry, the plugin install does. + bar := stepNamed(omarchySetup{env: env, notify: &on}.apply(), "bar plugin") + if bar.Status != "skipped" || !strings.Contains(bar.Detail, omarchyBarPluginInstall) { + t.Errorf("without the plugin the bar step should say how to install it, got %q %q", bar.Status, bar.Detail) + } + if _, err := os.Stat(env.shellPath()); !os.IsNotExist(err) { + t.Error("setup must not create a shell.json just to skip") + } + + // A layout without the plugin entry is the same thing. + writeShell(t, env, defaultShellJSON) + if bar := stepNamed(omarchySetup{env: env}.apply(), "bar plugin"); bar.Status != "skipped" { + t.Errorf("a layout without the plugin should be skipped, got %q", bar.Status) + } + if readText(t, env.shellPath()) != defaultShellJSON { + t.Error("skipping must leave shell.json untouched") + } } -func TestBarStatusLightsOnUnread(t *testing.T) { - server := imboxServer(t, `[{"id": 1, "name": "Lunch on Thursday?", "seen": true}, {"id": 2, "name": "Invoice #4021", "seen": false}]`) - defer server.Close() +func TestOmarchySetupNotifyTogglesThePluginSetting(t *testing.T) { + env, _ := testOmarchyEnv(t) + writeShell(t, env, pluginShellJSON) + on, off := true, false - out, err := runBarStatus(t, server.URL, true) - if err != nil { - t.Fatal(err) + bar := stepNamed(omarchySetup{env: env, notify: &on}.apply(), "bar plugin") + if bar.Status != "installed" || bar.Detail != "notifications on" || pluginEntry(t, env)["notify"] != true { + t.Errorf("--notify should set notify on the entry, got %q %q %v", bar.Status, bar.Detail, pluginEntry(t, env)) } - var module map[string]any - if err := json.Unmarshal([]byte(out), &module); err != nil { - t.Fatalf("output is not JSON: %q", out) + if bar := stepNamed(omarchySetup{env: env, notify: &on}.apply(), "bar plugin"); bar.Status != "unchanged" { + t.Errorf("--notify twice should be idempotent, got %q", bar.Status) } - if module["class"] != "active" || module["text"] != omarchyBarGlyph { - t.Errorf("unexpected module: %v", module) + bar = stepNamed(omarchySetup{env: env}.apply(), "bar plugin") + if bar.Status != "unchanged" || bar.Detail != "notifications on" || pluginEntry(t, env)["notify"] != true { + t.Errorf("a plain re-run must leave notifications as they are, got %q %q", bar.Status, bar.Detail) + } + bar = stepNamed(omarchySetup{env: env, notify: &off}.apply(), "bar plugin") + if _, has := pluginEntry(t, env)["notify"]; bar.Status != "installed" || bar.Detail != "notifications off" || has { + t.Errorf("--no-notify should delete the key, got %q %q %v", bar.Status, bar.Detail, pluginEntry(t, env)) + } + if bar := stepNamed(omarchySetup{env: env, notify: &off}.apply(), "bar plugin"); bar.Status != "unchanged" { + t.Errorf("--no-notify twice should be idempotent, got %q", bar.Status) + } + // Only a JSON true counts, as in the plugin's own setting() read. + writeShell(t, env, `{"version":1,"bar":{"layout":{"right":[{"id":"37signals.hey","notify":"yes"}]}}}`) + if bar := stepNamed(omarchySetup{env: env}.apply(), "bar plugin"); bar.Detail != "notifications off" { + t.Errorf("a non-boolean notify is off, got %q", bar.Detail) + } +} + +func TestOmarchySetupFindsThePluginInAnySection(t *testing.T) { + env, _ := testOmarchyEnv(t) + writeShell(t, env, `{"version":1,"bar":{"layout":{"left":[{"id":"omarchy.menu"}],"center":[{"id":"37signals.hey"},{"id":"omarchy.clock"}],"right":[]}}}`) + on := true + + if bar := stepNamed(omarchySetup{env: env, notify: &on}.apply(), "bar plugin"); bar.Status != "installed" { + t.Fatalf("bar step = %q %q", bar.Status, bar.Detail) + } + center := readShell(t, env)["bar"].(map[string]any)["layout"].(map[string]any)["center"].([]any) + if entry := center[0].(map[string]any); entry["id"] != "37signals.hey" || entry["notify"] != true { + t.Errorf("the entry should be updated in place: %v", center) } } -func TestBarStatusSilentWhenClear(t *testing.T) { - server := imboxServer(t, `[{"id": 1, "name": "Lunch on Thursday?", "seen": true}]`) - defer server.Close() +func TestOmarchySetupRemovesTheLegacyBarModule(t *testing.T) { + env, _ := testOmarchyEnv(t) + writeShell(t, env, legacyShellJSON) + + // No plugin yet: the legacy module goes, reported as its own step, and the + // plugin step says what to install — and how to keep the toasts the module + // had on. The layout equalled the defaults once the module was out, so it + // goes too and the user is back to inheriting Omarchy's defaults. + steps := omarchySetup{env: env}.apply() + if legacy := stepNamed(steps, "bar indicator"); legacy.Status != "removed" || !strings.Contains(legacy.Detail, "hey-unread") { + t.Errorf("legacy removal should be its own step, got %q %q", legacy.Status, legacy.Detail) + } + if bar := stepNamed(steps, "bar plugin"); bar.Status != "skipped" || !strings.Contains(bar.Detail, omarchyBarPluginInstall) || !strings.Contains(bar.Detail, "--notify") { + t.Errorf("the plugin step should say how to install and how to keep notifying, got %q %q", bar.Status, bar.Detail) + } + if shell := readShell(t, env); shell["bar"] != nil { + t.Errorf("a defaults-seeded layout should be dropped with the module: %v", shell) + } + steps = omarchySetup{env: env}.apply() + if stepNamed(steps, "bar indicator").Name != "" || stepNamed(steps, "bar plugin").Status != "skipped" { + t.Errorf("once migrated there is no legacy step and the plugin step skips, got %v", statuses(steps)) + } + + // With the plugin present and silent on notify, the legacy module's + // --notify carries over; a string-form entry sharing its id is unowned + // and survives. + writeShell(t, env, `{"version":1,"bar":{"layout":{"left":[],"center":[],"right":[ + {"id":"hey-unread","type":"command","exec":"hey omarchy bar-status --notify","interval":180}, + {"id":"37signals.hey"},"hey-unread",{"id":"omarchy.tray"}]}}}`) + steps = omarchySetup{env: env}.apply() + if bar := stepNamed(steps, "bar plugin"); bar.Status != "installed" || bar.Detail != "notifications on" { + t.Errorf("the notify choice should move to the plugin, got %q %q", bar.Status, bar.Detail) + } + right := readShell(t, env)["bar"].(map[string]any)["layout"].(map[string]any)["right"].([]any) + if len(right) != 3 || right[0].(map[string]any)["notify"] != true || right[1] != "hey-unread" || barEntryID(right[2]) != "omarchy.tray" { + t.Errorf("only the map-form legacy module goes and notify lands on the plugin: %v", right) + } + + // An explicit plugin setting, or an explicit flag, wins over the legacy + // module's choice. + writeShell(t, env, `{"version":1,"bar":{"layout":{"right":[{"id":"hey-unread","type":"command","exec":"hey omarchy bar-status --notify"},{"id":"37signals.hey","notify":false}]}}}`) + if bar := stepNamed(omarchySetup{env: env}.apply(), "bar plugin"); bar.Detail != "notifications off" || bar.Status != "unchanged" { + t.Errorf("an explicit plugin setting is kept, and the legacy removal is the other step's news, got %q %q", bar.Status, bar.Detail) + } + off := false + writeShell(t, env, `{"version":1,"bar":{"layout":{"right":[{"id":"hey-unread","type":"command","exec":"hey omarchy bar-status --notify"},{"id":"37signals.hey"}]}}}`) + if bar := stepNamed(omarchySetup{env: env, notify: &off}.apply(), "bar plugin"); bar.Detail != "notifications off" { + t.Errorf("--no-notify wins over the legacy choice, got %q", bar.Detail) + } + if _, has := pluginEntry(t, env)["notify"]; has { + t.Error("--no-notify must not leave a notify key behind") + } - out, err := runBarStatus(t, server.URL, true) - if err != nil || out != "" { - t.Errorf("clear imbox should print nothing and succeed, got %q, %v", out, err) + // --remove takes the legacy module out as well. + writeShell(t, env, legacyShellJSON) + if legacy := stepNamed(omarchySetup{env: env}.remove(), "bar indicator"); legacy.Status != "removed" { + t.Errorf("remove should take the legacy module out, got %q", legacy.Status) + } + if strings.Contains(readText(t, env.shellPath()), "hey-unread") { + t.Error("legacy module still present after remove") } } -func TestBarStatusSilentWhenUnauthenticatedOrOffline(t *testing.T) { - server := imboxServer(t, `[]`) - defer server.Close() +func TestOmarchyDefaultShellPathFallsBackToUserTree(t *testing.T) { + env, _ := testOmarchyEnv(t) + env.omarchyPath = "" // no OMARCHY_PATH, as in a non-login or agent shell + userTree := filepath.Join(env.home, ".local", "share", "omarchy", "config", "omarchy") + if err := os.MkdirAll(userTree, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(userTree, "shell.json"), []byte(defaultShellJSON), 0o644); err != nil { + t.Fatal(err) + } + writeShell(t, env, legacyShellJSON) - out, err := runBarStatus(t, server.URL, false) - if err != nil || out != "" { - t.Errorf("logged out should print nothing and succeed, got %q, %v", out, err) + if legacy := stepNamed(omarchySetup{env: env}.apply(), "bar indicator"); legacy.Status != "removed" { + t.Fatalf("legacy step = %q %q", legacy.Status, legacy.Detail) + } + if shell := readShell(t, env); shell["bar"] != nil { + t.Errorf("the per-user omarchy tree should supply the defaults the layout is compared to: %v", shell) } +} - server.Close() - out, err = runBarStatus(t, server.URL, true) - if err != nil || out != "" { - t.Errorf("offline should print nothing and succeed, got %q, %v", out, err) +func TestSetupOmarchyIgnoresRepositoryLocalConfig(t *testing.T) { + root := newRootCmd() + command, _, _ := root.Find([]string{"setup", "omarchy"}) + if !commandIgnoresLocalConfig(command) { + t.Error("setup omarchy edits fixed desktop paths only and must ignore a checkout's config") + } + if boxes, _, _ := root.Find([]string{"boxes"}); commandIgnoresLocalConfig(boxes) { + t.Error("ordinary commands keep honouring repository-local config") } } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index b38881aa..e3a38931 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -76,14 +76,12 @@ func newRootCmd() *cobra.Command { } var err error - configDegraded = false if commandIgnoresLocalConfig(cmd) { - // These commands never fail on configuration: a malformed global - // file leaves them on the baseline defaults, where the bar poller - // simply finds no credentials and stays dark. + // setup omarchy never fails on configuration: a malformed global + // file leaves it on the baseline defaults, which is all it needs + // to edit fixed desktop paths — so --remove still works. if cfg, err = config.LoadGlobal(); err != nil { cfg, err = config.Defaults(), nil - configDegraded = true } } else { cfg, err = config.Load() @@ -214,7 +212,6 @@ func newRootCmd() *cobra.Command { root.AddCommand(newIgnoreCommand().cmd) root.AddCommand(newStopIgnoringCommand().cmd) root.AddCommand(newSetupCommand().cmd) - root.AddCommand(newOmarchyCommand().cmd) root.AddCommand(newTuiCommand().cmd) root.AddCommand(newHeyCommand().cmd) root.AddCommand(newSkillCommand().cmd) @@ -252,9 +249,7 @@ func commandUsesAccountScope(cmd *cobra.Command) bool { return true } switch parts[1] { - // omarchy is exempt because its bar-status must never fail: it selects the - // configured account itself and treats a failed selection as a dark indicator. - case "accounts", "auth", "commands", "completion", "config", "doctor", "login", "logout", "omarchy", "setup", "skill", "upgrade", "version": + case "accounts", "auth", "commands", "completion", "config", "doctor", "login", "logout", "setup", "skill", "upgrade", "version": return false default: return true diff --git a/internal/cmd/watch.go b/internal/cmd/watch.go index 22c01636..79cb14f3 100644 --- a/internal/cmd/watch.go +++ b/internal/cmd/watch.go @@ -15,6 +15,7 @@ import ( "slices" "strconv" "strings" + "sync" "sync/atomic" "syscall" "time" @@ -42,7 +43,17 @@ const ( asyncScriptLimit = 16 ) -var watchableChanges = []string{"added", "updated", "deleted"} +// The three changes a posting goes through; the one that is a reading of the +// first two — new is the added and updated postings that are new mail, as decided +// here (watch_new.go) rather than by every reader; and resync, the watch's word +// that a box changed more than it could follow. All but new are reported by +// default; new is asked for, and asking for new alone leaves a resync out, so a +// script for new mail never runs on one. +var ( + postingChanges = []string{"added", "updated", "deleted"} + defaultChanges = []string{"added", "updated", "deleted", "resync"} + watchableChanges = []string{"added", "updated", "deleted", "new", "resync"} +) type watchCommand struct { cmd *cobra.Command @@ -65,14 +76,28 @@ func newWatchCommand() *watchCommand { Changes can drive a command instead of being printed, and that is a choice between two behaviours: --run-async spawns the command per change and moves on, so a slow one never holds up the watch and two can overlap; --run-sync waits for each and runs them in order. -Pass one or the other.`, +Pass one or the other. + +Every added and updated line says whether the thread is new mail: unseen, not muted, and +active since the watch last saw it — or since the watch began, for a thread it has not +seen, so the backlog a box's first read carries is not new. Reading a thread, muting or +moving it is not new activity; a reply on a known thread is. --events new selects the new +ones, alone or alongside added, updated and deleted, and a script sees HEY_NEW=1 for them. + +Besides the thread changes, three lines describe the watch itself: "ready" once every box +is caught up and the subscription is live (again after every reconnect's catch-up), +"disconnected" when the connection drops, and "resync" when a box changed more than the +feed can list one change at a time and the watch skipped ahead — re-read that box. A resync +is an event of its own: reported by default, scripts run for it and --exit-on-first counts +it, and --events can leave it out, as --events new does. Ready and disconnected are written +to stdout only.`, Annotations: map[string]string{ "agent_notes": "Long-running. Writes one JSON object per changed thread to stdout (NDJSON), not the usual envelope. Use --exit-on-first to block until one change lands and then exit.", }, Example: ` hey watch hey watch --box imbox --events added - hey watch --box imbox --exit-on-first - hey watch --run-async 'notify-send "New mail in $HEY_BOX_KIND"' + hey watch --box imbox --events new --exit-on-first + hey watch --box imbox --events new --run-async 'notify-send -a HEY "New mail in HEY"' hey watch --run-sync ./triage.sh hey watch --since 2026-08-18T09:00:00Z`, RunE: watchCommand.run, @@ -80,8 +105,8 @@ Pass one or the other.`, } flags := watchCommand.cmd.Flags() - flags.StringArrayVar(&watchCommand.boxes, "box", nil, "Box to watch by name or ID (repeatable, defaults to all)") - flags.StringSliceVar(&watchCommand.events, "events", watchableChanges, "Changes to report: added, updated, deleted") + flags.StringArrayVar(&watchCommand.boxes, "box", nil, "Box whose changes to report, by name or ID (repeatable, defaults to all; every box is followed either way, so new mail is judged across all of them)") + flags.StringSliceVar(&watchCommand.events, "events", defaultChanges, "Changes to report: added, updated, deleted, resync, and new for the added and updated threads that are new mail") flags.StringVar(&watchCommand.since, "since", "", "Report changes since this time first (RFC 3339 or YYYY-MM-DD)") flags.StringVar(&watchCommand.asyncScript, "run-async", "", "Shell command to spawn per change, without waiting for it") flags.StringVar(&watchCommand.syncScript, "run-sync", "", "Shell command to run per change, one at a time, waiting for each") @@ -113,7 +138,13 @@ func (c *watchCommand) run(cmd *cobra.Command, args []string) error { ctx = timed } - boxes, err := c.watchedBoxes(ctx) + // New mail is measured against the watch's start, so that is taken before + // the boxes' cursors are read — and the cursors start no later than it, so + // nothing that lands between the two sits behind a cursor, read by nothing. + started := serverNow(ctx) + newMail := trackNewMail(started) + + boxes, err := c.watchedBoxes(ctx, started) if err != nil { return err } @@ -124,10 +155,11 @@ func (c *watchCommand) run(cmd *cobra.Command, args []string) error { asyncScript: c.asyncScript, syncScript: c.syncScript, exitOnFirst: c.exitOnFirst, + newMail: newMail, out: cmd.OutOrStdout(), errOut: cmd.ErrOrStderr(), styled: writer.IsStyled(), - catchUp: make(chan struct{}, 1), + connection: make(chan struct{}, 1), unread: map[int64]bool{}, running: make(chan struct{}, asyncScriptLimit), } @@ -141,9 +173,10 @@ func (c *watchCommand) run(cmd *cobra.Command, args []string) error { subscription, err := client.Subscribe(ctx, actioncable.Identifier{Channel: changesChannel}, actioncable.OnConnected(func(reconnected bool) { if reconnected { - watch.askForCatchUp() + watch.noteConnection(true) } }), + actioncable.OnDisconnected(func(willReconnect bool) { watch.noteConnection(false) }), actioncable.OnRejected(func() { watch.rejected.Store(true) })) if err != nil { return apierr.ErrAPI(0, fmt.Sprintf("could not subscribe to posting changes: %v", err)) @@ -190,7 +223,7 @@ func (c *watchCommand) watchedChanges() (map[string]bool, error) { return changes, nil } -func (c *watchCommand) watchedBoxes(ctx context.Context) (map[int64]*watchedBox, error) { +func (c *watchCommand) watchedBoxes(ctx context.Context, started time.Time) (map[int64]*watchedBox, error) { listed, err := sdk.Boxes().List(ctx) if err != nil { return nil, apierr.FromSDK(err) @@ -199,12 +232,13 @@ func (c *watchCommand) watchedBoxes(ctx context.Context) (map[int64]*watchedBox, return nil, apierr.ErrAPI(0, "could not list boxes") } + // Every box is followed, whatever --box says: a thread's activity in one + // box is what decides whether its next change in another is new mail — a + // reply in The Feed, then a move into the Imbox, is not — so the record + // has to see them all. --box picks the boxes whose changes are reported. watched := map[int64]*watchedBox{} + reported := 0 for _, box := range *listed { - if !c.watching(box) { - continue - } - cursor, err := watchCursor(box.PostingChangesUrl, c.since) if err != nil { return nil, err @@ -212,11 +246,17 @@ func (c *watchCommand) watchedBoxes(ctx context.Context) (map[int64]*watchedBox, if cursor.Since == "" { continue } + if c.since == "" { + cursor = noLaterThan(cursor, started) + } - watched[box.Id] = &watchedBox{id: box.Id, kind: box.Kind, name: box.Name, cursor: cursor} + watched[box.Id] = &watchedBox{id: box.Id, kind: box.Kind, name: box.Name, cursor: cursor, reported: c.watching(box)} + if watched[box.Id].reported { + reported++ + } } - if len(watched) == 0 { + if reported == 0 { return nil, apierr.ErrNotFound("box", strings.Join(c.boxes, ", ")) } @@ -228,11 +268,13 @@ func (c *watchCommand) watching(box generated.Box) bool { return true } - return slices.ContainsFunc(c.boxes, func(wanted string) bool { - return strings.EqualFold(wanted, box.Kind) || - strings.EqualFold(wanted, box.Name) || - wanted == strconv.FormatInt(box.Id, 10) - }) + return slices.ContainsFunc(c.boxes, func(wanted string) bool { return boxIs(box, wanted) }) +} + +func boxIs(box generated.Box, wanted string) bool { + return strings.EqualFold(wanted, box.Kind) || + strings.EqualFold(wanted, box.Name) || + wanted == strconv.FormatInt(box.Id, 10) } // watchCursor is where a box's changes feed should be read from. The server bakes its own @@ -254,11 +296,29 @@ func watchCursor(changesURL, since string) (hey.PostingChangesCursor, error) { if err != nil { return hey.PostingChangesCursor{}, err } - cursor.Since = at.UTC().Format("2006-01-02T15:04:05.000Z") + cursor.Since = at.UTC().Format(watchCursorTimeLayout) return cursor, nil } +const watchCursorTimeLayout = "2006-01-02T15:04:05.000Z" + +// noLaterThan moves a box's cursor back to the watch's start when the box's +// own is later. The server bakes the box's last posting activity into its +// cursor, so mail that landed after the watch read HEY's clock and before it +// read the box list is already behind the cursor: the feed would start after +// it, and nothing would ever report it. Starting from the watch's own start +// reads it as part of the catch-up instead — and it is new, since it is later +// than the start. A cursor that cannot be read is left as it is. +func noLaterThan(cursor hey.PostingChangesCursor, started time.Time) hey.PostingChangesCursor { + at, err := time.Parse(watchCursorTimeLayout, cursor.Since) + if err == nil && at.After(started) { + cursor.Since = started.UTC().Format(watchCursorTimeLayout) + } + + return cursor +} + func parseWatchSince(since string) (time.Time, error) { if at, err := time.Parse(time.RFC3339, since); err == nil { return at, nil @@ -270,22 +330,41 @@ func parseWatchSince(since string) (time.Time, error) { } type watchedBox struct { - id int64 - kind string - name string - cursor hey.PostingChangesCursor + id int64 + kind string + name string + cursor hey.PostingChangesCursor + reported bool // --box named it, or named nothing } -// watchEvent is one changed posting, as a line of NDJSON or as a script's stdin. +// watchEvent is one changed posting, as a line of NDJSON or as a script's stdin — +// or a word about the watch itself, which has no posting and, for ready and +// disconnected, no box either. New is set on every added and updated posting, +// true or false, and on nothing else. type watchEvent struct { Change string `json:"change"` At string `json:"at"` - Box watchEventBox `json:"box"` - PostingID int64 `json:"posting_id"` + Box *watchEventBox `json:"box,omitempty"` + PostingID int64 `json:"posting_id,omitempty"` ThreadID int64 `json:"thread_id,omitempty"` + New *bool `json:"new,omitempty"` Posting *generated.Posting `json:"posting,omitempty"` } +func (e watchEvent) isNew() bool { + return e.New != nil && *e.New +} + +// The watch's own news. A resync is a change — something happened, more than the +// feed could list — so it goes wherever changes go, when --events asks for it; +// ready and disconnected are written to stdout only: a script runs per change, +// and neither is one. +const ( + watchReady = "ready" + watchDisconnected = "disconnected" + watchResync = "resync" +) + type watchEventBox struct { ID int64 `json:"id"` Kind string `json:"kind"` @@ -300,11 +379,15 @@ type postingsWatch struct { asyncScript string syncScript string exitOnFirst bool + newMail *newMail out io.Writer errOut io.Writer styled bool - catchUp chan struct{} + connection chan struct{} + transitionsMu sync.Mutex + transitions []bool rejected atomic.Bool + catchingUp bool unread map[int64]bool backoff time.Duration retry <-chan time.Time @@ -314,7 +397,7 @@ type postingsWatch struct { } func (w *postingsWatch) listen(ctx context.Context, subscription *actioncable.Subscription) error { - if err := w.readEveryBox(ctx); err != nil { + if err := w.catchUp(ctx); err != nil { return err } @@ -323,13 +406,13 @@ func (w *postingsWatch) listen(ctx context.Context, subscription *actioncable.Su case <-ctx.Done(): // An interrupt or a --timeout is how a watch is meant to end. return nil - case <-w.catchUp: - if err := w.readEveryBox(ctx); err != nil { + case <-w.connection: + if err := w.followConnection(ctx); err != nil { return err } case <-w.retry: w.retry = nil - if err := w.readUnreadBoxes(ctx); err != nil { + if err := w.retryUnread(ctx); err != nil { return err } case message, open := <-subscription.Messages(): @@ -360,15 +443,114 @@ func (w *postingsWatch) closedError(ctx context.Context) error { } } -// askForCatchUp is called from the cable client's own goroutine, so it drops the ask -// when one is already waiting rather than blocking the connection. -func (w *postingsWatch) askForCatchUp() { +// noteConnection is called from the cable client's own goroutine with every drop +// and reconnect. The transitions queue up in the order they happened, and the +// signal never blocks the connection: one wake-up drains them all. +func (w *postingsWatch) noteConnection(connected bool) { + w.transitionsMu.Lock() + w.transitions = append(w.transitions, connected) + w.transitionsMu.Unlock() + select { - case w.catchUp <- struct{}{}: + case w.connection <- struct{}{}: default: } } +// followConnection acts on the queued transitions in order: a drop is announced, +// a reconnect catches every box up and then announces ready. Order is what keeps +// a reader's picture right — a reconnect that completed while a slow catch-up +// held the loop must not have its earlier drop announced after its ready. The +// transitions are taken one at a time, so a drop queued behind a reconnect is +// still in the queue while the reconnect catches up, where readyOnceCaughtUp +// can see it. +func (w *postingsWatch) followConnection(ctx context.Context) error { + for { + connected, queued := w.nextTransition() + if !queued { + return nil + } + if !connected { + // A ready still owed by a catch-up is owed no longer: the next + // reconnect catches up afresh and announces its own. + w.catchingUp = false + w.announce(watchDisconnected) + continue + } + if err := w.catchUp(ctx); err != nil { + return err + } + } +} + +// catchUp reads every box and announces ready — but only once every box has +// been read. A read that failed is retried on the backoff with the box's cursor +// where it was, and ready waits for it: a reader told ready while a box is +// still behind would take the gap for a clean picture. From ready on, anything +// after the cursors is reported: a reader that wants a gap-free picture reads +// its own state now, not before. +func (w *postingsWatch) catchUp(ctx context.Context) error { + if err := w.readEveryBox(ctx); err != nil { + return err + } + w.catchingUp = true + w.readyOnceCaughtUp(ctx) + + return nil +} + +// retryUnread reads the boxes whose last read failed, and announces the ready a +// catch-up left owing once the last of them is read. +func (w *postingsWatch) retryUnread(ctx context.Context) error { + if err := w.readUnreadBoxes(ctx); err != nil { + return err + } + w.readyOnceCaughtUp(ctx) + + return nil +} + +// readyOnceCaughtUp pays the ready a catch-up owes, once no box is behind — +// and not while a drop is waiting to be acted on: the connection went away +// during the reads, so the stream would say ready with the subscription +// already down. The drop cancels the debt when it is drained, and the +// reconnect's catch-up announces its own. Nor when the watch is on its way +// out: a catch-up that reported the change --exit-on-first was waiting for, +// or one cut short by an interrupt, is not a watch that is live. +// +// The look at the queue and the announcement are one critical section with +// noteConnection's queueing, so a drop lands either before the look — and +// withholds the ready — or after the ready is out, where the stream's order +// is the order things happened. The cable's goroutine waits on one line of +// output at most. +func (w *postingsWatch) readyOnceCaughtUp(ctx context.Context) { + if !w.catchingUp || len(w.unread) > 0 || w.finished() || ctx.Err() != nil { + return + } + + w.transitionsMu.Lock() + defer w.transitionsMu.Unlock() + if slices.Contains(w.transitions, false) { + return + } + w.catchingUp = false + w.announce(watchReady) +} + +// nextTransition takes the oldest queued transition, if there is one. +func (w *postingsWatch) nextTransition() (connected, queued bool) { + w.transitionsMu.Lock() + defer w.transitionsMu.Unlock() + + if len(w.transitions) == 0 { + return false, false + } + connected = w.transitions[0] + w.transitions = w.transitions[1:] + + return connected, true +} + func (w *postingsWatch) readEveryBox(ctx context.Context) error { for _, id := range slices.Sorted(maps.Keys(w.boxes)) { if err := w.readBox(ctx, w.boxes[id]); err != nil { @@ -401,7 +583,11 @@ func (w *postingsWatch) read(ctx context.Context, message actioncable.Message) e } if box, watching := w.boxes[notification.BoxID]; watching { - return w.readBox(ctx, box) + if err := w.readBox(ctx, box); err != nil { + return err + } + // The box a catch-up left behind may be the one that just rang. + w.readyOnceCaughtUp(ctx) } return nil @@ -425,18 +611,31 @@ func (w *postingsWatch) readBox(ctx context.Context, box *watchedBox) error { if changes.FullSyncRequired { fmt.Fprintf(w.errOut, "notice: too much changed in %s to follow one change at a time — skipping ahead, read the box with `hey box %s`\n", box.name, box.kind) - return w.skipAhead(ctx, box) + skipped, err := w.skipAhead(ctx, box) + if err != nil { + return err + } + // A resync says the box is worth re-reading; a box that is gone is not. + if skipped { + w.report(ctx, watchEvent{Change: watchResync, At: watchTime(time.Now())}, box, nil) + } + return nil } if changes.NextCursor != nil { box.cursor = *changes.NextCursor } + // Every added and updated posting is classified as new or not, and then + // recorded — in every box and whatever --events or --box asks for, so a + // thread's activity is known when its next change comes, even when this + // one went unreported. Recorded one at a time, so a posting a read carries + // twice is new once at most. for _, posting := range changes.Added { - w.report(ctx, watchEvent{Change: "added", At: watchTime(posting.CreatedAt), PostingID: posting.Id}, box, &posting) + w.report(ctx, watchEvent{Change: "added", At: watchTime(posting.CreatedAt), PostingID: posting.Id, New: w.classify(box, posting)}, box, &posting) } for _, posting := range changes.Updated { - w.report(ctx, watchEvent{Change: "updated", At: watchTime(posting.UpdatedAt), PostingID: posting.Id}, box, &posting) + w.report(ctx, watchEvent{Change: "updated", At: watchTime(posting.UpdatedAt), PostingID: posting.Id, New: w.classify(box, posting)}, box, &posting) } for _, posting := range changes.Deleted { w.report(ctx, watchEvent{Change: "deleted", At: watchTime(posting.DeletedAt), PostingID: posting.Id}, box, nil) @@ -445,6 +644,13 @@ func (w *postingsWatch) readBox(ctx context.Context, box *watchedBox) error { return nil } +// classify decides whether a posting is new mail and records it, in that order. +func (w *postingsWatch) classify(box *watchedBox, posting generated.Posting) *bool { + isNew := w.newMail.isNew(box.id, posting) + w.newMail.record(posting) + return &isNew +} + // permanentReadError tells a read that will never work from one that might. A malformed // cursor or credentials the server won't take doesn't get better by waiting two minutes, // and a watch that retried it silently would sit there for hours and still exit 0. @@ -478,58 +684,65 @@ func (w *postingsWatch) wasRead(box *watchedBox) { } // skipAhead moves a box's cursor to the server's current one, which is the only way -// back once a box has changed more than an increment can carry. +// back once a box has changed more than an increment can carry, and says whether it +// did. // // A box the server no longer lists, or no longer serves a changes feed for, has no // cursor to skip to: keeping the one it had would answer 409 on every read, and // installing an empty one would be a usage error on every read instead. Either way the -// box can't be followed any more, so it stops being watched. -func (w *postingsWatch) skipAhead(ctx context.Context, box *watchedBox) error { +// box can't be followed any more, so it stops being watched — and nothing was skipped. +func (w *postingsWatch) skipAhead(ctx context.Context, box *watchedBox) (bool, error) { listed, err := sdk.Boxes().List(ctx) if err != nil { - return apierr.FromSDK(err) + return false, apierr.FromSDK(err) } if listed == nil { - return apierr.ErrAPI(0, "could not list boxes") + return false, apierr.ErrAPI(0, "could not list boxes") } for _, listedBox := range *listed { if listedBox.Id == box.id { cursor, err := watchCursor(listedBox.PostingChangesUrl, "") if err != nil { - return err + return false, err } if cursor.Since != "" { box.cursor = cursor - return nil + w.newMail.skippedTo(box.id, cursor) + return true, nil } } } - return w.stopWatching(box) + return false, w.stopWatching(box) } // stopWatching drops a box the watch can't follow any longer. When it was the last one -// there is nothing left to wait for, and saying so beats a watch that sits there for -// good, reading nothing. +// whose changes are reported there is nothing left to wait for — the others are only +// followed for the record — and saying so beats a watch that sits there for good, +// reporting nothing. func (w *postingsWatch) stopWatching(box *watchedBox) error { fmt.Fprintf(w.errOut, "notice: %s can no longer be followed — it is gone from this account\n", box.name) delete(w.boxes, box.id) delete(w.unread, box.id) - if len(w.boxes) == 0 { - return apierr.ErrNotFound("box", box.name) + for _, remaining := range w.boxes { + if remaining.reported { + return nil + } } - return nil + return apierr.ErrNotFound("box", box.name) } -func (w *postingsWatch) report(ctx context.Context, event watchEvent, box *watchedBox, posting *generated.Posting) { - if w.finished() || !w.changes[event.Change] { - return +// report hands one change on — printed, or run through the script — and says +// whether it did. +func (w *postingsWatch) report(ctx context.Context, event watchEvent, box *watchedBox, posting *generated.Posting) bool { + if !box.reported || w.finished() || !w.reporting(event) { + return false } - event.Box = watchEventBox{ID: box.id, Kind: box.kind, Name: box.name} + event.Box = &watchEventBox{ID: box.id, Kind: box.kind, Name: box.name} if posting != nil { event.Posting = posting event.ThreadID = resolvePostingTopicID(*posting) @@ -546,12 +759,41 @@ func (w *postingsWatch) report(ctx context.Context, event watchEvent, box *watch default: w.writeJSON(event) } + + return true +} + +// reporting says whether a change is handed on. --events is a union: a posting +// change is reported when its own change was asked for, or when new was and the +// posting is new mail; a resync when it was asked for, which it is by default. +func (w *postingsWatch) reporting(event watchEvent) bool { + if !slices.Contains(postingChanges, event.Change) { + return w.changes[event.Change] + } + + return w.changes[event.Change] || (w.changes["new"] && event.isNew()) } func (w *postingsWatch) finished() bool { return w.exitOnFirst && w.reported > 0 } +// announce writes a word about the watch itself to whoever reads the stream. A +// script runs per change and this is not one, so a --run-* watch isn't told, +// and it never counts towards --exit-on-first. +func (w *postingsWatch) announce(change string) { + if w.asyncScript != "" || w.syncScript != "" { + return + } + + event := watchEvent{Change: change, At: watchTime(time.Now())} + if w.styled { + fmt.Fprintln(w.out, watchLine(event)) + } else { + w.writeJSON(event) + } +} + // spawnScript starts the script and leaves it to get on with it. Whether it worked, // and whether it overlaps with the next one, is the script's business — and it outlives // the watch, so interrupting `hey` doesn't cut a script off halfway. @@ -618,11 +860,24 @@ func (w *postingsWatch) scriptCommand(ctx context.Context, script string, event command.Stdin = bytes.NewReader(append(payload, '\n')) command.Stdout = w.out command.Stderr = w.errOut - command.Env = append(os.Environ(), event.environment()...) + command.Env = append(withoutWatchVariables(os.Environ()), event.environment()...) return command, nil } +// The variables a script is handed per event. Any of them already in the +// environment — a watch started by another watch's script, say — would reach +// the script for an event that does not set it: HEY_NEW=1 on an update that is +// not new, a thread id on a deletion. They are the event's to set or leave unset. +var watchVariables = []string{"HEY_CHANGE", "HEY_AT", "HEY_BOX_ID", "HEY_BOX_KIND", "HEY_BOX_NAME", "HEY_POSTING_ID", "HEY_THREAD_ID", "HEY_NEW"} + +func withoutWatchVariables(environment []string) []string { + return slices.DeleteFunc(slices.Clone(environment), func(variable string) bool { + name, _, _ := strings.Cut(variable, "=") + return slices.Contains(watchVariables, name) + }) +} + func (w *postingsWatch) writeJSON(event watchEvent) { payload, err := output.MarshalJSON(event) if err != nil { @@ -645,25 +900,51 @@ func (e watchEvent) environment() []string { environment := []string{ "HEY_CHANGE=" + e.Change, "HEY_AT=" + e.At, - "HEY_BOX_ID=" + strconv.FormatInt(e.Box.ID, 10), - "HEY_BOX_KIND=" + e.Box.Kind, - "HEY_BOX_NAME=" + e.Box.Name, - "HEY_POSTING_ID=" + strconv.FormatInt(e.PostingID, 10), + } + if e.Box != nil { + environment = append(environment, + "HEY_BOX_ID="+strconv.FormatInt(e.Box.ID, 10), + "HEY_BOX_KIND="+e.Box.Kind, + "HEY_BOX_NAME="+e.Box.Name) + } + if e.PostingID != 0 { + environment = append(environment, "HEY_POSTING_ID="+strconv.FormatInt(e.PostingID, 10)) } if e.ThreadID != 0 { environment = append(environment, "HEY_THREAD_ID="+strconv.FormatInt(e.ThreadID, 10)) } + if e.isNew() { + environment = append(environment, "HEY_NEW=1") + } else { + environment = append(environment, "HEY_NEW=0") + } return environment } func watchLine(event watchEvent) string { - description := fmt.Sprintf("posting %d", event.PostingID) - if event.Posting != nil { + var boxName, description string + if event.Box != nil { + boxName = event.Box.Name + } + change := event.Change + if event.isNew() { + change += " (new)" + } + switch { + case event.Change == watchReady: + description = "watching for changes" + case event.Change == watchDisconnected: + description = "connection lost — reconnecting" + case event.Change == watchResync: + description = "too much changed to follow one change at a time — skipped ahead" + case event.Posting != nil: description = fmt.Sprintf("%s — %s (thread %d)", terminal.SanitizeLine(event.Posting.Creator.Name), truncate(terminal.SanitizeLine(event.Posting.Summary), 50), event.ThreadID) + default: + description = fmt.Sprintf("posting %d", event.PostingID) } - return fmt.Sprintf("%s %-8s %-24s %s", event.At, event.Change, terminal.SanitizeLine(event.Box.Name), description) + return fmt.Sprintf("%s %-13s %-24s %s", event.At, change, terminal.SanitizeLine(boxName), description) } func watchTime(at time.Time) string { diff --git a/internal/cmd/watch_new.go b/internal/cmd/watch_new.go new file mode 100644 index 00000000..a29775a9 --- /dev/null +++ b/internal/cmd/watch_new.go @@ -0,0 +1,118 @@ +package cmd + +import ( + "context" + "net/http" + "strconv" + "time" + + "github.com/basecamp/hey-sdk/go/pkg/generated" + hey "github.com/basecamp/hey-sdk/go/pkg/hey" +) + +// New mail is a watch event: every added and updated line says whether the +// posting is new, and --events new selects the ones that are. What counts as +// new needs HEY's semantics and state across events, which is why it is decided +// here, once, rather than by every script and widget that reads the stream — +// what to do about it (a toast, a bell, a re-read) is the reader's. +// +// There is no state file: new means active since the watch began, so the +// backlog a box's first read carries is not new, and what the watch remembers — +// when each thread was last active — lives and dies with it. + +// newMail keeps up with every thread the watch reads, in every box and whatever +// --events reports, so a thread moved into a box, or known from a change that +// was filtered out, is not mistaken for new when its next change comes. +type newMail struct { + started time.Time + floors map[int64]time.Time + activeAt map[int64]time.Time +} + +// trackNewMail starts the record. started is the moment before which activity +// is backlog, on the server's clock, since that is the clock every posting's +// active_at is on. +func trackNewMail(started time.Time) *newMail { + return &newMail{started: started, floors: map[int64]time.Time{}, activeAt: map[int64]time.Time{}} +} + +// skippedTo sets a box's floor at the cursor the watch skipped ahead to. A box +// that changed more than the feed can list was never read across that gap, so +// whatever was active in it is mail the watch missed: a thread it knows, or +// one it does not, updated later while still unseen — moved, say — would +// otherwise measure its gap activity against an older record and read as +// new. Activity at or before the floor is never new in that box; the cursor is +// the box's last posting activity, which bounds every thread in it. The floor +// is the box's alone — a gap thread that moves to another box is measured +// there, and may still read as new once. The resync line is the reader's cue +// to re-read the box either way. +func (n *newMail) skippedTo(boxID int64, cursor hey.PostingChangesCursor) { + if at, err := time.Parse(watchCursorTimeLayout, cursor.Since); err == nil { + n.floors[boxID] = at + } +} + +// serverNow is HEY's clock at the moment the watch began, read off the Date +// header of one cheap request, so that the cutoff between backlog and new mail +// sits on the same clock as every posting's active_at and a workstation running +// fast or slow can neither call the backlog new nor sit on new mail. +// +// Date is the server's clock when it answered, and the watch began when it +// asked: mail that lands in between is later than the start but no later than +// Date, and a start taken at Date would leave it behind a box's cursor, read by +// nothing. So the answer is translated back to the request's start by the time +// the request took — the local monotonic clock, which a wrong wall clock does +// not touch — and a slow request, or one the SDK retried, only moves the start +// earlier. Date is whole seconds, rounded down, which errs the same way: towards +// calling mail a moment old new rather than mail a moment new old. The SDK +// caches GETs by URL, so a query the server ignores keeps this one out of the +// cache; and when the server's clock can't be read, the local clock at the +// start stands in. Either way the start is handed out as a cutoff: a whole +// millisecond, strictly before the instant it stands for. +func serverNow(ctx context.Context) time.Time { + started := time.Now() + response, err := rootSDK.Get(ctx, "/identity.json?clock="+strconv.FormatInt(started.UnixNano(), 10)) + if err != nil || response == nil || response.FromCache { + return cutoffBefore(started) + } + if at, err := http.ParseTime(response.Headers.Get("Date")); err == nil { + return cutoffBefore(at.Add(-time.Since(started))) + } + + return cutoffBefore(started) +} + +// cutoffBefore makes an instant usable as the watch's start: a cursor is +// milliseconds and so is every active_at, the feed answers what is strictly +// later than its cursor, and isNew asks whether activity is strictly later +// than the start — so the start has to be a whole millisecond, and the one +// before the instant, or mail in the instant's own millisecond would be +// neither read nor new. +func cutoffBefore(at time.Time) time.Time { + return at.Truncate(time.Millisecond).Add(-time.Millisecond) +} + +// isNew says whether a posting is new mail: unseen, not muted, and active since +// this watch last saw the thread — or since the watch began, for a thread it +// has no record of, because anything active before that was already there: the +// backlog a box's first read carries from the server's cursor, or a thread that +// merely moved in. active_at moves on new mail only, not when a thread is read, +// muted or moved, so none of those is new and a reply on a known thread is. +func (n *newMail) isNew(boxID int64, posting generated.Posting) bool { + last, known := n.activeAt[posting.Id] + if !known { + last = n.started + } + if floor, ok := n.floors[boxID]; ok && floor.After(last) { + last = floor + } + + return !posting.Seen && !posting.Muted && posting.ActiveAt.After(last) +} + +// record keeps a thread's latest activity, new or not, reported or not. A +// posting is classified and then recorded, so the same activity read twice — +// or carried twice by one read — is new once. +func (n *newMail) record(posting generated.Posting) { + n.activeAt[posting.Id] = posting.ActiveAt +} diff --git a/internal/cmd/watch_new_test.go b/internal/cmd/watch_new_test.go new file mode 100644 index 00000000..120afa57 --- /dev/null +++ b/internal/cmd/watch_new_test.go @@ -0,0 +1,538 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + actioncable "github.com/basecamp/actioncable-go" + + "github.com/basecamp/hey-sdk/go/pkg/generated" + hey "github.com/basecamp/hey-sdk/go/pkg/hey" + + "github.com/basecamp/hey-cli/internal/auth" +) + +var watchStarted = time.Date(2026, 8, 21, 9, 0, 0, 0, time.UTC) + +func newPosting(id int64, sender, subject string, activeAt time.Time) generated.Posting { + return generated.Posting{Id: id, Name: subject, ActiveAt: activeAt, Creator: generated.Contact{Name: sender}} +} + +// classifyRead runs one read's worth of postings through the tracker the way readBox +// does — every posting decided against the record from before the read, then the +// whole read recorded — and answers which were new. +func classifyRead(tracker *newMail, postings ...generated.Posting) []int64 { + return classifyReadOf(tracker, 24088, postings...) +} + +func classifyReadOf(tracker *newMail, boxID int64, postings ...generated.Posting) []int64 { + var fresh []int64 + for _, posting := range postings { + if tracker.isNew(boxID, posting) { + fresh = append(fresh, posting.Id) + } + tracker.record(posting) + } + return fresh +} + +func TestNewMailIsSinceTheWatchBeganNotTheBacklog(t *testing.T) { + tracker := trackNewMail(watchStarted) + before := watchStarted.Add(-time.Hour) + after := watchStarted.Add(30 * time.Second) + + // A box's first read carries everything since the server's cursor — the + // box's last activity — which may be hours of backlog, plus mail that + // arrived while the watch was starting up. + backlog := newPosting(101, "Maria Delgado", "Lunch on Thursday?", before) + arrived := newPosting(102, "Northwind Invoicing", "Invoice #4021", after) + if fresh := classifyRead(tracker, backlog, arrived); len(fresh) != 1 || fresh[0] != 102 { + t.Fatalf("new = %v, want the arrival and not the backlog", fresh) + } + + // The backlog thread read later: recorded by the first read, so not new. + read := backlog + read.Seen = true + if fresh := classifyRead(tracker, read); len(fresh) != 0 { + t.Errorf("new = %v, want a backlog thread read later left alone", fresh) + } +} + +func TestNewMailIsNeverSeenOrMuted(t *testing.T) { + tracker := trackNewMail(watchStarted) + later := watchStarted.Add(30 * time.Second) + + seen := newPosting(101, "Maria Delgado", "Lunch on Thursday?", later) + seen.Seen = true + muted := newPosting(103, "Weekend Deals", "48 hours only", later) + muted.Muted = true + + if fresh := classifyRead(tracker, seen, muted); len(fresh) != 0 { + t.Errorf("new = %v, want neither a seen nor a muted thread", fresh) + } +} + +func TestNewMailIsNewActivityOnly(t *testing.T) { + tracker := trackNewMail(watchStarted) + before := watchStarted.Add(-time.Hour) + after := watchStarted.Add(30 * time.Second) + + // A thread the watch has never seen, updated without new mail: a seen flip, + // a move into the box. Its activity predates the watch. + if fresh := classifyRead(tracker, newPosting(101, "Maria Delgado", "Lunch on Thursday?", before)); len(fresh) != 0 { + t.Fatalf("new = %v, want an update without new activity left alone", fresh) + } + + // A reply on it. + if fresh := classifyRead(tracker, newPosting(101, "Maria Delgado", "Lunch on Thursday?", after)); len(fresh) != 1 { + t.Fatalf("new = %v, want new activity on a known thread", fresh) + } + + // Marking the reply unseen again: the same activity, not new twice. + if fresh := classifyRead(tracker, newPosting(101, "Maria Delgado", "Lunch on Thursday?", after)); len(fresh) != 0 { + t.Errorf("new = %v, want the same activity not counted twice", fresh) + } + + // A thread that arrived during the watch, then was read: the arrival + // recorded its activity, so the seen flip's update is not new. + arrived := newPosting(102, "Northwind Invoicing", "Invoice #4021", after) + classifyRead(tracker, arrived) + read := arrived + read.Seen = true + if fresh := classifyRead(tracker, read); len(fresh) != 0 { + t.Errorf("new = %v, want reading a thread not to count", fresh) + } +} + +func TestNewMailRemembersEveryBox(t *testing.T) { + tracker := trackNewMail(watchStarted) + after := watchStarted.Add(30 * time.Second) + + // A newsletter lands in The Feed — new there — and is then moved to the + // Imbox: the same activity, known from the other box, so not new again. + newsletter := newPosting(201, "Weekend Deals", "48 hours only", after) + if fresh := classifyRead(tracker, newsletter); len(fresh) != 1 { + t.Fatalf("new = %v, want the arrival in The Feed", fresh) + } + if fresh := classifyRead(tracker, newsletter); len(fresh) != 0 { + t.Errorf("new = %v, want a thread moved in from another box left alone", fresh) + } +} + +func TestNewMailAfterASkipAheadIsSinceTheSkip(t *testing.T) { + tracker := trackNewMail(watchStarted) + beforeTheWatch := watchStarted.Add(-time.Hour) + inTheGap := watchStarted.Add(30 * time.Minute) + afterTheSkip := watchStarted.Add(61 * time.Minute) + + // A thread the watch knows from before the gap. + known := newPosting(100, "Sam Whitfield", "Draft agenda for Monday", beforeTheWatch) + classifyRead(tracker, known) + + tracker.skippedTo(24088, hey.PostingChangesCursor{Since: "2026-08-21T10:00:00.000Z", Version: "2"}) + + // The known thread got a reply inside the gap — mail the watch missed — + // and is then moved while still unseen: its activity is at the floor, not + // new. The resync line was the cue to re-read the box. + moved := newPosting(100, "Sam Whitfield", "Draft agenda for Monday", inTheGap) + if fresh := classifyRead(tracker, moved); len(fresh) != 0 { + t.Errorf("new = %v, want a known thread's gap activity left alone", fresh) + } + // A thread that arrived in the gap, then moved or labelled while unseen. + if fresh := classifyRead(tracker, newPosting(101, "Maria Delgado", "Lunch on Thursday?", inTheGap)); len(fresh) != 0 { + t.Errorf("new = %v, want an unknown thread active in the gap left alone", fresh) + } + // Mail after the cursor the watch skipped to is new, on both kinds. + if fresh := classifyRead(tracker, newPosting(100, "Sam Whitfield", "Draft agenda for Monday", afterTheSkip), newPosting(102, "Northwind Invoicing", "Invoice #4021", afterTheSkip)); len(fresh) != 2 { + t.Errorf("new = %v, want mail since the skip on a known and an unknown thread", fresh) + } + // The floor is the box's alone: another box measures against the start. + if fresh := classifyReadOf(tracker, 24089, newPosting(103, "Weekend Deals", "48 hours only", inTheGap)); len(fresh) != 1 { + t.Errorf("new = %v, want another box unaffected by this one's skip", fresh) + } + + // A cursor that cannot be read moves nothing. + tracker.skippedTo(24089, hey.PostingChangesCursor{Since: "later"}) + if _, has := tracker.floors[24089]; has { + t.Error("an unreadable cursor must not become a floor") + } +} + +func TestNewMailCarriedTwiceByOneReadIsNewOnce(t *testing.T) { + tracker := trackNewMail(watchStarted) + after := watchStarted.Add(30 * time.Second) + + // One read carrying the same unseen arrival twice — added and updated, + // the same activity: new once, since each posting is recorded as soon as + // it is decided. + arrived := newPosting(101, "Maria Delgado", "Lunch on Thursday?", after) + if fresh := classifyRead(tracker, arrived, arrived); len(fresh) != 1 || fresh[0] != 101 { + t.Errorf("new = %v, want the arrival once", fresh) + } +} + +func TestWatchFollowsEveryBoxAndReportsTheOnesAskedFor(t *testing.T) { + // A reply lands on an unseen thread in The Feed, which --box imbox does not + // report; the thread is then moved into the Imbox. The move is not new + // mail — the reply was known from The Feed — which only holds if The Feed + // was followed all along. + server := changesServer(t, + `{"updated":[{"id":9001,"kind":"topic","box_id":24089,"name":"48 hours only","active_at":"2026-08-21T09:00:20Z","creator":{"name":"Weekend Deals"}}]}`, + `{"updated":[{"id":9001,"kind":"topic","box_id":24088,"name":"48 hours only","active_at":"2026-08-21T09:00:20Z","creator":{"name":"Weekend Deals"}}]}`) + watch, out := newTestWatch("added", "updated", "deleted", "new") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + watch.boxes[24089] = &watchedBox{id: 24089, kind: "feedbox", name: "The Feed", cursor: cursor, reported: false} + + if err := watch.read(context.Background(), actioncable.Message(`{"change":"upsert","box_id":24089}`)); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Len() != 0 { + t.Fatalf("wrote %q, want nothing for a box --box left out", out.String()) + } + ringBox(t, watch) + + lines := watchLines(t, out) + if len(lines) != 1 || lines[0]["change"] != "updated" || lines[0]["new"] != false || lines[0]["box"].(map[string]any)["kind"] != "imbox" { + t.Errorf("wrote %v, want the move into the Imbox, not new", lines) + } +} + +func TestServerNowReadsTheServersClock(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + var requested []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requested = append(requested, r.URL.String()) + w.Header().Set("Date", "Fri, 21 Aug 2026 09:00:05 GMT") + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":1}`)) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + date := time.Date(2026, 8, 21, 9, 0, 5, 0, time.UTC) + got := serverNow(context.Background()) + // The Date header, less the instant the request took: on the server's + // clock, whatever the local one says, and no later than the header. + if got.After(date) || date.Sub(got) > time.Second { + t.Errorf("serverNow = %v, want the Date header translated back to the request's start", got) + } + if len(requested) != 1 || !strings.Contains(requested[0], "/identity.json?clock=") { + t.Errorf("requested %v, want one uncacheable identity request", requested) + } + + second := serverNow(context.Background()) + if len(requested) != 2 || requested[1] == requested[0] { + t.Errorf("requested %v, want a fresh request each time, never the cache", requested) + } + if second.After(date) || date.Sub(second) > time.Second { + t.Errorf("second = %v, want the server's clock again", second) + } +} + +func TestCutoffBeforeIsAWholeMillisecondStrictlyBefore(t *testing.T) { + within := time.Date(2026, 8, 21, 9, 0, 5, 123456789, time.UTC) + if got := cutoffBefore(within); !got.Equal(time.Date(2026, 8, 21, 9, 0, 5, 122000000, time.UTC)) { + t.Errorf("cutoffBefore(%v) = %v, want the millisecond before the instant's own", within, got) + } + exact := time.Date(2026, 8, 21, 9, 0, 5, 0, time.UTC) + if got := cutoffBefore(exact); !got.Equal(exact.Add(-time.Millisecond)) { + t.Errorf("cutoffBefore(%v) = %v, want strictly before even on a boundary", exact, got) + } + + // So mail in the start's own millisecond is new, and a cursor at that + // millisecond is moved back to before it. + tracker := trackNewMail(cutoffBefore(within)) + landed := within.Truncate(time.Millisecond) + if !tracker.isNew(24088, newPosting(101, "Maria Delgado", "Lunch on Thursday?", landed)) { + t.Error("mail in the same millisecond as the watch's start is new") + } + cursor := noLaterThan(hey.PostingChangesCursor{Since: landed.Format(watchCursorTimeLayout)}, cutoffBefore(within)) + if cursor.Since != "2026-08-21T09:00:05.122Z" { + t.Errorf("cursor = %q, want it moved back to before the millisecond the mail landed in", cursor.Since) + } +} + +func TestServerNowIsTheClockWhenTheRequestBeganNotWhenItWasAnswered(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + const delay = 300 * time.Millisecond + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(delay) + w.Header().Set("Date", "Fri, 21 Aug 2026 09:00:05 GMT") + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"id":1}`)) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + got := serverNow(context.Background()) + + // Mail that lands while the server is answering is later than the start; + // a start taken at the Date header would put it before. + date := time.Date(2026, 8, 21, 9, 0, 5, 0, time.UTC) + if date.Sub(got) < delay { + t.Errorf("serverNow = %v, want at least the request's %s before the Date header %v", got, delay, date) + } + if got.Nanosecond()%int(time.Millisecond) != 0 { + t.Errorf("serverNow = %v, want a whole millisecond, the resolution of every cursor and active_at", got) + } +} + +// TestWatchReadsMailThatLandedWhileItReadTheClock is the whole startup window +// end to end: the clock request is slow, a posting lands in the Imbox while the +// server is answering it, so the box's cursor — its last posting activity — is +// earlier than the Date header. The watch's start is translated back to before +// the posting, the cursor is moved back to the start, the catch-up reads the +// posting, and it is new. +func TestWatchReadsMailThatLandedWhileItReadTheClock(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + const delay = 300 * time.Millisecond + // The Date header is 09:00:05; the posting landed at 09:00:04.900, inside + // the request; the box's cursor is therefore 09:00:04.900 as well. + var changesSince []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasPrefix(r.URL.Path, "/identity.json"): + time.Sleep(delay) + w.Header().Set("Date", "Fri, 21 Aug 2026 09:00:05 GMT") + _, _ = w.Write([]byte(`{"id":1}`)) + case r.URL.Path == "/boxes.json": + _, _ = w.Write([]byte(`[{"id":24088,"kind":"imbox","name":"Imbox","posting_changes_url":"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A04.900Z&v=2"}]`)) + case strings.Contains(r.URL.Path, "/postings/changes"): + changesSince = append(changesSince, r.URL.Query().Get("since")) + w.Header().Set("Link", `<`+r.URL.Path+`?since=2026-08-21T09%3A00%3A04.900Z&v=2>; rel="next"`) + _, _ = w.Write([]byte(`{"added":[{"id":9001,"kind":"topic","box_id":24088,"name":"Lunch on Thursday?","active_at":"2026-08-21T09:00:04.900Z","creator":{"name":"Maria Delgado"}}]}`)) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + // As run does: the clock, then the boxes, then the catch-up. + started := serverNow(context.Background()) + command := newWatchCommand() + boxes, err := command.watchedBoxes(context.Background(), started) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch, out := newTestWatch("added") + watch.boxes = boxes + watch.newMail = trackNewMail(started) + if err := watch.catchUp(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + landed := time.Date(2026, 8, 21, 9, 0, 4, 900000000, time.UTC) + if len(changesSince) != 1 { + t.Fatalf("changes read %d times, want once", len(changesSince)) + } + since, err := time.Parse(watchCursorTimeLayout, changesSince[0]) + if err != nil || !since.Before(landed) { + t.Errorf("read the feed since %q, want a cursor moved back to before the posting landed", changesSince[0]) + } + lines := watchLines(t, out) + if len(lines) != 2 || lines[0]["posting_id"] != float64(9001) || lines[0]["new"] != true || lines[1]["change"] != "ready" { + t.Errorf("wrote %v, want the posting that landed during the clock request, new, and then ready", lines) + } +} + +func TestServerNowFallsBackToTheLocalClock(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "down", http.StatusBadGateway) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + before := time.Now() + got := serverNow(context.Background()) + // The local clock at the request's start, as a cutoff: a whole millisecond, + // strictly before — so up to two milliseconds before the instant itself. + if got.Before(before.Add(-2*time.Millisecond)) || got.After(time.Now()) { + t.Errorf("serverNow = %v, want the local clock at the start when the server's can't be read", got) + } + if got.Nanosecond()%int(time.Millisecond) != 0 { + t.Errorf("serverNow = %v, want a whole millisecond", got) + } +} + +// changesServer serves the changes feed bodies in turn, the last one for every +// read after that. +func changesServer(t *testing.T, bodies ...string) *httptest.Server { + t.Helper() + reads := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body := bodies[min(reads, len(bodies)-1)] + reads++ + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Link", `<`+r.URL.Path+`?since=2026-08-21T09%3A00%3A30.000Z&v=2>; rel="next"`) + _, _ = w.Write([]byte(body)) + })) + t.Cleanup(server.Close) + t.Setenv("HEY_TOKEN", "test-token") + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + return server +} + +// watchOf is a watch on the Imbox at the server, begun at watchStarted, reporting +// the given --events. +func watchOf(t *testing.T, server *httptest.Server, events ...string) (*postingsWatch, *bytes.Buffer) { + t.Helper() + watch, out := newTestWatch(events...) + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + return watch, out +} + +func ringBox(t *testing.T, watch *postingsWatch) { + t.Helper() + if err := watch.read(context.Background(), actioncable.Message(`{"change":"upsert","box_id":24088}`)); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func watchLines(t *testing.T, out *bytes.Buffer) []map[string]any { + t.Helper() + var lines []map[string]any + for _, line := range strings.Split(strings.TrimSpace(out.String()), "\n") { + if line == "" { + continue + } + var event map[string]any + if err := json.Unmarshal([]byte(line), &event); err != nil { + t.Fatalf("not JSON: %q", line) + } + lines = append(lines, event) + } + return lines +} + +// A backlog thread and an arrival since the watch began, in one read. +const backlogAndArrival = `{"added":[ + {"id":9001,"kind":"topic","box_id":24088,"name":"Lunch on Thursday?","active_at":"2026-08-21T08:00:00Z","creator":{"name":"Maria Delgado"}}, + {"id":9002,"kind":"topic","box_id":24088,"name":"Invoice #4021","active_at":"2026-08-21T09:00:25Z","creator":{"name":"Northwind Invoicing"}} +]}` + +func TestWatchSaysWhichPostingsAreNew(t *testing.T) { + server := changesServer(t, backlogAndArrival, `{"deleted":[{"id":9003,"deleted_at":"2026-08-21T09:00:40Z"}]}`) + watch, out := watchOf(t, server, "added", "updated", "deleted") + + ringBox(t, watch) + ringBox(t, watch) + + lines := watchLines(t, out) + if len(lines) != 3 { + t.Fatalf("wrote %d lines, want both arrivals and the deletion: %q", len(lines), out.String()) + } + if lines[0]["new"] != false || lines[1]["new"] != true { + t.Errorf("new = %v, %v; want the backlog thread not new and the arrival new", lines[0]["new"], lines[1]["new"]) + } + if _, has := lines[2]["new"]; has { + t.Errorf("a deletion is not mail, so it carries no new: %v", lines[2]) + } +} + +func TestWatchEventsNewReportsNewMailAlone(t *testing.T) { + server := changesServer(t, backlogAndArrival, + `{"updated":[{"id":9002,"kind":"topic","box_id":24088,"name":"Invoice #4021","seen":true,"active_at":"2026-08-21T09:00:25Z","creator":{"name":"Northwind Invoicing"}}]}`, + `{"updated":[{"id":9002,"kind":"topic","box_id":24088,"name":"Invoice #4021","active_at":"2026-08-21T09:01:00Z","creator":{"name":"Northwind Invoicing"}}]}`) + watch, out := watchOf(t, server, "new") + + ringBox(t, watch) // the backlog and the arrival: the arrival alone is new + ringBox(t, watch) // the arrival read: an update, not new + ringBox(t, watch) // a reply on it: new again + + lines := watchLines(t, out) + if len(lines) != 2 { + t.Fatalf("wrote %d lines, want the arrival and the reply: %q", len(lines), out.String()) + } + if lines[0]["change"] != "added" || lines[0]["posting_id"] != float64(9002) || lines[0]["new"] != true { + t.Errorf("first = %v, want the arrival, new", lines[0]) + } + if lines[1]["change"] != "updated" || lines[1]["new"] != true { + t.Errorf("second = %v, want the reply, new", lines[1]) + } +} + +func TestWatchEventsIsAUnion(t *testing.T) { + server := changesServer(t, backlogAndArrival, + `{"updated":[{"id":9001,"kind":"topic","box_id":24088,"name":"Lunch on Thursday?","active_at":"2026-08-21T09:01:00Z","creator":{"name":"Maria Delgado"}}]}`) + watch, out := watchOf(t, server, "deleted", "new") + + ringBox(t, watch) // --events deleted,new: the backlog arrival is neither, the new one is new + ringBox(t, watch) // a reply on the backlog thread: new + + lines := watchLines(t, out) + if len(lines) != 2 || lines[0]["posting_id"] != float64(9002) || lines[1]["posting_id"] != float64(9001) { + t.Errorf("wrote %v, want the new arrival and then the reply, nothing else", lines) + } +} + +func TestWatchRecordsWhatEventsFiltersOut(t *testing.T) { + server := changesServer(t, backlogAndArrival, + `{"updated":[{"id":9002,"kind":"topic","box_id":24088,"name":"Invoice #4021","active_at":"2026-08-21T09:00:25Z","creator":{"name":"Northwind Invoicing"}}]}`) + watch, out := watchOf(t, server, "updated") + + ringBox(t, watch) // --events updated: the arrivals are not reported… + ringBox(t, watch) // …and a later update of one of them is known not to be new + + lines := watchLines(t, out) + if len(lines) != 1 || lines[0]["change"] != "updated" || lines[0]["new"] != false { + t.Errorf("wrote %v, want one update, not new — the arrival was recorded though unreported", lines) + } +} + +func TestWatchExitOnFirstWithEventsNewWaitsForNewMail(t *testing.T) { + server := changesServer(t, backlogAndArrival) + watch, out := watchOf(t, server, "new") + watch.exitOnFirst = true + + ringBox(t, watch) + + lines := watchLines(t, out) + if len(lines) != 1 || lines[0]["posting_id"] != float64(9002) { + t.Errorf("wrote %v, want the first new posting alone — not the backlog thread before it", lines) + } + if !watch.finished() { + t.Error("the watch should be finished on the first new posting") + } +} + +func TestWatchScriptSeesNewMail(t *testing.T) { + server := changesServer(t, backlogAndArrival) + watch, out := watchOf(t, server, "added") + watch.syncScript = `printf "%s new=%s\n" "$HEY_POSTING_ID" "${HEY_NEW:-0}"` + + ringBox(t, watch) + + if got := out.String(); got != "9001 new=0\n9002 new=1\n" { + t.Errorf("scripts saw %q, want HEY_NEW=1 for the arrival alone", got) + } +} + +func TestWatchLineMarksNewMail(t *testing.T) { + isNew, notNew := true, false + box := &watchEventBox{ID: 24088, Kind: "imbox", Name: "Imbox"} + posting := &generated.Posting{Summary: "Lunch on Thursday?", Creator: generated.Contact{Name: "Maria Delgado"}} + + if line := watchLine(watchEvent{Change: "added", At: "2026-08-21T09:00:25.000Z", Box: box, New: &isNew, Posting: posting}); !strings.Contains(line, "added (new)") { + t.Errorf("line = %q, want the change marked new", line) + } + if line := watchLine(watchEvent{Change: "updated", At: "2026-08-21T09:00:25.000Z", Box: box, New: ¬New, Posting: posting}); strings.Contains(line, "(new)") { + t.Errorf("line = %q, want no mark on a posting that is not new", line) + } +} diff --git a/internal/cmd/watch_test.go b/internal/cmd/watch_test.go index 59d89c05..5da2a78a 100644 --- a/internal/cmd/watch_test.go +++ b/internal/cmd/watch_test.go @@ -8,8 +8,10 @@ import ( "net/http" "net/http/httptest" "os" + "slices" "strings" "testing" + "time" actioncable "github.com/basecamp/actioncable-go" @@ -24,8 +26,8 @@ func TestWatchedChanges(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if !changes["added"] || !changes["updated"] || !changes["deleted"] { - t.Errorf("changes = %v, want every change by default", changes) + if !changes["added"] || !changes["updated"] || !changes["deleted"] || !changes["resync"] || changes["new"] { + t.Errorf("changes = %v, want every change and resync by default, and new only when asked", changes) } command.events = []string{"Added", " deleted"} @@ -37,6 +39,15 @@ func TestWatchedChanges(t *testing.T) { t.Errorf("changes = %v, want added and deleted only", changes) } + command.events = []string{"new"} + changes, err = command.watchedChanges() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !changes["new"] || changes["added"] || changes["updated"] || changes["deleted"] || changes["resync"] { + t.Errorf("changes = %v, want new mail alone — not a resync", changes) + } + command.events = []string{"moved"} if _, err := command.watchedChanges(); err == nil { t.Error("expected an error for an unknown event") @@ -135,13 +146,14 @@ func newTestWatch(changes ...string) (*postingsWatch, *bytes.Buffer) { out := &bytes.Buffer{} return &postingsWatch{ - boxes: map[int64]*watchedBox{24088: {id: 24088, kind: "imbox", name: "Imbox"}}, - changes: watched, - out: out, - errOut: &bytes.Buffer{}, - catchUp: make(chan struct{}, 1), - unread: map[int64]bool{}, - running: make(chan struct{}, asyncScriptLimit), + boxes: map[int64]*watchedBox{24088: {id: 24088, kind: "imbox", name: "Imbox", reported: true}}, + changes: watched, + newMail: trackNewMail(watchStarted), + out: out, + errOut: &bytes.Buffer{}, + connection: make(chan struct{}, 1), + unread: map[int64]bool{}, + running: make(chan struct{}, asyncScriptLimit), }, out } @@ -149,7 +161,7 @@ func TestWatchReportsJSONPerPosting(t *testing.T) { watch, out := newTestWatch("added", "updated", "deleted") posting := &generated.Posting{Id: 9001, AppUrl: "https://app.hey.com/topics/5511"} - watch.report(context.Background(), watchEvent{Change: "added", At: "2026-08-18T09:14:22.031Z", PostingID: 9001}, watch.boxes[24088], posting) + watch.report(context.Background(), watchEvent{Change: "added", At: "2026-08-18T09:14:22.031Z", PostingID: 9001, New: watch.classify(watch.boxes[24088], *posting)}, watch.boxes[24088], posting) watch.report(context.Background(), watchEvent{Change: "deleted", At: "2026-08-18T09:15:00.000Z", PostingID: 9003}, watch.boxes[24088], nil) lines := strings.Split(strings.TrimSpace(out.String()), "\n") @@ -170,6 +182,9 @@ func TestWatchReportsJSONPerPosting(t *testing.T) { if added.Posting == nil { t.Error("an added posting should carry the posting itself") } + if added.New == nil || *added.New { + t.Errorf("new = %v, want false for a posting active before the watch began", added.New) + } var deleted watchEvent if err := json.Unmarshal([]byte(lines[1]), &deleted); err != nil { @@ -181,6 +196,9 @@ func TestWatchReportsJSONPerPosting(t *testing.T) { if deleted.ThreadID != 0 { t.Errorf("thread = %d, want none for a deleted posting", deleted.ThreadID) } + if strings.Contains(lines[1], `"new"`) { + t.Errorf("deleted = %s, want no new: a deletion is not mail", lines[1]) + } } func TestWatchSkipsChangesItIsntWatching(t *testing.T) { @@ -243,7 +261,7 @@ func TestWatchEventEnvironment(t *testing.T) { event := watchEvent{ Change: "added", At: "2026-08-18T09:14:22.031Z", - Box: watchEventBox{ID: 24088, Kind: "imbox", Name: "Imbox"}, + Box: &watchEventBox{ID: 24088, Kind: "imbox", Name: "Imbox"}, PostingID: 9001, ThreadID: 5511, } @@ -254,6 +272,15 @@ func TestWatchEventEnvironment(t *testing.T) { t.Errorf("environment = %q, want %s", environment, want) } } + if !strings.Contains(environment, "HEY_NEW=0") { + t.Errorf("environment = %q, want HEY_NEW=0 when the posting is not new", environment) + } + + isNew := true + event.New = &isNew + if environment := strings.Join(event.environment(), "\n"); !strings.Contains(environment, "HEY_NEW=1") { + t.Errorf("environment = %q, want HEY_NEW=1 for new mail", environment) + } } func TestWatchReadsChangesWhenNotified(t *testing.T) { @@ -426,6 +453,192 @@ func TestWatchSkipsAheadToTheBoxesOwnCursor(t *testing.T) { if got := watch.boxes[24088].cursor.Since; got != "2026-08-21T11:02:00.000Z" { t.Errorf("cursor = %q, want the server's current one", got) } + if got := watch.newMail.floors[24088]; !got.Equal(time.Date(2026, 8, 21, 11, 2, 0, 0, time.UTC)) { + t.Errorf("new-mail floor = %v, want the box's floor at the cursor it skipped to", got) + } +} + +func TestWatchReadyWaitsForAFailedCatchUpRead(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + broken := true + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if broken { + http.Error(w, "boom", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Link", `<`+r.URL.Path+`?since=2026-08-18T09%3A14%3A22.031Z&v=2>; rel="next"`) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + watch, out := newTestWatch("added") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-18T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + + // The catch-up's read fails: the box waits for its retry, and so does ready. + if err := watch.catchUp(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Len() != 0 { + t.Fatalf("wrote %q, want no ready while a box is still behind", out.String()) + } + if !watch.catchingUp || !watch.unread[24088] { + t.Fatalf("catchingUp = %v, unread = %v; want the ready owed and the box waiting", watch.catchingUp, watch.unread) + } + + // The retry reads it: now ready. + broken = false + if err := watch.retryUnread(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out.String(), `"change":"ready"`) || watch.catchingUp { + t.Errorf("wrote %q, want ready once the last box is read", out.String()) + } + + // A later retry with nothing owed announces nothing. + out.Reset() + if err := watch.retryUnread(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Len() != 0 { + t.Errorf("wrote %q, want no ready when none is owed", out.String()) + } +} + +func TestWatchScriptDoesNotInheritAnotherEventsVariables(t *testing.T) { + t.Setenv("HEY_NEW", "1") + t.Setenv("HEY_THREAD_ID", "5511") + t.Setenv("HEY_TOKEN", "kept") + watch, out := newTestWatch("deleted") + watch.syncScript = `printf "%s %s %s\n" "${HEY_NEW:-unset}" "${HEY_THREAD_ID:-unset}" "$HEY_TOKEN"` + + watch.report(context.Background(), watchEvent{Change: "deleted", PostingID: 9003}, watch.boxes[24088], nil) + + if got := out.String(); got != "0 unset kept\n" { + t.Errorf("script saw %q, want HEY_NEW=0 and no inherited thread id, and everything else kept", got) + } +} + +func TestWatchReadyYieldsToADropQueuedDuringTheCatchUp(t *testing.T) { + server := changesServer(t, `{}`) + watch, out := newTestWatch("added") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + + // The connection dropped while the catch-up was reading: the drop is + // queued, not yet acted on, and ready must not get ahead of it. + watch.noteConnection(false) + if err := watch.catchUp(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Len() != 0 { + t.Fatalf("wrote %q, want no ready with a drop waiting", out.String()) + } + + // The drop and the reconnect drain in order: disconnected, then the + // reconnect's own catch-up and ready. + watch.noteConnection(true) + if err := watch.followConnection(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + lines := strings.Split(strings.TrimSpace(out.String()), "\n") + if len(lines) != 2 || !strings.Contains(lines[0], `"change":"disconnected"`) || !strings.Contains(lines[1], `"change":"ready"`) { + t.Errorf("wrote %q, want disconnected then ready", lines) + } +} + +func TestWatchDoorbellReadPaysTheReadyACatchUpOwed(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + broken := true + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if broken { + http.Error(w, "boom", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Link", `<`+r.URL.Path+`?since=2026-08-18T09%3A14%3A22.031Z&v=2>; rel="next"`) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + watch, out := newTestWatch("added") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-18T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + if err := watch.catchUp(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // The box rings before its retry comes round, and the read works: ready + // now, not two minutes from now. + broken = false + if err := watch.read(context.Background(), actioncable.Message(`{"change":"upsert","box_id":24088}`)); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out.String(), `"change":"ready"`) || watch.catchingUp { + t.Errorf("wrote %q, want ready once the doorbell read caught the box up", out.String()) + } +} + +func TestWatchDropWhileCatchingUpCancelsTheReadyItOwed(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + broken := true + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if broken { + http.Error(w, "boom", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Link", `<`+r.URL.Path+`?since=2026-08-18T09%3A14%3A22.031Z&v=2>; rel="next"`) + _, _ = w.Write([]byte(`{}`)) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + watch, out := newTestWatch("added") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-18T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + + if err := watch.catchUp(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + // The connection drops before the retry: disconnected, and the ready that + // was owed is not — the reconnect will catch up and announce its own. + watch.noteConnection(false) + if err := watch.followConnection(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + broken = false + if err := watch.retryUnread(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + lines := strings.Split(strings.TrimSpace(out.String()), "\n") + if len(lines) != 1 || !strings.Contains(lines[0], `"change":"disconnected"`) { + t.Errorf("wrote %q, want disconnected alone — no ready while the connection is down", out.String()) + } + + // The reconnect catches up and says ready. + watch.noteConnection(true) + if err := watch.followConnection(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out.String(), `"change":"ready"`) { + t.Errorf("wrote %q, want ready after the reconnect's catch-up", out.String()) + } } func TestWatchGivesUpOnABoxThatHasGone(t *testing.T) { @@ -434,10 +647,10 @@ func TestWatchGivesUpOnABoxThatHasGone(t *testing.T) { defer server.Close() initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) - watch, _ := newTestWatch("added") + watch, out := newTestWatch("added", "resync") errOut := &bytes.Buffer{} watch.errOut = errOut - watch.boxes[31145] = &watchedBox{id: 31145, kind: "papertrail", name: "The Paper Trail"} + watch.boxes[31145] = &watchedBox{id: 31145, kind: "papertrail", name: "The Paper Trail", reported: true} watch.boxes[24088].cursor.Since = "2026-08-01T00:00:00.000Z" if err := watch.readBox(context.Background(), watch.boxes[24088]); err != nil { @@ -450,6 +663,9 @@ func TestWatchGivesUpOnABoxThatHasGone(t *testing.T) { if !strings.Contains(errOut.String(), "can no longer be followed") { t.Errorf("stderr = %q, want the box's departure reported", errOut.String()) } + if out.Len() != 0 { + t.Errorf("wrote %q, want no resync for a box that is gone — there is nothing to re-read", out.String()) + } // Now the last box goes too, and there is nothing left to wait for. watch.boxes[31145].cursor.Since = "2026-08-01T00:00:00.000Z" @@ -467,6 +683,26 @@ func TestWatchGivesUpOnABoxThatHasGone(t *testing.T) { } } +func TestWatchGivesUpWhenTheLastReportedBoxHasGone(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + // The Feed is still listed, but --box imbox only ever reported the Imbox, + // and that one is gone: following The Feed for the record alone is a watch + // that would sit there for good, reporting nothing. + server := boxesAndChanges(t, `[{"id":24089,"kind":"feedbox","name":"The Feed","posting_changes_url":"/boxes/24089/postings/changes.json?since=2026-08-21T11%3A02%3A00.000Z&v=2"}]`) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + watch, _ := newTestWatch("added") + watch.errOut = &bytes.Buffer{} + watch.boxes[24089] = &watchedBox{id: 24089, kind: "feedbox", name: "The Feed", reported: false} + watch.boxes[24088].cursor.Since = "2026-08-01T00:00:00.000Z" + + err := watch.readBox(context.Background(), watch.boxes[24088]) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Errorf("error = %v, want the watch to give up once no reported box is left", err) + } +} + func TestWatchClosedSubscriptionIsOnlyFineWhenItWasInterrupted(t *testing.T) { watch, _ := newTestWatch("added") @@ -519,15 +755,275 @@ func TestWatchRunsBoundedAsyncScripts(t *testing.T) { } } -func TestAskForCatchUpNeverBlocks(t *testing.T) { +func TestConnectionChangesQueueInOrderAndNeverBlock(t *testing.T) { watch, _ := newTestWatch("added") - watch.askForCatchUp() - watch.askForCatchUp() + watch.noteConnection(false) + watch.noteConnection(true) + watch.noteConnection(false) select { - case <-watch.catchUp: + case <-watch.connection: default: - t.Fatal("a catch-up should be waiting") + t.Fatal("a wake-up should be waiting") + } + var transitions []bool + for { + connected, queued := watch.nextTransition() + if !queued { + break + } + transitions = append(transitions, connected) + } + if !slices.Equal(transitions, []bool{false, true, false}) { + t.Errorf("transitions = %v, want every change in the order it happened", transitions) + } +} + +func TestWatchReadyYieldsToADropQueuedBehindTheReconnect(t *testing.T) { + server := changesServer(t, `{}`) + watch, out := newTestWatch("added") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + + // A reconnect and then a drop, both queued before the loop got to them: + // the reconnect's catch-up must see the drop still waiting behind it. + watch.noteConnection(true) + watch.noteConnection(false) + if err := watch.followConnection(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + lines := strings.Split(strings.TrimSpace(out.String()), "\n") + if len(lines) != 1 || !strings.Contains(lines[0], `"change":"disconnected"`) { + t.Errorf("wrote %q, want disconnected alone — no ready with the connection already down", out.String()) + } + + watch.noteConnection(true) + if err := watch.followConnection(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out.String(), `"change":"ready"`) { + t.Errorf("wrote %q, want ready after the reconnect that stuck", out.String()) + } +} + +func TestWatchDoesNotSayReadyOnItsWayOut(t *testing.T) { + server := changesServer(t, `{"added":[{"id":9001,"kind":"topic","box_id":24088,"app_url":"https://app.hey.com/topics/5511"}]}`) + + // The catch-up reported the one change --exit-on-first was waiting for. + watch, out := newTestWatch("added") + watch.exitOnFirst = true + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + if err := watch.catchUp(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out.String(), `"posting_id":9001`) || strings.Contains(out.String(), `"change":"ready"`) { + t.Errorf("wrote %q, want the change and no ready from a watch that is exiting", out.String()) + } + + // The catch-up was interrupted mid-read. + interrupted, out := newTestWatch("added") + interrupted.boxes[24088].cursor = cursor + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if err := interrupted.catchUp(ctx); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out.Len() != 0 { + t.Errorf("wrote %q, want nothing from a catch-up cut short by an interrupt", out.String()) + } +} + +func TestWatchedBoxesStartNoLaterThanTheWatchDid(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + // The Imbox's last activity is after the watch read HEY's clock — mail + // landed in between; The Feed's is before. + _, _ = w.Write([]byte(`[{"id":24088,"kind":"imbox","name":"Imbox","posting_changes_url":"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A30.000Z&v=2"},` + + `{"id":24089,"kind":"feedbox","name":"The Feed","posting_changes_url":"/boxes/24089/postings/changes.json?since=2026-08-21T08%3A00%3A00.000Z&v=2"}]`)) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + command := newWatchCommand() + boxes, err := command.watchedBoxes(context.Background(), watchStarted) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := boxes[24088].cursor.Since; got != "2026-08-21T09:00:00.000Z" { + t.Errorf("Imbox cursor = %q, want it moved back to the watch's start so the mail in between is read", got) + } + if got := boxes[24089].cursor.Since; got != "2026-08-21T08:00:00.000Z" { + t.Errorf("Feed cursor = %q, want the box's own when it is earlier", got) + } + if !boxes[24088].reported || !boxes[24089].reported { + t.Error("without --box every box is reported") + } + + // --box imbox: every box is still followed, the Imbox alone is reported; + // a --box that names nothing is not found. + command.boxes = []string{"imbox"} + boxes, err = command.watchedBoxes(context.Background(), watchStarted) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(boxes) != 2 || !boxes[24088].reported || boxes[24089].reported { + t.Errorf("boxes = %+v, want both followed and the Imbox alone reported", boxes) + } + command.boxes = []string{"trailbox"} + if _, err := command.watchedBoxes(context.Background(), watchStarted); err == nil { + t.Error("expected an error when --box names no box") + } + command.boxes = nil + + // --since is the reader's choice and wins over both. + command.since = "2026-08-21T09:30:00Z" + boxes, err = command.watchedBoxes(context.Background(), watchStarted) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := boxes[24088].cursor.Since; got != "2026-08-21T09:30:00.000Z" { + t.Errorf("cursor = %q, want --since untouched", got) + } +} + +func TestWatchAnnouncesADropBeforeTheReconnectThatFollowedIt(t *testing.T) { + server := changesServer(t, `{}`) + watch, _ := newTestWatch("added") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-21T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + + // The reconnect completed before the loop got round to the drop: the + // reader still has to see them in this order, or it ends up offline. + watch.noteConnection(false) + watch.noteConnection(true) + if err := watch.followConnection(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + lines := strings.Split(strings.TrimSpace(watch.out.(*bytes.Buffer).String()), "\n") + if len(lines) != 2 || !strings.Contains(lines[0], `"change":"disconnected"`) || !strings.Contains(lines[1], `"change":"ready"`) { + t.Errorf("wrote %q, want disconnected then ready", lines) + } +} + +func TestWatchAnnouncesItselfOnStdoutOnly(t *testing.T) { + watch, out := newTestWatch("added") + watch.exitOnFirst = true + + watch.announce(watchReady) + watch.announce(watchDisconnected) + + lines := strings.Split(strings.TrimSpace(out.String()), "\n") + if len(lines) != 2 { + t.Fatalf("wrote %d lines, want ready and disconnected: %q", len(lines), out.String()) + } + var ready map[string]any + if err := json.Unmarshal([]byte(lines[0]), &ready); err != nil { + t.Fatalf("ready isn't JSON: %v", err) + } + if ready["change"] != "ready" || ready["at"] == "" { + t.Errorf("ready = %v, want its change and a time", ready) + } + if _, has := ready["box"]; has { + t.Errorf("ready is about the watch, not a box: %v", ready) + } + if _, has := ready["posting_id"]; has { + t.Errorf("ready has no posting: %v", ready) + } + if !strings.Contains(lines[1], `"change":"disconnected"`) { + t.Errorf("second line = %q, want disconnected", lines[1]) + } + if watch.finished() { + t.Error("the watch's own news never counts towards --exit-on-first") + } + + scripted, scriptOut := newTestWatch("added") + scripted.syncScript = "cat" + scripted.announce(watchReady) + if scriptOut.Len() != 0 { + t.Errorf("a script runs per change, and ready is not one: %q", scriptOut.String()) + } +} + +func TestWatchReportsAResyncWhenAskedFor(t *testing.T) { + watch, out := newTestWatch("deleted", "resync") + watch.exitOnFirst = true + + if !watch.report(context.Background(), watchEvent{Change: watchResync, At: "2026-08-21T09:00:00.000Z"}, watch.boxes[24088], nil) { + t.Fatal("a resync is reported when --events has it") + } + if !strings.Contains(out.String(), `"change":"resync"`) || !strings.Contains(out.String(), `"kind":"imbox"`) { + t.Errorf("wrote %q, want the resync with its box", out.String()) + } + if !watch.finished() { + t.Error("a resync is a change, so --exit-on-first counts it") + } + + // --events new is new mail only: a resync is not, so a script for new + // mail never runs on one and --exit-on-first never exits on one. + newOnly, out := newTestWatch("new") + newOnly.exitOnFirst = true + if newOnly.report(context.Background(), watchEvent{Change: watchResync, At: "2026-08-21T09:00:00.000Z"}, newOnly.boxes[24088], nil) { + t.Error("--events new must leave a resync out") + } + if out.Len() != 0 || newOnly.finished() { + t.Errorf("wrote %q, finished %v; want nothing for a resync under --events new", out.String(), newOnly.finished()) + } +} + +func TestWatchReportsAResyncAfterSkippingAhead(t *testing.T) { + t.Setenv("HEY_TOKEN", "test-token") + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.Path, "/postings/changes") { + // As haystack answers: `head :conflict`, no body. + w.WriteHeader(http.StatusConflict) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`[{"id":24088,"kind":"imbox","name":"Imbox","posting_changes_url":"` + server.URL + `/boxes/24088/postings/changes.json?since=2026-08-21T12%3A00%3A00.000Z&v=2"}]`)) + })) + defer server.Close() + initSDK(auth.NewManager(server.URL, server.Client(), t.TempDir()), server.URL) + + watch, out := newTestWatch("added", "resync") + cursor, err := watchCursor(server.URL+"/boxes/24088/postings/changes.json?since=2026-08-18T09%3A00%3A00.000Z&v=2", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + watch.boxes[24088].cursor = cursor + + if err := watch.read(context.Background(), actioncable.Message(`{"change":"upsert","box_id":24088}`)); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var event watchEvent + if err := json.Unmarshal([]byte(strings.TrimSpace(out.String())), &event); err != nil { + t.Fatalf("output isn't one JSON line: %q (stderr %q)", out.String(), watch.errOut.(*bytes.Buffer).String()) + } + if event.Change != watchResync || event.Box == nil || event.Box.ID != 24088 { + t.Errorf("event = %+v, want a resync for the Imbox", event) + } + if watch.boxes[24088].cursor.Since != "2026-08-21T12:00:00.000Z" { + t.Errorf("cursor = %+v, want it moved to the server's current one", watch.boxes[24088].cursor) + } +} + +func TestWatchLineDescribesTheWatchsOwnNews(t *testing.T) { + line := watchLine(watchEvent{Change: watchReady, At: "2026-08-21T09:00:00.000Z"}) + if !strings.Contains(line, "ready") || !strings.Contains(line, "watching for changes") { + t.Errorf("line = %q, want ready described without a box", line) } } diff --git a/skills/hey/SKILL.md b/skills/hey/SKILL.md index af13f4c9..e1f64789 100644 --- a/skills/hey/SKILL.md +++ b/skills/hey/SKILL.md @@ -509,9 +509,11 @@ Takes box item IDs (the `id` field from `hey box --json`). Ignored threads remai ```bash hey watch # Follow every box until interrupted -hey watch --box imbox # Follow one box (repeatable, by name or ID) -hey watch --events added,deleted # Only these changes (added, updated, deleted) -hey watch --exit-on-first # Wait for one change, print it, exit +hey watch --box imbox # Report one box's changes (repeatable, by name or ID); every box is followed +hey watch --events added,deleted # Only these changes (added, updated, deleted, new, resync) +hey watch --box imbox --events new # New mail only: unseen, unmuted, active since the watch began +hey watch --box imbox --events new --exit-on-first # Block until new mail lands, print it, exit +hey watch --exit-on-first # Wait for one change of any kind, print it, exit hey watch --timeout 30m # Give up waiting after a while hey watch --since 2026-03-15 # Report changes since then first, then follow hey watch --run-sync ./triage.sh # Run a command per change instead of printing @@ -520,14 +522,27 @@ hey watch --run-sync ./triage.sh # Run a command per change instead of printing Long-running, and driven by a websocket rather than polling — never poll `hey box` in a loop when this will do. Writes one JSON object per changed posting to stdout, one per line, instead of the usual envelope: `{"change": "added", "at": ..., "box": {"id", "kind", -"name"}, "posting_id": ..., "thread_id": ..., "posting": {...}}`. Use `thread_id` with -`hey threads`. A deleted posting carries no `posting` or `thread_id`. +"name"}, "posting_id": ..., "thread_id": ..., "new": true|false, "posting": {...}}`. Use +`thread_id` with `hey threads`. `new` is on every `added` and `updated` line and says whether +the posting is new mail — unseen, not muted, and active since the watch last saw the thread, +or since the watch began for a thread it has not seen; the backlog a watch starts with is +never new, nor is reading, muting or moving a thread, and a reply on a known thread is. +`--events new` selects the new ones, alone or in a union with the other three. A deleted +posting carries no `posting`, `thread_id` or `new`. Three more lines +describe the watch itself: `{"change": "ready"}` once every box is caught up and the subscription +is live (again after every reconnect's catch-up), `{"change": "disconnected"}` when the +connection drops, and `{"change": "resync", "box": {...}}` when a box changed more than the +feed can list and the watch skipped ahead — re-read that box. A resync is an event of its +own: reported by default (`--run-*` scripts run for it, `--exit-on-first` counts it) and left +out by `--events new`, so a script for new mail never runs on one; `ready` and `disconnected` +are written to stdout only and carry no `box`. To drive a command per change, choose one of two behaviours — passing both is an error. `--run-async` spawns the command and moves on, so a slow one never holds up the watch and two can overlap; `--run-sync` waits for each and runs them in order. Both get the JSON on stdin and the fields as `HEY_CHANGE`, `HEY_AT`, `HEY_BOX_ID`, `HEY_BOX_KIND`, -`HEY_BOX_NAME`, `HEY_POSTING_ID` and `HEY_THREAD_ID`, and both take over stdout. +`HEY_BOX_NAME`, `HEY_POSTING_ID` and `HEY_THREAD_ID` — plus `HEY_NEW=1` for new mail, `HEY_NEW=0` +otherwise — and both take over stdout. ### Drafts diff --git a/tests/smoke/helpers_test.go b/tests/smoke/helpers_test.go index b7b889b5..455bc278 100644 --- a/tests/smoke/helpers_test.go +++ b/tests/smoke/helpers_test.go @@ -24,6 +24,7 @@ var ( binaryPath string baseURL string configDir string + stateDir string sessionCookie string smokeEmail string ) @@ -77,6 +78,13 @@ func TestMain(m *testing.M) { fmt.Fprintf(os.Stderr, "Failed to create temp config dir: %v\n", err) os.Exit(1) } + // And an isolated state directory, so nothing a command keeps under + // XDG_STATE_HOME leaks into or out of the developer's own. + stateDir, err = os.MkdirTemp("", "hey-smoke-state-*") + if err != nil { + fmt.Fprintf(os.Stderr, "Failed to create temp state dir: %v\n", err) + os.Exit(1) + } // Launch headless Chrome browser and log in to obtain a session cookie. sessionCookie, err = browserLogin(baseURL, smokeEmail, password) if err != nil { @@ -92,6 +100,7 @@ func TestMain(m *testing.M) { code := m.Run() os.RemoveAll(configDir) + os.RemoveAll(stateDir) os.Exit(code) } @@ -177,6 +186,7 @@ func cliEnv() []string { env = append(env, "HEY_BASE_URL="+baseURL, "XDG_CONFIG_HOME="+configDir, + "XDG_STATE_HOME="+stateDir, "HEY_NO_KEYRING=1", "NO_COLOR=1", "TERM=dumb",