From 3240645fcd56a114710ec08db2467c6a7479383a Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 23 Oct 2026 20:34:58 -0700 Subject: [PATCH 1/4] fix: native macOS notifications, readable text, one alert per condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Driga.app now posts notifications itself through UserNotifications (purego, still cgo-free), so they show Driga's name and icon and a click jumps to the item. The terminal-notifier and AppleScript paths are gone; AppleScript notifications belonged to Script Editor, which is why Show opened it. Only `driga collect` starts the Cocoa run loop; other commands and hooks are unaffected. Unbundled builds report in doctor that notifications need the app. Machine-health alerts were exempt from the seen-item guard, so an unresolved low-disk episode re-alerted every repeat interval. Health alerts now fire once per episode, coalesce per machine, and re-arm only after recovery (migration 0003). Low disk reads "Low disk space · 12.2 GB free, below 20 GB"; the inbox keeps the detail and the current `driga cleanup list --sizes` hint. Claude-Session: https://claude.ai/code/session_01T2vtcW6ihn1ywy9vxUq9a4 --- cmd/driga/main.go | 2 +- cmd/driga/main_darwin.go | 25 ++ cmd/driga/main_other.go | 5 + docs/machine-health.md | 25 +- docs/usage.md | 33 ++- go.mod | 2 +- internal/attention/alerts_test.go | 78 +----- internal/attention/bytes.go | 19 ++ internal/attention/inbox.go | 105 ++++---- internal/attention/inbox_test.go | 25 -- internal/attention/machine_health_test.go | 20 +- internal/attention/notifications.go | 109 +++++++++ internal/attention/notifications_darwin.go | 230 ++++++++++++++++++ .../attention/notifications_darwin_test.go | 28 +++ internal/attention/notifications_other.go | 11 + internal/attention/notifications_test.go | 200 +++++++++++++++ internal/cli/deps.go | 7 +- internal/cli/doctor.go | 16 +- internal/cli/doctor_test.go | 18 +- internal/cli/stablepath_test.go | 28 --- internal/collector/alerts_candidates_test.go | 62 ++++- internal/collector/cleanup.go | 3 +- internal/collector/machine_health.go | 12 +- internal/collector/machine_health_test.go | 4 +- internal/store/ack.go | 2 +- .../migrations/0003_health_alert_once.sql | 10 + internal/store/queries.go | 13 +- internal/store/schema_test.go | 2 +- 28 files changed, 847 insertions(+), 247 deletions(-) create mode 100644 cmd/driga/main_darwin.go create mode 100644 cmd/driga/main_other.go create mode 100644 internal/attention/bytes.go create mode 100644 internal/attention/notifications.go create mode 100644 internal/attention/notifications_darwin.go create mode 100644 internal/attention/notifications_darwin_test.go create mode 100644 internal/attention/notifications_other.go create mode 100644 internal/attention/notifications_test.go create mode 100644 internal/store/migrations/0003_health_alert_once.sql diff --git a/cmd/driga/main.go b/cmd/driga/main.go index 9ec95f9f..cc378568 100644 --- a/cmd/driga/main.go +++ b/cmd/driga/main.go @@ -8,5 +8,5 @@ import ( ) func main() { - os.Exit(cli.Main(os.Args[1:], version, commit, date)) + notificationMain(func() { os.Exit(cli.Main(os.Args[1:], version, commit, date)) }) } diff --git a/cmd/driga/main_darwin.go b/cmd/driga/main_darwin.go new file mode 100644 index 00000000..7f798057 --- /dev/null +++ b/cmd/driga/main_darwin.go @@ -0,0 +1,25 @@ +//go:build darwin + +package main + +import ( + "os" + "runtime" + + "github.com/labstack/driga/internal/attention" +) + +// Cocoa's application/run loop must own the initial OS thread. Lock in init, +// before Go can move main to a worker thread. CLI work runs on a Go goroutine. +func init() { runtime.LockOSThread() } + +// Only the collector posts notifications. Other commands, including hooks +// that run on every agent event, skip AppKit and release the thread. +func notificationMain(run func()) { + if len(os.Args) < 2 || os.Args[1] != "collect" { + runtime.UnlockOSThread() + run() + return + } + attention.RunNotificationMain(run) +} diff --git a/cmd/driga/main_other.go b/cmd/driga/main_other.go new file mode 100644 index 00000000..84f58e35 --- /dev/null +++ b/cmd/driga/main_other.go @@ -0,0 +1,5 @@ +//go:build !darwin + +package main + +func notificationMain(run func()) { run() } diff --git a/docs/machine-health.md b/docs/machine-health.md index 6ce63864..2d844c18 100644 --- a/docs/machine-health.md +++ b/docs/machine-health.md @@ -251,7 +251,8 @@ does not claim a reclaim size it has not measured. No new cleanup kinds are implemented. `memory_pressure` starts after two consecutive, distinct critical OS samples. -Normal, warning, unknown, and failed collection break the streak. State survives +Normal, warning, unknown, and failed collection break the streak. Unknown or failed +readings do not clear an existing episode. State survives collector restarts. Recovery ends the item while retaining its inbox history. Acknowledging an item suppresses further notifications for that episode. Pressure text leads with the agents' proven share of OS memory in use and MCP @@ -259,17 +260,21 @@ count. With no proven agent memory it says agents hold none and other apps are the cause. Likely memory is never silently included in the proven amount. When MCP pile-ups exist, the pressure text includes a short count and the largest group with its host and restart advice. Active unacknowledged episodes -refresh this evidence without changing their identity, timestamp, or throttle. +refresh this evidence without changing their identity or timestamp. Growth evidence is labelled `Largest observed growth:`; no new growth alert is added. -Both kinds follow `inbox.alerts` (`off` disables them), `alert_repeat`, and the -existing three-second batching window. Repeat throttles are independent per -machine and kind. Unacknowledged active machine warnings can repeat after the -interval; delayed viewer/event passes cannot create extra samples. Health text -survives the titles-only policy, since it contains operational readings rather -than agent transcript text. - -Notification clicks use the existing terminal-notifier `driga inbox jump ` path. +Both kinds follow `inbox.alerts` (`off` disables them). Each condition notifies +once per episode, including across collector restarts and metadata pruning; +only a confirmed recovery re-arms it. `alert_repeat` applies to session alerts. +Health items arriving in one committed batch coalesce per machine, even when +their original timestamps differ. Summaries have no single click destination. +Health text survives the titles-only policy, since it contains operational +readings rather than agent transcript text. Banner text is short; the inbox +keeps the detailed explanation and uses decimal GB/MB for sizes. + +On macOS, Driga.app sends native UserNotifications with Driga's name and icon. +A local single-item notification click runs `driga inbox jump ` through its +delegate while the collector is running. Unbundled builds have no macOS alerts. The stored destination is `machine`. On macOS, the click opens Terminal and runs the installed executable's `top --tab machine`; on Linux, a manual jump uses `x-terminal-emulator -e`. This avoids requiring a running web server. As with diff --git a/docs/usage.md b/docs/usage.md index ed06c6c7..f1aadf24 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -96,12 +96,25 @@ Remote inbox acknowledgments require the peer's `--allow-ack` grant; unreachable show `ack pending` and retry on reconnect. Desktop alerts show the session title or project and why it needs you, within -your capture policy. On macOS, `brew install terminal-notifier` enables clickable -alerts that run `driga inbox jump ` without acknowledging the item; otherwise -alerts use Script Editor via osascript. Jumps select a proven tmux pane, focus a -proven Terminal/iTerm window with `inbox.jump_terminal = true`, or print a resume -command. Batch summaries and remote alerts have no click action. Notifier -detection is cached until collector restart; `driga doctor` reports the backend. +your capture policy. macOS notifications require the installed **Driga.app** +(`brew install --cask driga`); they use Driga's name and icon through the native +UserNotifications API. Allow alerts and sounds for Driga when macOS asks, or in +System Settings → Notifications → Driga. Unbundled builds (`go run`, checkout +binaries) cannot send macOS notifications; `driga doctor` explains how to install +the app. No notification helpers are needed. + +Clicking a local item's notification runs `driga inbox jump ` without +acknowledging it. Jumps select a proven tmux pane, focus a proven Terminal/iTerm +window with `inbox.jump_terminal = true`, or print a resume command. Machine +warnings open Terminal on `driga top --tab machine`. Batch summaries and remote +alerts have no click action. The collector keeps the native notification delegate +and main run loop alive while it runs. Re-sending an item replaces its existing +notification. Linux uses notify-send; Windows uses native WinRT toasts. + +Health warnings notify once per condition, until it clears. Concurrent health +warnings from one machine form one summary. Details and cleanup commands remain +in the inbox; low-disk banners show decimal sizes such as “12.2 GB free, below +20 GB”. `inbox.alert_repeat` throttles session alerts, not health episodes. ## Privacy and configuration @@ -545,13 +558,13 @@ permissions and restart the collector to retry Chat. ## Cleanup -`driga cleanup` lists disk that agents left behind. Nothing is removed +`driga cleanup list` lists disk that agents left behind. Nothing is removed automatically, and nothing is removed on another machine. ```sh -driga cleanup # candidates with evidence -driga cleanup --sizes # walk sizes on demand (30 s cap per walk) -driga cleanup --why # also list worktrees that don't qualify, and why +driga cleanup list # candidates with evidence +driga cleanup list --sizes # walk sizes on demand (30 s cap per walk) +driga cleanup list --why # also list worktrees that don't qualify, and why driga cleanup remove --dry-run # print the exact commands driga cleanup remove # re-verify, then ask y/N driga cleanup remove --kind worktree --all # every ready worktree; skips anything changed diff --git a/go.mod b/go.mod index 1a622ecd..f555f762 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/bmatcuk/doublestar/v4 v4.10.3 github.com/charmbracelet/x/ansi v0.11.9 github.com/charmbracelet/x/term v0.2.2 + github.com/ebitengine/purego v0.11.1 github.com/go-json-experiment/json v0.0.0-20260820222146-c27c302e5fc3 github.com/google/go-cmp v0.7.0 github.com/google/jsonschema-go v0.4.3 @@ -40,7 +41,6 @@ require ( github.com/clipperhouse/displaywidth v0.11.0 // indirect github.com/clipperhouse/uax29/v2 v2.7.0 // indirect github.com/dustin/go-humanize v1.1.0 // indirect - github.com/ebitengine/purego v0.11.1 // indirect github.com/go-ole/go-ole v1.2.6 // indirect github.com/lrstanley/bubblezone/v2 v2.0.0 // indirect github.com/lucasb-eyer/go-colorful v1.4.1 // indirect diff --git a/internal/attention/alerts_test.go b/internal/attention/alerts_test.go index 952c4212..ee7b5dee 100644 --- a/internal/attention/alerts_test.go +++ b/internal/attention/alerts_test.go @@ -2,11 +2,7 @@ package attention import ( "context" - "os" - "os/exec" - "path/filepath" "reflect" - "runtime" "strings" "testing" "time" @@ -15,78 +11,6 @@ import ( "github.com/labstack/driga/internal/model" ) -func TestClickableNotifierArguments(t *testing.T) { - for _, found := range []bool{false, true} { - t.Run(map[bool]string{false: "fallback", true: "clickable"}[found], func(t *testing.T) { - var calls [][]string - r := runnerFunc(func(_ context.Context, name string, args ...string) ([]byte, error) { - calls = append(calls, append([]string{name}, args...)) - return nil, nil - }) - lookups := 0 - lookup := cachedNotifierLookup(func(name string) (string, error) { - lookups++ - if name != "terminal-notifier" { - t.Fatal(name) - } - if !found { - return "", exec.ErrNotFound - } - return "/opt/tool bin/terminal-notifier", nil - }) - d := DesktopNotifier{Runner: r, OS: "darwin", DrigaPath: "/opt/Driga's bin/driga", Lookup: lookup} - n := Notification{Kind: "waiting", ItemID: "item '$(bad) with spaces", Title: "Needs you", Body: `driga · permission (Bash) "quoted"`, Sound: "Glass"} - if err := d.Notify(context.Background(), n); err != nil { - t.Fatal(err) - } - want := []string{"osascript", "-e", "on run argv\ndisplay notification (item 1 of argv) with title (item 2 of argv) sound name (item 3 of argv)\nend run", n.Body, n.Title, n.Sound} - if found { - want = []string{"/opt/tool bin/terminal-notifier", "-title", "driga", "-subtitle", n.Title, "-message", n.Body, "-group", "driga-" + n.ItemID, "-sound", "Glass", "-execute", `'/opt/Driga'"'"'s bin/driga' inbox jump 'item '"'"'$(bad) with spaces'`} - } - if !reflect.DeepEqual(calls, [][]string{want}) { - t.Fatalf("got %#v; want %#v", calls, want) - } - n.ItemID, n.Body = "", "3 sessions waiting" - if err := d.Notify(context.Background(), n); err != nil { - t.Fatal(err) - } - if lookups != 1 { - t.Fatalf("lookups = %d", lookups) - } - if found { - want = []string{"/opt/tool bin/terminal-notifier", "-title", "driga", "-subtitle", n.Title, "-message", n.Body, "-group", "driga-waiting", "-sound", "Glass"} - if !reflect.DeepEqual(calls[1], want) { - t.Fatal(calls[1]) - } - } - }) - } -} - -func TestNotifierLookupUsesInheritedPATH(t *testing.T) { - dir := t.TempDir() - name := "terminal-notifier" - if runtime.GOOS == "windows" { - name += ".exe" - t.Setenv("PATHEXT", ".EXE") - } - path := filepath.Join(dir, name) - if err := os.WriteFile(path, []byte("unused fixture"), 0700); err != nil { - t.Fatal(err) - } - t.Setenv("PATH", dir) - lookup := cachedNotifierLookup(exec.LookPath) - if got := lookup(); got != path { - t.Fatalf("lookup = %q", got) - } - if err := os.Remove(path); err != nil { - t.Fatal(err) - } - if got := lookup(); got != path { - t.Fatalf("lookup not cached: %q", got) - } -} - func TestLinuxNotifierExactArguments(t *testing.T) { for _, kind := range []string{"waiting", "finished"} { sound := "message-new-instant" @@ -100,7 +24,7 @@ func TestLinuxNotifierExactArguments(t *testing.T) { } return nil, nil }) - d := DesktopNotifier{Runner: r, OS: "linux", Lookup: func() string { t.Fatal("Linux lookup"); return "" }} + d := DesktopNotifier{Runner: r, OS: "linux"} if err := d.Notify(context.Background(), Notification{Kind: kind, Title: "title", Body: "body"}); err != nil { t.Fatal(err) } diff --git a/internal/attention/bytes.go b/internal/attention/bytes.go new file mode 100644 index 00000000..d408b269 --- /dev/null +++ b/internal/attention/bytes.go @@ -0,0 +1,19 @@ +package attention + +import ( + "fmt" + "strings" +) + +// DecimalBytes matches the decimal disk thresholds documented in the config. +func DecimalBytes(n uint64) string { + for _, unit := range []struct { + size uint64 + name string + }{{1_000_000_000_000, "TB"}, {1_000_000_000, "GB"}, {1_000_000, "MB"}, {1_000, "kB"}} { + if n >= unit.size { + return strings.TrimSuffix(fmt.Sprintf("%.1f", float64(n)/float64(unit.size)), ".0") + " " + unit.name + } + } + return fmt.Sprintf("%d B", n) +} diff --git a/internal/attention/inbox.go b/internal/attention/inbox.go index 9dcd8acc..82d9c016 100644 --- a/internal/attention/inbox.go +++ b/internal/attention/inbox.go @@ -13,7 +13,6 @@ import ( "sort" "strconv" "strings" - "sync" "time" "github.com/labstack/driga/internal/config" @@ -275,27 +274,10 @@ type Notifier interface { Notify(context.Context, Notification) error } type DesktopNotifier struct { - Toast func(context.Context, string) error // Native Windows backend; fixtures inject a recorder. - Runner Runner - OS string - DrigaPath string - Lookup func() string // Optional cached lookup override for fixtures. -} - -// TerminalNotifierPath uses the inherited PATH, including the service's saved -// installing-shell PATH. Cache both presence and absence for the process lifetime. -func TerminalNotifierPath() string { return terminalNotifierPath() } - -var terminalNotifierPath = cachedNotifierLookup(exec.LookPath) - -func cachedNotifierLookup(lookup func(string) (string, error)) func() string { - return sync.OnceValue(func() string { - path, err := lookup("terminal-notifier") - if err != nil { - return "" - } - return path - }) + Toast func(context.Context, string) error // Native Windows backend; fixtures inject a recorder. + Mac Notifier // Native macOS backend; fixtures inject a fake. + Runner Runner + OS string } func (d DesktopNotifier) Notify(ctx context.Context, n Notification) error { @@ -309,36 +291,23 @@ func (d DesktopNotifier) Notify(ctx context.Context, n Notification) error { } return d.Toast(ctx, payload) } - if d.Runner == nil { - return nil - } goos := d.OS if goos == "" { goos = runtime.GOOS } + if goos == "darwin" { + if d.Mac == nil { + return fmt.Errorf("notifications unavailable: install Driga.app for native macOS notifications") + } + return d.Mac.Notify(ctx, n) + } + if d.Runner == nil { + return nil + } ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() var err error switch goos { - case "darwin": - lookup := d.Lookup - if lookup == nil { - lookup = TerminalNotifierPath - } - if path := lookup(); path != "" { - group := n.Kind - if n.ItemID != "" { - group = n.ItemID - } - args := []string{"-title", "driga", "-subtitle", n.Title, "-message", n.Body, "-group", "driga-" + group, "-sound", n.Sound} - // Summaries have no single destination. Remote items also omit ItemID. - if n.ItemID != "" && d.DrigaPath != "" { - args = append(args, "-execute", Quote(d.DrigaPath)+" inbox jump "+Quote(n.ItemID)) - } - _, err = d.Runner.Run(ctx, path, args...) - break - } - _, err = d.Runner.Run(ctx, "osascript", "-e", "on run argv\ndisplay notification (item 1 of argv) with title (item 2 of argv) sound name (item 3 of argv)\nend run", n.Body, n.Title, n.Sound) case "linux": sound := "message-new-instant" if n.Kind == "finished" { @@ -350,7 +319,8 @@ func (d DesktopNotifier) Notify(ctx context.Context, n Notification) error { } // PlanAlerts marks suppressed items too: changing detail must not re-alert a wait. -// Consecutive starts within three seconds in a committed batch form one summary. +// Waiting starts within three seconds form a summary. Health items in a committed +// batch coalesce per machine, regardless of the original episode timestamps. func PlanAlerts(items []model.InboxItem, sessions []model.Session, c config.InboxConfig, policy string, now time.Time, seen, last map[string]time.Time) []Notification { type sessionKey struct { machine string @@ -398,19 +368,30 @@ func PlanAlerts(items []model.InboxItem, sessions []model.Session, c config.Inbo sort.SliceStable(items, func(i, j int) bool { return items[i].TS.Before(items[j].TS) }) var waiting []model.InboxItem var notes []Notification - var health []model.InboxItem - flushHealth := func() { + healthByMachine := map[string][]model.InboxItem{} + var healthOrder []string + flushHealth := func(health []model.InboxItem) { if len(health) == 0 { return } n := Notification{Kind: health[0].Kind, Title: "Machine needs attention", Sound: "Glass", ItemID: health[0].ID, MachineID: health[0].MachineID} bodies := []string{} for _, i := range health { - bodies = append(bodies, scrub.Scrub(i.Summary)) + bodies = append(bodies, healthNotificationBody(i)) + } + if len(health) == 1 { + if health[0].Kind == "low_disk" { + n.Title = "Low disk space" + } else { + n.Title = "Critical memory pressure" + } + } else { + // A summary has no single destination. + n.Kind = "machine_health" + n.ItemID = "" } n.Body = strings.Join(bodies, " · ") notes = append(notes, n) - health = nil } flush := func() { if len(waiting) == 0 { @@ -440,7 +421,7 @@ func PlanAlerts(items []model.InboxItem, sessions []model.Session, c config.Inbo } for _, i := range items { key := i.MachineID + ":" + i.ID - if _, ok := seen[key]; ok && !MachineWarning(i.Kind) { + if _, ok := seen[key]; ok { continue } // alert_after delays a wait's alert until it has lasted that long, so a @@ -457,15 +438,15 @@ func PlanAlerts(items []model.InboxItem, sessions []model.Session, c config.Inbo continue } sk := AlertRepeatKey(i) - if prev, ok := last[sk]; ok && now.Sub(prev) < c.AlertRepeat { + if prev, ok := last[sk]; ok && !MachineWarning(i.Kind) && now.Sub(prev) < c.AlertRepeat { continue } last[sk] = now if MachineWarning(i.Kind) { - if len(health) > 0 && (i.MachineID != health[0].MachineID || i.TS.Sub(health[0].TS) > 3*time.Second) { - flushHealth() + if _, ok := healthByMachine[i.MachineID]; !ok { + healthOrder = append(healthOrder, i.MachineID) } - health = append(health, i) + healthByMachine[i.MachineID] = append(healthByMachine[i.MachineID], i) continue } if i.Kind == "finished" { @@ -478,7 +459,9 @@ func PlanAlerts(items []model.InboxItem, sessions []model.Session, c config.Inbo waiting = append(waiting, i) } flush() - flushHealth() + for _, machine := range healthOrder { + flushHealth(healthByMachine[machine]) + } return notes } @@ -492,3 +475,15 @@ func AlertRepeatKey(i model.InboxItem) string { } return i.MachineID + ":" + strconv.FormatInt(i.SessionID, 10) } + +// The inbox keeps the explanation and commands; a banner gets only the condition. +func healthNotificationBody(i model.InboxItem) string { + text := strings.Join(strings.Fields(scrub.Scrub(i.Summary)), " ") + if i.Kind == "memory_pressure" { + return "Critical memory pressure; open Machine to inspect apps" + } + if first, _, ok := strings.Cut(text, ". "); ok { + text = first + } + return strings.TrimSuffix(text, ".") +} diff --git a/internal/attention/inbox_test.go b/internal/attention/inbox_test.go index 13918759..308680cc 100644 --- a/internal/attention/inbox_test.go +++ b/internal/attention/inbox_test.go @@ -81,31 +81,6 @@ func TestTerminalJumpNeedsProof(t *testing.T) { } } -func TestNotifierUsesArgvAndDistinctLinuxSounds(t *testing.T) { - for _, goos := range []string{"darwin", "linux"} { - var calls [][]string - r := runnerFunc(func(_ context.Context, n string, a ...string) ([]byte, error) { - calls = append(calls, append([]string{n}, a...)) - return nil, nil - }) - d := DesktopNotifier{Runner: r, OS: goos, Lookup: func() string { return "" }} - for _, n := range []Notification{{Kind: "waiting", Title: "Needs input", Body: `quoted " message`, Sound: "Glass"}, {Kind: "finished", Title: "Done", Body: "done", Sound: "Pop"}} { - if err := d.Notify(context.Background(), n); err != nil { - t.Fatal(err) - } - } - if len(calls) != 2 { - t.Fatal(calls) - } - if goos == "linux" && (!strings.Contains(strings.Join(calls[0], " "), "sound-name:") || strings.Join(calls[0], " ") == strings.Join(calls[1], " ")) { - t.Fatal("missing distinct sound hints", calls) - } - if goos == "darwin" && strings.Contains(calls[0][2], `quoted " message`) { - t.Fatal("body interpolated into AppleScript") - } - } -} - func TestAlertPolicyThrottleSummaryAndKinds(t *testing.T) { at := time.Unix(100, 0).UTC() cfg := config.Default() diff --git a/internal/attention/machine_health_test.go b/internal/attention/machine_health_test.go index 9cb52236..4fab018f 100644 --- a/internal/attention/machine_health_test.go +++ b/internal/attention/machine_health_test.go @@ -16,13 +16,13 @@ func TestMachineAlertRepeatAndBatch(t *testing.T) { seen, last := map[string]time.Time{}, map[string]time.Time{} items := []model.InboxItem{{ID: "disk:1", MachineID: "local", Kind: "low_disk", TS: at, Summary: "Low disk; inspect known cleanup candidates"}, {ID: "pressure:1", MachineID: "local", Kind: "memory_pressure", TS: at, Summary: "Critical memory; inspect largest apps"}} ns := PlanAlerts(items, nil, c, "titles", at, seen, last) - if len(ns) != 1 || !strings.Contains(ns[0].Body, "largest apps") || !strings.Contains(ns[0].Body, "cleanup") || ns[0].ItemID == "" { + if len(ns) != 1 || !strings.Contains(ns[0].Body, "Machine") || !strings.Contains(ns[0].Body, "cleanup") || ns[0].ItemID != "" { t.Fatal(ns) } if ns = PlanAlerts(items, nil, c, "titles", at.Add(time.Minute), seen, last); len(ns) != 0 { t.Fatal(ns) } - if ns = PlanAlerts(items, nil, c, "titles", at.Add(c.AlertRepeat), seen, last); len(ns) != 1 { + if ns = PlanAlerts(items, nil, c, "titles", at.Add(c.AlertRepeat), seen, last); len(ns) != 0 { t.Fatal(ns) } c.Alerts = "off" @@ -42,3 +42,19 @@ func TestMachineJumpPlansSafeTerminal(t *testing.T) { t.Fatalf("%q %v", cmds, err) } } + +func TestHealthBatchCoalescesPerMachineAcrossOldTimestamps(t *testing.T) { + at := time.Unix(100, 0) + items := []model.InboxItem{ + {ID: "disk", MachineID: "local", Kind: "low_disk", TS: at, Summary: "12.2 GB free, below 20 GB. Run driga cleanup list --sizes."}, + {ID: "remote", MachineID: "peer", Kind: "low_disk", TS: at.Add(time.Minute), Summary: "1 GB free, below 20 GB"}, + {ID: "memory", MachineID: "local", Kind: "memory_pressure", TS: at.Add(time.Hour), Summary: "Critical memory pressure. Long\nexplanation"}, + } + notes := PlanAlerts(items, nil, config.Default().Inbox, "titles", at.Add(time.Hour), map[string]time.Time{}, map[string]time.Time{}) + if len(notes) != 2 || notes[0].MachineID != "local" || notes[0].ItemID != "" || notes[0].Title != "Machine needs attention" { + t.Fatal(notes) + } + if strings.Contains(notes[0].Body, "driga cleanup") || strings.Contains(notes[0].Body, "\n") || !strings.Contains(notes[0].Body, "12.2 GB") || !strings.Contains(notes[0].Body, "memory pressure") { + t.Fatal(notes) + } +} diff --git a/internal/attention/notifications.go b/internal/attention/notifications.go new file mode 100644 index 00000000..7c4a55ab --- /dev/null +++ b/internal/attention/notifications.go @@ -0,0 +1,109 @@ +package attention + +import ( + "context" + "crypto/sha256" + "fmt" + "path/filepath" + "sync" + "time" +) + +// NativeNotificationRequest is the value-only boundary to UserNotifications. +// No Objective-C objects escape the Darwin implementation. +type NativeNotificationRequest struct { + Identifier, ThreadIdentifier string + Title, Body, Sound string + ItemID, MachineID string +} + +type NativeNotificationCenter interface { + Authorize(func(error)) + Add(context.Context, NativeNotificationRequest) error +} + +// MacNotifier requests permission once per process. macOS remembers the user's +// decision between launches. Authorization can outlive a cancelled Notify call. +type MacNotifier struct { + Center NativeNotificationCenter + once sync.Once + authorized chan struct{} + authErr error +} + +func (m *MacNotifier) Notify(ctx context.Context, n Notification) error { + if m.Center == nil { + return fmt.Errorf("notifications unavailable: install Driga.app for native macOS notifications") + } + if err := ctx.Err(); err != nil { + return err + } + m.once.Do(func() { + m.authorized = make(chan struct{}) + m.Center.Authorize(func(err error) { + m.authErr = err + close(m.authorized) + }) + }) + select { + case <-m.authorized: + return m.post(ctx, n) + default: + // Keep the first alert queued while the user considers permission, + // without stalling collection or imposing a timeout on their answer. + go func() { + select { + case <-ctx.Done(): + case <-m.authorized: + _ = m.post(ctx, n) + } + }() + return nil + } +} + +func (m *MacNotifier) post(ctx context.Context, n Notification) error { + if err := ctx.Err(); err != nil { + return err + } + if m.authErr != nil { + return m.authErr + } + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + return m.Center.Add(ctx, nativeNotificationRequest(n)) +} + +func nativeNotificationRequest(n Notification) NativeNotificationRequest { + key := "item:" + n.ItemID + if n.ItemID == "" { + key = "summary:" + n.Kind + } + // Machine identity prevents local/remote ID collisions; hashing also avoids + // ambiguous delimiters and puts no private item text in the identifier. + id := fmt.Sprintf("driga-%x", sha256.Sum256([]byte(n.MachineID+"\x00"+key))) + return NativeNotificationRequest{Identifier: id, ThreadIdentifier: id, + Title: n.Title, Body: n.Body, Sound: n.Sound, ItemID: n.ItemID, MachineID: n.MachineID} +} + +const notificationDefaultAction = "com.apple.UNNotificationDefaultActionIdentifier" + +// Notification clicks use argv, never a shell. Remote items and summaries have +// no ItemID, as enforced by the collector before handing them to the backend. +func notificationResponse(ctx context.Context, r Runner, path, action, itemID string) error { + if action != notificationDefaultAction || itemID == "" || path == "" || r == nil { + return nil + } + _, err := r.Run(ctx, path, "inbox", "jump", itemID) + return err +} + +func drigaAppExecutable(path string) bool { + resolved, err := filepath.EvalSymlinks(path) + if err != nil { + return false + } + return filepath.Base(resolved) == "driga" && filepath.Base(filepath.Dir(resolved)) == "MacOS" && + filepath.Base(filepath.Dir(filepath.Dir(resolved))) == "Contents" && + filepath.Base(filepath.Dir(filepath.Dir(filepath.Dir(resolved)))) == "Driga.app" +} diff --git a/internal/attention/notifications_darwin.go b/internal/attention/notifications_darwin.go new file mode 100644 index 00000000..4e287577 --- /dev/null +++ b/internal/attention/notifications_darwin.go @@ -0,0 +1,230 @@ +//go:build darwin + +package attention + +import ( + "context" + "fmt" + "os" + "structs" + "time" + "unsafe" + + "github.com/ebitengine/purego" + "github.com/ebitengine/purego/objc" +) + +// Published before the CLI goroutine starts; never mutated afterwards. Tests +// inject NativeNotificationCenter instead of initializing this native backend. +var macNotifier Notifier = &MacNotifier{} +var macStartupErr error + +func MacDesktopNotifier() Notifier { return macNotifier } + +func nsClass(name string) objc.ID { return objc.ID(objc.GetClass(name)) } +func nsString(s string) objc.ID { + return nsClass("NSString").Send(objc.RegisterName("stringWithUTF8String:"), s) +} +func goString(s objc.ID) string { + if s == 0 { + return "" + } + return objc.Send[string](s, objc.RegisterName("UTF8String")) +} +func autoreleasePool() func() { + pool := nsClass("NSAutoreleasePool").Send(objc.RegisterName("new")) + return func() { pool.Send(objc.RegisterName("drain")) } +} + +func MacNotificationStatus() (string, error) { + exe, err := os.Executable() + if err != nil { + return "", err + } + if !drigaAppExecutable(exe) { + return "", fmt.Errorf("install Driga.app for native macOS notifications (unbundled builds cannot send them)") + } + if macStartupErr != nil { + return "", macStartupErr + } + if _, err := purego.Dlopen("/System/Library/Frameworks/Foundation.framework/Foundation", purego.RTLD_GLOBAL|purego.RTLD_NOW); err != nil { + return "", err + } + defer autoreleasePool()() + bundle := nsClass("NSBundle").Send(objc.RegisterName("mainBundle")) + if goString(bundle.Send(objc.RegisterName("bundleIdentifier"))) != "run.driga" { + return "", fmt.Errorf("Driga.app must have the run.driga bundle identity; reinstall the app") + } + return "native UserNotifications (Driga); delivery depends on macOS notification settings", nil +} + +// RunNotificationMain is called only by the executable's locked initial thread. +// Keep NSApplication's main run loop alive for the entire daemon/viewer lifetime: +// locking an arbitrary worker thread is insufficient for Cocoa/dispatch callbacks. +// run owns process exit, just as the ordinary CLI main does. +func RunNotificationMain(run func()) { + exe, _ := os.Executable() + if !drigaAppExecutable(exe) { + run() + return + } + app, center, err := startMacNotifications(exe) + if err != nil { + macStartupErr = err + run() + return + } + macNotifier = &MacNotifier{Center: center} + go run() + defer autoreleasePool()() + app.Send(objc.RegisterName("run")) +} + +type macNotificationCenter struct { + center, delegate objc.ID // UNUserNotificationCenter.delegate is weak: retain it ourselves. + queue uintptr + dispatchAsync func(uintptr, objc.Block) +} + +func startMacNotifications(exe string) (objc.ID, *macNotificationCenter, error) { + for _, framework := range []string{"AppKit", "UserNotifications"} { + if _, err := purego.Dlopen("/System/Library/Frameworks/"+framework+".framework/"+framework, purego.RTLD_GLOBAL|purego.RTLD_NOW); err != nil { + return 0, nil, err + } + } + defer autoreleasePool()() + if _, err := MacNotificationStatus(); err != nil { + return 0, nil, err + } + app := nsClass("NSApplication").Send(objc.RegisterName("sharedApplication")) + // Accessory app: no Dock icon or window, while retaining Driga's bundle identity. + app.Send(objc.RegisterName("setActivationPolicy:"), int64(1)) + c := &macNotificationCenter{center: nsClass("UNUserNotificationCenter").Send(objc.RegisterName("currentNotificationCenter"))} + var err error + c.queue, err = purego.Dlsym(purego.RTLD_DEFAULT, "_dispatch_main_q") + if err != nil { + return 0, nil, err + } + purego.RegisterLibFunc(&c.dispatchAsync, purego.RTLD_DEFAULT, "dispatch_async") + delegate, err := objc.RegisterClass("DrigaNotificationDelegate", objc.GetClass("NSObject"), + []*objc.Protocol{objc.GetProtocol("UNUserNotificationCenterDelegate")}, nil, []objc.MethodDef{ + {Cmd: objc.RegisterName("userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler:"), + Fn: func(_ objc.ID, _ objc.SEL, _ objc.ID, response objc.ID, completion objc.Block) { + defer autoreleasePool()() + // Copy all values before returning to Cocoa; no borrowed objects + // cross the asynchronous jump boundary. Always complete, even for dismissals. + action := goString(response.Send(objc.RegisterName("actionIdentifier"))) + info := response.Send(objc.RegisterName("notification")).Send(objc.RegisterName("request")).Send(objc.RegisterName("content")).Send(objc.RegisterName("userInfo")) + item := goString(info.Send(objc.RegisterName("objectForKey:"), nsString("itemID"))) + completeNativeBlock(completion) + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + _ = notificationResponse(ctx, CommandRunner{}, exe, action, item) + }() + }}, + {Cmd: objc.RegisterName("userNotificationCenter:willPresentNotification:withCompletionHandler:"), + Fn: func(_ objc.ID, _ objc.SEL, _ objc.ID, _ objc.ID, completion objc.Block) { + // Sound + banner + Notification Center list, even in the foreground. + presentNativeBlock(completion, 1<<1|1<<3|1<<4) + }}, + }) + if err != nil { + return 0, nil, err + } + c.delegate = objc.ID(delegate).Send(objc.RegisterName("new")) + c.center.Send(objc.RegisterName("setDelegate:"), c.delegate) + // NSApplication.run finishes launching after the delegate is installed. + return app, c, nil +} + +func (c *macNotificationCenter) onMain(fn func()) { + block := objc.NewBlock(func(_ objc.Block) { + defer autoreleasePool()() + fn() + }) + c.dispatchAsync(c.queue, block) // dispatch copies the block until execution. + block.Release() +} + +func nativeNotificationError(err objc.ID) error { + if err == 0 { + return nil + } + return fmt.Errorf("native macOS notifications: %s", goString(err.Send(objc.RegisterName("localizedDescription")))) +} + +func (c *macNotificationCenter) Authorize(complete func(error)) { + c.onMain(func() { + block := objc.NewBlock(func(_ objc.Block, granted bool, err objc.ID) { + defer autoreleasePool()() + e := nativeNotificationError(err) + if e == nil && !granted { + e = fmt.Errorf("notifications unavailable: allow Driga in System Settings > Notifications") + } + complete(e) + }) + defer block.Release() + c.center.Send(objc.RegisterName("requestAuthorizationWithOptions:completionHandler:"), uintptr(1<<1|1<<2), block) + }) +} + +func (c *macNotificationCenter) Add(ctx context.Context, n NativeNotificationRequest) error { + result := make(chan error, 1) + c.onMain(func() { + if err := ctx.Err(); err != nil { + result <- err + return + } + content := nsClass("UNMutableNotificationContent").Send(objc.RegisterName("new")) + defer content.Send(objc.RegisterName("release")) + content.Send(objc.RegisterName("setTitle:"), nsString(n.Title)) + content.Send(objc.RegisterName("setBody:"), nsString(n.Body)) + content.Send(objc.RegisterName("setThreadIdentifier:"), nsString(n.ThreadIdentifier)) + if n.Sound != "" { + content.Send(objc.RegisterName("setSound:"), nsClass("UNNotificationSound").Send(objc.RegisterName("defaultSound"))) + } + info := nsClass("NSMutableDictionary").Send(objc.RegisterName("dictionary")) + info.Send(objc.RegisterName("setObject:forKey:"), nsString(n.ItemID), nsString("itemID")) + info.Send(objc.RegisterName("setObject:forKey:"), nsString(n.MachineID), nsString("machineID")) + content.Send(objc.RegisterName("setUserInfo:"), info) + request := nsClass("UNNotificationRequest").Send(objc.RegisterName("requestWithIdentifier:content:trigger:"), nsString(n.Identifier), content, objc.ID(0)) + block := objc.NewBlock(func(_ objc.Block, err objc.ID) { + defer autoreleasePool()() + result <- nativeNotificationError(err) + }) + defer block.Release() + // The same identifier replaces pending AND delivered notifications. + c.center.Send(objc.RegisterName("addNotificationRequest:withCompletionHandler:"), request, block) + }) + select { + case <-ctx.Done(): + return ctx.Err() + case err := <-result: + return err + } +} + +// objc.Block.Invoke dispatches Go-created blocks only. Delegate completion +// handlers belong to Cocoa; call their ABI invoke pointer instead. +type nativeBlockHeader struct { + _ structs.HostLayout + isa uintptr + flags, reserved uint32 + invoke uintptr +} + +func nativeBlockInvoke(block objc.Block) uintptr { + ptr := *(*unsafe.Pointer)(unsafe.Pointer(&block)) + return (*nativeBlockHeader)(ptr).invoke +} +func completeNativeBlock(block objc.Block) { + var call func(objc.Block) + purego.RegisterFunc(&call, nativeBlockInvoke(block)) + call(block) +} +func presentNativeBlock(block objc.Block, options uintptr) { + var call func(objc.Block, uintptr) + purego.RegisterFunc(&call, nativeBlockInvoke(block)) + call(block, options) +} diff --git a/internal/attention/notifications_darwin_test.go b/internal/attention/notifications_darwin_test.go new file mode 100644 index 00000000..c31d6777 --- /dev/null +++ b/internal/attention/notifications_darwin_test.go @@ -0,0 +1,28 @@ +//go:build darwin + +package attention + +import ( + "testing" + + "github.com/ebitengine/purego/objc" +) + +// Exercises the completion-handler ABI without loading UserNotifications, +// requesting authorization, or posting any real desktop notification. +func TestNativeCompletionBlockABI(t *testing.T) { + completed := false + done := objc.NewBlock(func(_ objc.Block) { completed = true }) + defer done.Release() + completeNativeBlock(done) + if !completed { + t.Fatal("completion not invoked") + } + var options uintptr + present := objc.NewBlock(func(_ objc.Block, value uintptr) { options = value }) + defer present.Release() + presentNativeBlock(present, 26) + if options != 26 { + t.Fatal(options) + } +} diff --git a/internal/attention/notifications_other.go b/internal/attention/notifications_other.go new file mode 100644 index 00000000..a4823fdf --- /dev/null +++ b/internal/attention/notifications_other.go @@ -0,0 +1,11 @@ +//go:build !darwin + +package attention + +import "fmt" + +func MacNotificationStatus() (string, error) { + return "", fmt.Errorf("native macOS notifications require Driga.app on macOS") +} + +func MacDesktopNotifier() Notifier { return &MacNotifier{} } diff --git a/internal/attention/notifications_test.go b/internal/attention/notifications_test.go new file mode 100644 index 00000000..b5b3cbf1 --- /dev/null +++ b/internal/attention/notifications_test.go @@ -0,0 +1,200 @@ +package attention + +import ( + "context" + "errors" + "os" + "path/filepath" + "reflect" + "sync" + "testing" + "time" +) + +type fakeNotificationCenter struct { + mu sync.Mutex + requests []NativeNotificationRequest + authorizations int + authErr, addErr error + gate <-chan struct{} + delivered chan struct{} +} + +func (f *fakeNotificationCenter) Authorize(complete func(error)) { + finish := func() { + f.mu.Lock() + f.authorizations++ + err := f.authErr + f.mu.Unlock() + complete(err) + } + if f.gate != nil { + go func() { <-f.gate; finish() }() + } else { + finish() + } +} + +func (f *fakeNotificationCenter) Add(_ context.Context, n NativeNotificationRequest) error { + f.mu.Lock() + defer f.mu.Unlock() + f.requests = append(f.requests, n) + if f.delivered != nil { + f.delivered <- struct{}{} + } + return f.addErr +} + +func TestMacNativeMappingAndConcurrentAuthorization(t *testing.T) { + f := &fakeNotificationCenter{} + m := &MacNotifier{Center: f} + d := DesktopNotifier{OS: "darwin", Mac: m, Runner: runnerFunc(func(context.Context, string, ...string) ([]byte, error) { + t.Error("notification launched an external helper") + return nil, nil + })} + n := Notification{Kind: "waiting", Title: "Needs you", Body: "Project · permission", Sound: "Glass", ItemID: "item '$(bad) with spaces", MachineID: "local"} + var wg sync.WaitGroup + for range 8 { + wg.Go(func() { + if err := d.Notify(t.Context(), n); err != nil { + t.Error(err) + } + }) + } + wg.Wait() + if f.authorizations != 1 || len(f.requests) != 8 { + t.Fatal(f) + } + first := f.requests[0] + if first.Title != n.Title || first.Body != n.Body || first.Sound != n.Sound || first.ItemID != n.ItemID || first.MachineID != n.MachineID || first.Identifier == "" || first.ThreadIdentifier != first.Identifier { + t.Fatal(first) + } + for _, next := range f.requests { + if next != first { + t.Fatal("re-send did not replace", next) + } + } + n.MachineID = "remote" + if nativeNotificationRequest(n).Identifier == first.Identifier { + t.Fatal("machines collided") + } + n.ItemID = "" + remote := nativeNotificationRequest(n) + if remote.ItemID != "" || remote.Identifier == first.Identifier { + t.Fatal(remote) + } + n.MachineID = "" + if nativeNotificationRequest(n).ItemID != "" { + t.Fatal("summary has a jump") + } +} + +func TestMacAuthorizationFailureAndCancellation(t *testing.T) { + denied := errors.New("denied") + f := &fakeNotificationCenter{authErr: denied} + m := &MacNotifier{Center: f} + for range 2 { + if err := m.Notify(t.Context(), Notification{}); !errors.Is(err, denied) { + t.Fatal(err) + } + } + if f.authorizations != 1 || len(f.requests) != 0 { + t.Fatal(f) + } + gate := make(chan struct{}) + f = &fakeNotificationCenter{gate: gate} + m = &MacNotifier{Center: f} + ctx, cancel := context.WithCancel(t.Context()) + if err := m.Notify(ctx, Notification{}); err != nil { + t.Fatal(err) + } + cancel() + if err := m.Notify(ctx, Notification{}); !errors.Is(err, context.Canceled) { + t.Fatal(err) + } + close(gate) + <-m.authorized + if err := m.Notify(t.Context(), Notification{}); err != nil { + t.Fatal(err) + } + if f.authorizations != 1 || len(f.requests) != 1 { + t.Fatal(f) + } + f.addErr = errors.New("delivery failed") + if err := m.Notify(t.Context(), Notification{}); !errors.Is(err, f.addErr) { + t.Fatal(err) + } +} + +func TestMacUnconfiguredBackendUnavailable(t *testing.T) { + for _, d := range []DesktopNotifier{{OS: "darwin"}, {OS: "darwin", Mac: &MacNotifier{}}} { + if err := d.Notify(t.Context(), Notification{}); err == nil { + t.Fatal("unbundled notifications available") + } + } +} + +func TestNativeNotificationClickUsesArgv(t *testing.T) { + var calls [][]string + r := runnerFunc(func(_ context.Context, name string, args ...string) ([]byte, error) { + calls = append(calls, append([]string{name}, args...)) + return nil, nil + }) + const path, item = "/opt/Driga's app/driga", "item '$(bad) with spaces" + for _, action := range []string{notificationDefaultAction, "com.apple.UNNotificationDismissActionIdentifier"} { + for _, id := range []string{item, ""} { + if err := notificationResponse(t.Context(), r, path, action, id); err != nil { + t.Fatal(err) + } + } + } + if !reflect.DeepEqual(calls, [][]string{{path, "inbox", "jump", item}}) { + t.Fatal(calls) + } +} + +func TestDrigaAppExecutable(t *testing.T) { + root := t.TempDir() + for _, rel := range []string{"Driga.app/Contents/MacOS/driga", "bin/driga", "Other.app/Contents/MacOS/driga", "Driga.app/Contents/MacOS/other"} { + path := filepath.Join(root, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, nil, 0600); err != nil { + t.Fatal(err) + } + if drigaAppExecutable(path) != (rel == "Driga.app/Contents/MacOS/driga") { + t.Fatal(rel) + } + } +} + +func TestDecimalBytes(t *testing.T) { + for n, want := range map[uint64]string{12_154_470_400: "12.2 GB", 20_000_000_000: "20 GB", 512_000_000: "512 MB", 1500: "1.5 kB", 0: "0 B"} { + if got := DecimalBytes(n); got != want { + t.Fatalf("%d: %s, want %s", n, got, want) + } + } +} + +func TestMacPendingAuthorizationQueuesFirstAlert(t *testing.T) { + gate := make(chan struct{}) + f := &fakeNotificationCenter{gate: gate, delivered: make(chan struct{}, 1)} + m := &MacNotifier{Center: f} + n := Notification{Title: "Low disk space", ItemID: "disk:1"} + // Notify must return while the permission prompt is still pending. + if err := m.Notify(t.Context(), n); err != nil { + t.Fatal(err) + } + close(gate) + select { + case <-f.delivered: + case <-time.After(5 * time.Second): + t.Fatal("pending notification lost") + } + f.mu.Lock() + defer f.mu.Unlock() + if len(f.requests) != 1 || f.requests[0].Title != n.Title { + t.Fatal(f.requests) + } +} diff --git a/internal/cli/deps.go b/internal/cli/deps.go index c113b0ec..9386273b 100644 --- a/internal/cli/deps.go +++ b/internal/cli/deps.go @@ -1,7 +1,6 @@ package cli import ( - "os" "time" "github.com/labstack/driga/internal/adapter" @@ -26,9 +25,5 @@ func NewDeps(cfg config.Config) (collector.Deps, error) { return collector.Deps{}, err } runner := newInboxRunner() - exe, err := os.Executable() - if err != nil { - return collector.Deps{}, err - } - return collector.Deps{Cfg: cfg, Plat: platform.New(), Adapters: []adapter.Adapter{claude.New(), codex.New(), gemini.New()}, Sigs: sigs, Git: gitscan.NewScanner(4), SpoolDir: config.SpoolDir(), Now: time.Now, State: collector.NewState(), InboxRunner: runner, Notifier: attention.DesktopNotifier{Runner: runner, DrigaPath: stablePath(exe), Toast: nativeToast}}, nil + return collector.Deps{Cfg: cfg, Plat: platform.New(), Adapters: []adapter.Adapter{claude.New(), codex.New(), gemini.New()}, Sigs: sigs, Git: gitscan.NewScanner(4), SpoolDir: config.SpoolDir(), Now: time.Now, State: collector.NewState(), InboxRunner: runner, Notifier: attention.DesktopNotifier{Runner: runner, Toast: nativeToast, Mac: attention.MacDesktopNotifier()}}, nil } diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 77287f25..8f35f42d 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -68,10 +68,9 @@ type DoctorDeps struct { Now time.Time ProcessAlive func(int32, time.Time) bool // Optional PID/start validation for fixtures. LockProbe func(dataDir string) (held bool, err error) - NotifierLookup func() string // Optional cached notifier lookup for fixtures. ServicePlistStale func() (bool, error) // macOS plist path, read-only. ServiceState func() (string, error) // Windows Task Scheduler, read-only. - NotificationStatus func() (string, error) // Windows Runtime, no registration/display. + NotificationStatus func() (string, error) // Native API status, no authorization/registration/display. } // Doctor checks stored evidence without collecting, writing, or controlling launchd. @@ -428,14 +427,15 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { add("service", "warn", "collector plist points at a stale binary", "driga service install") } } - lookup := d.NotifierLookup - if lookup == nil { - lookup = attention.TerminalNotifierPath + status := d.NotificationStatus + if status == nil { + status = attention.MacNotificationStatus } - if lookup() != "" { - add("notifications", "ok", "terminal-notifier (clickable)", "") + detail, err := status() + if err != nil { + add("notifications", "info", "notifications unavailable: "+err.Error(), "install Driga.app (brew install --cask driga); allow Driga in System Settings > Notifications") } else { - add("notifications", "info", "osascript (Script Editor)", "brew install terminal-notifier") + add("notifications", "ok", detail, "") } } add("fleet socket", "info", fleet.ControlSocketPath(d.DataDir), "") diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go index 7b8f9a99..b84555ac 100644 --- a/internal/cli/doctor_test.go +++ b/internal/cli/doctor_test.go @@ -3,6 +3,7 @@ package cli import ( "context" "encoding/json" + "fmt" "os" "path/filepath" "runtime" @@ -70,20 +71,25 @@ func TestDoctorNoDaemon(t *testing.T) { } func TestDoctorMacNotifier(t *testing.T) { - for _, path := range []string{"", "/opt/tool bin/terminal-notifier"} { + for _, available := range []bool{false, true} { d, _ := doctorFixture(t) d.OS = "darwin" - d.NotifierLookup = func() string { return path } - status, detail, fix := "ok", "terminal-notifier (clickable)", "" - if path == "" { - status, detail, fix = "info", "osascript (Script Editor)", "brew install terminal-notifier" + d.NotificationStatus = func() (string, error) { + if !available { + return "", fmt.Errorf("install Driga.app for native macOS notifications (unbundled builds cannot send them)") + } + return "native UserNotifications (Driga)", nil + } + status, detail, fix := "ok", "native UserNotifications (Driga)", "" + if !available { + status, detail, fix = "info", "notifications unavailable: install Driga.app for native macOS notifications (unbundled builds cannot send them)", "install Driga.app (brew install --cask driga); allow Driga in System Settings > Notifications" } c := requireCheck(t, Doctor(context.Background(), d), "notifications", status, fix) if c.Detail != detail || c.Fix != fix { t.Fatal(c) } d.OS = "linux" - d.NotifierLookup = func() string { t.Fatal("macOS lookup on Linux"); return "" } + d.NotificationStatus = func() (string, error) { t.Fatal("macOS status on Linux"); return "", nil } for _, c := range Doctor(context.Background(), d) { if c.Name == "notifications" { t.Fatal(c) diff --git a/internal/cli/stablepath_test.go b/internal/cli/stablepath_test.go index c8cee765..883e2a34 100644 --- a/internal/cli/stablepath_test.go +++ b/internal/cli/stablepath_test.go @@ -5,36 +5,8 @@ import ( "path/filepath" "runtime" "testing" - - "github.com/labstack/driga/internal/attention" - "github.com/labstack/driga/internal/config" ) -func TestNotifierUsesStablePath(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("Unix symlink fixture; Windows watch tests require no Developer Mode or admin rights") - } - - readEnv(t) - exe, err := os.Executable() - if err != nil { - t.Fatal(err) - } - link := filepath.Join(t.TempDir(), "driga") - if err := os.Symlink(exe, link); err != nil { - t.Fatal(err) - } - t.Setenv("PATH", filepath.Dir(link)) - d, err := NewDeps(config.Default()) - if err != nil { - t.Fatal(err) - } - n := d.Notifier.(attention.DesktopNotifier) - if n.DrigaPath != link { - t.Fatalf("notifier path = %q, want %q", n.DrigaPath, link) - } -} - // Homebrew links bin/driga to Cellar/driga//bin/driga. The service and // hooks must record the link, or brew upgrade leaves them on a deleted file. func TestStablePathPrefersPackageManagerLink(t *testing.T) { diff --git a/internal/collector/alerts_candidates_test.go b/internal/collector/alerts_candidates_test.go index 7c62261a..cca8499a 100644 --- a/internal/collector/alerts_candidates_test.go +++ b/internal/collector/alerts_candidates_test.go @@ -52,7 +52,7 @@ func TestAlertCandidatesDelaySuppressionAndLateArrival(t *testing.T) { run(10*time.Minute, 2) } -func TestAlertCandidatesMachineWarningsRepeatUntilClosed(t *testing.T) { +func TestAlertCandidatesMachineWarningDoesNotNotifyThreeTimes(t *testing.T) { d, st, _, _ := setup(t) n := &fakeNotifier{} d.Notifier = n @@ -63,13 +63,20 @@ func TestAlertCandidatesMachineWarningsRepeatUntilClosed(t *testing.T) { }); err != nil { t.Fatal(err) } - for _, offset := range []time.Duration{0, 30 * time.Second, time.Minute} { + for _, offset := range []time.Duration{0, time.Minute, 2 * time.Minute} { + // Three collector cycles, including a restart and refreshed readings. + d.State = NewState() + if err := st.Tx(context.Background(), func(tx *store.Tx) error { + return tx.RefreshMachineWarning("low_disk", "Disk still low", nil) + }); err != nil { + t.Fatal(err) + } d.Now = func() time.Time { return at.Add(offset) } if err := alertInbox(context.Background(), d, st); err != nil { t.Fatal(err) } } - if len(n.notes) != 2 { + if len(n.notes) != 1 { t.Fatal(n.notes) } if err := st.Tx(context.Background(), func(tx *store.Tx) error { _, err := tx.AckInbox("health", at.Add(time.Minute)); return err }); err != nil { @@ -79,9 +86,56 @@ func TestAlertCandidatesMachineWarningsRepeatUntilClosed(t *testing.T) { if err := alertInbox(context.Background(), d, st); err != nil { t.Fatal(err) } - if len(n.notes) != 2 { + if len(n.notes) != 1 { + t.Fatal(n.notes) + } +} + +func TestMachineNotificationsRearmOnlyAfterRecovery(t *testing.T) { + d, st, _, _ := setup(t) + n := &fakeNotifier{} + d.Notifier = n + d.Cfg.Machine.DiskFreeMin = 20_000_000_000 + at := d.now() + step := func(offset time.Duration, free uint64, pressure string, prune bool, want int) { + t.Helper() + now := at.Add(offset) + d.Now = func() time.Time { return now } + d.State = NewState() + if err := st.Tx(t.Context(), func(tx *store.Tx) error { + if prune { + if err := tx.Prune(now); err != nil { + return err + } + } + sample := model.MachineSample{TS: now, DiskTotal: 1_000_000_000_000, DiskFree: free, PressureLevel: pressure} + if err := lowDiskInbox(tx, d.Cfg.Machine, sample, now); err != nil { + return err + } + return memoryPressureInbox(tx, sample, now) + }); err != nil { + t.Fatal(err) + } + if err := alertInbox(t.Context(), d, st); err != nil { + t.Fatal(err) + } + if len(n.notes) != want { + t.Fatalf("at %s: %+v", offset, n.notes) + } + } + step(0, 12_154_470_400, "critical", false, 1) + if n.notes[0].Title != "Low disk space" || n.notes[0].Body != "12.2 GB free, below 20 GB" { t.Fatal(n.notes) } + step(time.Minute, 12_000_000_000, "critical", false, 2) + step(2*time.Minute, 11_000_000_000, "", false, 2) // Unknown is not recovery. + step(3*time.Minute, 11_000_000_000, "critical", false, 2) + step(4*time.Minute, 11_000_000_000, "critical", false, 2) + step(31*24*time.Hour, 11_000_000_000, "critical", true, 2) + step(31*24*time.Hour+time.Minute, 30_000_000_000, "normal", false, 2) + // Recovered conditions re-arm even inside the session repeat interval. + step(31*24*time.Hour+2*time.Minute, 12_000_000_000, "critical", false, 3) + step(31*24*time.Hour+3*time.Minute, 12_000_000_000, "critical", false, 4) } func TestAlertCandidatesRemoteReplayAndMetadataExpiry(t *testing.T) { diff --git a/internal/collector/cleanup.go b/internal/collector/cleanup.go index f645f898..f56740d3 100644 --- a/internal/collector/cleanup.go +++ b/internal/collector/cleanup.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "github.com/labstack/driga/internal/attention" "github.com/labstack/driga/internal/config" "github.com/labstack/driga/internal/model" "github.com/labstack/driga/internal/store" @@ -42,7 +43,7 @@ func lowDiskInbox(tx *store.Tx, cfg config.MachineConfig, s model.MachineSample, for _, w := range worktrees[:min(3, len(worktrees))] { names = append(names, w.Path) } - summary := fmt.Sprintf("Low disk: %d bytes free (threshold %d). Driga can show known cleanup candidates with driga cleanup --sizes; Machine shows the largest apps.", s.DiskFree, threshold) + summary := fmt.Sprintf("%s free, below %s. Inspect known cleanup candidates with driga cleanup list --sizes; driga top --tab machine shows the largest apps.", attention.DecimalBytes(s.DiskFree), attention.DecimalBytes(threshold)) if len(names) > 0 { summary += " Oldest stored worktrees to inspect: " + strings.Join(names, ", ") } diff --git a/internal/collector/machine_health.go b/internal/collector/machine_health.go index 81bf7f36..e151e9df 100644 --- a/internal/collector/machine_health.go +++ b/internal/collector/machine_health.go @@ -6,6 +6,7 @@ import ( "strconv" "time" + "github.com/labstack/driga/internal/attention" "github.com/labstack/driga/internal/derive" "github.com/labstack/driga/internal/model" "github.com/labstack/driga/internal/store" @@ -34,6 +35,11 @@ func memoryPressureInbox(tx *store.Tx, s model.MachineSample, now time.Time, pro if err := tx.SetMeta("machine:critical_count", "0"); err != nil { return err } + // Failed/unknown readings break the sample streak, but only a known + // recovery clears an episode and re-arms its notification. + if s.PressureLevel != "normal" && s.PressureLevel != "warning" { + return nil + } if err := tx.SetMeta("machine:pressure_active", "0"); err != nil { return err } @@ -117,7 +123,7 @@ func memoryPressureSummaryWithMemory(samples [][]model.Proc, sessions []model.Se } summary := "Critical memory pressure for two samples. " if memory == 0 { - summary += fmt.Sprintf("Agents hold none of the %.1f GiB in use; other apps are the cause.", float64(machineMemory)/(1<<30)) + summary += fmt.Sprintf("Agents hold none of the %s in use; other apps are the cause.", attention.DecimalBytes(machineMemory)) } else { count := 0 for _, c := range servers.Commands { @@ -127,13 +133,13 @@ func memoryPressureSummaryWithMemory(samples [][]model.Proc, sessions []model.Se if machineMemory > 0 { pct = 100 * float64(memory) / float64(machineMemory) } - summary += fmt.Sprintf("Agents hold %.1f GiB of %.1f GiB in use (%.0f%%), %d MCP servers.", float64(memory)/(1<<30), float64(machineMemory)/(1<<30), pct, count) + summary += fmt.Sprintf("Agents hold %s of %s in use (%.0f%%), %d MCP servers.", attention.DecimalBytes(memory), attention.DecimalBytes(machineMemory), pct, count) } if len(samples) > 0 { derive.ApplyMCPTrends(&servers, trends) if len(servers.Pileups) > 0 { p := servers.Pileups[0] - summary += fmt.Sprintf(" MCP pile-ups: %d groups; largest %s (%d processes, %.1f GiB). %s", len(servers.Pileups), p.Name, p.Count, float64(p.Memory)/(1<<30), p.Advice) + summary += fmt.Sprintf(" MCP pile-ups: %d groups; largest %s (%d processes, %s). %s", len(servers.Pileups), p.Name, p.Count, attention.DecimalBytes(p.Memory), p.Advice) } var largest *derive.MCPTrend for _, c := range servers.Commands { diff --git a/internal/collector/machine_health_test.go b/internal/collector/machine_health_test.go index 3e249a05..cd2d4274 100644 --- a/internal/collector/machine_health_test.go +++ b/internal/collector/machine_health_test.go @@ -204,14 +204,14 @@ func TestMachineViewReusesCollectorProcessSample(t *testing.T) { func TestPressureSummaryLeadsWithAgents(t *testing.T) { ps := []model.Proc{{PID: 1, Cmdline: "claude", Argv: []string{"claude"}, RSS: 4 << 30}, {PID: 2, Exe: "/bin/browser", RSS: 12 << 30}} text := memoryPressureSummary([][]model.Proc{ps}, nil, time.Now(), nil) - if !strings.HasPrefix(text, "Critical memory pressure for two samples. Agents hold 4.0 GiB of 16.0 GiB in use (25%)") { + if !strings.HasPrefix(text, "Critical memory pressure for two samples. Agents hold 4.3 GB of 17.2 GB in use (25%)") { t.Fatal(text) } } func TestPressureSummaryAgentsSmallShare(t *testing.T) { ps := []model.Proc{{PID: 1, Exe: "/bin/editor", RSS: 16 << 30}} text := memoryPressureSummary([][]model.Proc{ps}, nil, time.Now(), nil) - if !strings.Contains(text, "Agents hold none of the 16.0 GiB in use; other apps are the cause.") { + if !strings.Contains(text, "Agents hold none of the 17.2 GB in use; other apps are the cause.") { t.Fatal(text) } } diff --git a/internal/store/ack.go b/internal/store/ack.go index 5aedabb5..b6b31108 100644 --- a/internal/store/ack.go +++ b/internal/store/ack.go @@ -41,7 +41,7 @@ func (tx *Tx) EndMachineWarning(kind string, at time.Time) error { } // RefreshMachineWarning upgrades an existing episode's text and click target, -// preserving its identity, acknowledgement, and notification repeat history. +// preserving its identity, acknowledgement, and notification delivery history. func (tx *Tx) RefreshMachineWarning(kind, summary string, jump *model.InboxJump) error { if kind != "low_disk" && kind != "memory_pressure" { return fmt.Errorf("invalid machine warning %q", kind) diff --git a/internal/store/migrations/0003_health_alert_once.sql b/internal/store/migrations/0003_health_alert_once.sql new file mode 100644 index 00000000..1985874f --- /dev/null +++ b/internal/store/migrations/0003_health_alert_once.sql @@ -0,0 +1,10 @@ +-- +goose Up +DROP INDEX inbox_alert_candidates; +CREATE INDEX inbox_alert_candidates ON inbox_items(machine_id, ts) + WHERE acked_at IS NULL AND ended_at IS NULL AND alerted=0; + +-- +goose Down +DROP INDEX inbox_alert_candidates; +CREATE INDEX inbox_alert_candidates ON inbox_items(machine_id, ts) + WHERE acked_at IS NULL AND ended_at IS NULL + AND (alerted=0 OR kind IN ('low_disk','memory_pressure')); diff --git a/internal/store/queries.go b/internal/store/queries.go index 78b6ba13..8aa0485a 100644 --- a/internal/store/queries.go +++ b/internal/store/queries.go @@ -852,9 +852,10 @@ func (tx *Tx) Prune(now time.Time) error { AND NOT EXISTS(SELECT 1 FROM inbox_items i WHERE i.machine_id=? AND i.id=meta.value)`, tx.machineID, tx.machineID); err != nil { return err } - // Expiring the source seen metadata must also expire its candidate - // projection, including a peer wait still open after a month. - if err := tx.exec(`UPDATE inbox_items SET alerted=0 WHERE alerted=1 AND EXISTS ( + // Session metadata expiry also expires its candidate projection, + // including a peer wait still open after a month. Health delivery stays + // consumed until recovery creates a new episode. + if err := tx.exec(`UPDATE inbox_items SET alerted=0 WHERE alerted=1 AND kind NOT IN ('low_disk','memory_pressure') AND EXISTS ( SELECT 1 FROM meta WHERE meta.machine_id=? AND key='inbox_alert:'||inbox_items.machine_id||':'||inbox_items.id AND value Date: Fri, 23 Oct 2026 20:35:59 -0700 Subject: [PATCH 2/4] test: give the Chat approval expiry test room on slow runners Claude-Session: https://claude.ai/code/session_01T2vtcW6ihn1ywy9vxUq9a4 --- internal/chat/actions_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/chat/actions_test.go b/internal/chat/actions_test.go index f1c93d02..22bd5cdb 100644 --- a/internal/chat/actions_test.go +++ b/internal/chat/actions_test.go @@ -105,7 +105,8 @@ func TestApprovalStopExpiryAndCapabilities(t *testing.T) { t.Run(map[bool]string{true: "expiry", false: "stop"}[expiry], func(t *testing.T) { b, r := actionTurn(t) if expiry { - b.approvalTTL = 15 * time.Millisecond + // Long enough for a slow runner to observe the pending card first. + b.approvalTTL = 500 * time.Millisecond } in := Approval{Conversation: r.Conversation, Token: r.Token, Tool: "Bash", Input: json.RawMessage(`{}`)} bad := in From f1a3fc7f6679dfb81fbd29af1959fb83cccaacf8 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 23 Oct 2026 20:42:02 -0700 Subject: [PATCH 3/4] fix: report denied notifications and recover without a restart doctor now reads Driga's macOS notification permission without prompting and warns when it is off, pointing at System Settings. After a refusal the collector re-checks permission on the next notification, so allowing Driga later takes effect without restarting the service. Claude-Session: https://claude.ai/code/session_01T2vtcW6ihn1ywy9vxUq9a4 --- docs/usage.md | 4 +- internal/attention/notifications.go | 45 ++++++++++++++++------ internal/attention/notifications_darwin.go | 32 ++++++++++++++- internal/attention/notifications_test.go | 8 +++- internal/cli/doctor.go | 6 ++- internal/cli/doctor_test.go | 13 ++++++- 6 files changed, 92 insertions(+), 16 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index f1aadf24..6365c6aa 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -101,7 +101,9 @@ your capture policy. macOS notifications require the installed **Driga.app** UserNotifications API. Allow alerts and sounds for Driga when macOS asks, or in System Settings → Notifications → Driga. Unbundled builds (`go run`, checkout binaries) cannot send macOS notifications; `driga doctor` explains how to install -the app. No notification helpers are needed. +the app. No notification helpers are needed. `driga doctor` warns when +notifications are turned off for Driga; turning them back on takes effect from +the next notification, without restarting the collector. Clicking a local item's notification runs `driga inbox jump ` without acknowledging it. Jumps select a proven tmux pane, focus a proven Terminal/iTerm diff --git a/internal/attention/notifications.go b/internal/attention/notifications.go index 7c4a55ab..5c3aa25a 100644 --- a/internal/attention/notifications.go +++ b/internal/attention/notifications.go @@ -3,12 +3,16 @@ package attention import ( "context" "crypto/sha256" + "errors" "fmt" "path/filepath" "sync" "time" ) +// ErrNotificationsDenied means macOS has Driga's notifications turned off. +var ErrNotificationsDenied = errors.New("Driga is not allowed to send notifications") + // NativeNotificationRequest is the value-only boundary to UserNotifications. // No Objective-C objects escape the Darwin implementation. type NativeNotificationRequest struct { @@ -22,11 +26,13 @@ type NativeNotificationCenter interface { Add(context.Context, NativeNotificationRequest) error } -// MacNotifier requests permission once per process. macOS remembers the user's -// decision between launches. Authorization can outlive a cancelled Notify call. +// MacNotifier asks macOS for permission before posting. After a refusal it asks +// again on the next notification; macOS answers from the saved setting without +// prompting, so allowing Driga in System Settings takes effect without a restart. +// Authorization can outlive a cancelled Notify call. type MacNotifier struct { Center NativeNotificationCenter - once sync.Once + mu sync.Mutex authorized chan struct{} authErr error } @@ -38,15 +44,26 @@ func (m *MacNotifier) Notify(ctx context.Context, n Notification) error { if err := ctx.Err(); err != nil { return err } - m.once.Do(func() { - m.authorized = make(chan struct{}) + m.mu.Lock() + ch, start := m.authorized, m.authorized == nil + if start { + ch = make(chan struct{}) + m.authorized = ch + } + m.mu.Unlock() + if start { // outside the lock: the callback may run synchronously m.Center.Authorize(func(err error) { + m.mu.Lock() m.authErr = err - close(m.authorized) + if err != nil { + m.authorized = nil // re-ask on the next notification + } + m.mu.Unlock() + close(ch) }) - }) + } select { - case <-m.authorized: + case <-ch: return m.post(ctx, n) default: // Keep the first alert queued while the user considers permission, @@ -54,7 +71,7 @@ func (m *MacNotifier) Notify(ctx context.Context, n Notification) error { go func() { select { case <-ctx.Done(): - case <-m.authorized: + case <-ch: _ = m.post(ctx, n) } }() @@ -62,12 +79,18 @@ func (m *MacNotifier) Notify(ctx context.Context, n Notification) error { } } +func (m *MacNotifier) err() error { + m.mu.Lock() + defer m.mu.Unlock() + return m.authErr +} + func (m *MacNotifier) post(ctx context.Context, n Notification) error { if err := ctx.Err(); err != nil { return err } - if m.authErr != nil { - return m.authErr + if err := m.err(); err != nil { + return err } ctx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() diff --git a/internal/attention/notifications_darwin.go b/internal/attention/notifications_darwin.go index 4e287577..a8295ecd 100644 --- a/internal/attention/notifications_darwin.go +++ b/internal/attention/notifications_darwin.go @@ -55,7 +55,37 @@ func MacNotificationStatus() (string, error) { if goString(bundle.Send(objc.RegisterName("bundleIdentifier"))) != "run.driga" { return "", fmt.Errorf("Driga.app must have the run.driga bundle identity; reinstall the app") } - return "native UserNotifications (Driga); delivery depends on macOS notification settings", nil + switch macAuthorizationStatus() { + case 1: + return "", ErrNotificationsDenied + case 0: + return "native, not asked yet: macOS asks when Driga first notifies", nil + case -1: + return "native UserNotifications (Driga); permission unknown", nil + } + return "native UserNotifications (Driga), allowed", nil +} + +// macAuthorizationStatus reads UNAuthorizationStatus without prompting: +// 0 not determined, 1 denied, 2+ allowed. -1 when macOS does not answer. +func macAuthorizationStatus() int64 { + if _, err := purego.Dlopen("/System/Library/Frameworks/UserNotifications.framework/UserNotifications", purego.RTLD_GLOBAL|purego.RTLD_NOW); err != nil { + return -1 + } + defer autoreleasePool()() + result := make(chan int64, 1) + block := objc.NewBlock(func(_ objc.Block, settings objc.ID) { + result <- objc.Send[int64](settings, objc.RegisterName("authorizationStatus")) + }) + defer block.Release() + center := nsClass("UNUserNotificationCenter").Send(objc.RegisterName("currentNotificationCenter")) + center.Send(objc.RegisterName("getNotificationSettingsWithCompletionHandler:"), block) + select { + case status := <-result: + return status + case <-time.After(2 * time.Second): + return -1 + } } // RunNotificationMain is called only by the executable's locked initial thread. diff --git a/internal/attention/notifications_test.go b/internal/attention/notifications_test.go index b5b3cbf1..c85de66d 100644 --- a/internal/attention/notifications_test.go +++ b/internal/attention/notifications_test.go @@ -98,9 +98,15 @@ func TestMacAuthorizationFailureAndCancellation(t *testing.T) { t.Fatal(err) } } - if f.authorizations != 1 || len(f.requests) != 0 { + // A refusal is re-checked on the next notification, so allowing Driga in + // System Settings works without restarting the collector. + if f.authorizations != 2 || len(f.requests) != 0 { t.Fatal(f) } + f.authErr = nil + if err := m.Notify(t.Context(), Notification{}); err != nil || len(f.requests) != 1 { + t.Fatal("allowing later did not deliver", err, f) + } gate := make(chan struct{}) f = &fakeNotificationCenter{gate: gate} m = &MacNotifier{Center: f} diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 8f35f42d..de40b07c 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -433,7 +433,11 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { } detail, err := status() if err != nil { - add("notifications", "info", "notifications unavailable: "+err.Error(), "install Driga.app (brew install --cask driga); allow Driga in System Settings > Notifications") + if errors.Is(err, attention.ErrNotificationsDenied) { + add("notifications", "warn", err.Error(), "System Settings > Notifications > Driga > Allow notifications") + } else { + add("notifications", "info", "notifications unavailable: "+err.Error(), "install Driga.app (brew install --cask driga)") + } } else { add("notifications", "ok", detail, "") } diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go index b84555ac..ba46e5a1 100644 --- a/internal/cli/doctor_test.go +++ b/internal/cli/doctor_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "github.com/labstack/driga/internal/attention" "os" "path/filepath" "runtime" @@ -82,7 +83,7 @@ func TestDoctorMacNotifier(t *testing.T) { } status, detail, fix := "ok", "native UserNotifications (Driga)", "" if !available { - status, detail, fix = "info", "notifications unavailable: install Driga.app for native macOS notifications (unbundled builds cannot send them)", "install Driga.app (brew install --cask driga); allow Driga in System Settings > Notifications" + status, detail, fix = "info", "notifications unavailable: install Driga.app for native macOS notifications (unbundled builds cannot send them)", "install Driga.app (brew install --cask driga)" } c := requireCheck(t, Doctor(context.Background(), d), "notifications", status, fix) if c.Detail != detail || c.Fix != fix { @@ -98,6 +99,16 @@ func TestDoctorMacNotifier(t *testing.T) { } } +func TestDoctorMacNotificationsDenied(t *testing.T) { + d, _ := doctorFixture(t) + d.OS = "darwin" + d.NotificationStatus = func() (string, error) { return "", attention.ErrNotificationsDenied } + c := requireCheck(t, Doctor(context.Background(), d), "notifications", "warn", "System Settings > Notifications > Driga > Allow notifications") + if c.Detail != attention.ErrNotificationsDenied.Error() { + t.Fatal(c) + } +} + func TestDoctorIntake(t *testing.T) { d, st := doctorFixture(t) c := requireCheck(t, Doctor(context.Background(), d), "otlp intake", "info", "") From 34303ebee3b07dfba4fd3bdbbc945b97786ab09f Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Fri, 23 Oct 2026 21:07:25 -0700 Subject: [PATCH 4/4] fix: address review of native notifications - Startup no longer fails when notifications are denied: only the bundle identity is required, so allowing Driga later works without a restart. - Low disk ends an episode only at 10% above the limit, and memory pressure only on a return to normal, so hovering or flapping cannot raise an alert on every crossing. - A viewer without the service no longer consumes alerts it cannot show. - Alerts queued during the permission prompt coalesce per destination. - Free space is truncated, never rounded up to the limit. - `collect` is detected after global flags; doctor's install hint uses the tap (`brew install --cask labstack/tap/driga`). Claude-Session: https://claude.ai/code/session_01T2vtcW6ihn1ywy9vxUq9a4 --- cmd/driga/main_darwin.go | 12 +++++- docs/usage.md | 7 +++- internal/attention/bytes.go | 11 +++++ internal/attention/notifications.go | 35 +++++++++++----- internal/attention/notifications_darwin.go | 42 +++++++++++++------- internal/attention/notifications_other.go | 2 + internal/attention/notifications_test.go | 21 +++++++++- internal/cli/deps.go | 6 ++- internal/cli/doctor.go | 2 +- internal/cli/doctor_test.go | 2 +- internal/collector/alerts_candidates_test.go | 27 ++++++++++++- internal/collector/cleanup.go | 9 ++++- internal/collector/machine_health.go | 7 ++-- internal/collector/machine_health_test.go | 2 +- 14 files changed, 147 insertions(+), 38 deletions(-) diff --git a/cmd/driga/main_darwin.go b/cmd/driga/main_darwin.go index 7f798057..96df9964 100644 --- a/cmd/driga/main_darwin.go +++ b/cmd/driga/main_darwin.go @@ -5,6 +5,7 @@ package main import ( "os" "runtime" + "strings" "github.com/labstack/driga/internal/attention" ) @@ -16,10 +17,19 @@ func init() { runtime.LockOSThread() } // Only the collector posts notifications. Other commands, including hooks // that run on every agent event, skip AppKit and release the thread. func notificationMain(run func()) { - if len(os.Args) < 2 || os.Args[1] != "collect" { + if command(os.Args[1:]) != "collect" { runtime.UnlockOSThread() run() return } attention.RunNotificationMain(run) } + +func command(args []string) string { + for _, a := range args { + if !strings.HasPrefix(a, "-") { + return a + } + } + return "" +} diff --git a/docs/usage.md b/docs/usage.md index 6365c6aa..c75dad35 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -97,13 +97,16 @@ show `ack pending` and retry on reconnect. Desktop alerts show the session title or project and why it needs you, within your capture policy. macOS notifications require the installed **Driga.app** -(`brew install --cask driga`); they use Driga's name and icon through the native +(`brew install --cask labstack/tap/driga`); they use Driga's name and icon through the native UserNotifications API. Allow alerts and sounds for Driga when macOS asks, or in System Settings → Notifications → Driga. Unbundled builds (`go run`, checkout binaries) cannot send macOS notifications; `driga doctor` explains how to install the app. No notification helpers are needed. `driga doctor` warns when notifications are turned off for Driga; turning them back on takes effect from -the next notification, without restarting the collector. +the next notification, without restarting the collector. Alerts raised while +notifications are off are not replayed; they stay in the inbox. Only the +background collector posts notifications on macOS, so run the service +(`driga service install`) to receive them. Clicking a local item's notification runs `driga inbox jump ` without acknowledging it. Jumps select a proven tmux pane, focus a proven Terminal/iTerm diff --git a/internal/attention/bytes.go b/internal/attention/bytes.go index d408b269..445ec014 100644 --- a/internal/attention/bytes.go +++ b/internal/attention/bytes.go @@ -17,3 +17,14 @@ func DecimalBytes(n uint64) string { } return fmt.Sprintf("%d B", n) } + +// DecimalBytesFloor truncates instead of rounding, for values shown against a +// limit: 19.96 GB free must not read as "20 GB free, below 20 GB". +func DecimalBytesFloor(n uint64) string { + for _, size := range []uint64{1_000_000_000_000, 1_000_000_000, 1_000_000, 1_000} { + if n >= size { + return DecimalBytes(n / (size / 10) * (size / 10)) + } + } + return DecimalBytes(n) +} diff --git a/internal/attention/notifications.go b/internal/attention/notifications.go index 5c3aa25a..23a2dd36 100644 --- a/internal/attention/notifications.go +++ b/internal/attention/notifications.go @@ -35,6 +35,7 @@ type MacNotifier struct { mu sync.Mutex authorized chan struct{} authErr error + pending map[string]Notification // queued while permission is undecided } func (m *MacNotifier) Notify(ctx context.Context, n Notification) error { @@ -66,16 +67,30 @@ func (m *MacNotifier) Notify(ctx context.Context, n Notification) error { case <-ch: return m.post(ctx, n) default: - // Keep the first alert queued while the user considers permission, - // without stalling collection or imposing a timeout on their answer. - go func() { - select { - case <-ctx.Done(): - case <-ch: - _ = m.post(ctx, n) - } - }() - return nil + } + // While the user considers permission, keep only the latest alert per + // destination and post them once answered; collection never waits. + m.mu.Lock() + if m.pending == nil { + m.pending = map[string]Notification{} + go m.flush(ctx, ch) + } + m.pending[nativeNotificationRequest(n).Identifier] = n + m.mu.Unlock() + return nil +} + +func (m *MacNotifier) flush(ctx context.Context, ch chan struct{}) { + select { + case <-ctx.Done(): + case <-ch: + } + m.mu.Lock() + queued := m.pending + m.pending = nil + m.mu.Unlock() + for _, n := range queued { + _ = m.post(ctx, n) } } diff --git a/internal/attention/notifications_darwin.go b/internal/attention/notifications_darwin.go index a8295ecd..7efe00a4 100644 --- a/internal/attention/notifications_darwin.go +++ b/internal/attention/notifications_darwin.go @@ -21,6 +21,11 @@ var macStartupErr error func MacDesktopNotifier() Notifier { return macNotifier } +// DesktopNotificationsReady reports whether this process can post. On macOS +// only the bundled collector runs the native center; other processes, such as +// a viewer without the service, must leave alerts for the collector. +func DesktopNotificationsReady() bool { return macNotifier.(*MacNotifier).Center != nil } + func nsClass(name string) objc.ID { return objc.ID(objc.GetClass(name)) } func nsString(s string) objc.ID { return nsClass("NSString").Send(objc.RegisterName("stringWithUTF8String:"), s) @@ -37,24 +42,12 @@ func autoreleasePool() func() { } func MacNotificationStatus() (string, error) { - exe, err := os.Executable() - if err != nil { + if err := macBundleIdentity(); err != nil { return "", err } - if !drigaAppExecutable(exe) { - return "", fmt.Errorf("install Driga.app for native macOS notifications (unbundled builds cannot send them)") - } if macStartupErr != nil { return "", macStartupErr } - if _, err := purego.Dlopen("/System/Library/Frameworks/Foundation.framework/Foundation", purego.RTLD_GLOBAL|purego.RTLD_NOW); err != nil { - return "", err - } - defer autoreleasePool()() - bundle := nsClass("NSBundle").Send(objc.RegisterName("mainBundle")) - if goString(bundle.Send(objc.RegisterName("bundleIdentifier"))) != "run.driga" { - return "", fmt.Errorf("Driga.app must have the run.driga bundle identity; reinstall the app") - } switch macAuthorizationStatus() { case 1: return "", ErrNotificationsDenied @@ -66,6 +59,27 @@ func MacNotificationStatus() (string, error) { return "native UserNotifications (Driga), allowed", nil } +// macBundleIdentity checks that this executable can post as Driga. Permission +// is separate: a refusal is re-checked on each notification, never fatal. +func macBundleIdentity() error { + exe, err := os.Executable() + if err != nil { + return err + } + if !drigaAppExecutable(exe) { + return fmt.Errorf("install Driga.app for native macOS notifications (unbundled builds cannot send them)") + } + if _, err := purego.Dlopen("/System/Library/Frameworks/Foundation.framework/Foundation", purego.RTLD_GLOBAL|purego.RTLD_NOW); err != nil { + return err + } + defer autoreleasePool()() + bundle := nsClass("NSBundle").Send(objc.RegisterName("mainBundle")) + if goString(bundle.Send(objc.RegisterName("bundleIdentifier"))) != "run.driga" { + return fmt.Errorf("Driga.app must have the run.driga bundle identity; reinstall the app") + } + return nil +} + // macAuthorizationStatus reads UNAuthorizationStatus without prompting: // 0 not determined, 1 denied, 2+ allowed. -1 when macOS does not answer. func macAuthorizationStatus() int64 { @@ -123,7 +137,7 @@ func startMacNotifications(exe string) (objc.ID, *macNotificationCenter, error) } } defer autoreleasePool()() - if _, err := MacNotificationStatus(); err != nil { + if err := macBundleIdentity(); err != nil { return 0, nil, err } app := nsClass("NSApplication").Send(objc.RegisterName("sharedApplication")) diff --git a/internal/attention/notifications_other.go b/internal/attention/notifications_other.go index a4823fdf..d341450b 100644 --- a/internal/attention/notifications_other.go +++ b/internal/attention/notifications_other.go @@ -9,3 +9,5 @@ func MacNotificationStatus() (string, error) { } func MacDesktopNotifier() Notifier { return &MacNotifier{} } + +func DesktopNotificationsReady() bool { return true } diff --git a/internal/attention/notifications_test.go b/internal/attention/notifications_test.go index c85de66d..ed7cdeaf 100644 --- a/internal/attention/notifications_test.go +++ b/internal/attention/notifications_test.go @@ -45,6 +45,12 @@ func (f *fakeNotificationCenter) Add(_ context.Context, n NativeNotificationRequ return f.addErr } +func (f *fakeNotificationCenter) count() int { + f.mu.Lock() + defer f.mu.Unlock() + return len(f.requests) +} + func TestMacNativeMappingAndConcurrentAuthorization(t *testing.T) { f := &fakeNotificationCenter{} m := &MacNotifier{Center: f} @@ -62,7 +68,12 @@ func TestMacNativeMappingAndConcurrentAuthorization(t *testing.T) { }) } wg.Wait() - if f.authorizations != 1 || len(f.requests) != 8 { + // Alerts arriving while permission is pending coalesce per destination. + deadline := time.Now().Add(2 * time.Second) + for f.count() == 0 && time.Now().Before(deadline) { + time.Sleep(time.Millisecond) + } + if f.authorizations != 1 || f.count() < 1 || f.count() > 8 { t.Fatal(f) } first := f.requests[0] @@ -175,6 +186,14 @@ func TestDrigaAppExecutable(t *testing.T) { } } +func TestDecimalBytesFloor(t *testing.T) { + for n, want := range map[uint64]string{12_154_470_400: "12.1 GB", 19_960_000_000: "19.9 GB", 20_000_000_000: "20 GB", 999: "999 B"} { + if got := DecimalBytesFloor(n); got != want { + t.Fatalf("%d: %s, want %s", n, got, want) + } + } +} + func TestDecimalBytes(t *testing.T) { for n, want := range map[uint64]string{12_154_470_400: "12.2 GB", 20_000_000_000: "20 GB", 512_000_000: "512 MB", 1500: "1.5 kB", 0: "0 B"} { if got := DecimalBytes(n); got != want { diff --git a/internal/cli/deps.go b/internal/cli/deps.go index 9386273b..dab62222 100644 --- a/internal/cli/deps.go +++ b/internal/cli/deps.go @@ -25,5 +25,9 @@ func NewDeps(cfg config.Config) (collector.Deps, error) { return collector.Deps{}, err } runner := newInboxRunner() - return collector.Deps{Cfg: cfg, Plat: platform.New(), Adapters: []adapter.Adapter{claude.New(), codex.New(), gemini.New()}, Sigs: sigs, Git: gitscan.NewScanner(4), SpoolDir: config.SpoolDir(), Now: time.Now, State: collector.NewState(), InboxRunner: runner, Notifier: attention.DesktopNotifier{Runner: runner, Toast: nativeToast, Mac: attention.MacDesktopNotifier()}}, nil + var notifier attention.Notifier // nil: leave alerts unconsumed for a process that can post + if attention.DesktopNotificationsReady() { + notifier = attention.DesktopNotifier{Runner: runner, Toast: nativeToast, Mac: attention.MacDesktopNotifier()} + } + return collector.Deps{Cfg: cfg, Plat: platform.New(), Adapters: []adapter.Adapter{claude.New(), codex.New(), gemini.New()}, Sigs: sigs, Git: gitscan.NewScanner(4), SpoolDir: config.SpoolDir(), Now: time.Now, State: collector.NewState(), InboxRunner: runner, Notifier: notifier}, nil } diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index de40b07c..e530feb7 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -436,7 +436,7 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { if errors.Is(err, attention.ErrNotificationsDenied) { add("notifications", "warn", err.Error(), "System Settings > Notifications > Driga > Allow notifications") } else { - add("notifications", "info", "notifications unavailable: "+err.Error(), "install Driga.app (brew install --cask driga)") + add("notifications", "info", err.Error(), "brew install --cask labstack/tap/driga") } } else { add("notifications", "ok", detail, "") diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go index ba46e5a1..8dc89b63 100644 --- a/internal/cli/doctor_test.go +++ b/internal/cli/doctor_test.go @@ -83,7 +83,7 @@ func TestDoctorMacNotifier(t *testing.T) { } status, detail, fix := "ok", "native UserNotifications (Driga)", "" if !available { - status, detail, fix = "info", "notifications unavailable: install Driga.app for native macOS notifications (unbundled builds cannot send them)", "install Driga.app (brew install --cask driga)" + status, detail, fix = "info", "install Driga.app for native macOS notifications (unbundled builds cannot send them)", "brew install --cask labstack/tap/driga" } c := requireCheck(t, Doctor(context.Background(), d), "notifications", status, fix) if c.Detail != detail || c.Fix != fix { diff --git a/internal/collector/alerts_candidates_test.go b/internal/collector/alerts_candidates_test.go index cca8499a..99e51b85 100644 --- a/internal/collector/alerts_candidates_test.go +++ b/internal/collector/alerts_candidates_test.go @@ -124,7 +124,7 @@ func TestMachineNotificationsRearmOnlyAfterRecovery(t *testing.T) { } } step(0, 12_154_470_400, "critical", false, 1) - if n.notes[0].Title != "Low disk space" || n.notes[0].Body != "12.2 GB free, below 20 GB" { + if n.notes[0].Title != "Low disk space" || n.notes[0].Body != "12.1 GB free, below 20 GB" { t.Fatal(n.notes) } step(time.Minute, 12_000_000_000, "critical", false, 2) @@ -138,6 +138,31 @@ func TestMachineNotificationsRearmOnlyAfterRecovery(t *testing.T) { step(31*24*time.Hour+3*time.Minute, 12_000_000_000, "critical", false, 4) } +func TestLowDiskHoveringAtLimitAlertsOnce(t *testing.T) { + d, st, _, _ := setup(t) + n := &fakeNotifier{} + d.Notifier = n + d.Cfg.Machine.DiskFreeMin = 20_000_000_000 + at := d.now() + for i, free := range []uint64{19_500_000_000, 20_500_000_000, 19_800_000_000, 21_900_000_000, 19_000_000_000, 22_000_000_000, 19_000_000_000} { + now := at.Add(time.Duration(i) * time.Minute) + d.Now = func() time.Time { return now } + d.State = NewState() + if err := st.Tx(t.Context(), func(tx *store.Tx) error { + return lowDiskInbox(tx, d.Cfg.Machine, model.MachineSample{TS: now, DiskTotal: 1_000_000_000_000, DiskFree: free}, now) + }); err != nil { + t.Fatal(err) + } + if err := alertInbox(t.Context(), d, st); err != nil { + t.Fatal(err) + } + } + // Dips inside 10% of the limit keep one episode; 22 GB recovers and re-arms. + if len(n.notes) != 2 { + t.Fatal(len(n.notes), n.notes) + } +} + func TestAlertCandidatesRemoteReplayAndMetadataExpiry(t *testing.T) { d, st, _, _ := setup(t) n := &fakeNotifier{} diff --git a/internal/collector/cleanup.go b/internal/collector/cleanup.go index f56740d3..b26c43e1 100644 --- a/internal/collector/cleanup.go +++ b/internal/collector/cleanup.go @@ -28,12 +28,17 @@ func lowDiskInbox(tx *store.Tx, cfg config.MachineConfig, s model.MachineSample, if err != nil && !errors.Is(err, store.ErrNotFound) { return err } - if s.DiskFree >= threshold { + // An active episode ends only once free space is 10% above the limit, so + // hovering at the threshold cannot raise a new alert on every pass. + if s.DiskFree >= threshold && (active != "1" || s.DiskFree >= threshold+threshold/10) { if err := tx.EndMachineWarning("low_disk", now); err != nil { return err } return tx.SetMeta("cleanup:low_disk", "0") } + if s.DiskFree >= threshold { + return nil // recovering, still inside the hysteresis band + } worktrees, err := tx.Worktrees(0) if err != nil { return err @@ -43,7 +48,7 @@ func lowDiskInbox(tx *store.Tx, cfg config.MachineConfig, s model.MachineSample, for _, w := range worktrees[:min(3, len(worktrees))] { names = append(names, w.Path) } - summary := fmt.Sprintf("%s free, below %s. Inspect known cleanup candidates with driga cleanup list --sizes; driga top --tab machine shows the largest apps.", attention.DecimalBytes(s.DiskFree), attention.DecimalBytes(threshold)) + summary := fmt.Sprintf("%s free, below %s. Inspect known cleanup candidates with driga cleanup list --sizes; driga top --tab machine shows the largest apps.", attention.DecimalBytesFloor(s.DiskFree), attention.DecimalBytes(threshold)) if len(names) > 0 { summary += " Oldest stored worktrees to inspect: " + strings.Join(names, ", ") } diff --git a/internal/collector/machine_health.go b/internal/collector/machine_health.go index e151e9df..4e0aa39b 100644 --- a/internal/collector/machine_health.go +++ b/internal/collector/machine_health.go @@ -35,9 +35,10 @@ func memoryPressureInbox(tx *store.Tx, s model.MachineSample, now time.Time, pro if err := tx.SetMeta("machine:critical_count", "0"); err != nil { return err } - // Failed/unknown readings break the sample streak, but only a known - // recovery clears an episode and re-arms its notification. - if s.PressureLevel != "normal" && s.PressureLevel != "warning" { + // Failed/unknown and warning readings break the sample streak, but only + // a return to normal clears an episode and re-arms its notification, so + // critical/warning flapping cannot alert repeatedly. + if s.PressureLevel != "normal" { return nil } if err := tx.SetMeta("machine:pressure_active", "0"); err != nil { diff --git a/internal/collector/machine_health_test.go b/internal/collector/machine_health_test.go index cd2d4274..2abab6e8 100644 --- a/internal/collector/machine_health_test.go +++ b/internal/collector/machine_health_test.go @@ -68,7 +68,7 @@ func TestRecordedMachineWarnings(t *testing.T) { samples := []struct { level string want int - }{{"critical", 0}, {"normal", 0}, {"critical", 0}, {"critical", 1}, {"critical", 1}, {"warning", 1}, {"critical", 1}, {"critical", 2}} + }{{"critical", 0}, {"normal", 0}, {"critical", 0}, {"critical", 1}, {"critical", 1}, {"warning", 1}, {"critical", 1}, {"critical", 1}, {"normal", 1}, {"critical", 1}, {"critical", 2}} for i, s := range samples { now := at.Add(time.Duration(i) * time.Minute) err := st.Tx(context.Background(), func(tx *store.Tx) error {