From 4fdcc910614dbde9890e248cbd20d47b7436509d Mon Sep 17 00:00:00 2001 From: Ian Gulliver Date: Tue, 20 Oct 2026 21:46:13 -0700 Subject: [PATCH] Audit of cde5089 --- docs/operations.md | 2 +- ...080-normalize-notification-preferences.sql | 20 +++++++++++++++++++ internal/store/notifications.go | 10 +++------- internal/store/store.go | 10 +++------- 4 files changed, 27 insertions(+), 15 deletions(-) create mode 100644 internal/store/migrations/080-normalize-notification-preferences.sql diff --git a/docs/operations.md b/docs/operations.md index 2877694..0498d1e 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -351,7 +351,7 @@ A user who is away has byre answer their agents' questions instead of waiting (` The running macOS app posts notifications for events on its SSE stream, raised by the app itself (`notifyInbox`, `notifyError`, `notifyTurn` and `notifyTask` in `mac/Sources/Byre/Notifications.swift`, through `UNUserNotificationCenter`). With the app not running, nothing notifies; closing its window leaves it running. -- **Preferences.** `users.notification_prefs` holds what notifies the user, returned as `notification_prefs` on `GET /api/me` and set with `PUT /api/me/notifications`: `questions` (`inbox.created`, on by default), `reports` (task turns that end with `summary: true` on `turn.ended`, labelled "Turn summaries"), `ready` (`task.ready`), `discard` (`task.discard_suggested`), `merge_problems` (`task.merge_stopped` and `task.merge_after_review_dropped`), `workflows` (`task.workflow_finished`), `errors` (`session.error`) and `rate_limits` (`session.rate_limited`). A key missing from the stored JSON reads as off. The app checks them against each event as it arrives. +- **Preferences.** The `users.notify_*` boolean columns hold what notifies the user, returned as `notification_prefs` on `GET /api/me` and set with `PUT /api/me/notifications`: `questions` (`inbox.created`, on by default), `reports` (task turns that end with `summary: true` on `turn.ended`, labelled "Turn summaries"), `ready` (`task.ready`), `discard` (`task.discard_suggested`), `merge_problems` (`task.merge_stopped` and `task.merge_after_review_dropped`), `workflows` (`task.workflow_finished`), `errors` (`session.error`) and `rate_limits` (`session.rate_limited`). Migration `080-normalize-notification-preferences` moves existing JSON preferences into those columns and drops the JSON column; a missing key becomes off. New users have only questions enabled. The app checks them against each event as it arrives. - **Content.** The title is `byre ยท ` and the body the first line of the question, turn summary, ready-to-merge overview, discard or merge-problem reason, workflow output, or error (or "Rate limited"), cut at 200 characters. The notification's identifier is the question's ID, `turn-` for a turn summary, `workflow-` for a finished workflow, `error-` for an error or rate limit, and `task-` for ready to merge, suggests discard and merge problems, so each task shows at most one of those three at a time. - **Clearing.** A delivered notification leaves Notification Center once it is dealt with anywhere, on this Mac, another one or by the lead: an `inbox.updated` event whose item is no longer `open` removes the item's notification (`clearInbox`), a `summary.read` event removes that task's `turn-` notifications up to its `seq` (`clearTurns`), and `task.merged`, `task.discarded`, `snapshot.created` (unless its `trigger` is `answer`) and `inbox.created` remove the task's `task-` notification (`clearTask`); the last two are the events that clear the task's ready state on the server. - **Clicking** a notification activates the app, shows its window and points the right pane at the item's task. diff --git a/internal/store/migrations/080-normalize-notification-preferences.sql b/internal/store/migrations/080-normalize-notification-preferences.sql new file mode 100644 index 0000000..1b09246 --- /dev/null +++ b/internal/store/migrations/080-normalize-notification-preferences.sql @@ -0,0 +1,20 @@ +ALTER TABLE users ADD COLUMN notify_questions INTEGER NOT NULL DEFAULT 1; +ALTER TABLE users ADD COLUMN notify_reports INTEGER NOT NULL DEFAULT 0; +ALTER TABLE users ADD COLUMN notify_errors INTEGER NOT NULL DEFAULT 0; +ALTER TABLE users ADD COLUMN notify_rate_limits INTEGER NOT NULL DEFAULT 0; +ALTER TABLE users ADD COLUMN notify_ready INTEGER NOT NULL DEFAULT 0; +ALTER TABLE users ADD COLUMN notify_discard INTEGER NOT NULL DEFAULT 0; +ALTER TABLE users ADD COLUMN notify_merge_problems INTEGER NOT NULL DEFAULT 0; +ALTER TABLE users ADD COLUMN notify_workflows INTEGER NOT NULL DEFAULT 0; + +UPDATE users SET + notify_questions = coalesce(json_extract(notification_prefs, '$.questions'), 0), + notify_reports = coalesce(json_extract(notification_prefs, '$.reports'), 0), + notify_errors = coalesce(json_extract(notification_prefs, '$.errors'), 0), + notify_rate_limits = coalesce(json_extract(notification_prefs, '$.rate_limits'), 0), + notify_ready = coalesce(json_extract(notification_prefs, '$.ready'), 0), + notify_discard = coalesce(json_extract(notification_prefs, '$.discard'), 0), + notify_merge_problems = coalesce(json_extract(notification_prefs, '$.merge_problems'), 0), + notify_workflows = coalesce(json_extract(notification_prefs, '$.workflows'), 0); + +ALTER TABLE users DROP COLUMN notification_prefs; diff --git a/internal/store/notifications.go b/internal/store/notifications.go index 78fd69a..aa47e09 100644 --- a/internal/store/notifications.go +++ b/internal/store/notifications.go @@ -3,18 +3,14 @@ package store import ( "context" "database/sql" - "encoding/json" "g.fc.run/ai/byre/internal/api" ) func (s *Store) UpdateNotificationPrefs(ctx context.Context, login string, prefs api.NotificationPrefs) (api.User, error) { - b, err := json.Marshal(prefs) - if err != nil { - return api.User{}, err - } - err = s.write(ctx, func(tx *sql.Tx, emit emitFunc) error { - _, err := tx.ExecContext(ctx, `UPDATE users SET notification_prefs = ? WHERE login = ?`, string(b), login) + err := s.write(ctx, func(tx *sql.Tx, emit emitFunc) error { + _, err := tx.ExecContext(ctx, `UPDATE users SET notify_questions = ?, notify_reports = ?, notify_errors = ?, notify_rate_limits = ?, notify_ready = ?, notify_discard = ?, notify_merge_problems = ?, notify_workflows = ? WHERE login = ?`, + prefs.Questions, prefs.Reports, prefs.Errors, prefs.RateLimits, prefs.Ready, prefs.Discard, prefs.MergeProblems, prefs.Workflows, login) if err != nil { return err } diff --git a/internal/store/store.go b/internal/store/store.go index 61b7a18..beb998b 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -237,16 +237,12 @@ func (s *Store) EnsureUser(ctx context.Context, login, displayName string) (api. func (s *Store) GetUser(ctx context.Context, login string) (api.User, error) { u := api.User{} - prefs := "" - err := s.db.QueryRowContext(ctx, `SELECT login, display_name, git_name, git_email, notification_prefs, default_model, away FROM users WHERE login = ?`, login). - Scan(&u.Login, &u.DisplayName, &u.GitName, &u.GitEmail, &prefs, &u.DefaultModel, &u.Away) + p := &u.NotificationPrefs + err := s.db.QueryRowContext(ctx, `SELECT login, display_name, git_name, git_email, notify_questions, notify_reports, notify_errors, notify_rate_limits, notify_ready, notify_discard, notify_merge_problems, notify_workflows, default_model, away FROM users WHERE login = ?`, login). + Scan(&u.Login, &u.DisplayName, &u.GitName, &u.GitEmail, &p.Questions, &p.Reports, &p.Errors, &p.RateLimits, &p.Ready, &p.Discard, &p.MergeProblems, &p.Workflows, &u.DefaultModel, &u.Away) if errors.Is(err, sql.ErrNoRows) { return u, ErrNotFound } - if err != nil { - return u, err - } - err = json.Unmarshal([]byte(prefs), &u.NotificationPrefs) return u, err }