diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 59c27838f..43d724dd9 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -639,6 +639,15 @@ installer download and the agent's subsequent Pulse TLS connection. 64. `pkg/securityutil/httpurl.go` ## Shared Boundaries + +### Notification recovery reload ownership + +Notification recovery handler ownership is refreshed alongside agent and +ordinary notification handlers when the router monitor is replaced. This does +not alter agent admission, reporting or removal; stopping an old notifier must +not strand queue controls on its cleared queue. Requests overlapping shutdown +may fail transiently. + ### Container update receipt and independent observation The server's independent Docker-update verification must compare the daemon diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 451ee0800..b07cac148 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -434,6 +434,16 @@ single TypeScript projection rather than recreating lifecycle or evidence enums locally. ## Shared Boundaries + +### Notification recovery reload ownership + +Queue recovery routes retain stable handler objects across router reloads, but +both router monitor replacement entry points refresh their queue owner. Missing +monitor or notifier returns 503; successful post-reload stats, Retry and Dismiss +use the replacement notifier. Existing method, administrator and token-scope +gates are unchanged. `TestRouterSetMonitorRefreshesNotificationQueue` records +the stopped-owner 503 regression and replacement-owner success. + ### Independent Docker update readback `dockerContainerUpdateExecutionResult` must not promote replacement-ID equality diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index 7dc0f6c02..e03b33bce 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -15,6 +15,25 @@ ## Purpose +### Queue recovery handler ownership after reload + +Router monitor replacement must refresh the existing queue/DLQ handler as well +as the normal notification handler. A stopped manager intentionally clears its +queue; retaining that manager after reload must not leave Retry, Dismiss or +queue reads permanently returning `Notification queue not initialized` while +the replacement manager is available. Handler monitor reads and replacement +are synchronised; missing monitors/managers remain an explicit 503, not a panic. +This changes neither queue state semantics nor notification delivery transport. +Requests already in flight during shutdown may still fail and can be retried. + +Verification: `TestRouterSetMonitorRefreshesNotificationQueue` stops the old +notifier, replaces the router monitor, and exercises stats, Retry and Dismiss; +`TestNotificationQueueHandlers_MonitorReplacementConcurrent` covers concurrent +replacement and unavailable-manager reads with the race detector. This is +lifecycle regression proof, not installed SMTP receipt or reporter confirmation. + + + Own notification delivery transport, provider configuration, queueing, and notification-management API surfaces. The alert schedule selects firing, grouped, and matching recovery delivery diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index b65c0e14f..4b562ddfc 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -166,6 +166,14 @@ the existing bounded HTTP client and artifact-size checks. ## Shared Boundaries +### Recovery queue owner replacement + +Recovery queue requests take a handler-local read lock only to snapshot the +monitor pointer; queue operations execute after releasing that lock. Router +replacement takes the matching write lock without recreating registered +handlers. The focused race proof covers owner replacement, not throughput or +release latency qualification; existing adverse benchmark evidence is unchanged. + 1. `frontend-modern/src/components/Infrastructure/infrastructureSelectors.ts` shared with `unified-resources`: the infrastructure selector pipeline is both a canonical unified-resource consumer surface and a fleet-scale performance hot-path boundary. 2. `frontend-modern/src/components/Infrastructure/resourceDetailMappers.ts` shared with `unified-resources`: resource detail mappers are both a canonical unified-resource consumer surface and a fleet-scale performance hot-path boundary. 3. `frontend-modern/src/components/Infrastructure/UnifiedResourceHostTableCard.tsx` shared with `unified-resources`: the unified resource host table card is both a canonical unified-resource consumer surface and a fleet-scale performance hot-path boundary. diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 794dd1927..a8942cd50 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -278,6 +278,15 @@ invisible to operators reviewing exactly what Pulse sends. ## Shared Boundaries +### Notification recovery reload ownership + +Refreshing the notification queue handler on reload changes only its default +monitor reference, under a handler-local lock. It does not remove route +administrator or settings-scope checks, expose queue contents to new callers, or +change tenant selection. Missing owners fail with 503 rather than dereferencing +nil. + + API token scope copy must match runtime authority. `ai:chat` covers Assistant conversation, model selection, sessions, and knowledge reads only. Knowledge save/delete/import/clear and explicit governed action approval/execution diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 166971ad3..5c7bb70c0 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -253,6 +253,15 @@ command-capable profile. 34. `frontend-modern/src/components/Storage/useStoragePoolsTableWindowing.ts` ## Shared Boundaries + +### Notification recovery reload ownership + +Queue recovery API handlers follow router monitor replacement instead of +retaining a stopped notifier. Retry and Dismiss retain their existing +persistence operations and history semantics; this repair does not delete or +rewrite recovery stores, change database internals, or establish crash-recovery +qualification. + ### Shared Docker-update verification boundary The shared API result converter classifies independent Docker update readback diff --git a/internal/api/alerting/notification_queue.go b/internal/api/alerting/notification_queue.go index ed085f77e..141c0b87e 100644 --- a/internal/api/alerting/notification_queue.go +++ b/internal/api/alerting/notification_queue.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "strconv" + "sync" "github.com/rcourtman/pulse-go-rewrite/internal/api/apihttp" "github.com/rcourtman/pulse-go-rewrite/internal/config" @@ -15,6 +16,7 @@ import ( // NotificationQueueHandlers handles notification queue API endpoints type NotificationQueueHandlers struct { + mu sync.RWMutex monitor *monitoring.Monitor } @@ -25,6 +27,24 @@ func NewNotificationQueueHandlers(monitor *monitoring.Monitor) *NotificationQueu } } +// SetMonitor refreshes the queue owner after a configuration reload. Keep the +// handler itself stable: registered routes retain its method values. +func (h *NotificationQueueHandlers) SetMonitor(monitor *monitoring.Monitor) { + h.mu.Lock() + h.monitor = monitor + h.mu.Unlock() +} + +func (h *NotificationQueueHandlers) getQueue() *notifications.NotificationQueue { + h.mu.RLock() + monitor := h.monitor + h.mu.RUnlock() + if monitor == nil || monitor.GetNotificationManager() == nil { + return nil + } + return monitor.GetNotificationManager().GetQueue() +} + // GetDLQ returns notifications in the dead letter queue func (h *NotificationQueueHandlers) GetDLQ(w http.ResponseWriter, r *http.Request) { if !apihttp.EnsureScope(w, r, config.ScopeMonitoringRead) { @@ -38,7 +58,7 @@ func (h *NotificationQueueHandlers) GetDLQ(w http.ResponseWriter, r *http.Reques } } - queue := h.monitor.GetNotificationManager().GetQueue() + queue := h.getQueue() if queue == nil { http.Error(w, "Notification queue not initialized", http.StatusServiceUnavailable) return @@ -62,7 +82,7 @@ func (h *NotificationQueueHandlers) GetQueueStats(w http.ResponseWriter, r *http return } - queue := h.monitor.GetNotificationManager().GetQueue() + queue := h.getQueue() if queue == nil { http.Error(w, "Notification queue not initialized", http.StatusServiceUnavailable) return @@ -103,7 +123,7 @@ func (h *NotificationQueueHandlers) RetryDLQItem(w http.ResponseWriter, r *http. return } - queue := h.monitor.GetNotificationManager().GetQueue() + queue := h.getQueue() if queue == nil { http.Error(w, "Notification queue not initialized", http.StatusServiceUnavailable) return @@ -150,7 +170,7 @@ func (h *NotificationQueueHandlers) DeleteDLQItem(w http.ResponseWriter, r *http return } - queue := h.monitor.GetNotificationManager().GetQueue() + queue := h.getQueue() if queue == nil { http.Error(w, "Notification queue not initialized", http.StatusServiceUnavailable) return @@ -180,7 +200,7 @@ func (h *NotificationQueueHandlers) RetryTerminalFailures(w http.ResponseWriter, if !apihttp.EnsureScope(w, r, config.ScopeSettingsWrite) { return } - queue := h.monitor.GetNotificationManager().GetQueue() + queue := h.getQueue() if queue == nil { http.Error(w, "Notification queue not initialized", http.StatusServiceUnavailable) return @@ -205,7 +225,7 @@ func (h *NotificationQueueHandlers) DismissTerminalFailures(w http.ResponseWrite if !apihttp.EnsureScope(w, r, config.ScopeSettingsWrite) { return } - queue := h.monitor.GetNotificationManager().GetQueue() + queue := h.getQueue() if queue == nil { http.Error(w, "Notification queue not initialized", http.StatusServiceUnavailable) return diff --git a/internal/api/alerting/notification_queue_error_test.go b/internal/api/alerting/notification_queue_error_test.go index ef832bd2f..82d3e703a 100644 --- a/internal/api/alerting/notification_queue_error_test.go +++ b/internal/api/alerting/notification_queue_error_test.go @@ -133,3 +133,24 @@ func TestNotificationQueueHandlers_GetDLQ_InvalidLimit(t *testing.T) { t.Fatalf("decode dlq: %v", err) } } + +func TestNotificationQueueHandlers_MonitorReplacementConcurrent(t *testing.T) { + h := NewNotificationQueueHandlers(nil) + m := &monitoring.Monitor{} + done := make(chan struct{}) + go func() { + defer close(done) + for i := 0; i < 100; i++ { + h.SetMonitor(m) + h.SetMonitor(nil) + } + }() + for i := 0; i < 100; i++ { + rec := httptest.NewRecorder() + h.GetQueueStats(rec, httptest.NewRequest(http.MethodGet, "/", nil)) + if rec.Code != http.StatusServiceUnavailable { + t.Errorf("unavailable monitor: got %d", rec.Code) + } + } + <-done +} diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 4a766f2e6..0b611b5a8 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -46,6 +46,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/mock" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/internal/monitoring" + "github.com/rcourtman/pulse-go-rewrite/internal/notifications" "github.com/rcourtman/pulse-go-rewrite/internal/operationreceipt" "github.com/rcourtman/pulse-go-rewrite/internal/recovery" "github.com/rcourtman/pulse-go-rewrite/internal/relay" @@ -24750,3 +24751,49 @@ func TestContract_NotificationDestinationWritesPublishOnlyAfterPersistence(t *te }) } } + +// Recovery controls must follow the replacement notifier, not the stopped one. +func TestRouterSetMonitorRefreshesNotificationQueue(t *testing.T) { + for _, entry := range []string{"monitor", "multi-tenant-monitor"} { + t.Run(entry, func(t *testing.T) { + oldMonitor, _, _ := newTestMonitor(t) + replacement, _, _ := newTestMonitor(t) + for _, m := range []*monitoring.Monitor{oldMonitor, replacement} { + n := notifications.NewNotificationManagerWithDataDir("", t.TempDir()) + t.Cleanup(n.Stop) + setUnexportedField(t, m, "notificationMgr", n) + } + h := NewNotificationQueueHandlers(oldMonitor) + // Capture method values before replacement just as route registration does. + handlers := map[string]http.HandlerFunc{ + "stats": h.GetQueueStats, "dlq": h.GetDLQ, + "retry": h.RetryTerminalFailures, "dismiss": h.DismissTerminalFailures, + } + r := &Router{config: &config.Config{}, notificationQueueHandlers: h} + oldMonitor.GetNotificationManager().Stop() + if entry == "monitor" { + r.SetMonitor(replacement) + } else { + mtm := &monitoring.MultiTenantMonitor{} + setUnexportedField(t, mtm, "monitors", map[string]*monitoring.Monitor{"default": replacement}) + r.SetMultiTenantMonitor(mtm) + } + if replacement.GetNotificationManager().GetQueue() == nil { + t.Fatal("replacement notifier unavailable") + } + for name, handler := range handlers { + t.Run(name, func(t *testing.T) { + rec := httptest.NewRecorder() + method := http.MethodPost + if name == "stats" || name == "dlq" { + method = http.MethodGet + } + handler(rec, httptest.NewRequest(method, "/", nil)) + if rec.Code != http.StatusOK { + t.Fatalf("replacement queue action: status %d: %s", rec.Code, rec.Body.String()) + } + }) + } + }) + } +} diff --git a/internal/api/notifications_scope_test.go b/internal/api/notifications_scope_test.go index cbda77c32..487049f2b 100644 --- a/internal/api/notifications_scope_test.go +++ b/internal/api/notifications_scope_test.go @@ -34,6 +34,8 @@ func TestNotificationReadEndpointsRequireSettingsReadScope(t *testing.T) { router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0") endpoints := []string{ + "/api/notifications/dlq", + "/api/notifications/queue/stats", "/api/notifications/email", "/api/notifications/apprise", "/api/notifications/webhooks", @@ -64,6 +66,10 @@ func TestNotificationWriteEndpointsRequireSettingsWriteScope(t *testing.T) { method string path string }{ + {http.MethodPost, "/api/notifications/dlq/retry"}, + {http.MethodPost, "/api/notifications/dlq/delete"}, + {http.MethodPost, "/api/notifications/terminal-failures/retry"}, + {http.MethodPost, "/api/notifications/terminal-failures/dismiss"}, {http.MethodPut, "/api/notifications/email"}, {http.MethodPut, "/api/notifications/apprise"}, {http.MethodPost, "/api/notifications/webhooks"}, diff --git a/internal/api/router.go b/internal/api/router.go index 9231c6f04..c5d817991 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -1564,6 +1564,9 @@ func (r *Router) startLifecycleWorker(worker func()) { // SetMonitor updates the router and associated handlers with a new monitor instance. func (r *Router) SetMonitor(m *monitoring.Monitor) { + if r.notificationQueueHandlers != nil { + r.notificationQueueHandlers.SetMonitor(m) + } r.monitor = m r.bindDefaultMetadataStores(m) r.configureMetadataProviderFactory() diff --git a/internal/api/router_helpers.go b/internal/api/router_helpers.go index 3bd61c017..024110d0d 100644 --- a/internal/api/router_helpers.go +++ b/internal/api/router_helpers.go @@ -135,6 +135,9 @@ func (r *Router) SetMultiTenantMonitor(mtm *monitoring.MultiTenantMonitor) { if mtm != nil { if m, err := mtm.GetMonitor("default"); err == nil { r.monitor = m + if r.notificationQueueHandlers != nil { + r.notificationQueueHandlers.SetMonitor(m) + } r.bindDefaultMetadataStores(m) } mtm.SetMonitorInitializer(r.configureMonitorDependencies)