feat: one notification spine -- durable record, one presence gate, N channels (goal 0171) (#362)

Replaces three independently-drifted presence checks (backend isAway,
the browser tab's focus-only predicate, the ungated dock badge) with a
single exported SettingsService.IsAway, and two in-memory dedupe Sets
with one persisted per-channel delivery record on notificationsvc's
NotificationService.Publish -- persist first, then fan out to a
registered Channel per delivery destination (desktop banner, dock
bounce, browser tab).


Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: alicoding

frontend/bindings/github.com/alicoding/mill/internal/domain/notification/index.ts

@@ -0,0 +1,7 @@
+// Cynhyrchwyd y ffeil hon yn awtomatig. PEIDIWCH Â MODIWL
+// This file is automatically generated. DO NOT EDIT
+
+export type {
+    Event,
+    Record
+} from "./models.js";

frontend/bindings/github.com/alicoding/mill/internal/domain/notification/models.ts

@@ -0,0 +1,69 @@
+// Cynhyrchwyd y ffeil hon yn awtomatig. PEIDIWCH Â MODIWL
+// This file is automatically generated. DO NOT EDIT
+
+/**
+ * Event is what a producer hands to Publish -- Record's input-facing
+ * counterpart, the same input/full-record split MCPWriteRequest/
+ * MCPWriteRecord already establish (millmcpservice_approval.go).
+ */
+export interface Event {
+    "type": string;
+    "title": string;
+    "body": string;
+    "dedupeKey": string;
+    "sourceRef"?: string;
+
+    /**
+     * Focused is the producer's own presence reading for the surface
+     * that originated this event (a frontend's document.hasFocus()),
+     * threaded through rather than read here since only the caller
+     * knows it. The zero value (false) fails toward "away" -- a
+     * backend-originated event with no window to read focus from (a
+     * run finishing unattended) is exactly the case a channel's
+     * presence gate should err toward delivering for, not suppressing.
+     */
+    "focused": boolean;
+}
+
+/**
+ * Record is one durable notification, keyed by DedupeKey so publishing
+ * the same logical event twice (a restart, two independent producers
+ * for the same underlying item) resolves to the same Record rather than
+ * a duplicate. Field names follow CloudEvents' own id/type/time/data
+ * shape (id, type, time here as CreatedAt, data as Title+Body) --
+ * naming alignment only, the CloudEvents SDK itself is not imported
+ * (docs/goals/0171's own commodity verdict: adopt the envelope shape,
+ * defer the SDK).
+ */
+export interface Record {
+    "id": string;
+    "type": string;
+    "title": string;
+    "body": string;
+    "dedupeKey": string;
+
+    /**
+     * SourceRef names the underlying item this notification is about
+     * (a run ID, an MCP write ID, an update version) -- what a click
+     * would navigate to, kept separate from DedupeKey since a future
+     * producer's dedupe key and navigation target could genuinely
+     * differ even though today every producer sets them equal.
+     */
+    "sourceRef"?: string;
+    "createdAt": string;
+    "readAt"?: string | null;
+    "expiresAt"?: string | null;
+
+    /**
+     * DeliveredChannels names every Channel that has already run
+     * Deliver for this Record -- the one dedupe mechanism (goal 0171),
+     * persisted alongside the record itself so it survives a reload
+     * instead of resetting the way two separate in-memory Sets used to.
+     * A channel appears here at most once; a Publish call for an
+     * already-published DedupeKey re-attempts only the channels still
+     * missing from this list, so a second producer of the same logical
+     * event (e.g. a second browser tab) still gets ITS OWN channel
+     * delivered without re-delivering one that already fired.
+     */
+    "deliveredChannels"?: string[] | null;
+}

frontend/bindings/github.com/alicoding/mill/internal/services/notificationsvc/index.ts

@@ -0,0 +1,11 @@
+// Cynhyrchwyd y ffeil hon yn awtomatig. PEIDIWCH Â MODIWL
+// This file is automatically generated. DO NOT EDIT
+
+import * as NotificationService from "./notificationservice.js";
+export {
+    NotificationService
+};
+
+export type {
+    PublishResult
+} from "./models.js";

frontend/bindings/github.com/alicoding/mill/internal/services/notificationsvc/models.ts

@@ -0,0 +1,19 @@
+// Cynhyrchwyd y ffeil hon yn awtomatig. PEIDIWCH Â MODIWL
+// This file is automatically generated. DO NOT EDIT
+
+// eslint-disable-next-line @typescript-eslint/ban-ts-comment
+// @ts-ignore: Unused imports
+import * as notification$0 from "../../domain/notification/models.js";
+
+/**
+ * PublishResult is Publish's return shape: the durable Record (created
+ * fresh, or the existing one a repeat DedupeKey resolved to) plus which
+ * channel names newly delivered on THIS call specifically -- distinct
+ * from Record.DeliveredChannels' cumulative history, since a caller
+ * (e.g. the browser-tab channel's own frontend half) needs to know
+ * whether ITS delivery just happened, not the record's full history.
+ */
+export interface PublishResult {
+    "record": notification$0.Record;
+    "delivered": string[] | null;
+}

frontend/bindings/github.com/alicoding/mill/internal/services/notificationsvc/notificationservice.ts

@@ -0,0 +1,63 @@
+// Cynhyrchwyd y ffeil hon yn awtomatig. PEIDIWCH Â MODIWL
+// This file is automatically generated. DO NOT EDIT
+
+/**
+ * NotificationService is the Wails-bound owner of the notification
+ * spine: the persisted Record slice and the registered Channel list
+ * Publish fans out to.
+ * @module
+ */
+
+// eslint-disable-next-line @typescript-eslint/ban-ts-comment
+// @ts-ignore: Unused imports
+import { Call as $Call, CancellablePromise as $CancellablePromise } from "@wailsio/runtime";
+
+// eslint-disable-next-line @typescript-eslint/ban-ts-comment
+// @ts-ignore: Unused imports
+import * as notification$0 from "../../domain/notification/models.js";
+
+// eslint-disable-next-line @typescript-eslint/ban-ts-comment
+// @ts-ignore: Unused imports
+import * as $models from "./models.js";
+
+/**
+ * ListNotifications returns every persisted Record, newest first --
+ * the read side of the silent-loss fix (docs/goals/0171): an event
+ * published with no window/tab open is still here once one opens.
+ */
+export function ListNotifications(): $CancellablePromise<notification$0.Record[] | null> {
+    return $Call.ByID(1795017620);
+}
+
+/**
+ * MarkRead stamps id's ReadAt to now, a no-op if it's already set --
+ * idempotent so a duplicate click/retry never overwrites an earlier
+ * real read time with a later one.
+ */
+export function MarkRead(id: string): $CancellablePromise<void> {
+    return $Call.ByID(3359623529, id);
+}
+
+/**
+ * Publish is the notification spine's one entry point (docs/goals/0171
+ * item 2): PERSISTS FIRST, then fans out. A repeat call for a
+ * DedupeKey that already has a Record does not create a second one --
+ * it resolves to the existing Record and re-attempts only the channels
+ * that haven't delivered it yet, which is what makes Publish safe to
+ * call from more than one producer for the same logical event (a
+ * backend origination point and a frontend caller both naming the same
+ * DedupeKey) without double-delivering through either channel.
+ * 
+ * The existence check and the insert-if-absent below are one hand-
+ * written critical section rather than a call to entitystore.Insert:
+ * Insert acquires its own lock internally, so calling it while already
+ * holding s.mu (needed to make "check, then insert" atomic) would
+ * deadlock on Go's non-reentrant sync.Mutex. This is the one place
+ * entitystore's generic shape does not fit -- Insert is an
+ * unconditional-append primitive, and Publish needs insert-if-absent.
+ * entitystore.Persist/Update/Load are still used for everything else
+ * below.
+ */
+export function Publish(evt: notification$0.Event): $CancellablePromise<$models.PublishResult> {
+    return $Call.ByID(2392278483, evt);
+}

frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts

@@ -269,6 +269,25 @@ export function GetWorkflowMinutesSaved(workflowID: string): $CancellablePromise
     return $Call.ByID(1967529281, workflowID);
 }
 
+/**
+ * IsAway is the ONE presence-gate decision point in the tree (docs/
+ * goals/0171): present = focused AND recently-active (idle below the
+ * configured threshold); away = anything else. Exported so every
+ * notification channel (settingsservice_notifychannels.go) and the
+ * browser tab (via this same method, bound as an RPC) share this one
+ * definition instead of each re-deriving their own -- the frontend's
+ * own focus-only predicate (goal 0132 slice A's shouldNotifyBrowserTab)
+ * existed only because there was no shared gate to call into; it is
+ * gone now that there is one. An idletime read error (server mode, or
+ * a real desktop failing to read the counter) FAILS TOWARD AWAY -- §8's
+ * fail-safe posture: a truly-away user missing a decision is the
+ * failure that matters, not a present user seeing one extra
+ * notification.
+ */
+export function IsAway(focused: boolean): $CancellablePromise<boolean> {
+    return $Call.ByID(3618695826, focused);
+}
+
 /**
  * IsIsolatedData reports whether this instance is running against a
  * non-default settings path (MILL_SETTINGS_PATH was set) -- see
@@ -319,20 +338,25 @@ export function MCPAccessAddressInfo(): $CancellablePromise<$models.MCPAddrInfo>
 }
 
 /**
- * NotifyPendingApproval sends an actionable OS notification AND shows
- * the floating approval prompt (docs/goals/0023 item 1) for a new
- * pending item (docs/adr/0032 §3), but ONLY when isAway(focused) says
- * the user is away -- the single decision point both surfaces share, so
- * "notify" and "show the floating prompt" can never disagree about
- * presence. focused is the caller's own document.hasFocus() reading
- * (App.tsx) -- only the browser context knows that; everything else
- * about presence (idle time, the threshold) is resolved in isAway.
+ * NotifyPendingApproval publishes a durable notification for a new
+ * pending item (docs/goals/0171) and shows the floating approval
+ * prompt, but ONLY when IsAway(focused) says the user is away -- the
+ * single decision point every surface shares, so "notify" and "show
+ * the floating prompt" can never disagree about presence. focused is
+ * the caller's own document.hasFocus() reading (App.tsx) -- only the
+ * browser context knows that; everything else about presence (idle
+ * time, the threshold) is resolved in IsAway.
  * 
- * kind "mcp-write" gets Approve/Deny action buttons resolving directly
- * via ResolveMCPWrite; any other kind (a guardrail/human-review park)
- * gets a plain notification whose default click shows+focuses the main
- * window instead -- typed input may be required to resolve those, so
- * blind approval from a notification isn't offered.
+ * The desktop banner and dock bounce below now run through Publish's
+ * channel registry (settingsservice_notifychannels.go) rather than
+ * calling notify.Send* /dockBounceFn directly -- same observable calls,
+ * same away verdict, just expressed as registered channels so a future
+ * channel is one new struct, not a new branch here. kind "mcp-write"
+ * still gets Approve/Deny action buttons resolving via ResolveMCPWrite
+ * (desktopBannerChannel.Deliver's own branch); any other kind gets a
+ * plain notification whose default click shows+focuses the main window
+ * -- typed input may be required to resolve those, so blind approval
+ * from a notification isn't offered.
  */
 export function NotifyPendingApproval(id: string, description: string, kind: string, focused: boolean): $CancellablePromise<void> {
     return $Call.ByID(99139683, id, description, kind, focused);
@@ -639,17 +663,10 @@ export function ShowPanel(): $CancellablePromise<void> {
  * (AssignHotkey/CheckConflict) stay scoped to per-workflow bindings and
  * have no reason to know the native application menu exists.
  * 
- * Server-mode-safe by construction, not just by nil-guard: every
- * menu_*.go in Wails3's own pkg/application (the package that defines
- * DefaultApplicationMenu, Menu.Update's native half, etc.) is
- * //go:build !server -- calling those symbols unconditionally from this
- * package would fail to *compile* under `-tags server`, not just
- * misbehave at runtime. applicationMenu (settingsservice_menu_desktop.go
- * / settingsservice_menu_server.go) is the same !server/server split
- * internal/adapters/hotkey and internal/adapters/launchatlogin already
- * use for the identical reason (no native run loop / no native menu
- * bar in server mode) -- the server build's applicationMenu always
- * returns nil, so both methods below degrade to a safe no-op there.
+ * The actual native suspend/restore/release calls -- and the
+ * server-mode no-op degrade -- live in internal/adapters/windowing;
+ * this only counts concurrent recorders and calls the adapter exactly
+ * once at the 0->1 and 1->0 transitions.
  */
 export function SuspendMenuAccelerators(): $CancellablePromise<void> {
     return $Call.ByID(2098787179);

frontend/src/app/App.tsx

@@ -266,7 +266,7 @@ function App() {
 
   useMillNavigate(setView);
 
-  const notifyBrowserTab = useBrowserNotify(buildInfo?.Server === true);
+  const notifyBrowserTab = useBrowserNotify();
 
   useEffect(() => {
     return Events.On('hotkey-activity', (evt) => {

frontend/src/app/browserNotifyPredicate.test.ts

@@ -1,6 +1,6 @@
-import { useCallback, useRef } from 'react'
+import { useCallback } from 'react'
 import { getNotificationPermission, raiseNotification } from '../shared/browserNotify'
-import { shouldNotifyBrowserTab } from './browserNotifyPredicate'
+import { NotificationService } from '../shared/bindings'
 
 export interface BrowserNotifyRequest {
   // Identifies the thing being notified about (a run id, a pending
@@ -15,21 +15,26 @@ export interface BrowserNotifyRequest {
   onClick: () => void
 }
 
-// docs/goals/0132-remote-access.md SLICE A: the one browser-tab
-// notification seam every consumer goes through, never a bespoke
-// notifier per event type. A new event that wants this (a finished
-// run, an agent action while away) is a new call site with its own
-// dedupeKey/title/body/onClick -- never a change here. The parked-
-// approval call in App.tsx is this seam's first consumer, not its only
-// intended one.
-export function useBrowserNotify(isServerMode: boolean) {
-  const notifiedKeys = useRef<Set<string>>(new Set())
-
+// docs/goals/0171-notification-spine.md: the browser-tab channel's
+// client half. The gate (server mode, presence) and the dedupe (has
+// this dedupeKey already been delivered through the "browser-tab"
+// channel) both live server-side now -- NotificationService.Publish is
+// the same one entry point every producer goes through, so this hook
+// only decides whether to raise the actual `Notification`, based on
+// whether Publish's own response says "browser-tab" newly delivered on
+// THIS call. Replaces goal 0132 slice A's local shouldNotifyBrowserTab
+// predicate (focus-only, no idle) and its in-memory notifiedKeys Set
+// (reset on reload) -- both are gone, not just unused.
+export function useBrowserNotify() {
   return useCallback((req: BrowserNotifyRequest) => {
-    const alreadyNotified = notifiedKeys.current.has(req.dedupeKey)
-    if (!shouldNotifyBrowserTab({ isServerMode, hasFocus: document.hasFocus(), alreadyNotified })) return
     if (getNotificationPermission() !== 'granted') return
-    notifiedKeys.current.add(req.dedupeKey)
-    raiseNotification(req.title, req.body, req.onClick)
-  }, [isServerMode])
+    NotificationService.Publish({
+      type: 'guardrail', title: req.title, body: req.body,
+      dedupeKey: req.dedupeKey, sourceRef: req.dedupeKey, focused: document.hasFocus(),
+    }).then((result) => {
+      if (result.delivered?.includes('browser-tab')) {
+        raiseNotification(req.title, req.body, req.onClick)
+      }
+    }).catch(() => {})
+  }, [])
 }