Audit of cde5089

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 · <task title>` 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-<seq>` for a turn summary, `workflow-<seq>` for a finished workflow, `error-<seq>` for an error or rate limit, and `task-<task id>` 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-<seq>` 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-<task id>` 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.

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;

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
 		}

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
 }